On Mon, Sep 14, 2026 at 09:26:12PM +0200, Marek Vasut wrote: > On 9/14/26 9:42 AM, Stefan Eichenberger wrote: > > From: Stefan Eichenberger <[email protected]> > > > > On Verdin iMX95, with a USB hub permanently attached to the DWC3 host > > port the hub itself enumerates fine but a USB stick plugged into it > > fails intermittently: > > > > Verdin iMX95 # usb start > > starting USB... > > USB XHCI 1.10 > > Device not responding to set address. > > > > USB device not accepting new address (error=80000000) > > Bus usb@4c100000: 2 USB Device(s) found > > scanning usb for storage devices... 0 Storage Device(s) found > > Verdin iMX95 # usb reset > > resetting USB... > > USB XHCI 1.10 > > WARN endpoint is halted > > WARN endpoint is halted > > unable to get device descriptor (error=-22) > > > > Commit c6583354b7c1 ("usb: dwc3: Look up reference clock DT phandle in > > both controller DT nodes") added a fallback that searches the DWC3 > > child node's "clock-names" for "ref"/"ref_clk" when it isn't found on > > the parent glue node, but always resolves the found index against > > glue->clks, a clk_bulk populated only from the parent glue node's own > > clock-names list. This is wrong since the child node's clock-names list > > is not guaranteed to be in the same order as the parent's, and indeed on > > Verdin iMX95 the child node has a different order than the parent node. > > > > Fix it by looking the reference clock up by name with clk_get_by_name(), > > walking the dwc3 device first and then its parent glue device, so the > > clock is always resolved through the device that owns the node its name > > was found in. Also enable the clock before use, the child node's clocks > > are not part of the glue node's clk_bulk and are therefore not enabled > > by dwc3_glue_clk_init(), and disable it again in dwc3_generic_remove() > > to keep the enable balanced. > > > > Fixes: c6583354b7c1 ("usb: dwc3: Look up reference clock DT phandle in both > > controller DT nodes") > > Signed-off-by: Stefan Eichenberger <[email protected]> > > --- > > drivers/usb/dwc3/dwc3-generic.c | 47 ++++++++++++++++++++++----------- > > 1 file changed, 31 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/usb/dwc3/dwc3-generic.c > > b/drivers/usb/dwc3/dwc3-generic.c > > index 2356b3bc0aa..704dd242553 100644 > > --- a/drivers/usb/dwc3/dwc3-generic.c > > +++ b/drivers/usb/dwc3/dwc3-generic.c > > @@ -31,6 +31,7 @@ struct dwc3_generic_priv { > > struct dwc3 dwc3; > > struct phy_bulk phys; > > struct gpio_desc *ulpi_reset; > > + struct clk ref_clk; > > }; > > struct dwc3_generic_host_priv { > > @@ -47,8 +48,8 @@ static int dwc3_generic_probe(struct udevice *dev, > > struct dwc3_generic_plat *plat = dev_get_plat(dev); > > struct dwc3 *dwc3 = &priv->dwc3; > > struct dwc3_glue_data *glue = dev_get_plat(dev->parent); > > - int __maybe_unused index; > > - ofnode __maybe_unused node; > > + struct udevice * const __maybe_unused clk_devs[] = { dev, dev->parent }; > > + int __maybe_unused i; > > dwc3->dev = dev; > > dwc3->maximum_speed = plat->maximum_speed; > > @@ -63,22 +64,31 @@ static int dwc3_generic_probe(struct udevice *dev, > > * - in top level generic node, with no subnode (i.MX8MQ) > > * - in generic subnode, with other clock in top level node (i.MX8MP) > > * - in both top level node and generic subnode (Rockchip) > > - * Cover all the possibilities here by looking into both nodes, start > > - * with the top level node as that seems to be used in majority of DTs > > - * to reference the clock. > > + * Cover all the possibilities here by looking into both nodes, the > > + * generic node first and the top level glue node second. The clock > > + * must be looked up on the device that owns the node it is described > > + * in, the clock-names lists of the two nodes are unrelated (e.g. on > > + * i.MX95 the glue node has "hsio", "suspend" while the generic > > + * subnode has "bus_early", "ref", "suspend"). > > */ > > - node = dev_ofnode(dev->parent); > > - index = ofnode_stringlist_search(node, "clock-names", "ref"); > > - if (index < 0) > > - index = ofnode_stringlist_search(node, "clock-names", > > "ref_clk"); > > - if (index < 0) { > > - node = dev_ofnode(dev); > > - index = ofnode_stringlist_search(node, "clock-names", "ref"); > > - if (index < 0) > > - index = ofnode_stringlist_search(node, "clock-names", > > "ref_clk"); > > + for (i = 0; i < ARRAY_SIZE(clk_devs); i++) { > > Do we need a loop for two device pointers, where we are basically sure that > there won't ever be any more device pointers in the list ? (I think no, so > please use four clk_get_by_name() calls and be done with it) > > Also, please send this patch separately, so it can be picked as a bugfix for > 2026.10 . > > Thank you.
My fault, I somehow missunderstood the last comment and thought I should create a loop. I will change that and send it as a separate patch. Thanks for the review. Stefan
