[PATCH] Clean up thermal API

[PATCH] Clean up thermal API

[Matthew Garrett]

The thermal layer passes temperatures around as strings. This is fine 
for sysfs, but makes it hard to use them for other purposes in-kernel. 
Change them to longs and do the string conversion in the sysfs-specific 
code.

Signed-off-by: Matthew Garrett <mjg@redhat.com>

---

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 504385b..28b3782 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -886,7 +886,8 @@ static void acpi_thermal_check(void *data)
 /* sys I/F for generic thermal sysfs support */
 #define KELVIN_TO_MILLICELSIUS(t) (t * 100 - 273200)
 
-static int thermal_get_temp(struct thermal_zone_device *thermal, char *buf)
+static int thermal_get_temp(struct thermal_zone_device *thermal, 
+			    unsigned long *temp)
 {
 	struct acpi_thermal *tz = thermal->devdata;
 	int result;
@@ -898,7 +899,8 @@ static int thermal_get_temp(struct thermal_zone_device *thermal, char *buf)
 	if (result)
 		return result;
 
-	return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(tz->temperature));
+	*temp = KELVIN_TO_MILLICELSIUS(tz->temperature);
+	return 0;
 }
 
 static const char enabled[] = "kernel";
@@ -982,7 +984,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, char *buf)
+				 int trip, unsigned long *temp)
 {
 	struct acpi_thermal *tz = thermal->devdata;
 	int i;
@@ -991,31 +993,40 @@ static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
 		return -EINVAL;
 
 	if (tz->trips.critical.flags.valid) {
-		if (!trip)
-			return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(
-				tz->trips.critical.temperature));
+		if (!trip) {
+			*temp = KELVIN_TO_MILLICELSIUS(
+				tz->trips.critical.temperature);
+			return 0;
+		}
+		
 		trip--;
 	}
 
 	if (tz->trips.hot.flags.valid) {
-		if (!trip)
-			return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(
-					tz->trips.hot.temperature));
+		if (!trip) {
+			*temp = KELVIN_TO_MILLICELSIUS(
+				tz->trips.hot.temperature);
+			return 0;
+		}
 		trip--;
 	}
 
 	if (tz->trips.passive.flags.valid) {
-		if (!trip)
-			return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(
-					tz->trips.passive.temperature));
+		if (!trip) {
+			*temp = KELVIN_TO_MILLICELSIUS(
+				tz->trips.passive.temperature);
+			return 0;
+		}
 		trip--;
 	}
 
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
 		tz->trips.active[i].flags.valid; i++) {
-		if (!trip)
-			return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(
-					tz->trips.active[i].temperature));
+		if (!trip) {
+			*temp = KELVIN_TO_MILLICELSIUS(
+				tz->trips.active[i].temperature);
+			return 0;
+		}
 		trip--;
 	}
 
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 6098787..c537a5b 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -104,11 +104,18 @@ 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;
 
 	if (!tz->ops->get_temp)
 		return -EPERM;
 
-	return tz->ops->get_temp(tz, buf);
+	ret = tz->ops->get_temp(tz,&temperature);
+
+	if (ret)
+		return ret;
+
+	return sprintf(buf,"%ld\n",temperature);
 }
 
 static ssize_t
@@ -160,7 +167,8 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
 		     char *buf)
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
-	int trip;
+	int trip, ret;
+	long temperature;
 
 	if (!tz->ops->get_trip_temp)
 		return -EPERM;
@@ -168,7 +176,12 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
 	if (!sscanf(attr->attr.name, "trip_point_%d_temp", &trip))
 		return -EINVAL;
 
-	return tz->ops->get_trip_temp(tz, trip, buf);
+	ret = tz->ops->get_trip_temp(tz, trip, &temperature);
+
+	if (ret)
+		return ret;
+
+	return sprintf (buf, "%ld\n", temperature);
 }
 
 static DEVICE_ATTR(type, 0444, type_show, NULL);
@@ -312,13 +325,20 @@ 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 ret;
 	struct thermal_hwmon_attr *hwmon_attr
 			= container_of(attr, struct thermal_hwmon_attr, attr);
 	struct thermal_zone_device *tz
 			= container_of(hwmon_attr, struct thermal_zone_device,
 				       temp_input);
 
-	return tz->ops->get_temp(tz, buf);
+	ret = tz->ops->get_temp(tz, &temperature);
+
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%ld\n", temperature);
 }
 
 static ssize_t
@@ -330,8 +350,14 @@ temp_crit_show(struct device *dev, struct device_attribute *attr,
 	struct thermal_zone_device *tz
 			= container_of(hwmon_attr, struct thermal_zone_device,
 				       temp_crit);
+	long temperature;
+	int ret;
+
+	ret = tz->ops->get_trip_temp(tz, 0, &temperature);
+	if (ret)
+		return ret;
 
-	return tz->ops->get_trip_temp(tz, 0, buf);
+	return sprintf(buf, "%ld\n", temperature);
 }
 
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 06d3e6e..63e6619 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -36,11 +36,12 @@ 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 *, char *);
+	int (*get_temp) (struct thermal_zone_device *, unsigned long *);
 	int (*get_mode) (struct thermal_zone_device *, char *);
 	int (*set_mode) (struct thermal_zone_device *, const char *);
 	int (*get_trip_type) (struct thermal_zone_device *, int, char *);
-	int (*get_trip_temp) (struct thermal_zone_device *, int, char *);
+	int (*get_trip_temp) (struct thermal_zone_device *, int, 
+			      unsigned long *);
 	int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
 };
 

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[PATCH] More cleanup of the thermal API

[Matthew Garrett]

The state management functions in the thermal API also pass strings 
around. Change them to ints and do the conversion at the sysfs point.

Signed-off-by: Matthew Garrett <mjg@redhat.com>

---

diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index 6cf10cb..f3db49e 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -69,27 +69,30 @@ static struct acpi_driver acpi_fan_driver = {
 };
 
 /* thermal cooling device callbacks */
-static int fan_get_max_state(struct thermal_cooling_device *cdev, char *buf)
+static int fan_get_max_state(struct thermal_cooling_device *cdev, unsigned int
+			     *state)
 {
 	/* ACPI fan device only support two states: ON/OFF */
-	return sprintf(buf, "1\n");
+	*state = 1;
+	return 0;
 }
 
-static int fan_get_cur_state(struct thermal_cooling_device *cdev, char *buf)
+static int fan_get_cur_state(struct thermal_cooling_device *cdev, unsigned int
+			     *state)
 {
 	struct acpi_device *device = cdev->devdata;
-	int state;
 	int result;
 
 	if (!device)
 		return -EINVAL;
 
-	result = acpi_bus_get_power(device->handle, &state);
+	result = acpi_bus_get_power(device->handle, state);
 	if (result)
 		return result;
 
-	return sprintf(buf, "%s\n", state == ACPI_STATE_D3 ? "0" :
-			 (state == ACPI_STATE_D0 ? "1" : "unknown"));
+	*state = (*state == ACPI_STATE_D3 ? 0 : 
+		 (*state == ACPI_STATE_D0 ? 1 : -1));
+	return 0;
 }
 
 static int
diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index ef34b18..2a3721d 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -374,7 +374,8 @@ static int acpi_processor_max_state(struct acpi_processor *pr)
 	return max_state;
 }
 static int
-processor_get_max_state(struct thermal_cooling_device *cdev, char *buf)
+processor_get_max_state(struct thermal_cooling_device *cdev, unsigned int 
+			*state)
 {
 	struct acpi_device *device = cdev->devdata;
 	struct acpi_processor *pr = acpi_driver_data(device);
@@ -382,24 +383,24 @@ processor_get_max_state(struct thermal_cooling_device *cdev, char *buf)
 	if (!device || !pr)
 		return -EINVAL;
 
-	return sprintf(buf, "%d\n", acpi_processor_max_state(pr));
+	*state = acpi_processor_max_state(pr);
+	return 0;
 }
 
 static int
-processor_get_cur_state(struct thermal_cooling_device *cdev, char *buf)
+processor_get_cur_state(struct thermal_cooling_device *cdev, unsigned int 
+			*cur_state)
 {
 	struct acpi_device *device = cdev->devdata;
 	struct acpi_processor *pr = acpi_driver_data(device);
-	int cur_state;
 
 	if (!device || !pr)
 		return -EINVAL;
 
-	cur_state = cpufreq_get_cur_state(pr->id);
+	*cur_state = cpufreq_get_cur_state(pr->id);
 	if (pr->flags.throttling)
-		cur_state += pr->throttling.state;
-
-	return sprintf(buf, "%d\n", cur_state);
+		*cur_state += pr->throttling.state;
+	return 0;
 }
 
 static int
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 5e5dda3..d8d7596 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -358,26 +358,30 @@ static struct output_properties acpi_output_properties = {
 
 
 /* thermal cooling device callbacks */
-static int video_get_max_state(struct thermal_cooling_device *cdev, char *buf)
+static int video_get_max_state(struct thermal_cooling_device *cdev, unsigned
+			       int *state)
 {
 	struct acpi_device *device = cdev->devdata;
 	struct acpi_video_device *video = acpi_driver_data(device);
 
-	return sprintf(buf, "%d\n", video->brightness->count - 3);
+	*state = video->brightness->count - 3;
+	return 0;
 }
 
-static int video_get_cur_state(struct thermal_cooling_device *cdev, char *buf)
+static int video_get_cur_state(struct thermal_cooling_device *cdev, unsigned
+			       int *state)
 {
 	struct acpi_device *device = cdev->devdata;
 	struct acpi_video_device *video = acpi_driver_data(device);
 	unsigned long level;
-	int state;
+	int offset;
 
 	acpi_video_device_lcd_get_level_current(video, &level);
-	for (state = 2; state < video->brightness->count; state++)
-		if (level == video->brightness->levels[state])
-			return sprintf(buf, "%d\n",
-				       video->brightness->count - state - 1);
+	for (offset = 2; offset < video->brightness->count; offset++)
+		if (level == video->brightness->levels[offset]) {
+			*state = video->brightness->count - offset - 1;
+			return 0;
+		}
 
 	return -EINVAL;
 }
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index c537a5b..02abaf0 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -249,8 +249,12 @@ thermal_cooling_device_max_state_show(struct device *dev,
 				      struct device_attribute *attr, char *buf)
 {
 	struct thermal_cooling_device *cdev = to_cooling_device(dev);
+	int state, ret;
 
-	return cdev->ops->get_max_state(cdev, buf);
+	ret = cdev->ops->get_max_state(cdev, &state);
+	if (ret)
+		return ret;
+	return sprintf(buf, "%d\n", state);
 }
 
 static ssize_t
@@ -258,8 +262,12 @@ thermal_cooling_device_cur_state_show(struct device *dev,
 				      struct device_attribute *attr, char *buf)
 {
 	struct thermal_cooling_device *cdev = to_cooling_device(dev);
+	int state, ret;
 
-	return cdev->ops->get_cur_state(cdev, buf);
+	ret = cdev->ops->get_cur_state(cdev, &state);
+	if (ret)
+		return ret;
+	return sprintf(buf, "%d\n", state);
 }
 
 static ssize_t
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 63e6619..5ddbd4f 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -46,8 +46,8 @@ struct thermal_zone_device_ops {
 };
 
 struct thermal_cooling_device_ops {
-	int (*get_max_state) (struct thermal_cooling_device *, char *);
-	int (*get_cur_state) (struct thermal_cooling_device *, char *);
+	int (*get_max_state) (struct thermal_cooling_device *, unsigned int *);
+	int (*get_cur_state) (struct thermal_cooling_device *, unsigned int *);
 	int (*set_cur_state) (struct thermal_cooling_device *, unsigned int);
 };

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[RFC] Implement thermal limiting in generic thermal class

[Matthew Garrett]

In the absence of an explicitly defined passive cooling zone any 
machine unable to manage its thermal profile through active cooling will 
reach its critical shutdown temperature and power off, resulting in 
potential data loss. Add support to the generic thermal class for 
initiating passive cooling at a temperature defaulting to just below the 
critical temperature, with this value being overridable by the admin via 
sysfs.

Signed-off-by: Matthew Garrett <mjg@redhat.com>

---

I've got bug reports from multiple users with a wide range of hardware 
that can't keep itself sufficiently cool under Linux. Whether this is 
bad hardware design, machines being used outside recommended thermal 
conditions or whatever, failing to do anything about this is resulting 
in data loss (one user is unable to even get through a Fedora install 
without his machine shutting down). There's no evidence that the low 
level of polling used by this patch (one temperature read per zone per 
10 seconds while the temperature is below the limit point) will 
interfere with any hardware. For all we know, this is how Windows 
implements it...

This sits on top of the previous two patches that clean up the internal 
thermal API.

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 70d68ce..5553b18 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -197,6 +197,10 @@ cdev[0-*]_trip_point		The trip point with which cdev[0-*] is associated in this
 				RO
 				Optional
 
+passive				If the thermal zone does not provide its own passive trip point, one
+				can be set here. Since there will be no hardware reporting in this
+				case, polling will be automatically enabled to support it.				
+
 ******************************
 * Cooling device  attributes *
 ******************************
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 02abaf0..1e4b77e 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -30,6 +30,7 @@
 #include <linux/idr.h>
 #include <linux/thermal.h>
 #include <linux/spinlock.h>
+#include <linux/timer.h>
 
 MODULE_AUTHOR("Zhang Rui");
 MODULE_DESCRIPTION("Generic thermal management sysfs support");
@@ -48,6 +49,9 @@ struct thermal_cooling_device_instance {
 	struct list_head node;
 };
 
+static struct timer_list poll_timer;
+static struct work_struct thermal_poll_queue;
+
 static DEFINE_IDR(thermal_tz_idr);
 static DEFINE_IDR(thermal_cdev_idr);
 static DEFINE_MUTEX(thermal_idr_lock);
@@ -115,7 +119,38 @@ temp_show(struct device *dev, struct device_attribute *attr, char *buf)
 	if (ret)
 		return ret;
 
-	return sprintf(buf,"%ld\n",temperature);
+	return sprintf(buf, "%ld\n", temperature);
+}
+
+static ssize_t
+passive_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct thermal_zone_device *tz = to_thermal_zone(dev);
+	return sprintf(buf, "%ld\n", tz->force_passive_temp);
+}
+
+static ssize_t
+passive_store(struct device *dev, struct device_attribute *attr,
+	      const char *buf, size_t count)
+{
+	struct thermal_zone_device *tz = to_thermal_zone(dev);
+	unsigned long temperature, critical_temp;
+	int ret = strict_strtoul(buf, 10, &temperature);
+
+	if (ret)
+		return ret;
+
+	ret = tz->ops->get_crit_temp(tz, &critical_temp);
+
+	if (ret)
+		return ret;
+
+	if (temperature > critical_temp)
+		return -EINVAL;
+
+	tz->force_passive_temp = temperature;
+
+	return count;
 }
 
 static ssize_t
@@ -187,6 +222,7 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
 static DEVICE_ATTR(type, 0444, type_show, NULL);
 static DEVICE_ATTR(temp, 0444, temp_show, NULL);
 static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
+static DEVICE_ATTR(passive, 0644, passive_show, passive_store);
 
 static struct device_attribute trip_point_attrs[] = {
 	__ATTR(trip_point_0_type, 0444, trip_point_type_show, NULL),
@@ -486,6 +522,83 @@ thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
 }
 #endif
 
