diff mbox series

[2/4] i2c: xlp9xx: Fix issue seen when updating receive length

Message ID 1526454019-32714-3-git-send-email-george.cherian@cavium.com
State Accepted
Headers show
Series i2c-xlp9xx Add support for SMBAlert and minor fixes | expand

Commit Message

George Cherian May 16, 2018, 7 a.m. UTC 
The hardware does not handle updates to the length register gracefully
if the new value is less than the number of bytes received so far. If
this happens, the i2c controller will not stop the receive transaction
properly.

Fix this by ensuring that the updated length is ok. This is done by
making sure that the new length written to hardware is at least few
bytes more than the bytes received so far.

While at that refactor the length updation to a new function.

Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>
Signed-off-by: George Cherian <george.cherian@cavium.com>
---
 drivers/i2c/busses/i2c-xlp9xx.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

Comments

Wolfram Sang May 22, 2018, 12:08 p.m. UTC | #1 
On Wed, May 16, 2018 at 12:00:17AM -0700, George Cherian wrote:
> The hardware does not handle updates to the length register gracefully
> if the new value is less than the number of bytes received so far. If
> this happens, the i2c controller will not stop the receive transaction
> properly.
> 
> Fix this by ensuring that the updated length is ok. This is done by
> making sure that the new length written to hardware is at least few
> bytes more than the bytes received so far.
> 
> While at that refactor the length updation to a new function.
> 
> Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>
> Signed-off-by: George Cherian <george.cherian@cavium.com>

Applied to for-next, thanks!
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index fe54512..c268fde 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -158,9 +158,28 @@  static void xlp9xx_i2c_fill_tx_fifo(struct xlp9xx_i2c_dev *priv)
 	priv->msg_buf += len;
 }
 
+static void xlp9xx_i2c_update_rlen(struct xlp9xx_i2c_dev *priv)
+{
+	u32 val, len;
+
+	/*
+	 * Update receive length. Re-read len to get the latest value,
+	 * and then add 4 to have a minimum value that can be safely
+	 * written. This is to account for the byte read above, the
+	 * transfer in progress and any delays in the register I/O
+	 */
+	val = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_CTRL);
+	len = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_FIFOWCNT) &
+				  XLP9XX_I2C_FIFO_WCNT_MASK;
+	len = max_t(u32, priv->msg_len, len + 4);
+	val = (val & ~XLP9XX_I2C_CTRL_MCTLEN_MASK) |
+			(len << XLP9XX_I2C_CTRL_MCTLEN_SHIFT);
+	xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_CTRL, val);
+}
+
 static void xlp9xx_i2c_drain_rx_fifo(struct xlp9xx_i2c_dev *priv)
 {
-	u32 len, i, val;
+	u32 len, i;
 	u8 rlen, *buf = priv->msg_buf;
 
 	len = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_FIFOWCNT) &
@@ -171,20 +190,13 @@  static void xlp9xx_i2c_drain_rx_fifo(struct xlp9xx_i2c_dev *priv)
 		/* read length byte */
 		rlen = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_MRXFIFO);
 		*buf++ = rlen;
-		len--;
-
 		if (priv->client_pec)
 			++rlen;
 		/* update remaining bytes and message length */
 		priv->msg_buf_remaining = rlen;
 		priv->msg_len = rlen + 1;
 		priv->len_recv = false;
-
-		/* Update transfer length to read only actual data */
-		val = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_CTRL);
-		val = (val & ~XLP9XX_I2C_CTRL_MCTLEN_MASK) |
-			((rlen + 1) << XLP9XX_I2C_CTRL_MCTLEN_SHIFT);
-		xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_CTRL, val);
+		xlp9xx_i2c_update_rlen(priv);
 	} else {
 		len = min(priv->msg_buf_remaining, len);
 		for (i = 0; i < len; i++, buf++)