[U-Boot] [PATCH v2 0/5] Extend FIT support for falcon boot
This patch set extends FIT support for falcon boot. The difference between U-Boot FIT and Linux FIT lies not only on the images inside, but also the data offset. U-Boot FIT image has data outside of the FIT structure while Linux FIT image can have data embedded within. Linux FIT can use compressed image as well.
Changes in v2: Combine Kconfig change and actual code into one patch Rebase on top of "SPL: FIT: factor out spl_load_fit_image()" by Andre Przywara Split from previous patch, rebased on top of "SPL: FIT: allow loading multiple images" by Andre Przywara.
York Sun (5): tools: pblimage: Fix address calculation cmd: spl: Fix compiling warning spl: fit: Eanble GZIP support for image decompression spl: fit: Support both external and embedded data spl: fit: Add booting OS first
cmd/spl.c | 8 +-- common/spl/spl_fit.c | 140 ++++++++++++++++++++++++++++++++++++--------------- lib/Kconfig | 8 +++ lib/Makefile | 5 +- tools/pblimage.c | 2 +- 5 files changed, 116 insertions(+), 47 deletions(-)
The image size should be added to the initial pbl command, not bit "ORed".
Signed-off-by: York Sun york.sun@nxp.com ---
Changes in v2: None
tools/pblimage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/pblimage.c b/tools/pblimage.c index ffc3268..d25a733 100644 --- a/tools/pblimage.c +++ b/tools/pblimage.c @@ -293,7 +293,7 @@ int pblimage_check_params(struct image_tool_params *params) pbi_crc_cmd2 = 0; pbl_cmd_initaddr = params->addr & PBL_ADDR_24BIT_MASK; pbl_cmd_initaddr |= PBL_ACS_CONT_CMD; - pbl_cmd_initaddr |= uboot_size; + pbl_cmd_initaddr += uboot_size; pbl_end_cmd[0] = 0x09610000; pbl_end_cmd[1] = 0x00000000; pbl_end_cmd[2] = 0x096100c0;
Fix warning "cast from pointer to integer of different size".
Signed-off-by: York Sun york.sun@nxp.com ---
Changes in v2: None
cmd/spl.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/cmd/spl.c b/cmd/spl.c index 057764a..562140a 100644 --- a/cmd/spl.c +++ b/cmd/spl.c @@ -108,12 +108,12 @@ static int spl_export(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
c = find_cmd_tbl(argv[1], &cmd_spl_export_sub[0], ARRAY_SIZE(cmd_spl_export_sub)); - if ((c) && ((int)c->cmd <= SPL_EXPORT_LAST)) { + if ((c) && ((long)c->cmd <= SPL_EXPORT_LAST)) { argc -= 2; argv += 2; - if (call_bootm(argc, argv, subcmd_list[(int)c->cmd])) + if (call_bootm(argc, argv, subcmd_list[(long)c->cmd])) return -1; - switch ((int)c->cmd) { + switch ((long)c->cmd) { #ifdef CONFIG_OF_LIBFDT case SPL_EXPORT_FDT: printf("Argument image is now in RAM: 0x%p\n", @@ -147,7 +147,7 @@ static int do_spl(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
c = find_cmd_tbl(argv[1], &cmd_spl_sub[0], ARRAY_SIZE(cmd_spl_sub)); if (c) { - cmd = (int)c->cmd; + cmd = (long)c->cmd; switch (cmd) { case SPL_EXPORT: argc--;
On Mon, Aug 07, 2017 at 04:16:23PM -0700, York Sun wrote:
Fix warning "cast from pointer to integer of different size".
Signed-off-by: York Sun york.sun@nxp.com
Reviewed-by: Tom Rini trini@konsulko.com
Add Kconfig option SPL_GZIP and SPL_ZLIB to enable gunzip support for SPL boot, eg. falcon boot compressed kernel image.
Signed-off-by: York Sun york.sun@nxp.com
---
Changes in v2: Combine Kconfig change and actual code into one patch
common/spl/spl_fit.c | 28 ++++++++++++++++++++++++++-- lib/Kconfig | 8 ++++++++ lib/Makefile | 5 +++-- 3 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index d2a352e..23f85d2 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -135,6 +135,19 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector, ulong overhead; int nr_sectors; int align_len = ARCH_DMA_MINALIGN - 1; +#if defined(CONFIG_SPL_OS_BOOT) && defined(CONFIG_SPL_GZIP) + uint8_t image_comp, type; + + if (fit_image_get_comp(fit, node, &image_comp)) + puts("Cannot get image compression format.\n"); + else + debug("%s ", genimg_get_comp_name(image_comp)); + + if (fit_image_get_type(fit, node, &type)) + puts("Cannot get image type.\n"); + else + debug("%s ", genimg_get_type_name(type)); +#endif
offset = fdt_getprop_u32(fit, node, "data-offset"); if (offset == FDT_ERROR) @@ -154,7 +167,7 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector, if (info->read(info, sector + get_aligned_image_offset(info, offset), nr_sectors, (void*)load_ptr) != nr_sectors) return -EIO; - debug("image: dst=%lx, offset=%lx, size=%lx\n", load_ptr, offset, + debug("image dst=%lx, offset=%lx, size=%lx\n", load_ptr, offset, (unsigned long)length);
src = (void *)load_ptr + overhead; @@ -162,7 +175,18 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector, board_fit_image_post_process(&src, &length); #endif
- memcpy((void*)load_addr, src, length); +#if defined(CONFIG_SPL_OS_BOOT) && defined(CONFIG_SPL_GZIP) + if (image_comp == IH_COMP_GZIP && type == IH_TYPE_KERNEL) { + if (gunzip((void *)load_addr, CONFIG_SYS_BOOTM_LEN, + src, &length)) { + puts("Uncompressing error\n"); + return -EIO; + } + } else +#endif + { + memcpy((void *)load_addr, src, length); + }
if (image_info) { image_info->load_addr = load_addr; diff --git a/lib/Kconfig b/lib/Kconfig index 2f5a210..a3c6520 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -160,6 +160,14 @@ config LZO bool "Enable LZO decompression support" help This enables support for LZO compression algorithm.r + +config SPL_GZIP + bool "Enable gzip decompression support for SPL build" + select SPL_ZLIB + +config SPL_ZLIB + bool + endmenu
config ERRNO_STR diff --git a/lib/Makefile b/lib/Makefile index eacc7d6..455cc9d 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -11,7 +11,6 @@ obj-$(CONFIG_EFI) += efi/ obj-$(CONFIG_EFI_LOADER) += efi_loader/ obj-$(CONFIG_LZMA) += lzma/ obj-$(CONFIG_LZO) += lzo/ -obj-$(CONFIG_ZLIB) += zlib/ obj-$(CONFIG_BZIP2) += bzip2/ obj-$(CONFIG_TIZEN) += tizen/ obj-$(CONFIG_FIT) += libfdt/ @@ -26,7 +25,6 @@ obj-y += crc16.o obj-$(CONFIG_ERRNO_STR) += errno_str.o obj-$(CONFIG_FIT) += fdtdec_common.o obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o -obj-$(CONFIG_GZIP) += gunzip.o obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o obj-y += initcall.o @@ -49,6 +47,9 @@ obj-$(CONFIG_RSA) += rsa/ obj-$(CONFIG_SHA1) += sha1.o obj-$(CONFIG_SHA256) += sha256.o
+obj-$(CONFIG_$(SPL_)ZLIB) += zlib/ +obj-$(CONFIG_$(SPL_)GZIP) += gunzip.o + obj-$(CONFIG_SPL_SAVEENV) += qsort.o obj-$(CONFIG_$(SPL_)OF_LIBFDT) += libfdt/ ifneq ($(CONFIG_SPL_BUILD)$(CONFIG_SPL_OF_PLATDATA),yy)
On Mon, Aug 07, 2017 at 04:16:24PM -0700, York Sun wrote:
Add Kconfig option SPL_GZIP and SPL_ZLIB to enable gunzip support for SPL boot, eg. falcon boot compressed kernel image.
Signed-off-by: York Sun york.sun@nxp.com
Reviewed-by: Tom Rini trini@konsulko.com
Hi York,
On 7 August 2017 at 17:16, York Sun york.sun@nxp.com wrote:
Add Kconfig option SPL_GZIP and SPL_ZLIB to enable gunzip support for SPL boot, eg. falcon boot compressed kernel image.
Signed-off-by: York Sun york.sun@nxp.com
Changes in v2: Combine Kconfig change and actual code into one patch
common/spl/spl_fit.c | 28 ++++++++++++++++++++++++++-- lib/Kconfig | 8 ++++++++ lib/Makefile | 5 +++-- 3 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index d2a352e..23f85d2 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -135,6 +135,19 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector, ulong overhead; int nr_sectors; int align_len = ARCH_DMA_MINALIGN - 1; +#if defined(CONFIG_SPL_OS_BOOT) && defined(CONFIG_SPL_GZIP)
If these are in Kconfig can we use if (IS_ENABLED()) instead of #if ?
uint8_t image_comp, type;if (fit_image_get_comp(fit, node, &image_comp))puts("Cannot get image compression format.\n");elsedebug("%s ", genimg_get_comp_name(image_comp));if (fit_image_get_type(fit, node, &type))puts("Cannot get image type.\n");elsedebug("%s ", genimg_get_type_name(type));+#endif
offset = fdt_getprop_u32(fit, node, "data-offset"); if (offset == FDT_ERROR)@@ -154,7 +167,7 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector, if (info->read(info, sector + get_aligned_image_offset(info, offset), nr_sectors, (void*)load_ptr) != nr_sectors) return -EIO;
debug("image: dst=%lx, offset=%lx, size=%lx\n", load_ptr, offset,
debug("image dst=%lx, offset=%lx, size=%lx\n", load_ptr, offset, (unsigned long)length); src = (void *)load_ptr + overhead;@@ -162,7 +175,18 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector, board_fit_image_post_process(&src, &length); #endif
memcpy((void*)load_addr, src, length);+#if defined(CONFIG_SPL_OS_BOOT) && defined(CONFIG_SPL_GZIP)
if (image_comp == IH_COMP_GZIP && type == IH_TYPE_KERNEL) {if (gunzip((void *)load_addr, CONFIG_SYS_BOOTM_LEN,src, &length)) {puts("Uncompressing error\n");return -EIO;}} else+#endif
{memcpy((void *)load_addr, src, length);} if (image_info) { image_info->load_addr = load_addr;diff --git a/lib/Kconfig b/lib/Kconfig index 2f5a210..a3c6520 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -160,6 +160,14 @@ config LZO bool "Enable LZO decompression support" help This enables support for LZO compression algorithm.r
+config SPL_GZIP
bool "Enable gzip decompression support for SPL build"select SPL_ZLIB+config SPL_ZLIB
bool
Help?
endmenu
config ERRNO_STR diff --git a/lib/Makefile b/lib/Makefile index eacc7d6..455cc9d 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -11,7 +11,6 @@ obj-$(CONFIG_EFI) += efi/ obj-$(CONFIG_EFI_LOADER) += efi_loader/ obj-$(CONFIG_LZMA) += lzma/ obj-$(CONFIG_LZO) += lzo/ -obj-$(CONFIG_ZLIB) += zlib/ obj-$(CONFIG_BZIP2) += bzip2/ obj-$(CONFIG_TIZEN) += tizen/ obj-$(CONFIG_FIT) += libfdt/ @@ -26,7 +25,6 @@ obj-y += crc16.o obj-$(CONFIG_ERRNO_STR) += errno_str.o obj-$(CONFIG_FIT) += fdtdec_common.o obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o -obj-$(CONFIG_GZIP) += gunzip.o obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o obj-y += initcall.o @@ -49,6 +47,9 @@ obj-$(CONFIG_RSA) += rsa/ obj-$(CONFIG_SHA1) += sha1.o obj-$(CONFIG_SHA256) += sha256.o
+obj-$(CONFIG_$(SPL_)ZLIB) += zlib/ +obj-$(CONFIG_$(SPL_)GZIP) += gunzip.o
obj-$(CONFIG_SPL_SAVEENV) += qsort.o obj-$(CONFIG_$(SPL_)OF_LIBFDT) += libfdt/ ifneq ($(CONFIG_SPL_BUILD)$(CONFIG_SPL_OF_PLATDATA),yy) -- 2.7.4
Regards, Simon
On 08/13/2017 02:35 PM, Simon Glass wrote:
Hi York,
On 7 August 2017 at 17:16, York Sun york.sun@nxp.com wrote:
Add Kconfig option SPL_GZIP and SPL_ZLIB to enable gunzip support for SPL boot, eg. falcon boot compressed kernel image.
Signed-off-by: York Sun york.sun@nxp.com
Changes in v2: Combine Kconfig change and actual code into one patch
common/spl/spl_fit.c | 28 ++++++++++++++++++++++++++-- lib/Kconfig | 8 ++++++++ lib/Makefile | 5 +++-- 3 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index d2a352e..23f85d2 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -135,6 +135,19 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector, ulong overhead; int nr_sectors; int align_len = ARCH_DMA_MINALIGN - 1; +#if defined(CONFIG_SPL_OS_BOOT) && defined(CONFIG_SPL_GZIP)
If these are in Kconfig can we use if (IS_ENABLED()) instead of #if ?
Sure thing. Will change in next version.
York
On 08/08/2017 01:16 AM, York Sun wrote:
Add Kconfig option SPL_GZIP and SPL_ZLIB to enable gunzip support for SPL boot, eg. falcon boot compressed kernel image.
Signed-off-by: York Sun york.sun@nxp.com Reviewed-by: Tom Rini trini@konsulko.com
Changes in v2: Combine Kconfig change and actual code into one patch
common/spl/spl_fit.c | 28 ++++++++++++++++++++++++++-- lib/Kconfig | 8 ++++++++ lib/Makefile | 5 +++-- 3 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index d2a352e..23f85d2 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -135,6 +135,19 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector, ulong overhead; int nr_sectors; int align_len = ARCH_DMA_MINALIGN - 1; +#if defined(CONFIG_SPL_OS_BOOT) && defined(CONFIG_SPL_GZIP)
- uint8_t image_comp, type;
- if (fit_image_get_comp(fit, node, &image_comp))
puts("Cannot get image compression format.\n");- else
debug("%s ", genimg_get_comp_name(image_comp));- if (fit_image_get_type(fit, node, &type))
puts("Cannot get image type.\n");- else
debug("%s ", genimg_get_type_name(type));+#endif
offset = fdt_getprop_u32(fit, node, "data-offset"); if (offset == FDT_ERROR) @@ -154,7 +167,7 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector, if (info->read(info, sector + get_aligned_image_offset(info, offset), nr_sectors, (void*)load_ptr) != nr_sectors) return -EIO;
- debug("image: dst=%lx, offset=%lx, size=%lx\n", load_ptr, offset,
debug("image dst=%lx, offset=%lx, size=%lx\n", load_ptr, offset, (unsigned long)length);
src = (void *)load_ptr + overhead;
@@ -162,7 +175,18 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector, board_fit_image_post_process(&src, &length); #endif
- memcpy((void*)load_addr, src, length);
+#if defined(CONFIG_SPL_OS_BOOT) && defined(CONFIG_SPL_GZIP)
- if (image_comp == IH_COMP_GZIP && type == IH_TYPE_KERNEL) {
if (gunzip((void *)load_addr, CONFIG_SYS_BOOTM_LEN,src, &length)) {
In this file length is defined as size_t.
In include/common.h the last parameter of gunzip is defined as unsigned long *.
This leads to a compilation warning and probably incorrect results: CC spl/common/spl/spl_fit.o common/spl/spl_fit.c: In function ‘spl_load_fit_image’: common/spl/spl_fit.c:201:12: warning: passing argument 4 of ‘gunzip’ from incompatible pointer type [-Wincompatible-pointer-types] src, &length)) {
Please, correct the patch to pass a compatible parameter. The patch already made it into the U-Boot git master. So possibly you want to send a follow up patch.
Best regards
Heinrich
puts("Uncompressing error\n");return -EIO;}- } else
+#endif
{
memcpy((void *)load_addr, src, length);}
if (image_info) { image_info->load_addr = load_addr;
diff --git a/lib/Kconfig b/lib/Kconfig index 2f5a210..a3c6520 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -160,6 +160,14 @@ config LZO bool "Enable LZO decompression support" help This enables support for LZO compression algorithm.r
+config SPL_GZIP
- bool "Enable gzip decompression support for SPL build"
- select SPL_ZLIB
+config SPL_ZLIB
- bool
endmenu
config ERRNO_STR diff --git a/lib/Makefile b/lib/Makefile index eacc7d6..455cc9d 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -11,7 +11,6 @@ obj-$(CONFIG_EFI) += efi/ obj-$(CONFIG_EFI_LOADER) += efi_loader/ obj-$(CONFIG_LZMA) += lzma/ obj-$(CONFIG_LZO) += lzo/ -obj-$(CONFIG_ZLIB) += zlib/ obj-$(CONFIG_BZIP2) += bzip2/ obj-$(CONFIG_TIZEN) += tizen/ obj-$(CONFIG_FIT) += libfdt/ @@ -26,7 +25,6 @@ obj-y += crc16.o obj-$(CONFIG_ERRNO_STR) += errno_str.o obj-$(CONFIG_FIT) += fdtdec_common.o obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o -obj-$(CONFIG_GZIP) += gunzip.o obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o obj-y += initcall.o @@ -49,6 +47,9 @@ obj-$(CONFIG_RSA) += rsa/ obj-$(CONFIG_SHA1) += sha1.o obj-$(CONFIG_SHA256) += sha256.o
+obj-$(CONFIG_$(SPL_)ZLIB) += zlib/ +obj-$(CONFIG_$(SPL_)GZIP) += gunzip.o
obj-$(CONFIG_SPL_SAVEENV) += qsort.o obj-$(CONFIG_$(SPL_)OF_LIBFDT) += libfdt/ ifneq ($(CONFIG_SPL_BUILD)$(CONFIG_SPL_OF_PLATDATA),yy)
On 09/13/2017 01:38 PM, Heinrich Schuchardt wrote:
On 08/08/2017 01:16 AM, York Sun wrote:
Add Kconfig option SPL_GZIP and SPL_ZLIB to enable gunzip support for SPL boot, eg. falcon boot compressed kernel image.
Signed-off-by: York Sun york.sun@nxp.com Reviewed-by: Tom Rini trini@konsulko.com
Changes in v2: Combine Kconfig change and actual code into one patch
common/spl/spl_fit.c | 28 ++++++++++++++++++++++++++-- lib/Kconfig | 8 ++++++++ lib/Makefile | 5 +++-- 3 files changed, 37 insertions(+), 4 deletions(-)
<snip>
- memcpy((void*)load_addr, src, length);
+#if defined(CONFIG_SPL_OS_BOOT) && defined(CONFIG_SPL_GZIP)
- if (image_comp == IH_COMP_GZIP && type == IH_TYPE_KERNEL) {
if (gunzip((void *)load_addr, CONFIG_SYS_BOOTM_LEN,src, &length)) {In this file length is defined as size_t.
In include/common.h the last parameter of gunzip is defined as unsigned long *.
This leads to a compilation warning and probably incorrect results: CC spl/common/spl/spl_fit.o common/spl/spl_fit.c: In function ‘spl_load_fit_image’: common/spl/spl_fit.c:201:12: warning: passing argument 4 of ‘gunzip’ from incompatible pointer type [-Wincompatible-pointer-types] src, &length)) {
Please, correct the patch to pass a compatible parameter. The patch already made it into the U-Boot git master. So possibly you want to send a follow up patch.
Heinrich,
Thanks for the heads up. I used travis-ci to build all targets but didn't see this warning. What target did you build?
York
On 09/13/2017 11:07 PM, York Sun wrote:
On 09/13/2017 01:38 PM, Heinrich Schuchardt wrote:
On 08/08/2017 01:16 AM, York Sun wrote:
Add Kconfig option SPL_GZIP and SPL_ZLIB to enable gunzip support for SPL boot, eg. falcon boot compressed kernel image.
Signed-off-by: York Sun york.sun@nxp.com Reviewed-by: Tom Rini trini@konsulko.com
Changes in v2: Combine Kconfig change and actual code into one patch
common/spl/spl_fit.c | 28 ++++++++++++++++++++++++++-- lib/Kconfig | 8 ++++++++ lib/Makefile | 5 +++-- 3 files changed, 37 insertions(+), 4 deletions(-)
<snip>
- memcpy((void*)load_addr, src, length);
+#if defined(CONFIG_SPL_OS_BOOT) && defined(CONFIG_SPL_GZIP)
- if (image_comp == IH_COMP_GZIP && type == IH_TYPE_KERNEL) {
if (gunzip((void *)load_addr, CONFIG_SYS_BOOTM_LEN,src, &length)) {In this file length is defined as size_t.
In include/common.h the last parameter of gunzip is defined as unsigned long *.
This leads to a compilation warning and probably incorrect results: CC spl/common/spl/spl_fit.o common/spl/spl_fit.c: In function ‘spl_load_fit_image’: common/spl/spl_fit.c:201:12: warning: passing argument 4 of ‘gunzip’ from incompatible pointer type [-Wincompatible-pointer-types] src, &length)) {
Please, correct the patch to pass a compatible parameter. The patch already made it into the U-Boot git master. So possibly you want to send a follow up patch.
Thanks for the heads up. I used travis-ci to build all targets but didn't see this warning. What target did you build?
make mrproper make qemu-x86_64_defconfig make
Regards
Heinrich
York
On 09/13/2017 09:27 PM, Heinrich Schuchardt wrote:
On 09/13/2017 11:07 PM, York Sun wrote:
On 09/13/2017 01:38 PM, Heinrich Schuchardt wrote:
On 08/08/2017 01:16 AM, York Sun wrote:
Add Kconfig option SPL_GZIP and SPL_ZLIB to enable gunzip support for SPL boot, eg. falcon boot compressed kernel image.
Signed-off-by: York Sun york.sun@nxp.com Reviewed-by: Tom Rini trini@konsulko.com
Changes in v2: Combine Kconfig change and actual code into one patch
common/spl/spl_fit.c | 28 ++++++++++++++++++++++++++-- lib/Kconfig | 8 ++++++++ lib/Makefile | 5 +++-- 3 files changed, 37 insertions(+), 4 deletions(-)
<snip>
- memcpy((void*)load_addr, src, length);
+#if defined(CONFIG_SPL_OS_BOOT) && defined(CONFIG_SPL_GZIP)
- if (image_comp == IH_COMP_GZIP && type == IH_TYPE_KERNEL) {
if (gunzip((void *)load_addr, CONFIG_SYS_BOOTM_LEN,src, &length)) {In this file length is defined as size_t.
In include/common.h the last parameter of gunzip is defined as unsigned long *.
This leads to a compilation warning and probably incorrect results: CC spl/common/spl/spl_fit.o common/spl/spl_fit.c: In function ‘spl_load_fit_image’: common/spl/spl_fit.c:201:12: warning: passing argument 4 of ‘gunzip’ from incompatible pointer type [-Wincompatible-pointer-types] src, &length)) {
Please, correct the patch to pass a compatible parameter. The patch already made it into the U-Boot git master. So possibly you want to send a follow up patch.
Thanks for the heads up. I used travis-ci to build all targets but didn't see this warning. What target did you build?
make mrproper make qemu-x86_64_defconfig make
Heinrich,
Thanks. I check travis-ci log. Surprisingly, it has this warning but reports as "passed". Now I start to question what else I missed.
York
On Thu, Sep 14, 2017 at 03:20:18PM +0000, York Sun wrote:
On 09/13/2017 09:27 PM, Heinrich Schuchardt wrote:
On 09/13/2017 11:07 PM, York Sun wrote:
On 09/13/2017 01:38 PM, Heinrich Schuchardt wrote:
On 08/08/2017 01:16 AM, York Sun wrote:
Add Kconfig option SPL_GZIP and SPL_ZLIB to enable gunzip support for SPL boot, eg. falcon boot compressed kernel image.
Signed-off-by: York Sun york.sun@nxp.com Reviewed-by: Tom Rini trini@konsulko.com
Changes in v2: Combine Kconfig change and actual code into one patch
common/spl/spl_fit.c | 28 ++++++++++++++++++++++++++-- lib/Kconfig | 8 ++++++++ lib/Makefile | 5 +++-- 3 files changed, 37 insertions(+), 4 deletions(-)
<snip>
- memcpy((void*)load_addr, src, length);
+#if defined(CONFIG_SPL_OS_BOOT) && defined(CONFIG_SPL_GZIP)
- if (image_comp == IH_COMP_GZIP && type == IH_TYPE_KERNEL) {
if (gunzip((void *)load_addr, CONFIG_SYS_BOOTM_LEN,src, &length)) {In this file length is defined as size_t.
In include/common.h the last parameter of gunzip is defined as unsigned long *.
This leads to a compilation warning and probably incorrect results: CC spl/common/spl/spl_fit.o common/spl/spl_fit.c: In function ‘spl_load_fit_image’: common/spl/spl_fit.c:201:12: warning: passing argument 4 of ‘gunzip’ from incompatible pointer type [-Wincompatible-pointer-types] src, &length)) {
Please, correct the patch to pass a compatible parameter. The patch already made it into the U-Boot git master. So possibly you want to send a follow up patch.
Thanks for the heads up. I used travis-ci to build all targets but didn't see this warning. What target did you build?
make mrproper make qemu-x86_64_defconfig make
Heinrich,
Thanks. I check travis-ci log. Surprisingly, it has this warning but reports as "passed". Now I start to question what else I missed.
So, yes. One of the things that we don't do today is have -Werror passed. I would love to see the series of patches that fixes the handful of warnings we have today so we could at least make this an option.
On 09/14/2017 05:23 PM, Tom Rini wrote:
On Thu, Sep 14, 2017 at 03:20:18PM +0000, York Sun wrote:
On 09/13/2017 09:27 PM, Heinrich Schuchardt wrote:
On 09/13/2017 11:07 PM, York Sun wrote:
On 09/13/2017 01:38 PM, Heinrich Schuchardt wrote:
On 08/08/2017 01:16 AM, York Sun wrote:
Add Kconfig option SPL_GZIP and SPL_ZLIB to enable gunzip support for SPL boot, eg. falcon boot compressed kernel image.
Signed-off-by: York Sun york.sun@nxp.com Reviewed-by: Tom Rini trini@konsulko.com ---
Changes in v2: Combine Kconfig change and actual code into one patch
common/spl/spl_fit.c | 28 ++++++++++++++++++++++++++-- lib/Kconfig | 8 ++++++++ lib/Makefile | 5 +++-- 3 files changed, 37 insertions(+), 4 deletions(-)
<snip>
- memcpy((void*)load_addr, src, length); +#if
defined(CONFIG_SPL_OS_BOOT) && defined(CONFIG_SPL_GZIP) + if (image_comp == IH_COMP_GZIP && type == IH_TYPE_KERNEL) { + if (gunzip((void *)load_addr, CONFIG_SYS_BOOTM_LEN,
src, &length)) {In this file length is defined as size_t.
In include/common.h the last parameter of gunzip is defined as unsigned long *.
This leads to a compilation warning and probably incorrect results: CC spl/common/spl/spl_fit.o common/spl/spl_fit.c: In function ‘spl_load_fit_image’: common/spl/spl_fit.c:201:12: warning: passing argument 4 of ‘gunzip’ from incompatible pointer type [-Wincompatible-pointer-types] src, &length)) {
Please, correct the patch to pass a compatible parameter. The patch already made it into the U-Boot git master. So possibly you want to send a follow up patch.
Thanks for the heads up. I used travis-ci to build all targets but didn't see this warning. What target did you build?
make mrproper make qemu-x86_64_defconfig make
Heinrich,
Thanks. I check travis-ci log. Surprisingly, it has this warning but reports as "passed". Now I start to question what else I missed.
So, yes. One of the things that we don't do today is have -Werror passed. I would love to see the series of patches that fixes the handful of warnings we have today so we could at least make this an option.
You just have to switch the compiler and these warnings become real errors.
Under Visual C++ long is always 32bit: https://msdn.microsoft.com/en-us/library/s3f49ktz(v=vs.140).aspx
If it is really only a handful of warnings we should switch -Werror on in the 2017.11 release.
Regards
Heinrich
SPL supports U-Boot image in FIT format which has data outside of FIT structure. This adds support for embedded data for normal FIT images.
Signed-off-by: York Sun york.sun@nxp.com
---
Changes in v2: Rebase on top of "SPL: FIT: factor out spl_load_fit_image()" by Andre Przywara
common/spl/spl_fit.c | 52 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 19 deletions(-)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 23f85d2..0de4f40 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -128,13 +128,15 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector, void *fit, ulong base_offset, int node, struct spl_image_info *image_info) { - ulong offset; + int offset; size_t length; + int len; ulong load_addr, load_ptr; void *src; ulong overhead; int nr_sectors; int align_len = ARCH_DMA_MINALIGN - 1; + const void *data; #if defined(CONFIG_SPL_OS_BOOT) && defined(CONFIG_SPL_GZIP) uint8_t image_comp, type;
@@ -149,28 +151,40 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector, debug("%s ", genimg_get_type_name(type)); #endif
- offset = fdt_getprop_u32(fit, node, "data-offset"); - if (offset == FDT_ERROR) - return -ENOENT; - offset += base_offset; - length = fdt_getprop_u32(fit, node, "data-size"); - if (length == FDT_ERROR) - return -ENOENT; - load_addr = fdt_getprop_u32(fit, node, "load"); - if (load_addr == FDT_ERROR && image_info) + if (fit_image_get_load(fit, node, &load_addr)) load_addr = image_info->load_addr; - load_ptr = (load_addr + align_len) & ~align_len;
- overhead = get_aligned_image_overhead(info, offset); - nr_sectors = get_aligned_image_size(info, length, offset); + if (!fit_image_get_data_offset(fit, node, &offset)) { + /* External data */ + offset += base_offset; + if (fit_image_get_data_size(fit, node, &len)) + return -ENOENT;
- if (info->read(info, sector + get_aligned_image_offset(info, offset), - nr_sectors, (void*)load_ptr) != nr_sectors) - return -EIO; - debug("image dst=%lx, offset=%lx, size=%lx\n", load_ptr, offset, - (unsigned long)length); + load_ptr = (load_addr + align_len) & ~align_len; + length = len; + + overhead = get_aligned_image_overhead(info, offset); + nr_sectors = get_aligned_image_size(info, length, offset); + + if (info->read(info, + sector + get_aligned_image_offset(info, offset), + nr_sectors, (void *)load_ptr) != nr_sectors) + return -EIO; + + debug("External data: dst=%lx, offset=%x, size=%lx\n", + load_ptr, offset, (unsigned long)length); + src = (void *)load_ptr + overhead; + } else { + /* Embedded data */ + if (fit_image_get_data(fit, node, &data, &length)) { + puts("Cannot get image data/size\n"); + return -ENOENT; + } + debug("Embedded data: dst=%lx, size=%lx\n", load_addr, + (unsigned long)length); + src = (void *)data; + }
- src = (void *)load_ptr + overhead; #ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS board_fit_image_post_process(&src, &length); #endif
On Mon, Aug 07, 2017 at 04:16:25PM -0700, York Sun wrote:
SPL supports U-Boot image in FIT format which has data outside of FIT structure. This adds support for embedded data for normal FIT images.
Signed-off-by: York Sun york.sun@nxp.com
Reviewed-by: Tom Rini trini@konsulko.com
Hi York,
On 7 August 2017 at 17:16, York Sun york.sun@nxp.com wrote:
SPL supports U-Boot image in FIT format which has data outside of FIT structure. This adds support for embedded data for normal FIT images.
Signed-off-by: York Sun york.sun@nxp.com
Changes in v2: Rebase on top of "SPL: FIT: factor out spl_load_fit_image()" by Andre Przywara
common/spl/spl_fit.c | 52 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 19 deletions(-)
There is some docs here:
doc/uImage.FIT/source_file_format.txt
I feel that what you have here should be documented in some way, associated with SPL. Any ideas?
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 23f85d2..0de4f40 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -128,13 +128,15 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector, void *fit, ulong base_offset, int node, struct spl_image_info *image_info) {
ulong offset;
int offset; size_t length;int len; ulong load_addr, load_ptr; void *src; ulong overhead; int nr_sectors; int align_len = ARCH_DMA_MINALIGN - 1;const void *data;#if defined(CONFIG_SPL_OS_BOOT) && defined(CONFIG_SPL_GZIP) uint8_t image_comp, type;
@@ -149,28 +151,40 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector, debug("%s ", genimg_get_type_name(type)); #endif
offset = fdt_getprop_u32(fit, node, "data-offset");if (offset == FDT_ERROR)return -ENOENT;offset += base_offset;length = fdt_getprop_u32(fit, node, "data-size");if (length == FDT_ERROR)return -ENOENT;load_addr = fdt_getprop_u32(fit, node, "load");if (load_addr == FDT_ERROR && image_info)
if (fit_image_get_load(fit, node, &load_addr)) load_addr = image_info->load_addr;
load_ptr = (load_addr + align_len) & ~align_len;overhead = get_aligned_image_overhead(info, offset);nr_sectors = get_aligned_image_size(info, length, offset);
if (!fit_image_get_data_offset(fit, node, &offset)) {/* External data */offset += base_offset;if (fit_image_get_data_size(fit, node, &len))return -ENOENT;
if (info->read(info, sector + get_aligned_image_offset(info, offset),nr_sectors, (void*)load_ptr) != nr_sectors)return -EIO;debug("image dst=%lx, offset=%lx, size=%lx\n", load_ptr, offset,(unsigned long)length);
load_ptr = (load_addr + align_len) & ~align_len;length = len;overhead = get_aligned_image_overhead(info, offset);nr_sectors = get_aligned_image_size(info, length, offset);if (info->read(info,sector + get_aligned_image_offset(info, offset),nr_sectors, (void *)load_ptr) != nr_sectors)return -EIO;debug("External data: dst=%lx, offset=%x, size=%lx\n",load_ptr, offset, (unsigned long)length);src = (void *)load_ptr + overhead;} else {/* Embedded data */if (fit_image_get_data(fit, node, &data, &length)) {puts("Cannot get image data/size\n");return -ENOENT;}debug("Embedded data: dst=%lx, size=%lx\n", load_addr,(unsigned long)length);src = (void *)data;}
src = (void *)load_ptr + overhead;#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS board_fit_image_post_process(&src, &length);
#endif
2.7.4
Regards, Simon
On 08/13/2017 02:35 PM, Simon Glass wrote:
Hi York,
On 7 August 2017 at 17:16, York Sun york.sun@nxp.com wrote:
SPL supports U-Boot image in FIT format which has data outside of FIT structure. This adds support for embedded data for normal FIT images.
Signed-off-by: York Sun york.sun@nxp.com
Changes in v2: Rebase on top of "SPL: FIT: factor out spl_load_fit_image()" by Andre Przywara
common/spl/spl_fit.c | 52 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 19 deletions(-)
There is some docs here:
doc/uImage.FIT/source_file_format.txt
I feel that what you have here should be documented in some way, associated with SPL. Any ideas?
Yes. That's my plan. I have been waiting for Andre to comment on the priority of "kervel" vs "firmware". I believe it makes sense to favor "kernel" when falcon boot is enabled. The U-Boot image shouldn't contain "kernel" node in falcon boot scenario. Or it can be changed to "standalone" as Andre did for U-Boot image (to favor "loadables").
York
If CONFIG_SPL_OS_BOOT is enabled, boot OS if kernel image is found in FIT structure.
Signed-off-by: York Sun york.sun@nxp.com
--- This presums the kernel image doesn't exist in a FIT image intended for U-Boot. If kernel image normally co-exists with U-Boot and other images and user intends to boot U-Boot, this patch needs to rewrite to favor "loadables" over either "firmware" or "kernel" so user can select which image to boot.
Changes in v2: Split from previous patch, rebased on top of "SPL: FIT: allow loading multiple images" by Andre Przywara.
common/spl/spl_fit.c | 60 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 19 deletions(-)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 0de4f40..0e4e87f 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -218,13 +218,16 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, ulong size; unsigned long count; struct spl_image_info image_info; - int node, images, ret; + bool boot_os = false; + int node = -1; + int images, ret; int base_offset, align_len = ARCH_DMA_MINALIGN - 1; int index = 0;
/* - * Figure out where the external images start. This is the base for the - * data-offset properties in each image. + * For FIT with external data, figure out where the external images + * start. This is the base for the data-offset properties in each + * image. */ size = fdt_totalsize(fit); size = (size + 3) & ~3; @@ -243,6 +246,9 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, * * In fact the FIT has its own load address, but we assume it cannot * be before CONFIG_SYS_TEXT_BASE. + * + * For FIT with data embedded, data is loaded as part of FIT image. + * For FIT with external data, data is not loaded in this step. */ fit = (void *)((CONFIG_SYS_TEXT_BASE - size - info->bl_len - align_len) & ~align_len); @@ -260,8 +266,17 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, return -1; }
+#ifdef CONFIG_SPL_OS_BOOT + /* Find OS image first */ + node = spl_fit_get_image_node(fit, images, FIT_KERNEL_PROP, 0); + if (node < 0) + debug("No kernel image.\n"); + else + boot_os = true; +#endif /* find the U-Boot image */ - node = spl_fit_get_image_node(fit, images, "firmware", 0); + if (node < 0) + node = spl_fit_get_image_node(fit, images, "firmware", 0); if (node < 0) { debug("could not find firmware image, trying loadables...\n"); node = spl_fit_get_image_node(fit, images, "loadables", 0); @@ -283,24 +298,31 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, if (ret) return ret;
+#ifdef CONFIG_SPL_OS_BOOT + if (!fit_image_get_os(fit, node, &spl_image->os)) + debug("Image OS is %s\n", genimg_get_os_name(spl_image->os)); +#else spl_image->os = IH_OS_U_BOOT; +#endif
- /* Figure out which device tree the board wants to use */ - node = spl_fit_get_image_node(fit, images, FIT_FDT_PROP, 0); - if (node < 0) { - debug("%s: cannot find FDT node\n", __func__); - return node; - } + if (!boot_os) { + /* Figure out which device tree the board wants to use */ + node = spl_fit_get_image_node(fit, images, FIT_FDT_PROP, 0); + if (node < 0) { + debug("%s: cannot find FDT node\n", __func__); + return node; + }
- /* - * Read the device tree and place it after the image. - * Align the destination address to ARCH_DMA_MINALIGN. - */ - image_info.load_addr = spl_image->load_addr + spl_image->size; - ret = spl_load_fit_image(info, sector, fit, base_offset, node, - &image_info); - if (ret < 0) - return ret; + /* + * Read the device tree and place it after the image. + * Align the destination address to ARCH_DMA_MINALIGN. + */ + image_info.load_addr = spl_image->load_addr + spl_image->size; + ret = spl_load_fit_image(info, sector, fit, base_offset, node, + &image_info); + if (ret < 0) + return ret; + }
/* Now check if there are more images for us to load */ for (; ; index++) {
On Mon, Aug 07, 2017 at 04:16:26PM -0700, York Sun wrote:
If CONFIG_SPL_OS_BOOT is enabled, boot OS if kernel image is found in FIT structure.
Signed-off-by: York Sun york.sun@nxp.com
Reviewed-by: Tom Rini trini@konsulko.com
On 08/07/2017 04:16 PM, York Sun wrote:
If CONFIG_SPL_OS_BOOT is enabled, boot OS if kernel image is found in FIT structure.
Signed-off-by: York Sun york.sun@nxp.com
This presums the kernel image doesn't exist in a FIT image intended for U-Boot. If kernel image normally co-exists with U-Boot and other images and user intends to boot U-Boot, this patch needs to rewrite to favor "loadables" over either "firmware" or "kernel" so user can select which image to boot.
Andre,
Need your comment on this. Since you created spl_load_simple_fit() and it is used for both loading kernel and U-Boot (and other images), it favors "firmware" over "loadables" when booting U-Boot. I added CONFIG_SPL_OS_BOOT to favor "kernel". However, if your image contains both kernel and U-Boot and you intend to boot U-Boot, this will break when CONFIG_SPL_OS_BOOT is defined. In this case, I can drop detecting "kernel" and use "loadables" to find kernel image.
Please respond.
York
participants (4)
-
Heinrich Schuchardt -
Simon Glass -
Tom Rini -
York Sun