| Message ID | 2c7c806ea271da383b3a9c1cf29dab965241fdac.1567769914.git.fitzsim@fitzsim.org |
|---|---|
| State | Accepted |
| Commit | 7f3289bf6dee6e4e6c7d95d3ee16d3ab3d55de55 |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | dm: CONFIG_OF_PRIOR_STAGE request number fixes | expand |
Hi Thomas, On Fri, Sep 6, 2019 at 7:52 PM Thomas Fitzsimmons <fitzsim@fitzsim.org> wrote: > > For CONFIG_OF_PRIOR_STAGE, in the absence of a device tree alias for a > given device, use the next request number for that type of device. > This allows aliases to be used when they're available, while still > allowing unaliased devices to be probed. > > Signed-off-by: Thomas Fitzsimmons <fitzsim@fitzsim.org> > Cc: Bin Meng <bmeng.cn@gmail.com> > Cc: Simon Glass <sjg@chromium.org> > --- > drivers/core/device.c | 5 +++++ > drivers/core/uclass.c | 4 +++- > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/core/device.c b/drivers/core/device.c > index 474c1642ee..ca8be208a9 100644 > --- a/drivers/core/device.c > +++ b/drivers/core/device.c > @@ -82,6 +82,11 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, > if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) { > if (uc->uc_drv->name && ofnode_valid(node)) > dev_read_alias_seq(dev, &dev->req_seq); > +#if CONFIG_IS_ENABLED(OF_PRIOR_STAGE) I was wondering whether we should limit such only for OF_PRIOR_STATE, instead change the behaviors for all DM devices. Because as I pointed out in https://lists.denx.de/pipermail/u-boot/2019-August/382368.html, it seems there are quite some codes in the existing code base that tried to workaround such limitation in their own way. > + if (dev->req_seq == -1) > + dev->req_seq = > + uclass_find_next_free_req_seq(drv->id); > +#endif > } 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 b33296542f..d7aedac351 100644 > --- a/drivers/core/uclass.c > +++ b/drivers/core/uclass.c > @@ -269,7 +269,9 @@ int uclass_find_device_by_name(enum uclass_id id, const char *name, > return -ENODEV; > } > > -#if !CONFIG_IS_ENABLED(OF_CONTROL) || CONFIG_IS_ENABLED(OF_PLATDATA) > +#if !CONFIG_IS_ENABLED(OF_CONTROL) || \ > + CONFIG_IS_ENABLED(OF_PLATDATA) || \ > + CONFIG_IS_ENABLED(OF_PRIOR_STAGE) > int uclass_find_next_free_req_seq(enum uclass_id id) > { > struct uclass *uc; > -- Regards, Bin
Hi Bin, Bin Meng <bmeng.cn@gmail.com> writes: [...] > On Fri, Sep 6, 2019 at 7:52 PM Thomas Fitzsimmons <fitzsim@fitzsim.org> wrote: >> >> For CONFIG_OF_PRIOR_STAGE, in the absence of a device tree alias for a >> given device, use the next request number for that type of device. >> This allows aliases to be used when they're available, while still >> allowing unaliased devices to be probed. >> >> Signed-off-by: Thomas Fitzsimmons <fitzsim@fitzsim.org> >> Cc: Bin Meng <bmeng.cn@gmail.com> >> Cc: Simon Glass <sjg@chromium.org> >> --- >> drivers/core/device.c | 5 +++++ >> drivers/core/uclass.c | 4 +++- >> 2 files changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/core/device.c b/drivers/core/device.c >> index 474c1642ee..ca8be208a9 100644 >> --- a/drivers/core/device.c >> +++ b/drivers/core/device.c >> @@ -82,6 +82,11 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, >> if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) { >> if (uc->uc_drv->name && ofnode_valid(node)) >> dev_read_alias_seq(dev, &dev->req_seq); >> +#if CONFIG_IS_ENABLED(OF_PRIOR_STAGE) > > I was wondering whether we should limit such only for OF_PRIOR_STATE, > instead change the behaviors for all DM devices. Maybe, though I wouldn't want to break assumptions made in this area by non-OF_PRIOR_STAGE boards. > Because as I pointed out in > https://lists.denx.de/pipermail/u-boot/2019-August/382368.html, it > seems there are quite some codes in the existing code base that tried > to workaround such limitation in their own way. I could create a separate config option to control this behavior, and document what it does in Kconfig. Then other ports could adopt it gradually, and eventually we could make it unconditional. I think OF_PRIOR_STAGE should select the new option, since I can confirm this is an improvement for my OF_PRIOR_STAGE-using board. Thomas
On Sat, 14 Sep 2019 at 06:41, Thomas Fitzsimmons <fitzsim@fitzsim.org> wrote: > > Hi Bin, > > Bin Meng <bmeng.cn@gmail.com> writes: > > [...] > > > On Fri, Sep 6, 2019 at 7:52 PM Thomas Fitzsimmons <fitzsim@fitzsim.org> wrote: > >> > >> For CONFIG_OF_PRIOR_STAGE, in the absence of a device tree alias for a > >> given device, use the next request number for that type of device. > >> This allows aliases to be used when they're available, while still > >> allowing unaliased devices to be probed. > >> > >> Signed-off-by: Thomas Fitzsimmons <fitzsim@fitzsim.org> > >> Cc: Bin Meng <bmeng.cn@gmail.com> > >> Cc: Simon Glass <sjg@chromium.org> > >> --- > >> drivers/core/device.c | 5 +++++ > >> drivers/core/uclass.c | 4 +++- > >> 2 files changed, 8 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/core/device.c b/drivers/core/device.c > >> index 474c1642ee..ca8be208a9 100644 > >> --- a/drivers/core/device.c > >> +++ b/drivers/core/device.c > >> @@ -82,6 +82,11 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, > >> if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) { > >> if (uc->uc_drv->name && ofnode_valid(node)) > >> dev_read_alias_seq(dev, &dev->req_seq); > >> +#if CONFIG_IS_ENABLED(OF_PRIOR_STAGE) > > > > I was wondering whether we should limit such only for OF_PRIOR_STATE, > > instead change the behaviors for all DM devices. > > Maybe, though I wouldn't want to break assumptions made in this area by > non-OF_PRIOR_STAGE boards. > > > Because as I pointed out in > > https://lists.denx.de/pipermail/u-boot/2019-August/382368.html, it > > seems there are quite some codes in the existing code base that tried > > to workaround such limitation in their own way. > > I could create a separate config option to control this behavior, and > document what it does in Kconfig. Then other ports could adopt it > gradually, and eventually we could make it unconditional. I think > OF_PRIOR_STAGE should select the new option, since I can confirm this is > an improvement for my OF_PRIOR_STAGE-using board. > > Thomas Reviewed-by: Simon Glass <sjg@chromium.org> It is best to use if() instead of #if if possible.
On Sat, 14 Sep 2019 at 06:41, Thomas Fitzsimmons <fitzsim@fitzsim.org> wrote: > > Hi Bin, > > Bin Meng <bmeng.cn@gmail.com> writes: > > [...] > > > On Fri, Sep 6, 2019 at 7:52 PM Thomas Fitzsimmons <fitzsim@fitzsim.org> wrote: > >> > >> For CONFIG_OF_PRIOR_STAGE, in the absence of a device tree alias for a > >> given device, use the next request number for that type of device. > >> This allows aliases to be used when they're available, while still > >> allowing unaliased devices to be probed. > >> > >> Signed-off-by: Thomas Fitzsimmons <fitzsim@fitzsim.org> > >> Cc: Bin Meng <bmeng.cn@gmail.com> > >> Cc: Simon Glass <sjg@chromium.org> > >> --- > >> drivers/core/device.c | 5 +++++ > >> drivers/core/uclass.c | 4 +++- > >> 2 files changed, 8 insertions(+), 1 deletion(-) > >> Applied to u-boot-dm/next, thanks!
diff --git a/drivers/core/device.c b/drivers/core/device.c index 474c1642ee..ca8be208a9 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -82,6 +82,11 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) { if (uc->uc_drv->name && ofnode_valid(node)) dev_read_alias_seq(dev, &dev->req_seq); +#if CONFIG_IS_ENABLED(OF_PRIOR_STAGE) + if (dev->req_seq == -1) + dev->req_seq = + uclass_find_next_free_req_seq(drv->id); +#endif } 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 b33296542f..d7aedac351 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -269,7 +269,9 @@ int uclass_find_device_by_name(enum uclass_id id, const char *name, return -ENODEV; } -#if !CONFIG_IS_ENABLED(OF_CONTROL) || CONFIG_IS_ENABLED(OF_PLATDATA) +#if !CONFIG_IS_ENABLED(OF_CONTROL) || \ + CONFIG_IS_ENABLED(OF_PLATDATA) || \ + CONFIG_IS_ENABLED(OF_PRIOR_STAGE) int uclass_find_next_free_req_seq(enum uclass_id id) { struct uclass *uc;
For CONFIG_OF_PRIOR_STAGE, in the absence of a device tree alias for a given device, use the next request number for that type of device. This allows aliases to be used when they're available, while still allowing unaliased devices to be probed. Signed-off-by: Thomas Fitzsimmons <fitzsim@fitzsim.org> Cc: Bin Meng <bmeng.cn@gmail.com> Cc: Simon Glass <sjg@chromium.org> --- drivers/core/device.c | 5 +++++ drivers/core/uclass.c | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-)