* [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
@ 2008-02-25 21:31 ` Zhang, Rui
0 siblings, 0 replies; 50+ messages in thread
From: Zhang, Rui @ 2008-02-25 21:31 UTC (permalink / raw)
To: linux-acpi, lm-sensors
Cc: Hans de Goede, Jean Delvare, Matthew Garrett, Thomas Renninger,
Thomas, Sujith, Mark M. Hoffman, Henrique de Moraes Holschuh,
Len Brown, Richard Hughes, Zhang, Rui
Add hwmon sys I/F for the generic thermal device.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
Documentation/thermal/sysfs-api.txt | 22 +++++----
drivers/thermal/Kconfig | 1
drivers/thermal/thermal.c | 86 ++++++++++++++++++++++++++++++++----
include/linux/thermal.h | 1
4 files changed, 92 insertions(+), 18 deletions(-)
Index: linux-2.6.25-rc2/Documentation/thermal/sysfs-api.txt
===================================================================
--- linux-2.6.25-rc2.orig/Documentation/thermal/sysfs-api.txt 2008-02-25 22:43:29.000000000 -0500
+++ linux-2.6.25-rc2/Documentation/thermal/sysfs-api.txt 2008-02-25 22:43:38.000000000 -0500
@@ -141,12 +141,14 @@
type Strings which represent the thermal zone type.
This is given by thermal zone driver as part of registration.
+ In order to keep the compatibility with hwmon,
+ it should not contain any spaces or dashes.
Eg: "ACPI thermal zone" indicates it's a ACPI thermal device
RO
- Optional
+ Required
temp Current temperature as reported by thermal zone (sensor)
- Unit: degree Celsius
+ Unit: millidegree Celsius
RO
Required
@@ -163,7 +165,7 @@
charge of the thermal management.
trip_point_[0-*]_temp The temperature above which trip point will be fired
- Unit: degree Celsius
+ Unit: millidegree Celsius
RO
Optional
@@ -219,16 +221,16 @@
|thermal_zone1:
|-----type: ACPI thermal zone
- |-----temp: 37
+ |-----temp: 37000
|-----mode: kernel
- |-----trip_point_0_temp: 100
+ |-----trip_point_0_temp: 100000
|-----trip_point_0_type: critical
- |-----trip_point_1_temp: 80
+ |-----trip_point_1_temp: 80000
|-----trip_point_1_type: passive
- |-----trip_point_2_temp: 70
- |-----trip_point_2_type: active[0]
- |-----trip_point_3_temp: 60
- |-----trip_point_3_type: active[1]
+ |-----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
Index: linux-2.6.25-rc2/drivers/thermal/Kconfig
===================================================================
--- linux-2.6.25-rc2.orig/drivers/thermal/Kconfig 2008-02-25 22:43:29.000000000 -0500
+++ linux-2.6.25-rc2/drivers/thermal/Kconfig 2008-02-25 22:43:38.000000000 -0500
@@ -4,6 +4,7 @@
menuconfig THERMAL
bool "Generic Thermal sysfs driver"
+ select HWMON
default y
help
Generic Thermal Sysfs driver offers a generic mechanism for
Index: linux-2.6.25-rc2/drivers/thermal/thermal.c
===================================================================
--- linux-2.6.25-rc2.orig/drivers/thermal/thermal.c 2008-02-25 22:43:29.000000000 -0500
+++ linux-2.6.25-rc2/drivers/thermal/thermal.c 2008-02-26 00:37:57.000000000 -0500
@@ -30,8 +30,9 @@
#include <linux/idr.h>
#include <linux/thermal.h>
#include <linux/spinlock.h>
+#include <linux/hwmon.h>
-MODULE_AUTHOR("Zhang Rui")
+MODULE_AUTHOR("Zhang Rui");
MODULE_DESCRIPTION("Generic thermal management sysfs support");
MODULE_LICENSE("GPL");
@@ -171,6 +172,47 @@
return tz->ops->get_trip_temp(tz, trip, buf);
}
+static ssize_t
+hwmon_type_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct device *device = dev->parent;
+ return type_show(device, attr, buf);
+}
+
+static ssize_t
+hwmon_temp_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct device *device = dev->parent;
+ return temp_show(device, attr, buf);
+}
+
+static ssize_t
+crit_trip_temp_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct device *device = dev->parent;
+ struct thermal_zone_device *tz = to_thermal_zone(device);
+ int result;
+
+ if (!tz->ops->get_trip_temp )
+ return -EPERM;
+
+ /* assume trip 0 to be the critical trip point by default */
+ if (tz->ops->get_trip_type) {
+ result = tz->ops->get_trip_type(tz, 0, buf);
+ if (result < 0)
+ return result;
+ if (strcmp(buf, "critical\n"))
+ return -ENODEV;
+ }
+
+ return tz->ops->get_trip_temp(tz, 0, buf);
+}
+
+static DEVICE_ATTR(name, 0444, hwmon_type_show, NULL);
+static DEVICE_ATTR(temp1_input, 0444, hwmon_temp_show, NULL);
+static DEVICE_ATTR(temp1_crit, 0444, crit_trip_temp_show, NULL);
+
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);
@@ -569,6 +611,9 @@
int result;
int count;
+ if (!type)
+ return ERR_PTR(-EINVAL);
+
if (strlen(type) >= THERMAL_NAME_LENGTH)
return NULL;
@@ -604,13 +649,33 @@
return NULL;
}
- /* sys I/F */
- if (type) {
- result = device_create_file(&tz->device, &dev_attr_type);
- if (result)
- goto unregister;
+ /* hwmon sys I/F */
+ tz->hwmon = hwmon_device_register(&tz->device);
+ if (IS_ERR(tz->hwmon)) {
+ result = PTR_ERR(tz->hwmon);
+ tz->hwmon = NULL;
+ printk(KERN_ERR PREFIX
+ "unable to register hwmon device\n");
+ goto unregister;
}
+ result = device_create_file(tz->hwmon, &dev_attr_name);
+ if (result)
+ goto unregister;
+
+ result = device_create_file(tz->hwmon, &dev_attr_temp1_input);
+ if (result)
+ goto unregister;
+
+ result = device_create_file(tz->hwmon, &dev_attr_temp1_crit);
+ if (result)
+ goto unregister;
+
+ /* sys I/F */
+ result = device_create_file(&tz->device, &dev_attr_type);
+ if (result)
+ goto unregister;
+
result = device_create_file(&tz->device, &dev_attr_temp);
if (result)
goto unregister;
@@ -676,8 +741,13 @@
tz->ops->unbind(tz, cdev);
mutex_unlock(&thermal_list_lock);
- if (tz->type[0])
- device_remove_file(&tz->device, &dev_attr_type);
+ device_remove_file(&tz->device, &dev_attr_name);
+ device_remove_file(&tz->device, &dev_attr_temp1_input);
+ device_remove_file(&tz->device, &dev_attr_temp1_crit);
+ hwmon_device_unregister(tz->hwmon);
+ tz->hwmon = NULL;
+
+ device_remove_file(&tz->device, &dev_attr_type);
device_remove_file(&tz->device, &dev_attr_temp);
if (tz->ops->get_mode)
device_remove_file(&tz->device, &dev_attr_mode);
Index: linux-2.6.25-rc2/include/linux/thermal.h
===================================================================
--- linux-2.6.25-rc2.orig/include/linux/thermal.h 2008-02-25 22:43:29.000000000 -0500
+++ linux-2.6.25-rc2/include/linux/thermal.h 2008-02-25 22:43:38.000000000 -0500
@@ -69,6 +69,7 @@
int id;
char type[THERMAL_NAME_LENGTH];
struct device device;
+ struct device *hwmon;
void *devdata;
int trips;
struct thermal_zone_device_ops *ops;
^ permalink raw reply [flat|nested] 50+ messages in thread
* [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-02-25 21:31 ` Zhang, Rui
0 siblings, 0 replies; 50+ messages in thread
From: Zhang, Rui @ 2008-02-25 21:31 UTC (permalink / raw)
To: linux-acpi, lm-sensors
Cc: Hans de Goede, Jean Delvare, Matthew Garrett, Thomas Renninger,
Thomas, Sujith, Mark M. Hoffman, Henrique de Moraes Holschuh,
Len Brown, Richard Hughes, Zhang, Rui
Add hwmon sys I/F for the generic thermal device.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
Documentation/thermal/sysfs-api.txt | 22 +++++----
drivers/thermal/Kconfig | 1
drivers/thermal/thermal.c | 86 ++++++++++++++++++++++++++++++++----
include/linux/thermal.h | 1
4 files changed, 92 insertions(+), 18 deletions(-)
Index: linux-2.6.25-rc2/Documentation/thermal/sysfs-api.txt
=================================--- linux-2.6.25-rc2.orig/Documentation/thermal/sysfs-api.txt 2008-02-25 22:43:29.000000000 -0500
+++ linux-2.6.25-rc2/Documentation/thermal/sysfs-api.txt 2008-02-25 22:43:38.000000000 -0500
@@ -141,12 +141,14 @@
type Strings which represent the thermal zone type.
This is given by thermal zone driver as part of registration.
+ In order to keep the compatibility with hwmon,
+ it should not contain any spaces or dashes.
Eg: "ACPI thermal zone" indicates it's a ACPI thermal device
RO
- Optional
+ Required
temp Current temperature as reported by thermal zone (sensor)
- Unit: degree Celsius
+ Unit: millidegree Celsius
RO
Required
@@ -163,7 +165,7 @@
charge of the thermal management.
trip_point_[0-*]_temp The temperature above which trip point will be fired
- Unit: degree Celsius
+ Unit: millidegree Celsius
RO
Optional
@@ -219,16 +221,16 @@
|thermal_zone1:
|-----type: ACPI thermal zone
- |-----temp: 37
+ |-----temp: 37000
|-----mode: kernel
- |-----trip_point_0_temp: 100
+ |-----trip_point_0_temp: 100000
|-----trip_point_0_type: critical
- |-----trip_point_1_temp: 80
+ |-----trip_point_1_temp: 80000
|-----trip_point_1_type: passive
- |-----trip_point_2_temp: 70
- |-----trip_point_2_type: active[0]
- |-----trip_point_3_temp: 60
- |-----trip_point_3_type: active[1]
+ |-----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
Index: linux-2.6.25-rc2/drivers/thermal/Kconfig
=================================--- linux-2.6.25-rc2.orig/drivers/thermal/Kconfig 2008-02-25 22:43:29.000000000 -0500
+++ linux-2.6.25-rc2/drivers/thermal/Kconfig 2008-02-25 22:43:38.000000000 -0500
@@ -4,6 +4,7 @@
menuconfig THERMAL
bool "Generic Thermal sysfs driver"
+ select HWMON
default y
help
Generic Thermal Sysfs driver offers a generic mechanism for
Index: linux-2.6.25-rc2/drivers/thermal/thermal.c
=================================--- linux-2.6.25-rc2.orig/drivers/thermal/thermal.c 2008-02-25 22:43:29.000000000 -0500
+++ linux-2.6.25-rc2/drivers/thermal/thermal.c 2008-02-26 00:37:57.000000000 -0500
@@ -30,8 +30,9 @@
#include <linux/idr.h>
#include <linux/thermal.h>
#include <linux/spinlock.h>
+#include <linux/hwmon.h>
-MODULE_AUTHOR("Zhang Rui")
+MODULE_AUTHOR("Zhang Rui");
MODULE_DESCRIPTION("Generic thermal management sysfs support");
MODULE_LICENSE("GPL");
@@ -171,6 +172,47 @@
return tz->ops->get_trip_temp(tz, trip, buf);
}
+static ssize_t
+hwmon_type_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct device *device = dev->parent;
+ return type_show(device, attr, buf);
+}
+
+static ssize_t
+hwmon_temp_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct device *device = dev->parent;
+ return temp_show(device, attr, buf);
+}
+
+static ssize_t
+crit_trip_temp_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct device *device = dev->parent;
+ struct thermal_zone_device *tz = to_thermal_zone(device);
+ int result;
+
+ if (!tz->ops->get_trip_temp )
+ return -EPERM;
+
+ /* assume trip 0 to be the critical trip point by default */
+ if (tz->ops->get_trip_type) {
+ result = tz->ops->get_trip_type(tz, 0, buf);
+ if (result < 0)
+ return result;
+ if (strcmp(buf, "critical\n"))
+ return -ENODEV;
+ }
+
+ return tz->ops->get_trip_temp(tz, 0, buf);
+}
+
+static DEVICE_ATTR(name, 0444, hwmon_type_show, NULL);
+static DEVICE_ATTR(temp1_input, 0444, hwmon_temp_show, NULL);
+static DEVICE_ATTR(temp1_crit, 0444, crit_trip_temp_show, NULL);
+
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);
@@ -569,6 +611,9 @@
int result;
int count;
+ if (!type)
+ return ERR_PTR(-EINVAL);
+
if (strlen(type) >= THERMAL_NAME_LENGTH)
return NULL;
@@ -604,13 +649,33 @@
return NULL;
}
- /* sys I/F */
- if (type) {
- result = device_create_file(&tz->device, &dev_attr_type);
- if (result)
- goto unregister;
+ /* hwmon sys I/F */
+ tz->hwmon = hwmon_device_register(&tz->device);
+ if (IS_ERR(tz->hwmon)) {
+ result = PTR_ERR(tz->hwmon);
+ tz->hwmon = NULL;
+ printk(KERN_ERR PREFIX
+ "unable to register hwmon device\n");
+ goto unregister;
}
+ result = device_create_file(tz->hwmon, &dev_attr_name);
+ if (result)
+ goto unregister;
+
+ result = device_create_file(tz->hwmon, &dev_attr_temp1_input);
+ if (result)
+ goto unregister;
+
+ result = device_create_file(tz->hwmon, &dev_attr_temp1_crit);
+ if (result)
+ goto unregister;
+
+ /* sys I/F */
+ result = device_create_file(&tz->device, &dev_attr_type);
+ if (result)
+ goto unregister;
+
result = device_create_file(&tz->device, &dev_attr_temp);
if (result)
goto unregister;
@@ -676,8 +741,13 @@
tz->ops->unbind(tz, cdev);
mutex_unlock(&thermal_list_lock);
- if (tz->type[0])
- device_remove_file(&tz->device, &dev_attr_type);
+ device_remove_file(&tz->device, &dev_attr_name);
+ device_remove_file(&tz->device, &dev_attr_temp1_input);
+ device_remove_file(&tz->device, &dev_attr_temp1_crit);
+ hwmon_device_unregister(tz->hwmon);
+ tz->hwmon = NULL;
+
+ device_remove_file(&tz->device, &dev_attr_type);
device_remove_file(&tz->device, &dev_attr_temp);
if (tz->ops->get_mode)
device_remove_file(&tz->device, &dev_attr_mode);
Index: linux-2.6.25-rc2/include/linux/thermal.h
=================================--- linux-2.6.25-rc2.orig/include/linux/thermal.h 2008-02-25 22:43:29.000000000 -0500
+++ linux-2.6.25-rc2/include/linux/thermal.h 2008-02-25 22:43:38.000000000 -0500
@@ -69,6 +69,7 @@
int id;
char type[THERMAL_NAME_LENGTH];
struct device device;
+ struct device *hwmon;
void *devdata;
int trips;
struct thermal_zone_device_ops *ops;
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-02-25 21:31 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
@ 2008-02-26 8:39 ` Hans de Goede
-1 siblings, 0 replies; 50+ messages in thread
From: Hans de Goede @ 2008-02-26 8:39 UTC (permalink / raw)
To: Zhang, Rui
Cc: linux-acpi, lm-sensors, Jean Delvare, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
Zhang, Rui wrote:
> Add hwmon sys I/F for the generic thermal device.
>
Great!
But I have several remarks:
1) Looking at the new code, you only add temp1_input, so I'm guessing that you
are registering a seperate hwmon class entry per zone? Please don't do that,
please register one hwmon class entry, and add multiple temp#_input attr to it
(and the same for crit).
2) I see that temp_crit may not be always available:
> +static ssize_t
> +crit_trip_temp_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct device *device = dev->parent;
> + struct thermal_zone_device *tz = to_thermal_zone(device);
> + int result;
> +
> + if (!tz->ops->get_trip_temp )
> + return -EPERM;
> +
> + /* assume trip 0 to be the critical trip point by default */
> + if (tz->ops->get_trip_type) {
> + result = tz->ops->get_trip_type(tz, 0, buf);
> + if (result < 0)
> + return result;
> + if (strcmp(buf, "critical\n"))
> + return -ENODEV;
> + }
> +
> + return tz->ops->get_trip_temp(tz, 0, buf);
> +}
But you do always register a temp#_crit sysfs attr, it would be much much
better to only add this if reading it actually has a chance of returning a
value, so if tz->ops->get_trip_temp != NULL and tz->ops->get_trip_type(tz, 0,
buf) returns "critical" in buf.
Thanks & Regards,
Hans
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
> Documentation/thermal/sysfs-api.txt | 22 +++++----
> drivers/thermal/Kconfig | 1
> drivers/thermal/thermal.c | 86 ++++++++++++++++++++++++++++++++----
> include/linux/thermal.h | 1
> 4 files changed, 92 insertions(+), 18 deletions(-)
>
> Index: linux-2.6.25-rc2/Documentation/thermal/sysfs-api.txt
> ===================================================================
> --- linux-2.6.25-rc2.orig/Documentation/thermal/sysfs-api.txt 2008-02-25 22:43:29.000000000 -0500
> +++ linux-2.6.25-rc2/Documentation/thermal/sysfs-api.txt 2008-02-25 22:43:38.000000000 -0500
> @@ -141,12 +141,14 @@
>
> type Strings which represent the thermal zone type.
> This is given by thermal zone driver as part of registration.
> + In order to keep the compatibility with hwmon,
> + it should not contain any spaces or dashes.
> Eg: "ACPI thermal zone" indicates it's a ACPI thermal device
> RO
> - Optional
> + Required
>
> temp Current temperature as reported by thermal zone (sensor)
> - Unit: degree Celsius
> + Unit: millidegree Celsius
> RO
> Required
>
> @@ -163,7 +165,7 @@
> charge of the thermal management.
>
> trip_point_[0-*]_temp The temperature above which trip point will be fired
> - Unit: degree Celsius
> + Unit: millidegree Celsius
> RO
> Optional
>
> @@ -219,16 +221,16 @@
>
> |thermal_zone1:
> |-----type: ACPI thermal zone
> - |-----temp: 37
> + |-----temp: 37000
> |-----mode: kernel
> - |-----trip_point_0_temp: 100
> + |-----trip_point_0_temp: 100000
> |-----trip_point_0_type: critical
> - |-----trip_point_1_temp: 80
> + |-----trip_point_1_temp: 80000
> |-----trip_point_1_type: passive
> - |-----trip_point_2_temp: 70
> - |-----trip_point_2_type: active[0]
> - |-----trip_point_3_temp: 60
> - |-----trip_point_3_type: active[1]
> + |-----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
> Index: linux-2.6.25-rc2/drivers/thermal/Kconfig
> ===================================================================
> --- linux-2.6.25-rc2.orig/drivers/thermal/Kconfig 2008-02-25 22:43:29.000000000 -0500
> +++ linux-2.6.25-rc2/drivers/thermal/Kconfig 2008-02-25 22:43:38.000000000 -0500
> @@ -4,6 +4,7 @@
>
> menuconfig THERMAL
> bool "Generic Thermal sysfs driver"
> + select HWMON
> default y
> help
> Generic Thermal Sysfs driver offers a generic mechanism for
> Index: linux-2.6.25-rc2/drivers/thermal/thermal.c
> ===================================================================
> --- linux-2.6.25-rc2.orig/drivers/thermal/thermal.c 2008-02-25 22:43:29.000000000 -0500
> +++ linux-2.6.25-rc2/drivers/thermal/thermal.c 2008-02-26 00:37:57.000000000 -0500
> @@ -30,8 +30,9 @@
> #include <linux/idr.h>
> #include <linux/thermal.h>
> #include <linux/spinlock.h>
> +#include <linux/hwmon.h>
>
> -MODULE_AUTHOR("Zhang Rui")
> +MODULE_AUTHOR("Zhang Rui");
> MODULE_DESCRIPTION("Generic thermal management sysfs support");
> MODULE_LICENSE("GPL");
>
> @@ -171,6 +172,47 @@
> return tz->ops->get_trip_temp(tz, trip, buf);
> }
>
> +static ssize_t
> +hwmon_type_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct device *device = dev->parent;
> + return type_show(device, attr, buf);
> +}
> +
> +static ssize_t
> +hwmon_temp_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct device *device = dev->parent;
> + return temp_show(device, attr, buf);
> +}
> +
> +static ssize_t
> +crit_trip_temp_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct device *device = dev->parent;
> + struct thermal_zone_device *tz = to_thermal_zone(device);
> + int result;
> +
> + if (!tz->ops->get_trip_temp )
> + return -EPERM;
> +
> + /* assume trip 0 to be the critical trip point by default */
> + if (tz->ops->get_trip_type) {
> + result = tz->ops->get_trip_type(tz, 0, buf);
> + if (result < 0)
> + return result;
> + if (strcmp(buf, "critical\n"))
> + return -ENODEV;
> + }
> +
> + return tz->ops->get_trip_temp(tz, 0, buf);
> +}
> +
> +static DEVICE_ATTR(name, 0444, hwmon_type_show, NULL);
> +static DEVICE_ATTR(temp1_input, 0444, hwmon_temp_show, NULL);
> +static DEVICE_ATTR(temp1_crit, 0444, crit_trip_temp_show, NULL);
> +
> 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);
> @@ -569,6 +611,9 @@
> int result;
> int count;
>
> + if (!type)
> + return ERR_PTR(-EINVAL);
> +
> if (strlen(type) >= THERMAL_NAME_LENGTH)
> return NULL;
>
> @@ -604,13 +649,33 @@
> return NULL;
> }
>
> - /* sys I/F */
> - if (type) {
> - result = device_create_file(&tz->device, &dev_attr_type);
> - if (result)
> - goto unregister;
> + /* hwmon sys I/F */
> + tz->hwmon = hwmon_device_register(&tz->device);
> + if (IS_ERR(tz->hwmon)) {
> + result = PTR_ERR(tz->hwmon);
> + tz->hwmon = NULL;
> + printk(KERN_ERR PREFIX
> + "unable to register hwmon device\n");
> + goto unregister;
> }
>
> + result = device_create_file(tz->hwmon, &dev_attr_name);
> + if (result)
> + goto unregister;
> +
> + result = device_create_file(tz->hwmon, &dev_attr_temp1_input);
> + if (result)
> + goto unregister;
> +
> + result = device_create_file(tz->hwmon, &dev_attr_temp1_crit);
> + if (result)
> + goto unregister;
> +
> + /* sys I/F */
> + result = device_create_file(&tz->device, &dev_attr_type);
> + if (result)
> + goto unregister;
> +
> result = device_create_file(&tz->device, &dev_attr_temp);
> if (result)
> goto unregister;
> @@ -676,8 +741,13 @@
> tz->ops->unbind(tz, cdev);
> mutex_unlock(&thermal_list_lock);
>
> - if (tz->type[0])
> - device_remove_file(&tz->device, &dev_attr_type);
> + device_remove_file(&tz->device, &dev_attr_name);
> + device_remove_file(&tz->device, &dev_attr_temp1_input);
> + device_remove_file(&tz->device, &dev_attr_temp1_crit);
> + hwmon_device_unregister(tz->hwmon);
> + tz->hwmon = NULL;
> +
> + device_remove_file(&tz->device, &dev_attr_type);
> device_remove_file(&tz->device, &dev_attr_temp);
> if (tz->ops->get_mode)
> device_remove_file(&tz->device, &dev_attr_mode);
> Index: linux-2.6.25-rc2/include/linux/thermal.h
> ===================================================================
> --- linux-2.6.25-rc2.orig/include/linux/thermal.h 2008-02-25 22:43:29.000000000 -0500
> +++ linux-2.6.25-rc2/include/linux/thermal.h 2008-02-25 22:43:38.000000000 -0500
> @@ -69,6 +69,7 @@
> int id;
> char type[THERMAL_NAME_LENGTH];
> struct device device;
> + struct device *hwmon;
> void *devdata;
> int trips;
> struct thermal_zone_device_ops *ops;
>
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-02-26 8:39 ` Hans de Goede
0 siblings, 0 replies; 50+ messages in thread
From: Hans de Goede @ 2008-02-26 8:39 UTC (permalink / raw)
To: Zhang, Rui
Cc: linux-acpi, lm-sensors, Jean Delvare, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
Zhang, Rui wrote:
> Add hwmon sys I/F for the generic thermal device.
>
Great!
But I have several remarks:
1) Looking at the new code, you only add temp1_input, so I'm guessing that you
are registering a seperate hwmon class entry per zone? Please don't do that,
please register one hwmon class entry, and add multiple temp#_input attr to it
(and the same for crit).
2) I see that temp_crit may not be always available:
> +static ssize_t
> +crit_trip_temp_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct device *device = dev->parent;
> + struct thermal_zone_device *tz = to_thermal_zone(device);
> + int result;
> +
> + if (!tz->ops->get_trip_temp )
> + return -EPERM;
> +
> + /* assume trip 0 to be the critical trip point by default */
> + if (tz->ops->get_trip_type) {
> + result = tz->ops->get_trip_type(tz, 0, buf);
> + if (result < 0)
> + return result;
> + if (strcmp(buf, "critical\n"))
> + return -ENODEV;
> + }
> +
> + return tz->ops->get_trip_temp(tz, 0, buf);
> +}
But you do always register a temp#_crit sysfs attr, it would be much much
better to only add this if reading it actually has a chance of returning a
value, so if tz->ops->get_trip_temp != NULL and tz->ops->get_trip_type(tz, 0,
buf) returns "critical" in buf.
Thanks & Regards,
Hans
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
> Documentation/thermal/sysfs-api.txt | 22 +++++----
> drivers/thermal/Kconfig | 1
> drivers/thermal/thermal.c | 86 ++++++++++++++++++++++++++++++++----
> include/linux/thermal.h | 1
> 4 files changed, 92 insertions(+), 18 deletions(-)
>
> Index: linux-2.6.25-rc2/Documentation/thermal/sysfs-api.txt
> =================================> --- linux-2.6.25-rc2.orig/Documentation/thermal/sysfs-api.txt 2008-02-25 22:43:29.000000000 -0500
> +++ linux-2.6.25-rc2/Documentation/thermal/sysfs-api.txt 2008-02-25 22:43:38.000000000 -0500
> @@ -141,12 +141,14 @@
>
> type Strings which represent the thermal zone type.
> This is given by thermal zone driver as part of registration.
> + In order to keep the compatibility with hwmon,
> + it should not contain any spaces or dashes.
> Eg: "ACPI thermal zone" indicates it's a ACPI thermal device
> RO
> - Optional
> + Required
>
> temp Current temperature as reported by thermal zone (sensor)
> - Unit: degree Celsius
> + Unit: millidegree Celsius
> RO
> Required
>
> @@ -163,7 +165,7 @@
> charge of the thermal management.
>
> trip_point_[0-*]_temp The temperature above which trip point will be fired
> - Unit: degree Celsius
> + Unit: millidegree Celsius
> RO
> Optional
>
> @@ -219,16 +221,16 @@
>
> |thermal_zone1:
> |-----type: ACPI thermal zone
> - |-----temp: 37
> + |-----temp: 37000
> |-----mode: kernel
> - |-----trip_point_0_temp: 100
> + |-----trip_point_0_temp: 100000
> |-----trip_point_0_type: critical
> - |-----trip_point_1_temp: 80
> + |-----trip_point_1_temp: 80000
> |-----trip_point_1_type: passive
> - |-----trip_point_2_temp: 70
> - |-----trip_point_2_type: active[0]
> - |-----trip_point_3_temp: 60
> - |-----trip_point_3_type: active[1]
> + |-----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
> Index: linux-2.6.25-rc2/drivers/thermal/Kconfig
> =================================> --- linux-2.6.25-rc2.orig/drivers/thermal/Kconfig 2008-02-25 22:43:29.000000000 -0500
> +++ linux-2.6.25-rc2/drivers/thermal/Kconfig 2008-02-25 22:43:38.000000000 -0500
> @@ -4,6 +4,7 @@
>
> menuconfig THERMAL
> bool "Generic Thermal sysfs driver"
> + select HWMON
> default y
> help
> Generic Thermal Sysfs driver offers a generic mechanism for
> Index: linux-2.6.25-rc2/drivers/thermal/thermal.c
> =================================> --- linux-2.6.25-rc2.orig/drivers/thermal/thermal.c 2008-02-25 22:43:29.000000000 -0500
> +++ linux-2.6.25-rc2/drivers/thermal/thermal.c 2008-02-26 00:37:57.000000000 -0500
> @@ -30,8 +30,9 @@
> #include <linux/idr.h>
> #include <linux/thermal.h>
> #include <linux/spinlock.h>
> +#include <linux/hwmon.h>
>
> -MODULE_AUTHOR("Zhang Rui")
> +MODULE_AUTHOR("Zhang Rui");
> MODULE_DESCRIPTION("Generic thermal management sysfs support");
> MODULE_LICENSE("GPL");
>
> @@ -171,6 +172,47 @@
> return tz->ops->get_trip_temp(tz, trip, buf);
> }
>
> +static ssize_t
> +hwmon_type_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct device *device = dev->parent;
> + return type_show(device, attr, buf);
> +}
> +
> +static ssize_t
> +hwmon_temp_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct device *device = dev->parent;
> + return temp_show(device, attr, buf);
> +}
> +
> +static ssize_t
> +crit_trip_temp_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct device *device = dev->parent;
> + struct thermal_zone_device *tz = to_thermal_zone(device);
> + int result;
> +
> + if (!tz->ops->get_trip_temp )
> + return -EPERM;
> +
> + /* assume trip 0 to be the critical trip point by default */
> + if (tz->ops->get_trip_type) {
> + result = tz->ops->get_trip_type(tz, 0, buf);
> + if (result < 0)
> + return result;
> + if (strcmp(buf, "critical\n"))
> + return -ENODEV;
> + }
> +
> + return tz->ops->get_trip_temp(tz, 0, buf);
> +}
> +
> +static DEVICE_ATTR(name, 0444, hwmon_type_show, NULL);
> +static DEVICE_ATTR(temp1_input, 0444, hwmon_temp_show, NULL);
> +static DEVICE_ATTR(temp1_crit, 0444, crit_trip_temp_show, NULL);
> +
> 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);
> @@ -569,6 +611,9 @@
> int result;
> int count;
>
> + if (!type)
> + return ERR_PTR(-EINVAL);
> +
> if (strlen(type) >= THERMAL_NAME_LENGTH)
> return NULL;
>
> @@ -604,13 +649,33 @@
> return NULL;
> }
>
> - /* sys I/F */
> - if (type) {
> - result = device_create_file(&tz->device, &dev_attr_type);
> - if (result)
> - goto unregister;
> + /* hwmon sys I/F */
> + tz->hwmon = hwmon_device_register(&tz->device);
> + if (IS_ERR(tz->hwmon)) {
> + result = PTR_ERR(tz->hwmon);
> + tz->hwmon = NULL;
> + printk(KERN_ERR PREFIX
> + "unable to register hwmon device\n");
> + goto unregister;
> }
>
> + result = device_create_file(tz->hwmon, &dev_attr_name);
> + if (result)
> + goto unregister;
> +
> + result = device_create_file(tz->hwmon, &dev_attr_temp1_input);
> + if (result)
> + goto unregister;
> +
> + result = device_create_file(tz->hwmon, &dev_attr_temp1_crit);
> + if (result)
> + goto unregister;
> +
> + /* sys I/F */
> + result = device_create_file(&tz->device, &dev_attr_type);
> + if (result)
> + goto unregister;
> +
> result = device_create_file(&tz->device, &dev_attr_temp);
> if (result)
> goto unregister;
> @@ -676,8 +741,13 @@
> tz->ops->unbind(tz, cdev);
> mutex_unlock(&thermal_list_lock);
>
> - if (tz->type[0])
> - device_remove_file(&tz->device, &dev_attr_type);
> + device_remove_file(&tz->device, &dev_attr_name);
> + device_remove_file(&tz->device, &dev_attr_temp1_input);
> + device_remove_file(&tz->device, &dev_attr_temp1_crit);
> + hwmon_device_unregister(tz->hwmon);
> + tz->hwmon = NULL;
> +
> + device_remove_file(&tz->device, &dev_attr_type);
> device_remove_file(&tz->device, &dev_attr_temp);
> if (tz->ops->get_mode)
> device_remove_file(&tz->device, &dev_attr_mode);
> Index: linux-2.6.25-rc2/include/linux/thermal.h
> =================================> --- linux-2.6.25-rc2.orig/include/linux/thermal.h 2008-02-25 22:43:29.000000000 -0500
> +++ linux-2.6.25-rc2/include/linux/thermal.h 2008-02-25 22:43:38.000000000 -0500
> @@ -69,6 +69,7 @@
> int id;
> char type[THERMAL_NAME_LENGTH];
> struct device device;
> + struct device *hwmon;
> void *devdata;
> int trips;
> struct thermal_zone_device_ops *ops;
>
>
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-02-26 8:39 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Hans de Goede
@ 2008-02-26 21:40 ` Zhang, Rui
-1 siblings, 0 replies; 50+ messages in thread
From: Zhang, Rui @ 2008-02-26 21:40 UTC (permalink / raw)
To: Hans de Goede
Cc: linux-acpi, lm-sensors, Jean Delvare, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
On Tue, 2008-02-26 at 16:39 +0800, Hans de Goede wrote:
> Zhang, Rui wrote:
> > Add hwmon sys I/F for the generic thermal device.
> >
>
> Great!
>
> But I have several remarks:
> 1) Looking at the new code, you only add temp1_input, so I'm guessing
> that you
> are registering a seperate hwmon class entry per zone? Please don't do
> that,
> please register one hwmon class entry, and add multiple temp#_input
> attr to it
> (and the same for crit).
>
> 2) I see that temp_crit may not be always available:
> > +static ssize_t
> > +crit_trip_temp_show(struct device *dev, struct device_attribute
> *attr,
> > + char *buf)
> > +{
> > + struct device *device = dev->parent;
> > + struct thermal_zone_device *tz = to_thermal_zone(device);
> > + int result;
> > +
> > + if (!tz->ops->get_trip_temp )
> > + return -EPERM;
> > +
> > + /* assume trip 0 to be the critical trip point by default */
> > + if (tz->ops->get_trip_type) {
> > + result = tz->ops->get_trip_type(tz, 0, buf);
> > + if (result < 0)
> > + return result;
> > + if (strcmp(buf, "critical\n"))
> > + return -ENODEV;
> > + }
> > +
> > + return tz->ops->get_trip_temp(tz, 0, buf);
> > +}
>
> But you do always register a temp#_crit sysfs attr, it would be much
> much
> better to only add this if reading it actually has a chance of
> returning a
> value, so if tz->ops->get_trip_temp != NULL and
> tz->ops->get_trip_type(tz, 0,
> buf) returns "critical" in buf.
then how about this one? :)
Add hwmon sys I/F for the generic thermal device.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
---
Documentation/thermal/sysfs-api.txt | 22 ++--
drivers/thermal/Kconfig | 1
drivers/thermal/thermal.c | 161 ++++++++++++++++++++++++++++++------
3 files changed, 147 insertions(+), 37 deletions(-)
Index: linux-2.6/Documentation/thermal/sysfs-api.txt
===================================================================
--- linux-2.6.orig/Documentation/thermal/sysfs-api.txt
+++ linux-2.6/Documentation/thermal/sysfs-api.txt
@@ -143,10 +143,10 @@ type Strings which represent the ther
This is given by thermal zone driver as part of registration.
Eg: "ACPI thermal zone" indicates it's a ACPI thermal device
RO
- Optional
+ Required
temp Current temperature as reported by thermal zone (sensor)
- Unit: degree Celsius
+ Unit: millidegree Celsius
RO
Required
@@ -163,7 +163,7 @@ mode One of the predefined values in
charge of the thermal management.
trip_point_[0-*]_temp The temperature above which trip point will be fired
- Unit: degree Celsius
+ Unit: millidegree Celsius
RO
Optional
@@ -193,7 +193,7 @@ type String which represents the type
eg. For memory controller device on intel_menlow platform:
this should be "Memory controller"
RO
- Optional
+ Required
max_state The maximum permissible cooling state of this cooling device.
RO
@@ -219,16 +219,16 @@ the sys I/F structure will be built like
|thermal_zone1:
|-----type: ACPI thermal zone
- |-----temp: 37
+ |-----temp: 37000
|-----mode: kernel
- |-----trip_point_0_temp: 100
+ |-----trip_point_0_temp: 100000
|-----trip_point_0_type: critical
- |-----trip_point_1_temp: 80
+ |-----trip_point_1_temp: 80000
|-----trip_point_1_type: passive
- |-----trip_point_2_temp: 70
- |-----trip_point_2_type: active[0]
- |-----trip_point_3_temp: 60
- |-----trip_point_3_type: active[1]
+ |-----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
Index: linux-2.6/drivers/thermal/Kconfig
===================================================================
--- linux-2.6.orig/drivers/thermal/Kconfig
+++ linux-2.6/drivers/thermal/Kconfig
@@ -4,6 +4,7 @@
menuconfig THERMAL
bool "Generic Thermal sysfs driver"
+ select HWMON
default y
help
Generic Thermal Sysfs driver offers a generic mechanism for
Index: linux-2.6/drivers/thermal/thermal.c
===================================================================
--- linux-2.6.orig/drivers/thermal/thermal.c
+++ linux-2.6/drivers/thermal/thermal.c
@@ -30,8 +30,10 @@
#include <linux/idr.h>
#include <linux/thermal.h>
#include <linux/spinlock.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
-MODULE_AUTHOR("Zhang Rui")
+MODULE_AUTHOR("Zhang Rui");
MODULE_DESCRIPTION("Generic thermal management sysfs support");
MODULE_LICENSE("GPL");
@@ -56,6 +58,8 @@ static LIST_HEAD(thermal_tz_list);
static LIST_HEAD(thermal_cdev_list);
static DEFINE_MUTEX(thermal_list_lock);
+static struct device *thermal_hwmon;
+
static int get_idr(struct idr *idr, struct mutex *lock, int *id)
{
int err;
@@ -87,7 +91,67 @@ static void release_idr(struct idr *idr,
mutex_unlock(lock);
}
-/* sys I/F for thermal zone */
+/* hwmon sys I/F*/
+static ssize_t
+name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "thermal_sys_class\n");
+}
+
+static ssize_t
+temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct thermal_zone_device *tz;
+ struct sensor_device_attribute *sensor_attr
+ = to_sensor_dev_attr(attr);
+
+ list_for_each_entry(tz, &thermal_tz_list, node)
+ if (tz->id == sensor_attr->index)
+ return tz->ops->get_temp(tz, buf);
+
+ return -ENODEV;
+}
+
+static ssize_t
+temp_crit_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct thermal_zone_device *tz;
+ struct sensor_device_attribute *sensor_attr
+ = to_sensor_dev_attr(attr);
+
+ list_for_each_entry(tz, &thermal_tz_list, node)
+ if (tz->id == sensor_attr->index)
+ return tz->ops->get_trip_temp(tz, 0, buf);
+
+ return -ENODEV;
+}
+
+static DEVICE_ATTR(name, 0444, name_show, NULL);
+static struct sensor_device_attribute sensor_attrs[] = {
+ SENSOR_ATTR(temp1_input, 0444, temp_input_show, NULL, 0),
+ SENSOR_ATTR(temp1_crit, 0444, temp_crit_show, NULL, 0),
+ SENSOR_ATTR(temp2_input, 0444, temp_input_show, NULL, 1),
+ SENSOR_ATTR(temp2_crit, 0444, temp_crit_show, NULL, 1),
+ SENSOR_ATTR(temp3_input, 0444, temp_input_show, NULL, 2),
+ SENSOR_ATTR(temp3_crit, 0444, temp_crit_show, NULL, 2),
+ SENSOR_ATTR(temp4_input, 0444, temp_input_show, NULL, 3),
+ SENSOR_ATTR(temp4_crit, 0444, temp_crit_show, NULL, 3),
+ SENSOR_ATTR(temp5_input, 0444, temp_input_show, NULL, 4),
+ SENSOR_ATTR(temp5_crit, 0444, temp_crit_show, NULL, 4),
+ SENSOR_ATTR(temp6_input, 0444, temp_input_show, NULL, 5),
+ SENSOR_ATTR(temp6_crit, 0444, temp_crit_show, NULL, 5),
+ SENSOR_ATTR(temp7_input, 0444, temp_input_show, NULL, 6),
+ SENSOR_ATTR(temp7_crit, 0444, temp_crit_show, NULL, 6),
+ SENSOR_ATTR(temp8_input, 0444, temp_input_show, NULL, 7),
+ SENSOR_ATTR(temp8_crit, 0444, temp_crit_show, NULL, 7),
+ SENSOR_ATTR(temp9_input, 0444, temp_input_show, NULL, 8),
+ SENSOR_ATTR(temp9_crit, 0444, temp_crit_show, NULL, 8),
+ SENSOR_ATTR(temp10_input, 0444, temp_input_show, NULL, 9),
+ SENSOR_ATTR(temp10_crit, 0444, temp_crit_show, NULL, 9),
+};
+
+/* thermal zone sys I/F */
#define to_thermal_zone(_dev) \
container_of(_dev, struct thermal_zone_device, device)
@@ -214,7 +278,7 @@ do { \
device_remove_file(_dev, &trip_point_attrs[_index * 2 + 1]); \
} while (0)
-/* sys I/F for cooling device */
+/* cooling device sys I/F */
#define to_cooling_device(_dev) \
container_of(_dev, struct thermal_cooling_device, device)
@@ -447,6 +511,9 @@ struct thermal_cooling_device *thermal_c
struct thermal_zone_device *pos;
int result;
+ if (!type)
+ return ERR_PTR(-EINVAL);
+
if (strlen(type) >= THERMAL_NAME_LENGTH)
return ERR_PTR(-EINVAL);
@@ -477,11 +544,9 @@ struct thermal_cooling_device *thermal_c
}
/* sys I/F */
- if (type) {
- result = device_create_file(&cdev->device, &dev_attr_cdev_type);
- if (result)
- goto unregister;
- }
+ result = device_create_file(&cdev->device, &dev_attr_cdev_type);
+ if (result)
+ goto unregister;
result = device_create_file(&cdev->device, &dev_attr_max_state);
if (result)
@@ -547,8 +612,8 @@ void thermal_cooling_device_unregister(s
tz->ops->unbind(tz, cdev);
}
mutex_unlock(&thermal_list_lock);
- if (cdev->type[0])
- device_remove_file(&cdev->device, &dev_attr_cdev_type);
+
+ device_remove_file(&cdev->device, &dev_attr_cdev_type);
device_remove_file(&cdev->device, &dev_attr_max_state);
device_remove_file(&cdev->device, &dev_attr_cur_state);
@@ -580,6 +645,9 @@ struct thermal_zone_device *thermal_zone
int result;
int count;
+ if (!type)
+ return ERR_PTR(-EINVAL);
+
if (strlen(type) >= THERMAL_NAME_LENGTH)
return ERR_PTR(-EINVAL);
@@ -615,13 +683,28 @@ struct thermal_zone_device *thermal_zone
return ERR_PTR(result);
}
- /* sys I/F */
- if (type) {
- result = device_create_file(&tz->device, &dev_attr_type);
- if (result)
- goto unregister;
+ /* hwmon sys I/F */
+ result = device_create_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2].dev_attr);
+ if (result)
+ goto unregister;
+
+ if (trips > 0) {
+ char buf[40];
+ result = tz->ops->get_trip_type(tz, 0, buf);
+ if (result > 0 && !strcmp(buf, "critical\n")) {
+ result = device_create_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2 + 1].dev_attr);
+ if (result)
+ goto unregister;
+ }
}
+ /* sys I/F */
+ result = device_create_file(&tz->device, &dev_attr_type);
+ if (result)
+ goto unregister;
+
result = device_create_file(&tz->device, &dev_attr_temp);
if (result)
goto unregister;
@@ -687,8 +770,17 @@ void thermal_zone_device_unregister(stru
tz->ops->unbind(tz, cdev);
mutex_unlock(&thermal_list_lock);
- if (tz->type[0])
- device_remove_file(&tz->device, &dev_attr_type);
+ device_remove_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2].dev_attr);
+ if (tz->trips > 0) {
+ char buf[40];
+ if (tz->ops->get_trip_type(tz, 0, buf) > 0)
+ if (!strcmp(buf, "critical\n"))
+ device_remove_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2 + 1].dev_attr);
+ }
+
+ device_remove_file(&tz->device, &dev_attr_type);
device_remove_file(&tz->device, &dev_attr_temp);
if (tz->ops->get_mode)
device_remove_file(&tz->device, &dev_attr_mode);
@@ -705,6 +797,19 @@ void thermal_zone_device_unregister(stru
EXPORT_SYMBOL(thermal_zone_device_unregister);
+static void __exit thermal_exit(void)
+{
+ if (thermal_hwmon) {
+ device_remove_file(thermal_hwmon, &dev_attr_name);
+ hwmon_device_unregister(thermal_hwmon);
+ }
+ class_unregister(&thermal_class);
+ idr_destroy(&thermal_tz_idr);
+ idr_destroy(&thermal_cdev_idr);
+ mutex_destroy(&thermal_idr_lock);
+ mutex_destroy(&thermal_list_lock);
+}
+
static int __init thermal_init(void)
{
int result = 0;
@@ -716,16 +821,20 @@ static int __init thermal_init(void)
mutex_destroy(&thermal_idr_lock);
mutex_destroy(&thermal_list_lock);
}
- return result;
-}
-static void __exit thermal_exit(void)
-{
- class_unregister(&thermal_class);
- idr_destroy(&thermal_tz_idr);
- idr_destroy(&thermal_cdev_idr);
- mutex_destroy(&thermal_idr_lock);
- mutex_destroy(&thermal_list_lock);
+ thermal_hwmon = hwmon_device_register(NULL);
+ if (IS_ERR(thermal_hwmon)) {
+ result = PTR_ERR(thermal_hwmon);
+ thermal_hwmon = NULL;
+ printk(KERN_ERR PREFIX
+ "unable to register hwmon device\n");
+ thermal_exit();
+ return result;
+ }
+
+ result = device_create_file(thermal_hwmon, &dev_attr_name);
+
+ return result;
}
subsys_initcall(thermal_init);
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-02-26 21:40 ` Zhang, Rui
0 siblings, 0 replies; 50+ messages in thread
From: Zhang, Rui @ 2008-02-26 21:40 UTC (permalink / raw)
To: Hans de Goede
Cc: linux-acpi, lm-sensors, Jean Delvare, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
On Tue, 2008-02-26 at 16:39 +0800, Hans de Goede wrote:
> Zhang, Rui wrote:
> > Add hwmon sys I/F for the generic thermal device.
> >
>
> Great!
>
> But I have several remarks:
> 1) Looking at the new code, you only add temp1_input, so I'm guessing
> that you
> are registering a seperate hwmon class entry per zone? Please don't do
> that,
> please register one hwmon class entry, and add multiple temp#_input
> attr to it
> (and the same for crit).
>
> 2) I see that temp_crit may not be always available:
> > +static ssize_t
> > +crit_trip_temp_show(struct device *dev, struct device_attribute
> *attr,
> > + char *buf)
> > +{
> > + struct device *device = dev->parent;
> > + struct thermal_zone_device *tz = to_thermal_zone(device);
> > + int result;
> > +
> > + if (!tz->ops->get_trip_temp )
> > + return -EPERM;
> > +
> > + /* assume trip 0 to be the critical trip point by default */
> > + if (tz->ops->get_trip_type) {
> > + result = tz->ops->get_trip_type(tz, 0, buf);
> > + if (result < 0)
> > + return result;
> > + if (strcmp(buf, "critical\n"))
> > + return -ENODEV;
> > + }
> > +
> > + return tz->ops->get_trip_temp(tz, 0, buf);
> > +}
>
> But you do always register a temp#_crit sysfs attr, it would be much
> much
> better to only add this if reading it actually has a chance of
> returning a
> value, so if tz->ops->get_trip_temp != NULL and
> tz->ops->get_trip_type(tz, 0,
> buf) returns "critical" in buf.
then how about this one? :)
Add hwmon sys I/F for the generic thermal device.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
---
Documentation/thermal/sysfs-api.txt | 22 ++--
drivers/thermal/Kconfig | 1
drivers/thermal/thermal.c | 161 ++++++++++++++++++++++++++++++------
3 files changed, 147 insertions(+), 37 deletions(-)
Index: linux-2.6/Documentation/thermal/sysfs-api.txt
=================================--- linux-2.6.orig/Documentation/thermal/sysfs-api.txt
+++ linux-2.6/Documentation/thermal/sysfs-api.txt
@@ -143,10 +143,10 @@ type Strings which represent the ther
This is given by thermal zone driver as part of registration.
Eg: "ACPI thermal zone" indicates it's a ACPI thermal device
RO
- Optional
+ Required
temp Current temperature as reported by thermal zone (sensor)
- Unit: degree Celsius
+ Unit: millidegree Celsius
RO
Required
@@ -163,7 +163,7 @@ mode One of the predefined values in
charge of the thermal management.
trip_point_[0-*]_temp The temperature above which trip point will be fired
- Unit: degree Celsius
+ Unit: millidegree Celsius
RO
Optional
@@ -193,7 +193,7 @@ type String which represents the type
eg. For memory controller device on intel_menlow platform:
this should be "Memory controller"
RO
- Optional
+ Required
max_state The maximum permissible cooling state of this cooling device.
RO
@@ -219,16 +219,16 @@ the sys I/F structure will be built like
|thermal_zone1:
|-----type: ACPI thermal zone
- |-----temp: 37
+ |-----temp: 37000
|-----mode: kernel
- |-----trip_point_0_temp: 100
+ |-----trip_point_0_temp: 100000
|-----trip_point_0_type: critical
- |-----trip_point_1_temp: 80
+ |-----trip_point_1_temp: 80000
|-----trip_point_1_type: passive
- |-----trip_point_2_temp: 70
- |-----trip_point_2_type: active[0]
- |-----trip_point_3_temp: 60
- |-----trip_point_3_type: active[1]
+ |-----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
Index: linux-2.6/drivers/thermal/Kconfig
=================================--- linux-2.6.orig/drivers/thermal/Kconfig
+++ linux-2.6/drivers/thermal/Kconfig
@@ -4,6 +4,7 @@
menuconfig THERMAL
bool "Generic Thermal sysfs driver"
+ select HWMON
default y
help
Generic Thermal Sysfs driver offers a generic mechanism for
Index: linux-2.6/drivers/thermal/thermal.c
=================================--- linux-2.6.orig/drivers/thermal/thermal.c
+++ linux-2.6/drivers/thermal/thermal.c
@@ -30,8 +30,10 @@
#include <linux/idr.h>
#include <linux/thermal.h>
#include <linux/spinlock.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
-MODULE_AUTHOR("Zhang Rui")
+MODULE_AUTHOR("Zhang Rui");
MODULE_DESCRIPTION("Generic thermal management sysfs support");
MODULE_LICENSE("GPL");
@@ -56,6 +58,8 @@ static LIST_HEAD(thermal_tz_list);
static LIST_HEAD(thermal_cdev_list);
static DEFINE_MUTEX(thermal_list_lock);
+static struct device *thermal_hwmon;
+
static int get_idr(struct idr *idr, struct mutex *lock, int *id)
{
int err;
@@ -87,7 +91,67 @@ static void release_idr(struct idr *idr,
mutex_unlock(lock);
}
-/* sys I/F for thermal zone */
+/* hwmon sys I/F*/
+static ssize_t
+name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "thermal_sys_class\n");
+}
+
+static ssize_t
+temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct thermal_zone_device *tz;
+ struct sensor_device_attribute *sensor_attr
+ = to_sensor_dev_attr(attr);
+
+ list_for_each_entry(tz, &thermal_tz_list, node)
+ if (tz->id = sensor_attr->index)
+ return tz->ops->get_temp(tz, buf);
+
+ return -ENODEV;
+}
+
+static ssize_t
+temp_crit_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct thermal_zone_device *tz;
+ struct sensor_device_attribute *sensor_attr
+ = to_sensor_dev_attr(attr);
+
+ list_for_each_entry(tz, &thermal_tz_list, node)
+ if (tz->id = sensor_attr->index)
+ return tz->ops->get_trip_temp(tz, 0, buf);
+
+ return -ENODEV;
+}
+
+static DEVICE_ATTR(name, 0444, name_show, NULL);
+static struct sensor_device_attribute sensor_attrs[] = {
+ SENSOR_ATTR(temp1_input, 0444, temp_input_show, NULL, 0),
+ SENSOR_ATTR(temp1_crit, 0444, temp_crit_show, NULL, 0),
+ SENSOR_ATTR(temp2_input, 0444, temp_input_show, NULL, 1),
+ SENSOR_ATTR(temp2_crit, 0444, temp_crit_show, NULL, 1),
+ SENSOR_ATTR(temp3_input, 0444, temp_input_show, NULL, 2),
+ SENSOR_ATTR(temp3_crit, 0444, temp_crit_show, NULL, 2),
+ SENSOR_ATTR(temp4_input, 0444, temp_input_show, NULL, 3),
+ SENSOR_ATTR(temp4_crit, 0444, temp_crit_show, NULL, 3),
+ SENSOR_ATTR(temp5_input, 0444, temp_input_show, NULL, 4),
+ SENSOR_ATTR(temp5_crit, 0444, temp_crit_show, NULL, 4),
+ SENSOR_ATTR(temp6_input, 0444, temp_input_show, NULL, 5),
+ SENSOR_ATTR(temp6_crit, 0444, temp_crit_show, NULL, 5),
+ SENSOR_ATTR(temp7_input, 0444, temp_input_show, NULL, 6),
+ SENSOR_ATTR(temp7_crit, 0444, temp_crit_show, NULL, 6),
+ SENSOR_ATTR(temp8_input, 0444, temp_input_show, NULL, 7),
+ SENSOR_ATTR(temp8_crit, 0444, temp_crit_show, NULL, 7),
+ SENSOR_ATTR(temp9_input, 0444, temp_input_show, NULL, 8),
+ SENSOR_ATTR(temp9_crit, 0444, temp_crit_show, NULL, 8),
+ SENSOR_ATTR(temp10_input, 0444, temp_input_show, NULL, 9),
+ SENSOR_ATTR(temp10_crit, 0444, temp_crit_show, NULL, 9),
+};
+
+/* thermal zone sys I/F */
#define to_thermal_zone(_dev) \
container_of(_dev, struct thermal_zone_device, device)
@@ -214,7 +278,7 @@ do { \
device_remove_file(_dev, &trip_point_attrs[_index * 2 + 1]); \
} while (0)
-/* sys I/F for cooling device */
+/* cooling device sys I/F */
#define to_cooling_device(_dev) \
container_of(_dev, struct thermal_cooling_device, device)
@@ -447,6 +511,9 @@ struct thermal_cooling_device *thermal_c
struct thermal_zone_device *pos;
int result;
+ if (!type)
+ return ERR_PTR(-EINVAL);
+
if (strlen(type) >= THERMAL_NAME_LENGTH)
return ERR_PTR(-EINVAL);
@@ -477,11 +544,9 @@ struct thermal_cooling_device *thermal_c
}
/* sys I/F */
- if (type) {
- result = device_create_file(&cdev->device, &dev_attr_cdev_type);
- if (result)
- goto unregister;
- }
+ result = device_create_file(&cdev->device, &dev_attr_cdev_type);
+ if (result)
+ goto unregister;
result = device_create_file(&cdev->device, &dev_attr_max_state);
if (result)
@@ -547,8 +612,8 @@ void thermal_cooling_device_unregister(s
tz->ops->unbind(tz, cdev);
}
mutex_unlock(&thermal_list_lock);
- if (cdev->type[0])
- device_remove_file(&cdev->device, &dev_attr_cdev_type);
+
+ device_remove_file(&cdev->device, &dev_attr_cdev_type);
device_remove_file(&cdev->device, &dev_attr_max_state);
device_remove_file(&cdev->device, &dev_attr_cur_state);
@@ -580,6 +645,9 @@ struct thermal_zone_device *thermal_zone
int result;
int count;
+ if (!type)
+ return ERR_PTR(-EINVAL);
+
if (strlen(type) >= THERMAL_NAME_LENGTH)
return ERR_PTR(-EINVAL);
@@ -615,13 +683,28 @@ struct thermal_zone_device *thermal_zone
return ERR_PTR(result);
}
- /* sys I/F */
- if (type) {
- result = device_create_file(&tz->device, &dev_attr_type);
- if (result)
- goto unregister;
+ /* hwmon sys I/F */
+ result = device_create_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2].dev_attr);
+ if (result)
+ goto unregister;
+
+ if (trips > 0) {
+ char buf[40];
+ result = tz->ops->get_trip_type(tz, 0, buf);
+ if (result > 0 && !strcmp(buf, "critical\n")) {
+ result = device_create_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2 + 1].dev_attr);
+ if (result)
+ goto unregister;
+ }
}
+ /* sys I/F */
+ result = device_create_file(&tz->device, &dev_attr_type);
+ if (result)
+ goto unregister;
+
result = device_create_file(&tz->device, &dev_attr_temp);
if (result)
goto unregister;
@@ -687,8 +770,17 @@ void thermal_zone_device_unregister(stru
tz->ops->unbind(tz, cdev);
mutex_unlock(&thermal_list_lock);
- if (tz->type[0])
- device_remove_file(&tz->device, &dev_attr_type);
+ device_remove_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2].dev_attr);
+ if (tz->trips > 0) {
+ char buf[40];
+ if (tz->ops->get_trip_type(tz, 0, buf) > 0)
+ if (!strcmp(buf, "critical\n"))
+ device_remove_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2 + 1].dev_attr);
+ }
+
+ device_remove_file(&tz->device, &dev_attr_type);
device_remove_file(&tz->device, &dev_attr_temp);
if (tz->ops->get_mode)
device_remove_file(&tz->device, &dev_attr_mode);
@@ -705,6 +797,19 @@ void thermal_zone_device_unregister(stru
EXPORT_SYMBOL(thermal_zone_device_unregister);
+static void __exit thermal_exit(void)
+{
+ if (thermal_hwmon) {
+ device_remove_file(thermal_hwmon, &dev_attr_name);
+ hwmon_device_unregister(thermal_hwmon);
+ }
+ class_unregister(&thermal_class);
+ idr_destroy(&thermal_tz_idr);
+ idr_destroy(&thermal_cdev_idr);
+ mutex_destroy(&thermal_idr_lock);
+ mutex_destroy(&thermal_list_lock);
+}
+
static int __init thermal_init(void)
{
int result = 0;
@@ -716,16 +821,20 @@ static int __init thermal_init(void)
mutex_destroy(&thermal_idr_lock);
mutex_destroy(&thermal_list_lock);
}
- return result;
-}
-static void __exit thermal_exit(void)
-{
- class_unregister(&thermal_class);
- idr_destroy(&thermal_tz_idr);
- idr_destroy(&thermal_cdev_idr);
- mutex_destroy(&thermal_idr_lock);
- mutex_destroy(&thermal_list_lock);
+ thermal_hwmon = hwmon_device_register(NULL);
+ if (IS_ERR(thermal_hwmon)) {
+ result = PTR_ERR(thermal_hwmon);
+ thermal_hwmon = NULL;
+ printk(KERN_ERR PREFIX
+ "unable to register hwmon device\n");
+ thermal_exit();
+ return result;
+ }
+
+ result = device_create_file(thermal_hwmon, &dev_attr_name);
+
+ return result;
}
subsys_initcall(thermal_init);
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-02-25 21:31 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
@ 2008-02-27 0:37 ` Zhang, Rui
-1 siblings, 0 replies; 50+ messages in thread
From: Zhang, Rui @ 2008-02-27 0:37 UTC (permalink / raw)
To: Len Brown; +Cc: linux-acpi, lm-sensors, Hans de Goede
Add hwmon sys I/F for the generic thermal device.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
---
Documentation/thermal/sysfs-api.txt | 22 ++--
drivers/thermal/Kconfig | 1
drivers/thermal/thermal.c | 169 ++++++++++++++++++++++++++++++------
3 files changed, 155 insertions(+), 37 deletions(-)
Index: linux-2.6/Documentation/thermal/sysfs-api.txt
===================================================================
--- linux-2.6.orig/Documentation/thermal/sysfs-api.txt
+++ linux-2.6/Documentation/thermal/sysfs-api.txt
@@ -143,10 +143,10 @@ type Strings which represent the ther
This is given by thermal zone driver as part of registration.
Eg: "ACPI thermal zone" indicates it's a ACPI thermal device
RO
- Optional
+ Required
temp Current temperature as reported by thermal zone (sensor)
- Unit: degree Celsius
+ Unit: millidegree Celsius
RO
Required
@@ -163,7 +163,7 @@ mode One of the predefined values in
charge of the thermal management.
trip_point_[0-*]_temp The temperature above which trip point will be fired
- Unit: degree Celsius
+ Unit: millidegree Celsius
RO
Optional
@@ -193,7 +193,7 @@ type String which represents the type
eg. For memory controller device on intel_menlow platform:
this should be "Memory controller"
RO
- Optional
+ Required
max_state The maximum permissible cooling state of this cooling device.
RO
@@ -219,16 +219,16 @@ the sys I/F structure will be built like
|thermal_zone1:
|-----type: ACPI thermal zone
- |-----temp: 37
+ |-----temp: 37000
|-----mode: kernel
- |-----trip_point_0_temp: 100
+ |-----trip_point_0_temp: 100000
|-----trip_point_0_type: critical
- |-----trip_point_1_temp: 80
+ |-----trip_point_1_temp: 80000
|-----trip_point_1_type: passive
- |-----trip_point_2_temp: 70
- |-----trip_point_2_type: active[0]
- |-----trip_point_3_temp: 60
- |-----trip_point_3_type: active[1]
+ |-----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
Index: linux-2.6/drivers/thermal/Kconfig
===================================================================
--- linux-2.6.orig/drivers/thermal/Kconfig
+++ linux-2.6/drivers/thermal/Kconfig
@@ -4,6 +4,7 @@
menuconfig THERMAL
bool "Generic Thermal sysfs driver"
+ select HWMON
default y
help
Generic Thermal Sysfs driver offers a generic mechanism for
Index: linux-2.6/drivers/thermal/thermal.c
===================================================================
--- linux-2.6.orig/drivers/thermal/thermal.c
+++ linux-2.6/drivers/thermal/thermal.c
@@ -30,8 +30,10 @@
#include <linux/idr.h>
#include <linux/thermal.h>
#include <linux/spinlock.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
-MODULE_AUTHOR("Zhang Rui")
+MODULE_AUTHOR("Zhang Rui");
MODULE_DESCRIPTION("Generic thermal management sysfs support");
MODULE_LICENSE("GPL");
@@ -56,6 +58,9 @@ static LIST_HEAD(thermal_tz_list);
static LIST_HEAD(thermal_cdev_list);
static DEFINE_MUTEX(thermal_list_lock);
+static struct device *thermal_hwmon;
+#define MAX_THERMAL_ZONES 10
+
static int get_idr(struct idr *idr, struct mutex *lock, int *id)
{
int err;
@@ -87,7 +92,67 @@ static void release_idr(struct idr *idr,
mutex_unlock(lock);
}
-/* sys I/F for thermal zone */
+/* hwmon sys I/F*/
+static ssize_t
+name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "thermal_sys_class\n");
+}
+
+static ssize_t
+temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct thermal_zone_device *tz;
+ struct sensor_device_attribute *sensor_attr
+ = to_sensor_dev_attr(attr);
+
+ list_for_each_entry(tz, &thermal_tz_list, node)
+ if (tz->id == sensor_attr->index)
+ return tz->ops->get_temp(tz, buf);
+
+ return -ENODEV;
+}
+
+static ssize_t
+temp_crit_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct thermal_zone_device *tz;
+ struct sensor_device_attribute *sensor_attr
+ = to_sensor_dev_attr(attr);
+
+ list_for_each_entry(tz, &thermal_tz_list, node)
+ if (tz->id == sensor_attr->index)
+ return tz->ops->get_trip_temp(tz, 0, buf);
+
+ return -ENODEV;
+}
+
+static DEVICE_ATTR(name, 0444, name_show, NULL);
+static struct sensor_device_attribute sensor_attrs[] = {
+ SENSOR_ATTR(temp1_input, 0444, temp_input_show, NULL, 0),
+ SENSOR_ATTR(temp1_crit, 0444, temp_crit_show, NULL, 0),
+ SENSOR_ATTR(temp2_input, 0444, temp_input_show, NULL, 1),
+ SENSOR_ATTR(temp2_crit, 0444, temp_crit_show, NULL, 1),
+ SENSOR_ATTR(temp3_input, 0444, temp_input_show, NULL, 2),
+ SENSOR_ATTR(temp3_crit, 0444, temp_crit_show, NULL, 2),
+ SENSOR_ATTR(temp4_input, 0444, temp_input_show, NULL, 3),
+ SENSOR_ATTR(temp4_crit, 0444, temp_crit_show, NULL, 3),
+ SENSOR_ATTR(temp5_input, 0444, temp_input_show, NULL, 4),
+ SENSOR_ATTR(temp5_crit, 0444, temp_crit_show, NULL, 4),
+ SENSOR_ATTR(temp6_input, 0444, temp_input_show, NULL, 5),
+ SENSOR_ATTR(temp6_crit, 0444, temp_crit_show, NULL, 5),
+ SENSOR_ATTR(temp7_input, 0444, temp_input_show, NULL, 6),
+ SENSOR_ATTR(temp7_crit, 0444, temp_crit_show, NULL, 6),
+ SENSOR_ATTR(temp8_input, 0444, temp_input_show, NULL, 7),
+ SENSOR_ATTR(temp8_crit, 0444, temp_crit_show, NULL, 7),
+ SENSOR_ATTR(temp9_input, 0444, temp_input_show, NULL, 8),
+ SENSOR_ATTR(temp9_crit, 0444, temp_crit_show, NULL, 8),
+ SENSOR_ATTR(temp10_input, 0444, temp_input_show, NULL, 9),
+ SENSOR_ATTR(temp10_crit, 0444, temp_crit_show, NULL, 9),
+};
+
+/* thermal zone sys I/F */
#define to_thermal_zone(_dev) \
container_of(_dev, struct thermal_zone_device, device)
@@ -214,7 +279,7 @@ do { \
device_remove_file(_dev, &trip_point_attrs[_index * 2 + 1]); \
} while (0)
-/* sys I/F for cooling device */
+/* cooling device sys I/F */
#define to_cooling_device(_dev) \
container_of(_dev, struct thermal_cooling_device, device)
@@ -447,6 +512,9 @@ struct thermal_cooling_device *thermal_c
struct thermal_zone_device *pos;
int result;
+ if (!type)
+ return ERR_PTR(-EINVAL);
+
if (strlen(type) >= THERMAL_NAME_LENGTH)
return ERR_PTR(-EINVAL);
@@ -477,11 +545,9 @@ struct thermal_cooling_device *thermal_c
}
/* sys I/F */
- if (type) {
- result = device_create_file(&cdev->device, &dev_attr_cdev_type);
- if (result)
- goto unregister;
- }
+ result = device_create_file(&cdev->device, &dev_attr_cdev_type);
+ if (result)
+ goto unregister;
result = device_create_file(&cdev->device, &dev_attr_max_state);
if (result)
@@ -547,8 +613,8 @@ void thermal_cooling_device_unregister(s
tz->ops->unbind(tz, cdev);
}
mutex_unlock(&thermal_list_lock);
- if (cdev->type[0])
- device_remove_file(&cdev->device, &dev_attr_cdev_type);
+
+ device_remove_file(&cdev->device, &dev_attr_cdev_type);
device_remove_file(&cdev->device, &dev_attr_max_state);
device_remove_file(&cdev->device, &dev_attr_cur_state);
@@ -580,6 +646,9 @@ struct thermal_zone_device *thermal_zone
int result;
int count;
+ if (!type)
+ return ERR_PTR(-EINVAL);
+
if (strlen(type) >= THERMAL_NAME_LENGTH)
return ERR_PTR(-EINVAL);
@@ -601,6 +670,13 @@ struct thermal_zone_device *thermal_zone
kfree(tz);
return ERR_PTR(result);
}
+ if (tz->id >= MAX_THERMAL_ZONES) {
+ printk(KERN_ERR PREFIX
+ "Too many thermal zones\n");
+ release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
+ kfree(tz);
+ return ERR_PTR(-EINVAL);
+ }
strcpy(tz->type, type);
tz->ops = ops;
@@ -615,13 +691,28 @@ struct thermal_zone_device *thermal_zone
return ERR_PTR(result);
}
- /* sys I/F */
- if (type) {
- result = device_create_file(&tz->device, &dev_attr_type);
- if (result)
- goto unregister;
+ /* hwmon sys I/F */
+ result = device_create_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2].dev_attr);
+ if (result)
+ goto unregister;
+
+ if (trips > 0) {
+ char buf[40];
+ result = tz->ops->get_trip_type(tz, 0, buf);
+ if (result > 0 && !strcmp(buf, "critical\n")) {
+ result = device_create_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2 + 1].dev_attr);
+ if (result)
+ goto unregister;
+ }
}
+ /* sys I/F */
+ result = device_create_file(&tz->device, &dev_attr_type);
+ if (result)
+ goto unregister;
+
result = device_create_file(&tz->device, &dev_attr_temp);
if (result)
goto unregister;
@@ -687,8 +778,17 @@ void thermal_zone_device_unregister(stru
tz->ops->unbind(tz, cdev);
mutex_unlock(&thermal_list_lock);
- if (tz->type[0])
- device_remove_file(&tz->device, &dev_attr_type);
+ device_remove_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2].dev_attr);
+ if (tz->trips > 0) {
+ char buf[40];
+ if (tz->ops->get_trip_type(tz, 0, buf) > 0)
+ if (!strcmp(buf, "critical\n"))
+ device_remove_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2 + 1].dev_attr);
+ }
+
+ device_remove_file(&tz->device, &dev_attr_type);
device_remove_file(&tz->device, &dev_attr_temp);
if (tz->ops->get_mode)
device_remove_file(&tz->device, &dev_attr_mode);
@@ -705,6 +805,19 @@ void thermal_zone_device_unregister(stru
EXPORT_SYMBOL(thermal_zone_device_unregister);
+static void __exit thermal_exit(void)
+{
+ if (thermal_hwmon) {
+ device_remove_file(thermal_hwmon, &dev_attr_name);
+ hwmon_device_unregister(thermal_hwmon);
+ }
+ class_unregister(&thermal_class);
+ idr_destroy(&thermal_tz_idr);
+ idr_destroy(&thermal_cdev_idr);
+ mutex_destroy(&thermal_idr_lock);
+ mutex_destroy(&thermal_list_lock);
+}
+
static int __init thermal_init(void)
{
int result = 0;
@@ -716,16 +829,20 @@ static int __init thermal_init(void)
mutex_destroy(&thermal_idr_lock);
mutex_destroy(&thermal_list_lock);
}
- return result;
-}
-static void __exit thermal_exit(void)
-{
- class_unregister(&thermal_class);
- idr_destroy(&thermal_tz_idr);
- idr_destroy(&thermal_cdev_idr);
- mutex_destroy(&thermal_idr_lock);
- mutex_destroy(&thermal_list_lock);
+ thermal_hwmon = hwmon_device_register(NULL);
+ if (IS_ERR(thermal_hwmon)) {
+ result = PTR_ERR(thermal_hwmon);
+ thermal_hwmon = NULL;
+ printk(KERN_ERR PREFIX
+ "unable to register hwmon device\n");
+ thermal_exit();
+ return result;
+ }
+
+ result = device_create_file(thermal_hwmon, &dev_attr_name);
+
+ return result;
}
subsys_initcall(thermal_init);
^ permalink raw reply [flat|nested] 50+ messages in thread
* [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-02-27 0:37 ` Zhang, Rui
0 siblings, 0 replies; 50+ messages in thread
From: Zhang, Rui @ 2008-02-27 0:37 UTC (permalink / raw)
To: Len Brown; +Cc: linux-acpi, lm-sensors, Hans de Goede
Add hwmon sys I/F for the generic thermal device.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
---
Documentation/thermal/sysfs-api.txt | 22 ++--
drivers/thermal/Kconfig | 1
drivers/thermal/thermal.c | 169 ++++++++++++++++++++++++++++++------
3 files changed, 155 insertions(+), 37 deletions(-)
Index: linux-2.6/Documentation/thermal/sysfs-api.txt
=================================--- linux-2.6.orig/Documentation/thermal/sysfs-api.txt
+++ linux-2.6/Documentation/thermal/sysfs-api.txt
@@ -143,10 +143,10 @@ type Strings which represent the ther
This is given by thermal zone driver as part of registration.
Eg: "ACPI thermal zone" indicates it's a ACPI thermal device
RO
- Optional
+ Required
temp Current temperature as reported by thermal zone (sensor)
- Unit: degree Celsius
+ Unit: millidegree Celsius
RO
Required
@@ -163,7 +163,7 @@ mode One of the predefined values in
charge of the thermal management.
trip_point_[0-*]_temp The temperature above which trip point will be fired
- Unit: degree Celsius
+ Unit: millidegree Celsius
RO
Optional
@@ -193,7 +193,7 @@ type String which represents the type
eg. For memory controller device on intel_menlow platform:
this should be "Memory controller"
RO
- Optional
+ Required
max_state The maximum permissible cooling state of this cooling device.
RO
@@ -219,16 +219,16 @@ the sys I/F structure will be built like
|thermal_zone1:
|-----type: ACPI thermal zone
- |-----temp: 37
+ |-----temp: 37000
|-----mode: kernel
- |-----trip_point_0_temp: 100
+ |-----trip_point_0_temp: 100000
|-----trip_point_0_type: critical
- |-----trip_point_1_temp: 80
+ |-----trip_point_1_temp: 80000
|-----trip_point_1_type: passive
- |-----trip_point_2_temp: 70
- |-----trip_point_2_type: active[0]
- |-----trip_point_3_temp: 60
- |-----trip_point_3_type: active[1]
+ |-----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
Index: linux-2.6/drivers/thermal/Kconfig
=================================--- linux-2.6.orig/drivers/thermal/Kconfig
+++ linux-2.6/drivers/thermal/Kconfig
@@ -4,6 +4,7 @@
menuconfig THERMAL
bool "Generic Thermal sysfs driver"
+ select HWMON
default y
help
Generic Thermal Sysfs driver offers a generic mechanism for
Index: linux-2.6/drivers/thermal/thermal.c
=================================--- linux-2.6.orig/drivers/thermal/thermal.c
+++ linux-2.6/drivers/thermal/thermal.c
@@ -30,8 +30,10 @@
#include <linux/idr.h>
#include <linux/thermal.h>
#include <linux/spinlock.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
-MODULE_AUTHOR("Zhang Rui")
+MODULE_AUTHOR("Zhang Rui");
MODULE_DESCRIPTION("Generic thermal management sysfs support");
MODULE_LICENSE("GPL");
@@ -56,6 +58,9 @@ static LIST_HEAD(thermal_tz_list);
static LIST_HEAD(thermal_cdev_list);
static DEFINE_MUTEX(thermal_list_lock);
+static struct device *thermal_hwmon;
+#define MAX_THERMAL_ZONES 10
+
static int get_idr(struct idr *idr, struct mutex *lock, int *id)
{
int err;
@@ -87,7 +92,67 @@ static void release_idr(struct idr *idr,
mutex_unlock(lock);
}
-/* sys I/F for thermal zone */
+/* hwmon sys I/F*/
+static ssize_t
+name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "thermal_sys_class\n");
+}
+
+static ssize_t
+temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct thermal_zone_device *tz;
+ struct sensor_device_attribute *sensor_attr
+ = to_sensor_dev_attr(attr);
+
+ list_for_each_entry(tz, &thermal_tz_list, node)
+ if (tz->id = sensor_attr->index)
+ return tz->ops->get_temp(tz, buf);
+
+ return -ENODEV;
+}
+
+static ssize_t
+temp_crit_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct thermal_zone_device *tz;
+ struct sensor_device_attribute *sensor_attr
+ = to_sensor_dev_attr(attr);
+
+ list_for_each_entry(tz, &thermal_tz_list, node)
+ if (tz->id = sensor_attr->index)
+ return tz->ops->get_trip_temp(tz, 0, buf);
+
+ return -ENODEV;
+}
+
+static DEVICE_ATTR(name, 0444, name_show, NULL);
+static struct sensor_device_attribute sensor_attrs[] = {
+ SENSOR_ATTR(temp1_input, 0444, temp_input_show, NULL, 0),
+ SENSOR_ATTR(temp1_crit, 0444, temp_crit_show, NULL, 0),
+ SENSOR_ATTR(temp2_input, 0444, temp_input_show, NULL, 1),
+ SENSOR_ATTR(temp2_crit, 0444, temp_crit_show, NULL, 1),
+ SENSOR_ATTR(temp3_input, 0444, temp_input_show, NULL, 2),
+ SENSOR_ATTR(temp3_crit, 0444, temp_crit_show, NULL, 2),
+ SENSOR_ATTR(temp4_input, 0444, temp_input_show, NULL, 3),
+ SENSOR_ATTR(temp4_crit, 0444, temp_crit_show, NULL, 3),
+ SENSOR_ATTR(temp5_input, 0444, temp_input_show, NULL, 4),
+ SENSOR_ATTR(temp5_crit, 0444, temp_crit_show, NULL, 4),
+ SENSOR_ATTR(temp6_input, 0444, temp_input_show, NULL, 5),
+ SENSOR_ATTR(temp6_crit, 0444, temp_crit_show, NULL, 5),
+ SENSOR_ATTR(temp7_input, 0444, temp_input_show, NULL, 6),
+ SENSOR_ATTR(temp7_crit, 0444, temp_crit_show, NULL, 6),
+ SENSOR_ATTR(temp8_input, 0444, temp_input_show, NULL, 7),
+ SENSOR_ATTR(temp8_crit, 0444, temp_crit_show, NULL, 7),
+ SENSOR_ATTR(temp9_input, 0444, temp_input_show, NULL, 8),
+ SENSOR_ATTR(temp9_crit, 0444, temp_crit_show, NULL, 8),
+ SENSOR_ATTR(temp10_input, 0444, temp_input_show, NULL, 9),
+ SENSOR_ATTR(temp10_crit, 0444, temp_crit_show, NULL, 9),
+};
+
+/* thermal zone sys I/F */
#define to_thermal_zone(_dev) \
container_of(_dev, struct thermal_zone_device, device)
@@ -214,7 +279,7 @@ do { \
device_remove_file(_dev, &trip_point_attrs[_index * 2 + 1]); \
} while (0)
-/* sys I/F for cooling device */
+/* cooling device sys I/F */
#define to_cooling_device(_dev) \
container_of(_dev, struct thermal_cooling_device, device)
@@ -447,6 +512,9 @@ struct thermal_cooling_device *thermal_c
struct thermal_zone_device *pos;
int result;
+ if (!type)
+ return ERR_PTR(-EINVAL);
+
if (strlen(type) >= THERMAL_NAME_LENGTH)
return ERR_PTR(-EINVAL);
@@ -477,11 +545,9 @@ struct thermal_cooling_device *thermal_c
}
/* sys I/F */
- if (type) {
- result = device_create_file(&cdev->device, &dev_attr_cdev_type);
- if (result)
- goto unregister;
- }
+ result = device_create_file(&cdev->device, &dev_attr_cdev_type);
+ if (result)
+ goto unregister;
result = device_create_file(&cdev->device, &dev_attr_max_state);
if (result)
@@ -547,8 +613,8 @@ void thermal_cooling_device_unregister(s
tz->ops->unbind(tz, cdev);
}
mutex_unlock(&thermal_list_lock);
- if (cdev->type[0])
- device_remove_file(&cdev->device, &dev_attr_cdev_type);
+
+ device_remove_file(&cdev->device, &dev_attr_cdev_type);
device_remove_file(&cdev->device, &dev_attr_max_state);
device_remove_file(&cdev->device, &dev_attr_cur_state);
@@ -580,6 +646,9 @@ struct thermal_zone_device *thermal_zone
int result;
int count;
+ if (!type)
+ return ERR_PTR(-EINVAL);
+
if (strlen(type) >= THERMAL_NAME_LENGTH)
return ERR_PTR(-EINVAL);
@@ -601,6 +670,13 @@ struct thermal_zone_device *thermal_zone
kfree(tz);
return ERR_PTR(result);
}
+ if (tz->id >= MAX_THERMAL_ZONES) {
+ printk(KERN_ERR PREFIX
+ "Too many thermal zones\n");
+ release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
+ kfree(tz);
+ return ERR_PTR(-EINVAL);
+ }
strcpy(tz->type, type);
tz->ops = ops;
@@ -615,13 +691,28 @@ struct thermal_zone_device *thermal_zone
return ERR_PTR(result);
}
- /* sys I/F */
- if (type) {
- result = device_create_file(&tz->device, &dev_attr_type);
- if (result)
- goto unregister;
+ /* hwmon sys I/F */
+ result = device_create_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2].dev_attr);
+ if (result)
+ goto unregister;
+
+ if (trips > 0) {
+ char buf[40];
+ result = tz->ops->get_trip_type(tz, 0, buf);
+ if (result > 0 && !strcmp(buf, "critical\n")) {
+ result = device_create_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2 + 1].dev_attr);
+ if (result)
+ goto unregister;
+ }
}
+ /* sys I/F */
+ result = device_create_file(&tz->device, &dev_attr_type);
+ if (result)
+ goto unregister;
+
result = device_create_file(&tz->device, &dev_attr_temp);
if (result)
goto unregister;
@@ -687,8 +778,17 @@ void thermal_zone_device_unregister(stru
tz->ops->unbind(tz, cdev);
mutex_unlock(&thermal_list_lock);
- if (tz->type[0])
- device_remove_file(&tz->device, &dev_attr_type);
+ device_remove_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2].dev_attr);
+ if (tz->trips > 0) {
+ char buf[40];
+ if (tz->ops->get_trip_type(tz, 0, buf) > 0)
+ if (!strcmp(buf, "critical\n"))
+ device_remove_file(thermal_hwmon,
+ &sensor_attrs[tz->id * 2 + 1].dev_attr);
+ }
+
+ device_remove_file(&tz->device, &dev_attr_type);
device_remove_file(&tz->device, &dev_attr_temp);
if (tz->ops->get_mode)
device_remove_file(&tz->device, &dev_attr_mode);
@@ -705,6 +805,19 @@ void thermal_zone_device_unregister(stru
EXPORT_SYMBOL(thermal_zone_device_unregister);
+static void __exit thermal_exit(void)
+{
+ if (thermal_hwmon) {
+ device_remove_file(thermal_hwmon, &dev_attr_name);
+ hwmon_device_unregister(thermal_hwmon);
+ }
+ class_unregister(&thermal_class);
+ idr_destroy(&thermal_tz_idr);
+ idr_destroy(&thermal_cdev_idr);
+ mutex_destroy(&thermal_idr_lock);
+ mutex_destroy(&thermal_list_lock);
+}
+
static int __init thermal_init(void)
{
int result = 0;
@@ -716,16 +829,20 @@ static int __init thermal_init(void)
mutex_destroy(&thermal_idr_lock);
mutex_destroy(&thermal_list_lock);
}
- return result;
-}
-static void __exit thermal_exit(void)
-{
- class_unregister(&thermal_class);
- idr_destroy(&thermal_tz_idr);
- idr_destroy(&thermal_cdev_idr);
- mutex_destroy(&thermal_idr_lock);
- mutex_destroy(&thermal_list_lock);
+ thermal_hwmon = hwmon_device_register(NULL);
+ if (IS_ERR(thermal_hwmon)) {
+ result = PTR_ERR(thermal_hwmon);
+ thermal_hwmon = NULL;
+ printk(KERN_ERR PREFIX
+ "unable to register hwmon device\n");
+ thermal_exit();
+ return result;
+ }
+
+ result = device_create_file(thermal_hwmon, &dev_attr_name);
+
+ return result;
}
subsys_initcall(thermal_init);
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-02-26 21:40 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
@ 2008-02-27 8:32 ` Hans de Goede
-1 siblings, 0 replies; 50+ messages in thread
From: Hans de Goede @ 2008-02-27 8:32 UTC (permalink / raw)
To: Zhang, Rui
Cc: linux-acpi, lm-sensors, Jean Delvare, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
Zhang, Rui wrote:
> On Tue, 2008-02-26 at 16:39 +0800, Hans de Goede wrote:
>> Zhang, Rui wrote:
>>> Add hwmon sys I/F for the generic thermal device.
>>>
>> Great!
>>
>> But I have several remarks:
>> 1) Looking at the new code, you only add temp1_input, so I'm guessing
>> that you
>> are registering a seperate hwmon class entry per zone? Please don't do
>> that,
>> please register one hwmon class entry, and add multiple temp#_input
>> attr to it
>> (and the same for crit).
>>
>> 2) I see that temp_crit may not be always available:
>> > +static ssize_t
>> > +crit_trip_temp_show(struct device *dev, struct device_attribute
>> *attr,
>> > + char *buf)
>> > +{
>> > + struct device *device = dev->parent;
>> > + struct thermal_zone_device *tz = to_thermal_zone(device);
>> > + int result;
>> > +
>> > + if (!tz->ops->get_trip_temp )
>> > + return -EPERM;
>> > +
>> > + /* assume trip 0 to be the critical trip point by default */
>> > + if (tz->ops->get_trip_type) {
>> > + result = tz->ops->get_trip_type(tz, 0, buf);
>> > + if (result < 0)
>> > + return result;
>> > + if (strcmp(buf, "critical\n"))
>> > + return -ENODEV;
>> > + }
>> > +
>> > + return tz->ops->get_trip_temp(tz, 0, buf);
>> > +}
>>
>> But you do always register a temp#_crit sysfs attr, it would be much
>> much
>> better to only add this if reading it actually has a chance of
>> returning a
>> value, so if tz->ops->get_trip_temp != NULL and
>> tz->ops->get_trip_type(tz, 0,
>> buf) returns "critical" in buf.
> then how about this one? :)
>
From a hwmon interface perspective its much better, thanks! As for doing a
full review, I'm afraid I'm not familiar enough with the code for that.
Regards,
Hans
> Add hwmon sys I/F for the generic thermal device.
>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
> ---
> Documentation/thermal/sysfs-api.txt | 22 ++--
> drivers/thermal/Kconfig | 1
> drivers/thermal/thermal.c | 161 ++++++++++++++++++++++++++++++------
> 3 files changed, 147 insertions(+), 37 deletions(-)
>
> Index: linux-2.6/Documentation/thermal/sysfs-api.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/thermal/sysfs-api.txt
> +++ linux-2.6/Documentation/thermal/sysfs-api.txt
> @@ -143,10 +143,10 @@ type Strings which represent the ther
> This is given by thermal zone driver as part of registration.
> Eg: "ACPI thermal zone" indicates it's a ACPI thermal device
> RO
> - Optional
> + Required
>
> temp Current temperature as reported by thermal zone (sensor)
> - Unit: degree Celsius
> + Unit: millidegree Celsius
> RO
> Required
>
> @@ -163,7 +163,7 @@ mode One of the predefined values in
> charge of the thermal management.
>
> trip_point_[0-*]_temp The temperature above which trip point will be fired
> - Unit: degree Celsius
> + Unit: millidegree Celsius
> RO
> Optional
>
> @@ -193,7 +193,7 @@ type String which represents the type
> eg. For memory controller device on intel_menlow platform:
> this should be "Memory controller"
> RO
> - Optional
> + Required
>
> max_state The maximum permissible cooling state of this cooling device.
> RO
> @@ -219,16 +219,16 @@ the sys I/F structure will be built like
>
> |thermal_zone1:
> |-----type: ACPI thermal zone
> - |-----temp: 37
> + |-----temp: 37000
> |-----mode: kernel
> - |-----trip_point_0_temp: 100
> + |-----trip_point_0_temp: 100000
> |-----trip_point_0_type: critical
> - |-----trip_point_1_temp: 80
> + |-----trip_point_1_temp: 80000
> |-----trip_point_1_type: passive
> - |-----trip_point_2_temp: 70
> - |-----trip_point_2_type: active[0]
> - |-----trip_point_3_temp: 60
> - |-----trip_point_3_type: active[1]
> + |-----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
> Index: linux-2.6/drivers/thermal/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/thermal/Kconfig
> +++ linux-2.6/drivers/thermal/Kconfig
> @@ -4,6 +4,7 @@
>
> menuconfig THERMAL
> bool "Generic Thermal sysfs driver"
> + select HWMON
> default y
> help
> Generic Thermal Sysfs driver offers a generic mechanism for
> Index: linux-2.6/drivers/thermal/thermal.c
> ===================================================================
> --- linux-2.6.orig/drivers/thermal/thermal.c
> +++ linux-2.6/drivers/thermal/thermal.c
> @@ -30,8 +30,10 @@
> #include <linux/idr.h>
> #include <linux/thermal.h>
> #include <linux/spinlock.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
>
> -MODULE_AUTHOR("Zhang Rui")
> +MODULE_AUTHOR("Zhang Rui");
> MODULE_DESCRIPTION("Generic thermal management sysfs support");
> MODULE_LICENSE("GPL");
>
> @@ -56,6 +58,8 @@ static LIST_HEAD(thermal_tz_list);
> static LIST_HEAD(thermal_cdev_list);
> static DEFINE_MUTEX(thermal_list_lock);
>
> +static struct device *thermal_hwmon;
> +
> static int get_idr(struct idr *idr, struct mutex *lock, int *id)
> {
> int err;
> @@ -87,7 +91,67 @@ static void release_idr(struct idr *idr,
> mutex_unlock(lock);
> }
>
> -/* sys I/F for thermal zone */
> +/* hwmon sys I/F*/
> +static ssize_t
> +name_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "thermal_sys_class\n");
> +}
> +
> +static ssize_t
> +temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct thermal_zone_device *tz;
> + struct sensor_device_attribute *sensor_attr
> + = to_sensor_dev_attr(attr);
> +
> + list_for_each_entry(tz, &thermal_tz_list, node)
> + if (tz->id == sensor_attr->index)
> + return tz->ops->get_temp(tz, buf);
> +
> + return -ENODEV;
> +}
> +
> +static ssize_t
> +temp_crit_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct thermal_zone_device *tz;
> + struct sensor_device_attribute *sensor_attr
> + = to_sensor_dev_attr(attr);
> +
> + list_for_each_entry(tz, &thermal_tz_list, node)
> + if (tz->id == sensor_attr->index)
> + return tz->ops->get_trip_temp(tz, 0, buf);
> +
> + return -ENODEV;
> +}
> +
> +static DEVICE_ATTR(name, 0444, name_show, NULL);
> +static struct sensor_device_attribute sensor_attrs[] = {
> + SENSOR_ATTR(temp1_input, 0444, temp_input_show, NULL, 0),
> + SENSOR_ATTR(temp1_crit, 0444, temp_crit_show, NULL, 0),
> + SENSOR_ATTR(temp2_input, 0444, temp_input_show, NULL, 1),
> + SENSOR_ATTR(temp2_crit, 0444, temp_crit_show, NULL, 1),
> + SENSOR_ATTR(temp3_input, 0444, temp_input_show, NULL, 2),
> + SENSOR_ATTR(temp3_crit, 0444, temp_crit_show, NULL, 2),
> + SENSOR_ATTR(temp4_input, 0444, temp_input_show, NULL, 3),
> + SENSOR_ATTR(temp4_crit, 0444, temp_crit_show, NULL, 3),
> + SENSOR_ATTR(temp5_input, 0444, temp_input_show, NULL, 4),
> + SENSOR_ATTR(temp5_crit, 0444, temp_crit_show, NULL, 4),
> + SENSOR_ATTR(temp6_input, 0444, temp_input_show, NULL, 5),
> + SENSOR_ATTR(temp6_crit, 0444, temp_crit_show, NULL, 5),
> + SENSOR_ATTR(temp7_input, 0444, temp_input_show, NULL, 6),
> + SENSOR_ATTR(temp7_crit, 0444, temp_crit_show, NULL, 6),
> + SENSOR_ATTR(temp8_input, 0444, temp_input_show, NULL, 7),
> + SENSOR_ATTR(temp8_crit, 0444, temp_crit_show, NULL, 7),
> + SENSOR_ATTR(temp9_input, 0444, temp_input_show, NULL, 8),
> + SENSOR_ATTR(temp9_crit, 0444, temp_crit_show, NULL, 8),
> + SENSOR_ATTR(temp10_input, 0444, temp_input_show, NULL, 9),
> + SENSOR_ATTR(temp10_crit, 0444, temp_crit_show, NULL, 9),
> +};
> +
> +/* thermal zone sys I/F */
>
> #define to_thermal_zone(_dev) \
> container_of(_dev, struct thermal_zone_device, device)
> @@ -214,7 +278,7 @@ do { \
> device_remove_file(_dev, &trip_point_attrs[_index * 2 + 1]); \
> } while (0)
>
> -/* sys I/F for cooling device */
> +/* cooling device sys I/F */
> #define to_cooling_device(_dev) \
> container_of(_dev, struct thermal_cooling_device, device)
>
> @@ -447,6 +511,9 @@ struct thermal_cooling_device *thermal_c
> struct thermal_zone_device *pos;
> int result;
>
> + if (!type)
> + return ERR_PTR(-EINVAL);
> +
> if (strlen(type) >= THERMAL_NAME_LENGTH)
> return ERR_PTR(-EINVAL);
>
> @@ -477,11 +544,9 @@ struct thermal_cooling_device *thermal_c
> }
>
> /* sys I/F */
> - if (type) {
> - result = device_create_file(&cdev->device, &dev_attr_cdev_type);
> - if (result)
> - goto unregister;
> - }
> + result = device_create_file(&cdev->device, &dev_attr_cdev_type);
> + if (result)
> + goto unregister;
>
> result = device_create_file(&cdev->device, &dev_attr_max_state);
> if (result)
> @@ -547,8 +612,8 @@ void thermal_cooling_device_unregister(s
> tz->ops->unbind(tz, cdev);
> }
> mutex_unlock(&thermal_list_lock);
> - if (cdev->type[0])
> - device_remove_file(&cdev->device, &dev_attr_cdev_type);
> +
> + device_remove_file(&cdev->device, &dev_attr_cdev_type);
> device_remove_file(&cdev->device, &dev_attr_max_state);
> device_remove_file(&cdev->device, &dev_attr_cur_state);
>
> @@ -580,6 +645,9 @@ struct thermal_zone_device *thermal_zone
> int result;
> int count;
>
> + if (!type)
> + return ERR_PTR(-EINVAL);
> +
> if (strlen(type) >= THERMAL_NAME_LENGTH)
> return ERR_PTR(-EINVAL);
>
> @@ -615,13 +683,28 @@ struct thermal_zone_device *thermal_zone
> return ERR_PTR(result);
> }
>
> - /* sys I/F */
> - if (type) {
> - result = device_create_file(&tz->device, &dev_attr_type);
> - if (result)
> - goto unregister;
> + /* hwmon sys I/F */
> + result = device_create_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2].dev_attr);
> + if (result)
> + goto unregister;
> +
> + if (trips > 0) {
> + char buf[40];
> + result = tz->ops->get_trip_type(tz, 0, buf);
> + if (result > 0 && !strcmp(buf, "critical\n")) {
> + result = device_create_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2 + 1].dev_attr);
> + if (result)
> + goto unregister;
> + }
> }
>
> + /* sys I/F */
> + result = device_create_file(&tz->device, &dev_attr_type);
> + if (result)
> + goto unregister;
> +
> result = device_create_file(&tz->device, &dev_attr_temp);
> if (result)
> goto unregister;
> @@ -687,8 +770,17 @@ void thermal_zone_device_unregister(stru
> tz->ops->unbind(tz, cdev);
> mutex_unlock(&thermal_list_lock);
>
> - if (tz->type[0])
> - device_remove_file(&tz->device, &dev_attr_type);
> + device_remove_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2].dev_attr);
> + if (tz->trips > 0) {
> + char buf[40];
> + if (tz->ops->get_trip_type(tz, 0, buf) > 0)
> + if (!strcmp(buf, "critical\n"))
> + device_remove_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2 + 1].dev_attr);
> + }
> +
> + device_remove_file(&tz->device, &dev_attr_type);
> device_remove_file(&tz->device, &dev_attr_temp);
> if (tz->ops->get_mode)
> device_remove_file(&tz->device, &dev_attr_mode);
> @@ -705,6 +797,19 @@ void thermal_zone_device_unregister(stru
>
> EXPORT_SYMBOL(thermal_zone_device_unregister);
>
> +static void __exit thermal_exit(void)
> +{
> + if (thermal_hwmon) {
> + device_remove_file(thermal_hwmon, &dev_attr_name);
> + hwmon_device_unregister(thermal_hwmon);
> + }
> + class_unregister(&thermal_class);
> + idr_destroy(&thermal_tz_idr);
> + idr_destroy(&thermal_cdev_idr);
> + mutex_destroy(&thermal_idr_lock);
> + mutex_destroy(&thermal_list_lock);
> +}
> +
> static int __init thermal_init(void)
> {
> int result = 0;
> @@ -716,16 +821,20 @@ static int __init thermal_init(void)
> mutex_destroy(&thermal_idr_lock);
> mutex_destroy(&thermal_list_lock);
> }
> - return result;
> -}
>
> -static void __exit thermal_exit(void)
> -{
> - class_unregister(&thermal_class);
> - idr_destroy(&thermal_tz_idr);
> - idr_destroy(&thermal_cdev_idr);
> - mutex_destroy(&thermal_idr_lock);
> - mutex_destroy(&thermal_list_lock);
> + thermal_hwmon = hwmon_device_register(NULL);
> + if (IS_ERR(thermal_hwmon)) {
> + result = PTR_ERR(thermal_hwmon);
> + thermal_hwmon = NULL;
> + printk(KERN_ERR PREFIX
> + "unable to register hwmon device\n");
> + thermal_exit();
> + return result;
> + }
> +
> + result = device_create_file(thermal_hwmon, &dev_attr_name);
> +
> + return result;
> }
>
> subsys_initcall(thermal_init);
>
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-02-27 8:32 ` Hans de Goede
0 siblings, 0 replies; 50+ messages in thread
From: Hans de Goede @ 2008-02-27 8:32 UTC (permalink / raw)
To: Zhang, Rui
Cc: linux-acpi, lm-sensors, Jean Delvare, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
Zhang, Rui wrote:
> On Tue, 2008-02-26 at 16:39 +0800, Hans de Goede wrote:
>> Zhang, Rui wrote:
>>> Add hwmon sys I/F for the generic thermal device.
>>>
>> Great!
>>
>> But I have several remarks:
>> 1) Looking at the new code, you only add temp1_input, so I'm guessing
>> that you
>> are registering a seperate hwmon class entry per zone? Please don't do
>> that,
>> please register one hwmon class entry, and add multiple temp#_input
>> attr to it
>> (and the same for crit).
>>
>> 2) I see that temp_crit may not be always available:
>> > +static ssize_t
>> > +crit_trip_temp_show(struct device *dev, struct device_attribute
>> *attr,
>> > + char *buf)
>> > +{
>> > + struct device *device = dev->parent;
>> > + struct thermal_zone_device *tz = to_thermal_zone(device);
>> > + int result;
>> > +
>> > + if (!tz->ops->get_trip_temp )
>> > + return -EPERM;
>> > +
>> > + /* assume trip 0 to be the critical trip point by default */
>> > + if (tz->ops->get_trip_type) {
>> > + result = tz->ops->get_trip_type(tz, 0, buf);
>> > + if (result < 0)
>> > + return result;
>> > + if (strcmp(buf, "critical\n"))
>> > + return -ENODEV;
>> > + }
>> > +
>> > + return tz->ops->get_trip_temp(tz, 0, buf);
>> > +}
>>
>> But you do always register a temp#_crit sysfs attr, it would be much
>> much
>> better to only add this if reading it actually has a chance of
>> returning a
>> value, so if tz->ops->get_trip_temp != NULL and
>> tz->ops->get_trip_type(tz, 0,
>> buf) returns "critical" in buf.
> then how about this one? :)
>
From a hwmon interface perspective its much better, thanks! As for doing a
full review, I'm afraid I'm not familiar enough with the code for that.
Regards,
Hans
> Add hwmon sys I/F for the generic thermal device.
>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
> ---
> Documentation/thermal/sysfs-api.txt | 22 ++--
> drivers/thermal/Kconfig | 1
> drivers/thermal/thermal.c | 161 ++++++++++++++++++++++++++++++------
> 3 files changed, 147 insertions(+), 37 deletions(-)
>
> Index: linux-2.6/Documentation/thermal/sysfs-api.txt
> =================================> --- linux-2.6.orig/Documentation/thermal/sysfs-api.txt
> +++ linux-2.6/Documentation/thermal/sysfs-api.txt
> @@ -143,10 +143,10 @@ type Strings which represent the ther
> This is given by thermal zone driver as part of registration.
> Eg: "ACPI thermal zone" indicates it's a ACPI thermal device
> RO
> - Optional
> + Required
>
> temp Current temperature as reported by thermal zone (sensor)
> - Unit: degree Celsius
> + Unit: millidegree Celsius
> RO
> Required
>
> @@ -163,7 +163,7 @@ mode One of the predefined values in
> charge of the thermal management.
>
> trip_point_[0-*]_temp The temperature above which trip point will be fired
> - Unit: degree Celsius
> + Unit: millidegree Celsius
> RO
> Optional
>
> @@ -193,7 +193,7 @@ type String which represents the type
> eg. For memory controller device on intel_menlow platform:
> this should be "Memory controller"
> RO
> - Optional
> + Required
>
> max_state The maximum permissible cooling state of this cooling device.
> RO
> @@ -219,16 +219,16 @@ the sys I/F structure will be built like
>
> |thermal_zone1:
> |-----type: ACPI thermal zone
> - |-----temp: 37
> + |-----temp: 37000
> |-----mode: kernel
> - |-----trip_point_0_temp: 100
> + |-----trip_point_0_temp: 100000
> |-----trip_point_0_type: critical
> - |-----trip_point_1_temp: 80
> + |-----trip_point_1_temp: 80000
> |-----trip_point_1_type: passive
> - |-----trip_point_2_temp: 70
> - |-----trip_point_2_type: active[0]
> - |-----trip_point_3_temp: 60
> - |-----trip_point_3_type: active[1]
> + |-----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
> Index: linux-2.6/drivers/thermal/Kconfig
> =================================> --- linux-2.6.orig/drivers/thermal/Kconfig
> +++ linux-2.6/drivers/thermal/Kconfig
> @@ -4,6 +4,7 @@
>
> menuconfig THERMAL
> bool "Generic Thermal sysfs driver"
> + select HWMON
> default y
> help
> Generic Thermal Sysfs driver offers a generic mechanism for
> Index: linux-2.6/drivers/thermal/thermal.c
> =================================> --- linux-2.6.orig/drivers/thermal/thermal.c
> +++ linux-2.6/drivers/thermal/thermal.c
> @@ -30,8 +30,10 @@
> #include <linux/idr.h>
> #include <linux/thermal.h>
> #include <linux/spinlock.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
>
> -MODULE_AUTHOR("Zhang Rui")
> +MODULE_AUTHOR("Zhang Rui");
> MODULE_DESCRIPTION("Generic thermal management sysfs support");
> MODULE_LICENSE("GPL");
>
> @@ -56,6 +58,8 @@ static LIST_HEAD(thermal_tz_list);
> static LIST_HEAD(thermal_cdev_list);
> static DEFINE_MUTEX(thermal_list_lock);
>
> +static struct device *thermal_hwmon;
> +
> static int get_idr(struct idr *idr, struct mutex *lock, int *id)
> {
> int err;
> @@ -87,7 +91,67 @@ static void release_idr(struct idr *idr,
> mutex_unlock(lock);
> }
>
> -/* sys I/F for thermal zone */
> +/* hwmon sys I/F*/
> +static ssize_t
> +name_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "thermal_sys_class\n");
> +}
> +
> +static ssize_t
> +temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct thermal_zone_device *tz;
> + struct sensor_device_attribute *sensor_attr
> + = to_sensor_dev_attr(attr);
> +
> + list_for_each_entry(tz, &thermal_tz_list, node)
> + if (tz->id = sensor_attr->index)
> + return tz->ops->get_temp(tz, buf);
> +
> + return -ENODEV;
> +}
> +
> +static ssize_t
> +temp_crit_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct thermal_zone_device *tz;
> + struct sensor_device_attribute *sensor_attr
> + = to_sensor_dev_attr(attr);
> +
> + list_for_each_entry(tz, &thermal_tz_list, node)
> + if (tz->id = sensor_attr->index)
> + return tz->ops->get_trip_temp(tz, 0, buf);
> +
> + return -ENODEV;
> +}
> +
> +static DEVICE_ATTR(name, 0444, name_show, NULL);
> +static struct sensor_device_attribute sensor_attrs[] = {
> + SENSOR_ATTR(temp1_input, 0444, temp_input_show, NULL, 0),
> + SENSOR_ATTR(temp1_crit, 0444, temp_crit_show, NULL, 0),
> + SENSOR_ATTR(temp2_input, 0444, temp_input_show, NULL, 1),
> + SENSOR_ATTR(temp2_crit, 0444, temp_crit_show, NULL, 1),
> + SENSOR_ATTR(temp3_input, 0444, temp_input_show, NULL, 2),
> + SENSOR_ATTR(temp3_crit, 0444, temp_crit_show, NULL, 2),
> + SENSOR_ATTR(temp4_input, 0444, temp_input_show, NULL, 3),
> + SENSOR_ATTR(temp4_crit, 0444, temp_crit_show, NULL, 3),
> + SENSOR_ATTR(temp5_input, 0444, temp_input_show, NULL, 4),
> + SENSOR_ATTR(temp5_crit, 0444, temp_crit_show, NULL, 4),
> + SENSOR_ATTR(temp6_input, 0444, temp_input_show, NULL, 5),
> + SENSOR_ATTR(temp6_crit, 0444, temp_crit_show, NULL, 5),
> + SENSOR_ATTR(temp7_input, 0444, temp_input_show, NULL, 6),
> + SENSOR_ATTR(temp7_crit, 0444, temp_crit_show, NULL, 6),
> + SENSOR_ATTR(temp8_input, 0444, temp_input_show, NULL, 7),
> + SENSOR_ATTR(temp8_crit, 0444, temp_crit_show, NULL, 7),
> + SENSOR_ATTR(temp9_input, 0444, temp_input_show, NULL, 8),
> + SENSOR_ATTR(temp9_crit, 0444, temp_crit_show, NULL, 8),
> + SENSOR_ATTR(temp10_input, 0444, temp_input_show, NULL, 9),
> + SENSOR_ATTR(temp10_crit, 0444, temp_crit_show, NULL, 9),
> +};
> +
> +/* thermal zone sys I/F */
>
> #define to_thermal_zone(_dev) \
> container_of(_dev, struct thermal_zone_device, device)
> @@ -214,7 +278,7 @@ do { \
> device_remove_file(_dev, &trip_point_attrs[_index * 2 + 1]); \
> } while (0)
>
> -/* sys I/F for cooling device */
> +/* cooling device sys I/F */
> #define to_cooling_device(_dev) \
> container_of(_dev, struct thermal_cooling_device, device)
>
> @@ -447,6 +511,9 @@ struct thermal_cooling_device *thermal_c
> struct thermal_zone_device *pos;
> int result;
>
> + if (!type)
> + return ERR_PTR(-EINVAL);
> +
> if (strlen(type) >= THERMAL_NAME_LENGTH)
> return ERR_PTR(-EINVAL);
>
> @@ -477,11 +544,9 @@ struct thermal_cooling_device *thermal_c
> }
>
> /* sys I/F */
> - if (type) {
> - result = device_create_file(&cdev->device, &dev_attr_cdev_type);
> - if (result)
> - goto unregister;
> - }
> + result = device_create_file(&cdev->device, &dev_attr_cdev_type);
> + if (result)
> + goto unregister;
>
> result = device_create_file(&cdev->device, &dev_attr_max_state);
> if (result)
> @@ -547,8 +612,8 @@ void thermal_cooling_device_unregister(s
> tz->ops->unbind(tz, cdev);
> }
> mutex_unlock(&thermal_list_lock);
> - if (cdev->type[0])
> - device_remove_file(&cdev->device, &dev_attr_cdev_type);
> +
> + device_remove_file(&cdev->device, &dev_attr_cdev_type);
> device_remove_file(&cdev->device, &dev_attr_max_state);
> device_remove_file(&cdev->device, &dev_attr_cur_state);
>
> @@ -580,6 +645,9 @@ struct thermal_zone_device *thermal_zone
> int result;
> int count;
>
> + if (!type)
> + return ERR_PTR(-EINVAL);
> +
> if (strlen(type) >= THERMAL_NAME_LENGTH)
> return ERR_PTR(-EINVAL);
>
> @@ -615,13 +683,28 @@ struct thermal_zone_device *thermal_zone
> return ERR_PTR(result);
> }
>
> - /* sys I/F */
> - if (type) {
> - result = device_create_file(&tz->device, &dev_attr_type);
> - if (result)
> - goto unregister;
> + /* hwmon sys I/F */
> + result = device_create_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2].dev_attr);
> + if (result)
> + goto unregister;
> +
> + if (trips > 0) {
> + char buf[40];
> + result = tz->ops->get_trip_type(tz, 0, buf);
> + if (result > 0 && !strcmp(buf, "critical\n")) {
> + result = device_create_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2 + 1].dev_attr);
> + if (result)
> + goto unregister;
> + }
> }
>
> + /* sys I/F */
> + result = device_create_file(&tz->device, &dev_attr_type);
> + if (result)
> + goto unregister;
> +
> result = device_create_file(&tz->device, &dev_attr_temp);
> if (result)
> goto unregister;
> @@ -687,8 +770,17 @@ void thermal_zone_device_unregister(stru
> tz->ops->unbind(tz, cdev);
> mutex_unlock(&thermal_list_lock);
>
> - if (tz->type[0])
> - device_remove_file(&tz->device, &dev_attr_type);
> + device_remove_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2].dev_attr);
> + if (tz->trips > 0) {
> + char buf[40];
> + if (tz->ops->get_trip_type(tz, 0, buf) > 0)
> + if (!strcmp(buf, "critical\n"))
> + device_remove_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2 + 1].dev_attr);
> + }
> +
> + device_remove_file(&tz->device, &dev_attr_type);
> device_remove_file(&tz->device, &dev_attr_temp);
> if (tz->ops->get_mode)
> device_remove_file(&tz->device, &dev_attr_mode);
> @@ -705,6 +797,19 @@ void thermal_zone_device_unregister(stru
>
> EXPORT_SYMBOL(thermal_zone_device_unregister);
>
> +static void __exit thermal_exit(void)
> +{
> + if (thermal_hwmon) {
> + device_remove_file(thermal_hwmon, &dev_attr_name);
> + hwmon_device_unregister(thermal_hwmon);
> + }
> + class_unregister(&thermal_class);
> + idr_destroy(&thermal_tz_idr);
> + idr_destroy(&thermal_cdev_idr);
> + mutex_destroy(&thermal_idr_lock);
> + mutex_destroy(&thermal_list_lock);
> +}
> +
> static int __init thermal_init(void)
> {
> int result = 0;
> @@ -716,16 +821,20 @@ static int __init thermal_init(void)
> mutex_destroy(&thermal_idr_lock);
> mutex_destroy(&thermal_list_lock);
> }
> - return result;
> -}
>
> -static void __exit thermal_exit(void)
> -{
> - class_unregister(&thermal_class);
> - idr_destroy(&thermal_tz_idr);
> - idr_destroy(&thermal_cdev_idr);
> - mutex_destroy(&thermal_idr_lock);
> - mutex_destroy(&thermal_list_lock);
> + thermal_hwmon = hwmon_device_register(NULL);
> + if (IS_ERR(thermal_hwmon)) {
> + result = PTR_ERR(thermal_hwmon);
> + thermal_hwmon = NULL;
> + printk(KERN_ERR PREFIX
> + "unable to register hwmon device\n");
> + thermal_exit();
> + return result;
> + }
> +
> + result = device_create_file(thermal_hwmon, &dev_attr_name);
> +
> + return result;
> }
>
> subsys_initcall(thermal_init);
>
>
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-02-27 0:37 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
@ 2008-03-12 4:29 ` Len Brown
-1 siblings, 0 replies; 50+ messages in thread
From: Len Brown @ 2008-03-12 4:29 UTC (permalink / raw)
To: Zhang, Rui; +Cc: linux-acpi, lm-sensors, Hans de Goede
Applied.
thanks,
-Len
On Tuesday 26 February 2008, Zhang, Rui wrote:
>
> Add hwmon sys I/F for the generic thermal device.
>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
> ---
> Documentation/thermal/sysfs-api.txt | 22 ++--
> drivers/thermal/Kconfig | 1
> drivers/thermal/thermal.c | 169 ++++++++++++++++++++++++++++++------
> 3 files changed, 155 insertions(+), 37 deletions(-)
>
> Index: linux-2.6/Documentation/thermal/sysfs-api.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/thermal/sysfs-api.txt
> +++ linux-2.6/Documentation/thermal/sysfs-api.txt
> @@ -143,10 +143,10 @@ type Strings which represent the ther
> This is given by thermal zone driver as part of registration.
> Eg: "ACPI thermal zone" indicates it's a ACPI thermal device
> RO
> - Optional
> + Required
>
> temp Current temperature as reported by thermal zone (sensor)
> - Unit: degree Celsius
> + Unit: millidegree Celsius
> RO
> Required
>
> @@ -163,7 +163,7 @@ mode One of the predefined values in
> charge of the thermal management.
>
> trip_point_[0-*]_temp The temperature above which trip point will be fired
> - Unit: degree Celsius
> + Unit: millidegree Celsius
> RO
> Optional
>
> @@ -193,7 +193,7 @@ type String which represents the type
> eg. For memory controller device on intel_menlow platform:
> this should be "Memory controller"
> RO
> - Optional
> + Required
>
> max_state The maximum permissible cooling state of this cooling device.
> RO
> @@ -219,16 +219,16 @@ the sys I/F structure will be built like
>
> |thermal_zone1:
> |-----type: ACPI thermal zone
> - |-----temp: 37
> + |-----temp: 37000
> |-----mode: kernel
> - |-----trip_point_0_temp: 100
> + |-----trip_point_0_temp: 100000
> |-----trip_point_0_type: critical
> - |-----trip_point_1_temp: 80
> + |-----trip_point_1_temp: 80000
> |-----trip_point_1_type: passive
> - |-----trip_point_2_temp: 70
> - |-----trip_point_2_type: active[0]
> - |-----trip_point_3_temp: 60
> - |-----trip_point_3_type: active[1]
> + |-----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
> Index: linux-2.6/drivers/thermal/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/thermal/Kconfig
> +++ linux-2.6/drivers/thermal/Kconfig
> @@ -4,6 +4,7 @@
>
> menuconfig THERMAL
> bool "Generic Thermal sysfs driver"
> + select HWMON
> default y
> help
> Generic Thermal Sysfs driver offers a generic mechanism for
> Index: linux-2.6/drivers/thermal/thermal.c
> ===================================================================
> --- linux-2.6.orig/drivers/thermal/thermal.c
> +++ linux-2.6/drivers/thermal/thermal.c
> @@ -30,8 +30,10 @@
> #include <linux/idr.h>
> #include <linux/thermal.h>
> #include <linux/spinlock.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
>
> -MODULE_AUTHOR("Zhang Rui")
> +MODULE_AUTHOR("Zhang Rui");
> MODULE_DESCRIPTION("Generic thermal management sysfs support");
> MODULE_LICENSE("GPL");
>
> @@ -56,6 +58,9 @@ static LIST_HEAD(thermal_tz_list);
> static LIST_HEAD(thermal_cdev_list);
> static DEFINE_MUTEX(thermal_list_lock);
>
> +static struct device *thermal_hwmon;
> +#define MAX_THERMAL_ZONES 10
> +
> static int get_idr(struct idr *idr, struct mutex *lock, int *id)
> {
> int err;
> @@ -87,7 +92,67 @@ static void release_idr(struct idr *idr,
> mutex_unlock(lock);
> }
>
> -/* sys I/F for thermal zone */
> +/* hwmon sys I/F*/
> +static ssize_t
> +name_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "thermal_sys_class\n");
> +}
> +
> +static ssize_t
> +temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct thermal_zone_device *tz;
> + struct sensor_device_attribute *sensor_attr
> + = to_sensor_dev_attr(attr);
> +
> + list_for_each_entry(tz, &thermal_tz_list, node)
> + if (tz->id == sensor_attr->index)
> + return tz->ops->get_temp(tz, buf);
> +
> + return -ENODEV;
> +}
> +
> +static ssize_t
> +temp_crit_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct thermal_zone_device *tz;
> + struct sensor_device_attribute *sensor_attr
> + = to_sensor_dev_attr(attr);
> +
> + list_for_each_entry(tz, &thermal_tz_list, node)
> + if (tz->id == sensor_attr->index)
> + return tz->ops->get_trip_temp(tz, 0, buf);
> +
> + return -ENODEV;
> +}
> +
> +static DEVICE_ATTR(name, 0444, name_show, NULL);
> +static struct sensor_device_attribute sensor_attrs[] = {
> + SENSOR_ATTR(temp1_input, 0444, temp_input_show, NULL, 0),
> + SENSOR_ATTR(temp1_crit, 0444, temp_crit_show, NULL, 0),
> + SENSOR_ATTR(temp2_input, 0444, temp_input_show, NULL, 1),
> + SENSOR_ATTR(temp2_crit, 0444, temp_crit_show, NULL, 1),
> + SENSOR_ATTR(temp3_input, 0444, temp_input_show, NULL, 2),
> + SENSOR_ATTR(temp3_crit, 0444, temp_crit_show, NULL, 2),
> + SENSOR_ATTR(temp4_input, 0444, temp_input_show, NULL, 3),
> + SENSOR_ATTR(temp4_crit, 0444, temp_crit_show, NULL, 3),
> + SENSOR_ATTR(temp5_input, 0444, temp_input_show, NULL, 4),
> + SENSOR_ATTR(temp5_crit, 0444, temp_crit_show, NULL, 4),
> + SENSOR_ATTR(temp6_input, 0444, temp_input_show, NULL, 5),
> + SENSOR_ATTR(temp6_crit, 0444, temp_crit_show, NULL, 5),
> + SENSOR_ATTR(temp7_input, 0444, temp_input_show, NULL, 6),
> + SENSOR_ATTR(temp7_crit, 0444, temp_crit_show, NULL, 6),
> + SENSOR_ATTR(temp8_input, 0444, temp_input_show, NULL, 7),
> + SENSOR_ATTR(temp8_crit, 0444, temp_crit_show, NULL, 7),
> + SENSOR_ATTR(temp9_input, 0444, temp_input_show, NULL, 8),
> + SENSOR_ATTR(temp9_crit, 0444, temp_crit_show, NULL, 8),
> + SENSOR_ATTR(temp10_input, 0444, temp_input_show, NULL, 9),
> + SENSOR_ATTR(temp10_crit, 0444, temp_crit_show, NULL, 9),
> +};
> +
> +/* thermal zone sys I/F */
>
> #define to_thermal_zone(_dev) \
> container_of(_dev, struct thermal_zone_device, device)
> @@ -214,7 +279,7 @@ do { \
> device_remove_file(_dev, &trip_point_attrs[_index * 2 + 1]); \
> } while (0)
>
> -/* sys I/F for cooling device */
> +/* cooling device sys I/F */
> #define to_cooling_device(_dev) \
> container_of(_dev, struct thermal_cooling_device, device)
>
> @@ -447,6 +512,9 @@ struct thermal_cooling_device *thermal_c
> struct thermal_zone_device *pos;
> int result;
>
> + if (!type)
> + return ERR_PTR(-EINVAL);
> +
> if (strlen(type) >= THERMAL_NAME_LENGTH)
> return ERR_PTR(-EINVAL);
>
> @@ -477,11 +545,9 @@ struct thermal_cooling_device *thermal_c
> }
>
> /* sys I/F */
> - if (type) {
> - result = device_create_file(&cdev->device, &dev_attr_cdev_type);
> - if (result)
> - goto unregister;
> - }
> + result = device_create_file(&cdev->device, &dev_attr_cdev_type);
> + if (result)
> + goto unregister;
>
> result = device_create_file(&cdev->device, &dev_attr_max_state);
> if (result)
> @@ -547,8 +613,8 @@ void thermal_cooling_device_unregister(s
> tz->ops->unbind(tz, cdev);
> }
> mutex_unlock(&thermal_list_lock);
> - if (cdev->type[0])
> - device_remove_file(&cdev->device, &dev_attr_cdev_type);
> +
> + device_remove_file(&cdev->device, &dev_attr_cdev_type);
> device_remove_file(&cdev->device, &dev_attr_max_state);
> device_remove_file(&cdev->device, &dev_attr_cur_state);
>
> @@ -580,6 +646,9 @@ struct thermal_zone_device *thermal_zone
> int result;
> int count;
>
> + if (!type)
> + return ERR_PTR(-EINVAL);
> +
> if (strlen(type) >= THERMAL_NAME_LENGTH)
> return ERR_PTR(-EINVAL);
>
> @@ -601,6 +670,13 @@ struct thermal_zone_device *thermal_zone
> kfree(tz);
> return ERR_PTR(result);
> }
> + if (tz->id >= MAX_THERMAL_ZONES) {
> + printk(KERN_ERR PREFIX
> + "Too many thermal zones\n");
> + release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
> + kfree(tz);
> + return ERR_PTR(-EINVAL);
> + }
>
> strcpy(tz->type, type);
> tz->ops = ops;
> @@ -615,13 +691,28 @@ struct thermal_zone_device *thermal_zone
> return ERR_PTR(result);
> }
>
> - /* sys I/F */
> - if (type) {
> - result = device_create_file(&tz->device, &dev_attr_type);
> - if (result)
> - goto unregister;
> + /* hwmon sys I/F */
> + result = device_create_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2].dev_attr);
> + if (result)
> + goto unregister;
> +
> + if (trips > 0) {
> + char buf[40];
> + result = tz->ops->get_trip_type(tz, 0, buf);
> + if (result > 0 && !strcmp(buf, "critical\n")) {
> + result = device_create_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2 + 1].dev_attr);
> + if (result)
> + goto unregister;
> + }
> }
>
> + /* sys I/F */
> + result = device_create_file(&tz->device, &dev_attr_type);
> + if (result)
> + goto unregister;
> +
> result = device_create_file(&tz->device, &dev_attr_temp);
> if (result)
> goto unregister;
> @@ -687,8 +778,17 @@ void thermal_zone_device_unregister(stru
> tz->ops->unbind(tz, cdev);
> mutex_unlock(&thermal_list_lock);
>
> - if (tz->type[0])
> - device_remove_file(&tz->device, &dev_attr_type);
> + device_remove_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2].dev_attr);
> + if (tz->trips > 0) {
> + char buf[40];
> + if (tz->ops->get_trip_type(tz, 0, buf) > 0)
> + if (!strcmp(buf, "critical\n"))
> + device_remove_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2 + 1].dev_attr);
> + }
> +
> + device_remove_file(&tz->device, &dev_attr_type);
> device_remove_file(&tz->device, &dev_attr_temp);
> if (tz->ops->get_mode)
> device_remove_file(&tz->device, &dev_attr_mode);
> @@ -705,6 +805,19 @@ void thermal_zone_device_unregister(stru
>
> EXPORT_SYMBOL(thermal_zone_device_unregister);
>
> +static void __exit thermal_exit(void)
> +{
> + if (thermal_hwmon) {
> + device_remove_file(thermal_hwmon, &dev_attr_name);
> + hwmon_device_unregister(thermal_hwmon);
> + }
> + class_unregister(&thermal_class);
> + idr_destroy(&thermal_tz_idr);
> + idr_destroy(&thermal_cdev_idr);
> + mutex_destroy(&thermal_idr_lock);
> + mutex_destroy(&thermal_list_lock);
> +}
> +
> static int __init thermal_init(void)
> {
> int result = 0;
> @@ -716,16 +829,20 @@ static int __init thermal_init(void)
> mutex_destroy(&thermal_idr_lock);
> mutex_destroy(&thermal_list_lock);
> }
> - return result;
> -}
>
> -static void __exit thermal_exit(void)
> -{
> - class_unregister(&thermal_class);
> - idr_destroy(&thermal_tz_idr);
> - idr_destroy(&thermal_cdev_idr);
> - mutex_destroy(&thermal_idr_lock);
> - mutex_destroy(&thermal_list_lock);
> + thermal_hwmon = hwmon_device_register(NULL);
> + if (IS_ERR(thermal_hwmon)) {
> + result = PTR_ERR(thermal_hwmon);
> + thermal_hwmon = NULL;
> + printk(KERN_ERR PREFIX
> + "unable to register hwmon device\n");
> + thermal_exit();
> + return result;
> + }
> +
> + result = device_create_file(thermal_hwmon, &dev_attr_name);
> +
> + return result;
> }
>
> subsys_initcall(thermal_init);
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-12 4:29 ` Len Brown
0 siblings, 0 replies; 50+ messages in thread
From: Len Brown @ 2008-03-12 4:29 UTC (permalink / raw)
To: Zhang, Rui; +Cc: linux-acpi, lm-sensors, Hans de Goede
Applied.
thanks,
-Len
On Tuesday 26 February 2008, Zhang, Rui wrote:
>
> Add hwmon sys I/F for the generic thermal device.
>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
> ---
> Documentation/thermal/sysfs-api.txt | 22 ++--
> drivers/thermal/Kconfig | 1
> drivers/thermal/thermal.c | 169 ++++++++++++++++++++++++++++++------
> 3 files changed, 155 insertions(+), 37 deletions(-)
>
> Index: linux-2.6/Documentation/thermal/sysfs-api.txt
> =================================> --- linux-2.6.orig/Documentation/thermal/sysfs-api.txt
> +++ linux-2.6/Documentation/thermal/sysfs-api.txt
> @@ -143,10 +143,10 @@ type Strings which represent the ther
> This is given by thermal zone driver as part of registration.
> Eg: "ACPI thermal zone" indicates it's a ACPI thermal device
> RO
> - Optional
> + Required
>
> temp Current temperature as reported by thermal zone (sensor)
> - Unit: degree Celsius
> + Unit: millidegree Celsius
> RO
> Required
>
> @@ -163,7 +163,7 @@ mode One of the predefined values in
> charge of the thermal management.
>
> trip_point_[0-*]_temp The temperature above which trip point will be fired
> - Unit: degree Celsius
> + Unit: millidegree Celsius
> RO
> Optional
>
> @@ -193,7 +193,7 @@ type String which represents the type
> eg. For memory controller device on intel_menlow platform:
> this should be "Memory controller"
> RO
> - Optional
> + Required
>
> max_state The maximum permissible cooling state of this cooling device.
> RO
> @@ -219,16 +219,16 @@ the sys I/F structure will be built like
>
> |thermal_zone1:
> |-----type: ACPI thermal zone
> - |-----temp: 37
> + |-----temp: 37000
> |-----mode: kernel
> - |-----trip_point_0_temp: 100
> + |-----trip_point_0_temp: 100000
> |-----trip_point_0_type: critical
> - |-----trip_point_1_temp: 80
> + |-----trip_point_1_temp: 80000
> |-----trip_point_1_type: passive
> - |-----trip_point_2_temp: 70
> - |-----trip_point_2_type: active[0]
> - |-----trip_point_3_temp: 60
> - |-----trip_point_3_type: active[1]
> + |-----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
> Index: linux-2.6/drivers/thermal/Kconfig
> =================================> --- linux-2.6.orig/drivers/thermal/Kconfig
> +++ linux-2.6/drivers/thermal/Kconfig
> @@ -4,6 +4,7 @@
>
> menuconfig THERMAL
> bool "Generic Thermal sysfs driver"
> + select HWMON
> default y
> help
> Generic Thermal Sysfs driver offers a generic mechanism for
> Index: linux-2.6/drivers/thermal/thermal.c
> =================================> --- linux-2.6.orig/drivers/thermal/thermal.c
> +++ linux-2.6/drivers/thermal/thermal.c
> @@ -30,8 +30,10 @@
> #include <linux/idr.h>
> #include <linux/thermal.h>
> #include <linux/spinlock.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
>
> -MODULE_AUTHOR("Zhang Rui")
> +MODULE_AUTHOR("Zhang Rui");
> MODULE_DESCRIPTION("Generic thermal management sysfs support");
> MODULE_LICENSE("GPL");
>
> @@ -56,6 +58,9 @@ static LIST_HEAD(thermal_tz_list);
> static LIST_HEAD(thermal_cdev_list);
> static DEFINE_MUTEX(thermal_list_lock);
>
> +static struct device *thermal_hwmon;
> +#define MAX_THERMAL_ZONES 10
> +
> static int get_idr(struct idr *idr, struct mutex *lock, int *id)
> {
> int err;
> @@ -87,7 +92,67 @@ static void release_idr(struct idr *idr,
> mutex_unlock(lock);
> }
>
> -/* sys I/F for thermal zone */
> +/* hwmon sys I/F*/
> +static ssize_t
> +name_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "thermal_sys_class\n");
> +}
> +
> +static ssize_t
> +temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct thermal_zone_device *tz;
> + struct sensor_device_attribute *sensor_attr
> + = to_sensor_dev_attr(attr);
> +
> + list_for_each_entry(tz, &thermal_tz_list, node)
> + if (tz->id = sensor_attr->index)
> + return tz->ops->get_temp(tz, buf);
> +
> + return -ENODEV;
> +}
> +
> +static ssize_t
> +temp_crit_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct thermal_zone_device *tz;
> + struct sensor_device_attribute *sensor_attr
> + = to_sensor_dev_attr(attr);
> +
> + list_for_each_entry(tz, &thermal_tz_list, node)
> + if (tz->id = sensor_attr->index)
> + return tz->ops->get_trip_temp(tz, 0, buf);
> +
> + return -ENODEV;
> +}
> +
> +static DEVICE_ATTR(name, 0444, name_show, NULL);
> +static struct sensor_device_attribute sensor_attrs[] = {
> + SENSOR_ATTR(temp1_input, 0444, temp_input_show, NULL, 0),
> + SENSOR_ATTR(temp1_crit, 0444, temp_crit_show, NULL, 0),
> + SENSOR_ATTR(temp2_input, 0444, temp_input_show, NULL, 1),
> + SENSOR_ATTR(temp2_crit, 0444, temp_crit_show, NULL, 1),
> + SENSOR_ATTR(temp3_input, 0444, temp_input_show, NULL, 2),
> + SENSOR_ATTR(temp3_crit, 0444, temp_crit_show, NULL, 2),
> + SENSOR_ATTR(temp4_input, 0444, temp_input_show, NULL, 3),
> + SENSOR_ATTR(temp4_crit, 0444, temp_crit_show, NULL, 3),
> + SENSOR_ATTR(temp5_input, 0444, temp_input_show, NULL, 4),
> + SENSOR_ATTR(temp5_crit, 0444, temp_crit_show, NULL, 4),
> + SENSOR_ATTR(temp6_input, 0444, temp_input_show, NULL, 5),
> + SENSOR_ATTR(temp6_crit, 0444, temp_crit_show, NULL, 5),
> + SENSOR_ATTR(temp7_input, 0444, temp_input_show, NULL, 6),
> + SENSOR_ATTR(temp7_crit, 0444, temp_crit_show, NULL, 6),
> + SENSOR_ATTR(temp8_input, 0444, temp_input_show, NULL, 7),
> + SENSOR_ATTR(temp8_crit, 0444, temp_crit_show, NULL, 7),
> + SENSOR_ATTR(temp9_input, 0444, temp_input_show, NULL, 8),
> + SENSOR_ATTR(temp9_crit, 0444, temp_crit_show, NULL, 8),
> + SENSOR_ATTR(temp10_input, 0444, temp_input_show, NULL, 9),
> + SENSOR_ATTR(temp10_crit, 0444, temp_crit_show, NULL, 9),
> +};
> +
> +/* thermal zone sys I/F */
>
> #define to_thermal_zone(_dev) \
> container_of(_dev, struct thermal_zone_device, device)
> @@ -214,7 +279,7 @@ do { \
> device_remove_file(_dev, &trip_point_attrs[_index * 2 + 1]); \
> } while (0)
>
> -/* sys I/F for cooling device */
> +/* cooling device sys I/F */
> #define to_cooling_device(_dev) \
> container_of(_dev, struct thermal_cooling_device, device)
>
> @@ -447,6 +512,9 @@ struct thermal_cooling_device *thermal_c
> struct thermal_zone_device *pos;
> int result;
>
> + if (!type)
> + return ERR_PTR(-EINVAL);
> +
> if (strlen(type) >= THERMAL_NAME_LENGTH)
> return ERR_PTR(-EINVAL);
>
> @@ -477,11 +545,9 @@ struct thermal_cooling_device *thermal_c
> }
>
> /* sys I/F */
> - if (type) {
> - result = device_create_file(&cdev->device, &dev_attr_cdev_type);
> - if (result)
> - goto unregister;
> - }
> + result = device_create_file(&cdev->device, &dev_attr_cdev_type);
> + if (result)
> + goto unregister;
>
> result = device_create_file(&cdev->device, &dev_attr_max_state);
> if (result)
> @@ -547,8 +613,8 @@ void thermal_cooling_device_unregister(s
> tz->ops->unbind(tz, cdev);
> }
> mutex_unlock(&thermal_list_lock);
> - if (cdev->type[0])
> - device_remove_file(&cdev->device, &dev_attr_cdev_type);
> +
> + device_remove_file(&cdev->device, &dev_attr_cdev_type);
> device_remove_file(&cdev->device, &dev_attr_max_state);
> device_remove_file(&cdev->device, &dev_attr_cur_state);
>
> @@ -580,6 +646,9 @@ struct thermal_zone_device *thermal_zone
> int result;
> int count;
>
> + if (!type)
> + return ERR_PTR(-EINVAL);
> +
> if (strlen(type) >= THERMAL_NAME_LENGTH)
> return ERR_PTR(-EINVAL);
>
> @@ -601,6 +670,13 @@ struct thermal_zone_device *thermal_zone
> kfree(tz);
> return ERR_PTR(result);
> }
> + if (tz->id >= MAX_THERMAL_ZONES) {
> + printk(KERN_ERR PREFIX
> + "Too many thermal zones\n");
> + release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
> + kfree(tz);
> + return ERR_PTR(-EINVAL);
> + }
>
> strcpy(tz->type, type);
> tz->ops = ops;
> @@ -615,13 +691,28 @@ struct thermal_zone_device *thermal_zone
> return ERR_PTR(result);
> }
>
> - /* sys I/F */
> - if (type) {
> - result = device_create_file(&tz->device, &dev_attr_type);
> - if (result)
> - goto unregister;
> + /* hwmon sys I/F */
> + result = device_create_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2].dev_attr);
> + if (result)
> + goto unregister;
> +
> + if (trips > 0) {
> + char buf[40];
> + result = tz->ops->get_trip_type(tz, 0, buf);
> + if (result > 0 && !strcmp(buf, "critical\n")) {
> + result = device_create_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2 + 1].dev_attr);
> + if (result)
> + goto unregister;
> + }
> }
>
> + /* sys I/F */
> + result = device_create_file(&tz->device, &dev_attr_type);
> + if (result)
> + goto unregister;
> +
> result = device_create_file(&tz->device, &dev_attr_temp);
> if (result)
> goto unregister;
> @@ -687,8 +778,17 @@ void thermal_zone_device_unregister(stru
> tz->ops->unbind(tz, cdev);
> mutex_unlock(&thermal_list_lock);
>
> - if (tz->type[0])
> - device_remove_file(&tz->device, &dev_attr_type);
> + device_remove_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2].dev_attr);
> + if (tz->trips > 0) {
> + char buf[40];
> + if (tz->ops->get_trip_type(tz, 0, buf) > 0)
> + if (!strcmp(buf, "critical\n"))
> + device_remove_file(thermal_hwmon,
> + &sensor_attrs[tz->id * 2 + 1].dev_attr);
> + }
> +
> + device_remove_file(&tz->device, &dev_attr_type);
> device_remove_file(&tz->device, &dev_attr_temp);
> if (tz->ops->get_mode)
> device_remove_file(&tz->device, &dev_attr_mode);
> @@ -705,6 +805,19 @@ void thermal_zone_device_unregister(stru
>
> EXPORT_SYMBOL(thermal_zone_device_unregister);
>
> +static void __exit thermal_exit(void)
> +{
> + if (thermal_hwmon) {
> + device_remove_file(thermal_hwmon, &dev_attr_name);
> + hwmon_device_unregister(thermal_hwmon);
> + }
> + class_unregister(&thermal_class);
> + idr_destroy(&thermal_tz_idr);
> + idr_destroy(&thermal_cdev_idr);
> + mutex_destroy(&thermal_idr_lock);
> + mutex_destroy(&thermal_list_lock);
> +}
> +
> static int __init thermal_init(void)
> {
> int result = 0;
> @@ -716,16 +829,20 @@ static int __init thermal_init(void)
> mutex_destroy(&thermal_idr_lock);
> mutex_destroy(&thermal_list_lock);
> }
> - return result;
> -}
>
> -static void __exit thermal_exit(void)
> -{
> - class_unregister(&thermal_class);
> - idr_destroy(&thermal_tz_idr);
> - idr_destroy(&thermal_cdev_idr);
> - mutex_destroy(&thermal_idr_lock);
> - mutex_destroy(&thermal_list_lock);
> + thermal_hwmon = hwmon_device_register(NULL);
> + if (IS_ERR(thermal_hwmon)) {
> + result = PTR_ERR(thermal_hwmon);
> + thermal_hwmon = NULL;
> + printk(KERN_ERR PREFIX
> + "unable to register hwmon device\n");
> + thermal_exit();
> + return result;
> + }
> +
> + result = device_create_file(thermal_hwmon, &dev_attr_name);
> +
> + return result;
> }
>
> subsys_initcall(thermal_init);
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-12 4:29 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Len Brown
@ 2008-03-13 5:09 ` Len Brown
-1 siblings, 0 replies; 50+ messages in thread
From: Len Brown @ 2008-03-13 5:09 UTC (permalink / raw)
To: Zhang, Rui; +Cc: linux-acpi, lm-sensors, Hans de Goede
> On Tuesday 26 February 2008, Zhang, Rui wrote:
> >
> > Add hwmon sys I/F for the generic thermal device.
> >
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
> > ---
> > Documentation/thermal/sysfs-api.txt | 22 ++--
> > drivers/thermal/Kconfig | 1
> > drivers/thermal/thermal.c | 169 ++++++++++++++++++++++++++++++------
> > 3 files changed, 155 insertions(+), 37 deletions(-)
> >
...
> > {
> > int result = 0;
> > @@ -716,16 +829,20 @@ static int __init thermal_init(void)
> > mutex_destroy(&thermal_idr_lock);
> > mutex_destroy(&thermal_list_lock);
> > }
> > - return result;
> > -}
> >
> > -static void __exit thermal_exit(void)
> > -{
> > - class_unregister(&thermal_class);
> > - idr_destroy(&thermal_tz_idr);
> > - idr_destroy(&thermal_cdev_idr);
> > - mutex_destroy(&thermal_idr_lock);
> > - mutex_destroy(&thermal_list_lock);
> > + thermal_hwmon = hwmon_device_register(NULL);
> > + if (IS_ERR(thermal_hwmon)) {
> > + result = PTR_ERR(thermal_hwmon);
> > + thermal_hwmon = NULL;
> > + printk(KERN_ERR PREFIX
> > + "unable to register hwmon device\n");
> > + thermal_exit();
An __exit routine can not be called from an __init routine.
this doesn't build on ia64, which discards .exit sections:
.exit.text' referenced in section `.init.text' of drivers/built-in.o: defined in discarded section `.exit.text' of drivers/built-in.o
-Len
> > + return result;
> > + }
> > +
> > + result = device_create_file(thermal_hwmon, &dev_attr_name);
> > +
> > + return result;
> > }
> >
> > subsys_initcall(thermal_init);
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-13 5:09 ` Len Brown
0 siblings, 0 replies; 50+ messages in thread
From: Len Brown @ 2008-03-13 5:09 UTC (permalink / raw)
To: Zhang, Rui; +Cc: linux-acpi, lm-sensors, Hans de Goede
> On Tuesday 26 February 2008, Zhang, Rui wrote:
> >
> > Add hwmon sys I/F for the generic thermal device.
> >
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
> > ---
> > Documentation/thermal/sysfs-api.txt | 22 ++--
> > drivers/thermal/Kconfig | 1
> > drivers/thermal/thermal.c | 169 ++++++++++++++++++++++++++++++------
> > 3 files changed, 155 insertions(+), 37 deletions(-)
> >
...
> > {
> > int result = 0;
> > @@ -716,16 +829,20 @@ static int __init thermal_init(void)
> > mutex_destroy(&thermal_idr_lock);
> > mutex_destroy(&thermal_list_lock);
> > }
> > - return result;
> > -}
> >
> > -static void __exit thermal_exit(void)
> > -{
> > - class_unregister(&thermal_class);
> > - idr_destroy(&thermal_tz_idr);
> > - idr_destroy(&thermal_cdev_idr);
> > - mutex_destroy(&thermal_idr_lock);
> > - mutex_destroy(&thermal_list_lock);
> > + thermal_hwmon = hwmon_device_register(NULL);
> > + if (IS_ERR(thermal_hwmon)) {
> > + result = PTR_ERR(thermal_hwmon);
> > + thermal_hwmon = NULL;
> > + printk(KERN_ERR PREFIX
> > + "unable to register hwmon device\n");
> > + thermal_exit();
An __exit routine can not be called from an __init routine.
this doesn't build on ia64, which discards .exit sections:
.exit.text' referenced in section `.init.text' of drivers/built-in.o: defined in discarded section `.exit.text' of drivers/built-in.o
-Len
> > + return result;
> > + }
> > +
> > + result = device_create_file(thermal_hwmon, &dev_attr_name);
> > +
> > + return result;
> > }
> >
> > subsys_initcall(thermal_init);
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-13 5:09 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Len Brown
@ 2008-03-13 8:46 ` Zhang, Rui
-1 siblings, 0 replies; 50+ messages in thread
From: Zhang, Rui @ 2008-03-13 8:46 UTC (permalink / raw)
To: Len Brown; +Cc: linux-acpi, lm-sensors, Hans de Goede
On Thu, 2008-03-13 at 13:09 +0800, Len Brown wrote:
>
> > On Tuesday 26 February 2008, Zhang, Rui wrote:
> > >
> > > Add hwmon sys I/F for the generic thermal device.
> > >
> > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
> > > ---
> > > Documentation/thermal/sysfs-api.txt | 22 ++--
> > > drivers/thermal/Kconfig | 1
> > > drivers/thermal/thermal.c | 169
> ++++++++++++++++++++++++++++++------
> > > 3 files changed, 155 insertions(+), 37 deletions(-)
> > >
> ...
> > > {
> > > int result = 0;
> > > @@ -716,16 +829,20 @@ static int __init thermal_init(void)
> > > mutex_destroy(&thermal_idr_lock);
> > > mutex_destroy(&thermal_list_lock);
> > > }
> > > - return result;
> > > -}
> > >
> > > -static void __exit thermal_exit(void)
> > > -{
> > > - class_unregister(&thermal_class);
> > > - idr_destroy(&thermal_tz_idr);
> > > - idr_destroy(&thermal_cdev_idr);
> > > - mutex_destroy(&thermal_idr_lock);
> > > - mutex_destroy(&thermal_list_lock);
> > > + thermal_hwmon = hwmon_device_register(NULL);
> > > + if (IS_ERR(thermal_hwmon)) {
> > > + result = PTR_ERR(thermal_hwmon);
> > > + thermal_hwmon = NULL;
> > > + printk(KERN_ERR PREFIX
> > > + "unable to register hwmon device\n");
> > > + thermal_exit();
>
> An __exit routine can not be called from an __init routine.
> this doesn't build on ia64, which discards .exit sections:
>
> .exit.text' referenced in section `.init.text' of drivers/built-in.o:
> defined in discarded section `.exit.text' of drivers/built-in.o
>
> -Len
As I don't have an ia64 machine,
len, could you please help me test this incremental patch? :)
From: Zhang Rui <rui.zhang@intel.com>
An __exit routine can not be called from an __init routine.
this doesn't build on ia64, which discards .exit sections:
".exit.text' referenced in section `.init.text' of drivers/built-in.o:
defined in discarded section `.exit.text' of drivers/built-in.o"
http://marc.info/?l=linux-acpi&m=120538509025142&w=2
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/thermal/thermal.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
Index: linux-2.6/drivers/thermal/thermal.c
===================================================================
--- linux-2.6.orig/drivers/thermal/thermal.c
+++ linux-2.6/drivers/thermal/thermal.c
@@ -823,12 +823,8 @@ static int __init thermal_init(void)
int result = 0;
result = class_register(&thermal_class);
- if (result) {
- idr_destroy(&thermal_tz_idr);
- idr_destroy(&thermal_cdev_idr);
- mutex_destroy(&thermal_idr_lock);
- mutex_destroy(&thermal_list_lock);
- }
+ if (result)
+ goto err;
thermal_hwmon = hwmon_device_register(NULL);
if (IS_ERR(thermal_hwmon)) {
@@ -836,13 +832,20 @@ static int __init thermal_init(void)
thermal_hwmon = NULL;
printk(KERN_ERR PREFIX
"unable to register hwmon device\n");
- thermal_exit();
- return result;
+ class_unregister(&thermal_class);
+ goto err;
}
result = device_create_file(thermal_hwmon, &dev_attr_name);
return result;
+ err:
+ idr_destroy(&thermal_tz_idr);
+ idr_destroy(&thermal_cdev_idr);
+ mutex_destroy(&thermal_idr_lock);
+ mutex_destroy(&thermal_list_lock);
+
+ return result;
}
subsys_initcall(thermal_init);
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-13 8:46 ` Zhang, Rui
0 siblings, 0 replies; 50+ messages in thread
From: Zhang, Rui @ 2008-03-13 8:46 UTC (permalink / raw)
To: Len Brown; +Cc: linux-acpi, lm-sensors, Hans de Goede
On Thu, 2008-03-13 at 13:09 +0800, Len Brown wrote:
>
> > On Tuesday 26 February 2008, Zhang, Rui wrote:
> > >
> > > Add hwmon sys I/F for the generic thermal device.
> > >
> > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
> > > ---
> > > Documentation/thermal/sysfs-api.txt | 22 ++--
> > > drivers/thermal/Kconfig | 1
> > > drivers/thermal/thermal.c | 169
> ++++++++++++++++++++++++++++++------
> > > 3 files changed, 155 insertions(+), 37 deletions(-)
> > >
> ...
> > > {
> > > int result = 0;
> > > @@ -716,16 +829,20 @@ static int __init thermal_init(void)
> > > mutex_destroy(&thermal_idr_lock);
> > > mutex_destroy(&thermal_list_lock);
> > > }
> > > - return result;
> > > -}
> > >
> > > -static void __exit thermal_exit(void)
> > > -{
> > > - class_unregister(&thermal_class);
> > > - idr_destroy(&thermal_tz_idr);
> > > - idr_destroy(&thermal_cdev_idr);
> > > - mutex_destroy(&thermal_idr_lock);
> > > - mutex_destroy(&thermal_list_lock);
> > > + thermal_hwmon = hwmon_device_register(NULL);
> > > + if (IS_ERR(thermal_hwmon)) {
> > > + result = PTR_ERR(thermal_hwmon);
> > > + thermal_hwmon = NULL;
> > > + printk(KERN_ERR PREFIX
> > > + "unable to register hwmon device\n");
> > > + thermal_exit();
>
> An __exit routine can not be called from an __init routine.
> this doesn't build on ia64, which discards .exit sections:
>
> .exit.text' referenced in section `.init.text' of drivers/built-in.o:
> defined in discarded section `.exit.text' of drivers/built-in.o
>
> -Len
As I don't have an ia64 machine,
len, could you please help me test this incremental patch? :)
From: Zhang Rui <rui.zhang@intel.com>
An __exit routine can not be called from an __init routine.
this doesn't build on ia64, which discards .exit sections:
".exit.text' referenced in section `.init.text' of drivers/built-in.o:
defined in discarded section `.exit.text' of drivers/built-in.o"
http://marc.info/?l=linux-acpi&m\x120538509025142&w=2
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/thermal/thermal.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
Index: linux-2.6/drivers/thermal/thermal.c
=================================--- linux-2.6.orig/drivers/thermal/thermal.c
+++ linux-2.6/drivers/thermal/thermal.c
@@ -823,12 +823,8 @@ static int __init thermal_init(void)
int result = 0;
result = class_register(&thermal_class);
- if (result) {
- idr_destroy(&thermal_tz_idr);
- idr_destroy(&thermal_cdev_idr);
- mutex_destroy(&thermal_idr_lock);
- mutex_destroy(&thermal_list_lock);
- }
+ if (result)
+ goto err;
thermal_hwmon = hwmon_device_register(NULL);
if (IS_ERR(thermal_hwmon)) {
@@ -836,13 +832,20 @@ static int __init thermal_init(void)
thermal_hwmon = NULL;
printk(KERN_ERR PREFIX
"unable to register hwmon device\n");
- thermal_exit();
- return result;
+ class_unregister(&thermal_class);
+ goto err;
}
result = device_create_file(thermal_hwmon, &dev_attr_name);
return result;
+ err:
+ idr_destroy(&thermal_tz_idr);
+ idr_destroy(&thermal_cdev_idr);
+ mutex_destroy(&thermal_idr_lock);
+ mutex_destroy(&thermal_list_lock);
+
+ return result;
}
subsys_initcall(thermal_init);
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-13 5:09 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Len Brown
@ 2008-03-13 10:59 ` Thomas Renninger
-1 siblings, 0 replies; 50+ messages in thread
From: Thomas Renninger @ 2008-03-13 10:59 UTC (permalink / raw)
To: Len Brown
Cc: Zhang, Rui, linux-acpi, lm-sensors, Hans de Goede,
Henrique de Moraes Holschuh
On Thu, 2008-03-13 at 01:09 -0400, Len Brown wrote:
> > On Tuesday 26 February 2008, Zhang, Rui wrote:
> > >
> > > Add hwmon sys I/F for the generic thermal device.
> > >
> > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
> > > ---
> > > Documentation/thermal/sysfs-api.txt | 22 ++--
> > > drivers/thermal/Kconfig | 1
> > > drivers/thermal/thermal.c | 169 ++++++++++++++++++++++++++++++------
> > > 3 files changed, 155 insertions(+), 37 deletions(-)
> > >
> ...
> > > {
> > > int result = 0;
> > > @@ -716,16 +829,20 @@ static int __init thermal_init(void)
> > > mutex_destroy(&thermal_idr_lock);
> > > mutex_destroy(&thermal_list_lock);
> > > }
> > > - return result;
> > > -}
> > >
> > > -static void __exit thermal_exit(void)
> > > -{
> > > - class_unregister(&thermal_class);
> > > - idr_destroy(&thermal_tz_idr);
> > > - idr_destroy(&thermal_cdev_idr);
> > > - mutex_destroy(&thermal_idr_lock);
> > > - mutex_destroy(&thermal_list_lock);
> > > + thermal_hwmon = hwmon_device_register(NULL);
> > > + if (IS_ERR(thermal_hwmon)) {
> > > + result = PTR_ERR(thermal_hwmon);
> > > + thermal_hwmon = NULL;
> > > + printk(KERN_ERR PREFIX
> > > + "unable to register hwmon device\n");
> > > + thermal_exit();
>
> An __exit routine can not be called from an __init routine.
> this doesn't build on ia64, which discards .exit sections:
>
> .exit.text' referenced in section `.init.text' of drivers/built-in.o: defined in discarded section `.exit.text' of drivers/built-in.o
This also happens in drivers/misc/thinkpad_acpi.c.
While this is not used on IA64, it should also be cleaned up on
long-term.
Thomas
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-13 10:59 ` Thomas Renninger
0 siblings, 0 replies; 50+ messages in thread
From: Thomas Renninger @ 2008-03-13 10:59 UTC (permalink / raw)
To: Len Brown
Cc: Zhang, Rui, linux-acpi, lm-sensors, Hans de Goede,
Henrique de Moraes Holschuh
On Thu, 2008-03-13 at 01:09 -0400, Len Brown wrote:
> > On Tuesday 26 February 2008, Zhang, Rui wrote:
> > >
> > > Add hwmon sys I/F for the generic thermal device.
> > >
> > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
> > > ---
> > > Documentation/thermal/sysfs-api.txt | 22 ++--
> > > drivers/thermal/Kconfig | 1
> > > drivers/thermal/thermal.c | 169 ++++++++++++++++++++++++++++++------
> > > 3 files changed, 155 insertions(+), 37 deletions(-)
> > >
> ...
> > > {
> > > int result = 0;
> > > @@ -716,16 +829,20 @@ static int __init thermal_init(void)
> > > mutex_destroy(&thermal_idr_lock);
> > > mutex_destroy(&thermal_list_lock);
> > > }
> > > - return result;
> > > -}
> > >
> > > -static void __exit thermal_exit(void)
> > > -{
> > > - class_unregister(&thermal_class);
> > > - idr_destroy(&thermal_tz_idr);
> > > - idr_destroy(&thermal_cdev_idr);
> > > - mutex_destroy(&thermal_idr_lock);
> > > - mutex_destroy(&thermal_list_lock);
> > > + thermal_hwmon = hwmon_device_register(NULL);
> > > + if (IS_ERR(thermal_hwmon)) {
> > > + result = PTR_ERR(thermal_hwmon);
> > > + thermal_hwmon = NULL;
> > > + printk(KERN_ERR PREFIX
> > > + "unable to register hwmon device\n");
> > > + thermal_exit();
>
> An __exit routine can not be called from an __init routine.
> this doesn't build on ia64, which discards .exit sections:
>
> .exit.text' referenced in section `.init.text' of drivers/built-in.o: defined in discarded section `.exit.text' of drivers/built-in.o
This also happens in drivers/misc/thinkpad_acpi.c.
While this is not used on IA64, it should also be cleaned up on
long-term.
Thomas
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-13 10:59 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Thomas Renninger
@ 2008-03-13 23:09 ` Henrique de Moraes Holschuh
-1 siblings, 0 replies; 50+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-03-13 23:09 UTC (permalink / raw)
To: Thomas Renninger
Cc: Len Brown, Zhang, Rui, linux-acpi, lm-sensors, Hans de Goede
On Thu, 13 Mar 2008, Thomas Renninger wrote:
> This also happens in drivers/misc/thinkpad_acpi.c.
I was sure I had removed __exit from every thinkpad-acpi exit handler that
was also called from __init a long time ago...
Let's see... nope, there isn't a single instance of __exit in
thinkpad-acpi.c, so I must have misunderstood you. I do have lots of
__init code, but I don't get any warnings from gcc 4.3 about that on ia32.
Can you please send me the warnings/errors you get in thinkpad-acpi?
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-13 23:09 ` Henrique de Moraes Holschuh
0 siblings, 0 replies; 50+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-03-13 23:09 UTC (permalink / raw)
To: Thomas Renninger
Cc: Len Brown, Zhang, Rui, linux-acpi, lm-sensors, Hans de Goede
On Thu, 13 Mar 2008, Thomas Renninger wrote:
> This also happens in drivers/misc/thinkpad_acpi.c.
I was sure I had removed __exit from every thinkpad-acpi exit handler that
was also called from __init a long time ago...
Let's see... nope, there isn't a single instance of __exit in
thinkpad-acpi.c, so I must have misunderstood you. I do have lots of
__init code, but I don't get any warnings from gcc 4.3 about that on ia32.
Can you please send me the warnings/errors you get in thinkpad-acpi?
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-13 23:09 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Henrique de Moraes Holschuh
@ 2008-03-14 9:03 ` Thomas Renninger
-1 siblings, 0 replies; 50+ messages in thread
From: Thomas Renninger @ 2008-03-14 9:03 UTC (permalink / raw)
To: Henrique de Moraes Holschuh
Cc: Len Brown, Zhang, Rui, linux-acpi, lm-sensors, Hans de Goede
On Thu, 2008-03-13 at 20:09 -0300, Henrique de Moraes Holschuh wrote:
> On Thu, 13 Mar 2008, Thomas Renninger wrote:
> > This also happens in drivers/misc/thinkpad_acpi.c.
>
> I was sure I had removed __exit from every thinkpad-acpi exit handler that
> was also called from __init a long time ago...
>
> Let's see... nope, there isn't a single instance of __exit in
> thinkpad-acpi.c, so I must have misunderstood you. I do have lots of
> __init code, but I don't get any warnings from gcc 4.3 about that on ia32.
Ahh, I just checked that the module_init function calls the module_exit
function...
But the module_exit function probably because of this is not declared
__exit.
This looks a bit uncommon. I don't know whether it should be avoided,
but I cannot see why it should hurt.
Thomas
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-14 9:03 ` Thomas Renninger
0 siblings, 0 replies; 50+ messages in thread
From: Thomas Renninger @ 2008-03-14 9:03 UTC (permalink / raw)
To: Henrique de Moraes Holschuh
Cc: Len Brown, Zhang, Rui, linux-acpi, lm-sensors, Hans de Goede
On Thu, 2008-03-13 at 20:09 -0300, Henrique de Moraes Holschuh wrote:
> On Thu, 13 Mar 2008, Thomas Renninger wrote:
> > This also happens in drivers/misc/thinkpad_acpi.c.
>
> I was sure I had removed __exit from every thinkpad-acpi exit handler that
> was also called from __init a long time ago...
>
> Let's see... nope, there isn't a single instance of __exit in
> thinkpad-acpi.c, so I must have misunderstood you. I do have lots of
> __init code, but I don't get any warnings from gcc 4.3 about that on ia32.
Ahh, I just checked that the module_init function calls the module_exit
function...
But the module_exit function probably because of this is not declared
__exit.
This looks a bit uncommon. I don't know whether it should be avoided,
but I cannot see why it should hurt.
Thomas
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-14 9:03 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Thomas Renninger
@ 2008-03-15 4:25 ` Henrique de Moraes Holschuh
-1 siblings, 0 replies; 50+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-03-15 4:25 UTC (permalink / raw)
To: Thomas Renninger
Cc: linux-acpi, Zhang, Rui, lm-sensors, Hans de Goede, Len Brown
On Fri, 14 Mar 2008, Thomas Renninger wrote:
> Ahh, I just checked that the module_init function calls the module_exit
> function...
> But the module_exit function probably because of this is not declared
> __exit.
>
> This looks a bit uncommon. I don't know whether it should be avoided,
> but I cannot see why it should hurt.
Well, I didn't see a reason to duplicate code just to save a few hundred
bytes (if that much!) of text for those who don't use a modular
thinkpad-acpi...
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-15 4:25 ` Henrique de Moraes Holschuh
0 siblings, 0 replies; 50+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-03-15 4:25 UTC (permalink / raw)
To: Thomas Renninger
Cc: linux-acpi, Zhang, Rui, lm-sensors, Hans de Goede, Len Brown
On Fri, 14 Mar 2008, Thomas Renninger wrote:
> Ahh, I just checked that the module_init function calls the module_exit
> function...
> But the module_exit function probably because of this is not declared
> __exit.
>
> This looks a bit uncommon. I don't know whether it should be avoided,
> but I cannot see why it should hurt.
Well, I didn't see a reason to duplicate code just to save a few hundred
bytes (if that much!) of text for those who don't use a modular
thinkpad-acpi...
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-02-26 8:39 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Hans de Goede
@ 2008-03-17 12:37 ` Jean Delvare
-1 siblings, 0 replies; 50+ messages in thread
From: Jean Delvare @ 2008-03-17 12:37 UTC (permalink / raw)
To: Hans de Goede
Cc: Zhang, Rui, linux-acpi, lm-sensors, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
Hi Hans,
On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
> Zhang, Rui wrote:
> > Add hwmon sys I/F for the generic thermal device.
> >
>
> Great!
>
> But I have several remarks:
> 1) Looking at the new code, you only add temp1_input, so I'm guessing that you
> are registering a seperate hwmon class entry per zone? Please don't do that,
> please register one hwmon class entry, and add multiple temp#_input attr to it
> (and the same for crit).
I am sorry that I did not notice when you suggested this. I disagree,
but now Rui's code is upstream so I guess it's too late to complain.
Still here are my reasons:
One of the great things about libsensors is that it gives unique names
to hardware monitoring devices, and for each device, each feature has a
unique name as well. This makes it possible to ignore or label a
specific feature in /etc/sensors.conf in a way that is stable over
reboot and addition of new hardware.
By going with a single virtual device for all thermal zones, you break
this model. Depending on which thermal zone drivers are loaded and in
which order they are loaded, there will be more or less temp* files in
the hwmon directory and you also can't predict their order. The
labelling issue could be solved by adding temp*_label files, but this
still prevents the user from overriding a label. And there's no way to
reliably ignore a specific thermal zone or to ask for information about
a specific thermal zone with the current model.
For this reason, I think it would be much better to have one hwmon
class device for each _type_ of thermal zone. For example, all ACPI
thermal zones would be listed as one hwmon class device. If we later
add support for another type of thermal zones, all thermal zones of
this type would be listed as one (different) hwmon class device. This
makes each specific thermal zone driver responsible for the stability
of the numbering of the various thermal zones of a given type. This
would also let us give names to the different thermal zone types (e.g.
"acpitz" for ACPI thermal zones) so that the users and supporters have
an idea who is providing these temperature values and limits.
--
Jean Delvare
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-17 12:37 ` Jean Delvare
0 siblings, 0 replies; 50+ messages in thread
From: Jean Delvare @ 2008-03-17 12:37 UTC (permalink / raw)
To: Hans de Goede
Cc: Zhang, Rui, linux-acpi, lm-sensors, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
Hi Hans,
On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
> Zhang, Rui wrote:
> > Add hwmon sys I/F for the generic thermal device.
> >
>
> Great!
>
> But I have several remarks:
> 1) Looking at the new code, you only add temp1_input, so I'm guessing that you
> are registering a seperate hwmon class entry per zone? Please don't do that,
> please register one hwmon class entry, and add multiple temp#_input attr to it
> (and the same for crit).
I am sorry that I did not notice when you suggested this. I disagree,
but now Rui's code is upstream so I guess it's too late to complain.
Still here are my reasons:
One of the great things about libsensors is that it gives unique names
to hardware monitoring devices, and for each device, each feature has a
unique name as well. This makes it possible to ignore or label a
specific feature in /etc/sensors.conf in a way that is stable over
reboot and addition of new hardware.
By going with a single virtual device for all thermal zones, you break
this model. Depending on which thermal zone drivers are loaded and in
which order they are loaded, there will be more or less temp* files in
the hwmon directory and you also can't predict their order. The
labelling issue could be solved by adding temp*_label files, but this
still prevents the user from overriding a label. And there's no way to
reliably ignore a specific thermal zone or to ask for information about
a specific thermal zone with the current model.
For this reason, I think it would be much better to have one hwmon
class device for each _type_ of thermal zone. For example, all ACPI
thermal zones would be listed as one hwmon class device. If we later
add support for another type of thermal zones, all thermal zones of
this type would be listed as one (different) hwmon class device. This
makes each specific thermal zone driver responsible for the stability
of the numbering of the various thermal zones of a given type. This
would also let us give names to the different thermal zone types (e.g.
"acpitz" for ACPI thermal zones) so that the users and supporters have
an idea who is providing these temperature values and limits.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-17 12:37 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Jean Delvare
@ 2008-03-17 12:55 ` Hans de Goede
-1 siblings, 0 replies; 50+ messages in thread
From: Hans de Goede @ 2008-03-17 12:55 UTC (permalink / raw)
To: Jean Delvare
Cc: Zhang, Rui, linux-acpi, lm-sensors, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
Jean Delvare wrote:
> Hi Hans,
>
> On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
>> Zhang, Rui wrote:
>>> Add hwmon sys I/F for the generic thermal device.
>>>
>> Great!
>>
>> But I have several remarks:
>> 1) Looking at the new code, you only add temp1_input, so I'm guessing that you
>> are registering a seperate hwmon class entry per zone? Please don't do that,
>> please register one hwmon class entry, and add multiple temp#_input attr to it
>> (and the same for crit).
>
> I am sorry that I did not notice when you suggested this. I disagree,
> but now Rui's code is upstream so I guess it's too late to complain.
> Still here are my reasons:
>
> One of the great things about libsensors is that it gives unique names
> to hardware monitoring devices, and for each device, each feature has a
> unique name as well. This makes it possible to ignore or label a
> specific feature in /etc/sensors.conf in a way that is stable over
> reboot and addition of new hardware.
>
> By going with a single virtual device for all thermal zones, you break
> this model. Depending on which thermal zone drivers are loaded and in
> which order they are loaded, there will be more or less temp* files in
> the hwmon directory and you also can't predict their order. The
> labelling issue could be solved by adding temp*_label files, but this
> still prevents the user from overriding a label. And there's no way to
> reliably ignore a specific thermal zone or to ask for information about
> a specific thermal zone with the current model.
>
> For this reason, I think it would be much better to have one hwmon
> class device for each _type_ of thermal zone. For example, all ACPI
> thermal zones would be listed as one hwmon class device. If we later
> add support for another type of thermal zones, all thermal zones of
> this type would be listed as one (different) hwmon class device. This
> makes each specific thermal zone driver responsible for the stability
> of the numbering of the various thermal zones of a given type. This
> would also let us give names to the different thermal zone types (e.g.
> "acpitz" for ACPI thermal zones) so that the users and supporters have
> an idea who is providing these temperature values and limits.
>
I fully agree, I didn't know there was a generic thermal zone model, and that
there could be multiple drivers implementing it (let alone multiple thermal
zone drivers active for one system ??) I thought this was all ACPI only,
If this (multiple thermal zone drivers active for one system) can really happen
then we really should fix it so that there is one hwmon class entry per thermal
zone driver. This can be done without changing the ABI, as things would still
follow the standard hwmon ABI.
Regards,
Hans
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-17 12:55 ` Hans de Goede
0 siblings, 0 replies; 50+ messages in thread
From: Hans de Goede @ 2008-03-17 12:55 UTC (permalink / raw)
To: Jean Delvare
Cc: Zhang, Rui, linux-acpi, lm-sensors, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
Jean Delvare wrote:
> Hi Hans,
>
> On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
>> Zhang, Rui wrote:
>>> Add hwmon sys I/F for the generic thermal device.
>>>
>> Great!
>>
>> But I have several remarks:
>> 1) Looking at the new code, you only add temp1_input, so I'm guessing that you
>> are registering a seperate hwmon class entry per zone? Please don't do that,
>> please register one hwmon class entry, and add multiple temp#_input attr to it
>> (and the same for crit).
>
> I am sorry that I did not notice when you suggested this. I disagree,
> but now Rui's code is upstream so I guess it's too late to complain.
> Still here are my reasons:
>
> One of the great things about libsensors is that it gives unique names
> to hardware monitoring devices, and for each device, each feature has a
> unique name as well. This makes it possible to ignore or label a
> specific feature in /etc/sensors.conf in a way that is stable over
> reboot and addition of new hardware.
>
> By going with a single virtual device for all thermal zones, you break
> this model. Depending on which thermal zone drivers are loaded and in
> which order they are loaded, there will be more or less temp* files in
> the hwmon directory and you also can't predict their order. The
> labelling issue could be solved by adding temp*_label files, but this
> still prevents the user from overriding a label. And there's no way to
> reliably ignore a specific thermal zone or to ask for information about
> a specific thermal zone with the current model.
>
> For this reason, I think it would be much better to have one hwmon
> class device for each _type_ of thermal zone. For example, all ACPI
> thermal zones would be listed as one hwmon class device. If we later
> add support for another type of thermal zones, all thermal zones of
> this type would be listed as one (different) hwmon class device. This
> makes each specific thermal zone driver responsible for the stability
> of the numbering of the various thermal zones of a given type. This
> would also let us give names to the different thermal zone types (e.g.
> "acpitz" for ACPI thermal zones) so that the users and supporters have
> an idea who is providing these temperature values and limits.
>
I fully agree, I didn't know there was a generic thermal zone model, and that
there could be multiple drivers implementing it (let alone multiple thermal
zone drivers active for one system ??) I thought this was all ACPI only,
If this (multiple thermal zone drivers active for one system) can really happen
then we really should fix it so that there is one hwmon class entry per thermal
zone driver. This can be done without changing the ABI, as things would still
follow the standard hwmon ABI.
Regards,
Hans
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-17 12:55 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Hans de Goede
@ 2008-03-17 13:48 ` Jean Delvare
-1 siblings, 0 replies; 50+ messages in thread
From: Jean Delvare @ 2008-03-17 13:48 UTC (permalink / raw)
To: Hans de Goede, Zhang, Rui, Len Brown
Cc: linux-acpi, lm-sensors, Matthew Garrett, Thomas Renninger,
Thomas, Sujith, Mark M. Hoffman, Henrique de Moraes Holschuh,
Richard Hughes
On Mon, 17 Mar 2008 13:55:00 +0100, Hans de Goede wrote:
> Jean Delvare wrote:
> > Hi Hans,
> >
> > On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
> >> Zhang, Rui wrote:
> >>> Add hwmon sys I/F for the generic thermal device.
> >>>
> >> Great!
> >>
> >> But I have several remarks:
> >> 1) Looking at the new code, you only add temp1_input, so I'm guessing that you
> >> are registering a seperate hwmon class entry per zone? Please don't do that,
> >> please register one hwmon class entry, and add multiple temp#_input attr to it
> >> (and the same for crit).
> >
> > I am sorry that I did not notice when you suggested this. I disagree,
> > but now Rui's code is upstream so I guess it's too late to complain.
> > Still here are my reasons:
> >
> > One of the great things about libsensors is that it gives unique names
> > to hardware monitoring devices, and for each device, each feature has a
> > unique name as well. This makes it possible to ignore or label a
> > specific feature in /etc/sensors.conf in a way that is stable over
> > reboot and addition of new hardware.
> >
> > By going with a single virtual device for all thermal zones, you break
> > this model. Depending on which thermal zone drivers are loaded and in
> > which order they are loaded, there will be more or less temp* files in
> > the hwmon directory and you also can't predict their order. The
> > labelling issue could be solved by adding temp*_label files, but this
> > still prevents the user from overriding a label. And there's no way to
> > reliably ignore a specific thermal zone or to ask for information about
> > a specific thermal zone with the current model.
> >
> > For this reason, I think it would be much better to have one hwmon
> > class device for each _type_ of thermal zone. For example, all ACPI
> > thermal zones would be listed as one hwmon class device. If we later
> > add support for another type of thermal zones, all thermal zones of
> > this type would be listed as one (different) hwmon class device. This
> > makes each specific thermal zone driver responsible for the stability
> > of the numbering of the various thermal zones of a given type. This
> > would also let us give names to the different thermal zone types (e.g.
> > "acpitz" for ACPI thermal zones) so that the users and supporters have
> > an idea who is providing these temperature values and limits.
> >
>
> I fully agree, I didn't know there was a generic thermal zone model, and that
> there could be multiple drivers implementing it (let alone multiple thermal
> zone drivers active for one system ??) I thought this was all ACPI only,
My understanding is that the new thermal zone code is meant to be
generic and ACPI is only one possible underlying implementations, which
sounds very good to me. ACPI just happens to be the only implementation
at the time being.
> If this (multiple thermal zone drivers active for one system) can really happen
> then we really should fix it so that there is one hwmon class entry per thermal
> zone driver. This can be done without changing the ABI, as things would still
> follow the standard hwmon ABI.
I have to admit that I'm not sure what sense that would make (and how
safe it would be) to have more than one type of thermal zones active at
the same time. If the idea if that only one type of thermal zones can
exist for a given system (i.e. the selection happens at build time),
then my objection can safely be ignored, except for the fact that I
still would like the "name" attribute to reflect the type of the
thermal zones.
Rui, Len, how did you originally envision the coexistence (or not) of
different types of thermal zones?
--
Jean Delvare
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-17 13:48 ` Jean Delvare
0 siblings, 0 replies; 50+ messages in thread
From: Jean Delvare @ 2008-03-17 13:48 UTC (permalink / raw)
To: Hans de Goede, Zhang, Rui, Len Brown
Cc: linux-acpi, lm-sensors, Matthew Garrett, Thomas Renninger,
Thomas, Sujith, Mark M. Hoffman, Henrique de Moraes Holschuh,
Richard Hughes
On Mon, 17 Mar 2008 13:55:00 +0100, Hans de Goede wrote:
> Jean Delvare wrote:
> > Hi Hans,
> >
> > On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
> >> Zhang, Rui wrote:
> >>> Add hwmon sys I/F for the generic thermal device.
> >>>
> >> Great!
> >>
> >> But I have several remarks:
> >> 1) Looking at the new code, you only add temp1_input, so I'm guessing that you
> >> are registering a seperate hwmon class entry per zone? Please don't do that,
> >> please register one hwmon class entry, and add multiple temp#_input attr to it
> >> (and the same for crit).
> >
> > I am sorry that I did not notice when you suggested this. I disagree,
> > but now Rui's code is upstream so I guess it's too late to complain.
> > Still here are my reasons:
> >
> > One of the great things about libsensors is that it gives unique names
> > to hardware monitoring devices, and for each device, each feature has a
> > unique name as well. This makes it possible to ignore or label a
> > specific feature in /etc/sensors.conf in a way that is stable over
> > reboot and addition of new hardware.
> >
> > By going with a single virtual device for all thermal zones, you break
> > this model. Depending on which thermal zone drivers are loaded and in
> > which order they are loaded, there will be more or less temp* files in
> > the hwmon directory and you also can't predict their order. The
> > labelling issue could be solved by adding temp*_label files, but this
> > still prevents the user from overriding a label. And there's no way to
> > reliably ignore a specific thermal zone or to ask for information about
> > a specific thermal zone with the current model.
> >
> > For this reason, I think it would be much better to have one hwmon
> > class device for each _type_ of thermal zone. For example, all ACPI
> > thermal zones would be listed as one hwmon class device. If we later
> > add support for another type of thermal zones, all thermal zones of
> > this type would be listed as one (different) hwmon class device. This
> > makes each specific thermal zone driver responsible for the stability
> > of the numbering of the various thermal zones of a given type. This
> > would also let us give names to the different thermal zone types (e.g.
> > "acpitz" for ACPI thermal zones) so that the users and supporters have
> > an idea who is providing these temperature values and limits.
> >
>
> I fully agree, I didn't know there was a generic thermal zone model, and that
> there could be multiple drivers implementing it (let alone multiple thermal
> zone drivers active for one system ??) I thought this was all ACPI only,
My understanding is that the new thermal zone code is meant to be
generic and ACPI is only one possible underlying implementations, which
sounds very good to me. ACPI just happens to be the only implementation
at the time being.
> If this (multiple thermal zone drivers active for one system) can really happen
> then we really should fix it so that there is one hwmon class entry per thermal
> zone driver. This can be done without changing the ABI, as things would still
> follow the standard hwmon ABI.
I have to admit that I'm not sure what sense that would make (and how
safe it would be) to have more than one type of thermal zones active at
the same time. If the idea if that only one type of thermal zones can
exist for a given system (i.e. the selection happens at build time),
then my objection can safely be ignored, except for the fact that I
still would like the "name" attribute to reflect the type of the
thermal zones.
Rui, Len, how did you originally envision the coexistence (or not) of
different types of thermal zones?
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-17 12:37 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Jean Delvare
@ 2008-03-18 1:59 ` Zhang, Rui
-1 siblings, 0 replies; 50+ messages in thread
From: Zhang, Rui @ 2008-03-18 1:59 UTC (permalink / raw)
To: Jean Delvare
Cc: Hans de Goede, linux-acpi, lm-sensors, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
On Mon, 2008-03-17 at 20:37 +0800, Jean Delvare wrote:
> Hi Hans,
>
> On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
> > Zhang, Rui wrote:
> > > Add hwmon sys I/F for the generic thermal device.
> > >
> >
> > Great!
> >
> > But I have several remarks:
> > 1) Looking at the new code, you only add temp1_input, so I'm
> guessing that you
> > are registering a seperate hwmon class entry per zone? Please don't
> do that,
> > please register one hwmon class entry, and add multiple temp#_input
> attr to it
> > (and the same for crit).
>
> I am sorry that I did not notice when you suggested this. I disagree,
> but now Rui's code is upstream so I guess it's too late to complain.
> Still here are my reasons:
>
> One of the great things about libsensors is that it gives unique names
> to hardware monitoring devices, and for each device, each feature has
> a
> unique name as well. This makes it possible to ignore or label a
> specific feature in /etc/sensors.conf in a way that is stable over
> reboot and addition of new hardware.
Interesting.
> By going with a single virtual device for all thermal zones, you break
> this model. Depending on which thermal zone drivers are loaded and in
> which order they are loaded, there will be more or less temp* files in
> the hwmon directory and you also can't predict their order.
Yes, you're right.
> The
> labelling issue could be solved by adding temp*_label files, but this
> still prevents the user from overriding a label.
why user needs to override the label?
> And there's no way to
> reliably ignore a specific thermal zone or to ask for information
> about a specific thermal zone with the current model.
>
> For this reason, I think it would be much better to have one hwmon
> class device for each _type_ of thermal zone.
Well. I disagree.
First, the generic thermal driver only checks the callbacks and builds
the sys I/F for each thermal zone registered. So there is no difference
between different thermal zones. Checking the type of different thermal
zone and register a hwmon device for each _type of thermal zones doesn't
make sense to me. :(
Second, take ACPI thermal zone for example, the number/order of the ACPI
thermal zones can not be predicted either. All of this information is
gotten during system boot time.
> For example, all ACPI
> thermal zones would be listed as one hwmon class device. If we later
> add support for another type of thermal zones, all thermal zones of
> this type would be listed as one (different) hwmon class device. This
> makes each specific thermal zone driver responsible for the stability
> of the numbering of the various thermal zones of a given type. This
> would also let us give names to the different thermal zone types (e.g.
> "acpitz" for ACPI thermal zones) so that the users and supporters have
> an idea who is providing these temperature values and limits.
If we really need to do this, I suggest we do this in the native driver.
Say acpi thermal driver is responsible for registering a hwmon device
for ACPI thermal zones, just like the other hwmon native sensor drivers.
what do you think?
thanks,
rui
>
> --
> Jean Delvare
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-18 1:59 ` Zhang, Rui
0 siblings, 0 replies; 50+ messages in thread
From: Zhang, Rui @ 2008-03-18 1:59 UTC (permalink / raw)
To: Jean Delvare
Cc: Hans de Goede, linux-acpi, lm-sensors, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
On Mon, 2008-03-17 at 20:37 +0800, Jean Delvare wrote:
> Hi Hans,
>
> On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
> > Zhang, Rui wrote:
> > > Add hwmon sys I/F for the generic thermal device.
> > >
> >
> > Great!
> >
> > But I have several remarks:
> > 1) Looking at the new code, you only add temp1_input, so I'm
> guessing that you
> > are registering a seperate hwmon class entry per zone? Please don't
> do that,
> > please register one hwmon class entry, and add multiple temp#_input
> attr to it
> > (and the same for crit).
>
> I am sorry that I did not notice when you suggested this. I disagree,
> but now Rui's code is upstream so I guess it's too late to complain.
> Still here are my reasons:
>
> One of the great things about libsensors is that it gives unique names
> to hardware monitoring devices, and for each device, each feature has
> a
> unique name as well. This makes it possible to ignore or label a
> specific feature in /etc/sensors.conf in a way that is stable over
> reboot and addition of new hardware.
Interesting.
> By going with a single virtual device for all thermal zones, you break
> this model. Depending on which thermal zone drivers are loaded and in
> which order they are loaded, there will be more or less temp* files in
> the hwmon directory and you also can't predict their order.
Yes, you're right.
> The
> labelling issue could be solved by adding temp*_label files, but this
> still prevents the user from overriding a label.
why user needs to override the label?
> And there's no way to
> reliably ignore a specific thermal zone or to ask for information
> about a specific thermal zone with the current model.
>
> For this reason, I think it would be much better to have one hwmon
> class device for each _type_ of thermal zone.
Well. I disagree.
First, the generic thermal driver only checks the callbacks and builds
the sys I/F for each thermal zone registered. So there is no difference
between different thermal zones. Checking the type of different thermal
zone and register a hwmon device for each _type of thermal zones doesn't
make sense to me. :(
Second, take ACPI thermal zone for example, the number/order of the ACPI
thermal zones can not be predicted either. All of this information is
gotten during system boot time.
> For example, all ACPI
> thermal zones would be listed as one hwmon class device. If we later
> add support for another type of thermal zones, all thermal zones of
> this type would be listed as one (different) hwmon class device. This
> makes each specific thermal zone driver responsible for the stability
> of the numbering of the various thermal zones of a given type. This
> would also let us give names to the different thermal zone types (e.g.
> "acpitz" for ACPI thermal zones) so that the users and supporters have
> an idea who is providing these temperature values and limits.
If we really need to do this, I suggest we do this in the native driver.
Say acpi thermal driver is responsible for registering a hwmon device
for ACPI thermal zones, just like the other hwmon native sensor drivers.
what do you think?
thanks,
rui
>
> --
> Jean Delvare
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-17 12:55 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Hans de Goede
@ 2008-03-18 3:11 ` Zhang, Rui
-1 siblings, 0 replies; 50+ messages in thread
From: Zhang, Rui @ 2008-03-18 3:11 UTC (permalink / raw)
To: Hans de Goede
Cc: Jean Delvare, linux-acpi, lm-sensors, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
On Mon, 2008-03-17 at 20:55 +0800, Hans de Goede wrote:
> Jean Delvare wrote:
> > Hi Hans,
> >
> > On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
> >> Zhang, Rui wrote:
> >>> Add hwmon sys I/F for the generic thermal device.
> >>>
> >> Great!
> >>
> >> But I have several remarks:
> >> 1) Looking at the new code, you only add temp1_input, so I'm
> guessing that you
> >> are registering a seperate hwmon class entry per zone? Please don't
> do that,
> >> please register one hwmon class entry, and add multiple temp#_input
> attr to it
> >> (and the same for crit).
> >
> > I am sorry that I did not notice when you suggested this. I
> disagree,
> > but now Rui's code is upstream so I guess it's too late to complain.
> > Still here are my reasons:
> >
> > One of the great things about libsensors is that it gives unique
> names
> > to hardware monitoring devices, and for each device, each feature
> has a
> > unique name as well. This makes it possible to ignore or label a
> > specific feature in /etc/sensors.conf in a way that is stable over
> > reboot and addition of new hardware.
> >
> > By going with a single virtual device for all thermal zones, you
> break
> > this model. Depending on which thermal zone drivers are loaded and
> in
> > which order they are loaded, there will be more or less temp* files
> in
> > the hwmon directory and you also can't predict their order. The
> > labelling issue could be solved by adding temp*_label files, but
> this
> > still prevents the user from overriding a label. And there's no way
> to
> > reliably ignore a specific thermal zone or to ask for information
> about
> > a specific thermal zone with the current model.
> >
> > For this reason, I think it would be much better to have one hwmon
> > class device for each _type_ of thermal zone. For example, all ACPI
> > thermal zones would be listed as one hwmon class device. If we later
> > add support for another type of thermal zones, all thermal zones of
> > this type would be listed as one (different) hwmon class device.
> This
> > makes each specific thermal zone driver responsible for the
> stability
> > of the numbering of the various thermal zones of a given type. This
> > would also let us give names to the different thermal zone types
> (e.g.
> > "acpitz" for ACPI thermal zones) so that the users and supporters
> have
> > an idea who is providing these temperature values and limits.
> >
>
> I fully agree, I didn't know there was a generic thermal zone model,
> and that
> there could be multiple drivers implementing it (let alone multiple
> thermal
> zone drivers active for one system ??) I thought this was all ACPI
> only,
The purpose of the thermal zone driver is to build a generic sys I/F for
thermal management in user space. Different type of thermal zones can
make use of it doesn't mean they are active for one system. :)
thanks,
rui
> If this (multiple thermal zone drivers active for one system) can
> really happen
> then we really should fix it so that there is one hwmon class entry
> per thermal
> zone driver.
> This can be done without changing the ABI, as things would still
> follow the standard hwmon ABI.
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-18 3:11 ` Zhang, Rui
0 siblings, 0 replies; 50+ messages in thread
From: Zhang, Rui @ 2008-03-18 3:11 UTC (permalink / raw)
To: Hans de Goede
Cc: Jean Delvare, linux-acpi, lm-sensors, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
On Mon, 2008-03-17 at 20:55 +0800, Hans de Goede wrote:
> Jean Delvare wrote:
> > Hi Hans,
> >
> > On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
> >> Zhang, Rui wrote:
> >>> Add hwmon sys I/F for the generic thermal device.
> >>>
> >> Great!
> >>
> >> But I have several remarks:
> >> 1) Looking at the new code, you only add temp1_input, so I'm
> guessing that you
> >> are registering a seperate hwmon class entry per zone? Please don't
> do that,
> >> please register one hwmon class entry, and add multiple temp#_input
> attr to it
> >> (and the same for crit).
> >
> > I am sorry that I did not notice when you suggested this. I
> disagree,
> > but now Rui's code is upstream so I guess it's too late to complain.
> > Still here are my reasons:
> >
> > One of the great things about libsensors is that it gives unique
> names
> > to hardware monitoring devices, and for each device, each feature
> has a
> > unique name as well. This makes it possible to ignore or label a
> > specific feature in /etc/sensors.conf in a way that is stable over
> > reboot and addition of new hardware.
> >
> > By going with a single virtual device for all thermal zones, you
> break
> > this model. Depending on which thermal zone drivers are loaded and
> in
> > which order they are loaded, there will be more or less temp* files
> in
> > the hwmon directory and you also can't predict their order. The
> > labelling issue could be solved by adding temp*_label files, but
> this
> > still prevents the user from overriding a label. And there's no way
> to
> > reliably ignore a specific thermal zone or to ask for information
> about
> > a specific thermal zone with the current model.
> >
> > For this reason, I think it would be much better to have one hwmon
> > class device for each _type_ of thermal zone. For example, all ACPI
> > thermal zones would be listed as one hwmon class device. If we later
> > add support for another type of thermal zones, all thermal zones of
> > this type would be listed as one (different) hwmon class device.
> This
> > makes each specific thermal zone driver responsible for the
> stability
> > of the numbering of the various thermal zones of a given type. This
> > would also let us give names to the different thermal zone types
> (e.g.
> > "acpitz" for ACPI thermal zones) so that the users and supporters
> have
> > an idea who is providing these temperature values and limits.
> >
>
> I fully agree, I didn't know there was a generic thermal zone model,
> and that
> there could be multiple drivers implementing it (let alone multiple
> thermal
> zone drivers active for one system ??) I thought this was all ACPI
> only,
The purpose of the thermal zone driver is to build a generic sys I/F for
thermal management in user space. Different type of thermal zones can
make use of it doesn't mean they are active for one system. :)
thanks,
rui
> If this (multiple thermal zone drivers active for one system) can
> really happen
> then we really should fix it so that there is one hwmon class entry
> per thermal
> zone driver.
> This can be done without changing the ABI, as things would still
> follow the standard hwmon ABI.
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-17 13:48 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Jean Delvare
@ 2008-03-18 3:45 ` Zhang, Rui
-1 siblings, 0 replies; 50+ messages in thread
From: Zhang, Rui @ 2008-03-18 3:45 UTC (permalink / raw)
To: Jean Delvare
Cc: Hans de Goede, Len Brown, linux-acpi, lm-sensors,
Matthew Garrett, Thomas Renninger, Thomas, Sujith,
Mark M. Hoffman, Henrique de Moraes Holschuh, Richard Hughes
On Mon, 2008-03-17 at 21:48 +0800, Jean Delvare wrote:
> On Mon, 17 Mar 2008 13:55:00 +0100, Hans de Goede wrote:
> > Jean Delvare wrote:
> > > Hi Hans,
> > >
> > > On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
> > >> Zhang, Rui wrote:
> > >>> Add hwmon sys I/F for the generic thermal device.
> > >>>
> > >> Great!
> > >>
> > >> But I have several remarks:
> > >> 1) Looking at the new code, you only add temp1_input, so I'm
> guessing that you
> > >> are registering a seperate hwmon class entry per zone? Please
> don't do that,
> > >> please register one hwmon class entry, and add multiple
> temp#_input attr to it
> > >> (and the same for crit).
> > >
> > > I am sorry that I did not notice when you suggested this. I
> disagree,
> > > but now Rui's code is upstream so I guess it's too late to
> complain.
> > > Still here are my reasons:
> > >
> > > One of the great things about libsensors is that it gives unique
> names
> > > to hardware monitoring devices, and for each device, each feature
> has a
> > > unique name as well. This makes it possible to ignore or label a
> > > specific feature in /etc/sensors.conf in a way that is stable over
> > > reboot and addition of new hardware.
> > >
> > > By going with a single virtual device for all thermal zones, you
> break
> > > this model. Depending on which thermal zone drivers are loaded and
> in
> > > which order they are loaded, there will be more or less temp*
> files in
> > > the hwmon directory and you also can't predict their order. The
> > > labelling issue could be solved by adding temp*_label files, but
> this
> > > still prevents the user from overriding a label. And there's no
> way to
> > > reliably ignore a specific thermal zone or to ask for information
> about
> > > a specific thermal zone with the current model.
> > >
> > > For this reason, I think it would be much better to have one hwmon
> > > class device for each _type_ of thermal zone. For example, all
> ACPI
> > > thermal zones would be listed as one hwmon class device. If we
> later
> > > add support for another type of thermal zones, all thermal zones
> of
> > > this type would be listed as one (different) hwmon class device.
> This
> > > makes each specific thermal zone driver responsible for the
> stability
> > > of the numbering of the various thermal zones of a given type.
> This
> > > would also let us give names to the different thermal zone types
> (e.g.
> > > "acpitz" for ACPI thermal zones) so that the users and supporters
> have
> > > an idea who is providing these temperature values and limits.
> > >
> >
> > I fully agree, I didn't know there was a generic thermal zone model,
> and that
> > there could be multiple drivers implementing it (let alone multiple
> thermal
> > zone drivers active for one system ??) I thought this was all ACPI
> only,
>
> My understanding is that the new thermal zone code is meant to be
> generic and ACPI is only one possible underlying implementations,
> which
> sounds very good to me. ACPI just happens to be the only
> implementation
> at the time being.
>
> > If this (multiple thermal zone drivers active for one system) can
> really happen
> > then we really should fix it so that there is one hwmon class entry
> per thermal
> > zone driver. This can be done without changing the ABI, as things
> would still
> > follow the standard hwmon ABI.
>
> I have to admit that I'm not sure what sense that would make (and how
> safe it would be) to have more than one type of thermal zones active
> at
> the same time.
> If the idea if that only one type of thermal zones can
> exist for a given system (i.e. the selection happens at build time),
> then my objection can safely be ignored, except for the fact that I
> still would like the "name" attribute to reflect the type of the
> thermal zones.
>
> Rui, Len, how did you originally envision the coexistence (or not) of
> different types of thermal zones?
driver/thermal/thermal.c won't change any behavior of the current
system. It just creates a generic sys I/F, that's why we call it the
Generic Thermal Sysfs driver. :)
We want to introduce a generic solution for thermal management, which
usually contains a user application for policy control, a generic
thermal sysfs driver which provides a set of platform-independent
interfaces, native sensor drivers and device drivers for thermal
monitoring and device throttling.
Note that the target is the handheld devices which is not covered by
hwmon.
The idea comes from Len's ols paper, please refer to
http://www.kernel.org/pub/linux/kernel/people/lenb/acpi/doc/OLS2007-cool-web/
I don't think the generic thermal sysfs driver need to handle the
coexistence of different types of thermal zones, because:
If there are any, they always exist without the generic thermal driver.
If they break something, it's broken before the generic thermal driver
is implemented, and the generic thermal driver give it a chance to
handle this in user space.
Please correct me if I misunderstand your question. :)
thanks,
rui
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-18 3:45 ` Zhang, Rui
0 siblings, 0 replies; 50+ messages in thread
From: Zhang, Rui @ 2008-03-18 3:45 UTC (permalink / raw)
To: Jean Delvare
Cc: Hans de Goede, Len Brown, linux-acpi, lm-sensors,
Matthew Garrett, Thomas Renninger, Thomas, Sujith,
Mark M. Hoffman, Henrique de Moraes Holschuh, Richard Hughes
On Mon, 2008-03-17 at 21:48 +0800, Jean Delvare wrote:
> On Mon, 17 Mar 2008 13:55:00 +0100, Hans de Goede wrote:
> > Jean Delvare wrote:
> > > Hi Hans,
> > >
> > > On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
> > >> Zhang, Rui wrote:
> > >>> Add hwmon sys I/F for the generic thermal device.
> > >>>
> > >> Great!
> > >>
> > >> But I have several remarks:
> > >> 1) Looking at the new code, you only add temp1_input, so I'm
> guessing that you
> > >> are registering a seperate hwmon class entry per zone? Please
> don't do that,
> > >> please register one hwmon class entry, and add multiple
> temp#_input attr to it
> > >> (and the same for crit).
> > >
> > > I am sorry that I did not notice when you suggested this. I
> disagree,
> > > but now Rui's code is upstream so I guess it's too late to
> complain.
> > > Still here are my reasons:
> > >
> > > One of the great things about libsensors is that it gives unique
> names
> > > to hardware monitoring devices, and for each device, each feature
> has a
> > > unique name as well. This makes it possible to ignore or label a
> > > specific feature in /etc/sensors.conf in a way that is stable over
> > > reboot and addition of new hardware.
> > >
> > > By going with a single virtual device for all thermal zones, you
> break
> > > this model. Depending on which thermal zone drivers are loaded and
> in
> > > which order they are loaded, there will be more or less temp*
> files in
> > > the hwmon directory and you also can't predict their order. The
> > > labelling issue could be solved by adding temp*_label files, but
> this
> > > still prevents the user from overriding a label. And there's no
> way to
> > > reliably ignore a specific thermal zone or to ask for information
> about
> > > a specific thermal zone with the current model.
> > >
> > > For this reason, I think it would be much better to have one hwmon
> > > class device for each _type_ of thermal zone. For example, all
> ACPI
> > > thermal zones would be listed as one hwmon class device. If we
> later
> > > add support for another type of thermal zones, all thermal zones
> of
> > > this type would be listed as one (different) hwmon class device.
> This
> > > makes each specific thermal zone driver responsible for the
> stability
> > > of the numbering of the various thermal zones of a given type.
> This
> > > would also let us give names to the different thermal zone types
> (e.g.
> > > "acpitz" for ACPI thermal zones) so that the users and supporters
> have
> > > an idea who is providing these temperature values and limits.
> > >
> >
> > I fully agree, I didn't know there was a generic thermal zone model,
> and that
> > there could be multiple drivers implementing it (let alone multiple
> thermal
> > zone drivers active for one system ??) I thought this was all ACPI
> only,
>
> My understanding is that the new thermal zone code is meant to be
> generic and ACPI is only one possible underlying implementations,
> which
> sounds very good to me. ACPI just happens to be the only
> implementation
> at the time being.
>
> > If this (multiple thermal zone drivers active for one system) can
> really happen
> > then we really should fix it so that there is one hwmon class entry
> per thermal
> > zone driver. This can be done without changing the ABI, as things
> would still
> > follow the standard hwmon ABI.
>
> I have to admit that I'm not sure what sense that would make (and how
> safe it would be) to have more than one type of thermal zones active
> at
> the same time.
> If the idea if that only one type of thermal zones can
> exist for a given system (i.e. the selection happens at build time),
> then my objection can safely be ignored, except for the fact that I
> still would like the "name" attribute to reflect the type of the
> thermal zones.
>
> Rui, Len, how did you originally envision the coexistence (or not) of
> different types of thermal zones?
driver/thermal/thermal.c won't change any behavior of the current
system. It just creates a generic sys I/F, that's why we call it the
Generic Thermal Sysfs driver. :)
We want to introduce a generic solution for thermal management, which
usually contains a user application for policy control, a generic
thermal sysfs driver which provides a set of platform-independent
interfaces, native sensor drivers and device drivers for thermal
monitoring and device throttling.
Note that the target is the handheld devices which is not covered by
hwmon.
The idea comes from Len's ols paper, please refer to
http://www.kernel.org/pub/linux/kernel/people/lenb/acpi/doc/OLS2007-cool-web/
I don't think the generic thermal sysfs driver need to handle the
coexistence of different types of thermal zones, because:
If there are any, they always exist without the generic thermal driver.
If they break something, it's broken before the generic thermal driver
is implemented, and the generic thermal driver give it a chance to
handle this in user space.
Please correct me if I misunderstand your question. :)
thanks,
rui
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-13 8:46 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
@ 2008-03-18 4:59 ` Len Brown
-1 siblings, 0 replies; 50+ messages in thread
From: Len Brown @ 2008-03-18 4:59 UTC (permalink / raw)
To: Zhang, Rui; +Cc: linux-acpi, lm-sensors, Hans de Goede
On Thursday 13 March 2008, Zhang, Rui wrote:
>
> On Thu, 2008-03-13 at 13:09 +0800, Len Brown wrote:
> >
> > > On Tuesday 26 February 2008, Zhang, Rui wrote:
> > > >
> > > > Add hwmon sys I/F for the generic thermal device.
> > > >
> > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > > Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
> > > > ---
> > > > Documentation/thermal/sysfs-api.txt | 22 ++--
> > > > drivers/thermal/Kconfig | 1
> > > > drivers/thermal/thermal.c | 169
> > ++++++++++++++++++++++++++++++------
> > > > 3 files changed, 155 insertions(+), 37 deletions(-)
> > > >
> > ...
> > > > {
> > > > int result = 0;
> > > > @@ -716,16 +829,20 @@ static int __init thermal_init(void)
> > > > mutex_destroy(&thermal_idr_lock);
> > > > mutex_destroy(&thermal_list_lock);
> > > > }
> > > > - return result;
> > > > -}
> > > >
> > > > -static void __exit thermal_exit(void)
> > > > -{
> > > > - class_unregister(&thermal_class);
> > > > - idr_destroy(&thermal_tz_idr);
> > > > - idr_destroy(&thermal_cdev_idr);
> > > > - mutex_destroy(&thermal_idr_lock);
> > > > - mutex_destroy(&thermal_list_lock);
> > > > + thermal_hwmon = hwmon_device_register(NULL);
> > > > + if (IS_ERR(thermal_hwmon)) {
> > > > + result = PTR_ERR(thermal_hwmon);
> > > > + thermal_hwmon = NULL;
> > > > + printk(KERN_ERR PREFIX
> > > > + "unable to register hwmon device\n");
> > > > + thermal_exit();
> >
> > An __exit routine can not be called from an __init routine.
> > this doesn't build on ia64, which discards .exit sections:
> >
> > .exit.text' referenced in section `.init.text' of drivers/built-in.o:
> > defined in discarded section `.exit.text' of drivers/built-in.o
> >
> > -Len
> As I don't have an ia64 machine,
> len, could you please help me test this incremental patch? :)
>
> From: Zhang Rui <rui.zhang@intel.com>
>
> An __exit routine can not be called from an __init routine.
> this doesn't build on ia64, which discards .exit sections:
> ".exit.text' referenced in section `.init.text' of drivers/built-in.o:
> defined in discarded section `.exit.text' of drivers/built-in.o"
>
> http://marc.info/?l=linux-acpi&m=120538509025142&w=2
>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
> drivers/thermal/thermal.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> Index: linux-2.6/drivers/thermal/thermal.c
> ===================================================================
> --- linux-2.6.orig/drivers/thermal/thermal.c
> +++ linux-2.6/drivers/thermal/thermal.c
> @@ -823,12 +823,8 @@ static int __init thermal_init(void)
> int result = 0;
>
> result = class_register(&thermal_class);
> - if (result) {
> - idr_destroy(&thermal_tz_idr);
> - idr_destroy(&thermal_cdev_idr);
> - mutex_destroy(&thermal_idr_lock);
> - mutex_destroy(&thermal_list_lock);
> - }
> + if (result)
> + goto err;
>
> thermal_hwmon = hwmon_device_register(NULL);
> if (IS_ERR(thermal_hwmon)) {
> @@ -836,13 +832,20 @@ static int __init thermal_init(void)
> thermal_hwmon = NULL;
> printk(KERN_ERR PREFIX
> "unable to register hwmon device\n");
> - thermal_exit();
> - return result;
> + class_unregister(&thermal_class);
> + goto err;
> }
>
> result = device_create_file(thermal_hwmon, &dev_attr_name);
>
> return result;
> + err:
> + idr_destroy(&thermal_tz_idr);
> + idr_destroy(&thermal_cdev_idr);
> + mutex_destroy(&thermal_idr_lock);
> + mutex_destroy(&thermal_list_lock);
> +
> + return result;
> }
>
> subsys_initcall(thermal_init);
>
>
I already addressed this by modifying your patch that called thermal_exit() from __init
to simply remove the __exit from thermal_exit().
ie. as Henrique does in thinkpad_acpi.c -- we simply don't reward =y builds
by labeling anything with __exit for the tools to strip.
thanks,
-Len
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-18 4:59 ` Len Brown
0 siblings, 0 replies; 50+ messages in thread
From: Len Brown @ 2008-03-18 4:59 UTC (permalink / raw)
To: Zhang, Rui; +Cc: linux-acpi, lm-sensors, Hans de Goede
On Thursday 13 March 2008, Zhang, Rui wrote:
>
> On Thu, 2008-03-13 at 13:09 +0800, Len Brown wrote:
> >
> > > On Tuesday 26 February 2008, Zhang, Rui wrote:
> > > >
> > > > Add hwmon sys I/F for the generic thermal device.
> > > >
> > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > > Cc: Hans de Geode <j.w.r.degoede@hhs.nl>
> > > > ---
> > > > Documentation/thermal/sysfs-api.txt | 22 ++--
> > > > drivers/thermal/Kconfig | 1
> > > > drivers/thermal/thermal.c | 169
> > ++++++++++++++++++++++++++++++------
> > > > 3 files changed, 155 insertions(+), 37 deletions(-)
> > > >
> > ...
> > > > {
> > > > int result = 0;
> > > > @@ -716,16 +829,20 @@ static int __init thermal_init(void)
> > > > mutex_destroy(&thermal_idr_lock);
> > > > mutex_destroy(&thermal_list_lock);
> > > > }
> > > > - return result;
> > > > -}
> > > >
> > > > -static void __exit thermal_exit(void)
> > > > -{
> > > > - class_unregister(&thermal_class);
> > > > - idr_destroy(&thermal_tz_idr);
> > > > - idr_destroy(&thermal_cdev_idr);
> > > > - mutex_destroy(&thermal_idr_lock);
> > > > - mutex_destroy(&thermal_list_lock);
> > > > + thermal_hwmon = hwmon_device_register(NULL);
> > > > + if (IS_ERR(thermal_hwmon)) {
> > > > + result = PTR_ERR(thermal_hwmon);
> > > > + thermal_hwmon = NULL;
> > > > + printk(KERN_ERR PREFIX
> > > > + "unable to register hwmon device\n");
> > > > + thermal_exit();
> >
> > An __exit routine can not be called from an __init routine.
> > this doesn't build on ia64, which discards .exit sections:
> >
> > .exit.text' referenced in section `.init.text' of drivers/built-in.o:
> > defined in discarded section `.exit.text' of drivers/built-in.o
> >
> > -Len
> As I don't have an ia64 machine,
> len, could you please help me test this incremental patch? :)
>
> From: Zhang Rui <rui.zhang@intel.com>
>
> An __exit routine can not be called from an __init routine.
> this doesn't build on ia64, which discards .exit sections:
> ".exit.text' referenced in section `.init.text' of drivers/built-in.o:
> defined in discarded section `.exit.text' of drivers/built-in.o"
>
> http://marc.info/?l=linux-acpi&m\x120538509025142&w=2
>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
> drivers/thermal/thermal.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> Index: linux-2.6/drivers/thermal/thermal.c
> =================================> --- linux-2.6.orig/drivers/thermal/thermal.c
> +++ linux-2.6/drivers/thermal/thermal.c
> @@ -823,12 +823,8 @@ static int __init thermal_init(void)
> int result = 0;
>
> result = class_register(&thermal_class);
> - if (result) {
> - idr_destroy(&thermal_tz_idr);
> - idr_destroy(&thermal_cdev_idr);
> - mutex_destroy(&thermal_idr_lock);
> - mutex_destroy(&thermal_list_lock);
> - }
> + if (result)
> + goto err;
>
> thermal_hwmon = hwmon_device_register(NULL);
> if (IS_ERR(thermal_hwmon)) {
> @@ -836,13 +832,20 @@ static int __init thermal_init(void)
> thermal_hwmon = NULL;
> printk(KERN_ERR PREFIX
> "unable to register hwmon device\n");
> - thermal_exit();
> - return result;
> + class_unregister(&thermal_class);
> + goto err;
> }
>
> result = device_create_file(thermal_hwmon, &dev_attr_name);
>
> return result;
> + err:
> + idr_destroy(&thermal_tz_idr);
> + idr_destroy(&thermal_cdev_idr);
> + mutex_destroy(&thermal_idr_lock);
> + mutex_destroy(&thermal_list_lock);
> +
> + return result;
> }
>
> subsys_initcall(thermal_init);
>
>
I already addressed this by modifying your patch that called thermal_exit() from __init
to simply remove the __exit from thermal_exit().
ie. as Henrique does in thinkpad_acpi.c -- we simply don't reward =y builds
by labeling anything with __exit for the tools to strip.
thanks,
-Len
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-17 13:48 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Jean Delvare
@ 2008-03-18 5:12 ` Len Brown
-1 siblings, 0 replies; 50+ messages in thread
From: Len Brown @ 2008-03-18 5:12 UTC (permalink / raw)
To: Jean Delvare
Cc: Hans de Goede, Zhang, Rui, linux-acpi, lm-sensors,
Matthew Garrett, Thomas Renninger, Thomas, Sujith,
Mark M. Hoffman, Henrique de Moraes Holschuh, Richard Hughes
On Monday 17 March 2008, Jean Delvare wrote:
> On Mon, 17 Mar 2008 13:55:00 +0100, Hans de Goede wrote:
> > Jean Delvare wrote:
> > > Hi Hans,
> > >
> > > On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
> > >> Zhang, Rui wrote:
> > >>> Add hwmon sys I/F for the generic thermal device.
> > >>>
> > >> Great!
> > >>
> > >> But I have several remarks:
> > >> 1) Looking at the new code, you only add temp1_input, so I'm guessing that you
> > >> are registering a seperate hwmon class entry per zone? Please don't do that,
> > >> please register one hwmon class entry, and add multiple temp#_input attr to it
> > >> (and the same for crit).
> > >
> > > I am sorry that I did not notice when you suggested this. I disagree,
> > > but now Rui's code is upstream so I guess it's too late to complain.
> > > Still here are my reasons:
> > >
> > > One of the great things about libsensors is that it gives unique names
> > > to hardware monitoring devices, and for each device, each feature has a
> > > unique name as well. This makes it possible to ignore or label a
> > > specific feature in /etc/sensors.conf in a way that is stable over
> > > reboot and addition of new hardware.
> > >
> > > By going with a single virtual device for all thermal zones, you break
> > > this model. Depending on which thermal zone drivers are loaded and in
> > > which order they are loaded, there will be more or less temp* files in
> > > the hwmon directory and you also can't predict their order. The
> > > labelling issue could be solved by adding temp*_label files, but this
> > > still prevents the user from overriding a label. And there's no way to
> > > reliably ignore a specific thermal zone or to ask for information about
> > > a specific thermal zone with the current model.
> > >
> > > For this reason, I think it would be much better to have one hwmon
> > > class device for each _type_ of thermal zone. For example, all ACPI
> > > thermal zones would be listed as one hwmon class device. If we later
> > > add support for another type of thermal zones, all thermal zones of
> > > this type would be listed as one (different) hwmon class device. This
> > > makes each specific thermal zone driver responsible for the stability
> > > of the numbering of the various thermal zones of a given type. This
> > > would also let us give names to the different thermal zone types (e.g.
> > > "acpitz" for ACPI thermal zones) so that the users and supporters have
> > > an idea who is providing these temperature values and limits.
> > >
> >
> > I fully agree, I didn't know there was a generic thermal zone model, and that
> > there could be multiple drivers implementing it (let alone multiple thermal
> > zone drivers active for one system ??) I thought this was all ACPI only,
>
> My understanding is that the new thermal zone code is meant to be
> generic and ACPI is only one possible underlying implementations, which
> sounds very good to me. ACPI just happens to be the only implementation
> at the time being.
Your understanding is correct.
While ACPI is currently the only customer of the generic thermal I/F,
there is a future platform in the pipeline that has no ACPI
and it too will use the generic thermal I/F.
> > If this (multiple thermal zone drivers active for one system) can really happen
> > then we really should fix it so that there is one hwmon class entry per thermal
> > zone driver. This can be done without changing the ABI, as things would still
> > follow the standard hwmon ABI.
>
> I have to admit that I'm not sure what sense that would make (and how
> safe it would be) to have more than one type of thermal zones active at
> the same time. If the idea if that only one type of thermal zones can
> exist for a given system (i.e. the selection happens at build time),
> then my objection can safely be ignored, except for the fact that I
> still would like the "name" attribute to reflect the type of the
> thermal zones.
>
> Rui, Len, how did you originally envision the coexistence (or not) of
> different types of thermal zones?
Right now we seem to be ACPI and then non-ACPI on different systems.
However, I don't see any reason that both can't be on the same system.
-Len
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-18 5:12 ` Len Brown
0 siblings, 0 replies; 50+ messages in thread
From: Len Brown @ 2008-03-18 5:12 UTC (permalink / raw)
To: Jean Delvare
Cc: Hans de Goede, Zhang, Rui, linux-acpi, lm-sensors,
Matthew Garrett, Thomas Renninger, Thomas, Sujith,
Mark M. Hoffman, Henrique de Moraes Holschuh, Richard Hughes
On Monday 17 March 2008, Jean Delvare wrote:
> On Mon, 17 Mar 2008 13:55:00 +0100, Hans de Goede wrote:
> > Jean Delvare wrote:
> > > Hi Hans,
> > >
> > > On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
> > >> Zhang, Rui wrote:
> > >>> Add hwmon sys I/F for the generic thermal device.
> > >>>
> > >> Great!
> > >>
> > >> But I have several remarks:
> > >> 1) Looking at the new code, you only add temp1_input, so I'm guessing that you
> > >> are registering a seperate hwmon class entry per zone? Please don't do that,
> > >> please register one hwmon class entry, and add multiple temp#_input attr to it
> > >> (and the same for crit).
> > >
> > > I am sorry that I did not notice when you suggested this. I disagree,
> > > but now Rui's code is upstream so I guess it's too late to complain.
> > > Still here are my reasons:
> > >
> > > One of the great things about libsensors is that it gives unique names
> > > to hardware monitoring devices, and for each device, each feature has a
> > > unique name as well. This makes it possible to ignore or label a
> > > specific feature in /etc/sensors.conf in a way that is stable over
> > > reboot and addition of new hardware.
> > >
> > > By going with a single virtual device for all thermal zones, you break
> > > this model. Depending on which thermal zone drivers are loaded and in
> > > which order they are loaded, there will be more or less temp* files in
> > > the hwmon directory and you also can't predict their order. The
> > > labelling issue could be solved by adding temp*_label files, but this
> > > still prevents the user from overriding a label. And there's no way to
> > > reliably ignore a specific thermal zone or to ask for information about
> > > a specific thermal zone with the current model.
> > >
> > > For this reason, I think it would be much better to have one hwmon
> > > class device for each _type_ of thermal zone. For example, all ACPI
> > > thermal zones would be listed as one hwmon class device. If we later
> > > add support for another type of thermal zones, all thermal zones of
> > > this type would be listed as one (different) hwmon class device. This
> > > makes each specific thermal zone driver responsible for the stability
> > > of the numbering of the various thermal zones of a given type. This
> > > would also let us give names to the different thermal zone types (e.g.
> > > "acpitz" for ACPI thermal zones) so that the users and supporters have
> > > an idea who is providing these temperature values and limits.
> > >
> >
> > I fully agree, I didn't know there was a generic thermal zone model, and that
> > there could be multiple drivers implementing it (let alone multiple thermal
> > zone drivers active for one system ??) I thought this was all ACPI only,
>
> My understanding is that the new thermal zone code is meant to be
> generic and ACPI is only one possible underlying implementations, which
> sounds very good to me. ACPI just happens to be the only implementation
> at the time being.
Your understanding is correct.
While ACPI is currently the only customer of the generic thermal I/F,
there is a future platform in the pipeline that has no ACPI
and it too will use the generic thermal I/F.
> > If this (multiple thermal zone drivers active for one system) can really happen
> > then we really should fix it so that there is one hwmon class entry per thermal
> > zone driver. This can be done without changing the ABI, as things would still
> > follow the standard hwmon ABI.
>
> I have to admit that I'm not sure what sense that would make (and how
> safe it would be) to have more than one type of thermal zones active at
> the same time. If the idea if that only one type of thermal zones can
> exist for a given system (i.e. the selection happens at build time),
> then my objection can safely be ignored, except for the fact that I
> still would like the "name" attribute to reflect the type of the
> thermal zones.
>
> Rui, Len, how did you originally envision the coexistence (or not) of
> different types of thermal zones?
Right now we seem to be ACPI and then non-ACPI on different systems.
However, I don't see any reason that both can't be on the same system.
-Len
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-18 1:59 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
@ 2008-03-18 9:25 ` Hans de Goede
-1 siblings, 0 replies; 50+ messages in thread
From: Hans de Goede @ 2008-03-18 9:25 UTC (permalink / raw)
To: Zhang, Rui
Cc: Jean Delvare, linux-acpi, lm-sensors, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
Zhang, Rui wrote:
> On Mon, 2008-03-17 at 20:37 +0800, Jean Delvare wrote:
>> The
>> labelling issue could be solved by adding temp*_label files, but this
>> still prevents the user from overriding a label.
> why user needs to override the label?
Well allowing the user to specify user visible names is always good. But that
can still be done with temp*_label files. Last time I checked (when I wrote the
_label sysfs atrr support code) it would only check for and use a foo#_label
file if no label was specified in sensors.conf.
This would of course require the temp# numbering to be consistent over reboots
and kernel versions.
Regards,
Hans
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-18 9:25 ` Hans de Goede
0 siblings, 0 replies; 50+ messages in thread
From: Hans de Goede @ 2008-03-18 9:25 UTC (permalink / raw)
To: Zhang, Rui
Cc: Jean Delvare, linux-acpi, lm-sensors, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
Zhang, Rui wrote:
> On Mon, 2008-03-17 at 20:37 +0800, Jean Delvare wrote:
>> The
>> labelling issue could be solved by adding temp*_label files, but this
>> still prevents the user from overriding a label.
> why user needs to override the label?
Well allowing the user to specify user visible names is always good. But that
can still be done with temp*_label files. Last time I checked (when I wrote the
_label sysfs atrr support code) it would only check for and use a foo#_label
file if no label was specified in sensors.conf.
This would of course require the temp# numbering to be consistent over reboots
and kernel versions.
Regards,
Hans
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-18 1:59 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
@ 2008-03-18 9:40 ` Jean Delvare
-1 siblings, 0 replies; 50+ messages in thread
From: Jean Delvare @ 2008-03-18 9:40 UTC (permalink / raw)
To: Zhang, Rui
Cc: Hans de Goede, linux-acpi, lm-sensors, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
Hi Rui,
On Tue, 18 Mar 2008 09:59:29 +0800, Zhang, Rui wrote:
> On Mon, 2008-03-17 at 20:37 +0800, Jean Delvare wrote:
> > I am sorry that I did not notice when you suggested this. I disagree,
> > but now Rui's code is upstream so I guess it's too late to complain.
> > Still here are my reasons:
> >
> > One of the great things about libsensors is that it gives unique names
> > to hardware monitoring devices, and for each device, each feature has
> > a
> > unique name as well. This makes it possible to ignore or label a
> > specific feature in /etc/sensors.conf in a way that is stable over
> > reboot and addition of new hardware.
> Interesting.
>
> > By going with a single virtual device for all thermal zones, you break
> > this model. Depending on which thermal zone drivers are loaded and in
> > which order they are loaded, there will be more or less temp* files in
> > the hwmon directory and you also can't predict their order.
> Yes, you're right.
>
> > The
> > labelling issue could be solved by adding temp*_label files, but this
> > still prevents the user from overriding a label.
> why user needs to override the label?
Originally, the kernel drivers don't provide any labels (because they
don't know which temperature sensor corresponds to what) so the default
labels are temp1, temp2, temp3 etc. The users instead what to see "CPU
Temp", "M/B Temp" etc. which is why libsensors makes it possible to
associate a label to each input. If a given kernel driver knows what
temperature sensors correspond to, then this driver could propose a
label. However there are many reasons why the user may want to override
the proposed label:
* The BIOS is broken and mislabeled the zone.
* The default label is too long and doesn't fit it some graphical
interface.
* The user wants to translate the text to his/her native language.
* Different thermal zones end up with the same label and the user
wants to differentiate them.
> > And there's no way to
> > reliably ignore a specific thermal zone or to ask for information
> > about a specific thermal zone with the current model.
> >
> > For this reason, I think it would be much better to have one hwmon
> > class device for each _type_ of thermal zone.
> Well. I disagree.
> First, the generic thermal driver only checks the callbacks and builds
> the sys I/F for each thermal zone registered. So there is no difference
> between different thermal zones. Checking the type of different thermal
> zone and register a hwmon device for each _type of thermal zones doesn't
> make sense to me. :(
The type of the thermal zone is known to the generic thermal driver
somehow, as it exposes the information through the "type" sysfs file. I
understand that it doesn't make any difference between different types
other than that, but as long as it can differentiate between them, it
could create different hwmon devices. Whether we want to do it or not,
can be discussed of course.
> Second, take ACPI thermal zone for example, the number/order of the ACPI
> thermal zones can not be predicted either. All of this information is
> gotten during system boot time.
The number/order of the ACPI thermal zones is stable accross reboots,
isn't it? That's all we really care about.
>
> > For example, all ACPI
> > thermal zones would be listed as one hwmon class device. If we later
> > add support for another type of thermal zones, all thermal zones of
> > this type would be listed as one (different) hwmon class device. This
> > makes each specific thermal zone driver responsible for the stability
> > of the numbering of the various thermal zones of a given type. This
> > would also let us give names to the different thermal zone types (e.g.
> > "acpitz" for ACPI thermal zones) so that the users and supporters have
> > an idea who is providing these temperature values and limits.
> If we really need to do this, I suggest we do this in the native driver.
> Say acpi thermal driver is responsible for registering a hwmon device
> for ACPI thermal zones, just like the other hwmon native sensor drivers.
> what do you think?
That's what I wanted to do originally, before you showed up with this
generic thermal driver. Of course each native thermal zone driver could
register its own hwmon interface. This will however cause code
duplication. I don't really care (I doubt there will be that many
different native thermal zone drivers anyway), it's really up to you
how you want to implement it, as long as it integrates properly with
libsensors.
--
Jean Delvare
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-18 9:40 ` Jean Delvare
0 siblings, 0 replies; 50+ messages in thread
From: Jean Delvare @ 2008-03-18 9:40 UTC (permalink / raw)
To: Zhang, Rui
Cc: Hans de Goede, linux-acpi, lm-sensors, Matthew Garrett,
Thomas Renninger, Thomas, Sujith, Mark M. Hoffman,
Henrique de Moraes Holschuh, Len Brown, Richard Hughes
Hi Rui,
On Tue, 18 Mar 2008 09:59:29 +0800, Zhang, Rui wrote:
> On Mon, 2008-03-17 at 20:37 +0800, Jean Delvare wrote:
> > I am sorry that I did not notice when you suggested this. I disagree,
> > but now Rui's code is upstream so I guess it's too late to complain.
> > Still here are my reasons:
> >
> > One of the great things about libsensors is that it gives unique names
> > to hardware monitoring devices, and for each device, each feature has
> > a
> > unique name as well. This makes it possible to ignore or label a
> > specific feature in /etc/sensors.conf in a way that is stable over
> > reboot and addition of new hardware.
> Interesting.
>
> > By going with a single virtual device for all thermal zones, you break
> > this model. Depending on which thermal zone drivers are loaded and in
> > which order they are loaded, there will be more or less temp* files in
> > the hwmon directory and you also can't predict their order.
> Yes, you're right.
>
> > The
> > labelling issue could be solved by adding temp*_label files, but this
> > still prevents the user from overriding a label.
> why user needs to override the label?
Originally, the kernel drivers don't provide any labels (because they
don't know which temperature sensor corresponds to what) so the default
labels are temp1, temp2, temp3 etc. The users instead what to see "CPU
Temp", "M/B Temp" etc. which is why libsensors makes it possible to
associate a label to each input. If a given kernel driver knows what
temperature sensors correspond to, then this driver could propose a
label. However there are many reasons why the user may want to override
the proposed label:
* The BIOS is broken and mislabeled the zone.
* The default label is too long and doesn't fit it some graphical
interface.
* The user wants to translate the text to his/her native language.
* Different thermal zones end up with the same label and the user
wants to differentiate them.
> > And there's no way to
> > reliably ignore a specific thermal zone or to ask for information
> > about a specific thermal zone with the current model.
> >
> > For this reason, I think it would be much better to have one hwmon
> > class device for each _type_ of thermal zone.
> Well. I disagree.
> First, the generic thermal driver only checks the callbacks and builds
> the sys I/F for each thermal zone registered. So there is no difference
> between different thermal zones. Checking the type of different thermal
> zone and register a hwmon device for each _type of thermal zones doesn't
> make sense to me. :(
The type of the thermal zone is known to the generic thermal driver
somehow, as it exposes the information through the "type" sysfs file. I
understand that it doesn't make any difference between different types
other than that, but as long as it can differentiate between them, it
could create different hwmon devices. Whether we want to do it or not,
can be discussed of course.
> Second, take ACPI thermal zone for example, the number/order of the ACPI
> thermal zones can not be predicted either. All of this information is
> gotten during system boot time.
The number/order of the ACPI thermal zones is stable accross reboots,
isn't it? That's all we really care about.
>
> > For example, all ACPI
> > thermal zones would be listed as one hwmon class device. If we later
> > add support for another type of thermal zones, all thermal zones of
> > this type would be listed as one (different) hwmon class device. This
> > makes each specific thermal zone driver responsible for the stability
> > of the numbering of the various thermal zones of a given type. This
> > would also let us give names to the different thermal zone types (e.g.
> > "acpitz" for ACPI thermal zones) so that the users and supporters have
> > an idea who is providing these temperature values and limits.
> If we really need to do this, I suggest we do this in the native driver.
> Say acpi thermal driver is responsible for registering a hwmon device
> for ACPI thermal zones, just like the other hwmon native sensor drivers.
> what do you think?
That's what I wanted to do originally, before you showed up with this
generic thermal driver. Of course each native thermal zone driver could
register its own hwmon interface. This will however cause code
duplication. I don't really care (I doubt there will be that many
different native thermal zone drivers anyway), it's really up to you
how you want to implement it, as long as it integrates properly with
libsensors.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-18 5:12 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Len Brown
@ 2008-03-18 9:44 ` Jean Delvare
-1 siblings, 0 replies; 50+ messages in thread
From: Jean Delvare @ 2008-03-18 9:44 UTC (permalink / raw)
To: Len Brown
Cc: Hans de Goede, Zhang, Rui, linux-acpi, lm-sensors,
Matthew Garrett, Thomas Renninger, Thomas, Sujith,
Mark M. Hoffman, Henrique de Moraes Holschuh, Richard Hughes
On Tue, 18 Mar 2008 01:12:58 -0400, Len Brown wrote:
> While ACPI is currently the only customer of the generic thermal I/F,
> there is a future platform in the pipeline that has no ACPI
> and it too will use the generic thermal I/F.
A future platform that has no ACPI? Yeeehoooo! :)
(Sorry couldn't resist.)
> > (...)
> > Rui, Len, how did you originally envision the coexistence (or not) of
> > different types of thermal zones?
>
> Right now we seem to be ACPI and then non-ACPI on different systems.
> However, I don't see any reason that both can't be on the same system.
OK, in this case this confirms that the current model (all thermal
zones in one hwmon device) is not correct.
--
Jean Delvare
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-18 9:44 ` Jean Delvare
0 siblings, 0 replies; 50+ messages in thread
From: Jean Delvare @ 2008-03-18 9:44 UTC (permalink / raw)
To: Len Brown
Cc: Hans de Goede, Zhang, Rui, linux-acpi, lm-sensors,
Matthew Garrett, Thomas Renninger, Thomas, Sujith,
Mark M. Hoffman, Henrique de Moraes Holschuh, Richard Hughes
On Tue, 18 Mar 2008 01:12:58 -0400, Len Brown wrote:
> While ACPI is currently the only customer of the generic thermal I/F,
> there is a future platform in the pipeline that has no ACPI
> and it too will use the generic thermal I/F.
A future platform that has no ACPI? Yeeehoooo! :)
(Sorry couldn't resist.)
> > (...)
> > Rui, Len, how did you originally envision the coexistence (or not) of
> > different types of thermal zones?
>
> Right now we seem to be ACPI and then non-ACPI on different systems.
> However, I don't see any reason that both can't be on the same system.
OK, in this case this confirms that the current model (all thermal
zones in one hwmon device) is not correct.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-18 3:45 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
@ 2008-03-18 10:06 ` Jean Delvare
-1 siblings, 0 replies; 50+ messages in thread
From: Jean Delvare @ 2008-03-18 10:06 UTC (permalink / raw)
To: Zhang, Rui
Cc: Hans de Goede, Len Brown, linux-acpi, lm-sensors,
Matthew Garrett, Thomas Renninger, Thomas, Sujith,
Mark M. Hoffman, Henrique de Moraes Holschuh, Richard Hughes
On Tue, 18 Mar 2008 11:45:52 +0800, Zhang, Rui wrote:
> On Mon, 2008-03-17 at 21:48 +0800, Jean Delvare wrote:
> > Rui, Len, how did you originally envision the coexistence (or not) of
> > different types of thermal zones?
>
> driver/thermal/thermal.c won't change any behavior of the current
> system. It just creates a generic sys I/F, that's why we call it the
> Generic Thermal Sysfs driver. :)
>
> We want to introduce a generic solution for thermal management, which
> usually contains a user application for policy control, a generic
> thermal sysfs driver which provides a set of platform-independent
> interfaces, native sensor drivers and device drivers for thermal
> monitoring and device throttling.
> Note that the target is the handheld devices which is not covered by
> hwmon.
> The idea comes from Len's ols paper, please refer to
> http://www.kernel.org/pub/linux/kernel/people/lenb/acpi/doc/OLS2007-cool-web/
>
> I don't think the generic thermal sysfs driver need to handle the
> coexistence of different types of thermal zones, because:
> If there are any, they always exist without the generic thermal driver.
> If they break something, it's broken before the generic thermal driver
> is implemented, and the generic thermal driver give it a chance to
> handle this in user space.
> Please correct me if I misunderstand your question. :)
Maybe I have not been clear, but my question was not about the generic
thermal driver itself. I understand that it's only adding an interface
to other drivers and not creating anything new. My question was about
thermal zones in general, i.e.: Do we expect systems to have more than
one thermal zone type at a given time, or not? Len seems to think we do.
--
Jean Delvare
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-18 10:06 ` Jean Delvare
0 siblings, 0 replies; 50+ messages in thread
From: Jean Delvare @ 2008-03-18 10:06 UTC (permalink / raw)
To: Zhang, Rui
Cc: Hans de Goede, Len Brown, linux-acpi, lm-sensors,
Matthew Garrett, Thomas Renninger, Thomas, Sujith,
Mark M. Hoffman, Henrique de Moraes Holschuh, Richard Hughes
On Tue, 18 Mar 2008 11:45:52 +0800, Zhang, Rui wrote:
> On Mon, 2008-03-17 at 21:48 +0800, Jean Delvare wrote:
> > Rui, Len, how did you originally envision the coexistence (or not) of
> > different types of thermal zones?
>
> driver/thermal/thermal.c won't change any behavior of the current
> system. It just creates a generic sys I/F, that's why we call it the
> Generic Thermal Sysfs driver. :)
>
> We want to introduce a generic solution for thermal management, which
> usually contains a user application for policy control, a generic
> thermal sysfs driver which provides a set of platform-independent
> interfaces, native sensor drivers and device drivers for thermal
> monitoring and device throttling.
> Note that the target is the handheld devices which is not covered by
> hwmon.
> The idea comes from Len's ols paper, please refer to
> http://www.kernel.org/pub/linux/kernel/people/lenb/acpi/doc/OLS2007-cool-web/
>
> I don't think the generic thermal sysfs driver need to handle the
> coexistence of different types of thermal zones, because:
> If there are any, they always exist without the generic thermal driver.
> If they break something, it's broken before the generic thermal driver
> is implemented, and the generic thermal driver give it a chance to
> handle this in user space.
> Please correct me if I misunderstand your question. :)
Maybe I have not been clear, but my question was not about the generic
thermal driver itself. I understand that it's only adding an interface
to other drivers and not creating anything new. My question was about
thermal zones in general, i.e.: Do we expect systems to have more than
one thermal zone type at a given time, or not? Len seems to think we do.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device
2008-03-18 10:06 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Jean Delvare
@ 2008-03-20 14:58 ` Henrique de Moraes Holschuh
-1 siblings, 0 replies; 50+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-03-20 14:58 UTC (permalink / raw)
To: Jean Delvare
Cc: Zhang, Rui, Hans de Goede, Len Brown, linux-acpi, lm-sensors,
Matthew Garrett, Thomas Renninger, Thomas, Sujith,
Mark M. Hoffman, Richard Hughes
On Tue, 18 Mar 2008, Jean Delvare wrote:
> Maybe I have not been clear, but my question was not about the generic
> thermal driver itself. I understand that it's only adding an interface
> to other drivers and not creating anything new. My question was about
> thermal zones in general, i.e.: Do we expect systems to have more than
> one thermal zone type at a given time, or not? Len seems to think we do.
We do, unless we decide to emulate ACPI-style zones to hide that.
Thinkpads have at least two types *always*, for example. EC ones, and the
ACPI ones... and there is some variable overlap between them. And there are
at least four devices related to the thermal control (CPU, GPU, gig-E, fan),
of which one (fan) is systemic and works for all zones.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal
@ 2008-03-20 14:58 ` Henrique de Moraes Holschuh
0 siblings, 0 replies; 50+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-03-20 14:58 UTC (permalink / raw)
To: Jean Delvare
Cc: Zhang, Rui, Hans de Goede, Len Brown, linux-acpi, lm-sensors,
Matthew Garrett, Thomas Renninger, Thomas, Sujith,
Mark M. Hoffman, Richard Hughes
On Tue, 18 Mar 2008, Jean Delvare wrote:
> Maybe I have not been clear, but my question was not about the generic
> thermal driver itself. I understand that it's only adding an interface
> to other drivers and not creating anything new. My question was about
> thermal zones in general, i.e.: Do we expect systems to have more than
> one thermal zone type at a given time, or not? Len seems to think we do.
We do, unless we decide to emulate ACPI-style zones to hide that.
Thinkpads have at least two types *always*, for example. EC ones, and the
ACPI ones... and there is some variable overlap between them. And there are
at least four devices related to the thermal control (CPU, GPU, gig-E, fan),
of which one (fan) is systemic and works for all zones.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 50+ messages in thread
end of thread, other threads:[~2008-03-20 14:58 UTC | newest]
Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-27 0:37 [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Zhang, Rui
2008-02-27 0:37 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
2008-03-12 4:29 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Len Brown
2008-03-12 4:29 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Len Brown
2008-03-13 5:09 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Len Brown
2008-03-13 5:09 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Len Brown
2008-03-13 8:46 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Zhang, Rui
2008-03-13 8:46 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
2008-03-18 4:59 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Len Brown
2008-03-18 4:59 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Len Brown
2008-03-13 10:59 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Thomas Renninger
2008-03-13 10:59 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Thomas Renninger
2008-03-13 23:09 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Henrique de Moraes Holschuh
2008-03-13 23:09 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Henrique de Moraes Holschuh
2008-03-14 9:03 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Thomas Renninger
2008-03-14 9:03 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Thomas Renninger
2008-03-15 4:25 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Henrique de Moraes Holschuh
2008-03-15 4:25 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Henrique de Moraes Holschuh
-- strict thread matches above, loose matches on Subject: below --
2008-02-25 21:31 [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Zhang, Rui
2008-02-25 21:31 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
2008-02-26 8:39 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Hans de Goede
2008-02-26 8:39 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Hans de Goede
2008-02-26 21:40 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Zhang, Rui
2008-02-26 21:40 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
2008-02-27 8:32 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Hans de Goede
2008-02-27 8:32 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Hans de Goede
2008-03-17 12:37 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Jean Delvare
2008-03-17 12:37 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Jean Delvare
2008-03-17 12:55 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Hans de Goede
2008-03-17 12:55 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Hans de Goede
2008-03-17 13:48 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Jean Delvare
2008-03-17 13:48 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Jean Delvare
2008-03-18 3:45 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Zhang, Rui
2008-03-18 3:45 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
2008-03-18 10:06 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Jean Delvare
2008-03-18 10:06 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Jean Delvare
2008-03-20 14:58 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Henrique de Moraes Holschuh
2008-03-20 14:58 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Henrique de Moraes Holschuh
2008-03-18 5:12 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Len Brown
2008-03-18 5:12 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Len Brown
2008-03-18 9:44 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Jean Delvare
2008-03-18 9:44 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Jean Delvare
2008-03-18 3:11 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Zhang, Rui
2008-03-18 3:11 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
2008-03-18 1:59 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Zhang, Rui
2008-03-18 1:59 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Zhang, Rui
2008-03-18 9:25 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Hans de Goede
2008-03-18 9:25 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Hans de Goede
2008-03-18 9:40 ` [PATCH 1/2] thermal: add hwmon sys I/F for thermal device Jean Delvare
2008-03-18 9:40 ` [lm-sensors] [PATCH 1/2] thermal: add hwmon sys I/F for thermal Jean Delvare
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.