Re: [PATCH v3 3/4] cpufreq: Remove driver default policy->min/max init

[Rafael J. Wysocki]

Hi,

On Wed, Jun 3, 2026 at 9:49 AM Pierre Gondois <pierre.gondois@xxxxxxx> wrote:
>
> Hello Rafael,
>
> On 6/1/26 20:08, Rafael J. Wysocki wrote:
> > On Thu, May 28, 2026 at 11:10 AM Pierre Gondois <pierre.gondois@xxxxxxx> wrote:
> >> Prior to [1], drivers were setting policy->min/max and
> >> the value was used as a QoS constraint. After that change,
> >> the values were only temporarily used: cpufreq_set_policy()
> >> ultimately overriding them through:
> >> cpufreq_policy_online()
> >> \-cpufreq_init_policy()
> >>    \-cpufreq_set_policy()
> >>      \-/* Set policy->min/max */
> >>
> >> This patch reinstate the initial behaviour. This will allow
> >> drivers to request min/max QoS frequencies if desired.
> >> For instance, the cppc driver advertises a lowest non-linear
> >> frequency, which should be used as a min QoS value.
> >>
> >> To avoid having drivers setting policy->min/max to default
> >> values which are considered as QoS values (i.e. the reason
> >> why [1] was introduced), remove the initialization of
> >> policy->min/max in .init() callbacks wherever the
> >> policy->min/max values are identical to the
> >> policy->cpuinfo.min/max_freq.
> >>
> >> Indeed, the previous patch ("cpufreq: Set default
> >> policy->min/max values for all drivers") makes this initialization
> >> redundant.
> >>
> >> The only drivers where these values are different are:
> >> - gx-suspmod.c (min)
> >> - cppc-cpufreq.c (min)
> >> - longrun.c
> >>
> >> [1]
> >> commit 521223d8b3ec ("cpufreq: Fix initialization of min and
> >> max frequency QoS requests")
> >>
> >> Signed-off-by: Pierre Gondois <pierre.gondois@xxxxxxx>
> >> Acked-by: Jie Zhan <zhanjie9@xxxxxxxxxxxxx>
> > sashiko.dev has some feedback on this patch and appears to have a point:
> >
> > https://sashiko.dev/#/patchset/20260528090913.2759118-1-pierre.gondois%40arm.com
> >
> > Can you have a look at it please?
> >
> [sashiko]
>
>  > Does removing the policy->max = max_freq assignment here break UAPI
>  > expectations by exposing the unlisted boost frequency in
> scaling_max_freq?
>  >
>  > Commit 538b0188da4653 intentionally allowed drivers like acpi-cpufreq
> to set
>  > policy->cpuinfo.max_freq to a higher boost frequency while relying on
>  > cpufreq_frequency_table_cpuinfo() to clamp policy->max to the frequency
>  > table's nominal maximum (max_freq). This ensured that user-space
> tools saw
>  > the nominal maximum in scaling_max_freq.
>  >
>  > Although commit 521223d8b3ec temporarily disrupted this by defaulting
> the QoS
>  > max to -1, a subsequent patch in this series changes the core to
> initialize
>  > the QoS request using policy->max.
>
> Effectively PATCH [4/4] cpufreq: Use policy->min/max init as QoS request
> now uses the policy->max value set by the .init() callback to set
> the max_freq_req QoS constraint.
>
>  >
>  > If the policy->max = max_freq assignment were preserved, the subsequent
>  > patch would successfully use the nominal frequency as the QoS max
> request,
>  > restoring the correct clamping behavior.
>
> IIUC this suggests to use the nominal freq. as the QoS max request.
> This was behaving like that prior to 521223d8b3ec. However doing
> that would mean that if boost is enabled and the max_freq_req sysfs
> is not updated, then the frequency would still be clamped by
> the max_freq_req. 521223d8b3ec intended to correct that.
>
> Sashiko seems to suggest modifications to come back to the
> pre-521223d8b3ec behaviour, but I think 521223d8b3ec is correct
> and we should conserve this behaviour.

So there is some confusion in the patch changelogs of this series, but
not in the code, regarding the role of the last argument of
freq_qos_add_request().  Namely, that argument is the initial request
value for the given request object which is subsequently managed by
user space.  User space may in fact change it to whatever value it
wants (either lower or higher) and it is only taken into account along
with the other requests in the given chain.  IMV it is better to
clarify that, so I have updated the changelogs when applying the
patches.

Please see

https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=bleeding-edge&id=8c83947c5dbbd49b36d08bb99e344327c6278781

and its ancestors and let me know if there's anything missing in the
changelogs thereof.

Thanks!