[PATCH] thermal: core: warn on non-critical trips at or above critical temp

Gaurav Kohli posted 1 patch 2 weeks, 1 day ago
drivers/thermal/thermal_core.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
[PATCH] thermal: core: warn on non-critical trips at or above critical temp
Posted by Gaurav Kohli 2 weeks, 1 day ago
Thermal zones can be registered or updated with non-critical trips at or
above the critical trip temperature. Such trips cannot provide useful
mitigation before critical trip handling runs, and a HOT trip at that
temperature can coincide with shutdown.

Warn at registration time and on trip temperature changes to surface
misconfigured platform data and incorrect dynamic updates early. When
multiple critical trips exist, warn against the lowest one.

Signed-off-by: Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>
---
 drivers/thermal/thermal_core.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 28a20d4b475c..0e37e3787565 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -434,6 +434,38 @@ static void thermal_trip_crossed(struct thermal_zone_device *tz,
 	thermal_governor_trip_crossed(governor, tz, trip, upward);
 }
 
+static void thermal_zone_check_trip_ordering(struct thermal_zone_device *tz)
+{
+	const struct thermal_trip_desc *td;
+	int crit_temp = INT_MAX;
+
+	for_each_trip_desc(tz, td) {
+		const struct thermal_trip *trip = &td->trip;
+
+		if (trip->type == THERMAL_TRIP_CRITICAL &&
+		    trip->temperature != THERMAL_TEMP_INVALID &&
+		    trip->temperature < crit_temp)
+			crit_temp = trip->temperature;
+	}
+
+	if (crit_temp == INT_MAX)
+		return;
+
+	for_each_trip_desc(tz, td) {
+		const struct thermal_trip *trip = &td->trip;
+
+		if (trip->type == THERMAL_TRIP_CRITICAL ||
+		    trip->temperature == THERMAL_TEMP_INVALID)
+			continue;
+
+		if (trip->temperature >= crit_temp)
+			dev_warn(&tz->device,
+				 "%s trip temperature %d is not below critical trip temperature %d\n",
+				 thermal_trip_type_name(trip->type),
+				 trip->temperature, crit_temp);
+	}
+}
+
 void thermal_zone_set_trip_hyst(struct thermal_zone_device *tz,
 				struct thermal_trip *trip, int hyst)
 {
@@ -462,6 +494,7 @@ void thermal_zone_set_trip_temp(struct thermal_zone_device *tz,
 
 	WRITE_ONCE(trip->temperature, temp);
 	thermal_notify_tz_trip_change(tz, trip);
+	thermal_zone_check_trip_ordering(tz);
 
 	if (old_temp == THERMAL_TEMP_INVALID) {
 		/*
@@ -1509,6 +1542,8 @@ thermal_zone_device_register_with_trips(const char *type,
 	if (result)
 		goto remove_id;
 
+	thermal_zone_check_trip_ordering(tz);
+
 	thermal_zone_device_init(tz);
 
 	result = thermal_zone_init_governor(tz);

---
base-commit: 4f441960e691d37c880d2cc004de06bb5b6bd5e4
change-id: 20260710-critical_trip-7b015d270e36

Best regards,
-- 
Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>
Re: [PATCH] thermal: core: warn on non-critical trips at or above critical temp
Posted by Rafael J. Wysocki (Intel) 3 days, 5 hours ago
On Fri, Jul 10, 2026 at 11:12 AM Gaurav Kohli
<gaurav.kohli@oss.qualcomm.com> wrote:
>
> Thermal zones can be registered or updated with non-critical trips at or
> above the critical trip temperature. Such trips cannot provide useful
> mitigation before critical trip handling runs, and a HOT trip at that
> temperature can coincide with shutdown.
>
> Warn at registration time and on trip temperature changes to surface
> misconfigured platform data and incorrect dynamic updates early. When
> multiple critical trips exist, warn against the lowest one.
>
> Signed-off-by: Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>

There are a couple of reasons why I don't like this, the most
important one being the overhead: two walks over all trips added to
every trip temperature update is a bit of a stretch.

Moreover, the messages produced by this code may not even be noticed
on systems in the field and users may not be able to do anything about
them, so the benefit is questionable.

> ---
>  drivers/thermal/thermal_core.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 28a20d4b475c..0e37e3787565 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -434,6 +434,38 @@ static void thermal_trip_crossed(struct thermal_zone_device *tz,
>         thermal_governor_trip_crossed(governor, tz, trip, upward);
>  }
>
> +static void thermal_zone_check_trip_ordering(struct thermal_zone_device *tz)
> +{
> +       const struct thermal_trip_desc *td;
> +       int crit_temp = INT_MAX;
> +
> +       for_each_trip_desc(tz, td) {
> +               const struct thermal_trip *trip = &td->trip;
> +
> +               if (trip->type == THERMAL_TRIP_CRITICAL &&
> +                   trip->temperature != THERMAL_TEMP_INVALID &&
> +                   trip->temperature < crit_temp)
> +                       crit_temp = trip->temperature;
> +       }
> +
> +       if (crit_temp == INT_MAX)
> +               return;
> +
> +       for_each_trip_desc(tz, td) {
> +               const struct thermal_trip *trip = &td->trip;
> +
> +               if (trip->type == THERMAL_TRIP_CRITICAL ||
> +                   trip->temperature == THERMAL_TEMP_INVALID)
> +                       continue;
> +
> +               if (trip->temperature >= crit_temp)
> +                       dev_warn(&tz->device,
> +                                "%s trip temperature %d is not below critical trip temperature %d\n",
> +                                thermal_trip_type_name(trip->type),
> +                                trip->temperature, crit_temp);
> +       }
> +}
> +
>  void thermal_zone_set_trip_hyst(struct thermal_zone_device *tz,
>                                 struct thermal_trip *trip, int hyst)
>  {
> @@ -462,6 +494,7 @@ void thermal_zone_set_trip_temp(struct thermal_zone_device *tz,
>
>         WRITE_ONCE(trip->temperature, temp);
>         thermal_notify_tz_trip_change(tz, trip);
> +       thermal_zone_check_trip_ordering(tz);
>
>         if (old_temp == THERMAL_TEMP_INVALID) {
>                 /*
> @@ -1509,6 +1542,8 @@ thermal_zone_device_register_with_trips(const char *type,
>         if (result)
>                 goto remove_id;
>
> +       thermal_zone_check_trip_ordering(tz);
> +
>         thermal_zone_device_init(tz);
>
>         result = thermal_zone_init_governor(tz);
>
> ---
> base-commit: 4f441960e691d37c880d2cc004de06bb5b6bd5e4
> change-id: 20260710-critical_trip-7b015d270e36
>
> Best regards,
> --
> Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>
>