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: <20240826100430.33194702@xps-13>
Date: Mon, 26 Aug 2024 10:04:30 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Frank Li <Frank.li@....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

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?

> 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?

Thanks,
Miquèl