Re: [PATCH] Fix detection of odd memory configurations on sunxi

["Jernej Škrabec" <jernej.skrabec@gmail.com>]

Hi,

CC: Andre, Jagan, linux-sunxi (maintainers and customary linux-sunxi ML)

Dne ponedeljek, 11. avgust 2025 ob 09:35:19 Srednjeevropski poletni čas je Rudi Horn napisal(a):
> Hello,
> 
> I encountered a bug where u-boot detects that my OrangePI zero 3 (with 
> 1.5GB)
> has 2GB and crashes. 

This is known missing feature, but you can also call it a bug. Anyway...

> The orangepi u-boot source code contains an additional
> modification in the `mctl_calc_size` which removes a quarter of the 
> memory the
> calculated last address cannot be written to:
> 
> https://github.com/orangepi-xunlong/u-boot-orangepi/blob/v2024.01/arch/arm/mach-sunxi/dram_sun50i_h616.c#L1365-L1368
> 
> I'm not entirely sure if there is some specific logic that applies to this
> modifier, but it does fix the issue on my system.
> 
> I propose the following patch, but am open to any further suggestions.

This is pretty neat solution. Please send proper submission according to rules
with fixes below.

> 
> Thanks,
> Rudi Horn
> 
> Note: Submitted in my personal capacity and is not affiliated with my 
> employer.
> 
> 
>  From 2199f3b28e5fc853ed1921586358c33f3f1502d3 Mon Sep 17 00:00:00 2001
> From: Rudi Horn <dyn-git@rudi-horn.de>
> Date: Mon, 11 Aug 2025 08:58:34 +0200
> Subject: [PATCH] arch: arm: mach-sonxi: Fix detection of odd memory
>   configurations

sonxi -> sunxi

However, better prefix would be "sunxi: H616: dram:" (based on git log.)

> 
> Fix detection of odd memory configurations. Previously 1.5GB devices were
> incorrectly detected as 2GB devices, causing u-boot to crash.
> 
> Signed-off-by: Rudi Horn <dyn-git@rudi-horn.de>
> ---
>   arch/arm/include/asm/arch-sunxi/dram.h |  1 +
>   arch/arm/mach-sunxi/dram_dw_helpers.c  | 10 +++++++++-
>   arch/arm/mach-sunxi/dram_helpers.c     |  8 ++++++++
>   3 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/dram.h 
> b/arch/arm/include/asm/arch-sunxi/dram.h
> index 0eccb1e6c28..7580421ca77 100644
> --- a/arch/arm/include/asm/arch-sunxi/dram.h
> +++ b/arch/arm/include/asm/arch-sunxi/dram.h
> @@ -44,6 +44,7 @@
>   unsigned long sunxi_dram_init(void);
>   void mctl_await_completion(u32 *reg, u32 mask, u32 val);
>   bool mctl_mem_matches(u32 offset);
> +bool mctl_mem_matches_upto(u32 offset);

I don't see a benefit for public function here, since it's unlikely to be
used anywhere else. Just mark it static. Compiler can optimize it better.

>   bool mctl_mem_matches_base(u32 offset, ulong base);
> 
>   #endif /* _SUNXI_DRAM_H */
> diff --git a/arch/arm/mach-sunxi/dram_dw_helpers.c 
> b/arch/arm/mach-sunxi/dram_dw_helpers.c
> index 24767354935..5bcd2672465 100644
> --- a/arch/arm/mach-sunxi/dram_dw_helpers.c
> +++ b/arch/arm/mach-sunxi/dram_dw_helpers.c
> @@ -146,5 +146,13 @@ unsigned long mctl_calc_size(const struct 
> dram_config *config)
>          u8 width = config->bus_full_width ? 4 : 2;
> 
>          /* 8 banks */
> -       return (1ULL << (config->cols + config->rows + 3)) * width * 
> config->ranks;
> +       unsigned long size = (1ULL << (config->cols + config->rows + 3)) 
> * width * config->ranks;
> +
> +       /* some memory configurations such as 1.5GB rely on this to 
> compute the correct size */
> +       if (!mctl_mem_matches_upto(size)) {
> +               size = (size * 3) / 4;
> +               debug("capping memory at 0x%lx\n", size);

Maybe test again? It seems to me a bit dangerous to just assume that it
will work. If it fails, panic() should be called.

Best regards,
Jernej

> +       }
> +
> +  return size;
>   }
> diff --git a/arch/arm/mach-sunxi/dram_helpers.c 
> b/arch/arm/mach-sunxi/dram_helpers.c
> index 83dbe4ca98f..68c75fa07a6 100644
> --- a/arch/arm/mach-sunxi/dram_helpers.c
> +++ b/arch/arm/mach-sunxi/dram_helpers.c
> @@ -61,4 +61,12 @@ bool mctl_mem_matches(u32 offset)
>   {
>          return mctl_mem_matches_base(offset, CFG_SYS_SDRAM_BASE);
>   }
> +
> +/*
> + * Test if memory at offset matches memory at end of DRAM
> + */
> +bool mctl_mem_matches_upto(u32 offset)
> +{
> +       return mctl_mem_matches_base(offset - sizeof(u32), 
> CFG_SYS_SDRAM_BASE);
> +}
>   #endif
> --
> 2.43.0
> 
>