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

[Ovidiu Panait]

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.

Please let me know what you think.

Thanks!
Ovidiu

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