[PATCH 0/3] bootm: make uImage support optional

[PATCH 0/3] bootm: make uImage support optional

[Ahmad Fatoum]

While looking into CONFIG_TIMESTAMP, I noticed we still build UIMAGE
support unconditionally, when BOOTM is enabled. This series makes it
optional as most users no longer need it.

Saves me 4KiB in an LZO-compressed ARM64 image.

Ahmad Fatoum (3):
  uimage: move file_to_sdram implementation to libfile
  bootm: allow disabling uImage support
  uimage: have TIMESTMAP depend on UIMAGE

 commands/Kconfig  |  1 -
 common/Kconfig    | 13 ++++++++--
 common/bootm.c    | 22 +++++++++++++----
 common/uimage.c   | 59 ---------------------------------------------
 include/image.h   |  1 -
 include/libfile.h |  4 ++++
 lib/libfile.c     | 61 +++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 93 insertions(+), 68 deletions(-)

-- 
2.39.2

[PATCH 1/3] uimage: move file_to_sdram implementation to libfile

[Ahmad Fatoum]

There's nothing uImage specific about file_to_sdram, but it didn't
matter so far, because the only user is bootm and bootm always selected
uImage support. This is about to change, so move file_to_sdram to
another location that's always available independent of CONFIG_UIMAGE.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/uimage.c   | 59 ---------------------------------------------
 include/image.h   |  1 -
 include/libfile.h |  4 ++++
 lib/libfile.c     | 61 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 60 deletions(-)

diff --git a/common/uimage.c b/common/uimage.c
index cc9e5e510a97..140a08c1e426 100644
--- a/common/uimage.c
+++ b/common/uimage.c
@@ -364,65 +364,6 @@ static long uimage_sdram_flush(void *buf, unsigned long len)
 	return len;
 }
 
-#define BUFSIZ	(PAGE_SIZE * 32)
-
-struct resource *file_to_sdram(const char *filename, unsigned long adr)
-{
-	struct resource *res;
-	size_t size = BUFSIZ;
-	size_t ofs = 0;
-	ssize_t now;
-	int fd;
-
-	fd = open(filename, O_RDONLY);
-	if (fd < 0)
-		return NULL;
-
-	while (1) {
-		res = request_sdram_region("image", adr, size);
-		if (!res) {
-			printf("unable to request SDRAM 0x%08lx-0x%08lx\n",
-				adr, adr + size - 1);
-			goto out;
-		}
-
-		if (zero_page_contains(res->start + ofs)) {
-			void *tmp = malloc(BUFSIZ);
-			if (!tmp)
-				now = -ENOMEM;
-			else
-				now = read_full(fd, tmp, BUFSIZ);
-
-			if (now > 0)
-				zero_page_memcpy((void *)(res->start + ofs), tmp, now);
-			free(tmp);
-		} else {
-			now = read_full(fd, (void *)(res->start + ofs), BUFSIZ);
-		}
-
-		if (now < 0) {
-			release_sdram_region(res);
-			res = NULL;
-			goto out;
-		}
-
-		if (now < BUFSIZ) {
-			release_sdram_region(res);
-			res = request_sdram_region("image", adr, ofs + now);
-			goto out;
-		}
-
-		release_sdram_region(res);
-
-		ofs += BUFSIZ;
-		size += BUFSIZ;
-	}
-out:
-	close(fd);
-
-	return res;
-}
-
 /*
  * Load an uImage to a dynamically allocated sdram resource.
  * the resource must be freed afterwards with release_sdram_region
diff --git a/include/image.h b/include/image.h
index b4c69d9a025b..277a546c8500 100644
--- a/include/image.h
+++ b/include/image.h
@@ -307,7 +307,6 @@ struct resource *uimage_load_to_sdram(struct uimage_handle *handle,
 		int image_no, unsigned long load_address);
 void *uimage_load_to_buf(struct uimage_handle *handle, int image_no,
 		size_t *size);
-struct resource *file_to_sdram(const char *filename, unsigned long adr);
 #define MAX_MULTI_IMAGE_COUNT 16
 
 struct uimage_handle {
diff --git a/include/libfile.h b/include/libfile.h
index 1240276e1d74..f772ff8b6af2 100644
--- a/include/libfile.h
+++ b/include/libfile.h
@@ -4,6 +4,8 @@
 
 #include <linux/types.h>
 
+struct resource;
+
 int pread_full(int fd, void *buf, size_t size, loff_t offset);
 int pwrite_full(int fd, const void *buf, size_t size, loff_t offset);
 int write_full(int fd, const void *buf, size_t size);
@@ -41,4 +43,6 @@ char *make_temp(const char *template);
 
 int cache_file(const char *path, char **newpath);
 
+struct resource *file_to_sdram(const char *filename, unsigned long adr);
+
 #endif /* __LIBFILE_H */
diff --git a/lib/libfile.c b/lib/libfile.c
index 67fc9cc7f3a2..a34e011f4f7c 100644
--- a/lib/libfile.c
+++ b/lib/libfile.c
@@ -12,6 +12,8 @@
  *
  */
 #include <common.h>
+#include <memory.h>
+#include <zero_page.h>
 #include <fs.h>
 #include <fcntl.h>
 #include <malloc.h>
@@ -741,3 +743,62 @@ int cache_file(const char *path, char **newpath)
 
 	return 0;
 }
+
+#define BUFSIZ	(PAGE_SIZE * 32)
+
+struct resource *file_to_sdram(const char *filename, unsigned long adr)
+{
+	struct resource *res;
+	size_t size = BUFSIZ;
+	size_t ofs = 0;
+	ssize_t now;
+	int fd;
+
+	fd = open(filename, O_RDONLY);
+	if (fd < 0)
+		return NULL;
+
+	while (1) {
+		res = request_sdram_region("image", adr, size);
+		if (!res) {
+			printf("unable to request SDRAM 0x%08lx-0x%08lx\n",
+				adr, adr + size - 1);
+			goto out;
+		}
+
+		if (zero_page_contains(res->start + ofs)) {
+			void *tmp = malloc(BUFSIZ);
+			if (!tmp)
+				now = -ENOMEM;
+			else
+				now = read_full(fd, tmp, BUFSIZ);
+
+			if (now > 0)
+				zero_page_memcpy((void *)(res->start + ofs), tmp, now);
+			free(tmp);
+		} else {
+			now = read_full(fd, (void *)(res->start + ofs), BUFSIZ);
+		}
+
+		if (now < 0) {
+			release_sdram_region(res);
+			res = NULL;
+			goto out;
+		}
+
+		if (now < BUFSIZ) {
+			release_sdram_region(res);
+			res = request_sdram_region("image", adr, ofs + now);
+			goto out;
+		}
+
+		release_sdram_region(res);
+
+		ofs += BUFSIZ;
+		size += BUFSIZ;
+	}
+out:
+	close(fd);
+
+	return res;
+}
-- 
2.39.2

[PATCH 2/3] bootm: allow disabling uImage support

[Ahmad Fatoum]

Enabling uImage support unconditionally for bootm is a legacy left-over.
Let's make this configurable via a separate BOOTM_UIMAGE option that can
be disabled for new platforms.

Disabling uImage support saves 4KiB in a LZO compressed ARM64 image.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/Kconfig |  1 -
 common/Kconfig   | 12 ++++++++++--
 common/bootm.c   | 22 +++++++++++++++++-----
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/commands/Kconfig b/commands/Kconfig
index 8d0816c4d0f0..47caae141312 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -427,7 +427,6 @@ config CMD_BOOTM
 	default y
 	depends on BOOTM
 	select CRC32
-	select UIMAGE
 	select UNCOMPRESS
 	select FILETYPE
 	depends on GLOBALVAR
diff --git a/common/Kconfig b/common/Kconfig
index 0000dac8740e..ea7223cffd05 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -564,7 +564,6 @@ config TIMESTAMP
 	  commands like bootm or uimage.
 
 menuconfig BOOTM
-	select UIMAGE
 	default y if COMMAND_SUPPORT
 	bool "bootm support"
 
@@ -608,10 +607,19 @@ config BOOTM_OFTREE
 
 	  -o DTS  specify device tree
 
+config BOOTM_UIMAGE
+	def_bool y
+	prompt "support the legacy uImage format"
+	select UIMAGE
+	depends on BOOTM
+	help
+	  Support using uImages. This format has been superseded
+	  by FIT images and is not as frequently used nowadays.
+
 config BOOTM_OFTREE_UIMAGE
 	bool
 	prompt "support passing device tree (oftree) uImages"
-	depends on BOOTM_OFTREE
+	depends on BOOTM_OFTREE && BOOTM_UIMAGE
 	help
 	  Support using oftree uImages. Without this only raw oftree
 	  blobs can be used.
diff --git a/common/bootm.c b/common/bootm.c
index 4cc88eed76b5..09c93ac4952f 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -92,6 +92,11 @@ static int uimage_part_num(const char *partname)
 	return simple_strtoul(partname, NULL, 0);
 }
 
+static inline bool image_is_uimage(struct image_data *data)
+{
+	return IS_ENABLED(CONFIG_BOOTM_UIMAGE) && data->os;
+}
+
 /*
  * bootm_load_os() - load OS to RAM
  *
@@ -129,7 +134,7 @@ int bootm_load_os(struct image_data *data, unsigned long load_address)
 		return 0;
 	}
 
-	if (data->os) {
+	if (image_is_uimage(data)) {
 		int num;
 
 		num = uimage_part_num(data->os_part);
@@ -175,6 +180,9 @@ static int bootm_open_initrd_uimage(struct image_data *data)
 {
 	int ret;
 
+	if (!IS_ENABLED(CONFIG_BOOTM_UIMAGE))
+		return -ENOSYS;
+
 	if (strcmp(data->os_file, data->initrd_file)) {
 		data->initrd = uimage_open(data->initrd_file);
 		if (!data->initrd)
@@ -482,7 +490,7 @@ int bootm_get_os_size(struct image_data *data)
 
 	if (data->elf)
 		return elf_get_mem_size(data->elf);
-	if (data->os)
+	if (image_is_uimage(data))
 		return uimage_get_size(data->os, uimage_part_num(data->os_part));
 	if (data->os_fit)
 		return data->fit_kernel_size;
@@ -502,6 +510,9 @@ static int bootm_open_os_uimage(struct image_data *data)
 {
 	int ret;
 
+	if (!IS_ENABLED(CONFIG_BOOTM_UIMAGE))
+		return -ENOSYS;
+
 	data->os = uimage_open(data->os_file);
 	if (!data->os)
 		return -EINVAL;
@@ -837,10 +848,11 @@ int bootm_boot(struct bootm_data *bootm_data)
 		release_sdram_region(data->oftree_res);
 	if (data->tee_res)
 		release_sdram_region(data->tee_res);
-	if (data->initrd && data->initrd != data->os)
-		uimage_close(data->initrd);
-	if (data->os)
+	if (image_is_uimage(data)) {
+		if (data->initrd && data->initrd != data->os)
+			uimage_close(data->initrd);
 		uimage_close(data->os);
+	}
 	if (IS_ENABLED(CONFIG_ELF) && data->elf)
 		elf_close(data->elf);
 	if (IS_ENABLED(CONFIG_FITIMAGE) && data->os_fit)
-- 
2.39.2

[PATCH 3/3] uimage: have TIMESTMAP depend on UIMAGE

[Ahmad Fatoum]

The only users of CONFIG_TIMESTAMP are the uImage command, file system
and the bootm integration. Therefore make it depends on UIMAGE, so
unaffected users are not prompted for it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/Kconfig b/common/Kconfig
index ea7223cffd05..07cb3d5616cc 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -556,6 +556,7 @@ config ERRNO_MESSAGES
 config TIMESTAMP
 	bool
 	default y
+	depends on UIMAGE
 	select GREGORIAN_CALENDER
 	prompt "print timestamp information from uImages"
 	help
-- 
2.39.2

Re: [PATCH 0/3] bootm: make uImage support optional

[Sascha Hauer]

On Tue, 30 Apr 2024 20:06:30 +0200, Ahmad Fatoum wrote:
> While looking into CONFIG_TIMESTAMP, I noticed we still build UIMAGE
> support unconditionally, when BOOTM is enabled. This series makes it
> optional as most users no longer need it.
> 
> Saves me 4KiB in an LZO-compressed ARM64 image.
> 
> Ahmad Fatoum (3):
>   uimage: move file_to_sdram implementation to libfile
>   bootm: allow disabling uImage support
>   uimage: have TIMESTMAP depend on UIMAGE
> 
> [...]

Applied, thanks!

[1/3] uimage: move file_to_sdram implementation to libfile
      https://git.pengutronix.de/cgit/barebox/commit/?id=7b29c539e461 (link may not be stable)
[2/3] bootm: allow disabling uImage support
      https://git.pengutronix.de/cgit/barebox/commit/?id=59edd0c3be21 (link may not be stable)
[3/3] uimage: have TIMESTMAP depend on UIMAGE
      https://git.pengutronix.de/cgit/barebox/commit/?id=76131012282d (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>