[PATCH 0/8] image: Refactor ramdisk code to avoid #ifdef

[PATCH 0/8] image: Refactor ramdisk code to avoid #ifdef

[Simon Glass]

The previous attempt at this[1] broke a board and was reverted in [2].
This series adopts a slightly different approach, splitting the changes
into many commits.

[1] f33a2c1bd0f ("image: Remove #ifdefs from select_ramdisk()")
[2] 621158d106f ("Revert "image: Remove #ifdefs from select_ramdisk()"")


Simon Glass (8):
  image: Fix up ANDROID_BOOT_IMAGE ramdisk code
  image: Track when ramdisk processing is completed
  image: Drop #ifdefs for LEGACY_IMAGE_FORMAT
  image: Drop one #ifdef for FIT
  image: Drop another #ifdef for FIT
  image: Drop remaining FIT #ifdef
  image: Correct indentation in select_ramdisk()
  image: Drop some other #ifdefs in image-board.c

 boot/image-board.c | 212 ++++++++++++++++++++++++---------------------
 include/image.h    |   6 ++
 2 files changed, 121 insertions(+), 97 deletions(-)

-- 
2.37.2.672.g94769d06f0-goog

[PATCH 1/8] image: Fix up ANDROID_BOOT_IMAGE ramdisk code

[Simon Glass]

Convert this to an if(), fix the cast from an address to a pointer and
make sure that any error is returned correctly.

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

 boot/image-board.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/boot/image-board.c b/boot/image-board.c
index 4e4d1c157d7..14b595977e1 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -421,12 +421,19 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 			images->fit_noffset_rd = rd_noffset;
 			break;
 #endif
-#ifdef CONFIG_ANDROID_BOOT_IMAGE
 		case IMAGE_FORMAT_ANDROID:
-			android_image_get_ramdisk((void *)images->os.start,
-						  rd_datap, rd_lenp);
-			break;
-#endif
+			if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE)) {
+				void *ptr = map_sysmem(images->os.start, 0);
+				int ret;
+
+				ret = android_image_get_ramdisk(ptr, rd_datap,
+								rd_lenp);
+				unmap_sysmem(ptr);
+				if (ret)
+					return ret;
+				break;
+			}
+			fallthrough;
 		default:
 			if (IS_ENABLED(CONFIG_SUPPORT_RAW_INITRD)) {
 				char *end = NULL;
-- 
2.37.2.672.g94769d06f0-goog

[PATCH 2/8] image: Track when ramdisk processing is completed

[Simon Glass]

The current switch default is tricky since it relies on #ifdefs to work.
Use a bool instead.

Also fix the comment on @select, since it has a dual purpose.

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

 boot/image-board.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/boot/image-board.c b/boot/image-board.c
index 14b595977e1..2d5e5b6e6f0 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -314,7 +314,7 @@ int genimg_has_config(bootm_headers_t *images)
  * select_ramdisk() - Select and locate the ramdisk to use
  *
  * @images: pointer to the bootm images structure
- * @select: name of ramdisk to select, or NULL for any
+ * @select: name of ramdisk to select, or hex address, NULL for any
  * @arch: expected ramdisk architecture
  * @rd_datap: pointer to a ulong variable, will hold ramdisk pointer
  * @rd_lenp: pointer to a ulong variable, will hold ramdisk length
@@ -324,6 +324,7 @@ int genimg_has_config(bootm_headers_t *images)
 static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 			  ulong *rd_datap, ulong *rd_lenp)
 {
+	bool done = false;
 	ulong rd_addr;
 	char *buf;
 
@@ -401,6 +402,7 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 
 			*rd_datap = image_get_data(rd_hdr);
 			*rd_lenp = image_get_data_size(rd_hdr);
+			done = true;
 			break;
 		}
 #endif
@@ -419,6 +421,7 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 			images->fit_hdr_rd = map_sysmem(rd_addr, 0);
 			images->fit_uname_rd = fit_uname_ramdisk;
 			images->fit_noffset_rd = rd_noffset;
+			done = true;
 			break;
 #endif
 		case IMAGE_FORMAT_ANDROID:
@@ -431,24 +434,29 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 				unmap_sysmem(ptr);
 				if (ret)
 					return ret;
-				break;
+				done = true;
 			}
-			fallthrough;
-		default:
-			if (IS_ENABLED(CONFIG_SUPPORT_RAW_INITRD)) {
-				char *end = NULL;
-
-				if (select)
-					end = strchr(select, ':');
-				if (end) {
-					*rd_lenp = hextoul(++end, NULL);
-					*rd_datap = rd_addr;
-					break;
-				}
+			break;
+		}
+
+	if (!done) {
+		if (IS_ENABLED(CONFIG_SUPPORT_RAW_INITRD)) {
+			char *end = NULL;
+
+			if (select)
+				end = strchr(select, ':');
+			if (end) {
+				*rd_lenp = hextoul(++end, NULL);
+				*rd_datap = rd_addr;
+				done = true;
 			}
+		}
+
+		if (!done) {
 			puts("Wrong Ramdisk Image Format\n");
 			return -EINVAL;
 		}
+	}
 
 	return 0;
 }
-- 
2.37.2.672.g94769d06f0-goog

[PATCH 3/8] image: Drop #ifdefs for LEGACY_IMAGE_FORMAT

[Simon Glass]

Use if() instead of the #ifdef in select_ramdisk().

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

 boot/image-board.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/boot/image-board.c b/boot/image-board.c
index 2d5e5b6e6f0..a80ad08b15f 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -24,7 +24,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#if CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)
 /**
  * image_get_ramdisk - get and verify ramdisk image
  * @rd_addr: ramdisk image start address
@@ -83,7 +82,6 @@ static const image_header_t *image_get_ramdisk(ulong rd_addr, u8 arch,
 
 	return rd_hdr;
 }
-#endif
 
 /*****************************************************************************/
 /* Shared dual-format routines */
@@ -386,26 +384,25 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 		 */
 		buf = map_sysmem(rd_addr, 0);
 		switch (genimg_get_format(buf)) {
-#if CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)
-		case IMAGE_FORMAT_LEGACY: {
-			const image_header_t *rd_hdr;
+		case IMAGE_FORMAT_LEGACY:
+			if (CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)) {
+				const image_header_t *rd_hdr;
 
-			printf("## Loading init Ramdisk from Legacy Image at %08lx ...\n",
-			       rd_addr);
+				printf("## Loading init Ramdisk from Legacy Image at %08lx ...\n",
+				       rd_addr);
 
-			bootstage_mark(BOOTSTAGE_ID_CHECK_RAMDISK);
-			rd_hdr = image_get_ramdisk(rd_addr, arch,
-						   images->verify);
+				bootstage_mark(BOOTSTAGE_ID_CHECK_RAMDISK);
+				rd_hdr = image_get_ramdisk(rd_addr, arch,
+							   images->verify);
 
-			if (!rd_hdr)
-				return -ENOENT;
+				if (!rd_hdr)
+					return -ENOENT;
 
-			*rd_datap = image_get_data(rd_hdr);
-			*rd_lenp = image_get_data_size(rd_hdr);
-			done = true;
+				*rd_datap = image_get_data(rd_hdr);
+				*rd_lenp = image_get_data_size(rd_hdr);
+				done = true;
+			}
 			break;
-		}
-#endif
 #if CONFIG_IS_ENABLED(FIT)
 		case IMAGE_FORMAT_FIT:
 			rd_noffset = fit_image_load(images,
-- 
2.37.2.672.g94769d06f0-goog

[PATCH 4/8] image: Drop one #ifdef for FIT

[Simon Glass]

Drop the #ifdef from near the end of select_ramdisk(). Move some variables
to the top of the function to make this work.

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

 boot/image-board.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/boot/image-board.c b/boot/image-board.c
index a80ad08b15f..07b7e467cc4 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -322,14 +322,16 @@ int genimg_has_config(bootm_headers_t *images)
 static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 			  ulong *rd_datap, ulong *rd_lenp)
 {
+	const char *fit_uname_config;
+	const char *fit_uname_ramdisk;
 	bool done = false;
+	int rd_noffset;
 	ulong rd_addr;
 	char *buf;
 
 #if CONFIG_IS_ENABLED(FIT)
-		const char *fit_uname_config = images->fit_uname_cfg;
-		const char *fit_uname_ramdisk = NULL;
-		int rd_noffset;
+		fit_uname_config = images->fit_uname_cfg;
+		fit_uname_ramdisk = NULL;
 
 		if (select) {
 			ulong default_addr;
@@ -403,24 +405,23 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 				done = true;
 			}
 			break;
-#if CONFIG_IS_ENABLED(FIT)
 		case IMAGE_FORMAT_FIT:
-			rd_noffset = fit_image_load(images,
-						    rd_addr, &fit_uname_ramdisk,
-						    &fit_uname_config, arch,
-						    IH_TYPE_RAMDISK,
-						    BOOTSTAGE_ID_FIT_RD_START,
-						    FIT_LOAD_OPTIONAL_NON_ZERO,
-						    rd_datap, rd_lenp);
-			if (rd_noffset < 0)
-				return rd_noffset;
-
-			images->fit_hdr_rd = map_sysmem(rd_addr, 0);
-			images->fit_uname_rd = fit_uname_ramdisk;
-			images->fit_noffset_rd = rd_noffset;
-			done = true;
+			if (CONFIG_IS_ENABLED(FIT)) {
+				rd_noffset = fit_image_load(images, rd_addr,
+					&fit_uname_ramdisk, &fit_uname_config,
+					arch, IH_TYPE_RAMDISK,
+					BOOTSTAGE_ID_FIT_RD_START,
+					FIT_LOAD_OPTIONAL_NON_ZERO,
+					rd_datap, rd_lenp);
+				if (rd_noffset < 0)
+					return rd_noffset;
+
+				images->fit_hdr_rd = map_sysmem(rd_addr, 0);
+				images->fit_uname_rd = fit_uname_ramdisk;
+				images->fit_noffset_rd = rd_noffset;
+				done = true;
+			}
 			break;
-#endif
 		case IMAGE_FORMAT_ANDROID:
 			if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE)) {
 				void *ptr = map_sysmem(images->os.start, 0);
-- 
2.37.2.672.g94769d06f0-goog

[PATCH 5/8] image: Drop another #ifdef for FIT

[Simon Glass]

Drop the prenultimate one of these from select_ramdisk().

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

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

diff --git a/boot/image-board.c b/boot/image-board.c
index 07b7e467cc4..8858a2fe126 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -363,7 +363,9 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 				      rd_addr);
 			}
 #if CONFIG_IS_ENABLED(FIT)
