[PATCH v2 2/2] rockchip: rock5b-rk3588: Add support for ROCK 5B+

[Quentin Schulz]

Hi Jonas,

On 8/1/25 7:09 PM, Jonas Karlman wrote:
> Include FDTs for both ROCK 5B and 5B+ in the FIT and add board selection
> code to load the 5B+ FDT when the DRAM type is LPDDR5 and ADC channel 5
> value is close to 4095.
> 
>    U-Boot 2025.07 (Jul 14 2025 - 21:28:20 +0000)
> 
>    Model: Radxa ROCK 5B+
>    SoC:   RK3588
>    DRAM:  8 GiB
> 
> Features tested on a ROCK 5B+ v1.2:
> - SD-card boot
> - eMMC boot
> - SPI flash boot
> - PCIe/NVMe
> - Ethernet
> - USB/TCPM
> 
> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
> ---
> Changes in v2:
> - Add DRAM type check in addition to SARADC check
> ---
>   arch/arm/dts/rk3588-rock-5b-plus-u-boot.dtsi |  3 +
>   arch/arm/dts/rk3588-rock-5b-u-boot.dtsi      |  5 ++
>   board/radxa/rock5b-rk3588/Kconfig            |  5 ++
>   board/radxa/rock5b-rk3588/MAINTAINERS        |  3 +-
>   board/radxa/rock5b-rk3588/rock5b-rk3588.c    | 63 ++++++++++++++++++++
>   configs/rock5b-rk3588_defconfig              |  1 +
>   doc/board/rockchip/rockchip.rst              |  2 +-
>   7 files changed, 79 insertions(+), 3 deletions(-)
>   create mode 100644 arch/arm/dts/rk3588-rock-5b-plus-u-boot.dtsi
> 
> diff --git a/arch/arm/dts/rk3588-rock-5b-plus-u-boot.dtsi b/arch/arm/dts/rk3588-rock-5b-plus-u-boot.dtsi
> new file mode 100644
> index 000000000000..c07696c83913
> --- /dev/null
> +++ b/arch/arm/dts/rk3588-rock-5b-plus-u-boot.dtsi
> @@ -0,0 +1,3 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +#include "rk3588-rock-5b-u-boot.dtsi"
> diff --git a/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi b/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
> index d51fbf51cb88..e07b549c767f 100644
> --- a/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
> +++ b/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
> @@ -46,6 +46,11 @@
>   	};
>   };
>   
> +&saradc {
> +	bootph-pre-ram;
> +	vdd-microvolts = <1800000>;
> +};
> +
>   &sdhci {
>   	cap-mmc-highspeed;
>   	mmc-hs200-1_8v;
> diff --git a/board/radxa/rock5b-rk3588/Kconfig b/board/radxa/rock5b-rk3588/Kconfig
> index 41dfe2402b12..98d630117836 100644
> --- a/board/radxa/rock5b-rk3588/Kconfig
> +++ b/board/radxa/rock5b-rk3588/Kconfig
> @@ -9,4 +9,9 @@ config SYS_VENDOR
>   config SYS_CONFIG_NAME
>   	default "rock5b-rk3588"
>   
> +config BOARD_SPECIFIC_OPTIONS # dummy
> +	def_bool y
> +	select ADC
> +	select SPL_ADC
> +
>   endif
> diff --git a/board/radxa/rock5b-rk3588/MAINTAINERS b/board/radxa/rock5b-rk3588/MAINTAINERS
> index 4460c9971a96..c8a43769105e 100644
> --- a/board/radxa/rock5b-rk3588/MAINTAINERS
> +++ b/board/radxa/rock5b-rk3588/MAINTAINERS
> @@ -5,5 +5,4 @@ S:	Maintained
>   F:	board/radxa/rock5b-rk3588
>   F:	include/configs/rock5b-rk3588.h
>   F:	configs/rock5b-rk3588_defconfig
> -F:	arch/arm/dts/rk3588-rock-5b.dts
> -F:	arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
> +F:	arch/arm/dts/rk3588-rock-5b*
> diff --git a/board/radxa/rock5b-rk3588/rock5b-rk3588.c b/board/radxa/rock5b-rk3588/rock5b-rk3588.c
> index fc2f69db2241..6bf4497ce3ae 100644
> --- a/board/radxa/rock5b-rk3588/rock5b-rk3588.c
> +++ b/board/radxa/rock5b-rk3588/rock5b-rk3588.c
> @@ -3,8 +3,71 @@
>    * Copyright (c) 2023-2024 Collabora Ltd.
>    */
>   
> +#include <adc.h>
> +#include <env.h>
>   #include <fdtdec.h>
>   #include <fdt_support.h>
> +#include <asm/arch-rockchip/sdram.h>
> +#include <linux/errno.h>
> +
> +#define PMU1GRF_BASE		0xfd58a000
> +#define OS_REG2_REG		0x208
> +

I assume the register address and meaning is stable across all RK3588, 
so maybe we should abstract that somewhere in the SoC file 
(arch/arm/mach-rockchip/rk3588) so that the caller doesn't need to 
specify them?

> +#define HW_ID_CHANNEL		5
> +
> +struct board_model {
> +	unsigned int dram;
> +	unsigned int low;
> +	unsigned int high;
> +	const char *fdtfile;
> +};
> +
> +static const struct board_model board_models[] = {
> +	{ LPDDR5, 4005, 4185, "rockchip/rk3588-rock-5b-plus.dtb" },
> +};
> +
> +static const struct board_model *get_board_model(void)
> +{
> +	unsigned int val, dram_type;
> +	int i, ret;

i could be an unsigned number type as well. (and it should since we're 
using it to access items in an array)

> +
> +	dram_type = rockchip_sdram_type(PMU1GRF_BASE + OS_REG2_REG);
> +
> +	ret = adc_channel_single_shot("adc at fec10000", HW_ID_CHANNEL, &val);
> +	if (ret)
> +		return NULL;
> +

While checking whether adc_channel_single_shot() is the right function 
to call, I stumbled upon the call in arch/arm/mach-rockchip/boot_mode.c 
which i believe wouldn't work for Rockchip SoCs whose DTSI uses adc@ as 
node name instead of saradc at . That would be rk3328, rk3588, rk3576, 
rk3528 and rv1126.

I guess there are different ways to fix this. We could have each SoC 
define a constant string with the node name of the SARADC or have an 
additional entry in /aliases for saradc in the DT? Maybe some other way 
could be used as well, just throwing ideas :)

Unrelated to this patch though, so feel free to ignore.

Cheers,
Quentin