]> www.pilppa.org Git - linux-2.6-omap-h63xx.git/commitdiff
twl4030-pwrirq simplification, cleanup
authorDavid Brownell <david-b@pacbell.net>
Wed, 8 Oct 2008 14:22:30 +0000 (17:22 +0300)
committerTony Lindgren <tony@atomide.com>
Fri, 10 Oct 2008 11:55:42 +0000 (14:55 +0300)
I can't test the pwrbutton stuff, but rtc and musb behave...

With one issue:  the usb transceiver glue misbehaves on
startup if the device is connected.  (Again.)  I looked at
this a bit, and I think the issue is probably caused by
not actually using key USB registers ... IRQ trigger
mode state transitions are at best a fragile proxy for the
real state.

(This is similar to the GPIO patch I just sent, but simpler
except for the impact on the few drivers thinking oddly
about IRQs.  Those patches cover the key SIH modules, and
a similar one affects the PIH in twl4030-core.)

- Dave

================================
Streamline the "power irq" code, and some of the mechanisms
it uses:

 - Support IRQ masking, not just unmasking; simpler code.
 - Use the standard handle_edge_irq() handler for these edge IRQs.
 - Use generic_handle_irq() instead of a manual expansion thereof.
 - Provide a missing spinlock for the shared data.

In short, use more of the standard genirq code ... more correctly.

Also, update the drivers using the "power IRQ" accordingly:

 - Don't request IRQF_DISABLED if the handler makes I2C calls;
   and defend against lockdep forcing it on us.

 - Let the irq_chip handle IRQ mask/unmask; it handles mutual
   exclusion between drivers, among other things.

 - (Unrelated) remove useless MODULE_ALIAS in pwrbutton.

The USB transceiver driver still places some dodgey games with IRQ
enable/disable, and IRQ trigger flags.  At least some of them seem
like they'd be simplified by using USB transceiver registers ...
notably, startup code, which doesn't seem to check state before
it enters an irq-driven mode.

For the moment, these drivers still (wrongly) try to configure IRQ
trigger modes themselves ... again, that's something that an irq_chip
handles (but not yet, for this one).

NOTE:  tested (MUSB and RTC only) along with the init/retry hack
to make twl4030-pwrirq work around the i2c-omap timeout problems.

drivers/i2c/chips/twl4030-pwrbutton.c
drivers/i2c/chips/twl4030-pwrirq.c
drivers/i2c/chips/twl4030-usb.c
drivers/rtc/rtc-twl4030.c

index 9a361b3b6d1fb1afc4bdc01c303a50aa5987357d..cd4d06e8c9626b4a8057e2dd4afff5ccb91ce66f 100644 (file)
@@ -49,6 +49,14 @@ static irqreturn_t powerbutton_irq(int irq, void *dev_id)
        int err;
        u8 value;
 
