public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/2] i2c: imx: fix i2c issue when reading multiple messages
       [not found] <20260218150940.131354-1-eichest@gmail.com>
@ 2026-02-18 15:08 ` Stefan Eichenberger
  2026-02-18 16:22   ` Frank Li
  2026-02-18 15:08 ` [PATCH v1 2/2] i2c: imx: ensure no clock is generated after last read Stefan Eichenberger
  1 sibling, 1 reply; 10+ messages in thread
From: Stefan Eichenberger @ 2026-02-18 15:08 UTC (permalink / raw)
  To: o.rempel, kernel, andi.shyti, Frank.Li, s.hauer, festevam,
	stefan.eichenberger, francesco.dolcini
  Cc: linux-i2c, imx, linux-arm-kernel, linux-kernel, stable

From: Stefan Eichenberger <stefan.eichenberger@toradex.com>

When reading multiple messages, meaning a repeated start is required,
polling the bus busy bit must be avoided. This must only be done for
the last message. Otherwise, the driver will timeout.

Here an example of such a sequence that fails with an error:
i2ctransfer -y -a 0 w1@0x00 0x02 r1 w1@0x00 0x02 r1
Error: Sending messages failed: Connection timed out

Fixes: 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma mode")
Cc: <stable@vger.kernel.org> # v6.13+
Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
---
 drivers/i2c/busses/i2c-imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 85f554044cf1e..56e2a14495a9a 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1522,7 +1522,7 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs,
 		dev_err(&i2c_imx->adapter.dev, "<%s> read timedout\n", __func__);
 		return -ETIMEDOUT;
 	}
-	if (!i2c_imx->stopped)
+	if (i2c_imx->is_lastmsg && !i2c_imx->stopped)
 		return i2c_imx_bus_busy(i2c_imx, 0, false);
 
 	return 0;
-- 
2.51.0


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

* [PATCH v1 2/2] i2c: imx: ensure no clock is generated after last read
       [not found] <20260218150940.131354-1-eichest@gmail.com>
  2026-02-18 15:08 ` [PATCH v1 1/2] i2c: imx: fix i2c issue when reading multiple messages Stefan Eichenberger
@ 2026-02-18 15:08 ` Stefan Eichenberger
  2026-02-18 16:26   ` Frank Li
  1 sibling, 1 reply; 10+ messages in thread
From: Stefan Eichenberger @ 2026-02-18 15:08 UTC (permalink / raw)
  To: o.rempel, kernel, andi.shyti, Frank.Li, s.hauer, festevam,
	stefan.eichenberger, francesco.dolcini
  Cc: linux-i2c, imx, linux-arm-kernel, linux-kernel, stable

From: Stefan Eichenberger <stefan.eichenberger@toradex.com>

When reading from the I2DR register, right after releasing the bus by
clearing MSTA and MTX, the I2C controller might still generate an
additional clock cycle which can cause devices to misbehave. Ensure to
only read from I2DR after the bus is not busy anymore. Because this
requires polling, the read of the last byte is moved outside of the
interrupt handler.

An example for such a failing transfer is this:
i2ctransfer -y -a 0 w1@0x00 0x02 r1
Error: Sending messages failed: Connection timed out
It does not happen with every device because not all devices react to
the additional clock cycle.

Fixes: 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma mode")
Cc: <stable@vger.kernel.org> # v6.13+
Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
---
 drivers/i2c/busses/i2c-imx.c | 51 ++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 56e2a14495a9a..452d120a210b1 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1018,8 +1018,9 @@ static inline int i2c_imx_isr_read(struct imx_i2c_struct *i2c_imx)
 	return 0;
 }
 
-static inline void i2c_imx_isr_read_continue(struct imx_i2c_struct *i2c_imx)
+static inline enum imx_i2c_state i2c_imx_isr_read_continue(struct imx_i2c_struct *i2c_imx)
 {
+	enum imx_i2c_state next_state = IMX_I2C_STATE_READ_CONTINUE;
 	unsigned int temp;
 
 	if ((i2c_imx->msg->len - 1) == i2c_imx->msg_buf_idx) {
@@ -1033,18 +1034,20 @@ static inline void i2c_imx_isr_read_continue(struct imx_i2c_struct *i2c_imx)
 				i2c_imx->stopped =  1;
 			temp &= ~(I2CR_MSTA | I2CR_MTX);
 			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
-		} else {
-			/*
-			 * For i2c master receiver repeat restart operation like:
-			 * read -> repeat MSTA -> read/write
-			 * The controller must set MTX before read the last byte in
-			 * the first read operation, otherwise the first read cost
-			 * one extra clock cycle.
-			 */
-			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
-			temp |= I2CR_MTX;
-			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+			return IMX_I2C_STATE_DONE;
 		}
+		/*
+		 * For i2c master receiver repeat restart operation like:
+		 * read -> repeat MSTA -> read/write
+		 * The controller must set MTX before read the last byte in
+		 * the first read operation, otherwise the first read cost
+		 * one extra clock cycle.
+		 */
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+		temp |= I2CR_MTX;
+		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+		next_state = IMX_I2C_STATE_DONE;
 	} else if (i2c_imx->msg_buf_idx == (i2c_imx->msg->len - 2)) {
 		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
 		temp |= I2CR_TXAK;
@@ -1052,6 +1055,7 @@ static inline void i2c_imx_isr_read_continue(struct imx_i2c_struct *i2c_imx)
 	}
 
 	i2c_imx->msg->buf[i2c_imx->msg_buf_idx++] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+	return next_state;
 }
 
 static inline void i2c_imx_isr_read_block_data_len(struct imx_i2c_struct *i2c_imx)
@@ -1088,11 +1092,9 @@ static irqreturn_t i2c_imx_master_isr(struct imx_i2c_struct *i2c_imx, unsigned i
 		break;
 
 	case IMX_I2C_STATE_READ_CONTINUE:
-		i2c_imx_isr_read_continue(i2c_imx);
-		if (i2c_imx->msg_buf_idx == i2c_imx->msg->len) {
-			i2c_imx->state = IMX_I2C_STATE_DONE;
+		i2c_imx->state = i2c_imx_isr_read_continue(i2c_imx);
+		if (i2c_imx->state == IMX_I2C_STATE_DONE)
 			wake_up(&i2c_imx->queue);
-		}
 		break;
 
 	case IMX_I2C_STATE_READ_BLOCK_DATA:
@@ -1490,6 +1492,7 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs,
 			bool is_lastmsg)
 {
 	int block_data = msgs->flags & I2C_M_RECV_LEN;
+	int ret = 0;
 
 	dev_dbg(&i2c_imx->adapter.dev,
 		"<%s> write slave address: addr=0x%x\n",
@@ -1522,10 +1525,20 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs,
 		dev_err(&i2c_imx->adapter.dev, "<%s> read timedout\n", __func__);
 		return -ETIMEDOUT;
 	}
-	if (i2c_imx->is_lastmsg && !i2c_imx->stopped)
-		return i2c_imx_bus_busy(i2c_imx, 0, false);
+	if (i2c_imx->is_lastmsg) {
+		if (!i2c_imx->stopped)
+			ret = i2c_imx_bus_busy(i2c_imx, 0, false);
+		/*
+		 * Only read the last byte of the last message after the bus is
+		 * not busy. Else the controller generates another clock which
+		 * might confuse devices.
+		 */
+		if (!ret)
+			i2c_imx->msg->buf[i2c_imx->msg_buf_idx++] = imx_i2c_read_reg(i2c_imx,
+										     IMX_I2C_I2DR);
+	}
 
-	return 0;
+	return ret;
 }
 
 static int i2c_imx_xfer_common(struct i2c_adapter *adapter,
-- 
2.51.0


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

* Re: [PATCH v1 1/2] i2c: imx: fix i2c issue when reading multiple messages
  2026-02-18 15:08 ` [PATCH v1 1/2] i2c: imx: fix i2c issue when reading multiple messages Stefan Eichenberger
@ 2026-02-18 16:22   ` Frank Li
  0 siblings, 0 replies; 10+ messages in thread
From: Frank Li @ 2026-02-18 16:22 UTC (permalink / raw)
  To: Stefan Eichenberger
  Cc: o.rempel, kernel, andi.shyti, s.hauer, festevam,
	stefan.eichenberger, francesco.dolcini, linux-i2c, imx,
	linux-arm-kernel, linux-kernel, stable

On Wed, Feb 18, 2026 at 04:08:49PM +0100, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> When reading multiple messages, meaning a repeated start is required,
> polling the bus busy bit must be avoided. This must only be done for
> the last message. Otherwise, the driver will timeout.
>
> Here an example of such a sequence that fails with an error:
> i2ctransfer -y -a 0 w1@0x00 0x02 r1 w1@0x00 0x02 r1
> Error: Sending messages failed: Connection timed out
>
> Fixes: 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma mode")
> Cc: <stable@vger.kernel.org> # v6.13+
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
>  drivers/i2c/busses/i2c-imx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 85f554044cf1e..56e2a14495a9a 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -1522,7 +1522,7 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs,
>  		dev_err(&i2c_imx->adapter.dev, "<%s> read timedout\n", __func__);
>  		return -ETIMEDOUT;
>  	}
> -	if (!i2c_imx->stopped)
> +	if (i2c_imx->is_lastmsg && !i2c_imx->stopped)
>  		return i2c_imx_bus_busy(i2c_imx, 0, false);
>
>  	return 0;
> --
> 2.51.0
>

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

