[PATCH v2 0/4] cpufreq: Set policy->min and max as real QoS constraints

[PATCH v2 0/4] cpufreq: Set policy->min and max as real QoS constraints

[Pierre Gondois]

This patch is a follow-up from the serie:
- [PATCH v6 0/4] cpufreq: Introduce boost frequency QoS
https://lore.kernel.org/lkml/20260317101753.2284763-1-pierre.gondois@arm.com/

v2:
- Split the patch in multiple steps
- Replace min/max -> min_freq/max_freq
- Add references to commit 521223d8b3ec ("cpufreq: Fix initialization
  of min and max frequency QoS requests") to explain the intent
  of the patch
- Update documentation (cpu-drivers.rst)
- Set default policy->min/max values before the call to
  blocking_notifier_call_chain()
- Create a new cpufreq_policy_init_qos() function to put all
  the QoS and policy->min/max logic inside.
- Didn't add Sumit's reviewed-by as the patches changed a bit.
v1:
https://lore.kernel.org/lkml/20260423084731.1090384-1-pierre.gondois@arm.com/#t

Pierre Gondois (4):
  cpufreq: Extract cpufreq_policy_init_qos() function
  cpufreq: Set default policy->min/max values for all drivers
  cpufreq: Remove driver default policy->min/max init
  cpufreq: Use policy->min/max init as QoS request

 Documentation/cpu-freq/cpu-drivers.rst | 10 +++-
 drivers/cpufreq/amd-pstate.c           | 14 +++---
 drivers/cpufreq/cppc_cpufreq.c         |  5 +-
 drivers/cpufreq/cpufreq-nforce2.c      |  4 +-
 drivers/cpufreq/cpufreq.c              | 68 ++++++++++++++++++--------
 drivers/cpufreq/freq_table.c           |  7 ++-
 drivers/cpufreq/gx-suspmod.c           |  2 +-
 drivers/cpufreq/intel_pstate.c         |  3 --
 drivers/cpufreq/pcc-cpufreq.c          | 10 ++--
 drivers/cpufreq/pxa3xx-cpufreq.c       |  5 +-
 drivers/cpufreq/sh-cpufreq.c           |  6 +--
 drivers/cpufreq/virtual-cpufreq.c      |  5 +-
 12 files changed, 78 insertions(+), 61 deletions(-)

--
2.43.0

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

[Pierre Gondois]

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@arm.com>
---
 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;
+}
+
 static int cpufreq_policy_online(struct cpufreq_policy *policy,
 				 unsigned int cpu, bool new_policy)
 {
@@ -1442,6 +1468,12 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 		if (ret)
 			goto out_offline_policy;
 
+		if (new_policy) {
+			ret = cpufreq_policy_init_qos(policy);
+			if (ret < 0)
+				goto out_offline_policy;
+		}
+
 		/* related_cpus should at least include policy->cpus. */
 		cpumask_copy(policy->related_cpus, policy->cpus);
 	}
@@ -1458,27 +1490,6 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 			add_cpu_dev_symlink(policy, j, get_cpu_device(j));
 		}
 
-		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)
-				goto out_destroy_policy;
-		}
-
-		ret = freq_qos_add_request(&policy->constraints,
-					   &policy->min_freq_req, FREQ_QOS_MIN,
-					   FREQ_QOS_MIN_DEFAULT_VALUE);
-		if (ret < 0)
-			goto out_destroy_policy;
-
-		ret = freq_qos_add_request(&policy->constraints,
-					   &policy->max_freq_req, FREQ_QOS_MAX,
-					   FREQ_QOS_MAX_DEFAULT_VALUE);
-		if (ret < 0)
-			goto out_destroy_policy;
-
 		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
 				CPUFREQ_CREATE_POLICY, policy);
 	}
-- 
2.43.0

[PATCH v2 2/4] cpufreq: Set default policy->min/max values for all drivers

[Pierre Gondois]

Some drivers set policy->min/max in their .init() callback.
cpufreq_set_policy() will ultimately override them through:
cpufreq_policy_online()
\-cpufreq_init_policy()
  \-cpufreq_set_policy()
    \-/* Set policy->min/max */
Thus the policy min/max values provided are only temporary.

There is an exception if CPUFREQ_NEED_INITIAL_FREQ_CHECK is set and:
cpufreq_policy_online()
\-__cpufreq_driver_target()
  \-cpufreq_driver->target()

To prepare for a following patch that will remove all
policy->min/max initialization in the driver .init() callback
if the min/max value is equal to the cpuinfo.min/max_freq,
set a default policy->min/max value for all drivers.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 drivers/cpufreq/cpufreq.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 034603c2af325..9e2d9d3fc5351 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1401,6 +1401,13 @@ static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
 {
 	int ret;
 
+	/*
+	 * If the driver didn't set policy->min/max, set them as
+	 * they are used to clamp frequency requests.
+	 */
+	policy->min = policy->min ? policy->min : policy->cpuinfo.min_freq;
+	policy->max = policy->max ? policy->max : policy->cpuinfo.max_freq;
+
 	if (policy->boost_supported) {
 		ret = freq_qos_add_request(&policy->constraints,
 						&policy->boost_freq_req,
-- 
2.43.0

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

[Pierre Gondois]

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@arm.com>
---
 drivers/cpufreq/amd-pstate.c      | 14 ++++++--------
 drivers/cpufreq/cppc_cpufreq.c    |  5 ++---
 drivers/cpufreq/cpufreq-nforce2.c |  4 ++--
 drivers/cpufreq/freq_table.c      |  7 +++----
 drivers/cpufreq/gx-suspmod.c      |  2 +-
 drivers/cpufreq/intel_pstate.c    |  3 ---
 drivers/cpufreq/pcc-cpufreq.c     | 10 ++++------
 drivers/cpufreq/pxa3xx-cpufreq.c  |  5 ++---
 drivers/cpufreq/sh-cpufreq.c      |  6 ++----
 drivers/cpufreq/virtual-cpufreq.c |  5 +----
 10 files changed, 23 insertions(+), 38 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 453084c67327f..ecc3779e047d5 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1090,10 +1090,9 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
 
 	perf = READ_ONCE(cpudata->perf);
 
-	policy->cpuinfo.min_freq = policy->min = perf_to_freq(perf,
-							      cpudata->nominal_freq,
-							      perf.lowest_perf);
-	policy->cpuinfo.max_freq = policy->max = cpudata->max_freq;
+	policy->cpuinfo.min_freq = perf_to_freq(perf, cpudata->nominal_freq,
+						perf.lowest_perf);
+	policy->cpuinfo.max_freq = cpudata->max_freq;
 
 	policy->driver_data = cpudata;
 	ret = amd_pstate_cppc_enable(policy);
@@ -1907,10 +1906,9 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 
 	perf = READ_ONCE(cpudata->perf);
 
-	policy->cpuinfo.min_freq = policy->min = perf_to_freq(perf,
-							      cpudata->nominal_freq,
-							      perf.lowest_perf);
-	policy->cpuinfo.max_freq = policy->max = cpudata->max_freq;
+	policy->cpuinfo.min_freq = perf_to_freq(perf, cpudata->nominal_freq,
+						perf.lowest_perf);
+	policy->cpuinfo.max_freq = cpudata->max_freq;
 	policy->driver_data = cpudata;
 
 	ret = amd_pstate_cppc_enable(policy);
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 7e7f9dfb7a24c..5abac50df7508 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -660,8 +660,6 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	 * Section 8.4.7.1.1.5 of ACPI 6.1 spec)
 	 */
 	policy->min = cppc_perf_to_khz(caps, caps->lowest_nonlinear_perf);
-	policy->max = cppc_perf_to_khz(caps, policy->boost_enabled ?
-						caps->highest_perf : caps->nominal_perf);
 
 	/*
 	 * Set cpuinfo.min_freq to Lowest to make the full range of performance
@@ -669,7 +667,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	 * nonlinear perf
 	 */
 	policy->cpuinfo.min_freq = cppc_perf_to_khz(caps, caps->lowest_perf);
-	policy->cpuinfo.max_freq = policy->max;
+	policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, policy->boost_enabled ?
+						    caps->highest_perf : caps->nominal_perf);
 
 	policy->transition_delay_us = cppc_cpufreq_get_transition_delay_us(cpu);
 	policy->shared_type = cpu_data->shared_type;
diff --git a/drivers/cpufreq/cpufreq-nforce2.c b/drivers/cpufreq/cpufreq-nforce2.c
index fbbbe501cf2dc..831102522ad64 100644
--- a/drivers/cpufreq/cpufreq-nforce2.c
+++ b/drivers/cpufreq/cpufreq-nforce2.c
@@ -355,8 +355,8 @@ static int nforce2_cpu_init(struct cpufreq_policy *policy)
 		min_fsb = NFORCE2_MIN_FSB;
 
 	/* cpuinfo and default policy values */
-	policy->min = policy->cpuinfo.min_freq = min_fsb * fid * 100;
-	policy->max = policy->cpuinfo.max_freq = max_fsb * fid * 100;
+	policy->cpuinfo.min_freq = min_fsb * fid * 100;
+	policy->cpuinfo.max_freq = max_fsb * fid * 100;
 
 	return 0;
 }
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index 5b364d8da4f92..ea994647abc88 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -49,16 +49,15 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy)
 			max_freq = freq;
 	}
 
-	policy->min = policy->cpuinfo.min_freq = min_freq;
-	policy->max = max_freq;
+	policy->cpuinfo.min_freq = min_freq;
 	/*
 	 * If the driver has set its own cpuinfo.max_freq above max_freq, leave
 	 * it as is.
 	 */
 	if (policy->cpuinfo.max_freq < max_freq)
-		policy->max = policy->cpuinfo.max_freq = max_freq;
+		policy->cpuinfo.max_freq = max_freq;
 
-	if (policy->min == ~0)
+	if (min_freq == ~0)
 		return -EINVAL;
 	else
 		return 0;
diff --git a/drivers/cpufreq/gx-suspmod.c b/drivers/cpufreq/gx-suspmod.c
index d269a4f26f98e..d40c9e0bbb740 100644
--- a/drivers/cpufreq/gx-suspmod.c
+++ b/drivers/cpufreq/gx-suspmod.c
@@ -421,7 +421,7 @@ static int cpufreq_gx_cpu_init(struct cpufreq_policy *policy)
 		policy->min = maxfreq / max_duration;
 	else
 		policy->min = maxfreq / POLICY_MIN_DIV;
-	policy->max = maxfreq;
+
 	policy->cpuinfo.min_freq = maxfreq / max_duration;
 	policy->cpuinfo.max_freq = maxfreq;
 
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 1292da53e5fcb..68ccc6eb1ef30 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -3049,9 +3049,6 @@ static int __intel_pstate_cpu_init(struct cpufreq_policy *policy)
 	policy->cpuinfo.max_freq = READ_ONCE(global.no_turbo) ?
 			cpu->pstate.max_freq : cpu->pstate.turbo_freq;
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
-
 	intel_pstate_init_acpi_perf_limits(policy);
 
 	policy->fast_switch_possible = true;
diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
index ac2e90a65f0c4..0f185a13577f8 100644
--- a/drivers/cpufreq/pcc-cpufreq.c
+++ b/drivers/cpufreq/pcc-cpufreq.c
@@ -551,13 +551,11 @@ static int pcc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		goto out;
 	}
 
-	policy->max = policy->cpuinfo.max_freq =
-		ioread32(&pcch_hdr->nominal) * 1000;
-	policy->min = policy->cpuinfo.min_freq =
-		ioread32(&pcch_hdr->minimum_frequency) * 1000;
+	policy->cpuinfo.max_freq = ioread32(&pcch_hdr->nominal) * 1000;
+	policy->cpuinfo.min_freq = ioread32(&pcch_hdr->minimum_frequency) * 1000;
 
-	pr_debug("init: policy->max is %d, policy->min is %d\n",
-		policy->max, policy->min);
+	pr_debug("init: max_freq is %d, min_freq is %d\n",
+		 policy->cpuinfo.max_freq, policy->cpuinfo.min_freq);
 out:
 	return result;
 }
diff --git a/drivers/cpufreq/pxa3xx-cpufreq.c b/drivers/cpufreq/pxa3xx-cpufreq.c
index 50ff3b6a69000..06b27cbc59d6a 100644
--- a/drivers/cpufreq/pxa3xx-cpufreq.c
+++ b/drivers/cpufreq/pxa3xx-cpufreq.c
@@ -185,9 +185,8 @@ static int pxa3xx_cpufreq_init(struct cpufreq_policy *policy)
 	int ret = -EINVAL;
 
 	/* set default policy and cpuinfo */
-	policy->min = policy->cpuinfo.min_freq = 104000;
-	policy->max = policy->cpuinfo.max_freq =
-		(cpu_is_pxa320()) ? 806000 : 624000;
+	policy->cpuinfo.min_freq = 104000;
+	policy->cpuinfo.max_freq = (cpu_is_pxa320()) ? 806000 : 624000;
 	policy->cpuinfo.transition_latency = 1000; /* FIXME: 1 ms, assumed */
 
 	if (cpu_is_pxa300() || cpu_is_pxa310())
diff --git a/drivers/cpufreq/sh-cpufreq.c b/drivers/cpufreq/sh-cpufreq.c
index 642ddb9ea217e..3c99d7009cbe2 100644
--- a/drivers/cpufreq/sh-cpufreq.c
+++ b/drivers/cpufreq/sh-cpufreq.c
@@ -124,10 +124,8 @@ static int sh_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		dev_notice(dev, "no frequency table found, falling back "
 			   "to rate rounding.\n");
 
-		policy->min = policy->cpuinfo.min_freq =
-			(clk_round_rate(cpuclk, 1) + 500) / 1000;
-		policy->max = policy->cpuinfo.max_freq =
-			(clk_round_rate(cpuclk, ~0UL) + 500) / 1000;
+		policy->cpuinfo.min_freq = (clk_round_rate(cpuclk, 1) + 500) / 1000;
+		policy->cpuinfo.max_freq = (clk_round_rate(cpuclk, ~0UL) + 500) / 1000;
 	}
 
 	return 0;
diff --git a/drivers/cpufreq/virtual-cpufreq.c b/drivers/cpufreq/virtual-cpufreq.c
index 4159f31349b16..dc78b74409af4 100644
--- a/drivers/cpufreq/virtual-cpufreq.c
+++ b/drivers/cpufreq/virtual-cpufreq.c
@@ -164,10 +164,7 @@ static int virt_cpufreq_get_freq_info(struct cpufreq_policy *policy)
 		policy->cpuinfo.min_freq = 1;
 		policy->cpuinfo.max_freq = virt_cpufreq_get_perftbl_entry(policy->cpu, 0);
 
-		policy->min = policy->cpuinfo.min_freq;
-		policy->max = policy->cpuinfo.max_freq;
-
-		policy->cur = policy->max;
+		policy->cur = policy->cpuinfo.max_freq;
 		return 0;
 	}
 
-- 
2.43.0

[PATCH v2 4/4] cpufreq: Use policy->min/max init as QoS request

[Pierre Gondois]

Consider policy->min/max being set in the driver .init()
callback as a QoS request. Impacted driver are:
- gx-suspmod.c (min)
- cppc-cpufreq.c (min)
- longrun.c (min/max)

Update the documentation accordingly.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 Documentation/cpu-freq/cpu-drivers.rst | 10 ++++++++--
 drivers/cpufreq/cpufreq.c              | 12 ++++++++++--
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/Documentation/cpu-freq/cpu-drivers.rst b/Documentation/cpu-freq/cpu-drivers.rst
index c5635ac3de547..ab4f3c0f3a89b 100644
--- a/Documentation/cpu-freq/cpu-drivers.rst
+++ b/Documentation/cpu-freq/cpu-drivers.rst
@@ -114,8 +114,14 @@ Then, the driver must fill in the following values:
 |policy->cur			    | The current operating frequency of   |
 |				    | this CPU (if appropriate)		   |
 +-----------------------------------+--------------------------------------+
