[PATCH v3] Thermal hardware trip points and Mediatek thermal driver

linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v3] Thermal hardware trip points and Mediatek thermal driver
@ 2015-05-13  8:52 Sascha Hauer
  2015-05-13  8:52 ` [PATCH 01/15] thermal: consistently use int for temperatures Sascha Hauer
                   ` (14 more replies)
  0 siblings, 15 replies; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris

This series adds support for hardware trip points. It picks up earlier
work from Mikko Perttunen. Mikko implemented hardware trip points as part
of the device tree support. It was suggested back then to move the
functionality to the thermal core instead of putting more code into the
device tree support. This series does exactly that.

The majority of the patches are cleanups and fixes. Patch 11 introduces
hardware tracked trip points and finally a driver for the Mediatek
thermal controller is introduced as the first user is added.

All comments welcome

Changes since v2:
- Add Mediatek thermal controller driver as first user for hardware trip
  points

Changes since v1:
- Use int instead of unsigned long consistently for temperatures
- Instead of misfixing the emulation code add a comment how the
  code is meant
- Add doc entry for .set_trips callback
- initialize prev_low_trip/prev_high_trip properly
- get tz->lock before calling thermal_zone_set_trips()

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 01/15] thermal: consistently use int for temperatures
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  2015-05-20  7:12   ` Mikko Perttunen
  2015-05-13  8:52 ` [PATCH 02/15] thermal: trivial: fix typo in comment Sascha Hauer
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

The thermal code uses int, long and unsigned long for temperatures
in different places. Using an unsigned type limits the thermal framework
to positive temperatures without need. 'long' is 64bit on several
architectures which is not needed. Consistently use a plain 'int'
for temperatures.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/acpi/thermal.c                             | 12 +++++-----
 drivers/hwmon/lm75.c                               |  2 +-
 drivers/hwmon/ntc_thermistor.c                     |  2 +-
 drivers/hwmon/tmp102.c                             |  2 +-
 drivers/input/touchscreen/sun4i-ts.c               |  8 +++----
 drivers/platform/x86/acerhdf.c                     |  9 ++++----
 drivers/power/power_supply_core.c                  |  2 +-
 drivers/thermal/armada_thermal.c                   |  2 +-
 drivers/thermal/db8500_thermal.c                   |  7 +++---
 drivers/thermal/dove_thermal.c                     |  2 +-
 drivers/thermal/fair_share.c                       |  2 +-
 drivers/thermal/gov_bang_bang.c                    |  5 ++--
 drivers/thermal/imx_thermal.c                      | 27 +++++++++++-----------
 .../thermal/int340x_thermal/int340x_thermal_zone.c | 10 ++++----
 .../thermal/int340x_thermal/int340x_thermal_zone.h |  8 +++----
 drivers/thermal/intel_soc_dts_thermal.c            |  7 +++---
 drivers/thermal/of-thermal.c                       | 14 +++++------
 drivers/thermal/rcar_thermal.c                     |  7 +++---
 drivers/thermal/rockchip_thermal.c                 | 10 ++++----
 drivers/thermal/samsung/exynos_tmu.c               | 19 ++++++++-------
 drivers/thermal/spear_thermal.c                    |  2 +-
 drivers/thermal/st/st_thermal.c                    |  5 ++--
 drivers/thermal/step_wise.c                        |  4 ++--
 drivers/thermal/tegra_soctherm.c                   |  4 ++--
 drivers/thermal/thermal_core.c                     | 26 ++++++++++-----------
 drivers/thermal/thermal_hwmon.c                    | 10 ++++----
 drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 10 ++++----
 drivers/thermal/x86_pkg_temp_thermal.c             | 10 ++++----
 include/linux/thermal.h                            | 26 +++++++++------------
 29 files changed, 120 insertions(+), 134 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index d24fa19..68bff60 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -529,8 +529,7 @@ static void acpi_thermal_check(void *data)
 
 /* sys I/F for generic thermal sysfs support */
 
-static int thermal_get_temp(struct thermal_zone_device *thermal,
-			    unsigned long *temp)
+static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp)
 {
 	struct acpi_thermal *tz = thermal->devdata;
 	int result;
@@ -637,7 +636,7 @@ static int thermal_get_trip_type(struct thermal_zone_device *thermal,
 }
 
 static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
-				 int trip, unsigned long *temp)
+				 int trip, int *temp)
 {
 	struct acpi_thermal *tz = thermal->devdata;
 	int i;
@@ -690,7 +689,8 @@ static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
 }
 
 static int thermal_get_crit_temp(struct thermal_zone_device *thermal,
-				unsigned long *temperature) {
+				int *temperature)
+{
 	struct acpi_thermal *tz = thermal->devdata;
 
 	if (tz->trips.critical.flags.valid) {
@@ -713,8 +713,8 @@ static int thermal_get_trend(struct thermal_zone_device *thermal,
 		return -EINVAL;
 
 	if (type == THERMAL_TRIP_ACTIVE) {
-		unsigned long trip_temp;
-		unsigned long temp = DECI_KELVIN_TO_MILLICELSIUS_WITH_OFFSET(
+		int trip_temp;
+		int temp = DECI_KELVIN_TO_MILLICELSIUS_WITH_OFFSET(
 					tz->temperature, tz->kelvin_offset);
 		if (thermal_get_trip_temp(thermal, trip, &trip_temp))
 			return -EINVAL;
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index fe41d5a..e4e57bb 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -104,7 +104,7 @@ static inline long lm75_reg_to_mc(s16 temp, u8 resolution)
 
 /* sysfs attributes for hwmon */
 
-static int lm75_read_temp(void *dev, long *temp)
+static int lm75_read_temp(void *dev, int *temp)
 {
 	struct lm75_data *data = lm75_update_device(dev);
 
diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c
index 112e4d4..9f085df 100644
--- a/drivers/hwmon/ntc_thermistor.c
+++ b/drivers/hwmon/ntc_thermistor.c
@@ -430,7 +430,7 @@ static int ntc_thermistor_get_ohm(struct ntc_data *data)
 	return -EINVAL;
 }
 
-static int ntc_read_temp(void *dev, long *temp)
+static int ntc_read_temp(void *dev, int *temp)
 {
 	struct ntc_data *data = dev_get_drvdata(dev);
 	int ohm;
diff --git a/drivers/hwmon/tmp102.c b/drivers/hwmon/tmp102.c
index 9da2735..6548262 100644
--- a/drivers/hwmon/tmp102.c
+++ b/drivers/hwmon/tmp102.c
@@ -98,7 +98,7 @@ static struct tmp102 *tmp102_update_device(struct device *dev)
 	return tmp102;
 }
 
-static int tmp102_read_temp(void *dev, long *temp)
+static int tmp102_read_temp(void *dev, int *temp)
 {
 	struct tmp102 *tmp102 = tmp102_update_device(dev);
 
diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c
index c011699..4857943 100644
--- a/drivers/input/touchscreen/sun4i-ts.c
+++ b/drivers/input/touchscreen/sun4i-ts.c
@@ -191,7 +191,7 @@ static void sun4i_ts_close(struct input_dev *dev)
 	writel(TEMP_IRQ_EN(1), ts->base + TP_INT_FIFOC);
 }
 
-static int sun4i_get_temp(const struct sun4i_ts_data *ts, long *temp)
+static int sun4i_get_temp(const struct sun4i_ts_data *ts, int *temp)
 {
 	/* No temp_data until the first irq */
 	if (ts->temp_data == -1)
@@ -202,7 +202,7 @@ static int sun4i_get_temp(const struct sun4i_ts_data *ts, long *temp)
 	return 0;
 }
 
-static int sun4i_get_tz_temp(void *data, long *temp)
+static int sun4i_get_tz_temp(void *data, int *temp)
 {
 	return sun4i_get_temp(data, temp);
 }
@@ -215,14 +215,14 @@ static ssize_t show_temp(struct device *dev, struct device_attribute *devattr,
 			 char *buf)
 {
 	struct sun4i_ts_data *ts = dev_get_drvdata(dev);
-	long temp;
+	int temp;
 	int error;
 
 	error = sun4i_get_temp(ts, &temp);
 	if (error)
 		return error;
 
-	return sprintf(buf, "%ld\n", temp);
+	return sprintf(buf, "%d\n", temp);
 }
 
 static ssize_t show_temp_label(struct device *dev,
diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index 594c918..f2ce63c 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -346,8 +346,7 @@ static void acerhdf_check_param(struct thermal_zone_device *thermal)
  * as late as the polling interval is since we can't do that in the respective
  * accessors of the module parameters.
  */
-static int acerhdf_get_ec_temp(struct thermal_zone_device *thermal,
-			       unsigned long *t)
+static int acerhdf_get_ec_temp(struct thermal_zone_device *thermal, int *t)
 {
 	int temp, err = 0;
 
@@ -452,7 +451,7 @@ static int acerhdf_get_trip_type(struct thermal_zone_device *thermal, int trip,
 }
 
 static int acerhdf_get_trip_hyst(struct thermal_zone_device *thermal, int trip,
-				 unsigned long *temp)
+				 int *temp)
 {
 	if (trip != 0)
 		return -EINVAL;
@@ -463,7 +462,7 @@ static int acerhdf_get_trip_hyst(struct thermal_zone_device *thermal, int trip,
 }
 
 static int acerhdf_get_trip_temp(struct thermal_zone_device *thermal, int trip,
-				 unsigned long *temp)
+				 int *temp)
 {
 	if (trip == 0)
 		*temp = fanon;
@@ -476,7 +475,7 @@ static int acerhdf_get_trip_temp(struct thermal_zone_device *thermal, int trip,
 }
 
 static int acerhdf_get_crit_temp(struct thermal_zone_device *thermal,
-				 unsigned long *temperature)
+				 int *temperature)
 {
 	*temperature = ACERHDF_TEMP_CRIT;
 	return 0;
diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 2ed4a4a..87e2fd1 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -492,7 +492,7 @@ EXPORT_SYMBOL_GPL(power_supply_unreg_notifier);
 
 #ifdef CONFIG_THERMAL
 static int power_supply_read_temp(struct thermal_zone_device *tzd,
-		unsigned long *temp)
+		int *temp)
 {
 	struct power_supply *psy;
 	union power_supply_propval val;
diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index c2556cf..6feb25d 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -155,7 +155,7 @@ static bool armada_is_valid(struct armada_thermal_priv *priv)
 }
 
 static int armada_get_temp(struct thermal_zone_device *thermal,
-			  unsigned long *temp)
+			  int *temp)
 {
 	struct armada_thermal_priv *priv = thermal->devdata;
 	unsigned long reg;
diff --git a/drivers/thermal/db8500_thermal.c b/drivers/thermal/db8500_thermal.c
index 20adfbe..b3eca71 100644
--- a/drivers/thermal/db8500_thermal.c
+++ b/drivers/thermal/db8500_thermal.c
@@ -107,8 +107,7 @@ static int db8500_cdev_unbind(struct thermal_zone_device *thermal,
 }
 
 /* Callback to get current temperature */
-static int db8500_sys_get_temp(struct thermal_zone_device *thermal,
-		unsigned long *temp)
+static int db8500_sys_get_temp(struct thermal_zone_device *thermal, int *temp)
 {
 	struct db8500_thermal_zone *pzone = thermal->devdata;
 
@@ -180,7 +179,7 @@ static int db8500_sys_get_trip_type(struct thermal_zone_device *thermal,
 
 /* Callback to get trip point temperature */
 static int db8500_sys_get_trip_temp(struct thermal_zone_device *thermal,
-		int trip, unsigned long *temp)
+		int trip, int *temp)
 {
 	struct db8500_thermal_zone *pzone = thermal->devdata;
 	struct db8500_thsens_platform_data *ptrips = pzone->trip_tab;
@@ -195,7 +194,7 @@ static int db8500_sys_get_trip_temp(struct thermal_zone_device *thermal,
 
 /* Callback to get critical trip point temperature */
 static int db8500_sys_get_crit_temp(struct thermal_zone_device *thermal,
-		unsigned long *temp)
+		int *temp)
 {
 	struct db8500_thermal_zone *pzone = thermal->devdata;
 	struct db8500_thsens_platform_data *ptrips = pzone->trip_tab;
diff --git a/drivers/thermal/dove_thermal.c b/drivers/thermal/dove_thermal.c
index 09f6e30..a0bc9de 100644
--- a/drivers/thermal/dove_thermal.c
+++ b/drivers/thermal/dove_thermal.c
@@ -93,7 +93,7 @@ static int dove_init_sensor(const struct dove_thermal_priv *priv)
 }
 
 static int dove_get_temp(struct thermal_zone_device *thermal,
-			  unsigned long *temp)
+			  int *temp)
 {
 	unsigned long reg;
 	struct dove_thermal_priv *priv = thermal->devdata;
diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
index 6e0a3fb..efad70e 100644
--- a/drivers/thermal/fair_share.c
+++ b/drivers/thermal/fair_share.c
@@ -34,7 +34,7 @@
 static int get_trip_level(struct thermal_zone_device *tz)
 {
 	int count = 0;
-	unsigned long trip_temp;
+	int trip_temp;
 	enum thermal_trip_type trip_type;
 
 	if (tz->trips == 0 || !tz->ops->get_trip_temp)
diff --git a/drivers/thermal/gov_bang_bang.c b/drivers/thermal/gov_bang_bang.c
index c5dd76b..70836c5 100644
--- a/drivers/thermal/gov_bang_bang.c
+++ b/drivers/thermal/gov_bang_bang.c
@@ -25,14 +25,13 @@
 
 static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
 {
-	long trip_temp;
-	unsigned long trip_hyst;
+	int trip_temp, trip_hyst;
 	struct thermal_instance *instance;
 
 	tz->ops->get_trip_temp(tz, trip, &trip_temp);
 	tz->ops->get_trip_hyst(tz, trip, &trip_hyst);
 
-	dev_dbg(&tz->device, "Trip%d[temp=%ld]:temp=%d:hyst=%ld\n",
+	dev_dbg(&tz->device, "Trip%d[temp=%d]:temp=%d:hyst=%d\n",
 				trip, trip_temp, tz->temperature,
 				trip_hyst);
 
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 2ccbc07..f1424f0 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -98,10 +98,10 @@ struct imx_thermal_data {
 	enum thermal_device_mode mode;
 	struct regmap *tempmon;
 	u32 c1, c2; /* See formula in imx_get_sensor_data() */
-	unsigned long temp_passive;
-	unsigned long temp_critical;
-	unsigned long alarm_temp;
-	unsigned long last_temp;
+	int temp_passive;
+	int temp_critical;
+	int alarm_temp;
+	int last_temp;
 	bool irq_enabled;
 	int irq;
 	struct clk *thermal_clk;
@@ -109,7 +109,7 @@ struct imx_thermal_data {
 };
 
 static void imx_set_panic_temp(struct imx_thermal_data *data,
-			       signed long panic_temp)
+			       int panic_temp)
 {
 	struct regmap *map = data->tempmon;
 	int critical_value;
@@ -121,7 +121,7 @@ static void imx_set_panic_temp(struct imx_thermal_data *data,
 }
 
 static void imx_set_alarm_temp(struct imx_thermal_data *data,
-			       signed long alarm_temp)
+			       int alarm_temp)
 {
 	struct regmap *map = data->tempmon;
 	int alarm_value;
@@ -133,7 +133,7 @@ static void imx_set_alarm_temp(struct imx_thermal_data *data,
 			TEMPSENSE0_ALARM_VALUE_SHIFT);
 }
 
-static int imx_get_temp(struct thermal_zone_device *tz, unsigned long *temp)
+static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
 {
 	struct imx_thermal_data *data = tz->devdata;
 	struct regmap *map = data->tempmon;
@@ -189,13 +189,13 @@ static int imx_get_temp(struct thermal_zone_device *tz, unsigned long *temp)
 		if (data->alarm_temp == data->temp_critical &&
 			*temp < data->temp_passive) {
 			imx_set_alarm_temp(data, data->temp_passive);
-			dev_dbg(&tz->device, "thermal alarm off: T < %lu\n",
+			dev_dbg(&tz->device, "thermal alarm off: T < %d\n",
 				data->alarm_temp / 1000);
 		}
 	}
 
 	if (*temp != data->last_temp) {
-		dev_dbg(&tz->device, "millicelsius: %ld\n", *temp);
+		dev_dbg(&tz->device, "millicelsius: %d\n", *temp);
 		data->last_temp = *temp;
 	}
 
@@ -262,8 +262,7 @@ static int imx_get_trip_type(struct thermal_zone_device *tz, int trip,
 	return 0;
 }
 
-static int imx_get_crit_temp(struct thermal_zone_device *tz,
-			     unsigned long *temp)
+static int imx_get_crit_temp(struct thermal_zone_device *tz, int *temp)
 {
 	struct imx_thermal_data *data = tz->devdata;
 
@@ -272,7 +271,7 @@ static int imx_get_crit_temp(struct thermal_zone_device *tz,
 }
 
 static int imx_get_trip_temp(struct thermal_zone_device *tz, int trip,
-			     unsigned long *temp)
+			     int *temp)
 {
 	struct imx_thermal_data *data = tz->devdata;
 
@@ -282,7 +281,7 @@ static int imx_get_trip_temp(struct thermal_zone_device *tz, int trip,
 }
 
 static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
-			     unsigned long temp)
+			     int temp)
 {
 	struct imx_thermal_data *data = tz->devdata;
 
@@ -433,7 +432,7 @@ static irqreturn_t imx_thermal_alarm_irq_thread(int irq, void *dev)
 {
 	struct imx_thermal_data *data = dev;
 
-	dev_dbg(&data->tz->device, "THERMAL ALARM: T > %lu\n",
+	dev_dbg(&data->tz->device, "THERMAL ALARM: T > %d\n",
 		data->alarm_temp / 1000);
 
 	thermal_zone_device_update(data->tz);
diff --git a/drivers/thermal/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/int340x_thermal/int340x_thermal_zone.c
index 1e25133..b9b2666 100644
--- a/drivers/thermal/int340x_thermal/int340x_thermal_zone.c
+++ b/drivers/thermal/int340x_thermal/int340x_thermal_zone.c
@@ -20,7 +20,7 @@
 #include "int340x_thermal_zone.h"
 
 static int int340x_thermal_get_zone_temp(struct thermal_zone_device *zone,
-					 unsigned long *temp)
+					 int *temp)
 {
 	struct int34x_thermal_zone *d = zone->devdata;
 	unsigned long long tmp;
@@ -49,7 +49,7 @@ static int int340x_thermal_get_zone_temp(struct thermal_zone_device *zone,
 }
 
 static int int340x_thermal_get_trip_temp(struct thermal_zone_device *zone,
-					 int trip, unsigned long *temp)
+					 int trip, int *temp)
 {
 	struct int34x_thermal_zone *d = zone->devdata;
 	int i;
@@ -114,7 +114,7 @@ static int int340x_thermal_get_trip_type(struct thermal_zone_device *zone,
 }
 
 static int int340x_thermal_set_trip_temp(struct thermal_zone_device *zone,
-				      int trip, unsigned long temp)
+				      int trip, int temp)
 {
 	struct int34x_thermal_zone *d = zone->devdata;
 	acpi_status status;
@@ -136,7 +136,7 @@ static int int340x_thermal_set_trip_temp(struct thermal_zone_device *zone,
 
 
 static int int340x_thermal_get_trip_hyst(struct thermal_zone_device *zone,
-		int trip, unsigned long *temp)
+		int trip, int *temp)
 {
 	struct int34x_thermal_zone *d = zone->devdata;
 	acpi_status status;
@@ -163,7 +163,7 @@ static struct thermal_zone_device_ops int340x_thermal_zone_ops = {
 };
 
 static int int340x_thermal_get_trip_config(acpi_handle handle, char *name,
-				      unsigned long *temp)
+				      int *temp)
 {
 	unsigned long long r;
 	acpi_status status;
diff --git a/drivers/thermal/int340x_thermal/int340x_thermal_zone.h b/drivers/thermal/int340x_thermal/int340x_thermal_zone.h
index 9f38ab7..aaadf72 100644
--- a/drivers/thermal/int340x_thermal/int340x_thermal_zone.h
+++ b/drivers/thermal/int340x_thermal/int340x_thermal_zone.h
@@ -21,7 +21,7 @@
 #define INT340X_THERMAL_MAX_ACT_TRIP_COUNT	10
 
 struct active_trip {
-	unsigned long temp;
+	int temp;
 	int id;
 	bool valid;
 };
@@ -31,11 +31,11 @@ struct int34x_thermal_zone {
 	struct active_trip act_trips[INT340X_THERMAL_MAX_ACT_TRIP_COUNT];
 	unsigned long *aux_trips;
 	int aux_trip_nr;
-	unsigned long psv_temp;
+	int psv_temp;
 	int psv_trip_id;
-	unsigned long crt_temp;
+	int crt_temp;
 	int crt_trip_id;
-	unsigned long hot_temp;
+	int hot_temp;
 	int hot_trip_id;
 	struct thermal_zone_device *zone;
 	struct thermal_zone_device_ops *override_ops;
diff --git a/drivers/thermal/intel_soc_dts_thermal.c b/drivers/thermal/intel_soc_dts_thermal.c
index 9013505..fd550b9 100644
--- a/drivers/thermal/intel_soc_dts_thermal.c
+++ b/drivers/thermal/intel_soc_dts_thermal.c
@@ -106,7 +106,7 @@ err_ret:
 }
 
 static int sys_get_trip_temp(struct thermal_zone_device *tzd,
-					int trip, unsigned long *temp)
+					int trip, int *temp)
 {
 	int status;
 	u32 out;
@@ -224,7 +224,7 @@ err_restore_ptps:
 }
 
 static int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip,
-							unsigned long temp)
+							int temp)
 {
 	struct soc_sensor_entry *aux_entry = tzd->devdata;
 	int status;
@@ -250,8 +250,7 @@ static int sys_get_trip_type(struct thermal_zone_device *thermal,
 	return 0;
 }
 
-static int sys_get_curr_temp(struct thermal_zone_device *tzd,
-						unsigned long *temp)
+static int sys_get_curr_temp(struct thermal_zone_device *tzd, int *temp)
 {
 	int status;
 	u32 out;
diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index 668fb1b..03839df 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -87,7 +87,7 @@ struct __thermal_zone {
 /***   DT thermal zone device callbacks   ***/
 
 static int of_thermal_get_temp(struct thermal_zone_device *tz,
-			       unsigned long *temp)
+			       int *temp)
 {
 	struct __thermal_zone *data = tz->devdata;
 
@@ -173,7 +173,7 @@ EXPORT_SYMBOL_GPL(of_thermal_get_trip_points);
  * Return: zero on success, error code otherwise
  */
 static int of_thermal_set_emul_temp(struct thermal_zone_device *tz,
-				    unsigned long temp)
+				    int temp)
 {
 	struct __thermal_zone *data = tz->devdata;
 
@@ -306,7 +306,7 @@ static int of_thermal_get_trip_type(struct thermal_zone_device *tz, int trip,
 }
 
 static int of_thermal_get_trip_temp(struct thermal_zone_device *tz, int trip,
-				    unsigned long *temp)
+				    int *temp)
 {
 	struct __thermal_zone *data = tz->devdata;
 
@@ -319,7 +319,7 @@ static int of_thermal_get_trip_temp(struct thermal_zone_device *tz, int trip,
 }
 
 static int of_thermal_set_trip_temp(struct thermal_zone_device *tz, int trip,
-				    unsigned long temp)
+				    int temp)
 {
 	struct __thermal_zone *data = tz->devdata;
 
@@ -333,7 +333,7 @@ static int of_thermal_set_trip_temp(struct thermal_zone_device *tz, int trip,
 }
 
 static int of_thermal_get_trip_hyst(struct thermal_zone_device *tz, int trip,
-				    unsigned long *hyst)
+				    int *hyst)
 {
 	struct __thermal_zone *data = tz->devdata;
 
@@ -346,7 +346,7 @@ static int of_thermal_get_trip_hyst(struct thermal_zone_device *tz, int trip,
 }
 
 static int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip,
-				    unsigned long hyst)
+				    int hyst)
 {
 	struct __thermal_zone *data = tz->devdata;
 
@@ -360,7 +360,7 @@ static int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip,
 }
 
 static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
-				    unsigned long *temp)
+				    int *temp)
 {
 	struct __thermal_zone *data = tz->devdata;
 	int i;
diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index fe4e767..5d4ae7d 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -200,8 +200,7 @@ err_out_unlock:
 	return ret;
 }
 
-static int rcar_thermal_get_temp(struct thermal_zone_device *zone,
-				 unsigned long *temp)
+static int rcar_thermal_get_temp(struct thermal_zone_device *zone, int *temp)
 {
 	struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
 
@@ -235,7 +234,7 @@ static int rcar_thermal_get_trip_type(struct thermal_zone_device *zone,
 }
 
 static int rcar_thermal_get_trip_temp(struct thermal_zone_device *zone,
-				      int trip, unsigned long *temp)
+				      int trip, int *temp)
 {
 	struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
 	struct device *dev = rcar_priv_to_dev(priv);
@@ -299,7 +298,7 @@ static void _rcar_thermal_irq_ctrl(struct rcar_thermal_priv *priv, int enable)
 static void rcar_thermal_work(struct work_struct *work)
 {
 	struct rcar_thermal_priv *priv;
-	unsigned long cctemp, nctemp;
+	int cctemp, nctemp;
 
 	priv = container_of(work, struct rcar_thermal_priv, work.work);
 
diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index 3aa46ac..0cc219c 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -64,7 +64,7 @@ struct rockchip_tsadc_chip {
 	void (*control)(void __iomem *reg, bool on);
 
 	/* Per-sensor methods */
-	int (*get_temp)(int chn, void __iomem *reg, long *temp);
+	int (*get_temp)(int chn, void __iomem *reg, int *temp);
 	void (*set_tshut_temp)(int chn, void __iomem *reg, long temp);
 	void (*set_tshut_mode)(int chn, void __iomem *reg, enum tshut_mode m);
 };
@@ -191,7 +191,7 @@ static u32 rk_tsadcv2_temp_to_code(long temp)
 	return 0;
 }
 
-static long rk_tsadcv2_code_to_temp(u32 code)
+static int rk_tsadcv2_code_to_temp(u32 code)
 {
 	unsigned int low = 0;
 	unsigned int high = ARRAY_SIZE(v2_code_table) - 1;
@@ -277,7 +277,7 @@ static void rk_tsadcv2_control(void __iomem *regs, bool enable)
 	writel_relaxed(val, regs + TSADCV2_AUTO_CON);
 }
 
-static int rk_tsadcv2_get_temp(int chn, void __iomem *regs, long *temp)
+static int rk_tsadcv2_get_temp(int chn, void __iomem *regs, int *temp)
 {
 	u32 val;
 
@@ -366,7 +366,7 @@ static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
-static int rockchip_thermal_get_temp(void *_sensor, long *out_temp)
+static int rockchip_thermal_get_temp(void *_sensor, int *out_temp)
 {
 	struct rockchip_thermal_sensor *sensor = _sensor;
 	struct rockchip_thermal_data *thermal = sensor->thermal;
@@ -374,7 +374,7 @@ static int rockchip_thermal_get_temp(void *_sensor, long *out_temp)
 	int retval;
 
 	retval = tsadc->get_temp(sensor->id, thermal->regs, out_temp);
-	dev_dbg(&thermal->pdev->dev, "sensor %d - temp: %ld, retval: %d\n",
+	dev_dbg(&thermal->pdev->dev, "sensor %d - temp: %d, retval: %d\n",
 		sensor->id, *out_temp, retval);
 
 	return retval;
diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 1d30b09..29eaf4d 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -181,8 +181,7 @@ struct exynos_tmu_data {
 	int (*tmu_initialize)(struct platform_device *pdev);
 	void (*tmu_control)(struct platform_device *pdev, bool on);
 	int (*tmu_read)(struct exynos_tmu_data *data);
-	void (*tmu_set_emulation)(struct exynos_tmu_data *data,
-				  unsigned long temp);
+	void (*tmu_set_emulation)(struct exynos_tmu_data *data, int temp);
 	void (*tmu_clear_irqs)(struct exynos_tmu_data *data);
 };
 
@@ -190,7 +189,7 @@ static void exynos_report_trigger(struct exynos_tmu_data *p)
 {
 	char data[10], *envp[] = { data, NULL };
 	struct thermal_zone_device *tz = p->tzd;
-	unsigned long temp;
+	int temp;
 	unsigned int i;
 
 	if (!tz) {
@@ -489,7 +488,7 @@ static int exynos5440_tmu_initialize(struct platform_device *pdev)
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	unsigned int trim_info = 0, con, rising_threshold;
 	int ret = 0, threshold_code;
-	unsigned long crit_temp = 0;
+	int crit_temp = 0;
 
 	/*
 	 * For exynos5440 soc triminfo value is swapped between TMU0 and
@@ -542,7 +541,7 @@ static int exynos7_tmu_initialize(struct platform_device *pdev)
 	unsigned int status, trim_info;
 	unsigned int rising_threshold = 0, falling_threshold = 0;
 	int ret = 0, threshold_code, i;
-	unsigned long temp, temp_hist;
+	int temp, temp_hist;
 	unsigned int reg_off, bit_off;
 
 	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
@@ -713,7 +712,7 @@ static void exynos7_tmu_control(struct platform_device *pdev, bool on)
 	writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
 }
 
-static int exynos_get_temp(void *p, long *temp)
+static int exynos_get_temp(void *p, int *temp)
 {
 	struct exynos_tmu_data *data = p;
 
@@ -733,7 +732,7 @@ static int exynos_get_temp(void *p, long *temp)
 
 #ifdef CONFIG_THERMAL_EMULATION
 static u32 get_emul_con_reg(struct exynos_tmu_data *data, unsigned int val,
-			    unsigned long temp)
+			    int temp)
 {
 	if (temp) {
 		temp /= MCELSIUS;
@@ -763,7 +762,7 @@ static u32 get_emul_con_reg(struct exynos_tmu_data *data, unsigned int val,
 }
 
 static void exynos4412_tmu_set_emulation(struct exynos_tmu_data *data,
-					 unsigned long temp)
+					 int temp)
 {
 	unsigned int val;
 	u32 emul_con;
@@ -781,7 +780,7 @@ static void exynos4412_tmu_set_emulation(struct exynos_tmu_data *data,
 }
 
 static void exynos5440_tmu_set_emulation(struct exynos_tmu_data *data,
-					 unsigned long temp)
+					 int temp)
 {
 	unsigned int val;
 
@@ -790,7 +789,7 @@ static void exynos5440_tmu_set_emulation(struct exynos_tmu_data *data,
 	writel(val, data->base + EXYNOS5440_TMU_S0_7_DEBUG);
 }
 
-static int exynos_tmu_set_emulation(void *drv_data, unsigned long temp)
+static int exynos_tmu_set_emulation(void *drv_data, int temp)
 {
 	struct exynos_tmu_data *data = drv_data;
 	int ret = -EINVAL;
diff --git a/drivers/thermal/spear_thermal.c b/drivers/thermal/spear_thermal.c
index bddb717..534dd91 100644
--- a/drivers/thermal/spear_thermal.c
+++ b/drivers/thermal/spear_thermal.c
@@ -38,7 +38,7 @@ struct spear_thermal_dev {
 };
 
 static inline int thermal_get_temp(struct thermal_zone_device *thermal,
-				unsigned long *temp)
+				int *temp)
 {
 	struct spear_thermal_dev *stdev = thermal->devdata;
 
diff --git a/drivers/thermal/st/st_thermal.c b/drivers/thermal/st/st_thermal.c
index 76c515d..44cbba9 100644
--- a/drivers/thermal/st/st_thermal.c
+++ b/drivers/thermal/st/st_thermal.c
@@ -111,8 +111,7 @@ static int st_thermal_calibration(struct st_thermal_sensor *sensor)
 }
 
 /* Callback to get temperature from HW*/
-static int st_thermal_get_temp(struct thermal_zone_device *th,
-		unsigned long *temperature)
+static int st_thermal_get_temp(struct thermal_zone_device *th, int *temperature)
 {
 	struct st_thermal_sensor *sensor = th->devdata;
 	struct device *dev = sensor->dev;
@@ -159,7 +158,7 @@ static int st_thermal_get_trip_type(struct thermal_zone_device *th,
 }
 
 static int st_thermal_get_trip_temp(struct thermal_zone_device *th,
-				    int trip, unsigned long *temp)
+				    int trip, int *temp)
 {
 	struct st_thermal_sensor *sensor = th->devdata;
 	struct device *dev = sensor->dev;
diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
index 5a0f12d..2f9f708 100644
--- a/drivers/thermal/step_wise.c
+++ b/drivers/thermal/step_wise.c
@@ -113,7 +113,7 @@ static void update_passive_instance(struct thermal_zone_device *tz,
 
 static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
 {
-	long trip_temp;
+	int trip_temp;
 	enum thermal_trip_type trip_type;
 	enum thermal_trend trend;
 	struct thermal_instance *instance;
@@ -135,7 +135,7 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
 		trace_thermal_zone_trip(tz, trip, trip_type);
 	}
 
-	dev_dbg(&tz->device, "Trip%d[type=%d,temp=%ld]:trend=%d,throttle=%d\n",
+	dev_dbg(&tz->device, "Trip%d[type=%d,temp=%d]:trend=%d,throttle=%d\n",
 				trip, trip_type, trip_temp, trend, throttle);
 
 	mutex_lock(&tz->lock);
diff --git a/drivers/thermal/tegra_soctherm.c b/drivers/thermal/tegra_soctherm.c
index 9197fc0..74ea576 100644
--- a/drivers/thermal/tegra_soctherm.c
+++ b/drivers/thermal/tegra_soctherm.c
@@ -293,7 +293,7 @@ static int enable_tsensor(struct tegra_soctherm *tegra,
  * H denotes an addition of 0.5 Celsius and N denotes negation
  * of the final value.
  */
-static long translate_temp(u16 val)
+static int translate_temp(u16 val)
 {
 	long t;
 
@@ -306,7 +306,7 @@ static long translate_temp(u16 val)
 	return t;
 }
 
-static int tegra_thermctl_get_temp(void *data, long *out_temp)
+static int tegra_thermctl_get_temp(void *data, int *out_temp)
 {
 	struct tegra_thermctl_zone *zone = data;
 	u32 val;
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 4108db7..62cc82a 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -363,7 +363,7 @@ static void handle_non_critical_trips(struct thermal_zone_device *tz,
 static void handle_critical_trips(struct thermal_zone_device *tz,
 				int trip, enum thermal_trip_type trip_type)
 {
-	long trip_temp;
+	int trip_temp;
 
 	tz->ops->get_trip_temp(tz, trip, &trip_temp);
 
@@ -411,12 +411,12 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
  *
  * Return: On success returns 0, an error code otherwise
  */
-int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp)
+int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
 {
 	int ret = -EINVAL;
 #ifdef CONFIG_THERMAL_EMULATION
 	int count;
-	unsigned long crit_temp = -1UL;
+	int crit_temp = INT_MAX;
 	enum thermal_trip_type type;
 #endif
 
@@ -453,8 +453,7 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_temp);
 
 static void update_temperature(struct thermal_zone_device *tz)
 {
-	long temp;
-	int ret;
+	int temp, ret;
 
 	ret = thermal_zone_get_temp(tz, &temp);
 	if (ret) {
@@ -514,15 +513,14 @@ static ssize_t
 temp_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
-	long temperature;
-	int ret;
+	int temperature, ret;
 
 	ret = thermal_zone_get_temp(tz, &temperature);
 
 	if (ret)
 		return ret;
 
-	return sprintf(buf, "%ld\n", temperature);
+	return sprintf(buf, "%d\n", temperature);
 }
 
 static ssize_t
@@ -626,7 +624,7 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
 	int trip, ret;
-	long temperature;
+	int temperature;
 
 	if (!tz->ops->get_trip_temp)
 		return -EPERM;
@@ -639,7 +637,7 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
 	if (ret)
 		return ret;
 
-	return sprintf(buf, "%ld\n", temperature);
+	return sprintf(buf, "%d\n", temperature);
 }
 
 static ssize_t
@@ -648,7 +646,7 @@ trip_point_hyst_store(struct device *dev, struct device_attribute *attr,
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
 	int trip, ret;
-	unsigned long temperature;
+	int temperature;
 
 	if (!tz->ops->set_trip_hyst)
 		return -EPERM;
@@ -656,7 +654,7 @@ trip_point_hyst_store(struct device *dev, struct device_attribute *attr,
 	if (!sscanf(attr->attr.name, "trip_point_%d_hyst", &trip))
 		return -EINVAL;
 
-	if (kstrtoul(buf, 10, &temperature))
+	if (kstrtoint(buf, 10, &temperature))
 		return -EINVAL;
 
 	/*
@@ -675,7 +673,7 @@ trip_point_hyst_show(struct device *dev, struct device_attribute *attr,
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
 	int trip, ret;
-	unsigned long temperature;
+	int temperature;
 
 	if (!tz->ops->get_trip_hyst)
 		return -EPERM;
@@ -685,7 +683,7 @@ trip_point_hyst_show(struct device *dev, struct device_attribute *attr,
 
 	ret = tz->ops->get_trip_hyst(tz, trip, &temperature);
 
-	return ret ? ret : sprintf(buf, "%ld\n", temperature);
+	return ret ? ret : sprintf(buf, "%d\n", temperature);
 }
 
 static ssize_t
diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
index 1967bee..06fd2ed9 100644
--- a/drivers/thermal/thermal_hwmon.c
+++ b/drivers/thermal/thermal_hwmon.c
@@ -69,7 +69,7 @@ static DEVICE_ATTR(name, 0444, name_show, NULL);
 static ssize_t
 temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
-	long temperature;
+	int temperature;
 	int ret;
 	struct thermal_hwmon_attr *hwmon_attr
 			= container_of(attr, struct thermal_hwmon_attr, attr);
@@ -83,7 +83,7 @@ temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
 	if (ret)
 		return ret;
 
-	return sprintf(buf, "%ld\n", temperature);
+	return sprintf(buf, "%d\n", temperature);
 }
 
 static ssize_t
@@ -95,14 +95,14 @@ temp_crit_show(struct device *dev, struct device_attribute *attr, char *buf)
 			= container_of(hwmon_attr, struct thermal_hwmon_temp,
 				       temp_crit);
 	struct thermal_zone_device *tz = temp->tz;
-	long temperature;
+	int temperature;
 	int ret;
 
 	ret = tz->ops->get_trip_temp(tz, 0, &temperature);
 	if (ret)
 		return ret;
 
-	return sprintf(buf, "%ld\n", temperature);
+	return sprintf(buf, "%d\n", temperature);
 }
 
 
@@ -142,7 +142,7 @@ thermal_hwmon_lookup_temp(const struct thermal_hwmon_device *hwmon,
 
 static bool thermal_zone_crit_temp_valid(struct thermal_zone_device *tz)
 {
-	unsigned long temp;
+	int temp;
 	return tz->ops->get_crit_temp && !tz->ops->get_crit_temp(tz, &temp);
 }
 
diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index a38c175..d3a42bf 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -76,14 +76,14 @@ static inline int ti_thermal_hotspot_temperature(int t, int s, int c)
 
 /* thermal zone ops */
 /* Get temperature callback function for thermal zone*/
-static inline int __ti_thermal_get_temp(void *devdata, long *temp)
+static inline int __ti_thermal_get_temp(void *devdata, int *temp)
 {
 	struct thermal_zone_device *pcb_tz = NULL;
 	struct ti_thermal_data *data = devdata;
 	struct ti_bandgap *bgp;
 	const struct ti_temp_sensor *s;
 	int ret, tmp, slope, constant;
-	unsigned long pcb_temp;
+	int pcb_temp;
 
 	if (!data)
 		return 0;
@@ -119,7 +119,7 @@ static inline int __ti_thermal_get_temp(void *devdata, long *temp)
 }
 
 static inline int ti_thermal_get_temp(struct thermal_zone_device *thermal,
-				      unsigned long *temp)
+				      int *temp)
 {
 	struct ti_thermal_data *data = thermal->devdata;
 
@@ -228,7 +228,7 @@ static int ti_thermal_get_trip_type(struct thermal_zone_device *thermal,
 
 /* Get trip temperature callback functions for thermal zone */
 static int ti_thermal_get_trip_temp(struct thermal_zone_device *thermal,
-				    int trip, unsigned long *temp)
+				    int trip, int *temp)
 {
 	if (!ti_thermal_is_valid_trip(trip))
 		return -EINVAL;
@@ -279,7 +279,7 @@ static int ti_thermal_get_trend(struct thermal_zone_device *thermal,
 
 /* Get critical temperature callback functions for thermal zone */
 static int ti_thermal_get_crit_temp(struct thermal_zone_device *thermal,
-				    unsigned long *temp)
+				    int *temp)
 {
 	/* shutdown zone */
 	return ti_thermal_get_trip_temp(thermal, OMAP_TRIP_NUMBER - 1, temp);
diff --git a/drivers/thermal/x86_pkg_temp_thermal.c b/drivers/thermal/x86_pkg_temp_thermal.c
index 9ea3d9d..054c6d45 100644
--- a/drivers/thermal/x86_pkg_temp_thermal.c
+++ b/drivers/thermal/x86_pkg_temp_thermal.c
@@ -164,7 +164,7 @@ err_ret:
 	return err;
 }
 
-static int sys_get_curr_temp(struct thermal_zone_device *tzd, unsigned long *temp)
+static int sys_get_curr_temp(struct thermal_zone_device *tzd, int *temp)
 {
 	u32 eax, edx;
 	struct phy_dev_entry *phy_dev_entry;
@@ -175,7 +175,7 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd, unsigned long *tem
 	if (eax & 0x80000000) {
 		*temp = phy_dev_entry->tj_max -
 				((eax >> 16) & 0x7f) * 1000;
-		pr_debug("sys_get_curr_temp %ld\n", *temp);
+		pr_debug("sys_get_curr_temp %d\n", *temp);
 		return 0;
 	}
 
@@ -183,7 +183,7 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd, unsigned long *tem
 }
 
 static int sys_get_trip_temp(struct thermal_zone_device *tzd,
-		int trip, unsigned long *temp)
+		int trip, int *temp)
 {
 	u32 eax, edx;
 	struct phy_dev_entry *phy_dev_entry;
@@ -214,13 +214,13 @@ static int sys_get_trip_temp(struct thermal_zone_device *tzd,
 		*temp = phy_dev_entry->tj_max - thres_reg_value * 1000;
 	else
 		*temp = 0;
-	pr_debug("sys_get_trip_temp %ld\n", *temp);
+	pr_debug("sys_get_trip_temp %d\n", *temp);
 
 	return 0;
 }
 
 static int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip,
-							unsigned long temp)
+							int temp)
 {
 	u32 l, h;
 	struct phy_dev_entry *phy_dev_entry;
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 5eac316..e9f2863 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -86,23 +86,19 @@ struct thermal_zone_device_ops {
 		     struct thermal_cooling_device *);
 	int (*unbind) (struct thermal_zone_device *,
 		       struct thermal_cooling_device *);
-	int (*get_temp) (struct thermal_zone_device *, unsigned long *);
+	int (*get_temp) (struct thermal_zone_device *, int *);
 	int (*get_mode) (struct thermal_zone_device *,
 			 enum thermal_device_mode *);
 	int (*set_mode) (struct thermal_zone_device *,
 		enum thermal_device_mode);
 	int (*get_trip_type) (struct thermal_zone_device *, int,
 		enum thermal_trip_type *);
-	int (*get_trip_temp) (struct thermal_zone_device *, int,
-			      unsigned long *);
-	int (*set_trip_temp) (struct thermal_zone_device *, int,
-			      unsigned long);
-	int (*get_trip_hyst) (struct thermal_zone_device *, int,
-			      unsigned long *);
-	int (*set_trip_hyst) (struct thermal_zone_device *, int,
-			      unsigned long);
-	int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
-	int (*set_emul_temp) (struct thermal_zone_device *, unsigned long);
+	int (*get_trip_temp) (struct thermal_zone_device *, int, int *);
+	int (*set_trip_temp) (struct thermal_zone_device *, int, int);
+	int (*get_trip_hyst) (struct thermal_zone_device *, int, int *);
+	int (*set_trip_hyst) (struct thermal_zone_device *, int, int);
+	int (*get_crit_temp) (struct thermal_zone_device *, int *);
+	int (*set_emul_temp) (struct thermal_zone_device *, int);
 	int (*get_trend) (struct thermal_zone_device *, int,
 			  enum thermal_trend *);
 	int (*notify) (struct thermal_zone_device *, int,
@@ -272,9 +268,9 @@ struct thermal_genl_event {
  *		   temperature.
  */
 struct thermal_zone_of_device_ops {
-	int (*get_temp)(void *, long *);
+	int (*get_temp)(void *, int *);
 	int (*get_trend)(void *, long *);
-	int (*set_emul_temp)(void *, unsigned long);
+	int (*set_emul_temp)(void *, int);
 };
 
 /**
@@ -335,7 +331,7 @@ thermal_of_cooling_device_register(struct device_node *np, char *, void *,
 				   const struct thermal_cooling_device_ops *);
 void thermal_cooling_device_unregister(struct thermal_cooling_device *);
 struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name);
-int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp);
+int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
 
 int get_tz_trend(struct thermal_zone_device *, int);
 struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
@@ -378,7 +374,7 @@ static inline struct thermal_zone_device *thermal_zone_get_zone_by_name(
 		const char *name)
 { return ERR_PTR(-ENODEV); }
 static inline int thermal_zone_get_temp(
-		struct thermal_zone_device *tz, unsigned long *temp)
+		struct thermal_zone_device *tz, int *temp)
 { return -ENODEV; }
 static inline int get_tz_trend(struct thermal_zone_device *tz, int trip)
 { return -ENODEV; }
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 01/15] thermal: consistently use int for temperatures
  2015-05-13  8:52 ` [PATCH 01/15] thermal: consistently use int for temperatures Sascha Hauer
@ 2015-05-20  7:12   ` Mikko Perttunen
  2015-05-20  8:34     ` Sascha Hauer
  0 siblings, 1 reply; 34+ messages in thread
From: Mikko Perttunen @ 2015-05-20  7:12 UTC (permalink / raw)
  To: Sascha Hauer, linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	kernel, linux-mediatek, linux-arm-kernel, Brian Norris

On 05/13/15 11:52, Sascha Hauer wrote:
> The thermal code uses int, long and unsigned long for temperatures
> in different places. Using an unsigned type limits the thermal framework
> to positive temperatures without need. 'long' is 64bit on several
> architectures which is not needed. Consistently use a plain 'int'
> for temperatures.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> ...

It looks like some longs remain in db8500_thermal.c, int340x_thermal/*,
intel_soc_dts_thermal.c, kirkwood_thermal.c, rockchip_thermal.c and 
samsung/exynos_tmu.c. (found by 'grep -R long drivers/thermal/*)

Cheers,
Mikko.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 01/15] thermal: consistently use int for temperatures
  2015-05-20  7:12   ` Mikko Perttunen
@ 2015-05-20  8:34     ` Sascha Hauer
  2015-05-20  8:42       ` Mikko Perttunen
  0 siblings, 1 reply; 34+ messages in thread
From: Sascha Hauer @ 2015-05-20  8:34 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: linux-pm, Zhang Rui, Eduardo Valentin, linux-kernel,
	Stephen Warren, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris

On Wed, May 20, 2015 at 10:12:44AM +0300, Mikko Perttunen wrote:
> On 05/13/15 11:52, Sascha Hauer wrote:
> >The thermal code uses int, long and unsigned long for temperatures
> >in different places. Using an unsigned type limits the thermal framework
> >to positive temperatures without need. 'long' is 64bit on several
> >architectures which is not needed. Consistently use a plain 'int'
> >for temperatures.
> >
> >Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> >---
> >...
> 
> It looks like some longs remain in db8500_thermal.c, int340x_thermal/*,
> intel_soc_dts_thermal.c, kirkwood_thermal.c, rockchip_thermal.c and
> samsung/exynos_tmu.c. (found by 'grep -R long drivers/thermal/*)

I looked over it again and found the following. Sorry, I really thought
I had compile tested all drivers. I found nothing in db8500_thermal.c,
intel_soc_dts_thermal.c and rockchip_thermal.c though. There are some
(unsigned)longs used as temperatures in these files, but I haven't
changed them on purpose to not make the patch bigger as necessary. I
only changed the places where (unsigned)longs are used as pointers.
Do you think it's necessary to change all temperatures, even the ones
only used internally in drivers?

Sascha

-----------------------8<------------------

>From 4cf8fe3b3ef1bd9db6090305ea2b9995f0dbffa4 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Wed, 20 May 2015 10:28:39 +0200
Subject: [PATCH] fixup! thermal: consistently use int for temperatures

---
 drivers/thermal/int340x_thermal/int3400_thermal.c          | 4 ++--
 drivers/thermal/int340x_thermal/processor_thermal_device.c | 4 ++--
 drivers/thermal/kirkwood_thermal.c                         | 2 +-
 drivers/thermal/samsung/exynos_tmu.c                       | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/int340x_thermal/int3400_thermal.c b/drivers/thermal/int340x_thermal/int3400_thermal.c
index 96bdf8a..5836e55 100644
--- a/drivers/thermal/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/int340x_thermal/int3400_thermal.c
@@ -186,7 +186,7 @@ static int int3400_thermal_run_osc(acpi_handle handle,
 }
 
 static int int3400_thermal_get_temp(struct thermal_zone_device *thermal,
-			unsigned long *temp)
+			int *temp)
 {
 	*temp = 20 * 1000; /* faked temp sensor with 20C */
 	return 0;
@@ -231,7 +231,7 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
 	return result;
 }
 
-static const struct thermal_zone_device_ops int3400_thermal_ops = {
+static struct thermal_zone_device_ops int3400_thermal_ops = {
 	.get_temp = int3400_thermal_get_temp,
 };
 
diff --git a/drivers/thermal/int340x_thermal/processor_thermal_device.c b/drivers/thermal/int340x_thermal/processor_thermal_device.c
index 5e8d8e9..c8afd34 100644
--- a/drivers/thermal/int340x_thermal/processor_thermal_device.c
+++ b/drivers/thermal/int340x_thermal/processor_thermal_device.c
@@ -139,7 +139,7 @@ static int get_tjmax(void)
 	return -EINVAL;
 }
 
-static int read_temp_msr(unsigned long *temp)
+static int read_temp_msr(int *temp)
 {
 	int cpu;
 	u32 eax, edx;
@@ -171,7 +171,7 @@ err_ret:
 }
 
 static int proc_thermal_get_zone_temp(struct thermal_zone_device *zone,
-					 unsigned long *temp)
+					 int *temp)
 {
 	int ret;
 
diff --git a/drivers/thermal/kirkwood_thermal.c b/drivers/thermal/kirkwood_thermal.c
index abba3e2..5bcdbd6 100644
--- a/drivers/thermal/kirkwood_thermal.c
+++ b/drivers/thermal/kirkwood_thermal.c
@@ -33,7 +33,7 @@ struct kirkwood_thermal_priv {
 };
 
 static int kirkwood_get_temp(struct thermal_zone_device *thermal,
-			  unsigned long *temp)
+			  int *temp)
 {
 	unsigned long reg;
 	struct kirkwood_thermal_priv *priv = thermal->devdata;
diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 29eaf4d..ed55fd9 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -812,7 +812,7 @@ out:
 #else
 #define exynos4412_tmu_set_emulation NULL
 #define exynos5440_tmu_set_emulation NULL
-static int exynos_tmu_set_emulation(void *drv_data,	unsigned long temp)
+static int exynos_tmu_set_emulation(void *drv_data, int temp)
 	{ return -EINVAL; }
 #endif /* CONFIG_THERMAL_EMULATION */
 
-- 
2.1.4

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 01/15] thermal: consistently use int for temperatures
  2015-05-20  8:34     ` Sascha Hauer
@ 2015-05-20  8:42       ` Mikko Perttunen
  0 siblings, 0 replies; 34+ messages in thread
From: Mikko Perttunen @ 2015-05-20  8:42 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-pm, Zhang Rui, Eduardo Valentin, linux-kernel,
	Stephen Warren, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris

On 05/20/15 11:34, Sascha Hauer wrote:
> On Wed, May 20, 2015 at 10:12:44AM +0300, Mikko Perttunen wrote:
>> On 05/13/15 11:52, Sascha Hauer wrote:
>>> The thermal code uses int, long and unsigned long for temperatures
>>> in different places. Using an unsigned type limits the thermal framework
>>> to positive temperatures without need. 'long' is 64bit on several
>>> architectures which is not needed. Consistently use a plain 'int'
>>> for temperatures.
>>>
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> ---
>>> ...
>>
>> It looks like some longs remain in db8500_thermal.c, int340x_thermal/*,
>> intel_soc_dts_thermal.c, kirkwood_thermal.c, rockchip_thermal.c and
>> samsung/exynos_tmu.c. (found by 'grep -R long drivers/thermal/*)
>
> I looked over it again and found the following. Sorry, I really thought
> I had compile tested all drivers. I found nothing in db8500_thermal.c,
> intel_soc_dts_thermal.c and rockchip_thermal.c though. There are some
> (unsigned)longs used as temperatures in these files, but I haven't
> changed them on purpose to not make the patch bigger as necessary. I
> only changed the places where (unsigned)longs are used as pointers.
> Do you think it's necessary to change all temperatures, even the ones
> only used internally in drivers?
>
> Sascha

I guess changing just the framework interface would be fine for now,
though I don't think there are that many internal uses remaining either.

Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>

Cheers,
Mikko.

>
> -----------------------8<------------------
>
>>From 4cf8fe3b3ef1bd9db6090305ea2b9995f0dbffa4 Mon Sep 17 00:00:00 2001
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Date: Wed, 20 May 2015 10:28:39 +0200
> Subject: [PATCH] fixup! thermal: consistently use int for temperatures
>
> ---
>   drivers/thermal/int340x_thermal/int3400_thermal.c          | 4 ++--
>   drivers/thermal/int340x_thermal/processor_thermal_device.c | 4 ++--
>   drivers/thermal/kirkwood_thermal.c                         | 2 +-
>   drivers/thermal/samsung/exynos_tmu.c                       | 2 +-
>   4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/thermal/int340x_thermal/int3400_thermal.c b/drivers/thermal/int340x_thermal/int3400_thermal.c
> index 96bdf8a..5836e55 100644
> --- a/drivers/thermal/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/int340x_thermal/int3400_thermal.c
> @@ -186,7 +186,7 @@ static int int3400_thermal_run_osc(acpi_handle handle,
>   }
>
>   static int int3400_thermal_get_temp(struct thermal_zone_device *thermal,
> -			unsigned long *temp)
> +			int *temp)
>   {
>   	*temp = 20 * 1000; /* faked temp sensor with 20C */
>   	return 0;
> @@ -231,7 +231,7 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
>   	return result;
>   }
>
> -static const struct thermal_zone_device_ops int3400_thermal_ops = {
> +static struct thermal_zone_device_ops int3400_thermal_ops = {
>   	.get_temp = int3400_thermal_get_temp,
>   };
>
> diff --git a/drivers/thermal/int340x_thermal/processor_thermal_device.c b/drivers/thermal/int340x_thermal/processor_thermal_device.c
> index 5e8d8e9..c8afd34 100644
> --- a/drivers/thermal/int340x_thermal/processor_thermal_device.c
> +++ b/drivers/thermal/int340x_thermal/processor_thermal_device.c
> @@ -139,7 +139,7 @@ static int get_tjmax(void)
>   	return -EINVAL;
>   }
>
> -static int read_temp_msr(unsigned long *temp)
> +static int read_temp_msr(int *temp)
>   {
>   	int cpu;
>   	u32 eax, edx;
> @@ -171,7 +171,7 @@ err_ret:
>   }
>
>   static int proc_thermal_get_zone_temp(struct thermal_zone_device *zone,
> -					 unsigned long *temp)
> +					 int *temp)
>   {
>   	int ret;
>
> diff --git a/drivers/thermal/kirkwood_thermal.c b/drivers/thermal/kirkwood_thermal.c
> index abba3e2..5bcdbd6 100644
> --- a/drivers/thermal/kirkwood_thermal.c
> +++ b/drivers/thermal/kirkwood_thermal.c
> @@ -33,7 +33,7 @@ struct kirkwood_thermal_priv {
>   };
>
>   static int kirkwood_get_temp(struct thermal_zone_device *thermal,
> -			  unsigned long *temp)
> +			  int *temp)
>   {
>   	unsigned long reg;
>   	struct kirkwood_thermal_priv *priv = thermal->devdata;
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 29eaf4d..ed55fd9 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -812,7 +812,7 @@ out:
>   #else
>   #define exynos4412_tmu_set_emulation NULL
>   #define exynos5440_tmu_set_emulation NULL
> -static int exynos_tmu_set_emulation(void *drv_data,	unsigned long temp)
> +static int exynos_tmu_set_emulation(void *drv_data, int temp)
>   	{ return -EINVAL; }
>   #endif /* CONFIG_THERMAL_EMULATION */
>
>


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 02/15] thermal: trivial: fix typo in comment
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
  2015-05-13  8:52 ` [PATCH 01/15] thermal: consistently use int for temperatures Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  2015-05-13  8:52 ` [PATCH 03/15] thermal: remove useless call to thermal_zone_device_set_polling Sascha Hauer
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Acked-by: Eduardo Valentin <edubezval@gmail.com>
---
 drivers/thermal/thermal_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 62cc82a..244784f 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -402,7 +402,7 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
 }
 
 /**
- * thermal_zone_get_temp() - returns its the temperature of thermal zone
+ * thermal_zone_get_temp() - returns the temperature of a thermal zone
  * @tz: a valid pointer to a struct thermal_zone_device
  * @temp: a valid pointer to where to store the resulting temperature.
  *
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 03/15] thermal: remove useless call to thermal_zone_device_set_polling
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
  2015-05-13  8:52 ` [PATCH 01/15] thermal: consistently use int for temperatures Sascha Hauer
  2015-05-13  8:52 ` [PATCH 02/15] thermal: trivial: fix typo in comment Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  2015-05-13  8:52 ` [PATCH 04/15] thermal: Use IS_ENABLED instead of #ifdef Sascha Hauer
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

When the thermal zone has no get_temp callback then thermal_zone_device_register()
calls thermal_zone_device_set_polling() with a polling delay of 0. This
only cancels the poll_queue. Since the poll_queue hasn't been scheduled this
is a no-op. Remove it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Acked-by: Eduardo Valentin <edubezval@gmail.com>
---
 drivers/thermal/thermal_core.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 244784f..1b68d20 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1571,9 +1571,6 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
 
 	INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check);
 
-	if (!tz->ops->get_temp)
-		thermal_zone_device_set_polling(tz, 0);
-
 	thermal_zone_device_update(tz);
 
 	return tz;
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 04/15] thermal: Use IS_ENABLED instead of #ifdef
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
                   ` (2 preceding siblings ...)
  2015-05-13  8:52 ` [PATCH 03/15] thermal: remove useless call to thermal_zone_device_set_polling Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  2015-05-13  8:52 ` [PATCH 05/15] thermal: Add comment explaining test for critical temperature Sascha Hauer
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

Use IS_ENABLED(CONFIG_THERMAL_EMULATION) to make the code more readable
and to get rid of the addtional #ifdef around the variable definitions
in thermal_zone_get_temp().

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/thermal/thermal_core.c | 45 ++++++++++++++++++------------------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 1b68d20..3e0fe55 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -414,11 +414,9 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
 int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
 {
 	int ret = -EINVAL;
-#ifdef CONFIG_THERMAL_EMULATION
 	int count;
 	int crit_temp = INT_MAX;
 	enum thermal_trip_type type;
-#endif
 
 	if (!tz || IS_ERR(tz) || !tz->ops->get_temp)
 		goto exit;
@@ -426,25 +424,21 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
 	mutex_lock(&tz->lock);
 
 	ret = tz->ops->get_temp(tz, temp);
-#ifdef CONFIG_THERMAL_EMULATION
-	if (!tz->emul_temperature)
-		goto skip_emul;
-
-	for (count = 0; count < tz->trips; count++) {
-		ret = tz->ops->get_trip_type(tz, count, &type);
-		if (!ret && type == THERMAL_TRIP_CRITICAL) {
-			ret = tz->ops->get_trip_temp(tz, count, &crit_temp);
-			break;
-		}
-	}
 
-	if (ret)
-		goto skip_emul;
+	if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) {
+		for (count = 0; count < tz->trips; count++) {
+			ret = tz->ops->get_trip_type(tz, count, &type);
+			if (!ret && type == THERMAL_TRIP_CRITICAL) {
+				ret = tz->ops->get_trip_temp(tz, count,
+						&crit_temp);
+				break;
+			}
+		}
 
-	if (*temp < crit_temp)
-		*temp = tz->emul_temperature;
-skip_emul:
-#endif
+		if (!ret && *temp < crit_temp)
+			*temp = tz->emul_temperature;
+	}
+ 
 	mutex_unlock(&tz->lock);
 exit:
 	return ret;
@@ -780,7 +774,6 @@ policy_show(struct device *dev, struct device_attribute *devattr, char *buf)
 	return sprintf(buf, "%s\n", tz->governor->name);
 }
 
-#ifdef CONFIG_THERMAL_EMULATION
 static ssize_t
 emul_temp_store(struct device *dev, struct device_attribute *attr,
 		     const char *buf, size_t count)
@@ -806,7 +799,6 @@ emul_temp_store(struct device *dev, struct device_attribute *attr,
 	return ret ? ret : count;
 }
 static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store);
-#endif/*CONFIG_THERMAL_EMULATION*/
 
 static DEVICE_ATTR(type, 0444, type_show, NULL);
 static DEVICE_ATTR(temp, 0444, temp_show, NULL);
@@ -1536,11 +1528,12 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
 			goto unregister;
 	}
 
-#ifdef CONFIG_THERMAL_EMULATION
-	result = device_create_file(&tz->device, &dev_attr_emul_temp);
-	if (result)
-		goto unregister;
-#endif
+	if (IS_ENABLED(CONFIG_THERMAL_EMULATION)) {
+		result = device_create_file(&tz->device, &dev_attr_emul_temp);
+		if (result)
+			goto unregister;
+	}
+
 	/* Create policy attribute */
 	result = device_create_file(&tz->device, &dev_attr_policy);
 	if (result)
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 05/15] thermal: Add comment explaining test for critical temperature
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
                   ` (3 preceding siblings ...)
  2015-05-13  8:52 ` [PATCH 04/15] thermal: Use IS_ENABLED instead of #ifdef Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  2015-05-20  7:18   ` Mikko Perttunen
  2015-05-13  8:52 ` [PATCH 06/15] thermal: inline only once used function Sascha Hauer
                   ` (9 subsequent siblings)
  14 siblings, 1 reply; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

The code testing if a temperature should be emulated or not is
not obvious. Add a comment explaining why this test is done.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/thermal/thermal_core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 3e0fe55..e204deb 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -435,6 +435,11 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
 			}
 		}
 
+		/*
+		 * Only allow emulating a temperature when the real temperature
+		 * is below the critical temperature so that the emulation code
+		 * cannot hide critical conditions.
+		 */
 		if (!ret && *temp < crit_temp)
 			*temp = tz->emul_temperature;
 	}
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 05/15] thermal: Add comment explaining test for critical temperature
  2015-05-13  8:52 ` [PATCH 05/15] thermal: Add comment explaining test for critical temperature Sascha Hauer
@ 2015-05-20  7:18   ` Mikko Perttunen
  0 siblings, 0 replies; 34+ messages in thread
From: Mikko Perttunen @ 2015-05-20  7:18 UTC (permalink / raw)
  To: Sascha Hauer, linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	kernel, linux-mediatek, linux-arm-kernel, Brian Norris

On 05/13/15 11:52, Sascha Hauer wrote:
> The code testing if a temperature should be emulated or not is
> not obvious. Add a comment explaining why this test is done.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>   drivers/thermal/thermal_core.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 3e0fe55..e204deb 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -435,6 +435,11 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
>   			}
>   		}
>
> +		/*
> +		 * Only allow emulating a temperature when the real temperature
> +		 * is below the critical temperature so that the emulation code
> +		 * cannot hide critical conditions.
> +		 */
>   		if (!ret && *temp < crit_temp)
>   			*temp = tz->emul_temperature;
>   	}
>

Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 06/15] thermal: inline only once used function
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
                   ` (4 preceding siblings ...)
  2015-05-13  8:52 ` [PATCH 05/15] thermal: Add comment explaining test for critical temperature Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  2015-05-20  7:28   ` Mikko Perttunen
  2015-05-13  8:52 ` [PATCH 07/15] thermal: streamline get_trend callbacks Sascha Hauer
                   ` (8 subsequent siblings)
  14 siblings, 1 reply; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

Inline update_temperature into its only caller to make the code
more readable.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/thermal/thermal_core.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index e204deb..19da022 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -450,9 +450,12 @@ exit:
 }
 EXPORT_SYMBOL_GPL(thermal_zone_get_temp);
 
-static void update_temperature(struct thermal_zone_device *tz)
+void thermal_zone_device_update(struct thermal_zone_device *tz)
 {
-	int temp, ret;
+	int temp, ret, count;
+
+	if (!tz->ops->get_temp)
+		return;
 
 	ret = thermal_zone_get_temp(tz, &temp);
 	if (ret) {
@@ -471,16 +474,6 @@ static void update_temperature(struct thermal_zone_device *tz)
 	trace_thermal_temperature(tz);
 	dev_dbg(&tz->device, "last_temperature=%d, current_temperature=%d\n",
 				tz->last_temperature, tz->temperature);
-}
-
-void thermal_zone_device_update(struct thermal_zone_device *tz)
-{
-	int count;
-
-	if (!tz->ops->get_temp)
-		return;
-
-	update_temperature(tz);
 
 	for (count = 0; count < tz->trips; count++)
 		handle_thermal_trip(tz, count);
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 06/15] thermal: inline only once used function
  2015-05-13  8:52 ` [PATCH 06/15] thermal: inline only once used function Sascha Hauer
@ 2015-05-20  7:28   ` Mikko Perttunen
  0 siblings, 0 replies; 34+ messages in thread
From: Mikko Perttunen @ 2015-05-20  7:28 UTC (permalink / raw)
  To: Sascha Hauer, linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	kernel, linux-mediatek, linux-arm-kernel, Brian Norris

On 05/13/15 11:52, Sascha Hauer wrote:
> Inline update_temperature into its only caller to make the code
> more readable.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>   drivers/thermal/thermal_core.c | 17 +++++------------
>   1 file changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index e204deb..19da022 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -450,9 +450,12 @@ exit:
>   }
>   EXPORT_SYMBOL_GPL(thermal_zone_get_temp);
>
> -static void update_temperature(struct thermal_zone_device *tz)
> +void thermal_zone_device_update(struct thermal_zone_device *tz)
>   {
> -	int temp, ret;
> +	int temp, ret, count;
> +
> +	if (!tz->ops->get_temp)
> +		return;
>
>   	ret = thermal_zone_get_temp(tz, &temp);
>   	if (ret) {
> @@ -471,16 +474,6 @@ static void update_temperature(struct thermal_zone_device *tz)
>   	trace_thermal_temperature(tz);
>   	dev_dbg(&tz->device, "last_temperature=%d, current_temperature=%d\n",
>   				tz->last_temperature, tz->temperature);
> -}
> -
> -void thermal_zone_device_update(struct thermal_zone_device *tz)
> -{
> -	int count;
> -
> -	if (!tz->ops->get_temp)
> -		return;
> -
> -	update_temperature(tz);
>
>   	for (count = 0; count < tz->trips; count++)
>   		handle_thermal_trip(tz, count);
>

This is a slight change in that if get_temp fails, governors were 
updated before but aren't now. But I don't know why that would matter.

Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 07/15] thermal: streamline get_trend callbacks
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
                   ` (5 preceding siblings ...)
  2015-05-13  8:52 ` [PATCH 06/15] thermal: inline only once used function Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  2015-05-13  8:52 ` [PATCH 08/15] thermal: Allow sensor ops to fail with -ENOSYS Sascha Hauer
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

The .get_trend callback in struct thermal_zone_device_ops has the prototype:

	int (*get_trend) (struct thermal_zone_device *, int,
			  enum thermal_trend *);

whereas the .get_trend callback in struct thermal_zone_of_device_ops has:

	int (*get_trend)(void *, long *);

Streamline both prototypes and add the trip argument to the OF callback
aswell and use enum thermal_trend * instead of an integer pointer.

While the OF prototype may be the better one, this should be decided at
framework level and not on OF level.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/thermal/of-thermal.c                       | 11 +---------
 drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 25 +++++++---------------
 include/linux/thermal.h                            |  2 +-
 3 files changed, 10 insertions(+), 28 deletions(-)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index 03839df..c84404d 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -187,24 +187,15 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip,
 				enum thermal_trend *trend)
 {
 	struct __thermal_zone *data = tz->devdata;
-	long dev_trend;
 	int r;
 
 	if (!data->ops->get_trend)
 		return -EINVAL;
 
-	r = data->ops->get_trend(data->sensor_data, &dev_trend);
+	r = data->ops->get_trend(data->sensor_data, trip, trend);
 	if (r)
 		return r;
 
-	/* TODO: These intervals might have some thresholds, but in core code */
-	if (dev_trend > 0)
-		*trend = THERMAL_TREND_RAISING;
-	else if (dev_trend < 0)
-		*trend = THERMAL_TREND_DROPPING;
-	else
-		*trend = THERMAL_TREND_STABLE;
-
 	return 0;
 }
 
diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index d3a42bf..ade78eb 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -238,7 +238,7 @@ static int ti_thermal_get_trip_temp(struct thermal_zone_device *thermal,
 	return 0;
 }
 
-static int __ti_thermal_get_trend(void *p, long *trend)
+static int __ti_thermal_get_trend(void *p, int trip, enum thermal_trend *trend)
 {
 	struct ti_thermal_data *data = p;
 	struct ti_bandgap *bgp;
@@ -251,22 +251,6 @@ static int __ti_thermal_get_trend(void *p, long *trend)
 	if (ret)
 		return ret;
 
-	*trend = tr;
-
-	return 0;
-}
-
-/* Get the temperature trend callback functions for thermal zone */
-static int ti_thermal_get_trend(struct thermal_zone_device *thermal,
-				int trip, enum thermal_trend *trend)
-{
-	int ret;
-	long tr;
-
-	ret = __ti_thermal_get_trend(thermal->devdata, &tr);
-	if (ret)
-		return ret;
-
 	if (tr > 0)
 		*trend = THERMAL_TREND_RAISING;
 	else if (tr < 0)
@@ -277,6 +261,13 @@ static int ti_thermal_get_trend(struct thermal_zone_device *thermal,
 	return 0;
 }
 
+/* Get the temperature trend callback functions for thermal zone */
+static int ti_thermal_get_trend(struct thermal_zone_device *thermal,
+				int trip, enum thermal_trend *trend)
+{
+	return __ti_thermal_get_trend(thermal->devdata, trip, trend);
+}
+
 /* Get critical temperature callback functions for thermal zone */
 static int ti_thermal_get_crit_temp(struct thermal_zone_device *thermal,
 				    int *temp)
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index e9f2863..5c6a589 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -269,7 +269,7 @@ struct thermal_genl_event {
  */
 struct thermal_zone_of_device_ops {
 	int (*get_temp)(void *, int *);
-	int (*get_trend)(void *, long *);
+	int (*get_trend)(void *, int, enum thermal_trend *);
 	int (*set_emul_temp)(void *, int);
 };
 
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 08/15] thermal: Allow sensor ops to fail with -ENOSYS
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
                   ` (6 preceding siblings ...)
  2015-05-13  8:52 ` [PATCH 07/15] thermal: streamline get_trend callbacks Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  2015-05-13  8:52 ` [PATCH 09/15] thermal: of: always set sensor related callbacks Sascha Hauer
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

The thermal core uses the existence of the .get_temp, .get_trend and
.set_emul_temp to detect whether this operation exists and should be
used or whether it should be emulated in software. This makes problems
for of-thermal which has to modify the struct thermal_zone_device_ops
during runtime whenever a sensor is registered or unregistered.

Let the core test for -ENOSYS from these callbacks and treat it like
if the callbacks were not present.

This allows of-thermal to always set the sensor related callbacks and
to make struct thermal_zone_device_ops const again.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/thermal/thermal_core.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 19da022..3d8f9f9 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -413,13 +413,16 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
  */
 int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
 {
-	int ret = -EINVAL;
+	int ret;
 	int count;
 	int crit_temp = INT_MAX;
 	enum thermal_trip_type type;
 
-	if (!tz || IS_ERR(tz) || !tz->ops->get_temp)
-		goto exit;
+	if (!tz || IS_ERR(tz))
+		return -EINVAL;
+
+	if (!tz->ops->get_temp)
+		return -ENOSYS;
 
 	mutex_lock(&tz->lock);
 
@@ -445,7 +448,7 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
 	}
  
 	mutex_unlock(&tz->lock);
-exit:
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(thermal_zone_get_temp);
@@ -454,10 +457,11 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
 {
 	int temp, ret, count;
 
-	if (!tz->ops->get_temp)
+	ret = thermal_zone_get_temp(tz, &temp);
+
+	if (ret == -ENOSYS)
 		return;
 
-	ret = thermal_zone_get_temp(tz, &temp);
 	if (ret) {
 		if (ret != -EAGAIN)
 			dev_warn(&tz->device,
@@ -783,10 +787,16 @@ emul_temp_store(struct device *dev, struct device_attribute *attr,
 	if (kstrtoul(buf, 10, &temperature))
 		return -EINVAL;
 
-	if (!tz->ops->set_emul_temp) {
+	if (tz->ops->set_emul_temp)
+		ret = tz->ops->set_emul_temp(tz, temperature);
+	else
+		ret = -ENOSYS;
+
+	if (ret == -ENOSYS) {
 		mutex_lock(&tz->lock);
 		tz->emul_temperature = temperature;
 		mutex_unlock(&tz->lock);
+		ret = 0;
 	} else {
 		ret = tz->ops->set_emul_temp(tz, temperature);
 	}
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 09/15] thermal: of: always set sensor related callbacks
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
                   ` (7 preceding siblings ...)
  2015-05-13  8:52 ` [PATCH 08/15] thermal: Allow sensor ops to fail with -ENOSYS Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  2015-05-13  8:52 ` [PATCH 10/15] thermal: Make struct thermal_zone_device_ops const Sascha Hauer
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

Now that the thermal core treats -ENOSYS like the callbacks were
not present at all we no longer have to overwrite the ops during
runtime but instead can always set them and return -ENOSYS if no
sensor is registered.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/thermal/of-thermal.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index c84404d..b9c35bd 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -91,7 +91,7 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz,
 {
 	struct __thermal_zone *data = tz->devdata;
 
-	if (!data->ops->get_temp)
+	if (!data->ops)
 		return -EINVAL;
 
 	return data->ops->get_temp(data->sensor_data, temp);
@@ -178,7 +178,7 @@ static int of_thermal_set_emul_temp(struct thermal_zone_device *tz,
 	struct __thermal_zone *data = tz->devdata;
 
 	if (!data->ops || !data->ops->set_emul_temp)
-		return -EINVAL;
+		return -ENOSYS;
 
 	return data->ops->set_emul_temp(data->sensor_data, temp);
 }
@@ -189,8 +189,8 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip,
 	struct __thermal_zone *data = tz->devdata;
 	int r;
 
-	if (!data->ops->get_trend)
-		return -EINVAL;
+	if (!data->ops || !data->ops->get_trend)
+		return -ENOSYS;
 
 	r = data->ops->get_trend(data->sensor_data, trip, trend);
 	if (r)
@@ -366,6 +366,10 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
 }
 
 static struct thermal_zone_device_ops of_thermal_ops = {
+	.get_temp = of_thermal_get_temp,
+	.get_trend = of_thermal_get_trend,
+	.set_emul_temp = of_thermal_set_emul_temp,
+
 	.get_mode = of_thermal_get_mode,
 	.set_mode = of_thermal_set_mode,
 
@@ -399,13 +403,13 @@ thermal_zone_of_add_sensor(struct device_node *zone,
 	if (!ops)
 		return ERR_PTR(-EINVAL);
 
+	if (!ops->get_temp)
+		return ERR_PTR(-EINVAL);
+
 	mutex_lock(&tzd->lock);
 	tz->ops = ops;
 	tz->sensor_data = data;
 
-	tzd->ops->get_temp = of_thermal_get_temp;
-	tzd->ops->get_trend = of_thermal_get_trend;
-	tzd->ops->set_emul_temp = of_thermal_set_emul_temp;
 	mutex_unlock(&tzd->lock);
 
 	return tzd;
@@ -535,9 +539,6 @@ void thermal_zone_of_sensor_unregister(struct device *dev,
 		return;
 
 	mutex_lock(&tzd->lock);
-	tzd->ops->get_temp = NULL;
-	tzd->ops->get_trend = NULL;
-	tzd->ops->set_emul_temp = NULL;
 
 	tz->ops = NULL;
 	tz->sensor_data = NULL;
@@ -845,7 +846,6 @@ int __init of_parse_thermal_zones(void)
 {
 	struct device_node *np, *child;
 	struct __thermal_zone *tz;
-	struct thermal_zone_device_ops *ops;
 
 	np = of_find_node_by_name(NULL, "thermal-zones");
 	if (!np) {
@@ -869,29 +869,22 @@ int __init of_parse_thermal_zones(void)
 			continue;
 		}
 
-		ops = kmemdup(&of_thermal_ops, sizeof(*ops), GFP_KERNEL);
-		if (!ops)
-			goto exit_free;
-
 		tzp = kzalloc(sizeof(*tzp), GFP_KERNEL);
-		if (!tzp) {
-			kfree(ops);
+		if (!tzp)
 			goto exit_free;
-		}
 
 		/* No hwmon because there might be hwmon drivers registering */
 		tzp->no_hwmon = true;
 
 		zone = thermal_zone_device_register(child->name, tz->ntrips,
 						    0, tz,
-						    ops, tzp,
+						    &of_thermal_ops, tzp,
 						    tz->passive_delay,
 						    tz->polling_delay);
 		if (IS_ERR(zone)) {
 			pr_err("Failed to build %s zone %ld\n", child->name,
 			       PTR_ERR(zone));
 			kfree(tzp);
-			kfree(ops);
 			of_thermal_free_zone(tz);
 			/* attempting to build remaining zones still */
 		}
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 10/15] thermal: Make struct thermal_zone_device_ops const
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
                   ` (8 preceding siblings ...)
  2015-05-13  8:52 ` [PATCH 09/15] thermal: of: always set sensor related callbacks Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  2015-05-13  8:52 ` [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points Sascha Hauer
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

Now that the of thermal support no longer changes the
thermal_zone_device_ops it can be const again.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 Documentation/thermal/sysfs-api.txt                    | 2 +-
 drivers/acpi/thermal.c                                 | 2 +-
 drivers/platform/x86/acerhdf.c                         | 2 +-
 drivers/platform/x86/intel_mid_thermal.c               | 2 +-
 drivers/power/power_supply_core.c                      | 2 +-
 drivers/thermal/armada_thermal.c                       | 2 +-
 drivers/thermal/db8500_thermal.c                       | 2 +-
 drivers/thermal/dove_thermal.c                         | 2 +-
 drivers/thermal/imx_thermal.c                          | 2 +-
 drivers/thermal/int340x_thermal/int3400_thermal.c      | 2 +-
 drivers/thermal/int340x_thermal/int340x_thermal_zone.c | 2 +-
 drivers/thermal/intel_soc_dts_thermal.c                | 2 +-
 drivers/thermal/kirkwood_thermal.c                     | 2 +-
 drivers/thermal/of-thermal.c                           | 2 +-
 drivers/thermal/rcar_thermal.c                         | 2 +-
 drivers/thermal/spear_thermal.c                        | 2 +-
 drivers/thermal/st/st_thermal.c                        | 2 +-
 drivers/thermal/thermal_core.c                         | 2 +-
 drivers/thermal/ti-soc-thermal/ti-thermal-common.c     | 2 +-
 drivers/thermal/x86_pkg_temp_thermal.c                 | 2 +-
 include/linux/thermal.h                                | 6 +++---
 21 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 87519cb..bb346a2 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -33,7 +33,7 @@ temperature) and throttle appropriate devices.
 1.1 thermal zone device interface
 1.1.1 struct thermal_zone_device *thermal_zone_device_register(char *type,
 		int trips, int mask, void *devdata,
-		struct thermal_zone_device_ops *ops,
+		const struct thermal_zone_device_ops *ops,
 		const struct thermal_zone_params *tzp,
 		int passive_delay, int polling_delay))
 
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 68bff60..6b11462 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -869,7 +869,7 @@ acpi_thermal_unbind_cooling_device(struct thermal_zone_device *thermal,
 	return acpi_thermal_cooling_device_cb(thermal, cdev, false);
 }
 
-static struct thermal_zone_device_ops acpi_thermal_zone_ops = {
+static const struct thermal_zone_device_ops acpi_thermal_zone_ops = {
 	.bind = acpi_thermal_bind_cooling_device,
 	.unbind	= acpi_thermal_unbind_cooling_device,
 	.get_temp = thermal_get_temp,
diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index f2ce63c..bae9ca0 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -482,7 +482,7 @@ static int acerhdf_get_crit_temp(struct thermal_zone_device *thermal,
 }
 
 /* bind callback functions to thermalzone */
-static struct thermal_zone_device_ops acerhdf_dev_ops = {
+static const struct thermal_zone_device_ops acerhdf_dev_ops = {
 	.bind = acerhdf_bind,
 	.unbind = acerhdf_unbind,
 	.get_temp = acerhdf_get_ec_temp,
diff --git a/drivers/platform/x86/intel_mid_thermal.c b/drivers/platform/x86/intel_mid_thermal.c
index 0944e83..069d36b 100644
--- a/drivers/platform/x86/intel_mid_thermal.c
+++ b/drivers/platform/x86/intel_mid_thermal.c
@@ -460,7 +460,7 @@ static int read_curr_temp(struct thermal_zone_device *tzd, unsigned long *temp)
 }
 
 /* Can't be const */
-static struct thermal_zone_device_ops tzd_ops = {
+static const struct thermal_zone_device_ops tzd_ops = {
 	.get_temp = read_curr_temp,
 };
 
diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 87e2fd1..878cb4e 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -509,7 +509,7 @@ static int power_supply_read_temp(struct thermal_zone_device *tzd,
 	return ret;
 }
 
-static struct thermal_zone_device_ops psy_tzd_ops = {
+static const struct thermal_zone_device_ops psy_tzd_ops = {
 	.get_temp = power_supply_read_temp,
 };
 
diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 6feb25d..8d9c689 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -183,7 +183,7 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 	return 0;
 }
 
-static struct thermal_zone_device_ops ops = {
+static const struct thermal_zone_device_ops ops = {
 	.get_temp = armada_get_temp,
 };
 
diff --git a/drivers/thermal/db8500_thermal.c b/drivers/thermal/db8500_thermal.c
index b3eca71..38d6aab9 100644
--- a/drivers/thermal/db8500_thermal.c
+++ b/drivers/thermal/db8500_thermal.c
@@ -210,7 +210,7 @@ static int db8500_sys_get_crit_temp(struct thermal_zone_device *thermal,
 	return -EINVAL;
 }
 
-static struct thermal_zone_device_ops thdev_ops = {
+static const struct thermal_zone_device_ops thdev_ops = {
 	.bind = db8500_cdev_bind,
 	.unbind = db8500_cdev_unbind,
 	.get_temp = db8500_sys_get_temp,
diff --git a/drivers/thermal/dove_thermal.c b/drivers/thermal/dove_thermal.c
index a0bc9de..e8fd627 100644
--- a/drivers/thermal/dove_thermal.c
+++ b/drivers/thermal/dove_thermal.c
@@ -118,7 +118,7 @@ static int dove_get_temp(struct thermal_zone_device *thermal,
 	return 0;
 }
 
-static struct thermal_zone_device_ops ops = {
+static const struct thermal_zone_device_ops ops = {
 	.get_temp = dove_get_temp,
 };
 
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index f1424f0..8a3cfed 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -332,7 +332,7 @@ static int imx_unbind(struct thermal_zone_device *tz,
 	return 0;
 }
 
-static struct thermal_zone_device_ops imx_tz_ops = {
+static const struct thermal_zone_device_ops imx_tz_ops = {
 	.bind = imx_bind,
 	.unbind = imx_unbind,
 	.get_temp = imx_get_temp,
diff --git a/drivers/thermal/int340x_thermal/int3400_thermal.c b/drivers/thermal/int340x_thermal/int3400_thermal.c
index 031018e..96bdf8a 100644
--- a/drivers/thermal/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/int340x_thermal/int3400_thermal.c
@@ -231,7 +231,7 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
 	return result;
 }
 
-static struct thermal_zone_device_ops int3400_thermal_ops = {
+static const struct thermal_zone_device_ops int3400_thermal_ops = {
 	.get_temp = int3400_thermal_get_temp,
 };
 
diff --git a/drivers/thermal/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/int340x_thermal/int340x_thermal_zone.c
index b9b2666..bd9f9e8 100644
--- a/drivers/thermal/int340x_thermal/int340x_thermal_zone.c
+++ b/drivers/thermal/int340x_thermal/int340x_thermal_zone.c
@@ -154,7 +154,7 @@ static int int340x_thermal_get_trip_hyst(struct thermal_zone_device *zone,
 	return 0;
 }
 
-static struct thermal_zone_device_ops int340x_thermal_zone_ops = {
+static const struct thermal_zone_device_ops int340x_thermal_zone_ops = {
 	.get_temp       = int340x_thermal_get_zone_temp,
 	.get_trip_temp	= int340x_thermal_get_trip_temp,
 	.get_trip_type	= int340x_thermal_get_trip_type,
diff --git a/drivers/thermal/intel_soc_dts_thermal.c b/drivers/thermal/intel_soc_dts_thermal.c
index fd550b9..625ba6f 100644
--- a/drivers/thermal/intel_soc_dts_thermal.c
+++ b/drivers/thermal/intel_soc_dts_thermal.c
@@ -270,7 +270,7 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd, int *temp)
 	return 0;
 }
 
-static struct thermal_zone_device_ops tzone_ops = {
+static const struct thermal_zone_device_ops tzone_ops = {
 	.get_temp = sys_get_curr_temp,
 	.get_trip_temp = sys_get_trip_temp,
 	.get_trip_type = sys_get_trip_type,
diff --git a/drivers/thermal/kirkwood_thermal.c b/drivers/thermal/kirkwood_thermal.c
index 11041fe..abba3e2 100644
--- a/drivers/thermal/kirkwood_thermal.c
+++ b/drivers/thermal/kirkwood_thermal.c
@@ -60,7 +60,7 @@ static int kirkwood_get_temp(struct thermal_zone_device *thermal,
 	return 0;
 }
 
-static struct thermal_zone_device_ops ops = {
+static const struct thermal_zone_device_ops ops = {
 	.get_temp = kirkwood_get_temp,
 };
 
diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index b9c35bd..bd3185e 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -365,7 +365,7 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
 	return -EINVAL;
 }
 
-static struct thermal_zone_device_ops of_thermal_ops = {
+static const struct thermal_zone_device_ops of_thermal_ops = {
 	.get_temp = of_thermal_get_temp,
 	.get_trend = of_thermal_get_trend,
 	.set_emul_temp = of_thermal_set_emul_temp,
diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index 5d4ae7d..320ceac 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -270,7 +270,7 @@ static int rcar_thermal_notify(struct thermal_zone_device *zone,
 	return 0;
 }
 
-static struct thermal_zone_device_ops rcar_thermal_zone_ops = {
+static const struct thermal_zone_device_ops rcar_thermal_zone_ops = {
 	.get_temp	= rcar_thermal_get_temp,
 	.get_trip_type	= rcar_thermal_get_trip_type,
 	.get_trip_temp	= rcar_thermal_get_trip_temp,
diff --git a/drivers/thermal/spear_thermal.c b/drivers/thermal/spear_thermal.c
index 534dd91..ec07743 100644
--- a/drivers/thermal/spear_thermal.c
+++ b/drivers/thermal/spear_thermal.c
@@ -50,7 +50,7 @@ static inline int thermal_get_temp(struct thermal_zone_device *thermal,
 	return 0;
 }
 
-static struct thermal_zone_device_ops ops = {
+static const struct thermal_zone_device_ops ops = {
 	.get_temp = thermal_get_temp,
 };
 
diff --git a/drivers/thermal/st/st_thermal.c b/drivers/thermal/st/st_thermal.c
index 44cbba9..0cb5c19 100644
--- a/drivers/thermal/st/st_thermal.c
+++ b/drivers/thermal/st/st_thermal.c
@@ -175,7 +175,7 @@ static int st_thermal_get_trip_temp(struct thermal_zone_device *th,
 	return 0;
 }
 
-static struct thermal_zone_device_ops st_tz_ops = {
+static const struct thermal_zone_device_ops st_tz_ops = {
 	.get_temp	= st_thermal_get_temp,
 	.get_trip_type	= st_thermal_get_trip_type,
 	.get_trip_temp	= st_thermal_get_trip_temp,
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 3d8f9f9..6bbf61f 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1451,7 +1451,7 @@ static void remove_trip_attrs(struct thermal_zone_device *tz)
  */
 struct thermal_zone_device *thermal_zone_device_register(const char *type,
 	int trips, int mask, void *devdata,
-	struct thermal_zone_device_ops *ops,
+	const struct thermal_zone_device_ops *ops,
 	const struct thermal_zone_params *tzp,
 	int passive_delay, int polling_delay)
 {
diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index ade78eb..e9c82fc 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -281,7 +281,7 @@ static const struct thermal_zone_of_device_ops ti_of_thermal_ops = {
 	.get_trend = __ti_thermal_get_trend,
 };
 
-static struct thermal_zone_device_ops ti_thermal_ops = {
+static const struct thermal_zone_device_ops ti_thermal_ops = {
 	.get_temp = ti_thermal_get_temp,
 	.get_trend = ti_thermal_get_trend,
 	.bind = ti_thermal_bind,
diff --git a/drivers/thermal/x86_pkg_temp_thermal.c b/drivers/thermal/x86_pkg_temp_thermal.c
index 054c6d45..bb2b975 100644
--- a/drivers/thermal/x86_pkg_temp_thermal.c
+++ b/drivers/thermal/x86_pkg_temp_thermal.c
@@ -274,7 +274,7 @@ static int sys_get_trip_type(struct thermal_zone_device *thermal,
 }
 
 /* Thermal zone callback registry */
-static struct thermal_zone_device_ops tzone_ops = {
+static const struct thermal_zone_device_ops tzone_ops = {
 	.get_temp = sys_get_curr_temp,
 	.get_trip_temp = sys_get_trip_temp,
 	.get_trip_type = sys_get_trip_type,
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 5c6a589..07bd5e8 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -181,7 +181,7 @@ struct thermal_zone_device {
 	int emul_temperature;
 	int passive;
 	unsigned int forced_passive;
-	struct thermal_zone_device_ops *ops;
+	const struct thermal_zone_device_ops *ops;
 	const struct thermal_zone_params *tzp;
 	struct thermal_governor *governor;
 	struct list_head thermal_instances;
@@ -313,7 +313,7 @@ void thermal_zone_of_sensor_unregister(struct device *dev,
 
 #if IS_ENABLED(CONFIG_THERMAL)
 struct thermal_zone_device *thermal_zone_device_register(const char *, int, int,
-		void *, struct thermal_zone_device_ops *,
+		void *, const struct thermal_zone_device_ops *,
 		const struct thermal_zone_params *, int, int);
 void thermal_zone_device_unregister(struct thermal_zone_device *);
 
@@ -341,7 +341,7 @@ void thermal_notify_framework(struct thermal_zone_device *, int);
 #else
 static inline struct thermal_zone_device *thermal_zone_device_register(
 	const char *type, int trips, int mask, void *devdata,
-	struct thermal_zone_device_ops *ops,
+	const struct thermal_zone_device_ops *ops,
 	const struct thermal_zone_params *tzp,
 	int passive_delay, int polling_delay)
 { return ERR_PTR(-ENODEV); }
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
                   ` (9 preceding siblings ...)
  2015-05-13  8:52 ` [PATCH 10/15] thermal: Make struct thermal_zone_device_ops const Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  2015-05-18  9:06   ` Mikko Perttunen
  2015-05-13  8:52 ` [PATCH 12/15] thermal: of: implement .set_trips for device tree thermal zones Sascha Hauer
                   ` (3 subsequent siblings)
  14 siblings, 1 reply; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

This adds support for hardware-tracked trip points to the device tree
thermal sensor framework.

The framework supports an arbitrary number of trip points. Whenever
the current temperature is updated, the trip points immediately
below and above the current temperature are found. A .set_trips
callback is then called with the temperatures. If there is no trip
point above or below the current temperature, the passed trip
temperature will be -INT_MAX or INT_MAX respectively. In this callback,
the driver should program the hardware such that it is notified
when either of these trip points are triggered. When a trip point
is triggered, the driver should call `thermal_zone_device_update'
for the respective thermal zone. This will cause the trip points
to be updated again.

If .set_trips is not implemented, the framework behaves as before.

This patch is based on an earlier version from Mikko Perttunen
<mikko.perttunen@kapsi.fi>

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/thermal/thermal_core.c | 43 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/thermal.h        |  3 +++
 2 files changed, 46 insertions(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 6bbf61f..3ae1795 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -453,6 +453,45 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
 }
 EXPORT_SYMBOL_GPL(thermal_zone_get_temp);
 
+static void thermal_zone_set_trips(struct thermal_zone_device *tz)
+{
+	int low = -INT_MAX;
+	int high = INT_MAX;
+	int trip_temp, hysteresis;
+	int temp = tz->temperature;
+	int i;
+
+	if (!tz->ops->set_trips)
+		return;
+
+	/* No need to change trip points */
+	if (temp > tz->prev_low_trip && temp < tz->prev_high_trip)
+		return;
+
+	for (i = 0; i < tz->trips; i++) {
+		int trip_low;
+
+		tz->ops->get_trip_temp(tz, i, &trip_temp);
+		tz->ops->get_trip_hyst(tz, i, &hysteresis);
+
+		trip_low = trip_temp - hysteresis;
+
+		if (trip_low < temp && trip_low > low)
+			low = trip_low;
+
+		if (trip_temp > temp && trip_temp < high)
+			high = trip_temp;
+	}
+
+	tz->prev_low_trip = low;
+	tz->prev_high_trip = high;
+
+	dev_dbg(&tz->device, "new temperature boundaries: %d < x < %d\n",
+			low, high);
+
+	tz->ops->set_trips(tz, low, high);
+}
+
 void thermal_zone_device_update(struct thermal_zone_device *tz)
 {
 	int temp, ret, count;
@@ -479,6 +518,8 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
 	dev_dbg(&tz->device, "last_temperature=%d, current_temperature=%d\n",
 				tz->last_temperature, tz->temperature);
 
+	thermal_zone_set_trips(tz);
+
 	for (count = 0; count < tz->trips; count++)
 		handle_thermal_trip(tz, count);
 }
@@ -1494,6 +1535,8 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
 	tz->trips = trips;
 	tz->passive_delay = passive_delay;
 	tz->polling_delay = polling_delay;
+	tz->prev_low_trip = INT_MAX;
+	tz->prev_high_trip = -INT_MAX;
 
 	dev_set_name(&tz->device, "thermal_zone%d", tz->id);
 	result = device_register(&tz->device);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 07bd5e8..aef6e13 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -87,6 +87,7 @@ struct thermal_zone_device_ops {
 	int (*unbind) (struct thermal_zone_device *,
 		       struct thermal_cooling_device *);
 	int (*get_temp) (struct thermal_zone_device *, int *);
+	int (*set_trips) (struct thermal_zone_device *, int, int);
 	int (*get_mode) (struct thermal_zone_device *,
 			 enum thermal_device_mode *);
 	int (*set_mode) (struct thermal_zone_device *,
@@ -180,6 +181,8 @@ struct thermal_zone_device {
 	int last_temperature;
 	int emul_temperature;
 	int passive;
+	int prev_low_trip;
+	int prev_high_trip;
 	unsigned int forced_passive;
 	const struct thermal_zone_device_ops *ops;
 	const struct thermal_zone_params *tzp;
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points
  2015-05-13  8:52 ` [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points Sascha Hauer
@ 2015-05-18  9:06   ` Mikko Perttunen
  2015-05-18 12:09     ` Sascha Hauer
  0 siblings, 1 reply; 34+ messages in thread
From: Mikko Perttunen @ 2015-05-18  9:06 UTC (permalink / raw)
  To: Sascha Hauer, linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	kernel, linux-mediatek, linux-arm-kernel, Brian Norris

Thanks for picking up this patch :)

Sorry for being late with these, but here's a few comments..

On 05/13/2015 11:52 AM, Sascha Hauer wrote:
 > This adds support for hardware-tracked trip points to the device tree
 > thermal sensor framework.
 >
 > The framework supports an arbitrary number of trip points. Whenever
 > the current temperature is updated, the trip points immediately
 > below and above the current temperature are found. A .set_trips
 > callback is then called with the temperatures. If there is no trip
 > point above or below the current temperature, the passed trip
 > temperature will be -INT_MAX or INT_MAX respectively. In this callback,
 > the driver should program the hardware such that it is notified
 > when either of these trip points are triggered. When a trip point
 > is triggered, the driver should call `thermal_zone_device_update'
 > for the respective thermal zone. This will cause the trip points
 > to be updated again.
 >
 > If .set_trips is not implemented, the framework behaves as before.
 >
 > This patch is based on an earlier version from Mikko Perttunen
 > <mikko.perttunen@kapsi.fi>
 >
 > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
 > ---
 >  drivers/thermal/thermal_core.c | 43 
++++++++++++++++++++++++++++++++++++++++++
 >  include/linux/thermal.h        |  3 +++
 >  2 files changed, 46 insertions(+)
 >
 > diff --git a/drivers/thermal/thermal_core.c 
b/drivers/thermal/thermal_core.c
 > index 6bbf61f..3ae1795 100644
 > --- a/drivers/thermal/thermal_core.c
 > +++ b/drivers/thermal/thermal_core.c
 > @@ -453,6 +453,45 @@ int thermal_zone_get_temp(struct 
thermal_zone_device *tz, int *temp)
 >  }
 >  EXPORT_SYMBOL_GPL(thermal_zone_get_temp);
 >
 > +static void thermal_zone_set_trips(struct thermal_zone_device *tz)
 > +{
 > +	int low = -INT_MAX;
 > +	int high = INT_MAX;
 > +	int trip_temp, hysteresis;
 > +	int temp = tz->temperature;
 > +	int i;
 > +
 > +	if (!tz->ops->set_trips)
 > +		return;
 > +
 > +	/* No need to change trip points */
 > +	if (temp > tz->prev_low_trip && temp < tz->prev_high_trip)
 > +		return;
 > +
 > +	for (i = 0; i < tz->trips; i++) {
 > +		int trip_low;
 > +
 > +		tz->ops->get_trip_temp(tz, i, &trip_temp);
 > +		tz->ops->get_trip_hyst(tz, i, &hysteresis);
 > +
 > +		trip_low = trip_temp - hysteresis;
 > +
 > +		if (trip_low < temp && trip_low > low)
 > +			low = trip_low;
 > +
 > +		if (trip_temp > temp && trip_temp < high)
 > +			high = trip_temp;
 > +	}
 > +
 > +	tz->prev_low_trip = low;
 > +	tz->prev_high_trip = high;
 > +
 > +	dev_dbg(&tz->device, "new temperature boundaries: %d < x < %d\n",
 > +			low, high);
 > +
 > +	tz->ops->set_trips(tz, low, high);

This should probably do something if set_trips returns an error code; at 
least an error message, perhaps enable polling? I'm not exactly sure 
what safety features the thermal framework has in general if errors happen..

One interesting thing I noticed was that at least the bang-bang governor 
only acts if the temperature is properly smaller than (trip temp - 
hysteresis). So perhaps we should specify the non-tripping range as 
[low, high)? Or we could change bang-bang.

 > +}
 > +
 >  void thermal_zone_device_update(struct thermal_zone_device *tz)
 >  {
 >  	int temp, ret, count;
 > @@ -479,6 +518,8 @@ void thermal_zone_device_update(struct 
thermal_zone_device *tz)
 >  	dev_dbg(&tz->device, "last_temperature=%d, current_temperature=%d\n",
 >  				tz->last_temperature, tz->temperature);
 >
 > +	thermal_zone_set_trips(tz);
 > +
 >  	for (count = 0; count < tz->trips; count++)
 >  		handle_thermal_trip(tz, count);
 >  }

set_trips should also be called from temp_store and other places that 
modify values that affect the trip points.

 > @@ -1494,6 +1535,8 @@ struct thermal_zone_device 
*thermal_zone_device_register(const char *type,
 >  	tz->trips = trips;
 >  	tz->passive_delay = passive_delay;
 >  	tz->polling_delay = polling_delay;
 > +	tz->prev_low_trip = INT_MAX;
 > +	tz->prev_high_trip = -INT_MAX;
 >
 >  	dev_set_name(&tz->device, "thermal_zone%d", tz->id);
 >  	result = device_register(&tz->device);
 > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
 > index 07bd5e8..aef6e13 100644
 > --- a/include/linux/thermal.h
 > +++ b/include/linux/thermal.h
 > @@ -87,6 +87,7 @@ struct thermal_zone_device_ops {
 >  	int (*unbind) (struct thermal_zone_device *,
 >  		       struct thermal_cooling_device *);
 >  	int (*get_temp) (struct thermal_zone_device *, int *);
 > +	int (*set_trips) (struct thermal_zone_device *, int, int);
 >  	int (*get_mode) (struct thermal_zone_device *,
 >  			 enum thermal_device_mode *);
 >  	int (*set_mode) (struct thermal_zone_device *,
 > @@ -180,6 +181,8 @@ struct thermal_zone_device {
 >  	int last_temperature;
 >  	int emul_temperature;
 >  	int passive;
 > +	int prev_low_trip;
 > +	int prev_high_trip;
 >  	unsigned int forced_passive;
 >  	const struct thermal_zone_device_ops *ops;
 >  	const struct thermal_zone_params *tzp;
 >

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points
  2015-05-18  9:06   ` Mikko Perttunen
@ 2015-05-18 12:09     ` Sascha Hauer
  2015-05-18 18:44       ` Brian Norris
  2015-05-19 13:58       ` Sascha Hauer
  0 siblings, 2 replies; 34+ messages in thread
From: Sascha Hauer @ 2015-05-18 12:09 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: linux-pm, Zhang Rui, Eduardo Valentin, linux-kernel,
	Stephen Warren, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris

Hi Mikko,

On Mon, May 18, 2015 at 12:06:50PM +0300, Mikko Perttunen wrote:
> > +	for (i = 0; i < tz->trips; i++) {
> > +		int trip_low;
> > +
> > +		tz->ops->get_trip_temp(tz, i, &trip_temp);
> > +		tz->ops->get_trip_hyst(tz, i, &hysteresis);
> > +
> > +		trip_low = trip_temp - hysteresis;
> > +
> > +		if (trip_low < temp && trip_low > low)
> > +			low = trip_low;
> > +
> > +		if (trip_temp > temp && trip_temp < high)
> > +			high = trip_temp;
> > +	}
> > +
> > +	tz->prev_low_trip = low;
> > +	tz->prev_high_trip = high;
> > +
> > +	dev_dbg(&tz->device, "new temperature boundaries: %d < x < %d\n",
> > +			low, high);
> > +
> > +	tz->ops->set_trips(tz, low, high);
> 
> This should probably do something if set_trips returns an error
> code; at least an error message, perhaps enable polling? I'm not
> exactly sure what safety features the thermal framework has in
> general if errors happen..

Currently a thermal zone has the passive_delay and polling_delay
variables. If these are nonzero the thermal core will always poll. A
purely interrupt driven thermal zone would set these values to zero.
In this case the thermal core has no basis for polling, so we would
have to make up polling intervals when set_trips fails. Another
possibility would be to interpret the *_delay variables as 'when
set_trips is available, do not poll. When something goes wrong, use
*_delay as polling intervals'

> 
> One interesting thing I noticed was that at least the bang-bang
> governor only acts if the temperature is properly smaller than (trip
> temp - hysteresis). So perhaps we should specify the non-tripping
> range as [low, high)? Or we could change bang-bang.

I wonder how we can protect against such off-by-one errors anyway.
Generally a hardware might operate on raw values rather than directly
in temperature values in °C. This means a driver for this must have
celsius_to_raw and raw_to_celsius conversion functions. Now it can
happen that due to rounding errors celsius_to_raw(Tcrit) returns a raw
value that when converted back to celsius is different from the
original value in °C. This would mean the hardware triggers an interrupt
for a trip point and the thermal core does not react because get_temp
actually returns a different temperature than previously programmed as
interrupt trigger. This way we would lose hot (or cold) events.

> 
> > +}
> > +
> >  void thermal_zone_device_update(struct thermal_zone_device *tz)
> >  {
> >  	int temp, ret, count;
> > @@ -479,6 +518,8 @@ void thermal_zone_device_update(struct
> thermal_zone_device *tz)
> >  	dev_dbg(&tz->device, "last_temperature=%d, current_temperature=%d\n",
> >  				tz->last_temperature, tz->temperature);
> >
> > +	thermal_zone_set_trips(tz);
> > +
> >  	for (count = 0; count < tz->trips; count++)
> >  		handle_thermal_trip(tz, count);
> >  }
> 
> set_trips should also be called from temp_store and other places
> that modify values that affect the trip points.

Good point.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points
  2015-05-18 12:09     ` Sascha Hauer
@ 2015-05-18 18:44       ` Brian Norris
  2015-05-18 19:13         ` Mikko Perttunen
  2015-05-19 14:05         ` Sascha Hauer
  2015-05-19 13:58       ` Sascha Hauer
  1 sibling, 2 replies; 34+ messages in thread
From: Brian Norris @ 2015-05-18 18:44 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Mikko Perttunen, linux-pm, Zhang Rui, Eduardo Valentin,
	linux-kernel, Stephen Warren, kernel, linux-mediatek,
	linux-arm-kernel

On Mon, May 18, 2015 at 02:09:44PM +0200, Sascha Hauer wrote:
> On Mon, May 18, 2015 at 12:06:50PM +0300, Mikko Perttunen wrote:
> > One interesting thing I noticed was that at least the bang-bang
> > governor only acts if the temperature is properly smaller than (trip
> > temp - hysteresis). So perhaps we should specify the non-tripping
> > range as [low, high)? Or we could change bang-bang.
> 
> I wonder how we can protect against such off-by-one errors anyway.
> Generally a hardware might operate on raw values rather than directly
> in temperature values in °C. This means a driver for this must have
> celsius_to_raw and raw_to_celsius conversion functions. Now it can
> happen that due to rounding errors celsius_to_raw(Tcrit) returns a raw
> value that when converted back to celsius is different from the
> original value in °C. This would mean the hardware triggers an interrupt
> for a trip point and the thermal core does not react because get_temp
> actually returns a different temperature than previously programmed as
> interrupt trigger. This way we would lose hot (or cold) events.

This also highlights another fact: there's a race between interrupt
generation and temperature reading (->get_temp()). I would expect any
hardware interrupt thermal sensor would also have a latched temperature
reading to correspond with it, and there would be no guarantee that this
latched temperature will match the polled reading seen once you reach
thermal_zone_device_update(). So a hardware driver might report a
thermal update, but the temperature reported to the core won't
necessarily match what interrupt was meant for.

I have a patch that adds a thermal_zone_device_update_temp() API, so
drivers can report the temperature along with the interrupt
notification. (Such a patch also helps so that the driver can choose to
round down on cold events and up on hot events, resolving your rounding
issue too.)

Brian

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points
  2015-05-18 18:44       ` Brian Norris
@ 2015-05-18 19:13         ` Mikko Perttunen
  2015-05-18 20:28           ` Brian Norris
  2015-05-19 14:05         ` Sascha Hauer
  1 sibling, 1 reply; 34+ messages in thread
From: Mikko Perttunen @ 2015-05-18 19:13 UTC (permalink / raw)
  To: Brian Norris, Sascha Hauer
  Cc: linux-pm, Zhang Rui, Eduardo Valentin, linux-kernel,
	Stephen Warren, kernel, linux-mediatek, linux-arm-kernel

On 05/18/2015 09:44 PM, Brian Norris wrote:
> On Mon, May 18, 2015 at 02:09:44PM +0200, Sascha Hauer wrote:
>> On Mon, May 18, 2015 at 12:06:50PM +0300, Mikko Perttunen wrote:
>>> One interesting thing I noticed was that at least the bang-bang
>>> governor only acts if the temperature is properly smaller than (trip
>>> temp - hysteresis). So perhaps we should specify the non-tripping
>>> range as [low, high)? Or we could change bang-bang.
>>
>> I wonder how we can protect against such off-by-one errors anyway.
>> Generally a hardware might operate on raw values rather than directly
>> in temperature values in °C. This means a driver for this must have
>> celsius_to_raw and raw_to_celsius conversion functions. Now it can
>> happen that due to rounding errors celsius_to_raw(Tcrit) returns a raw
>> value that when converted back to celsius is different from the
>> original value in °C. This would mean the hardware triggers an interrupt
>> for a trip point and the thermal core does not react because get_temp
>> actually returns a different temperature than previously programmed as
>> interrupt trigger. This way we would lose hot (or cold) events.
>
> This also highlights another fact: there's a race between interrupt
> generation and temperature reading (->get_temp()). I would expect any
> hardware interrupt thermal sensor would also have a latched temperature
> reading to correspond with it, and there would be no guarantee that this
> latched temperature will match the polled reading seen once you reach
> thermal_zone_device_update(). So a hardware driver might report a
> thermal update, but the temperature reported to the core won't
> necessarily match what interrupt was meant for.

Does this actually matter? The thermal core will reset trips and apply 
cooling using the new - most recent - value. Using bang bang as example, 
if the temperature has risen since the interrupt fired, the cooling 
device will correctly not be switched off. If the temperature has 
fallen, it will again be correctly switched off. The only issue is then 
if the temperature is exactly 'trip temp - trip hyst' which will cause 
set_trips to load the trip points below, but not cause bang bang to turn 
off the cooling device, and the next chance it will have will only be at 
the next below trip point. Well, this is still safe (at least until you 
replace "cooling device" with "heating device"), so maybe it isn't that 
big of an issue.

Please point out if there's a problem with my line of reasoning.

FWIW - at least Tegra doesn't have a latched register like this. There's 
just a bit indicating that an interrupt was raised and a temperature 
register that updates according to the sensor's input clock.

>
> I have a patch that adds a thermal_zone_device_update_temp() API, so
> drivers can report the temperature along with the interrupt
> notification. (Such a patch also helps so that the driver can choose to
> round down on cold events and up on hot events, resolving your rounding
> issue too.)
>
> Brian
>

Cheers,
Mikko


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points
  2015-05-18 19:13         ` Mikko Perttunen
@ 2015-05-18 20:28           ` Brian Norris
  2015-05-19 12:43             ` Mikko Perttunen
  0 siblings, 1 reply; 34+ messages in thread
From: Brian Norris @ 2015-05-18 20:28 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: Sascha Hauer, linux-pm, Zhang Rui, Eduardo Valentin,
	linux-kernel, Stephen Warren, kernel, linux-mediatek,
	linux-arm-kernel

On Mon, May 18, 2015 at 10:13:46PM +0300, Mikko Perttunen wrote:
> On 05/18/2015 09:44 PM, Brian Norris wrote:
> >On Mon, May 18, 2015 at 02:09:44PM +0200, Sascha Hauer wrote:
> >>On Mon, May 18, 2015 at 12:06:50PM +0300, Mikko Perttunen wrote:
> >>>One interesting thing I noticed was that at least the bang-bang
> >>>governor only acts if the temperature is properly smaller than (trip
> >>>temp - hysteresis). So perhaps we should specify the non-tripping
> >>>range as [low, high)? Or we could change bang-bang.
> >>
> >>I wonder how we can protect against such off-by-one errors anyway.
> >>Generally a hardware might operate on raw values rather than directly
> >>in temperature values in °C. This means a driver for this must have
> >>celsius_to_raw and raw_to_celsius conversion functions. Now it can
> >>happen that due to rounding errors celsius_to_raw(Tcrit) returns a raw
> >>value that when converted back to celsius is different from the
> >>original value in °C. This would mean the hardware triggers an interrupt
> >>for a trip point and the thermal core does not react because get_temp
> >>actually returns a different temperature than previously programmed as
> >>interrupt trigger. This way we would lose hot (or cold) events.
> >
> >This also highlights another fact: there's a race between interrupt
> >generation and temperature reading (->get_temp()). I would expect any
> >hardware interrupt thermal sensor would also have a latched temperature
> >reading to correspond with it, and there would be no guarantee that this
> >latched temperature will match the polled reading seen once you reach
> >thermal_zone_device_update(). So a hardware driver might report a
> >thermal update, but the temperature reported to the core won't
> >necessarily match what interrupt was meant for.
> 
> Does this actually matter? The thermal core will reset trips and
> apply cooling using the new - most recent - value. Using bang bang
> as example, if the temperature has risen since the interrupt fired,
> the cooling device will correctly not be switched off. If the
> temperature has fallen, it will again be correctly switched off. The
> only issue is then if the temperature is exactly 'trip temp - trip
> hyst' which will cause set_trips to load the trip points below, but
> not cause bang bang to turn off the cooling device, and the next
> chance it will have will only be at the next below trip point. Well,
> this is still safe (at least until you replace "cooling device" with
> "heating device"), so maybe it isn't that big of an issue.
> 
> Please point out if there's a problem with my line of reasoning.

I'm not sure I followed exactly the reason for the low-temp/hyst corner
case, but otherwise I guess that makes sense. The only problem IMO, is
that you're encouraging the generation of spurious notifications; if the
temperature is constantly changing right around 'trip temp', but it
never settles above 'trip temp' long enough for the core to re-capture
the high temperature scenario, you'll just keep making useless calls to
thermal_zone_device_update(). This kind of defeats the purpose of the
hysteresis, right?

I'd really rather have a high temperature interrupt generate exactly one
notification to the core framework, and that the sensor driver can rely
on that one interrupt being handled as a high temperature situation,
allowing it to disable the high-temp interrupt.

One of my biggest problems with the thermal subsystem so far is that
thermal_zone_device_update() doesn't actually seem to have any specific
semantic meaning. It just means that there was some reason to update
something. So then, you have to reason about a particular thermal
governor (bang bang) in order to make something sensible. If I want to
use a different sort of user-space governor, then I have to reevaluate
all these same assumptions, and it seems like I end up with a sub-par
solution.

As a side note: I have patches to extend some of the uevent information
passed by the user-space governor too, to accomplish what I'm suggesting
above. Perhaps that would be a better way to discuss what I'm thinking.

> FWIW - at least Tegra doesn't have a latched register like this.
> There's just a bit indicating that an interrupt was raised and a
> temperature register that updates according to the sensor's input
> clock.

A sensor for Broadcom's BCM7xxx has a latched register. If I get the
time, I'll post my driver soon.

Brian

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points
  2015-05-18 20:28           ` Brian Norris
@ 2015-05-19 12:43             ` Mikko Perttunen
  0 siblings, 0 replies; 34+ messages in thread
From: Mikko Perttunen @ 2015-05-19 12:43 UTC (permalink / raw)
  To: Brian Norris
  Cc: Sascha Hauer, linux-pm, Zhang Rui, Eduardo Valentin,
	linux-kernel, Stephen Warren, kernel, linux-mediatek,
	linux-arm-kernel

On 05/18/15 23:28, Brian Norris wrote:
> On Mon, May 18, 2015 at 10:13:46PM +0300, Mikko Perttunen wrote:
>> On 05/18/2015 09:44 PM, Brian Norris wrote:
>>> On Mon, May 18, 2015 at 02:09:44PM +0200, Sascha Hauer wrote:
>>>> On Mon, May 18, 2015 at 12:06:50PM +0300, Mikko Perttunen wrote:
>>>>> One interesting thing I noticed was that at least the bang-bang
>>>>> governor only acts if the temperature is properly smaller than (trip
>>>>> temp - hysteresis). So perhaps we should specify the non-tripping
>>>>> range as [low, high)? Or we could change bang-bang.
>>>>
>>>> I wonder how we can protect against such off-by-one errors anyway.
>>>> Generally a hardware might operate on raw values rather than directly
>>>> in temperature values in °C. This means a driver for this must have
>>>> celsius_to_raw and raw_to_celsius conversion functions. Now it can
>>>> happen that due to rounding errors celsius_to_raw(Tcrit) returns a raw
>>>> value that when converted back to celsius is different from the
>>>> original value in °C. This would mean the hardware triggers an interrupt
>>>> for a trip point and the thermal core does not react because get_temp
>>>> actually returns a different temperature than previously programmed as
>>>> interrupt trigger. This way we would lose hot (or cold) events.
>>>
>>> This also highlights another fact: there's a race between interrupt
>>> generation and temperature reading (->get_temp()). I would expect any
>>> hardware interrupt thermal sensor would also have a latched temperature
>>> reading to correspond with it, and there would be no guarantee that this
>>> latched temperature will match the polled reading seen once you reach
>>> thermal_zone_device_update(). So a hardware driver might report a
>>> thermal update, but the temperature reported to the core won't
>>> necessarily match what interrupt was meant for.
>>
>> Does this actually matter? The thermal core will reset trips and
>> apply cooling using the new - most recent - value. Using bang bang
>> as example, if the temperature has risen since the interrupt fired,
>> the cooling device will correctly not be switched off. If the
>> temperature has fallen, it will again be correctly switched off. The
>> only issue is then if the temperature is exactly 'trip temp - trip
>> hyst' which will cause set_trips to load the trip points below, but
>> not cause bang bang to turn off the cooling device, and the next
>> chance it will have will only be at the next below trip point. Well,
>> this is still safe (at least until you replace "cooling device" with
>> "heating device"), so maybe it isn't that big of an issue.
>>
>> Please point out if there's a problem with my line of reasoning.
>
> I'm not sure I followed exactly the reason for the low-temp/hyst corner
> case, but otherwise I guess that makes sense. The only problem IMO, is
> that you're encouraging the generation of spurious notifications; if the
> temperature is constantly changing right around 'trip temp', but it
> never settles above 'trip temp' long enough for the core to re-capture
> the high temperature scenario, you'll just keep making useless calls to
> thermal_zone_device_update(). This kind of defeats the purpose of the
> hysteresis, right?

The corner case with bang bang is as follows:
- Say we have trip points as 50C and 80C, both with 5C hysteresis, and 
these are programmed into hardware. So the actual hardware trip points 
are 45C and 80C.
- Currently the temperature is, say, 60C and the fan is turned on.
- Temperature drops to 45C, the lower trip point is triggered.
- 45C >= 50C - 5C, so the fan is not turned off.

If we said that the hysteresis was 0C, then bang bang is certainly 
correct in that if the trip point was at 50C, it shouldn't turn the fan 
off, since that is greater than or equal to the requested temperature 
for cooling.

The function you describe would certainly be useful for eliminating 
possible superfluous interrupts due to temperature wobble, though I'm 
not sure how much of a problem that even would be.

>
> I'd really rather have a high temperature interrupt generate exactly one
> notification to the core framework, and that the sensor driver can rely
> on that one interrupt being handled as a high temperature situation,
> allowing it to disable the high-temp interrupt.
>
> One of my biggest problems with the thermal subsystem so far is that
> thermal_zone_device_update() doesn't actually seem to have any specific
> semantic meaning. It just means that there was some reason to update
> something. So then, you have to reason about a particular thermal
> governor (bang bang) in order to make something sensible. If I want to
> use a different sort of user-space governor, then I have to reevaluate
> all these same assumptions, and it seems like I end up with a sub-par
> solution.

Yeah, though I'm not sure if you can ever be sure that the governor is 
fine not getting regular temperature updates, so I imagine you might 
always end up needing to pick your governors with that in mind. In 
practice, this might not be so horrible.

>
> As a side note: I have patches to extend some of the uevent information
> passed by the user-space governor too, to accomplish what I'm suggesting
> above. Perhaps that would be a better way to discuss what I'm thinking.
>
>> FWIW - at least Tegra doesn't have a latched register like this.
>> There's just a bit indicating that an interrupt was raised and a
>> temperature register that updates according to the sensor's input
>> clock.
>
> A sensor for Broadcom's BCM7xxx has a latched register. If I get the
> time, I'll post my driver soon.
>
> Brian
>


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points
  2015-05-18 18:44       ` Brian Norris
  2015-05-18 19:13         ` Mikko Perttunen
@ 2015-05-19 14:05         ` Sascha Hauer
  1 sibling, 0 replies; 34+ messages in thread
From: Sascha Hauer @ 2015-05-19 14:05 UTC (permalink / raw)
  To: Brian Norris
  Cc: Mikko Perttunen, linux-pm, Zhang Rui, Eduardo Valentin,
	linux-kernel, Stephen Warren, kernel, linux-mediatek,
	linux-arm-kernel

On Mon, May 18, 2015 at 11:44:33AM -0700, Brian Norris wrote:
> On Mon, May 18, 2015 at 02:09:44PM +0200, Sascha Hauer wrote:
> > On Mon, May 18, 2015 at 12:06:50PM +0300, Mikko Perttunen wrote:
> > > One interesting thing I noticed was that at least the bang-bang
> > > governor only acts if the temperature is properly smaller than (trip
> > > temp - hysteresis). So perhaps we should specify the non-tripping
> > > range as [low, high)? Or we could change bang-bang.
> > 
> > I wonder how we can protect against such off-by-one errors anyway.
> > Generally a hardware might operate on raw values rather than directly
> > in temperature values in °C. This means a driver for this must have
> > celsius_to_raw and raw_to_celsius conversion functions. Now it can
> > happen that due to rounding errors celsius_to_raw(Tcrit) returns a raw
> > value that when converted back to celsius is different from the
> > original value in °C. This would mean the hardware triggers an interrupt
> > for a trip point and the thermal core does not react because get_temp
> > actually returns a different temperature than previously programmed as
> > interrupt trigger. This way we would lose hot (or cold) events.
> 
> This also highlights another fact: there's a race between interrupt
> generation and temperature reading (->get_temp()). I would expect any
> hardware interrupt thermal sensor would also have a latched temperature
> reading to correspond with it, and there would be no guarantee that this
> latched temperature will match the polled reading seen once you reach
> thermal_zone_device_update(). So a hardware driver might report a
> thermal update, but the temperature reported to the core won't
> necessarily match what interrupt was meant for.
> 
> I have a patch that adds a thermal_zone_device_update_temp() API, so
> drivers can report the temperature along with the interrupt
> notification. (Such a patch also helps so that the driver can choose to
> round down on cold events and up on hot events, resolving your rounding
> issue too.)

Could you send me that patch? Thinking about it this might indeed work.
The only thing that a driver needs to make sure then is that it actually
at least one time reports a temperature beyond the currently programmed
thresholds. With the patch you describe a driver could simply do that by
ignoring the current ADC values and simply reporting the previously
desired values.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points
  2015-05-18 12:09     ` Sascha Hauer
  2015-05-18 18:44       ` Brian Norris
@ 2015-05-19 13:58       ` Sascha Hauer
  2015-05-19 14:05         ` Mikko Perttunen
  1 sibling, 1 reply; 34+ messages in thread
From: Sascha Hauer @ 2015-05-19 13:58 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: Stephen Warren, linux-pm, linux-kernel, Eduardo Valentin,
	linux-mediatek, kernel, Zhang Rui, Brian Norris,
	linux-arm-kernel

On Mon, May 18, 2015 at 02:09:44PM +0200, Sascha Hauer wrote:
> Hi Mikko,
> 
> On Mon, May 18, 2015 at 12:06:50PM +0300, Mikko Perttunen wrote:
> > > +	for (i = 0; i < tz->trips; i++) {
> > > +		int trip_low;
> > > +
> > > +		tz->ops->get_trip_temp(tz, i, &trip_temp);
> > > +		tz->ops->get_trip_hyst(tz, i, &hysteresis);
> > > +
> > > +		trip_low = trip_temp - hysteresis;
> > > +
> > > +		if (trip_low < temp && trip_low > low)
> > > +			low = trip_low;
> > > +
> > > +		if (trip_temp > temp && trip_temp < high)
> > > +			high = trip_temp;
> > > +	}
> > > +
> > > +	tz->prev_low_trip = low;
> > > +	tz->prev_high_trip = high;
> > > +
> > > +	dev_dbg(&tz->device, "new temperature boundaries: %d < x < %d\n",
> > > +			low, high);
> > > +
> > > +	tz->ops->set_trips(tz, low, high);
> > 
> > This should probably do something if set_trips returns an error
> > code; at least an error message, perhaps enable polling? I'm not
> > exactly sure what safety features the thermal framework has in
> > general if errors happen..
> 
> Currently a thermal zone has the passive_delay and polling_delay
> variables. If these are nonzero the thermal core will always poll. A
> purely interrupt driven thermal zone would set these values to zero.
> In this case the thermal core has no basis for polling, so we would
> have to make up polling intervals when set_trips fails. Another
> possibility would be to interpret the *_delay variables as 'when
> set_trips is available, do not poll. When something goes wrong, use
> *_delay as polling intervals'
> 
> > 
> > One interesting thing I noticed was that at least the bang-bang
> > governor only acts if the temperature is properly smaller than (trip
> > temp - hysteresis). So perhaps we should specify the non-tripping
> > range as [low, high)? Or we could change bang-bang.
> 
> I wonder how we can protect against such off-by-one errors anyway.
> Generally a hardware might operate on raw values rather than directly
> in temperature values in °C. This means a driver for this must have
> celsius_to_raw and raw_to_celsius conversion functions. Now it can
> happen that due to rounding errors celsius_to_raw(Tcrit) returns a raw
> value that when converted back to celsius is different from the
> original value in °C. This would mean the hardware triggers an interrupt
> for a trip point and the thermal core does not react because get_temp
> actually returns a different temperature than previously programmed as
> interrupt trigger. This way we would lose hot (or cold) events.

As a simple example we could imagine a 12bit adc which has:

u32 mcelsius_to_raw(int temp)
{
	return temp / 30;
}

int raw_to_mcelsius(u32 raw)
{
	return temp * 30;
}

Now if the thermal framework requests an interrupt at 77000mC we
would program a raw value of 77000 / 30 = 2566.666667, due to integer
rounding we would program 2566. Now when the interrupt is triggered with
this exact raw value we would convert it back to 2566 * 30 = 76980. The
thermal framework would realize that this is below the threshold, do
nothing and go back to sleep.
I am beginning to think that implementing interrupts like this is not a
good idea, at least I found no convenient way out of this situation.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points
  2015-05-19 13:58       ` Sascha Hauer
@ 2015-05-19 14:05         ` Mikko Perttunen
  2015-05-20 13:21           ` Sascha Hauer
  0 siblings, 1 reply; 34+ messages in thread
From: Mikko Perttunen @ 2015-05-19 14:05 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Stephen Warren, linux-pm, linux-kernel, Eduardo Valentin,
	linux-mediatek, kernel, Zhang Rui, Brian Norris,
	linux-arm-kernel

On 05/19/15 16:58, Sascha Hauer wrote:
> On Mon, May 18, 2015 at 02:09:44PM +0200, Sascha Hauer wrote:
>> Hi Mikko,
>>
>> On Mon, May 18, 2015 at 12:06:50PM +0300, Mikko Perttunen wrote:
>>>> +	for (i = 0; i < tz->trips; i++) {
>>>> +		int trip_low;
>>>> +
>>>> +		tz->ops->get_trip_temp(tz, i, &trip_temp);
>>>> +		tz->ops->get_trip_hyst(tz, i, &hysteresis);
>>>> +
>>>> +		trip_low = trip_temp - hysteresis;
>>>> +
>>>> +		if (trip_low < temp && trip_low > low)
>>>> +			low = trip_low;
>>>> +
>>>> +		if (trip_temp > temp && trip_temp < high)
>>>> +			high = trip_temp;
>>>> +	}
>>>> +
>>>> +	tz->prev_low_trip = low;
>>>> +	tz->prev_high_trip = high;
>>>> +
>>>> +	dev_dbg(&tz->device, "new temperature boundaries: %d < x < %d\n",
>>>> +			low, high);
>>>> +
>>>> +	tz->ops->set_trips(tz, low, high);
>>>
>>> This should probably do something if set_trips returns an error
>>> code; at least an error message, perhaps enable polling? I'm not
>>> exactly sure what safety features the thermal framework has in
>>> general if errors happen..
>>
>> Currently a thermal zone has the passive_delay and polling_delay
>> variables. If these are nonzero the thermal core will always poll. A
>> purely interrupt driven thermal zone would set these values to zero.
>> In this case the thermal core has no basis for polling, so we would
>> have to make up polling intervals when set_trips fails. Another
>> possibility would be to interpret the *_delay variables as 'when
>> set_trips is available, do not poll. When something goes wrong, use
>> *_delay as polling intervals'
>>
>>>
>>> One interesting thing I noticed was that at least the bang-bang
>>> governor only acts if the temperature is properly smaller than (trip
>>> temp - hysteresis). So perhaps we should specify the non-tripping
>>> range as [low, high)? Or we could change bang-bang.
>>
>> I wonder how we can protect against such off-by-one errors anyway.
>> Generally a hardware might operate on raw values rather than directly
>> in temperature values in °C. This means a driver for this must have
>> celsius_to_raw and raw_to_celsius conversion functions. Now it can
>> happen that due to rounding errors celsius_to_raw(Tcrit) returns a raw
>> value that when converted back to celsius is different from the
>> original value in °C. This would mean the hardware triggers an interrupt
>> for a trip point and the thermal core does not react because get_temp
>> actually returns a different temperature than previously programmed as
>> interrupt trigger. This way we would lose hot (or cold) events.
>
> As a simple example we could imagine a 12bit adc which has:
>
> u32 mcelsius_to_raw(int temp)
> {
> 	return temp / 30;
> }
>
> int raw_to_mcelsius(u32 raw)
> {
> 	return temp * 30;
> }
>
> Now if the thermal framework requests an interrupt at 77000mC we
> would program a raw value of 77000 / 30 = 2566.666667, due to integer
> rounding we would program 2566. Now when the interrupt is triggered with
> this exact raw value we would convert it back to 2566 * 30 = 76980. The
> thermal framework would realize that this is below the threshold, do
> nothing and go back to sleep.
> I am beginning to think that implementing interrupts like this is not a
> good idea, at least I found no convenient way out of this situation.

Couldn't you just specify that the driver should do the best it can? 
That is, in this case, the driver would program the hardware for the 
least possible value x for which raw_to_mcelsius(x) >= 77000.

>
> Sascha
>

Cheers,
Mikko.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points
  2015-05-19 14:05         ` Mikko Perttunen
@ 2015-05-20 13:21           ` Sascha Hauer
  0 siblings, 0 replies; 34+ messages in thread
From: Sascha Hauer @ 2015-05-20 13:21 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: Stephen Warren, linux-pm, linux-kernel, Eduardo Valentin,
	linux-mediatek, kernel, Zhang Rui, Brian Norris,
	linux-arm-kernel

On Tue, May 19, 2015 at 05:05:29PM +0300, Mikko Perttunen wrote:
> On 05/19/15 16:58, Sascha Hauer wrote:
> >On Mon, May 18, 2015 at 02:09:44PM +0200, Sascha Hauer wrote:
> >>Hi Mikko,
> >>
> >>On Mon, May 18, 2015 at 12:06:50PM +0300, Mikko Perttunen wrote:
> >>>>+	for (i = 0; i < tz->trips; i++) {
> >>>>+		int trip_low;
> >>>>+
> >>>>+		tz->ops->get_trip_temp(tz, i, &trip_temp);
> >>>>+		tz->ops->get_trip_hyst(tz, i, &hysteresis);
> >>>>+
> >>>>+		trip_low = trip_temp - hysteresis;
> >>>>+
> >>>>+		if (trip_low < temp && trip_low > low)
> >>>>+			low = trip_low;
> >>>>+
> >>>>+		if (trip_temp > temp && trip_temp < high)
> >>>>+			high = trip_temp;
> >>>>+	}
> >>>>+
> >>>>+	tz->prev_low_trip = low;
> >>>>+	tz->prev_high_trip = high;
> >>>>+
> >>>>+	dev_dbg(&tz->device, "new temperature boundaries: %d < x < %d\n",
> >>>>+			low, high);
> >>>>+
> >>>>+	tz->ops->set_trips(tz, low, high);
> >>>
> >>>This should probably do something if set_trips returns an error
> >>>code; at least an error message, perhaps enable polling? I'm not
> >>>exactly sure what safety features the thermal framework has in
> >>>general if errors happen..
> >>
> >>Currently a thermal zone has the passive_delay and polling_delay
> >>variables. If these are nonzero the thermal core will always poll. A
> >>purely interrupt driven thermal zone would set these values to zero.
> >>In this case the thermal core has no basis for polling, so we would
> >>have to make up polling intervals when set_trips fails. Another
> >>possibility would be to interpret the *_delay variables as 'when
> >>set_trips is available, do not poll. When something goes wrong, use
> >>*_delay as polling intervals'
> >>
> >>>
> >>>One interesting thing I noticed was that at least the bang-bang
> >>>governor only acts if the temperature is properly smaller than (trip
> >>>temp - hysteresis). So perhaps we should specify the non-tripping
> >>>range as [low, high)? Or we could change bang-bang.
> >>
> >>I wonder how we can protect against such off-by-one errors anyway.
> >>Generally a hardware might operate on raw values rather than directly
> >>in temperature values in °C. This means a driver for this must have
> >>celsius_to_raw and raw_to_celsius conversion functions. Now it can
> >>happen that due to rounding errors celsius_to_raw(Tcrit) returns a raw
> >>value that when converted back to celsius is different from the
> >>original value in °C. This would mean the hardware triggers an interrupt
> >>for a trip point and the thermal core does not react because get_temp
> >>actually returns a different temperature than previously programmed as
> >>interrupt trigger. This way we would lose hot (or cold) events.
> >
> >As a simple example we could imagine a 12bit adc which has:
> >
> >u32 mcelsius_to_raw(int temp)
> >{
> >	return temp / 30;
> >}
> >
> >int raw_to_mcelsius(u32 raw)
> >{
> >	return temp * 30;
> >}
> >
> >Now if the thermal framework requests an interrupt at 77000mC we
> >would program a raw value of 77000 / 30 = 2566.666667, due to integer
> >rounding we would program 2566. Now when the interrupt is triggered with
> >this exact raw value we would convert it back to 2566 * 30 = 76980. The
> >thermal framework would realize that this is below the threshold, do
> >nothing and go back to sleep.
> >I am beginning to think that implementing interrupts like this is not a
> >good idea, at least I found no convenient way out of this situation.
> 
> Couldn't you just specify that the driver should do the best it can?
> That is, in this case, the driver would program the hardware for the
> least possible value x for which raw_to_mcelsius(x) >= 77000.

That's what I did now.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 12/15] thermal: of: implement .set_trips for device tree thermal zones
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
                   ` (10 preceding siblings ...)
  2015-05-13  8:52 ` [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  2015-05-18  9:09   ` Mikko Perttunen
  2015-05-13  8:52 ` [PATCH 13/15] dt-bindings: thermal: Add binding document for Mediatek thermal controller Sascha Hauer
                   ` (2 subsequent siblings)
  14 siblings, 1 reply; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/thermal/of-thermal.c | 12 ++++++++++++
 include/linux/thermal.h      |  3 +++
 2 files changed, 15 insertions(+)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index bd3185e..f8dd847 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -97,6 +97,17 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz,
 	return data->ops->get_temp(data->sensor_data, temp);
 }
 
+static int of_thermal_set_trips(struct thermal_zone_device *tz,
+			       int low, int high)
+{
+	struct __thermal_zone *data = tz->devdata;
+
+	if (!data->ops || !data->ops->set_trips)
+		return -ENOSYS;
+
+	return data->ops->set_trips(data->sensor_data, low, high);
+}
+
 /**
  * of_thermal_get_ntrips - function to export number of available trip
  *			   points.
@@ -367,6 +378,7 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
 
 static const struct thermal_zone_device_ops of_thermal_ops = {
 	.get_temp = of_thermal_get_temp,
+	.set_trips = of_thermal_set_trips,
 	.get_trend = of_thermal_get_trend,
 	.set_emul_temp = of_thermal_set_emul_temp,
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index aef6e13..b751f6b 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -267,12 +267,15 @@ struct thermal_genl_event {
  *
  * Optional:
  * @get_trend: a pointer to a function that reads the sensor temperature trend.
+ * @set_trips: a pointer to a function that sets a temperature window which shall
+ *             trigger an interrupt when it is left.
  * @set_emul_temp: a pointer to a function that sets sensor emulated
  *		   temperature.
  */
 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 (*set_emul_temp)(void *, int);
 };
 
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 12/15] thermal: of: implement .set_trips for device tree thermal zones
  2015-05-13  8:52 ` [PATCH 12/15] thermal: of: implement .set_trips for device tree thermal zones Sascha Hauer
@ 2015-05-18  9:09   ` Mikko Perttunen
  0 siblings, 0 replies; 34+ messages in thread
From: Mikko Perttunen @ 2015-05-18  9:09 UTC (permalink / raw)
  To: Sascha Hauer, linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	kernel, linux-mediatek, linux-arm-kernel, Brian Norris

On 05/13/15 11:52, Sascha Hauer wrote:
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>   drivers/thermal/of-thermal.c | 12 ++++++++++++
>   include/linux/thermal.h      |  3 +++
>   2 files changed, 15 insertions(+)
>
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index bd3185e..f8dd847 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -97,6 +97,17 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz,
>   	return data->ops->get_temp(data->sensor_data, temp);
>   }
>
> +static int of_thermal_set_trips(struct thermal_zone_device *tz,
> +			       int low, int high)
> +{
> +	struct __thermal_zone *data = tz->devdata;
> +
> +	if (!data->ops || !data->ops->set_trips)
> +		return -ENOSYS;
> +
> +	return data->ops->set_trips(data->sensor_data, low, high);
> +}
> +
>   /**
>    * of_thermal_get_ntrips - function to export number of available trip
>    *			   points.
> @@ -367,6 +378,7 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
>
>   static const struct thermal_zone_device_ops of_thermal_ops = {
>   	.get_temp = of_thermal_get_temp,
> +	.set_trips = of_thermal_set_trips,
>   	.get_trend = of_thermal_get_trend,
>   	.set_emul_temp = of_thermal_set_emul_temp,
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index aef6e13..b751f6b 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -267,12 +267,15 @@ struct thermal_genl_event {
>    *
>    * Optional:
>    * @get_trend: a pointer to a function that reads the sensor temperature trend.
> + * @set_trips: a pointer to a function that sets a temperature window which shall
> + *             trigger an interrupt when it is left.

Perhaps this documentation should be expanded a little; the commit 
message in the previous patch looked pretty good. The fact that the 
driver needs to call thermal_zone_device_update should be mentioned 
somewhere.

>    * @set_emul_temp: a pointer to a function that sets sensor emulated
>    *		   temperature.
>    */
>   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 (*set_emul_temp)(void *, int);
>   };
>
>

Thanks,
Mikko.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 13/15] dt-bindings: thermal: Add binding document for Mediatek thermal controller
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
                   ` (11 preceding siblings ...)
  2015-05-13  8:52 ` [PATCH 12/15] thermal: of: implement .set_trips for device tree thermal zones Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  2015-05-13  8:52 ` [PATCH 14/15] thermal: Add Mediatek thermal controller support Sascha Hauer
  2015-05-13  8:52 ` [PATCH 15/15] ARM64: dts: mt8173: Add thermal/auxadc device nodes Sascha Hauer
  14 siblings, 0 replies; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../bindings/thermal/mediatek-thermal.txt          | 36 ++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/mediatek-thermal.txt

diff --git a/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt b/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt
new file mode 100644
index 0000000..adf5d2c
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt
@@ -0,0 +1,36 @@
+* Mediatek Thermal
+
+This describes the device tree binding for the Mediatek thermal controller
+which measures the on-SoC temperatures. This device does not have its own ADC,
+instead it directly controls the AUXADC via AHB bus accesses. For this reason
+this device needs phandles to the AUXADC.
+
+Required properties:
+- compatible: "mediatek,mt8173-thermal"
+- reg: Address range of the thermal controller
+- interrupts: IRQ for the thermal controller
+- clocks, clock-names: Clocks needed for the thermal controller. required
+                       clocks are:
+		"therm":	Main clock needed for register access
+		"auxadc":	The AUXADC clock
+- resets, reset-names: Reference to the reset controller controlling the thermal
+                       controller. Required reset-names:
+		"therm":	The main reset line
+- auxadc: A phandle to the AUXADC which the thermal controller uses
+- apmixedsys: A phandle to the APMIXEDSYS controller.
+- #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description
+
+Example:
+
+	thermal: thermal@1100b000 {
+		#thermal-sensor-cells = <1>;
+		compatible = "mediatek,mt8173-thermal";
+		reg = <0 0x1100b000 0 0x1000>;
+		interrupts = <0 70 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&pericfg CLK_PERI_THERM>, <&pericfg CLK_PERI_AUXADC>;
+		clock-names = "therm", "auxadc";
+		resets = <&pericfg MT8173_PERI_THERM_SW_RST>;
+		reset-names = "therm";
+		auxadc = <&auxadc>;
+		apmixedsys = <&apmixedsys>;
+	};
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 14/15] thermal: Add Mediatek thermal controller support
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
                   ` (12 preceding siblings ...)
  2015-05-13  8:52 ` [PATCH 13/15] dt-bindings: thermal: Add binding document for Mediatek thermal controller Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  2015-05-14  9:52   ` Paul Bolle
  2015-05-13  8:52 ` [PATCH 15/15] ARM64: dts: mt8173: Add thermal/auxadc device nodes Sascha Hauer
  14 siblings, 1 reply; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

This adds support for the Mediatek thermal controller found on MT8173
and likely other SoCs.
The controller is a bit special. It does not have its own ADC, instead
it controls the on-SoC AUXADC via AHB bus accesses. For this reason
we need the physical address of the AUXADC. Also it controls a mux
using AHB bus accesses, so we need the APMIXEDSYS physical address aswell.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/thermal/Kconfig       |   8 +
 drivers/thermal/Makefile      |   1 +
 drivers/thermal/mtk_thermal.c | 728 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 737 insertions(+)
 create mode 100644 drivers/thermal/mtk_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index af40db0..3aa5500 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -285,6 +285,14 @@ config ACPI_THERMAL_REL
 	tristate
 	depends on ACPI
 
+config MTK_THERMAL
+	tristate "Temperature sensor driver for mediatek SoCs"
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	default y
+	help
+	  Enable this option if you want to have support for thermal management
+	  controller present in Mediatek SoCs
+
 menu "Texas Instruments thermal drivers"
 source "drivers/thermal/ti-soc-thermal/Kconfig"
 endmenu
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index fa0dc48..51cfab7 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -39,3 +39,4 @@ obj-$(CONFIG_TI_SOC_THERMAL)	+= ti-soc-thermal/
 obj-$(CONFIG_INT340X_THERMAL)  += int340x_thermal/
 obj-$(CONFIG_ST_THERMAL)	+= st/
 obj-$(CONFIG_TEGRA_SOCTHERM)	+= tegra_soctherm.o
+obj-$(CONFIG_MTK_THERMAL)	+= mtk_thermal.o
diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
new file mode 100644
index 0000000..27aab12
--- /dev/null
+++ b/drivers/thermal/mtk_thermal.c
@@ -0,0 +1,728 @@
+/*
+ * Copyright (c) 2014 MediaTek Inc.
+ * Author: Hanyi.Wu <hanyi.wu@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/dmi.h>
+#include <linux/thermal.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/clk.h>
+#include <linux/time.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/interrupt.h>
+#include <linux/reset.h>
+
+/* AUXADC Registers */
+#define AUXADC_CON0_V		0x000
+#define AUXADC_CON1_V		0x004
+#define AUXADC_CON1_SET_V	0x008
+#define AUXADC_CON1_CLR_V	0x00c
+#define AUXADC_CON2_V		0x010
+#define AUXADC_DATA(channel)	(0x14 + (channel) * 4)
+#define AUXADC_MISC_V		0x094
+
+#define AUXADC_CON1_CHANNEL(x)	(1 << (x))
+
+/* Thermal Controller Registers */
+#define TEMPMONCTL0		0x000
+#define TEMPMONCTL1		0x004
+#define TEMPMONCTL2		0x008
+#define TEMPMONINT		0x00c
+#define TEMPMONINTSTS		0x010
+#define TEMPMONIDET0		0x014
+#define TEMPMONIDET1		0x018
+#define TEMPMONIDET2		0x01c
+#define TEMPH2NTHRE		0x024
+#define TEMPHTHRE		0x028
+#define TEMPCTHRE		0x02c
+#define TEMPOFFSETH		0x030
+#define TEMPOFFSETL		0x034
+#define TEMPMSRCTL0		0x038
+#define TEMPMSRCTL1		0x03c
+#define TEMPAHBPOLL		0x040
+#define TEMPAHBTO		0x044
+#define TEMPADCPNP0		0x048
+#define TEMPADCPNP1		0x04c
+#define TEMPADCPNP2		0x050
+#define TEMPADCPNP3		0x0b4
+
+#define TEMPADCMUX		0x054
+#define TEMPADCEXT		0x058
+#define TEMPADCEXT1		0x05c
+#define TEMPADCEN		0x060
+#define TEMPPNPMUXADDR		0x064
+#define TEMPADCMUXADDR		0x068
+#define TEMPADCEXTADDR		0x06c
+#define TEMPADCEXT1ADDR		0x070
+#define TEMPADCENADDR		0x074
+#define TEMPADCVALIDADDR	0x078
+#define TEMPADCVOLTADDR		0x07c
+#define TEMPRDCTRL		0x080
+#define TEMPADCVALIDMASK	0x084
+#define TEMPADCVOLTAGESHIFT	0x088
+#define TEMPADCWRITECTRL	0x08c
+#define TEMPMSR0		0x090
+#define TEMPMSR1		0x094
+#define TEMPMSR2		0x098
+#define TEMPMSR3		0x0B8
+
+#define TEMPIMMD0		0x0a0
+#define TEMPIMMD1		0x0a4
+#define TEMPIMMD2		0x0a8
+
+#define TEMPPROTCTL		0x0c0
+#define TEMPPROTTA		0x0c4
+#define TEMPPROTTB		0x0c8
+#define TEMPPROTTC		0x0cc
+
+#define TEMPSPARE0		0x0f0
+#define TEMPSPARE1		0x0f4
+#define TEMPSPARE2		0x0f8
+#define TEMPSPARE3		0x0fc
+
+#define PTPCORESEL		0x400
+#define THERMINTST		0x404
+#define PTPODINTST		0x408
+#define THSTAGE0ST		0x40c
+#define THSTAGE1ST		0x410
+#define THSTAGE2ST		0x414
+#define THAHBST0		0x418
+#define THAHBST1		0x41c	/* Only for DE debug */
+#define PTPSPARE0		0x420
+#define PTPSPARE1		0x424
+#define PTPSPARE2		0x428
+#define PTPSPARE3		0x42c
+#define THSLPEVEB		0x430
+
+#define TEMPMONINT_COLD(sp)			((1 << 0) << ((sp) * 5))
+#define TEMPMONINT_HOT(sp)			((1 << 1) << ((sp) * 5))
+#define TEMPMONINT_LOW_OFS(sp)			((1 << 2) << ((sp) * 5))
+#define TEMPMONINT_HIGH_OFS(sp)			((1 << 3) << ((sp) * 5))
+#define TEMPMONINT_HOT_TO_NORM(sp)		((1 << 4) << ((sp) * 5))
+#define TEMPMONINT_TIMEOUT			(1 << 15)
+#define TEMPMONINT_IMMEDIATE_SENSE(sp)		(1 << (16 + (sp)))
+#define TEMPMONINT_FILTER_SENSE(sp)		(1 << (19 + (sp)))
+
+#define TEMPADCWRITECTRL_ADC_PNP_WRITE		(1 << 0)
+#define TEMPADCWRITECTRL_ADC_MUX_WRITE		(1 << 1)
+#define TEMPADCWRITECTRL_ADC_EXTRA_WRITE	(1 << 2)
+#define TEMPADCWRITECTRL_ADC_EXTRA1_WRITE	(1 << 3)
+
+#define TEMPADCVALIDMASK_VALID_HIGH		(1 << 5)
+#define TEMPADCVALIDMASK_VALID_POS(bit)		(bit)
+
+#define TEMPPROTCTL_AVERAGE			(0 << 16)
+#define TEMPPROTCTL_MAXIMUM			(1 << 16)
+#define TEMPPROTCTL_SELECTED			(2 << 16)
+
+#define MT8173_THERMAL_ZONE_CA57		0
+#define MT8173_THERMAL_ZONE_CA53		1
+#define MT8173_THERMAL_ZONE_GPU			2
+#define MT8173_THERMAL_ZONE_CORE		3
+
+#define MT8173_TS1	0
+#define MT8173_TS2	1
+#define MT8173_TS3	2
+#define MT8173_TS4	3
+#define MT8173_TSABB	4
+
+/* AUXADC channel 11 is used for the temperature sensors */
+#define MT8173_TEMP_AUXADC_CHANNEL	11
+
+/* The total number of temperature sensors in the MT8173 */
+#define MT8173_NUM_SENSORS		5
+
+/* The number of banks in the MT8173 */
+#define MT8173_NUM_BANKS		4
+
+/* The number of sensing points per bank */
+#define MT8173_NUM_SENSING_POINTS	4
+
+#define THERMAL_NAME    "mtk-thermal"
+
+struct mtk_thermal;
+
+struct mtk_thermal_bank {
+	struct mtk_thermal *mt;
+	struct thermal_zone_device *tz;
+	int id;
+};
+
+struct mtk_thermal {
+	struct device *dev;
+	void __iomem *thermal_base;
+	void __iomem *auxadc_base;
+
+	u64 auxadc_phys_base;
+	u64 apmixed_phys_base;
+	struct reset_control *reset;
+	struct clk *clk_peri_therm;
+	struct clk *clk_auxadc;
+
+	struct mtk_thermal_bank banks[MT8173_NUM_BANKS];
+
+	struct mutex lock;
+
+	/* Calibration values */
+	s32 adc_ge;
+	s32 adc_oe;
+	s32 degc_cali;
+	s32 o_slope;
+	s32 vts;
+};
+
+struct mtk_thermal_bank_cfg {
+	unsigned int enable_mask;
+	unsigned int sensors[4];
+};
+
+static int sensor_mux_values[MT8173_NUM_SENSORS] = { 0, 1, 2, 3, 16 };
+
+/*
+ * The MT8173 thermal controller has four banks. Each bank can read up to
+ * four temperature sensors simultaneously. The MT8173 has a total of 5
+ * temperature sensors. We use each bank to measure a certain area of the
+ * SoC. Since TS2 is located centrally in the SoC it is influenced by multiple
+ * areas, hence is used in different banks.
+ */
+static struct mtk_thermal_bank_cfg bank_data[] = {
+	{
+		.enable_mask = 3,
+		.sensors = { MT8173_TS2, MT8173_TS3 },
+	}, {
+		.enable_mask = 3,
+		.sensors = { MT8173_TS2, MT8173_TS4 },
+	}, {
+		.enable_mask = 7,
+		.sensors = { MT8173_TS1, MT8173_TS2, MT8173_TSABB },
+	}, {
+		.enable_mask = 1,
+		.sensors = { MT8173_TS2 },
+	},
+};
+
+static int tempmsr_ofs[MT8173_NUM_SENSING_POINTS] = {
+	TEMPMSR0, TEMPMSR1, TEMPMSR2, TEMPMSR3
+};
+
+static int tempadcpnp_ofs[MT8173_NUM_SENSING_POINTS] = {
+	TEMPADCPNP0, TEMPADCPNP1, TEMPADCPNP2, TEMPADCPNP3
+};
+
+/**
+ * raw_to_mcelsius - convert a raw ADC value to mcelsius
+ * @mt:		The thermal controller
+ * @raw:	raw ADC value
+ *
+ * This converts the raw ADC value to mcelsius using the SoC specific
+ * calibration constants
+ */
+static int raw_to_mcelsius(struct mtk_thermal *mt, u32 raw)
+{
+	s32 format_1, format_2, format_3, format_4;
+	s32 xtoomt;
+	s32 gain;
+
+	raw &= 0xfff;
+
+	gain = (10000 + mt->adc_ge);
+
+	xtoomt = ((((mt->vts + 3350 - mt->adc_oe) * 10000) / 4096) * 10000) /
+		gain;
+
+	format_1 = ((mt->degc_cali * 10) >> 1);
+	format_2 = (raw - mt->adc_oe);
+	format_3 = (((((format_2) * 10000) >> 12) * 10000) / gain) - xtoomt;
+	format_3 = format_3 * 15 / 18;
+	format_4 = ((format_3 * 100) / (165 + mt->o_slope));
+	format_4 = format_4 - (format_4 << 1);
+
+	return (format_1 + format_4) * 100;
+}
+
+/**
+ * mcelsius_to_raw - convert mcelsius to raw ADC value
+ * @mt:		The thermal controller
+ * @temp:	The temperature in mcelsius
+ *
+ * This converts a temperature in mcelsius to a raw ADC value, needed to
+ * calculate the trigger values for interrupt generation.
+ */
+static u32 mcelsius_to_raw(struct mtk_thermal *mt, int temp)
+{
+	s32 format_1, format_2, format_3, format_4;
+	s32 xtoomt;
+	s32 gain;
+
+	gain = (10000 + mt->adc_ge);
+
+	xtoomt = ((((mt->vts + 3350 - mt->adc_oe) * 10000) / 4096) * 10000) /
+		gain;
+
+	format_1 = temp - (mt->degc_cali * 1000 / 2);
+	format_2 = format_1 * (165 + mt->o_slope) * 18 / 15;
+	format_2 = format_2 - 2 * format_2;
+	format_3 = format_2 / 1000 + xtoomt * 10;
+	format_4 = (format_3 * 4096 / 10000 * gain) / 100000 + mt->adc_oe;
+
+	return format_4;
+}
+
+/**
+ * mtk_thermal_get_bank - get bank
+ * @bank:	The bank
+ *
+ * The bank registers are banked, we have to select a bank in the
+ * PTPCORESEL register to access it.
+ */
+static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank)
+{
+	struct mtk_thermal *mt = bank->mt;
+	u32 val;
+
+	mutex_lock(&mt->lock);
+
+	val = readl(mt->thermal_base + PTPCORESEL);
+	val &= ~0xf;
+	val |= bank->id;
+	writel(val, mt->thermal_base + PTPCORESEL);
+}
+
+/**
+ * mtk_thermal_put_bank - release bank
+ * @bank:	The bank
+ *
+ * release a bank previously taken with mtk_thermal_get_bank,
+ */
+static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank)
+{
+	struct mtk_thermal *mt = bank->mt;
+
+	mutex_unlock(&mt->lock);
+}
+
+/**
+ * mtk_thermal_bank_temperature - get the temperature of a bank
+ * @bank:	The bank
+ *
+ * The temperature of a bank is considered the maximum temperature of
+ * the sensors associated to the bank.
+ */
+static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
+{
+	struct mtk_thermal *mt = bank->mt;
+	int temp, i, max;
+	u32 raw;
+
+	temp = max = -INT_MAX;
+
+	for (i = 0; i < 4; i++) {
+		int sensno;
+
+		if (!(bank_data[bank->id].enable_mask & (1 << i)))
+			continue;
+
+		raw = readl(mt->thermal_base + tempmsr_ofs[i]);
+
+		sensno = bank_data[bank->id].sensors[i];
+		temp = raw_to_mcelsius(mt, raw);
+
+		if (temp > max)
+			max = temp;
+	}
+
+	return max;
+}
+
+static void mtk_thermal_irq_bank(struct mtk_thermal_bank *bank)
+{
+	struct mtk_thermal *mt = bank->mt;
+	int sp;
+	u32 irqstat;
+	bool update = false;
+
+	mtk_thermal_get_bank(bank);
+
+	irqstat = readl(mt->thermal_base + TEMPMONINTSTS);
+
+	mtk_thermal_put_bank(bank);
+
+	for (sp = 0; sp < 3; sp++) {
+		if (irqstat & TEMPMONINT_LOW_OFS(sp)) {
+			update = true;
+			dev_vdbg(mt->dev, "bank %d sensor %d low offset interrupt\n",
+					bank->id, sp);
+		}
+
+		if (irqstat & TEMPMONINT_HIGH_OFS(sp)) {
+			update = true;
+			dev_vdbg(mt->dev, "bank %d sensor %d high offset interrupt\n",
+					bank->id, sp);
+		}
+	}
+
+	if (update)
+		thermal_zone_device_update(bank->tz);
+}
+
+static irqreturn_t mtk_thermal_irq(int irq, void *dev_id)
+{
+	struct mtk_thermal *mt = dev_id;
+	u32 irqstat = 0;
+	int i;
+
+	irqstat = readl(mt->thermal_base + THERMINTST);
+
+	dev_vdbg(mt->dev, "thermal_interrupt_handler : THERMINTST = 0x%x\n",
+			irqstat);
+
+	for (i = 0; i < MT8173_NUM_BANKS; i++) {
+		if (!(irqstat & (1 << i)))
+			mtk_thermal_irq_bank(&mt->banks[i]);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int mtk_read_temp(void *data, int *temp)
+{
+	struct mtk_thermal_bank *bank = data;
+
+	mtk_thermal_get_bank(bank);
+
+	*temp = mtk_thermal_bank_temperature(bank);
+
+	mtk_thermal_put_bank(bank);
+
+	return 0;
+}
+
+static int mtk_set_trips(void *data, int low, int high)
+{
+	struct mtk_thermal_bank *bank = data;
+	struct mtk_thermal *mt = bank->mt;
+	int i;
+	u32 val, enable_mask;
+	u32 raw_low, raw_high;
+
+	raw_low = mcelsius_to_raw(mt, low);
+	raw_high = mcelsius_to_raw(mt, high);
+
+	mtk_thermal_get_bank(bank);
+
+	writel(0x0, mt->thermal_base + TEMPMONINT);
+
+	writel(TEMPPROTCTL_SELECTED, mt->thermal_base + TEMPPROTCTL);
+
+	writel(raw_low, mt->thermal_base + TEMPOFFSETL);
+	writel(raw_high, mt->thermal_base + TEMPOFFSETH);
+
+	enable_mask = readl(mt->thermal_base + TEMPMONCTL0);
+
+	val = 0;
+	for (i = 0; i < MT8173_NUM_SENSING_POINTS; i++)
+		if (enable_mask & (1 << i))
+			val |=	TEMPMONINT_LOW_OFS(i) | TEMPMONINT_HIGH_OFS(i);
+
+	writel(val, mt->thermal_base + TEMPMONINT);
+
+	mtk_thermal_put_bank(bank);
+
+	dev_dbg(mt->dev, "new boundaries: %d (0x%04x) < x < %d (0x%04x)\n",
+			low, mcelsius_to_raw(mt, low),
+			high, mcelsius_to_raw(mt, high));
+
+	return 0;
+}
+
+static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
+	.get_temp = mtk_read_temp,
+	.set_trips = mtk_set_trips,
+};
+
+static void mtk_thermal_init_bank(struct mtk_thermal_bank *bank)
+{
+	struct mtk_thermal *mt = bank->mt;
+	struct mtk_thermal_bank_cfg *cfg = &bank_data[bank->id];
+	int i;
+
+	mtk_thermal_get_bank(bank);
+
+	/* bus clock 66M counting unit is 12 * 15.15ns * 256 = 46.540us */
+	writel(0x0000000c, mt->thermal_base + TEMPMONCTL1);
+
+	/*
+	 * filt interval is 1 * 46.540us = 46.54us,
+	 * sen interval is 429 * 46.540us = 19.96ms
+	 */
+	writel(0x000101ad, mt->thermal_base + TEMPMONCTL2);
+
+	/* poll is set to 10u */
+	writel(0x00000300, mt->thermal_base + TEMPAHBPOLL);
+
+	/* temperature sampling control, 1 sample */
+	writel(0x00000000, mt->thermal_base + TEMPMSRCTL0);
+
+	/* exceed this polling time, IRQ would be inserted */
+	writel(0xffffffff, mt->thermal_base + TEMPAHBTO);
+
+	/* number of interrupts per event, 1 is enough */
+	writel(0x0, mt->thermal_base + TEMPMONIDET0);
+	writel(0x0, mt->thermal_base + TEMPMONIDET1);
+
+	/*
+	 * The MT8173 thermal controller does not have its own ADC. Instead it
+	 * uses AHB bus accesses to control the AUXADC. To do this the thermal
+	 * controller has to be programmed with the physical addresses of the
+	 * AUXADC registers and with the various bit positions in the AUXADC.
+	 * Also the thermal controller controls a mux in the APMIXEDSYS register
+	 * space.
+	 */
+
+	/*
+	 * this value will be stored to TEMPPNPMUXADDR (TEMPSPARE0)
+	 * automatically by hw
+	 */
+	writel(1 << MT8173_TEMP_AUXADC_CHANNEL, mt->thermal_base + TEMPADCMUX);
+
+	/* AHB address for auxadc mux selection */
+	writel(mt->auxadc_phys_base + 0x00c,
+			mt->thermal_base + TEMPADCMUXADDR);
+
+	/* AHB address for pnp sensor mux selection */
+	writel(mt->apmixed_phys_base + 0x0604,
+			mt->thermal_base + TEMPPNPMUXADDR);
+
+	/* AHB value for auxadc enable */
+	writel(1 << MT8173_TEMP_AUXADC_CHANNEL, mt->thermal_base + TEMPADCEN);
+
+	/* AHB address for auxadc enable (channel 0 immediate mode selected) */
+	writel(mt->auxadc_phys_base + AUXADC_CON1_SET_V,
+			mt->thermal_base + TEMPADCENADDR);
+
+	/* AHB address for auxadc valid bit */
+	writel(mt->auxadc_phys_base + AUXADC_DATA(MT8173_TEMP_AUXADC_CHANNEL),
+			mt->thermal_base + TEMPADCVALIDADDR);
+
+	/* AHB address for auxadc voltage output */
+	writel(mt->auxadc_phys_base + AUXADC_DATA(MT8173_TEMP_AUXADC_CHANNEL),
+			mt->thermal_base + TEMPADCVOLTADDR);
+
+	/* read valid & voltage are at the same register */
+	writel(0x0, mt->thermal_base + TEMPRDCTRL);
+
+	/* indicate where the valid bit is */
+	writel(TEMPADCVALIDMASK_VALID_HIGH | TEMPADCVALIDMASK_VALID_POS(12),
+			mt->thermal_base + TEMPADCVALIDMASK);
+
+	/* no shift */
+	writel(0x0, mt->thermal_base + TEMPADCVOLTAGESHIFT);
+
+	/* enable auxadc mux write transaction */
+	writel(TEMPADCWRITECTRL_ADC_MUX_WRITE,
+			mt->thermal_base + TEMPADCWRITECTRL);
+
+	for (i = 0; i < MT8173_NUM_SENSING_POINTS; i++)
+		writel(sensor_mux_values[cfg->sensors[i]],
+				mt->thermal_base + tempadcpnp_ofs[i]);
+
+	writel(cfg->enable_mask, mt->thermal_base + TEMPMONCTL0);
+
+	writel(TEMPADCWRITECTRL_ADC_PNP_WRITE | TEMPADCWRITECTRL_ADC_MUX_WRITE,
+			mt->thermal_base + TEMPADCWRITECTRL);
+
+	mtk_thermal_put_bank(bank);
+}
+
+static u64 of_get_phys_base(struct device_node *np)
+{
+	u64 size64;
+	const __be32 *regaddr_p;
+
+	regaddr_p = of_get_address(np, 0, &size64, NULL);
+	if (!regaddr_p)
+		return OF_BAD_ADDR;
+
+	return of_translate_address(np, regaddr_p);
+}
+
+static int mtk_thermal_probe(struct platform_device *pdev)
+{
+	int ret, i;
+	struct device_node *auxadc, *apmixedsys, *np = pdev->dev.of_node;
+	int irq;
+	struct mtk_thermal *mt;
+	struct resource *res;
+
+	mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
+	if (!mt)
+		return -ENOMEM;
+
+	mt->clk_peri_therm = devm_clk_get(&pdev->dev, "therm");
+	if (IS_ERR(mt->clk_peri_therm))
+		return PTR_ERR(mt->clk_peri_therm);
+
+	mt->clk_auxadc = devm_clk_get(&pdev->dev, "auxadc");
+	if (IS_ERR(mt->clk_auxadc))
+		return PTR_ERR(mt->clk_auxadc);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mt->thermal_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(mt->thermal_base))
+		return PTR_ERR(mt->thermal_base);
+
+	mt->reset = devm_reset_control_get(&pdev->dev, "therm");
+	if (IS_ERR(mt->reset)) {
+		ret = PTR_ERR(mt->reset);
+		dev_err(&pdev->dev, "cannot get reset: %d\n", ret);
+		return ret;
+	}
+
+	mutex_init(&mt->lock);
+
+	mt->dev = &pdev->dev;
+
+	auxadc = of_parse_phandle(np, "auxadc", 0);
+	if (!auxadc) {
+		dev_err(&pdev->dev, "missing auxadc node\n");
+		return -ENODEV;
+	}
+
+	mt->auxadc_phys_base = of_get_phys_base(auxadc);
+	if (mt->auxadc_phys_base == OF_BAD_ADDR) {
+		dev_err(&pdev->dev, "Can't get auxadc phys address\n");
+		return -EINVAL;
+	}
+
+	apmixedsys = of_parse_phandle(np, "apmixedsys", 0);
+	if (!apmixedsys) {
+		dev_err(&pdev->dev, "missing apmixedsys node\n");
+		return -ENODEV;
+	}
+
+	mt->apmixed_phys_base = of_get_phys_base(apmixedsys);
+	if (mt->apmixed_phys_base == OF_BAD_ADDR) {
+		dev_err(&pdev->dev, "Can't get auxadc phys address\n");
+		return -EINVAL;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (!irq) {
+		dev_err(&pdev->dev, "Can't find irq\n");
+		return -EINVAL;
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, mtk_thermal_irq,
+			IRQF_ONESHOT, THERMAL_NAME, mt);
+	if (ret) {
+		dev_err(&pdev->dev, "Can't request irq %d: %d\n", irq, ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(mt->clk_auxadc);
+	if (ret) {
+		dev_err(&pdev->dev, "Can't enable auxadc clk: %d\n", ret);
+		return ret;
+	}
+
+	reset_control_reset(mt->reset);
+
+	ret = clk_prepare_enable(mt->clk_peri_therm);
+	if (ret) {
+		dev_err(&pdev->dev, "Can't enable peri clk: %d\n", ret);
+		goto err_enable_clk;
+	}
+
+	/*
+	 * These calibration values should finally be provided by the
+	 * firmware or fuses. For now use default values.
+	 */
+	mt->adc_ge = ((512 - 512) * 10000) / 4096;
+	mt->adc_oe = 512 - 512;
+	mt->degc_cali = 40;
+	mt->o_slope = 0;
+	mt->vts = 260;
+
+	for (i = 0; i < MT8173_NUM_BANKS; i++) {
+		struct mtk_thermal_bank *bank = &mt->banks[i];
+
+		bank->id = i;
+		bank->mt = mt;
+		mtk_thermal_init_bank(&mt->banks[i]);
+	}
+
+	platform_set_drvdata(pdev, mt);
+
+	/*
+	 * This is needed after initialising the banks because otherwise
+	 * the first temperature read contains bogus high temperatures which
+	 * immediately cause a system shutdown.
+	 */
+	msleep(100);
+
+	for (i = 0; i < MT8173_NUM_BANKS; i++) {
+		struct mtk_thermal_bank *bank = &mt->banks[i];
+
+		bank->tz = thermal_zone_of_sensor_register(&pdev->dev, i, bank,
+				&mtk_thermal_ops);
+	}
+
+	return 0;
+
+err_enable_clk:
+	clk_disable_unprepare(mt->clk_peri_therm);
+
+	return ret;
+}
+
+static int mtk_thermal_remove(struct platform_device *pdev)
+{
+	struct mtk_thermal *mt = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < MT8173_NUM_BANKS; i++) {
+		struct mtk_thermal_bank *bank = &mt->banks[i];
+
+		if (!IS_ERR(bank))
+			thermal_zone_of_sensor_unregister(&pdev->dev, bank->tz);
+	}
+
+	clk_disable_unprepare(mt->clk_peri_therm);
+	clk_disable_unprepare(mt->clk_auxadc);
+
+	return 0;
+}
+
+static const struct of_device_id mtk_thermal_of_match[] = {
+	{
+		.compatible = "mediatek,mt8173-thermal",
+	}, {
+	},
+};
+
+static struct platform_driver mtk_thermal_driver = {
+	.probe = mtk_thermal_probe,
+	.remove = mtk_thermal_remove,
+	.driver = {
+		.name = THERMAL_NAME,
+		.of_match_table = mtk_thermal_of_match,
+	},
+};
+
+module_platform_driver(mtk_thermal_driver);
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 14/15] thermal: Add Mediatek thermal controller support
  2015-05-13  8:52 ` [PATCH 14/15] thermal: Add Mediatek thermal controller support Sascha Hauer
@ 2015-05-14  9:52   ` Paul Bolle
       [not found]     ` <CABicQ-XxXbMMnYyFAst=Xk1HMOXc6N1-J1bvyutMFY_=iNd0fg@mail.gmail.com>
  0 siblings, 1 reply; 34+ messages in thread
From: Paul Bolle @ 2015-05-14  9:52 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-pm, Zhang Rui, Eduardo Valentin, linux-kernel,
	Stephen Warren, Mikko Perttunen, kernel, linux-mediatek,
	linux-arm-kernel, Brian Norris

On Wed, 2015-05-13 at 10:52 +0200, Sascha Hauer wrote:
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig

> +config MTK_THERMAL
> +	tristate "Temperature sensor driver for mediatek SoCs"
> +	depends on ARCH_MEDIATEK || COMPILE_TEST
> +	default y
> +	help
> +	  Enable this option if you want to have support for thermal management
> +	  controller present in Mediatek SoCs

> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile

> +obj-$(CONFIG_MTK_THERMAL)	+= mtk_thermal.o

> --- /dev/null
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -0,0 +1,728 @@

> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.

> +#include <linux/module.h>

> +static const struct of_device_id mtk_thermal_of_match[] = {
> +	{
> +		.compatible = "mediatek,mt8173-thermal",
> +	}, {
> +	},
> +};
> +
> +static struct platform_driver mtk_thermal_driver = {
> +	.probe = mtk_thermal_probe,
> +	.remove = mtk_thermal_remove,
> +	.driver = {
> +		.name = THERMAL_NAME,
> +		.of_match_table = mtk_thermal_of_match,
> +	},
> +};
> +
> +module_platform_driver(mtk_thermal_driver);

If MTK_THERMAL is set to 'm' a mtk_thermal.ko module will be built. I
didn't spot a MODULE_LICENSE() macro in this file. So I think that, if
mtk_thermal.ko gets loaded - I guess through the of_match_table magic -
a warning will be issued and the kernel will be tainted.

(I didn't actually test any of this, so I've wasted your time if I've
overlooked something here.)


Paul Bolle


^ permalink raw reply	[flat|nested] 34+ messages in thread

[parent not found: <CABicQ-XxXbMMnYyFAst=Xk1HMOXc6N1-J1bvyutMFY_=iNd0fg@mail.gmail.com>]

* Re: [PATCH 14/15] thermal: Add Mediatek thermal controller support
       [not found]     ` <CABicQ-XxXbMMnYyFAst=Xk1HMOXc6N1-J1bvyutMFY_=iNd0fg@mail.gmail.com>
@ 2015-05-15  6:12       ` Sascha Hauer
  0 siblings, 0 replies; 34+ messages in thread
From: Sascha Hauer @ 2015-05-15  6:12 UTC (permalink / raw)
  To: Wei-Ning Huang
  Cc: Paul Bolle, Stephen Warren, linux-pm, linux-kernel,
	Eduardo Valentin, Mikko Perttunen, linux-mediatek, kernel,
	Zhang Rui, Brian Norris, linux-arm-kernel

Hi Wei-Ning,

On Fri, May 15, 2015 at 01:33:02PM +0800, Wei-Ning Huang wrote:
>    Hi Sascha,
>    I tried these patches but I didn't see any entries under
>    '/sys/class/thermal', am I missing something?
>    Wei-Ning

Please write a mail to me rather than to Paul if you want me to react.

You'll need a dts snippet that registers a thermal zone. Try the
attached patch. I haven't posted it because it only contains my testing
code which registers a fake gpio controlled fan.

Sascha

-----------------------------8<-----------------------------

>From 2ea6d94fe430c317064d06b46a8d0d2036ca4236 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Tue, 12 May 2015 09:24:25 +0200
Subject: [PATCH] ARM64: dts: mt8173-evb: debugging: Add fake thermal zone

This adds a thermal zone for the CPU which is controlled by a fan. No
fan exists on this board and the temperatures are bogus. This is only
for debugging purposes until the cpufreq driver is available.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm64/boot/dts/mediatek/mt8173-evb.dts | 39 +++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
index 43d5401..0a1f905 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
@@ -14,6 +14,7 @@
 
 /dts-v1/;
 #include "mt8173.dtsi"
+#include <dt-bindings/thermal/thermal.h>
 
 / {
 	model = "mediatek,mt8173-evb";
@@ -31,6 +32,44 @@
 	};
 
 	chosen { };
+
+	fan: gpio_fan {
+		compatible = "gpio-fan";
+		gpios = <&pio 24 0>;
+		gpio-fan,speed-map = <0    0
+				      4500 1>;
+		#cooling-cells = <2>;
+	};
+
+	thermal-zones {
+		cpu_thermal: cpu_thermal {
+			polling-delay-passive = <1000>; /* milliseconds */
+			polling-delay = <0>; /* milliseconds */
+
+			thermal-sensors = <&thermal 0>;
+
+			trips {
+				cpu_passive: cpu_passive {
+					temperature = <47000>; /* millicelsius */
+					hysteresis = <2000>; /* millicelsius */
+					type = "passive";
+				};
+
+				cpu_crit {
+					temperature = <90000>; /* millicelsius */
+					hysteresis = <2000>; /* millicelsius */
+					type = "critical";
+				};
+			};
+
+			cooling-maps {
+				map0 {
+					trip = <&cpu_passive>;
+					cooling-device = <&fan THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				};
+			};
+		};
+	};
 };
 
 &uart0 {
-- 
2.1.4


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 15/15] ARM64: dts: mt8173: Add thermal/auxadc device nodes
  2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
                   ` (13 preceding siblings ...)
  2015-05-13  8:52 ` [PATCH 14/15] thermal: Add Mediatek thermal controller support Sascha Hauer
@ 2015-05-13  8:52 ` Sascha Hauer
  14 siblings, 0 replies; 34+ messages in thread
From: Sascha Hauer @ 2015-05-13  8:52 UTC (permalink / raw)
  To: linux-pm
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, Stephen Warren,
	Mikko Perttunen, kernel, linux-mediatek, linux-arm-kernel,
	Brian Norris, Sascha Hauer

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 924fdb6..50d424f 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -147,6 +147,11 @@
 				(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
 		};
 
+		auxadc: auxadc@11001000 {
+			compatible = "mediatek,mt8173-auxadc";
+			reg = <0 0x11001000 0 0x1000>;
+		};
+
 		uart0: serial@11002000 {
 			compatible = "mediatek,mt8173-uart",
 					"mediatek,mt6577-uart";
@@ -182,6 +187,19 @@
 			clocks = <&uart_clk>;
 			status = "disabled";
 		};
+
+		thermal: thermal@1100b000 {
+			#thermal-sensor-cells = <1>;
+			compatible = "mediatek,mt8173-thermal";
+			reg = <0 0x1100b000 0 0x1000>;
+			interrupts = <0 70 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&pericfg CLK_PERI_THERM>, <&pericfg CLK_PERI_AUXADC>;
+			clock-names = "therm", "auxadc";
+			resets = <&pericfg MT8173_PERI_THERM_SW_RST>;
+			reset-names = "therm";
+			auxadc = <&auxadc>;
+			apmixedsys = <&apmixedsys>;
+		};
 	};
 
 };
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2015-05-20 13:23 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-13  8:52 [PATCH v3] Thermal hardware trip points and Mediatek thermal driver Sascha Hauer
2015-05-13  8:52 ` [PATCH 01/15] thermal: consistently use int for temperatures Sascha Hauer
2015-05-20  7:12   ` Mikko Perttunen
2015-05-20  8:34     ` Sascha Hauer
2015-05-20  8:42       ` Mikko Perttunen
2015-05-13  8:52 ` [PATCH 02/15] thermal: trivial: fix typo in comment Sascha Hauer
2015-05-13  8:52 ` [PATCH 03/15] thermal: remove useless call to thermal_zone_device_set_polling Sascha Hauer
2015-05-13  8:52 ` [PATCH 04/15] thermal: Use IS_ENABLED instead of #ifdef Sascha Hauer
2015-05-13  8:52 ` [PATCH 05/15] thermal: Add comment explaining test for critical temperature Sascha Hauer
2015-05-20  7:18   ` Mikko Perttunen
2015-05-13  8:52 ` [PATCH 06/15] thermal: inline only once used function Sascha Hauer
2015-05-20  7:28   ` Mikko Perttunen
2015-05-13  8:52 ` [PATCH 07/15] thermal: streamline get_trend callbacks Sascha Hauer
2015-05-13  8:52 ` [PATCH 08/15] thermal: Allow sensor ops to fail with -ENOSYS Sascha Hauer
2015-05-13  8:52 ` [PATCH 09/15] thermal: of: always set sensor related callbacks Sascha Hauer
2015-05-13  8:52 ` [PATCH 10/15] thermal: Make struct thermal_zone_device_ops const Sascha Hauer
2015-05-13  8:52 ` [PATCH 11/15] thermal: thermal: Add support for hardware-tracked trip points Sascha Hauer
2015-05-18  9:06   ` Mikko Perttunen
2015-05-18 12:09     ` Sascha Hauer
2015-05-18 18:44       ` Brian Norris
2015-05-18 19:13         ` Mikko Perttunen
2015-05-18 20:28           ` Brian Norris
2015-05-19 12:43             ` Mikko Perttunen
2015-05-19 14:05         ` Sascha Hauer
2015-05-19 13:58       ` Sascha Hauer
2015-05-19 14:05         ` Mikko Perttunen
2015-05-20 13:21           ` Sascha Hauer
2015-05-13  8:52 ` [PATCH 12/15] thermal: of: implement .set_trips for device tree thermal zones Sascha Hauer
2015-05-18  9:09   ` Mikko Perttunen
2015-05-13  8:52 ` [PATCH 13/15] dt-bindings: thermal: Add binding document for Mediatek thermal controller Sascha Hauer
2015-05-13  8:52 ` [PATCH 14/15] thermal: Add Mediatek thermal controller support Sascha Hauer
2015-05-14  9:52   ` Paul Bolle
     [not found]     ` <CABicQ-XxXbMMnYyFAst=Xk1HMOXc6N1-J1bvyutMFY_=iNd0fg@mail.gmail.com>
2015-05-15  6:12       ` Sascha Hauer
2015-05-13  8:52 ` [PATCH 15/15] ARM64: dts: mt8173: Add thermal/auxadc device nodes Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).