[U-Boot] [PATCH v2 2/3] sunxi: retrieve FEL-provided values to environment variables

[Siarhei Siamashka <siarhei.siamashka@gmail.com>]

On Mon, 14 Sep 2015 15:15:29 +0200
Bernhard Nortmann <bernhard.nortmann@web.de> wrote:

> This patch extends the misc_init_r() function on sunxi boards
> to test for the presence of a suitable "sunxi" SPL header. If
> found, and the loader ("fel" utility) provided a non-zero value
> for the boot.scr address, then the corresponding environment
> variable fel_scriptaddr gets set.
> 
> misc_init_r() also sets (or clears) the "fel_booted" variable depending
> on the active boot device, using the same logic as spl_boot_device().
> 
> The goal is to provide sufficient information (within the U-Boot
> environment) to make intelligent decisions on how to continue the boot
> process, allowing specific customizations for the "FEL boot" case.
> 
> Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
> ---
> 
> Changes in v2:
> - renamed fel_data_addr to fel_script_addr, discarded fel_data_size
> - make sure that FEL-related environment vars are always cleared first
> - support minimum and maximum SPL (header) version, more verbose error messages
> 
>  board/sunxi/board.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 9c855f6..aa26e57 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -516,6 +516,52 @@ void get_board_serial(struct tag_serialnr *serialnr)
>  }
>  #endif
>  
> +#if !defined(CONFIG_SPL_BUILD)
> +static int check_signature(unsigned long io_addr, const char *signature,
> +			   int length)
> +{
> +	do {
> +		if (readb(io_addr) != *signature)
> +			return 0;
> +		io_addr++;
> +		signature++;
> +	} while (--length > 0);

It is probably better to just use memcmp() instead of this loop.

Admittedly, I'm responsible for abusing readl() in the old code,
because it allowed to fit the FEL mode check into just a single line
(the pointer casts would make it exceed the 80 characters limit):

     if (readl(4) != 0x4E4F4765 || readl(8) != 0x3054422E) { /* eGON.BT0 */

However now this readb() usage seems to do exactly the opposite and only
inflates the code unnecessarily.

> +	return 1;
> +}
>
> +#define SPL_SIGNATURE			"SPL" /* marks "sunxi" header */
> +#define SPL_MIN_VERSION			1
> +#define SPL_MAX_VERSION			1

Can we have a way to share these defines between "board/sunxi/board.c"
and "tools/mksunxiboot.c"?

> +/*
> + * Check the SPL header for the "sunxi" variant. If found: parse values
> + * that might have been passed by the loader ("fel" utility), and update
> + * the environment accordingly.
> + */
> +static void parse_spl_header(void)
> +{
> +	uint8_t spl_header_version;
> +	uint32_t fel_script_addr;
> +
> +	if (check_signature(0x14, SPL_SIGNATURE, 3)) {
> +		spl_header_version = readb(0x17);
> +		if (spl_header_version < SPL_MIN_VERSION) {
> +			printf("sunxi SPL version mismatch: found 0x%02X < required minimum 0x%02X\n",
> +			       spl_header_version, SPL_MIN_VERSION);
> +			return;
> +		}
> +		if (spl_header_version > SPL_MAX_VERSION) {
> +			printf("sunxi SPL version mismatch: found 0x%02X > maximum supported 0x%02X\n",
> +			       spl_header_version, SPL_MAX_VERSION);
> +			return;
> +		}

Yes, having this signature check before extracting the information from
the SPL header is a good idea. Because we can have Allwinner's boot0
used together with the main U-Boot binary (for the SoC variants, which
do not have SPL support yet).

But here it is probably better to just expect the exact SPL header
version match? The SPL and the main U-Boot binary are usually both
built together from the same sources and combined into a single
u-boot-sunxi-with-spl.bin file.

> +		fel_script_addr = readl(0x18);
> +		if (fel_script_addr)
> +			setenv_hex("fel_scriptaddr", fel_script_addr);
> +	}
> +}
> +#endif /* !defined(CONFIG_SPL_BUILD) */
> +
>  #ifdef CONFIG_MISC_INIT_R
>  int misc_init_r(void)
>  {
> @@ -524,6 +570,16 @@ int misc_init_r(void)
>  	uint8_t mac_addr[6];
>  	int ret;
>  
> +#if !defined(CONFIG_SPL_BUILD)
> +	setenv("fel_booted", NULL);
> +	setenv("fel_scriptaddr", NULL);
> +	/* determine if we are running in FEL mode */
> +	if (readl(4) != 0x4E4F4765 || readl(8) != 0x3054422E) { /* eGON.BT0 */
> +		setenv("fel_booted", "1");
> +		parse_spl_header();
> +	}
> +#endif
> +
>  	ret = sunxi_get_sid(sid);
>  	if (ret == 0 && sid[0] != 0 && sid[3] != 0) {
>  		if (!getenv("ethaddr")) {


-- 
Best regards,
Siarhei Siamashka