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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAJZ5v0h=i9XF_SQMOhz3P+4SAH3Qy-r1oUiiw7Bp=PcRnJjVbQ@mail.gmail.com>
Date: Wed, 3 Sep 2025 21:30:47 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Alan Stern <stern@...land.harvard.edu>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Thinh Nguyen <Thinh.Nguyen@...opsys.com>, 
	ryan zhou <ryanzhou54@...il.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

On Tue, Sep 2, 2025 at 4:41 AM Alan Stern <stern@...land.harvard.edu> wrote:
>
> 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.

That's reasonable IMV.

> 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.
>
> 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.

This is not just about the parent, but also about suppliers and things
get fairly complicated at that point, so I would just rely on the
observation that runtime wakeup should trigger shortly.

> 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.)
>
> There probably are some detailed reasons why that won't always work, but
> maybe you figure out something like it that will be okay.

I personally think that it would be reasonable to simply preserve
device states in error paths unless they have been changed already
before the error (or suspend abort due to a wakeup signal).

By this rule, B would be left in runtime suspend if it were still in
runtime suspend when the error (or suspend abort in general) occurred
and then it doesn't matter what happens to A.

The PM core can do something like that for the drivers opting in for
runtime PM integration assistance, so to speak.  That is, drivers that
point their ->suspend() and ->resume() callbacks to
pm_runtime_force_suspend() and pm_runtime_force_resume(),
respectively, or set DPM_FLAG_SMART_SUSPEND (or both at the same time
which is now feasible).  Otherwise, it is hard to say what the
expectations of the driver are and some code between the driver and
the PM core may be involved (say, the PCI bus type).