Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754903Ab3DVHQI (ORCPT ); Mon, 22 Apr 2013 03:16:08 -0400 Received: from mga01.intel.com ([192.55.52.88]:11637 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754496Ab3DVHQG (ORCPT ); Mon, 22 Apr 2013 03:16:06 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.87,524,1363158000"; d="scan'208";a="326036303" Date: Mon, 22 Apr 2013 10:19:43 +0300 From: Mika Westerberg To: Josef Ahmad Cc: Wolfram Sang , Ben Dooks , Jean Delvare , Stefan Roese , Axel Lin , linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, Dirk Brandewie Subject: Re: [PATCH] i2c-designware: fix RX FIFO overrun Message-ID: <20130422071943.GK1283@intel.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2910 Lines: 76 On Fri, Apr 19, 2013 at 07:05:30PM +0100, Josef Ahmad wrote: > >From a969728248c3b439dc97a69e7dac133b5efa34e7 Mon Sep 17 00:00:00 2001 > From: Josef Ahmad > Date: Fri, 19 Apr 2013 17:28:10 +0100 > Subject: [PATCH] i2c-designware: fix RX FIFO overrun > > i2c_dw_xfer_msg() pushes a number of bytes to transmit/receive > to/from the bus into the TX FIFO. > For master-rx transactions, the maximum amount of data that can be > received is calculated depending solely on TX and RX FIFO load. > > This is racy - TX FIFO may contain master-rx data yet to be > processed, which will eventually land into the RX FIFO. This > data is not taken into account and the function may request more > data than the controller is actually capable of storing. > > This patch ensures the driver takes into account the outstanding > master-rx data in TX FIFO to prevent RX FIFO overrun. Can you add something to the changelog to show what the error looks like (a dump from dmesg for example)? > Signed-off-by: Josef Ahmad > --- > drivers/i2c/busses/i2c-designware-core.c | 11 ++++++++++- > drivers/i2c/busses/i2c-designware-core.h | 2 ++ > 2 files changed, 12 insertions(+), 1 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c > index 94fd818..8dbeef1 100644 > --- a/drivers/i2c/busses/i2c-designware-core.c > +++ b/drivers/i2c/busses/i2c-designware-core.c > @@ -426,8 +426,14 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev) > cmd |= BIT(9); > > if (msgs[dev->msg_write_idx].flags & I2C_M_RD) { > + > + /* avoid rx buffer overrun */ > + if (rx_limit - dev->rx_outstanding <= 0) > + break; > + > dw_writel(dev, cmd | 0x100, DW_IC_DATA_CMD); > rx_limit--; > + dev->rx_outstanding++; Instead of adding a new variable, is there something preventing a use of DW_IC_STATUS bits RFNE and TFNF? > } else > dw_writel(dev, cmd | *buf++, DW_IC_DATA_CMD); > tx_limit--; buf_len--; > @@ -480,8 +486,10 @@ i2c_dw_read(struct dw_i2c_dev *dev) > > rx_valid = dw_readl(dev, DW_IC_RXFLR); > > - for (; len > 0 && rx_valid > 0; len--, rx_valid--) > + for (; len > 0 && rx_valid > 0; len--, rx_valid--) { > *buf++ = dw_readl(dev, DW_IC_DATA_CMD); > + dev->rx_outstanding--; > + } > > if (len > 0) { > dev->status |= STATUS_READ_IN_PROGRESS; > @@ -539,6 +547,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) > dev->msg_err = 0; > dev->status = STATUS_IDLE; > dev->abort_source = 0; > + dev->rx_outstanding = 0; > > ret = i2c_dw_wait_bus_not_busy(dev); > if (ret < 0) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/