All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
@ 2018-07-24 13:07 Michal Simek
  2018-07-24 16:26 ` York Sun
  2018-07-25  6:26 ` Simon Goldschmidt
  0 siblings, 2 replies; 20+ messages in thread
From: Michal Simek @ 2018-07-24 13:07 UTC (permalink / raw)
  To: u-boot

There is no reason to limit gzip usage only for OS_BOOT and kernel image
type.

Signed-off-by: Michal Simek <michal.simek@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)) {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-24 13:07 [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions Michal Simek
@ 2018-07-24 16:26 ` York Sun
  2018-07-25  5:57   ` Michal Simek
  2018-07-25  6:26 ` Simon Goldschmidt
  1 sibling, 1 reply; 20+ messages in thread
From: York Sun @ 2018-07-24 16:26 UTC (permalink / raw)
  To: u-boot

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

York

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-24 16:26 ` York Sun
@ 2018-07-25  5:57   ` Michal Simek
  2018-07-25 21:18     ` York Sun
  0 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2018-07-25  5:57 UTC (permalink / raw)
  To: u-boot

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@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?

Thanks,
Michal

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-24 13:07 [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions Michal Simek
  2018-07-24 16:26 ` York Sun
@ 2018-07-25  6:26 ` Simon Goldschmidt
  2018-07-25  6:40   ` Michal Simek
  1 sibling, 1 reply; 20+ messages in thread
From: Simon Goldschmidt @ 2018-07-25  6:26 UTC (permalink / raw)
  To: u-boot



On 24.07.2018 15:07, 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@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)) {
> 

I suppose this is to support a gziped fpga image in a fit. Does this 
work for U-Boot proper already?

Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-25  6:26 ` Simon Goldschmidt
@ 2018-07-25  6:40   ` Michal Simek
  2018-07-25  6:52     ` Simon Goldschmidt
  0 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2018-07-25  6:40 UTC (permalink / raw)
  To: u-boot

On 25.7.2018 08:26, Simon Goldschmidt wrote:
> 
> 
> On 24.07.2018 15:07, 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@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)) {
>>
> 
> I suppose this is to support a gziped fpga image in a fit. Does this
> work for U-Boot proper already?

Luis has tested it some days ago based on my suggestion. I have tried
that yesterday on zynq zc706 and it was also working for internal data.
Please take a look at second thread where also times are listed.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-25  6:40   ` Michal Simek
@ 2018-07-25  6:52     ` Simon Goldschmidt
  2018-07-25  7:04       ` Michal Simek
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Goldschmidt @ 2018-07-25  6:52 UTC (permalink / raw)
  To: u-boot

On 25.07.2018 08:40, Michal Simek wrote:
> On 25.7.2018 08:26, Simon Goldschmidt wrote:
>>
>>
>> On 24.07.2018 15:07, 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@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)) {
>>>
>>
>> I suppose this is to support a gziped fpga image in a fit. Does this
>> work for U-Boot proper already?
> 
> Luis has tested it some days ago based on my suggestion. I have tried
> that yesterday on zynq zc706 and it was also working for internal data.
> Please take a look at second thread where also times are listed.

Isn't that 2nd thread on SPL also? I was asking for U-Boot proper in 
comparison.

We are programming the FPGA from U-Boot proper, not SPL, using a 
Multi-config FIT image including matching Kernel and Device tree. And 
here, using a gziped FPGA might be nice. But from reading the sources (I 
think I also tested it once), that doesn't work. That's why I ask.

Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-25  6:52     ` Simon Goldschmidt
@ 2018-07-25  7:04       ` Michal Simek
  2018-07-25 10:14         ` Simon Goldschmidt
  0 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2018-07-25  7:04 UTC (permalink / raw)
  To: u-boot

On 25.7.2018 08:52, Simon Goldschmidt wrote:
> On 25.07.2018 08:40, Michal Simek wrote:
>> On 25.7.2018 08:26, Simon Goldschmidt wrote:
>>>
>>>
>>> On 24.07.2018 15:07, 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@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)) {
>>>>
>>>
>>> I suppose this is to support a gziped fpga image in a fit. Does this
>>> work for U-Boot proper already?
>>
>> Luis has tested it some days ago based on my suggestion. I have tried
>> that yesterday on zynq zc706 and it was also working for internal data.
>> Please take a look at second thread where also times are listed.
> 
> Isn't that 2nd thread on SPL also? I was asking for U-Boot proper in
> comparison.

I have tested several cases for this series.
https://lists.denx.de/pipermail/u-boot/2018-July/335193.html

This is my script for generating images.

mkimage -d download.bin -T firmware -C none -a 0x1000000 -e 0x1000000 -A
arm -n "fpga download.bin" download.ub

gzip < download.bin > download.bin.gz
mkimage -d download.bin.gz -T firmware -C gzip -a 0x10000000 -e
0x10000000 -A arm -n "fpga download.bin.gz" download.gz.ub

mkimage -f fit.its  download-fit.ub

fit.its unfortunately is not using gzip format.

> We are programming the FPGA from U-Boot proper, not SPL, using a
> Multi-config FIT image including matching Kernel and Device tree. And
> here, using a gziped FPGA might be nice. But from reading the sources (I
> think I also tested it once), that doesn't work. That's why I ask.

I am going to retest the whole series and I can try it but if this is
working in SPL I can't see any issue why this shouldn't work in full
u-boot. I am not using -E for mkimage but please test it and let me know
if this is working or not.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-25  7:04       ` Michal Simek
@ 2018-07-25 10:14         ` Simon Goldschmidt
  2018-07-25 10:37           ` Michal Simek
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Goldschmidt @ 2018-07-25 10:14 UTC (permalink / raw)
  To: u-boot



On 25.07.2018 09:04, Michal Simek wrote:
> On 25.7.2018 08:52, Simon Goldschmidt wrote:
>> On 25.07.2018 08:40, Michal Simek wrote:
>>> On 25.7.2018 08:26, Simon Goldschmidt wrote:
>>>>
>>>>
>>>> On 24.07.2018 15:07, 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@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)) {
>>>>>
>>>>
>>>> I suppose this is to support a gziped fpga image in a fit. Does this
>>>> work for U-Boot proper already?
>>>
>>> Luis has tested it some days ago based on my suggestion. I have tried
>>> that yesterday on zynq zc706 and it was also working for internal data.
>>> Please take a look at second thread where also times are listed.
>>
>> Isn't that 2nd thread on SPL also? I was asking for U-Boot proper in
>> comparison.
> 
> I have tested several cases for this series.
> https://lists.denx.de/pipermail/u-boot/2018-July/335193.html
> 
> This is my script for generating images.
> 
> mkimage -d download.bin -T firmware -C none -a 0x1000000 -e 0x1000000 -A
> arm -n "fpga download.bin" download.ub
> 
> gzip < download.bin > download.bin.gz
> mkimage -d download.bin.gz -T firmware -C gzip -a 0x10000000 -e
> 0x10000000 -A arm -n "fpga download.bin.gz" download.gz.ub
> 
> mkimage -f fit.its  download-fit.ub
> 
> fit.its unfortunately is not using gzip format.

I'm using the 'mkimage -f fit.its boot.itb' case (in U-Boot, not SPL), 
so I if I read your comments above correctly, this is still not 
supported for FIT images but only for "legacy" images?

But the recent patches add gzip support for FIT in SPL, did I get that 
correctly?

If so, would it make sense to add FPGA gzip support in FIT to U-Boot proper?

> 
>> We are programming the FPGA from U-Boot proper, not SPL, using a
>> Multi-config FIT image including matching Kernel and Device tree. And
>> here, using a gziped FPGA might be nice. But from reading the sources (I
>> think I also tested it once), that doesn't work. That's why I ask.
> 
> I am going to retest the whole series and I can try it but if this is
> working in SPL I can't see any issue why this shouldn't work in full
> u-boot. I am not using -E for mkimage but please test it and let me know
> if this is working or not.

I'm not familiar with the -E option for mkimage. Does it apply to -f 
fit.its? Given the above statements, I think my use case just does not 
support gziped FPGA, yet.

Thanks,
Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-25 10:14         ` Simon Goldschmidt
@ 2018-07-25 10:37           ` Michal Simek
  0 siblings, 0 replies; 20+ messages in thread
From: Michal Simek @ 2018-07-25 10:37 UTC (permalink / raw)
  To: u-boot

On 25.7.2018 12:14, Simon Goldschmidt wrote:
> 
> 
> On 25.07.2018 09:04, Michal Simek wrote:
>> On 25.7.2018 08:52, Simon Goldschmidt wrote:
>>> On 25.07.2018 08:40, Michal Simek wrote:
>>>> On 25.7.2018 08:26, Simon Goldschmidt wrote:
>>>>>
>>>>>
>>>>> On 24.07.2018 15:07, 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@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)) {
>>>>>>
>>>>>
>>>>> I suppose this is to support a gziped fpga image in a fit. Does this
>>>>> work for U-Boot proper already?
>>>>
>>>> Luis has tested it some days ago based on my suggestion. I have tried
>>>> that yesterday on zynq zc706 and it was also working for internal data.
>>>> Please take a look at second thread where also times are listed.
>>>
>>> Isn't that 2nd thread on SPL also? I was asking for U-Boot proper in
>>> comparison.
>>
>> I have tested several cases for this series.
>> https://lists.denx.de/pipermail/u-boot/2018-July/335193.html
>>
>> This is my script for generating images.
>>
>> mkimage -d download.bin -T firmware -C none -a 0x1000000 -e 0x1000000 -A
>> arm -n "fpga download.bin" download.ub
>>
>> gzip < download.bin > download.bin.gz
>> mkimage -d download.bin.gz -T firmware -C gzip -a 0x10000000 -e
>> 0x10000000 -A arm -n "fpga download.bin.gz" download.gz.ub
>>
>> mkimage -f fit.its  download-fit.ub
>>
>> fit.its unfortunately is not using gzip format.
> 
> I'm using the 'mkimage -f fit.its boot.itb' case (in U-Boot, not SPL),
> so I if I read your comments above correctly, this is still not
> supported for FIT images but only for "legacy" images?
> 
> But the recent patches add gzip support for FIT in SPL, did I get that
> correctly?

We need to solve one issue with socfpga support but yes it is enabling
gzip support for FIT in SPL.

> 
> If so, would it make sense to add FPGA gzip support in FIT to U-Boot
> proper?

Sure why not. I expect this shouldn't be hard to do.


>>
>>> We are programming the FPGA from U-Boot proper, not SPL, using a
>>> Multi-config FIT image including matching Kernel and Device tree. And
>>> here, using a gziped FPGA might be nice. But from reading the sources (I
>>> think I also tested it once), that doesn't work. That's why I ask.
>>
>> I am going to retest the whole series and I can try it but if this is
>> working in SPL I can't see any issue why this shouldn't work in full
>> u-boot. I am not using -E for mkimage but please test it and let me know
>> if this is working or not.
> 
> I'm not familiar with the -E option for mkimage. Does it apply to -f
> fit.its? Given the above statements, I think my use case just does not
> support gziped FPGA, yet.

-E should be external data format that data are not stored with
information about partition. It means you can load just header and then
images which you need that you don't need to load everything what it is
there. Imaging fit image with 10 bitstreams and 10 configurations and
you only want to load one which you want to load.
It is faster to read 1 bitstream from sd then 10.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-25  5:57   ` Michal Simek
@ 2018-07-25 21:18     ` York Sun
  2018-07-26  6:52       ` Michal Simek
  0 siblings, 1 reply; 20+ messages in thread
From: York Sun @ 2018-07-25 21:18 UTC (permalink / raw)
  To: u-boot

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@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";
			load = <0xa0000000>;
		};
	};

	configurations {
		default = "config at 1";
		config at 1 {
			description = "Boot Linux kernel";
			kernel = "kernel at 1";
			fdt = "fdt at 1";
			ramdisk = "ramdisk at 1";
		};
	};
};

York

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-25 21:18     ` York Sun
@ 2018-07-26  6:52       ` Michal Simek
  2018-07-26  6:58         ` Simon Goldschmidt
  2018-07-26 16:23         ` York Sun
  0 siblings, 2 replies; 20+ messages in thread
From: Michal Simek @ 2018-07-26  6:52 UTC (permalink / raw)
  To: u-boot

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

Also I think that "load" entry is that one which matters not "entry".

Thanks,
Michal

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-26  6:52       ` Michal Simek
@ 2018-07-26  6:58         ` Simon Goldschmidt
  2018-07-26 16:23         ` York Sun
  1 sibling, 0 replies; 20+ messages in thread
From: Simon Goldschmidt @ 2018-07-26  6:58 UTC (permalink / raw)
  To: u-boot



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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-26  6:52       ` Michal Simek
  2018-07-26  6:58         ` Simon Goldschmidt
@ 2018-07-26 16:23         ` York Sun
  2018-07-26 17:16           ` Tom Rini
                             ` (2 more replies)
  1 sibling, 3 replies; 20+ messages in thread
From: York Sun @ 2018-07-26 16:23 UTC (permalink / raw)
  To: u-boot

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

York

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-26 16:23         ` York Sun
@ 2018-07-26 17:16           ` Tom Rini
  2018-07-30  8:54             ` Michal Simek
  2018-07-30  8:47           ` Michal Simek
  2018-07-30 12:46           ` Michal Simek
  2 siblings, 1 reply; 20+ messages in thread
From: Tom Rini @ 2018-07-26 17:16 UTC (permalink / raw)
  To: u-boot

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

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

load / entry matter for various use cases and platforms, yes.  And since
we're talking about FIT images I feel compelled to bring up 'type =
"kernel_noload"' as the way to avoid U-Boot moving the kernel when it
doesn't have to, as part of speeding up boot time.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180726/1c6ec7b7/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-26 16:23         ` York Sun
  2018-07-26 17:16           ` Tom Rini
@ 2018-07-30  8:47           ` Michal Simek
  2018-07-30 12:46           ` Michal Simek
  2 siblings, 0 replies; 20+ messages in thread
From: Michal Simek @ 2018-07-30  8:47 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-26 17:16           ` Tom Rini
@ 2018-07-30  8:54             ` Michal Simek
  2018-07-30 11:28               ` Simon Goldschmidt
  0 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2018-07-30  8:54 UTC (permalink / raw)
  To: u-boot

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@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?

Thanks,
Michal

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-30  8:54             ` Michal Simek
@ 2018-07-30 11:28               ` Simon Goldschmidt
  2018-07-30 12:48                 ` Michal Simek
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Goldschmidt @ 2018-07-30 11:28 UTC (permalink / raw)
  To: u-boot



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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-26 16:23         ` York Sun
  2018-07-26 17:16           ` Tom Rini
  2018-07-30  8:47           ` Michal Simek
@ 2018-07-30 12:46           ` Michal Simek
  2018-07-30 16:55             ` York Sun
  2 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2018-07-30 12:46 UTC (permalink / raw)
  To: u-boot

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

ok. Can you please retest this patch with none and give me some tags to
add it to this patch?

Thanks,
Michal

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-30 11:28               ` Simon Goldschmidt
@ 2018-07-30 12:48                 ` Michal Simek
  0 siblings, 0 replies; 20+ messages in thread
From: Michal Simek @ 2018-07-30 12:48 UTC (permalink / raw)
  To: u-boot

On 30.7.2018 13:28, Simon Goldschmidt wrote:
> 
> 
> 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@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

Thanks.

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

I would like to resolve spl fpga fit image loading first and fpga
mkimage is next one to check. Also we have fpga image loading as the
part of bootm command. All of these should be checked.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions
  2018-07-30 12:46           ` Michal Simek
@ 2018-07-30 16:55             ` York Sun
  0 siblings, 0 replies; 20+ messages in thread
