Re: [PATCH V2 3/7] i3c: master: Prevent reuse of dynamic address on device add failure

[Adrian Hunter]

On 08/06/2026 20:45, Frank Li wrote:
> On Mon, Jun 08, 2026 at 10:57:56AM +0300, Adrian Hunter wrote:
>> i3c_master_add_i3c_dev_locked() is called after a device has already
>> been assigned a dynamic address.  If the function fails, the address
>> remains marked as free and may be reallocated to another device,
>> leading to address conflicts on the bus.
> 
> There are other error patch, which also free dynamic address. And if add
> dev failure, should it send CCC command to clean device dymatic address?

Between bus init and bus cleanup, there is only i3c_master_add_i3c_dev_locked()
as far as I can see.

Also, Direct RSTDAA is deprecated starting v1.1 and v1.1+ target devices are
meant to NACK it.  So that is not an option going forward.

> 
> Frank
>>
>> Ensure the address is not marked as free on failure, by updating the
>> address slot state to prevent the address from being re-used.
>>
>> Emit an error message to inform of the failure.
>>
>> Opportunistically remove the !master check because it is impossible.
>>
>> Fixes: 3a379bbcea0af ("i3c: Add core I3C infrastructure")
>> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
>> ---
>>
>>
>> Changes in V2:
>>
>> 	Fix 'if (IS_ERR(newdev)' error path.
>> 	Be defensive and do not change the addr_slot_status if it is not
>> 	free, and update commit message accordingly.
>> 	Amend commit message to note removal of unnecesary 'if (!master)'
>> 	check.
>> 	Add Fixes tag.
>>
>>
>>  drivers/i3c/master.c | 19 ++++++++++++++-----
>>  1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
>> index f87bf0099d3c..7b60b0c7f646 100644
>> --- a/drivers/i3c/master.c
>> +++ b/drivers/i3c/master.c
>> @@ -2345,12 +2345,11 @@ int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
>>  	bool enable_ibi = false;
>>  	int ret;
>>
>> -	if (!master)
>> -		return -EINVAL;
>> -
>>  	newdev = i3c_master_alloc_i3c_dev(master, &info);
>> -	if (IS_ERR(newdev))
>> -		return PTR_ERR(newdev);
>> +	if (IS_ERR(newdev)) {
>> +		ret = PTR_ERR(newdev);
>> +		goto err_prevent_addr_reuse;
>> +	}
>>
>>  	ret = i3c_master_attach_i3c_dev(master, newdev);
>>  	if (ret)
>> @@ -2472,6 +2471,16 @@ int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
>>  err_free_dev:
>>  	i3c_master_free_i3c_dev(newdev);
>>
>> +err_prevent_addr_reuse:
>> +	/*
>> +	 * Although the device has not been added, the address has been
>> +	 * assigned. Prevent the address from being used again.
>> +	 */
>> +	if (i3c_bus_get_addr_slot_status(&master->bus, addr) == I3C_ADDR_SLOT_FREE)
>> +		i3c_bus_set_addr_slot_status(&master->bus, addr, I3C_ADDR_SLOT_I3C_DEV);
>> +
>> +	dev_err(&master->dev, "Failed to add I3C device at address %u, error %d\n", addr, ret);
>> +
>>  	return ret;
>>  }
>>  EXPORT_SYMBOL_GPL(i3c_master_add_i3c_dev_locked);
>> --
>> 2.51.0
>>