[PATCH 09/10] fdtgrep: Allow propagating properties up to supernodes

[Simon Glass]

Hi Manorit,

On Thu, 19 Dec 2024 at 02:16, Manorit Chawdhry <m-chawdhry at ti.com> wrote:
>
> Hi Simon,
>
> On 10:12-20240716, Manorit Chawdhry wrote:
> > Hi Simon,
> >
> > On 10:39-20240711, Simon Glass wrote:
> > > Hi Manorit,
> > >
> > > On Wed, 10 Jul 2024 at 07:15, Manorit Chawdhry <m-chawdhry at ti.com> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On 09:36-20231217, Simon Glass wrote:
> > > > > The existing bootph binding is defined such that properties in a
> > > > > subnode are also implied in the supernode also, as in this example:
> > > > >
> > > > >    buttons {
> > > > >       /* bootph,pre-ram is implied by btn1 */
> > > > >       compatible = "gpio-keys";
> > > > >
> > > > >       btn1 {
> > > > >          bootph,pre-ram;
> > > > >          gpios = <&gpio_a 3 0>;
> > > > >          label = "button1";
> > > > >          linux,code = <BTN_1>;
> > > > >       };
> > > > >
> > > > > Provide an option to implement this in fdtgrep.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > >
> > > > Coming back to this patch as am facing some issues with the bootph-all
> > > > propagation to the parent nodes [0]. We have a dmsc/sms node in our
> > > > devices and it is a parent node of k3_pds, k3_clks, k3_reset.
> > > >
> > > > During U-boot proper we are initializing the serial console and during
> > > > that time we have to make some calls to our Device manager for clocks
> > > > and power domains. With the DT modelling, the clocks and power domains
> > > > come under the device manager node ( dmsc/sms ) but interestingly that
> > > > driver is not getting probed while putting bootph-all in the child
> > > > nodes. Do you know anything about this? I thought putting it in child
> > > > node will propagate it to the parent nodes as well so it'll be probed as
> > > > well. We are having to put bootph-all explicitely in the dmsc/sms node
> > > > for it to work.
> > >
> > > Yes, that is because fdtgrep is only used with the SPL DT. The U-Boot
> > > one is used unmodified by U-Boot and it does not check parents for
> > > such properties (neither does SPL, but fdtgrep has logic to imply
> > > these properties internally).
> > >
> > > The easiest way to fix this would probably be to process the DT in
> > > Binman for insertion into the final image, since Binman already does
> > > lots of other DT processing.
> > >
> > > If you like, you could create an issue for it [1]. If you want to have
> > > a crack at fixing it, I could provide some pointers.
> >
> > I am not sure if I'll be able to focus on this now but feel free to give
> > your pointers, might help anyone else as well if they want to take a
> > look at it.
>
> Would you be able to give some pointers?

Can we first just be clear on the problem? Is it that before
relocation, devices are not bound because the bootph tag is not there?
Can you update your issue with a bit more info?

If so, then there are two options I can think of:

1. Update dm_scan_fdt_node() / lists_bind_fdt() so that it scans
children before relocation, even when a parent does not have the
required bootph tag. But it only binds the parent if any of its
children have the tag. This should be possible and I suspect the
performance would be OK.

2. Update binman so that it adds tags into the devicetree when it sees
that they are implied in a parent. This would avoid code changes in
U-Boot, and perhaps be a bit faster.

> Also I was wondering why do we
> even need to process the DT in binman, why isn't the parent node probed
> by default in this flow, is there some use case like that where the
> child needs to be probed but not the parent before it or is it some
> other bug?

Basically, Rob Herring wanted to have child tags implied in the
parent. This was the 'deal' under which the bootph stuff went into the
schema. U-Boot has never supported this previously, of course and we
have had to add the tags manually in the parent, when any child needs
it. But anyway Rob's idea seemed reasonable to me since it is actually
a pain to add these tags in parents, particular when there are several
levels of hierarchy, e.g. an soc node, etc.

So I then implemented this in fdtgrep, thinking that this was the end
of the story, but as you have found (if I have this right), it is not.

>
> >
> > Meanwhile I will open a issue as you mentioned for tracking this, trying
> > to register on the URL that you provided. Thanks!
>
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/21

> > > [1] https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues
> > >
> > >
> > > >
> > > > [0]: https://lore.kernel.org/linux-arm-kernel/20240705-b4-upstream-bootph-all-v2-0-9007681ed7d8@ti.com/T/#t
> > > >
> > > > Regards,
> > > > Manorit

Regards,
Simon