fix for certain sequnce of request_irq can cause irq storm

The problem is that hardware handled by arm/plat-orion/gpio.c, require ack for edge irq, and no ack for level irq.

Quoted 2 lines · Evgeniy Dushistov

The problem is that hardware handled by arm/plat-orion/gpio.c, require ack for edge irq, and no ack for level irq.

Hi Evgeniy

Thanks for the problem report and patch. What device are you using? An Orion5x based board or mv78xx0? Those are the only two left using this code.

Quoted 4 lines · Andrew Lunn

Quoted 2 lines · Evgeniy Dushistov

The problem is that hardware handled by arm/plat-orion/gpio.c, require ack for edge irq, and no ack for level irq.

Quoted 5 lines

Hi Evgeniy

Thanks for the problem report and patch. What device are you using? An Orion5x based board or mv78xx0? Those are the only two left using this code.

I used custom board with mv78200 CPU.

Quoted 1 line

I used custom board with mv78200 CPU.

So you're still using the mainline kernel on this system?

We were
actually starting to consider getting rid of the support for this platform, since it doesn't receive much attention, and there are almost no systems supported in mainline that use this CPU.

Are you actually using the two CPUs on mv78200 ? If so, in what configurations ?

Would you be willing to put some effort into moving mv78xx0 to the Device Tree, and all other modern subsystems (clock, pinmux, etc.) ?

Quoted 1 line

So you're still using the mainline kernel on this system?

Yes, we reuse old hardware design with new enougth (3.12) kernel.

Quoted 5 lines

We were
actually starting to consider getting rid of the support for this platform, since it doesn't receive much attention, and there are almost no systems supported in mainline that use this CPU.

Quoted 3 lines

Are you actually using the two CPUs on mv78200 ? If so, in what configurations ?

We use several configurations, on some of them the second cpu is disabled, on others the second CPU connected to the first via gigabit ethernet (eth2<->eth3 in loopback like mode [I fixed one bug several years ago about usage of such config, it should be in mainline]).

Quoted 3 lines

Would you be willing to put some effort into moving mv78xx0 to the Device Tree, and all other modern subsystems (clock, pinmux, etc.) ?

At now I have several git branches for slightly different device configurations, so I suppose support of Device Tree should simplify things for me.

But I can only test it on mv78200 CPU, and only
compiled into the kernel, because of u-boot from marvell old enough(no Device Tree support),
and not sure that mainstream u-boot support the usage of two cores.

Ok. And are you interested in seeing the mv78xx0 support being kept in the mainline kernel for the foreseeable future?

Right, you probably have no other choice but keeping the original U-Boot. But that's fine, with appended DTB, you can use this old U-Boot and boot modern kernels.

But are you interested in helping doing this migration, but sending some patches, doing some testing and so on?

The problem is that hardware handled by arm/plat-orion/gpio.c, require ack for edge irq, and no ack for level irq.

The code handle this issue, by two "struct irq_chip_type" per one "struct irq_chip_generic". For one "struct irq_chip_generic" irq_ack pointer is setted, for another it is NULL.

But we have only one mask_cache per two "struct irq_chip_type". So if we
1)unmask interrupt A for "edge type" trigger, 2)unmask interrupt B for "level type" trigger, 3)unmask interrupt C for "edge type",

we, because of usage of generic irq_gc_mask_clr_bit/irq_gc_mask_set_bit, have hardware configured to trigger interrupt B on "edge type", because of shared mask_cache. But kernel think that B is "level type", so when interrupt B occur via "edge" reason, we don't ack it, and B triggered again and again.

Signed-off-by Evgeniy A. Dushistov <[email protected]>

---
 arch/arm/plat-orion/gpio.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

arch/arm/plat-orion/gpio.c +32 -4

index b61a3bc..e048f61 100644
@@ -497,6 +497,34 @@ static void orion_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 #define orion_gpio_dbg_show NULL
 #endif
 
