Hi Rafael,
I was looking at cpufreq core this morning for some work and got these minor fixes on the way. Please see if they look good.
-- viresh
Viresh Kumar (7): cpufreq: remove redundant CPUFREQ_INCOMPATIBLE notifier event cpufreq: use memcpy() to copy policy cpufreq: update user_policy.* on success cpufreq: remove redundant 'governor' field from user_policy cpufreq: remove redundant 'policy' field from user_policy cpufreq: rename cpufreq_real_policy as cpufreq_user_policy cpufreq: drop !cpufreq_driver check from cpufreq_parse_governor()
Documentation/cpu-freq/core.txt | 7 ++----- drivers/acpi/processor_perflib.c | 2 +- drivers/cpufreq/cpufreq.c | 31 +++---------------------------- drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 4 ++-- drivers/video/fbdev/pxafb.c | 1 - drivers/video/fbdev/sa1100fb.c | 1 - include/linux/cpufreq.h | 15 ++++++--------- 7 files changed, 14 insertions(+), 47 deletions(-)
What's being done from CPUFREQ_INCOMPATIBLE, can also be done with CPUFREQ_ADJUST. There is nothing special with CPUFREQ_INCOMPATIBLE notifier.
Kill CPUFREQ_INCOMPATIBLE and fix its usage sites.
This also updates the numbering of notifier events to remove holes.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- Documentation/cpu-freq/core.txt | 7 ++----- drivers/acpi/processor_perflib.c | 2 +- drivers/cpufreq/cpufreq.c | 4 ---- drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 4 ++-- drivers/video/fbdev/pxafb.c | 1 - drivers/video/fbdev/sa1100fb.c | 1 - include/linux/cpufreq.h | 9 ++++----- 7 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/Documentation/cpu-freq/core.txt b/Documentation/cpu-freq/core.txt index 70933eadc308..ba78e7c2a069 100644 --- a/Documentation/cpu-freq/core.txt +++ b/Documentation/cpu-freq/core.txt @@ -55,16 +55,13 @@ transition notifiers. ----------------------------
These are notified when a new policy is intended to be set. Each -CPUFreq policy notifier is called three times for a policy transition: +CPUFreq policy notifier is called twice for a policy transition:
1.) During CPUFREQ_ADJUST all CPUFreq notifiers may change the limit if they see a need for this - may it be thermal considerations or hardware limitations.
-2.) During CPUFREQ_INCOMPATIBLE only changes may be done in order to avoid - hardware failure. - -3.) And during CPUFREQ_NOTIFY all notifiers are informed of the new policy +2.) And during CPUFREQ_NOTIFY all notifiers are informed of the new policy - if two hardware drivers failed to agree on a new policy before this stage, the incompatible hardware shall be shut down, and the user informed of this. diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c index 47af702bb6a2..bb01dea39fdc 100644 --- a/drivers/acpi/processor_perflib.c +++ b/drivers/acpi/processor_perflib.c @@ -83,7 +83,7 @@ static int acpi_processor_ppc_notifier(struct notifier_block *nb, if (ignore_ppc) return 0;
- if (event != CPUFREQ_INCOMPATIBLE) + if (event != CPUFREQ_ADJUST) return 0;
mutex_lock(&performance_mutex); diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 76a26609d96b..293f47b814bf 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2206,10 +2206,6 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy, blocking_notifier_call_chain(&cpufreq_policy_notifier_list, CPUFREQ_ADJUST, new_policy);
- /* adjust if necessary - hardware incompatibility*/ - blocking_notifier_call_chain(&cpufreq_policy_notifier_list, - CPUFREQ_INCOMPATIBLE, new_policy); - /* * verify the cpu speed can be set within this limit, which might be * different to the first one diff --git a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c index d29e8da396a0..7969f7690498 100644 --- a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c +++ b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c @@ -97,8 +97,8 @@ static int pmi_notifier(struct notifier_block *nb, struct cpufreq_frequency_table *cbe_freqs; u8 node;
- /* Should this really be called for CPUFREQ_ADJUST, CPUFREQ_INCOMPATIBLE - * and CPUFREQ_NOTIFY policy events?) + /* Should this really be called for CPUFREQ_ADJUST and CPUFREQ_NOTIFY + * policy events?) */ if (event == CPUFREQ_START) return 0; diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c index 7245611ec963..94813af97f09 100644 --- a/drivers/video/fbdev/pxafb.c +++ b/drivers/video/fbdev/pxafb.c @@ -1668,7 +1668,6 @@ pxafb_freq_policy(struct notifier_block *nb, unsigned long val, void *data)
switch (val) { case CPUFREQ_ADJUST: - case CPUFREQ_INCOMPATIBLE: pr_debug("min dma period: %d ps, " "new clock %d kHz\n", pxafb_display_dma_period(var), policy->max); diff --git a/drivers/video/fbdev/sa1100fb.c b/drivers/video/fbdev/sa1100fb.c index 89dd7e02197f..dcf774c15889 100644 --- a/drivers/video/fbdev/sa1100fb.c +++ b/drivers/video/fbdev/sa1100fb.c @@ -1042,7 +1042,6 @@ sa1100fb_freq_policy(struct notifier_block *nb, unsigned long val,
switch (val) { case CPUFREQ_ADJUST: - case CPUFREQ_INCOMPATIBLE: dev_dbg(fbi->dev, "min dma period: %d ps, " "new clock %d kHz\n", sa1100fb_min_dma_period(fbi), policy->max); diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index bde1e567b3a9..bedcc90c0757 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -369,11 +369,10 @@ static inline void cpufreq_resume(void) {}
/* Policy Notifiers */ #define CPUFREQ_ADJUST (0) -#define CPUFREQ_INCOMPATIBLE (1) -#define CPUFREQ_NOTIFY (2) -#define CPUFREQ_START (3) -#define CPUFREQ_CREATE_POLICY (4) -#define CPUFREQ_REMOVE_POLICY (5) +#define CPUFREQ_NOTIFY (1) +#define CPUFREQ_START (2) +#define CPUFREQ_CREATE_POLICY (3) +#define CPUFREQ_REMOVE_POLICY (4)
#ifdef CONFIG_CPU_FREQ int cpufreq_register_notifier(struct notifier_block *nb, unsigned int list);
On Monday, August 03, 2015 08:36:14 AM Viresh Kumar wrote:
What's being done from CPUFREQ_INCOMPATIBLE, can also be done with CPUFREQ_ADJUST. There is nothing special with CPUFREQ_INCOMPATIBLE notifier.
The above part of the changelog is a disaster to me. :-(
It not only doesn't explain what really goes on, but it's actively confusing.
What really happens is that the core sends CPUFREQ_INCOMPATIBLE notifications unconditionally right after sending the CPUFREQ_ADJUST ones, so the former is just redundant and it's more efficient to merge the two into one.
Kill CPUFREQ_INCOMPATIBLE and fix its usage sites.
This also updates the numbering of notifier events to remove holes.
Why don't you redefine CPUFREQ_ADJUST as 1 instead?
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
Documentation/cpu-freq/core.txt | 7 ++----- drivers/acpi/processor_perflib.c | 2 +- drivers/cpufreq/cpufreq.c | 4 ---- drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 4 ++-- drivers/video/fbdev/pxafb.c | 1 - drivers/video/fbdev/sa1100fb.c | 1 - include/linux/cpufreq.h | 9 ++++----- 7 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/Documentation/cpu-freq/core.txt b/Documentation/cpu-freq/core.txt index 70933eadc308..ba78e7c2a069 100644 --- a/Documentation/cpu-freq/core.txt +++ b/Documentation/cpu-freq/core.txt @@ -55,16 +55,13 @@ transition notifiers.
These are notified when a new policy is intended to be set. Each -CPUFreq policy notifier is called three times for a policy transition: +CPUFreq policy notifier is called twice for a policy transition: 1.) During CPUFREQ_ADJUST all CPUFreq notifiers may change the limit if they see a need for this - may it be thermal considerations or hardware limitations. -2.) During CPUFREQ_INCOMPATIBLE only changes may be done in order to avoid
- hardware failure.
-3.) And during CPUFREQ_NOTIFY all notifiers are informed of the new policy +2.) And during CPUFREQ_NOTIFY all notifiers are informed of the new policy - if two hardware drivers failed to agree on a new policy before this stage, the incompatible hardware shall be shut down, and the user informed of this. diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c index 47af702bb6a2..bb01dea39fdc 100644 --- a/drivers/acpi/processor_perflib.c +++ b/drivers/acpi/processor_perflib.c @@ -83,7 +83,7 @@ static int acpi_processor_ppc_notifier(struct notifier_block *nb, if (ignore_ppc) return 0;
- if (event != CPUFREQ_INCOMPATIBLE)
- if (event != CPUFREQ_ADJUST) return 0;
mutex_lock(&performance_mutex); diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 76a26609d96b..293f47b814bf 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2206,10 +2206,6 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy, blocking_notifier_call_chain(&cpufreq_policy_notifier_list, CPUFREQ_ADJUST, new_policy);
- /* adjust if necessary - hardware incompatibility*/
- blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
CPUFREQ_INCOMPATIBLE, new_policy);- /*
- verify the cpu speed can be set within this limit, which might be
- different to the first one
diff --git a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c index d29e8da396a0..7969f7690498 100644 --- a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c +++ b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c @@ -97,8 +97,8 @@ static int pmi_notifier(struct notifier_block *nb, struct cpufreq_frequency_table *cbe_freqs; u8 node;
- /* Should this really be called for CPUFREQ_ADJUST, CPUFREQ_INCOMPATIBLE
* and CPUFREQ_NOTIFY policy events?)
- /* Should this really be called for CPUFREQ_ADJUST and CPUFREQ_NOTIFY
* policy events?)diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c index 7245611ec963..94813af97f09 100644 --- a/drivers/video/fbdev/pxafb.c +++ b/drivers/video/fbdev/pxafb.c @@ -1668,7 +1668,6 @@ pxafb_freq_policy(struct notifier_block *nb, unsigned long val, void *data) switch (val) { case CPUFREQ_ADJUST:
- case CPUFREQ_INCOMPATIBLE: pr_debug("min dma period: %d ps, " "new clock %d kHz\n", pxafb_display_dma_period(var), policy->max);
diff --git a/drivers/video/fbdev/sa1100fb.c b/drivers/video/fbdev/sa1100fb.c index 89dd7e02197f..dcf774c15889 100644 --- a/drivers/video/fbdev/sa1100fb.c +++ b/drivers/video/fbdev/sa1100fb.c @@ -1042,7 +1042,6 @@ sa1100fb_freq_policy(struct notifier_block *nb, unsigned long val, switch (val) { case CPUFREQ_ADJUST:
- case CPUFREQ_INCOMPATIBLE: dev_dbg(fbi->dev, "min dma period: %d ps, " "new clock %d kHz\n", sa1100fb_min_dma_period(fbi), policy->max);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index bde1e567b3a9..bedcc90c0757 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -369,11 +369,10 @@ static inline void cpufreq_resume(void) {} /* Policy Notifiers */ #define CPUFREQ_ADJUST (0) -#define CPUFREQ_INCOMPATIBLE (1) -#define CPUFREQ_NOTIFY (2) -#define CPUFREQ_START (3) -#define CPUFREQ_CREATE_POLICY (4) -#define CPUFREQ_REMOVE_POLICY (5) +#define CPUFREQ_NOTIFY (1) +#define CPUFREQ_START (2) +#define CPUFREQ_CREATE_POLICY (3) +#define CPUFREQ_REMOVE_POLICY (4) #ifdef CONFIG_CPU_FREQ int cpufreq_register_notifier(struct notifier_block *nb, unsigned int list);
On 10-09-15, 01:26, Rafael J. Wysocki wrote:
On Monday, August 03, 2015 08:36:14 AM Viresh Kumar wrote:
What's being done from CPUFREQ_INCOMPATIBLE, can also be done with CPUFREQ_ADJUST. There is nothing special with CPUFREQ_INCOMPATIBLE notifier.
The above part of the changelog is a disaster to me. :-(
It not only doesn't explain what really goes on, but it's actively confusing.
What really happens is that the core sends CPUFREQ_INCOMPATIBLE notifications unconditionally right after sending the CPUFREQ_ADJUST ones, so the former is just redundant and it's more efficient to merge the two into one.
Undoubtedly this looks far better :)
But, isn't this series already applied some time back ?
Kill CPUFREQ_INCOMPATIBLE and fix its usage sites.
This also updates the numbering of notifier events to remove holes.
Why don't you redefine CPUFREQ_ADJUST as 1 instead?
So that there is no request with 0? Yeah that could have been done.
Hi,
On Thu, Sep 10, 2015 at 2:39 AM, Viresh Kumar viresh.kumar@linaro.org wrote:
On 10-09-15, 01:26, Rafael J. Wysocki wrote:
On Monday, August 03, 2015 08:36:14 AM Viresh Kumar wrote:
What's being done from CPUFREQ_INCOMPATIBLE, can also be done with CPUFREQ_ADJUST. There is nothing special with CPUFREQ_INCOMPATIBLE notifier.
The above part of the changelog is a disaster to me. :-(
It not only doesn't explain what really goes on, but it's actively confusing.
What really happens is that the core sends CPUFREQ_INCOMPATIBLE notifications unconditionally right after sending the CPUFREQ_ADJUST ones, so the former is just redundant and it's more efficient to merge the two into one.
Undoubtedly this looks far better :)
But, isn't this series already applied some time back ?
Right, never mind. For some reason that patch was left in the "New" state.
The code is OK.
Thanks, Rafael
On 09/09/2015 05:53 PM, Rafael J. Wysocki wrote:
Hi,
On Thu, Sep 10, 2015 at 2:39 AM, Viresh Kumar viresh.kumar@linaro.org wrote:
On 10-09-15, 01:26, Rafael J. Wysocki wrote:
On Monday, August 03, 2015 08:36:14 AM Viresh Kumar wrote:
What's being done from CPUFREQ_INCOMPATIBLE, can also be done with CPUFREQ_ADJUST. There is nothing special with CPUFREQ_INCOMPATIBLE notifier.
The above part of the changelog is a disaster to me. :-(
It not only doesn't explain what really goes on, but it's actively confusing.
What really happens is that the core sends CPUFREQ_INCOMPATIBLE notifications unconditionally right after sending the CPUFREQ_ADJUST ones, so the former is just redundant and it's more efficient to merge the two into one.
Undoubtedly this looks far better :)
But, isn't this series already applied some time back ?
Right, never mind. For some reason that patch was left in the "New" state.
The code is OK.
I guess I didn't notice this change when it was sent out.
The comment that was deleted in this patch clearly states why the INCOMPATIBLE notifier is needed. Some client might want to boost the CPU min freq for performance or other reasons, but thermal might want to limit it. So, by having thermal register for INCOMPATIBLE notifiers to enforce the limits, we provide a way to guarantee it gets the final say.
The real fix should have been to change drivers/thermal/cpu_cooling.c to use CPUFREQ_INCOMPATIBLE instead of CPUFREQ_ADJUST.
Is there something I'm missing? If not, can we please revert this patch?
Thanks, Saravana
On Wed, Apr 6, 2016 at 10:30 PM, Saravana Kannan skannan@codeaurora.org wrote:
On 09/09/2015 05:53 PM, Rafael J. Wysocki wrote:
Hi,
On Thu, Sep 10, 2015 at 2:39 AM, Viresh Kumar viresh.kumar@linaro.org wrote:
On 10-09-15, 01:26, Rafael J. Wysocki wrote:
On Monday, August 03, 2015 08:36:14 AM Viresh Kumar wrote:
What's being done from CPUFREQ_INCOMPATIBLE, can also be done with CPUFREQ_ADJUST. There is nothing special with CPUFREQ_INCOMPATIBLE notifier.
The above part of the changelog is a disaster to me. :-(
It not only doesn't explain what really goes on, but it's actively confusing.
What really happens is that the core sends CPUFREQ_INCOMPATIBLE notifications unconditionally right after sending the CPUFREQ_ADJUST ones, so the former is just redundant and it's more efficient to merge the two into one.
Undoubtedly this looks far better :)
But, isn't this series already applied some time back ?
Right, never mind. For some reason that patch was left in the "New" state.
The code is OK.
I guess I didn't notice this change when it was sent out.
The comment that was deleted in this patch clearly states why the INCOMPATIBLE notifier is needed. Some client might want to boost the CPU min freq for performance or other reasons, but thermal might want to limit it. So, by having thermal register for INCOMPATIBLE notifiers to enforce the limits, we provide a way to guarantee it gets the final say.
The real fix should have been to change drivers/thermal/cpu_cooling.c to use CPUFREQ_INCOMPATIBLE instead of CPUFREQ_ADJUST.
Is there something I'm missing? If not, can we please revert this patch?
Well, nobody was using that event.
Thanks, Rafael
On 04/06/2016 02:21 PM, Rafael J. Wysocki wrote:
On Wed, Apr 6, 2016 at 10:30 PM, Saravana Kannan skannan@codeaurora.org wrote:
On 09/09/2015 05:53 PM, Rafael J. Wysocki wrote:
Hi,
On Thu, Sep 10, 2015 at 2:39 AM, Viresh Kumar viresh.kumar@linaro.org wrote:
On 10-09-15, 01:26, Rafael J. Wysocki wrote:
On Monday, August 03, 2015 08:36:14 AM Viresh Kumar wrote:
What's being done from CPUFREQ_INCOMPATIBLE, can also be done with CPUFREQ_ADJUST. There is nothing special with CPUFREQ_INCOMPATIBLE notifier.
The above part of the changelog is a disaster to me. :-(
It not only doesn't explain what really goes on, but it's actively confusing.
What really happens is that the core sends CPUFREQ_INCOMPATIBLE notifications unconditionally right after sending the CPUFREQ_ADJUST ones, so the former is just redundant and it's more efficient to merge the two into one.
Undoubtedly this looks far better :)
But, isn't this series already applied some time back ?
Right, never mind. For some reason that patch was left in the "New" state.
The code is OK.
I guess I didn't notice this change when it was sent out.
The comment that was deleted in this patch clearly states why the INCOMPATIBLE notifier is needed. Some client might want to boost the CPU min freq for performance or other reasons, but thermal might want to limit it. So, by having thermal register for INCOMPATIBLE notifiers to enforce the limits, we provide a way to guarantee it gets the final say.
The real fix should have been to change drivers/thermal/cpu_cooling.c to use CPUFREQ_INCOMPATIBLE instead of CPUFREQ_ADJUST.
Is there something I'm missing? If not, can we please revert this patch?
Well, nobody was using that event.
True, but that's more of a bug in drivers/thermal/cpu-cooling.c and drivers/acpi/processor_thermal.c. We should revert this patch and fix those drivers. Does that seem acceptable to you?
-Saravana
On Wed, Apr 6, 2016 at 11:29 PM, Saravana Kannan skannan@codeaurora.org wrote:
On 04/06/2016 02:21 PM, Rafael J. Wysocki wrote:
On Wed, Apr 6, 2016 at 10:30 PM, Saravana Kannan skannan@codeaurora.org wrote:
On 09/09/2015 05:53 PM, Rafael J. Wysocki wrote:
[cut]
Well, nobody was using that event.
True, but that's more of a bug in drivers/thermal/cpu-cooling.c and drivers/acpi/processor_thermal.c. We should revert this patch and fix those drivers. Does that seem acceptable to you?
I'd rather see a patch series adding the event back along with some users. One user at least.
On 04/06/2016 02:45 PM, Rafael J. Wysocki wrote:
On Wed, Apr 6, 2016 at 11:29 PM, Saravana Kannan skannan@codeaurora.org wrote:
On 04/06/2016 02:21 PM, Rafael J. Wysocki wrote:
On Wed, Apr 6, 2016 at 10:30 PM, Saravana Kannan skannan@codeaurora.org wrote:
On 09/09/2015 05:53 PM, Rafael J. Wysocki wrote:
[cut]
Well, nobody was using that event.
True, but that's more of a bug in drivers/thermal/cpu-cooling.c and drivers/acpi/processor_thermal.c. We should revert this patch and fix those drivers. Does that seem acceptable to you?
I'd rather see a patch series adding the event back along with some users. One user at least.
Ok, I'll make those two drivers use them and send it out. It's very clearly a bug in those drivers.
-Saravana`
cpufreq_get_policy() is useful if the pointer to policy isn't available in advance. But if it is available, then there is no need to call cpufreq_get_policy(). Directly use memcpy() to copy the policy.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 293f47b814bf..86d69416821b 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -606,9 +606,7 @@ static ssize_t store_##file_name \ int ret, temp; \ struct cpufreq_policy new_policy; \ \ - ret = cpufreq_get_policy(&new_policy, policy->cpu); \ - if (ret) \ - return -EINVAL; \ + memcpy(&new_policy, policy, sizeof(*policy)); \ \ ret = sscanf(buf, "%u", &new_policy.object); \ if (ret != 1) \ @@ -662,9 +660,7 @@ static ssize_t store_scaling_governor(struct cpufreq_policy *policy, char str_governor[16]; struct cpufreq_policy new_policy;
- ret = cpufreq_get_policy(&new_policy, policy->cpu); - if (ret) - return ret; + memcpy(&new_policy, policy, sizeof(*policy));
ret = sscanf(buf, "%15s", str_governor); if (ret != 1)
'user_policy' caches properties of a policy that are set by userspace. And these must be updated only if cpufreq core was successful in updating them based on request from user space.
In store_scaling_governor(), we are updating user_policy.policy and user_policy.governor even if cpufreq_set_policy() failed. That's incorrect.
Fix this by updating user_policy.* only if we were successful in updating the properties.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 86d69416821b..8e71d8e08439 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -671,14 +671,12 @@ static ssize_t store_scaling_governor(struct cpufreq_policy *policy, return -EINVAL;
ret = cpufreq_set_policy(policy, &new_policy); + if (ret) + return ret;
policy->user_policy.policy = policy->policy; policy->user_policy.governor = policy->governor; - - if (ret) - return ret; - else - return count; + return count; }
/**
Its always same as policy->governor, and there is no need to keep another copy of it. Remove it.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 7 ++----- include/linux/cpufreq.h | 1 - 2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 8e71d8e08439..3033952391fe 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -675,7 +675,6 @@ static ssize_t store_scaling_governor(struct cpufreq_policy *policy, return ret;
policy->user_policy.policy = policy->policy; - policy->user_policy.governor = policy->governor; return count; }
@@ -1323,10 +1322,9 @@ static int cpufreq_online(unsigned int cpu) goto out_exit_policy; }
- if (new_policy) { + if (new_policy) policy->user_policy.policy = policy->policy; - policy->user_policy.governor = policy->governor; - } + up_write(&policy->rwsem);
kobject_uevent(&policy->kobj, KOBJ_ADD); @@ -2305,7 +2303,6 @@ int cpufreq_update_policy(unsigned int cpu) new_policy.min = policy->user_policy.min; new_policy.max = policy->user_policy.max; new_policy.policy = policy->user_policy.policy; - new_policy.governor = policy->user_policy.governor;
/* * BIOS might change freq behind our back diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index bedcc90c0757..752bf8a5c314 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -55,7 +55,6 @@ struct cpufreq_real_policy { unsigned int min; /* in kHz */ unsigned int max; /* in kHz */ unsigned int policy; /* see above */ - struct cpufreq_governor *governor; /* see below */ };
struct cpufreq_policy {
Its always same as policy->policy, and there is no need to keep another copy of it. Remove it.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 10 +--------- include/linux/cpufreq.h | 1 - 2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 3033952391fe..a80bd68bbd74 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -671,11 +671,7 @@ static ssize_t store_scaling_governor(struct cpufreq_policy *policy, return -EINVAL;
ret = cpufreq_set_policy(policy, &new_policy); - if (ret) - return ret; - - policy->user_policy.policy = policy->policy; - return count; + return ret ? ret : count; }
/** @@ -1322,9 +1318,6 @@ static int cpufreq_online(unsigned int cpu) goto out_exit_policy; }
- if (new_policy) - policy->user_policy.policy = policy->policy; - up_write(&policy->rwsem);
kobject_uevent(&policy->kobj, KOBJ_ADD); @@ -2302,7 +2295,6 @@ int cpufreq_update_policy(unsigned int cpu) memcpy(&new_policy, policy, sizeof(*policy)); new_policy.min = policy->user_policy.min; new_policy.max = policy->user_policy.max; - new_policy.policy = policy->user_policy.policy;
/* * BIOS might change freq behind our back diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 752bf8a5c314..54dbbff0a55e 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -54,7 +54,6 @@ struct cpufreq_cpuinfo { struct cpufreq_real_policy { unsigned int min; /* in kHz */ unsigned int max; /* in kHz */ - unsigned int policy; /* see above */ };
struct cpufreq_policy {
Its all about caching min/max freq requested by userspace, and the name 'cpufreq_real_policy' doesn't fit that well. Rename it to cpufreq_user_policy.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- include/linux/cpufreq.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 54dbbff0a55e..6ff6a4d95eea 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -51,7 +51,7 @@ struct cpufreq_cpuinfo { unsigned int transition_latency; };
-struct cpufreq_real_policy { +struct cpufreq_user_policy { unsigned int min; /* in kHz */ unsigned int max; /* in kHz */ }; @@ -86,7 +86,7 @@ struct cpufreq_policy { struct work_struct update; /* if update_policy() needs to be * called, but you're in IRQ context */
- struct cpufreq_real_policy user_policy; + struct cpufreq_user_policy user_policy; struct cpufreq_frequency_table *freq_table;
struct list_head policy_list;
Driver is guaranteed to be present on a call to cpufreq_parse_governor() and there is no need to check for !cpufreq_driver. Drop 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 a80bd68bbd74..a05cc75cc45d 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -520,9 +520,6 @@ static int cpufreq_parse_governor(char *str_governor, unsigned int *policy, { int err = -EINVAL;
- if (!cpufreq_driver) - goto out; - if (cpufreq_driver->setpolicy) { if (!strncasecmp(str_governor, "performance", CPUFREQ_NAME_LEN)) { *policy = CPUFREQ_POLICY_PERFORMANCE; @@ -557,7 +554,6 @@ static int cpufreq_parse_governor(char *str_governor, unsigned int *policy,
mutex_unlock(&cpufreq_governor_mutex); } -out: return err; }
linaro-kernel@lists.linaro.org
-
Rafael J. Wysocki -
Rafael J. Wysocki -
Saravana Kannan -
Viresh Kumar