Re: [PATCH V2 7/7] i3c: master: Reconcile dynamic addresses after DAA
Frank Li <[email protected]>
| Newsgroups | org.infradead.lists.linux-i3c,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <aighSvgqAn4ui1OM@SMW015318> |
On Tue, Jun 09, 2026 at 12:23:36PM +0300, Adrian Hunter wrote: > On 08/06/2026 21:06, Frank Li wrote: > > 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 <[email protected]> > >> --- ... > >> + > >> +#define I3C_DEV_PROBE_INITIAL_DELAY_US 20 > >> +#define I3C_DEV_PROBE_DELAY_FACTOR 2 > >> +#define I3C_DEV_PROBE_CNT 5 > > They are somewhat arbitrary, but should give a device enough > opportunity to respond. Need comments for it in case someone need adjust it in future. if spec required, it can't change and need consider more. > > > > > what's these value coming from? from i3c spec? > > ... > > >> + /* 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? > > Currently, addr is always I3C_ADDR_SLOT_FREE on all paths that call > __i3c_master_add_i3c_dev_locked(), so this is consistent. > > There is no i3c_dev_desc for this addr since that was checked just > above. > > Then a success path will use i3c_master_attach_i3c_dev() which sets > I3C_ADDR_SLOT_I3C_DEV via i3c_master_get_i3c_addrs() Okay, I3C address allocate have little complex. Frank > > > > > 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 > >> > -- linux-i3c mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-i3c