Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus
@ 2026-07-13 18:11 Vincent Jardin
  2026-07-13 18:11 ` [PATCH v3 1/2] i2c: imx: fix locked bus on SMBus block-read of 0 (atomic) Vincent Jardin
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Vincent Jardin @ 2026-07-13 18:11 UTC (permalink / raw)
  To: Oleksij Rempel, Pengutronix Kernel Team, Andi Shyti, Frank Li,
	Sascha Hauer, Fabio Estevam, Wolfram Sang, Kaushal Butala,
	Shawn Guo, Stefan Eichenberger
  Cc: linux-i2c, imx, linux-arm-kernel, linux-kernel, Vincent Jardin,
	stable, Carlos Song, Stefan Eichenberger

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>



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 1/2] i2c: imx: fix locked bus on SMBus block-read of 0 (atomic)
  2026-07-13 18:11 [PATCH v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus Vincent Jardin
@ 2026-07-13 18:11 ` Vincent Jardin
  2026-07-13 18:12 ` [PATCH v3 2/2] i2c: imx: fix locked bus on SMBus block-read of 0 (IRQ) Vincent Jardin
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Vincent Jardin @ 2026-07-13 18:11 UTC (permalink / raw)
  To: Oleksij Rempel, Pengutronix Kernel Team, Andi Shyti, Frank Li,
	Sascha Hauer, Fabio Estevam, Wolfram Sang, Kaushal Butala,
	Shawn Guo, Stefan Eichenberger
  Cc: linux-i2c, imx, linux-arm-kernel, linux-kernel, Vincent Jardin,
	stable, Carlos Song, Stefan Eichenberger

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



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 2/2] i2c: imx: fix locked bus on SMBus block-read of 0 (IRQ)
  2026-07-13 18:11 [PATCH v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus Vincent Jardin
  2026-07-13 18:11 ` [PATCH v3 1/2] i2c: imx: fix locked bus on SMBus block-read of 0 (atomic) Vincent Jardin
@ 2026-07-13 18:12 ` Vincent Jardin
  2026-07-14 15:00 ` [PATCH v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus Andi Shyti
  2026-07-14 15:45 ` Wolfram Sang
  3 siblings, 0 replies; 5+ messages in thread
From: Vincent Jardin @ 2026-07-13 18:12 UTC (permalink / raw)
  To: Oleksij Rempel, Pengutronix Kernel Team, Andi Shyti, Frank Li,
	Sascha Hauer, Fabio Estevam, Wolfram Sang, Kaushal Butala,
	Shawn Guo, Stefan Eichenberger
  Cc: linux-i2c, imx, linux-arm-kernel, linux-kernel, Vincent Jardin,
	stable, Carlos Song, Stefan Eichenberger

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



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus
  2026-07-13 18:11 [PATCH v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus Vincent Jardin
  2026-07-13 18:11 ` [PATCH v3 1/2] i2c: imx: fix locked bus on SMBus block-read of 0 (atomic) Vincent Jardin
  2026-07-13 18:12 ` [PATCH v3 2/2] i2c: imx: fix locked bus on SMBus block-read of 0 (IRQ) Vincent Jardin
@ 2026-07-14 15:00 ` Andi Shyti
  2026-07-14 15:45 ` Wolfram Sang
  3 siblings, 0 replies; 5+ messages in thread
From: Andi Shyti @ 2026-07-14 15:00 UTC (permalink / raw)
  To: Vincent Jardin
  Cc: Oleksij Rempel, Pengutronix Kernel Team, Frank Li, Sascha Hauer,
	Fabio Estevam, Wolfram Sang, Kaushal Butala, Shawn Guo,
	Stefan Eichenberger, linux-i2c, imx, linux-arm-kernel,
	linux-kernel, stable, Carlos Song, Stefan Eichenberger

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus
  2026-07-13 18:11 [PATCH v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus Vincent Jardin
                   ` (2 preceding siblings ...)
  2026-07-14 15:00 ` [PATCH v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus Andi Shyti
@ 2026-07-14 15:45 ` Wolfram Sang
  3 siblings, 0 replies; 5+ messages in thread
From: Wolfram Sang @ 2026-07-14 15:45 UTC (permalink / raw)
  To: Vincent Jardin
  Cc: Oleksij Rempel, Pengutronix Kernel Team, Andi Shyti, Frank Li,
	Sascha Hauer, Fabio Estevam, Wolfram Sang, Kaushal Butala,
	Shawn Guo, Stefan Eichenberger, linux-i2c, imx, linux-arm-kernel,
	linux-kernel, stable, Carlos Song, Stefan Eichenberger

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.



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-14 16:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-13 18:11 [PATCH v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus Vincent Jardin
2026-07-13 18:11 ` [PATCH v3 1/2] i2c: imx: fix locked bus on SMBus block-read of 0 (atomic) Vincent Jardin
2026-07-13 18:12 ` [PATCH v3 2/2] i2c: imx: fix locked bus on SMBus block-read of 0 (IRQ) Vincent Jardin
2026-07-14 15:00 ` [PATCH v3 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus Andi Shyti
2026-07-14 15:45 ` Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox