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

[Hans de Goede]

Hi,

On 15-04-16 02:46, Siarhei Siamashka wrote:
> 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"

I was surprised by this myself too, but I noticed that mctl_mem_matches
has started to return always false on one A23 tablet (detecting 1G of mem
instead of 512MB), I then tried it on multiple A23 tablets (3 in total)
and they all showed the same problem.

I also could not remember seeing this before, so went back u-boot versions
all the way to v2015.07 but the problem was still there, which makes
me believe that it is triggered by recent gcc versions. Note I'm seeing
the problem with u-boot build with gcc-5.3 as well as with gcc-6.

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

3 a23 devices.
>
> 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?

I do believe that this patch addresses a real problem and is not hiding
some dram timing issues, I might be wrong about the write-buffer being
the cause, it could simply be that the compiler is doing something bad
(despite the accesses being marked as volatile)  and that the DSB stops
the compiler from optimizing things too much.

Regards,

Hans