[RFC PATCH 1/4] thermal: hwmon: move hwmon support to single file

[RFC PATCH 1/4] thermal: hwmon: move hwmon support to single file

[Eduardo Valentin]

In order to improve code organization, this patch
moves the hwmon sysfs support to a file named
thermal_hwmon. This helps to add extra support
for hwmon without scrambling the code.

In order to do this move, the hwmon list head is now
using its own locking. Before, the list used
the global thermal locking.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
---
 drivers/thermal/Kconfig         |   9 ++
 drivers/thermal/Makefile        |   3 +
 drivers/thermal/thermal_core.c  | 255 +-------------------------------------
 drivers/thermal/thermal_hwmon.c | 268 ++++++++++++++++++++++++++++++++++++++++
 drivers/thermal/thermal_hwmon.h |  49 ++++++++
 5 files changed, 330 insertions(+), 254 deletions(-)
 create mode 100644 drivers/thermal/thermal_hwmon.c
 create mode 100644 drivers/thermal/thermal_hwmon.h

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index e988c81..7fb16bc 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -17,8 +17,17 @@ if THERMAL
 
 config THERMAL_HWMON
 	bool
+	prompt "Expose thermal sensors as hwmon device"
 	depends on HWMON=y || HWMON=THERMAL
 	default y
+	help
+	  In case a sensor is registered with the thermal
+	  framework, this option will also register it
+	  as a hwmon. The sensor will then have the common
+	  hwmon sysfs interface.
+
+	  Say 'Y' here if you want all thermal sensors to
+	  have hwmon sysfs interface too.
 
 choice
 	prompt "Default Thermal governor"
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 67184a2..24cb894 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -5,6 +5,9 @@
 obj-$(CONFIG_THERMAL)		+= thermal_sys.o
 thermal_sys-y			+= thermal_core.o
 
+# interface to/from other layers providing sensors
+thermal_sys-$(CONFIG_THERMAL_HWMON)		+= thermal_hwmon.o
+
 # governors
 thermal_sys-$(CONFIG_THERMAL_GOV_FAIR_SHARE)	+= fair_share.o
 thermal_sys-$(CONFIG_THERMAL_GOV_STEP_WISE)	+= step_wise.o
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 1f02e8e..247528b 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -38,6 +38,7 @@
 #include <net/genetlink.h>
 
 #include "thermal_core.h"
+#include "thermal_hwmon.h"
 
 MODULE_AUTHOR("Zhang Rui");
 MODULE_DESCRIPTION("Generic thermal management sysfs support");
@@ -859,260 +860,6 @@ thermal_cooling_device_trip_point_show(struct device *dev,
 
 /* Device management */
 
-#if defined(CONFIG_THERMAL_HWMON)
-
-/* hwmon sys I/F */
-#include <linux/hwmon.h>
-
-/* 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];
-};
-
-/* one temperature input for each thermal zone */
-struct thermal_hwmon_temp {
-	struct list_head hwmon_node;
-	struct thermal_zone_device *tz;
-	struct thermal_hwmon_attr temp_input;	/* hwmon sys attr */
-	struct thermal_hwmon_attr temp_crit;	/* hwmon sys attr */
-};
-
-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_get_drvdata(dev);
-	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)
-{
-	long temperature;
-	int ret;
-	struct thermal_hwmon_attr *hwmon_attr
-			= container_of(attr, struct thermal_hwmon_attr, attr);
-	struct thermal_hwmon_temp *temp
-			= container_of(hwmon_attr, struct thermal_hwmon_temp,
-				       temp_input);
-	struct thermal_zone_device *tz = temp->tz;
-
-	ret = thermal_zone_get_temp(tz, &temperature);
-
-	if (ret)
-		return ret;
-
-	return sprintf(buf, "%ld\n", temperature);
-}
-
-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_hwmon_temp *temp
-			= container_of(hwmon_attr, struct thermal_hwmon_temp,
-				       temp_crit);
-	struct thermal_zone_device *tz = temp->tz;
-	long temperature;
-	int ret;
-
-	ret = tz->ops->get_trip_temp(tz, 0, &temperature);
-	if (ret)
-		return ret;
-
-	return sprintf(buf, "%ld\n", temperature);
-}
-
-
-static struct thermal_hwmon_device *
-thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
-{
-	struct thermal_hwmon_device *hwmon;
-
-	mutex_lock(&thermal_list_lock);
-	list_for_each_entry(hwmon, &thermal_hwmon_list, node)
-		if (!strcmp(hwmon->type, tz->type)) {
-			mutex_unlock(&thermal_list_lock);
-			return hwmon;
-		}
-	mutex_unlock(&thermal_list_lock);
-
-	return NULL;
-}
-
-/* Find the temperature input matching a given thermal zone */
-static struct thermal_hwmon_temp *
-thermal_hwmon_lookup_temp(const struct thermal_hwmon_device *hwmon,
-			  const struct thermal_zone_device *tz)
-{
-	struct thermal_hwmon_temp *temp;
-
-	mutex_lock(&thermal_list_lock);
-	list_for_each_entry(temp, &hwmon->tz_list, hwmon_node)
-		if (temp->tz == tz) {
-			mutex_unlock(&thermal_list_lock);
-			return temp;
-		}
-	mutex_unlock(&thermal_list_lock);
-
-	return NULL;
-}
-
-static int
-thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
-{
-	struct thermal_hwmon_device *hwmon;
-	struct thermal_hwmon_temp *temp;
-	int new_hwmon_device = 1;
-	int result;
-
-	hwmon = thermal_hwmon_lookup_by_type(tz);
-	if (hwmon) {
-		new_hwmon_device = 0;
-		goto register_sys_interface;
-	}
-
-	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;
-	}
-	dev_set_drvdata(hwmon->device, hwmon);
-	result = device_create_file(hwmon->device, &dev_attr_name);
-	if (result)
-		goto free_mem;
-
- register_sys_interface:
-	temp = kzalloc(sizeof(struct thermal_hwmon_temp), GFP_KERNEL);
-	if (!temp) {
-		result = -ENOMEM;
-		goto unregister_name;
-	}
-
-	temp->tz = tz;
-	hwmon->count++;
-
-	snprintf(temp->temp_input.name, sizeof(temp->temp_input.name),
-		 "temp%d_input", hwmon->count);
-	temp->temp_input.attr.attr.name = temp->temp_input.name;
-	temp->temp_input.attr.attr.mode = 0444;
-	temp->temp_input.attr.show = temp_input_show;
-	sysfs_attr_init(&temp->temp_input.attr.attr);
-	result = device_create_file(hwmon->device, &temp->temp_input.attr);
-	if (result)
-		goto free_temp_mem;
-
-	if (tz->ops->get_crit_temp) {
-		unsigned long temperature;
-		if (!tz->ops->get_crit_temp(tz, &temperature)) {
-			snprintf(temp->temp_crit.name,
-				 sizeof(temp->temp_crit.name),
-				"temp%d_crit", hwmon->count);
-			temp->temp_crit.attr.attr.name = temp->temp_crit.name;
-			temp->temp_crit.attr.attr.mode = 0444;
-			temp->temp_crit.attr.show = temp_crit_show;
-			sysfs_attr_init(&temp->temp_crit.attr.attr);
-			result = device_create_file(hwmon->device,
-						    &temp->temp_crit.attr);
-			if (result)
-				goto unregister_input;
-		}
-	}
-
-	mutex_lock(&thermal_list_lock);
-	if (new_hwmon_device)
-		list_add_tail(&hwmon->node, &thermal_hwmon_list);
-	list_add_tail(&temp->hwmon_node, &hwmon->tz_list);
-	mutex_unlock(&thermal_list_lock);
-
-	return 0;
-
- unregister_input:
-	device_remove_file(hwmon->device, &temp->temp_input.attr);
- free_temp_mem:
-	kfree(temp);
- unregister_name:
-	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;
-	struct thermal_hwmon_temp *temp;
-
-	hwmon = thermal_hwmon_lookup_by_type(tz);
-	if (unlikely(!hwmon)) {
-		/* Should never happen... */
-		dev_dbg(&tz->device, "hwmon device lookup failed!\n");
-		return;
-	}
-
-	temp = thermal_hwmon_lookup_temp(hwmon, tz);
-	if (unlikely(!temp)) {
-		/* Should never happen... */
-		dev_dbg(&tz->device, "temperature input lookup failed!\n");
-		return;
-	}
-
-	device_remove_file(hwmon->device, &temp->temp_input.attr);
-	if (tz->ops->get_crit_temp)
-		device_remove_file(hwmon->device, &temp->temp_crit.attr);
-
-	mutex_lock(&thermal_list_lock);
-	list_del(&temp->hwmon_node);
-	kfree(temp);
-	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:		pointer to struct thermal_zone_device
diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
new file mode 100644
index 0000000..7c665c8
--- /dev/null
+++ b/drivers/thermal/thermal_hwmon.c
@@ -0,0 +1,268 @@
+/*
+ *  thermal_hwmon.c - Generic Thermal Management hwmon support.
+ *
+ *  Code based on Intel thermal_core.c. Copyrights of the original code:
+ *  Copyright (C) 2008 Intel Corp
+ *  Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com>
+ *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com>
+ *
+ *  Copyright (C) 2013 Texas Instruments
+ *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
+ *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+#include <linux/hwmon.h>
+#include <linux/thermal.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+
+/* hwmon sys I/F */
+/* 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];
+};
+
+/* one temperature input for each thermal zone */
+struct thermal_hwmon_temp {
+	struct list_head hwmon_node;
+	struct thermal_zone_device *tz;
+	struct thermal_hwmon_attr temp_input;	/* hwmon sys attr */
+	struct thermal_hwmon_attr temp_crit;	/* hwmon sys attr */
+};
+
+static LIST_HEAD(thermal_hwmon_list);
+
+static DEFINE_MUTEX(thermal_hwmon_list_lock);
+
+static ssize_t
+name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct thermal_hwmon_device *hwmon = dev_get_drvdata(dev);
+	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)
+{
+	long temperature;
+	int ret;
+	struct thermal_hwmon_attr *hwmon_attr
+			= container_of(attr, struct thermal_hwmon_attr, attr);
+	struct thermal_hwmon_temp *temp
+			= container_of(hwmon_attr, struct thermal_hwmon_temp,
+				       temp_input);
+	struct thermal_zone_device *tz = temp->tz;
+
+	ret = thermal_zone_get_temp(tz, &temperature);
+
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%ld\n", temperature);
+}
+
+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_hwmon_temp *temp
+			= container_of(hwmon_attr, struct thermal_hwmon_temp,
+				       temp_crit);
+	struct thermal_zone_device *tz = temp->tz;
+	long temperature;
+	int ret;
+
+	ret = tz->ops->get_trip_temp(tz, 0, &temperature);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%ld\n", temperature);
+}
+
+
+static struct thermal_hwmon_device *
+thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
+{
+	struct thermal_hwmon_device *hwmon;
+
+	mutex_lock(&thermal_hwmon_list_lock);
+	list_for_each_entry(hwmon, &thermal_hwmon_list, node)
+		if (!strcmp(hwmon->type, tz->type)) {
+			mutex_unlock(&thermal_hwmon_list_lock);
+			return hwmon;
+		}
+	mutex_unlock(&thermal_hwmon_list_lock);
+
+	return NULL;
+}
+
+/* Find the temperature input matching a given thermal zone */
+static struct thermal_hwmon_temp *
+thermal_hwmon_lookup_temp(const struct thermal_hwmon_device *hwmon,
+			  const struct thermal_zone_device *tz)
+{
+	struct thermal_hwmon_temp *temp;
+
+	mutex_lock(&thermal_hwmon_list_lock);
+	list_for_each_entry(temp, &hwmon->tz_list, hwmon_node)
+		if (temp->tz == tz) {
+			mutex_unlock(&thermal_hwmon_list_lock);
+			return temp;
+		}
+	mutex_unlock(&thermal_hwmon_list_lock);
+
+	return NULL;
+}
+
+int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+	struct thermal_hwmon_device *hwmon;
+	struct thermal_hwmon_temp *temp;
+	int new_hwmon_device = 1;
+	int result;
+
+	hwmon = thermal_hwmon_lookup_by_type(tz);
+	if (hwmon) {
+		new_hwmon_device = 0;
+		goto register_sys_interface;
+	}
+
+	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;
+	}
+	dev_set_drvdata(hwmon->device, hwmon);
+	result = device_create_file(hwmon->device, &dev_attr_name);
+	if (result)
+		goto free_mem;
+
+ register_sys_interface:
+	temp = kzalloc(sizeof(struct thermal_hwmon_temp), GFP_KERNEL);
+	if (!temp) {
+		result = -ENOMEM;
+		goto unregister_name;
+	}
+
+	temp->tz = tz;
+	hwmon->count++;
+
+	snprintf(temp->temp_input.name, sizeof(temp->temp_input.name),
+		 "temp%d_input", hwmon->count);
+	temp->temp_input.attr.attr.name = temp->temp_input.name;
+	temp->temp_input.attr.attr.mode = 0444;
+	temp->temp_input.attr.show = temp_input_show;
+	sysfs_attr_init(&temp->temp_input.attr.attr);
+	result = device_create_file(hwmon->device, &temp->temp_input.attr);
+	if (result)
+		goto free_temp_mem;
+
+	if (tz->ops->get_crit_temp) {
+		unsigned long temperature;
+		if (!tz->ops->get_crit_temp(tz, &temperature)) {
+			snprintf(temp->temp_crit.name,
+				 sizeof(temp->temp_crit.name),
+				"temp%d_crit", hwmon->count);
+			temp->temp_crit.attr.attr.name = temp->temp_crit.name;
+			temp->temp_crit.attr.attr.mode = 0444;
+			temp->temp_crit.attr.show = temp_crit_show;
+			sysfs_attr_init(&temp->temp_crit.attr.attr);
+			result = device_create_file(hwmon->device,
+						    &temp->temp_crit.attr);
+			if (result)
+				goto unregister_input;
+		}
+	}
+
+	mutex_lock(&thermal_hwmon_list_lock);
+	if (new_hwmon_device)
+		list_add_tail(&hwmon->node, &thermal_hwmon_list);
+	list_add_tail(&temp->hwmon_node, &hwmon->tz_list);
+	mutex_unlock(&thermal_hwmon_list_lock);
+
+	return 0;
+
+ unregister_input:
+	device_remove_file(hwmon->device, &temp->temp_input.attr);
+ free_temp_mem:
+	kfree(temp);
+ unregister_name:
+	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;
+}
+
+void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+	struct thermal_hwmon_device *hwmon;
+	struct thermal_hwmon_temp *temp;
+
+	hwmon = thermal_hwmon_lookup_by_type(tz);
+	if (unlikely(!hwmon)) {
+		/* Should never happen... */
+		dev_dbg(&tz->device, "hwmon device lookup failed!\n");
+		return;
+	}
+
+	temp = thermal_hwmon_lookup_temp(hwmon, tz);
+	if (unlikely(!temp)) {
+		/* Should never happen... */
+		dev_dbg(&tz->device, "temperature input lookup failed!\n");
+		return;
+	}
+
+	device_remove_file(hwmon->device, &temp->temp_input.attr);
+	if (tz->ops->get_crit_temp)
+		device_remove_file(hwmon->device, &temp->temp_crit.attr);
+
+	mutex_lock(&thermal_hwmon_list_lock);
+	list_del(&temp->hwmon_node);
+	kfree(temp);
+	if (!list_empty(&hwmon->tz_list)) {
+		mutex_unlock(&thermal_hwmon_list_lock);
+		return;
+	}
+	list_del(&hwmon->node);
+	mutex_unlock(&thermal_hwmon_list_lock);
+
+	device_remove_file(hwmon->device, &dev_attr_name);
+	hwmon_device_unregister(hwmon->device);
+	kfree(hwmon);
+}
diff --git a/drivers/thermal/thermal_hwmon.h b/drivers/thermal/thermal_hwmon.h
new file mode 100644
index 0000000..c798fdb
--- /dev/null
+++ b/drivers/thermal/thermal_hwmon.h
@@ -0,0 +1,49 @@
+/*
+ *  thermal_hwmon.h - Generic Thermal Management hwmon support.
+ *
+ *  Code based on Intel thermal_core.c. Copyrights of the original code:
+ *  Copyright (C) 2008 Intel Corp
+ *  Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com>
+ *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com>
+ *
+ *  Copyright (C) 2013 Texas Instruments
+ *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
+ *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+#ifndef __THERMAL_HWMON_H__
+#define __THERMAL_HWMON_H__
+
+#include <linux/thermal.h>
+
+#ifdef CONFIG_THERMAL_HWMON
+int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
+void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz);
+#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
+
+#endif /* __THERMAL_HWMON_H__ */
-- 
1.8.2.1.342.gfa7285d

RE: [RFC PATCH 1/4] thermal: hwmon: move hwmon support to single file

[R, Durgadoss]

Hi Eduardo,

> -----Original Message-----
> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> owner@vger.kernel.org] On Behalf Of Eduardo Valentin
> Sent: Tuesday, July 09, 2013 7:30 PM
> To: linux-pm@vger.kernel.org; R, Durgadoss; amit.daniel@samsung.com
> Cc: Zhang, Rui; Eduardo Valentin; linux-kernel@vger.kernel.org
> Subject: [RFC PATCH 1/4] thermal: hwmon: move hwmon support to single file
> 
> In order to improve code organization, this patch
> moves the hwmon sysfs support to a file named
> thermal_hwmon. This helps to add extra support
> for hwmon without scrambling the code.

Nice clean up for thermal_core.c. +1 from me.

I am inclined to even remove the hwmon related code completely.
AFAIK, there are not many users of this config option.

However people are looking for the other option. If they have a
hwmon driver, how can it use the thermal framework with ease.
[like what you mentioned about this in 0/5]

Thanks,
Durga

> 
> In order to do this move, the hwmon list head is now
> using its own locking. Before, the list used
> the global thermal locking.
> 
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> ---
>  drivers/thermal/Kconfig         |   9 ++
>  drivers/thermal/Makefile        |   3 +
>  drivers/thermal/thermal_core.c  | 255 +-------------------------------------
>  drivers/thermal/thermal_hwmon.c | 268
> ++++++++++++++++++++++++++++++++++++++++
>  drivers/thermal/thermal_hwmon.h |  49 ++++++++
>  5 files changed, 330 insertions(+), 254 deletions(-)
>  create mode 100644 drivers/thermal/thermal_hwmon.c
>  create mode 100644 drivers/thermal/thermal_hwmon.h
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index e988c81..7fb16bc 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -17,8 +17,17 @@ if THERMAL
> 
>  config THERMAL_HWMON
>  	bool
> +	prompt "Expose thermal sensors as hwmon device"
>  	depends on HWMON=y || HWMON=THERMAL
>  	default y
> +	help
> +	  In case a sensor is registered with the thermal
> +	  framework, this option will also register it
> +	  as a hwmon. The sensor will then have the common
> +	  hwmon sysfs interface.
> +
> +	  Say 'Y' here if you want all thermal sensors to
> +	  have hwmon sysfs interface too.
> 
>  choice
>  	prompt "Default Thermal governor"
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 67184a2..24cb894 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -5,6 +5,9 @@
>  obj-$(CONFIG_THERMAL)		+= thermal_sys.o
>  thermal_sys-y			+= thermal_core.o
> 
> +# interface to/from other layers providing sensors
> +thermal_sys-$(CONFIG_THERMAL_HWMON)		+= thermal_hwmon.o
> +
>  # governors
>  thermal_sys-$(CONFIG_THERMAL_GOV_FAIR_SHARE)	+= fair_share.o
>  thermal_sys-$(CONFIG_THERMAL_GOV_STEP_WISE)	+= step_wise.o
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 1f02e8e..247528b 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -38,6 +38,7 @@
>  #include <net/genetlink.h>
> 
>  #include "thermal_core.h"
> +#include "thermal_hwmon.h"
> 
>  MODULE_AUTHOR("Zhang Rui");
>  MODULE_DESCRIPTION("Generic thermal management sysfs support");
> @@ -859,260 +860,6 @@ thermal_cooling_device_trip_point_show(struct device
> *dev,
> 
>  /* Device management */
> 
> -#if defined(CONFIG_THERMAL_HWMON)
> -
> -/* hwmon sys I/F */
> -#include <linux/hwmon.h>
> -
> -/* 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];
> -};
> -
> -/* one temperature input for each thermal zone */
> -struct thermal_hwmon_temp {
> -	struct list_head hwmon_node;
> -	struct thermal_zone_device *tz;
> -	struct thermal_hwmon_attr temp_input;	/* hwmon sys attr */
> -	struct thermal_hwmon_attr temp_crit;	/* hwmon sys attr */
> -};
> -
> -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_get_drvdata(dev);
> -	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)
> -{
> -	long temperature;
> -	int ret;
> -	struct thermal_hwmon_attr *hwmon_attr
> -			= container_of(attr, struct thermal_hwmon_attr, attr);
> -	struct thermal_hwmon_temp *temp
> -			= container_of(hwmon_attr, struct
> thermal_hwmon_temp,
> -				       temp_input);
> -	struct thermal_zone_device *tz = temp->tz;
> -
> -	ret = thermal_zone_get_temp(tz, &temperature);
> -
> -	if (ret)
> -		return ret;
> -
> -	return sprintf(buf, "%ld\n", temperature);
> -}
> -
> -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_hwmon_temp *temp
> -			= container_of(hwmon_attr, struct
> thermal_hwmon_temp,
> -				       temp_crit);
> -	struct thermal_zone_device *tz = temp->tz;
> -	long temperature;
> -	int ret;
> -
> -	ret = tz->ops->get_trip_temp(tz, 0, &temperature);
> -	if (ret)
> -		return ret;
> -
> -	return sprintf(buf, "%ld\n", temperature);
> -}
> -
> -
> -static struct thermal_hwmon_device *
> -thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
> -{
> -	struct thermal_hwmon_device *hwmon;
> -
> -	mutex_lock(&thermal_list_lock);
> -	list_for_each_entry(hwmon, &thermal_hwmon_list, node)
> -		if (!strcmp(hwmon->type, tz->type)) {
> -			mutex_unlock(&thermal_list_lock);
> -			return hwmon;
> -		}
> -	mutex_unlock(&thermal_list_lock);
> -
> -	return NULL;
> -}
> -
> -/* Find the temperature input matching a given thermal zone */
> -static struct thermal_hwmon_temp *
> -thermal_hwmon_lookup_temp(const struct thermal_hwmon_device *hwmon,
> -			  const struct thermal_zone_device *tz)
> -{
> -	struct thermal_hwmon_temp *temp;
> -
> -	mutex_lock(&thermal_list_lock);
> -	list_for_each_entry(temp, &hwmon->tz_list, hwmon_node)
> -		if (temp->tz == tz) {
> -			mutex_unlock(&thermal_list_lock);
> -			return temp;
> -		}
> -	mutex_unlock(&thermal_list_lock);
> -
> -	return NULL;
> -}
> -
> -static int
> -thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
> -{
> -	struct thermal_hwmon_device *hwmon;
> -	struct thermal_hwmon_temp *temp;
> -	int new_hwmon_device = 1;
> -	int result;
> -
> -	hwmon = thermal_hwmon_lookup_by_type(tz);
> -	if (hwmon) {
> -		new_hwmon_device = 0;
> -		goto register_sys_interface;
> -	}
> -
> -	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;
> -	}
> -	dev_set_drvdata(hwmon->device, hwmon);
> -	result = device_create_file(hwmon->device, &dev_attr_name);
> -	if (result)
> -		goto free_mem;
> -
> - register_sys_interface:
> -	temp = kzalloc(sizeof(struct thermal_hwmon_temp), GFP_KERNEL);
> -	if (!temp) {
> -		result = -ENOMEM;
> -		goto unregister_name;
> -	}
> -
> -	temp->tz = tz;
> -	hwmon->count++;
> -
> -	snprintf(temp->temp_input.name, sizeof(temp->temp_input.name),
> -		 "temp%d_input", hwmon->count);
> -	temp->temp_input.attr.attr.name = temp->temp_input.name;
> -	temp->temp_input.attr.attr.mode = 0444;
> -	temp->temp_input.attr.show = temp_input_show;
> -	sysfs_attr_init(&temp->temp_input.attr.attr);
> -	result = device_create_file(hwmon->device, &temp->temp_input.attr);
> -	if (result)
> -		goto free_temp_mem;
> -
> -	if (tz->ops->get_crit_temp) {
> -		unsigned long temperature;
> -		if (!tz->ops->get_crit_temp(tz, &temperature)) {
> -			snprintf(temp->temp_crit.name,
> -				 sizeof(temp->temp_crit.name),
> -				"temp%d_crit", hwmon->count);
> -			temp->temp_crit.attr.attr.name = temp-
> >temp_crit.name;
> -			temp->temp_crit.attr.attr.mode = 0444;
> -			temp->temp_crit.attr.show = temp_crit_show;
> -			sysfs_attr_init(&temp->temp_crit.attr.attr);
> -			result = device_create_file(hwmon->device,
> -						    &temp->temp_crit.attr);
> -			if (result)
> -				goto unregister_input;
> -		}
> -	}
> -
> -	mutex_lock(&thermal_list_lock);
> -	if (new_hwmon_device)
> -		list_add_tail(&hwmon->node, &thermal_hwmon_list);
> -	list_add_tail(&temp->hwmon_node, &hwmon->tz_list);
> -	mutex_unlock(&thermal_list_lock);
> -
> -	return 0;
> -
> - unregister_input:
> -	device_remove_file(hwmon->device, &temp->temp_input.attr);
> - free_temp_mem:
> -	kfree(temp);
> - unregister_name:
> -	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;
> -	struct thermal_hwmon_temp *temp;
> -
> -	hwmon = thermal_hwmon_lookup_by_type(tz);
> -	if (unlikely(!hwmon)) {
> -		/* Should never happen... */
> -		dev_dbg(&tz->device, "hwmon device lookup failed!\n");
> -		return;
> -	}
> -
> -	temp = thermal_hwmon_lookup_temp(hwmon, tz);
> -	if (unlikely(!temp)) {
> -		/* Should never happen... */
> -		dev_dbg(&tz->device, "temperature input lookup failed!\n");
> -		return;
> -	}
> -
> -	device_remove_file(hwmon->device, &temp->temp_input.attr);
> -	if (tz->ops->get_crit_temp)
> -		device_remove_file(hwmon->device, &temp->temp_crit.attr);
> -
> -	mutex_lock(&thermal_list_lock);
> -	list_del(&temp->hwmon_node);
> -	kfree(temp);
> -	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:		pointer to struct thermal_zone_device
> diff --git a/drivers/thermal/thermal_hwmon.c
> b/drivers/thermal/thermal_hwmon.c
> new file mode 100644
> index 0000000..7c665c8
> --- /dev/null
> +++ b/drivers/thermal/thermal_hwmon.c
> @@ -0,0 +1,268 @@
> +/*
> + *  thermal_hwmon.c - Generic Thermal Management hwmon support.
> + *
> + *  Code based on Intel thermal_core.c. Copyrights of the original code:
> + *  Copyright (C) 2008 Intel Corp
> + *  Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com>
> + *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com>
> + *
> + *  Copyright (C) 2013 Texas Instruments
> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
> + *
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; version 2 of the License.
> + *
> + *  This program is distributed in the hope that it will be useful, but
> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> + *
> + *
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~
> + */
> +#include <linux/hwmon.h>
> +#include <linux/thermal.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +
> +/* hwmon sys I/F */
> +/* 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];
> +};
> +
> +/* one temperature input for each thermal zone */
> +struct thermal_hwmon_temp {
> +	struct list_head hwmon_node;
> +	struct thermal_zone_device *tz;
> +	struct thermal_hwmon_attr temp_input;	/* hwmon sys attr */
> +	struct thermal_hwmon_attr temp_crit;	/* hwmon sys attr */
> +};
> +
> +static LIST_HEAD(thermal_hwmon_list);
> +
> +static DEFINE_MUTEX(thermal_hwmon_list_lock);
> +
> +static ssize_t
> +name_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct thermal_hwmon_device *hwmon = dev_get_drvdata(dev);
> +	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)
> +{
> +	long temperature;
> +	int ret;
> +	struct thermal_hwmon_attr *hwmon_attr
> +			= container_of(attr, struct thermal_hwmon_attr, attr);
> +	struct thermal_hwmon_temp *temp
> +			= container_of(hwmon_attr, struct
> thermal_hwmon_temp,
> +				       temp_input);
> +	struct thermal_zone_device *tz = temp->tz;
> +
> +	ret = thermal_zone_get_temp(tz, &temperature);
> +
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%ld\n", temperature);
> +}
> +
> +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_hwmon_temp *temp
> +			= container_of(hwmon_attr, struct
> thermal_hwmon_temp,
> +				       temp_crit);
> +	struct thermal_zone_device *tz = temp->tz;
> +	long temperature;
> +	int ret;
> +
> +	ret = tz->ops->get_trip_temp(tz, 0, &temperature);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%ld\n", temperature);
> +}
> +
> +
> +static struct thermal_hwmon_device *
> +thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
> +{
> +	struct thermal_hwmon_device *hwmon;
> +
> +	mutex_lock(&thermal_hwmon_list_lock);
> +	list_for_each_entry(hwmon, &thermal_hwmon_list, node)
> +		if (!strcmp(hwmon->type, tz->type)) {
> +			mutex_unlock(&thermal_hwmon_list_lock);
> +			return hwmon;
> +		}
> +	mutex_unlock(&thermal_hwmon_list_lock);
> +
> +	return NULL;
> +}
> +
> +/* Find the temperature input matching a given thermal zone */
> +static struct thermal_hwmon_temp *
> +thermal_hwmon_lookup_temp(const struct thermal_hwmon_device *hwmon,
> +			  const struct thermal_zone_device *tz)
> +{
> +	struct thermal_hwmon_temp *temp;
> +
> +	mutex_lock(&thermal_hwmon_list_lock);
> +	list_for_each_entry(temp, &hwmon->tz_list, hwmon_node)
> +		if (temp->tz == tz) {
> +			mutex_unlock(&thermal_hwmon_list_lock);
> +			return temp;
> +		}
> +	mutex_unlock(&thermal_hwmon_list_lock);
> +
> +	return NULL;
> +}
> +
> +int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
> +{
> +	struct thermal_hwmon_device *hwmon;
> +	struct thermal_hwmon_temp *temp;
> +	int new_hwmon_device = 1;
> +	int result;
> +
> +	hwmon = thermal_hwmon_lookup_by_type(tz);
> +	if (hwmon) {
> +		new_hwmon_device = 0;
> +		goto register_sys_interface;
> +	}
> +
> +	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;
> +	}
> +	dev_set_drvdata(hwmon->device, hwmon);
> +	result = device_create_file(hwmon->device, &dev_attr_name);
> +	if (result)
> +		goto free_mem;
> +
> + register_sys_interface:
> +	temp = kzalloc(sizeof(struct thermal_hwmon_temp), GFP_KERNEL);
> +	if (!temp) {
> +		result = -ENOMEM;
> +		goto unregister_name;
> +	}
> +
> +	temp->tz = tz;
> +	hwmon->count++;
> +
> +	snprintf(temp->temp_input.name, sizeof(temp->temp_input.name),
> +		 "temp%d_input", hwmon->count);
> +	temp->temp_input.attr.attr.name = temp->temp_input.name;
> +	temp->temp_input.attr.attr.mode = 0444;
> +	temp->temp_input.attr.show = temp_input_show;
> +	sysfs_attr_init(&temp->temp_input.attr.attr);
> +	result = device_create_file(hwmon->device, &temp->temp_input.attr);
> +	if (result)
> +		goto free_temp_mem;
> +
> +	if (tz->ops->get_crit_temp) {
> +		unsigned long temperature;
> +		if (!tz->ops->get_crit_temp(tz, &temperature)) {
> +			snprintf(temp->temp_crit.name,
> +				 sizeof(temp->temp_crit.name),
> +				"temp%d_crit", hwmon->count);
> +			temp->temp_crit.attr.attr.name = temp-
> >temp_crit.name;
> +			temp->temp_crit.attr.attr.mode = 0444;
> +			temp->temp_crit.attr.show = temp_crit_show;
> +			sysfs_attr_init(&temp->temp_crit.attr.attr);
> +			result = device_create_file(hwmon->device,
> +						    &temp->temp_crit.attr);
> +			if (result)
> +				goto unregister_input;
> +		}
> +	}
> +
> +	mutex_lock(&thermal_hwmon_list_lock);
> +	if (new_hwmon_device)
> +		list_add_tail(&hwmon->node, &thermal_hwmon_list);
> +	list_add_tail(&temp->hwmon_node, &hwmon->tz_list);
> +	mutex_unlock(&thermal_hwmon_list_lock);
> +
> +	return 0;
> +
> + unregister_input:
> +	device_remove_file(hwmon->device, &temp->temp_input.attr);
> + free_temp_mem:
> +	kfree(temp);
> + unregister_name:
> +	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;
> +}
> +
> +void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
> +{
> +	struct thermal_hwmon_device *hwmon;
> +	struct thermal_hwmon_temp *temp;
> +
> +	hwmon = thermal_hwmon_lookup_by_type(tz);
> +	if (unlikely(!hwmon)) {
> +		/* Should never happen... */
> +		dev_dbg(&tz->device, "hwmon device lookup failed!\n");
> +		return;
> +	}
> +
> +	temp = thermal_hwmon_lookup_temp(hwmon, tz);
> +	if (unlikely(!temp)) {
> +		/* Should never happen... */
> +		dev_dbg(&tz->device, "temperature input lookup failed!\n");
> +		return;
> +	}
> +
> +	device_remove_file(hwmon->device, &temp->temp_input.attr);
> +	if (tz->ops->get_crit_temp)
> +		device_remove_file(hwmon->device, &temp->temp_crit.attr);
> +
> +	mutex_lock(&thermal_hwmon_list_lock);
> +	list_del(&temp->hwmon_node);
> +	kfree(temp);
> +	if (!list_empty(&hwmon->tz_list)) {
> +		mutex_unlock(&thermal_hwmon_list_lock);
> +		return;
> +	}
> +	list_del(&hwmon->node);
> +	mutex_unlock(&thermal_hwmon_list_lock);
> +
> +	device_remove_file(hwmon->device, &dev_attr_name);
> +	hwmon_device_unregister(hwmon->device);
> +	kfree(hwmon);
> +}
> diff --git a/drivers/thermal/thermal_hwmon.h
> b/drivers/thermal/thermal_hwmon.h
> new file mode 100644
> index 0000000..c798fdb
> --- /dev/null
> +++ b/drivers/thermal/thermal_hwmon.h
> @@ -0,0 +1,49 @@
> +/*
> + *  thermal_hwmon.h - Generic Thermal Management hwmon support.
> + *
> + *  Code based on Intel thermal_core.c. Copyrights of the original code:
> + *  Copyright (C) 2008 Intel Corp
> + *  Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com>
> + *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com>
> + *
> + *  Copyright (C) 2013 Texas Instruments
> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
> + *
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; version 2 of the License.
> + *
> + *  This program is distributed in the hope that it will be useful, but
> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> + *
> + *
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~
> + */
> +#ifndef __THERMAL_HWMON_H__
> +#define __THERMAL_HWMON_H__
> +
> +#include <linux/thermal.h>
> +
> +#ifdef CONFIG_THERMAL_HWMON
> +int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
> +void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz);
> +#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
> +
> +#endif /* __THERMAL_HWMON_H__ */
> --
> 1.8.2.1.342.gfa7285d
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/4] thermal: hwmon: move hwmon support to single file

[Eduardo Valentin]

[-- Attachment #1: Type: text/plain, Size: 23710 bytes --]

On 09-07-2013 12:04, R, Durgadoss wrote:
> Hi Eduardo,
> 
>> -----Original Message-----
>> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
>> owner@vger.kernel.org] On Behalf Of Eduardo Valentin
>> Sent: Tuesday, July 09, 2013 7:30 PM
>> To: linux-pm@vger.kernel.org; R, Durgadoss; amit.daniel@samsung.com
>> Cc: Zhang, Rui; Eduardo Valentin; linux-kernel@vger.kernel.org
>> Subject: [RFC PATCH 1/4] thermal: hwmon: move hwmon support to single file
>>
>> In order to improve code organization, this patch
>> moves the hwmon sysfs support to a file named
>> thermal_hwmon. This helps to add extra support
>> for hwmon without scrambling the code.
> 
> Nice clean up for thermal_core.c. +1 from me.
> 
> I am inclined to even remove the hwmon related code completely.
> AFAIK, there are not many users of this config option.
> 

Hmm. OK. I thought of keeping it as I dont know if there are users.
Besides, if new code comes out of the hwmon2thermalfw exercise, then it
would be a good place to keep all the hwmon code.

> However people are looking for the other option. If they have a
> hwmon driver, how can it use the thermal framework with ease.
> [like what you mentioned about this in 0/5]

yes, problem is that hwmon does not have a standard way of representing
the drivers. So, one cannot simply write a simple wrapper because hwmon
drivers does not have a standard get_temperature operation, for instance.

We could either propose a way to standardize then or implement the call
to thermal fw driver by driver. Probably the later is desirable, if we
implement it in a need basis.

> 
> Thanks,
> Durga
> 
>>
>> In order to do this move, the hwmon list head is now
>> using its own locking. Before, the list used
>> the global thermal locking.
>>
>> Cc: Zhang Rui <rui.zhang@intel.com>
>> Cc: linux-pm@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
>> ---
>>  drivers/thermal/Kconfig         |   9 ++
>>  drivers/thermal/Makefile        |   3 +
>>  drivers/thermal/thermal_core.c  | 255 +-------------------------------------
>>  drivers/thermal/thermal_hwmon.c | 268
>> ++++++++++++++++++++++++++++++++++++++++
>>  drivers/thermal/thermal_hwmon.h |  49 ++++++++
>>  5 files changed, 330 insertions(+), 254 deletions(-)
>>  create mode 100644 drivers/thermal/thermal_hwmon.c
>>  create mode 100644 drivers/thermal/thermal_hwmon.h
>>
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index e988c81..7fb16bc 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -17,8 +17,17 @@ if THERMAL
>>
>>  config THERMAL_HWMON
>>  	bool
>> +	prompt "Expose thermal sensors as hwmon device"
>>  	depends on HWMON=y || HWMON=THERMAL
>>  	default y
>> +	help
>> +	  In case a sensor is registered with the thermal
>> +	  framework, this option will also register it
>> +	  as a hwmon. The sensor will then have the common
>> +	  hwmon sysfs interface.
>> +
>> +	  Say 'Y' here if you want all thermal sensors to
>> +	  have hwmon sysfs interface too.
>>
>>  choice
>>  	prompt "Default Thermal governor"
>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>> index 67184a2..24cb894 100644
>> --- a/drivers/thermal/Makefile
>> +++ b/drivers/thermal/Makefile
>> @@ -5,6 +5,9 @@
>>  obj-$(CONFIG_THERMAL)		+= thermal_sys.o
>>  thermal_sys-y			+= thermal_core.o
>>
>> +# interface to/from other layers providing sensors
>> +thermal_sys-$(CONFIG_THERMAL_HWMON)		+= thermal_hwmon.o
>> +
>>  # governors
>>  thermal_sys-$(CONFIG_THERMAL_GOV_FAIR_SHARE)	+= fair_share.o
>>  thermal_sys-$(CONFIG_THERMAL_GOV_STEP_WISE)	+= step_wise.o
>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>> index 1f02e8e..247528b 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -38,6 +38,7 @@
>>  #include <net/genetlink.h>
>>
>>  #include "thermal_core.h"
>> +#include "thermal_hwmon.h"
>>
>>  MODULE_AUTHOR("Zhang Rui");
>>  MODULE_DESCRIPTION("Generic thermal management sysfs support");
>> @@ -859,260 +860,6 @@ thermal_cooling_device_trip_point_show(struct device
>> *dev,
>>
>>  /* Device management */
>>
>> -#if defined(CONFIG_THERMAL_HWMON)
>> -
>> -/* hwmon sys I/F */
>> -#include <linux/hwmon.h>
>> -
>> -/* 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];
>> -};
>> -
>> -/* one temperature input for each thermal zone */
>> -struct thermal_hwmon_temp {
>> -	struct list_head hwmon_node;
>> -	struct thermal_zone_device *tz;
>> -	struct thermal_hwmon_attr temp_input;	/* hwmon sys attr */
>> -	struct thermal_hwmon_attr temp_crit;	/* hwmon sys attr */
>> -};
>> -
>> -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_get_drvdata(dev);
>> -	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)
>> -{
>> -	long temperature;
>> -	int ret;
>> -	struct thermal_hwmon_attr *hwmon_attr
>> -			= container_of(attr, struct thermal_hwmon_attr, attr);
>> -	struct thermal_hwmon_temp *temp
>> -			= container_of(hwmon_attr, struct
>> thermal_hwmon_temp,
>> -				       temp_input);
>> -	struct thermal_zone_device *tz = temp->tz;
>> -
>> -	ret = thermal_zone_get_temp(tz, &temperature);
>> -
>> -	if (ret)
>> -		return ret;
>> -
>> -	return sprintf(buf, "%ld\n", temperature);
>> -}
>> -
>> -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_hwmon_temp *temp
>> -			= container_of(hwmon_attr, struct
>> thermal_hwmon_temp,
>> -				       temp_crit);
>> -	struct thermal_zone_device *tz = temp->tz;
>> -	long temperature;
>> -	int ret;
>> -
>> -	ret = tz->ops->get_trip_temp(tz, 0, &temperature);
>> -	if (ret)
>> -		return ret;
>> -
>> -	return sprintf(buf, "%ld\n", temperature);
>> -}
>> -
>> -
>> -static struct thermal_hwmon_device *
>> -thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
>> -{
>> -	struct thermal_hwmon_device *hwmon;
>> -
>> -	mutex_lock(&thermal_list_lock);
>> -	list_for_each_entry(hwmon, &thermal_hwmon_list, node)
>> -		if (!strcmp(hwmon->type, tz->type)) {
>> -			mutex_unlock(&thermal_list_lock);
>> -			return hwmon;
>> -		}
>> -	mutex_unlock(&thermal_list_lock);
>> -
>> -	return NULL;
>> -}
>> -
>> -/* Find the temperature input matching a given thermal zone */
>> -static struct thermal_hwmon_temp *
>> -thermal_hwmon_lookup_temp(const struct thermal_hwmon_device *hwmon,
>> -			  const struct thermal_zone_device *tz)
>> -{
>> -	struct thermal_hwmon_temp *temp;
>> -
>> -	mutex_lock(&thermal_list_lock);
>> -	list_for_each_entry(temp, &hwmon->tz_list, hwmon_node)
>> -		if (temp->tz == tz) {
>> -			mutex_unlock(&thermal_list_lock);
>> -			return temp;
>> -		}
>> -	mutex_unlock(&thermal_list_lock);
>> -
>> -	return NULL;
>> -}
>> -
>> -static int
>> -thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
>> -{
>> -	struct thermal_hwmon_device *hwmon;
>> -	struct thermal_hwmon_temp *temp;
>> -	int new_hwmon_device = 1;
>> -	int result;
>> -
>> -	hwmon = thermal_hwmon_lookup_by_type(tz);
>> -	if (hwmon) {
>> -		new_hwmon_device = 0;
>> -		goto register_sys_interface;
>> -	}
>> -
>> -	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;
>> -	}
>> -	dev_set_drvdata(hwmon->device, hwmon);
>> -	result = device_create_file(hwmon->device, &dev_attr_name);
>> -	if (result)
>> -		goto free_mem;
>> -
>> - register_sys_interface:
>> -	temp = kzalloc(sizeof(struct thermal_hwmon_temp), GFP_KERNEL);
>> -	if (!temp) {
>> -		result = -ENOMEM;
>> -		goto unregister_name;
>> -	}
>> -
>> -	temp->tz = tz;
>> -	hwmon->count++;
>> -
>> -	snprintf(temp->temp_input.name, sizeof(temp->temp_input.name),
>> -		 "temp%d_input", hwmon->count);
>> -	temp->temp_input.attr.attr.name = temp->temp_input.name;
>> -	temp->temp_input.attr.attr.mode = 0444;
>> -	temp->temp_input.attr.show = temp_input_show;
>> -	sysfs_attr_init(&temp->temp_input.attr.attr);
>> -	result = device_create_file(hwmon->device, &temp->temp_input.attr);
>> -	if (result)
>> -		goto free_temp_mem;
>> -
>> -	if (tz->ops->get_crit_temp) {
>> -		unsigned long temperature;
>> -		if (!tz->ops->get_crit_temp(tz, &temperature)) {
>> -			snprintf(temp->temp_crit.name,
>> -				 sizeof(temp->temp_crit.name),
>> -				"temp%d_crit", hwmon->count);
>> -			temp->temp_crit.attr.attr.name = temp-
>>> temp_crit.name;
>> -			temp->temp_crit.attr.attr.mode = 0444;
>> -			temp->temp_crit.attr.show = temp_crit_show;
>> -			sysfs_attr_init(&temp->temp_crit.attr.attr);
>> -			result = device_create_file(hwmon->device,
>> -						    &temp->temp_crit.attr);
>> -			if (result)
>> -				goto unregister_input;
>> -		}
>> -	}
>> -
>> -	mutex_lock(&thermal_list_lock);
>> -	if (new_hwmon_device)
>> -		list_add_tail(&hwmon->node, &thermal_hwmon_list);
>> -	list_add_tail(&temp->hwmon_node, &hwmon->tz_list);
>> -	mutex_unlock(&thermal_list_lock);
>> -
>> -	return 0;
>> -
>> - unregister_input:
>> -	device_remove_file(hwmon->device, &temp->temp_input.attr);
>> - free_temp_mem:
>> -	kfree(temp);
>> - unregister_name:
>> -	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;
>> -	struct thermal_hwmon_temp *temp;
>> -
>> -	hwmon = thermal_hwmon_lookup_by_type(tz);
>> -	if (unlikely(!hwmon)) {
>> -		/* Should never happen... */
>> -		dev_dbg(&tz->device, "hwmon device lookup failed!\n");
>> -		return;
>> -	}
>> -
>> -	temp = thermal_hwmon_lookup_temp(hwmon, tz);
>> -	if (unlikely(!temp)) {
>> -		/* Should never happen... */
>> -		dev_dbg(&tz->device, "temperature input lookup failed!\n");
>> -		return;
>> -	}
>> -
>> -	device_remove_file(hwmon->device, &temp->temp_input.attr);
>> -	if (tz->ops->get_crit_temp)
>> -		device_remove_file(hwmon->device, &temp->temp_crit.attr);
>> -
>> -	mutex_lock(&thermal_list_lock);
>> -	list_del(&temp->hwmon_node);
>> -	kfree(temp);
>> -	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:		pointer to struct thermal_zone_device
>> diff --git a/drivers/thermal/thermal_hwmon.c
>> b/drivers/thermal/thermal_hwmon.c
>> new file mode 100644
>> index 0000000..7c665c8
>> --- /dev/null
>> +++ b/drivers/thermal/thermal_hwmon.c
>> @@ -0,0 +1,268 @@
>> +/*
>> + *  thermal_hwmon.c - Generic Thermal Management hwmon support.
>> + *
>> + *  Code based on Intel thermal_core.c. Copyrights of the original code:
>> + *  Copyright (C) 2008 Intel Corp
>> + *  Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com>
>> + *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com>
>> + *
>> + *  Copyright (C) 2013 Texas Instruments
>> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
>> + *
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ~~~~~~~~~~~~
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; version 2 of the License.
>> + *
>> + *  This program is distributed in the hope that it will be useful, but
>> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + *  General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License along
>> + *  with this program; if not, write to the Free Software Foundation, Inc.,
>> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
>> + *
>> + *
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ~~~~~~~~~~~~
>> + */
>> +#include <linux/hwmon.h>
>> +#include <linux/thermal.h>
>> +#include <linux/slab.h>
>> +#include <linux/err.h>
>> +
>> +/* hwmon sys I/F */
>> +/* 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];
>> +};
>> +
>> +/* one temperature input for each thermal zone */
>> +struct thermal_hwmon_temp {
>> +	struct list_head hwmon_node;
>> +	struct thermal_zone_device *tz;
>> +	struct thermal_hwmon_attr temp_input;	/* hwmon sys attr */
>> +	struct thermal_hwmon_attr temp_crit;	/* hwmon sys attr */
>> +};
>> +
>> +static LIST_HEAD(thermal_hwmon_list);
>> +
>> +static DEFINE_MUTEX(thermal_hwmon_list_lock);
>> +
>> +static ssize_t
>> +name_show(struct device *dev, struct device_attribute *attr, char *buf)
>> +{
>> +	struct thermal_hwmon_device *hwmon = dev_get_drvdata(dev);
>> +	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)
>> +{
>> +	long temperature;
>> +	int ret;
>> +	struct thermal_hwmon_attr *hwmon_attr
>> +			= container_of(attr, struct thermal_hwmon_attr, attr);
>> +	struct thermal_hwmon_temp *temp
>> +			= container_of(hwmon_attr, struct
>> thermal_hwmon_temp,
>> +				       temp_input);
>> +	struct thermal_zone_device *tz = temp->tz;
>> +
>> +	ret = thermal_zone_get_temp(tz, &temperature);
>> +
>> +	if (ret)
>> +		return ret;
>> +
>> +	return sprintf(buf, "%ld\n", temperature);
>> +}
>> +
>> +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_hwmon_temp *temp
>> +			= container_of(hwmon_attr, struct
>> thermal_hwmon_temp,
>> +				       temp_crit);
>> +	struct thermal_zone_device *tz = temp->tz;
>> +	long temperature;
>> +	int ret;
>> +
>> +	ret = tz->ops->get_trip_temp(tz, 0, &temperature);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return sprintf(buf, "%ld\n", temperature);
>> +}
>> +
>> +
>> +static struct thermal_hwmon_device *
>> +thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
>> +{
>> +	struct thermal_hwmon_device *hwmon;
>> +
>> +	mutex_lock(&thermal_hwmon_list_lock);
>> +	list_for_each_entry(hwmon, &thermal_hwmon_list, node)
>> +		if (!strcmp(hwmon->type, tz->type)) {
>> +			mutex_unlock(&thermal_hwmon_list_lock);
>> +			return hwmon;
>> +		}
>> +	mutex_unlock(&thermal_hwmon_list_lock);
>> +
>> +	return NULL;
>> +}
>> +
>> +/* Find the temperature input matching a given thermal zone */
>> +static struct thermal_hwmon_temp *
>> +thermal_hwmon_lookup_temp(const struct thermal_hwmon_device *hwmon,
>> +			  const struct thermal_zone_device *tz)
>> +{
>> +	struct thermal_hwmon_temp *temp;
>> +
>> +	mutex_lock(&thermal_hwmon_list_lock);
>> +	list_for_each_entry(temp, &hwmon->tz_list, hwmon_node)
>> +		if (temp->tz == tz) {
>> +			mutex_unlock(&thermal_hwmon_list_lock);
>> +			return temp;
>> +		}
>> +	mutex_unlock(&thermal_hwmon_list_lock);
>> +
>> +	return NULL;
>> +}
>> +
>> +int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
>> +{
>> +	struct thermal_hwmon_device *hwmon;
>> +	struct thermal_hwmon_temp *temp;
>> +	int new_hwmon_device = 1;
>> +	int result;
>> +
>> +	hwmon = thermal_hwmon_lookup_by_type(tz);
>> +	if (hwmon) {
>> +		new_hwmon_device = 0;
>> +		goto register_sys_interface;
>> +	}
>> +
>> +	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;
>> +	}
>> +	dev_set_drvdata(hwmon->device, hwmon);
>> +	result = device_create_file(hwmon->device, &dev_attr_name);
>> +	if (result)
>> +		goto free_mem;
>> +
>> + register_sys_interface:
>> +	temp = kzalloc(sizeof(struct thermal_hwmon_temp), GFP_KERNEL);
>> +	if (!temp) {
>> +		result = -ENOMEM;
>> +		goto unregister_name;
>> +	}
>> +
>> +	temp->tz = tz;
>> +	hwmon->count++;
>> +
>> +	snprintf(temp->temp_input.name, sizeof(temp->temp_input.name),
>> +		 "temp%d_input", hwmon->count);
>> +	temp->temp_input.attr.attr.name = temp->temp_input.name;
>> +	temp->temp_input.attr.attr.mode = 0444;
>> +	temp->temp_input.attr.show = temp_input_show;
>> +	sysfs_attr_init(&temp->temp_input.attr.attr);
>> +	result = device_create_file(hwmon->device, &temp->temp_input.attr);
>> +	if (result)
>> +		goto free_temp_mem;
>> +
>> +	if (tz->ops->get_crit_temp) {
>> +		unsigned long temperature;
>> +		if (!tz->ops->get_crit_temp(tz, &temperature)) {
>> +			snprintf(temp->temp_crit.name,
>> +				 sizeof(temp->temp_crit.name),
>> +				"temp%d_crit", hwmon->count);
>> +			temp->temp_crit.attr.attr.name = temp-
>>> temp_crit.name;
>> +			temp->temp_crit.attr.attr.mode = 0444;
>> +			temp->temp_crit.attr.show = temp_crit_show;
>> +			sysfs_attr_init(&temp->temp_crit.attr.attr);
>> +			result = device_create_file(hwmon->device,
>> +						    &temp->temp_crit.attr);
>> +			if (result)
>> +				goto unregister_input;
>> +		}
>> +	}
>> +
>> +	mutex_lock(&thermal_hwmon_list_lock);
>> +	if (new_hwmon_device)
>> +		list_add_tail(&hwmon->node, &thermal_hwmon_list);
>> +	list_add_tail(&temp->hwmon_node, &hwmon->tz_list);
>> +	mutex_unlock(&thermal_hwmon_list_lock);
>> +
>> +	return 0;
>> +
>> + unregister_input:
>> +	device_remove_file(hwmon->device, &temp->temp_input.attr);
>> + free_temp_mem:
>> +	kfree(temp);
>> + unregister_name:
>> +	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;
>> +}
>> +
>> +void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
>> +{
>> +	struct thermal_hwmon_device *hwmon;
>> +	struct thermal_hwmon_temp *temp;
>> +
>> +	hwmon = thermal_hwmon_lookup_by_type(tz);
>> +	if (unlikely(!hwmon)) {
>> +		/* Should never happen... */
>> +		dev_dbg(&tz->device, "hwmon device lookup failed!\n");
>> +		return;
>> +	}
>> +
>> +	temp = thermal_hwmon_lookup_temp(hwmon, tz);
>> +	if (unlikely(!temp)) {
>> +		/* Should never happen... */
>> +		dev_dbg(&tz->device, "temperature input lookup failed!\n");
>> +		return;
>> +	}
>> +
>> +	device_remove_file(hwmon->device, &temp->temp_input.attr);
>> +	if (tz->ops->get_crit_temp)
>> +		device_remove_file(hwmon->device, &temp->temp_crit.attr);
>> +
>> +	mutex_lock(&thermal_hwmon_list_lock);
>> +	list_del(&temp->hwmon_node);
>> +	kfree(temp);
>> +	if (!list_empty(&hwmon->tz_list)) {
>> +		mutex_unlock(&thermal_hwmon_list_lock);
>> +		return;
>> +	}
>> +	list_del(&hwmon->node);
>> +	mutex_unlock(&thermal_hwmon_list_lock);
>> +
>> +	device_remove_file(hwmon->device, &dev_attr_name);
>> +	hwmon_device_unregister(hwmon->device);
>> +	kfree(hwmon);
>> +}
>> diff --git a/drivers/thermal/thermal_hwmon.h
>> b/drivers/thermal/thermal_hwmon.h
>> new file mode 100644
>> index 0000000..c798fdb
>> --- /dev/null
>> +++ b/drivers/thermal/thermal_hwmon.h
>> @@ -0,0 +1,49 @@
>> +/*
>> + *  thermal_hwmon.h - Generic Thermal Management hwmon support.
>> + *
>> + *  Code based on Intel thermal_core.c. Copyrights of the original code:
>> + *  Copyright (C) 2008 Intel Corp
>> + *  Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com>
>> + *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com>
>> + *
>> + *  Copyright (C) 2013 Texas Instruments
>> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
>> + *
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ~~~~~~~~~~~~
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; version 2 of the License.
>> + *
>> + *  This program is distributed in the hope that it will be useful, but
>> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + *  General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License along
>> + *  with this program; if not, write to the Free Software Foundation, Inc.,
>> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
>> + *
>> + *
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ~~~~~~~~~~~~
>> + */
>> +#ifndef __THERMAL_HWMON_H__
>> +#define __THERMAL_HWMON_H__
>> +
>> +#include <linux/thermal.h>
>> +
>> +#ifdef CONFIG_THERMAL_HWMON
>> +int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
>> +void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz);
>> +#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
>> +
>> +#endif /* __THERMAL_HWMON_H__ */
>> --
>> 1.8.2.1.342.gfa7285d
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 


-- 
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 295 bytes --]

RE: [RFC PATCH 1/4] thermal: hwmon: move hwmon support to single file

[R, Durgadoss]

> -----Original Message-----
> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> owner@vger.kernel.org] On Behalf Of Eduardo Valentin
> Sent: Tuesday, July 09, 2013 10:24 PM
> To: R, Durgadoss
> Cc: Eduardo Valentin; linux-pm@vger.kernel.org; amit.daniel@samsung.com;
> Zhang, Rui; linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH 1/4] thermal: hwmon: move hwmon support to single
> file
> 
> On 09-07-2013 12:04, R, Durgadoss wrote:
> > Hi Eduardo,
> >
> >> -----Original Message-----
> >> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> >> owner@vger.kernel.org] On Behalf Of Eduardo Valentin
> >> Sent: Tuesday, July 09, 2013 7:30 PM
> >> To: linux-pm@vger.kernel.org; R, Durgadoss; amit.daniel@samsung.com
> >> Cc: Zhang, Rui; Eduardo Valentin; linux-kernel@vger.kernel.org
> >> Subject: [RFC PATCH 1/4] thermal: hwmon: move hwmon support to single file
> >>
> >> In order to improve code organization, this patch
> >> moves the hwmon sysfs support to a file named
> >> thermal_hwmon. This helps to add extra support
> >> for hwmon without scrambling the code.
> >
> > Nice clean up for thermal_core.c. +1 from me.
> >
> > I am inclined to even remove the hwmon related code completely.
> > AFAIK, there are not many users of this config option.
> >
> 
> Hmm. OK. I thought of keeping it as I dont know if there are users.

Yes. This is fine for now.

> Besides, if new code comes out of the hwmon2thermalfw exercise, then it
> would be a good place to keep all the hwmon code.
> 
> > However people are looking for the other option. If they have a
> > hwmon driver, how can it use the thermal framework with ease.
> > [like what you mentioned about this in 0/5]
> 
> yes, problem is that hwmon does not have a standard way of representing
> the drivers. So, one cannot simply write a simple wrapper because hwmon
> drivers does not have a standard get_temperature operation, for instance.
> 
> We could either propose a way to standardize then or implement the call
> to thermal fw driver by driver. Probably the later is desirable, if we
> implement it in a need basis.

later is desirable: Agreed.

Thanks,
Durga

> 
> >
> > Thanks,
> > Durga
> >
> >>
> >> In order to do this move, the hwmon list head is now
> >> using its own locking. Before, the list used
> >> the global thermal locking.
> >>
> >> Cc: Zhang Rui <rui.zhang@intel.com>
> >> Cc: linux-pm@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> >> ---
> >>  drivers/thermal/Kconfig         |   9 ++
> >>  drivers/thermal/Makefile        |   3 +
> >>  drivers/thermal/thermal_core.c  | 255 +-------------------------------------
> >>  drivers/thermal/thermal_hwmon.c | 268
> >> ++++++++++++++++++++++++++++++++++++++++
> >>  drivers/thermal/thermal_hwmon.h |  49 ++++++++
> >>  5 files changed, 330 insertions(+), 254 deletions(-)
> >>  create mode 100644 drivers/thermal/thermal_hwmon.c
> >>  create mode 100644 drivers/thermal/thermal_hwmon.h
> >>
> >> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> >> index e988c81..7fb16bc 100644
> >> --- a/drivers/thermal/Kconfig
> >> +++ b/drivers/thermal/Kconfig
> >> @@ -17,8 +17,17 @@ if THERMAL
> >>
> >>  config THERMAL_HWMON
> >>  	bool
> >> +	prompt "Expose thermal sensors as hwmon device"
> >>  	depends on HWMON=y || HWMON=THERMAL
> >>  	default y
> >> +	help
> >> +	  In case a sensor is registered with the thermal
> >> +	  framework, this option will also register it
> >> +	  as a hwmon. The sensor will then have the common
> >> +	  hwmon sysfs interface.
> >> +
> >> +	  Say 'Y' here if you want all thermal sensors to
> >> +	  have hwmon sysfs interface too.
> >>
> >>  choice
> >>  	prompt "Default Thermal governor"
> >> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> >> index 67184a2..24cb894 100644
> >> --- a/drivers/thermal/Makefile
> >> +++ b/drivers/thermal/Makefile
> >> @@ -5,6 +5,9 @@
> >>  obj-$(CONFIG_THERMAL)		+= thermal_sys.o
> >>  thermal_sys-y			+= thermal_core.o
> >>
> >> +# interface to/from other layers providing sensors
> >> +thermal_sys-$(CONFIG_THERMAL_HWMON)		+=
> thermal_hwmon.o
> >> +
> >>  # governors
> >>  thermal_sys-$(CONFIG_THERMAL_GOV_FAIR_SHARE)	+= fair_share.o
> >>  thermal_sys-$(CONFIG_THERMAL_GOV_STEP_WISE)	+= step_wise.o
> >> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> >> index 1f02e8e..247528b 100644
> >> --- a/drivers/thermal/thermal_core.c
> >> +++ b/drivers/thermal/thermal_core.c
> >> @@ -38,6 +38,7 @@
> >>  #include <net/genetlink.h>
> >>
> >>  #include "thermal_core.h"
> >> +#include "thermal_hwmon.h"
> >>
> >>  MODULE_AUTHOR("Zhang Rui");
> >>  MODULE_DESCRIPTION("Generic thermal management sysfs support");
> >> @@ -859,260 +860,6 @@ thermal_cooling_device_trip_point_show(struct
> device
> >> *dev,
> >>
> >>  /* Device management */
> >>
> >> -#if defined(CONFIG_THERMAL_HWMON)
> >> -
> >> -/* hwmon sys I/F */
> >> -#include <linux/hwmon.h>
> >> -
> >> -/* 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];
> >> -};
> >> -
> >> -/* one temperature input for each thermal zone */
> >> -struct thermal_hwmon_temp {
> >> -	struct list_head hwmon_node;
> >> -	struct thermal_zone_device *tz;
> >> -	struct thermal_hwmon_attr temp_input;	/* hwmon sys attr */
> >> -	struct thermal_hwmon_attr temp_crit;	/* hwmon sys attr */
> >> -};
> >> -
> >> -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_get_drvdata(dev);
> >> -	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)
> >> -{
> >> -	long temperature;
> >> -	int ret;
> >> -	struct thermal_hwmon_attr *hwmon_attr
> >> -			= container_of(attr, struct thermal_hwmon_attr, attr);
> >> -	struct thermal_hwmon_temp *temp
> >> -			= container_of(hwmon_attr, struct
> >> thermal_hwmon_temp,
> >> -				       temp_input);
> >> -	struct thermal_zone_device *tz = temp->tz;
> >> -
> >> -	ret = thermal_zone_get_temp(tz, &temperature);
> >> -
> >> -	if (ret)
> >> -		return ret;
> >> -
> >> -	return sprintf(buf, "%ld\n", temperature);
> >> -}
> >> -
> >> -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_hwmon_temp *temp
> >> -			= container_of(hwmon_attr, struct
> >> thermal_hwmon_temp,
> >> -				       temp_crit);
> >> -	struct thermal_zone_device *tz = temp->tz;
> >> -	long temperature;
> >> -	int ret;
> >> -
> >> -	ret = tz->ops->get_trip_temp(tz, 0, &temperature);
> >> -	if (ret)
> >> -		return ret;
> >> -
> >> -	return sprintf(buf, "%ld\n", temperature);
> >> -}
> >> -
> >> -
> >> -static struct thermal_hwmon_device *
> >> -thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
> >> -{
> >> -	struct thermal_hwmon_device *hwmon;
> >> -
> >> -	mutex_lock(&thermal_list_lock);
> >> -	list_for_each_entry(hwmon, &thermal_hwmon_list, node)
> >> -		if (!strcmp(hwmon->type, tz->type)) {
> >> -			mutex_unlock(&thermal_list_lock);
> >> -			return hwmon;
> >> -		}
> >> -	mutex_unlock(&thermal_list_lock);
> >> -
> >> -	return NULL;
> >> -}
> >> -
> >> -/* Find the temperature input matching a given thermal zone */
> >> -static struct thermal_hwmon_temp *
> >> -thermal_hwmon_lookup_temp(const struct thermal_hwmon_device
> *hwmon,
> >> -			  const struct thermal_zone_device *tz)
> >> -{
> >> -	struct thermal_hwmon_temp *temp;
> >> -
> >> -	mutex_lock(&thermal_list_lock);
> >> -	list_for_each_entry(temp, &hwmon->tz_list, hwmon_node)
> >> -		if (temp->tz == tz) {
> >> -			mutex_unlock(&thermal_list_lock);
> >> -			return temp;
> >> -		}
> >> -	mutex_unlock(&thermal_list_lock);
> >> -
> >> -	return NULL;
> >> -}
> >> -
> >> -static int
> >> -thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
> >> -{
> >> -	struct thermal_hwmon_device *hwmon;
> >> -	struct thermal_hwmon_temp *temp;
> >> -	int new_hwmon_device = 1;
> >> -	int result;
> >> -
> >> -	hwmon = thermal_hwmon_lookup_by_type(tz);
> >> -	if (hwmon) {
> >> -		new_hwmon_device = 0;
> >> -		goto register_sys_interface;
> >> -	}
> >> -
> >> -	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;
> >> -	}
> >> -	dev_set_drvdata(hwmon->device, hwmon);
> >> -	result = device_create_file(hwmon->device, &dev_attr_name);
> >> -	if (result)
> >> -		goto free_mem;
> >> -
> >> - register_sys_interface:
> >> -	temp = kzalloc(sizeof(struct thermal_hwmon_temp), GFP_KERNEL);
> >> -	if (!temp) {
> >> -		result = -ENOMEM;
> >> -		goto unregister_name;
> >> -	}
> >> -
> >> -	temp->tz = tz;
> >> -	hwmon->count++;
> >> -
> >> -	snprintf(temp->temp_input.name, sizeof(temp->temp_input.name),
> >> -		 "temp%d_input", hwmon->count);
> >> -	temp->temp_input.attr.attr.name = temp->temp_input.name;
> >> -	temp->temp_input.attr.attr.mode = 0444;
> >> -	temp->temp_input.attr.show = temp_input_show;
> >> -	sysfs_attr_init(&temp->temp_input.attr.attr);
> >> -	result = device_create_file(hwmon->device, &temp->temp_input.attr);
> >> -	if (result)
> >> -		goto free_temp_mem;
> >> -
> >> -	if (tz->ops->get_crit_temp) {
> >> -		unsigned long temperature;
> >> -		if (!tz->ops->get_crit_temp(tz, &temperature)) {
> >> -			snprintf(temp->temp_crit.name,
> >> -				 sizeof(temp->temp_crit.name),
> >> -				"temp%d_crit", hwmon->count);
> >> -			temp->temp_crit.attr.attr.name = temp-
> >>> temp_crit.name;
> >> -			temp->temp_crit.attr.attr.mode = 0444;
> >> -			temp->temp_crit.attr.show = temp_crit_show;
> >> -			sysfs_attr_init(&temp->temp_crit.attr.attr);
> >> -			result = device_create_file(hwmon->device,
> >> -						    &temp->temp_crit.attr);
> >> -			if (result)
> >> -				goto unregister_input;
> >> -		}
> >> -	}
> >> -
> >> -	mutex_lock(&thermal_list_lock);
> >> -	if (new_hwmon_device)
> >> -		list_add_tail(&hwmon->node, &thermal_hwmon_list);
> >> -	list_add_tail(&temp->hwmon_node, &hwmon->tz_list);
> >> -	mutex_unlock(&thermal_list_lock);
> >> -
> >> -	return 0;
> >> -
> >> - unregister_input:
> >> -	device_remove_file(hwmon->device, &temp->temp_input.attr);
> >> - free_temp_mem:
> >> -	kfree(temp);
> >> - unregister_name:
> >> -	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;
> >> -	struct thermal_hwmon_temp *temp;
> >> -
> >> -	hwmon = thermal_hwmon_lookup_by_type(tz);
> >> -	if (unlikely(!hwmon)) {
> >> -		/* Should never happen... */
> >> -		dev_dbg(&tz->device, "hwmon device lookup failed!\n");
> >> -		return;
> >> -	}
> >> -
> >> -	temp = thermal_hwmon_lookup_temp(hwmon, tz);
> >> -	if (unlikely(!temp)) {
> >> -		/* Should never happen... */
> >> -		dev_dbg(&tz->device, "temperature input lookup failed!\n");
> >> -		return;
> >> -	}
> >> -
> >> -	device_remove_file(hwmon->device, &temp->temp_input.attr);
> >> -	if (tz->ops->get_crit_temp)
> >> -		device_remove_file(hwmon->device, &temp->temp_crit.attr);
> >> -
> >> -	mutex_lock(&thermal_list_lock);
> >> -	list_del(&temp->hwmon_node);
> >> -	kfree(temp);
> >> -	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:		pointer to struct thermal_zone_device
> >> diff --git a/drivers/thermal/thermal_hwmon.c
> >> b/drivers/thermal/thermal_hwmon.c
> >> new file mode 100644
> >> index 0000000..7c665c8
> >> --- /dev/null
> >> +++ b/drivers/thermal/thermal_hwmon.c
> >> @@ -0,0 +1,268 @@
> >> +/*
> >> + *  thermal_hwmon.c - Generic Thermal Management hwmon support.
> >> + *
> >> + *  Code based on Intel thermal_core.c. Copyrights of the original code:
> >> + *  Copyright (C) 2008 Intel Corp
> >> + *  Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com>
> >> + *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com>
> >> + *
> >> + *  Copyright (C) 2013 Texas Instruments
> >> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
> >> + *
> >>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> ~~~~~~~~~~~~
> >> + *
> >> + *  This program is free software; you can redistribute it and/or modify
> >> + *  it under the terms of the GNU General Public License as published by
> >> + *  the Free Software Foundation; version 2 of the License.
> >> + *
> >> + *  This program is distributed in the hope that it will be useful, but
> >> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU
> >> + *  General Public License for more details.
> >> + *
> >> + *  You should have received a copy of the GNU General Public License along
> >> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> >> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> >> + *
> >> + *
> >>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> ~~~~~~~~~~~~
> >> + */
> >> +#include <linux/hwmon.h>
> >> +#include <linux/thermal.h>
> >> +#include <linux/slab.h>
> >> +#include <linux/err.h>
> >> +
> >> +/* hwmon sys I/F */
> >> +/* 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];
> >> +};
> >> +
> >> +/* one temperature input for each thermal zone */
> >> +struct thermal_hwmon_temp {
> >> +	struct list_head hwmon_node;
> >> +	struct thermal_zone_device *tz;
> >> +	struct thermal_hwmon_attr temp_input;	/* hwmon sys attr */
> >> +	struct thermal_hwmon_attr temp_crit;	/* hwmon sys attr */
> >> +};
> >> +
> >> +static LIST_HEAD(thermal_hwmon_list);
> >> +
> >> +static DEFINE_MUTEX(thermal_hwmon_list_lock);
> >> +
> >> +static ssize_t
> >> +name_show(struct device *dev, struct device_attribute *attr, char *buf)
> >> +{
> >> +	struct thermal_hwmon_device *hwmon = dev_get_drvdata(dev);
> >> +	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)
> >> +{
> >> +	long temperature;
> >> +	int ret;
> >> +	struct thermal_hwmon_attr *hwmon_attr
> >> +			= container_of(attr, struct thermal_hwmon_attr, attr);
> >> +	struct thermal_hwmon_temp *temp
> >> +			= container_of(hwmon_attr, struct
> >> thermal_hwmon_temp,
> >> +				       temp_input);
> >> +	struct thermal_zone_device *tz = temp->tz;
> >> +
> >> +	ret = thermal_zone_get_temp(tz, &temperature);
> >> +
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	return sprintf(buf, "%ld\n", temperature);
> >> +}
> >> +
> >> +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_hwmon_temp *temp
> >> +			= container_of(hwmon_attr, struct
> >> thermal_hwmon_temp,
> >> +				       temp_crit);
> >> +	struct thermal_zone_device *tz = temp->tz;
> >> +	long temperature;
> >> +	int ret;
> >> +
> >> +	ret = tz->ops->get_trip_temp(tz, 0, &temperature);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	return sprintf(buf, "%ld\n", temperature);
> >> +}
> >> +
> >> +
> >> +static struct thermal_hwmon_device *
> >> +thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
> >> +{
> >> +	struct thermal_hwmon_device *hwmon;
> >> +
> >> +	mutex_lock(&thermal_hwmon_list_lock);
> >> +	list_for_each_entry(hwmon, &thermal_hwmon_list, node)
> >> +		if (!strcmp(hwmon->type, tz->type)) {
> >> +			mutex_unlock(&thermal_hwmon_list_lock);
> >> +			return hwmon;
> >> +		}
> >> +	mutex_unlock(&thermal_hwmon_list_lock);
> >> +
> >> +	return NULL;
> >> +}
> >> +
> >> +/* Find the temperature input matching a given thermal zone */
> >> +static struct thermal_hwmon_temp *
> >> +thermal_hwmon_lookup_temp(const struct thermal_hwmon_device
> *hwmon,
> >> +			  const struct thermal_zone_device *tz)
> >> +{
> >> +	struct thermal_hwmon_temp *temp;
> >> +
> >> +	mutex_lock(&thermal_hwmon_list_lock);
> >> +	list_for_each_entry(temp, &hwmon->tz_list, hwmon_node)
> >> +		if (temp->tz == tz) {
> >> +			mutex_unlock(&thermal_hwmon_list_lock);
> >> +			return temp;
> >> +		}
> >> +	mutex_unlock(&thermal_hwmon_list_lock);
> >> +
> >> +	return NULL;
> >> +}
> >> +
> >> +int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
> >> +{
> >> +	struct thermal_hwmon_device *hwmon;
> >> +	struct thermal_hwmon_temp *temp;
> >> +	int new_hwmon_device = 1;
> >> +	int result;
> >> +
> >> +	hwmon = thermal_hwmon_lookup_by_type(tz);
> >> +	if (hwmon) {
> >> +		new_hwmon_device = 0;
> >> +		goto register_sys_interface;
> >> +	}
> >> +
> >> +	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;
> >> +	}
> >> +	dev_set_drvdata(hwmon->device, hwmon);
> >> +	result = device_create_file(hwmon->device, &dev_attr_name);
> >> +	if (result)
> >> +		goto free_mem;
> >> +
> >> + register_sys_interface:
> >> +	temp = kzalloc(sizeof(struct thermal_hwmon_temp), GFP_KERNEL);
> >> +	if (!temp) {
> >> +		result = -ENOMEM;
> >> +		goto unregister_name;
> >> +	}
> >> +
> >> +	temp->tz = tz;
> >> +	hwmon->count++;
> >> +
> >> +	snprintf(temp->temp_input.name, sizeof(temp->temp_input.name),
> >> +		 "temp%d_input", hwmon->count);
> >> +	temp->temp_input.attr.attr.name = temp->temp_input.name;
> >> +	temp->temp_input.attr.attr.mode = 0444;
> >> +	temp->temp_input.attr.show = temp_input_show;
> >> +	sysfs_attr_init(&temp->temp_input.attr.attr);
> >> +	result = device_create_file(hwmon->device, &temp->temp_input.attr);
> >> +	if (result)
> >> +		goto free_temp_mem;
> >> +
> >> +	if (tz->ops->get_crit_temp) {
> >> +		unsigned long temperature;
> >> +		if (!tz->ops->get_crit_temp(tz, &temperature)) {
> >> +			snprintf(temp->temp_crit.name,
> >> +				 sizeof(temp->temp_crit.name),
> >> +				"temp%d_crit", hwmon->count);
> >> +			temp->temp_crit.attr.attr.name = temp-
> >>> temp_crit.name;
> >> +			temp->temp_crit.attr.attr.mode = 0444;
> >> +			temp->temp_crit.attr.show = temp_crit_show;
> >> +			sysfs_attr_init(&temp->temp_crit.attr.attr);
> >> +			result = device_create_file(hwmon->device,
> >> +						    &temp->temp_crit.attr);
> >> +			if (result)
> >> +				goto unregister_input;
> >> +		}
> >> +	}
> >> +
> >> +	mutex_lock(&thermal_hwmon_list_lock);
> >> +	if (new_hwmon_device)
> >> +		list_add_tail(&hwmon->node, &thermal_hwmon_list);
> >> +	list_add_tail(&temp->hwmon_node, &hwmon->tz_list);
> >> +	mutex_unlock(&thermal_hwmon_list_lock);
> >> +
> >> +	return 0;
> >> +
> >> + unregister_input:
> >> +	device_remove_file(hwmon->device, &temp->temp_input.attr);
> >> + free_temp_mem:
> >> +	kfree(temp);
> >> + unregister_name:
> >> +	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;
> >> +}
> >> +
> >> +void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
> >> +{
> >> +	struct thermal_hwmon_device *hwmon;
> >> +	struct thermal_hwmon_temp *temp;
> >> +
> >> +	hwmon = thermal_hwmon_lookup_by_type(tz);
> >> +	if (unlikely(!hwmon)) {
> >> +		/* Should never happen... */
> >> +		dev_dbg(&tz->device, "hwmon device lookup failed!\n");
> >> +		return;
> >> +	}
> >> +
> >> +	temp = thermal_hwmon_lookup_temp(hwmon, tz);
> >> +	if (unlikely(!temp)) {
> >> +		/* Should never happen... */
> >> +		dev_dbg(&tz->device, "temperature input lookup failed!\n");
> >> +		return;
> >> +	}
> >> +
> >> +	device_remove_file(hwmon->device, &temp->temp_input.attr);
> >> +	if (tz->ops->get_crit_temp)
> >> +		device_remove_file(hwmon->device, &temp->temp_crit.attr);
> >> +
> >> +	mutex_lock(&thermal_hwmon_list_lock);
> >> +	list_del(&temp->hwmon_node);
> >> +	kfree(temp);
> >> +	if (!list_empty(&hwmon->tz_list)) {
> >> +		mutex_unlock(&thermal_hwmon_list_lock);
> >> +		return;
> >> +	}
> >> +	list_del(&hwmon->node);
> >> +	mutex_unlock(&thermal_hwmon_list_lock);
> >> +
> >> +	device_remove_file(hwmon->device, &dev_attr_name);
> >> +	hwmon_device_unregister(hwmon->device);
> >> +	kfree(hwmon);
> >> +}
> >> diff --git a/drivers/thermal/thermal_hwmon.h
> >> b/drivers/thermal/thermal_hwmon.h
> >> new file mode 100644
> >> index 0000000..c798fdb
> >> --- /dev/null
> >> +++ b/drivers/thermal/thermal_hwmon.h
> >> @@ -0,0 +1,49 @@
> >> +/*
> >> + *  thermal_hwmon.h - Generic Thermal Management hwmon support.
> >> + *
> >> + *  Code based on Intel thermal_core.c. Copyrights of the original code:
> >> + *  Copyright (C) 2008 Intel Corp
> >> + *  Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com>
> >> + *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com>
> >> + *
> >> + *  Copyright (C) 2013 Texas Instruments
> >> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
> >> + *
> >>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> ~~~~~~~~~~~~
> >> + *
> >> + *  This program is free software; you can redistribute it and/or modify
> >> + *  it under the terms of the GNU General Public License as published by
> >> + *  the Free Software Foundation; version 2 of the License.
> >> + *
> >> + *  This program is distributed in the hope that it will be useful, but
> >> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU
> >> + *  General Public License for more details.
> >> + *
> >> + *  You should have received a copy of the GNU General Public License along
> >> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> >> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> >> + *
> >> + *
> >>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> ~~~~~~~~~~~~
> >> + */
> >> +#ifndef __THERMAL_HWMON_H__
> >> +#define __THERMAL_HWMON_H__
> >> +
> >> +#include <linux/thermal.h>
> >> +
> >> +#ifdef CONFIG_THERMAL_HWMON
> >> +int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
> >> +void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz);
> >> +#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
> >> +
> >> +#endif /* __THERMAL_HWMON_H__ */
> >> --
> >> 1.8.2.1.342.gfa7285d
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> >
> 
> 
> --
> You have got to be excited about what you are doing. (L. Lamport)
> 
> Eduardo Valentin

Re: [RFC PATCH 1/4] thermal: hwmon: move hwmon support to single file

[Wei Ni]

On 07/10/2013 12:54 AM, Eduardo Valentin wrote:
> * PGP Signed: 07/09/2013 at 09:54:04 AM
> 
> On 09-07-2013 12:04, R, Durgadoss wrote:
>> Hi Eduardo,
>>
>>> -----Original Message-----
>>> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
>>> owner@vger.kernel.org] On Behalf Of Eduardo Valentin
>>> Sent: Tuesday, July 09, 2013 7:30 PM
>>> To: linux-pm@vger.kernel.org; R, Durgadoss; amit.daniel@samsung.com
>>> Cc: Zhang, Rui; Eduardo Valentin; linux-kernel@vger.kernel.org
>>> Subject: [RFC PATCH 1/4] thermal: hwmon: move hwmon support to single file
>>>
>>> In order to improve code organization, this patch
>>> moves the hwmon sysfs support to a file named
>>> thermal_hwmon. This helps to add extra support
>>> for hwmon without scrambling the code.
>>
>> Nice clean up for thermal_core.c. +1 from me.
>>
>> I am inclined to even remove the hwmon related code completely.
>> AFAIK, there are not many users of this config option.
>>
> 
> Hmm. OK. I thought of keeping it as I dont know if there are users.
> Besides, if new code comes out of the hwmon2thermalfw exercise, then it
> would be a good place to keep all the hwmon code.
> 
>> However people are looking for the other option. If they have a
>> hwmon driver, how can it use the thermal framework with ease.
>> [like what you mentioned about this in 0/5]

Yes, I'm trying to add lm90 driver to the thermal framework, but this
driver already register as a hwmon device, so when register as thermal
zone device, the thermal framework will register another virtual hwmon
device, it mean we have duplicate hwmon devices, it will make confusion.
How about to add a flag in the thermal_zone_params, so when call
thermal_zone_device_register() to register, this flag can indicate if we
want to register the virtual hwmon device or not.

Thanks.
Wei.

> 
> yes, problem is that hwmon does not have a standard way of representing
> the drivers. So, one cannot simply write a simple wrapper because hwmon
> drivers does not have a standard get_temperature operation, for instance.
> 
> We could either propose a way to standardize then or implement the call
> to thermal fw driver by driver. Probably the later is desirable, if we
> implement it in a need basis.
> 
>>
>> Thanks,
>> Durga
>>
>>>
>>> In order to do this move, the hwmon list head is now
>>> using its own locking. Before, the list used
>>> the global thermal locking.
>>>
>>> Cc: Zhang Rui <rui.zhang@intel.com>
>>> Cc: linux-pm@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
>>> ---
>>>  drivers/thermal/Kconfig         |   9 ++
>>>  drivers/thermal/Makefile        |   3 +
>>>  drivers/thermal/thermal_core.c  | 255 +-------------------------------------
>>>  drivers/thermal/thermal_hwmon.c | 268
>>> ++++++++++++++++++++++++++++++++++++++++
>>>  drivers/thermal/thermal_hwmon.h |  49 ++++++++
>>>  5 files changed, 330 insertions(+), 254 deletions(-)
>>>  create mode 100644 drivers/thermal/thermal_hwmon.c
>>>  create mode 100644 drivers/thermal/thermal_hwmon.h
>>>
>>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>>> index e988c81..7fb16bc 100644
>>> --- a/drivers/thermal/Kconfig
>>> +++ b/drivers/thermal/Kconfig
>>> @@ -17,8 +17,17 @@ if THERMAL
>>>
>>>  config THERMAL_HWMON
>>>      bool
>>> +    prompt "Expose thermal sensors as hwmon device"
>>>      depends on HWMON=y || HWMON=THERMAL
>>>      default y
>>> +    help
>>> +      In case a sensor is registered with the thermal
>>> +      framework, this option will also register it
>>> +      as a hwmon. The sensor will then have the common
>>> +      hwmon sysfs interface.
>>> +
>>> +      Say 'Y' here if you want all thermal sensors to
>>> +      have hwmon sysfs interface too.
>>>
>>>  choice
>>>      prompt "Default Thermal governor"
>>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>>> index 67184a2..24cb894 100644
>>> --- a/drivers/thermal/Makefile
>>> +++ b/drivers/thermal/Makefile
>>> @@ -5,6 +5,9 @@
>>>  obj-$(CONFIG_THERMAL)               += thermal_sys.o
>>>  thermal_sys-y                       += thermal_core.o
>>>
>>> +# interface to/from other layers providing sensors
>>> +thermal_sys-$(CONFIG_THERMAL_HWMON)         += thermal_hwmon.o
>>> +
>>>  # governors
>>>  thermal_sys-$(CONFIG_THERMAL_GOV_FAIR_SHARE)        += fair_share.o
>>>  thermal_sys-$(CONFIG_THERMAL_GOV_STEP_WISE) += step_wise.o
>>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>>> index 1f02e8e..247528b 100644
>>> --- a/drivers/thermal/thermal_core.c
>>> +++ b/drivers/thermal/thermal_core.c
>>> @@ -38,6 +38,7 @@
>>>  #include <net/genetlink.h>
>>>
>>>  #include "thermal_core.h"
>>> +#include "thermal_hwmon.h"
>>>
>>>  MODULE_AUTHOR("Zhang Rui");
>>>  MODULE_DESCRIPTION("Generic thermal management sysfs support");
>>> @@ -859,260 +860,6 @@ thermal_cooling_device_trip_point_show(struct device
>>> *dev,
>>>
>>>  /* Device management */
>>>
>>> -#if defined(CONFIG_THERMAL_HWMON)
>>> -
>>> -/* hwmon sys I/F */
>>> -#include <linux/hwmon.h>
>>> -
>>> -/* 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];
>>> -};
>>> -
>>> -/* one temperature input for each thermal zone */
>>> -struct thermal_hwmon_temp {
>>> -    struct list_head hwmon_node;
>>> -    struct thermal_zone_device *tz;
>>> -    struct thermal_hwmon_attr temp_input;   /* hwmon sys attr */
>>> -    struct thermal_hwmon_attr temp_crit;    /* hwmon sys attr */
>>> -};
>>> -
>>> -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_get_drvdata(dev);
>>> -    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)
>>> -{
>>> -    long temperature;
>>> -    int ret;
>>> -    struct thermal_hwmon_attr *hwmon_attr
>>> -                    = container_of(attr, struct thermal_hwmon_attr, attr);
>>> -    struct thermal_hwmon_temp *temp
>>> -                    = container_of(hwmon_attr, struct
>>> thermal_hwmon_temp,
>>> -                                   temp_input);
>>> -    struct thermal_zone_device *tz = temp->tz;
>>> -
>>> -    ret = thermal_zone_get_temp(tz, &temperature);
>>> -
>>> -    if (ret)
>>> -            return ret;
>>> -
>>> -    return sprintf(buf, "%ld\n", temperature);
>>> -}
>>> -
>>> -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_hwmon_temp *temp
>>> -                    = container_of(hwmon_attr, struct
>>> thermal_hwmon_temp,
>>> -                                   temp_crit);
>>> -    struct thermal_zone_device *tz = temp->tz;
>>> -    long temperature;
>>> -    int ret;
>>> -
>>> -    ret = tz->ops->get_trip_temp(tz, 0, &temperature);
>>> -    if (ret)
>>> -            return ret;
>>> -
>>> -    return sprintf(buf, "%ld\n", temperature);
>>> -}
>>> -
>>> -
>>> -static struct thermal_hwmon_device *
>>> -thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
>>> -{
>>> -    struct thermal_hwmon_device *hwmon;
>>> -
>>> -    mutex_lock(&thermal_list_lock);
>>> -    list_for_each_entry(hwmon, &thermal_hwmon_list, node)
>>> -            if (!strcmp(hwmon->type, tz->type)) {
>>> -                    mutex_unlock(&thermal_list_lock);
>>> -                    return hwmon;
>>> -            }
>>> -    mutex_unlock(&thermal_list_lock);
>>> -
>>> -    return NULL;
>>> -}
>>> -
>>> -/* Find the temperature input matching a given thermal zone */
>>> -static struct thermal_hwmon_temp *
>>> -thermal_hwmon_lookup_temp(const struct thermal_hwmon_device *hwmon,
>>> -                      const struct thermal_zone_device *tz)
>>> -{
>>> -    struct thermal_hwmon_temp *temp;
>>> -
>>> -    mutex_lock(&thermal_list_lock);
>>> -    list_for_each_entry(temp, &hwmon->tz_list, hwmon_node)
>>> -            if (temp->tz == tz) {
>>> -                    mutex_unlock(&thermal_list_lock);
>>> -                    return temp;
>>> -            }
>>> -    mutex_unlock(&thermal_list_lock);
>>> -
>>> -    return NULL;
>>> -}
>>> -
>>> -static int
>>> -thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
>>> -{
>>> -    struct thermal_hwmon_device *hwmon;
>>> -    struct thermal_hwmon_temp *temp;
>>> -    int new_hwmon_device = 1;
>>> -    int result;
>>> -
>>> -    hwmon = thermal_hwmon_lookup_by_type(tz);
>>> -    if (hwmon) {
>>> -            new_hwmon_device = 0;
>>> -            goto register_sys_interface;
>>> -    }
>>> -
>>> -    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;
>>> -    }
>>> -    dev_set_drvdata(hwmon->device, hwmon);
>>> -    result = device_create_file(hwmon->device, &dev_attr_name);
>>> -    if (result)
>>> -            goto free_mem;
>>> -
>>> - register_sys_interface:
>>> -    temp = kzalloc(sizeof(struct thermal_hwmon_temp), GFP_KERNEL);
>>> -    if (!temp) {
>>> -            result = -ENOMEM;
>>> -            goto unregister_name;
>>> -    }
>>> -
>>> -    temp->tz = tz;
>>> -    hwmon->count++;
>>> -
>>> -    snprintf(temp->temp_input.name, sizeof(temp->temp_input.name),
>>> -             "temp%d_input", hwmon->count);
>>> -    temp->temp_input.attr.attr.name = temp->temp_input.name;
>>> -    temp->temp_input.attr.attr.mode = 0444;
>>> -    temp->temp_input.attr.show = temp_input_show;
>>> -    sysfs_attr_init(&temp->temp_input.attr.attr);
>>> -    result = device_create_file(hwmon->device, &temp->temp_input.attr);
>>> -    if (result)
>>> -            goto free_temp_mem;
>>> -
>>> -    if (tz->ops->get_crit_temp) {
>>> -            unsigned long temperature;
>>> -            if (!tz->ops->get_crit_temp(tz, &temperature)) {
>>> -                    snprintf(temp->temp_crit.name,
>>> -                             sizeof(temp->temp_crit.name),
>>> -                            "temp%d_crit", hwmon->count);
>>> -                    temp->temp_crit.attr.attr.name = temp-
>>>> temp_crit.name;
>>> -                    temp->temp_crit.attr.attr.mode = 0444;
>>> -                    temp->temp_crit.attr.show = temp_crit_show;
>>> -                    sysfs_attr_init(&temp->temp_crit.attr.attr);
>>> -                    result = device_create_file(hwmon->device,
>>> -                                                &temp->temp_crit.attr);
>>> -                    if (result)
>>> -                            goto unregister_input;
>>> -            }
>>> -    }
>>> -
>>> -    mutex_lock(&thermal_list_lock);
>>> -    if (new_hwmon_device)
>>> -            list_add_tail(&hwmon->node, &thermal_hwmon_list);
>>> -    list_add_tail(&temp->hwmon_node, &hwmon->tz_list);
>>> -    mutex_unlock(&thermal_list_lock);
>>> -
>>> -    return 0;
>>> -
>>> - unregister_input:
>>> -    device_remove_file(hwmon->device, &temp->temp_input.attr);
>>> - free_temp_mem:
>>> -    kfree(temp);
>>> - unregister_name:
>>> -    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;
>>> -    struct thermal_hwmon_temp *temp;
>>> -
>>> -    hwmon = thermal_hwmon_lookup_by_type(tz);
>>> -    if (unlikely(!hwmon)) {
>>> -            /* Should never happen... */
>>> -            dev_dbg(&tz->device, "hwmon device lookup failed!\n");
>>> -            return;
>>> -    }
>>> -
>>> -    temp = thermal_hwmon_lookup_temp(hwmon, tz);
>>> -    if (unlikely(!temp)) {
>>> -            /* Should never happen... */
>>> -            dev_dbg(&tz->device, "temperature input lookup failed!\n");
>>> -            return;
>>> -    }
>>> -
>>> -    device_remove_file(hwmon->device, &temp->temp_input.attr);
>>> -    if (tz->ops->get_crit_temp)
>>> -            device_remove_file(hwmon->device, &temp->temp_crit.attr);
>>> -
>>> -    mutex_lock(&thermal_list_lock);
>>> -    list_del(&temp->hwmon_node);
>>> -    kfree(temp);
>>> -    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:             pointer to struct thermal_zone_device
>>> diff --git a/drivers/thermal/thermal_hwmon.c
>>> b/drivers/thermal/thermal_hwmon.c
>>> new file mode 100644
>>> index 0000000..7c665c8
>>> --- /dev/null
>>> +++ b/drivers/thermal/thermal_hwmon.c
>>> @@ -0,0 +1,268 @@
>>> +/*
>>> + *  thermal_hwmon.c - Generic Thermal Management hwmon support.
>>> + *
>>> + *  Code based on Intel thermal_core.c. Copyrights of the original code:
>>> + *  Copyright (C) 2008 Intel Corp
>>> + *  Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com>
>>> + *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com>
>>> + *
>>> + *  Copyright (C) 2013 Texas Instruments
>>> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
>>> + *
>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> ~~~~~~~~~~~~
>>> + *
>>> + *  This program is free software; you can redistribute it and/or modify
>>> + *  it under the terms of the GNU General Public License as published by
>>> + *  the Free Software Foundation; version 2 of the License.
>>> + *
>>> + *  This program is distributed in the hope that it will be useful, but
>>> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + *  General Public License for more details.
>>> + *
>>> + *  You should have received a copy of the GNU General Public License along
>>> + *  with this program; if not, write to the Free Software Foundation, Inc.,
>>> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
>>> + *
>>> + *
>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> ~~~~~~~~~~~~
>>> + */
>>> +#include <linux/hwmon.h>
>>> +#include <linux/thermal.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/err.h>
>>> +
>>> +/* hwmon sys I/F */
>>> +/* 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];
>>> +};
>>> +
>>> +/* one temperature input for each thermal zone */
>>> +struct thermal_hwmon_temp {
>>> +    struct list_head hwmon_node;
>>> +    struct thermal_zone_device *tz;
>>> +    struct thermal_hwmon_attr temp_input;   /* hwmon sys attr */
>>> +    struct thermal_hwmon_attr temp_crit;    /* hwmon sys attr */
>>> +};
>>> +
>>> +static LIST_HEAD(thermal_hwmon_list);
>>> +
>>> +static DEFINE_MUTEX(thermal_hwmon_list_lock);
>>> +
>>> +static ssize_t
>>> +name_show(struct device *dev, struct device_attribute *attr, char *buf)
>>> +{
>>> +    struct thermal_hwmon_device *hwmon = dev_get_drvdata(dev);
>>> +    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)
>>> +{
>>> +    long temperature;
>>> +    int ret;
>>> +    struct thermal_hwmon_attr *hwmon_attr
>>> +                    = container_of(attr, struct thermal_hwmon_attr, attr);
>>> +    struct thermal_hwmon_temp *temp
>>> +                    = container_of(hwmon_attr, struct
>>> thermal_hwmon_temp,
>>> +                                   temp_input);
>>> +    struct thermal_zone_device *tz = temp->tz;
>>> +
>>> +    ret = thermal_zone_get_temp(tz, &temperature);
>>> +
>>> +    if (ret)
>>> +            return ret;
>>> +
>>> +    return sprintf(buf, "%ld\n", temperature);
>>> +}
>>> +
>>> +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_hwmon_temp *temp
>>> +                    = container_of(hwmon_attr, struct
>>> thermal_hwmon_temp,
>>> +                                   temp_crit);
>>> +    struct thermal_zone_device *tz = temp->tz;
>>> +    long temperature;
>>> +    int ret;
>>> +
>>> +    ret = tz->ops->get_trip_temp(tz, 0, &temperature);
>>> +    if (ret)
>>> +            return ret;
>>> +
>>> +    return sprintf(buf, "%ld\n", temperature);
>>> +}
>>> +
>>> +
>>> +static struct thermal_hwmon_device *
>>> +thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
>>> +{
>>> +    struct thermal_hwmon_device *hwmon;
>>> +
>>> +    mutex_lock(&thermal_hwmon_list_lock);
>>> +    list_for_each_entry(hwmon, &thermal_hwmon_list, node)
>>> +            if (!strcmp(hwmon->type, tz->type)) {
>>> +                    mutex_unlock(&thermal_hwmon_list_lock);
>>> +                    return hwmon;
>>> +            }
>>> +    mutex_unlock(&thermal_hwmon_list_lock);
>>> +
>>> +    return NULL;
>>> +}
>>> +
>>> +/* Find the temperature input matching a given thermal zone */
>>> +static struct thermal_hwmon_temp *
>>> +thermal_hwmon_lookup_temp(const struct thermal_hwmon_device *hwmon,
>>> +                      const struct thermal_zone_device *tz)
>>> +{
>>> +    struct thermal_hwmon_temp *temp;
>>> +
>>> +    mutex_lock(&thermal_hwmon_list_lock);
>>> +    list_for_each_entry(temp, &hwmon->tz_list, hwmon_node)
>>> +            if (temp->tz == tz) {
>>> +                    mutex_unlock(&thermal_hwmon_list_lock);
>>> +                    return temp;
>>> +            }
>>> +    mutex_unlock(&thermal_hwmon_list_lock);
>>> +
>>> +    return NULL;
>>> +}
>>> +
>>> +int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
>>> +{
>>> +    struct thermal_hwmon_device *hwmon;
>>> +    struct thermal_hwmon_temp *temp;
>>> +    int new_hwmon_device = 1;
>>> +    int result;
>>> +
>>> +    hwmon = thermal_hwmon_lookup_by_type(tz);
>>> +    if (hwmon) {
>>> +            new_hwmon_device = 0;
>>> +            goto register_sys_interface;
>>> +    }
>>> +
>>> +    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;
>>> +    }
>>> +    dev_set_drvdata(hwmon->device, hwmon);
>>> +    result = device_create_file(hwmon->device, &dev_attr_name);
>>> +    if (result)
>>> +            goto free_mem;
>>> +
>>> + register_sys_interface:
>>> +    temp = kzalloc(sizeof(struct thermal_hwmon_temp), GFP_KERNEL);
>>> +    if (!temp) {
>>> +            result = -ENOMEM;
>>> +            goto unregister_name;
>>> +    }
>>> +
>>> +    temp->tz = tz;
>>> +    hwmon->count++;
>>> +
>>> +    snprintf(temp->temp_input.name, sizeof(temp->temp_input.name),
>>> +             "temp%d_input", hwmon->count);
>>> +    temp->temp_input.attr.attr.name = temp->temp_input.name;
>>> +    temp->temp_input.attr.attr.mode = 0444;
>>> +    temp->temp_input.attr.show = temp_input_show;
>>> +    sysfs_attr_init(&temp->temp_input.attr.attr);
>>> +    result = device_create_file(hwmon->device, &temp->temp_input.attr);
>>> +    if (result)
>>> +            goto free_temp_mem;
>>> +
>>> +    if (tz->ops->get_crit_temp) {
>>> +            unsigned long temperature;
>>> +            if (!tz->ops->get_crit_temp(tz, &temperature)) {
>>> +                    snprintf(temp->temp_crit.name,
>>> +                             sizeof(temp->temp_crit.name),
>>> +                            "temp%d_crit", hwmon->count);
>>> +                    temp->temp_crit.attr.attr.name = temp-
>>>> temp_crit.name;
>>> +                    temp->temp_crit.attr.attr.mode = 0444;
>>> +                    temp->temp_crit.attr.show = temp_crit_show;
>>> +                    sysfs_attr_init(&temp->temp_crit.attr.attr);
>>> +                    result = device_create_file(hwmon->device,
>>> +                                                &temp->temp_crit.attr);
>>> +                    if (result)
>>> +                            goto unregister_input;
>>> +            }
>>> +    }
>>> +
>>> +    mutex_lock(&thermal_hwmon_list_lock);
>>> +    if (new_hwmon_device)
>>> +            list_add_tail(&hwmon->node, &thermal_hwmon_list);
>>> +    list_add_tail(&temp->hwmon_node, &hwmon->tz_list);
>>> +    mutex_unlock(&thermal_hwmon_list_lock);
>>> +
>>> +    return 0;
>>> +
>>> + unregister_input:
>>> +    device_remove_file(hwmon->device, &temp->temp_input.attr);
>>> + free_temp_mem:
>>> +    kfree(temp);
>>> + unregister_name:
>>> +    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;
>>> +}
>>> +
>>> +void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
>>> +{
>>> +    struct thermal_hwmon_device *hwmon;
>>> +    struct thermal_hwmon_temp *temp;
>>> +
>>> +    hwmon = thermal_hwmon_lookup_by_type(tz);
>>> +    if (unlikely(!hwmon)) {
>>> +            /* Should never happen... */
>>> +            dev_dbg(&tz->device, "hwmon device lookup failed!\n");
>>> +            return;
>>> +    }
>>> +
>>> +    temp = thermal_hwmon_lookup_temp(hwmon, tz);
>>> +    if (unlikely(!temp)) {
>>> +            /* Should never happen... */
>>> +            dev_dbg(&tz->device, "temperature input lookup failed!\n");
>>> +            return;
>>> +    }
>>> +
>>> +    device_remove_file(hwmon->device, &temp->temp_input.attr);
>>> +    if (tz->ops->get_crit_temp)
>>> +            device_remove_file(hwmon->device, &temp->temp_crit.attr);
>>> +
>>> +    mutex_lock(&thermal_hwmon_list_lock);
>>> +    list_del(&temp->hwmon_node);
>>> +    kfree(temp);
>>> +    if (!list_empty(&hwmon->tz_list)) {
>>> +            mutex_unlock(&thermal_hwmon_list_lock);
>>> +            return;
>>> +    }
>>> +    list_del(&hwmon->node);
>>> +    mutex_unlock(&thermal_hwmon_list_lock);
>>> +
>>> +    device_remove_file(hwmon->device, &dev_attr_name);
>>> +    hwmon_device_unregister(hwmon->device);
>>> +    kfree(hwmon);
>>> +}
>>> diff --git a/drivers/thermal/thermal_hwmon.h
>>> b/drivers/thermal/thermal_hwmon.h
>>> new file mode 100644
>>> index 0000000..c798fdb
>>> --- /dev/null
>>> +++ b/drivers/thermal/thermal_hwmon.h
>>> @@ -0,0 +1,49 @@
>>> +/*
>>> + *  thermal_hwmon.h - Generic Thermal Management hwmon support.
>>> + *
>>> + *  Code based on Intel thermal_core.c. Copyrights of the original code:
>>> + *  Copyright (C) 2008 Intel Corp
>>> + *  Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com>
>>> + *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com>
>>> + *
>>> + *  Copyright (C) 2013 Texas Instruments
>>> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
>>> + *
>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> ~~~~~~~~~~~~
>>> + *
>>> + *  This program is free software; you can redistribute it and/or modify
>>> + *  it under the terms of the GNU General Public License as published by
>>> + *  the Free Software Foundation; version 2 of the License.
>>> + *
>>> + *  This program is distributed in the hope that it will be useful, but
>>> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + *  General Public License for more details.
>>> + *
>>> + *  You should have received a copy of the GNU General Public License along
>>> + *  with this program; if not, write to the Free Software Foundation, Inc.,
>>> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
>>> + *
>>> + *
>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> ~~~~~~~~~~~~
>>> + */
>>> +#ifndef __THERMAL_HWMON_H__
>>> +#define __THERMAL_HWMON_H__
>>> +
>>> +#include <linux/thermal.h>
>>> +
>>> +#ifdef CONFIG_THERMAL_HWMON
>>> +int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
>>> +void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz);
>>> +#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
>>> +
>>> +#endif /* __THERMAL_HWMON_H__ */
>>> --
>>> 1.8.2.1.342.gfa7285d
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
> 
> 
> --
> You have got to be excited about what you are doing. (L. Lamport)
> 
> Eduardo Valentin
> 
> 
> * Eduardo Valentin <eduardo.valentin@ti.com>
> * 0x75D0BCFD
> 

RE: [RFC PATCH 1/4] thermal: hwmon: move hwmon support to single file

[R, Durgadoss]

Hi Wei,

> -----Original Message-----
> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> owner@vger.kernel.org] On Behalf Of Wei Ni
> Sent: Wednesday, July 17, 2013 3:19 PM
> To: Eduardo Valentin
> Cc: R, Durgadoss; linux-pm@vger.kernel.org; amit.daniel@samsung.com; Zhang,
> Rui; linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH 1/4] thermal: hwmon: move hwmon support to single
> file
> 
> On 07/10/2013 12:54 AM, Eduardo Valentin wrote:
> > * PGP Signed: 07/09/2013 at 09:54:04 AM
> >
> > On 09-07-2013 12:04, R, Durgadoss wrote:
> >> Hi Eduardo,
> >>
> >>> -----Original Message-----
> >>> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> >>> owner@vger.kernel.org] On Behalf Of Eduardo Valentin
> >>> Sent: Tuesday, July 09, 2013 7:30 PM
> >>> To: linux-pm@vger.kernel.org; R, Durgadoss; amit.daniel@samsung.com
> >>> Cc: Zhang, Rui; Eduardo Valentin; linux-kernel@vger.kernel.org
> >>> Subject: [RFC PATCH 1/4] thermal: hwmon: move hwmon support to single
> file
> >>>
> >>> In order to improve code organization, this patch
> >>> moves the hwmon sysfs support to a file named
> >>> thermal_hwmon. This helps to add extra support
> >>> for hwmon without scrambling the code.
> >>
> >> Nice clean up for thermal_core.c. +1 from me.
> >>
> >> I am inclined to even remove the hwmon related code completely.
> >> AFAIK, there are not many users of this config option.
> >>
> >
> > Hmm. OK. I thought of keeping it as I dont know if there are users.
> > Besides, if new code comes out of the hwmon2thermalfw exercise, then it
> > would be a good place to keep all the hwmon code.
> >
> >> However people are looking for the other option. If they have a
> >> hwmon driver, how can it use the thermal framework with ease.
> >> [like what you mentioned about this in 0/5]
> 
> Yes, I'm trying to add lm90 driver to the thermal framework, but this
> driver already register as a hwmon device, so when register as thermal
> zone device, the thermal framework will register another virtual hwmon
> device, it mean we have duplicate hwmon devices, it will make confusion.
> How about to add a flag in the thermal_zone_params, so when call
> thermal_zone_device_register() to register, this flag can indicate if we
> want to register the virtual hwmon device or not.

I was following your threads in both lm-sensors and here.
I tend to agree with adding this flag in tzp structure.

But, we don't have tzp defined for all thermal zones
that are registering today. So, we need to define the default behavior.
I suggest we set the default to 'No', meaning 'do not create hwmon
devices when this zone is registered'.

Thanks,
Durga

> 
> Thanks.
> Wei.
> 
> >
> > yes, problem is that hwmon does not have a standard way of representing
> > the drivers. So, one cannot simply write a simple wrapper because hwmon
> > drivers does not have a standard get_temperature operation, for instance.
> >
> > We could either propose a way to standardize then or implement the call
> > to thermal fw driver by driver. Probably the later is desirable, if we
> > implement it in a need basis.
> >
> >>
> >> Thanks,
> >> Durga
> >>
> >>>
> >>> In order to do this move, the hwmon list head is now
> >>> using its own locking. Before, the list used
> >>> the global thermal locking.
> >>>
> >>> Cc: Zhang Rui <rui.zhang@intel.com>
> >>> Cc: linux-pm@vger.kernel.org
> >>> Cc: linux-kernel@vger.kernel.org
> >>> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> >>> ---
> >>>  drivers/thermal/Kconfig         |   9 ++
> >>>  drivers/thermal/Makefile        |   3 +
> >>>  drivers/thermal/thermal_core.c  | 255 +-------------------------------------
> >>>  drivers/thermal/thermal_hwmon.c | 268
> >>> ++++++++++++++++++++++++++++++++++++++++
> >>>  drivers/thermal/thermal_hwmon.h |  49 ++++++++
> >>>  5 files changed, 330 insertions(+), 254 deletions(-)
> >>>  create mode 100644 drivers/thermal/thermal_hwmon.c
> >>>  create mode 100644 drivers/thermal/thermal_hwmon.h
> >>>
> >>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> >>> index e988c81..7fb16bc 100644
> >>> --- a/drivers/thermal/Kconfig
> >>> +++ b/drivers/thermal/Kconfig
> >>> @@ -17,8 +17,17 @@ if THERMAL
> >>>
> >>>  config THERMAL_HWMON
> >>>      bool
> >>> +    prompt "Expose thermal sensors as hwmon device"
> >>>      depends on HWMON=y || HWMON=THERMAL
> >>>      default y
> >>> +    help
> >>> +      In case a sensor is registered with the thermal
> >>> +      framework, this option will also register it
> >>> +      as a hwmon. The sensor will then have the common
> >>> +      hwmon sysfs interface.
> >>> +
> >>> +      Say 'Y' here if you want all thermal sensors to
> >>> +      have hwmon sysfs interface too.
> >>>
> >>>  choice
> >>>      prompt "Default Thermal governor"
> >>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> >>> index 67184a2..24cb894 100644
> >>> --- a/drivers/thermal/Makefile
> >>> +++ b/drivers/thermal/Makefile
> >>> @@ -5,6 +5,9 @@
> >>>  obj-$(CONFIG_THERMAL)               += thermal_sys.o
> >>>  thermal_sys-y                       += thermal_core.o
> >>>
> >>> +# interface to/from other layers providing sensors
> >>> +thermal_sys-$(CONFIG_THERMAL_HWMON)         += thermal_hwmon.o
> >>> +
> >>>  # governors
> >>>  thermal_sys-$(CONFIG_THERMAL_GOV_FAIR_SHARE)        += fair_share.o
> >>>  thermal_sys-$(CONFIG_THERMAL_GOV_STEP_WISE) += step_wise.o
> >>> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c
> >>> index 1f02e8e..247528b 100644
> >>> --- a/drivers/thermal/thermal_core.c
> >>> +++ b/drivers/thermal/thermal_core.c
> >>> @@ -38,6 +38,7 @@
> >>>  #include <net/genetlink.h>
> >>>
> >>>  #include "thermal_core.h"
> >>> +#include "thermal_hwmon.h"
> >>>
> >>>  MODULE_AUTHOR("Zhang Rui");
> >>>  MODULE_DESCRIPTION("Generic thermal management sysfs support");
> >>> @@ -859,260 +860,6 @@ thermal_cooling_device_trip_point_show(struct
> device
> >>> *dev,
> >>>
> >>>  /* Device management */
> >>>
> >>> -#if defined(CONFIG_THERMAL_HWMON)
> >>> -
> >>> -/* hwmon sys I/F */
> >>> -#include <linux/hwmon.h>
> >>> -
> >>> -/* 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];
> >>> -};
> >>> -
> >>> -/* one temperature input for each thermal zone */
> >>> -struct thermal_hwmon_temp {
> >>> -    struct list_head hwmon_node;
> >>> -    struct thermal_zone_device *tz;
> >>> -    struct thermal_hwmon_attr temp_input;   /* hwmon sys attr */
> >>> -    struct thermal_hwmon_attr temp_crit;    /* hwmon sys attr */
> >>> -};
> >>> -
> >>> -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_get_drvdata(dev);
> >>> -    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)
> >>> -{
> >>> -    long temperature;
> >>> -    int ret;
> >>> -    struct thermal_hwmon_attr *hwmon_attr
> >>> -                    = container_of(attr, struct thermal_hwmon_attr, attr);
> >>> -    struct thermal_hwmon_temp *temp
> >>> -                    = container_of(hwmon_attr, struct
> >>> thermal_hwmon_temp,
> >>> -                                   temp_input);
> >>> -    struct thermal_zone_device *tz = temp->tz;
> >>> -
> >>> -    ret = thermal_zone_get_temp(tz, &temperature);
> >>> -
> >>> -    if (ret)
> >>> -            return ret;
> >>> -
> >>> -    return sprintf(buf, "%ld\n", temperature);
> >>> -}
> >>> -
> >>> -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_hwmon_temp *temp
> >>> -                    = container_of(hwmon_attr, struct
> >>> thermal_hwmon_temp,
> >>> -                                   temp_crit);
> >>> -    struct thermal_zone_device *tz = temp->tz;
> >>> -    long temperature;
> >>> -    int ret;
> >>> -
> >>> -    ret = tz->ops->get_trip_temp(tz, 0, &temperature);
> >>> -    if (ret)
> >>> -            return ret;
> >>> -
> >>> -    return sprintf(buf, "%ld\n", temperature);
> >>> -}
> >>> -
> >>> -
> >>> -static struct thermal_hwmon_device *
> >>> -thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
> >>> -{
> >>> -    struct thermal_hwmon_device *hwmon;
> >>> -
> >>> -    mutex_lock(&thermal_list_lock);
> >>> -    list_for_each_entry(hwmon, &thermal_hwmon_list, node)
> >>> -            if (!strcmp(hwmon->type, tz->type)) {
> >>> -                    mutex_unlock(&thermal_list_lock);
> >>> -                    return hwmon;
> >>> -            }
> >>> -    mutex_unlock(&thermal_list_lock);
> >>> -
> >>> -    return NULL;
> >>> -}
> >>> -
> >>> -/* Find the temperature input matching a given thermal zone */
> >>> -static struct thermal_hwmon_temp *
> >>> -thermal_hwmon_lookup_temp(const struct thermal_hwmon_device
> *hwmon,
> >>> -                      const struct thermal_zone_device *tz)
> >>> -{
> >>> -    struct thermal_hwmon_temp *temp;
> >>> -
> >>> -    mutex_lock(&thermal_list_lock);
> >>> -    list_for_each_entry(temp, &hwmon->tz_list, hwmon_node)
> >>> -            if (temp->tz == tz) {
> >>> -                    mutex_unlock(&thermal_list_lock);
> >>> -                    return temp;
> >>> -            }
> >>> -    mutex_unlock(&thermal_list_lock);
> >>> -
> >>> -    return NULL;
> >>> -}
> >>> -
> >>> -static int
> >>> -thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
> >>> -{
> >>> -    struct thermal_hwmon_device *hwmon;
> >>> -    struct thermal_hwmon_temp *temp;
> >>> -    int new_hwmon_device = 1;
> >>> -    int result;
> >>> -
> >>> -    hwmon = thermal_hwmon_lookup_by_type(tz);
> >>> -    if (hwmon) {
> >>> -            new_hwmon_device = 0;
> >>> -            goto register_sys_interface;
> >>> -    }
> >>> -
> >>> -    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;
> >>> -    }
> >>> -    dev_set_drvdata(hwmon->device, hwmon);
> >>> -    result = device_create_file(hwmon->device, &dev_attr_name);
> >>> -    if (result)
> >>> -            goto free_mem;
> >>> -
> >>> - register_sys_interface:
> >>> -    temp = kzalloc(sizeof(struct thermal_hwmon_temp), GFP_KERNEL);
> >>> -    if (!temp) {
> >>> -            result = -ENOMEM;
> >>> -            goto unregister_name;
> >>> -    }
> >>> -
> >>> -    temp->tz = tz;
> >>> -    hwmon->count++;
> >>> -
> >>> -    snprintf(temp->temp_input.name, sizeof(temp->temp_input.name),
> >>> -             "temp%d_input", hwmon->count);
> >>> -    temp->temp_input.attr.attr.name = temp->temp_input.name;
> >>> -    temp->temp_input.attr.attr.mode = 0444;
> >>> -    temp->temp_input.attr.show = temp_input_show;
> >>> -    sysfs_attr_init(&temp->temp_input.attr.attr);
> >>> -    result = device_create_file(hwmon->device, &temp->temp_input.attr);
> >>> -    if (result)
> >>> -            goto free_temp_mem;
> >>> -
> >>> -    if (tz->ops->get_crit_temp) {
> >>> -            unsigned long temperature;
> >>> -            if (!tz->ops->get_crit_temp(tz, &temperature)) {
> >>> -                    snprintf(temp->temp_crit.name,
> >>> -                             sizeof(temp->temp_crit.name),
> >>> -                            "temp%d_crit", hwmon->count);
> >>> -                    temp->temp_crit.attr.attr.name = temp-
> >>>> temp_crit.name;
> >>> -                    temp->temp_crit.attr.attr.mode = 0444;
> >>> -                    temp->temp_crit.attr.show = temp_crit_show;
> >>> -                    sysfs_attr_init(&temp->temp_crit.attr.attr);
> >>> -                    result = device_create_file(hwmon->device,
> >>> -                                                &temp->temp_crit.attr);
> >>> -                    if (result)
> >>> -                            goto unregister_input;
> >>> -            }
> >>> -    }
> >>> -
> >>> -    mutex_lock(&thermal_list_lock);
> >>> -    if (new_hwmon_device)
> >>> -            list_add_tail(&hwmon->node, &thermal_hwmon_list);
> >>> -    list_add_tail(&temp->hwmon_node, &hwmon->tz_list);
> >>> -    mutex_unlock(&thermal_list_lock);
> >>> -
> >>> -    return 0;
> >>> -
> >>> - unregister_input:
> >>> -    device_remove_file(hwmon->device, &temp->temp_input.attr);
> >>> - free_temp_mem:
> >>> -    kfree(temp);
> >>> - unregister_name:
> >>> -    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;
> >>> -    struct thermal_hwmon_temp *temp;
> >>> -
> >>> -    hwmon = thermal_hwmon_lookup_by_type(tz);
> >>> -    if (unlikely(!hwmon)) {
> >>> -            /* Should never happen... */
> >>> -            dev_dbg(&tz->device, "hwmon device lookup failed!\n");
> >>> -            return;
> >>> -    }
> >>> -
> >>> -    temp = thermal_hwmon_lookup_temp(hwmon, tz);
> >>> -    if (unlikely(!temp)) {
> >>> -            /* Should never happen... */
> >>> -            dev_dbg(&tz->device, "temperature input lookup failed!\n");
> >>> -            return;
> >>> -    }
> >>> -
> >>> -    device_remove_file(hwmon->device, &temp->temp_input.attr);
> >>> -    if (tz->ops->get_crit_temp)
> >>> -            device_remove_file(hwmon->device, &temp->temp_crit.attr);
> >>> -
> >>> -    mutex_lock(&thermal_list_lock);
> >>> -    list_del(&temp->hwmon_node);
> >>> -    kfree(temp);
> >>> -    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:             pointer to struct thermal_zone_device
> >>> diff --git a/drivers/thermal/thermal_hwmon.c
> >>> b/drivers/thermal/thermal_hwmon.c
> >>> new file mode 100644
> >>> index 0000000..7c665c8
> >>> --- /dev/null
> >>> +++ b/drivers/thermal/thermal_hwmon.c
> >>> @@ -0,0 +1,268 @@
> >>> +/*
> >>> + *  thermal_hwmon.c - Generic Thermal Management hwmon support.
> >>> + *
> >>> + *  Code based on Intel thermal_core.c. Copyrights of the original code:
> >>> + *  Copyright (C) 2008 Intel Corp
> >>> + *  Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com>
> >>> + *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com>
> >>> + *
> >>> + *  Copyright (C) 2013 Texas Instruments
> >>> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
> >>> + *
> >>>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>> ~~~~~~~~~~~~
> >>> + *
> >>> + *  This program is free software; you can redistribute it and/or modify
> >>> + *  it under the terms of the GNU General Public License as published by
> >>> + *  the Free Software Foundation; version 2 of the License.
> >>> + *
> >>> + *  This program is distributed in the hope that it will be useful, but
> >>> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU
> >>> + *  General Public License for more details.
> >>> + *
> >>> + *  You should have received a copy of the GNU General Public License
> along
> >>> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> >>> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> >>> + *
> >>> + *
> >>>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>> ~~~~~~~~~~~~
> >>> + */
> >>> +#include <linux/hwmon.h>
> >>> +#include <linux/thermal.h>
> >>> +#include <linux/slab.h>
> >>> +#include <linux/err.h>
> >>> +
> >>> +/* hwmon sys I/F */
> >>> +/* 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];
> >>> +};
> >>> +
> >>> +/* one temperature input for each thermal zone */
> >>> +struct thermal_hwmon_temp {
> >>> +    struct list_head hwmon_node;
> >>> +    struct thermal_zone_device *tz;
> >>> +    struct thermal_hwmon_attr temp_input;   /* hwmon sys attr */
> >>> +    struct thermal_hwmon_attr temp_crit;    /* hwmon sys attr */
> >>> +};
> >>> +
> >>> +static LIST_HEAD(thermal_hwmon_list);
> >>> +
> >>> +static DEFINE_MUTEX(thermal_hwmon_list_lock);
> >>> +
> >>> +static ssize_t
> >>> +name_show(struct device *dev, struct device_attribute *attr, char *buf)
> >>> +{
> >>> +    struct thermal_hwmon_device *hwmon = dev_get_drvdata(dev);
> >>> +    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)
> >>> +{
> >>> +    long temperature;
> >>> +    int ret;
> >>> +    struct thermal_hwmon_attr *hwmon_attr
> >>> +                    = container_of(attr, struct thermal_hwmon_attr, attr);
> >>> +    struct thermal_hwmon_temp *temp
> >>> +                    = container_of(hwmon_attr, struct
> >>> thermal_hwmon_temp,
> >>> +                                   temp_input);
> >>> +    struct thermal_zone_device *tz = temp->tz;
> >>> +
> >>> +    ret = thermal_zone_get_temp(tz, &temperature);
> >>> +
> >>> +    if (ret)
> >>> +            return ret;
> >>> +
> >>> +    return sprintf(buf, "%ld\n", temperature);
> >>> +}
> >>> +
> >>> +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_hwmon_temp *temp
> >>> +                    = container_of(hwmon_attr, struct
> >>> thermal_hwmon_temp,
> >>> +                                   temp_crit);
> >>> +    struct thermal_zone_device *tz = temp->tz;
> >>> +    long temperature;
> >>> +    int ret;
> >>> +
> >>> +    ret = tz->ops->get_trip_temp(tz, 0, &temperature);
> >>> +    if (ret)
> >>> +            return ret;
> >>> +
> >>> +    return sprintf(buf, "%ld\n", temperature);
> >>> +}
> >>> +
> >>> +
> >>> +static struct thermal_hwmon_device *
> >>> +thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
> >>> +{
> >>> +    struct thermal_hwmon_device *hwmon;
> >>> +
> >>> +    mutex_lock(&thermal_hwmon_list_lock);
> >>> +    list_for_each_entry(hwmon, &thermal_hwmon_list, node)
> >>> +            if (!strcmp(hwmon->type, tz->type)) {
> >>> +                    mutex_unlock(&thermal_hwmon_list_lock);
> >>> +                    return hwmon;
> >>> +            }
> >>> +    mutex_unlock(&thermal_hwmon_list_lock);
> >>> +
> >>> +    return NULL;
> >>> +}
> >>> +
> >>> +/* Find the temperature input matching a given thermal zone */
> >>> +static struct thermal_hwmon_temp *
> >>> +thermal_hwmon_lookup_temp(const struct thermal_hwmon_device
> *hwmon,
> >>> +                      const struct thermal_zone_device *tz)
> >>> +{
> >>> +    struct thermal_hwmon_temp *temp;
> >>> +
> >>> +    mutex_lock(&thermal_hwmon_list_lock);
> >>> +    list_for_each_entry(temp, &hwmon->tz_list, hwmon_node)
> >>> +            if (temp->tz == tz) {
> >>> +                    mutex_unlock(&thermal_hwmon_list_lock);
> >>> +                    return temp;
> >>> +            }
> >>> +    mutex_unlock(&thermal_hwmon_list_lock);
> >>> +
> >>> +    return NULL;
> >>> +}
> >>> +
> >>> +int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
> >>> +{
> >>> +    struct thermal_hwmon_device *hwmon;
> >>> +    struct thermal_hwmon_temp *temp;
> >>> +    int new_hwmon_device = 1;
> >>> +    int result;
> >>> +
> >>> +    hwmon = thermal_hwmon_lookup_by_type(tz);
> >>> +    if (hwmon) {
> >>> +            new_hwmon_device = 0;
> >>> +            goto register_sys_interface;
> >>> +    }
> >>> +
> >>> +    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;
> >>> +    }
> >>> +    dev_set_drvdata(hwmon->device, hwmon);
> >>> +    result = device_create_file(hwmon->device, &dev_attr_name);
> >>> +    if (result)
> >>> +            goto free_mem;
> >>> +
> >>> + register_sys_interface:
> >>> +    temp = kzalloc(sizeof(struct thermal_hwmon_temp), GFP_KERNEL);
> >>> +    if (!temp) {
> >>> +            result = -ENOMEM;
> >>> +            goto unregister_name;
> >>> +    }
> >>> +
> >>> +    temp->tz = tz;
> >>> +    hwmon->count++;
> >>> +
> >>> +    snprintf(temp->temp_input.name, sizeof(temp->temp_input.name),
> >>> +             "temp%d_input", hwmon->count);
> >>> +    temp->temp_input.attr.attr.name = temp->temp_input.name;
> >>> +    temp->temp_input.attr.attr.mode = 0444;
> >>> +    temp->temp_input.attr.show = temp_input_show;
> >>> +    sysfs_attr_init(&temp->temp_input.attr.attr);
> >>> +    result = device_create_file(hwmon->device, &temp->temp_input.attr);
> >>> +    if (result)
> >>> +            goto free_temp_mem;
> >>> +
> >>> +    if (tz->ops->get_crit_temp) {
> >>> +            unsigned long temperature;
> >>> +            if (!tz->ops->get_crit_temp(tz, &temperature)) {
> >>> +                    snprintf(temp->temp_crit.name,
> >>> +                             sizeof(temp->temp_crit.name),
> >>> +                            "temp%d_crit", hwmon->count);
> >>> +                    temp->temp_crit.attr.attr.name = temp-
> >>>> temp_crit.name;
> >>> +                    temp->temp_crit.attr.attr.mode = 0444;
> >>> +                    temp->temp_crit.attr.show = temp_crit_show;
> >>> +                    sysfs_attr_init(&temp->temp_crit.attr.attr);
> >>> +                    result = device_create_file(hwmon->device,
> >>> +                                                &temp->temp_crit.attr);
> >>> +                    if (result)
> >>> +                            goto unregister_input;
> >>> +            }
> >>> +    }
> >>> +
> >>> +    mutex_lock(&thermal_hwmon_list_lock);
> >>> +    if (new_hwmon_device)
> >>> +            list_add_tail(&hwmon->node, &thermal_hwmon_list);
> >>> +    list_add_tail(&temp->hwmon_node, &hwmon->tz_list);
> >>> +    mutex_unlock(&thermal_hwmon_list_lock);
> >>> +
> >>> +    return 0;
> >>> +
> >>> + unregister_input:
> >>> +    device_remove_file(hwmon->device, &temp->temp_input.attr);
> >>> + free_temp_mem:
> >>> +    kfree(temp);
> >>> + unregister_name:
> >>> +    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;
> >>> +}
> >>> +
> >>> +void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
> >>> +{
> >>> +    struct thermal_hwmon_device *hwmon;
> >>> +    struct thermal_hwmon_temp *temp;
> >>> +
> >>> +    hwmon = thermal_hwmon_lookup_by_type(tz);
> >>> +    if (unlikely(!hwmon)) {
> >>> +            /* Should never happen... */
> >>> +            dev_dbg(&tz->device, "hwmon device lookup failed!\n");
> >>> +            return;
> >>> +    }
> >>> +
> >>> +    temp = thermal_hwmon_lookup_temp(hwmon, tz);
> >>> +    if (unlikely(!temp)) {
> >>> +            /* Should never happen... */
> >>> +            dev_dbg(&tz->device, "temperature input lookup failed!\n");
> >>> +            return;
> >>> +    }
> >>> +
> >>> +    device_remove_file(hwmon->device, &temp->temp_input.attr);
> >>> +    if (tz->ops->get_crit_temp)
> >>> +            device_remove_file(hwmon->device, &temp->temp_crit.attr);
> >>> +
> >>> +    mutex_lock(&thermal_hwmon_list_lock);
> >>> +    list_del(&temp->hwmon_node);
> >>> +    kfree(temp);
> >>> +    if (!list_empty(&hwmon->tz_list)) {
> >>> +            mutex_unlock(&thermal_hwmon_list_lock);
> >>> +            return;
> >>> +    }
> >>> +    list_del(&hwmon->node);
> >>> +    mutex_unlock(&thermal_hwmon_list_lock);
> >>> +
> >>> +    device_remove_file(hwmon->device, &dev_attr_name);
> >>> +    hwmon_device_unregister(hwmon->device);
> >>> +    kfree(hwmon);
> >>> +}
> >>> diff --git a/drivers/thermal/thermal_hwmon.h
> >>> b/drivers/thermal/thermal_hwmon.h
> >>> new file mode 100644
> >>> index 0000000..c798fdb
> >>> --- /dev/null
> >>> +++ b/drivers/thermal/thermal_hwmon.h
> >>> @@ -0,0 +1,49 @@
> >>> +/*
> >>> + *  thermal_hwmon.h - Generic Thermal Management hwmon support.
> >>> + *
> >>> + *  Code based on Intel thermal_core.c. Copyrights of the original code:
> >>> + *  Copyright (C) 2008 Intel Corp
> >>> + *  Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com>
> >>> + *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com>
> >>> + *
> >>> + *  Copyright (C) 2013 Texas Instruments
> >>> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
> >>> + *
> >>>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>> ~~~~~~~~~~~~
> >>> + *
> >>> + *  This program is free software; you can redistribute it and/or modify
> >>> + *  it under the terms of the GNU General Public License as published by
> >>> + *  the Free Software Foundation; version 2 of the License.
> >>> + *
> >>> + *  This program is distributed in the hope that it will be useful, but
> >>> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU
> >>> + *  General Public License for more details.
> >>> + *
> >>> + *  You should have received a copy of the GNU General Public License
> along
> >>> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> >>> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> >>> + *
> >>> + *
> >>>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>> ~~~~~~~~~~~~
> >>> + */
> >>> +#ifndef __THERMAL_HWMON_H__
> >>> +#define __THERMAL_HWMON_H__
> >>> +
> >>> +#include <linux/thermal.h>
> >>> +
> >>> +#ifdef CONFIG_THERMAL_HWMON
> >>> +int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
> >>> +void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz);
> >>> +#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
> >>> +
> >>> +#endif /* __THERMAL_HWMON_H__ */
> >>> --
> >>> 1.8.2.1.342.gfa7285d
> >>>
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> >>
> >
> >
> > --
> > You have got to be excited about what you are doing. (L. Lamport)
> >
> > Eduardo Valentin
> >
> >
> > * Eduardo Valentin <eduardo.valentin@ti.com>
> > * 0x75D0BCFD
> >
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/4] thermal: hwmon: move hwmon support to single file

[Zhang Rui]

Hi, Eduardo,

thanks for the patch, just one minor comments.

On 二, 2013-07-09 at 10:00 -0400, Eduardo Valentin wrote:
> diff --git a/drivers/thermal/thermal_hwmon.h b/drivers/thermal/thermal_hwmon.h
> new file mode 100644
> index 0000000..c798fdb
> --- /dev/null
> +++ b/drivers/thermal/thermal_hwmon.h
> @@ -0,0 +1,49 @@
> +/*
> + *  thermal_hwmon.h - Generic Thermal Management hwmon support.
> + *
> + *  Code based on Intel thermal_core.c. Copyrights of the original code:
> + *  Copyright (C) 2008 Intel Corp
> + *  Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com>
> + *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com>
> + *
> + *  Copyright (C) 2013 Texas Instruments
> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
> + *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; version 2 of the License.
> + *
> + *  This program is distributed in the hope that it will be useful, but
> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + */
> +#ifndef __THERMAL_HWMON_H__
> +#define __THERMAL_HWMON_H__
> +
> +#include <linux/thermal.h>
> +
> +#ifdef CONFIG_THERMAL_HWMON
> +int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
> +void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz);
> +#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
> +

I'd prefer use "inline" here.

thanks,
rui

> +#endif /* __THERMAL_HWMON_H__ */

[RFC PATCH 2/4] thermal: introduce device tree parser

[Eduardo Valentin]

In order to be able to build thermal policies
based on generic sensors, like I2C device, that
can be places in different points on different boards,
there is a need to have a way to feed board dependent
data into the thermal framework.

This patch introduces a thermal data parser for device
tree. The parsed data is used to build thermal zones
and thermal binding parameters. The output data
can then be used to deploy thermal policies.

This patch adds also documentation regarding this
API and how to define define tree nodes to use
this infrastructure.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
---
 .../devicetree/bindings/thermal/thermal.txt        |  92 +++++
 drivers/thermal/Kconfig                            |  13 +
 drivers/thermal/Makefile                           |   1 +
 drivers/thermal/thermal_dt.c                       | 412 +++++++++++++++++++++
 drivers/thermal/thermal_dt.h                       |  44 +++
 include/linux/thermal.h                            |   3 +
 6 files changed, 565 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/thermal.txt
 create mode 100644 drivers/thermal/thermal_dt.c
 create mode 100644 drivers/thermal/thermal_dt.h

diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
new file mode 100644
index 0000000..2c25ab2
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/thermal.txt
@@ -0,0 +1,92 @@
+----------------------------------------
+Thermal Framework Device Tree descriptor
+----------------------------------------
+
+This file describes how to define a thermal structure using device tree.
+A thermal structure includes thermal zones and their components, such
+as name, governor, trip points, polling intervals and cooling devices
+binding descriptors. A binding descriptor may contain information on
+how to react, with a cooling stepped action or a weight on a fair share.
+
+****
+trip
+****
+
+The trip node is a node to describe a point in the temperature domain
+in which the system takes an action. This node describes just the point,
+not the action.
+
+A node describing a trip must contain:
+- temperature: the trip temperature level, in milliCelsius.
+- hysteresis: a (low) hysteresis value on 'temperature'. This is a relative
+value, in milliCelsius.
+- type: the trip type. Here is the type mapping:
+	THERMAL_TRIP_ACTIVE = 0
+	THERMAL_TRIP_PASSIVE = 1
+	THERMAL_TRIP_HOT = 2
+	THERMAL_TRIP_CRITICAL = 3
+
+**********
+bind_param
+**********
+
+The bind_param node is a node to describe how cooling devices get assigned
+to trip points of the zone. The cooling devices are expected to be loaded
+in the target system.
+
+A node describing a bind_param must contain:
+- cooling_device: A string with the cooling device name.
+- weight: The 'influence' of a particular cooling device on this zone.
+             This is on a percentage scale. The sum of all these weights
+             (for a particular zone) cannot exceed 100.
+- trip_mask: This is a bit mask that gives the binding relation between
+               this thermal zone and cdev, for a particular trip point.
+               If nth bit is set, then the cdev and thermal zone are bound
+               for trip point n.
+
+************
+thermal_zone
+************
+
+The thermal_zone node is the node containing all the required info
+for describing a thermal zone, including its cdev bindings. The thermal_zone
+node must contain, apart from its own properties, one node containing
+trip nodes and one node containing all the zone bind parameters.
+
+Required properties:
+- type: this is a string containing the zone type. Say 'cpu', 'core', 'mem', etc.
+- mask: Bit string: If 'n'th bit is set, then trip point 'n' is writeable.
+
+- passive_delay: number of milliseconds to wait between polls when
+	performing passive cooling.
+- polling_delay: number of milliseconds to wait between polls when checking
+
+- governor: A string containing the default governor for this zone.
+
+Below is an example:
+thermal_zone {
+            type = "CPU";
+            mask = <0x03>; /* trips writability */
+            passive_delay = <250>; /* milliseconds */
+            polling_delay = <1000>; /* milliseconds */
+            governor = "step_wise";
+            trips {
+                    alert@100000{
+                            temperature = <100000>; /* milliCelsius */
+                            hysteresis = <0>; /* milliCelsius */
+                            type = <1>;
+                    };
+                    crit@125000{
+                            temperature = <125000>; /* milliCelsius */
+                            hysteresis = <0>; /* milliCelsius */
+                            type = <3>;
+                    };
+            };
+            bind_params {
+                    action@0{
+                            cooling_device = "thermal-cpufreq";
+                            weight = <100>; /* percentage */
+                            mask = <0x01>;
+                    };
+            };
+};
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 7fb16bc..753f0dc 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -29,6 +29,19 @@ config THERMAL_HWMON
 	  Say 'Y' here if you want all thermal sensors to
 	  have hwmon sysfs interface too.
 
+config THERMAL_OF
+	bool
+	prompt "APIs to parse thermal data out of device tree"
+	depends on OF
+	default y
+	help
+	  This options provides helpers to add the support to
+	  read and parse thermal data definitions out of the
+	  device tree blob.
+
+	  Say 'Y' here if you need to build thermal infrastructure
+	  based on device tree.
+
 choice
 	prompt "Default Thermal governor"
 	default THERMAL_DEFAULT_GOV_STEP_WISE
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 24cb894..eedb273 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -7,6 +7,7 @@ thermal_sys-y			+= thermal_core.o
 
 # interface to/from other layers providing sensors
 thermal_sys-$(CONFIG_THERMAL_HWMON)		+= thermal_hwmon.o
+thermal_sys-$(CONFIG_THERMAL_OF)		+= thermal_dt.o
 
 # governors
 thermal_sys-$(CONFIG_THERMAL_GOV_FAIR_SHARE)	+= fair_share.o
