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

Thinh Nguyen
2025-09-12T22:23:52Z

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?

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.

BR,
Thinh