[U-Boot] [PATCH] net: fec: Avoid MX28 bus sync issue

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

Hi Troy,

On Mon, 15 Jul 2013 10:39:54 -0700, Troy Kisky
<troy.kisky@boundarydevices.com> wrote:

> > Besides, Marek and I had in fact investigated barriers, adding some as
> > I did in times past in mvgbe.c, and fiddling with the one already in
> > dcache_flush_range(). None of this had any effect.
> 
> You tried adding a  dsb()  to dcache_flush_range()?
> That should have fixed the problem as well.

There already was a memory barrier -- the only one kind known bu
ARM926J-S, which is a write buffer(s) drain -- and no, it should not
have fixed the problem (and did not), because the issue is not about
pushing the transactions out of the CPU soon enough; it is about having
the EMI perform them before it passes our 'go' command to the ENET DMA.

> > However, if our barriers are lacking, then they may fail us somehow on
> > other occasions, so best is if we analyze the failings. Can you clarify
> > in what respect exactly they are lacking? For instance, are all our
> > barriers lacking, or only some, and which ones?
> >
> > Amicalement,
> 
> Linux has
> 
> Kconfig:config ARM_DMA_MEM_BUFFERABLE
> Kconfig-        bool "Use non-cacheable memory for DMA" if (CPU_V6 || 
> CPU_V6K) && !CPU_V7
> Kconfig-        depends on !(MACH_REALVIEW_PB1176 || REALVIEW_EB_ARM11MP 
> || \
> Kconfig-                     MACH_REALVIEW_PB11MP)
> Kconfig-        default y if CPU_V6 || CPU_V6K || CPU_V7
> Kconfig-        help
> 
> 
> So, if this symbol is y then all writel/readl will be preceded by a 
> dsb() as shown above.

While I do understand what is done there, I do not see the interest
since 1) in our issue, the problem was not in any writel() or readl(),
and 2) U-Boot uses readl() and writel() for device memory ranges, which
are not cached at all and thus do not need any flush, invalidate or
barrier.

> However, u-boot probably uses cacheable memory for dma, so maybe u-boot 
> is also correct with

Actually, in the driver where the issue occurred, U-boot does use
cacheable memory for DMA; that's why the driver contains calls to
flush_dcache_range() and invalidate_dcache_range() on the descriptors...

> asm/io.h-#define dmb()          __asm__ __volatile__ ("" : : : "memory")
> asm/io.h-#define __iormb()    dmb()
> asm/io.h:#define __iowmb()    dmb()
> 
> and no dsb(), but perhaps flush_dcache still needs one at the end.

... but that's for descriptors, which are not accessed uwing writel()
or readl(); for device registers, such as ENET, we use writel() and
readl() but no cache.

So far we never had to use any data memory barrier on peripheral
accesses. The worst that happened AFAIK is when we needed instruction
barriers to prevent the compiler from mis-ordering device writes wrt
their source code order; and at that time, our readl() and writel()
implementations were not volatile. Today, I am pretty sure that these
instruction barriers are uneeded.

> Troy

Amicalement,
-- 
Albert.