RE: [PATCH] phy: renesas: phy-rzg3e-usb3: Fix runtime PM underflow during suspend

[Ovidiu Panait <ovidiu.panait.rb@renesas.com>]

Hi Geert,

> 
> Hi Ovidiu,
> 
> On Mon, 4 May 2026 at 13:11, Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> wrote:
> > > On 27-04-26, 19:47, Ovidiu Panait wrote:
> > > > On the Renesas RZ/V2H platform, if the xhcd driver is unbound and
> the
> > > > system is suspended afterwards, a PM underflow error will occur:
> > > >
> > > >  # echo 15850000.usb > /sys/bus/platform/drivers/xhci-renesas-
> hcd/unbind
> > > >  # systemctl suspend
> > > >  15870000.usb-phy: PM: dpm_run_callback(): genpd_resume_noirq
> returns -
> > > 13
> > > >  15870000.usb-phy: PM: failed to resume noirq: error -13
> > > >  15870000.usb-phy: Runtime PM usage count underflow!
> > > >
> > > > Since the PHY framework is managing the runtime PM of the PHY via
> > > > phy_power_on()/phy_power_off(), there is no need for the PHY driver
> to
> > > > manipulate the runtime PM state during suspend.
> > > >
> > > > To fix this, remove the runtime PM calls from the suspend/resume
> paths
> > > > and add a get/put pair inside rzg3e_phy_usb3_init_helper() to make
> sure
> > > > the clock is enabled during init, even when there is no consumer for
> > > > the PHY.
> > >
> > > > Also, change the suspend ops from NOIRQ_SYSTEM_SLEEP_PM_OPS to
> > > > SYSTEM_SLEEP_PM_OPS because runtime PM is disabled during the noirq
> > > phase
> > > > and pm_runtime_resume_and_get() would not actually enable the device
> > > clock.
> > >
> > > > Fixes: ee5f1a3f90a4 ("phy: renesas: Add Renesas RZ/G3E USB3.0 PHY
> > > driver")
> > > > Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> 
> > > > index 6b3453ea0004..055775e1a0f7 100644
> > > > --- a/drivers/phy/renesas/phy-rzg3e-usb3.c
> > > > +++ b/drivers/phy/renesas/phy-rzg3e-usb3.c
> 
> > > > @@ -215,27 +226,21 @@ static int rzg3e_phy_usb3_resume(struct device
> > > *dev)
> > > >     if (ret)
> > > >             return ret;
> > > >
> > > > -   ret = pm_runtime_resume_and_get(dev);
> > > > +   ret = rzg3e_phy_usb3_init_helper(r);
> > > >     if (ret)
> > > >             goto reset_assert;
> > > >
> > > > -   ret = rzg3e_phy_usb3_init_helper(r->base);
> > > > -   if (ret)
> > > > -           goto pm_put;
> > > > -
> > > >     r->skip_reinit = true;
> > >
> > > https://sashiko.dev/#/patchset/20260427194741.161533-1-
> > > ovidiu.panait.rb%40renesas.com
> > >
> >
> > I think this is not applicable for our platforms, as the power domain
> only
> > gates the clock during runtime suspend, so the register state is not
> lost
> > across runtime suspend/resume.
> >
> > The power domain (drivers/clk/renesas/rzv2h-cpg.c) is marked as
> > GENPD_FLAG_ALWAYS_ON.
> 
> Won't the SoC be powered down during s2ram?
> 

Yes, the SoC will be powered down during s2ram and the USB3 PHY is
reinitialized in the resume callback:

static int rzg3e_phy_usb3_resume(struct device *dev)
{
...
	ret = rzg3e_phy_usb3_init_helper(r->base);
	if (ret)
		goto pm_put;

My understanding is that Sashiko's comment ([1]) is about runtime PM and
whether the PM domain will be powered off after the refcount reaches 0,
destroying the PHY state. Which does not happen on our platforms, as the
CPG driver sets GENPD_FLAG_ALWAYS_ON and it only gates/ungates the clock
across runtime suspend/resume, preserving the state.

[1] https://sashiko.dev/#/patchset/20260427194741.161533-1-ovidiu.panait.rb%40renesas.com

Thanks,
Ovidiu

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But
> when I'm talking to journalists I just say "programmer" or something like
> that.
>                                 -- Linus Torvalds
-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy