Re: [PATCH 2/3] pinctrl:Intel: clear interrupt status for every IRQ setup

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Mon, Mar 14, 2016 at 9:44 AM, Westerberg, Mika
<mika.westerberg@xxxxxxxxx> wrote:
> On Mon, Mar 14, 2016 at 03:24:06AM +0200, Zheng, Qi wrote:

>> > +   intel_gpio_irq_ack(d);
>>
>> > If the pin toggles right here, we still have the same issue, no?
>>
>> Yes. But it is very short time from here to the place IRQ got enabled.
>
> That is still a race.
>
>> To me, it is the only available platform dependent interface to do
> the "ACK" in the flow of request_irq. Maybe you have other better
> option here?
(...)
> Drivers need to deal with the fact that they might get spurious
> interrupts from time to time. You need to check in the interrupt handler
> if the interrupt was from the device you are driving or not.
>
> request_irq() enables the interrupt line and if the pin is already in
> a state that triggers an interrupt, the driver interrupt handler will
> be called.

This looks like something is wrong in the irqchip.

Your set_type() is supporting edges but have all IRQs
handled by handle_simple_irq() rather than handle_edge_irq()
for the edges, which gives a more robust control flow
from IRQ to ACK to calling the handler.

Zheng/Mika: please look at how the level/edge
IRQs are handled in drivers/gpio/gpio-pl061.c
where I *tried* to do things right, switching handler
in .set_type() using irq_set_handler_locked(). I think
you may need to use handle_edge_irq() for the edge IRQs
and handle_level_irq() for the level IRQs just like I do
in the PL061 driver.

(I might be wrong.)

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html