On Tue, Aug 25, 2026 at 01:40:44AM +0200, Marek Vasut wrote: > On 8/24/26 8:29 PM, 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 resolving the index against the child node's clk and ensure to > > enable the clock before using it. > > > > 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 | 24 ++++++++++++++++++++++-- > > 1 file changed, 22 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/dwc3/dwc3-generic.c > > b/drivers/usb/dwc3/dwc3-generic.c > > index 2356b3bc0aa..e16d236bd9d 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 { > > @@ -76,9 +77,28 @@ static int dwc3_generic_probe(struct udevice *dev, > > index = ofnode_stringlist_search(node, "clock-names", "ref"); > > if (index < 0) > > index = ofnode_stringlist_search(node, "clock-names", > > "ref_clk"); > > - } > > - if (index >= 0) > > + /* > > + * glue->clks was populated from the parent glue node's own > > + * clock-names list, which can differ in length/order from > > + * this child node's list (e.g. i.MX95: glue has "hsio", > > + * "suspend", the dwc3 child has "bus_early", "ref", > > + * "suspend"). Reusing this index against glue->clks would > > + * silently resolve to the wrong clock, so fetch it directly > > + * from this node instead. > > + */ > > + if (index >= 0) { > > + if (!clk_get_by_index(dev, index, &priv->ref_clk)) { > > Would it be possible to call clk_get_by_name(dev, ...) and > clk_get_by_name(dev->parent, ...) instead, until one of them succeeds ? That > could make the code simpler. >
You are right, I try to rework that in the next version. Thanks for the suggestion. I will also remove/replace the debug() calls. Regards, Stefan
