From: Viresh Kumar <viresh.kumar@linaro.org>
To: Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>,
Stephen Boyd <sboyd@kernel.org>
Cc: Viresh Kumar <viresh.kumar@linaro.org>,
Rafael Wysocki <rjw@rjwysocki.net>,
linux-pm@vger.kernel.org,
Vincent Guittot <vincent.guittot@linaro.org>,
Niklas Cassel <niklas.cassel@linaro.org>,
linux-kernel@vger.kernel.org
Subject: [PATCH V2 11/12] OPP: Prevent creating multiple OPP tables for devices sharing OPP nodes
Date: Wed, 19 Sep 2018 15:20:30 -0700 [thread overview]
Message-ID: <faf830f8d0612a2fa03144f09c9243f4c292400f.1537394233.git.viresh.kumar@linaro.org> (raw)
In-Reply-To: <cover.1537394233.git.viresh.kumar@linaro.org>
In-Reply-To: <cover.1537394233.git.viresh.kumar@linaro.org>
When two or more devices are sharing their clock and voltage rails, they
share the same OPP table. But there are some corner cases where the OPP
core incorrectly creates separate OPP tables for them.
For example, CPU 0 and 1 share clock/voltage rails. The platform
specific code calls dev_pm_opp_set_regulators() for CPU0 and the OPP
core creates an OPP table for it (the individual OPPs aren't initialized
as of now). The same is repeated for CPU1 then. Because
_opp_get_opp_table() doesn't compare DT node pointers currently, it
fails to find the link between CPU0 and CPU1 and so creates a new OPP
table.
Fix this by calling _managed_opp() from _opp_get_opp_table().
_managed_opp() gain an additional argument (index) to get the right node
pointer. This resulted in simplifying code in _of_add_opp_table_v2() as
well.
Tested-by: Niklas Cassel <niklas.cassel@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/opp/core.c | 25 ++++++++++++++++++++++---
drivers/opp/of.c | 35 +++++++++++++----------------------
drivers/opp/opp.h | 2 ++
3 files changed, 37 insertions(+), 25 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index d3e33fd32694..aaef20cf4df2 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -759,8 +759,8 @@ static void _remove_opp_dev(struct opp_device *opp_dev,
kfree(opp_dev);
}
-struct opp_device *_add_opp_dev(const struct device *dev,
- struct opp_table *opp_table)
+struct opp_device *_add_opp_dev_unlocked(const struct device *dev,
+ struct opp_table *opp_table)
{
struct opp_device *opp_dev;
int ret;
@@ -772,7 +772,6 @@ struct opp_device *_add_opp_dev(const struct device *dev,
/* Initialize opp-dev */
opp_dev->dev = dev;
- mutex_lock(&opp_table->lock);
list_add(&opp_dev->node, &opp_table->dev_list);
/* Create debugfs entries for the opp_table */
@@ -780,6 +779,17 @@ struct opp_device *_add_opp_dev(const struct device *dev,
if (ret)
dev_err(dev, "%s: Failed to register opp debugfs (%d)\n",
__func__, ret);
+
+ return opp_dev;
+}
+
+struct opp_device *_add_opp_dev(const struct device *dev,
+ struct opp_table *opp_table)
+{
+ struct opp_device *opp_dev;
+
+ mutex_lock(&opp_table->lock);
+ opp_dev = _add_opp_dev_unlocked(dev, opp_table);
mutex_unlock(&opp_table->lock);
return opp_dev;
@@ -844,6 +854,15 @@ static struct opp_table *_opp_get_opp_table(struct device *dev, int index)
if (!IS_ERR(opp_table))
goto unlock;
+ opp_table = _managed_opp(dev, index);
+ if (opp_table) {
+ if (!_add_opp_dev_unlocked(dev, opp_table)) {
+ dev_pm_opp_put_opp_table(opp_table);
+ opp_table = NULL;
+ }
+ goto unlock;
+ }
+
opp_table = _allocate_opp_table(dev, index);
unlock:
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index ae0436eaa911..1ddef52c27fd 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -41,11 +41,14 @@ struct device_node *dev_pm_opp_of_get_opp_desc_node(struct device *dev)
}
EXPORT_SYMBOL_GPL(dev_pm_opp_of_get_opp_desc_node);
-static struct opp_table *_managed_opp(const struct device_node *np)
+struct opp_table *_managed_opp(struct device *dev, int index)
{
struct opp_table *opp_table, *managed_table = NULL;
+ struct device_node *np;
- mutex_lock(&opp_table_lock);
+ np = _opp_of_get_opp_desc_node(dev->of_node, index);
+ if (!np)
+ return NULL;
list_for_each_entry(opp_table, &opp_tables, node) {
if (opp_table->np == np) {
@@ -65,7 +68,7 @@ static struct opp_table *_managed_opp(const struct device_node *np)
}
}
- mutex_unlock(&opp_table_lock);
+ of_node_put(np);
return managed_table;
}
@@ -401,30 +404,19 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np,
{
struct device_node *np;
struct opp_table *opp_table;
- int ret = 0, count = 0, pstate_count = 0;
+ int ret, count = 0, pstate_count = 0;
struct dev_pm_opp *opp;
- opp_table = _managed_opp(opp_np);
- if (opp_table) {
- /* OPPs are already managed */
- if (!_add_opp_dev(dev, opp_table)) {
- ret = -ENOMEM;
- goto put_opp_table;
- }
-
- if (opp_table->parsed_static_opps) {
- kref_get(&opp_table->list_kref);
- return 0;
- }
-
- goto initialize_static_opps;
- }
-
opp_table = dev_pm_opp_get_opp_table_indexed(dev, index);
if (!opp_table)
return -ENOMEM;
-initialize_static_opps:
+ /* OPP table is already initialized for the device */
+ if (opp_table->parsed_static_opps) {
+ kref_get(&opp_table->list_kref);
+ return 0;
+ }
+
kref_init(&opp_table->list_kref);
/* We have opp-table node now, iterate over it and add OPPs */
@@ -466,7 +458,6 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np,
put_list_kref:
_put_opp_list_kref(opp_table);
-put_opp_table:
dev_pm_opp_put_opp_table(opp_table);
return ret;
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index f9fbb7553fc4..9c6544b4f4f9 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -206,8 +206,10 @@ void _put_opp_list_kref(struct opp_table *opp_table);
#ifdef CONFIG_OF
void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index);
+struct opp_table *_managed_opp(struct device *dev, int index);
#else
static inline void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index) {}
+static inline struct opp_table *_managed_opp(struct device *dev, int index) { return NULL; }
#endif
#ifdef CONFIG_DEBUG_FS
--
2.14.1
next prev parent reply other threads:[~2018-09-19 22:21 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-19 22:20 [PATCH V2 00/12] OPP: Don't create multiple OPP tables for devices sharing OPP table Viresh Kumar
2018-09-19 22:20 ` [PATCH V2 01/12] OPP: Free OPP table properly on performance state irregularities Viresh Kumar
2018-09-19 22:20 ` [PATCH V2 02/12] OPP: Don't try to remove all OPP tables on failure Viresh Kumar
2018-09-19 22:20 ` [PATCH V2 03/12] OPP: Protect dev_list with opp_table lock Viresh Kumar
2018-09-19 22:20 ` [PATCH V2 04/12] OPP: Pass index to _of_init_opp_table() Viresh Kumar
2018-09-19 22:20 ` [PATCH V2 05/12] OPP: Parse OPP table's DT properties from _of_init_opp_table() Viresh Kumar
2018-09-19 22:20 ` [PATCH V2 06/12] OPP: Don't take OPP table's kref for static OPPs Viresh Kumar
2018-09-19 22:20 ` [PATCH V2 07/12] OPP: Create separate kref for static OPPs list Viresh Kumar
2018-09-19 22:20 ` [PATCH V2 08/12] cpufreq: mvebu: Remove OPPs using dev_pm_opp_remove() Viresh Kumar
2018-09-20 7:59 ` Rafael J. Wysocki
2018-09-19 22:20 ` [PATCH V2 09/12] OPP: Don't remove dynamic OPPs from _dev_pm_opp_remove_table() Viresh Kumar
2018-09-19 22:20 ` [PATCH V2 10/12] OPP: Use a single mechanism to free the OPP table Viresh Kumar
2018-09-19 22:20 ` Viresh Kumar [this message]
2018-09-19 22:20 ` [PATCH V2 12/12] OPP: Pass OPP table to _of_add_opp_table_v{1|2}() Viresh Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=faf830f8d0612a2fa03144f09c9243f4c292400f.1537394233.git.viresh.kumar@linaro.org \
--to=viresh.kumar@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=niklas.cassel@linaro.org \
--cc=nm@ti.com \
--cc=rjw@rjwysocki.net \
--cc=sboyd@kernel.org \
--cc=vincent.guittot@linaro.org \
--cc=vireshk@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).