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

[PATCH v3 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/

v3:
- Added Reviewed-by tags
- Return 0 on cpufreq_policy_init_qos() success instead of
  a random positive value
- Moved policy->min/max assignment out of cpufreq_policy_init_qos()
- Updated documentation
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 |  9 +++-
 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, 77 insertions(+), 61 deletions(-)

--
2.43.0

[PATCH v3 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>
Reviewed-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
Reviewed-by: Jie Zhan <zhanjie9@hisilicon.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..50616cfebc617 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 0;
+}
+
 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 v3 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>
Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com>
---
 drivers/cpufreq/cpufreq.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 50616cfebc617..d75c6739aa780 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1474,6 +1474,13 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 				goto out_offline_policy;
 		}
 
+		/*
+		 * 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;
+
 		/* related_cpus should at least include policy->cpus. */
 		cpumask_copy(policy->related_cpus, policy->cpus);
 	}
-- 
2.43.0

[PATCH v3 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>
Acked-by: Jie Zhan <zhanjie9@hisilicon.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 62b5d995281d2..2f0bb732ea173 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1086,10 +1086,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);
@@ -1915,10 +1914,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 1f093e3464301..dfc1e321f9e02 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 v3 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 |  9 +++++++--
 drivers/cpufreq/cpufreq.c              | 12 ++++++++++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/cpu-freq/cpu-drivers.rst b/Documentation/cpu-freq/cpu-drivers.rst
index c5635ac3de547..95e4c513c4533 100644
--- a/Documentation/cpu-freq/cpu-drivers.rst
+++ b/Documentation/cpu-freq/cpu-drivers.rst
@@ -114,8 +114,13 @@ 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,			    | The min/max scaling frequency.       |
+|policy->max			    | If set by the driver in ->init(),    |
+|				    | used as initial min/max frequency    |
+|				    | QoS request; otherwise, follow       |
+|				    | the min/max allowed frequency        |
+|				    | the cpufreq driver can set.          |
++-----------------------------------+--------------------------------------+
 |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 d75c6739aa780..759e85bf4ba17 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 (policy->boost_supported) {
 		ret = freq_qos_add_request(&policy->constraints,
 						&policy->boost_freq_req,
@@ -1411,12 +1419,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 v3 0/4] cpufreq: Set policy->min and max as real QoS constraints

[Viresh Kumar]

On 28-05-26, 11:09, 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/
> 
> v3:
> - Added Reviewed-by tags
> - Return 0 on cpufreq_policy_init_qos() success instead of
>   a random positive value
> - Moved policy->min/max assignment out of cpufreq_policy_init_qos()
> - Updated documentation

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

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

[Zhongqiu Han]

On 5/28/2026 5:09 PM, 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/
> 
> v3:
> - Added Reviewed-by tags
> - Return 0 on cpufreq_policy_init_qos() success instead of
>    a random positive value
> - Moved policy->min/max assignment out of cpufreq_policy_init_qos()
> - Updated documentation


Sorry for the delay.

Looks good to me for patches 2/4, 3/4 and 4/4.

Reviewed-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.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 |  9 +++-
>   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, 77 insertions(+), 61 deletions(-)
> 
> --
> 2.43.0


-- 
Thx and BRs,
Zhongqiu Han

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

[Rafael J. Wysocki]

On Thu, May 28, 2026 at 11:10 AM Pierre Gondois <pierre.gondois@arm.com> 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@arm.com>
> Acked-by: Jie Zhan <zhanjie9@hisilicon.com>

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?

> ---
>  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 62b5d995281d2..2f0bb732ea173 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1086,10 +1086,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);
> @@ -1915,10 +1914,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 1f093e3464301..dfc1e321f9e02 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
>

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

[Pierre Gondois]

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@arm.com> 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@arm.com>
>> Acked-by: Jie Zhan <zhanjie9@hisilicon.com>
> 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.

 >
 > However, since this patch explicitly deletes the policy->max = max_freq
 > assignment, policy->max remains 0, causing the subsequent patch to 
fall back
 > to the default -1. The core then resolves -1 to cpuinfo.max_freq (the 
boost
 > frequency) and permanently overwrites policy->max.

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@arm.com> 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@arm.com> 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@arm.com>
> >> Acked-by: Jie Zhan <zhanjie9@hisilicon.com>
> > 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!

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

[Pierre Gondois]

Hello Rafael,

On 6/8/26 18:50, Rafael J. Wysocki wrote:
> Hi,
>
> On Wed, Jun 3, 2026 at 9:49 AM Pierre Gondois <pierre.gondois@arm.com> 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@arm.com> 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@arm.com>
>>>> Acked-by: Jie Zhan <zhanjie9@hisilicon.com>
>>> 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.

The new commit message is indeed clearer.
Thanks for the update.

Regards,

Pierre


> Thanks!