Re: [PATCH v3 2/2] cpufreq: fix policy max not returning to non-boost freq on boost disable

[Ananthu C V <ananthu.cv@oss.qualcomm.com>]

Hi Zhongqiu,

On Sun, Oct 11, 2026 at 10:17:14PM +0800, Zhongqiu Han wrote:
> Patch 2/2 currently depends on a helper introduced by patch 1/2, which
> prevents it from being backported independently. Since 2/2 is an
> independent fix that should be backported to stable on its own, I think
> it should be placed before patch 1/2 in the series and kept self
> contained.

The idea was that currently patch 2/2 would serve no actual value without the
fix in patch 1/2, and that is why it was the second one. But yes, we cannot
fix 1/2 and leave 2/2 as is, which would turn the current benign issue to an
actual problem, so the order should be probably reversed. I'll work on that.

> To avoid introducing a new member that is only used by frequency-table
> drivers, please consider the following implementation instead:
> 
> If there are boost entries in the frequency table, the table is the only
> source of cpuinfo.max_freq, so let cpufreq_frequency_table_cpuinfo() set
> it unconditionally in that case.
> 
> 
> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
> index ea99464..8c8bad2 100644
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -33,14 +33,17 @@ int cpufreq_frequency_table_cpuinfo(struct
> cpufreq_policy *policy)
>  	struct cpufreq_frequency_table *pos, *table = policy->freq_table;
>  	unsigned int min_freq = ~0;
>  	unsigned int max_freq = 0;
> +	bool has_boost = false;
>  	unsigned int freq, i;
> 
>  	cpufreq_for_each_valid_entry_idx(pos, table, i) {
>  		freq = pos->frequency;
> 
> -		if ((!cpufreq_boost_enabled() || !policy->boost_enabled)
> -		    && (pos->flags & CPUFREQ_BOOST_FREQ))
> -			continue;
> +		if (pos->flags & CPUFREQ_BOOST_FREQ) {
> +			has_boost = true;
> +			if (!cpufreq_boost_enabled() || !policy->boost_enabled)
> +				continue;
> +		}
> 
>  		pr_debug("table entry %u: %u kHz\n", i, freq);
>  		if (freq < min_freq)
> @@ -50,17 +53,20 @@ int cpufreq_frequency_table_cpuinfo(struct
> cpufreq_policy *policy)
>  	}
> 
>  	policy->cpuinfo.min_freq = min_freq;
> +
> +	if (min_freq == ~0)
> +		return -EINVAL;
> +
>  	/*
>  	 * If the driver has set its own cpuinfo.max_freq above max_freq, leave
> -	 * it as is.
> +	 * it as is.  This does not apply to tables with boost entries, because
> +	 * the table is the only source of cpuinfo.max_freq then and it needs
> +	 * to go down again when boost is disabled.
>  	 */
> -	if (policy->cpuinfo.max_freq < max_freq)
> +	if (has_boost || policy->cpuinfo.max_freq < max_freq)
>  		policy->cpuinfo.max_freq = max_freq;
> 
> -	if (min_freq == ~0)
> -		return -EINVAL;
> -	else
> -		return 0;
> +	return 0;
>  }
> 
>  int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy)
> 
> 
> >   	/* in 10^(-9) s = nanoseconds */
> >   	unsigned int		transition_latency;
> > @@ -791,6 +792,8 @@ struct cpufreq_frequency_table {
> >   int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy);
> > +bool policy_has_boost_freq(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);

I wanted more discussion on this part regarding finer implementation details
and hence went with a working version, and I think I asked in the last revision
thread regarding what are people's thoughts on this added field. I am also not
keen on adding more fields, so this is useful suggestion. If we are to continue
this implementation route, I'll keep this in mind, thanks.

> Thx and BRs,
> Zhongqiu Han

Best,
Ananthu