Re: [PATCH v3 03/11] i3c: master: Extend address status bit to 4 and add I3C_ADDR_SLOT_EXT_INIT

Frank Li
2024-08-26T15:57:40Z

On Mon, Aug 26, 2024 at 10:04:30AM +0200, Miquel Raynal wrote:
> Hi Frank,
>
> Frank.li@nxp.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