Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 03/19] ACPI: CPPC: add cppc enable register function
To:     Huang Rui <ray.huang@amd.com>,
        "Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        Shuah Khan <skhan@linuxfoundation.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>,
        Jinzhou Su <Jinzhou.Su@amd.com>,
        Xiaojian Du <Xiaojian.Du@amd.com>,
        linux-kernel@vger.kernel.org, x86@kernel.org
References: <20210908150001.3702552-1-ray.huang@amd.com>
 <20210908150001.3702552-4-ray.huang@amd.com>
From:   "Fontenot, Nathan" <Nathan.Fontenot@amd.com>
Message-ID: <53962105-07cb-d964-28d0-1cc4d2e7fe8b@amd.com>
Date:   Wed, 8 Sep 2021 14:14:37 -0500
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
In-Reply-To: <20210908150001.3702552-4-ray.huang@amd.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a2ppN0hjSkRvNnhma3ZJN2hwZk1ZN3hRV3dVb2p4VVpmVkVKWTBwRkxGMHJX?=
 =?utf-8?B?NVVEcHJ6cjFCamxheGNaQ2tLZTA5dWNHN0xUYTBJQWUwR2NwV0tDRUhNWmtY?=
 =?utf-8?B?RmJ0bnYrZDc3UmpiVkUxWUJrSGdtOCsvQnM3d3N3Mmt1TnlBQWdwWXkrOElk?=
 =?utf-8?B?OWUrb2VndWIvbFdMLzRta2p0cGZVblp4aTh0Uk9aa010K21VMUx0dkRuWGx2?=
 =?utf-8?B?Y3k0bE9WdXMwakNWSGprT3VLVGVQYi9XUk4zMXc1N2thOGJlajVlUmRNS0xX?=
 =?utf-8?B?MjhJRHRBL3JXVmlIN0Y0TjRhTTZiSk5wd21rNnRqWXNoQXV1S2I5djh0SGJD?=
 =?utf-8?B?WUNmUkhhNmVMVVlHS1RBc3lSZEdpSHZkTXJrN3NJRitCVVlBSmJKak5iaHBY?=
 =?utf-8?B?QUt3bDBkUk9La1VzdkluZ0pKY1VLby8wVTF0bzRPeTd1ZmtjYjdTMXdIODFC?=
 =?utf-8?B?Nm9DdEpUT0FWR29GRFNuTHBYakkyVysyMlB6RER6cG13VUVEdW5lMm9pUzhs?=
 =?utf-8?B?ZWFhazk0NzhRNXZpaWFQMi9lK1NxcGRGYmVZUWlwWWlpOEpCWmg1UXFweU4y?=
 =?utf-8?B?RHlEUUIyQTdvUGVhVzhPSUFEUitLWTFwc2dUc3gzTUU1aTVjNThuOHVlbmlP?=
 =?utf-8?B?alZHQU1jejFtMzJvRXJ2S2U0bC82ZlFnVjBYZjk5QjBJQnhEMzNlcjdhL1hM?=
 =?utf-8?B?dldSVkhBalBlRXhMejRZSlBGc1pmZlRZTzBUT21tcXdlb3ZydUFTWUFjNDAr?=
 =?utf-8?B?SXI3eW9YeGZrRUp5OW96Znk3M0orM0dxcGhxZEt6eTc2OVAxZDRaVDYwU3Jq?=
 =?utf-8?B?Z3hvSXRIemJid0dFY1VKOTFEUTVSblVKTVVBbmx2Rnk3R2l4MzBkODBpQUsv?=
 =?utf-8?B?bE9KMmlPc3V4UWRlZktSN1Z1bGdVQjhLRkYvSXRJYTlZSDRFUVFJMDNFMDJt?=
 =?utf-8?B?OGpobng2d1V0TVd0dzAyek9ZMFdYNGs2ZElneFhHRVBuVEN0Z3UrSlY1UlBI?=
 =?utf-8?B?TGh5eHB3eUlTNkVKUEtKWXMySDVpazJ0QURyUThDUXViQ0ZpNC9uVzhQc0Fu?=
 =?utf-8?B?WVJFaWxwUGVQNlZUYXpXaTloeGRJYk1xNjF2SzBERk9nRk81WVR5Q3hpN3Fn?=
 =?utf-8?B?Y3JSZ1JhM210YTZibmlQVm14OVoycEVZR3BIUXJrK2hJYmJFMWRwb1ltdkJ4?=
 =?utf-8?B?Zm5vQko1Q0ZObFNlT0N1emk2STJyalBBZG5xR2p3MW96M1N2cWRnY2prSUhV?=
 =?utf-8?B?MERjeWg3VEc4U0R4ZncrcEJ2SFZNajBMWWl6Q09DTEk3REc4ckd3d3VtSVBK?=
 =?utf-8?B?ZSt3SFNiUXpvdXRkamc5RTFKN2hzb0VzLytBQXBQejFHWG5rbS9xTHEzVTFP?=
 =?utf-8?B?MWxUMm1Ic2FBQktYS0s4N3Zlc2VDSzByempIbGFhT3dGSFVvQ1gxWEltTW1y?=
 =?utf-8?B?akZhcVk2dnNwRW1sWkl1ZE1uRjhUK3N0SFRQQ2tRQkhDQVVIQnc5S2twOTN5?=
 =?utf-8?B?ZE5uN3U4Q2FrT25iMFFFRi9pQnNMVEFZUzE3OW5yZFIzaHNnam8vZFdoMkFI?=
 =?utf-8?B?TzlRSStWb1ZZYVBtZzA4NFpsYVJ4Q3htdFA1Q21DZEZCUEVMcHdMREh6L2da?=
 =?utf-8?B?R09Qcm9xcHpnZmM4UWVuN1lManZnRWxFQVMrWm5iNWpZbCtQVUt2cE9PQzdx?=
 =?utf-8?B?OGhvc1ovR0RNVnFyazNJOHh2UEpYVitueTVxREZOTllzaEpHOUNrNHlySUhl?=
 =?utf-8?Q?km4D2gtGpK2/TILG7xqyYGJG0T3L/dlHURTQ2IF?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 488a52d7-6b08-44b3-afe4-08d972fce843
X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB4720.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Sep 2021 19:14:40.5799
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: kpsTuIxNBSxxhuHRke+gQNhPV2NFM/nRJvtCy0EXIyNTy9i/v+dNuquLje1To2a88myg7hS1FI6+3UW+SSicWg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR12MB2480
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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
>