linux-kernel - Re: [PATCH v1 1/3] cpufreq: Drop redundant freq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0hcnqD39OAjFfscCnQ2ZGu9Z1gP5WAPfu3jbeEWr6rGwQ@mail.gmail.com>
Date: Thu, 4 Sep 2025 15:31:42 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Zihuan Zhang <zhangzihuan@...inos.cn>
Cc: "Rafael J . wysocki" <rafael@...nel.org>, Viresh Kumar <viresh.kumar@...aro.org>, 
	Saravana Kannan <saravanak@...gle.com>, zhenglifeng <zhenglifeng1@...wei.com>, linux-pm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/3] cpufreq: Drop redundant freq_table parameter

On Thu, Sep 4, 2025 at 5:22 AM Zihuan Zhang <zhangzihuan@...inos.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@...inos.cn>
> Acked-by: Viresh Kumar <viresh.kumar@...aro.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
>