-		} else {
+		}
+#endif
+		if (CONFIG_IS_ENABLED(FIT) && !select) {
 			/* use FIT configuration provided in first bootm
 			 * command argument. If the property is not defined,
 			 * quit silently (with -ENOPKG)
@@ -377,7 +379,6 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 			else if (rd_noffset < 0)
 				return rd_noffset;
 		}
-#endif
 
 		/*
 		 * Check if there is an initrd image at the
-- 
2.37.2.672.g94769d06f0-goog

[PATCH 6/8] image: Drop remaining FIT #ifdef

[Simon Glass]

Drop the last one of these, by using a done_select variable to control
whether to fall back to using 'select' as a hex value.

Note that the indentation is not adjusted, to make this easier to review.

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

 boot/image-board.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/boot/image-board.c b/boot/image-board.c
index 8858a2fe126..7c0948b592a 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -324,12 +324,13 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 {
 	const char *fit_uname_config;
 	const char *fit_uname_ramdisk;
+	bool done_select = !select;
 	bool done = false;
 	int rd_noffset;
 	ulong rd_addr;
 	char *buf;
 
-#if CONFIG_IS_ENABLED(FIT)
+	if (CONFIG_IS_ENABLED(FIT)) {
 		fit_uname_config = images->fit_uname_cfg;
 		fit_uname_ramdisk = NULL;
 
@@ -350,21 +351,21 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 					   &rd_addr, &fit_uname_config)) {
 				debug("*  ramdisk: config '%s' from image at 0x%08lx\n",
 				      fit_uname_config, rd_addr);
+				done_select = true;
 			} else if (fit_parse_subimage(select, default_addr,
 						      &rd_addr,
 						      &fit_uname_ramdisk)) {
 				debug("*  ramdisk: subimage '%s' from image at 0x%08lx\n",
 				      fit_uname_ramdisk, rd_addr);
-			} else
-#endif
-			{
+				done_select = true;
+			}
+		}
+	}
+	if (!done_select) {
 				rd_addr = hextoul(select, NULL);
 				debug("*  ramdisk: cmdline image address = 0x%08lx\n",
 				      rd_addr);
-			}
-#if CONFIG_IS_ENABLED(FIT)
-		}
-#endif
+	}
 		if (CONFIG_IS_ENABLED(FIT) && !select) {
 			/* use FIT configuration provided in first bootm
 			 * command argument. If the property is not defined,
-- 
2.37.2.672.g94769d06f0-goog

[PATCH 7/8] image: Correct indentation in select_ramdisk()

[Simon Glass]

Finish off the refactoring by correcting the indent levels. Note that this
does not include any functional changes.

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

 boot/image-board.c | 140 ++++++++++++++++++++++-----------------------
 1 file changed, 69 insertions(+), 71 deletions(-)

diff --git a/boot/image-board.c b/boot/image-board.c
index 7c0948b592a..7a17ffb7f7f 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -347,8 +347,8 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 			else
 				default_addr = image_load_addr;
 
-			if (fit_parse_conf(select, default_addr,
-					   &rd_addr, &fit_uname_config)) {
+			if (fit_parse_conf(select, default_addr, &rd_addr,
+					   &fit_uname_config)) {
 				debug("*  ramdisk: config '%s' from image at 0x%08lx\n",
 				      fit_uname_config, rd_addr);
 				done_select = true;
@@ -362,82 +362,80 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 		}
 	}
 	if (!done_select) {
-				rd_addr = hextoul(select, NULL);
-				debug("*  ramdisk: cmdline image address = 0x%08lx\n",
-				      rd_addr);
+		rd_addr = hextoul(select, NULL);
+		debug("*  ramdisk: cmdline image address = 0x%08lx\n", rd_addr);
 	}
-		if (CONFIG_IS_ENABLED(FIT) && !select) {
-			/* use FIT configuration provided in first bootm
-			 * command argument. If the property is not defined,
-			 * quit silently (with -ENOPKG)
-			 */
-			rd_addr = map_to_sysmem(images->fit_hdr_os);
-			rd_noffset = fit_get_node_from_config(images,
-							      FIT_RAMDISK_PROP,
-							      rd_addr);
-			if (rd_noffset == -ENOENT)
-				return -ENOPKG;
-			else if (rd_noffset < 0)
-				return rd_noffset;
-		}
-
-		/*
-		 * Check if there is an initrd image at the
-		 * address provided in the second bootm argument
-		 * check image type, for FIT images get FIT node.
+	if (CONFIG_IS_ENABLED(FIT) && !select) {
+		/* use FIT configuration provided in first bootm
+		 * command argument. If the property is not defined,
+		 * quit silently (with -ENOPKG)
 		 */
