| Message ID | 1538660864-30399-4-git-send-email-jjhiblot@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | DM_I2C_COMPAT removal for all ti platforms | expand |
On Thu, Oct 4, 2018 at 8:48 AM Jean-Jacques Hiblot <jjhiblot@ti.com> wrote: > > If OF_CONTROL is not enabled and DM_SEQ_ALIAS is enabled, we must > assign an alias (requested sequence number) to devices that belongs to a > class with the DM_UC_FLAG_SEQ_ALIAS flag. Otherwise What about conditions where both OF_CONTROL and PLATDATA are set? I am not sure what it all does, so maybe it's OK, but I know PLATDATA is a reduced library which I know the omap3_logic uses to keep SPL small. > uclass_find_device_by_seq() cannot be used to get/probe a device. In > particular i2c_get_chip_for_busnum() cannot be used. > > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> > --- > > drivers/core/device.c | 10 ++++++---- > drivers/core/uclass.c | 24 ++++++++++++++++++++++++ > include/dm/uclass-internal.h | 13 +++++++++++++ > 3 files changed, 43 insertions(+), 4 deletions(-) > > diff --git a/drivers/core/device.c b/drivers/core/device.c > index feed43c..e441021 100644 > --- a/drivers/core/device.c > +++ b/drivers/core/device.c > @@ -70,7 +70,8 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, > > dev->seq = -1; > dev->req_seq = -1; > - if (CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(DM_SEQ_ALIAS)) { > + if (CONFIG_IS_ENABLED(DM_SEQ_ALIAS) && Is there are reason these cannot be #if statements (opposed to just 'if') to let the pre-compiler enable/disable code? I am assuming these values won't change, but I could be wrong. > + (uc->uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS)) { > /* > * Some devices, such as a SPI bus, I2C bus and serial ports > * are numbered using aliases. > @@ -78,10 +79,11 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, > * This is just a 'requested' sequence, and will be > * resolved (and ->seq updated) when the device is probed. > */ > - if (uc->uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS) { > - if (uc->uc_drv->name && ofnode_valid(node)) { > + if (CONFIG_IS_ENABLED(OF_CONTROL)) { > + if (uc->uc_drv->name && ofnode_valid(node)) > dev_read_alias_seq(dev, &dev->req_seq); > - } > + } else { > + dev->req_seq = uclass_find_next_free_req_seq(drv->id); > } > } > > diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c > index 3113d6a..376f882 100644 > --- a/drivers/core/uclass.c > +++ b/drivers/core/uclass.c > @@ -269,6 +269,30 @@ int uclass_find_device_by_name(enum uclass_id id, const char *name, > return -ENODEV; > } > > +#if !CONFIG_IS_ENABLED(OF_CONTROL) > +int uclass_find_next_free_req_seq(enum uclass_id id) > +{ > + struct uclass *uc; > + struct udevice *dev; > + int ret; > + int max = -1; > + > + ret = uclass_get(id, &uc); > + if (ret) > + return ret; > + > + list_for_each_entry(dev, &uc->dev_head, uclass_node) { > + if ((dev->req_seq != -1) && (dev->req_seq > max)) > + max = dev->req_seq; > + } > + > + if (max == -1) > + return 0; > + > + return max + 1; > +} > +#endif > + > int uclass_find_device_by_seq(enum uclass_id id, int seq_or_req_seq, > bool find_req_seq, struct udevice **devp) > { > diff --git a/include/dm/uclass-internal.h b/include/dm/uclass-internal.h > index 30d5a4f..18a838c 100644 > --- a/include/dm/uclass-internal.h > +++ b/include/dm/uclass-internal.h > @@ -12,6 +12,19 @@ > #include <dm/ofnode.h> > > /** > + * uclass_find_next_free_req_seq() - Get the next free req_seq number > + * > + * This returns the next free req_seq number. This is useful only if > + * OF_CONTROL is not used. The next free req_seq number is simply the > + * maximum req_seq of the uclass + 1. > + * This allows assiging req_seq number in the binding order. > + * > + * @id: Id number of the uclass > + * @return The next free req_seq number > + */ > +int uclass_find_next_free_req_seq(enum uclass_id id); > + > +/** > * uclass_get_device_tail() - handle the end of a get_device call > * > * This handles returning an error or probing a device as needed. > -- > 2.7.4 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
Hi Adam, On 05/10/2018 00:42, Adam Ford wrote: > On Thu, Oct 4, 2018 at 8:48 AM Jean-Jacques Hiblot <jjhiblot@ti.com> wrote: >> If OF_CONTROL is not enabled and DM_SEQ_ALIAS is enabled, we must >> assign an alias (requested sequence number) to devices that belongs to a >> class with the DM_UC_FLAG_SEQ_ALIAS flag. Otherwise > What about conditions where both OF_CONTROL and PLATDATA are set? I > am not sure what it all does, so maybe it's OK, but I know PLATDATA is > a reduced library which I know the omap3_logic uses to keep SPL small. Thanks for the review and for trying the patches. if OF_PLATDATA is used, dts is not parsed at all, so alias can't be read from dt => the seq_alias should be computed as if OF_CONTROL is not set. Now I'm curious how enabling OF_PLATDATA even works on omap3_logic. Most TI drivers (all ?) do not support it. I don't have any omap3 board, but I'll give it a try on beaglebone. > >> uclass_find_device_by_seq() cannot be used to get/probe a device. In >> particular i2c_get_chip_for_busnum() cannot be used. >> >> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >> --- >> >> drivers/core/device.c | 10 ++++++---- >> drivers/core/uclass.c | 24 ++++++++++++++++++++++++ >> include/dm/uclass-internal.h | 13 +++++++++++++ >> 3 files changed, 43 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/core/device.c b/drivers/core/device.c >> index feed43c..e441021 100644 >> --- a/drivers/core/device.c >> +++ b/drivers/core/device.c >> @@ -70,7 +70,8 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, >> >> dev->seq = -1; >> dev->req_seq = -1; >> - if (CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(DM_SEQ_ALIAS)) { >> + if (CONFIG_IS_ENABLED(DM_SEQ_ALIAS) && > Is there are reason these cannot be #if statements (opposed to just > 'if') to let the pre-compiler enable/disable code? I am assuming > these values won't change, but I could be wrong. The solution woulb de to use if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) instead of if (CONFIG_IS_ENABLED(OF_CONTROL) > >> + (uc->uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS)) { >> /* >> * Some devices, such as a SPI bus, I2C bus and serial ports >> * are numbered using aliases. >> @@ -78,10 +79,11 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, >> * This is just a 'requested' sequence, and will be >> * resolved (and ->seq updated) when the device is probed. >> */ >> - if (uc->uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS) { >> - if (uc->uc_drv->name && ofnode_valid(node)) { >> + if (CONFIG_IS_ENABLED(OF_CONTROL)) { >> + if (uc->uc_drv->name && ofnode_valid(node)) >> dev_read_alias_seq(dev, &dev->req_seq); >> - } >> + } else { >> + dev->req_seq = uclass_find_next_free_req_seq(drv->id); >> } >> } >> >> diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c >> index 3113d6a..376f882 100644 >> --- a/drivers/core/uclass.c >> +++ b/drivers/core/uclass.c >> @@ -269,6 +269,30 @@ int uclass_find_device_by_name(enum uclass_id id, const char *name, >> return -ENODEV; >> } >> >> +#if !CONFIG_IS_ENABLED(OF_CONTROL) ditto >> +int uclass_find_next_free_req_seq(enum uclass_id id) >> +{ >> + struct uclass *uc; >> + struct udevice *dev; >> + int ret; >> + int max = -1; >> + >> + ret = uclass_get(id, &uc); >> + if (ret) >> + return ret; >> + >> + list_for_each_entry(dev, &uc->dev_head, uclass_node) { >> + if ((dev->req_seq != -1) && (dev->req_seq > max)) >> + max = dev->req_seq; >> + } >> + >> + if (max == -1) >> + return 0; >> + >> + return max + 1; >> +} >> +#endif >> + >> int uclass_find_device_by_seq(enum uclass_id id, int seq_or_req_seq, >> bool find_req_seq, struct udevice **devp) >> { >> diff --git a/include/dm/uclass-internal.h b/include/dm/uclass-internal.h >> index 30d5a4f..18a838c 100644 >> --- a/include/dm/uclass-internal.h >> +++ b/include/dm/uclass-internal.h >> @@ -12,6 +12,19 @@ >> #include <dm/ofnode.h> >> >> /** >> + * uclass_find_next_free_req_seq() - Get the next free req_seq number >> + * >> + * This returns the next free req_seq number. This is useful only if >> + * OF_CONTROL is not used. The next free req_seq number is simply the >> + * maximum req_seq of the uclass + 1. >> + * This allows assiging req_seq number in the binding order. >> + * >> + * @id: Id number of the uclass >> + * @return The next free req_seq number >> + */ >> +int uclass_find_next_free_req_seq(enum uclass_id id); >> + >> +/** >> * uclass_get_device_tail() - handle the end of a get_device call >> * >> * This handles returning an error or probing a device as needed. >> -- >> 2.7.4 >> >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> https://lists.denx.de/listinfo/u-boot
On 05/10/2018 10:50, Jean-Jacques Hiblot wrote: > Hi Adam, > > > > On 05/10/2018 00:42, Adam Ford wrote: >> On Thu, Oct 4, 2018 at 8:48 AM Jean-Jacques Hiblot <jjhiblot@ti.com> >> wrote: >>> If OF_CONTROL is not enabled and DM_SEQ_ALIAS is enabled, we must >>> assign an alias (requested sequence number) to devices that belongs >>> to a >>> class with the DM_UC_FLAG_SEQ_ALIAS flag. Otherwise >> What about conditions where both OF_CONTROL and PLATDATA are set? I >> am not sure what it all does, so maybe it's OK, but I know PLATDATA is >> a reduced library which I know the omap3_logic uses to keep SPL small. > Thanks for the review and for trying the patches. > > if OF_PLATDATA is used, dts is not parsed at all, so alias can't be > read from dt => the seq_alias should be computed as if OF_CONTROL is > not set. > > Now I'm curious how enabling OF_PLATDATA even works on omap3_logic. > Most TI drivers (all ?) do not support it. > I don't have any omap3 board, but I'll give it a try on beaglebone. OK. So I think I understand a bit more what is happening here. The omap3_logic does not actually take advantage of SPL_OF_PLATDATA nor SPL_OF_CONTROL. Devices are instantiated by the SPL the old way, using U_BOOT_DEVICE() (board/logicpd/omap3logic.c:45-70). In the SPL, the I2C instantiation is missing, but I guess that it is ok because nobody checks the return value of the calls to twl4030 functions. You probably can remove SPL_OF_PLATDATA and SPL_OF_CONTROL and still be able to boot. JJ > >> >>> uclass_find_device_by_seq() cannot be used to get/probe a device. In >>> particular i2c_get_chip_for_busnum() cannot be used. >>> >>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >>> --- >>> >>> drivers/core/device.c | 10 ++++++---- >>> drivers/core/uclass.c | 24 ++++++++++++++++++++++++ >>> include/dm/uclass-internal.h | 13 +++++++++++++ >>> 3 files changed, 43 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/core/device.c b/drivers/core/device.c >>> index feed43c..e441021 100644 >>> --- a/drivers/core/device.c >>> +++ b/drivers/core/device.c >>> @@ -70,7 +70,8 @@ static int device_bind_common(struct udevice >>> *parent, const struct driver *drv, >>> >>> dev->seq = -1; >>> dev->req_seq = -1; >>> - if (CONFIG_IS_ENABLED(OF_CONTROL) && >>> CONFIG_IS_ENABLED(DM_SEQ_ALIAS)) { >>> + if (CONFIG_IS_ENABLED(DM_SEQ_ALIAS) && >> Is there are reason these cannot be #if statements (opposed to just >> 'if') to let the pre-compiler enable/disable code? I am assuming >> these values won't change, but I could be wrong. > The solution woulb de to use > if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) > instead of > if (CONFIG_IS_ENABLED(OF_CONTROL) >> >>> + (uc->uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS)) { >>> /* >>> * Some devices, such as a SPI bus, I2C bus and >>> serial ports >>> * are numbered using aliases. >>> @@ -78,10 +79,11 @@ static int device_bind_common(struct udevice >>> *parent, const struct driver *drv, >>> * This is just a 'requested' sequence, and will be >>> * resolved (and ->seq updated) when the device is >>> probed. >>> */ >>> - if (uc->uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS) { >>> - if (uc->uc_drv->name && ofnode_valid(node)) { >>> + if (CONFIG_IS_ENABLED(OF_CONTROL)) { >>> + if (uc->uc_drv->name && ofnode_valid(node)) >>> dev_read_alias_seq(dev, >>> &dev->req_seq); >>> - } >>> + } else { >>> + dev->req_seq = >>> uclass_find_next_free_req_seq(drv->id); >>> } >>> } >>> >>> diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c >>> index 3113d6a..376f882 100644 >>> --- a/drivers/core/uclass.c >>> +++ b/drivers/core/uclass.c >>> @@ -269,6 +269,30 @@ int uclass_find_device_by_name(enum uclass_id >>> id, const char *name, >>> return -ENODEV; >>> } >>> >>> +#if !CONFIG_IS_ENABLED(OF_CONTROL) > ditto >>> +int uclass_find_next_free_req_seq(enum uclass_id id) >>> +{ >>> + struct uclass *uc; >>> + struct udevice *dev; >>> + int ret; >>> + int max = -1; >>> + >>> + ret = uclass_get(id, &uc); >>> + if (ret) >>> + return ret; >>> + >>> + list_for_each_entry(dev, &uc->dev_head, uclass_node) { >>> + if ((dev->req_seq != -1) && (dev->req_seq > max)) >>> + max = dev->req_seq; >>> + } >>> + >>> + if (max == -1) >>> + return 0; >>> + >>> + return max + 1; >>> +} >>> +#endif >>> + >>> int uclass_find_device_by_seq(enum uclass_id id, int seq_or_req_seq, >>> bool find_req_seq, struct udevice >>> **devp) >>> { >>> diff --git a/include/dm/uclass-internal.h >>> b/include/dm/uclass-internal.h >>> index 30d5a4f..18a838c 100644 >>> --- a/include/dm/uclass-internal.h >>> +++ b/include/dm/uclass-internal.h >>> @@ -12,6 +12,19 @@ >>> #include <dm/ofnode.h> >>> >>> /** >>> + * uclass_find_next_free_req_seq() - Get the next free req_seq number >>> + * >>> + * This returns the next free req_seq number. This is useful only if >>> + * OF_CONTROL is not used. The next free req_seq number is simply the >>> + * maximum req_seq of the uclass + 1. >>> + * This allows assiging req_seq number in the binding order. >>> + * >>> + * @id: Id number of the uclass >>> + * @return The next free req_seq number >>> + */ >>> +int uclass_find_next_free_req_seq(enum uclass_id id); >>> + >>> +/** >>> * uclass_get_device_tail() - handle the end of a get_device call >>> * >>> * This handles returning an error or probing a device as needed. >>> -- >>> 2.7.4 >>> >>> _______________________________________________ >>> U-Boot mailing list >>> U-Boot@lists.denx.de >>> https://lists.denx.de/listinfo/u-boot >
diff --git a/drivers/core/device.c b/drivers/core/device.c index feed43c..e441021 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -70,7 +70,8 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, dev->seq = -1; dev->req_seq = -1; - if (CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(DM_SEQ_ALIAS)) { + if (CONFIG_IS_ENABLED(DM_SEQ_ALIAS) && + (uc->uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS)) { /* * Some devices, such as a SPI bus, I2C bus and serial ports * are numbered using aliases. @@ -78,10 +79,11 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, * This is just a 'requested' sequence, and will be * resolved (and ->seq updated) when the device is probed. */ - if (uc->uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS) { - if (uc->uc_drv->name && ofnode_valid(node)) { + if (CONFIG_IS_ENABLED(OF_CONTROL)) { + if (uc->uc_drv->name && ofnode_valid(node)) dev_read_alias_seq(dev, &dev->req_seq); - } + } else { + dev->req_seq = uclass_find_next_free_req_seq(drv->id); } } diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index 3113d6a..376f882 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -269,6 +269,30 @@ int uclass_find_device_by_name(enum uclass_id id, const char *name, return -ENODEV; } +#if !CONFIG_IS_ENABLED(OF_CONTROL) +int uclass_find_next_free_req_seq(enum uclass_id id) +{ + struct uclass *uc; + struct udevice *dev; + int ret; + int max = -1; + + ret = uclass_get(id, &uc); + if (ret) + return ret; + + list_for_each_entry(dev, &uc->dev_head, uclass_node) { + if ((dev->req_seq != -1) && (dev->req_seq > max)) + max = dev->req_seq; + } + + if (max == -1) + return 0; + + return max + 1; +} +#endif + int uclass_find_device_by_seq(enum uclass_id id, int seq_or_req_seq, bool find_req_seq, struct udevice **devp) { diff --git a/include/dm/uclass-internal.h b/include/dm/uclass-internal.h index 30d5a4f..18a838c 100644 --- a/include/dm/uclass-internal.h +++ b/include/dm/uclass-internal.h @@ -12,6 +12,19 @@ #include <dm/ofnode.h> /** + * uclass_find_next_free_req_seq() - Get the next free req_seq number + * + * This returns the next free req_seq number. This is useful only if + * OF_CONTROL is not used. The next free req_seq number is simply the + * maximum req_seq of the uclass + 1. + * This allows assiging req_seq number in the binding order. + * + * @id: Id number of the uclass + * @return The next free req_seq number + */ +int uclass_find_next_free_req_seq(enum uclass_id id); + +/** * uclass_get_device_tail() - handle the end of a get_device call * * This handles returning an error or probing a device as needed.
If OF_CONTROL is not enabled and DM_SEQ_ALIAS is enabled, we must assign an alias (requested sequence number) to devices that belongs to a class with the DM_UC_FLAG_SEQ_ALIAS flag. Otherwise uclass_find_device_by_seq() cannot be used to get/probe a device. In particular i2c_get_chip_for_busnum() cannot be used. Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> --- drivers/core/device.c | 10 ++++++---- drivers/core/uclass.c | 24 ++++++++++++++++++++++++ include/dm/uclass-internal.h | 13 +++++++++++++ 3 files changed, 43 insertions(+), 4 deletions(-)