| Message ID | 6c9c0889feaceeea55bd2b6fef0c19e408425ef5.1246846125.git.michael@ellerman.id.au (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | 11a6b292c1bc9cb39970e44edd6958250f23d3a8 |
| Delegated to: | Benjamin Herrenschmidt |
| Headers | show |
On Mon, Jul 06, 2009 at 12:08:52PM +1000, Michael Ellerman wrote: > The workaround enabled by CONFIG_MPIC_BROKEN_REGREAD does not work > on non-broken MPICs. The symptom is no interrupts being received. > > The fix is twofold. Firstly the code was broken for multiple isus, > we need to index into the shadow array with the src_no, not the idx. > Secondly, we always do the read, but only use the VECPRI_MASK and > VECPRI_ACTIVITY bits from the hardware, the rest of "val" comes > from the shadow. I'm travelling without remote access to a machine to test this on. Given that it changes the errata workaround (subtly), I'd appreciate the chance to give it a go before it gets merged. Unfortunately I forgot to make sure that my remote console server was up before I left. :-) -Olof
On Mon, 2009-07-06 at 08:59 -0500, Olof Johansson wrote: > On Mon, Jul 06, 2009 at 12:08:52PM +1000, Michael Ellerman wrote: > > The workaround enabled by CONFIG_MPIC_BROKEN_REGREAD does not work > > on non-broken MPICs. The symptom is no interrupts being received. > > > > The fix is twofold. Firstly the code was broken for multiple isus, > > we need to index into the shadow array with the src_no, not the idx. > > Secondly, we always do the read, but only use the VECPRI_MASK and > > VECPRI_ACTIVITY bits from the hardware, the rest of "val" comes > > from the shadow. > > I'm travelling without remote access to a machine to test this on. Given > that it changes the errata workaround (subtly), I'd appreciate the chance > to give it a go before it gets merged. Sure. I was also wondering, was this workaround ever required in a released chip - or was it just dev samples? If it's the latter, do we still need to carry it at all? cheers
On Tue, Jul 07, 2009 at 10:32:33AM +1000, Michael Ellerman wrote: > I was also wondering, was this workaround ever required in a released > chip - or was it just dev samples? If it's the latter, do we still need > to carry it at all? It's needed on all versions of hardware but for slightly different reasons. I had to research it when the discussion came up, since the first time I used this workaround it was needed for a different erratum. But yes, it is there in shipping products -- I was usually pretty careful not to merge unneeded patches. -Olof
On Mon, Jul 06, 2009 at 12:08:52PM +1000, Michael Ellerman wrote: > The workaround enabled by CONFIG_MPIC_BROKEN_REGREAD does not work > on non-broken MPICs. The symptom is no interrupts being received. > > The fix is twofold. Firstly the code was broken for multiple isus, > we need to index into the shadow array with the src_no, not the idx. > Secondly, we always do the read, but only use the VECPRI_MASK and > VECPRI_ACTIVITY bits from the hardware, the rest of "val" comes > from the shadow. > > Signed-off-by: Michael Ellerman <michael@ellerman.id.au> Signed-off-by: Olof Johansson <olof@lixom.net> Tested OK here on an electra. Thanks for fixing this! -Olof
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c index 82e8e88..e35ecf5 100644 --- a/arch/powerpc/sysdev/mpic.c +++ b/arch/powerpc/sysdev/mpic.c @@ -230,14 +230,16 @@ static inline u32 _mpic_irq_read(struct mpic *mpic, unsigned int src_no, unsigne { unsigned int isu = src_no >> mpic->isu_shift; unsigned int idx = src_no & mpic->isu_mask; + unsigned int val; + val = _mpic_read(mpic->reg_type, &mpic->isus[isu], + reg + (idx * MPIC_INFO(IRQ_STRIDE))); #ifdef CONFIG_MPIC_BROKEN_REGREAD if (reg == 0) - return mpic->isu_reg0_shadow[idx]; - else + val = (val & (MPIC_VECPRI_MASK | MPIC_VECPRI_ACTIVITY)) | + mpic->isu_reg0_shadow[src_no]; #endif - return _mpic_read(mpic->reg_type, &mpic->isus[isu], - reg + (idx * MPIC_INFO(IRQ_STRIDE))); + return val; } static inline void _mpic_irq_write(struct mpic *mpic, unsigned int src_no, @@ -251,7 +253,8 @@ static inline void _mpic_irq_write(struct mpic *mpic, unsigned int src_no, #ifdef CONFIG_MPIC_BROKEN_REGREAD if (reg == 0) - mpic->isu_reg0_shadow[idx] = value; + mpic->isu_reg0_shadow[src_no] = + value & ~(MPIC_VECPRI_MASK | MPIC_VECPRI_ACTIVITY); #endif }
The workaround enabled by CONFIG_MPIC_BROKEN_REGREAD does not work on non-broken MPICs. The symptom is no interrupts being received. The fix is twofold. Firstly the code was broken for multiple isus, we need to index into the shadow array with the src_no, not the idx. Secondly, we always do the read, but only use the VECPRI_MASK and VECPRI_ACTIVITY bits from the hardware, the rest of "val" comes from the shadow. Signed-off-by: Michael Ellerman <michael@ellerman.id.au> --- arch/powerpc/sysdev/mpic.c | 13 ++++++++----- 1 files changed, 8 insertions(+), 5 deletions(-)