[U-Boot] [PATCH V9 1/4] Add cmd_spl command

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20111207/b375c51a/attachment.pgp>