linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
To: w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org
Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>,
	Linux OMAP Mailing List
	<linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Linux ARM Kernel Mailing List
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>,
	Santosh Shilimkar
	<santosh.shilimkar-l0cyMroinI0@public.gmane.org>,
	Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
Subject: [PATCH v2] i2c: omap: ensure writes to dev->buf_len are ordered
Date: Mon, 5 Nov 2012 10:04:43 +0200	[thread overview]
Message-ID: <1352102683-2243-1-git-send-email-balbi@ti.com> (raw)
In-Reply-To: <20121102085447.GE17063-S8G//mZuvNWo5Im9Ml3/Zg@public.gmane.org>

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: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
---

Changes since v1:
	- use barrier() instead of wmb()

Note: this version was compile-tested only

 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..ba03bec 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;
+	barrier();
 
 	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);
-- 
1.8.0

  parent reply	other threads:[~2012-11-05  8:04 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-25  9:00 [PATCH] i2c: omap: ensure writes to dev->buf_len are ordered Felipe Balbi
     [not found] ` <1351155648-20429-1-git-send-email-balbi-l0cyMroinI0@public.gmane.org>
2012-10-25  9:16   ` Shubhrajyoti Datta
2012-10-26 23:01   ` Paul Walmsley
2012-10-27 10:50     ` Santosh Shilimkar
2012-10-27 15:59       ` Paul Walmsley
     [not found]         ` <alpine.DEB.2.00.1210271545390.16409-rwI8Ez+7Ko+d5PgPZx9QOdBPR1lH4CV8@public.gmane.org>
2012-10-28  4:11           ` Santosh Shilimkar
2012-10-25 16:38 ` Kevin Hilman
     [not found]   ` <8739124idt.fsf-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org>
2012-10-25 18:03     ` Felipe Balbi
2012-11-01 22:23 ` Wolfram Sang
     [not found]   ` <20121101222316.GC22956-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-11-02  8:54     ` Felipe Balbi
     [not found]       ` <20121102085447.GE17063-S8G//mZuvNWo5Im9Ml3/Zg@public.gmane.org>
2012-11-05  8:04         ` Felipe Balbi [this message]
2012-11-14 11:20           ` [PATCH v2] " Wolfram Sang
     [not found]             ` <20121114112050.GG5954-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-11-14 14:22               ` [PATCH v3] " Felipe Balbi
2012-11-14 16:46                 ` Wolfram Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1352102683-2243-1-git-send-email-balbi@ti.com \
    --to=balbi-l0cymroini0@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org \
    --cc=santosh.shilimkar-l0cyMroinI0@public.gmane.org \
    --cc=tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org \
    --cc=w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).