[PATCH] gpio: vf610: handle level IRQ's properly

[PATCH] gpio: vf610: handle level IRQ's properly

[Stefan Agner]

The GPIO IRQ controller is able to generate level triggered
interrupts, however, these were handled by handle_simple_irq so far
which did not take care of IRQ masking. This lead to "nobody cared
(try booting with the "irqpoll" option)" stack traces.

Use the generic interrupt handlers depending on the IRQ type.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpio/gpio-vf610.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
index 7bd9f20..b6a0887 100644
--- a/drivers/gpio/gpio-vf610.c
+++ b/drivers/gpio/gpio-vf610.c
@@ -60,6 +60,8 @@ struct vf610_gpio_port {
 #define PORT_INT_EITHER_EDGE	0xb
 #define PORT_INT_LOGIC_ONE	0xc
 
+static struct irq_chip vf610_gpio_irq_chip;
+
 static const struct of_device_id vf610_gpio_dt_ids[] = {
 	{ .compatible = "fsl,vf610-gpio" },
 	{ /* sentinel */ }
@@ -173,6 +175,11 @@ static int vf610_gpio_irq_set_type(struct irq_data *d, u32 type)
 
 	port->irqc[d->hwirq] = irqc;
 
+	if (type & IRQ_TYPE_LEVEL_MASK)
+		__irq_set_handler_locked(d->irq, handle_level_irq);
+	else
+		__irq_set_handler_locked(d->irq, handle_edge_irq);
+
 	return 0;
 }
 
@@ -263,7 +270,7 @@ static int vf610_gpio_probe(struct platform_device *pdev)
 	vf610_gpio_writel(~0, port->base + PORT_ISFR);
 
 	ret = gpiochip_irqchip_add(gc, &vf610_gpio_irq_chip, 0,
-				   handle_simple_irq, IRQ_TYPE_NONE);
+				   handle_edge_irq, IRQ_TYPE_NONE);
 	if (ret) {
 		dev_err(dev, "failed to add irqchip\n");
 		gpiochip_remove(gc);
-- 
2.5.0

Re: [PATCH] gpio: vf610: handle level IRQ's properly

[Linus Walleij]

On Sat, Aug 22, 2015 at 12:56 AM, Stefan Agner <stefan@agner.ch> wrote:

> The GPIO IRQ controller is able to generate level triggered
> interrupts, however, these were handled by handle_simple_irq so far
> which did not take care of IRQ masking. This lead to "nobody cared
> (try booting with the "irqpoll" option)" stack traces.
>
> Use the generic interrupt handlers depending on the IRQ type.
>
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Patch applied.

Hm, this looks like a problem that could exist in a few more
GPIO drivers. Can you look around and see if there is something
immediately suspicious in drivers/gpio?

Yours,
Linus Walleij

Re: [PATCH] gpio: vf610: handle level IRQ's properly

[Stefan Agner]

On 2015-08-26 05:47, Linus Walleij wrote:
> On Sat, Aug 22, 2015 at 12:56 AM, Stefan Agner <stefan@agner.ch> wrote:
> 
>> The GPIO IRQ controller is able to generate level triggered
>> interrupts, however, these were handled by handle_simple_irq so far
>> which did not take care of IRQ masking. This lead to "nobody cared
>> (try booting with the "irqpoll" option)" stack traces.
>>
>> Use the generic interrupt handlers depending on the IRQ type.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
> 
> Patch applied.

Thx

 
> Hm, this looks like a problem that could exist in a few more
> GPIO drivers. Can you look around and see if there is something
> immediately suspicious in drivers/gpio?

Just looked a bit around. There are some which use handle_simple_irq but
also run their handler in threaded/one-shot handler. I guess in that
case it is fine to use the simple_irq handler.

However, those GPIO drivers look suspicious:

gpio-altera.c: Uses handle_simple_irq, supports edge and level
gpio-bcm-kona.c: Uses handle_simple_irq, supports only edge
gpio-grgpio.c: Uses handle_simple_irq, supports edge and level
gpio-intel-mid.c: Uses handle_simple_irq, supports only edge
gpio-lynxpoint.c: Uses handle_simple_irq, supports edge and level
gpio-ml-ioh.c: Uses handle_simple_irq, supports edge and level
gpio-pl061.c: Uses handle_simple_irq, supports edge and level
gpio-rcar.c: Uses handle_level_irq, supports edge and level
gpio-timberdale.c: Uses handle_simple_irq, supports edge and level

--
Stefan

Re: [PATCH] gpio: vf610: handle level IRQ's properly

[Linus Walleij]

On Wed, Aug 26, 2015 at 10:11 AM, Stefan Agner <stefan@agner.ch> wrote:
> On 2015-08-26 05:47, Linus Walleij wrote:
>> On Sat, Aug 22, 2015 at 12:56 AM, Stefan Agner <stefan@agner.ch> wrote:
>>
>>> The GPIO IRQ controller is able to generate level triggered
>>> interrupts, however, these were handled by handle_simple_irq so far
>>> which did not take care of IRQ masking. This lead to "nobody cared
>>> (try booting with the "irqpoll" option)" stack traces.
>>>
>>> Use the generic interrupt handlers depending on the IRQ type.
>>>
>>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>>
>> Patch applied.
>
> Thx
>
>
>> Hm, this looks like a problem that could exist in a few more
>> GPIO drivers. Can you look around and see if there is something
>> immediately suspicious in drivers/gpio?
>
> Just looked a bit around. There are some which use handle_simple_irq but
> also run their handler in threaded/one-shot handler. I guess in that
> case it is fine to use the simple_irq handler.
>
> However, those GPIO drivers look suspicious:
>
> gpio-altera.c: Uses handle_simple_irq, supports edge and level
> gpio-bcm-kona.c: Uses handle_simple_irq, supports only edge
> gpio-grgpio.c: Uses handle_simple_irq, supports edge and level
> gpio-intel-mid.c: Uses handle_simple_irq, supports only edge
> gpio-lynxpoint.c: Uses handle_simple_irq, supports edge and level
> gpio-ml-ioh.c: Uses handle_simple_irq, supports edge and level
> gpio-pl061.c: Uses handle_simple_irq, supports edge and level
> gpio-rcar.c: Uses handle_level_irq, supports edge and level
> gpio-timberdale.c: Uses handle_simple_irq, supports edge and level

Thanks!

I'm gonna look at pl061 myself, for the rest I ping the maintainers,
(To-line) I suspect a lot of them have never really been tested with
level IRQs because most just use edge IRQs.

Yours,
Linus Walleij