[PATCH 3/6] spl: LOAD_FIT_FULL: Relocate FDT for u-boot payloads

[Simon Glass]

Hi Alexandru,

On Fri, 12 Mar 2021 at 10:32, Alexandru Gagniuc <mr.nuke.me at gmail.com> wrote:
>
> 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).

We could do that, but the current behaviour is OK. It is required that
you can joined U-Boot and its FDT and get a valid image.

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

I think it is fine to require it be after U-Boot, in fact that is
defined by U-Boot. So I don't think you need to think of it as a hack.

>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me at 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 8f6c8dba6f..e63f05bb33 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;

I don't think this is needed and it just confuses things. If U-Boot is
not an aligned sign, it can't boot because the DT ends up in the wrong
place. The build system makes sure this doesn't happen.

> +                       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)
> --
> 2.26.2
>

Regards,
Simon