On 5/21/25 17:18, Quentin Schulz wrote:
Hi Michal,
On 5/21/25 4:34 PM, Michal Simek wrote:
On 5/12/25 09:06, Quentin Schulz wrote:
Hi Anand,
On 5/9/25 7:45 PM, Anand Moon wrote:
Hi Quentin,
On Fri, 9 May 2025 at 17:21, Quentin Schulz <quentin.sch...@cherry.de> wrote:
Hi Neil, Anand,
On 5/9/25 9:57 AM, Neil Armstrong wrote:
On 09/05/2025 09:02, Anand Moon wrote:
Add conditional compilation for the usb5744_i2c_init() function based
on the CONFIG_DM_I2C configuration, to avoid compilation failure.
CC net/net-common.o
AR net/built-in.o
LDS u-boot.lds
LD u-boot
aarch64-linux-gnu-ld.bfd: common/usb_onboard_hub.o: in function
`usb5744_i2c_init':
/home/amoon/mainline/u-boot/amlogic/u-boot-maineline/common/
usb_onboard_hub.c:74:(.text.usb5744_i2c_init+0xfc): undefined
reference to `i2c_get_chip'
aarch64-linux-gnu-ld.bfd: /home/amoon/mainline/u-boot/amlogic/u-boot-
maineline/common/usb_onboard_hub.c:89:(.text.usb5744_i2c_init+0x1a0):
undefined reference to `dm_i2c_write'
aarch64-linux-gnu-ld.bfd: /home/amoon/mainline/u-boot/amlogic/u-boot-
maineline/common/usb_onboard_hub.c:96:(.text.usb5744_i2c_init+0x1c8):
undefined reference to `dm_i2c_write'
aarch64-linux-gnu-ld.bfd: /home/amoon/mainline/u-boot/amlogic/u-boot-
maineline/common/usb_onboard_hub.c:104: (.text.usb5744_i2c_init+0x1f0):
undefined reference to `dm_i2c_write'
Segmentation fault (core dumped)
make: *** [Makefile:1824: u-boot] Error 139
make: *** Deleting file 'u-boot'
Signed-off-by: Anand Moon <linux.am...@gmail.com>
---
common/usb_onboard_hub.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c
index 1242513c631..92e3f3a92c9 100644
--- a/common/usb_onboard_hub.c
+++ b/common/usb_onboard_hub.c
@@ -30,6 +30,7 @@ struct onboard_hub_data {
int (*init)(struct udevice *dev);
};
+#if CONFIG_IS_ENABLED(DM_I2C)
static int usb5744_i2c_init(struct udevice *dev)
{
/*
@@ -109,6 +110,7 @@ static int usb5744_i2c_init(struct udevice *dev)
return 0;
}
+#endif
int usb_onboard_hub_reset(struct udevice *dev)
{
@@ -222,7 +224,9 @@ static const struct onboard_hub_data usb2514_data = {
};
static const struct onboard_hub_data usb5744_data = {
+#if CONFIG_IS_ENABLED(DM_I2C)
.init = usb5744_i2c_init,
+#endif
.power_on_delay_us = 1000,
.reset_us = 5,
};
I think you should completely disable usb5744_data in this case
I would recommend to go the same route as the Linux kernel which has a
special Kconfig option for it, c.f. USB_ONBOARD_DEV_USB5744 which adds
the necessary dependencies.
On Linux kernel, it needs to parse the device node with i2c-bus.
Seems like it does in U-Boot too? c.f. usb5744_i2c_init() with
dev_read_phandle_with_args with i2c-bus?
We probably should be careful and figure out which boards are actually
using the usb5744 and add this symbol to their defconfig so they don't
regress.
From my initial analysis configs/xilinx_zynqmp_kria_defconfig
which use CONFIG_USB_ONBOARD_HUB
I believe using DM_I2C to guard the code is the best approach.
it should be around .init() callback to guard this to work.
I disagree. The driver clearly supports the USB5744 and the driver is
compiled regardless of DM_I2C support which is required (as far as I
understood, maybe that's incorrect?) for being able to use USB5744? I don't
like hidden dependencies.
Also, I believe Neil's point is we shouldn't try to probe the USB5744 if
there is no DM_I2C support, simply excluding .init() callback will probably
just render this "support" of USB5744 non-functional?
USB5744 can be configured differently. We are using two configurations.
1. no initialization over i2c
(but still you can have a need to have driver for reset or power)
Fair enough, some hubs are configured "properly" by default so don't necessarily
need an interface. Such is the case for the Cypress HX3 we use on RK3399 Puma
for example. The default is enough so we don't need the i2c interface even
though it's exposed by the IC. Thanks for the reminder.
2. initialization over i2c to start the hub
And there is also one more configuration via SPI.
It means I don't think DM_I2C should be real dependency on getting usb5744 to
operate.
Would diverge from how it's handled in the kernel (today) though.
Can we please follow the same logic as in the kernel though? Meaning we should
report some message if the i2c-bus property is present but DM_I2C is not
enabled, in which case it's likely a misconfiguration of the bootloader.
Make sense.
M
