Skip to contentSkip to conversation

ARM: shmobile: sh73a0 legacy: Set .control_parent for all irqpin instances

PATCH New Geert Uytterhoeven 4 participants 5 messages Jan 6, 2015 – Jan 8, 2015

Subsystems (4)
Arm (32-bit) Arm sub-architectures SuperH Arm/RISC-V/Renesas architecture
lore
Share & tools
lore

Geert Uytterhoeven

author maintainer
#0 lore raw

PATCH 1/1 ARM: shmobile: sh73a0 legacy: Set .control_parent for all irqpin instances

The sh73a0 INTC can't mask interrupts properly most likely due to a hardware bug. Set the .control_parent flag to delegate masking to the parent interrupt controller, like was already done for irqpin1.

Without this, accessing the three-axis digital accelerometer ADXL345 on kzm9g through /dev/input/event1 causes an interrupt storm, which requires a power-cycle to recover from.

This was inspired by a patch for arch/arm/boot/dts/sh73a0.dtsi from Laurent Pinchart <[email protected]>.

Signed-off-by Geert Uytterhoeven <[email protected]>
Fixes 341eb5465f67437a ("ARM: shmobile: INTC External IRQ pin driver on sh73a0")
---
 arch/arm/mach-shmobile/setup-sh73a0.c | 3 +++
 1 file changed, 3 insertions(+)
