public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length
@ 2025-12-29  8:16 LI Qingwu
  2026-01-07 20:13 ` Andi Shyti
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: LI Qingwu @ 2025-12-29  8:16 UTC (permalink / raw)
  To: o.rempel, kernel, andi.shyti, shawnguo, s.hauer, festevam,
	linux-i2c, imx, linux-arm-kernel, linux-kernel
  Cc: bsp-development.geo, LI Qingwu

SMBus block read transfers encode the payload length in the first data
byte. When this first byte is zero, there is no payload and the
transaction should terminate immediately.

On i.MX, if the first byte of an SMBus block read is zero, the driver
unconditionally overwrites the state with IMX_I2C_STATE_READ_CONTINUE.
This causes the state machine to enter an endless read loop, eventually
overrunning internal buffers and leading to a crash.

At the same time, the controller remains in master receive mode and
never generates a proper STOP condition, leaving the I2C bus permanently
busy and preventing any further transfers on the bus.

Fix this by handling the zero-length case explicitly: when the first
byte is zero, ensure that a clean STOP is generated. In this situation
the controller is in master receive mode, so it must be switched to
master transmit mode before stopping. This is done by draining the
pending received byte from I2DR, setting I2CR_MTX to enter transmit
mode, waiting briefly for the mode change, and then proceeding with the
normal STOP sequence.

This change has been tested on i.MX 8M Plus platform.

Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
---
 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 dcce882f3eba..f40deecf0f66 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -735,6 +735,16 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx, bool atomic)
 		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
 		if (!(temp & I2CR_MSTA))
 			i2c_imx->stopped = 1;
+		if ((temp & I2CR_MSTA) && !(temp & I2CR_MTX)) {
+			(void)imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+			temp |= I2CR_MTX;
+			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+			if (atomic)
+				udelay(25);
+			else
+				usleep_range(25, 50);
+			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+		}
 		temp &= ~(I2CR_MSTA | I2CR_MTX);
 		if (i2c_imx->dma)
 			temp &= ~I2CR_DMAEN;
@@ -1103,7 +1113,8 @@ static irqreturn_t i2c_imx_master_isr(struct imx_i2c_struct *i2c_imx, unsigned i
 
 	case IMX_I2C_STATE_READ_BLOCK_DATA_LEN:
 		i2c_imx_isr_read_block_data_len(i2c_imx);
-		i2c_imx->state = IMX_I2C_STATE_READ_CONTINUE;
+		if (i2c_imx->state == IMX_I2C_STATE_READ_BLOCK_DATA_LEN)
+			i2c_imx->state = IMX_I2C_STATE_READ_CONTINUE;
 		break;
 
 	case IMX_I2C_STATE_WRITE:
-- 
2.43.0



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

* Re: [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length
  2025-12-29  8:16 [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length LI Qingwu
@ 2026-01-07 20:13 ` Andi Shyti
  2026-01-08  7:59   ` Oleksij Rempel
  2026-01-08  8:37   ` LI Qingwu
  2026-01-09  3:02 ` Carlos Song
  2026-01-09 13:53 ` Oleksij Rempel
  2 siblings, 2 replies; 7+ messages in thread
From: Andi Shyti @ 2026-01-07 20:13 UTC (permalink / raw)
  To: LI Qingwu
  Cc: o.rempel, kernel, shawnguo, s.hauer, festevam, linux-i2c, imx,
	linux-arm-kernel, linux-kernel, bsp-development.geo

Hi,

Can anyone from Pengutronix give this patch a look and possibly a
try? Oleksij?

On Mon, Dec 29, 2025 at 08:16:29AM +0000, LI Qingwu wrote:
> SMBus block read transfers encode the payload length in the first data
> byte. When this first byte is zero, there is no payload and the
> transaction should terminate immediately.
> 
> On i.MX, if the first byte of an SMBus block read is zero, the driver
> unconditionally overwrites the state with IMX_I2C_STATE_READ_CONTINUE.
> This causes the state machine to enter an endless read loop, eventually
> overrunning internal buffers and leading to a crash.
> 
> At the same time, the controller remains in master receive mode and
> never generates a proper STOP condition, leaving the I2C bus permanently
> busy and preventing any further transfers on the bus.
> 
> Fix this by handling the zero-length case explicitly: when the first
> byte is zero, ensure that a clean STOP is generated. In this situation
> the controller is in master receive mode, so it must be switched to
> master transmit mode before stopping. This is done by draining the
> pending received byte from I2DR, setting I2CR_MTX to enter transmit
> mode, waiting briefly for the mode change, and then proceeding with the
> normal STOP sequence.
> 
> This change has been tested on i.MX 8M Plus platform.
> 
> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>

Is this a fix?

> ---
>  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 dcce882f3eba..f40deecf0f66 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -735,6 +735,16 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx, bool atomic)
>  		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>  		if (!(temp & I2CR_MSTA))
>  			i2c_imx->stopped = 1;
> +		if ((temp & I2CR_MSTA) && !(temp & I2CR_MTX)) {
> +			(void)imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);

why do we need a cast here?

> +			temp |= I2CR_MTX;
> +			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +			if (atomic)
> +				udelay(25);

where is this 25 coming from?

> +			else
> +				usleep_range(25, 50);
> +			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);

Thanks,
Andi


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

