[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

Marek Vasut
2012-04-01T15:35:18Z

Dear Stefano Babic,

> On 01/04/2012 15:46, Marek Vasut wrote:
> > Dear Stefano Babic,
>
> Hi Marek,
>
> >> If the range passed to flush_cache is not multiple
> >> of ARCH_DMA_MINALIGN, a warning due to mislaignment
> >> is printed.
> >> Detected with fec_mxc, mx35 boards:
> >>
> >> CACHE: Misaligned operation at range [80800000, 8083c310]
> >>
> >> Signed-off-by: Stefano Babic <sbabic@denx.de>
> >> CC: Marek Vasut <marex@denx.de>
> >> CC: Joe Hershberger <joe.hershberger@gmail.com>
> >> Cc: Wolfgang Denk <wd@denx.de>
> >> ---
> >>
> >> common/cmd_net.c | 2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/common/cmd_net.c b/common/cmd_net.c
> >> index 65f32bc..a500919 100644
> >> --- a/common/cmd_net.c
> >> +++ b/common/cmd_net.c
> >> @@ -256,7 +256,7 @@ static int netboot_common(enum proto_t proto,
> >> cmd_tbl_t *cmdtp, int argc, }
> >>
> >> /* flush cache */
> >>
> >> - flush_cache(load_addr, size);
> >> + flush_cache(load_addr, roundup(size, ARCH_DMA_MINALIGN));
> >
> > This ain't gonna slide. You might overwrite something, even though this
> > is just loading into memory, right?
>
> Really we do not reserve space before a network transfer and we pass
> only the start address (load address) where the file is copied. It is
> not known its size before the transfer. So maybe it is not important up
> to further 31 bytes are flushed ;-)
>
> Anyway, for my understanding: we are calling a function to flush caches.
> This means that if the size is something more as required there will be
> a cache hit misseing, and the SOC will load data again - but without
> corrupt data, right ?

Sure, but you can flush data into the RAM that should have never been flushed to
the ram in the first place and that might confuse some controller -- or I might
be overly paranoid.

>
> > I'm not quite sure how to handle this kind of
> > unaligned access.
>
> Well, my first goal was to explain which is the cause of the
> misalignment I found last friday testing Anatolij's patch, proofing that
> it is not due to last FEC patches ;-)
>
> > But adding at least if (unaligned) debug(...); to aid people easily
> > finding these trouble would be nice ;-)
>
> Not sure - where should be inserted or what do you exactly mean? size
> is the length of the transfered bytes, and can assueme any value, so it
> is often not aligned.

Well just before the cache-line aligning.

> Best regards,
> Stefano Babic

Best regards,
Marek Vasut