Issue description:During the wake-up sequence, if the system invokes
 dwc3->resume and detects that the parent device of dwc3 is in a
runtime suspend state, the system will generate an error: runtime PM
trying to activate child device xxx.dwc3 but parent is not active.

Solution:At the dwc3->resume entry point, if the dwc3 controller
is detected in a suspended state, the function shall return
immediately without executing any further operations.

Signed-off-by: Ryan Zhou <ryanzhou54@gmail.com>
---
 drivers/usb/dwc3/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 370fc524a468..06a6f8a67129 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);
-- 
2.25.1

On Wed, Sep 10, 2025, Ryan Zhou wrote:
> Issue description:During the wake-up sequence, if the system invokes
>  dwc3->resume and detects that the parent device of dwc3 is in a
> runtime suspend state, the system will generate an error: runtime PM
> trying to activate child device xxx.dwc3 but parent is not active.
> 
> Solution:At the dwc3->resume entry point, if the dwc3 controller
> is detected in a suspended state, the function shall return
> immediately without executing any further operations.
> 
> Signed-off-by: Ryan Zhou <ryanzhou54@gmail.com>
> ---
>  drivers/usb/dwc3/core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 370fc524a468..06a6f8a67129 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;
> +

Is this a documented behavior where the device should remain runtime
suspend on system resume? I feel that that this should be configurable
by the user or defined the PM core. I don't think we should change
default behavior here just to workaround the issue that we're facing.

What if the user wants to keep the old behavior and resume up the device
on system resume?

BR,
Thinh

>  	pinctrl_pm_select_default_state(dev);
>  
>  	pm_runtime_disable(dev);
> -- 
> 2.25.1
> 

Thinh Nguyen <Thinh.Nguyen@synopsys.com> 于2025年9月11日周四 09:32写道：
>
> On Wed, Sep 10, 2025, Ryan Zhou wrote:
> > Issue description:During the wake-up sequence, if the system invokes
> >  dwc3->resume and detects that the parent device of dwc3 is in a
> > runtime suspend state, the system will generate an error: runtime PM
> > trying to activate child device xxx.dwc3 but parent is not active.
> >
> > Solution:At the dwc3->resume entry point, if the dwc3 controller
> > is detected in a suspended state, the function shall return
> > immediately without executing any further operations.
> >
> > Signed-off-by: Ryan Zhou <ryanzhou54@gmail.com>
> > ---
> >  drivers/usb/dwc3/core.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 370fc524a468..06a6f8a67129 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;
> > +
>
> Is this a documented behavior where the device should remain runtime
> suspend on system resume? I feel that that this should be configurable
> by the user or defined the PM core. I don't think we should change
> default behavior here just to workaround the issue that we're facing.

No documentation was found, but modifying the runtime suspend state
after wakeup from sleep seems unnecessary if the device was already
in runtime suspend before sleep.

> What if the user wants to keep the old behavior and resume up the device
> on system resume?
For USB devices, RPM resume should be initiated by plug/unplug events,
not PM resume when the device is physically disconnected.


Thanks,

Ryan

Hi Ryan,

On Thu, Sep 11, 2025 at 01:32:47AM +0000, Thinh Nguyen wrote:
> On Wed, Sep 10, 2025, Ryan Zhou wrote:
> > Issue description:During the wake-up sequence, if the system invokes
> >  dwc3->resume and detects that the parent device of dwc3 is in a
> > runtime suspend state, the system will generate an error: runtime PM
> > trying to activate child device xxx.dwc3 but parent is not active.
> > 
> > Solution:At the dwc3->resume entry point, if the dwc3 controller
> > is detected in a suspended state, the function shall return
> > immediately without executing any further operations.
> > 
> > Signed-off-by: Ryan Zhou <ryanzhou54@gmail.com>
> > ---
> >  drivers/usb/dwc3/core.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 370fc524a468..06a6f8a67129 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;
> > +
> 
> Is this a documented behavior where the device should remain runtime
> suspend on system resume? I feel that that this should be configurable
> by the user or defined the PM core. I don't think we should change
> default behavior here just to workaround the issue that we're facing.
> 
> What if the user wants to keep the old behavior and resume up the device
> on system resume?

What about resume the device firstly if it's already runtime suspended when
call dwc3_pm_suspend(). Therefor, the old behavior can be kept and the issue
can be avoided.

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 370fc524a468..1b8dbb260017 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -2672,6 +2672,9 @@ int dwc3_pm_suspend(struct dwc3 *dwc)
        struct device *dev = dwc->dev;
        int             ret;

