The driver was disabling the USB2 PHY clock immediately after register initialization in probe() and after each reset operation. This left the PHY unclocked even though it must remain active for USB functionality.
The behavior appeared to work only when another driver (e.g., USB controller) had already enabled the clock, making operation unreliable and hardware-dependent. In configurations where this driver is the sole clock user, USB functionality would fail.
Fix this by: - Enabling the clock once in probe() via pm_runtime_resume_and_get() - Removing all pm_runtime_put() calls from assert/deassert/status - Registering a devm cleanup action to release the clock at removal - Removed rzv2h_usbphy_assert_helper() and its call in rzv2h_usb2phy_reset_probe()
This ensures the PHY clock remains enabled for the entire device lifetime, preventing instability and aligning with hardware requirements.
Cc: stable@vger.kernel.org Fixes: e3911d7f865b ("reset: Add USB2PHY port reset driver for Renesas RZ/V2H(P)") Signed-off-by: Tommaso Merciai tommaso.merciai.xr@bp.renesas.com --- v5->v6: - No changes
v4->v5: - No changes.
v3->v4: - No changes.
v2->v3: - Added missing Cc: stable@vger.kernel.org - Improved commit body describing the removal of rzv2h_usbphy_assert_helper() from rzv2h_usb2phy_reset_probe().
v1->v2: - Improve commit body and commit msg - Added Fixes tag - Dropped unnecessary rzv2h_usbphy_assert_helper() function
drivers/reset/reset-rzv2h-usb2phy.c | 64 ++++++++--------------------- 1 file changed, 18 insertions(+), 46 deletions(-)
diff --git a/drivers/reset/reset-rzv2h-usb2phy.c b/drivers/reset/reset-rzv2h-usb2phy.c index ae643575b067..5bdd39274612 100644 --- a/drivers/reset/reset-rzv2h-usb2phy.c +++ b/drivers/reset/reset-rzv2h-usb2phy.c @@ -49,9 +49,10 @@ static inline struct rzv2h_usb2phy_reset_priv return container_of(rcdev, struct rzv2h_usb2phy_reset_priv, rcdev); }
-/* This function must be called only after pm_runtime_resume_and_get() has been called */ -static void rzv2h_usbphy_assert_helper(struct rzv2h_usb2phy_reset_priv *priv) +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); const struct rzv2h_usb2phy_reset_of_data *data = priv->data;
scoped_guard(spinlock, &priv->lock) { @@ -60,24 +61,6 @@ static void rzv2h_usbphy_assert_helper(struct rzv2h_usb2phy_reset_priv *priv) }
usleep_range(11, 20); -} - -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 +70,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 +77,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 +84,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 +104,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 +143,14 @@ 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 +158,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; }
/*