[PATCH] usb: dwc3: am62: handle usb2_refclk enable failures

[PATCH] usb: dwc3: am62: handle usb2_refclk enable failures

[Votokina Victoria]

The AM62 DWC3 glue enables usb2_refclk via clk_prepare_enable(), but
the return value was ignored in the init path. If the clock cannot be
enabled (including -EPROBE_DEFER), the driver continues and may access
the controller/PHY in an undefined state.

Also check dwc3_ti_init() return value on resume when power/context was
lost.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 31d500c2d0d4 ("usb: dwc3: dwc3-am62: Re-initialize controller if lost power in PM suspend")
Signed-off-by: Votokina Victoria <Victoria.Votokina@kaspersky.com>
---
 drivers/usb/dwc3/dwc3-am62.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c
index e11d7643f966..d8b35098e921 100644
--- a/drivers/usb/dwc3/dwc3-am62.c
+++ b/drivers/usb/dwc3/dwc3-am62.c
@@ -205,7 +205,9 @@ static int dwc3_ti_init(struct dwc3_am62 *am62)
 
 	dwc3_ti_writel(am62, USBSS_PHY_CONFIG, reg);
 
-	clk_prepare_enable(am62->usb2_refclk);
+	ret = clk_prepare_enable(am62->usb2_refclk);
+	if (ret)
+		return dev_err_probe(am62->dev, ret, "failed to enable usb2_refclk\n");
 
 	/* Set mode valid bit to indicate role is valid */
 	reg = dwc3_ti_readl(am62, USBSS_MODE_CONTROL);
@@ -361,14 +363,17 @@ static int dwc3_ti_resume_common(struct device *dev)
 {
 	struct dwc3_am62 *am62 = dev_get_drvdata(dev);
 	u32 reg;
+	int ret;
 
 	reg = dwc3_ti_readl(am62, USBSS_DEBUG_CFG);
 	if (reg != USBSS_DEBUG_CFG_DISABLED) {
 		/* lost power/context */
-		dwc3_ti_init(am62);
+		return dwc3_ti_init(am62);
 	} else {
+		ret = clk_prepare_enable(am62->usb2_refclk);
+		if (ret)
+			return ret;
 		dwc3_ti_writel(am62, USBSS_DEBUG_CFG, USBSS_DEBUG_CFG_OFF);
-		clk_prepare_enable(am62->usb2_refclk);
 	}
 
 	if (device_may_wakeup(dev)) {
-- 
2.43.0

Re: [PATCH] usb: dwc3: am62: handle usb2_refclk enable failures

[Thinh Nguyen]

On Wed, Feb 04, 2026, Votokina Victoria wrote:
> The AM62 DWC3 glue enables usb2_refclk via clk_prepare_enable(), but
> the return value was ignored in the init path. If the clock cannot be
> enabled (including -EPROBE_DEFER), the driver continues and may access
> the controller/PHY in an undefined state.

What's the actual error here that you encountered? Or is this a
hypothesis/static check?

> 
> Also check dwc3_ti_init() return value on resume when power/context was
> lost.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 31d500c2d0d4 ("usb: dwc3: dwc3-am62: Re-initialize controller if lost power in PM suspend")
> Signed-off-by: Votokina Victoria <Victoria.Votokina@kaspersky.com>
> ---
>  drivers/usb/dwc3/dwc3-am62.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c
> index e11d7643f966..d8b35098e921 100644
> --- a/drivers/usb/dwc3/dwc3-am62.c
> +++ b/drivers/usb/dwc3/dwc3-am62.c
> @@ -205,7 +205,9 @@ static int dwc3_ti_init(struct dwc3_am62 *am62)
>  
>  	dwc3_ti_writel(am62, USBSS_PHY_CONFIG, reg);
>  
> -	clk_prepare_enable(am62->usb2_refclk);
> +	ret = clk_prepare_enable(am62->usb2_refclk);
> +	if (ret)
> +		return dev_err_probe(am62->dev, ret, "failed to enable usb2_refclk\n");
>  
>  	/* Set mode valid bit to indicate role is valid */
>  	reg = dwc3_ti_readl(am62, USBSS_MODE_CONTROL);
> @@ -361,14 +363,17 @@ static int dwc3_ti_resume_common(struct device *dev)
>  {
>  	struct dwc3_am62 *am62 = dev_get_drvdata(dev);
>  	u32 reg;
> +	int ret;
>  
>  	reg = dwc3_ti_readl(am62, USBSS_DEBUG_CFG);
>  	if (reg != USBSS_DEBUG_CFG_DISABLED) {
>  		/* lost power/context */
> -		dwc3_ti_init(am62);
> +		return dwc3_ti_init(am62);

This changes the current logic. This is wrong. How did you test this.

>  	} else {
> +		ret = clk_prepare_enable(am62->usb2_refclk);
> +		if (ret)
> +			return ret;
>  		dwc3_ti_writel(am62, USBSS_DEBUG_CFG, USBSS_DEBUG_CFG_OFF);
> -		clk_prepare_enable(am62->usb2_refclk);

This does more than what's described in the commit. Please explain.

BR,
Thinh

>  	}
>  
>  	if (device_may_wakeup(dev)) {
> -- 
> 2.43.0
> 

Re: [PATCH] usb: dwc3: am62: handle usb2_refclk enable failures

[Votokina Victoria]

Thinh,

This was reported by static analysis (SVACE), not from a runtime
failure I could reproduce. The analyzer flagged that
clk_prepare_enable() return value is ignored in dwc3_ti_init().
Since clk_prepare_enable() can legitimately fail (incl. -EPROBE_DEFER)
and the driver continues to touch controller/PHY registers afterwards,
this can lead to operating the IP in an undefined state.
Many drivers treat “enable clock failed” as a hard error/deferral
and bail out; that’s why the checker considers the pattern suspicious.

Regarding dwc3_ti_resume_common(): you’re right — returning
dwc3_ti_init() directly changes the current logic and is not correct
as-is. The intent was only to check and propagate the error from
dwc3_ti_init() when power/context was lost, while keeping the remaining
resume steps (e.g. wakeup handling) intact. 
I’ll respin this so it becomes:

ret = dwc3_ti_init(am62); if (ret) return ret; 
and then continue with the existing flow.

I also suggest enabling usb2_refclk before touching USBSS_DEBUG_CFG.
Accessing USBSS/DWC3 registers assumes the refclk is active;
if clk_prepare_enable() fails (e.g. -EPROBE_DEFER), returning early
avoids partially programming the block while clocks are off.
This follows the usual “clock first, registers second” ordering used
across drivers.

I tested compile/build only; I don’t have AM62 hardware to validate
runtime behavior. If you prefer, I can drop any changes beyond 
return-value checking and keep the patch minimal.

Thanks,
Victoria

Re: [PATCH] usb: dwc3: am62: handle usb2_refclk enable failures

[Greg KH]

On Tue, Feb 10, 2026 at 12:36:59PM +0300, Votokina Victoria wrote:
> Thinh,
> 
> This was reported by static analysis (SVACE), not from a runtime
> failure I could reproduce.

Sorry, but what is "this"?

I see no context in this email at all.  Always properly quote your
emails (by not doing top-posting), as remember, many of us get over 1000
emails a day and can not naturally keep them all in their inbox.

thanks,

greg k-h

Re: [PATCH] usb: dwc3: am62: handle usb2_refclk enable failures

[Thinh Nguyen]

On Tue, Feb 10, 2026, Votokina Victoria wrote:
> Thinh,
> 
> This was reported by static analysis (SVACE), not from a runtime
> failure I could reproduce. The analyzer flagged that
> clk_prepare_enable() return value is ignored in dwc3_ti_init().
> Since clk_prepare_enable() can legitimately fail (incl. -EPROBE_DEFER)

Can clk_prepare_enable() actually return -EPROBE_DEFER in this context?
Have you verified the AM62 platform implementation to determine if there
are any dependencies that could cause deferral even after successfully
obtaining the refclk handle? What other error codes might be returned?

> and the driver continues to touch controller/PHY registers afterwards,
> this can lead to operating the IP in an undefined state.
> Many drivers treat “enable clock failed” as a hard error/deferral
> and bail out; that’s why the checker considers the pattern suspicious.

Suspicion alone is insufficient justification. I need you to provide a
definitive analysis of the possible error conditions by auditing the
AM62 platform's clk_prepare_enable() implementation.

> 
> Regarding dwc3_ti_resume_common(): you’re right — returning
> dwc3_ti_init() directly changes the current logic and is not correct
> as-is. The intent was only to check and propagate the error from
> dwc3_ti_init() when power/context was lost, while keeping the remaining
> resume steps (e.g. wakeup handling) intact. 
> I’ll respin this so it becomes:
> 
> ret = dwc3_ti_init(am62); if (ret) return ret; 
> and then continue with the existing flow.
> 
> I also suggest enabling usb2_refclk before touching USBSS_DEBUG_CFG.
> Accessing USBSS/DWC3 registers assumes the refclk is active;
> if clk_prepare_enable() fails (e.g. -EPROBE_DEFER), returning early
> avoids partially programming the block while clocks are off.
> This follows the usual “clock first, registers second” ordering used
> across drivers.
> 
> I tested compile/build only; I don’t have AM62 hardware to validate
> runtime behavior. If you prefer, I can drop any changes beyond 
> return-value checking and keep the patch minimal.
> 

For submissions based solely on compilation testing, I cannot accept
changes founded on incomplete analysis, especially patches carrying a
"Fixes" tag, which require thorough investigation and concrete evidence
of the issue being addressed.

BR,
Thinh