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

Hi Fabio,

On 8/13/26 1:44 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]>
> Reviewed-by: Quentin Schulz <[email protected]>
> ---
> Changes since v4:
> - Fixed mezzanine build error.
> 
>   arch/arm/dts/rk3399-roc-pc-u-boot.dtsi      |  8 ++++
>   board/firefly/roc-pc-rk3399/roc-pc-rk3399.c | 42 +++++++++++++++++++++
>   configs/roc-pc-rk3399_defconfig             |  3 ++
>   doc/board/rockchip/rockchip.rst             |  2 +-
>   4 files changed, 54 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/dts/rk3399-roc-pc-u-boot.dtsi b/arch/arm/dts/rk3399-roc-pc-u-boot.dtsi
> index a85e9549c83..e7e4a2c8907 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 6937a27176f..8f54e526c19 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,36 @@ 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 (!(CONFIG_IS_ENABLED(I2C) && CONFIG_IS_ENABLED(DM_I2C)))
> +		return false;
> +
> +	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);

Between v4 and v5, the exclamation point now inverts the result of 
dm_i2c_probe and I believe this is incorrect.

dm_i2c_probe returns 0 if a chip was found, -ve otherwise. The 
ROC-RK3399-PC-PLUS does NOT have this chip, therefore is_roc_pc_plus() 
should return false (0) when the chip is NOT detected, meaning we should 
NOT invert dm_i2c_probe(). Is this correct?

Cheers,
Quentin