[U-Boot] [PATCH 1/2] doc: FIT image: clarify usage of "compression" property

[U-Boot] [PATCH 1/2] doc: FIT image: clarify usage of "compression" property

[Simon Goldschmidt]

Compressed images should have their compression property
set to "none" if U-Boot should leave them compressed.

This is especially the case for compressed ramdisks that
should be uncompressed by the kernel only.

Signed-off-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>
---

 doc/uImage.FIT/source_file_format.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/doc/uImage.FIT/source_file_format.txt b/doc/uImage.FIT/source_file_format.txt
index d2793a195d..d701b9bb76 100644
--- a/doc/uImage.FIT/source_file_format.txt
+++ b/doc/uImage.FIT/source_file_format.txt
@@ -164,7 +164,9 @@ the '/images' node should have the following layout:
   - data : Path to the external file which contains this node's binary data.
   - compression : Compression used by included data. Supported compressions
     are "gzip" and "bzip2". If no compression is used compression property
-    should be set to "none".
+    should be set to "none". If the data is compressed but it should not be
+    uncompressed by U-Boot (e.g. compressed ramdisk), this should also be set
+    to "none".
 
   Conditionally mandatory property:
   - os : OS name, mandatory for types "kernel" and "ramdisk". Valid OS names
-- 
2.11.0

[U-Boot] [PATCH 2/2] FIT image: use compression = "none" for ramdisks

[Simon Goldschmidt]

To prepare supporting compression for all image types, change
compression to "none" for ramdisks in all examples.

Signed-off-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>
---

 arch/arm/cpu/armv8/fsl-layerscape/doc/README.falcon | 2 +-
 doc/README.bcm7xxx                                  | 2 +-
 doc/uImage.FIT/multi.its                            | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/doc/README.falcon b/arch/arm/cpu/armv8/fsl-layerscape/doc/README.falcon
index 7dae9f03c3..b3c6693a42 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/doc/README.falcon
+++ b/arch/arm/cpu/armv8/fsl-layerscape/doc/README.falcon
@@ -110,7 +110,7 @@ Example:
 			type = "ramdisk";
 			arch = "arm64";
 			os = "linux";
-			compression = "gzip";
+			compression = "none";
 			load = <0xa0000000>;
 		};
 	};
diff --git a/doc/README.bcm7xxx b/doc/README.bcm7xxx
index 9b5eae4741..1de2f63181 100644
--- a/doc/README.bcm7xxx
+++ b/doc/README.bcm7xxx
@@ -91,7 +91,7 @@ image.its:
 			type = "ramdisk";
 			arch = "arm";
 			os = "linux";
-			compression = "gzip";
+			compression = "none";
 			/*
 			 * Set the environment variable initrd_high to
 			 * 0xffffffff, and set "load" and "entry" here
diff --git a/doc/uImage.FIT/multi.its b/doc/uImage.FIT/multi.its
index 26c8dad6a2..814d1f73b5 100644
--- a/doc/uImage.FIT/multi.its
+++ b/doc/uImage.FIT/multi.its
@@ -60,7 +60,7 @@
 			type = "ramdisk";
 			arch = "ppc";
 			os = "linux";
-			compression = "gzip";
+			compression = "none";
 			load = <00000000>;
 			entry = <00000000>;
 			hash-1 {
@@ -74,7 +74,7 @@
 			type = "ramdisk";
 			arch = "ppc";
 			os = "linux";
-			compression = "gzip";
+			compression = "none";
 			load = <00000000>;
 			entry = <00000000>;
 			hash-1 {
-- 
2.11.0

[U-Boot] [PATCH 1/2] doc: FIT image: clarify usage of "compression" property

[Wolfgang Denk]

Dear Simon,

In message <20180730105319.79424-1-sgoldschmidt@de.pepperl-fuchs.com> you wrote:
> Compressed images should have their compression property
> set to "none" if U-Boot should leave them compressed.
> 
> This is especially the case for compressed ramdisks that
> should be uncompressed by the kernel only.

Is this not self-explaining as is?  When you use "none", U-boot wil
do nothing to the data - it passes it on unchanged as binay blob.

I donrt see the need for this additional explanation of what seems
obvious to me.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If you're not part of the solution, then you're part of the  precipi-
tate.

[U-Boot] [PATCH 1/2] doc: FIT image: clarify usage of "compression" property

[Tom Rini]

On Mon, Jul 30, 2018 at 01:19:16PM +0200, Wolfgang Denk wrote:

> Dear Simon,
> 
> In message <20180730105319.79424-1-sgoldschmidt@de.pepperl-fuchs.com> you wrote:
> > Compressed images should have their compression property
> > set to "none" if U-Boot should leave them compressed.
> > 
> > This is especially the case for compressed ramdisks that
> > should be uncompressed by the kernel only.
> 
> Is this not self-explaining as is?  When you use "none", U-boot wil
> do nothing to the data - it passes it on unchanged as binay blob.
> 
> I donrt see the need for this additional explanation of what seems
> obvious to me.

Ah, but it's not spelled out.  And also given that currently we don't
decompress say a ramdisk that spells out compression = "gzip" (is that a
regression from initial FIT behavior?) it helps to be clear that putting
none here for "don't touch the data" is valid and that the compression
property isn't just a descriptive one, U-Boot may decompress the data.

-- 
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/20180730/f735c856/attachment.sig>

[U-Boot] [PATCH 1/2] doc: FIT image: clarify usage of "compression" property

[Simon Goldschmidt]

On 30.07.2018 13:19, Wolfgang Denk wrote:
> Dear Simon,
> 
> In message <20180730105319.79424-1-sgoldschmidt@de.pepperl-fuchs.com> you wrote:
>> Compressed images should have their compression property
>> set to "none" if U-Boot should leave them compressed.
>>
>> This is especially the case for compressed ramdisks that
>> should be uncompressed by the kernel only.
> 
> Is this not self-explaining as is?  When you use "none", U-boot wil
> do nothing to the data - it passes it on unchanged as binay blob.
> 
> I donrt see the need for this additional explanation of what seems
> obvious to me.

This has been explicitly requested in this mail:
https://lists.denx.de/pipermail/u-boot/2018-July/336435.html

It might seem obvious to you, but given the examples had both "none" and 
"gzip" for ramdisk, it seems it has not been obvious for everybody.


Simon

[U-Boot] [PATCH 2/2] FIT image: use compression = "none" for ramdisks

[Wolfgang Denk]

Dear Simon,

In message <20180730105319.79424-2-sgoldschmidt@de.pepperl-fuchs.com> you wrote:
> To prepare supporting compression for all image types, change
> compression to "none" for ramdisks in all examples.

What makes you think this is a correct thing to do?

There are different approaches to handle things.  For example,
traditionally on Power Architecture we would use a raw kernel
binray, compress this (for example with gzip) before wrapping it
with mkimage into an (uImage or FIT) U-Boot image, and then let
U-Boot uncompress the kernel image into rum and start it.  On ARM
the kernel comes traditionally with it's own wrapper that does
unompressions and such.

Same for ramdisk handling.  On some systems it may make sense to
have U-Boot handle the uncompressing, so compression = "gzip";
may be fully intentional.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Pardon me for breathing, which I never do anyway so I don't know why
I bother to say it, oh God, I'm so depressed. Here's another of those
self-satisfied doors. Life! Don't talk to me about life."
                                        - Marvin the Paranoid Android

[U-Boot] [PATCH 1/2] doc: FIT image: clarify usage of "compression" property

[Wolfgang Denk]

Dear Simon,

In message <6009778b-1c55-d67b-26a5-7d9039c85e47@de.pepperl-fuchs.com> you wrote:
> 
> It might seem obvious to you, but given the examples had both "none" and 
> "gzip" for ramdisk, it seems it has not been obvious for everybody.

These may be different examples, documenting different use cases
which do exactly what they say?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The shortest unit of time in the multiverse is the News York  Second,
defined  as  the  period  of  time between the traffic lights turning
green and the cab behind you honking.
                                - Terry Pratchett, _Lords and Ladies_

[U-Boot] [PATCH 1/2] doc: FIT image: clarify usage of "compression" property

[Simon Goldschmidt]

Dear Wolfgang,

On 30.07.2018 13:46, Wolfgang Denk wrote:
> Dear Simon,
> 
> In message <6009778b-1c55-d67b-26a5-7d9039c85e47@de.pepperl-fuchs.com> you wrote:
>>
>> It might seem obvious to you, but given the examples had both "none" and
>> "gzip" for ramdisk, it seems it has not been obvious for everybody.
> 
> These may be different examples, documenting different use cases
> which do exactly what they say?

That might well be, but given that compression in FIT images only works 
for kernel sub-images up to now, I strongly doubt that.

Best Regards,
Simon

[U-Boot] [PATCH 2/2] FIT image: use compression = "none" for ramdisks

[Simon Goldschmidt]

Dear Wolfgang,

On 30.07.2018 13:45, Wolfgang Denk wrote:
> Dear Simon,
> 
> In message <20180730105319.79424-2-sgoldschmidt@de.pepperl-fuchs.com> you wrote:
>> To prepare supporting compression for all image types, change
>> compression to "none" for ramdisks in all examples.
> 
> What makes you think this is a correct thing to do?
> 
> There are different approaches to handle things.  For example,
> traditionally on Power Architecture we would use a raw kernel
> binray, compress this (for example with gzip) before wrapping it
> with mkimage into an (uImage or FIT) U-Boot image, and then let
> U-Boot uncompress the kernel image into rum and start it.  On ARM
> the kernel comes traditionally with it's own wrapper that does
> unompressions and such.
> 
> Same for ramdisk handling.  On some systems it may make sense to
> have U-Boot handle the uncompressing, so compression = "gzip";
> may be fully intentional.

That's the whole point and in the thread I mentioned 
(https://lists.denx.de/pipermail/u-boot/2018-July/336435.html) it has 
been discussed that this is the future goal.

However, uncompression currently is only implemented for kernels, not 
for other sub-images. This patch aims at updating the docs and the 
current .its examples to what they do now.

Best regards,
Simon

[U-Boot] [PATCH 1/2] doc: FIT image: clarify usage of "compression" property

[Wolfgang Denk]

Dear Simon,

In message <a218ccb8-a461-b119-f2d7-d1cb7c88b8c6@de.pepperl-fuchs.com> you wrote:
>
> > These may be different examples, documenting different use cases
> > which do exactly what they say?
>
> That might well be, but given that compression in FIT images only works 
> for kernel sub-images up to now, I strongly doubt that.

This might actually be a regression the image handling rework that
went unnoticed for a long time.  I'm pretty sure that the original
implementation of FIT images handled this correctly.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Plan to throw one away. You will anyway."
                              - Fred Brooks, "The Mythical Man Month"

[U-Boot] [PATCH 1/2] doc: FIT image: clarify usage of "compression" property

[Simon Goldschmidt]

On 30.07.2018 14:32, Wolfgang Denk wrote:
> Dear Simon,
> 
> In message <a218ccb8-a461-b119-f2d7-d1cb7c88b8c6@de.pepperl-fuchs.com> you wrote:
>>
>>> These may be different examples, documenting different use cases
>>> which do exactly what they say?
>>
>> That might well be, but given that compression in FIT images only works
>> for kernel sub-images up to now, I strongly doubt that.
> 
> This might actually be a regression the image handling rework that
> went unnoticed for a long time.  I'm pretty sure that the original
> implementation of FIT images handled this correctly.

In that case, we should ignore parts of patch 2/2, of course. I'm pretty 
sure the default examples would still leave the ramdisk compressed for 
the kernel to uncompress though.

As I'm not that long with U-Boot, can you point me to a rough date of a 
release that I could check to see if it worked at that time?

Best regards,
Simon

[U-Boot] [PATCH 2/2] FIT image: use compression = "none" for ramdisks

[Wolfgang Denk]

Dear Simon,

In message <6bce9049-a812-d8e4-cd2a-4b86dccae67e@de.pepperl-fuchs.com> you wrote:
>
> However, uncompression currently is only implemented for kernels, not 
> for other sub-images. This patch aims at updating the docs and the 
> current .its examples to what they do now.

Yes, but the current state should IMO not been taken as reference.
I don't have time ATM to achtually check with older versions, but
I'm pretty sure that that was handled correctly in the original FIT
implementation, and the examples made sense at the time they were
being written.  I suspect that later changes broke the code, so only
the code needs to be fixed.

In general for the image handling there should be no distinction at
all whether this is a kernel or ramdisk or FPGA or whatever image -
compression is a feature independent of the image type, and that's
how the code should handle it.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Why can you only have two doors on a chicken coop? If it had four  it
would be a chicken sedan.

[U-Boot] [PATCH 1/2] doc: FIT image: clarify usage of "compression" property

[Wolfgang Denk]

Dear Simon,

In message <642b8dae-f941-5255-42c7-3761e12d04cb@de.pepperl-fuchs.com> you wrote:
> 
> As I'm not that long with U-Boot, can you point me to a rough date of a 
> release that I could check to see if it worked at that time?

I can only speculate...  The first bigger rework of the cose appears
to be this patch series:

30864 859e92b775   2013-05-14 15:37:25 -0400   image: Move timestamp #ifdefs to header file
30863 61a439a873   2013-05-14 15:37:25 -0400   image: Export fit_check_ramdisk()
30862 53fbb7e885   2013-05-14 15:37:25 -0400   image: Split FIT code into new image-fit.c
30861 604f23dde0   2013-05-14 15:37:25 -0400   image: Move HOSTCC image code to tools/
30860 94e5fa46a0   2013-05-14 15:37:25 -0400   image: Split hash node processing into its own function
30859 b7260910dc   2013-05-14 15:37:25 -0400   image: Convert fit_image_hash_set_value() to static, and rename
30858 b8da836650   2013-05-14 15:37:25 -0400   image: Rename fit_image_check_hashes() to fit_image_verify()
30857 ab9efc665a   2013-05-14 15:37:25 -0400   image: Move hash checking into its own function
30856 e754da2aee   2013-05-14 15:37:25 -0400   image: Move error! string to common place
30855 003efd7da4   2013-05-14 15:37:25 -0400   image: Export fit_conf_get_prop_node()
30854 bbb467dc3c   2013-05-14 15:37:25 -0400   image: Rename fit_add_hashes() to fit_add_verification_data()
30853 d8b75360ee   2013-05-14 15:37:25 -0400   image: Rename hash printing to fit_image_print_verification_dat      a()
30852 35e7b0f179   2013-05-14 15:37:25 -0400   sandbox: image: Add support for booting images in sandbox
30851 aa6d6db4d4   2013-05-14 15:37:25 -0400   mkimage: Put FIT loading in function and tidy error handling
30850 1fe7d93891   2013-05-14 15:37:25 -0400   image: Remove remaining #ifdefs in image-fit.c
30849 87ebee39e9   2013-05-14 15:37:25 -0400   image: Add CONFIG_FIT_SPL_PRINT to control FIT image printing i      n SPL
30848 44d3a3066b   2013-05-14 15:37:25 -0400   image: Split libfdt code into image-fdt.c
30847 13d06981a9   2013-05-14 15:37:25 -0400   image: Add device tree setup to image library
30846 c19d13b030   2013-05-14 15:37:25 -0400   arm: Refactor bootm to reduce #ifdefs
30845 6caa195614   2013-05-14 15:37:25 -0400   arm: Use image_setup_linux() instead of local code
30844 3e51266a4e   2013-05-14 15:37:25 -0400   powerpc: Use image_setup_linux() instead of local code
30843 24507cf50a   2013-05-14 15:37:26 -0400   m68k: Use image_setup_linux() instead of local code
30842 2a08b740e3   2013-05-14 15:37:26 -0400   sparc: Use image_setup_linux() instead of local code

But there have been earlier changes, and many later (heavy) reworks,
too.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Nothing ever becomes real until it is experienced.       - John Keats

[U-Boot] [PATCH 2/2] FIT image: use compression = "none" for ramdisks

[Simon Goldschmidt]

On 30.07.2018 14:36, Wolfgang Denk wrote:
> Dear Simon,
> 
> In message <6bce9049-a812-d8e4-cd2a-4b86dccae67e@de.pepperl-fuchs.com> you wrote:
>>
>> However, uncompression currently is only implemented for kernels, not
>> for other sub-images. This patch aims at updating the docs and the
>> current .its examples to what they do now.
> 
> Yes, but the current state should IMO not been taken as reference.
> I don't have time ATM to achtually check with older versions, but
> I'm pretty sure that that was handled correctly in the original FIT
> implementation, and the examples made sense at the time they were
> being written.  I suspect that later changes broke the code, so only
> the code needs to be fixed.
> 
> In general for the image handling there should be no distinction at
> all whether this is a kernel or ramdisk or FPGA or whatever image -
> compression is a feature independent of the image type, and that's
> how the code should handle it.

OK, so at least we all seem to have a common sense of how it should be :-)

I'd still argue that the standard example in 'multi.its' should have 
compression = "none" for the ramdisks.

Best regards,
Simon

[U-Boot] [PATCH 1/2] doc: FIT image: clarify usage of "compression" property

[Simon Goldschmidt]

On 30.07.2018 14:44, Wolfgang Denk wrote:
> Dear Simon,
> 
> In message <642b8dae-f941-5255-42c7-3761e12d04cb@de.pepperl-fuchs.com> you wrote:
>>
>> As I'm not that long with U-Boot, can you point me to a rough date of a
>> release that I could check to see if it worked at that time?
> 
> I can only speculate...  The first bigger rework of the cose appears
> to be this patch series:
> 
> 30864 859e92b775   2013-05-14 15:37:25 -0400   image: Move timestamp #ifdefs to header file
> 30863 61a439a873   2013-05-14 15:37:25 -0400   image: Export fit_check_ramdisk()
> 30862 53fbb7e885   2013-05-14 15:37:25 -0400   image: Split FIT code into new image-fit.c
> 30861 604f23dde0   2013-05-14 15:37:25 -0400   image: Move HOSTCC image code to tools/
> 30860 94e5fa46a0   2013-05-14 15:37:25 -0400   image: Split hash node processing into its own function
> 30859 b7260910dc   2013-05-14 15:37:25 -0400   image: Convert fit_image_hash_set_value() to static, and rename
> 30858 b8da836650   2013-05-14 15:37:25 -0400   image: Rename fit_image_check_hashes() to fit_image_verify()
> 30857 ab9efc665a   2013-05-14 15:37:25 -0400   image: Move hash checking into its own function
> 30856 e754da2aee   2013-05-14 15:37:25 -0400   image: Move error! string to common place
> 30855 003efd7da4   2013-05-14 15:37:25 -0400   image: Export fit_conf_get_prop_node()
> 30854 bbb467dc3c   2013-05-14 15:37:25 -0400   image: Rename fit_add_hashes() to fit_add_verification_data()
> 30853 d8b75360ee   2013-05-14 15:37:25 -0400   image: Rename hash printing to fit_image_print_verification_dat      a()
> 30852 35e7b0f179   2013-05-14 15:37:25 -0400   sandbox: image: Add support for booting images in sandbox
> 30851 aa6d6db4d4   2013-05-14 15:37:25 -0400   mkimage: Put FIT loading in function and tidy error handling
> 30850 1fe7d93891   2013-05-14 15:37:25 -0400   image: Remove remaining #ifdefs in image-fit.c
> 30849 87ebee39e9   2013-05-14 15:37:25 -0400   image: Add CONFIG_FIT_SPL_PRINT to control FIT image printing i      n SPL
> 30848 44d3a3066b   2013-05-14 15:37:25 -0400   image: Split libfdt code into image-fdt.c
> 30847 13d06981a9   2013-05-14 15:37:25 -0400   image: Add device tree setup to image library
> 30846 c19d13b030   2013-05-14 15:37:25 -0400   arm: Refactor bootm to reduce #ifdefs
> 30845 6caa195614   2013-05-14 15:37:25 -0400   arm: Use image_setup_linux() instead of local code
> 30844 3e51266a4e   2013-05-14 15:37:25 -0400   powerpc: Use image_setup_linux() instead of local code
> 30843 24507cf50a   2013-05-14 15:37:26 -0400   m68k: Use image_setup_linux() instead of local code
> 30842 2a08b740e3   2013-05-14 15:37:26 -0400   sparc: Use image_setup_linux() instead of local code
> 
> But there have been earlier changes, and many later (heavy) reworks,
> too.

I have checked starting with v1.3.3 (i.e. v2008.05) and even then, the 
compression property has only been handled for kernels. It seems like 
only the kernel has been loaded from FIT back then...

Next I checked v2009.01, where FDT and ramdisk are loaded from FIT, too. 
There is even a check that FDT is uncompressed. For ramdisk, the 
compression property is not checked.

Reading that, I tend to think uncompression has always been like it is now.

Best Regards,
Simon

> 
> Best regards,
> 
> Wolfgang Denk
> 

[U-Boot] [PATCH 2/2] FIT image: use compression = "none" for ramdisks

[Wolfgang Denk]

Dear Simon,

In message <0787afb7-2da1-4eaa-437e-6f7c7582809c@de.pepperl-fuchs.com> you wrote:
> 
> I'd still argue that the standard example in 'multi.its' should have 
> compression = "none" for the ramdisks.

OK, this is your position then.  I can only explain where we are
coming from: in the early days of U-Boot (well, PPCBoot by then)
resources were much tighter than today - look at the examples in the
README: a v2.4.4 Linux kernel image would be around 780 kB
uncompressed or 330 kB compressed.  At this time it was considered a
waste of resources to have the gzip uncompression code in the boot
loader and again duplicated in the Linux kernel - so it was omitted
there and U-Boot would do all uncompression, also of the ramdisks.
I agree this is not standard any more, but it is still a valid use
case, I think.  I know, I'm an old man ;-)

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Brain: an apparatus with which we think we think.    - Ambrose Bierce

[U-Boot] [PATCH 2/2] FIT image: use compression = "none" for ramdisks

[Simon Goldschmidt]

On 30.07.2018 15:25, Wolfgang Denk wrote:
> Dear Simon,
> 
> In message <0787afb7-2da1-4eaa-437e-6f7c7582809c@de.pepperl-fuchs.com> you wrote:
>>
>> I'd still argue that the standard example in 'multi.its' should have
>> compression = "none" for the ramdisks.
> 
> OK, this is your position then.  I can only explain where we are
> coming from: in the early days of U-Boot (well, PPCBoot by then)
> resources were much tighter than today - look at the examples in the
> README: a v2.4.4 Linux kernel image would be around 780 kB
> uncompressed or 330 kB compressed.  At this time it was considered a
> waste of resources to have the gzip uncompression code in the boot
> loader and again duplicated in the Linux kernel - so it was omitted
> there and U-Boot would do all uncompression, also of the ramdisks.

Well, my point was that I couldn't see U-Boot ever handling this. But 
this discussion got a bit confused, split into two threads...

> I agree this is not standard any more, but it is still a valid use
> case, I think.  I know, I'm an old man ;-)

It *is* a valid use case. But for current kernels (if compression is 
enabled), it might be faster to let the kernel unzip the content 
directly to the memory location where it should reside in the end.

However, I don't have a strong opinion on these examples. I'll let the 
maintainers decide :-)


Best regards,
Simon

[U-Boot] [PATCH 2/2] FIT image: use compression = "none" for ramdisks

[Wolfgang Denk]

Dear Simon,

In message <97a1d1e5-4415-4db4-9c77-72023f37bb4f@de.pepperl-fuchs.com> you wrote:
> 
> It *is* a valid use case. But for current kernels (if compression is 
> enabled), it might be faster to let the kernel unzip the content 
> directly to the memory location where it should reside in the end.

This was true a long, long time ago on ARM (and this is where most
of the unclean implmentations originate), when U-Boot did not enable
caches.  With this fixed, it is almost always wrong, because we now
have an unnecessary memory copy: U-Boot loads the compressed kernel
image to one location in RAM, which then runs and uncompresses the
code to another address.  Is it not obvious that it would be more
efficient if U-Boot did the uncompressing directly? 


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
For every complex problem, there is a solution that is simple,  neat,
and wrong.                                           -- H. L. Mencken

[U-Boot] [U-Boot, 1/2] doc: FIT image: clarify usage of "compression" property

[Tom Rini]

On Mon, Jul 30, 2018 at 12:53:18PM +0200, Simon Goldschmidt wrote:

> Compressed images should have their compression property
> set to "none" if U-Boot should leave them compressed.
> 
> This is especially the case for compressed ramdisks that
> should be uncompressed by the kernel only.
> 
> Signed-off-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>

Applied to u-boot/master, thanks!

-- 
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/20180810/f3c06d31/attachment.sig>

[U-Boot] [U-Boot, 2/2] FIT image: use compression = "none" for ramdisks

[Tom Rini]

On Mon, Jul 30, 2018 at 12:53:19PM +0200, Simon Goldschmidt wrote:

> To prepare supporting compression for all image types, change
> compression to "none" for ramdisks in all examples.
> 
> Signed-off-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>
> ---
> 
>  arch/arm/cpu/armv8/fsl-layerscape/doc/README.falcon | 2 +-
>  doc/README.bcm7xxx                                  | 2 +-
>  doc/uImage.FIT/multi.its                            | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)

Sorry for the delay.  I think this patch as-is, is wrong.  Please split
it into a patch per file that CCs the appropriate custodian(s) for them
to ack/nak and pick up.  It's also appropriate to do a patch now that
corrects the behavior of uncompressing ramdisks that can be uncompressed
automatically (and no need for a CONFIG option).  Thanks!

-- 
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/20180817/f7a8ca68/attachment.sig>

[U-Boot] [U-Boot, 2/2] FIT image: use compression = "none" for ramdisks

[Simon Goldschmidt]

On Sat, Aug 18, 2018 at 1:14 AM Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Jul 30, 2018 at 12:53:19PM +0200, Simon Goldschmidt wrote:
>
> > To prepare supporting compression for all image types, change
> > compression to "none" for ramdisks in all examples.
> >
> > Signed-off-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>
> > ---
> >
> >  arch/arm/cpu/armv8/fsl-layerscape/doc/README.falcon | 2 +-
> >  doc/README.bcm7xxx                                  | 2 +-
> >  doc/uImage.FIT/multi.its                            | 4 ++--
> >  3 files changed, 4 insertions(+), 4 deletions(-)
>
> Sorry for the delay.  I think this patch as-is, is wrong.  Please split
> it into a patch per file that CCs the appropriate custodian(s) for them
> to ack/nak and pick up.  It's also appropriate to do a patch now that
> corrects the behavior of uncompressing ramdisks that can be uncompressed
> automatically (and no need for a CONFIG option).  Thanks!

OK, I'll rework it. But I won't have time for that until in two weeks
or even later, so I won't be able to provide something for 2018.09.

Simon