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

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

[Ovidiu Panait]

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>
---
 drivers/phy/renesas/phy-rzg3e-usb3.c | 31 ++++++++++++++++------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/phy/renesas/phy-rzg3e-usb3.c b/drivers/phy/renesas/phy-rzg3e-usb3.c
index 6b3453ea0004..055775e1a0f7 100644
--- a/drivers/phy/renesas/phy-rzg3e-usb3.c
+++ b/drivers/phy/renesas/phy-rzg3e-usb3.c
@@ -64,6 +64,7 @@
 #define USB3_TEST_LANECONFIG0_DEFAULT		(0xd)
 
 struct rz_usb3 {
+	struct device *dev;
 	void __iomem *base;
 	struct reset_control *rstc;
 	bool skip_reinit;
@@ -130,11 +131,21 @@ static int rzg3e_phy_usb3test_phy_init(void __iomem *base)
 	return 0;
 }
 
-static int rzg3e_phy_usb3_init_helper(void __iomem *base)
+static int rzg3e_phy_usb3_init_helper(struct rz_usb3 *r)
 {
-	rzg3e_phy_usb2test_phy_init(base);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(r->dev);
+	if (ret)
+		return ret;
+
+	rzg3e_phy_usb2test_phy_init(r->base);
 
-	return rzg3e_phy_usb3test_phy_init(base);
+	ret = rzg3e_phy_usb3test_phy_init(r->base);
+
+	pm_runtime_put_sync(r->dev);
+
+	return ret;
 }
 
 static int rzg3e_phy_usb3_init(struct phy *p)
@@ -143,7 +154,7 @@ static int rzg3e_phy_usb3_init(struct phy *p)
 	int ret = 0;
 
 	if (!r->skip_reinit)
-		ret = rzg3e_phy_usb3_init_helper(r->base);
+		ret = rzg3e_phy_usb3_init_helper(r);
 
 	return ret;
 }
@@ -187,6 +198,7 @@ static int rzg3e_phy_usb3_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, r);
 	phy_set_drvdata(phy, r);
+	r->dev = dev;
 
 	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
 	if (IS_ERR(provider))
@@ -199,7 +211,6 @@ static int rzg3e_phy_usb3_suspend(struct device *dev)
 {
 	struct rz_usb3 *r = dev_get_drvdata(dev);
 
-	pm_runtime_put(dev);
 	reset_control_assert(r->rstc);
 	r->skip_reinit = false;
 
@@ -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;
 
 	return 0;
 
-pm_put:
-	pm_runtime_put(dev);
 reset_assert:
 	reset_control_assert(r->rstc);
 	return ret;
 }
 
 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

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

[Biju Das]

Hi Ovidiu,

Thanks for the patch

> -----Original Message-----
> From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> Sent: 27 April 2026 20:48
> Subject: [PATCH] phy: renesas: phy-rzg3e-usb3: Fix runtime PM underflow during suspend
> 
> 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>
> ---
>  drivers/phy/renesas/phy-rzg3e-usb3.c | 31 ++++++++++++++++------------
>  1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/phy/renesas/phy-rzg3e-usb3.c b/drivers/phy/renesas/phy-rzg3e-usb3.c
> index 6b3453ea0004..055775e1a0f7 100644
> --- a/drivers/phy/renesas/phy-rzg3e-usb3.c
> +++ b/drivers/phy/renesas/phy-rzg3e-usb3.c
> @@ -64,6 +64,7 @@
>  #define USB3_TEST_LANECONFIG0_DEFAULT		(0xd)
> 
>  struct rz_usb3 {
> +	struct device *dev;

Do you need dev as struct phy has dev member [1]?

[1]
https://elixir.bootlin.com/linux/v6.0-rc4/source/include/linux/phy/phy.h#L153

>  	void __iomem *base;
>  	struct reset_control *rstc;
>  	bool skip_reinit;
> @@ -130,11 +131,21 @@ static int rzg3e_phy_usb3test_phy_init(void __iomem *base)
>  	return 0;
>  }
> 
> -static int rzg3e_phy_usb3_init_helper(void __iomem *base)

you can add one more function parameter dev here.

static int rzg3e_phy_usb3_init_helper(struct device *dev, void __iomem *base)

