* [PATCH V3 0/4] opp: general cleanups
@ 2020-08-20 7:56 Viresh Kumar
2020-08-20 7:56 ` [PATCH V3 1/4] opp: Rename regulator_enabled and use it as status of all resources Viresh Kumar
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Viresh Kumar @ 2020-08-20 7:56 UTC (permalink / raw)
To: rnayak, Nishanth Menon, Stephen Boyd, Viresh Kumar
Cc: Viresh Kumar, linux-pm, Vincent Guittot, Rafael Wysocki, sibis,
sbhanu, linux-kernel
Hi,
Here is another version of the cleanups I sent earlier.
Rajendra: Please see if these work fine now.
V3:
- Dropped v2 1/4 as it is already merged.
- New patch 4/4 added.
- Reordered the first two patches here (Stephen)
- disable regulator only if present
Viresh Kumar (4):
opp: Rename regulator_enabled and use it as status of all resources
opp: Track device's resources configuration status
opp: Split out _opp_set_rate_zero()
opp: Remove _dev_pm_opp_find_and_remove_table() wrapper
drivers/opp/core.c | 103 +++++++++++++++++++++------------------------
drivers/opp/cpu.c | 2 +-
drivers/opp/of.c | 2 +-
drivers/opp/opp.h | 5 +--
4 files changed, 52 insertions(+), 60 deletions(-)
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V3 1/4] opp: Rename regulator_enabled and use it as status of all resources
2020-08-20 7:56 [PATCH V3 0/4] opp: general cleanups Viresh Kumar
@ 2020-08-20 7:56 ` Viresh Kumar
2020-08-20 7:56 ` [PATCH V3 2/4] opp: Reuse the enabled flag in !target_freq path Viresh Kumar
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Viresh Kumar @ 2020-08-20 7:56 UTC (permalink / raw)
To: rnayak, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: Viresh Kumar, linux-pm, Vincent Guittot, Rafael Wysocki, sibis,
sbhanu, linux-kernel
Expand the scope of the regulator_enabled flag and use it to track
status of all the resources. This will be used for other stuff in the
next patch.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/opp/core.c | 19 +++++++++----------
drivers/opp/opp.h | 4 ++--
2 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 9668ea04cc80..6f43ef4945b7 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -703,12 +703,10 @@ static int _generic_set_opp_regulator(struct opp_table *opp_table,
* Enable the regulator after setting its voltages, otherwise it breaks
* some boot-enabled regulators.
*/
- if (unlikely(!opp_table->regulator_enabled)) {
+ if (unlikely(!opp_table->enabled)) {
ret = regulator_enable(reg);
if (ret < 0)
dev_warn(dev, "Failed to enable regulator: %d", ret);
- else
- opp_table->regulator_enabled = true;
}
return 0;
@@ -909,12 +907,12 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
if (ret)
goto put_opp_table;
- if (opp_table->regulator_enabled) {
+ if (opp_table->regulators)
regulator_disable(opp_table->regulators[0]);
- opp_table->regulator_enabled = false;
- }
ret = _set_required_opps(dev, opp_table, NULL);
+
+ opp_table->enabled = false;
goto put_opp_table;
}
@@ -1001,8 +999,11 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
dev_err(dev, "Failed to set required opps: %d\n", ret);
}
- if (!ret)
+ if (!ret) {
ret = _set_opp_bw(opp_table, opp, dev, false);
+ if (!ret)
+ opp_table->enabled = true;
+ }
put_opp:
dev_pm_opp_put(opp);
@@ -1796,11 +1797,9 @@ void dev_pm_opp_put_regulators(struct opp_table *opp_table)
/* Make sure there are no concurrent readers while updating opp_table */
WARN_ON(!list_empty(&opp_table->opp_list));
- if (opp_table->regulator_enabled) {
+ if (opp_table->enabled) {
for (i = opp_table->regulator_count - 1; i >= 0; i--)
regulator_disable(opp_table->regulators[i]);
-
- opp_table->regulator_enabled = false;
}
for (i = opp_table->regulator_count - 1; i >= 0; i--)
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index e51646ff279e..0c3de3f6db5c 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -147,11 +147,11 @@ enum opp_table_access {
* @clk: Device's clock handle
* @regulators: Supply regulators
* @regulator_count: Number of power supply regulators. Its value can be -1
- * @regulator_enabled: Set to true if regulators were previously enabled.
* (uninitialized), 0 (no opp-microvolt property) or > 0 (has opp-microvolt
* property).
* @paths: Interconnect path handles
* @path_count: Number of interconnect paths
+ * @enabled: Set to true if the device's resources are enabled/configured.
* @genpd_performance_state: Device's power domain support performance state.
* @is_genpd: Marks if the OPP table belongs to a genpd.
* @set_opp: Platform specific set_opp callback
@@ -195,9 +195,9 @@ struct opp_table {
struct clk *clk;
struct regulator **regulators;
int regulator_count;
- bool regulator_enabled;
struct icc_path **paths;
unsigned int path_count;
+ bool enabled;
bool genpd_performance_state;
bool is_genpd;
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V3 2/4] opp: Reuse the enabled flag in !target_freq path
2020-08-20 7:56 [PATCH V3 0/4] opp: general cleanups Viresh Kumar
2020-08-20 7:56 ` [PATCH V3 1/4] opp: Rename regulator_enabled and use it as status of all resources Viresh Kumar
@ 2020-08-20 7:56 ` Viresh Kumar
2020-08-20 7:56 ` [PATCH V3 3/4] opp: Split out _opp_set_rate_zero() Viresh Kumar
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Viresh Kumar @ 2020-08-20 7:56 UTC (permalink / raw)
To: rnayak, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: Viresh Kumar, linux-pm, Vincent Guittot, Rafael Wysocki, sibis,
sbhanu, linux-kernel
The OPP core needs to track if the resources of devices are
enabled/configured or not, as it disables the resources when target_freq
is set to 0.
Handle that with the new enabled flag and remove otherwise complex
conditional statements.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/opp/core.c | 29 +++++++++++------------------
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 6f43ef4945b7..0b437d483b75 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -886,22 +886,18 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
}
if (unlikely(!target_freq)) {
+ ret = 0;
+
+ if (!opp_table->enabled)
+ goto put_opp_table;
+
/*
* Some drivers need to support cases where some platforms may
* have OPP table for the device, while others don't and
* opp_set_rate() just needs to behave like clk_set_rate().
*/
- if (!_get_opp_count(opp_table)) {
- ret = 0;
- goto put_opp_table;
- }
-
- if (!opp_table->required_opp_tables && !opp_table->regulators &&
- !opp_table->paths) {
- dev_err(dev, "target frequency can't be 0\n");
- ret = -EINVAL;
+ if (!_get_opp_count(opp_table))
goto put_opp_table;
- }
ret = _set_opp_bw(opp_table, NULL, dev, true);
if (ret)
@@ -931,14 +927,11 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
old_freq = clk_get_rate(clk);
/* Return early if nothing to do */
- if (old_freq == freq) {
- if (!opp_table->required_opp_tables && !opp_table->regulators &&
- !opp_table->paths) {
- dev_dbg(dev, "%s: old/new frequencies (%lu Hz) are same, nothing to do\n",
- __func__, freq);
- ret = 0;
- goto put_opp_table;
- }
+ if (opp_table->enabled && old_freq == freq) {
+ dev_dbg(dev, "%s: old/new frequencies (%lu Hz) are same, nothing to do\n",
+ __func__, freq);
+ ret = 0;
+ goto put_opp_table;
}
/*
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V3 3/4] opp: Split out _opp_set_rate_zero()
2020-08-20 7:56 [PATCH V3 0/4] opp: general cleanups Viresh Kumar
2020-08-20 7:56 ` [PATCH V3 1/4] opp: Rename regulator_enabled and use it as status of all resources Viresh Kumar
2020-08-20 7:56 ` [PATCH V3 2/4] opp: Reuse the enabled flag in !target_freq path Viresh Kumar
@ 2020-08-20 7:56 ` Viresh Kumar
2020-08-20 7:56 ` [PATCH V3 4/4] opp: Remove _dev_pm_opp_find_and_remove_table() wrapper Viresh Kumar
2020-08-23 5:48 ` [PATCH V3 0/4] opp: general cleanups Rajendra Nayak
4 siblings, 0 replies; 6+ messages in thread
From: Viresh Kumar @ 2020-08-20 7:56 UTC (permalink / raw)
To: rnayak, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: Viresh Kumar, linux-pm, Vincent Guittot, Rafael Wysocki, sibis,
sbhanu, linux-kernel
Create separate routine _opp_set_rate_zero() to handle !target_freq
case.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/opp/core.c | 52 ++++++++++++++++++++++++++--------------------
1 file changed, 29 insertions(+), 23 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 0b437d483b75..4edd2c3d6d91 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -860,6 +860,34 @@ int dev_pm_opp_set_bw(struct device *dev, struct dev_pm_opp *opp)
}
EXPORT_SYMBOL_GPL(dev_pm_opp_set_bw);
+static int _opp_set_rate_zero(struct device *dev, struct opp_table *opp_table)
+{
+ int ret;
+
+ if (!opp_table->enabled)
+ return 0;
+
+ /*
+ * Some drivers need to support cases where some platforms may
+ * have OPP table for the device, while others don't and
+ * opp_set_rate() just needs to behave like clk_set_rate().
+ */
+ if (!_get_opp_count(opp_table))
+ return 0;
+
+ ret = _set_opp_bw(opp_table, NULL, dev, true);
+ if (ret)
+ return ret;
+
+ if (opp_table->regulators)
+ regulator_disable(opp_table->regulators[0]);
+
+ ret = _set_required_opps(dev, opp_table, NULL);
+
+ opp_table->enabled = false;
+ return ret;
+}
+
/**
* dev_pm_opp_set_rate() - Configure new OPP based on frequency
* @dev: device for which we do this operation
@@ -886,29 +914,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
}
if (unlikely(!target_freq)) {
- ret = 0;
-
- if (!opp_table->enabled)
- goto put_opp_table;
-
- /*
- * Some drivers need to support cases where some platforms may
- * have OPP table for the device, while others don't and
- * opp_set_rate() just needs to behave like clk_set_rate().
- */
- if (!_get_opp_count(opp_table))
- goto put_opp_table;
-
- ret = _set_opp_bw(opp_table, NULL, dev, true);
- if (ret)
- goto put_opp_table;
-
- if (opp_table->regulators)
- regulator_disable(opp_table->regulators[0]);
-
- ret = _set_required_opps(dev, opp_table, NULL);
-
- opp_table->enabled = false;
+ ret = _opp_set_rate_zero(dev, opp_table);
goto put_opp_table;
}
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V3 4/4] opp: Remove _dev_pm_opp_find_and_remove_table() wrapper
2020-08-20 7:56 [PATCH V3 0/4] opp: general cleanups Viresh Kumar
` (2 preceding siblings ...)
2020-08-20 7:56 ` [PATCH V3 3/4] opp: Split out _opp_set_rate_zero() Viresh Kumar
@ 2020-08-20 7:56 ` Viresh Kumar
2020-08-23 5:48 ` [PATCH V3 0/4] opp: general cleanups Rajendra Nayak
4 siblings, 0 replies; 6+ messages in thread
From: Viresh Kumar @ 2020-08-20 7:56 UTC (permalink / raw)
To: rnayak, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: Viresh Kumar, linux-pm, Vincent Guittot, Rafael Wysocki, sibis,
sbhanu, linux-kernel
Remove the unnecessary wrapper and merge
_dev_pm_opp_find_and_remove_table() with dev_pm_opp_remove_table().
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/opp/core.c | 21 ++++++++-------------
drivers/opp/cpu.c | 2 +-
drivers/opp/of.c | 2 +-
drivers/opp/opp.h | 1 -
4 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 4edd2c3d6d91..6978b9218c6e 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2395,7 +2395,14 @@ int dev_pm_opp_unregister_notifier(struct device *dev,
}
EXPORT_SYMBOL(dev_pm_opp_unregister_notifier);
-void _dev_pm_opp_find_and_remove_table(struct device *dev)
+/**
+ * dev_pm_opp_remove_table() - Free all OPPs associated with the device
+ * @dev: device pointer used to lookup OPP table.
+ *
+ * Free both OPPs created using static entries present in DT and the
+ * dynamically added entries.
+ */
+void dev_pm_opp_remove_table(struct device *dev)
{
struct opp_table *opp_table;
@@ -2420,16 +2427,4 @@ void _dev_pm_opp_find_and_remove_table(struct device *dev)
/* Drop reference taken while the OPP table was added */
dev_pm_opp_put_opp_table(opp_table);
}
-
-/**
- * dev_pm_opp_remove_table() - Free all OPPs associated with the device
- * @dev: device pointer used to lookup OPP table.
- *
- * Free both OPPs created using static entries present in DT and the
- * dynamically added entries.
- */
-void dev_pm_opp_remove_table(struct device *dev)
-{
- _dev_pm_opp_find_and_remove_table(dev);
-}
EXPORT_SYMBOL_GPL(dev_pm_opp_remove_table);
diff --git a/drivers/opp/cpu.c b/drivers/opp/cpu.c
index b5055cc886ef..5004335cf0de 100644
--- a/drivers/opp/cpu.c
+++ b/drivers/opp/cpu.c
@@ -124,7 +124,7 @@ void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask,
continue;
}
- _dev_pm_opp_find_and_remove_table(cpu_dev);
+ dev_pm_opp_remove_table(cpu_dev);
}
}
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 0430290670ab..7d9d4455a59e 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -616,7 +616,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_find_and_remove_table(dev);
+ dev_pm_opp_remove_table(dev);
}
EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table);
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index 0c3de3f6db5c..78e876ec803e 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -217,7 +217,6 @@ void _get_opp_table_kref(struct opp_table *opp_table);
int _get_opp_count(struct opp_table *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_find_and_remove_table(struct device *dev);
struct dev_pm_opp *_opp_allocate(struct opp_table *opp_table);
void _opp_free(struct dev_pm_opp *opp);
int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2);
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V3 0/4] opp: general cleanups
2020-08-20 7:56 [PATCH V3 0/4] opp: general cleanups Viresh Kumar
` (3 preceding siblings ...)
2020-08-20 7:56 ` [PATCH V3 4/4] opp: Remove _dev_pm_opp_find_and_remove_table() wrapper Viresh Kumar
@ 2020-08-23 5:48 ` Rajendra Nayak
4 siblings, 0 replies; 6+ messages in thread
From: Rajendra Nayak @ 2020-08-23 5:48 UTC (permalink / raw)
To: Viresh Kumar, Nishanth Menon, Stephen Boyd, Viresh Kumar
Cc: linux-pm, Vincent Guittot, Rafael Wysocki, sibis, sbhanu,
linux-kernel
On 8/20/2020 1:26 PM, Viresh Kumar wrote:
> Hi,
>
> Here is another version of the cleanups I sent earlier.
>
> Rajendra: Please see if these work fine now.
I gave these a quick spin, and they don';t result in the crash I
earlier observed
Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
>
> V3:
> - Dropped v2 1/4 as it is already merged.
> - New patch 4/4 added.
> - Reordered the first two patches here (Stephen)
> - disable regulator only if present
>
> Viresh Kumar (4):
> opp: Rename regulator_enabled and use it as status of all resources
> opp: Track device's resources configuration status
> opp: Split out _opp_set_rate_zero()
> opp: Remove _dev_pm_opp_find_and_remove_table() wrapper
>
> drivers/opp/core.c | 103 +++++++++++++++++++++------------------------
> drivers/opp/cpu.c | 2 +-
> drivers/opp/of.c | 2 +-
> drivers/opp/opp.h | 5 +--
> 4 files changed, 52 insertions(+), 60 deletions(-)
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-08-23 5:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-20 7:56 [PATCH V3 0/4] opp: general cleanups Viresh Kumar
2020-08-20 7:56 ` [PATCH V3 1/4] opp: Rename regulator_enabled and use it as status of all resources Viresh Kumar
2020-08-20 7:56 ` [PATCH V3 2/4] opp: Reuse the enabled flag in !target_freq path Viresh Kumar
2020-08-20 7:56 ` [PATCH V3 3/4] opp: Split out _opp_set_rate_zero() Viresh Kumar
2020-08-20 7:56 ` [PATCH V3 4/4] opp: Remove _dev_pm_opp_find_and_remove_table() wrapper Viresh Kumar
2020-08-23 5:48 ` [PATCH V3 0/4] opp: general cleanups Rajendra Nayak
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.