Re: [PATCH] gpio/msm-v1: re-read IRQ flags on each iteration

From: Linus Walleij
Date: Wed May 16 2012 - 03:23:38 EST


On Wed, May 16, 2012 at 3:09 AM, David Brown <davidb@xxxxxxxxxxxxxx> wrote:
> On Fri, May 11, 2012 at 11:41:12AM -0600, Grant Likely wrote:

>> I'll need an ack from an msm developer before applying this.
>
> I though I saw Jeff Ohlstein reply, but I'm not finding that message.

He was replying to a similar patch affecting the DMA controller
interrupt-like construct.

> I believe this patch is actually incorrect.  The register needs to
> only be read once, and the bits walked through.  If another interrupt
> comes in during the loop, the interrupt will remain asserted, and we
> will re-enter the irq handler.  I suppose it could be wrapped in yet
> another loop, but I'm not sure it is worth it.

OK the code doesn't say really. (It did say something like that
in the DMA controller). It's quite uncommon with these type of
latched-clear registers (reading IRQ status clears the flags) type
of hardware so maybe it should be commented.

> I'm pretty sure the code works as it is, since it is the same as the
> code in the Android kernel.

The problem with this bug is that it often works until you get a
demanding case. On the VIC I guess it wasn't noticed until tested with
a device that raised a storm of IRQs so the handler started missing
them.

On a GPIO this would be the case where a GPIO IRQ is asserted
in real quick succession, like the same IRQ being asserted while
the previous IRQ on the same line is being handled and where
missing one of these IRQs creates problems.

It's gonna be error phenomena of the type "it looks like we're
missing some IRQs from our touchscreen". etc.

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