Hi Rafael et al
Along with the big changes I have for cpufreq core simplification, I also accumulated these trivial cleanups. In order to not confuse the much dedicated series on solving the real problems, I am sending these separately.
These are independent of the stats cleanups sent earlier and so it doesn't matter which one you apply first.
Pushed here: git://git.linaro.org/people/viresh.kumar/linux.git cpufreq/core/trivial-cleanups
@Prarit/Saravana: It would be helpful if you guys can go through this as well :)
Viresh Kumar (17): cpufreq: remove dangling comment cpufreq: remove extra parenthesis cpufreq: don't need line break in show_scaling_cur_freq() cpufreq: merge 'if' blocks in __cpufreq_remove_dev_prepare() cpufreq: s/__find_governor/find_governor cpufreq: No need to check for has_target() cpufreq: pass policy to cpufreq_out_of_sync cpufreq: pass policy to __cpufreq_get() cpufreq: update driver->flags only if we are registering driver cpufreq: get rid of CONFIG_{HOTPLUG_CPU|SMP} mess cpufreq: get rid of 'tpolicy' from __cpufreq_add_dev() cpufreq: use light-weight cpufreq_cpu_get_raw() in __cpufreq_add_dev cpufreq: limit the scope of l_p_j variables cpufreq: check cpufreq_policy_list instead of getting policies for all CPUs cpufreq: don't check if cpu > nr_cpu_ids cpufreq: remove check for cpufreq_disabled() from cpufreq_cpu_{get|put}() cpufreq: move some initialization stuff to cpufreq_policy_alloc()
drivers/cpufreq/cpufreq.c | 129 ++++++++++++++-------------------------------- 1 file changed, 40 insertions(+), 89 deletions(-)
It doesn't make any sense at all and is a leftover of some earlier commit. Remove it.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 6d97dffeaaf7..891672b913c8 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2000,10 +2000,6 @@ int cpufreq_driver_target(struct cpufreq_policy *policy, } EXPORT_SYMBOL_GPL(cpufreq_driver_target);
-/* - * when "event" is CPUFREQ_GOV_LIMITS - */ - static int __cpufreq_governor(struct cpufreq_policy *policy, unsigned int event) {
We can live without it and so we should.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 891672b913c8..8417313943d5 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -902,7 +902,7 @@ static int cpufreq_add_dev_interface(struct cpufreq_policy *policy,
/* set up files for this cpu device */ drv_attr = cpufreq_driver->attr; - while ((drv_attr) && (*drv_attr)) { + while (drv_attr && *drv_attr) { ret = sysfs_create_file(&policy->kobj, &((*drv_attr)->attr)); if (ret) return ret;
No need of an unnecessary line break.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 8417313943d5..4271d68aec41 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -513,8 +513,7 @@ show_one(cpuinfo_transition_latency, cpuinfo.transition_latency); show_one(scaling_min_freq, min); show_one(scaling_max_freq, max);
-static ssize_t show_scaling_cur_freq( - struct cpufreq_policy *policy, char *buf) +static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf) { ssize_t ret;
There are two 'if' blocks here, checking for !cpufreq_driver->setpolicy and has_target(). Both are actually doing the same thing, merge them.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 4271d68aec41..4309b075df06 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1364,11 +1364,10 @@ static int __cpufreq_remove_dev_prepare(struct device *dev, pr_err("%s: Failed to stop governor\n", __func__); return ret; } - }
- if (!cpufreq_driver->setpolicy) strncpy(per_cpu(cpufreq_cpu_governor, cpu), policy->governor->name, CPUFREQ_NAME_LEN); + }
down_read(&policy->rwsem); cpus = cpumask_weight(policy->cpus);
Remove unnecessary __ present in find_governor's name.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 4309b075df06..c9ba1d64015c 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -432,7 +432,7 @@ static ssize_t store_boost(struct kobject *kobj, struct attribute *attr, } define_one_global_rw(boost);
-static struct cpufreq_governor *__find_governor(const char *str_governor) +static struct cpufreq_governor *find_governor(const char *str_governor) { struct cpufreq_governor *t;
@@ -468,7 +468,7 @@ static int cpufreq_parse_governor(char *str_governor, unsigned int *policy,
mutex_lock(&cpufreq_governor_mutex);
- t = __find_governor(str_governor); + t = find_governor(str_governor);
if (t == NULL) { int ret; @@ -478,7 +478,7 @@ static int cpufreq_parse_governor(char *str_governor, unsigned int *policy, mutex_lock(&cpufreq_governor_mutex);
if (ret == 0) - t = __find_governor(str_governor); + t = find_governor(str_governor); }
if (t != NULL) { @@ -935,7 +935,7 @@ static void cpufreq_init_policy(struct cpufreq_policy *policy) memcpy(&new_policy, policy, sizeof(*policy));
/* Update governor of new_policy to the governor used before hotplug */ - gov = __find_governor(per_cpu(cpufreq_cpu_governor, policy->cpu)); + gov = find_governor(per_cpu(cpufreq_cpu_governor, policy->cpu)); if (gov) pr_debug("Restoring governor %s for cpu %d\n", policy->governor->name, policy->cpu); @@ -2089,7 +2089,7 @@ int cpufreq_register_governor(struct cpufreq_governor *governor)
governor->initialized = 0; err = -EBUSY; - if (__find_governor(governor->name) == NULL) { + if (!find_governor(governor->name)) { err = 0; list_add(&governor->governor_list, &cpufreq_governor_list); }
Either we can be setpolicy or target type, nothing else. And so the else part of setpolicy will automatically be of has_target() type. And so we don't need to check it again.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index c9ba1d64015c..5e60b3259cf5 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -463,7 +463,7 @@ static int cpufreq_parse_governor(char *str_governor, unsigned int *policy, *policy = CPUFREQ_POLICY_POWERSAVE; err = 0; } - } else if (has_target()) { + } else { struct cpufreq_governor *t;
mutex_lock(&cpufreq_governor_mutex);
There is no point finding out the 'policy' again within cpufreq_out_of_sync() when all the callers already have it. Just make them pass policy instead.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 5e60b3259cf5..3e80b97d64b8 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1502,30 +1502,23 @@ static void handle_update(struct work_struct *work) /** * cpufreq_out_of_sync - If actual and saved CPU frequency differs, we're * in deep trouble. - * @cpu: cpu number - * @old_freq: CPU frequency the kernel thinks the CPU runs at + * @policy: policy managing CPUs * @new_freq: CPU frequency the CPU actually runs at * * We adjust to current frequency first, and need to clean up later. * So either call to cpufreq_update_policy() or schedule handle_update()). */ -static void cpufreq_out_of_sync(unsigned int cpu, unsigned int old_freq, +static void cpufreq_out_of_sync(struct cpufreq_policy *policy, unsigned int new_freq) { - struct cpufreq_policy *policy; struct cpufreq_freqs freqs; - unsigned long flags;
pr_debug("Warning: CPU frequency out of sync: cpufreq and timing core thinks of %u, is %u kHz\n", - old_freq, new_freq); + policy->cur, new_freq);
- freqs.old = old_freq; + freqs.old = policy->cur; freqs.new = new_freq;
- read_lock_irqsave(&cpufreq_driver_lock, flags); - policy = per_cpu(cpufreq_cpu_data, cpu); - read_unlock_irqrestore(&cpufreq_driver_lock, flags); - cpufreq_freq_transition_begin(policy, &freqs); cpufreq_freq_transition_end(policy, &freqs, 0); } @@ -1590,7 +1583,7 @@ static unsigned int __cpufreq_get(unsigned int cpu) /* verify no discrepancy between actual and saved value exists */ if (unlikely(ret_freq != policy->cur)) { - cpufreq_out_of_sync(cpu, policy->cur, ret_freq); + cpufreq_out_of_sync(policy, ret_freq); schedule_work(&policy->update); } } @@ -2289,8 +2282,7 @@ int cpufreq_update_policy(unsigned int cpu) policy->cur = new_policy.cur; } else { if (policy->cur != new_policy.cur && has_target()) - cpufreq_out_of_sync(cpu, policy->cur, - new_policy.cur); + cpufreq_out_of_sync(policy, new_policy.cur); } }
There is no point finding out the 'policy' again within __cpufreq_get() when all the callers already have it. Just make them pass policy instead.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 3e80b97d64b8..35d4734ce856 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -62,7 +62,7 @@ static DECLARE_RWSEM(cpufreq_rwsem); /* internal prototypes */ static int __cpufreq_governor(struct cpufreq_policy *policy, unsigned int event); -static unsigned int __cpufreq_get(unsigned int cpu); +static unsigned int __cpufreq_get(struct cpufreq_policy *policy); static void handle_update(struct work_struct *work);
/** @@ -562,7 +562,7 @@ store_one(scaling_max_freq, max); static ssize_t show_cpuinfo_cur_freq(struct cpufreq_policy *policy, char *buf) { - unsigned int cur_freq = __cpufreq_get(policy->cpu); + unsigned int cur_freq = __cpufreq_get(policy); if (!cur_freq) return sprintf(buf, "<unknown>"); return sprintf(buf, "%u\n", cur_freq); @@ -1568,15 +1568,14 @@ unsigned int cpufreq_quick_get_max(unsigned int cpu) } EXPORT_SYMBOL(cpufreq_quick_get_max);
-static unsigned int __cpufreq_get(unsigned int cpu) +static unsigned int __cpufreq_get(struct cpufreq_policy *policy) { - struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu); unsigned int ret_freq = 0;
if (!cpufreq_driver->get) return ret_freq;
- ret_freq = cpufreq_driver->get(cpu); + ret_freq = cpufreq_driver->get(policy->cpu);
if (ret_freq && policy->cur && !(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) { @@ -1604,7 +1603,7 @@ unsigned int cpufreq_get(unsigned int cpu)
if (policy) { down_read(&policy->rwsem); - ret_freq = __cpufreq_get(cpu); + ret_freq = __cpufreq_get(policy); up_read(&policy->rwsem);
cpufreq_cpu_put(policy);
We should first check if a cpufreq driver is already registered or not before updating driver_data->flags.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 35d4734ce856..9595107477a6 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2427,9 +2427,6 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data)
pr_debug("trying to register driver %s\n", driver_data->name);
- if (driver_data->setpolicy) - driver_data->flags |= CPUFREQ_CONST_LOOPS; - write_lock_irqsave(&cpufreq_driver_lock, flags); if (cpufreq_driver) { write_unlock_irqrestore(&cpufreq_driver_lock, flags); @@ -2438,6 +2435,9 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data) cpufreq_driver = driver_data; write_unlock_irqrestore(&cpufreq_driver_lock, flags);
+ if (driver_data->setpolicy) + driver_data->flags |= CPUFREQ_CONST_LOOPS; + if (cpufreq_boost_supported()) { /* * Check if driver provides function to enable boost -
These are messing up more than the benefit they provide. It isn't a lot of code anyway, that we will compile without them.
Kill them.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 9595107477a6..3893c87739d1 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -957,7 +957,6 @@ static void cpufreq_init_policy(struct cpufreq_policy *policy) } }
-#ifdef CONFIG_HOTPLUG_CPU static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu, struct device *dev) { @@ -995,7 +994,6 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy,
return sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq"); } -#endif
static struct cpufreq_policy *cpufreq_policy_restore(unsigned int cpu) { @@ -1100,19 +1098,15 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) { unsigned int j, cpu = dev->id; int ret = -ENOMEM; - struct cpufreq_policy *policy; + struct cpufreq_policy *policy, *tpolicy; unsigned long flags; bool recover_policy = cpufreq_suspended; -#ifdef CONFIG_HOTPLUG_CPU - struct cpufreq_policy *tpolicy; -#endif
if (cpu_is_offline(cpu)) return 0;
pr_debug("adding CPU %u\n", cpu);
-#ifdef CONFIG_SMP /* check whether a different CPU already registered this * CPU because it is in the same boat. */ policy = cpufreq_cpu_get(cpu); @@ -1120,12 +1114,10 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) cpufreq_cpu_put(policy); return 0; } -#endif
if (!down_read_trylock(&cpufreq_rwsem)) return 0;
-#ifdef CONFIG_HOTPLUG_CPU /* Check if this cpu was hot-unplugged earlier and has siblings */ read_lock_irqsave(&cpufreq_driver_lock, flags); list_for_each_entry(tpolicy, &cpufreq_policy_list, policy_list) { @@ -1137,7 +1129,6 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) } } read_unlock_irqrestore(&cpufreq_driver_lock, flags); -#endif
/* * Restore the saved policy when doing light-weight init and fall back
There is no need of this separate variable, use 'policy' instead.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 3893c87739d1..3061a5823e3c 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1098,7 +1098,7 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) { unsigned int j, cpu = dev->id; int ret = -ENOMEM; - struct cpufreq_policy *policy, *tpolicy; + struct cpufreq_policy *policy; unsigned long flags; bool recover_policy = cpufreq_suspended;
@@ -1120,10 +1120,10 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
/* Check if this cpu was hot-unplugged earlier and has siblings */ read_lock_irqsave(&cpufreq_driver_lock, flags); - list_for_each_entry(tpolicy, &cpufreq_policy_list, policy_list) { - if (cpumask_test_cpu(cpu, tpolicy->related_cpus)) { + list_for_each_entry(policy, &cpufreq_policy_list, policy_list) { + if (cpumask_test_cpu(cpu, policy->related_cpus)) { read_unlock_irqrestore(&cpufreq_driver_lock, flags); - ret = cpufreq_add_policy_cpu(tpolicy, cpu, dev); + ret = cpufreq_add_policy_cpu(policy, cpu, dev); up_read(&cpufreq_rwsem); return ret; }
We just need to check if a 'policy' is already present for the cpu we are adding. We don't need to take all the locks and do kobject usage updates. Use the light-weight cpufreq_cpu_get_raw() routine instead.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 3061a5823e3c..788016cded33 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1109,11 +1109,9 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
/* check whether a different CPU already registered this * CPU because it is in the same boat. */ - policy = cpufreq_cpu_get(cpu); - if (unlikely(policy)) { - cpufreq_cpu_put(policy); + policy = cpufreq_cpu_get_raw(cpu); + if (unlikely(policy)) return 0; - }
if (!down_read_trylock(&cpufreq_rwsem)) return 0;
These variables are just used within adjust_jiffies() and so must be local to it. Also there is no need of a dummy routine for CONFIG_SMP case as we can take care of all that with help of macros in the same routine. It doesn't look that ugly.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 788016cded33..74169ad47546 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -249,12 +249,12 @@ EXPORT_SYMBOL_GPL(cpufreq_cpu_put); * systems as each CPU might be scaled differently. So, use the arch * per-CPU loops_per_jiffy value wherever possible. */ -#ifndef CONFIG_SMP -static unsigned long l_p_j_ref; -static unsigned int l_p_j_ref_freq; - static void adjust_jiffies(unsigned long val, struct cpufreq_freqs *ci) { +#ifndef CONFIG_SMP + static unsigned long l_p_j_ref; + static unsigned int l_p_j_ref_freq; + if (ci->flags & CPUFREQ_CONST_LOOPS) return;
@@ -270,13 +270,8 @@ static void adjust_jiffies(unsigned long val, struct cpufreq_freqs *ci) pr_debug("scaling loops_per_jiffy to %lu for frequency %u kHz\n", loops_per_jiffy, ci->new); } -} -#else -static inline void adjust_jiffies(unsigned long val, struct cpufreq_freqs *ci) -{ - return; -} #endif +}
static void __cpufreq_notify_transition(struct cpufreq_policy *policy, struct cpufreq_freqs *freqs, unsigned int state)
CPUFREQ_STICKY flag is set by drivers which don't want to get unregistered even if cpufreq-core isn't able to initialize policy for any CPU.
When this flag isn't set, we try to unregister the driver. To find out which CPUs are registered and which are not, we try to check per_cpu cpufreq_cpu_data for all CPUs. Because we have a list of valid policies available now, we better check if the list is empty or not instead of the 'for' loop. That will be much more efficient.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 74169ad47546..09426671eb79 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2442,23 +2442,12 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data) if (ret) goto err_boost_unreg;
- if (!(cpufreq_driver->flags & CPUFREQ_STICKY)) { - int i; - ret = -ENODEV; - - /* check for at least one working CPU */ - for (i = 0; i < nr_cpu_ids; i++) - if (cpu_possible(i) && per_cpu(cpufreq_cpu_data, i)) { - ret = 0; - break; - } - + if (!(cpufreq_driver->flags & CPUFREQ_STICKY) && + list_empty(&cpufreq_policy_list)) { /* if all ->init() calls failed, unregister */ - if (ret) { - pr_debug("no CPU initialized for driver %s\n", - driver_data->name); - goto err_if_unreg; - } + pr_debug("%s: No CPU initialized for driver %s\n", __func__, + driver_data->name); + goto err_if_unreg; }
register_hotcpu_notifier(&cpufreq_cpu_notifier);
I don't know for what kind of callers do we have this check for. The problem is that such checks introduce an unnecessary delay for the good users.
So, I am dropping it for now, let me know if this is a bad change.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 09426671eb79..14d637a28dd8 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -202,7 +202,7 @@ struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu) struct cpufreq_policy *policy = NULL; unsigned long flags;
- if (cpufreq_disabled() || (cpu >= nr_cpu_ids)) + if (cpufreq_disabled()) return NULL;
if (!down_read_trylock(&cpufreq_rwsem))
In most of the cases cpufreq wouldn't be disabled and this adds unnecessary delay for its users. In the case if cpufreq is really disabled, then the per-cpu variable will also return NULL and things will continue working as is. Remove this unnecessary check.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 14d637a28dd8..62c6a0b8b0d1 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -202,9 +202,6 @@ struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu) struct cpufreq_policy *policy = NULL; unsigned long flags;
- if (cpufreq_disabled()) - return NULL; - if (!down_read_trylock(&cpufreq_rwsem)) return NULL;
@@ -229,9 +226,6 @@ EXPORT_SYMBOL_GPL(cpufreq_cpu_get);
void cpufreq_cpu_put(struct cpufreq_policy *policy) { - if (cpufreq_disabled()) - return; - kobject_put(&policy->kobj); up_read(&cpufreq_rwsem); }
On 2 January 2015 at 12:34, Viresh Kumar viresh.kumar@linaro.org wrote:
In most of the cases cpufreq wouldn't be disabled and this adds unnecessary delay for its users. In the case if cpufreq is really disabled, then the per-cpu variable will also return NULL and things will continue working as is. Remove this unnecessary check.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
drivers/cpufreq/cpufreq.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 14d637a28dd8..62c6a0b8b0d1 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -202,9 +202,6 @@ struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu) struct cpufreq_policy *policy = NULL; unsigned long flags;
if (cpufreq_disabled())return NULL;if (!down_read_trylock(&cpufreq_rwsem)) return NULL;@@ -229,9 +226,6 @@ EXPORT_SYMBOL_GPL(cpufreq_cpu_get);
void cpufreq_cpu_put(struct cpufreq_policy *policy) {
if (cpufreq_disabled())return;kobject_put(&policy->kobj); up_read(&cpufreq_rwsem);}
@Rafael: Are you fine with this patch as is? I meant, you just left it for rebase conflict or because you want it to be part of the WARN() you suggested?
The WARN here might not be a good idea as this can be called by users while cpufreq is disabled. But we don't need this check because of the reasons I mentioned in commit log.
On Sunday, January 25, 2015 06:54:27 PM Viresh Kumar wrote:
On 2 January 2015 at 12:34, Viresh Kumar viresh.kumar@linaro.org wrote:
In most of the cases cpufreq wouldn't be disabled and this adds unnecessary delay for its users. In the case if cpufreq is really disabled, then the per-cpu variable will also return NULL and things will continue working as is. Remove this unnecessary check.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
drivers/cpufreq/cpufreq.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 14d637a28dd8..62c6a0b8b0d1 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -202,9 +202,6 @@ struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu) struct cpufreq_policy *policy = NULL; unsigned long flags;
if (cpufreq_disabled())return NULL;if (!down_read_trylock(&cpufreq_rwsem)) return NULL;@@ -229,9 +226,6 @@ EXPORT_SYMBOL_GPL(cpufreq_cpu_get);
void cpufreq_cpu_put(struct cpufreq_policy *policy) {
if (cpufreq_disabled())return;kobject_put(&policy->kobj); up_read(&cpufreq_rwsem);}
@Rafael: Are you fine with this patch as is? I meant, you just left it for rebase conflict or because you want it to be part of the WARN() you suggested?
It depended on [15/17] that I didn't apply.
The WARN here might not be a good idea as this can be called by users while cpufreq is disabled. But we don't need this check because of the reasons I mentioned in commit log.
I'm not sure which commit log are you talking about, this one or the [15/17] one?
On 26 January 2015 at 06:09, Rafael J. Wysocki rjw@rjwysocki.net wrote:
It depended on [15/17] that I didn't apply.
That's all I wanted to know.
The WARN here might not be a good idea as this can be called by users while cpufreq is disabled. But we don't need this check because of the reasons I mentioned in commit log.
I'm not sure which commit log are you talking about, this one or the [15/17] one?
This one.
We need to initialize completion and work only on policy allocation and not really on the policy restore side and so we better move this piece of code to cpufreq_policy_alloc().
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 62c6a0b8b0d1..ace5dd4cd271 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1019,6 +1019,8 @@ static struct cpufreq_policy *cpufreq_policy_alloc(void) init_rwsem(&policy->rwsem); spin_lock_init(&policy->transition_lock); init_waitqueue_head(&policy->transition_wait); + init_completion(&policy->kobj_unregister); + INIT_WORK(&policy->update, handle_update);
return policy;
@@ -1142,9 +1144,6 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
cpumask_copy(policy->cpus, cpumask_of(cpu));
- init_completion(&policy->kobj_unregister); - INIT_WORK(&policy->update, handle_update); - /* call driver. From then on the cpufreq must be able * to accept all calls to ->verify and ->setpolicy for this CPU */
On 2 January 2015 at 12:34, Viresh Kumar viresh.kumar@linaro.org wrote:
Along with the big changes I have for cpufreq core simplification, I also accumulated these trivial cleanups. In order to not confuse the much dedicated series on solving the real problems, I am sending these separately.
These are independent of the stats cleanups sent earlier and so it doesn't matter which one you apply first.
Ping for some reviews :)
linaro-kernel@lists.linaro.org
-
Rafael J. Wysocki -
Viresh Kumar