diff mbox series

[v3,1/2] dm: fix probing of all devices that have u-boot, dm-pre-reloc in SPL/TPL

Message ID 20220922155326.3293815-1-foss+uboot@0leil.net
State Accepted
Commit 942918f2acd3634dee7813f7b9f801ffd54d9f2e
Delegated to: Tom Rini
Headers show
Series [v3,1/2] dm: fix probing of all devices that have u-boot, dm-pre-reloc in SPL/TPL | expand

Commit Message

Quentin Schulz Sept. 22, 2022, 3:53 p.m. UTC
From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

Currently, dm_probe_devices checks that the flags of the device contains
DM_FLAG_PRE_RELOC. However DM_FLAG_PRE_RELOC is a driver - and not a
device - flag. This means that the check in pre_reloc_only mode would
always fail.

Instead, what was aimed to be checked is that either the driver of the
device has the flag set, or that the device has the u-boot,dm-pre-reloc
Device Tree property set.

So let's fix the check to allow u-boot,dm-pre-reloc devices to be
probed.

Cc: Quentin Schulz <foss+uboot@0leil.net>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---

v3:
 - instead of adding the DM_FLAG_PRE_RELOC driver flag to the device,
 check ofnode_pre_reloc(node) in dm_probe_devices before attempting to
 probe,
 This is better than v1 in lists_bind_fdt because the latter function
 does not work on non UCLASS drivers (e.g. gpio-hog) and also because it
 fixes the cause and does not create a work-around,

v1:
 - https://lore.kernel.org/u-boot/20220922134540.3268586-1-foss+uboot@0leil.net/

 drivers/core/root.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Simon Glass Sept. 25, 2022, 2:15 p.m. UTC | #1
Hi Quentin,

On Thu, 22 Sept 2022 at 09:53, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> Currently, dm_probe_devices checks that the flags of the device contains
> DM_FLAG_PRE_RELOC. However DM_FLAG_PRE_RELOC is a driver - and not a
> device - flag. This means that the check in pre_reloc_only mode would
> always fail.

Please can you shorten the subject?

>
> Instead, what was aimed to be checked is that either the driver of the
> device has the flag set, or that the device has the u-boot,dm-pre-reloc
> Device Tree property set.
>
> So let's fix the check to allow u-boot,dm-pre-reloc devices to be
> probed.
>
> Cc: Quentin Schulz <foss+uboot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>
> v3:
>  - instead of adding the DM_FLAG_PRE_RELOC driver flag to the device,
>  check ofnode_pre_reloc(node) in dm_probe_devices before attempting to
>  probe,
>  This is better than v1 in lists_bind_fdt because the latter function
>  does not work on non UCLASS drivers (e.g. gpio-hog) and also because it
>  fixes the cause and does not create a work-around,
>
> v1:
>  - https://lore.kernel.org/u-boot/20220922134540.3268586-1-foss+uboot@0leil.net/
>
>  drivers/core/root.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/core/root.c b/drivers/core/root.c
> index f24ddfa521..c4fb48548b 100644
> --- a/drivers/core/root.c
> +++ b/drivers/core/root.c
> @@ -363,20 +363,22 @@ void *dm_priv_to_rw(void *priv)
>
>  static int dm_probe_devices(struct udevice *dev, bool pre_reloc_only)
>  {
> -       u32 mask = DM_FLAG_PROBE_AFTER_BIND;
> -       u32 flags = dev_get_flags(dev);
> +       ofnode node = dev_ofnode(dev);
>         struct udevice *child;
>         int ret;
>
> -       if (pre_reloc_only)
> -               mask |= DM_FLAG_PRE_RELOC;
> +       if (pre_reloc_only &&
> +           (!ofnode_valid(node) || !ofnode_pre_reloc(node)) &&
> +           !(dev->driver->flags & DM_FLAG_PRE_RELOC))
> +               goto probe_children;
>
> -       if ((flags & mask) == mask) {
> +       if (dev_get_flags(dev) & DM_FLAG_PROBE_AFTER_BIND) {
>                 ret = device_probe(dev);
>                 if (ret)
>                         return ret;
>         }
>
> +probe_children:

can you use some other way, to avoid the goto?

>         list_for_each_entry(child, &dev->child_head, sibling_node)
>                 dm_probe_devices(child, pre_reloc_only);
>
> --
> 2.37.3
>

If this fixes a bug, can we we have a test for it, please?

REgards,
Simon
Tom Rini Jan. 13, 2023, 12:16 a.m. UTC | #2
On Thu, Sep 22, 2022 at 05:53:25PM +0200, Quentin Schulz wrote:

> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> Currently, dm_probe_devices checks that the flags of the device contains
> DM_FLAG_PRE_RELOC. However DM_FLAG_PRE_RELOC is a driver - and not a
> device - flag. This means that the check in pre_reloc_only mode would
> always fail.
> 
> Instead, what was aimed to be checked is that either the driver of the
> device has the flag set, or that the device has the u-boot,dm-pre-reloc
> Device Tree property set.
> 
> So let's fix the check to allow u-boot,dm-pre-reloc devices to be
> probed.
> 
> Cc: Quentin Schulz <foss+uboot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/drivers/core/root.c b/drivers/core/root.c
index f24ddfa521..c4fb48548b 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -363,20 +363,22 @@  void *dm_priv_to_rw(void *priv)
 
 static int dm_probe_devices(struct udevice *dev, bool pre_reloc_only)
 {
-	u32 mask = DM_FLAG_PROBE_AFTER_BIND;
-	u32 flags = dev_get_flags(dev);
+	ofnode node = dev_ofnode(dev);
 	struct udevice *child;
 	int ret;
 
-	if (pre_reloc_only)
-		mask |= DM_FLAG_PRE_RELOC;
+	if (pre_reloc_only &&
+	    (!ofnode_valid(node) || !ofnode_pre_reloc(node)) &&
+	    !(dev->driver->flags & DM_FLAG_PRE_RELOC))
+		goto probe_children;
 
-	if ((flags & mask) == mask) {
+	if (dev_get_flags(dev) & DM_FLAG_PROBE_AFTER_BIND) {
 		ret = device_probe(dev);
 		if (ret)
 			return ret;
 	}
 
+probe_children:
 	list_for_each_entry(child, &dev->child_head, sibling_node)
 		dm_probe_devices(child, pre_reloc_only);