[U-Boot] [PATCH v2] dm: core: also parse chosen node

[Simon Glass]

Hi Rob,

On 7 August 2017 at 13:55, Rob Clark <robdclark at gmail.com> wrote:
> This is the node that would contain, for example, the framebuffer setup
> by an earlier stage.
>
> Signed-off-by: Rob Clark <robdclark at gmail.com>
> ---
> v2: simplify a bit (and fix incorrect handling of pre_reloc_only) by
>     calling dm_scan_fdt_node() recursively to handle chosen node; add
>     to sandbox tests as suggested by Simon
>
>  arch/sandbox/dts/test.dts |  7 +++++++
>  drivers/core/root.c       | 12 ++++++++++++
>  test/dm/bus.c             |  2 +-
>  test/dm/test-fdt.c        |  2 +-
>  4 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index 65b2f8ecda..5aed470741 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -476,6 +476,13 @@
>         wdt0: wdt at 0 {
>                 compatible = "sandbox,wdt";
>         };
> +
> +       chosen {
> +               chosen-test {
> +                       compatible = "denx,u-boot-fdt-test";
> +                       reg = <9 1>;
> +               };
> +       };

How will people know about this feature? Perhaps a new section in
docs/driver-model/README.txt?

>  };
>
>  #include "sandbox_pmic.dtsi"
> diff --git a/drivers/core/root.c b/drivers/core/root.c
> index d691d6ff94..023e6abce8 100644
> --- a/drivers/core/root.c
> +++ b/drivers/core/root.c
> @@ -266,6 +266,18 @@ static int dm_scan_fdt_node(struct udevice *parent, const void *blob,

There is also dm_scan_fdt_live() which needs updating, right?

In fact perhaps we can combine those two function now that we have ofnode?

>         for (offset = fdt_first_subnode(blob, offset);
>              offset > 0;
>              offset = fdt_next_subnode(blob, offset)) {

How will this work with pre_reloc_only? It seems to be ignored.

> +
> +               /* "chosen" node isn't a device itself but may contain some: */
> +               if (!strcmp(fdt_get_name(blob, offset, NULL), "chosen")) {

Can this move to lists_bind_fdt()? There we have a check for a
compatible string which presumably will not be present.

I am not so keen on having it here since it happens for every node and
only one of them will be called 'chosen'. i.e. it slows things down.

> +                       dm_dbg("parsing subnodes of \"chosen\"\n");
> +
> +                       err = dm_scan_fdt_node(parent, blob, offset,
> +                                              pre_reloc_only);
> +                       if (err && !ret)
> +                               ret = err;
> +                       continue;
> +               }
> +
>                 if (pre_reloc_only &&
>                     !dm_fdt_pre_reloc(blob, offset))
>                         continue;
> diff --git a/test/dm/bus.c b/test/dm/bus.c
> index 7006d4163d..1da398ae3a 100644
> --- a/test/dm/bus.c
> +++ b/test/dm/bus.c
> @@ -105,7 +105,7 @@ UCLASS_DRIVER(testbus) = {
>  /* Test that we can probe for children */
>  static int dm_test_bus_children(struct unit_test_state *uts)
>  {
> -       int num_devices = 6;
> +       int num_devices = 7;
>         struct udevice *bus;
>         struct uclass *uc;
>
> diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c
> index dcc2ef8b65..920ccbf016 100644
> --- a/test/dm/test-fdt.c
> +++ b/test/dm/test-fdt.c
> @@ -167,7 +167,7 @@ int dm_check_devices(struct unit_test_state *uts, int num_devices)
>  /* Test that FDT-based binding works correctly */
>  static int dm_test_fdt(struct unit_test_state *uts)
>  {
> -       const int num_devices = 6;
> +       const int num_devices = 7;
>         struct udevice *dev;
>         struct uclass *uc;
>         int ret;
> --
> 2.13.0
>

Regards,
Simon