On 23/01/17 08:53, Lokesh Vutla wrote:

Hi Lokesh,

thanks a lot for having a thorough look at it!

>> On Friday 20 January 2017 07:23 AM, Andre Przywara 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.przyw...@arm.com>
>> ---
>>  common/spl/spl_fit.c | 122 
>> +++++++++++++++++++++------------------------------
>>  1 file changed, 51 insertions(+), 71 deletions(-)
>>
>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> index 381ed1f..d4149c5 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -138,19 +138,58 @@ 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)
>> +{
>> +    ulong offset;
>> +    size_t length;
>> +    ulong load, entry;
>> +    void *src;
>> +    ulong overhead;
>> +    int nr_sectors;
>> +
>> +    offset = fdt_getprop_u32(fit, node, "data-offset") + base_offset;
>> +    length = fdt_getprop_u32(fit, node, "data-size");
>> +    load = fdt_getprop_u32(fit, node, "load");
>> +    if (load == -1U && image_info)
>> +            load = image_info->load_addr;
> 
> What if load_addr is not aligned with ARCH_DMA_MINALIGN like in case of
> DT loading (u-boot's load_addr + size cannot be always aligned with
> DMA). I keep getting this error when loading DT: "FAT: Misaligned buffer
> address (808675a0)."

Ah, good point, though I didn't encounter this error.

> Something similar is required to fix it:
> http://pastebin.ubuntu.com/23850970/

Yeah, looks like it. I try to bake this in an upcoming release.

>> +    entry = fdt_getprop_u32(fit, node, "entry");
>> +
>> +    overhead = get_aligned_image_overhead(info, offset);
>> +    nr_sectors = get_aligned_image_size(info, overhead + length, offset);
>                                                                               
>      ^^^^
> Need not add overhead here as get_aligned_image_size() takes care of
> adding overhead.

Right, I somehow got lost in translation here, probably during some
rebasing/refactoring.

>> +
>> +    if (info->read(info, sector + get_aligned_image_offset(info, offset),
>> +                   nr_sectors, (void*)load) != nr_sectors)
>> +            return -EIO;
>> +    debug("image: dst=%lx, offset=%lx, size=%lx\n", load, offset,
>> +          (unsigned long)length);
>> +
>> +    src = (void *)load + overhead;
>> +#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
>> +    board_fit_image_post_process(&src, &length);
>> +#endif
>> +
>> +    memcpy((void*)load, src, length);
>> +
>> +    if (image_info) {
>> +            image_info->load_addr = load;
>> +            image_info->size = length;
>> +            image_info->entry_point = entry;
> 
> If entry_point is not provided can we default to load_addr?
> Existing U-Boot fit image generation does not provide entry option. So
> this patch alone breaks boot.(I understand that it is fixed in next
> patch but this is breaking git bisectability).

Good point, this is indeed a good idea.

Cheers,
Andre.

> Thanks and regards,
> Lokesh
> 
> 
>> +    }
>> +
>> +    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
>> @@ -202,82 +241,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);
>>      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.
>> +     * Read the device tree and place it after the image.
>> +     * 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.
>> -     */
>> -    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);
>>  
>>      return 0;
>>  }
>>

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to