Re: [PATCH] gpio/langwell: re-read the IRQ status register aftereach iteration

From: Julia Lawall
Date: Thu May 10 2012 - 16:34:56 EST


On Thu, 10 May 2012, Linus Walleij wrote:

On Thu, May 10, 2012 at 12:01 PM, Mika Westerberg
<mika.westerberg@xxxxxxxxxxxxxxx> wrote:

Spotted by Grant Likely. The IRQ status register should be re-read after
each iteration. Otherwise the loop misses the interrupt if it gets raised
immediately after handled.

Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
---
This patch applies on top of my previous patch ("gpio: langwell: convert to
use irq_domain").

Oh yeah that thing I recognize!
Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx>

If someone has time I think it would be a good idea to
conjure a cocienelle sematic patch to catch this pattern, because
I suspect it might exist in more IRQ handlers.

- pending = readl(gedr);
- while (pending) {
+ while ((pending = readl(gedr))) {
gpio = __ffs(pending);
mask = BIT(gpio);
- pending &= ~mask;

I tried the following:

@@
expression pending,gedr,e1;
statement S;
@@

*pending = readl(gedr);
... when != pending = e1
while (pending) S

That is, a readl call followed by a while loop with no reassignment of pending.

Perhaps the following are suspicious:

arch/arm/mach-msm/dma.c, function msm_datamover_irq_handler
drivers/gpio/gpio-msm-v1.c, function msm_gpio_irq_handler

But I am not completely sure to understand what is wanted (or not wanted).

julia
--
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/