+#ifdef CONFIG_LOCKDEP
+       /* WORKAROUND for lockdep forcing IRQF_DISABLED on us, which
+        * we don't want and can't tolerate.  Although it might be
+        * friendlier not to borrow this thread context...
+        */
+       local_irq_enable();
+#endif
+
        err = twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &value,
                                  STS_HW_CONDITIONS);
        if (!err)  {
@@ -67,6 +75,7 @@ static int __init twl4030_pwrbutton_init(void)
        int err = 0;
        u8 value;
 
+       /* PWRBTN == PWRON */
        if (request_irq(TWL4030_PWRIRQ_PWRBTN, powerbutton_irq, 0,
                        "PwrButton", NULL) < 0) {
                printk(KERN_ERR "Unable to allocate IRQ for power button\n");
@@ -92,22 +101,9 @@ static int __init twl4030_pwrbutton_init(void)
                goto free_irq_and_out;
        }
 
-       err = twl4030_i2c_read_u8(TWL4030_MODULE_INT, &value, PWR_IMR1);
-       if (err) {
-               printk(KERN_WARNING "I2C error %d while reading TWL4030"
-                                       " INT PWR_IMR1 register\n", err);
-
-               goto free_input_dev;
-       }
-
-       err = twl4030_i2c_write_u8(TWL4030_MODULE_INT,
-                                  value & (~PWR_PWRON_IRQ), PWR_IMR1);
-       if (err) {
-               printk(KERN_WARNING "I2C error %d while writing TWL4030"
-                                   " INT PWR_IMR1 register\n", err);
-               goto free_input_dev;
-       }
-
+       /* FIXME just pass IRQF_EDGE_FALLING | IRQF_EDGE_RISING
+        * to request_irq(), once MODULE_INT supports them...
+        */
        err = twl4030_i2c_read_u8(TWL4030_MODULE_INT, &value, PWR_EDR1);
        if (err) {
                printk(KERN_WARNING "I2C error %d while reading TWL4030"
@@ -136,19 +132,16 @@ free_irq_and_out:
 out:
        return err;
 }
+module_init(twl4030_pwrbutton_init);
 
 static void __exit twl4030_pwrbutton_exit(void)
 {
        free_irq(TWL4030_PWRIRQ_PWRBTN, NULL);
        input_unregister_device(powerbutton_dev);
        input_free_device(powerbutton_dev);
-
 }
-
-module_init(twl4030_pwrbutton_init);
 module_exit(twl4030_pwrbutton_exit);
 
-MODULE_ALIAS("i2c:twl4030-pwrbutton");
 MODULE_DESCRIPTION("Triton2 Power Button");
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Peter De Schrijver");
index 1ef3b7eaa9a35d2c20c97d4e2295c0cbe6c5feb4..ce66dd4424d8ff20ad7b99e60c51713f63e2e9f4 100644 (file)
 #include <linux/i2c/twl4030.h>
 
 
+static DEFINE_SPINLOCK(pwr_lock);
 static u8 twl4030_pwrirq_mask;
-static u8 twl4030_pwrirq_pending_unmask;
 
 static struct task_struct *twl4030_pwrirq_unmask_thread;
 
 static void twl4030_pwrirq_ack(unsigned int irq) {}
 
-static void twl4030_pwrirq_disableint(unsigned int irq) {}
+static void twl4030_pwrirq_disableint(unsigned int irq)
+{
+       unsigned long flags;
+
+       spin_lock_irqsave(&pwr_lock, flags);
+       twl4030_pwrirq_mask |= 1 << (irq - TWL4030_PWR_IRQ_BASE);
+       if (twl4030_pwrirq_unmask_thread
+                       && twl4030_pwrirq_unmask_thread->state != TASK_RUNNING)
+               wake_up_process(twl4030_pwrirq_unmask_thread);
+       spin_unlock_irqrestore(&pwr_lock, flags);
+}
 
 static void twl4030_pwrirq_enableint(unsigned int irq)
 {
-       twl4030_pwrirq_pending_unmask |= 1 << (irq - TWL4030_PWR_IRQ_BASE);
-       if (twl4030_pwrirq_unmask_thread &&
-               twl4030_pwrirq_unmask_thread->state != TASK_RUNNING)
+       unsigned long flags;
+
+       spin_lock_irqsave(&pwr_lock, flags);
+       twl4030_pwrirq_mask &= ~(1 << (irq - TWL4030_PWR_IRQ_BASE));
+       if (twl4030_pwrirq_unmask_thread
+                       && twl4030_pwrirq_unmask_thread->state != TASK_RUNNING)
                wake_up_process(twl4030_pwrirq_unmask_thread);
+       spin_unlock_irqrestore(&pwr_lock, flags);
 }
 
 static struct irq_chip twl4030_pwrirq_chip = {
@@ -53,48 +67,6 @@ static struct irq_chip twl4030_pwrirq_chip = {
        .unmask = twl4030_pwrirq_enableint,
 };
 
-static void do_twl4030_pwrmodule_irq(unsigned int irq, irq_desc_t *desc)
-{
-       struct irqaction *action;
-       const unsigned int cpu = smp_processor_id();
-
-       desc->status |= IRQ_LEVEL;
-
-       if (!desc->depth) {
-               kstat_cpu(cpu).irqs[irq]++;
-
-               action = desc->action;
-               if (action) {
-                       int ret;
-                       int status = 0;
-                       int retval = 0;
-
-                       do {
-                               ret = action->handler(irq, action->dev_id);
-                               if (ret == IRQ_HANDLED)
-                                       status |= action->flags;
-                               retval |= ret;
-                               action = action->next;
-                       } while (action);
-
-                       if (status & IRQF_SAMPLE_RANDOM)
-                               add_interrupt_randomness(irq);
-
-                       if (retval != IRQ_HANDLED)
-                               printk(KERN_ERR "ISR for TWL4030 power module"
-                                       " irq %d can't handle interrupt\n",
-                                       irq);
-               } else {
-                       local_irq_disable();
-                       twl4030_pwrirq_mask |= 1 << (irq - TWL4030_PWR_IRQ_BASE);
-                       local_irq_enable();
-                       twl4030_i2c_write_u8(TWL4030_MODULE_INT,
-                                            twl4030_pwrirq_mask,
-                                            TWL4030_INT_PWR_IMR1);
-               }
-       }
-}
-
 static void do_twl4030_pwrirq(unsigned int irq, irq_desc_t *desc)
 {
        const unsigned int cpu = smp_processor_id();
@@ -113,6 +85,7 @@ static void do_twl4030_pwrirq(unsigned int irq, irq_desc_t *desc)
                local_irq_enable();
                ret = twl4030_i2c_read_u8(TWL4030_MODULE_INT, &pwr_isr,
                                          TWL4030_INT_PWR_ISR1);
+               local_irq_disable();
                if (ret) {
                        printk(KERN_WARNING
                                "I2C error %d while reading TWL4030"
@@ -122,13 +95,10 @@ static void do_twl4030_pwrirq(unsigned int irq, irq_desc_t *desc)
 
                for (module_irq = TWL4030_PWR_IRQ_BASE; pwr_isr != 0;
                        module_irq++, pwr_isr >>= 1) {
-                       if (pwr_isr & 1) {
-                               irq_desc_t *d = irq_desc + module_irq;
-                               d->handle_irq(module_irq, d);
-                       }
+                       if (pwr_isr & 1)
+                               generic_handle_irq(module_irq);
                }
 
-               local_irq_disable();
                desc->chip->unmask(irq);
        }
 }
@@ -138,22 +108,19 @@ static int twl4030_pwrirq_thread(void *data)
        current->flags |= PF_NOFREEZE;
 
        while (!kthread_should_stop()) {
-               u8 local_unmask;
-
-               local_irq_disable();
-               local_unmask = twl4030_pwrirq_pending_unmask;
-               twl4030_pwrirq_pending_unmask = 0;
-               local_irq_enable();
+               u8 local_mask;
 
-               twl4030_pwrirq_mask &= ~local_unmask;
+               spin_lock_irq(&pwr_lock);
+               local_mask = twl4030_pwrirq_mask;
+               spin_unlock_irq(&pwr_lock);
 
-               twl4030_i2c_write_u8(TWL4030_MODULE_INT, twl4030_pwrirq_mask,
+               twl4030_i2c_write_u8(TWL4030_MODULE_INT, local_mask,
                                     TWL4030_INT_PWR_IMR1);
 
-               local_irq_disable();
-               if (!twl4030_pwrirq_pending_unmask)
+               spin_lock_irq(&pwr_lock);
+               if (local_mask == twl4030_pwrirq_mask)
                        set_current_state(TASK_INTERRUPTIBLE);
-               local_irq_enable();
+               spin_unlock_irq(&pwr_lock);
 
                schedule();
        }
@@ -166,7 +133,6 @@ static int __init twl4030_pwrirq_init(void)
        int i, err;
 
        twl4030_pwrirq_mask = 0xff;
-       twl4030_pwrirq_pending_unmask = 0;
 
        err = twl4030_i2c_write_u8(TWL4030_MODULE_INT, twl4030_pwrirq_mask,
                                        TWL4030_INT_PWR_IMR1);
@@ -191,8 +157,8 @@ static int __init twl4030_pwrirq_init(void)
        }
 
        for (i = TWL4030_PWR_IRQ_BASE; i < TWL4030_PWR_IRQ_END; i++) {
-               set_irq_chip(i, &twl4030_pwrirq_chip);
-               set_irq_handler(i, do_twl4030_pwrmodule_irq);
+               set_irq_chip_and_handler(i, &twl4030_pwrirq_chip,
+                               handle_edge_irq);
                set_irq_flags(i, IRQF_VALID);
        }
 
index 61c831cfdbf4463889dee90ccd03c016efb4a4a9..7852dca4a6dd9e2d46702422cc6e3d2597a79d50 100644 (file)
 #define GPIO_USB_4PIN_ULPI_2430C       (3 << 0)
 
 /* In module TWL4030_MODULE_INT */
-#define REG_PWR_ISR1                   0x00
-#define REG_PWR_IMR1                   0x01
-#define USB_PRES                       (1 << 2)
 #define REG_PWR_EDR1                   0x05
 #define USB_PRES_FALLING               (1 << 4)
 #define USB_PRES_RISING                        (1 << 5)
-#define REG_PWR_SIH_CTRL               0x07
-#define COR                            (1 << 2)
 
 /* bits in OTG_CTRL */
 #define        OTG_XCEIV_OUTPUTS \
@@ -274,6 +269,7 @@ struct twl4030_usb {
        unsigned                vbus:1;
        int                     irq;
        u8                      asleep;
+       bool                    irq_enabled;
 };
 
 /* internal define on top of container_of */
@@ -417,6 +413,8 @@ static void usb_irq_enable(struct twl4030_usb *twl, int rising, int falling)
 {
        u8 val;
 
+       /* FIXME use set_irq_type(...) when that (soon) works */
+
        /* edge setup */
        WARN_ON(twl4030_i2c_read_u8(TWL4030_MODULE_INT,
                                &val, REG_PWR_EDR1) < 0);
@@ -429,34 +427,18 @@ static void usb_irq_enable(struct twl4030_usb *twl, int rising, int falling)
        WARN_ON(twl4030_i2c_write_u8_verify(twl, TWL4030_MODULE_INT,
                                val, REG_PWR_EDR1) < 0);
 
-       /* un-mask interrupt */
-       WARN_ON(twl4030_i2c_read_u8(TWL4030_MODULE_INT,
-                               &val, REG_PWR_IMR1) < 0);
-
-       val &= ~USB_PRES;
-
-       WARN_ON(twl4030_i2c_write_u8_verify(twl, TWL4030_MODULE_INT,
-                               val, REG_PWR_IMR1) < 0);
+       if (!twl->irq_enabled) {
+               enable_irq(twl->irq);
+               twl->irq_enabled = true;
+       }
 }
 
 static void usb_irq_disable(struct twl4030_usb *twl)
 {
-       u8 val;
-
-       /* undo edge setup */
-       WARN_ON(twl4030_i2c_read_u8(TWL4030_MODULE_INT,
-                               &val, REG_PWR_EDR1) < 0);
-       val &= ~(USB_PRES_RISING | USB_PRES_FALLING);
-       WARN_ON(twl4030_i2c_write_u8_verify(twl, TWL4030_MODULE_INT,
-                               val, REG_PWR_EDR1) < 0);
-
-       /* mask interrupt */
-       WARN_ON(twl4030_i2c_read_u8(TWL4030_MODULE_INT,
-                               &val, REG_PWR_IMR1) < 0);
-       val |= USB_PRES;
-
-       WARN_ON(twl4030_i2c_write_u8_verify(twl, TWL4030_MODULE_INT,
-                               val, REG_PWR_IMR1) < 0);
+       if (twl->irq_enabled) {
+               disable_irq(twl->irq);
+               twl->irq_enabled = false;
+       }
 }
 
 static void twl4030_phy_power(struct twl4030_usb *twl, int on)
@@ -565,6 +547,21 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
        struct twl4030_usb *twl = _twl;
        u8 val;
 
+#ifdef CONFIG_LOCKDEP
+       /* WORKAROUND for lockdep forcing IRQF_DISABLED on us, which
+        * we don't want and can't tolerate.  Although it might be
+        * friendlier not to borrow this thread context...
+        */
+       local_irq_enable();
+#endif
+
+       /* FIXME stop accessing PWR_EDR1 ... if nothing else, we
+        * know which edges we told the IRQ to trigger on.  And
+        * there seem to be OTG_specific registers and irqs that
+        * provide the right info without guessing like this:
+        * USB_INT_STS, ID_STATUS, STS_HW_CONDITIONS, etc.
+        */
+
        /* action based on cable attach or detach */
        WARN_ON(twl4030_i2c_read_u8(TWL4030_MODULE_INT,
                                &val, REG_PWR_EDR1) < 0);
@@ -697,7 +694,7 @@ static int __init twl4030_usb_probe(struct platform_device *pdev)
        /* init irq workqueue before request_irq */
        INIT_WORK(&twl->irq_work, twl4030_usb_irq_work);
 
-       usb_irq_disable(twl);
+       twl->irq_enabled = true;
        status = request_irq(twl->irq, twl4030_usb_irq, 0, "twl4030_usb", twl);
        if (status < 0) {
                dev_dbg(&pdev->dev, "can't get IRQ %d, err %d\n",
index 53c67c636ce547c9ec09157f4735e187f13a5ab6..41f66dba4f498eb0a73bf39e34676b0acaa45903 100644 (file)
@@ -347,6 +347,14 @@ static irqreturn_t twl4030_rtc_interrupt(int irq, void *rtc)
        int res;
        u8 rd_reg;
 
+#ifdef CONFIG_LOCKDEP
+       /* WORKAROUND for lockdep forcing IRQF_DISABLED on us, which
+        * we don't want and can't tolerate.  Although it might be
+        * friendlier not to borrow this thread context...
+        */
+       local_irq_enable();
+#endif
+
        res = twl4030_rtc_read_u8(&rd_reg, REG_RTC_STATUS_REG);
        if (res)
                goto out;