Hi Quentin,

pt., 16 maj 2025 o 16:28 Quentin Schulz <quentin.sch...@cherry.de> napisał(a):
>
> Hi Lukasz,
>
> On 4/25/25 12:56 PM, Lukasz Czechowski wrote:
> > Currently the check in usb_onboard_hub_bind is relying on specific
> > compatible string for the Michrochip USB5744. Replace this with
> > more generic approach that will allow to add new types of devices
> > to the of_match table. Because the driver only needs to bind one
> > "half" of the hub, the peer-hub node is used to find out if it
> > was already done. In case peer-hub was bound, -ENODEV is returned.
> >
> > Signed-off-by: Lukasz Czechowski <lukasz.czechow...@thaumatec.com>
> > ---
> >   common/usb_onboard_hub.c | 15 ++++++++++-----
> >   1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c
> > index 7fe62b043e6a..9f8c1a304cdf 100644
> > --- a/common/usb_onboard_hub.c
> > +++ b/common/usb_onboard_hub.c
> > @@ -9,6 +9,7 @@
> >
> >   #include <asm/gpio.h>
> >   #include <dm.h>
> > +#include <dm/device.h>
> >   #include <dm/device_compat.h>
> >   #include <i2c.h>
> >   #include <linux/delay.h>
> > @@ -179,8 +180,8 @@ err:
> >   static int usb_onboard_hub_bind(struct udevice *dev)
> >   {
> >       struct ofnode_phandle_args phandle;
> > -     const void *fdt = gd->fdt_blob;
> > -     int ret, off;
> > +     struct udevice *peerdev;
> > +     int ret;
> >
> >       ret = dev_read_phandle_with_args(dev, "peer-hub", NULL, 0, 0, 
> > &phandle);
> >       if (ret == -ENOENT) {
> > @@ -193,10 +194,14 @@ static int usb_onboard_hub_bind(struct udevice *dev)
> >               return ret;
> >       }
> >
> > -     off = ofnode_to_offset(phandle.node);
> > -     ret = fdt_node_check_compatible(fdt, off, "usb424,5744");
> > -     if (!ret)
> > +     ret = device_find_global_by_ofnode(phandle.node, &peerdev);
>
> Wondering if uclass_find_device_by_ofnode(UCLASS_USB_HUB, phandle.node,
> &peerdev) wouldn't be faster/more appropriate here?
>

Thanks for the remark. I'm wondering what are the differences between the
device_find_global_by_ofnode and uclass_find_device_by_ofnode, other than
the additional uclass_id parameter. Knowing that uclass_id is always
UCLASS_USB_HUB it might be good idea indeed to use the
uclass_find_device_by_ofnode that checks only devices within specific uclass.

> Looks good in any case so:
>
> Reviewed-by: Quentin Schulz <quentin.sch...@cherry.de>
>
> Just for general information (I assume specifically to Tom since he's
> the maintainer there), I would really appreciate this to be merged
> before we sync the kernel DT with 6.16 (if Heiko manages to squeeze one
> last merge request to Arnd before the cut-off date; or 6.17 failing
> that) otherwise RK3399 Puma will have USB completely broken (I assume we
> could have -u-boot.dtsi hacking the device tree to remove/add nodes to
> keep it as it is today, but better to not do it :) ).
>
> Timeline wise, I assume 2025.10 at the earliest, but maybe 2026.01? It
> depends when 6.16/6.17 will be released and/or if we take -rc1 :)
>
> Cheers,
> Quentin

Best regards,
Lukasz

Reply via email to