[PATCH v4 0/2] counter: ti-eqep: Fix runtime PM leak and use devm_counter_add()

[PATCH v4 0/2] counter: ti-eqep: Fix runtime PM leak and use devm_counter_add()

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Hi,

This series picks up Stepan's v3 [1], which had no follow-up since
May, with William's agreement [2].

v4:
 - Reword commit message of patch 1 as suggested by Andy
 - Add patch 2 to use devm_counter_add() and drop ti_eqep_remove()
   as suggested by William.

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

[1] https://lore.kernel.org/all/20260529095834.2561-1-sozdayvek@gmail.com/
[2] https://lore.kernel.org/all/20261106193208.4136410-1-wbg@kernel.org/

Compile-tested only.

Best regards,
Phuc

Stepan Ionichev (1):
  counter: ti-eqep: Fix runtime PM leak on probe failure

bui duc phuc (1):
  counter: ti-eqep: Use devm_counter_add()

 drivers/counter/ti-eqep.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

-- 
2.43.0

[PATCH v4 1/2] counter: ti-eqep: Fix runtime PM leak on probe failure

[phucduc.bui]

From: Stepan Ionichev <sozdayvek@gmail.com>

ti_eqep_probe() enables runtime PM and takes a usage reference manually,
then returns directly via dev_err_probe() if devm_clk_get_enabled()
fails, leaking both.

Manual cleanup via goto would mix managed and unmanaged resources,
which leads to the wrong release order. Instead, use
devm_pm_runtime_enable() and pm_runtime_resume_and_get() with a devm
action for the put, so every probe error path unwinds in the correct
reverse order. As a side effect, a runtime resume failure now aborts
the probe instead of being ignored.

ti_eqep_remove() no longer needs to touch runtime PM. While at it,
report counter_add() failures with dev_err_probe().

Fixes: f213729f6796 ("counter: new TI eQEP driver")
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
[phuc: reworded commit message]
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---

v4:
 - Reword commit message of patch as suggested by Andy

 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 0dda3f2b14c7..2326515d5585 100644
--- a/drivers/counter/ti-eqep.c
+++ b/drivers/counter/ti-eqep.c
@@ -491,6 +491,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;
@@ -543,19 +548,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;
 }
@@ -563,11 +574,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

[PATCH v4 2/2] counter: ti-eqep: Use devm_counter_add()

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Use devm_counter_add() instead of counter_add() to automatically
unregister the counter when the device is removed or probe fails.

This makes ti_eqep_remove() unnecessary, so remove it.

Suggested-by: William Breathitt Gray <wbg@kernel.org>
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/counter/ti-eqep.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/counter/ti-eqep.c b/drivers/counter/ti-eqep.c
index 2326515d5585..1c26008846a0 100644
--- a/drivers/counter/ti-eqep.c
+++ b/drivers/counter/ti-eqep.c
@@ -564,20 +564,13 @@ static int ti_eqep_probe(struct platform_device *pdev)
 	if (IS_ERR(clk))
 		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable clock\n");
 
-	err = counter_add(counter);
+	err = devm_counter_add(dev, counter);
 	if (err < 0)
 		return dev_err_probe(dev, err, "failed to add counter\n");
 
 	return 0;
 }
 
-static void ti_eqep_remove(struct platform_device *pdev)
-{
-	struct counter_device *counter = platform_get_drvdata(pdev);
-
-	counter_unregister(counter);
-}
-
 static const struct of_device_id ti_eqep_of_match[] = {
 	{ .compatible = "ti,am3352-eqep", },
 	{ .compatible = "ti,am62-eqep", },
@@ -587,7 +580,6 @@ MODULE_DEVICE_TABLE(of, ti_eqep_of_match);
 
 static struct platform_driver ti_eqep_driver = {
 	.probe = ti_eqep_probe,
-	.remove = ti_eqep_remove,
 	.driver = {
 		.name = "ti-eqep-cnt",
 		.of_match_table = ti_eqep_of_match,
-- 
2.43.0

Re: [PATCH v4 1/2] counter: ti-eqep: Fix runtime PM leak on probe failure

[Andy Shevchenko]

On Mon, Nov 09, 2026 at 03:43:41PM +0700, phucduc.bui@gmail.com wrote:

> ti_eqep_probe() enables runtime PM and takes a usage reference manually,
> then returns directly via dev_err_probe() if devm_clk_get_enabled()
> fails, leaking both.
> 
> Manual cleanup via goto would mix managed and unmanaged resources,
> which leads to the wrong release order. Instead, use
> devm_pm_runtime_enable() and pm_runtime_resume_and_get() with a devm
> action for the put, so every probe error path unwinds in the correct
> reverse order. As a side effect, a runtime resume failure now aborts
> the probe instead of being ignored.
> 
> ti_eqep_remove() no longer needs to touch runtime PM. While at it,
> report counter_add() failures with dev_err_probe().

I'm confused. Why the PM runtime is used (and announced) in this driver at all?
Do we have upper layer (like a bus driver) disable power before handing over to
the device driver?

TL;DR: what's the point in runtime PM here?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/2] counter: ti-eqep: Fix runtime PM leak on probe failure

[Bui Duc Phuc]

Hi Andy,

> I'm confused. Why the PM runtime is used (and announced) in this driver at all?
> Do we have upper layer (like a bus driver) disable power before handing over to
> the device driver?
>
> TL;DR: what's the point in runtime PM here?
>

This patch does not change the runtime PM design. It only fixes the handling of
the existing runtime PM state and usage reference, ensuring they are properly
released on probe failures and driver removal.

If there are concerns about whether runtime PM is needed at all, that can be
investigated and addressed separately without mixing it with this cleanup fix.

Best regards,
Phuc

Re: [PATCH v4 1/2] counter: ti-eqep: Fix runtime PM leak on probe failure

[David Lechner]

On 11/9/26 5:23 PM, Andy Shevchenko wrote:
> On Mon, Nov 09, 2026 at 03:43:41PM +0700, phucduc.bui@gmail.com wrote:
> 
>> ti_eqep_probe() enables runtime PM and takes a usage reference manually,
>> then returns directly via dev_err_probe() if devm_clk_get_enabled()
>> fails, leaking both.
>>
>> Manual cleanup via goto would mix managed and unmanaged resources,
>> which leads to the wrong release order. Instead, use
>> devm_pm_runtime_enable() and pm_runtime_resume_and_get() with a devm
>> action for the put, so every probe error path unwinds in the correct
>> reverse order. As a side effect, a runtime resume failure now aborts
>> the probe instead of being ignored.
>>
>> ti_eqep_remove() no longer needs to touch runtime PM. While at it,
>> report counter_add() failures with dev_err_probe().
> 
> I'm confused. Why the PM runtime is used (and announced) in this driver at all?
> Do we have upper layer (like a bus driver) disable power before handing over to
> the device driver?

Your guess is right. There is a parent bus shared by eQEP, ECAP and EHRPWM
that has to be turned on so that any of them can work.

> 
> TL;DR: what's the point in runtime PM here?
>