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

Rafael J. Wysocki
2025-09-13T12:43:47Z

On Sat, Sep 13, 2025 at 12:23 AM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
>
> On Thu, Sep 04, 2025, Alan Stern wrote:
> > 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@rowland.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.
> >
>
> So, can this solution be the default behavior in the PM core? That it
> would initiate pm_runtime_resume by default?

Not really because there are drivers that don't want that to be done.
For example, everybody pointing their suspend/resume callbacks to
pm_runtime_force_suspend/resume(), respectively, and not setting
SMART_SUSPEND.

> Seems you want this to be handled in the device driver and not PM core
> right? ie. the condition "the device needs to be woken up" will not be a
> PM user flag/config but the device driver needs to check that itself.

Not necessarily. I'll write more about this next week (well, hopefully).