[PATCH v1 0/3] ACPI: thermal: Clean up simple things

[PATCH v1 0/3] ACPI: thermal: Clean up simple things

[Rafael J. Wysocki]

Hi All,

I've just noticed that the ACPI thermal driver is in a need of extensive
cleanup, so here are just a few simple changes in that direction I would
like to get out of the table quickly before doing more intrusive stuff.

Thanks!

[PATCH v1 1/3] ACPI: thermal: Use white space more consistently

[Rafael J. Wysocki]

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

The usage of white space in the ACPI thermal driver is not very
consistent, so improve that a bit.

While at it, add missing braces to if()/else in a few places.

No functional impact.

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

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -158,7 +158,7 @@ struct acpi_thermal_flags {
 };
 
 struct acpi_thermal {
-	struct acpi_device * device;
+	struct acpi_device *device;
 	acpi_bus_id name;
 	unsigned long temperature;
 	unsigned long last_temperature;
@@ -270,8 +270,7 @@ static int acpi_thermal_trips_update(str
 
 	/* Critical Shutdown */
 	if (flag & ACPI_TRIPS_CRITICAL) {
-		status = acpi_evaluate_integer(tz->device->handle,
-				"_CRT", NULL, &tmp);
+		status = acpi_evaluate_integer(tz->device->handle, "_CRT", NULL, &tmp);
 		tz->trips.critical.temperature = tmp;
 		/*
 		 * Treat freezing temperatures as invalid as well; some
@@ -284,8 +283,7 @@ static int acpi_thermal_trips_update(str
 			acpi_handle_debug(tz->device->handle,
 					  "No critical threshold\n");
 		} else if (tmp <= 2732) {
-			pr_info(FW_BUG "Invalid critical threshold (%llu)\n",
-				tmp);
+			pr_info(FW_BUG "Invalid critical threshold (%llu)\n", tmp);
 			tz->trips.critical.flags.valid = 0;
 		} else {
 			tz->trips.critical.flags.valid = 1;
@@ -312,8 +310,7 @@ static int acpi_thermal_trips_update(str
 
 	/* Critical Sleep (optional) */
 	if (flag & ACPI_TRIPS_HOT) {
-		status = acpi_evaluate_integer(tz->device->handle,
-				"_HOT", NULL, &tmp);
+		status = acpi_evaluate_integer(tz->device->handle, "_HOT", NULL, &tmp);
 		if (ACPI_FAILURE(status)) {
 			tz->trips.hot.flags.valid = 0;
 			acpi_handle_debug(tz->device->handle,
@@ -329,7 +326,7 @@ static int acpi_thermal_trips_update(str
 
 	/* Passive (optional) */
 	if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips.passive.flags.valid) ||
-		(flag == ACPI_TRIPS_INIT)) {
+	    (flag == ACPI_TRIPS_INIT)) {
 		valid = tz->trips.passive.flags.valid;
 		if (psv == -1) {
 			status = AE_SUPPORT;
@@ -338,32 +335,31 @@ static int acpi_thermal_trips_update(str
 			status = AE_OK;
 		} else {
 			status = acpi_evaluate_integer(tz->device->handle,
-				"_PSV", NULL, &tmp);
+						       "_PSV", NULL, &tmp);
 		}
 
-		if (ACPI_FAILURE(status))
+		if (ACPI_FAILURE(status)) {
 			tz->trips.passive.flags.valid = 0;
-		else {
+		} else {
 			tz->trips.passive.temperature = tmp;
 			tz->trips.passive.flags.valid = 1;
 			if (flag == ACPI_TRIPS_INIT) {
-				status = acpi_evaluate_integer(
-						tz->device->handle, "_TC1",
-						NULL, &tmp);
+				status = acpi_evaluate_integer(tz->device->handle,
+							       "_TC1", NULL, &tmp);
 				if (ACPI_FAILURE(status))
 					tz->trips.passive.flags.valid = 0;
 				else
 					tz->trips.passive.tc1 = tmp;
-				status = acpi_evaluate_integer(
-						tz->device->handle, "_TC2",
-						NULL, &tmp);
+
+				status = acpi_evaluate_integer(tz->device->handle,
+							       "_TC2", NULL, &tmp);
 				if (ACPI_FAILURE(status))
 					tz->trips.passive.flags.valid = 0;
 				else
 					tz->trips.passive.tc2 = tmp;
-				status = acpi_evaluate_integer(
-						tz->device->handle, "_TSP",
-						NULL, &tmp);
+
+				status = acpi_evaluate_integer(tz->device->handle,
+							       "_TSP", NULL, &tmp);
 				if (ACPI_FAILURE(status))
 					tz->trips.passive.flags.valid = 0;
 				else
@@ -374,25 +370,25 @@ static int acpi_thermal_trips_update(str
 	if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.passive.flags.valid) {
 		memset(&devices, 0, sizeof(struct acpi_handle_list));
 		status = acpi_evaluate_reference(tz->device->handle, "_PSL",
-							NULL, &devices);
+						 NULL, &devices);
 		if (ACPI_FAILURE(status)) {
 			acpi_handle_info(tz->device->handle,
 					 "Invalid passive threshold\n");
 			tz->trips.passive.flags.valid = 0;
-		}
-		else
+		} else {
 			tz->trips.passive.flags.valid = 1;
+		}
 
 		if (memcmp(&tz->trips.passive.devices, &devices,
-				sizeof(struct acpi_handle_list))) {
+			   sizeof(struct acpi_handle_list))) {
 			memcpy(&tz->trips.passive.devices, &devices,
-				sizeof(struct acpi_handle_list));
+			       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.flags.valid)
-				ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
+			ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
 	}
 
 	/* Active (optional) */
@@ -404,28 +400,30 @@ static int acpi_thermal_trips_update(str
 			break; /* disable all active trip points */
 
 		if ((flag == ACPI_TRIPS_INIT) || ((flag & ACPI_TRIPS_ACTIVE) &&
-			tz->trips.active[i].flags.valid)) {
+		    tz->trips.active[i].flags.valid)) {
 			status = acpi_evaluate_integer(tz->device->handle,
-							name, NULL, &tmp);
+						       name, NULL, &tmp);
 			if (ACPI_FAILURE(status)) {
 				tz->trips.active[i].flags.valid = 0;
 				if (i == 0)
 					break;
+
 				if (act <= 0)
 					break;
+
 				if (i == 1)
-					tz->trips.active[0].temperature =
-						celsius_to_deci_kelvin(act);
+					tz->trips.active[0].temperature = celsius_to_deci_kelvin(act);
 				else
 					/*
 					 * Don't allow override higher than
 					 * the next higher trip point
 					 */
-					tz->trips.active[i - 1].temperature =
-						(tz->trips.active[i - 2].temperature <
+					tz->trips.active[i-1].temperature =
+						(tz->trips.active[i-2].temperature <
 						celsius_to_deci_kelvin(act) ?
-						tz->trips.active[i - 2].temperature :
+						tz->trips.active[i-2].temperature :
 						celsius_to_deci_kelvin(act));
+
 				break;
 			} else {
 				tz->trips.active[i].temperature = tmp;
@@ -434,22 +432,22 @@ static int acpi_thermal_trips_update(str
 		}
 
 		name[2] = 'L';
-		if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.active[i].flags.valid ) {
+		if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.active[i].flags.valid) {
 			memset(&devices, 0, sizeof(struct acpi_handle_list));
 			status = acpi_evaluate_reference(tz->device->handle,
-						name, NULL, &devices);
+							 name, NULL, &devices);
 			if (ACPI_FAILURE(status)) {
 				acpi_handle_info(tz->device->handle,
 						 "Invalid active%d threshold\n", i);
 				tz->trips.active[i].flags.valid = 0;
-			}
-			else
+			} else {
 				tz->trips.active[i].flags.valid = 1;
+			}
 
 			if (memcmp(&tz->trips.active[i].devices, &devices,
-					sizeof(struct acpi_handle_list))) {
+				   sizeof(struct acpi_handle_list))) {
 				memcpy(&tz->trips.active[i].devices, &devices,
-					sizeof(struct acpi_handle_list));
+				       sizeof(struct acpi_handle_list));
 				ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
 			}
 		}
@@ -464,9 +462,9 @@ static int acpi_thermal_trips_update(str
 	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))) {
+						 NULL, &devices);
+		if (ACPI_SUCCESS(status) &&
+		    memcmp(&tz->devices, &devices, sizeof(devices))) {
 			tz->devices = devices;
 			ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
 		}
@@ -548,8 +546,7 @@ static int thermal_get_trip_type(struct
 		trip--;
 	}
 
-	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
-		tz->trips.active[i].flags.valid; i++) {
+	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE && tz->trips.active[i].flags.valid; i++) {
 		if (!trip) {
 			*type = THERMAL_TRIP_ACTIVE;
 			return 0;
@@ -572,8 +569,8 @@ static int thermal_get_trip_temp(struct
 	if (tz->trips.critical.flags.valid) {
 		if (!trip) {
 			*temp = deci_kelvin_to_millicelsius_with_offset(
-				tz->trips.critical.temperature,
-				tz->kelvin_offset);
+					tz->trips.critical.temperature,
+					tz->kelvin_offset);
 			return 0;
 		}
 		trip--;
@@ -582,8 +579,8 @@ static int thermal_get_trip_temp(struct
 	if (tz->trips.hot.flags.valid) {
 		if (!trip) {
 			*temp = deci_kelvin_to_millicelsius_with_offset(
-				tz->trips.hot.temperature,
-				tz->kelvin_offset);
+					tz->trips.hot.temperature,
+					tz->kelvin_offset);
 			return 0;
 		}
 		trip--;
@@ -592,8 +589,8 @@ static int thermal_get_trip_temp(struct
 	if (tz->trips.passive.flags.valid) {
 		if (!trip) {
 			*temp = deci_kelvin_to_millicelsius_with_offset(
-				tz->trips.passive.temperature,
-				tz->kelvin_offset);
+					tz->trips.passive.temperature,
+					tz->kelvin_offset);
 			return 0;
 		}
 		trip--;
@@ -603,8 +600,8 @@ static int thermal_get_trip_temp(struct
 		tz->trips.active[i].flags.valid; i++) {
 		if (!trip) {
 			*temp = deci_kelvin_to_millicelsius_with_offset(
-				tz->trips.active[i].temperature,
-				tz->kelvin_offset);
+					tz->trips.active[i].temperature,
+					tz->kelvin_offset);
 			return 0;
 		}
 		trip--;
@@ -620,15 +617,15 @@ static int thermal_get_crit_temp(struct
 
 	if (tz->trips.critical.flags.valid) {
 		*temperature = deci_kelvin_to_millicelsius_with_offset(
-				tz->trips.critical.temperature,
-				tz->kelvin_offset);
+					tz->trips.critical.temperature,
+					tz->kelvin_offset);
 		return 0;
 	} else
 		return -EINVAL;
 }
 
 static int thermal_get_trend(struct thermal_zone_device *thermal,
-				int trip, enum thermal_trend *trend)
+			     int trip, enum thermal_trend *trend)
 {
 	struct acpi_thermal *tz = thermal->devdata;
 	enum thermal_trip_type type;
@@ -657,9 +654,8 @@ static int thermal_get_trend(struct ther
 	 * tz->temperature has already been updated by generic thermal layer,
 	 * before this callback being invoked
 	 */
-	i = (tz->trips.passive.tc1 * (tz->temperature - tz->last_temperature))
-		+ (tz->trips.passive.tc2
-		* (tz->temperature - tz->trips.passive.temperature));
+	i = (tz->trips.passive.tc1 * (tz->temperature - tz->last_temperature)) +
+	    (tz->trips.passive.tc2 * (tz->temperature - tz->trips.passive.temperature));
 
 	if (i > 0)
 		*trend = THERMAL_TREND_RAISING;
@@ -667,6 +663,7 @@ static int thermal_get_trend(struct ther
 		*trend = THERMAL_TREND_DROPPING;
 	else
 		*trend = THERMAL_TREND_STABLE;
+
 	return 0;
 }
 
@@ -691,8 +688,8 @@ static void acpi_thermal_zone_device_cri
 }
 
 static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
-					struct thermal_cooling_device *cdev,
-					bool bind)
+					  struct thermal_cooling_device *cdev,
+					  bool bind)
 {
 	struct acpi_device *device = cdev->devdata;
 	struct acpi_thermal *tz = thermal->devdata;
@@ -711,22 +708,23 @@ static int acpi_thermal_cooling_device_c
 
 	if (tz->trips.passive.flags.valid) {
 		trip++;
-		for (i = 0; i < tz->trips.passive.devices.count;
-		    i++) {
+		for (i = 0; i < tz->trips.passive.devices.count; i++) {
 			handle = tz->trips.passive.devices.handles[i];
 			dev = acpi_fetch_acpi_dev(handle);
 			if (dev != device)
 				continue;
+
 			if (bind)
-				result =
-					thermal_zone_bind_cooling_device
-					(thermal, trip, cdev,
-					 THERMAL_NO_LIMIT, THERMAL_NO_LIMIT,
-					 THERMAL_WEIGHT_DEFAULT);
+				result = thermal_zone_bind_cooling_device(
+						thermal, trip, cdev,
+						THERMAL_NO_LIMIT,
+						THERMAL_NO_LIMIT,
+						THERMAL_WEIGHT_DEFAULT);
 			else
 				result =
-					thermal_zone_unbind_cooling_device
-					(thermal, trip, cdev);
+					thermal_zone_unbind_cooling_device(
+						thermal, trip, cdev);
+
 			if (result)
 				goto failed;
 		}
@@ -735,22 +733,24 @@ static int acpi_thermal_cooling_device_c
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
 		if (!tz->trips.active[i].flags.valid)
 			break;
+
 		trip++;
-		for (j = 0;
-		    j < tz->trips.active[i].devices.count;
-		    j++) {
+		for (j = 0; j < tz->trips.active[i].devices.count; j++) {
 			handle = tz->trips.active[i].devices.handles[j];
 			dev = acpi_fetch_acpi_dev(handle);
 			if (dev != device)
 				continue;
+
 			if (bind)
-				result = thermal_zone_bind_cooling_device
-					(thermal, trip, cdev,
-					 THERMAL_NO_LIMIT, THERMAL_NO_LIMIT,
-					 THERMAL_WEIGHT_DEFAULT);
+				result = thermal_zone_bind_cooling_device(
+						thermal, trip, cdev,
+						THERMAL_NO_LIMIT,
+						THERMAL_NO_LIMIT,
+						THERMAL_WEIGHT_DEFAULT);
 			else
-				result = thermal_zone_unbind_cooling_device
-					(thermal, trip, cdev);
+				result = thermal_zone_unbind_cooling_device(
+						thermal, trip, cdev);
+
 			if (result)
 				goto failed;
 		}
@@ -762,14 +762,14 @@ failed:
 
 static int
 acpi_thermal_bind_cooling_device(struct thermal_zone_device *thermal,
-					struct thermal_cooling_device *cdev)
+				 struct thermal_cooling_device *cdev)
 {
 	return acpi_thermal_cooling_device_cb(thermal, cdev, true);
 }
 
 static int
 acpi_thermal_unbind_cooling_device(struct thermal_zone_device *thermal,
-					struct thermal_cooling_device *cdev)
+				   struct thermal_cooling_device *cdev)
 {
 	return acpi_thermal_cooling_device_cb(thermal, cdev, false);
 }
@@ -802,20 +802,20 @@ static int acpi_thermal_register_thermal
 	if (tz->trips.passive.flags.valid)
 		trips++;
 
-	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
-			tz->trips.active[i].flags.valid; i++, trips++);
+	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE && tz->trips.active[i].flags.valid;
+	     i++, trips++);
 
 	if (tz->trips.passive.flags.valid)
-		tz->thermal_zone =
-			thermal_zone_device_register("acpitz", trips, 0, tz,
-						&acpi_thermal_zone_ops, NULL,
-						     tz->trips.passive.tsp*100,
-						     tz->polling_frequency*100);
+		tz->thermal_zone = thermal_zone_device_register("acpitz", trips, 0, tz,
+								&acpi_thermal_zone_ops, NULL,
+								tz->trips.passive.tsp * 100,
+								tz->polling_frequency * 100);
 	else
 		tz->thermal_zone =
 			thermal_zone_device_register("acpitz", trips, 0, tz,
-						&acpi_thermal_zone_ops, NULL,
-						0, tz->polling_frequency*100);
+						     &acpi_thermal_zone_ops, NULL,
+						     0, tz->polling_frequency * 100);
+
 	if (IS_ERR(tz->thermal_zone))
 		return -ENODEV;
 
@@ -881,7 +881,6 @@ static void acpi_thermal_notify(struct a
 {
 	struct acpi_thermal *tz = acpi_driver_data(device);
 
-
 	if (!tz)
 		return;
 
@@ -893,13 +892,13 @@ static void acpi_thermal_notify(struct a
 		acpi_thermal_trips_update(tz, ACPI_TRIPS_REFRESH_THRESHOLDS);
 		acpi_queue_thermal_check(tz);
 		acpi_bus_generate_netlink_event(device->pnp.device_class,
-						  dev_name(&device->dev), event, 0);
+						dev_name(&device->dev), event, 0);
 		break;
 	case ACPI_THERMAL_NOTIFY_DEVICES:
 		acpi_thermal_trips_update(tz, ACPI_TRIPS_REFRESH_DEVICES);
 		acpi_queue_thermal_check(tz);
 		acpi_bus_generate_netlink_event(device->pnp.device_class,
-						  dev_name(&device->dev), event, 0);
+						dev_name(&device->dev), event, 0);
 		break;
 	default:
 		acpi_handle_debug(device->handle, "Unsupported event [0x%x]\n",
@@ -944,7 +943,6 @@ static int acpi_thermal_get_info(struct
 {
 	int result = 0;
 
-
 	if (!tz)
 		return -EINVAL;
 
@@ -1023,7 +1021,6 @@ static int acpi_thermal_add(struct acpi_
 	int result = 0;
 	struct acpi_thermal *tz = NULL;
 
-
 	if (!device)
 		return -EINVAL;
 
@@ -1099,6 +1096,7 @@ static int acpi_thermal_resume(struct de
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
 		if (!tz->trips.active[i].flags.valid)
 			break;
+
 		tz->trips.active[i].flags.enabled = 1;
 		for (j = 0; j < tz->trips.active[i].devices.count; j++) {
 			result = acpi_bus_update_power(
@@ -1119,7 +1117,6 @@ static int acpi_thermal_resume(struct de
 #endif
 
 static int thermal_act(const struct dmi_system_id *d) {
-
 	if (act == 0) {
 		pr_notice("%s detected: disabling all active thermal trip points\n",
 			  d->ident);
@@ -1128,14 +1125,12 @@ static int thermal_act(const struct dmi_
 	return 0;
 }
 static int thermal_nocrt(const struct dmi_system_id *d) {
-
 	pr_notice("%s detected: disabling all critical thermal trip point actions.\n",
 		  d->ident);
 	nocrt = 1;
 	return 0;
 }
 static int thermal_tzp(const struct dmi_system_id *d) {
-
 	if (tzp == 0) {
 		pr_notice("%s detected: enabling thermal zone polling\n",
 			  d->ident);
@@ -1144,7 +1139,6 @@ static int thermal_tzp(const struct dmi_
 	return 0;
 }
 static int thermal_psv(const struct dmi_system_id *d) {
-
 	if (psv == 0) {
 		pr_notice("%s detected: disabling all passive thermal trip points\n",
 			  d->ident);

[PATCH v1 2/3] ACPI: thermal: Drop redundant parens from expressions

[Rafael J. Wysocki]

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

Some expressions in the ACPI thermal driver contain redundant
parentheses.  Drop them.

No functional impact.

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

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -326,7 +326,7 @@ static int acpi_thermal_trips_update(str
 
 	/* Passive (optional) */
 	if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips.passive.flags.valid) ||
-	    (flag == ACPI_TRIPS_INIT)) {
+	    flag == ACPI_TRIPS_INIT) {
 		valid = tz->trips.passive.flags.valid;
 		if (psv == -1) {
 			status = AE_SUPPORT;
@@ -399,7 +399,7 @@ static int acpi_thermal_trips_update(str
 		if (act == -1)
 			break; /* disable all active trip points */
 
-		if ((flag == ACPI_TRIPS_INIT) || ((flag & ACPI_TRIPS_ACTIVE) &&
+		if (flag == ACPI_TRIPS_INIT || ((flag & ACPI_TRIPS_ACTIVE) &&
 		    tz->trips.active[i].flags.valid)) {
 			status = acpi_evaluate_integer(tz->device->handle,
 						       name, NULL, &tmp);
@@ -654,8 +654,8 @@ static int thermal_get_trend(struct ther
 	 * tz->temperature has already been updated by generic thermal layer,
 	 * before this callback being invoked
 	 */
-	i = (tz->trips.passive.tc1 * (tz->temperature - tz->last_temperature)) +
-	    (tz->trips.passive.tc2 * (tz->temperature - tz->trips.passive.temperature));
+	i = tz->trips.passive.tc1 * (tz->temperature - tz->last_temperature) +
+	    tz->trips.passive.tc2 * (tz->temperature - tz->trips.passive.temperature);
 
 	if (i > 0)
 		*trend = THERMAL_TREND_RAISING;

[PATCH v1 3/3] ACPI: thermal: Drop some redundant code

[Rafael J. Wysocki]

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

Drop some redundant initialization of local variables, a redundant
return statement and a redundant "else" from the ACPI thermal driver.

No functional impact.

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

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -262,7 +262,7 @@ do {	\
 
 static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 {
-	acpi_status status = AE_OK;
+	acpi_status status;
 	unsigned long long tmp;
 	struct acpi_handle_list devices;
 	int valid = 0;
@@ -620,8 +620,9 @@ static int thermal_get_crit_temp(struct
 					tz->trips.critical.temperature,
 					tz->kelvin_offset);
 		return 0;
-	} else
-		return -EINVAL;
+	}
+
+	return -EINVAL;
 }
 
 static int thermal_get_trend(struct thermal_zone_device *thermal,
@@ -941,7 +942,7 @@ static void acpi_thermal_aml_dependency_
 
 static int acpi_thermal_get_info(struct acpi_thermal *tz)
 {
-	int result = 0;
+	int result;
 
 	if (!tz)
 		return -EINVAL;
@@ -1018,8 +1019,8 @@ static void acpi_thermal_check_fn(struct
 
 static int acpi_thermal_add(struct acpi_device *device)
 {
-	int result = 0;
-	struct acpi_thermal *tz = NULL;
+	struct acpi_thermal *tz;
+	int result;
 
 	if (!device)
 		return -EINVAL;
@@ -1060,7 +1061,7 @@ end:
 
 static int acpi_thermal_remove(struct acpi_device *device)
 {
-	struct acpi_thermal *tz = NULL;
+	struct acpi_thermal *tz;
 
 	if (!device || !acpi_driver_data(device))
 		return -EINVAL;
@@ -1189,7 +1190,7 @@ static const struct dmi_system_id therma
 
 static int __init acpi_thermal_init(void)
 {
-	int result = 0;
+	int result;
 
 	dmi_check_system(thermal_dmi_table);
 
@@ -1216,8 +1217,6 @@ static void __exit acpi_thermal_exit(voi
 {
 	acpi_bus_unregister_driver(&acpi_thermal_driver);
 	destroy_workqueue(acpi_thermal_pm_queue);
-
-	return;
 }
 
 module_init(acpi_thermal_init);

Re: [PATCH v1 0/3] ACPI: thermal: Clean up simple things

[Daniel Lezcano]

On 04/10/2022 18:28, Rafael J. Wysocki wrote:
> Hi All,
> 
> I've just noticed that the ACPI thermal driver is in a need of extensive
> cleanup, so here are just a few simple changes in that direction I would
> like to get out of the table quickly before doing more intrusive stuff.

I've done some cleanups in the ACPI driver. In order to not have 
duplicate effort, shall I send in response to this cover letter a RFC 
series, so we can join our efforts?


-- 
<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 0/3] ACPI: thermal: Clean up simple things

[Rafael J. Wysocki]

On Tue, Oct 4, 2022 at 6:45 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 04/10/2022 18:28, Rafael J. Wysocki wrote:
> > Hi All,
> >
> > I've just noticed that the ACPI thermal driver is in a need of extensive
> > cleanup, so here are just a few simple changes in that direction I would
> > like to get out of the table quickly before doing more intrusive stuff.
>
> I've done some cleanups in the ACPI driver. In order to not have
> duplicate effort, shall I send in response to this cover letter a RFC
> series, so we can join our efforts?

Well, I guess it won't hurt.

[PATCH RFC 0/9] ACPI thermal cleanups

[Daniel Lezcano]

The thermal framework is being cleanup by changing how the thermal trips are
managed. In this process, the ACPI thermal drivers deserves a big cleanup in
order to use the generic trip points.

This series, which is still work in progress, has been tested on an ACPI based
platform.

Daniel Lezcano (9):
  thermal/acpi: Remove the intermediate acpi_thermal_trip structure
  thermal/acpi: Change to a common acpi_thermal_trip structure
  thermal/acpi: Convert the acpi thermal trips to an array
  thermal/acpi: Move the active trip points to the same array
  thermal/acpi: Optimize get_trip_points()
  thermal/acpi: Encapsualte in functions the trip initialization
  thermal/acpi: Simplifify the condition check
  thermal/acpi: Remove active and enabled flags
  thermal/acpi: Rewrite the trip point intialization to use the generic
    thermal trip

 drivers/acpi/thermal.c | 670 ++++++++++++++++++++++++++---------------
 1 file changed, 433 insertions(+), 237 deletions(-)

-- 
2.34.1

[PATCH RFC 1/9] thermal/acpi: Remove the intermediate acpi_thermal_trip structure

[Daniel Lezcano]

The struct acpi_thermal_trips() contains the critical, hot, passive
and active trip points structure. In order to use the generic thermal
trips, let's move out those fields in the struct acpi_thermal instead
of having them encapsulated in an intermediate structure.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/acpi/thermal.c | 183 +++++++++++++++++++++--------------------
 1 file changed, 93 insertions(+), 90 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 539660ef93c7..b2e73e45c6d6 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -166,7 +166,10 @@ struct acpi_thermal {
 	volatile u8 zombie;
 	struct acpi_thermal_flags flags;
 	struct acpi_thermal_state state;
-	struct acpi_thermal_trips trips;
+	struct acpi_thermal_critical critical;
+	struct acpi_thermal_hot hot;
+	struct acpi_thermal_passive passive;
+	struct acpi_thermal_active active[ACPI_THERMAL_MAX_ACTIVE];
 	struct acpi_handle_list devices;
 	struct thermal_zone_device *thermal_zone;
 	int kelvin_offset;	/* in millidegrees */
@@ -272,7 +275,7 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 	if (flag & ACPI_TRIPS_CRITICAL) {
 		status = acpi_evaluate_integer(tz->device->handle,
 				"_CRT", NULL, &tmp);
-		tz->trips.critical.temperature = tmp;
+		tz->critical.temperature = tmp;
 		/*
 		 * Treat freezing temperatures as invalid as well; some
 		 * BIOSes return really low values and cause reboots at startup.
@@ -280,32 +283,32 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 		 * ... so lets discard those as invalid.
 		 */
 		if (ACPI_FAILURE(status)) {
-			tz->trips.critical.flags.valid = 0;
+			tz->critical.flags.valid = 0;
 			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.flags.valid = 0;
+			tz->critical.flags.valid = 0;
 		} else {
-			tz->trips.critical.flags.valid = 1;
+			tz->critical.flags.valid = 1;
 			acpi_handle_debug(tz->device->handle,
 					  "Found critical threshold [%lu]\n",
-					  tz->trips.critical.temperature);
+					  tz->critical.temperature);
 		}
-		if (tz->trips.critical.flags.valid == 1) {
+		if (tz->critical.flags.valid == 1) {
 			if (crt == -1) {
-				tz->trips.critical.flags.valid = 0;
+				tz->critical.flags.valid = 0;
 			} else if (crt > 0) {
 				unsigned long crt_k = celsius_to_deci_kelvin(crt);
 
 				/*
 				 * Allow override critical threshold
 				 */
-				if (crt_k > tz->trips.critical.temperature)
+				if (crt_k > tz->critical.temperature)
 					pr_info("Critical threshold %d C\n", crt);
 
-				tz->trips.critical.temperature = crt_k;
+				tz->critical.temperature = crt_k;
 			}
 		}
 	}
@@ -315,22 +318,22 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 		status = acpi_evaluate_integer(tz->device->handle,
 				"_HOT", NULL, &tmp);
 		if (ACPI_FAILURE(status)) {
-			tz->trips.hot.flags.valid = 0;
+			tz->hot.flags.valid = 0;
 			acpi_handle_debug(tz->device->handle,
 					  "No hot threshold\n");
 		} else {
-			tz->trips.hot.temperature = tmp;
-			tz->trips.hot.flags.valid = 1;
+			tz->hot.temperature = tmp;
+			tz->hot.flags.valid = 1;
 			acpi_handle_debug(tz->device->handle,
 					  "Found hot threshold [%lu]\n",
-					  tz->trips.hot.temperature);
+					  tz->hot.temperature);
 		}
 	}
 
 	/* Passive (optional) */
-	if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips.passive.flags.valid) ||
+	if (((flag & ACPI_TRIPS_PASSIVE) && tz->passive.flags.valid) ||
 		(flag == ACPI_TRIPS_INIT)) {
-		valid = tz->trips.passive.flags.valid;
+		valid = tz->passive.flags.valid;
 		if (psv == -1) {
 			status = AE_SUPPORT;
 		} else if (psv > 0) {
@@ -342,122 +345,122 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 		}
 
 		if (ACPI_FAILURE(status))
-			tz->trips.passive.flags.valid = 0;
+			tz->passive.flags.valid = 0;
 		else {
-			tz->trips.passive.temperature = tmp;
-			tz->trips.passive.flags.valid = 1;
+			tz->passive.temperature = tmp;
+			tz->passive.flags.valid = 1;
 			if (flag == ACPI_TRIPS_INIT) {
 				status = acpi_evaluate_integer(
 						tz->device->handle, "_TC1",
 						NULL, &tmp);
 				if (ACPI_FAILURE(status))
-					tz->trips.passive.flags.valid = 0;
+					tz->passive.flags.valid = 0;
 				else
-					tz->trips.passive.tc1 = tmp;
+					tz->passive.tc1 = tmp;
 				status = acpi_evaluate_integer(
 						tz->device->handle, "_TC2",
 						NULL, &tmp);
 				if (ACPI_FAILURE(status))
-					tz->trips.passive.flags.valid = 0;
+					tz->passive.flags.valid = 0;
 				else
-					tz->trips.passive.tc2 = tmp;
+					tz->passive.tc2 = tmp;
 				status = acpi_evaluate_integer(
 						tz->device->handle, "_TSP",
 						NULL, &tmp);
 				if (ACPI_FAILURE(status))
-					tz->trips.passive.flags.valid = 0;
+					tz->passive.flags.valid = 0;
 				else
-					tz->trips.passive.tsp = tmp;
+					tz->passive.tsp = tmp;
 			}
 		}
 	}
-	if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.passive.flags.valid) {
+	if ((flag & ACPI_TRIPS_DEVICES) && tz->passive.flags.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.flags.valid = 0;
+			tz->passive.flags.valid = 0;
 		}
 		else
-			tz->trips.passive.flags.valid = 1;
+			tz->passive.flags.valid = 1;
 
-		if (memcmp(&tz->trips.passive.devices, &devices,
+		if (memcmp(&tz->passive.devices, &devices,
 				sizeof(struct acpi_handle_list))) {
-			memcpy(&tz->trips.passive.devices, &devices,
+			memcpy(&tz->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.flags.valid)
+		if (valid != tz->passive.flags.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' };
-		valid = tz->trips.active[i].flags.valid;
+		valid = tz->active[i].flags.valid;
 
 		if (act == -1)
 			break; /* disable all active trip points */
 
 		if ((flag == ACPI_TRIPS_INIT) || ((flag & ACPI_TRIPS_ACTIVE) &&
-			tz->trips.active[i].flags.valid)) {
+			tz->active[i].flags.valid)) {
 			status = acpi_evaluate_integer(tz->device->handle,
 							name, NULL, &tmp);
 			if (ACPI_FAILURE(status)) {
-				tz->trips.active[i].flags.valid = 0;
+				tz->active[i].flags.valid = 0;
 				if (i == 0)
 					break;
 				if (act <= 0)
 					break;
 				if (i == 1)
-					tz->trips.active[0].temperature =
+					tz->active[0].temperature =
 						celsius_to_deci_kelvin(act);
 				else
 					/*
 					 * Don't allow override higher than
 					 * the next higher trip point
 					 */
-					tz->trips.active[i - 1].temperature =
-						(tz->trips.active[i - 2].temperature <
+					tz->active[i - 1].temperature =
+						(tz->active[i - 2].temperature <
 						celsius_to_deci_kelvin(act) ?
-						tz->trips.active[i - 2].temperature :
+						tz->active[i - 2].temperature :
 						celsius_to_deci_kelvin(act));
 				break;
 			} else {
-				tz->trips.active[i].temperature = tmp;
-				tz->trips.active[i].flags.valid = 1;
+				tz->active[i].temperature = tmp;
+				tz->active[i].flags.valid = 1;
 			}
 		}
 
 		name[2] = 'L';
-		if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.active[i].flags.valid ) {
+		if ((flag & ACPI_TRIPS_DEVICES) && tz->active[i].flags.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].flags.valid = 0;
+				tz->active[i].flags.valid = 0;
 			}
 			else
-				tz->trips.active[i].flags.valid = 1;
+				tz->active[i].flags.valid = 1;
 
-			if (memcmp(&tz->trips.active[i].devices, &devices,
+			if (memcmp(&tz->active[i].devices, &devices,
 					sizeof(struct acpi_handle_list))) {
-				memcpy(&tz->trips.active[i].devices, &devices,
+				memcpy(&tz->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].flags.valid)
+			if (valid != tz->active[i].flags.valid)
 				ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
 
-		if (!tz->trips.active[i].flags.valid)
+		if (!tz->active[i].flags.valid)
 			break;
 	}
 
@@ -482,12 +485,12 @@ static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
 	if (ret)
 		return ret;
 
-	valid = tz->trips.critical.flags.valid |
-		tz->trips.hot.flags.valid |
-		tz->trips.passive.flags.valid;
+	valid = tz->critical.flags.valid |
+		tz->hot.flags.valid |
+		tz->passive.flags.valid;
 
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
-		valid |= tz->trips.active[i].flags.valid;
+		valid |= tz->active[i].flags.valid;
 
 	if (!valid) {
 		pr_warn(FW_BUG "No valid trip found\n");
@@ -524,7 +527,7 @@ static int thermal_get_trip_type(struct thermal_zone_device *thermal,
 	if (!tz || trip < 0)
 		return -EINVAL;
 
-	if (tz->trips.critical.flags.valid) {
+	if (tz->critical.flags.valid) {
 		if (!trip) {
 			*type = THERMAL_TRIP_CRITICAL;
 			return 0;
@@ -532,7 +535,7 @@ static int thermal_get_trip_type(struct thermal_zone_device *thermal,
 		trip--;
 	}
 
-	if (tz->trips.hot.flags.valid) {
+	if (tz->hot.flags.valid) {
 		if (!trip) {
 			*type = THERMAL_TRIP_HOT;
 			return 0;
@@ -540,7 +543,7 @@ static int thermal_get_trip_type(struct thermal_zone_device *thermal,
 		trip--;
 	}
 
-	if (tz->trips.passive.flags.valid) {
+	if (tz->passive.flags.valid) {
 		if (!trip) {
 			*type = THERMAL_TRIP_PASSIVE;
 			return 0;
@@ -549,7 +552,7 @@ static int thermal_get_trip_type(struct thermal_zone_device *thermal,
 	}
 
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
-		tz->trips.active[i].flags.valid; i++) {
+		tz->active[i].flags.valid; i++) {
 		if (!trip) {
 			*type = THERMAL_TRIP_ACTIVE;
 			return 0;
@@ -569,30 +572,30 @@ static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
 	if (!tz || trip < 0)
 		return -EINVAL;
 
-	if (tz->trips.critical.flags.valid) {
+	if (tz->critical.flags.valid) {
 		if (!trip) {
 			*temp = deci_kelvin_to_millicelsius_with_offset(
-				tz->trips.critical.temperature,
+				tz->critical.temperature,
 				tz->kelvin_offset);
 			return 0;
 		}
 		trip--;
 	}
 
-	if (tz->trips.hot.flags.valid) {
+	if (tz->hot.flags.valid) {
 		if (!trip) {
 			*temp = deci_kelvin_to_millicelsius_with_offset(
-				tz->trips.hot.temperature,
+				tz->hot.temperature,
 				tz->kelvin_offset);
 			return 0;
 		}
 		trip--;
 	}
 
-	if (tz->trips.passive.flags.valid) {
+	if (tz->passive.flags.valid) {
 		if (!trip) {
 			*temp = deci_kelvin_to_millicelsius_with_offset(
-				tz->trips.passive.temperature,
+				tz->passive.temperature,
 				tz->kelvin_offset);
 			return 0;
 		}
@@ -600,10 +603,10 @@ static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
 	}
 
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
-		tz->trips.active[i].flags.valid; i++) {
+		tz->active[i].flags.valid; i++) {
 		if (!trip) {
 			*temp = deci_kelvin_to_millicelsius_with_offset(
-				tz->trips.active[i].temperature,
+				tz->active[i].temperature,
 				tz->kelvin_offset);
 			return 0;
 		}
@@ -618,9 +621,9 @@ static int thermal_get_crit_temp(struct thermal_zone_device *thermal,
 {
 	struct acpi_thermal *tz = thermal->devdata;
 
-	if (tz->trips.critical.flags.valid) {
+	if (tz->critical.flags.valid) {
 		*temperature = deci_kelvin_to_millicelsius_with_offset(
-				tz->trips.critical.temperature,
+				tz->critical.temperature,
 				tz->kelvin_offset);
 		return 0;
 	} else
@@ -657,9 +660,9 @@ static int thermal_get_trend(struct thermal_zone_device *thermal,
 	 * tz->temperature has already been updated by generic thermal layer,
 	 * before this callback being invoked
 	 */
-	i = (tz->trips.passive.tc1 * (tz->temperature - tz->last_temperature))
-		+ (tz->trips.passive.tc2
-		* (tz->temperature - tz->trips.passive.temperature));
+	i = (tz->passive.tc1 * (tz->temperature - tz->last_temperature))
+		+ (tz->passive.tc2
+		* (tz->temperature - tz->passive.temperature));
 
 	if (i > 0)
 		*trend = THERMAL_TREND_RAISING;
@@ -703,17 +706,17 @@ static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
 	int trip = -1;
 	int result = 0;
 
-	if (tz->trips.critical.flags.valid)
+	if (tz->critical.flags.valid)
 		trip++;
 
-	if (tz->trips.hot.flags.valid)
+	if (tz->hot.flags.valid)
 		trip++;
 
-	if (tz->trips.passive.flags.valid) {
+	if (tz->passive.flags.valid) {
 		trip++;
-		for (i = 0; i < tz->trips.passive.devices.count;
+		for (i = 0; i < tz->passive.devices.count;
 		    i++) {
-			handle = tz->trips.passive.devices.handles[i];
+			handle = tz->passive.devices.handles[i];
 			dev = acpi_fetch_acpi_dev(handle);
 			if (dev != device)
 				continue;
@@ -733,13 +736,13 @@ static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
 	}
 
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
-		if (!tz->trips.active[i].flags.valid)
+		if (!tz->active[i].flags.valid)
 			break;
 		trip++;
 		for (j = 0;
-		    j < tz->trips.active[i].devices.count;
+		    j < tz->active[i].devices.count;
 		    j++) {
-			handle = tz->trips.active[i].devices.handles[j];
+			handle = tz->active[i].devices.handles[j];
 			dev = acpi_fetch_acpi_dev(handle);
 			if (dev != device)
 				continue;
@@ -793,23 +796,23 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
 	acpi_status status;
 	int i;
 
-	if (tz->trips.critical.flags.valid)
+	if (tz->critical.flags.valid)
 		trips++;
 
-	if (tz->trips.hot.flags.valid)
+	if (tz->hot.flags.valid)
 		trips++;
 
-	if (tz->trips.passive.flags.valid)
+	if (tz->passive.flags.valid)
 		trips++;
 
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
-			tz->trips.active[i].flags.valid; i++, trips++);
+			tz->active[i].flags.valid; i++, trips++);
 
-	if (tz->trips.passive.flags.valid)
+	if (tz->passive.flags.valid)
 		tz->thermal_zone =
 			thermal_zone_device_register("acpitz", trips, 0, tz,
 						&acpi_thermal_zone_ops, NULL,
-						     tz->trips.passive.tsp*100,
+						     tz->passive.tsp*100,
 						     tz->polling_frequency*100);
 	else
 		tz->thermal_zone =
@@ -986,8 +989,8 @@ static int acpi_thermal_get_info(struct acpi_thermal *tz)
  */
 static void acpi_thermal_guess_offset(struct acpi_thermal *tz)
 {
-	if (tz->trips.critical.flags.valid &&
-	    (tz->trips.critical.temperature % 5) == 1)
+	if (tz->critical.flags.valid &&
+	    (tz->critical.temperature % 5) == 1)
 		tz->kelvin_offset = 273100;
 	else
 		tz->kelvin_offset = 273200;
@@ -1097,19 +1100,19 @@ static int acpi_thermal_resume(struct device *dev)
 		return -EINVAL;
 
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
-		if (!tz->trips.active[i].flags.valid)
+		if (!tz->active[i].flags.valid)
 			break;
-		tz->trips.active[i].flags.enabled = 1;
-		for (j = 0; j < tz->trips.active[i].devices.count; j++) {
+		tz->active[i].flags.enabled = 1;
+		for (j = 0; j < tz->active[i].devices.count; j++) {
 			result = acpi_bus_update_power(
-					tz->trips.active[i].devices.handles[j],
+					tz->active[i].devices.handles[j],
 					&power_state);
 			if (result || (power_state != ACPI_STATE_D0)) {
-				tz->trips.active[i].flags.enabled = 0;
+				tz->active[i].flags.enabled = 0;
 				break;
 			}
 		}
-		tz->state.active |= tz->trips.active[i].flags.enabled;
+		tz->state.active |= tz->active[i].flags.enabled;
 	}
 
 	acpi_queue_thermal_check(tz);
-- 
2.34.1

[PATCH RFC 2/9] thermal/acpi: Change to a common acpi_thermal_trip structure

[Daniel Lezcano]

Do not differentiate hot, critical, passive and active trip
points. Use a single acpi_thermal_trip structure.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/acpi/thermal.c | 35 ++++++-----------------------------
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index b2e73e45c6d6..9620128f05d2 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -119,36 +119,13 @@ struct acpi_thermal_state_flags {
 	u8 reserved:6;
 };
 
-struct acpi_thermal_critical {
-	struct acpi_thermal_state_flags flags;
-	unsigned long temperature;
-};
-
-struct acpi_thermal_hot {
-	struct acpi_thermal_state_flags flags;
-	unsigned long temperature;
-};
-
-struct acpi_thermal_passive {
+struct acpi_thermal_trip {
 	struct acpi_thermal_state_flags flags;
+	struct acpi_handle_list devices;
 	unsigned long temperature;
 	unsigned long tc1;
 	unsigned long tc2;
 	unsigned long tsp;
-	struct acpi_handle_list devices;
-};
-
-struct acpi_thermal_active {
-	struct acpi_thermal_state_flags flags;
-	unsigned long temperature;
-	struct acpi_handle_list devices;
-};
-
-struct acpi_thermal_trips {
-	struct acpi_thermal_critical critical;
-	struct acpi_thermal_hot hot;
-	struct acpi_thermal_passive passive;
-	struct acpi_thermal_active active[ACPI_THERMAL_MAX_ACTIVE];
 };
 
 struct acpi_thermal_flags {
@@ -166,10 +143,10 @@ struct acpi_thermal {
 	volatile u8 zombie;
 	struct acpi_thermal_flags flags;
 	struct acpi_thermal_state state;
-	struct acpi_thermal_critical critical;
-	struct acpi_thermal_hot hot;
-	struct acpi_thermal_passive passive;
-	struct acpi_thermal_active active[ACPI_THERMAL_MAX_ACTIVE];
+	struct acpi_thermal_trip critical;
+	struct acpi_thermal_trip hot;
+	struct acpi_thermal_trip passive;
+	struct acpi_thermal_trip active[ACPI_THERMAL_MAX_ACTIVE];
 	struct acpi_handle_list devices;
 	struct thermal_zone_device *thermal_zone;
 	int kelvin_offset;	/* in millidegrees */
-- 
2.34.1

[PATCH RFC 3/9] thermal/acpi: Convert the acpi thermal trips to an array

[Daniel Lezcano]

Instead of having multiple trip points in the structure fields for
each trip type, let's create an array of trip points.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/acpi/thermal.c | 131 ++++++++++++++++++++++-------------------
 1 file changed, 69 insertions(+), 62 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 9620128f05d2..8bf2b25acdf1 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -104,6 +104,15 @@ static struct acpi_driver acpi_thermal_driver = {
 	.drv.pm = &acpi_thermal_pm,
 };
 
+enum {
+	ACPI_THERMAL_TRIP_CRITICAL,
+	ACPI_THERMAL_TRIP_HOT,
+	ACPI_THERMAL_TRIP_PASSIVE,
+	ACPI_THERMAL_TRIP_ACTIVE
+};
+
+#define ACPI_THERMAL_TRIP_MAX (ACPI_THERMAL_TRIP_ACTIVE + ACPI_THERMAL_MAX_ACTIVE)
+
 struct acpi_thermal_state {
 	u8 critical:1;
 	u8 hot:1;
@@ -143,9 +152,7 @@ struct acpi_thermal {
 	volatile u8 zombie;
 	struct acpi_thermal_flags flags;
 	struct acpi_thermal_state state;
-	struct acpi_thermal_trip critical;
-	struct acpi_thermal_trip hot;
-	struct acpi_thermal_trip passive;
+	struct acpi_thermal_trip trips[ACPI_THERMAL_TRIP_MAX];
 	struct acpi_thermal_trip active[ACPI_THERMAL_MAX_ACTIVE];
 	struct acpi_handle_list devices;
 	struct thermal_zone_device *thermal_zone;
@@ -252,7 +259,7 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 	if (flag & ACPI_TRIPS_CRITICAL) {
 		status = acpi_evaluate_integer(tz->device->handle,
 				"_CRT", NULL, &tmp);
-		tz->critical.temperature = tmp;
+		tz->trips[ACPI_THERMAL_TRIP_CRITICAL].temperature = tmp;
 		/*
 		 * Treat freezing temperatures as invalid as well; some
 		 * BIOSes return really low values and cause reboots at startup.
@@ -260,32 +267,32 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 		 * ... so lets discard those as invalid.
 		 */
 		if (ACPI_FAILURE(status)) {
-			tz->critical.flags.valid = 0;
+			tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid = 0;
 			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->critical.flags.valid = 0;
+			tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid = 0;
 		} else {
-			tz->critical.flags.valid = 1;
+			tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid = 1;
 			acpi_handle_debug(tz->device->handle,
 					  "Found critical threshold [%lu]\n",
-					  tz->critical.temperature);
+					  tz->trips[ACPI_THERMAL_TRIP_CRITICAL].temperature);
 		}
-		if (tz->critical.flags.valid == 1) {
+		if (tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid == 1) {
 			if (crt == -1) {
-				tz->critical.flags.valid = 0;
+				tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid = 0;
 			} else if (crt > 0) {
 				unsigned long crt_k = celsius_to_deci_kelvin(crt);
 
 				/*
 				 * Allow override critical threshold
 				 */
-				if (crt_k > tz->critical.temperature)
+				if (crt_k > tz->trips[ACPI_THERMAL_TRIP_CRITICAL].temperature)
 					pr_info("Critical threshold %d C\n", crt);
 
-				tz->critical.temperature = crt_k;
+				tz->trips[ACPI_THERMAL_TRIP_CRITICAL].temperature = crt_k;
 			}
 		}
 	}
@@ -295,22 +302,22 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 		status = acpi_evaluate_integer(tz->device->handle,
 				"_HOT", NULL, &tmp);
 		if (ACPI_FAILURE(status)) {
-			tz->hot.flags.valid = 0;
+			tz->trips[ACPI_THERMAL_TRIP_HOT].flags.valid = 0;
 			acpi_handle_debug(tz->device->handle,
 					  "No hot threshold\n");
 		} else {
-			tz->hot.temperature = tmp;
-			tz->hot.flags.valid = 1;
+			tz->trips[ACPI_THERMAL_TRIP_HOT].temperature = tmp;
+			tz->trips[ACPI_THERMAL_TRIP_HOT].flags.valid = 1;
 			acpi_handle_debug(tz->device->handle,
 					  "Found hot threshold [%lu]\n",
-					  tz->hot.temperature);
+					  tz->trips[ACPI_THERMAL_TRIP_HOT].temperature);
 		}
 	}
 
 	/* Passive (optional) */
-	if (((flag & ACPI_TRIPS_PASSIVE) && tz->passive.flags.valid) ||
+	if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid) ||
 		(flag == ACPI_TRIPS_INIT)) {
-		valid = tz->passive.flags.valid;
+		valid = tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid;
 		if (psv == -1) {
 			status = AE_SUPPORT;
 		} else if (psv > 0) {
@@ -322,56 +329,56 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 		}
 
 		if (ACPI_FAILURE(status))
-			tz->passive.flags.valid = 0;
+			tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 0;
 		else {
-			tz->passive.temperature = tmp;
-			tz->passive.flags.valid = 1;
+			tz->trips[ACPI_THERMAL_TRIP_PASSIVE].temperature = tmp;
+			tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 1;
 			if (flag == ACPI_TRIPS_INIT) {
 				status = acpi_evaluate_integer(
 						tz->device->handle, "_TC1",
 						NULL, &tmp);
 				if (ACPI_FAILURE(status))
-					tz->passive.flags.valid = 0;
+					tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 0;
 				else
-					tz->passive.tc1 = tmp;
+					tz->trips[ACPI_THERMAL_TRIP_PASSIVE].tc1 = tmp;
 				status = acpi_evaluate_integer(
 						tz->device->handle, "_TC2",
 						NULL, &tmp);
 				if (ACPI_FAILURE(status))
-					tz->passive.flags.valid = 0;
+					tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 0;
 				else
-					tz->passive.tc2 = tmp;
+					tz->trips[ACPI_THERMAL_TRIP_PASSIVE].tc2 = tmp;
 				status = acpi_evaluate_integer(
 						tz->device->handle, "_TSP",
 						NULL, &tmp);
 				if (ACPI_FAILURE(status))
-					tz->passive.flags.valid = 0;
+					tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 0;
 				else
-					tz->passive.tsp = tmp;
+					tz->trips[ACPI_THERMAL_TRIP_PASSIVE].tsp = tmp;
 			}
 		}
 	}
-	if ((flag & ACPI_TRIPS_DEVICES) && tz->passive.flags.valid) {
+	if ((flag & ACPI_TRIPS_DEVICES) && tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.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->passive.flags.valid = 0;
+			tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 0;
 		}
 		else
-			tz->passive.flags.valid = 1;
+			tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 1;
 
-		if (memcmp(&tz->passive.devices, &devices,
+		if (memcmp(&tz->trips[ACPI_THERMAL_TRIP_PASSIVE].devices, &devices,
 				sizeof(struct acpi_handle_list))) {
-			memcpy(&tz->passive.devices, &devices,
+			memcpy(&tz->trips[ACPI_THERMAL_TRIP_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->passive.flags.valid)
+		if (valid != tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid)
 				ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
 	}
 
@@ -462,9 +469,9 @@ static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
 	if (ret)
 		return ret;
 
-	valid = tz->critical.flags.valid |
-		tz->hot.flags.valid |
-		tz->passive.flags.valid;
+	valid = tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid |
+		tz->trips[ACPI_THERMAL_TRIP_HOT].flags.valid |
+		tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid;
 
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
 		valid |= tz->active[i].flags.valid;
@@ -504,7 +511,7 @@ static int thermal_get_trip_type(struct thermal_zone_device *thermal,
 	if (!tz || trip < 0)
 		return -EINVAL;
 
-	if (tz->critical.flags.valid) {
+	if (tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid) {
 		if (!trip) {
 			*type = THERMAL_TRIP_CRITICAL;
 			return 0;
@@ -512,7 +519,7 @@ static int thermal_get_trip_type(struct thermal_zone_device *thermal,
 		trip--;
 	}
 
-	if (tz->hot.flags.valid) {
+	if (tz->trips[ACPI_THERMAL_TRIP_HOT].flags.valid) {
 		if (!trip) {
 			*type = THERMAL_TRIP_HOT;
 			return 0;
@@ -520,7 +527,7 @@ static int thermal_get_trip_type(struct thermal_zone_device *thermal,
 		trip--;
 	}
 
-	if (tz->passive.flags.valid) {
+	if (tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid) {
 		if (!trip) {
 			*type = THERMAL_TRIP_PASSIVE;
 			return 0;
@@ -549,30 +556,30 @@ static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
 	if (!tz || trip < 0)
 		return -EINVAL;
 
-	if (tz->critical.flags.valid) {
+	if (tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid) {
 		if (!trip) {
 			*temp = deci_kelvin_to_millicelsius_with_offset(
-				tz->critical.temperature,
+				tz->trips[ACPI_THERMAL_TRIP_CRITICAL].temperature,
 				tz->kelvin_offset);
 			return 0;
 		}
 		trip--;
 	}
 
-	if (tz->hot.flags.valid) {
+	if (tz->trips[ACPI_THERMAL_TRIP_HOT].flags.valid) {
 		if (!trip) {
 			*temp = deci_kelvin_to_millicelsius_with_offset(
-				tz->hot.temperature,
+				tz->trips[ACPI_THERMAL_TRIP_HOT].temperature,
 				tz->kelvin_offset);
 			return 0;
 		}
 		trip--;
 	}
 
-	if (tz->passive.flags.valid) {
+	if (tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid) {
 		if (!trip) {
 			*temp = deci_kelvin_to_millicelsius_with_offset(
-				tz->passive.temperature,
+				tz->trips[ACPI_THERMAL_TRIP_PASSIVE].temperature,
 				tz->kelvin_offset);
 			return 0;
 		}
@@ -598,9 +605,9 @@ static int thermal_get_crit_temp(struct thermal_zone_device *thermal,
 {
 	struct acpi_thermal *tz = thermal->devdata;
 
-	if (tz->critical.flags.valid) {
+	if (tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid) {
 		*temperature = deci_kelvin_to_millicelsius_with_offset(
-				tz->critical.temperature,
+				tz->trips[ACPI_THERMAL_TRIP_CRITICAL].temperature,
 				tz->kelvin_offset);
 		return 0;
 	} else
@@ -637,9 +644,9 @@ static int thermal_get_trend(struct thermal_zone_device *thermal,
 	 * tz->temperature has already been updated by generic thermal layer,
 	 * before this callback being invoked
 	 */
-	i = (tz->passive.tc1 * (tz->temperature - tz->last_temperature))
-		+ (tz->passive.tc2
-		* (tz->temperature - tz->passive.temperature));
+	i = (tz->trips[ACPI_THERMAL_TRIP_PASSIVE].tc1 * (tz->temperature - tz->last_temperature))
+		+ (tz->trips[ACPI_THERMAL_TRIP_PASSIVE].tc2
+		* (tz->temperature - tz->trips[ACPI_THERMAL_TRIP_PASSIVE].temperature));
 
 	if (i > 0)
 		*trend = THERMAL_TREND_RAISING;
@@ -683,17 +690,17 @@ static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
 	int trip = -1;
 	int result = 0;
 
-	if (tz->critical.flags.valid)
+	if (tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid)
 		trip++;
 
-	if (tz->hot.flags.valid)
+	if (tz->trips[ACPI_THERMAL_TRIP_HOT].flags.valid)
 		trip++;
 
-	if (tz->passive.flags.valid) {
+	if (tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid) {
 		trip++;
-		for (i = 0; i < tz->passive.devices.count;
+		for (i = 0; i < tz->trips[ACPI_THERMAL_TRIP_PASSIVE].devices.count;
 		    i++) {
-			handle = tz->passive.devices.handles[i];
+			handle = tz->trips[ACPI_THERMAL_TRIP_PASSIVE].devices.handles[i];
 			dev = acpi_fetch_acpi_dev(handle);
 			if (dev != device)
 				continue;
@@ -773,23 +780,23 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
 	acpi_status status;
 	int i;
 
-	if (tz->critical.flags.valid)
+	if (tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid)
 		trips++;
 
-	if (tz->hot.flags.valid)
+	if (tz->trips[ACPI_THERMAL_TRIP_HOT].flags.valid)
 		trips++;
 
-	if (tz->passive.flags.valid)
+	if (tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid)
 		trips++;
 
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
 			tz->active[i].flags.valid; i++, trips++);
 
-	if (tz->passive.flags.valid)
+	if (tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid)
 		tz->thermal_zone =
 			thermal_zone_device_register("acpitz", trips, 0, tz,
 						&acpi_thermal_zone_ops, NULL,
-						     tz->passive.tsp*100,
+						     tz->trips[ACPI_THERMAL_TRIP_PASSIVE].tsp*100,
 						     tz->polling_frequency*100);
 	else
 		tz->thermal_zone =
@@ -966,8 +973,8 @@ static int acpi_thermal_get_info(struct acpi_thermal *tz)
  */
 static void acpi_thermal_guess_offset(struct acpi_thermal *tz)
 {
-	if (tz->critical.flags.valid &&
-	    (tz->critical.temperature % 5) == 1)
+	if (tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid &&
+	    (tz->trips[ACPI_THERMAL_TRIP_CRITICAL].temperature % 5) == 1)
 		tz->kelvin_offset = 273100;
 	else
 		tz->kelvin_offset = 273200;
-- 
2.34.1

[PATCH RFC 4/9] thermal/acpi: Move the active trip points to the same array

[Daniel Lezcano]

This change does the second pass to move the active trip points in the
thermal trip array.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/acpi/thermal.c | 75 +++++++++++++++++++++---------------------
 1 file changed, 37 insertions(+), 38 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 8bf2b25acdf1..ce37494bd133 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -153,7 +153,6 @@ struct acpi_thermal {
 	struct acpi_thermal_flags flags;
 	struct acpi_thermal_state state;
 	struct acpi_thermal_trip trips[ACPI_THERMAL_TRIP_MAX];
-	struct acpi_thermal_trip active[ACPI_THERMAL_MAX_ACTIVE];
 	struct acpi_handle_list devices;
 	struct thermal_zone_device *thermal_zone;
 	int kelvin_offset;	/* in millidegrees */
@@ -383,68 +382,68 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 	}
 
 	/* Active (optional) */
-	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
+	for (i = ACPI_THERMAL_TRIP_ACTIVE; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
 		char name[5] = { '_', 'A', 'C', ('0' + i), '\0' };
-		valid = tz->active[i].flags.valid;
+		valid = tz->trips[i].flags.valid;
 
 		if (act == -1)
 			break; /* disable all active trip points */
 
 		if ((flag == ACPI_TRIPS_INIT) || ((flag & ACPI_TRIPS_ACTIVE) &&
-			tz->active[i].flags.valid)) {
+			tz->trips[i].flags.valid)) {
 			status = acpi_evaluate_integer(tz->device->handle,
 							name, NULL, &tmp);
 			if (ACPI_FAILURE(status)) {
-				tz->active[i].flags.valid = 0;
+				tz->trips[i].flags.valid = 0;
 				if (i == 0)
 					break;
 				if (act <= 0)
 					break;
 				if (i == 1)
-					tz->active[0].temperature =
+					tz->trips[0].temperature =
 						celsius_to_deci_kelvin(act);
 				else
 					/*
 					 * Don't allow override higher than
 					 * the next higher trip point
 					 */
-					tz->active[i - 1].temperature =
-						(tz->active[i - 2].temperature <
+					tz->trips[i - 1].temperature =
+						(tz->trips[i - 2].temperature <
 						celsius_to_deci_kelvin(act) ?
-						tz->active[i - 2].temperature :
+						tz->trips[i - 2].temperature :
 						celsius_to_deci_kelvin(act));
 				break;
 			} else {
-				tz->active[i].temperature = tmp;
-				tz->active[i].flags.valid = 1;
+				tz->trips[i].temperature = tmp;
+				tz->trips[i].flags.valid = 1;
 			}
 		}
 
 		name[2] = 'L';
-		if ((flag & ACPI_TRIPS_DEVICES) && tz->active[i].flags.valid ) {
+		if ((flag & ACPI_TRIPS_DEVICES) && tz->trips[i].flags.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->active[i].flags.valid = 0;
+				tz->trips[i].flags.valid = 0;
 			}
 			else
-				tz->active[i].flags.valid = 1;
+				tz->trips[i].flags.valid = 1;
 
-			if (memcmp(&tz->active[i].devices, &devices,
+			if (memcmp(&tz->trips[i].devices, &devices,
 					sizeof(struct acpi_handle_list))) {
-				memcpy(&tz->active[i].devices, &devices,
+				memcpy(&tz->trips[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->active[i].flags.valid)
+			if (valid != tz->trips[i].flags.valid)
 				ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
 
-		if (!tz->active[i].flags.valid)
+		if (!tz->trips[i].flags.valid)
 			break;
 	}
 
@@ -473,8 +472,8 @@ static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
 		tz->trips[ACPI_THERMAL_TRIP_HOT].flags.valid |
 		tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid;
 
-	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
-		valid |= tz->active[i].flags.valid;
+	for (i = ACPI_THERMAL_TRIP_ACTIVE; i < ACPI_THERMAL_MAX_ACTIVE; i++)
+		valid |= tz->trips[i].flags.valid;
 
 	if (!valid) {
 		pr_warn(FW_BUG "No valid trip found\n");
@@ -535,8 +534,8 @@ static int thermal_get_trip_type(struct thermal_zone_device *thermal,
 		trip--;
 	}
 
-	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
-		tz->active[i].flags.valid; i++) {
+	for (i = ACPI_THERMAL_TRIP_ACTIVE; i < ACPI_THERMAL_MAX_ACTIVE &&
+		tz->trips[i].flags.valid; i++) {
 		if (!trip) {
 			*type = THERMAL_TRIP_ACTIVE;
 			return 0;
@@ -586,11 +585,11 @@ static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
 		trip--;
 	}
 
-	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
-		tz->active[i].flags.valid; i++) {
+	for (i = ACPI_THERMAL_TRIP_ACTIVE; i < ACPI_THERMAL_MAX_ACTIVE &&
+		tz->trips[i].flags.valid; i++) {
 		if (!trip) {
 			*temp = deci_kelvin_to_millicelsius_with_offset(
-				tz->active[i].temperature,
+				tz->trips[i].temperature,
 				tz->kelvin_offset);
 			return 0;
 		}
@@ -719,14 +718,14 @@ static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
 		}
 	}
 
-	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
-		if (!tz->active[i].flags.valid)
+	for (i = ACPI_THERMAL_TRIP_ACTIVE; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
+		if (!tz->trips[i].flags.valid)
 			break;
 		trip++;
 		for (j = 0;
-		    j < tz->active[i].devices.count;
+		    j < tz->trips[i].devices.count;
 		    j++) {
-			handle = tz->active[i].devices.handles[j];
+			handle = tz->trips[i].devices.handles[j];
 			dev = acpi_fetch_acpi_dev(handle);
 			if (dev != device)
 				continue;
@@ -789,8 +788,8 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
 	if (tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid)
 		trips++;
 
-	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
-			tz->active[i].flags.valid; i++, trips++);
+	for (i = ACPI_THERMAL_TRIP_ACTIVE; i < ACPI_THERMAL_MAX_ACTIVE &&
+			tz->trips[i].flags.valid; i++, trips++);
 
 	if (tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid)
 		tz->thermal_zone =
@@ -1083,20 +1082,20 @@ static int acpi_thermal_resume(struct device *dev)
 	if (!tz)
 		return -EINVAL;
 
-	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
-		if (!tz->active[i].flags.valid)
+	for (i = ACPI_THERMAL_TRIP_ACTIVE; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
+		if (!tz->trips[i].flags.valid)
 			break;
-		tz->active[i].flags.enabled = 1;
-		for (j = 0; j < tz->active[i].devices.count; j++) {
+		tz->trips[i].flags.enabled = 1;
+		for (j = 0; j < tz->trips[i].devices.count; j++) {
 			result = acpi_bus_update_power(
-					tz->active[i].devices.handles[j],
+					tz->trips[i].devices.handles[j],
 					&power_state);
 			if (result || (power_state != ACPI_STATE_D0)) {
-				tz->active[i].flags.enabled = 0;
+				tz->trips[i].flags.enabled = 0;
 				break;
 			}
 		}
-		tz->state.active |= tz->active[i].flags.enabled;
+		tz->state.active |= tz->trips[i].flags.enabled;
 	}
 
 	acpi_queue_thermal_check(tz);
-- 
2.34.1

[PATCH RFC 5/9] thermal/acpi: Optimize get_trip_points()

[Daniel Lezcano]

The function has two purposes. Initialize the trip points by reading
the ACPI table and then doing a check trip points exists.

This check will go through all the trip points and at the end if a
valid trip point is found, the parsing is considered valid.

Instead of checking all the trip points, exit when a valid trip point
is found.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/acpi/thermal.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index ce37494bd133..9841b597a9c7 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -463,23 +463,20 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 
 static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
 {
-	int i, valid, ret = acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT);
+	int i, ret;
 
+	ret = acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT);
 	if (ret)
 		return ret;
 
-	valid = tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid |
-		tz->trips[ACPI_THERMAL_TRIP_HOT].flags.valid |
-		tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid;
+	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
+		if (tz->trips[i].flags.valid)
+			return 0;
+	}
 
-	for (i = ACPI_THERMAL_TRIP_ACTIVE; i < ACPI_THERMAL_MAX_ACTIVE; i++)
-		valid |= tz->trips[i].flags.valid;
+	pr_warn(FW_BUG "No valid trip found\n");
 
-	if (!valid) {
-		pr_warn(FW_BUG "No valid trip found\n");
-		return -ENODEV;
-	}
-	return 0;
+	return -ENODEV;
 }
 
 /* sys I/F for generic thermal sysfs support */
-- 
2.34.1

[PATCH RFC 6/9] thermal/acpi: Encapsualte in functions the trip initialization

[Daniel Lezcano]

The thermal trip update function is a bit difficult to read. In order
to improve its readability, let's encapuslate the different parts into
dedicated functions.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/acpi/thermal.c | 267 ++++++++++++++++++++++++-----------------
 1 file changed, 159 insertions(+), 108 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 9841b597a9c7..e62381561255 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -246,117 +246,123 @@ do {	\
 		"Please report to linux-acpi@vger.kernel.org\n", str); \
 } while (0)
 
-static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
+static int acpi_thermal_trips_update_critical(struct acpi_thermal *tz, int flag)
 {
 	acpi_status status = AE_OK;
 	unsigned long long tmp;
-	struct acpi_handle_list devices;
-	int valid = 0;
-	int i;
 
-	/* Critical Shutdown */
-	if (flag & ACPI_TRIPS_CRITICAL) {
-		status = acpi_evaluate_integer(tz->device->handle,
+	status = acpi_evaluate_integer(tz->device->handle,
 				"_CRT", NULL, &tmp);
-		tz->trips[ACPI_THERMAL_TRIP_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[ACPI_THERMAL_TRIP_CRITICAL].flags.valid = 0;
-			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[ACPI_THERMAL_TRIP_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[ACPI_THERMAL_TRIP_CRITICAL].flags.valid = 0;
+		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[ACPI_THERMAL_TRIP_CRITICAL].flags.valid = 0;
+	} else {
+		tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid = 1;
+		acpi_handle_debug(tz->device->handle,
+				  "Found critical threshold [%lu]\n",
+				  tz->trips[ACPI_THERMAL_TRIP_CRITICAL].temperature);
+	}
+	if (tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid == 1) {
+		if (crt == -1) {
 			tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid = 0;
-		} else {
-			tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid = 1;
-			acpi_handle_debug(tz->device->handle,
-					  "Found critical threshold [%lu]\n",
-					  tz->trips[ACPI_THERMAL_TRIP_CRITICAL].temperature);
-		}
-		if (tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid == 1) {
-			if (crt == -1) {
-				tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid = 0;
-			} else if (crt > 0) {
-				unsigned long crt_k = celsius_to_deci_kelvin(crt);
-
-				/*
-				 * Allow override critical threshold
-				 */
-				if (crt_k > tz->trips[ACPI_THERMAL_TRIP_CRITICAL].temperature)
-					pr_info("Critical threshold %d C\n", crt);
-
-				tz->trips[ACPI_THERMAL_TRIP_CRITICAL].temperature = crt_k;
-			}
+		} else if (crt > 0) {
+			unsigned long crt_k = celsius_to_deci_kelvin(crt);
+
+			/*
+			 * Allow override critical threshold
+			 */
+			if (crt_k > tz->trips[ACPI_THERMAL_TRIP_CRITICAL].temperature)
+				pr_info("Critical threshold %d C\n", crt);
+
+			tz->trips[ACPI_THERMAL_TRIP_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[ACPI_THERMAL_TRIP_HOT].flags.valid = 0;
-			acpi_handle_debug(tz->device->handle,
-					  "No hot threshold\n");
-		} else {
-			tz->trips[ACPI_THERMAL_TRIP_HOT].temperature = tmp;
-			tz->trips[ACPI_THERMAL_TRIP_HOT].flags.valid = 1;
-			acpi_handle_debug(tz->device->handle,
-					  "Found hot threshold [%lu]\n",
-					  tz->trips[ACPI_THERMAL_TRIP_HOT].temperature);
-		}
+	return 0;
+}
+
+static int acpi_thermal_trips_update_hot(struct acpi_thermal *tz, int flag)
+{
+	acpi_status status = AE_OK;
+	unsigned long long tmp;
+
+	status = acpi_evaluate_integer(tz->device->handle,
+				       "_HOT", NULL, &tmp);
+	if (ACPI_FAILURE(status)) {
+		tz->trips[ACPI_THERMAL_TRIP_HOT].flags.valid = 0;
+		acpi_handle_debug(tz->device->handle,
+				  "No hot threshold\n");
+	} else {
+		tz->trips[ACPI_THERMAL_TRIP_HOT].temperature = tmp;
+		tz->trips[ACPI_THERMAL_TRIP_HOT].flags.valid = 1;
+		acpi_handle_debug(tz->device->handle,
+				  "Found hot threshold [%lu]\n",
+				  tz->trips[ACPI_THERMAL_TRIP_HOT].temperature);
 	}
 
-	/* Passive (optional) */
-	if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid) ||
-		(flag == ACPI_TRIPS_INIT)) {
-		valid = tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.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);
-		}
+	return 0;
+}
 
-		if (ACPI_FAILURE(status))
-			tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 0;
-		else {
-			tz->trips[ACPI_THERMAL_TRIP_PASSIVE].temperature = tmp;
-			tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 1;
-			if (flag == ACPI_TRIPS_INIT) {
-				status = acpi_evaluate_integer(
-						tz->device->handle, "_TC1",
-						NULL, &tmp);
-				if (ACPI_FAILURE(status))
-					tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 0;
-				else
-					tz->trips[ACPI_THERMAL_TRIP_PASSIVE].tc1 = tmp;
-				status = acpi_evaluate_integer(
-						tz->device->handle, "_TC2",
-						NULL, &tmp);
-				if (ACPI_FAILURE(status))
-					tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 0;
-				else
-					tz->trips[ACPI_THERMAL_TRIP_PASSIVE].tc2 = tmp;
-				status = acpi_evaluate_integer(
-						tz->device->handle, "_TSP",
-						NULL, &tmp);
-				if (ACPI_FAILURE(status))
-					tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 0;
-				else
-					tz->trips[ACPI_THERMAL_TRIP_PASSIVE].tsp = tmp;
-			}
+static int acpi_thermal_trips_update_passive(struct acpi_thermal *tz, int flag)
+{
+	acpi_status status = AE_OK;
+	unsigned long long tmp;
+	struct acpi_handle_list devices;
+	int valid = 0;
+
+	valid = tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.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[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 0;
+	else {
+		tz->trips[ACPI_THERMAL_TRIP_PASSIVE].temperature = tmp;
+		tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 1;
+		if (flag == ACPI_TRIPS_INIT) {
+			status = acpi_evaluate_integer(
+				tz->device->handle, "_TC1",
+				NULL, &tmp);
+			if (ACPI_FAILURE(status))
+				tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 0;
+			else
+				tz->trips[ACPI_THERMAL_TRIP_PASSIVE].tc1 = tmp;
+			status = acpi_evaluate_integer(
+				tz->device->handle, "_TC2",
+				NULL, &tmp);
+			if (ACPI_FAILURE(status))
+				tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 0;
+			else
+				tz->trips[ACPI_THERMAL_TRIP_PASSIVE].tc2 = tmp;
+			status = acpi_evaluate_integer(
+				tz->device->handle, "_TSP",
+				NULL, &tmp);
+			if (ACPI_FAILURE(status))
+				tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid = 0;
+			else
+				tz->trips[ACPI_THERMAL_TRIP_PASSIVE].tsp = tmp;
 		}
 	}
+
 	if ((flag & ACPI_TRIPS_DEVICES) && tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid) {
 		memset(&devices, 0, sizeof(struct acpi_handle_list));
 		status = acpi_evaluate_reference(tz->device->handle, "_PSL",
@@ -376,12 +382,23 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 			ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
 		}
 	}
+
 	if ((flag & ACPI_TRIPS_PASSIVE) || (flag & ACPI_TRIPS_DEVICES)) {
 		if (valid != tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid)
-				ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
+			ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
 	}
 
-	/* Active (optional) */
+	return 0;
+}
+
+static int acpi_thermal_trips_update_active(struct acpi_thermal *tz, int flag)
+{
+	acpi_status status = AE_OK;
+	unsigned long long tmp;
+	struct acpi_handle_list devices;
+	int valid = 0;
+	int i;
+
 	for (i = ACPI_THERMAL_TRIP_ACTIVE; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
 		char name[5] = { '_', 'A', 'C', ('0' + i), '\0' };
 		valid = tz->trips[i].flags.valid;
@@ -447,17 +464,51 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 			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");
-		}
+	return 0;
+}
+
+static int acpi_thermal_trips_update_devices(struct acpi_thermal *tz, int flag)
+{
+	acpi_status status = AE_OK;
+	struct acpi_handle_list 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");
+	}
+
+	return 0;
+}
+
+static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
+{
+	/* Critical Shutdown */
+	if (flag & ACPI_TRIPS_CRITICAL) {
+		acpi_thermal_trips_update_critical(tz, flag);
 	}
 
+	/* Critical Sleep (optional) */
+	if (flag & ACPI_TRIPS_HOT) {
+		acpi_thermal_trips_update_hot(tz, flag);
+	}
+
+	/* Passive (optional) */
+	if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid) ||
+		(flag == ACPI_TRIPS_INIT)) {
+		acpi_thermal_trips_update_passive(tz, flag);
+	}
+
+	/* Active (optional) */
+	acpi_thermal_trips_update_active(tz, flag);
+
+	if (flag & ACPI_TRIPS_DEVICES) {
+		acpi_thermal_trips_update_devices(tz, flag);
+	}
+	
 	return 0;
 }
 
-- 
2.34.1

[PATCH RFC 7/9] thermal/acpi: Simplifify the condition check

[Daniel Lezcano]

The condition:

if ((flag == ACPI_TRIPS_INIT) || ((flag & ACPI_TRIPS_ACTIVE))

and on the other side: ACPI_TRIPS_INIT (... | ACPI_TRIPS_ACTIVE)

So if the first predicate is true, the second is also true.

The 'valid' flag for the trip point is also checked before, so it is
pointless to redo the same check again and again.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/acpi/thermal.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index e62381561255..116e5cf19c5d 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -323,6 +323,9 @@ static int acpi_thermal_trips_update_passive(struct acpi_thermal *tz, int flag)
 	int valid = 0;
 
 	valid = tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid;
+	if (!valid)
+		return 0;
+
 	if (psv == -1) {
 		status = AE_SUPPORT;
 	} else if (psv > 0) {
@@ -401,13 +404,16 @@ static int acpi_thermal_trips_update_active(struct acpi_thermal *tz, int flag)
 
 	for (i = ACPI_THERMAL_TRIP_ACTIVE; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
 		char name[5] = { '_', 'A', 'C', ('0' + i), '\0' };
+
 		valid = tz->trips[i].flags.valid;
 
 		if (act == -1)
 			break; /* disable all active trip points */
 
-		if ((flag == ACPI_TRIPS_INIT) || ((flag & ACPI_TRIPS_ACTIVE) &&
-			tz->trips[i].flags.valid)) {
+		if (!tz->trips[i].flags.valid)
+			continue;
+		
+		if (flag & ACPI_TRIPS_ACTIVE)  {
 			status = acpi_evaluate_integer(tz->device->handle,
 							name, NULL, &tmp);
 			if (ACPI_FAILURE(status)) {
@@ -437,7 +443,7 @@ static int acpi_thermal_trips_update_active(struct acpi_thermal *tz, int flag)
 		}
 
 		name[2] = 'L';
-		if ((flag & ACPI_TRIPS_DEVICES) && tz->trips[i].flags.valid ) {
+		if (flag & ACPI_TRIPS_DEVICES) {
 			memset(&devices, 0, sizeof(struct acpi_handle_list));
 			status = acpi_evaluate_reference(tz->device->handle,
 						name, NULL, &devices);
@@ -456,6 +462,7 @@ static int acpi_thermal_trips_update_active(struct acpi_thermal *tz, int flag)
 				ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
 			}
 		}
+
 		if ((flag & ACPI_TRIPS_ACTIVE) || (flag & ACPI_TRIPS_DEVICES))
 			if (valid != tz->trips[i].flags.valid)
 				ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
@@ -497,8 +504,7 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 	}
 
 	/* Passive (optional) */
-	if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid) ||
-		(flag == ACPI_TRIPS_INIT)) {
+	if (flag & ACPI_TRIPS_PASSIVE) {
 		acpi_thermal_trips_update_passive(tz, flag);
 	}
 
-- 
2.34.1

[PATCH RFC 8/9] thermal/acpi: Remove active and enabled flags

[Daniel Lezcano]

The 'active' field in the struct acpi_thermal_state is never used.

The 'enabled' field of the structure acpi_thermal_state_flags is
assigned but never used.

Remove them.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/acpi/thermal.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 116e5cf19c5d..f530dbfa80db 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -117,14 +117,12 @@ struct acpi_thermal_state {
 	u8 critical:1;
 	u8 hot:1;
 	u8 passive:1;
-	u8 active:1;
 	u8 reserved:4;
 	int active_index;
 };
 
 struct acpi_thermal_state_flags {
 	u8 valid:1;
-	u8 enabled:1;
 	u8 reserved:6;
 };
 
@@ -1139,17 +1137,14 @@ static int acpi_thermal_resume(struct device *dev)
 	for (i = ACPI_THERMAL_TRIP_ACTIVE; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
 		if (!tz->trips[i].flags.valid)
 			break;
-		tz->trips[i].flags.enabled = 1;
+
 		for (j = 0; j < tz->trips[i].devices.count; j++) {
 			result = acpi_bus_update_power(
 					tz->trips[i].devices.handles[j],
 					&power_state);
-			if (result || (power_state != ACPI_STATE_D0)) {
-				tz->trips[i].flags.enabled = 0;
+			if (result || (power_state != ACPI_STATE_D0))
 				break;
-			}
 		}
-		tz->state.active |= tz->trips[i].flags.enabled;
 	}
 
 	acpi_queue_thermal_check(tz);
-- 
2.34.1

[PATCH RFC 9/9] thermal/acpi: Rewrite the trip point intialization to use the generic thermal trip

[Daniel Lezcano]

We can use the thermal trip points defined in the thermal.h.

Let's initialize them properly and when the code will be moved to the
generic thermal structure, we will be able to remove the specific acpi
trip points. Still WIP.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/acpi/thermal.c | 211 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 186 insertions(+), 25 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index f530dbfa80db..994b96807be3 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -244,6 +244,171 @@ do {	\
 		"Please report to linux-acpi@vger.kernel.org\n", str); \
 } while (0)
 
+static void acpi_thermal_trips_override_critical(struct thermal_trip *trip,
+						 int temperature)
+{
+        int ktemp = = celsius_to_deci_kelvin(temperature);
+
+	if (ktemp > trip->temperature)
+		pr_info("Overriding %d C\n", temperature);
+
+	trip->temperature = ktemp;
+}
+
+static struct thermal_trip *acpi_thermal_trips_alloc_critical(struct acpi_thermal *tz,
+							      struct thermal_trip *trips,
+							      int *num_trips)
+{
+	acpi_status status = AE_OK;
+	unsigned long long temp;
+
+	/*
+	 * Module parameters disable the critical trip point
+	 */
+	if (crt < 0)
+		goto out;
+
+	status = acpi_evaluate_integer(tz->device->handle, "_CRT", NULL, &temp);
+	if (ACPI_FAILURE(status)) {
+		acpi_handle_debug(tz->device->handle, "No critical threshold\n");
+		goto out;
+	}
+
+	if (temp <= 2732) {
+		pr_info(FW_BUG "Invalid critical threshold (%llu)\n", temp);
+		goto out;
+	}
+
+	trips = krealloc(trips, sizeof(*trips) * (*num_trips + 1), GFP_KERNEL);
+	if (!trips)
+		goto out;
+
+	memset(&trips[*num_trips], 0, sizeof(*trips));
+
+	trips[*num_trips].temperature = deci_kelvin_to_millicelsius(temp);
+	trips[*num_trips].type = THERMAL_TRIP_CRITICAL;
+
+	if (crt > 0)
+		acpi_thermal_trips_override_critical(&trips[*num_trips], crt);
+	
+	(*num_trips)++;
+out:
+	return trips;
+}
+
+static struct thermal_trip *acpi_thermal_trips_alloc_hot(struct acpi_thermal *tz,
+							 struct thermal_trip *trips,
+							 int *num_trips)
+{
+	acpi_status status = AE_OK;
+	unsigned long long temp;
+
+	status = acpi_evaluate_integer(tz->device->handle, "_HOT", NULL, &temp);
+	if (ACPI_FAILURE(status)) {
+		acpi_handle_debug(tz->device->handle, "No hot threshold\n");
+		goto out;
+	}
+
+	trips = krealloc(trips, sizeof(*trips) * (*num_trips + 1), GFP_KERNEL);
+	if (!trips)
+		goto out;
+
+	memset(&trips[*num_trips], 0, sizeof(*trips));
+
+	trips[*num_trips].temperature = deci_kelvin_to_millicelsius(temp);
+	trips[*num_trips].type = THERMAL_TRIP_HOT;
+	
+	(*num_trips)++;
+out:
+	return trips;
+}
+
+static struct thermal_trip *acpi_thermal_trips_alloc_passive(struct acpi_thermal *tz,
+							     struct thermal_trip *trips,
+							     int *num_trips)
+{
+	acpi_status status;
+	unsigned long long temp;
+
+	/*
+	 * Module parameters disable all passive trip points
+	 */
+	if (psv < 0)
+		goto out;
+	
+	status = acpi_evaluate_integer(tz->device->handle, "_PSV", NULL, &temp);
+	if (ACPI_FAILURE(status)) {
+		acpi_handle_debug(tz->device->handle, "No passive threshold\n");
+		goto out;
+	}
+
+	trips = krealloc(trips, sizeof(*trips) * (*num_trips + 1), GFP_KERNEL);
+	if (!trips)
+		goto out;
+
+	memset(&trips[*num_trips], 0, sizeof(*trips));
+
+	trips[*num_trips].temperature = deci_kelvin_to_millicelsius(temp);
+	trips[*num_trips].type = THERMAL_TRIP_PASSIVE;
+	
+	(*num_trips)++;
+out:
+	return trips;
+}
+
+static struct thermal_trip *acpi_thermal_trips_alloc_active(struct acpi_thermal *tz,
+							    struct thermal_trip *trips,
+							    int *num_trips)
+{
+	acpi_status status;
+	unsigned long long temp;
+	int i;
+
+	/*
+	 * Module parameters disable all active trip points
+	 */
+	if (act < 0)
+		return trips;
+
+	for (i = ACPI_THERMAL_TRIP_ACTIVE; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
+		char name[5] = { '_', 'A', 'C', ('0' + i), '\0' };
+
+		status = acpi_evaluate_integer(tz->device->handle, name, NULL, &temp);
+		if (ACPI_FAILURE(status))
+			break;
+
+		trips = krealloc(trips, sizeof(*trips) * (*num_trips + 1), GFP_KERNEL);
+		if (!trips)
+			break;
+
+		memset(&trips[*num_trips], 0, sizeof(*trips));
+
+		trips[*num_trips].temperature = deci_kelvin_to_millicelsius(temp);
+		trips[*num_trips].type = THERMAL_TRIP_ACTIVE;
+	
+		(*num_trips)++;
+	}
+
+	return trips;
+}
+
+static struct thermal_trip *acpi_thermal_trips_alloc(struct acpi_thermal *tz, int *num_trips)
+{
+	struct thermal_trip *trips = NULL;
+
+	*num_trips = 0;
+
+	trips = acpi_thermal_trips_alloc_critical(tz, trips, num_trips);
+
+	trips = acpi_thermal_trips_alloc_hot(tz, trips, num_trips);
+
+	trips = acpi_thermal_trips_alloc_passive(tz, trips, num_trips);
+
+	trips = acpi_thermal_trips_alloc_active(tz, trips, num_trips);
+
+	return trips;
+}
+
 static int acpi_thermal_trips_update_critical(struct acpi_thermal *tz, int flag)
 {
 	acpi_status status = AE_OK;
@@ -824,36 +989,24 @@ static struct thermal_zone_device_ops acpi_thermal_zone_ops = {
 	.critical = acpi_thermal_zone_device_critical,
 };
 
-static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
+static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz,
+					      struct thermal_trip *trips,
+					      int num_trips)
 {
-	int trips = 0;
 	int result;
 	acpi_status status;
-	int i;
-
-	if (tz->trips[ACPI_THERMAL_TRIP_CRITICAL].flags.valid)
-		trips++;
-
-	if (tz->trips[ACPI_THERMAL_TRIP_HOT].flags.valid)
-		trips++;
-
-	if (tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid)
-		trips++;
-
-	for (i = ACPI_THERMAL_TRIP_ACTIVE; i < ACPI_THERMAL_MAX_ACTIVE &&
-			tz->trips[i].flags.valid; i++, trips++);
 
 	if (tz->trips[ACPI_THERMAL_TRIP_PASSIVE].flags.valid)
 		tz->thermal_zone =
-			thermal_zone_device_register("acpitz", trips, 0, tz,
-						&acpi_thermal_zone_ops, NULL,
-						     tz->trips[ACPI_THERMAL_TRIP_PASSIVE].tsp*100,
-						     tz->polling_frequency*100);
+			thermal_zone_device_register_with_trips("acpitz", trips, num_trips, 0, tz,
+								&acpi_thermal_zone_ops, NULL,
+								tz->trips[ACPI_THERMAL_TRIP_PASSIVE].tsp*100,
+								tz->polling_frequency*100);
 	else
 		tz->thermal_zone =
-			thermal_zone_device_register("acpitz", trips, 0, tz,
-						&acpi_thermal_zone_ops, NULL,
-						0, tz->polling_frequency*100);
+			thermal_zone_device_register_with_trips("acpitz", trips, num_trips, 0, tz,
+								&acpi_thermal_zone_ops, NULL,
+								0, tz->polling_frequency*100);
 	if (IS_ERR(tz->thermal_zone))
 		return -ENODEV;
 
@@ -1060,7 +1213,8 @@ static int acpi_thermal_add(struct acpi_device *device)
 {
 	int result = 0;
 	struct acpi_thermal *tz = NULL;
-
+	struct thermal_trip *trips;
+	int num_trips;
 
 	if (!device)
 		return -EINVAL;
@@ -1081,9 +1235,13 @@ static int acpi_thermal_add(struct acpi_device *device)
 
 	acpi_thermal_guess_offset(tz);
 
-	result = acpi_thermal_register_thermal_zone(tz);
+	trips = acpi_thermal_trips_alloc(tz, &num_trips);
+	if (!trips)
+		goto free_trips;
+
+	result = acpi_thermal_register_thermal_zone(tz, trips, num_trips);
 	if (result)
-		goto free_memory;
+		goto free_trips;
 
 	refcount_set(&tz->thermal_check_count, 3);
 	mutex_init(&tz->thermal_check_lock);
@@ -1095,6 +1253,8 @@ static int acpi_thermal_add(struct acpi_device *device)
 
 free_memory:
 	kfree(tz);
+free_trips:
+	kfree(trips);
 end:
 	return result;
 }
@@ -1109,6 +1269,7 @@ static int acpi_thermal_remove(struct acpi_device *device)
 	flush_workqueue(acpi_thermal_pm_queue);
 	tz = acpi_driver_data(device);
 
+	kfree(tz->trips);
 	acpi_thermal_unregister_thermal_zone(tz);
 	kfree(tz);
 	return 0;
-- 
2.34.1

Re: [PATCH v1 1/3] ACPI: thermal: Use white space more consistently

[Daniel Lezcano]

On 04/10/2022 18:31, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The usage of white space in the ACPI thermal driver is not very
> consistent, so improve that a bit.
> 
> While at it, add missing braces to if()/else in a few places.
> 
> No 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 2/3] ACPI: thermal: Drop redundant parens from expressions

[Daniel Lezcano]

On 04/10/2022 18:32, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Some expressions in the ACPI thermal driver contain redundant
> parentheses.  Drop them.
> 
> No 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 3/3] ACPI: thermal: Drop some redundant code

[Daniel Lezcano]

On 04/10/2022 18:32, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Drop some redundant initialization of local variables, a redundant
> return statement and a redundant "else" from the ACPI thermal driver.
> 
> No 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