+static void thermal_throttle_cpus(void)
+{
+	struct thermal_cooling_device *cdev;
+	list_for_each_entry(cdev, &thermal_cdev_list, node)
+		if (!strncmp(cdev->type, "Processor", 9))
+			cdev->throttle = 1;
+}
+
+static void thermal_poll(unsigned long data)
+{
+	schedule_work(&thermal_poll_queue);
+}
+
+static void thermal_update(struct work_struct *work)
+{
+	struct thermal_zone_device *tz;
+	struct thermal_cooling_device *cdev;
+	unsigned long temp;
+	int sleep_time = 10, max_state, state, cpus_throttled = 0;
+
+	if (list_empty(&thermal_cdev_list))
+		goto out;
+
+	list_for_each_entry(cdev, &thermal_cdev_list, node)
+		cdev->throttle = 0;
+
+	if (list_empty(&thermal_tz_list))
+		goto out;
+
+	list_for_each_entry(tz, &thermal_tz_list, node) {
+		if (!tz->force_passive)
+			continue;
+
+		tz->ops->get_temp(tz, &temp);
+
+		/* If the temperature trend is downwards, reduce throttling
+		   in an attempt to end up at a steady state */
+		if (temp > tz->force_passive_temp) {
+			if (((temp - tz->prev_temp) +
+			     (temp - tz->force_passive_temp)) > 0) {
+				if (list_empty(&tz->cooling_devices) &&
+				    !cpus_throttled) {
+					thermal_throttle_cpus();
+					cpus_throttled = 1;
+				} else
+					list_for_each_entry(cdev, 
+							    &tz->cooling_devices,
+							    node)
+						cdev->throttle = 1;
+			}
+		}
+		tz->prev_temp = temp;
+
+		/* Increase polling interval near the cut-off temperature */
+		if (temp > tz->force_passive_temp - 5000)
+			sleep_time = 1;
+	}
+
+	list_for_each_entry(cdev, &thermal_cdev_list, node) {
+		if (!strncmp(cdev->type, "Fan", 3))
+			continue;
+		cdev->ops->get_cur_state(cdev, &state);
+		if (cdev->throttle) {
+			cdev->ops->get_max_state(cdev, &max_state);
+			if (++state < max_state)
+				cdev->ops->set_cur_state(cdev, state);
+		} else
+			if (state > 0) {
+				cdev->ops->set_cur_state(cdev, --state);
+				sleep_time = 1;
+			}
+	}
+out:
+	poll_timer.function = thermal_poll;
+	poll_timer.expires = round_jiffies(jiffies + sleep_time*HZ);
+	add_timer(&poll_timer);
+}
 
 /**
  * thermal_zone_bind_cooling_device - bind a cooling device to a thermal zone
@@ -775,6 +888,7 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
 	struct thermal_cooling_device *pos;
 	int result;
 	int count;
+	char trip_type[THERMAL_NAME_LENGTH];
 
 	if (strlen(type) >= THERMAL_NAME_LENGTH)
 		return ERR_PTR(-EINVAL);
@@ -803,6 +917,7 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
 	tz->device.class = &thermal_class;
 	tz->devdata = devdata;
 	tz->trips = trips;
+	tz->force_passive = 1;
 	sprintf(tz->device.bus_id, "thermal_zone%d", tz->id);
 	result = device_register(&tz->device);
 	if (result) {
@@ -811,6 +926,12 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
 		return ERR_PTR(result);
 	}
 
+	for (count = 0; count < trips; count++) {
+		tz->ops->get_trip_type(tz, count, trip_type);
+		if (!strcmp(trip_type, "passive"))
+			tz->force_passive = 0;
+	}
+
 	/* sys I/F */
 	if (type) {
 		result = device_create_file(&tz->device, &dev_attr_type);
@@ -848,8 +969,26 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
 		}
 	mutex_unlock(&thermal_list_lock);
 
-	if (!result)
+	if (!result) {
+		if (tz->force_passive) {
+			unsigned long crit_temp;
+			tz->ops->get_crit_temp(tz, &crit_temp);
+			tz->force_passive_temp = crit_temp-5000;
+
+			result = device_create_file(&tz->device,
+						    &dev_attr_passive);
+			if (result)
+				goto unregister;
+
+			if (!timer_pending(&poll_timer)) {
+				poll_timer.function = thermal_poll;
+				poll_timer.expires = round_jiffies(jiffies
+								   +(HZ*10));
+				add_timer(&poll_timer);
+			}
+		}
 		return tz;
+	}
 
       unregister:
 	release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
@@ -910,6 +1049,9 @@ static int __init thermal_init(void)
 {
 	int result = 0;
 
+	init_timer(&poll_timer);
+	INIT_WORK(&thermal_poll_queue, thermal_update);
+
 	result = class_register(&thermal_class);
 	if (result) {
 		idr_destroy(&thermal_tz_idr);
@@ -922,6 +1064,7 @@ static int __init thermal_init(void)
 
 static void __exit thermal_exit(void)
 {
+	del_timer(&poll_timer);
 	class_unregister(&thermal_class);
 	idr_destroy(&thermal_tz_idr);
 	idr_destroy(&thermal_cdev_idr);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 5ddbd4f..d398cff 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -61,6 +61,7 @@ struct thermal_cooling_device {
 	void *devdata;
 	struct thermal_cooling_device_ops *ops;
 	struct list_head node;
+	bool throttle;
 };
 
 #define KELVIN_TO_CELSIUS(t)	(long)(((long)t-2732 >= 0) ?	\
@@ -102,6 +103,9 @@ struct thermal_zone_device {
 	struct thermal_hwmon_attr temp_input;	/* hwmon sys attr */
 	struct thermal_hwmon_attr temp_crit;	/* hwmon sys attr */
 #endif
+	unsigned long force_passive_temp;
+	unsigned long prev_temp;
+	bool force_passive;
 };
 
 struct thermal_zone_device *thermal_zone_device_register(char *, int, void *,


-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Clean up thermal API

[Zhang Rui]

On Wed, 2008-06-11 at 18:06 +0800, Matthew Garrett wrote:
> The thermal layer passes temperatures around as strings. This is fine
> for sysfs, but makes it hard to use them for other purposes in-kernel.
> Change them to longs and do the string conversion in the
> sysfs-specific
> code.
this patch looks okay,
except that it has some codingstyle problems and failed to pass
checkpatch.

thanks,
rui
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> 
> ---
> 
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 504385b..28b3782 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -886,7 +886,8 @@ static void acpi_thermal_check(void *data)
>  /* sys I/F for generic thermal sysfs support */
>  #define KELVIN_TO_MILLICELSIUS(t) (t * 100 - 273200)
> 
> -static int thermal_get_temp(struct thermal_zone_device *thermal, char
> *buf)
> +static int thermal_get_temp(struct thermal_zone_device *thermal,
> +                           unsigned long *temp)
>  {
>         struct acpi_thermal *tz = thermal->devdata;
>         int result;
> @@ -898,7 +899,8 @@ static int thermal_get_temp(struct
> thermal_zone_device *thermal, char *buf)
>         if (result)
>                 return result;
> 
> -       return sprintf(buf, "%ld\n",
> KELVIN_TO_MILLICELSIUS(tz->temperature));
> +       *temp = KELVIN_TO_MILLICELSIUS(tz->temperature);
> +       return 0;
>  }
> 
>  static const char enabled[] = "kernel";
> @@ -982,7 +984,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, char *buf)
> +                                int trip, unsigned long *temp)
>  {
>         struct acpi_thermal *tz = thermal->devdata;
>         int i;
> @@ -991,31 +993,40 @@ static int thermal_get_trip_temp(struct
> thermal_zone_device *thermal,
>                 return -EINVAL;
> 
>         if (tz->trips.critical.flags.valid) {
> -               if (!trip)
> -                       return sprintf(buf, "%ld\n",
> KELVIN_TO_MILLICELSIUS(
> -                               tz->trips.critical.temperature));
> +               if (!trip) {
> +                       *temp = KELVIN_TO_MILLICELSIUS(
> +                               tz->trips.critical.temperature);
> +                       return 0;
> +               }
> +              
>                 trip--;
>         }
> 
>         if (tz->trips.hot.flags.valid) {
> -               if (!trip)
> -                       return sprintf(buf, "%ld\n",
> KELVIN_TO_MILLICELSIUS(
> -                                       tz->trips.hot.temperature));
> +               if (!trip) {
> +                       *temp = KELVIN_TO_MILLICELSIUS(
> +                               tz->trips.hot.temperature);
> +                       return 0;
> +               }
>                 trip--;
>         }
> 
>         if (tz->trips.passive.flags.valid) {
> -               if (!trip)
> -                       return sprintf(buf, "%ld\n",
> KELVIN_TO_MILLICELSIUS(
> -
> tz->trips.passive.temperature));
> +               if (!trip) {
> +                       *temp = KELVIN_TO_MILLICELSIUS(
> +                               tz->trips.passive.temperature);
> +                       return 0;
> +               }
>                 trip--;
>         }
> 
>         for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
>                 tz->trips.active[i].flags.valid; i++) {
> -               if (!trip)
> -                       return sprintf(buf, "%ld\n",
> KELVIN_TO_MILLICELSIUS(
> -
> tz->trips.active[i].temperature));
> +               if (!trip) {
> +                       *temp = KELVIN_TO_MILLICELSIUS(
> +                               tz->trips.active[i].temperature);
> +                       return 0;
> +               }
>                 trip--;
>         }
> 
> diff --git a/drivers/thermal/thermal_sys.c
> b/drivers/thermal/thermal_sys.c
> index 6098787..c537a5b 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -104,11 +104,18 @@ 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;
> 
>         if (!tz->ops->get_temp)
>                 return -EPERM;
> 
> -       return tz->ops->get_temp(tz, buf);
> +       ret = tz->ops->get_temp(tz,&temperature);
> +
> +       if (ret)
> +               return ret;
> +
> +       return sprintf(buf,"%ld\n",temperature);
>  }
> 
>  static ssize_t
> @@ -160,7 +167,8 @@ trip_point_temp_show(struct device *dev, struct
> device_attribute *attr,
>                      char *buf)
>  {
>         struct thermal_zone_device *tz = to_thermal_zone(dev);
> -       int trip;
> +       int trip, ret;
> +       long temperature;
> 
>         if (!tz->ops->get_trip_temp)
>                 return -EPERM;
> @@ -168,7 +176,12 @@ trip_point_temp_show(struct device *dev, struct
> device_attribute *attr,
>         if (!sscanf(attr->attr.name, "trip_point_%d_temp", &trip))
>                 return -EINVAL;
> 
> -       return tz->ops->get_trip_temp(tz, trip, buf);
> +       ret = tz->ops->get_trip_temp(tz, trip, &temperature);
> +
> +       if (ret)
> +               return ret;
> +
> +       return sprintf (buf, "%ld\n", temperature);
>  }
> 
>  static DEVICE_ATTR(type, 0444, type_show, NULL);
> @@ -312,13 +325,20 @@ 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 ret;
>         struct thermal_hwmon_attr *hwmon_attr
>                         = container_of(attr, struct
> thermal_hwmon_attr, attr);
>         struct thermal_zone_device *tz
>                         = container_of(hwmon_attr, struct
> thermal_zone_device,
>                                        temp_input);
> 
> -       return tz->ops->get_temp(tz, buf);
> +       ret = tz->ops->get_temp(tz, &temperature);
> +
> +       if (ret)
> +               return ret;
> +
> +       return sprintf(buf, "%ld\n", temperature);
>  }
> 
>  static ssize_t
> @@ -330,8 +350,14 @@ temp_crit_show(struct device *dev, struct
> device_attribute *attr,
>         struct thermal_zone_device *tz
>                         = container_of(hwmon_attr, struct
> thermal_zone_device,
>                                        temp_crit);
> +       long temperature;
> +       int ret;
> +
> +       ret = tz->ops->get_trip_temp(tz, 0, &temperature);
> +       if (ret)
> +               return ret;
> 
> -       return tz->ops->get_trip_temp(tz, 0, buf);
> +       return sprintf(buf, "%ld\n", temperature);
>  }
> 
> 
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 06d3e6e..63e6619 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -36,11 +36,12 @@ 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 *, char *);
> +       int (*get_temp) (struct thermal_zone_device *, unsigned long
> *);
>         int (*get_mode) (struct thermal_zone_device *, char *);
>         int (*set_mode) (struct thermal_zone_device *, const char *);
>         int (*get_trip_type) (struct thermal_zone_device *, int, char
> *);
> -       int (*get_trip_temp) (struct thermal_zone_device *, int, char
> *);
> +       int (*get_trip_temp) (struct thermal_zone_device *, int,
> +                             unsigned long *);
>         int (*get_crit_temp) (struct thermal_zone_device *, unsigned
> long *);
>  };
> 
> 
> --
> Matthew Garrett | mjg59@srcf.ucam.org
> 
> 