-|policy->min,			    |					   |
-|policy->max,			    |					   |
+|policy->min			    | If set by the driver in ->init(),    |
+|				    | used as initial minimum frequency	   |
+|				    | QoS request.			   |
++-----------------------------------+--------------------------------------+
+|policy->max			    | If set by the driver in ->init(),    |
+|				    | used as initial maximum frequency	   |
+|				    | QoS request.			   |
++-----------------------------------+--------------------------------------+
 |policy->policy and, if necessary,  |					   |
 |policy->governor		    | must contain the "default policy" for|
 |				    | this CPU. A few moments later,       |
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 9e2d9d3fc5351..9a005367ed87b 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1399,8 +1399,16 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
 
 static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
 {
+	unsigned int min_freq, max_freq;
 	int ret;
 
+	/* Use policy->min/max set by the driver as QoS requests. */
+	min_freq = max(FREQ_QOS_MIN_DEFAULT_VALUE, policy->min);
+	if (policy->max)
+		max_freq = min(FREQ_QOS_MAX_DEFAULT_VALUE, policy->max);
+	else
+		max_freq = FREQ_QOS_MAX_DEFAULT_VALUE;
+
 	/*
 	 * If the driver didn't set policy->min/max, set them as
 	 * they are used to clamp frequency requests.
@@ -1418,12 +1426,12 @@ static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
 	}
 
 	ret = freq_qos_add_request(&policy->constraints, &policy->min_freq_req,
-				   FREQ_QOS_MIN, FREQ_QOS_MIN_DEFAULT_VALUE);
+				   FREQ_QOS_MIN, min_freq);
 	if (ret < 0)
 		return ret;
 
 	ret = freq_qos_add_request(&policy->constraints, &policy->max_freq_req,
-				   FREQ_QOS_MAX, FREQ_QOS_MAX_DEFAULT_VALUE);
+				   FREQ_QOS_MAX, max_freq);
 	if (ret < 0)
 		return ret;
 
-- 
2.43.0

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

[Zhongqiu Han]

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@arm.com>


Looks good to me apart from a minor nit inline.

Reviewed-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>


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

Alternatively, if this behavior is intentional, it might be helpful to
document it with a kdoc comment (e.g. that the function may return 0 or
1 on success and callers should check 'ret < 0' for errors).



> +}
> +
>   static int cpufreq_policy_online(struct cpufreq_policy *policy,
>   				 unsigned int cpu, bool new_policy)
>   {
> @@ -1442,6 +1468,12 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>   		if (ret)
>   			goto out_offline_policy;
>   
> +		if (new_policy) {
> +			ret = cpufreq_policy_init_qos(policy);
> +			if (ret < 0)
> +				goto out_offline_policy;
> +		}
> +
>   		/* related_cpus should at least include policy->cpus. */
>   		cpumask_copy(policy->related_cpus, policy->cpus);
>   	}
> @@ -1458,27 +1490,6 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>   			add_cpu_dev_symlink(policy, j, get_cpu_device(j));
>   		}
>   
> -		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)
> -				goto out_destroy_policy;
> -		}
> -
> -		ret = freq_qos_add_request(&policy->constraints,
> -					   &policy->min_freq_req, FREQ_QOS_MIN,
> -					   FREQ_QOS_MIN_DEFAULT_VALUE);
> -		if (ret < 0)
> -			goto out_destroy_policy;
> -
> -		ret = freq_qos_add_request(&policy->constraints,
> -					   &policy->max_freq_req, FREQ_QOS_MAX,
> -					   FREQ_QOS_MAX_DEFAULT_VALUE);
> -		if (ret < 0)
> -			goto out_destroy_policy;
> -
>   		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>   				CPUFREQ_CREATE_POLICY, policy);
>   	}


-- 
Thx and BRs,
Zhongqiu Han

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

[Jie Zhan]

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.
> 
LGTM, thanks!
Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com>
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  drivers/cpufreq/cpufreq.c | 53 +++++++++++++++++++++++----------------
>  1 file changed, 32 insertions(+), 21 deletions(-)
> 
[ ... ]

Re: [PATCH v2 2/4] cpufreq: Set default policy->min/max values for all drivers

[Jie Zhan]

On 5/11/2026 9:55 PM, Pierre Gondois wrote:
> Some drivers set policy->min/max in their .init() callback.
> cpufreq_set_policy() will ultimately override them through:
> cpufreq_policy_online()
> \-cpufreq_init_policy()
>   \-cpufreq_set_policy()
>     \-/* Set policy->min/max */
> Thus the policy min/max values provided are only temporary.
> 
> There is an exception if CPUFREQ_NEED_INITIAL_FREQ_CHECK is set and:
> cpufreq_policy_online()
> \-__cpufreq_driver_target()
>   \-cpufreq_driver->target()
> 
> To prepare for a following patch that will remove all
> policy->min/max initialization in the driver .init() callback
> if the min/max value is equal to the cpuinfo.min/max_freq,
> set a default policy->min/max value for all drivers.
> 
Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  drivers/cpufreq/cpufreq.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
[ ... ]

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

[Jie Zhan]

On 5/11/2026 9:55 PM, Pierre Gondois 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")
> 
Acked-by: Jie Zhan <zhanjie9@hisilicon.com>
for the CPPC part, though the rest looks fine but in case I miss something.
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  drivers/cpufreq/amd-pstate.c      | 14 ++++++--------
>  drivers/cpufreq/cppc_cpufreq.c    |  5 ++---
>  drivers/cpufreq/cpufreq-nforce2.c |  4 ++--
>  drivers/cpufreq/freq_table.c      |  7 +++----
>  drivers/cpufreq/gx-suspmod.c      |  2 +-
>  drivers/cpufreq/intel_pstate.c    |  3 ---
>  drivers/cpufreq/pcc-cpufreq.c     | 10 ++++------
>  drivers/cpufreq/pxa3xx-cpufreq.c  |  5 ++---
>  drivers/cpufreq/sh-cpufreq.c      |  6 ++----
>  drivers/cpufreq/virtual-cpufreq.c |  5 +----
>  10 files changed, 23 insertions(+), 38 deletions(-)
> 
[ ... ]

Re: [PATCH v2 4/4] cpufreq: Use policy->min/max init as QoS request

[Jie Zhan]

On 5/11/2026 9:55 PM, Pierre Gondois wrote:
> Consider policy->min/max being set in the driver .init()
> callback as a QoS request. Impacted driver are:
> - gx-suspmod.c (min)
> - cppc-cpufreq.c (min)
> - longrun.c (min/max)
> 
> Update the documentation accordingly.
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  Documentation/cpu-freq/cpu-drivers.rst | 10 ++++++++--
>  drivers/cpufreq/cpufreq.c              | 12 ++++++++++--
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/cpu-freq/cpu-drivers.rst b/Documentation/cpu-freq/cpu-drivers.rst
> index c5635ac3de547..ab4f3c0f3a89b 100644
> --- a/Documentation/cpu-freq/cpu-drivers.rst
> +++ b/Documentation/cpu-freq/cpu-drivers.rst
> @@ -114,8 +114,14 @@ Then, the driver must fill in the following values:
>  |policy->cur			    | The current operating frequency of   |
>  |				    | this CPU (if appropriate)		   |
>  +-----------------------------------+--------------------------------------+
> -|policy->min,			    |					   |
> -|policy->max,			    |					   |
> +|policy->min			    | If set by the driver in ->init(),    |
> +|				    | used as initial minimum frequency	   |
> +|				    | QoS request.			   |
> ++-----------------------------------+--------------------------------------+
> +|policy->max			    | If set by the driver in ->init(),    |
> +|				    | used as initial maximum frequency	   |
> +|				    | QoS request.			   |
> ++-----------------------------------+--------------------------------------+
Hi Pierre,

Trivial bit: add the general meaning alongside its driver usage at the init
stage, and mention it defaults to cpuinfo_min/max_freq if not set?

I mean something like:
The minimum/maximum scaling frequency.  If set by the driver in ->init(),
used as initial minimum/maximum frequency QoS request; otherwise, follow
policy->cpuinfo.min/max_freq.

Thanks,
Jie
>  |policy->policy and, if necessary,  |					   |
>  |policy->governor		    | must contain the "default policy" for|
>  |				    | this CPU. A few moments later,       |
[ ... ]

Re: [PATCH v2 4/4] cpufreq: Use policy->min/max init as QoS request

[Viresh Kumar]

On 11-05-26, 15:55, Pierre Gondois wrote:
> @@ -1399,8 +1399,16 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
>  
>  static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
>  {
> +	unsigned int min_freq, max_freq;
>  	int ret;
>  
> +	/* Use policy->min/max set by the driver as QoS requests. */
> +	min_freq = max(FREQ_QOS_MIN_DEFAULT_VALUE, policy->min);
> +	if (policy->max)
> +		max_freq = min(FREQ_QOS_MAX_DEFAULT_VALUE, policy->max);
> +	else
> +		max_freq = FREQ_QOS_MAX_DEFAULT_VALUE;
> +

Why is this required to be done before setting policy->min/max ? And
so I don't think patch 1/4 is required at all.

>  	/*
>  	 * If the driver didn't set policy->min/max, set them as
>  	 * they are used to clamp frequency requests.
> @@ -1418,12 +1426,12 @@ static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
>  	}
>  
>  	ret = freq_qos_add_request(&policy->constraints, &policy->min_freq_req,
> -				   FREQ_QOS_MIN, FREQ_QOS_MIN_DEFAULT_VALUE);
> +				   FREQ_QOS_MIN, min_freq);
>  	if (ret < 0)
>  		return ret;
>  
>  	ret = freq_qos_add_request(&policy->constraints, &policy->max_freq_req,
> -				   FREQ_QOS_MAX, FREQ_QOS_MAX_DEFAULT_VALUE);
> +				   FREQ_QOS_MAX, max_freq);
>  	if (ret < 0)
>  		return ret;
>  
> -- 
> 2.43.0

-- 
viresh

Re: [PATCH v2 4/4] cpufreq: Use policy->min/max init as QoS request

[zhenglifeng (A)]

On 5/11/2026 9:55 PM, Pierre Gondois wrote:
> Consider policy->min/max being set in the driver .init()
> callback as a QoS request. Impacted driver are:
> - gx-suspmod.c (min)
> - cppc-cpufreq.c (min)
> - longrun.c (min/max)
> 
> Update the documentation accordingly.
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  Documentation/cpu-freq/cpu-drivers.rst | 10 ++++++++--
>  drivers/cpufreq/cpufreq.c              | 12 ++++++++++--
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/cpu-freq/cpu-drivers.rst b/Documentation/cpu-freq/cpu-drivers.rst
> index c5635ac3de547..ab4f3c0f3a89b 100644
> --- a/Documentation/cpu-freq/cpu-drivers.rst
> +++ b/Documentation/cpu-freq/cpu-drivers.rst
> @@ -114,8 +114,14 @@ Then, the driver must fill in the following values:
>  |policy->cur			    | The current operating frequency of   |
>  |				    | this CPU (if appropriate)		   |
>  +-----------------------------------+--------------------------------------+
> -|policy->min,			    |					   |
> -|policy->max,			    |					   |
> +|policy->min			    | If set by the driver in ->init(),    |
> +|				    | used as initial minimum frequency	   |
> +|				    | QoS request.			   |
> ++-----------------------------------+--------------------------------------+
> +|policy->max			    | If set by the driver in ->init(),    |
> +|				    | used as initial maximum frequency	   |
> +|				    | QoS request.			   |
> ++-----------------------------------+--------------------------------------+
>  |policy->policy and, if necessary,  |					   |
>  |policy->governor		    | must contain the "default policy" for|
>  |				    | this CPU. A few moments later,       |
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 9e2d9d3fc5351..9a005367ed87b 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1399,8 +1399,16 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
>  
>  static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
>  {
> +	unsigned int min_freq, max_freq;
>  	int ret;
>  
> +	/* Use policy->min/max set by the driver as QoS requests. */
> +	min_freq = max(FREQ_QOS_MIN_DEFAULT_VALUE, policy->min);
> +	if (policy->max)
> +		max_freq = min(FREQ_QOS_MAX_DEFAULT_VALUE, policy->max);
> +	else
> +		max_freq = FREQ_QOS_MAX_DEFAULT_VALUE;
> +

Can't see the point of this. Why not just use policy->max and policy->min
to init qos?

>  	/*
>  	 * If the driver didn't set policy->min/max, set them as
>  	 * they are used to clamp frequency requests.
> @@ -1418,12 +1426,12 @@ static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
>  	}
>  
>  	ret = freq_qos_add_request(&policy->constraints, &policy->min_freq_req,
> -				   FREQ_QOS_MIN, FREQ_QOS_MIN_DEFAULT_VALUE);
> +				   FREQ_QOS_MIN, min_freq);
>  	if (ret < 0)
>  		return ret;
>  
>  	ret = freq_qos_add_request(&policy->constraints, &policy->max_freq_req,
> -				   FREQ_QOS_MAX, FREQ_QOS_MAX_DEFAULT_VALUE);
> +				   FREQ_QOS_MAX, max_freq);
>  	if (ret < 0)
>  		return ret;
>  

Re: [PATCH v2 4/4] cpufreq: Use policy->min/max init as QoS request

[Pierre Gondois]

Hello Viresh,

On 5/20/26 12:03, Viresh Kumar wrote:
> On 11-05-26, 15:55, Pierre Gondois wrote:
>> @@ -1399,8 +1399,16 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
>>   
>>   static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
>>   {
>> +	unsigned int min_freq, max_freq;
>>   	int ret;
>>   
>> +	/* Use policy->min/max set by the driver as QoS requests. */
>> +	min_freq = max(FREQ_QOS_MIN_DEFAULT_VALUE, policy->min);
>> +	if (policy->max)
>> +		max_freq = min(FREQ_QOS_MAX_DEFAULT_VALUE, policy->max);
>> +	else
>> +		max_freq = FREQ_QOS_MAX_DEFAULT_VALUE;
>> +
> Why is this required to be done before setting policy->min/max ? And
> so I don't think patch 1/4 is required at all.
Sorry if I misunderstand, but if we do:
"""
/*
  * If the driver didn't set policy->min/max, set them as
  * they are used to clamp frequency requests.
  */
policy->min = policy->min ? policy->min : policy->cpuinfo.min_freq;
policy->max = policy->max ? policy->max : policy->cpuinfo.max_freq;


/* Use policy->min/max set by the driver as QoS requests. */
min_freq = max(FREQ_QOS_MIN_DEFAULT_VALUE, policy->min);
if (policy->max)
   max_freq = min(FREQ_QOS_MAX_DEFAULT_VALUE, policy->max);
else
   max_freq = FREQ_QOS_MAX_DEFAULT_VALUE;
"""

then drivers that don't set policy->min/max in their .init() callback
will end up with a QoS constraint of:
[cpuinfo.min_freq:cpuinfo.max_freq].

This would bring us to what the following patch tried to solve:
521223d8b3ec ("cpufreq: Fix initialization of min and max
frequency QoS requests")

------

About removing patch [1/4], Zhongqiu noted that policy->min/max should
be set before the CPUFREQ_CREATE_POLICY notifier [1].
I then thought it would be better to save policy->min/max values
that are meant to become QoS constraint:
- as close as possible to the cpufreq_driver->init() call
- in a separate function, to do all the QoS creation in a separate
   function.

[1]

https://lore.kernel.org/all/73fac9ca-451d-49f0-b9c7-5ef6bc0119bf@oss.qualcomm.com/


>>   	/*
>>   	 * If the driver didn't set policy->min/max, set them as
>>   	 * they are used to clamp frequency requests.
>> @@ -1418,12 +1426,12 @@ static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
>>   	}
>>   
>>   	ret = freq_qos_add_request(&policy->constraints, &policy->min_freq_req,
>> -				   FREQ_QOS_MIN, FREQ_QOS_MIN_DEFAULT_VALUE);
>> +				   FREQ_QOS_MIN, min_freq);
>>   	if (ret < 0)
>>   		return ret;
>>   
>>   	ret = freq_qos_add_request(&policy->constraints, &policy->max_freq_req,
>> -				   FREQ_QOS_MAX, FREQ_QOS_MAX_DEFAULT_VALUE);
>> +				   FREQ_QOS_MAX, max_freq);
>>   	if (ret < 0)
>>   		return ret;
>>   
>> -- 
>> 2.43.0

Re: [PATCH v2 4/4] cpufreq: Use policy->min/max init as QoS request

[Pierre Gondois]

Hello Lifeng,

On 5/21/26 13:26, zhenglifeng (A) wrote:
> On 5/11/2026 9:55 PM, Pierre Gondois wrote:
>> Consider policy->min/max being set in the driver .init()
>> callback as a QoS request. Impacted driver are:
>> - gx-suspmod.c (min)
>> - cppc-cpufreq.c (min)
>> - longrun.c (min/max)
>>
>> Update the documentation accordingly.
>>
>> Signed-off-by: Pierre Gondois<pierre.gondois@arm.com>
>> ---
>>   Documentation/cpu-freq/cpu-drivers.rst | 10 ++++++++--
>>   drivers/cpufreq/cpufreq.c              | 12 ++++++++++--
>>   2 files changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/cpu-freq/cpu-drivers.rst b/Documentation/cpu-freq/cpu-drivers.rst
>> index c5635ac3de547..ab4f3c0f3a89b 100644
>> --- a/Documentation/cpu-freq/cpu-drivers.rst
>> +++ b/Documentation/cpu-freq/cpu-drivers.rst
>> @@ -114,8 +114,14 @@ Then, the driver must fill in the following values:
>>   |policy->cur			    | The current operating frequency of   |
>>   |				    | this CPU (if appropriate)		   |
>>   +-----------------------------------+--------------------------------------+
>> -|policy->min,			    |					   |
>> -|policy->max,			    |					   |
>> +|policy->min			    | If set by the driver in ->init(),    |
>> +|				    | used as initial minimum frequency	   |
>> +|				    | QoS request.			   |
>> ++-----------------------------------+--------------------------------------+
>> +|policy->max			    | If set by the driver in ->init(),    |
>> +|				    | used as initial maximum frequency	   |
>> +|				    | QoS request.			   |
>> ++-----------------------------------+--------------------------------------+
>>   |policy->policy and, if necessary,  |					   |
>>   |policy->governor		    | must contain the "default policy" for|
>>   |				    | this CPU. A few moments later,       |
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index 9e2d9d3fc5351..9a005367ed87b 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -1399,8 +1399,16 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
>>   
>>   static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
>>   {
>> +	unsigned int min_freq, max_freq;
>>   	int ret;
>>   
>> +	/* Use policy->min/max set by the driver as QoS requests. */
>> +	min_freq = max(FREQ_QOS_MIN_DEFAULT_VALUE, policy->min);
>> +	if (policy->max)
>> +		max_freq = min(FREQ_QOS_MAX_DEFAULT_VALUE, policy->max);
>> +	else
>> +		max_freq = FREQ_QOS_MAX_DEFAULT_VALUE;
>> +
> Can't see the point of this. Why not just use policy->max and policy->min
> to init qos?

With patch [3/4] "cpufreq: Remove driver default policy->min/max init",

some drivers don't set policy->min/max. For the max value, we would end-up

with a max QoS constraint of 0.

If we were to use cpuinfo.max_freq instead, then we this would bring us

back to:

521223d8b3ec ("cpufreq: Fix initialization of min and max
frequency QoS requests")


>>   	/*
>>   	 * If the driver didn't set policy->min/max, set them as
>>   	 * they are used to clamp frequency requests.
>> @@ -1418,12 +1426,12 @@ static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
>>   	}
>>   
>>   	ret = freq_qos_add_request(&policy->constraints, &policy->min_freq_req,
>> -				   FREQ_QOS_MIN, FREQ_QOS_MIN_DEFAULT_VALUE);
>> +				   FREQ_QOS_MIN, min_freq);
>>   	if (ret < 0)
>>   		return ret;
>>   
>>   	ret = freq_qos_add_request(&policy->constraints, &policy->max_freq_req,
>> -				   FREQ_QOS_MAX, FREQ_QOS_MAX_DEFAULT_VALUE);
>> +				   FREQ_QOS_MAX, max_freq);
>>   	if (ret < 0)
>>   		return ret;
>>   

Re: [PATCH v2 4/4] cpufreq: Use policy->min/max init as QoS request

[Pierre Gondois]

Hello Jie,

On 5/20/26 10:38, Jie Zhan wrote:
> On 5/11/2026 9:55 PM, Pierre Gondois wrote:
>> Consider policy->min/max being set in the driver .init()
>> callback as a QoS request. Impacted driver are:
>> - gx-suspmod.c (min)
>> - cppc-cpufreq.c (min)
>> - longrun.c (min/max)
>>
>> Update the documentation accordingly.
>>
>> Signed-off-by: Pierre Gondois<pierre.gondois@arm.com>
>> ---
>>   Documentation/cpu-freq/cpu-drivers.rst | 10 ++++++++--
>>   drivers/cpufreq/cpufreq.c              | 12 ++++++++++--
>>   2 files changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/cpu-freq/cpu-drivers.rst b/Documentation/cpu-freq/cpu-drivers.rst
>> index c5635ac3de547..ab4f3c0f3a89b 100644
>> --- a/Documentation/cpu-freq/cpu-drivers.rst
>> +++ b/Documentation/cpu-freq/cpu-drivers.rst
>> @@ -114,8 +114,14 @@ Then, the driver must fill in the following values:
>>   |policy->cur			    | The current operating frequency of   |
>>   |				    | this CPU (if appropriate)		   |
>>   +-----------------------------------+--------------------------------------+
>> -|policy->min,			    |					   |
>> -|policy->max,			    |					   |
>> +|policy->min			    | If set by the driver in ->init(),    |
>> +|				    | used as initial minimum frequency	   |
>> +|				    | QoS request.			   |
>> ++-----------------------------------+--------------------------------------+
>> +|policy->max			    | If set by the driver in ->init(),    |
>> +|				    | used as initial maximum frequency	   |
>> +|				    | QoS request.			   |
>> ++-----------------------------------+--------------------------------------+
> Hi Pierre,
>
> Trivial bit: add the general meaning alongside its driver usage at the init
> stage, and mention it defaults to cpuinfo_min/max_freq if not set?
>
> I mean something like:
> The minimum/maximum scaling frequency.  If set by the driver in ->init(),
> used as initial minimum/maximum frequency QoS request; otherwise, follow
> policy->cpuinfo.min/max_freq.

Just one NIT, policy->min/max should follow the min/max allowed freq.

the cpufreq driver can set. E.g. a thermal constraint can impact it (cf. 
dtpm_cpu.c).

Would this fit ?

The minimum/maximum scaling frequency.  If set by the driver in ->init(),
used as initial minimum/maximum frequency QoS request; otherwise, follow
the min/max allowed freq. the cpufreq driver can set.


> Thanks,
> Jie
>>   |policy->policy and, if necessary,  |					   |
>>   |policy->governor		    | must contain the "default policy" for|
>>   |				    | this CPU. A few moments later,       |
> [ ... ]

Re: [PATCH v2 2/4] cpufreq: Set default policy->min/max values for all drivers

[Viresh Kumar]

On 11-05-26, 15:55, Pierre Gondois wrote:
> Some drivers set policy->min/max in their .init() callback.
> cpufreq_set_policy() will ultimately override them through:
> cpufreq_policy_online()
> \-cpufreq_init_policy()
>   \-cpufreq_set_policy()
>     \-/* Set policy->min/max */
> Thus the policy min/max values provided are only temporary.
> 
> There is an exception if CPUFREQ_NEED_INITIAL_FREQ_CHECK is set and:
> cpufreq_policy_online()
> \-__cpufreq_driver_target()
>   \-cpufreq_driver->target()
> 
> To prepare for a following patch that will remove all
> policy->min/max initialization in the driver .init() callback
> if the min/max value is equal to the cpuinfo.min/max_freq,
> set a default policy->min/max value for all drivers.
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  drivers/cpufreq/cpufreq.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 034603c2af325..9e2d9d3fc5351 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1401,6 +1401,13 @@ static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
>  {
>  	int ret;
>  
> +	/*
> +	 * If the driver didn't set policy->min/max, set them as
> +	 * they are used to clamp frequency requests.
> +	 */
> +	policy->min = policy->min ? policy->min : policy->cpuinfo.min_freq;
> +	policy->max = policy->max ? policy->max : policy->cpuinfo.max_freq;
> +

This has nothing to do with qos-init, we should do it along with:

        cpumask_copy(policy->related_cpus, policy->cpus);

in cpufreq_policy_online().

-- 
viresh

Re: [PATCH v2 4/4] cpufreq: Use policy->min/max init as QoS request

[Viresh Kumar]

On 21-05-26, 13:58, Pierre Gondois wrote:
> Sorry if I misunderstand, but if we do:

> This would bring us to what the following patch tried to solve:
> 521223d8b3ec ("cpufreq: Fix initialization of min and max
> frequency QoS requests")

I missed this patch, that's all.

-- 
viresh

Re: [PATCH v2 0/4] cpufreq: Set policy->min and max as real QoS constraints

[Viresh Kumar]

On 11-05-26, 15:55, Pierre Gondois wrote:
> This patch is a follow-up from the serie:
> - [PATCH v6 0/4] cpufreq: Introduce boost frequency QoS
> https://lore.kernel.org/lkml/20260317101753.2284763-1-pierre.gondois@arm.com/

Apart from a minor change in patch 2, looks fine now.

-- 
viresh

Re: [PATCH v2 4/4] cpufreq: Use policy->min/max init as QoS request

[Jie Zhan]

On 5/21/2026 7:58 PM, Pierre Gondois wrote:
> Hello Jie,
> 
> On 5/20/26 10:38, Jie Zhan wrote:
>> On 5/11/2026 9:55 PM, Pierre Gondois wrote:
>>> Consider policy->min/max being set in the driver .init()
>>> callback as a QoS request. Impacted driver are:
>>> - gx-suspmod.c (min)
>>> - cppc-cpufreq.c (min)
>>> - longrun.c (min/max)
>>>
>>> Update the documentation accordingly.
>>>
>>> Signed-off-by: Pierre Gondois<pierre.gondois@arm.com>
>>> ---
>>>   Documentation/cpu-freq/cpu-drivers.rst | 10 ++++++++--
>>>   drivers/cpufreq/cpufreq.c              | 12 ++++++++++--
>>>   2 files changed, 18 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/cpu-freq/cpu-drivers.rst b/Documentation/cpu-freq/cpu-drivers.rst
>>> index c5635ac3de547..ab4f3c0f3a89b 100644
>>> --- a/Documentation/cpu-freq/cpu-drivers.rst
>>> +++ b/Documentation/cpu-freq/cpu-drivers.rst
>>> @@ -114,8 +114,14 @@ Then, the driver must fill in the following values:
>>>   |policy->cur                | The current operating frequency of   |
>>>   |                    | this CPU (if appropriate)           |
>>>   +-----------------------------------+--------------------------------------+
>>> -|policy->min,                |                       |
>>> -|policy->max,                |                       |
>>> +|policy->min                | If set by the driver in ->init(),    |
>>> +|                    | used as initial minimum frequency       |
>>> +|                    | QoS request.               |
>>> ++-----------------------------------+--------------------------------------+
>>> +|policy->max                | If set by the driver in ->init(),    |
>>> +|                    | used as initial maximum frequency       |
>>> +|                    | QoS request.               |
>>> ++-----------------------------------+--------------------------------------+
>> Hi Pierre,
>>
>> Trivial bit: add the general meaning alongside its driver usage at the init
>> stage, and mention it defaults to cpuinfo_min/max_freq if not set?
>>
>> I mean something like:
>> The minimum/maximum scaling frequency.  If set by the driver in ->init(),
>> used as initial minimum/maximum frequency QoS request; otherwise, follow
>> policy->cpuinfo.min/max_freq.
> 
> Just one NIT, policy->min/max should follow the min/max allowed freq.
> 
> the cpufreq driver can set. E.g. a thermal constraint can impact it (cf. dtpm_cpu.c).
> 
> Would this fit ?
> 
> The minimum/maximum scaling frequency.  If set by the driver in ->init(),
> used as initial minimum/maximum frequency QoS request; otherwise, follow
> the min/max allowed freq. the cpufreq driver can set.
> 
Sure! That's good.
> 
>> Thanks,
>> Jie
>>>   |policy->policy and, if necessary,  |                       |
>>>   |policy->governor            | must contain the "default policy" for|
>>>   |                    | this CPU. A few moments later,       |
>> [ ... ]
> 

Re: [PATCH v2 2/4] cpufreq: Set default policy->min/max values for all drivers

[Pierre Gondois]

Hello Viresh,

On 5/22/26 07:43, Viresh Kumar wrote:
> On 11-05-26, 15:55, Pierre Gondois wrote:
>> Some drivers set policy->min/max in their .init() callback.
>> cpufreq_set_policy() will ultimately override them through:
>> cpufreq_policy_online()
>> \-cpufreq_init_policy()
>>    \-cpufreq_set_policy()
>>      \-/* Set policy->min/max */
>> Thus the policy min/max values provided are only temporary.
>>
>> There is an exception if CPUFREQ_NEED_INITIAL_FREQ_CHECK is set and:
>> cpufreq_policy_online()
>> \-__cpufreq_driver_target()
>>    \-cpufreq_driver->target()
>>
>> To prepare for a following patch that will remove all
>> policy->min/max initialization in the driver .init() callback
>> if the min/max value is equal to the cpuinfo.min/max_freq,
>> set a default policy->min/max value for all drivers.
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>   drivers/cpufreq/cpufreq.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index 034603c2af325..9e2d9d3fc5351 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -1401,6 +1401,13 @@ static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
>>   {
>>   	int ret;
>>   
>> +	/*
>> +	 * If the driver didn't set policy->min/max, set them as
>> +	 * they are used to clamp frequency requests.
>> +	 */
>> +	policy->min = policy->min ? policy->min : policy->cpuinfo.min_freq;
>> +	policy->max = policy->max ? policy->max : policy->cpuinfo.max_freq;
>> +
> This has nothing to do with qos-init, we should do it along with:
>
>          cpumask_copy(policy->related_cpus, policy->cpus);
>
> in cpufreq_policy_online().
>
Ok right, I changed it in v3

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@arm.com>
>
>
> Looks good to me apart from a minor nit inline.
>
> Reviewed-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
>
>
>> ---
>>   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.