i2c-omap: close suspected race between omap_i2c_idle() and omap_i2c_isr()

omap_i2c_idle() sets an internal flag, "dev->idle", instructing its
ISR to decline interrupts.  It sets this flag before it actually masks
the interrupts on the I2C controller.  This is problematic, since an
I2C interrupt could arrive after dev->idle is set, but before the
interrupt source is masked.  When this happens, Linux disables the I2C
controller's IRQ, causing all future transactions on the bus to fail.

Symptoms, happening on about 7% of boots:

   irq 56: nobody cared (try booting with the "irqpoll" option)
   <warning traceback here>
   Disabling IRQ #56
   i2c_omap i2c_omap.1: controller timed out

In omap_i2c_idle(), this patch sets dev->idle only after the interrupt
mask write to the I2C controller has left the ARM write buffer.
That's probably the major offender.  For additional prophylaxis, in
omap_i2c_unidle(), the patch clears the dev->idle flag before
interrupts are enabled, rather than afterwards.

The patch has survived twenty-two reboots on the 3430SDP here without
wedging I2C1.  Not absolutely dispositive, but promising!

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 55779f55ace5c0e8839a52265ccf26c410a1223a..ed7e9ad754d421e9d5fa8db77ef8fbcaf664d057 100644 (file)
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -209,22 +209,28 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
        if (dev->iclk != NULL)
                clk_enable(dev->iclk);
        clk_enable(dev->fclk);
+       dev->idle = 0;
        if (dev->iestate)
                omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
-       dev->idle = 0;
 }
 
 static void omap_i2c_idle(struct omap_i2c_dev *dev)
 {
        u16 iv;
 
-       dev->idle = 1;
        dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
        omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
        if (dev->rev1)
                iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
        else
                omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, dev->iestate);
+       /*
+        * The wmb() is to ensure that the I2C interrupt mask write
+        * reaches the I2C controller before the dev->idle store
+        * occurs.
+        */
+       wmb();
+       dev->idle = 1;
        clk_disable(dev->fclk);
        if (dev->iclk != NULL)
                clk_disable(dev->iclk);