[PATCH v1 0/3] cpufreq: Clean up frequency table handling

[PATCH v1 0/3] cpufreq: Clean up frequency table handling

[Zihuan Zhang]

This series contains a set of cleanups around cpufreq frequency table
handling.

The first patch drops the redundant @freq_table parameter from
cpufreq_frequency_table_verify(). This has already been Acked, but is
included here to avoid build issues with the following changes.

The second patch makes sure that policy limits are always enforced
even when a driver does not provide a frequency table. This removes
the implicit dependency on the table and makes the enforcement logic
consistent across drivers.

The third patch makes cpufreq_frequency_table_verify() internal,
since no external user should call it directly after the parameter
removal and the refactoring.

Together these patches simplify the API surface, reduce redundancy,
and make policy enforcement more robust.

Zihuan Zhang (3):
  cpufreq: Drop redundant freq_table parameter
  cpufreq: Always enforce policy limits even without frequency table
  cpufreq: Make cpufreq_frequency_table_verify() internal

 drivers/cpufreq/cpufreq.c         |  2 +-
 drivers/cpufreq/freq_table.c      | 18 +++++++-----------
 drivers/cpufreq/sh-cpufreq.c      |  8 ++------
 drivers/cpufreq/virtual-cpufreq.c |  5 +----
 include/linux/cpufreq.h           |  5 +----
 5 files changed, 12 insertions(+), 26 deletions(-)

-- 
2.25.1

[PATCH v1 1/3] cpufreq: Drop redundant freq_table parameter

[Zihuan Zhang]

Since commit e0b3165ba521 ("cpufreq: add 'freq_table' in struct
cpufreq_policy"),
freq_table has been stored in struct cpufreq_policy instead of being
maintained separately.

However, several helpers in freq_table.c still take both policy and
freq_table as parameters, even though policy->freq_table can always be
used. This leads to redundant function arguments and increases the chance
of inconsistencies.

This patch removes the unnecessary freq_table argument from these functions
and updates their callers to only pass policy. This makes the code simpler,
more consistent, and avoids duplication.

V2:
 - Merge three patches into one to fix compile error
 - simple the check suggested by Viresh Kumar

Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c         |  2 +-
 drivers/cpufreq/freq_table.c      | 14 ++++++--------
 drivers/cpufreq/sh-cpufreq.c      |  6 ++----
 drivers/cpufreq/virtual-cpufreq.c |  2 +-
 include/linux/cpufreq.h           |  7 +++----
 5 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index a615c98d80ca..5fcc99f768d2 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2793,7 +2793,7 @@ int cpufreq_boost_set_sw(struct cpufreq_policy *policy, int state)
 	if (!policy->freq_table)
 		return -ENXIO;
 
-	ret = cpufreq_frequency_table_cpuinfo(policy, policy->freq_table);
+	ret = cpufreq_frequency_table_cpuinfo(policy);
 	if (ret) {
 		pr_err("%s: Policy frequency update failed\n", __func__);
 		return ret;
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index 35de513af6c9..d5111ee56e38 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -28,10 +28,9 @@ static bool policy_has_boost_freq(struct cpufreq_policy *policy)
 	return false;
 }
 
-int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
-				    struct cpufreq_frequency_table *table)
+int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy)
 {
-	struct cpufreq_frequency_table *pos;
+	struct cpufreq_frequency_table *pos, *table = policy->freq_table;
 	unsigned int min_freq = ~0;
 	unsigned int max_freq = 0;
 	unsigned int freq;
@@ -65,10 +64,9 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
 		return 0;
 }
 
-int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy,
-				   struct cpufreq_frequency_table *table)
+int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy)
 {
-	struct cpufreq_frequency_table *pos;
+	struct cpufreq_frequency_table *pos, *table = policy->freq_table;
 	unsigned int freq, prev_smaller = 0;
 	bool found = false;
 
@@ -110,7 +108,7 @@ int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy)
 	if (!policy->freq_table)
 		return -ENODEV;
 
-	return cpufreq_frequency_table_verify(policy, policy->freq_table);
+	return cpufreq_frequency_table_verify(policy);
 }
 EXPORT_SYMBOL_GPL(cpufreq_generic_frequency_table_verify);
 
@@ -354,7 +352,7 @@ int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy)
 		return 0;
 	}
 
-	ret = cpufreq_frequency_table_cpuinfo(policy, policy->freq_table);
+	ret = cpufreq_frequency_table_cpuinfo(policy);
 	if (ret)
 		return ret;
 
diff --git a/drivers/cpufreq/sh-cpufreq.c b/drivers/cpufreq/sh-cpufreq.c
index 9c0b01e00508..642ddb9ea217 100644
--- a/drivers/cpufreq/sh-cpufreq.c
+++ b/drivers/cpufreq/sh-cpufreq.c
@@ -89,11 +89,9 @@ static int sh_cpufreq_target(struct cpufreq_policy *policy,
 static int sh_cpufreq_verify(struct cpufreq_policy_data *policy)
 {
 	struct clk *cpuclk = &per_cpu(sh_cpuclk, policy->cpu);
-	struct cpufreq_frequency_table *freq_table;
 
-	freq_table = cpuclk->nr_freqs ? cpuclk->freq_table : NULL;
-	if (freq_table)
-		return cpufreq_frequency_table_verify(policy, freq_table);
+	if (policy->freq_table)
+		return cpufreq_frequency_table_verify(policy);
 
 	cpufreq_verify_within_cpu_limits(policy);
 
diff --git a/drivers/cpufreq/virtual-cpufreq.c b/drivers/cpufreq/virtual-cpufreq.c
index 7dd1b0c263c7..6ffa16d239b2 100644
--- a/drivers/cpufreq/virtual-cpufreq.c
+++ b/drivers/cpufreq/virtual-cpufreq.c
@@ -250,7 +250,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, policy->freq_table);
+		return cpufreq_frequency_table_verify(policy);
 
 	cpufreq_verify_within_cpu_limits(policy);
 	return 0;
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 95f3807c8c55..40966512ea18 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -780,11 +780,10 @@ struct cpufreq_frequency_table {
 		else
 
 
-int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
-				    struct cpufreq_frequency_table *table);
+int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy);
+
+int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy);
 
-int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy,
-				   struct cpufreq_frequency_table *table);
 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 1/3] cpufreq: Drop redundant freq_table parameter

[Rafael J. Wysocki]

On Thu, Sep 4, 2025 at 5:22 AM Zihuan Zhang <zhangzihuan@kylinos.cn> wrote:
>
> Since commit e0b3165ba521 ("cpufreq: add 'freq_table' in struct
> cpufreq_policy"),
> freq_table has been stored in struct cpufreq_policy instead of being
> maintained separately.
>
> However, several helpers in freq_table.c still take both policy and
> freq_table as parameters, even though policy->freq_table can always be
> used. This leads to redundant function arguments and increases the chance
> of inconsistencies.
>
> This patch removes the unnecessary freq_table argument from these functions
> and updates their callers to only pass policy. This makes the code simpler,
> more consistent, and avoids duplication.
>
> V2:
>  - Merge three patches into one to fix compile error
>  - simple the check suggested by Viresh Kumar
>
> Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Do I think correctly that this is a resend of
https://lore.kernel.org/all/20250902073323.48330-1-zhangzihuan@kylinos.cn/
?

There's no need to resend it.

If you want to make new changes on top of it, just say in their
changelogs that they depend on it.

OTOH, if the new patch is not a resend, you should have listed the
differences between it and the old one.

> ---
>  drivers/cpufreq/cpufreq.c         |  2 +-
>  drivers/cpufreq/freq_table.c      | 14 ++++++--------
>  drivers/cpufreq/sh-cpufreq.c      |  6 ++----
>  drivers/cpufreq/virtual-cpufreq.c |  2 +-
>  include/linux/cpufreq.h           |  7 +++----
>  5 files changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index a615c98d80ca..5fcc99f768d2 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -2793,7 +2793,7 @@ int cpufreq_boost_set_sw(struct cpufreq_policy *policy, int state)
>         if (!policy->freq_table)
>                 return -ENXIO;
>
> -       ret = cpufreq_frequency_table_cpuinfo(policy, policy->freq_table);
> +       ret = cpufreq_frequency_table_cpuinfo(policy);
>         if (ret) {
>                 pr_err("%s: Policy frequency update failed\n", __func__);
>                 return ret;
> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
> index 35de513af6c9..d5111ee56e38 100644
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -28,10 +28,9 @@ static bool policy_has_boost_freq(struct cpufreq_policy *policy)
>         return false;
>  }
>
> -int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
> -                                   struct cpufreq_frequency_table *table)
> +int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy)
>  {
> -       struct cpufreq_frequency_table *pos;
> +       struct cpufreq_frequency_table *pos, *table = policy->freq_table;
>         unsigned int min_freq = ~0;
>         unsigned int max_freq = 0;
>         unsigned int freq;
> @@ -65,10 +64,9 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
>                 return 0;
>  }
>
> -int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy,
> -                                  struct cpufreq_frequency_table *table)
> +int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy)
>  {
> -       struct cpufreq_frequency_table *pos;
> +       struct cpufreq_frequency_table *pos, *table = policy->freq_table;
>         unsigned int freq, prev_smaller = 0;
>         bool found = false;
>
> @@ -110,7 +108,7 @@ int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy)
>         if (!policy->freq_table)
>                 return -ENODEV;
>
> -       return cpufreq_frequency_table_verify(policy, policy->freq_table);
> +       return cpufreq_frequency_table_verify(policy);
>  }
>  EXPORT_SYMBOL_GPL(cpufreq_generic_frequency_table_verify);
>
> @@ -354,7 +352,7 @@ int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy)
>                 return 0;
>         }
>
> -       ret = cpufreq_frequency_table_cpuinfo(policy, policy->freq_table);
> +       ret = cpufreq_frequency_table_cpuinfo(policy);
>         if (ret)
>                 return ret;
>
> diff --git a/drivers/cpufreq/sh-cpufreq.c b/drivers/cpufreq/sh-cpufreq.c
> index 9c0b01e00508..642ddb9ea217 100644
> --- a/drivers/cpufreq/sh-cpufreq.c
> +++ b/drivers/cpufreq/sh-cpufreq.c
> @@ -89,11 +89,9 @@ static int sh_cpufreq_target(struct cpufreq_policy *policy,
>  static int sh_cpufreq_verify(struct cpufreq_policy_data *policy)
>  {
>         struct clk *cpuclk = &per_cpu(sh_cpuclk, policy->cpu);
> -       struct cpufreq_frequency_table *freq_table;
>
> -       freq_table = cpuclk->nr_freqs ? cpuclk->freq_table : NULL;
> -       if (freq_table)
> -               return cpufreq_frequency_table_verify(policy, freq_table);
> +       if (policy->freq_table)
> +               return cpufreq_frequency_table_verify(policy);
>
>         cpufreq_verify_within_cpu_limits(policy);
>
> diff --git a/drivers/cpufreq/virtual-cpufreq.c b/drivers/cpufreq/virtual-cpufreq.c
> index 7dd1b0c263c7..6ffa16d239b2 100644
> --- a/drivers/cpufreq/virtual-cpufreq.c
> +++ b/drivers/cpufreq/virtual-cpufreq.c
> @@ -250,7 +250,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, policy->freq_table);
> +               return cpufreq_frequency_table_verify(policy);
>
>         cpufreq_verify_within_cpu_limits(policy);
>         return 0;
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 95f3807c8c55..40966512ea18 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -780,11 +780,10 @@ struct cpufreq_frequency_table {
>                 else
>
>
> -int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
> -                                   struct cpufreq_frequency_table *table);
> +int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy);
> +
> +int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy);
>
> -int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy,
> -                                  struct cpufreq_frequency_table *table);
>  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 1/3] cpufreq: Drop redundant freq_table parameter

[Zihuan Zhang]

在 2025/9/4 21:31, Rafael J. Wysocki 写道:
> On Thu, Sep 4, 2025 at 5:22 AM Zihuan Zhang <zhangzihuan@kylinos.cn> wrote:
>> Since commit e0b3165ba521 ("cpufreq: add 'freq_table' in struct
>> cpufreq_policy"),
>> freq_table has been stored in struct cpufreq_policy instead of being
>> maintained separately.
>>
>> However, several helpers in freq_table.c still take both policy and
>> freq_table as parameters, even though policy->freq_table can always be
>> used. This leads to redundant function arguments and increases the chance
>> of inconsistencies.
>>
>> This patch removes the unnecessary freq_table argument from these functions
>> and updates their callers to only pass policy. This makes the code simpler,
>> more consistent, and avoids duplication.
>>
>> V2:
>>   - Merge three patches into one to fix compile error
>>   - simple the check suggested by Viresh Kumar
>>
>> Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Do I think correctly that this is a resend of
> https://lore.kernel.org/all/20250902073323.48330-1-zhangzihuan@kylinos.cn/
> ?
>
> There's no need to resend it.


Yes, this is indeed a resend.  Sorry for the noise.

> If you want to make new changes on top of it, just say in their
> changelogs that they depend on it.
>
> OTOH, if the new patch is not a resend, you should have listed the
> differences between it and the old one.
>
>> ---
>>   drivers/cpufreq/cpufreq.c         |  2 +-
>>   drivers/cpufreq/freq_table.c      | 14 ++++++--------
>>   drivers/cpufreq/sh-cpufreq.c      |  6 ++----
>>   drivers/cpufreq/virtual-cpufreq.c |  2 +-
>>   include/linux/cpufreq.h           |  7 +++----
>>   5 files changed, 13 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index a615c98d80ca..5fcc99f768d2 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -2793,7 +2793,7 @@ int cpufreq_boost_set_sw(struct cpufreq_policy *policy, int state)
>>          if (!policy->freq_table)
>>                  return -ENXIO;
>>
>> -       ret = cpufreq_frequency_table_cpuinfo(policy, policy->freq_table);
>> +       ret = cpufreq_frequency_table_cpuinfo(policy);
>>          if (ret) {
>>                  pr_err("%s: Policy frequency update failed\n", __func__);
>>                  return ret;
>> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
>> index 35de513af6c9..d5111ee56e38 100644
>> --- a/drivers/cpufreq/freq_table.c
>> +++ b/drivers/cpufreq/freq_table.c
>> @@ -28,10 +28,9 @@ static bool policy_has_boost_freq(struct cpufreq_policy *policy)
>>          return false;
>>   }
>>
>> -int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
>> -                                   struct cpufreq_frequency_table *table)
>> +int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy)
>>   {
>> -       struct cpufreq_frequency_table *pos;
>> +       struct cpufreq_frequency_table *pos, *table = policy->freq_table;
>>          unsigned int min_freq = ~0;
>>          unsigned int max_freq = 0;
>>          unsigned int freq;
>> @@ -65,10 +64,9 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
>>                  return 0;
>>   }
>>
>> -int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy,
>> -                                  struct cpufreq_frequency_table *table)
>> +int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy)
>>   {
>> -       struct cpufreq_frequency_table *pos;
>> +       struct cpufreq_frequency_table *pos, *table = policy->freq_table;
>>          unsigned int freq, prev_smaller = 0;
>>          bool found = false;
>>
>> @@ -110,7 +108,7 @@ int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy)
>>          if (!policy->freq_table)
>>                  return -ENODEV;
>>
>> -       return cpufreq_frequency_table_verify(policy, policy->freq_table);
>> +       return cpufreq_frequency_table_verify(policy);
>>   }
>>   EXPORT_SYMBOL_GPL(cpufreq_generic_frequency_table_verify);
>>
>> @@ -354,7 +352,7 @@ int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy)
>>                  return 0;
>>          }
>>
>> -       ret = cpufreq_frequency_table_cpuinfo(policy, policy->freq_table);
>> +       ret = cpufreq_frequency_table_cpuinfo(policy);
>>          if (ret)
>>                  return ret;
>>
>> diff --git a/drivers/cpufreq/sh-cpufreq.c b/drivers/cpufreq/sh-cpufreq.c
>> index 9c0b01e00508..642ddb9ea217 100644
>> --- a/drivers/cpufreq/sh-cpufreq.c
>> +++ b/drivers/cpufreq/sh-cpufreq.c
>> @@ -89,11 +89,9 @@ static int sh_cpufreq_target(struct cpufreq_policy *policy,
>>   static int sh_cpufreq_verify(struct cpufreq_policy_data *policy)
>>   {
>>          struct clk *cpuclk = &per_cpu(sh_cpuclk, policy->cpu);
>> -       struct cpufreq_frequency_table *freq_table;
>>
>> -       freq_table = cpuclk->nr_freqs ? cpuclk->freq_table : NULL;
>> -       if (freq_table)
>> -               return cpufreq_frequency_table_verify(policy, freq_table);
>> +       if (policy->freq_table)
>> +               return cpufreq_frequency_table_verify(policy);
>>
>>          cpufreq_verify_within_cpu_limits(policy);
>>
>> diff --git a/drivers/cpufreq/virtual-cpufreq.c b/drivers/cpufreq/virtual-cpufreq.c
>> index 7dd1b0c263c7..6ffa16d239b2 100644
>> --- a/drivers/cpufreq/virtual-cpufreq.c
>> +++ b/drivers/cpufreq/virtual-cpufreq.c
>> @@ -250,7 +250,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, policy->freq_table);
>> +               return cpufreq_frequency_table_verify(policy);
>>
>>          cpufreq_verify_within_cpu_limits(policy);
>>          return 0;
>> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
>> index 95f3807c8c55..40966512ea18 100644
>> --- a/include/linux/cpufreq.h
>> +++ b/include/linux/cpufreq.h
>> @@ -780,11 +780,10 @@ struct cpufreq_frequency_table {
>>                  else
>>
>>
>> -int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
>> -                                   struct cpufreq_frequency_table *table);
>> +int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy);
>> +
>> +int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy);
>>
>> -int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy,
>> -                                  struct cpufreq_frequency_table *table);
>>   int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy);
>>
>>   int cpufreq_table_index_unsorted(struct cpufreq_policy *policy,
>> --
>> 2.25.1
>>

[PATCH v1 2/3] cpufreq: Always enforce policy limits even without frequency table

[Zihuan Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=yes, Size: 1348 bytes --]

Currently, cpufreq_frequency_table_verify() simply returns when the driver’s
frequency table is missing (policy->freq_table == NULL).
This means that cpufreq_verify_within_cpu_limits() is not invoked in such
cases, leaving policy->min and policy->max unchecked.

Some cpufreq drivers handle this manually by calling
cpufreq_verify_within_cpu_limits() even when no frequency table is present,
in order to ensure the policy stays within CPU limits.

To avoid this inconsistency and potential misuse, make
cpufreq_generic_frequency_table_verify() always call
cpufreq_verify_within_cpu_limits(), regardless of whether policy->freq_table
is available. This unifies the behavior across all drivers and makes the helper
safe to use universally.

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

diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index d5111ee56e38..f4b05dcc479b 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -105,6 +105,7 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_verify);
  */
 int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy)
 {
+	cpufreq_verify_within_cpu_limits(policy);
 	if (!policy->freq_table)
 		return -ENODEV;
 
-- 
2.25.1

Re: [PATCH v1 2/3] cpufreq: Always enforce policy limits even without frequency table

[Viresh Kumar]

On 04-09-25, 11:22, Zihuan Zhang wrote:
> Currently, cpufreq_frequency_table_verify() simply returns when the driver’s
> frequency table is missing (policy->freq_table == NULL).
> This means that cpufreq_verify_within_cpu_limits() is not invoked in such
> cases, leaving policy->min and policy->max unchecked.
> 
> Some cpufreq drivers handle this manually by calling
> cpufreq_verify_within_cpu_limits() even when no frequency table is present,
> in order to ensure the policy stays within CPU limits.
> 
> To avoid this inconsistency and potential misuse, make
> cpufreq_generic_frequency_table_verify() always call
> cpufreq_verify_within_cpu_limits(), regardless of whether policy->freq_table
> is available. This unifies the behavior across all drivers and makes the helper
> safe to use universally.
> 
> Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
> ---
>  drivers/cpufreq/freq_table.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
> index d5111ee56e38..f4b05dcc479b 100644
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -105,6 +105,7 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_verify);
>   */
>  int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy)
>  {
> +	cpufreq_verify_within_cpu_limits(policy);

So if we have a freq-table, we will call this twice now. Why make it
bad for the existing users ?

And then the name of this function, it is all about freq-table. If it
isn't there, not sure we should call it at all.

>  	if (!policy->freq_table)
>  		return -ENODEV;
>  
> -- 
> 2.25.1

-- 
viresh

Re: [PATCH v1 2/3] cpufreq: Always enforce policy limits even without frequency table

[Zihuan Zhang]

在 2025/9/4 12:48, Viresh Kumar 写道:
> On 04-09-25, 11:22, Zihuan Zhang wrote:
>> Currently, cpufreq_frequency_table_verify() simply returns when the driver’s
>> frequency table is missing (policy->freq_table == NULL).
>> This means that cpufreq_verify_within_cpu_limits() is not invoked in such
>> cases, leaving policy->min and policy->max unchecked.
>>
>> Some cpufreq drivers handle this manually by calling
>> cpufreq_verify_within_cpu_limits() even when no frequency table is present,
>> in order to ensure the policy stays within CPU limits.
>>
>> To avoid this inconsistency and potential misuse, make
>> cpufreq_generic_frequency_table_verify() always call
>> cpufreq_verify_within_cpu_limits(), regardless of whether policy->freq_table
>> is available. This unifies the behavior across all drivers and makes the helper
>> safe to use universally.
>>
>> Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
>> ---
>>   drivers/cpufreq/freq_table.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
>> index d5111ee56e38..f4b05dcc479b 100644
>> --- a/drivers/cpufreq/freq_table.c
>> +++ b/drivers/cpufreq/freq_table.c
>> @@ -105,6 +105,7 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_verify);
>>    */
>>   int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy)
>>   {
>> +	cpufreq_verify_within_cpu_limits(policy);
> So if we have a freq-table, we will call this twice now. Why make it
> bad for the existing users ?


Just to clarify, in the third patch of this series we remove
cpufreq_generic_frequency_table_verify() from the table_verify path,
so cpufreq_verify_within_cpu_limits() is now only called here. There
won’t be any duplicate invocation for drivers that already have a
frequency table.

This also resolves the semantic concern, since the helper is no
longer invoked outside of this context.

Thanks!


> And then the name of this function, it is all about freq-table. If it
> isn't there, not sure we should call it at all.


As it stands, only drivers that have a frequency table actually call
cpufreq_generic_frequency_table_verify(). Drivers without a table
implement their own verification logic. So in practice, this helper
is still only used in the context of a frequency table, keeping the
semantics consistent with its name.

>>   	if (!policy->freq_table)
>>   		return -ENODEV;
>>   
>> -- 
>> 2.25.1

Re: [PATCH v1 2/3] cpufreq: Always enforce policy limits even without frequency table

[Viresh Kumar]

On 04-09-25, 13:23, Zihuan Zhang wrote:
> 在 2025/9/4 12:48, Viresh Kumar 写道:
> > On 04-09-25, 11:22, Zihuan Zhang wrote:
> > >   int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy)
> > >   {
> > > +	cpufreq_verify_within_cpu_limits(policy);
> > So if we have a freq-table, we will call this twice now. Why make it
> > bad for the existing users ?
> 
> 
> Just to clarify, in the third patch of this series we remove
> cpufreq_generic_frequency_table_verify() from the table_verify path,
> so cpufreq_verify_within_cpu_limits() is now only called here. There
> won’t be any duplicate invocation for drivers that already have a
> frequency table.

Maybe I wasn't clear enough.

int cpufreq_frequency_table_verify(...)
{
	cpufreq_verify_within_cpu_limits(...);
        ...
}

int cpufreq_generic_frequency_table_verify(...)
{
        cpufreq_verify_within_cpu_limits(...);
        cpufreq_frequency_table_verify(...);
        ...
}

For a driver with a valid freq-table, we will call
cpufreq_verify_within_cpu_limits() unnecessarily, isn't it ?

-- 
viresh

Re: [PATCH v1 2/3] cpufreq: Always enforce policy limits even without frequency table

[Zihuan Zhang]

在 2025/9/4 13:37, Viresh Kumar 写道:
> On 04-09-25, 13:23, Zihuan Zhang wrote:
>> 在 2025/9/4 12:48, Viresh Kumar 写道:
>>> On 04-09-25, 11:22, Zihuan Zhang wrote:
>>>>    int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy)
>>>>    {
>>>> +	cpufreq_verify_within_cpu_limits(policy);
>>> So if we have a freq-table, we will call this twice now. Why make it
>>> bad for the existing users ?
>>
>> Just to clarify, in the third patch of this series we remove
>> cpufreq_generic_frequency_table_verify() from the table_verify path,
>> so cpufreq_verify_within_cpu_limits() is now only called here. There
>> won’t be any duplicate invocation for drivers that already have a
>> frequency table.
> Maybe I wasn't clear enough.
>
> int cpufreq_frequency_table_verify(...)
> {
> 	cpufreq_verify_within_cpu_limits(...);
>          ...
> }
>
> int cpufreq_generic_frequency_table_verify(...)
> {
>          cpufreq_verify_within_cpu_limits(...);
>          cpufreq_frequency_table_verify(...);
>          ...
> }
>
> For a driver with a valid freq-table, we will call
> cpufreq_verify_within_cpu_limits() unnecessarily, isn't it ?

I understand your point about the potential duplicate call to
cpufreq_verify_within_cpu_limits() for drivers with a valid freq-table.
However, in the third patch of this series, we removed the call to
cpufreq_generic_frequency_table_verify() from the table_verify path.


In the Third patch:

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


Now, the verification and the CPU limits check are unified in the
generic function, so there is no longer a redundant invocation for
drivers with a frequency table.

Thanks!

Re: [PATCH v1 2/3] cpufreq: Always enforce policy limits even without frequency table

[Viresh Kumar]

On 04-09-25, 13:48, Zihuan Zhang wrote:
> I understand your point about the potential duplicate call to
> cpufreq_verify_within_cpu_limits() for drivers with a valid freq-table.
> However, in the third patch of this series, we removed the call to
> cpufreq_generic_frequency_table_verify() from the table_verify path.

Yeah, I missed that.

-- 
viresh

Re: [PATCH v1 2/3] cpufreq: Always enforce policy limits even without frequency table

[Zihuan Zhang]

在 2025/9/8 14:13, Viresh Kumar 写道:
> On 04-09-25, 13:48, Zihuan Zhang wrote:
>> I understand your point about the potential duplicate call to
>> cpufreq_verify_within_cpu_limits() for drivers with a valid freq-table.
>> However, in the third patch of this series, we removed the call to
>> cpufreq_generic_frequency_table_verify() from the table_verify path.
> Yeah, I missed that.
>

We are currently considering moving the check that ensures a driver 
providing a freq_table also implements target_index() into the driver 
registration path.

This way, freq_table.c no longer needs to defensively check for NULL 
pointers.

Additionally, we are thinking about merging the two related APIs into a 
single one. Do you think this is a good idea?

Re: [PATCH v1 2/3] cpufreq: Always enforce policy limits even without frequency table

[Viresh Kumar]

On 08-09-25, 14:51, Zihuan Zhang wrote:
> We are currently considering moving the check that ensures a driver
> providing a freq_table also implements target_index() into the driver
> registration path.

That won't work AFAIU. The freq table is initialized during
policy->init and that's not done at the time of registration.

> This way, freq_table.c no longer needs to defensively check for NULL
> pointers.
> 
> Additionally, we are thinking about merging the two related APIs into a
> single one. Do you think this is a good idea?

Which ones ? target/target_index ? I am not sure if that can be done.

We are fine with improvements generally, but please make sure whatever
you send doesn't break existing users. That will help saving some of
our review time.

-- 
viresh

Re: [PATCH v1 2/3] cpufreq: Always enforce policy limits even without frequency table

[Zihuan Zhang]

在 2025/9/8 14:55, Viresh Kumar 写道:
> On 08-09-25, 14:51, Zihuan Zhang wrote:
>> We are currently considering moving the check that ensures a driver
>> providing a freq_table also implements target_index() into the driver
>> registration path.
> That won't work AFAIU. The freq table is initialized during
> policy->init and that's not done at the time of registration.

One idea we are considering is to check whether driver->verify points to 
cpufreq_generic_frequency_table_verify and use that as a heuristic to 
enforce the presence of target_index():

  ((driver_data->verify == cpufreq_generic_frequency_table_verify) != 
!!driver_data->target_index)

I haven’t tested this approach yet, so I’m not sure if it will be fully 
reliable.


>> This way, freq_table.c no longer needs to defensively check for NULL
>> pointers.
>>
>> Additionally, we are thinking about merging the two related APIs into a
>> single one. Do you think this is a good idea?
> Which ones ? target/target_index ? I am not sure if that can be done.

If this approach 
works,  cpufreq_generic_frequency_table_verify and cpufreq_frequency_table_verify 
should be the same because we dont't need to check pointer is NULL  in 
freq_table.c.

> We are fine with improvements generally, but please make sure whatever
> you send doesn't break existing users. That will help saving some of
> our review time.

Re: [PATCH v1 2/3] cpufreq: Always enforce policy limits even without frequency table

[Viresh Kumar]

On 08-09-25, 15:08, Zihuan Zhang wrote:
> One idea we are considering is to check whether driver->verify points to
> cpufreq_generic_frequency_table_verify and use that as a heuristic to
> enforce the presence of target_index():
> 
>  ((driver_data->verify == cpufreq_generic_frequency_table_verify) !=
> !!driver_data->target_index)
> 
> I haven’t tested this approach yet, so I’m not sure if it will be fully
> reliable.

I don't this is a good idea. It isn't necessary for any driver to use
the generic functions.

-- 
viresh

Re: [PATCH v1 2/3] cpufreq: Always enforce policy limits even without frequency table

[Zihuan Zhang]

在 2025/9/8 15:19, Viresh Kumar 写道:
> On 08-09-25, 15:08, Zihuan Zhang wrote:
>> One idea we are considering is to check whether driver->verify points to
>> cpufreq_generic_frequency_table_verify and use that as a heuristic to
>> enforce the presence of target_index():
>>
>>   ((driver_data->verify == cpufreq_generic_frequency_table_verify) !=
>> !!driver_data->target_index)
>>
>> I haven’t tested this approach yet, so I’m not sure if it will be fully
>> reliable.
> I don't this is a good idea. It isn't necessary for any driver to use
> the generic functions.
>
Understood, I thinks there is some reason that the two separate 
verification functions exist.

By the way, Do you think it’s necessary to add some defensive checks 
during driver registration?

For instance, we could enforce that a driver cannot implement both 
has_target and has_target_index at the same time.

Re: [PATCH v1 2/3] cpufreq: Always enforce policy limits even without frequency table

[Viresh Kumar]

On 08-09-25, 15:36, Zihuan Zhang wrote:
> For instance, we could enforce that a driver cannot implement both
> has_target and has_target_index at the same time.

Can be added.

-- 
viresh

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

[Zihuan Zhang]

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

[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.

-- 
viresh

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

[Zihuan Zhang]

在 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

[Viresh Kumar]

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

Re: [PATCH v1 0/3] cpufreq: Clean up frequency table handling

[Viresh Kumar]

On 04-09-25, 11:22, Zihuan Zhang wrote:
> This series contains a set of cleanups around cpufreq frequency table
> handling.
> 
> The first patch drops the redundant @freq_table parameter from
> cpufreq_frequency_table_verify(). This has already been Acked, but is
> included here to avoid build issues with the following changes.

Yeah, this is not okay. You are resending an earlier patch, which is
already Acked as V1 now. You could have just mentioned the link to the
dependency patch here, and that's all.

Don't mix things like this, it doesn't look nice.

-- 
viresh

Re: [PATCH v1 0/3] cpufreq: Clean up frequency table handling

[Zihuan Zhang]

在 2025/9/4 12:24, Viresh Kumar 写道:
> On 04-09-25, 11:22, Zihuan Zhang wrote:
>> This series contains a set of cleanups around cpufreq frequency table
>> handling.
>>
>> The first patch drops the redundant @freq_table parameter from
>> cpufreq_frequency_table_verify(). This has already been Acked, but is
>> included here to avoid build issues with the following changes.
> Yeah, this is not okay. You are resending an earlier patch, which is
> already Acked as V1 now. You could have just mentioned the link to the
> dependency patch here, and that's all.
>
> Don't mix things like this, it doesn't look nice.
>

Thanks for clarifying, Viresh.


I wasn’t aware that I could just reference the earlier Acked patch as a
dependency instead of resending it. Your suggestion makes perfect sense,
and I will do that in future submissions.

Sorry for the noise, and thanks again for the guidance!