* [PATCH V2 1/6] cpufreq: s3c24xx: Remove useless checks
2016-06-02 14:04 [PATCH V2 0/6] cpufreq: cleanups and reorganization Viresh Kumar
@ 2016-06-02 14:04 ` Viresh Kumar
2016-06-02 14:04 ` [PATCH V2 2/6] cpufreq: Remove cpufreq_frequency_get_table() Viresh Kumar
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2016-06-02 14:04 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Krzysztof Kozlowski
These aren't required at all, remove them.
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/s3c24xx-cpufreq.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/cpufreq/s3c24xx-cpufreq.c b/drivers/cpufreq/s3c24xx-cpufreq.c
index ae8eaed77b70..4567c3cab095 100644
--- a/drivers/cpufreq/s3c24xx-cpufreq.c
+++ b/drivers/cpufreq/s3c24xx-cpufreq.c
@@ -571,11 +571,7 @@ static int s3c_cpufreq_build_freq(void)
{
int size, ret;
- if (!cpu_cur.info->calc_freqtable)
- return -EINVAL;
-
kfree(ftab);
- ftab = NULL;
size = cpu_cur.info->calc_freqtable(&cpu_cur, NULL, 0);
size++;
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V2 2/6] cpufreq: Remove cpufreq_frequency_get_table()
2016-06-02 14:04 [PATCH V2 0/6] cpufreq: cleanups and reorganization Viresh Kumar
2016-06-02 14:04 ` [PATCH V2 1/6] cpufreq: s3c24xx: Remove useless checks Viresh Kumar
@ 2016-06-02 14:04 ` Viresh Kumar
2016-06-02 14:59 ` Javi Merino
2016-06-02 14:04 ` [PATCH V2 3/6] cpufreq: ondemand: Don't keep a copy of freq_table pointer Viresh Kumar
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Viresh Kumar @ 2016-06-02 14:04 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Amit Daniel Kachhap, Javi Merino,
Zhang Rui, Eduardo Valentin
Cc: linaro-kernel, linux-pm, linux-kernel
Most of the callers of cpufreq_frequency_get_table() already have the
pointer to a valid 'policy' structure and they don't really need to go
through the per-cpu variable first and then a check to validate the
frequency, in order to find the freq-table for the policy.
Directly use the policy->freq_table field instead for them.
Only one user of that API is left after above changes, cpu_cooling.c and
it accesses the freq_table in a racy way as the policy can get freed in
between.
Fix it by using cpufreq_cpu_get() properly.
Since there are no more users of cpufreq_frequency_get_table() left, get
rid of it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 38 +++++++++++++----------------------
drivers/cpufreq/cpufreq_ondemand.c | 2 +-
drivers/cpufreq/cpufreq_stats.c | 3 +--
drivers/cpufreq/freq_table.c | 9 +++------
drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 3 +--
drivers/thermal/cpu_cooling.c | 22 +++++++++++++++-----
include/linux/cpufreq.h | 2 --
7 files changed, 37 insertions(+), 42 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index fbc97b1fa371..1833eda1f9d4 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -126,15 +126,6 @@ struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy)
}
EXPORT_SYMBOL_GPL(get_governor_parent_kobj);
-struct cpufreq_frequency_table *cpufreq_frequency_get_table(unsigned int cpu)
-{
- struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
-
- return policy && !policy_is_inactive(policy) ?
- policy->freq_table : NULL;
-}
-EXPORT_SYMBOL_GPL(cpufreq_frequency_get_table);
-
static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
{
u64 idle_time;
@@ -1950,7 +1941,7 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
if (!cpufreq_driver->target_index)
return -EINVAL;
- freq_table = cpufreq_frequency_get_table(policy->cpu);
+ freq_table = policy->freq_table;
if (unlikely(!freq_table)) {
pr_err("%s: Unable to find freq_table\n", __func__);
return -EINVAL;
@@ -2345,26 +2336,25 @@ static struct notifier_block __refdata cpufreq_cpu_notifier = {
*********************************************************************/
static int cpufreq_boost_set_sw(int state)
{
- struct cpufreq_frequency_table *freq_table;
struct cpufreq_policy *policy;
int ret = -EINVAL;
for_each_active_policy(policy) {
- freq_table = cpufreq_frequency_get_table(policy->cpu);
- if (freq_table) {
- ret = cpufreq_frequency_table_cpuinfo(policy,
- freq_table);
- if (ret) {
- pr_err("%s: Policy frequency update failed\n",
- __func__);
- break;
- }
+ if (!policy->freq_table)
+ continue;
- down_write(&policy->rwsem);
- policy->user_policy.max = policy->max;
- cpufreq_governor_limits(policy);
- up_write(&policy->rwsem);
+ ret = cpufreq_frequency_table_cpuinfo(policy,
+ policy->freq_table);
+ if (ret) {
+ pr_err("%s: Policy frequency update failed\n",
+ __func__);
+ break;
}
+
+ down_write(&policy->rwsem);
+ policy->user_policy.max = policy->max;
+ cpufreq_governor_limits(policy);
+ up_write(&policy->rwsem);
}
return ret;
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index c84fc2240d49..4d2fe2710c5d 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -113,7 +113,7 @@ static void ondemand_powersave_bias_init(struct cpufreq_policy *policy)
{
struct od_policy_dbs_info *dbs_info = to_dbs_info(policy->governor_data);
- dbs_info->freq_table = cpufreq_frequency_get_table(policy->cpu);
+ dbs_info->freq_table = policy->freq_table;
dbs_info->freq_lo = 0;
}
diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index c6e7f81a0397..06d3abdffd3a 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -157,11 +157,10 @@ void cpufreq_stats_create_table(struct cpufreq_policy *policy)
unsigned int i = 0, count = 0, ret = -ENOMEM;
struct cpufreq_stats *stats;
unsigned int alloc_size;
- unsigned int cpu = policy->cpu;
struct cpufreq_frequency_table *pos, *table;
/* We need cpufreq table for creating stats table */
- table = cpufreq_frequency_get_table(cpu);
+ table = policy->freq_table;
if (unlikely(!table))
return;
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index 4e5c5dbfed7a..f52b5473b1f4 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -106,12 +106,10 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_verify);
*/
int cpufreq_generic_frequency_table_verify(struct cpufreq_policy *policy)
{
- struct cpufreq_frequency_table *table =
- cpufreq_frequency_get_table(policy->cpu);
- if (!table)
+ if (!policy->freq_table)
return -ENODEV;
- return cpufreq_frequency_table_verify(policy, table);
+ return cpufreq_frequency_table_verify(policy, policy->freq_table);
}
EXPORT_SYMBOL_GPL(cpufreq_generic_frequency_table_verify);
@@ -210,9 +208,8 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_target);
int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy,
unsigned int freq)
{
- struct cpufreq_frequency_table *pos, *table;
+ struct cpufreq_frequency_table *pos, *table = policy->freq_table;
- table = cpufreq_frequency_get_table(policy->cpu);
if (unlikely(!table)) {
pr_debug("%s: Unable to find frequency table\n", __func__);
return -ENOENT;
diff --git a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
index 7c4cd5c634f2..dc112481a408 100644
--- a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
+++ b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
@@ -94,7 +94,7 @@ static int pmi_notifier(struct notifier_block *nb,
unsigned long event, void *data)
{
struct cpufreq_policy *policy = data;
- struct cpufreq_frequency_table *cbe_freqs;
+ struct cpufreq_frequency_table *cbe_freqs = policy->freq_table;
u8 node;
/* Should this really be called for CPUFREQ_ADJUST and CPUFREQ_NOTIFY
@@ -103,7 +103,6 @@ static int pmi_notifier(struct notifier_block *nb,
if (event == CPUFREQ_START)
return 0;
- cbe_freqs = cpufreq_frequency_get_table(policy->cpu);
node = cbe_cpu_to_node(policy->cpu);
pr_debug("got notified, event=%lu, node=%u\n", event, node);
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 6ceac4f2d4b2..63f760869651 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -787,6 +787,7 @@ __cpufreq_cooling_register(struct device_node *np,
const struct cpumask *clip_cpus, u32 capacitance,
get_static_t plat_static_func)
{
+ struct cpufreq_policy *policy;
struct thermal_cooling_device *cool_dev;
struct cpufreq_cooling_device *cpufreq_dev;
char dev_name[THERMAL_NAME_LENGTH];
@@ -794,15 +795,24 @@ __cpufreq_cooling_register(struct device_node *np,
unsigned int freq, i, num_cpus;
int ret;
- table = cpufreq_frequency_get_table(cpumask_first(clip_cpus));
+ policy = cpufreq_cpu_get(cpumask_first(clip_cpus));
+ if (!policy) {
+ pr_debug("%s: CPUFreq policy not found\n", __func__);
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
+ table = policy->freq_table;
if (!table) {
pr_debug("%s: CPUFreq table not found\n", __func__);
- return ERR_PTR(-EPROBE_DEFER);
+ cool_dev = ERR_PTR(-ENODEV);
+ goto put_policy;
}
cpufreq_dev = kzalloc(sizeof(*cpufreq_dev), GFP_KERNEL);
- if (!cpufreq_dev)
- return ERR_PTR(-ENOMEM);
+ if (!cpufreq_dev) {
+ cool_dev = ERR_PTR(-ENOMEM);
+ goto put_policy;
+ }
num_cpus = cpumask_weight(clip_cpus);
cpufreq_dev->time_in_idle = kcalloc(num_cpus,
@@ -892,7 +902,7 @@ __cpufreq_cooling_register(struct device_node *np,
CPUFREQ_POLICY_NOTIFIER);
mutex_unlock(&cooling_cpufreq_lock);
- return cool_dev;
+ goto put_policy;
remove_idr:
release_idr(&cpufreq_idr, cpufreq_dev->id);
@@ -906,6 +916,8 @@ __cpufreq_cooling_register(struct device_node *np,
kfree(cpufreq_dev->time_in_idle);
free_cdev:
kfree(cpufreq_dev);
+put_policy:
+ cpufreq_cpu_put(policy);
return cool_dev;
}
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 7ed93c310c08..1342cbc0f25e 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -632,8 +632,6 @@ static inline bool policy_has_boost_freq(struct cpufreq_policy *policy)
return false;
}
#endif
-/* the following funtion is for cpufreq core use only */
-struct cpufreq_frequency_table *cpufreq_frequency_get_table(unsigned int cpu);
/* the following are really really optional */
extern struct freq_attr cpufreq_freq_attr_scaling_available_freqs;
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH V2 2/6] cpufreq: Remove cpufreq_frequency_get_table()
2016-06-02 14:04 ` [PATCH V2 2/6] cpufreq: Remove cpufreq_frequency_get_table() Viresh Kumar
@ 2016-06-02 14:59 ` Javi Merino
2016-06-02 15:36 ` Viresh Kumar
0 siblings, 1 reply; 11+ messages in thread
From: Javi Merino @ 2016-06-02 14:59 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Amit Daniel Kachhap, Zhang Rui, Eduardo Valentin,
linaro-kernel, linux-pm, linux-kernel
Hi Viresh,
On Thu, Jun 02, 2016 at 07:34:56PM +0530, Viresh Kumar wrote:
> Most of the callers of cpufreq_frequency_get_table() already have the
> pointer to a valid 'policy' structure and they don't really need to go
> through the per-cpu variable first and then a check to validate the
> frequency, in order to find the freq-table for the policy.
>
> Directly use the policy->freq_table field instead for them.
>
> Only one user of that API is left after above changes, cpu_cooling.c and
> it accesses the freq_table in a racy way as the policy can get freed in
> between.
>
> Fix it by using cpufreq_cpu_get() properly.
In 5a31d594a973 ("cpufreq: Allow freq_table to be obtained for offline
CPUs") you did the opposite: don't use cpufreq_cpu_get_raw() because
it won't give you the policy of a cpu that is offline. Now you are
arguing that we should go back to cpufreq_cpu_get() which implicitly
calls cpufreq_cpu_get_raw(). Won't we hit the same issue that
5a31d594a973 was trying to prevent: that we can't get a freq_table for
a cpu that is offline?
Cheers,
Javi
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/cpufreq.c | 38 +++++++++++++----------------------
> drivers/cpufreq/cpufreq_ondemand.c | 2 +-
> drivers/cpufreq/cpufreq_stats.c | 3 +--
> drivers/cpufreq/freq_table.c | 9 +++------
> drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 3 +--
> drivers/thermal/cpu_cooling.c | 22 +++++++++++++++-----
> include/linux/cpufreq.h | 2 --
> 7 files changed, 37 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index fbc97b1fa371..1833eda1f9d4 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -126,15 +126,6 @@ struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy)
> }
> EXPORT_SYMBOL_GPL(get_governor_parent_kobj);
>
> -struct cpufreq_frequency_table *cpufreq_frequency_get_table(unsigned int cpu)
> -{
> - struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
> -
> - return policy && !policy_is_inactive(policy) ?
> - policy->freq_table : NULL;
> -}
> -EXPORT_SYMBOL_GPL(cpufreq_frequency_get_table);
> -
> static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
> {
> u64 idle_time;
> @@ -1950,7 +1941,7 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
> if (!cpufreq_driver->target_index)
> return -EINVAL;
>
> - freq_table = cpufreq_frequency_get_table(policy->cpu);
> + freq_table = policy->freq_table;
> if (unlikely(!freq_table)) {
> pr_err("%s: Unable to find freq_table\n", __func__);
> return -EINVAL;
> @@ -2345,26 +2336,25 @@ static struct notifier_block __refdata cpufreq_cpu_notifier = {
> *********************************************************************/
> static int cpufreq_boost_set_sw(int state)
> {
> - struct cpufreq_frequency_table *freq_table;
> struct cpufreq_policy *policy;
> int ret = -EINVAL;
>
> for_each_active_policy(policy) {
> - freq_table = cpufreq_frequency_get_table(policy->cpu);
> - if (freq_table) {
> - ret = cpufreq_frequency_table_cpuinfo(policy,
> - freq_table);
> - if (ret) {
> - pr_err("%s: Policy frequency update failed\n",
> - __func__);
> - break;
> - }
> + if (!policy->freq_table)
> + continue;
>
> - down_write(&policy->rwsem);
> - policy->user_policy.max = policy->max;
> - cpufreq_governor_limits(policy);
> - up_write(&policy->rwsem);
> + ret = cpufreq_frequency_table_cpuinfo(policy,
> + policy->freq_table);
> + if (ret) {
> + pr_err("%s: Policy frequency update failed\n",
> + __func__);
> + break;
> }
> +
> + down_write(&policy->rwsem);
> + policy->user_policy.max = policy->max;
> + cpufreq_governor_limits(policy);
> + up_write(&policy->rwsem);
> }
>
> return ret;
> diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
> index c84fc2240d49..4d2fe2710c5d 100644
> --- a/drivers/cpufreq/cpufreq_ondemand.c
> +++ b/drivers/cpufreq/cpufreq_ondemand.c
> @@ -113,7 +113,7 @@ static void ondemand_powersave_bias_init(struct cpufreq_policy *policy)
> {
> struct od_policy_dbs_info *dbs_info = to_dbs_info(policy->governor_data);
>
> - dbs_info->freq_table = cpufreq_frequency_get_table(policy->cpu);
> + dbs_info->freq_table = policy->freq_table;
> dbs_info->freq_lo = 0;
> }
>
> diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
> index c6e7f81a0397..06d3abdffd3a 100644
> --- a/drivers/cpufreq/cpufreq_stats.c
> +++ b/drivers/cpufreq/cpufreq_stats.c
> @@ -157,11 +157,10 @@ void cpufreq_stats_create_table(struct cpufreq_policy *policy)
> unsigned int i = 0, count = 0, ret = -ENOMEM;
> struct cpufreq_stats *stats;
> unsigned int alloc_size;
> - unsigned int cpu = policy->cpu;
> struct cpufreq_frequency_table *pos, *table;
>
> /* We need cpufreq table for creating stats table */
> - table = cpufreq_frequency_get_table(cpu);
> + table = policy->freq_table;
> if (unlikely(!table))
> return;
>
> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
> index 4e5c5dbfed7a..f52b5473b1f4 100644
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -106,12 +106,10 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_verify);
> */
> int cpufreq_generic_frequency_table_verify(struct cpufreq_policy *policy)
> {
> - struct cpufreq_frequency_table *table =
> - cpufreq_frequency_get_table(policy->cpu);
> - if (!table)
> + if (!policy->freq_table)
> return -ENODEV;
>
> - return cpufreq_frequency_table_verify(policy, table);
> + return cpufreq_frequency_table_verify(policy, policy->freq_table);
> }
> EXPORT_SYMBOL_GPL(cpufreq_generic_frequency_table_verify);
>
> @@ -210,9 +208,8 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_target);
> int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy,
> unsigned int freq)
> {
> - struct cpufreq_frequency_table *pos, *table;
> + struct cpufreq_frequency_table *pos, *table = policy->freq_table;
>
> - table = cpufreq_frequency_get_table(policy->cpu);
> if (unlikely(!table)) {
> pr_debug("%s: Unable to find frequency table\n", __func__);
> return -ENOENT;
> diff --git a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
> index 7c4cd5c634f2..dc112481a408 100644
> --- a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
> +++ b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
> @@ -94,7 +94,7 @@ static int pmi_notifier(struct notifier_block *nb,
> unsigned long event, void *data)
> {
> struct cpufreq_policy *policy = data;
> - struct cpufreq_frequency_table *cbe_freqs;
> + struct cpufreq_frequency_table *cbe_freqs = policy->freq_table;
> u8 node;
>
> /* Should this really be called for CPUFREQ_ADJUST and CPUFREQ_NOTIFY
> @@ -103,7 +103,6 @@ static int pmi_notifier(struct notifier_block *nb,
> if (event == CPUFREQ_START)
> return 0;
>
> - cbe_freqs = cpufreq_frequency_get_table(policy->cpu);
> node = cbe_cpu_to_node(policy->cpu);
>
> pr_debug("got notified, event=%lu, node=%u\n", event, node);
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index 6ceac4f2d4b2..63f760869651 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -787,6 +787,7 @@ __cpufreq_cooling_register(struct device_node *np,
> const struct cpumask *clip_cpus, u32 capacitance,
> get_static_t plat_static_func)
> {
> + struct cpufreq_policy *policy;
> struct thermal_cooling_device *cool_dev;
> struct cpufreq_cooling_device *cpufreq_dev;
> char dev_name[THERMAL_NAME_LENGTH];
> @@ -794,15 +795,24 @@ __cpufreq_cooling_register(struct device_node *np,
> unsigned int freq, i, num_cpus;
> int ret;
>
> - table = cpufreq_frequency_get_table(cpumask_first(clip_cpus));
> + policy = cpufreq_cpu_get(cpumask_first(clip_cpus));
> + if (!policy) {
> + pr_debug("%s: CPUFreq policy not found\n", __func__);
> + return ERR_PTR(-EPROBE_DEFER);
> + }
> +
> + table = policy->freq_table;
> if (!table) {
> pr_debug("%s: CPUFreq table not found\n", __func__);
> - return ERR_PTR(-EPROBE_DEFER);
> + cool_dev = ERR_PTR(-ENODEV);
> + goto put_policy;
> }
>
> cpufreq_dev = kzalloc(sizeof(*cpufreq_dev), GFP_KERNEL);
> - if (!cpufreq_dev)
> - return ERR_PTR(-ENOMEM);
> + if (!cpufreq_dev) {
> + cool_dev = ERR_PTR(-ENOMEM);
> + goto put_policy;
> + }
>
> num_cpus = cpumask_weight(clip_cpus);
> cpufreq_dev->time_in_idle = kcalloc(num_cpus,
> @@ -892,7 +902,7 @@ __cpufreq_cooling_register(struct device_node *np,
> CPUFREQ_POLICY_NOTIFIER);
> mutex_unlock(&cooling_cpufreq_lock);
>
> - return cool_dev;
> + goto put_policy;
>
> remove_idr:
> release_idr(&cpufreq_idr, cpufreq_dev->id);
> @@ -906,6 +916,8 @@ __cpufreq_cooling_register(struct device_node *np,
> kfree(cpufreq_dev->time_in_idle);
> free_cdev:
> kfree(cpufreq_dev);
> +put_policy:
> + cpufreq_cpu_put(policy);
>
> return cool_dev;
> }
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 7ed93c310c08..1342cbc0f25e 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -632,8 +632,6 @@ static inline bool policy_has_boost_freq(struct cpufreq_policy *policy)
> return false;
> }
> #endif
> -/* the following funtion is for cpufreq core use only */
> -struct cpufreq_frequency_table *cpufreq_frequency_get_table(unsigned int cpu);
>
> /* the following are really really optional */
> extern struct freq_attr cpufreq_freq_attr_scaling_available_freqs;
> --
> 2.7.1.410.g6faf27b
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH V2 2/6] cpufreq: Remove cpufreq_frequency_get_table()
2016-06-02 14:59 ` Javi Merino
@ 2016-06-02 15:36 ` Viresh Kumar
2016-06-02 18:02 ` Javi Merino
0 siblings, 1 reply; 11+ messages in thread
From: Viresh Kumar @ 2016-06-02 15:36 UTC (permalink / raw)
To: Javi Merino
Cc: Rafael Wysocki, Amit Daniel Kachhap, Zhang Rui, Eduardo Valentin,
Linaro Kernel Mailman List, linux-pm, Linux Kernel Mailing List
On 2 June 2016 at 20:29, Javi Merino <javi.merino@arm.com> wrote:
> In 5a31d594a973 ("cpufreq: Allow freq_table to be obtained for offline
> CPUs") you did the opposite: don't use cpufreq_cpu_get_raw() because
> it won't give you the policy of a cpu that is offline. Now you are
> arguing that we should go back to cpufreq_cpu_get() which implicitly
> calls cpufreq_cpu_get_raw(). Won't we hit the same issue that
> 5a31d594a973 was trying to prevent: that we can't get a freq_table for
> a cpu that is offline?
Yes, that should be fixed. Thanks for letting me know about it :)
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH V2 2/6] cpufreq: Remove cpufreq_frequency_get_table()
2016-06-02 15:36 ` Viresh Kumar
@ 2016-06-02 18:02 ` Javi Merino
2016-06-03 5:11 ` Viresh Kumar
0 siblings, 1 reply; 11+ messages in thread
From: Javi Merino @ 2016-06-02 18:02 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Amit Daniel Kachhap, Zhang Rui, Eduardo Valentin,
Linaro Kernel Mailman List, linux-pm, Linux Kernel Mailing List
On Thu, Jun 02, 2016 at 09:06:26PM +0530, Viresh Kumar wrote:
> On 2 June 2016 at 20:29, Javi Merino <javi.merino@arm.com> wrote:
> > In 5a31d594a973 ("cpufreq: Allow freq_table to be obtained for offline
> > CPUs") you did the opposite: don't use cpufreq_cpu_get_raw() because
> > it won't give you the policy of a cpu that is offline. Now you are
> > arguing that we should go back to cpufreq_cpu_get() which implicitly
> > calls cpufreq_cpu_get_raw(). Won't we hit the same issue that
> > 5a31d594a973 was trying to prevent: that we can't get a freq_table for
> > a cpu that is offline?
>
> Yes, that should be fixed. Thanks for letting me know about it :)
Ok, that was my only nit. Other than that, it looks good to me. For cpu_cooling.c
Acked-by: Javi Merino <javi.merino@arm.com>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH V2 2/6] cpufreq: Remove cpufreq_frequency_get_table()
2016-06-02 18:02 ` Javi Merino
@ 2016-06-03 5:11 ` Viresh Kumar
0 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2016-06-03 5:11 UTC (permalink / raw)
To: Javi Merino
Cc: Rafael Wysocki, Amit Daniel Kachhap, Zhang Rui, Eduardo Valentin,
Linaro Kernel Mailman List, linux-pm, Linux Kernel Mailing List
On 02-06-16, 19:02, Javi Merino wrote:
> On Thu, Jun 02, 2016 at 09:06:26PM +0530, Viresh Kumar wrote:
> > On 2 June 2016 at 20:29, Javi Merino <javi.merino@arm.com> wrote:
> > > In 5a31d594a973 ("cpufreq: Allow freq_table to be obtained for offline
> > > CPUs") you did the opposite: don't use cpufreq_cpu_get_raw() because
> > > it won't give you the policy of a cpu that is offline. Now you are
> > > arguing that we should go back to cpufreq_cpu_get() which implicitly
> > > calls cpufreq_cpu_get_raw(). Won't we hit the same issue that
> > > 5a31d594a973 was trying to prevent: that we can't get a freq_table for
> > > a cpu that is offline?
> >
> > Yes, that should be fixed. Thanks for letting me know about it :)
>
> Ok, that was my only nit. Other than that, it looks good to me. For cpu_cooling.c
>
> Acked-by: Javi Merino <javi.merino@arm.com>
Thanks, I will be adding this to the original patch.
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 63f760869651..4d678cfc81b1 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -792,10 +792,12 @@ __cpufreq_cooling_register(struct device_node *np,
struct cpufreq_cooling_device *cpufreq_dev;
char dev_name[THERMAL_NAME_LENGTH];
struct cpufreq_frequency_table *pos, *table;
+ struct cpumask temp_mask;
unsigned int freq, i, num_cpus;
int ret;
- policy = cpufreq_cpu_get(cpumask_first(clip_cpus));
+ cpumask_and(&temp_mask, clip_cpus, cpu_online_mask);
+ policy = cpufreq_cpu_get(cpumask_first(&temp_mask));
if (!policy) {
pr_debug("%s: CPUFreq policy not found\n", __func__);
return ERR_PTR(-EPROBE_DEFER);
This will also make this problem clear, otherwise it was just hidden in the
function call which was really easy to miss, as I missed it as well :(
--
viresh
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V2 3/6] cpufreq: ondemand: Don't keep a copy of freq_table pointer
2016-06-02 14:04 [PATCH V2 0/6] cpufreq: cleanups and reorganization Viresh Kumar
2016-06-02 14:04 ` [PATCH V2 1/6] cpufreq: s3c24xx: Remove useless checks Viresh Kumar
2016-06-02 14:04 ` [PATCH V2 2/6] cpufreq: Remove cpufreq_frequency_get_table() Viresh Kumar
@ 2016-06-02 14:04 ` Viresh Kumar
2016-06-02 14:04 ` [PATCH V2 4/6] cpufreq: Drop freq-table param to cpufreq_frequency_table_target() Viresh Kumar
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2016-06-02 14:04 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar; +Cc: linaro-kernel, linux-pm, linux-kernel
There is absolutely no need to keep a copy to the freq-table in 'struct
od_policy_dbs_info'. Use policy->freq_table instead.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/amd_freq_sensitivity.c | 7 +++----
drivers/cpufreq/cpufreq_ondemand.c | 22 +++++++++++-----------
drivers/cpufreq/cpufreq_ondemand.h | 1 -
3 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/cpufreq/amd_freq_sensitivity.c b/drivers/cpufreq/amd_freq_sensitivity.c
index 404360cad25c..bc86816693a8 100644
--- a/drivers/cpufreq/amd_freq_sensitivity.c
+++ b/drivers/cpufreq/amd_freq_sensitivity.c
@@ -48,9 +48,8 @@ static unsigned int amd_powersave_bias_target(struct cpufreq_policy *policy,
struct policy_dbs_info *policy_dbs = policy->governor_data;
struct dbs_data *od_data = policy_dbs->dbs_data;
struct od_dbs_tuners *od_tuners = od_data->tuners;
- struct od_policy_dbs_info *od_info = to_dbs_info(policy_dbs);
- if (!od_info->freq_table)
+ if (!policy->freq_table)
return freq_next;
rdmsr_on_cpu(policy->cpu, MSR_AMD64_FREQ_SENSITIVITY_ACTUAL,
@@ -93,9 +92,9 @@ static unsigned int amd_powersave_bias_target(struct cpufreq_policy *policy,
unsigned int index;
cpufreq_frequency_table_target(policy,
- od_info->freq_table, policy->cur - 1,
+ policy->freq_table, policy->cur - 1,
CPUFREQ_RELATION_H, &index);
- freq_next = od_info->freq_table[index].frequency;
+ freq_next = policy->freq_table[index].frequency;
}
data->freq_prev = freq_next;
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 4d2fe2710c5d..528353f204fd 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -71,28 +71,29 @@ static unsigned int generic_powersave_bias_target(struct cpufreq_policy *policy,
struct od_policy_dbs_info *dbs_info = to_dbs_info(policy_dbs);
struct dbs_data *dbs_data = policy_dbs->dbs_data;
struct od_dbs_tuners *od_tuners = dbs_data->tuners;
+ struct cpufreq_frequency_table *freq_table = policy->freq_table;
- if (!dbs_info->freq_table) {
+ if (!freq_table) {
dbs_info->freq_lo = 0;
dbs_info->freq_lo_delay_us = 0;
return freq_next;
}
- cpufreq_frequency_table_target(policy, dbs_info->freq_table, freq_next,
- relation, &index);
- freq_req = dbs_info->freq_table[index].frequency;
+ cpufreq_frequency_table_target(policy, freq_table, freq_next, relation,
+ &index);
+ freq_req = freq_table[index].frequency;
freq_reduc = freq_req * od_tuners->powersave_bias / 1000;
freq_avg = freq_req - freq_reduc;
/* Find freq bounds for freq_avg in freq_table */
index = 0;
- cpufreq_frequency_table_target(policy, dbs_info->freq_table, freq_avg,
- CPUFREQ_RELATION_H, &index);
- freq_lo = dbs_info->freq_table[index].frequency;
+ cpufreq_frequency_table_target(policy, freq_table, freq_avg,
+ CPUFREQ_RELATION_H, &index);
+ freq_lo = freq_table[index].frequency;
index = 0;
- cpufreq_frequency_table_target(policy, dbs_info->freq_table, freq_avg,
- CPUFREQ_RELATION_L, &index);
- freq_hi = dbs_info->freq_table[index].frequency;
+ cpufreq_frequency_table_target(policy, freq_table, freq_avg,
+ CPUFREQ_RELATION_L, &index);
+ freq_hi = freq_table[index].frequency;
/* Find out how long we have to be in hi and lo freqs */
if (freq_hi == freq_lo) {
@@ -113,7 +114,6 @@ static void ondemand_powersave_bias_init(struct cpufreq_policy *policy)
{
struct od_policy_dbs_info *dbs_info = to_dbs_info(policy->governor_data);
- dbs_info->freq_table = policy->freq_table;
dbs_info->freq_lo = 0;
}
diff --git a/drivers/cpufreq/cpufreq_ondemand.h b/drivers/cpufreq/cpufreq_ondemand.h
index f0121db3cd9e..640ea4e97106 100644
--- a/drivers/cpufreq/cpufreq_ondemand.h
+++ b/drivers/cpufreq/cpufreq_ondemand.h
@@ -13,7 +13,6 @@
struct od_policy_dbs_info {
struct policy_dbs_info policy_dbs;
- struct cpufreq_frequency_table *freq_table;
unsigned int freq_lo;
unsigned int freq_lo_delay_us;
unsigned int freq_hi_delay_us;
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V2 4/6] cpufreq: Drop freq-table param to cpufreq_frequency_table_target()
2016-06-02 14:04 [PATCH V2 0/6] cpufreq: cleanups and reorganization Viresh Kumar
` (2 preceding siblings ...)
2016-06-02 14:04 ` [PATCH V2 3/6] cpufreq: ondemand: Don't keep a copy of freq_table pointer Viresh Kumar
@ 2016-06-02 14:04 ` Viresh Kumar
2016-06-02 14:04 ` [PATCH V2 5/6] cpufreq: Drop 'freq_table' argument of __target_index() Viresh Kumar
2016-06-02 14:05 ` [PATCH V2 6/6] cpufreq: Return index from cpufreq_frequency_table_target() Viresh Kumar
5 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2016-06-02 14:04 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman
Cc: linaro-kernel, linux-pm, linux-kernel, linux-doc, linuxppc-dev
The policy already has this pointer set, use it instead.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Documentation/cpu-freq/cpu-drivers.txt | 1 -
drivers/cpufreq/amd_freq_sensitivity.c | 3 +--
drivers/cpufreq/cpufreq.c | 4 ++--
drivers/cpufreq/cpufreq_ondemand.c | 11 +++++------
drivers/cpufreq/freq_table.c | 2 +-
drivers/cpufreq/powernv-cpufreq.c | 3 +--
drivers/cpufreq/s3c24xx-cpufreq.c | 11 +++++------
drivers/cpufreq/s5pv210-cpufreq.c | 3 +--
include/linux/cpufreq.h | 1 -
9 files changed, 16 insertions(+), 23 deletions(-)
diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
index decbb8cb5573..74e812f0719c 100644
--- a/Documentation/cpu-freq/cpu-drivers.txt
+++ b/Documentation/cpu-freq/cpu-drivers.txt
@@ -244,7 +244,6 @@ policy->max, and all other criteria are met. This is helpful for the
->verify call.
int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
- struct cpufreq_frequency_table *table,
unsigned int target_freq,
unsigned int relation,
unsigned int *index);
diff --git a/drivers/cpufreq/amd_freq_sensitivity.c b/drivers/cpufreq/amd_freq_sensitivity.c
index bc86816693a8..3bea1bb791a9 100644
--- a/drivers/cpufreq/amd_freq_sensitivity.c
+++ b/drivers/cpufreq/amd_freq_sensitivity.c
@@ -92,8 +92,7 @@ static unsigned int amd_powersave_bias_target(struct cpufreq_policy *policy,
unsigned int index;
cpufreq_frequency_table_target(policy,
- policy->freq_table, policy->cur - 1,
- CPUFREQ_RELATION_H, &index);
+ policy->cur - 1, CPUFREQ_RELATION_H, &index);
freq_next = policy->freq_table[index].frequency;
}
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 1833eda1f9d4..e68611a67bd9 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1947,8 +1947,8 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
return -EINVAL;
}
- retval = cpufreq_frequency_table_target(policy, freq_table, target_freq,
- relation, &index);
+ retval = cpufreq_frequency_table_target(policy, target_freq, relation,
+ &index);
if (unlikely(retval)) {
pr_err("%s: Unable to find matching freq\n", __func__);
return retval;
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 528353f204fd..2ee476f5a2bd 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -79,20 +79,19 @@ static unsigned int generic_powersave_bias_target(struct cpufreq_policy *policy,
return freq_next;
}
- cpufreq_frequency_table_target(policy, freq_table, freq_next, relation,
- &index);
+ cpufreq_frequency_table_target(policy, freq_next, relation, &index);
freq_req = freq_table[index].frequency;
freq_reduc = freq_req * od_tuners->powersave_bias / 1000;
freq_avg = freq_req - freq_reduc;
/* Find freq bounds for freq_avg in freq_table */
index = 0;
- cpufreq_frequency_table_target(policy, freq_table, freq_avg,
- CPUFREQ_RELATION_H, &index);
+ cpufreq_frequency_table_target(policy, freq_avg, CPUFREQ_RELATION_H,
+ &index);
freq_lo = freq_table[index].frequency;
index = 0;
- cpufreq_frequency_table_target(policy, freq_table, freq_avg,
- CPUFREQ_RELATION_L, &index);
+ cpufreq_frequency_table_target(policy, freq_avg, CPUFREQ_RELATION_L,
+ &index);
freq_hi = freq_table[index].frequency;
/* Find out how long we have to be in hi and lo freqs */
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index f52b5473b1f4..f145b64649ef 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -114,7 +114,6 @@ int cpufreq_generic_frequency_table_verify(struct cpufreq_policy *policy)
EXPORT_SYMBOL_GPL(cpufreq_generic_frequency_table_verify);
int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
- struct cpufreq_frequency_table *table,
unsigned int target_freq,
unsigned int relation,
unsigned int *index)
@@ -128,6 +127,7 @@ int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
.frequency = 0,
};
struct cpufreq_frequency_table *pos;
+ struct cpufreq_frequency_table *table = policy->freq_table;
unsigned int freq, diff, i = 0;
pr_debug("request for target %u kHz (relation: %u) for cpu %u\n",
diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
index 54c45368e3f1..bf267c2dfe20 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -760,8 +760,7 @@ void powernv_cpufreq_work_fn(struct work_struct *work)
struct cpufreq_policy policy;
cpufreq_get_policy(&policy, cpu);
- cpufreq_frequency_table_target(&policy, policy.freq_table,
- policy.cur,
+ cpufreq_frequency_table_target(&policy, policy.cur,
CPUFREQ_RELATION_C, &index);
powernv_cpufreq_target_index(&policy, index);
cpumask_andnot(&mask, &mask, policy.cpus);
diff --git a/drivers/cpufreq/s3c24xx-cpufreq.c b/drivers/cpufreq/s3c24xx-cpufreq.c
index 4567c3cab095..05a9737278f3 100644
--- a/drivers/cpufreq/s3c24xx-cpufreq.c
+++ b/drivers/cpufreq/s3c24xx-cpufreq.c
@@ -293,9 +293,8 @@ static int s3c_cpufreq_target(struct cpufreq_policy *policy,
__func__, policy, target_freq, relation);
if (ftab) {
- if (cpufreq_frequency_table_target(policy, ftab,
- target_freq, relation,
- &index)) {
+ if (cpufreq_frequency_table_target(policy, target_freq,
+ relation, &index)) {
s3c_freq_dbg("%s: table failed\n", __func__);
return -EINVAL;
}
@@ -323,14 +322,14 @@ static int s3c_cpufreq_target(struct cpufreq_policy *policy,
tmp_policy.min = policy->min * 1000;
tmp_policy.max = policy->max * 1000;
tmp_policy.cpu = policy->cpu;
+ tmp_policy.freq_table = pll_reg;
/* cpufreq_frequency_table_target uses a pointer to 'index'
* which is the number of the table entry, not the value of
* the table entry's index field. */
- ret = cpufreq_frequency_table_target(&tmp_policy, pll_reg,
- target_freq, relation,
- &index);
+ ret = cpufreq_frequency_table_target(&tmp_policy, target_freq,
+ relation, &index);
if (ret < 0) {
pr_err("%s: no PLL available\n", __func__);
diff --git a/drivers/cpufreq/s5pv210-cpufreq.c b/drivers/cpufreq/s5pv210-cpufreq.c
index 06d85917b6d5..6b0cfc3b8c46 100644
--- a/drivers/cpufreq/s5pv210-cpufreq.c
+++ b/drivers/cpufreq/s5pv210-cpufreq.c
@@ -246,8 +246,7 @@ static int s5pv210_target(struct cpufreq_policy *policy, unsigned int index)
new_freq = s5pv210_freq_table[index].frequency;
/* Finding current running level index */
- if (cpufreq_frequency_table_target(policy, s5pv210_freq_table,
- old_freq, CPUFREQ_RELATION_H,
+ if (cpufreq_frequency_table_target(policy, old_freq, CPUFREQ_RELATION_H,
&priv_index)) {
ret = -EINVAL;
goto exit;
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 1342cbc0f25e..bdd7f0c035ae 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -598,7 +598,6 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy *policy,
int cpufreq_generic_frequency_table_verify(struct cpufreq_policy *policy);
int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
- struct cpufreq_frequency_table *table,
unsigned int target_freq,
unsigned int relation,
unsigned int *index);
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V2 5/6] cpufreq: Drop 'freq_table' argument of __target_index()
2016-06-02 14:04 [PATCH V2 0/6] cpufreq: cleanups and reorganization Viresh Kumar
` (3 preceding siblings ...)
2016-06-02 14:04 ` [PATCH V2 4/6] cpufreq: Drop freq-table param to cpufreq_frequency_table_target() Viresh Kumar
@ 2016-06-02 14:04 ` Viresh Kumar
2016-06-02 14:05 ` [PATCH V2 6/6] cpufreq: Return index from cpufreq_frequency_table_target() Viresh Kumar
5 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2016-06-02 14:04 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar; +Cc: linaro-kernel, linux-pm, linux-kernel
It is already present as part of the policy and so no need to pass it
from the caller. Also, 'freq_table' is guaranteed to be valid in this
function and so no need to check it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index e68611a67bd9..30f05dd5c872 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1852,14 +1852,17 @@ static int __target_intermediate(struct cpufreq_policy *policy,
return ret;
}
-static int __target_index(struct cpufreq_policy *policy,
- struct cpufreq_frequency_table *freq_table, int index)
+static int __target_index(struct cpufreq_policy *policy, int index)
{
struct cpufreq_freqs freqs = {.old = policy->cur, .flags = 0};
unsigned int intermediate_freq = 0;
+ unsigned int newfreq = policy->freq_table[index].frequency;
int retval = -EINVAL;
bool notify;
+ if (newfreq == policy->cur)
+ return 0;
+
notify = !(cpufreq_driver->flags & CPUFREQ_ASYNC_NOTIFICATION);
if (notify) {
/* Handle switching to intermediate frequency */
@@ -1874,7 +1877,7 @@ static int __target_index(struct cpufreq_policy *policy,
freqs.old = freqs.new;
}
- freqs.new = freq_table[index].frequency;
+ freqs.new = newfreq;
pr_debug("%s: cpu: %d, oldfreq: %u, new freq: %u\n",
__func__, policy->cpu, freqs.old, freqs.new);
@@ -1911,7 +1914,6 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
unsigned int relation)
{
unsigned int old_target_freq = target_freq;
- struct cpufreq_frequency_table *freq_table;
int index, retval;
if (cpufreq_disabled())
@@ -1941,12 +1943,6 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
if (!cpufreq_driver->target_index)
return -EINVAL;
- freq_table = policy->freq_table;
- if (unlikely(!freq_table)) {
- pr_err("%s: Unable to find freq_table\n", __func__);
- return -EINVAL;
- }
-
retval = cpufreq_frequency_table_target(policy, target_freq, relation,
&index);
if (unlikely(retval)) {
@@ -1954,10 +1950,7 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
return retval;
}
- if (freq_table[index].frequency == policy->cur)
- return 0;
-
- return __target_index(policy, freq_table, index);
+ return __target_index(policy, index);
}
EXPORT_SYMBOL_GPL(__cpufreq_driver_target);
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V2 6/6] cpufreq: Return index from cpufreq_frequency_table_target()
2016-06-02 14:04 [PATCH V2 0/6] cpufreq: cleanups and reorganization Viresh Kumar
` (4 preceding siblings ...)
2016-06-02 14:04 ` [PATCH V2 5/6] cpufreq: Drop 'freq_table' argument of __target_index() Viresh Kumar
@ 2016-06-02 14:05 ` Viresh Kumar
5 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2016-06-02 14:05 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman
Cc: linaro-kernel, linux-pm, linux-kernel, linux-doc, linuxppc-dev
This routine can't fail unless the frequency table is invalid and
doesn't contain any valid entries.
Make it return the index and WARN() in case it is used for an invalid
table.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Documentation/cpu-freq/cpu-drivers.txt | 7 +++----
drivers/cpufreq/amd_freq_sensitivity.c | 4 ++--
drivers/cpufreq/cpufreq.c | 9 ++-------
drivers/cpufreq/cpufreq_ondemand.c | 14 ++++++--------
drivers/cpufreq/freq_table.c | 24 +++++++++++++-----------
drivers/cpufreq/powernv-cpufreq.c | 4 ++--
drivers/cpufreq/s3c24xx-cpufreq.c | 26 ++++++--------------------
drivers/cpufreq/s5pv210-cpufreq.c | 7 ++-----
include/linux/cpufreq.h | 3 +--
9 files changed, 37 insertions(+), 61 deletions(-)
diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
index 74e812f0719c..772b94fde264 100644
--- a/Documentation/cpu-freq/cpu-drivers.txt
+++ b/Documentation/cpu-freq/cpu-drivers.txt
@@ -245,12 +245,11 @@ policy->max, and all other criteria are met. This is helpful for the
int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
unsigned int target_freq,
- unsigned int relation,
- unsigned int *index);
+ unsigned int relation);
is the corresponding frequency table helper for the ->target
-stage. Just pass the values to this function, and the unsigned int
-index returns the number of the frequency table entry which contains
+stage. Just pass the values to this function, and this function
+returns the number of the frequency table entry which contains
the frequency the CPU shall be set to.
The following macros can be used as iterators over cpufreq_frequency_table:
diff --git a/drivers/cpufreq/amd_freq_sensitivity.c b/drivers/cpufreq/amd_freq_sensitivity.c
index 3bea1bb791a9..6d5dc04c3a37 100644
--- a/drivers/cpufreq/amd_freq_sensitivity.c
+++ b/drivers/cpufreq/amd_freq_sensitivity.c
@@ -91,8 +91,8 @@ static unsigned int amd_powersave_bias_target(struct cpufreq_policy *policy,
else {
unsigned int index;
- cpufreq_frequency_table_target(policy,
- policy->cur - 1, CPUFREQ_RELATION_H, &index);
+ index = cpufreq_frequency_table_target(policy,
+ policy->cur - 1, CPUFREQ_RELATION_H);
freq_next = policy->freq_table[index].frequency;
}
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 30f05dd5c872..9ae58a18ccb9 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1914,7 +1914,7 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
unsigned int relation)
{
unsigned int old_target_freq = target_freq;
- int index, retval;
+ int index;
if (cpufreq_disabled())
return -ENODEV;
@@ -1943,12 +1943,7 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
if (!cpufreq_driver->target_index)
return -EINVAL;
- retval = cpufreq_frequency_table_target(policy, target_freq, relation,
- &index);
- if (unlikely(retval)) {
- pr_err("%s: Unable to find matching freq\n", __func__);
- return retval;
- }
+ index = cpufreq_frequency_table_target(policy, target_freq, relation);
return __target_index(policy, index);
}
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 2ee476f5a2bd..0c93cd9dee99 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -65,7 +65,7 @@ static unsigned int generic_powersave_bias_target(struct cpufreq_policy *policy,
{
unsigned int freq_req, freq_reduc, freq_avg;
unsigned int freq_hi, freq_lo;
- unsigned int index = 0;
+ unsigned int index;
unsigned int delay_hi_us;
struct policy_dbs_info *policy_dbs = policy->governor_data;
struct od_policy_dbs_info *dbs_info = to_dbs_info(policy_dbs);
@@ -79,19 +79,17 @@ static unsigned int generic_powersave_bias_target(struct cpufreq_policy *policy,
return freq_next;
}
- cpufreq_frequency_table_target(policy, freq_next, relation, &index);
+ index = cpufreq_frequency_table_target(policy, freq_next, relation);
freq_req = freq_table[index].frequency;
freq_reduc = freq_req * od_tuners->powersave_bias / 1000;
freq_avg = freq_req - freq_reduc;
/* Find freq bounds for freq_avg in freq_table */
- index = 0;
- cpufreq_frequency_table_target(policy, freq_avg, CPUFREQ_RELATION_H,
- &index);
+ index = cpufreq_frequency_table_target(policy, freq_avg,
+ CPUFREQ_RELATION_H);
freq_lo = freq_table[index].frequency;
- index = 0;
- cpufreq_frequency_table_target(policy, freq_avg, CPUFREQ_RELATION_L,
- &index);
+ index = cpufreq_frequency_table_target(policy, freq_avg,
+ CPUFREQ_RELATION_L);
freq_hi = freq_table[index].frequency;
/* Find out how long we have to be in hi and lo freqs */
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index f145b64649ef..eac8bcbdaad1 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -114,9 +114,8 @@ int cpufreq_generic_frequency_table_verify(struct cpufreq_policy *policy)
EXPORT_SYMBOL_GPL(cpufreq_generic_frequency_table_verify);
int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
- unsigned int target_freq,
- unsigned int relation,
- unsigned int *index)
+ unsigned int target_freq,
+ unsigned int relation)
{
struct cpufreq_frequency_table optimal = {
.driver_data = ~0,
@@ -129,6 +128,7 @@ int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
struct cpufreq_frequency_table *pos;
struct cpufreq_frequency_table *table = policy->freq_table;
unsigned int freq, diff, i = 0;
+ int index;
pr_debug("request for target %u kHz (relation: %u) for cpu %u\n",
target_freq, relation, policy->cpu);
@@ -192,16 +192,18 @@ int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
}
}
if (optimal.driver_data > i) {
- if (suboptimal.driver_data > i)
- return -EINVAL;
- *index = suboptimal.driver_data;
- } else
- *index = optimal.driver_data;
+ if (suboptimal.driver_data > i) {
+ WARN(1, "Invalid frequency table: %d\n", policy->cpu);
+ return 0;
+ }
- pr_debug("target index is %u, freq is:%u kHz\n", *index,
- table[*index].frequency);
+ index = suboptimal.driver_data;
+ } else
+ index = optimal.driver_data;
- return 0;
+ pr_debug("target index is %u, freq is:%u kHz\n", index,
+ table[index].frequency);
+ return index;
}
EXPORT_SYMBOL_GPL(cpufreq_frequency_table_target);
diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
index bf267c2dfe20..b29c5c20c3a1 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -760,8 +760,8 @@ void powernv_cpufreq_work_fn(struct work_struct *work)
struct cpufreq_policy policy;
cpufreq_get_policy(&policy, cpu);
- cpufreq_frequency_table_target(&policy, policy.cur,
- CPUFREQ_RELATION_C, &index);
+ index = cpufreq_frequency_table_target(&policy, policy.cur,
+ CPUFREQ_RELATION_C);
powernv_cpufreq_target_index(&policy, index);
cpumask_andnot(&mask, &mask, policy.cpus);
}
diff --git a/drivers/cpufreq/s3c24xx-cpufreq.c b/drivers/cpufreq/s3c24xx-cpufreq.c
index 05a9737278f3..7b596fa38ad2 100644
--- a/drivers/cpufreq/s3c24xx-cpufreq.c
+++ b/drivers/cpufreq/s3c24xx-cpufreq.c
@@ -293,11 +293,8 @@ static int s3c_cpufreq_target(struct cpufreq_policy *policy,
__func__, policy, target_freq, relation);
if (ftab) {
- if (cpufreq_frequency_table_target(policy, target_freq,
- relation, &index)) {
- s3c_freq_dbg("%s: table failed\n", __func__);
- return -EINVAL;
- }
+ index = cpufreq_frequency_table_target(policy, target_freq,
+ relation);
s3c_freq_dbg("%s: adjust %d to entry %d (%u)\n", __func__,
target_freq, index, ftab[index].frequency);
@@ -314,7 +311,6 @@ static int s3c_cpufreq_target(struct cpufreq_policy *policy,
pll = NULL;
} else {
struct cpufreq_policy tmp_policy;
- int ret;
/* we keep the cpu pll table in Hz, to ensure we get an
* accurate value for the PLL output. */
@@ -324,18 +320,12 @@ static int s3c_cpufreq_target(struct cpufreq_policy *policy,
tmp_policy.cpu = policy->cpu;
tmp_policy.freq_table = pll_reg;
- /* cpufreq_frequency_table_target uses a pointer to 'index'
- * which is the number of the table entry, not the value of
+ /* cpufreq_frequency_table_target returns the index
+ * of the table entry, not the value of
* the table entry's index field. */
- ret = cpufreq_frequency_table_target(&tmp_policy, target_freq,
- relation, &index);
-
- if (ret < 0) {
- pr_err("%s: no PLL available\n", __func__);
- goto err_notpossible;
- }
-
+ index = cpufreq_frequency_table_target(&tmp_policy, target_freq,
+ relation);
pll = pll_reg + index;
s3c_freq_dbg("%s: target %u => %u\n",
@@ -345,10 +335,6 @@ static int s3c_cpufreq_target(struct cpufreq_policy *policy,
}
return s3c_cpufreq_settarget(policy, target_freq, pll);
-
- err_notpossible:
- pr_err("no compatible settings for %d\n", target_freq);
- return -EINVAL;
}
struct clk *s3c_cpufreq_clk_get(struct device *dev, const char *name)
diff --git a/drivers/cpufreq/s5pv210-cpufreq.c b/drivers/cpufreq/s5pv210-cpufreq.c
index 6b0cfc3b8c46..4f4e9df9b7fc 100644
--- a/drivers/cpufreq/s5pv210-cpufreq.c
+++ b/drivers/cpufreq/s5pv210-cpufreq.c
@@ -246,11 +246,8 @@ static int s5pv210_target(struct cpufreq_policy *policy, unsigned int index)
new_freq = s5pv210_freq_table[index].frequency;
/* Finding current running level index */
- if (cpufreq_frequency_table_target(policy, old_freq, CPUFREQ_RELATION_H,
- &priv_index)) {
- ret = -EINVAL;
- goto exit;
- }
+ priv_index = cpufreq_frequency_table_target(policy, old_freq,
+ CPUFREQ_RELATION_H);
arm_volt = dvs_conf[index].arm_volt;
int_volt = dvs_conf[index].int_volt;
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index bdd7f0c035ae..c378776628b4 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -599,8 +599,7 @@ int cpufreq_generic_frequency_table_verify(struct cpufreq_policy *policy);
int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
unsigned int target_freq,
- unsigned int relation,
- unsigned int *index);
+ unsigned int relation);
int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy,
unsigned int freq);
--
2.7.1.410.g6faf27b
^ permalink raw reply related [flat|nested] 11+ messages in thread