Hi,

On 14-09-15 12:33, Siarhei Siamashka wrote:
On Fri, 11 Sep 2015 11:31:50 +0200
Bernhard Nortmann <bernhard.nortm...@web.de> wrote:

Hi!

Am 10.09.2015 um 20:36 schrieb Hans de Goede:
Hi,

I would prefer to have this like this:

     "bootcmd_fel=" \
         "if test -n ${fel_booted} && test -n ${fel_data_addr}; then " \
             "echo '(FEL boot)';" \
             "source ${fel_data_addr}; " \
         "fi\0"


Sure, we could do that. I wanted to make clear that ${fel_booted} is
independent of a script being present (and thus ${fel_data_addr} set).
If the user feels inclined to do so, he might e.g. tweak bootcmd_fel
to override some defaults even with no boot.scr involved.

Also if we are not using fel_data_size, then why do we even
have it ?


I thought it unnecessary to restrict ourselves to not being able to
pass the size information, and kept it optional deliberately.

Admittedly it's pointless in the "standard" case of boot.scr, as that
is expected to be an image with a well-defined header (including data
size). I could imagine other uses, e.g. a customized fel utility
passing uEnv.txt-style data, and integrating that via bootcmd_fel
"import -t ${fel_data_addr} ${fel_data_size}".

We could probably have this data represented in the following way
in the SPL header:

        union {
                struct {
                        uint32_t fel_boot_script_address;
                        uint32_t must_be_zero;
                };
                struct {
                        uint32_t fel_uenv_txt_address;
                        uint32_t fel_uenv_txt_size;
                };
        };

So that if 'fel_uenv_txt_size' is non-zero, then we can do
"import -t ${fel_uenv_txt_address} ${fel_uenv_txt_size}".
And if it is zero, then treat this pointer as boot.scr data.

And we don't even need to use the union if we don't want to. This all
data could be also stored in a straightforward way (at the expense
of using additional 4 bytes in the SPL header):

        uint32_t fel_boot_script_address;
        uint32_t fel_uenv_txt_address;
        uint32_t fel_uenv_txt_size;

And because it takes more storage space, we would need to increase
the SPL header size. But this can be easily done.

Personally I like to do this when testing; I find it easier to
simply edit a text file (without having to go through a mkimage
.scr on each cycle).

Supporting both boot.scr and uEnv.txt for FEL boot seems to be
reasonably simple to me. You can even do it in a single patch
series. As Hans suggests, please take care of the boot.scr case
first. Then maybe introduce uEnv.txt support with an additional
patch.

I'm still not convinced that support uEnv.txt is necessary at all,
but I agree that if we this having this done through extra fields,
rather then through a union would be better.

Regards,

Hans

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to