[U-Boot] [PATCH] sunxi: mctl_mem_matches: Add missing memory barrier

[Siarhei Siamashka]

Hello Hans,

On Thu, 14 Apr 2016 18:58:04 +0200
Hans de Goede <hdegoede at redhat.com> wrote:

> We are running with the caches disabled when mctl_mem_matches gets called,
> but the cpu's write buffer is still there and can still get in the way,

This does not make much sense to me. The SPL is running with the MMU
disabled, because disabling the MMU is one of the first things done by
the SPL at the very start. And when the MMU is disabled, all the data
accesses are treated as Strongly-ordered and are not supposed to use
the write buffer. A quote from the ARMv7 Architecture Manual:

   "a write to Strongly-ordered memory can complete only when it
   reaches the peripheral or memory component accessed by the write"

We can even verify whether the write buffer is actually in use by simply
benchmarking something like the memset function. If the write buffer is
working, then the sequential write speed will be around 1 GB/s or more.

> add a memory barrier to fix this.
> 
> This avoids mctl_mem_matches always returning false in some cases, which
> was resulting in:
> 
> U-Boot SPL 2015.07 (Apr 14 2016 - 18:47:26)
> DRAM: 1024 MiB
> 
> U-Boot 2015.07 (Apr 14 2016 - 18:47:26 +0200) Allwinner Technology
> 
> CPU:   Allwinner A23 (SUN8I)
> DRAM:  512 MiB
> 
> Where 512 MiB is the right amount, but the DRAM controller would be
> initialized for 1024 MiB.

Is it just a single device or board? Has anybody seen anything like
this on other devices with the same SoC?

I wonder if what you are observing could be possibly explained by just
a usual data corruption problem? Which may be happening when the DRAM
clock speed is set higher than this particular device is able to handle
in a reliable way. Inserting just one or more NOP instructions instead
of the barrier could possibly change some timings too.

If this patch helps, then it's fine. But I wonder if it is not merely
making the problem latent instead of fixing the root cause?

> 
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  arch/arm/mach-sunxi/dram_helpers.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/mach-sunxi/dram_helpers.c b/arch/arm/mach-sunxi/dram_helpers.c
> index 50318d2..e0c823a 100644
> --- a/arch/arm/mach-sunxi/dram_helpers.c
> +++ b/arch/arm/mach-sunxi/dram_helpers.c
> @@ -7,6 +7,7 @@
>   */
>  
>  #include <common.h>
> +#include <asm/armv7.h>
>  #include <asm/io.h>
>  #include <asm/arch/dram.h>
>  
> @@ -31,6 +32,7 @@ bool mctl_mem_matches(u32 offset)
>  	/* Try to write different values to RAM at two addresses */
>  	writel(0, CONFIG_SYS_SDRAM_BASE);
>  	writel(0xaa55aa55, (ulong)CONFIG_SYS_SDRAM_BASE + offset);
> +	DSB;
>  	/* Check if the same value is actually observed when reading back */
>  	return readl(CONFIG_SYS_SDRAM_BASE) ==
>  	       readl((ulong)CONFIG_SYS_SDRAM_BASE + offset);

-- 
Best regards,
Siarhei Siamashka