[PATCH v1 0/5] ACPI: processor: perflib: Assorted cleanups

[PATCH v1 0/5] ACPI: processor: perflib: Assorted cleanups

[Rafael J. Wysocki]

Hi All,

This is a series of assorted ACPI processor performance library cleanups.

Please refer to the patch changelogs for details.

Thanks!

[PATCH v1 1/5] ACPI: processor: perflib: Adjust white space

[Rafael J. Wysocki]

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

Some inconsistent usage of white space in the ACPI processor performance
library code causes that code to be somewhat harder to read that it
would have been otherwise, so adjust the white space in there to
address that.

No expected functional impact.

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

Index: linux-pm/drivers/acpi/processor_perflib.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_perflib.c
+++ linux-pm/drivers/acpi/processor_perflib.c
@@ -142,6 +142,7 @@ int acpi_processor_get_bios_limit(int cp
 	pr = per_cpu(processors, cpu);
 	if (!pr || !pr->performance || !pr->performance->state_count)
 		return -ENODEV;
+
 	*limit = pr->performance->states[pr->performance_platform_limit].
 		core_frequency * 1000;
 	return 0;
@@ -201,8 +202,7 @@ static int acpi_processor_get_performanc
 	}
 
 	pct = (union acpi_object *)buffer.pointer;
-	if (!pct || (pct->type != ACPI_TYPE_PACKAGE)
-	    || (pct->package.count != 2)) {
+	if (!pct || (pct->type != ACPI_TYPE_PACKAGE) || (pct->package.count != 2)) {
 		pr_err("Invalid _PCT data\n");
 		result = -EFAULT;
 		goto end;
@@ -214,9 +214,9 @@ static int acpi_processor_get_performanc
 
 	obj = pct->package.elements[0];
 
-	if ((obj.type != ACPI_TYPE_BUFFER)
-	    || (obj.buffer.length < sizeof(struct acpi_pct_register))
-	    || (obj.buffer.pointer == NULL)) {
+	if ((obj.type != ACPI_TYPE_BUFFER) ||
+	    (obj.buffer.length < sizeof(struct acpi_pct_register)) ||
+	    (obj.buffer.pointer == NULL)) {
 		pr_err("Invalid _PCT data (control_register)\n");
 		result = -EFAULT;
 		goto end;
@@ -230,9 +230,9 @@ static int acpi_processor_get_performanc
 
 	obj = pct->package.elements[1];
 
-	if ((obj.type != ACPI_TYPE_BUFFER)
-	    || (obj.buffer.length < sizeof(struct acpi_pct_register))
-	    || (obj.buffer.pointer == NULL)) {
+	if ((obj.type != ACPI_TYPE_BUFFER) ||
+	    (obj.buffer.length < sizeof(struct acpi_pct_register)) ||
+	    (obj.buffer.pointer == NULL)) {
 		pr_err("Invalid _PCT data (status_register)\n");
 		result = -EFAULT;
 		goto end;
@@ -260,8 +260,8 @@ static void amd_fixup_frequency(struct a
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
 		return;
 
-	if ((boot_cpu_data.x86 == 0x10 && boot_cpu_data.x86_model < 10)
-	    || boot_cpu_data.x86 == 0x11) {
+	if ((boot_cpu_data.x86 == 0x10 && boot_cpu_data.x86_model < 10) ||
+	    boot_cpu_data.x86 == 0x11) {
 		rdmsr(MSR_AMD_PSTATE_DEF_BASE + index, lo, hi);
 		/*
 		 * MSR C001_0064+:
@@ -353,8 +353,7 @@ static int acpi_processor_get_performanc
 		 * Check that ACPI's u64 MHz will be valid as u32 KHz in cpufreq
 		 */
 		if (!px->core_frequency ||
-		    ((u32)(px->core_frequency * 1000) !=
-		     (px->core_frequency * 1000))) {
+		    ((u32)(px->core_frequency * 1000) != (px->core_frequency * 1000))) {
 			pr_err(FW_BUG
 			       "Invalid BIOS _PSS frequency found for processor %d: 0x%llx MHz\n",
 			       pr->id, px->core_frequency);
@@ -499,7 +498,6 @@ int acpi_processor_notify_smm(struct mod
 
 	return 0;
 }
-
 EXPORT_SYMBOL(acpi_processor_notify_smm);
 
 int acpi_processor_get_psd(acpi_handle handle, struct acpi_psd_package *pdomain)
@@ -532,8 +530,7 @@ int acpi_processor_get_psd(acpi_handle h
 	state.length = sizeof(struct acpi_psd_package);
 	state.pointer = pdomain;
 
-	status = acpi_extract_package(&(psd->package.elements[0]),
-		&format, &state);
+	status = acpi_extract_package(&(psd->package.elements[0]), &format, &state);
 	if (ACPI_FAILURE(status)) {
 		pr_err("Invalid _PSD data\n");
 		result = -EFAULT;
@@ -716,9 +713,8 @@ err_out:
 }
 EXPORT_SYMBOL(acpi_processor_preregister_performance);
 
-int
-acpi_processor_register_performance(struct acpi_processor_performance
-				    *performance, unsigned int cpu)
+int acpi_processor_register_performance(struct acpi_processor_performance
+					*performance, unsigned int cpu)
 {
 	struct acpi_processor *pr;
 
@@ -751,7 +747,6 @@ acpi_processor_register_performance(stru
 	mutex_unlock(&performance_mutex);
 	return 0;
 }
-
 EXPORT_SYMBOL(acpi_processor_register_performance);
 
 void acpi_processor_unregister_performance(unsigned int cpu)
@@ -768,11 +763,11 @@ void acpi_processor_unregister_performan
 
 	if (pr->performance)
 		kfree(pr->performance->states);
+
 	pr->performance = NULL;
 
 	mutex_unlock(&performance_mutex);
 
 	return;
 }
-
 EXPORT_SYMBOL(acpi_processor_unregister_performance);

[PATCH v1 2/5] ACPI: processor: perflib: Drop redundant parentheses

[Rafael J. Wysocki]

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

Drop some redundant parentheses and rearrange some checks using them
in the ACPI processor performance library code for better code clarity.

No expected functional impact.

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

Index: linux-pm/drivers/acpi/processor_perflib.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_perflib.c
+++ linux-pm/drivers/acpi/processor_perflib.c
@@ -202,7 +202,7 @@ static int acpi_processor_get_performanc
 	}
 
 	pct = (union acpi_object *)buffer.pointer;
-	if (!pct || (pct->type != ACPI_TYPE_PACKAGE) || (pct->package.count != 2)) {
+	if (!pct || pct->type != ACPI_TYPE_PACKAGE || pct->package.count != 2) {
 		pr_err("Invalid _PCT data\n");
 		result = -EFAULT;
 		goto end;
@@ -214,9 +214,8 @@ static int acpi_processor_get_performanc
 
 	obj = pct->package.elements[0];
 
-	if ((obj.type != ACPI_TYPE_BUFFER) ||
-	    (obj.buffer.length < sizeof(struct acpi_pct_register)) ||
-	    (obj.buffer.pointer == NULL)) {
+	if (!obj.buffer.pointer || obj.type != ACPI_TYPE_BUFFER ||
+	    obj.buffer.length < sizeof(struct acpi_pct_register)) {
 		pr_err("Invalid _PCT data (control_register)\n");
 		result = -EFAULT;
 		goto end;
@@ -230,9 +229,8 @@ static int acpi_processor_get_performanc
 
 	obj = pct->package.elements[1];
 
-	if ((obj.type != ACPI_TYPE_BUFFER) ||
-	    (obj.buffer.length < sizeof(struct acpi_pct_register)) ||
-	    (obj.buffer.pointer == NULL)) {
+	if (!obj.buffer.pointer || obj.type != ACPI_TYPE_BUFFER ||
+	    obj.buffer.length < sizeof(struct acpi_pct_register)) {
 		pr_err("Invalid _PCT data (status_register)\n");
 		result = -EFAULT;
 		goto end;
@@ -300,7 +298,7 @@ static int acpi_processor_get_performanc
 	}
 
 	pss = buffer.pointer;
-	if (!pss || (pss->type != ACPI_TYPE_PACKAGE)) {
+	if (!pss || pss->type != ACPI_TYPE_PACKAGE) {
 		pr_err("Invalid _PSS data\n");
 		result = -EFAULT;
 		goto end;
@@ -353,7 +351,7 @@ static int acpi_processor_get_performanc
 		 * Check that ACPI's u64 MHz will be valid as u32 KHz in cpufreq
 		 */
 		if (!px->core_frequency ||
-		    ((u32)(px->core_frequency * 1000) != (px->core_frequency * 1000))) {
+		    (u32)(px->core_frequency * 1000) != px->core_frequency * 1000) {
 			pr_err(FW_BUG
 			       "Invalid BIOS _PSS frequency found for processor %d: 0x%llx MHz\n",
 			       pr->id, px->core_frequency);
@@ -515,7 +513,7 @@ int acpi_processor_get_psd(acpi_handle h
 	}
 
 	psd = buffer.pointer;
-	if (!psd || (psd->type != ACPI_TYPE_PACKAGE)) {
+	if (!psd || psd->type != ACPI_TYPE_PACKAGE) {
 		pr_err("Invalid _PSD data\n");
 		result = -EFAULT;
 		goto end;

[PATCH v1 3/5] ACPI: processor: perflib: Rearrange unregistration routine

[Rafael J. Wysocki]

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

Rearrange acpi_processor_unregister_performance() to follow a more
common error handling pattern and drop a redundant "return" statement
from the end of it.

No expected functional impact.

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

Index: linux-pm/drivers/acpi/processor_perflib.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_perflib.c
+++ linux-pm/drivers/acpi/processor_perflib.c
@@ -754,18 +754,15 @@ void acpi_processor_unregister_performan
 	mutex_lock(&performance_mutex);
 
 	pr = per_cpu(processors, cpu);
-	if (!pr) {
-		mutex_unlock(&performance_mutex);
-		return;
-	}
+	if (!pr)
+		goto unlock;
 
 	if (pr->performance)
 		kfree(pr->performance->states);
 
 	pr->performance = NULL;
 
+unlock:
 	mutex_unlock(&performance_mutex);
-
-	return;
 }
 EXPORT_SYMBOL(acpi_processor_unregister_performance);

[PATCH v1 4/5] ACPI: processor: perflib: Rearrange acpi_processor_notify_smm()

[Rafael J. Wysocki]

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

Rearrange the code in acpi_processor_notify_smm() to consolidate error
handling in it and improve the comments in there while at it.

No expected functional impact.

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

Index: linux-pm/drivers/acpi/processor_perflib.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_perflib.c
+++ linux-pm/drivers/acpi/processor_perflib.c
@@ -453,7 +453,7 @@ int acpi_processor_pstate_control(void)
 int acpi_processor_notify_smm(struct module *calling_module)
 {
 	static int is_done;
-	int result;
+	int result = 0;
 
 	if (!acpi_processor_cpufreq_init)
 		return -EBUSY;
@@ -461,40 +461,38 @@ int acpi_processor_notify_smm(struct mod
 	if (!try_module_get(calling_module))
 		return -EINVAL;
 
-	/* is_done is set to negative if an error occurred,
-	 * and to postitive if _no_ error occurred, but SMM
-	 * was already notified. This avoids double notification
-	 * which might lead to unexpected results...
+	/*
+	 * is_done is set to negative if an error occurs and to 1 if no error 
+	 * occurrs, but SMM has been notified already. This avoids repeated
+	 * notification which might lead to unexpected results.
 	 */
-	if (is_done > 0) {
-		module_put(calling_module);
-		return 0;
-	} else if (is_done < 0) {
-		module_put(calling_module);
-		return is_done;
-	}
+	if (is_done != 0) {
+		if (is_done < 0)
+			result = is_done;
 
-	is_done = -EIO;
+		goto out_put;
+	}
 
 	result = acpi_processor_pstate_control();
-	if (!result) {
-		pr_debug("No SMI port or pstate_control\n");
-		module_put(calling_module);
-		return 0;
-	}
-	if (result < 0) {
-		module_put(calling_module);
-		return result;
+	if (result <= 0) {
+		if (!result)
+			pr_debug("No SMI port or pstate_control\n");
+
+		is_done = -EIO;
+		goto out_put;
 	}
 
-	/* Success. If there's no _PPC, we need to fear nothing, so
-	 * we can allow the cpufreq driver to be rmmod'ed. */
 	is_done = 1;
+	/*
+	 * Success. If there _PPC, unloading the cpufreq driver would be risky,
+	 * so disallow it in that case.
+	 */
+	if (acpi_processor_ppc_in_use)
+		return 0;
 
-	if (!acpi_processor_ppc_in_use)
-		module_put(calling_module);
-
-	return 0;
+out_put:
+	module_put(calling_module);
+	return result;
 }
 EXPORT_SYMBOL(acpi_processor_notify_smm);
 

[PATCH v1 5/5] ACPI: processor: perflib: Adjust acpi_processor_notify_smm() return value

[Rafael J. Wysocki]

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

Avoid returning a confusing error code from acpi_processor_notify_smm()
if it is called for the second time in the case when SMM notification
regarding P-state control is not supported.

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

Index: linux-pm/drivers/acpi/processor_perflib.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_perflib.c
+++ linux-pm/drivers/acpi/processor_perflib.c
@@ -475,10 +475,12 @@ int acpi_processor_notify_smm(struct mod
 
 	result = acpi_processor_pstate_control();
 	if (result <= 0) {
-		if (!result)
+		if (result) {
+			is_done = result;
+		} else {
 			pr_debug("No SMI port or pstate_control\n");
-
-		is_done = -EIO;
+			is_done = 1;
+		}
 		goto out_put;
 	}