[PATCH v1 0/9] ACPI: thermal: Removal of redundant data and cleanup

[PATCH v1 0/9] ACPI: thermal: Removal of redundant data and cleanup

[Rafael J. Wysocki]

Hi Everyone,

This patch series removes some data items that have become redundant after
recent changes made to the ACPI thermal driver and changes the code to be
easier to follow.  Among other things it does the following:

1. Removes the redundant copies of the critical and hot trip point temperature.
2. Separates the initialization of the critical and hot trip points from
   the post-init trip point management.
3. Reorganizes the initialization and updates of the passive and active trips.
4. Removes the redundant valid bit from the representations of the passive and
   active trips.

Please refer to the individual patch changelogs for details.

Thanks!

[PATCH v1 1/9] ACPI: thermal: Simplify initialization of critical and hot trips

[Rafael J. Wysocki]

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

Use the observation that the critical and hot trip points are never
updated by the ACPI thermal driver, because the flags passed from
acpi_thermal_notify() to acpi_thermal_trips_update() do not include
ACPI_TRIPS_CRITICAL or ACPI_TRIPS_HOT, to move the initialization
of those trip points directly into acpi_thermal_get_trip_points() and
reduce the size of __acpi_thermal_trips_update().

Also make the critical and hot trip points initialization code more
straightforward and drop the flags that are not needed any more.

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

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -43,17 +43,13 @@
 #define ACPI_THERMAL_MAX_ACTIVE		10
 #define ACPI_THERMAL_MAX_LIMIT_STR_LEN	65
 
-#define ACPI_TRIPS_CRITICAL	BIT(0)
-#define ACPI_TRIPS_HOT		BIT(1)
-#define ACPI_TRIPS_PASSIVE	BIT(2)
-#define ACPI_TRIPS_ACTIVE	BIT(3)
-#define ACPI_TRIPS_DEVICES	BIT(4)
+#define ACPI_TRIPS_PASSIVE	BIT(0)
+#define ACPI_TRIPS_ACTIVE	BIT(1)
+#define ACPI_TRIPS_DEVICES	BIT(2)
 
 #define ACPI_TRIPS_THRESHOLDS	(ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE)
 
-#define ACPI_TRIPS_INIT		(ACPI_TRIPS_CRITICAL | ACPI_TRIPS_HOT | \
-				 ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE | \
-				 ACPI_TRIPS_DEVICES)
+#define ACPI_TRIPS_INIT		(ACPI_TRIPS_THRESHOLDS | ACPI_TRIPS_DEVICES)
 
 /*
  * This exception is thrown out in two cases:
@@ -196,62 +192,6 @@ static void __acpi_thermal_trips_update(
 	bool valid = false;
 	int i;
 
-	/* Critical Shutdown */
-	if (flag & ACPI_TRIPS_CRITICAL) {
-		status = acpi_evaluate_integer(tz->device->handle, "_CRT", NULL, &tmp);
-		tz->trips.critical.temperature = tmp;
-		/*
-		 * Treat freezing temperatures as invalid as well; some
-		 * BIOSes return really low values and cause reboots at startup.
-		 * Below zero (Celsius) values clearly aren't right for sure..
-		 * ... so lets discard those as invalid.
-		 */
-		if (ACPI_FAILURE(status)) {
-			tz->trips.critical.valid = false;
-			acpi_handle_debug(tz->device->handle,
-					  "No critical threshold\n");
-		} else if (tmp <= 2732) {
-			pr_info(FW_BUG "Invalid critical threshold (%llu)\n", tmp);
-			tz->trips.critical.valid = false;
-		} else {
-			tz->trips.critical.valid = true;
-			acpi_handle_debug(tz->device->handle,
-					  "Found critical threshold [%lu]\n",
-					  tz->trips.critical.temperature);
-		}
-		if (tz->trips.critical.valid) {
-			if (crt == -1) {
-				tz->trips.critical.valid = false;
-			} else if (crt > 0) {
-				unsigned long crt_k = celsius_to_deci_kelvin(crt);
-
-				/*
-				 * Allow override critical threshold
-				 */
-				if (crt_k > tz->trips.critical.temperature)
-					pr_info("Critical threshold %d C\n", crt);
-
-				tz->trips.critical.temperature = crt_k;
-			}
-		}
-	}
-
-	/* Critical Sleep (optional) */
-	if (flag & ACPI_TRIPS_HOT) {
-		status = acpi_evaluate_integer(tz->device->handle, "_HOT", NULL, &tmp);
-		if (ACPI_FAILURE(status)) {
-			tz->trips.hot.valid = false;
-			acpi_handle_debug(tz->device->handle,
-					  "No hot threshold\n");
-		} else {
-			tz->trips.hot.temperature = tmp;
-			tz->trips.hot.valid = true;
-			acpi_handle_debug(tz->device->handle,
-					  "Found hot threshold [%lu]\n",
-					  tz->trips.hot.temperature);
-		}
-	}
-
 	/* Passive (optional) */
 	if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips.passive.trip.valid) ||
 	    flag == ACPI_TRIPS_INIT) {
@@ -451,11 +391,73 @@ static void acpi_thermal_trips_update(st
 					dev_name(&adev->dev), event, 0);
 }
 
+static void acpi_thermal_get_critical_trip(struct acpi_thermal *tz)
+{
+	unsigned long long tmp;
+	acpi_status status;
+
+	if (crt > 0) {
+		tmp = celsius_to_deci_kelvin(crt);
+		goto set;
+	}
+	if (crt == -1) {
+		acpi_handle_debug(tz->device->handle, "Critical threshold disabled\n");
+		goto fail;
+	}
+
+	status = acpi_evaluate_integer(tz->device->handle, "_CRT", NULL, &tmp);
+	if (ACPI_FAILURE(status)) {
+		acpi_handle_debug(tz->device->handle, "No critical threshold\n");
+		goto fail;
+	}
+	if (tmp <= 2732) {
+		/*
+		 * Below zero (Celsius) values clearly aren't right for sure,
+		 * so discard them as invalid.
+		 */
+		pr_info(FW_BUG "Invalid critical threshold (%llu)\n", tmp);
+		goto fail;
+	}
+
+set:
+	tz->trips.critical.valid = true;
+	tz->trips.critical.temperature = tmp;
+	acpi_handle_debug(tz->device->handle, "Critical threshold [%lu]\n",
+			  tz->trips.critical.temperature);
+	return;
+
+fail:
+	tz->trips.critical.valid = false;
+	tz->trips.critical.temperature = THERMAL_TEMP_INVALID;
+}
+
+static void acpi_thermal_get_hot_trip(struct acpi_thermal *tz)
+{
+	unsigned long long tmp;
+	acpi_status status;
+
+	status = acpi_evaluate_integer(tz->device->handle, "_HOT", NULL, &tmp);
+	if (ACPI_FAILURE(status)) {
+		tz->trips.hot.valid = false;
+		tz->trips.hot.temperature = THERMAL_TEMP_INVALID;
+		acpi_handle_debug(tz->device->handle, "No hot threshold\n");
+		return;
+	}
+
+	tz->trips.hot.valid = true;
+	tz->trips.hot.temperature = tmp;
+	acpi_handle_debug(tz->device->handle, "Hot threshold [%lu]\n",
+			  tz->trips.hot.temperature);
+}
+
 static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
 {
 	bool valid;
 	int i;
 
+	acpi_thermal_get_critical_trip(tz);
+	acpi_thermal_get_hot_trip(tz);
+	/* Passive and active trip points (optional). */
 	__acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT);
 
 	valid = tz->trips.critical.valid |

[PATCH v1 2/9] ACPI: thermal: Fold acpi_thermal_get_info() into its caller

[Rafael J. Wysocki]

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

There is only one caller of acpi_thermal_get_info() and the code from
it can be folded into its caller just fine, so do that.

No intentional functional impact.

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

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -846,38 +846,6 @@ static void acpi_thermal_aml_dependency_
 	acpi_evaluate_integer(handle, "_TMP", NULL, &value);
 }
 
-static int acpi_thermal_get_info(struct acpi_thermal *tz)
-{
-	int result;
-
-	if (!tz)
-		return -EINVAL;
-
-	acpi_thermal_aml_dependency_fix(tz);
-
-	/* Get trip points [_CRT, _PSV, etc.] (required) */
-	result = acpi_thermal_get_trip_points(tz);
-	if (result)
-		return result;
-
-	/* Get temperature [_TMP] (required) */
-	result = acpi_thermal_get_temperature(tz);
-	if (result)
-		return result;
-
-	/* Set the cooling mode [_SCP] to active cooling (default) */
-	acpi_execute_simple_method(tz->device->handle, "_SCP",
-				   ACPI_THERMAL_MODE_ACTIVE);
-
-	/* Get default polling frequency [_TZP] (optional) */
-	if (tzp)
-		tz->polling_frequency = tzp;
-	else
-		acpi_thermal_get_polling_frequency(tz);
-
-	return 0;
-}
-
 /*
  * The exact offset between Kelvin and degree Celsius is 273.15. However ACPI
  * handles temperature values with a single decimal place. As a consequence,
@@ -940,10 +908,28 @@ static int acpi_thermal_add(struct acpi_
 	strcpy(acpi_device_class(device), ACPI_THERMAL_CLASS);
 	device->driver_data = tz;
 
-	result = acpi_thermal_get_info(tz);
+	acpi_thermal_aml_dependency_fix(tz);
+
+	/* Get trip points [_CRT, _PSV, etc.] (required). */
+	result = acpi_thermal_get_trip_points(tz);
 	if (result)
 		goto free_memory;
 
+	/* Get temperature [_TMP] (required). */
+	result = acpi_thermal_get_temperature(tz);
+	if (result)
+		goto free_memory;
+
+	/* Set the cooling mode [_SCP] to active cooling. */
+	acpi_execute_simple_method(tz->device->handle, "_SCP",
+				   ACPI_THERMAL_MODE_ACTIVE);
+
+	/* Determine the default polling frequency [_TZP]. */
+	if (tzp)
+		tz->polling_frequency = tzp;
+	else
+		acpi_thermal_get_polling_frequency(tz);
+
 	acpi_thermal_guess_offset(tz);
 
 	result = acpi_thermal_register_thermal_zone(tz);

[PATCH v1 3/9] ACPI: thermal: Determine the number of trip points earlier

[Rafael J. Wysocki]

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

Compute the number of trip points in acpi_thermal_add() so as to allow the
driver's data structures to be simplified going forward.

No intentional functional impact.

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

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -452,7 +452,7 @@ static void acpi_thermal_get_hot_trip(st
 
 static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
 {
-	bool valid;
+	unsigned int count = 0;
 	int i;
 
 	acpi_thermal_get_critical_trip(tz);
@@ -460,18 +460,24 @@ static int acpi_thermal_get_trip_points(
 	/* Passive and active trip points (optional). */
 	__acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT);
 
-	valid = tz->trips.critical.valid |
-		tz->trips.hot.valid |
-		tz->trips.passive.trip.valid;
-
-	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
-		valid = valid || tz->trips.active[i].trip.valid;
-
-	if (!valid) {
-		pr_warn(FW_BUG "No valid trip found\n");
-		return -ENODEV;
+	if (tz->trips.critical.valid)
+		count++;
+
+	if (tz->trips.hot.valid)
+		count++;
+
+	if (tz->trips.passive.trip.valid)
+		count++;
+
+	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
+		if (tz->trips.active[i].trip.valid)
+			count++;
+		else
+			break;
+
 	}
-	return 0;
+
+	return count;
 }
 
 /* sys I/F for generic thermal sysfs support */
@@ -681,29 +687,15 @@ static void acpi_thermal_zone_sysfs_remo
 	sysfs_remove_link(&tzdev->kobj, "device");
 }
 
-static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
+static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz,
+					      unsigned int trip_count)
 {
 	struct acpi_thermal_trip *acpi_trip;
 	struct thermal_trip *trip;
 	int passive_delay = 0;
-	int trip_count = 0;
 	int result;
 	int i;
 
-	if (tz->trips.critical.valid)
-		trip_count++;
-
-	if (tz->trips.hot.valid)
-		trip_count++;
-
-	if (tz->trips.passive.trip.valid) {
-		trip_count++;
-		passive_delay = tz->trips.passive.tsp * 100;
-	}
-
-	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE && tz->trips.active[i].trip.valid; i++)
-		trip_count++;
-
 	trip = kcalloc(trip_count, sizeof(*trip), GFP_KERNEL);
 	if (!trip)
 		return -ENOMEM;
@@ -724,6 +716,8 @@ static int acpi_thermal_register_thermal
 
 	acpi_trip = &tz->trips.passive.trip;
 	if (acpi_trip->valid) {
+		passive_delay = tz->trips.passive.tsp * 100;
+
 		trip->type = THERMAL_TRIP_PASSIVE;
 		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temperature);
 		trip->priv = acpi_trip;
@@ -893,6 +887,7 @@ static void acpi_thermal_check_fn(struct
 static int acpi_thermal_add(struct acpi_device *device)
 {
 	struct acpi_thermal *tz;
+	unsigned int trip_count;
 	int result;
 
 	if (!device)
@@ -911,9 +906,12 @@ static int acpi_thermal_add(struct acpi_
 	acpi_thermal_aml_dependency_fix(tz);
 
 	/* Get trip points [_CRT, _PSV, etc.] (required). */
-	result = acpi_thermal_get_trip_points(tz);
-	if (result)
+	trip_count = acpi_thermal_get_trip_points(tz);
+	if (!trip_count) {
+		pr_warn(FW_BUG "No valid trip points!\n");
+		result = -ENODEV;
 		goto free_memory;
+	}
 
 	/* Get temperature [_TMP] (required). */
 	result = acpi_thermal_get_temperature(tz);
@@ -932,7 +930,7 @@ static int acpi_thermal_add(struct acpi_
 
 	acpi_thermal_guess_offset(tz);
 
-	result = acpi_thermal_register_thermal_zone(tz);
+	result = acpi_thermal_register_thermal_zone(tz, trip_count);
 	if (result)
 		goto free_memory;
 

[PATCH v1 4/9] ACPI: thermal: Create and populate trip points table earlier

[Rafael J. Wysocki]

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

Create and populate the driver's trip points table in acpi_thermal_add()
so as to allow the its data structures to be simplified going forward.

No intentional functional impact.

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

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -688,53 +688,10 @@ static void acpi_thermal_zone_sysfs_remo
 }
 
 static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz,
-					      unsigned int trip_count)
+					      unsigned int trip_count,
+					      int passive_delay)
 {
-	struct acpi_thermal_trip *acpi_trip;
-	struct thermal_trip *trip;
-	int passive_delay = 0;
 	int result;
-	int i;
-
-	trip = kcalloc(trip_count, sizeof(*trip), GFP_KERNEL);
-	if (!trip)
-		return -ENOMEM;
-
-	tz->trip_table = trip;
-
-	if (tz->trips.critical.valid) {
-		trip->type = THERMAL_TRIP_CRITICAL;
-		trip->temperature = acpi_thermal_temp(tz, tz->trips.critical.temperature);
-		trip++;
-	}
-
-	if (tz->trips.hot.valid) {
-		trip->type = THERMAL_TRIP_HOT;
-		trip->temperature = acpi_thermal_temp(tz, tz->trips.hot.temperature);
-		trip++;
-	}
-
-	acpi_trip = &tz->trips.passive.trip;
-	if (acpi_trip->valid) {
-		passive_delay = tz->trips.passive.tsp * 100;
-
-		trip->type = THERMAL_TRIP_PASSIVE;
-		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temperature);
-		trip->priv = acpi_trip;
-		trip++;
-	}
-
-	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
-		acpi_trip = &tz->trips.active[i].trip;
-
-		if (!acpi_trip->valid)
-			break;
-
-		trip->type = THERMAL_TRIP_ACTIVE;
-		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temperature);
-		trip->priv = acpi_trip;
-		trip++;
-	}
 
 	tz->thermal_zone = thermal_zone_device_register_with_trips("acpitz",
 								   tz->trip_table,
@@ -744,10 +701,8 @@ static int acpi_thermal_register_thermal
 								   NULL,
 								   passive_delay,
 								   tz->polling_frequency * 100);
-	if (IS_ERR(tz->thermal_zone)) {
-		result = PTR_ERR(tz->thermal_zone);
-		goto free_trip_table;
-	}
+	if (IS_ERR(tz->thermal_zone))
+		return PTR_ERR(tz->thermal_zone);
 
 	result = acpi_thermal_zone_sysfs_add(tz);
 	if (result)
@@ -766,8 +721,6 @@ remove_links:
 	acpi_thermal_zone_sysfs_remove(tz);
 unregister_tzd:
 	thermal_zone_device_unregister(tz->thermal_zone);
-free_trip_table:
-	kfree(tz->trip_table);
 
 	return result;
 }
@@ -886,9 +839,13 @@ static void acpi_thermal_check_fn(struct
 
 static int acpi_thermal_add(struct acpi_device *device)
 {
+	struct acpi_thermal_trip *acpi_trip;
+	struct thermal_trip *trip;
 	struct acpi_thermal *tz;
 	unsigned int trip_count;
+	int passive_delay = 0;
 	int result;
+	int i;
 
 	if (!device)
 		return -EINVAL;
@@ -930,9 +887,49 @@ static int acpi_thermal_add(struct acpi_
 
 	acpi_thermal_guess_offset(tz);
 
-	result = acpi_thermal_register_thermal_zone(tz, trip_count);
+	trip = kcalloc(trip_count, sizeof(*trip), GFP_KERNEL);
+	if (!trip)
+		return -ENOMEM;
+
+	tz->trip_table = trip;
+
+	if (tz->trips.critical.valid) {
+		trip->type = THERMAL_TRIP_CRITICAL;
+		trip->temperature = acpi_thermal_temp(tz, tz->trips.critical.temperature);
+		trip++;
+	}
+
+	if (tz->trips.hot.valid) {
+		trip->type = THERMAL_TRIP_HOT;
+		trip->temperature = acpi_thermal_temp(tz, tz->trips.hot.temperature);
+		trip++;
+	}
+
+	acpi_trip = &tz->trips.passive.trip;
+	if (acpi_trip->valid) {
+		passive_delay = tz->trips.passive.tsp * 100;
+
+		trip->type = THERMAL_TRIP_PASSIVE;
+		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temperature);
+		trip->priv = acpi_trip;
+		trip++;
+	}
+
+	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
+		acpi_trip =  &tz->trips.active[i].trip;
+
+		if (!acpi_trip->valid)
+			break;
+
+		trip->type = THERMAL_TRIP_ACTIVE;
+		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temperature);
+		trip->priv = acpi_trip;
+		trip++;
+	}
+
+	result = acpi_thermal_register_thermal_zone(tz, trip_count, passive_delay);
 	if (result)
-		goto free_memory;
+		goto free_trips;
 
 	refcount_set(&tz->thermal_check_count, 3);
 	mutex_init(&tz->thermal_check_lock);
@@ -951,6 +948,8 @@ static int acpi_thermal_add(struct acpi_
 flush_wq:
 	flush_workqueue(acpi_thermal_pm_queue);
 	acpi_thermal_unregister_thermal_zone(tz);
+free_trips:
+	kfree(tz->trip_table);
 free_memory:
 	kfree(tz);
 

[PATCH v1 5/9] ACPI: thermal: Simplify critical and hot trips representation

[Rafael J. Wysocki]

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

Notice that the only piece of information regarding the critical and hot
trips that needs to be stored in the driver's local data structures is
whether or not these trips are valid, so drop all of the redundant
information from there and adjust the code accordingly.

Among other things, this requires acpi_thermal_add() to be rearranged
so as to obtain the critical trip temperature before populating the trip
points table and for symmetry, the hot trip temperature is obtained
earlier too.

No intentional functional impact.

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

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -107,10 +107,10 @@ struct acpi_thermal_active {
 };
 
 struct acpi_thermal_trips {
-	struct acpi_thermal_trip critical;
-	struct acpi_thermal_trip hot;
 	struct acpi_thermal_passive passive;
 	struct acpi_thermal_active active[ACPI_THERMAL_MAX_ACTIVE];
+	bool critical_valid;
+	bool hot_valid;
 };
 
 struct acpi_thermal {
@@ -391,7 +391,7 @@ static void acpi_thermal_trips_update(st
 					dev_name(&adev->dev), event, 0);
 }
 
-static void acpi_thermal_get_critical_trip(struct acpi_thermal *tz)
+static long acpi_thermal_get_critical_trip(struct acpi_thermal *tz)
 {
 	unsigned long long tmp;
 	acpi_status status;
@@ -420,34 +420,30 @@ static void acpi_thermal_get_critical_tr
 	}
 
 set:
-	tz->trips.critical.valid = true;
-	tz->trips.critical.temperature = tmp;
-	acpi_handle_debug(tz->device->handle, "Critical threshold [%lu]\n",
-			  tz->trips.critical.temperature);
-	return;
+	tz->trips.critical_valid = true;
+	acpi_handle_debug(tz->device->handle, "Critical threshold [%llu]\n", tmp);
+	return tmp;
 
 fail:
-	tz->trips.critical.valid = false;
-	tz->trips.critical.temperature = THERMAL_TEMP_INVALID;
+	tz->trips.critical_valid = false;
+	return THERMAL_TEMP_INVALID;
 }
 
-static void acpi_thermal_get_hot_trip(struct acpi_thermal *tz)
+static long acpi_thermal_get_hot_trip(struct acpi_thermal *tz)
 {
 	unsigned long long tmp;
 	acpi_status status;
 
 	status = acpi_evaluate_integer(tz->device->handle, "_HOT", NULL, &tmp);
 	if (ACPI_FAILURE(status)) {
-		tz->trips.hot.valid = false;
-		tz->trips.hot.temperature = THERMAL_TEMP_INVALID;
+		tz->trips.hot_valid = false;
 		acpi_handle_debug(tz->device->handle, "No hot threshold\n");
-		return;
+		return THERMAL_TEMP_INVALID;
 	}
 
-	tz->trips.hot.valid = true;
-	tz->trips.hot.temperature = tmp;
-	acpi_handle_debug(tz->device->handle, "Hot threshold [%lu]\n",
-			  tz->trips.hot.temperature);
+	tz->trips.hot_valid = true;
+	acpi_handle_debug(tz->device->handle, "Hot threshold [%llu]\n", tmp);
+	return tmp;
 }
 
 static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
@@ -455,17 +451,9 @@ static int acpi_thermal_get_trip_points(
 	unsigned int count = 0;
 	int i;
 
-	acpi_thermal_get_critical_trip(tz);
-	acpi_thermal_get_hot_trip(tz);
 	/* Passive and active trip points (optional). */
 	__acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT);
 
-	if (tz->trips.critical.valid)
-		count++;
-
-	if (tz->trips.hot.valid)
-		count++;
-
 	if (tz->trips.passive.trip.valid)
 		count++;
 
@@ -578,10 +566,10 @@ static int acpi_thermal_cooling_device_c
 	int trip = -1;
 	int result = 0;
 
-	if (tz->trips.critical.valid)
+	if (tz->trips.critical_valid)
 		trip++;
 
-	if (tz->trips.hot.valid)
+	if (tz->trips.hot_valid)
 		trip++;
 
 	if (tz->trips.passive.trip.valid) {
@@ -803,10 +791,9 @@ static void acpi_thermal_aml_dependency_
  * The heuristic below should work for all ACPI thermal zones which have a
  * critical trip point with a value being a multiple of 0.5 degree Celsius.
  */
-static void acpi_thermal_guess_offset(struct acpi_thermal *tz)
+static void acpi_thermal_guess_offset(struct acpi_thermal *tz, long crit_temp)
 {
-	if (tz->trips.critical.valid &&
-	    (tz->trips.critical.temperature % 5) == 1)
+	if (tz->trips.critical_valid && crit_temp % 5 == 1)
 		tz->kelvin_offset = 273100;
 	else
 		tz->kelvin_offset = 273200;
@@ -843,6 +830,7 @@ static int acpi_thermal_add(struct acpi_
 	struct thermal_trip *trip;
 	struct acpi_thermal *tz;
 	unsigned int trip_count;
+	int crit_temp, hot_temp;
 	int passive_delay = 0;
 	int result;
 	int i;
@@ -864,6 +852,15 @@ static int acpi_thermal_add(struct acpi_
 
 	/* Get trip points [_CRT, _PSV, etc.] (required). */
 	trip_count = acpi_thermal_get_trip_points(tz);
+
+	crit_temp = acpi_thermal_get_critical_trip(tz);
+	if (tz->trips.critical_valid)
+		trip_count++;
+
+	hot_temp = acpi_thermal_get_hot_trip(tz);
+	if (tz->trips.hot_valid)
+		trip_count++;
+
 	if (!trip_count) {
 		pr_warn(FW_BUG "No valid trip points!\n");
 		result = -ENODEV;
@@ -885,7 +882,7 @@ static int acpi_thermal_add(struct acpi_
 	else
 		acpi_thermal_get_polling_frequency(tz);
 
-	acpi_thermal_guess_offset(tz);
+	acpi_thermal_guess_offset(tz, crit_temp);
 
 	trip = kcalloc(trip_count, sizeof(*trip), GFP_KERNEL);
 	if (!trip)
@@ -893,15 +890,15 @@ static int acpi_thermal_add(struct acpi_
 
 	tz->trip_table = trip;
 
-	if (tz->trips.critical.valid) {
+	if (tz->trips.critical_valid) {
 		trip->type = THERMAL_TRIP_CRITICAL;
-		trip->temperature = acpi_thermal_temp(tz, tz->trips.critical.temperature);
+		trip->temperature = acpi_thermal_temp(tz, crit_temp);
 		trip++;
 	}
 
-	if (tz->trips.hot.valid) {
+	if (tz->trips.hot_valid) {
 		trip->type = THERMAL_TRIP_HOT;
-		trip->temperature = acpi_thermal_temp(tz, tz->trips.hot.temperature);
+		trip->temperature = acpi_thermal_temp(tz, hot_temp);
 		trip++;
 	}
 

[PATCH v1 6/9] ACPI: thermal: Untangle initialization and updates of the passive trip

[Rafael J. Wysocki]

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

Separate the code needed to update the passive trip (in a response to a
notification from the platform firmware) as well as to initialize it
from the code that is only necessary for its initialization and cleanly
divide it into functions that each carry out a specific action.

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

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -192,73 +192,6 @@ static void __acpi_thermal_trips_update(
 	bool valid = false;
 	int i;
 
-	/* Passive (optional) */
-	if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips.passive.trip.valid) ||
-	    flag == ACPI_TRIPS_INIT) {
-		valid = tz->trips.passive.trip.valid;
-		if (psv == -1) {
-			status = AE_SUPPORT;
-		} else if (psv > 0) {
-			tmp = celsius_to_deci_kelvin(psv);
-			status = AE_OK;
-		} else {
-			status = acpi_evaluate_integer(tz->device->handle,
-						       "_PSV", NULL, &tmp);
-		}
-
-		if (ACPI_FAILURE(status)) {
-			tz->trips.passive.trip.valid = false;
-		} else {
-			tz->trips.passive.trip.temperature = tmp;
-			tz->trips.passive.trip.valid = true;
-			if (flag == ACPI_TRIPS_INIT) {
-				status = acpi_evaluate_integer(tz->device->handle,
-							       "_TC1", NULL, &tmp);
-				if (ACPI_FAILURE(status))
-					tz->trips.passive.trip.valid = false;
-				else
-					tz->trips.passive.tc1 = tmp;
-
-				status = acpi_evaluate_integer(tz->device->handle,
-							       "_TC2", NULL, &tmp);
-				if (ACPI_FAILURE(status))
-					tz->trips.passive.trip.valid = false;
-				else
-					tz->trips.passive.tc2 = tmp;
-
-				status = acpi_evaluate_integer(tz->device->handle,
-							       "_TSP", NULL, &tmp);
-				if (ACPI_FAILURE(status))
-					tz->trips.passive.trip.valid = false;
-				else
-					tz->trips.passive.tsp = tmp;
-			}
-		}
-	}
-	if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.passive.trip.valid) {
-		memset(&devices, 0, sizeof(struct acpi_handle_list));
-		status = acpi_evaluate_reference(tz->device->handle, "_PSL",
-						 NULL, &devices);
-		if (ACPI_FAILURE(status)) {
-			acpi_handle_info(tz->device->handle,
-					 "Invalid passive threshold\n");
-			tz->trips.passive.trip.valid = false;
-		} else {
-			tz->trips.passive.trip.valid = true;
-		}
-
-		if (memcmp(&tz->trips.passive.devices, &devices,
-			   sizeof(struct acpi_handle_list))) {
-			memcpy(&tz->trips.passive.devices, &devices,
-			       sizeof(struct acpi_handle_list));
-			ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
-		}
-	}
-	if ((flag & ACPI_TRIPS_PASSIVE) || (flag & ACPI_TRIPS_DEVICES)) {
-		if (valid != tz->trips.passive.trip.valid)
-			ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
-	}
-
 	/* Active (optional) */
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
 		char name[5] = { '_', 'A', 'C', ('0' + i), '\0' };
@@ -339,6 +272,72 @@ static void __acpi_thermal_trips_update(
 	}
 }
 
+static void update_acpi_thermal_trip_temp(struct acpi_thermal_trip *acpi_trip,
+					  int temp)
+{
+	acpi_trip->valid = temp != THERMAL_TEMP_INVALID;
+	acpi_trip->temperature = temp;
+}
+
+static long get_passive_temp(struct acpi_thermal *tz)
+{
+	unsigned long long tmp;
+	acpi_status status;
+
+	status = acpi_evaluate_integer(tz->device->handle, "_PSV", NULL, &tmp);
+	if (ACPI_FAILURE(status))
+		return THERMAL_TEMP_INVALID;
+
+	return tmp;
+}
+
+static void acpi_thermal_update_passive_trip(struct acpi_thermal *tz)
+{
+	struct acpi_thermal_trip *acpi_trip = &tz->trips.passive.trip;
+
+	if (!acpi_trip->valid || psv > 0)
+		return;
+
+	update_acpi_thermal_trip_temp(acpi_trip, get_passive_temp(tz));
+	if (!acpi_trip->valid)
+		ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_PASSIVE, tz, "state");
+}
+
+static bool update_passive_devices(struct acpi_thermal *tz, bool compare)
+{
+	struct acpi_handle_list devices;
+	acpi_status status;
+
+	memset(&devices, 0, sizeof(devices));
+
+	status = acpi_evaluate_reference(tz->device->handle, "_PSL", NULL, &devices);
+	if (ACPI_FAILURE(status)) {
+		acpi_handle_info(tz->device->handle,
+				 "Missing device list for passive threshold\n");
+		return false;
+	}
+
+	if (compare && memcmp(&tz->trips.passive.devices, &devices, sizeof(devices)))
+		ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_PASSIVE, tz, "device");
+
+	memcpy(&tz->trips.passive.devices, &devices, sizeof(devices));
+	return true;
+}
+
+static void acpi_thermal_update_passive_devices(struct acpi_thermal *tz)
+{
+	struct acpi_thermal_trip *acpi_trip = &tz->trips.passive.trip;
+
+	if (!acpi_trip->valid)
+		return;
+
+	if (update_passive_devices(tz, true))
+		return;
+
+	update_acpi_thermal_trip_temp(acpi_trip, THERMAL_TEMP_INVALID);
+	ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_PASSIVE, tz, "state");
+}
+
 static int acpi_thermal_adjust_trip(struct thermal_trip *trip, void *data)
 {
 	struct acpi_thermal_trip *acpi_trip = trip->priv;
@@ -359,8 +358,15 @@ static void acpi_thermal_adjust_thermal_
 					     unsigned long data)
 {
 	struct acpi_thermal *tz = thermal_zone_device_priv(thermal);
-	int flag = data == ACPI_THERMAL_NOTIFY_THRESHOLDS ?
-				ACPI_TRIPS_THRESHOLDS : ACPI_TRIPS_DEVICES;
+	int flag;
+
+	if (data == ACPI_THERMAL_NOTIFY_THRESHOLDS) {
+		acpi_thermal_update_passive_trip(tz);
+		flag = ACPI_TRIPS_THRESHOLDS;
+	} else {
+		acpi_thermal_update_passive_devices(tz);
+		flag = ACPI_TRIPS_DEVICES;
+	}
 
 	__acpi_thermal_trips_update(tz, flag);
 
@@ -446,17 +452,63 @@ static long acpi_thermal_get_hot_trip(st
 	return tmp;
 }
 
+static bool acpi_thermal_init_passive_trip(struct acpi_thermal *tz)
+{
+	unsigned long long tmp;
+	acpi_status status;
+	int temp;
+
+	if (psv == -1)
+		goto fail;
+
+	if (psv > 0) {
+		temp = celsius_to_deci_kelvin(psv);
+	} else {
+		temp = get_passive_temp(tz);
+		if (temp == THERMAL_TEMP_INVALID)
+			goto fail;
+	}
+
+	status = acpi_evaluate_integer(tz->device->handle, "_TC1", NULL, &tmp);
+	if (ACPI_FAILURE(status))
+		goto fail;
+
+	tz->trips.passive.tc1 = tmp;
+
+	status = acpi_evaluate_integer(tz->device->handle, "_TC2", NULL, &tmp);
+	if (ACPI_FAILURE(status))
+		goto fail;
+
+	tz->trips.passive.tc2 = tmp;
+
+	status = acpi_evaluate_integer(tz->device->handle, "_TSP", NULL, &tmp);
+	if (ACPI_FAILURE(status))
+		goto fail;
+
+	tz->trips.passive.tsp = tmp;
+
+	if (!update_passive_devices(tz, false))
+		goto fail;
+
+	update_acpi_thermal_trip_temp(&tz->trips.passive.trip, temp);
+	return true;
+
+fail:
+	update_acpi_thermal_trip_temp(&tz->trips.passive.trip, THERMAL_TEMP_INVALID);
+	return false;
+}
+
 static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
 {
 	unsigned int count = 0;
 	int i;
 
-	/* Passive and active trip points (optional). */
-	__acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT);
-
-	if (tz->trips.passive.trip.valid)
+	if (acpi_thermal_init_passive_trip(tz))
 		count++;
 
+	/* Active trip points (optional). */
+	__acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT);
+
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
 		if (tz->trips.active[i].trip.valid)
 			count++;

[PATCH v1 7/9] ACPI: thermal: Untangle initialization and updates of active trips

[Rafael J. Wysocki]

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

Separate the code needed to update active trips (in a response to a
notification from the platform firmware) as well as to initialize them
from the code that is only necessary for their initialization and
cleanly divide it into functions that each carry out a specific action.

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

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -184,94 +184,6 @@ static int acpi_thermal_temp(struct acpi
 						       tz->kelvin_offset);
 }
 
-static void __acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
-{
-	acpi_status status;
-	unsigned long long tmp;
-	struct acpi_handle_list devices;
-	bool valid = false;
-	int i;
-
-	/* Active (optional) */
-	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
-		char name[5] = { '_', 'A', 'C', ('0' + i), '\0' };
-		valid = tz->trips.active[i].trip.valid;
-
-		if (act == -1)
-			break; /* disable all active trip points */
-
-		if (flag == ACPI_TRIPS_INIT || ((flag & ACPI_TRIPS_ACTIVE) &&
-		    tz->trips.active[i].trip.valid)) {
-			status = acpi_evaluate_integer(tz->device->handle,
-						       name, NULL, &tmp);
-			if (ACPI_FAILURE(status)) {
-				tz->trips.active[i].trip.valid = false;
-				if (i == 0)
-					break;
-
-				if (act <= 0)
-					break;
-
-				if (i == 1)
-					tz->trips.active[0].trip.temperature =
-							celsius_to_deci_kelvin(act);
-				else
-					/*
-					 * Don't allow override higher than
-					 * the next higher trip point
-					 */
-					tz->trips.active[i-1].trip.temperature =
-						min_t(unsigned long,
-						      tz->trips.active[i-2].trip.temperature,
-						      celsius_to_deci_kelvin(act));
-
-				break;
-			} else {
-				tz->trips.active[i].trip.temperature = tmp;
-				tz->trips.active[i].trip.valid = true;
-			}
-		}
-
-		name[2] = 'L';
-		if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.active[i].trip.valid) {
-			memset(&devices, 0, sizeof(struct acpi_handle_list));
-			status = acpi_evaluate_reference(tz->device->handle,
-							 name, NULL, &devices);
-			if (ACPI_FAILURE(status)) {
-				acpi_handle_info(tz->device->handle,
-						 "Invalid active%d threshold\n", i);
-				tz->trips.active[i].trip.valid = false;
-			} else {
-				tz->trips.active[i].trip.valid = true;
-			}
-
-			if (memcmp(&tz->trips.active[i].devices, &devices,
-				   sizeof(struct acpi_handle_list))) {
-				memcpy(&tz->trips.active[i].devices, &devices,
-				       sizeof(struct acpi_handle_list));
-				ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
-			}
-		}
-		if ((flag & ACPI_TRIPS_ACTIVE) || (flag & ACPI_TRIPS_DEVICES))
-			if (valid != tz->trips.active[i].trip.valid)
-				ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
-
-		if (!tz->trips.active[i].trip.valid)
-			break;
-	}
-
-	if (flag & ACPI_TRIPS_DEVICES) {
-		memset(&devices, 0, sizeof(devices));
-		status = acpi_evaluate_reference(tz->device->handle, "_TZD",
-						 NULL, &devices);
-		if (ACPI_SUCCESS(status) &&
-		    memcmp(&tz->devices, &devices, sizeof(devices))) {
-			tz->devices = devices;
-			ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
-		}
-	}
-}
-
 static void update_acpi_thermal_trip_temp(struct acpi_thermal_trip *acpi_trip,
 					  int temp)
 {
@@ -338,6 +250,78 @@ static void acpi_thermal_update_passive_
 	ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_PASSIVE, tz, "state");
 }
 
+static long get_active_temp(struct acpi_thermal *tz, int index)
+{
+	char method[] = { '_', 'A', 'C', '0' + index, '\0' };
+	unsigned long long tmp;
+	acpi_status status;
+
+	status = acpi_evaluate_integer(tz->device->handle, method, NULL, &tmp);
+	if (ACPI_FAILURE(status))
+		return THERMAL_TEMP_INVALID;
+
+	/*
+	 * If an override has been provided, apply it so there are no active
+	 * trips with thresholds greater than the override.
+	 */
+	if (act > 0) {
+		unsigned long long override = celsius_to_deci_kelvin(act);
+
+		if (tmp > override)
+			tmp = override;
+	}
+	return tmp;
+}
+
+static void acpi_thermal_update_active_trip(struct acpi_thermal *tz, int index)
+{
+	struct acpi_thermal_trip *acpi_trip = &tz->trips.active[index].trip;
+
+	if (!acpi_trip->valid)
+		return;
+
+	update_acpi_thermal_trip_temp(acpi_trip, get_active_temp(tz, index));
+	if (!acpi_trip->valid)
+		ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "state");
+}
+
+static bool update_active_devices(struct acpi_thermal *tz, int index, bool compare)
+{
+	char method[] = { '_', 'A', 'L', '0' + index, '\0' };
+	struct acpi_handle_list devices;
+	acpi_status status;
+
+	memset(&devices, 0, sizeof(devices));
+
+	status = acpi_evaluate_reference(tz->device->handle, method, NULL, &devices);
+	if (ACPI_FAILURE(status)) {
+		acpi_handle_info(tz->device->handle,
+				 "Missing device list for active threshold %d\n",
+				 index);
+		return false;
+	}
+
+	if (compare && memcmp(&tz->trips.active[index].devices, &devices, sizeof(devices)))
+		ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "device");
+
+	memcpy(&tz->trips.active[index].devices, &devices, sizeof(devices));
+	return true;
+}
+
+static void acpi_thermal_update_active_devices(struct acpi_thermal *tz, int index)
+{
+	struct acpi_thermal_trip *acpi_trip = &tz->trips.active[index].trip;
+
+	if (!acpi_trip->valid)
+		return;
+
+	if (update_active_devices(tz, index, true))
+		return;
+
+	update_acpi_thermal_trip_temp(acpi_trip, THERMAL_TEMP_INVALID);
+	ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "state");
+}
+
 static int acpi_thermal_adjust_trip(struct thermal_trip *trip, void *data)
 {
 	struct acpi_thermal_trip *acpi_trip = trip->priv;
@@ -358,18 +342,18 @@ static void acpi_thermal_adjust_thermal_
 					     unsigned long data)
 {
 	struct acpi_thermal *tz = thermal_zone_device_priv(thermal);
-	int flag;
+	int i;
 
 	if (data == ACPI_THERMAL_NOTIFY_THRESHOLDS) {
 		acpi_thermal_update_passive_trip(tz);
-		flag = ACPI_TRIPS_THRESHOLDS;
+		for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
+			acpi_thermal_update_active_trip(tz, i);
 	} else {
 		acpi_thermal_update_passive_devices(tz);
-		flag = ACPI_TRIPS_DEVICES;
+		for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
+			acpi_thermal_update_active_devices(tz, i);
 	}
 
-	__acpi_thermal_trips_update(tz, flag);
-
 	for_each_thermal_trip(tz->thermal_zone, acpi_thermal_adjust_trip, tz);
 }
 
@@ -498,6 +482,28 @@ fail:
 	return false;
 }
 
+static bool acpi_thermal_init_active_trip(struct acpi_thermal *tz, int index)
+{
+	long temp;
+
+	if (act == -1)
+		goto fail;
+
+	temp = get_active_temp(tz, index);
+	if (temp == THERMAL_TEMP_INVALID)
+		goto fail;
+
+	if (!update_active_devices(tz, false, index))
+		goto fail;
+
+	update_acpi_thermal_trip_temp(&tz->trips.active[index].trip, temp);
+	return true;
+
+fail:
+	update_acpi_thermal_trip_temp(&tz->trips.active[index].trip, THERMAL_TEMP_INVALID);
+	return false;
+}
+
 static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
 {
 	unsigned int count = 0;
@@ -506,11 +512,8 @@ static int acpi_thermal_get_trip_points(
 	if (acpi_thermal_init_passive_trip(tz))
 		count++;
 
-	/* Active trip points (optional). */
-	__acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT);
-
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
-		if (tz->trips.active[i].trip.valid)
+		if (acpi_thermal_init_active_trip(tz, i))
 			count++;
 		else
 			break;

[PATCH v1 8/9] ACPI: thermal: Drop redundant trip point flags

[Rafael J. Wysocki]

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

Trip point flags previously used by the driver need not be used any more
after the preceding changes, so drop them and adjust the code accordingly.

No intentional functional impact.

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

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -43,14 +43,6 @@
 #define ACPI_THERMAL_MAX_ACTIVE		10
 #define ACPI_THERMAL_MAX_LIMIT_STR_LEN	65
 
-#define ACPI_TRIPS_PASSIVE	BIT(0)
-#define ACPI_TRIPS_ACTIVE	BIT(1)
-#define ACPI_TRIPS_DEVICES	BIT(2)
-
-#define ACPI_TRIPS_THRESHOLDS	(ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE)
-
-#define ACPI_TRIPS_INIT		(ACPI_TRIPS_THRESHOLDS | ACPI_TRIPS_DEVICES)
-
 /*
  * This exception is thrown out in two cases:
  * 1.An invalid trip point becomes invalid or a valid trip point becomes invalid
@@ -58,12 +50,11 @@
  * 2.TODO: Devices listed in _PSL, _ALx, _TZD may change.
  *   We need to re-bind the cooling devices of a thermal zone when this occurs.
  */
-#define ACPI_THERMAL_TRIPS_EXCEPTION(flags, tz, str) \
+#define ACPI_THERMAL_TRIPS_EXCEPTION(tz, str) \
 do { \
-	if (flags != ACPI_TRIPS_INIT) \
-		acpi_handle_info(tz->device->handle, \
-			"ACPI thermal trip point %s changed\n" \
-			"Please report to linux-acpi@vger.kernel.org\n", str); \
+	acpi_handle_info(tz->device->handle, \
+			 "ACPI thermal trip point %s changed\n" \
+			 "Please report to linux-acpi@vger.kernel.org\n", str); \
 } while (0)
 
 static int act;
@@ -212,7 +203,7 @@ static void acpi_thermal_update_passive_
 
 	update_acpi_thermal_trip_temp(acpi_trip, get_passive_temp(tz));
 	if (!acpi_trip->valid)
-		ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_PASSIVE, tz, "state");
+		ACPI_THERMAL_TRIPS_EXCEPTION(tz, "state");
 }
 
 static bool update_passive_devices(struct acpi_thermal *tz, bool compare)
@@ -230,7 +221,7 @@ static bool update_passive_devices(struc
 	}
 
 	if (compare && memcmp(&tz->trips.passive.devices, &devices, sizeof(devices)))
-		ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_PASSIVE, tz, "device");
+		ACPI_THERMAL_TRIPS_EXCEPTION(tz, "device");
 
 	memcpy(&tz->trips.passive.devices, &devices, sizeof(devices));
 	return true;
@@ -247,7 +238,7 @@ static void acpi_thermal_update_passive_
 		return;
 
 	update_acpi_thermal_trip_temp(acpi_trip, THERMAL_TEMP_INVALID);
-	ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_PASSIVE, tz, "state");
+	ACPI_THERMAL_TRIPS_EXCEPTION(tz, "state");
 }
 
 static long get_active_temp(struct acpi_thermal *tz, int index)
@@ -282,7 +273,7 @@ static void acpi_thermal_update_active_t
 
 	update_acpi_thermal_trip_temp(acpi_trip, get_active_temp(tz, index));
 	if (!acpi_trip->valid)
-		ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "state");
+		ACPI_THERMAL_TRIPS_EXCEPTION(tz, "state");
 }
 
 static bool update_active_devices(struct acpi_thermal *tz, int index, bool compare)
@@ -302,7 +293,7 @@ static bool update_active_devices(struct
 	}
 
 	if (compare && memcmp(&tz->trips.active[index].devices, &devices, sizeof(devices)))
-		ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "device");
+		ACPI_THERMAL_TRIPS_EXCEPTION(tz, "device");
 
 	memcpy(&tz->trips.active[index].devices, &devices, sizeof(devices));
 	return true;
@@ -319,7 +310,7 @@ static void acpi_thermal_update_active_d
 		return;
 
 	update_acpi_thermal_trip_temp(acpi_trip, THERMAL_TEMP_INVALID);
-	ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "state");
+	ACPI_THERMAL_TRIPS_EXCEPTION(tz, "state");
 }
 
 static int acpi_thermal_adjust_trip(struct thermal_trip *trip, void *data)

[PATCH v1 9/9] ACPI: thermal: Drop valid flag from struct acpi_thermal_trip

[Rafael J. Wysocki]

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

Notice that the valid flag in struct acpi_thermal_trip is in fact
redundant, because the temperature field of invalid trips is always
equal to THERMAL_TEMP_INVALID, so drop it from there and adjust the
code accordingly.

No intentional functional impact.

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

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -81,7 +81,6 @@ static struct workqueue_struct *acpi_the
 
 struct acpi_thermal_trip {
 	unsigned long temperature;
-	bool valid;
 };
 
 struct acpi_thermal_passive {
@@ -175,11 +174,9 @@ static int acpi_thermal_temp(struct acpi
 						       tz->kelvin_offset);
 }
 
-static void update_acpi_thermal_trip_temp(struct acpi_thermal_trip *acpi_trip,
-					  int temp)
+static bool acpi_thermal_trip_valid(struct acpi_thermal_trip *acpi_trip)
 {
-	acpi_trip->valid = temp != THERMAL_TEMP_INVALID;
-	acpi_trip->temperature = temp;
+	return acpi_trip->temperature != THERMAL_TEMP_INVALID;
 }
 
 static long get_passive_temp(struct acpi_thermal *tz)
