The correct FDT to use is described by the "fdt" property of the configuration node. When the fit_unamep argument to fit_image_load() is "fdt", we get the "/images/fdt" node. This is incorrect, as it ignores the "fdt" property of the config node, and in most cases, the "/images/fdt" node doesn't exist.

Use NULL for the 'fit_unamep' argument. With NULL, fit_image_load() uses the IH_TYPE_FLATDT value to read the config property "fdt", which points to the correct FDT node(s).

fit_image_load() should probably be split into a function that reads an image by name, and one that reads an image by config reference. I don't make those decisions, I just point out the craziness.

Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com Reviewed-by: Simon Glass sjg@chromium.org --- common/spl/spl.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c index e3d84082f4..986cfbc6fd 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -201,7 +201,6 @@ static int spl_load_fit_image(struct spl_image_info *spl_image, { bootm_headers_t images; const char *fit_uname_config = NULL; - const char *fit_uname_fdt = FIT_FDT_PROP; const char *uname; ulong fw_data = 0, dt_data = 0, img_data = 0; ulong fw_len = 0, dt_len = 0, img_len = 0; @@ -230,8 +229,7 @@ static int spl_load_fit_image(struct spl_image_info *spl_image, #ifdef CONFIG_SPL_FIT_SIGNATURE images.verify = 1; #endif - ret = fit_image_load(&images, (ulong)header, - &fit_uname_fdt, &fit_uname_config, + ret = fit_image_load(&images, (ulong)header, NULL, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_FLATDT, -1, FIT_LOAD_OPTIONAL, &dt_data, &dt_len); if (ret >= 0)

The information on the OS should be contained in the FIT, as the self-explanatory "os" property of a node under /images. Hard-coding this to U_BOOT might send us down the wrong path later in the boot process.

Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- common/spl/spl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c index 986cfbc6fd..f3b4c3a036 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -220,8 +220,9 @@ static int spl_load_fit_image(struct spl_image_info *spl_image, spl_image->size = fw_len; spl_image->entry_point = fw_data; spl_image->load_addr = fw_data; - spl_image->os = IH_OS_U_BOOT; - spl_image->name = "U-Boot"; + if (fit_image_get_os(header, ret, &spl_image->os)) + spl_image->os = IH_OS_INVALID; + spl_image->name = genimg_get_os_name(spl_image->os);

debug(SPL_TPL_PROMPT "payload image: %32s load addr: 0x%lx size: %d\n", spl_image->name, spl_image->load_addr, spl_image->size);

U-Boot expects the FDT to be located right after the _end linker symbol (see fdtdec.c: board_fdt_blob_setup())

The "basic" LOAD_FIT path is aware of this limitation, and relocates the FDT at the expected location. Guessing the expected location probably only works reliably on 32-bit arm, and it feels like a hack. One proposal would be to pass the FDT address to u-boot (e.g. using 'r2' on arm platforms).

The variable is named "fdt_hack" to remind future contributors that, "hey! we should fix the underlying problem". However, that is beyond the scope of this patch.

Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- common/spl/spl.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c index f3b4c3a036..5a059c6dc5 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -201,6 +201,7 @@ static int spl_load_fit_image(struct spl_image_info *spl_image, { bootm_headers_t images; const char *fit_uname_config = NULL; + uintptr_t fdt_hack; const char *uname; ulong fw_data = 0, dt_data = 0, img_data = 0; ulong fw_len = 0, dt_len = 0, img_len = 0; @@ -233,9 +234,18 @@ static int spl_load_fit_image(struct spl_image_info *spl_image, ret = fit_image_load(&images, (ulong)header, NULL, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_FLATDT, -1, FIT_LOAD_OPTIONAL, &dt_data, &dt_len); - if (ret >= 0) + if (ret >= 0) { spl_image->fdt_addr = (void *)dt_data;

+ if (spl_image->os == IH_OS_U_BOOT) { + /* HACK: U-boot expects FDT at a specific address */ + fdt_hack = spl_image->load_addr + spl_image->size; + fdt_hack = (fdt_hack + 3) & ~3; + debug("Relocating FDT to %p\n", spl_image->fdt_addr); + memcpy((void *)fdt_hack, spl_image->fdt_addr, dt_len); + } + } + conf_noffset = fit_conf_get_node((const void *)header, fit_uname_config); if (conf_noffset <= 0)

The 'firmware' property of a config node takes precedence over the 'kernel' property. 'standalone' is deprecated. However, give users a couple of releases where 'standalone' still works, but warns loudly.

Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- common/spl/spl.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c index 5a059c6dc5..4e028abf77 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -214,7 +214,24 @@ static int spl_load_fit_image(struct spl_image_info *spl_image, ret = fit_image_load(&images, (ulong)header, NULL, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_STANDALONE, -1, - FIT_LOAD_REQUIRED, &fw_data, &fw_len); + FIT_LOAD_OPTIONAL, &fw_data, &fw_len); + if (ret >= 0) { + printf("DEPRECATED: 'standalone = ' property."); + printf("Please use either 'firmware =' or 'kernel ='\n"); + } else { + ret = fit_image_load(&images, (ulong)header, NULL, + &fit_uname_config, IH_ARCH_DEFAULT, + IH_TYPE_FIRMWARE, -1, FIT_LOAD_OPTIONAL, + &fw_data, &fw_len); + } + + if (ret < 0) { + ret = fit_image_load(&images, (ulong)header, NULL, + &fit_uname_config, IH_ARCH_DEFAULT, + IH_TYPE_KERNEL, -1, FIT_LOAD_OPTIONAL, + &fw_data, &fw_len); + } + if (ret < 0) return ret;

Consider the following FIT:

images { whipple {}; }; configurations { conf-1 { firmware = "whipple"; }; };

Getting the 'firmware' image with fit_image_load() is not possible, as it doesn't understand 'firmware =' properties. Although one could pass IH_TYPE_FIRMWARE for 'image_type', this needs to be converted to a "firmware" string for FDT lookup -- exactly what this change does.

Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com Reviewed-by: Simon Glass sjg@chromium.org --- common/image-fit.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/common/image-fit.c b/common/image-fit.c index 94501b1071..970e3f89da 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -1955,6 +1955,8 @@ static const char *fit_get_image_type_property(int type) return FIT_FDT_PROP; case IH_TYPE_KERNEL: return FIT_KERNEL_PROP; + case IH_TYPE_FIRMWARE: + return FIT_FIRMWARE_PROP; case IH_TYPE_RAMDISK: return FIT_RAMDISK_PROP; case IH_TYPE_X86_SETUP:

OP-TEE images are normally packaged with type = "tee; os = "tee";

However, fit_image_load() thinks that is somehow invalid. However if they were declared as type = "kernel", os = "linux", fit_image_load() would happily accept them and allow the boot to continue. There is no technical limitation to excluding "tee".

Allowing "tee" images is useful in a boot flow where OP-TEE is executed before linux.

In fact, I think it's unintuitive for a "load"ing function to also do parsing and contain a bunch ad-hoc heuristics that only its caller might know. But I don't make the rules, I just write fixes. In more polite terms: refactoring the fit_image API is beyond the scope of this change.

Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com Reviewed-by: Simon Glass sjg@chromium.org --- common/image-fit.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/common/image-fit.c b/common/image-fit.c index 970e3f89da..ee51f5e76f 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -2089,6 +2089,7 @@ int fit_image_load(bootm_headers_t *images, ulong addr, bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL); type_ok = fit_image_check_type(fit, noffset, image_type) || fit_image_check_type(fit, noffset, IH_TYPE_FIRMWARE) || + fit_image_check_type(fit, noffset, IH_TYPE_TEE) || (image_type == IH_TYPE_KERNEL && fit_image_check_type(fit, noffset, IH_TYPE_KERNEL_NOLOAD));

@@ -2096,6 +2097,7 @@ int fit_image_load(bootm_headers_t *images, ulong addr, image_type == IH_TYPE_FPGA || fit_image_check_os(fit, noffset, IH_OS_LINUX) || fit_image_check_os(fit, noffset, IH_OS_U_BOOT) || + fit_image_check_os(fit, noffset, IH_OS_TEE) || fit_image_check_os(fit, noffset, IH_OS_OPENRTOS) || fit_image_check_os(fit, noffset, IH_OS_EFI) || fit_image_check_os(fit, noffset, IH_OS_VXWORKS);