Re: [PATCH] ACPI: CPPC: Skip writes to unsupported performance controls

[Rafael J. Wysocki (Intel)]

On Mon, Jul 27, 2026 at 2:27 PM Christian Loehle
<christian.loehle@xxxxxxx> wrote:
>
> On 7/27/26 12:14, zhenglifeng (A) wrote:
> > On 7/24/2026 6:40 PM, Christian Loehle wrote:
> >> MIN_PERF and MAX_PERF are optional CPPC controls. DESIRED_PERF is also
> >> optional with CPPC2 when autonomous selection is supported.
> >>
> >> The cppc-cpufreq target callbacks populate both limits for every request
> >> without checking whether the controls are implemented. cppc_set_perf()
> >> consequently passes NULL register descriptors to cpc_write(). The writes
> >> fail width validation and their return values are ignored, so the failed
> >> access paths are repeated on every target request. An autonomous-only
> >> platform can take the same path for DESIRED_PERF.
> >>
> >> Check that each performance control is supported before calling
> >> cpc_write().
> >>
> >> Fixes: ea3db45ae476 ("cpufreq: cppc: Update MIN_PERF/MAX_PERF in target callbacks")
> >> Reviewed-by: Sumit Gupta <sumitg@xxxxxxxxxx>
> >> Signed-off-by: Christian Loehle <christian.loehle@xxxxxxx>
> >> ---
> >> v2: Also added desired_perf check (Sumit)
> >>
> >>  drivers/acpi/cppc_acpi.c | 7 ++++---
> >>  1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> >> index 1d3a94100491..53d09ca98f06 100644
> >> --- a/drivers/acpi/cppc_acpi.c
> >> +++ b/drivers/acpi/cppc_acpi.c
> >> @@ -1963,16 +1963,17 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
> >>              cpc_desc->write_cmd_status = 0;
> >>      }
> >>
> >> -    cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
> >> +    if (CPC_SUPPORTED(desired_reg))
> >> +            cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
> >>
> >>      /*
> >>       * Only write if min_perf and max_perf not zero. Some drivers pass zero
> >>       * value to min and max perf, but they don't mean to set the zero value,
> >>       * they just don't want to write to those registers.
> >>       */
> >> -    if (perf_ctrls->min_perf)
> >> +    if (perf_ctrls->min_perf && CPC_SUPPORTED(min_perf_reg))
> >>              cpc_write(cpu, min_perf_reg, perf_ctrls->min_perf);
> >> -    if (perf_ctrls->max_perf)
> >> +    if (perf_ctrls->max_perf && CPC_SUPPORTED(max_perf_reg))
> >>              cpc_write(cpu, max_perf_reg, perf_ctrls->max_perf);
> >>
> >>      if (CPC_IN_PCC(desired_reg) || CPC_IN_PCC(min_perf_reg) || CPC_IN_PCC(max_perf_reg))
> >
> > LGTM.
> >
> > Reviewed-by: Lifeng Zheng <zhenglifeng1@xxxxxxxxxx>
> >
> > Apologize for replying to v1 before seeing v2.
> >
>
> Thanks for taking a look!
> You can blame it on me for messing up the title and forgetting to put v2 there :/

Applied as 7.2-rc material, thanks!