Thread (2 messages) 2 messages, 2 authors, 2012-10-25
STALE4973d

[PATCH] i2c: omap: ensure writes to dev->buf_len are ordered

From: Kevin Hilman <hidden>
Date: 2012-10-25 16:38:06
Also in: linux-i2c, linux-omap

Possibly related (same subject, not in this thread)

+Paul

Felipe Balbi [off-list ref] writes:
if we allow compiler reorder our writes, we could
fall into a situation where dev->buf_len is reset
for no apparent reason.
Any chance this would help with the bug Paul found with I2C timeouts on
beagle userspace startup?

Kevin
quoted hunk
This bug was found with a simple script which would
transfer data to an i2c client from 1 to 1024 bytes
(a simple for loop), when we got to transfer sizes
bigger than the fifo size, dev->buf_len was reset
to zero before we had an oportunity to handle XDR
Interrupt. Because dev->buf_len was zero, we entered
omap_i2c_transmit_data() to transfer zero bytes,
which would mean we would just silently exit
omap_i2c_transmit_data() without actually writing
anything to DATA register. That would cause XDR
IRQ to trigger forever and we would never transfer
the remaining bytes.

After adding the memory barrier, we also drop resetting
dev->buf_len to zero in omap_i2c_xfer_msg() because
both omap_i2c_transmit_data() and omap_i2c_receive_data()
will act until dev->buf_len reaches zero, rendering the
other write in omap_i2c_xfer_msg() redundant.

This patch has been tested with pandaboard for a few
iterations of the script mentioned above.

Signed-off-by: Felipe Balbi <redacted>
---

This bug has been there forever, but it's quite annoying.
I think it deserves being pushed upstream during this -rc
cycle, but if Wolfram decides to wait until v3.8, I don't
mind.

 drivers/i2c/busses/i2c-omap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index db31eae..1ec4e6e 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -521,6 +521,7 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 	/* REVISIT: Could the STB bit of I2C_CON be used with probing? */
 	dev->buf = msg->buf;
 	dev->buf_len = msg->len;
+	wmb();
 
 	omap_i2c_write_reg(dev, OMAP_I2C_CNT_REG, dev->buf_len);
 
@@ -579,7 +580,6 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 	 */
 	timeout = wait_for_completion_timeout(&dev->cmd_complete,
 						OMAP_I2C_TIMEOUT);
-	dev->buf_len = 0;
 	if (timeout == 0) {
 		dev_err(dev->dev, "controller timed out\n");
 		omap_i2c_init(dev);
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help