Re: [PATCH] More cleanup of the thermal API

[Zhang Rui]

On Thu, 2008-06-12 at 00:42 +0800, Matthew Garrett wrote:
> The state management functions in the thermal API also pass strings
> around. Change them to ints and do the conversion at the sysfs point.
just some codingstyle problems. :)

thanks,
rui

> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> 
> ---
> 
> diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
> index 6cf10cb..f3db49e 100644
> --- a/drivers/acpi/fan.c
> +++ b/drivers/acpi/fan.c
> @@ -69,27 +69,30 @@ static struct acpi_driver acpi_fan_driver = {
>  };
> 
>  /* thermal cooling device callbacks */
> -static int fan_get_max_state(struct thermal_cooling_device *cdev,
> char *buf)
> +static int fan_get_max_state(struct thermal_cooling_device *cdev,
> unsigned int
> +                            *state)
>  {
>         /* ACPI fan device only support two states: ON/OFF */
> -       return sprintf(buf, "1\n");
> +       *state = 1;
> +       return 0;
>  }
> 
> -static int fan_get_cur_state(struct thermal_cooling_device *cdev,
> char *buf)
> +static int fan_get_cur_state(struct thermal_cooling_device *cdev,
> unsigned int
> +                            *state)
>  {
>         struct acpi_device *device = cdev->devdata;
> -       int state;
>         int result;
> 
>         if (!device)
>                 return -EINVAL;
> 
> -       result = acpi_bus_get_power(device->handle, &state);
> +       result = acpi_bus_get_power(device->handle, state);
>         if (result)
>                 return result;
> 
> -       return sprintf(buf, "%s\n", state == ACPI_STATE_D3 ? "0" :
> -                        (state == ACPI_STATE_D0 ? "1" : "unknown"));
> +       *state = (*state == ACPI_STATE_D3 ? 0 :
> +                (*state == ACPI_STATE_D0 ? 1 : -1));
> +       return 0;
>  }
> 
>  static int
> diff --git a/drivers/acpi/processor_thermal.c
> b/drivers/acpi/processor_thermal.c
> index ef34b18..2a3721d 100644
> --- a/drivers/acpi/processor_thermal.c
> +++ b/drivers/acpi/processor_thermal.c
> @@ -374,7 +374,8 @@ static int acpi_processor_max_state(struct
> acpi_processor *pr)
>         return max_state;
>  }
>  static int
> -processor_get_max_state(struct thermal_cooling_device *cdev, char
> *buf)
> +processor_get_max_state(struct thermal_cooling_device *cdev, unsigned
> int
> +                       *state)
>  {
>         struct acpi_device *device = cdev->devdata;
>         struct acpi_processor *pr = acpi_driver_data(device);
> @@ -382,24 +383,24 @@ processor_get_max_state(struct
> thermal_cooling_device *cdev, char *buf)
>         if (!device || !pr)
>                 return -EINVAL;
> 
> -       return sprintf(buf, "%d\n", acpi_processor_max_state(pr));
> +       *state = acpi_processor_max_state(pr);
> +       return 0;
>  }
> 
>  static int
> -processor_get_cur_state(struct thermal_cooling_device *cdev, char
> *buf)
> +processor_get_cur_state(struct thermal_cooling_device *cdev, unsigned
> int
> +                       *cur_state)
>  {
>         struct acpi_device *device = cdev->devdata;
>         struct acpi_processor *pr = acpi_driver_data(device);
> -       int cur_state;
> 
>         if (!device || !pr)
>                 return -EINVAL;
> 
> -       cur_state = cpufreq_get_cur_state(pr->id);
> +       *cur_state = cpufreq_get_cur_state(pr->id);
>         if (pr->flags.throttling)
> -               cur_state += pr->throttling.state;
> -
> -       return sprintf(buf, "%d\n", cur_state);
> +               *cur_state += pr->throttling.state;
> +       return 0;
>  }
> 
>  static int
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 5e5dda3..d8d7596 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -358,26 +358,30 @@ static struct output_properties
> acpi_output_properties = {
> 
> 
>  /* thermal cooling device callbacks */
> -static int video_get_max_state(struct thermal_cooling_device *cdev,
> char *buf)
> +static int video_get_max_state(struct thermal_cooling_device *cdev,
> unsigned
> +                              int *state)
>  {
>         struct acpi_device *device = cdev->devdata;
>         struct acpi_video_device *video = acpi_driver_data(device);
> 
> -       return sprintf(buf, "%d\n", video->brightness->count - 3);
> +       *state = video->brightness->count - 3;
> +       return 0;
>  }
> 
> -static int video_get_cur_state(struct thermal_cooling_device *cdev,
> char *buf)
> +static int video_get_cur_state(struct thermal_cooling_device *cdev,
> unsigned
> +                              int *state)
>  {
>         struct acpi_device *device = cdev->devdata;
>         struct acpi_video_device *video = acpi_driver_data(device);
>         unsigned long level;
> -       int state;
> +       int offset;
> 
>         acpi_video_device_lcd_get_level_current(video, &level);
> -       for (state = 2; state < video->brightness->count; state++)
> -               if (level == video->brightness->levels[state])
> -                       return sprintf(buf, "%d\n",
> -                                      video->brightness->count -
> state - 1);
> +       for (offset = 2; offset < video->brightness->count; offset++)
> +               if (level == video->brightness->levels[offset]) {
> +                       *state = video->brightness->count - offset -
> 1;
> +                       return 0;
> +               }
> 
>         return -EINVAL;
>  }
> diff --git a/drivers/thermal/thermal_sys.c
> b/drivers/thermal/thermal_sys.c
> index c537a5b..02abaf0 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -249,8 +249,12 @@ thermal_cooling_device_max_state_show(struct
> device *dev,
>                                       struct device_attribute *attr,
> char *buf)
>  {
>         struct thermal_cooling_device *cdev = to_cooling_device(dev);
> +       int state, ret;
> 
> -       return cdev->ops->get_max_state(cdev, buf);
> +       ret = cdev->ops->get_max_state(cdev, &state);
> +       if (ret)
> +               return ret;
> +       return sprintf(buf, "%d\n", state);
>  }
> 
>  static ssize_t
> @@ -258,8 +262,12 @@ thermal_cooling_device_cur_state_show(struct
> device *dev,
>                                       struct device_attribute *attr,
> char *buf)
>  {
>         struct thermal_cooling_device *cdev = to_cooling_device(dev);
> +       int state, ret;
> 
> -       return cdev->ops->get_cur_state(cdev, buf);
> +       ret = cdev->ops->get_cur_state(cdev, &state);
> +       if (ret)
> +               return ret;
> +       return sprintf(buf, "%d\n", state);
>  }
> 
>  static ssize_t
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 63e6619..5ddbd4f 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -46,8 +46,8 @@ struct thermal_zone_device_ops {
>  };
> 
>  struct thermal_cooling_device_ops {
> -       int (*get_max_state) (struct thermal_cooling_device *, char
> *);
> -       int (*get_cur_state) (struct thermal_cooling_device *, char
> *);
> +       int (*get_max_state) (struct thermal_cooling_device *,
> unsigned int *);
> +       int (*get_cur_state) (struct thermal_cooling_device *,
> unsigned int *);
>         int (*set_cur_state) (struct thermal_cooling_device *,
> unsigned int);
>  };
> 
> --
> Matthew Garrett | mjg59@srcf.ucam.org
> 
> 

Re: [RFC] Implement thermal limiting in generic thermal class

[Zhang Rui]

On Thu, 2008-06-12 at 00:58 +0800, Matthew Garrett wrote:
> In the absence of an explicitly defined passive cooling zone any
> machine unable to manage its thermal profile through active cooling
> will
> reach its critical shutdown temperature and power off, resulting in
> potential data loss. Add support to the generic thermal class for
> initiating passive cooling at a temperature defaulting to just below
> the
> critical temperature, with this value being overridable by the admin
> via
> sysfs.

this patch introduce two things that are obsolete in ACPI thermal
driver, polling and trip point overriding.
I'm under the impression that they are known to break some laptops for
some reason. Len may have comments on this? :p

I'd say I like the idea which make Linux more robust when overheating IF
and ONLY IF it's okay to use polling.

> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> 
> ---
> 
> I've got bug reports from multiple users with a wide range of hardware
> that can't keep itself sufficiently cool under Linux. Whether this is
> bad hardware design, machines being used outside recommended thermal
> conditions or whatever, failing to do anything about this is resulting
> in data loss (one user is unable to even get through a Fedora install
> without his machine shutting down). There's no evidence that the low
> level of polling used by this patch (one temperature read per zone per
> 10 seconds while the temperature is below the limit point) will
> interfere with any hardware. For all we know, this is how Windows
> implements it...
> 
> This sits on top of the previous two patches that clean up the
> internal
> thermal API.

> +static void thermal_update(struct work_struct *work)
> +{
> +       struct thermal_zone_device *tz;
> +       struct thermal_cooling_device *cdev;
> +       unsigned long temp;
> +       int sleep_time = 10, max_state, state, cpus_throttled = 0;
> +
> +       if (list_empty(&thermal_cdev_list))
> +               goto out;
> +
> +       list_for_each_entry(cdev, &thermal_cdev_list, node)
> +               cdev->throttle = 0;
> +
> +       if (list_empty(&thermal_tz_list))
> +               goto out;
> +
> +       list_for_each_entry(tz, &thermal_tz_list, node) {
> +               if (!tz->force_passive)
> +                       continue;
> +
> +               tz->ops->get_temp(tz, &temp);
> +
> +               /* If the temperature trend is downwards, reduce
> throttling
> +                  in an attempt to end up at a steady state */
> +               if (temp > tz->force_passive_temp) {
> +                       if (((temp - tz->prev_temp) +
> +                            (temp - tz->force_passive_temp)) > 0) {
> +                               if (list_empty(&tz->cooling_devices)
> &&
> +                                   !cpus_throttled) {
> +                                       thermal_throttle_cpus();
> +                                       cpus_throttled = 1;
> +                               } else
> +                                       list_for_each_entry(cdev,
> +
> &tz->cooling_devices,
> +                                                           node)
> +                                               cdev->throttle = 1;
> +                       }
> +               }
> +               tz->prev_temp = temp;
> +
> +               /* Increase polling interval near the cut-off
> temperature */
> +               if (temp > tz->force_passive_temp - 5000)
> +                       sleep_time = 1;
> +       }
> +
> +       list_for_each_entry(cdev, &thermal_cdev_list, node) {
> +               if (!strncmp(cdev->type, "Fan", 3))
> +                       continue;
> +               cdev->ops->get_cur_state(cdev, &state);
> +               if (cdev->throttle) {
> +                       cdev->ops->get_max_state(cdev, &max_state);
> +                       if (++state < max_state)
> +                               cdev->ops->set_cur_state(cdev, state);
> +               } else
> +                       if (state > 0) {
> +                               cdev->ops->set_cur_state(cdev,
> --state);
> +                               sleep_time = 1;
> +                       }
as cdev->throttle is cleared by default, this may affect the cooling
devices in the thermal zones that don't enable passive cooling.

> +       }
> +out:
> +       poll_timer.function = thermal_poll;
> +       poll_timer.expires = round_jiffies(jiffies + sleep_time*HZ);
> +       add_timer(&poll_timer);
> +}
> 
>  /**
>   * thermal_zone_bind_cooling_device - bind a cooling device to a
> thermal zone
> @@ -775,6 +888,7 @@ struct thermal_zone_device
> *thermal_zone_device_register(char *type,
>         struct thermal_cooling_device *pos;
>         int result;
>         int count;
> +       char trip_type[THERMAL_NAME_LENGTH];
> 
>         if (strlen(type) >= THERMAL_NAME_LENGTH)
>                 return ERR_PTR(-EINVAL);
> @@ -803,6 +917,7 @@ struct thermal_zone_device
> *thermal_zone_device_register(char *type,
>         tz->device.class = &thermal_class;
>         tz->devdata = devdata;
>         tz->trips = trips;
> +       tz->force_passive = 1;
>         sprintf(tz->device.bus_id, "thermal_zone%d", tz->id);
>         result = device_register(&tz->device);
>         if (result) {
> @@ -811,6 +926,12 @@ struct thermal_zone_device
> *thermal_zone_device_register(char *type,
>                 return ERR_PTR(result);
>         }
> 
> +       for (count = 0; count < trips; count++) {
> +               tz->ops->get_trip_type(tz, count, trip_type);
> +               if (!strcmp(trip_type, "passive"))
> +                       tz->force_passive = 0;
> +       }
> +
Hmm, I don't think we should enable the force_passive by default.
IMO, we should just create "passive" attribute for thermal zones without
a passive trip point, and enable it until user set a valid value.

thanks,
rui

Re: [RFC] Implement thermal limiting in generic thermal class

[Matthew Garrett]

On Thu, Jun 12, 2008 at 10:25:34AM +0800, Zhang Rui wrote:

> this patch introduce two things that are obsolete in ACPI thermal
> driver, polling and trip point overriding.

Overriding active trip points was pretty much doomed to failure, given 
that the platform can redefine them at any time.

> I'm under the impression that they are known to break some laptops for
> some reason. Len may have comments on this? :p

Polling is known to break in one case, where the laptop provides a 
passive cooling zone that's at around 50 degrees C. The hardware doesn't 
send any notifications, but if you enable polling thermal limiting will 
start at an excessively low temperature. This patch won't affect that 
case. I'm not aware of any other situations where polling has been 
demonstrated to break things, especially when at a frequency as low as 
this. Of course, it's possible that some machines take an excessively 
long time to respond to temperature reads - that's something that could 
be handled without any trouble.

> as cdev->throttle is cleared by default, this may affect the cooling
> devices in the thermal zones that don't enable passive cooling.

Hm, yes, that's a good point and needs rectifying.

> Hmm, I don't think we should enable the force_passive by default.
> IMO, we should just create "passive" attribute for thermal zones without
> a passive trip point, and enable it until user set a valid value.

That would be an option (and certainly better than the current 
situation), but I'd prefer systems to work out of the box where 
possible.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[PATCH v2] Clean up thermal API

[Matthew Garrett]

The thermal layer passes temperatures around as strings. This is fine 
for sysfs, but makes it hard to use them for other purposes in-kernel. 
Change them to longs and do the string conversion in the sysfs-specific 
code.

Signed-off-by: Matthew Garrett <mjg@redhat.com>

---

This incorporates both the previous patches and has the whitespace fixes

diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index 6cf10cb..490a998 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -69,27 +69,30 @@ static struct acpi_driver acpi_fan_driver = {
 };
 
 /* thermal cooling device callbacks */
-static int fan_get_max_state(struct thermal_cooling_device *cdev, char *buf)
+static int fan_get_max_state(struct thermal_cooling_device *cdev, unsigned int
+			     *state)
 {
 	/* ACPI fan device only support two states: ON/OFF */
-	return sprintf(buf, "1\n");
+	*state = 1;
+	return 0;
 }
 
-static int fan_get_cur_state(struct thermal_cooling_device *cdev, char *buf)
+static int fan_get_cur_state(struct thermal_cooling_device *cdev, unsigned int
+			     *state)
 {
 	struct acpi_device *device = cdev->devdata;
-	int state;
 	int result;
 
 	if (!device)
 		return -EINVAL;
 
-	result = acpi_bus_get_power(device->handle, &state);
+	result = acpi_bus_get_power(device->handle, state);
 	if (result)
 		return result;
 
-	return sprintf(buf, "%s\n", state == ACPI_STATE_D3 ? "0" :
-			 (state == ACPI_STATE_D0 ? "1" : "unknown"));
+	*state = (*state == ACPI_STATE_D3 ? 0 :
+		 (*state == ACPI_STATE_D0 ? 1 : -1));
+	return 0;
 }
 
 static int
diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index ef34b18..4bc094c 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -374,7 +374,8 @@ static int acpi_processor_max_state(struct acpi_processor *pr)
 	return max_state;
 }
 static int
