[U-Boot] [PATCH 2/2] odroid: Add boot script (boot.scr) support

[Przemyslaw Marczak <p.marczak@samsung.com>]

Hello Guillaume,

On 09/28/2015 11:42 AM, Guillaume GARDET wrote:

Please, add commit message.

> Signed-off-by: Guillaume GARDET <guillaume.gardet@free.fr>
> Cc: Przemyslaw Marczak <p.marczak@samsung.com>
> Cc: Minkyu Kang <mk7.kang@samsung.com>
>
> ---
>   include/configs/odroid.h | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/include/configs/odroid.h b/include/configs/odroid.h
> index e45b00e..cc9f818 100644
> --- a/include/configs/odroid.h
> +++ b/include/configs/odroid.h
> @@ -108,6 +108,8 @@
>    * 2.  ROOT:  -
>   */
>   #define CONFIG_EXTRA_ENV_SETTINGS \
> +	"loadbootscript=load mmc ${mmcbootdev}:${mmcbootpart} ${loadaddr} " \
> +		"boot.scr\0" \
>   	"loadkernel=load mmc ${mmcbootdev}:${mmcbootpart} ${kerneladdr} " \
>   		"${kernelname}\0" \
>   	"loadinitrd=load mmc ${mmcbootdev}:${mmcbootpart} ${initrdaddr} " \
> @@ -129,6 +131,8 @@
>   	"kernel_args=" \
>   		"setenv bootargs root=/dev/mmcblk${mmcrootdev}p${mmcrootpart}" \
>   		" rootwait ${console} ${opts}\0" \

Please keep the code consistency, use boot_script, like boot_zimg, 
boot_fit, etc...

I think, that we don't need echoing such information since we usually 
read the script from one place, and the "source" prints info, that 
script is executing.

Please start command with the new line, it looks better :)

"boot_script =" \
	"run loadbootscript;" \
	"source ...

> +	"bootscript=echo Running bootscript from mmc${mmcbootdev}:${mmcbootpart} ...; " \
> +		"source ${loadaddr}\0" \
>   	"boot_fit=" \
>   		"setenv kerneladdr 0x42000000;" \
>   		"setenv kernelname Image.itb;" \
> @@ -152,6 +156,9 @@
>   		"run kernel_args;" \
>   		"bootz ${kerneladdr} ${initrd_addr} ${fdt_addr};\0" \
>   	"autoboot=" \

Also in this place, please use consistent code:

"if test -e mmc 0 boot.scr; then; " \
	"run bootscript; " \
"elif test -e mmc 0 Image.itb; then; " \

Using load command without checking before, if file exists with: "if 
test -e...", causes printing error to console - we don't like this :)

> +		"if run loadbootscript; then " \
> +			"run bootscript; " \
> +		"fi; " \
>   		"if test -e mmc 0 Image.itb; then; " \
>   			"run boot_fit;" \
>   		"elif test -e mmc 0 zImage; then; " \
> @@ -171,6 +178,7 @@
>   	"consoleoff=set console console=ram; save; reset\0" \
>   	"initrdname=uInitrd\0" \
>   	"initrdaddr=42000000\0" \
> +	"loadaddr=0x42000000\0" \

Generally we load everything, so please use more precisely name, like:

"scriptaddr=0x42000000\0" \

>   	"fdtaddr=40800000\0"
>
>   /* I2C */
>

Best regards,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com