* [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer
@ 2026-01-20 9:28 Benoît Monin
2026-01-20 9:28 ` [PATCH v5 1/6] i2c: designware: Optimize flag reading in i2c_dw_read() Benoît Monin
` (6 more replies)
0 siblings, 7 replies; 18+ messages in thread
From: Benoît Monin @ 2026-01-20 9:28 UTC (permalink / raw)
To: Andi Shyti, Mika Westerberg, Andy Shevchenko, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt
Cc: Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel, Benoît Monin
Add support for the I2C_M_STOP flag to the .xfer() function of the
designware driver. This allows grouping multiple accesses in a single
call and changing the target address after a STOP flag. This is achieved
by splitting i2c_dw_xfer() in two functions. The core logic handling
the transaction is now in __i2c_dw_xfer_one_part(), while i2c_dw_xfer()
loops over the messages to search for the I2C_M_STOP flag and calls
__i2c_dw_xfer_one_part().
Handle controllers that lack the ability to emit a RESTART when two
consecutive messages have the same address and direction by aborting
transfers that contain such a sequence of messages. For those controllers,
we also check that we do not get any unwanted STOP caused by a Tx FIFO
underrun, as they lack the ability to hold the clock during a transaction.
And we set the irq as non-threaded to prevent underrun on PREEMPT-RT
kernel when filling the FIFO.
The I2C controllers found in the EyeQ6Lplus and EyeQ7H SoCs from Mobileye
lack such capability, so a compatible string is added because this cannot
be detected at runtime.
This patch series also brings four cleanups:
* Optimize the read of the message flags in i2c_dw_read().
* Sort the compatible strings alphabetically in dw_i2c_of_match[].
* Simplify runtime PM handling in i2c_dw_xfer() with guard.
* Add a dedicated i2c_algorithm for AMD NAVI controller.
Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
Changes in v5:
- Rebased on v6.19-rc6 and fully retested on EyeQ6Lplus.
- Drop dt-binding patch: already merged upstream in v6.19-rc2.
- From Andi Shyti's review:
- __i2c_dw_xfer_one_part() now returns 0 on success instead
of the number of messages transferred.
- Drop inline keyword from i2c_dw_msg_is_valid() declaration.
- Link to v4: https://lore.kernel.org/r/20251126-i2c-dw-v4-0-b0654598e7c5@bootlin.com
Changes in v4:
- Use a boolean property instead of a flag for handling controllers
IC_EMPTYFIFO_HOLD_MASTER_EN.
- Reorder the patches so goto and runtime PM auto-cleanup are not mixed
in i2c_dw_xfer().
- Drop goto in __i2c_dw_xfer_one_part() and use return.
- Tidy up comments for i2c_dw_msg_is_valid() and irq_flags.
- Link to v3: https://lore.kernel.org/r/20251119-i2c-dw-v3-0-bc4bc2a2cbac@bootlin.com
Changes in v3:
- Add a compatible entry for Mobileye EyeQ7H i2c controllers.
- Use runtime PM macro for auto-cleanup in i2c_dw_xfer().
- Add a dedicated i2c_algorithm for AMD NAVI controllers.
- Rename __i2c_dw_xfer_unlocked() as it is not about locking at all.
- Rework the validity check of i2c_msg.
- Flag the irq as non-threaded for controllers that cannot hold the
clock to avoid underrun.
- Link to v2: https://lore.kernel.org/r/20251031-i2c-dw-v2-0-90416874fcc0@bootlin.com
Changes in v2:
- Add a single compatible for mobileye i2c controllers based in
DesignWare IP.
- Handle the I2C_M_STOP flag instead of emitting a STOP on target address
change.
- Abort transfer when it requires a RESTART on controller that cannot
emit them.
- Detect FIFO underrun instead of disabling threaded interrupt on
PREEMPT_RT kernel.
- Sort the compatible entries in a separate patch.
- Add a cleanup patch on flag reading in i2c_dw_read().
- Link to v1: https://lore.kernel.org/r/20251017-i2c-dw-v1-0-7b85b71c7a87@bootlin.com
---
Benoît Monin (6):
i2c: designware: Optimize flag reading in i2c_dw_read()
i2c: designware: Sort compatible strings in alphabetical order
i2c: designware: Add dedicated algorithm for AMD NAVI
i2c: designware: Implement I2C_M_STOP support
i2c: designware: Use runtime PM macro for auto-cleanup
i2c: designware: Support of controller with IC_EMPTYFIFO_HOLD_MASTER disabled
drivers/i2c/busses/i2c-designware-common.c | 6 +
drivers/i2c/busses/i2c-designware-core.h | 3 +
drivers/i2c/busses/i2c-designware-master.c | 197 ++++++++++++++++++++--------
drivers/i2c/busses/i2c-designware-platdrv.c | 5 +-
4 files changed, 153 insertions(+), 58 deletions(-)
---
base-commit: 24d479d26b25bce5faea3ddd9fa8f3a6c3129ea7
change-id: 20251014-i2c-dw-da315f758296
Best regards,
--
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v5 1/6] i2c: designware: Optimize flag reading in i2c_dw_read()
2026-01-20 9:28 [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer Benoît Monin
@ 2026-01-20 9:28 ` Benoît Monin
2026-01-22 13:29 ` Andi Shyti
2026-01-20 9:28 ` [PATCH v5 2/6] i2c: designware: Sort compatible strings in alphabetical order Benoît Monin
` (5 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Benoît Monin @ 2026-01-20 9:28 UTC (permalink / raw)
To: Andi Shyti, Mika Westerberg, Andy Shevchenko, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt
Cc: Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel, Benoît Monin
Optimize the i2c_dw_read() function by reading the message flags only
once per message, rather than for every byte.
The message flags are accessed both in the outer loop and the inner loop,
so move the declaration of the local flags variable to the outer loop.
The message index is only modified by the outer loop, so reading the
flags in the inner loop was always getting the same value.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
drivers/i2c/busses/i2c-designware-master.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 45bfca05bb30..4493568e2fa3 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -593,11 +593,12 @@ i2c_dw_read(struct dw_i2c_dev *dev)
unsigned int rx_valid;
for (; dev->msg_read_idx < dev->msgs_num; dev->msg_read_idx++) {
+ u32 flags = msgs[dev->msg_read_idx].flags;
unsigned int tmp;
u32 len;
u8 *buf;
- if (!(msgs[dev->msg_read_idx].flags & I2C_M_RD))
+ if (!(flags & I2C_M_RD))
continue;
if (!(dev->status & STATUS_READ_IN_PROGRESS)) {
@@ -611,8 +612,6 @@ i2c_dw_read(struct dw_i2c_dev *dev)
regmap_read(dev->map, DW_IC_RXFLR, &rx_valid);
for (; len > 0 && rx_valid > 0; len--, rx_valid--) {
- u32 flags = msgs[dev->msg_read_idx].flags;
-
regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
tmp &= DW_IC_DATA_CMD_DAT;
/* Ensure length byte is a valid value */
--
2.52.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v5 2/6] i2c: designware: Sort compatible strings in alphabetical order
2026-01-20 9:28 [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer Benoît Monin
2026-01-20 9:28 ` [PATCH v5 1/6] i2c: designware: Optimize flag reading in i2c_dw_read() Benoît Monin
@ 2026-01-20 9:28 ` Benoît Monin
2026-01-22 13:30 ` Andi Shyti
2026-01-20 9:28 ` [PATCH v5 3/6] i2c: designware: Add dedicated algorithm for AMD NAVI Benoît Monin
` (4 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Benoît Monin @ 2026-01-20 9:28 UTC (permalink / raw)
To: Andi Shyti, Mika Westerberg, Andy Shevchenko, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt
Cc: Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel, Benoît Monin
Reorder the of_device_id structures so that they are in alphabetical
order. Also drop the unneeded inner trailing comma in the
"snps,designware-i2c" struct.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
drivers/i2c/busses/i2c-designware-platdrv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 7be99656a67d..077b34535ec7 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -334,9 +334,9 @@ 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 = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
+ { .compatible = "snps,designware-i2c" },
{}
};
MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
--
2.52.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v5 3/6] i2c: designware: Add dedicated algorithm for AMD NAVI
2026-01-20 9:28 [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer Benoît Monin
2026-01-20 9:28 ` [PATCH v5 1/6] i2c: designware: Optimize flag reading in i2c_dw_read() Benoît Monin
2026-01-20 9:28 ` [PATCH v5 2/6] i2c: designware: Sort compatible strings in alphabetical order Benoît Monin
@ 2026-01-20 9:28 ` Benoît Monin
2026-01-20 9:39 ` Andy Shevchenko
2026-01-20 9:28 ` [PATCH v5 4/6] i2c: designware: Implement I2C_M_STOP support Benoît Monin
` (3 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Benoît Monin @ 2026-01-20 9:28 UTC (permalink / raw)
To: Andi Shyti, Mika Westerberg, Andy Shevchenko, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt
Cc: Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel, Benoît Monin
Apart from runtime PM, there is nothing in common between i2c_dw_xfer()
and amd_i2c_dw_xfer_quirk(), so give AMD NAVI controller its own algorithm
instead of calling the quirk from i2c_dw_xfer().
Add runtime PM handling to amd_i2c_dw_xfer_quirk() and a dedicated
i2c_algorithm for AMD NAVI controllers. The adapter algorithm is set
during probe based on the device model.
This way we avoid checking for the device model at the start of every
transfer.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
drivers/i2c/busses/i2c-designware-master.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 4493568e2fa3..f247cf323207 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -361,6 +361,10 @@ static int amd_i2c_dw_xfer_quirk(struct i2c_adapter *adap, struct i2c_msg *msgs,
u8 *tx_buf;
unsigned int val;
+ ACQUIRE(pm_runtime_active_auto_try, pm)(dev->dev);
+ if (ACQUIRE_ERR(pm_runtime_active_auto_try, &pm))
+ return -ENXIO;
+
/*
* In order to enable the interrupt for UCSI i.e. AMD NAVI GPU card,
* it is mandatory to set the right value in specific register
@@ -820,14 +824,6 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
pm_runtime_get_sync(dev->dev);
- switch (dev->flags & MODEL_MASK) {
- case MODEL_AMD_NAVI_GPU:
- ret = amd_i2c_dw_xfer_quirk(adap, msgs, num);
- goto done_nolock;
- default:
- break;
- }
-
reinit_completion(&dev->cmd_complete);
dev->msgs = msgs;
dev->msgs_num = num;
@@ -917,6 +913,11 @@ static const struct i2c_algorithm i2c_dw_algo = {
.functionality = i2c_dw_func,
};
+static const struct i2c_algorithm amd_i2c_dw_algo = {
+ .xfer = amd_i2c_dw_xfer_quirk,
+ .functionality = i2c_dw_func,
+};
+
static const struct i2c_adapter_quirks i2c_dw_quirks = {
.flags = I2C_AQ_NO_ZERO_LEN,
};
@@ -1052,7 +1053,10 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
scnprintf(adap->name, sizeof(adap->name),
"Synopsys DesignWare I2C adapter");
adap->retries = 3;
- adap->algo = &i2c_dw_algo;
+ if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU)
+ adap->algo = &amd_i2c_dw_algo;
+ else
+ adap->algo = &i2c_dw_algo;
adap->quirks = &i2c_dw_quirks;
adap->dev.parent = dev->dev;
i2c_set_adapdata(adap, dev);
--
2.52.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v5 4/6] i2c: designware: Implement I2C_M_STOP support
2026-01-20 9:28 [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer Benoît Monin
` (2 preceding siblings ...)
2026-01-20 9:28 ` [PATCH v5 3/6] i2c: designware: Add dedicated algorithm for AMD NAVI Benoît Monin
@ 2026-01-20 9:28 ` Benoît Monin
2026-01-22 16:25 ` Andy Shevchenko
2026-01-20 9:28 ` [PATCH v5 5/6] i2c: designware: Use runtime PM macro for auto-cleanup Benoît Monin
` (2 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Benoît Monin @ 2026-01-20 9:28 UTC (permalink / raw)
To: Andi Shyti, Mika Westerberg, Andy Shevchenko, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt
Cc: Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel, Benoît Monin
Add the support of the I2C_M_STOP flag in i2c_msg by splitting
i2c_dw_xfer() in two: __i2c_dw_xfer_one_part() for the core transfer logic
and i2c_dw_xfer() for handling the high-level transaction management.
In detail __i2c_dw_xfer_one_part() starts a transaction and wait for its
completion, either with a STOP on the bus or an error. i2c_dw_xfer()
loops over the messages to search for the I2C_M_STOP flag and calls
__i2c_dw_xfer_one_part() for each part of the messages up to a STOP or
the end of the messages array.
i2c_dw_xfer() takes care of runtime PM and holds the hardware lock on
the bus while calling __i2c_dw_xfer_one_part(), this allows grouping
multiple accesses to device that support a STOP in a transaction when
done via i2c_dev I2C_RDWR ioctl.
Also, now that we have a lookup of the messages in i2c_dw_xfer() prior
to each transaction, we use it to make sure the messages are valid for
the transaction, via a new function i2c_dw_msg_is_valid(). We check
that the target address does not change before starting the transaction
instead of aborting the transfer while it is happening, as it was done
in i2c_dw_xfer_msg(). The target address can only be changed after an
I2C_M_STOP flag, i.e after a STOP on the i2c bus.
The I2C_FUNC_PROTOCOL_MANGLING flag is added to the list of
functionalities supported by the controller, except for the AMD NAVI
i2c controller which uses its own xfer() function and is left untouched.
Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
drivers/i2c/busses/i2c-designware-master.c | 130 ++++++++++++++++++++---------
1 file changed, 90 insertions(+), 40 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index f247cf323207..a0ff0e2db065 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -442,7 +442,6 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
struct i2c_msg *msgs = dev->msgs;
u32 intr_mask;
int tx_limit, rx_limit;
- u32 addr = msgs[dev->msg_write_idx].addr;
u32 buf_len = dev->tx_buf_len;
u8 *buf = dev->tx_buf;
bool need_restart = false;
@@ -453,18 +452,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.
- */
- if (msgs[dev->msg_write_idx].addr != addr) {
- dev_err(dev->dev,
- "%s: invalid target address\n", __func__);
- dev->msg_err = -EINVAL;
- break;
- }
-
if (!(dev->status & STATUS_WRITE_IN_PROGRESS)) {
/* new i2c_msg */
buf = msgs[dev->msg_write_idx].buf;
@@ -812,18 +799,15 @@ static int i2c_dw_wait_transfer(struct dw_i2c_dev *dev)
}
/*
- * Prepare controller for a transaction and call i2c_dw_xfer_msg.
+ * Prepare controller for a transaction, start the transfer of the msgs
+ * and wait for completion, either a STOP or a error.
+ * Return 0 or a negative error code.
*/
static int
-i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
+__i2c_dw_xfer_one_part(struct dw_i2c_dev *dev, struct i2c_msg *msgs, size_t num)
{
- struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
int ret;
- dev_dbg(dev->dev, "%s: msgs: %d\n", __func__, num);
-
- pm_runtime_get_sync(dev->dev);
-
reinit_completion(&dev->cmd_complete);
dev->msgs = msgs;
dev->msgs_num = num;
@@ -835,13 +819,9 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
dev->abort_source = 0;
dev->rx_outstanding = 0;
- ret = i2c_dw_acquire_lock(dev);
- if (ret)
- goto done_nolock;
-
ret = i2c_dw_wait_bus_not_busy(dev);
if (ret < 0)
- goto done;
+ return ret;
/* Start the transfers */
i2c_dw_xfer_init(dev);
@@ -853,7 +833,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
/* i2c_dw_init_master() implicitly disables the adapter */
i2c_recover_bus(&dev->adapter);
i2c_dw_init_master(dev);
- goto done;
+ return ret;
}
/*
@@ -876,28 +856,93 @@ 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;
- }
+ if (dev->msg_err)
+ return dev->msg_err;
/* No error */
- if (likely(!dev->cmd_err && !dev->status)) {
- ret = num;
- goto done;
- }
+ if (likely(!dev->cmd_err && !dev->status))
+ return 0;
/* We have an error */
- if (dev->cmd_err == DW_IC_ERR_TX_ABRT) {
- ret = i2c_dw_handle_tx_abort(dev);
- goto done;
- }
+ if (dev->cmd_err == DW_IC_ERR_TX_ABRT)
+ return i2c_dw_handle_tx_abort(dev);
if (dev->status)
dev_err(dev->dev,
"transfer terminated early - interrupt latency too high?\n");
- ret = -EIO;
+ return -EIO;
+}
+
+/*
+ * Verify that the message at index @idx can be processed as part
+ * of a single transaction. The @msgs array contains the messages
+ * of the transaction. The message is checked against its predecessor
+ * to ensure that it respects the limitation of the controller.
+ */
+static bool
+i2c_dw_msg_is_valid(struct dw_i2c_dev *dev, const struct i2c_msg *msgs, size_t idx)
+{
+ /*
+ * The first message of a transaction is valid,
+ * no constraint from a previous message.
+ */
+ if (!idx)
+ return true;
+
+ /*
+ * We cannot change the target address during a transaction, so make
+ * sure the address is identical to the one of the previous message.
+ */
+ if (msgs[idx - 1].addr != msgs[idx].addr) {
+ dev_err(dev->dev, "invalid target address\n");
+ return false;
+ }
+
+ return true;
+}
+
+static int
+i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+ struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
+ struct i2c_msg *msgs_part;
+ size_t cnt;
+ int ret;
+
+ dev_dbg(dev->dev, "msgs: %d\n", num);
+
+ pm_runtime_get_sync(dev->dev);
+
+ ret = i2c_dw_acquire_lock(dev);
+ if (ret)
+ goto done_nolock;
+
+ /*
+ * If the I2C_M_STOP is present in some the messages,
+ * we do one transaction for each part up to the STOP.
+ */
+ for (msgs_part = msgs; msgs_part < msgs + num; msgs_part += cnt) {
+ /*
+ * Count the messages in a transaction, up to a STOP
+ * or the end of the msgs.
+ */
+ for (cnt = 1; ; cnt++) {
+ if (!i2c_dw_msg_is_valid(dev, msgs_part, cnt - 1)) {
+ ret = -EINVAL;
+ goto done;
+ }
+
+ if ((msgs_part[cnt - 1].flags & I2C_M_STOP) ||
+ (msgs_part + cnt == msgs + num))
+ break;
+ }
+
+ /* transfer one part up to a STOP */
+ ret = __i2c_dw_xfer_one_part(dev, msgs_part, cnt);
+ if (ret < 0)
+ break;
+ }
done:
i2c_dw_release_lock(dev);
@@ -905,7 +950,9 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
done_nolock:
pm_runtime_put_autosuspend(dev->dev);
- return ret;
+ if (ret < 0)
+ return ret;
+ return num;
}
static const struct i2c_algorithm i2c_dw_algo = {
@@ -928,6 +975,9 @@ void i2c_dw_configure_master(struct dw_i2c_dev *dev)
dev->functionality = I2C_FUNC_10BIT_ADDR | DW_IC_DEFAULT_FUNCTIONALITY;
+ if ((dev->flags & MODEL_MASK) != MODEL_AMD_NAVI_GPU)
+ dev->functionality |= I2C_FUNC_PROTOCOL_MANGLING;
+
dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
DW_IC_CON_RESTART_EN;
--
2.52.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v5 5/6] i2c: designware: Use runtime PM macro for auto-cleanup
2026-01-20 9:28 [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer Benoît Monin
` (3 preceding siblings ...)
2026-01-20 9:28 ` [PATCH v5 4/6] i2c: designware: Implement I2C_M_STOP support Benoît Monin
@ 2026-01-20 9:28 ` Benoît Monin
2026-01-20 9:28 ` [PATCH v5 6/6] i2c: designware: Support of controller with IC_EMPTYFIFO_HOLD_MASTER disabled Benoît Monin
2026-01-20 10:01 ` [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer Mika Westerberg
6 siblings, 0 replies; 18+ messages in thread
From: Benoît Monin @ 2026-01-20 9:28 UTC (permalink / raw)
To: Andi Shyti, Mika Westerberg, Andy Shevchenko, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt
Cc: Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel, Benoît Monin
Simplify runtime PM handling in i2c_dw_xfer() by using the
pm_runtime_active_auto_try guard. This adds the proper handling for
runtime PM resume errors and allows us to get rid of the done and
done_nolock labels.
Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/i2c/busses/i2c-designware-master.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index a0ff0e2db065..ebebd8e0409c 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -912,11 +912,13 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
dev_dbg(dev->dev, "msgs: %d\n", num);
- pm_runtime_get_sync(dev->dev);
+ ACQUIRE(pm_runtime_active_auto_try, pm)(dev->dev);
+ if (ACQUIRE_ERR(pm_runtime_active_auto_try, &pm))
+ return -ENXIO;
ret = i2c_dw_acquire_lock(dev);
if (ret)
- goto done_nolock;
+ return ret;
/*
* If the I2C_M_STOP is present in some the messages,
@@ -930,13 +932,15 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
for (cnt = 1; ; cnt++) {
if (!i2c_dw_msg_is_valid(dev, msgs_part, cnt - 1)) {
ret = -EINVAL;
- goto done;
+ break;
}
if ((msgs_part[cnt - 1].flags & I2C_M_STOP) ||
(msgs_part + cnt == msgs + num))
break;
}
+ if (ret < 0)
+ break;
/* transfer one part up to a STOP */
ret = __i2c_dw_xfer_one_part(dev, msgs_part, cnt);
@@ -944,12 +948,8 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
break;
}
-done:
i2c_dw_release_lock(dev);
-done_nolock:
- pm_runtime_put_autosuspend(dev->dev);
-
if (ret < 0)
return ret;
return num;
--
2.52.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v5 6/6] i2c: designware: Support of controller with IC_EMPTYFIFO_HOLD_MASTER disabled
2026-01-20 9:28 [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer Benoît Monin
` (4 preceding siblings ...)
2026-01-20 9:28 ` [PATCH v5 5/6] i2c: designware: Use runtime PM macro for auto-cleanup Benoît Monin
@ 2026-01-20 9:28 ` Benoît Monin
2026-01-23 7:03 ` Andy Shevchenko
2026-01-20 10:01 ` [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer Mika Westerberg
6 siblings, 1 reply; 18+ messages in thread
From: Benoît Monin @ 2026-01-20 9:28 UTC (permalink / raw)
To: Andi Shyti, Mika Westerberg, Andy Shevchenko, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt
Cc: Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel, Benoît Monin
If IC_EMPTYFIFO_HOLD_MASTER_EN parameter is 0, "Stop" and "Repeated Start"
bits in command register does not 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 consecutive write messages are 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
In i2c_dw_msg_is_valid(), we ensure that we do not have such sequence
of messages requiring a RESTART, aborting the transfer on controller
that cannot emit them explicitly.
This behavior is activated by compatible entries because the state of
the IC_EMPTYFIFO_HOLD_MASTER_EN parameter cannot be detected at runtime.
The new flag emptyfifo_hold_master reflects the state of the parameter,
it is set to true for all controllers except those found in Mobileye
SoCs. For now, the controllers in Mobileye SoCs are the only ones known
to need the workaround. The behavior of the driver is left unmodified
for other controllers.
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.
If we continue writing the remainder of the message to the FIFO, the
controller will start emitting a new transaction with those data. This
turns a single a single message into multiple I2C transactions. To
protect against FIFO underrun, two changes are done:
First we flag the interrupt with IRQF_NO_THREAD, to prevent it from
running in a thread on PREEMPT-RT kernel. This ensures that we are not
interrupted when filling the FIFO as it is very time-senstive. For
example, being preempted after writing a single byte in the FIFO with
a 1MHz bus gives us only 18µs before an underrun.
Second in i2c_dw_process_transfer(), we abort if a STOP is detected
while a read or a write is in progress. This can occur when processing
a message larger than the FIFO. In that case the message is processed in
parts, and rely on the TX EMPTY interrupt to refill the FIFO when it gets
below a threshold. If servicing this interrupt is delayed for too long,
it can trigger a FIFO underrun, thus an unwanted STOP.
Originally-by: Dmitry Guzman <dmitry.guzman@mobileye.com>
Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
drivers/i2c/busses/i2c-designware-common.c | 6 ++++++
drivers/i2c/busses/i2c-designware-core.h | 3 +++
drivers/i2c/busses/i2c-designware-master.c | 32 +++++++++++++++++++++++++++++
drivers/i2c/busses/i2c-designware-platdrv.c | 1 +
4 files changed, 42 insertions(+)
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 5b1e8f74c4ac..446d567eafeb 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -388,6 +388,12 @@ int i2c_dw_fw_parse_and_configure(struct dw_i2c_dev *dev)
dev->clk_freq_optimized = device_property_read_bool(device, "snps,clk-freq-optimized");
+ /* Mobileye controllers do not hold the clock on empty FIFO */
+ if (device_is_compatible(device, "mobileye,eyeq6lplus-i2c"))
+ dev->emptyfifo_hold_master = false;
+ else
+ dev->emptyfifo_hold_master = true;
+
i2c_dw_adjust_bus_speed(dev);
if (is_of_node(fwnode))
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index bb5ce0a382f9..215b6161f242 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -247,6 +247,8 @@ struct reset_control;
* @clk_freq_optimized: if this value is true, it means the hardware reduces
* its internal clock frequency by reducing the internal latency required
* to generate the high period and low period of SCL line.
+ * @emptyfifo_hold_master: true if the controller acting as master holds
+ * the clock when the Tx FIFO is empty instead of emitting a stop.
*
* HCNT and LCNT parameters can be used if the platform knows more accurate
* values than the one computed based only on the input clock frequency.
@@ -306,6 +308,7 @@ struct dw_i2c_dev {
struct i2c_bus_recovery_info rinfo;
u32 bus_capacitance_pF;
bool clk_freq_optimized;
+ bool emptyfifo_hold_master;
};
#define ACCESS_INTR_MASK BIT(0)
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index ebebd8e0409c..ac4fbb6aa6ab 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -717,6 +717,14 @@ static void i2c_dw_process_transfer(struct dw_i2c_dev *dev, unsigned int stat)
if (stat & DW_IC_INTR_TX_EMPTY)
i2c_dw_xfer_msg(dev);
+ /* Abort if we detect a STOP in the middle of a read or a write */
+ if ((stat & DW_IC_INTR_STOP_DET) &&
+ (dev->status & (STATUS_READ_IN_PROGRESS | STATUS_WRITE_IN_PROGRESS))) {
+ dev_err(dev->dev, "spurious STOP detected\n");
+ dev->rx_outstanding = 0;
+ dev->msg_err = -EIO;
+ }
+
/*
* No need to modify or disable the interrupt mask here.
* i2c_dw_xfer_msg() will take care of it according to
@@ -899,6 +907,16 @@ i2c_dw_msg_is_valid(struct dw_i2c_dev *dev, const struct i2c_msg *msgs, size_t i
return false;
}
+ /*
+ * Make sure we don't need explicit RESTART between two messages
+ * in the same direction for controllers that cannot emit them.
+ */
+ if (!dev->emptyfifo_hold_master &&
+ (msgs[idx - 1].flags & I2C_M_RD) == (msgs[idx].flags & I2C_M_RD)) {
+ dev_err(dev->dev, "cannot emit RESTART\n");
+ return false;
+ }
+
return true;
}
@@ -1117,6 +1135,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 when filling the FIFO.
+ */
+ if (!dev->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 077b34535ec7..0fe6f10a14c0 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -335,6 +335,7 @@ static void dw_i2c_plat_remove(struct platform_device *pdev)
static const struct of_device_id dw_i2c_of_match[] = {
{ .compatible = "baikal,bt1-sys-i2c", .data = (void *)MODEL_BAIKAL_BT1 },
+ { .compatible = "mobileye,eyeq6lplus-i2c" },
{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
{ .compatible = "snps,designware-i2c" },
{}
--
2.52.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v5 3/6] i2c: designware: Add dedicated algorithm for AMD NAVI
2026-01-20 9:28 ` [PATCH v5 3/6] i2c: designware: Add dedicated algorithm for AMD NAVI Benoît Monin
@ 2026-01-20 9:39 ` Andy Shevchenko
2026-01-22 13:31 ` Andi Shyti
0 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2026-01-20 9:39 UTC (permalink / raw)
To: Benoît Monin
Cc: Andi Shyti, Mika Westerberg, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel
On Tue, Jan 20, 2026 at 10:28:03AM +0100, Benoît Monin wrote:
> Apart from runtime PM, there is nothing in common between i2c_dw_xfer()
> and amd_i2c_dw_xfer_quirk(), so give AMD NAVI controller its own algorithm
> instead of calling the quirk from i2c_dw_xfer().
>
> Add runtime PM handling to amd_i2c_dw_xfer_quirk() and a dedicated
> i2c_algorithm for AMD NAVI controllers. The adapter algorithm is set
> during probe based on the device model.
>
> This way we avoid checking for the device model at the start of every
> transfer.
...
> + ACQUIRE(pm_runtime_active_auto_try, pm)(dev->dev);
> + if (ACQUIRE_ERR(pm_runtime_active_auto_try, &pm))
> + return -ENXIO;
There are PM_ACQUIRE_*() macros available now.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer
2026-01-20 9:28 [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer Benoît Monin
` (5 preceding siblings ...)
2026-01-20 9:28 ` [PATCH v5 6/6] i2c: designware: Support of controller with IC_EMPTYFIFO_HOLD_MASTER disabled Benoît Monin
@ 2026-01-20 10:01 ` Mika Westerberg
2026-01-20 15:17 ` Benoît Monin
6 siblings, 1 reply; 18+ messages in thread
From: Mika Westerberg @ 2026-01-20 10:01 UTC (permalink / raw)
To: Benoît Monin
Cc: Andi Shyti, Andy Shevchenko, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel
On Tue, Jan 20, 2026 at 10:28:00AM +0100, Benoît Monin wrote:
> Add support for the I2C_M_STOP flag to the .xfer() function of the
> designware driver. This allows grouping multiple accesses in a single
> call and changing the target address after a STOP flag. This is achieved
> by splitting i2c_dw_xfer() in two functions. The core logic handling
> the transaction is now in __i2c_dw_xfer_one_part(), while i2c_dw_xfer()
> loops over the messages to search for the I2C_M_STOP flag and calls
> __i2c_dw_xfer_one_part().
>
> Handle controllers that lack the ability to emit a RESTART when two
> consecutive messages have the same address and direction by aborting
> transfers that contain such a sequence of messages. For those controllers,
> we also check that we do not get any unwanted STOP caused by a Tx FIFO
> underrun, as they lack the ability to hold the clock during a transaction.
> And we set the irq as non-threaded to prevent underrun on PREEMPT-RT
> kernel when filling the FIFO.
>
> The I2C controllers found in the EyeQ6Lplus and EyeQ7H SoCs from Mobileye
> lack such capability, so a compatible string is added because this cannot
> be detected at runtime.
>
> This patch series also brings four cleanups:
> * Optimize the read of the message flags in i2c_dw_read().
> * Sort the compatible strings alphabetically in dw_i2c_of_match[].
> * Simplify runtime PM handling in i2c_dw_xfer() with guard.
> * Add a dedicated i2c_algorithm for AMD NAVI controller.
>
> Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
> ---
> Changes in v5:
> - Rebased on v6.19-rc6 and fully retested on EyeQ6Lplus.
> - Drop dt-binding patch: already merged upstream in v6.19-rc2.
> - From Andi Shyti's review:
> - __i2c_dw_xfer_one_part() now returns 0 on success instead
> of the number of messages transferred.
> - Drop inline keyword from i2c_dw_msg_is_valid() declaration.
> - Link to v4: https://lore.kernel.org/r/20251126-i2c-dw-v4-0-b0654598e7c5@bootlin.com
Why you dropped my ack?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer
2026-01-20 10:01 ` [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer Mika Westerberg
@ 2026-01-20 15:17 ` Benoît Monin
2026-01-22 13:33 ` Andi Shyti
0 siblings, 1 reply; 18+ messages in thread
From: Benoît Monin @ 2026-01-20 15:17 UTC (permalink / raw)
To: Mika Westerberg
Cc: Andi Shyti, Andy Shevchenko, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel
On Tuesday, 20 January 2026 at 11:01:29 CET, Mika Westerberg wrote:
> On Tue, Jan 20, 2026 at 10:28:00AM +0100, Benoît Monin wrote:
[...]
> > Changes in v5:
> > - Rebased on v6.19-rc6 and fully retested on EyeQ6Lplus.
> > - Drop dt-binding patch: already merged upstream in v6.19-rc2.
> > - From Andi Shyti's review:
> > - __i2c_dw_xfer_one_part() now returns 0 on success instead
> > of the number of messages transferred.
> > - Drop inline keyword from i2c_dw_msg_is_valid() declaration.
> > - Link to v4: https://lore.kernel.org/r/20251126-i2c-dw-v4-0-b0654598e7c5@bootlin.com
>
> Why you dropped my ack?
>
Sorry about that! I somehow forgot about it and b4 did not catch it either.
@Andi: Should I resend the series with Mika's acked-by or you handle it on
your side?
Thanks,
--
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 1/6] i2c: designware: Optimize flag reading in i2c_dw_read()
2026-01-20 9:28 ` [PATCH v5 1/6] i2c: designware: Optimize flag reading in i2c_dw_read() Benoît Monin
@ 2026-01-22 13:29 ` Andi Shyti
0 siblings, 0 replies; 18+ messages in thread
From: Andi Shyti @ 2026-01-22 13:29 UTC (permalink / raw)
To: Benoît Monin
Cc: Mika Westerberg, Andy Shevchenko, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel
Hi Benoit,
On Tue, Jan 20, 2026 at 10:28:01AM +0100, Benoît Monin wrote:
> Optimize the i2c_dw_read() function by reading the message flags only
> once per message, rather than for every byte.
>
> The message flags are accessed both in the outer loop and the inner loop,
> so move the declaration of the local flags variable to the outer loop.
>
> The message index is only modified by the outer loop, so reading the
> flags in the inner loop was always getting the same value.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
This patch has already been applied. If you are going to respin,
please take it off.
Andi
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 2/6] i2c: designware: Sort compatible strings in alphabetical order
2026-01-20 9:28 ` [PATCH v5 2/6] i2c: designware: Sort compatible strings in alphabetical order Benoît Monin
@ 2026-01-22 13:30 ` Andi Shyti
0 siblings, 0 replies; 18+ messages in thread
From: Andi Shyti @ 2026-01-22 13:30 UTC (permalink / raw)
To: Benoît Monin
Cc: Mika Westerberg, Andy Shevchenko, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel
Hi Benoit,
On Tue, Jan 20, 2026 at 10:28:02AM +0100, Benoît Monin wrote:
> Reorder the of_device_id structures so that they are in alphabetical
> order. Also drop the unneeded inner trailing comma in the
> "snps,designware-i2c" struct.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
This patch has already been applied, if you are going to respin,
please take it off.
Thanks,
Andi
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 3/6] i2c: designware: Add dedicated algorithm for AMD NAVI
2026-01-20 9:39 ` Andy Shevchenko
@ 2026-01-22 13:31 ` Andi Shyti
0 siblings, 0 replies; 18+ messages in thread
From: Andi Shyti @ 2026-01-22 13:31 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Benoît Monin, Mika Westerberg, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel
Hi Benoit,
On Tue, Jan 20, 2026 at 11:39:46AM +0200, Andy Shevchenko wrote:
> On Tue, Jan 20, 2026 at 10:28:03AM +0100, Benoît Monin wrote:
> > Apart from runtime PM, there is nothing in common between i2c_dw_xfer()
> > and amd_i2c_dw_xfer_quirk(), so give AMD NAVI controller its own algorithm
> > instead of calling the quirk from i2c_dw_xfer().
> >
> > Add runtime PM handling to amd_i2c_dw_xfer_quirk() and a dedicated
> > i2c_algorithm for AMD NAVI controllers. The adapter algorithm is set
> > during probe based on the device model.
> >
> > This way we avoid checking for the device model at the start of every
> > transfer.
>
> ...
>
> > + ACQUIRE(pm_runtime_active_auto_try, pm)(dev->dev);
> > + if (ACQUIRE_ERR(pm_runtime_active_auto_try, &pm))
> > + return -ENXIO;
>
> There are PM_ACQUIRE_*() macros available now.
I already applied this patch. You could eventually send a new one
using the PM_ACQUIRE_*() as suggested by Andy.
Thanks,
Andi
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer
2026-01-20 15:17 ` Benoît Monin
@ 2026-01-22 13:33 ` Andi Shyti
0 siblings, 0 replies; 18+ messages in thread
From: Andi Shyti @ 2026-01-22 13:33 UTC (permalink / raw)
To: Benoît Monin
Cc: Mika Westerberg, Andy Shevchenko, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel
Hi Benoit,
On Tue, Jan 20, 2026 at 04:17:46PM +0100, Benoît Monin wrote:
> On Tuesday, 20 January 2026 at 11:01:29 CET, Mika Westerberg wrote:
> > On Tue, Jan 20, 2026 at 10:28:00AM +0100, Benoît Monin wrote:
> [...]
> > > Changes in v5:
> > > - Rebased on v6.19-rc6 and fully retested on EyeQ6Lplus.
> > > - Drop dt-binding patch: already merged upstream in v6.19-rc2.
> > > - From Andi Shyti's review:
> > > - __i2c_dw_xfer_one_part() now returns 0 on success instead
> > > of the number of messages transferred.
> > > - Drop inline keyword from i2c_dw_msg_is_valid() declaration.
> > > - Link to v4: https://lore.kernel.org/r/20251126-i2c-dw-v4-0-b0654598e7c5@bootlin.com
> >
> > Why you dropped my ack?
> >
> Sorry about that! I somehow forgot about it and b4 did not catch it either.
>
> @Andi: Should I resend the series with Mika's acked-by or you handle it on
> your side?
The ones I have applied so far have Mika's ack. If I write:
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
b4 will take it automatically for all the remaining patches :-)
No need to resend unless required by other reasons.
Thanks,
Andi
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 4/6] i2c: designware: Implement I2C_M_STOP support
2026-01-20 9:28 ` [PATCH v5 4/6] i2c: designware: Implement I2C_M_STOP support Benoît Monin
@ 2026-01-22 16:25 ` Andy Shevchenko
2026-01-23 14:25 ` Benoît Monin
0 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2026-01-22 16:25 UTC (permalink / raw)
To: Benoît Monin
Cc: Andi Shyti, Mika Westerberg, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel
On Tue, Jan 20, 2026 at 10:28:04AM +0100, Benoît Monin wrote:
> Add the support of the I2C_M_STOP flag in i2c_msg by splitting
> i2c_dw_xfer() in two: __i2c_dw_xfer_one_part() for the core transfer logic
> and i2c_dw_xfer() for handling the high-level transaction management.
>
> In detail __i2c_dw_xfer_one_part() starts a transaction and wait for its
> completion, either with a STOP on the bus or an error. i2c_dw_xfer()
> loops over the messages to search for the I2C_M_STOP flag and calls
> __i2c_dw_xfer_one_part() for each part of the messages up to a STOP or
> the end of the messages array.
>
> i2c_dw_xfer() takes care of runtime PM and holds the hardware lock on
> the bus while calling __i2c_dw_xfer_one_part(), this allows grouping
> multiple accesses to device that support a STOP in a transaction when
> done via i2c_dev I2C_RDWR ioctl.
Does i2c-tools support this. I.o.w. can you put an example of user space call
to achieve the above?
> Also, now that we have a lookup of the messages in i2c_dw_xfer() prior
> to each transaction, we use it to make sure the messages are valid for
> the transaction, via a new function i2c_dw_msg_is_valid(). We check
> that the target address does not change before starting the transaction
> instead of aborting the transfer while it is happening, as it was done
> in i2c_dw_xfer_msg(). The target address can only be changed after an
> I2C_M_STOP flag, i.e after a STOP on the i2c bus.
>
> The I2C_FUNC_PROTOCOL_MANGLING flag is added to the list of
> functionalities supported by the controller, except for the AMD NAVI
> i2c controller which uses its own xfer() function and is left untouched.
...
While the below is not a kernel-doc, I would still use a couple of things to
make it a bit aligned with that.
> /*
> - * Prepare controller for a transaction and call i2c_dw_xfer_msg.
> + * Prepare controller for a transaction, start the transfer of the msgs
@msgs
> + * and wait for completion, either a STOP or a error.
> + * Return 0 or a negative error code.
Return:
> */
...
> +/*
> + * Verify that the message at index @idx can be processed as part
> + * of a single transaction. The @msgs array contains the messages
> + * of the transaction. The message is checked against its predecessor
> + * to ensure that it respects the limitation of the controller.
Ha, you even used @ notation here!
Perhaps also add Return: line?
> + */
...
> + /*
> + * The first message of a transaction is valid,
> + * no constraint from a previous message.
constraints ?
> + */
...
> +static int
> +i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> +{
> + struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
> + struct i2c_msg *msgs_part;
> + size_t cnt;
> + int ret;
> + dev_dbg(dev->dev, "msgs: %d\n", num);
> +
> + pm_runtime_get_sync(dev->dev);
> +
> + ret = i2c_dw_acquire_lock(dev);
> + if (ret)
> + goto done_nolock;
A side note: Perhaps make sense to factor the body out to another helper.
> + /*
> + * If the I2C_M_STOP is present in some the messages,
> + * we do one transaction for each part up to the STOP.
> + */
> + for (msgs_part = msgs; msgs_part < msgs + num; msgs_part += cnt) {
> + /*
> + * Count the messages in a transaction, up to a STOP
> + * or the end of the msgs.
> + */
> + for (cnt = 1; ; cnt++) {
Do we have a guarantee that this doesn't become an infinite loop or more
precisely out-of-boundary access?
I hope to see this be explicitly mentioned in the comment on top of for-loop.
> + if (!i2c_dw_msg_is_valid(dev, msgs_part, cnt - 1)) {
> + ret = -EINVAL;
> + goto done;
> + }
> +
> + if ((msgs_part[cnt - 1].flags & I2C_M_STOP) ||
> + (msgs_part + cnt == msgs + num))
> + break;
> + }
> +
> + /* transfer one part up to a STOP */
> + ret = __i2c_dw_xfer_one_part(dev, msgs_part, cnt);
> + if (ret < 0)
> + break;
> + }
>
> done:
> i2c_dw_release_lock(dev);
> done_nolock:
> pm_runtime_put_autosuspend(dev->dev);
>
> - return ret;
> + if (ret < 0)
> + return ret;
> + return num;
> }
...
> + if ((dev->flags & MODEL_MASK) != MODEL_AMD_NAVI_GPU)
> + dev->functionality |= I2C_FUNC_PROTOCOL_MANGLING;
Why do we need this flag? I mean can't we use the fact that the code for that
platform uses custom xfer implementation?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 6/6] i2c: designware: Support of controller with IC_EMPTYFIFO_HOLD_MASTER disabled
2026-01-20 9:28 ` [PATCH v5 6/6] i2c: designware: Support of controller with IC_EMPTYFIFO_HOLD_MASTER disabled Benoît Monin
@ 2026-01-23 7:03 ` Andy Shevchenko
0 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2026-01-23 7:03 UTC (permalink / raw)
To: Benoît Monin
Cc: Andi Shyti, Mika Westerberg, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel
On Tue, Jan 20, 2026 at 10:28:06AM +0100, Benoît Monin wrote:
> If IC_EMPTYFIFO_HOLD_MASTER_EN parameter is 0, "Stop" and "Repeated Start"
> bits in command register does not exist, thus it is impossible to send
do not exist
> several consecutive write messages in a single hardware batch. The
> existing implementation worked with such configuration incorrectly:
> all consecutive write messages are 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
>
> In i2c_dw_msg_is_valid(), we ensure that we do not have such sequence
> of messages requiring a RESTART, aborting the transfer on controller
> that cannot emit them explicitly.
>
> This behavior is activated by compatible entries because the state of
> the IC_EMPTYFIFO_HOLD_MASTER_EN parameter cannot be detected at runtime.
> The new flag emptyfifo_hold_master reflects the state of the parameter,
> it is set to true for all controllers except those found in Mobileye
> SoCs. For now, the controllers in Mobileye SoCs are the only ones known
> to need the workaround. The behavior of the driver is left unmodified
> for other controllers.
>
> 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.
>
> If we continue writing the remainder of the message to the FIFO, the
> controller will start emitting a new transaction with those data. This
> turns a single a single message into multiple I2C transactions. To
a single a single ?
> protect against FIFO underrun, two changes are done:
>
> First we flag the interrupt with IRQF_NO_THREAD, to prevent it from
> running in a thread on PREEMPT-RT kernel. This ensures that we are not
> interrupted when filling the FIFO as it is very time-senstive. For
> example, being preempted after writing a single byte in the FIFO with
> a 1MHz bus gives us only 18µs before an underrun.
>
> Second in i2c_dw_process_transfer(), we abort if a STOP is detected
> while a read or a write is in progress. This can occur when processing
> a message larger than the FIFO. In that case the message is processed in
> parts, and rely on the TX EMPTY interrupt to refill the FIFO when it gets
> below a threshold. If servicing this interrupt is delayed for too long,
> it can trigger a FIFO underrun, thus an unwanted STOP.
Can DMA enablement fix this issue?
...
> static const struct of_device_id dw_i2c_of_match[] = {
> { .compatible = "baikal,bt1-sys-i2c", .data = (void *)MODEL_BAIKAL_BT1 },
You need to rebase on top of the latest tip of i2c-host tree.
> + { .compatible = "mobileye,eyeq6lplus-i2c" },
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 4/6] i2c: designware: Implement I2C_M_STOP support
2026-01-22 16:25 ` Andy Shevchenko
@ 2026-01-23 14:25 ` Benoît Monin
2026-01-23 14:37 ` Andy Shevchenko
0 siblings, 1 reply; 18+ messages in thread
From: Benoît Monin @ 2026-01-23 14:25 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andi Shyti, Mika Westerberg, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel
On Thursday, 22 January 2026 at 17:25:58 CET, Andy Shevchenko wrote:
> On Tue, Jan 20, 2026 at 10:28:04AM +0100, Benoît Monin wrote:
> > Add the support of the I2C_M_STOP flag in i2c_msg by splitting
> > i2c_dw_xfer() in two: __i2c_dw_xfer_one_part() for the core transfer logic
> > and i2c_dw_xfer() for handling the high-level transaction management.
> >
> > In detail __i2c_dw_xfer_one_part() starts a transaction and wait for its
> > completion, either with a STOP on the bus or an error. i2c_dw_xfer()
> > loops over the messages to search for the I2C_M_STOP flag and calls
> > __i2c_dw_xfer_one_part() for each part of the messages up to a STOP or
> > the end of the messages array.
> >
> > i2c_dw_xfer() takes care of runtime PM and holds the hardware lock on
> > the bus while calling __i2c_dw_xfer_one_part(), this allows grouping
> > multiple accesses to device that support a STOP in a transaction when
> > done via i2c_dev I2C_RDWR ioctl.
>
> Does i2c-tools support this. I.o.w. can you put an example of user space call
> to achieve the above?
>
I posted a patch series for i2c-tools, adding support for message modifier
flags including I2C_M_STOP to i2ctransfer:
https://lore.kernel.org/all/20251223-msg-flags-v2-0-8d934a4366e2@bootlin.com/
I am working on version 3, following Jean and Wolfram's reviews.
[...]
Thanks for the review!
--
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 4/6] i2c: designware: Implement I2C_M_STOP support
2026-01-23 14:25 ` Benoît Monin
@ 2026-01-23 14:37 ` Andy Shevchenko
0 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2026-01-23 14:37 UTC (permalink / raw)
To: Benoît Monin
Cc: Andi Shyti, Mika Westerberg, Jan Dabros,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Thomas Petazzoni, Gregory CLEMENT, Théo Lebrun,
Tawfik Bayouk, Vladimir Kondratiev, Dmitry Guzman, linux-i2c,
linux-kernel, linux-rt-devel
On Fri, Jan 23, 2026 at 03:25:24PM +0100, Benoît Monin wrote:
> On Thursday, 22 January 2026 at 17:25:58 CET, Andy Shevchenko wrote:
> > On Tue, Jan 20, 2026 at 10:28:04AM +0100, Benoît Monin wrote:
> > > Add the support of the I2C_M_STOP flag in i2c_msg by splitting
> > > i2c_dw_xfer() in two: __i2c_dw_xfer_one_part() for the core transfer logic
> > > and i2c_dw_xfer() for handling the high-level transaction management.
> > >
> > > In detail __i2c_dw_xfer_one_part() starts a transaction and wait for its
> > > completion, either with a STOP on the bus or an error. i2c_dw_xfer()
> > > loops over the messages to search for the I2C_M_STOP flag and calls
> > > __i2c_dw_xfer_one_part() for each part of the messages up to a STOP or
> > > the end of the messages array.
> > >
> > > i2c_dw_xfer() takes care of runtime PM and holds the hardware lock on
> > > the bus while calling __i2c_dw_xfer_one_part(), this allows grouping
> > > multiple accesses to device that support a STOP in a transaction when
> > > done via i2c_dev I2C_RDWR ioctl.
> >
> > Does i2c-tools support this. I.o.w. can you put an example of user space call
> > to achieve the above?
> >
> I posted a patch series for i2c-tools, adding support for message modifier
> flags including I2C_M_STOP to i2ctransfer:
> https://lore.kernel.org/all/20251223-msg-flags-v2-0-8d934a4366e2@bootlin.com/
>
> I am working on version 3, following Jean and Wolfram's reviews.
Thanks, it would be nice to have this being mentioned in the commit message
that the user space tools are going to support this in near future.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-01-23 14:37 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-20 9:28 [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer Benoît Monin
2026-01-20 9:28 ` [PATCH v5 1/6] i2c: designware: Optimize flag reading in i2c_dw_read() Benoît Monin
2026-01-22 13:29 ` Andi Shyti
2026-01-20 9:28 ` [PATCH v5 2/6] i2c: designware: Sort compatible strings in alphabetical order Benoît Monin
2026-01-22 13:30 ` Andi Shyti
2026-01-20 9:28 ` [PATCH v5 3/6] i2c: designware: Add dedicated algorithm for AMD NAVI Benoît Monin
2026-01-20 9:39 ` Andy Shevchenko
2026-01-22 13:31 ` Andi Shyti
2026-01-20 9:28 ` [PATCH v5 4/6] i2c: designware: Implement I2C_M_STOP support Benoît Monin
2026-01-22 16:25 ` Andy Shevchenko
2026-01-23 14:25 ` Benoît Monin
2026-01-23 14:37 ` Andy Shevchenko
2026-01-20 9:28 ` [PATCH v5 5/6] i2c: designware: Use runtime PM macro for auto-cleanup Benoît Monin
2026-01-20 9:28 ` [PATCH v5 6/6] i2c: designware: Support of controller with IC_EMPTYFIFO_HOLD_MASTER disabled Benoît Monin
2026-01-23 7:03 ` Andy Shevchenko
2026-01-20 10:01 ` [PATCH v5 0/6] i2c: designware: Improve support of multi-messages transfer Mika Westerberg
2026-01-20 15:17 ` Benoît Monin
2026-01-22 13:33 ` Andi Shyti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox