[PATCH v2 0/4] thermal: intel: int340x: Assorted cleanups

[PATCH v2 0/4] thermal: intel: int340x: Assorted cleanups

[Rafael J. Wysocki]

Hi All,

This is a new version of

https://lore.kernel.org/linux-pm/12159228.O9o76ZdvQC@kreacher/

without a few patches that have been folded into other changes in the meantime
and one extra patch.

Please refer to the individual patch changelogs for details.

This series is applicable on top of

https://patchwork.kernel.org/project/linux-pm/patch/5641279.DvuYhMxLoT@kreacher/

which in turn applies on top of the current thermal branch in linux-pm.git,
that is also present in the linux-next branch in linux-pm.git.

Thanks!

[PATCH v2 1/4] 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.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v1 -> v2:
   * Rebase
   * Skip trip_cnt update
   * More white space adjustments

---
 drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c |   16 ++++-------
 1 file changed, 7 insertions(+), 9 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,15 +30,16 @@ 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;
 }
 
 static int int340x_thermal_set_trip_temp(struct thermal_zone_device *zone,
-				      int trip, int temp)
+					 int trip, int temp)
 {
 	struct int34x_thermal_zone *d = zone->devdata;
 	acpi_status status;
@@ -46,7 +47,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;
 
@@ -92,7 +93,6 @@ static int int340x_thermal_read_trips(st
 	}
 
 	for (i = 0; i < INT340X_THERMAL_MAX_ACT_TRIP_COUNT; i++) {
-
 		ret = thermal_acpi_active_trip_temp(zone_adev, i,
 						    &zone_trips[trip_cnt].temperature);
 		if (ret)
@@ -121,8 +121,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);
 
@@ -139,7 +138,7 @@ 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;
 	}
@@ -169,8 +168,7 @@ struct int34x_thermal_zone *int340x_ther
 
 	int34x_thermal_zone->trips = zone_trips;
 
-	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 v2 2/4] thermal: intel: int340x: Rename variable in int340x_thermal_zone_add()

[Rafael J. Wysocki]

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

Rename local variables int34x_thermal_zone in int340x_thermal_zone_add()
and int340x_thermal_zone_remove() to int34x_zone which allows a number
of code lines to be 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>
---

v1 -> v2: Rebase

---
 drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c |   67 +++++------
 1 file changed, 33 insertions(+), 34 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
@@ -113,7 +113,7 @@ 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;
 	struct thermal_trip *zone_trips;
 	unsigned long long trip_cnt = 0;
 	unsigned long long hyst;
@@ -121,25 +121,25 @@ struct int34x_thermal_zone *int340x_ther
 	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;
 	}
 
@@ -166,48 +166,47 @@ struct int34x_thermal_zone *int340x_ther
 	for (i = 0; i < trip_cnt; ++i)
 		zone_trips[i].hysteresis = hyst;
 
-	int34x_thermal_zone->trips = zone_trips;
+	int34x_zone->trips = zone_trips;
 
-	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),
-						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),
+							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);
 
-void int340x_thermal_zone_remove(struct int34x_thermal_zone
-				 *int34x_thermal_zone)
+void int340x_thermal_zone_remove(struct int34x_thermal_zone *int34x_zone)
 {
-	thermal_zone_device_unregister(int34x_thermal_zone->zone);
-	acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
-	kfree(int34x_thermal_zone->trips);
-	kfree(int34x_thermal_zone->ops);
-	kfree(int34x_thermal_zone);
+	thermal_zone_device_unregister(int34x_zone->zone);
+	acpi_lpat_free_conversion_table(int34x_zone->lpat_table);
+	kfree(int34x_zone->trips);
+	kfree(int34x_zone->ops);
+	kfree(int34x_zone);
 }
 EXPORT_SYMBOL_GPL(int340x_thermal_zone_remove);
 

[PATCH v2 3/4] 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>
---

v1 -> v2:
   * It was [6/6] in v1
   * Rebase

---
 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);

[PATCH v2 4/4] thermal: intel: int340x: Improve int340x_thermal_set_trip_temp()

[Rafael J. Wysocki]

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

Instead of using snprintf() to populate the ACPI object name in
int340x_thermal_set_trip_temp(), use an appropriate initializer
and make the function fail if its trip argument is greater than 9,
because ACPI object names can only be 4 characters long and it does
not make sense to even try to evaluate objects with longer names (that
argument is guaranteed to be non-negative, because it comes from the
thermal code that will not pass negative trip numbers to zone
callbacks).

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v1 -> v2: New patch in v2

---
 drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 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
@@ -42,10 +42,12 @@ static int int340x_thermal_set_trip_temp
 					 int trip, int temp)
 {
 	struct int34x_thermal_zone *d = zone->devdata;
+	char name[] = {'P', 'A', 'T', '0' + trip, '\0'};
 	acpi_status status;
-	char name[10];
 
-	snprintf(name, sizeof(name), "PAT%d", trip);
+	if (trip > 9)
+		return -EINVAL;
+
 	status = acpi_execute_simple_method(d->adev->handle, name,
 					    millicelsius_to_deci_kelvin(temp));
 	if (ACPI_FAILURE(status))

Re: [PATCH v2 1/4] thermal: intel: int340x: Assorted minor cleanups

[Daniel Lezcano]

On 30/01/2023 19:42, Rafael J. Wysocki wrote:
> 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.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v2 3/4] thermal: intel: int340x: Drop pointless cast to unsigned long

[Daniel Lezcano]

On 30/01/2023 19:45, Rafael J. Wysocki wrote:
> 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>
> ---

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v2 4/4] thermal: intel: int340x: Improve int340x_thermal_set_trip_temp()

[Daniel Lezcano]

On 30/01/2023 19:47, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Instead of using snprintf() to populate the ACPI object name in
> int340x_thermal_set_trip_temp(), use an appropriate initializer
> and make the function fail if its trip argument is greater than 9,
> because ACPI object names can only be 4 characters long and it does
> not make sense to even try to evaluate objects with longer names (that
> argument is guaranteed to be non-negative, because it comes from the
> thermal code that will not pass negative trip numbers to zone
> callbacks).
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v2 0/4] thermal: intel: int340x: Assorted cleanups

[Zhang, Rui]

On Mon, 2023-01-30 at 19:37 +0100, Rafael J. Wysocki wrote:
> Hi All,
> 
> This is a new version of
> 
> https://lore.kernel.org/linux-pm/12159228.O9o76ZdvQC@kreacher/
> 
> without a few patches that have been folded into other changes in the
> meantime
> and one extra patch.
> 
> Please refer to the individual patch changelogs for details.
> 
> This series is applicable on top of
> 
> https://patchwork.kernel.org/project/linux-pm/patch/5641279.DvuYhMxLoT@kreacher/
> 
> which in turn applies on top of the current thermal branch in linux-
> pm.git,
> that is also present in the linux-next branch in linux-pm.git.
> 
> Thanks!
> 
> 
Tested on one KBL-R platform, everything works fine.

Tested-by: Zhang Rui <rui.zhang@intel.com>
Reviewed-by: Zhang Rui <rui.zhang@intel.com>

thanks,
rui

>