Re: [PATCH 3/3] usb: dwc3: remove redundant pm_runtime_dont_use_autosuspend() calls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] usb: dwc3: autosuspend left enabled on dwc3_core_probe() runtime
  error
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [High] usb: dwc3-am62: unchecked pm_runtime_get_sync() in
  dwc3_ti_remove() [1]
--

commit ff359600a9a87664827e5cdde0c2f86b3442fcb7
Author: Joshua Crofts <crofts@kernel.org>

usb: dwc3: remove redundant pm_runtime_dont_use_autosuspend() calls

This commit removes pm_runtime_dont_use_autosuspend() calls from the error
and removal paths in dwc3_core_probe(), dwc3_core_remove(), and
dwc3_ti_remove().

> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index fd5c2cd36c595..c4f0aa9627477 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -2431,7 +2431,6 @@ int dwc3_core_probe(const struct dwc3_probe_data *data)
>  err_allow_rpm:
>  	pm_runtime_allow(dev);
>  	pm_runtime_disable(dev);
> -	pm_runtime_dont_use_autosuspend(dev);
>  	pm_runtime_set_suspended(dev);
>  	pm_runtime_put_noidle(dev);
>  err_disable_clks:

[Severity: Low]
Can this leave dev->power.use_autosuspend enabled if dwc3_core_probe()
fails at runtime without unbinding the driver?

In drivers/usb/dwc3/dwc3-apple.c, dwc3_apple_probe() can succeed in the
DWC3_APPLE_PROBE_PENDING state without calling dwc3_core_probe(), and later
invoke dwc3_usb_role_switch_set() -> dwc3_apple_init() ->
dwc3_apple_core_init() -> dwc3_apple_core_probe() -> dwc3_core_probe() when
a cable is connected.  If dwc3_core_probe() fails after enabling
autosuspend and unwinds via err_allow_rpm, the dwc3-apple driver remains
bound so pm_runtime_reinit() does not run, leaving autosuspend enabled in
sysfs and leaking the runtime PM usage reference if a negative delay was
configured.

[ ... ]

> diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c
> index d03b3950ce046..0df7e436aeb0d 100644
> --- a/drivers/usb/dwc3/dwc3-am62.c
> +++ b/drivers/usb/dwc3/dwc3-am62.c
> @@ -322,7 +322,6 @@ static void dwc3_ti_remove(struct platform_device *pdev)
>  
>  	pm_runtime_put_sync(dev);
>  	pm_runtime_disable(dev);
> -	pm_runtime_dont_use_autosuspend(dev);
>  	pm_runtime_set_suspended(dev);
>  }

[Severity: High]
This problem wasn't introduced by this patch, but can an unchecked
pm_runtime_get_sync() failure in dwc3_ti_remove() [1] cause a
synchronous external abort when accessing USBSS_MODE_CONTROL?

drivers/usb/dwc3/dwc3-am62.c:dwc3_ti_remove() {
	...
	pm_runtime_get_sync(dev);
	device_init_wakeup(dev, false);
	of_platform_depopulate(dev);

	/* Clear mode valid bit */
	reg = dwc3_ti_readl(am62, USBSS_MODE_CONTROL);
	...
}

If runtime resume fails during unbind, the device remains suspended with
its clocks gated while dwc3_ti_readl() and dwc3_ti_writel() access the
hardware registers.

[1] https://sashiko.dev/#/bug/linux-413fac67-663c-422c-bdfa-d64685df2b24

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106133933.1577-1-crofts@kernel.org?part=3