[PATCH 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus

[PATCH 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus

[Vincent Jardin]

i2c-imx rejects a SMBus Block Read byte count of 0 (valid per SMBus 3.1
6.5.7) and it returns without a NACK+STOP, leaving the target
holding SDA so the bus is stuck until a power cycle occur.

The same bug is occuring with two independently introduced spots, so the
fix is two patches with their respective Fixes: tags and backport ranges:

  1/2  atomic/polling path       Fixes: 8e8782c71595   v3.16+
  2/2  IRQ-driven state machine  Fixes: 5f5c2d4579ca   v6.13+

Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
Vincent Jardin (2):
      i2c: imx: fix locked bus on SMBus block-read of 0 (atomic)
      i2c: imx: fix locked bus on SMBus block-read of 0 (IRQ)

 drivers/i2c/busses/i2c-imx.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)
---
base-commit: 6916d5703ddf9a38f1f6c2cc793381a24ee914c6
change-id: 20260525-for-upstream-i2c-lx2160-fix-v1-0cba0a0093e5

Best regards,
-- 
Vincent Jardin <vjardin@free.fr>

[PATCH 1/2] i2c: imx: fix locked bus on SMBus block-read of 0 (atomic)

[Vincent Jardin]

SMBus 3.1 6.5.7 allows a Block Read byte count of 0, but the atomic
(polling) path rejects it as -EPROTO. Worse, it returns without a
NACK+STOP: the next receive cycle has already started, so the target
keeps holding SDA and the bus stays stuck until a power cycle for
this i2c controller.

Accept count=0: NACK the in-flight dummy byte (TXAK) and extend msgs->len
so the existing last-byte handling emits STOP. The dummy byte is
discarded; block-read callers only consume buf[0..count-1].

The interrupt-driven path has the same flaw from a later commit and is
fixed separately, as it carries a different Fixes:

Fixes: 8e8782c71595 ("i2c: imx: add SMBus block read support")
Cc: <stable@vger.kernel.org> # v3.16+
Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 drivers/i2c/busses/i2c-imx.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index a208fefd3c3b..0cd4f5892591 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1436,8 +1436,19 @@ static int i2c_imx_atomic_read(struct imx_i2c_struct *i2c_imx,
 		 */
 		if ((!i) && block_data) {
 			len = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
-			if ((len == 0) || (len > I2C_SMBUS_BLOCK_MAX))
+			if (len > I2C_SMBUS_BLOCK_MAX)
 				return -EPROTO;
+			if (len == 0) {
+				/*
+				 * SMBus 3.1 6.5.7: support count byte of 0.
+				 */
+				temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+				temp |= I2CR_TXAK;
+				imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+				msgs->buf[0] = 0;
+				msgs->len = 2;
+				continue;
+			}
 			dev_dbg(&i2c_imx->adapter.dev,
 				"<%s> read length: 0x%X\n",
 				__func__, len);

-- 
2.43.0

[PATCH 2/2] i2c: imx: fix locked bus on SMBus block-read of 0 (IRQ)

[Vincent Jardin]

SMBus 3.1 6.5.7 allows a Block Read byte count of 0, but the
interrupt-driven block-read state machine rejects it as -EPROTO. Worse,
it returns without a NACK+STOP: the next receive cycle has already
started, so the target keeps holding SDA and the bus stays stuck until a
power cycle of this i2c controller.

Accept count=0: NACK the in-flight dummy byte (TXAK) and set msg->len to
2 so i2c_imx_isr_read_continue() emits STOP via its normal last-byte
path. The dummy byte is discarded; block-read callers only consume
buf[0..count-1].

While here, return early on the I2C_SMBUS_BLOCK_MAX error path instead
of falling through and overwriting msg->len/msg->buf with the rejected
count byte.

The atomic path regressed earlier (v3.16) and is fixed separately; this
patch covers only the v6.13 state-machine rework.

Fixes: 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma mode")
Cc: <stable@vger.kernel.org> # v6.13+
Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 drivers/i2c/busses/i2c-imx.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 0cd4f5892591..8792cb5cb9a8 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1061,11 +1061,26 @@ static inline enum imx_i2c_state i2c_imx_isr_read_continue(struct imx_i2c_struct
 static inline void i2c_imx_isr_read_block_data_len(struct imx_i2c_struct *i2c_imx)
 {
 	u8 len = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+	unsigned int temp;
 
-	if (len == 0 || len > I2C_SMBUS_BLOCK_MAX) {
+	if (len > I2C_SMBUS_BLOCK_MAX) {
 		i2c_imx->isr_result = -EPROTO;
 		i2c_imx->state = IMX_I2C_STATE_FAILED;
 		wake_up(&i2c_imx->queue);
+		return;
+	}
+
+	if (len == 0) {
+		/*
+		 * SMBus 3.1 6.5.7 "Block Write/Read": byte count can be 0
+		 */
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+		temp |= I2CR_TXAK;
+		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+		i2c_imx->msg->buf[i2c_imx->msg_buf_idx++] = 0;
+		i2c_imx->msg->len = 2;
+		return;
 	}
 	i2c_imx->msg->len += len;
 	i2c_imx->msg->buf[i2c_imx->msg_buf_idx++] = len;

-- 
2.43.0

[PATCH v2 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus

[Vincent Jardin]

i2c-imx rejects a SMBus Block Read byte count of 0 (valid per SMBus 3.1
6.5.7) and it returns without a NACK+STOP, leaving the target
holding SDA so the bus is stuck until a power cycle occur.

The same bug is occuring with two independently introduced spots, so the
fix is two patches with their respective Fixes: tags and backport ranges:

  1/2  atomic/polling path       Fixes: 8e8782c71595   v3.16+
  2/2  IRQ-driven state machine  Fixes: 5f5c2d4579ca   v6.13+

Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
Changes in v2:
- Handle when count > I2C_SMBUS_BLOCK_MAX the same way as count == 0
  Reported by the Sashiko AI review on v1.

---
Vincent Jardin (2):
      i2c: imx: fix locked bus on SMBus block-read of 0 (atomic)
      i2c: imx: fix locked bus on SMBus block-read of 0 (IRQ)

 drivers/i2c/busses/i2c-imx.c | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)
---
base-commit: 6916d5703ddf9a38f1f6c2cc793381a24ee914c6
change-id: 20260525-for-upstream-i2c-lx2160-fix-v1-0cba0a0093e5

Best regards,
-- 
Vincent Jardin <vjardin@free.fr>

[PATCH v2 1/2] i2c: imx: fix locked bus on SMBus block-read of 0 (atomic)

[Vincent Jardin]

SMBus 3.1 6.5.7 allows a Block Read byte count of 0, but the atomic
(polling) path rejects it as -EPROTO. Worse, it returns without a
NACK+STOP: the next receive cycle has already started, so the target
keeps holding SDA and the bus stays stuck until a power cycle for
this i2c controller.

Reading I2DR to obtain the count likewise arms the next byte on the
count > I2C_SMBUS_BLOCK_MAX path, which also returned -EPROTO directly
and left the bus held.

Handle both: NACK the in-flight dummy byte (TXAK) and extend msgs->len so
the existing last-byte handling emits STOP; the dummy byte is discarded.
A count of 0 is a valid empty block read; a count above
I2C_SMBUS_BLOCK_MAX is still reported as -EPROTO, but only after the bus
has been released.

The interrupt-driven path has the same flaw from a later commit and is
fixed separately, as it carries a different Fixes: tag and stable range.

Fixes: 8e8782c71595 ("i2c: imx: add SMBus block read support")
Cc: <stable@vger.kernel.org> # v3.16+
Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 drivers/i2c/busses/i2c-imx.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index a208fefd3c3b..14107e1ad413 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1415,6 +1415,7 @@ static int i2c_imx_atomic_read(struct imx_i2c_struct *i2c_imx,
 	int i, result;
 	unsigned int temp;
 	int block_data = msgs->flags & I2C_M_RECV_LEN;
+	int block_err = 0;
 
 	result = i2c_imx_prepare_read(i2c_imx, msgs, false);
 	if (result)
@@ -1436,8 +1437,20 @@ static int i2c_imx_atomic_read(struct imx_i2c_struct *i2c_imx,
 		 */
 		if ((!i) && block_data) {
 			len = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
-			if ((len == 0) || (len > I2C_SMBUS_BLOCK_MAX))
-				return -EPROTO;
+			if ((len == 0) || (len > I2C_SMBUS_BLOCK_MAX)) {
+				/*
+				 * SMBus 3.1 6.5.7: support count byte of 0.
+				 * I2C_SMBUS_BLOCK_MAX case should not hold the SDA either.
+				 */
+				if (len > I2C_SMBUS_BLOCK_MAX)
+					block_err = -EPROTO;
+				temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+				temp |= I2CR_TXAK;
+				imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+				msgs->buf[0] = 0;
+				msgs->len = 2;
+				continue;
+			}
 			dev_dbg(&i2c_imx->adapter.dev,
 				"<%s> read length: 0x%X\n",
 				__func__, len);
@@ -1485,7 +1498,7 @@ static int i2c_imx_atomic_read(struct imx_i2c_struct *i2c_imx,
 			"<%s> read byte: B%d=0x%X\n",
 			__func__, i, msgs->buf[i]);
 	}
-	return 0;
+	return block_err;
 }
 
 static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs,

-- 
2.43.0

[PATCH v2 2/2] i2c: imx: fix locked bus on SMBus block-read of 0 (IRQ)

[Vincent Jardin]

SMBus 3.1 6.5.7 allows a Block Read byte count of 0, but the
interrupt-driven block-read state machine rejects it as -EPROTO. Worse,
it returns without a NACK+STOP: the next receive cycle has already
started, so the target keeps holding SDA and the bus stays stuck until a
power cycle of this i2c controller.

Accept count=0: NACK the in-flight dummy byte (TXAK) and set msg->len to
2 so i2c_imx_isr_read_continue() emits STOP via its normal last-byte
path. The dummy byte is discarded; block-read callers only consume
buf[0..count-1].

Reading I2DR has likewise already armed the next byte on the
count > I2C_SMBUS_BLOCK_MAX error path, so NACK it (TXAK) before aborting
with -EPROTO; otherwise the failing transfer's STOP cannot complete and
the bus stays held.

The atomic path regressed earlier (v3.16) and is fixed separately; this
patch covers only the v6.13 state-machine rework.

Fixes: 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma mode")
Cc: <stable@vger.kernel.org> # v6.13+
Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 drivers/i2c/busses/i2c-imx.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 14107e1ad413..8db8d2e10f5c 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1061,11 +1061,28 @@ static inline enum imx_i2c_state i2c_imx_isr_read_continue(struct imx_i2c_struct
 static inline void i2c_imx_isr_read_block_data_len(struct imx_i2c_struct *i2c_imx)
 {
 	u8 len = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+	unsigned int temp;
 
 	if (len == 0 || len > I2C_SMBUS_BLOCK_MAX) {
+		/*
+		 * SMBus 3.1 6.5.7: support count byte of 0.
+		 * I2C_SMBUS_BLOCK_MAX case should not hold the SDA either.
+		 * So NACK it (TXAK) to not hold the bus.
+		 */
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+		temp |= I2CR_TXAK;
+		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+		if (len == 0) {
+			i2c_imx->msg->buf[i2c_imx->msg_buf_idx++] = 0;
+			i2c_imx->msg->len = 2;
+			return;
+		}
+
 		i2c_imx->isr_result = -EPROTO;
 		i2c_imx->state = IMX_I2C_STATE_FAILED;
 		wake_up(&i2c_imx->queue);
+		return;
 	}
 	i2c_imx->msg->len += len;
 	i2c_imx->msg->buf[i2c_imx->msg_buf_idx++] = len;

-- 
2.43.0

RE: [PATCH v2 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus

[Carlos Song (OSS)]

> -----Original Message-----
> From: Vincent Jardin <vjardin@free.fr>
> Sent: Tuesday, May 26, 2026 12:43 AM
> To: Oleksij Rempel <o.rempel@pengutronix.de>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Andi Shyti <andi.shyti@kernel.org>; Frank Li
> <frank.li@nxp.com>; Sascha Hauer <s.hauer@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Wolfram Sang <wsa@kernel.org>; Kaushal Butala
> <kaushalkernelmailinglist@gmail.com>; Shawn Guo
> <shawn.guo@freescale.com>; Stefan Eichenberger
> <stefan.eichenberger@toradex.com>
> Cc: linux-i2c@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Vincent
> Jardin <vjardin@free.fr>; stable@vger.kernel.org
> Subject: [PATCH v2 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus
> 
> [You don't often get email from vjardin@free.fr. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
> 
> i2c-imx rejects a SMBus Block Read byte count of 0 (valid per SMBus 3.1
> 6.5.7) and it returns without a NACK+STOP, leaving the target holding SDA so the
> bus is stuck until a power cycle occur.
> 
> The same bug is occuring with two independently introduced spots, so the fix is
> two patches with their respective Fixes: tags and backport ranges:
> 
>   1/2  atomic/polling path       Fixes: 8e8782c71595   v3.16+
>   2/2  IRQ-driven state machine  Fixes: 5f5c2d4579ca   v6.13+
> 


Hi Vincent,

Thanks for working on this fix, this looks good to me.

SMBus block reads with a length of 0 seem quite uncommon in practice.
Was this triggered by a specific device behavior, or mainly found
during boundary / compliance testing?

Regarding the handling of len == 0,
I see that the patch sets:

    msg->buf[0] = 0;
    msg->len = 2;

It relies on the last-byte STOP handling together with TXAK. It will help I2C-IMX generate NACK + STOP and
release the bus, right? len = 0 is a legal behavior, So it go into a successful path.
But len > I2C_SMBUS_BLOCK_MAX is abnormal behavior. So it go into a fail path.
Do I understand it right?

Also, if possible could you briefly describe how you validated this change
(e.g. test setup or steps, with and without the fix)?

Thanks again for the fix.

Carlos

> ---
> Changes in v2:
> - Handle when count > I2C_SMBUS_BLOCK_MAX the same way as count == 0
>   Reported by the Sashiko AI review on v1.
> 
> ---
> Vincent Jardin (2):
>       i2c: imx: fix locked bus on SMBus block-read of 0 (atomic)
>       i2c: imx: fix locked bus on SMBus block-read of 0 (IRQ)
> 
>  drivers/i2c/busses/i2c-imx.c | 36 +++++++++++++++++++++++++++++++++---
>  1 file changed, 33 insertions(+), 3 deletions(-)
> ---
> base-commit: 6916d5703ddf9a38f1f6c2cc793381a24ee914c6
> change-id: 20260525-for-upstream-i2c-lx2160-fix-v1-0cba0a0093e5
> 
> Best regards,
> --
> Vincent Jardin <vjardin@free.fr>
> 

Re: [PATCH v2 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus

[Vincent Jardin]

Hi Carlos,

> Thanks for working on this fix, this looks good to me.

thanks for checking it. It took some times to isolate this issue and
then find a fix.

> SMBus block reads with a length of 0 seem quite uncommon in practice.
> Was this triggered by a specific device behavior, or mainly found
> during boundary / compliance testing?

It is trigger by the usage of a mpq8785 on the i2c bus: when the kernel attaches
on it using its pmsbus/hwmon framework, then the i2c bus get locked on lx2160 !

> Regarding the handling of len == 0,
> I see that the patch sets:
> 
>     msg->buf[0] = 0;
>     msg->len = 2;
> 
> It relies on the last-byte STOP handling together with TXAK. It will help I2C-IMX generate NACK + STOP and
> release the bus, right?

Yes, exactly. Reading I2DR for the length byte has already armed the
next byte, so we set TXAK to NACK it and extend msg->len to 2.
Next then i2c_imx_isr_read_continue() at msg_buf_idx == msg->len - 1,
ie the normal last-byte path, which clears MSTA to emit STOP. So NACK + STOP,
and THEN the bus is released. I do not see any other means to handle it.

> len = 0 is a legal behavior, So it go into a successful path.

Yes. count == 0 is legal (SMBus 3.1 6.5.7), so the transfer reaches
STATE_DONE and returns success.

> But len > I2C_SMBUS_BLOCK_MAX is abnormal behavior. So it go into a fail path.

Correct, and it is a protocol error, so it needs to end up with a -EPROTO while
a count of 0 is an ok case.

> Do I understand it right?

yes. I do not see any other means to handle it.

> Also, if possible could you briefly describe how you validated this change
> (e.g. test setup or steps, with and without the fix)?

On a lx2160a board, on its i2c, bind a mpq8785, and enable the Kernel pmbus/hwmon
framework, then the i2c bus becomes un-useable. Using a scope, we can confirm that
the lx21260a i2c cannot recover.

Best regards,
  Vincent 

RE: [PATCH v2 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus

[Carlos Song (OSS)]

> -----Original Message-----
> From: Vincent Jardin <vjardin@free.fr>
> Sent: Tuesday, May 26, 2026 4:12 PM
> To: Carlos Song (OSS) <carlos.song@oss.nxp.com>
> Cc: Oleksij Rempel <o.rempel@pengutronix.de>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Andi Shyti <andi.shyti@kernel.org>; Frank Li
> <frank.li@nxp.com>; Sascha Hauer <s.hauer@pengutronix.de>; Fabio
> Estevam <festevam@gmail.com>; Wolfram Sang <wsa@kernel.org>; Kaushal
> Butala <kaushalkernelmailinglist@gmail.com>; Shawn Guo
> <shawn.guo@freescale.com>; Stefan Eichenberger
> <stefan.eichenberger@toradex.com>; linux-i2c@vger.kernel.org;
> imx@lists.linux.dev; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH v2 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus
> 
> [You don't often get email from vjardin@free.fr. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
> 
> Hi Carlos,
> 
> > Thanks for working on this fix, this looks good to me.
> 
> thanks for checking it. It took some times to isolate this issue and then find a
> fix.
> 
> > SMBus block reads with a length of 0 seem quite uncommon in practice.
> > Was this triggered by a specific device behavior, or mainly found
> > during boundary / compliance testing?
> 
> It is trigger by the usage of a mpq8785 on the i2c bus: when the kernel
> attaches on it using its pmsbus/hwmon framework, then the i2c bus get
> locked on lx2160 !
> 
> > Regarding the handling of len == 0,
> > I see that the patch sets:
> >
> >     msg->buf[0] = 0;
> >     msg->len = 2;
> >
> > It relies on the last-byte STOP handling together with TXAK. It will
> > help I2C-IMX generate NACK + STOP and release the bus, right?
> 
> Yes, exactly. Reading I2DR for the length byte has already armed the next byte,
> so we set TXAK to NACK it and extend msg->len to 2.
> Next then i2c_imx_isr_read_continue() at msg_buf_idx == msg->len - 1, ie the
> normal last-byte path, which clears MSTA to emit STOP. So NACK + STOP, and
> THEN the bus is released. I do not see any other means to handle it.
> 
> > len = 0 is a legal behavior, So it go into a successful path.
> 
> Yes. count == 0 is legal (SMBus 3.1 6.5.7), so the transfer reaches
> STATE_DONE and returns success.
> 
> > But len > I2C_SMBUS_BLOCK_MAX is abnormal behavior. So it go into a fail
> path.
> 
> Correct, and it is a protocol error, so it needs to end up with a -EPROTO while
> a count of 0 is an ok case.
> 
> > Do I understand it right?
> 
> yes. I do not see any other means to handle it.
> 
> > Also, if possible could you briefly describe how you validated this
> > change (e.g. test setup or steps, with and without the fix)?
> 
> On a lx2160a board, on its i2c, bind a mpq8785, and enable the Kernel
> pmbus/hwmon framework, then the i2c bus becomes un-useable. Using a
> scope, we can confirm that the lx21260a i2c cannot recover.
> 
Hi, Vincent

Thank you very much!

Acked-by: Carlos Song <carlos.song@nxp.com>

> Best regards,
>   Vincent