[PATCH 00/29] bootm: Refactoring to reduce reliance on CMDLINE (part A)

[PATCH 00/29] bootm: Refactoring to reduce reliance on CMDLINE (part A)

[Simon Glass]

It would be useful to be able to boot an OS when CONFIG_CMDLINE is
disabled. This could allow reduced code size.

Standard boot provides a way to handle programmatic boot, without
scripts, so such a feature is possible. The main impediment is the
inability to use the booting features of U-Boot without a command line.
So the solution is to avoid passing command arguments and the like to
code in boot/

A similar process has taken place with filesystems, for example, where
we have (somewhat) separate Kconfig options for the filesystem commands
and the filesystems themselves.

This series starts the process of refactoring the bootm logic so that
it can be called from standard boot without using the command line.
Mostly it removes the use of argc, argv and cmdtbl from the internal
logic.

Some limited tidy-up is included, but this is kept to smaller patches,
rather than trying to remove all #ifdefs etc. Some function comments
are added, however.

A simple programmatic boot is provided as a starting point.

This work will likely take many series, so this is just the start.

Size growth with this series for firefly-rk3288 (Thumb2) is:

       arm: (for 1/1 boards) all +23.0 rodata -49.0 text +72.0

This should be removed by:

   https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/11

but it is not included in this series as it is already large enough.

No functional change is intended in this series.


Simon Glass (29):
  arm: x86: Drop discarding of command linker-lists
  mmc: env: Unify the U_BOOT_ENV_LOCATION conditions
  treewide: Tidy up semicolon after command macros
  bootstd: Add missing header file from bootdev.h
  bootstd: Introduce programmable boot
  bootm: Drop arguments from bootm_start()
  bootm: Simplify arguments for bootm_pre_load()
  bootm: Move boot_get_kernel() higher in the file
  image: Tidy up genimg_get_kernel_addr_fit()
  bootm: Reduce arguments to boot_get_kernel()
  image: Document error codes from fit_image_load()
  bootm: Adjust boot_get_kernel() to return an error
  bootm: Use the error return from boot_get_kernel()
  bootstage: Drop BOOTSTAGE_ID_FIT_KERNEL_INFO
  bootm: Move error printing out of boot_get_kernel()
  bootm: Reduce arguments to boot_find_os()
  bootm: Reduce arguments to boot_get_ramdisk()
  fdt: Allow use of fdt_support inside if() statements
  bootm: Drop #ifdef in bootm_find_images()
  bootm: Pass image buffer to boot_get_fdt()
  bootm: Reduce arguments to boot_get_fdt()
  bootm: Reduce arguments to boot_get_fpga()
  bootm: Reduce arguments to boot_get_loadables()
  bootm: Simplify Android ramdisk addr in bootm_find_images()
  bootm: efi: Drop special call to bootm_find_other()
  bootm: optee: Drop special call to bootm_find_other()
  bootm: Adjust the parameters of bootm_find_images()
  bootm: Add a function to check overlap
  bootm: Reduce arguments to bootm_find_other()

 arch/arm/cpu/u-boot.lds      |   3 -
 arch/x86/cpu/u-boot-64.lds   |   4 -
 arch/x86/cpu/u-boot-spl.lds  |   4 -
 arch/x86/cpu/u-boot.lds      |   4 -
 board/freescale/common/vid.c |   2 +-
 board/xilinx/common/fru.c    |   2 +-
 board/xilinx/versal/cmds.c   |   2 +-
 board/xilinx/zynqmp/cmds.c   |   2 +-
 boot/Kconfig                 |  11 +
 boot/Makefile                |   2 +
 boot/bootm.c                 | 576 +++++++++++++++++++----------------
 boot/bootm_os.c              |  16 -
 boot/image-board.c           |  67 +---
 boot/image-fdt.c             |  39 +--
 boot/prog_boot.c             |  51 ++++
 cmd/booti.c                  |   4 +-
 cmd/bootz.c                  |   4 +-
 cmd/btrfs.c                  |   2 +-
 cmd/eeprom.c                 |   2 +-
 cmd/ext2.c                   |   4 +-
 cmd/fs.c                     |   8 +-
 cmd/pinmux.c                 |   2 +-
 cmd/qfw.c                    |   2 +-
 common/main.c                |   9 +
 env/mmc.c                    |   2 +-
 include/bootdev.h            |   1 +
 include/bootm.h              |  26 +-
 include/bootstage.h          |   1 -
 include/bootstd.h            |   9 +
 include/command.h            |   2 +-
 include/fdt_support.h        |   5 +-
 include/image.h              | 127 ++++++--
 32 files changed, 550 insertions(+), 445 deletions(-)
 create mode 100644 boot/prog_boot.c

-- 
2.42.0.869.gea05f2083d-goog

[PATCH 01/29] arm: x86: Drop discarding of command linker-lists

[Simon Glass]

Since we can now cleanly disable CMDLINE when needed, drop the rules
which discard the command code.  It will not be built in the first
place.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/arm/cpu/u-boot.lds     | 3 ---
 arch/x86/cpu/u-boot-64.lds  | 4 ----
 arch/x86/cpu/u-boot-spl.lds | 4 ----
 arch/x86/cpu/u-boot.lds     | 4 ----
 4 files changed, 15 deletions(-)

diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index fc4f63d83489..7724c9332c3b 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -14,9 +14,6 @@ OUTPUT_ARCH(arm)
 ENTRY(_start)
 SECTIONS
 {
-#ifndef CONFIG_CMDLINE
-	/DISCARD/ : { *(__u_boot_list_2_cmd_*) }
-#endif
 #if defined(CONFIG_ARMV7_SECURE_BASE) && defined(CONFIG_ARMV7_NONSEC)
 	/*
 	 * If CONFIG_ARMV7_SECURE_BASE is true, secure code will not
diff --git a/arch/x86/cpu/u-boot-64.lds b/arch/x86/cpu/u-boot-64.lds
index d0398ff00d71..00a6d8691211 100644
--- a/arch/x86/cpu/u-boot-64.lds
+++ b/arch/x86/cpu/u-boot-64.lds
@@ -11,10 +11,6 @@ ENTRY(_start)
 
 SECTIONS
 {
-#ifndef CONFIG_CMDLINE
-	/DISCARD/ : { *(__u_boot_list_2_cmd_*) }
-#endif
-
 #ifdef CONFIG_TEXT_BASE
 	. = CONFIG_TEXT_BASE;	/* Location of bootcode in flash */
 #endif
diff --git a/arch/x86/cpu/u-boot-spl.lds b/arch/x86/cpu/u-boot-spl.lds
index a0a2a06a18cd..50b4b1608552 100644
--- a/arch/x86/cpu/u-boot-spl.lds
+++ b/arch/x86/cpu/u-boot-spl.lds
@@ -11,10 +11,6 @@ ENTRY(_start)
 
 SECTIONS
 {
-#ifndef CONFIG_CMDLINE
-	/DISCARD/ : { *(__u_boot_list_2_cmd_*) }
-#endif
-
 	. = IMAGE_TEXT_BASE;	/* Location of bootcode in flash */
 	__text_start = .;
 	.text  : {
diff --git a/arch/x86/cpu/u-boot.lds b/arch/x86/cpu/u-boot.lds
index a31f4220a000..c418ff44aa08 100644
--- a/arch/x86/cpu/u-boot.lds
+++ b/arch/x86/cpu/u-boot.lds
@@ -11,10 +11,6 @@ ENTRY(_start)
 
 SECTIONS
 {
-#ifndef CONFIG_CMDLINE
-	/DISCARD/ : { *(__u_boot_list_2_cmd_*) }
-#endif
-
 	. = CONFIG_TEXT_BASE;	/* Location of bootcode in flash */
 	__text_start = .;
 
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 02/29] mmc: env: Unify the U_BOOT_ENV_LOCATION conditions

[Simon Glass]

The declaration of U_BOOT_ENV_LOCATION() uses a different #ifdef
condition from the code it calls. Use the same condition to avoid a
build warning if CONFIG_CMD_SAVEENV is disabled.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 env/mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/env/mmc.c b/env/mmc.c
index cb14bbb58f13..da84cddd74f0 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -495,7 +495,7 @@ U_BOOT_ENV_LOCATION(mmc) = {
 	.location	= ENVL_MMC,
 	ENV_NAME("MMC")
 	.load		= env_mmc_load,
-#ifndef CONFIG_SPL_BUILD
+#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_SPL_BUILD)
 	.save		= env_save_ptr(env_mmc_save),
 	.erase		= ENV_ERASE_PTR(env_mmc_erase)
 #endif
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 03/29] treewide: Tidy up semicolon after command macros

[Simon Glass]

The U_BOOT_CMD_COMPLETE() macro has a semicolon at the end, perhaps
inadvertently. Some code has taken advantage of this.

Tidy this up by dropping the semicolon from the macro and adding it to
macro invocations as required.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 board/freescale/common/vid.c | 2 +-
 board/xilinx/common/fru.c    | 2 +-
 board/xilinx/versal/cmds.c   | 2 +-
 board/xilinx/zynqmp/cmds.c   | 2 +-
 cmd/btrfs.c                  | 2 +-
 cmd/eeprom.c                 | 2 +-
 cmd/ext2.c                   | 4 ++--
 cmd/fs.c                     | 8 ++++----
 cmd/pinmux.c                 | 2 +-
 cmd/qfw.c                    | 2 +-
 include/command.h            | 2 +-
 11 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/board/freescale/common/vid.c b/board/freescale/common/vid.c
index 5ec3f2a76b19..fc5d400cfe18 100644
--- a/board/freescale/common/vid.c
+++ b/board/freescale/common/vid.c
@@ -793,4 +793,4 @@ U_BOOT_CMD(
 	vdd_read, 1, 0, do_vdd_read,
 	"read VDD",
 	" - Read the voltage specified in mV"
-)
+);
diff --git a/board/xilinx/common/fru.c b/board/xilinx/common/fru.c
index c916c3d6b4c8..12b21317496a 100644
--- a/board/xilinx/common/fru.c
+++ b/board/xilinx/common/fru.c
@@ -85,4 +85,4 @@ U_BOOT_CMD(
 	fru, 8, 1, do_fru,
 	"FRU table info",
 	fru_help_text
-)
+);
diff --git a/board/xilinx/versal/cmds.c b/board/xilinx/versal/cmds.c
index 9cc2cdcebf1c..2a74e49aedec 100644
--- a/board/xilinx/versal/cmds.c
+++ b/board/xilinx/versal/cmds.c
@@ -98,4 +98,4 @@ U_BOOT_LONGHELP(versal,
 U_BOOT_CMD(versal, 4, 1, do_versal,
 	   "versal sub-system",
 	   versal_help_text
-)
+);
diff --git a/board/xilinx/zynqmp/cmds.c b/board/xilinx/zynqmp/cmds.c
index f1f3eff501e1..9524688f27d9 100644
--- a/board/xilinx/zynqmp/cmds.c
+++ b/board/xilinx/zynqmp/cmds.c
@@ -427,4 +427,4 @@ U_BOOT_CMD(
 	zynqmp, 9, 1, do_zynqmp,
 	"ZynqMP sub-system",
 	zynqmp_help_text
-)
+);
diff --git a/cmd/btrfs.c b/cmd/btrfs.c
index 98daea99e9ed..2843835d08b8 100644
--- a/cmd/btrfs.c
+++ b/cmd/btrfs.c
@@ -24,4 +24,4 @@ U_BOOT_CMD(btrsubvol, 3, 1, do_btrsubvol,
 	"list subvolumes of a BTRFS filesystem",
 	"<interface> <dev[:part]>\n"
 	"     - List subvolumes of a BTRFS filesystem."
-)
+);
diff --git a/cmd/eeprom.c b/cmd/eeprom.c
index 0b6ca8c505fb..322765ad02a0 100644
--- a/cmd/eeprom.c
+++ b/cmd/eeprom.c
@@ -435,4 +435,4 @@ U_BOOT_CMD(
 	"The values which can be provided with the -l option are:\n"
 	CONFIG_EEPROM_LAYOUT_HELP_STRING"\n"
 #endif
-)
+);
diff --git a/cmd/ext2.c b/cmd/ext2.c
index 57a99516a6ac..a0ce0cf5796b 100644
--- a/cmd/ext2.c
+++ b/cmd/ext2.c
@@ -42,7 +42,7 @@ U_BOOT_CMD(
 	"list files in a directory (default /)",
 	"<interface> <dev[:part]> [directory]\n"
 	"    - list files from 'dev' on 'interface' in a 'directory'"
-)
+);
 
 U_BOOT_CMD(
 	ext2load,	6,	0,	do_ext2load,
@@ -50,4 +50,4 @@ U_BOOT_CMD(
 	"<interface> [<dev[:part]> [addr [filename [bytes [pos]]]]]\n"
 	"    - load binary file 'filename' from 'dev' on 'interface'\n"
 	"      to address 'addr' from ext2 filesystem."
-)
+);
diff --git a/cmd/fs.c b/cmd/fs.c
index 6044f73af5b4..46cb43dcdb5b 100644
--- a/cmd/fs.c
+++ b/cmd/fs.c
@@ -39,7 +39,7 @@ U_BOOT_CMD(
 	"      If 'bytes' is 0 or omitted, the file is read until the end.\n"
 	"      'pos' gives the file byte position to start reading from.\n"
 	"      If 'pos' is 0 or omitted, the file is read from the start."
-)
+);
 
 static int do_save_wrapper(struct cmd_tbl *cmdtp, int flag, int argc,
 			   char *const argv[])
@@ -56,7 +56,7 @@ U_BOOT_CMD(
 	"      'bytes' gives the size to save in bytes and is mandatory.\n"
 	"      'pos' gives the file byte position to start writing to.\n"
 	"      If 'pos' is 0 or omitted, the file is written from the start."
-)
+);
 
 static int do_ls_wrapper(struct cmd_tbl *cmdtp, int flag, int argc,
 			 char *const argv[])
@@ -70,7 +70,7 @@ U_BOOT_CMD(
 	"<interface> [<dev[:part]> [directory]]\n"
 	"    - List files in directory 'directory' of partition 'part' on\n"
 	"      device type 'interface' instance 'dev'."
-)
+);
 
 static int do_ln_wrapper(struct cmd_tbl *cmdtp, int flag, int argc,
 			 char *const argv[])
@@ -84,7 +84,7 @@ U_BOOT_CMD(
 	"<interface> <dev[:part]> target linkname\n"
 	"    - create a symbolic link to 'target' with the name 'linkname' on\n"
 	"      device type 'interface' instance 'dev'."
-)
+);
 
 static int do_fstype_wrapper(struct cmd_tbl *cmdtp, int flag, int argc,
 			     char *const argv[])
diff --git a/cmd/pinmux.c b/cmd/pinmux.c
index f17cf4110d9f..105f01eaafff 100644
--- a/cmd/pinmux.c
+++ b/cmd/pinmux.c
@@ -178,4 +178,4 @@ U_BOOT_CMD(pinmux, CONFIG_SYS_MAXARGS, 1, do_pinmux,
 	   "list                     - list UCLASS_PINCTRL devices\n"
 	   "pinmux dev [pincontroller-name] - select pin-controller device\n"
 	   "pinmux status [-a | pin-name]   - print pin-controller muxing [for all | for pin-name]\n"
-)
+);
diff --git a/cmd/qfw.c b/cmd/qfw.c
index d6ecfa60d5a7..1b8c775ebf5a 100644
--- a/cmd/qfw.c
+++ b/cmd/qfw.c
@@ -121,4 +121,4 @@ U_BOOT_CMD(
 	"    - list                             : print firmware(s) currently loaded\n"
 	"    - cpus                             : print online cpu number\n"
 	"    - load <kernel addr> <initrd addr> : load kernel and initrd (if any), and setup for zboot\n"
-)
+);
diff --git a/include/command.h b/include/command.h
index 6262365e128f..5bd3ecbe8f91 100644
--- a/include/command.h
+++ b/include/command.h
@@ -390,7 +390,7 @@ int cmd_source_script(ulong addr, const char *fit_uname, const char *confname);
 #define U_BOOT_CMD_COMPLETE(_name, _maxargs, _rep, _cmd, _usage, _help, _comp) \
 	ll_entry_declare(struct cmd_tbl, _name, cmd) =			\
 		U_BOOT_CMD_MKENT_COMPLETE(_name, _maxargs, _rep, _cmd,	\
-						_usage, _help, _comp);
+						_usage, _help, _comp)
 
 #define U_BOOT_CMDREP_COMPLETE(_name, _maxargs, _cmd_rep, _usage,	\
 			       _help, _comp)				\
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 04/29] bootstd: Add missing header file from bootdev.h

[Simon Glass]

Add a dm/uclass-id.h to the bootdev header file, since it uses
enum uclass_id

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/bootdev.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/bootdev.h b/include/bootdev.h
index b079a91b5b7f..c1362673d418 100644
--- a/include/bootdev.h
+++ b/include/bootdev.h
@@ -7,6 +7,7 @@
 #ifndef __bootdev_h
 #define __bootdev_h
 
+#include <dm/uclass-id.h>
 #include <linux/list.h>
 
 struct bootflow;
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 05/29] bootstd: Introduce programmable boot

[Simon Glass]

At present bootstd requires CONFIG_CMDLINE to operate. Add a new
'programmable' boot which can be used when no command line is available.
For now it does almost nothing, since most bootmeths require the
command line.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/Kconfig      | 11 ++++++++++
 boot/Makefile     |  2 ++
 boot/prog_boot.c  | 51 +++++++++++++++++++++++++++++++++++++++++++++++
 common/main.c     |  9 +++++++++
 include/bootstd.h |  9 +++++++++
 5 files changed, 82 insertions(+)
 create mode 100644 boot/prog_boot.c

diff --git a/boot/Kconfig b/boot/Kconfig
index ef71883a5026..586d7a69fab3 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -459,6 +459,17 @@ config BOOTSTD_BOOTCOMMAND
 	  standard boot does not support all of the features of distro boot
 	  yet.
 
+config BOOTSTD_PROG
+	bool "Use programmatic boot"
+	depends on !CMDLINE
+	default y
+	help
+	  Enable this to provide a board_run_command() function which can boot
+	  a systen without using commands.
+
+	  Note: This currently has many limitations and is not a useful booting
+	  solution. Future work will eventually make this a viable option.
+
 config BOOTMETH_GLOBAL
 	bool
 	help
diff --git a/boot/Makefile b/boot/Makefile
index 3fd048bb41ab..de0eafed14b1 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -25,6 +25,8 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootflow.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootmeth-uclass.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootstd-uclass.o
 
+obj-$(CONFIG_$(SPL_TPL_)BOOTSTD_PROG) += prog_boot.o
+
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EXTLINUX) += bootmeth_extlinux.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EXTLINUX_PXE) += bootmeth_pxe.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EFILOADER) += bootmeth_efi.o
diff --git a/boot/prog_boot.c b/boot/prog_boot.c
new file mode 100644
index 000000000000..045554b93db0
--- /dev/null
+++ b/boot/prog_boot.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#define LOG_CATEGORY UCLASS_BOOTSTD
+
+#include <bootflow.h>
+#include <bootstd.h>
+#include <command.h>
+#include <dm.h>
+
+/*
+ * show_bootmeths() - List available bootmeths
+ *
+ * We could refactor this to use do_bootmeth_list() if more detail (or ordering)
+ * are needed
+ */
+static void show_bootmeths(void)
+{
+	struct udevice *dev;
+	struct uclass *uc;
+
+	printf("Bootmeths: ");
+	uclass_id_foreach_dev(UCLASS_BOOTMETH, dev, uc)
+		printf(" %s", dev->name);
+	printf("\n");
+}
+
+int bootstd_prog_boot(void)
+{
+	struct bootflow_iter iter;
+	struct bootflow bflow;
+	int ret, flags, i;
+
+	printf("Programmatic boot starting\n");
+	show_bootmeths();
+	flags = BOOTFLOWIF_HUNT | BOOTFLOWIF_SHOW | BOOTFLOWIF_SKIP_GLOBAL;
+
+	bootstd_clear_glob();
+	for (i = 0, ret = bootflow_scan_first(NULL, NULL, &iter, flags, &bflow);
+	     i < 1000 && ret != -ENODEV;
+	     i++, ret = bootflow_scan_next(&iter, &bflow)) {
+		if (!bflow.err)
+			bootflow_run_boot(&iter, &bflow);
+		bootflow_free(&bflow);
+	}
+
+	return -EFAULT;
+}
diff --git a/common/main.c b/common/main.c
index 7c70de2e59a8..2668f3bb1637 100644
--- a/common/main.c
+++ b/common/main.c
@@ -9,6 +9,7 @@
 #include <common.h>
 #include <autoboot.h>
 #include <bootstage.h>
+#include <bootstd.h>
 #include <cli.h>
 #include <command.h>
 #include <console.h>
@@ -67,6 +68,14 @@ void main_loop(void)
 
 	autoboot_command(s);
 
+	if (IS_ENABLED(CONFIG_BOOTSTD_PROG)) {
+		int ret;
+
+		ret = bootstd_prog_boot();
+		printf("Standard boot failed (err=%dE)\n", ret);
+	}
+
 	cli_loop();
+
 	panic("No CLI available");
 }
diff --git a/include/bootstd.h b/include/bootstd.h
index 7802564bcc63..99ce7b64e7c5 100644
--- a/include/bootstd.h
+++ b/include/bootstd.h
@@ -94,4 +94,13 @@ int bootstd_get_priv(struct bootstd_priv **stdp);
  */
 void bootstd_clear_glob(void);
 
+/**
+ * bootstd_prog_boot() - Run standard boot in a fully programmatic mode
+ *
+ * Attempts to boot without making any use of U-Boot commands
+ *
+ * Returns: -ve error value (does not return except on failure to boot)
+ */
+int bootstd_prog_boot(void);
+
 #endif
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 06/29] bootm: Drop arguments from bootm_start()

[Simon Glass]

This function does not use its arguments. Drop them.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index cb61485c226c..fda97706fc26 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -69,8 +69,7 @@ static void boot_start_lmb(struct bootm_headers *images)
 static inline void boot_start_lmb(struct bootm_headers *images) { }
 #endif
 
-static int bootm_start(struct cmd_tbl *cmdtp, int flag, int argc,
-		       char *const argv[])
+static int bootm_start(void)
 {
 	memset((void *)&images, 0, sizeof(images));
 	images.verify = env_get_yesno("verify");
@@ -783,7 +782,7 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc,
 	 * any error.
 	 */
 	if (states & BOOTM_STATE_START)
-		ret = bootm_start(cmdtp, flag, argc, argv);
+		ret = bootm_start();
 
 	if (!ret && (states & BOOTM_STATE_PRE_LOAD))
 		ret = bootm_pre_load(cmdtp, flag, argc, argv);
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 07/29] bootm: Simplify arguments for bootm_pre_load()

[Simon Glass]

Move the argument decoding to the caller, to avoid needing to pass the
command-line arguments.

Add a function comment while we are here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index fda97706fc26..1482a2cc783f 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -82,22 +82,31 @@ static int bootm_start(void)
 	return 0;
 }
 
-static ulong bootm_data_addr(int argc, char *const argv[])
+static ulong bootm_data_addr(const char *addr_str)
 {
 	ulong addr;
 
-	if (argc > 0)
-		addr = simple_strtoul(argv[0], NULL, 16);
+	if (addr_str)
+		addr = hextoul(addr_str, NULL);
 	else
 		addr = image_load_addr;
 
 	return addr;
 }
 
-static int bootm_pre_load(struct cmd_tbl *cmdtp, int flag, int argc,
-			  char *const argv[])
+/**
+ * bootm_pre_load() - Handle the pre-load processing
+ *
+ * This can be used to do a full signature check of the image, for example.
+ * It calls image_pre_load() with the data address of the image to check.
+ *
+ * @addr_str: String containing load address in hex, or NULL to use
+ * image_load_addr
+ * Return: 0 if OK, CMD_RET_FAILURE on failure
+ */
+static int bootm_pre_load(const char *addr_str)
 {
-	ulong data_addr = bootm_data_addr(argc, argv);
+	ulong data_addr = bootm_data_addr(addr_str);
 	int ret = 0;
 
 	if (IS_ENABLED(CONFIG_CMD_BOOTM_PRE_LOAD))
@@ -785,7 +794,7 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc,
 		ret = bootm_start();
 
 	if (!ret && (states & BOOTM_STATE_PRE_LOAD))
-		ret = bootm_pre_load(cmdtp, flag, argc, argv);
+		ret = bootm_pre_load(argv[0]);
 
 	if (!ret && (states & BOOTM_STATE_FINDOS))
 		ret = bootm_find_os(cmdtp, flag, argc, argv);
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 08/29] bootm: Move boot_get_kernel() higher in the file

[Simon Glass]

Move this code and image_get_kernel() higher in the file to avoid the
need for a forward declaration.

No attempt is made to remove #ifdefs or adjust the code in any other
way.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c | 376 +++++++++++++++++++++++++--------------------------
 1 file changed, 186 insertions(+), 190 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 1482a2cc783f..598f880d86dd 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -44,12 +44,195 @@ DECLARE_GLOBAL_DATA_PTR;
 
 struct bootm_headers images;		/* pointers to os/initrd/fdt images */
 
+__weak void board_quiesce_devices(void)
+{
+}
+
+#if CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)
+/**
+ * image_get_kernel - verify legacy format kernel image
+ * @img_addr: in RAM address of the legacy format image to be verified
+ * @verify: data CRC verification flag
+ *
+ * image_get_kernel() verifies legacy image integrity and returns pointer to
+ * legacy image header if image verification was completed successfully.
+ *
+ * returns:
+ *     pointer to a legacy image header if valid image was found
+ *     otherwise return NULL
+ */
+static struct legacy_img_hdr *image_get_kernel(ulong img_addr, int verify)
+{
+	struct legacy_img_hdr *hdr = (struct legacy_img_hdr *)img_addr;
+
+	if (!image_check_magic(hdr)) {
+		puts("Bad Magic Number\n");
+		bootstage_error(BOOTSTAGE_ID_CHECK_MAGIC);
+		return NULL;
+	}
+	bootstage_mark(BOOTSTAGE_ID_CHECK_HEADER);
+
+	if (!image_check_hcrc(hdr)) {
+		puts("Bad Header Checksum\n");
+		bootstage_error(BOOTSTAGE_ID_CHECK_HEADER);
+		return NULL;
+	}
+
+	bootstage_mark(BOOTSTAGE_ID_CHECK_CHECKSUM);
+	image_print_contents(hdr);
+
+	if (verify) {
+		puts("   Verifying Checksum ... ");
+		if (!image_check_dcrc(hdr)) {
+			printf("Bad Data CRC\n");
+			bootstage_error(BOOTSTAGE_ID_CHECK_CHECKSUM);
+			return NULL;
+		}
+		puts("OK\n");
+	}
+	bootstage_mark(BOOTSTAGE_ID_CHECK_ARCH);
+
+	if (!image_check_target_arch(hdr)) {
+		printf("Unsupported Architecture 0x%x\n", image_get_arch(hdr));
+		bootstage_error(BOOTSTAGE_ID_CHECK_ARCH);
+		return NULL;
+	}
+	return hdr;
+}
+#endif
+
+/**
+ * boot_get_kernel - find kernel image
+ * @os_data: pointer to a ulong variable, will hold os data start address
+ * @os_len: pointer to a ulong variable, will hold os data length
+ *
+ * boot_get_kernel() tries to find a kernel image, verifies its integrity
+ * and locates kernel data.
+ *
+ * returns:
+ *     pointer to image header if valid image was found, plus kernel start
+ *     address and length, otherwise NULL
+ */
 static const void *boot_get_kernel(struct cmd_tbl *cmdtp, int flag, int argc,
 				   char *const argv[], struct bootm_headers *images,
-				   ulong *os_data, ulong *os_len);
-
-__weak void board_quiesce_devices(void)
+				   ulong *os_data, ulong *os_len)
 {
+#if CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)
+	struct legacy_img_hdr	*hdr;
+#endif
+	ulong		img_addr;
+	const void *buf;
+	const char	*fit_uname_config = NULL;
+	const char	*fit_uname_kernel = NULL;
+#if CONFIG_IS_ENABLED(FIT)
+	int		os_noffset;
+#endif
+
+#ifdef CONFIG_ANDROID_BOOT_IMAGE
+	const void *boot_img;
+	const void *vendor_boot_img;
+#endif
+	img_addr = genimg_get_kernel_addr_fit(argc < 1 ? NULL : argv[0],
+					      &fit_uname_config,
+					      &fit_uname_kernel);
+
+	if (IS_ENABLED(CONFIG_CMD_BOOTM_PRE_LOAD))
+		img_addr += image_load_offset;
+
+	bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);
+
+	/* check image type, for FIT images get FIT kernel node */
+	*os_data = *os_len = 0;
+	buf = map_sysmem(img_addr, 0);
+	switch (genimg_get_format(buf)) {
+#if CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)
+	case IMAGE_FORMAT_LEGACY:
+		printf("## Booting kernel from Legacy Image at %08lx ...\n",
+		       img_addr);
+		hdr = image_get_kernel(img_addr, images->verify);
+		if (!hdr)
+			return NULL;
+		bootstage_mark(BOOTSTAGE_ID_CHECK_IMAGETYPE);
+
+		/* get os_data and os_len */
+		switch (image_get_type(hdr)) {
+		case IH_TYPE_KERNEL:
+		case IH_TYPE_KERNEL_NOLOAD:
+			*os_data = image_get_data(hdr);
+			*os_len = image_get_data_size(hdr);
+			break;
+		case IH_TYPE_MULTI:
+			image_multi_getimg(hdr, 0, os_data, os_len);
+			break;
+		case IH_TYPE_STANDALONE:
+			*os_data = image_get_data(hdr);
+			*os_len = image_get_data_size(hdr);
+			break;
+		default:
+			printf("Wrong Image Type for %s command\n",
+			       cmdtp->name);
+			bootstage_error(BOOTSTAGE_ID_CHECK_IMAGETYPE);
+			return NULL;
+		}
+
+		/*
+		 * copy image header to allow for image overwrites during
+		 * kernel decompression.
+		 */
+		memmove(&images->legacy_hdr_os_copy, hdr,
+			sizeof(struct legacy_img_hdr));
+
+		/* save pointer to image header */
+		images->legacy_hdr_os = hdr;
+
+		images->legacy_hdr_valid = 1;
+		bootstage_mark(BOOTSTAGE_ID_DECOMP_IMAGE);
+		break;
+#endif
+#if CONFIG_IS_ENABLED(FIT)
+	case IMAGE_FORMAT_FIT:
+		os_noffset = fit_image_load(images, img_addr,
+				&fit_uname_kernel, &fit_uname_config,
+				IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
+				BOOTSTAGE_ID_FIT_KERNEL_START,
+				FIT_LOAD_IGNORED, os_data, os_len);
+		if (os_noffset < 0)
+			return NULL;
+
+		images->fit_hdr_os = map_sysmem(img_addr, 0);
+		images->fit_uname_os = fit_uname_kernel;
+		images->fit_uname_cfg = fit_uname_config;
+		images->fit_noffset_os = os_noffset;
+		break;
+#endif
+#ifdef CONFIG_ANDROID_BOOT_IMAGE
+	case IMAGE_FORMAT_ANDROID:
+		boot_img = buf;
+		vendor_boot_img = NULL;
+		if (IS_ENABLED(CONFIG_CMD_ABOOTIMG)) {
+			boot_img = map_sysmem(get_abootimg_addr(), 0);
+			vendor_boot_img = map_sysmem(get_avendor_bootimg_addr(), 0);
+		}
+		printf("## Booting Android Image at 0x%08lx ...\n", img_addr);
+		if (android_image_get_kernel(boot_img, vendor_boot_img, images->verify,
+					     os_data, os_len))
+			return NULL;
+		if (IS_ENABLED(CONFIG_CMD_ABOOTIMG)) {
+			unmap_sysmem(vendor_boot_img);
+			unmap_sysmem(boot_img);
+		}
+		break;
+#endif
+	default:
+		printf("Wrong Image Format for %s command\n", cmdtp->name);
+		bootstage_error(BOOTSTAGE_ID_FIT_KERNEL_INFO);
+		return NULL;
+	}
+
+	debug("   kernel data at 0x%08lx, len = 0x%08lx (%ld)\n",
+	      *os_data, *os_len, *os_len);
+
+	return buf;
 }
 
 #ifdef CONFIG_LMB
