Re: [RFC/PATCH] drivers/of: add debug for early dump of the dtb strcutrue

[Saravana Kannan]

On Fri, Aug 8, 2025 at 9:25 AM Rob Herring <robh@xxxxxxxxxx> wrote:
>
> On Fri, Aug 8, 2025 at 9:26 AM Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx> wrote:
> >
> > When testing for boot issues, it was helpful to dump the
> > list of nodes and properties in the device-tree passed into
> > the kernel.
>
> Shouldn't the bootloader be able to dump that?
>
> > Add CONFIG_OF_EARLY_DUMP option to dump the list of nodes
> > and properties to the standard console output early in the
> > boot sequence. Note, you may need to have some sort of
>
> s/may/will/
>
> > early or debug console output if there are issues stopping
> > the kernel starting properly.
>
> Seems to me this is giving the user the haystack to find the needle...

Completely agree with Rob.

Ben, can you give more context on what kind of issues this has helped
you (or anticipate it will) solve? Maybe there are better ways of
getting what you need.

-Saravana

>
> > Signed-off-by: Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx>
> > ---
> >  drivers/of/Kconfig |  8 ++++++++
> >  drivers/of/fdt.c   | 39 +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 47 insertions(+)
> >
> > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> > index 50697cc3b07e..ed2c52c54a7d 100644
> > --- a/drivers/of/Kconfig
> > +++ b/drivers/of/Kconfig
> > @@ -126,4 +126,12 @@ config OF_OVERLAY_KUNIT_TEST
> >  config OF_NUMA
> >         bool
> >
> > +config OF_EARLY_DUMP
> > +       bool "Dump node list at startup"
>
> This needs to depend on OF_EARLY_FLATTREE.
>
> > +       help
> > +         This debug feature runs through all the nodes/properties at startup
> > +         to show if the dtb has been passed correctly from the boot stage.
> > +
> > +         If unsure, say N here
> > +
> >  endif # OF
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index 0edd639898a6..ab40db0e71a5 100644
> > --- a/drivers/of/fdt.c
> > +++ b/drivers/of/fdt.c
> > @@ -1164,6 +1164,43 @@ static void * __init early_init_dt_alloc_memory_arch(u64 size, u64 align)
> >         return memblock_alloc_or_panic(size, align);
> >  }
> >
> > +#ifdef CONFIG_OF_EARLY_DUMP
> > +static int __init early_init_iterate_nodes(unsigned long node,
> > +                                          const char *uname,
> > +                                          int depth, void *data)
> > +{
> > +       void *blob = initial_boot_params;
> > +       int cur;
> > +
> > +       pr_info("node '%s', depth %d\n", uname, depth);
>
> Can you add indentation for the depth rather than printing the depth?
>
> I'm not completely sure if calling this is safe always. How early this
> will run depends on the architecture. So need to test on a variety of
> architectures. Or perhaps limit in kconfig to tested architectures.
> I'd rather not do that though.
>
> > +
> > +       for (cur = fdt_first_property_offset(blob, node);
> > +            cur >= 0;
> > +            cur = fdt_next_property_offset(blob, cur)) {
> > +               const char *pname;
> > +               const __be32 *val;
> > +               u32 sz;
> > +
> > +               val = fdt_getprop_by_offset(blob, cur, &pname, &sz);
> > +               if (!val) {
> > +                       pr_warn(" Cannot locate property at 0x%x\n", cur);
> > +                       continue;
>
> If this fails, you should probably just stop and bail out.
>
> > +               }
> > +
> > +               pr_info("node %s: property %s\n", uname, pname ? pname : "unnamed");
>
> Can unnamed actually happen?
>
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static inline void early_init_dump_dt(void)
> > +{
> > +       of_scan_flat_dt(early_init_iterate_nodes, NULL);
>
> This way to iterate is left over from before having libfdt. Is there
> not a libfdt way to iterate thru all nodes?
>
> > +}
> > +#else
> > +static inline void early_init_dump_dt(void) { }
> > +#endif /* CONFIG_OF_EARLY_DUMP */
> > +
> >  bool __init early_init_dt_verify(void *dt_virt, phys_addr_t dt_phys)
> >  {
> >         if (!dt_virt)
> > @@ -1178,6 +1215,8 @@ bool __init early_init_dt_verify(void *dt_virt, phys_addr_t dt_phys)
> >         initial_boot_params_pa = dt_phys;
> >         of_fdt_crc32 = crc32_be(~0, initial_boot_params,
> >                                 fdt_totalsize(initial_boot_params));
> > +
> > +       early_init_dump_dt();
>
> Use "if (IS_ENABLED(CONFIG_OF_EARLY_DUMP))" here instead of #ifdef.
>
> >
> >         /* Initialize {size,address}-cells info */
> >         early_init_dt_scan_root();
> > --
> > 2.37.2.352.g3c44437643
> >