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@xxxxxxxxxxxxxxx # v6.16+
> Signed-off-by: Sumit Gupta <sumitg@xxxxxxxxxx>

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

> ---
> 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@xxxxxxxxxx/
> 
>  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;
>  }