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. 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 | 108 +++++++++++++++++++++--------
 1 file changed, 81 insertions(+), 27 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 12a1194bec52..2977c13c709c 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -435,7 +435,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;
@@ -446,18 +445,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;
@@ -805,20 +792,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 the number of messages transferred 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);
-
-	ACQUIRE(pm_runtime_active_auto_try, pm)(dev->dev);
-	if (ACQUIRE_ERR(pm_runtime_active_auto_try, &pm))
-		return -ENXIO;
-
 	reinit_completion(&dev->cmd_complete);
 	dev->msgs = msgs;
 	dev->msgs_num = num;
@@ -830,10 +812,6 @@ 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)
-		return ret;
-
 	ret = i2c_dw_wait_bus_not_busy(dev);
 	if (ret < 0)
 		goto done;
@@ -894,10 +872,83 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 
 	ret = -EIO;
 
+done:
+	return ret;
+}
+
+/*
+ * Check that the message pointed by @idx in the @msgs array
+ * respects the limitation of the controller.
+ */
+static inline 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 */
+	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);
+
+	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)
+		return ret;
+
+	/*
+	 * 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);
 
-	return ret;
+	if (ret < 0)
+		return ret;
+	return num;
 }
 
 static const struct i2c_algorithm i2c_dw_algo = {
@@ -920,6 +971,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.51.1

On Wed, Nov 19, 2025 at 04:05:35PM +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.
> 
> 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. 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.

...

>  	ret = -EIO;
>  
> +done:
> +	return ret;

I don't see the point of keeping this label anymore. Is there a followup
that addresses this?

> +static inline 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 */
> +	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;
> +}

Hmm... I would do it in the nested conditionals if nothing comes between
the lines in the future.

static inline bool
i2c_dw_msg_is_valid(struct dw_i2c_dev *dev, const struct i2c_msg *msgs, size_t idx)
{
	/*
	 * We cannot change the target address during a transaction, so make
	 * sure the address of the sequential messages is identical to the one
	 * of the previous message.
	 */
	if (idx && (msgs[idx - 1].addr != msgs[idx].addr) {
		dev_err(dev->dev, "invalid target address\n");
		return false;
	}

	/*
	 * Addresses match. For the first message return true as it sets the
	 * address for the whole transaction.
	 */
	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);
> +
> +	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)
> +		return ret;
> +
> +	/*
> +	 * 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;

What about

				break;

> +			}
> +
> +			if ((msgs_part[cnt - 1].flags & I2C_M_STOP) ||
> +			    (msgs_part + cnt == msgs + num))
> +				break;
> +		}
		if (ret < 0)
			break;

?

The motivation is to avoid mixing auto ptr with goto style
(it's not recommended as per cleanup.h).

> +		/* 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);
>  
> -	return ret;
> +	if (ret < 0)
> +		return ret;
> +	return num;
>  }

-- 
With Best Regards,
Andy Shevchenko