[U-Boot] [PATCH] sunxi: Add the ability to pass (script) filesize in the SPL header

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

Hello Bernhard,

On Sat,  4 Jun 2016 19:12:20 +0200
Bernhard Nortmann <bernhard.nortmann@web.de> wrote:

> Convert one of the "reserved" fields in the sunxi SPL header to
> a fel_script_length entry. That enables the sunxi-fel utility
> to pass the script size when booting over USB ("FEL mode").
> 
> If board.c encounters a non-zero value in this header field, it
> will set U-Boot's "filesize" environment variable accordingly.
> 
> sunxi-fel currently doesn't use this field (i.e. fel_script_length
> will remain 0), but it would allow for new use cases, e.g. passing
> tweaked/additional settings via a text file (uEnv.txt style), and
> then using "import -t ${fel_script_addr} ${filesize}" on them.

How does this work in general with "boot.scr" and "uEnv.txt" use
cases? Could you provide a bit more detailed description?

I mean, who is going to do "import -t ${fel_script_addr} ${filesize}"
invocation?

This particular FEL feature in the SPL header is used to allow running
a boot script provided by the user (boot.scr). Without it, we only
have the default U-Boot environment. And the default U-Boot environment
does not have the "import -t ${fel_script_addr} ${filesize}" statement
yet. This looks a bit like a chicken/egg problem or am I missing
something?

> 
> Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
> 
> ---
> 
>  arch/arm/include/asm/arch-sunxi/spl.h | 3 ++-
>  board/sunxi/board.c                   | 8 +++++++-
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/spl.h b/arch/arm/include/asm/arch-sunxi/spl.h
> index a0f33b0..0e18438 100644
> --- a/arch/arm/include/asm/arch-sunxi/spl.h
> +++ b/arch/arm/include/asm/arch-sunxi/spl.h
> @@ -49,7 +49,8 @@ struct boot_file_head {
>  		uint8_t spl_signature[4];
>  	};
>  	uint32_t fel_script_address;
> -	uint32_t reserved1[3];
> +	uint32_t fel_script_length;
> +	uint32_t reserved1[2];
>  	uint32_t boot_media;		/* written here by the boot ROM */
>  	uint32_t reserved2[5];		/* padding, align to 64 bytes */
>  };
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index d09cf6d..cf0ff33 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -585,9 +585,15 @@ static void parse_spl_header(const uint32_t spl_addr)
>  	if (memcmp(spl->spl_signature, SPL_SIGNATURE, 3) == 0) {
>  		uint8_t spl_header_version = spl->spl_signature[3];
>  		if (spl_header_version == SPL_HEADER_VERSION) {
> -			if (spl->fel_script_address)
> +			if (spl->fel_script_address) {
>  				setenv_hex("fel_scriptaddr",
>  					   spl->fel_script_address);
> +				if (spl->fel_script_length)
> +					setenv_hex("filesize",
> +						   spl->fel_script_length);
> +				else
> +					setenv("filesize", NULL);

I have no real opinion about this, but "filesize" looks like a
rather generic name for this environment variable. Are there some
advantages/disadvantages picking this particular name instead
of something like "fel_scriptsize"?

> +			}
>  			return;
>  		}
>  		printf("sunxi SPL version mismatch: expected %u, got %u\n",

That said, I have no objections to supporting "uEnv.txt" for FEL boot,
as long as it works correctly and does not regress the existing
"boot.scr" support.

-- 
Best regards,
Siarhei Siamashka