Re: [PATCH v2 10/21] cpufreq: amd: add amd-pstate checking support check attribute

From: "Fontenot, Nathan" <Nathan.Fontenot@amd.com>
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>,
	Peter Zijlstra <peterz@infradead.org>,
	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
Subject: Re: [PATCH v2 10/21] cpufreq: amd: add amd-pstate checking support check attribute
Date: Tue, 28 Sep 2021 16:24:11 -0500	[thread overview]
Message-ID: <93e1079f-7450-5c5d-f1f8-7d607899343c@amd.com> (raw)
In-Reply-To: <20210926090605.3556134-11-ray.huang@amd.com>

On 9/26/2021 4:05 AM, Huang Rui wrote:
> The amd-pstate hardware support check will be needed by cpupower to know
> whether amd-pstate is enabled and supported.
>

Is this needed?

If acpi_cpc_valid() is false, then the amd_pstate driver would not have been
loaded (see patch 5/21, module init routine will return -ENODEV). If this
driver isn't loaded then this sysfs file won't get created.

Shouldn't the cpupower command just check the scaling_driver in sysfs to see
if it's amd_pstate?

-Nathan

> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 847ba00e3351..74f896232d5a 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -517,6 +517,19 @@ static int amd_pstate_cpu_exit(struct cpufreq_policy *policy)
>  	return 0;
>  }
>  
> +static ssize_t show_is_amd_pstate_enabled(struct cpufreq_policy *policy,
> +					char *buf)
> +{
> +	return sprintf(&buf[0], "%d\n", acpi_cpc_valid() ?  1 : 0);
> +}
> +
> +cpufreq_freq_attr_ro(is_amd_pstate_enabled);
> +
> +static struct freq_attr *amd_pstate_attr[] = {
> +	&is_amd_pstate_enabled,
> +	NULL,
> +};
> +
>  static struct cpufreq_driver amd_pstate_driver = {
>  	.flags		= CPUFREQ_CONST_LOOPS | CPUFREQ_NEED_UPDATE_LIMITS,
>  	.verify		= amd_pstate_verify,
> @@ -525,6 +538,7 @@ static struct cpufreq_driver amd_pstate_driver = {
>  	.exit		= amd_pstate_cpu_exit,
>  	.set_boost	= amd_pstate_set_boost,
>  	.name		= "amd-pstate",
> +	.attr		= amd_pstate_attr,
>  };
>  
>  static int __init amd_pstate_init(void)
> -- 
> 2.25.1
> 