powerpc: Add workaround for MPICs with broken register reads

PATCH Superseded Olof Johansson 2 participants 7 messages Sep 5, 2007 – Sep 7, 2007 PowerPC

Share & tools

Olof Johansson

author

2007-09-05 03:37 UTC #0 lore raw

PATCH 1/1 powerpc: Add workaround for MPICs with broken register reads

Some versions of PWRficient 1682M have an interrupt controller in which the first register in each pair for interrupt sources doesn't always read with the right polarity/sense values.

To work around this, keep a software copy of the register instead. Since it's not modified from the mpic itself, it's a feasible solution. Still, keep it under a config option to avoid wasting memory on other platforms.

Signed-off-by Olof Johansson <[email protected]>

arch/powerpc/platforms/Kconfig +4 -0

index 041df77..b9f1efa 100644
@@ -137,6 +137,10 @@ config MPIC_U3_HT_IRQS
 	depends on PPC_MAPLE
 	default y
 
+config MPIC_BROKEN_REGREAD
+	bool
+	depends on PPC_PASEMI
+
 config IBMVIO
 	depends on PPC_PSERIES || PPC_ISERIES
 	bool

arch/powerpc/platforms/pasemi/Kconfig +1 -0

index 95cd90f..117d90a 100644
@@ -5,6 +5,7 @@ config PPC_PASEMI
 	select MPIC
 	select PPC_UDBG_16550
 	select PPC_NATIVE
+	select MPIC_BROKEN_REGREAD
 	help
 	  This option enables support for PA Semi's PWRficient line
 	  of SoC processors, including PA6T-1682M

arch/powerpc/sysdev/mpic.c +12 -2

index 74c64c0..c0fe063 100644
@@ -228,8 +228,13 @@ 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;
 
-	return _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
+#endif
+		return _mpic_read(mpic->reg_type, &mpic->isus[isu],
+				  reg + (idx * MPIC_INFO(IRQ_STRIDE)));
 }
 
 static inline void _mpic_irq_write(struct mpic *mpic, unsigned int src_no,

@@ -240,6 +245,11 @@ static inline void _mpic_irq_write(struct mpic *mpic, unsigned int src_no,
 
 	_mpic_write(mpic->reg_type, &mpic->isus[isu],
 		    reg + (idx * MPIC_INFO(IRQ_STRIDE)), value);
+
+#ifdef CONFIG_MPIC_BROKEN_REGREAD
+	if (reg == 0)
+		mpic->isu_reg0_shadow[idx] = value;
+#endif
 }
 
 #define mpic_read(b,r)		_mpic_read(mpic->reg_type,&(b),(r))

include/asm-powerpc/mpic.h +4 -0

index 262db6b..c877fa7 100644
@@ -309,6 +309,10 @@ struct mpic
 	unsigned long		*hwirq_bitmap;
 #endif
 
+#ifdef CONFIG_MPIC_BROKEN_REGREAD
+	u32			isu_reg0_shadow[MPIC_MAX_IRQ_SOURCES];
+#endif
+
 	/* link */
 	struct mpic		*next;

Milton Miller

2007-09-06 14:54 UTC #1 lore raw

Quoted 20 lines · Olof Johansson

arch/powerpc/platforms/Kconfig +4 -0

index 041df77..b9f1efa 100644
@@ -137,6 +137,10 @@ config MPIC_U3_HT_IRQS
        depends on PPC_MAPLE
        default y
 
+config MPIC_BROKEN_REGREAD
+       bool
+       depends on PPC_PASEMI
+
 config IBMVIO
        depends on PPC_PSERIES || PPC_ISERIES
        bool

arch/powerpc/platforms/pasemi/Kconfig +0 -0

index 95cd90f..117d90a 100644

arch/powerpc/platforms/pasemi/Kconfig config PPC_PASEMI · line 11

Quoted 7 lines

arch/powerpc/platforms/pasemi/Kconfig +1 -0

@@ -5,6 +5,7 @@ config PPC_PASEMI
        select MPIC
        select PPC_UDBG_16550
        select PPC_NATIVE
+       select MPIC_BROKEN_REGREAD
        help
          This option enables support for PA Semi's PWRficient line
          of SoC processors, including PA6T-1682M

Since you are using select (and not default y), instead of the depends on PPC_PASEMI how about adding a short description of what the config does as help text, in case some future mpic has a similar bug?

milton

Olof Johansson

author

2007-09-07 09:16 UTC #2 lore raw

Quoted 27 lines · Milton Miller

Quoted 26 lines · Olof Johansson

arch/powerpc/platforms/Kconfig +4 -0

index 041df77..b9f1efa 100644
@@ -137,6 +137,10 @@ config MPIC_U3_HT_IRQS
        depends on PPC_MAPLE
        default y
 
+config MPIC_BROKEN_REGREAD
+       bool
+       depends on PPC_PASEMI
+
 config IBMVIO
        depends on PPC_PSERIES || PPC_ISERIES
        bool

arch/powerpc/platforms/pasemi/Kconfig +1 -0

index 95cd90f..117d90a 100644
@@ -5,6 +5,7 @@ config PPC_PASEMI
        select MPIC
        select PPC_UDBG_16550
        select PPC_NATIVE
+       select MPIC_BROKEN_REGREAD

Quoted 8 lines

arch/powerpc/platforms/pasemi/Kconfig config PPC_PASEMI · line 11
Quoted 3 lines
arch/powerpc/platforms/pasemi/Kconfig +0 -0
        help
          This option enables support for PA Semi's PWRficient line
          of SoC processors, including PA6T-1682M
Since you are using select (and not default y), instead of the depends on PPC_PASEMI how about adding a short description of what the config does as help text, in case some future mpic has a similar bug?

Thanks Milton, that's a reasonable suggestion (not that any of the other options do it, including the very vague MPIC_WEIRD :-).

New patch posted shortly.

-Olof

Olof Johansson

author

2007-09-07 09:21 UTC #3 lore raw

PATCH v2 1/1 powerpc: Add workaround for MPICs with broken register reads

In reply to Olof Johansson

Some versions of PWRficient 1682M have an interrupt controller in which the first register in each pair for interrupt sources doesn't always read with the right polarity/sense values.

Signed-off-by Olof Johansson <[email protected]>

arch/powerpc/platforms/Kconfig +12 -0

index 041df77..f2e7049 100644
@@ -137,6 +137,18 @@ config MPIC_U3_HT_IRQS
 	depends on PPC_MAPLE
 	default y
 
+config MPIC_BROKEN_REGREAD
+	bool "MPIC workaround for broken register reads"
+	depends on MPIC
+	help
+	  Say Y here to enable a MPIC driver workaround for some chips that
+	  have a bug that causes some interrupt source information to not
+	  read back properly. It is safe to use on other chips as well, but
+	  enabling it uses about 8KB of memory to keep copies of the register
+	  contents in software.
+
+	  Say N if you are unsure.
+
 config IBMVIO
 	depends on PPC_PSERIES || PPC_ISERIES
 	bool

arch/powerpc/platforms/pasemi/Kconfig +1 -0

index 95cd90f..117d90a 100644
@@ -5,6 +5,7 @@ config PPC_PASEMI
 	select MPIC
 	select PPC_UDBG_16550
 	select PPC_NATIVE
+	select MPIC_BROKEN_REGREAD
 	help
 	  This option enables support for PA Semi's PWRficient line
 	  of SoC processors, including PA6T-1682M

arch/powerpc/sysdev/mpic.c +12 -2

index 74c64c0..c0fe063 100644
@@ -228,8 +228,13 @@ 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;
 
-	return _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
+#endif
+		return _mpic_read(mpic->reg_type, &mpic->isus[isu],
+				  reg + (idx * MPIC_INFO(IRQ_STRIDE)));
 }
 
 static inline void _mpic_irq_write(struct mpic *mpic, unsigned int src_no,

@@ -240,6 +245,11 @@ static inline void _mpic_irq_write(struct mpic *mpic, unsigned int src_no,
 
 	_mpic_write(mpic->reg_type, &mpic->isus[isu],
 		    reg + (idx * MPIC_INFO(IRQ_STRIDE)), value);
+
+#ifdef CONFIG_MPIC_BROKEN_REGREAD
+	if (reg == 0)
+		mpic->isu_reg0_shadow[idx] = value;
+#endif
 }
 
 #define mpic_read(b,r)		_mpic_read(mpic->reg_type,&(b),(r))

include/asm-powerpc/mpic.h +4 -0

index 262db6b..c877fa7 100644
@@ -309,6 +309,10 @@ struct mpic
 	unsigned long		*hwirq_bitmap;
 #endif
 
+#ifdef CONFIG_MPIC_BROKEN_REGREAD
+	u32			isu_reg0_shadow[MPIC_MAX_IRQ_SOURCES];
+#endif
+
 	/* link */
 	struct mpic		*next;

Milton Miller

2007-09-07 14:16 UTC #4 lore raw

Quoted 22 lines · Olof Johansson

Some versions of PWRficient 1682M have an interrupt controller in which the first register in each pair for interrupt sources doesn't always read with the right polarity/sense values.
To work around this, keep a software copy of the register instead. Since it's not modified from the mpic itself, it's a feasible solution. Still, keep it under a config option to avoid wasting memory on other platforms.
Signed-off-by Olof Johansson <[email protected]>
arch/powerpc/platforms/Kconfig +3 -0
index 041df77..f2e7049 100644
@@ -137,6 +137,18 @@ config MPIC_U3_HT_IRQS
 	depends on PPC_MAPLE
 	default y
 
+config MPIC_BROKEN_REGREAD
+	bool "MPIC workaround for broken register reads"
+	depends on MPIC

arch/powerpc/platforms/Kconfig config MPIC_U3_HT_IRQS · line 152

Quoted 10 lines

arch/powerpc/platforms/Kconfig +9 -0

+	help
+	  Say Y here to enable a MPIC driver workaround for some chips that
+	  have a bug that causes some interrupt source information to not
+	  read back properly. It is safe to use on other chips as well, but
+	  enabling it uses about 8KB of memory to keep copies of the register
+	  contents in software.
+
+	  Say N if you are unsure.
+

I'm sorry, I didn't mean to imply it should be asked / selectable, just that the help should be provided. I won't object to leaving it except that it asks the average user unnecessary questions. In other words, drop the question after bool.

milton

Olof Johansson

author

2007-09-07 19:12 UTC #5 lore raw

Quoted 4 lines · Milton Miller

I'm sorry, I didn't mean to imply it should be asked / selectable, just
that the help should be provided. I won't object to leaving it except
that it asks the average user unnecessary questions. In other words, drop
the question after bool.

Sigh. Why is it always that the simple patches/changes require 3 respins? It's a valid point though, posting v3.

-Olof

Olof Johansson

author

2007-09-07 19:13 UTC #6 lore raw

PATCH v3 1/1 powerpc: Add workaround for MPICs with broken register reads

In reply to Olof Johansson

Some versions of PWRficient 1682M have an interrupt controller in which the first register in each pair for interrupt sources doesn't always read with the right polarity/sense values.

Signed-off-by Olof Johansson <[email protected]>

arch/powerpc/platforms/Kconfig +10 -0

index 041df77..22a8fa9 100644
@@ -137,6 +137,16 @@ config MPIC_U3_HT_IRQS
 	depends on PPC_MAPLE
 	default y
 
+config MPIC_BROKEN_REGREAD
+	bool
+	depends on MPIC
+	help
+	  This option enables a MPIC driver workaround for some chips
+	  that have a bug that causes some interrupt source information
+	  to not read back properly. It is safe to use on other chips as
+	  well, but enabling it uses about 8KB of memory to keep copies
+	  of the register contents in software.
+
 config IBMVIO
 	depends on PPC_PSERIES || PPC_ISERIES
 	bool

arch/powerpc/platforms/pasemi/Kconfig +1 -0

index 95cd90f..117d90a 100644
@@ -5,6 +5,7 @@ config PPC_PASEMI
 	select MPIC
 	select PPC_UDBG_16550
 	select PPC_NATIVE
+	select MPIC_BROKEN_REGREAD
 	help
 	  This option enables support for PA Semi's PWRficient line
 	  of SoC processors, including PA6T-1682M

arch/powerpc/sysdev/mpic.c +12 -2

index 74c64c0..c0fe063 100644
@@ -228,8 +228,13 @@ 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;
 
-	return _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
+#endif
+		return _mpic_read(mpic->reg_type, &mpic->isus[isu],
+				  reg + (idx * MPIC_INFO(IRQ_STRIDE)));
 }
 
 static inline void _mpic_irq_write(struct mpic *mpic, unsigned int src_no,

@@ -240,6 +245,11 @@ static inline void _mpic_irq_write(struct mpic *mpic, unsigned int src_no,
 
 	_mpic_write(mpic->reg_type, &mpic->isus[isu],
 		    reg + (idx * MPIC_INFO(IRQ_STRIDE)), value);
+
+#ifdef CONFIG_MPIC_BROKEN_REGREAD
+	if (reg == 0)
+		mpic->isu_reg0_shadow[idx] = value;
+#endif
 }
 
 #define mpic_read(b,r)		_mpic_read(mpic->reg_type,&(b),(r))

include/asm-powerpc/mpic.h +4 -0

index 262db6b..c877fa7 100644
@@ -309,6 +309,10 @@ struct mpic
 	unsigned long		*hwirq_bitmap;
 #endif
 
+#ifdef CONFIG_MPIC_BROKEN_REGREAD
+	u32			isu_reg0_shadow[MPIC_MAX_IRQ_SOURCES];
+#endif
+
 	/* link */
 	struct mpic		*next;