[U-Boot] [PATCH v2 1/2] Add README for the "Falcon" mode

[Stefano Babic]

On 13/11/2012 16:55, Andreas Bießmann wrote:

>> +CONFIG_SPL_OS_BOOT	Activate Falcon mode.
>> +			A board should implement the following functions:
> 
> I think reordering this list to have the required functions directly
> after the colon would be helpful. Alternatively add a pointer to the
> list of functions after the list of defines.

Yes, it is what I wanted to do. In fact, the phrase now before the colon
seems incomplete, because the functions are missing. I will this entry
at the end of the define, so that the functions will follow after it.

> 
>> +spl - SPL configuration
>> +
>> +Usage:
>> +
>> +spl export <img=atags|fdt> [kernel_addr] [fdt_addr if <img> = fdt] - export a kernel parameter image
>> +
>> +img		: "atags" or "fdt"
>> +kernel_addr	: kernel is loaded as part of the boot process, but it is not started.
>> +		  This is the address where a kernel image is stored.
>> +fdt_addr	: in case of fdt, the address of the device tree.
>> +
>> +
>> +The spl puts its result at a self gained position. The position is defined at compile
>> +time or when generating the uImage but not at command line for 'spl export'
>> +(see spl_export(): gd->bd->bi_boot_params vs. images.ft_addr).
> 
> I think you got me wrong by my last remarks about the usage() of 'spl
> export'.

Probably...

> 
> 
> First of all there is presumably a typo in the descriptive text.
> 
> The current code has this usage():
> ---8<---
> 	"export <img=atags|fdt> [kernel_addr] [initrd_addr] "
> 	"[fdt_addr if <img> = fdt] - export a kernel parameter image\n"
> 	"\t initrd_img can be set to \"-\" if fdt_addr without initrd img is"
> 	"used")
> --->8---
> The arguments say something about 'initrd_addr' but the descriptive text
> has 'initrd_img'. I think (but do not know since I have not used the spl
> command so often) that the 'initrd_img' should be written 'initrd_addr'
> (cc Simon Schwarz for that).

Ok, I cannot avoid to fix this typo the command with a separate patch.
Else documentation and code remain inconsistent.

> 
> The second thing in my last review of your patch was maybe shortly
> described. You did write there the correct usage() of 'spl export' but
> later on add a more descriptive text about the parameters of 'spl
> export'. In these descriptive text you did write 'init_addr' instead of
> 'initrd_addr' - that was my comment about.

Ok, it seems I misunderstood. But I think your comment about where spl
stores the result is helpful, even if the address is printed by the
command itself.

> Additionally I thought your descriptive text is way better than the
> current descriptive text of 'spl export', so I asked to add your
> description to the command.

Ok - I will put it into the spl help as well.

> 
> Now the main reason for my writing here ;)
> Your usage() of 'spl export' some lines above is wrong cause it is
> missing the 'initrd_addr' parameter.


Correct - to be fixed ;-)

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================