[U-Boot] [RFC PATCH 2/2] sunxi: add "fel" boot target

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 14-09-15 12:33, Siarhei Siamashka wrote:
> On Fri, 11 Sep 2015 11:31:50 +0200
> Bernhard Nortmann <bernhard.nortmann@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