Re: [PATCH v1 2/6] usb: dwc3-generic: fix ref clock lookup indexing wrong clk_bulk array

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