* [PATCH v3] counter: ti-eqep: use devm for runtime PM to fix probe error path
@ 2026-05-29 9:58 Stepan Ionichev
2026-06-03 5:42 ` Andy Shevchenko
2026-06-04 2:19 ` William Breathitt Gray
0 siblings, 2 replies; 3+ messages in thread
From: Stepan Ionichev @ 2026-05-29 9:58 UTC (permalink / raw)
To: wbg; +Cc: m32285159, dlechner, nuno.sa, andy, linux-iio, linux-kernel,
sozdayvek
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v3] counter: ti-eqep: use devm for runtime PM to fix probe error path
2026-05-29 9:58 [PATCH v3] counter: ti-eqep: use devm for runtime PM to fix probe error path Stepan Ionichev
@ 2026-06-03 5:42 ` Andy Shevchenko
2026-06-04 2:19 ` William Breathitt Gray
1 sibling, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2026-06-03 5:42 UTC (permalink / raw)
To: Stepan Ionichev
Cc: wbg, m32285159, dlechner, nuno.sa, andy, linux-iio, linux-kernel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] counter: ti-eqep: use devm for runtime PM to fix probe error path
2026-05-29 9:58 [PATCH v3] counter: ti-eqep: use devm for runtime PM to fix probe error path Stepan Ionichev
2026-06-03 5:42 ` Andy Shevchenko
@ 2026-06-04 2:19 ` William Breathitt Gray
1 sibling, 0 replies; 3+ messages in thread
From: William Breathitt Gray @ 2026-06-04 2:19 UTC (permalink / raw)
To: Stepan Ionichev
Cc: William Breathitt Gray, m32285159, dlechner, nuno.sa, andy,
linux-iio, linux-kernel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-04 2:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-29 9:58 [PATCH v3] counter: ti-eqep: use devm for runtime PM to fix probe error path Stepan Ionichev
2026-06-03 5:42 ` Andy Shevchenko
2026-06-04 2:19 ` William Breathitt Gray
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox