[PATCH 0/3] i2c: designware: Improve support of multi-messages transfer

[PATCH 0/3] i2c: designware: Improve support of multi-messages transfer

[Benoît Monin]

Extend what can be done when transferring multiple messages in a single
call to .xfer().

Allow changing the target address by waiting for a STOP then looping
in i2c_dw_xfer() instead of erroring out when a change of address is
detected. The loop then re-run i2c_dw_xfer_init() which changes the
target address and restart the transfer for the rest of the messages.

Handle controllers that lack the ability to emit a RESTART when two
consecutive messages have the same address and direction, by waiting
for a STOP and restarting the rest of the transfer.

The i2c controllers found in the EyeQ6Lplus and EyeQ7H SoC from
Mobileye lack such capability, so compatible strings are added because
this cannot be detected at runtime.

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
Benoît Monin (3):
      dt-bindings: i2c: dw: Add Mobileye I2C controllers
      i2c: designware: Enable transfer with different target addresses
      i2c: designware: Support of controller with IC_EMPTYFIFO_HOLD_MASTER disabled

 .../bindings/i2c/snps,designware-i2c.yaml          |  2 +
 drivers/i2c/busses/i2c-designware-core.h           |  1 +
 drivers/i2c/busses/i2c-designware-master.c         | 97 ++++++++++++++--------
 drivers/i2c/busses/i2c-designware-platdrv.c        |  6 +-
 4 files changed, 68 insertions(+), 38 deletions(-)
---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20251014-i2c-dw-da315f758296

Best regards,
-- 
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 1/3] dt-bindings: i2c: dw: Add Mobileye I2C controllers

[Benoît Monin]

Add compatible strings for the I2C controllers present in Mobileye
Eyeq6Lplus and EyeQ7H SoCs.

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
 Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
index d904191bb0c6e..6d63dc67f7bf0 100644
--- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
@@ -36,6 +36,8 @@ properties:
         const: baikal,bt1-sys-i2c
       - items:
           - enum:
+              - mobileye,eyeq6lplus-i2c
+              - mobileye,eyeq7h-i2c
               - mscc,ocelot-i2c
               - sophgo,sg2044-i2c
               - thead,th1520-i2c

-- 
2.51.0

[PATCH 2/3] i2c: designware: Enable transfer with different target addresses

[Benoît Monin]

When i2c_dw_xfer() is called with more than one message, it sets the
target address according to the first message. If any of the following
messages have a different target address, the transfer finishes with
an error.

Instead, if the next message has a different target address, wait until
all previous messages are sent and the STOP condition is detected. This
will complete the current part of the transfer. The next part is then
handled by looping in i2c_dw_xfer(), calling i2c_dw_xfer_init() and
i2c_dw_wait_transfer() until all messages of the transfer have been
processed, or an error is detected.

The RESTART bit is now set after the first message of each part of the
transfer, instead of just after the very first message of the whole
transfer.

For each address change, i2c_dw_xfer_init() is called, which takes care
of disabling the adapter before changing the target address register,
then re-enabling it. Given that we cannot know the value of the
I2C_DYNAMIC_TAR_UPDATE parameter, this is the only sure way to change
the target address.

Based on the work of Dmitry Guzman <dmitry.guzman@mobileye.com>

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
 drivers/i2c/busses/i2c-designware-master.c | 58 ++++++++++++++++--------------
 1 file changed, 31 insertions(+), 27 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index c7a72c28786c2..f9a180b145da8 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -436,6 +436,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 	u8 *buf = dev->tx_buf;
 	bool need_restart = false;
 	unsigned int flr;
+	int first_idx = dev->msg_write_idx;
 
 	intr_mask = DW_IC_INTR_MASTER_MASK;
 
@@ -446,11 +447,11 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 		 * If target address has changed, we need to
 		 * reprogram the target address in the I2C
 		 * adapter when we are done with this transfer.
+		 * This can be done after STOP_DET IRQ flag is raised.
+		 * So, disable "TX FIFO empty" interrupt.
 		 */
 		if (msgs[dev->msg_write_idx].addr != addr) {
-			dev_err(dev->dev,
-				"%s: invalid target address\n", __func__);
-			dev->msg_err = -EINVAL;
+			intr_mask &= ~DW_IC_INTR_TX_EMPTY;
 			break;
 		}
 
@@ -465,7 +466,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 			 * set restart bit between messages.
 			 */
 			if ((dev->master_cfg & DW_IC_CON_RESTART_EN) &&
-					(dev->msg_write_idx > 0))
+					(dev->msg_write_idx > first_idx))
 				need_restart = true;
 		}
 
@@ -822,7 +823,6 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 		break;
 	}
 
-	reinit_completion(&dev->cmd_complete);
 	dev->msgs = msgs;
 	dev->msgs_num = num;
 	dev->cmd_err = 0;
@@ -841,18 +841,33 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	if (ret < 0)
 		goto done;
 
-	/* Start the transfers */
-	i2c_dw_xfer_init(dev);
+	do {
+		reinit_completion(&dev->cmd_complete);
 
-	/* Wait for tx to complete */
-	ret = i2c_dw_wait_transfer(dev);
-	if (ret) {
-		dev_err(dev->dev, "controller timed out\n");
-		/* i2c_dw_init_master() implicitly disables the adapter */
-		i2c_recover_bus(&dev->adapter);
-		i2c_dw_init_master(dev);
-		goto done;
-	}
+		/* Start the transfers */
+		i2c_dw_xfer_init(dev);
+
+		/* Wait for tx to complete */
+		ret = i2c_dw_wait_transfer(dev);
+		if (ret) {
+			dev_err(dev->dev, "controller timed out\n");
+			/* i2c_dw_init_master() implicitly disables the adapter */
+			i2c_recover_bus(&dev->adapter);
+			i2c_dw_init_master(dev);
+			goto done;
+		}
+
+		if (dev->msg_err) {
+			ret = dev->msg_err;
+			goto done;
+		}
+
+		/* We have an error */
+		if (dev->cmd_err == DW_IC_ERR_TX_ABRT) {
+			ret = i2c_dw_handle_tx_abort(dev);
+			goto done;
+		}
+	} while (dev->msg_write_idx < num);
 
 	/*
 	 * This happens rarely (~1:500) and is hard to reproduce. Debug trace
@@ -874,23 +889,12 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	 */
 	__i2c_dw_disable_nowait(dev);
 
-	if (dev->msg_err) {
-		ret = dev->msg_err;
-		goto done;
-	}
-
 	/* No error */
 	if (likely(!dev->cmd_err && !dev->status)) {
 		ret = num;
 		goto done;
 	}
 
-	/* We have an error */
-	if (dev->cmd_err == DW_IC_ERR_TX_ABRT) {
-		ret = i2c_dw_handle_tx_abort(dev);
-		goto done;
-	}
-
 	if (dev->status)
 		dev_err(dev->dev,
 			"transfer terminated early - interrupt latency too high?\n");

-- 
2.51.0

[PATCH 3/3] i2c: designware: Support of controller with IC_EMPTYFIFO_HOLD_MASTER disabled

[Benoît Monin]

If IC_EMPTYFIFO_HOLD_MASTER_EN parameter is 0, "Stop" and "Repeated
Start" bits in command register doesn't exist, thus it is impossible to
send several consecutive write messages in a single hardware batch. The
existing implementation worked with such configuration incorrectly: all
consequent write messages joined into a single message without any
Start/Stop or Repeated Start conditions. For example, the following
command:

    i2ctransfer -y 0 w1@0x55 0x00 w1@0x55 0x01

does the same as

    i2ctransfer -y 0 w2@0x55 0x00 0x01

To fix it, for the controllers that behave this way, if the next message
to the same slave device has the same direction as the previous one, it
is sent to the controller only after the previous message is sent and
STOP_DET IRQ flag is raised by the controller.

This behavior is activated by compatible entries, because the state of
the IC_EMPTYFIFO_HOLD_MASTER_EN parameter cannot be detected at runtime.
Add the compatible entries of Mobileye SoCs needing the work-around and
sort the entries alphabetically.

There is another possible problem with this controller configuration:
When the CPU is putting commands to the FIFO, this process must not be
interrupted because if FIFO buffer gets empty, the controller finishes
the I2C transaction and generates STOP condition on the bus.

In a PREEMPT-RT kernel, interrupt handlers are by default executed in
thread and may be interrupted, which can lead to breaking an I2C message
by inserting an unwanted STOP.

To ensure proper operation on realtime kernel, use IRQF_NO_THREAD flag
when requesting IRQ.

Based on the work of Dmitry Guzman <dmitry.guzman@mobileye.com>

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
 drivers/i2c/busses/i2c-designware-core.h    |  1 +
 drivers/i2c/busses/i2c-designware-master.c  | 45 +++++++++++++++++++++--------
 drivers/i2c/busses/i2c-designware-platdrv.c |  6 ++--
 3 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 347843b4f5dd7..a31a8698e511a 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -311,6 +311,7 @@ struct dw_i2c_dev {
 #define ACCESS_NO_IRQ_SUSPEND			BIT(1)
 #define ARBITRATION_SEMAPHORE			BIT(2)
 #define ACCESS_POLLING				BIT(3)
+#define NO_EMPTYFIFO_HOLD_MASTER		BIT(4)
 
 #define MODEL_MSCC_OCELOT			BIT(8)
 #define MODEL_BAIKAL_BT1			BIT(9)
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index f9a180b145da8..e5af0439ec832 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -443,18 +443,6 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 	for (; dev->msg_write_idx < dev->msgs_num; dev->msg_write_idx++) {
 		u32 flags = msgs[dev->msg_write_idx].flags;
 
-		/*
-		 * If target address has changed, we need to
-		 * reprogram the target address in the I2C
-		 * adapter when we are done with this transfer.
-		 * This can be done after STOP_DET IRQ flag is raised.
-		 * So, disable "TX FIFO empty" interrupt.
-		 */
-		if (msgs[dev->msg_write_idx].addr != addr) {
-			intr_mask &= ~DW_IC_INTR_TX_EMPTY;
-			break;
-		}
-
 		if (!(dev->status & STATUS_WRITE_IN_PROGRESS)) {
 			/* new i2c_msg */
 			buf = msgs[dev->msg_write_idx].buf;
@@ -470,6 +458,25 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 				need_restart = true;
 		}
 
+		/*
+		 * If target address has changed, we need to
+		 * reprogram the target address in the I2C
+		 * adapter when we are done with this transfer.
+		 * This can be done after STOP_DET IRQ flag is raised.
+		 * So, disable "TX FIFO empty" interrupt.
+		 * Also force a stop-then-start between two messages
+		 * in the same direction if we need to restart on a
+		 * adapter that does not handle restart.
+		 */
+		if (msgs[dev->msg_write_idx].addr != addr ||
+		    ((need_restart &&
+		     dev->flags & NO_EMPTYFIFO_HOLD_MASTER &&
+		     ((msgs[dev->msg_write_idx].flags & I2C_M_RD) ==
+		      (msgs[dev->msg_write_idx - 1].flags & I2C_M_RD))))) {
+			intr_mask &= ~DW_IC_INTR_TX_EMPTY;
+			break;
+		}
+
 		regmap_read(dev->map, DW_IC_TXFLR, &flr);
 		tx_limit = dev->tx_fifo_depth - flr;
 
@@ -1062,6 +1069,20 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
 		irq_flags = IRQF_SHARED | IRQF_COND_SUSPEND;
 	}
 
+	/*
+	 * The first writing to TX FIFO buffer causes transmission start. If
+	 * IC_EMPTYFIFO_HOLD_MASTER_EN is not set, when TX FIFO gets empty, I2C
+	 * controller finishes the transaction. If writing to FIFO is
+	 * interrupted, FIFO can get empty and the transaction will be finished
+	 * prematurely. FIFO buffer is filled in IRQ handler, but in PREEMPT_RT
+	 * kernel IRQ handler by default is executed in thread that can be
+	 * preempted with another higher priority thread or an interrupt. So,
+	 * IRQF_NO_THREAD flag is required in order to prevent any preemption
+	 * during filling the FIFO buffer and possible data lost.
+	 */
+	if (dev->flags & NO_EMPTYFIFO_HOLD_MASTER)
+		irq_flags |= IRQF_NO_THREAD;
+
 	ret = i2c_dw_acquire_lock(dev);
 	if (ret)
 		return ret;
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 34d881572351c..5e459175dcdb2 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -345,9 +345,11 @@ static void dw_i2c_plat_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id dw_i2c_of_match[] = {
-	{ .compatible = "snps,designware-i2c", },
-	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
 	{ .compatible = "baikal,bt1-sys-i2c", .data = (void *)MODEL_BAIKAL_BT1 },
+	{ .compatible = "mobileye,eyeq6lplus-i2c", .data = (void *)NO_EMPTYFIFO_HOLD_MASTER },
+	{ .compatible = "mobileye,eyeq7h-i2c", .data = (void *)NO_EMPTYFIFO_HOLD_MASTER },
+	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
+	{ .compatible = "snps,designware-i2c", },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dw_i2c_of_match);

-- 
2.51.0

Re: [PATCH 1/3] dt-bindings: i2c: dw: Add Mobileye I2C controllers

[Krzysztof Kozlowski]

On 17/10/2025 16:59, Benoît Monin wrote:
> Add compatible strings for the I2C controllers present in Mobileye
> Eyeq6Lplus and EyeQ7H SoCs.
> 
> Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
> ---
>  Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> index d904191bb0c6e..6d63dc67f7bf0 100644
> --- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> @@ -36,6 +36,8 @@ properties:
>          const: baikal,bt1-sys-i2c
>        - items:
>            - enum:
> +              - mobileye,eyeq6lplus-i2c
> +              - mobileye,eyeq7h-i2c

It seems these are compatible with each other, at least same driver
match data suggests that, so express it here and in the commit msg.

Best regards,
Krzysztof

Re: [PATCH 3/3] i2c: designware: Support of controller with IC_EMPTYFIFO_HOLD_MASTER disabled

[Krzysztof Kozlowski]

On 17/10/2025 16:59, Benoît Monin wrote:
>  
>  static const struct of_device_id dw_i2c_of_match[] = {
> -	{ .compatible = "snps,designware-i2c", },
> -	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
>  	{ .compatible = "baikal,bt1-sys-i2c", .data = (void *)MODEL_BAIKAL_BT1 },
> +	{ .compatible = "mobileye,eyeq6lplus-i2c", .data = (void *)NO_EMPTYFIFO_HOLD_MASTER },
> +	{ .compatible = "mobileye,eyeq7h-i2c", .data = (void *)NO_EMPTYFIFO_HOLD_MASTER },

That's not necessary. Don't create duplicated entries, express
compatibility.

Best regards,
Krzysztof

Re: [PATCH 0/3] i2c: designware: Improve support of multi-messages transfer

[Andy Shevchenko]

+Cc: Hans. Hans, isn't it what you wanted to have for your use-case?

On Fri, Oct 17, 2025 at 04:59:31PM +0200, Benoît Monin wrote:
> Extend what can be done when transferring multiple messages in a single
> call to .xfer().
> 
> Allow changing the target address by waiting for a STOP then looping
> in i2c_dw_xfer() instead of erroring out when a change of address is
> detected. The loop then re-run i2c_dw_xfer_init() which changes the
> target address and restart the transfer for the rest of the messages.
> 
> Handle controllers that lack the ability to emit a RESTART when two
> consecutive messages have the same address and direction, by waiting
> for a STOP and restarting the rest of the transfer.
> 
> The i2c controllers found in the EyeQ6Lplus and EyeQ7H SoC from
> Mobileye lack such capability, so compatible strings are added because
> this cannot be detected at runtime.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] i2c: designware: Support of controller with IC_EMPTYFIFO_HOLD_MASTER disabled

[Andy Shevchenko]

On Fri, Oct 17, 2025 at 04:59:34PM +0200, Benoît Monin wrote:
> If IC_EMPTYFIFO_HOLD_MASTER_EN parameter is 0, "Stop" and "Repeated
> Start" bits in command register doesn't exist, thus it is impossible to
> send several consecutive write messages in a single hardware batch. The
> existing implementation worked with such configuration incorrectly: all
> consequent write messages joined into a single message without any
> Start/Stop or Repeated Start conditions. For example, the following
> command:
> 
>     i2ctransfer -y 0 w1@0x55 0x00 w1@0x55 0x01
> 
> does the same as
> 
>     i2ctransfer -y 0 w2@0x55 0x00 0x01
> 
> To fix it, for the controllers that behave this way, if the next message
> to the same slave device has the same direction as the previous one, it
> is sent to the controller only after the previous message is sent and
> STOP_DET IRQ flag is raised by the controller.
> 
> This behavior is activated by compatible entries, because the state of
> the IC_EMPTYFIFO_HOLD_MASTER_EN parameter cannot be detected at runtime.
> Add the compatible entries of Mobileye SoCs needing the work-around and
> sort the entries alphabetically.
> 
> There is another possible problem with this controller configuration:
> When the CPU is putting commands to the FIFO, this process must not be
> interrupted because if FIFO buffer gets empty, the controller finishes
> the I2C transaction and generates STOP condition on the bus.
> 
> In a PREEMPT-RT kernel, interrupt handlers are by default executed in
> thread and may be interrupted, which can lead to breaking an I2C message
> by inserting an unwanted STOP.
> 
> To ensure proper operation on realtime kernel, use IRQF_NO_THREAD flag
> when requesting IRQ.

> Based on the work of Dmitry Guzman <dmitry.guzman@mobileye.com>

You may also use a tag "Originally-by".

...

>  static const struct of_device_id dw_i2c_of_match[] = {
> -	{ .compatible = "snps,designware-i2c", },
> -	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
>  	{ .compatible = "baikal,bt1-sys-i2c", .data = (void *)MODEL_BAIKAL_BT1 },
> +	{ .compatible = "mobileye,eyeq6lplus-i2c", .data = (void *)NO_EMPTYFIFO_HOLD_MASTER },
> +	{ .compatible = "mobileye,eyeq7h-i2c", .data = (void *)NO_EMPTYFIFO_HOLD_MASTER },
> +	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
> +	{ .compatible = "snps,designware-i2c", },

Sorting can be moved to a separate change (and while at that the inner trailing
comma can be dropped).

>  	{}
>  };

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3] i2c: designware: Enable transfer with different target addresses

[Hans Verkuil]

Hi Benoît,

On 17/10/2025 16:59, Benoît Monin wrote:
> When i2c_dw_xfer() is called with more than one message, it sets the
> target address according to the first message. If any of the following
> messages have a different target address, the transfer finishes with
> an error.
> 
> Instead, if the next message has a different target address, wait until
> all previous messages are sent and the STOP condition is detected. This
> will complete the current part of the transfer. The next part is then
> handled by looping in i2c_dw_xfer(), calling i2c_dw_xfer_init() and
> i2c_dw_wait_transfer() until all messages of the transfer have been
> processed, or an error is detected.
> 
> The RESTART bit is now set after the first message of each part of the
> transfer, instead of just after the very first message of the whole
> transfer.
> 
> For each address change, i2c_dw_xfer_init() is called, which takes care
> of disabling the adapter before changing the target address register,
> then re-enabling it. Given that we cannot know the value of the
> I2C_DYNAMIC_TAR_UPDATE parameter, this is the only sure way to change
> the target address.

I have the problem described here:

https://lore.kernel.org/linux-i2c/ee6afdd7-3117-43cd-831f-e0ec5ee46f46@kernel.org/

And it looks like this patch is intended to solve that problem (one transaction
with two writes to different target addresses).

I tried this patch, but it doesn't work. Instead I get a time out:

[  111.695238] i2c_designware 1f00074000.i2c: controller timed out

Is it indeed meant to solve the problem I have or is it addressing another
issue?

I'm happy to help test patches.

Regards,

	Hans

> 
> Based on the work of Dmitry Guzman <dmitry.guzman@mobileye.com>
> 
> Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
> ---
>  drivers/i2c/busses/i2c-designware-master.c | 58 ++++++++++++++++--------------
>  1 file changed, 31 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
> index c7a72c28786c2..f9a180b145da8 100644
> --- a/drivers/i2c/busses/i2c-designware-master.c
> +++ b/drivers/i2c/busses/i2c-designware-master.c
> @@ -436,6 +436,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
>  	u8 *buf = dev->tx_buf;
>  	bool need_restart = false;
>  	unsigned int flr;
> +	int first_idx = dev->msg_write_idx;
>  
>  	intr_mask = DW_IC_INTR_MASTER_MASK;
>  
> @@ -446,11 +447,11 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
>  		 * If target address has changed, we need to
>  		 * reprogram the target address in the I2C
>  		 * adapter when we are done with this transfer.
> +		 * This can be done after STOP_DET IRQ flag is raised.
> +		 * So, disable "TX FIFO empty" interrupt.
>  		 */
>  		if (msgs[dev->msg_write_idx].addr != addr) {
> -			dev_err(dev->dev,
> -				"%s: invalid target address\n", __func__);
> -			dev->msg_err = -EINVAL;
> +			intr_mask &= ~DW_IC_INTR_TX_EMPTY;
>  			break;
>  		}
>  
> @@ -465,7 +466,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
>  			 * set restart bit between messages.
>  			 */
>  			if ((dev->master_cfg & DW_IC_CON_RESTART_EN) &&
> -					(dev->msg_write_idx > 0))
> +					(dev->msg_write_idx > first_idx))
>  				need_restart = true;
>  		}
>  
> @@ -822,7 +823,6 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
>  		break;
>  	}
>  
> -	reinit_completion(&dev->cmd_complete);
>  	dev->msgs = msgs;
>  	dev->msgs_num = num;
>  	dev->cmd_err = 0;
> @@ -841,18 +841,33 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
>  	if (ret < 0)
>  		goto done;
>  
> -	/* Start the transfers */
> -	i2c_dw_xfer_init(dev);
> +	do {
> +		reinit_completion(&dev->cmd_complete);
>  
> -	/* Wait for tx to complete */
> -	ret = i2c_dw_wait_transfer(dev);
> -	if (ret) {
> -		dev_err(dev->dev, "controller timed out\n");
> -		/* i2c_dw_init_master() implicitly disables the adapter */
> -		i2c_recover_bus(&dev->adapter);
> -		i2c_dw_init_master(dev);
> -		goto done;
> -	}
> +		/* Start the transfers */
> +		i2c_dw_xfer_init(dev);
> +
> +		/* Wait for tx to complete */
> +		ret = i2c_dw_wait_transfer(dev);
> +		if (ret) {
> +			dev_err(dev->dev, "controller timed out\n");
> +			/* i2c_dw_init_master() implicitly disables the adapter */
> +			i2c_recover_bus(&dev->adapter);
> +			i2c_dw_init_master(dev);
> +			goto done;
> +		}
> +
> +		if (dev->msg_err) {
> +			ret = dev->msg_err;
> +			goto done;
> +		}
> +
> +		/* We have an error */
> +		if (dev->cmd_err == DW_IC_ERR_TX_ABRT) {
> +			ret = i2c_dw_handle_tx_abort(dev);
> +			goto done;
> +		}
> +	} while (dev->msg_write_idx < num);
>  
>  	/*
>  	 * This happens rarely (~1:500) and is hard to reproduce. Debug trace
> @@ -874,23 +889,12 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
>  	 */
>  	__i2c_dw_disable_nowait(dev);
>  
> -	if (dev->msg_err) {
> -		ret = dev->msg_err;
> -		goto done;
> -	}
> -
>  	/* No error */
>  	if (likely(!dev->cmd_err && !dev->status)) {
>  		ret = num;
>  		goto done;
>  	}
>  
> -	/* We have an error */
> -	if (dev->cmd_err == DW_IC_ERR_TX_ABRT) {
> -		ret = i2c_dw_handle_tx_abort(dev);
> -		goto done;
> -	}
> -
>  	if (dev->status)
>  		dev_err(dev->dev,
>  			"transfer terminated early - interrupt latency too high?\n");
> 

Re: [PATCH 2/3] i2c: designware: Enable transfer with different target addresses

[Benoît Monin]

Hello Hans,

On Monday, 20 October 2025 at 11:38:38 CEST, Hans Verkuil wrote:
> Hi Benoît,
> 
> On 17/10/2025 16:59, Benoît Monin wrote:
> > When i2c_dw_xfer() is called with more than one message, it sets the
> > target address according to the first message. If any of the following
> > messages have a different target address, the transfer finishes with
> > an error.
> > 
> > Instead, if the next message has a different target address, wait until
> > all previous messages are sent and the STOP condition is detected. This
> > will complete the current part of the transfer. The next part is then
> > handled by looping in i2c_dw_xfer(), calling i2c_dw_xfer_init() and
> > i2c_dw_wait_transfer() until all messages of the transfer have been
> > processed, or an error is detected.
> > 
> > The RESTART bit is now set after the first message of each part of the
> > transfer, instead of just after the very first message of the whole
> > transfer.
> > 
> > For each address change, i2c_dw_xfer_init() is called, which takes care
> > of disabling the adapter before changing the target address register,
> > then re-enabling it. Given that we cannot know the value of the
> > I2C_DYNAMIC_TAR_UPDATE parameter, this is the only sure way to change
> > the target address.
> 
> I have the problem described here:
> 
> https://lore.kernel.org/linux-i2c/ee6afdd7-3117-43cd-831f-e0ec5ee46f46@kernel.org/
> 
> And it looks like this patch is intended to solve that problem (one transaction
> with two writes to different target addresses).
> 
> I tried this patch, but it doesn't work. Instead I get a time out:
> 
> [  111.695238] i2c_designware 1f00074000.i2c: controller timed out
> 
> Is it indeed meant to solve the problem I have or is it addressing another
> issue?
> 
For your particular case, that will not help reaching the other segments as
we wait for a STOP before changing the target address. So, it should not
fail but do a write to segment 0 in your eeprom.

> I'm happy to help test patches.
> 
Can you enable debug in i2c-designware-master to see which transaction
is failing?

Best regards,
-- 
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 2/3] i2c: designware: Enable transfer with different target addresses

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 492 bytes --]


> For your particular case, that will not help reaching the other segments as
> we wait for a STOP before changing the target address. So, it should not
> fail but do a write to segment 0 in your eeprom.

So, this patch replaces a repeated start with a stop + start
combination? Please don't do this. It will give users a false impression
that proper repeated start is supported. Honestly reporting that the HW
does not support is the better option, so the user can decide what to do
then.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/3] i2c: designware: Support of controller with IC_EMPTYFIFO_HOLD_MASTER disabled

[Mika Westerberg]

Hi,

