i2c: riic: correctly finish transfers
authorChris Brandt <chris.brandt@renesas.com>
Wed, 8 Feb 2017 02:41:22 +0000 (21:41 -0500)
committerWolfram Sang <wsa@the-dreams.de>
Thu, 9 Feb 2017 16:43:31 +0000 (17:43 +0100)
This fixes the condition where the controller has not fully completed its
final transfer and leaves the bus and controller in a undesirable state.

At the end of the last transmitted byte, the existing driver would just
signal for a STOP condition to be transmitted then immediately signal
completion. However, the full STOP procedure might not have fully taken
place by the time the runtime PM shuts off the peripheral clock, leaving
the bus in a suspended state.

Alternatively, the STOP condition on the bus may have completed, but when
the next transaction is requested by the upper layer, not all the
necessary register cleanup was finished from the last transfer which made
the driver return BUS BUSY when it really wasn't.

This patch now makes all transmit and receive transactions wait for the
STOP condition to fully complete before signaling a completed transaction.
With this new method, runtime PM no longer seems to be an issue.

Fixes: 310c18a41450 ("i2c: riic: add driver")
Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
drivers/i2c/busses/i2c-riic.c

index 6263ea82d6ac3b10b97905647c1dc6034b78c100..8f11d347b3ec482815e37d3170fa6abef4537c31 100644 (file)
@@ -80,6 +80,7 @@
 #define ICIER_TEIE     0x40
 #define ICIER_RIE      0x20
 #define ICIER_NAKIE    0x10
+#define ICIER_SPIE     0x08
 
 #define ICSR2_NACKF    0x10
 
@@ -216,11 +217,10 @@ static irqreturn_t riic_tend_isr(int irq, void *data)
                return IRQ_NONE;
        }
 
-       if (riic->is_last || riic->err)
+       if (riic->is_last || riic->err) {
+               riic_clear_set_bit(riic, 0, ICIER_SPIE, RIIC_ICIER);
                writeb(ICCR2_SP, riic->base + RIIC_ICCR2);
-
-       writeb(0, riic->base + RIIC_ICIER);
-       complete(&riic->msg_done);
+       }
 
        return IRQ_HANDLED;
 }
@@ -240,13 +240,13 @@ static irqreturn_t riic_rdrf_isr(int irq, void *data)
 
        if (riic->bytes_left == 1) {
                /* STOP must come before we set ACKBT! */
-               if (riic->is_last)
+               if (riic->is_last) {
+                       riic_clear_set_bit(riic, 0, ICIER_SPIE, RIIC_ICIER);
                        writeb(ICCR2_SP, riic->base + RIIC_ICCR2);
+               }
 
                riic_clear_set_bit(riic, 0, ICMR3_ACKBT, RIIC_ICMR3);
 
-               writeb(0, riic->base + RIIC_ICIER);
-               complete(&riic->msg_done);
        } else {
                riic_clear_set_bit(riic, ICMR3_ACKBT, 0, RIIC_ICMR3);
        }
@@ -259,6 +259,21 @@ static irqreturn_t riic_rdrf_isr(int irq, void *data)
        return IRQ_HANDLED;
 }
 
+static irqreturn_t riic_stop_isr(int irq, void *data)
+{
+       struct riic_dev *riic = data;
+
+       /* read back registers to confirm writes have fully propagated */
+       writeb(0, riic->base + RIIC_ICSR2);
+       readb(riic->base + RIIC_ICSR2);
+       writeb(0, riic->base + RIIC_ICIER);
+       readb(riic->base + RIIC_ICIER);
+
+       complete(&riic->msg_done);
+
+       return IRQ_HANDLED;
+}
+
 static u32 riic_func(struct i2c_adapter *adap)
 {
        return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
@@ -326,6 +341,7 @@ static struct riic_irq_desc riic_irqs[] = {
        { .res_num = 0, .isr = riic_tend_isr, .name = "riic-tend" },
        { .res_num = 1, .isr = riic_rdrf_isr, .name = "riic-rdrf" },
        { .res_num = 2, .isr = riic_tdre_isr, .name = "riic-tdre" },
+       { .res_num = 3, .isr = riic_stop_isr, .name = "riic-stop" },
        { .res_num = 5, .isr = riic_tend_isr, .name = "riic-nack" },
 };