[PATCH 3/6] thermal: add hwmon sysfs I/F

[PATCH 3/6] thermal: add hwmon sysfs I/F

[Zhang, Rui]

Add hwmon sys I/F for generic thermal driver.

Note: we have one hwmon class device for EACH TYPE of the thermal zone device.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/thermal/thermal.c |  164 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/thermal.h   |   24 ++++++
 2 files changed, 188 insertions(+)

Index: linux-2.6/drivers/thermal/thermal.c
===================================================================
--- linux-2.6.orig/drivers/thermal/thermal.c
+++ linux-2.6/drivers/thermal/thermal.c
@@ -291,6 +291,165 @@ thermal_cooling_device_trip_point_show(s
 
 /* Device management */
 
+#if defined(CONFIG_HWMON) ||	\
+	(defined(CONFIG_HWMON_MODULE) && defined(CONFIG_THERMAL_MODULE))
+/* hwmon sys I/F */
+#include <linux/hwmon.h>
+static LIST_HEAD(thermal_hwmon_list);
+
+static ssize_t
+name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct thermal_hwmon_device *hwmon = dev->driver_data;
+	return sprintf(buf, "%s\n", hwmon->type);
+}
+static DEVICE_ATTR(name, 0444, name_show, NULL);
+
+static ssize_t
+temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct thermal_hwmon_attr *hwmon_attr
+			= container_of(attr, struct thermal_hwmon_attr, attr);
+	struct thermal_zone_device *tz
+			= container_of(hwmon_attr, struct thermal_zone_device,
+				       temp_input);
+
+	return tz->ops->get_temp(tz, buf);
+}
+
+static ssize_t
+temp_crit_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct thermal_hwmon_attr *hwmon_attr
+			= container_of(attr, struct thermal_hwmon_attr, attr);
+	struct thermal_zone_device *tz
+			= container_of(hwmon_attr, struct thermal_zone_device,
+				       temp_crit);
+
+	return tz->ops->get_trip_temp(tz, 0, buf);
+}
+
+
+static int
+thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+	struct thermal_hwmon_device *hwmon;
+	int new_hwmon_device = 0;
+	int result;
+
+	mutex_lock(&thermal_list_lock);
+	list_for_each_entry(hwmon, &thermal_hwmon_list, node)
+		if (!strcmp(hwmon->type, tz->type)) {
+			new_hwmon_device = 1;
+			mutex_unlock(&thermal_list_lock);
+			goto register_sys_interface;
+		}
+	mutex_unlock(&thermal_list_lock);
+
+	hwmon = kzalloc(sizeof(struct thermal_hwmon_device), GFP_KERNEL);
+	if (!hwmon)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&hwmon->tz_list);
+	strncpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH);
+	hwmon->device = hwmon_device_register(NULL);
+	if (IS_ERR(hwmon->device)) {
+		result = -EINVAL;
+		goto free_mem;
+	}
+	hwmon->device->driver_data = hwmon;
+	result = device_create_file(hwmon->device, &dev_attr_name);
+	if (result)
+		goto unregister_hwmon_device;
+
+ register_sys_interface:
+	tz->hwmon = hwmon;
+	hwmon->count++;
+
+	snprintf(tz->temp_input.name, THERMAL_NAME_LENGTH,
+		 "temp%d_input", hwmon->count);
+	tz->temp_input.attr.attr.name = tz->temp_input.name;
+	tz->temp_input.attr.attr.mode = 0444;
+	tz->temp_input.attr.show = temp_input_show;
+	result = device_create_file(hwmon->device, &tz->temp_input.attr);
+	if (result)
+		goto unregister_hwmon_device;
+
+	if (tz->ops->get_crit_temp) {
+		unsigned long temperature;
+		if (!tz->ops->get_crit_temp(tz, &temperature)) {
+			snprintf(tz->temp_crit.name, THERMAL_NAME_LENGTH,
+				"temp%d_crit", hwmon->count);
+			tz->temp_crit.attr.attr.name = tz->temp_crit.name;
+			tz->temp_crit.attr.attr.mode = 0444;
+			tz->temp_crit.attr.show = temp_crit_show;
+			result = device_create_file(hwmon->device,
+						    &tz->temp_crit.attr);
+			if (result)
+				goto unregister_hwmon_device;
+		}
+	}
+
+	mutex_lock(&thermal_list_lock);
+	if (!new_hwmon_device)
+		list_add_tail(&hwmon->node, &thermal_hwmon_list);
+	list_add_tail(&tz->hwmon_node, &hwmon->tz_list);
+	mutex_unlock(&thermal_list_lock);
+
+	return 0;
+
+ unregister_hwmon_device:
+	device_remove_file(hwmon->device, &tz->temp_crit.attr);
+	device_remove_file(hwmon->device, &tz->temp_input.attr);
+	if (!new_hwmon_device) {
+		device_remove_file(hwmon->device, &dev_attr_name);
+		hwmon_device_unregister(hwmon->device);
+	}
+ free_mem:
+	kfree(hwmon);
+
+	return result;
+}
+
+static void
+thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+	struct thermal_hwmon_device *hwmon = tz->hwmon;
+
+	tz->hwmon = NULL;
+	device_remove_file(hwmon->device, &tz->temp_input.attr);
+	device_remove_file(hwmon->device, &tz->temp_crit.attr);
+
+	mutex_lock(&thermal_list_lock);
+	list_del(&tz->hwmon_node);
+	if (!list_empty(&hwmon->tz_list)) {
+		mutex_unlock(&thermal_list_lock);
+		return;
+	}
+	list_del(&hwmon->node);
+	mutex_unlock(&thermal_list_lock);
+
+	device_remove_file(hwmon->device, &dev_attr_name);
+	hwmon_device_unregister(hwmon->device);
+	kfree(hwmon);
+	return;
+}
+#else
+static int
+thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+	return 0;
+}
+
+static void
+thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+	return;
+}
+#endif
+
+
 /**
  * thermal_zone_bind_cooling_device - bind a cooling device to a thermal zone
  * @tz:		thermal zone device
@@ -638,6 +797,10 @@ struct thermal_zone_device *thermal_zone
 			goto unregister;
 	}
 
+	result = thermal_add_hwmon_sysfs(tz);
+	if (result)
+		goto unregister;
+
 	mutex_lock(&thermal_list_lock);
 	list_add_tail(&tz->node, &thermal_tz_list);
 	if (ops->bind)
@@ -696,6 +859,7 @@ void thermal_zone_device_unregister(stru
 	for (count = 0; count < tz->trips; count++)
 		TRIP_POINT_ATTR_REMOVE(&tz->device, count);
 
+	thermal_remove_hwmon_sysfs(tz);
 	release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
 	idr_destroy(&tz->idr);
 	mutex_destroy(&tz->lock);
Index: linux-2.6/include/linux/thermal.h
===================================================================
--- linux-2.6.orig/include/linux/thermal.h
+++ linux-2.6/include/linux/thermal.h
@@ -66,6 +66,23 @@ struct thermal_cooling_device {
 				((long)t-2732+5)/10 : ((long)t-2732-5)/10)
 #define CELSIUS_TO_KELVIN(t)	((t)*10+2732)
 
+#if defined(CONFIG_HWMON) ||	\
+	(defined(CONFIG_HWMON_MODULE) && defined(CONFIG_THERMAL_MODULE))
+/* thermal zone devices with the same type share one hwmon device */
+struct thermal_hwmon_device {
+	char type[THERMAL_NAME_LENGTH];
+	struct device *device;
+	int count;
+	struct list_head tz_list;
+	struct list_head node;
+};
+
+struct thermal_hwmon_attr {
+	struct device_attribute attr;
+	char name[16];
+};
+#endif
+
 struct thermal_zone_device {
 	int id;
 	char type[THERMAL_NAME_LENGTH];
@@ -77,6 +94,13 @@ struct thermal_zone_device {
 	struct idr idr;
 	struct mutex lock;	/* protect cooling devices list */
 	struct list_head node;
+#if defined(CONFIG_HWMON) ||	\
+	(defined(CONFIG_HWMON_MODULE) && defined(CONFIG_THERMAL_MODULE))
+	struct list_head hwmon_node;
+	struct thermal_hwmon_device *hwmon;
+	struct thermal_hwmon_attr temp_input;	/* hwmon sys attr */
+	struct thermal_hwmon_attr temp_crit;	/* hwmon sys attr */
+#endif
 };
 
 struct thermal_zone_device *thermal_zone_device_register(char *, int, void *,

Re: [PATCH 3/6] thermal: add hwmon sysfs I/F

[Jean Delvare]

Hi Rui,

On Thu, 10 Apr 2008 16:16:00 +0800, Zhang, Rui wrote:
> 
> Add hwmon sys I/F for generic thermal driver.
> 
> Note: we have one hwmon class device for EACH TYPE of the thermal zone device.
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/thermal/thermal.c |  164 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/thermal.h   |   24 ++++++
>  2 files changed, 188 insertions(+)

Much better than the previous version but there are a few problems left:

> 
> Index: linux-2.6/drivers/thermal/thermal.c
> ===================================================================
> --- linux-2.6.orig/drivers/thermal/thermal.c
> +++ linux-2.6/drivers/thermal/thermal.c
> @@ -291,6 +291,165 @@ thermal_cooling_device_trip_point_show(s
>  
>  /* Device management */
>  
> +#if defined(CONFIG_HWMON) ||	\
> +	(defined(CONFIG_HWMON_MODULE) && defined(CONFIG_THERMAL_MODULE))
> +/* hwmon sys I/F */
> +#include <linux/hwmon.h>
> +static LIST_HEAD(thermal_hwmon_list);
> +
> +static ssize_t
> +name_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct thermal_hwmon_device *hwmon = dev->driver_data;
> +	return sprintf(buf, "%s\n", hwmon->type);
> +}
> +static DEVICE_ATTR(name, 0444, name_show, NULL);
> +
> +static ssize_t
> +temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct thermal_hwmon_attr *hwmon_attr
> +			= container_of(attr, struct thermal_hwmon_attr, attr);
> +	struct thermal_zone_device *tz
> +			= container_of(hwmon_attr, struct thermal_zone_device,
> +				       temp_input);
> +
> +	return tz->ops->get_temp(tz, buf);
> +}
> +
> +static ssize_t
> +temp_crit_show(struct device *dev, struct device_attribute *attr,
> +		char *buf)
> +{
> +	struct thermal_hwmon_attr *hwmon_attr
> +			= container_of(attr, struct thermal_hwmon_attr, attr);
> +	struct thermal_zone_device *tz
> +			= container_of(hwmon_attr, struct thermal_zone_device,
> +				       temp_crit);
> +
> +	return tz->ops->get_trip_temp(tz, 0, buf);
> +}
> +
> +
> +static int
> +thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
> +{
> +	struct thermal_hwmon_device *hwmon;
> +	int new_hwmon_device = 0;
> +	int result;
> +
> +	mutex_lock(&thermal_list_lock);
> +	list_for_each_entry(hwmon, &thermal_hwmon_list, node)
> +		if (!strcmp(hwmon->type, tz->type)) {
> +			new_hwmon_device = 1;
> +			mutex_unlock(&thermal_list_lock);
> +			goto register_sys_interface;
> +		}
> +	mutex_unlock(&thermal_list_lock);

I'm confused. new_hwmon_device is set to 1 when you do NOT create a new
hwmon device, and to 0 when you DO create a new hwmon device? Wouldn't
it make way more sense the other way around?

> +
> +	hwmon = kzalloc(sizeof(struct thermal_hwmon_device), GFP_KERNEL);
> +	if (!hwmon)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&hwmon->tz_list);
> +	strncpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH);

strlcpy please. strncpy is broken by design and should never be used.

> +	hwmon->device = hwmon_device_register(NULL);
> +	if (IS_ERR(hwmon->device)) {
> +		result = -EINVAL;

Better return the actual error code: PTR_ERR(hwmon->device).

> +		goto free_mem;
> +	}
> +	hwmon->device->driver_data = hwmon;
> +	result = device_create_file(hwmon->device, &dev_attr_name);
> +	if (result)
> +		goto unregister_hwmon_device;
> +
> + register_sys_interface:
> +	tz->hwmon = hwmon;
> +	hwmon->count++;
> +
> +	snprintf(tz->temp_input.name, THERMAL_NAME_LENGTH,
> +		 "temp%d_input", hwmon->count);
> +	tz->temp_input.attr.attr.name = tz->temp_input.name;
> +	tz->temp_input.attr.attr.mode = 0444;
> +	tz->temp_input.attr.show = temp_input_show;
> +	result = device_create_file(hwmon->device, &tz->temp_input.attr);
> +	if (result)
> +		goto unregister_hwmon_device;
> +
> +	if (tz->ops->get_crit_temp) {
> +		unsigned long temperature;
> +		if (!tz->ops->get_crit_temp(tz, &temperature)) {
> +			snprintf(tz->temp_crit.name, THERMAL_NAME_LENGTH,
> +				"temp%d_crit", hwmon->count);
> +			tz->temp_crit.attr.attr.name = tz->temp_crit.name;
> +			tz->temp_crit.attr.attr.mode = 0444;
> +			tz->temp_crit.attr.show = temp_crit_show;
> +			result = device_create_file(hwmon->device,
> +						    &tz->temp_crit.attr);
> +			if (result)
> +				goto unregister_hwmon_device;
> +		}
> +	}
> +
> +	mutex_lock(&thermal_list_lock);
> +	if (!new_hwmon_device)
> +		list_add_tail(&hwmon->node, &thermal_hwmon_list);
> +	list_add_tail(&tz->hwmon_node, &hwmon->tz_list);
> +	mutex_unlock(&thermal_list_lock);
> +
> +	return 0;
> +
> + unregister_hwmon_device:
> +	device_remove_file(hwmon->device, &tz->temp_crit.attr);
> +	device_remove_file(hwmon->device, &tz->temp_input.attr);
> +	if (!new_hwmon_device) {
> +		device_remove_file(hwmon->device, &dev_attr_name);
> +		hwmon_device_unregister(hwmon->device);
> +	}
> + free_mem:
> +	kfree(hwmon);

