[PATCH 6.18 680/752] i2c: imx-lpi2c: fix SMBus block read NACK after byte count

Sasha Levin
2026-02-28T17:58:16Z

From: Carlos Song <carlos.song@nxp.com>

[ Upstream commit efdc383d1cc28d45cbf5a23b5ffa997010aaacb4 ]

The LPI2C controller sends a NACK at the end of a receive command
unless another receive command is already queued in MTDR. During
SMBus block reads, this causes the controller to NACK immediately
after receiving the block length byte, aborting the transfer before
the data bytes are read.

Fix this by queueing a second receive command as soon as the block
length byte is received, keeping MTDR non-empty and ensuring
continuous ACKs. The initial receive command reads the block length,
and the subsequent command reads the remaining data bytes according
to the reported length.

Fixes: a55fa9d0e42e ("i2c: imx-lpi2c: add low power i2c bus driver")
Signed-off-by: Carlos Song <carlos.song@nxp.com>
Cc: <stable@vger.kernel.org> # v4.10+
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
Link: https://lore.kernel.org/r/20260123105459.3448822-1-carlos.song@nxp.com
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/i2c/busses/i2c-imx-lpi2c.c | 107 ++++++++++++++++++++++-------
1 file changed, 83 insertions(+), 24 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index d882126c1778c..519a1ac832a41 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -5,6 +5,7 @@
* Copyright 2016 Freescale Semiconductor, Inc.
*/

+#include <linux/bitfield.h>
#include <linux/clk.h>
#include <linux/completion.h>
#include <linux/delay.h>
@@ -90,6 +91,7 @@
#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)
@@ -461,7 +463,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 +474,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 +486,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 +524,77 @@ static int lpi2c_imx_write_atomic(struct lpi2c_imx_struct *lpi2c_imx,
return err;
}

-static void lpi2c_imx_read_init(struct lpi2c_imx_struct *lpi2c_imx,
- struct i2c_msg *msgs)
+static unsigned int lpi2c_SMBus_block_read_length_byte(struct lpi2c_imx_struct *lpi2c_imx)
{
- unsigned int temp;
+ unsigned int data;
+
+ data = readl(lpi2c_imx->base + LPI2C_MRDR);
+ lpi2c_imx->rx_buf[lpi2c_imx->delivered++] = data & 0xff;
+
+ return data;
+}
+
+static int lpi2c_imx_read_init(struct lpi2c_imx_struct *lpi2c_imx,
+ struct i2c_msg *msgs)
+{
+ unsigned int temp, val, block_len;
+ int ret;

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 sends a NACK after the last byte of a
+ * receive command, unless the next command in MTDR is also a receive command.
+ * If MTDR is empty when a receive completes, a NACK is sent by default.
+ *
+ * To comply with the SMBus block read spec, we start with a 2-byte read:
+ * The first byte in RXFIFO is the block length. Once this byte arrives, the
+ * controller immediately updates MTDR with the next read command, ensuring
+ * continuous ACK instead of NACK.
+ *
+ * The second byte is the first block data byte. Therefore, the subsequent
+ * read command should request (block_len - 1) bytes, since one data byte
+ * has already been read.
+ */
+
+ 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);
+ if (ret) {
+ dev_err(&lpi2c_imx->adapter.dev, "SMBus read count failed %d\n", ret);
+ return ret;
+ }
+
+ /* Read block length byte and confirm this SMBus transfer meets protocol */
+ block_len = lpi2c_SMBus_block_read_length_byte(lpi2c_imx);
+ if (block_len == 0 || block_len > I2C_SMBUS_BLOCK_MAX) {
+ dev_err(&lpi2c_imx->adapter.dev, "Invalid SMBus block read length\n");
+ return -EPROTO;
+ }
+
+ /*
+ * When block_len shows more bytes need to be read, update second read command to
+ * 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.
+ */
+ if (block_len > 1)
+ writel((RECV_DATA << 8) | (block_len - 2), lpi2c_imx->base + LPI2C_MTDR);
+
+ lpi2c_imx->msglen += block_len;
+ msgs->len += block_len;
+ }
+
+ return 0;
}

static bool lpi2c_imx_read_chunk_atomic(struct lpi2c_imx_struct *lpi2c_imx)
@@ -592,6 +639,10 @@ static bool is_use_dma(struct lpi2c_imx_struct *lpi2c_imx, struct i2c_msg *msg)
if (!lpi2c_imx->can_use_dma)
return false;

+ /* DMA is not suitable for SMBus block read */
+ if (msg->flags & I2C_M_RECV_LEN)
+ return false;
+
/*
* A system-wide suspend or resume transition is in progress. LPI2C should use PIO to
* transfer data to avoid issue caused by no ready DMA HW resource.
@@ -609,10 +660,14 @@ static bool is_use_dma(struct lpi2c_imx_struct *lpi2c_imx, struct i2c_msg *msg)
static int lpi2c_imx_pio_xfer(struct lpi2c_imx_struct *lpi2c_imx,
struct i2c_msg *msg)
{
+ int ret;
+
reinit_completion(&lpi2c_imx->complete);

if (msg->flags & I2C_M_RD) {
- lpi2c_imx_read_init(lpi2c_imx, msg);
+ ret = lpi2c_imx_read_init(lpi2c_imx, msg);
+ if (ret)
+ return ret;
lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE | MIER_NDIE);
} else {
lpi2c_imx_write(lpi2c_imx, msg);
@@ -624,8 +679,12 @@ static int lpi2c_imx_pio_xfer(struct lpi2c_imx_struct *lpi2c_imx,
static int lpi2c_imx_pio_xfer_atomic(struct lpi2c_imx_struct *lpi2c_imx,
struct i2c_msg *msg)
{
+ int ret;
+
if (msg->flags & I2C_M_RD) {
- lpi2c_imx_read_init(lpi2c_imx, msg);
+ ret = lpi2c_imx_read_init(lpi2c_imx, msg);
+ if (ret)
+ return ret;
return lpi2c_imx_read_atomic(lpi2c_imx, msg);
}

--
2.51.0