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

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

[Joshua Crofts]

Since commit d1706f9da9f4 ("PM: runtime: call pm_runtime_dont_use_autosuspend() on reinit()"),
driver core is now responsible for undoing any
pm_runtime_use_autosuspend() calls on driver exit.

Remove the redundant function calls.

Joshua Crofts (3):
  usb: chipidea: remove redundant pm_runtime_dont_use_autosuspend() call
  usb: musb: remove redundant pm_runtime_dont_use_autosuspend() calls
  usb: dwc3: remove redundant pm_runtime_dont_use_autosuspend() calls

 drivers/usb/chipidea/core.c  | 1 -
 drivers/usb/dwc3/core.c      | 2 --
 drivers/usb/dwc3/dwc3-am62.c | 1 -
 drivers/usb/musb/musb_core.c | 2 --
 4 files changed, 6 deletions(-)


base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.47.3

[PATCH 1/3] usb: chipidea: remove redundant pm_runtime_dont_use_autosuspend() call

[Joshua Crofts]

Since commit d1706f9da9f4 ("PM: runtime: call pm_runtime_dont_use_autosuspend() on reinit()"),
driver core is now responsible for undoing any
pm_runtime_use_autosuspend() calls on driver exit.

Remove the redundant function call.

Signed-off-by: Joshua Crofts <crofts@kernel.org>
---
 drivers/usb/chipidea/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 3461dd763a6e..7edc512cc37d 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -1250,7 +1250,6 @@ static void ci_hdrc_remove(struct platform_device *pdev)
 		usb_role_switch_unregister(ci->role_switch);
 
 	if (ci->supports_runtime_pm) {
-		pm_runtime_dont_use_autosuspend(&pdev->dev);
 		pm_runtime_get_sync(&pdev->dev);
 		pm_runtime_disable(&pdev->dev);
 		pm_runtime_put_noidle(&pdev->dev);
-- 
2.47.3

Re: [PATCH 1/3] usb: chipidea: remove redundant pm_runtime_dont_use_autosuspend() call

[sashiko-bot]

> Since commit d1706f9da9f4 ("PM: runtime: call pm_runtime_dont_use_autosuspend() on reinit()"),
> driver core is now responsible for undoing any
> pm_runtime_use_autosuspend() calls on driver exit.
> 
> Remove the redundant function call.
> 
> Signed-off-by: Joshua Crofts <crofts@kernel.org>

Sashiko has reviewed this patch and found no issues.

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

[PATCH 2/3] usb: musb: remove redundant pm_runtime_dont_use_autosuspend() calls

[Joshua Crofts]

Since commit d1706f9da9f4 ("PM: runtime: call pm_runtime_dont_use_autosuspend() on reinit()"),
driver core is now responsible for undoing any
pm_runtime_use_autosuspend() calls on driver exit.

Remove the redundant function calls.

Signed-off-by: Joshua Crofts <crofts@kernel.org>
---
 drivers/usb/musb/musb_core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 73ac25f53698..28e59b53fd02 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2576,7 +2576,6 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	usb_phy_shutdown(musb->xceiv);
 
 err_usb_phy_init:
-	pm_runtime_dont_use_autosuspend(musb->controller);
 	pm_runtime_put_sync(musb->controller);
 	pm_runtime_disable(musb->controller);
 
@@ -2644,7 +2643,6 @@ static void musb_remove(struct platform_device *pdev)
 	spin_unlock_irqrestore(&musb->lock, flags);
 	musb_platform_exit(musb);
 
-	pm_runtime_dont_use_autosuspend(musb->controller);
 	pm_runtime_put_sync(musb->controller);
 	pm_runtime_disable(musb->controller);
 	musb_phy_callback = NULL;
-- 
2.47.3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] usb: musb: use-after-free in musb_remove() [1]
--

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

usb: musb: remove redundant pm_runtime_dont_use_autosuspend() calls

This commit removes redundant pm_runtime_dont_use_autosuspend() calls in
musb_init_controller() and musb_remove() since the driver core now handles
this on driver exit.

> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 73ac25f53698f..28e59b53fd02f 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c

[ ... ]

> @@ -2644,7 +2643,6 @@ static void musb_remove(struct platform_device *pdev)
>  	spin_unlock_irqrestore(&musb->lock, flags);
>  	musb_platform_exit(musb);
>  
> -	pm_runtime_dont_use_autosuspend(musb->controller);
>  	pm_runtime_put_sync(musb->controller);
>  	pm_runtime_disable(musb->controller);
>  	musb_phy_callback = NULL;

[Severity: Critical]
This problem wasn't introduced by this patch, but can calling
musb_platform_exit(musb) before usb_phy_shutdown(musb->xceiv) a few lines
below cause a use-after-free on musb->xceiv [1]?

Platform exit callbacks such as da8xx_musb_exit() call
usb_put_phy(musb->xceiv), which drops the final reference and frees the PHY
before musb_remove() passes musb->xceiv to usb_phy_shutdown():

drivers/usb/musb/musb_core.c:musb_remove() {
	...
	musb_platform_exit(musb);

	pm_runtime_put_sync(musb->controller);
	pm_runtime_disable(musb->controller);
	musb_phy_callback = NULL;
	if (musb->dma_controller)
		musb_dma_controller_destroy(musb->dma_controller);
	usb_phy_shutdown(musb->xceiv);
	...
}

[1] https://sashiko.dev/#/bug/linux-d180c2b9-0006-4638-a22b-13df8c201235

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

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

[Joshua Crofts]

Since commit d1706f9da9f4 ("PM: runtime: call pm_runtime_dont_use_autosuspend() on reinit()"),
driver core is now responsible for undoing any
pm_runtime_use_autosuspend() calls on driver exit.

Remove the redundant function calls.

Signed-off-by: Joshua Crofts <crofts@kernel.org>
---
 drivers/usb/dwc3/core.c      | 2 --
 drivers/usb/dwc3/dwc3-am62.c | 1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index fd5c2cd36c59..c4f0aa962747 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:
@@ -2486,7 +2485,6 @@ void dwc3_core_remove(struct dwc3 *dwc)
 
 	pm_runtime_allow(dwc->dev);
 	pm_runtime_disable(dwc->dev);
-	pm_runtime_dont_use_autosuspend(dwc->dev);
 	pm_runtime_put_noidle(dwc->dev);
 	/*
 	 * HACK: Clear the driver data, which is currently accessed by parent
diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c
index d03b3950ce04..0df7e436aeb0 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);
 }
 
-- 
2.47.3

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

[sashiko-bot]

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