Re: [PATCH 03/19] ACPI: CPPC: add cppc enable register function

Fontenot, Nathan
2021-09-08T19:14:44Z

On 9/8/2021 9:59 AM, Huang Rui wrote:
> From: Jinzhou Su <Jinzhou.Su@amd.com>
>
> Export the cppc enable register function for future use.
>
> Signed-off-by: Jinzhou Su <Jinzhou.Su@amd.com>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
> drivers/acpi/cppc_acpi.c | 42 ++++++++++++++++++++++++++++++++++++++++
> include/acpi/cppc_acpi.h | 5 +++++
> 2 files changed, 47 insertions(+)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index a4d4eebba1da..de4b30545215 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1220,6 +1220,48 @@ int cppc_get_perf_ctrs(int cpunum, struct cppc_perf_fb_ctrs *perf_fb_ctrs)
> }
> EXPORT_SYMBOL_GPL(cppc_get_perf_ctrs);
>
> +/**
> + * cppc_set_enable - Set to enable CPPC register.
> + * @cpu: CPU for which to enable CPPC register.
> + * @enable: enable field to write into share memory.
> + *
> + * Return: 0 for success, -ERRNO otherwise.
> + */
> +int cppc_set_enable(int cpu, u32 enable)
> +{
> + int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
> + struct cpc_register_resource *enable_reg;
> + struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
> + struct cppc_pcc_data *pcc_ss_data = NULL;
> + int ret = -1;
> +
> + if (!cpc_desc) {
> + pr_debug("No CPC descriptor for CPU:%d\n", cpu);
> + return -ENODEV;
> + }
> +
> + enable_reg = &cpc_desc->cpc_regs[ENABLE];
> +
> + if (CPC_IN_PCC(enable_reg)) {
> +
> + if (pcc_ss_id < 0)
> + return -EIO;
> +
> + ret = cpc_write(cpu, enable_reg, enable);
> + if (ret)
> + return ret;
> +
> + pcc_ss_data = pcc_data[pcc_ss_id];
> +
> + down_write(&pcc_ss_data->pcc_lock);
> + send_pcc_cmd(pcc_ss_id, CMD_WRITE);

Shouldn't we be checking the return value from send_pcc_cmd()?

Also, if the call to send_pcc_cmd() fails do we need to update
enable_reg? i.e. cpc_write(..., !enable);

-Nathan

> + up_write(&pcc_ss_data->pcc_lock);
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(cppc_set_enable);
> +
> /**
> * cppc_set_perf - Set a CPU's performance controls.
> * @cpu: CPU for which to set performance controls.
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 9f4985b4d64d..3fdae40a75fc 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -137,6 +137,7 @@ struct cppc_cpudata {
> extern int cppc_get_desired_perf(int cpunum, u64 *desired_perf);
> extern int cppc_get_perf_ctrs(int cpu, struct cppc_perf_fb_ctrs *perf_fb_ctrs);
> extern int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls);
> +extern int cppc_set_enable(int cpu, u32 enable);
> extern int cppc_get_perf_caps(int cpu, struct cppc_perf_caps *caps);
> extern bool acpi_cpc_valid(void);
> extern int acpi_get_psd_map(unsigned int cpu, struct cppc_cpudata *cpu_data);
> @@ -157,6 +158,10 @@ static inline int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
> {
> return -ENOTSUPP;
> }
> +static inline int cppc_set_enable(int cpu, u32 enable)
> +{
> + return -ENOTSUPP;
> +}
> static inline int cppc_get_perf_caps(int cpu, struct cppc_perf_caps *caps)
> {
> return -ENOTSUPP;
> --
> 2.25.1
>