Re: [PATCH 2/3] cpufreq: intel_pstate: wrong flag checking for no_acpi_perf

[Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>]

On Sat, 2015-11-14 at 01:46 +0100, Rafael J. Wysocki wrote:
> On Monday, November 09, 2015 06:58:10 PM Srinivas Pandruvada wrote:
> > The checking of no_acpi_perf flag is wrong. Since there is null
> > check in acpi_processor_unregister_performance this doesn't cause
> > any issue when this flag is 0.
> > 
> > Signed-off-by: Srinivas Pandruvada <
> > srinivas.pandruvada@linux.intel.com>
> > ---
> >  drivers/cpufreq/intel_pstate.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cpufreq/intel_pstate.c
> > b/drivers/cpufreq/intel_pstate.c
> > index a0b8180..69dc9b5 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -328,7 +328,7 @@ static int intel_pstate_exit_perf_limits(struct
> > cpufreq_policy *policy)
> >  {
> >  	struct cpudata *cpu;
> >  
> > -	if (!no_acpi_perf)
> > +	if (no_acpi_perf)
> >  		return 0;
> >  
> >  	cpu = all_cpu_data[policy->cpu];
> 
> While the patch is correct, the changelog doesn't make a lot of sense
> to me
> to be honest.
> 
> In addition to that, the cpu variable here is actually not used, so
> what
> about doing something like the below instead here?
Sure, I will resubmit this.

Thanks,
Srinivas

---
>  drivers/cpufreq/intel_pstate.c |    6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> Index: linux-pm/drivers/cpufreq/intel_pstate.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> +++ linux-pm/drivers/cpufreq/intel_pstate.c
> @@ -326,13 +326,9 @@ static int intel_pstate_init_perf_limits
>  
>  static int intel_pstate_exit_perf_limits(struct cpufreq_policy
> *policy)
>  {
> -	struct cpudata *cpu;
> -
>  	if (!no_acpi_perf)
> -		return 0;
> +		acpi_processor_unregister_performance(policy->cpu);
>  
> -	cpu = all_cpu_data[policy->cpu];
> -	acpi_processor_unregister_performance(policy->cpu);
>  	return 0;
>  }
>  
>