From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jerry Van Baren <gerald.vanbaren@ge.com>
Date: Thu, 18 Sep 2008 07:25:36 -0400
Subject: [U-Boot] [RFC] bootm: Add sub commands
In-Reply-To: <20080918065428.1E84C248C6@gemini.denx.de>
References: <1221688841-3197-1-git-send-email-galak@kernel.crashing.org>
	<20080918065428.1E84C248C6@gemini.denx.de>
Message-ID: <48D23AB0.9050301@ge.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Wolfgang Denk wrote:
> Dear Kumar Gala,
> 
> In message <1221688841-3197-1-git-send-email-galak@kernel.crashing.org> you wrote:
>> Having bootm sub-commands allows both of these as we can break up
>> the sequeunce of steps that are part of the bootm process.
> 
> OK.
> 
>> +int do_bootm_subcommand (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>> +{
>> +	int ret = 0;
>> +
>> +	/* start */
>> +	if (argv[1][0] == 's') {
> 
> I think just matching on the first letter is to restrictive; we'll end
> up with artificical command "names" that nobody can remember.

That is what the all the other command subcommands do: match on the 
minimum unique sequence, preferably one letter.

>> +	else if (argv[1][0] == 'i') {
> 
> And we need comments what all this means. WTF is 'i' ?

Ich in German :-D  (That is a bad cross-lingual pun: "ick" is what you 
say in English when you step in dog poo.)

You snipped the preceding comment line.  Granted, it was pretty terse:
> +	/* initrd relocate */

The "help" string helps:

> @@ -782,6 +883,17 @@ U_BOOT_CMD(
>  	"\tUse iminfo command to get the list of existing component\n"
>  	"\timages and configurations.\n"
>  #endif
> +	"\t\nSub-commands to do part of the bootm sequence:\n"
> +	"\tstart [addr [arg ...]]\n"
> +	"\tloados - load OS image\n"
> +	"\tprepos - OS specific prep before relocation or go\n"
> +#if defined(CONFIG_PPC) || defined(CONFIG_M68K) || defined(CONFIG_SPARC)
> +	"\tinitrd - relocate initrd, set env initrd_start/initrd_end\n"
> +#endif
> +#if defined(CONFIG_OF_LIBFDT)
> +	"\tfdt - relocate initrd\n"
> +#endif
> +	"\tgo - start os\n"
>  );

[snip]

> Best regards,
> 
> Wolfgang Denk

Ditto,
gvb