Re: [PATCHv2 3/3] ACPI: CPPC: Stop reading desired_perf in cppc_get_perf()

[Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>]

On 7/29/2026 6:02 PM, Christian Loehle wrote:
> cppc_get_perf() has one in-tree caller, cppc_cpufreq_get_cpu_data().
> It uses the function to preserve existing controls before writing them, but
> overwrites desired_perf with highest_perf before the first cppc_set_perf().
> Consequently, the current Desired Performance value is not consumed.
> 
> Remove the Desired Performance read from this aggregate getter and
> document that the field is returned as zero.
> 
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
>   drivers/acpi/cppc_acpi.c | 23 ++++++++---------------
>   1 file changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 210988d57b71..8ce2033ba993 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1828,22 +1828,22 @@ int cppc_set_enable(int cpu, bool enable)
>   EXPORT_SYMBOL_GPL(cppc_set_enable);
>   
>   /**
> - * cppc_get_perf - Get a CPU's performance controls.
> + * cppc_get_perf - Get a CPU's readable performance controls.

Small nit: is this also applicable to ACPI <6.6?

>    * @cpu: CPU for which to get performance controls.
>    * @perf_ctrls: ptr to cppc_perf_ctrls. See cppc_acpi.h
>    *
> + * Desired Performance is not read and is returned as 0.

Likewise.

> + *
>    * Return: 0 for success with perf_ctrls, -ERRNO otherwise.
>    */
>   int cppc_get_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>   {
>   	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
> -	struct cpc_register_resource *desired_perf_reg,
> -				     *min_perf_reg, *max_perf_reg,
> +	struct cpc_register_resource *min_perf_reg, *max_perf_reg,
>   				     *energy_perf_reg, *auto_sel_reg;
> -	u64 desired_perf = 0, min = 0, max = 0, energy_perf = 0, auto_sel = 0;
> +	u64 min = 0, max = 0, energy_perf = 0, auto_sel = 0;
>   	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
>   	struct cppc_pcc_data *pcc_ss_data = NULL;
> -	bool read_desired_perf = cppc_desired_perf_readable();
>   	int ret = 0, regs_in_pcc = 0;
>   
>   	if (!cpc_desc) {
> @@ -1856,16 +1856,14 @@ int cppc_get_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>   		return -EINVAL;
>   	}
>   
> -	desired_perf_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
>   	min_perf_reg = &cpc_desc->cpc_regs[MIN_PERF];
>   	max_perf_reg = &cpc_desc->cpc_regs[MAX_PERF];
>   	energy_perf_reg = &cpc_desc->cpc_regs[ENERGY_PERF];
>   	auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
>   
>   	/* Are any of the regs PCC ?*/
> -	if ((read_desired_perf && CPC_IN_PCC(desired_perf_reg)) ||
> -	    CPC_IN_PCC(min_perf_reg) ||
> -	    CPC_IN_PCC(max_perf_reg) || CPC_IN_PCC(energy_perf_reg) ||
> +	if (CPC_IN_PCC(min_perf_reg) || CPC_IN_PCC(max_perf_reg) ||
> +	    CPC_IN_PCC(energy_perf_reg) ||
>   	    CPC_IN_PCC(auto_sel_reg)) {
>   		if (pcc_ss_id < 0) {
>   			pr_debug("Invalid pcc_ss_id for CPU:%d\n", cpu);
> @@ -1896,12 +1894,7 @@ int cppc_get_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>   	}
>   	perf_ctrls->min_perf = min;
>   
> -	if (read_desired_perf && CPC_SUPPORTED(desired_perf_reg)) {
> -		ret = cpc_read(cpu, desired_perf_reg, &desired_perf);
> -		if (ret)
> -			goto out_err;
> -	}
> -	perf_ctrls->desired_perf = desired_perf;
> +	perf_ctrls->desired_perf = 0;
>   
>   	if (CPC_SUPPORTED(energy_perf_reg)) {
>   		ret = cpc_read(cpu, energy_perf_reg, &energy_perf);

This is an exported API, I'm not sure whether this would break any out
of-tree users, but I don't think compatibility concerns for out-of-tree
users should weigh heavily in this case.

Looks good to me.

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

-- 
Thx and BRs,
Zhongqiu Han