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

[Siarhei Siamashka]

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

> Hi Siarhei!
> 
> Am 05.06.2016 um 13:44 schrieb Siarhei Siamashka:
> > Hello Bernhard,
> >
> > [...]
> > 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?  
> 
> 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.

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

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

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

-- 
Regards,
Sierž