[PATCH 0/2] cpufreq: minor cleanups around policy parsing and API

[PATCH 0/2] cpufreq: minor cleanups around policy parsing and API

[Zihuan Zhang]

This series contains two small cpufreq cleanups:

  1) Use strlen() as the length argument for strncasecmp() in
     cpufreq_parse_policy(). This makes comparison more permissive
     (prefix match), which is consistent with the intention in our
     deployment and improves readability over using CPUFREQ_NAME_LEN.

  2) Drop the redundant `new_pol` parameter from cpufreq_set_policy().
     Only drivers that implement ->setpolicy would ever care about
     policy values, and passing `new_pol` around is unnecessary.
     Callers now pass just the policy and (where applicable) the
     governor pointer; drivers can read policy->policy directly.

No functional regressions are expected for drivers which do not
implement ->setpolicy (e.g. acpi-cpufreq), as they ignore policy
values anyway.

Zihuan Zhang (2):
  cpufreq: use strlen() for governor name comparison
  cpufreq: simplify cpufreq_set_policy() interface

 drivers/cpufreq/cpufreq.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

-- 
2.25.1

[PATCH 1/2] cpufreq: use strlen() for governor name comparison

[Zihuan Zhang]

Most kernel code using strncasecmp()/strncmp() passes strlen("xxx")
as the length argument. cpufreq_parse_policy() previously used
CPUFREQ_NAME_LEN (16), which is longer than the actual strings
("performance" is 11 chars, "powersave" is 9 chars).

This patch switches to strlen() for the comparison, making the
matching slightly more permissive (e.g., "powersavexxx" will now
also match "powersave"). While this is unlikely to cause functional
issues, it aligns cpufreq with common kernel style and makes the
behavior more intuitive.

Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
---
 drivers/cpufreq/cpufreq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index fc7eace8b65b..a067b5447fe8 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -664,10 +664,10 @@ static struct cpufreq_governor *get_governor(const char *str_governor)
 
 static unsigned int cpufreq_parse_policy(char *str_governor)
 {
-	if (!strncasecmp(str_governor, "performance", CPUFREQ_NAME_LEN))
+	if (!strncasecmp(str_governor, "performance", strlen("performance")))
 		return CPUFREQ_POLICY_PERFORMANCE;
 
-	if (!strncasecmp(str_governor, "powersave", CPUFREQ_NAME_LEN))
+	if (!strncasecmp(str_governor, "powersave", strlen("powersave")))
 		return CPUFREQ_POLICY_POWERSAVE;
 
 	return CPUFREQ_POLICY_UNKNOWN;
-- 
2.25.1

Re: [PATCH 1/2] cpufreq: use strlen() for governor name comparison

[Viresh Kumar]

On 19-08-25, 18:39, Zihuan Zhang wrote:
> Most kernel code using strncasecmp()/strncmp() passes strlen("xxx")
> as the length argument. cpufreq_parse_policy() previously used
> CPUFREQ_NAME_LEN (16), which is longer than the actual strings
> ("performance" is 11 chars, "powersave" is 9 chars).
> 
> This patch switches to strlen() for the comparison, making the
> matching slightly more permissive (e.g., "powersavexxx" will now
> also match "powersave"). While this is unlikely to cause functional
> issues, it aligns cpufreq with common kernel style and makes the
> behavior more intuitive.
> 
> Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
> ---
>  drivers/cpufreq/cpufreq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[PATCH 2/2] cpufreq: simplify cpufreq_set_policy() interface

[Zihuan Zhang]

The current cpufreq_set_policy() takes three arguments, including
new_pol. However, new_pol is only meaningful when a driver provides
the ->setpolicy callback. Most cpufreq drivers (e.g. ACPI) do not
implement this callback, so the extra parameter is unused.

Passing new_pol in such cases is unnecessary and may mislead readers
into thinking it has a functional effect, while in reality it is only
relevant for a very limited set of drivers.

Simplify the interface by removing the redundant argument and letting
drivers that implement ->setpolicy rely directly on
policy->policy. This reduces parameter passing overhead and makes the
API clearer.

Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
---
 drivers/cpufreq/cpufreq.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index a067b5447fe8..ea4a5d3f786c 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -85,8 +85,7 @@ static int cpufreq_init_governor(struct cpufreq_policy *policy);
 static void cpufreq_exit_governor(struct cpufreq_policy *policy);
 static void cpufreq_governor_limits(struct cpufreq_policy *policy);
 static int cpufreq_set_policy(struct cpufreq_policy *policy,
-			      struct cpufreq_governor *new_gov,
-			      unsigned int new_pol);
+			      struct cpufreq_governor *new_gov);
 static bool cpufreq_boost_supported(void);
 static int cpufreq_boost_trigger_state(int state);
 
@@ -822,7 +821,8 @@ static ssize_t store_scaling_governor(struct cpufreq_policy *policy,
 		if (!new_pol)
 			return -EINVAL;
 
-		ret = cpufreq_set_policy(policy, NULL, new_pol);
+		policy->policy = new_pol;
+		ret = cpufreq_set_policy(policy, NULL);
 	} else {
 		struct cpufreq_governor *new_gov;
 
@@ -830,8 +830,8 @@ static ssize_t store_scaling_governor(struct cpufreq_policy *policy,
 		if (!new_gov)
 			return -EINVAL;
 
-		ret = cpufreq_set_policy(policy, new_gov,
-					 CPUFREQ_POLICY_UNKNOWN);
+		policy->policy = CPUFREQ_POLICY_UNKNOWN;
+		ret = cpufreq_set_policy(policy, new_gov);
 
 		module_put(new_gov->owner);
 	}
@@ -1154,7 +1154,8 @@ static int cpufreq_init_policy(struct cpufreq_policy *policy)
 			return -ENODATA;
 	}
 
