opp: Release current OPP properly

PATCH In review Dmitry Osipenko 2 participants 3 messages Aug 12, 2021 – Aug 13, 2021 Operating performance points (OPP)

lore

Share & tools

lore

Dmitry Osipenko

author

2021-08-12 10:14 UTC #0 lore raw

PATCH 1/1 opp: Release current OPP properly

The current_opp is released only when whole OPP table is released, otherwise it's only marked as removed by dev_pm_opp_remove_table(). Functions like dev_pm_opp_put_clkname() and dev_pm_opp_put_supported_hw() are checking whether OPP table is empty and it's not if current_opp is set since it holds the refcount of OPP, this produces a noisy warning from these functions about busy OPP table. Release current_opp when OPP table is removed to fix it.

Cc [email protected]

Fixes 81c4d8a3c414 ("opp: Keep track of currently programmed OPP")

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/opp/core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

drivers/opp/core.c +6 -3

index b335c077f215..73da968b5c86 100644
@@ -1378,9 +1378,6 @@ static void _opp_table_kref_release(struct kref *kref)
 	list_del(&opp_table->node);
 	mutex_unlock(&opp_table_lock);
 
-	if (opp_table->current_opp)
-		dev_pm_opp_put(opp_table->current_opp);
-
 	_of_clear_opp_table(opp_table);
 
 	/* Release clk */

@@ -2901,6 +2898,12 @@ void dev_pm_opp_remove_table(struct device *dev)
 	if (_opp_remove_all_static(opp_table))
 		dev_pm_opp_put_opp_table(opp_table);
 
+	/* Drop reference taken by _find_current_opp() */
+	if (opp_table->current_opp) {
+		dev_pm_opp_put(opp_table->current_opp);
+		opp_table->current_opp = NULL;
+	}
+
 	/* Drop reference taken by _find_opp_table() */
 	dev_pm_opp_put_opp_table(opp_table);
 }

Signature · 1 line

2.32.0

Viresh Kumar

2021-08-13 04:12 UTC #1 lore raw

Quoted 30 lines · Dmitry Osipenko

The current_opp is released only when whole OPP table is released, otherwise it's only marked as removed by dev_pm_opp_remove_table(). Functions like dev_pm_opp_put_clkname() and dev_pm_opp_put_supported_hw() are checking whether OPP table is empty and it's not if current_opp is set since it holds the refcount of OPP, this produces a noisy warning from these functions about busy OPP table. Release current_opp when OPP table is removed to fix it.
Cc [email protected]
Fixes 81c4d8a3c414 ("opp: Keep track of currently programmed OPP")
Signed-off-by Dmitry Osipenko <[email protected]>
---
 drivers/opp/core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/opp/core.c +0 -3
index b335c077f215..73da968b5c86 100644
@@ -1378,9 +1378,6 @@ static void _opp_table_kref_release(struct kref *kref)
 	list_del(&opp_table->node);
 	mutex_unlock(&opp_table_lock);
 
-	if (opp_table->current_opp)
-		dev_pm_opp_put(opp_table->current_opp);
-
 	_of_clear_opp_table(opp_table);
 
 	/* Release clk */

drivers/opp/core.c void dev_pm_opp_remove_table(struct device *dev) · line 2906

Quoted 9 lines

drivers/opp/core.c +6 -0

@@ -2901,6 +2898,12 @@ void dev_pm_opp_remove_table(struct device *dev)
 	if (_opp_remove_all_static(opp_table))
 		dev_pm_opp_put_opp_table(opp_table);
 
+	/* Drop reference taken by _find_current_opp() */
+	if (opp_table->current_opp) {
+		dev_pm_opp_put(opp_table->current_opp);
+		opp_table->current_opp = NULL;
+	}
+

It is better to drop the reference when the OPP table is really getting removed.

I think the WARN_ON() in the put_* functions can be dropped. It is important to check this when the stuff is getting set, like in dev_pm_opp_set_supported_hw(), but removal is just fine.

Signature · 1 line

viresh

Dmitry Osipenko

author

2021-08-13 17:23 UTC #2 lore raw

13.08.2021 07:12, Viresh Kumar пишет:

Quoted 44 lines · Dmitry Osipenko

Quoted 30 lines
The current_opp is released only when whole OPP table is released, otherwise it's only marked as removed by dev_pm_opp_remove_table(). Functions like dev_pm_opp_put_clkname() and dev_pm_opp_put_supported_hw() are checking whether OPP table is empty and it's not if current_opp is set since it holds the refcount of OPP, this produces a noisy warning from these functions about busy OPP table. Release current_opp when OPP table is removed to fix it.
Cc [email protected]
Fixes 81c4d8a3c414 ("opp: Keep track of currently programmed OPP")
Signed-off-by Dmitry Osipenko <[email protected]>
---
 drivers/opp/core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/opp/core.c +0 -3
index b335c077f215..73da968b5c86 100644
@@ -1378,9 +1378,6 @@ static void _opp_table_kref_release(struct kref *kref)
 	list_del(&opp_table->node);
 	mutex_unlock(&opp_table_lock);
 
-	if (opp_table->current_opp)
-		dev_pm_opp_put(opp_table->current_opp);
-
 	_of_clear_opp_table(opp_table);
 
 	/* Release clk */
drivers/opp/core.c void dev_pm_opp_remove_table(struct device *dev) · line 2906
Quoted 9 lines
drivers/opp/core.c +6 -0
@@ -2901,6 +2898,12 @@ void dev_pm_opp_remove_table(struct device *dev)
 	if (_opp_remove_all_static(opp_table))
 		dev_pm_opp_put_opp_table(opp_table);
 
+	/* Drop reference taken by _find_current_opp() */
+	if (opp_table->current_opp) {
+		dev_pm_opp_put(opp_table->current_opp);
+		opp_table->current_opp = NULL;
+	}
+
It is better to drop the reference when the OPP table is really getting removed.

Quoted 4 lines

I think the WARN_ON() in the put_* functions can be dropped. It is important to check this when the stuff is getting set, like in dev_pm_opp_set_supported_hw(), but removal is just fine.

Alright, I'll make another patch then.