public inbox for linux-i3c@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Fix paths unexpectedly returning Mx error codes
@ 2026-03-08 16:47 Jorge Marques
  2026-03-08 16:47 ` [PATCH 1/5] i3c: master: Move rstdaa error suppression Jorge Marques
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Jorge Marques @ 2026-03-08 16:47 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Przemysław Gaj
  Cc: linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron,
	Jorge Marques

A smatch warning on the iio/adc/ad4062.c driver raised that many i3c
methods that documented returning "0 on success or negative error code"
but actually propagate positive Mx error codes (I3C_ERROR_M0=1, M1=2,
M2=3) from i3c_master_send_ccc_cmd_locked().

Close paths returning positive Mx error codes when 0 for success or
negative error code otherwise are explicitly stated, ambiguous or
expected.

If any Mx error code is present, for each controller:
- adi: returns -EIO
- cdns: returns -EIO
- dw:
  - RESPONSE_ERROR_IBA_NACK -> I3C_ERROR_M2 : returns -EIO
  - RESPONSE_ERROR_ADDRESS_NACK : returns -EINVAL
- renesas :
  - NRSPQP_ERROR_IBA_NACK : returns -EIO
  - NRSPQP_ERROR_ADDRESS_NACK : returns -EINVAL
- svc : Unclear ret value, but cmd->err = I3C_ERROR_M2 for any ret

Each i3c_master_send_ccc_cmd_locked caller handles cmd->err directly.
There are three exceptions all related to the bus initialization:
* RSTDAA -> i3c_master_rstdaa_locked
* ENTDAA -> i3c_master_entdaa_locked
* DISEC (broadcast address only) -> i3c_master_enec_disec_locked

For direct enable ibi, disable ibi to a target, error code M2 should not
be suppressed, the device must acknowledge it.

The patch series start with moving the error check suppressions, then
does the actual change in i3c_master_send_ccc_cmd_locked to return
0 on success or negative error code otherwise (drops the positive Mx
error codes). Then ends with returning the xfer->err at
adi_i3c_master_end_xfer_locked.

The series was tested with adi-i3c-master.c and iio/adc/ad4062.c.

Link: https://lore.kernel.org/linux-iio/aYXvT5FW0hXQwhm_@stanley.mountain/

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
---
Jorge Marques (5):
      i3c: master: Move rstdaa error suppression
      i3c: master: Move entdaa error suppression
      i3c: master: Move bus_init error suppression
      i3c: master: Negative error codes at send_ccc_cmd
      i3c: master: adi: Return xfer->ret at send CCC

 drivers/i3c/master.c                 | 62 +++++++++++++++++++++---------------
 drivers/i3c/master/adi-i3c-master.c  |  5 ++-
 drivers/i3c/master/i3c-master-cdns.c |  2 +-
 3 files changed, 39 insertions(+), 30 deletions(-)
---
base-commit: df8d8e0a9edaec6f1083870f8fb6724e9c663508
change-id: 20260305-ad4062-positive-error-fix-dc833cd2f088

Best regards,
-- 
Jorge Marques <jorge.marques@analog.com>


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* [PATCH 1/5] i3c: master: Move rstdaa error suppression
  2026-03-08 16:47 [PATCH 0/5] Fix paths unexpectedly returning Mx error codes Jorge Marques
@ 2026-03-08 16:47 ` Jorge Marques
  2026-03-09 11:39   ` Adrian Hunter
  2026-03-10 19:13   ` Adrian Hunter
  2026-03-08 16:47 ` [PATCH 2/5] i3c: master: Move entdaa " Jorge Marques
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 17+ messages in thread
From: Jorge Marques @ 2026-03-08 16:47 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Przemysław Gaj
  Cc: linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron,
	Jorge Marques

The CCC RSTDAA is invoked with i3c_master_rstdaa_locked
even if there are no devices active on the bus, resulting
in error I3C_ERROR_M2. Handle inside i3c_master_rstdaa_locked,
checking cmd->err directly.

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
---
 drivers/i3c/master.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 9e6be49bebb2..31822fd5ffde 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1016,6 +1016,10 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
 	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
 	i3c_ccc_cmd_dest_cleanup(&dest);
 
+	/* No active devices on the bus. */
+	if (ret && cmd.err == I3C_ERROR_M2)
+		ret = 0;
+
 	return ret;
 }
 
@@ -1785,7 +1789,6 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
  */
 int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
 {
-	int rstret = 0;
 	int ret;
 
 	ret = i3c_master_rpm_get(master);
@@ -1795,9 +1798,9 @@ int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
 	i3c_bus_maintenance_lock(&master->bus);
 
 	if (rstdaa) {
-		rstret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
-		if (rstret == I3C_ERROR_M2)
-			rstret = 0;
+		ret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
+		if (ret)
+			goto out;
 	}
 
 	ret = master->ops->do_daa(master);
@@ -1813,7 +1816,7 @@ int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
 out:
 	i3c_master_rpm_put(master);
 
-	return rstret ?: ret;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(i3c_master_do_daa_ext);
 
@@ -2093,7 +2096,7 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
 	 * (assigned by the bootloader for example).
 	 */
 	ret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
-	if (ret && ret != I3C_ERROR_M2)
+	if (ret)
 		goto err_bus_cleanup;
 
 	if (master->ops->set_speed) {

-- 
2.51.1


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* [PATCH 2/5] i3c: master: Move entdaa error suppression
  2026-03-08 16:47 [PATCH 0/5] Fix paths unexpectedly returning Mx error codes Jorge Marques
  2026-03-08 16:47 ` [PATCH 1/5] i3c: master: Move rstdaa error suppression Jorge Marques
@ 2026-03-08 16:47 ` Jorge Marques
  2026-03-10 19:13   ` Adrian Hunter
  2026-03-08 16:47 ` [PATCH 3/5] i3c: master: Move bus_init " Jorge Marques
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Jorge Marques @ 2026-03-08 16:47 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Przemysław Gaj
  Cc: linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron,
	Jorge Marques

The CCC ENTDAA is invoked with i3c_master_entdaa_locked and yields
error I3C_ERROR_M2 if there are no devices active on the bus.
Some controllers may also yield if there are no more devices need an
dynamic address, since the sequence do always end in a NACK.

Handle inside i3c_master_entdaa_locked, checking cmd->err directly.

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
---
 drivers/i3c/master.c                 | 7 +++++--
 drivers/i3c/master/adi-i3c-master.c  | 3 +--
 drivers/i3c/master/i3c-master-cdns.c | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 31822fd5ffde..ce1898345810 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1036,8 +1036,7 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
  *
  * This function must be called with the bus lock held in write mode.
  *
- * Return: 0 in case of success, a positive I3C error code if the error is
- * one of the official Mx error codes, and a negative error code otherwise.
+ * Return: 0 in case of success, or a negative error code otherwise.
  */
 int i3c_master_entdaa_locked(struct i3c_master_controller *master)
 {
@@ -1050,6 +1049,10 @@ int i3c_master_entdaa_locked(struct i3c_master_controller *master)
 	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
 	i3c_ccc_cmd_dest_cleanup(&dest);
 
+	/* No active devices need an address. */
+	if (ret && cmd.err == I3C_ERROR_M2)
+		ret = 0;
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(i3c_master_entdaa_locked);
diff --git a/drivers/i3c/master/adi-i3c-master.c b/drivers/i3c/master/adi-i3c-master.c
index 6616f751075a..fb9a48830446 100644
--- a/drivers/i3c/master/adi-i3c-master.c
+++ b/drivers/i3c/master/adi-i3c-master.c
@@ -655,8 +655,7 @@ static int adi_i3c_master_do_daa(struct i3c_master_controller *m)
 
 	writel(irq_mask, master->regs + REG_IRQ_MASK);
 
-	/* DAA always finishes with CE2_ERROR or NACK_RESP */
-	if (ret && ret != I3C_ERROR_M2)
+	if (ret)
 		return ret;
 
 	/* Add I3C devices discovered */
diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
index b78aebf6b2ca..5cfec6761494 100644
--- a/drivers/i3c/master/i3c-master-cdns.c
+++ b/drivers/i3c/master/i3c-master-cdns.c
@@ -1143,7 +1143,7 @@ static int cdns_i3c_master_do_daa(struct i3c_master_controller *m)
 	}
 
 	ret = i3c_master_entdaa_locked(&master->base);
-	if (ret && ret != I3C_ERROR_M2)
+	if (ret)
 		return ret;
 
 	newdevs = readl(master->regs + DEVS_CTRL) & DEVS_CTRL_DEVS_ACTIVE_MASK;

-- 
2.51.1


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* [PATCH 3/5] i3c: master: Move bus_init error suppression
  2026-03-08 16:47 [PATCH 0/5] Fix paths unexpectedly returning Mx error codes Jorge Marques
  2026-03-08 16:47 ` [PATCH 1/5] i3c: master: Move rstdaa error suppression Jorge Marques
  2026-03-08 16:47 ` [PATCH 2/5] i3c: master: Move entdaa " Jorge Marques
@ 2026-03-08 16:47 ` Jorge Marques
  2026-03-10 19:14   ` Adrian Hunter
  2026-03-08 16:47 ` [PATCH 4/5] i3c: master: Negative error codes at send_ccc_cmd Jorge Marques
  2026-03-08 16:47 ` [PATCH 5/5] i3c: master: adi: Return xfer->ret at send CCC Jorge Marques
  4 siblings, 1 reply; 17+ messages in thread
From: Jorge Marques @ 2026-03-08 16:47 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Przemysław Gaj
  Cc: linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron,
	Jorge Marques

The CCC DISEC to broadcast address is invoked with
i3c_master_enec_disec_locked and yields error I3C_ERROR_M2 if there are
no devices active on the bus. This is expected at the bus initialization
stage, where it is not known yet that there are no active devices on the
bus.

Handle inside i3c_master_enec_disec_locked the exact corner case to not
require propagating positive Mx error codes.

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
---
 drivers/i3c/master.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index ce1898345810..3e465587c9c7 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1078,6 +1078,15 @@ static int i3c_master_enec_disec_locked(struct i3c_master_controller *master,
 	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
 	i3c_ccc_cmd_dest_cleanup(&dest);
 
+	/*
+	 * If the addr is I3C_BROADCAST_ADDR and enable is false, the return
+	 * error is cleared if the Mx error is I3C_ERROR_M2, to match the
+	 * initialization state where there is no active device on the bus.
+	 */
+	if (ret && addr == I3C_BROADCAST_ADDR && !enable &&
+	    cmd.err == I3C_ERROR_M2)
+		ret = 0;
+
 	return ret;
 }
 
@@ -2112,7 +2121,7 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
 	ret = i3c_master_disec_locked(master, I3C_BROADCAST_ADDR,
 				      I3C_CCC_EVENT_SIR | I3C_CCC_EVENT_MR |
 				      I3C_CCC_EVENT_HJ);
-	if (ret && ret != I3C_ERROR_M2)
+	if (ret)
 		goto err_bus_cleanup;
 
 	/*

-- 
2.51.1


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* [PATCH 4/5] i3c: master: Negative error codes at send_ccc_cmd
  2026-03-08 16:47 [PATCH 0/5] Fix paths unexpectedly returning Mx error codes Jorge Marques
                   ` (2 preceding siblings ...)
  2026-03-08 16:47 ` [PATCH 3/5] i3c: master: Move bus_init " Jorge Marques
@ 2026-03-08 16:47 ` Jorge Marques
  2026-03-10 19:14   ` Adrian Hunter
  2026-03-08 16:47 ` [PATCH 5/5] i3c: master: adi: Return xfer->ret at send CCC Jorge Marques
  4 siblings, 1 reply; 17+ messages in thread
From: Jorge Marques @ 2026-03-08 16:47 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Przemysław Gaj
  Cc: linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron,
	Jorge Marques

i3c_master_send_ccc_cmd_locked would propagate cmd->err (positive,
Mx codes) to the ret variable, cascading down multiple methods until
reaching methods that explicitly stated they would return 0 on success
or negative error code. For example, the call chain:

  i3c_device_enable_ibi <- i3c_dev_enable_ibi_locked <-
  master->ops.enable_ibi <- i3c_master_enec_locked <-
  i3c_master_enec_disec_locked <- i3c_master_send_ccc_cmd_locked

Fix this by returning the ret value, callers can
still read the cmd->err value if ret is negative.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/linux-iio/aYXvT5FW0hXQwhm_@stanley.mountain/

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
---
 drivers/i3c/master.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 3e465587c9c7..8459fffbdebb 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -898,11 +898,17 @@ static void i3c_ccc_cmd_init(struct i3c_ccc_cmd *cmd, bool rnw, u8 id,
 	cmd->err = I3C_ERROR_UNKNOWN;
 }
 
+/**
+ * i3c_master_send_ccc_cmd_locked() - send a CCC (Common Command Codes)
+ * @master: master used to send frames on the bus
+ * @cmd: command to send
+ *
+ * Return: 0 in case of success, or a negative error code otherwise.
+ *         I3C Mx error codes are stored in cmd->err.
+ */
 static int i3c_master_send_ccc_cmd_locked(struct i3c_master_controller *master,
 					  struct i3c_ccc_cmd *cmd)
 {
-	int ret;
-
 	if (!cmd || !master)
 		return -EINVAL;
 
@@ -920,15 +926,7 @@ static int i3c_master_send_ccc_cmd_locked(struct i3c_master_controller *master,
 	    !master->ops->supports_ccc_cmd(master, cmd))
 		return -EOPNOTSUPP;
 
-	ret = master->ops->send_ccc_cmd(master, cmd);
-	if (ret) {
-		if (cmd->err != I3C_ERROR_UNKNOWN)
-			return cmd->err;
-
-		return ret;
-	}
-
-	return 0;
+	return master->ops->send_ccc_cmd(master, cmd);
 }
 
 static struct i2c_dev_desc *