* Re: [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length
  2026-01-07 20:13 ` Andi Shyti
@ 2026-01-08  7:59   ` Oleksij Rempel
  2026-01-08  8:37   ` LI Qingwu
  1 sibling, 0 replies; 7+ messages in thread
From: Oleksij Rempel @ 2026-01-08  7:59 UTC (permalink / raw)
  To: Andi Shyti
  Cc: LI Qingwu, kernel, shawnguo, s.hauer, festevam, linux-i2c, imx,
	linux-arm-kernel, linux-kernel, bsp-development.geo

Hi Andi,

On Wed, Jan 07, 2026 at 09:13:24PM +0100, Andi Shyti wrote:
> Hi,
> 
> Can anyone from Pengutronix give this patch a look and possibly a
> try? Oleksij?

I'm just back from my vacation. I'll take a look tomorrow.

Thank you!
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


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

* RE: [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length
  2026-01-07 20:13 ` Andi Shyti
  2026-01-08  7:59   ` Oleksij Rempel
@ 2026-01-08  8:37   ` LI Qingwu
  2026-01-14 15:52     ` Andi Shyti
  1 sibling, 1 reply; 7+ messages in thread
From: LI Qingwu @ 2026-01-08  8:37 UTC (permalink / raw)
  To: Andi Shyti
  Cc: o.rempel@pengutronix.de, kernel@pengutronix.de,
	shawnguo@kernel.org, s.hauer@pengutronix.de, festevam@gmail.com,
	linux-i2c@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, GEO-CHHER-bsp-development



> -----Original Message-----
> From: Andi Shyti <andi.shyti@kernel.org>
> Sent: Thursday, January 8, 2026 4:13 AM
> To: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> Cc: o.rempel@pengutronix.de; kernel@pengutronix.de; shawnguo@kernel.org;
> s.hauer@pengutronix.de; festevam@gmail.com; linux-i2c@vger.kernel.org;
> imx@lists.linux.dev; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; GEO-CHHER-bsp-development
> <bsp-development.geo@leica-geosystems.com>
> Subject: Re: [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length
> 
> [You don't often get email from andi.shyti@kernel.org. Learn why this is
> important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> This email is not from Hexagon's Office 365 instance. Please be careful while
> clicking links, opening attachments, or replying to this email.
> 
> 
> Hi,
> 
> Can anyone from Pengutronix give this patch a look and possibly a try? Oleksij?
> 
> On Mon, Dec 29, 2025 at 08:16:29AM +0000, LI Qingwu wrote:
> > SMBus block read transfers encode the payload length in the first data
> > byte. When this first byte is zero, there is no payload and the
> > transaction should terminate immediately.
> >
> > On i.MX, if the first byte of an SMBus block read is zero, the driver
> > unconditionally overwrites the state with IMX_I2C_STATE_READ_CONTINUE.
> > This causes the state machine to enter an endless read loop,
> > eventually overrunning internal buffers and leading to a crash.
> >
> > At the same time, the controller remains in master receive mode and
> > never generates a proper STOP condition, leaving the I2C bus
> > permanently busy and preventing any further transfers on the bus.
> >
> > Fix this by handling the zero-length case explicitly: when the first
> > byte is zero, ensure that a clean STOP is generated. In this situation
> > the controller is in master receive mode, so it must be switched to
> > master transmit mode before stopping. This is done by draining the
> > pending received byte from I2DR, setting I2CR_MTX to enter transmit
> > mode, waiting briefly for the mode change, and then proceeding with
> > the normal STOP sequence.
> >
> > This change has been tested on i.MX 8M Plus platform.
> >
> > Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> 
> Is this a fix?

Yes, this is a fix. Without this patch, zero-length SMBus block reads cause a system crash 
and permanent bus lockup on i.MX 8M Plus. The fix ensures proper STOP generation and 
prevents buffer overruns.

> > ---
> >  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 dcce882f3eba..f40deecf0f66 100644
> > --- a/drivers/i2c/busses/i2c-imx.c
> > +++ b/drivers/i2c/busses/i2c-imx.c
> > @@ -735,6 +735,16 @@ static void i2c_imx_stop(struct imx_i2c_struct
> *i2c_imx, bool atomic)
> >               temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> >               if (!(temp & I2CR_MSTA))
> >                       i2c_imx->stopped = 1;
> > +             if ((temp & I2CR_MSTA) && !(temp & I2CR_MTX)) {
> > +                     (void)imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> 
> why do we need a cast here?

The void cast explicitly marks this as a dummy read. We're reading I2DR solely to drain the
receive buffer (a required side effect before mode switching), not because we need the value.
This makes the intent clear to both the compiler and code reviewers.

> 
> > +                     temp |= I2CR_MTX;
> > +                     imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> > +                     if (atomic)
> > +                             udelay(25);
> 
> where is this 25 coming from?

It's a conservative value derived from practical testing. at standard mode (100kHz),
this equals ~2.5 I2C clock periods; at fast mode (400kHz), ~10 periods

> 
> > +                     else
> > +                             usleep_range(25, 50);
> > +                     temp = imx_i2c_read_reg(i2c_imx,
> IMX_I2C_I2CR);
> 
> Thanks,
> Andi


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

* [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length
  2025-12-29  8:16 [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length LI Qingwu
  2026-01-07 20:13 ` Andi Shyti
@ 2026-01-09  3:02 ` Carlos Song
  2026-01-09 13:53 ` Oleksij Rempel
  2 siblings, 0 replies; 7+ messages in thread
From: Carlos Song @ 2026-01-09  3:02 UTC (permalink / raw)
  To: LI Qingwu, o.rempel@pengutronix.de, kernel@pengutronix.de,
	andi.shyti@kernel.org, shawnguo@kernel.org,
	s.hauer@pengutronix.de, festevam@gmail.com,
	linux-i2c@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
  Cc: bsp-development.geo@leica-geosystems.com



> -----Original Message-----
> From: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> Sent: Monday, December 29, 2025 4:16 PM
> To: o.rempel@pengutronix.de; kernel@pengutronix.de; andi.shyti@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com;
> linux-i2c@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Cc: bsp-development.geo@leica-geosystems.com; LI Qingwu
> <Qing-wu.Li@leica-geosystems.com.cn>
> Subject: [EXT] [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> SMBus block read transfers encode the payload length in the first data byte.
> When this first byte is zero, there is no payload and the transaction should
> terminate immediately.
> 
> On i.MX, if the first byte of an SMBus block read is zero, the driver
> unconditionally overwrites the state with IMX_I2C_STATE_READ_CONTINUE.
> This causes the state machine to enter an endless read loop, eventually
> overrunning internal buffers and leading to a crash.
> 
> At the same time, the controller remains in master receive mode and never
> generates a proper STOP condition, leaving the I2C bus permanently busy and
> preventing any further transfers on the bus.
> 
> Fix this by handling the zero-length case explicitly: when the first byte is zero,
> ensure that a clean STOP is generated. In this situation the controller is in
> master receive mode, so it must be switched to master transmit mode before
> stopping. This is done by draining the pending received byte from I2DR, setting
> I2CR_MTX to enter transmit mode, waiting briefly for the mode change, and
> then proceeding with the normal STOP sequence.
> 
> This change has been tested on i.MX 8M Plus platform.
> 
> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> ---
>  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
> dcce882f3eba..f40deecf0f66 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -735,6 +735,16 @@ static void i2c_imx_stop(struct imx_i2c_struct
> *i2c_imx, bool atomic)
>                 temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>                 if (!(temp & I2CR_MSTA))
>                         i2c_imx->stopped = 1;
> +               if ((temp & I2CR_MSTA) && !(temp & I2CR_MTX)) {
> +                       (void)imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> +                       temp |= I2CR_MTX;
> +                       imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +                       if (atomic)
> +                               udelay(25);
> +                       else
> +                               usleep_range(25, 50);
> +                       temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +               }
Hi,

for fix a smbus block read stop, you added these codes to normal stop logic. Could you help check if it will effect stop in other path like i2c DMA/CPU read/write stop logic except for smbus block read?

And do you have mind to move this logic to smbus block read error path? This may be a minimal fix?

Carlos
>                 temp &= ~(I2CR_MSTA | I2CR_MTX);
>                 if (i2c_imx->dma)
>                         temp &= ~I2CR_DMAEN; @@ -1103,7 +1113,8
> @@ static irqreturn_t i2c_imx_master_isr(struct imx_i2c_struct *i2c_imx,
> unsigned i
> 
>         case IMX_I2C_STATE_READ_BLOCK_DATA_LEN:
>                 i2c_imx_isr_read_block_data_len(i2c_imx);
> -               i2c_imx->state = IMX_I2C_STATE_READ_CONTINUE;
> +               if (i2c_imx->state ==
> IMX_I2C_STATE_READ_BLOCK_DATA_LEN)
> +                       i2c_imx->state =
> IMX_I2C_STATE_READ_CONTINUE;
>                 break;
> 
>         case IMX_I2C_STATE_WRITE:
> --
> 2.43.0
> 



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

* Re: [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length
  2025-12-29  8:16 [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length LI Qingwu
  2026-01-07 20:13 ` Andi Shyti
  2026-01-09  3:02 ` Carlos Song
@ 2026-01-09 13:53 ` Oleksij Rempel
  2 siblings, 0 replies; 7+ messages in thread
From: Oleksij Rempel @ 2026-01-09 13:53 UTC (permalink / raw)
  To: LI Qingwu
  Cc: kernel, andi.shyti, shawnguo, s.hauer, festevam, linux-i2c, imx,
	linux-arm-kernel, linux-kernel, bsp-development.geo

Hi,

On Mon, Dec 29, 2025 at 08:16:29AM +0000, LI Qingwu wrote:
> SMBus block read transfers encode the payload length in the first data
> byte. When this first byte is zero, there is no payload and the
> transaction should terminate immediately.
> 
> On i.MX, if the first byte of an SMBus block read is zero, the driver
> unconditionally overwrites the state with IMX_I2C_STATE_READ_CONTINUE.
> This causes the state machine to enter an endless read loop, eventually
> overrunning internal buffers and leading to a crash.
> 
> At the same time, the controller remains in master receive mode and
> never generates a proper STOP condition, leaving the I2C bus permanently
> busy and preventing any further transfers on the bus.
> 
> Fix this by handling the zero-length case explicitly: when the first
> byte is zero, ensure that a clean STOP is generated. In this situation
> the controller is in master receive mode, so it must be switched to
> master transmit mode before stopping. This is done by draining the
> pending received byte from I2DR, setting I2CR_MTX to enter transmit
> mode, waiting briefly for the mode change, and then proceeding with the
> normal STOP sequence.
> 
> This change has been tested on i.MX 8M Plus platform.
> 
> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>

Sorry for delay and thank you for your work.

> ---
>  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 dcce882f3eba..f40deecf0f66 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -735,6 +735,16 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx, bool atomic)
>  		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>  		if (!(temp & I2CR_MSTA))
>  			i2c_imx->stopped = 1;

I would love to have more comments, so I'll add some. Otherwise it is hard
to recall everything needed with my cold cache :)

        /*
         * Condition: We are in Master Mode (MSTA=1) AND Receive Mode (MTX=0).
         *
         * Ref: IMX8MPRM Rev. 1, 06/2021: 17.1.6.3 I2Cx_I2CR:
         * - Bit 5 (MSTA): 1 = Master Mode.
         * - Bit 4 (MTX):  0 = Receive.
         */

> +		if ((temp & I2CR_MSTA) && !(temp & I2CR_MTX)) {

            /*
             * Dummy read of I2C Data Register (I2DR).
             *
             * Ref: IMX8MPRM Rev. 1, 06/2021: 17.1.6.5 I2C Data I/O Register (I2Cx_I2DR):
             * "Reading the data register... initiates the next byte to be received."
             *
             * Ref: IMX8MPRM Rev. 1, 06/2021: 17.1.6.4 I2C Status Register (I2Cx_I2SR) -> ICF (Bit 7):
             * "The data transferring bit (ICF) is cleared... by reading from I2C_I2DR in Receive mode."
             *
             * This dummy read is essential to clear the 'Transfer Complete' (ICF)
             * flag and release the SCL line if the hardware
             * was stretching the clock waiting for a read.
             */


> +			(void)imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);

            /*
             * Force the controller into Master Transmit Mode (MTX=1).
             *
             * Ref: IMX8MPRM Rev. 1, 06/2021: 17.1.6.3 I2Cx_I2CR -> MTX (Bit 4):
             * "1 = Transmit"
             *
             * We cannot safely STOP while waiting for data (RX). By switching to TX,
             * the Master asserts control over SDA to generate the STOP condition
             * without the ambiguity of an expected incoming byte.
             */
> +			temp |= I2CR_MTX;
> +			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);

            /*
             * Wait for the mode switch to settle.
             *
             * Ref: IMX8MPRM Rev. 1, 06/2021: 17.1.6.4 Note on Timeout:
             * "The minimum timeout... is 25 us".
             *
             * The state machine needs time to latch the new mode (TX) before
             * we immediately command it to STOP. 25us is the documented safe lower bound
             * for I2C bus event processing at 400kHz.
             * T_min = 10/F_SCL
             * T_min = 10 / 400000 Hz = 0,00002 Sec
             */

             I would recommend here to calculate delay based on current clock,
             probably i2c_imx->cur_clk is the right variable.
             May be i2c_imx->disable_delay should be used? Cirrently it is used only
             for imx1.

             I'm still not sure about impact of this delay withing this context.

> +			if (atomic)
> +				udelay(25);
> +			else
> +				usleep_range(25, 50);

            /*
             * Read back the control register to ensure the write persisted and
             * to have the freshest state for the final STOP command?
             */

> +			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +		}
>  		temp &= ~(I2CR_MSTA | I2CR_MTX);
>  		if (i2c_imx->dma)
>  			temp &= ~I2CR_DMAEN;
> @@ -1103,7 +1113,8 @@ static irqreturn_t i2c_imx_master_isr(struct imx_i2c_struct *i2c_imx, unsigned i
>  
>  	case IMX_I2C_STATE_READ_BLOCK_DATA_LEN:
>  		i2c_imx_isr_read_block_data_len(i2c_imx);
> -		i2c_imx->state = IMX_I2C_STATE_READ_CONTINUE;

I ques, this part can go as separate directly upstream to the stable.

> +		if (i2c_imx->state == IMX_I2C_STATE_READ_BLOCK_DATA_LEN)
> +			i2c_imx->state = IMX_I2C_STATE_READ_CONTINUE;
>  		break;
>  
>  	case IMX_I2C_STATE_WRITE:
> -- 
> 2.43.0
> 
> 

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


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

* Re: [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length
  2026-01-08  8:37   ` LI Qingwu
@ 2026-01-14 15:52     ` Andi Shyti
  0 siblings, 0 replies; 7+ messages in thread
From: Andi Shyti @ 2026-01-14 15:52 UTC (permalink / raw)
  To: LI Qingwu
  Cc: o.rempel@pengutronix.de, kernel@pengutronix.de,
	shawnguo@kernel.org, s.hauer@pengutronix.de, festevam@gmail.com,
	linux-i2c@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, GEO-CHHER-bsp-development

Hi,

...

> > > master transmit mode before stopping. This is done by draining the
> > > pending received byte from I2DR, setting I2CR_MTX to enter transmit
> > > mode, waiting briefly for the mode change, and then proceeding with
> > > the normal STOP sequence.
> > >
> > > This change has been tested on i.MX 8M Plus platform.
> > >
> > > Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> > 
> > Is this a fix?
> 
> Yes, this is a fix. Without this patch, zero-length SMBus block reads cause a system crash 
> and permanent bus lockup on i.MX 8M Plus. The fix ensures proper STOP generation and 
> prevents buffer overruns.

Then, please add the Fixes tag.

> > > ---
> > >  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 dcce882f3eba..f40deecf0f66 100644
> > > --- a/drivers/i2c/busses/i2c-imx.c
> > > +++ b/drivers/i2c/busses/i2c-imx.c
> > > @@ -735,6 +735,16 @@ static void i2c_imx_stop(struct imx_i2c_struct
> > *i2c_imx, bool atomic)
> > >               temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> > >               if (!(temp & I2CR_MSTA))
> > >                       i2c_imx->stopped = 1;
> > > +             if ((temp & I2CR_MSTA) && !(temp & I2CR_MTX)) {
> > > +                     (void)imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> > 
> > why do we need a cast here?
> 
> The void cast explicitly marks this as a dummy read. We're reading I2DR solely to drain the
> receive buffer (a required side effect before mode switching), not because we need the value.
> This makes the intent clear to both the compiler and code reviewers.

Please drop it.

> > > +                     temp |= I2CR_MTX;
> > > +                     imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> > > +                     if (atomic)
> > > +                             udelay(25);
> > 
> > where is this 25 coming from?
> 
> It's a conservative value derived from practical testing. at standard mode (100kHz),
> this equals ~2.5 I2C clock periods; at fast mode (400kHz), ~10 periods

Please add a comment.

Andi


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

end of thread, other threads:[~2026-01-14 15:52 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-29  8:16 [PATCH V1] i2c: imx: Fix SMBus block read hang on zero length LI Qingwu
2026-01-07 20:13 ` Andi Shyti
2026-01-08  7:59   ` Oleksij Rempel
2026-01-08  8:37   ` LI Qingwu
2026-01-14 15:52     ` Andi Shyti
2026-01-09  3:02 ` Carlos Song
2026-01-09 13:53 ` Oleksij Rempel

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