2009-10-26 07:39:05

by Frans Pop

[permalink] [raw]
Subject: [PATCH 0/6] [resend] thermal: improvements re. forced passive cooling


Please consider this patch set for 2.6.32. It was previously submitted
for 2.6.31, but AFAICT it has not yet been picked up.

All patches have been acked by either Rui or Matthew, with the
exception of 1/6 and 4/6, but the entire series has been implicitly
acked by Rui in http://bugzilla.kernel.org/show_bug.cgi?id=13918#c26.

The patch set closes http://bugzilla.kernel.org/show_bug.cgi?id=13918.

Andrew, could you take the set just in case?

Cheers,
FJP


2009-10-26 07:40:04

by Frans Pop

[permalink] [raw]
Subject: [PATCH 1/6] thermal: sysfs-api.txt - reformat for improved readability

The document currently uses large indentations which make the text
too wide for easy readability. Also improve general consistency.

Signed-off-by: Frans Pop <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Sujith Thomas <[email protected]>
Cc: Matthew Garrett <[email protected]>
---
Documentation/thermal/sysfs-api.txt | 380 ++++++++++++++++++-----------------
1 files changed, 192 insertions(+), 188 deletions(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 70d68ce..895337f 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -1,5 +1,5 @@
Generic Thermal Sysfs driver How To
-=========================
+===================================

Written by Sujith Thomas <[email protected]>, Zhang Rui <[email protected]>

@@ -10,20 +10,20 @@ Copyright (c) 2008 Intel Corporation

0. Introduction

-The generic thermal sysfs provides a set of interfaces for thermal zone devices (sensors)
-and thermal cooling devices (fan, processor...) to register with the thermal management
-solution and to be a part of it.
+The generic thermal sysfs provides a set of interfaces for thermal zone
+devices (sensors) and thermal cooling devices (fan, processor...) to register
+with the thermal management solution and to be a part of it.

-This how-to focuses on enabling new thermal zone and cooling devices to participate
-in thermal management.
-This solution is platform independent and any type of thermal zone devices and
-cooling devices should be able to make use of the infrastructure.
+This how-to focuses on enabling new thermal zone and cooling devices to
+participate in thermal management.
+This solution is platform independent and any type of thermal zone devices
+and cooling devices should be able to make use of the infrastructure.

-The main task of the thermal sysfs driver is to expose thermal zone attributes as well
-as cooling device attributes to the user space.
-An intelligent thermal management application can make decisions based on inputs
-from thermal zone attributes (the current temperature and trip point temperature)
-and throttle appropriate devices.
+The main task of the thermal sysfs driver is to expose thermal zone attributes
+as well as cooling device attributes to the user space.
+An intelligent thermal management application can make decisions based on
+inputs from thermal zone attributes (the current temperature and trip point
+temperature) and throttle appropriate devices.

[0-*] denotes any positive number starting from 0
[1-*] denotes any positive number starting from 1
@@ -31,77 +31,77 @@ and throttle appropriate devices.
1. thermal sysfs driver interface functions

1.1 thermal zone device interface
-1.1.1 struct thermal_zone_device *thermal_zone_device_register(char *name, int trips,
- void *devdata, struct thermal_zone_device_ops *ops)
-
- This interface function adds a new thermal zone device (sensor) to
- /sys/class/thermal folder as thermal_zone[0-*].
- It tries to bind all the thermal cooling devices registered at the same time.
-
- name: the thermal zone name.
- trips: the total number of trip points this thermal zone supports.
- devdata: device private data
- ops: thermal zone device call-backs.
- .bind: bind the thermal zone device with a thermal cooling device.
- .unbind: unbind the thermal zone device with a thermal cooling device.
- .get_temp: get the current temperature of the thermal zone.
- .get_mode: get the current mode (user/kernel) of the thermal zone.
- "kernel" means thermal management is done in kernel.
- "user" will prevent kernel thermal driver actions upon trip points
- so that user applications can take charge of thermal management.
- .set_mode: set the mode (user/kernel) of the thermal zone.
- .get_trip_type: get the type of certain trip point.
- .get_trip_temp: get the temperature above which the certain trip point
- will be fired.
+1.1.1 struct thermal_zone_device *thermal_zone_device_register(char *name,
+ int trips, void *devdata, struct thermal_zone_device_ops *ops)
+
+ This interface function adds a new thermal zone device (sensor) to
+ /sys/class/thermal folder as thermal_zone[0-*]. It tries to bind all the
+ thermal cooling devices registered at the same time.
+
+ name: the thermal zone name.
+ trips: the total number of trip points this thermal zone supports.
+ devdata: device private data
+ ops: thermal zone device call-backs.
+ .bind: bind the thermal zone device with a thermal cooling device.
+ .unbind: unbind the thermal zone device with a thermal cooling device.
+ .get_temp: get the current temperature of the thermal zone.
+ .get_mode: get the current mode (user/kernel) of the thermal zone.
+ - "kernel" means thermal management is done in kernel.
+ - "user" will prevent kernel thermal driver actions upon trip points
+ so that user applications can take charge of thermal management.
+ .set_mode: set the mode (user/kernel) of the thermal zone.
+ .get_trip_type: get the type of certain trip point.
+ .get_trip_temp: get the temperature above which the certain trip point
+ will be fired.

1.1.2 void thermal_zone_device_unregister(struct thermal_zone_device *tz)

- This interface function removes the thermal zone device.
- It deletes the corresponding entry form /sys/class/thermal folder and unbind all
- the thermal cooling devices it uses.
+ This interface function removes the thermal zone device.
+ It deletes the corresponding entry form /sys/class/thermal folder and
+ unbind all the thermal cooling devices it uses.

1.2 thermal cooling device interface
1.2.1 struct thermal_cooling_device *thermal_cooling_device_register(char *name,
- void *devdata, struct thermal_cooling_device_ops *)
-
- This interface function adds a new thermal cooling device (fan/processor/...) to
- /sys/class/thermal/ folder as cooling_device[0-*].
- It tries to bind itself to all the thermal zone devices register at the same time.
- name: the cooling device name.
- devdata: device private data.
- ops: thermal cooling devices call-backs.
- .get_max_state: get the Maximum throttle state of the cooling device.
- .get_cur_state: get the Current throttle state of the cooling device.
- .set_cur_state: set the Current throttle state of the cooling device.
+ void *devdata, struct thermal_cooling_device_ops *)
+
+ This interface function adds a new thermal cooling device (fan/processor/...)
+ to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
+ to all the thermal zone devices register at the same time.
+ name: the cooling device name.
+ devdata: device private data.
+ ops: thermal cooling devices call-backs.
+ .get_max_state: get the Maximum throttle state of the cooling device.
+ .get_cur_state: get the Current throttle state of the cooling device.
+ .set_cur_state: set the Current throttle state of the cooling device.

1.2.2 void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)

- This interface function remove the thermal cooling device.
- It deletes the corresponding entry form /sys/class/thermal folder and unbind
- itself from all the thermal zone devices using it.
+ This interface function remove the thermal cooling device.
+ It deletes the corresponding entry form /sys/class/thermal folder and
+ unbind itself from all the thermal zone devices using it.

1.3 interface for binding a thermal zone device with a thermal cooling device
1.3.1 int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
- int trip, struct thermal_cooling_device *cdev);
+ int trip, struct thermal_cooling_device *cdev);

- This interface function bind a thermal cooling device to the certain trip point
- of a thermal zone device.
- This function is usually called in the thermal zone device .bind callback.
- tz: the thermal zone device
- cdev: thermal cooling device
- trip: indicates which trip point the cooling devices is associated with
- in this thermal zone.
+ This interface function bind a thermal cooling device to the certain trip
+ point of a thermal zone device.
+ This function is usually called in the thermal zone device .bind callback.
+ tz: the thermal zone device
+ cdev: thermal cooling device
+ trip: indicates which trip point the cooling devices is associated with
+ in this thermal zone.

1.3.2 int thermal_zone_unbind_cooling_device(struct thermal_zone_device *tz,
- int trip, struct thermal_cooling_device *cdev);
+ int trip, struct thermal_cooling_device *cdev);

- This interface function unbind a thermal cooling device from the certain trip point
- of a thermal zone device.
- This function is usually called in the thermal zone device .unbind callback.
- tz: the thermal zone device
- cdev: thermal cooling device
- trip: indicates which trip point the cooling devices is associated with
- in this thermal zone.
+ This interface function unbind a thermal cooling device from the certain
+ trip point of a thermal zone device. This function is usually called in
+ the thermal zone device .unbind callback.
+ tz: the thermal zone device
+ cdev: thermal cooling device
+ trip: indicates which trip point the cooling devices is associated with
+ in this thermal zone.

2. sysfs attributes structure

@@ -114,153 +114,157 @@ if hwmon is compiled in or built as a module.

Thermal zone device sys I/F, created once it's registered:
/sys/class/thermal/thermal_zone[0-*]:
- |-----type: Type of the thermal zone
- |-----temp: Current temperature
- |-----mode: Working mode of the thermal zone
- |-----trip_point_[0-*]_temp: Trip point temperature
- |-----trip_point_[0-*]_type: Trip point type
+ |---type: Type of the thermal zone
+ |---temp: Current temperature
+ |---mode: Working mode of the thermal zone
+ |---trip_point_[0-*]_temp: Trip point temperature
+ |---trip_point_[0-*]_type: Trip point type

Thermal cooling device sys I/F, created once it's registered:
/sys/class/thermal/cooling_device[0-*]:
- |-----type : Type of the cooling device(processor/fan/...)
- |-----max_state: Maximum cooling state of the cooling device
- |-----cur_state: Current cooling state of the cooling device
+ |---type: Type of the cooling device(processor/fan/...)
+ |---max_state: Maximum cooling state of the cooling device
+ |---cur_state: Current cooling state of the cooling device


-These two dynamic attributes are created/removed in pairs.
-They represent the relationship between a thermal zone and its associated cooling device.
-They are created/removed for each
-thermal_zone_bind_cooling_device/thermal_zone_unbind_cooling_device successful execution.
+Then next two dynamic attributes are created/removed in pairs. They represent
+the relationship between a thermal zone and its associated cooling device.
+They are created/removed for each successful execution of
+thermal_zone_bind_cooling_device/thermal_zone_unbind_cooling_device.

-/sys/class/thermal/thermal_zone[0-*]
- |-----cdev[0-*]: The [0-*]th cooling device in the current thermal zone
- |-----cdev[0-*]_trip_point: Trip point that cdev[0-*] is associated with
+/sys/class/thermal/thermal_zone[0-*]:
+ |---cdev[0-*]: [0-*]th cooling device in current thermal zone
+ |---cdev[0-*]_trip_point: Trip point that cdev[0-*] is associated with

Besides the thermal zone device sysfs I/F and cooling device sysfs I/F,
-the generic thermal driver also creates a hwmon sysfs I/F for each _type_ of
-thermal zone device. E.g. the generic thermal driver registers one hwmon class device
-and build the associated hwmon sysfs I/F for all the registered ACPI thermal zones.
+the generic thermal driver also creates a hwmon sysfs I/F for each _type_
+of thermal zone device. E.g. the generic thermal driver registers one hwmon
+class device and build the associated hwmon sysfs I/F for all the registered
+ACPI thermal zones.
+
/sys/class/hwmon/hwmon[0-*]:
- |-----name: The type of the thermal zone devices.
- |-----temp[1-*]_input: The current temperature of thermal zone [1-*].
- |-----temp[1-*]_critical: The critical trip point of thermal zone [1-*].
+ |---name: The type of the thermal zone devices
+ |---temp[1-*]_input: The current temperature of thermal zone [1-*]
+ |---temp[1-*]_critical: The critical trip point of thermal zone [1-*]
+
Please read Documentation/hwmon/sysfs-interface for additional information.

***************************
* Thermal zone attributes *
***************************

