On Tuesday 03 May 2016 06:29 PM, Michal Simek wrote:
> On 3.5.2016 11:05, Lokesh Vutla wrote:
>> This provides a way to load a FIT containing U-Boot and a selection of device
>> tree files from a File system. Making sure that all the reads and writes
>> are aligned to their respective needs.
>>
>> Signed-off-by: Lokesh Vutla <[email protected]>
>> ---
>> - Assuming info->priv is used for passing filename for now and using this 
>> for 
>>   detecting if it a fs read. If this is not preferred I can create a new 
>> field
>>   for filename.
>>
>>  common/spl/spl_fit.c | 76 
>> +++++++++++++++++++++++++++++++++++++++++-----------
>>  include/spl.h        | 10 +++++++
>>  2 files changed, 71 insertions(+), 15 deletions(-)
>>
>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> index b1cfa97..d696354 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -82,6 +82,42 @@ static int spl_fit_select_fdt(const void *fdt, int 
>> images, int *fdt_offsetp)
>>      return -ENOENT;
>>  }
>>  
>> +static int get_aligned_image_offset(struct spl_load_info *info, int offset)
>> +{
>> +    /*
>> +     * If it is a FS read, get the first address before offset which is
>> +     * aligned to ARCH_DMA_MINALIGN. If it is raw read return the
>> +     * block number to which offset belongs.
>> +     */
>> +    if (info->priv)
>> +            return offset & ~(ARCH_DMA_MINALIGN - 1);
>> +    else
> 
> remove this else.

Sure.

> 
>> +            return offset / info->bl_len;
>> +}
>> +
>> +static int get_aligned_image_overhead(struct spl_load_info *info, int 
>> offset)
>> +{
>> +    /*
>> +     * If it is a FS read, get the difference between the offset and
>> +     * the first address before offset which is aligned to
>> +     * ARCH_DMA_MINALIGN. If it is raw read return the offset within the
>> +     * block.
>> +     */
>> +    if (info->priv)
>> +            return offset & (ARCH_DMA_MINALIGN - 1);
>> +    else
> 
> remove this else.

Sure.

> 
>> +            return offset % info->bl_len;
>> +}
>> +
>> +static int get_aligned_image_size(struct spl_load_info *info, int data_size,
>> +                              int offset)
>> +{
>> +    if (info->priv)
>> +            return data_size + get_aligned_image_overhead(info, offset);
>> +    else
> 
> remove this else.

Sure.

> 
> 
>> +            return (data_size + info->bl_len - 1) / info->bl_len;
>> +}
>> +
>>  int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>>  {
>>      int sectors;
>> @@ -91,7 +127,7 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong 
>> sector, void *fit)
>>      void *load_ptr;
>>      int fdt_offset, fdt_len;
>>      int data_offset, data_size;
>> -    int base_offset;
>> +    int base_offset, align_len;
>>      int src_sector;
>>      void *dst;
>>  
>> @@ -117,8 +153,10 @@ int spl_load_simple_fit(struct spl_load_info *info, 
>> ulong sector, void *fit)
>>       * In fact the FIT has its own load address, but we assume it cannot
>>       * be before CONFIG_SYS_TEXT_BASE.
>>       */
>> -    fit = (void *)(CONFIG_SYS_TEXT_BASE - size - info->bl_len);
>> -    sectors = (size + info->bl_len - 1) / info->bl_len;
>> +    align_len = ARCH_DMA_MINALIGN - 1;
>> +    fit = (void *)((CONFIG_SYS_TEXT_BASE - size - info->bl_len -
>> +                    align_len) & ~align_len);
> 
> Don't you want to use ALIGN macro here?

ALIGN() will return the next aligned address which is equal to:

fit = (void *)((CONFIG_SYS_TEXT_BASE - size - info->bl_len + align_len)
& ~align_len);

But here we need the previous aligned address so that we do not loose
any data.

Thanks and regards,
Lokesh

> 
>> +    sectors = get_aligned_image_size(info, size, 0);
>>      count = info->read(info, sector, sectors, fit);
>>      debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu\n",
>>            sector, sectors, fit, count);
>> @@ -151,19 +189,23 @@ int spl_load_simple_fit(struct spl_load_info *info, 
>> ulong sector, void *fit)
>>       * byte will be at 'load'. This may mean we need to load it starting
>>       * before then, since we can only read whole blocks.
>>       */
>> -    sectors = (data_size + info->bl_len - 1) / info->bl_len;
>> +    sectors = get_aligned_image_size(info, data_size, data_offset);
>>      data_offset += base_offset;
>>      load_ptr = (void *)load;
>>      debug("U-Boot size %x, data %p\n", data_size, load_ptr);
>> -    dst = load_ptr - (data_offset % info->bl_len);
>> +    dst = load_ptr;
>>  
>>      /* Read the image */
>> -    src_sector = sector + data_offset / info->bl_len;
>> -    debug("image: data_offset=%x, dst=%p, src_sector=%x, sectors=%x\n",
>> -          data_offset, dst, src_sector, sectors);
>> +    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);
>> +    memcpy(dst, dst + get_aligned_image_overhead(info, data_offset),
>> +           data_size);
>>  
>>      /* Figure out which device tree the board wants to use */
>>      fdt_len = spl_fit_select_fdt(fit, images, &fdt_offset);
>> @@ -173,14 +215,15 @@ int spl_load_simple_fit(struct spl_load_info *info, 
>> ulong sector, void *fit)
>>      /*
>>       * 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 = load_ptr + data_size;
>> +    dst = (void *)((load + data_size + align_len) & ~align_len);
> 
> Same here.
> 
> Thanks,
> Michal
> 
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to