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

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

[Vincent Jardin]

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

It was triggered by an MPQ8785 PMBus regulator on a LX2160A i2c
bus: when the kernel binds it using the pmbus/hwmon framework, the bus
locks up and it does never recovers. It was confirmed with a scope, with
and without the fix.

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 v3:
- no functional change; collected the review tags received on v2:
  Acked-by Oleksij Rempel, Acked-by Carlos Song, Reviewed-by Stefan
  Eichenberger (both patches)
- cover letter: add the real-world trigger (MPQ8785 PMBus regulator
  on the LX2160A) and how the fix was validated, asked by Carlos Song
- resend as a new thread, per Andi Shyti's request
- Link to v2: https://lore.kernel.org/r/20260525-for-upstream-i2c-lx2160-fix-v1-v2-0-26a3cc8cd055@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: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260525-for-upstream-i2c-lx2160-fix-v1-0cba0a0093e5

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

[PATCH v3 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+
Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
Acked-by: Carlos Song <carlos.song@nxp.com>
Reviewed-by: Stefan Eichenberger <eichest@gmail.com>
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 28313d0fad37..cfd1e63359e7 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 v3 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+
Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
Acked-by: Carlos Song <carlos.song@nxp.com>
Reviewed-by: Stefan Eichenberger <eichest@gmail.com>
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 cfd1e63359e7..d5e6e2eca3b3 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 v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus

[Andi Shyti]

Hi Vincent,

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

merged to i2c/i2c-fixes.

Thanks,
Andi

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

[Wolfram Sang]

On Mon, Jul 13, 2026 at 08:11:58PM +0200, Vincent Jardin wrote:
> i2c-imx rejects an SMBus Block Read byte count of 0 (valid per SMBus 3.1
> 6.5.7) as -EPROTO and returns without emitting a NACK + STOP, leaving the
> target holding SDA so the bus stays stuck until a power cycle.

Bigger picture: Linux does not support SMBus3 which also allows byte
counts of up to 255. I started sketching support for all that but could
never implement it.

That being said, despite no SMBus3 support, it should not hang the bus
like here.

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

[Andi Shyti]

On Tue, Jul 14, 2026 at 05:45:31PM +0200, Wolfram Sang wrote:
> On Mon, Jul 13, 2026 at 08:11:58PM +0200, Vincent Jardin wrote:
> > i2c-imx rejects an SMBus Block Read byte count of 0 (valid per SMBus 3.1
> > 6.5.7) as -EPROTO and returns without emitting a NACK + STOP, leaving the
> > target holding SDA so the bus stays stuck until a power cycle.
> 
> Bigger picture: Linux does not support SMBus3 which also allows byte
> counts of up to 255. I started sketching support for all that but could
> never implement it.

Yeah... I have claimed many times to have patches that add
support to smbus3. I also promised many times that I should
update and send them over :-)

Andi

> That being said, despite no SMBus3 support, it should not hang the bus
> like here.