On Fri, Oct 17, 2025 at 04:59:34PM +0200, Benoît Monin wrote:
> If IC_EMPTYFIFO_HOLD_MASTER_EN parameter is 0, "Stop" and "Repeated
> Start" bits in command register doesn't exist, thus it is impossible to
> send several consecutive write messages in a single hardware batch. The
> existing implementation worked with such configuration incorrectly: all
> consequent write messages joined into a single message without any
> Start/Stop or Repeated Start conditions. For example, the following
> command:
> 
>     i2ctransfer -y 0 w1@0x55 0x00 w1@0x55 0x01
> 
> does the same as
> 
>     i2ctransfer -y 0 w2@0x55 0x00 0x01
> 
> To fix it, for the controllers that behave this way, if the next message
> to the same slave device has the same direction as the previous one, it
> is sent to the controller only after the previous message is sent and
> STOP_DET IRQ flag is raised by the controller.
> 
> This behavior is activated by compatible entries, because the state of
> the IC_EMPTYFIFO_HOLD_MASTER_EN parameter cannot be detected at runtime.
> Add the compatible entries of Mobileye SoCs needing the work-around and
> sort the entries alphabetically.
> 
> There is another possible problem with this controller configuration:
> When the CPU is putting commands to the FIFO, this process must not be
> interrupted because if FIFO buffer gets empty, the controller finishes
> the I2C transaction and generates STOP condition on the bus.
> 
> In a PREEMPT-RT kernel, interrupt handlers are by default executed in
> thread and may be interrupted, which can lead to breaking an I2C message
> by inserting an unwanted STOP.
> 
> To ensure proper operation on realtime kernel, use IRQF_NO_THREAD flag
> when requesting IRQ.

But even with that, it is still possible that something else is running
with local interrupt disabled so this may still happen, although likelyhood
is smaller.

You could use ACCESS_POLLING but that too can be preempted.

Perhaps best is to fail the transfer and let the caller retry?

> Based on the work of Dmitry Guzman <dmitry.guzman@mobileye.com>
> 
> Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
> ---
>  drivers/i2c/busses/i2c-designware-core.h    |  1 +
>  drivers/i2c/busses/i2c-designware-master.c  | 45 +++++++++++++++++++++--------
>  drivers/i2c/busses/i2c-designware-platdrv.c |  6 ++--
>  3 files changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> index 347843b4f5dd7..a31a8698e511a 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -311,6 +311,7 @@ struct dw_i2c_dev {
>  #define ACCESS_NO_IRQ_SUSPEND			BIT(1)
>  #define ARBITRATION_SEMAPHORE			BIT(2)
>  #define ACCESS_POLLING				BIT(3)
> +#define NO_EMPTYFIFO_HOLD_MASTER		BIT(4)
>  
>  #define MODEL_MSCC_OCELOT			BIT(8)
>  #define MODEL_BAIKAL_BT1			BIT(9)
> diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
> index f9a180b145da8..e5af0439ec832 100644
> --- a/drivers/i2c/busses/i2c-designware-master.c
> +++ b/drivers/i2c/busses/i2c-designware-master.c
> @@ -443,18 +443,6 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
>  	for (; dev->msg_write_idx < dev->msgs_num; dev->msg_write_idx++) {
>  		u32 flags = msgs[dev->msg_write_idx].flags;
>  
> -		/*
> -		 * If target address has changed, we need to
> -		 * reprogram the target address in the I2C
> -		 * adapter when we are done with this transfer.
> -		 * This can be done after STOP_DET IRQ flag is raised.
> -		 * So, disable "TX FIFO empty" interrupt.
> -		 */
> -		if (msgs[dev->msg_write_idx].addr != addr) {
> -			intr_mask &= ~DW_IC_INTR_TX_EMPTY;
> -			break;
> -		}
> -
>  		if (!(dev->status & STATUS_WRITE_IN_PROGRESS)) {
>  			/* new i2c_msg */
>  			buf = msgs[dev->msg_write_idx].buf;
> @@ -470,6 +458,25 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
>  				need_restart = true;
>  		}
>  
> +		/*
> +		 * If target address has changed, we need to
> +		 * reprogram the target address in the I2C
> +		 * adapter when we are done with this transfer.
> +		 * This can be done after STOP_DET IRQ flag is raised.
> +		 * So, disable "TX FIFO empty" interrupt.
> +		 * Also force a stop-then-start between two messages
> +		 * in the same direction if we need to restart on a
> +		 * adapter that does not handle restart.
> +		 */
> +		if (msgs[dev->msg_write_idx].addr != addr ||
> +		    ((need_restart &&
> +		     dev->flags & NO_EMPTYFIFO_HOLD_MASTER &&
> +		     ((msgs[dev->msg_write_idx].flags & I2C_M_RD) ==
> +		      (msgs[dev->msg_write_idx - 1].flags & I2C_M_RD))))) {
> +			intr_mask &= ~DW_IC_INTR_TX_EMPTY;
> +			break;

This, if we want to add it even, needs a helper function. The above is
really hard to parse ;-)

> +		}
> +
>  		regmap_read(dev->map, DW_IC_TXFLR, &flr);
>  		tx_limit = dev->tx_fifo_depth - flr;
>  
> @@ -1062,6 +1069,20 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
>  		irq_flags = IRQF_SHARED | IRQF_COND_SUSPEND;
>  	}
>  
> +	/*
> +	 * The first writing to TX FIFO buffer causes transmission start. If
> +	 * IC_EMPTYFIFO_HOLD_MASTER_EN is not set, when TX FIFO gets empty, I2C
> +	 * controller finishes the transaction. If writing to FIFO is
> +	 * interrupted, FIFO can get empty and the transaction will be finished
> +	 * prematurely. FIFO buffer is filled in IRQ handler, but in PREEMPT_RT
> +	 * kernel IRQ handler by default is executed in thread that can be
> +	 * preempted with another higher priority thread or an interrupt. So,
> +	 * IRQF_NO_THREAD flag is required in order to prevent any preemption
> +	 * during filling the FIFO buffer and possible data lost.
> +	 */
> +	if (dev->flags & NO_EMPTYFIFO_HOLD_MASTER)
> +		irq_flags |= IRQF_NO_THREAD;
> +
>  	ret = i2c_dw_acquire_lock(dev);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 34d881572351c..5e459175dcdb2 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -345,9 +345,11 @@ static void dw_i2c_plat_remove(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id dw_i2c_of_match[] = {
> -	{ .compatible = "snps,designware-i2c", },
> -	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
>  	{ .compatible = "baikal,bt1-sys-i2c", .data = (void *)MODEL_BAIKAL_BT1 },
> +	{ .compatible = "mobileye,eyeq6lplus-i2c", .data = (void *)NO_EMPTYFIFO_HOLD_MASTER },
> +	{ .compatible = "mobileye,eyeq7h-i2c", .data = (void *)NO_EMPTYFIFO_HOLD_MASTER },
> +	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
> +	{ .compatible = "snps,designware-i2c", },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
> 
> -- 
> 2.51.0

Re: [PATCH 2/3] i2c: designware: Enable transfer with different target addresses

[Benoît Monin]

On Monday, 20 October 2025 at 21:52:01 CEST, Wolfram Sang wrote:
> 
> > For your particular case, that will not help reaching the other segments as
> > we wait for a STOP before changing the target address. So, it should not
> > fail but do a write to segment 0 in your eeprom.
> 
> So, this patch replaces a repeated start with a stop + start
> combination? Please don't do this. It will give users a false impression
> that proper repeated start is supported. Honestly reporting that the HW
> does not support is the better option, so the user can decide what to do
> then.
> 
This patch replaces a -EINVAL in the middle of the transfer by a
STOP-then-START, but you are right, the expectation is to have a single
STOP at the end of a combined transfer. I somehow overlooked that part.
 
Maybe I could add support for the I2C_M_STOP flag instead? Or does an
adapter has to support all the protocol mangling if flagged with
I2C_FUNC_PROTOCOL_MANGLING?
 
That would still allow to group multiple accesses to device that support a
STOP in a transaction when done via i2c_dev I2C_RDWR ioctl, in a single-master
configuration.

Best regards,
-- 
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 2/3] i2c: designware: Enable transfer with different target addresses

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]

Hi Benoît,

> This patch replaces a -EINVAL in the middle of the transfer by a
> STOP-then-START, but you are right, the expectation is to have a single
> STOP at the end of a combined transfer. I somehow overlooked that part.

Yes. Sadly, the designware controllers are broken in that regard, it
seems :(

> Maybe I could add support for the I2C_M_STOP flag instead? Or does an
> adapter has to support all the protocol mangling if flagged with
> I2C_FUNC_PROTOCOL_MANGLING?

It doesn't. It is a known weakness of I2C_FUNC_PROTOCOL_MANGLING. Maybe
we could extrapolate it like we did for I2C_FUNC_NOSTART? Just an idea,
I haven't really looked into it.

> That would still allow to group multiple accesses to device that support a
> STOP in a transaction when done via i2c_dev I2C_RDWR ioctl, in a single-master
> configuration.

I'd think such transfers are super rare but if you still want to
implement it, please do. Note that some devices will still not work
because they reset their state machine on STOP. See Hans' example.

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]