ti_eqep_probe() enables runtime PM and takes a reference manually,
then returns directly via dev_err_probe() if devm_clk_get_enabled()
fails, leaking the runtime PM enable and usage count. v2 tried to
route that error through a manual cleanup label, but mixing a
devm-managed resource (the clock) with a manual pm_runtime unwind
is itself wrong: the devm clock release runs after the manual
unwind, in the wrong order.

Manage the runtime PM with devm instead: devm_pm_runtime_enable()
for the enable, pm_runtime_resume_and_get() with a devm action for
the get/put. Every probe error path then unwinds through devm in
the correct reverse order, and ti_eqep_remove() no longer has to
touch runtime PM.

Fixes: f213729f6796 ("counter: new TI eQEP driver")
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
v3:
- Manage runtime PM with devm (devm_pm_runtime_enable plus a devm
  action for pm_runtime_put_sync); remove now-empty PM unwind from
  ti_eqep_remove() (Andy)
- Reword commit message: v2 mixed devm with manual unwind, fix is
  full devm conversion

v2: https://lore.kernel.org/all/20260525152137.7428-1-sozdayvek@gmail.com/
v1: https://lore.kernel.org/all/20260523184448.7609-1-sozdayvek@gmail.com/

 drivers/counter/ti-eqep.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/counter/ti-eqep.c b/drivers/counter/ti-eqep.c
index d21c157e531a..b24c8a29201d 100644
--- a/drivers/counter/ti-eqep.c
+++ b/drivers/counter/ti-eqep.c
@@ -492,6 +492,11 @@ static const struct regmap_config ti_eqep_regmap16_config = {
 	.max_register = QCPRDLAT,
 };
 
+static void ti_eqep_pm_put(void *dev)
+{
+	pm_runtime_put_sync(dev);
+}
+
 static int ti_eqep_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -544,19 +549,25 @@ static int ti_eqep_probe(struct platform_device *pdev)
 	 * parent PWMSS bus driver. On AM17xx, this comes from the PSC power
 	 * domain.
 	 */
-	pm_runtime_enable(dev);
-	pm_runtime_get_sync(dev);
+	err = devm_pm_runtime_enable(dev);
+	if (err)
+		return err;
+
+	err = pm_runtime_resume_and_get(dev);
+	if (err)
+		return err;
+
+	err = devm_add_action_or_reset(dev, ti_eqep_pm_put, dev);
+	if (err)
+		return err;
 
 	clk = devm_clk_get_enabled(dev, NULL);
 	if (IS_ERR(clk))
 		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable clock\n");
 
 	err = counter_add(counter);
-	if (err < 0) {
-		pm_runtime_put_sync(dev);
-		pm_runtime_disable(dev);
-		return err;
-	}
+	if (err < 0)
+		return dev_err_probe(dev, err, "Failed to add counter\n");
 
 	return 0;
 }
@@ -564,11 +575,8 @@ static int ti_eqep_probe(struct platform_device *pdev)
 static void ti_eqep_remove(struct platform_device *pdev)
 {
 	struct counter_device *counter = platform_get_drvdata(pdev);
-	struct device *dev = &pdev->dev;
 
 	counter_unregister(counter);
-	pm_runtime_put_sync(dev);
-	pm_runtime_disable(dev);
 }
 
 static const struct of_device_id ti_eqep_of_match[] = {
-- 
2.43.0

On Fri, May 29, 2026 at 02:58:34PM +0500, Stepan Ionichev wrote:
>  	err = counter_add(counter);
> -	if (err < 0) {
> -		pm_runtime_put_sync(dev);
> -		pm_runtime_disable(dev);
> -		return err;
> -	}
> +	if (err < 0)
> +		return dev_err_probe(dev, err, "Failed to add counter\n");
> 
>  	return 0;
>  }
> @@ -564,11 +575,8 @@ static int ti_eqep_probe(struct platform_device *pdev)
>  static void ti_eqep_remove(struct platform_device *pdev)
>  {
>  	struct counter_device *counter = platform_get_drvdata(pdev);
> -	struct device *dev = &pdev->dev;
> 
>  	counter_unregister(counter);
> -	pm_runtime_put_sync(dev);
> -	pm_runtime_disable(dev);
>  }

The ti_eqep_remove() callback can go away entirely if you use
devm_counter_add() to replace counter_add(). It's not really part of
this PM fix, but it is a nice clean up to simplify the code if you would
like to add a subsequent patch doing so after the PM fix.

William Breathitt Gray

On Fri, May 29, 2026 at 02:58:34PM +0500, Stepan Ionichev wrote:
> ti_eqep_probe() enables runtime PM and takes a reference manually,
> then returns directly via dev_err_probe() if devm_clk_get_enabled()
> fails, leaking the runtime PM enable and usage count.

> v2 tried to
> route that error through a manual cleanup label, but mixing a
> devm-managed resource (the clock) with a manual pm_runtime unwind
> is itself wrong: the devm clock release runs after the manual
> unwind, in the wrong order.

It sounds like unneeded detail (because started with what v2 did, which
is part of rather changelog or cover letter). Perhaps we may rephrase
that in a way that "Manual cleanup via goto will mix the managed resource
allocations and unmanaged that leads to a wrong order on the release."

Also note devm-managed is tautology since 'm' is for managed.

> Manage the runtime PM with devm instead: devm_pm_runtime_enable()
> for the enable, pm_runtime_resume_and_get() with a devm action for
> the get/put. Every probe error path then unwinds through devm in
> the correct reverse order, and ti_eqep_remove() no longer has to
> touch runtime PM.

-- 
With Best Regards,
Andy Shevchenko