From patchwork Mon Nov 5 08:04:43 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Felipe Balbi X-Patchwork-Id: 1695361 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id 85BAFDF2AB for ; Mon, 5 Nov 2012 08:12:49 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TVHlj-0006sI-GB; Mon, 05 Nov 2012 08:10:51 +0000 Received: from arroyo.ext.ti.com ([192.94.94.40]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1TVHlf-0006rz-IO for linux-arm-kernel@lists.infradead.org; Mon, 05 Nov 2012 08:10:48 +0000 Received: from dlelxv30.itg.ti.com ([172.17.2.17]) by arroyo.ext.ti.com (8.13.7/8.13.7) with ESMTP id qA58Akup002671; Mon, 5 Nov 2012 02:10:46 -0600 Received: from DFLE73.ent.ti.com (dfle73.ent.ti.com [128.247.5.110]) by dlelxv30.itg.ti.com (8.13.8/8.13.8) with ESMTP id qA58AkxA012278; Mon, 5 Nov 2012 02:10:46 -0600 Received: from dlelxv22.itg.ti.com (172.17.1.197) by dfle73.ent.ti.com (128.247.5.110) with Microsoft SMTP Server id 14.1.323.3; Mon, 5 Nov 2012 02:10:45 -0600 Received: from localhost (h64-12.vpn.ti.com [172.24.64.12]) by dlelxv22.itg.ti.com (8.13.8/8.13.8) with ESMTP id qA58Aj4L017561; Mon, 5 Nov 2012 02:10:45 -0600 From: Felipe Balbi To: Subject: [PATCH v2] i2c: omap: ensure writes to dev->buf_len are ordered Date: Mon, 5 Nov 2012 10:04:43 +0200 Message-ID: <1352102683-2243-1-git-send-email-balbi@ti.com> X-Mailer: git-send-email 1.8.0 In-Reply-To: <20121102085447.GE17063@arwen.pp.htv.fi> References: <20121102085447.GE17063@arwen.pp.htv.fi> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20121105_031047_769801_DD4D8E74 X-CRM114-Status: GOOD ( 14.19 ) X-Spam-Score: -7.6 (-------) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-7.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high trust [192.94.94.40 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -0.7 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Paul Walmsley , Tony Lindgren , Felipe Balbi , Santosh Shilimkar , linux-i2c@vger.kernel.org, Linux OMAP Mailing List , Linux ARM Kernel Mailing List X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.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 --- 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);