Clean up thermal API

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 <[email protected]>

---

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.

drivers/thermal/thermal_sys.c +23 -0

+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

drivers/thermal/thermal_sys.c +5 -0

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

&&

drivers/thermal/thermal_sys.c +6 -0

+                                   !cpus_throttled) {
+                                       thermal_throttle_cpus();
+                                       cpus_throttled = 1;
+                               } else
+                                       list_for_each_entry(cdev,
+

&tz->cooling_devices,

drivers/thermal/thermal_sys.c +7 -0

+                                                           node)
+                                               cdev->throttle = 1;
+                       }
+               }
+               tz->prev_temp = temp;
+
+               /* Increase polling interval near the cut-off

temperature */

drivers/thermal/thermal_sys.c +8 -0

+               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);

drivers/thermal/thermal_sys.c +6 -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

drivers/thermal/thermal_sys.c +1 -0

@@ -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);

drivers/thermal/thermal_sys.c +1 -0

@@ -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) {

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

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

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.

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 <[email protected]>

---

drivers/acpi/fan.c +1 -1

index 6cf10cb..f3db49e 100644
@@ -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

drivers/acpi/fan.c +3 -2

+                            *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

drivers/acpi/fan.c +5 -4

+                            *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

drivers/acpi/processor_thermal.c +1 -1

index ef34b18..2a3721d 100644
@@ -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

drivers/acpi/processor_thermal.c +4 -2

+                       *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

drivers/acpi/processor_thermal.c +4 -5

+                       *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

drivers/acpi/video.c +1 -1

index 5e5dda3..d8d7596 100644
@@ -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

drivers/acpi/video.c +3 -2

+                              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

drivers/acpi/video.c +2 -5

+                              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);

drivers/acpi/video.c +3 -0

+       for (offset = 2; offset < video->brightness->count; offset++)
+               if (level == video->brightness->levels[offset]) {
+                       *state = video->brightness->count - offset -

1;

drivers/acpi/video.c +2 -0

+                       return 0;
+               }
 
        return -EINVAL;
 }

drivers/thermal/thermal_sys.c +5 -1

index c537a5b..02abaf0 100644
@@ -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

drivers/thermal/thermal_sys.c +5 -1

@@ -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

include/linux/thermal.h +2 -2

index 63e6619..5ddbd4f 100644
@@ -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 | [email protected]

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 <[email protected]>

---

drivers/acpi/thermal.c +2 -1

index 504385b..28b3782 100644
@@ -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;

drivers/acpi/thermal.c +2 -1

@@ -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";

drivers/acpi/thermal.c +1 -1

@@ -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;

drivers/acpi/thermal.c +0 -2

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

drivers/acpi/thermal.c +6 -3

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

drivers/acpi/thermal.c +5 -3

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

drivers/acpi/thermal.c +5 -2

+               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));

drivers/acpi/thermal.c +5 -0

+               if (!trip) {
+                       *temp = KELVIN_TO_MILLICELSIUS(
+                               tz->trips.active[i].temperature);
+                       return 0;
+               }
                trip--;
        }

drivers/thermal/thermal_sys.c +8 -1

index 6098787..c537a5b 100644
@@ -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

drivers/thermal/thermal_sys.c +2 -1

@@ -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;

drivers/thermal/thermal_sys.c +6 -1

@@ -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);

drivers/thermal/thermal_sys.c +8 -1

@@ -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

drivers/thermal/thermal_sys.c +7 -1

@@ -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);
 }

include/linux/thermal.h +3 -2

index 06d3e6e..63e6619 100644
@@ -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 | [email protected]

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.

-----Original Message-----
From: Pavel Machek [mailto:[email protected]]
Sent: Wednesday, June 18, 2008 2:54 AM
To: Matthew Garrett
Cc: Zhang, Rui; [email protected];

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

Hi!

Quoted 2 lines

In the absence of an explicitly defined passive cooling zone any machine unable to manage its thermal profile through active cooling

Quoted 3 lines

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

Quoted 1 line

critical temperature, with this value being overridable by the admin

Quoted 1 line

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

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...

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 <[email protected]>

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 <[email protected]>

---

drivers/acpi/thermal.c +0 -0

index 504385b..28b3782 100644