[PATCH v2 0/3] fdt: introduce "fsapply" command
Hi,
updating the series, basically just changing the name of the command to "fsapply", as suggested by Simon. The first two patches are still the same fixes, Simon said he merged them to u-boot-dm in July already, but I don't see them there or in master. I am still looking into a test for fsapply, but wanted to get at least the fixes merged now, hence this new version.
=============================== 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 fsapply 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 fsapply 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
Changelog v1 ... v2: - add Simon's tags - change command name from "apply-all" to "fsapply"
Andre Przywara (3): cmd: fdt: move: Use map_sysmem to convert pointers cmd: fdt: allow standalone "fdt move" fdt: introduce fsapply command
cmd/fdt.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 107 insertions(+), 14 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 Reviewed-by: Simon Glass sjg@chromium.org --- cmd/fdt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/cmd/fdt.c b/cmd/fdt.c index 8e51a431261..0ba691c573b 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -231,11 +231,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 @@ -262,7 +262,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) {
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 Reviewed-by: Simon Glass sjg@chromium.org --- cmd/fdt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
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 Reviewed-by: Simon Glass sjg@chromium.org --- cmd/fdt.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/cmd/fdt.c b/cmd/fdt.c index 0ba691c573b..1972490bdc2 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -208,19 +208,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; @@ -263,9 +255,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) { @@ -273,11 +276,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 */
Hi,
On Fri, 10 Feb 2023 11:02:12 +0000 Andre Przywara 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 Reviewed-by: Simon Glass sjg@chromium.org
cmd/fdt.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/cmd/fdt.c b/cmd/fdt.c index 0ba691c573b..1972490bdc2 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -208,19 +208,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;
@@ -263,9 +255,20 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return 1; }
It's not part of your changes, but this should rather be: return CMD_RET_FAILURE;
Lothar Waßmann
Hi,
On Fri, 10 Feb 2023 11:02:12 +0000 Andre Przywara 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 Reviewed-by: Simon Glass sjg@chromium.org
cmd/fdt.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-)
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 fsapply" 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 | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+)
diff --git a/cmd/fdt.c b/cmd/fdt.c index 1972490bdc2..00f92dbbb5d 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 */ @@ -127,6 +129,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. */ @@ -747,6 +824,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], "fsap", 4) == 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 */ @@ -1104,6 +1190,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 fsapply <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,
On Fri, 10 Feb 2023 11:02:13 +0000 Andre Przywara 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 fsapply" 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 | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+)
diff --git a/cmd/fdt.c b/cmd/fdt.c index 1972490bdc2..00f92dbbb5d 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -127,6 +129,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;
return CMD_RET_SUCCESS;
Lothar Waßmann
Hi,
On Fri, 10 Feb 2023 at 04:32, Lothar Waßmann LW@karo-electronics.de wrote:
Hi,
On Fri, 10 Feb 2023 11:02:13 +0000 Andre Przywara 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 fsapply" 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 | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+)
Please add some help at doc/usage/cmd
Also please add a test for this subcommand in test/cmd
diff --git a/cmd/fdt.c b/cmd/fdt.c index 1972490bdc2..00f92dbbb5d 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -127,6 +129,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;return CMD_RET_SUCCESS;
There is no need for that...0 means success in U-Boot. It is shorter and clearer IMO.
Regards, SImon
On Fri, 10 Feb 2023 09:05:34 -0700 Simon Glass sjg@chromium.org wrote:
Hi Simon,
On Fri, 10 Feb 2023 at 04:32, Lothar Waßmann LW@karo-electronics.de wrote:
Hi,
On Fri, 10 Feb 2023 11:02:13 +0000 Andre Przywara 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 fsapply" 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 | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+)
Please add some help at doc/usage/cmd
Right, will do.
Also please add a test for this subcommand in test/cmd
Yeah, I knew you would say that ;-) It's still the same problem as last time: sandboxfs doesn't implement .readdir, so this doesn't work easily there. So I started with filling this gap, and was just wondering if I should piggy back on the already existing sandbox_fs_ls abstraction, and somewhat re-translate this back into dirent structures, or whether I should properly wrap {open,read,close}dir in os_*dir() helpers, and build sandbox_fs_readdir() based on that?
Any advice? Both seem equally doable.
Cheers, Andre
diff --git a/cmd/fdt.c b/cmd/fdt.c index 1972490bdc2..00f92dbbb5d 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -127,6 +129,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;return CMD_RET_SUCCESS;
There is no need for that...0 means success in U-Boot. It is shorter and clearer IMO.
Regards, SImon
Hi Andre,
On Mon, 13 Feb 2023 at 10:26, Andre Przywara andre.przywara@arm.com wrote:
On Fri, 10 Feb 2023 09:05:34 -0700 Simon Glass sjg@chromium.org wrote:
Hi Simon,
On Fri, 10 Feb 2023 at 04:32, Lothar Waßmann LW@karo-electronics.de wrote:
Hi,
On Fri, 10 Feb 2023 11:02:13 +0000 Andre Przywara 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 fsapply" 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 | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+)
Please add some help at doc/usage/cmd
Right, will do.
Also please add a test for this subcommand in test/cmd
Yeah, I knew you would say that ;-) It's still the same problem as last time: sandboxfs doesn't implement .readdir, so this doesn't work easily there. So I started with filling this gap, and was just wondering if I should piggy back on the already existing sandbox_fs_ls abstraction, and somewhat re-translate this back into dirent structures, or whether I should properly wrap {open,read,close}dir in os_*dir() helpers, and build sandbox_fs_readdir() based on that?
Any advice? Both seem equally doable.
I like option two as I think it will help in the future too!
Regards, SImon
Cheers, Andre
diff --git a/cmd/fdt.c b/cmd/fdt.c index 1972490bdc2..00f92dbbb5d 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -127,6 +129,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;return CMD_RET_SUCCESS;
There is no need for that...0 means success in U-Boot. It is shorter and clearer IMO.
Regards, SImon
On 2/10/23 12:02, Andre Przywara wrote:
Hi,
updating the series, basically just changing the name of the command to "fsapply", as suggested by Simon. The first two patches are still the same fixes, Simon said he merged them to u-boot-dm in July already, but I don't see them there or in master. I am still looking into a test for fsapply, but wanted to get at least the fixes merged now, hence this new version.
=============================== 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 fsapply 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 fsapply 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
Changelog v1 ... v2:
- add Simon's tags
- change command name from "apply-all" to "fsapply"
Andre Przywara (3): cmd: fdt: move: Use map_sysmem to convert pointers cmd: fdt: allow standalone "fdt move" fdt: introduce fsapply command
cmd/fdt.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 107 insertions(+), 14 deletions(-)
Hello Andre,
We should document this change in a document doc/usage/cmd/fdt.rst.
Best regards
Heinrich
participants (4)
-
Andre Przywara -
Heinrich Schuchardt -
Lothar Waßmann -
Simon Glass