+       if (pm_runtime_suspended(dev))
+               pm_runtime_resume(dev);
+
        ret = dwc3_suspend_common(dwc, PMSG_SUSPEND);
        if (ret)
                return ret;

Thanks,
Xu Yang

> 
> BR,
> Thinh
> 
> >  	pinctrl_pm_select_default_state(dev);
> >  
> >  	pm_runtime_disable(dev);
> > -- 
> > 2.25.1
> >

Hi Xu，

Xu Yang <xu.yang_2@nxp.com> 于2025年9月11日周四 18:58写道：
>
> Hi Ryan,
>
> On Thu, Sep 11, 2025 at 01:32:47AM +0000, Thinh Nguyen wrote:
> > On Wed, Sep 10, 2025, Ryan Zhou wrote:
> > > Issue description:During the wake-up sequence, if the system invokes
> > >  dwc3->resume and detects that the parent device of dwc3 is in a
> > > runtime suspend state, the system will generate an error: runtime PM
> > > trying to activate child device xxx.dwc3 but parent is not active.
> > >
> > > Solution:At the dwc3->resume entry point, if the dwc3 controller
> > > is detected in a suspended state, the function shall return
> > > immediately without executing any further operations.
> > >
> > > Signed-off-by: Ryan Zhou <ryanzhou54@gmail.com>
> > > ---
> > >  drivers/usb/dwc3/core.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > index 370fc524a468..06a6f8a67129 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;
> > > +
> >
> > Is this a documented behavior where the device should remain runtime
> > suspend on system resume? I feel that that this should be configurable
> > by the user or defined the PM core. I don't think we should change
> > default behavior here just to workaround the issue that we're facing.
> >
> > What if the user wants to keep the old behavior and resume up the device
> > on system resume?
>
> What about resume the device firstly if it's already runtime suspended when
> call dwc3_pm_suspend(). Therefor, the old behavior can be kept and the issue
> can be avoided.

Originally, I also believed that forcing the device to remain active
before PM suspend
 was necessary. However, this approach has two drawbacks:
    1. It prolongs the system's sleep transition time.
    2. Worse, if a USB insertion wakes the system during enumeration,
the system may
    re-enter sleep before the USB device is fully recognized.

> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 370fc524a468..1b8dbb260017 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -2672,6 +2672,9 @@ int dwc3_pm_suspend(struct dwc3 *dwc)
>         struct device *dev = dwc->dev;
>         int             ret;
>
> +       if (pm_runtime_suspended(dev))
> +               pm_runtime_resume(dev);
> +
>         ret = dwc3_suspend_common(dwc, PMSG_SUSPEND);
>         if (ret)
>                 return ret;

On Thu, Sep 11, 2025 at 09:51:50PM +0800, ryan zhou wrote:
> Hi Xu，
> 
> Xu Yang <xu.yang_2@nxp.com> 于2025年9月11日周四 18:58写道：
> >
> > Hi Ryan,
> >
> > On Thu, Sep 11, 2025 at 01:32:47AM +0000, Thinh Nguyen wrote:
> > > On Wed, Sep 10, 2025, Ryan Zhou wrote:
> > > > Issue description:During the wake-up sequence, if the system invokes
> > > >  dwc3->resume and detects that the parent device of dwc3 is in a
> > > > runtime suspend state, the system will generate an error: runtime PM
> > > > trying to activate child device xxx.dwc3 but parent is not active.
> > > >
> > > > Solution:At the dwc3->resume entry point, if the dwc3 controller
> > > > is detected in a suspended state, the function shall return
> > > > immediately without executing any further operations.
> > > >
> > > > Signed-off-by: Ryan Zhou <ryanzhou54@gmail.com>
> > > > ---
> > > >  drivers/usb/dwc3/core.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > index 370fc524a468..06a6f8a67129 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;
> > > > +
> > >
> > > Is this a documented behavior where the device should remain runtime
> > > suspend on system resume? I feel that that this should be configurable
> > > by the user or defined the PM core. I don't think we should change
> > > default behavior here just to workaround the issue that we're facing.
> > >
> > > What if the user wants to keep the old behavior and resume up the device
> > > on system resume?
> >
> > What about resume the device firstly if it's already runtime suspended when
> > call dwc3_pm_suspend(). Therefor, the old behavior can be kept and the issue
> > can be avoided.
> 
> Originally, I also believed that forcing the device to remain active
> before PM suspend
>  was necessary. However, this approach has two drawbacks:
>     1. It prolongs the system's sleep transition time.
>     2. Worse, if a USB insertion wakes the system during enumeration,
> the system may
>     re-enter sleep before the USB device is fully recognized.

