Re: [PATCH v20 6/8] i3c: hub: Add support for the I3C interface in the I3C hub

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] i3c: hub: ENTDAA-discovered downstream devices are misrouted to parent bus
--

commit bf3909722bdabc30e1d69d666d5732199356fb61
Author: Lakshay Piplani <lakshay.piplani@nxp.com>

i3c: hub: Add support for the I3C interface in the I3C hub

This commit adds virtual I3C bus support for hubs by creating virtual
master controllers for downstream ports and forwarding bus operations
through the hub device.

> diff --git a/drivers/i3c/hub.c b/drivers/i3c/hub.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/i3c/hub.c
[ ... ]
> +static int i3c_hub_do_daa(struct i3c_hub *hub,
> +			  struct i3c_master_controller *controller)
> +{
> +	struct i3c_master_controller *parent;
> +	int ret;
[ ... ]
> +	mutex_lock(&hub->lock);
> +	i3c_hub_enable_port(controller);
> +
> +	/*
> +	 * Downstream devices reachable through hub target-port routes share the
> +	 * parent controller's I3C address space. The hub gates access to a
> +	 * target-port network, but it does not create an independent dynamic
> +	 * address domain per virtual bus.
> +	 *
> +	 * Run DAA on the parent controller so dynamic addresses remain unique
> +	 * across all downstream devices, even when they are behind different
> +	 * target ports.
> +	 *
> +	 * The current hub model shares the parent controller's dynamic-address
> +	 * domain and does not support independent per-port DAA domains, so a
> +	 * DAA result does not go stale behind a port toggle. Addresses reserved
> +	 * from the Device Tree are held for the lifetime of the parent bus and
> +	 * are never released on a port toggle, so enabling or disabling a port
> +	 * only gates reachability and never causes an address to be handed out
> +	 * twice.
> +	 */
> +	ret = i3c_master_do_daa(parent);

[Severity: High]
Does delegating DAA to the parent controller here misroute newly discovered
downstream devices?

When i3c_master_do_daa() executes ENTDAA on the parent controller, any
downstream I3C device lacking a static address will be assigned an address
and added directly to the physical parent's bus.devs.i3c list instead of
the virtual hub bus.

Because these devices are attached directly to the parent, won't their
subsequent transfers bypass the virtual controller's priv_xfers routing?
If so, the hub port would never be enabled during normal communication,
causing all transfers to the device to fail with a NACK or timeout.

> +	i3c_hub_disable_port(controller);
> +	mutex_unlock(&hub->lock);
> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922103551.2754613-1-lakshay.piplani@nxp.com?part=6

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c