[U-Boot] [PATCH 05/17] SPL: FIT: allow loading multiple images

[Simon Glass]

Hi Andre,

On 28 February 2017 at 19:25, Andre Przywara <andre.przywara at arm.com> wrote:
> So far we were not using the FIT image format to its full potential:
> The SPL FIT loader was just loading the first image from the /images
> node plus one of the listed DTBs.
> Now with the refactored loader code it's easy to load an arbitrary
> number of images in addition to the two mentioned above.
> As described in the FIT image source file format description, iterate
> over all images listed at the "loadables" property in the configuration
> node and load every image at its desired location.
> This allows to load any kind of images:
> - firmware images to execute before U-Boot proper (for instance
>   ARM Trusted Firmware (ATF))
> - firmware images for management processors (SCP, arisc, ...)
> - firmware images for devices like WiFi controllers
> - bit files for FPGAs
> - additional configuration data
> - kernels and/or ramdisks
> The actual usage of this feature would be platform and/or board specific.
>
> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> ---
>  common/spl/spl_fit.c | 32 ++++++++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index ad5ba15..5583e09 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -178,10 +178,7 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
>         if (image_info) {
>                 image_info->load_addr = load_addr;
>                 image_info->size = length;
> -               if (entry == -1UL)
> -                       image_info->entry_point = load_addr;
> -               else
> -                       image_info->entry_point = entry;
> +               image_info->entry_point = entry;

Need to update function comment to indicate that it can put -1 in here.

>         }
>
>         return 0;
> @@ -196,6 +193,7 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
>         struct spl_image_info image_info;
>         int node, images;
>         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
> @@ -240,6 +238,11 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
>         if (node < 0) {
>                 debug("could not find firmware image, trying loadables...\n");
>                 node = spl_fit_get_image_node(fit, images, "loadables", 0);
> +               /*
> +                * If we pick the U-Boot image from "loadables", start at
> +                * the second image when later loading additional images.
> +                */
> +               index = 1;
>         }
>         if (node < 0) {
>                 debug("%s: Cannot find u-boot image node: %d\n",
> @@ -265,5 +268,26 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
>         image_info.load_addr = spl_image->load_addr + spl_image->size;
>         spl_load_fit_image(info, sector, fit, base_offset, node, &image_info);
>
> +       /* Now check if there are more images for us to load */
> +       for (; ; index++) {
> +               node = spl_fit_get_image_node(fit, images, "loadables", index);
> +               if (node < 0)
> +                       break;
> +
> +               spl_load_fit_image(info, sector, fit, base_offset, node,
> +                                  &image_info);

Error check?

> +
> +               /*
> +                * If the "firmware" image did not provide an entry point,
> +                * use the first valid entry point from the loadables.
> +                */
> +               if (spl_image->entry_point == -1UL &&
> +                   image_info.entry_point != -1UL)
> +                       spl_image->entry_point = image_info.entry_point;
> +       }
> +
> +       if (spl_image->entry_point == -1UL || spl_image->entry_point == 0)

Why does 0 mean there is no entry point? I suppose that is safe, but
would anyone use this?

> +               spl_image->entry_point = spl_image->load_addr;
> +
>         return 0;
>  }
> --
> 2.8.2
>

Regards,
Simon