CPUFreq: Minor cleanups

PATCH In review Viresh Kumar 4 participants 16 messages Aug 3, 2015 – Apr 6, 2016 1 message not in the archive

Subsystems (4)

Documentation ACPI Framebuffer layer CPU frequency scaling framework

lore

Share & tools

lore

Viresh Kumar

author maintainer

2015-08-03 03:06 UTC #0 lore raw

cover CPUFreq: Minor cleanups

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(-)

Signature · 1 line

2.4.0

Viresh Kumar

author maintainer

2015-08-03 03:06 UTC #1 lore raw

PATCH 1/7 cpufreq: remove redundant CPUFREQ_INCOMPATIBLE notifier event

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 <[email protected]>

---
 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(-)

Documentation/cpu-freq/core.txt +2 -5

index 70933eadc308..ba78e7c2a069 100644
@@ -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.

drivers/acpi/processor_perflib.c +1 -1

index 47af702bb6a2..bb01dea39fdc 100644
@@ -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);

drivers/cpufreq/cpufreq.c +0 -4

index 76a26609d96b..293f47b814bf 100644
@@ -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

drivers/cpufreq/ppc_cbe_cpufreq_pmi.c +2 -2

index d29e8da396a0..7969f7690498 100644
@@ -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;

drivers/video/fbdev/pxafb.c +0 -1

index 7245611ec963..94813af97f09 100644
@@ -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);

drivers/video/fbdev/sa1100fb.c +0 -1

index 89dd7e02197f..dcf774c15889 100644
@@ -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);

include/linux/cpufreq.h +4 -5

index bde1e567b3a9..bedcc90c0757 100644
@@ -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);

Signature · 1 line

2.4.0

Rafael J. Wysocki

2015-09-09 22:58 UTC #2 lore raw

Re: [PATCH 1/7] cpufreq: remove redundant CPUFREQ_INCOMPATIBLE notifier event

Quoted 3 lines · Viresh Kumar

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.

Quoted 3 lines

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?

Quoted 122 lines

Signed-off-by Viresh Kumar <[email protected]>

---
 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(-)

Documentation/cpu-freq/core.txt +2 -5

index 70933eadc308..ba78e7c2a069 100644
@@ -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.

drivers/acpi/processor_perflib.c +1 -1

index 47af702bb6a2..bb01dea39fdc 100644
@@ -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);

drivers/cpufreq/cpufreq.c +0 -4

index 76a26609d96b..293f47b814bf 100644
@@ -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

drivers/cpufreq/ppc_cbe_cpufreq_pmi.c +2 -2

index d29e8da396a0..7969f7690498 100644
@@ -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;

drivers/video/fbdev/pxafb.c +0 -1

index 7245611ec963..94813af97f09 100644
@@ -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);

drivers/video/fbdev/sa1100fb.c +0 -1

index 89dd7e02197f..dcf774c15889 100644
@@ -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);

include/linux/cpufreq.h +4 -5

index bde1e567b3a9..bedcc90c0757 100644
@@ -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);

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Viresh Kumar

author maintainer

2015-09-10 00:39 UTC #3 lore raw

Re: [PATCH 1/7] cpufreq: remove redundant CPUFREQ_INCOMPATIBLE notifier event

Quoted 5 lines · Rafael J. Wysocki

Quoted 3 lines · Viresh Kumar
What's being done from CPUFREQ_INCOMPATIBLE, can also be done with CPUFREQ_ADJUST. There is nothing special with CPUFREQ_INCOMPATIBLE notifier.

Quoted 7 lines

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 ?

Quoted 5 lines

Quoted 3 lines
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.

Signature · 1 line

viresh

Rafael J. Wysocki

maintainer

2015-09-10 00:53 UTC #4 lore raw

Re: [PATCH 1/7] cpufreq: remove redundant CPUFREQ_INCOMPATIBLE notifier event

Hi,

Quoted 10 lines · Viresh Kumar

Quoted 9 lines · Rafael J. Wysocki
Quoted 3 lines · Viresh Kumar
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.

Quoted 7 lines

Quoted 3 lines
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

A message that is not in the archive (sent off-list or not yet received): [email protected]

Rafael J. Wysocki

maintainer

2016-04-06 21:21 UTC #6 lore raw

Re: [PATCH 1/7] cpufreq: remove redundant CPUFREQ_INCOMPATIBLE notifier event

Quoted 48 lines · Saravana Kannan

Quoted 29 lines · Rafael J. Wysocki
Hi,
Quoted 24 lines · Viresh Kumar
Quoted 10 lines · Rafael J. Wysocki
Quoted 4 lines · Viresh Kumar
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. :-(
Quoted 8 lines
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 :)
Quoted 7 lines
Quoted 1 line
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.

Quoted 4 lines

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

Saravana Kannan

2016-04-06 21:29 UTC #7 lore raw

Re: [PATCH 1/7] cpufreq: remove redundant CPUFREQ_INCOMPATIBLE notifier event

Quoted 49 lines · Rafael J. Wysocki

Quoted 48 lines · Saravana Kannan
Quoted 29 lines · Rafael J. Wysocki
Hi,
Quoted 24 lines · Viresh Kumar
Quoted 10 lines · Rafael J. Wysocki
Quoted 4 lines · Viresh Kumar
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. :-(
Quoted 8 lines
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 :)
Quoted 7 lines
Quoted 1 line
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.

Quoted 7 lines

Quoted 4 lines
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

Signature · 3 lines

Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Rafael J. Wysocki

maintainer

2016-04-06 21:45 UTC #8 lore raw

Re: [PATCH 1/7] cpufreq: remove redundant CPUFREQ_INCOMPATIBLE notifier event

Quoted 7 lines · Saravana Kannan

Quoted 6 lines · Rafael J. Wysocki
Quoted 3 lines · Saravana Kannan
Quoted 1 line · Rafael J. Wysocki

[cut]

Quoted 7 lines

Quoted 3 lines
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.

Saravana Kannan

2016-04-06 21:49 UTC #9 lore raw

Re: [PATCH 1/7] cpufreq: remove redundant CPUFREQ_INCOMPATIBLE notifier event

Quoted 15 lines · Rafael J. Wysocki

Quoted 7 lines · Saravana Kannan
Quoted 6 lines · Rafael J. Wysocki
Quoted 3 lines · Saravana Kannan
Quoted 1 line · Rafael J. Wysocki
[cut]
Quoted 4 lines
Quoted 3 lines
Well, nobody was using that event.

Quoted 7 lines

Quoted 3 lines
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`

Signature · 3 lines

Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Viresh Kumar

author maintainer

2015-08-03 03:06 UTC #10 lore raw

PATCH 2/7 cpufreq: use memcpy() to copy policy

In reply to Viresh Kumar

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 <[email protected]>

---
 drivers/cpufreq/cpufreq.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

drivers/cpufreq/cpufreq.c +2 -6

index 293f47b814bf..86d69416821b 100644
@@ -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)

Signature · 1 line

2.4.0

Viresh Kumar

author maintainer

2015-08-03 03:06 UTC #11 lore raw

PATCH 3/7 cpufreq: update user_policy.* on success

In reply to Viresh Kumar

'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 <[email protected]>

---
 drivers/cpufreq/cpufreq.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

drivers/cpufreq/cpufreq.c +3 -5

index 86d69416821b..8e71d8e08439 100644
@@ -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;
 }
 
 /**

Signature · 1 line

2.4.0

Viresh Kumar

author maintainer

2015-08-03 03:06 UTC #12 lore raw

PATCH 4/7 cpufreq: remove redundant 'governor' field from user_policy

In reply to Viresh Kumar

Its always same as policy->governor, and there is no need to keep another copy of it. Remove it.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/cpufreq/cpufreq.c | 7 ++-----
 include/linux/cpufreq.h   | 1 -
 2 files changed, 2 insertions(+), 6 deletions(-)

drivers/cpufreq/cpufreq.c +2 -5

index 8e71d8e08439..3033952391fe 100644
@@ -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

include/linux/cpufreq.h +0 -1

index bedcc90c0757..752bf8a5c314 100644
@@ -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 {

Signature · 1 line

2.4.0

Viresh Kumar

author maintainer

2015-08-03 03:06 UTC #13 lore raw

PATCH 5/7 cpufreq: remove redundant 'policy' field from user_policy

In reply to Viresh Kumar

Its always same as policy->policy, and there is no need to keep another copy of it. Remove it.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/cpufreq/cpufreq.c | 10 +---------
 include/linux/cpufreq.h   |  1 -
 2 files changed, 1 insertion(+), 10 deletions(-)

drivers/cpufreq/cpufreq.c +1 -9

index 3033952391fe..a80bd68bbd74 100644
@@ -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

include/linux/cpufreq.h +0 -1

index 752bf8a5c314..54dbbff0a55e 100644
@@ -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 {

Signature · 1 line

2.4.0

Viresh Kumar

author maintainer

2015-08-03 03:06 UTC #14 lore raw

PATCH 6/7 cpufreq: rename cpufreq_real_policy as cpufreq_user_policy

In reply to Viresh Kumar

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 <[email protected]>

---
 include/linux/cpufreq.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

include/linux/cpufreq.h +2 -2

index 54dbbff0a55e..6ff6a4d95eea 100644
@@ -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;

Signature · 1 line

2.4.0

Viresh Kumar

author maintainer

2015-08-03 03:06 UTC #15 lore raw

PATCH 7/7 cpufreq: drop !cpufreq_driver check from cpufreq_parse_governor()

In reply to Viresh Kumar

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 <[email protected]>

---
 drivers/cpufreq/cpufreq.c | 4 ----
 1 file changed, 4 deletions(-)

drivers/cpufreq/cpufreq.c +0 -4

index a80bd68bbd74..a05cc75cc45d 100644
@@ -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;
 }

Signature · 1 line

2.4.0

Rafael J. Wysocki

2015-08-28 14:59 UTC #16 lore raw

In reply to Viresh Kumar

Quoted 4 lines · Viresh Kumar

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.

I'm queuing up this series for 4.3, thanks!

Rafael