Commit aedc256d authored by Felipe Balbi's avatar Felipe Balbi Committed by Wolfram Sang

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

if we allow compiler reorder our writes, we could
fall into a situation where dev->buf_len is reset
for no apparent reason.

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: default avatarFelipe Balbi <balbi@ti.com>
Signed-off-by: default avatarWolfram Sang <w.sang@pengutronix.de>
parent 9aadd70a
...@@ -524,6 +524,9 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap, ...@@ -524,6 +524,9 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
dev->buf = msg->buf; dev->buf = msg->buf;
dev->buf_len = msg->len; dev->buf_len = msg->len;
/* make sure writes to dev->buf_len are ordered */
barrier();
omap_i2c_write_reg(dev, OMAP_I2C_CNT_REG, dev->buf_len); omap_i2c_write_reg(dev, OMAP_I2C_CNT_REG, dev->buf_len);
/* Clear the FIFO Buffers */ /* Clear the FIFO Buffers */
...@@ -581,7 +584,6 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap, ...@@ -581,7 +584,6 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
*/ */
timeout = wait_for_completion_timeout(&dev->cmd_complete, timeout = wait_for_completion_timeout(&dev->cmd_complete,
OMAP_I2C_TIMEOUT); OMAP_I2C_TIMEOUT);
dev->buf_len = 0;
if (timeout == 0) { if (timeout == 0) {
dev_err(dev->dev, "controller timed out\n"); dev_err(dev->dev, "controller timed out\n");
omap_i2c_init(dev); omap_i2c_init(dev);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment