[PATCH 1/1] sunxi: dram: Fix incorrect ram size detection for some H6 boards

[Andre Przywara]

On Sun,  1 Oct 2023 21:43:32 +0530
Gunjan Gupta <viraniac at gmail.com> wrote:

(fixing Jernej's email) 

Hi Gunjan,

thanks for sending a patch!
 
> On some H6 boards like Orange Pi 3 LTS, some times U-Boot fails to detect
> ram size correctly. Instead of 2GB thats available, it detects 4GB of ram
> and then SPL just hangs there making board not to boot further.
> 
> On debugging, I found that the rows value were being determined correctly,
> but columns were sometimes off by one value. I found that adding some
> delay after the mctl_core_init call along with making use of dsb in the
> start of the mctl_mem_matches solves the issue.
> 
> Signed-off-by: Gunjan Gupta <viraniac at gmail.com>
> ---
> 
>  arch/arm/mach-sunxi/dram_helpers.c   | 1 +
>  arch/arm/mach-sunxi/dram_sun50i_h6.c | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/arch/arm/mach-sunxi/dram_helpers.c b/arch/arm/mach-sunxi/dram_helpers.c
> index cdf2750f1c..5758c58e07 100644
> --- a/arch/arm/mach-sunxi/dram_helpers.c
> +++ b/arch/arm/mach-sunxi/dram_helpers.c
> @@ -32,6 +32,7 @@ void mctl_await_completion(u32 *reg, u32 mask, u32 val)
>  #ifndef CONFIG_MACH_SUNIV
>  bool mctl_mem_matches(u32 offset)
>  {
> +	dsb();

This looks a bit odd, do you have an explanation for that? And are you
sure that is really needed?
I understand why we need the DSB after the writel's below, but before that?
The only thing I could think of is that we are missing a barrier in
mctl_core_init() - which is the function called before mctl_mem_matches().
Can you move that dsb(); into mctl_auto_detect_dram_size(), right after
the mctl_core_init() call (where you add the udelay() below)? And I wonder
if a dmb() would already be sufficient? I noticed recently that the
clr/setbit_le32() functions don't have a barrier at all, maybe that should
be fixed instead?

>  	/* Try to write different values to RAM at two addresses */
>  	writel(0, CFG_SYS_SDRAM_BASE);
>  	writel(0xaa55aa55, (ulong)CFG_SYS_SDRAM_BASE + offset);
> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> index bff2e42513..a031a845f5 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> @@ -623,6 +623,8 @@ static void mctl_auto_detect_dram_size(struct dram_para *para)
>  	para->cols = 11;
>  	mctl_core_init(para);
>  
> +	udelay(50);

The location of that udelay() looks a bit odd, any chance that belongs at
the end of mctl_channel_init() instead? And how did you come up with that
and the value of 50? Just pure experimentation? I think the original BSP
DRAM code has plenty of delay calls, so we might have missed this one
here, but I would love to see some explanation here.

Cheers,
Andre

>  	for (para->cols = 8; para->cols < 11; para->cols++) {
>  		/* 8 bits per byte and 16/32 bit width */
>  		if (mctl_mem_matches(1 << (para->cols + 1 +