[U-Boot] [PATCH] Fix unreliable detection of DRAM size on Orange Pi 3

[Ondřej Jirman]

Hello,

On Sun, Aug 25, 2019 at 05:41:55PM +0300, Siarhei Siamashka wrote:
> On Sat, 24 Aug 2019 22:07:43 +0200
> Ondřej Jirman <megous at megous.com> wrote:
> 
> > Hi Jagan,
> > 
> > can you please apply this patch to the sunxi tree, so that it
> > doesn't get lost.
> > 
> > thank you,
> > 	Ondrej
> > 
> > On Mon, Jul 29, 2019 at 01:39:42AM +0200, megous hlavni wrote:
> > > From: Ondrej Jirman <megous at megous.com>
> > > 
> > > Orange Pi 3 has 2 GiB of DRAM, that sometime get misdetected
> > > as 4 GiB, due to false negative result from mctl_mem_matches()
> > > when detecting number of column address bits. This leads to
> > > u-boot detecting more address bits than there are and the
> > > boot process hangs shortly after.
> > > 
> > > In mctl_mem_matches() we need to wait for each write to finish,
> > > separately. Without this, the check is not reliable for some
> > > unknown reason, probably having to do with unpredictable memory
> > > access ordering.
> > > 
> > > Patch was made with help from André Przywara, who noticed that
> > > my original idea about detection failing due to read-back from
> > > cache without involving DRAM was false, because data cache is
> > > still of at the time of the DRAM size autodetection.
> > > 
> > > Signed-off-by: Ondrej Jirman <megous at megous.com>
> > > Cc: André Przywara <andre.przywara at arm.com>
> > > ---
> > >  arch/arm/mach-sunxi/dram_helpers.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/arch/arm/mach-sunxi/dram_helpers.c b/arch/arm/mach-sunxi/dram_helpers.c
> > > index 239ab421a8..6dba448638 100644
> > > --- a/arch/arm/mach-sunxi/dram_helpers.c
> > > +++ b/arch/arm/mach-sunxi/dram_helpers.c
> > > @@ -30,6 +30,7 @@ bool mctl_mem_matches(u32 offset)
> > >  {
> > >  	/* Try to write different values to RAM at two addresses */
> > >  	writel(0, CONFIG_SYS_SDRAM_BASE);
> > > +	dsb();
> > >  	writel(0xaa55aa55, (ulong)CONFIG_SYS_SDRAM_BASE + offset);
> > >  	dsb();
> > >  	/* Check if the same value is actually observed when reading back */
> > > -- 
> 
> Hi!
> 
> This looks like resurfacing of the old problem, which did not get a
> complete fix back in 2016:
>    https://lists.denx.de/pipermail/u-boot/2016-April/251803.html
> 
> Now the main question is whether the DSB instruction's barrier
> magic is really required here or maybe it's just a timing issue.

That's certainly possible, because without this patch, the problem
appears intermittently, and not always. I also have the same end
result, where mctl_mem_matches returns false when it should not.

> Could you please experiment with this problem a little bit more?
> 
>  1. What if you just move this second DSB instruction after the
>     second write and have two of them there? Does this also make
>     the problem disappear?
> 
>  2. What if you just replace DSB with udelay(1) / udelay(10) /
>     udelay(100)? Does this make the problem disappear?
> 
> 
> I suspect that we may be dealing with some sort of buffering in
> the DRAM controller itself and the only reason why DSB helps is
> that it introduces some delay because it's a rather slow
> instruction.

Thanks for suggestions. If it's just the delay, then whatever delay
dsb introduces seems enough. It'll probably need just a small delay,
because most of the time it works without one.

regards,
	Ondrej

> -- 
> Best regards,
> Siarhei Siamashka