[PATCH 00/19] vbe: Series part E

[PATCH 00/19] vbe: Series part E

[Simon Glass]

This includes various patches towards implementing the VBE abrec
bootmeth in U-Boot. It mostly focuses on SPL tweaks and adjusting what
fatures are available in VPL.


Simon Glass (19):
  image: Add a prototype for fit_image_get_phase()
  serial: ns16550: Allow clocks to be missing
  boot: Allow FIT to fall back from best-match option
  bootstd: Avoid sprintf() in SPL when creating bootdevs
  boot: Respect the load_op in fit_image_load()
  malloc: Show amount of used space when memory runs out
  malloc: Provide a simple malloc for VPL
  Support setting a maximum size for the VPL image
  spl: Report a loader failure
  spl: Allow serial to be disabled in any XPL phase
  spl: Support a relocated stack in any XPL phase
  spl: Drop use of uintptr_t
  spl: Drop a duplicate variable in boot_from_devices()
  spl: Add some more debugging to load_simple_fit()
  spl: lib: Allow for decompression in any SPL build
  boot: Allow use of FIT in TPL and VPL
  lib: Allow crc8 in TPL and VPL
  boot: Imply CRC8 with VBE
  hash: Plumb crc8 into the hash functions

 boot/Kconfig                  | 71 ++++++++++++++++++++++++++++++++++-
 boot/Makefile                 |  4 +-
 boot/bootdev-uclass.c         | 10 ++++-
 boot/image-fit.c              | 29 ++++++++------
 common/hash.c                 |  8 ++++
 common/malloc_simple.c        |  3 +-
 common/spl/Kconfig.vpl        | 17 +++++++++
 common/spl/spl.c              | 15 +++++---
 common/spl/spl_atf.c          | 36 +++++++++---------
 common/spl/spl_fit.c          | 12 +++++-
 common/spl/spl_legacy.c       |  8 ++--
 configs/sandbox_vpl_defconfig |  3 +-
 drivers/serial/ns16550.c      |  2 +-
 include/image.h               | 16 +++++++-
 include/spl.h                 | 28 +++++++-------
 include/u-boot/crc.h          |  3 ++
 lib/Kconfig                   | 53 ++++++++++++++++++++++++++
 lib/Makefile                  |  8 ++--
 lib/crc8.c                    |  6 +++
 19 files changed, 263 insertions(+), 69 deletions(-)

-- 
2.34.1

[PATCH 01/19] image: Add a prototype for fit_image_get_phase()

[Simon Glass]

This function exists but is not exported. Add a prototype so it can be
used elsewhere.

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

 include/image.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/image.h b/include/image.h
index dd4042d1bd9..dbf8d0e7ba9 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1183,6 +1183,18 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset,
 int fit_image_get_data_and_size(const void *fit, int noffset,
 				const void **data, size_t *size);
 
+/**
+ * fit_image_get_phase() - Get the phase from a FIT image
+ *
+ * @fit: FIT to read from
+ * @offset: offset node to read
+ * @phasep: Returns phase, if any
+ * Return: 0 if read OK and *phasep is value, -ENOENT if there was no phase
+ * property in the node, other -ve value on other error
+ */
+int fit_image_get_phase(const void *fit, int offset,
+			enum image_phase_t *phasep);
+
 /**
  * fit_get_data_node() - Get verified image data for an image
  * @fit: Pointer to the FIT format image header
-- 
2.34.1

[PATCH 02/19] serial: ns16550: Allow clocks to be missing

[Simon Glass]

Allow serial init when clock support is not enabled in a particular phase.

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

 drivers/serial/ns16550.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index 6fcb5b523ac..5cda273623d 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -570,7 +570,7 @@ int ns16550_serial_of_to_plat(struct udevice *dev)
 		err = clk_get_rate(&clk);
 		if (!IS_ERR_VALUE(err))
 			plat->clock = err;
-	} else if (err != -ENOENT && err != -ENODEV && err != -ENOSYS) {
+	} else if (err != -ENOENT && err != -ENODEV && err != -ENOSYS && err != -EALREADY) {
 		debug("ns16550 failed to get clock\n");
 		return err;
 	}
-- 
2.34.1

[PATCH 03/19] boot: Allow FIT to fall back from best-match option

[Simon Glass]

When the best-match feature fails to find something, use the provided
config name as a fallback. The allows SPL to select a suitable config
when best-match is enabled.

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

 boot/image-fit.c | 19 ++++++++++---------
 include/image.h  |  4 +++-
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/boot/image-fit.c b/boot/image-fit.c
index 7d56f0b5e6e..439ff51edf6 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -1729,13 +1729,13 @@ int fit_conf_find_compat(const void *fit, const void *fdt)
 	images_noffset = fdt_path_offset(fit, FIT_IMAGES_PATH);
 	if (confs_noffset < 0 || images_noffset < 0) {
 		debug("Can't find configurations or images nodes.\n");
-		return -1;
+		return -EINVAL;
 	}
 
 	fdt_compat = fdt_getprop(fdt, 0, "compatible", &fdt_compat_len);
 	if (!fdt_compat) {
 		debug("Fdt for comparison has no \"compatible\" property.\n");
-		return -1;
+		return -ENXIO;
 	}
 
 	/*
@@ -1812,7 +1812,7 @@ int fit_conf_find_compat(const void *fit, const void *fdt)
 	}
 	if (!best_match_offset) {
 		debug("No match found.\n");
-		return -1;
+		return -ENOENT;
 	}
 
 	return best_match_offset;
@@ -2095,17 +2095,18 @@ int fit_image_load(struct bootm_headers *images, ulong addr,
 		 * fit_conf_get_node() will try to find default config node
 		 */
 		bootstage_mark(bootstage_id + BOOTSTAGE_SUB_NO_UNIT_NAME);
