RE: [PATCH v2] phy: renesas: phy-rzg3e-usb3: Use SYSTEM_SLEEP_PM_OPS instead of NOIRQ

[Biju Das]

> -----Original Message-----
> From: Ovidiu Panait <ovidiu.panait.rb@xxxxxxxxxxx>
> Sent: 11 September 2026 09:53
> To: Manivannan Sadhasivam <mani@xxxxxxxxxx>
> Cc: Vinod Koul <vkoul@xxxxxxxxxx>; Neil Armstrong <neil.armstrong@xxxxxxxxxx>; Biju Das
> <biju.das.jz@xxxxxxxxxxxxxx>; Geert Uytterhoeven <geert+renesas@xxxxxxxxx>; linux-
> phy@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-renesas-soc@xxxxxxxxxxxxxxx
> Subject: RE: [PATCH v2] phy: renesas: phy-rzg3e-usb3: Use SYSTEM_SLEEP_PM_OPS instead of NOIRQ
> 
> Hi Manivannan,
> 
> >
> > On Wed, May 27, 2026 at 08:20:52AM +0000, Ovidiu Panait wrote:
> > > On the Renesas RZ/V2H platform, if the xhci 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!
> > >
> > > When the xhci driver is unbound, the runtime PM usage count of the
> > > PHY drops to 0. Since the suspend/resume callbacks are in the noirq
> > > phase, the pm_runtime_resume_and_get() on resume errors out with
> > > -EACCES and the usage count stays at 0. The subsequent put in
> > > device_complete() then underflows the counter.
> > >
> > > To fix this, move the suspend/resume callbacks out of the noirq
> > > phase by switching from NOIRQ_SYSTEM_SLEEP_PM_OPS to SYSTEM_SLEEP_PM_OPS.
> > >
> >
> > While this patch fixes the runtime PM underflow, it does not
> > necessarily fix the underlying issue. The issue here is that the PHY
> > was already suspended due to xhci driver unbound and the
> > pm_runtime_resume_and_get() in
> > rzg3e_phy_usb3_resume() unnecessarily tries to wakes up the PHY.
> >
> > I think what you need is pm_runtime_force_{suspend/resume} to
> > conditionally resume the PHY in its rzg3e_phy_usb3_resume() callback.
> >
> 
> I agree, the PHY should not be resumed if there is no consumer.
> 
> However, the suspend callback asserts the PHY reset, so its state is
> always lost across system suspend. Resuming the PHY with
> pm_runtime_force_resume() in rzg3e_phy_usb3_resume() might leave it
> uninitialized if it was runtime suspended, and relies on the consumer
> always calling phy_init() again before using it. I am not sure that is
> always the case.
> 
> The cleanest solution I see is to move the reset assert/deassert and
> the PHY initialization to runtime PM callbacks, and implement the
> system sleep as:
> 
> NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)

Is this behaviour tested on RZ/G3E with s2ram?

Cheers,
Biju

> 
> This should ensure the PHY is reinitialized whenever it is used and
> that it stays suspended when it is not used.
> 
> Please let me know what you think.
> 
> Thanks!
> Ovidiu
> 
> > - Mani
> >
> > --
> > மணிவண்ணன் சதாசிவம்