[U-Boot] [PATCH] ARM: rpi: Add CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG support

[Stephen Warren <swarren@wwwdotorg.org>]

On 08/18/2015 08:03 AM, Guillaume GARDET wrote:
> Add CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG support to set 'board_rev' and 
> 'board_name' envs.

That states what the patch does rather than why its useful to do it. Can
you expand on why it's useful to set these variables?

> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c

> @@ -240,6 +240,12 @@ int misc_init_r(void)
>  {
>  	set_fdtfile();
>  	set_usbethaddr();
> +#ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
> +	char str_rev[11];
> +	sprintf(str_rev, "0x%X", rpi_board_rev);
> +	setenv("board_rev", str_rev);
> +	setenv("board_name", models[rpi_board_rev].name);
> +#endif
>  	return 0;
>  }

That adds a variable declaration in the middle of code. I'd suggest
moving the new code into set_board_info() (a name that some other
boards/SoCs that honor CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG use), and
calling that function inside the ifdef in misc_init_r(). You can always
make the function static and implement it before misc_init_r() so that
the compiler is likely to inline it.

I'm not sure that models[rpi_board_rev].name contains values that are
useful to place into an environment variable. It depends what you expect
to do with that variable. Note that the values are not unique, and
contain spaces, which might make the value hard to use and/or not
reliable to differentiate between all the different types of boards.

Conceptually I have no general objection to this patch, although I am a
little worried about turning the board_name variable into some kind of ABI.