@@ -942,193 +1125,6 @@ int bootm_boot_start(ulong addr, const char *cmdline)
 	return ret;
 }
 
-#if CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)
-/**
- * image_get_kernel - verify legacy format kernel image
- * @img_addr: in RAM address of the legacy format image to be verified
- * @verify: data CRC verification flag
- *
- * image_get_kernel() verifies legacy image integrity and returns pointer to
- * legacy image header if image verification was completed successfully.
- *
- * returns:
- *     pointer to a legacy image header if valid image was found
- *     otherwise return NULL
- */
-static struct legacy_img_hdr *image_get_kernel(ulong img_addr, int verify)
-{
-	struct legacy_img_hdr *hdr = (struct legacy_img_hdr *)img_addr;
-
-	if (!image_check_magic(hdr)) {
-		puts("Bad Magic Number\n");
-		bootstage_error(BOOTSTAGE_ID_CHECK_MAGIC);
-		return NULL;
-	}
-	bootstage_mark(BOOTSTAGE_ID_CHECK_HEADER);
-
-	if (!image_check_hcrc(hdr)) {
-		puts("Bad Header Checksum\n");
-		bootstage_error(BOOTSTAGE_ID_CHECK_HEADER);
-		return NULL;
-	}
-
-	bootstage_mark(BOOTSTAGE_ID_CHECK_CHECKSUM);
-	image_print_contents(hdr);
-
-	if (verify) {
-		puts("   Verifying Checksum ... ");
-		if (!image_check_dcrc(hdr)) {
-			printf("Bad Data CRC\n");
-			bootstage_error(BOOTSTAGE_ID_CHECK_CHECKSUM);
-			return NULL;
-		}
-		puts("OK\n");
-	}
-	bootstage_mark(BOOTSTAGE_ID_CHECK_ARCH);
-
-	if (!image_check_target_arch(hdr)) {
-		printf("Unsupported Architecture 0x%x\n", image_get_arch(hdr));
-		bootstage_error(BOOTSTAGE_ID_CHECK_ARCH);
-		return NULL;
-	}
-	return hdr;
-}
-#endif
-
-/**
- * boot_get_kernel - find kernel image
- * @os_data: pointer to a ulong variable, will hold os data start address
- * @os_len: pointer to a ulong variable, will hold os data length
- *
- * boot_get_kernel() tries to find a kernel image, verifies its integrity
- * and locates kernel data.
- *
- * returns:
- *     pointer to image header if valid image was found, plus kernel start
- *     address and length, otherwise NULL
- */
-static const void *boot_get_kernel(struct cmd_tbl *cmdtp, int flag, int argc,
-				   char *const argv[], struct bootm_headers *images,
-				   ulong *os_data, ulong *os_len)
-{
-#if CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)
-	struct legacy_img_hdr	*hdr;
-#endif
-	ulong		img_addr;
-	const void *buf;
-	const char	*fit_uname_config = NULL;
-	const char	*fit_uname_kernel = NULL;
-#if CONFIG_IS_ENABLED(FIT)
-	int		os_noffset;
-#endif
-
-#ifdef CONFIG_ANDROID_BOOT_IMAGE
-	const void *boot_img;
-	const void *vendor_boot_img;
-#endif
-	img_addr = genimg_get_kernel_addr_fit(argc < 1 ? NULL : argv[0],
-					      &fit_uname_config,
-					      &fit_uname_kernel);
-
-	if (IS_ENABLED(CONFIG_CMD_BOOTM_PRE_LOAD))
-		img_addr += image_load_offset;
-
-	bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);
-
-	/* check image type, for FIT images get FIT kernel node */
-	*os_data = *os_len = 0;
-	buf = map_sysmem(img_addr, 0);
-	switch (genimg_get_format(buf)) {
-#if CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)
-	case IMAGE_FORMAT_LEGACY:
-		printf("## Booting kernel from Legacy Image at %08lx ...\n",
-		       img_addr);
-		hdr = image_get_kernel(img_addr, images->verify);
-		if (!hdr)
-			return NULL;
-		bootstage_mark(BOOTSTAGE_ID_CHECK_IMAGETYPE);
-
-		/* get os_data and os_len */
-		switch (image_get_type(hdr)) {
-		case IH_TYPE_KERNEL:
-		case IH_TYPE_KERNEL_NOLOAD:
-			*os_data = image_get_data(hdr);
-			*os_len = image_get_data_size(hdr);
-			break;
-		case IH_TYPE_MULTI:
-			image_multi_getimg(hdr, 0, os_data, os_len);
-			break;
-		case IH_TYPE_STANDALONE:
-			*os_data = image_get_data(hdr);
-			*os_len = image_get_data_size(hdr);
-			break;
-		default:
-			printf("Wrong Image Type for %s command\n",
-			       cmdtp->name);
-			bootstage_error(BOOTSTAGE_ID_CHECK_IMAGETYPE);
-			return NULL;
-		}
-
-		/*
-		 * copy image header to allow for image overwrites during
-		 * kernel decompression.
-		 */
-		memmove(&images->legacy_hdr_os_copy, hdr,
-			sizeof(struct legacy_img_hdr));
-
-		/* save pointer to image header */
-		images->legacy_hdr_os = hdr;
-
-		images->legacy_hdr_valid = 1;
-		bootstage_mark(BOOTSTAGE_ID_DECOMP_IMAGE);
-		break;
-#endif
-#if CONFIG_IS_ENABLED(FIT)
-	case IMAGE_FORMAT_FIT:
-		os_noffset = fit_image_load(images, img_addr,
-				&fit_uname_kernel, &fit_uname_config,
-				IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
-				BOOTSTAGE_ID_FIT_KERNEL_START,
-				FIT_LOAD_IGNORED, os_data, os_len);
-		if (os_noffset < 0)
-			return NULL;
-
-		images->fit_hdr_os = map_sysmem(img_addr, 0);
-		images->fit_uname_os = fit_uname_kernel;
-		images->fit_uname_cfg = fit_uname_config;
-		images->fit_noffset_os = os_noffset;
-		break;
-#endif
-#ifdef CONFIG_ANDROID_BOOT_IMAGE
-	case IMAGE_FORMAT_ANDROID:
-		boot_img = buf;
-		vendor_boot_img = NULL;
-		if (IS_ENABLED(CONFIG_CMD_ABOOTIMG)) {
-			boot_img = map_sysmem(get_abootimg_addr(), 0);
-			vendor_boot_img = map_sysmem(get_avendor_bootimg_addr(), 0);
-		}
-		printf("## Booting Android Image at 0x%08lx ...\n", img_addr);
-		if (android_image_get_kernel(boot_img, vendor_boot_img, images->verify,
-					     os_data, os_len))
-			return NULL;
-		if (IS_ENABLED(CONFIG_CMD_ABOOTIMG)) {
-			unmap_sysmem(vendor_boot_img);
-			unmap_sysmem(boot_img);
-		}
-		break;
-#endif
-	default:
-		printf("Wrong Image Format for %s command\n", cmdtp->name);
-		bootstage_error(BOOTSTAGE_ID_FIT_KERNEL_INFO);
-		return NULL;
-	}
-
-	debug("   kernel data at 0x%08lx, len = 0x%08lx (%ld)\n",
-	      *os_data, *os_len, *os_len);
-
-	return buf;
-}
-
 /**
  * switch_to_non_secure_mode() - switch to non-secure mode
  *
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 09/29] image: Tidy up genimg_get_kernel_addr_fit()

[Simon Glass]

This function does not modify its first argument, so mark it const. Also
move the comments to the header file and expand them to provide more
useful information.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c       |  3 +--
 boot/image-board.c | 17 +----------------
 include/image.h    | 29 ++++++++++++++++++++++++++---
 3 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 598f880d86dd..e323c8b758e9 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -122,8 +122,7 @@ static const void *boot_get_kernel(struct cmd_tbl *cmdtp, int flag, int argc,
 #endif
 	ulong		img_addr;
 	const void *buf;
-	const char	*fit_uname_config = NULL;
-	const char	*fit_uname_kernel = NULL;
+	const char *fit_uname_config = NULL, *fit_uname_kernel = NULL;
 #if CONFIG_IS_ENABLED(FIT)
 	int		os_noffset;
 #endif
diff --git a/boot/image-board.c b/boot/image-board.c
index d500da1b4b91..062c76badecc 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -198,22 +198,7 @@ void memmove_wd(void *to, void *from, size_t len, ulong chunksz)
 	}
 }
 
-/**
- * genimg_get_kernel_addr_fit - get the real kernel address and return 2
- *                              FIT strings
- * @img_addr: a string might contain real image address
- * @fit_uname_config: double pointer to a char, will hold pointer to a
- *                    configuration unit name
- * @fit_uname_kernel: double pointer to a char, will hold pointer to a subimage
- *                    name
- *
- * genimg_get_kernel_addr_fit get the real kernel start address from a string
- * which is normally the first argv of bootm/bootz
- *
- * returns:
- *     kernel start address
- */
-ulong genimg_get_kernel_addr_fit(char * const img_addr,
+ulong genimg_get_kernel_addr_fit(const char *const img_addr,
 				 const char **fit_uname_config,
 				 const char **fit_uname_kernel)
 {
diff --git a/include/image.h b/include/image.h
index 2e3cf839ee36..798c5f9c16e4 100644
--- a/include/image.h
+++ b/include/image.h
@@ -612,9 +612,32 @@ int boot_get_setup(struct bootm_headers *images, uint8_t arch, ulong *setup_star
 #define IMAGE_FORMAT_FIT	0x02	/* new, libfdt based format */
 #define IMAGE_FORMAT_ANDROID	0x03	/* Android boot image */
 
-ulong genimg_get_kernel_addr_fit(char * const img_addr,
-			         const char **fit_uname_config,
-			         const char **fit_uname_kernel);
+/**
+ * genimg_get_kernel_addr_fit() - Parse FIT specifier
+ *
+ * Get the real kernel start address from a string which is normally the first
+ * argv of bootm/bootz
+ *
+ * These cases are dealt with, based on the value of @img_addr:
+ *    NULL: Returns image_load_addr, does not set last two args
+ *    "<addr>": Returns address
+ *
+ * For FIT:
+ *    "[<addr>]#<conf>": Returns address (or image_load_addr),
+ *	sets fit_uname_config to config name
+ *    "[<addr>]:<subimage>": Returns address (or image_load_addr) and sets
+ *	fit_uname_kernel to the subimage name
+ *
+ * @img_addr: a string might contain real image address (or NULL)
+ * @fit_uname_config: Returns configuration unit name
+ * @fit_uname_kernel: Returns subimage name
+ *
+ * Returns: kernel start address
+ */
+ulong genimg_get_kernel_addr_fit(const char *const img_addr,
+				 const char **fit_uname_config,
+				 const char **fit_uname_kernel);
+
 ulong genimg_get_kernel_addr(char * const img_addr);
 int genimg_get_format(const void *img_addr);
 int genimg_has_config(struct bootm_headers *images);
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 10/29] bootm: Reduce arguments to boot_get_kernel()

[Simon Glass]

This function only uses one argument and just needs to know the name of
the command which called it. Adjust the function to use only what it
needs. This will make it easier to call from a non-command context.

Tidy up the function comment while we are here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index e323c8b758e9..2b986ca71b92 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -102,9 +102,14 @@ static struct legacy_img_hdr *image_get_kernel(ulong img_addr, int verify)
 #endif
 
 /**
- * boot_get_kernel - find kernel image
+ * boot_get_kernel() - find kernel image
+ *
+ * @cmd_name: Name of the command calling this function, e.g. "bootm"
+ * @addr_fit: first argument to bootm: address, fit configuration, etc.
  * @os_data: pointer to a ulong variable, will hold os data start address
  * @os_len: pointer to a ulong variable, will hold os data length
+ *     address and length, otherwise NULL
+ *     pointer to image header if valid image was found, plus kernel start
  *
  * boot_get_kernel() tries to find a kernel image, verifies its integrity
  * and locates kernel data.
@@ -113,8 +118,8 @@ static struct legacy_img_hdr *image_get_kernel(ulong img_addr, int verify)
  *     pointer to image header if valid image was found, plus kernel start
  *     address and length, otherwise NULL
  */
-static const void *boot_get_kernel(struct cmd_tbl *cmdtp, int flag, int argc,
-				   char *const argv[], struct bootm_headers *images,
+static const void *boot_get_kernel(const char *cmd_name, const char *addr_fit,
+				   struct bootm_headers *images,
 				   ulong *os_data, ulong *os_len)
 {
 #if CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)
@@ -131,8 +136,7 @@ static const void *boot_get_kernel(struct cmd_tbl *cmdtp, int flag, int argc,
 	const void *boot_img;
 	const void *vendor_boot_img;
 #endif
-	img_addr = genimg_get_kernel_addr_fit(argc < 1 ? NULL : argv[0],
-					      &fit_uname_config,
+	img_addr = genimg_get_kernel_addr_fit(addr_fit, &fit_uname_config,
 					      &fit_uname_kernel);
 
 	if (IS_ENABLED(CONFIG_CMD_BOOTM_PRE_LOAD))
@@ -169,7 +173,7 @@ static const void *boot_get_kernel(struct cmd_tbl *cmdtp, int flag, int argc,
 			break;
 		default:
 			printf("Wrong Image Type for %s command\n",
-			       cmdtp->name);
+			       cmd_name);
 			bootstage_error(BOOTSTAGE_ID_CHECK_IMAGETYPE);
 			return NULL;
 		}
@@ -223,7 +227,7 @@ static const void *boot_get_kernel(struct cmd_tbl *cmdtp, int flag, int argc,
 		break;
 #endif
 	default:
-		printf("Wrong Image Format for %s command\n", cmdtp->name);
+		printf("Wrong Image Format for %s command\n", cmd_name);
 		bootstage_error(BOOTSTAGE_ID_FIT_KERNEL_INFO);
 		return NULL;
 	}
@@ -312,8 +316,8 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc,
 	int ret;
 
 	/* get kernel image header, start address and length */
-	os_hdr = boot_get_kernel(cmdtp, flag, argc, argv,
-			&images, &images.os.image_start, &images.os.image_len);
+	os_hdr = boot_get_kernel("bootm", argv[0], &images,
+				 &images.os.image_start, &images.os.image_len);
 	if (images.os.image_len == 0) {
 		puts("ERROR: can't get kernel image!\n");
 		return 1;
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 11/29] image: Document error codes from fit_image_load()

[Simon Glass]

Put a list of these in the function documentation so it is easier to
decode what went wrong.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/image.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/image.h b/include/image.h
index 798c5f9c16e4..d37e44721672 100644
--- a/include/image.h
+++ b/include/image.h
@@ -728,7 +728,13 @@ int boot_get_fdt_fit(struct bootm_headers *images, ulong addr,
  * @param load_op	Decribes what to do with the load address
  * @param datap		Returns address of loaded image
  * @param lenp		Returns length of loaded image
- * Return: node offset of image, or -ve error code on error
+ * Return: node offset of image, or -ve error code on error:
+ *   -ENOEXEC - unsupported architecture
+ *   -ENOENT - could not find image / subimage
+ *   -EACCES - hash, signature or decryptions failure
+ *   -EBADF - invalid OS or image type, or cannot get image load-address
+ *   -EXDEV - memory overwritten / overlap
+ *   -NOEXEC - image decompression error, or invalid FDT
  */
 int fit_image_load(struct bootm_headers *images, ulong addr,
 		   const char **fit_unamep, const char **fit_uname_configp,
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 12/29] bootm: Adjust boot_get_kernel() to return an error

[Simon Glass]

This function obtains lots of error codes and then throws them away.
Update it to return the error, moving the image pointer to an
argument.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 2b986ca71b92..57eaa1ff10df 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -110,6 +110,7 @@ static struct legacy_img_hdr *image_get_kernel(ulong img_addr, int verify)
  * @os_len: pointer to a ulong variable, will hold os data length
  *     address and length, otherwise NULL
  *     pointer to image header if valid image was found, plus kernel start
+ * @kernp: image header if valid image was found, otherwise NULL
  *
  * boot_get_kernel() tries to find a kernel image, verifies its integrity
  * and locates kernel data.
@@ -118,9 +119,9 @@ static struct legacy_img_hdr *image_get_kernel(ulong img_addr, int verify)
  *     pointer to image header if valid image was found, plus kernel start
  *     address and length, otherwise NULL
  */
-static const void *boot_get_kernel(const char *cmd_name, const char *addr_fit,
-				   struct bootm_headers *images,
-				   ulong *os_data, ulong *os_len)
+static int boot_get_kernel(const char *cmd_name, const char *addr_fit,
+			   struct bootm_headers *images, ulong *os_data,
+			   ulong *os_len, const void **kernp)
 {
 #if CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)
 	struct legacy_img_hdr	*hdr;
@@ -154,7 +155,7 @@ static const void *boot_get_kernel(const char *cmd_name, const char *addr_fit,
 		       img_addr);
 		hdr = image_get_kernel(img_addr, images->verify);
 		if (!hdr)
-			return NULL;
+			return -EINVAL;
 		bootstage_mark(BOOTSTAGE_ID_CHECK_IMAGETYPE);
 
 		/* get os_data and os_len */
@@ -175,7 +176,7 @@ static const void *boot_get_kernel(const char *cmd_name, const char *addr_fit,
 			printf("Wrong Image Type for %s command\n",
 			       cmd_name);
 			bootstage_error(BOOTSTAGE_ID_CHECK_IMAGETYPE);
-			return NULL;
+			return -EPROTOTYPE;
 		}
 
 		/*
@@ -200,7 +201,7 @@ static const void *boot_get_kernel(const char *cmd_name, const char *addr_fit,
 				BOOTSTAGE_ID_FIT_KERNEL_START,
 				FIT_LOAD_IGNORED, os_data, os_len);
 		if (os_noffset < 0)
-			return NULL;
+			return -ENOENT;
 
 		images->fit_hdr_os = map_sysmem(img_addr, 0);
 		images->fit_uname_os = fit_uname_kernel;
@@ -209,7 +210,9 @@ static const void *boot_get_kernel(const char *cmd_name, const char *addr_fit,
 		break;
 #endif
 #ifdef CONFIG_ANDROID_BOOT_IMAGE
-	case IMAGE_FORMAT_ANDROID:
+	case IMAGE_FORMAT_ANDROID: {
+		int ret;
+
 		boot_img = buf;
 		vendor_boot_img = NULL;
 		if (IS_ENABLED(CONFIG_CMD_ABOOTIMG)) {
@@ -217,25 +220,28 @@ static const void *boot_get_kernel(const char *cmd_name, const char *addr_fit,
 			vendor_boot_img = map_sysmem(get_avendor_bootimg_addr(), 0);
 		}
 		printf("## Booting Android Image at 0x%08lx ...\n", img_addr);
-		if (android_image_get_kernel(boot_img, vendor_boot_img, images->verify,
-					     os_data, os_len))
-			return NULL;
+		ret = android_image_get_kernel(boot_img, vendor_boot_img,
+					       images->verify, os_data, os_len);
+		if (ret)
+			return ret;
 		if (IS_ENABLED(CONFIG_CMD_ABOOTIMG)) {
 			unmap_sysmem(vendor_boot_img);
 			unmap_sysmem(boot_img);
 		}
 		break;
+	}
 #endif
 	default:
 		printf("Wrong Image Format for %s command\n", cmd_name);
 		bootstage_error(BOOTSTAGE_ID_FIT_KERNEL_INFO);
-		return NULL;
+		return -EBADF;
 	}
 
 	debug("   kernel data at 0x%08lx, len = 0x%08lx (%ld)\n",
 	      *os_data, *os_len, *os_len);
+	*kernp = buf;
 
-	return buf;
+	return 0;
 }
 
 #ifdef CONFIG_LMB
@@ -316,8 +322,8 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc,
 	int ret;
 
 	/* get kernel image header, start address and length */
-	os_hdr = boot_get_kernel("bootm", argv[0], &images,
-				 &images.os.image_start, &images.os.image_len);
+	ret = boot_get_kernel("bootm", argv[0], &images, &images.os.image_start,
+			      &images.os.image_len, &os_hdr);
 	if (images.os.image_len == 0) {
 		puts("ERROR: can't get kernel image!\n");
 		return 1;
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 13/29] bootm: Use the error return from boot_get_kernel()

[Simon Glass]

Rather than looking for a zero-sized image, use the error code returned
to determine if things are OK.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 57eaa1ff10df..5e3b5e940734 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -324,8 +324,8 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc,
 	/* get kernel image header, start address and length */
 	ret = boot_get_kernel("bootm", argv[0], &images, &images.os.image_start,
 			      &images.os.image_len, &os_hdr);
-	if (images.os.image_len == 0) {
-		puts("ERROR: can't get kernel image!\n");
+	if (ret) {
+		printf("ERROR %dE: can't get kernel image!\n", ret);
 		return 1;
 	}
 
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 14/29] bootstage: Drop BOOTSTAGE_ID_FIT_KERNEL_INFO

[Simon Glass]

This is a misnomer since we don't necessarily know that the image is a
FIT. Use the existing BOOTSTAGE_ID_CHECK_IMAGETYPE instead.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c        | 2 +-
 include/bootstage.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 5e3b5e940734..a64c253069f9 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -233,7 +233,7 @@ static int boot_get_kernel(const char *cmd_name, const char *addr_fit,
 #endif
 	default:
 		printf("Wrong Image Format for %s command\n", cmd_name);
-		bootstage_error(BOOTSTAGE_ID_FIT_KERNEL_INFO);
+		bootstage_error(BOOTSTAGE_ID_CHECK_IMAGETYPE);
 		return -EBADF;
 	}
 
diff --git a/include/bootstage.h b/include/bootstage.h
index affb0e5c6a6a..59a76d0f0c40 100644
--- a/include/bootstage.h
+++ b/include/bootstage.h
@@ -147,7 +147,6 @@ enum bootstage_id {
 
 	BOOTSTAGE_ID_FIT_CONFIG = 110,
 	BOOTSTAGE_ID_FIT_TYPE,
-	BOOTSTAGE_ID_FIT_KERNEL_INFO,
 
 	BOOTSTAGE_ID_FIT_COMPRESSION,
 	BOOTSTAGE_ID_FIT_OS,
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 15/29] bootm: Move error printing out of boot_get_kernel()

[Simon Glass]

The same error message is printed in two places. Move it out to the
caller so we can avoid passing in the command name. Leave the bootstage
handling where it is.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index a64c253069f9..ec43d4e7e8ba 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -104,7 +104,6 @@ static struct legacy_img_hdr *image_get_kernel(ulong img_addr, int verify)
 /**
  * boot_get_kernel() - find kernel image
  *
- * @cmd_name: Name of the command calling this function, e.g. "bootm"
  * @addr_fit: first argument to bootm: address, fit configuration, etc.
  * @os_data: pointer to a ulong variable, will hold os data start address
  * @os_len: pointer to a ulong variable, will hold os data length
@@ -115,13 +114,11 @@ static struct legacy_img_hdr *image_get_kernel(ulong img_addr, int verify)
  * boot_get_kernel() tries to find a kernel image, verifies its integrity
  * and locates kernel data.
  *
- * returns:
- *     pointer to image header if valid image was found, plus kernel start
- *     address and length, otherwise NULL
+ * Return: 0 on success, -ve on error. -EPROTOTYPE means that the image is in
+ * a wrong or unsupported format
  */
-static int boot_get_kernel(const char *cmd_name, const char *addr_fit,
-			   struct bootm_headers *images, ulong *os_data,
-			   ulong *os_len, const void **kernp)
+static int boot_get_kernel(const char *addr_fit, struct bootm_headers *images,
+			   ulong *os_data, ulong *os_len, const void **kernp)
 {
 #if CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)
 	struct legacy_img_hdr	*hdr;
@@ -173,8 +170,6 @@ static int boot_get_kernel(const char *cmd_name, const char *addr_fit,
 			*os_len = image_get_data_size(hdr);
 			break;
 		default:
-			printf("Wrong Image Type for %s command\n",
-			       cmd_name);
 			bootstage_error(BOOTSTAGE_ID_CHECK_IMAGETYPE);
 			return -EPROTOTYPE;
 		}
@@ -232,9 +227,8 @@ static int boot_get_kernel(const char *cmd_name, const char *addr_fit,
 	}
 #endif
 	default:
-		printf("Wrong Image Format for %s command\n", cmd_name);
 		bootstage_error(BOOTSTAGE_ID_CHECK_IMAGETYPE);
-		return -EBADF;
+		return -EPROTOTYPE;
 	}
 
 	debug("   kernel data at 0x%08lx, len = 0x%08lx (%ld)\n",
@@ -322,9 +316,12 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc,
 	int ret;
 
 	/* get kernel image header, start address and length */
-	ret = boot_get_kernel("bootm", argv[0], &images, &images.os.image_start,
+	ret = boot_get_kernel(argv[0], &images, &images.os.image_start,
 			      &images.os.image_len, &os_hdr);
 	if (ret) {
+		if (ret == -EPROTOTYPE)
+			printf("Wrong Image Type for bootm command\n");
+
 		printf("ERROR %dE: can't get kernel image!\n", ret);
 		return 1;
 	}
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 16/29] bootm: Reduce arguments to boot_find_os()

[Simon Glass]

This function only uses one argument so pass it in directly.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index ec43d4e7e8ba..2358d68c2861 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -304,8 +304,13 @@ static int bootm_pre_load(const char *addr_str)
 	return ret;
 }
 
-static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc,
-			 char *const argv[])
+/**
+ * bootm_find_os(): Find the OS to boot
+ *
+ * @addr_fit: Address and/or FIT specifier (first arg of bootm command)
+ * Return: 0 on success, -ve on error
+ */
+static int bootm_find_os(const char *addr_fit)
 {
 	const void *os_hdr;
 #ifdef CONFIG_ANDROID_BOOT_IMAGE
@@ -316,7 +321,7 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc,
 	int ret;
 
 	/* get kernel image header, start address and length */
-	ret = boot_get_kernel(argv[0], &images, &images.os.image_start,
+	ret = boot_get_kernel(addr_fit, &images, &images.os.image_start,
 			      &images.os.image_len, &os_hdr);
 	if (ret) {
 		if (ret == -EPROTOTYPE)
@@ -986,7 +991,7 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc,
 		ret = bootm_pre_load(argv[0]);
 
 	if (!ret && (states & BOOTM_STATE_FINDOS))
-		ret = bootm_find_os(cmdtp, flag, argc, argv);
+		ret = bootm_find_os(argv[0]);
 
 	if (!ret && (states & BOOTM_STATE_FINDOTHER))
 		ret = bootm_find_other(cmdtp, flag, argc, argv);
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 17/29] bootm: Reduce arguments to boot_get_ramdisk()

[Simon Glass]

This function normally only uses one argument so pass it in directly.
Move comments to the header file so could one day include these
functions in API docs. Fix up the u8 argument while here, since it
avoids the compiler having to mask the value on some machines.

The Android case here is bit strange, since it can use argv[0], so deal
with that in the caller.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c       | 15 ++++++++++++++-
 boot/image-board.c | 39 ++-------------------------------------
 include/image.h    | 27 +++++++++++++++++++++++++--
 3 files changed, 41 insertions(+), 40 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 2358d68c2861..5782a1e2a57b 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -488,10 +488,23 @@ static int bootm_find_os(const char *addr_fit)
 int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
 		      ulong size)
 {
+	const char *select = NULL;
 	int ret;
 
+	if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE)) {
+		char *buf;
+
+		/* Look for an Android boot image */
+		buf = map_sysmem(images.os.start, 0);
+		if (buf && genimg_get_format(buf) == IMAGE_FORMAT_ANDROID)
+			select = argc ? argv[0] : env_get("loadaddr");
+	}
+
+	if (argc >= 2)
+		select = argv[1];
+
 	/* find ramdisk */
-	ret = boot_get_ramdisk(argc, argv, &images, IH_INITRD_ARCH,
+	ret = boot_get_ramdisk(select, &images, IH_INITRD_ARCH,
 			       &images.rd_start, &images.rd_end);
 	if (ret) {
 		puts("Ramdisk image is corrupt or invalid\n");
diff --git a/boot/image-board.c b/boot/image-board.c
index 062c76badecc..60e514fc150e 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -456,49 +456,14 @@ static int select_ramdisk(struct bootm_headers *images, const char *select, u8 a
 	return 0;
 }
 
-/**
- * boot_get_ramdisk - main ramdisk handling routine
- * @argc: command argument count
- * @argv: command argument list
- * @images: pointer to the bootm images structure
- * @arch: expected ramdisk architecture
- * @rd_start: pointer to a ulong variable, will hold ramdisk start address
- * @rd_end: pointer to a ulong variable, will hold ramdisk end
- *
- * boot_get_ramdisk() is responsible for finding a valid ramdisk image.
- * Currently supported are the following ramdisk sources:
- *      - multicomponent kernel/ramdisk image,
- *      - commandline provided address of decicated ramdisk image.
- *
- * returns:
- *     0, if ramdisk image was found and valid, or skiped
- *     rd_start and rd_end are set to ramdisk start/end addresses if
- *     ramdisk image is found and valid
- *
- *     1, if ramdisk image is found but corrupted, or invalid
- *     rd_start and rd_end are set to 0 if no ramdisk exists
- */
-int boot_get_ramdisk(int argc, char *const argv[], struct bootm_headers *images,
-		     u8 arch, ulong *rd_start, ulong *rd_end)
+int boot_get_ramdisk(char const *select, struct bootm_headers *images,
+		     uint arch, ulong *rd_start, ulong *rd_end)
 {
 	ulong rd_data, rd_len;
-	const char *select = NULL;
 
 	*rd_start = 0;
 	*rd_end = 0;
 
-	if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE)) {
-		char *buf;
-
-		/* Look for an Android boot image */
-		buf = map_sysmem(images->os.start, 0);
-		if (buf && genimg_get_format(buf) == IMAGE_FORMAT_ANDROID)
-			select = (argc == 0) ? env_get("loadaddr") : argv[0];
-	}
-
-	if (argc >= 2)
-		select = argv[1];
-
 	/*
 	 * Look for a '-' which indicates to ignore the
 	 * ramdisk argument
diff --git a/include/image.h b/include/image.h
index d37e44721672..3e48ad5b303e 100644
--- a/include/image.h
+++ b/include/image.h
@@ -644,8 +644,31 @@ int genimg_has_config(struct bootm_headers *images);
 
 int boot_get_fpga(int argc, char *const argv[], struct bootm_headers *images,
 		  uint8_t arch, const ulong *ld_start, ulong * const ld_len);
-int boot_get_ramdisk(int argc, char *const argv[], struct bootm_headers *images,
-		     uint8_t arch, ulong *rd_start, ulong *rd_end);
+
+/**
+ * boot_get_ramdisk() - Locate the ramdisk
+ *
+ * @select: address or name of ramdisk to use, or NULL for default
+ * @images: pointer to the bootm images structure
+ * @arch: expected ramdisk architecture
+ * @rd_start: pointer to a ulong variable, will hold ramdisk start address
+ * @rd_end: pointer to a ulong variable, will hold ramdisk end
+ *
+ * boot_get_ramdisk() is responsible for finding a valid ramdisk image.
+ * Currently supported are the following ramdisk sources:
+ *      - multicomponent kernel/ramdisk image,
+ *      - commandline provided address of decicated ramdisk image.
+ *
+ * returns:
+ *     0, if ramdisk image was found and valid, or skiped
+ *     rd_start and rd_end are set to ramdisk start/end addresses if
+ *     ramdisk image is found and valid
+ *
+ *     1, if ramdisk image is found but corrupted, or invalid
+ *     rd_start and rd_end are set to 0 if no ramdisk exists
+ */
+int boot_get_ramdisk(char const *select, struct bootm_headers *images,
+		     uint arch, ulong *rd_start, ulong *rd_end);
 
 /**
  * boot_get_loadable - routine to load a list of binaries to memory
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 18/29] fdt: Allow use of fdt_support inside if() statements

[Simon Glass]

Most of the fdt_support.h header file is included only if OF_LIBFDT or
OF_CONTROL are enabled. This means that calling functions defined in
that file must happen inside an #ifdef

This is unnecessary, so reduce the condition to just !USE_HOSTCC

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/fdt_support.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/fdt_support.h b/include/fdt_support.h
index 2cd836689821..feda0d997409 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -7,8 +7,7 @@
 #ifndef __FDT_SUPPORT_H
 #define __FDT_SUPPORT_H
 
-#if (defined(CONFIG_OF_LIBFDT) || defined(CONFIG_OF_CONTROL)) && \
-	!defined(USE_HOSTCC)
+#if !defined(USE_HOSTCC)
 
 #include <asm/u-boot.h>
 #include <linux/libfdt.h>
@@ -418,7 +417,7 @@ int fdt_valid(struct fdt_header **blobp);
  */
 int fdt_get_cells_len(const void *blob, char *nr_cells_name);
 
-#endif /* ifdef CONFIG_OF_LIBFDT */
+#endif /* !USE_HOSTCC */
 
 #ifdef USE_HOSTCC
 int fdtdec_get_int(const void *blob, int node, const char *prop_name,
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 19/29] bootm: Drop #ifdef in bootm_find_images()

[Simon Glass]

The OF_LIBFDT #ifdef makes it harder to use a local variable for that
code block. Convert it to if() instead.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 5782a1e2a57b..0de49a9d1fa1 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -523,29 +523,29 @@ int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
 		return 1;
 	}
 
-#if CONFIG_IS_ENABLED(OF_LIBFDT)
-	/* find flattened device tree */
-	ret = boot_get_fdt(flag, argc, argv, IH_ARCH_DEFAULT, &images,
-			   &images.ft_addr, &images.ft_len);
-	if (ret) {
-		puts("Could not find a valid device tree\n");
-		return 1;
-	}
+	if (CONFIG_IS_ENABLED(OF_LIBFDT)) {
+		/* find flattened device tree */
+		ret = boot_get_fdt(flag, argc, argv, IH_ARCH_DEFAULT, &images,
+				   &images.ft_addr, &images.ft_len);
+		if (ret) {
+			puts("Could not find a valid device tree\n");
+			return 1;
+		}
 
-	/* check if FDT overlaps OS image */
-	if (images.ft_addr &&
-	    (((ulong)images.ft_addr >= start &&
-	      (ulong)images.ft_addr < start + size) ||
-	     ((ulong)images.ft_addr + images.ft_len >= start &&
-	      (ulong)images.ft_addr + images.ft_len < start + size))) {
-		printf("ERROR: FDT image overlaps OS image (OS=0x%lx..0x%lx)\n",
-		       start, start + size);
-		return 1;
-	}
+		/* check if FDT overlaps OS image */
+		if (images.ft_addr &&
+		    (((ulong)images.ft_addr >= start &&
+		      (ulong)images.ft_addr < start + size) ||
+		     ((ulong)images.ft_addr + images.ft_len >= start &&
+		      (ulong)images.ft_addr + images.ft_len < start + size))) {
+			printf("ERROR: FDT image overlaps OS image (OS=0x%lx..0x%lx)\n",
+			       start, start + size);
+			return 1;
+		}
 
-	if (IS_ENABLED(CONFIG_CMD_FDT))
-		set_working_fdt_addr(map_to_sysmem(images.ft_addr));
-#endif
+		if (IS_ENABLED(CONFIG_CMD_FDT))
+			set_working_fdt_addr(map_to_sysmem(images.ft_addr));
+	}
 
 #if CONFIG_IS_ENABLED(FIT)
 	if (IS_ENABLED(CONFIG_FPGA)) {
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 20/29] bootm: Pass image buffer to boot_get_fdt()

[Simon Glass]

Rather than having boot_get_fdt() calculate this, move the calculation
into the caller. This removes the access to argv[0] in this function,
so we can later refactor it to just accept argv[2] instead of the whole
argv[].

Move the function comment to the header file and fix the u8 argument,
while we are here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c     | 11 +++++++----
 boot/image-fdt.c | 32 +++-----------------------------
 include/image.h  | 31 ++++++++++++++++++++++++++++---
 3 files changed, 38 insertions(+), 36 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 0de49a9d1fa1..4959fcd91533 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -489,11 +489,11 @@ int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
 		      ulong size)
 {
 	const char *select = NULL;
+	ulong img_addr;
+	void *buf;
 	int ret;
 
 	if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE)) {
-		char *buf;
-
 		/* Look for an Android boot image */
 		buf = map_sysmem(images.os.start, 0);
 		if (buf && genimg_get_format(buf) == IMAGE_FORMAT_ANDROID)
@@ -524,9 +524,12 @@ int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
 	}
 
 	if (CONFIG_IS_ENABLED(OF_LIBFDT)) {
+		img_addr = argc ? hextoul(argv[0], NULL) : image_load_addr;
+		buf = map_sysmem(img_addr, 0);
+
 		/* find flattened device tree */
-		ret = boot_get_fdt(flag, argc, argv, IH_ARCH_DEFAULT, &images,
-				   &images.ft_addr, &images.ft_len);
+		ret = boot_get_fdt(buf, flag, argc, argv, IH_ARCH_DEFAULT,
+				   &images, &images.ft_addr, &images.ft_len);
 		if (ret) {
 			puts("Could not find a valid device tree\n");
 			return 1;
diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index f10200f64743..6ff2d52d6c70 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -447,43 +447,17 @@ static int select_fdt(struct bootm_headers *images, const char *select, u8 arch,
 	return 0;
 }
 
-/**
- * boot_get_fdt - main fdt handling routine
- * @argc: command argument count
- * @argv: command argument list
- * @arch: architecture (IH_ARCH_...)
- * @images: pointer to the bootm images structure
- * @of_flat_tree: pointer to a char* variable, will hold fdt start address
- * @of_size: pointer to a ulong variable, will hold fdt length
- *
- * boot_get_fdt() is responsible for finding a valid flat device tree image.
- * Currently supported are the following ramdisk sources:
- *      - multicomponent kernel/ramdisk image,
- *      - commandline provided address of decicated ramdisk image.
- *
- * returns:
- *     0, if fdt image was found and valid, or skipped
- *     of_flat_tree and of_size are set to fdt start address and length if
- *     fdt image is found and valid
- *
- *     1, if fdt image is found but corrupted
- *     of_flat_tree and of_size are set to 0 if no fdt exists
- */
-int boot_get_fdt(int flag, int argc, char *const argv[], uint8_t arch,
-		 struct bootm_headers *images, char **of_flat_tree, ulong *of_size)
+int boot_get_fdt(void *buf, int flag, int argc, char *const argv[], uint arch,
+		 struct bootm_headers *images, char **of_flat_tree,
+		 ulong *of_size)
 {
-	ulong		img_addr;
 	ulong		fdt_addr;
 	char		*fdt_blob = NULL;
-	void		*buf;
 	const char *select = NULL;
 
 	*of_flat_tree = NULL;
 	*of_size = 0;
 
-	img_addr = (argc == 0) ? image_load_addr : hextoul(argv[0], NULL);
-	buf = map_sysmem(img_addr, 0);
-
 	if (argc > 2)
 		select = argv[2];
 	if (select || genimg_has_config(images)) {
diff --git a/include/image.h b/include/image.h
index 3e48ad5b303e..a4555f25fb56 100644
--- a/include/image.h
+++ b/include/image.h
@@ -808,9 +808,34 @@ int image_locate_script(void *buf, int size, const char *fit_uname,
 int fit_get_node_from_config(struct bootm_headers *images,
 			     const char *prop_name, ulong addr);
 
-int boot_get_fdt(int flag, int argc, char *const argv[], uint8_t arch,
-		 struct bootm_headers *images,
-		 char **of_flat_tree, ulong *of_size);
+/**
+ * boot_get_fdt() - locate FDT devicetree to use for booting
+ *
+ * @buf: Pointer to image
+ * @argc: command argument count
+ * @argv: command argument list
+ * @arch: architecture (IH_ARCH_...)
+ * @images: pointer to the bootm images structure
+ * @of_flat_tree: pointer to a char* variable, will hold fdt start address
+ * @of_size: pointer to a ulong variable, will hold fdt length
+ *
+ * boot_get_fdt() is responsible for finding a valid flat device tree image.
+ * Currently supported are the following FDT sources:
+ *      - multicomponent kernel/ramdisk/FDT image,
+ *      - commandline provided address of decicated FDT image.
+ *
+ * Return:
+ *     0, if fdt image was found and valid, or skipped
+ *     of_flat_tree and of_size are set to fdt start address and length if
+ *     fdt image is found and valid
+ *
+ *     1, if fdt image is found but corrupted
+ *     of_flat_tree and of_size are set to 0 if no fdt exists
+ */
+int boot_get_fdt(void *buf, int flag, int argc, char *const argv[], uint arch,
+		 struct bootm_headers *images, char **of_flat_tree,
+		 ulong *of_size);
+
 void boot_fdt_add_mem_rsv_regions(struct lmb *lmb, void *fdt_blob);
 int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong *of_size);
 
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 21/29] bootm: Reduce arguments to boot_get_fdt()

[Simon Glass]

This function only uses one argument from bootm (argv[2]) so pass it in
directly.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c     | 5 +++--
 boot/image-fdt.c | 9 +++------
 include/image.h  | 5 ++---
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 4959fcd91533..4d732719ad84 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -528,8 +528,9 @@ int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
 		buf = map_sysmem(img_addr, 0);
 
 		/* find flattened device tree */
-		ret = boot_get_fdt(buf, flag, argc, argv, IH_ARCH_DEFAULT,
-				   &images, &images.ft_addr, &images.ft_len);
+		ret = boot_get_fdt(buf, argc > 2 ? argv[2] : NULL,
+				   IH_ARCH_DEFAULT, &images, &images.ft_addr,
+				   &images.ft_len);
 		if (ret) {
 			puts("Could not find a valid device tree\n");
 			return 1;
diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index 6ff2d52d6c70..08fca08b710a 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -447,19 +447,16 @@ static int select_fdt(struct bootm_headers *images, const char *select, u8 arch,
 	return 0;
 }
 
-int boot_get_fdt(void *buf, int flag, int argc, char *const argv[], uint arch,
+int boot_get_fdt(void *buf, const char *select, uint arch,
 		 struct bootm_headers *images, char **of_flat_tree,
 		 ulong *of_size)
 {
-	ulong		fdt_addr;
-	char		*fdt_blob = NULL;
-	const char *select = NULL;
+	char *fdt_blob = NULL;
+	ulong fdt_addr;
 
 	*of_flat_tree = NULL;
 	*of_size = 0;
 
-	if (argc > 2)
-		select = argv[2];
 	if (select || genimg_has_config(images)) {
 		int ret;
 
diff --git a/include/image.h b/include/image.h
index a4555f25fb56..04f35de9d082 100644
--- a/include/image.h
+++ b/include/image.h
@@ -812,8 +812,7 @@ int fit_get_node_from_config(struct bootm_headers *images,
  * boot_get_fdt() - locate FDT devicetree to use for booting
  *
  * @buf: Pointer to image
- * @argc: command argument count
- * @argv: command argument list
+ * @select: FDT to select (this is normally argv[2] of the bootm command)
  * @arch: architecture (IH_ARCH_...)
  * @images: pointer to the bootm images structure
  * @of_flat_tree: pointer to a char* variable, will hold fdt start address
@@ -832,7 +831,7 @@ int fit_get_node_from_config(struct bootm_headers *images,
  *     1, if fdt image is found but corrupted
  *     of_flat_tree and of_size are set to 0 if no fdt exists
  */
-int boot_get_fdt(void *buf, int flag, int argc, char *const argv[], uint arch,
+int boot_get_fdt(void *buf, const char *select, uint arch,
 		 struct bootm_headers *images, char **of_flat_tree,
 		 ulong *of_size);
 
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 22/29] bootm: Reduce arguments to boot_get_fpga()

[Simon Glass]

This function only uses two arguments. The 'arch' always has a constant
value, so drop it. This simplifies the function call.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c       | 3 +--
 boot/image-board.c | 5 ++---
 include/image.h    | 9 +++++++--
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 4d732719ad84..4bfa28f4377a 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -554,8 +554,7 @@ int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
 #if CONFIG_IS_ENABLED(FIT)
 	if (IS_ENABLED(CONFIG_FPGA)) {
 		/* find bitstreams */
-		ret = boot_get_fpga(argc, argv, &images, IH_ARCH_DEFAULT,
-				    NULL, NULL);
+		ret = boot_get_fpga(&images);
 		if (ret) {
 			printf("FPGA image is corrupted or invalid\n");
 			return 1;
diff --git a/boot/image-board.c b/boot/image-board.c
index 60e514fc150e..b926275ec917 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -616,8 +616,7 @@ int boot_get_setup(struct bootm_headers *images, u8 arch,
 	return boot_get_setup_fit(images, arch, setup_start, setup_len);
 }
 
-int boot_get_fpga(int argc, char *const argv[], struct bootm_headers *images,
-		  u8 arch, const ulong *ld_start, ulong * const ld_len)
+int boot_get_fpga(struct bootm_headers *images)
 {
 	ulong tmp_img_addr, img_data, img_len;
 	void *buf;
@@ -659,7 +658,7 @@ int boot_get_fpga(int argc, char *const argv[], struct bootm_headers *images,
 						tmp_img_addr,
 						(const char **)&uname,
 						&images->fit_uname_cfg,
-						arch,
+						IH_ARCH_DEFAULT,
 						IH_TYPE_FPGA,
 						BOOTSTAGE_ID_FPGA_INIT,
 						FIT_LOAD_OPTIONAL_NON_ZERO,
diff --git a/include/image.h b/include/image.h
index 04f35de9d082..63f2bd3b3713 100644
--- a/include/image.h
+++ b/include/image.h
@@ -642,8 +642,13 @@ ulong genimg_get_kernel_addr(char * const img_addr);
 int genimg_get_format(const void *img_addr);
 int genimg_has_config(struct bootm_headers *images);
 
-int boot_get_fpga(int argc, char *const argv[], struct bootm_headers *images,
-		  uint8_t arch, const ulong *ld_start, ulong * const ld_len);
+/**
+ * boot_get_fpga() - Locate the FPGA image
+ *
+ * @images: Information about images being loaded
+ * Return 0 if OK, non-zero on failure
+ */
+int boot_get_fpga(struct bootm_headers *images);
 
 /**
  * boot_get_ramdisk() - Locate the ramdisk
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 23/29] bootm: Reduce arguments to boot_get_loadables()

[Simon Glass]

This function only uses two arguments. The 'arch' always has a constant
value, so drop it. This simplifies the function call.

Tidy up the function comment while we are here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c       |  3 +--
 boot/image-board.c |  6 +++---
 include/image.h    | 24 +++++++++---------------
 3 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 4bfa28f4377a..c71368431c48 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -562,8 +562,7 @@ int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
 	}
 
 	/* find all of the loadables */
-	ret = boot_get_loadable(argc, argv, &images, IH_ARCH_DEFAULT,
-			       NULL, NULL);
+	ret = boot_get_loadable(&images);
 	if (ret) {
 		printf("Loadable(s) is corrupt or invalid\n");
 		return 1;
diff --git a/boot/image-board.c b/boot/image-board.c
index b926275ec917..bb0ca9d7f22f 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -718,8 +718,7 @@ static void fit_loadable_process(u8 img_type,
 			fit_loadable_handler->handler(img_data, img_len);
 }
 
-int boot_get_loadable(int argc, char *const argv[], struct bootm_headers *images,
-		      u8 arch, const ulong *ld_start, ulong * const ld_len)
+int boot_get_loadable(struct bootm_headers *images)
 {
 	/*
 	 * These variables are used to hold the current image location
@@ -765,7 +764,8 @@ int boot_get_loadable(int argc, char *const argv[], struct bootm_headers *images
 			fit_img_result = fit_image_load(images, tmp_img_addr,
 							&uname,
 							&images->fit_uname_cfg,
-							arch, IH_TYPE_LOADABLE,
+							IH_ARCH_DEFAULT,
+							IH_TYPE_LOADABLE,
 							BOOTSTAGE_ID_FIT_LOADABLE_START,
 							FIT_LOAD_OPTIONAL_NON_ZERO,
 							&img_data, &img_len);
diff --git a/include/image.h b/include/image.h
index 63f2bd3b3713..b89912a50f98 100644
--- a/include/image.h
+++ b/include/image.h
@@ -676,28 +676,22 @@ int boot_get_ramdisk(char const *select, struct bootm_headers *images,
 		     uint arch, ulong *rd_start, ulong *rd_end);
 
 /**
- * boot_get_loadable - routine to load a list of binaries to memory
- * @argc: Ignored Argument
- * @argv: Ignored Argument
+ * boot_get_loadable() - load a list of binaries to memory
+ *
  * @images: pointer to the bootm images structure
- * @arch: expected architecture for the image
- * @ld_start: Ignored Argument
- * @ld_len: Ignored Argument
  *
- * boot_get_loadable() will take the given FIT configuration, and look
- * for a field named "loadables".  Loadables, is a list of elements in
- * the FIT given as strings.  exe:
+ * Takes the given FIT configuration, then looks for a field named
+ * "loadables", a list of elements in the FIT given as strings, e.g.:
  *   loadables = "linux_kernel", "fdt-2";
- * this function will attempt to parse each string, and load the
- * corresponding element from the FIT into memory.  Once placed,
- * no aditional actions are taken.
  *
- * @return:
+ * Each string is parsed, loading the corresponding element from the FIT into
+ * memory.  Once placed, no additional actions are taken.
+ *
+ * Return:
  *     0, if only valid images or no images are found
  *     error code, if an error occurs during fit_image_load
  */
-int boot_get_loadable(int argc, char *const argv[], struct bootm_headers *images,
-		      uint8_t arch, const ulong *ld_start, ulong *const ld_len);
+int boot_get_loadable(struct bootm_headers *images);
 
 int boot_get_setup_fit(struct bootm_headers *images, uint8_t arch,
 		       ulong *setup_start, ulong *setup_len);
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 24/29] bootm: Simplify Android ramdisk addr in bootm_find_images()

[Simon Glass]

The Android mechanism uses the loadaddr envrionment-variable to get the
load address, if none is provided. This is equivalent to
image_load_addr so use that instead, converting it to a string as
needed.

This change will permit passing img_addr to this function, in a future
change.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index c71368431c48..41cb52fb72c7 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -489,15 +489,20 @@ int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
 		      ulong size)
 {
 	const char *select = NULL;
+	char addr_str[17];
 	ulong img_addr;
 	void *buf;
 	int ret;
 
+	img_addr = argc ? hextoul(argv[0], NULL) : image_load_addr;
+
 	if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE)) {
 		/* Look for an Android boot image */
 		buf = map_sysmem(images.os.start, 0);
-		if (buf && genimg_get_format(buf) == IMAGE_FORMAT_ANDROID)
-			select = argc ? argv[0] : env_get("loadaddr");
+		if (buf && genimg_get_format(buf) == IMAGE_FORMAT_ANDROID) {
+			strcpy(addr_str, simple_xtoa(img_addr));
+			select = addr_str;
+		}
 	}
 
 	if (argc >= 2)
@@ -524,7 +529,6 @@ int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
 	}
 
 	if (CONFIG_IS_ENABLED(OF_LIBFDT)) {
-		img_addr = argc ? hextoul(argv[0], NULL) : image_load_addr;
 		buf = map_sysmem(img_addr, 0);
 
 		/* find flattened device tree */
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 25/29] bootm: efi: Drop special call to bootm_find_other()

[Simon Glass]

The normal bootm flow calls bootm_find_other() can call the
BOOTM_STATE_FINDOTHER state as part of its processing. Fix the
condition there so that this hack can be removed.

-off-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c    | 10 +++++-----
 boot/bootm_os.c |  6 ------
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 41cb52fb72c7..342d6211a097 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -579,11 +579,11 @@ int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
 static int bootm_find_other(struct cmd_tbl *cmdtp, int flag, int argc,
 			    char *const argv[])
 {
-	if (((images.os.type == IH_TYPE_KERNEL) ||
-	     (images.os.type == IH_TYPE_KERNEL_NOLOAD) ||
-	     (images.os.type == IH_TYPE_MULTI)) &&
-	    (images.os.os == IH_OS_LINUX ||
-		 images.os.os == IH_OS_VXWORKS))
+	if ((images.os.type == IH_TYPE_KERNEL ||
+	     images.os.type == IH_TYPE_KERNEL_NOLOAD ||
+	     images.os.type == IH_TYPE_MULTI) &&
+	    (images.os.os == IH_OS_LINUX || images.os.os == IH_OS_VXWORKS ||
+	     images.os.os == IH_OS_EFI))
 		return bootm_find_images(flag, argc, argv, 0, 0);
 
 	return 0;
diff --git a/boot/bootm_os.c b/boot/bootm_os.c
index 9c035b5be886..9af084a9f823 100644
--- a/boot/bootm_os.c
+++ b/boot/bootm_os.c
@@ -493,18 +493,12 @@ static int do_bootm_tee(int flag, int argc, char *const argv[],
 static int do_bootm_efi(int flag, int argc, char *const argv[],
 			struct bootm_headers *images)
 {
-	int ret;
 	efi_status_t efi_ret;
 	void *image_buf;
 
 	if (flag != BOOTM_STATE_OS_GO)
 		return 0;
 
-	/* Locate FDT, if provided */
-	ret = bootm_find_images(flag, argc, argv, 0, 0);
-	if (ret)
-		return ret;
-
 	/* Initialize EFI drivers */
 	efi_ret = efi_init_obj_list();
 	if (efi_ret != EFI_SUCCESS) {
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 26/29] bootm: optee: Drop special call to bootm_find_other()

[Simon Glass]

The normal bootm flow calls bootm_find_other() can call the
BOOTM_STATE_FINDOTHER state as part of its processing. Fix the
condition there so that this hack can be removed.

Also drop the confusing check for the OS type, since do_bootm_tee() is
only called if the condition is met - see bootm_os_get_boot_func()

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c    |  2 +-
 boot/bootm_os.c | 10 ----------
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 342d6211a097..62b6ca957f5b 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -583,7 +583,7 @@ static int bootm_find_other(struct cmd_tbl *cmdtp, int flag, int argc,
 	     images.os.type == IH_TYPE_KERNEL_NOLOAD ||
 	     images.os.type == IH_TYPE_MULTI) &&
 	    (images.os.os == IH_OS_LINUX || images.os.os == IH_OS_VXWORKS ||
-	     images.os.os == IH_OS_EFI))
+	     images.os.os == IH_OS_EFI || images.os.os == IH_OS_TEE))
 		return bootm_find_images(flag, argc, argv, 0, 0);
 
 	return 0;
diff --git a/boot/bootm_os.c b/boot/bootm_os.c
index 9af084a9f823..265df3dda626 100644
--- a/boot/bootm_os.c
+++ b/boot/bootm_os.c
@@ -467,11 +467,6 @@ static int do_bootm_tee(int flag, int argc, char *const argv[],
 {
 	int ret;
 
-	/* Verify OS type */
-	if (images->os.os != IH_OS_TEE) {
-		return 1;
-	};
-
 	/* Validate OPTEE header */
 	ret = optee_verify_bootm_image(images->os.image_start,
 				       images->os.load,
@@ -479,11 +474,6 @@ static int do_bootm_tee(int flag, int argc, char *const argv[],
 	if (ret)
 		return ret;
 
-	/* Locate FDT etc */
-	ret = bootm_find_images(flag, argc, argv, 0, 0);
-	if (ret)
-		return ret;
-
 	/* From here we can run the regular linux boot path */
 	return do_bootm_linux(flag, argc, argv, images);
 }
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 27/29] bootm: Adjust the parameters of bootm_find_images()

[Simon Glass]

Rather than passing it all the command-line args, pass in the pieces
that it needs. These are the image address, the ramdisk address/name
and the FDT address/name.

Ultimately this will allow usage of this function without being called
from the command line.

Move the function comment to the header file and tidy it a little.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c    | 45 ++++++++++++++-------------------------------
 cmd/booti.c     |  4 +++-
 cmd/bootz.c     |  4 +++-
 include/bootm.h | 26 +++++++++++++++++++++++---
 4 files changed, 43 insertions(+), 36 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 62b6ca957f5b..54e4b48e907c 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -467,35 +467,14 @@ static int bootm_find_os(const char *addr_fit)
 	return 0;
 }
 
-/**
- * bootm_find_images - wrapper to find and locate various images
- * @flag: Ignored Argument
- * @argc: command argument count
- * @argv: command argument list
- * @start: OS image start address
- * @size: OS image size
- *
- * boot_find_images() will attempt to load an available ramdisk,
- * flattened device tree, as well as specifically marked
- * "loadable" images (loadables are FIT only)
- *
- * Note: bootm_find_images will skip an image if it is not found
- *
- * @return:
- *     0, if all existing images were loaded correctly
- *     1, if an image is found but corrupted, or invalid
- */
-int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
-		      ulong size)
+int bootm_find_images(ulong img_addr, const char *conf_ramdisk,
+		      const char *conf_fdt, ulong start, ulong size)
 {
-	const char *select = NULL;
+	const char *select = conf_ramdisk;
 	char addr_str[17];
-	ulong img_addr;
 	void *buf;
 	int ret;
 
-	img_addr = argc ? hextoul(argv[0], NULL) : image_load_addr;
-
 	if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE)) {
 		/* Look for an Android boot image */
 		buf = map_sysmem(images.os.start, 0);
@@ -505,8 +484,8 @@ int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
 		}
 	}
 
-	if (argc >= 2)
-		select = argv[1];
+	if (conf_ramdisk)
+		select = conf_ramdisk;
 
 	/* find ramdisk */
 	ret = boot_get_ramdisk(select, &images, IH_INITRD_ARCH,
@@ -532,9 +511,8 @@ int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
 		buf = map_sysmem(img_addr, 0);
 
 		/* find flattened device tree */
-		ret = boot_get_fdt(buf, argc > 2 ? argv[2] : NULL,
-				   IH_ARCH_DEFAULT, &images, &images.ft_addr,
-				   &images.ft_len);
+		ret = boot_get_fdt(buf, conf_fdt, IH_ARCH_DEFAULT, &images,
+				   &images.ft_addr, &images.ft_len);
 		if (ret) {
 			puts("Could not find a valid device tree\n");
 			return 1;
@@ -583,8 +561,13 @@ static int bootm_find_other(struct cmd_tbl *cmdtp, int flag, int argc,
 	     images.os.type == IH_TYPE_KERNEL_NOLOAD ||
 	     images.os.type == IH_TYPE_MULTI) &&
 	    (images.os.os == IH_OS_LINUX || images.os.os == IH_OS_VXWORKS ||
-	     images.os.os == IH_OS_EFI || images.os.os == IH_OS_TEE))
-		return bootm_find_images(flag, argc, argv, 0, 0);
+	     images.os.os == IH_OS_EFI || images.os.os == IH_OS_TEE)) {
+		ulong img_addr;
+
+		img_addr = argc ? hextoul(argv[0], NULL) : image_load_addr;
+		return bootm_find_images(img_addr, argc > 1 ? argv[1] : NULL,
+					 argc > 2 ? argv[2] : NULL, 0, 0);
+	}
 
 	return 0;
 }
diff --git a/cmd/booti.c b/cmd/booti.c
index a6c7db272c57..dc73c83f0db0 100644
--- a/cmd/booti.c
+++ b/cmd/booti.c
@@ -95,7 +95,9 @@ static int booti_start(struct cmd_tbl *cmdtp, int flag, int argc,
 	 * Handle the BOOTM_STATE_FINDOTHER state ourselves as we do not
 	 * have a header that provide this informaiton.
 	 */
-	if (bootm_find_images(flag, argc, argv, relocated_addr, image_size))
+	if (bootm_find_images(image_load_addr, argc > 1 ? argv[1] : NULL,
+			      argc > 2 ? argv[2] : NULL, relocated_addr,
+			      image_size))
 		return 1;
 
 	return 0;
diff --git a/cmd/bootz.c b/cmd/bootz.c
index dd6fe4904b02..bcf232b4f305 100644
--- a/cmd/bootz.c
+++ b/cmd/bootz.c
@@ -54,7 +54,9 @@ static int bootz_start(struct cmd_tbl *cmdtp, int flag, int argc,
 	 * Handle the BOOTM_STATE_FINDOTHER state ourselves as we do not
 	 * have a header that provide this informaiton.
 	 */
-	if (bootm_find_images(flag, argc, argv, images->ep, zi_end - zi_start))
+	if (bootm_find_images(image_load_addr, argc > 1 ? argv[1] : NULL,
+			      argc > 2 ? argv[2] : NULL, images->ep,
+			      zi_end - zi_start))
 		return 1;
 
 	return 0;
diff --git a/include/bootm.h b/include/bootm.h
index 10a1bd65a754..f5229ea90b33 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -52,9 +52,29 @@ int boot_selected_os(int argc, char *const argv[], int state,
 
 ulong bootm_disable_interrupts(void);
 
-/* This is a special function used by booti/bootz */
-int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
-		      ulong size);
+/**
+ * bootm_find_images() - find and locate various images
+ *
+ * @img_addr: Address of image being loaded
+ * @conf_ramdisk: Indicates the ramdisk to use (typically second arg of bootm)
+ * @conf_fdt: Indicates the FDT to use (typically third arg of bootm)
+ * @start: OS image start address
+ * @size: OS image size
+ *
+ * boot_find_images() will attempt to load an available ramdisk,
+ * flattened device tree, as well as specifically marked
+ * "loadable" images (loadables are FIT only)
+ *
+ * Note: bootm_find_images will skip an image if it is not found
+ *
+ * This is a special function used by booti/bootz
+ *
+ * Return:
+ *     0, if all existing images were loaded correctly
+ *     1, if an image is found but corrupted, or invalid
+ */
+int bootm_find_images(ulong img_addr, const char *conf_ramdisk,
+		      const char *conf_fdt, ulong start, ulong size);
 
 /*
  * Measure the boot images. Measurement is the process of hashing some binary
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 28/29] bootm: Add a function to check overlap

[Simon Glass]

Move this code into a function to reduce code size and make it easier
to understand. Drop the unnecessary 0x to help a little with code size.

Use this in bootm_find_images()

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c | 51 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 54e4b48e907c..a803b0749be0 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -467,6 +467,37 @@ static int bootm_find_os(const char *addr_fit)
 	return 0;
 }
 
+/**
+ * check_overlap() - Check if an image overlaps the OS
+ *
+ * @name: Name of image to check (used to print error)
+ * @base: Base address of image
+ * @end: End address of image (+1)
+ * @os_start: Start of OS
+ * @os_size: Size of OS in bytes
+ * Return: 0 if OK, -EXDEV if the image overlaps the OS
+ */
+static int check_overlap(const char *name, ulong base, ulong end,
+			 ulong os_start, ulong os_size)
+{
+	ulong os_end;
+
+	if (!base)
+		return 0;
+	os_end = os_start + os_size;
+
+	if ((base >= os_start && base < os_end) ||
+	    (end > os_start && end <= os_end) ||
+	    (base < os_start && end >= os_end)) {
+		printf("ERROR: %s image overlaps OS image (OS=%lx..%lx)\n",
+		       name, os_start, os_end);
+
+		return -EXDEV;
+	}
+
+	return 0;
+}
+
 int bootm_find_images(ulong img_addr, const char *conf_ramdisk,
 		      const char *conf_fdt, ulong start, ulong size)
 {
@@ -496,16 +527,8 @@ int bootm_find_images(ulong img_addr, const char *conf_ramdisk,
 	}
 
 	/* check if ramdisk overlaps OS image */
-	if (images.rd_start && (((ulong)images.rd_start >= start &&
-				 (ulong)images.rd_start < start + size) ||
-				((ulong)images.rd_end > start &&
-				 (ulong)images.rd_end <= start + size) ||
-				((ulong)images.rd_start < start &&
-				 (ulong)images.rd_end >= start + size))) {
-		printf("ERROR: RD image overlaps OS image (OS=0x%lx..0x%lx)\n",
-		       start, start + size);
+	if (check_overlap("RD", images.rd_start, images.rd_end, start, size))
 		return 1;
-	}
 
 	if (CONFIG_IS_ENABLED(OF_LIBFDT)) {
 		buf = map_sysmem(img_addr, 0);
@@ -519,15 +542,9 @@ int bootm_find_images(ulong img_addr, const char *conf_ramdisk,
 		}
 
 		/* check if FDT overlaps OS image */
-		if (images.ft_addr &&
-		    (((ulong)images.ft_addr >= start &&
-		      (ulong)images.ft_addr < start + size) ||
-		     ((ulong)images.ft_addr + images.ft_len >= start &&
-		      (ulong)images.ft_addr + images.ft_len < start + size))) {
-			printf("ERROR: FDT image overlaps OS image (OS=0x%lx..0x%lx)\n",
-			       start, start + size);
+		if (check_overlap("FDT", map_to_sysmem(images.ft_addr),
+				  images.ft_len, start, size))
 			return 1;
-		}
 
 		if (IS_ENABLED(CONFIG_CMD_FDT))
 			set_working_fdt_addr(map_to_sysmem(images.ft_addr));
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 29/29] bootm: Reduce arguments to bootm_find_other()

[Simon Glass]

Rather than passing the full list of command arguments, pass only those
which are needed.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootm.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index a803b0749be0..b0eecbb0dd3c 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -571,19 +571,16 @@ int bootm_find_images(ulong img_addr, const char *conf_ramdisk,
 	return 0;
 }
 
-static int bootm_find_other(struct cmd_tbl *cmdtp, int flag, int argc,
-			    char *const argv[])
+static int bootm_find_other(ulong img_addr, const char *conf_ramdisk,
+			    const char *conf_fdt)
 {
 	if ((images.os.type == IH_TYPE_KERNEL ||
 	     images.os.type == IH_TYPE_KERNEL_NOLOAD ||
 	     images.os.type == IH_TYPE_MULTI) &&
 	    (images.os.os == IH_OS_LINUX || images.os.os == IH_OS_VXWORKS ||
 	     images.os.os == IH_OS_EFI || images.os.os == IH_OS_TEE)) {
-		ulong img_addr;
-
-		img_addr = argc ? hextoul(argv[0], NULL) : image_load_addr;
-		return bootm_find_images(img_addr, argc > 1 ? argv[1] : NULL,
-					 argc > 2 ? argv[2] : NULL, 0, 0);
+		return bootm_find_images(img_addr, conf_ramdisk, conf_fdt, 0,
+					 0);
 	}
 
 	return 0;
@@ -1012,8 +1009,13 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc,
 	if (!ret && (states & BOOTM_STATE_FINDOS))
 		ret = bootm_find_os(argv[0]);
 
-	if (!ret && (states & BOOTM_STATE_FINDOTHER))
-		ret = bootm_find_other(cmdtp, flag, argc, argv);
+	if (!ret && (states & BOOTM_STATE_FINDOTHER)) {
+		ulong img_addr;
+
+		img_addr = argc ? hextoul(argv[0], NULL) : image_load_addr;
+		ret = bootm_find_other(img_addr, argc > 1 ? argv[1] : NULL,
+				       argc > 2 ? argv[2] : NULL);
+	}
 
 	if (IS_ENABLED(CONFIG_MEASURED_BOOT) && !ret &&
 	    (states & BOOTM_STATE_MEASURE))
-- 
2.42.0.869.gea05f2083d-goog

Re: [PATCH 02/29] mmc: env: Unify the U_BOOT_ENV_LOCATION conditions

[Heinrich Schuchardt]

On 11/12/23 01:08, Simon Glass wrote:
> The declaration of U_BOOT_ENV_LOCATION() uses a different #ifdef
> condition from the code it calls. Use the same condition to avoid a
> build warning if CONFIG_CMD_SAVEENV is disabled.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   env/mmc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/env/mmc.c b/env/mmc.c
> index cb14bbb58f13..da84cddd74f0 100644
> --- a/env/mmc.c
> +++ b/env/mmc.c
> @@ -495,7 +495,7 @@ U_BOOT_ENV_LOCATION(mmc) = {
>   	.location	= ENVL_MMC,
>   	ENV_NAME("MMC")
>   	.load		= env_mmc_load,
> -#ifndef CONFIG_SPL_BUILD
> +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_SPL_BUILD)

According to README CONFIG_SPL_BUILD is not defined for TPL builds.

I assume that we don't want to have below fields in TPL either. Please, use

#if CONFIG_IS_ENABLED(CMD_SAVEENV)

Best regards

Heinrich

>   	.save		= env_save_ptr(env_mmc_save),
>   	.erase		= ENV_ERASE_PTR(env_mmc_erase)
>   #endif

Re: [PATCH 01/29] arm: x86: Drop discarding of command linker-lists

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 327 bytes --]

On Sat, Nov 11, 2023 at 05:08:46PM -0700, Simon Glass wrote:

> Since we can now cleanly disable CMDLINE when needed, drop the rules
> which discard the command code.  It will not be built in the first
> place.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 03/29] treewide: Tidy up semicolon after command macros

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 415 bytes --]

On Sat, Nov 11, 2023 at 05:08:48PM -0700, Simon Glass wrote:

> The U_BOOT_CMD_COMPLETE() macro has a semicolon at the end, perhaps
> inadvertently. Some code has taken advantage of this.
> 
> Tidy this up by dropping the semicolon from the macro and adding it to
> macro invocations as required.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 07/29] bootm: Simplify arguments for bootm_pre_load()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 325 bytes --]

On Sat, Nov 11, 2023 at 05:08:52PM -0700, Simon Glass wrote:

> Move the argument decoding to the caller, to avoid needing to pass the
> command-line arguments.
> 
> Add a function comment while we are here.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 06/29] bootm: Drop arguments from bootm_start()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 231 bytes --]

On Sat, Nov 11, 2023 at 05:08:51PM -0700, Simon Glass wrote:

> This function does not use its arguments. Drop them.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 08/29] bootm: Move boot_get_kernel() higher in the file

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 367 bytes --]

On Sat, Nov 11, 2023 at 05:08:53PM -0700, Simon Glass wrote:

> Move this code and image_get_kernel() higher in the file to avoid the
> need for a forward declaration.
> 
> No attempt is made to remove #ifdefs or adjust the code in any other
> way.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 09/29] image: Tidy up genimg_get_kernel_addr_fit()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

On Sat, Nov 11, 2023 at 05:08:54PM -0700, Simon Glass wrote:

> This function does not modify its first argument, so mark it const. Also
> move the comments to the header file and expand them to provide more
> useful information.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 10/29] bootm: Reduce arguments to boot_get_kernel()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 979 bytes --]

On Sat, Nov 11, 2023 at 05:08:55PM -0700, Simon Glass wrote:

> This function only uses one argument and just needs to know the name of
> the command which called it. Adjust the function to use only what it
> needs. This will make it easier to call from a non-command context.
> 
> Tidy up the function comment while we are here.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
[snip]
> @@ -312,8 +316,8 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc,
>  	int ret;
>  
>  	/* get kernel image header, start address and length */
> -	os_hdr = boot_get_kernel(cmdtp, flag, argc, argv,
> -			&images, &images.os.image_start, &images.os.image_len);
> +	os_hdr = boot_get_kernel("bootm", argv[0], &images,
> +				 &images.os.image_start, &images.os.image_len);

Shouldn't this be cmdtp->name not "bootm" ? The eventual use case is
reporting back to the user that what they tried to use with
bootm/booti/bootz isn't a match.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 11/29] image: Document error codes from fit_image_load()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

On Sat, Nov 11, 2023 at 05:08:56PM -0700, Simon Glass wrote:

> Put a list of these in the function documentation so it is easier to
> decode what went wrong.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 12/29] bootm: Adjust boot_get_kernel() to return an error

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1165 bytes --]

On Sat, Nov 11, 2023 at 05:08:57PM -0700, Simon Glass wrote:

> This function obtains lots of error codes and then throws them away.
> Update it to return the error, moving the image pointer to an
> argument.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
[snip]
> @@ -217,25 +220,28 @@ static const void *boot_get_kernel(const char *cmd_name, const char *addr_fit,
>  			vendor_boot_img = map_sysmem(get_avendor_bootimg_addr(), 0);
>  		}
>  		printf("## Booting Android Image at 0x%08lx ...\n", img_addr);
> -		if (android_image_get_kernel(boot_img, vendor_boot_img, images->verify,
> -					     os_data, os_len))
> -			return NULL;
> +		ret = android_image_get_kernel(boot_img, vendor_boot_img,
> +					       images->verify, os_data, os_len);
> +		if (ret)
> +			return ret;
>  		if (IS_ENABLED(CONFIG_CMD_ABOOTIMG)) {
>  			unmap_sysmem(vendor_boot_img);
>  			unmap_sysmem(boot_img);
>  		}

This highlights a pre-existing leak, yes? If so, can you please do a
follow-up to move if (ret) return ret; to after the unmaps? Or am I
missing something? For the patch itself:

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 13/29] bootm: Use the error return from boot_get_kernel()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 284 bytes --]

On Sat, Nov 11, 2023 at 05:08:58PM -0700, Simon Glass wrote:

> Rather than looking for a zero-sized image, use the error code returned
> to determine if things are OK.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 14/29] bootstage: Drop BOOTSTAGE_ID_FIT_KERNEL_INFO

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 312 bytes --]

On Sat, Nov 11, 2023 at 05:08:59PM -0700, Simon Glass wrote:

> This is a misnomer since we don't necessarily know that the image is a
> FIT. Use the existing BOOTSTAGE_ID_CHECK_IMAGETYPE instead.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 15/29] bootm: Move error printing out of boot_get_kernel()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 925 bytes --]

On Sat, Nov 11, 2023 at 05:09:00PM -0700, Simon Glass wrote:

> The same error message is printed in two places. Move it out to the
> caller so we can avoid passing in the command name. Leave the bootstage
> handling where it is.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
[snip]
> @@ -322,9 +316,12 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc,
>  	int ret;
>  
>  	/* get kernel image header, start address and length */
> -	ret = boot_get_kernel("bootm", argv[0], &images, &images.os.image_start,
> +	ret = boot_get_kernel(argv[0], &images, &images.os.image_start,
>  			      &images.os.image_len, &os_hdr);
>  	if (ret) {
> +		if (ret == -EPROTOTYPE)
> +			printf("Wrong Image Type for bootm command\n");


This compounds what I said on the earlier patch, yes? Or can we not
actually get here via booti/bootz as their header checking kicks in
earlier?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 16/29] bootm: Reduce arguments to boot_find_os()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 295 bytes --]

