[PATCH v1 3/3] cpufreq: Make cpufreq_frequency_table_verify() internal

Zihuan Zhang posted 3 patches 4 weeks, 1 day ago
[PATCH v1 3/3] cpufreq: Make cpufreq_frequency_table_verify() internal
Posted by Zihuan Zhang 4 weeks, 1 day ago
The helper cpufreq_frequency_table_verify() was previously exported and used
directly by a few cpufreq drivers. With the previous change ensuring that
cpufreq_generic_frequency_table_verify() always calls
cpufreq_verify_within_cpu_limits(), drivers no longer need to call
cpufreq_frequency_table_verify() explicitly.

Update the affected drivers (sh-cpufreq and virtual-cpufreq) to use
cpufreq_generic_frequency_table_verify() instead, and convert
cpufreq_frequency_table_verify() to a private static function inside
freq_table.c.

This reduces the exported cpufreq API surface and enforces a single, consistent
entry point (cpufreq_generic_frequency_table_verify()) for drivers.

No functional changes intended.

Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
---
 drivers/cpufreq/freq_table.c      | 5 +----
 drivers/cpufreq/sh-cpufreq.c      | 6 ++----
 drivers/cpufreq/virtual-cpufreq.c | 5 +----
 include/linux/cpufreq.h           | 2 --
 4 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index f4b05dcc479b..79fa65aa3859 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -64,7 +64,7 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy)
 		return 0;
 }
 
-int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy)
+static int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy)
 {
 	struct cpufreq_frequency_table *pos, *table = policy->freq_table;
 	unsigned int freq, prev_smaller = 0;
@@ -73,8 +73,6 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy)
 	pr_debug("request for verification of policy (%u - %u kHz) for cpu %u\n",
 					policy->min, policy->max, policy->cpu);
 
-	cpufreq_verify_within_cpu_limits(policy);
-
 	cpufreq_for_each_valid_entry(pos, table) {
 		freq = pos->frequency;
 
@@ -97,7 +95,6 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(cpufreq_frequency_table_verify);
 
 /*
  * Generic routine to verify policy & frequency table, requires driver to set
diff --git a/drivers/cpufreq/sh-cpufreq.c b/drivers/cpufreq/sh-cpufreq.c
index 642ddb9ea217..ee3fd1e71b90 100644
--- a/drivers/cpufreq/sh-cpufreq.c
+++ b/drivers/cpufreq/sh-cpufreq.c
@@ -90,10 +90,8 @@ static int sh_cpufreq_verify(struct cpufreq_policy_data *policy)
 {
 	struct clk *cpuclk = &per_cpu(sh_cpuclk, policy->cpu);
 
-	if (policy->freq_table)
-		return cpufreq_frequency_table_verify(policy);
-
-	cpufreq_verify_within_cpu_limits(policy);
+	if (!cpufreq_generic_frequency_table_verify(policy))
+		return 0;
 
 	policy->min = (clk_round_rate(cpuclk, 1) + 500) / 1000;
 	policy->max = (clk_round_rate(cpuclk, ~0UL) + 500) / 1000;
diff --git a/drivers/cpufreq/virtual-cpufreq.c b/drivers/cpufreq/virtual-cpufreq.c
index 6ffa16d239b2..2498f40cd57e 100644
--- a/drivers/cpufreq/virtual-cpufreq.c
+++ b/drivers/cpufreq/virtual-cpufreq.c
@@ -249,10 +249,7 @@ static int virt_cpufreq_offline(struct cpufreq_policy *policy)
 
 static int virt_cpufreq_verify_policy(struct cpufreq_policy_data *policy)
 {
-	if (policy->freq_table)
-		return cpufreq_frequency_table_verify(policy);
-
-	cpufreq_verify_within_cpu_limits(policy);
+	cpufreq_generic_frequency_table_verify(policy);
 	return 0;
 }
 
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 40966512ea18..577f1cd723a0 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -782,8 +782,6 @@ struct cpufreq_frequency_table {
 
 int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy);
 
-int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy);
-
 int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy);
 
 int cpufreq_table_index_unsorted(struct cpufreq_policy *policy,
-- 
2.25.1
Re: [PATCH v1 3/3] cpufreq: Make cpufreq_frequency_table_verify() internal
Posted by Viresh Kumar 4 weeks, 1 day ago
On 04-09-25, 11:22, Zihuan Zhang wrote:
> diff --git a/drivers/cpufreq/sh-cpufreq.c b/drivers/cpufreq/sh-cpufreq.c
> index 642ddb9ea217..ee3fd1e71b90 100644
> --- a/drivers/cpufreq/sh-cpufreq.c
> +++ b/drivers/cpufreq/sh-cpufreq.c
> @@ -90,10 +90,8 @@ static int sh_cpufreq_verify(struct cpufreq_policy_data *policy)
>  {
>  	struct clk *cpuclk = &per_cpu(sh_cpuclk, policy->cpu);
>  
> -	if (policy->freq_table)
> -		return cpufreq_frequency_table_verify(policy);
> -
> -	cpufreq_verify_within_cpu_limits(policy);
> +	if (!cpufreq_generic_frequency_table_verify(policy))
> +		return 0;
>  
>  	policy->min = (clk_round_rate(cpuclk, 1) + 500) / 1000;
>  	policy->max = (clk_round_rate(cpuclk, ~0UL) + 500) / 1000;
> diff --git a/drivers/cpufreq/virtual-cpufreq.c b/drivers/cpufreq/virtual-cpufreq.c
> index 6ffa16d239b2..2498f40cd57e 100644
> --- a/drivers/cpufreq/virtual-cpufreq.c
> +++ b/drivers/cpufreq/virtual-cpufreq.c
> @@ -249,10 +249,7 @@ static int virt_cpufreq_offline(struct cpufreq_policy *policy)
>  
>  static int virt_cpufreq_verify_policy(struct cpufreq_policy_data *policy)
>  {
> -	if (policy->freq_table)
> -		return cpufreq_frequency_table_verify(policy);
> -
> -	cpufreq_verify_within_cpu_limits(policy);
> +	cpufreq_generic_frequency_table_verify(policy);
>  	return 0;
>  }

You ended up changing the logic of both these files and it isn't the
same anymore. Earlier if freq_table was there and
cpufreq_frequency_table_verify() failed, we used to return, but not
anymore.

And we don't return the error anymore for virtual driver.

-- 
viresh
Re: [PATCH v1 3/3] cpufreq: Make cpufreq_frequency_table_verify() internal
Posted by Zihuan Zhang 4 weeks, 1 day ago
在 2025/9/4 12:45, Viresh Kumar 写道:
> On 04-09-25, 11:22, Zihuan Zhang wrote:
>> diff --git a/drivers/cpufreq/sh-cpufreq.c b/drivers/cpufreq/sh-cpufreq.c
>> index 642ddb9ea217..ee3fd1e71b90 100644
>> --- a/drivers/cpufreq/sh-cpufreq.c
>> +++ b/drivers/cpufreq/sh-cpufreq.c
>> @@ -90,10 +90,8 @@ static int sh_cpufreq_verify(struct cpufreq_policy_data *policy)
>>   {
>>   	struct clk *cpuclk = &per_cpu(sh_cpuclk, policy->cpu);
>>   
>> -	if (policy->freq_table)
>> -		return cpufreq_frequency_table_verify(policy);
>> -
>> -	cpufreq_verify_within_cpu_limits(policy);
>> +	if (!cpufreq_generic_frequency_table_verify(policy))
>> +		return 0;
>>   
>>   	policy->min = (clk_round_rate(cpuclk, 1) + 500) / 1000;
>>   	policy->max = (clk_round_rate(cpuclk, ~0UL) + 500) / 1000;
>> diff --git a/drivers/cpufreq/virtual-cpufreq.c b/drivers/cpufreq/virtual-cpufreq.c
>> index 6ffa16d239b2..2498f40cd57e 100644
>> --- a/drivers/cpufreq/virtual-cpufreq.c
>> +++ b/drivers/cpufreq/virtual-cpufreq.c
>> @@ -249,10 +249,7 @@ static int virt_cpufreq_offline(struct cpufreq_policy *policy)
>>   
>>   static int virt_cpufreq_verify_policy(struct cpufreq_policy_data *policy)
>>   {
>> -	if (policy->freq_table)
>> -		return cpufreq_frequency_table_verify(policy);
>> -
>> -	cpufreq_verify_within_cpu_limits(policy);
>> +	cpufreq_generic_frequency_table_verify(policy);
>>   	return 0;
>>   }
> You ended up changing the logic of both these files and it isn't the
> same anymore. Earlier if freq_table was there and
> cpufreq_frequency_table_verify() failed, we used to return, but not
> anymore.
>
> And we don't return the error anymore for virtual driver.


Thanks for pointing this out, Viresh.

You are correct — with the current changes, if
cpufreq_generic_frequency_table_verify() fails, we no longer return
an error in these drivers. For drivers that lack a frequency table,
they can still operate, which is why I wasn’t sure whether returning
an error is strictly necessary.

I would appreciate your advice here: should we preserve the old
behavior and return an error on failure, or is it acceptable for
drivers without a table to continue without it?


>
Re: [PATCH v1 3/3] cpufreq: Make cpufreq_frequency_table_verify() internal
Posted by Viresh Kumar 3 weeks, 3 days ago
On 04-09-25, 13:34, Zihuan Zhang wrote:
> Thanks for pointing this out, Viresh.
> 
> You are correct — with the current changes, if
> cpufreq_generic_frequency_table_verify() fails, we no longer return
> an error in these drivers. For drivers that lack a frequency table,
> they can still operate, which is why I wasn’t sure whether returning
> an error is strictly necessary.
> 
> I would appreciate your advice here: should we preserve the old
> behavior and return an error on failure, or is it acceptable for
> drivers without a table to continue without it?

I looked at the code again, and it feels like the current code is
doing the right thing right now and making the suggested changes will
only make it less readable. The two function have different purpose
and it looks better if the callers call them explicitly.

So, I would suggest dropping patches 2 and 3.

-- 
viresh