Re: [PATCH v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus [stable backport question/offer]

[Dominique Martinet]

Vincent Jardin wrote on Mon, Jul 13, 2026 at 08:11:58PM +0200:
> i2c-imx rejects an SMBus Block Read byte count of 0 (valid per SMBus 3.1
> 6.5.7) as -EPROTO and returns without emitting a NACK + STOP, leaving the
> target holding SDA so the bus stays stuck until a power cycle.
> 
> It was triggered by an MPQ8785 PMBus regulator on a LX2160A i2c
> bus: when the kernel binds it using the pmbus/hwmon framework, the bus
> locks up and it does never recovers. It was confirmed with a scope, with
> and without the fix.
> 
> 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+

Silly question (half for stable people, half for i2c-imx maintainers),
but the backport for commit cb2fc3785769 ("i2c: imx: fix locked bus on
SMBus block-read of 0 (atomic)") to stable brought in b460b15b3cc2
("i2c: imx: separate atomic, dma and non-dma use case") (also 6.13+) as
a dep (all the way back to 5.10!);
with that commit in, 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non
dma mode") applies almost cleanly on 6.6/6.12[1]... So should we grab
more easy fixes?
In particular, I doubt I'll ever hit this SMBus 3.1 bug, but
5f5c2d4579ca also was a real fix[2], so would it make sense to jump in
and get both 5f5c2d4579ca and 07fd9385f0d8 ("i2c: imx: fix locked bus on
SMBus block-read of 0 (IRQ)") for 6.6/6.12?

[1] just a trivial context conflict in the struct there, but it starts
being more iffy on 6.1 and earlier kernels
[2] ... We actually ran into that bug on 5.10, our kludgy backport being
the reason I noticed during today's 5.10.266-rc1 testing...


Honestly, I wouldn't have considered backporting either as b460b15b3cc2
("i2c: imx: separate atomic, dma and non-dma use case") looks too big to
backport to me, so I definitely wouldn't have done it before, but that
ship has sailed (it's in the 5.10 -rc right now, but it's been merged a
couple of weeks ago in higher versions stables), so at this point I
don't think it's worth reverting either and we might as well keep
falling forward...



tl;dr: If maintainers agree, I can send these two for a future 6.6/6.12:
5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma mode")
07fd9385f0d8 ("i2c: imx: fix locked bus on SMBus block-read of 0 (IRQ)")


Thanks,
-- 
Dominique Martinet | Asmadeus

Re: [PATCH v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus [stable backport question/offer]

[Sasha Levin]

Dominique Martinet wrote on Fri, Aug 21, 2026 at 06:07:35PM +0900:
> tl;dr: If maintainers agree, I can send these two for a future 6.6/6.12:
> 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma mode")
> 07fd9385f0d8 ("i2c: imx: fix locked bus on SMBus block-read of 0 (IRQ)")

Thanks for the offer, but let's not take these on 6.6/6.12.

5f5c2d4579ca splits the read path, and the split leaves the block_err
handling in the atomic variant only, while the live non-dma path ends up
back in the unfixed handler. On 6.6/6.12 the block-read-of-0 fix that
shipped in 6.12.101 and its 6.6 sibling landed in the shared
i2c_imx_read(), which both paths still go through - so pulling the rework
in would actually re-open the bus lockup those trees are already
protected against.

07fd9385f0d8 on its own buys nothing there either: it patches
i2c_imx_isr_read_block_data_len(), which only exists once 5f5c2d4579ca is
applied.

Your premise is right that b460b15b3cc2 is now everywhere, but
5f5c2d4579ca also carries five Fixes: follow-ups, four of them tagged
Cc: stable # v6.13+, none of which are on 6.6/6.12 - so doing it properly
means backporting a six-commit ISR state-machine rework into two frozen
trees, and upstream was still fixing that rework in February.

-- 
Thanks,
Sasha

Re: [PATCH v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus [stable backport question/offer]

[Dominique Martinet]

Sasha Levin wrote on Tue, Aug 25, 2026 at 07:49:30AM -0400:
> Dominique Martinet wrote on Fri, Aug 21, 2026 at 06:07:35PM +0900:
> > tl;dr: If maintainers agree, I can send these two for a future 6.6/6.12:
> > 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma mode")
> > 07fd9385f0d8 ("i2c: imx: fix locked bus on SMBus block-read of 0 (IRQ)")
> 
> Thanks for the offer, but let's not take these on 6.6/6.12.

Ok, thanks for taking the time to check and reply!

> 5f5c2d4579ca splits the read path, and the split leaves the block_err
> handling in the atomic variant only, while the live non-dma path ends up
> back in the unfixed handler. On 6.6/6.12 the block-read-of-0 fix that
> shipped in 6.12.101 and its 6.6 sibling landed in the shared
> i2c_imx_read(), which both paths still go through - so pulling the rework
> in would actually re-open the bus lockup those trees are already
> protected against.

Yes, a backport would only make sense with both patches...

> Your premise is right that b460b15b3cc2 is now everywhere, but
> 5f5c2d4579ca also carries five Fixes: follow-ups, four of them tagged

... but now that you've pointed out the others I agree it's more trouble
than it's worth (even if they all apply cleanly after trivially fixing
5f5c2d4579ca's backport), so I'm fine with this
(And that can be reconsidered if more people report timeouts fixed by
5f5c2d4579ca...)


Thanks,
-- 
Dominique Martinet | Asmadeus