[ARM] 5401/1: Orion: fix edge triggered GPIO interrupt support
authorNicolas Pitre <nico@cam.org>
Tue, 17 Feb 2009 19:45:50 +0000 (20:45 +0100)
committerRussell King <rmk+kernel@arm.linux.org.uk>
Tue, 17 Feb 2009 22:37:09 +0000 (22:37 +0000)
The GPIO interrupts can be configured as either level triggered or edge
triggered, with a default of level triggered.  When an edge triggered
interrupt is requested, the gpio_irq_set_type method is called which
currently switches the given IRQ descriptor between two struct irq_chip
instances: orion_gpio_irq_level_chip and orion_gpio_irq_edge_chip. This
happens via __setup_irq() which also calls irq_chip_set_defaults() to
assign default methods to uninitialized ones.  The problem is that
irq_chip_set_defaults() is called before the irq_chip reference is
switched, leaving the new irq_chip (orion_gpio_irq_edge_chip in this
case) with uninitialized methods such as chip->startup() causing a kernel
oops.

Many solutions are possible, such as making irq_chip_set_defaults() global
and calling it from gpio_irq_set_type(), or calling __irq_set_trigger()
before irq_chip_set_defaults() in __setup_irq().  But those require
modifications to the generic IRQ code which might have adverse effect on
other architectures, and that would still be a fragile arrangement.
Manually copying the missing methods from within gpio_irq_set_type()
would be really ugly and it would break again the day new methods with
automatic defaults are added.

A better solution is to have a single irq_chip instance which can deal
with both edge and level triggered interrupts.  It is also a good idea
to switch the IRQ handler instead, as the edge IRQ handler allows for
one edge IRQ event to be queued as the IRQ is actually masked only when
that second IRQ is received, at which point the hardware can queue an
additional IRQ event, making edge triggered interrupts a bit more
reliable.

Tested-by: Martin Michlmayr <tbm@cyrius.com>
Signed-off-by: Nicolas Pitre <nico@marvell.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
arch/arm/mach-kirkwood/irq.c
arch/arm/mach-mv78xx0/irq.c
arch/arm/mach-orion5x/irq.c
arch/arm/plat-orion/gpio.c
arch/arm/plat-orion/include/plat/gpio.h

