LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Shuah Khan <skhan@linuxfoundation.org>
To: Huang Rui <ray.huang@amd.com>,
	"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Borislav Petkov <bp@suse.de>, Ingo Molnar <mingo@kernel.org>,
	linux-pm@vger.kernel.org
Cc: Deepak Sharma <deepak.sharma@amd.com>,
	Alex Deucher <alexander.deucher@amd.com>,
	Mario Limonciello <mario.limonciello@amd.com>,
	Nathan Fontenot <nathan.fontenot@amd.com>,
	Jinzhou Su <Jinzhou.Su@amd.com>,
	Xiaojian Du <Xiaojian.Du@amd.com>,
	linux-kernel@vger.kernel.org, x86@kernel.org,
	Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH 03/19] ACPI: CPPC: add cppc enable register function
Date: Wed, 8 Sep 2021 18:21:48 -0600	[thread overview]
Message-ID: <e23d49d3-1591-bd12-549a-efd2a1f28dea@linuxfoundation.org> (raw)
In-Reply-To: <20210908150001.3702552-4-ray.huang@amd.com>

On 9/8/21 8:59 AM, Huang Rui wrote:
> From: Jinzhou Su <Jinzhou.Su@amd.com>
> 
> Export the cppc enable register function for future use.

This patch also adds a new function. How about saying something about
adding a new function.

> 
> 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.

Please make this more descriptive - does it write to register
What is the behavior in error paths etc.

> + * @cpu: CPU for which to enable CPPC register.
> + * @enable: enable field to write into share memory.

What should this be? What are the valid values to write?
Also aren't we adding this to header file where prtotype
is defined these days?

> + *
> + * 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;
> +	}
> +

Don't we need to do some error checking on input args? What is the
valid range for cpu and enbale?

> +	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);

Could this fail?

> +		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;
> 

thanks,
-- Shuah

  parent reply	other threads:[~2021-09-09  0:21 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-08 14:59 [PATCH 00/19] cpufreq: introduce a new AMD CPU frequency control mechanism Huang Rui
2021-09-08 14:59 ` [PATCH 01/19] x86/cpufreatures: add AMD CPPC extension feature flag Huang Rui
2021-09-08 20:00   ` Shuah Khan
2021-09-09  9:45     ` Huang Rui
2021-09-09 17:58   ` Borislav Petkov
2021-09-13  9:48     ` Huang Rui
2021-09-13 13:04       ` Borislav Petkov
2021-09-16  9:30         ` Huang Rui
2021-09-08 14:59 ` [PATCH 02/19] x86/msr: add AMD CPPC MSR definitions Huang Rui
2021-09-08 14:59 ` [PATCH 03/19] ACPI: CPPC: add cppc enable register function Huang Rui
2021-09-08 19:14   ` Fontenot, Nathan
2021-09-09  9:50     ` Huang Rui
2021-09-09  0:21   ` Shuah Khan [this message]
2021-09-09  9:58     ` Huang Rui
2021-09-08 14:59 ` [PATCH 04/19] cpufreq: amd: introduce a new amd pstate driver to support future processors Huang Rui
2021-09-09 15:01   ` Peter Zijlstra
2021-09-13  8:11     ` Huang Rui
2021-09-13  8:56       ` Peter Zijlstra
2021-09-13 10:54         ` Huang Rui
2021-09-13 11:56           ` Peter Zijlstra
2021-09-16 10:09             ` Huang Rui
2021-09-16 11:19               ` Rafael J. Wysocki
2021-09-17  3:41                 ` Huang Rui
2021-09-09 15:03   ` Peter Zijlstra
2021-09-13 11:55     ` Huang Rui
2021-09-09 19:31   ` Fontenot, Nathan
2021-09-13 11:18     ` Huang Rui
2021-09-08 14:59 ` [PATCH 05/19] cpufreq: amd: add fast switch function for amd-pstate module Huang Rui
2021-09-08 14:59 ` [PATCH 06/19] cpufreq: amd: add acpi cppc function as the backend for legacy processors Huang Rui
2021-09-08 14:59 ` [PATCH 07/19] cpufreq: amd: add trace for amd-pstate module Huang Rui
2021-09-08 14:59 ` [PATCH 08/19] cpufreq: amd: add boost mode support for amd-pstate Huang Rui
2021-09-08 18:24   ` Fontenot, Nathan
2021-09-09 10:12     ` Huang Rui
2021-09-08 14:59 ` [PATCH 09/19] cpufreq: amd: add amd-pstate checking support check attribute Huang Rui
2021-09-08 14:59 ` [PATCH 10/19] cpufreq: amd: add amd-pstate frequencies attributes Huang Rui
2021-09-08 18:13   ` Fontenot, Nathan
2021-09-08 14:59 ` [PATCH 11/19] cpufreq: amd: add amd-pstate performance attributes Huang Rui
2021-09-08 18:20   ` Fontenot, Nathan
2021-09-08 14:59 ` [PATCH 12/19] cpupower: add AMD P-state capability flag Huang Rui
2021-09-08 14:59 ` [PATCH 13/19] cpupower: add the function to check amd-pstate enabled Huang Rui
2021-09-09 22:16   ` Shuah Khan
2021-09-13 11:29     ` Huang Rui
2021-09-08 14:59 ` [PATCH 14/19] cpupower: initial AMD P-state capability Huang Rui
2021-09-09 22:16   ` Shuah Khan
2021-09-13 12:58     ` Huang Rui
2021-09-08 14:59 ` [PATCH 15/19] cpupower: add amd-pstate sysfs entries into libcpufreq Huang Rui
2021-09-09 22:26   ` Shuah Khan
2021-09-16  8:47     ` Huang Rui
2021-09-08 14:59 ` [PATCH 16/19] cpupower: enable boost state support for amd-pstate module Huang Rui
2021-09-08 17:32   ` Fontenot, Nathan
2021-09-09  9:59     ` Huang Rui
2021-09-09 22:42   ` Shuah Khan
2021-09-16  9:27     ` Huang Rui
2021-09-08 14:59 ` [PATCH 17/19] cpupower: add amd-pstate get data function to query the info Huang Rui
2021-09-09 22:45   ` Shuah Khan
2021-09-08 15:00 ` [PATCH 18/19] cpupower: print amd-pstate information on cpupower Huang Rui
2021-09-09 22:46   ` Shuah Khan
2021-09-16  9:29     ` Huang Rui
2021-09-08 15:00 ` [PATCH 19/19] Documentation: amd-pstate: add amd-pstate driver introduction Huang Rui

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e23d49d3-1591-bd12-549a-efd2a1f28dea@linuxfoundation.org \
    --to=skhan@linuxfoundation.org \
    --cc=Jinzhou.Su@amd.com \
    --cc=Xiaojian.Du@amd.com \
    --cc=alexander.deucher@amd.com \
    --cc=bp@suse.de \
    --cc=deepak.sharma@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mario.limonciello@amd.com \
    --cc=mingo@kernel.org \
    --cc=nathan.fontenot@amd.com \
    --cc=rafael.j.wysocki@intel.com \
    --cc=ray.huang@amd.com \
    --cc=viresh.kumar@linaro.org \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH 03/19] ACPI: CPPC: add cppc enable register function' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).