-processor_get_max_state(struct thermal_cooling_device *cdev, char *buf)
+processor_get_max_state(struct thermal_cooling_device *cdev, unsigned int
+			*state)
 {
 	struct acpi_device *device = cdev->devdata;
 	struct acpi_processor *pr = acpi_driver_data(device);
@@ -382,24 +383,24 @@ processor_get_max_state(struct thermal_cooling_device *cdev, char *buf)
 	if (!device || !pr)
 		return -EINVAL;
 
-	return sprintf(buf, "%d\n", acpi_processor_max_state(pr));
+	*state = acpi_processor_max_state(pr);
+	return 0;
 }
 
 static int
-processor_get_cur_state(struct thermal_cooling_device *cdev, char *buf)
+processor_get_cur_state(struct thermal_cooling_device *cdev, unsigned int
+			*cur_state)
 {
 	struct acpi_device *device = cdev->devdata;
 	struct acpi_processor *pr = acpi_driver_data(device);
-	int cur_state;
 
 	if (!device || !pr)
 		return -EINVAL;
 
-	cur_state = cpufreq_get_cur_state(pr->id);
+	*cur_state = cpufreq_get_cur_state(pr->id);
 	if (pr->flags.throttling)
-		cur_state += pr->throttling.state;
-
-	return sprintf(buf, "%d\n", cur_state);
+		*cur_state += pr->throttling.state;
+	return 0;
 }
 
 static int
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 504385b..bd6072d 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -886,7 +886,8 @@ static void acpi_thermal_check(void *data)
 /* sys I/F for generic thermal sysfs support */
 #define KELVIN_TO_MILLICELSIUS(t) (t * 100 - 273200)
 
-static int thermal_get_temp(struct thermal_zone_device *thermal, char *buf)
+static int thermal_get_temp(struct thermal_zone_device *thermal,
+			    unsigned long *temp)
 {
 	struct acpi_thermal *tz = thermal->devdata;
 	int result;
@@ -898,7 +899,8 @@ static int thermal_get_temp(struct thermal_zone_device *thermal, char *buf)
 	if (result)
 		return result;
 
-	return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(tz->temperature));
+	*temp = KELVIN_TO_MILLICELSIUS(tz->temperature);
+	return 0;
 }
 
 static const char enabled[] = "kernel";
@@ -982,7 +984,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, char *buf)
+				 int trip, unsigned long *temp)
 {
 	struct acpi_thermal *tz = thermal->devdata;
 	int i;
@@ -991,31 +993,40 @@ static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
 		return -EINVAL;
 
 	if (tz->trips.critical.flags.valid) {
-		if (!trip)
-			return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(
-				tz->trips.critical.temperature));
+		if (!trip) {
+			*temp = KELVIN_TO_MILLICELSIUS(
+				tz->trips.critical.temperature);
+			return 0;
+		}
+
 		trip--;
 	}
 
 	if (tz->trips.hot.flags.valid) {
-		if (!trip)
-			return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(
-					tz->trips.hot.temperature));
+		if (!trip) {
+			*temp = KELVIN_TO_MILLICELSIUS(
+				tz->trips.hot.temperature);
+			return 0;
+		}
 		trip--;
 	}
 
 	if (tz->trips.passive.flags.valid) {
-		if (!trip)
-			return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(
-					tz->trips.passive.temperature));
+		if (!trip) {
+			*temp = KELVIN_TO_MILLICELSIUS(
+				tz->trips.passive.temperature);
+			return 0;
+		}
 		trip--;
 	}
 
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
 		tz->trips.active[i].flags.valid; i++) {
-		if (!trip)
-			return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(
-					tz->trips.active[i].temperature));
+		if (!trip) {
+			*temp = KELVIN_TO_MILLICELSIUS(
+				tz->trips.active[i].temperature);
+			return 0;
+		}
 		trip--;
 	}
 
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 5e5dda3..d8d7596 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -358,26 +358,30 @@ static struct output_properties acpi_output_properties = {
 
 
 /* thermal cooling device callbacks */
-static int video_get_max_state(struct thermal_cooling_device *cdev, char *buf)
+static int video_get_max_state(struct thermal_cooling_device *cdev, unsigned
+			       int *state)
 {
 	struct acpi_device *device = cdev->devdata;
 	struct acpi_video_device *video = acpi_driver_data(device);
 
-	return sprintf(buf, "%d\n", video->brightness->count - 3);
+	*state = video->brightness->count - 3;
+	return 0;
 }
 
-static int video_get_cur_state(struct thermal_cooling_device *cdev, char *buf)
+static int video_get_cur_state(struct thermal_cooling_device *cdev, unsigned
+			       int *state)
 {
 	struct acpi_device *device = cdev->devdata;
 	struct acpi_video_device *video = acpi_driver_data(device);
 	unsigned long level;
-	int state;
+	int offset;
 
 	acpi_video_device_lcd_get_level_current(video, &level);
-	for (state = 2; state < video->brightness->count; state++)
-		if (level == video->brightness->levels[state])
-			return sprintf(buf, "%d\n",
-				       video->brightness->count - state - 1);
+	for (offset = 2; offset < video->brightness->count; offset++)
+		if (level == video->brightness->levels[offset]) {
+			*state = video->brightness->count - offset - 1;
+			return 0;
+		}
 
 	return -EINVAL;
 }
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 6098787..7aaa8e3 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -104,11 +104,18 @@ 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;
 
 	if (!tz->ops->get_temp)
 		return -EPERM;
 
-	return tz->ops->get_temp(tz, buf);
+	ret = tz->ops->get_temp(tz, &temperature);
+
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%ld\n", temperature);
 }
 
 static ssize_t
@@ -160,7 +167,8 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
 		     char *buf)
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
-	int trip;
+	int trip, ret;
+	long temperature;
 
 	if (!tz->ops->get_trip_temp)
 		return -EPERM;
@@ -168,7 +176,12 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
 	if (!sscanf(attr->attr.name, "trip_point_%d_temp", &trip))
 		return -EINVAL;
 
-	return tz->ops->get_trip_temp(tz, trip, buf);
+	ret = tz->ops->get_trip_temp(tz, trip, &temperature);
+
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%ld\n", temperature);
 }
 
 static DEVICE_ATTR(type, 0444, type_show, NULL);
@@ -236,8 +249,12 @@ thermal_cooling_device_max_state_show(struct device *dev,
 				      struct device_attribute *attr, char *buf)
 {
 	struct thermal_cooling_device *cdev = to_cooling_device(dev);
+	int state, ret;
 
-	return cdev->ops->get_max_state(cdev, buf);
+	ret = cdev->ops->get_max_state(cdev, &state);
+	if (ret)
+		return ret;
+	return sprintf(buf, "%d\n", state);
 }
 
 static ssize_t
