[PATCH] i2c: imx-lpi2c: support smbus block read feature

[PATCH] i2c: imx-lpi2c: support smbus block read feature

[Carlos Song]

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@nxp.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@imx93evk:~# 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@imx93evk:~# i2cget -f -y 3 0x64 0x10 s
Error: Read failed
root@imx93evk:~# i2cget -f -y 3 0x64 0x40 s
Error: Read failed
root@imx93evk:~# i2cget -f -y 3 0x64 0x50 s
Error: Read failed
root@imx93evk:~# i2cget -f -y 3 0x64 0x60 s
0x00

After apply this fix:
root@imx93evk:~# 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@imx93evk:~# i2cget -f -y 3 0x64 0x40 s
0x00 0x01 0x02 0x03 0x04 0x05 0x06 0x07
root@imx93evk:~# i2cget -f -y 3 0x64 0x50 s
[  184.098094] i2c i2c-3: Invalid SMBus block read length
Error: Read failed
root@imx93evk:~# 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

Re: [PATCH] i2c: imx-lpi2c: support smbus block read feature

[Frank Li]

On Tue, Nov 18, 2025 at 07:13:23PM +0800, Carlos Song wrote:
> 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@nxp.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@imx93evk:~# 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@imx93evk:~# i2cget -f -y 3 0x64 0x10 s
> Error: Read failed
> root@imx93evk:~# i2cget -f -y 3 0x64 0x40 s
> Error: Read failed
> root@imx93evk:~# i2cget -f -y 3 0x64 0x50 s
> Error: Read failed
> root@imx93evk:~# i2cget -f -y 3 0x64 0x60 s
> 0x00
>
> After apply this fix:
> root@imx93evk:~# i2cget -f -y 3 0x64 0x10 s

where show your read 32 byte, do you means "i2cget -f -y 3 0x64 0x10 s 0x20"?

> 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

according to above dump, the data from 0x11, not 0x10.

Frank

> root@imx93evk:~# i2cget -f -y 3 0x64 0x40 s
> 0x00 0x01 0x02 0x03 0x04 0x05 0x06 0x07
> root@imx93evk:~# i2cget -f -y 3 0x64 0x50 s
> [  184.098094] i2c i2c-3: Invalid SMBus block read length
> Error: Read failed
> root@imx93evk:~# 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
>

RE: [PATCH] i2c: imx-lpi2c: support smbus block read feature

[Carlos Song]

> -----Original Message-----
> From: Frank Li <frank.li@nxp.com>
> Sent: Wednesday, November 19, 2025 12:45 AM
> To: Carlos Song <carlos.song@nxp.com>
> Cc: Aisheng Dong <aisheng.dong@nxp.com>; andi.shyti@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; pandy.gao@nxp.com; wsa@kernel.org; vz@mleia.com;
> B38611@freescale.com; linux-i2c@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] i2c: imx-lpi2c: support smbus block read feature
> 
> On Tue, Nov 18, 2025 at 07:13:23PM +0800, Carlos Song wrote:
> > 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@nxp.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@imx93evk:~# 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@imx93evk:~# i2cget -f -y 3 0x64 0x10 s
> > Error: Read failed
> > root@imx93evk:~# i2cget -f -y 3 0x64 0x40 s
> > Error: Read failed
> > root@imx93evk:~# i2cget -f -y 3 0x64 0x50 s
> > Error: Read failed
> > root@imx93evk:~# i2cget -f -y 3 0x64 0x60 s
> > 0x00
> >
> > After apply this fix:
> > root@imx93evk:~# i2cget -f -y 3 0x64 0x10 s
> 
> where show your read 32 byte, do you means "i2cget -f -y 3 0x64 0x10 s 0x20"?
> 
> > 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
> 
> according to above dump, the data from 0x11, not 0x10.
> 
> Frank
> 
Hi, Frank

Wow! Important point!

SMBUS protocol shows. RX buffer[0] is Byte Count. Then RX buffer[1] [2] [3].. dataN.
[Slave Addr][Command][Read] → [Byte Count][Data0][Data1]...[DataN]

The printed values are indeed the actual data bytes returned by the device, starting from the “first data byte” and excluding the "SMBus protocol length byte".

Raw code in i2ctool is:

/* Returns the number of read bytes */
__s32 i2c_smbus_read_block_data(int file, __u8 command, __u8 *values)
{
	union i2c_smbus_data data;
	int i, err;

	err = i2c_smbus_access(file, I2C_SMBUS_READ, command,
			       I2C_SMBUS_BLOCK_DATA, &data);
	if (err < 0)
		return err;

	for (i = 1; i <= data.block[0]; i++)
		values[i-1] = data.block[i];
	return data.block[0];
}

int main(int argc, char *argv[])
{
...
	case I2C_SMBUS_BLOCK_DATA:
		res = i2c_smbus_read_block_data(file, daddress, block_data);
		break;
...
	if (size == I2C_SMBUS_BLOCK_DATA ||
	    size == I2C_SMBUS_I2C_BLOCK_DATA) {
		int i;

		for (i = 0; i < res - 1; ++i)
			printf("0x%02hhx ", block_data[i]);
		printf("0x%02hhx\n", block_data[res - 1]);
...

Carlos
> > root@imx93evk:~# i2cget -f -y 3 0x64 0x40 s
> > 0x00 0x01 0x02 0x03 0x04 0x05 0x06 0x07 root@imx93evk:~# i2cget -f -y
> > 3 0x64 0x50 s [  184.098094] i2c i2c-3: Invalid SMBus block read
> > length
> > Error: Read failed
> > root@imx93evk:~# 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
> >

Re: [PATCH] i2c: imx-lpi2c: support smbus block read feature

[Frank Li]

On Tue, Nov 18, 2025 at 07:13:23PM +0800, Carlos Song wrote:
> 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@nxp.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@imx93evk:~# 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@imx93evk:~# i2cget -f -y 3 0x64 0x10 s
> Error: Read failed
> root@imx93evk:~# i2cget -f -y 3 0x64 0x40 s
> Error: Read failed
> root@imx93evk:~# i2cget -f -y 3 0x64 0x50 s
> Error: Read failed
> root@imx93evk:~# i2cget -f -y 3 0x64 0x60 s
> 0x00
>
> After apply this fix:
> root@imx93evk:~# 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@imx93evk:~# i2cget -f -y 3 0x64 0x40 s
> 0x00 0x01 0x02 0x03 0x04 0x05 0x06 0x07
> root@imx93evk:~# i2cget -f -y 3 0x64 0x50 s
> [  184.098094] i2c i2c-3: Invalid SMBus block read length
> Error: Read failed
> root@imx93evk:~# 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.

looks like transfer STOP? You get data, it should be ACK when received data.

> 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.

Add empty line here.
> +		 * 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.

Remove "should",
why keep TXFIFO no empty here, data should be in RXFIFO.

Frank
> 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
>

RE: [PATCH] i2c: imx-lpi2c: support smbus block read feature

[Carlos Song]

> -----Original Message-----
> From: Frank Li <frank.li@nxp.com>
> Sent: Thursday, November 20, 2025 12:44 AM
> To: Carlos Song <carlos.song@nxp.com>
> Cc: Aisheng Dong <aisheng.dong@nxp.com>; andi.shyti@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; pandy.gao@nxp.com; wsa@kernel.org; vz@mleia.com;
> B38611@freescale.com; linux-i2c@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] i2c: imx-lpi2c: support smbus block read feature
> 
> On Tue, Nov 18, 2025 at 07:13:23PM +0800, Carlos Song wrote:
> > 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@nxp.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@imx93evk:~# 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@imx93evk:~# i2cget -f -y 3 0x64 0x10 s
> > Error: Read failed
> > root@imx93evk:~# i2cget -f -y 3 0x64 0x40 s
> > Error: Read failed
> > root@imx93evk:~# i2cget -f -y 3 0x64 0x50 s
> > Error: Read failed
> > root@imx93evk:~# i2cget -f -y 3 0x64 0x60 s
> > 0x00
> >
> > After apply this fix:
> > root@imx93evk:~# 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@imx93evk:~# i2cget -f -y 3 0x64 0x40 s
> > 0x00 0x01 0x02 0x03 0x04 0x05 0x06 0x07 root@imx93evk:~# i2cget -f -y
> > 3 0x64 0x50 s [  184.098094] i2c i2c-3: Invalid SMBus block read
> > length
> > Error: Read failed
> > root@imx93evk:~# 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.
> 
> looks like transfer STOP? You get data, it should be ACK when received data.
> 

According to LPI2C RM MTDR register shows:
When controller need to read data, we should send command word first. 
     Bit10| bit9 | bit8 |          bit7~0
Receive (DATA[7:0] + 1) bytes    byte counter

DATA[7:0] is used as a byte counter. Receive that many bytes and check each for a data match (if configured) before storing the received data in the receive FIFO.
We can prefill the bytes count to controller, controller will auto ACK after every bytes until count is exhausted. Then controller send auto NACK.

> > 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.
> 
> Add empty line here.
> > +		 * 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.
> 
> Remove "should",
> why keep TXFIFO no empty here, data should be in RXFIFO.
> 

When controller read, we should write command to MTDR(TXFIFO) first. Then read data from RXFIFO.
But LPI2C controller HW behaviour always send NACK when finished the command.
According to SMbus spec, controller shouldn't send NACK when first bytes received. So continued read commands are needed.
But if next new command has been prefilled to MTDR(TXFIFO) before current read command finished, controller will continue ACK not NACK when first command finished. So two read commands are connected with ACK.
So we keep TXFIFO not empty. If confusing, I can change to "MTDR not empty"(MTDR and TXFIFO is the same register).

> Frank
> > 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
> >

Re: [PATCH] i2c: imx-lpi2c: support smbus block read feature

[Frank Li]

On Thu, Nov 20, 2025 at 03:02:26AM +0000, Carlos Song wrote:
>
>
> > -----Original Message-----
> > From: Frank Li <frank.li@nxp.com>
> > Sent: Thursday, November 20, 2025 12:44 AM
> > To: Carlos Song <carlos.song@nxp.com>
> > Cc: Aisheng Dong <aisheng.dong@nxp.com>; andi.shyti@kernel.org;
> > shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> > festevam@gmail.com; pandy.gao@nxp.com; wsa@kernel.org; vz@mleia.com;
> > B38611@freescale.com; linux-i2c@vger.kernel.org; imx@lists.linux.dev;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] i2c: imx-lpi2c: support smbus block read feature
> >
> > On Tue, Nov 18, 2025 at 07:13:23PM +0800, Carlos Song wrote:
> > > 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@nxp.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
> > >
...
> > > -	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.
> >
> > looks like transfer STOP? You get data, it should be ACK when received data.
> >
>
> According to LPI2C RM MTDR register shows:
> When controller need to read data, we should send command word first.
>      Bit10| bit9 | bit8 |          bit7~0
> Receive (DATA[7:0] + 1) bytes    byte counter
>
> DATA[7:0] is used as a byte counter. Receive that many bytes and check each for a data match (if configured) before storing the received data in the receive FIFO.
> We can prefill the bytes count to controller, controller will auto ACK after every bytes until count is exhausted. Then controller send auto NACK.
>