-		buf = map_sysmem(rd_addr, 0);
-		switch (genimg_get_format(buf)) {
-		case IMAGE_FORMAT_LEGACY:
-			if (CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)) {
-				const image_header_t *rd_hdr;
+		rd_addr = map_to_sysmem(images->fit_hdr_os);
+		rd_noffset = fit_get_node_from_config(images, FIT_RAMDISK_PROP,
+						      rd_addr);
+		if (rd_noffset == -ENOENT)
+			return -ENOPKG;
+		else if (rd_noffset < 0)
+			return rd_noffset;
+	}
 
-				printf("## Loading init Ramdisk from Legacy Image at %08lx ...\n",
-				       rd_addr);
+	/*
+	 * Check if there is an initrd image at the
+	 * address provided in the second bootm argument
+	 * check image type, for FIT images get FIT node.
+	 */
+	buf = map_sysmem(rd_addr, 0);
+	switch (genimg_get_format(buf)) {
+	case IMAGE_FORMAT_LEGACY:
+		if (CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)) {
+			const image_header_t *rd_hdr;
 
-				bootstage_mark(BOOTSTAGE_ID_CHECK_RAMDISK);
-				rd_hdr = image_get_ramdisk(rd_addr, arch,
-							   images->verify);
+			printf("## Loading init Ramdisk from Legacy Image at %08lx ...\n",
+			       rd_addr);
 
-				if (!rd_hdr)
-					return -ENOENT;
+			bootstage_mark(BOOTSTAGE_ID_CHECK_RAMDISK);
+			rd_hdr = image_get_ramdisk(rd_addr, arch,
+						   images->verify);
 
-				*rd_datap = image_get_data(rd_hdr);
-				*rd_lenp = image_get_data_size(rd_hdr);
-				done = true;
-			}
-			break;
-		case IMAGE_FORMAT_FIT:
-			if (CONFIG_IS_ENABLED(FIT)) {
-				rd_noffset = fit_image_load(images, rd_addr,
-					&fit_uname_ramdisk, &fit_uname_config,
-					arch, IH_TYPE_RAMDISK,
-					BOOTSTAGE_ID_FIT_RD_START,
-					FIT_LOAD_OPTIONAL_NON_ZERO,
-					rd_datap, rd_lenp);
-				if (rd_noffset < 0)
-					return rd_noffset;
-
-				images->fit_hdr_rd = map_sysmem(rd_addr, 0);
-				images->fit_uname_rd = fit_uname_ramdisk;
-				images->fit_noffset_rd = rd_noffset;
-				done = true;
-			}
-			break;
-		case IMAGE_FORMAT_ANDROID:
-			if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE)) {
-				void *ptr = map_sysmem(images->os.start, 0);
-				int ret;
-
-				ret = android_image_get_ramdisk(ptr, rd_datap,
-								rd_lenp);
-				unmap_sysmem(ptr);
-				if (ret)
-					return ret;
-				done = true;
-			}
-			break;
+			if (!rd_hdr)
+				return -ENOENT;
+
+			*rd_datap = image_get_data(rd_hdr);
+			*rd_lenp = image_get_data_size(rd_hdr);
+			done = true;
+		}
+		break;
+	case IMAGE_FORMAT_FIT:
+		if (CONFIG_IS_ENABLED(FIT)) {
+			rd_noffset = fit_image_load(images, rd_addr,
+						    &fit_uname_ramdisk,
+						    &fit_uname_config,
+						    arch, IH_TYPE_RAMDISK,
+						    BOOTSTAGE_ID_FIT_RD_START,
+						    FIT_LOAD_OPTIONAL_NON_ZERO,
+						    rd_datap, rd_lenp);
+			if (rd_noffset < 0)
+				return rd_noffset;
+
+			images->fit_hdr_rd = map_sysmem(rd_addr, 0);
+			images->fit_uname_rd = fit_uname_ramdisk;
+			images->fit_noffset_rd = rd_noffset;
+			done = true;
+		}
+		break;
+	case IMAGE_FORMAT_ANDROID:
+		if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE)) {
+			void *ptr = map_sysmem(images->os.start, 0);
+			int ret;
+
+			ret = android_image_get_ramdisk(ptr, rd_datap, rd_lenp);
+			unmap_sysmem(ptr);
+			if (ret)
+				return ret;
+			done = true;
 		}
