From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bernhard Nortmann Date: Tue, 7 Jun 2016 16:09:58 +0200 Subject: [U-Boot] [PATCH] sunxi: Add the ability to pass (script) filesize in the SPL header In-Reply-To: <20160606122026.2cb26778@i7> References: <2a4fc4ba-9448-e871-460f-c5dc70ea1fc5@redhat.com> <1465060340-7209-1-git-send-email-bernhard.nortmann@web.de> <20160605144407.09c25a1c@i7> <575422AA.5050505@web.de> <20160606122026.2cb26778@i7> Message-ID: <5756D5B6.8050201@web.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Siarhei! Am 06.06.2016 um 11:20 schrieb Siarhei Siamashka: > On Sun, 5 Jun 2016 15:01:30 +0200 > Bernhard Nortmann 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. 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. > >> 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 = pairs) with a special marker. 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. > >> 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). > >>> 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). 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). 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 = 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. Regards, B. Nortmann