From: York Sun @ 2018-07-30 16:55 UTC (permalink / raw)
  To: u-boot

On 07/30/2018 05:46 AM, Michal Simek wrote:

<snip>

>>> 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.
> 
> ok. Can you please retest this patch with none and give me some tags to
> add it to this patch?
> 

Tested-by: York Sun <york.sun@nxp.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2018-07-30 16:55 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-24 13:07 [U-Boot] [PATCH] spl: fit: Enable GZIP compression also for no kernel partitions Michal Simek
2018-07-24 16:26 ` York Sun
2018-07-25  5:57   ` Michal Simek
2018-07-25 21:18     ` York Sun
2018-07-26  6:52       ` Michal Simek
2018-07-26  6:58         ` Simon Goldschmidt
2018-07-26 16:23         ` York Sun
2018-07-26 17:16           ` Tom Rini
2018-07-30  8:54             ` Michal Simek
2018-07-30 11:28               ` Simon Goldschmidt
2018-07-30 12:48                 ` Michal Simek
2018-07-30  8:47           ` Michal Simek
2018-07-30 12:46           ` Michal Simek
2018-07-30 16:55             ` York Sun
2018-07-25  6:26 ` Simon Goldschmidt
2018-07-25  6:40   ` Michal Simek
2018-07-25  6:52     ` Simon Goldschmidt
2018-07-25  7:04       ` Michal Simek
2018-07-25 10:14         ` Simon Goldschmidt
2018-07-25 10:37           ` Michal Simek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.