diff mbox

i2c: riic: correctly finish transfers

Message ID 20170208024122.13116-1-chris.brandt@renesas.com (mailing list archive)
State Accepted
Delegated to: Geert Uytterhoeven
Headers show

Commit Message

Chris Brandt Feb. 8, 2017, 2:41 a.m. UTC
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>
---
 drivers/i2c/busses/i2c-riic.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

Comments

Wolfram Sang Feb. 9, 2017, 4:44 p.m. UTC | #1
On Tue, Feb 07, 2017 at 09:41:22PM -0500, Chris Brandt wrote:
> 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>

Applied to for-next, thanks!
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
index 6263ea8..8f11d34 100644
--- a/drivers/i2c/busses/i2c-riic.c
+++ b/drivers/i2c/busses/i2c-riic.c
@@ -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" },
 };