| Message ID | 1245824392.9237.85.camel@concordia (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wed, 2009-06-24 at 16:19 +1000, Michael Ellerman wrote: > sym53c8xx 0000:00:0c.0: enabling device (0140 -> 0143) > sym0: <896> rev 0x7 at pci 0000:00:0c.0 irq 17 > sym0: No NVRAM, ID 7, Fast-40, SE, parity checking > _mpic_irq_read: val 0x80480004 shadow 0x80080014 > _mpic_irq_read: val 0x480004 shadow 0x480004 Strange.. it's like we read a different vector and polarity than what is in the cache. Oh well, I'll have a closer look tomorrow in the office. Ben.
Hello! Thank you very much, this patch works me too. Maybe this patch will be in the debian kernel someday? Thank you: blackluck Michael Ellerman wrote: > On Wed, 2009-06-24 at 15:56 +1000, Benjamin Herrenschmidt wrote: > >> On Wed, 2009-06-24 at 15:53 +1000, Michael Ellerman wrote: >> >>> Doesn't fix my machine :/ >>> >>> >> That doesn't make sense ... What if you remove the bit inside the ifdef >> CONFIG_MPIC_BROKEN_REGREAD in _mpic_read() ? >> >> If that makes a difference, then it would be interesting to add a printk >> in there that prints what the original value "val" is and what we have >> in the shadow... >> > > With this patch it boots: > > diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c > index 2353adc..fc17289 100644 > --- a/arch/powerpc/sysdev/mpic.c > +++ b/arch/powerpc/sysdev/mpic.c > @@ -231,13 +231,16 @@ static inline u32 _mpic_irq_read(struct mpic *mpic, unsign > unsigned int isu = src_no >> mpic->isu_shift; > unsigned int idx = src_no & mpic->isu_mask; > unsigned int val; > + unsigned int shadow; > > val = _mpic_read(mpic->reg_type, &mpic->isus[isu], > reg + (idx * MPIC_INFO(IRQ_STRIDE))); > #ifdef CONFIG_MPIC_BROKEN_REGREAD > - if (reg == 0) > - val = (val & (MPIC_VECPRI_MASK | MPIC_VECPRI_ACTIVITY)) | > + if (reg == 0) { > + shadow = (val & (MPIC_VECPRI_MASK | MPIC_VECPRI_ACTIVITY)) | > mpic->isu_reg0_shadow[idx]; > + printk("%s: val 0x%x shadow 0x%x\n", __func__, val, shadow); > + } > #endif > return val; > } > > > And I see: > > sym53c8xx 0000:00:0c.0: enabling device (0140 -> 0143) > sym0: <896> rev 0x7 at pci 0000:00:0c.0 irq 17 > sym0: No NVRAM, ID 7, Fast-40, SE, parity checking > _mpic_irq_read: val 0x80480004 shadow 0x80080014 > _mpic_irq_read: val 0x480004 shadow 0x480004 > > > > cheers >
On Sat, 2009-06-27 at 19:46 +0200, Laszlo Fekete wrote: > Hello! > > Thank you very much, this patch works me too. > > Maybe this patch will be in the debian kernel someday? The patch isn't actually correct just yet :-) Michael will be posting a proper one next week. It should be possible to request its inclusion into debian separately, we'll probably send it to stable@kernel.org as well. Cheers, Ben. > Thank you: blackluck > > Michael Ellerman wrote: > > On Wed, 2009-06-24 at 15:56 +1000, Benjamin Herrenschmidt wrote: > > > > > On Wed, 2009-06-24 at 15:53 +1000, Michael Ellerman wrote: > > > > > > > Doesn't fix my machine :/ > > > > > > > > > > > That doesn't make sense ... What if you remove the bit inside the ifdef > > > CONFIG_MPIC_BROKEN_REGREAD in _mpic_read() ? > > > > > > If that makes a difference, then it would be interesting to add a printk > > > in there that prints what the original value "val" is and what we have > > > in the shadow... > > > > > > > With this patch it boots: > > > > diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c > > index 2353adc..fc17289 100644 > > --- a/arch/powerpc/sysdev/mpic.c > > +++ b/arch/powerpc/sysdev/mpic.c > > @@ -231,13 +231,16 @@ static inline u32 _mpic_irq_read(struct mpic *mpic, unsign > > unsigned int isu = src_no >> mpic->isu_shift; > > unsigned int idx = src_no & mpic->isu_mask; > > unsigned int val; > > + unsigned int shadow; > > > > val = _mpic_read(mpic->reg_type, &mpic->isus[isu], > > reg + (idx * MPIC_INFO(IRQ_STRIDE))); > > #ifdef CONFIG_MPIC_BROKEN_REGREAD > > - if (reg == 0) > > - val = (val & (MPIC_VECPRI_MASK | MPIC_VECPRI_ACTIVITY)) | > > + if (reg == 0) { > > + shadow = (val & (MPIC_VECPRI_MASK | MPIC_VECPRI_ACTIVITY)) | > > mpic->isu_reg0_shadow[idx]; > > + printk("%s: val 0x%x shadow 0x%x\n", __func__, val, shadow); > > + } > > #endif > > return val; > > } > > > > > > And I see: > > > > sym53c8xx 0000:00:0c.0: enabling device (0140 -> 0143) > > sym0: <896> rev 0x7 at pci 0000:00:0c.0 irq 17 > > sym0: No NVRAM, ID 7, Fast-40, SE, parity checking > > _mpic_irq_read: val 0x80480004 shadow 0x80080014 > > _mpic_irq_read: val 0x480004 shadow 0x480004 > > > > > > > > cheers > >
On Sun, 2009-06-28 at 08:54 +1000, Benjamin Herrenschmidt wrote: > On Sat, 2009-06-27 at 19:46 +0200, Laszlo Fekete wrote: > > Hello! > > > > Thank you very much, this patch works me too. > > > > Maybe this patch will be in the debian kernel someday? > > The patch isn't actually correct just yet :-) Michael will > be posting a proper one next week. .. or the week after: http://patchwork.ozlabs.org/patch/29475/ cheers
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c index 2353adc..fc17289 100644 --- a/arch/powerpc/sysdev/mpic.c +++ b/arch/powerpc/sysdev/mpic.c @@ -231,13 +231,16 @@ static inline u32 _mpic_irq_read(struct mpic *mpic, unsign unsigned int isu = src_no >> mpic->isu_shift; unsigned int idx = src_no & mpic->isu_mask; unsigned int val; + unsigned int shadow; val = _mpic_read(mpic->reg_type, &mpic->isus[isu], reg + (idx * MPIC_INFO(IRQ_STRIDE))); #ifdef CONFIG_MPIC_BROKEN_REGREAD - if (reg == 0) - val = (val & (MPIC_VECPRI_MASK | MPIC_VECPRI_ACTIVITY)) | + if (reg == 0) { + shadow = (val & (MPIC_VECPRI_MASK | MPIC_VECPRI_ACTIVITY)) | mpic->isu_reg0_shadow[idx]; + printk("%s: val 0x%x shadow 0x%x\n", __func__, val, shadow); + } #endif return val; }