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

[Simon Glass]

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.

Regards,
Simon

[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
> > ---
> >
> >  tools/fdtgrep.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c
> > index ca639a2d9f4f..f1ff1946bd4a 100644
> > --- a/tools/fdtgrep.c
> > +++ b/tools/fdtgrep.c
> > @@ -63,6 +63,7 @@ struct display_info {
> >       int types_inc;          /* Mask of types that we include (FDT_IS...) */
> >       int types_exc;          /* Mask of types that we exclude (FDT_IS...) */
> >       int invert;             /* Invert polarity of match */
> > +     int props_up;           /* Imply properties up to supernodes */
> >       struct value_node *value_head;  /* List of values to match */
> >       const char *output_fname;       /* Output filename */
> >       FILE *fout;             /* File to write dts/dtb output */
> > @@ -606,6 +607,16 @@ static int check_props(struct display_info *disp, const void *fdt, int node,
> >                                        strlen(str));
> >       }
> >
> > +     /* if requested, check all subnodes for this property too */
> > +     if (inc != 1 && disp->props_up) {
> > +             int subnode;
> > +
> > +             for (subnode = fdt_first_subnode(fdt, node);
> > +                  subnode > 0 && inc != 1;
> > +                  subnode = fdt_next_subnode(fdt, subnode))
> > +                     inc = check_props(disp, fdt, subnode, inc);
> > +     }
> > +
> >       return inc;
> >  }
> >
> > @@ -955,7 +966,7 @@ static const char usage_synopsis[] =
> >       case '?': usage("unknown option");
> >
> >  static const char usage_short_opts[] =
> > -             "haAc:b:C:defg:G:HIlLmn:N:o:O:p:P:rRsStTv"
> > +             "haAc:b:C:defg:G:HIlLmn:N:o:O:p:P:rRsStTuv"
> >               USAGE_COMMON_SHORT_OPTS;
> >  static const struct option usage_long_opts[] = {
> >       {"show-address",        no_argument, NULL, 'a'},
> > @@ -985,6 +996,7 @@ static const struct option usage_long_opts[] = {
> >       {"skip-supernodes",     no_argument, NULL, 'S'},
> >       {"show-stringtab",      no_argument, NULL, 't'},
> >       {"show-aliases",        no_argument, NULL, 'T'},
> > +     {"props-up-to-supernode", no_argument, NULL, 'u'},
> >       {"invert-match",        no_argument, NULL, 'v'},
> >       USAGE_COMMON_LONG_OPTS,
> >  };
> > @@ -1016,6 +1028,7 @@ static const char * const usage_opts_help[] = {
> >       "Don't include supernodes of matching nodes",
> >       "Include string table in binary output",
> >       "Include matching aliases in output",
> > +     "Add -p properties to supernodes too",
> >       "Invert the sense of matching (select non-matching lines)",
> >       USAGE_COMMON_OPTS_HELP
> >  };
> > @@ -1202,6 +1215,9 @@ static void scan_args(struct display_info *disp, int argc, char *argv[])
> >               case 'T':
> >                       disp->add_aliases = 1;
> >                       break;
> > +             case 'u':
> > +                     disp->props_up = 1;
> > +                     break;
> >               case 'v':
> >                       disp->invert = 1;
> >                       break;
> > --
> > 2.43.0.472.g3155946c3a-goog
> >