@@ -198,11 +195,11 @@ static void acpi_thermal_update_passive_
 {
 	struct acpi_thermal_trip *acpi_trip = &tz->trips.passive.trip;
 
-	if (!acpi_trip->valid || psv > 0)
+	if (!acpi_thermal_trip_valid(acpi_trip) || psv > 0)
 		return;
 
-	update_acpi_thermal_trip_temp(acpi_trip, get_passive_temp(tz));
-	if (!acpi_trip->valid)
+	acpi_trip->temperature = get_passive_temp(tz);
+	if (!acpi_thermal_trip_valid(acpi_trip))
 		ACPI_THERMAL_TRIPS_EXCEPTION(tz, "state");
 }
 
@@ -231,13 +228,13 @@ static void acpi_thermal_update_passive_
 {
 	struct acpi_thermal_trip *acpi_trip = &tz->trips.passive.trip;
 
-	if (!acpi_trip->valid)
+	if (!acpi_thermal_trip_valid(acpi_trip))
 		return;
 
 	if (update_passive_devices(tz, true))
 		return;
 
-	update_acpi_thermal_trip_temp(acpi_trip, THERMAL_TEMP_INVALID);
+	acpi_trip->temperature = THERMAL_TEMP_INVALID;
 	ACPI_THERMAL_TRIPS_EXCEPTION(tz, "state");
 }
 
@@ -268,11 +265,11 @@ static void acpi_thermal_update_active_t
 {
 	struct acpi_thermal_trip *acpi_trip = &tz->trips.active[index].trip;
 
-	if (!acpi_trip->valid)
+	if (!acpi_thermal_trip_valid(acpi_trip))
 		return;
 
-	update_acpi_thermal_trip_temp(acpi_trip, get_active_temp(tz, index));
-	if (!acpi_trip->valid)
+	acpi_trip->temperature = get_active_temp(tz, index);
+	if (!acpi_thermal_trip_valid(acpi_trip))
 		ACPI_THERMAL_TRIPS_EXCEPTION(tz, "state");
 }
 
@@ -303,13 +300,13 @@ static void acpi_thermal_update_active_d
 {
 	struct acpi_thermal_trip *acpi_trip = &tz->trips.active[index].trip;
 
-	if (!acpi_trip->valid)
+	if (!acpi_thermal_trip_valid(acpi_trip))
 		return;
 
 	if (update_active_devices(tz, index, true))
 		return;
 
-	update_acpi_thermal_trip_temp(acpi_trip, THERMAL_TEMP_INVALID);
+	acpi_trip->temperature = THERMAL_TEMP_INVALID;
 	ACPI_THERMAL_TRIPS_EXCEPTION(tz, "state");
 }
 
@@ -321,7 +318,7 @@ static int acpi_thermal_adjust_trip(stru
 	if (!acpi_trip)
 		return 0;
 
-	if (acpi_trip->valid)
+	if (acpi_thermal_trip_valid(acpi_trip))
 		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temperature);
 	else
 		trip->temperature = THERMAL_TEMP_INVALID;
@@ -465,11 +462,11 @@ static bool acpi_thermal_init_passive_tr
 	if (!update_passive_devices(tz, false))
 		goto fail;
 
-	update_acpi_thermal_trip_temp(&tz->trips.passive.trip, temp);
+	tz->trips.passive.trip.temperature = temp;
 	return true;
 
 fail:
-	update_acpi_thermal_trip_temp(&tz->trips.passive.trip, THERMAL_TEMP_INVALID);
+	tz->trips.passive.trip.temperature = THERMAL_TEMP_INVALID;
 	return false;
 }
 
@@ -487,11 +484,11 @@ static bool acpi_thermal_init_active_tri
 	if (!update_active_devices(tz, false, index))
 		goto fail;
 
-	update_acpi_thermal_trip_temp(&tz->trips.active[index].trip, temp);
+	tz->trips.active[index].trip.temperature = temp;
 	return true;
 
 fail:
-	update_acpi_thermal_trip_temp(&tz->trips.active[index].trip, THERMAL_TEMP_INVALID);
+	tz->trips.active[index].trip.temperature = THERMAL_TEMP_INVALID;
 	return false;
 }
 
@@ -545,7 +542,7 @@ static int thermal_get_trend(struct ther
 		return -EINVAL;
 
 	acpi_trip = trip->priv;
-	if (!acpi_trip || !acpi_trip->valid)
+	if (!acpi_trip || !acpi_thermal_trip_valid(acpi_trip))
 		return -EINVAL;
 
 	switch (trip->type) {
@@ -618,7 +615,7 @@ static int acpi_thermal_cooling_device_c
 	if (tz->trips.hot_valid)
 		trip++;
 
-	if (tz->trips.passive.trip.valid) {
+	if (acpi_thermal_trip_valid(&tz->trips.passive.trip)) {
 		trip++;
 		for (i = 0; i < tz->trips.passive.devices.count; i++) {
 			handle = tz->trips.passive.devices.handles[i];
@@ -643,7 +640,7 @@ static int acpi_thermal_cooling_device_c
 	}
 
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
-		if (!tz->trips.active[i].trip.valid)
+		if (!acpi_thermal_trip_valid(&tz->trips.active[i].trip))
 			break;
 
 		trip++;
@@ -949,7 +946,7 @@ static int acpi_thermal_add(struct acpi_
 	}
 
 	acpi_trip = &tz->trips.passive.trip;
-	if (acpi_trip->valid) {
+	if (acpi_thermal_trip_valid(acpi_trip)) {
 		passive_delay = tz->trips.passive.tsp * 100;
 
 		trip->type = THERMAL_TRIP_PASSIVE;
@@ -961,7 +958,7 @@ static int acpi_thermal_add(struct acpi_
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
 		acpi_trip =  &tz->trips.active[i].trip;
 
-		if (!acpi_trip->valid)
+		if (!acpi_thermal_trip_valid(acpi_trip))
 			break;
 
 		trip->type = THERMAL_TRIP_ACTIVE;
@@ -1038,7 +1035,7 @@ static int acpi_thermal_resume(struct de
 		return -EINVAL;
 
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
-		if (!tz->trips.active[i].trip.valid)
+		if (!acpi_thermal_trip_valid(&tz->trips.active[i].trip))
 			break;
 
 		for (j = 0; j < tz->trips.active[i].devices.count; j++) {

Re: [PATCH v1 7/9] ACPI: thermal: Untangle initialization and updates of active trips

[Rafael J. Wysocki]

On Tue, Sep 12, 2023 at 8:47 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Separate the code needed to update active trips (in a response to a
> notification from the platform firmware) as well as to initialize them
> from the code that is only necessary for their initialization and
> cleanly divide it into functions that each carry out a specific action.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/acpi/thermal.c |  197 ++++++++++++++++++++++++-------------------------
>  1 file changed, 100 insertions(+), 97 deletions(-)
>
> Index: linux-pm/drivers/acpi/thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/thermal.c
> +++ linux-pm/drivers/acpi/thermal.c
> @@ -184,94 +184,6 @@ static int acpi_thermal_temp(struct acpi
>                                                        tz->kelvin_offset);
>  }
>
> -static void __acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
> -{
> -       acpi_status status;
> -       unsigned long long tmp;
> -       struct acpi_handle_list devices;
> -       bool valid = false;
> -       int i;
> -
> -       /* Active (optional) */
> -       for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
> -               char name[5] = { '_', 'A', 'C', ('0' + i), '\0' };
> -               valid = tz->trips.active[i].trip.valid;
> -
> -               if (act == -1)
> -                       break; /* disable all active trip points */
> -
> -               if (flag == ACPI_TRIPS_INIT || ((flag & ACPI_TRIPS_ACTIVE) &&
> -                   tz->trips.active[i].trip.valid)) {
> -                       status = acpi_evaluate_integer(tz->device->handle,
> -                                                      name, NULL, &tmp);
> -                       if (ACPI_FAILURE(status)) {
> -                               tz->trips.active[i].trip.valid = false;
> -                               if (i == 0)
> -                                       break;
> -
> -                               if (act <= 0)
> -                                       break;
> -
> -                               if (i == 1)
> -                                       tz->trips.active[0].trip.temperature =
> -                                                       celsius_to_deci_kelvin(act);
> -                               else
> -                                       /*
> -                                        * Don't allow override higher than
> -                                        * the next higher trip point
> -                                        */
> -                                       tz->trips.active[i-1].trip.temperature =
> -                                               min_t(unsigned long,
> -                                                     tz->trips.active[i-2].trip.temperature,
> -                                                     celsius_to_deci_kelvin(act));
> -
> -                               break;
> -                       } else {
> -                               tz->trips.active[i].trip.temperature = tmp;
> -                               tz->trips.active[i].trip.valid = true;
> -                       }
> -               }
> -
> -               name[2] = 'L';
> -               if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.active[i].trip.valid) {
> -                       memset(&devices, 0, sizeof(struct acpi_handle_list));
> -                       status = acpi_evaluate_reference(tz->device->handle,
> -                                                        name, NULL, &devices);
> -                       if (ACPI_FAILURE(status)) {
> -                               acpi_handle_info(tz->device->handle,
> -                                                "Invalid active%d threshold\n", i);
> -                               tz->trips.active[i].trip.valid = false;
> -                       } else {
> -                               tz->trips.active[i].trip.valid = true;
> -                       }
> -
> -                       if (memcmp(&tz->trips.active[i].devices, &devices,
> -                                  sizeof(struct acpi_handle_list))) {
> -                               memcpy(&tz->trips.active[i].devices, &devices,
> -                                      sizeof(struct acpi_handle_list));
> -                               ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
> -                       }
> -               }
> -               if ((flag & ACPI_TRIPS_ACTIVE) || (flag & ACPI_TRIPS_DEVICES))
> -                       if (valid != tz->trips.active[i].trip.valid)
> -                               ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
> -
> -               if (!tz->trips.active[i].trip.valid)
> -                       break;
> -       }
> -
> -       if (flag & ACPI_TRIPS_DEVICES) {
> -               memset(&devices, 0, sizeof(devices));
> -               status = acpi_evaluate_reference(tz->device->handle, "_TZD",
> -                                                NULL, &devices);
> -               if (ACPI_SUCCESS(status) &&
> -                   memcmp(&tz->devices, &devices, sizeof(devices))) {
> -                       tz->devices = devices;
> -                       ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
> -               }
> -       }
> -}
> -
>  static void update_acpi_thermal_trip_temp(struct acpi_thermal_trip *acpi_trip,
>                                           int temp)
>  {
> @@ -338,6 +250,78 @@ static void acpi_thermal_update_passive_
>         ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_PASSIVE, tz, "state");
>  }
>
> +static long get_active_temp(struct acpi_thermal *tz, int index)
> +{
> +       char method[] = { '_', 'A', 'C', '0' + index, '\0' };
> +       unsigned long long tmp;
> +       acpi_status status;
> +
> +       status = acpi_evaluate_integer(tz->device->handle, method, NULL, &tmp);
> +       if (ACPI_FAILURE(status))
> +               return THERMAL_TEMP_INVALID;
> +
> +       /*
> +        * If an override has been provided, apply it so there are no active
> +        * trips with thresholds greater than the override.
> +        */
> +       if (act > 0) {
> +               unsigned long long override = celsius_to_deci_kelvin(act);
> +
> +               if (tmp > override)
> +                       tmp = override;
> +       }
> +       return tmp;
> +}
> +
> +static void acpi_thermal_update_active_trip(struct acpi_thermal *tz, int index)
> +{
> +       struct acpi_thermal_trip *acpi_trip = &tz->trips.active[index].trip;
> +
> +       if (!acpi_trip->valid)
> +               return;
> +
> +       update_acpi_thermal_trip_temp(acpi_trip, get_active_temp(tz, index));
> +       if (!acpi_trip->valid)
> +               ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "state");
> +}
> +
> +static bool update_active_devices(struct acpi_thermal *tz, int index, bool compare)
> +{
> +       char method[] = { '_', 'A', 'L', '0' + index, '\0' };
> +       struct acpi_handle_list devices;
> +       acpi_status status;
> +
> +       memset(&devices, 0, sizeof(devices));
> +
> +       status = acpi_evaluate_reference(tz->device->handle, method, NULL, &devices);
> +       if (ACPI_FAILURE(status)) {
> +               acpi_handle_info(tz->device->handle,
> +                                "Missing device list for active threshold %d\n",
> +                                index);
> +               return false;
> +       }
> +
> +       if (compare && memcmp(&tz->trips.active[index].devices, &devices, sizeof(devices)))
> +               ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "device");
> +
> +       memcpy(&tz->trips.active[index].devices, &devices, sizeof(devices));
> +       return true;
> +}
> +
> +static void acpi_thermal_update_active_devices(struct acpi_thermal *tz, int index)
> +{
> +       struct acpi_thermal_trip *acpi_trip = &tz->trips.active[index].trip;
> +
> +       if (!acpi_trip->valid)
> +               return;
> +
> +       if (update_active_devices(tz, index, true))
> +               return;
> +
> +       update_acpi_thermal_trip_temp(acpi_trip, THERMAL_TEMP_INVALID);
> +       ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "state");
> +}
> +
>  static int acpi_thermal_adjust_trip(struct thermal_trip *trip, void *data)
>  {
>         struct acpi_thermal_trip *acpi_trip = trip->priv;
> @@ -358,18 +342,18 @@ static void acpi_thermal_adjust_thermal_
>                                              unsigned long data)
>  {
>         struct acpi_thermal *tz = thermal_zone_device_priv(thermal);
> -       int flag;
> +       int i;
>
>         if (data == ACPI_THERMAL_NOTIFY_THRESHOLDS) {
>                 acpi_thermal_update_passive_trip(tz);
> -               flag = ACPI_TRIPS_THRESHOLDS;
> +               for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
> +                       acpi_thermal_update_active_trip(tz, i);
>         } else {
>                 acpi_thermal_update_passive_devices(tz);
> -               flag = ACPI_TRIPS_DEVICES;
> +               for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
> +                       acpi_thermal_update_active_devices(tz, i);
>         }
>
> -       __acpi_thermal_trips_update(tz, flag);
> -
>         for_each_thermal_trip(tz->thermal_zone, acpi_thermal_adjust_trip, tz);
>  }
>
> @@ -498,6 +482,28 @@ fail:
>         return false;
>  }
>
> +static bool acpi_thermal_init_active_trip(struct acpi_thermal *tz, int index)
> +{
> +       long temp;
> +
> +       if (act == -1)
> +               goto fail;
> +
> +       temp = get_active_temp(tz, index);
> +       if (temp == THERMAL_TEMP_INVALID)
> +               goto fail;
> +
> +       if (!update_active_devices(tz, false, index))

This should be

      if (!update_active_devices(tz, index, false))

and I've already fixed it in the tree.

> +               goto fail;
> +
> +       update_acpi_thermal_trip_temp(&tz->trips.active[index].trip, temp);
> +       return true;
> +
> +fail:
> +       update_acpi_thermal_trip_temp(&tz->trips.active[index].trip, THERMAL_TEMP_INVALID);
> +       return false;
> +}
> +
>  static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
>  {
>         unsigned int count = 0;
> @@ -506,11 +512,8 @@ static int acpi_thermal_get_trip_points(
>         if (acpi_thermal_init_passive_trip(tz))
>                 count++;
>
> -       /* Active trip points (optional). */
> -       __acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT);
> -
>         for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
> -               if (tz->trips.active[i].trip.valid)
> +               if (acpi_thermal_init_active_trip(tz, i))
>                         count++;
>                 else
>                         break;
>
>
>

Re: [PATCH v1 9/9] ACPI: thermal: Drop valid flag from struct acpi_thermal_trip

[Daniel Lezcano]

On Tue, Sep 12, 2023 at 08:47:23PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Notice that the valid flag in struct acpi_thermal_trip is in fact
> redundant, because the temperature field of invalid trips is always
> equal to THERMAL_TEMP_INVALID, so drop it from there and adjust the
> code accordingly.
> 
> 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 v1 1/9] ACPI: thermal: Simplify initialization of critical and hot trips

[Daniel Lezcano]

On 12/09/2023 20:35, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Use the observation that the critical and hot trip points are never
> updated by the ACPI thermal driver, because the flags passed from
> acpi_thermal_notify() to acpi_thermal_trips_update() do not include
> ACPI_TRIPS_CRITICAL or ACPI_TRIPS_HOT, to move the initialization
> of those trip points directly into acpi_thermal_get_trip_points() and
> reduce the size of __acpi_thermal_trips_update().
> 
> Also make the critical and hot trip points initialization code more
> straightforward and drop the flags that are not needed any more.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---

[ ... ]

> +static void acpi_thermal_get_critical_trip(struct acpi_thermal *tz)
> +{
> +	unsigned long long tmp;
> +	acpi_status status;
> +
> +	if (crt > 0) {
> +		tmp = celsius_to_deci_kelvin(crt);
> +		goto set;
> +	}
> +	if (crt == -1) {
> +		acpi_handle_debug(tz->device->handle, "Critical threshold disabled\n");
> +		goto fail;
> +	}
> +
> +	status = acpi_evaluate_integer(tz->device->handle, "_CRT", NULL, &tmp);
> +	if (ACPI_FAILURE(status)) {
> +		acpi_handle_debug(tz->device->handle, "No critical threshold\n");
> +		goto fail;
> +	}
> +	if (tmp <= 2732) {
> +		/*
> +		 * Below zero (Celsius) values clearly aren't right for sure,
> +		 * so discard them as invalid.
> +		 */
> +		pr_info(FW_BUG "Invalid critical threshold (%llu)\n", tmp);
> +		goto fail;
> +	}
> +
> +set:
> +	tz->trips.critical.valid = true;
> +	tz->trips.critical.temperature = tmp;
> +	acpi_handle_debug(tz->device->handle, "Critical threshold [%lu]\n",
> +			  tz->trips.critical.temperature);
> +	return;
> +
> +fail:

nit: 'notset' may be more adequate

> +	tz->trips.critical.valid = false;
> +	tz->trips.critical.temperature = THERMAL_TEMP_INVALID;
> +}

Other than that,

Reviewed-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 v1 2/9] ACPI: thermal: Fold acpi_thermal_get_info() into its caller

[Daniel Lezcano]

On 12/09/2023 20:36, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> There is only one caller of acpi_thermal_get_info() and the code from
> it can be folded into its caller just fine, so do that.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---

Reviewed-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 v1 3/9] ACPI: thermal: Determine the number of trip points earlier

[Daniel Lezcano]

On 12/09/2023 20:37, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Compute the number of trip points in acpi_thermal_add() so as to allow the
> driver's data structures to be simplified going forward.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>


Reviewed-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 v1 4/9] ACPI: thermal: Create and populate trip points table earlier

[Daniel Lezcano]

On 12/09/2023 20:39, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Create and populate the driver's trip points table in acpi_thermal_add()
> so as to allow the its data structures to be simplified going forward.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

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

