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

[Simon Goldschmidt]

On 30.07.2018 10:54, Michal Simek wrote:
> On 26.7.2018 19:16, Tom Rini wrote:
>> On Thu, Jul 26, 2018 at 04:23:14PM +0000, 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.
>>
>> Please also update the document to be clear that "none" for "don't touch
>> my compressed data!" is expected.  The existing language isn't clear but
>> I agree it makes sense (and follows the long standing practice of "-C
>> none" on compressed ramdisks for legacy style images).
>>
>> And per Simon Goldschmidt's suggestion, after we've made the docs
>> clearer, making it so that U-Boot does decompress things would be good,
>> but we may also need to make that behavior configurable as I can see
>> people having put the correct compression in and not wanting it to be
>> uncompressed as we have examples of both none and gzip for ramdisks
>> today.  It's also possible (and if someone wants to dig back /
>> experiment and confirm) that long ago it did auto-decompress the data
>> and now doesn't, it would be a bugfix and no we don't need to make it
>> configurable.
> 
> Simon Goldschmidt: Can you please update that doc?

Done here:
https://lists.denx.de/pipermail/u-boot/2018-July/336438.html

Are you planning to implement fpga uncompression in U-Boot proper? I 
don't know when I would find the time to do so...

Thanks,
Simon