powerpc/mpic: Fix MPIC_BROKEN_REGREAD on non broken MPICs

Submitted by Michael Ellerman on July 6, 2009, 2:08 a.m.

Details

Message ID 6c9c0889feaceeea55bd2b6fef0c19e408425ef5.1246846125.git.michael@ellerman.id.au
State Accepted, archived
Commit 11a6b292c1bc9cb39970e44edd6958250f23d3a8
Delegated to: Benjamin Herrenschmidt
Headers show

Commit Message

Michael Ellerman July 6, 2009, 2:08 a.m.
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(-)

Comments

Olof Johansson July 6, 2009, 1:59 p.m.
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
Michael Ellerman July 7, 2009, 12:32 a.m.
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
Olof Johansson July 7, 2009, 8:20 a.m.
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
Olof Johansson July 19, 2009, 3:08 p.m.
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

Patch hide | download patch | download mbox

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
 }