[PATCH v2 1/2] rockchip: sdram: Add rockchip_sdram_type() helper

[Quentin Schulz]

Hi Jonas,

On 8/1/25 7:09 PM, Jonas Karlman wrote:
> Add a helper function based on rockchip_sdram_size() that return what
> DRAM type is used on current running board.
> 
> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
> ---
> v2: New patch
> ---
>   arch/arm/include/asm/arch-rockchip/sdram.h |  3 +++
>   arch/arm/mach-rockchip/sdram.c             | 15 +++++++++++++++
>   2 files changed, 18 insertions(+)
> 
> diff --git a/arch/arm/include/asm/arch-rockchip/sdram.h b/arch/arm/include/asm/arch-rockchip/sdram.h
> index 4fb45ac5c76e..476fc1c4ee33 100644
> --- a/arch/arm/include/asm/arch-rockchip/sdram.h
> +++ b/arch/arm/include/asm/arch-rockchip/sdram.h
> @@ -87,6 +87,9 @@ enum {
>   #define SYS_REG_CS1_COL_SHIFT(ch)		(0 + (ch) * 2)
>   #define SYS_REG_CS1_COL_MASK			3
>   
> +/* Get sdram type decode from reg */

s/decode/decoded/

Please define in the comment how to map the u8 to the actual SDRAM type? 
I guess ideally we should have the enum in 
arch/arm/include/asm/arch-rockchip/sdram.h typedef'ed or at least named 
and return that type instead of a u8?

Please also document which register we should be passing there and the 
assumptions made in the fucntion (that the version (of what?) will be 
read in the next register as well).

> +u8 rockchip_sdram_type(phys_addr_t reg);
> +
>   /* Get sdram size decode from reg */
>   size_t rockchip_sdram_size(phys_addr_t reg);
>   
> diff --git a/arch/arm/mach-rockchip/sdram.c b/arch/arm/mach-rockchip/sdram.c
> index 3bc482331c7e..d560f90e873d 100644
> --- a/arch/arm/mach-rockchip/sdram.c
> +++ b/arch/arm/mach-rockchip/sdram.c
> @@ -345,6 +345,21 @@ int dram_init_banksize(void)
>   	return 0;
>   }
>   
> +u8 rockchip_sdram_type(phys_addr_t reg)
> +{
> +	u32 dram_type, version;
> +	u32 sys_reg2 = readl(reg);
> +	u32 sys_reg3 = readl(reg + 4);
> +
> +	dram_type = (sys_reg2 >> SYS_REG_DDRTYPE_SHIFT) & SYS_REG_DDRTYPE_MASK;
> +	version = (sys_reg3 >> SYS_REG_VERSION_SHIFT) & SYS_REG_VERSION_MASK;
> +	if (version >= 3)
> +		dram_type |= ((sys_reg3 >> SYS_REG_EXTEND_DDRTYPE_SHIFT) &
> +			      SYS_REG_EXTEND_DDRTYPE_MASK) << 3;
> +
> +	return dram_type;
> +}

I would have preferred to use FIELD_GET here but I now see that this is 
extracted from rockchip_sdram_size where it's like that.

Can we make rockchip_sdram_size() a user of rockchip_sdram_type() to 
make sure it doesn't get outdated?

Cheers,
Quentin