Re: [PATCH v1 11/11] [RFC] rockchip: rk356x: attempt to fix ram detection

Simon Glass
2022-03-12T02:28:09Z

Hi Peter,

On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
>
> This patch attempts to fix ram detection on rk3568.
> Prior to this, the rk3568 incorrectly detected 8gb ram as 2gb.
> On top of this, the board panics when u-boot accesses ram above 4gb.
>
> Fix this by correcting ram detection in hopefully a backwards compatable
> way, and extend board_f.c to enforce an upper limit on the ram u-boot
> uses.
> This allows us to limit the ram u-boot accesses, while passing the
> correctly detected size to follow on software (eg linux).
>
> This has been tested on rk3566 2gb, 4gb, and 8gb configurations, as well
> as rk3399 4gb configurations.
>
> I do not have other configurations available, and I do not have the
> insights into rockchip ram handling to tell if this is the correct way
> to go about this.
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
> arch/arm/mach-rockchip/Kconfig | 1 +
> arch/arm/mach-rockchip/rk3568/rk3568.c | 29 ++++++++++++++++++++++++++
> arch/arm/mach-rockchip/sdram.c | 19 +++++++++++------
> common/board_f.c | 7 +++++++
> include/configs/rk3568_common.h | 5 +++++
> 5 files changed, 55 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index 92f35309e4a6..58393cc623f8 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -264,6 +264,7 @@ config ROCKCHIP_RK3568
> select SYSCON
> select BOARD_LATE_INIT
> imply ROCKCHIP_COMMON_BOARD
> + imply OF_SYSTEM_SETUP
> help
> The Rockchip RK3568 is a ARM-based SoC with quad-core Cortex-A55,
> including NEON and GPU, 512K L3 cache, Mali-G52 based graphics,
> diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> index ef6bc67a88b0..8d2a59bc649d 100644
> --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> @@ -5,6 +5,7 @@
>
> #include <common.h>
> #include <dm.h>
> +#include <fdt_support.h>
> #include <asm/armv8/mmu.h>
> #include <asm/io.h>
> #include <asm/arch-rockchip/bootrom.h>
> @@ -187,3 +188,31 @@ int board_usb_init(int index, enum usb_init_type init)
> #endif /* CONFIG_USB_DWC3_GADGET */
>
> #endif /* CONFIG_USB_GADGET */
> +
> +#ifdef CONFIG_OF_SYSTEM_SETUP
> +int ft_system_setup(void *blob, struct bd_info *bd)
> +{
> + int ret;
> + int areas = 1;
> + u64 start[2], size[2];
> +
> + /* Reserve the io address space. */
> + if (gd->ram_top > SDRAM_UPPER_ADDR_MIN) {

Can you put SDRAM_UPPER_ADDR_MIN and SDRAM_LOWER_ADDR_MAX into the
device tree, perhaps?

> + start[0] = gd->bd->bi_dram[0].start;
> + size[0] = SDRAM_LOWER_ADDR_MAX - gd->bd->bi_dram[0].start;
> +
> + /* Add the upper 4GB address space */
> + start[1] = SDRAM_UPPER_ADDR_MIN;
> + size[1] = gd->ram_top - SDRAM_UPPER_ADDR_MIN;
> + areas = 2;
> +
> + ret = fdt_set_usable_memory(blob, start, size, areas);
> + if (ret) {
> + printf("Cannot set usable memory\n");
> + return ret;
> + }
> + }
> +
> + return 0;
> +};
> +#endif
> diff --git a/arch/arm/mach-rockchip/sdram.c b/arch/arm/mach-rockchip/sdram.c
> index 705ec7ba6450..52974e6dc333 100644
> --- a/arch/arm/mach-rockchip/sdram.c
> +++ b/arch/arm/mach-rockchip/sdram.c
> @@ -3,6 +3,8 @@
> * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
> */
>
> +#define DEBUG
> +
> #include <common.h>
> #include <dm.h>
> #include <init.h>
> @@ -98,8 +100,7 @@ size_t rockchip_sdram_size(phys_addr_t reg)
> SYS_REG_COL_MASK);
> cs1_col = cs0_col;
> bk = 3 - ((sys_reg2 >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
> - if ((sys_reg3 >> SYS_REG_VERSION_SHIFT &
> - SYS_REG_VERSION_MASK) == 0x2) {
> + if ((sys_reg3 >> SYS_REG_VERSION_SHIFT & SYS_REG_VERSION_MASK) >= 0x2) {
> cs1_col = 9 + (sys_reg3 >> SYS_REG_CS1_COL_SHIFT(ch) &
> SYS_REG_CS1_COL_MASK);
> if (((sys_reg3 >> SYS_REG_EXTEND_CS0_ROW_SHIFT(ch) &
> @@ -136,7 +137,7 @@ size_t rockchip_sdram_size(phys_addr_t reg)
> SYS_REG_BW_MASK));
> row_3_4 = sys_reg2 >> SYS_REG_ROW_3_4_SHIFT(ch) &
> SYS_REG_ROW_3_4_MASK;
> - if (dram_type == DDR4) {
> + if ((dram_type == DDR4) && (sys_reg3 >> SYS_REG_VERSION_SHIFT & SYS_REG_VERSION_MASK) != 0x3){

Space before {

Also can you please add a comment as to what this is doing?

> dbw = (sys_reg2 >> SYS_REG_DBW_SHIFT(ch)) &
> SYS_REG_DBW_MASK;
> bg = (dbw == 2) ? 2 : 1;
> @@ -176,9 +177,11 @@ size_t rockchip_sdram_size(phys_addr_t reg)
> * 2. update board_get_usable_ram_top() and dram_init_banksize()
> * to reserve memory for peripheral space after previous update.
> */
> +
> +#ifndef __aarch64__

if (!IS_ENABLED(CONFIG_ARM64))

> if (size_mb > (SDRAM_MAX_SIZE >> 20))
> size_mb = (SDRAM_MAX_SIZE >> 20);
> -
> +#endif
> return (size_t)size_mb << 20;
> }
>
> @@ -208,6 +211,10 @@ int dram_init(void)
> ulong board_get_usable_ram_top(ulong total_size)
> {
> unsigned long top = CONFIG_SYS_SDRAM_BASE + SDRAM_MAX_SIZE;
> -
> - return (gd->ram_top > top) ? top : gd->ram_top;
> +#ifdef SDRAM_UPPER_ADDR_MIN

eek, what is going on here? Can you use C instead of the preprocessor?

> + if (gd->ram_top > SDRAM_UPPER_ADDR_MIN)
> + return gd->ram_top;
> + else
> +#endif
> + return (gd->ram_top > top) ? top : gd->ram_top;
> }
> diff --git a/common/board_f.c b/common/board_f.c
> index a68760092ac1..933ba7aedac0 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -344,7 +344,14 @@ static int setup_dest_addr(void)
> #endif
> gd->ram_top = gd->ram_base + get_effective_memsize();
> gd->ram_top = board_get_usable_ram_top(gd->mon_len);
> +#ifdef SDRAM_LOWER_ADDR_MAX
> + if (gd->ram_top > SDRAM_LOWER_ADDR_MAX)
> + gd->relocaddr = SDRAM_LOWER_ADDR_MAX;
> + else
> + gd->relocaddr = gd->ram_top;
> +#else
> gd->relocaddr = gd->ram_top;
> +#endif
> debug("Ram top: %08lX\n", (ulong)gd->ram_top);
> #if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
> /*
> diff --git a/include/configs/rk3568_common.h b/include/configs/rk3568_common.h
> index 25d7c5cc8fff..8dd1b033017b 100644
> --- a/include/configs/rk3568_common.h
> +++ b/include/configs/rk3568_common.h
> @@ -27,6 +27,11 @@
> #define CONFIG_SYS_SDRAM_BASE 0
> #define SDRAM_MAX_SIZE 0xf0000000
>
> +#ifdef CONFIG_OF_SYSTEM_SETUP
> +#define SDRAM_LOWER_ADDR_MAX 0xf0000000
> +#define SDRAM_UPPER_ADDR_MIN 0x100000000
> +#endif

These config files are going away, so either use Kconfig or device tree.

> +
> #ifndef CONFIG_SPL_BUILD
> #define ENV_MEM_LAYOUT_SETTINGS \
> "scriptaddr=0x00c00000\0" \
> --
> 2.25.1
>

Regards,
Simon