Hi Simon,
On 10:39-20240711, Simon Glass wrote:
> Hi Manorit,
>
> On Wed, 10 Jul 2024 at 07:15, Manorit Chawdhry <m-chawd...@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 <s...@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.
Meanwhile I will open a issue as you mentioned for tracking this, trying
to register on the URL that you provided. Thanks!
Regards,
Manorit
>
> 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-9007681ed...@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
> > >
