[U-Boot] [PATCH 0/4] Correct some remaining bootm problems

[U-Boot] [PATCH 0/4] Correct some remaining bootm problems

[Simon Glass]

The tortuous refactoring of bootm and fit_image_load() has thrown up
four more issues:

- Support for bootm (without arguments) on many OSes is broken since the
OS functions do not handle the PREP stage, which is now required
- In the case of loading a FIT image with a bootm-selected kernel
configuration, this is not used for ramdisk and fdt but must be manually
specified for each.
- The Elf image loader arguments are out by 1
- There is an extra OK message in the case of loading an uncompressed
kernel

This series corrects these problems. Verification was done using some
sandbox tests written for the occassion. These will be sent to the list
during the next merge window (or earlier if anyone wants to try them).

For the record the tests run are:

$ ./test/image/test-fit.py -u b/sandbox/u-boot
FIT Tests
=========
Kernel load
Kernel + FDT load
Kernel + FDT + Ramdisk load
Kernel + FDT + Ramdisk load, Config 2

Tests passed
Caveat: this is only a sanity check - test coverage is poor
$ ./test/image/test-legacy.py -u b/sandbox/u-boot
Legacy Image Tests
==================
Testing 22 image types
['netbsd', 'lynxos', 'rtems', 'ose', 'plan9', 'vxworks', 'qnx', 'integrity']
netbsd: False
netbsd: True
lynxos: False
lynxos: True
rtems: False
rtems: True
ose: False
ose: True
plan9: False
plan9: True
vxworks: False
vxworks: True
qnx: False
qnx: True
integrity: False
integrity: True

Tests passed
Caveat: this is only a sanity check - test coverage is poor


Simon Glass (4):
  bootm: Remove extra OK message
  blackfin: x86: bootm: Handle PREP stage of bootm
  bootm: Use selected configuration for ramdisk and fdt
  bootm: Correct the arguments for the ELF image loader

 arch/blackfin/lib/boot.c |  2 ++
 arch/x86/lib/bootm.c     |  2 ++
 common/cmd_bootm.c       | 20 ++++++++++++++++++--
 common/cmd_elf.c         |  6 +++---
 common/image-fdt.c       |  4 ++--
 common/image-fit.c       |  6 +++++-
 common/image.c           |  4 ++--
 include/image.h          |  7 ++++---
 8 files changed, 38 insertions(+), 13 deletions(-)

-- 
1.8.3

[U-Boot] [PATCH 1/4] bootm: Remove extra OK message

[Simon Glass]

This is not needed as we already print 'OK' later in all cases.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 common/cmd_bootm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index a783cea..7a7c760 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -396,7 +396,6 @@ static int bootm_load_os(bootm_headers_t *images, unsigned long *load_end,
 			memmove_wd(load_buf, image_buf, image_len, CHUNKSZ);
 		}
 		*load_end = load + image_len;
-		puts("OK\n");
 		break;
 #ifdef CONFIG_GZIP
 	case IH_COMP_GZIP:
-- 
1.8.3

[U-Boot] [PATCH 2/4] blackfin: x86: bootm: Handle PREP stage of bootm

[Simon Glass]

The OS function is now always called with the PREP stage. Adjust the
remaining bootm OS functions to deal with this correctly.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/blackfin/lib/boot.c |  2 ++
 arch/x86/lib/bootm.c     |  2 ++
 common/cmd_bootm.c       | 16 ++++++++++++++++
 3 files changed, 20 insertions(+)