Can you provide more detail about point 2? When is the USB device inserted?
Is the re-enter behavior caused by pm_runtime_resume()?

Thanks,
Xu Yang

> 
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 370fc524a468..1b8dbb260017 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -2672,6 +2672,9 @@ int dwc3_pm_suspend(struct dwc3 *dwc)
> >         struct device *dev = dwc->dev;
> >         int             ret;
> >
> > +       if (pm_runtime_suspended(dev))
> > +               pm_runtime_resume(dev);
> > +
> >         ret = dwc3_suspend_common(dwc, PMSG_SUSPEND);
> >         if (ret)
> >                 return ret;

On Thu, Sep 11, 2025 at 12:58 PM Xu Yang <xu.yang_2@nxp.com> wrote:
>
> Hi Ryan,
>
> On Thu, Sep 11, 2025 at 01:32:47AM +0000, Thinh Nguyen wrote:
> > On Wed, Sep 10, 2025, Ryan Zhou wrote:
> > > Issue description:During the wake-up sequence, if the system invokes
> > >  dwc3->resume and detects that the parent device of dwc3 is in a
> > > runtime suspend state, the system will generate an error: runtime PM
> > > trying to activate child device xxx.dwc3 but parent is not active.
> > >
> > > Solution:At the dwc3->resume entry point, if the dwc3 controller
> > > is detected in a suspended state, the function shall return
> > > immediately without executing any further operations.
> > >
> > > Signed-off-by: Ryan Zhou <ryanzhou54@gmail.com>
> > > ---
> > >  drivers/usb/dwc3/core.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > index 370fc524a468..06a6f8a67129 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;
> > > +
> >
> > Is this a documented behavior where the device should remain runtime
> > suspend on system resume? I feel that that this should be configurable
> > by the user or defined the PM core. I don't think we should change
> > default behavior here just to workaround the issue that we're facing.
> >
> > What if the user wants to keep the old behavior and resume up the device
> > on system resume?
>
> What about resume the device firstly if it's already runtime suspended when
> call dwc3_pm_suspend(). Therefor, the old behavior can be kept and the issue
> can be avoided.
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 370fc524a468..1b8dbb260017 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -2672,6 +2672,9 @@ int dwc3_pm_suspend(struct dwc3 *dwc)
>         struct device *dev = dwc->dev;
>         int             ret;
>
> +       if (pm_runtime_suspended(dev))
> +               pm_runtime_resume(dev);

You can just call pm_runtime_resume() here without the preliminary check.

> +
>         ret = dwc3_suspend_common(dwc, PMSG_SUSPEND);
>         if (ret)
>                 return ret;
>
> Thanks,
> Xu Yang
>
> >
> > BR,
> > Thinh
> >
> > >     pinctrl_pm_select_default_state(dev);
> > >
> > >     pm_runtime_disable(dev);
> > > --

On Thu, Sep 11, 2025 at 01:35:59PM +0200, Rafael J. Wysocki wrote:
> On Thu, Sep 11, 2025 at 12:58 PM Xu Yang <xu.yang_2@nxp.com> wrote:
> >
> > Hi Ryan,
> >
> > On Thu, Sep 11, 2025 at 01:32:47AM +0000, Thinh Nguyen wrote:
> > > On Wed, Sep 10, 2025, Ryan Zhou wrote:
> > > > Issue description:During the wake-up sequence, if the system invokes
> > > >  dwc3->resume and detects that the parent device of dwc3 is in a
> > > > runtime suspend state, the system will generate an error: runtime PM
> > > > trying to activate child device xxx.dwc3 but parent is not active.
> > > >

[...]

> > @@ -2672,6 +2672,9 @@ int dwc3_pm_suspend(struct dwc3 *dwc)
> >         struct device *dev = dwc->dev;
> >         int             ret;
> >
> > +       if (pm_runtime_suspended(dev))
> > +               pm_runtime_resume(dev);
> 
> You can just call pm_runtime_resume() here without the preliminary check.

True.

On Fri, Sep 12, 2025, Xu Yang wrote:
> On Thu, Sep 11, 2025 at 01:35:59PM +0200, Rafael J. Wysocki wrote:
> > On Thu, Sep 11, 2025 at 12:58 PM Xu Yang <xu.yang_2@nxp.com> wrote:
> > >
> > > Hi Ryan,
> > >
> > > On Thu, Sep 11, 2025 at 01:32:47AM +0000, Thinh Nguyen wrote:
> > > > On Wed, Sep 10, 2025, Ryan Zhou wrote:
> > > > > Issue description:During the wake-up sequence, if the system invokes
> > > > >  dwc3->resume and detects that the parent device of dwc3 is in a
> > > > > runtime suspend state, the system will generate an error: runtime PM
> > > > > trying to activate child device xxx.dwc3 but parent is not active.
> > > > >
> 
> [...]
> 
> > > @@ -2672,6 +2672,9 @@ int dwc3_pm_suspend(struct dwc3 *dwc)
> > >         struct device *dev = dwc->dev;
> > >         int             ret;
> > >
> > > +       if (pm_runtime_suspended(dev))
> > > +               pm_runtime_resume(dev);
> > 
> > You can just call pm_runtime_resume() here without the preliminary check.
> 
> True.

I like this solution.

Hi Ryan, can we try this?

Thanks,
Thinh

Rafael J. Wysocki <rafael@kernel.org> 于2025年9月11日周四 19:36写道：
>
> On Thu, Sep 11, 2025 at 12:58 PM Xu Yang <xu.yang_2@nxp.com> wrote:
> >
> > Hi Ryan,
> >
> > On Thu, Sep 11, 2025 at 01:32:47AM +0000, Thinh Nguyen wrote:
> > > On Wed, Sep 10, 2025, Ryan Zhou wrote:
> > > > Issue description:During the wake-up sequence, if the system invokes
> > > >  dwc3->resume and detects that the parent device of dwc3 is in a
> > > > runtime suspend state, the system will generate an error: runtime PM
> > > > trying to activate child device xxx.dwc3 but parent is not active.
> > > >
> > > > Solution:At the dwc3->resume entry point, if the dwc3 controller
> > > > is detected in a suspended state, the function shall return
> > > > immediately without executing any further operations.
> > > >
> > > > Signed-off-by: Ryan Zhou <ryanzhou54@gmail.com>
> > > > ---
> > > >  drivers/usb/dwc3/core.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > index 370fc524a468..06a6f8a67129 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;
> > > > +
> > >
> > > Is this a documented behavior where the device should remain runtime
> > > suspend on system resume? I feel that that this should be configurable
> > > by the user or defined the PM core. I don't think we should change
> > > default behavior here just to workaround the issue that we're facing.
> > >
> > > What if the user wants to keep the old behavior and resume up the device
> > > on system resume?
> >
> > What about resume the device firstly if it's already runtime suspended when
> > call dwc3_pm_suspend(). Therefor, the old behavior can be kept and the issue
> > can be avoided.
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 370fc524a468..1b8dbb260017 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -2672,6 +2672,9 @@ int dwc3_pm_suspend(struct dwc3 *dwc)
> >         struct device *dev = dwc->dev;
> >         int             ret;
> >
> > +       if (pm_runtime_suspended(dev))
> > +               pm_runtime_resume(dev);
>
> You can just call pm_runtime_resume() here without the preliminary check.

If the device is active before sleep, skip runtime_resume after wakeup
and just call dwc3->suspend.

Thanks，

Ryan

On Thu, Sep 11, 2025 at 3:57 PM ryan zhou <ryanzhou54@gmail.com> wrote:
>
> Rafael J. Wysocki <rafael@kernel.org> 于2025年9月11日周四 19:36写道：
> >
> > On Thu, Sep 11, 2025 at 12:58 PM Xu Yang <xu.yang_2@nxp.com> wrote:
> > >
> > > Hi Ryan,
> > >
> > > On Thu, Sep 11, 2025 at 01:32:47AM +0000, Thinh Nguyen wrote:
> > > > On Wed, Sep 10, 2025, Ryan Zhou wrote:
> > > > > Issue description:During the wake-up sequence, if the system invokes
> > > > >  dwc3->resume and detects that the parent device of dwc3 is in a
> > > > > runtime suspend state, the system will generate an error: runtime PM
> > > > > trying to activate child device xxx.dwc3 but parent is not active.
> > > > >
> > > > > Solution:At the dwc3->resume entry point, if the dwc3 controller
> > > > > is detected in a suspended state, the function shall return
> > > > > immediately without executing any further operations.
> > > > >
> > > > > Signed-off-by: Ryan Zhou <ryanzhou54@gmail.com>
> > > > > ---
> > > > >  drivers/usb/dwc3/core.c | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > > index 370fc524a468..06a6f8a67129 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;
> > > > > +
> > > >
> > > > Is this a documented behavior where the device should remain runtime
> > > > suspend on system resume? I feel that that this should be configurable
> > > > by the user or defined the PM core. I don't think we should change
> > > > default behavior here just to workaround the issue that we're facing.
> > > >
> > > > What if the user wants to keep the old behavior and resume up the device
> > > > on system resume?
> > >
> > > What about resume the device firstly if it's already runtime suspended when
> > > call dwc3_pm_suspend(). Therefor, the old behavior can be kept and the issue
> > > can be avoided.
> > >
> > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > index 370fc524a468..1b8dbb260017 100644
> > > --- a/drivers/usb/dwc3/core.c
> > > +++ b/drivers/usb/dwc3/core.c
> > > @@ -2672,6 +2672,9 @@ int dwc3_pm_suspend(struct dwc3 *dwc)
> > >         struct device *dev = dwc->dev;
> > >         int             ret;
> > >
> > > +       if (pm_runtime_suspended(dev))
> > > +               pm_runtime_resume(dev);
> >
> > You can just call pm_runtime_resume() here without the preliminary check.
>
> If the device is active before sleep, skip runtime_resume after wakeup
> and just call dwc3->suspend.

But pm_runtime_resume() will not resume an active device, will it?

On Wed, Sep 10, 2025 at 12:19:01AM +0800, Ryan Zhou wrote:
> Issue description:During the wake-up sequence, if the system invokes
>  dwc3->resume and detects that the parent device of dwc3 is in a
> runtime suspend state, the system will generate an error: runtime PM
> trying to activate child device xxx.dwc3 but parent is not active.
> 
> Solution:At the dwc3->resume entry point, if the dwc3 controller
> is detected in a suspended state, the function shall return
> immediately without executing any further operations.
> 
> Signed-off-by: Ryan Zhou <ryanzhou54@gmail.com>
> ---
>  drivers/usb/dwc3/core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 370fc524a468..06a6f8a67129 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);
> -- 
> 2.25.1
> 
> 

What commit id does this fi?

thanks,

greg k-h

Hi Greg KH,
Sorry, I didn't understand your question. Are you asking for my patch
commit ID? I've resubmitted patch v3, and the commit details are as
follows:

commit 92bc5086f53404f6d14d8550209d1c8cd3fa9036 (HEAD -> usb-next-develop)

Or do you need the commit that introduced this issue?



Greg KH <gregkh@linuxfoundation.org> 于2025年9月10日周三 00:25写道：
>
> On Wed, Sep 10, 2025 at 12:19:01AM +0800, Ryan Zhou wrote:
> > Issue description:During the wake-up sequence, if the system invokes
> >  dwc3->resume and detects that the parent device of dwc3 is in a
> > runtime suspend state, the system will generate an error: runtime PM
> > trying to activate child device xxx.dwc3 but parent is not active.
> >
> > Solution:At the dwc3->resume entry point, if the dwc3 controller
> > is detected in a suspended state, the function shall return
> > immediately without executing any further operations.
> >
> > Signed-off-by: Ryan Zhou <ryanzhou54@gmail.com>
> > ---
> >  drivers/usb/dwc3/core.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 370fc524a468..06a6f8a67129 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);
> > --
> > 2.25.1
> >
> >
>
> What commit id does this fi?
>
> thanks,
>
> greg k-h

On Wed, Sep 10, 2025 at 08:56:36PM +0800, ryan zhou wrote:
> Hi Greg KH,
> Sorry, I didn't understand your question. Are you asking for my patch
> commit ID? I've resubmitted patch v3, and the commit details are as
> follows:
> 
> commit 92bc5086f53404f6d14d8550209d1c8cd3fa9036 (HEAD -> usb-next-develop)
> 
> Or do you need the commit that introduced this issue?

Sorry, I mean "what commit does this fix", so that you can add a
"Fixes:" tag to it.

thanks,

greg k-h

Greg KH <gregkh@linuxfoundation.org> 于2025年9月10日周三 21:07写道：
>
> On Wed, Sep 10, 2025 at 08:56:36PM +0800, ryan zhou wrote:
> > Hi Greg KH,
> > Sorry, I didn't understand your question. Are you asking for my patch
> > commit ID? I've resubmitted patch v3, and the commit details are as
> > follows:
> >
> > commit 92bc5086f53404f6d14d8550209d1c8cd3fa9036 (HEAD -> usb-next-develop)
> >
> > Or do you need the commit that introduced this issue?
>
> Sorry, I mean "what commit does this fix", so that you can add a
> "Fixes:" tag to it.

I initially targeted these two issues:
commit1： 0227cc84c44417a29c8102e41db8ec2c11ebc6b2
usb: dwc3: core: don't do suspend for device mode if already suspended
commit2： 68c26fe58182f5af56bfa577d1cc0c949740baab
usb: dwc3: set pm runtime active before resume common

When the DWC3 controller is in a runtime suspend state, an interruption occurs
during the system sleep transition, resulting in USB failure to resume
properly after wakeup.
The detailed sequence is as follows:（refer to commit e3a9bd247cddf
merged by Ray Chi）
    EX.
    RPM suspend: ... -> dwc3_runtime_suspend()
                          -> rpm_suspend() of parent device
    ...
    PM suspend: ... -> dwc3_suspend() -> pm_suspend of parent device
                                     ^ interrupt, so resume suspended device
              ...  <-  dwc3_resume()  <-/
                          ^ pm_runtime_set_active() returns erro

Post-analysis reveals:
    ‌Commit 2‌ generates unexpected error logs ( runtime PM trying to
activate child device xxx.dwc3 but parent is not active).
    ‌Commit 1‌ disrupts USB recovery in this context, attributable to
the following factors:

    EX.
    RPM suspend: ... -> dwc3_runtime_suspend()
                          -> rpm_suspend() of parent device
    ...

    PM suspend: ... -> dwc3_suspend()
                                     |___dwc3_suspend_common()
                                              ^ if
(pm_runtime_suspended(dwc->dev)) then skip suspend process
                                          |___dwc3_core_exit()
                                               |___dwc3_phy_exit()
      PM resume   ...  <-  dwc3_resume()
                          |___dwc3_resume_common()
                                       ^ pm_runtime_set_active()
report error(error logs : runtime PM trying to activate child device
xxx.dwc3 but parent is not active).
                              |___dwc3_core_init_for_resume()
                                  |___dwc3_core_init()
                                      |___dwc3_phy_init()
                                               ^ phy->init_count++ and
phy->power_count++
     ... Next,usb connect (Note: dwc3 is always in runtime suspend)
      RPM resume   ...  <-  dwc3_runtime_resume()
                          |___dwc3_resume_common()
                              |___dwc3_core_init_for_resume()
                                  |___dwc3_core_init()
                                      |___dwc3_phy_init()
                                              ^PHY reinitialization is
prevented due to non-zero values in phy->init_count and phy->power_on.

However, during my submission process, I found that Ray Chi
encountered the same issue and has already merged commit e3a9bd247cddf
(usb: dwc3:
Skip resume if pm_runtime_set_active() fails), which fixed the problem
introduced by commit 2. But the error logs (runtime PM trying to
activate child
device xxx.dwc3 but parent is not active) introduced by commit 1 still remains.
I will now evaluate whether to proceed with further fixes for the
issue introduced by commit 1, based on Ray Chi's submission. And also
I will incorporate
the relevant background details in the subsequent commit.In my view,
commit e3a9bd247cddf (usb: dwc3:Skip resume if pm_runtime_set_active()
fails)
appears to be more of a workaround solution.


thanks,

Ryan

Issue description:During the wake-up sequence, if the system invokes
 dwc3->resume and detects that the parent device of dwc3 is in a
runtime suspend state, the system will generate an error: runtime PM
trying to activate child device xxx.dwc3 but parent is not active.

Solution:At the dwc3->resume entry point, if the dwc3 controller
is detected in a suspended state, the function shall return
immediately without executing any further operations.

Signed-off-by: Ryan Zhou <ryanzhou54@gmail.com>
---
 drivers/usb/dwc3/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 370fc524a468..06a6f8a67129 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);
-- 
2.25.1