[PATCH v3] counter: ti-eqep: use devm for runtime PM to fix probe error path

[PATCH v3] counter: ti-eqep: use devm for runtime PM to fix probe error path

[Stepan Ionichev]

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

Re: [PATCH v3] counter: ti-eqep: use devm for runtime PM to fix probe error path

[Andy Shevchenko]

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

Re: [PATCH v3] counter: ti-eqep: use devm for runtime PM to fix probe error path

[William Breathitt Gray]

On Wed, Jun 03, 2026 at 08:42:32AM +0300, Andy Shevchenko wrote:
> 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.

Stepan,

Do you have any disagreements with Andy's suggestions? I'll be happy to
merge a v4 of this patch if you rephrase the description as suggested.

Thanks,

William Breathitt Gray

Re: [PATCH v3] counter: ti-eqep: use devm for runtime PM to

[phucduc.bui]

Hi Stepan,

William pointed me to this thread after I sent a similar patch. It
looks like there has been no v4 since May. Are you still planning to
send one? If not, I'm happy to pick it up, keeping you as the author,
and address the comments from Andy and William.

Best regards,
Phuc

Re: [PATCH v3] counter: ti-eqep: use devm for runtime PM to fix probe error path

[William Breathitt Gray]

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