Re: [PATCH v2 1/4] cpufreq: Extract cpufreq_policy_init_qos() function

[Pierre Gondois]

Hello Zhongqiu,

On 5/19/26 15:51, Zhongqiu Han wrote:
> On 5/11/2026 9:55 PM, Pierre Gondois wrote:
> > Extract the QoS related logic from cpufreq_policy_online()
> > to make the function shorter/simpler.
> >
> > The logic is placed in cpufreq_policy_init_qos() and is
> > now executed right after the following calls:
> > - cpufreq_driver->init()
> > - cpufreq_table_validate_and_sort()
> >
> > This helps preparing following patches that will,
> > in cpufreq_policy_init_qos():
> > - treat the policy->min/max values set by drivers as QoS requests.
> > - set a default policy->min/max value to all policies.
> >
> > No functional change.
> >
> > Signed-off-by: Pierre Gondois <pierre.gondois@xxxxxxx>
>
>
> Looks good to me apart from a minor nit inline.
>
> Reviewed-by: Zhongqiu Han <zhongqiu.han@xxxxxxxxxxxxxxxx>
>
>
> > ---
> >   drivers/cpufreq/cpufreq.c | 53 +++++++++++++++++++++++----------------
> >   1 file changed, 32 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 44eb1b7e7fc1b..034603c2af325 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -1397,6 +1397,32 @@ static void cpufreq_policy_free(struct 
> > cpufreq_policy *policy)
> >       kfree(policy);
> >   }
> >   +static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
> > +{
> > +    int ret;
> > +
> > +    if (policy->boost_supported) {
> > +        ret = freq_qos_add_request(&policy->constraints,
> > +                        &policy->boost_freq_req,
> > +                        FREQ_QOS_MAX,
> > +                        policy->cpuinfo.max_freq);
> > +        if (ret < 0)
> > +            return ret;
> > +    }
> > +
> > +    ret = freq_qos_add_request(&policy->constraints, 
> > &policy->min_freq_req,
> > +                   FREQ_QOS_MIN, FREQ_QOS_MIN_DEFAULT_VALUE);
> > +    if (ret < 0)
> > +        return ret;
> > +
> > +    ret = freq_qos_add_request(&policy->constraints, 
> > &policy->max_freq_req,
> > +                   FREQ_QOS_MAX, FREQ_QOS_MAX_DEFAULT_VALUE);
> > +    if (ret < 0)
> > +        return ret;
> > +
> > +    return ret;
>
>
>
> Just minor nit: cpufreq_policy_init_qos() could perhaps return 0 on
> success.
>
> In the original inline code we only checked 'ret < 0', so positive
> return values were not intended to propagate as part of the API
> contract. Returning 'ret' here may expose a positive success code (e.g.
> 1) and make the helper easier to misuse (e.g. 'if (ret)' checks).
> Returning 0 would keep the semantics unambiguous.


Yes right indeed, I ll do that.