Message-Id: <20251118111323.1452329-1-carlos.song@nxp.com>
Date: Tue, 18 Nov 2025 19:13:23 +0800
From: Carlos Song <carlos.song@....com>
To: aisheng.dong@....com,
	andi.shyti@...nel.org,
	shawnguo@...nel.org,
	s.hauer@...gutronix.de,
	kernel@...gutronix.de,
	festevam@...il.com,
	pandy.gao@....com,
	wsa@...nel.org,
	vz@...ia.com,
	B38611@...escale.com,
	frank.li@....com
Cc: linux-i2c@...r.kernel.org,
	imx@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	Carlos Song <carlos.song@....com>
Subject: [PATCH] i2c: imx-lpi2c: support smbus block read feature

The LPI2C controller automatically transmits a NACK on the last byte
of a receive data command. It transmits the NACK unless the next
command in the TXFIFO is also a receive data command. If the transmit
FIFO is empty when a receive data command completes, a NACK is also
automatically transmitted.

Specially set read 2 bytes command initially. When the RXFIFO receives
count data, controller should immediately read out this value and update
MTDR register to keep the TXFIFO not empty. Set new receive data command
to read other data before the 2nd byte is returned.

Fixes: a55fa9d0e42e ("i2c: imx-lpi2c: add low power i2c bus driver")
Signed-off-by: Carlos Song <carlos.song@....com>

---
I setup an env to test this feature change:

On I.MX93-EVK:
LPI2C5 as slave device and use i2ctool to redesign the slave-eeprom mem:

hexdump -C /sys/bus/i2c/devices/5-1064/slave-eeprom

root@...93evk:~# hexdump -C /sys/bus/i2c/devices/5-1064/slave-eeprom
00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000010  20 00 01 02 03 04 05 06  07 08 09 0a 0b 0c 0d 0e  | ...............|
00000020  0f 10 11 12 13 14 15 16  17 18 19 1a 1b 1c 1d 1e  |................|
00000030  1f ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000040  08 00 01 02 03 04 05 06  07 ff ff ff ff ff ff ff  |................|
00000050  00 ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000060  40 ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |@...............|
00000070  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|

case1: SMBus block read 0x10 register. SMBus len = 32 bytes.
case2: SMBus block read 0x40 register. SMBus len = 8 bytes.
case3: SMBus block read 0x50 register. SMBus len = 0 bytes. It break the
SMBus spec(len = 0).
case4: SMBus block read 0x60 register. SMBus len = 64 bytes. It break
the SMBus spec(len >= 32bytes).

LPI2C3 as master controller to smbus block tead the slave device at 0x64.
Not apply this fix:
root@...93evk:~# i2cget -f -y 3 0x64 0x10 s
Error: Read failed
root@...93evk:~# i2cget -f -y 3 0x64 0x40 s
Error: Read failed
root@...93evk:~# i2cget -f -y 3 0x64 0x50 s
Error: Read failed
root@...93evk:~# i2cget -f -y 3 0x64 0x60 s
0x00

After apply this fix:
root@...93evk:~# i2cget -f -y 3 0x64 0x10 s
0x00 0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a 0x0b 0x0c 0x0d 0x0e 0x0f
0x10 0x11 0x12 0x13 0x14 0x15 0x16 0x17 0x18 0x19 0x1a 0x1b 0x1c 0x1d 0x1e 0x1f
root@...93evk:~# i2cget -f -y 3 0x64 0x40 s
0x00 0x01 0x02 0x03 0x04 0x05 0x06 0x07
root@...93evk:~# i2cget -f -y 3 0x64 0x50 s
[  184.098094] i2c i2c-3: Invalid SMBus block read length
Error: Read failed
root@...93evk:~# i2cget -f -y 3 0x64 0x60 s
[  179.722105] i2c i2c-3: Invalid SMBus block read length
Error: Read failed

---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 93 +++++++++++++++++++++++-------
 1 file changed, 73 insertions(+), 20 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index d882126c1778..39088567db59 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -90,6 +90,7 @@
 #define MRDR_RXEMPTY	BIT(14)
 #define MDER_TDDE	BIT(0)
 #define MDER_RDDE	BIT(1)
+#define MSR_RDF_ASSERT(x) FIELD_GET(MSR_RDF, (x))
 
 #define SCR_SEN		BIT(0)
 #define SCR_RST		BIT(1)
@@ -461,7 +462,7 @@ static bool lpi2c_imx_write_txfifo(struct lpi2c_imx_struct *lpi2c_imx, bool atom
 
 static bool lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx, bool atomic)
 {
-	unsigned int blocklen, remaining;
+	unsigned int remaining;
 	unsigned int temp, data;
 
 	do {
@@ -472,15 +473,6 @@ static bool lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx, bool atomi
 		lpi2c_imx->rx_buf[lpi2c_imx->delivered++] = data & 0xff;
 	} while (1);
 
-	/*
-	 * First byte is the length of remaining packet in the SMBus block
-	 * data read. Add it to msgs->len.
-	 */
-	if (lpi2c_imx->block_data) {
-		blocklen = lpi2c_imx->rx_buf[0];
-		lpi2c_imx->msglen += blocklen;
-	}
-
 	remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
 
 	if (!remaining) {
@@ -493,12 +485,7 @@ static bool lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx, bool atomi
 	lpi2c_imx_set_rx_watermark(lpi2c_imx);
 
 	/* multiple receive commands */
-	if (lpi2c_imx->block_data) {
-		lpi2c_imx->block_data = 0;
-		temp = remaining;
-		temp |= (RECV_DATA << 8);
-		writel(temp, lpi2c_imx->base + LPI2C_MTDR);
-	} else if (!(lpi2c_imx->delivered & 0xff)) {
+	if (!(lpi2c_imx->delivered & 0xff)) {
 		temp = (remaining > CHUNK_DATA ? CHUNK_DATA : remaining) - 1;
 		temp |= (RECV_DATA << 8);
 		writel(temp, lpi2c_imx->base + LPI2C_MTDR);
@@ -536,18 +523,80 @@ static int lpi2c_imx_write_atomic(struct lpi2c_imx_struct *lpi2c_imx,
 	return err;
 }
 
+static unsigned int lpi2c_SMBus_block_read_single_byte(struct lpi2c_imx_struct *lpi2c_imx)
+{
+	unsigned int val, data;
+	int ret;
+
+	ret = readl_poll_timeout(lpi2c_imx->base + LPI2C_MSR, val,
+				 MSR_RDF_ASSERT(val), 1, 1000);
+	if (ret) {
+		dev_err(&lpi2c_imx->adapter.dev, "SMBus read count timeout %d\n", ret);
+		return ret;
+	}
+
+	data = readl(lpi2c_imx->base + LPI2C_MRDR);
+	lpi2c_imx->rx_buf[lpi2c_imx->delivered++] = data & 0xff;
+
+	return data;
+}
+
 static void lpi2c_imx_read_init(struct lpi2c_imx_struct *lpi2c_imx,
 				struct i2c_msg *msgs)
 {
-	unsigned int temp;
+	unsigned int temp, ret, block_len;
 
 	lpi2c_imx->rx_buf = msgs->buf;
 	lpi2c_imx->block_data = msgs->flags & I2C_M_RECV_LEN;
 
 	lpi2c_imx_set_rx_watermark(lpi2c_imx);
-	temp = msgs->len > CHUNK_DATA ? CHUNK_DATA - 1 : msgs->len - 1;
-	temp |= (RECV_DATA << 8);
-	writel(temp, lpi2c_imx->base + LPI2C_MTDR);
+
+	if (!lpi2c_imx->block_data) {
+		temp = msgs->len > CHUNK_DATA ? CHUNK_DATA - 1 : msgs->len - 1;
+		temp |= (RECV_DATA << 8);
+		writel(temp, lpi2c_imx->base + LPI2C_MTDR);
+	} else {
+		/*
+		 * The LPI2C controller automatically transmits a NACK on the last byte
+		 * of a receive data command. It transmits the NACK unless the next
+		 * command in the TXFIFO is also a receive data command. If the transmit
+		 * FIFO is empty when a receive data command completes, a NACK is also
+		 * automatically transmitted.
+		 * So specially set read 2 bytes read command initially. First byte in
+		 * RXFIFO is SMBus block data length, when the data enter the RXFIFO,
+		 * controller should immediately read out and update MTDR register to keep
+		 * the TXFIFO not empty. Second byte is the first byte of block data.
+		 * So the data length of the subsequent block data read command should be
+		 * block_len - 1, because in the first read command, the first byte of block
+		 * data has already been stored in RXFIFO.
+		 */
+		writel((RECV_DATA << 8) | 0x01, lpi2c_imx->base + LPI2C_MTDR);
+
+		/* Read the first byte as block len */
+		block_len = lpi2c_SMBus_block_read_single_byte(lpi2c_imx);
+		if (block_len < 0) {
+			dev_err(&lpi2c_imx->adapter.dev, "SMBus read data length timeout\n");
+			return;
+		}
+
+		/* Confirm SMBus transfer meets protocol */
+		if (block_len == 0 || block_len > I2C_SMBUS_BLOCK_MAX) {
+			dev_err(&lpi2c_imx->adapter.dev, "Invalid SMBus block read length\n");
+			return;
+		}
+
+		/* If just read 1 byte then read out from fifo. No need new command update */
+		if (block_len == 1) {
+			ret = lpi2c_SMBus_block_read_single_byte(lpi2c_imx);
+			if (ret < 0)
+				dev_err(&lpi2c_imx->adapter.dev, "SMBus read data timeout\n");
+			return;
+		}
+
+		/* Block read other length data need to update command again*/
+		writel((RECV_DATA << 8) | (block_len - 2), lpi2c_imx->base + LPI2C_MTDR);
+		lpi2c_imx->msglen += block_len;
+	}
 }
 
 static bool lpi2c_imx_read_chunk_atomic(struct lpi2c_imx_struct *lpi2c_imx)
@@ -599,6 +648,10 @@ static bool is_use_dma(struct lpi2c_imx_struct *lpi2c_imx, struct i2c_msg *msg)
 	if (pm_suspend_in_progress())
 		return false;
 
+	/* DMA is not suitable for SMBus block read */
+	if (msg->flags & I2C_M_RECV_LEN)
+		return false;
+
 	/*
 	 * When the length of data is less than I2C_DMA_THRESHOLD,
 	 * cpu mode is used directly to avoid low performance.
-- 
2.34.1

Powered by blists - more mailing lists