[PATCH 0/2] ACPI / cpufreq: ACPI processor driver and ACPI cpufreq driver cleanups

[PATCH 0/2] ACPI / cpufreq: ACPI processor driver and ACPI cpufreq driver cleanups

[Rafael J. Wysocki]

Hi,

The following two patches clean up a couple of things in the ACPI processor
driver and the ACPI cpufreq driver:

[1/2] Drop the unused first argument of acpi_processor_unregister_performance().
[2/2] Drop the now redundant acpi_data pointer from acpi_cpufreq_data.

Both patches on top of the current linux-next branch of the linux-pm.git tree.

Thanks!


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH 1/2] ACPI / processor: Drop an unused argument of a cleanup routine

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

acpi_processor_unregister_performance() actually doesn't use its
first argument, so drop it and update the callers accordingly.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/processor_perflib.c    |    4 +---
 drivers/cpufreq/acpi-cpufreq.c      |    5 ++---
 drivers/cpufreq/e_powersaver.c      |    2 +-
 drivers/cpufreq/ia64-acpi-cpufreq.c |    5 ++---
 drivers/cpufreq/powernow-k7.c       |    4 ++--
 drivers/cpufreq/powernow-k8.c       |    5 ++---
 drivers/xen/xen-acpi-processor.c    |    4 ++--
 include/acpi/processor.h            |    5 +----
 8 files changed, 13 insertions(+), 21 deletions(-)

