| Message ID | 20180730105319.79424-1-sgoldschmidt@de.pepperl-fuchs.com |
|---|---|
| State | Accepted |
| Commit | fd15a9e2565f831bf95c2152d1966d068a642175 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot,1/2] doc: FIT image: clarify usage of "compression" property | expand |
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
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.
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
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
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
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
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
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
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 >
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!
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
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(-)