[PATCH v2 0/8] thermal/debugfs: Assorted improvements for the 6.11 cycle

[PATCH v2 0/8] thermal/debugfs: Assorted improvements for the 6.11 cycle

[Rafael J. Wysocki]

Hi Everyone,

This is an update of

https://lore.kernel.org/linux-pm/12438864.O9o76ZdvQC@kreacher/

which mostly is a rebase on top of the new fixes:

https://lore.kernel.org/linux-pm/12438941.O9o76ZdvQC@kreacher/

but one new patch has been added to the series (patch [5/8]).

The patches in the series address some minor issues in the thermal
debugfs code and clean it up somewhat.

Please refer to the individual patch changelogs for details.

At one point I'm going to put this series on a separate git branch
for easier access/testing.

Thanks!

[PATCH v2 1/8] thermal/debugfs: Use helper to update trip point overstepping duration

[Rafael J. Wysocki]

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

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 <rafael.j.wysocki@intel.com>
---

v1 -> v2: Rebase.

---
 drivers/thermal/thermal_debugfs.c |   22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.c
+++ linux-pm/drivers/thermal/thermal_debugfs.c
@@ -653,14 +653,24 @@ unlock:
 	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 t
 
 	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

[PATCH v2 2/8] thermal/debugfs: Do not extend mitigation episodes beyond system resume

[Rafael J. Wysocki]

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

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 <rafael.j.wysocki@intel.com>
---

v1 -> v2: Rebase.

---
 drivers/thermal/thermal_core.c    |    1 +
 drivers/thermal/thermal_debugfs.c |   36 ++++++++++++++++++++++++++++++++++++
 drivers/thermal/thermal_debugfs.h |    2 ++
 3 files changed, 39 insertions(+)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1641,6 +1641,7 @@ static void thermal_zone_device_resume(s
 
 	tz->suspended = false;
 
+	thermal_debug_tz_resume(tz);
 	thermal_zone_device_init(tz);
 	__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
 
Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.c
+++ linux-pm/drivers/thermal/thermal_debugfs.c
@@ -926,3 +926,39 @@ void thermal_debug_tz_remove(struct ther
 	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);
+}
Index: linux-pm/drivers/thermal/thermal_debugfs.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.h
+++ linux-pm/drivers/thermal/thermal_debugfs.h
@@ -7,6 +7,7 @@ void thermal_debug_cdev_remove(struct th
 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_st
 						   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,

[PATCH v2 3/8] thermal/debugfs: Print mitigation timestamp value in milliseconds

[Rafael J. Wysocki]

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

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 <rafael.j.wysocki@intel.com>
---

v1 -> v2: Rebase.

---
 drivers/thermal/thermal_debugfs.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.c
+++ linux-pm/drivers/thermal/thermal_debugfs.c
@@ -797,8 +797,8 @@ static int tze_seq_show(struct seq_file
 		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");
 

[PATCH v2 4/8] thermal/debugfs: Fix up units in "mitigations" files

[Rafael J. Wysocki]

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

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 <rafael.j.wysocki@intel.com>
---

v1 -> v2: Rebase.

---
 drivers/thermal/thermal_debugfs.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.c
+++ linux-pm/drivers/thermal/thermal_debugfs.c
@@ -800,7 +800,7 @@ static int tze_seq_show(struct seq_file
 	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
 			   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);

[PATCH v2 5/8] thermal/debugfs: Adjust check for trips without statistics in tze_seq_show()

[Rafael J. Wysocki]

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

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 <rafael.j.wysocki@intel.com>
---

v1 -> v2: New patch.

---
 drivers/thermal/thermal_debugfs.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.c
+++ linux-pm/drivers/thermal/thermal_debugfs.c
@@ -563,6 +563,7 @@ static struct tz_episode *thermal_debugf
 	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
 		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)

[PATCH v2 6/8] thermal/debugfs: Compute maximum temperature for mitigation episode as a whole

[Rafael J. Wysocki]

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

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 <rafael.j.wysocki@intel.com>
---

v1 -> v2:
   * Rebase.
   * Take patch [5/8] into account.

---
 drivers/thermal/thermal_debugfs.c |   21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.c
+++ linux-pm/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_debugf
 	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(str
 
 	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
 		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
 			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;

[PATCH v2 7/8] thermal/debugfs: Move some statements from under thermal_dbg->lock

[Rafael J. Wysocki]

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

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 <rafael.j.wysocki@intel.com>
---

v1 -> v2: Rebase.

---
 drivers/thermal/thermal_debugfs.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.c
+++ linux-pm/drivers/thermal/thermal_debugfs.c
@@ -574,20 +574,20 @@ static struct tz_episode *thermal_debugf
 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 t
 	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(str
 	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;
 

[PATCH v2 8/8] thermal: trip: Use common set of trip type names

[Rafael J. Wysocki]

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

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 <rafael.j.wysocki@intel.com>
---

v1 -> v2: Rebase.

---
 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(-)

Index: linux-pm/drivers/thermal/thermal_trip.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_trip.c
+++ linux-pm/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)
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -240,6 +240,8 @@ void thermal_governor_update_tz(struct t
 #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);
Index: linux-pm/drivers/thermal/thermal_sysfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_sysfs.c
+++ linux-pm/drivers/thermal/thermal_sysfs.c
@@ -88,18 +88,7 @@ trip_point_type_show(struct device *dev,
 	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
Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.c
+++ linux-pm/drivers/thermal/thermal_debugfs.c
@@ -784,7 +784,6 @@ static int tze_seq_show(struct seq_file
 	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
 		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
 
 		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,

Re: [PATCH v2 1/8] thermal/debugfs: Use helper to update trip point overstepping duration

[Daniel Lezcano]

On 28/05/2024 16:52, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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 <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 v2 2/8] thermal/debugfs: Do not extend mitigation episodes beyond system resume

[Daniel Lezcano]

On 28/05/2024 16:53, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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.

Why it is done at resume time and not at suspend time ?

> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> v1 -> v2: Rebase.
> 
> ---
>   drivers/thermal/thermal_core.c    |    1 +
>   drivers/thermal/thermal_debugfs.c |   36 ++++++++++++++++++++++++++++++++++++
>   drivers/thermal/thermal_debugfs.h |    2 ++
>   3 files changed, 39 insertions(+)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -1641,6 +1641,7 @@ static void thermal_zone_device_resume(s
>   
>   	tz->suspended = false;
>   
> +	thermal_debug_tz_resume(tz);
>   	thermal_zone_device_init(tz);
>   	__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>   
> Index: linux-pm/drivers/thermal/thermal_debugfs.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_debugfs.c
> +++ linux-pm/drivers/thermal/thermal_debugfs.c
> @@ -926,3 +926,39 @@ void thermal_debug_tz_remove(struct ther
>   	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);
> +}
> Index: linux-pm/drivers/thermal/thermal_debugfs.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_debugfs.h
> +++ linux-pm/drivers/thermal/thermal_debugfs.h
> @@ -7,6 +7,7 @@ void thermal_debug_cdev_remove(struct th
>   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_st
>   						   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,
> 
> 
> 

-- 
<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 v2 3/8] thermal/debugfs: Print mitigation timestamp value in milliseconds

[Daniel Lezcano]

On 28/05/2024 16:55, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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 <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 v2 4/8] thermal/debugfs: Fix up units in "mitigations" files

[Daniel Lezcano]

On 28/05/2024 16:55, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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 <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 v2 5/8] thermal/debugfs: Adjust check for trips without statistics in tze_seq_show()

[Daniel Lezcano]

On 28/05/2024 16:57, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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 <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 v2 6/8] thermal/debugfs: Compute maximum temperature for mitigation episode as a whole

[Daniel Lezcano]

On 28/05/2024 16:58, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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 <rafael.j.wysocki@intel.com>

Indeed... :)

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 v2 2/8] thermal/debugfs: Do not extend mitigation episodes beyond system resume

[Rafael J. Wysocki]

On Mon, Jun 10, 2024 at 10:28 AM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 28/05/2024 16:53, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > 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.
>
> Why it is done at resume time and not at suspend time ?

Because it is related to thermal_zone_device_init() which also runs at
the resume time, so IMV it's better to keep these two pieces together.

Why would it be better to run this during suspend?

> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >
> > v1 -> v2: Rebase.
> >
> > ---
> >   drivers/thermal/thermal_core.c    |    1 +
> >   drivers/thermal/thermal_debugfs.c |   36 ++++++++++++++++++++++++++++++++++++
> >   drivers/thermal/thermal_debugfs.h |    2 ++
> >   3 files changed, 39 insertions(+)
> >
> > Index: linux-pm/drivers/thermal/thermal_core.c
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/thermal_core.c
> > +++ linux-pm/drivers/thermal/thermal_core.c
> > @@ -1641,6 +1641,7 @@ static void thermal_zone_device_resume(s
> >
> >       tz->suspended = false;
> >
> > +     thermal_debug_tz_resume(tz);
> >       thermal_zone_device_init(tz);
> >       __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> >
> > Index: linux-pm/drivers/thermal/thermal_debugfs.c
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/thermal_debugfs.c
> > +++ linux-pm/drivers/thermal/thermal_debugfs.c
> > @@ -926,3 +926,39 @@ void thermal_debug_tz_remove(struct ther
> >       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);
> > +}
> > Index: linux-pm/drivers/thermal/thermal_debugfs.h
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/thermal_debugfs.h
> > +++ linux-pm/drivers/thermal/thermal_debugfs.h
> > @@ -7,6 +7,7 @@ void thermal_debug_cdev_remove(struct th
> >   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_st
> >                                                  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,
> >
> >
> >
>
> --
> <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 v2 7/8] thermal/debugfs: Move some statements from under thermal_dbg->lock

[Daniel Lezcano]

On 28/05/2024 16:59, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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, 

May be explain why ?

> 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 <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 v2 8/8] thermal: trip: Use common set of trip type names

[Daniel Lezcano]

On 28/05/2024 17:00, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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 <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 v2 2/8] thermal/debugfs: Do not extend mitigation episodes beyond system resume

[Daniel Lezcano]

On 10/06/2024 13:29, Rafael J. Wysocki wrote:
> On Mon, Jun 10, 2024 at 10:28 AM Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>>
>> On 28/05/2024 16:53, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>
>>> 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.
>>
>> Why it is done at resume time and not at suspend time ?
> 
> Because it is related to thermal_zone_device_init() which also runs at
> the resume time, so IMV it's better to keep these two pieces together.
> 
> Why would it be better to run this during suspend?

 From a logical point of view, it makes more sense to cancel something 
at suspend time rather than resume. That prevents future readers to be 
puzzled by an action done in an unexpected place.

Technically speaking there is no difference if it is done during suspend 
or resume. Well... we want to prevent actions to be done at resume time 
in order to not increase the resume duration but I'm not sure this code 
is doing a big difference.

If you want to keep it as is, feel free to add my:

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 v2 2/8] thermal/debugfs: Do not extend mitigation episodes beyond system resume

[Rafael J. Wysocki]

On Mon, Jun 10, 2024 at 3:39 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 10/06/2024 13:29, Rafael J. Wysocki wrote:
> > On Mon, Jun 10, 2024 at 10:28 AM Daniel Lezcano
> > <daniel.lezcano@linaro.org> wrote:
> >>
> >> On 28/05/2024 16:53, Rafael J. Wysocki wrote:
> >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>>
> >>> 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.
> >>
> >> Why it is done at resume time and not at suspend time ?
> >
> > Because it is related to thermal_zone_device_init() which also runs at
> > the resume time, so IMV it's better to keep these two pieces together.
> >
> > Why would it be better to run this during suspend?
>
>  From a logical point of view, it makes more sense to cancel something
> at suspend time rather than resume. That prevents future readers to be
> puzzled by an action done in an unexpected place.
>
> Technically speaking there is no difference if it is done during suspend
> or resume. Well... we want to prevent actions to be done at resume time
> in order to not increase the resume duration but I'm not sure this code
> is doing a big difference.
>
> If you want to keep it as is, feel free to add my:
>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

I will, thank you!

And thanks for all of the other ACKs.