This kfree should be conditioned by the value of new_hwmon_device.

> +
> +	return result;
> +}
> +
> +static void
> +thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
> +{
> +	struct thermal_hwmon_device *hwmon = tz->hwmon;
> +
> +	tz->hwmon = NULL;
> +	device_remove_file(hwmon->device, &tz->temp_input.attr);
> +	device_remove_file(hwmon->device, &tz->temp_crit.attr);
> +
> +	mutex_lock(&thermal_list_lock);
> +	list_del(&tz->hwmon_node);
> +	if (!list_empty(&hwmon->tz_list)) {
> +		mutex_unlock(&thermal_list_lock);
> +		return;
> +	}
> +	list_del(&hwmon->node);
> +	mutex_unlock(&thermal_list_lock);
> +
> +	device_remove_file(hwmon->device, &dev_attr_name);
> +	hwmon_device_unregister(hwmon->device);
> +	kfree(hwmon);
> +	return;

Void functions need no return.

> +}
> +#else
> +static int
> +thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
> +{
> +	return 0;
> +}
> +
> +static void
> +thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
> +{
> +	return;

Idem.

> +}
> +#endif
> +
> +
>  /**
>   * thermal_zone_bind_cooling_device - bind a cooling device to a thermal zone
>   * @tz:		thermal zone device
> @@ -638,6 +797,10 @@ struct thermal_zone_device *thermal_zone
>  			goto unregister;
>  	}
>  
> +	result = thermal_add_hwmon_sysfs(tz);
> +	if (result)
> +		goto unregister;
> +
>  	mutex_lock(&thermal_list_lock);
>  	list_add_tail(&tz->node, &thermal_tz_list);
>  	if (ops->bind)
> @@ -696,6 +859,7 @@ void thermal_zone_device_unregister(stru
>  	for (count = 0; count < tz->trips; count++)
>  		TRIP_POINT_ATTR_REMOVE(&tz->device, count);
>  
> +	thermal_remove_hwmon_sysfs(tz);
>  	release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
>  	idr_destroy(&tz->idr);
>  	mutex_destroy(&tz->lock);
> Index: linux-2.6/include/linux/thermal.h
> ===================================================================
> --- linux-2.6.orig/include/linux/thermal.h
> +++ linux-2.6/include/linux/thermal.h
> @@ -66,6 +66,23 @@ struct thermal_cooling_device {
>  				((long)t-2732+5)/10 : ((long)t-2732-5)/10)
>  #define CELSIUS_TO_KELVIN(t)	((t)*10+2732)
>  
> +#if defined(CONFIG_HWMON) ||	\
> +	(defined(CONFIG_HWMON_MODULE) && defined(CONFIG_THERMAL_MODULE))
> +/* thermal zone devices with the same type share one hwmon device */
> +struct thermal_hwmon_device {
> +	char type[THERMAL_NAME_LENGTH];
> +	struct device *device;
> +	int count;
> +	struct list_head tz_list;
> +	struct list_head node;
> +};
> +
> +struct thermal_hwmon_attr {
> +	struct device_attribute attr;
> +	char name[16];
> +};
> +#endif
> +
>  struct thermal_zone_device {
>  	int id;
>  	char type[THERMAL_NAME_LENGTH];
> @@ -77,6 +94,13 @@ struct thermal_zone_device {
>  	struct idr idr;
>  	struct mutex lock;	/* protect cooling devices list */
>  	struct list_head node;
> +#if defined(CONFIG_HWMON) ||	\
> +	(defined(CONFIG_HWMON_MODULE) && defined(CONFIG_THERMAL_MODULE))
> +	struct list_head hwmon_node;
> +	struct thermal_hwmon_device *hwmon;
> +	struct thermal_hwmon_attr temp_input;	/* hwmon sys attr */
> +	struct thermal_hwmon_attr temp_crit;	/* hwmon sys attr */
> +#endif
>  };
>  
>  struct thermal_zone_device *thermal_zone_device_register(char *, int, void *,
> 
> 


-- 
Jean Delvare

Re: [PATCH 3/6] thermal: add hwmon sysfs I/F

[Zhang Rui]

On Thu, 2008-04-17 at 04:45 +0800, Jean Delvare wrote:
> Hi Rui,
> 
> On Thu, 10 Apr 2008 16:16:00 +0800, Zhang, Rui wrote:
> >
> > Add hwmon sys I/F for generic thermal driver.
> >
> > Note: we have one hwmon class device for EACH TYPE of the thermal
> zone device.
> >
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  drivers/thermal/thermal.c |  164
> ++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/thermal.h   |   24 ++++++
> >  2 files changed, 188 insertions(+)
> 
> Much better than the previous version but there are a few problems
> left:
> 
> >
> > Index: linux-2.6/drivers/thermal/thermal.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/thermal/thermal.c
> > +++ linux-2.6/drivers/thermal/thermal.c
> > @@ -291,6 +291,165 @@ thermal_cooling_device_trip_point_show(s
> > 
> >  /* Device management */
> > 
> > +#if defined(CONFIG_HWMON) || \
> > +     (defined(CONFIG_HWMON_MODULE) &&
> defined(CONFIG_THERMAL_MODULE))
> > +/* hwmon sys I/F */
> > +#include <linux/hwmon.h>
> > +static LIST_HEAD(thermal_hwmon_list);
> > +
> > +static ssize_t
> > +name_show(struct device *dev, struct device_attribute *attr, char
> *buf)
> > +{
> > +     struct thermal_hwmon_device *hwmon = dev->driver_data;
> > +     return sprintf(buf, "%s\n", hwmon->type);
> > +}
> > +static DEVICE_ATTR(name, 0444, name_show, NULL);
> > +
> > +static ssize_t
> > +temp_input_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> > +{
> > +     struct thermal_hwmon_attr *hwmon_attr
> > +                     = container_of(attr, struct
> thermal_hwmon_attr, attr);
> > +     struct thermal_zone_device *tz
> > +                     = container_of(hwmon_attr, struct
> thermal_zone_device,
> > +                                    temp_input);
> > +
> > +     return tz->ops->get_temp(tz, buf);
> > +}
> > +
> > +static ssize_t
> > +temp_crit_show(struct device *dev, struct device_attribute *attr,
> > +             char *buf)
> > +{
> > +     struct thermal_hwmon_attr *hwmon_attr
> > +                     = container_of(attr, struct
> thermal_hwmon_attr, attr);
> > +     struct thermal_zone_device *tz
> > +                     = container_of(hwmon_attr, struct
> thermal_zone_device,
> > +                                    temp_crit);
> > +
> > +     return tz->ops->get_trip_temp(tz, 0, buf);
> > +}
> > +
> > +
> > +static int
> > +thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
> > +{
> > +     struct thermal_hwmon_device *hwmon;
> > +     int new_hwmon_device = 0;
> > +     int result;
> > +
> > +     mutex_lock(&thermal_list_lock);
> > +     list_for_each_entry(hwmon, &thermal_hwmon_list, node)
> > +             if (!strcmp(hwmon->type, tz->type)) {
> > +                     new_hwmon_device = 1;
> > +                     mutex_unlock(&thermal_list_lock);
> > +                     goto register_sys_interface;
> > +             }
> > +     mutex_unlock(&thermal_list_lock);
> 
> I'm confused. new_hwmon_device is set to 1 when you do NOT create a
> new
> hwmon device, and to 0 when you DO create a new hwmon device? Wouldn't
> it make way more sense the other way around?

"hwmon_already_available" was used at the beginning.
I renamed this flag later but forget to change the logic. :)
Please review the patch below.


Add hwmon sys I/F for generic thermal driver.

Note: we have one hwmon class device for EACH TYPE of the thermal zone device.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/thermal/thermal.c |  163 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/thermal.h   |   24 ++++++
 2 files changed, 187 insertions(+)

Index: old-git/drivers/thermal/thermal.c
===================================================================
--- old-git.orig/drivers/thermal/thermal.c	2008-04-21 15:06:41.000000000 +0800
+++ old-git/drivers/thermal/thermal.c	2008-04-21 15:19:47.000000000 +0800
@@ -291,6 +291,164 @@
 
 /* Device management */
 
+#if defined(CONFIG_HWMON) ||	\
+	(defined(CONFIG_HWMON_MODULE) && defined(CONFIG_THERMAL_MODULE))
+/* hwmon sys I/F */
+#include <linux/hwmon.h>
+static LIST_HEAD(thermal_hwmon_list);
+
+static ssize_t
+name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct thermal_hwmon_device *hwmon = dev->driver_data;
+	return sprintf(buf, "%s\n", hwmon->type);
+}
+static DEVICE_ATTR(name, 0444, name_show, NULL);
+
+static ssize_t
+temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct thermal_hwmon_attr *hwmon_attr
+			= container_of(attr, struct thermal_hwmon_attr, attr);
+	struct thermal_zone_device *tz
+			= container_of(hwmon_attr, struct thermal_zone_device,
+				       temp_input);
+
+	return tz->ops->get_temp(tz, buf);
+}
+
+static ssize_t
+temp_crit_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct thermal_hwmon_attr *hwmon_attr
+			= container_of(attr, struct thermal_hwmon_attr, attr);
+	struct thermal_zone_device *tz
+			= container_of(hwmon_attr, struct thermal_zone_device,
+				       temp_crit);
+
+	return tz->ops->get_trip_temp(tz, 0, buf);
+}
+
+
+static int
+thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+	struct thermal_hwmon_device *hwmon;
+	int new_hwmon_device = 1;
+	int result;
+
+	mutex_lock(&thermal_list_lock);
+	list_for_each_entry(hwmon, &thermal_hwmon_list, node)
+		if (!strcmp(hwmon->type, tz->type)) {
+			new_hwmon_device = 0;
+			mutex_unlock(&thermal_list_lock);
+			goto register_sys_interface;
+		}
+	mutex_unlock(&thermal_list_lock);
+
+	hwmon = kzalloc(sizeof(struct thermal_hwmon_device), GFP_KERNEL);
+	if (!hwmon)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&hwmon->tz_list);
+	strlcpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH);
+	hwmon->device = hwmon_device_register(NULL);
+	if (IS_ERR(hwmon->device)) {
+		result = PTR_ERR(hwmon->device);
+		goto free_mem;
+	}
+	hwmon->device->driver_data = hwmon;
+	result = device_create_file(hwmon->device, &dev_attr_name);
+	if (result)
+		goto unregister_hwmon_device;
+
+ register_sys_interface:
+	tz->hwmon = hwmon;
+	hwmon->count++;
+
+	snprintf(tz->temp_input.name, THERMAL_NAME_LENGTH,
+		 "temp%d_input", hwmon->count);
+	tz->temp_input.attr.attr.name = tz->temp_input.name;
+	tz->temp_input.attr.attr.mode = 0444;
+	tz->temp_input.attr.show = temp_input_show;
+	result = device_create_file(hwmon->device, &tz->temp_input.attr);
+	if (result)
+		goto unregister_hwmon_device;
+
+	if (tz->ops->get_crit_temp) {
+		unsigned long temperature;
+		if (!tz->ops->get_crit_temp(tz, &temperature)) {
+			snprintf(tz->temp_crit.name, THERMAL_NAME_LENGTH,
+				"temp%d_crit", hwmon->count);
+			tz->temp_crit.attr.attr.name = tz->temp_crit.name;
+			tz->temp_crit.attr.attr.mode = 0444;
+			tz->temp_crit.attr.show = temp_crit_show;
+			result = device_create_file(hwmon->device,
+						    &tz->temp_crit.attr);
+			if (result)
+				goto unregister_hwmon_device;
+		}
+	}
+
+	mutex_lock(&thermal_list_lock);
+	if (new_hwmon_device)
+		list_add_tail(&hwmon->node, &thermal_hwmon_list);
+	list_add_tail(&tz->hwmon_node, &hwmon->tz_list);
+	mutex_unlock(&thermal_list_lock);
+
+	return 0;
+
+ unregister_hwmon_device:
+	device_remove_file(hwmon->device, &tz->temp_crit.attr);
+	device_remove_file(hwmon->device, &tz->temp_input.attr);
+	if (new_hwmon_device) {
+		device_remove_file(hwmon->device, &dev_attr_name);
+		hwmon_device_unregister(hwmon->device);
+	}
+ free_mem:
+	if (new_hwmon_device)
+		kfree(hwmon);
+
+	return result;
+}
+
+static void
+thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+	struct thermal_hwmon_device *hwmon = tz->hwmon;
+
+	tz->hwmon = NULL;
+	device_remove_file(hwmon->device, &tz->temp_input.attr);
+	device_remove_file(hwmon->device, &tz->temp_crit.attr);
+
+	mutex_lock(&thermal_list_lock);
+	list_del(&tz->hwmon_node);
+	if (!list_empty(&hwmon->tz_list)) {
+		mutex_unlock(&thermal_list_lock);
+		return;
+	}
+	list_del(&hwmon->node);
+	mutex_unlock(&thermal_list_lock);
+
+	device_remove_file(hwmon->device, &dev_attr_name);
+	hwmon_device_unregister(hwmon->device);
+	kfree(hwmon);
+}
+#else
+static int
+thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+	return 0;
+}
+
+static void
+thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+}
+#endif
+
+
 /**
  * thermal_zone_bind_cooling_device - bind a cooling device to a thermal zone
  * @tz:		thermal zone device
@@ -638,6 +796,10 @@
 			goto unregister;
 	}
 
+	result = thermal_add_hwmon_sysfs(tz);
+	if (result)
+		goto unregister;
+
 	mutex_lock(&thermal_list_lock);
 	list_add_tail(&tz->node, &thermal_tz_list);
 	if (ops->bind)
@@ -696,6 +858,7 @@
 	for (count = 0; count < tz->trips; count++)
 		TRIP_POINT_ATTR_REMOVE(&tz->device, count);
 
+	thermal_remove_hwmon_sysfs(tz);
 	release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
 	idr_destroy(&tz->idr);
 	mutex_destroy(&tz->lock);
Index: old-git/include/linux/thermal.h
===================================================================
--- old-git.orig/include/linux/thermal.h	2008-04-21 15:09:30.000000000 +0800
+++ old-git/include/linux/thermal.h	2008-04-21 15:09:34.000000000 +0800
@@ -66,6 +66,23 @@
 				((long)t-2732+5)/10 : ((long)t-2732-5)/10)
 #define CELSIUS_TO_KELVIN(t)	((t)*10+2732)
 
+#if defined(CONFIG_HWMON) ||	\
+	(defined(CONFIG_HWMON_MODULE) && defined(CONFIG_THERMAL_MODULE))
+/* thermal zone devices with the same type share one hwmon device */
+struct thermal_hwmon_device {
+	char type[THERMAL_NAME_LENGTH];
+	struct device *device;
+	int count;
+	struct list_head tz_list;
+	struct list_head node;
+};
+
+struct thermal_hwmon_attr {
+	struct device_attribute attr;
+	char name[16];
+};
+#endif
+
 struct thermal_zone_device {
 	int id;
 	char type[THERMAL_NAME_LENGTH];
@@ -77,6 +94,13 @@
 	struct idr idr;
 	struct mutex lock;	/* protect cooling devices list */
 	struct list_head node;
+#if defined(CONFIG_HWMON) ||	\
+	(defined(CONFIG_HWMON_MODULE) && defined(CONFIG_THERMAL_MODULE))
+	struct list_head hwmon_node;
+	struct thermal_hwmon_device *hwmon;
+	struct thermal_hwmon_attr temp_input;	/* hwmon sys attr */
+	struct thermal_hwmon_attr temp_crit;	/* hwmon sys attr */
+#endif
 };
 
 struct thermal_zone_device *thermal_zone_device_register(char *, int, void *,

Re: [PATCH 3/6] thermal: add hwmon sysfs I/F

[Jean Delvare]

On Mon, 21 Apr 2008 16:07:52 +0800, Zhang Rui wrote:
> "hwmon_already_available" was used at the beginning.
> I renamed this flag later but forget to change the logic. :)
> Please review the patch below.
> 
> 
> Add hwmon sys I/F for generic thermal driver.
> 
> Note: we have one hwmon class device for EACH TYPE of the thermal zone device.
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/thermal/thermal.c |  163 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/thermal.h   |   24 ++++++
>  2 files changed, 187 insertions(+)

Acked-by: Jean Delvare <khali@linux-fr.org>

-- 
Jean Delvare

Re: [PATCH 3/6] thermal: add hwmon sysfs I/F

[Len Brown]

On Monday 21 April 2008, Jean Delvare wrote:
> Acked-by: Jean Delvare <khali@linux-fr.org>

applied

thanks
-len