+		break;
+	}
 
 	if (!done) {
 		if (IS_ENABLED(CONFIG_SUPPORT_RAW_INITRD)) {
-- 
2.37.2.672.g94769d06f0-goog

[PATCH 8/8] image: Drop some other #ifdefs in image-board.c

[Simon Glass]

Remove all but a few that are difficult, relying on legacy CONFIG options
or optional global_data fields.

Drop the duplicate function name in the comment for boot_get_cmdline().

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

 boot/image-board.c | 67 +++++++++++++++++++++++++---------------------
 include/image.h    |  6 +++++
 2 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/boot/image-board.c b/boot/image-board.c
index 7a17ffb7f7f..1be0a359aba 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -16,6 +16,7 @@
 #include <fpga.h>
 #include <image.h>
 #include <init.h>
+#include <log.h>
 #include <mapmem.h>
 #include <rtc.h>
 #include <watchdog.h>
@@ -172,29 +173,29 @@ void memmove_wd(void *to, void *from, size_t len, ulong chunksz)
 	if (to == from)
 		return;
 
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
-	if (to > from) {
-		from += len;
-		to += len;
-	}
-	while (len > 0) {
-		size_t tail = (len > chunksz) ? chunksz : len;
-
-		WATCHDOG_RESET();
+	if (IS_ENABLED(CONFIG_HW_WATCHDOG) || IS_ENABLED(CONFIG_WATCHDOG)) {
 		if (to > from) {
-			to -= tail;
-			from -= tail;
+			from += len;
+			to += len;
 		}
-		memmove(to, from, tail);
-		if (to < from) {
-			to += tail;
-			from += tail;
+		while (len > 0) {
+			size_t tail = (len > chunksz) ? chunksz : len;
+
+			WATCHDOG_RESET();
+			if (to > from) {
+				to -= tail;
+				from -= tail;
+			}
+			memmove(to, from, tail);
+			if (to < from) {
+				to += tail;
+				from += tail;
+			}
+			len -= tail;
 		}
-		len -= tail;
+	} else {
+		memmove(to, from, len);
 	}
-#else	/* !(CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG) */
-	memmove(to, from, len);
-#endif	/* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */
 }
 
 /**
@@ -551,7 +552,6 @@ int boot_get_ramdisk(int argc, char *const argv[], bootm_headers_t *images,
 	return 0;
 }
 
-#if defined(CONFIG_LMB)
 /**
  * boot_ramdisk_high - relocate init ramdisk
  * @lmb: pointer to lmb handle, will be used for memory mgmt
@@ -645,7 +645,6 @@ int boot_ramdisk_high(struct lmb *lmb, ulong rd_data, ulong rd_len,
 error:
 	return -1;
 }
-#endif
 
 int boot_get_setup(bootm_headers_t *images, u8 arch,
 		   ulong *setup_start, ulong *setup_len)
@@ -839,15 +838,13 @@ int boot_get_loadable(int argc, char *const argv[], bootm_headers_t *images,
 	return 0;
 }
 
-#if defined(CONFIG_LMB)
-#ifdef CONFIG_SYS_BOOT_GET_CMDLINE
 /**
  * boot_get_cmdline - allocate and initialize kernel cmdline
  * @lmb: pointer to lmb handle, will be used for memory mgmt
  * @cmd_start: pointer to a ulong variable, will hold cmdline start
  * @cmd_end: pointer to a ulong variable, will hold cmdline end
  *
- * boot_get_cmdline() allocates space for kernel command line below
+ * This allocates space for kernel command line below
  * BOOTMAPSZ + env_get_bootm_low() address. If "bootargs" U-Boot environment
  * variable is present its contents is copied to allocated kernel
  * command line.
@@ -858,10 +855,19 @@ int boot_get_loadable(int argc, char *const argv[], bootm_headers_t *images,
  */
 int boot_get_cmdline(struct lmb *lmb, ulong *cmd_start, ulong *cmd_end)
 {
+	int barg;
 	char *cmdline;
 	char *s;
 
-	cmdline = (char *)(ulong)lmb_alloc_base(lmb, CONFIG_SYS_BARGSIZE, 0xf,
+	/*
+	 * Help the compiler detect that this function is only called when
+	 * CONFIG_SYS_BOOT_GET_CMDLINE is enabled
+	 */
+	if (!IS_ENABLED(CONFIG_SYS_BOOT_GET_CMDLINE))
+		return 0;
+
+	barg = IF_ENABLED_INT(CONFIG_SYS_BOOT_GET_CMDLINE, CONFIG_SYS_BARGSIZE);
+	cmdline = (char *)(ulong)lmb_alloc_base(lmb, barg, 0xf,
 				env_get_bootm_mapsize() + env_get_bootm_low());
 	if (!cmdline)
 		return -1;
@@ -907,22 +913,22 @@ int boot_get_kbd(struct lmb *lmb, struct bd_info **kbd)
 
 	debug("## kernel board info at 0x%08lx\n", (ulong)*kbd);
 
-#if defined(DEBUG)
-	if (IS_ENABLED(CONFIG_CMD_BDI))
+	if (_DEBUG && IS_ENABLED(CONFIG_CMD_BDI))
 		do_bdinfo(NULL, 0, 0, NULL);
-#endif
 
 	return 0;
 }
-#endif
 
 int image_setup_linux(bootm_headers_t *images)
 {
 	ulong of_size = images->ft_len;
 	char **of_flat_tree = &images->ft_addr;
-	struct lmb *lmb = &images->lmb;
+	struct lmb *lmb = images_lmb(images);
 	int ret;
 
+	/* This function cannot be called without lmb support */
+	if (!CONFIG_IS_ENABLED(LMB))
+		return -EFAULT;
 	if (CONFIG_IS_ENABLED(OF_LIBFDT))
 		boot_fdt_add_mem_rsv_regions(lmb, *of_flat_tree);
 
@@ -949,7 +955,6 @@ int image_setup_linux(bootm_headers_t *images)
 
 	return 0;
 }
-#endif
 
 void genimg_print_size(uint32_t size)
 {
diff --git a/include/image.h b/include/image.h
index e4c6a50b885..a148073113a 100644
--- a/include/image.h
+++ b/include/image.h
@@ -360,6 +360,12 @@ typedef struct bootm_headers {
 #endif
 } bootm_headers_t;
 
+#ifdef CONFIG_LMB
+#define images_lmb(_images)	(&(_images)->lmb)
+#else
+#define images_lmb(_images)	NULL
+#endif
+
 extern bootm_headers_t images;
 
 /*
-- 
2.37.2.672.g94769d06f0-goog

Re: [PATCH 0/8] image: Refactor ramdisk code to avoid #ifdef

[Tom Rini]

On Sun, 28 Aug 2022 12:32:45 -0600, Simon Glass wrote:

> The previous attempt at this[1] broke a board and was reverted in [2].
> This series adopts a slightly different approach, splitting the changes
> into many commits.
> 
> [1] f33a2c1bd0f ("image: Remove #ifdefs from select_ramdisk()")
> [2] 621158d106f ("Revert "image: Remove #ifdefs from select_ramdisk()"")
> 
> [...]

Applied to u-boot/next, thanks!

-- 
Tom

Re: [PATCH 8/8] image: Drop some other #ifdefs in image-board.c

[Heinrich Schuchardt]

On 8/28/22 20:32, Simon Glass wrote:
> Remove all but a few that are difficult, relying on legacy CONFIG options
> or optional global_data fields.
>
> Drop the duplicate function name in the comment for boot_get_cmdline().
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   boot/image-board.c | 67 +++++++++++++++++++++++++---------------------
>   include/image.h    |  6 +++++
>   2 files changed, 42 insertions(+), 31 deletions(-)
>
> diff --git a/boot/image-board.c b/boot/image-board.c
> index 7a17ffb7f7f..1be0a359aba 100644
> --- a/boot/image-board.c
> +++ b/boot/image-board.c
> @@ -16,6 +16,7 @@
>   #include <fpga.h>
>   #include <image.h>
>   #include <init.h>
> +#include <log.h>

This looks like an unrelated change.

Best regards

Heinrich

>   #include <mapmem.h>
>   #include <rtc.h>
>   #include <watchdog.h>
> @@ -172,29 +173,29 @@ void memmove_wd(void *to, void *from, size_t len, ulong chunksz)
>   	if (to == from)
>   		return;
>
> -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
> -	if (to > from) {
> -		from += len;
> -		to += len;
> -	}
> -	while (len > 0) {
> -		size_t tail = (len > chunksz) ? chunksz : len;
> -
> -		WATCHDOG_RESET();
> +	if (IS_ENABLED(CONFIG_HW_WATCHDOG) || IS_ENABLED(CONFIG_WATCHDOG)) {
>   		if (to > from) {
> -			to -= tail;
> -			from -= tail;
> +			from += len;
> +			to += len;
>   		}
> -		memmove(to, from, tail);
> -		if (to < from) {
> -			to += tail;
> -			from += tail;
> +		while (len > 0) {
> +			size_t tail = (len > chunksz) ? chunksz : len;
> +
> +			WATCHDOG_RESET();
> +			if (to > from) {
> +				to -= tail;
> +				from -= tail;
> +			}
> +			memmove(to, from, tail);
> +			if (to < from) {
> +				to += tail;
> +				from += tail;
> +			}
> +			len -= tail;
>   		}
> -		len -= tail;
> +	} else {
> +		memmove(to, from, len);
>   	}
> -#else	/* !(CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG) */
> -	memmove(to, from, len);
> -#endif	/* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */
>   }
>
>   /**
> @@ -551,7 +552,6 @@ int boot_get_ramdisk(int argc, char *const argv[], bootm_headers_t *images,
>   	return 0;
>   }
>
> -#if defined(CONFIG_LMB)
>   /**
>    * boot_ramdisk_high - relocate init ramdisk
>    * @lmb: pointer to lmb handle, will be used for memory mgmt
> @@ -645,7 +645,6 @@ int boot_ramdisk_high(struct lmb *lmb, ulong rd_data, ulong rd_len,
>   error:
>   	return -1;
>   }
> -#endif
>
>   int boot_get_setup(bootm_headers_t *images, u8 arch,
>   		   ulong *setup_start, ulong *setup_len)
> @@ -839,15 +838,13 @@ int boot_get_loadable(int argc, char *const argv[], bootm_headers_t *images,
>   	return 0;
>   }
>
> -#if defined(CONFIG_LMB)
> -#ifdef CONFIG_SYS_BOOT_GET_CMDLINE
>   /**
>    * boot_get_cmdline - allocate and initialize kernel cmdline
>    * @lmb: pointer to lmb handle, will be used for memory mgmt
>    * @cmd_start: pointer to a ulong variable, will hold cmdline start
>    * @cmd_end: pointer to a ulong variable, will hold cmdline end
>    *
> - * boot_get_cmdline() allocates space for kernel command line below
> + * This allocates space for kernel command line below
>    * BOOTMAPSZ + env_get_bootm_low() address. If "bootargs" U-Boot environment
>    * variable is present its contents is copied to allocated kernel
>    * command line.
> @@ -858,10 +855,19 @@ int boot_get_loadable(int argc, char *const argv[], bootm_headers_t *images,
>    */
>   int boot_get_cmdline(struct lmb *lmb, ulong *cmd_start, ulong *cmd_end)
>   {
> +	int barg;
>   	char *cmdline;
>   	char *s;
>
> -	cmdline = (char *)(ulong)lmb_alloc_base(lmb, CONFIG_SYS_BARGSIZE, 0xf,
> +	/*
> +	 * Help the compiler detect that this function is only called when
> +	 * CONFIG_SYS_BOOT_GET_CMDLINE is enabled
> +	 */
> +	if (!IS_ENABLED(CONFIG_SYS_BOOT_GET_CMDLINE))
> +		return 0;
> +
> +	barg = IF_ENABLED_INT(CONFIG_SYS_BOOT_GET_CMDLINE, CONFIG_SYS_BARGSIZE);
> +	cmdline = (char *)(ulong)lmb_alloc_base(lmb, barg, 0xf,
>   				env_get_bootm_mapsize() + env_get_bootm_low());
>   	if (!cmdline)
>   		return -1;
> @@ -907,22 +913,22 @@ int boot_get_kbd(struct lmb *lmb, struct bd_info **kbd)
>
>   	debug("## kernel board info at 0x%08lx\n", (ulong)*kbd);
>
> -#if defined(DEBUG)
> -	if (IS_ENABLED(CONFIG_CMD_BDI))
> +	if (_DEBUG && IS_ENABLED(CONFIG_CMD_BDI))
>   		do_bdinfo(NULL, 0, 0, NULL);
> -#endif
>
>   	return 0;
>   }
> -#endif
>
>   int image_setup_linux(bootm_headers_t *images)
>   {
>   	ulong of_size = images->ft_len;
>   	char **of_flat_tree = &images->ft_addr;
> -	struct lmb *lmb = &images->lmb;
> +	struct lmb *lmb = images_lmb(images);
>   	int ret;
>
> +	/* This function cannot be called without lmb support */
> +	if (!CONFIG_IS_ENABLED(LMB))
> +		return -EFAULT;
>   	if (CONFIG_IS_ENABLED(OF_LIBFDT))
>   		boot_fdt_add_mem_rsv_regions(lmb, *of_flat_tree);
>
> @@ -949,7 +955,6 @@ int image_setup_linux(bootm_headers_t *images)
>
>   	return 0;
>   }
> -#endif
>
>   void genimg_print_size(uint32_t size)
>   {
> diff --git a/include/image.h b/include/image.h
> index e4c6a50b885..a148073113a 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -360,6 +360,12 @@ typedef struct bootm_headers {
>   #endif
>   } bootm_headers_t;
>
> +#ifdef CONFIG_LMB
> +#define images_lmb(_images)	(&(_images)->lmb)
> +#else
> +#define images_lmb(_images)	NULL
> +#endif
> +
>   extern bootm_headers_t images;
>
>   /*

Re: [PATCH 8/8] image: Drop some other #ifdefs in image-board.c

[Simon Glass]

Hi Heinrich,

On Wed, 14 Sept 2022 at 10:22, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 8/28/22 20:32, Simon Glass wrote:
> > Remove all but a few that are difficult, relying on legacy CONFIG options
> > or optional global_data fields.
> >
> > Drop the duplicate function name in the comment for boot_get_cmdline().
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >   boot/image-board.c | 67 +++++++++++++++++++++++++---------------------
> >   include/image.h    |  6 +++++
> >   2 files changed, 42 insertions(+), 31 deletions(-)
> >
> > diff --git a/boot/image-board.c b/boot/image-board.c
> > index 7a17ffb7f7f..1be0a359aba 100644
> > --- a/boot/image-board.c
> > +++ b/boot/image-board.c
> > @@ -16,6 +16,7 @@
> >   #include <fpga.h>
> >   #include <image.h>
> >   #include <init.h>
> > +#include <log.h>
>
> This looks like an unrelated change.

Yes, probably left in after some debugging. But it doesn't hurt, since
at present log.h is being included by one of the other includes...

Regards,
Simon