index efb86b700276bf965c611526e83d8973d20b7a90..06083b23bb446a6743a894afd0c605c5b2932274 100644 (file)
@@ -42,7 +42,7 @@ void __init kirkwood_init_irq(void)
        writel(0, GPIO_EDGE_CAUSE(32));
 
        for (i = IRQ_KIRKWOOD_GPIO_START; i < NR_IRQS; i++) {
-               set_irq_chip(i, &orion_gpio_irq_level_chip);
+               set_irq_chip(i, &orion_gpio_irq_chip);
                set_irq_handler(i, handle_level_irq);
                irq_desc[i].status |= IRQ_LEVEL;
                set_irq_flags(i, IRQF_VALID);
index e273418797b41cbaa309469fac12656aff97b986..30b7e4bcdbc7c7523affdb36f4884e819acf0420 100644 (file)
@@ -40,7 +40,7 @@ void __init mv78xx0_init_irq(void)
        writel(0, GPIO_EDGE_CAUSE(0));
 
        for (i = IRQ_MV78XX0_GPIO_START; i < NR_IRQS; i++) {
-               set_irq_chip(i, &orion_gpio_irq_level_chip);
+               set_irq_chip(i, &orion_gpio_irq_chip);
                set_irq_handler(i, handle_level_irq);
                irq_desc[i].status |= IRQ_LEVEL;
                set_irq_flags(i, IRQF_VALID);
index 0caae43301e54e472d82187680ed69fca4feda72..e03f7b45cb0d114ad0b0e343d424ba1122b03472 100644 (file)
@@ -44,7 +44,7 @@ void __init orion5x_init_irq(void)
         * User can use set_type() if he wants to use edge types handlers.
         */
        for (i = IRQ_ORION5X_GPIO_START; i < NR_IRQS; i++) {
-               set_irq_chip(i, &orion_gpio_irq_level_chip);
+               set_irq_chip(i, &orion_gpio_irq_chip);
                set_irq_handler(i, handle_level_irq);
                irq_desc[i].status |= IRQ_LEVEL;
                set_irq_flags(i, IRQF_VALID);
index 967186425ca1073ecea837206ebe22c081335693..0d12c21647663d5191167569354bd3af1d7d84b5 100644 (file)
@@ -265,51 +265,36 @@ EXPORT_SYMBOL(orion_gpio_set_blink);
  *        polarity    LEVEL          mask
  *
  ****************************************************************************/
-static void gpio_irq_edge_ack(u32 irq)
-{
-       int pin = irq_to_gpio(irq);
-
-       writel(~(1 << (pin & 31)), GPIO_EDGE_CAUSE(pin));
-}
-
-static void gpio_irq_edge_mask(u32 irq)
-{
-       int pin = irq_to_gpio(irq);
-       u32 u;
-
-       u = readl(GPIO_EDGE_MASK(pin));
-       u &= ~(1 << (pin & 31));
-       writel(u, GPIO_EDGE_MASK(pin));
-}
 
-static void gpio_irq_edge_unmask(u32 irq)
+static void gpio_irq_ack(u32 irq)
 {
-       int pin = irq_to_gpio(irq);
-       u32 u;
-
-       u = readl(GPIO_EDGE_MASK(pin));
-       u |= 1 << (pin & 31);
-       writel(u, GPIO_EDGE_MASK(pin));
+       int type = irq_desc[irq].status & IRQ_TYPE_SENSE_MASK;
+       if (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING)) {
+               int pin = irq_to_gpio(irq);
+               writel(~(1 << (pin & 31)), GPIO_EDGE_CAUSE(pin));
+       }
 }
 
-static void gpio_irq_level_mask(u32 irq)
+static void gpio_irq_mask(u32 irq)
 {
        int pin = irq_to_gpio(irq);
-       u32 u;
-
-       u = readl(GPIO_LEVEL_MASK(pin));
+       int type = irq_desc[irq].status & IRQ_TYPE_SENSE_MASK;
+       u32 reg = (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING)) ?
+               GPIO_EDGE_MASK(pin) : GPIO_LEVEL_MASK(pin);
+       u32 u = readl(reg);
        u &= ~(1 << (pin & 31));
-       writel(u, GPIO_LEVEL_MASK(pin));
+       writel(u, reg);
 }
 
-static void gpio_irq_level_unmask(u32 irq)
+static void gpio_irq_unmask(u32 irq)
 {
        int pin = irq_to_gpio(irq);
-       u32 u;
-
-       u = readl(GPIO_LEVEL_MASK(pin));
+       int type = irq_desc[irq].status & IRQ_TYPE_SENSE_MASK;
+       u32 reg = (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING)) ?
+               GPIO_EDGE_MASK(pin) : GPIO_LEVEL_MASK(pin);
+       u32 u = readl(reg);
        u |= 1 << (pin & 31);
-       writel(u, GPIO_LEVEL_MASK(pin));
+       writel(u, reg);
 }
 
 static int gpio_irq_set_type(u32 irq, u32 type)
@@ -331,9 +316,9 @@ static int gpio_irq_set_type(u32 irq, u32 type)
         * Set edge/level type.
         */
        if (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING)) {
-               desc->chip = &orion_gpio_irq_edge_chip;
+               desc->handle_irq = handle_edge_irq;
        } else if (type & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW)) {
-               desc->chip = &orion_gpio_irq_level_chip;
+               desc->handle_irq = handle_level_irq;
        } else {
                printk(KERN_ERR "failed to set irq=%d (type=%d)\n", irq, type);
                return -EINVAL;
@@ -371,19 +356,11 @@ static int gpio_irq_set_type(u32 irq, u32 type)
        return 0;
 }
 
-struct irq_chip orion_gpio_irq_edge_chip = {
-       .name           = "orion_gpio_irq_edge",
-       .ack            = gpio_irq_edge_ack,
-       .mask           = gpio_irq_edge_mask,
-       .unmask         = gpio_irq_edge_unmask,
-       .set_type       = gpio_irq_set_type,
-};
-
-struct irq_chip orion_gpio_irq_level_chip = {
-       .name           = "orion_gpio_irq_level",
-       .mask           = gpio_irq_level_mask,
-       .mask_ack       = gpio_irq_level_mask,
-       .unmask         = gpio_irq_level_unmask,
+struct irq_chip orion_gpio_irq_chip = {
+       .name           = "orion_gpio",
+       .ack            = gpio_irq_ack,
+       .mask           = gpio_irq_mask,
+       .unmask         = gpio_irq_unmask,
        .set_type       = gpio_irq_set_type,
 };
 
index 54deaf274b5268a6983dae399f8285f4e4a92ea6..ec743e82c876cab82abd00a9c415c265258feea9 100644 (file)
@@ -31,8 +31,7 @@ void orion_gpio_set_blink(unsigned pin, int blink);
 /*
  * GPIO interrupt handling.
  */
-extern struct irq_chip orion_gpio_irq_edge_chip;
-extern struct irq_chip orion_gpio_irq_level_chip;
+extern struct irq_chip orion_gpio_irq_chip;
 void orion_gpio_irq_handler(int irqoff);