gpio: langwell: Cleanup the interrupt mess

I don't have the specs for this beast, but it looks a lot like the PXA GPIO block. Though I bet it's the same IP and the driver should have reused the PXA code.

Acknowleding the edge detect status after handling one or more gpio interrupts looks wrong. We might lose an edge which came in while we handled the previous one.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Feng Tang <[email protected]>

Cc Alek Du <[email protected]>

Cc Alan Cox <[email protected]>

---
 drivers/gpio/langwell_gpio.c |   13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Index: linux-2.6-tip/drivers/gpio/langwell_gpio.c
===================================================================

drivers/gpio/langwell_gpio.c +6 -7

@@ -190,23 +190,22 @@ static void lnw_irq_handler(unsigned irq
 	struct irq_data *data = irq_desc_get_irq_data(desc);
 	struct lnw_gpio *lnw = irq_data_get_irq_handler_data(data);
 	struct irq_chip *chip = irq_data_get_irq_chip(data);
-	u32 base, gpio, gedr_v;
+	u32 base, gpio, mask;
 	unsigned long pending;
 	void __iomem *gedr;
 
 	/* check GPIO controller to check which pin triggered the interrupt */
 	for (base = 0; base < lnw->chip.ngpio; base += 32) {
 		gedr = gpio_reg(&lnw->chip, base, GEDR);
-		gedr_v = pending = readl(gedr);
-		if (!gedr_v)
-			continue;
+		pending = readl(gedr);
 		while (pending) {
 			gpio = __ffs(pending) - 1;
-			pending &= ~BIT(gpio);
+			mask = BIT(gpio);
+			pending &= ~mask;
+			/* Clear before handling so we can't lose an edge */
+			writel(mask, gedr);
 			generic_handle_irq(lnw->irq_base + base + gpio);
 		}
-		/* clear the edge detect status bit */
-		writel(gedr_v, gedr);
 	}
 
 	chip->irq_eoi(data);

Quoted 4 lines · On Fri, 18 Mar 2011 03:32:58 +0800 Thomas Gleixner <[email protected]> wrote:

I don't have the specs for this beast, but it looks a lot like the PXA GPIO block. Though I bet it's the same IP and the driver should have reused the PXA code.

I think so.

Quoted 4 lines

Acknowleding the edge detect status after handling one or more gpio interrupts looks wrong. We might lose an edge which came in while we handled the previous one.

Thanks for it. Although losing interrupt can always happen if the pending IRQs are more than 1, but your patch will reduce the possibility.

Thanks,
Alek

Quoted 45 lines

Signed-off-by Thomas Gleixner <[email protected]>

Cc Feng Tang <[email protected]>

Cc Alek Du <[email protected]>

Cc Alan Cox <[email protected]>

---
 drivers/gpio/langwell_gpio.c |   13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Index: linux-2.6-tip/drivers/gpio/langwell_gpio.c
===================================================================

drivers/gpio/langwell_gpio.c +6 -7

@@ -190,23 +190,22 @@ static void lnw_irq_handler(unsigned irq
 	struct irq_data *data = irq_desc_get_irq_data(desc);
 	struct lnw_gpio *lnw = irq_data_get_irq_handler_data(data);
 	struct irq_chip *chip = irq_data_get_irq_chip(data);
-	u32 base, gpio, gedr_v;
+	u32 base, gpio, mask;
 	unsigned long pending;
 	void __iomem *gedr;
 
 	/* check GPIO controller to check which pin triggered the interrupt */
 	for (base = 0; base < lnw->chip.ngpio; base += 32) {
 		gedr = gpio_reg(&lnw->chip, base, GEDR);
-		gedr_v = pending = readl(gedr);
-		if (!gedr_v)
-			continue;
+		pending = readl(gedr);
 		while (pending) {
 			gpio = __ffs(pending) - 1;
-			pending &= ~BIT(gpio);
+			mask = BIT(gpio);
+			pending &= ~mask;
+			/* Clear before handling so we can't lose an edge */
+			writel(mask, gedr);
 			generic_handle_irq(lnw->irq_base + base + gpio);
 		}
-		/* clear the edge detect status bit */
-		writel(gedr_v, gedr);
 	}
 
 	chip->irq_eoi(data);

The langwell interrupt handler is interesting and breaks when an architecture enables GENERIC_HARDIRQS_NO_DEPRECATED.

Also this driver along with the other intel ones should depend on x86.

The series cleans that up. The last patch 5/5 is RFC and needs a close look and testing by folks at Intel.

Quoted 7 lines · Thomas Gleixner

The langwell interrupt handler is interesting and breaks when an architecture enables GENERIC_HARDIRQS_NO_DEPRECATED.

Also this driver along with the other intel ones should depend on x86.

The series cleans that up. The last patch 5/5 is RFC and needs a close look and testing by folks at Intel.

Applied first 4, thanks. Will apply last one when I get acks from the Intel folks.