> +static int rzg3e_phy_usb3_init_helper(struct rz_usb3 *r)
>  {
> -	rzg3e_phy_usb2test_phy_init(base);
> +	int ret;
> +
> +	ret = pm_runtime_resume_and_get(r->dev);
> +	if (ret)
> +		return ret;
> +
> +	rzg3e_phy_usb2test_phy_init(r->base);
> 
> -	return rzg3e_phy_usb3test_phy_init(base);
> +	ret = rzg3e_phy_usb3test_phy_init(r->base);
> +
> +	pm_runtime_put_sync(r->dev);
> +
> +	return ret;
>  }
> 
>  static int rzg3e_phy_usb3_init(struct phy *p) @@ -143,7 +154,7 @@ static int
> rzg3e_phy_usb3_init(struct phy *p)
>  	int ret = 0;
> 
>  	if (!r->skip_reinit)
> -		ret = rzg3e_phy_usb3_init_helper(r->base);
> +		ret = rzg3e_phy_usb3_init_helper(r);

ret = rzg3e_phy_usb3_init_helper(&p->dev, r->base);

> 
>  	return ret;
>  }
> @@ -187,6 +198,7 @@ static int rzg3e_phy_usb3_probe(struct platform_device *pdev)
> 
>  	platform_set_drvdata(pdev, r);

>  	phy_set_drvdata(phy, r);
> +	r->dev = dev;

Drop this.

> 
>  	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>  	if (IS_ERR(provider))
> @@ -199,7 +211,6 @@ static int rzg3e_phy_usb3_suspend(struct device *dev)  {
>  	struct rz_usb3 *r = dev_get_drvdata(dev);
> 
> -	pm_runtime_put(dev);
>  	reset_control_assert(r->rstc);
>  	r->skip_reinit = false;
> 
> @@ -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);

	ret = rzg3e_phy_usb3_init_helper(dev, r->base);

With the above addressed,

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

Cheers,
Biju

>  	if (ret)
>  		goto reset_assert;
> 
> -	ret = rzg3e_phy_usb3_init_helper(r->base);
> -	if (ret)
> -		goto pm_put;
> -
>  	r->skip_reinit = true;
> 
>  	return 0;
> 
> -pm_put:
> -	pm_runtime_put(dev);
>  reset_assert:
>  	reset_control_assert(r->rstc);
>  	return ret;
>  }
> 
>  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

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

[Ovidiu Panait]

Hi Biju,

> 
> Hi Ovidiu,
> 
> Thanks for the patch
> 
> > -----Original Message-----
> > From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > Sent: 27 April 2026 20:48
> > Subject: [PATCH] phy: renesas: phy-rzg3e-usb3: Fix runtime PM underflow
> during suspend
> >
> > 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>
> > ---
> >  drivers/phy/renesas/phy-rzg3e-usb3.c | 31 ++++++++++++++++------------
> >  1 file changed, 18 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/phy/renesas/phy-rzg3e-usb3.c
> b/drivers/phy/renesas/phy-rzg3e-usb3.c
> > index 6b3453ea0004..055775e1a0f7 100644
> > --- a/drivers/phy/renesas/phy-rzg3e-usb3.c
> > +++ b/drivers/phy/renesas/phy-rzg3e-usb3.c
> > @@ -64,6 +64,7 @@
> >  #define USB3_TEST_LANECONFIG0_DEFAULT		(0xd)
> >
> >  struct rz_usb3 {
> > +	struct device *dev;
> 
> Do you need dev as struct phy has dev member [1]?
> 
> [1]
> https://elixir.bootlin.com/linux/v6.0-
> rc4/source/include/linux/phy/phy.h#L153
> 
> >  	void __iomem *base;
> >  	struct reset_control *rstc;
> >  	bool skip_reinit;
> > @@ -130,11 +131,21 @@ static int rzg3e_phy_usb3test_phy_init(void
> __iomem *base)
> >  	return 0;
> >  }
> >
> > -static int rzg3e_phy_usb3_init_helper(void __iomem *base)
> 
> you can add one more function parameter dev here.
> 
> static int rzg3e_phy_usb3_init_helper(struct device *dev, void __iomem
> *base)
> 
> > +static int rzg3e_phy_usb3_init_helper(struct rz_usb3 *r)
> >  {
> > -	rzg3e_phy_usb2test_phy_init(base);
> > +	int ret;
> > +
> > +	ret = pm_runtime_resume_and_get(r->dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	rzg3e_phy_usb2test_phy_init(r->base);
> >
> > -	return rzg3e_phy_usb3test_phy_init(base);
> > +	ret = rzg3e_phy_usb3test_phy_init(r->base);
> > +
> > +	pm_runtime_put_sync(r->dev);
> > +
> > +	return ret;
> >  }
> >
> >  static int rzg3e_phy_usb3_init(struct phy *p) @@ -143,7 +154,7 @@
> static int
> > rzg3e_phy_usb3_init(struct phy *p)
> >  	int ret = 0;
> >
> >  	if (!r->skip_reinit)
> > -		ret = rzg3e_phy_usb3_init_helper(r->base);
> > +		ret = rzg3e_phy_usb3_init_helper(r);
> 
> ret = rzg3e_phy_usb3_init_helper(&p->dev, r->base);
> 
> >
> >  	return ret;
> >  }
> > @@ -187,6 +198,7 @@ static int rzg3e_phy_usb3_probe(struct
> platform_device *pdev)
> >
> >  	platform_set_drvdata(pdev, r);
> 
> >  	phy_set_drvdata(phy, r);
> > +	r->dev = dev;
> 
> Drop this.
> 
> >
> >  	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> >  	if (IS_ERR(provider))
> > @@ -199,7 +211,6 @@ static int rzg3e_phy_usb3_suspend(struct device
> *dev)  {
> >  	struct rz_usb3 *r = dev_get_drvdata(dev);
> >
> > -	pm_runtime_put(dev);
> >  	reset_control_assert(r->rstc);
> >  	r->skip_reinit = false;
> >
> > @@ -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);
> 
> 	ret = rzg3e_phy_usb3_init_helper(dev, r->base);

"dev" is the platform device, which is different from phy->dev. Phy->dev
is a child of the platform device.

We could be referencing the platform device from phy->dev with &phy->dev.parent,
but I don't think that will make the code cleaner.

Considering this, I think the current implementation is fine.

Ovidiu


> 
> With the above addressed,
> 
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Cheers,
> Biju
> 
> >  	if (ret)
> >  		goto reset_assert;
> >
> > -	ret = rzg3e_phy_usb3_init_helper(r->base);
> > -	if (ret)
> > -		goto pm_put;
> > -
> >  	r->skip_reinit = true;
> >
> >  	return 0;
> >
> > -pm_put:
> > -	pm_runtime_put(dev);
> >  reset_assert:
> >  	reset_control_assert(r->rstc);
> >  	return ret;
> >  }
> >
> >  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

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

[Biju Das]

Hi Ovidiu,

> -----Original Message-----
> From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> Sent: 28 April 2026 10:21
> Subject: RE: [PATCH] phy: renesas: phy-rzg3e-usb3: Fix runtime PM underflow during suspend
> 
> Hi Biju,
> 
> >
> > Hi Ovidiu,
> >
> > Thanks for the patch
> >
> > > -----Original Message-----
> > > From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > > Sent: 27 April 2026 20:48
> > > Subject: [PATCH] phy: renesas: phy-rzg3e-usb3: Fix runtime PM
> > > underflow
> > during suspend
> > >
> > > 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>
> > > ---
> > >  drivers/phy/renesas/phy-rzg3e-usb3.c | 31
> > > ++++++++++++++++------------
> > >  1 file changed, 18 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/phy/renesas/phy-rzg3e-usb3.c
> > b/drivers/phy/renesas/phy-rzg3e-usb3.c
> > > index 6b3453ea0004..055775e1a0f7 100644
> > > --- a/drivers/phy/renesas/phy-rzg3e-usb3.c
> > > +++ b/drivers/phy/renesas/phy-rzg3e-usb3.c
> > > @@ -64,6 +64,7 @@
> > >  #define USB3_TEST_LANECONFIG0_DEFAULT		(0xd)
> > >
> > >  struct rz_usb3 {
> > > +	struct device *dev;
> >
> > Do you need dev as struct phy has dev member [1]?
> >
> > [1]
> > https://elixir.bootlin.com/linux/v6.0-
> > rc4/source/include/linux/phy/phy.h#L153
> >
> > >  	void __iomem *base;
> > >  	struct reset_control *rstc;
> > >  	bool skip_reinit;
> > > @@ -130,11 +131,21 @@ static int rzg3e_phy_usb3test_phy_init(void
> > __iomem *base)
> > >  	return 0;
> > >  }
> > >
> > > -static int rzg3e_phy_usb3_init_helper(void __iomem *base)
> >
> > you can add one more function parameter dev here.
> >
> > static int rzg3e_phy_usb3_init_helper(struct device *dev, void __iomem
> > *base)
> >
> > > +static int rzg3e_phy_usb3_init_helper(struct rz_usb3 *r)
> > >  {
> > > -	rzg3e_phy_usb2test_phy_init(base);
> > > +	int ret;
> > > +
> > > +	ret = pm_runtime_resume_and_get(r->dev);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	rzg3e_phy_usb2test_phy_init(r->base);
> > >
> > > -	return rzg3e_phy_usb3test_phy_init(base);
> > > +	ret = rzg3e_phy_usb3test_phy_init(r->base);
> > > +
> > > +	pm_runtime_put_sync(r->dev);
> > > +
> > > +	return ret;
> > >  }
> > >
> > >  static int rzg3e_phy_usb3_init(struct phy *p) @@ -143,7 +154,7 @@
> > static int
> > > rzg3e_phy_usb3_init(struct phy *p)
> > >  	int ret = 0;
> > >
> > >  	if (!r->skip_reinit)
> > > -		ret = rzg3e_phy_usb3_init_helper(r->base);
> > > +		ret = rzg3e_phy_usb3_init_helper(r);
> >
> > ret = rzg3e_phy_usb3_init_helper(&p->dev, r->base);
> >
> > >
> > >  	return ret;
> > >  }
> > > @@ -187,6 +198,7 @@ static int rzg3e_phy_usb3_probe(struct
> > platform_device *pdev)
> > >
> > >  	platform_set_drvdata(pdev, r);
> >
> > >  	phy_set_drvdata(phy, r);
> > > +	r->dev = dev;
> >
> > Drop this.
> >
> > >
> > >  	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> > >  	if (IS_ERR(provider))
> > > @@ -199,7 +211,6 @@ static int rzg3e_phy_usb3_suspend(struct device
> > *dev)  {
> > >  	struct rz_usb3 *r = dev_get_drvdata(dev);
> > >
> > > -	pm_runtime_put(dev);
> > >  	reset_control_assert(r->rstc);
> > >  	r->skip_reinit = false;
> > >
> > > @@ -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);
> >
> > 	ret = rzg3e_phy_usb3_init_helper(dev, r->base);
> 
> "dev" is the platform device, which is different from phy->dev. Phy->dev is a child of the platform
> device.

