Re: [PATCH] usb: usb3503: Propagate resume errors and unwind the clock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] usb: usb3503: unbalanced clk_disable_unprepare() after
  usb3503_resume() error
--

commit 76ce1fbd04c6d5d5b3ad3db2594ab4f0c33cb352
Author: Zhu Jun <zhujun2@cmss.chinamobile.com>

usb: usb3503: Propagate resume errors and unwind the clock

This commit checks for errors from clk_prepare_enable() and
usb3503_switch_mode() in usb3503_resume(), disabling and unpreparing
hub->clk if restoring the mode fails.

> diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c
> index 9f356b4bd84e3..bcc6174b80d1d 100644
> --- a/drivers/usb/misc/usb3503.c
> +++ b/drivers/usb/misc/usb3503.c
> @@ -353,10 +353,17 @@ static int __maybe_unused usb3503_suspend(struct usb3503 *hub)
>  
>  static int __maybe_unused usb3503_resume(struct usb3503 *hub)
>  {
> -	clk_prepare_enable(hub->clk);
> -	usb3503_switch_mode(hub, hub->mode);
> +	int ret;
>  
> -	return 0;
> +	ret = clk_prepare_enable(hub->clk);
> +	if (ret)
> +		return ret;
> +
> +	ret = usb3503_switch_mode(hub, hub->mode);
> +	if (ret)
> +		clk_disable_unprepare(hub->clk);
> +
> +	return ret;
>  }

[Severity: Medium]
Can calling clk_disable_unprepare(hub->clk) on usb3503_switch_mode()
failure cause a clock reference count underflow on the next suspend or
driver removal?

During suspend, usb3503_suspend() disables and unprepares hub->clk.
When device_resume() runs, the PM core clears dev->power.is_suspended and
moves the device back to the active DPM list even if the driver's resume
callback returns an error.  Because usb3503_suspend(),
usb3503_i2c_remove(), and usb3503_platform_remove() all call
clk_disable_unprepare(hub->clk) unconditionally without tracking whether
the clock is enabled, disabling hub->clk here (or failing in
clk_prepare_enable()) causes the next suspend or driver unbind to call
clk_disable_unprepare(hub->clk) on an already disabled clock.

In addition, when usb3503_switch_mode() is called with USB3503_MODE_HUB
and fails inside usb3503_connect(), it has already deasserted hub->reset
and cleared hub->bypass:

drivers/usb/misc/usb3503.c:usb3503_switch_mode() {
	...
	if (hub->reset)
		gpiod_set_value_cansleep(hub->reset, rst);

	if (hub->bypass)
		gpiod_set_value_cansleep(hub->bypass, bypass);

	if (conn) {
		/* Wait T_HUBINIT == 4ms for hub logic to stabilize */
		usleep_range(4000, 10000);
		return usb3503_connect(hub);
	}
	...
}

Gating hub->clk without switching the hardware back to standby leaves the
chip out of reset without its reference clock.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112071723.154558-1-zhujun2@cmss.chinamobile.com?part=1