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

Troy Kisky
2013-07-15T17:39:54Z

On 7/15/2013 6:41 AM, Albert ARIBAUD wrote:
> Hi Troy,
>
> On Fri, 12 Jul 2013 19:43:07 -0700, Troy Kisky
> <troy.kisky@boundarydevices.com> wrote:
>
>> On 7/11/2013 4:18 PM, Fabio Estevam wrote:
>>> On Thu, Jul 11, 2013 at 8:03 PM, Marek Vasut <marex@denx.de> wrote:
>>>> The MX28 multi-layer AHB bus can be too slow and trigger the
>>>> FEC DMA too early, before all the data hit the DRAM. This patch
>>>> ensures the data are written in the RAM before the DMA starts.
>>>> Please see the comment in the patch for full details.
>>>>
>>>> This patch was produced with an amazing help from Albert Aribaud,
>>>> who pointed out it can possibly be such a bus synchronisation
>>>> issue.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
>>>> Cc: Fabio Estevam <fabio.estevam@freescale.com>
>>>> Cc: Stefano Babic <sbabic@denx.de>
>>> Excellent, managed to transfer 90MB via TFTP on mx28evk without a
>>> single timeout.
>>>
>>> Tested-by: Fabio Estevam <fabio.estevam@freescale.com>
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>
>> Perhaps this because our memory barriers are lacking.
>>
>> Linux has this code
>> asm/io.h:#define writel(v,c) ({ __iowmb();
>> writel_relaxed(v,c); })
>>
>> asm/io.h-#ifdef CONFIG_ARM_DMA_MEM_BUFFERABLE
>> asm/io.h-#include <asm/barrier.h>
>> asm/io.h-#define __iormb() rmb()
>> asm/io.h:#define __iowmb() wmb()
>> asm/io.h-#else
>> asm/io.h-#define __iormb() do { } while (0)
>> asm/io.h:#define __iowmb() do { } while (0)
>> asm/io.h-#endif
>>
>> asm/io.h-#ifdef CONFIG_ARM_DMA_MEM_BUFFERABLE
>> asm/io.h-#include <asm/barrier.h>
>> asm/io.h-#define __iormb() rmb()
>> asm/io.h:#define __iowmb() wmb()
>> asm/io.h-#else
>> asm/io.h-#define __iormb() do { } while (0)
>> asm/io.h:#define __iowmb() do { } while (0)
>> asm/io.h-#endif
>>
>> asm/barrier.h-#elif defined(CONFIG_ARM_DMA_MEM_BUFFERABLE) ||
>> defined(CONFIG_SMP)
>> asm/barrier.h-#define mb() do { dsb(); outer_sync(); }
>> while (0)
>> asm/barrier.h-#define rmb() dsb()
>> asm/barrier.h:#define wmb() mb()
>> asm/barrier.h-#else
>> asm/barrier.h-#define mb() barrier()
>> asm/barrier.h-#define rmb() barrier()
>> asm/barrier.h:#define wmb() barrier()
>> asm/barrier.h-#endif
>>
>> asm/barrier.h-#if __LINUX_ARM_ARCH__ >= 7
>> asm/barrier.h-#define isb() __asm__ __volatile__ ("isb" : : : "memory")
>> asm/barrier.h:#define dsb() __asm__ __volatile__ ("dsb" : : : "memory")
>> asm/barrier.h-#define dmb() __asm__ __volatile__ ("dmb" : : : "memory")
>> asm/barrier.h-#elif defined(CONFIG_CPU_XSC3) || __LINUX_ARM_ARCH__ == 6
>> asm/barrier.h-#define isb() __asm__ __volatile__ ("mcr p15, 0, %0, c7,
>> c5, 4" \
>> asm/barrier.h- : : "r" (0) : "memory")
>> asm/barrier.h:#define dsb() __asm__ __volatile__ ("mcr p15, 0, %0, c7,
>> c10, 4" \
>> asm/barrier.h- : : "r" (0) : "memory")
>> asm/barrier.h-#define dmb() __asm__ __volatile__ ("mcr p15, 0, %0, c7,
>> c10, 5" \
>>
>> _____________________________________
>> Can you try just adding a dsb() instead of the dummy read?
>>
>> If this also fixes your problem, it seems better to fix our writel macro
> Not sure I understand who you are answering to exactly, as Fabio
> indicates his problem is solved.
>
> 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.

>
> 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.

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

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.

Troy