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

[Manivannan Sadhasivam]

On Fri, Sep 11, 2026 at 08:53:29AM +0000, Ovidiu Panait wrote:
> 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)
> 
> This should ensure the PHY is reinitialized whenever it is used and
> that it stays suspended when it is not used.
> 

Sounds good to me.

- Mani

-- 
மணிவண்ணன் சதாசிவம்