[PATCH v1 0/6] thermal: intel: int340x: Assorted cleanup changes

[PATCH v1 0/6] thermal: intel: int340x: Assorted cleanup changes

[Rafael J. Wysocki]

Hi All,

This patch series makes several changes in int340x_thermal_zone.c that are not
expected to make any functional impact on it, but should overall improve the
maintainability of that file.  Please refer to the individual patch changelogs
for details.

The series is based on the current linux-next branch in linux-pm.git.

Thanks!

[PATCH v1 1/6] thermal: intel: int340x: Assorted minor cleanups

[Rafael J. Wysocki]

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

Improve some inconsistent usage of white space in int340x_thermal_zone.c,
fix up one coding style issue in it (missing braces around an else
branch of a conditional) and while at it replace a !ACPI_FAILURE()
check with an equivalent ACPI_SUCCESS() one.

Also add INT340X_THERMAL_MAX_TRIP_COUNT to trip_cnt before using it
for the allocation of trips for more clarity.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c |   22 +++++------
 1 file changed, 10 insertions(+), 12 deletions(-)

Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -30,10 +30,10 @@ static int int340x_thermal_get_zone_temp
 			return conv_temp;
 
 		*temp = (unsigned long)conv_temp * 10;
-	} else
+	} else {
 		/* _TMP returns the temperature in tenths of degrees Kelvin */
 		*temp = deci_kelvin_to_millicelsius(tmp);
-
+	}
 	return 0;
 }
 
@@ -46,7 +46,7 @@ static int int340x_thermal_set_trip_temp
 
 	snprintf(name, sizeof(name), "PAT%d", trip);
 	status = acpi_execute_simple_method(d->adev->handle, name,
-			millicelsius_to_deci_kelvin(temp));
+					    millicelsius_to_deci_kelvin(temp));
 	if (ACPI_FAILURE(status))
 		return -EIO;
 
@@ -99,7 +99,6 @@ int int340x_thermal_read_trips(struct in
 		trip_cnt++;
 
 	for (i = 0; i < INT340X_THERMAL_MAX_ACT_TRIP_COUNT; i++) {
-
 		ret = thermal_acpi_trip_active(int34x_zone->adev, i, &int34x_zone->trips[trip_cnt]);
 		if (ret)
 			break;
@@ -125,8 +124,7 @@ struct int34x_thermal_zone *int340x_ther
 	acpi_status status;
 	int i, ret;
 
-	int34x_thermal_zone = kzalloc(sizeof(*int34x_thermal_zone),
-				      GFP_KERNEL);
+	int34x_thermal_zone = kzalloc(sizeof(*int34x_thermal_zone), GFP_KERNEL);
 	if (!int34x_thermal_zone)
 		return ERR_PTR(-ENOMEM);
 
@@ -143,14 +141,15 @@ struct int34x_thermal_zone *int340x_ther
 		int34x_thermal_zone->ops->get_temp = get_temp;
 
 	status = acpi_evaluate_integer(adev->handle, "PATC", NULL, &trip_cnt);
-	if (!ACPI_FAILURE(status)) {
+	if (ACPI_SUCCESS(status)) {
 		int34x_thermal_zone->aux_trip_nr = trip_cnt;
 		trip_mask = BIT(trip_cnt) - 1;
 	}
 
-	int34x_thermal_zone->trips = kzalloc(sizeof(*int34x_thermal_zone->trips) *
-					     (INT340X_THERMAL_MAX_TRIP_COUNT + trip_cnt),
-					      GFP_KERNEL);
+	trip_cnt += INT340X_THERMAL_MAX_TRIP_COUNT;
+
+	int34x_thermal_zone->trips = kzalloc(sizeof(*int34x_thermal_zone->trips) * trip_cnt,
+					     GFP_KERNEL);
 	if (!int34x_thermal_zone->trips) {
 		ret = -ENOMEM;
 		goto err_trips_alloc;
@@ -166,8 +165,7 @@ struct int34x_thermal_zone *int340x_ther
 		int34x_thermal_zone->trips[i].temperature = THERMAL_TEMP_INVALID;
 	}
 
-	int34x_thermal_zone->lpat_table = acpi_lpat_get_conversion_table(
-								adev->handle);
+	int34x_thermal_zone->lpat_table = acpi_lpat_get_conversion_table(adev->handle);
 
 	int34x_thermal_zone->zone = thermal_zone_device_register_with_trips(
 						acpi_device_bid(adev),

[PATCH v1 2/6] thermal: intel: int340x: Rename variable in int340x_thermal_zone_add()

[Rafael J. Wysocki]

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

Rename local variable int34x_thermal_zone in int340x_thermal_zone_add()
to int34x_zone which allows a number of code lines to become shorter and
easier to read and adjust some white space for consistency.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c |   69 +++++------
 1 file changed, 34 insertions(+), 35 deletions(-)

Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -118,82 +118,81 @@ static struct thermal_zone_params int340
 struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
 						     int (*get_temp) (struct thermal_zone_device *, int *))
 {
-	struct int34x_thermal_zone *int34x_thermal_zone;
+	struct int34x_thermal_zone *int34x_zone;
 	unsigned long long trip_cnt = 0;
 	int trip_mask = 0;
 	acpi_status status;
 	int i, ret;
 
-	int34x_thermal_zone = kzalloc(sizeof(*int34x_thermal_zone), GFP_KERNEL);
-	if (!int34x_thermal_zone)
+	int34x_zone = kzalloc(sizeof(*int34x_zone), GFP_KERNEL);
+	if (!int34x_zone)
 		return ERR_PTR(-ENOMEM);
 
-	int34x_thermal_zone->adev = adev;
+	int34x_zone->adev = adev;
 
-	int34x_thermal_zone->ops = kmemdup(&int340x_thermal_zone_ops,
-					   sizeof(int340x_thermal_zone_ops), GFP_KERNEL);
-	if (!int34x_thermal_zone->ops) {
+	int34x_zone->ops = kmemdup(&int340x_thermal_zone_ops,
+				   sizeof(int340x_thermal_zone_ops), GFP_KERNEL);
+	if (!int34x_zone->ops) {
 		ret = -ENOMEM;
 		goto err_ops_alloc;
 	}
 
 	if (get_temp)
-		int34x_thermal_zone->ops->get_temp = get_temp;
+		int34x_zone->ops->get_temp = get_temp;
 
 	status = acpi_evaluate_integer(adev->handle, "PATC", NULL, &trip_cnt);
 	if (ACPI_SUCCESS(status)) {
-		int34x_thermal_zone->aux_trip_nr = trip_cnt;
+		int34x_zone->aux_trip_nr = trip_cnt;
 		trip_mask = BIT(trip_cnt) - 1;
 	}
 
 	trip_cnt += INT340X_THERMAL_MAX_TRIP_COUNT;
 
-	int34x_thermal_zone->trips = kzalloc(sizeof(*int34x_thermal_zone->trips) * trip_cnt,
-					     GFP_KERNEL);
-	if (!int34x_thermal_zone->trips) {
+	int34x_zone->trips = kzalloc(sizeof(*int34x_zone->trips) * trip_cnt, GFP_KERNEL);
+	if (!int34x_zone->trips) {
 		ret = -ENOMEM;
 		goto err_trips_alloc;
 	}
 
-	trip_cnt = int340x_thermal_read_trips(int34x_thermal_zone);
+	trip_cnt = int340x_thermal_read_trips(int34x_zone);
 
 	for (i = 0; i < trip_cnt; ++i)
-		int340x_thermal_get_global_hyst(adev, &int34x_thermal_zone->trips[i].hysteresis);
+		int340x_thermal_get_global_hyst(adev, &int34x_zone->trips[i].hysteresis);
 
-	for (i = 0; i < int34x_thermal_zone->aux_trip_nr; i++) {
-		int34x_thermal_zone->trips[i].type = THERMAL_TRIP_PASSIVE;
-		int34x_thermal_zone->trips[i].temperature = THERMAL_TEMP_INVALID;
+	for (i = 0; i < int34x_zone->aux_trip_nr; i++) {
+		int34x_zone->trips[i].type = THERMAL_TRIP_PASSIVE;
+		int34x_zone->trips[i].temperature = THERMAL_TEMP_INVALID;
 	}
 
-	int34x_thermal_zone->lpat_table = acpi_lpat_get_conversion_table(adev->handle);
+	int34x_zone->lpat_table = acpi_lpat_get_conversion_table(adev->handle);
 
-	int34x_thermal_zone->zone = thermal_zone_device_register_with_trips(
-						acpi_device_bid(adev),
-						int34x_thermal_zone->trips,
-						trip_cnt,
-						trip_mask, int34x_thermal_zone,
-						int34x_thermal_zone->ops,
-						&int340x_thermal_params,
-						0, 0);
-	if (IS_ERR(int34x_thermal_zone->zone)) {
-		ret = PTR_ERR(int34x_thermal_zone->zone);
+	int34x_zone->zone = thermal_zone_device_register_with_trips(
+							acpi_device_bid(adev),
+							int34x_zone->trips,
+							trip_cnt,
+							trip_mask, int34x_zone,
+							int34x_zone->ops,
+							&int340x_thermal_params,
+							0, 0);
+	if (IS_ERR(int34x_zone->zone)) {
+		ret = PTR_ERR(int34x_zone->zone);
 		goto err_thermal_zone;
 	}
-	ret = thermal_zone_device_enable(int34x_thermal_zone->zone);
+	ret = thermal_zone_device_enable(int34x_zone->zone);
 	if (ret)
 		goto err_enable;
 
-	return int34x_thermal_zone;
+	return int34x_zone;
 
 err_enable:
-	thermal_zone_device_unregister(int34x_thermal_zone->zone);
+	thermal_zone_device_unregister(int34x_zone->zone);
 err_thermal_zone:
-	kfree(int34x_thermal_zone->trips);
-	acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
+	kfree(int34x_zone->trips);
+	acpi_lpat_free_conversion_table(int34x_zone->lpat_table);
 err_trips_alloc:
-	kfree(int34x_thermal_zone->ops);
+	kfree(int34x_zone->ops);
 err_ops_alloc:
-	kfree(int34x_thermal_zone);
+	kfree(int34x_zone);
 	return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(int340x_thermal_zone_add);

[PATCH v1 3/6] thermal: intel: int340x: Evaluate GTSH once

[Rafael J. Wysocki]

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

It is not necessary to evaluate the GTSH ACPI object for every trip
point in int340x_thermal_zone_add() and the function used for that
is not really necessary either, because it only has one caller.

Accordingly, fold the evaluation of GTSH directly into
int340x_thermal_zone_add(), evaluate it only once and populate the
hysteresis for all of the trip points with the value produced by it.

No expected functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c |   24 +++--------
 1 file changed, 8 insertions(+), 16 deletions(-)

Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -53,21 +53,6 @@ static int int340x_thermal_set_trip_temp
 	return 0;
 }
 
-static int int340x_thermal_get_global_hyst(struct acpi_device *adev, int *temp)
-{
-	acpi_status status;
-	unsigned long long hyst;
-
-	status = acpi_evaluate_integer(adev->handle, "GTSH", NULL, &hyst);
-	if (ACPI_FAILURE(status))
-		*temp = 0;
-	else
-		*temp = hyst * 100;
-
-	return 0;
-}
-
-
 static void int340x_thermal_critical(struct thermal_zone_device *zone)
 {
 	dev_dbg(&zone->device, "%s: critical temperature reached\n", zone->type);
@@ -120,6 +105,7 @@ struct int34x_thermal_zone *int340x_ther
 {
 	struct int34x_thermal_zone *int34x_zone;
 	unsigned long long trip_cnt = 0;
+	unsigned long long hyst;
 	int trip_mask = 0;
 	acpi_status status;
 	int i, ret;
@@ -156,8 +142,14 @@ struct int34x_thermal_zone *int340x_ther
 
 	trip_cnt = int340x_thermal_read_trips(int34x_zone);
 
+	status = acpi_evaluate_integer(adev->handle, "GTSH", NULL, &hyst);
+	if (ACPI_SUCCESS(status))
+		hyst *= 100;
+	else
+		hyst = 0;
+
 	for (i = 0; i < trip_cnt; ++i)
-		int340x_thermal_get_global_hyst(adev, &int34x_zone->trips[i].hysteresis);
+		int34x_zone->trips[i].hysteresis = hyst;
 
 	for (i = 0; i < int34x_zone->aux_trip_nr; i++) {
 		int34x_zone->trips[i].type = THERMAL_TRIP_PASSIVE;

[PATCH v1 4/6] thermal: intel: int340x: Rearrange int340x_thermal_read_trips()

[Rafael J. Wysocki]

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

Use local variables trips and adev to store the addresses of the given
thermal zone's trip point table and ACPI device, respectively, and use
them to shorten some code lines.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -66,25 +66,27 @@ static struct thermal_zone_device_ops in
 
 int int340x_thermal_read_trips(struct int34x_thermal_zone *int34x_zone)
 {
+	struct thermal_trip *trips = int34x_zone->trips;
+	struct acpi_device *adev = int34x_zone->adev;
 	int trip_cnt;
 	int i, ret;
 
 	trip_cnt = int34x_zone->aux_trip_nr;
 
-	ret = thermal_acpi_trip_critical(int34x_zone->adev, &int34x_zone->trips[trip_cnt]);
+	ret = thermal_acpi_trip_critical(adev, &trips[trip_cnt]);
 	if (!ret)
 		trip_cnt++;
 
-	ret = thermal_acpi_trip_hot(int34x_zone->adev, &int34x_zone->trips[trip_cnt]);
+	ret = thermal_acpi_trip_hot(adev, &trips[trip_cnt]);
 	if (!ret)
 		trip_cnt++;
 
-	ret = thermal_acpi_trip_passive(int34x_zone->adev, &int34x_zone->trips[trip_cnt]);
+	ret = thermal_acpi_trip_passive(adev, &trips[trip_cnt]);
 	if (!ret)
 		trip_cnt++;
 
 	for (i = 0; i < INT340X_THERMAL_MAX_ACT_TRIP_COUNT; i++) {
-		ret = thermal_acpi_trip_active(int34x_zone->adev, i, &int34x_zone->trips[trip_cnt]);
+		ret = thermal_acpi_trip_active(adev, i, &trips[trip_cnt]);
 		if (ret)
 			break;
 

[PATCH v1 5/6] thermal: intel: int340x: Populate auxiliary trip points first

[Rafael J. Wysocki]

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

Populate the auxiliary trip points in the int340x thermal zone before
the "regular" ACPI ones, because they occupy the initial entries of
the trip points table.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -142,6 +142,11 @@ struct int34x_thermal_zone *int340x_ther
 		goto err_trips_alloc;
 	}
 
+	for (i = 0; i < int34x_zone->aux_trip_nr; i++) {
+		int34x_zone->trips[i].type = THERMAL_TRIP_PASSIVE;
+		int34x_zone->trips[i].temperature = THERMAL_TEMP_INVALID;
+	}
+
 	trip_cnt = int340x_thermal_read_trips(int34x_zone);
 
 	status = acpi_evaluate_integer(adev->handle, "GTSH", NULL, &hyst);
@@ -153,11 +158,6 @@ struct int34x_thermal_zone *int340x_ther
 	for (i = 0; i < trip_cnt; ++i)
 		int34x_zone->trips[i].hysteresis = hyst;
 
-	for (i = 0; i < int34x_zone->aux_trip_nr; i++) {
-		int34x_zone->trips[i].type = THERMAL_TRIP_PASSIVE;
-		int34x_zone->trips[i].temperature = THERMAL_TEMP_INVALID;
-	}
-
 	int34x_zone->lpat_table = acpi_lpat_get_conversion_table(adev->handle);
 
 	int34x_zone->zone = thermal_zone_device_register_with_trips(

[PATCH v1 6/6] thermal: intel: int340x: Drop pointless cast to unsigned long

[Rafael J. Wysocki]

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

The explicit casting from int to unsigned long in
int340x_thermal_get_zone_temp() is pointless, becuase the multiplication
result is cast back to int by the assignment in the same statement, so
drop it.

No expected functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -29,7 +29,7 @@ static int int340x_thermal_get_zone_temp
 		if (conv_temp < 0)
 			return conv_temp;
 
-		*temp = (unsigned long)conv_temp * 10;
+		*temp = conv_temp * 10;
 	} else {
 		/* _TMP returns the temperature in tenths of degrees Kelvin */
 		*temp = deci_kelvin_to_millicelsius(tmp);