| Message ID | 20240819-i3c_fix-v3-3-7d69f7b0a05e@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i3c: master: some fix and improvemnt for hotjoin | expand |
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? > for (addr = start_addr; addr < I3C_MAX_ADDR; addr++) { > status = i3c_bus_get_addr_slot_status(bus, addr); > if (status == I3C_ADDR_SLOT_FREE) > @@ -1906,9 +1931,9 @@ static int i3c_master_bus_init(struct i3c_master_controller *master) > goto err_rstdaa; > } > > - i3c_bus_set_addr_slot_status(&master->bus, > - i3cboardinfo->init_dyn_addr, > - I3C_ADDR_SLOT_I3C_DEV); > + i3c_bus_set_addr_slot_status_ext(&master->bus, > + i3cboardinfo->init_dyn_addr, > + I3C_ADDR_SLOT_I3C_DEV | I3C_ADDR_SLOT_EXT_INIT); > > /* > * Only try to create/attach devices that have a static > diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h > index 4601b6957f799..c923b76bbc321 100644 > --- a/include/linux/i3c/master.h > +++ b/include/linux/i3c/master.h > @@ -284,6 +284,8 @@ enum i3c_bus_mode { > * @I3C_ADDR_SLOT_I2C_DEV: address is assigned to an I2C device > * @I3C_ADDR_SLOT_I3C_DEV: address is assigned to an I3C device > * @I3C_ADDR_SLOT_STATUS_MASK: address slot mask > + * @I3C_ADDR_SLOT_EXT_INIT: the bit mask display of addresses is preferred by some devices, I'm sorry, but I don't understand what "bit mask display of addresses" means. > + * such as the "assigned-address" in device tree source (dts). > * > * On an I3C bus, addresses are assigned dynamically, and we need to know which > * addresses are free to use and which ones are already assigned. > @@ -297,9 +299,11 @@ enum i3c_addr_slot_status { > I3C_ADDR_SLOT_I2C_DEV, > I3C_ADDR_SLOT_I3C_DEV, > I3C_ADDR_SLOT_STATUS_MASK = 3, > + I3C_ADDR_SLOT_EXT_STATUS_MASK = 7, > + I3C_ADDR_SLOT_EXT_INIT = BIT(2), > }; > > -#define I3C_ADDR_SLOT_BITS 2 > +#define I3C_ADDR_SLOT_BITS 4 > > /** > * struct i3c_bus - I3C bus object > Thanks, Miquèl
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). 2. Look for an available slot address. 3. Look for an address that is pre-reserved by another device with assigned-address in DT, but where the device is currently offline. > > > for (addr = start_addr; addr < I3C_MAX_ADDR; addr++) { > > status = i3c_bus_get_addr_slot_status(bus, addr); > > if (status == I3C_ADDR_SLOT_FREE) > > @@ -1906,9 +1931,9 @@ static int i3c_master_bus_init(struct i3c_master_controller *master) > > goto err_rstdaa; > > } > > > > - i3c_bus_set_addr_slot_status(&master->bus, > > - i3cboardinfo->init_dyn_addr, > > - I3C_ADDR_SLOT_I3C_DEV); > > + i3c_bus_set_addr_slot_status_ext(&master->bus, > > + i3cboardinfo->init_dyn_addr, > > + I3C_ADDR_SLOT_I3C_DEV | I3C_ADDR_SLOT_EXT_INIT); > > > > /* > > * Only try to create/attach devices that have a static > > diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h > > index 4601b6957f799..c923b76bbc321 100644 > > --- a/include/linux/i3c/master.h > > +++ b/include/linux/i3c/master.h > > @@ -284,6 +284,8 @@ enum i3c_bus_mode { > > * @I3C_ADDR_SLOT_I2C_DEV: address is assigned to an I2C device > > * @I3C_ADDR_SLOT_I3C_DEV: address is assigned to an I3C device > > * @I3C_ADDR_SLOT_STATUS_MASK: address slot mask > > + * @I3C_ADDR_SLOT_EXT_INIT: the bit mask display of addresses is preferred by some devices, > > I'm sorry, but I don't understand what "bit mask display of addresses" > means. > > > + * such as the "assigned-address" in device tree source (dts). > > * > > * On an I3C bus, addresses are assigned dynamically, and we need to know which > > * addresses are free to use and which ones are already assigned. > > @@ -297,9 +299,11 @@ enum i3c_addr_slot_status { > > I3C_ADDR_SLOT_I2C_DEV, > > I3C_ADDR_SLOT_I3C_DEV, > > I3C_ADDR_SLOT_STATUS_MASK = 3, > > + I3C_ADDR_SLOT_EXT_STATUS_MASK = 7, > > + I3C_ADDR_SLOT_EXT_INIT = BIT(2), > > }; > > > > -#define I3C_ADDR_SLOT_BITS 2 > > +#define I3C_ADDR_SLOT_BITS 4 > > > > /** > > * struct i3c_bus - I3C bus object > > > > > Thanks, > Miquèl
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? > 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
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
Hi Frank, Frank.li@nxp.com wrote on Mon, 26 Aug 2024 11:56:57 -0400: > 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: f a a f > * 1 Initial Address Assignment: Attempt to obtain a free address first, 1. a a > then gather additional information such as PID, BCR, and DRCR > * 2 Address switch: 2. > - 2a: Check if this target device is appeared before. Switch > to use prevous address of this device. the previous for > * - 2b: Check if this target device have assigned-address property in dt, has a preferred address based on firmware data (DT). Switch to it if it is the case and the address is free. Today it's the DT, maybe not tomorrow. You take these values from the firmware on the board, that feels more generic than talking about a DT property name. > 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). This does not make sense, if you want to optimize for 2b, why not selecting the assigned-address property in the first place if it's available? Also, I don't understand why you would care to specifically *not* return an address that might be the default one for another device in the first place. Changing to a free slot (if possible) not reserved by another device might be done in 2b, which makes operation 1 much more simple, so you put all the complexity at the same time. Even if you are proceeding with two devices asking the DAA at the same time, the procedure should work in a way which does not impact the fact that the second device will get its desired address if the first can take another one. > 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
On Mon, Aug 26, 2024 at 06:49:24PM +0200, Miquel Raynal wrote: > Hi Frank, > > Frank.li@nxp.com wrote on Mon, 26 Aug 2024 11:56:57 -0400: > > > 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: > > f a a f > > > * 1 Initial Address Assignment: Attempt to obtain a free address first, > > 1. a a > > > then gather additional information such as PID, BCR, and DRCR > > * 2 Address switch: > > 2. > > > - 2a: Check if this target device is appeared before. Switch > > to use prevous address of this device. > > the previous for > > > * - 2b: Check if this target device have assigned-address property in dt, > > has a preferred address based on > firmware data (DT). Switch to it if > it is the case and the address is > free. > > Today it's the DT, maybe not tomorrow. You take these values from the > firmware on the board, that feels more generic than talking about a DT > property name. > > > 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). > > This does not make sense, if you want to optimize for 2b, why not > selecting the assigned-address property in the first place if it's > available? This is my first idea. But I gived up this way. Select an assigned-address here will involve a big change in i3c framework. There are no PID information in i3c_master_get_free_addr(). In DAA flow: - SVC is get PID first, the get_free_addr(). This case, we can use PID to get dt assigned address.(if change/add API) - But HCI, it is difference, hci_cmd_v2_daa(), get_free_addr() firstly then send out DAA command. So no PID information when call get_free_addr(). To cover both case, return a *real* free address here is simplest solution. > Also, I don't understand why you would care to specifically > *not* return an address that might be the default one for another > device in the first place. If devices A (want adddress 0xA), device B (want address 0xB), if both device send hot join at the same time. device B's PID less than device A, Device B will be found firstly, call get_free_addr(), 0xA will be return if no this patch. Device A, call try_get_freeaddr() to get 0xB. So Devcie B will be assign to 0xA, and Device A will be assign to address 0xB. After do_daa command, framework will add device B and device A into i3c bus. When framework try to add device B to i3c bus, framework will try switch device B's address to 0xB from 0xA, but it will be fail because 0xB already assigned to device A. > Changing to a free slot (if possible) not > reserved by another device might be done in 2b, which makes operation 1 > much more simple, so you put all the complexity at the same time. I am sure if I understand your means here. I put all address (not reserved by another device) to free slot. reserved addr by another devices but offline, should be used only when not free slot avaible. > Even > if you are proceeding with two devices asking the DAA at the same time, > the procedure should work in a way which does not impact the fact that > the second device will get its desired address if the first can take > another one. I am sure if this is still validate if you understand HCI case. Frank > > > 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
Hi Frank, > > > 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). > > > > This does not make sense, if you want to optimize for 2b, why not > > selecting the assigned-address property in the first place if it's > > available? > > This is my first idea. But I gived up this way. > > Select an assigned-address here will involve a big change in i3c framework. > There are no PID information in i3c_master_get_free_addr(). > > In DAA flow: > - SVC is get PID first, the get_free_addr(). This case, we can use PID to > get dt assigned address.(if change/add API) > - But HCI, it is difference, hci_cmd_v2_daa(), get_free_addr() firstly then > send out DAA command. So no PID information when call get_free_addr(). > > To cover both case, return a *real* free address here is simplest solution. But this is a limitation of the HCI driver? So why not addressing this in the HCI driver instead? It would greatly simplify the core logic which becomes complex for wrong reasons. > > Also, I don't understand why you would care to specifically > > *not* return an address that might be the default one for another > > device in the first place. > > If devices A (want adddress 0xA), device B (want address 0xB), if both > device send hot join at the same time. device B's PID less than device A, > > Device B will be found firstly, call get_free_addr(), 0xA will be return > if no this patch. > > Device A, call try_get_freeaddr() to get 0xB. > > So Devcie B will be assign to 0xA, and Device A will be assign to address 0xB. > > After do_daa command, framework will add device B and device A into i3c bus. > > When framework try to add device B to i3c bus, framework will try switch > device B's address to 0xB from 0xA, but it will be fail because 0xB already > assigned to device A. Well, okay, but that's exactly the situation that will happen if these devices are not described in your DT. I guess it's expected that a device not described in your DT can be connected, thanks to the hot-join feature. In this case you cannot know what is this device preferred address and you might end-up in the exact same situation. May I question the need for preferred addresses at all? Is this even part of the spec? What is the use-case? Thanks, Miquèl
On Mon, Sep 02, 2024 at 04:12:50PM +0200, Miquel Raynal wrote: > Hi Frank, > > > > > 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). > > > > > > This does not make sense, if you want to optimize for 2b, why not > > > selecting the assigned-address property in the first place if it's > > > available? > > > > This is my first idea. But I gived up this way. > > > > Select an assigned-address here will involve a big change in i3c framework. > > There are no PID information in i3c_master_get_free_addr(). > > > > In DAA flow: > > - SVC is get PID first, the get_free_addr(). This case, we can use PID to > > get dt assigned address.(if change/add API) > > - But HCI, it is difference, hci_cmd_v2_daa(), get_free_addr() firstly then > > send out DAA command. So no PID information when call get_free_addr(). > > > > To cover both case, return a *real* free address here is simplest solution. > > But this is a limitation of the HCI driver? So why not addressing this > in the HCI driver instead? It would greatly simplify the core logic > which becomes complex for wrong reasons. It is reasonable requirement to reduce stall SCL time. After get PID, SCL have to stall low to wait for software get dynamtic address, I3C spec allow relative long time for this, but still better if hardware can send out PID and dynamatic address together withoug stall SCL low. Pre-alloc adddress is good method if consider this. > > > > Also, I don't understand why you would care to specifically > > > *not* return an address that might be the default one for another > > > device in the first place. > > > > If devices A (want adddress 0xA), device B (want address 0xB), if both > > device send hot join at the same time. device B's PID less than device A, > > > > Device B will be found firstly, call get_free_addr(), 0xA will be return > > if no this patch. > > > > Device A, call try_get_freeaddr() to get 0xB. > > > > So Devcie B will be assign to 0xA, and Device A will be assign to address 0xB. > > > > After do_daa command, framework will add device B and device A into i3c bus. > > > > When framework try to add device B to i3c bus, framework will try switch > > device B's address to 0xB from 0xA, but it will be fail because 0xB already > > assigned to device A. > > Well, okay, but that's exactly the situation that will happen if these > devices are not described in your DT. I guess it's expected that a > device not described in your DT can be connected, thanks to the > hot-join feature. In this case you cannot know what is this device > preferred address and you might end-up in the exact same situation. If not descript in DT, it means that any dynmatic address can be assigned. > > May I question the need for preferred addresses at all? Is this even > part of the spec? What is the use-case? It is implements detail. I3C spec said lower dynamtic address have high IBI priority. Spec just said assign lower dynamtic address if want to higher IBI prioerity. Using DT assign-address just is one implement method. > > Thanks, > Miquèl
Hi Frank, Frank.li@nxp.com wrote on Mon, 2 Sep 2024 14:20:51 -0400: > On Mon, Sep 02, 2024 at 04:12:50PM +0200, Miquel Raynal wrote: > > Hi Frank, > > > > > > > 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). > > > > > > > > This does not make sense, if you want to optimize for 2b, why not > > > > selecting the assigned-address property in the first place if it's > > > > available? > > > > > > This is my first idea. But I gived up this way. > > > > > > Select an assigned-address here will involve a big change in i3c framework. > > > There are no PID information in i3c_master_get_free_addr(). > > > > > > In DAA flow: > > > - SVC is get PID first, the get_free_addr(). This case, we can use PID to > > > get dt assigned address.(if change/add API) > > > - But HCI, it is difference, hci_cmd_v2_daa(), get_free_addr() firstly then > > > send out DAA command. So no PID information when call get_free_addr(). > > > > > > To cover both case, return a *real* free address here is simplest solution. > > > > But this is a limitation of the HCI driver? So why not addressing this > > in the HCI driver instead? It would greatly simplify the core logic > > which becomes complex for wrong reasons. > > It is reasonable requirement to reduce stall SCL time. After get PID, SCL > have to stall low to wait for software get dynamtic address, I3C spec allow > relative long time for this, but still better if hardware can send out PID > and dynamatic address together withoug stall SCL low. Pre-alloc adddress is > good method if consider this. I don't think it is worth the trouble, given the complexity of all the changes. I prefer to simplify a bit the software and keep it readable than gaining few us with SCL low. In this case you also spend more time on the configuration I guess, so is it better than keeping SCL low (it will be low for some time anyway). > > > > Also, I don't understand why you would care to specifically > > > > *not* return an address that might be the default one for another > > > > device in the first place. > > > > > > If devices A (want adddress 0xA), device B (want address 0xB), if both > > > device send hot join at the same time. device B's PID less than device A, > > > > > > Device B will be found firstly, call get_free_addr(), 0xA will be return > > > if no this patch. > > > > > > Device A, call try_get_freeaddr() to get 0xB. > > > > > > So Devcie B will be assign to 0xA, and Device A will be assign to address 0xB. > > > > > > After do_daa command, framework will add device B and device A into i3c bus. > > > > > > When framework try to add device B to i3c bus, framework will try switch > > > device B's address to 0xB from 0xA, but it will be fail because 0xB already > > > assigned to device A. > > > > Well, okay, but that's exactly the situation that will happen if these > > devices are not described in your DT. I guess it's expected that a > > device not described in your DT can be connected, thanks to the > > hot-join feature. In this case you cannot know what is this device > > preferred address and you might end-up in the exact same situation. > > If not descript in DT, it means that any dynmatic address can be assigned. That's the point of view of the host. But a device might be "critical" and expect a low address, but the host not being aware. This is the same situation as your A and B conflict example. > > May I question the need for preferred addresses at all? Is this even > > part of the spec? What is the use-case? > > It is implements detail. I3C spec said lower dynamtic address have high IBI > priority. Spec just said assign lower dynamtic address if want to higher > IBI prioerity. Using DT assign-address just is one implement method. Thanks for all the information, for me the HCI driver must be modified to retrieve the PID before assigning the dynamic address. Thanks, Miquèl
On Tue, Sep 03, 2024 at 03:00:38PM +0200, Miquel Raynal wrote: > Hi Frank, > > Frank.li@nxp.com wrote on Mon, 2 Sep 2024 14:20:51 -0400: > > > On Mon, Sep 02, 2024 at 04:12:50PM +0200, Miquel Raynal wrote: > > > Hi Frank, > > > > > > > > > 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). > > > > > > > > > > This does not make sense, if you want to optimize for 2b, why not > > > > > selecting the assigned-address property in the first place if it's > > > > > available? > > > > > > > > This is my first idea. But I gived up this way. > > > > > > > > Select an assigned-address here will involve a big change in i3c framework. > > > > There are no PID information in i3c_master_get_free_addr(). > > > > > > > > In DAA flow: > > > > - SVC is get PID first, the get_free_addr(). This case, we can use PID to > > > > get dt assigned address.(if change/add API) > > > > - But HCI, it is difference, hci_cmd_v2_daa(), get_free_addr() firstly then > > > > send out DAA command. So no PID information when call get_free_addr(). > > > > > > > > To cover both case, return a *real* free address here is simplest solution. > > > > > > But this is a limitation of the HCI driver? So why not addressing this > > > in the HCI driver instead? It would greatly simplify the core logic > > > which becomes complex for wrong reasons. > > > > It is reasonable requirement to reduce stall SCL time. After get PID, SCL > > have to stall low to wait for software get dynamtic address, I3C spec allow > > relative long time for this, but still better if hardware can send out PID > > and dynamatic address together withoug stall SCL low. Pre-alloc adddress is > > good method if consider this. > > I don't think it is worth the trouble, given the complexity of all > the changes. I prefer to simplify a bit the software and keep it > readable than gaining few us with SCL low. In this case you also spend > more time on the configuration I guess, so is it better than keeping > SCL low (it will be low for some time anyway). Yes, but see below about HCI. But two solutions will be worse. > > > > > > Also, I don't understand why you would care to specifically > > > > > *not* return an address that might be the default one for another > > > > > device in the first place. > > > > > > > > If devices A (want adddress 0xA), device B (want address 0xB), if both > > > > device send hot join at the same time. device B's PID less than device A, > > > > > > > > Device B will be found firstly, call get_free_addr(), 0xA will be return > > > > if no this patch. > > > > > > > > Device A, call try_get_freeaddr() to get 0xB. > > > > > > > > So Devcie B will be assign to 0xA, and Device A will be assign to address 0xB. > > > > > > > > After do_daa command, framework will add device B and device A into i3c bus. > > > > > > > > When framework try to add device B to i3c bus, framework will try switch > > > > device B's address to 0xB from 0xA, but it will be fail because 0xB already > > > > assigned to device A. > > > > > > Well, okay, but that's exactly the situation that will happen if these > > > devices are not described in your DT. I guess it's expected that a > > > device not described in your DT can be connected, thanks to the > > > hot-join feature. In this case you cannot know what is this device > > > preferred address and you might end-up in the exact same situation. > > > > If not descript in DT, it means that any dynmatic address can be assigned. > > That's the point of view of the host. But a device might be "critical" > and expect a low address, but the host not being aware. This is the > same situation as your A and B conflict example. DT provided addtional information to let host aware it. > > > > May I question the need for preferred addresses at all? Is this even > > > part of the spec? What is the use-case? > > > > It is implements detail. I3C spec said lower dynamtic address have high IBI > > priority. Spec just said assign lower dynamtic address if want to higher > > IBI prioerity. Using DT assign-address just is one implement method. > > Thanks for all the information, for me the HCI driver must be modified > to retrieve the PID before assigning the dynamic address. I am not famillar with HCI, but according to I3C HCI Spec 1.2, sec 6.4.1 Dynamic Address Assignment with ENTDAA: I think it is impossible to do that. A dynatimic address must be provided before issue ENTDAA command. HCI is MIPI I3C standard defined Host interface. we have to consider this. Frank > > Thanks, > Miquèl
On Tue, Sep 03, 2024 at 11:06:30AM -0400, Frank Li wrote: > On Tue, Sep 03, 2024 at 03:00:38PM +0200, Miquel Raynal wrote: > > Hi Frank, > > > > Frank.li@nxp.com wrote on Mon, 2 Sep 2024 14:20:51 -0400: > > > > > On Mon, Sep 02, 2024 at 04:12:50PM +0200, Miquel Raynal wrote: > > > > Hi Frank, > > > > > > > > > > > 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). > > > > > > > > > > > > This does not make sense, if you want to optimize for 2b, why not > > > > > > selecting the assigned-address property in the first place if it's > > > > > > available? > > > > > > > > > > This is my first idea. But I gived up this way. > > > > > > > > > > Select an assigned-address here will involve a big change in i3c framework. > > > > > There are no PID information in i3c_master_get_free_addr(). > > > > > > > > > > In DAA flow: > > > > > - SVC is get PID first, the get_free_addr(). This case, we can use PID to > > > > > get dt assigned address.(if change/add API) > > > > > - But HCI, it is difference, hci_cmd_v2_daa(), get_free_addr() firstly then > > > > > send out DAA command. So no PID information when call get_free_addr(). > > > > > > > > > > To cover both case, return a *real* free address here is simplest solution. > > > > > > > > But this is a limitation of the HCI driver? So why not addressing this > > > > in the HCI driver instead? It would greatly simplify the core logic > > > > which becomes complex for wrong reasons. > > > > > > It is reasonable requirement to reduce stall SCL time. After get PID, SCL > > > have to stall low to wait for software get dynamtic address, I3C spec allow > > > relative long time for this, but still better if hardware can send out PID > > > and dynamatic address together withoug stall SCL low. Pre-alloc adddress is > > > good method if consider this. > > > > I don't think it is worth the trouble, given the complexity of all > > the changes. I prefer to simplify a bit the software and keep it > > readable than gaining few us with SCL low. In this case you also spend > > more time on the configuration I guess, so is it better than keeping > > SCL low (it will be low for some time anyway). > > Yes, but see below about HCI. But two solutions will be worse. > > > > > > > > > Also, I don't understand why you would care to specifically > > > > > > *not* return an address that might be the default one for another > > > > > > device in the first place. > > > > > > > > > > If devices A (want adddress 0xA), device B (want address 0xB), if both > > > > > device send hot join at the same time. device B's PID less than device A, > > > > > > > > > > Device B will be found firstly, call get_free_addr(), 0xA will be return > > > > > if no this patch. > > > > > > > > > > Device A, call try_get_freeaddr() to get 0xB. > > > > > > > > > > So Devcie B will be assign to 0xA, and Device A will be assign to address 0xB. > > > > > > > > > > After do_daa command, framework will add device B and device A into i3c bus. > > > > > > > > > > When framework try to add device B to i3c bus, framework will try switch > > > > > device B's address to 0xB from 0xA, but it will be fail because 0xB already > > > > > assigned to device A. > > > > > > > > Well, okay, but that's exactly the situation that will happen if these > > > > devices are not described in your DT. I guess it's expected that a > > > > device not described in your DT can be connected, thanks to the > > > > hot-join feature. In this case you cannot know what is this device > > > > preferred address and you might end-up in the exact same situation. > > > > > > If not descript in DT, it means that any dynmatic address can be assigned. > > > > That's the point of view of the host. But a device might be "critical" > > and expect a low address, but the host not being aware. This is the > > same situation as your A and B conflict example. > > DT provided addtional information to let host aware it. > > > > > > > May I question the need for preferred addresses at all? Is this even > > > > part of the spec? What is the use-case? > > > > > > It is implements detail. I3C spec said lower dynamtic address have high IBI > > > priority. Spec just said assign lower dynamtic address if want to higher > > > IBI prioerity. Using DT assign-address just is one implement method. > > > > Thanks for all the information, for me the HCI driver must be modified > > to retrieve the PID before assigning the dynamic address. > > I am not famillar with HCI, but according to I3C HCI Spec 1.2, sec 6.4.1 > Dynamic Address Assignment with ENTDAA: > > I think it is impossible to do that. A dynatimic address must be provided > before issue ENTDAA command. HCI is MIPI I3C standard defined Host > interface. we have to consider this. Miquèl: Do you have any additional comments for this? Frank > > Frank > > > > Thanks, > > Miquèl
On Mon, Sep 09, 2024 at 04:01:32PM -0400, Frank Li wrote: > On Tue, Sep 03, 2024 at 11:06:30AM -0400, Frank Li wrote: > > On Tue, Sep 03, 2024 at 03:00:38PM +0200, Miquel Raynal wrote: > > > Hi Frank, > > > > > > Frank.li@nxp.com wrote on Mon, 2 Sep 2024 14:20:51 -0400: > > > > > > > On Mon, Sep 02, 2024 at 04:12:50PM +0200, Miquel Raynal wrote: > > > > > Hi Frank, > > > > > > > > > > > > > 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). > > > > > > > > > > > > > > This does not make sense, if you want to optimize for 2b, why not > > > > > > > selecting the assigned-address property in the first place if it's > > > > > > > available? > > > > > > > > > > > > This is my first idea. But I gived up this way. > > > > > > > > > > > > Select an assigned-address here will involve a big change in i3c framework. > > > > > > There are no PID information in i3c_master_get_free_addr(). > > > > > > > > > > > > In DAA flow: > > > > > > - SVC is get PID first, the get_free_addr(). This case, we can use PID to > > > > > > get dt assigned address.(if change/add API) > > > > > > - But HCI, it is difference, hci_cmd_v2_daa(), get_free_addr() firstly then > > > > > > send out DAA command. So no PID information when call get_free_addr(). > > > > > > > > > > > > To cover both case, return a *real* free address here is simplest solution. > > > > > > > > > > But this is a limitation of the HCI driver? So why not addressing this > > > > > in the HCI driver instead? It would greatly simplify the core logic > > > > > which becomes complex for wrong reasons. > > > > > > > > It is reasonable requirement to reduce stall SCL time. After get PID, SCL > > > > have to stall low to wait for software get dynamtic address, I3C spec allow > > > > relative long time for this, but still better if hardware can send out PID > > > > and dynamatic address together withoug stall SCL low. Pre-alloc adddress is > > > > good method if consider this. > > > > > > I don't think it is worth the trouble, given the complexity of all > > > the changes. I prefer to simplify a bit the software and keep it > > > readable than gaining few us with SCL low. In this case you also spend > > > more time on the configuration I guess, so is it better than keeping > > > SCL low (it will be low for some time anyway). > > > > Yes, but see below about HCI. But two solutions will be worse. > > > > > > > > > > > > Also, I don't understand why you would care to specifically > > > > > > > *not* return an address that might be the default one for another > > > > > > > device in the first place. > > > > > > > > > > > > If devices A (want adddress 0xA), device B (want address 0xB), if both > > > > > > device send hot join at the same time. device B's PID less than device A, > > > > > > > > > > > > Device B will be found firstly, call get_free_addr(), 0xA will be return > > > > > > if no this patch. > > > > > > > > > > > > Device A, call try_get_freeaddr() to get 0xB. > > > > > > > > > > > > So Devcie B will be assign to 0xA, and Device A will be assign to address 0xB. > > > > > > > > > > > > After do_daa command, framework will add device B and device A into i3c bus. > > > > > > > > > > > > When framework try to add device B to i3c bus, framework will try switch > > > > > > device B's address to 0xB from 0xA, but it will be fail because 0xB already > > > > > > assigned to device A. > > > > > > > > > > Well, okay, but that's exactly the situation that will happen if these > > > > > devices are not described in your DT. I guess it's expected that a > > > > > device not described in your DT can be connected, thanks to the > > > > > hot-join feature. In this case you cannot know what is this device > > > > > preferred address and you might end-up in the exact same situation. > > > > > > > > If not descript in DT, it means that any dynmatic address can be assigned. > > > > > > That's the point of view of the host. But a device might be "critical" > > > and expect a low address, but the host not being aware. This is the > > > same situation as your A and B conflict example. > > > > DT provided addtional information to let host aware it. > > > > > > > > > > May I question the need for preferred addresses at all? Is this even > > > > > part of the spec? What is the use-case? > > > > > > > > It is implements detail. I3C spec said lower dynamtic address have high IBI > > > > priority. Spec just said assign lower dynamtic address if want to higher > > > > IBI prioerity. Using DT assign-address just is one implement method. > > > > > > Thanks for all the information, for me the HCI driver must be modified > > > to retrieve the PID before assigning the dynamic address. > > > > I am not famillar with HCI, but according to I3C HCI Spec 1.2, sec 6.4.1 > > Dynamic Address Assignment with ENTDAA: > > > > I think it is impossible to do that. A dynatimic address must be provided > > before issue ENTDAA command. HCI is MIPI I3C standard defined Host > > interface. we have to consider this. > > Miquèl: > Do you have any additional comments for this? > > Frank Do you have any additional comments? HCI is MIPI I3C standard. we have to consider this. Frank > > > > > Frank > > > > > > Thanks, > > > Miquèl > > -- > linux-i3c mailing list > linux-i3c@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-i3c
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index 85c737554c940..4281f673e08d8 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -345,7 +345,7 @@ const struct bus_type i3c_bus_type = { EXPORT_SYMBOL_GPL(i3c_bus_type); static enum i3c_addr_slot_status -i3c_bus_get_addr_slot_status(struct i3c_bus *bus, u16 addr) +i3c_bus_get_addr_slot_status_ext(struct i3c_bus *bus, u16 addr) { unsigned long status; int bitpos = addr * I3C_ADDR_SLOT_BITS; @@ -356,11 +356,17 @@ i3c_bus_get_addr_slot_status(struct i3c_bus *bus, u16 addr) status = bus->addrslots[bitpos / BITS_PER_LONG]; status >>= bitpos % BITS_PER_LONG; - return status & I3C_ADDR_SLOT_STATUS_MASK; + return status & I3C_ADDR_SLOT_EXT_STATUS_MASK; } -static void i3c_bus_set_addr_slot_status(struct i3c_bus *bus, u16 addr, - enum i3c_addr_slot_status status) +static enum i3c_addr_slot_status +i3c_bus_get_addr_slot_status(struct i3c_bus *bus, u16 addr) +{ + return i3c_bus_get_addr_slot_status_ext(bus, addr) & I3C_ADDR_SLOT_STATUS_MASK; +} + +static void i3c_bus_set_addr_slot_status_mask(struct i3c_bus *bus, u16 addr, + enum i3c_addr_slot_status status, int mask) { int bitpos = addr * I3C_ADDR_SLOT_BITS; unsigned long *ptr; @@ -369,11 +375,22 @@ static void i3c_bus_set_addr_slot_status(struct i3c_bus *bus, u16 addr, return; ptr = bus->addrslots + (bitpos / BITS_PER_LONG); - *ptr &= ~((unsigned long)I3C_ADDR_SLOT_STATUS_MASK << - (bitpos % BITS_PER_LONG)); + *ptr &= ~((unsigned long)mask << (bitpos % BITS_PER_LONG)); *ptr |= (unsigned long)status << (bitpos % BITS_PER_LONG); } +static void i3c_bus_set_addr_slot_status(struct i3c_bus *bus, u16 addr, + enum i3c_addr_slot_status status) +{ + i3c_bus_set_addr_slot_status_mask(bus, addr, status, I3C_ADDR_SLOT_STATUS_MASK); +} + +static void i3c_bus_set_addr_slot_status_ext(struct i3c_bus *bus, u16 addr, + enum i3c_addr_slot_status status) +{ + i3c_bus_set_addr_slot_status_mask(bus, addr, status, I3C_ADDR_SLOT_EXT_STATUS_MASK); +} + 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 */ + 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*/ for (addr = start_addr; addr < I3C_MAX_ADDR; addr++) { status = i3c_bus_get_addr_slot_status(bus, addr); if (status == I3C_ADDR_SLOT_FREE) @@ -1906,9 +1931,9 @@ static int i3c_master_bus_init(struct i3c_master_controller *master) goto err_rstdaa; } - i3c_bus_set_addr_slot_status(&master->bus, - i3cboardinfo->init_dyn_addr, - I3C_ADDR_SLOT_I3C_DEV); + i3c_bus_set_addr_slot_status_ext(&master->bus, + i3cboardinfo->init_dyn_addr, + I3C_ADDR_SLOT_I3C_DEV | I3C_ADDR_SLOT_EXT_INIT); /* * Only try to create/attach devices that have a static diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h index 4601b6957f799..c923b76bbc321 100644 --- a/include/linux/i3c/master.h +++ b/include/linux/i3c/master.h @@ -284,6 +284,8 @@ enum i3c_bus_mode { * @I3C_ADDR_SLOT_I2C_DEV: address is assigned to an I2C device * @I3C_ADDR_SLOT_I3C_DEV: address is assigned to an I3C device * @I3C_ADDR_SLOT_STATUS_MASK: address slot mask + * @I3C_ADDR_SLOT_EXT_INIT: the bit mask display of addresses is preferred by some devices, + * such as the "assigned-address" in device tree source (dts). * * On an I3C bus, addresses are assigned dynamically, and we need to know which * addresses are free to use and which ones are already assigned. @@ -297,9 +299,11 @@ enum i3c_addr_slot_status { I3C_ADDR_SLOT_I2C_DEV, I3C_ADDR_SLOT_I3C_DEV, I3C_ADDR_SLOT_STATUS_MASK = 3, + I3C_ADDR_SLOT_EXT_STATUS_MASK = 7, + I3C_ADDR_SLOT_EXT_INIT = BIT(2), }; -#define I3C_ADDR_SLOT_BITS 2 +#define I3C_ADDR_SLOT_BITS 4 /** * struct i3c_bus - I3C bus object
Extend the address status bit to 4 and introduce the I3C_ADDR_SLOT_EXT_INIT macro to indicate that a device prefers a specific address. This is generally set by the 'assigned-address' in the device tree source (dts) file. When an i3c device is removed from the bus, the old address status is set to FREE, allowing other devices to use the address during hotjoin. The I3C_ADDR_SLOT_EXT_INIT status indicates that an address is preferred by some devices. The function i3c_bus_get_free_addr() will first attempt to use unassigned addresses before searching for assigned addresses of devices that have been removed from the bus, trying to best match the 'assigned-address'. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- drivers/i3c/master.c | 43 ++++++++++++++++++++++++++++++++++--------- include/linux/i3c/master.h | 6 +++++- 2 files changed, 39 insertions(+), 10 deletions(-)