Hi Fabio,
On 8/12/26 1:01 AM, Fabio Estevam wrote:
Hi Quentin,
On Tue, Aug 11, 2026 at 8:55 AM Quentin Schulz <[email protected]> wrote:
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.
DM_I2C is also selected by roc-pc-mezzanine-rk3399_defconfig.
Yes, but it's missing SPL_DM_I2C.
We can use SPL_I2C to distinguish, like this:
Unfortunately, we cannot simply rely on SPL_I2C as the function is only
implemented in i2c-uclass.c which is only compiled when SPL_DM_I2C (and
SPL_I2C) is enabled.
There's also an issue in the dependencies for SPL_DM_I2C and TPL_DM_I2C
as they both depend on I2C (via the if at the top of
drivers/i2c/Kconfig) and DM_I2C while they should depend on SPL_I2C
(respectively TPL_I2C). I'm not sure why they depend on DM_I2C as it's
for proper, not xPL...
TPL_SYS_I2C_LEGACY depends on !SPL_DM_I2C but I'm assuming it should
rather depend on !TPL_DM_I2C.
Same for SPL_DM_I2C_GPIO, not sure why it depends on DM_I2C_GPIO.
SPL_I2C_MUX also depends on I2C_MUX, for what reason?
VPL_DM_I2C should depend on VPL_I2C but it doesn't exist, so we probably
should just remove the symbol since it won't do anything as is.
None are a blocker for this series.
--- a/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c
+++ b/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c
@@ -68,6 +68,9 @@ static bool is_roc_pc_plus(void)
{
struct udevice *bus, *dev;
+ /* roc-pc-mezzanine-rk3399_defconfig does not select SPL_I2C */
+ if (!CONFIG_IS_ENABLED(SPL_I2C))
+ return false;
Please add a newline after the return false;
It should be !CONFIG_IS_ENABLED(DM_I2C) as CONFIG_IS_ENABLED is going to
add the appropriate prefix based on the current stage being built (so
definitely no SPL/TPL/VPL to be added manually) and CONFIG_SPL_I2C is
not enough to build the dm_i2c_probe implementation, we need
CONFIG_SPL_DM_I2C (well, and CONFIG_SPL_I2C because otherwise the
drivers/i2c directory isn't traversed and because SPL_DM_I2C is missing
a dependency on it).
Cheers,
Quentin
