Dear Marek Vasut, In message <[email protected]> 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 -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [email protected] Do you suppose the reason the ends of the `Intel Inside' logo don't match up is that it was drawn on a Pentium? _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
