Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Daniel Lezcano <daniel.lezcano@linaro.org>
To:     rafael@kernel.org
Cc:     linux-acpi@vger.kernel.org, linux-pm@vger.kernel.org,
        linux-kernel@vger.kernel.org, Zhang Rui <rui.zhang@intel.com>,
        Len Brown <lenb@kernel.org>
Subject: [PATCH RFC 6/9] thermal/acpi: Encapsualte in functions the trip initialization
Date:   Tue,  4 Oct 2022 19:26:55 +0200
Message-Id: <20221004172658.2302511-7-daniel.lezcano@linaro.org>
In-Reply-To: <20221004172658.2302511-1-daniel.lezcano@linaro.org>
References: <12068304.O9o76ZdvQC@kreacher>
 <20221004172658.2302511-1-daniel.lezcano@linaro.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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