> ---
>   drivers/acpi/thermal.c |  105 ++++++++++++++++++++++++-------------------------
>   1 file changed, 52 insertions(+), 53 deletions(-)
> 
> Index: linux-pm/drivers/acpi/thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/thermal.c
> +++ linux-pm/drivers/acpi/thermal.c
> @@ -688,53 +688,10 @@ static void acpi_thermal_zone_sysfs_remo
>   }
>   
>   static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz,
> -					      unsigned int trip_count)
> +					      unsigned int trip_count,
> +					      int passive_delay)
>   {
> -	struct acpi_thermal_trip *acpi_trip;
> -	struct thermal_trip *trip;
> -	int passive_delay = 0;
>   	int result;
> -	int i;
> -
> -	trip = kcalloc(trip_count, sizeof(*trip), GFP_KERNEL);
> -	if (!trip)
> -		return -ENOMEM;
> -
> -	tz->trip_table = trip;
> -
> -	if (tz->trips.critical.valid) {
> -		trip->type = THERMAL_TRIP_CRITICAL;
> -		trip->temperature = acpi_thermal_temp(tz, tz->trips.critical.temperature);
> -		trip++;
> -	}
> -
> -	if (tz->trips.hot.valid) {
> -		trip->type = THERMAL_TRIP_HOT;
> -		trip->temperature = acpi_thermal_temp(tz, tz->trips.hot.temperature);
> -		trip++;
> -	}
> -
> -	acpi_trip = &tz->trips.passive.trip;
> -	if (acpi_trip->valid) {
> -		passive_delay = tz->trips.passive.tsp * 100;
> -
> -		trip->type = THERMAL_TRIP_PASSIVE;
> -		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temperature);
> -		trip->priv = acpi_trip;
> -		trip++;
> -	}
> -
> -	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
> -		acpi_trip = &tz->trips.active[i].trip;
> -
> -		if (!acpi_trip->valid)
> -			break;
> -
> -		trip->type = THERMAL_TRIP_ACTIVE;
> -		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temperature);
> -		trip->priv = acpi_trip;
> -		trip++;
> -	}
>   
>   	tz->thermal_zone = thermal_zone_device_register_with_trips("acpitz",
>   								   tz->trip_table,
> @@ -744,10 +701,8 @@ static int acpi_thermal_register_thermal
>   								   NULL,
>   								   passive_delay,
>   								   tz->polling_frequency * 100);
> -	if (IS_ERR(tz->thermal_zone)) {
> -		result = PTR_ERR(tz->thermal_zone);
> -		goto free_trip_table;
> -	}
> +	if (IS_ERR(tz->thermal_zone))
> +		return PTR_ERR(tz->thermal_zone);
>   
>   	result = acpi_thermal_zone_sysfs_add(tz);
>   	if (result)
> @@ -766,8 +721,6 @@ remove_links:
>   	acpi_thermal_zone_sysfs_remove(tz);
>   unregister_tzd:
>   	thermal_zone_device_unregister(tz->thermal_zone);
> -free_trip_table:
> -	kfree(tz->trip_table);
>   
>   	return result;
>   }
> @@ -886,9 +839,13 @@ static void acpi_thermal_check_fn(struct
>   
>   static int acpi_thermal_add(struct acpi_device *device)
>   {
> +	struct acpi_thermal_trip *acpi_trip;
> +	struct thermal_trip *trip;
>   	struct acpi_thermal *tz;
>   	unsigned int trip_count;
> +	int passive_delay = 0;
>   	int result;
> +	int i;
>   
>   	if (!device)
>   		return -EINVAL;
> @@ -930,9 +887,49 @@ static int acpi_thermal_add(struct acpi_
>   
>   	acpi_thermal_guess_offset(tz);
>   
> -	result = acpi_thermal_register_thermal_zone(tz, trip_count);
> +	trip = kcalloc(trip_count, sizeof(*trip), GFP_KERNEL);
> +	if (!trip)
> +		return -ENOMEM;
> +
> +	tz->trip_table = trip;
> +
> +	if (tz->trips.critical.valid) {
> +		trip->type = THERMAL_TRIP_CRITICAL;
> +		trip->temperature = acpi_thermal_temp(tz, tz->trips.critical.temperature);
> +		trip++;
> +	}
> +
> +	if (tz->trips.hot.valid) {
> +		trip->type = THERMAL_TRIP_HOT;
> +		trip->temperature = acpi_thermal_temp(tz, tz->trips.hot.temperature);
> +		trip++;
> +	}
> +
> +	acpi_trip = &tz->trips.passive.trip;
> +	if (acpi_trip->valid) {
> +		passive_delay = tz->trips.passive.tsp * 100;
> +
> +		trip->type = THERMAL_TRIP_PASSIVE;
> +		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temperature);
> +		trip->priv = acpi_trip;
> +		trip++;
> +	}
> +
> +	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
> +		acpi_trip =  &tz->trips.active[i].trip;
> +
> +		if (!acpi_trip->valid)
> +			break;
> +
> +		trip->type = THERMAL_TRIP_ACTIVE;
> +		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temperature);
> +		trip->priv = acpi_trip;
> +		trip++;
> +	}
> +
> +	result = acpi_thermal_register_thermal_zone(tz, trip_count, passive_delay);
>   	if (result)
> -		goto free_memory;
> +		goto free_trips;
>   
>   	refcount_set(&tz->thermal_check_count, 3);
>   	mutex_init(&tz->thermal_check_lock);
> @@ -951,6 +948,8 @@ static int acpi_thermal_add(struct acpi_
>   flush_wq:
>   	flush_workqueue(acpi_thermal_pm_queue);
>   	acpi_thermal_unregister_thermal_zone(tz);
> +free_trips:
> +	kfree(tz->trip_table);
>   free_memory:
>   	kfree(tz);
>   
> 
> 
> 

-- 
<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 v1 5/9] ACPI: thermal: Simplify critical and hot trips representation

[Daniel Lezcano]

On 12/09/2023 20:41, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Notice that the only piece of information regarding the critical and hot
> trips that needs to be stored in the driver's local data structures is
> whether or not these trips are valid, so drop all of the redundant
> information from there and adjust the code accordingly.
> 
> Among other things, this requires acpi_thermal_add() to be rearranged
> so as to obtain the critical trip temperature before populating the trip
> points table and for symmetry, the hot trip temperature is obtained
> earlier too.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Reviewed-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 v1 1/9] ACPI: thermal: Simplify initialization of critical and hot trips

[Rafael J. Wysocki]

On Mon, Sep 25, 2023 at 5:20 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 12/09/2023 20:35, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Use the observation that the critical and hot trip points are never
> > updated by the ACPI thermal driver, because the flags passed from
> > acpi_thermal_notify() to acpi_thermal_trips_update() do not include
> > ACPI_TRIPS_CRITICAL or ACPI_TRIPS_HOT, to move the initialization
> > of those trip points directly into acpi_thermal_get_trip_points() and
> > reduce the size of __acpi_thermal_trips_update().
> >
> > Also make the critical and hot trip points initialization code more
> > straightforward and drop the flags that are not needed any more.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
>
> [ ... ]
>
> > +static void acpi_thermal_get_critical_trip(struct acpi_thermal *tz)
> > +{
> > +     unsigned long long tmp;
> > +     acpi_status status;
> > +
> > +     if (crt > 0) {
> > +             tmp = celsius_to_deci_kelvin(crt);
> > +             goto set;
> > +     }
> > +     if (crt == -1) {
> > +             acpi_handle_debug(tz->device->handle, "Critical threshold disabled\n");
> > +             goto fail;
> > +     }
> > +
> > +     status = acpi_evaluate_integer(tz->device->handle, "_CRT", NULL, &tmp);
> > +     if (ACPI_FAILURE(status)) {
> > +             acpi_handle_debug(tz->device->handle, "No critical threshold\n");
> > +             goto fail;
> > +     }
> > +     if (tmp <= 2732) {
> > +             /*
> > +              * Below zero (Celsius) values clearly aren't right for sure,
> > +              * so discard them as invalid.
> > +              */
> > +             pr_info(FW_BUG "Invalid critical threshold (%llu)\n", tmp);
> > +             goto fail;
> > +     }
> > +
> > +set:
> > +     tz->trips.critical.valid = true;
> > +     tz->trips.critical.temperature = tmp;
> > +     acpi_handle_debug(tz->device->handle, "Critical threshold [%lu]\n",
> > +                       tz->trips.critical.temperature);
> > +     return;
> > +
> > +fail:
>
> nit: 'notset' may be more adequate

Well, this is a bit moot, because the label goes away in one of the
later patches anyway.

> > +     tz->trips.critical.valid = false;
> > +     tz->trips.critical.temperature = THERMAL_TEMP_INVALID;
> > +}
>
> Other than that,
>
> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Thanks!

Re: [PATCH v1 6/9] ACPI: thermal: Untangle initialization and updates of the passive trip

[Daniel Lezcano]

On 12/09/2023 20:43, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Separate the code needed to update the passive trip (in a response to a
> notification from the platform firmware) as well as to initialize it
> from the code that is only necessary for its initialization and cleanly
> divide it into functions that each carry out a specific action.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Reviewed-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 v1 7/9] ACPI: thermal: Untangle initialization and updates of active trips

[Daniel Lezcano]

On 12/09/2023 20:43, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Separate the code needed to update active trips (in a response to a
> notification from the platform firmware) as well as to initialize them
> from the code that is only necessary for their initialization and
> cleanly divide it into functions that each carry out a specific action.
> 
> 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 v1 8/9] ACPI: thermal: Drop redundant trip point flags

[Daniel Lezcano]

On 12/09/2023 20:46, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Trip point flags previously used by the driver need not be used any more
> after the preceding changes, so drop them and adjust the code accordingly.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---

Reviewed-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 v1 9/9] ACPI: thermal: Drop valid flag from struct acpi_thermal_trip

[Daniel Lezcano]

On 12/09/2023 20:47, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Notice that the valid flag in struct acpi_thermal_trip is in fact
> redundant, because the temperature field of invalid trips is always
> equal to THERMAL_TEMP_INVALID, so drop it from there and adjust the
> code accordingly.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Reviewed-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