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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPwe5RM85di_ibJtt3Da+CUW9ZS_GhkXqdEgJssu_k+xjMSdmw@mail.gmail.com>
Date: Wed, 3 Sep 2025 19:51:14 +0800
From: ryan zhou <ryanzhou54@...il.com>
To: Alan Stern <stern@...land.harvard.edu>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Thinh Nguyen <Thinh.Nguyen@...opsys.com>, 
	Roy Luo <royluo@...gle.com>, 
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>, 
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>
Subject: Re: [PATCH] drvier: usb: dwc3: Fix runtime PM trying to activate
 child device xxx.dwc3 but parent is not active

Alan Stern <stern@...land.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?