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.