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

[David Brown]

On Fri, May 11, 2012 at 11:41:12AM -0600, Grant Likely wrote:
> On Fri, 11 May 2012 14:07:12 +0200, Linus Walleij <linus.walleij@xxxxxxxxxxxxxx> wrote:
> > From: Linus Walleij <linus.walleij@xxxxxxxxxx>
> > 
> > We have recently found a number or erroneous IRQ handlers in
> > the kernel where the flag iterator loop miss IRQs that get
> > raised when the loop is executing. This was spotted in the
> > MSM v1 GPIO driver by Julia Lawall using this cocinelle
> > snippet:
> > 
> > @@
> > expression pending,gedr,e1;
> > statement S;
> > @@
> > 
> > *pending = readl(gedr);
> > ... when != pending = e1
> > while (pending) S
> > 
> > Reported-by: Julia Lawall <julia.lawall@xxxxxxx>
> > Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> 
> Thanks Linus,
> 
> 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.
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.

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

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
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/