| Message ID | 1438234483-3738-2-git-send-email-vigneshr@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
Hi Simon, On 30 July 2015 at 11:04, Vignesh R <vigneshr@ti.com> wrote: > From: Ravi Babu <ravibabu@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@ti.com> > Signed-off-by: Vignesh R <vigneshr@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? > if (cmp_buf) { > ulong last_update = get_timer(0); > > @@ -484,12 +484,12 @@ static int do_spi_flash_test(int argc, char * const argv[]) > if (*argv[2] == 0 || *endp != 0) > return -1; > > - vbuf = malloc(len); > + vbuf = memalign(ARCH_DMA_MINALIGN, len); > if (!vbuf) { > printf("Cannot allocate memory (%lu bytes)\n", len); > return 1; > } > - buf = malloc(len); > + buf = memalign(ARCH_DMA_MINALIGN, len); > if (!buf) { > free(vbuf); > printf("Cannot allocate memory (%lu bytes)\n", len); > -- > 2.5.0 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot thanks!
HI Jagan, On 13 August 2015 at 11:39, Jagan Teki <jteki@openedev.com> wrote: > > Hi Simon, > > On 30 July 2015 at 11:04, Vignesh R <vigneshr@ti.com> wrote: > > From: Ravi Babu <ravibabu@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@ti.com> > > Signed-off-by: Vignesh R <vigneshr@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? I don't understand what you are saying, sorry. > > > > if (cmp_buf) { > > ulong last_update = get_timer(0); > > > > @@ -484,12 +484,12 @@ static int do_spi_flash_test(int argc, char * const argv[]) > > if (*argv[2] == 0 || *endp != 0) > > return -1; > > > > - vbuf = malloc(len); > > + vbuf = memalign(ARCH_DMA_MINALIGN, len); > > if (!vbuf) { > > printf("Cannot allocate memory (%lu bytes)\n", len); > > return 1; > > } > > - buf = malloc(len); > > + buf = memalign(ARCH_DMA_MINALIGN, len); > > if (!buf) { > > free(vbuf); > > printf("Cannot allocate memory (%lu bytes)\n", len); > > -- > > 2.5.0 > > > > _______________________________________________ > > U-Boot mailing list > > U-Boot@lists.denx.de > > http://lists.denx.de/mailman/listinfo/u-boot > > > thanks! > -- > Jagan | openedev. Regards, Simon
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@ti.com> wrote: > > From: Ravi Babu <ravibabu@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@ti.com> > > Signed-off-by: Vignesh R <vigneshr@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().
On 13 August 2015 at 23:23, Simon Glass <sjg@chromium.org> wrote: > HI Jagan, > > On 13 August 2015 at 11:39, Jagan Teki <jteki@openedev.com> wrote: >> >> Hi Simon, >> >> On 30 July 2015 at 11:04, Vignesh R <vigneshr@ti.com> wrote: >> > From: Ravi Babu <ravibabu@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@ti.com> >> > Signed-off-by: Vignesh R <vigneshr@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); memalign is calling memalign_simple(size_t align, size_t bytes) in common/malloc_simple.c except __libc_memalign So, the align argument is never used on code, why? >> >> I always have a confusion like align macro is never used on code >> memalign_simple() >> Was it a typo missing or some thing? > > I don't understand what you are saying, sorry. > >> >> >> > if (cmp_buf) { >> > ulong last_update = get_timer(0); >> > >> > @@ -484,12 +484,12 @@ static int do_spi_flash_test(int argc, char * const argv[]) >> > if (*argv[2] == 0 || *endp != 0) >> > return -1; >> > >> > - vbuf = malloc(len); >> > + vbuf = memalign(ARCH_DMA_MINALIGN, len); >> > if (!vbuf) { >> > printf("Cannot allocate memory (%lu bytes)\n", len); >> > return 1; >> > } >> > - buf = malloc(len); >> > + buf = memalign(ARCH_DMA_MINALIGN, len); >> > if (!buf) { >> > free(vbuf); >> > printf("Cannot allocate memory (%lu bytes)\n", len); >> > -- >> > 2.5.0 >> > >> > _______________________________________________ >> > U-Boot mailing list >> > U-Boot@lists.denx.de >> > http://lists.denx.de/mailman/listinfo/u-boot thanks!
On 13 August 2015 at 23:24, Tom Rini <trini@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@ti.com> wrote: >> > From: Ravi Babu <ravibabu@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@ti.com> >> > Signed-off-by: Vignesh R <vigneshr@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() thanks! -- Jagan.
On Thu, Aug 13, 2015 at 11:38:00PM +0530, Jagan Teki wrote: > On 13 August 2015 at 23:24, Tom Rini <trini@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@ti.com> wrote: > >> > From: Ravi Babu <ravibabu@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@ti.com> > >> > Signed-off-by: Vignesh R <vigneshr@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..
Hi, On 13 August 2015 at 12:23, Tom Rini <trini@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@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@ti.com> wrote: >> >> > From: Ravi Babu <ravibabu@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@ti.com> >> >> > Signed-off-by: Vignesh R <vigneshr@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); Regards, Simon
On 14 August 2015 at 00:08, Simon Glass <sjg@chromium.org> wrote: > Hi, > > On 13 August 2015 at 12:23, Tom Rini <trini@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@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@ti.com> wrote: >>> >> > From: Ravi Babu <ravibabu@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@ti.com> >>> >> > Signed-off-by: Vignesh R <vigneshr@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); I think we can do the requested bytes as well like new_ptr = ALIGN(addr + bytes, align); thanks!
On Thu, Aug 13, 2015 at 12:38:52PM -0600, Simon Glass wrote: > Hi, > > On 13 August 2015 at 12:23, Tom Rini <trini@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@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@ti.com> wrote: > >> >> > From: Ravi Babu <ravibabu@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@ti.com> > >> >> > Signed-off-by: Vignesh R <vigneshr@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!
Hi, On 13 August 2015 at 14:54, Tom Rini <trini@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@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@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@ti.com> wrote: >> >> >> > From: Ravi Babu <ravibabu@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@ti.com> >> >> >> > Signed-off-by: Vignesh R <vigneshr@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
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); if (cmp_buf) { ulong last_update = get_timer(0); @@ -484,12 +484,12 @@ static int do_spi_flash_test(int argc, char * const argv[]) if (*argv[2] == 0 || *endp != 0) return -1; - vbuf = malloc(len); + vbuf = memalign(ARCH_DMA_MINALIGN, len); if (!vbuf) { printf("Cannot allocate memory (%lu bytes)\n", len); return 1; } - buf = malloc(len); + buf = memalign(ARCH_DMA_MINALIGN, len); if (!buf) { free(vbuf); printf("Cannot allocate memory (%lu bytes)\n", len);