[U-Boot] [U-Boot PATCH v2 01/10] sf: allocate cache aligned buffers to copy from flash

[Simon Glass]

Hi,

On 13 August 2015 at 14:54, Tom Rini <trini at konsulko.com> wrote:
> On Thu, Aug 13, 2015 at 12:38:52PM -0600, Simon Glass wrote:
>> Hi,
>>
>> On 13 August 2015 at 12:23, Tom Rini <trini at konsulko.com> wrote:
>> > On Thu, Aug 13, 2015 at 11:38:00PM +0530, Jagan Teki wrote:
>> >> On 13 August 2015 at 23:24, Tom Rini <trini at konsulko.com> wrote:
>> >> > On Thu, Aug 13, 2015 at 11:09:03PM +0530, Jagan Teki wrote:
>> >> >> Hi Simon,
>> >> >>
>> >> >> On 30 July 2015 at 11:04, Vignesh R <vigneshr at ti.com> wrote:
>> >> >> > From: Ravi Babu <ravibabu at ti.com>
>> >> >> >
>> >> >> > Use memalign() with ARCH_DMA_MINALIGN to allocate read buffers.
>> >> >> > This is required because, flash drivers may use DMA for read operations
>> >> >> > and may have to invalidate the buffer before read.
>> >> >> >
>> >> >> > Signed-off-by: Ravi Babu <ravibabu at ti.com>
>> >> >> > Signed-off-by: Vignesh R <vigneshr at ti.com>
>> >> >> > ---
>> >> >> >  common/cmd_sf.c | 6 +++---
>> >> >> >  1 file changed, 3 insertions(+), 3 deletions(-)
>> >> >> >
>> >> >> > diff --git a/common/cmd_sf.c b/common/cmd_sf.c
>> >> >> > index 3746e0d9644f..ac7f5dfb8181 100644
>> >> >> > --- a/common/cmd_sf.c
>> >> >> > +++ b/common/cmd_sf.c
>> >> >> > @@ -223,7 +223,7 @@ static int spi_flash_update(struct spi_flash *flash, u32 offset,
>> >> >> >
>> >> >> >         if (end - buf >= 200)
>> >> >> >                 scale = (end - buf) / 100;
>> >> >> > -       cmp_buf = malloc(flash->sector_size);
>> >> >> > +       cmp_buf = memalign(ARCH_DMA_MINALIGN, flash->sector_size);
>> >> >>
>> >> >> I always have a confusion like align macro is never used on code
>> >> >> memalign_simple()
>> >> >> Was it a typo missing or some thing?
>> >> >
>> >> > Sorry?  We always call memalign() rather than memalign_simple().
>> >>
>> >> memalign() is calling memalign_simple() except __libc_memalign
>> >>
>> >> Am I missing something here, because ARCH_DMA_MINALIGN is never used
>> >> on memalign_simple()
>> >
>> > Yes, unless I'm missing something now, memalign_simple() is only used on
>> > CONFIG_SYS_MALLOC_SIMPLE which has some very specific use cases.  We
>> > otherwise use memalign from common/dlmalloc.c..
>>
>> It looks like memalign_simple()  should use
>>
>>         addr = ALIGN(gd->malloc_base + gd->malloc_ptr, align);
>>
>> instead of:
>>
>>         addr = ALIGN(gd->malloc_base + gd->malloc_ptr, bytes);
>
> Now we're getting somewhere, good spotting Jagan!
>
> --
> Tom

Is that to catch the case where bytes is not a multiple of align? Why
would that be and is it correct to 'waste' those bytes?

I'll send a patch and perhaps we can continue the discussion there.

Regards,
Simon