diff mbox series

[v2,2/3] cmd: fdt: allow standalone "fdt move"

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

Commit Message

Andre Przywara Feb. 10, 2023, 11:02 a.m. UTC
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(-)

Comments

Lothar Waßmann Feb. 10, 2023, 11:34 a.m. UTC | #1
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
Simon Glass Feb. 13, 2023, 12:34 a.m. UTC | #2
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 mbox series

Patch

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 */