Re: [PATCH v6 05/12] i3c: master: Add support for devices without PID

On Mon, Jul 27, 2026 at 02:12:00PM +0000, Akhil R wrote:
> On Tue, 21 Jul 2026 04:26:04 +0000, Sashiko AI review <[email protected]> wrote:
> > commit 671c3fae66002e874a13051a9c956e9987d76674
> > Author: Akhil R <[email protected]>
> >
> > i3c: master: Add support for devices without PID
> >
> > Devices using SETAASA for address assignment are not required to have
> > a 48-bit PID according to the I3C specification. Allow such devices to
> > register and use the static address where PID was required.
> >
> > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > > index b18dda89c4737..7b2f819bf4437 100644
> > > --- a/drivers/i3c/master.c
> > > +++ b/drivers/i3c/master.c
> >
> > [ ... ]
> >
> > > @@ -2389,8 +2398,18 @@ static void i3c_master_attach_boardinfo(struct i3c_dev_desc *i3cdev)
> > >  	struct i3c_dev_boardinfo *i3cboardinfo;
> > >
> > >  	list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) {
> > > -		if (i3cdev->info.pid != i3cboardinfo->pid)
> > > -			continue;
> > > +		if (i3cboardinfo->pid) {
> > > +			if (i3cdev->info.pid != i3cboardinfo->pid)
> > > +				continue;
> > > +		} else {
> > > +			if (!i3cboardinfo->static_addr ||
> > > +			    i3cdev->info.static_addr != i3cboardinfo->static_addr)
> > > +				continue;
> > > +		}
> >
> > [Severity: Medium]
> > Will this static address matching logic ever evaluate to false?
> >
> > Since i3c_master_attach_boardinfo() is only called during the dynamic
> > address assignment (ENTDAA) phase, i3cdev->info.static_addr is inherently 0
> > in this context.
> >
> > Because i3cboardinfo->static_addr is validated to be non-zero at creation,
> > the condition i3cdev->info.static_addr != i3cboardinfo->static_addr will
> > evaluate to 0 != non-zero, which is always true.
> >
> > This means the loop will always continue, making this branch effectively
> > dead code that fails to perform the intended matching.
>
> This branch is harmless and does not affect functionality. It is kept for
> symmetry/clarity and can be cleaned up if preferred.
>
> > >
> > >  		i3cdev->boardinfo = i3cboardinfo;
> > >  		i3cdev->info.static_addr = i3cboardinfo->static_addr;
> >
> > [ ... ]
> >
> > > @@ -2848,9 +2871,16 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
> > >
> > >  	boardinfo->pid = ((u64)reg[1] << 32) | reg[2];
> > >
> > > -	if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
> > > -	    I3C_PID_RND_LOWER_32BITS(boardinfo->pid))
> > > -		return -EINVAL;
> > > +	/* For SETAASA devices, validate the static address instead of PID */
> > > +	if (boardinfo->static_addr_method & I3C_ADDR_METHOD_SETAASA) {
> > > +		if (!boardinfo->static_addr)
> > > +			return -EINVAL;
> > > +	} else {
> > > +		if (!boardinfo->pid ||
> > > +		    (boardinfo->pid & GENMASK_ULL(63, 48)) ||
> > > +		    I3C_PID_RND_LOWER_32BITS(boardinfo->pid))
> > > +			return -EINVAL;
> > > +	}
> >
> > [ ... ]
> >
> > > @@ -2873,10 +2903,10 @@ static int i3c_master_add_of_dev(struct i3c_master_controller *master,
> > > -	if (!reg[1])
> > > +	if (!reg[1] && !fwnode_property_present(fwnode, "mipi-i3c-static-method"))
> > >  		ret = i3c_master_add_i2c_boardinfo(master, fwnode, reg);
> > >  	else
> > >  		ret = i3c_master_add_i3c_boardinfo(master, fwnode, reg);
> >
> > [Severity: Medium]
> > Could this change allow devices requiring a PID to be registered with an
> > invalid Manufacturer ID of 0?
> >
> > By permitting reg[1] == 0 when mipi-i3c-static-method is present in
> > i3c_master_add_of_dev(), it appears we can bypass the strict Manufacturer ID
> > validation in i3c_master_add_i3c_boardinfo().
> >
> > If the method is SETDASA, which still requires a PID, boardinfo->pid is set
> > to ((u64)0 << 32) | reg[2].  If reg[2] is non-zero, !boardinfo->pid is false.
> >
> > Would this result in successfully registering the device despite the zero
> > Manufacturer ID, violating the MIPI I3C specification?
>
> This is only if the reg property is intentionally malformed. If needed,
> '!boardinfo->pid' can be replaced with !I3C_PID_MANUF_ID(pid) to fix this.

Yes, try it

Frank

>
> Best Regards,
> Akhil

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