Re: [PATCH V2 7/7] i3c: master: Reconcile dynamic addresses after DAA

[Frank Li <Frank.Li@oss.nxp.com>]

On Mon, Jun 08, 2026 at 10:58:00AM +0300, Adrian Hunter wrote:
> After Dynamic Address Assignment (DAA), there may be cases where
> devices have been assigned dynamic addresses on the bus, but are not
> successfully registered in the device model.  This can happen, for
> example, if errors occur during device addition, leaving the bus state
> and software state inconsistent.
>
> Introduce a reconciliation step to resolve such inconsistencies.
>
> Scan all address slots marked as I3C devices by the bus, and compare
> them against the set of devices currently registered.  For any dynamic
> address that is marked occupied but has no corresponding i3c_dev_desc,
> probe for device presence using a GETSTATUS CCC.
>
> Retry the probe (with exponential backoff delay) to handle transient NACK
> conditions.  If a device responds, register it via
> i3c_master_add_i3c_dev_locked().  Otherwise, free the address
> slot so it may be reused in future DAA operations.
>
> Note, i3c_master_add_i3c_dev_locked() may fail (again), in which case the
> dynamic address remains marked as occupied.  A future DAA will try again.
>
> This also handles a corner case where a device is assigned a dynamic
> address but not successfully added, and subsequently loses that address
> (e.g. due to power management).  If DAA is run again, the device may
> receive a new dynamic address while the old one remains marked as
> occupied.  Repeated occurrences of this scenario could eventually
> exhaust the dynamic address space.  The reconciliation step ensures that
> stale addresses are detected and freed, preventing address leakage.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>
>
> Changes in V2:
>
> 	Add bitmap.h include for bitmap_zero() etc.
> 	Re-base due to changes in previous patches.
>
>
>  drivers/i3c/master.c | 115 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 113 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 5021d25b718a..6656c8591fab 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -6,7 +6,9 @@
>   */
>
>  #include <linux/atomic.h>
> +#include <linux/bitmap.h>
>  #include <linux/bug.h>
> +#include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/err.h>
> @@ -1613,6 +1615,56 @@ static int i3c_master_retrieve_dev_info(struct i3c_dev_desc *dev)
>  	return 0;
>  }
>
> +static int i3c_master_getstatus_locked(struct i3c_master_controller *master,
> +				       u8 addr, u16 *status)
> +{
> +	struct i3c_ccc_getstatus *getstatus;
> +	struct i3c_ccc_cmd_dest dest;
> +	struct i3c_ccc_cmd cmd;
> +	int ret;
> +
> +	getstatus = i3c_ccc_cmd_dest_init(&dest, addr, sizeof(*getstatus));
> +	if (!getstatus)
> +		return -ENOMEM;
> +
> +	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETSTATUS, &dest, 1);
> +	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
> +	if (ret)
> +		goto out;
> +
> +	if (dest.payload.len != sizeof(*getstatus)) {
> +		ret = -EIO;
> +		goto out;
> +	}
> +
> +	if (status)
> +		*status = be16_to_cpu(getstatus->status);
> +out:
> +	i3c_ccc_cmd_dest_cleanup(&dest);
> +
> +	return ret;
> +}
> +
> +#define I3C_DEV_PROBE_INITIAL_DELAY_US	20
> +#define I3C_DEV_PROBE_DELAY_FACTOR	2
> +#define I3C_DEV_PROBE_CNT		5

what's these value coming from? from i3c spec?

> +
> +static bool i3c_master_i3c_dev_present(struct i3c_master_controller *master, unsigned int addr)
> +{
> +	int delay = I3C_DEV_PROBE_INITIAL_DELAY_US;
> +
> +	for (int i = 0; i < I3C_DEV_PROBE_CNT; i++) {
> +		if (i) {
> +			fsleep(delay);
> +			delay *= I3C_DEV_PROBE_DELAY_FACTOR;
> +		}
> +		if (!i3c_master_getstatus_locked(master, addr, NULL))
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
...
>
> +static void i3c_master_reconcile_dyn_addrs(struct i3c_master_controller *master)
> +{
> +	DECLARE_BITMAP(dev_dyn_addrs, I2C_MAX_ADDR + 1);
> +	enum i3c_addr_slot_status status;
> +	struct i3c_dev_desc *desc;
> +
> +	/* Mark all devices' dynamic addresses in the bitmap */
> +	bitmap_zero(dev_dyn_addrs, I2C_MAX_ADDR + 1);
> +	i3c_bus_for_each_i3cdev(&master->bus, desc)
> +		__set_bit(desc->info.dyn_addr, dev_dyn_addrs);
> +	/* Reconcile the bitmap with the bus address slot status */
> +	for (unsigned int addr = 0; addr <= I2C_MAX_ADDR; addr++) {
> +		status = i3c_bus_get_addr_slot_status(&master->bus, addr);
> +		if (status != I3C_ADDR_SLOT_I3C_DEV || test_bit(addr, dev_dyn_addrs))
> +			continue;
> +		i3c_bus_set_addr_slot_status(&master->bus, addr, I3C_ADDR_SLOT_FREE);
> +		/* Try to add the device, but probe to see if it is really present */
> +		__i3c_master_add_i3c_dev_locked(master, addr, true);

If dev add success for addr, but you free addr at previous line, any
issue?

Frank
> +	}
> +}
> +
>  /**
>   * i3c_master_do_daa_ext() - Dynamic Address Assignment (extended version)
>   * @master: controller
> @@ -2445,6 +2551,11 @@ int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
>  		if (rstdaa)
>  			rstret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
>  		ret = master->ops->do_daa(master);
> +		/*
> +		 * Handle cases where a dynamic address was assigned but the
> +		 * device was not successfully added.
> +		 */
> +		i3c_master_reconcile_dyn_addrs(master);
>  	}
>
>  	i3c_bus_maintenance_unlock(&master->bus);
> --
> 2.51.0
>