Message ID | 20230210110213.2531190-3-andre.przywara@arm.com |
---|---|
State | Accepted |
Commit | 4ee85df9ce0b6385a28f538af31680eed4ee153e |
Delegated to: | Simon Glass |
Headers | show |
Series | fdt: introduce "fsapply" command | expand |
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!
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 */