@@ -245,8 +262,12 @@ thermal_cooling_device_cur_state_show(struct device *dev,
 				      struct device_attribute *attr, char *buf)
 {
 	struct thermal_cooling_device *cdev = to_cooling_device(dev);
+	int state, ret;
 
-	return cdev->ops->get_cur_state(cdev, buf);
+	ret = cdev->ops->get_cur_state(cdev, &state);
+	if (ret)
+		return ret;
+	return sprintf(buf, "%d\n", state);
 }
 
 static ssize_t
@@ -312,13 +333,20 @@ 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 ret;
 	struct thermal_hwmon_attr *hwmon_attr
 			= container_of(attr, struct thermal_hwmon_attr, attr);
 	struct thermal_zone_device *tz
 			= container_of(hwmon_attr, struct thermal_zone_device,
 				       temp_input);
 
-	return tz->ops->get_temp(tz, buf);
+	ret = tz->ops->get_temp(tz, &temperature);
+
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%ld\n", temperature);
 }
 
 static ssize_t
@@ -330,8 +358,14 @@ temp_crit_show(struct device *dev, struct device_attribute *attr,
 	struct thermal_zone_device *tz
 			= container_of(hwmon_attr, struct thermal_zone_device,
 				       temp_crit);
+	long temperature;
+	int ret;
+
+	ret = tz->ops->get_trip_temp(tz, 0, &temperature);
+	if (ret)
+		return ret;
 
-	return tz->ops->get_trip_temp(tz, 0, buf);
+	return sprintf(buf, "%ld\n", temperature);
 }
 
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 06d3e6e..5185781 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -36,17 +36,18 @@ 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 *, char *);
+	int (*get_temp) (struct thermal_zone_device *, unsigned long *);
 	int (*get_mode) (struct thermal_zone_device *, char *);
 	int (*set_mode) (struct thermal_zone_device *, const char *);
 	int (*get_trip_type) (struct thermal_zone_device *, int, char *);
-	int (*get_trip_temp) (struct thermal_zone_device *, int, char *);
+	int (*get_trip_temp) (struct thermal_zone_device *, int,
+			      unsigned long *);
 	int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
 };
 
 struct thermal_cooling_device_ops {
-	int (*get_max_state) (struct thermal_cooling_device *, char *);
-	int (*get_cur_state) (struct thermal_cooling_device *, char *);
+	int (*get_max_state) (struct thermal_cooling_device *, unsigned int *);
+	int (*get_cur_state) (struct thermal_cooling_device *, unsigned int *);
 	int (*set_cur_state) (struct thermal_cooling_device *, unsigned int);
 };

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[Patch v2] Implement thermal limiting in the generic thermal class

[Matthew Garrett]

In the absence of an explicitly defined passive cooling zone any                                                                                                                                        
machine unable to manage its thermal profile through active cooling will                                                                                                                                
reach its critical shutdown temperature and power off, resulting in                                                                                                                                     
potential data loss. Add support to the generic thermal class for                                                                                                                                       
initiating passive cooling at a temperature defaulting to just below the                                                                                                                                
critical temperature, with this value being overridable by the admin via                                                                                                                                
sysfs.                                                                                                                                                                                                  
                                                                                                                                                                                                        
Signed-off-by: Matthew Garrett <mjg@redhat.com>                                                                                                                                                         
                                                                                  
---

This version avoids the case where the passive code interferes with 
devices that have been throttled in response to active trip points.

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 70d68ce..5553b18 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -197,6 +197,10 @@ cdev[0-*]_trip_point		The trip point with which cdev[0-*] is associated in this
 				RO
 				Optional
 
+passive				If the thermal zone does not provide its own passive trip point, one
+				can be set here. Since there will be no hardware reporting in this
+				case, polling will be automatically enabled to support it.				
+
 ******************************
 * Cooling device  attributes *
 ******************************
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 7aaa8e3..4853f79 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -30,6 +30,7 @@
 #include <linux/idr.h>
 #include <linux/thermal.h>
 #include <linux/spinlock.h>
+#include <linux/timer.h>
 
 MODULE_AUTHOR("Zhang Rui");
 MODULE_DESCRIPTION("Generic thermal management sysfs support");
@@ -48,6 +49,9 @@ struct thermal_cooling_device_instance {
 	struct list_head node;
 };
 
+static struct timer_list poll_timer;
+static struct work_struct thermal_poll_queue;
+
 static DEFINE_IDR(thermal_tz_idr);
 static DEFINE_IDR(thermal_cdev_idr);
 static DEFINE_MUTEX(thermal_idr_lock);
@@ -119,6 +123,36 @@ temp_show(struct device *dev, struct device_attribute *attr, char *buf)
 }
 
 static ssize_t
+passive_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct thermal_zone_device *tz = to_thermal_zone(dev);
+	return sprintf(buf, "%ld\n", tz->force_passive_temp);
+}
+
+static ssize_t
+passive_store(struct device *dev, struct device_attribute *attr,
+	      const char *buf, size_t count)
+{
+	struct thermal_zone_device *tz = to_thermal_zone(dev);
+	unsigned long temperature, critical_temp;
+	int ret = strict_strtoul(buf, 10, &temperature);
+
+	if (ret)
+		return ret;
+
+	ret = tz->ops->get_crit_temp(tz, &critical_temp);
+
+	if (ret)
+		return ret;
+	if (temperature > critical_temp)
+		return -EINVAL;
+
+	tz->force_passive_temp = temperature;
+
+	return count;
+}
+
+static ssize_t
 mode_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
@@ -187,6 +221,7 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
 static DEVICE_ATTR(type, 0444, type_show, NULL);
 static DEVICE_ATTR(temp, 0444, temp_show, NULL);
 static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
+static DEVICE_ATTR(passive, 0644, passive_show, passive_store);
 
 static struct device_attribute trip_point_attrs[] = {
 	__ATTR(trip_point_0_type, 0444, trip_point_type_show, NULL),
@@ -486,6 +521,86 @@ thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
 }
 #endif
 
+static void thermal_throttle_cpus(void)
+{
+	struct thermal_cooling_device *cdev;
+	list_for_each_entry(cdev, &thermal_cdev_list, node)
+		if (!strncmp(cdev->type, "Processor", 9))
+			cdev->throttle = 1;
+}
+
+static void thermal_poll(unsigned long data)
+{
+	schedule_work(&thermal_poll_queue);
+}
+
+static void thermal_update(struct work_struct *work)
+{
+	struct thermal_zone_device *tz;
+	struct thermal_cooling_device *cdev;
+	unsigned long temp;
+	int sleep_time = 10, max_state, state, cpus_throttled = 0;
+
+	if (list_empty(&thermal_cdev_list))
+		goto out;
+
+	list_for_each_entry(cdev, &thermal_cdev_list, node)
+		cdev->throttle = 0;
+
+	if (list_empty(&thermal_tz_list))
+		goto out;
+
+	list_for_each_entry(tz, &thermal_tz_list, node) {
+		if (!tz->force_passive)
+			continue;
+
+		tz->ops->get_temp(tz, &temp);
+
+		/* If the temperature trend is downwards, reduce throttling
+		   in an attempt to end up at a steady state */
+		if (temp > tz->force_passive_temp) {
+			if (((temp - tz->prev_temp) +
+			     (temp - tz->force_passive_temp)) > 0) {
+				if (list_empty(&tz->cooling_devices) &&
+				    !cpus_throttled) {
+					thermal_throttle_cpus();
+					cpus_throttled = 1;
+				} else
+					list_for_each_entry(cdev,
+							    &tz->cooling_devices,
+							    node)
+						cdev->throttle = 1;
+			}
+		}
+		tz->prev_temp = temp;
+
+		/* Increase polling interval near the cut-off temperature */
+		if (temp > tz->force_passive_temp - 5000)
+			sleep_time = 1;
+	}
+
+	list_for_each_entry(cdev, &thermal_cdev_list, node) {
+		if (!strncmp(cdev->type, "Fan", 3))
+			continue;
+		cdev->ops->get_cur_state(cdev, &state);
+		if (cdev->throttle) {
+			cdev->passively_throttled = 1;
+			cdev->ops->get_max_state(cdev, &max_state);
+			if (++state < max_state)
+				cdev->ops->set_cur_state(cdev, state);
+		} else if (cdev->passively_throttled)
+			if (state > 0) {
+				cdev->ops->set_cur_state(cdev, --state);
+				sleep_time = 1;
+				if (state == 0)
+					cdev->passively_throttled = 0;
+			}
+	}
+out:
+	poll_timer.function = thermal_poll;
+	poll_timer.expires = round_jiffies(jiffies + sleep_time*HZ);
+	add_timer(&poll_timer);
+}
 
 /**
  * thermal_zone_bind_cooling_device - bind a cooling device to a thermal zone
@@ -775,6 +890,7 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
 	struct thermal_cooling_device *pos;
 	int result;
 	int count;
+	char trip_type[THERMAL_NAME_LENGTH];
 
 	if (strlen(type) >= THERMAL_NAME_LENGTH)
 		return ERR_PTR(-EINVAL);
@@ -803,6 +919,7 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
 	tz->device.class = &thermal_class;
 	tz->devdata = devdata;
 	tz->trips = trips;
+	tz->force_passive = 1;
 	sprintf(tz->device.bus_id, "thermal_zone%d", tz->id);
 	result = device_register(&tz->device);
 	if (result) {
@@ -811,6 +928,12 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
 		return ERR_PTR(result);
 	}
 
+	for (count = 0; count < trips; count++) {
+		tz->ops->get_trip_type(tz, count, trip_type);
+		if (!strcmp(trip_type, "passive"))
+			tz->force_passive = 0;
+	}
+
 	/* sys I/F */
 	if (type) {
 		result = device_create_file(&tz->device, &dev_attr_type);
@@ -848,8 +971,26 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
 		}
 	mutex_unlock(&thermal_list_lock);
 
-	if (!result)
+	if (!result) {
+		if (tz->force_passive) {
+			unsigned long crit_temp;
+			tz->ops->get_crit_temp(tz, &crit_temp);
+			tz->force_passive_temp = crit_temp-5000;
+
+			result = device_create_file(&tz->device,
+						    &dev_attr_passive);
+			if (result)
+				goto unregister;
+
+			if (!timer_pending(&poll_timer)) {
+				poll_timer.function = thermal_poll;
+				poll_timer.expires = round_jiffies(jiffies
+								   +(HZ*10));
+				add_timer(&poll_timer);
+			}
+		}
 		return tz;
+	}
 
       unregister:
 	release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
@@ -910,6 +1051,9 @@ static int __init thermal_init(void)
 {
 	int result = 0;
 
+	init_timer(&poll_timer);
+	INIT_WORK(&thermal_poll_queue, thermal_update);
+
 	result = class_register(&thermal_class);
 	if (result) {
 		idr_destroy(&thermal_tz_idr);
@@ -922,6 +1066,7 @@ static int __init thermal_init(void)
 
 static void __exit thermal_exit(void)
 {
+	del_timer(&poll_timer);
 	class_unregister(&thermal_class);
 	idr_destroy(&thermal_tz_idr);
 	idr_destroy(&thermal_cdev_idr);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 5185781..d1ab35a 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -61,6 +61,8 @@ struct thermal_cooling_device {
 	void *devdata;
 	struct thermal_cooling_device_ops *ops;
 	struct list_head node;
+	bool throttle;
+	bool passively_throttled;
 };
 
 #define KELVIN_TO_CELSIUS(t)	(long)(((long)t-2732 >= 0) ?	\
@@ -102,6 +104,9 @@ struct thermal_zone_device {
 	struct thermal_hwmon_attr temp_input;	/* hwmon sys attr */
 	struct thermal_hwmon_attr temp_crit;	/* hwmon sys attr */
 #endif
+	unsigned long force_passive_temp;
+	unsigned long prev_temp;
+	bool force_passive;
 };
 
 struct thermal_zone_device *thermal_zone_device_register(char *, int, void *,

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Clean up thermal API

[Pavel Machek]

Hi!

> The thermal layer passes temperatures around as strings. This is fine 
> for sysfs, but makes it hard to use them for other purposes in-kernel. 
> Change them to longs and do the string conversion in the sysfs-specific 
> code.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>

Looks mostly ok to me.


> -static int thermal_get_temp(struct thermal_zone_device *thermal, char *buf)
> +static int thermal_get_temp(struct thermal_zone_device *thermal, 
> +			    unsigned long *temp)

Hmm, it would be cool to create typedef unsigned long milicelsius, so
that this is self-documenting and possibly sparse-checkable.



> @@ -898,7 +899,8 @@ static int thermal_get_temp(struct thermal_zone_device *thermal, char *buf)
>  	if (result)
>  		return result;
>  
> -	return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(tz->temperature));
> +	*temp = KELVIN_TO_MILLICELSIUS(tz->temperature);
> +	return 0;
>  }

Hmmm, if we did interface in miliKelvins, we would be able to do
-errno trick :-).


>  	if (!tz->ops->get_temp)
>  		return -EPERM;
>  
> -	return tz->ops->get_temp(tz, buf);
> +	ret = tz->ops->get_temp(tz,&temperature);

missing space after , .

> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf,"%ld\n",temperature);
>  }

More mising spaces.

> +	if (ret)
> +		return ret;
> +
> +	return sprintf (buf, "%ld\n", temperature);
>  }

And some extra spaces here: 'sprintf('.

							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Clean up thermal API

[Pavel Machek]

On Wed 2008-06-11 11:06:47, Matthew Garrett wrote:
> The thermal layer passes temperatures around as strings. This is fine 
> for sysfs, but makes it hard to use them for other purposes in-kernel. 
> Change them to longs and do the string conversion in the sysfs-specific 
> code.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> 
> ---
> 
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 504385b..28b3782 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -886,7 +886,8 @@ static void acpi_thermal_check(void *data)
>  /* sys I/F for generic thermal sysfs support */
>  #define KELVIN_TO_MILLICELSIUS(t) (t * 100 - 273200)
>  
> -static int thermal_get_temp(struct thermal_zone_device *thermal, char *buf)
> +static int thermal_get_temp(struct thermal_zone_device *thermal, 
> +			    unsigned long *temp)

Hmm, ulong is really wrong:

1) temperature in celsius can easily go below zero.

2) it can hardly go above 2000000C, so int should be enough.

							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [Patch v2] Implement thermal limiting in the generic thermal class

[Pavel Machek]

Hi!

> In the absence of an explicitly defined passive cooling zone any                                                                                                                                        
> machine unable to manage its thermal profile through active cooling will                                                                                                                                
> reach its critical shutdown temperature and power off, resulting in                                                                                                                                     
> potential data loss. Add support to the generic thermal class for                                                                                                                                       
> initiating passive cooling at a temperature defaulting to just below the                                                                                                                                
> critical temperature, with this value being overridable by the admin via                                                                                                                                
> sysfs.                                                                             

This means we get two copies of passive cooling code: one generic, and
one in acpi. Can we make sure we have just one of them?

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

RE: [Patch v2] Implement thermal limiting in the generic thermal class

[Zhang, Rui]

One thing I'm concerning is that if we could enable polling in Linux.
Bug #8842 shows that polling may break some laptops.
And we have known that windows never enable polling.
Although this patch will not break the laptop in #8842, it still may
bring some potential risks if we enable polling in Linux.
IMO, a dmi entry would be much more acceptable...

Thanks,
rui

>-----Original Message-----
>From: Pavel Machek [mailto:pavel@suse.cz]
>Sent: Wednesday, June 18, 2008 2:54 AM
>To: Matthew Garrett
>Cc: Zhang, Rui; linux-acpi@vger.kernel.org;
linux-kernel@vger.kernel.org
>Subject: Re: [Patch v2] Implement thermal limiting in the generic
thermal class
>
>Hi!
>
>> In the absence of an explicitly defined passive cooling zone any
>> machine unable to manage its thermal profile through active cooling
will
>> reach its critical shutdown temperature and power off, resulting in
>> potential data loss. Add support to the generic thermal class for
>> initiating passive cooling at a temperature defaulting to just below
the
>> critical temperature, with this value being overridable by the admin
via
>> sysfs.
>
>This means we get two copies of passive cooling code: one generic, and
>one in acpi. Can we make sure we have just one of them?
>
>--
>(english) http://www.livejournal.com/~pavelmachek
>(cesky, pictures)
>http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [Patch v2] Implement thermal limiting in the generic thermal class

[Matthew Garrett]

On Wed, Jun 18, 2008 at 05:28:01PM +0800, Zhang, Rui wrote:
> One thing I'm concerning is that if we could enable polling in Linux.
> Bug #8842 shows that polling may break some laptops.
> And we have known that windows never enable polling.
> Although this patch will not break the laptop in #8842, it still may
> bring some potential risks if we enable polling in Linux.
> IMO, a dmi entry would be much more acceptable...

If it breaks anything, we can revisit the decision. I'd be surprised if 
it did.

-- 
Matthew Garrett | mjg59@srcf.ucam.org