[U-Boot,1/2] dm: device: Request next sequence number
diff mbox series

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
Related show

Commit Message

Thomas Fitzsimmons Sept. 6, 2019, 11:51 a.m. UTC
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(-)

Comments

Bin Meng Sept. 6, 2019, 1:24 p.m. UTC | #1
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
Thomas Fitzsimmons Sept. 14, 2019, 1:41 p.m. UTC | #2
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
Simon Glass Sept. 27, 2019, 1:49 a.m. UTC | #3
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.
Simon Glass Sept. 27, 2019, 11:28 p.m. UTC | #4
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!

Patch
diff mbox series

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;