[U-Boot] [PATCH 04/17] SPL: FIT: factor out spl_load_fit_image()

Simon Glass sjg at chromium.org
Wed Mar 8 21:00:56 UTC 2017


Hi Andre,

On 28 February 2017 at 19:25, Andre Przywara <andre.przywara at arm.com> wrote:
> At the moment we load two images from a FIT image: the actual U-Boot
> image and the DTB. Both times we have very similar code to deal with
> alignment requirement the media we load from imposes upon us.
> Factor out this code into a new function, which we just call twice.
>
> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> ---
>  common/spl/spl_fit.c | 129 +++++++++++++++++++++++----------------------------
>  1 file changed, 57 insertions(+), 72 deletions(-)
>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 572a5db..ad5ba15 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -18,7 +18,7 @@ static ulong fdt_getprop_u32(const void *fdt, int node, const char *prop)
>
>         cell = fdt_getprop(fdt, node, prop, &len);
>         if (len != sizeof(*cell))
> -               return -1U;
> +               return -1UL;

Looks like an unrelated change but I see you need it below, so OK.

>         return fdt32_to_cpu(*cell);
>  }
>
> @@ -139,19 +139,63 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
>         return (data_size + info->bl_len - 1) / info->bl_len;
>  }
>
> +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)

Please add a full comment to this one.

> +{
> +       ulong offset;
> +       size_t length;
> +       ulong load_addr, load_ptr, entry;
> +       void *src;
> +       ulong overhead;
> +       int nr_sectors;
> +       int align_len = ARCH_DMA_MINALIGN - 1;
> +
> +       offset = fdt_getprop_u32(fit, node, "data-offset") + base_offset;

I think you need to check that offset is not -1 first.

> +       length = fdt_getprop_u32(fit, node, "data-size");
> +       load_addr = fdt_getprop_u32(fit, node, "load");
> +       if (load_addr == -1UL && image_info)
> +               load_addr = image_info->load_addr;
> +       load_ptr = (load_addr + align_len) & ~align_len;
> +       entry = fdt_getprop_u32(fit, node, "entry");
> +
> +       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("image: dst=%lx, offset=%lx, size=%lx\n", load_ptr, offset,
> +             (unsigned long)length);
> +
> +       src = (void *)load_ptr + overhead;
> +#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
> +       board_fit_image_post_process(&src, &length);
> +#endif
> +
> +       memcpy((void*)load_addr, src, length);
> +
> +       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;
> +       }
> +
> +       return 0;
> +}
> +
>  int spl_load_simple_fit(struct spl_image_info *spl_image,
>                         struct spl_load_info *info, ulong sector, void *fit)
>  {
>         int sectors;
> -       ulong size, load;
> +       ulong size;
>         unsigned long count;
> +       struct spl_image_info image_info;
>         int node, images;
> -       void *load_ptr;
> -       int fdt_offset, fdt_len;
> -       int data_offset, data_size;
>         int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
> -       int src_sector;
> -       void *dst, *src;
>
>         /*
>          * Figure out where the external images start. This is the base for the
> @@ -203,82 +247,23 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
>                 return -1;
>         }
>
> -       /* Get its information and set up the spl_image structure */
> -       data_offset = fdt_getprop_u32(fit, node, "data-offset");
> -       data_size = fdt_getprop_u32(fit, node, "data-size");
> -       load = fdt_getprop_u32(fit, node, "load");
> -       debug("data_offset=%x, data_size=%x\n", data_offset, data_size);
> -       spl_image->load_addr = load;
> -       spl_image->entry_point = load;
> +       /* Load the image and set up the spl_image structure */
> +       spl_load_fit_image(info, sector, fit, base_offset, node, spl_image);

Check error return?

>         spl_image->os = IH_OS_U_BOOT;
>
> -       /*
> -        * Work out where to place the image. We read it so that the first
> -        * byte will be at 'load'. This may mean we need to load it starting
> -        * before then, since we can only read whole blocks.
> -        */
> -       data_offset += base_offset;
> -       sectors = get_aligned_image_size(info, data_size, data_offset);
> -       load_ptr = (void *)load;
> -       debug("U-Boot size %x, data %p\n", data_size, load_ptr);
> -       dst = load_ptr;
> -
> -       /* Read the image */
> -       src_sector = sector + get_aligned_image_offset(info, data_offset);
> -       debug("Aligned image read: dst=%p, src_sector=%x, sectors=%x\n",
> -             dst, src_sector, sectors);
> -       count = info->read(info, src_sector, sectors, dst);
> -       if (count != sectors)
> -               return -EIO;
> -       debug("image: dst=%p, data_offset=%x, size=%x\n", dst, data_offset,
> -             data_size);
> -       src = dst + get_aligned_image_overhead(info, data_offset);
> -
> -#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
> -       board_fit_image_post_process((void **)&src, (size_t *)&data_size);
> -#endif
> -
> -       memcpy(dst, src, data_size);
> -
>         /* 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;
>         }
> -       fdt_offset = fdt_getprop_u32(fit, node, "data-offset");
> -       fdt_len = fdt_getprop_u32(fit, node, "data-size");
> -
> -       /*
> -        * Read the device tree and place it after the image. There may be
> -        * some extra data before it since we can only read entire blocks.
> -        * And also align the destination address to ARCH_DMA_MINALIGN.
> -        */
> -       dst = (void *)((load + data_size + align_len) & ~align_len);
> -       fdt_offset += base_offset;
> -       sectors = get_aligned_image_size(info, fdt_len, fdt_offset);
> -       src_sector = sector + get_aligned_image_offset(info, fdt_offset);
> -       count = info->read(info, src_sector, sectors, dst);
> -       debug("Aligned fdt read: dst %p, src_sector = %x, sectors %x\n",
> -             dst, src_sector, sectors);
> -       if (count != sectors)
> -               return -EIO;
>
>         /*
> -        * Copy the device tree so that it starts immediately after the image.
> -        * After this we will have the U-Boot image and its device tree ready
> -        * for us to start.
> +        * Read the device tree and place it after the image.
> +        * Align the destination address to ARCH_DMA_MINALIGN.
>          */
> -       debug("fdt: dst=%p, data_offset=%x, size=%x\n", dst, fdt_offset,
> -             fdt_len);
> -       src = dst + get_aligned_image_overhead(info, fdt_offset);
> -       dst = load_ptr + data_size;
> -
> -#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
> -       board_fit_image_post_process((void **)&src, (size_t *)&fdt_len);
> -#endif
> -
> -       memcpy(dst, src, fdt_len);
> +       image_info.load_addr = spl_image->load_addr + spl_image->size;
> +       spl_load_fit_image(info, sector, fit, base_offset, node, &image_info);

Check error return?

>
>         return 0;
>  }
> --
> 2.8.2
>

Regards,
Simon


More information about the U-Boot mailing list