Index: linux-pm/drivers/acpi/processor_perflib.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_perflib.c
+++ linux-pm/drivers/acpi/processor_perflib.c
@@ -780,9 +780,7 @@ acpi_processor_register_performance(stru
 
 EXPORT_SYMBOL(acpi_processor_register_performance);
 
-void
-acpi_processor_unregister_performance(struct acpi_processor_performance
-				      *performance, unsigned int cpu)
+void acpi_processor_unregister_performance(unsigned int cpu)
 {
 	struct acpi_processor *pr;
 
Index: linux-pm/include/acpi/processor.h
===================================================================
--- linux-pm.orig/include/acpi/processor.h
+++ linux-pm/include/acpi/processor.h
@@ -228,10 +228,7 @@ extern int acpi_processor_preregister_pe
 
 extern int acpi_processor_register_performance(struct acpi_processor_performance
 					       *performance, unsigned int cpu);
-extern void acpi_processor_unregister_performance(struct
-						  acpi_processor_performance
-						  *performance,
-						  unsigned int cpu);
+extern void acpi_processor_unregister_performance(unsigned int cpu);
 
 /* note: this locks both the calling module and the processor module
          if a _PPC object exists, rmmod is disallowed then */
Index: linux-pm/drivers/cpufreq/acpi-cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/acpi-cpufreq.c
+++ linux-pm/drivers/cpufreq/acpi-cpufreq.c
@@ -842,7 +842,7 @@ static int acpi_cpufreq_cpu_init(struct
 err_freqfree:
 	kfree(data->freq_table);
 err_unreg:
-	acpi_processor_unregister_performance(perf, cpu);
+	acpi_processor_unregister_performance(cpu);
 err_free_mask:
 	free_cpumask_var(data->freqdomain_cpus);
 err_free:
@@ -860,8 +860,7 @@ static int acpi_cpufreq_cpu_exit(struct
 
 	if (data) {
 		policy->driver_data = NULL;
-		acpi_processor_unregister_performance(data->acpi_data,
-						      data->acpi_perf_cpu);
+		acpi_processor_unregister_performance(data->acpi_perf_cpu);
 		free_cpumask_var(data->freqdomain_cpus);
 		kfree(data->freq_table);
 		kfree(data);
Index: linux-pm/drivers/cpufreq/e_powersaver.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/e_powersaver.c
+++ linux-pm/drivers/cpufreq/e_powersaver.c
@@ -78,7 +78,7 @@ static int eps_acpi_init(void)
 static int eps_acpi_exit(struct cpufreq_policy *policy)
 {
 	if (eps_acpi_cpu_perf) {
-		acpi_processor_unregister_performance(eps_acpi_cpu_perf, 0);
+		acpi_processor_unregister_performance(0);
 		free_cpumask_var(eps_acpi_cpu_perf->shared_cpu_map);
 		kfree(eps_acpi_cpu_perf);
 		eps_acpi_cpu_perf = NULL;
Index: linux-pm/drivers/cpufreq/ia64-acpi-cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/ia64-acpi-cpufreq.c
+++ linux-pm/drivers/cpufreq/ia64-acpi-cpufreq.c
@@ -313,7 +313,7 @@ acpi_cpufreq_cpu_init (
  err_freqfree:
 	kfree(data->freq_table);
  err_unreg:
-	acpi_processor_unregister_performance(&data->acpi_data, cpu);
+	acpi_processor_unregister_performance(cpu);
  err_free:
 	kfree(data);
 	acpi_io_data[cpu] = NULL;
@@ -332,8 +332,7 @@ acpi_cpufreq_cpu_exit (
 
 	if (data) {
 		acpi_io_data[policy->cpu] = NULL;
-		acpi_processor_unregister_performance(&data->acpi_data,
-		                                      policy->cpu);
+		acpi_processor_unregister_performance(policy->cpu);
 		kfree(data);
 	}
 
Index: linux-pm/drivers/cpufreq/powernow-k7.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/powernow-k7.c
+++ linux-pm/drivers/cpufreq/powernow-k7.c
@@ -421,7 +421,7 @@ static int powernow_acpi_init(void)
 	return 0;
 
 err2:
-	acpi_processor_unregister_performance(acpi_processor_perf, 0);
+	acpi_processor_unregister_performance(0);
 err1:
 	free_cpumask_var(acpi_processor_perf->shared_cpu_map);
 err05:
@@ -661,7 +661,7 @@ static int powernow_cpu_exit(struct cpuf
 {
 #ifdef CONFIG_X86_POWERNOW_K7_ACPI
 	if (acpi_processor_perf) {
-		acpi_processor_unregister_performance(acpi_processor_perf, 0);
+		acpi_processor_unregister_performance(0);
 		free_cpumask_var(acpi_processor_perf->shared_cpu_map);
 		kfree(acpi_processor_perf);
 	}
Index: linux-pm/drivers/cpufreq/powernow-k8.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/powernow-k8.c
+++ linux-pm/drivers/cpufreq/powernow-k8.c
@@ -795,7 +795,7 @@ err_out_mem:
 	kfree(powernow_table);
 
 err_out:
-	acpi_processor_unregister_performance(&data->acpi_data, data->cpu);
+	acpi_processor_unregister_performance(data->cpu);
 
 	/* data->acpi_data.state_count informs us at ->exit()
 	 * whether ACPI was used */
@@ -863,8 +863,7 @@ static int fill_powernow_table_fidvid(st
 static void powernow_k8_cpu_exit_acpi(struct powernow_k8_data *data)
 {
 	if (data->acpi_data.state_count)
-		acpi_processor_unregister_performance(&data->acpi_data,
-				data->cpu);
+		acpi_processor_unregister_performance(data->cpu);
 	free_cpumask_var(data->acpi_data.shared_cpu_map);
 }
 
Index: linux-pm/drivers/xen/xen-acpi-processor.c
===================================================================
--- linux-pm.orig/drivers/xen/xen-acpi-processor.c
+++ linux-pm/drivers/xen/xen-acpi-processor.c
@@ -563,7 +563,7 @@ err_unregister:
 	for_each_possible_cpu(i) {
 		struct acpi_processor_performance *perf;
 		perf = per_cpu_ptr(acpi_perf_data, i);
-		acpi_processor_unregister_performance(perf, i);
+		acpi_processor_unregister_performance(i);
 	}
 err_out:
 	/* Freeing a NULL pointer is OK: alloc_percpu zeroes. */
@@ -582,7 +582,7 @@ static void __exit xen_acpi_processor_ex
 	for_each_possible_cpu(i) {
 		struct acpi_processor_performance *perf;
 		perf = per_cpu_ptr(acpi_perf_data, i);
-		acpi_processor_unregister_performance(perf, i);
+		acpi_processor_unregister_performance(i);
 	}
 	free_acpi_perf_data();
 }

Re: [PATCH 1/2] ACPI / processor: Drop an unused argument of a cleanup routine

[Pan Xinhui]

hi, Rafael
	thanks for your nice work :)
On 2015年07月18日 09:14, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> acpi_processor_unregister_performance() actually doesn't use its
> first argument, so drop it and update the callers accordingly.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/acpi/processor_perflib.c    |    4 +---
>  drivers/cpufreq/acpi-cpufreq.c      |    5 ++---
>  drivers/cpufreq/e_powersaver.c      |    2 +-
>  drivers/cpufreq/ia64-acpi-cpufreq.c |    5 ++---
>  drivers/cpufreq/powernow-k7.c       |    4 ++--
>  drivers/cpufreq/powernow-k8.c       |    5 ++---
>  drivers/xen/xen-acpi-processor.c    |    4 ++--
>  include/acpi/processor.h            |    5 +----
>  8 files changed, 13 insertions(+), 21 deletions(-)
> 

I wish I can test this patch. However I only have x86 hardware, not all codes can be tested on my side.

> Index: linux-pm/drivers/xen/xen-acpi-processor.c
> ===================================================================
> --- linux-pm.orig/drivers/xen/xen-acpi-processor.c
> +++ linux-pm/drivers/xen/xen-acpi-processor.c
> @@ -563,7 +563,7 @@ err_unregister:
>  	for_each_possible_cpu(i) {
>  		struct acpi_processor_performance *perf;
>  		perf = per_cpu_ptr(acpi_perf_data, i);
> -		acpi_processor_unregister_performance(perf, i);
> +		acpi_processor_unregister_performance(i);
>  	}
>  err_out:
>  	/* Freeing a NULL pointer is OK: alloc_percpu zeroes. */
> @@ -582,7 +582,7 @@ static void __exit xen_acpi_processor_ex
>  	for_each_possible_cpu(i) {
>  		struct acpi_processor_performance *perf;
>  		perf = per_cpu_ptr(acpi_perf_data, i);
> -		acpi_processor_unregister_performance(perf, i);
> +		acpi_processor_unregister_performance(i);


>  	}
>  	free_acpi_perf_data();
>  }
> 

After a simple review, in functions above *perf* is not used anymore, can we just change the codes like below
	@@ -580,9 +580,7 @@ static void __exit xen_acpi_processor_exit(void)
        kfree(acpi_id_present);
        kfree(acpi_id_cst_present);
        for_each_possible_cpu(i) {
-               struct acpi_processor_performance *perf;
-               perf = per_cpu_ptr(acpi_perf_data, i);
-               acpi_processor_unregister_performance(perf, i);
+               acpi_processor_unregister_performance(i);
        }
        free_acpi_perf_data();

thanks
xinhui
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] ACPI / processor: Drop an unused argument of a cleanup routine

[Pan Xinhui]

hi, Rafael
	thanks for your nice work :)
On 2015年07月18日 09:14, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> acpi_processor_unregister_performance() actually doesn't use its
> first argument, so drop it and update the callers accordingly.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/acpi/processor_perflib.c    |    4 +---
>  drivers/cpufreq/acpi-cpufreq.c      |    5 ++---
>  drivers/cpufreq/e_powersaver.c      |    2 +-
>  drivers/cpufreq/ia64-acpi-cpufreq.c |    5 ++---
>  drivers/cpufreq/powernow-k7.c       |    4 ++--
>  drivers/cpufreq/powernow-k8.c       |    5 ++---
>  drivers/xen/xen-acpi-processor.c    |    4 ++--
>  include/acpi/processor.h            |    5 +----
>  8 files changed, 13 insertions(+), 21 deletions(-)
> 

I wish I can test this patch. However I only have x86 hardware, not all codes can be tested on my side.

> Index: linux-pm/drivers/xen/xen-acpi-processor.c
> ===================================================================
> --- linux-pm.orig/drivers/xen/xen-acpi-processor.c
> +++ linux-pm/drivers/xen/xen-acpi-processor.c
> @@ -563,7 +563,7 @@ err_unregister:
>  	for_each_possible_cpu(i) {
>  		struct acpi_processor_performance *perf;
>  		perf = per_cpu_ptr(acpi_perf_data, i);
> -		acpi_processor_unregister_performance(perf, i);
> +		acpi_processor_unregister_performance(i);
>  	}
>  err_out:
>  	/* Freeing a NULL pointer is OK: alloc_percpu zeroes. */
> @@ -582,7 +582,7 @@ static void __exit xen_acpi_processor_ex
>  	for_each_possible_cpu(i) {
>  		struct acpi_processor_performance *perf;
>  		perf = per_cpu_ptr(acpi_perf_data, i);
> -		acpi_processor_unregister_performance(perf, i);
> +		acpi_processor_unregister_performance(i);


>  	}
>  	free_acpi_perf_data();
>  }
> 

After a simple review, in functions above *perf* is not used anymore, can we just change the codes like below
	@@ -580,9 +580,7 @@ static void __exit xen_acpi_processor_exit(void)
        kfree(acpi_id_present);
        kfree(acpi_id_cst_present);
        for_each_possible_cpu(i) {
-               struct acpi_processor_performance *perf;
-               perf = per_cpu_ptr(acpi_perf_data, i);
-               acpi_processor_unregister_performance(perf, i);
+               acpi_processor_unregister_performance(i);
        }
        free_acpi_perf_data();

thanks
xinhui

Re: [PATCH 1/2] ACPI / processor: Drop an unused argument of a cleanup routine

[Rafael J. Wysocki]

On Monday, July 20, 2015 09:45:30 AM Pan Xinhui wrote:
> hi, Rafael
> 	thanks for your nice work :)
> On 2015年07月18日 09:14, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > acpi_processor_unregister_performance() actually doesn't use its
> > first argument, so drop it and update the callers accordingly.
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/acpi/processor_perflib.c    |    4 +---
> >  drivers/cpufreq/acpi-cpufreq.c      |    5 ++---
> >  drivers/cpufreq/e_powersaver.c      |    2 +-
> >  drivers/cpufreq/ia64-acpi-cpufreq.c |    5 ++---
> >  drivers/cpufreq/powernow-k7.c       |    4 ++--
> >  drivers/cpufreq/powernow-k8.c       |    5 ++---
> >  drivers/xen/xen-acpi-processor.c    |    4 ++--
> >  include/acpi/processor.h            |    5 +----
> >  8 files changed, 13 insertions(+), 21 deletions(-)
> > 
> 
> I wish I can test this patch. However I only have x86 hardware, not all
> codes can be tested on my side.

Possible build errors should be sorted out by the 0-day testing and the
functional part will be OK if the name of the remaining argument is not
changed in any spot.


> > Index: linux-pm/drivers/xen/xen-acpi-processor.c
> > ===================================================================
> > --- linux-pm.orig/drivers/xen/xen-acpi-processor.c
> > +++ linux-pm/drivers/xen/xen-acpi-processor.c
> > @@ -563,7 +563,7 @@ err_unregister:
> >  	for_each_possible_cpu(i) {
> >  		struct acpi_processor_performance *perf;
> >  		perf = per_cpu_ptr(acpi_perf_data, i);
> > -		acpi_processor_unregister_performance(perf, i);
> > +		acpi_processor_unregister_performance(i);
> >  	}
> >  err_out:
> >  	/* Freeing a NULL pointer is OK: alloc_percpu zeroes. */
> > @@ -582,7 +582,7 @@ static void __exit xen_acpi_processor_ex
> >  	for_each_possible_cpu(i) {
> >  		struct acpi_processor_performance *perf;
> >  		perf = per_cpu_ptr(acpi_perf_data, i);
> > -		acpi_processor_unregister_performance(perf, i);
> > +		acpi_processor_unregister_performance(i);
> 
> 
> >  	}
> >  	free_acpi_perf_data();
> >  }
> > 
> 
> After a simple review, in functions above *perf* is not used anymore, can we just change the codes like below
> 	@@ -580,9 +580,7 @@ static void __exit xen_acpi_processor_exit(void)
>         kfree(acpi_id_present);
>         kfree(acpi_id_cst_present);
>         for_each_possible_cpu(i) {
> -               struct acpi_processor_performance *perf;
> -               perf = per_cpu_ptr(acpi_perf_data, i);
> -               acpi_processor_unregister_performance(perf, i);
> +               acpi_processor_unregister_performance(i);
>         }
>         free_acpi_perf_data();

You're right, thanks!

I'll send an update shortly.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH 2/2] cpufreq: acpi-cpufreq: Drop acpi_data from struct acpi_cpufreq_data

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

After commit 8cfcfd39000d (acpi-cpufreq: Fix an ACPI perf unregister
issue) we store both a pointer to per-CPU data of the first policy
CPU and the number of that CPU which are redundant.

Since the CPU number has to be stored anyway for the unregistration,
the pointer to the CPU's per-CPU data may be dropped and we can
access the data in question via per_cpu_ptr().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/acpi-cpufreq.c |   26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

Index: linux-pm/drivers/cpufreq/acpi-cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/acpi-cpufreq.c
+++ linux-pm/drivers/cpufreq/acpi-cpufreq.c
@@ -65,7 +65,6 @@ enum {
 #define MSR_K7_HWCR_CPB_DIS	(1ULL << 25)
 
 struct acpi_cpufreq_data {
-	struct acpi_processor_performance *acpi_data;
 	struct cpufreq_frequency_table *freq_table;
 	unsigned int resume;
 	unsigned int cpu_feature;
@@ -201,7 +200,7 @@ static unsigned extract_io(u32 value, st
 	struct acpi_processor_performance *perf;
 	int i;
 
-	perf = data->acpi_data;
+	perf = per_cpu_ptr(acpi_perf_data, data->acpi_perf_cpu);
 
 	for (i = 0; i < perf->state_count; i++) {
 		if (value == perf->states[i].status)
@@ -220,7 +219,7 @@ static unsigned extract_msr(u32 msr, str
 	else
 		msr &= INTEL_MSR_RANGE;
 
-	perf = data->acpi_data;
+	perf = per_cpu_ptr(acpi_perf_data, data->acpi_perf_cpu);
 
 	cpufreq_for_each_entry(pos, data->freq_table)
 		if (msr == perf->states[pos->driver_data].status)
@@ -346,7 +345,7 @@ get_cur_val(const struct cpumask *mask,
 		break;
 	case SYSTEM_IO_CAPABLE:
 		cmd.type = SYSTEM_IO_CAPABLE;
-		perf = data->acpi_data;
+		perf = per_cpu_ptr(acpi_perf_data, data->acpi_perf_cpu);
 		cmd.addr.io.port = perf->control_register.address;
 		cmd.addr.io.bit_width = perf->control_register.bit_width;
 		break;
@@ -364,6 +363,7 @@ get_cur_val(const struct cpumask *mask,
 
 static unsigned int get_cur_freq_on_cpu(unsigned int cpu)
 {
+	struct acpi_processor_performance *perf;
 	struct acpi_cpufreq_data *data;
 	struct cpufreq_policy *policy;
 	unsigned int freq;
@@ -377,10 +377,11 @@ static unsigned int get_cur_freq_on_cpu(
 
 	data = policy->driver_data;
 	cpufreq_cpu_put(policy);
-	if (unlikely(!data || !data->acpi_data || !data->freq_table))
+	if (unlikely(!data || !data->freq_table))
 		return 0;
 
-	cached_freq = data->freq_table[data->acpi_data->state].frequency;
+	perf = per_cpu_ptr(acpi_perf_data, data->acpi_perf_cpu);
+	cached_freq = data->freq_table[perf->state].frequency;
 	freq = extract_freq(get_cur_val(cpumask_of(cpu), data), data);
 	if (freq != cached_freq) {
 		/*
@@ -419,12 +420,11 @@ static int acpi_cpufreq_target(struct cp
 	unsigned int next_perf_state = 0; /* Index into perf table */
 	int result = 0;
 
-	if (unlikely(data == NULL ||
-	     data->acpi_data == NULL || data->freq_table == NULL)) {
+	if (unlikely(data == NULL || data->freq_table == NULL)) {
 		return -ENODEV;
 	}
 
-	perf = data->acpi_data;
+	perf = per_cpu_ptr(acpi_perf_data, data->acpi_perf_cpu);
 	next_perf_state = data->freq_table[index].driver_data;
 	if (perf->state == next_perf_state) {
 		if (unlikely(data->resume)) {
@@ -487,8 +487,9 @@ out:
 static unsigned long
 acpi_cpufreq_guess_freq(struct acpi_cpufreq_data *data, unsigned int cpu)
 {
-	struct acpi_processor_performance *perf = data->acpi_data;
+	struct acpi_processor_performance *perf;
 
+	perf = per_cpu_ptr(acpi_perf_data, data->acpi_perf_cpu);
 	if (cpu_khz) {
 		/* search the closest match to cpu_khz */
 		unsigned int i;
@@ -677,18 +678,17 @@ static int acpi_cpufreq_cpu_init(struct
 		goto err_free;
 	}
 
-	data->acpi_data = per_cpu_ptr(acpi_perf_data, cpu);
+	perf = per_cpu_ptr(acpi_perf_data, cpu);
 	data->acpi_perf_cpu = cpu;
 	policy->driver_data = data;
 
 	if (cpu_has(c, X86_FEATURE_CONSTANT_TSC))
 		acpi_cpufreq_driver.flags |= CPUFREQ_CONST_LOOPS;
 
-	result = acpi_processor_register_performance(data->acpi_data, cpu);
+	result = acpi_processor_register_performance(perf, cpu);
 	if (result)
 		goto err_free_mask;
 
-	perf = data->acpi_data;
 	policy->shared_type = perf->shared_type;
 
 	/*

Re: [PATCH 0/2] ACPI / cpufreq: ACPI processor driver and ACPI cpufreq driver cleanups

[Viresh Kumar]

On 18-07-15, 03:13, Rafael J. Wysocki wrote:
> Hi,
> 
> The following two patches clean up a couple of things in the ACPI processor
> driver and the ACPI cpufreq driver:
> 
> [1/2] Drop the unused first argument of acpi_processor_unregister_performance().
> [2/2] Drop the now redundant acpi_data pointer from acpi_cpufreq_data.
> 
> Both patches on top of the current linux-next branch of the linux-pm.git tree.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 0/2] ACPI / cpufreq: ACPI processor driver and ACPI cpufreq driver cleanups

[Rafael J. Wysocki]

On Saturday, July 18, 2015 11:34:46 AM Viresh Kumar wrote:
> On 18-07-15, 03:13, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > The following two patches clean up a couple of things in the ACPI processor
> > driver and the ACPI cpufreq driver:
> > 
> > [1/2] Drop the unused first argument of acpi_processor_unregister_performance().
> > [2/2] Drop the now redundant acpi_data pointer from acpi_cpufreq_data.
> > 
> > Both patches on top of the current linux-next branch of the linux-pm.git tree.
> 
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Thanks, but I've just sent new versions out.

Rafael

[PATCH v2 0/3] ACPI / cpufreq: ACPI processor driver and ACPI cpufreq driver cleanups

[Rafael J. Wysocki]

On Saturday, July 18, 2015 03:13:41 AM Rafael J. Wysocki wrote:
> Hi,
> 
> The following two patches clean up a couple of things in the ACPI processor
> driver and the ACPI cpufreq driver:
> 
> [1/2] Drop the unused first argument of acpi_processor_unregister_performance().
> [2/2] Drop the now redundant acpi_data pointer from acpi_cpufreq_data.

New versios of the two above (as [2/3] and [3/3], respectively) plus one
more ACPI cpufreq driver cleanup (discussed on linux-pm previously).

Thanks!


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH v2 1/3] cpufreq: acpi-cpufreq: Fix up the handling of the cpb sysfs attribute

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The cpb sysfs attribute is only exposed by the ACPI cpufreq driver
after a runtime check.  For this purpose, the driver keeps a NULL
placeholder in its table of sysfs attributes and replaces the NULL
with a pointer to an attribute structure if it decides to expose
cpb.

That is confusing, so make the driver set a pointer to the cpb
attribute structure upfront and replace it with NULL if the
attribute should not be exposed instead.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/acpi-cpufreq.c |   21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

Index: linux-pm/drivers/cpufreq/acpi-cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/acpi-cpufreq.c
+++ linux-pm/drivers/cpufreq/acpi-cpufreq.c
@@ -886,7 +886,9 @@ static int acpi_cpufreq_resume(struct cp
 static struct freq_attr *acpi_cpufreq_attr[] = {
 	&cpufreq_freq_attr_scaling_available_freqs,
 	&freqdomain_cpus,
-	NULL,	/* this is a placeholder for cpb, do not remove */
+#ifdef CONFIG_X86_ACPI_CPUFREQ_CPB
+	&cpb,
+#endif
 	NULL,
 };
 
@@ -959,17 +961,16 @@ static int __init acpi_cpufreq_init(void
 	 * only if configured. This is considered legacy code, which
 	 * will probably be removed at some point in the future.
 	 */
-	if (check_amd_hwpstate_cpu(0)) {
-		struct freq_attr **iter;
-
-		pr_debug("adding sysfs entry for cpb\n");
+	if (!check_amd_hwpstate_cpu(0)) {
+		struct freq_attr **attr;
 
-		for (iter = acpi_cpufreq_attr; *iter != NULL; iter++)
-			;
+		pr_debug("CPB unsupported, do not expose it\n");
 
-		/* make sure there is a terminator behind it */
-		if (iter[1] == NULL)
-			*iter = &cpb;
+		for (attr = acpi_cpufreq_attr; *attr; attr++)
+			if (*attr == &cpb) {
+				*attr = NULL;
+				break;
+			}
 	}
 #endif
 	acpi_cpufreq_boost_init();

[PATCH v2 2/3] ACPI / processor: Drop an unused argument of a cleanup routine

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

acpi_processor_unregister_performance() actually doesn't use its
first argument, so drop it and update the callers accordingly.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/processor_perflib.c    |    4 +---
 drivers/cpufreq/acpi-cpufreq.c      |    5 ++---
 drivers/cpufreq/e_powersaver.c      |    2 +-
 drivers/cpufreq/ia64-acpi-cpufreq.c |    5 ++---
 drivers/cpufreq/powernow-k7.c       |    4 ++--
 drivers/cpufreq/powernow-k8.c       |    5 ++---
 drivers/xen/xen-acpi-processor.c    |   16 ++++++----------
 include/acpi/processor.h            |    5 +----
 8 files changed, 17 insertions(+), 29 deletions(-)

Index: linux-pm/drivers/acpi/processor_perflib.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_perflib.c
+++ linux-pm/drivers/acpi/processor_perflib.c
@@ -780,9 +780,7 @@ acpi_processor_register_performance(stru
 
 EXPORT_SYMBOL(acpi_processor_register_performance);
 
-void
-acpi_processor_unregister_performance(struct acpi_processor_performance
-				      *performance, unsigned int cpu)
+void acpi_processor_unregister_performance(unsigned int cpu)
 {
 	struct acpi_processor *pr;
 
Index: linux-pm/include/acpi/processor.h
===================================================================
--- linux-pm.orig/include/acpi/processor.h
+++ linux-pm/include/acpi/processor.h
@@ -228,10 +228,7 @@ extern int acpi_processor_preregister_pe
 
 extern int acpi_processor_register_performance(struct acpi_processor_performance
 					       *performance, unsigned int cpu);
-extern void acpi_processor_unregister_performance(struct
-						  acpi_processor_performance
-						  *performance,
-						  unsigned int cpu);
+extern void acpi_processor_unregister_performance(unsigned int cpu);
 
 /* note: this locks both the calling module and the processor module
          if a _PPC object exists, rmmod is disallowed then */
Index: linux-pm/drivers/cpufreq/acpi-cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/acpi-cpufreq.c
+++ linux-pm/drivers/cpufreq/acpi-cpufreq.c
@@ -844,7 +844,7 @@ static int acpi_cpufreq_cpu_init(struct
 err_freqfree:
 	kfree(data->freq_table);
 err_unreg:
-	acpi_processor_unregister_performance(perf, cpu);
+	acpi_processor_unregister_performance(cpu);
 err_free_mask:
 	free_cpumask_var(data->freqdomain_cpus);
 err_free:
@@ -862,8 +862,7 @@ static int acpi_cpufreq_cpu_exit(struct
 
 	if (data) {
 		policy->driver_data = NULL;
-		acpi_processor_unregister_performance(data->acpi_data,
-						      data->acpi_perf_cpu);
+		acpi_processor_unregister_performance(data->acpi_perf_cpu);
 		free_cpumask_var(data->freqdomain_cpus);
 		kfree(data->freq_table);
 		kfree(data);
Index: linux-pm/drivers/cpufreq/e_powersaver.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/e_powersaver.c
+++ linux-pm/drivers/cpufreq/e_powersaver.c
@@ -78,7 +78,7 @@ static int eps_acpi_init(void)
 static int eps_acpi_exit(struct cpufreq_policy *policy)
 {
 	if (eps_acpi_cpu_perf) {
-		acpi_processor_unregister_performance(eps_acpi_cpu_perf, 0);
+		acpi_processor_unregister_performance(0);
 		free_cpumask_var(eps_acpi_cpu_perf->shared_cpu_map);
 		kfree(eps_acpi_cpu_perf);
 		eps_acpi_cpu_perf = NULL;
Index: linux-pm/drivers/cpufreq/ia64-acpi-cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/ia64-acpi-cpufreq.c
+++ linux-pm/drivers/cpufreq/ia64-acpi-cpufreq.c
@@ -313,7 +313,7 @@ acpi_cpufreq_cpu_init (
  err_freqfree:
 	kfree(data->freq_table);
  err_unreg:
-	acpi_processor_unregister_performance(&data->acpi_data, cpu);
+	acpi_processor_unregister_performance(cpu);
  err_free:
 	kfree(data);
 	acpi_io_data[cpu] = NULL;
@@ -332,8 +332,7 @@ acpi_cpufreq_cpu_exit (
 
 	if (data) {
 		acpi_io_data[policy->cpu] = NULL;
-		acpi_processor_unregister_performance(&data->acpi_data,
-		                                      policy->cpu);
+		acpi_processor_unregister_performance(policy->cpu);
 		kfree(data);
 	}
 
Index: linux-pm/drivers/cpufreq/powernow-k7.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/powernow-k7.c
+++ linux-pm/drivers/cpufreq/powernow-k7.c
@@ -421,7 +421,7 @@ static int powernow_acpi_init(void)
 	return 0;
 
 err2:
-	acpi_processor_unregister_performance(acpi_processor_perf, 0);
+	acpi_processor_unregister_performance(0);
 err1:
 	free_cpumask_var(acpi_processor_perf->shared_cpu_map);
 err05:
@@ -661,7 +661,7 @@ static int powernow_cpu_exit(struct cpuf
 {
 #ifdef CONFIG_X86_POWERNOW_K7_ACPI
 	if (acpi_processor_perf) {
-		acpi_processor_unregister_performance(acpi_processor_perf, 0);
+		acpi_processor_unregister_performance(0);
 		free_cpumask_var(acpi_processor_perf->shared_cpu_map);
 		kfree(acpi_processor_perf);
 	}
Index: linux-pm/drivers/cpufreq/powernow-k8.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/powernow-k8.c
+++ linux-pm/drivers/cpufreq/powernow-k8.c
@@ -795,7 +795,7 @@ err_out_mem:
 	kfree(powernow_table);
 
 err_out:
-	acpi_processor_unregister_performance(&data->acpi_data, data->cpu);
+	acpi_processor_unregister_performance(data->cpu);
 
 	/* data->acpi_data.state_count informs us at ->exit()
 	 * whether ACPI was used */
@@ -863,8 +863,7 @@ static int fill_powernow_table_fidvid(st
 static void powernow_k8_cpu_exit_acpi(struct powernow_k8_data *data)
 {
 	if (data->acpi_data.state_count)
-		acpi_processor_unregister_performance(&data->acpi_data,
-				data->cpu);
+		acpi_processor_unregister_performance(data->cpu);
 	free_cpumask_var(data->acpi_data.shared_cpu_map);
 }
 
Index: linux-pm/drivers/xen/xen-acpi-processor.c
===================================================================
--- linux-pm.orig/drivers/xen/xen-acpi-processor.c
+++ linux-pm/drivers/xen/xen-acpi-processor.c
@@ -560,11 +560,9 @@ static int __init xen_acpi_processor_ini
 
 	return 0;
 err_unregister:
-	for_each_possible_cpu(i) {
-		struct acpi_processor_performance *perf;
-		perf = per_cpu_ptr(acpi_perf_data, i);
-		acpi_processor_unregister_performance(perf, i);
-	}
+	for_each_possible_cpu(i)
+		acpi_processor_unregister_performance(i);
+
 err_out:
 	/* Freeing a NULL pointer is OK: alloc_percpu zeroes. */
 	free_acpi_perf_data();
@@ -579,11 +577,9 @@ static void __exit xen_acpi_processor_ex
 	kfree(acpi_ids_done);
 	kfree(acpi_id_present);
 	kfree(acpi_id_cst_present);
-	for_each_possible_cpu(i) {
-		struct acpi_processor_performance *perf;
-		perf = per_cpu_ptr(acpi_perf_data, i);
-		acpi_processor_unregister_performance(perf, i);
-	}
+	for_each_possible_cpu(i)
+		acpi_processor_unregister_performance(i);
+
 	free_acpi_perf_data();
 }
 

[PATCH v2 3/3] cpufreq: acpi-cpufreq: Drop acpi_data from struct acpi_cpufreq_data

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

After commit 8cfcfd39000d (acpi-cpufreq: Fix an ACPI perf unregister
issue) we store both a pointer to per-CPU data of the first policy
CPU and the number of that CPU which are redundant.

Since the CPU number has to be stored anyway for the unregistration,
the pointer to the CPU's per-CPU data may be dropped and we can
access the data in question via per_cpu_ptr().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/acpi-cpufreq.c |   29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

Index: linux-pm/drivers/cpufreq/acpi-cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/acpi-cpufreq.c
+++ linux-pm/drivers/cpufreq/acpi-cpufreq.c
@@ -65,7 +65,6 @@ enum {
 #define MSR_K7_HWCR_CPB_DIS	(1ULL << 25)
 
 struct acpi_cpufreq_data {
-	struct acpi_processor_performance *acpi_data;
 	struct cpufreq_frequency_table *freq_table;
 	unsigned int resume;
 	unsigned int cpu_feature;
@@ -76,6 +75,11 @@ struct acpi_cpufreq_data {
 /* acpi_perf_data is a pointer to percpu data. */
 static struct acpi_processor_performance __percpu *acpi_perf_data;
 
+static inline struct acpi_processor_performance *to_perf_data(struct acpi_cpufreq_data *data)
+{
+	return per_cpu_ptr(acpi_perf_data, data->acpi_perf_cpu);
+}
+
 static struct cpufreq_driver acpi_cpufreq_driver;
 
 static unsigned int acpi_pstate_strict;
@@ -201,7 +205,7 @@ static unsigned extract_io(u32 value, st
 	struct acpi_processor_performance *perf;
 	int i;
 
-	perf = data->acpi_data;
+	perf = to_perf_data(data);
 
 	for (i = 0; i < perf->state_count; i++) {
 		if (value == perf->states[i].status)
@@ -220,7 +224,7 @@ static unsigned extract_msr(u32 msr, str
 	else
 		msr &= INTEL_MSR_RANGE;
 
-	perf = data->acpi_data;
+	perf = to_perf_data(data);
 
 	cpufreq_for_each_entry(pos, data->freq_table)
 		if (msr == perf->states[pos->driver_data].status)
@@ -346,7 +350,7 @@ get_cur_val(const struct cpumask *mask,
 		break;
 	case SYSTEM_IO_CAPABLE:
 		cmd.type = SYSTEM_IO_CAPABLE;
-		perf = data->acpi_data;
+		perf = to_perf_data(data);
 		cmd.addr.io.port = perf->control_register.address;
 		cmd.addr.io.bit_width = perf->control_register.bit_width;
 		break;
@@ -377,10 +381,10 @@ static unsigned int get_cur_freq_on_cpu(
 
 	data = policy->driver_data;
 	cpufreq_cpu_put(policy);
-	if (unlikely(!data || !data->acpi_data || !data->freq_table))
+	if (unlikely(!data || !data->freq_table))
 		return 0;
 
-	cached_freq = data->freq_table[data->acpi_data->state].frequency;
+	cached_freq = data->freq_table[to_perf_data(data)->state].frequency;
 	freq = extract_freq(get_cur_val(cpumask_of(cpu), data), data);
 	if (freq != cached_freq) {
 		/*
@@ -419,12 +423,11 @@ static int acpi_cpufreq_target(struct cp
 	unsigned int next_perf_state = 0; /* Index into perf table */
 	int result = 0;
 
-	if (unlikely(data == NULL ||
-	     data->acpi_data == NULL || data->freq_table == NULL)) {
+	if (unlikely(data == NULL || data->freq_table == NULL)) {
 		return -ENODEV;
 	}
 
-	perf = data->acpi_data;
+	perf = to_perf_data(data);
 	next_perf_state = data->freq_table[index].driver_data;
 	if (perf->state == next_perf_state) {
 		if (unlikely(data->resume)) {
@@ -487,8 +490,9 @@ out:
 static unsigned long
 acpi_cpufreq_guess_freq(struct acpi_cpufreq_data *data, unsigned int cpu)
 {
-	struct acpi_processor_performance *perf = data->acpi_data;
+	struct acpi_processor_performance *perf;
 
+	perf = to_perf_data(data);
 	if (cpu_khz) {
 		/* search the closest match to cpu_khz */
 		unsigned int i;
@@ -677,18 +681,17 @@ static int acpi_cpufreq_cpu_init(struct
 		goto err_free;
 	}
 
-	data->acpi_data = per_cpu_ptr(acpi_perf_data, cpu);
+	perf = per_cpu_ptr(acpi_perf_data, cpu);
 	data->acpi_perf_cpu = cpu;
 	policy->driver_data = data;
 
 	if (cpu_has(c, X86_FEATURE_CONSTANT_TSC))
 		acpi_cpufreq_driver.flags |= CPUFREQ_CONST_LOOPS;
 
-	result = acpi_processor_register_performance(data->acpi_data, cpu);
+	result = acpi_processor_register_performance(perf, cpu);
 	if (result)
 		goto err_free_mask;
 
-	perf = data->acpi_data;
 	policy->shared_type = perf->shared_type;
 
 	/*

Re: [PATCH v2 0/3] ACPI / cpufreq: ACPI processor driver and ACPI cpufreq driver cleanups

[Viresh Kumar]

On 21-07-15, 00:12, Rafael J. Wysocki wrote:
> On Saturday, July 18, 2015 03:13:41 AM Rafael J. Wysocki wrote:
> > Hi,
> > 
> > The following two patches clean up a couple of things in the ACPI processor
> > driver and the ACPI cpufreq driver:
> > 
> > [1/2] Drop the unused first argument of acpi_processor_unregister_performance().
> > [2/2] Drop the now redundant acpi_data pointer from acpi_cpufreq_data.
> 
> New versios of the two above (as [2/3] and [3/3], respectively) plus one
> more ACPI cpufreq driver cleanup (discussed on linux-pm previously).

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh