Re: [PATCH] drvier: usb: dwc3: Fix runtime PM trying to activate child device xxx.dwc3 but parent is not active

ryan zhou
2025-09-03T11:51:26Z

Alan Stern <stern@rowland.harvard.edu> 于2025年9月2日周二 10:41写道：
>
> On Mon, Sep 01, 2025 at 10:40:25PM +0200, Rafael J. Wysocki wrote:
> > Of course, the driver of B may also choose to leave the device in
> > runtime suspend in its system resume callback. This requires checking
> > the runtime PM status of the device upfront, but the driver needs to
> > do that anyway in order to leave the device in runtime suspend during
> > system suspend, so it can record the fact that the device has been
> > left in runtime suspend. That record can be used later during system
> > resume.
>
> As a general rule, I think this is by default the best approach. That
> is, since B was in runtime suspend before the system sleep, you should
> just keep it in runtime suspend after the system sleep unless you have
> some good reason not to. In other words, strive to leave the entire
> system in the same state that it started in, as near as possible.
>
Alan, I fully concur with your perspective. Specifically, I maintain that the
device's runtime status should remain consistent before and after
system deep sleep.
To keep parent runtime-active during child wake, use device_link_add
create a link between them. Then dwc3_resume's pm_runtime_set_active
forces parent wake-up first.
However, for the dwc3 driver, both Rafael 's two solutions and
My aforementioned solution introduces new issues. When USB performs
deep sleep wake-up, the USB PHYS fails to initialize properly because
deep sleep wake-up executes runtime resume first, leaving the PHYS
initialized with per-sleep configurations. This ignores Type-C interface
requirements to reconfigure PHYS based on plug orientation.

> One good reason not to would obviously be if B is the source of a wakeup
> signal...
>
> > The kind of tricky aspect of this is when the device triggers a system
> > wakeup by generating a wakeup signal. In that case, it is probably
> > better to resume it during system resume, but the driver should know
> > that it is the case (it has access to the device's registers after
> > all).
>
> Not necessarily. Suppose that C is a child of B, and C is the wakeup
> source. B's driver might decide to keep B in runtime suspend
> since B wasn't the wakeup source -- but then C's driver would have to
> make the same decision and would not have access to C's registers.
>
> > It may, for example, use runtime_resume() for resuming the
> > device (and its parent etc) then.
>
> Consider this as a possible heuristic for B's ->resume callback, in the
> case where B was in runtime suspend throughout the system sleep:
>
> If B's parent A is not in runtime suspend, test whether B
> has a wakeup signal pending. If it does, do a runtime
> resume. If not (or if A is in runtime suspend), leave B
> in runtime suspend.
>
Following your suggestion, I conducted verification. If the child device
is in a runtime suspended state, the suspend and resume callback should
not be invoked. The proposed solution is as follows:
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 20945cad29a1..642bf4b5d3c4 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -2687,6 +2687,9 @@ int dwc3_pm_resume(struct dwc3 *dwc)
struct device *dev = dwc->dev;
int ret = 0;

+ if (pm_runtime_suspended(dev))
+ return ret;
+
pinctrl_pm_select_default_state(dev);

pm_runtime_disable(dev);

> At first glance I think that will work fairly well. Even if B is kept
> in runtime suspend when it shouldn't be, the normal runtime wakeup
> signalling mechanism should kick in without too much of a delay.
>
> The big problem is that this issue applies to all subsystems and
> devices. It would be better if we had a uniform solution that could be
> implemented in the PM core, not in every single subsystem or device
> driver.
>
> Here's another possibility, one that can be implemented in the PM core
> during the ->resume, ->resume_early, or ->resume_noirq phase of system
> wakeup:
>
> If A and B are both in runtime suspend, do not invoke B's
> ->resume callback. (Or maybe don't invoke it if A's ->resume
> callback wasn't invoked.)
>
It is preferable for the PM core to maintain the runtime status
between parent and child devices,
where feasible. Could you advise on the most effective approach to
settle this issue?