[U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions

[Michal Simek]

On 26.7.2018 18:23, York Sun wrote:
> On 07/25/2018 11:52 PM, Michal Simek wrote:
>> On 25.7.2018 23:18, York Sun wrote:
>>> On 07/24/2018 10:58 PM, Michal Simek wrote:
>>>> On 24.7.2018 18:26, York Sun wrote:
>>>>> On 07/24/2018 06:07 AM, Michal Simek wrote:
>>>>>> There is no reason to limit gzip usage only for OS_BOOT and kernel image
>>>>>> type.
>>>>>>
>>>>>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>>>>>> ---
>>>>>>
>>>>>>  common/spl/spl_fit.c | 5 +----
>>>>>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>>>>>> index 9eabb1c1058b..dbf5ac33a845 100644
>>>>>> --- a/common/spl/spl_fit.c
>>>>>> +++ b/common/spl/spl_fit.c
>>>>>> @@ -257,10 +257,7 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
>>>>>>  	board_fit_image_post_process(&src, &length);
>>>>>>  #endif
>>>>>>  
>>>>>> -	if (IS_ENABLED(CONFIG_SPL_OS_BOOT)	&&
>>>>>> -	    IS_ENABLED(CONFIG_SPL_GZIP)		&&
>>>>>> -	    image_comp == IH_COMP_GZIP		&&
>>>>>> -	    type == IH_TYPE_KERNEL) {
>>>>>> +	if (IS_ENABLED(CONFIG_SPL_GZIP) && image_comp == IH_COMP_GZIP) {
>>>>>>  		size = length;
>>>>>>  		if (gunzip((void *)load_addr, CONFIG_SYS_BOOTM_LEN,
>>>>>>  			   src, &size)) {
>>>>>>
>>>>>
>>>>> This will uncompress ramdisk unnecessarily.
>>>>
>>>> Can you please share your its fragment? Also is there any other image
>>>> which should be exclude?
>>>
>>> I used it for falcon boot. I guess the executable image should have "entry". In
>>> my setup, only kernel image has "entry". Here is my its file.
>>>
>>> /dts-v1/;
>>>
>>> / {
>>> 	description = "Image file for the LS1046A Linux Kernel";
>>> 	#address-cells = <1>;
>>>
>>> 	images {
>>> 		kernel at 1 {
>>> 			description = "ARM64 Linux kernel";
>>> 			data = /incbin/("./arch/arm64/boot/Image.gz");
>>> 			type = "kernel";
>>> 			arch = "arm64";
>>> 			os = "linux";
>>> 			compression = "gzip";
>>> 			load = <0x80080000>;
>>> 			entry = <0x80080000>;
>>> 		};
>>> 		fdt at 1 {
>>> 			description = "Flattened Device Tree blob";
>>> 			data = /incbin/("./fsl-ls1046ardb.dtb");
>>> 			type = "flat_dt";
>>> 			arch = "arm64";
>>> 			compression = "none";
>>> 			load = <0x90000000>;
>>> 		};
>>> 		ramdisk at 1 {
>>> 			description = "Buildroot initramfs";
>>>                         data = /incbin/("./rootfs.cpio.gz");
>>> 			type = "ramdisk";
>>> 			arch = "arm64";
>>> 			os = "linux";
>>> 			compression = "gzip";
>>
>> I have tested full u-boot and there is also no uncompression for ramdisk
>> when you put gzip compress there.
>> I have even tried gzip compression for fdt with expectation that u-boot
>> will uncompress it.
>>
>> Based on doc/uImage.FIT/source_file_format.txt:
>> 165  - compression : Compression used by included data. Supported
>> compressions
>> 166     are "gzip" and "bzip2". If no compression is used compression
>> property
>> 167     should be set to "none".
>>
>>
>> Based on me this means that data inside fit are compressed and you are
>> asking u-boot in boot to uncompress it. If you are not asking for that
>> you should put none there.
>> But it doesn't look like this is supported at all for fdt/ramdisk and it
>> is only handled for OS.
>> I see here two cases.
>> 1. I want u-boot to uncompress my data in fit image (whatever it is)
>> before passing control to OS that's why I putting there compression method.
>> 2. I want OS to uncompress data but I want pass this data unchanged from
>> u-boot to OS that's why I am putting compression method at "none"
>>
>> I am expecting when you put "none" there than you will boot in falcon
>> mode without any issue.
> 
> That will work. I can put "none" for the images I don't want U-Boot to
> uncompress.
> 
>>
>> I have no problem to change this patch and include only kernel and fpga
>> image but it sounds to me that we have gaps in implementation that not
>> all images inside the fit image have the same range of functionalities.
>>
>> Also I think that "load" entry is that one which matters not "entry".
>>
> 
> Not true here. The "entry" matters if you want to run it, for example
> Linux kernel. It may be different from "load".

yes - if you want to run it entry is used but if you want to uncompress
it you should say where you want to uncompress it. I am not quite sure
if we have any logic to automatically choose a place for uncompression.

Thanks,
Michal