[PATCH 1/2] i2c: imx-lpi2c: fix SMBus block read with PEC

[PATCH 1/2] i2c: imx-lpi2c: fix SMBus block read with PEC

[Michael Walle]

If PEC is enabled, the I2C core sets msgs->len to 2 instead of 1. The
length calculation for the second receive command assumes it's always
1. This will lead to a short read and eventually a timeout because the
PEC byte is never received.

Take msgs->len into account when calculating the remaining bytes.

This was tested on a board which uses an IPMI SSIF BMC.

Fixes: efdc383d1cc2 ("i2c: imx-lpi2c: fix SMBus block read NACK after byte count")
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 7302464c371a..f57e800ac3b5 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -563,7 +563,7 @@ static unsigned int lpi2c_SMBus_block_read_length_byte(struct lpi2c_imx_struct *
 static int lpi2c_imx_read_init(struct lpi2c_imx_struct *lpi2c_imx,
 			       struct i2c_msg *msgs)
 {
-	unsigned int temp, val, block_len;
+	unsigned int temp, val, block_len, remaining;
 	int ret;
 
 	lpi2c_imx->rx_buf = msgs->buf;
@@ -612,9 +612,13 @@ static int lpi2c_imx_read_init(struct lpi2c_imx_struct *lpi2c_imx,
 		 * keep MTDR non-empty and ensuring continuous ACKs. Only update command register
 		 * here. All block bytes will be read out at IRQ handler or lpi2c_imx_read_atomic()
 		 * function.
+		 *
+		 * msgs->len is 1 - or if PEC enabled msgs->len is 2. Two bytes
+		 * have already been requested.
 		 */
-		if (block_len > 1)
-			writel((RECV_DATA << 8) | (block_len - 2), lpi2c_imx->base + LPI2C_MTDR);
+		remaining = msgs->len + block_len - 2;
+		if (remaining)
+			writel((RECV_DATA << 8) | (remaining - 1), lpi2c_imx->base + LPI2C_MTDR);
 
 		lpi2c_imx->msglen += block_len;
 		msgs->len += block_len;
-- 
2.47.3

[PATCH 2/2] i2c: imx-lpi2c: don't treat a NACK in SMBus block read as an error

[Michael Walle]

An SMBus device might indicate that a value isn't ready yet by asserting
a NACK.
This will lead to the following error message which will flood the
kernel log for some device like an IPMI SSIF BMC:

   [    4.277005] i2c i2c-5: SMBus read count failed -110

Stop the polling if NDF is set and return -EIO, just like everywhere
else. While at it, drop the MSR_RDF_ASSERTED() macro, it's just one bit,
no need for FIELD_GET.

Fixes: efdc383d1cc2 ("i2c: imx-lpi2c: fix SMBus block read NACK after byte count")
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index f57e800ac3b5..583155802e8a 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -92,7 +92,6 @@
 #define MRDR_RXEMPTY	BIT(14)
 #define MDER_TDDE	BIT(0)
 #define MDER_RDDE	BIT(1)
-#define MSR_RDF_ASSERTED(x) FIELD_GET(MSR_RDF, (x))
 
 #define SCR_SEN		BIT(0)
 #define SCR_RST		BIT(1)
@@ -594,7 +593,10 @@ static int lpi2c_imx_read_init(struct lpi2c_imx_struct *lpi2c_imx,
 		writel((RECV_DATA << 8) | 0x01, lpi2c_imx->base + LPI2C_MTDR);
 
 		ret = readl_poll_timeout(lpi2c_imx->base + LPI2C_MSR, val,
-					 MSR_RDF_ASSERTED(val), 1, 1000);
+					 (val & (MSR_RDF | MSR_NDF)), 1, 1000);
+		if (val & MSR_NDF)
+			return -EIO;
+
 		if (ret) {
 			dev_err(&lpi2c_imx->adapter.dev, "SMBus read count failed %d\n", ret);
 			return ret;
-- 
2.47.3