@@ -1101,8 +1099,7 @@ static int i3c_master_enec_disec_locked(struct i3c_master_controller *master,
  *
  * This function must be called with the bus lock held in write mode.
  *
- * Return: 0 in case of success, a positive I3C error code if the error is
- * one of the official Mx error codes, and a negative error code otherwise.
+ * Return: 0 in case of success, or a negative error code otherwise.
  */
 int i3c_master_disec_locked(struct i3c_master_controller *master, u8 addr,
 			    u8 evts)
@@ -1122,8 +1119,7 @@ EXPORT_SYMBOL_GPL(i3c_master_disec_locked);
  *
  * This function must be called with the bus lock held in write mode.
  *
- * Return: 0 in case of success, a positive I3C error code if the error is
- * one of the official Mx error codes, and a negative error code otherwise.
+ * Return: 0 in case of success, or a negative error code otherwise.
  */
 int i3c_master_enec_locked(struct i3c_master_controller *master, u8 addr,
 			   u8 evts)
@@ -1148,8 +1144,7 @@ EXPORT_SYMBOL_GPL(i3c_master_enec_locked);
  *
  * This function must be called with the bus lock held in write mode.
  *
- * Return: 0 in case of success, a positive I3C error code if the error is
- * one of the official Mx error codes, and a negative error code otherwise.
+ * Return: 0 in case of success, or a negative error code otherwise.
  */
 int i3c_master_defslvs_locked(struct i3c_master_controller *master)
 {

-- 
2.51.1


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* [PATCH 5/5] i3c: master: adi: Return xfer->ret at send CCC
  2026-03-08 16:47 [PATCH 0/5] Fix paths unexpectedly returning Mx error codes Jorge Marques
                   ` (3 preceding siblings ...)
  2026-03-08 16:47 ` [PATCH 4/5] i3c: master: Negative error codes at send_ccc_cmd Jorge Marques
@ 2026-03-08 16:47 ` Jorge Marques
  2026-03-10 19:15   ` Adrian Hunter
  4 siblings, 1 reply; 17+ messages in thread
From: Jorge Marques @ 2026-03-08 16:47 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Przemysław Gaj
  Cc: linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron,
	Jorge Marques

Return the xfer-> ret error code at adi_i3c_master_send_ccc_cmd to
propagate the adi_i3c_master_end_xfer_locked value. In particular, if
any of the Mx values are present in the sent commands, returns -EIO.

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
---
 drivers/i3c/master/adi-i3c-master.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i3c/master/adi-i3c-master.c b/drivers/i3c/master/adi-i3c-master.c
index fb9a48830446..047081c9f064 100644
--- a/drivers/i3c/master/adi-i3c-master.c
+++ b/drivers/i3c/master/adi-i3c-master.c
@@ -361,7 +361,7 @@ static int adi_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
 
 	cmd->err = adi_i3c_cmd_get_err(&xfer->cmds[0]);
 
-	return 0;
+	return xfer->ret;
 }
 
 static int adi_i3c_master_i3c_xfers(struct i3c_dev_desc *dev,

-- 
2.51.1


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* Re: [PATCH 1/5] i3c: master: Move rstdaa error suppression
  2026-03-08 16:47 ` [PATCH 1/5] i3c: master: Move rstdaa error suppression Jorge Marques
@ 2026-03-09 11:39   ` Adrian Hunter
  2026-03-09 12:17     ` Jorge Marques
  2026-03-10 19:13   ` Adrian Hunter
  1 sibling, 1 reply; 17+ messages in thread
From: Adrian Hunter @ 2026-03-09 11:39 UTC (permalink / raw)
  To: Jorge Marques, Alexandre Belloni, Frank Li, Przemysław Gaj
  Cc: linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron

On 08/03/2026 18:47, Jorge Marques wrote:
> The CCC RSTDAA is invoked with i3c_master_rstdaa_locked
> even if there are no devices active on the bus, resulting
> in error I3C_ERROR_M2. Handle inside i3c_master_rstdaa_locked,
> checking cmd->err directly.
> 
> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
> ---
>  drivers/i3c/master.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 9e6be49bebb2..31822fd5ffde 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1016,6 +1016,10 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
>  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>  	i3c_ccc_cmd_dest_cleanup(&dest);
>  
> +	/* No active devices on the bus. */
> +	if (ret && cmd.err == I3C_ERROR_M2)
> +		ret = 0;
> +
>  	return ret;
>  }
>  
> @@ -1785,7 +1789,6 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>   */
>  int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
>  {
> -	int rstret = 0;
>  	int ret;
>  
>  	ret = i3c_master_rpm_get(master);
> @@ -1795,9 +1798,9 @@ int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
>  	i3c_bus_maintenance_lock(&master->bus);
>  
>  	if (rstdaa) {
> -		rstret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
> -		if (rstret == I3C_ERROR_M2)
> -			rstret = 0;
> +		ret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
> +		if (ret)
> +			goto out;

This is an unrelated change.  The original intention was to perform
DAA even if RSTDAA fails.  If you really want this, it needs to be
a separate patch with separate justification.

>  	}
>  
>  	ret = master->ops->do_daa(master);
> @@ -1813,7 +1816,7 @@ int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
>  out:
>  	i3c_master_rpm_put(master);
>  
> -	return rstret ?: ret;
> +	return ret;

Ditto

>  }
>  EXPORT_SYMBOL_GPL(i3c_master_do_daa_ext);
>  
> @@ -2093,7 +2096,7 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
>  	 * (assigned by the bootloader for example).
>  	 */
>  	ret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
> -	if (ret && ret != I3C_ERROR_M2)
> +	if (ret)
>  		goto err_bus_cleanup;
>  
>  	if (master->ops->set_speed) {
> 


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* Re: [PATCH 1/5] i3c: master: Move rstdaa error suppression
  2026-03-09 11:39   ` Adrian Hunter
@ 2026-03-09 12:17     ` Jorge Marques
  2026-03-09 12:34       ` Adrian Hunter
  0 siblings, 1 reply; 17+ messages in thread
From: Jorge Marques @ 2026-03-09 12:17 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Jorge Marques, Alexandre Belloni, Frank Li, Przemysław Gaj,
	linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron

On Mon, Mar 09, 2026 at 01:39:36PM +0200, Adrian Hunter wrote:
> On 08/03/2026 18:47, Jorge Marques wrote:
> > --- a/drivers/i3c/master.c
> > +++ b/drivers/i3c/master.c
> > @@ -1016,6 +1016,10 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
> >  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
> >  	i3c_ccc_cmd_dest_cleanup(&dest);
> >  
> > +	/* No active devices on the bus. */
> > +	if (ret && cmd.err == I3C_ERROR_M2)
> > +		ret = 0;
> > +
> >  	return ret;
> >  }
> >  
> > @@ -1785,7 +1789,6 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
> >   */
> >  int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
> >  {
> > -	int rstret = 0;
> >  	int ret;
> >  
> >  	ret = i3c_master_rpm_get(master);
> > @@ -1795,9 +1798,9 @@ int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
> >  	i3c_bus_maintenance_lock(&master->bus);
> >  
> >  	if (rstdaa) {
> > -		rstret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
> > -		if (rstret == I3C_ERROR_M2)
> > -			rstret = 0;
> > +		ret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
> > +		if (ret)
> > +			goto out;
> 
> This is an unrelated change.  The original intention was to perform
> DAA even if RSTDAA fails.  If you really want this, it needs to be
> a separate patch with separate justification.
> 
> >  	}
Hi Adrian, handling I3C_ERROR_M2 is unchanged.

The intention is to perform the DAA if the RSTDAA fail due to a
I3C_ERROR_M2, this behaviour is unchanged, the duplicated error
suppression was moved from the do_daa call to inside
i3c_master_rstdaa_locked, and added a comment to explaining why it is okay
to fail: the i3c controller may probe, without any active i3c device on
the bus to acknowledge the CCC broadcast transfer.

The core purpose of the series is to fix the accidental propagation of
Mx error codes where 0 or negative errors are expected, to achieve this,
the error suppression have been moved to scopes that contain cmd.err and
clarified why they exist in the first place, to not have "ret = cmd.err"
anymore.

There are three exceptions all related to the bus initialization:
* RSTDAA -> i3c_master_rstdaa_locked
* ENTDAA -> i3c_master_entdaa_locked
* DISEC (broadcast address only) -> i3c_master_enec_disec_locked

I was actually wondering if it wouldn't be more adequate to call a
"probe bus" broadcast CCC that would be the only acceptable to return
I3C_ERROR_M2, and if so, we don't do the RSTDAA,DISEC at all (no i3c
devices, why continue?). But since ENTDAA suppression is still needed,
because the DAA always ends in NACK and the specification doesn't
specify if the RTL should suppress an I3C_ERROR_M2 after the ENTDAA
initial ACK, the benefits are minimal.

Best regards,
Jorge


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* Re: [PATCH 1/5] i3c: master: Move rstdaa error suppression
  2026-03-09 12:17     ` Jorge Marques
@ 2026-03-09 12:34       ` Adrian Hunter
  2026-03-10  8:05         ` Jorge Marques
  0 siblings, 1 reply; 17+ messages in thread
From: Adrian Hunter @ 2026-03-09 12:34 UTC (permalink / raw)
  To: Jorge Marques
  Cc: Jorge Marques, Alexandre Belloni, Frank Li, Przemysław Gaj,
	linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron

On 09/03/2026 14:17, Jorge Marques wrote:
> On Mon, Mar 09, 2026 at 01:39:36PM +0200, Adrian Hunter wrote:
>> On 08/03/2026 18:47, Jorge Marques wrote:
>>> --- a/drivers/i3c/master.c
>>> +++ b/drivers/i3c/master.c
>>> @@ -1016,6 +1016,10 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
>>>  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>>>  	i3c_ccc_cmd_dest_cleanup(&dest);
>>>  
>>> +	/* No active devices on the bus. */
>>> +	if (ret && cmd.err == I3C_ERROR_M2)
>>> +		ret = 0;
>>> +
>>>  	return ret;
>>>  }
>>>  
>>> @@ -1785,7 +1789,6 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>>>   */
>>>  int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
>>>  {
>>> -	int rstret = 0;
>>>  	int ret;
>>>  
>>>  	ret = i3c_master_rpm_get(master);
>>> @@ -1795,9 +1798,9 @@ int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
>>>  	i3c_bus_maintenance_lock(&master->bus);
>>>  
>>>  	if (rstdaa) {
>>> -		rstret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
>>> -		if (rstret == I3C_ERROR_M2)
>>> -			rstret = 0;
>>> +		ret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
>>> +		if (ret)
>>> +			goto out;
>>
>> This is an unrelated change.  The original intention was to perform
>> DAA even if RSTDAA fails.  If you really want this, it needs to be
>> a separate patch with separate justification.
>>
>>>  	}
> Hi Adrian, handling I3C_ERROR_M2 is unchanged.
> 
> The intention is to perform the DAA if the RSTDAA fail due to a
> I3C_ERROR_M2, this behaviour is unchanged,
No, the intention was as the code is written.  DAA is done
irrespective of whether RSTDAA fails.  The behaviour has changed:
before it always does DAA, after it does not always.


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* Re: [PATCH 1/5] i3c: master: Move rstdaa error suppression
  2026-03-09 12:34       ` Adrian Hunter
@ 2026-03-10  8:05         ` Jorge Marques
  2026-03-10 19:13           ` Adrian Hunter
  0 siblings, 1 reply; 17+ messages in thread
From: Jorge Marques @ 2026-03-10  8:05 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Jorge Marques, Alexandre Belloni, Frank Li, Przemysław Gaj,
	linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron

On Mon, Mar 09, 2026 at 02:34:58PM +0200, Adrian Hunter wrote:
> On 09/03/2026 14:17, Jorge Marques wrote:
> > On Mon, Mar 09, 2026 at 01:39:36PM +0200, Adrian Hunter wrote:
> >> On 08/03/2026 18:47, Jorge Marques wrote:
> >>> --- a/drivers/i3c/master.c
> >>> +++ b/drivers/i3c/master.c
> >>> @@ -1016,6 +1016,10 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
> >>>  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
> >>>  	i3c_ccc_cmd_dest_cleanup(&dest);
> >>>  
> >>> +	/* No active devices on the bus. */
> >>> +	if (ret && cmd.err == I3C_ERROR_M2)
> >>> +		ret = 0;
> >>> +
> >>>  	return ret;
> >>>  }
> >>>  
> >>> @@ -1785,7 +1789,6 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
> >>>   */
> >>>  int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
> >>>  {
> >>> -	int rstret = 0;
> >>>  	int ret;
> >>>  
> >>>  	ret = i3c_master_rpm_get(master);
> >>> @@ -1795,9 +1798,9 @@ int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
> >>>  	i3c_bus_maintenance_lock(&master->bus);
> >>>  
> >>>  	if (rstdaa) {
> >>> -		rstret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
> >>> -		if (rstret == I3C_ERROR_M2)
> >>> -			rstret = 0;
> >>> +		ret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
> >>> +		if (ret)
> >>> +			goto out;
> >>
> >> This is an unrelated change.  The original intention was to perform
> >> DAA even if RSTDAA fails.  If you really want this, it needs to be
> >> a separate patch with separate justification.
> >>
> >>>  	}
> > Hi Adrian, handling I3C_ERROR_M2 is unchanged.
> > 
> > The intention is to perform the DAA if the RSTDAA fail due to a
> > I3C_ERROR_M2, this behaviour is unchanged,
> No, the intention was as the code is written.  DAA is done
> irrespective of whether RSTDAA fails.  The behaviour has changed:
> before it always does DAA, after it does not always.
> 

Hi Adrian, you are right, the og behaviour is to continue on any error.
v2 will bring the rstret back, thanks for pointing out! For context, did
you experience peripherals/controllers that failed with
err != I3C_ERROR_M2 on RSTDAA? Would be nice to map at least one
non-spec-conforming case.

Best regards,
Jorge

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* Re: [PATCH 1/5] i3c: master: Move rstdaa error suppression
  2026-03-10  8:05         ` Jorge Marques
@ 2026-03-10 19:13           ` Adrian Hunter
  0 siblings, 0 replies; 17+ messages in thread
From: Adrian Hunter @ 2026-03-10 19:13 UTC (permalink / raw)
  To: Jorge Marques
  Cc: Jorge Marques, Alexandre Belloni, Frank Li, Przemysław Gaj,
	linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron

On 10/03/2026 10:05, Jorge Marques wrote:
> On Mon, Mar 09, 2026 at 02:34:58PM +0200, Adrian Hunter wrote:
>> On 09/03/2026 14:17, Jorge Marques wrote:
>>> On Mon, Mar 09, 2026 at 01:39:36PM +0200, Adrian Hunter wrote:
>>>> On 08/03/2026 18:47, Jorge Marques wrote:
>>>>> --- a/drivers/i3c/master.c
>>>>> +++ b/drivers/i3c/master.c
>>>>> @@ -1016,6 +1016,10 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
>>>>>  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>>>>>  	i3c_ccc_cmd_dest_cleanup(&dest);
>>>>>  
>>>>> +	/* No active devices on the bus. */
>>>>> +	if (ret && cmd.err == I3C_ERROR_M2)
>>>>> +		ret = 0;
>>>>> +
>>>>>  	return ret;
>>>>>  }
>>>>>  
>>>>> @@ -1785,7 +1789,6 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>>>>>   */
>>>>>  int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
>>>>>  {
>>>>> -	int rstret = 0;
>>>>>  	int ret;
>>>>>  
>>>>>  	ret = i3c_master_rpm_get(master);
>>>>> @@ -1795,9 +1798,9 @@ int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
>>>>>  	i3c_bus_maintenance_lock(&master->bus);
>>>>>  
>>>>>  	if (rstdaa) {
>>>>> -		rstret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
>>>>> -		if (rstret == I3C_ERROR_M2)
>>>>> -			rstret = 0;
>>>>> +		ret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
>>>>> +		if (ret)
>>>>> +			goto out;
>>>>
>>>> This is an unrelated change.  The original intention was to perform
>>>> DAA even if RSTDAA fails.  If you really want this, it needs to be
>>>> a separate patch with separate justification.
>>>>
>>>>>  	}
>>> Hi Adrian, handling I3C_ERROR_M2 is unchanged.
>>>
>>> The intention is to perform the DAA if the RSTDAA fail due to a
>>> I3C_ERROR_M2, this behaviour is unchanged,
>> No, the intention was as the code is written.  DAA is done
>> irrespective of whether RSTDAA fails.  The behaviour has changed:
>> before it always does DAA, after it does not always.
>>
> 
> Hi Adrian, you are right, the og behaviour is to continue on any error.
> v2 will bring the rstret back, thanks for pointing out! For context, did
> you experience peripherals/controllers that failed with
> err != I3C_ERROR_M2 on RSTDAA? Would be nice to map at least one
> non-spec-conforming case.

DAA is being used to ensure devices that lost power in suspend
have a DAA assigned.

RSTDAA is used also in the case of hibernation because the hibernation
boot, suspend, restore, may have left the device with the wrong DAA.

So the two are not directly related.


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* Re: [PATCH 1/5] i3c: master: Move rstdaa error suppression
  2026-03-08 16:47 ` [PATCH 1/5] i3c: master: Move rstdaa error suppression Jorge Marques
  2026-03-09 11:39   ` Adrian Hunter
@ 2026-03-10 19:13   ` Adrian Hunter
  1 sibling, 0 replies; 17+ messages in thread
From: Adrian Hunter @ 2026-03-10 19:13 UTC (permalink / raw)
  To: Jorge Marques, Alexandre Belloni, Frank Li, Przemysław Gaj
  Cc: linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron

On 08/03/2026 18:47, Jorge Marques wrote:
> The CCC RSTDAA is invoked with i3c_master_rstdaa_locked
> even if there are no devices active on the bus, resulting
> in error I3C_ERROR_M2. Handle inside i3c_master_rstdaa_locked,
> checking cmd->err directly.

The commit message could use some explanation why the change
is being made, what it achieves in terms of preparing for the
later fix.

It should mention that there are 4 call sites of i3c_master_rstdaa_locked().
2 ignore the return value, and so need not be changed.
And the 2 in this patch.

Also, since this is a prerequisite for a bug fix, I would
suggest a fixes tag.

> 
> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
> ---
>  drivers/i3c/master.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 9e6be49bebb2..31822fd5ffde 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1016,6 +1016,10 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
>  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>  	i3c_ccc_cmd_dest_cleanup(&dest);
>  
> +	/* No active devices on the bus. */
> +	if (ret && cmd.err == I3C_ERROR_M2)
> +		ret = 0;
> +
>  	return ret;
>  }
>  
> @@ -1785,7 +1789,6 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>   */
>  int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
>  {
> -	int rstret = 0;
>  	int ret;
>  
>  	ret = i3c_master_rpm_get(master);
> @@ -1795,9 +1798,9 @@ int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
>  	i3c_bus_maintenance_lock(&master->bus);
>  
>  	if (rstdaa) {
> -		rstret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
> -		if (rstret == I3C_ERROR_M2)
> -			rstret = 0;
> +		ret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
> +		if (ret)
> +			goto out;
>  	}
>  
>  	ret = master->ops->do_daa(master);
> @@ -1813,7 +1816,7 @@ int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
>  out:
>  	i3c_master_rpm_put(master);
>  
> -	return rstret ?: ret;
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(i3c_master_do_daa_ext);
>  
> @@ -2093,7 +2096,7 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
>  	 * (assigned by the bootloader for example).
>  	 */
>  	ret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
> -	if (ret && ret != I3C_ERROR_M2)
> +	if (ret)
>  		goto err_bus_cleanup;
>  
>  	if (master->ops->set_speed) {
> 


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* Re: [PATCH 2/5] i3c: master: Move entdaa error suppression
  2026-03-08 16:47 ` [PATCH 2/5] i3c: master: Move entdaa " Jorge Marques
@ 2026-03-10 19:13   ` Adrian Hunter
  0 siblings, 0 replies; 17+ messages in thread
From: Adrian Hunter @ 2026-03-10 19:13 UTC (permalink / raw)
  To: Jorge Marques, Alexandre Belloni, Frank Li, Przemysław Gaj
  Cc: linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron

On 08/03/2026 18:47, Jorge Marques wrote:
> The CCC ENTDAA is invoked with i3c_master_entdaa_locked and yields
> error I3C_ERROR_M2 if there are no devices active on the bus.
> Some controllers may also yield if there are no more devices need an
> dynamic address, since the sequence do always end in a NACK.
> 
> Handle inside i3c_master_entdaa_locked, checking cmd->err directly.

Commit message has the same issues as patch 1

And the pertinent fact that there are only 2 call sites of
i3c_master_entdaa_locked() and both are dealt with.

> 
> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
> ---
>  drivers/i3c/master.c                 | 7 +++++--
>  drivers/i3c/master/adi-i3c-master.c  | 3 +--
>  drivers/i3c/master/i3c-master-cdns.c | 2 +-
>  3 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 31822fd5ffde..ce1898345810 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1036,8 +1036,7 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
>   *
>   * This function must be called with the bus lock held in write mode.
>   *
> - * Return: 0 in case of success, a positive I3C error code if the error is
> - * one of the official Mx error codes, and a negative error code otherwise.
> + * Return: 0 in case of success, or a negative error code otherwise.

That comment change seems premature.  Mx (x!=2) error codes are propagated
until patch 4

>   */
>  int i3c_master_entdaa_locked(struct i3c_master_controller *master)
>  {
> @@ -1050,6 +1049,10 @@ int i3c_master_entdaa_locked(struct i3c_master_controller *master)
>  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>  	i3c_ccc_cmd_dest_cleanup(&dest);
>  
> +	/* No active devices need an address. */
> +	if (ret && cmd.err == I3C_ERROR_M2)
> +		ret = 0;
> +
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(i3c_master_entdaa_locked);
> diff --git a/drivers/i3c/master/adi-i3c-master.c b/drivers/i3c/master/adi-i3c-master.c
> index 6616f751075a..fb9a48830446 100644
> --- a/drivers/i3c/master/adi-i3c-master.c
> +++ b/drivers/i3c/master/adi-i3c-master.c
> @@ -655,8 +655,7 @@ static int adi_i3c_master_do_daa(struct i3c_master_controller *m)
>  
>  	writel(irq_mask, master->regs + REG_IRQ_MASK);
>  
> -	/* DAA always finishes with CE2_ERROR or NACK_RESP */
> -	if (ret && ret != I3C_ERROR_M2)
> +	if (ret)
>  		return ret;
>  
>  	/* Add I3C devices discovered */
> diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
> index b78aebf6b2ca..5cfec6761494 100644
> --- a/drivers/i3c/master/i3c-master-cdns.c
> +++ b/drivers/i3c/master/i3c-master-cdns.c
> @@ -1143,7 +1143,7 @@ static int cdns_i3c_master_do_daa(struct i3c_master_controller *m)
>  	}
>  
>  	ret = i3c_master_entdaa_locked(&master->base);
> -	if (ret && ret != I3C_ERROR_M2)
> +	if (ret)
>  		return ret;
>  
>  	newdevs = readl(master->regs + DEVS_CTRL) & DEVS_CTRL_DEVS_ACTIVE_MASK;
> 


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* Re: [PATCH 3/5] i3c: master: Move bus_init error suppression
  2026-03-08 16:47 ` [PATCH 3/5] i3c: master: Move bus_init " Jorge Marques
@ 2026-03-10 19:14   ` Adrian Hunter
  0 siblings, 0 replies; 17+ messages in thread
From: Adrian Hunter @ 2026-03-10 19:14 UTC (permalink / raw)
  To: Jorge Marques, Alexandre Belloni, Frank Li, Przemysław Gaj
  Cc: linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron

On 08/03/2026 18:47, Jorge Marques wrote:
> The CCC DISEC to broadcast address is invoked with
> i3c_master_enec_disec_locked and yields error I3C_ERROR_M2 if there are
> no devices active on the bus. This is expected at the bus initialization
> stage, where it is not known yet that there are no active devices on the
> bus.
> 
> Handle inside i3c_master_enec_disec_locked the exact corner case to not
> require propagating positive Mx error codes.

Commit message has the same issues as patch 1

> 
> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
> ---
>  drivers/i3c/master.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index ce1898345810..3e465587c9c7 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1078,6 +1078,15 @@ static int i3c_master_enec_disec_locked(struct i3c_master_controller *master,
>  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>  	i3c_ccc_cmd_dest_cleanup(&dest);
>  
> +	/*
> +	 * If the addr is I3C_BROADCAST_ADDR and enable is false, the return
> +	 * error is cleared if the Mx error is I3C_ERROR_M2, to match the
> +	 * initialization state where there is no active device on the bus.
> +	 */
> +	if (ret && addr == I3C_BROADCAST_ADDR && !enable &&
> +	    cmd.err == I3C_ERROR_M2)
> +		ret = 0;

Seems a bit ugly.  Maybe it would be better to pass down a parameter
to specify caller's requirement for M2 errors e.g.

static int i3c_master_enec_disec_locked(struct i3c_master_controller *master,
					u8 addr, bool enable, u8 evts, bool suppress_m2)
...
	if (ret && suppress_m2 && cmd.err == I3C_ERROR_M2)
		ret = 0;

> +
>  	return ret;
>  }
>  
> @@ -2112,7 +2121,7 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
>  	ret = i3c_master_disec_locked(master, I3C_BROADCAST_ADDR,
>  				      I3C_CCC_EVENT_SIR | I3C_CCC_EVENT_MR |
>  				      I3C_CCC_EVENT_HJ);
> -	if (ret && ret != I3C_ERROR_M2)
> +	if (ret)
>  		goto err_bus_cleanup;
>  
>  	/*
> 


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* Re: [PATCH 4/5] i3c: master: Negative error codes at send_ccc_cmd
  2026-03-08 16:47 ` [PATCH 4/5] i3c: master: Negative error codes at send_ccc_cmd Jorge Marques
@ 2026-03-10 19:14   ` Adrian Hunter
  0 siblings, 0 replies; 17+ messages in thread
From: Adrian Hunter @ 2026-03-10 19:14 UTC (permalink / raw)
  To: Jorge Marques, Alexandre Belloni, Frank Li, Przemysław Gaj
  Cc: linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron

On 08/03/2026 18:47, Jorge Marques wrote:
> i3c_master_send_ccc_cmd_locked would propagate cmd->err (positive,
> Mx codes) to the ret variable, cascading down multiple methods until
> reaching methods that explicitly stated they would return 0 on success
> or negative error code. For example, the call chain:
> 
>   i3c_device_enable_ibi <- i3c_dev_enable_ibi_locked <-
>   master->ops.enable_ibi <- i3c_master_enec_locked <-
>   i3c_master_enec_disec_locked <- i3c_master_send_ccc_cmd_locked
> 
> Fix this by returning the ret value, callers can
> still read the cmd->err value if ret is negative.

You should say something about how you know that is safe
to do. e.g. although there are myriad call paths, the code
never distinguishes positive from negative error codes except
in cases where it explicitly checks for I3C_ERROR_M2 and
those have been dealt with in the preparation patches.

> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/linux-iio/aYXvT5FW0hXQwhm_@stanley.mountain/

Fixes tag?

> 
> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
> ---
>  drivers/i3c/master.c | 29 ++++++++++++-----------------
>  1 file changed, 12 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 3e465587c9c7..8459fffbdebb 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -898,11 +898,17 @@ static void i3c_ccc_cmd_init(struct i3c_ccc_cmd *cmd, bool rnw, u8 id,
>  	cmd->err = I3C_ERROR_UNKNOWN;
>  }
>  
> +/**
> + * i3c_master_send_ccc_cmd_locked() - send a CCC (Common Command Codes)
> + * @master: master used to send frames on the bus
> + * @cmd: command to send
> + *
> + * Return: 0 in case of success, or a negative error code otherwise.
> + *         I3C Mx error codes are stored in cmd->err.
> + */
>  static int i3c_master_send_ccc_cmd_locked(struct i3c_master_controller *master,
>  					  struct i3c_ccc_cmd *cmd)
>  {
> -	int ret;
> -
>  	if (!cmd || !master)
>  		return -EINVAL;
>  
> @@ -920,15 +926,7 @@ static int i3c_master_send_ccc_cmd_locked(struct i3c_master_controller *master,
>  	    !master->ops->supports_ccc_cmd(master, cmd))
>  		return -EOPNOTSUPP;
>  
> -	ret = master->ops->send_ccc_cmd(master, cmd);
> -	if (ret) {
> -		if (cmd->err != I3C_ERROR_UNKNOWN)
> -			return cmd->err;
> -
> -		return ret;
> -	}
> -
> -	return 0;
> +	return master->ops->send_ccc_cmd(master, cmd);
>  }
>  
>  static struct i2c_dev_desc *
> @@ -1101,8 +1099,7 @@ static int i3c_master_enec_disec_locked(struct i3c_master_controller *master,
>   *
>   * This function must be called with the bus lock held in write mode.
>   *
> - * Return: 0 in case of success, a positive I3C error code if the error is
> - * one of the official Mx error codes, and a negative error code otherwise.
> + * Return: 0 in case of success, or a negative error code otherwise.
>   */
>  int i3c_master_disec_locked(struct i3c_master_controller *master, u8 addr,
>  			    u8 evts)
> @@ -1122,8 +1119,7 @@ EXPORT_SYMBOL_GPL(i3c_master_disec_locked);
>   *
>   * This function must be called with the bus lock held in write mode.
>   *
> - * Return: 0 in case of success, a positive I3C error code if the error is
> - * one of the official Mx error codes, and a negative error code otherwise.
> + * Return: 0 in case of success, or a negative error code otherwise.
>   */
>  int i3c_master_enec_locked(struct i3c_master_controller *master, u8 addr,
>  			   u8 evts)
> @@ -1148,8 +1144,7 @@ EXPORT_SYMBOL_GPL(i3c_master_enec_locked);
>   *
>   * This function must be called with the bus lock held in write mode.
>   *
> - * Return: 0 in case of success, a positive I3C error code if the error is
> - * one of the official Mx error codes, and a negative error code otherwise.
> + * Return: 0 in case of success, or a negative error code otherwise.
>   */
>  int i3c_master_defslvs_locked(struct i3c_master_controller *master)
>  {
> 


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* Re: [PATCH 5/5] i3c: master: adi: Return xfer->ret at send CCC
  2026-03-08 16:47 ` [PATCH 5/5] i3c: master: adi: Return xfer->ret at send CCC Jorge Marques
@ 2026-03-10 19:15   ` Adrian Hunter
  2026-03-12 15:50     ` Jorge Marques
  0 siblings, 1 reply; 17+ messages in thread
From: Adrian Hunter @ 2026-03-10 19:15 UTC (permalink / raw)
  To: Jorge Marques, Alexandre Belloni, Frank Li, Przemysław Gaj
  Cc: linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron

On 08/03/2026 18:47, Jorge Marques wrote:
> Return the xfer-> ret error code at adi_i3c_master_send_ccc_cmd to
> propagate the adi_i3c_master_end_xfer_locked value. In particular, if
> any of the Mx values are present in the sent commands, returns -EIO.

Isn't this also a bug fix.   I suggest a fixes tag and
better commit message e.g.

  i3c: master: adi: Fix missing error propagation for CCC commands

  adi_i3c_master_send_ccc_cmd() always returned 0, ignoring the transfer
  result populated in the completion path. As a consequence, CCC command
  errors - such as those reported by adi_i3c_master_end_xfer_locked() - were
  silently dropped.

  Fix this by returning xfer->ret so that callers correctly receive any
  transfer error codes.

By the way, I don't see an error code being set in the timeout case?

> 
> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
> ---
>  drivers/i3c/master/adi-i3c-master.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i3c/master/adi-i3c-master.c b/drivers/i3c/master/adi-i3c-master.c
> index fb9a48830446..047081c9f064 100644
> --- a/drivers/i3c/master/adi-i3c-master.c
> +++ b/drivers/i3c/master/adi-i3c-master.c
> @@ -361,7 +361,7 @@ static int adi_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
>  
>  	cmd->err = adi_i3c_cmd_get_err(&xfer->cmds[0]);
>  
> -	return 0;
> +	return xfer->ret;
>  }
>  
>  static int adi_i3c_master_i3c_xfers(struct i3c_dev_desc *dev,
> 


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* Re: [PATCH 5/5] i3c: master: adi: Return xfer->ret at send CCC
  2026-03-10 19:15   ` Adrian Hunter
@ 2026-03-12 15:50     ` Jorge Marques
  0 siblings, 0 replies; 17+ messages in thread
From: Jorge Marques @ 2026-03-12 15:50 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Jorge Marques, Alexandre Belloni, Frank Li, Przemysław Gaj,
	linux-i3c, linux-kernel, Dan Carpenter, Jonathan Cameron

On Tue, Mar 10, 2026 at 09:15:25PM +0200, Adrian Hunter wrote:
> On 08/03/2026 18:47, Jorge Marques wrote:
> > Return the xfer-> ret error code at adi_i3c_master_send_ccc_cmd to
> > propagate the adi_i3c_master_end_xfer_locked value. In particular, if
> > any of the Mx values are present in the sent commands, returns -EIO.
> 
> Isn't this also a bug fix.   I suggest a fixes tag and
> better commit message e.g.
> 
>   i3c: master: adi: Fix missing error propagation for CCC commands
> 
>   adi_i3c_master_send_ccc_cmd() always returned 0, ignoring the transfer
>   result populated in the completion path. As a consequence, CCC command
>   errors - such as those reported by adi_i3c_master_end_xfer_locked() - were
>   silently dropped.
> 
>   Fix this by returning xfer->ret so that callers correctly receive any
>   transfer error codes.
> 
> By the way, I don't see an error code being set in the timeout case?
> 
Hi Adrian, when the xfer is allocated, the default xfer->ret value is
set to -ETIMEDOUT, so un-queuing retains this value and is (with this
patch) returned.

Regards,
Jorge

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

end of thread, other threads:[~2026-03-12 15:50 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-08 16:47 [PATCH 0/5] Fix paths unexpectedly returning Mx error codes Jorge Marques
2026-03-08 16:47 ` [PATCH 1/5] i3c: master: Move rstdaa error suppression Jorge Marques
2026-03-09 11:39   ` Adrian Hunter
2026-03-09 12:17     ` Jorge Marques
2026-03-09 12:34       ` Adrian Hunter
2026-03-10  8:05         ` Jorge Marques
2026-03-10 19:13           ` Adrian Hunter
2026-03-10 19:13   ` Adrian Hunter
2026-03-08 16:47 ` [PATCH 2/5] i3c: master: Move entdaa " Jorge Marques
2026-03-10 19:13   ` Adrian Hunter
2026-03-08 16:47 ` [PATCH 3/5] i3c: master: Move bus_init " Jorge Marques
2026-03-10 19:14   ` Adrian Hunter
2026-03-08 16:47 ` [PATCH 4/5] i3c: master: Negative error codes at send_ccc_cmd Jorge Marques
2026-03-10 19:14   ` Adrian Hunter
2026-03-08 16:47 ` [PATCH 5/5] i3c: master: adi: Return xfer->ret at send CCC Jorge Marques
2026-03-10 19:15   ` Adrian Hunter
2026-03-12 15:50     ` Jorge Marques

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