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

On Thu, Nov 20, 2025 at 03:02:26AM +0000, Carlos Song wrote:

quoted

-----Original Message-----
From: Frank Li <frank.li@nxp.com>
Sent: Thursday, November 20, 2025 12:44 AM
To: Carlos Song <redacted>
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:

quoted

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

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

...

quoted

quoted

-	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

quoted

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

quoted

quoted

+		 * 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");

quoted

+			return;
+		}
+
+		/* If just read 1 byte then read out from fifo. No need new command

update */

quoted

+		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");

quoted

+			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

quoted

quoted

+
+		/* Block read other length data need to update command again*/
+		writel((RECV_DATA << 8) | (block_len - 2), lpi2c_imx->base +

LPI2C_MTDR);

quoted

+		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)

quoted

 	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