[PATCH 3/3] fdt: introduce apply_all command

[Simon Glass]

Hi Andre,

On Wed, 13 Jul 2022 at 07:18, Andre Przywara <andre.przywara at arm.com> wrote:
>
> On Tue, 12 Jul 2022 04:58:35 -0600
> Simon Glass <sjg at chromium.org> wrote:
>
> Hi Simon,
>
> many thanks for having a look!
>
> > On Tue, 5 Jul 2022 at 11:14, Andre Przywara <andre.przywara at arm.com> wrote:
> > >
> > > Explicitly specifying the exact filenames of devicetree overlays files
> > > on a U-Boot command line can be quite tedious for users, especially
> > > when it should be made persistent for every boot.
> > >
> > > To simplify the task of applying (custom) DT overlays, introduce a
> > > "fdt apply-all" subcommand, that iterates a given directory in any
> > > supported filesystem, and tries to apply every .dtbo file found it
> > > there.
> > >
> > > This allows users to simply drop a DT overlay file into a magic
> > > directory, and it will be applied on the next boot automatically,
> > > by the virtue of just a generic U-Boot command call.
> > >
> > > Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> > > ---
> > >  cmd/fdt.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 88 insertions(+), 1 deletion(-)
> >
> > This looks OK, but can you please add a test (see test/dm/acpi.c for
> > example) and doc/usage/cmd file?
>
> Is that supposed to run inside the sandbox? I briefly tested this there,
> only to realise that the sandbox' hostfs does not support the directory
> operations (fs_opendir_unsupported). I haven't thought about it too much,
> nor do I have much experience with U-Boot's test framework, but this sounds
> like a problem?

Yes that is a problem, although it would not be too hard to implement, I think.

Also I sent a little series to add a test for 'fdt addr' which might
make it easier for you.

>
> > Also, apply_all is a bit annoying as we try to allow command
> > completion and abbreviations to work. Given that the args are
> > different I don't think a -d (for dir) flag makes sense.
> >
> > Perhaps 'fdt fsapply' ?
>
> Yeah, I wasn't happy with that name either, but couldn't come up with a
> better name. "fsapply" seems to be a nice alternative, I will go with that!

OK good.

Regards,
Simon