[PATCH 1/1] sunxi: dram: Fix incorrect ram size detection for some H6 boards
Andre Przywara
andre.przywara at arm.com
Mon Oct 2 13:26:26 CEST 2023
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 +
More information about the U-Boot
mailing list