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

[York Sun]

On 06/12/2017 04:16 PM, André Przywara wrote:
> 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 am trying to make the falcon boot work with FIT image. We only need 
the SPL part of U-Boot.

> 
>> 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 want to get rid of your "loadables" and use the "load" instead. If an 
image has load address, let's load it. What do you think?

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

That's not a problem. I was thinking to use CONFIG_SPL_OS_BOOT and 
CONFIG_SPL_GZIP together.

York