You are correct. I missed that.

> 
> We could be referencing the platform device from phy->dev with &phy->dev.parent, but I don't think that
> will make the code cleaner.

Thought of avoiding another global variable that saves 4 bytes.

Changing "&phy->dev"->""&phy->dev.parent" is not making the code cleaner
then leave as it is.

Cheers,
Biju

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

[Vinod Koul]

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.

Ok

> 
> 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.

This is a fix, so split this up please

> 
> Fixes: ee5f1a3f90a4 ("phy: renesas: Add Renesas RZ/G3E USB3.0 PHY driver")
> Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> ---
>  drivers/phy/renesas/phy-rzg3e-usb3.c | 31 ++++++++++++++++------------
>  1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/phy/renesas/phy-rzg3e-usb3.c b/drivers/phy/renesas/phy-rzg3e-usb3.c
> index 6b3453ea0004..055775e1a0f7 100644
> --- a/drivers/phy/renesas/phy-rzg3e-usb3.c
> +++ b/drivers/phy/renesas/phy-rzg3e-usb3.c
> @@ -64,6 +64,7 @@
>  #define USB3_TEST_LANECONFIG0_DEFAULT		(0xd)
>  
>  struct rz_usb3 {
> +	struct device *dev;

This does not belong in a fix, please split

>  	void __iomem *base;
>  	struct reset_control *rstc;
>  	bool skip_reinit;
> @@ -130,11 +131,21 @@ static int rzg3e_phy_usb3test_phy_init(void __iomem *base)
>  	return 0;
>  }
>  
> -static int rzg3e_phy_usb3_init_helper(void __iomem *base)
> +static int rzg3e_phy_usb3_init_helper(struct rz_usb3 *r)
>  {
> -	rzg3e_phy_usb2test_phy_init(base);
> +	int ret;
> +
> +	ret = pm_runtime_resume_and_get(r->dev);
> +	if (ret)
> +		return ret;
> +
> +	rzg3e_phy_usb2test_phy_init(r->base);
>  
> -	return rzg3e_phy_usb3test_phy_init(base);
> +	ret = rzg3e_phy_usb3test_phy_init(r->base);
> +
> +	pm_runtime_put_sync(r->dev);
> +
> +	return ret;
>  }
>  
>  static int rzg3e_phy_usb3_init(struct phy *p)
> @@ -143,7 +154,7 @@ static int rzg3e_phy_usb3_init(struct phy *p)
>  	int ret = 0;
>  
>  	if (!r->skip_reinit)
> -		ret = rzg3e_phy_usb3_init_helper(r->base);
> +		ret = rzg3e_phy_usb3_init_helper(r);
>  
>  	return ret;
>  }
> @@ -187,6 +198,7 @@ static int rzg3e_phy_usb3_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, r);
>  	phy_set_drvdata(phy, r);
> +	r->dev = dev;
>  
>  	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>  	if (IS_ERR(provider))
> @@ -199,7 +211,6 @@ static int rzg3e_phy_usb3_suspend(struct device *dev)
>  {
>  	struct rz_usb3 *r = dev_get_drvdata(dev);
>  
> -	pm_runtime_put(dev);
>  	reset_control_assert(r->rstc);
>  	r->skip_reinit = false;
>  
> @@ -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


-- 
~Vinod

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

[Ovidiu Panait]

Hi Vinod,

> 
> 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.
> 
> Ok
> 
> >
> > 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.
> 
> This is a fix, so split this up please
> 
> >
> > Fixes: ee5f1a3f90a4 ("phy: renesas: Add Renesas RZ/G3E USB3.0 PHY
> driver")
> > Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > ---
> >  drivers/phy/renesas/phy-rzg3e-usb3.c | 31 ++++++++++++++++------------
> >  1 file changed, 18 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/phy/renesas/phy-rzg3e-usb3.c
> b/drivers/phy/renesas/phy-rzg3e-usb3.c
> > index 6b3453ea0004..055775e1a0f7 100644
> > --- a/drivers/phy/renesas/phy-rzg3e-usb3.c
> > +++ b/drivers/phy/renesas/phy-rzg3e-usb3.c
> > @@ -64,6 +64,7 @@
> >  #define USB3_TEST_LANECONFIG0_DEFAULT		(0xd)
> >
> >  struct rz_usb3 {
> > +	struct device *dev;
> 
> This does not belong in a fix, please split
> 

Thanks for your review, I'll split this up into multiple patches.

> >  	void __iomem *base;
> >  	struct reset_control *rstc;
> >  	bool skip_reinit;
> > @@ -130,11 +131,21 @@ static int rzg3e_phy_usb3test_phy_init(void
> __iomem *base)
> >  	return 0;
> >  }
> >
> > -static int rzg3e_phy_usb3_init_helper(void __iomem *base)
> > +static int rzg3e_phy_usb3_init_helper(struct rz_usb3 *r)
> >  {
> > -	rzg3e_phy_usb2test_phy_init(base);
> > +	int ret;
> > +
> > +	ret = pm_runtime_resume_and_get(r->dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	rzg3e_phy_usb2test_phy_init(r->base);
> >
> > -	return rzg3e_phy_usb3test_phy_init(base);
> > +	ret = rzg3e_phy_usb3test_phy_init(r->base);
> > +
> > +	pm_runtime_put_sync(r->dev);
> > +
> > +	return ret;
> >  }
> >
> >  static int rzg3e_phy_usb3_init(struct phy *p)
> > @@ -143,7 +154,7 @@ static int rzg3e_phy_usb3_init(struct phy *p)
> >  	int ret = 0;
> >
> >  	if (!r->skip_reinit)
> > -		ret = rzg3e_phy_usb3_init_helper(r->base);
> > +		ret = rzg3e_phy_usb3_init_helper(r);
> >
> >  	return ret;
> >  }
> > @@ -187,6 +198,7 @@ static int rzg3e_phy_usb3_probe(struct
> platform_device *pdev)
> >
> >  	platform_set_drvdata(pdev, r);
> >  	phy_set_drvdata(phy, r);
> > +	r->dev = dev;
> >
> >  	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> >  	if (IS_ERR(provider))
> > @@ -199,7 +211,6 @@ static int rzg3e_phy_usb3_suspend(struct device
> *dev)
> >  {
> >  	struct rz_usb3 *r = dev_get_drvdata(dev);
> >
> > -	pm_runtime_put(dev);
> >  	reset_control_assert(r->rstc);
> >  	r->skip_reinit = false;
> >
> > @@ -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.

Thanks,
Ovidiu

> 
> --
> ~Vinod

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

[Geert Uytterhoeven]

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?

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

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

[Ovidiu Panait]

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