On Sat, Nov 11, 2023 at 05:09:01PM -0700, Simon Glass wrote:

> This function only uses one argument so pass it in directly.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Aside from a typo in the subject (boot_ not bootm_):

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 17/29] bootm: Reduce arguments to boot_get_ramdisk()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 558 bytes --]

On Sat, Nov 11, 2023 at 05:09:02PM -0700, Simon Glass wrote:

> This function normally only uses one argument so pass it in directly.
> Move comments to the header file so could one day include these
> functions in API docs. Fix up the u8 argument while here, since it
> avoids the compiler having to mask the value on some machines.
> 
> The Android case here is bit strange, since it can use argv[0], so deal
> with that in the caller.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 18/29] fdt: Allow use of fdt_support inside if() statements

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

On Sat, Nov 11, 2023 at 05:09:03PM -0700, Simon Glass wrote:

> Most of the fdt_support.h header file is included only if OF_LIBFDT or
> OF_CONTROL are enabled. This means that calling functions defined in
> that file must happen inside an #ifdef
> 
> This is unnecessary, so reduce the condition to just !USE_HOSTCC
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 21/29] bootm: Reduce arguments to boot_get_fdt()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 263 bytes --]

On Sat, Nov 11, 2023 at 05:09:06PM -0700, Simon Glass wrote:

> This function only uses one argument from bootm (argv[2]) so pass it in
> directly.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 22/29] bootm: Reduce arguments to boot_get_fpga()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]

On Sat, Nov 11, 2023 at 05:09:07PM -0700, Simon Glass wrote:

> This function only uses two arguments. The 'arch' always has a constant
> value, so drop it. This simplifies the function call.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 23/29] bootm: Reduce arguments to boot_get_loadables()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 362 bytes --]

On Sat, Nov 11, 2023 at 05:09:08PM -0700, Simon Glass wrote:

> This function only uses two arguments. The 'arch' always has a constant
> value, so drop it. This simplifies the function call.
> 
> Tidy up the function comment while we are here.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 24/29] bootm: Simplify Android ramdisk addr in bootm_find_images()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 479 bytes --]

On Sat, Nov 11, 2023 at 05:09:09PM -0700, Simon Glass wrote:

> The Android mechanism uses the loadaddr envrionment-variable to get the
> load address, if none is provided. This is equivalent to
> image_load_addr so use that instead, converting it to a string as
> needed.
> 
> This change will permit passing img_addr to this function, in a future
> change.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 27/29] bootm: Adjust the parameters of bootm_find_images()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

On Sat, Nov 11, 2023 at 05:09:12PM -0700, Simon Glass wrote:

> Rather than passing it all the command-line args, pass in the pieces
> that it needs. These are the image address, the ramdisk address/name
> and the FDT address/name.
> 
> Ultimately this will allow usage of this function without being called
> from the command line.

OK, so this goal is good.

[snip]
> +		return bootm_find_images(img_addr, argc > 1 ? argv[1] : NULL,
> +					 argc > 2 ? argv[2] : NULL, 0, 0);

That we repeat this much harder to read test/if/else three times now is
less good.  Can we find some way to hide the complexity here in the case
where it's coming from a command and so we have argc/argv[] ?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 28/29] bootm: Add a function to check overlap

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 362 bytes --]

On Sat, Nov 11, 2023 at 05:09:13PM -0700, Simon Glass wrote:

> Move this code into a function to reduce code size and make it easier
> to understand. Drop the unnecessary 0x to help a little with code size.
> 
> Use this in bootm_find_images()
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 29/29] bootm: Reduce arguments to bootm_find_other()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 271 bytes --]

On Sat, Nov 11, 2023 at 05:09:14PM -0700, Simon Glass wrote:

> Rather than passing the full list of command arguments, pass only those
> which are needed.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 27/29] bootm: Adjust the parameters of bootm_find_images()

[Simon Glass]

Hi Tom,

On Wed, 15 Nov 2023 at 15:38, Tom Rini <trini@konsulko.com> wrote:
>
> On Sat, Nov 11, 2023 at 05:09:12PM -0700, Simon Glass wrote:
>
> > Rather than passing it all the command-line args, pass in the pieces
> > that it needs. These are the image address, the ramdisk address/name
> > and the FDT address/name.
> >
> > Ultimately this will allow usage of this function without being called
> > from the command line.
>
> OK, so this goal is good.
>
> [snip]
> > +             return bootm_find_images(img_addr, argc > 1 ? argv[1] : NULL,
> > +                                      argc > 2 ? argv[2] : NULL, 0, 0);
>
> That we repeat this much harder to read test/if/else three times now is
> less good.  Can we find some way to hide the complexity here in the case
> where it's coming from a command and so we have argc/argv[] ?

