* [PATCH V3 01/10] PM / OPP: Fix memory leak while adding duplicate OPPs
2017-01-02 9:10 [PATCH V3 00/10] PM / OPP: Fixes and cleanups Viresh Kumar
@ 2017-01-02 9:10 ` Viresh Kumar
2017-01-02 9:10 ` [PATCH V3 02/10] PM / OPP: Remove useless TODO Viresh Kumar
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-01-02 9:10 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, Viresh Kumar
There are two types of duplicate OPPs that get different behavior from
the core:
A) An earlier OPP is marked 'available' and has same freq/voltages as
the new one.
B) An earlier OPP with same frequency, but is marked 'unavailable' OR
doesn't have same voltages as the new one.
The OPP core returns 0 for the first one, but -EEXIST for the second.
While the OPP core returns 0 for the first case, its callers don't free
the newly allocated OPP structure which isn't used anymore. Fix that by
returning -EBUSY instead of 0, but make the callers return 0 eventually.
As this isn't a critical fix, its not getting marked for stable kernel.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/base/power/opp/core.c | 18 ++++++++++++++++--
drivers/base/power/opp/of.c | 6 +++++-
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 35ff06283738..a8a5e01b7756 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -1067,6 +1067,16 @@ static bool _opp_supported_by_regulators(struct dev_pm_opp *opp,
return true;
}
+/*
+ * Returns:
+ * 0: On success. And appropriate error message for duplicate OPPs.
+ * -EBUSY: For OPP with same freq/volt and is available. The callers of
+ * _opp_add() must return 0 if they receive -EBUSY from it. This is to make
+ * sure we don't print error messages unnecessarily if different parts of
+ * kernel try to initialize the OPP table.
+ * -EEXIST: For OPP with same freq but different volt or is unavailable. This
+ * should be considered an error by the callers of _opp_add().
+ */
int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
struct opp_table *opp_table)
{
@@ -1099,7 +1109,7 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
/* Should we compare voltages for all regulators here ? */
return opp->available &&
- new_opp->supplies[0].u_volt == opp->supplies[0].u_volt ? 0 : -EEXIST;
+ new_opp->supplies[0].u_volt == opp->supplies[0].u_volt ? -EBUSY : -EEXIST;
}
new_opp->opp_table = opp_table;
@@ -1173,8 +1183,12 @@ int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
new_opp->dynamic = dynamic;
ret = _opp_add(dev, new_opp, opp_table);
- if (ret)
+ if (ret) {
+ /* Don't return error for duplicate OPPs */
+ if (ret == -EBUSY)
+ ret = 0;
goto free_opp;
+ }
mutex_unlock(&opp_table_lock);
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
index 3f7d2591b173..356c75edd656 100644
--- a/drivers/base/power/opp/of.c
+++ b/drivers/base/power/opp/of.c
@@ -327,8 +327,12 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
goto free_opp;
ret = _opp_add(dev, new_opp, opp_table);
- if (ret)
+ if (ret) {
+ /* Don't return error for duplicate OPPs */
+ if (ret == -EBUSY)
+ ret = 0;
goto free_opp;
+ }
/* OPP to select on device suspend */
if (of_property_read_bool(np, "opp-suspend")) {
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V3 02/10] PM / OPP: Remove useless TODO
2017-01-02 9:10 [PATCH V3 00/10] PM / OPP: Fixes and cleanups Viresh Kumar
2017-01-02 9:10 ` [PATCH V3 01/10] PM / OPP: Fix memory leak while adding duplicate OPPs Viresh Kumar
@ 2017-01-02 9:10 ` Viresh Kumar
2017-01-02 9:10 ` [PATCH V3 03/10] PM / OPP: Rename _allocate_opp() to _opp_allocate() Viresh Kumar
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-01-02 9:10 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, Viresh Kumar
This TODO doesn't make sense anymore as we have all the information in a
single OPP table. Remove it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/base/power/opp/of.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
index 356c75edd656..996ca3b42f47 100644
--- a/drivers/base/power/opp/of.c
+++ b/drivers/base/power/opp/of.c
@@ -246,8 +246,6 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table);
static struct device_node *_of_get_opp_desc_node(struct device *dev)
{
/*
- * TODO: Support for multiple OPP tables.
- *
* There should be only ONE phandle present in "operating-points-v2"
* property.
*/
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V3 03/10] PM / OPP: Rename _allocate_opp() to _opp_allocate()
2017-01-02 9:10 [PATCH V3 00/10] PM / OPP: Fixes and cleanups Viresh Kumar
2017-01-02 9:10 ` [PATCH V3 01/10] PM / OPP: Fix memory leak while adding duplicate OPPs Viresh Kumar
2017-01-02 9:10 ` [PATCH V3 02/10] PM / OPP: Remove useless TODO Viresh Kumar
@ 2017-01-02 9:10 ` Viresh Kumar
2017-01-02 9:10 ` [PATCH V3 04/10] PM / OPP: Error out on failing to add static OPPs for v1 bindings Viresh Kumar
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-01-02 9:10 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, Viresh Kumar
Make the naming consistent with how other routines are named.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/base/power/opp/core.c | 4 ++--
drivers/base/power/opp/of.c | 2 +-
drivers/base/power/opp/opp.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index a8a5e01b7756..422482575954 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -1014,7 +1014,7 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq)
}
EXPORT_SYMBOL_GPL(dev_pm_opp_remove);
-struct dev_pm_opp *_allocate_opp(struct device *dev,
+struct dev_pm_opp *_opp_allocate(struct device *dev,
struct opp_table **opp_table)
{
struct dev_pm_opp *opp;
@@ -1167,7 +1167,7 @@ int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
/* Hold our table modification lock here */
mutex_lock(&opp_table_lock);
- new_opp = _allocate_opp(dev, &opp_table);
+ new_opp = _opp_allocate(dev, &opp_table);
if (!new_opp) {
ret = -ENOMEM;
goto unlock;
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
index 996ca3b42f47..f8512ca2bf41 100644
--- a/drivers/base/power/opp/of.c
+++ b/drivers/base/power/opp/of.c
@@ -287,7 +287,7 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
/* Hold our table modification lock here */
mutex_lock(&opp_table_lock);
- new_opp = _allocate_opp(dev, &opp_table);
+ new_opp = _opp_allocate(dev, &opp_table);
if (!new_opp) {
ret = -ENOMEM;
goto unlock;
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index af9f2b849a66..0f23a1059605 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -193,7 +193,7 @@ struct opp_table {
struct opp_table *_find_opp_table(struct device *dev);
struct opp_device *_add_opp_dev(const struct device *dev, struct opp_table *opp_table);
void _dev_pm_opp_remove_table(struct device *dev, bool remove_all);
-struct dev_pm_opp *_allocate_opp(struct device *dev, struct opp_table **opp_table);
+struct dev_pm_opp *_opp_allocate(struct device *dev, struct opp_table **opp_table);
int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table);
void _opp_remove(struct opp_table *opp_table, struct dev_pm_opp *opp, bool notify);
int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, bool dynamic);
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V3 04/10] PM / OPP: Error out on failing to add static OPPs for v1 bindings
2017-01-02 9:10 [PATCH V3 00/10] PM / OPP: Fixes and cleanups Viresh Kumar
` (2 preceding siblings ...)
2017-01-02 9:10 ` [PATCH V3 03/10] PM / OPP: Rename _allocate_opp() to _opp_allocate() Viresh Kumar
@ 2017-01-02 9:10 ` Viresh Kumar
2017-01-02 9:10 ` [PATCH V3 05/10] PM / OPP: Add light weight _opp_free() routine Viresh Kumar
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-01-02 9:10 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, Viresh Kumar
The code adding static OPPs for V2 bindings already does so. Make the V1
bindings specific code behave the same.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/base/power/opp/of.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
index f8512ca2bf41..c8fe815774ff 100644
--- a/drivers/base/power/opp/of.c
+++ b/drivers/base/power/opp/of.c
@@ -433,7 +433,7 @@ static int _of_add_opp_table_v1(struct device *dev)
{
const struct property *prop;
const __be32 *val;
- int nr;
+ int nr, ret;
prop = of_find_property(dev->of_node, "operating-points", NULL);
if (!prop)
@@ -456,9 +456,13 @@ static int _of_add_opp_table_v1(struct device *dev)
unsigned long freq = be32_to_cpup(val++) * 1000;
unsigned long volt = be32_to_cpup(val++);
- if (_opp_add_v1(dev, freq, volt, false))
- dev_warn(dev, "%s: Failed to add OPP %ld\n",
- __func__, freq);
+ ret = _opp_add_v1(dev, freq, volt, false);
+ if (ret) {
+ dev_err(dev, "%s: Failed to add OPP %ld (%d)\n",
+ __func__, freq, ret);
+ dev_pm_opp_of_remove_table(dev);
+ return ret;
+ }
nr -= 2;
}
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V3 05/10] PM / OPP: Add light weight _opp_free() routine
2017-01-02 9:10 [PATCH V3 00/10] PM / OPP: Fixes and cleanups Viresh Kumar
` (3 preceding siblings ...)
2017-01-02 9:10 ` [PATCH V3 04/10] PM / OPP: Error out on failing to add static OPPs for v1 bindings Viresh Kumar
@ 2017-01-02 9:10 ` Viresh Kumar
2017-01-02 9:11 ` [PATCH V3 06/10] PM / OPP: Rename and split _dev_pm_opp_remove_table() Viresh Kumar
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-01-02 9:10 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, Viresh Kumar
The OPPs which are never successfully added using _opp_add() are not
required to be freed with the _opp_remove() routine, as a simple kfree()
is enough for them.
Introduce a new light weight routine _opp_free(), which will do that.
That also helps us removing the 'notify' parameter to _opp_remove(),
which isn't required anymore.
Note that _opp_free() contains a call to _remove_opp_table() as the OPP
table might have been added for this very OPP only. The
_remove_opp_table() routine returns quickly if there are more OPPs in
the table. This will be simplified in later patches though.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/base/power/opp/core.c | 20 +++++++++++---------
drivers/base/power/opp/of.c | 2 +-
drivers/base/power/opp/opp.h | 2 +-
3 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 422482575954..ea5b90e0882c 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -929,6 +929,12 @@ static void _remove_opp_table(struct opp_table *opp_table)
_kfree_device_rcu);
}
+void _opp_free(struct dev_pm_opp *opp, struct opp_table *opp_table)
+{
+ kfree(opp);
+ _remove_opp_table(opp_table);
+}
+
/**
* _kfree_opp_rcu() - Free OPP RCU handler
* @head: RCU head
@@ -944,7 +950,6 @@ static void _kfree_opp_rcu(struct rcu_head *head)
* _opp_remove() - Remove an OPP from a table definition
* @opp_table: points back to the opp_table struct this opp belongs to
* @opp: pointer to the OPP to remove
- * @notify: OPP_EVENT_REMOVE notification should be sent or not
*
* This function removes an opp definition from the opp table.
*
@@ -952,16 +957,13 @@ static void _kfree_opp_rcu(struct rcu_head *head)
* It is assumed that the caller holds required mutex for an RCU updater
* strategy.
*/
-void _opp_remove(struct opp_table *opp_table, struct dev_pm_opp *opp,
- bool notify)
+static void _opp_remove(struct opp_table *opp_table, struct dev_pm_opp *opp)
{
/*
* Notify the changes in the availability of the operable
* frequency/voltage list.
*/
- if (notify)
- srcu_notifier_call_chain(&opp_table->srcu_head,
- OPP_EVENT_REMOVE, opp);
+ srcu_notifier_call_chain(&opp_table->srcu_head, OPP_EVENT_REMOVE, opp);
opp_debug_remove_one(opp);
list_del_rcu(&opp->node);
call_srcu(&opp_table->srcu_head.srcu, &opp->rcu_head, _kfree_opp_rcu);
@@ -1008,7 +1010,7 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq)
goto unlock;
}
- _opp_remove(opp_table, opp, true);
+ _opp_remove(opp_table, opp);
unlock:
mutex_unlock(&opp_table_lock);
}
@@ -1200,7 +1202,7 @@ int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
return 0;
free_opp:
- _opp_remove(opp_table, new_opp, false);
+ _opp_free(new_opp, opp_table);
unlock:
mutex_unlock(&opp_table_lock);
return ret;
@@ -1912,7 +1914,7 @@ void _dev_pm_opp_remove_table(struct device *dev, bool remove_all)
/* Free static OPPs */
list_for_each_entry_safe(opp, tmp, &opp_table->opp_list, node) {
if (remove_all || !opp->dynamic)
- _opp_remove(opp_table, opp, true);
+ _opp_remove(opp_table, opp);
}
} else {
_remove_opp_dev(_find_opp_dev(dev, opp_table), opp_table);
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
index c8fe815774ff..67c9eeded4e1 100644
--- a/drivers/base/power/opp/of.c
+++ b/drivers/base/power/opp/of.c
@@ -362,7 +362,7 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
return 0;
free_opp:
- _opp_remove(opp_table, new_opp, false);
+ _opp_free(new_opp, opp_table);
unlock:
mutex_unlock(&opp_table_lock);
return ret;
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 0f23a1059605..334f7570df32 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -194,8 +194,8 @@ struct opp_table *_find_opp_table(struct device *dev);
struct opp_device *_add_opp_dev(const struct device *dev, struct opp_table *opp_table);
void _dev_pm_opp_remove_table(struct device *dev, bool remove_all);
struct dev_pm_opp *_opp_allocate(struct device *dev, struct opp_table **opp_table);
+void _opp_free(struct dev_pm_opp *opp, struct opp_table *opp_table);
int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table);
-void _opp_remove(struct opp_table *opp_table, struct dev_pm_opp *opp, bool notify);
int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, bool dynamic);
void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of);
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V3 06/10] PM / OPP: Rename and split _dev_pm_opp_remove_table()
2017-01-02 9:10 [PATCH V3 00/10] PM / OPP: Fixes and cleanups Viresh Kumar
` (4 preceding siblings ...)
2017-01-02 9:10 ` [PATCH V3 05/10] PM / OPP: Add light weight _opp_free() routine Viresh Kumar
@ 2017-01-02 9:11 ` Viresh Kumar
2017-01-02 9:11 ` [PATCH V3 07/10] PM / OPP: Don't allocate OPP table from _opp_allocate() Viresh Kumar
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-01-02 9:11 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, Viresh Kumar
Later patches would want to remove OPP table (and its OPPs) using the
opp_table pointer instead of 'dev'.
In order to prepare for that, rename _dev_pm_opp_remove_table() as
_dev_pm_opp_find_and_remove_table() split out part of it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/base/power/opp/core.c | 35 ++++++++++++++++++++++-------------
drivers/base/power/opp/of.c | 2 +-
drivers/base/power/opp/opp.h | 2 +-
3 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index ea5b90e0882c..b3a624a4af81 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -1888,11 +1888,29 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_notifier);
* Free OPPs either created using static entries present in DT or even the
* dynamically added entries based on remove_all param.
*/
-void _dev_pm_opp_remove_table(struct device *dev, bool remove_all)
+static void _dev_pm_opp_remove_table(struct opp_table *opp_table,
+ struct device *dev, bool remove_all)
{
- struct opp_table *opp_table;
struct dev_pm_opp *opp, *tmp;
+ opp_rcu_lockdep_assert();
+
+ /* Find if opp_table manages a single device */
+ if (list_is_singular(&opp_table->dev_list)) {
+ /* Free static OPPs */
+ list_for_each_entry_safe(opp, tmp, &opp_table->opp_list, node) {
+ if (remove_all || !opp->dynamic)
+ _opp_remove(opp_table, opp);
+ }
+ } else {
+ _remove_opp_dev(_find_opp_dev(dev, opp_table), opp_table);
+ }
+}
+
+void _dev_pm_opp_find_and_remove_table(struct device *dev, bool remove_all)
+{
+ struct opp_table *opp_table;
+
/* Hold our table modification lock here */
mutex_lock(&opp_table_lock);
@@ -1909,16 +1927,7 @@ void _dev_pm_opp_remove_table(struct device *dev, bool remove_all)
goto unlock;
}
- /* Find if opp_table manages a single device */
- if (list_is_singular(&opp_table->dev_list)) {
- /* Free static OPPs */
- list_for_each_entry_safe(opp, tmp, &opp_table->opp_list, node) {
- if (remove_all || !opp->dynamic)
- _opp_remove(opp_table, opp);
- }
- } else {
- _remove_opp_dev(_find_opp_dev(dev, opp_table), opp_table);
- }
+ _dev_pm_opp_remove_table(opp_table, dev, remove_all);
unlock:
mutex_unlock(&opp_table_lock);
@@ -1939,6 +1948,6 @@ void _dev_pm_opp_remove_table(struct device *dev, bool remove_all)
*/
void dev_pm_opp_remove_table(struct device *dev)
{
- _dev_pm_opp_remove_table(dev, true);
+ _dev_pm_opp_find_and_remove_table(dev, true);
}
EXPORT_SYMBOL_GPL(dev_pm_opp_remove_table);
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
index 67c9eeded4e1..442fa46c4f5c 100644
--- a/drivers/base/power/opp/of.c
+++ b/drivers/base/power/opp/of.c
@@ -238,7 +238,7 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
*/
void dev_pm_opp_of_remove_table(struct device *dev)
{
- _dev_pm_opp_remove_table(dev, false);
+ _dev_pm_opp_find_and_remove_table(dev, false);
}
EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table);
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 334f7570df32..c4b539a8533a 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -192,7 +192,7 @@ struct opp_table {
/* Routines internal to opp core */
struct opp_table *_find_opp_table(struct device *dev);
struct opp_device *_add_opp_dev(const struct device *dev, struct opp_table *opp_table);
-void _dev_pm_opp_remove_table(struct device *dev, bool remove_all);
+void _dev_pm_opp_find_and_remove_table(struct device *dev, bool remove_all);
struct dev_pm_opp *_opp_allocate(struct device *dev, struct opp_table **opp_table);
void _opp_free(struct dev_pm_opp *opp, struct opp_table *opp_table);
int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table);
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V3 07/10] PM / OPP: Don't allocate OPP table from _opp_allocate()
2017-01-02 9:10 [PATCH V3 00/10] PM / OPP: Fixes and cleanups Viresh Kumar
` (5 preceding siblings ...)
2017-01-02 9:11 ` [PATCH V3 06/10] PM / OPP: Rename and split _dev_pm_opp_remove_table() Viresh Kumar
@ 2017-01-02 9:11 ` Viresh Kumar
2017-01-02 9:11 ` [PATCH V3 08/10] PM / OPP: Rename dev_pm_opp_get_suspend_opp() and return OPP rate Viresh Kumar
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-01-02 9:11 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, Viresh Kumar
There is no point in trying to find/allocate the table for every OPP
that is added for a device. It would be far more efficient to allocate
the table only once and pass its pointer to the routines that add the
OPP entry.
Locking is removed from _opp_add_static_v2() and _opp_add_v1() now as
the callers call them with that lock already held.
Call to _remove_opp_table() routine is also removed from _opp_free()
now, as opp_table isn't allocated from within _opp_allocate(). This is
handled by the routines which created the OPP table in the first place.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/base/power/opp/core.c | 67 +++++++++++++++++++-------------------
drivers/base/power/opp/of.c | 75 ++++++++++++++++++++++---------------------
drivers/base/power/opp/opp.h | 9 ++++--
3 files changed, 78 insertions(+), 73 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index b3a624a4af81..dde7dd099aa0 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -829,7 +829,7 @@ struct opp_device *_add_opp_dev(const struct device *dev,
*
* Return: valid opp_table pointer if success, else NULL.
*/
-static struct opp_table *_add_opp_table(struct device *dev)
+struct opp_table *_add_opp_table(struct device *dev)
{
struct opp_table *opp_table;
struct opp_device *opp_dev;
@@ -929,10 +929,9 @@ static void _remove_opp_table(struct opp_table *opp_table)
_kfree_device_rcu);
}
-void _opp_free(struct dev_pm_opp *opp, struct opp_table *opp_table)
+void _opp_free(struct dev_pm_opp *opp)
{
kfree(opp);
- _remove_opp_table(opp_table);
}
/**
@@ -1016,16 +1015,10 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq)
}
EXPORT_SYMBOL_GPL(dev_pm_opp_remove);
-struct dev_pm_opp *_opp_allocate(struct device *dev,
- struct opp_table **opp_table)
+struct dev_pm_opp *_opp_allocate(struct opp_table *table)
{
struct dev_pm_opp *opp;
int count, supply_size;
- struct opp_table *table;
-
- table = _add_opp_table(dev);
- if (!table)
- return NULL;
/* Allocate space for at least one supply */
count = table->regulator_count ? table->regulator_count : 1;
@@ -1033,17 +1026,13 @@ struct dev_pm_opp *_opp_allocate(struct device *dev,
/* allocate new OPP node and supplies structures */
opp = kzalloc(sizeof(*opp) + supply_size, GFP_KERNEL);
- if (!opp) {
- kfree(table);
+ if (!opp)
return NULL;
- }
/* Put the supplies at the end of the OPP structure as an empty array */
opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
INIT_LIST_HEAD(&opp->node);
- *opp_table = table;
-
return opp;
}
@@ -1133,6 +1122,7 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
/**
* _opp_add_v1() - Allocate a OPP based on v1 bindings.
+ * @opp_table: OPP table
* @dev: device for which we do this operation
* @freq: Frequency in Hz for this OPP
* @u_volt: Voltage in uVolts for this OPP
@@ -1158,22 +1148,18 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
* Duplicate OPPs (both freq and volt are same) and !opp->available
* -ENOMEM Memory allocation failure
*/
-int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
- bool dynamic)
+int _opp_add_v1(struct opp_table *opp_table, struct device *dev,
+ unsigned long freq, long u_volt, bool dynamic)
{
- struct opp_table *opp_table;
struct dev_pm_opp *new_opp;
unsigned long tol;
int ret;
- /* Hold our table modification lock here */
- mutex_lock(&opp_table_lock);
+ opp_rcu_lockdep_assert();
- new_opp = _opp_allocate(dev, &opp_table);
- if (!new_opp) {
- ret = -ENOMEM;
- goto unlock;
- }
+ new_opp = _opp_allocate(opp_table);
+ if (!new_opp)
+ return -ENOMEM;
/* populate the opp table */
new_opp->rate = freq;
@@ -1192,8 +1178,6 @@ int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
goto free_opp;
}
- mutex_unlock(&opp_table_lock);
-
/*
* Notify the changes in the availability of the operable
* frequency/voltage list.
@@ -1202,9 +1186,8 @@ int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
return 0;
free_opp:
- _opp_free(new_opp, opp_table);
-unlock:
- mutex_unlock(&opp_table_lock);
+ _opp_free(new_opp);
+
return ret;
}
@@ -1722,7 +1705,25 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_register_put_opp_helper);
*/
int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
{
- return _opp_add_v1(dev, freq, u_volt, true);
+ struct opp_table *opp_table;
+ int ret;
+
+ /* Hold our table modification lock here */
+ mutex_lock(&opp_table_lock);
+
+ opp_table = _add_opp_table(dev);
+ if (!opp_table) {
+ ret = -ENOMEM;
+ goto unlock;
+ }
+
+ ret = _opp_add_v1(opp_table, dev, freq, u_volt, true);
+ if (ret)
+ _remove_opp_table(opp_table);
+
+unlock:
+ mutex_unlock(&opp_table_lock);
+ return ret;
}
EXPORT_SYMBOL_GPL(dev_pm_opp_add);
@@ -1888,8 +1889,8 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_notifier);
* Free OPPs either created using static entries present in DT or even the
* dynamically added entries based on remove_all param.
*/
-static void _dev_pm_opp_remove_table(struct opp_table *opp_table,
- struct device *dev, bool remove_all)
+void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev,
+ bool remove_all)
{
struct dev_pm_opp *opp, *tmp;
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
index 442fa46c4f5c..cdbf733ac9b1 100644
--- a/drivers/base/power/opp/of.c
+++ b/drivers/base/power/opp/of.c
@@ -255,6 +255,7 @@ static struct device_node *_of_get_opp_desc_node(struct device *dev)
/**
* _opp_add_static_v2() - Allocate static OPPs (As per 'v2' DT bindings)
+ * @opp_table: OPP table
* @dev: device for which we do this operation
* @np: device node
*
@@ -276,22 +277,17 @@ static struct device_node *_of_get_opp_desc_node(struct device *dev)
* -ENOMEM Memory allocation failure
* -EINVAL Failed parsing the OPP node
*/
-static int _opp_add_static_v2(struct device *dev, struct device_node *np)
+static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev,
+ struct device_node *np)
{
- struct opp_table *opp_table;
struct dev_pm_opp *new_opp;
u64 rate;
u32 val;
int ret;
- /* Hold our table modification lock here */
- mutex_lock(&opp_table_lock);
-
- new_opp = _opp_allocate(dev, &opp_table);
- if (!new_opp) {
- ret = -ENOMEM;
- goto unlock;
- }
+ new_opp = _opp_allocate(opp_table);
+ if (!new_opp)
+ return -ENOMEM;
ret = of_property_read_u64(np, "opp-hz", &rate);
if (ret < 0) {
@@ -347,8 +343,6 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
if (new_opp->clock_latency_ns > opp_table->clock_latency_ns_max)
opp_table->clock_latency_ns_max = new_opp->clock_latency_ns;
- mutex_unlock(&opp_table_lock);
-
pr_debug("%s: turbo:%d rate:%lu uv:%lu uvmin:%lu uvmax:%lu latency:%lu\n",
__func__, new_opp->turbo, new_opp->rate,
new_opp->supplies[0].u_volt, new_opp->supplies[0].u_volt_min,
@@ -362,9 +356,8 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
return 0;
free_opp:
- _opp_free(new_opp, opp_table);
-unlock:
- mutex_unlock(&opp_table_lock);
+ _opp_free(new_opp);
+
return ret;
}
@@ -382,16 +375,20 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
/* OPPs are already managed */
if (!_add_opp_dev(dev, opp_table))
ret = -ENOMEM;
- mutex_unlock(&opp_table_lock);
- return ret;
+ goto unlock;
+ }
+
+ opp_table = _add_opp_table(dev);
+ if (!opp_table) {
+ ret = -ENOMEM;
+ goto unlock;
}
- mutex_unlock(&opp_table_lock);
/* We have opp-table node now, iterate over it and add OPPs */
for_each_available_child_of_node(opp_np, np) {
count++;
- ret = _opp_add_static_v2(dev, np);
+ ret = _opp_add_static_v2(opp_table, dev, np);
if (ret) {
dev_err(dev, "%s: Failed to add OPP, %d\n", __func__,
ret);
@@ -400,15 +397,8 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
}
/* There should be one of more OPP defined */
- if (WARN_ON(!count))
- return -ENOENT;
-
- mutex_lock(&opp_table_lock);
-
- opp_table = _find_opp_table(dev);
- if (WARN_ON(IS_ERR(opp_table))) {
- ret = PTR_ERR(opp_table);
- mutex_unlock(&opp_table_lock);
+ if (WARN_ON(!count)) {
+ ret = -ENOENT;
goto free_table;
}
@@ -418,12 +408,12 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
else
opp_table->shared_opp = OPP_TABLE_ACCESS_EXCLUSIVE;
- mutex_unlock(&opp_table_lock);
-
- return 0;
+ goto unlock;
free_table:
- dev_pm_opp_of_remove_table(dev);
+ _dev_pm_opp_remove_table(opp_table, dev, false);
+unlock:
+ mutex_unlock(&opp_table_lock);
return ret;
}
@@ -431,9 +421,10 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
/* Initializes OPP tables based on old-deprecated bindings */
static int _of_add_opp_table_v1(struct device *dev)
{
+ struct opp_table *opp_table;
const struct property *prop;
const __be32 *val;
- int nr, ret;
+ int nr, ret = 0;
prop = of_find_property(dev->of_node, "operating-points", NULL);
if (!prop)
@@ -451,22 +442,32 @@ static int _of_add_opp_table_v1(struct device *dev)
return -EINVAL;
}
+ mutex_lock(&opp_table_lock);
+
+ opp_table = _add_opp_table(dev);
+ if (!opp_table) {
+ ret = -ENOMEM;
+ goto unlock;
+ }
+
val = prop->value;
while (nr) {
unsigned long freq = be32_to_cpup(val++) * 1000;
unsigned long volt = be32_to_cpup(val++);
- ret = _opp_add_v1(dev, freq, volt, false);
+ ret = _opp_add_v1(opp_table, dev, freq, volt, false);
if (ret) {
dev_err(dev, "%s: Failed to add OPP %ld (%d)\n",
__func__, freq, ret);
- dev_pm_opp_of_remove_table(dev);
- return ret;
+ _dev_pm_opp_remove_table(opp_table, dev, false);
+ break;
}
nr -= 2;
}
- return 0;
+unlock:
+ mutex_unlock(&opp_table_lock);
+ return ret;
}
/**
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index c4b539a8533a..0a5eb4d1e8f7 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -191,13 +191,16 @@ struct opp_table {
/* Routines internal to opp core */
struct opp_table *_find_opp_table(struct device *dev);
+struct opp_table *_add_opp_table(struct device *dev);
struct opp_device *_add_opp_dev(const struct device *dev, struct opp_table *opp_table);
+void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev, bool remove_all);
void _dev_pm_opp_find_and_remove_table(struct device *dev, bool remove_all);
-struct dev_pm_opp *_opp_allocate(struct device *dev, struct opp_table **opp_table);
-void _opp_free(struct dev_pm_opp *opp, struct opp_table *opp_table);
+struct dev_pm_opp *_opp_allocate(struct opp_table *opp_table);
+void _opp_free(struct dev_pm_opp *opp);
int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table);
-int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, bool dynamic);
+int _opp_add_v1(struct opp_table *opp_table, struct device *dev, unsigned long freq, long u_volt, bool dynamic);
void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of);
+struct opp_table *_add_opp_table(struct device *dev);
#ifdef CONFIG_OF
void _of_init_opp_table(struct opp_table *opp_table, struct device *dev);
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V3 08/10] PM / OPP: Rename dev_pm_opp_get_suspend_opp() and return OPP rate
2017-01-02 9:10 [PATCH V3 00/10] PM / OPP: Fixes and cleanups Viresh Kumar
` (6 preceding siblings ...)
2017-01-02 9:11 ` [PATCH V3 07/10] PM / OPP: Don't allocate OPP table from _opp_allocate() Viresh Kumar
@ 2017-01-02 9:11 ` Viresh Kumar
2017-01-02 9:11 ` [PATCH V3 09/10] PM / OPP: Don't expose srcu_head to register notifiers Viresh Kumar
2017-01-02 9:11 ` [PATCH V3 10/10] PM / OPP: Split out part of _add_opp_table() and _remove_opp_table() Viresh Kumar
9 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-01-02 9:11 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, Viresh Kumar
There is only one user of dev_pm_opp_get_suspend_opp() and that uses it
to get the OPP rate for the suspend_opp.
Rename dev_pm_opp_get_suspend_opp() as dev_pm_opp_get_suspend_opp_freq()
and return the rate directly from it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/base/power/opp/core.c | 27 +++++++++++++--------------
drivers/cpufreq/cpufreq-dt.c | 7 +------
include/linux/pm_opp.h | 6 +++---
3 files changed, 17 insertions(+), 23 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index dde7dd099aa0..614d779ab6a2 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -328,32 +328,31 @@ unsigned long dev_pm_opp_get_max_transition_latency(struct device *dev)
EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_transition_latency);
/**
- * dev_pm_opp_get_suspend_opp() - Get suspend opp
+ * dev_pm_opp_get_suspend_opp_freq() - Get frequency of suspend opp in Hz
* @dev: device for which we do this operation
*
- * Return: This function returns pointer to the suspend opp if it is
- * defined and available, otherwise it returns NULL.
- *
- * Locking: This function must be called under rcu_read_lock(). opp is a rcu
- * protected pointer. The reason for the same is that the opp pointer which is
- * returned will remain valid for use with opp_get_{voltage, freq} only while
- * under the locked area. The pointer returned must be used prior to unlocking
- * with rcu_read_unlock() to maintain the integrity of the pointer.
+ * Return: This function returns the frequency of the OPP marked as suspend_opp
+ * if one is available, else returns 0;
*/
-struct dev_pm_opp *dev_pm_opp_get_suspend_opp(struct device *dev)
+unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev)
{
struct opp_table *opp_table;
+ unsigned long freq = 0;
- opp_rcu_lockdep_assert();
+ rcu_read_lock();
opp_table = _find_opp_table(dev);
if (IS_ERR(opp_table) || !opp_table->suspend_opp ||
!opp_table->suspend_opp->available)
- return NULL;
+ goto unlock;
- return opp_table->suspend_opp;
+ freq = dev_pm_opp_get_freq(opp_table->suspend_opp);
+
+unlock:
+ rcu_read_unlock();
+ return freq;
}
-EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp);
+EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_freq);
/**
* dev_pm_opp_get_opp_count() - Get number of opps available in the opp table
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 269013311e79..c943787d761e 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -148,7 +148,6 @@ static int cpufreq_init(struct cpufreq_policy *policy)
struct private_data *priv;
struct device *cpu_dev;
struct clk *cpu_clk;
- struct dev_pm_opp *suspend_opp;
unsigned int transition_latency;
bool fallback = false;
const char *name;
@@ -252,11 +251,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
policy->driver_data = priv;
policy->clk = cpu_clk;
- rcu_read_lock();
- suspend_opp = dev_pm_opp_get_suspend_opp(cpu_dev);
- if (suspend_opp)
- policy->suspend_freq = dev_pm_opp_get_freq(suspend_opp) / 1000;
- rcu_read_unlock();
+ policy->suspend_freq = dev_pm_opp_get_suspend_opp_freq(cpu_dev) / 1000;
ret = cpufreq_table_validate_and_show(policy, freq_table);
if (ret) {
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 0edd88f93904..7483ff291a8e 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -88,7 +88,7 @@ int dev_pm_opp_get_opp_count(struct device *dev);
unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev);
unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev);
unsigned long dev_pm_opp_get_max_transition_latency(struct device *dev);
-struct dev_pm_opp *dev_pm_opp_get_suspend_opp(struct device *dev);
+unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev);
struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
unsigned long freq,
@@ -159,9 +159,9 @@ static inline unsigned long dev_pm_opp_get_max_transition_latency(struct device
return 0;
}
-static inline struct dev_pm_opp *dev_pm_opp_get_suspend_opp(struct device *dev)
+static inline unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev)
{
- return NULL;
+ return 0;
}
static inline struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V3 09/10] PM / OPP: Don't expose srcu_head to register notifiers
2017-01-02 9:10 [PATCH V3 00/10] PM / OPP: Fixes and cleanups Viresh Kumar
` (7 preceding siblings ...)
2017-01-02 9:11 ` [PATCH V3 08/10] PM / OPP: Rename dev_pm_opp_get_suspend_opp() and return OPP rate Viresh Kumar
@ 2017-01-02 9:11 ` Viresh Kumar
2017-01-02 9:11 ` [PATCH V3 10/10] PM / OPP: Split out part of _add_opp_table() and _remove_opp_table() Viresh Kumar
9 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-01-02 9:11 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd,
MyungJoo Ham, Kyungmin Park
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, Viresh Kumar
Let the OPP core provide helpers to register notifiers for any device,
instead of exposing srcu_head outside of the core.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/base/power/opp/core.c | 66 ++++++++++++++++++++++++++++++++-----------
drivers/devfreq/devfreq.c | 26 ++---------------
include/linux/pm_opp.h | 14 ++++++---
3 files changed, 62 insertions(+), 44 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 614d779ab6a2..f35effad60d1 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -1860,29 +1860,63 @@ int dev_pm_opp_disable(struct device *dev, unsigned long freq)
EXPORT_SYMBOL_GPL(dev_pm_opp_disable);
/**
- * dev_pm_opp_get_notifier() - find notifier_head of the device with opp
- * @dev: device pointer used to lookup OPP table.
+ * dev_pm_opp_register_notifier() - Register OPP notifier for the device
+ * @dev: Device for which notifier needs to be registered
+ * @nb: Notifier block to be registered
*
- * Return: pointer to notifier head if found, otherwise -ENODEV or
- * -EINVAL based on type of error casted as pointer. value must be checked
- * with IS_ERR to determine valid pointer or error result.
+ * Return: 0 on success or a negative error value.
+ */
+int dev_pm_opp_register_notifier(struct device *dev, struct notifier_block *nb)
+{
+ struct opp_table *opp_table;
+ int ret;
+
+ rcu_read_lock();
+
+ opp_table = _find_opp_table(dev);
+ if (IS_ERR(opp_table)) {
+ ret = PTR_ERR(opp_table);
+ goto unlock;
+ }
+
+ ret = srcu_notifier_chain_register(&opp_table->srcu_head, nb);
+
+unlock:
+ rcu_read_unlock();
+
+ return ret;
+}
+EXPORT_SYMBOL(dev_pm_opp_register_notifier);
+
+/**
+ * dev_pm_opp_unregister_notifier() - Unregister OPP notifier for the device
+ * @dev: Device for which notifier needs to be unregistered
+ * @nb: Notifier block to be unregistered
*
- * Locking: This function must be called under rcu_read_lock(). opp_table is a
- * RCU protected pointer. The reason for the same is that the opp pointer which
- * is returned will remain valid for use with opp_get_{voltage, freq} only while
- * under the locked area. The pointer returned must be used prior to unlocking
- * with rcu_read_unlock() to maintain the integrity of the pointer.
+ * Return: 0 on success or a negative error value.
*/
-struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev)
+int dev_pm_opp_unregister_notifier(struct device *dev,
+ struct notifier_block *nb)
{
- struct opp_table *opp_table = _find_opp_table(dev);
+ struct opp_table *opp_table;
+ int ret;
- if (IS_ERR(opp_table))
- return ERR_CAST(opp_table); /* matching type */
+ rcu_read_lock();
+
+ opp_table = _find_opp_table(dev);
+ if (IS_ERR(opp_table)) {
+ ret = PTR_ERR(opp_table);
+ goto unlock;
+ }
+
+ ret = srcu_notifier_chain_unregister(&opp_table->srcu_head, nb);
- return &opp_table->srcu_head;
+unlock:
+ rcu_read_unlock();
+
+ return ret;
}
-EXPORT_SYMBOL_GPL(dev_pm_opp_get_notifier);
+EXPORT_SYMBOL(dev_pm_opp_unregister_notifier);
/*
* Free OPPs either created using static entries present in DT or even the
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index a324801d6a66..b0de42972b74 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1260,18 +1260,7 @@ EXPORT_SYMBOL(devfreq_recommended_opp);
*/
int devfreq_register_opp_notifier(struct device *dev, struct devfreq *devfreq)
{
- struct srcu_notifier_head *nh;
- int ret = 0;
-
- rcu_read_lock();
- nh = dev_pm_opp_get_notifier(dev);
- if (IS_ERR(nh))
- ret = PTR_ERR(nh);
- rcu_read_unlock();
- if (!ret)
- ret = srcu_notifier_chain_register(nh, &devfreq->nb);
-
- return ret;
+ return dev_pm_opp_register_notifier(dev, &devfreq->nb);
}
EXPORT_SYMBOL(devfreq_register_opp_notifier);
@@ -1287,18 +1276,7 @@ EXPORT_SYMBOL(devfreq_register_opp_notifier);
*/
int devfreq_unregister_opp_notifier(struct device *dev, struct devfreq *devfreq)
{
- struct srcu_notifier_head *nh;
- int ret = 0;
-
- rcu_read_lock();
- nh = dev_pm_opp_get_notifier(dev);
- if (IS_ERR(nh))
- ret = PTR_ERR(nh);
- rcu_read_unlock();
- if (!ret)
- ret = srcu_notifier_chain_unregister(nh, &devfreq->nb);
-
- return ret;
+ return dev_pm_opp_unregister_notifier(dev, &devfreq->nb);
}
EXPORT_SYMBOL(devfreq_unregister_opp_notifier);
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 7483ff291a8e..66a02deeb03f 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -108,7 +108,9 @@ int dev_pm_opp_enable(struct device *dev, unsigned long freq);
int dev_pm_opp_disable(struct device *dev, unsigned long freq);
-struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev);
+int dev_pm_opp_register_notifier(struct device *dev, struct notifier_block *nb);
+int dev_pm_opp_unregister_notifier(struct device *dev, struct notifier_block *nb);
+
int dev_pm_opp_set_supported_hw(struct device *dev, const u32 *versions,
unsigned int count);
void dev_pm_opp_put_supported_hw(struct device *dev);
@@ -202,10 +204,14 @@ static inline int dev_pm_opp_disable(struct device *dev, unsigned long freq)
return 0;
}
-static inline struct srcu_notifier_head *dev_pm_opp_get_notifier(
- struct device *dev)
+static inline int dev_pm_opp_register_notifier(struct device *dev, struct notifier_block *nb)
{
- return ERR_PTR(-ENOTSUPP);
+ return -ENOTSUPP;
+}
+
+static inline int dev_pm_opp_unregister_notifier(struct device *dev, struct notifier_block *nb)
+{
+ return -ENOTSUPP;
}
static inline int dev_pm_opp_set_supported_hw(struct device *dev,
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V3 10/10] PM / OPP: Split out part of _add_opp_table() and _remove_opp_table()
2017-01-02 9:10 [PATCH V3 00/10] PM / OPP: Fixes and cleanups Viresh Kumar
` (8 preceding siblings ...)
2017-01-02 9:11 ` [PATCH V3 09/10] PM / OPP: Don't expose srcu_head to register notifiers Viresh Kumar
@ 2017-01-02 9:11 ` Viresh Kumar
9 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-01-02 9:11 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, Viresh Kumar
Split out parts of _add_opp_table() and _remove_opp_table() into
separate routines. This improves readability as well.
Should result in no functional changes.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/base/power/opp/core.c | 76 +++++++++++++++++++++++++------------------
1 file changed, 44 insertions(+), 32 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index f35effad60d1..622dd32f8dda 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -819,26 +819,12 @@ struct opp_device *_add_opp_dev(const struct device *dev,
return opp_dev;
}
-/**
- * _add_opp_table() - Find OPP table or allocate a new one
- * @dev: device for which we do this operation
- *
- * It tries to find an existing table first, if it couldn't find one, it
- * allocates a new OPP table and returns that.
- *
- * Return: valid opp_table pointer if success, else NULL.
- */
-struct opp_table *_add_opp_table(struct device *dev)
+static struct opp_table *_allocate_opp_table(struct device *dev)
{
struct opp_table *opp_table;
struct opp_device *opp_dev;
int ret;
- /* Check for existing table for 'dev' first */
- opp_table = _find_opp_table(dev);
- if (!IS_ERR(opp_table))
- return opp_table;
-
/*
* Allocate a new OPP table. In the infrequent case where a new
* device is needed to be added, we pay this penalty.
@@ -875,6 +861,27 @@ struct opp_table *_add_opp_table(struct device *dev)
}
/**
+ * _add_opp_table() - Find OPP table or allocate a new one
+ * @dev: device for which we do this operation
+ *
+ * It tries to find an existing table first, if it couldn't find one, it
+ * allocates a new OPP table and returns that.
+ *
+ * Return: valid opp_table pointer if success, else NULL.
+ */
+struct opp_table *_add_opp_table(struct device *dev)
+{
+ struct opp_table *opp_table;
+
+ /* Check for existing table for 'dev' first */
+ opp_table = _find_opp_table(dev);
+ if (!IS_ERR(opp_table))
+ return opp_table;
+
+ return _allocate_opp_table(dev);
+}
+
+/**
* _kfree_device_rcu() - Free opp_table RCU handler
* @head: RCU head
*/
@@ -886,6 +893,27 @@ static void _kfree_device_rcu(struct rcu_head *head)
kfree_rcu(opp_table, rcu_head);
}
+static void _free_opp_table(struct opp_table *opp_table)
+{
+ struct opp_device *opp_dev;
+
+ /* Release clk */
+ if (!IS_ERR(opp_table->clk))
+ clk_put(opp_table->clk);
+
+ opp_dev = list_first_entry(&opp_table->dev_list, struct opp_device,
+ node);
+
+ _remove_opp_dev(opp_dev, opp_table);
+
+ /* dev_list must be empty now */
+ WARN_ON(!list_empty(&opp_table->dev_list));
+
+ list_del_rcu(&opp_table->node);
+ call_srcu(&opp_table->srcu_head.srcu, &opp_table->rcu_head,
+ _kfree_device_rcu);
+}
+
/**
* _remove_opp_table() - Removes a OPP table
* @opp_table: OPP table to be removed.
@@ -894,8 +922,6 @@ static void _kfree_device_rcu(struct rcu_head *head)
*/
static void _remove_opp_table(struct opp_table *opp_table)
{
- struct opp_device *opp_dev;
-
if (!list_empty(&opp_table->opp_list))
return;
@@ -911,21 +937,7 @@ static void _remove_opp_table(struct opp_table *opp_table)
if (opp_table->set_opp)
return;
- /* Release clk */
- if (!IS_ERR(opp_table->clk))
- clk_put(opp_table->clk);
-
- opp_dev = list_first_entry(&opp_table->dev_list, struct opp_device,
- node);
-
- _remove_opp_dev(opp_dev, opp_table);
-
- /* dev_list must be empty now */
- WARN_ON(!list_empty(&opp_table->dev_list));
-
- list_del_rcu(&opp_table->node);
- call_srcu(&opp_table->srcu_head.srcu, &opp_table->rcu_head,
- _kfree_device_rcu);
+ _free_opp_table(opp_table);
}
void _opp_free(struct dev_pm_opp *opp)
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread