[U-Boot] Cache alignment warnings on Tegra (ARM)

[Marek Vasut]

Dear Thierry Reding,

> On Sat, Sep 15, 2012 at 10:11:54PM +0200, Marek Vasut wrote:
> > Dear Thierry Reding,
> > 
> > > On Fri, Sep 14, 2012 at 08:53:32AM -0700, Simon Glass wrote:
> > > > Hi,
> > > > 
> > > > On Wed, Sep 12, 2012 at 4:42 PM, Marek Vasut <marex at denx.de> wrote:
> > > > > Dear Stephen Warren,
> > > > > 
> > > > >> On 09/12/2012 04:38 PM, Marek Vasut wrote:
> > > > >> > Dear Stephen Warren,
> > > > >> > 
> > > > >> >> On 09/12/2012 10:19 AM, Tom Warren wrote:
> > > > >> >>> Folks,
> > > > >> >>> 
> > > > >> >>> Stephen Warren has posted an internal bug regarding the cache
> > > > >> >>> alignment 'warnings' seen on Tegra20 boards when accessing
> > > > >> >>> MMC. Here's the gist:
> > > > >> >>> 
> > > > >> >>> Executing "mmc dev 0" still yields cache warnings:
> > > > >> >>> 
> > > > >> >>> Tegra20 (Harmony) # mmc dev 0
> > > > >> >>> ERROR: v7_dcache_inval_range- stop address is not aligned-
> > > > >> >>> 0x3fb69908 mmc0 is current device
> > > > >> >> 
> > > > >> >> ...
> > > > >> >> 
> > > > >> >>> There have been patches in the past (IIRC) that have tried to
> > > > >> >>> ensure all callers (FS, MMC driver, USB driver, etc.) force
> > > > >> >>> their buffers to the appropriate alignment, but I don't know
> > > > >> >>> that we can ever correct every instance, now or in the
> > > > >> >>> future.
> > > > >> >>> 
> > > > >> >>> Can we start a discussion about what we can do about this
> > > > >> >>> warning? Adding an appropriate #ifdef
> > > > >> >>> (CONFIG_SYS_NO_CACHE_ALIGNMENT_WARNINGS, etc.) where Stephen
> > > > >> >>> put his #if 0's would be one approach, or changing the
> > > > >> >>> printf() to a debug(), perhaps. As far as I can tell, these
> > > > >> >>> alignment 'errors' don't seem to produce bad data in the
> > > > >> >>> transfer.
> > > > >> >> 
> > > > >> >> I don't think simply turning off the warning is the correct
> > > > >> >> approach; I believe they represent real problems that can in
> > > > >> >> fact cause data corruption. I don't believe we have any choice
> > > > >> >> other than to fully solve the root-cause.
> > > > 
> > > > Yes I agree, and I think it is pretty close - certainly much better
> > > > than it used to be. The good thing about them being annoying is that
> > > > they will likely get fixed :-)
> > > 
> > > I think I traced this to the copying of CSD a while back. The problem
> > > is that the transferred buffer is 8 bytes, so there's no way to make
> > > it aligned properly. Unfortunately the entailing discussion did not
> > > yield a solution at the time.
> > 
> > And how exactly does the MMC bounce buffer not help here?
> 
> The problem solved by the bounce buffer is that it is properly cache-
> line aligned. However the issue here is not that the buffer is not
> properly aligned but rather that the transfer is too small.
> 
> When the MMC core transfers the SCR, it requests 8 bytes. The buffer
> used to store these 8 bytes is properly allocated. However, those 8
> bytes eventually end up as the size of the range that is to be
> invalidated. This is the reason for the warning. Address x of the buffer
> is cache-line aligned, but x + 8 is never properly aligned and therefore
> the code complains.

The bounce buffer also up-aligns the newly buffer size to ARCH_DMA_MINALIGN, so 
you can safely up-align you cacheline flush/invalidate sizes.

> Thierry

Best regards,
Marek Vasut