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