Re: [PATCH] rockchip: rk3588s: Add support for ROC-PC-RK3588S

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Fabio,

On 6/26/26 3:53 AM, Fabio Estevam wrote:
> The ROC-PC-RK3588S board is an SBC made by Firefly, based on the RK3588S
> SoC.
> 
> Add support for it.
> 
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---
>   arch/arm/dts/rk3588s-roc-pc-u-boot.dtsi  |  7 ++
>   arch/arm/mach-rockchip/rk3588/Kconfig    |  7 ++
>   board/firefly/roc-pc-rk3588s/Kconfig     | 12 ++++
>   board/firefly/roc-pc-rk3588s/MAINTAINERS |  7 ++
>   configs/roc-pc-rk3588s_defconfig         | 91 ++++++++++++++++++++++++
>   doc/board/rockchip/rockchip.rst          |  1 +
>   include/configs/roc-pc-rk3588s.h         |  8 +++
>   7 files changed, 133 insertions(+)
>   create mode 100644 arch/arm/dts/rk3588s-roc-pc-u-boot.dtsi
>   create mode 100644 board/firefly/roc-pc-rk3588s/Kconfig
>   create mode 100644 board/firefly/roc-pc-rk3588s/MAINTAINERS
>   create mode 100644 configs/roc-pc-rk3588s_defconfig
>   create mode 100644 include/configs/roc-pc-rk3588s.h
> 
> diff --git a/arch/arm/dts/rk3588s-roc-pc-u-boot.dtsi b/arch/arm/dts/rk3588s-roc-pc-u-boot.dtsi
> new file mode 100644
> index 000000000000..e2ed7ce6232b
> --- /dev/null
> +++ b/arch/arm/dts/rk3588s-roc-pc-u-boot.dtsi
> @@ -0,0 +1,7 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +#include "rk3588s-u-boot.dtsi"
> +
> +&usbdp_phy0 {
> +	status = "okay";
> +};

Please send this upstream first and sync once it's available in 
devicetree-rebasing or justify why it needs to live in U-Boot only. We 
would like to have everything upstream for Rockchip DTSes (aside from 
the bootph- properties and some exceptions like binman nodes and other 
U-Boot-specific oddities).

> diff --git a/arch/arm/mach-rockchip/rk3588/Kconfig b/arch/arm/mach-rockchip/rk3588/Kconfig
> index 675f13ab66d7..841ae490e85a 100644
> --- a/arch/arm/mach-rockchip/rk3588/Kconfig
> +++ b/arch/arm/mach-rockchip/rk3588/Kconfig
> @@ -208,6 +208,12 @@ config TARGET_RK3588_NEU6
>   	  IO board and Neu6a needs to mount on top of this IO board in order to
>   	  create complete Edgeble Neural Compute Module 6B(Neu6B) IO platform.
>   
> +config TARGET_ROC_PC_RK3588S
> +	bool "Firefly ROC PC RK3588S"
> +	help
> +	  ROC-PC-RK3588S is a single board computer from Firefly
> +	  using the Rockchip RK3588S.
> +
>   config TARGET_ROCK5A_RK3588
>   	bool "Radxa ROCK5A RK3588 board"
>   	help
> @@ -421,6 +427,7 @@ config SYS_SOC
>   source "board/armsom/sige7-rk3588/Kconfig"
>   source "board/coolpi/genbook_cm5_rk3588/Kconfig"
>   source "board/edgeble/neural-compute-module-6/Kconfig"
> +source "board/firefly/roc-pc-rk3588s/Kconfig"
>   source "board/friendlyelec/cm3588-nas-rk3588/Kconfig"
>   source "board/friendlyelec/nanopc-t6-rk3588/Kconfig"
>   source "board/friendlyelec/nanopi-r6c-rk3588s/Kconfig"
> diff --git a/board/firefly/roc-pc-rk3588s/Kconfig b/board/firefly/roc-pc-rk3588s/Kconfig
> new file mode 100644
> index 000000000000..bded7a2fcba1
> --- /dev/null
> +++ b/board/firefly/roc-pc-rk3588s/Kconfig
> @@ -0,0 +1,12 @@
> +if TARGET_ROC_PC_RK3588S
> +
> +config SYS_BOARD
> +	default "roc-pc-rk3588s"
> +
> +config SYS_VENDOR
> +	default "firefly"
> +
> +config SYS_CONFIG_NAME
> +	default "roc-pc-rk3588s"
> +
> +endif
> diff --git a/board/firefly/roc-pc-rk3588s/MAINTAINERS b/board/firefly/roc-pc-rk3588s/MAINTAINERS
> new file mode 100644
> index 000000000000..9a2bb4e69e89
> --- /dev/null
> +++ b/board/firefly/roc-pc-rk3588s/MAINTAINERS
> @@ -0,0 +1,7 @@
> +ROC-PC-RK3588S
> +M:	Fabio Estevam <festevam@nabladev.com>
> +S:	Maintained
> +F:	arch/arm/dts/rk3588s-roc-pc-u-boot.dtsi
> +F:	board/firefly/roc-pc-rk3588s/
> +F:	configs/roc-pc-rk3588s_defconfig
> +F:	include/configs/roc-pc-rk3588s.h
> diff --git a/configs/roc-pc-rk3588s_defconfig b/configs/roc-pc-rk3588s_defconfig
> new file mode 100644
> index 000000000000..b625c93ad2d3
> --- /dev/null
> +++ b/configs/roc-pc-rk3588s_defconfig
> @@ -0,0 +1,91 @@
> +CONFIG_ARM=y
> +CONFIG_SKIP_LOWLEVEL_INIT=y
> +CONFIG_SYS_HAS_NONCACHED_MEMORY=y
> +CONFIG_COUNTER_FREQUENCY=24000000
> +CONFIG_ARCH_ROCKCHIP=y
> +CONFIG_ENV_OFFSET=0x400000

Historically, the environment has been stored at sector offset 8128, 
c.f. 
https://web.archive.org/web/20260501050119/https://opensource.rock-chips.com/wiki_Partitions. 
That is, it should be 0x3F8000. Sector offset 8192 is marked as 
reserved, so I guess you could store the redundant environment there, so 
have CONFIG_ENV_OFFSET_REDUND at 0x400000.

> +CONFIG_DEFAULT_DEVICE_TREE="rockchip/rk3588s-roc-pc"
> +CONFIG_ROCKCHIP_RK3588=y
> +CONFIG_SPL_SERIAL=y
> +CONFIG_TARGET_ROC_PC_RK3588S=y
> +CONFIG_SYS_LOAD_ADDR=0xc00800
> +CONFIG_DEBUG_UART_BASE=0xFEB50000
> +CONFIG_DEBUG_UART_CLOCK=24000000
> +CONFIG_ENV_OFFSET_REDUND=0x500000
> +CONFIG_PCI=y
> +CONFIG_DEBUG_UART=y
> +CONFIG_FIT=y
> +CONFIG_FIT_VERBOSE=y
> +CONFIG_SPL_FIT_SIGNATURE=y
> +CONFIG_SPL_LOAD_FIT=y
> +CONFIG_LEGACY_IMAGE_FORMAT=y
> +CONFIG_OF_SYSTEM_SETUP=y
> +CONFIG_DEFAULT_FDT_FILE="rockchip/rk3588s-roc-pc.dtb"
> +# CONFIG_DISPLAY_CPUINFO is not set
> +CONFIG_SPL_MAX_SIZE=0x40000
> +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
> +CONFIG_SPL_ATF=y
> +CONFIG_CMD_ADC=y
> +CONFIG_CMD_GPIO=y
> +CONFIG_CMD_GPT=y
> +CONFIG_CMD_MMC=y
> +CONFIG_CMD_PCI=y
> +CONFIG_CMD_USB=y
> +CONFIG_CMD_USB_MASS_STORAGE=y
> +# CONFIG_CMD_SETEXPR is not set
> +CONFIG_CMD_REGULATOR=y
> +# CONFIG_SPL_DOS_PARTITION is not set
> +CONFIG_SPL_OF_CONTROL=y
> +CONFIG_OF_LIVE=y
> +CONFIG_OF_SPL_REMOVE_PROPS="clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents"

Why do we need to remove clock-names in xPL DTB?

> +CONFIG_ENV_IS_IN_MMC=y
> +CONFIG_ENV_REDUNDANT=y
> +CONFIG_SPL_DM_SEQ_ALIAS=y
> +CONFIG_SPL_SYSCON=y
> +CONFIG_AHCI=y
> +CONFIG_SCSI_AHCI=y
> +CONFIG_AHCI_PCI=y
> +CONFIG_SPL_CLK=y
> +# CONFIG_USB_FUNCTION_FASTBOOT is not set
> +CONFIG_ROCKCHIP_GPIO=y
> +CONFIG_LED=y
> +CONFIG_LED_GPIO=y
> +CONFIG_MISC=y
> +CONFIG_SUPPORT_EMMC_RPMB=y
> +CONFIG_MMC_DW=y
> +CONFIG_MMC_DW_ROCKCHIP=y
> +CONFIG_MMC_SDHCI=y
> +CONFIG_MMC_SDHCI_SDMA=y
> +CONFIG_MMC_SDHCI_ROCKCHIP=y
> +CONFIG_DWC_ETH_QOS=y
> +CONFIG_DWC_ETH_QOS_ROCKCHIP=y
> +CONFIG_RTL8169=y

Is there really this PHY on the board? I know there's an RTL8211F, but I 
don't see an RTL8169 anywhere?

> +CONFIG_PHY_REALTEK=y
> +CONFIG_NVME_PCI=y
> +CONFIG_PCIE_DW_ROCKCHIP=y
> +CONFIG_PHY_ROCKCHIP_INNO_USB2=y
> +CONFIG_PHY_ROCKCHIP_NANENG_COMBOPHY=y
> +CONFIG_PHY_ROCKCHIP_USBDP=y

If we remove the usbdp_phy0 enabling in U-Boot DTS, then this can be 
removed too as nothing supported by this driver will be enabled.

Looks good to me otherwise, thanks!
Cheers,
Quentin