RE: [PATCH 11/12] cpufreq: amd-pstate: add ACPI disabled check

[Yuan, Perry]

[AMD Official Use Only - General]

Hi Nathan.

> -----Original Message-----
> From: Fontenot, Nathan <Nathan.Fontenot@xxxxxxx>
> Sent: Friday, July 8, 2022 3:46 AM
> To: Yuan, Perry <Perry.Yuan@xxxxxxx>; rafael.j.wysocki@xxxxxxxxx;
> viresh.kumar@xxxxxxxxxx; Huang, Ray <Ray.Huang@xxxxxxx>; Rafael J.
> Wysocki <rafael@xxxxxxxxxx>; Len Brown <lenb@xxxxxxxxxx>; linux-
> acpi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
> pm@xxxxxxxxxxxxxxx
> Cc: Sharma, Deepak <Deepak.Sharma@xxxxxxx>; Limonciello, Mario
> <Mario.Limonciello@xxxxxxx>; Fontenot, Nathan
> <Nathan.Fontenot@xxxxxxx>; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; Su, Jinzhou (Joe) <Jinzhou.Su@xxxxxxx>;
> Huang, Shimmer <Shimmer.Huang@xxxxxxx>; Du, Xiaojian
> <Xiaojian.Du@xxxxxxx>; Meng, Li (Jassmine) <Li.Meng@xxxxxxx>
> Subject: Re: [PATCH 11/12] cpufreq: amd-pstate: add ACPI disabled check
> 
> On 7/7/22 12:01, Perry Yuan wrote:
> > Add acpi function check in case ACPI is not enabled, that will cause
> > pstate driver failed to call cppc acpi to change perf or update epp
> > value for shared memory solution processors.
> >
> > When CPPC is invalid, warning log will be needed to be printed to tell
> > user what is wrong.
> >
> > Signed-off-by: Perry Yuan <Perry.Yuan@xxxxxxx>
> > ---
> >  drivers/acpi/cppc_acpi.c     | 3 +++
> >  drivers/cpufreq/amd-pstate.c | 2 +-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index
> > 6ff1901d7d43..17d67e3ededf 100644
> > --- a/drivers/acpi/cppc_acpi.c
> > +++ b/drivers/acpi/cppc_acpi.c
> > @@ -424,6 +424,9 @@ bool acpi_cpc_valid(void)
> >  	struct cpc_desc *cpc_ptr;
> >  	int cpu;
> >
> > +	if (acpi_disabled)
> > +		return false> +
> 
> This seems ok, the only other places I find that call acpi_cpc_valid() also
> check for acpi_disabled.
> 
> If the acpi_disabled check is added to acpi_cpc_valid() the other calling sites
> should be updated to remove their check for acpi_disabled.
> 
> -Nathan

You are correct. It needs to remove the same check code from other driver file.
Will add this feedback into V2 patch. 

Perry. 

> 
> >  	for_each_present_cpu(cpu) {
> >  		cpc_ptr = per_cpu(cpc_desc_ptr, cpu);
> >  		if (!cpc_ptr)
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index b54b3b559993..6d81a9a94dde
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -684,7 +684,7 @@ static int __init amd_pstate_init(void)
> >  		return -ENODEV;
> >
> >  	if (!acpi_cpc_valid()) {
> > -		pr_debug("the _CPC object is not present in SBIOS\n");
> > +		pr_warn_once("the _CPC object is not present in SBIOS or
> ACPI
> > +disabled\n");
> >  		return -ENODEV;
> >  	}
> >