On Tue, 12 Jul 2022 04:58:35 -0600
Simon Glass <s...@chromium.org> wrote:
Hi Simon,
many thanks for having a look!
> On Tue, 5 Jul 2022 at 11:14, Andre Przywara <andre.przyw...@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.przyw...@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?
> 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!
Cheers,
Andre
>
> Regards,
> Simon
>
>
> >
> > diff --git a/cmd/fdt.c b/cmd/fdt.c
> > index d6878c96f1..dc80e13c3d 100644
> > --- a/cmd/fdt.c
> > +++ b/cmd/fdt.c
> > @@ -12,12 +12,14 @@
> > #include <env.h>
> > #include <image.h>
> > #include <linux/ctype.h>
> > +#include <linux/sizes.h>
> > #include <linux/types.h>
> > #include <asm/global_data.h>
> > #include <linux/libfdt.h>
> > #include <fdt_support.h>
> > #include <mapmem.h>
> > #include <asm/io.h>
> > +#include <fs.h>
> >
> > #define MAX_LEVEL 32 /* how deeply nested we will go */
> > #define SCRATCHPAD 1024 /* bytes of scratchpad memory */
> > @@ -107,6 +109,81 @@ static int fdt_get_header_value(int argc, char *const
> > argv[])
> > return CMD_RET_FAILURE;
> > }
> >
> > +#ifdef CONFIG_OF_LIBFDT_OVERLAY
> > +static int apply_all_overlays(const char *ifname, const char *dev_part_str,
> > + const char *dirname)
> > +{
> > + unsigned long addr;
> > + struct fdt_header *dtbo;
> > + const char *addr_str;
> > + struct fs_dir_stream *dirs;
> > + struct fs_dirent *dent;
> > + char fname[256], *name_beg;
> > + int ret;
> > +
> > + addr_str = env_get("fdtoverlay_addr_r");
> > + if (!addr_str) {
> > + printf("Invalid fdtoverlay_addr_r for loading overlays\n");
> > + return CMD_RET_FAILURE;
> > + }
> > + addr = hextoul(addr_str, NULL);
> > +
> > + ret = fs_set_blk_dev(ifname, dev_part_str, FS_TYPE_ANY);
> > + if (ret)
> > + return CMD_RET_FAILURE;
> > +
> > + if (!dirname)
> > + dirname = "/";
> > + dirs = fs_opendir(dirname);
> > + if (!dirs) {
> > + printf("Cannot find directory \"%s\"\n", dirname);
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + strcpy(fname, dirname);
> > + name_beg = strchr(fname, 0);
> > + if (name_beg[-1] != '/')
> > + *name_beg++ = '/';
> > +
> > + dtbo = map_sysmem(addr, 0);
> > + while ((dent = fs_readdir(dirs))) {
> > + loff_t size = 0;
> > +
> > + if (dent->type == FS_DT_DIR)
> > + continue;
> > +
> > + if (strcmp(dent->name + strlen(dent->name) - 5, ".dtbo"))
> > + continue;
> > +
> > + printf("%s: ", dent->name);
> > + strcpy(name_beg, dent->name);
> > + fs_set_blk_dev(ifname, dev_part_str, FS_TYPE_ANY);
> > + if (dent->size > SZ_2M)
> > + size = SZ_2M;
> > + else
> > + size = dent->size;
> > + ret = fs_read(fname, addr, 0, size, &size);
> > + if (ret) {
> > + printf(" errno: %d\n", ret);
> > + continue;
> > + }
> > + if (!fdt_valid(&dtbo)) {
> > + /* fdt_valid() clears the pointer upon failure */
> > + dtbo = map_sysmem(addr, 0);
> > + continue;
> > + }
> > +
> > + if (fdt_overlay_apply_verbose(working_fdt, dtbo) == 0)
> > + printf("applied\n");
> > + }
> > + unmap_sysmem(dtbo);
> > +
> > + fs_closedir(dirs);
> > +
> > + return 0;
> > +}
> > +#endif
> > +
> > /*
> > * Flattened Device Tree command, see the help for parameter definitions.
> > */
> > @@ -703,7 +780,7 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int
> > argc, char *const argv[])
> > }
> > #ifdef CONFIG_OF_LIBFDT_OVERLAY
> > /* apply an overlay */
> > - else if (strncmp(argv[1], "ap", 2) == 0) {
> > + else if (strcmp(argv[1], "apply") == 0) {
> > unsigned long addr;
> > struct fdt_header *blob;
> > int ret;
> > @@ -723,6 +800,15 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int
> > argc, char *const argv[])
> > ret = fdt_overlay_apply_verbose(working_fdt, blob);
> > if (ret)
> > return CMD_RET_FAILURE;
> > + /* apply all .dtbo files from a directory */
> > + } else if (strncmp(argv[1], "apply", 5) == 0) {
> > + if (argc != 5)
> > + return CMD_RET_USAGE;
> > +
> > + if (!working_fdt)
> > + return CMD_RET_FAILURE;
> > +
> > + return apply_all_overlays(argv[2], argv[3], argv[4]);
> > }
> > #endif
> > /* resize the fdt */
> > @@ -1080,6 +1166,7 @@ static char fdt_help_text[] =
> > "addr [-c] [-q] <addr> [<size>] - Set the [control] fdt location
> > to <addr>\n"
> > #ifdef CONFIG_OF_LIBFDT_OVERLAY
> > "fdt apply <addr> - Apply overlay to the DT\n"
> > + "fdt apply_all <ifname> <dev:part> <dir> - Apply all overlays in
> > directory\n"
> > #endif
> > #ifdef CONFIG_OF_BOARD_SETUP
> > "fdt boardsetup - Do board-specific set up\n"
> > --
> > 2.25.1
> >
