[PATCH 0/3] fdt: introduce "apply-all" command
Hi,
this is an attempt to simplify the usage of user provided devicetree overlays. The idea is to let the user drop all desired .dtbo files into one directory (for instance on the EFI system partition), and U-Boot just applies all of them, with generic commands: => fdt move $fdtcontroladdr $fdt_addr_r => fdt resize => fdt apply-all mmc 0:1 overlays/
This would pick all files ending in .dtbo from the /overlays directory found on the first partition of the first MMC device. Ideally the device type, number and partition name would be provided by the distroboot scripting, so it checks the media it scans for boot scripts anyways.
Patch 1 fixes the "fdt move" operation in the sandbox. Since hostfs does not support fs_opendir/fs_readdir, which apply-all relies on, this cannot be tested in the sandbox, but I figured this bug is worth fixing anyway. Patch 2 avoids a redundant call to "fdt addr", if we just want to move (actually copy) the DTB. This is needed when we want to build on the control DT, since this might live in an area where it cannot be changed or grow enough. Patch 3 then implements the main attraction: It uses the U-Boot filesystem API to fs_readdir() the given directory, reads all files ending in ".dtbo" into memory, and tries to apply them to the working DT.
Please let me know what you think of the idea in general and the implementation in particular. The first two patches are actually bug fixes, and should be applied regardless.
Cheers, Andre
Andre Przywara (3): cmd: fdt: move: Use map_sysmem to convert pointers cmd: fdt: allow standalone "fdt move" fdt: introduce apply_all command
cmd/fdt.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 108 insertions(+), 15 deletions(-)
The "fdt move" subcommand was using the provided DTB addresses directly, without trying to "map" them into U-Boot's address space. This happened to work since on the vast majority of "real" platforms there is a simple 1:1 mapping of VA to PAs, so either value works fine.
However this is not true on the sandbox, so the "fdt move" command fails there miserably: => fdt addr $fdtcontroladdr => cp.l $fdtcontroladdr $fdt_addr_r 40 # simple memcpy works => fdt move $fdtcontroladdr $fdt_addr_r Segmentation fault
Use the proper "map_sysmem" call to convert PAs to VAs, to make this more robust in general and to enable operation in the sandbox.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- cmd/fdt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/cmd/fdt.c b/cmd/fdt.c index 842e6cb634..abdc553b2b 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -211,11 +211,11 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) /* * Set the address and length of the fdt. */ - working_fdt = (struct fdt_header *)hextoul(argv[2], NULL); + working_fdt = map_sysmem(hextoul(argv[2], NULL), 0); if (!fdt_valid(&working_fdt)) return 1;
- newaddr = (struct fdt_header *)hextoul(argv[3], NULL); + newaddr = map_sysmem(hextoul(argv[3], NULL), 0);
/* * If the user specifies a length, use that. Otherwise use the @@ -242,7 +242,7 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) fdt_strerror(err)); return 1; } - set_working_fdt_addr((ulong)newaddr); + set_working_fdt_addr(map_to_sysmem(newaddr)); #ifdef CONFIG_OF_SYSTEM_SETUP /* Call the board-specific fixup routine */ } else if (strncmp(argv[1], "sys", 3) == 0) {
On Tue, 5 Jul 2022 at 11:14, Andre Przywara andre.przywara@arm.com wrote:
The "fdt move" subcommand was using the provided DTB addresses directly, without trying to "map" them into U-Boot's address space. This happened to work since on the vast majority of "real" platforms there is a simple 1:1 mapping of VA to PAs, so either value works fine.
However this is not true on the sandbox, so the "fdt move" command fails there miserably: => fdt addr $fdtcontroladdr => cp.l $fdtcontroladdr $fdt_addr_r 40 # simple memcpy works => fdt move $fdtcontroladdr $fdt_addr_r Segmentation fault
Use the proper "map_sysmem" call to convert PAs to VAs, to make this more robust in general and to enable operation in the sandbox.
Signed-off-by: Andre Przywara andre.przywara@arm.com
cmd/fdt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Tue, 5 Jul 2022 at 11:14, Andre Przywara andre.przywara@arm.com wrote:
The "fdt move" subcommand was using the provided DTB addresses directly, without trying to "map" them into U-Boot's address space. This happened to work since on the vast majority of "real" platforms there is a simple 1:1 mapping of VA to PAs, so either value works fine.
However this is not true on the sandbox, so the "fdt move" command fails there miserably: => fdt addr $fdtcontroladdr => cp.l $fdtcontroladdr $fdt_addr_r 40 # simple memcpy works => fdt move $fdtcontroladdr $fdt_addr_r Segmentation fault
Use the proper "map_sysmem" call to convert PAs to VAs, to make this more robust in general and to enable operation in the sandbox.
Signed-off-by: Andre Przywara andre.przywara@arm.com
cmd/fdt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, thanks!
At the moment every subcommand of "fdt", except "addr" itself, requires the DT address to be set first. We explicitly check for that before even comparing against the subcommands' string. This early bailout also affects the "move" subcommand, even though that does not require or rely on a previous call to "fdt addr". In fact it even sets the FDT address to the target of the move command, so is a perfect beginning for a sequence of fdt commands.
Move the check for a previously set FDT address to after we handle the "move" command also, so we don't need a dummy call to "fdt addr" first, before being able to move the devicetree.
This skips one pointless "fdt addr" call in scripts which aim to alter the control DT, but need to copy it to a safe location first (for instance to $fdt_addr_r).
Signed-off-by: Andre Przywara andre.przywara@arm.com --- cmd/fdt.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/cmd/fdt.c b/cmd/fdt.c index abdc553b2b..d6878c96f1 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -188,19 +188,11 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) }
return CMD_RET_SUCCESS; - } - - if (!working_fdt) { - puts("No FDT memory address configured. Please configure\n" - "the FDT address via "fdt addr <address>" command.\n" - "Aborting!\n"); - return CMD_RET_FAILURE; - }
/* * Move the working_fdt */ - if (strncmp(argv[1], "mo", 2) == 0) { + } else if (strncmp(argv[1], "mo", 2) == 0) { struct fdt_header *newaddr; int len; int err; @@ -243,9 +235,20 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return 1; } set_working_fdt_addr(map_to_sysmem(newaddr)); + + return CMD_RET_SUCCESS; + } + + if (!working_fdt) { + puts("No FDT memory address configured. Please configure\n" + "the FDT address via "fdt addr <address>" command.\n" + "Aborting!\n"); + return CMD_RET_FAILURE; + } + #ifdef CONFIG_OF_SYSTEM_SETUP /* Call the board-specific fixup routine */ - } else if (strncmp(argv[1], "sys", 3) == 0) { + if (strncmp(argv[1], "sys", 3) == 0) { int err = ft_system_setup(working_fdt, gd->bd);
if (err) { @@ -253,11 +256,14 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) fdt_strerror(err)); return CMD_RET_FAILURE; } + + return CMD_RET_SUCCESS; + } #endif /* * Make a new node */ - } else if (strncmp(argv[1], "mk", 2) == 0) { + if (strncmp(argv[1], "mk", 2) == 0) { char *pathp; /* path */ char *nodep; /* new node to add */ int nodeoffset; /* node offset from libfdt */
On Tue, 5 Jul 2022 at 11:14, Andre Przywara andre.przywara@arm.com wrote:
At the moment every subcommand of "fdt", except "addr" itself, requires the DT address to be set first. We explicitly check for that before even comparing against the subcommands' string. This early bailout also affects the "move" subcommand, even though that does not require or rely on a previous call to "fdt addr". In fact it even sets the FDT address to the target of the move command, so is a perfect beginning for a sequence of fdt commands.
Move the check for a previously set FDT address to after we handle the "move" command also, so we don't need a dummy call to "fdt addr" first, before being able to move the devicetree.
This skips one pointless "fdt addr" call in scripts which aim to alter the control DT, but need to copy it to a safe location first (for instance to $fdt_addr_r).
Signed-off-by: Andre Przywara andre.przywara@arm.com
cmd/fdt.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Tue, 5 Jul 2022 at 11:14, Andre Przywara andre.przywara@arm.com wrote:
At the moment every subcommand of "fdt", except "addr" itself, requires the DT address to be set first. We explicitly check for that before even comparing against the subcommands' string. This early bailout also affects the "move" subcommand, even though that does not require or rely on a previous call to "fdt addr". In fact it even sets the FDT address to the target of the move command, so is a perfect beginning for a sequence of fdt commands.
Move the check for a previously set FDT address to after we handle the "move" command also, so we don't need a dummy call to "fdt addr" first, before being able to move the devicetree.
This skips one pointless "fdt addr" call in scripts which aim to alter the control DT, but need to copy it to a safe location first (for instance to $fdt_addr_r).
Signed-off-by: Andre Przywara andre.przywara@arm.com
cmd/fdt.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, thanks!
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(-)
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"
Hi Andre,
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?
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' ?
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;elsesize = 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
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;elsesize = 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
Hi Andre,
On Wed, 13 Jul 2022 at 07:18, Andre Przywara andre.przywara@arm.com wrote:
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?
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
participants (2)
-
Andre Przywara -
Simon Glass