On Tuesday 06 December 2011 13:34:35 Simon Schwarz wrote: > --- /dev/null > +++ b/common/cmd_spl.c > > +int call_bootm(int argc, char * const argv[], char *subcommand[])
static
> +int spl_export_fdt(int argc, char * const argv[])
static
> +#ifdef CONFIG_OF_LIBFDT
> + /* Create subcommand string */
> + char *subcommand[] = {"start",
that start needs to be on a new line
> + '\0'};
if this were NULL (and the call_bootm() checked for that) would be more
natural to argv[] processing
> +int spl_export_atags(int argc, char * const argv[])
static
> + char *subcommand[] = {"start", "loados",
> +#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
> + "ramdisk",
> +#endif
> + "cmdline", "bdt", "prep", '\0'};
char *subcommand[] = {
... some strings ...
... some more strings ...
};
> +int spl_export(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
static
> +{
> +
delete that newline
> + cmd_tbl_t *c;
i think you can const this ...
> +int do_spl(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
static
> + cmd_tbl_t *c;
i think you can const this ...
> + cmd = (int)c->cmd;
you're casting a pointer to an integer ? wtf is going on ?
> --- /dev/null
> +++ b/include/cmd_spl.h
needs #ifdef protection against multiple inclusion
> +extern bootm_headers_t images;
i get the feeling this isn't the right place for this and it should be in
include/image.h instead ...
> +enum image_type {FDT, ATAGS};
these names are too short, and the "image" namespace is taken by image.h
already ... but leading on to the following defines ...
> +#define SPL_EXPORT (0x00000001)
> +
> +#define SPL_EXPORT_FDT (0x00000001)
> +#define SPL_EXPORT_ATAGS (0x00000002)
why do these need to be defines ?
enum spl_export_type {
SPL_EXPORT_FDT = 1,
SPL_EXPORT_ATAGS = 2,
};
also, drop the paren here
> --- a/include/configs/devkit8000.h
> +++ b/include/configs/devkit8000.h
generally board updates should be a sep commit
-mike
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