-type Strings which represent the thermal zone type.
- This is given by thermal zone driver as part of registration.
- Eg: "acpitz" indicates it's an ACPI thermal device.
- In order to keep it consistent with hwmon sys attribute,
- this should be a short, lowercase string,
- not containing spaces nor dashes.
- RO
- Required
-
-temp Current temperature as reported by thermal zone (sensor)
- Unit: millidegree Celsius
- RO
- Required
-
-mode One of the predefined values in [kernel, user]
- This file gives information about the algorithm
- that is currently managing the thermal zone.
- It can be either default kernel based algorithm
- or user space application.
- RW
- Optional
- kernel = Thermal management in kernel thermal zone driver.
- user = Preventing kernel thermal zone driver actions upon
- trip points so that user application can take full
- charge of the thermal management.
-
-trip_point_[0-*]_temp The temperature above which trip point will be fired
- Unit: millidegree Celsius
- RO
- Optional
-
-trip_point_[0-*]_type Strings which indicate the type of the trip point
- E.g. it can be one of critical, hot, passive,
- active[0-*] for ACPI thermal zone.
- RO
- Optional
-
-cdev[0-*] Sysfs link to the thermal cooling device node where the sys I/F
- for cooling device throttling control represents.
- RO
- Optional
-
-cdev[0-*]_trip_point The trip point with which cdev[0-*] is associated in this thermal zone
- -1 means the cooling device is not associated with any trip point.
- RO
- Optional
-
-******************************
-* Cooling device attributes *
-******************************
-
-type String which represents the type of device
- eg: For generic ACPI: this should be "Fan",
- "Processor" or "LCD"
- eg. For memory controller device on intel_menlow platform:
- this should be "Memory controller"
- RO
- Required
-
-max_state The maximum permissible cooling state of this cooling device.
- RO
- Required
-
-cur_state The current cooling state of this cooling device.
- the value can any integer numbers between 0 and max_state,
- cur_state == 0 means no cooling
- cur_state == max_state means the maximum cooling.
- RW
- Required
+type
+ Strings which represent the thermal zone type.
+ This is given by thermal zone driver as part of registration.
+ E.g: "acpitz" indicates it's an ACPI thermal device.
+ In order to keep it consistent with hwmon sys attribute; this should
+ be a short, lowercase string, not containing spaces nor dashes.
+ RO, Required
+
+temp
+ Current temperature as reported by thermal zone (sensor).
+ Unit: millidegree Celsius
+ RO, Required
+
+mode
+ One of the predefined values in [kernel, user].
+ This file gives information about the algorithm that is currently
+ managing the thermal zone. It can be either default kernel based
+ algorithm or user space application.
+ kernel = Thermal management in kernel thermal zone driver.
+ user = Preventing kernel thermal zone driver actions upon
+ trip points so that user application can take full
+ charge of the thermal management.
+ RW, Optional
+
+trip_point_[0-*]_temp
+ The temperature above which trip point will be fired.
+ Unit: millidegree Celsius
+ RO, Optional
+
+trip_point_[0-*]_type
+ Strings which indicate the type of the trip point.
+ E.g. it can be one of critical, hot, passive, active[0-*] for ACPI
+ thermal zone.
+ RO, Optional
+
+cdev[0-*]
+ Sysfs link to the thermal cooling device node where the sys I/F
+ for cooling device throttling control represents.
+ RO, Optional
+
+cdev[0-*]_trip_point
+ The trip point with which cdev[0-*] is associated in this thermal
+ zone; -1 means the cooling device is not associated with any trip
+ point.
+ RO, Optional
+
+*****************************
+* Cooling device attributes *
+*****************************
+
+type
+ String which represents the type of device, e.g:
+ - for generic ACPI: should be "Fan", "Processor" or "LCD"
+ - for memory controller device on intel_menlow platform:
+ should be "Memory controller".
+ RO, Required
+
+max_state
+ The maximum permissible cooling state of this cooling device.
+ RO, Required
+
+cur_state
+ The current cooling state of this cooling device.
+ The value can any integer numbers between 0 and max_state:
+ - cur_state == 0 means no cooling
+ - cur_state == max_state means the maximum cooling.
+ RW, Required

3. A simple implementation

-ACPI thermal zone may support multiple trip points like critical/hot/passive/active.
-If an ACPI thermal zone supports critical, passive, active[0] and active[1] at the same time,
-it may register itself as a thermal_zone_device (thermal_zone1) with 4 trip points in all.
-It has one processor and one fan, which are both registered as thermal_cooling_device.
-If the processor is listed in _PSL method, and the fan is listed in _AL0 method,
-the sys I/F structure will be built like this:
+ACPI thermal zone may support multiple trip points like critical, hot,
+passive, active. If an ACPI thermal zone supports critical, passive,
+active[0] and active[1] at the same time, it may register itself as a
+thermal_zone_device (thermal_zone1) with 4 trip points in all.
+It has one processor and one fan, which are both registered as
+thermal_cooling_device.
+
+If the processor is listed in _PSL method, and the fan is listed in _AL0
+method, the sys I/F structure will be built like this:

/sys/class/thermal:

|thermal_zone1:
- |-----type: acpitz
- |-----temp: 37000
- |-----mode: kernel
- |-----trip_point_0_temp: 100000
- |-----trip_point_0_type: critical
- |-----trip_point_1_temp: 80000
- |-----trip_point_1_type: passive
- |-----trip_point_2_temp: 70000
- |-----trip_point_2_type: active0
- |-----trip_point_3_temp: 60000
- |-----trip_point_3_type: active1
- |-----cdev0: --->/sys/class/thermal/cooling_device0
- |-----cdev0_trip_point: 1 /* cdev0 can be used for passive */
- |-----cdev1: --->/sys/class/thermal/cooling_device3
- |-----cdev1_trip_point: 2 /* cdev1 can be used for active[0]*/
+ |---type: acpitz
+ |---temp: 37000
+ |---mode: kernel
+ |---trip_point_0_temp: 100000
+ |---trip_point_0_type: critical
+ |---trip_point_1_temp: 80000
+ |---trip_point_1_type: passive
+ |---trip_point_2_temp: 70000
+ |---trip_point_2_type: active0
+ |---trip_point_3_temp: 60000
+ |---trip_point_3_type: active1
+ |---cdev0: --->/sys/class/thermal/cooling_device0
+ |---cdev0_trip_point: 1 /* cdev0 can be used for passive */
+ |---cdev1: --->/sys/class/thermal/cooling_device3
+ |---cdev1_trip_point: 2 /* cdev1 can be used for active[0]*/

|cooling_device0:
- |-----type: Processor
- |-----max_state: 8
- |-----cur_state: 0
+ |---type: Processor
+ |---max_state: 8
+ |---cur_state: 0

|cooling_device3:
- |-----type: Fan
- |-----max_state: 2
- |-----cur_state: 0
+ |---type: Fan
+ |---max_state: 2
+ |---cur_state: 0

/sys/class/hwmon:

|hwmon0:
- |-----name: acpitz
- |-----temp1_input: 37000
- |-----temp1_crit: 100000
+ |---name: acpitz
+ |---temp1_input: 37000
+ |---temp1_crit: 100000
--
1.5.6.5

2009-10-26 07:39:10

by Frans Pop

[permalink] [raw]
Subject: [PATCH 2/6] thermal: sysfs-api.txt - document passive attribute for thermal zones

Signed-off-by: Frans Pop <[email protected]>
Acked-by: Zhang Rui <[email protected]>
Cc: Sujith Thomas <[email protected]>
Cc: Matthew Garrett <[email protected]>
---
Documentation/thermal/sysfs-api.txt | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 895337f..a87dc27 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -199,6 +199,15 @@ cdev[0-*]_trip_point
point.
RO, Optional

+passive
+ Attribute is only present for zones in which the passive cooling
+ policy is not supported by native thermal driver. Default is zero
+ and can be set to a temperature (in millidegrees) to enable a
+ passive trip point for the zone. Activation is done by polling with
+ an interval of 1 second.
+ Unit: millidegrees Celsius
+ RW, Optional
+
*****************************
* Cooling device attributes *
*****************************
--
1.5.6.5

2009-10-26 07:39:12

by Frans Pop

[permalink] [raw]
Subject: [PATCH 3/6] acpi: thermal: display forced passive trip points in proc

Users can force a passive trip point for a thermal zone that does not
have _PSV defined in ACPI by setting the passive attribute in sysfs.
It's useful to display such trip points in /proc/acpi/thermal_zone.

.../TZ1/cooling_mode:<setting not supported>
.../TZ1/polling_frequency:polling frequency: 10 seconds
.../TZ1/state:state: ok
.../TZ1/temperature:temperature: 53 C
.../TZ1/trip_points:critical (S5): 110 C
.../TZ1/trip_points:passive (forced): 95 C

And if not set (passive is 0):
.../TZ1/trip_points:passive (forced):<not set>

Signed-off-by: Frans Pop <[email protected]>
Acked-by: Zhang Rui <[email protected]>
Cc: Matthew Garrett <[email protected]>
---
drivers/acpi/thermal.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 65f6781..9073ada 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -1052,6 +1052,13 @@ static int acpi_thermal_trip_seq_show(struct seq_file *seq, void *offset)
acpi_device_bid(device));
}
seq_puts(seq, "\n");
+ } else {
+ seq_printf(seq, "passive (forced):");
+ if (tz->thermal_zone->forced_passive)
+ seq_printf(seq, " %i C\n",
+ tz->thermal_zone->forced_passive / 1000);
+ else
+ seq_printf(seq, "<not set>\n");
}

for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
--
1.5.6.5

2009-10-26 07:39:13

by Frans Pop

[permalink] [raw]
Subject: [PATCH 4/6] thermal: add sanity check for the passive attribute

Values below 1000 milli-celsius don't make sense and can cause the
system to go into a thermal heart attack: the actual temperature
will always be lower and thus the system will be throttled down to
its lowest setting.

An additional problem is that values below 1000 will show as 0 in
/proc/acpi/thermal/TZx/trip_points:passive.

cat passive
0
echo -n 90 >passive
bash: echo: write error: Invalid argument
echo -n 90000 >passive
cat passive
90000

Signed-off-by: Frans Pop <[email protected]>
Cc: Matthew Garrett <[email protected]>
Cc: Zhang Rui <[email protected]>
---
Documentation/thermal/sysfs-api.txt | 1 +
drivers/thermal/thermal_sys.c | 6 ++++++
2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index a87dc27..cb3d15b 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -206,6 +206,7 @@ passive
passive trip point for the zone. Activation is done by polling with
an interval of 1 second.
Unit: millidegrees Celsius
+ Valid values: 0 (disabled) or greater than 1000
RW, Optional

*****************************
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 4e83c29..74d2eb5 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -225,6 +225,12 @@ passive_store(struct device *dev, struct device_attribute *attr,
if (!sscanf(buf, "%d\n", &state))
return -EINVAL;

+ /* sanity check: values below 1000 millicelcius don't make sense
+ * and can cause the system to go into a thermal heart attack
+ */
+ if (state && state < 1000)
+ return -EINVAL;
+
if (state && !tz->forced_passive) {
mutex_lock(&thermal_list_lock);
list_for_each_entry(cdev, &thermal_cdev_list, node) {
--
1.5.6.5

2009-10-26 07:40:06

by Frans Pop

[permalink] [raw]
Subject: [PATCH 5/6] thermal: Only set passive_delay for forced_passive cooling

Setting polling_delay is useless as passive_delay has priority,
so the value shown in proc isn't the actual polling delay. It
also gives the impression to the user that he can change the
polling interval through proc, while in fact he can't.

Also, unset passive_delay when the forced passive trip point is
unbound to allow polling to be disabled.

Signed-off-by: Frans Pop <[email protected]>
Acked-by: Matthew Garrett <[email protected]>
Cc: Zhang Rui <[email protected]>
---
drivers/thermal/thermal_sys.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 74d2eb5..fc5e92e 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -241,6 +241,8 @@ passive_store(struct device *dev, struct device_attribute *attr,
cdev);
}
mutex_unlock(&thermal_list_lock);
+ if (!tz->passive_delay)
+ tz->passive_delay = 1000;
} else if (!state && tz->forced_passive) {
mutex_lock(&thermal_list_lock);
list_for_each_entry(cdev, &thermal_cdev_list, node) {
@@ -251,17 +253,12 @@ passive_store(struct device *dev, struct device_attribute *attr,
cdev);
}
mutex_unlock(&thermal_list_lock);
+ tz->passive_delay = 0;
}

tz->tc1 = 1;
tz->tc2 = 1;

- if (!tz->passive_delay)
- tz->passive_delay = 1000;
-
- if (!tz->polling_delay)
- tz->polling_delay = 10000;
-
tz->forced_passive = state;

thermal_zone_device_update(tz);
--
1.5.6.5

2009-10-26 07:39:37

by Frans Pop

[permalink] [raw]
Subject: [PATCH 6/6] thermal: disable polling if passive_delay and polling_delay are both unset

Otherwise polling will continue for the thermal zone even when
it is no longer needed, for example because forced passive cooling
was disabled.

Signed-off-by: Frans Pop <[email protected]>
Acked-by: Matthew Garrett <[email protected]>
Cc: Zhang Rui <[email protected]>
---
drivers/thermal/thermal_sys.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index fc5e92e..d69e6fd 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -1019,6 +1019,8 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
thermal_zone_device_set_polling(tz, tz->passive_delay);
else if (tz->polling_delay)
thermal_zone_device_set_polling(tz, tz->polling_delay);
+ else
+ thermal_zone_device_set_polling(tz, 0);
mutex_unlock(&tz->lock);
}
EXPORT_SYMBOL(thermal_zone_device_update);
--
1.5.6.5