diff --git a/arch/blackfin/lib/boot.c b/arch/blackfin/lib/boot.c
index 768a882..5644d58 100644
--- a/arch/blackfin/lib/boot.c
+++ b/arch/blackfin/lib/boot.c
@@ -42,6 +42,8 @@ int do_bootm_linux(int flag, int argc, char * const argv[], bootm_headers_t *ima
 	int	(*appl) (char *cmdline);
 	char	*cmdline;
 
+	if (flag & BOOTM_STATE_OS_PREP)
+		return 0;
 	if ((flag != 0) && (flag != BOOTM_STATE_OS_GO))
 		return 1;
 
diff --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c
index 0d3250c..b84e35a 100644
--- a/arch/x86/lib/bootm.c
+++ b/arch/x86/lib/bootm.c
@@ -48,6 +48,8 @@ int do_bootm_linux(int flag, int argc, char * const argv[],
 	size_t		len;
 #endif
 
+	if (flag & BOOTM_STATE_OS_PREP)
+		return 0;
 	if ((flag != 0) && (flag != BOOTM_STATE_OS_GO))
 		return 1;
 
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index 7a7c760..075e7dc 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -1473,6 +1473,8 @@ static int do_bootm_netbsd(int flag, int argc, char * const argv[],
 	char *consdev;
 	char *cmdline;
 
+	if (flag & BOOTM_STATE_OS_PREP)
+		return 0;
 	if ((flag != 0) && (flag != BOOTM_STATE_OS_GO))
 		return 1;
 
@@ -1552,6 +1554,8 @@ static int do_bootm_lynxkdi(int flag, int argc, char * const argv[],
 {
 	image_header_t *hdr = &images->legacy_hdr_os_copy;
 
+	if (flag & BOOTM_STATE_OS_PREP)
+		return 0;
 	if ((flag != 0) && (flag != BOOTM_STATE_OS_GO))
 		return 1;
 
@@ -1574,6 +1578,8 @@ static int do_bootm_rtems(int flag, int argc, char * const argv[],
 {
 	void (*entry_point)(bd_t *);
 
+	if (flag & BOOTM_STATE_OS_PREP)
+		return 0;
 	if ((flag != 0) && (flag != BOOTM_STATE_OS_GO))
 		return 1;
 
@@ -1607,6 +1613,8 @@ static int do_bootm_ose(int flag, int argc, char * const argv[],
 {
 	void (*entry_point)(void);
 
+	if (flag & BOOTM_STATE_OS_PREP)
+		return 0;
 	if ((flag != 0) && (flag != BOOTM_STATE_OS_GO))
 		return 1;
 
@@ -1641,6 +1649,8 @@ static int do_bootm_plan9(int flag, int argc, char * const argv[],
 	void (*entry_point)(void);
 	char *s;
 
+	if (flag & BOOTM_STATE_OS_PREP)
+		return 0;
 	if ((flag != 0) && (flag != BOOTM_STATE_OS_GO))
 		return 1;
 
@@ -1688,6 +1698,8 @@ static int do_bootm_vxworks(int flag, int argc, char * const argv[],
 {
 	char str[80];
 
+	if (flag & BOOTM_STATE_OS_PREP)
+		return 0;
 	if ((flag != 0) && (flag != BOOTM_STATE_OS_GO))
 		return 1;
 
@@ -1711,6 +1723,8 @@ static int do_bootm_qnxelf(int flag, int argc, char * const argv[],
 	char *local_args[2];
 	char str[16];
 
+	if (flag & BOOTM_STATE_OS_PREP)
+		return 0;
 	if ((flag != 0) && (flag != BOOTM_STATE_OS_GO))
 		return 1;
 
@@ -1736,6 +1750,8 @@ static int do_bootm_integrity(int flag, int argc, char * const argv[],
 {
 	void (*entry_point)(void);
 
+	if (flag & BOOTM_STATE_OS_PREP)
+		return 0;
 	if ((flag != 0) && (flag != BOOTM_STATE_OS_GO))
 		return 1;
 
-- 
1.8.3

[U-Boot] [PATCH 3/4] bootm: Use selected configuration for ramdisk and fdt

[Simon Glass]

If a specific configuraion is selected by the bootm command, e.g. with
'bootm 84000000#recoveryconf' we must honour this for not just the kernel,
but also the ramdisk and FDT.

In the conversion to using a common fit_image_load() function for loading
images from FITs (commits a51ec63 and 53f375f) this feature was lost.
Reinstate it by passing the selected configuration back from
fit_image_load() to boot_get_kernel(), then use this configuration
(which is stored in images->fit_uname_cfg) in both boot_get_ramdisk()
and boot_get_fdt().

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 common/cmd_bootm.c | 3 ++-
 common/image-fdt.c | 4 ++--
 common/image-fit.c | 6 +++++-
 common/image.c     | 4 ++--
 include/image.h    | 7 ++++---
 5 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index 075e7dc..c8bb33e 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -989,7 +989,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
 	case IMAGE_FORMAT_FIT:
 		os_noffset = fit_image_load(images, FIT_KERNEL_PROP,
 				img_addr,
-				&fit_uname_kernel, fit_uname_config,
+				&fit_uname_kernel, &fit_uname_config,
 				IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
 				BOOTSTAGE_ID_FIT_KERNEL_START,
 				FIT_LOAD_IGNORED, os_data, os_len);
@@ -998,6 +998,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
 
 		images->fit_hdr_os = map_sysmem(img_addr, 0);
 		images->fit_uname_os = fit_uname_kernel;
+		images->fit_uname_cfg = fit_uname_config;
 		images->fit_noffset_os = os_noffset;
 		break;
 #endif
diff --git a/common/image-fdt.c b/common/image-fdt.c
index d99f444..203404a 100644
--- a/common/image-fdt.c
+++ b/common/image-fdt.c
@@ -243,7 +243,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
 	ulong		load, load_end;
 	void		*buf;
 #if defined(CONFIG_FIT)
-	const char	*fit_uname_config = NULL;
+	const char	*fit_uname_config = images->fit_uname_cfg;
 	const char	*fit_uname_fdt = NULL;
 	ulong		default_addr;
 	int		fdt_noffset;
@@ -367,7 +367,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
 				fdt_noffset = fit_image_load(images,
 					FIT_FDT_PROP,
 					fdt_addr, &fit_uname_fdt,
-					fit_uname_config,
+					&fit_uname_config,
 					arch, IH_TYPE_FLATDT,
 					BOOTSTAGE_ID_FIT_FDT_START,
 					FIT_LOAD_OPTIONAL, &load, &len);
diff --git a/common/image-fit.c b/common/image-fit.c
index b75e119..e28dd05 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -1478,12 +1478,13 @@ int fit_get_node_from_config(bootm_headers_t *images, const char *prop_name,
 }
 
 int fit_image_load(bootm_headers_t *images, const char *prop_name, ulong addr,
-		   const char **fit_unamep, const char *fit_uname_config,
+		   const char **fit_unamep, const char **fit_uname_configp,
 		   int arch, int image_type, int bootstage_id,
 		   enum fit_load_op load_op, ulong *datap, ulong *lenp)
 {
 	int cfg_noffset, noffset;
 	const char *fit_uname;
+	const char *fit_uname_config;
 	const void *fit;
 	const void *buf;
 	size_t size;
@@ -1493,6 +1494,7 @@ int fit_image_load(bootm_headers_t *images, const char *prop_name, ulong addr,
 
 	fit = map_sysmem(addr, 0);
 	fit_uname = fit_unamep ? *fit_unamep : NULL;
+	fit_uname_config = fit_uname_configp ? *fit_uname_configp : NULL;
 	printf("## Loading %s from FIT Image at %08lx ...\n", prop_name, addr);
 
 	bootstage_mark(bootstage_id + BOOTSTAGE_SUB_FORMAT);
@@ -1658,6 +1660,8 @@ int fit_image_load(bootm_headers_t *images, const char *prop_name, ulong addr,
 	*lenp = len;
 	if (fit_unamep)
 		*fit_unamep = (char *)fit_uname;
+	if (fit_uname_configp)
+		*fit_uname_configp = (char *)fit_uname_config;
 
 	return noffset;
 }
diff --git a/common/image.c b/common/image.c
index 1be384f..327006e 100644
--- a/common/image.c
+++ b/common/image.c
@@ -811,7 +811,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
 	char *end;
 #endif
 #if defined(CONFIG_FIT)
-	const char	*fit_uname_config = NULL;
+	const char	*fit_uname_config = images->fit_uname_cfg;
 	const char	*fit_uname_ramdisk = NULL;
 	ulong		default_addr;
 	int		rd_noffset;
@@ -907,7 +907,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
 		case IMAGE_FORMAT_FIT:
 			rd_noffset = fit_image_load(images, FIT_RAMDISK_PROP,
 					rd_addr, &fit_uname_ramdisk,
-					fit_uname_config, arch,
+					&fit_uname_config, arch,
 					IH_TYPE_RAMDISK,
 					BOOTSTAGE_ID_FIT_RD_START,
 					FIT_LOAD_REQUIRED, &rd_data, &rd_len);
diff --git a/include/image.h b/include/image.h
index 9c3e46f..7b0108f 100644
--- a/include/image.h
+++ b/include/image.h
@@ -439,8 +439,9 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
  * @param fit_unamep	On entry this is the requested image name
  *			(e.g. "kernel at 1") or NULL to use the default. On exit
  *			points to the selected image name
- * @param fit_uname_config	Requested configuration name, or NULL for the
- *			default
+ * @param fit_uname_configp	On entry this is the requested configuration
+ *			name (e.g. "conf at 1") or NULL to use the default. On
+ *			exit points to the selected configuration name.
  * @param arch		Expected architecture (IH_ARCH_...)
  * @param image_type	Required image type (IH_TYPE_...). If this is
  *			IH_TYPE_KERNEL then we allow IH_TYPE_KERNEL_NOLOAD
@@ -453,7 +454,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
  * @param lenp		Returns length of loaded image
  */
 int fit_image_load(bootm_headers_t *images, const char *prop_name, ulong addr,
-		   const char **fit_unamep, const char *fit_uname_config,
+		   const char **fit_unamep, const char **fit_uname_configp,
 		   int arch, int image_type, int bootstage_id,
 		   enum fit_load_op load_op, ulong *datap, ulong *lenp);
 
-- 
1.8.3

[U-Boot] [PATCH 4/4] bootm: Correct the arguments for the ELF image loader

[Simon Glass]

The arguments were out of place since commit 983c72f, since this file was
missed and not tested. Correct this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 common/cmd_elf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/common/cmd_elf.c b/common/cmd_elf.c
index ab9c7e3..f741f6b 100644
--- a/common/cmd_elf.c
+++ b/common/cmd_elf.c
@@ -156,16 +156,16 @@ int do_bootvx(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	 * If we don't know where the image is then we're done.
 	 */
 
-	if (argc < 2)
+	if (argc < 1)
 		addr = load_addr;
 	else
-		addr = simple_strtoul(argv[1], NULL, 16);
+		addr = simple_strtoul(argv[0], NULL, 16);
 
 #if defined(CONFIG_CMD_NET)
 	/*
 	 * Check to see if we need to tftp the image ourselves before starting
 	 */
-	if ((argc == 2) && (strcmp(argv[1], "tftp") == 0)) {
+	if ((argc == 1) && (strcmp(argv[0], "tftp") == 0)) {
 		if (NetLoop(TFTPGET) <= 0)
 			return 1;
 		printf("Automatic boot of VxWorks image@address 0x%08lx ...\n",
-- 
1.8.3

[U-Boot] [PATCH 2/4] blackfin: x86: bootm: Handle PREP stage of bootm

[Sughosh Ganu]

On Wed Jul 10, 2013 at 11:08:09PM -0700, Simon Glass wrote:
> The OS function is now always called with the PREP stage. Adjust the
> remaining bootm OS functions to deal with this correctly.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  arch/blackfin/lib/boot.c |  2 ++
>  arch/x86/lib/bootm.c     |  2 ++
>  common/cmd_bootm.c       | 16 ++++++++++++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/arch/blackfin/lib/boot.c b/arch/blackfin/lib/boot.c
> index 768a882..5644d58 100644
> --- a/arch/blackfin/lib/boot.c
> +++ b/arch/blackfin/lib/boot.c
> @@ -42,6 +42,8 @@ int do_bootm_linux(int flag, int argc, char * const argv[], bootm_headers_t *ima
>  	int	(*appl) (char *cmdline);
>  	char	*cmdline;
>  
> +	if (flag & BOOTM_STATE_OS_PREP)
> +		return 0;
>  	if ((flag != 0) && (flag != BOOTM_STATE_OS_GO))
>  		return 1;

Minor nit; A blank line between the two if conditions would be
good. Applies for all the hunks in this patch.

-sughosh

[U-Boot] [PATCH 0/4] Correct some remaining bootm problems

[Tom Rini]

On Wed, Jul 10, 2013 at 11:08:07PM -0700, Simon Glass wrote:

> The tortuous refactoring of bootm and fit_image_load() has thrown up
> four more issues:
> 
> - Support for bootm (without arguments) on many OSes is broken since the
> OS functions do not handle the PREP stage, which is now required
> - In the case of loading a FIT image with a bootm-selected kernel
> configuration, this is not used for ramdisk and fdt but must be manually
> specified for each.
> - The Elf image loader arguments are out by 1
> - There is an extra OK message in the case of loading an uncompressed
> kernel
> 
> This series corrects these problems. Verification was done using some
> sandbox tests written for the occassion. These will be sent to the list
> during the next merge window (or earlier if anyone wants to try them).

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130712/0a7b08fd/attachment.pgp>