diff --git a/drivers/thermal/thermal_dt.c b/drivers/thermal/thermal_dt.c
new file mode 100644
index 0000000..6553582
--- /dev/null
+++ b/drivers/thermal/thermal_dt.c
@@ -0,0 +1,412 @@
+/*
+ *  thermal_dt.c - Generic Thermal Management device tree support.
+ *
+ *  Copyright (C) 2013 Texas Instruments
+ *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
+ *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+#include <linux/thermal.h>
+#include <linux/slab.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/string.h>
+
+struct __thermal_bind_params {
+	char cooling_device[THERMAL_NAME_LENGTH];
+};
+
+static
+int thermal_of_match(struct thermal_zone_device *tz,
+		     struct thermal_cooling_device *cdev);
+
+static int thermal_of_populate_bind_params(struct device *dev,
+					   struct device_node *node,
+					   struct __thermal_bind_params *__tbp,
+					   struct thermal_bind_params *tbp)
+{
+	const char *cdev_name;
+	int ret;
+	u32 prop;
+
+	ret = of_property_read_u32(node, "weight", &prop);
+	if (ret < 0) {
+		dev_err(dev, "missing weight property\n");
+		return ret;
+	}
+	tbp->weight = prop;
+
+	ret = of_property_read_u32(node, "mask", &prop);
+	if (ret < 0) {
+		dev_err(dev, "missing mask property\n");
+		return ret;
+	}
+	tbp->trip_mask = prop;
+
+	/* this will allow us to bind with cooling devices */
+	tbp->match = thermal_of_match;
+
+	ret = of_property_read_string(node, "cooling_device", &cdev_name);
+	if (ret < 0) {
+		dev_err(dev, "missing cooling_device property\n");
+		return ret;
+	}
+	strncpy(__tbp->cooling_device, cdev_name,
+		sizeof(__tbp->cooling_device));
+
+	return 0;
+}
+
+struct __thermal_trip {
+	unsigned long int temperature;
+	unsigned long int hysteresis;
+	enum thermal_trip_type type;
+};
+
+static
+int thermal_of_populate_trip(struct device *dev,
+			     struct device_node *node,
+			     struct __thermal_trip *trip)
+{
+	int ret;
+	int prop;
+
+	ret = of_property_read_u32(node, "temperature", &prop);
+	if (ret < 0) {
+		dev_err(dev, "missing temperature property\n");
+		return ret;
+	}
+	trip->temperature = prop;
+
+	ret = of_property_read_u32(node, "hysteresis", &prop);
+	if (ret < 0) {
+		dev_err(dev, "missing hysteresis property\n");
+		return ret;
+	}
+	trip->hysteresis = prop;
+
+	ret = of_property_read_u32(node, "type", &prop);
+	if (ret < 0) {
+		dev_err(dev, "missing type property\n");
+		return ret;
+	}
+	trip->type = prop;
+
+	return 0;
+}
+
+struct __thermal_zone_device {
+	enum thermal_device_mode mode;
+	int passive_delay;
+	int polling_delay;
+	int mask;
+	int ntrips;
+	char type[THERMAL_NAME_LENGTH];
+	struct __thermal_trip *trips;
+	struct __thermal_bind_params *bind_params;
+	struct thermal_bind_params *tbps;
+	struct thermal_zone_params zone_params;
+	int (*get_temp)(void *, unsigned long *);
+	void *devdata;
+};
+
+static
+struct __thermal_zone_device *thermal_of_build_thermal_zone(struct device *dev)
+{
+	struct device_node *child, *gchild, *node;
+	struct __thermal_zone_device *tz;
+	const char *name;
+	int ret, i;
+	u32 prop;
+
+	node = dev->of_node;
+	if (!node)
+		return ERR_PTR(-EINVAL);
+
+	node = of_find_node_by_name(node, "thermal_zone");
+	if (!node) {
+		dev_err(dev, "no thermal_zone node found\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	tz = devm_kzalloc(dev, sizeof(*tz), GFP_KERNEL);
+	if (!tz) {
+		dev_err(dev, "not enough memory for thermal of zone\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	ret = of_property_read_u32(node, "passive_delay", &prop);
+	if (ret < 0) {
+		dev_err(dev, "missing passive_delay property\n");
+		return ERR_PTR(ret);
+	}
+	tz->passive_delay = prop;
+
+	ret = of_property_read_u32(node, "polling_delay", &prop);
+	if (ret < 0) {
+		dev_err(dev, "missing polling_delay property\n");
+		return ERR_PTR(ret);
+	}
+	tz->polling_delay = prop;
+
+	ret = of_property_read_u32(node, "mask", &prop);
+	if (ret < 0) {
+		dev_err(dev, "missing mask property\n");
+		return ERR_PTR(ret);
+	}
+	tz->mask = prop;
+
+	ret = of_property_read_string(node, "type", &name);
+	if (ret < 0) {
+		dev_err(dev, "missing type property\n");
+		return ERR_PTR(ret);
+	}
+	strncpy(tz->type, name, sizeof(tz->type));
+
+	ret = of_property_read_string(node, "governor", &name);
+	if (ret < 0) {
+		dev_err(dev, "missing governor property\n");
+		return ERR_PTR(ret);
+	}
+	strncpy(tz->zone_params.governor_name, name,
+		sizeof(tz->zone_params.governor_name));
+
+	/* trips */
+	child = of_find_node_by_name(node, "trips");
+	tz->ntrips = of_get_child_count(child);
+	tz->trips = devm_kzalloc(dev, tz->ntrips * sizeof(*tz->trips),
+				 GFP_KERNEL);
+	if (!tz->trips)
+		return ERR_PTR(-ENOMEM);
+	i = 0;
+	for_each_child_of_node(child, gchild)
+		thermal_of_populate_trip(dev, gchild, &tz->trips[i++]);
+
+	/* bind_params */
+	child = of_find_node_by_name(node, "bind_params");
+	tz->zone_params.num_tbps = of_get_child_count(child);
+	tz->bind_params = devm_kzalloc(dev,
+				       tz->zone_params.num_tbps *
+						sizeof(*tz->bind_params),
+				       GFP_KERNEL);
+	if (!tz->bind_params)
+		return ERR_PTR(-ENOMEM);
+	tz->zone_params.tbp = devm_kzalloc(dev,
+					   tz->zone_params.num_tbps *
+						sizeof(*tz->zone_params.tbp),
+					   GFP_KERNEL);
+	if (!tz->zone_params.tbp)
+		return ERR_PTR(-ENOMEM);
+	i = 0;
+	for_each_child_of_node(child, gchild) {
+		thermal_of_populate_bind_params(dev, gchild,
+						&tz->bind_params[i],
+						&tz->zone_params.tbp[i]);
+		i++;
+	}
+	tz->mode = THERMAL_DEVICE_ENABLED;
+
+	return tz;
+}
+
+static
+int thermal_of_match(struct thermal_zone_device *tz,
+		     struct thermal_cooling_device *cdev)
+{
+	struct __thermal_zone_device *data = tz->devdata;
+	int i;
+
+	for (i = 0; i < data->zone_params.num_tbps; i++) {
+		if (!strncmp(data->bind_params[i].cooling_device,
+			     cdev->type,
+			     strlen(data->bind_params[i].cooling_device)) &&
+		    (data->zone_params.tbp[i].trip_mask & (1 << i)))
+			return 0;
+	}
+
+	return -EINVAL;
+}
+
+static
+int of_thermal_get_temp(struct thermal_zone_device *tz,
+			unsigned long *temp)
+{
+	struct __thermal_zone_device *data = tz->devdata;
+
+	return data->get_temp(data->devdata, temp);
+}
+
+static
+int of_thermal_get_mode(struct thermal_zone_device *tz,
+			enum thermal_device_mode *mode)
+{
+	struct __thermal_zone_device *data = tz->devdata;
+
+	*mode = data->mode;
+
+	return 0;
+}
+
+static
+int of_thermal_set_mode(struct thermal_zone_device *tz,
+			enum thermal_device_mode mode)
+{
+	struct __thermal_zone_device *data = tz->devdata;
+
+	mutex_lock(&tz->lock);
+
+	if (mode == THERMAL_DEVICE_ENABLED)
+		tz->polling_delay = data->polling_delay;
+	else
+		tz->polling_delay = 0;
+
+	mutex_unlock(&tz->lock);
+
+	data->mode = mode;
+	thermal_zone_device_update(tz);
+
+	return 0;
+}
+
+static
+int of_thermal_get_trip_type(struct thermal_zone_device *tz, int trip,
+			     enum thermal_trip_type *type)
+{
+	struct __thermal_zone_device *data = tz->devdata;
+
+	if (trip >= data->ntrips || trip < 0)
+		return -ERANGE;
+
+	*type = data->trips[trip].type;
+
+	return 0;
+}
+
+static
+int of_thermal_get_trip_temp(struct thermal_zone_device *tz, int trip,
+			     unsigned long *temp)
+{
+	struct __thermal_zone_device *data = tz->devdata;
+
+	if (trip >= data->ntrips || trip < 0)
+		return -ERANGE;
+
+	*temp = data->trips[trip].temperature;
+
+	return 0;
+}
+
+static
+int of_thermal_set_trip_temp(struct thermal_zone_device *tz, int trip,
+			     unsigned long temp)
+{
+	struct __thermal_zone_device *data = tz->devdata;
+
+	if (trip >= data->ntrips || trip < 0)
+		return -ERANGE;
+
+	/* thermal fw should take care of data->mask & (1 << trip) */
+	data->trips[trip].temperature = temp;
+
+	return 0;
+}
+
+static
+int of_thermal_get_trip_hyst(struct thermal_zone_device *tz, int trip,
+			     unsigned long *hyst)
+{
+	struct __thermal_zone_device *data = tz->devdata;
+
+	if (trip >= data->ntrips || trip < 0)
+		return -ERANGE;
+
+	*hyst = data->trips[trip].hysteresis;
+
+	return 0;
+}
+
+static
+int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip,
+			     unsigned long hyst)
+{
+	struct __thermal_zone_device *data = tz->devdata;
+
+	if (trip >= data->ntrips || trip < 0)
+		return -ERANGE;
+
+	/* thermal fw should take care of data->mask & (1 << trip) */
+	data->trips[trip].hysteresis = hyst;
+
+	return 0;
+}
+
+static int
+of_thermal_get_crit_temp(struct thermal_zone_device *tz, unsigned long *temp)
+{
+	struct __thermal_zone_device *data = tz->devdata;
+	int i;
+
+	for (i = 0; i < data->ntrips; i++)
+		if (data->trips[i].type == THERMAL_TRIP_CRITICAL) {
+			*temp = data->trips[i].temperature;
+			return 0;
+		}
+
+	return -EINVAL;
+}
+
+static const
+struct thermal_zone_device_ops of_thermal_ops = {
+	.get_temp = of_thermal_get_temp,
+	.get_mode = of_thermal_get_mode,
+	.set_mode = of_thermal_set_mode,
+	.get_trip_type = of_thermal_get_trip_type,
+	.get_trip_temp = of_thermal_get_trip_temp,
+	.set_trip_temp = of_thermal_set_trip_temp,
+	.get_trip_hyst = of_thermal_get_trip_hyst,
+	.set_trip_hyst = of_thermal_set_trip_hyst,
+	.get_crit_temp = of_thermal_get_crit_temp,
+};
+
+struct thermal_zone_device *thermal_zone_of_device_register(struct device *dev,
+		void *data, int (*get_temp)(void *, unsigned long *))
+{
+	struct __thermal_zone_device *tz;
+	struct thermal_zone_device_ops *ops;
+
+	tz = thermal_of_build_thermal_zone(dev);
+	if (IS_ERR(tz)) {
+		dev_err(dev, "failed to build thermal zone\n");
+		return NULL;
+	}
+	ops = devm_kzalloc(dev, sizeof(*ops), GFP_KERNEL);
+	if (!ops) {
+		dev_err(dev, "no memory available for thermal ops\n");
+		return NULL;
+	}
+	memcpy(ops, &of_thermal_ops, sizeof(*ops));
+	tz->get_temp = get_temp;
+	tz->devdata = data;
+
+	return thermal_zone_device_register(tz->type, tz->ntrips, tz->mask, tz,
+					    ops, &tz->zone_params,
+					    tz->passive_delay,
+					    tz->polling_delay);
+}
+EXPORT_SYMBOL_GPL(thermal_zone_of_device_register);
diff --git a/drivers/thermal/thermal_dt.h b/drivers/thermal/thermal_dt.h
new file mode 100644
index 0000000..5491d19
--- /dev/null
+++ b/drivers/thermal/thermal_dt.h
@@ -0,0 +1,44 @@
+/*
+ *  thermal_dt.h - Generic Thermal Management device tree support.
+ *
+ *  Copyright (C) 2013 Texas Instruments
+ *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
+ *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+#ifndef __THERMAL_DT_H__
+#define __THERMAL_DT_H__
+#include <linux/device.h>
+
+#ifdef CONFIG_THERMAL_OF
+struct thermal_bind_params *thermal_of_build_bind_params(struct device *dev);
+struct thermal_zone_device *thermal_of_build_thermal_zone(struct device *dev);
+#else
+static
+struct thermal_bind_params *thermal_of_build_bind_params(struct device *dev)
+{
+	return NULL;
+}
+
+static
+struct thermal_zone_device *thermal_of_build_thermal_zone(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
+#endif /* __THERMAL_DT_H__ */
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index a386a1c..a62ada0 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -224,6 +224,9 @@ struct thermal_genl_event {
 };
 
 /* Function declarations */
+struct thermal_zone_device
+*thermal_zone_of_device_register(struct device *, void *data,
+	int (*get_temp) (void *, unsigned long *));
 struct thermal_zone_device *thermal_zone_device_register(const char *, int, int,
 		void *, const struct thermal_zone_device_ops *,
 		const struct thermal_zone_params *, int, int);
-- 
1.8.2.1.342.gfa7285d

RE: [RFC PATCH 2/4] thermal: introduce device tree parser

[R, Durgadoss]

Hi Eduardo,

> -----Original Message-----
> From: Eduardo Valentin [mailto:eduardo.valentin@ti.com]
> Sent: Tuesday, July 09, 2013 7:30 PM
> To: linux-pm@vger.kernel.org; R, Durgadoss; amit.daniel@samsung.com
> Cc: Zhang, Rui; Eduardo Valentin; linux-kernel@vger.kernel.org
> Subject: [RFC PATCH 2/4] thermal: introduce device tree parser
> 
> In order to be able to build thermal policies
> based on generic sensors, like I2C device, that
> can be places in different points on different boards,
> there is a need to have a way to feed board dependent
> data into the thermal framework.
> 
> This patch introduces a thermal data parser for device
> tree. The parsed data is used to build thermal zones
> and thermal binding parameters. The output data
> can then be used to deploy thermal policies.
> 
> This patch adds also documentation regarding this
> API and how to define define tree nodes to use
> this infrastructure.
> 
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> ---

I looked at the Documentation part of this. And it looks good.

At some places you are using ERANGE. Technically, this represents
'Math result not representable'. May be should be use EINVAL 
itself ? I would leave it up to you ;)

Thanks,
Durga

>  .../devicetree/bindings/thermal/thermal.txt        |  92 +++++
>  drivers/thermal/Kconfig                            |  13 +
>  drivers/thermal/Makefile                           |   1 +
>  drivers/thermal/thermal_dt.c                       | 412 +++++++++++++++++++++
>  drivers/thermal/thermal_dt.h                       |  44 +++
>  include/linux/thermal.h                            |   3 +
>  6 files changed, 565 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/thermal.txt
>  create mode 100644 drivers/thermal/thermal_dt.c
>  create mode 100644 drivers/thermal/thermal_dt.h
> 
> diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt
> b/Documentation/devicetree/bindings/thermal/thermal.txt
> new file mode 100644
> index 0000000..2c25ab2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
> @@ -0,0 +1,92 @@
> +----------------------------------------
> +Thermal Framework Device Tree descriptor
> +----------------------------------------
> +
> +This file describes how to define a thermal structure using device tree.
> +A thermal structure includes thermal zones and their components, such
> +as name, governor, trip points, polling intervals and cooling devices
> +binding descriptors. A binding descriptor may contain information on
> +how to react, with a cooling stepped action or a weight on a fair share.
> +
> +****
> +trip
> +****
> +
> +The trip node is a node to describe a point in the temperature domain
> +in which the system takes an action. This node describes just the point,
> +not the action.
> +
> +A node describing a trip must contain:
> +- temperature: the trip temperature level, in milliCelsius.
> +- hysteresis: a (low) hysteresis value on 'temperature'. This is a relative
> +value, in milliCelsius.
> +- type: the trip type. Here is the type mapping:
> +	THERMAL_TRIP_ACTIVE = 0
> +	THERMAL_TRIP_PASSIVE = 1
> +	THERMAL_TRIP_HOT = 2
> +	THERMAL_TRIP_CRITICAL = 3
> +
> +**********
> +bind_param
> +**********
> +
> +The bind_param node is a node to describe how cooling devices get assigned
> +to trip points of the zone. The cooling devices are expected to be loaded
> +in the target system.
> +
> +A node describing a bind_param must contain:
> +- cooling_device: A string with the cooling device name.
> +- weight: The 'influence' of a particular cooling device on this zone.
> +             This is on a percentage scale. The sum of all these weights
> +             (for a particular zone) cannot exceed 100.
> +- trip_mask: This is a bit mask that gives the binding relation between
> +               this thermal zone and cdev, for a particular trip point.
> +               If nth bit is set, then the cdev and thermal zone are bound
> +               for trip point n.
> +
> +************
> +thermal_zone
> +************
> +
> +The thermal_zone node is the node containing all the required info
> +for describing a thermal zone, including its cdev bindings. The thermal_zone
> +node must contain, apart from its own properties, one node containing
> +trip nodes and one node containing all the zone bind parameters.
> +
> +Required properties:
> +- type: this is a string containing the zone type. Say 'cpu', 'core', 'mem', etc.
> +- mask: Bit string: If 'n'th bit is set, then trip point 'n' is writeable.
> +
> +- passive_delay: number of milliseconds to wait between polls when
> +	performing passive cooling.
> +- polling_delay: number of milliseconds to wait between polls when checking
> +
> +- governor: A string containing the default governor for this zone.
> +
> +Below is an example:
> +thermal_zone {
> +            type = "CPU";
> +            mask = <0x03>; /* trips writability */
> +            passive_delay = <250>; /* milliseconds */
> +            polling_delay = <1000>; /* milliseconds */
> +            governor = "step_wise";
> +            trips {
> +                    alert@100000{
> +                            temperature = <100000>; /* milliCelsius */
> +                            hysteresis = <0>; /* milliCelsius */
> +                            type = <1>;
> +                    };
> +                    crit@125000{
> +                            temperature = <125000>; /* milliCelsius */
> +                            hysteresis = <0>; /* milliCelsius */
> +                            type = <3>;
> +                    };
> +            };
> +            bind_params {
> +                    action@0{
> +                            cooling_device = "thermal-cpufreq";
> +                            weight = <100>; /* percentage */
> +                            mask = <0x01>;
> +                    };
> +            };
> +};
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 7fb16bc..753f0dc 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -29,6 +29,19 @@ config THERMAL_HWMON
>  	  Say 'Y' here if you want all thermal sensors to
>  	  have hwmon sysfs interface too.
> 
> +config THERMAL_OF
> +	bool
> +	prompt "APIs to parse thermal data out of device tree"
> +	depends on OF
> +	default y
> +	help
> +	  This options provides helpers to add the support to
> +	  read and parse thermal data definitions out of the
> +	  device tree blob.
> +
> +	  Say 'Y' here if you need to build thermal infrastructure
> +	  based on device tree.
> +
>  choice
>  	prompt "Default Thermal governor"
>  	default THERMAL_DEFAULT_GOV_STEP_WISE
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 24cb894..eedb273 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -7,6 +7,7 @@ thermal_sys-y			+= thermal_core.o
> 
>  # interface to/from other layers providing sensors
>  thermal_sys-$(CONFIG_THERMAL_HWMON)		+= thermal_hwmon.o
> +thermal_sys-$(CONFIG_THERMAL_OF)		+= thermal_dt.o
> 
>  # governors
>  thermal_sys-$(CONFIG_THERMAL_GOV_FAIR_SHARE)	+= fair_share.o
> diff --git a/drivers/thermal/thermal_dt.c b/drivers/thermal/thermal_dt.c
> new file mode 100644
> index 0000000..6553582
> --- /dev/null
> +++ b/drivers/thermal/thermal_dt.c
> @@ -0,0 +1,412 @@
> +/*
> + *  thermal_dt.c - Generic Thermal Management device tree support.
> + *
> + *  Copyright (C) 2013 Texas Instruments
> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
> + *
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; version 2 of the License.
> + *
> + *  This program is distributed in the hope that it will be useful, but
> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> + *
> + *
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~
> + */
> +#include <linux/thermal.h>
> +#include <linux/slab.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/string.h>
> +
> +struct __thermal_bind_params {
> +	char cooling_device[THERMAL_NAME_LENGTH];
> +};
> +
> +static
> +int thermal_of_match(struct thermal_zone_device *tz,
> +		     struct thermal_cooling_device *cdev);
> +
> +static int thermal_of_populate_bind_params(struct device *dev,
> +					   struct device_node *node,
> +					   struct __thermal_bind_params *__tbp,
> +					   struct thermal_bind_params *tbp)
> +{
> +	const char *cdev_name;
> +	int ret;
> +	u32 prop;
> +
> +	ret = of_property_read_u32(node, "weight", &prop);
> +	if (ret < 0) {
> +		dev_err(dev, "missing weight property\n");
> +		return ret;
> +	}
> +	tbp->weight = prop;
> +
> +	ret = of_property_read_u32(node, "mask", &prop);
> +	if (ret < 0) {
> +		dev_err(dev, "missing mask property\n");
> +		return ret;
> +	}
> +	tbp->trip_mask = prop;
> +
> +	/* this will allow us to bind with cooling devices */
> +	tbp->match = thermal_of_match;
> +
> +	ret = of_property_read_string(node, "cooling_device", &cdev_name);
> +	if (ret < 0) {
> +		dev_err(dev, "missing cooling_device property\n");
> +		return ret;
> +	}
> +	strncpy(__tbp->cooling_device, cdev_name,
> +		sizeof(__tbp->cooling_device));
> +
> +	return 0;
> +}
> +
> +struct __thermal_trip {
> +	unsigned long int temperature;
> +	unsigned long int hysteresis;
> +	enum thermal_trip_type type;
> +};
> +
> +static
> +int thermal_of_populate_trip(struct device *dev,
> +			     struct device_node *node,
> +			     struct __thermal_trip *trip)
> +{
> +	int ret;
> +	int prop;
> +
> +	ret = of_property_read_u32(node, "temperature", &prop);
> +	if (ret < 0) {
> +		dev_err(dev, "missing temperature property\n");
> +		return ret;
> +	}
> +	trip->temperature = prop;
> +
> +	ret = of_property_read_u32(node, "hysteresis", &prop);
> +	if (ret < 0) {
> +		dev_err(dev, "missing hysteresis property\n");
> +		return ret;
> +	}
> +	trip->hysteresis = prop;
> +
> +	ret = of_property_read_u32(node, "type", &prop);
> +	if (ret < 0) {
> +		dev_err(dev, "missing type property\n");
> +		return ret;
> +	}
> +	trip->type = prop;
> +
> +	return 0;
> +}
> +
> +struct __thermal_zone_device {
> +	enum thermal_device_mode mode;
> +	int passive_delay;
> +	int polling_delay;
> +	int mask;
> +	int ntrips;
> +	char type[THERMAL_NAME_LENGTH];
> +	struct __thermal_trip *trips;
> +	struct __thermal_bind_params *bind_params;
> +	struct thermal_bind_params *tbps;
> +	struct thermal_zone_params zone_params;
> +	int (*get_temp)(void *, unsigned long *);
> +	void *devdata;
> +};
> +
> +static
> +struct __thermal_zone_device *thermal_of_build_thermal_zone(struct device
> *dev)
> +{
> +	struct device_node *child, *gchild, *node;
> +	struct __thermal_zone_device *tz;
> +	const char *name;
> +	int ret, i;
> +	u32 prop;
> +
> +	node = dev->of_node;
> +	if (!node)
> +		return ERR_PTR(-EINVAL);
> +
> +	node = of_find_node_by_name(node, "thermal_zone");
> +	if (!node) {
> +		dev_err(dev, "no thermal_zone node found\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	tz = devm_kzalloc(dev, sizeof(*tz), GFP_KERNEL);
> +	if (!tz) {
> +		dev_err(dev, "not enough memory for thermal of zone\n");
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	ret = of_property_read_u32(node, "passive_delay", &prop);
> +	if (ret < 0) {
> +		dev_err(dev, "missing passive_delay property\n");
> +		return ERR_PTR(ret);
> +	}
> +	tz->passive_delay = prop;
> +
> +	ret = of_property_read_u32(node, "polling_delay", &prop);
> +	if (ret < 0) {
> +		dev_err(dev, "missing polling_delay property\n");
> +		return ERR_PTR(ret);
> +	}
> +	tz->polling_delay = prop;
> +
> +	ret = of_property_read_u32(node, "mask", &prop);
> +	if (ret < 0) {
> +		dev_err(dev, "missing mask property\n");
> +		return ERR_PTR(ret);
> +	}
> +	tz->mask = prop;
> +
> +	ret = of_property_read_string(node, "type", &name);
> +	if (ret < 0) {
> +		dev_err(dev, "missing type property\n");
> +		return ERR_PTR(ret);
> +	}
> +	strncpy(tz->type, name, sizeof(tz->type));
> +
> +	ret = of_property_read_string(node, "governor", &name);
> +	if (ret < 0) {
> +		dev_err(dev, "missing governor property\n");
> +		return ERR_PTR(ret);
> +	}
> +	strncpy(tz->zone_params.governor_name, name,
> +		sizeof(tz->zone_params.governor_name));
> +
> +	/* trips */
> +	child = of_find_node_by_name(node, "trips");
> +	tz->ntrips = of_get_child_count(child);
> +	tz->trips = devm_kzalloc(dev, tz->ntrips * sizeof(*tz->trips),
> +				 GFP_KERNEL);
> +	if (!tz->trips)
> +		return ERR_PTR(-ENOMEM);
> +	i = 0;
> +	for_each_child_of_node(child, gchild)
> +		thermal_of_populate_trip(dev, gchild, &tz->trips[i++]);
> +
> +	/* bind_params */
> +	child = of_find_node_by_name(node, "bind_params");
> +	tz->zone_params.num_tbps = of_get_child_count(child);
> +	tz->bind_params = devm_kzalloc(dev,
> +				       tz->zone_params.num_tbps *
> +						sizeof(*tz->bind_params),
> +				       GFP_KERNEL);
> +	if (!tz->bind_params)
> +		return ERR_PTR(-ENOMEM);
> +	tz->zone_params.tbp = devm_kzalloc(dev,
> +					   tz->zone_params.num_tbps *
> +						sizeof(*tz->zone_params.tbp),
> +					   GFP_KERNEL);
> +	if (!tz->zone_params.tbp)
> +		return ERR_PTR(-ENOMEM);
> +	i = 0;
> +	for_each_child_of_node(child, gchild) {
> +		thermal_of_populate_bind_params(dev, gchild,
> +						&tz->bind_params[i],
> +						&tz->zone_params.tbp[i]);
> +		i++;
> +	}
> +	tz->mode = THERMAL_DEVICE_ENABLED;
> +
> +	return tz;
> +}
> +
> +static
> +int thermal_of_match(struct thermal_zone_device *tz,
> +		     struct thermal_cooling_device *cdev)
> +{
> +	struct __thermal_zone_device *data = tz->devdata;
> +	int i;
> +
> +	for (i = 0; i < data->zone_params.num_tbps; i++) {
> +		if (!strncmp(data->bind_params[i].cooling_device,
> +			     cdev->type,
> +			     strlen(data->bind_params[i].cooling_device)) &&
> +		    (data->zone_params.tbp[i].trip_mask & (1 << i)))
> +			return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static
> +int of_thermal_get_temp(struct thermal_zone_device *tz,
> +			unsigned long *temp)
> +{
> +	struct __thermal_zone_device *data = tz->devdata;
> +
> +	return data->get_temp(data->devdata, temp);
> +}
> +
> +static
> +int of_thermal_get_mode(struct thermal_zone_device *tz,
> +			enum thermal_device_mode *mode)
> +{
> +	struct __thermal_zone_device *data = tz->devdata;
> +
> +	*mode = data->mode;
> +
> +	return 0;
> +}
> +
> +static
> +int of_thermal_set_mode(struct thermal_zone_device *tz,
> +			enum thermal_device_mode mode)
> +{
> +	struct __thermal_zone_device *data = tz->devdata;
> +
> +	mutex_lock(&tz->lock);
> +
> +	if (mode == THERMAL_DEVICE_ENABLED)
> +		tz->polling_delay = data->polling_delay;
> +	else
> +		tz->polling_delay = 0;
> +
> +	mutex_unlock(&tz->lock);
> +
> +	data->mode = mode;
> +	thermal_zone_device_update(tz);
> +
> +	return 0;
> +}
> +
> +static
> +int of_thermal_get_trip_type(struct thermal_zone_device *tz, int trip,
> +			     enum thermal_trip_type *type)
> +{
> +	struct __thermal_zone_device *data = tz->devdata;
> +
> +	if (trip >= data->ntrips || trip < 0)
> +		return -ERANGE;
> +
> +	*type = data->trips[trip].type;
> +
> +	return 0;
> +}
> +
> +static
> +int of_thermal_get_trip_temp(struct thermal_zone_device *tz, int trip,
> +			     unsigned long *temp)
> +{
> +	struct __thermal_zone_device *data = tz->devdata;
> +
> +	if (trip >= data->ntrips || trip < 0)
> +		return -ERANGE;
> +
> +	*temp = data->trips[trip].temperature;
> +
> +	return 0;
> +}
> +
> +static
> +int of_thermal_set_trip_temp(struct thermal_zone_device *tz, int trip,
> +			     unsigned long temp)
> +{
> +	struct __thermal_zone_device *data = tz->devdata;
> +
> +	if (trip >= data->ntrips || trip < 0)
> +		return -ERANGE;
> +
> +	/* thermal fw should take care of data->mask & (1 << trip) */
> +	data->trips[trip].temperature = temp;
> +
> +	return 0;
> +}
> +
> +static
> +int of_thermal_get_trip_hyst(struct thermal_zone_device *tz, int trip,
> +			     unsigned long *hyst)
> +{
> +	struct __thermal_zone_device *data = tz->devdata;
> +
> +	if (trip >= data->ntrips || trip < 0)
> +		return -ERANGE;
> +
> +	*hyst = data->trips[trip].hysteresis;
> +
> +	return 0;
> +}
> +
> +static
> +int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip,
> +			     unsigned long hyst)
> +{
> +	struct __thermal_zone_device *data = tz->devdata;
> +
> +	if (trip >= data->ntrips || trip < 0)
> +		return -ERANGE;
> +
> +	/* thermal fw should take care of data->mask & (1 << trip) */
> +	data->trips[trip].hysteresis = hyst;
> +
> +	return 0;
> +}
> +
> +static int
> +of_thermal_get_crit_temp(struct thermal_zone_device *tz, unsigned long
> *temp)
> +{
> +	struct __thermal_zone_device *data = tz->devdata;
> +	int i;
> +
> +	for (i = 0; i < data->ntrips; i++)
> +		if (data->trips[i].type == THERMAL_TRIP_CRITICAL) {
> +			*temp = data->trips[i].temperature;
> +			return 0;
> +		}
> +
> +	return -EINVAL;
> +}
> +
> +static const
> +struct thermal_zone_device_ops of_thermal_ops = {
> +	.get_temp = of_thermal_get_temp,
> +	.get_mode = of_thermal_get_mode,
> +	.set_mode = of_thermal_set_mode,
> +	.get_trip_type = of_thermal_get_trip_type,
> +	.get_trip_temp = of_thermal_get_trip_temp,
> +	.set_trip_temp = of_thermal_set_trip_temp,
> +	.get_trip_hyst = of_thermal_get_trip_hyst,
> +	.set_trip_hyst = of_thermal_set_trip_hyst,
> +	.get_crit_temp = of_thermal_get_crit_temp,
> +};
> +
> +struct thermal_zone_device *thermal_zone_of_device_register(struct device
> *dev,
> +		void *data, int (*get_temp)(void *, unsigned long *))
> +{
> +	struct __thermal_zone_device *tz;
> +	struct thermal_zone_device_ops *ops;
> +
> +	tz = thermal_of_build_thermal_zone(dev);
> +	if (IS_ERR(tz)) {
> +		dev_err(dev, "failed to build thermal zone\n");
> +		return NULL;
> +	}
> +	ops = devm_kzalloc(dev, sizeof(*ops), GFP_KERNEL);
> +	if (!ops) {
> +		dev_err(dev, "no memory available for thermal ops\n");
> +		return NULL;
> +	}
> +	memcpy(ops, &of_thermal_ops, sizeof(*ops));
> +	tz->get_temp = get_temp;
> +	tz->devdata = data;
> +
> +	return thermal_zone_device_register(tz->type, tz->ntrips, tz->mask, tz,
> +					    ops, &tz->zone_params,
> +					    tz->passive_delay,
> +					    tz->polling_delay);
> +}
> +EXPORT_SYMBOL_GPL(thermal_zone_of_device_register);
> diff --git a/drivers/thermal/thermal_dt.h b/drivers/thermal/thermal_dt.h
> new file mode 100644
> index 0000000..5491d19
> --- /dev/null
> +++ b/drivers/thermal/thermal_dt.h
> @@ -0,0 +1,44 @@
> +/*
> + *  thermal_dt.h - Generic Thermal Management device tree support.
> + *
> + *  Copyright (C) 2013 Texas Instruments
> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
> + *
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; version 2 of the License.
> + *
> + *  This program is distributed in the hope that it will be useful, but
> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> + *
> + *
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~
> + */
> +#ifndef __THERMAL_DT_H__
> +#define __THERMAL_DT_H__
> +#include <linux/device.h>
> +
> +#ifdef CONFIG_THERMAL_OF
> +struct thermal_bind_params *thermal_of_build_bind_params(struct device
> *dev);
> +struct thermal_zone_device *thermal_of_build_thermal_zone(struct device
> *dev);
> +#else
> +static
> +struct thermal_bind_params *thermal_of_build_bind_params(struct device
> *dev)
> +{
> +	return NULL;
> +}
> +
> +static
> +struct thermal_zone_device *thermal_of_build_thermal_zone(struct device
> *dev)
> +{
> +	return NULL;
> +}
> +#endif
> +
> +#endif /* __THERMAL_DT_H__ */
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index a386a1c..a62ada0 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -224,6 +224,9 @@ struct thermal_genl_event {
>  };
> 
>  /* Function declarations */
> +struct thermal_zone_device
> +*thermal_zone_of_device_register(struct device *, void *data,
> +	int (*get_temp) (void *, unsigned long *));
>  struct thermal_zone_device *thermal_zone_device_register(const char *, int,
> int,
>  		void *, const struct thermal_zone_device_ops *,
>  		const struct thermal_zone_params *, int, int);
> --
> 1.8.2.1.342.gfa7285d

Re: [RFC PATCH 2/4] thermal: introduce device tree parser

[Eduardo Valentin]

[-- Attachment #1: Type: text/plain, Size: 22950 bytes --]

On 09-07-2013 12:14, R, Durgadoss wrote:
> Hi Eduardo,
> 
>> -----Original Message-----
>> From: Eduardo Valentin [mailto:eduardo.valentin@ti.com]
>> Sent: Tuesday, July 09, 2013 7:30 PM
>> To: linux-pm@vger.kernel.org; R, Durgadoss; amit.daniel@samsung.com
>> Cc: Zhang, Rui; Eduardo Valentin; linux-kernel@vger.kernel.org
>> Subject: [RFC PATCH 2/4] thermal: introduce device tree parser
>>
>> In order to be able to build thermal policies
>> based on generic sensors, like I2C device, that
>> can be places in different points on different boards,
>> there is a need to have a way to feed board dependent
>> data into the thermal framework.
>>
>> This patch introduces a thermal data parser for device
>> tree. The parsed data is used to build thermal zones
>> and thermal binding parameters. The output data
>> can then be used to deploy thermal policies.
>>
>> This patch adds also documentation regarding this
>> API and how to define define tree nodes to use
>> this infrastructure.
>>
>> Cc: Zhang Rui <rui.zhang@intel.com>
>> Cc: linux-pm@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
>> ---
> 
> I looked at the Documentation part of this. And it looks good.
> 
> At some places you are using ERANGE. Technically, this represents
> 'Math result not representable'. May be should be use EINVAL 
> itself ? I would leave it up to you ;)

I will be using EDOM in next version, tks

> 
> Thanks,
> Durga
> 
>>  .../devicetree/bindings/thermal/thermal.txt        |  92 +++++
>>  drivers/thermal/Kconfig                            |  13 +
>>  drivers/thermal/Makefile                           |   1 +
>>  drivers/thermal/thermal_dt.c                       | 412 +++++++++++++++++++++
>>  drivers/thermal/thermal_dt.h                       |  44 +++
>>  include/linux/thermal.h                            |   3 +
>>  6 files changed, 565 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/thermal/thermal.txt
>>  create mode 100644 drivers/thermal/thermal_dt.c
>>  create mode 100644 drivers/thermal/thermal_dt.h
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt
>> b/Documentation/devicetree/bindings/thermal/thermal.txt
>> new file mode 100644
>> index 0000000..2c25ab2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
>> @@ -0,0 +1,92 @@
>> +----------------------------------------
>> +Thermal Framework Device Tree descriptor
>> +----------------------------------------
>> +
>> +This file describes how to define a thermal structure using device tree.
>> +A thermal structure includes thermal zones and their components, such
>> +as name, governor, trip points, polling intervals and cooling devices
>> +binding descriptors. A binding descriptor may contain information on
>> +how to react, with a cooling stepped action or a weight on a fair share.
>> +
>> +****
>> +trip
>> +****
>> +
>> +The trip node is a node to describe a point in the temperature domain
>> +in which the system takes an action. This node describes just the point,
>> +not the action.
>> +
>> +A node describing a trip must contain:
>> +- temperature: the trip temperature level, in milliCelsius.
>> +- hysteresis: a (low) hysteresis value on 'temperature'. This is a relative
>> +value, in milliCelsius.
>> +- type: the trip type. Here is the type mapping:
>> +	THERMAL_TRIP_ACTIVE = 0
>> +	THERMAL_TRIP_PASSIVE = 1
>> +	THERMAL_TRIP_HOT = 2
>> +	THERMAL_TRIP_CRITICAL = 3
>> +
>> +**********
>> +bind_param
>> +**********
>> +
>> +The bind_param node is a node to describe how cooling devices get assigned
>> +to trip points of the zone. The cooling devices are expected to be loaded
>> +in the target system.
>> +
>> +A node describing a bind_param must contain:
>> +- cooling_device: A string with the cooling device name.
>> +- weight: The 'influence' of a particular cooling device on this zone.
>> +             This is on a percentage scale. The sum of all these weights
>> +             (for a particular zone) cannot exceed 100.
>> +- trip_mask: This is a bit mask that gives the binding relation between
>> +               this thermal zone and cdev, for a particular trip point.
>> +               If nth bit is set, then the cdev and thermal zone are bound
>> +               for trip point n.
>> +
>> +************
>> +thermal_zone
>> +************
>> +
>> +The thermal_zone node is the node containing all the required info
>> +for describing a thermal zone, including its cdev bindings. The thermal_zone
>> +node must contain, apart from its own properties, one node containing
>> +trip nodes and one node containing all the zone bind parameters.
>> +
>> +Required properties:
>> +- type: this is a string containing the zone type. Say 'cpu', 'core', 'mem', etc.
>> +- mask: Bit string: If 'n'th bit is set, then trip point 'n' is writeable.
>> +
>> +- passive_delay: number of milliseconds to wait between polls when
>> +	performing passive cooling.
>> +- polling_delay: number of milliseconds to wait between polls when checking
>> +
>> +- governor: A string containing the default governor for this zone.
>> +
>> +Below is an example:
>> +thermal_zone {
>> +            type = "CPU";
>> +            mask = <0x03>; /* trips writability */
>> +            passive_delay = <250>; /* milliseconds */
>> +            polling_delay = <1000>; /* milliseconds */
>> +            governor = "step_wise";
>> +            trips {
>> +                    alert@100000{
>> +                            temperature = <100000>; /* milliCelsius */
>> +                            hysteresis = <0>; /* milliCelsius */
>> +                            type = <1>;
>> +                    };
>> +                    crit@125000{
>> +                            temperature = <125000>; /* milliCelsius */
>> +                            hysteresis = <0>; /* milliCelsius */
>> +                            type = <3>;
>> +                    };
>> +            };
>> +            bind_params {
>> +                    action@0{
>> +                            cooling_device = "thermal-cpufreq";
>> +                            weight = <100>; /* percentage */
>> +                            mask = <0x01>;
>> +                    };
>> +            };
>> +};
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index 7fb16bc..753f0dc 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -29,6 +29,19 @@ config THERMAL_HWMON
>>  	  Say 'Y' here if you want all thermal sensors to
>>  	  have hwmon sysfs interface too.
>>
>> +config THERMAL_OF
>> +	bool
>> +	prompt "APIs to parse thermal data out of device tree"
>> +	depends on OF
>> +	default y
>> +	help
>> +	  This options provides helpers to add the support to
>> +	  read and parse thermal data definitions out of the
>> +	  device tree blob.
>> +
>> +	  Say 'Y' here if you need to build thermal infrastructure
>> +	  based on device tree.
>> +
>>  choice
>>  	prompt "Default Thermal governor"
>>  	default THERMAL_DEFAULT_GOV_STEP_WISE
>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>> index 24cb894..eedb273 100644
>> --- a/drivers/thermal/Makefile
>> +++ b/drivers/thermal/Makefile
>> @@ -7,6 +7,7 @@ thermal_sys-y			+= thermal_core.o
>>
>>  # interface to/from other layers providing sensors
>>  thermal_sys-$(CONFIG_THERMAL_HWMON)		+= thermal_hwmon.o
>> +thermal_sys-$(CONFIG_THERMAL_OF)		+= thermal_dt.o
>>
>>  # governors
>>  thermal_sys-$(CONFIG_THERMAL_GOV_FAIR_SHARE)	+= fair_share.o
>> diff --git a/drivers/thermal/thermal_dt.c b/drivers/thermal/thermal_dt.c
>> new file mode 100644
>> index 0000000..6553582
>> --- /dev/null
>> +++ b/drivers/thermal/thermal_dt.c
>> @@ -0,0 +1,412 @@
>> +/*
>> + *  thermal_dt.c - Generic Thermal Management device tree support.
>> + *
>> + *  Copyright (C) 2013 Texas Instruments
>> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
>> + *
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ~~~~~~~~~~~~
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; version 2 of the License.
>> + *
>> + *  This program is distributed in the hope that it will be useful, but
>> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + *  General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License along
>> + *  with this program; if not, write to the Free Software Foundation, Inc.,
>> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
>> + *
>> + *
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ~~~~~~~~~~~~
>> + */
>> +#include <linux/thermal.h>
>> +#include <linux/slab.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/err.h>
>> +#include <linux/export.h>
>> +#include <linux/string.h>
>> +
>> +struct __thermal_bind_params {
>> +	char cooling_device[THERMAL_NAME_LENGTH];
>> +};
>> +
>> +static
>> +int thermal_of_match(struct thermal_zone_device *tz,
>> +		     struct thermal_cooling_device *cdev);
>> +
>> +static int thermal_of_populate_bind_params(struct device *dev,
>> +					   struct device_node *node,
>> +					   struct __thermal_bind_params *__tbp,
>> +					   struct thermal_bind_params *tbp)
>> +{
>> +	const char *cdev_name;
>> +	int ret;
>> +	u32 prop;
>> +
>> +	ret = of_property_read_u32(node, "weight", &prop);
>> +	if (ret < 0) {
>> +		dev_err(dev, "missing weight property\n");
>> +		return ret;
>> +	}
>> +	tbp->weight = prop;
>> +
>> +	ret = of_property_read_u32(node, "mask", &prop);
>> +	if (ret < 0) {
>> +		dev_err(dev, "missing mask property\n");
>> +		return ret;
>> +	}
>> +	tbp->trip_mask = prop;
>> +
>> +	/* this will allow us to bind with cooling devices */
>> +	tbp->match = thermal_of_match;
>> +
>> +	ret = of_property_read_string(node, "cooling_device", &cdev_name);
>> +	if (ret < 0) {
>> +		dev_err(dev, "missing cooling_device property\n");
>> +		return ret;
>> +	}
>> +	strncpy(__tbp->cooling_device, cdev_name,
>> +		sizeof(__tbp->cooling_device));
>> +
>> +	return 0;
>> +}
>> +
>> +struct __thermal_trip {
>> +	unsigned long int temperature;
>> +	unsigned long int hysteresis;
>> +	enum thermal_trip_type type;
>> +};
>> +
>> +static
>> +int thermal_of_populate_trip(struct device *dev,
>> +			     struct device_node *node,
>> +			     struct __thermal_trip *trip)
>> +{
>> +	int ret;
>> +	int prop;
>> +
>> +	ret = of_property_read_u32(node, "temperature", &prop);
>> +	if (ret < 0) {
>> +		dev_err(dev, "missing temperature property\n");
>> +		return ret;
>> +	}
>> +	trip->temperature = prop;
>> +
>> +	ret = of_property_read_u32(node, "hysteresis", &prop);
>> +	if (ret < 0) {
>> +		dev_err(dev, "missing hysteresis property\n");
>> +		return ret;
>> +	}
>> +	trip->hysteresis = prop;
>> +
>> +	ret = of_property_read_u32(node, "type", &prop);
>> +	if (ret < 0) {
>> +		dev_err(dev, "missing type property\n");
>> +		return ret;
>> +	}
>> +	trip->type = prop;
>> +
>> +	return 0;
>> +}
>> +
>> +struct __thermal_zone_device {
>> +	enum thermal_device_mode mode;
>> +	int passive_delay;
>> +	int polling_delay;
>> +	int mask;
>> +	int ntrips;
>> +	char type[THERMAL_NAME_LENGTH];
>> +	struct __thermal_trip *trips;
>> +	struct __thermal_bind_params *bind_params;
>> +	struct thermal_bind_params *tbps;
>> +	struct thermal_zone_params zone_params;
>> +	int (*get_temp)(void *, unsigned long *);
>> +	void *devdata;
>> +};
>> +
>> +static
>> +struct __thermal_zone_device *thermal_of_build_thermal_zone(struct device
>> *dev)
>> +{
>> +	struct device_node *child, *gchild, *node;
>> +	struct __thermal_zone_device *tz;
>> +	const char *name;
>> +	int ret, i;
>> +	u32 prop;
>> +
>> +	node = dev->of_node;
>> +	if (!node)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	node = of_find_node_by_name(node, "thermal_zone");
>> +	if (!node) {
>> +		dev_err(dev, "no thermal_zone node found\n");
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	tz = devm_kzalloc(dev, sizeof(*tz), GFP_KERNEL);
>> +	if (!tz) {
>> +		dev_err(dev, "not enough memory for thermal of zone\n");
>> +		return ERR_PTR(-ENOMEM);
>> +	}
>> +
>> +	ret = of_property_read_u32(node, "passive_delay", &prop);
>> +	if (ret < 0) {
>> +		dev_err(dev, "missing passive_delay property\n");
>> +		return ERR_PTR(ret);
>> +	}
>> +	tz->passive_delay = prop;
>> +
>> +	ret = of_property_read_u32(node, "polling_delay", &prop);
>> +	if (ret < 0) {
>> +		dev_err(dev, "missing polling_delay property\n");
>> +		return ERR_PTR(ret);
>> +	}
>> +	tz->polling_delay = prop;
>> +
>> +	ret = of_property_read_u32(node, "mask", &prop);
>> +	if (ret < 0) {
>> +		dev_err(dev, "missing mask property\n");
>> +		return ERR_PTR(ret);
>> +	}
>> +	tz->mask = prop;
>> +
>> +	ret = of_property_read_string(node, "type", &name);
>> +	if (ret < 0) {
>> +		dev_err(dev, "missing type property\n");
>> +		return ERR_PTR(ret);
>> +	}
>> +	strncpy(tz->type, name, sizeof(tz->type));
>> +
>> +	ret = of_property_read_string(node, "governor", &name);
>> +	if (ret < 0) {
>> +		dev_err(dev, "missing governor property\n");
>> +		return ERR_PTR(ret);
>> +	}
>> +	strncpy(tz->zone_params.governor_name, name,
>> +		sizeof(tz->zone_params.governor_name));
>> +
>> +	/* trips */
>> +	child = of_find_node_by_name(node, "trips");
>> +	tz->ntrips = of_get_child_count(child);
>> +	tz->trips = devm_kzalloc(dev, tz->ntrips * sizeof(*tz->trips),
>> +				 GFP_KERNEL);
>> +	if (!tz->trips)
>> +		return ERR_PTR(-ENOMEM);
>> +	i = 0;
>> +	for_each_child_of_node(child, gchild)
>> +		thermal_of_populate_trip(dev, gchild, &tz->trips[i++]);
>> +
>> +	/* bind_params */
>> +	child = of_find_node_by_name(node, "bind_params");
>> +	tz->zone_params.num_tbps = of_get_child_count(child);
>> +	tz->bind_params = devm_kzalloc(dev,
>> +				       tz->zone_params.num_tbps *
>> +						sizeof(*tz->bind_params),
>> +				       GFP_KERNEL);
>> +	if (!tz->bind_params)
>> +		return ERR_PTR(-ENOMEM);
>> +	tz->zone_params.tbp = devm_kzalloc(dev,
>> +					   tz->zone_params.num_tbps *
>> +						sizeof(*tz->zone_params.tbp),
>> +					   GFP_KERNEL);
>> +	if (!tz->zone_params.tbp)
>> +		return ERR_PTR(-ENOMEM);
>> +	i = 0;
>> +	for_each_child_of_node(child, gchild) {
>> +		thermal_of_populate_bind_params(dev, gchild,
>> +						&tz->bind_params[i],
>> +						&tz->zone_params.tbp[i]);
>> +		i++;
>> +	}
>> +	tz->mode = THERMAL_DEVICE_ENABLED;
>> +
>> +	return tz;
>> +}
>> +
>> +static
>> +int thermal_of_match(struct thermal_zone_device *tz,
>> +		     struct thermal_cooling_device *cdev)
>> +{
>> +	struct __thermal_zone_device *data = tz->devdata;
>> +	int i;
>> +
>> +	for (i = 0; i < data->zone_params.num_tbps; i++) {
>> +		if (!strncmp(data->bind_params[i].cooling_device,
>> +			     cdev->type,
>> +			     strlen(data->bind_params[i].cooling_device)) &&
>> +		    (data->zone_params.tbp[i].trip_mask & (1 << i)))
>> +			return 0;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static
>> +int of_thermal_get_temp(struct thermal_zone_device *tz,
>> +			unsigned long *temp)
>> +{
>> +	struct __thermal_zone_device *data = tz->devdata;
>> +
>> +	return data->get_temp(data->devdata, temp);
>> +}
>> +
>> +static
>> +int of_thermal_get_mode(struct thermal_zone_device *tz,
>> +			enum thermal_device_mode *mode)
>> +{
>> +	struct __thermal_zone_device *data = tz->devdata;
>> +
>> +	*mode = data->mode;
>> +
>> +	return 0;
>> +}
>> +
>> +static
>> +int of_thermal_set_mode(struct thermal_zone_device *tz,
>> +			enum thermal_device_mode mode)
>> +{
>> +	struct __thermal_zone_device *data = tz->devdata;
>> +
>> +	mutex_lock(&tz->lock);
>> +
>> +	if (mode == THERMAL_DEVICE_ENABLED)
>> +		tz->polling_delay = data->polling_delay;
>> +	else
>> +		tz->polling_delay = 0;
>> +
>> +	mutex_unlock(&tz->lock);
>> +
>> +	data->mode = mode;
>> +	thermal_zone_device_update(tz);
>> +
>> +	return 0;
>> +}
>> +
>> +static
>> +int of_thermal_get_trip_type(struct thermal_zone_device *tz, int trip,
>> +			     enum thermal_trip_type *type)
>> +{
>> +	struct __thermal_zone_device *data = tz->devdata;
>> +
>> +	if (trip >= data->ntrips || trip < 0)
>> +		return -ERANGE;
>> +
>> +	*type = data->trips[trip].type;
>> +
>> +	return 0;
>> +}
>> +
>> +static
>> +int of_thermal_get_trip_temp(struct thermal_zone_device *tz, int trip,
>> +			     unsigned long *temp)
>> +{
>> +	struct __thermal_zone_device *data = tz->devdata;
>> +
>> +	if (trip >= data->ntrips || trip < 0)
>> +		return -ERANGE;
>> +
>> +	*temp = data->trips[trip].temperature;
>> +
>> +	return 0;
>> +}
>> +
>> +static
>> +int of_thermal_set_trip_temp(struct thermal_zone_device *tz, int trip,
>> +			     unsigned long temp)
>> +{
>> +	struct __thermal_zone_device *data = tz->devdata;
>> +
>> +	if (trip >= data->ntrips || trip < 0)
>> +		return -ERANGE;
>> +
>> +	/* thermal fw should take care of data->mask & (1 << trip) */
>> +	data->trips[trip].temperature = temp;
>> +
>> +	return 0;
>> +}
>> +
>> +static
>> +int of_thermal_get_trip_hyst(struct thermal_zone_device *tz, int trip,
>> +			     unsigned long *hyst)
>> +{
>> +	struct __thermal_zone_device *data = tz->devdata;
>> +
>> +	if (trip >= data->ntrips || trip < 0)
>> +		return -ERANGE;
>> +
>> +	*hyst = data->trips[trip].hysteresis;
>> +
>> +	return 0;
>> +}
>> +
>> +static
>> +int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip,
>> +			     unsigned long hyst)
>> +{
>> +	struct __thermal_zone_device *data = tz->devdata;
>> +
>> +	if (trip >= data->ntrips || trip < 0)
>> +		return -ERANGE;
>> +
>> +	/* thermal fw should take care of data->mask & (1 << trip) */
>> +	data->trips[trip].hysteresis = hyst;
>> +
>> +	return 0;
>> +}
>> +
>> +static int
>> +of_thermal_get_crit_temp(struct thermal_zone_device *tz, unsigned long
>> *temp)
>> +{
>> +	struct __thermal_zone_device *data = tz->devdata;
>> +	int i;
>> +
>> +	for (i = 0; i < data->ntrips; i++)
>> +		if (data->trips[i].type == THERMAL_TRIP_CRITICAL) {
>> +			*temp = data->trips[i].temperature;
>> +			return 0;
>> +		}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static const
>> +struct thermal_zone_device_ops of_thermal_ops = {
>> +	.get_temp = of_thermal_get_temp,
>> +	.get_mode = of_thermal_get_mode,
>> +	.set_mode = of_thermal_set_mode,
>> +	.get_trip_type = of_thermal_get_trip_type,
>> +	.get_trip_temp = of_thermal_get_trip_temp,
>> +	.set_trip_temp = of_thermal_set_trip_temp,
>> +	.get_trip_hyst = of_thermal_get_trip_hyst,
>> +	.set_trip_hyst = of_thermal_set_trip_hyst,
>> +	.get_crit_temp = of_thermal_get_crit_temp,
>> +};
>> +
>> +struct thermal_zone_device *thermal_zone_of_device_register(struct device
>> *dev,
>> +		void *data, int (*get_temp)(void *, unsigned long *))
>> +{
>> +	struct __thermal_zone_device *tz;
>> +	struct thermal_zone_device_ops *ops;
>> +
>> +	tz = thermal_of_build_thermal_zone(dev);
>> +	if (IS_ERR(tz)) {
>> +		dev_err(dev, "failed to build thermal zone\n");
>> +		return NULL;
>> +	}
>> +	ops = devm_kzalloc(dev, sizeof(*ops), GFP_KERNEL);
>> +	if (!ops) {
>> +		dev_err(dev, "no memory available for thermal ops\n");
>> +		return NULL;
>> +	}
>> +	memcpy(ops, &of_thermal_ops, sizeof(*ops));
>> +	tz->get_temp = get_temp;
>> +	tz->devdata = data;
>> +
>> +	return thermal_zone_device_register(tz->type, tz->ntrips, tz->mask, tz,
>> +					    ops, &tz->zone_params,
>> +					    tz->passive_delay,
>> +					    tz->polling_delay);
>> +}
>> +EXPORT_SYMBOL_GPL(thermal_zone_of_device_register);
>> diff --git a/drivers/thermal/thermal_dt.h b/drivers/thermal/thermal_dt.h
>> new file mode 100644
>> index 0000000..5491d19
>> --- /dev/null
>> +++ b/drivers/thermal/thermal_dt.h
>> @@ -0,0 +1,44 @@
>> +/*
>> + *  thermal_dt.h - Generic Thermal Management device tree support.
>> + *
>> + *  Copyright (C) 2013 Texas Instruments
>> + *  Copyright (C) 2013 Eduardo Valentin <eduardo.valentin@ti.com>
>> + *
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ~~~~~~~~~~~~
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; version 2 of the License.
>> + *
>> + *  This program is distributed in the hope that it will be useful, but
>> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + *  General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License along
>> + *  with this program; if not, write to the Free Software Foundation, Inc.,
>> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
>> + *
>> + *
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ~~~~~~~~~~~~
>> + */
>> +#ifndef __THERMAL_DT_H__
>> +#define __THERMAL_DT_H__
>> +#include <linux/device.h>
>> +
>> +#ifdef CONFIG_THERMAL_OF
>> +struct thermal_bind_params *thermal_of_build_bind_params(struct device
>> *dev);
>> +struct thermal_zone_device *thermal_of_build_thermal_zone(struct device
>> *dev);
>> +#else
>> +static
>> +struct thermal_bind_params *thermal_of_build_bind_params(struct device
>> *dev)
>> +{
>> +	return NULL;
>> +}
>> +
>> +static
>> +struct thermal_zone_device *thermal_of_build_thermal_zone(struct device
>> *dev)
>> +{
>> +	return NULL;
>> +}
>> +#endif
>> +
>> +#endif /* __THERMAL_DT_H__ */
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index a386a1c..a62ada0 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -224,6 +224,9 @@ struct thermal_genl_event {
>>  };
>>
>>  /* Function declarations */
>> +struct thermal_zone_device
>> +*thermal_zone_of_device_register(struct device *, void *data,
>> +	int (*get_temp) (void *, unsigned long *));
>>  struct thermal_zone_device *thermal_zone_device_register(const char *, int,
>> int,
>>  		void *, const struct thermal_zone_device_ops *,
>>  		const struct thermal_zone_params *, int, int);
>> --
>> 1.8.2.1.342.gfa7285d
> 
> 
> 


-- 
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 295 bytes --]

Re: [RFC PATCH 2/4] thermal: introduce device tree parser

[Wei Ni]

On 07/09/2013 10:00 PM, Eduardo Valentin wrote:
> In order to be able to build thermal policies
> based on generic sensors, like I2C device, that
> can be places in different points on different boards,
> there is a need to have a way to feed board dependent
> data into the thermal framework.
> 
> This patch introduces a thermal data parser for device
> tree. The parsed data is used to build thermal zones
> and thermal binding parameters. The output data
> can then be used to deploy thermal policies.
> 
> This patch adds also documentation regarding this
> API and how to define define tree nodes to use
> this infrastructure.

It looks good, with this infrastructure, we can add generic sensor
driver into the thermal fw easily.


> +
> +Below is an example:
> +thermal_zone {
> +            type = "CPU";
> +            mask = <0x03>; /* trips writability */
> +            passive_delay = <250>; /* milliseconds */
> +            polling_delay = <1000>; /* milliseconds */
> +            governor = "step_wise";
> +            trips {
> +                    alert@100000{
> +                            temperature = <100000>; /* milliCelsius */
> +                            hysteresis = <0>; /* milliCelsius */
> +                            type = <1>;

how about to use the trip type name directly, such as named as
"passive-trip;", I think it's more readable. for example:
trip0 {
....
passive-trip;
}
trip1 {
....
active-trip;
}

> +                    };
> +                    crit@125000{
> +                            temperature = <125000>; /* milliCelsius */
> +                            hysteresis = <0>; /* milliCelsius */
> +                            type = <3>;
> +                    };
> +            };
> +            bind_params {
> +                    action@0{
> +                            cooling_device = "thermal-cpufreq";
> +                            weight = <100>; /* percentage */
> +                            mask = <0x01>;
> +                    };
> +            };
> +};

as we know, thermal_zone_bind_cooling_device() will set the upper/lower
in the thermal_instance. In the default .bind function, it just set to
THERMAL_NO_LIMIT, but for some platform, it need to set these
upper/lower values for different cooling device and trips, how to pass
these values in DT? how about to set:
action@0 {
...
mask = <0x03>; //or you can remove this property;
trip0 = <&alert 1 2>; //1 is lower value, 2 is upper value;
trip1 = <&crit 3 4>;
}


Thanks.
Wei.

Re: [RFC PATCH 2/4] thermal: introduce device tree parser

[Stephen Warren]

On 07/10/2013 12:48 AM, Wei Ni wrote:
> On 07/09/2013 10:00 PM, Eduardo Valentin wrote:
>> In order to be able to build thermal policies
>> based on generic sensors, like I2C device, that
>> can be places in different points on different boards,
>> there is a need to have a way to feed board dependent
>> data into the thermal framework.
>>
>> This patch introduces a thermal data parser for device
>> tree. The parsed data is used to build thermal zones
>> and thermal binding parameters. The output data
>> can then be used to deploy thermal policies.
>>
>> This patch adds also documentation regarding this
>> API and how to define define tree nodes to use
>> this infrastructure.
> 
> It looks good, with this infrastructure, we can add generic sensor
> driver into the thermal fw easily.
> 
> 
>> +
>> +Below is an example:
>> +thermal_zone {
>> +            type = "CPU";
>> +            mask = <0x03>; /* trips writability */
>> +            passive_delay = <250>; /* milliseconds */
>> +            polling_delay = <1000>; /* milliseconds */
>> +            governor = "step_wise";
>> +            trips {
>> +                    alert@100000{
>> +                            temperature = <100000>; /* milliCelsius */
>> +                            hysteresis = <0>; /* milliCelsius */
>> +                            type = <1>;
> 
> how about to use the trip type name directly, such as named as
> "passive-trip;", I think it's more readable. for example:
> trip0 {
> ....
> passive-trip;
> }
> trip1 {
> ....
> active-trip;
> }

You can always use the C pre-processor in DT now to define named constants:

include/dt-bindings/..../....h

	#define THERMAL_PASSIVE_TRIP 0
	#define THERMAL_ACTIVE_TRIP 1

*.dts:

	type = <THERMAL_PASSIVE_TRIP>;

Having a single 'property = value;' rather than n different Boolean
property names seems better, irrespective of whether value is an integer
or string; parsing and error-checking will be simpler.

Re: [RFC PATCH 2/4] thermal: introduce device tree parser

[Eduardo Valentin]

[-- Attachment #1: Type: text/plain, Size: 2357 bytes --]

On 10-07-2013 11:16, Stephen Warren wrote:
> On 07/10/2013 12:48 AM, Wei Ni wrote:
>> On 07/09/2013 10:00 PM, Eduardo Valentin wrote:
>>> In order to be able to build thermal policies
>>> based on generic sensors, like I2C device, that
>>> can be places in different points on different boards,
>>> there is a need to have a way to feed board dependent
>>> data into the thermal framework.
>>>
>>> This patch introduces a thermal data parser for device
>>> tree. The parsed data is used to build thermal zones
>>> and thermal binding parameters. The output data
>>> can then be used to deploy thermal policies.
>>>
>>> This patch adds also documentation regarding this
>>> API and how to define define tree nodes to use
>>> this infrastructure.
>>
>> It looks good, with this infrastructure, we can add generic sensor
>> driver into the thermal fw easily.
>>
>>
>>> +
>>> +Below is an example:
>>> +thermal_zone {
>>> +            type = "CPU";
>>> +            mask = <0x03>; /* trips writability */
>>> +            passive_delay = <250>; /* milliseconds */
>>> +            polling_delay = <1000>; /* milliseconds */
>>> +            governor = "step_wise";
>>> +            trips {
>>> +                    alert@100000{
>>> +                            temperature = <100000>; /* milliCelsius */
>>> +                            hysteresis = <0>; /* milliCelsius */
>>> +                            type = <1>;
>>
>> how about to use the trip type name directly, such as named as
>> "passive-trip;", I think it's more readable. for example:
>> trip0 {
>> ....
>> passive-trip;
>> }
>> trip1 {
>> ....
>> active-trip;
>> }
> 
> You can always use the C pre-processor in DT now to define named constants:
> 
> include/dt-bindings/..../....h
> 
> 	#define THERMAL_PASSIVE_TRIP 0
> 	#define THERMAL_ACTIVE_TRIP 1
> 
> *.dts:
> 
> 	type = <THERMAL_PASSIVE_TRIP>;
> 
> Having a single 'property = value;' rather than n different Boolean
> property names seems better, irrespective of whether value is an integer
> or string; parsing and error-checking will be simpler.

agreed here. I will amend with the above suggestions. makes the dt file
much more readable. thanks Stephen and Wei.

> 
> 


-- 
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 295 bytes --]

Re: [RFC PATCH 2/4] thermal: introduce device tree parser

[Eduardo Valentin]

[-- Attachment #1: Type: text/plain, Size: 3656 bytes --]

On 10-07-2013 02:48, Wei Ni wrote:
> On 07/09/2013 10:00 PM, Eduardo Valentin wrote:
>> In order to be able to build thermal policies
>> based on generic sensors, like I2C device, that
>> can be places in different points on different boards,
>> there is a need to have a way to feed board dependent
>> data into the thermal framework.
>>
>> This patch introduces a thermal data parser for device
>> tree. The parsed data is used to build thermal zones
>> and thermal binding parameters. The output data
>> can then be used to deploy thermal policies.
>>
>> This patch adds also documentation regarding this
>> API and how to define define tree nodes to use
>> this infrastructure.
> 
> It looks good, with this infrastructure, we can add generic sensor
> driver into the thermal fw easily.
> 
> 
>> +
>> +Below is an example:
>> +thermal_zone {
>> +            type = "CPU";
>> +            mask = <0x03>; /* trips writability */
>> +            passive_delay = <250>; /* milliseconds */
>> +            polling_delay = <1000>; /* milliseconds */
>> +            governor = "step_wise";
>> +            trips {
>> +                    alert@100000{
>> +                            temperature = <100000>; /* milliCelsius */
>> +                            hysteresis = <0>; /* milliCelsius */
>> +                            type = <1>;
> 
> how about to use the trip type name directly, such as named as
> "passive-trip;", I think it's more readable. for example:
> trip0 {
> ....
> passive-trip;
> }
> trip1 {
> ....
> active-trip;
> }
> 
>> +                    };
>> +                    crit@125000{
>> +                            temperature = <125000>; /* milliCelsius */
>> +                            hysteresis = <0>; /* milliCelsius */
>> +                            type = <3>;
>> +                    };
>> +            };
>> +            bind_params {
>> +                    action@0{
>> +                            cooling_device = "thermal-cpufreq";
>> +                            weight = <100>; /* percentage */
>> +                            mask = <0x01>;
>> +                    };
>> +            };
>> +};
> 
> as we know, thermal_zone_bind_cooling_device() will set the upper/lower
> in the thermal_instance. In the default .bind function, it just set to
> THERMAL_NO_LIMIT, but for some platform, it need to set these
> upper/lower values for different cooling device and trips, how to pass
> these values in DT? how about to set:
> action@0 {
> ...
> mask = <0x03>; //or you can remove this property;

Well, this has been added accordingly to current API needs.

> trip0 = <&alert 1 2>; //1 is lower value, 2 is upper value;
> trip1 = <&crit 3 4>;

I suppose the first item in you 3-tuple is the trip point.

> }

Yeah, I also noticed that I was missing the upper and lower limits. But
unfortunately this is a limitation on the thermal FW API too!

If one passes a bind params, the structure which represents platform
info, then it won't be able to pass the upper and lower limits. But by
passing a .bind callback, then you have the opportunity to match it
using these two values.

I believe we would need to change the data structures and the API to
support upper and lower limits via platform representation. We could
simply use the .bind callback of the dt thermal zone, but I believe that
would abusing the API, assuming that .match is for platform binding.
Durga, what do you think?


> 
> 
> Thanks.
> Wei.
> 
> 
> 


-- 
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 295 bytes --]

RE: [RFC PATCH 2/4] thermal: introduce device tree parser

[R, Durgadoss]

> -----Original Message-----
> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> owner@vger.kernel.org] On Behalf Of Eduardo Valentin
> Sent: Monday, July 15, 2013 5:25 PM
> To: Wei Ni
> Cc: Eduardo Valentin; linux-pm@vger.kernel.org; R, Durgadoss;
> amit.daniel@samsung.com; Zhang, Rui; linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH 2/4] thermal: introduce device tree parser
> 
> On 10-07-2013 02:48, Wei Ni wrote:
> > On 07/09/2013 10:00 PM, Eduardo Valentin wrote:
> >> In order to be able to build thermal policies
> >> based on generic sensors, like I2C device, that
> >> can be places in different points on different boards,
> >> there is a need to have a way to feed board dependent
> >> data into the thermal framework.
> >>
> >> This patch introduces a thermal data parser for device
> >> tree. The parsed data is used to build thermal zones
> >> and thermal binding parameters. The output data
> >> can then be used to deploy thermal policies.
> >>
> >> This patch adds also documentation regarding this
> >> API and how to define define tree nodes to use
> >> this infrastructure.
> >
> > It looks good, with this infrastructure, we can add generic sensor
> > driver into the thermal fw easily.
> >
> >
> >> +
> >> +Below is an example:
> >> +thermal_zone {
> >> +            type = "CPU";
> >> +            mask = <0x03>; /* trips writability */
> >> +            passive_delay = <250>; /* milliseconds */
> >> +            polling_delay = <1000>; /* milliseconds */
> >> +            governor = "step_wise";
> >> +            trips {
> >> +                    alert@100000{
> >> +                            temperature = <100000>; /* milliCelsius */
> >> +                            hysteresis = <0>; /* milliCelsius */
> >> +                            type = <1>;
> >
> > how about to use the trip type name directly, such as named as
> > "passive-trip;", I think it's more readable. for example:
> > trip0 {
> > ....
> > passive-trip;
> > }
> > trip1 {
> > ....
> > active-trip;
> > }
> >
> >> +                    };
> >> +                    crit@125000{
> >> +                            temperature = <125000>; /* milliCelsius */
> >> +                            hysteresis = <0>; /* milliCelsius */
> >> +                            type = <3>;
> >> +                    };
> >> +            };
> >> +            bind_params {
> >> +                    action@0{
> >> +                            cooling_device = "thermal-cpufreq";
> >> +                            weight = <100>; /* percentage */
> >> +                            mask = <0x01>;
> >> +                    };
> >> +            };
> >> +};
> >
> > as we know, thermal_zone_bind_cooling_device() will set the upper/lower
> > in the thermal_instance. In the default .bind function, it just set to
> > THERMAL_NO_LIMIT, but for some platform, it need to set these
> > upper/lower values for different cooling device and trips, how to pass
> > these values in DT? how about to set:
> > action@0 {
> > ...
> > mask = <0x03>; //or you can remove this property;
> 
> Well, this has been added accordingly to current API needs.
> 
> > trip0 = <&alert 1 2>; //1 is lower value, 2 is upper value;
> > trip1 = <&crit 3 4>;
> 
> I suppose the first item in you 3-tuple is the trip point.
> 
> > }
> 
> Yeah, I also noticed that I was missing the upper and lower limits. But
> unfortunately this is a limitation on the thermal FW API too!
> 
> If one passes a bind params, the structure which represents platform
> info, then it won't be able to pass the upper and lower limits. But by
> passing a .bind callback, then you have the opportunity to match it
> using these two values.
> 
> I believe we would need to change the data structures and the API to
> support upper and lower limits via platform representation. We could
> simply use the .bind callback of the dt thermal zone, but I believe that
> would abusing the API, assuming that .match is for platform binding.
> Durga, what do you think?

okay, I see.. Two approaches I could think of:
1. Introduce two arrays (size = number of trips in the tz) named
upper/lower_limits[size] in the 'thermal_bind_params' structure.
This way we don't need any API change. We can slightly change the
implementation inside '__bind' function in thermal_core.c to get this
working.

2. Pass 3 more parameters in the .match function:
.match(tz, cdev, trip, &lower, &upper). The platform layer
then determines whether there is a match; and if so,
provides sane values for lower and upper variables.

At this point of time, I think I prefer method 1 ;)
Let me know your thoughts.

Thanks,
Durga
> 
> 
> >
> >
> > Thanks.
> > Wei.
> >
> >
> >
> 
> 
> --
> You have got to be excited about what you are doing. (L. Lamport)
> 
> Eduardo Valentin

Re: [RFC PATCH 2/4] thermal: introduce device tree parser

[Eduardo Valentin]

[-- Attachment #1: Type: text/plain, Size: 5224 bytes --]

On 15-07-2013 13:03, R, Durgadoss wrote:
>> -----Original Message-----
>> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
>> owner@vger.kernel.org] On Behalf Of Eduardo Valentin
>> Sent: Monday, July 15, 2013 5:25 PM
>> To: Wei Ni
>> Cc: Eduardo Valentin; linux-pm@vger.kernel.org; R, Durgadoss;
>> amit.daniel@samsung.com; Zhang, Rui; linux-kernel@vger.kernel.org
>> Subject: Re: [RFC PATCH 2/4] thermal: introduce device tree parser
>>
>> On 10-07-2013 02:48, Wei Ni wrote:
>>> On 07/09/2013 10:00 PM, Eduardo Valentin wrote:
>>>> In order to be able to build thermal policies
>>>> based on generic sensors, like I2C device, that
>>>> can be places in different points on different boards,
>>>> there is a need to have a way to feed board dependent
>>>> data into the thermal framework.
>>>>
>>>> This patch introduces a thermal data parser for device
>>>> tree. The parsed data is used to build thermal zones
>>>> and thermal binding parameters. The output data
>>>> can then be used to deploy thermal policies.
>>>>
>>>> This patch adds also documentation regarding this
>>>> API and how to define define tree nodes to use
>>>> this infrastructure.
>>>
>>> It looks good, with this infrastructure, we can add generic sensor
>>> driver into the thermal fw easily.
>>>
>>>
>>>> +
>>>> +Below is an example:
>>>> +thermal_zone {
>>>> +            type = "CPU";
>>>> +            mask = <0x03>; /* trips writability */
>>>> +            passive_delay = <250>; /* milliseconds */
>>>> +            polling_delay = <1000>; /* milliseconds */
>>>> +            governor = "step_wise";
>>>> +            trips {
>>>> +                    alert@100000{
>>>> +                            temperature = <100000>; /* milliCelsius */
>>>> +                            hysteresis = <0>; /* milliCelsius */
>>>> +                            type = <1>;
>>>
>>> how about to use the trip type name directly, such as named as
>>> "passive-trip;", I think it's more readable. for example:
>>> trip0 {
>>> ....
>>> passive-trip;
>>> }
>>> trip1 {
>>> ....
>>> active-trip;
>>> }
>>>
>>>> +                    };
>>>> +                    crit@125000{
>>>> +                            temperature = <125000>; /* milliCelsius */
>>>> +                            hysteresis = <0>; /* milliCelsius */
>>>> +                            type = <3>;
>>>> +                    };
>>>> +            };
>>>> +            bind_params {
>>>> +                    action@0{
>>>> +                            cooling_device = "thermal-cpufreq";
>>>> +                            weight = <100>; /* percentage */
>>>> +                            mask = <0x01>;
>>>> +                    };
>>>> +            };
>>>> +};
>>>
>>> as we know, thermal_zone_bind_cooling_device() will set the upper/lower
>>> in the thermal_instance. In the default .bind function, it just set to
>>> THERMAL_NO_LIMIT, but for some platform, it need to set these
>>> upper/lower values for different cooling device and trips, how to pass
>>> these values in DT? how about to set:
>>> action@0 {
>>> ...
>>> mask = <0x03>; //or you can remove this property;
>>
>> Well, this has been added accordingly to current API needs.
>>
>>> trip0 = <&alert 1 2>; //1 is lower value, 2 is upper value;
>>> trip1 = <&crit 3 4>;
>>
>> I suppose the first item in you 3-tuple is the trip point.
>>
>>> }
>>
>> Yeah, I also noticed that I was missing the upper and lower limits. But
>> unfortunately this is a limitation on the thermal FW API too!
>>
>> If one passes a bind params, the structure which represents platform
>> info, then it won't be able to pass the upper and lower limits. But by
>> passing a .bind callback, then you have the opportunity to match it
>> using these two values.
>>
>> I believe we would need to change the data structures and the API to
>> support upper and lower limits via platform representation. We could
>> simply use the .bind callback of the dt thermal zone, but I believe that
>> would abusing the API, assuming that .match is for platform binding.
>> Durga, what do you think?
> 
> okay, I see.. Two approaches I could think of:
> 1. Introduce two arrays (size = number of trips in the tz) named
> upper/lower_limits[size] in the 'thermal_bind_params' structure.
> This way we don't need any API change. We can slightly change the
> implementation inside '__bind' function in thermal_core.c to get this
> working.
> 
> 2. Pass 3 more parameters in the .match function:
> .match(tz, cdev, trip, &lower, &upper). The platform layer
> then determines whether there is a match; and if so,
> provides sane values for lower and upper variables.
> 
> At this point of time, I think I prefer method 1 ;)
> Let me know your thoughts.
> 

Yeah, I agree that (1) is likely to scale. I will cook something with it
for next version.

> Thanks,
> Durga
>>
>>
>>>
>>>
>>> Thanks.
>>> Wei.
>>>
>>>
>>>
>>
>>
>> --
>> You have got to be excited about what you are doing. (L. Lamport)
>>
>> Eduardo Valentin
> 
> 
> 


-- 
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 295 bytes --]