* [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