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

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) {
>

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) {
>

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

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.

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

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.