Re: [PATCH v4 3/4] rockchip: rk3399: Add ROC-PC-PLUS board detection

Hi Fabio,

On 7/22/26 9:46 PM, Fabio Estevam wrote:
> From: Fabio Estevam <[email protected]>
> 
> The ROC-RK3399-PC has an MP8859 regulator on I2C7 at address 0x66,
> while the ROC-RK3399-PC-PLUS does not. Probe for the regulator in SPL
> and use the result to select the matching devicetree from the U-Boot
> FIT. Fall back to the original board if the I2C bus cannot be probed.
> 
> Build both devicetrees from the existing roc-pc-rk3399_defconfig and
> enable the XMC SPI NOR driver used by the Plus variant. Set fdtfile
> from the selected U-Boot devicetree so the matching Linux devicetree
> is used as well.
> 
> Signed-off-by: Fabio Estevam <[email protected]>
> ---
> Changes since v3:
> - Explicitly sets fdtfile for both PC and PC-PLUS.
> 
>   arch/arm/dts/rk3399-roc-pc-plus-u-boot.dtsi |  3 ++
>   arch/arm/dts/rk3399-roc-pc-u-boot.dtsi      |  8 +++++
>   board/firefly/roc-pc-rk3399/roc-pc-rk3399.c | 39 +++++++++++++++++++++
>   configs/roc-pc-rk3399_defconfig             |  3 ++
>   doc/board/rockchip/rockchip.rst             |  2 +-
>   5 files changed, 54 insertions(+), 1 deletion(-)
>   create mode 100644 arch/arm/dts/rk3399-roc-pc-plus-u-boot.dtsi
> 
> diff --git a/arch/arm/dts/rk3399-roc-pc-plus-u-boot.dtsi b/arch/arm/dts/rk3399-roc-pc-plus-u-boot.dtsi
> new file mode 100644
> index 000000000000..c80529a62b97
> --- /dev/null
> +++ b/arch/arm/dts/rk3399-roc-pc-plus-u-boot.dtsi
> @@ -0,0 +1,3 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include "rk3399-roc-pc-u-boot.dtsi"
> diff --git a/arch/arm/dts/rk3399-roc-pc-u-boot.dtsi b/arch/arm/dts/rk3399-roc-pc-u-boot.dtsi
> index a85e9549c83e..e7e4a2c89072 100644
> --- a/arch/arm/dts/rk3399-roc-pc-u-boot.dtsi
> +++ b/arch/arm/dts/rk3399-roc-pc-u-boot.dtsi
> @@ -12,6 +12,14 @@
>   	};
>   };
>   
> +&i2c7 {
> +	bootph-pre-ram;
> +};
> +
> +&i2c7_xfer {
> +	bootph-pre-ram;
> +};
> +
>   &gpio4 {
>   	bootph-pre-ram;
>   };
> diff --git a/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c b/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c
> index 6937a27176f9..a12180e65573 100644
> --- a/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c
> +++ b/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c
> @@ -5,14 +5,24 @@
>   
>   #include <dm.h>
>   #include <env.h>
> +#include <fdtdec.h>
> +#include <i2c.h>
> +#include <image.h>
>   #include <log.h>
>   #include <spl_gpio.h>
> +#include <asm/global_data.h>
>   #include <asm/io.h>
>   
>   #include <asm/arch-rockchip/cru.h>
>   #include <asm/arch-rockchip/gpio.h>
>   #include <asm/arch-rockchip/grf_rk3399.h>
>   
> +#define ROC_PC_MP8859_BUS	"i2c@ff160000"
> +#define ROC_PC_MP8859_ADDR	0x66
> +#define ROC_PC_PLUS_FDTFILE	"rockchip/rk3399-roc-pc-plus.dtb"
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
>   #ifdef CONFIG_XPL_BUILD
>   
>   #define PMUGRF_BASE	0xff320000
> @@ -54,4 +64,33 @@ void led_setup(void)
>   	spl_gpio_output(gpio0, GPIO(BANK_B, 5), 1);
>   }
>   
> +static bool is_roc_pc_plus(void)
> +{
> +	struct udevice *bus, *dev;
> +
> +	if (uclass_get_device_by_name(UCLASS_I2C, ROC_PC_MP8859_BUS, &bus))
> +		return false;
> +
> +	return dm_i2c_probe(bus, ROC_PC_MP8859_ADDR, 0, &dev);

This fails to build due to undefined reference to dm_i2c_probe in SPL:

+Some images are invalid
    aarch64:  +   roc-pc-mezzanine-rk3399
+aarch64-linux-ld: board/firefly/roc-pc-rk3399/roc-pc-rk3399.o: in 
function `is_roc_pc_plus':
+board/firefly/roc-pc-rk3399/roc-pc-rk3399.c:74:(.text.board_fit_config_name_match+0x38): 
undefined reference to `dm_i2c_probe'
+make[2]: *** [scripts/Makefile.xpl:546: spl/u-boot-spl] Error 1
+make[1]: *** [Makefile:2448: spl/u-boot-spl] Error 2
+make: *** [Makefile:189: __sub-make] Error 2

c.f. 
https://git.u-boot-project.org/u-boot/custodians/u-boot-rockchip/-/jobs/67143#L592

Note this applies to the roc-pc-mezzanine-rk3399 defconfig which is only 
slightly different from roc-pc-rk3399.

Ideally, we should be able to automatically detect whether it's a 
roc-pc-mezzanine-rk3399, but I couldn't find schematics for it and from 
the Device Tree, there doesn't seem to be something we could use to 
detect it like we do for ROC-PC-PLUS/ROC-PC. So instead I recommend to 
check for CONFIG_IS_ENABLED(DM_I2C). If it ever gets enabled for 
roc-pc-mezzanine-rk3399_defconfig, the logic will return that it's NOT a 
ROC-PC-PLUS and thus use CONFIG_DEFAULT_FDT_FILE which is 
rockchip/rk3399-roc-pc-mezzanine.dtb which is correct.

What do you think? Can you send a v5 for this? Please pick Takahiro's 
R-b and the Link that was suggested to be added to the commit log of 
patch 1.

Thanks!
Quentin