pinctrl/at91: Fix lockup when IRQ on PIOC and PIOD occurs

PATCH New Alexander Stein 4 participants 7 messages Apr 23, 2014 – May 27, 2014

Subsystems (3)

Arm/Microchip (AT91) SoC support Pin control Pin controller - Microchip AT91

lore

Share & tools

lore

Alexander Stein

author

2014-04-23 21:40 UTC #0 lore raw

PATCH 1/1 pinctrl/at91: Fix lockup when IRQ on PIOC and PIOD occurs

With commit 8d56dfcc (pinctrl/at91: convert driver to use gpiolib irqchip) gpiochip_set_chained_irqchip is called for PIOC, PIOD and PIOE. The associated GPIO chip for the IRQ chip is overwritten each time, because they share the same hard IRQ line.
Thus if an IRQ occurs on PIOC or PIOD, gpio_irq_handler will only check on PIOE (the assigned GPIO chip) where no event occured. Thus the IRQ will not be cleared, retriggering the ISR.
Fix that (like done before) by only set the PIOC GPIO chip to the IRQ chip.

Signed-off-by Alexander Stein <[email protected]>

---
 drivers/pinctrl/pinctrl-at91.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

drivers/pinctrl/pinctrl-at91.c +12 -0

index 64a8f8f..dd7d3e4 100644
@@ -1468,6 +1468,7 @@ static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
 static int at91_gpio_of_irq_setup(struct device_node *node,
 				  struct at91_gpio_chip *at91_gpio)
 {
+	struct at91_gpio_chip   *prev = NULL;
 	struct irq_data		*d = irq_get_irq_data(at91_gpio->pioc_virq);
 	int ret;

@@ -1493,6 +1494,17 @@ static int at91_gpio_of_irq_setup(struct device_node *node,
 		panic("at91_gpio.%d: couldn't allocate irq domain (DT).\n",
 			at91_gpio->pioc_idx);
 
+	/* Setup chained handler */
+	if (at91_gpio->pioc_idx)
+		prev = gpio_chips[at91_gpio->pioc_idx - 1];
+
+	/* The top level handler handles one bank of GPIOs, except
+	 * on some SoC it can handle up to three...
+	 * We only set up the handler for the first of the list.
+	 */
+	if (prev && prev->next == at91_gpio)
+		return 0;
+
 	/* Then register the chain on the parent IRQ */
 	gpiochip_set_chained_irqchip(&at91_gpio->chip,
 				     &gpio_irqchip,

Signature · 1 line

1.9.2

Alexander Stein

author

2014-04-23 22:34 UTC #1 lore raw

PATCH 1/1 pinctrl/at91: Fix lockup when IRQ on PIOC and PIOD occurs

Sorry,

drop that. This one creates another issue with that linear IRQ mapping. The correct bits from the correct register is read (which is what I checked), but unfortunatly irq_find_mapping now only returns mappings for PIOC :( I will rework that.

Alexander

On Wednesday 23 April 2014, 23:40:03 wrote Alexander Stein:

Quoted 45 lines

With commit 8d56dfcc (pinctrl/at91: convert driver to use gpiolib irqchip) gpiochip_set_chained_irqchip is called for PIOC, PIOD and PIOE. The associated GPIO chip for the IRQ chip is overwritten each time, because they share the same hard IRQ line.
Thus if an IRQ occurs on PIOC or PIOD, gpio_irq_handler will only check on PIOE (the assigned GPIO chip) where no event occured. Thus the IRQ will not be cleared, retriggering the ISR.
Fix that (like done before) by only set the PIOC GPIO chip to the IRQ chip.
Signed-off-by Alexander Stein <[email protected]>
---
 drivers/pinctrl/pinctrl-at91.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
drivers/pinctrl/pinctrl-at91.c +12 -0
index 64a8f8f..dd7d3e4 100644
@@ -1468,6 +1468,7 @@ static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
 static int at91_gpio_of_irq_setup(struct device_node *node,
 				  struct at91_gpio_chip *at91_gpio)
 {
+	struct at91_gpio_chip   *prev = NULL;
 	struct irq_data		*d = irq_get_irq_data(at91_gpio->pioc_virq);
 	int ret;
 
@@ -1493,6 +1494,17 @@ static int at91_gpio_of_irq_setup(struct device_node *node,
 		panic("at91_gpio.%d: couldn't allocate irq domain (DT).\n",
 			at91_gpio->pioc_idx);
 
+	/* Setup chained handler */
+	if (at91_gpio->pioc_idx)
+		prev = gpio_chips[at91_gpio->pioc_idx - 1];
+
+	/* The top level handler handles one bank of GPIOs, except
+	 * on some SoC it can handle up to three...
+	 * We only set up the handler for the first of the list.
+	 */
+	if (prev && prev->next == at91_gpio)
+		return 0;
+
 	/* Then register the chain on the parent IRQ */
 	gpiochip_set_chained_irqchip(&at91_gpio->chip,
 				     &gpio_irqchip,

Alexander Stein

author

2014-04-24 17:55 UTC #2 lore raw

PATCH v2 1/1 pinctrl/at91: Fix lockup when IRQ on PIOC and PIOD occurs

In reply to Alexander Stein

With commit 80cc3732 (pinctrl/at91: convert driver to use gpiolib irqchip) gpiochip_set_chained_irqchip is called for PIOC, PIOD and PIOE. The associated GPIO chip for the IRQ chip is overwritten each time, because they share the same hard IRQ line.
Thus if an IRQ occurs on PIOC or PIOD, gpio_irq_handler will only check on PIOE (the assigned GPIO chip) where no event occured. Thus the IRQ will not be cleared, retriggering the ISR.
Fix that (like done before) by only set the PIOC GPIO chip to the IRQ chip and walk the list in the irq handler.

Signed-off-by Alexander Stein <[email protected]>

---
Changes in v2:
* Fix git SHA1 in commit message (used local one)
* Switch to the next gpio_chip when walking the list

Now really tested with evtest on inputs with IRQ on PIO A, B, D and E! I don't have any useable GPIO on PIO C though.

 drivers/pinctrl/pinctrl-at91.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

drivers/pinctrl/pinctrl-at91.c +13 -0

index 64a8f8f..d90ae02 100644
@@ -1453,6 +1453,7 @@ static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
 				break;
 			at91_gpio = at91_gpio->next;
 			pio = at91_gpio->regbase;
+			gpio_chip = &at91_gpio->chip;
 			continue;
 		}

@@ -1468,6 +1469,7 @@ static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
 static int at91_gpio_of_irq_setup(struct device_node *node,
 				  struct at91_gpio_chip *at91_gpio)
 {
+	struct at91_gpio_chip   *prev = NULL;
 	struct irq_data		*d = irq_get_irq_data(at91_gpio->pioc_virq);
 	int ret;

@@ -1493,6 +1495,17 @@ static int at91_gpio_of_irq_setup(struct device_node *node,
 		panic("at91_gpio.%d: couldn't allocate irq domain (DT).\n",
 			at91_gpio->pioc_idx);
 
+	/* Setup chained handler */
+	if (at91_gpio->pioc_idx)
+		prev = gpio_chips[at91_gpio->pioc_idx - 1];
+
+	/* The top level handler handles one bank of GPIOs, except
+	 * on some SoC it can handle up to three...
+	 * We only set up the handler for the first of the list.
+	 */
+	if (prev && prev->next == at91_gpio)
+		return 0;
+
 	/* Then register the chain on the parent IRQ */
 	gpiochip_set_chained_irqchip(&at91_gpio->chip,
 				     &gpio_irqchip,

Signature · 1 line

1.9.2

Linus Walleij

2014-04-25 09:18 UTC #3 lore raw

PATCH v2 1/1 pinctrl/at91: Fix lockup when IRQ on PIOC and PIOD occurs

Quoted 10 lines · Alexander Stein

With commit 80cc3732 (pinctrl/at91: convert driver to use gpiolib irqchip) gpiochip_set_chained_irqchip is called for PIOC, PIOD and PIOE. The associated GPIO chip for the IRQ chip is overwritten each time, because they share the same hard IRQ line.
Thus if an IRQ occurs on PIOC or PIOD, gpio_irq_handler will only check on PIOE (the assigned GPIO chip) where no event occured. Thus the IRQ will not be cleared, retriggering the ISR.
Fix that (like done before) by only set the PIOC GPIO chip to the IRQ chip and walk the list in the irq handler.

Quoted 5 lines

Signed-off-by Alexander Stein <[email protected]>
---
Changes in v2:
* Fix git SHA1 in commit message (used local one)
* Switch to the next gpio_chip when walking the list

This v2 patch applied for next.

Yours,
Linus Walleij

Nicolas Ferre

2014-05-09 10:23 UTC #4 lore raw

PATCH v2 1/1 pinctrl/at91: Fix lockup when IRQ on PIOC and PIOD occurs

On 25/04/2014 11:18, Linus Walleij :

Quoted 12 lines · Alexander Stein

Quoted 11 lines
With commit 80cc3732 (pinctrl/at91: convert driver to use gpiolib irqchip) gpiochip_set_chained_irqchip is called for PIOC, PIOD and PIOE. The associated GPIO chip for the IRQ chip is overwritten each time, because they share the same hard IRQ line.
Thus if an IRQ occurs on PIOC or PIOD, gpio_irq_handler will only check on PIOE (the assigned GPIO chip) where no event occured. Thus the IRQ will not be cleared, retriggering the ISR.
Fix that (like done before) by only set the PIOC GPIO chip to the IRQ chip and walk the list in the irq handler.

Quoted 7 lines

Quoted 5 lines
Signed-off-by Alexander Stein <[email protected]>
---
Changes in v2:
* Fix git SHA1 in commit message (used local one)
* Switch to the next gpio_chip when walking the list
This v2 patch applied for next.

Linus,

I do not see this patch in your "for-next" branch nor in linux-next. Did I miss something?

Bye,

Signature · 1 line

Nicolas Ferre

Alexandre Belloni

2014-05-21 17:15 UTC #5 lore raw

PATCH v2 1/1 pinctrl/at91: Fix lockup when IRQ on PIOC and PIOD occurs

Hi Linus,

Quoted 19 lines · Nicolas Ferre

On 25/04/2014 11:18, Linus Walleij :
Quoted 18 lines · Alexander Stein
Quoted 16 lines
With commit 80cc3732 (pinctrl/at91: convert driver to use gpiolib irqchip) gpiochip_set_chained_irqchip is called for PIOC, PIOD and PIOE. The associated GPIO chip for the IRQ chip is overwritten each time, because they share the same hard IRQ line.
Thus if an IRQ occurs on PIOC or PIOD, gpio_irq_handler will only check on PIOE (the assigned GPIO chip) where no event occured. Thus the IRQ will not be cleared, retriggering the ISR.
Fix that (like done before) by only set the PIOC GPIO chip to the IRQ chip and walk the list in the irq handler.
Signed-off-by Alexander Stein <[email protected]>
---
Changes in v2:
* Fix git SHA1 in commit message (used local one)
* Switch to the next gpio_chip when walking the list

Quoted 7 lines

Quoted 1 line
This v2 patch applied for next.
Linus,
I do not see this patch in your "for-next" branch nor in linux-next. Did I miss something?

I don't see it either, I would prefer that we don't miss it for 3.16, can you check ?

Thanks,

Signature · 3 lines

Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Linus Walleij

2014-05-27 09:13 UTC #6 lore raw

PATCH v2 1/1 pinctrl/at91: Fix lockup when IRQ on PIOC and PIOD occurs

In reply to Nicolas Ferre

Quoted 19 lines · Nicolas Ferre

On 25/04/2014 11:18, Linus Walleij :
Quoted 18 lines · Alexander Stein
Quoted 16 lines
With commit 80cc3732 (pinctrl/at91: convert driver to use gpiolib irqchip) gpiochip_set_chained_irqchip is called for PIOC, PIOD and PIOE. The associated GPIO chip for the IRQ chip is overwritten each time, because they share the same hard IRQ line.
Thus if an IRQ occurs on PIOC or PIOD, gpio_irq_handler will only check on PIOE (the assigned GPIO chip) where no event occured. Thus the IRQ will not be cleared, retriggering the ISR.
Fix that (like done before) by only set the PIOC GPIO chip to the IRQ chip and walk the list in the irq handler.
Signed-off-by Alexander Stein <[email protected]>
---
Changes in v2:
* Fix git SHA1 in commit message (used local one)
* Switch to the next gpio_chip when walking the list

Quoted 6 lines

Quoted 1 line
This v2 patch applied for next.
Linus,
I do not see this patch in your "for-next" branch nor in linux-next. Did I miss something?

I screwed up somehow :-/

Patch applied *now*.

Yours,
Linus Walleij