On Mon, 16 May 2016 19:52:33 +0200 Hans de Goede hdegoede@redhat.com wrote:

Hi,

On 16-05-16 11:56, Bernhard Nortmann wrote:

...

Given that there now are quite a few additional "reserved" entries, and while we're still at SPL_HEADER_VERSION 1, I'd like to renew my request of dedicating one of these fields to the script length - which would enable us to set the U-Boot ${filesize} accordingly.

i.e. --- arch-arm-include-asm-arch-sunxi-spl.h +++ arch-arm-include-asm-arch-sunxi-spl.new.h @@ -49,7 +49,8 @@ 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 */
  

};

I do not intend to further push my specific use cases, however I still consider the (then somewhat theoretical) ability to do "import -t ${fel_script_addr} ${filesize}" useful. For reference, the previous discussion related to this was somewhere around http://lists.denx.de/pipermail/u-boot/2015-September/227454.html

Hmm, given that the boot-rom touches some of these, I wonder if we should be putting anything here at all.

Yes, this came as a bit of surprise because this was not clearly documented anywhere. Still it looks like that's just a single byte getting modified, albeit at a bit strange location.

BTW, do you remember what I said earlier about not always being in perfect control?

http://lists.denx.de/pipermail/u-boot/2015-September/228727.html

This particular issue just serves as a very nice demonstration :-)

Anyway, I think that we are already reasonably well prepared to handle it. The worst thing that can happen is that the boot ROM in the future Allwinner SoCs starts patching even more bytes in the header or moves this boot device id variable to some other address. If/when this happens, we can always update the SPL header format (do the "major" version change trick).

Other then that worry, I see no problem with adding a fel_script_length, Siarhei what is your opinion on this ?

I personally have no objections.

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.

Hello,

On Tue, 7 Jun 2016 16:09:58 +0200 Bernhard Nortmann bernhard.nortmann@web.de wrote:

Hello Siarhei!

Am 06.06.2016 um 11:20 schrieb Siarhei Siamashka:

...

On Sun, 5 Jun 2016 15:01:30 +0200 Bernhard Nortmann bernhard.nortmann@web.de wrote:

...

Hi Siarhei!

[...]

No, you're right and not missing anything. Setting ${filesize} alone doesn't achieve much, and would require further customization to do the actual import. Since this whole idea of having the script length didn't go down too well when I initially proposed it (back in September 2015), I stayed away from adding additional U-Boot modifications this time.

Maybe you can add the necessary changes to the U-Boot default environment in the v2 patch? Either way, we are not going to make any progress until it is feature complete and usable.

Back in 2015 Hans expressed concerns about overcomplicating what is already an exotic use-case. That is why I wanted to keep v1 as simple and 'generic' as possible - working from the assumption that if a user is sufficiently advanced to get fel_script_length set, (s)he'd also be proficient enough to tailor U-Boot to make actual use of ${filesize} according to his/her needs.

Changing the default environment to use the existing "import -t" command was just one example of what might be done - maybe there could be other future uses, which I currently haven't thought of. My basic goal was and is a way to pass a payload from sunxi-fel to U-Boot in a format-agnostic way. This requires a "length" / file size in addition to the memory address.

Out of curiosity, what kind of other payload are you trying to pass to U-Boot?

The discussion here seems to narrow down on "uEnv.txt style" usage now. That's okay for me - I can create a v2 which focuses on that, and fleshes out the needed details.

For the record, I'm personally not very much interested in uEnv.txt support myself. I was just assuming that it was the purpose of your patch. But now I'm a bit confused.

Again, I have no objections. But if people really want to have uEnv.txt support (or something else), then it's better to say so instead of beating around the bush :-)

...

...

One approach would be to have a modified "bootcmd_fel" that either tests for a non-empty ${filesize}, or tries to import the script data as a fallback ("source ${fel_scriptaddr} || import -t <...>;"). Another way is to always assume "uEnv.txt style" whenever fel_script_length is set, and do a himport_r() of the script data right away (the programmatic equivalent of "import -t"). I'm doing this in an experimental branch of mine, as it allows overriding everything from the default environment - including the "bootcmd" itself.

Of course, all of this also requires support from the sunxi-fel side of things (i.e. fel_script_length getting set in the first place). A quick-and-dirty solution I'm currently using is to assume a uEnv.txt script (and set fel_script_length) whenever sunxi-fel detects uploads of pure ASCII data.

The boot.scr file is nice because it has its own format and a magic signature. The uEnv.txt is difficult to recognize automatically, but maybe we can borrow the https://en.wikipedia.org/wiki/Shebang_%28Unix%29 approach used by scripting languages?

I mean, we can require that the first line of uEnv.txt is a comment in a special format. This can be described in the sunxi-tools documentation. And also the sunxi-fel tool could print a warning if it detects upload of pure ASCII data from a file with "uEnv" part in the name and ".txt" as the extension.

