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

[Simon Goldschmidt]

On 26.07.2018 08:52, 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.
> 
> 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.

I think it would be more consistent to have the compression setting 
control U-Boot unzip behaviour and implement unzip for all types (as you 
did for SPL). Of course, I also have the fpga image in mind :-)

The question is (as often): how many existing .its would be broken with 
such a change.

Simon

> 
> Also I think that "load" entry is that one which matters not "entry".
> 
> Thanks,
> Michal
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>