Re: [PATCH v6 04/12] i3c: master: Add support for devices using SETAASA

On Mon, Jul 27, 2026 at 12:57:45PM +0000, Akhil R wrote:
> On Tue, 21 Jul 2026 04:30:58 +0000, Sashiko AI review <[email protected]> wrote:
> > commit e158b576a9f733695d868ee16f34bacdb43285b3
> > Author: Akhil R <[email protected]>
> >
> > i3c: master: Add support for devices using SETAASA
> >
> > This commit adds support for discovering I3C devices using the SETAASA
> > procedure, such as SPD5118 and SPD5108 attached to DDR5 memory modules. It
> > adds the SETAASA and SETHID CCC commands and updates the discovery logic.
> >
> > Link: https://www.mipi.org/mipi-disco-for-i3c-download
> >
> > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > > index 623c6b2247d9f..b18dda89c4737 100644
> > > --- a/drivers/i3c/master.c
> > > +++ b/drivers/i3c/master.c
> >
> > [ ... ]
> >
> > > @@ -1102,6 +1103,51 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
> > >  	return ret;
> > >  }
> > >
> > > +static int i3c_master_setaasa_locked(struct i3c_master_controller *master)
> > > +{
> > > +	struct i3c_ccc_cmd_dest dest;
> > > +	struct i3c_ccc_cmd cmd;
> > > +	int ret;
> > > +
> > > +	i3c_ccc_cmd_dest_init(&dest, I3C_BROADCAST_ADDR, 0);
> > > +	i3c_ccc_cmd_init(&cmd, false, I3C_CCC_VENDOR(0, true), &dest, 1);
> > > +	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
> >
> > [Severity: High]
> > Will this fail on most existing I3C master controllers?
> >
> > Looking at i3c_master_send_ccc_cmd_locked(), it calls
> > master->ops->supports_ccc_cmd() before sending. Many core controller drivers
> > (like dw-i3c-master.c) have strict switch-statement whitelists for supported
> > CCC IDs that do not include these new SETAASA (0x29) and SETHID (0x61)
> > commands.
> >
> > Would this result in returning -EOPNOTSUPP and aborting the entire bus
> > initialization on systems with these controllers?
>
> SETAASA/SETHID are only issued when a device opts in to the SETAASA method
> (i3c_master_setaasa_locked() is called solely when addr_method has
> I3C_ADDR_METHOD_SETAASA, and SETHID is sent from within it). Existing
> drivers are unaffected because SETAASA is a new capability they do not yet
> advertise; they can whitelist these CCCs when they add SETAASA support,
> which will have to be updated in the individual drivers. This patchset
> does that for the DesignWare contoller driver.
>
> > [ ... ]
> >
> > > @@ -1878,6 +1924,22 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
> > >  	if (ret)
> > >  		goto err_free_dev;
> > >
> > > +	if (i3cdev->boardinfo->static_addr_method & I3C_ADDR_METHOD_SETAASA) {
> > > +		i3cdev->info.dyn_addr = i3cdev->boardinfo->static_addr;
> > > +		ret = i3c_master_reattach_i3c_dev_locked(i3cdev, 0);
> > > +		if (ret)
> > > +			goto err_rstdaa;
> >
> > [Severity: High]
> > If i3c_master_reattach_i3c_dev_locked() fails here, we jump to err_rstdaa
> > and eventually free the device structure. However, i3c_master_bus_init()
> > later unconditionally broadcasts SETAASA to the bus.
> >
> > Won't the physical device claim this static address during the broadcast,
> > even though the controller software no longer tracks it? This could lead to
> > a bus collision when the software later tries to dynamically assign this
> > "free" address slot to a different device.
>
> Address conflicts between statically-addressed devices are rejected up
> front in i3c_master_get_i3c_addrs(), so it should not cause any address
> collision with other devices on the bus unless the same address is being
> assigned through ENTDAA. If this is a concern, we can avoid freeing the
> static address for SETAASA devices even if software attachment fails.
>
> Let me know if this needs to be done.

Try it.

>
> > [Severity: High]
> > This is a pre-existing issue, but does jumping to err_rstdaa correctly reset
> > the hardware if reattachment fails during SETDASA or SETAASA?
> >
> > If i3c_master_reattach_i3c_dev_locked() fails, its internal error path calls
> > i3c_master_put_i3c_addrs(), which marks the slot as I3C_ADDR_SLOT_FREE.
> >
> > When we reach the err_rstdaa path:
> >
> > err_rstdaa:
> >     i3c_master_rstdaa_locked(master, i3cdev->boardinfo->init_dyn_addr);
> >
> > i3c_master_rstdaa_locked() checks i3c_bus_get_addr_slot_status(). Because the
> > slot is already marked FREE, it returns -EINVAL and silently aborts without
> > sending the RSTDAA command to the hardware.
> >
> > Does this leave the physical device holding the address while the software
> > considers it free?
>
> Pre-existing; not introduced by this patch. Better addressed separately.
>
> > > +
> > > +		return 0;
> > > +	}
> > > +
> > >  	ret = i3c_master_setdasa_locked(master, i3cdev->info.static_addr,
> > >  					i3cdev->boardinfo->init_dyn_addr);
> >
> > [ ... ]
> >
> > > @@ -2741,7 +2816,19 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
> > > +	if (boardinfo->static_addr_method & I3C_ADDR_METHOD_SETAASA) {
> > > +		/* For SETAASA, static address is taken as the dynamic address. */
> > > +		init_dyn_addr = boardinfo->static_addr;
> > > +	}
> > > +
> > > +	/* Update the address methods required for device discovery */
> > > +	master->addr_method |= boardinfo->static_addr_method;
> > > +
> > >  	boardinfo->pid = ((u64)reg[1] << 32) | reg[2];
> > >
> > >  	if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
> >
> > [Severity: High]
> > By relaxing the PID validation for SETAASA devices in this function, pid can
> > default to 0. If a system configures multiple SETAASA devices (like DDR5
> > SPDs) without PIDs, will this cause a kobject name collision?
> >
> > During registration, i3c_master_register_new_i3c_devs() names the device
> > using its bus ID and PID:
> >
> > dev_set_name(&desc->dev->dev, "%d-%llx", master->bus.id, desc->info.pid);
> >
> > If multiple devices have pid=0, they would all be named "0-0", causing
> > device_register() to fail with -EEXIST for subsequent devices and preventing
> > them from functioning.
>
> Static addressing requires each device to have a distinct static address on
> the bus,

Any check and report error if two devices have the same address?

Frank

> and PID-less devices are named from that unique static address
> ("%d-%02x"). The bus address-slot management also prevents two live devices
> from occupying the same address. So a collision would require a malformed
> description with duplicate static addresses, which is not a valid bus.
>
> Best Regards,
> Akhil

-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c