Re: [PATCH 2/7] of: Create of_root if no dtb provided by firmware

[Geert Uytterhoeven]

Hi Rob,

CC broonie
CC somlo

On Wed, Feb 21, 2024 at 3:06 PM Rob Herring <robh+dt@xxxxxxxxxx> wrote:
> On Fri, Feb 16, 2024 at 11:08 PM Saurabh Singh Sengar
> <ssengar@xxxxxxxxxxxxxxxxxxx> wrote:
> > On Fri, Feb 16, 2024 at 05:05:51PM -0800, Frank Rowand wrote:
> > > When enabling CONFIG_OF on a platform where 'of_root' is not populated
> > > by firmware, we end up without a root node. In order to apply overlays
> > > and create subnodes of the root node, we need one. Create this root node
> > > by unflattening an empty builtin dtb.
> > >
> > > If firmware provides a flattened device tree (FDT) then the FDT is
> > > unflattened via setup_arch(). Otherwise, the call to
> > > unflatten(_and_copy)?_device_tree() will create an empty root node.
> > >
> > > We make of_have_populated_dt() return true only if the DTB was loaded by
> > > firmware so that existing callers don't change behavior after this
> > > patch. The call in the of platform code is removed because it prevents
> > > overlays from creating platform devices when the empty root node is
> > > used.
> > >
> > > [sboyd@xxxxxxxxxx: Update of_have_populated_dt() to treat this empty dtb
> > > as not populated. Drop setup_of() initcall]
> > >
> > > Signed-off-by: Frank Rowand <frowand.list@xxxxxxxxx>
> > > Link: https://lore.kernel.org/r/20230317053415.2254616-2-frowand.list@xxxxxxxxx
> > > Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> > > Signed-off-by: Stephen Boyd <sboyd@xxxxxxxxxx>

Thanks for your patch, which is now commit 7b937cc243e5b1df ("of:
Create of_root if no dtb provided by firmware") in dt-rh/for-next
(next-20240312 and later).

On my OrangeCrab FPGA running Linux on LiteX/VexRiscv-V, the attached
OLED display now fails to probe:

    -ssd130x-i2c 0-003c: supply vcc not found, using dummy regulator
    -[drm] Initialized ssd130x 1.0.0 20220131 for 0-003c on minor 0
    +ssd130x-i2c 0-003c: incomplete constraints, dummy supplies not allowed
    +ssd130x-i2c 0-003c: error -ENODEV: Failed to get VCC regulator

> > > @@ -1645,6 +1635,21 @@ static inline bool of_device_is_system_power_controller(const struct device_node
> > >       return of_property_read_bool(np, "system-power-controller");
> > >  }
> > >
> > > +/**
> > > + * of_have_populated_dt() - Has DT been populated by bootloader
> > > + *
> > > + * Return: True if a DTB has been populated by the bootloader and it isn't the
> > > + * empty builtin one. False otherwise.
> > > + */
> > > +static inline bool of_have_populated_dt(void)
> > > +{
> > > +#ifdef CONFIG_OF
> > > +     return of_property_present(of_root, "compatible");
> >
> > This adds the strict check for compatible which makes compatible
> > to be mandatory for root nodes. So far, DeviceTree without compatible
> > property in root nodes can work. Do we want to make this documented
> > somewhere ?
>
> It already is in the DT spec and schemas.

How many systems in the wild violate this?

Apparently the DTS generated by LiteX does not have a root compatible
(and model) property, hence of_have_populated_dt() returns false.
While my gateware and DTS is quite old, a quick look at recent
litex_json2dts_linux.py history shows this is still true for current LiteX.

Now, how is this related to the failure I am seeing?

drivers/regulator/core.c has:

    static bool have_full_constraints(void)
    {
            return has_full_constraints || of_have_populated_dt();
    }

and

    static int __init regulator_init_complete(void)
    {
            /*
             * Since DT doesn't provide an idiomatic mechanism for
             * enabling full constraints and since it's much more natural
             * with DT to provide them just assume that a DT enabled
             * system has full constraints.
             */
            if (of_have_populated_dt())
                    has_full_constraints = true;

            ...
    }
    late_initcall_sync(regulator_init_complete);

(The latter looks a bit futile, as have_full_constraints() already
 contains a check for of_have_populated_dt()?)

When have_full_constraints() returns false, any dummy regulator will
fail to probe.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds