diff mbox series

[v6,18/25] spl: Convert nand to spl_load

Message ID 20231106022603.3405551-19-seanga2@gmail.com
State Superseded
Delegated to: Tom Rini
Headers show
Series spl: Use common function for loading/parsing images | expand

Commit Message

Sean Anderson Nov. 6, 2023, 2:25 a.m. UTC
This converts the nand load method to use spl_load. nand_page_size may not
be valid until after nand_spl_load_image is called (see e.g. fsl_ifc_spl),
so we set bl_len in spl_nand_read. Since spl_load reads the header for us,
we can remove that argument from spl_nand_load_element.

There are two possible regressions which could result from this commit.
First, we ask for a negative address from spl_get_load_buffer. That is,
instead of

	header = spl_get_load_buffer(0, sizeof(*header));

we do

	header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));

this could cause a problem if spl_get_load_buffer does not return valid
memory for negative offsets. Second, we now set bl_len for legacy images.
This can cause memory up to a bl_len - 1 before the image load address to
be written, which might not have been the case before. If this turns out to
be a problem, we can add an option for a bounce buffer.

We can't load FITs with external data with SPL_LOAD_FIT_FULL, so disable the
test in that case. No boards enable SPL_NAND_SUPPORT and SPL_LOAD_FIT_FULL, so
this is not a regression.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

Changes in v6:
- New

 common/spl/spl_nand.c      | 70 +++++++++-----------------------------
 include/spl_load.h         |  1 +
 test/image/spl_load_nand.c |  2 ++
 3 files changed, 19 insertions(+), 54 deletions(-)

Comments

Simon Glass Nov. 8, 2023, 4:24 a.m. UTC | #1
On Sun, 5 Nov 2023 at 19:26, Sean Anderson <seanga2@gmail.com> wrote:
>
> This converts the nand load method to use spl_load. nand_page_size may not
> be valid until after nand_spl_load_image is called (see e.g. fsl_ifc_spl),
> so we set bl_len in spl_nand_read. Since spl_load reads the header for us,
> we can remove that argument from spl_nand_load_element.
>
> There are two possible regressions which could result from this commit.
> First, we ask for a negative address from spl_get_load_buffer. That is,
> instead of
>
>         header = spl_get_load_buffer(0, sizeof(*header));
>
> we do
>
>         header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));
>
> this could cause a problem if spl_get_load_buffer does not return valid
> memory for negative offsets. Second, we now set bl_len for legacy images.
> This can cause memory up to a bl_len - 1 before the image load address to
> be written, which might not have been the case before. If this turns out to
> be a problem, we can add an option for a bounce buffer.
>
> We can't load FITs with external data with SPL_LOAD_FIT_FULL, so disable the
> test in that case. No boards enable SPL_NAND_SUPPORT and SPL_LOAD_FIT_FULL, so
> this is not a regression.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
> Changes in v6:
> - New
>
>  common/spl/spl_nand.c      | 70 +++++++++-----------------------------
>  include/spl_load.h         |  1 +
>  test/image/spl_load_nand.c |  2 ++
>  3 files changed, 19 insertions(+), 54 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>
diff mbox series

Patch

diff --git a/common/spl/spl_nand.c b/common/spl/spl_nand.c
index 91fa7674f17..3b0a1524238 100644
--- a/common/spl/spl_nand.c
+++ b/common/spl/spl_nand.c
@@ -10,6 +10,7 @@ 
 #include <imx_container.h>
 #include <log.h>
 #include <spl.h>
+#include <spl_load.h>
 #include <asm/io.h>
 #include <mapmem.h>
 #include <nand.h>
