Messages in this thread | | | From | "Huang, Ray" <> | Subject | RE: [PATCH v2 04/21] ACPI: CPPC: add cppc enable register function | Date | Wed, 13 Oct 2021 12:28:40 +0000 |
| |
[AMD Official Use Only]
Hi all,
Sorry to late response, I am just back from vacation.
> -----Original Message----- > From: Fontenot, Nathan <Nathan.Fontenot@amd.com> > Sent: Wednesday, September 29, 2021 1:06 AM > To: Huang, Ray <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: Sharma, Deepak <Deepak.Sharma@amd.com>; Deucher, Alexander > <Alexander.Deucher@amd.com>; Limonciello, Mario > <Mario.Limonciello@amd.com>; Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>; > Du, Xiaojian <Xiaojian.Du@amd.com>; linux-kernel@vger.kernel.org; > x86@kernel.org > Subject: Re: [PATCH v2 04/21] ACPI: CPPC: add cppc enable register function > > On 9/26/2021 4:05 AM, Huang Rui wrote: > > From: Jinzhou Su <Jinzhou.Su@amd.com> > > > > Add a new function to enable CPPC feature. This function > > will write Continuous Performance Control package > > EnableRegister field on the processor. > > > > Signed-off-by: Jinzhou Su <Jinzhou.Su@amd.com> > > Signed-off-by: Huang Rui <ray.huang@amd.com> > > --- > > drivers/acpi/cppc_acpi.c | 48 > ++++++++++++++++++++++++++++++++++++++++ > > include/acpi/cppc_acpi.h | 5 +++++ > > 2 files changed, 53 insertions(+) > > > > diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c > > index 2efe2ba97d96..b285960c35e7 100644 > > --- a/drivers/acpi/cppc_acpi.c > > +++ b/drivers/acpi/cppc_acpi.c > > @@ -1220,6 +1220,54 @@ 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 on the processor by writing the > > + * Continuous Performance Control package EnableRegister feild. > > + * @cpu: CPU for which to enable CPPC register. > > + * @enable: 0 - disable, 1 - enable CPPC feature on the processor. > > + * > > + * Return: 0 for success, -ERRNO or -EIO otherwise. > > + */ > > +int cppc_set_enable(int cpu, u32 enable) > > This should take a bool instead of u32 for enable, you can only enable > or diable cppc. The only caller I see is in patch 7/21 in which the > enable arg is already a bool that's converted to a u32. This also allows > for the removal of the enable value check. >
Yes, the bool type should be better than u32 here. Will update it in V3.
> You should consider merging this patch with patch 7/21. This patch adds > the cppc_set_enable() routine but has no users. The only caller I find is > in patch 7/21. >
I am looking back again, this patch is to provide the new helper in cppc_acpi library under ACPI subsystem. However, patch 7 is to enable the shared memory APIs in amd-pstate driver. They are actually the different function implementations. I prefer using the separated patch here.
Thanks, Ray
| |