linux-kernel - Re: [PATCH v3 03/11] i3c: master: Extend address status bit to 4 and add I3C_ADDR_SLOT_EXT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zsylya9TN4mVFL79@lizhi-Precision-Tower-5810>
Date: Mon, 26 Aug 2024 11:56:57 -0400
From: Frank Li <Frank.li@....com>
To: Miquel Raynal <miquel.raynal@...tlin.com>
Cc: Alexandre Belloni <alexandre.belloni@...tlin.com>,
	Boris Brezillon <boris.brezillon@...labora.com>,
	Parshuram Thombare <pthombar@...ence.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Boris Brezillon <bbrezillon@...nel.org>,
	Arnd Bergmann <arnd@...db.de>,
	Conor Culhane <conor.culhane@...vaco.com>,
	linux-i3c@...ts.infradead.org, linux-kernel@...r.kernel.org,
	imx@...ts.linux.dev
Subject: Re: [PATCH v3 03/11] i3c: master: Extend address status bit to 4 and
 add I3C_ADDR_SLOT_EXT_INIT

On Mon, Aug 26, 2024 at 10:04:30AM +0200, Miquel Raynal wrote:
> Hi Frank,
>
> Frank.li@....com wrote on Fri, 23 Aug 2024 13:55:15 -0400:
>
> > On Fri, Aug 23, 2024 at 06:04:26PM +0200, Miquel Raynal wrote:
> > > Hi Frank,
> > >
> > >
> > > >  static bool i3c_bus_dev_addr_is_avail(struct i3c_bus *bus, u8 addr)
> > > >  {
> > > >  	enum i3c_addr_slot_status status;
> > > > @@ -388,6 +405,14 @@ static int i3c_bus_get_free_addr(struct i3c_bus *bus, u8 start_addr)
> > > >  	enum i3c_addr_slot_status status;
> > > >  	u8 addr;
> > > >
> > > > +	/* try find an address, which have not pre-allocated by assigned-address */
> > >
> > > 	Try	to find			has   been
> > >
> > > pre-allocated?
> > >
> > > > +	for (addr = start_addr; addr < I3C_MAX_ADDR; addr++) {
> > > > +		status = i3c_bus_get_addr_slot_status_ext(bus, addr);
> > > > +		if (status == I3C_ADDR_SLOT_FREE)
> > > > +			return addr;
> > > > +	}
> > > > +
> > > > +	/* use pre-allocoated by assigned-address because such device was removed at bus*/
> > >
> > > 	  Use      allocated
> > >
> > > pre-allocated or assigned?
> > >
> > > I guess the logic should be:
> > > - try the assigned-address
> > > - look for a free slot
> > > - look for an already in use slot that must concern a disconnected
> > >   device
> > >
> > > But the comments are not precise enough IMHO. Can you rephrase them?
> >
> > How about:
> >
> > Follow the steps below to obtain the I3C dynamic address:
> >
> > 1. Retrieve the assigned-address from the device tree (DT).
>
> I guess here you mean that you try to pick that address, unless if
> already in use on the bus, right?
>

Sorry, It should be typo. See below


> > 2. Look for an available slot address.
>
> "available address slot"?
>
> > 3. Look for an address that is pre-reserved by another device with
> > assigned-address in DT, but where the device is currently offline.
>
> I don't think this part is precise enough. You don't look for addresses
> "pre-reserved" but rather more for busy address slots, which might no
> longer be in-use because the device is currently offline. The fact that
> the slot might have been pre-reserved in the DT is a detail that may,
> in some cases, not be true. And as far as I understand your changes,
> you're not checking the DT addresses but rather more the addresses that
> have been allocated live (which is anyway better, because i3c might
> very well be used on a !OF platform).
>
> Once we settle on a description, maybe this can be part of the kdoc for
> the main function searching for the best dynamic address?

I am not sure I understand what's your means here. The current framework
is

1. Get a free address first, the get more infromation from devices, like
BCR, DCR ...
2. Check if it is old device, or dt node have assigned-address property.
3. if it is old device, switch to use old address (if old address is free)
according to i3c spec. If dt pre-reserved address is free, switch to use
dt pre-reserved address.

To match 3's requirement as much as possible, when 1 return address, which
should avoid return dt's assigned-address.


/*
 * I3C Framework Address Assignment Flow:
 * 1 Initial Address Assignment: Attempt to obtain a free address first,
then gather additional information such as PID, BCR, and DRCR
 * 2 Address switch:
       - 2a: Check if this target device is appeared before. Switch
to use prevous address of this device.
 *     - 2b: Check if this target device have assigned-address property in dt,
switch to this address if it is free.
 *
In step 1, i3c_bus_get_free_addr() is called. To optimize for step 2b, this
function should return an address that is not pre-reserved by any target
device with an assigned address in the device tree (DT). If no such address
is available, it can return an address that was pre-reserved by a target
device that is currently offline.

 */

>
> Thanks,
> Miquèl