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

[Troy Kisky]

On 7/15/2013 10:39 AM, Troy Kisky wrote:
> 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 at 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 at 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 at denx.de>
>>>>> Cc: Albert ARIBAUD <albert.u.boot at aribaud.net>
>>>>> Cc: Fabio Estevam <fabio.estevam at freescale.com>
>>>>> Cc: Stefano Babic <sbabic at denx.de>
>>>> Excellent, managed to transfer 90MB via TFTP on mx28evk without a
>>>> single timeout.
>>>>
>>>> Tested-by: Fabio Estevam <fabio.estevam at 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
>

for armv7, flush dcache does have a dsb.

//u-boot
#define CP15DSB asm volatile ("mcr     p15, 0, %0, c7, c10, 4" : : "r" (0))

//linux
asm/barrier.h:#define dsb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, 
c10, 4"  : : "r" (0) : "memory")



Don't know why I didn't see that before.
So, I don't know why that wasn't good enough.

Maybe  CONFIG_SYS_DCACHE_OFF was set and

void flush_dcache_range(unsigned long start, unsigned long stop)
{
}

Needs a dsb too???


Troy