[U-Boot] [PATCH] BOOT: Add RAW ramdisk support to bootz

This patch allows loading RAW ramdisk via bootz command. The raw ramdisk is loaded only in case it's size is specified:
bootz <kernel addr> <ramdisk addr>:<ramdisk size> <fdt addr>
For example:
bootz 0x42000000 0x43000000:0x12345 0x44000000
Signed-off-by: Marek Vasut marex@denx.de Cc: Tom Warren TWarren@nvidia.com Cc: albert.u.boot@aribaud.net Cc: afleming@gmail.com, Cc: Simon Glass sjg@chromium.org, Cc: Stephen Warren swarren@nvidia.com Cc: Nicolas Pitre nico@fluxnic.net Cc: Wolfgang Denk wd@denx.de Cc: Detlev Zundel dzu@denx.de --- common/cmd_bootm.c | 6 ++++-- common/image.c | 25 +++++++++++++++++++++---- 2 files changed, 25 insertions(+), 6 deletions(-)
NOTE: This patch depends on previous zImage/bootz patch. Testers are very welcome!
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 9efac8b..872a49c 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -1628,9 +1628,11 @@ static int do_bootz(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) U_BOOT_CMD( bootz, CONFIG_SYS_MAXARGS, 1, do_bootz, "boot Linux zImage image from memory", - "[addr [initrd] [fdt]]\n - boot Linux zImage stored in memory\n" + "[addr [initrd[:size]] [fdt]]\n" + " - boot Linux zImage stored in memory\n" "\tThe argument 'initrd' is optional and specifies the address\n" - "\tof the initrd in memory.\n" + "\tof the initrd in memory. The optional argument ':size' allows\n" + "\tspecifying the size of RAW initrd.\n" #if defined(CONFIG_OF_LIBFDT) "\tWhen booting a Linux kernel which requires a flat device-tree\n" "\ta third argument is required which is the address of the\n" diff --git a/common/image.c b/common/image.c index 95c7a15..e9d87d8 100644 --- a/common/image.c +++ b/common/image.c @@ -797,6 +797,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images, ulong rd_addr, rd_load; ulong rd_data, rd_len; const image_header_t *rd_hdr; + char *end; #if defined(CONFIG_FIT) void *fit_hdr; const char *fit_uname_config = NULL; @@ -845,10 +846,18 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images, } else #endif { - rd_addr = simple_strtoul(argv[2], NULL, 16); + rd_addr = simple_strtoul(argv[2], &end, 16); debug("* ramdisk: cmdline image address = " "0x%08lx\n", rd_addr); + + if (end[0] == ':') { + rd_len = simple_strtoul(++end, + NULL, 16); + debug("* ramdisk: cmdline image " + "length = 0x%08lx\n", + rd_len); + } } #if defined(CONFIG_FIT) } else { @@ -990,9 +999,17 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images, break; #endif default: - puts("Wrong Ramdisk Image Format\n"); - rd_data = rd_len = rd_load = 0; - return 1; + /* + * Check if rd_len was manually overridden, if it was, + * we're loading RAW ramdisk. + */ + if (rd_len != 0) { + rd_data = rd_addr; + } else { + puts("Wrong Ramdisk Image Format\n"); + rd_data = rd_len = rd_load = 0; + return 1; + } } } else if (images->legacy_hdr_valid && image_check_type(&images->legacy_hdr_os_copy,

Dear Marek Vasut,
In message 1331857196-29512-1-git-send-email-marex@denx.de you wrote:
This patch allows loading RAW ramdisk via bootz command. The raw ramdisk is loaded only in case it's size is specified:
bootz <kernel addr> <ramdisk addr>:<ramdisk size> <fdt addr>
For example:
bootz 0x42000000 0x43000000:0x12345 0x44000000
...
--- a/common/image.c +++ b/common/image.c @@ -797,6 +797,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images, ulong rd_addr, rd_load; ulong rd_data, rd_len; const image_header_t *rd_hdr;
- char *end;
#if defined(CONFIG_FIT) void *fit_hdr; const char *fit_uname_config = NULL; @@ -845,10 +846,18 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images, } else #endif {
rd_addr = simple_strtoul(argv[2], NULL, 16);
rd_addr = simple_strtoul(argv[2], &end, 16); debug("* ramdisk: cmdline image address = " "0x%08lx\n", rd_addr);
if (end[0] == ':') {
rd_len = simple_strtoul(++end,
NULL, 16);
debug("* ramdisk: cmdline image "
"length = 0x%08lx\n",
rd_len);
}
This is common code, which gets used not only for bootz.
I see a number of problems:
- You add code size even for systems which don't enable "bootz" support.
- You change the syntax of all boot related commands where a ramdisk address may be passed (i. e. "bootm") without documenting it.
- Using this syntax with a mkimage wrapped ramdisk image will cause bad things to happen, even if you specific the correct size. The API should be more robust.
/*
* Check if rd_len was manually overridden, if it was,
* we're loading RAW ramdisk.
*/
This is undocumented policy, and I dislike such a side-effect based design (if we have a size parameter, it must be a raw image).
Best regards,
Wolfgang Denk

Dear Wolfgang Denk,
Dear Marek Vasut,
In message 1331857196-29512-1-git-send-email-marex@denx.de you wrote:
This patch allows loading RAW ramdisk via bootz command. The raw ramdisk is
loaded only in case it's size is specified: bootz <kernel addr> <ramdisk addr>:<ramdisk size> <fdt addr>
For example: bootz 0x42000000 0x43000000:0x12345 0x44000000
...
--- a/common/image.c +++ b/common/image.c @@ -797,6 +797,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
ulong rd_addr, rd_load; ulong rd_data, rd_len; const image_header_t *rd_hdr;
- char *end;
#if defined(CONFIG_FIT)
void *fit_hdr; const char *fit_uname_config = NULL;
@@ -845,10 +846,18 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
} else
#endif
{
rd_addr = simple_strtoul(argv[2], NULL, 16);
rd_addr = simple_strtoul(argv[2], &end, 16); debug("* ramdisk: cmdline image address = " "0x%08lx\n", rd_addr);
if (end[0] == ':') {
rd_len = simple_strtoul(++end,
NULL, 16);
debug("* ramdisk: cmdline image "
"length = 0x%08lx\n",
rd_len);
}
This is common code, which gets used not only for bootz.
I see a number of problems:
- You add code size even for systems which don't enable "bootz" support.
This is a good point, do you agree to introduce CONFIG_SUPPORT_RAW_RAMDISK to enable this feature?
- You change the syntax of all boot related commands where a ramdisk address may be passed (i. e. "bootm") without documenting it.
Right, this should be documented in the top-level README file? Or is there some other place?
- Using this syntax with a mkimage wrapped ramdisk image will cause bad things to happen, even if you specific the correct size. The API should be more robust.
It actually won't. If you use mkimage warped ramdisk, it'll recognise it's magic numbers and simply ignore the stuff behind ":". Though I hope noone in sane mind would do that.
/*
* Check if rd_len was manually overridden, if it was,
* we're loading RAW ramdisk.
*/
This is undocumented policy, and I dislike such a side-effect based design (if we have a size parameter, it must be a raw image).
It must not, but if it is, it'll be used as raw.
Best regards,
Wolfgang Denk
Thanks for good ideas!
Best regards, Marek Vasut

Dear Marek Vasut,
In message 201203160945.40467.marex@denx.de you wrote:
- You add code size even for systems which don't enable "bootz" support.
This is a good point, do you agree to introduce CONFIG_SUPPORT_RAW_RAMDISK to enable this feature?
Fine with me - please also document the new option.
- You change the syntax of all boot related commands where a ramdisk address may be passed (i. e. "bootm") without documenting it.
Right, this should be documented in the top-level README file? Or is there some other place?
Hm... I have no strong opinion here.
- Using this syntax with a mkimage wrapped ramdisk image will cause bad things to happen, even if you specific the correct size. The API should be more robust.
It actually won't. If you use mkimage warped ramdisk, it'll recognise it's magic numbers and simply ignore the stuff behind ":". Though I hope noone in sane mind would do that.
Argh... Passing arguments that silently get ignored under certain conditions is not something I like to see.
And are you sure hat it will work like that? [I mean, has this been tested?]
/*
* Check if rd_len was manually overridden, if it was,
* we're loading RAW ramdisk.
*/
This is undocumented policy, and I dislike such a side-effect based design (if we have a size parameter, it must be a raw image).
It must not, but if it is, it'll be used as raw.
Sorry, I cannot parse this.
Best regards,
Wolfgang Denk
participants (2)
-
Marek Vasut
-
Wolfgang Denk