[PATCH v2] cpufreq: CPPC: Set CPPC Enable register in cpu_init

[PATCH v2] cpufreq: CPPC: Set CPPC Enable register in cpu_init

[Sumit Gupta]

As per ACPI 6.x s8.4.6.1.4 (CPPC Enable Register):
  "If supported by the platform, OSPM writes a one to this register
   to enable CPPC on this processor. If not implemented, OSPM assumes
   the platform always has CPPC enabled."

The driver never writes this register. CPPC therefore stays disabled
on platforms such as NVIDIA GB10 that wait for OSPM to enable it.

Call cppc_set_enable() in cppc_cpufreq_cpu_init(). Fail init if
enabling CPPC fails. If init fails after CPPC is enabled, disable it
again. The register is optional, so -EOPNOTSUPP is not an error.

This relies on commit ab482f1bac6b ("ACPI: CPPC: Refactor register
value get and set ABIs") from v6.16. Before it, cppc_set_enable()
returned -EFAULT instead of -EOPNOTSUPP when the Enable register is
not implemented. On an older kernel this change would make init fail
on every platform without the register, so the stable range starts
at v6.16.

Fixes: 5477fb3bd1e8 ("ACPI / CPPC: Add a CPUFreq driver for use with CPPC")
Cc: stable@vger.kernel.org # v6.16+
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
v1 -> v2:
- Fail init if enabling CPPC fails and disable CPPC again if init
  fails after enabling it (Christian).
- Move enable after the policy->shared_type check, so cppc_set_perf()
  is the only failure after it.
- Mention NVIDIA GB10 in the commit message (Christian).

v1: https://lore.kernel.org/lkml/20261105123244.3132560-1-sumitg@nvidia.com/

 drivers/cpufreq/cppc_cpufreq.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 957fa7775260..b2af24ae5008 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -714,7 +714,17 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		pr_debug("Unsupported CPU co-ord type: %d\n",
 			 policy->shared_type);
 		ret = -EFAULT;
-		goto out;
+		goto out_put;
+	}
+
+	/*
+	 * Enable CPPC. The Enable register is optional - some platforms
+	 * may not support it.
+	 */
+	ret = cppc_set_enable(cpu, true);
+	if (ret && ret != -EOPNOTSUPP) {
+		pr_err("Failed to enable CPPC for CPU%u (%d)\n", cpu, ret);
+		goto out_put;
 	}
 
 	policy->fast_switch_possible = cppc_allow_fast_switch(policy->cpus);
@@ -742,6 +752,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	return 0;
 
 out:
+	cppc_set_enable(cpu, false);
+out_put:
 	cppc_cpufreq_put_cpu_data(policy);
 	return ret;
 }
-- 
2.34.1

Re: [PATCH v2] cpufreq: CPPC: Set CPPC Enable register in cpu_init

[Christian Loehle]

On 11/10/26 10:12, Sumit Gupta wrote:
> As per ACPI 6.x s8.4.6.1.4 (CPPC Enable Register):
>   "If supported by the platform, OSPM writes a one to this register
>    to enable CPPC on this processor. If not implemented, OSPM assumes
>    the platform always has CPPC enabled."
> 
> The driver never writes this register. CPPC therefore stays disabled
> on platforms such as NVIDIA GB10 that wait for OSPM to enable it.
> 
> Call cppc_set_enable() in cppc_cpufreq_cpu_init(). Fail init if
> enabling CPPC fails. If init fails after CPPC is enabled, disable it
> again. The register is optional, so -EOPNOTSUPP is not an error.
> 
> This relies on commit ab482f1bac6b ("ACPI: CPPC: Refactor register
> value get and set ABIs") from v6.16. Before it, cppc_set_enable()
> returned -EFAULT instead of -EOPNOTSUPP when the Enable register is
> not implemented. On an older kernel this change would make init fail
> on every platform without the register, so the stable range starts
> at v6.16.
> 
> Fixes: 5477fb3bd1e8 ("ACPI / CPPC: Add a CPUFreq driver for use with CPPC")
> Cc: stable@vger.kernel.org # v6.16+
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>

Reviewed-by: Christian Loehle <christian.loehle@arm.com>

> ---
> v1 -> v2:
> - Fail init if enabling CPPC fails and disable CPPC again if init
>   fails after enabling it (Christian).
> - Move enable after the policy->shared_type check, so cppc_set_perf()
>   is the only failure after it.
> - Mention NVIDIA GB10 in the commit message (Christian).
> 
> v1: https://lore.kernel.org/lkml/20261105123244.3132560-1-sumitg@nvidia.com/
> 
>  drivers/cpufreq/cppc_cpufreq.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 957fa7775260..b2af24ae5008 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -714,7 +714,17 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  		pr_debug("Unsupported CPU co-ord type: %d\n",
>  			 policy->shared_type);
>  		ret = -EFAULT;
> -		goto out;
> +		goto out_put;
> +	}
> +
> +	/*
> +	 * Enable CPPC. The Enable register is optional - some platforms
> +	 * may not support it.
> +	 */
> +	ret = cppc_set_enable(cpu, true);
> +	if (ret && ret != -EOPNOTSUPP) {
> +		pr_err("Failed to enable CPPC for CPU%u (%d)\n", cpu, ret);
> +		goto out_put;
>  	}
>  
>  	policy->fast_switch_possible = cppc_allow_fast_switch(policy->cpus);
> @@ -742,6 +752,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  	return 0;
>  
>  out:
> +	cppc_set_enable(cpu, false);
> +out_put:
>  	cppc_cpufreq_put_cpu_data(policy);
>  	return ret;
>  }