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

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

[Ovidiu Panait]

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.

Fixes: ee5f1a3f90a4 ("phy: renesas: Add Renesas RZ/G3E USB3.0 PHY driver")
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
v2:
- Dropped the runtime PM rework part from the previous version, as it
  was not needed to fix the original issue.

v1: https://lore.kernel.org/all/20260427194741.161533-1-ovidiu.panait.rb@renesas.com/

 drivers/phy/renesas/phy-rzg3e-usb3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/renesas/phy-rzg3e-usb3.c b/drivers/phy/renesas/phy-rzg3e-usb3.c
index 060309547ea5..4d2b0f5f5b09 100644
--- a/drivers/phy/renesas/phy-rzg3e-usb3.c
+++ b/drivers/phy/renesas/phy-rzg3e-usb3.c
@@ -233,7 +233,7 @@ static int rzg3e_phy_usb3_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops rzg3e_phy_usb3_pm = {
-	NOIRQ_SYSTEM_SLEEP_PM_OPS(rzg3e_phy_usb3_suspend, rzg3e_phy_usb3_resume)
+	SYSTEM_SLEEP_PM_OPS(rzg3e_phy_usb3_suspend, rzg3e_phy_usb3_resume)
 };
 
 static const struct of_device_id rzg3e_phy_usb3_match_table[] = {
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver manually manipulates the runtime PM usage count inside sleep callbacks, which defeats PM core synchronization and causes underflows on error paths.
- [High] Asynchronous pm_runtime_put() is used immediately before asserting a hardware reset, violating hardware ordering constraints and causing race conditions.
- [High] Flawed skip_reinit state machine leaves the PHY uninitialized if it runtime suspends after system resume.
--

commit cdc6f90932d427eedd494d80964982fd1f09abcb
Author: Ovidiu Panait <ovidiu.panait.rb@renesas.com>

phy: renesas: phy-rzg3e-usb3: Use SYSTEM_SLEEP_PM_OPS instead of NOIRQ

This patch moves the suspend/resume callbacks out of the noirq phase by
switching from NOIRQ_SYSTEM_SLEEP_PM_OPS to SYSTEM_SLEEP_PM_OPS. This aims
to fix a PM usage count underflow on suspend when the xhci driver is
unbound.

> diff --git a/drivers/phy/renesas/phy-rzg3e-usb3.c b/drivers/phy/renesas/phy-rzg3e-usb3.c
> index 6b3453ea0004c..7867ca3bc3ab2 100644
> --- a/drivers/phy/renesas/phy-rzg3e-usb3.c
> +++ b/drivers/phy/renesas/phy-rzg3e-usb3.c
> @@ -235,7 +235,7 @@ static int rzg3e_phy_usb3_resume(struct device *dev)
>  }
>  
>  static const struct dev_pm_ops rzg3e_phy_usb3_pm = {
> -	NOIRQ_SYSTEM_SLEEP_PM_OPS(rzg3e_phy_usb3_suspend, rzg3e_phy_usb3_resume)
> +	SYSTEM_SLEEP_PM_OPS(rzg3e_phy_usb3_suspend, rzg3e_phy_usb3_resume)
>  };

[Severity: High]
This isn't a bug introduced by this patch, but modifying these callbacks
brings up a preexisting issue. Does the manual manipulation of runtime PM
usage counts inside these sleep callbacks defeat the PM core's
synchronization?

The PM core takes a runtime PM reference on all devices during
device_prepare() to prevent concurrent transitions during system sleep, and
releases it in device_complete().

If rzg3e_phy_usb3_suspend() manually drops this reference, and
rzg3e_phy_usb3_resume() attempts to reacquire it but fails early on an error
path:

static int rzg3e_phy_usb3_resume(struct device *dev)
{
        ...
        ret = reset_control_deassert(r->rstc);
        if (ret)
                return ret;

        ret = pm_runtime_resume_and_get(dev);
        ...

Will this cause another negative usage count underflow when the PM core
unconditionally calls device_complete() and executes its own
pm_runtime_put()?

[Severity: High]
This is a pre-existing issue, but does the suspend path introduce a race
condition by using an asynchronous runtime put just before asserting the
hardware reset?

In rzg3e_phy_usb3_suspend(), the driver drops the runtime PM reference via
pm_runtime_put() and then immediately asserts the hardware reset line.
Because pm_runtime_put() is asynchronous, could the generic PM domain be
powered off concurrently with or immediately after the hardware reset is
synchronously asserted?

[Severity: High]
This is a pre-existing issue, but does the skip_reinit state machine leave
the PHY uninitialized if it runtime suspends after system resume?

During system resume, rzg3e_phy_usb3_resume() unconditionally sets
r->skip_reinit = true. This flag is only cleared during system suspend.

If the PHY consumer unbinds or runtime suspends, the PHY loses its hardware
state. When the consumer is later bound or runtime resumed, it calls
phy_init() to reinitialize. Since r->skip_reinit is still true, will
phy_init() skip the initialization sequence and leave the hardware broken
until the next full system suspend cycle?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527082052.117289-1-ovidiu.panait.rb@renesas.com?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Biju Das]

Hi Ovidiu,

Thanks for the patch.

> -----Original Message-----
> From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> Sent: 27 May 2026 09:21
> Subject: [PATCH v2] phy: renesas: phy-rzg3e-usb3: Use SYSTEM_SLEEP_PM_OPS instead of NOIRQ
> 
> 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.
> 
> Fixes: ee5f1a3f90a4 ("phy: renesas: Add Renesas RZ/G3E USB3.0 PHY driver")
> Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>

Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Cheers,
Biju

> ---
> v2:
> - Dropped the runtime PM rework part from the previous version, as it
>   was not needed to fix the original issue.
> 
> v1: https://lore.kernel.org/all/20260427194741.161533-1-ovidiu.panait.rb@renesas.com/
> 
>  drivers/phy/renesas/phy-rzg3e-usb3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/renesas/phy-rzg3e-usb3.c b/drivers/phy/renesas/phy-rzg3e-usb3.c
> index 060309547ea5..4d2b0f5f5b09 100644
> --- a/drivers/phy/renesas/phy-rzg3e-usb3.c
> +++ b/drivers/phy/renesas/phy-rzg3e-usb3.c
> @@ -233,7 +233,7 @@ static int rzg3e_phy_usb3_resume(struct device *dev)  }
> 
>  static const struct dev_pm_ops rzg3e_phy_usb3_pm = {
> -	NOIRQ_SYSTEM_SLEEP_PM_OPS(rzg3e_phy_usb3_suspend, rzg3e_phy_usb3_resume)
> +	SYSTEM_SLEEP_PM_OPS(rzg3e_phy_usb3_suspend, rzg3e_phy_usb3_resume)
>  };
> 
>  static const struct of_device_id rzg3e_phy_usb3_match_table[] = {
> --
> 2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy