From 8b95bed0cec98c9ff7a72acd6227c427a2b4f0a9 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 28 May 2024 16:52:50 +0200 Subject: [PATCH 01/13] thermal/debugfs: Use helper to update trip point overstepping duration Add a helper for updating trip point overstepping duration to be called from thermal_debug_tz_trip_down(). Subsequently, it will also be used during resume from system-wide suspend. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano --- drivers/thermal/thermal_debugfs.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/thermal/thermal_debugfs.c b/drivers/thermal/thermal_debugfs.c index 942447229157..66c0105465dd 100644 --- a/drivers/thermal/thermal_debugfs.c +++ b/drivers/thermal/thermal_debugfs.c @@ -653,14 +653,24 @@ void thermal_debug_tz_trip_up(struct thermal_zone_device *tz, mutex_unlock(&thermal_dbg->lock); } +static void tz_episode_close_trip(struct tz_episode *tze, int trip_id, ktime_t now) +{ + struct trip_stats *trip_stats = &tze->trip_stats[trip_id]; + ktime_t delta = ktime_sub(now, trip_stats->timestamp); + + trip_stats->duration = ktime_add(delta, trip_stats->duration); + /* Mark the end of mitigation for this trip point. */ + trip_stats->timestamp = KTIME_MAX; +} + void thermal_debug_tz_trip_down(struct thermal_zone_device *tz, const struct thermal_trip *trip) { struct thermal_debugfs *thermal_dbg = tz->debugfs; + int trip_id = thermal_zone_trip_id(tz, trip); + ktime_t now = ktime_get(); struct tz_episode *tze; struct tz_debugfs *tz_dbg; - ktime_t delta, now = ktime_get(); - int trip_id = thermal_zone_trip_id(tz, trip); int i; if (!thermal_dbg) @@ -695,13 +705,7 @@ void thermal_debug_tz_trip_down(struct thermal_zone_device *tz, tze = list_first_entry(&tz_dbg->tz_episodes, struct tz_episode, node); - delta = ktime_sub(now, tze->trip_stats[trip_id].timestamp); - - tze->trip_stats[trip_id].duration = - ktime_add(delta, tze->trip_stats[trip_id].duration); - - /* Mark the end of mitigation for this trip point. */ - tze->trip_stats[trip_id].timestamp = KTIME_MAX; + tz_episode_close_trip(tze, trip_id, now); /* * This event closes the mitigation as we are crossing the From 9b73b5052ae841037682da93a17f80f8ed57788d Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 28 May 2024 16:53:47 +0200 Subject: [PATCH 02/13] thermal/debugfs: Do not extend mitigation episodes beyond system resume Because thermal zone handling by the thermal core is started from scratch during resume from system-wide suspend, prevent the debug code from extending mitigation episodes beyond that point by ending the mitigation episode currently in progress, if any, for each thermal zone. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano --- drivers/thermal/thermal_core.c | 1 + drivers/thermal/thermal_debugfs.c | 36 +++++++++++++++++++++++++++++++ drivers/thermal/thermal_debugfs.h | 2 ++ 3 files changed, 39 insertions(+) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index d70e76dd3c94..bd31193bb583 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -1648,6 +1648,7 @@ static void thermal_zone_device_resume(struct work_struct *work) tz->suspended = false; + thermal_debug_tz_resume(tz); thermal_zone_device_init(tz); __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); diff --git a/drivers/thermal/thermal_debugfs.c b/drivers/thermal/thermal_debugfs.c index 66c0105465dd..c592eca11a50 100644 --- a/drivers/thermal/thermal_debugfs.c +++ b/drivers/thermal/thermal_debugfs.c @@ -926,3 +926,39 @@ void thermal_debug_tz_remove(struct thermal_zone_device *tz) thermal_debugfs_remove_id(thermal_dbg); kfree(trips_crossed); } + +void thermal_debug_tz_resume(struct thermal_zone_device *tz) +{ + struct thermal_debugfs *thermal_dbg = tz->debugfs; + ktime_t now = ktime_get(); + struct tz_debugfs *tz_dbg; + struct tz_episode *tze; + int i; + + if (!thermal_dbg) + return; + + mutex_lock(&thermal_dbg->lock); + + tz_dbg = &thermal_dbg->tz_dbg; + + if (!tz_dbg->nr_trips) + goto out; + + /* + * A mitigation episode was in progress before the preceding system + * suspend transition, so close it because the zone handling is starting + * over from scratch. + */ + tze = list_first_entry(&tz_dbg->tz_episodes, struct tz_episode, node); + + for (i = 0; i < tz_dbg->nr_trips; i++) + tz_episode_close_trip(tze, tz_dbg->trips_crossed[i], now); + + tze->duration = ktime_sub(now, tze->timestamp); + + tz_dbg->nr_trips = 0; + +out: + mutex_unlock(&thermal_dbg->lock); +} diff --git a/drivers/thermal/thermal_debugfs.h b/drivers/thermal/thermal_debugfs.h index 74ee65ee82ff..1c957ce2ec8f 100644 --- a/drivers/thermal/thermal_debugfs.h +++ b/drivers/thermal/thermal_debugfs.h @@ -7,6 +7,7 @@ void thermal_debug_cdev_remove(struct thermal_cooling_device *cdev); void thermal_debug_cdev_state_update(const struct thermal_cooling_device *cdev, int state); void thermal_debug_tz_add(struct thermal_zone_device *tz); void thermal_debug_tz_remove(struct thermal_zone_device *tz); +void thermal_debug_tz_resume(struct thermal_zone_device *tz); void thermal_debug_tz_trip_up(struct thermal_zone_device *tz, const struct thermal_trip *trip); void thermal_debug_tz_trip_down(struct thermal_zone_device *tz, @@ -20,6 +21,7 @@ static inline void thermal_debug_cdev_state_update(const struct thermal_cooling_ int state) {} static inline void thermal_debug_tz_add(struct thermal_zone_device *tz) {} static inline void thermal_debug_tz_remove(struct thermal_zone_device *tz) {} +static inline void thermal_debug_tz_resume(struct thermal_zone_device *tz) {} static inline void thermal_debug_tz_trip_up(struct thermal_zone_device *tz, const struct thermal_trip *trip) {}; static inline void thermal_debug_tz_trip_down(struct thermal_zone_device *tz, From cc86c139ae41a6107955db0b4c79d549cbccb4a6 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 28 May 2024 16:55:00 +0200 Subject: [PATCH 03/13] thermal/debugfs: Print mitigation timestamp value in milliseconds Because mitigation episode duration is printed in milliseconds, there is no reason to print timestamp information for mitigation episodes in smaller units which also makes it somewhat harder to interpret the numbers. Print it in milliseconds for consistency. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano --- drivers/thermal/thermal_debugfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/thermal/thermal_debugfs.c b/drivers/thermal/thermal_debugfs.c index c592eca11a50..d74d4cf3ac7b 100644 --- a/drivers/thermal/thermal_debugfs.c +++ b/drivers/thermal/thermal_debugfs.c @@ -797,8 +797,8 @@ static int tze_seq_show(struct seq_file *s, void *v) c = '='; } - seq_printf(s, ",-Mitigation at %lluus, duration%c%llums\n", - ktime_to_us(tze->timestamp), c, duration_ms); + seq_printf(s, ",-Mitigation at %llums, duration%c%llums\n", + ktime_to_ms(tze->timestamp), c, duration_ms); seq_printf(s, "| trip | type | temp(°mC) | hyst(°mC) | duration | avg(°mC) | min(°mC) | max(°mC) |\n"); From ea6a3c52021e552d904d0b1b67ef695bc5c3a54a Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 28 May 2024 16:55:53 +0200 Subject: [PATCH 04/13] thermal/debugfs: Fix up units in "mitigations" files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Print temperature units as m°C rather than °mC (the meaning of which is unclear) and add time unit to the duration column. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano --- drivers/thermal/thermal_debugfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/thermal/thermal_debugfs.c b/drivers/thermal/thermal_debugfs.c index d74d4cf3ac7b..ee9fc627ec81 100644 --- a/drivers/thermal/thermal_debugfs.c +++ b/drivers/thermal/thermal_debugfs.c @@ -800,7 +800,7 @@ static int tze_seq_show(struct seq_file *s, void *v) seq_printf(s, ",-Mitigation at %llums, duration%c%llums\n", ktime_to_ms(tze->timestamp), c, duration_ms); - seq_printf(s, "| trip | type | temp(°mC) | hyst(°mC) | duration | avg(°mC) | min(°mC) | max(°mC) |\n"); + seq_printf(s, "| trip | type | temp(m°C) | hyst(m°C) | duration(ms) | avg(m°C) | min(m°C) | max(m°C) |\n"); for_each_trip_desc(tz, td) { const struct thermal_trip *trip = &td->trip; @@ -846,7 +846,7 @@ static int tze_seq_show(struct seq_file *s, void *v) 8, type, 9, trip_stats->trip_temp, 9, trip_stats->trip_hyst, - c, 10, duration_ms, + c, 11, duration_ms, 9, trip_stats->avg, 9, trip_stats->min, 9, trip_stats->max); From 993c87047ddebb9cf922c489199d3a402a440979 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 28 May 2024 16:57:13 +0200 Subject: [PATCH 05/13] thermal/debugfs: Adjust check for trips without statistics in tze_seq_show() Initialize the trip_temp field in struct trip_stats to THERMAL_TEMP_INVALID and adjust the check for trips without statistics in tze_seq_show() to look at that field instead of comparing min and max. This will mostly be useful to simplify subsequent changes. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano --- drivers/thermal/thermal_debugfs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/thermal/thermal_debugfs.c b/drivers/thermal/thermal_debugfs.c index ee9fc627ec81..5f9e5e9d403c 100644 --- a/drivers/thermal/thermal_debugfs.c +++ b/drivers/thermal/thermal_debugfs.c @@ -563,6 +563,7 @@ static struct tz_episode *thermal_debugfs_tz_event_alloc(struct thermal_zone_dev tze->duration = KTIME_MIN; for (i = 0; i < tz->num_trips; i++) { + tze->trip_stats[i].trip_temp = THERMAL_TEMP_INVALID; tze->trip_stats[i].min = INT_MAX; tze->trip_stats[i].max = INT_MIN; } @@ -818,7 +819,7 @@ static int tze_seq_show(struct seq_file *s, void *v) trip_stats = &tze->trip_stats[trip_id]; /* Skip trips without any stats. */ - if (trip_stats->min > trip_stats->max) + if (trip_stats->trip_temp == THERMAL_TEMP_INVALID) continue; if (trip->type == THERMAL_TRIP_PASSIVE) From 881c084fc980d05d05dceac4d789a473b42e177d Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 28 May 2024 16:58:56 +0200 Subject: [PATCH 06/13] thermal/debugfs: Compute maximum temperature for mitigation episode as a whole Notice that the maximum temperature above the trip point must be the same for all of the trip points involved in a given mitigation episode, so it need not be computerd for each of them separately. It is sufficient to compute the maximum temperature for the mitigation episode as a whole and print it accordingly, so do that. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano --- drivers/thermal/thermal_debugfs.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/thermal/thermal_debugfs.c b/drivers/thermal/thermal_debugfs.c index 5f9e5e9d403c..dbfe678bd404 100644 --- a/drivers/thermal/thermal_debugfs.c +++ b/drivers/thermal/thermal_debugfs.c @@ -94,7 +94,6 @@ struct cdev_record { * @trip_temp: trip temperature at mitigation start * @trip_hyst: trip hysteresis at mitigation start * @count: the number of times the zone temperature was above the trip point - * @max: maximum recorded temperature above the trip point * @min: minimum recorded temperature above the trip point * @avg: average temperature above the trip point */ @@ -104,7 +103,6 @@ struct trip_stats { int trip_temp; int trip_hyst; int count; - int max; int min; int avg; }; @@ -122,12 +120,14 @@ struct trip_stats { * @timestamp: first trip point crossed the way up * @duration: total duration of the mitigation episode * @node: a list element to be added to the list of tz events + * @max_temp: maximum zone temperature during this episode * @trip_stats: per trip point statistics, flexible array */ struct tz_episode { ktime_t timestamp; ktime_t duration; struct list_head node; + int max_temp; struct trip_stats trip_stats[]; }; @@ -561,11 +561,11 @@ static struct tz_episode *thermal_debugfs_tz_event_alloc(struct thermal_zone_dev INIT_LIST_HEAD(&tze->node); tze->timestamp = now; tze->duration = KTIME_MIN; + tze->max_temp = INT_MIN; for (i = 0; i < tz->num_trips; i++) { tze->trip_stats[i].trip_temp = THERMAL_TEMP_INVALID; tze->trip_stats[i].min = INT_MAX; - tze->trip_stats[i].max = INT_MIN; } return tze; @@ -738,11 +738,13 @@ void thermal_debug_update_trip_stats(struct thermal_zone_device *tz) tze = list_first_entry(&tz_dbg->tz_episodes, struct tz_episode, node); + if (tz->temperature > tze->max_temp) + tze->max_temp = tz->temperature; + for (i = 0; i < tz_dbg->nr_trips; i++) { int trip_id = tz_dbg->trips_crossed[i]; struct trip_stats *trip_stats = &tze->trip_stats[trip_id]; - trip_stats->max = max(trip_stats->max, tz->temperature); trip_stats->min = min(trip_stats->min, tz->temperature); trip_stats->avg += (tz->temperature - trip_stats->avg) / ++trip_stats->count; @@ -798,10 +800,10 @@ static int tze_seq_show(struct seq_file *s, void *v) c = '='; } - seq_printf(s, ",-Mitigation at %llums, duration%c%llums\n", - ktime_to_ms(tze->timestamp), c, duration_ms); + seq_printf(s, ",-Mitigation at %llums, duration%c%llums, max. temp=%dm°C\n", + ktime_to_ms(tze->timestamp), c, duration_ms, tze->max_temp); - seq_printf(s, "| trip | type | temp(m°C) | hyst(m°C) | duration(ms) | avg(m°C) | min(m°C) | max(m°C) |\n"); + seq_printf(s, "| trip | type | temp(m°C) | hyst(m°C) | duration(ms) | avg(m°C) | min(m°C) |\n"); for_each_trip_desc(tz, td) { const struct thermal_trip *trip = &td->trip; @@ -842,15 +844,14 @@ static int tze_seq_show(struct seq_file *s, void *v) c = ' '; } - seq_printf(s, "| %*d | %*s | %*d | %*d | %c%*lld | %*d | %*d | %*d |\n", + seq_printf(s, "| %*d | %*s | %*d | %*d | %c%*lld | %*d | %*d |\n", 4 , trip_id, 8, type, 9, trip_stats->trip_temp, 9, trip_stats->trip_hyst, c, 11, duration_ms, 9, trip_stats->avg, - 9, trip_stats->min, - 9, trip_stats->max); + 9, trip_stats->min); } return 0; From 8f9025baf66ff4781ae3d40f9bc457b0a022b1a3 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 28 May 2024 16:59:55 +0200 Subject: [PATCH 07/13] thermal/debugfs: Move some statements from under thermal_dbg->lock The tz_dbg local variable assignments in thermal_debug_tz_trip_up(), thermal_debug_tz_trip_down(), and thermal_debug_update_trip_stats() need not be carried out under thermal_dbg->lock, so move them from under that lock (to avoid possible future confusion). While at it, reorder local variable definitions in thermal_debug_tz_trip_up() for more clarity. No functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano --- drivers/thermal/thermal_debugfs.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/thermal/thermal_debugfs.c b/drivers/thermal/thermal_debugfs.c index dbfe678bd404..d83263101ec3 100644 --- a/drivers/thermal/thermal_debugfs.c +++ b/drivers/thermal/thermal_debugfs.c @@ -574,20 +574,20 @@ static struct tz_episode *thermal_debugfs_tz_event_alloc(struct thermal_zone_dev void thermal_debug_tz_trip_up(struct thermal_zone_device *tz, const struct thermal_trip *trip) { - struct tz_episode *tze; - struct tz_debugfs *tz_dbg; struct thermal_debugfs *thermal_dbg = tz->debugfs; int trip_id = thermal_zone_trip_id(tz, trip); ktime_t now = ktime_get(); struct trip_stats *trip_stats; + struct tz_debugfs *tz_dbg; + struct tz_episode *tze; if (!thermal_dbg) return; - mutex_lock(&thermal_dbg->lock); - tz_dbg = &thermal_dbg->tz_dbg; + mutex_lock(&thermal_dbg->lock); + /* * The mitigation is starting. A mitigation can contain * several episodes where each of them is related to a @@ -677,10 +677,10 @@ void thermal_debug_tz_trip_down(struct thermal_zone_device *tz, if (!thermal_dbg) return; - mutex_lock(&thermal_dbg->lock); - tz_dbg = &thermal_dbg->tz_dbg; + mutex_lock(&thermal_dbg->lock); + /* * The temperature crosses the way down but there was not * mitigation detected before. That may happen when the @@ -729,10 +729,10 @@ void thermal_debug_update_trip_stats(struct thermal_zone_device *tz) if (!thermal_dbg) return; - mutex_lock(&thermal_dbg->lock); - tz_dbg = &thermal_dbg->tz_dbg; + mutex_lock(&thermal_dbg->lock); + if (!tz_dbg->nr_trips) goto out; From f41f23b0cae1d069c680dd9e5e2cd42db8ff6dfc Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 28 May 2024 17:00:51 +0200 Subject: [PATCH 08/13] thermal: trip: Use common set of trip type names Use the same set of trip type names in sysfs and in the thermal debug code output. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano --- drivers/thermal/thermal_core.h | 2 ++ drivers/thermal/thermal_debugfs.c | 10 +--------- drivers/thermal/thermal_sysfs.c | 13 +------------ drivers/thermal/thermal_trip.c | 15 +++++++++++++++ 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h index 20e7b45673d6..afb00b567e68 100644 --- a/drivers/thermal/thermal_core.h +++ b/drivers/thermal/thermal_core.h @@ -240,6 +240,8 @@ void thermal_governor_update_tz(struct thermal_zone_device *tz, #define trip_to_trip_desc(__trip) \ container_of(__trip, struct thermal_trip_desc, trip) +const char *thermal_trip_type_name(enum thermal_trip_type trip_type); + void __thermal_zone_set_trips(struct thermal_zone_device *tz); int thermal_zone_trip_id(const struct thermal_zone_device *tz, const struct thermal_trip *trip); diff --git a/drivers/thermal/thermal_debugfs.c b/drivers/thermal/thermal_debugfs.c index d83263101ec3..7dd67bf48571 100644 --- a/drivers/thermal/thermal_debugfs.c +++ b/drivers/thermal/thermal_debugfs.c @@ -784,7 +784,6 @@ static int tze_seq_show(struct seq_file *s, void *v) struct thermal_zone_device *tz = thermal_dbg->tz_dbg.tz; struct thermal_trip_desc *td; struct tz_episode *tze; - const char *type; u64 duration_ms; int trip_id; char c; @@ -824,13 +823,6 @@ static int tze_seq_show(struct seq_file *s, void *v) if (trip_stats->trip_temp == THERMAL_TEMP_INVALID) continue; - if (trip->type == THERMAL_TRIP_PASSIVE) - type = "passive"; - else if (trip->type == THERMAL_TRIP_ACTIVE) - type = "active"; - else - type = "hot"; - if (trip_stats->timestamp != KTIME_MAX) { /* Mitigation in progress. */ ktime_t delta = ktime_sub(ktime_get(), @@ -846,7 +838,7 @@ static int tze_seq_show(struct seq_file *s, void *v) seq_printf(s, "| %*d | %*s | %*d | %*d | %c%*lld | %*d | %*d |\n", 4 , trip_id, - 8, type, + 8, thermal_trip_type_name(trip->type), 9, trip_stats->trip_temp, 9, trip_stats->trip_hyst, c, 11, duration_ms, diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c index 88211ccdfbd6..e65bbbbf0054 100644 --- a/drivers/thermal/thermal_sysfs.c +++ b/drivers/thermal/thermal_sysfs.c @@ -88,18 +88,7 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr, if (sscanf(attr->attr.name, "trip_point_%d_type", &trip_id) != 1) return -EINVAL; - switch (tz->trips[trip_id].trip.type) { - case THERMAL_TRIP_CRITICAL: - return sprintf(buf, "critical\n"); - case THERMAL_TRIP_HOT: - return sprintf(buf, "hot\n"); - case THERMAL_TRIP_PASSIVE: - return sprintf(buf, "passive\n"); - case THERMAL_TRIP_ACTIVE: - return sprintf(buf, "active\n"); - default: - return sprintf(buf, "unknown\n"); - } + return sprintf(buf, "%s\n", thermal_trip_type_name(tz->trips[trip_id].trip.type)); } static ssize_t diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c index 49e63db68517..1ffb5aeab7fb 100644 --- a/drivers/thermal/thermal_trip.c +++ b/drivers/thermal/thermal_trip.c @@ -9,6 +9,21 @@ */ #include "thermal_core.h" +static const char *trip_type_names[] = { + [THERMAL_TRIP_ACTIVE] = "active", + [THERMAL_TRIP_PASSIVE] = "passive", + [THERMAL_TRIP_HOT] = "hot", + [THERMAL_TRIP_CRITICAL] = "critical", +}; + +const char *thermal_trip_type_name(enum thermal_trip_type trip_type) +{ + if (trip_type < THERMAL_TRIP_ACTIVE || trip_type > THERMAL_TRIP_CRITICAL) + return "unknown"; + + return trip_type_names[trip_type]; +} + int for_each_thermal_trip(struct thermal_zone_device *tz, int (*cb)(struct thermal_trip *, void *), void *data) From 28d5cc12671c8b23d928128d8d4ced586b873e5f Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 7 Jun 2024 09:33:53 +0200 Subject: [PATCH 09/13] thermal: trip: Rename __thermal_zone_set_trips() to thermal_zone_set_trips() Drop the pointless double underline prefix from the function name as per the subject. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano --- drivers/thermal/thermal_core.c | 2 +- drivers/thermal/thermal_core.h | 2 +- drivers/thermal/thermal_trip.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index bd31193bb583..6acaa4b50273 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -513,7 +513,7 @@ void __thermal_zone_device_update(struct thermal_zone_device *tz, if (tz->temperature == THERMAL_TEMP_INVALID) return; - __thermal_zone_set_trips(tz); + thermal_zone_set_trips(tz); tz->notify_event = event; diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h index afb00b567e68..4af6b940ed7a 100644 --- a/drivers/thermal/thermal_core.h +++ b/drivers/thermal/thermal_core.h @@ -242,7 +242,7 @@ void thermal_governor_update_tz(struct thermal_zone_device *tz, const char *thermal_trip_type_name(enum thermal_trip_type trip_type); -void __thermal_zone_set_trips(struct thermal_zone_device *tz); +void thermal_zone_set_trips(struct thermal_zone_device *tz); int thermal_zone_trip_id(const struct thermal_zone_device *tz, const struct thermal_trip *trip); void thermal_zone_trip_updated(struct thermal_zone_device *tz, diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c index 1ffb5aeab7fb..6ab1beb9bf0d 100644 --- a/drivers/thermal/thermal_trip.c +++ b/drivers/thermal/thermal_trip.c @@ -62,7 +62,7 @@ int thermal_zone_get_num_trips(struct thermal_zone_device *tz) EXPORT_SYMBOL_GPL(thermal_zone_get_num_trips); /** - * __thermal_zone_set_trips - Computes the next trip points for the driver + * thermal_zone_set_trips - Computes the next trip points for the driver * @tz: a pointer to a thermal zone device structure * * The function computes the next temperature boundaries by browsing @@ -76,7 +76,7 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_num_trips); * * It does not return a value */ -void __thermal_zone_set_trips(struct thermal_zone_device *tz) +void thermal_zone_set_trips(struct thermal_zone_device *tz) { const struct thermal_trip_desc *td; int low = -INT_MAX, high = INT_MAX; From 893bae92237d824334d68d6b67222aaf32e41458 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 28 May 2024 18:51:11 +0200 Subject: [PATCH 10/13] thermal: trip: Make thermal_zone_set_trips() use trip thresholds Modify thermal_zone_set_trips() to use trip thresholds instead of computing the low temperature for each trip to avoid deriving both the high and low temperature levels from the same trip (which may happen if the zone temperature falls into the hysteresis range of one trip). Accordingly, make __thermal_zone_device_update() call thermal_zone_set_trips() later, when threshold values have been updated for all trips. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano --- drivers/thermal/thermal_core.c | 4 ++-- drivers/thermal/thermal_trip.c | 14 ++++---------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 6acaa4b50273..57f20326aa91 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -513,13 +513,13 @@ void __thermal_zone_device_update(struct thermal_zone_device *tz, if (tz->temperature == THERMAL_TEMP_INVALID) return; - thermal_zone_set_trips(tz); - tz->notify_event = event; for_each_trip_desc(tz, td) handle_thermal_trip(tz, td, &way_up_list, &way_down_list); + thermal_zone_set_trips(tz); + list_sort(&way_up_list, &way_up_list, thermal_trip_notify_cmp); list_for_each_entry(td, &way_up_list, notify_list_node) thermal_trip_crossed(tz, &td->trip, governor, true); diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c index 6ab1beb9bf0d..86f408f84f17 100644 --- a/drivers/thermal/thermal_trip.c +++ b/drivers/thermal/thermal_trip.c @@ -88,17 +88,11 @@ void thermal_zone_set_trips(struct thermal_zone_device *tz) return; for_each_trip_desc(tz, td) { - const struct thermal_trip *trip = &td->trip; - int trip_low; + if (td->threshold < tz->temperature && td->threshold > low) + low = td->threshold; - trip_low = trip->temperature - trip->hysteresis; - - if (trip_low < tz->temperature && trip_low > low) - low = trip_low; - - if (trip->temperature > tz->temperature && - trip->temperature < high) - high = trip->temperature; + if (td->threshold > tz->temperature && td->threshold < high) + high = td->threshold; } /* No need to change trip points */ From a52641bc6293a24f25956a597e7f32148b0e2bb8 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 28 May 2024 18:52:13 +0200 Subject: [PATCH 11/13] thermal: trip: Use READ_ONCE() for lockless access to trip properties When accessing trip temperature and hysteresis without locking, it is better to use READ_ONCE() to prevent compiler optimizations possibly affecting the read from being applied. Of course, for the READ_ONCE() to be effective, WRITE_ONCE() needs to be used when updating their values. Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_sysfs.c | 6 +++--- drivers/thermal/thermal_trip.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c index e65bbbbf0054..434b577950be 100644 --- a/drivers/thermal/thermal_sysfs.c +++ b/drivers/thermal/thermal_sysfs.c @@ -139,7 +139,7 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr, if (sscanf(attr->attr.name, "trip_point_%d_temp", &trip_id) != 1) return -EINVAL; - return sprintf(buf, "%d\n", tz->trips[trip_id].trip.temperature); + return sprintf(buf, "%d\n", READ_ONCE(tz->trips[trip_id].trip.temperature)); } static ssize_t @@ -163,7 +163,7 @@ trip_point_hyst_store(struct device *dev, struct device_attribute *attr, trip = &tz->trips[trip_id].trip; if (hyst != trip->hysteresis) { - trip->hysteresis = hyst; + WRITE_ONCE(trip->hysteresis, hyst); thermal_zone_trip_updated(tz, trip); } @@ -183,7 +183,7 @@ trip_point_hyst_show(struct device *dev, struct device_attribute *attr, if (sscanf(attr->attr.name, "trip_point_%d_hyst", &trip_id) != 1) return -EINVAL; - return sprintf(buf, "%d\n", tz->trips[trip_id].trip.hysteresis); + return sprintf(buf, "%d\n", READ_ONCE(tz->trips[trip_id].trip.hysteresis)); } static ssize_t diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c index 86f408f84f17..2a876d3b93aa 100644 --- a/drivers/thermal/thermal_trip.c +++ b/drivers/thermal/thermal_trip.c @@ -161,7 +161,7 @@ void thermal_zone_set_trip_temp(struct thermal_zone_device *tz, if (trip->temperature == temp) return; - trip->temperature = temp; + WRITE_ONCE(trip->temperature, temp); thermal_notify_tz_trip_change(tz, trip); if (temp == THERMAL_TEMP_INVALID) { From 2c637af8a74d9a2a52ee5456a75dd29c8cb52da5 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 28 May 2024 18:54:01 +0200 Subject: [PATCH 12/13] thermal: gov_bang_bang: Drop unnecessary cooling device target state checks Some cooling device target state checks in bang_bang_control() done before setting the new target state are not necessary after recent changes, so drop them. Also avoid updating the target state before checking it for unexpected values. Signed-off-by: Rafael J. Wysocki --- drivers/thermal/gov_bang_bang.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/drivers/thermal/gov_bang_bang.c b/drivers/thermal/gov_bang_bang.c index acb52c9ee10f..4a2e869b9538 100644 --- a/drivers/thermal/gov_bang_bang.c +++ b/drivers/thermal/gov_bang_bang.c @@ -57,24 +57,16 @@ static void bang_bang_control(struct thermal_zone_device *tz, if (instance->trip != trip) continue; - if (instance->target == THERMAL_NO_TARGET) - instance->target = 0; - - if (instance->target != 0 && instance->target != 1) { + if (instance->target != 0 && instance->target != 1 && + instance->target != THERMAL_NO_TARGET) pr_debug("Unexpected state %ld of thermal instance %s in bang-bang\n", instance->target, instance->name); - instance->target = 1; - } - /* * Enable the fan when the trip is crossed on the way up and * disable it when the trip is crossed on the way down. */ - if (instance->target == 0 && crossed_up) - instance->target = 1; - else if (instance->target == 1 && !crossed_up) - instance->target = 0; + instance->target = crossed_up; dev_dbg(&instance->cdev->device, "target=%ld\n", instance->target); From 72196c20c38b50c4293696377145e6c4ad9a7c67 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 28 May 2024 18:54:11 +0200 Subject: [PATCH 13/13] thermal: core: Avoid calling .trip_crossed() for critical and hot trips Invoking the governor .trip_crossed() callback for critical and hot trips is pointless because they are handled directly by the core, so make thermal_governor_trip_crossed() avoid doing that. Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 57f20326aa91..8bb63a364cba 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -463,6 +463,9 @@ static void thermal_governor_trip_crossed(struct thermal_governor *governor, const struct thermal_trip *trip, bool crossed_up) { + if (trip->type == THERMAL_TRIP_HOT || trip->type == THERMAL_TRIP_CRITICAL) + return; + if (governor->trip_crossed) governor->trip_crossed(tz, trip, crossed_up); }