2009-10-28 22:35:56

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 0/6] [resend] thermal: improvements re. forced passive cooling

On Mon, 26 Oct 2009 08:38:58 +0100
Frans Pop <[email protected]> wrote:

>
> Please consider this patch set for 2.6.32. It was previously submitted
> for 2.6.31, but AFAICT it has not yet been picked up.
>
> All patches have been acked by either Rui or Matthew, with the
> exception of 1/6 and 4/6, but the entire series has been implicitly
> acked by Rui in http://bugzilla.kernel.org/show_bug.cgi?id=13918#c26.
>
> The patch set closes http://bugzilla.kernel.org/show_bug.cgi?id=13918.
>
> Andrew, could you take the set just in case?
>

I'm trying to work out what the actual bug is in here.

afacit some KDE tool put wrong numbers into /proc files, acpi didn't
sanity check them sufficiently and permitted the CPU to overheat, yes?

There seems to be rather a lot of non-bugfix stuff in this patch
series. Perhaps too much for 2.6.32, and a real problem if we want to
backport something into 2.6.31.x and earlier.

2009-10-28 22:49:05

by Frans Pop

[permalink] [raw]
Subject: Re: [PATCH 0/6] [resend] thermal: improvements re. forced passive cooling

Hi Andrew,

On Wednesday 28 October 2009, Andrew Morton wrote:
> I'm trying to work out what the actual bug is in here.

It turned out there was no bug in the kernel. This patch set is an
enhancement of existing kernel functionality.

> afacit some KDE tool put wrong numbers into /proc files, acpi didn't
> sanity check them sufficiently and permitted the CPU to overheat, yes?

No. The problem was that the thermal zone that overheated did not have a
passive trip point defined in ACPI. So the kernel had no way of knowing
the system was overheating.

The kernel already has an option to "force" a passive trip point for such
zones (using polling to check it), and that works. But while testing that
and looking at the code I spotted some possible enhancements.

> There seems to be rather a lot of non-bugfix stuff in this patch
> series. Perhaps too much for 2.6.32, and a real problem if we want to
> backport something into 2.6.31.x and earlier.

It would have been nice if it had made .32 from my first submission, but
now it's fine for .33. It's not stable material.

Thanks for taking the series.

Cheers,
FJP

2009-11-05 23:11:29

by Len Brown

[permalink] [raw]
Subject: Re: [PATCH 1/6] thermal: sysfs-api.txt - reformat for improved readability

applied

thanks,
Len Brown, Intel Open Source Technology Center

2009-11-05 23:11:57

by Len Brown

[permalink] [raw]
Subject: Re: [PATCH 2/6] thermal: sysfs-api.txt - document passive attribute for thermal zones

applied

thanks,
Len Brown, Intel Open Source Technology Center

2009-11-05 23:13:00

by Len Brown

[permalink] [raw]
Subject: Re: [PATCH 3/6] acpi: thermal: display forced passive trip points in proc

applied for 2.6.33

thanks,
Len Brown, Intel Open Source Technology Center

2009-11-06 00:27:16

by Len Brown

[permalink] [raw]
Subject: Re: [PATCH 0/6] [resend] thermal: improvements re. forced passive cooling

> It turned out there was no bug in the kernel...
...
> It would have been nice if it had made .32 from my first submission,

Yeah, the discussion settled down just as the merge window openend,
and this isn't high risk, so in retrospect I should have pushed
it into 2.6.32.

> but now it's fine for .33. It's not stable material.

I'll push the documentation update to 2.6.32.

I'll push the tweaks to 2.6.33.
I don't expec that this will need to be back-ported
as only advanced users who also have this BIOS bug will be messing
around setting their own custom passive trip point in the first place.

thanks,
Len Brown, Intel Open Source Technology Center