[U-Boot] [PATCH v2] sunxi: Add the ability to recognize and auto-import uEnv-style data

[Siarhei Siamashka]

Hi,

On Wed, 8 Jun 2016 22:13:50 +0200
Hans de Goede <hdegoede at redhat.com> wrote:

> Hi,
> 
> On 08-06-16 20:23, Bernhard Nortmann wrote:
> > The patch converts one of the "reserved" fields in the sunxi SPL
> > header to a fel_uEnv_length entry. When booting over USB ("FEL
> > mode"), this enables the sunxi-fel utility to pass the string
> > length of uEnv.txt compatible data; at the same time requesting
> > that this data be imported into the U-Boot environment.
> >
> > If parse_spl_header() in the sunxi board.c encounters a non-zero
> > value in this header field, it will therefore call himport_r() to
> > merge the string (lines) passed via FEL into the default settings.
> > Environment vars can be changed this way even before U-Boot will
> > attempt to autoboot - specifically, this also allows overriding
> > "bootcmd".
> >
> > With fel_script_addr set and a zero fel_uEnv_length, U-Boot is
> > safe to assume that data in .scr format (a mkimage-type script)
> > was passed at fel_script_addr, and will handle it using the
> > existing mechanism ("bootcmd_fel").
> >
> > Signed-off-by: Bernhard Nortmann <bernhard.nortmann at web.de>  
> 
> This patch looks good to me.
> 
> Siarhei any comments from your side ? If not then I'll add this to
> u-boot-sunxi/next.

Yes, it also looks good to me:
Acked-by: Siarhei Siamashka <siarhei.siamashka at gmail.com>

The only nitpick is about the subject line. It does not mention
FEL and may be a bit misleading.


Regarding the uEnv.txt support in general, I tried to grep U-Boot
sources and found that it's use is very much inconsistent on
different platforms. For example, there seems to be some sort
of leftover junk in sunxi-common.h:

http://git.denx.de/?p=u-boot.git;a=blob;f=include/configs/sunxi-common.h;h=b33cfb86f82e0831f5d19b1e473205f65efb5a96;hb=b104b3dc1dd90cdbf67ccf3c51b06e4f1592fe91#l481

Hardcoding the partition number and the file system type is hardly
something that the users may reasonably expect.

It also might be a good idea to do more or less uniform handling of
the uEnv.txt for both FEL boot and normal SD card boot. If Bernhard
wants to improve the uEnv.txt support in general, then could this
be possibly addressed later (not in this patch)?

Still the "doc/README.distro" mentions boot.scr but has no references
to uEnv.txt (which seems to imply that the uEnv.txt is a second class
citizen).


> >
> > ---
> > A corresponding proof-of-concept version of sunxi-fel is available
> > from my https://github.com/n1tehawk/sunxi-tools/tree/20160608_uEnv-magic
> > branch. I've picked up the suggestion to use a "#=uEnv" magic string
> > to request that a file be treated as uEnv.txt-style data.
> >
> > For example, use your text editor to save a my.env file with
> >
> > #=uEnv
> > myvar=world
> > bootcmd=echo "Hello $myvar."
> >
> > and then test it with
> > ./sunxi-fel uboot u-boot-sunxi-with-spl.bin write 0x43100000 my.env
> >
> > You should see U-Boot's autoboot print the corresponding message
> > and drop you to the prompt, proving that you have successfully
> > overwritten the "bootcmd".
> >
> > Changes in v2:
> > - Patch renamed to something more suitable (was "sunxi: Add the
> >   ability to pass (script) filesize in the SPL header")
> > - The data field is now named fel_uEnv_length, and comes with
> >   a corresponding description in spl.h
> > - Instead of simply passing file size, fel_uEnv_length is now
> >   associated with uEnv.txt format. The patch modifies U-Boot's
> >   sunxi parse_spl_header() to auto-import such data.
> >
> >  arch/arm/include/asm/arch-sunxi/spl.h |  9 ++++++++-
> >  board/sunxi/board.c                   | 30 ++++++++++++++++++++++--------
> >  2 files changed, 30 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/arch-sunxi/spl.h b/arch/arm/include/asm/arch-sunxi/spl.h
> > index a0f33b0..a966a88 100644
> > --- a/arch/arm/include/asm/arch-sunxi/spl.h
> > +++ b/arch/arm/include/asm/arch-sunxi/spl.h
> > @@ -49,7 +49,14 @@ struct boot_file_head {
> >  		uint8_t spl_signature[4];
> >  	};
> >  	uint32_t fel_script_address;
> > -	uint32_t reserved1[3];
> > +	/*
> > +	 * If the fel_uEnv_length member below is set to a non-zero value,
> > +	 * it specifies the size (byte count) of data at fel_script_address.
> > +	 * At the same time this indicates that the data is in uEnv.txt
> > +	 * compatible format, ready to be imported via "env import -t".
> > +	 */
> > +	uint32_t fel_uEnv_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..fc57e60 100644
> > --- a/board/sunxi/board.c
> > +++ b/board/sunxi/board.c
> > @@ -573,6 +573,7 @@ void get_board_serial(struct tag_serialnr *serialnr)
> >
> >  #if !defined(CONFIG_SPL_BUILD)
> >  #include <asm/arch/spl.h>
> > +#include <environment.h>
> >
> >  /*
> >   * Check the SPL header for the "sunxi" variant. If found: parse values
> > @@ -582,17 +583,30 @@ void get_board_serial(struct tag_serialnr *serialnr)
> >  static void parse_spl_header(const uint32_t spl_addr)
> >  {
> >  	struct boot_file_head *spl = (void *)(ulong)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)
> > -				setenv_hex("fel_scriptaddr",
> > -					   spl->fel_script_address);
> > -			return;
> > -		}
> > +	if (memcmp(spl->spl_signature, SPL_SIGNATURE, 3) != 0)
> > +		return; /* signature mismatch, no usable header */
> > +
> > +	uint8_t spl_header_version = spl->spl_signature[3];
> > +	if (spl_header_version != SPL_HEADER_VERSION) {
> >  		printf("sunxi SPL version mismatch: expected %u, got %u\n",
> >  		       SPL_HEADER_VERSION, spl_header_version);
> > +		return;
> > +	}
> > +	if (!spl->fel_script_address)
> > +		return;
> > +
> > +	if (spl->fel_uEnv_length != 0) {
> > +		/*
> > +		 * data is expected in uEnv.txt compatible format, so "env
> > +		 * import -t" the string(s) at fel_script_address right away.
> > +		 */
> > +		himport_r(&env_htab, (char *)spl->fel_script_address,
> > +			  spl->fel_uEnv_length, '\n', H_NOCLEAR, 0, 0, NULL);
> > +		return;
> >  	}
> > +	/* otherwise assume .scr format (mkimage-type script) */
> > +	setenv_hex("fel_scriptaddr", spl->fel_script_address);
> > +	return;
> >  }
> >  #endif