[PATCH] powerpc/mpic: Fix MPIC_BROKEN_REGREAD on non broken MPICs

[PATCH] powerpc/mpic: Fix MPIC_BROKEN_REGREAD on non broken MPICs

[Michael Ellerman]

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(-)

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
 }
 
-- 
1.6.2.1

Re: [PATCH] powerpc/mpic: Fix MPIC_BROKEN_REGREAD on non broken MPICs

[Olof Johansson]

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

Re: [PATCH] powerpc/mpic: Fix MPIC_BROKEN_REGREAD on non broken MPICs

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 969 bytes --]

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] powerpc/mpic: Fix MPIC_BROKEN_REGREAD on non broken MPICs

[Olof Johansson]

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

Re: [PATCH] powerpc/mpic: Fix MPIC_BROKEN_REGREAD on non broken MPICs

[Olof Johansson]

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