I can't really think of one. Ultimately this is coming from the fact
that the booti and bootz commands directly call bootm_find_images(). I
haven't got far enough to know whether that will still be true in the
end, but I hope not.

IMO the correct place for the logic above is in the command-processing
code, where it decides which arg means what.

I could imagine something like:

static const char *cmd_get_arg(int argc, char *const argv[], int argnum)
{
   return argc > argnum ? argv[argnum] : NULL;
}

but I'm not sure that is an improvement.

I have got a little further (15 more patches) and have added a 'struct
bootm_info' to hold the state of the boot, including the arguments for
the FIT address, ramdisk and fdt. But even then, the args will need to
be set up by the individual commands based on their syntax.

Regards,
Simon

Re: [PATCH 27/29] bootm: Adjust the parameters of bootm_find_images()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]

On Wed, Nov 15, 2023 at 06:42:19PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Wed, 15 Nov 2023 at 15:38, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sat, Nov 11, 2023 at 05:09:12PM -0700, Simon Glass wrote:
> >
> > > Rather than passing it all the command-line args, pass in the pieces
> > > that it needs. These are the image address, the ramdisk address/name
> > > and the FDT address/name.
> > >
> > > Ultimately this will allow usage of this function without being called
> > > from the command line.
> >
> > OK, so this goal is good.
> >
> > [snip]
> > > +             return bootm_find_images(img_addr, argc > 1 ? argv[1] : NULL,
> > > +                                      argc > 2 ? argv[2] : NULL, 0, 0);
> >
> > That we repeat this much harder to read test/if/else three times now is
> > less good.  Can we find some way to hide the complexity here in the case
> > where it's coming from a command and so we have argc/argv[] ?
> 
> I can't really think of one. Ultimately this is coming from the fact
> that the booti and bootz commands directly call bootm_find_images(). I
> haven't got far enough to know whether that will still be true in the
> end, but I hope not.
> 
> IMO the correct place for the logic above is in the command-processing
> code, where it decides which arg means what.
> 
> I could imagine something like:
> 
> static const char *cmd_get_arg(int argc, char *const argv[], int argnum)
> {
>    return argc > argnum ? argv[argnum] : NULL;
> }
> 
> but I'm not sure that is an improvement.

I was thinking about this more after I sent this. And I think we might
indeed want an inline / macro to handle this case more generally as I
suspect we'll have similar cases where we need argN-or-NULL as we
refactor other areas of code to split "here is the command" from "here
is the library functionality" of it. Then we'll have:
ulong foo = cmd_arg_one(argc, argv);
ulong bar = cmd_arg_two(argc, argv);

librarycall(foo, bar, ...);

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 27/29] bootm: Adjust the parameters of bootm_find_images()

[Simon Glass]

Hi Tom,

On Wed, 15 Nov 2023 at 18:47, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Nov 15, 2023 at 06:42:19PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Wed, 15 Nov 2023 at 15:38, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sat, Nov 11, 2023 at 05:09:12PM -0700, Simon Glass wrote:
> > >
> > > > Rather than passing it all the command-line args, pass in the pieces
> > > > that it needs. These are the image address, the ramdisk address/name
> > > > and the FDT address/name.
> > > >
> > > > Ultimately this will allow usage of this function without being called
> > > > from the command line.
> > >
> > > OK, so this goal is good.
> > >
> > > [snip]
> > > > +             return bootm_find_images(img_addr, argc > 1 ? argv[1] : NULL,
> > > > +                                      argc > 2 ? argv[2] : NULL, 0, 0);
> > >
> > > That we repeat this much harder to read test/if/else three times now is
> > > less good.  Can we find some way to hide the complexity here in the case
> > > where it's coming from a command and so we have argc/argv[] ?
> >
> > I can't really think of one. Ultimately this is coming from the fact
> > that the booti and bootz commands directly call bootm_find_images(). I
> > haven't got far enough to know whether that will still be true in the
> > end, but I hope not.
> >
> > IMO the correct place for the logic above is in the command-processing
> > code, where it decides which arg means what.
> >
> > I could imagine something like:
> >
> > static const char *cmd_get_arg(int argc, char *const argv[], int argnum)
> > {
> >    return argc > argnum ? argv[argnum] : NULL;
> > }
> >
> > but I'm not sure that is an improvement.
>
> I was thinking about this more after I sent this. And I think we might
> indeed want an inline / macro to handle this case more generally as I
> suspect we'll have similar cases where we need argN-or-NULL as we
> refactor other areas of code to split "here is the command" from "here
> is the library functionality" of it. Then we'll have:
> ulong foo = cmd_arg_one(argc, argv);
> ulong bar = cmd_arg_two(argc, argv);
>
> librarycall(foo, bar, ...);

OK I can try something. But note that the one, two terminology becomes
confusing. Is the first argument zero or one? Also we often drop an
argument in a subcommand to allow things to start from 0 again.

It might be better to use first and second, instead?

Regards,
Simon

Re: [PATCH 27/29] bootm: Adjust the parameters of bootm_find_images()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2733 bytes --]

On Wed, Nov 15, 2023 at 06:56:33PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Wed, 15 Nov 2023 at 18:47, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Nov 15, 2023 at 06:42:19PM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Wed, 15 Nov 2023 at 15:38, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Sat, Nov 11, 2023 at 05:09:12PM -0700, Simon Glass wrote:
> > > >
> > > > > Rather than passing it all the command-line args, pass in the pieces
> > > > > that it needs. These are the image address, the ramdisk address/name
> > > > > and the FDT address/name.
> > > > >
> > > > > Ultimately this will allow usage of this function without being called
> > > > > from the command line.
> > > >
> > > > OK, so this goal is good.
> > > >
> > > > [snip]
> > > > > +             return bootm_find_images(img_addr, argc > 1 ? argv[1] : NULL,
> > > > > +                                      argc > 2 ? argv[2] : NULL, 0, 0);
> > > >
> > > > That we repeat this much harder to read test/if/else three times now is
> > > > less good.  Can we find some way to hide the complexity here in the case
> > > > where it's coming from a command and so we have argc/argv[] ?
> > >
> > > I can't really think of one. Ultimately this is coming from the fact
> > > that the booti and bootz commands directly call bootm_find_images(). I
> > > haven't got far enough to know whether that will still be true in the
> > > end, but I hope not.
> > >
> > > IMO the correct place for the logic above is in the command-processing
> > > code, where it decides which arg means what.
> > >
> > > I could imagine something like:
> > >
> > > static const char *cmd_get_arg(int argc, char *const argv[], int argnum)
> > > {
> > >    return argc > argnum ? argv[argnum] : NULL;
> > > }
> > >
> > > but I'm not sure that is an improvement.
> >
> > I was thinking about this more after I sent this. And I think we might
> > indeed want an inline / macro to handle this case more generally as I
> > suspect we'll have similar cases where we need argN-or-NULL as we
> > refactor other areas of code to split "here is the command" from "here
> > is the library functionality" of it. Then we'll have:
> > ulong foo = cmd_arg_one(argc, argv);
> > ulong bar = cmd_arg_two(argc, argv);
> >
> > librarycall(foo, bar, ...);
> 
> OK I can try something. But note that the one, two terminology becomes
> confusing. Is the first argument zero or one? Also we often drop an
> argument in a subcommand to allow things to start from 0 again.
> 
> It might be better to use first and second, instead?

Yes, please come up with a better naming scheme, I'm terrible about
stuff like that.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 27/29] bootm: Adjust the parameters of bootm_find_images()

[Simon Glass]

Hi Tom,

On Wed, 15 Nov 2023 at 19:07, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Nov 15, 2023 at 06:56:33PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Wed, 15 Nov 2023 at 18:47, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Wed, Nov 15, 2023 at 06:42:19PM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Wed, 15 Nov 2023 at 15:38, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Sat, Nov 11, 2023 at 05:09:12PM -0700, Simon Glass wrote:
> > > > >
> > > > > > Rather than passing it all the command-line args, pass in the pieces
> > > > > > that it needs. These are the image address, the ramdisk address/name
> > > > > > and the FDT address/name.
> > > > > >
> > > > > > Ultimately this will allow usage of this function without being called
> > > > > > from the command line.
> > > > >
> > > > > OK, so this goal is good.
> > > > >
> > > > > [snip]
> > > > > > +             return bootm_find_images(img_addr, argc > 1 ? argv[1] : NULL,
> > > > > > +                                      argc > 2 ? argv[2] : NULL, 0, 0);
> > > > >
> > > > > That we repeat this much harder to read test/if/else three times now is
> > > > > less good.  Can we find some way to hide the complexity here in the case
> > > > > where it's coming from a command and so we have argc/argv[] ?
> > > >
> > > > I can't really think of one. Ultimately this is coming from the fact
> > > > that the booti and bootz commands directly call bootm_find_images(). I
> > > > haven't got far enough to know whether that will still be true in the
> > > > end, but I hope not.
> > > >
> > > > IMO the correct place for the logic above is in the command-processing
> > > > code, where it decides which arg means what.
> > > >
> > > > I could imagine something like:
> > > >
> > > > static const char *cmd_get_arg(int argc, char *const argv[], int argnum)
> > > > {
> > > >    return argc > argnum ? argv[argnum] : NULL;
> > > > }
> > > >
> > > > but I'm not sure that is an improvement.
> > >
> > > I was thinking about this more after I sent this. And I think we might
> > > indeed want an inline / macro to handle this case more generally as I
> > > suspect we'll have similar cases where we need argN-or-NULL as we
> > > refactor other areas of code to split "here is the command" from "here
> > > is the library functionality" of it. Then we'll have:
> > > ulong foo = cmd_arg_one(argc, argv);
> > > ulong bar = cmd_arg_two(argc, argv);
> > >
> > > librarycall(foo, bar, ...);
> >
> > OK I can try something. But note that the one, two terminology becomes
> > confusing. Is the first argument zero or one? Also we often drop an
> > argument in a subcommand to allow things to start from 0 again.
> >
> > It might be better to use first and second, instead?
>
> Yes, please come up with a better naming scheme, I'm terrible about
> stuff like that.

Actually my naming has problems too...hopefully this has been solved
properly elsewhere and someone will reply to my patch.

Regards,
Simon

Re: [PATCH 02/29] mmc: env: Unify the U_BOOT_ENV_LOCATION conditions

[Simon Glass]

Hi Heinrich,

On Wed, 15 Nov 2023 at 03:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 11/12/23 01:08, Simon Glass wrote:
> > The declaration of U_BOOT_ENV_LOCATION() uses a different #ifdef
> > condition from the code it calls. Use the same condition to avoid a
> > build warning if CONFIG_CMD_SAVEENV is disabled.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >   env/mmc.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/env/mmc.c b/env/mmc.c
> > index cb14bbb58f13..da84cddd74f0 100644
> > --- a/env/mmc.c
> > +++ b/env/mmc.c
> > @@ -495,7 +495,7 @@ U_BOOT_ENV_LOCATION(mmc) = {
> >       .location       = ENVL_MMC,
> >       ENV_NAME("MMC")
> >       .load           = env_mmc_load,
> > -#ifndef CONFIG_SPL_BUILD
> > +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_SPL_BUILD)
>
> According to README CONFIG_SPL_BUILD is not defined for TPL builds.
>
> I assume that we don't want to have below fields in TPL either. Please, use
>
> #if CONFIG_IS_ENABLED(CMD_SAVEENV)

I missed this comment in the new version. But note that
CONFIG_SPL_BUILD covers TPL (and others) as well.

It is a bit confusing. Perhaps we should introduce CONFIG_XPL_BUILD to
mean anything other than U-Boot proper?

>
> Best regards
>
> Heinrich
>
> >       .save           = env_save_ptr(env_mmc_save),
> >       .erase          = ENV_ERASE_PTR(env_mmc_erase)
> >   #endif
>

Regards,
Simon

Re: [PATCH 02/29] mmc: env: Unify the U_BOOT_ENV_LOCATION conditions

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1940 bytes --]

On Sun, Nov 19, 2023 at 07:49:32AM -0700, Simon Glass wrote:
> Hi Heinrich,
> 
> On Wed, 15 Nov 2023 at 03:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > On 11/12/23 01:08, Simon Glass wrote:
> > > The declaration of U_BOOT_ENV_LOCATION() uses a different #ifdef
> > > condition from the code it calls. Use the same condition to avoid a
> > > build warning if CONFIG_CMD_SAVEENV is disabled.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > >   env/mmc.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/env/mmc.c b/env/mmc.c
> > > index cb14bbb58f13..da84cddd74f0 100644
> > > --- a/env/mmc.c
> > > +++ b/env/mmc.c
> > > @@ -495,7 +495,7 @@ U_BOOT_ENV_LOCATION(mmc) = {
> > >       .location       = ENVL_MMC,
> > >       ENV_NAME("MMC")
> > >       .load           = env_mmc_load,
> > > -#ifndef CONFIG_SPL_BUILD
> > > +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_SPL_BUILD)
> >
> > According to README CONFIG_SPL_BUILD is not defined for TPL builds.
> >
> > I assume that we don't want to have below fields in TPL either. Please, use
> >
> > #if CONFIG_IS_ENABLED(CMD_SAVEENV)
> 
> I missed this comment in the new version. But note that
> CONFIG_SPL_BUILD covers TPL (and others) as well.
> 
> It is a bit confusing. Perhaps we should introduce CONFIG_XPL_BUILD to
> mean anything other than U-Boot proper?

We can see if something like that makes more sense as we further
separate out "library" functionality from "command" functionality. We
might swing back to needing to save environment changes from the
non-cmdline use cases all the same (assorted canary type environment
variables, etc) and be back to needing to tweak this differently. So
what's here is fine with me for today.

Reviewed-by: Tom Rini <trini@konsulko.com>

... and just put that on the next iteration of the series.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]