[U-Boot] [PATCH v1 0/4] Add FIT support for falcon boot

[André Przywara]

On 09/06/17 19:48, york sun wrote:

Hi York,

> On 05/19/2017 02:56 AM, Andre Przywara wrote:
>> Hi York,
>>
>> On 16/05/17 16:54, york sun wrote:
>>> On 05/15/2017 10:42 PM, Lokesh Vutla wrote:
>>>> + Andre
>>>>
>>>>
>>>> On Monday 15 May 2017 09:31 PM, York Sun wrote:
>>>>> This patch set adds FIT support for falcon boot. GZIP is enabled
>>>>> to supported compressed image.
>>>>
>>>> Did you get a chance to look at Andre's "SPL: extend FIT loading
>>>> support"[1] patch series? This series addresses similar problem in a
>>>> more generic way.
>>>>
>>>> [1] https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.mail-archive.com%2Fu-boot%40lists.denx.de%2Fmsg246692.html&data=01%7C01%7Cyork.sun%40nxp.com%7C63ec3a7e245445e2253d08d49c1e542b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=XrCyv6JBg02fwP7MEMo1hg8rVCOGnEQ741Hs0oLwZVg%3D&reserved=0
>>>
>>> I only received partial patches from Andre's set. Looks like that set
>>> should cover my changes by supporting multiple images. I was focusing on
>>> a fast boot path for the past two months. I can rebase my patch once
>>> Andre's set is merged.
>>
>> FYI: My patches have been merged into u-boot-sunxi/master[1], which is
>> based on origin/master as of earlier this week.
>> I take it they get merged into origin before -rc1, but meanwhile you
>> could base on u-boot-sunxi/master.
>>
> 
> Andre,
> 
> Reading through your commits, I am glad to see you can support multiple 
> images. My understanding is you use "loadables" to indicate extra images 
> to load after loading U-Boot. The spl_image->entry_point is set to 
> either the last image's entry point (if set), or to the 
> spl_image->load_addr. Do I get it right?

The idea is to use the start address from the _first_ image that
provides an entry point.
So if the "firmware" image has one, this is used, if not, the first
image from the loadables section which provides an explicit "entry"
property wins.
This is used for Allwinner, where we want to load U-Boot and append the
right DTB, but need to branch into the Trusted Firmware image.

> When you process the images, if there is a "firmware" image, you presume 
> it is U-Boot and load it. If no such image, you take the first image in 
> "loadables", presuming it is U-Boot. Do I understand you correctly?

Yes, this is right.
This whole scheme is just meant to provide compatibility with the
existing behaviour (with just the single image load).
The whole reason why U-Boot proper is treated specially here is to be
able to append the right DTB, which has to be loaded right behind the
end of the U-Boot image (to match the expectation of
CONFIG_OF_SEPARATE). Other than that U-Boot isn't very special.

> In 
> your example its file, you demonstrated multiple "firmware" images, and 
> U-Boot has type of "standalone". Since you didn't put "firmware" in the 
> config node, I presume you would hit the "loadables". Correct?

Exactly. U-Boot proper is just a loadable here. This special treatment
of the "firmware" node is just there to support older .its files.
Not sure if that is really needed, as I would expect the .its file *for
the SPL* to be generated and shipped with U-Boot, though.

> While I try to rebase my patch on top of yours, I realize you still 
> presume U-Boot image always exists. This is what I am trying to change 
> for falcon boot. I think the easiest way is to leave "firmware" node 
> absent in config node, and check the image type before setting 
> spl_image->os. After that I can follow the code flow to load ramdisk, 
> etc.

So you want to load Linux directly, without U-Boot proper?
FWIW, I once made an setup where I still use U-Boot proper, but load the
kernel and initrd together with it (by the SPL), so the default
environment can just proceed to booti, without needing to load anything.

> I just have to add the "loadables" into my its. How about this flow 
> with pseudo code
> 
> node = spl_fit_get_image_node(fit, images, "firmware", 0);
> if (node < 0) {
> 	node = spl_fit_get_image_node(fit, images, "loadables", 0);
> 	index = 1;
> }
> if (node < 0)
> 	return -1;
> 
> if (image_type(node) == IH_TYPE_KERNEL) {
> 	index = 0;	/* reset to first image */
> } else {
> 	/* presuming U-Boot */
> 	spl_image->os = IH_OS_U_BOOT;
> 	continue_to_read_device_tree();
> }
> 
> /* check if there are more images to load */
> for (; ; index++) {
> 	/* keep existing code */
> }
> 
> What do you think?

Yes, that sounds correct. Actually I was already toying with the idea of
supporting special image types in the SPL (like the kernel here), but
didn't want to extend the tight SPL code just for the sake of it.
But since you have a use case, I think this is a sensible extension.
It might even be worse to make this more flexible and provide some means
to extend this support for further image types later.

> I will need to modify spl_load_fit_image() a little bit to support 
> compressed kernel image.

Please consider to protect it with #ifdefs to avoid bloating the SPL
image. On sunxi we are quite tight already for AArch64 ...

Cheers,
Andre.