+static void orion_gpio_unmask_irq(struct irq_data *d)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
+	u32 reg_val;
+	u32 mask = d->mask;
+
+	irq_gc_lock(gc);
+	reg_val = irq_reg_readl(gc->reg_base + ct->regs.mask);
+	reg_val |= mask;
+	irq_reg_writel(reg_val, gc->reg_base + ct->regs.mask);
+	irq_gc_unlock(gc);
+}
+
+static void orion_gpio_mask_irq(struct irq_data *d)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
+	u32 mask = d->mask;
+	u32 reg_val;
+
+	irq_gc_lock(gc);
+	reg_val = irq_reg_readl(gc->reg_base + ct->regs.mask);
+	reg_val &= ~mask;
+	irq_reg_writel(reg_val, gc->reg_base + ct->regs.mask);
+	irq_gc_unlock(gc);
+}
+
 void __init orion_gpio_init(struct device_node *np,
 			    int gpio_base, int ngpio,
 			    void __iomem *base, int mask_offset,

@@ -565,8 +593,8 @@ void __init orion_gpio_init(struct device_node *np,
 	ct = gc->chip_types;
 	ct->regs.mask = ochip->mask_offset + GPIO_LEVEL_MASK_OFF;
 	ct->type = IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW;
-	ct->chip.irq_mask = irq_gc_mask_clr_bit;
-	ct->chip.irq_unmask = irq_gc_mask_set_bit;
+	ct->chip.irq_mask = orion_gpio_mask_irq;
+	ct->chip.irq_unmask = orion_gpio_unmask_irq;
 	ct->chip.irq_set_type = gpio_irq_set_type;
 	ct->chip.name = ochip->chip.label;
 

@@ -575,8 +603,8 @@ void __init orion_gpio_init(struct device_node *np,
 	ct->regs.ack = GPIO_EDGE_CAUSE_OFF;
 	ct->type = IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING;
 	ct->chip.irq_ack = irq_gc_ack_clr_bit;
-	ct->chip.irq_mask = irq_gc_mask_clr_bit;
-	ct->chip.irq_unmask = irq_gc_mask_set_bit;
+	ct->chip.irq_mask = orion_gpio_mask_irq;
+	ct->chip.irq_unmask = orion_gpio_unmask_irq;
 	ct->chip.irq_set_type = gpio_irq_set_type;
 	ct->handler = handle_edge_irq;
 	ct->chip.name = ochip->chip.label;

Signature · 4 lines

1.8.5.5

-- 
/Evgeniy

Did you look at the way gpio-mvebu.c handles this? It is a little bit different from your solution. I'm wondering if gpio-mvebu.c is just different, or wrong? It does seem to be using one mask_cache for both edge and level trigger.

The problem is that hardware handled by arm/plat-orion/gpio.c, require ack for edge irq, and no ack for level irq.

The code handle this issue, by two "struct irq_chip_type" per one "struct irq_chip_generic". For one "struct irq_chip_generic" irq_ack pointer is setted, for another it is NULL.

But we have only one mask_cache per two "struct irq_chip_type". So if we
1)unmask interrupt A for "edge type" trigger, 2)unmask interrupt B for "level type" trigger, 3)unmask interrupt C for "edge type",

we, because of usage of generic irq_gc_mask_clr_bit/irq_gc_mask_set_bit, have hardware configured to trigger interrupt B on "edge type", because of shared mask_cache. But kernel think that B is "level type", so when interrupt B occur via "edge" reason, we don't ack it, and B triggered again and again.

The problem is that hardware handled by arm/plat-orion/gpio.c, require ack for edge irq, and no ack for level irq.

The code handle this issue, by two "struct irq_chip_type" per one "struct irq_chip_generic". For one "struct irq_chip_generic" irq_ack pointer is setted, for another it is NULL.

But we have only one mask_cache per two "struct irq_chip_type". So if we
1)unmask interrupt A for "edge type" trigger, 2)unmask interrupt B for "level type" trigger, 3)unmask interrupt C for "edge type",

we, because of usage of generic irq_gc_mask_clr_bit/irq_gc_mask_set_bit, have hardware configured to trigger interrupt B on "edge type", because of shared mask_cache. But kernel think that B is "level type", so when interrupt B occur via "edge" reason, we don't ack it, and B triggered again and again.

Signed-off-by Evgeniy A. Dushistov <[email protected]>

---
 arch/arm/plat-orion/gpio.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)