-	ret = cpufreq_set_policy(policy, gov, pol);
+	policy->policy = pol;
+	ret = cpufreq_set_policy(policy, gov);
 	if (gov)
 		module_put(gov->owner);
 
@@ -1190,7 +1191,7 @@ void refresh_frequency_limits(struct cpufreq_policy *policy)
 	if (!policy_is_inactive(policy)) {
 		pr_debug("updating policy for CPU %u\n", policy->cpu);
 
-		cpufreq_set_policy(policy, policy->governor, policy->policy);
+		cpufreq_set_policy(policy, policy->governor);
 	}
 }
 EXPORT_SYMBOL(refresh_frequency_limits);
@@ -2610,7 +2611,6 @@ static void cpufreq_update_pressure(struct cpufreq_policy *policy)
  * cpufreq_set_policy - Modify cpufreq policy parameters.
  * @policy: Policy object to modify.
  * @new_gov: Policy governor pointer.
- * @new_pol: Policy value (for drivers with built-in governors).
  *
  * Invoke the cpufreq driver's ->verify() callback to sanity-check the frequency
  * limits to be set for the policy, update @policy with the verified limits
@@ -2622,8 +2622,7 @@ static void cpufreq_update_pressure(struct cpufreq_policy *policy)
  * The cpuinfo part of @policy is not updated by this function.
  */
 static int cpufreq_set_policy(struct cpufreq_policy *policy,
-			      struct cpufreq_governor *new_gov,
-			      unsigned int new_pol)
+			      struct cpufreq_governor *new_gov)
 {
 	struct cpufreq_policy_data new_data;
 	struct cpufreq_governor *old_gov;
@@ -2676,7 +2675,6 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
 		 policy->min, policy->max);
 
 	if (cpufreq_driver->setpolicy) {
-		policy->policy = new_pol;
 		pr_debug("setting range\n");
 		return cpufreq_driver->setpolicy(policy);
 	}
-- 
2.25.1

Re: [PATCH 2/2] cpufreq: simplify cpufreq_set_policy() interface

[Viresh Kumar]

On 19-08-25, 18:39, Zihuan Zhang wrote:
>  static int cpufreq_set_policy(struct cpufreq_policy *policy,
> -			      struct cpufreq_governor *new_gov,
> -			      unsigned int new_pol);
> +			      struct cpufreq_governor *new_gov);

A driver will either support the policy or the governor. If we are
keeping `new_gov` around, I don't see why `new_pol` should be dropped.
And changing the policy for a `setpolicy` driver should happen from
within cpufreq_set_policy() instead of the caller. Also there is at
least one case (verify()) where we may end up returning early, before
changing the policy.

-- 
viresh

Re: [PATCH 2/2] cpufreq: simplify cpufreq_set_policy() interface

[Zihuan Zhang]

在 2025/8/19 18:59, Viresh Kumar 写道:
> On 19-08-25, 18:39, Zihuan Zhang wrote:
>>   static int cpufreq_set_policy(struct cpufreq_policy *policy,
>> -			      struct cpufreq_governor *new_gov,
>> -			      unsigned int new_pol);
>> +			      struct cpufreq_governor *new_gov);
> A driver will either support the policy or the governor. If we are
> keeping `new_gov` around, I don't see why `new_pol` should be dropped.

Thanks for the reminder.

If we remove new_pol, then new_gov should indeed be removed as well.

> And changing the policy for a `setpolicy` driver should happen from
> within cpufreq_set_policy() instead of the caller. Also there is at
> least one case (verify()) where we may end up returning early, before
> changing the policy.
>
You’re right, we didn’t really consider that case before.

The interface of cpufreq_set_policy() does look a bit odd:

- drivers using governors don’t really need the new_pol parameter

- while drivers using the setpolicy method don’t need the new_gov one.


I guess this might be due to some historical reasons.

The question is whether it’s worth modifying this function, or if we 
should just keep it as it is.

Re: [PATCH 2/2] cpufreq: simplify cpufreq_set_policy() interface

[Viresh Kumar]

On 20-08-25, 13:42, Zihuan Zhang wrote:
> You’re right, we didn’t really consider that case before.
> 
> The interface of cpufreq_set_policy() does look a bit odd:
> 
> - drivers using governors don’t really need the new_pol parameter
> 
> - while drivers using the setpolicy method don’t need the new_gov one.

Right, one argument is _always_ unused. If we could have handled that via a
single argument, it would have been nice.

> I guess this might be due to some historical reasons.
> 
> The question is whether it’s worth modifying this function, or if we should
> just keep it as it is.

Unless there is a better way :)

-- 
viresh

Re: [PATCH 2/2] cpufreq: simplify cpufreq_set_policy() interface

[Zihuan Zhang]

在 2025/8/20 13:47, Viresh Kumar 写道:
> On 20-08-25, 13:42, Zihuan Zhang wrote:
>> You’re right, we didn’t really consider that case before.
>>
>> The interface of cpufreq_set_policy() does look a bit odd:
>>
>> - drivers using governors don’t really need the new_pol parameter
>>
>> - while drivers using the setpolicy method don’t need the new_gov one.
> Right, one argument is _always_ unused. If we could have handled that via a
> single argument, it would have been nice.
Sounds great.
>> I guess this might be due to some historical reasons.
>>
>> The question is whether it’s worth modifying this function, or if we should
>> just keep it as it is.
> Unless there is a better way :)

If we change cpufreq_set_policy() to only take one parameter, then we 
would need to add extra fields into struct cpufreq_policy to hold 
temporary values,

in order to avoid the issue where verify may return early and leave the 
policy in a partially updated state.


So maybe it is better not to modify it.
>