I dislike involving filename conventions here, and I'd also prefer not having to "tag" the uEnv-style files (which are basically <key>=<value> pairs) with a special marker.

Still that's not something that I invented myself. As I said, scripting languages are using it quite successfully. For example, you can find the line "#!/usr/bin/env python" in the beginning of many python source files.

And uEnv.txt files could use something similar to make them easier to detect. Maybe something like "#=uEnv" could be a reasonable choice?

As for the filename conventions, I only proposed this as an optional hint for the user. As a way to help people to make the transition. If such heuristics is correct and helpful even in 50% of cases, then it's good enough.

If sunxi-fel requires 'extra work' to recognise these files, we might just as well use a dedicated command for uploading them - e.g. "env" instead of "write". This command could also do sanity checks, like issue a warning if the file contains non-ASCII data or fails to meet the expected syntax.

Yes, this is also a reasonable solution.

...

...

This could also be done easily with a dedicated command, and I can even image sunxi-fel building the uEnv.txt string itself from given ("env") key/value pairs.

...

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"?

Well... this _is_ the generic name that U-Boot itself tends to use for data transfers. E.g. "load" or "tftp" commands set the ${filesize} too.

So you are suggesting to pretend that there was a "load" command executed for "uEnv.txt" right before running the code from the default environment? This seems to be rather fragile and does not look like it offers any real advantages over "fel_scriptsize".

It's based on the paradigm that any kind of data transfer (might) provide exactly this kind of information in ${filesize}. U-Boot users know this from a variety of "load" commands (Ymodem anyone?) - so if you like: yes, I'm pretending that something similar happened. Of course I can rename it to anything that you prefer. But if we're taking the uEnv route, we might easily do away with setting any dedicated environment variable at all (see below).

I don't have any preference. I was just wondering if other people (Hans?) are fine with your choice, which may potentially clash with the "load" commands in a rather obscure way.

...

...

...

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.

Our sunxi-fel utility is already testing for the script.bin format and can preserve the existing functionality by simply forcing fel_script_length to 0 in that case. And additional safeguards might be placed before "actively" setting that value to anything non-zero.

So it would serve both as the uEnv.txt length and also as the format type indicator (boot.scr or uEnv.txt)? This is probably okay if it is documented properly.

It would be trival for sunxi-fel to pass the size of .scr files, too - there's just no point in doing so, since this information is already redundant (available from the header anyway).

Yes, it just needs to be clearly documented, so that nothing can be easily misinterpreted. Right now your current patch looks like this:

--- 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 */

Setting the field to 0 seemed natural to me, and it also reflects what existing versions of the sunxi-fel utility would use (as they only know about setting fel_script_addr).

Natural or not, it still needs to be documented. Because different things may seem to be natural/obvious for different people. See my comment above.

Actually that might be the right idea to take the whole thing forward, if I modify some of my working assumptions accordingly:

• Redefine "fel_script_length" as "fel_env_size", and associate a 'format type indicator' meaning with it. Any non-zero value will also require the fel_script_addr to be specified, and signals uEnv-style data suitable for "import -t" (i.e. ASCII text with <key>=<value> lines).

• (fel_env_size == 0 && fel_script_addr != 0) means that U-Boot is safe to assume a .scr format was passed. This reflects and preserves the previous use case and existing sunxi-fel implementations. New sunxi-fel versions will make sure to pass (fel_env_size == 0) whenever they detect the transfers of a mkimage-type script.

• sunxi-fel will pass a non-zero fel_env_size if (and only if) requirements and/or safety checks are met, in a way that makes it safe for U-Boot to rely on the assumption of uEnv-style format. This may range from simple user request ("env" command) to actual syntax validation.

• If both fel_script_addr and fel_env_size are non-zero, U-Boot will auto-import the data right away upon start, and afterwards present a modified environment (merge of the default environment with anything the 'script' sets/overrides). This eliminates the need to further modify default env settings (e.g. sneak in "import -t" to some bootcmd) and also avoids setting dedicated environment variables just for this purpose.

OK. Please just try to formulate it as concise, but comprehensive comments for the spl.h file.

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@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.

Regards,

Hans

---

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

Hi,

On Wed, 8 Jun 2016 22:13:50 +0200 Hans de Goede hdegoede@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@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@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=b...

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

Hi,

On 09-06-16 07:37, 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@web.de Acked-by: Siarhei Siamashka siarhei.siamashka@gmail.com

Thanks, I've added this to u-boot-sunxi/next for merging into v2016.09 as soon as the merge window for that opens.

Regards,

Hans

---

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 v3:

• Dropped a surplus "return;" at the end of parse_spl_header()
• Have "FEL" in the subject line to clarify the scope
• Added "Acked-by" line

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 | 29 +++++++++++++++++++++-------- 2 files changed, 29 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..b5a50f4 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,29 @@ 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);

} #endif