Remove redundant pm_runtime_resume_and_get() and pm_runtime_put() calls
from the reset assert, deassert, and status paths.

These paths do not require runtime PM handling, as power management is
already taken care of during probe and remove.

Additionally, the IP is active only when its clock is enabled.
Previously, the clock was being turned off immediately after register
configuration, which is incorrect. The code may have appeared to work
if another module had incremented the clock usage count, but this
behavior is unreliable.

This change streamlines the code and prevents unnecessary PM and
clock state changes.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/reset/reset-rzv2h-usb2phy.c | 48 +++++++++--------------------
 1 file changed, 15 insertions(+), 33 deletions(-)

diff --git a/drivers/reset/reset-rzv2h-usb2phy.c b/drivers/reset/reset-rzv2h-usb2phy.c
index ae643575b067..7cd559bc52aa 100644
--- a/drivers/reset/reset-rzv2h-usb2phy.c
+++ b/drivers/reset/reset-rzv2h-usb2phy.c
@@ -66,19 +66,9 @@ static int rzv2h_usbphy_reset_assert(struct reset_controller_dev *rcdev,
 				     unsigned long id)
 {
 	struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
-	struct device *dev = priv->dev;
-	int ret;
-
-	ret = pm_runtime_resume_and_get(dev);
-	if (ret) {
-		dev_err(dev, "pm_runtime_resume_and_get failed\n");
-		return ret;
-	}
 
 	rzv2h_usbphy_assert_helper(priv);
 
-	pm_runtime_put(dev);
-
 	return 0;
 }
 
@@ -87,14 +77,6 @@ static int rzv2h_usbphy_reset_deassert(struct reset_controller_dev *rcdev,
 {
 	struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
 	const struct rzv2h_usb2phy_reset_of_data *data = priv->data;
-	struct device *dev = priv->dev;
-	int ret;
-
-	ret = pm_runtime_resume_and_get(dev);
-	if (ret) {
-		dev_err(dev, "pm_runtime_resume_and_get failed\n");
-		return ret;
-	}
 
 	scoped_guard(spinlock, &priv->lock) {
 		writel(data->reset_deassert_val, priv->base + data->reset_reg);
@@ -102,8 +84,6 @@ static int rzv2h_usbphy_reset_deassert(struct reset_controller_dev *rcdev,
 		writel(data->reset_release_val, priv->base + data->reset_reg);
 	}
 
-	pm_runtime_put(dev);
-
 	return 0;
 }
 
@@ -111,20 +91,10 @@ static int rzv2h_usbphy_reset_status(struct reset_controller_dev *rcdev,
 				     unsigned long id)
 {
 	struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev);
-	struct device *dev = priv->dev;
-	int ret;
 	u32 reg;
 
-	ret = pm_runtime_resume_and_get(dev);
-	if (ret) {
-		dev_err(dev, "pm_runtime_resume_and_get failed\n");
-		return ret;
-	}
-
 	reg = readl(priv->base + priv->data->reset_reg);
 
-	pm_runtime_put(dev);
-
 	return (reg & priv->data->reset_status_bits) == priv->data->reset_status_bits;
 }
 
@@ -141,6 +111,11 @@ static int rzv2h_usb2phy_reset_of_xlate(struct reset_controller_dev *rcdev,
 	return 0;
 }
 
+static void rzv2h_usb2phy_reset_pm_runtime_put(void *data)
+{
+	pm_runtime_put(data);
+}
+
 static int rzv2h_usb2phy_reset_probe(struct platform_device *pdev)
 {
 	const struct rzv2h_usb2phy_reset_of_data *data;
@@ -175,14 +150,17 @@ static int rzv2h_usb2phy_reset_probe(struct platform_device *pdev)
 	if (error)
 		return dev_err_probe(dev, error, "pm_runtime_resume_and_get failed\n");
 
+	error = devm_add_action_or_reset(dev, rzv2h_usb2phy_reset_pm_runtime_put,
+					 dev);
+	if (error)
+		return dev_err_probe(dev, error, "unable to register cleanup action\n");
+
 	for (unsigned int i = 0; i < data->init_val_count; i++)
 		writel(data->init_vals[i].val, priv->base + data->init_vals[i].reg);
 
 	/* keep usb2phy in asserted state */
 	rzv2h_usbphy_assert_helper(priv);
 
-	pm_runtime_put(dev);
-
 	priv->rcdev.ops = &rzv2h_usbphy_reset_ops;
 	priv->rcdev.of_reset_n_cells = 0;
 	priv->rcdev.nr_resets = 1;
@@ -190,7 +168,11 @@ static int rzv2h_usb2phy_reset_probe(struct platform_device *pdev)
 	priv->rcdev.of_node = dev->of_node;
 	priv->rcdev.dev = dev;
 
-	return devm_reset_controller_register(dev, &priv->rcdev);
+	error = devm_reset_controller_register(dev, &priv->rcdev);
+	if (error)
+		return dev_err_probe(dev, error, "could not register reset controller\n");
+
+	return 0;
 }
 
 /*
-- 
2.43.0

Hi Tommaso,

On Mi, 2025-10-01 at 23:26 +0200, Tommaso Merciai wrote:
> Remove redundant pm_runtime_resume_and_get() and pm_runtime_put() calls
> from the reset assert, deassert, and status paths.

These calls are only made redundant by this patch.

> These paths do not require runtime PM handling, as power management is
> already taken care of during probe and remove.

Only since you removed the pm_runtime_put() in
rzv2h_usb2phy_reset_probe(). It feels like the important part of this
patch is actually the side note:

> Additionally, the IP is active only when its clock is enabled.
> Previously, the clock was being turned off immediately after register
> configuration, which is incorrect. The code may have appeared to work
> if another module had incremented the clock usage count, but this
> behavior is unreliable.

So this is a reliability fix first and foremost?
The IP must be active to reliably keep reset lines at the configured
level?

If so, please make this clear in the commit subject and description.

regards
Philipp

Hi Philipp,
Thanks for your review.

On Mon, Oct 06, 2025 at 06:21:25PM +0200, Philipp Zabel wrote:
> Hi Tommaso,
> 
> On Mi, 2025-10-01 at 23:26 +0200, Tommaso Merciai wrote:
> > Remove redundant pm_runtime_resume_and_get() and pm_runtime_put() calls
> > from the reset assert, deassert, and status paths.
> 
> These calls are only made redundant by this patch.
> 
> > These paths do not require runtime PM handling, as power management is
> > already taken care of during probe and remove.
> 
> Only since you removed the pm_runtime_put() in
> rzv2h_usb2phy_reset_probe(). It feels like the important part of this
> patch is actually the side note:
> 
> > Additionally, the IP is active only when its clock is enabled.
> > Previously, the clock was being turned off immediately after register
> > configuration, which is incorrect. The code may have appeared to work
> > if another module had incremented the clock usage count, but this
> > behavior is unreliable.
> 
> So this is a reliability fix first and foremost?
> The IP must be active to reliably keep reset lines at the configured
> level?
> 
> If so, please make this clear in the commit subject and description.

The main purpose of this patch is to ensure the USB PHY controller
remain in a proper state by keeping the IP clock enabled and reset
deasserted for the normal operation.

I will make it clear in v2 also adding Fixes tag.

Thanks & Regards,
Tommaso



> 
> regards
> Philipp