Re: [PATCH 3/3] fdt: introduce apply_all command

On Tue, 12 Jul 2022 04:58:35 -0600 Simon Glass sjg@chromium.org wrote:

Hi Simon,

many thanks for having a look!

On Tue, 5 Jul 2022 at 11:14, Andre Przywara andre.przywara@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@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