[PATCH 0/2] thermal: thermal_of: pass-through change_mode & implement for tsens

[PATCH 0/2] thermal: thermal_of: pass-through change_mode & implement for tsens

[Benjamin Li]

Plumb through the change_mode callback from thermal core into thermal_of,
and implement change_mode for the Qualcomm tsens driver.

Supersedes "[PATCH v3] drivers: thermal: tsens: respect thermal_device_mode
in threshold irq reporting". Changelog for that patchset:

Changes in v3:
- Upgraded logging to dev_info_ratelimited and revised log message.
- Remove unrelated hunk.

Some drivers that support thermal zone disabling implement a set_mode
operation and simply disable the sensor or the relevant IRQ(s), so they
actually don't log anything when zones are disabled. These drivers are
imx_thermal.c, intel_quark_dts_thermal.c, and int3400_thermal.c.

For tsens.c, implementing a change_mode would require migrating the driver
from devm_thermal_zone_of_sensor_register to thermal_zone_device_register
(or updating thermal_of.c to add a change_mode operation in thermal_zone_
of_device_ops).

stm_thermal.c seems to use this patch's model of not disabling IRQs when
the zone is disabled (they still perform the thermal_zone_device_update
upon IRQ, but return -EAGAIN from their get_temp).

Changes in v2:
- Reordered sentences in first part of commit message to make sense.

Benjamin Li (2):
  thermal: thermal_of: add pass-through change_mode to ops struct
  drivers: thermal: tsens: implement change_mode to disable sensor IRQs

 drivers/thermal/qcom/tsens.c | 43 ++++++++++++++++++++++++++++++++++--
 drivers/thermal/qcom/tsens.h |  4 ++++
 drivers/thermal/thermal_of.c | 14 ++++++++++++
 include/linux/thermal.h      |  2 ++
 4 files changed, 61 insertions(+), 2 deletions(-)

-- 
2.17.1

[PATCH 1/2] thermal: thermal_of: add pass-through change_mode to ops struct

[Benjamin Li]

Add an optional change_mode method to the ops struct, allowing thermal_of-
based drivers to properly implement disabling of sensors/IRQs. Previously
thermal_core would just disable the polling timer when a zone is disabled,
as thermal_of drivers could not provide a change_mode implementation.

This meant thermal_of drivers supporting interrupt-based updates had to
keep sensors/IRQs enabled at all times, and needed to resort to reading
tzd->mode in their IRQ handler or get_temp implementation to actually stop
updating. This is a waste of resources in IRQ handlers to ignore updates,
and for sensors that can power down it's a waste of power.

Signed-off-by: Benjamin Li <benl@squareup.com>
---
 drivers/thermal/thermal_of.c | 14 ++++++++++++++
 include/linux/thermal.h      |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index 9233f7e74454..a69ec39780a7 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -106,6 +106,17 @@ static int of_thermal_set_trips(struct thermal_zone_device *tz,
 	return data->ops->set_trips(data->sensor_data, low, high);
 }
 
+static int of_thermal_change_mode(struct thermal_zone_device *tz,
+				enum thermal_device_mode mode)
+{
+	struct __thermal_zone *data = tz->devdata;
+
+	if (!data->ops || !data->ops->change_mode)
+		return -EINVAL;
+
+	return data->ops->change_mode(data->sensor_data, mode);
+}
+
 /**
  * of_thermal_get_ntrips - function to export number of available trip
  *			   points.
@@ -405,6 +416,9 @@ thermal_zone_of_add_sensor(struct device_node *zone,
 	if (ops->set_trips)
 		tzd->ops->set_trips = of_thermal_set_trips;
 
+	if (ops->change_mode)
+		tzd->ops->change_mode = of_thermal_change_mode;
+
 	if (ops->set_emul_temp)
 		tzd->ops->set_emul_temp = of_thermal_set_emul_temp;
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index c314893970b3..469a1664f1ed 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -295,6 +295,7 @@ struct thermal_zone_params {
  * @set_trips: a pointer to a function that sets a temperature window. When
  *	       this window is left the driver must inform the thermal core via
  *	       thermal_zone_device_update.
+ * @change_mode: a pointer to a function that enables/disables reporting.
  * @set_emul_temp: a pointer to a function that sets sensor emulated
  *		   temperature.
  * @set_trip_temp: a pointer to a function that sets the trip temperature on
@@ -304,6 +305,7 @@ struct thermal_zone_of_device_ops {
 	int (*get_temp)(void *, int *);
 	int (*get_trend)(void *, int, enum thermal_trend *);
 	int (*set_trips)(void *, int, int);
+	int (*change_mode)(void *, enum thermal_device_mode);
 	int (*set_emul_temp)(void *, int);
 	int (*set_trip_temp)(void *, int, int);
 };
-- 
2.17.1

[PATCH 2/2] drivers: thermal: tsens: implement change_mode to disable sensor IRQs

[Benjamin Li]

Implement change_mode() to disable sensor IRQs when a sensor is disabled.

Note that this commit does not touch:
- The tsens device's CPU IRQ, which is global across all sensors on a tsens
  device.
- Power/clock of the sensor. Future work is needed in the tsens_ops
  interface to support single-sensor disable + handle quirks (the only
  implementation of tsens_ops->enable and disable only supports individual
  enable/disable on some sensors, and others must be enabled/disabled as a
  block).

'echo disabled > .../thermal_zoneX/mode' will disable the thermal core's
polling mechanism to check for threshold trips. However, tsens supports
an interrupt mechanism to receive notification of trips, implemented in
commit 634e11d5b450 ("drivers: thermal: tsens: Add interrupt support").
This is used sometimes to run performance test cases.

Currently the thermal zone mode that's set by userspace does not control
threshold trip events from IRQs. Let's fix this to restore the abilty to
disable thermal throttling at runtime.

====================

Tested on MSM8939 running 5.17. This platform has 8 cores; the first four
thermal zones control cpu0-3 and the last zone is for the other four CPUs
together.

  for f in /sys/class/thermal/thermal_zone*; do
    echo "disabled" > $f/mode
    echo $f | paste - $f/type $f/mode
  done

/sys/class/thermal/thermal_zone0        cpu0-thermal    disabled
/sys/class/thermal/thermal_zone1        cpu1-thermal    disabled
/sys/class/thermal/thermal_zone2        cpu2-thermal    disabled
/sys/class/thermal/thermal_zone3        cpu3-thermal    disabled
/sys/class/thermal/thermal_zone4        cpu4567-thermal disabled

With mitigation thresholds at 75 degC and load running, we can now cruise
past temp=75000 without CPU throttling kicking in.

  watch -n 1 "grep '' /sys/class/thermal/*/temp
      /sys/class/thermal/*/cur_state
      /sys/bus/cpu/devices/cpu*/cpufreq/cpuinfo_cur_freq"

/sys/class/thermal/thermal_zone0/temp:82000
/sys/class/thermal/thermal_zone1/temp:84000
/sys/class/thermal/thermal_zone2/temp:87000
/sys/class/thermal/thermal_zone3/temp:84000
/sys/class/thermal/thermal_zone4/temp:84000
/sys/class/thermal/cooling_device0/cur_state:0
/sys/class/thermal/cooling_device1/cur_state:0
/sys/bus/cpu/devices/cpu0/cpufreq/cpuinfo_cur_freq:1113600
/sys/bus/cpu/devices/cpu1/cpufreq/cpuinfo_cur_freq:1113600
/sys/bus/cpu/devices/cpu2/cpufreq/cpuinfo_cur_freq:1113600
/sys/bus/cpu/devices/cpu3/cpufreq/cpuinfo_cur_freq:1113600
/sys/bus/cpu/devices/cpu4/cpufreq/cpuinfo_cur_freq:800000
/sys/bus/cpu/devices/cpu5/cpufreq/cpuinfo_cur_freq:800000
/sys/bus/cpu/devices/cpu6/cpufreq/cpuinfo_cur_freq:800000
/sys/bus/cpu/devices/cpu7/cpufreq/cpuinfo_cur_freq:800000

Reported-by: Zac Crosby <zac@squareup.com>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Benjamin Li <benl@squareup.com>
---
 drivers/thermal/qcom/tsens.c | 43 ++++++++++++++++++++++++++++++++++--
 drivers/thermal/qcom/tsens.h |  4 ++++
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 99a8d9f3e03c..d5263436f959 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -564,8 +564,12 @@ static int tsens_set_trips(void *_sensor, int low, int high)
 	/* Write the new thresholds and clear the status */
 	regmap_field_write(priv->rf[LOW_THRESH_0 + hw_id], low_val);
 	regmap_field_write(priv->rf[UP_THRESH_0 + hw_id], high_val);
-	tsens_set_interrupt(priv, hw_id, LOWER, true);
-	tsens_set_interrupt(priv, hw_id, UPPER, true);
+	s->trips_configured = true;
+
+	if (s->enable_irqs) {
+		tsens_set_interrupt(priv, hw_id, LOWER, true);
+		tsens_set_interrupt(priv, hw_id, UPPER, true);
+	}
 
 	spin_unlock_irqrestore(&priv->ul_lock, flags);
 
@@ -575,6 +579,40 @@ static int tsens_set_trips(void *_sensor, int low, int high)
 	return 0;
 }
 
+static int tsens_change_mode(void *_sensor, enum thermal_device_mode mode)
+{
+	struct tsens_sensor *s = _sensor;
+	struct tsens_priv *priv = s->priv;
+	u32 hw_id = s->hw_id;
+	bool enable = (mode == THERMAL_DEVICE_ENABLED);
+	unsigned long flags;
+
+	if (tsens_version(priv) < VER_0_1) {
+		/* Pre v0.1 IP had a single register for each type of interrupt
+		 * and threshold, so we can't support individual enable/disable.
+		 */
+		hw_id = 0;
+		enable = true;
+	}
+
+	spin_lock_irqsave(&priv->ul_lock, flags);
+
+	/* During sensor registration, thermal core calls change_mode(ENABLED)
+	 * before it calls set_trips(low, high). To avoid enabling threshold
+	 * interrupts before thresholds are configured, let's let set_trips do
+	 * the first enable.
+	 */
+	if (s->trips_configured) {
+		tsens_set_interrupt(priv, hw_id, LOWER, enable);
+		tsens_set_interrupt(priv, hw_id, UPPER, enable);
+	}
+	s->enable_irqs = enable;
+
+	spin_unlock_irqrestore(&priv->ul_lock, flags);
+
+	return 0;
+}
+
 static int tsens_enable_irq(struct tsens_priv *priv)
 {
 	int ret;
@@ -1002,6 +1040,7 @@ static const struct thermal_zone_of_device_ops tsens_of_ops = {
 	.get_temp = tsens_get_temp,
 	.get_trend = tsens_get_trend,
 	.set_trips = tsens_set_trips,
+	.change_mode = tsens_change_mode,
 };
 
 static int tsens_register_irq(struct tsens_priv *priv, char *irqname,
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 1471a2c00f15..1a835d7688e0 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -45,6 +45,8 @@ enum tsens_irq_type {
  * @offset: offset of temperature adjustment curve
  * @hw_id: HW ID can be used in case of platform-specific IDs
  * @slope: slope of temperature adjustment curve
+ * @trips_configured: whether this sensor's upper/lower thresholds are set
+ * @enable_irqs: whether this sensor's threshold IRQs should be enabled
  * @status: 8960-specific variable to track 8960 and 8660 status register offset
  */
 struct tsens_sensor {
@@ -53,6 +55,8 @@ struct tsens_sensor {
 	int				offset;
 	unsigned int			hw_id;
 	int				slope;
+	bool				trips_configured;
+	bool				enable_irqs;
 	u32				status;
 };
 
-- 
2.17.1