* Re: [PATCH v1 2/2] i2c: imx: ensure no clock is generated after last read
  2026-02-18 15:08 ` [PATCH v1 2/2] i2c: imx: ensure no clock is generated after last read Stefan Eichenberger
@ 2026-02-18 16:26   ` Frank Li
  2026-02-18 16:37     ` Stefan Eichenberger
  0 siblings, 1 reply; 10+ messages in thread
From: Frank Li @ 2026-02-18 16:26 UTC (permalink / raw)
  To: Stefan Eichenberger
  Cc: o.rempel, kernel, andi.shyti, s.hauer, festevam,
	stefan.eichenberger, francesco.dolcini, linux-i2c, imx,
	linux-arm-kernel, linux-kernel, stable

On Wed, Feb 18, 2026 at 04:08:50PM +0100, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> When reading from the I2DR register, right after releasing the bus by
> clearing MSTA and MTX, the I2C controller might still generate an
> additional clock cycle which can cause devices to misbehave. Ensure to

Do you means SCL have additional toggle? You capture waveform?

Frank
> only read from I2DR after the bus is not busy anymore. Because this
> requires polling, the read of the last byte is moved outside of the
> interrupt handler.
>
> An example for such a failing transfer is this:
> i2ctransfer -y -a 0 w1@0x00 0x02 r1
> Error: Sending messages failed: Connection timed out
> It does not happen with every device because not all devices react to
> the additional clock cycle.
>
> Fixes: 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma mode")
> Cc: <stable@vger.kernel.org> # v6.13+
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> ---
>  drivers/i2c/busses/i2c-imx.c | 51 ++++++++++++++++++++++--------------
>  1 file changed, 32 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 56e2a14495a9a..452d120a210b1 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -1018,8 +1018,9 @@ static inline int i2c_imx_isr_read(struct imx_i2c_struct *i2c_imx)
>  	return 0;
>  }
>
> -static inline void i2c_imx_isr_read_continue(struct imx_i2c_struct *i2c_imx)
> +static inline enum imx_i2c_state i2c_imx_isr_read_continue(struct imx_i2c_struct *i2c_imx)
>  {
> +	enum imx_i2c_state next_state = IMX_I2C_STATE_READ_CONTINUE;
>  	unsigned int temp;
>
>  	if ((i2c_imx->msg->len - 1) == i2c_imx->msg_buf_idx) {
> @@ -1033,18 +1034,20 @@ static inline void i2c_imx_isr_read_continue(struct imx_i2c_struct *i2c_imx)
>  				i2c_imx->stopped =  1;
>  			temp &= ~(I2CR_MSTA | I2CR_MTX);
>  			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> -		} else {
> -			/*
> -			 * For i2c master receiver repeat restart operation like:
> -			 * read -> repeat MSTA -> read/write
> -			 * The controller must set MTX before read the last byte in
> -			 * the first read operation, otherwise the first read cost
> -			 * one extra clock cycle.
> -			 */
> -			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> -			temp |= I2CR_MTX;
> -			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
> +			return IMX_I2C_STATE_DONE;
>  		}
> +		/*
> +		 * For i2c master receiver repeat restart operation like:
> +		 * read -> repeat MSTA -> read/write
> +		 * The controller must set MTX before read the last byte in
> +		 * the first read operation, otherwise the first read cost
> +		 * one extra clock cycle.
> +		 */
> +		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +		temp |= I2CR_MTX;
> +		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +		next_state = IMX_I2C_STATE_DONE;
>  	} else if (i2c_imx->msg_buf_idx == (i2c_imx->msg->len - 2)) {
>  		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>  		temp |= I2CR_TXAK;
> @@ -1052,6 +1055,7 @@ static inline void i2c_imx_isr_read_continue(struct imx_i2c_struct *i2c_imx)
>  	}
>
>  	i2c_imx->msg->buf[i2c_imx->msg_buf_idx++] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> +	return next_state;
>  }
>
>  static inline void i2c_imx_isr_read_block_data_len(struct imx_i2c_struct *i2c_imx)
> @@ -1088,11 +1092,9 @@ static irqreturn_t i2c_imx_master_isr(struct imx_i2c_struct *i2c_imx, unsigned i
>  		break;
>
>  	case IMX_I2C_STATE_READ_CONTINUE:
> -		i2c_imx_isr_read_continue(i2c_imx);
> -		if (i2c_imx->msg_buf_idx == i2c_imx->msg->len) {
> -			i2c_imx->state = IMX_I2C_STATE_DONE;
> +		i2c_imx->state = i2c_imx_isr_read_continue(i2c_imx);
> +		if (i2c_imx->state == IMX_I2C_STATE_DONE)
>  			wake_up(&i2c_imx->queue);
> -		}
>  		break;
>
>  	case IMX_I2C_STATE_READ_BLOCK_DATA:
> @@ -1490,6 +1492,7 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs,
>  			bool is_lastmsg)
>  {
>  	int block_data = msgs->flags & I2C_M_RECV_LEN;
> +	int ret = 0;
>
>  	dev_dbg(&i2c_imx->adapter.dev,
>  		"<%s> write slave address: addr=0x%x\n",
> @@ -1522,10 +1525,20 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs,
>  		dev_err(&i2c_imx->adapter.dev, "<%s> read timedout\n", __func__);
>  		return -ETIMEDOUT;
>  	}
> -	if (i2c_imx->is_lastmsg && !i2c_imx->stopped)
> -		return i2c_imx_bus_busy(i2c_imx, 0, false);
> +	if (i2c_imx->is_lastmsg) {
> +		if (!i2c_imx->stopped)
> +			ret = i2c_imx_bus_busy(i2c_imx, 0, false);
> +		/*
> +		 * Only read the last byte of the last message after the bus is
> +		 * not busy. Else the controller generates another clock which
> +		 * might confuse devices.
> +		 */
> +		if (!ret)
> +			i2c_imx->msg->buf[i2c_imx->msg_buf_idx++] = imx_i2c_read_reg(i2c_imx,
> +										     IMX_I2C_I2DR);
> +	}
>
> -	return 0;
> +	return ret;
>  }
>
>  static int i2c_imx_xfer_common(struct i2c_adapter *adapter,
> --
> 2.51.0
>

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

* Re: [PATCH v1 2/2] i2c: imx: ensure no clock is generated after last read
  2026-02-18 16:26   ` Frank Li
@ 2026-02-18 16:37     ` Stefan Eichenberger
  2026-03-03  9:14       ` Stefan Eichenberger
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Eichenberger @ 2026-02-18 16:37 UTC (permalink / raw)
  To: Frank Li
  Cc: o.rempel, kernel, andi.shyti, s.hauer, festevam,
	stefan.eichenberger, francesco.dolcini, linux-i2c, imx,
	linux-arm-kernel, linux-kernel, stable

Hi Frank,

On Wed, Feb 18, 2026 at 11:26:52AM -0500, Frank Li wrote:
> On Wed, Feb 18, 2026 at 04:08:50PM +0100, Stefan Eichenberger wrote:
> > From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> >
> > When reading from the I2DR register, right after releasing the bus by
> > clearing MSTA and MTX, the I2C controller might still generate an
> > additional clock cycle which can cause devices to misbehave. Ensure to
> 
> Do you means SCL have additional toggle? You capture waveform?
> 

Yes exactly. We were able to capture the waveform when the issue
happens. It doesn't always happen though, it depends on how much time
passes between clearing MSTA and MTX and reading from I2DR.

If you want to see the waveform, I uploaded it to our server:
https://share.toradex.com/dwnhcrl6b9toib6
You can see the additional clock at the right end, after "0x17 + NAK".

Regards,
Stefan

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

* Re: [PATCH v1 2/2] i2c: imx: ensure no clock is generated after last read
  2026-02-18 16:37     ` Stefan Eichenberger
@ 2026-03-03  9:14       ` Stefan Eichenberger
  2026-03-03 15:53         ` Frank Li
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Eichenberger @ 2026-03-03  9:14 UTC (permalink / raw)
  To: Frank Li
  Cc: o.rempel, kernel, andi.shyti, s.hauer, festevam,
	stefan.eichenberger, francesco.dolcini, linux-i2c, imx,
	linux-arm-kernel, linux-kernel, stable

Hi Frank,

On Wed, Feb 18, 2026 at 05:37:54PM +0100, Stefan Eichenberger wrote:
> Hi Frank,
> 
> On Wed, Feb 18, 2026 at 11:26:52AM -0500, Frank Li wrote:
> > On Wed, Feb 18, 2026 at 04:08:50PM +0100, Stefan Eichenberger wrote:
> > > From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> > >
> > > When reading from the I2DR register, right after releasing the bus by
> > > clearing MSTA and MTX, the I2C controller might still generate an
> > > additional clock cycle which can cause devices to misbehave. Ensure to
> > 
> > Do you means SCL have additional toggle? You capture waveform?
> > 
> 
> Yes exactly. We were able to capture the waveform when the issue
> happens. It doesn't always happen though, it depends on how much time
> passes between clearing MSTA and MTX and reading from I2DR.
> 
> If you want to see the waveform, I uploaded it to our server:
> https://share.toradex.com/dwnhcrl6b9toib6
> You can see the additional clock at the right end, after "0x17 + NAK".

Have you had a chance to look at the waveform? Do you have any concerns
about the proposed solution?

Best regards,
Stefan

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

* Re: [PATCH v1 2/2] i2c: imx: ensure no clock is generated after last read
  2026-03-03  9:14       ` Stefan Eichenberger
@ 2026-03-03 15:53         ` Frank Li
  2026-03-04  3:00           ` Carlos Song
  0 siblings, 1 reply; 10+ messages in thread
From: Frank Li @ 2026-03-03 15:53 UTC (permalink / raw)
  To: Stefan Eichenberger, Carlos Song
  Cc: o.rempel, kernel, andi.shyti, s.hauer, festevam,
	stefan.eichenberger, francesco.dolcini, linux-i2c, imx,
	linux-arm-kernel, linux-kernel, stable

On Tue, Mar 03, 2026 at 10:14:08AM +0100, Stefan Eichenberger wrote:
> Hi Frank,
>
> On Wed, Feb 18, 2026 at 05:37:54PM +0100, Stefan Eichenberger wrote:
> > Hi Frank,
> >
> > On Wed, Feb 18, 2026 at 11:26:52AM -0500, Frank Li wrote:
> > > On Wed, Feb 18, 2026 at 04:08:50PM +0100, Stefan Eichenberger wrote:
> > > > From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> > > >
> > > > When reading from the I2DR register, right after releasing the bus by
> > > > clearing MSTA and MTX, the I2C controller might still generate an
> > > > additional clock cycle which can cause devices to misbehave. Ensure to
> > >
> > > Do you means SCL have additional toggle? You capture waveform?
> > >
> >
> > Yes exactly. We were able to capture the waveform when the issue
> > happens. It doesn't always happen though, it depends on how much time
> > passes between clearing MSTA and MTX and reading from I2DR.
> >
> > If you want to see the waveform, I uploaded it to our server:
> > https://share.toradex.com/dwnhcrl6b9toib6
> > You can see the additional clock at the right end, after "0x17 + NAK".
>
> Have you had a chance to look at the waveform? Do you have any concerns
> about the proposed solution?

I am fine. Add carlos, who did many work about I2C.

Frank
>
> Best regards,
> Stefan

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

* RE: [PATCH v1 2/2] i2c: imx: ensure no clock is generated after last read
  2026-03-03 15:53         ` Frank Li
@ 2026-03-04  3:00           ` Carlos Song
  2026-03-19 23:09             ` Andi Shyti
  0 siblings, 1 reply; 10+ messages in thread
From: Carlos Song @ 2026-03-04  3:00 UTC (permalink / raw)
  To: Frank Li, Stefan Eichenberger
  Cc: o.rempel@pengutronix.de, kernel@pengutronix.de,
	andi.shyti@kernel.org, s.hauer@pengutronix.de, festevam@gmail.com,
	stefan.eichenberger@toradex.com, Francesco Dolcini,
	linux-i2c@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org



> -----Original Message-----
> From: Frank Li <frank.li@nxp.com>
> Sent: Tuesday, March 3, 2026 11:54 PM
> To: Stefan Eichenberger <eichest@gmail.com>; Carlos Song
> <carlos.song@nxp.com>
> Cc: o.rempel@pengutronix.de; kernel@pengutronix.de; andi.shyti@kernel.org;
> s.hauer@pengutronix.de; festevam@gmail.com;
> stefan.eichenberger@toradex.com; Francesco Dolcini
> <francesco.dolcini@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 v1 2/2] i2c: imx: ensure no clock is generated after last
> read
> 
> On Tue, Mar 03, 2026 at 10:14:08AM +0100, Stefan Eichenberger wrote:
> > Hi Frank,
> >
> > On Wed, Feb 18, 2026 at 05:37:54PM +0100, Stefan Eichenberger wrote:
> > > Hi Frank,
> > >
> > > On Wed, Feb 18, 2026 at 11:26:52AM -0500, Frank Li wrote:
> > > > On Wed, Feb 18, 2026 at 04:08:50PM +0100, Stefan Eichenberger wrote:
> > > > > From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> > > > >
> > > > > When reading from the I2DR register, right after releasing the
> > > > > bus by clearing MSTA and MTX, the I2C controller might still
> > > > > generate an additional clock cycle which can cause devices to
> > > > > misbehave. Ensure to
> > > >
> > > > Do you means SCL have additional toggle? You capture waveform?
> > > >
> > >
> > > Yes exactly. We were able to capture the waveform when the issue
> > > happens. It doesn't always happen though, it depends on how much
> > > time passes between clearing MSTA and MTX and reading from I2DR.
> > >
> > > If you want to see the waveform, I uploaded it to our server:
> > > https://share.toradex.com/dwnhcrl6b9toib6
> > > You can see the additional clock at the right end, after "0x17 + NAK".
> >
> > Have you had a chance to look at the waveform? Do you have any
> > concerns about the proposed solution?
> 
> I am fine. Add carlos, who did many work about I2C.
> 
> Frank

Hi, 

Just review this series, looks this series patch make this fix for the limitation[1] safer:
"It must generate STOP before read I2DR to prevent controller from generating another clock cycle".

Previous patch[2] has done this to avoid the limitation. However according to the waveform, I2C controller still generated an additional clock cycle sometime.

The key of patch is ensure to read the last bytes after the bus is not busy anymore to avoid this another clock cycle.So these patches are fine to me also.

[1] 054b62d9f25c ("i2c: imx: fix the i2c bus hang issue when do repeat restart")
[2] 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma mode")
> >
> > Best regards,
> > Stefan

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

* Re: [PATCH v1 2/2] i2c: imx: ensure no clock is generated after last read
  2026-03-04  3:00           ` Carlos Song
@ 2026-03-19 23:09             ` Andi Shyti
  2026-03-20  2:04               ` [EXT] " Carlos Song
  0 siblings, 1 reply; 10+ messages in thread
From: Andi Shyti @ 2026-03-19 23:09 UTC (permalink / raw)
  To: Carlos Song
  Cc: Frank Li, Stefan Eichenberger, o.rempel@pengutronix.de,
	kernel@pengutronix.de, s.hauer@pengutronix.de, festevam@gmail.com,
	stefan.eichenberger@toradex.com, Francesco Dolcini,
	linux-i2c@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org

Hi Carlos,

> > > > > > When reading from the I2DR register, right after releasing the
> > > > > > bus by clearing MSTA and MTX, the I2C controller might still
> > > > > > generate an additional clock cycle which can cause devices to
> > > > > > misbehave. Ensure to
> > > > >
> > > > > Do you means SCL have additional toggle? You capture waveform?
> > > > >
> > > >
> > > > Yes exactly. We were able to capture the waveform when the issue
> > > > happens. It doesn't always happen though, it depends on how much
> > > > time passes between clearing MSTA and MTX and reading from I2DR.
> > > >
> > > > If you want to see the waveform, I uploaded it to our server:
> > > > https://share.toradex.com/dwnhcrl6b9toib6
> > > > You can see the additional clock at the right end, after "0x17 + NAK".
> > >
> > > Have you had a chance to look at the waveform? Do you have any
> > > concerns about the proposed solution?
> > 
> > I am fine. Add carlos, who did many work about I2C.
> > 
> > Frank
> 
> Hi, 
> 
> Just review this series, looks this series patch make this fix for the limitation[1] safer:
> "It must generate STOP before read I2DR to prevent controller from generating another clock cycle".
> 
> Previous patch[2] has done this to avoid the limitation. However according to the waveform, I2C controller still generated an additional clock cycle sometime.
> 
> The key of patch is ensure to read the last bytes after the bus is not busy anymore to avoid this another clock cycle.So these patches are fine to me also.
> 
> [1] 054b62d9f25c ("i2c: imx: fix the i2c bus hang issue when do repeat restart")
> [2] 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma mode")

Sorry, I'm not understanding your point here. Are you suggesting
to change the Fixes tag to [1]?

Thanks,
Andi

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

* RE: [EXT] Re: [PATCH v1 2/2] i2c: imx: ensure no clock is generated after last read
  2026-03-19 23:09             ` Andi Shyti
@ 2026-03-20  2:04               ` Carlos Song
  0 siblings, 0 replies; 10+ messages in thread
From: Carlos Song @ 2026-03-20  2:04 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Frank Li, Stefan Eichenberger, o.rempel@pengutronix.de,
	kernel@pengutronix.de, s.hauer@pengutronix.de, festevam@gmail.com,
	stefan.eichenberger@toradex.com, Francesco Dolcini,
	linux-i2c@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org



> -----Original Message-----
> From: Andi Shyti <andi.shyti@kernel.org>
> Sent: Friday, March 20, 2026 7:10 AM
> To: Carlos Song <carlos.song@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; Stefan Eichenberger <eichest@gmail.com>;
> o.rempel@pengutronix.de; kernel@pengutronix.de; s.hauer@pengutronix.de;
> festevam@gmail.com; stefan.eichenberger@toradex.com; Francesco Dolcini
> <francesco.dolcini@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: [EXT] Re: [PATCH v1 2/2] i2c: imx: ensure no clock is generated after
> last read
> 
> 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
> 
> 
> Hi Carlos,
> 
> > > > > > > When reading from the I2DR register, right after releasing
> > > > > > > the bus by clearing MSTA and MTX, the I2C controller might
> > > > > > > still generate an additional clock cycle which can cause
> > > > > > > devices to misbehave. Ensure to
> > > > > >
> > > > > > Do you means SCL have additional toggle? You capture waveform?
> > > > > >
> > > > >
> > > > > Yes exactly. We were able to capture the waveform when the issue
> > > > > happens. It doesn't always happen though, it depends on how much
> > > > > time passes between clearing MSTA and MTX and reading from I2DR.
> > > > >
> > > > > If you want to see the waveform, I uploaded it to our server:
> > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%
> > > > >
> 2Fshare.toradex.com%2Fdwnhcrl6b9toib6&data=05%7C02%7Ccarlos.song
> > > > > %40nxp.com%7C9567b791c35345a75eba08de860c9954%7C686ea1d
> 3bc2b4c6f
> > > > >
> a92cd99c5c301635%7C0%7C0%7C639095585800985569%7CUnknown%7CT
> WFpbG
> > > > >
> Zsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMi
> > > > >
> IsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=AK6DGztAek
> > > > > 8Dq0M8G98PO%2F68UhcwfJPxsLSJQZ4jumE%3D&reserved=0
> > > > > You can see the additional clock at the right end, after "0x17 + NAK".
> > > >
> > > > Have you had a chance to look at the waveform? Do you have any
> > > > concerns about the proposed solution?
> > >
> > > I am fine. Add carlos, who did many work about I2C.
> > >
> > > Frank
> >
> > Hi,
> >
> > Just review this series, looks this series patch make this fix for the limitation[1]
> safer:
> > "It must generate STOP before read I2DR to prevent controller from
> generating another clock cycle".
> >
> > Previous patch[2] has done this to avoid the limitation. However according to
> the waveform, I2C controller still generated an additional clock cycle
> sometime.
> >
> > The key of patch is ensure to read the last bytes after the bus is not busy
> anymore to avoid this another clock cycle.So these patches are fine to me also.
> >
> > [1] 054b62d9f25c ("i2c: imx: fix the i2c bus hang issue when do repeat
> > restart") [2] 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma
> > mode")
> 
> Sorry, I'm not understanding your point here. Are you suggesting to change the
> Fixes tag to [1]?
> 

Hi, Andi

No, don't need to do anything, . I agreed with this patch totally, it is reasonable.

> Thanks,
> Andi


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

end of thread, other threads:[~2026-03-20  2:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260218150940.131354-1-eichest@gmail.com>
2026-02-18 15:08 ` [PATCH v1 1/2] i2c: imx: fix i2c issue when reading multiple messages Stefan Eichenberger
2026-02-18 16:22   ` Frank Li
2026-02-18 15:08 ` [PATCH v1 2/2] i2c: imx: ensure no clock is generated after last read Stefan Eichenberger
2026-02-18 16:26   ` Frank Li
2026-02-18 16:37     ` Stefan Eichenberger
2026-03-03  9:14       ` Stefan Eichenberger
2026-03-03 15:53         ` Frank Li
2026-03-04  3:00           ` Carlos Song
2026-03-19 23:09             ` Andi Shyti
2026-03-20  2:04               ` [EXT] " Carlos Song

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