@@ -59,6 +60,7 @@  static ulong spl_nand_read(struct spl_load_info *load, ulong offs, ulong size,
 	sector = *(int *)load->priv;
 	offs = sector + nand_spl_adjust_offset(sector, offs - sector);
 	err = nand_spl_load_image(offs, size, dst);
+	spl_set_bl_len(load, nand_page_size());
 	if (err)
 		return 0;
 
@@ -66,60 +68,20 @@  static ulong spl_nand_read(struct spl_load_info *load, ulong offs, ulong size,
 }
 
 static int spl_nand_load_element(struct spl_image_info *spl_image,
-				 struct spl_boot_device *bootdev,
-				 int offset, struct legacy_img_hdr *header)
+				 struct spl_boot_device *bootdev, int offset)
 {
-	int bl_len;
-	int err;
+	struct spl_load_info load;
 
-	err = nand_spl_load_image(offset, sizeof(*header), (void *)header);
-	if (err)
-		return err;
-
-	bl_len = nand_page_size();
-	if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
-	    image_get_magic(header) == FDT_MAGIC) {
-		struct spl_load_info load;
-
-		debug("Found FIT\n");
-		load.priv = &offset;
-		spl_set_bl_len(&load, bl_len);
-		load.read = spl_nand_read;
-		return spl_load_simple_fit(spl_image, &load, offset, header);
-	} else if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER) &&
-		   valid_container_hdr((void *)header)) {
-		struct spl_load_info load;
-
-		load.priv = &offset;
-		spl_set_bl_len(&load, bl_len);
-		load.read = spl_nand_read;
-		return spl_load_imx_container(spl_image, &load, offset);
-	} else if (IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_FORMAT) &&
-		   image_get_magic(header) == IH_MAGIC) {
-		struct spl_load_info load;
-
-		debug("Found legacy image\n");
-		load.priv = &offset;
-		spl_set_bl_len(&load, IS_ENABLED(CONFIG_SPL_LZMA) ? bl_len : 1);
-		load.read = spl_nand_read;
-		return spl_load_legacy_img(spl_image, bootdev, &load, offset, header);
-	} else {
-		err = spl_parse_image_header(spl_image, bootdev, header);
-		if (err)
-			return err;
-		return nand_spl_load_image(offset, spl_image->size,
-					   map_sysmem(spl_image->load_addr,
-						      spl_image->size));
-	}
+	load.priv = &offset;
+	spl_set_bl_len(&load, 1);
+	load.read = spl_nand_read;
+	return spl_load(spl_image, bootdev, &load, 0, offset);
 }
 
 static int spl_nand_load_image(struct spl_image_info *spl_image,
 			       struct spl_boot_device *bootdev)
 {
 	int err;
-	struct legacy_img_hdr *header;
-	int *src __attribute__((unused));
-	int *dst __attribute__((unused));
 
 #ifdef CONFIG_SPL_NAND_SOFTECC
 	debug("spl: nand - using sw ecc\n");
@@ -128,10 +90,12 @@  static int spl_nand_load_image(struct spl_image_info *spl_image,
 #endif
 	nand_init();
 
-	header = spl_get_load_buffer(0, sizeof(*header));
-
 #if CONFIG_IS_ENABLED(OS_BOOT)
 	if (!spl_start_uboot()) {
+		int *src, *dst;
+		struct legacy_img_hdr *header =
+			spl_get_load_buffer(0, sizeof(*header));
+
 		/*
 		 * load parameter image
 		 * load to temp position since nand_spl_load_image reads
@@ -174,20 +138,18 @@  static int spl_nand_load_image(struct spl_image_info *spl_image,
 	}
 #endif
 #ifdef CONFIG_NAND_ENV_DST
-	spl_nand_load_element(spl_image, bootdev, CONFIG_ENV_OFFSET, header);
+	spl_nand_load_element(spl_image, bootdev, CONFIG_ENV_OFFSET);
 #ifdef CONFIG_ENV_OFFSET_REDUND
-	spl_nand_load_element(spl_image, bootdev, CONFIG_ENV_OFFSET_REDUND, header);
+	spl_nand_load_element(spl_image, bootdev, CONFIG_ENV_OFFSET_REDUND);
 #endif
 #endif
 	/* Load u-boot */
-	err = spl_nand_load_element(spl_image, bootdev, spl_nand_get_uboot_raw_page(),
-				    header);
+	err = spl_nand_load_element(spl_image, bootdev, spl_nand_get_uboot_raw_page());
 #ifdef CONFIG_SYS_NAND_U_BOOT_OFFS_REDUND
 #if CONFIG_SYS_NAND_U_BOOT_OFFS != CONFIG_SYS_NAND_U_BOOT_OFFS_REDUND
 	if (err)
 		err = spl_nand_load_element(spl_image, bootdev,
-					    CONFIG_SYS_NAND_U_BOOT_OFFS_REDUND,
-					    header);
+					    CONFIG_SYS_NAND_U_BOOT_OFFS_REDUND);
 #endif
 #endif
 	nand_deselect();
diff --git a/include/spl_load.h b/include/spl_load.h
index d55a47ae7d7..38294a21ad0 100644
--- a/include/spl_load.h
+++ b/include/spl_load.h
@@ -98,6 +98,7 @@  static inline int _spl_load(struct spl_image_info *spl_image,
 	IS_ENABLED(CONFIG_SPL_FS_EXT4) + \
 	IS_ENABLED(CONFIG_SPL_FS_FAT) + \
 	IS_ENABLED(CONFIG_SPL_SYS_MMCSD_RAW_MODE) + \
+	(IS_ENABLED(CONFIG_SPL_NAND_SUPPORT) && !IS_ENABLED(CONFIG_SPL_UBI)) + \
 	0
 
 #if SPL_LOAD_USERS > 1
diff --git a/test/image/spl_load_nand.c b/test/image/spl_load_nand.c
index 30179de98e7..ec242207948 100644
--- a/test/image/spl_load_nand.c
+++ b/test/image/spl_load_nand.c
@@ -51,4 +51,6 @@  SPL_IMG_TEST(spl_test_nand, LEGACY, DM_FLAGS);
 SPL_IMG_TEST(spl_test_nand, LEGACY_LZMA, DM_FLAGS);
 SPL_IMG_TEST(spl_test_nand, IMX8, DM_FLAGS);
 SPL_IMG_TEST(spl_test_nand, FIT_INTERNAL, DM_FLAGS);
+#if !IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL)
 SPL_IMG_TEST(spl_test_nand, FIT_EXTERNAL, DM_FLAGS);
+#endif