-		if (IS_ENABLED(CONFIG_FIT_BEST_MATCH) && !fit_uname_config) {
-			cfg_noffset = fit_conf_find_compat(fit, gd_fdt_blob());
-		} else {
-			cfg_noffset = fit_conf_get_node(fit, fit_uname_config);
-		}
-		if (cfg_noffset < 0) {
+		ret = -ENXIO;
+		if (IS_ENABLED(CONFIG_FIT_BEST_MATCH) && !fit_uname_config)
+			ret = fit_conf_find_compat(fit, gd_fdt_blob());
+		if (ret < 0 && ret != -EINVAL)
+			ret = fit_conf_get_node(fit, fit_uname_config);
+		if (ret < 0) {
 			puts("Could not find configuration node\n");
 			bootstage_error(bootstage_id +
 					BOOTSTAGE_SUB_NO_UNIT_NAME);
 			return -ENOENT;
 		}
+		cfg_noffset = ret;
 
 		fit_base_uname_config = fdt_get_name(fit, cfg_noffset, NULL);
 		printf("   Using '%s' configuration\n", fit_base_uname_config);
diff --git a/include/image.h b/include/image.h
index dbf8d0e7ba9..92128180e0f 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1423,7 +1423,9 @@ int fit_check_format(const void *fit, ulong size);
  * copied into the configuration node in the FIT image. This is required to
  * match configurations with compressed FDTs.
  *
- * Returns: offset to the configuration to use if one was found, -1 otherwise
+ * Returns: offset to the configuration to use if one was found, -EINVAL if
+ * there a /configurations or /images node is missing, -ENOENT if no match was
+ * found, -ENXIO if the FDT node has no compatible string
  */
 int fit_conf_find_compat(const void *fit, const void *fdt);
 
-- 
2.34.1

[PATCH 04/19] bootstd: Avoid sprintf() in SPL when creating bootdevs

[Simon Glass]

The name of the bootdev device is not that important, particular in SPL.
Save a little code space by using a simpler name.

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

 boot/bootdev-uclass.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c
index 7c7bba088c9..6a0d8466e95 100644
--- a/boot/bootdev-uclass.c
+++ b/boot/bootdev-uclass.c
@@ -16,6 +16,7 @@
 #include <malloc.h>
 #include <part.h>
 #include <sort.h>
+#include <spl.h>
 #include <dm/device-internal.h>
 #include <dm/lists.h>
 #include <dm/uclass-internal.h>
@@ -278,8 +279,13 @@ int bootdev_setup_for_sibling_blk(struct udevice *blk, const char *drv_name)
 	int ret, len;
 
 	len = bootdev_get_suffix_start(blk, ".blk");
-	snprintf(dev_name, sizeof(dev_name), "%.*s.%s", len, blk->name,
-		 "bootdev");
+	if (spl_phase() < PHASE_BOARD_R) {
+		strlcpy(dev_name, blk->name, sizeof(dev_name) - 4);
+		strcat(dev_name, ".sib");
+	} else {
+		snprintf(dev_name, sizeof(dev_name), "%.*s.%s", len, blk->name,
+			 "bootdev");
+	}
 
 	parent = dev_get_parent(blk);
 	ret = device_find_child_by_name(parent, dev_name, &dev);
-- 
2.34.1

[PATCH 05/19] boot: Respect the load_op in fit_image_load()

[Simon Glass]

Some code has crept in which ignores this parameter. Fix this and add a
little debugging.

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

 boot/image-fit.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/boot/image-fit.c b/boot/image-fit.c
index 439ff51edf6..db7fb61bca9 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -2226,6 +2226,7 @@ int fit_image_load(struct bootm_headers *images, ulong addr,
 	data = map_to_sysmem(buf);
 	load = data;
 	if (load_op == FIT_LOAD_IGNORED) {
+		log_debug("load_op: not loading\n");
 		/* Don't load */
 	} else if (fit_image_get_load(fit, noffset, &load)) {
 		if (load_op == FIT_LOAD_REQUIRED) {
@@ -2262,10 +2263,13 @@ int fit_image_load(struct bootm_headers *images, ulong addr,
 	/* Kernel images get decompressed later in bootm_load_os(). */
 	if (!fit_image_get_comp(fit, noffset, &comp) &&
 	    comp != IH_COMP_NONE &&
+	    load_op != FIT_LOAD_IGNORED &&
 	    !(image_type == IH_TYPE_KERNEL ||
 	      image_type == IH_TYPE_KERNEL_NOLOAD ||
 	      image_type == IH_TYPE_RAMDISK)) {
 		ulong max_decomp_len = len * 20;
+
+		log_debug("decompressing image\n");
 		if (load == data) {
 			loadbuf = malloc(max_decomp_len);
 			load = map_to_sysmem(loadbuf);
@@ -2280,6 +2284,7 @@ int fit_image_load(struct bootm_headers *images, ulong addr,
 		}
 		len = load_end - load;
 	} else if (load != data) {
+		log_debug("copying\n");
 		loadbuf = map_sysmem(load, len);
 		memcpy(loadbuf, buf, len);
 	}
@@ -2289,8 +2294,9 @@ int fit_image_load(struct bootm_headers *images, ulong addr,
 		     " please fix your .its file!\n");
 
 	/* verify that image data is a proper FDT blob */
-	if (image_type == IH_TYPE_FLATDT && fdt_check_header(loadbuf)) {
-		puts("Subimage data is not a FDT");
+	if (load_op != FIT_LOAD_IGNORED && image_type == IH_TYPE_FLATDT &&
+	    fdt_check_header(loadbuf)) {
+		puts("Subimage data is not a FDT\n");
 		return -ENOEXEC;
 	}
 
-- 
2.34.1

[PATCH 06/19] malloc: Show amount of used space when memory runs out

[Simon Glass]

Show a bit more information when malloc() space is exhausted and
debugging is enabled.

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

 common/malloc_simple.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/common/malloc_simple.c b/common/malloc_simple.c
index 5a8ec538f8f..f2b3dc53689 100644
--- a/common/malloc_simple.c
+++ b/common/malloc_simple.c
@@ -26,7 +26,8 @@ static void *alloc_simple(size_t bytes, int align)
 	log_debug("size=%lx, ptr=%lx, limit=%x: ", (ulong)bytes, new_ptr,
 		  gd->malloc_limit);
 	if (new_ptr > gd->malloc_limit) {
-		log_err("alloc space exhausted\n");
+		log_err("alloc space exhausted %lx %x\n", new_ptr,
+			gd->malloc_limit);
 		return NULL;
 	}
 
-- 
2.34.1

[PATCH 07/19] malloc: Provide a simple malloc for VPL

[Simon Glass]

The VPL phase may want to use the smaller malloc() implementation, so
add an option for this.

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

 common/spl/Kconfig.vpl | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/common/spl/Kconfig.vpl b/common/spl/Kconfig.vpl
index d06f36d4ee4..3dc0e95e09f 100644
--- a/common/spl/Kconfig.vpl
+++ b/common/spl/Kconfig.vpl
@@ -222,6 +222,15 @@ config VPL_SPI_FLASH_SUPPORT
 	  lines). This enables the drivers in drivers/mtd/spi as part of a
 	  VPL build. This normally requires VPL_SPI_SUPPORT.
 
+config VPL_SYS_MALLOC_SIMPLE
+	bool "Only use malloc_simple functions in the VPL"
+	default y
+	help
+	  Say Y here to only use the *_simple malloc functions from
+	  malloc_simple.c, rather then using the versions from dlmalloc.c;
+	  this will make the VPL binary smaller at the cost of more heap
+	  usage as the *_simple malloc functions do not re-use free-ed mem.
+
 config VPL_TEXT_BASE
 	hex "VPL Text Base"
 	default 0x0
-- 
2.34.1

[PATCH 08/19] Support setting a maximum size for the VPL image

[Simon Glass]

Add a size limit for VPL, to match those for SPL and TPL

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

 common/spl/Kconfig.vpl | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/common/spl/Kconfig.vpl b/common/spl/Kconfig.vpl
index 3dc0e95e09f..eb57dfabea5 100644
--- a/common/spl/Kconfig.vpl
+++ b/common/spl/Kconfig.vpl
@@ -237,6 +237,14 @@ config VPL_TEXT_BASE
 	help
 	  The address in memory that VPL will be running from.
 
+config VPL_MAX_SIZE
+	hex "Maximum size (in bytes) for the VPL stage"
+	default 0x2e000 if ROCKCHIP_RK3399
+	default 0x0
+	help
+	  The maximum size (in bytes) of the TPL stage. This size is determined
+	  by the amount of internal SRAM memory.
+
 config VPL_BINMAN_SYMBOLS
 	bool "Declare binman symbols in VPL"
 	depends on VPL_FRAMEWORK && BINMAN
-- 
2.34.1

[PATCH 09/19] spl: Report a loader failure

[Simon Glass]

If a loader returns an error code it is silently ignored. Show a message
to at least provide some feedback to the user.

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

 common/spl/spl.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index da110ee0783..143ae96baf0 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -613,6 +613,7 @@ static int boot_from_devices(struct spl_image_info *spl_image,
 	for (i = 0; i < count && spl_boot_list[i] != BOOT_DEVICE_NONE; i++) {
 		struct spl_image_loader *loader;
 		int bootdev = spl_boot_list[i];
+		int ret;
 
 		if (CONFIG_IS_ENABLED(SHOW_ERRORS))
 			ret = -ENXIO;
@@ -632,10 +633,13 @@ static int boot_from_devices(struct spl_image_info *spl_image,
 					     "Unsupported Boot Device!\n");
 				}
 			}
-			if (loader &&
-				!spl_load_image(spl_image, loader)) {
-				spl_image->boot_device = bootdev;
-				return 0;
+			if (loader) {
+				ret = spl_load_image(spl_image, loader);
+				if (!ret) {
+					spl_image->boot_device = bootdev;
+					return 0;
+				}
+				printf("Error: %d\n", ret);
 			}
 		}
 	}
-- 
2.34.1

[PATCH 10/19] spl: Allow serial to be disabled in any XPL phase

[Simon Glass]

The current check looks only at SPL, but TPL or VPL might have a
different setting. Update the condition.

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

 common/spl/spl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 143ae96baf0..8b868a298f2 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -837,7 +837,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
  */
 void preloader_console_init(void)
 {
-#ifdef CONFIG_SPL_SERIAL
+#if CONFIG_IS_ENABLED(SERIAL)
 	gd->baudrate = CONFIG_BAUDRATE;
 
 	serial_init();		/* serial communications setup */
-- 
2.34.1

[PATCH 11/19] spl: Support a relocated stack in any XPL phase

[Simon Glass]

The current check looks only at SPL, but TPL or VPL might have a
different setting. Update the condition.

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

 common/spl/spl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 8b868a298f2..5c687cfc73b 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -896,7 +896,7 @@ __weak void spl_relocate_stack_check(void)
  */
 ulong spl_relocate_stack_gd(void)
 {
-#ifdef CONFIG_SPL_STACK_R
+#if CONFIG_IS_ENABLED(STACK_R)
 	gd_t *new_gd;
 	ulong ptr = CONFIG_SPL_STACK_R_ADDR;
 
-- 
2.34.1

[PATCH 12/19] spl: Drop use of uintptr_t

[Simon Glass]

U-Boot uses ulong for addresses. It is confusing to use uintptr_t in a
few places, since it makes people wonder if the types are compatible.
Change the few occurences in SPL to use ulong

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

 common/spl/spl_atf.c    | 36 ++++++++++++++++++------------------
 common/spl/spl_fit.c    |  2 +-
 common/spl/spl_legacy.c |  8 ++++----
 include/spl.h           | 28 ++++++++++++++--------------
 4 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c
index 0397b86a33b..8bc5db77395 100644
--- a/common/spl/spl_atf.c
+++ b/common/spl/spl_atf.c
@@ -41,9 +41,9 @@ struct bl2_to_bl31_params_mem_v2 {
 	struct entry_point_info bl31_ep_info;
 };
 
-struct bl31_params *bl2_plat_get_bl31_params_default(uintptr_t bl32_entry,
-						     uintptr_t bl33_entry,
-						     uintptr_t fdt_addr)
+struct bl31_params *bl2_plat_get_bl31_params_default(ulong bl32_entry,
+						     ulong bl33_entry,
+						     ulong fdt_addr)
 {
 	static struct bl2_to_bl31_params_mem bl31_params_mem;
 	struct bl31_params *bl2_to_bl31_params;
@@ -100,17 +100,17 @@ struct bl31_params *bl2_plat_get_bl31_params_default(uintptr_t bl32_entry,
 	return bl2_to_bl31_params;
 }
 
-__weak struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl32_entry,
-						    uintptr_t bl33_entry,
-						    uintptr_t fdt_addr)
+__weak struct bl31_params *bl2_plat_get_bl31_params(ulong bl32_entry,
+						    ulong bl33_entry,
+						    ulong fdt_addr)
 {
 	return bl2_plat_get_bl31_params_default(bl32_entry, bl33_entry,
 						fdt_addr);
 }
 
-struct bl_params *bl2_plat_get_bl31_params_v2_default(uintptr_t bl32_entry,
-						      uintptr_t bl33_entry,
-						      uintptr_t fdt_addr)
+struct bl_params *bl2_plat_get_bl31_params_v2_default(ulong bl32_entry,
+						      ulong bl33_entry,
+						      ulong fdt_addr)
 {
 	static struct bl2_to_bl31_params_mem_v2 bl31_params_mem;
 	struct bl_params *bl_params;
@@ -173,9 +173,9 @@ struct bl_params *bl2_plat_get_bl31_params_v2_default(uintptr_t bl32_entry,
 	return bl_params;
 }
 
-__weak struct bl_params *bl2_plat_get_bl31_params_v2(uintptr_t bl32_entry,
-						     uintptr_t bl33_entry,
-						     uintptr_t fdt_addr)
+__weak struct bl_params *bl2_plat_get_bl31_params_v2(ulong bl32_entry,
+						     ulong bl33_entry,
+						     ulong fdt_addr)
 {
 	return bl2_plat_get_bl31_params_v2_default(bl32_entry, bl33_entry,
 						   fdt_addr);
@@ -188,8 +188,8 @@ static inline void raw_write_daif(unsigned int daif)
 
 typedef void __noreturn (*atf_entry_t)(struct bl31_params *params, void *plat_params);
 
-static void __noreturn bl31_entry(uintptr_t bl31_entry, uintptr_t bl32_entry,
-				  uintptr_t bl33_entry, uintptr_t fdt_addr)
+static void __noreturn bl31_entry(ulong bl31_entry, ulong bl32_entry,
+				  ulong bl33_entry, ulong fdt_addr)
 {
 	atf_entry_t  atf_entry = (atf_entry_t)bl31_entry;
 	void *bl31_params;
@@ -238,7 +238,7 @@ static int spl_fit_images_find(void *blob, int os)
 	return -FDT_ERR_NOTFOUND;
 }
 
-uintptr_t spl_fit_images_get_entry(void *blob, int node)
+ulong spl_fit_images_get_entry(void *blob, int node)
 {
 	ulong  val;
 	int ret;
@@ -253,10 +253,10 @@ uintptr_t spl_fit_images_get_entry(void *blob, int node)
 
 void __noreturn spl_invoke_atf(struct spl_image_info *spl_image)
 {
-	uintptr_t  bl32_entry = 0;
-	uintptr_t  bl33_entry = CONFIG_TEXT_BASE;
+	ulong  bl32_entry = 0;
+	ulong  bl33_entry = CONFIG_TEXT_BASE;
 	void *blob = spl_image->fdt_addr;
-	uintptr_t platform_param = (uintptr_t)blob;
+	ulong platform_param = (ulong)blob;
 	int node;
 
 	/*
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 1ad5a69d807..614dea9ae18 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -862,7 +862,7 @@ int spl_load_fit_image(struct spl_image_info *spl_image,
 {
 	struct bootm_headers images;
 	const char *fit_uname_config = NULL;
-	uintptr_t fdt_hack;
+	ulong fdt_hack;
 	const char *uname;
 	ulong fw_data = 0, dt_data = 0, img_data = 0;
 	ulong fw_len = 0, dt_len = 0, img_len = 0;
diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c
index a77893455f2..3af7ea33ae9 100644
--- a/common/spl/spl_legacy.c
+++ b/common/spl/spl_legacy.c
@@ -16,11 +16,11 @@
 
 #define LZMA_LEN	(1 << 20)
 
-static void spl_parse_legacy_validate(uintptr_t start, uintptr_t size)
+static void spl_parse_legacy_validate(ulong start, ulong size)
 {
-	uintptr_t spl_start = (uintptr_t)_start;
-	uintptr_t spl_end = (uintptr_t)&_image_binary_end;
-	uintptr_t end = start + size;
+	ulong spl_start = (ulong)_start;
+	ulong spl_end = (ulong)&_image_binary_end;
+	ulong end = start + size;
 
 	if ((start >= spl_start && start < spl_end) ||
 	    (end > spl_start && end <= spl_end) ||
diff --git a/include/spl.h b/include/spl.h
index de808ccd413..d90eed956af 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -256,8 +256,8 @@ enum spl_sandbox_flags {
 struct spl_image_info {
 	const char *name;
 	u8 os;
-	uintptr_t load_addr;
-	uintptr_t entry_point;
+	ulong load_addr;
+	ulong entry_point;
 #if CONFIG_IS_ENABLED(LOAD_FIT) || CONFIG_IS_ENABLED(LOAD_FIT_FULL)
 	void *fdt_addr;
 #endif
@@ -939,9 +939,9 @@ void __noreturn spl_invoke_atf(struct spl_image_info *spl_image);
  *
  * Return: bl31 params structure pointer
  */
-struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl32_entry,
-					     uintptr_t bl33_entry,
-					     uintptr_t fdt_addr);
+struct bl31_params *bl2_plat_get_bl31_params(ulong bl32_entry,
+					     ulong bl33_entry,
+					     ulong fdt_addr);
 
 /**
  * bl2_plat_get_bl31_params_default() - prepare params for bl31.
@@ -960,9 +960,9 @@ struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl32_entry,
  *
  * Return: bl31 params structure pointer
  */
-struct bl31_params *bl2_plat_get_bl31_params_default(uintptr_t bl32_entry,
-						     uintptr_t bl33_entry,
-						     uintptr_t fdt_addr);
+struct bl31_params *bl2_plat_get_bl31_params_default(ulong bl32_entry,
+						     ulong bl33_entry,
+						     ulong fdt_addr);
 
 /**
  * bl2_plat_get_bl31_params_v2() - return params for bl31
@@ -976,9 +976,9 @@ struct bl31_params *bl2_plat_get_bl31_params_default(uintptr_t bl32_entry,
  *
  * Return: bl31 params structure pointer
  */
-struct bl_params *bl2_plat_get_bl31_params_v2(uintptr_t bl32_entry,
-					      uintptr_t bl33_entry,
-					      uintptr_t fdt_addr);
+struct bl_params *bl2_plat_get_bl31_params_v2(ulong bl32_entry,
+					      ulong bl33_entry,
+					      ulong fdt_addr);
 
 /**
  * bl2_plat_get_bl31_params_v2_default() - prepare params for bl31.
@@ -995,9 +995,9 @@ struct bl_params *bl2_plat_get_bl31_params_v2(uintptr_t bl32_entry,
  *
  * Return: bl31 params structure pointer
  */
-struct bl_params *bl2_plat_get_bl31_params_v2_default(uintptr_t bl32_entry,
-						      uintptr_t bl33_entry,
-						      uintptr_t fdt_addr);
+struct bl_params *bl2_plat_get_bl31_params_v2_default(ulong bl32_entry,
+						      ulong bl33_entry,
+						      ulong fdt_addr);
 /**
  * spl_optee_entry - entry function for optee
  *
-- 
2.34.1

[PATCH 13/19] spl: Drop a duplicate variable in boot_from_devices()

[Simon Glass]

The variable 'ret' is defined twice, which is not intended. This may
have been a local merge error.

Signed-off-by: Simon Glass <sjg@chromium.org>
Fixes: 2eefeb6d893 ("spl: Report a loader failure")
---

 common/spl/spl.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 5c687cfc73b..d60e60e708e 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -613,7 +613,6 @@ static int boot_from_devices(struct spl_image_info *spl_image,
 	for (i = 0; i < count && spl_boot_list[i] != BOOT_DEVICE_NONE; i++) {
 		struct spl_image_loader *loader;
 		int bootdev = spl_boot_list[i];
-		int ret;
 
 		if (CONFIG_IS_ENABLED(SHOW_ERRORS))
 			ret = -ENXIO;
-- 
2.34.1

[PATCH 14/19] spl: Add some more debugging to load_simple_fit()

[Simon Glass]

Add debugging of image-loading progress. Fix a stale comment in the
function comment while we are here.

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

 common/spl/spl_fit.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 614dea9ae18..303d5416b59 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -190,7 +190,7 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
 /**
  * load_simple_fit(): load the image described in a certain FIT node
  * @info:	points to information about the device to load data from
- * @sector:	the start sector of the FIT image on the device
+ * @fit_offset:	the offset of the FIT image on the device
  * @ctx:	points to the FIT context structure
  * @node:	offset of the DT node describing the image to load (relative
  *		to @fit)
@@ -243,11 +243,14 @@ static int load_simple_fit(struct spl_load_info *info, ulong fit_offset,
 	if (!fit_image_get_data_position(fit, node, &offset)) {
 		external_data = true;
 	} else if (!fit_image_get_data_offset(fit, node, &offset)) {
+		log_debug("read offset %x = offset from fit %lx\n",
+			  offset, (ulong)offset + ctx->ext_data_offset);
 		offset += ctx->ext_data_offset;
 		external_data = true;
 	}
 
 	if (external_data) {
+		ulong read_offset;
 		void *src_ptr;
 
 		/* External data */
@@ -270,6 +273,10 @@ static int load_simple_fit(struct spl_load_info *info, ulong fit_offset,
 
 		overhead = get_aligned_image_overhead(info, offset);
 		size = get_aligned_image_size(info, length, offset);
+		read_offset = fit_offset + get_aligned_image_offset(info,
+							    offset);
+		log_debug("reading from offset %x / %lx size %lx to %p: ",
+			  offset, read_offset, size, src_ptr);
 
 		if (info->read(info,
 			       fit_offset +
@@ -336,6 +343,7 @@ static int load_simple_fit(struct spl_load_info *info, ulong fit_offset,
 		else
 			image_info->entry_point = FDT_ERROR;
 	}
+	log_debug("- done loading\n");
 
 	upl_add_image(fit, node, load_addr, length);
 
-- 
2.34.1

[PATCH 15/19] spl: lib: Allow for decompression in any SPL build

[Simon Glass]

Add Kconfig symbols and update the Makefile rules so that decompression
can be used in TPL and VPL

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

 lib/Kconfig  | 35 +++++++++++++++++++++++++++++++++++
 lib/Makefile |  8 ++++----
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/lib/Kconfig b/lib/Kconfig
index 2059219a120..72a5f3cb04f 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -811,12 +811,36 @@ config SPL_LZ4
 	  fast compression and decompression speed. It belongs to the LZ77
 	  family of byte-oriented compression schemes.
 
+config TPL_LZ4
+	bool "Enable LZ4 decompression support in TPL"
+	depends on TPL
+	help
+	  This enables support for the LZ4 decompression algorithm in TPL. LZ4
+	  is a lossless data compression algorithm that is focused on
+	  fast compression and decompression speed. It belongs to the LZ77
+	  family of byte-oriented compression schemes.
+
+config VPL_LZ4
+	bool "Enable LZ4 decompression support in VPL"
+	depends on VPL
+	help
+	  This enables support for the LZ4 decompression algorithm in VPL. LZ4
+	  is a lossless data compression algorithm that is focused on
+	  fast compression and decompression speed. It belongs to the LZ77
+	  family of byte-oriented compression schemes.
+
 config SPL_LZMA
 	bool "Enable LZMA decompression support for SPL build"
 	depends on SPL
 	help
 	  This enables support for LZMA compression algorithm for SPL boot.
 
+config TPL_LZMA
+	bool "Enable LZMA decompression support for TPL build"
+	depends on TPL
+	help
+	  This enables support for LZMA compression algorithm for TPL boot.
+
 config VPL_LZMA
 	bool "Enable LZMA decompression support for VPL build"
 	default y if LZMA
@@ -835,11 +859,22 @@ config SPL_GZIP
 	help
 	  This enables support for the GZIP compression algorithm for SPL boot.
 
+config TPL_GZIP
+	bool "Enable gzip decompression support for SPL build"
+	select TPL_ZLIB
+	help
+	  This enables support for the GZIP compression algorithm for TPL
+
 config SPL_ZLIB
 	bool
 	help
 	  This enables compression lib for SPL boot.
 
+config TPL_ZLIB
+	bool
+	help
+	  This enables compression lib for TPL
+
 config SPL_ZSTD
 	bool "Enable Zstandard decompression support in SPL"
 	depends on SPL
diff --git a/lib/Makefile b/lib/Makefile
index 81b503ab526..d122f764f9f 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -82,12 +82,12 @@ obj-$(CONFIG_$(SPL_)SHA512) += sha512.o
 obj-$(CONFIG_CRYPT_PW) += crypt/
 obj-$(CONFIG_$(SPL_)ASN1_DECODER) += asn1_decoder.o
 
-obj-$(CONFIG_$(SPL_)ZLIB) += zlib/
+obj-$(CONFIG_$(SPL_TPL_)ZLIB) += zlib/
 obj-$(CONFIG_$(SPL_)ZSTD) += zstd/
-obj-$(CONFIG_$(SPL_)GZIP) += gunzip.o
+obj-$(CONFIG_$(SPL_TPL_)GZIP) += gunzip.o
 obj-$(CONFIG_$(SPL_)LZO) += lzo/
-obj-$(CONFIG_$(SPL_)LZMA) += lzma/
-obj-$(CONFIG_$(SPL_)LZ4) += lz4_wrapper.o
+obj-$(CONFIG_$(SPL_TPL_)LZMA) += lzma/
+obj-$(CONFIG_$(SPL_TPL_)LZ4) += lz4_wrapper.o
 
 obj-$(CONFIG_$(SPL_)LIB_RATIONAL) += rational.o
 
-- 
2.34.1

[PATCH 16/19] boot: Allow use of FIT in TPL and VPL

[Simon Glass]

With VBE we want to use FIT in all phases of the boot. Add Kconfig
options to support this.

Disable the options for sandbox_vpl for now.

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

 boot/Kconfig                  | 69 ++++++++++++++++++++++++++++++++++-
 boot/Makefile                 |  4 +-
 configs/sandbox_vpl_defconfig |  3 +-
 3 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index 7ac34574079..afe04c7327a 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -161,6 +161,18 @@ config SPL_FIT
 	select SPL_HASH
 	select SPL_OF_LIBFDT
 
+config VPL_FIT
+	bool "Support Flattened Image Tree within VPL"
+	depends on VPL
+	select VPL_HASH
+	select VPL_OF_LIBFDT
+
+config TPL_FIT
+	bool "Support Flattened Image Tree within TPL"
+	depends on TPL
+	select TPL_HASH
+	select TPL_OF_LIBFDT
+
 config SPL_FIT_PRINT
 	bool "Support FIT printing within SPL"
 	depends on SPL_FIT
@@ -265,6 +277,28 @@ config SPL_LOAD_FIT_FULL
 	  particular it can handle selecting from multiple device tree
 	  and passing the correct one to U-Boot.
 
+config TPL_LOAD_FIT
+	bool "Enable TPL loading U-Boot as a FIT (basic fitImage features)"
+	depends on TPL
+	select TPL_FIT
+	help
+	  Normally with the SPL framework a legacy image is generated as part
+	  of the build. This contains U-Boot along with information as to
+	  where it should be loaded. This option instead enables generation
+	  of a FIT (Flat Image Tree) which provides more flexibility. In
+	  particular it can handle selecting from multiple device tree
+	  and passing the correct one to U-Boot.
+
+	  This path has the following limitations:
+
+	  1. "loadables" images, other than FDTs, which do not have a "load"
+	     property will not be loaded. This limitation also applies to FPGA
+	     images with the correct "compatible" string.
+	  2. For FPGA images, the supported "compatible" list is in the
+	     doc/uImage.FIT/source_file_format.txt.
+	  3. FDTs are only loaded for images with an "os" property of "u-boot".
+	     "linux" images are also supported with Falcon boot mode.
+
 config SPL_FIT_IMAGE_POST_PROCESS
 	bool "Enable post-processing of FIT artifacts after loading by the SPL"
 	depends on SPL_LOAD_FIT
@@ -312,6 +346,22 @@ config VPL_FIT
 	select VPL_HASH
 	select VPL_OF_LIBFDT
 
+config VPL_LOAD_FIT
+	bool "Enable VPL loading U-Boot as a FIT (basic fitImage features)"
+	select VPL_FIT
+	default y
+
+config VPL_LOAD_FIT_FULL
+	bool "Enable SPL loading U-Boot as a FIT (full fitImage features)"
+	select VPL_FIT
+	help
+	  Normally with the SPL framework a legacy image is generated as part
+	  of the build. This contains U-Boot along with information as to
+	  where it should be loaded. This option instead enables generation
+	  of a FIT (Flat Image Tree) which provides more flexibility. In
+	  particular it can handle selecting from multiple device tree
+	  and passing the correct one to U-Boot.
+
 config VPL_FIT_PRINT
 	bool "Support FIT printing within VPL"
 	depends on VPL_FIT
@@ -632,6 +682,15 @@ config VPL_BOOTMETH_VBE
 	  supports selection of various firmware components, selection of an OS to
 	  boot as well as updating these using fwupd.
 
+config TPL_BOOTMETH_VBE
+	bool "Bootdev support for Verified Boot for Embedded (TPL)"
+	depends on TPL
+	default y
+	help
+	  Enables support for VBE boot. This is a standard boot method which
+	  supports selection of various firmware components, seleciton of an OS to
+	  boot as well as updating these using fwupd.
+
 if BOOTMETH_VBE
 
 config BOOTMETH_VBE_REQUEST
@@ -710,7 +769,15 @@ config VPL_BOOTMETH_VBE_SIMPLE_FW
 	  This option enabled for VPL, since it is the phase where the SPL
 	  decision is made.
 
-endif # BOOTMETH_VBE
+config TPL_BOOTMETH_VBE_SIMPLE_FW
+	bool "Bootdev support for VBE 'simple' method firmware phase (TPL)"
+	depends on VPL
+	default y
+	help
+	  Enables support for the firmware parts of VBE 'simple' boot, in TPL.
+	  TPL loads a FIT containing the VPL binary and a suitable devicetree.
+
+endif # BOOTMETH_VBE_SIMPLE
 
 config EXPO
 	bool "Support for expos - groups of scenes displaying a UI"
diff --git a/boot/Makefile b/boot/Makefile
index f4675d6ffd5..40e2337de0f 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -58,9 +58,7 @@ obj-$(CONFIG_$(SPL_TPL_)FIT_CIPHER) += image-cipher.o
 
 obj-$(CONFIG_CMD_ADTIMG) += image-android-dt.o
 
-ifdef CONFIG_SPL_BUILD
-obj-$(CONFIG_SPL_LOAD_FIT) += common_fit.o
-endif
+obj-$(CONFIG_$(SPL_TPL_)LOAD_FIT) += common_fit.o
 
 obj-$(CONFIG_$(SPL_TPL_)EXPO) += expo.o scene.o expo_build.o
 obj-$(CONFIG_$(SPL_TPL_)EXPO) += scene_menu.o scene_textline.o
diff --git a/configs/sandbox_vpl_defconfig b/configs/sandbox_vpl_defconfig
index 96e9211bd19..d18bb7a0353 100644
--- a/configs/sandbox_vpl_defconfig
+++ b/configs/sandbox_vpl_defconfig
@@ -27,9 +27,10 @@ CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_BEST_MATCH=y
 CONFIG_SPL_LOAD_FIT=y
+# CONFIG_TPL_BOOTMETH_VBE is not set
+# CONFIG_TPL_BOOTMETH_VBE_SIMPLE_FW is not set
 CONFIG_UPL=y
 CONFIG_UPL_IN=y
-CONFIG_SPL_UPL_OUT=y
 CONFIG_BOOTSTAGE=y
 CONFIG_BOOTSTAGE_REPORT=y
 CONFIG_BOOTSTAGE_FDT=y
-- 
2.34.1

[PATCH 17/19] lib: Allow crc8 in TPL and VPL

[Simon Glass]

Provide options to enable the CRC8 feature in TPL and VPL builds.

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

 lib/Kconfig | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/lib/Kconfig b/lib/Kconfig
index 72a5f3cb04f..b2d5e8d04a7 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -698,6 +698,24 @@ config SPL_CRC8
 	  checksum with feedback to produce an 8-bit result. The code is small
 	  and it does not require a lookup table (unlike CRC32).
 
+config TPL_CRC8
+	bool "Support CRC8 in TPL"
+	depends on SPL
+	help
+	  Enables CRC8 support in TPL. This is not normally required. CRC8 is
+	  a simple and fast checksumming algorithm which does a bytewise
+	  checksum with feedback to produce an 8-bit result. The code is small
+	  and it does not require a lookup table (unlike CRC32).
+
+config VPL_CRC8
+	bool "Support CRC8 in VPL"
+	depends on VPL
+	help
+	  Enables CRC8 support in VPL. This is not normally required. CRC8 is
+	  a simple and fast checksumming algorithm which does a bytewise
+	  checksum with feedback to produce an 8-bit result. The code is small
+	  and it does not require a lookup table (unlike CRC32).
+
 config SPL_CRC16
 	bool "Support CRC16 in SPL"
 	depends on SPL
-- 
2.34.1

[PATCH 18/19] boot: Imply CRC8 with VBE

[Simon Glass]

VBE uses a crc8 checksum to verify that the nvdata is valid, so make
sure it is available if VBE is enabled.

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

 boot/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/boot/Kconfig b/boot/Kconfig
index afe04c7327a..49e99e5e155 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -716,6 +716,8 @@ config SPL_BOOTMETH_VBE_REQUEST
 config BOOTMETH_VBE_SIMPLE
 	bool "Bootdev support for VBE 'simple' method"
 	default y
+	imply SPL_CRC8
+	imply VPL_CRC8
 	help
 	  Enables support for VBE 'simple' boot. This allows updating a single
 	  firmware image in boot media such as MMC. It does not support any sort
-- 
2.34.1

[PATCH 19/19] hash: Plumb crc8 into the hash functions

[Simon Glass]

Add an entry for crc8, with watchdog handling.

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

 common/hash.c        | 8 ++++++++
 include/u-boot/crc.h | 3 +++
 lib/crc8.c           | 6 ++++++
 3 files changed, 17 insertions(+)

diff --git a/common/hash.c b/common/hash.c
index ac63803fed9..43c4e185750 100644
--- a/common/hash.c
+++ b/common/hash.c
@@ -304,6 +304,14 @@ static struct hash_algo hash_algo[] = {
 		.hash_update	= hash_update_crc16_ccitt,
 		.hash_finish	= hash_finish_crc16_ccitt,
 	},
+#if CONFIG_IS_ENABLED(CRC8)
+	{
+		.name		= "crc8",
+		.digest_size	= 1,
+		.chunk_size	= CHUNKSZ_CRC32,
+		.hash_func_ws	= crc8_wd_buf,
+	},
+#endif
 #if CONFIG_IS_ENABLED(CRC32)
 	{
 		.name		= "crc32",
diff --git a/include/u-boot/crc.h b/include/u-boot/crc.h
index 5174bd7ac41..b2badaf6a97 100644
--- a/include/u-boot/crc.h
+++ b/include/u-boot/crc.h
@@ -25,6 +25,9 @@
  */
 unsigned int crc8(unsigned int crc_start, const unsigned char *vptr, int len);
 
+void crc8_wd_buf(const unsigned char *input, unsigned int len,
+		 unsigned char output[1], unsigned int chunk_sz);
+
 /* lib/crc16.c - 16 bit CRC with polynomial x^16 + x^15 + x^2 + 1 */
 uint16_t crc16(uint16_t crc, const unsigned char *buffer, size_t len);
 
diff --git a/lib/crc8.c b/lib/crc8.c
index 20d46d16147..811e19917b4 100644
--- a/lib/crc8.c
+++ b/lib/crc8.c
@@ -32,3 +32,9 @@ unsigned int crc8(unsigned int crc, const unsigned char *vptr, int len)
 
 	return crc;
 }
+
+void crc8_wd_buf(const unsigned char *input, unsigned int len,
+		 unsigned char output[1], unsigned int chunk_sz)
+{
+	*output = crc8(0, input, len);
+}
-- 
2.34.1

Re: [PATCH 02/19] serial: ns16550: Allow clocks to be missing

[Tom Rini]

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

On Thu, Aug 29, 2024 at 08:57:45AM -0600, Simon Glass wrote:
> Allow serial init when clock support is not enabled in a particular phase.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  drivers/serial/ns16550.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index 6fcb5b523ac..5cda273623d 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -570,7 +570,7 @@ int ns16550_serial_of_to_plat(struct udevice *dev)
>  		err = clk_get_rate(&clk);
>  		if (!IS_ERR_VALUE(err))
>  			plat->clock = err;
> -	} else if (err != -ENOENT && err != -ENODEV && err != -ENOSYS) {
> +	} else if (err != -ENOENT && err != -ENODEV && err != -ENOSYS && err != -EALREADY) {
>  		debug("ns16550 failed to get clock\n");
>  		return err;
>  	}

Your patch and
https://patchwork.ozlabs.org/project/uboot/patch/20240804150955.3521296-1-jonas@kwiboo.se/
would seem to be in conflict and I'm not sure what's the best thing
moving forward.

-- 
Tom

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

Re: [PATCH 06/19] malloc: Show amount of used space when memory runs out

[Tom Rini]

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

On Thu, Aug 29, 2024 at 08:57:49AM -0600, Simon Glass wrote:
> Show a bit more information when malloc() space is exhausted and
> debugging is enabled.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/malloc_simple.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/common/malloc_simple.c b/common/malloc_simple.c
> index 5a8ec538f8f..f2b3dc53689 100644
> --- a/common/malloc_simple.c
> +++ b/common/malloc_simple.c
> @@ -26,7 +26,8 @@ static void *alloc_simple(size_t bytes, int align)
>  	log_debug("size=%lx, ptr=%lx, limit=%x: ", (ulong)bytes, new_ptr,
>  		  gd->malloc_limit);
>  	if (new_ptr > gd->malloc_limit) {
> -		log_err("alloc space exhausted\n");
> +		log_err("alloc space exhausted %lx %x\n", new_ptr,
> +			gd->malloc_limit);
>  		return NULL;
>  	}

Since debugging is enabled you should update the error message to say
what the values mean...

-- 
Tom

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

Re: [PATCH 17/19] lib: Allow crc8 in TPL and VPL

[Tom Rini]

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

On Thu, Aug 29, 2024 at 08:58:00AM -0600, Simon Glass wrote:
> Provide options to enable the CRC8 feature in TPL and VPL builds.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  lib/Kconfig | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 72a5f3cb04f..b2d5e8d04a7 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -698,6 +698,24 @@ config SPL_CRC8
>  	  checksum with feedback to produce an 8-bit result. The code is small
>  	  and it does not require a lookup table (unlike CRC32).
>  
> +config TPL_CRC8
> +	bool "Support CRC8 in TPL"
> +	depends on SPL

Should be TPL not SPL here I assume.

-- 
Tom

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

Re: [PATCH 18/19] boot: Imply CRC8 with VBE

[Tom Rini]

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

On Thu, Aug 29, 2024 at 08:58:01AM -0600, Simon Glass wrote:
> VBE uses a crc8 checksum to verify that the nvdata is valid, so make
> sure it is available if VBE is enabled.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  boot/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/boot/Kconfig b/boot/Kconfig
> index afe04c7327a..49e99e5e155 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -716,6 +716,8 @@ config SPL_BOOTMETH_VBE_REQUEST
>  config BOOTMETH_VBE_SIMPLE
>  	bool "Bootdev support for VBE 'simple' method"
>  	default y
> +	imply SPL_CRC8
> +	imply VPL_CRC8
>  	help
>  	  Enables support for VBE 'simple' boot. This allows updating a single
>  	  firmware image in boot media such as MMC. It does not support any sort

I really worry the lack of "if SPL" / "if VPL" here leads to Kconfig
warnings that CI doesn't pick up.

-- 
Tom

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

Re: [PATCH 00/19] vbe: Series part E

[Tom Rini]

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

On Thu, Aug 29, 2024 at 08:57:43AM -0600, Simon Glass wrote:

> This includes various patches towards implementing the VBE abrec
> bootmeth in U-Boot. It mostly focuses on SPL tweaks and adjusting what
> fatures are available in VPL.

A lot of this series makes me really want to stop and rework how we
configure / build things. Having say ZLIB, SPL_ZLIB, TPL_ZLIB and
VPL_ZLIB really isn't good.

-- 
Tom

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

Re: [PATCH 00/19] vbe: Series part E

[Simon Glass]

Hi Tom,

On Thu, 29 Aug 2024 at 12:33, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Aug 29, 2024 at 08:57:43AM -0600, Simon Glass wrote:
>
> > This includes various patches towards implementing the VBE abrec
> > bootmeth in U-Boot. It mostly focuses on SPL tweaks and adjusting what
> > fatures are available in VPL.
>
> A lot of this series makes me really want to stop and rework how we
> configure / build things. Having say ZLIB, SPL_ZLIB, TPL_ZLIB and
> VPL_ZLIB really isn't good.

Yup and I had a solution or that a year or two ago and it died.

Any revival of that, or new solution will have to wait until VBE abrec
is done. I hope there is only one more series after this, then a small
one for the VBE thing itself.

Regards,
Simon

Re: [PATCH 19/19] hash: Plumb crc8 into the hash functions

[Peter Robinson]

On Thu, 29 Aug 2024 at 16:02, Simon Glass <sjg@chromium.org> wrote:
>
> Add an entry for crc8, with watchdog handling.

What's the watchdog handling do? What's this used for?

Looking at the use of crc8 it's minimally used.

> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  common/hash.c        | 8 ++++++++
>  include/u-boot/crc.h | 3 +++
>  lib/crc8.c           | 6 ++++++
>  3 files changed, 17 insertions(+)
>
> diff --git a/common/hash.c b/common/hash.c
> index ac63803fed9..43c4e185750 100644
> --- a/common/hash.c
> +++ b/common/hash.c
> @@ -304,6 +304,14 @@ static struct hash_algo hash_algo[] = {
>                 .hash_update    = hash_update_crc16_ccitt,
>                 .hash_finish    = hash_finish_crc16_ccitt,
>         },
> +#if CONFIG_IS_ENABLED(CRC8)
> +       {
> +               .name           = "crc8",
> +               .digest_size    = 1,
> +               .chunk_size     = CHUNKSZ_CRC32,
> +               .hash_func_ws   = crc8_wd_buf,
> +       },
> +#endif
>  #if CONFIG_IS_ENABLED(CRC32)
>         {
>                 .name           = "crc32",
> diff --git a/include/u-boot/crc.h b/include/u-boot/crc.h
> index 5174bd7ac41..b2badaf6a97 100644
> --- a/include/u-boot/crc.h
> +++ b/include/u-boot/crc.h
> @@ -25,6 +25,9 @@
>   */
>  unsigned int crc8(unsigned int crc_start, const unsigned char *vptr, int len);
>
> +void crc8_wd_buf(const unsigned char *input, unsigned int len,
> +                unsigned char output[1], unsigned int chunk_sz);
> +
>  /* lib/crc16.c - 16 bit CRC with polynomial x^16 + x^15 + x^2 + 1 */
>  uint16_t crc16(uint16_t crc, const unsigned char *buffer, size_t len);
>
> diff --git a/lib/crc8.c b/lib/crc8.c
> index 20d46d16147..811e19917b4 100644
> --- a/lib/crc8.c
> +++ b/lib/crc8.c
> @@ -32,3 +32,9 @@ unsigned int crc8(unsigned int crc, const unsigned char *vptr, int len)
>
>         return crc;
>  }
> +
> +void crc8_wd_buf(const unsigned char *input, unsigned int len,
> +                unsigned char output[1], unsigned int chunk_sz)
> +{
> +       *output = crc8(0, input, len);
> +}
> --
> 2.34.1
>

Re: [PATCH 00/19] vbe: Series part E

[Caleb Connolly]

Hi Simon,

On 29/08/2024 15:57, Simon Glass wrote:
> 
> This includes various patches towards implementing the VBE abrec
> bootmeth in U-Boot. It mostly focuses on SPL tweaks and adjusting what
> fatures are available in VPL.

It would be helpful if you could include some context as to what VBE is, 
how this series fits in to previous ones, etc... If this is part of some 
overarching changes then having some common background justification / 
context which is copy-pasted to all the relevant patch series would make 
sense imo.

Thanks and regards,
> 
> 
> Simon Glass (19):
>    image: Add a prototype for fit_image_get_phase()
>    serial: ns16550: Allow clocks to be missing
>    boot: Allow FIT to fall back from best-match option
>    bootstd: Avoid sprintf() in SPL when creating bootdevs
>    boot: Respect the load_op in fit_image_load()
>    malloc: Show amount of used space when memory runs out
>    malloc: Provide a simple malloc for VPL
>    Support setting a maximum size for the VPL image
>    spl: Report a loader failure
>    spl: Allow serial to be disabled in any XPL phase
>    spl: Support a relocated stack in any XPL phase
>    spl: Drop use of uintptr_t
>    spl: Drop a duplicate variable in boot_from_devices()
>    spl: Add some more debugging to load_simple_fit()
>    spl: lib: Allow for decompression in any SPL build
>    boot: Allow use of FIT in TPL and VPL
>    lib: Allow crc8 in TPL and VPL
>    boot: Imply CRC8 with VBE
>    hash: Plumb crc8 into the hash functions
> 
>   boot/Kconfig                  | 71 ++++++++++++++++++++++++++++++++++-
>   boot/Makefile                 |  4 +-
>   boot/bootdev-uclass.c         | 10 ++++-
>   boot/image-fit.c              | 29 ++++++++------
>   common/hash.c                 |  8 ++++
>   common/malloc_simple.c        |  3 +-
>   common/spl/Kconfig.vpl        | 17 +++++++++
>   common/spl/spl.c              | 15 +++++---
>   common/spl/spl_atf.c          | 36 +++++++++---------
>   common/spl/spl_fit.c          | 12 +++++-
>   common/spl/spl_legacy.c       |  8 ++--
>   configs/sandbox_vpl_defconfig |  3 +-
>   drivers/serial/ns16550.c      |  2 +-
>   include/image.h               | 16 +++++++-
>   include/spl.h                 | 28 +++++++-------
>   include/u-boot/crc.h          |  3 ++
>   lib/Kconfig                   | 53 ++++++++++++++++++++++++++
>   lib/Makefile                  |  8 ++--
>   lib/crc8.c                    |  6 +++
>   19 files changed, 263 insertions(+), 69 deletions(-)
> 

-- 
// Caleb (they/them)

Re: [PATCH 19/19] hash: Plumb crc8 into the hash functions

[Simon Glass]

Hi Peter,

On Fri, 30 Aug 2024 at 06:17, Peter Robinson <pbrobinson@gmail.com> wrote:
>
> On Thu, 29 Aug 2024 at 16:02, Simon Glass <sjg@chromium.org> wrote:
> >
> > Add an entry for crc8, with watchdog handling.
>
> What's the watchdog handling do? What's this used for?

It does the hashing in chunks and touches the watchdog after every
chunk, to avoid the board resetting.

>
> Looking at the use of crc8 it's minimally used.

Yes, it's a cheap algorithm which provides some protection, so I am
using it in the VBE relocating loader (to come).

>
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  common/hash.c        | 8 ++++++++
> >  include/u-boot/crc.h | 3 +++
> >  lib/crc8.c           | 6 ++++++
> >  3 files changed, 17 insertions(+)
> >
> > diff --git a/common/hash.c b/common/hash.c
> > index ac63803fed9..43c4e185750 100644
> > --- a/common/hash.c
> > +++ b/common/hash.c
> > @@ -304,6 +304,14 @@ static struct hash_algo hash_algo[] = {
> >                 .hash_update    = hash_update_crc16_ccitt,
> >                 .hash_finish    = hash_finish_crc16_ccitt,
> >         },
> > +#if CONFIG_IS_ENABLED(CRC8)
> > +       {
> > +               .name           = "crc8",
> > +               .digest_size    = 1,
> > +               .chunk_size     = CHUNKSZ_CRC32,
> > +               .hash_func_ws   = crc8_wd_buf,
> > +       },
> > +#endif
> >  #if CONFIG_IS_ENABLED(CRC32)
> >         {
> >                 .name           = "crc32",
> > diff --git a/include/u-boot/crc.h b/include/u-boot/crc.h
> > index 5174bd7ac41..b2badaf6a97 100644
> > --- a/include/u-boot/crc.h
> > +++ b/include/u-boot/crc.h
> > @@ -25,6 +25,9 @@
> >   */
> >  unsigned int crc8(unsigned int crc_start, const unsigned char *vptr, int len);
> >
> > +void crc8_wd_buf(const unsigned char *input, unsigned int len,
> > +                unsigned char output[1], unsigned int chunk_sz);
> > +
> >  /* lib/crc16.c - 16 bit CRC with polynomial x^16 + x^15 + x^2 + 1 */
> >  uint16_t crc16(uint16_t crc, const unsigned char *buffer, size_t len);
> >
> > diff --git a/lib/crc8.c b/lib/crc8.c
> > index 20d46d16147..811e19917b4 100644
> > --- a/lib/crc8.c
> > +++ b/lib/crc8.c
> > @@ -32,3 +32,9 @@ unsigned int crc8(unsigned int crc, const unsigned char *vptr, int len)
> >
> >         return crc;
> >  }
> > +
> > +void crc8_wd_buf(const unsigned char *input, unsigned int len,
> > +                unsigned char output[1], unsigned int chunk_sz)
> > +{
> > +       *output = crc8(0, input, len);
> > +}
> > --
> > 2.34.1
> >

Regards,
Simon

Re: [PATCH 03/19] boot: Allow FIT to fall back from best-match option

[Tom Rini]

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

On Thu, Aug 29, 2024 at 08:57:46AM -0600, Simon Glass wrote:
> When the best-match feature fails to find something, use the provided
> config name as a fallback. The allows SPL to select a suitable config
> when best-match is enabled.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  boot/image-fit.c | 19 ++++++++++---------
>  include/image.h  |  4 +++-
>  2 files changed, 13 insertions(+), 10 deletions(-)

Don't we need a documentation update? If no, because this mechanism
isn't well documented, can you please follow up with that? Thanks.

-- 
Tom

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

Re: [PATCH 05/19] boot: Respect the load_op in fit_image_load()

[Tom Rini]

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

On Thu, Aug 29, 2024 at 08:57:48AM -0600, Simon Glass wrote:

> Some code has crept in which ignores this parameter. Fix this and add a
> little debugging.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Can you please add a Fixes tag, as that will help in reviewing this too,
to ensure we aren't breaking the case that initially changed things
here, thanks.

-- 
Tom

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

Re: [PATCH 00/19] vbe: Series part E

[Simon Glass]

Hi Caleb,

On Fri, 30 Aug 2024 at 14:18, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> Hi Simon,
>
> On 29/08/2024 15:57, Simon Glass wrote:
> >
> > This includes various patches towards implementing the VBE abrec
> > bootmeth in U-Boot. It mostly focuses on SPL tweaks and adjusting what
> > fatures are available in VPL.
>
> It would be helpful if you could include some context as to what VBE is,
> how this series fits in to previous ones, etc... If this is part of some
> overarching changes then having some common background justification /
> context which is copy-pasted to all the relevant patch series would make
> sense imo.

Yes, please see [1]. It is essentially an effort to replace UEFI with
something simpler, more efficient, better-specified and easier to
test.

Regards,
Simon

>
> Thanks and regards,
> >
> >
> > Simon Glass (19):
> >    image: Add a prototype for fit_image_get_phase()
> >    serial: ns16550: Allow clocks to be missing
> >    boot: Allow FIT to fall back from best-match option
> >    bootstd: Avoid sprintf() in SPL when creating bootdevs
> >    boot: Respect the load_op in fit_image_load()
> >    malloc: Show amount of used space when memory runs out
> >    malloc: Provide a simple malloc for VPL
> >    Support setting a maximum size for the VPL image
> >    spl: Report a loader failure
> >    spl: Allow serial to be disabled in any XPL phase
> >    spl: Support a relocated stack in any XPL phase
> >    spl: Drop use of uintptr_t
> >    spl: Drop a duplicate variable in boot_from_devices()
> >    spl: Add some more debugging to load_simple_fit()
> >    spl: lib: Allow for decompression in any SPL build
> >    boot: Allow use of FIT in TPL and VPL
> >    lib: Allow crc8 in TPL and VPL
> >    boot: Imply CRC8 with VBE
> >    hash: Plumb crc8 into the hash functions
> >
> >   boot/Kconfig                  | 71 ++++++++++++++++++++++++++++++++++-
> >   boot/Makefile                 |  4 +-
> >   boot/bootdev-uclass.c         | 10 ++++-
> >   boot/image-fit.c              | 29 ++++++++------
> >   common/hash.c                 |  8 ++++
> >   common/malloc_simple.c        |  3 +-
> >   common/spl/Kconfig.vpl        | 17 +++++++++
> >   common/spl/spl.c              | 15 +++++---
> >   common/spl/spl_atf.c          | 36 +++++++++---------
> >   common/spl/spl_fit.c          | 12 +++++-
> >   common/spl/spl_legacy.c       |  8 ++--
> >   configs/sandbox_vpl_defconfig |  3 +-
> >   drivers/serial/ns16550.c      |  2 +-
> >   include/image.h               | 16 +++++++-
> >   include/spl.h                 | 28 +++++++-------
> >   include/u-boot/crc.h          |  3 ++
> >   lib/Kconfig                   | 53 ++++++++++++++++++++++++++
> >   lib/Makefile                  |  8 ++--
> >   lib/crc8.c                    |  6 +++
> >   19 files changed, 263 insertions(+), 69 deletions(-)
> >
>
> --
> // Caleb (they/them)

[1] https://docs.u-boot.org/en/latest/develop/vbe.html

Re: [PATCH 02/19] serial: ns16550: Allow clocks to be missing

[Simon Glass]

Hi Tom,

On Thu, 29 Aug 2024 at 19:24, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Aug 29, 2024 at 08:57:45AM -0600, Simon Glass wrote:
> > Allow serial init when clock support is not enabled in a particular phase.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  drivers/serial/ns16550.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> > index 6fcb5b523ac..5cda273623d 100644
> > --- a/drivers/serial/ns16550.c
> > +++ b/drivers/serial/ns16550.c
> > @@ -570,7 +570,7 @@ int ns16550_serial_of_to_plat(struct udevice *dev)
> >               err = clk_get_rate(&clk);
> >               if (!IS_ERR_VALUE(err))
> >                       plat->clock = err;
> > -     } else if (err != -ENOENT && err != -ENODEV && err != -ENOSYS) {
> > +     } else if (err != -ENOENT && err != -ENODEV && err != -ENOSYS && err != -EALREADY) {
> >               debug("ns16550 failed to get clock\n");
> >               return err;
> >       }
>
> Your patch and
> https://patchwork.ozlabs.org/project/uboot/patch/20240804150955.3521296-1-jonas@kwiboo.se/
> would seem to be in conflict and I'm not sure what's the best thing
> moving forward.

That patch seems good to me. For this one, I will drop it from the
next version. The existing conditions should be enough for the case I
have (clocks in the previous phase but not in this one) so long as
-ENOSYS is returned.

Regards,
Simon

Re: [PATCH 06/19] malloc: Show amount of used space when memory runs out

[Simon Glass]

Hi Tom,

On Thu, 29 Aug 2024 at 19:26, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Aug 29, 2024 at 08:57:49AM -0600, Simon Glass wrote:
> > Show a bit more information when malloc() space is exhausted and
> > debugging is enabled.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  common/malloc_simple.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/common/malloc_simple.c b/common/malloc_simple.c
> > index 5a8ec538f8f..f2b3dc53689 100644
> > --- a/common/malloc_simple.c
> > +++ b/common/malloc_simple.c
> > @@ -26,7 +26,8 @@ static void *alloc_simple(size_t bytes, int align)
> >       log_debug("size=%lx, ptr=%lx, limit=%x: ", (ulong)bytes, new_ptr,
> >                 gd->malloc_limit);
> >       if (new_ptr > gd->malloc_limit) {
> > -             log_err("alloc space exhausted\n");
> > +             log_err("alloc space exhausted %lx %x\n", new_ptr,
> > +                     gd->malloc_limit);
> >               return NULL;
> >       }
>
> Since debugging is enabled you should update the error message to say
> what the values mean...

OK, will do. This message is shown without debugging enabled, so I do
want to keep it short.

Regards,
Simon

Re: [PATCH 18/19] boot: Imply CRC8 with VBE

[Simon Glass]

Hi Tom,

On Thu, 29 Aug 2024 at 19:31, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Aug 29, 2024 at 08:58:01AM -0600, Simon Glass wrote:
> > VBE uses a crc8 checksum to verify that the nvdata is valid, so make
> > sure it is available if VBE is enabled.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  boot/Kconfig | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/boot/Kconfig b/boot/Kconfig
> > index afe04c7327a..49e99e5e155 100644
> > --- a/boot/Kconfig
> > +++ b/boot/Kconfig
> > @@ -716,6 +716,8 @@ config SPL_BOOTMETH_VBE_REQUEST
> >  config BOOTMETH_VBE_SIMPLE
> >       bool "Bootdev support for VBE 'simple' method"
> >       default y
> > +     imply SPL_CRC8
> > +     imply VPL_CRC8
> >       help
> >         Enables support for VBE 'simple' boot. This allows updating a single
> >         firmware image in boot media such as MMC. It does not support any sort
>
> I really worry the lack of "if SPL" / "if VPL" here leads to Kconfig
> warnings that CI doesn't pick up.

So far, BOOTMETH_VBE requires SPL and VPL, but I've added the
condition just to be safe.

Regards,
Simon

Re: [PATCH 06/19] malloc: Show amount of used space when memory runs out

[Tom Rini]

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

On Fri, Sep 20, 2024 at 09:25:44AM +0200, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 29 Aug 2024 at 19:26, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Aug 29, 2024 at 08:57:49AM -0600, Simon Glass wrote:
> > > Show a bit more information when malloc() space is exhausted and
> > > debugging is enabled.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > >  common/malloc_simple.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/common/malloc_simple.c b/common/malloc_simple.c
> > > index 5a8ec538f8f..f2b3dc53689 100644
> > > --- a/common/malloc_simple.c
> > > +++ b/common/malloc_simple.c
> > > @@ -26,7 +26,8 @@ static void *alloc_simple(size_t bytes, int align)
> > >       log_debug("size=%lx, ptr=%lx, limit=%x: ", (ulong)bytes, new_ptr,
> > >                 gd->malloc_limit);
> > >       if (new_ptr > gd->malloc_limit) {
> > > -             log_err("alloc space exhausted\n");
> > > +             log_err("alloc space exhausted %lx %x\n", new_ptr,
> > > +                     gd->malloc_limit);
> > >               return NULL;
> > >       }
> >
> > Since debugging is enabled you should update the error message to say
> > what the values mean...
> 
> OK, will do. This message is shown without debugging enabled, so I do
> want to keep it short.

The commit message says "and debugging is enabled", so is that the case
or not?

-- 
Tom

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

Re: [PATCH 03/19] boot: Allow FIT to fall back from best-match option

[Simon Glass]

Hi Tom,

On Thu, 5 Sept 2024 at 00:25, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Aug 29, 2024 at 08:57:46AM -0600, Simon Glass wrote:
> > When the best-match feature fails to find something, use the provided
> > config name as a fallback. The allows SPL to select a suitable config
> > when best-match is enabled.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  boot/image-fit.c | 19 ++++++++++---------
> >  include/image.h  |  4 +++-
> >  2 files changed, 13 insertions(+), 10 deletions(-)
>
> Don't we need a documentation update? If no, because this mechanism
> isn't well documented, can you please follow up with that? Thanks.

CONFIG_FIT_BEST_MATCH is mentioned in the spec [1]

But re the fallback, I'm not sure where that should be documented.
Perhaps add a note at [2]?

Regards,
Simon

[1] https://fitspec.osfw.foundation/
[2] https://docs.u-boot.org/en/latest/usage/fit/howto.html

Re: [PATCH 06/19] malloc: Show amount of used space when memory runs out

[Simon Glass]

Hi Tom,

On Fri, 20 Sept 2024 at 16:59, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Sep 20, 2024 at 09:25:44AM +0200, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 29 Aug 2024 at 19:26, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Aug 29, 2024 at 08:57:49AM -0600, Simon Glass wrote:
> > > > Show a bit more information when malloc() space is exhausted and
> > > > debugging is enabled.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > >
> > > >  common/malloc_simple.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/common/malloc_simple.c b/common/malloc_simple.c
> > > > index 5a8ec538f8f..f2b3dc53689 100644
> > > > --- a/common/malloc_simple.c
> > > > +++ b/common/malloc_simple.c
> > > > @@ -26,7 +26,8 @@ static void *alloc_simple(size_t bytes, int align)
> > > >       log_debug("size=%lx, ptr=%lx, limit=%x: ", (ulong)bytes, new_ptr,
> > > >                 gd->malloc_limit);
> > > >       if (new_ptr > gd->malloc_limit) {
> > > > -             log_err("alloc space exhausted\n");
> > > > +             log_err("alloc space exhausted %lx %x\n", new_ptr,
> > > > +                     gd->malloc_limit);
> > > >               return NULL;
> > > >       }
> > >
> > > Since debugging is enabled you should update the error message to say
> > > what the values mean...
> >
> > OK, will do. This message is shown without debugging enabled, so I do
> > want to keep it short.
>
> The commit message says "and debugging is enabled", so is that the case
> or not?

Oh, I see. It should say 'not' :-) I will send a new patch.


- Simon

Re: [PATCH 03/19] boot: Allow FIT to fall back from best-match option

[Tom Rini]

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

On Fri, Sep 20, 2024 at 05:59:00PM +0200, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 5 Sept 2024 at 00:25, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Aug 29, 2024 at 08:57:46AM -0600, Simon Glass wrote:
> > > When the best-match feature fails to find something, use the provided
> > > config name as a fallback. The allows SPL to select a suitable config
> > > when best-match is enabled.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > >  boot/image-fit.c | 19 ++++++++++---------
> > >  include/image.h  |  4 +++-
> > >  2 files changed, 13 insertions(+), 10 deletions(-)
> >
> > Don't we need a documentation update? If no, because this mechanism
> > isn't well documented, can you please follow up with that? Thanks.
> 
> CONFIG_FIT_BEST_MATCH is mentioned in the spec [1]
> 
> But re the fallback, I'm not sure where that should be documented.
> Perhaps add a note at [2]?

Should we be using the mechanism that I'd swear the documentation build
process has, to fetch an external document? I think we would have a
better user/developer experience if our documentation explained how to
use a feature, as that's where people will tend to look first.

-- 
Tom

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

Re: [PATCH 06/19] malloc: Show amount of used space when memory runs out

[Tom Rini]

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

On Fri, Sep 20, 2024 at 06:04:01PM +0200, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, 20 Sept 2024 at 16:59, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Sep 20, 2024 at 09:25:44AM +0200, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 29 Aug 2024 at 19:26, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Aug 29, 2024 at 08:57:49AM -0600, Simon Glass wrote:
> > > > > Show a bit more information when malloc() space is exhausted and
> > > > > debugging is enabled.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > >
> > > > >  common/malloc_simple.c | 3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/common/malloc_simple.c b/common/malloc_simple.c
> > > > > index 5a8ec538f8f..f2b3dc53689 100644
> > > > > --- a/common/malloc_simple.c
> > > > > +++ b/common/malloc_simple.c
> > > > > @@ -26,7 +26,8 @@ static void *alloc_simple(size_t bytes, int align)
> > > > >       log_debug("size=%lx, ptr=%lx, limit=%x: ", (ulong)bytes, new_ptr,
> > > > >                 gd->malloc_limit);
> > > > >       if (new_ptr > gd->malloc_limit) {
> > > > > -             log_err("alloc space exhausted\n");
> > > > > +             log_err("alloc space exhausted %lx %x\n", new_ptr,
> > > > > +                     gd->malloc_limit);
> > > > >               return NULL;
> > > > >       }
> > > >
> > > > Since debugging is enabled you should update the error message to say
> > > > what the values mean...
> > >
> > > OK, will do. This message is shown without debugging enabled, so I do
> > > want to keep it short.
> >
> > The commit message says "and debugging is enabled", so is that the case
> > or not?
> 
> Oh, I see. It should say 'not' :-) I will send a new patch.

Thanks!

-- 
Tom

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

Re: [PATCH 06/19] malloc: Show amount of used space when memory runs out

[Simon Glass]

Hi Tom,

On Fri, 20 Sept 2024 at 18:35, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Sep 20, 2024 at 06:04:01PM +0200, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 20 Sept 2024 at 16:59, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Fri, Sep 20, 2024 at 09:25:44AM +0200, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Thu, 29 Aug 2024 at 19:26, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Thu, Aug 29, 2024 at 08:57:49AM -0600, Simon Glass wrote:
> > > > > > Show a bit more information when malloc() space is exhausted and
> > > > > > debugging is enabled.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > ---
> > > > > >
> > > > > >  common/malloc_simple.c | 3 ++-
> > > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/common/malloc_simple.c b/common/malloc_simple.c
> > > > > > index 5a8ec538f8f..f2b3dc53689 100644
> > > > > > --- a/common/malloc_simple.c
> > > > > > +++ b/common/malloc_simple.c
> > > > > > @@ -26,7 +26,8 @@ static void *alloc_simple(size_t bytes, int align)
> > > > > >       log_debug("size=%lx, ptr=%lx, limit=%x: ", (ulong)bytes, new_ptr,
> > > > > >                 gd->malloc_limit);
> > > > > >       if (new_ptr > gd->malloc_limit) {
> > > > > > -             log_err("alloc space exhausted\n");
> > > > > > +             log_err("alloc space exhausted %lx %x\n", new_ptr,
> > > > > > +                     gd->malloc_limit);
> > > > > >               return NULL;
> > > > > >       }
> > > > >
> > > > > Since debugging is enabled you should update the error message to say
> > > > > what the values mean...
> > > >
> > > > OK, will do. This message is shown without debugging enabled, so I do
> > > > want to keep it short.
> > >
> > > The commit message says "and debugging is enabled", so is that the case
> > > or not?
> >
> > Oh, I see. It should say 'not' :-) I will send a new patch.
>
> Thanks!

OK done, it is here:

https://patchwork.ozlabs.org/project/uboot/patch/20240921113450.319072-6-sjg@chromium.org/

Regards,
Simon