Thank your for your explain. This may IC design choice or bug, look like
I2C controller should stall SCL to wait for new command?

Assume byte counter is 0, (1 bytes transfer)  [0x100]

SCL: 1,  2,  3,  4,  5,  6,  7,  8,  9,
SDA: B0, B1, B2, B3, B4, B5, B6, B7, ACK.
                                     ^ Do you means here is NACK when
MTDR empty?

> > > +		 * command in the TXFIFO is also a receive data command. If the
> > > +		/* 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;
> > > +		}

If irq/schedule happen here, you may not fill MTDR in time, so MTDR maybe
empty,  You just have 9 SCL time to fill new data to MTDR.

Frank

> > > +
> > > +		/* 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
> > >

RE: [PATCH] i2c: imx-lpi2c: support smbus block read feature

[Carlos Song]

> -----Original Message-----
> From: Frank Li <frank.li@nxp.com>
> Sent: Friday, November 21, 2025 12:17 AM
> To: Carlos Song <carlos.song@nxp.com>
> Cc: Aisheng Dong <aisheng.dong@nxp.com>; andi.shyti@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; pandy.gao@nxp.com; wsa@kernel.org; vz@mleia.com;
> B38611@freescale.com; linux-i2c@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] i2c: imx-lpi2c: support smbus block read feature
> 
> On Thu, Nov 20, 2025 at 03:02:26AM +0000, Carlos Song wrote:
> >
> >
> > > -----Original Message-----
> > > From: Frank Li <frank.li@nxp.com>
> > > Sent: Thursday, November 20, 2025 12:44 AM
> > > To: Carlos Song <carlos.song@nxp.com>
> > > Cc: Aisheng Dong <aisheng.dong@nxp.com>; andi.shyti@kernel.org;
> > > shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> > > festevam@gmail.com; pandy.gao@nxp.com; wsa@kernel.org;
> vz@mleia.com;
> > > B38611@freescale.com; linux-i2c@vger.kernel.org;
> > > imx@lists.linux.dev; linux-arm-kernel@lists.infradead.org;
> > > linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH] i2c: imx-lpi2c: support smbus block read
> > > feature
> > >
> > > On Tue, Nov 18, 2025 at 07:13:23PM +0800, Carlos Song wrote:
> > > > 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@nxp.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
> > > >
> ...
> > > > -	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.
> > >
> > > looks like transfer STOP? You get data, it should be ACK when received data.
> > >
> >
> > According to LPI2C RM MTDR register shows:
> > When controller need to read data, we should send command word first.
> >      Bit10| bit9 | bit8 |          bit7~0
> > Receive (DATA[7:0] + 1) bytes    byte counter
> >
> > DATA[7:0] is used as a byte counter. Receive that many bytes and check each
> for a data match (if configured) before storing the received data in the receive
> FIFO.
> > We can prefill the bytes count to controller, controller will auto ACK after
> every bytes until count is exhausted. Then controller send auto NACK.
> >
> 
> Thank your for your explain. This may IC design choice or bug, look like I2C
> controller should stall SCL to wait for new command?
> 
> Assume byte counter is 0, (1 bytes transfer)  [0x100]
> 
> SCL: 1,  2,  3,  4,  5,  6,  7,  8,  9,
> SDA: B0, B1, B2, B3, B4, B5, B6, B7, ACK.
>                                      ^ Do you means here is NACK
> when MTDR empty?
Yes, it is.
> 
> > > > +		 * command in the TXFIFO is also a receive data command. If the
> > > > +		/* 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;
> > > > +		}
> 
> If irq/schedule happen here, you may not fill MTDR in time, so MTDR maybe
> empty,  You just have 9 SCL time to fill new data to MTDR.
> 
> Frank
> 

Hi, Frank

Yes. Your are right. That is normal case. Timeout may happen when read smbus len when system in high stress.
That is not avoidable.

> > > > +
> > > > +		/* 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
> > > >