arch/arm/mach-shmobile/setup-sh73a0.c +3 -0
index 354cab111bf17ea6..613ce65184a6648c 100644
@@ -595,6 +595,7 @@ static struct platform_device ipmmu_device = {
 
 static struct renesas_intc_irqpin_config irqpin0_platform_data = {
 	.irq_base = irq_pin(0), /* IRQ0 -> IRQ7 */
+	.control_parent = true,
 };
 
 static struct resource irqpin0_resources[] = {

@@ -656,6 +657,7 @@ static struct platform_device irqpin1_device = {
 
 static struct renesas_intc_irqpin_config irqpin2_platform_data = {
 	.irq_base = irq_pin(16), /* IRQ16 -> IRQ23 */
+	.control_parent = true,
 };
 
 static struct resource irqpin2_resources[] = {

@@ -686,6 +688,7 @@ static struct platform_device irqpin2_device = {
 
 static struct renesas_intc_irqpin_config irqpin3_platform_data = {
 	.irq_base = irq_pin(24), /* IRQ24 -> IRQ31 */
+	.control_parent = true,
 };
 
 static struct resource irqpin3_resources[] = {
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 ARM: shmobile: sh73a0 legacy: Set .control_parent for all irqpin instances

Hi Geert,

Thank you for the patch.

Quoted 10 lines · On Tuesday 06 January 2015 14:39:10 Geert Uytterhoeven wrote:

The sh73a0 INTC can't mask interrupts properly most likely due to a hardware bug. Set the .control_parent flag to delegate masking to the parent interrupt controller, like was already done for irqpin1.

Without this, accessing the three-axis digital accelerometer ADXL345 on kzm9g through /dev/input/event1 causes an interrupt storm, which requires a power-cycle to recover from.

This was inspired by a patch for arch/arm/boot/dts/sh73a0.dtsi from Laurent Pinchart <[email protected]>.

This looks correct to me, but shouldn't we concentrate on getting rid of legacy code instead ?

Quoted 35 lines
Signed-off-by Geert Uytterhoeven <[email protected]>
Fixes 341eb5465f67437a ("ARM: shmobile: INTC External IRQ pin driver on

sh73a0") ---

 arch/arm/mach-shmobile/setup-sh73a0.c | 3 +++
 1 file changed, 3 insertions(+)
(quoted diff) +0 -0

b/arch/arm/mach-shmobile/setup-sh73a0.c index
354cab111bf17ea6..613ce65184a6648c 100644

arch/arm/mach-shmobile/setup-sh73a0.c +3 -0
@@ -595,6 +595,7 @@ static struct platform_device ipmmu_device = {
 
 static struct renesas_intc_irqpin_config irqpin0_platform_data = {
 	.irq_base = irq_pin(0), /* IRQ0 -> IRQ7 */
+	.control_parent = true,
 };
 
 static struct resource irqpin0_resources[] = {

@@ -656,6 +657,7 @@ static struct platform_device irqpin1_device = {
 
 static struct renesas_intc_irqpin_config irqpin2_platform_data = {
 	.irq_base = irq_pin(16), /* IRQ16 -> IRQ23 */
+	.control_parent = true,
 };
 
 static struct resource irqpin2_resources[] = {

@@ -686,6 +688,7 @@ static struct platform_device irqpin2_device = {
 
 static struct renesas_intc_irqpin_config irqpin3_platform_data = {
 	.irq_base = irq_pin(24), /* IRQ24 -> IRQ31 */
+	.control_parent = true,
 };
 
 static struct resource irqpin3_resources[] = {
Signature · 3 lines
Regards,

Laurent Pinchart
#2 lore raw

PATCH 1/1 ARM: shmobile: sh73a0 legacy: Set .control_parent for all irqpin instances

Quoted 13 lines · Laurent Pinchart

Hi Geert,

Thank you for the patch.

Quoted 8 lines · On Tuesday 06 January 2015 14:39:10 Geert Uytterhoeven wrote:

The sh73a0 INTC can't mask interrupts properly most likely due to a hardware bug. Set the .control_parent flag to delegate masking to the parent interrupt controller, like was already done for irqpin1.

Without this, accessing the three-axis digital accelerometer ADXL345 on kzm9g through /dev/input/event1 causes an interrupt storm, which requires a power-cycle to recover from.

Quoted 5 lines
Quoted 2 lines

This was inspired by a patch for arch/arm/boot/dts/sh73a0.dtsi from Laurent Pinchart <[email protected]>.

This looks correct to me, but shouldn't we concentrate on getting rid of legacy code instead ?

Probably, but as we now have this patch I'm inclined to apply it.

Quoted 41 lines
Quoted 35 lines
Signed-off-by Geert Uytterhoeven <[email protected]>
Fixes 341eb5465f67437a ("ARM: shmobile: INTC External IRQ pin driver on

sh73a0") ---

 arch/arm/mach-shmobile/setup-sh73a0.c | 3 +++
 1 file changed, 3 insertions(+)
(quoted diff) +0 -0

b/arch/arm/mach-shmobile/setup-sh73a0.c index
354cab111bf17ea6..613ce65184a6648c 100644

arch/arm/mach-shmobile/setup-sh73a0.c +3 -0
@@ -595,6 +595,7 @@ static struct platform_device ipmmu_device = {
 
 static struct renesas_intc_irqpin_config irqpin0_platform_data = {
 	.irq_base = irq_pin(0), /* IRQ0 -> IRQ7 */
+	.control_parent = true,
 };
 
 static struct resource irqpin0_resources[] = {

@@ -656,6 +657,7 @@ static struct platform_device irqpin1_device = {
 
 static struct renesas_intc_irqpin_config irqpin2_platform_data = {
 	.irq_base = irq_pin(16), /* IRQ16 -> IRQ23 */
+	.control_parent = true,
 };
 
 static struct resource irqpin2_resources[] = {

@@ -686,6 +688,7 @@ static struct platform_device irqpin2_device = {
 
 static struct renesas_intc_irqpin_config irqpin3_platform_data = {
 	.irq_base = irq_pin(24), /* IRQ24 -> IRQ31 */
+	.control_parent = true,
 };
 
 static struct resource irqpin3_resources[] = {
Signature · 3 lines
Regards,

Laurent Pinchart
#3 lore raw

PATCH 1/1 ARM: shmobile: sh73a0 legacy: Set .control_parent for all irqpin instances

In reply to Laurent Pinchart

Hi Laurent,

Quoted 9 lines · Laurent Pinchart
Quoted 8 lines · On Tuesday 06 January 2015 14:39:10 Geert Uytterhoeven wrote:

The sh73a0 INTC can't mask interrupts properly most likely due to a hardware bug. Set the .control_parent flag to delegate masking to the parent interrupt controller, like was already done for irqpin1.

Without this, accessing the three-axis digital accelerometer ADXL345 on kzm9g through /dev/input/event1 causes an interrupt storm, which requires a power-cycle to recover from.

Quoted 5 lines
Quoted 2 lines

This was inspired by a patch for arch/arm/boot/dts/sh73a0.dtsi from Laurent Pinchart <[email protected]>.

This looks correct to me, but shouldn't we concentrate on getting rid of legacy code instead ?

Sure.

But this is a serious bug, which allows to crash the kernel from user space. Worse, a simple reset is not sufficient to recover. You need to power-cycle the board.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#4 lore raw

PATCH 1/1 ARM: shmobile: sh73a0 legacy: Set .control_parent for all irqpin instances

Quoted 16 lines · Geert Uytterhoeven

Hi Laurent,

Quoted 12 lines · Laurent Pinchart
Quoted 10 lines · On Tuesday 06 January 2015 14:39:10 Geert Uytterhoeven wrote:

The sh73a0 INTC can't mask interrupts properly most likely due to a hardware bug. Set the .control_parent flag to delegate masking to the parent interrupt controller, like was already done for irqpin1.

Without this, accessing the three-axis digital accelerometer ADXL345 on kzm9g through /dev/input/event1 causes an interrupt storm, which requires a power-cycle to recover from.

This was inspired by a patch for arch/arm/boot/dts/sh73a0.dtsi from Laurent Pinchart <[email protected]>.

Quoted 8 lines
Quoted 2 lines

This looks correct to me, but shouldn't we concentrate on getting rid of legacy code instead ?

Sure.

But this is a serious bug, which allows to crash the kernel from user space. Worse, a simple reset is not sufficient to recover. You need to power-cycle the board.

With that in mind I have queued this up as a fix for v3.19.

For (my) reference, 341eb5465f67437a ("ARM: shmobile: INTC External IRQ pin driver on sh73a0"), which this patch fixes, was part of v3.9.