[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <385dccf3-234a-4f83-9610-81ac30bf1466@rowland.harvard.edu>
Date: Thu, 4 Sep 2025 14:54:21 -0400
From: Alan Stern <stern@...land.harvard.edu>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: 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 Thu, Sep 04, 2025 at 07:34:22PM +0200, Rafael J. Wysocki wrote:
> On Thu, Sep 4, 2025 at 4:19 PM Alan Stern <stern@...land.harvard.edu> wrote:
> >
> > On Thu, Sep 04, 2025 at 04:08:47PM +0200, Rafael J. Wysocki wrote:
> > > Say this is not the case and say that the device is runtime-suspended
> > > to start with. Then the "suspend" callback has two choices: either
> > > (1) it can runtime-resume the device before doing anything to it,
> > > which will also cause the device's parent and suppliers to
> > > runtime-resume, or (2) it can update the device's state without
> > > resuming it.
> > >
> > > If it chooses (1), then "resume" is straightforward. If it chooses
> > > (2), "resume" may just reverse the changes made by "suspend" and
> > > declare that the device is runtime-suspended. And if it really really
> > > wants to resume the device then, why not call runtime_resume() on it?
> >
> > That's what I meant by needing "cooperation from the driver". The
> > driver's ->resume callback needs to do this check to see which pathway
> > to follow: (1) or (2).
>
> Unless "suspend" always does the same thing, so it always does (1) or
> it always does (2).
>
> In that case, "resume" will know what to do without checking.
It still has to check whether the device is runtime suspended.
> I'd like to mention that if "suspend" chooses (2), it may need to
> resume the suspended parent or suppliers to be able to access the
> device even though the device itself won't be resumed. I'm not sure
> if (2) is really attractive then, though.
True.
> Also, in the example we've been considering so far, the assumption is
> that B can just stay in runtime suspend, so why would it need to be
> resumed by "resume"? And if there is a specific reason for resuming
> it, "resume" can just call runtime_resume() on it AFAICS.
So it appears to boil down to this, as far as ->resume is concerned: At
the start of the callback routine, there should be something like:
if (pm_runtime_suspended(dev)) {
if (the device needs to be woken up) {
pm_runtime_resume(dev);
... whatever else is needed ...
}
return 0;
}
If ->suspend is clever, it can clear or set the SMART_SUSPEND flag
according to whether the device will need to be woken up. Then the
second conditional above will always be true whenever the callback runs,
so the test can be skipped.
Alan Stern
Powered by blists - more mailing lists