| Message ID | 20191210101502.8401-3-pgaj@cadence.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | I3C device addresing adjustments | expand |
On Tue, 10 Dec 2019 11:14:58 +0100 Przemysław Gaj <pgaj@cadence.com> wrote: > It may be the case that SETDASA fails for some reason. Reserve > ->init_dyn_addr when it's defined to prevent assigning this address > to another slave device. This way when device shows up we don't > have to re-assign addresses. > > Signed-off-by: Przemysław Gaj <pgaj@cadence.com> > --- > drivers/i3c/master.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > index 5c06c41e6660..fab6e0609fca 100755 > --- a/drivers/i3c/master.c > +++ b/drivers/i3c/master.c > @@ -1263,7 +1263,8 @@ static void i3c_master_put_i3c_addrs(struct i3c_dev_desc *dev) > I3C_ADDR_SLOT_FREE); > > if (dev->boardinfo && dev->boardinfo->init_dyn_addr) > - i3c_bus_set_addr_slot_status(&master->bus, dev->info.dyn_addr, > + i3c_bus_set_addr_slot_status(&master->bus, > + dev->boardinfo->init_dyn_addr, > I3C_ADDR_SLOT_FREE); Should be fixed in a separate patch, or at least mentioned in the commit message. > } > > @@ -1675,6 +1676,11 @@ static int i3c_master_bus_init(struct i3c_master_controller *master) > ret = -EBUSY; > goto err_detach_devs; > } > + > + /* Reserve the slot. */ > + i3c_bus_set_addr_slot_status(&master->bus, > + i3cboardinfo->init_dyn_addr, > + I3C_ADDR_SLOT_I3C_DEV); Looks like the "reserve/release init_dyn_addr slot" logic is asymmetric. I wonder if that's not a problem. Can't we end up in a situation where the ->init_dyn_addr is released (when SETDASA() fails) and then re-used without being reserved (when the device is later discovered through a regular DAA)? So maybe I was wrong and we should indeed reserve the address in the i3c_master_get_i3c_addrs() path. > } > > i3cdev = i3c_master_alloc_i3c_dev(master, &info);
Hi Przemysław, From: Przemysław Gaj <pgaj@cadence.com> Date: Tue, Dec 10, 2019 at 10:14:58 > It may be the case that SETDASA fails for some reason. Reserve > ->init_dyn_addr when it's defined to prevent assigning this address > to another slave device. This way when device shows up we don't > have to re-assign addresses. > > Signed-off-by: Przemysław Gaj <pgaj@cadence.com> > --- > drivers/i3c/master.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > index 5c06c41e6660..fab6e0609fca 100755 > --- a/drivers/i3c/master.c > +++ b/drivers/i3c/master.c > @@ -1263,7 +1263,8 @@ static void i3c_master_put_i3c_addrs(struct i3c_dev_desc *dev) > I3C_ADDR_SLOT_FREE); > > if (dev->boardinfo && dev->boardinfo->init_dyn_addr) > - i3c_bus_set_addr_slot_status(&master->bus, dev->info.dyn_addr, > + i3c_bus_set_addr_slot_status(&master->bus, > + dev->boardinfo->init_dyn_addr, > I3C_ADDR_SLOT_FREE); > } > > @@ -1675,6 +1676,11 @@ static int i3c_master_bus_init(struct i3c_master_controller *master) > ret = -EBUSY; > goto err_detach_devs; > } > + > + /* Reserve the slot. */ > + i3c_bus_set_addr_slot_status(&master->bus, > + i3cboardinfo->init_dyn_addr, > + I3C_ADDR_SLOT_I3C_DEV); > } > > i3cdev = i3c_master_alloc_i3c_dev(master, &info); > -- > 2.14.0 In my experience we should pre-reserve all DT DA entries and not use them during the ENTDAA for safety reasons. That would be one of my next steps in this patch series. Best regards, Vitor Soares
Hi Vitor, The 12/10/2019 15:24, Vitor Soares wrote: > > Hi Przemysław, > > From: Przemysław Gaj <pgaj@cadence.com> > Date: Tue, Dec 10, 2019 at 10:14:58 > > > It may be the case that SETDASA fails for some reason. Reserve > > ->init_dyn_addr when it's defined to prevent assigning this address > > to another slave device. This way when device shows up we don't > > have to re-assign addresses. > > > > Signed-off-by: Przemysław Gaj <pgaj@cadence.com> > > --- > > drivers/i3c/master.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > > index 5c06c41e6660..fab6e0609fca 100755 > > --- a/drivers/i3c/master.c > > +++ b/drivers/i3c/master.c > > @@ -1263,7 +1263,8 @@ static void i3c_master_put_i3c_addrs(struct i3c_dev_desc *dev) > > I3C_ADDR_SLOT_FREE); > > > > if (dev->boardinfo && dev->boardinfo->init_dyn_addr) > > - i3c_bus_set_addr_slot_status(&master->bus, dev->info.dyn_addr, > > + i3c_bus_set_addr_slot_status(&master->bus, > > + dev->boardinfo->init_dyn_addr, > > I3C_ADDR_SLOT_FREE); > > } > > > > @@ -1675,6 +1676,11 @@ static int i3c_master_bus_init(struct i3c_master_controller *master) > > ret = -EBUSY; > > goto err_detach_devs; > > } > > + > > + /* Reserve the slot. */ > > + i3c_bus_set_addr_slot_status(&master->bus, > > + i3cboardinfo->init_dyn_addr, > > + I3C_ADDR_SLOT_I3C_DEV); > > } > > > > i3cdev = i3c_master_alloc_i3c_dev(master, &info); > > -- > > 2.14.0 > > In my experience we should pre-reserve all DT DA entries and not use them > during the ENTDAA for safety reasons. > That would be one of my next steps in this patch series. Actually, I reserve DT DA address above. The only thing I will rework, I won't free up the address. That will be documented in the code. We can rework it later if really needed. > > Best regards, > Vitor Soares >
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index 5c06c41e6660..fab6e0609fca 100755 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -1263,7 +1263,8 @@ static void i3c_master_put_i3c_addrs(struct i3c_dev_desc *dev) I3C_ADDR_SLOT_FREE); if (dev->boardinfo && dev->boardinfo->init_dyn_addr) - i3c_bus_set_addr_slot_status(&master->bus, dev->info.dyn_addr, + i3c_bus_set_addr_slot_status(&master->bus, + dev->boardinfo->init_dyn_addr, I3C_ADDR_SLOT_FREE); } @@ -1675,6 +1676,11 @@ static int i3c_master_bus_init(struct i3c_master_controller *master) ret = -EBUSY; goto err_detach_devs; } + + /* Reserve the slot. */ + i3c_bus_set_addr_slot_status(&master->bus, + i3cboardinfo->init_dyn_addr, + I3C_ADDR_SLOT_I3C_DEV); } i3cdev = i3c_master_alloc_i3c_dev(master, &info);
It may be the case that SETDASA fails for some reason. Reserve ->init_dyn_addr when it's defined to prevent assigning this address to another slave device. This way when device shows up we don't have to re-assign addresses. Signed-off-by: Przemysław Gaj <pgaj@cadence.com> --- drivers/i3c/master.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)