Hello all,
As you noticed, I am working in a way to represent thermal data
using device tree [1]. Essentially, this should be a way to say
what to do with a sensor and how to associate (cooling) actions
with it.
The motivation to create such infrastructure is:
(i) - to reuse the existing temperature sensor code base;
(ii) - have a way to easily describe thermal data across different
boards for the same sensor. Say you have an i2c temp sensor,
which is placed close to your battery on board A but on
board B, another instance of this same senor is placed
close to your display, for instance.
This series introduces then a DT parser. The data expected in
DT must contain the needed properties to build a thermal zone
out of the desired sensor. All properties are documented and
they are derived from the existing requirements of current
thermal API.
In order to perform a binding with cooling devices,
the new thermal zone built using DT nodes will use
the existing thermal API that uses binding parameters. This is
the current proposed way to register thermal zones with platform
information, written by Durga.
There are some virtual concepts that are pushed to device tree,
I know. But I believe using device tree to do this makes sense
because we are still describing the HW and how they are related
to each other. Things like cooling devices are not represented
in device tree though, as I believe that will cause a lot of
confusion with real devices (as already does).
So the series is short but I think it makes sense to describe
how it is organized, as it touches several places. First four
patches are a preparation for this parser. There is a change
on cpufreq-cpu0, so that it knows now how to load its
corresponding cooling device. There is a change on thermal_core
to split its hwmon code, because I think we may need to improve
this code base when we start to integrate better with hwmon
temperature sensors. Then, another needed preparation is to
improve the bind_params, so that we are able to bind with
upper and lower limits. The remaining patches are the changes
with the proposed DT parser. A part from the DT thermal zone
builder itself (patch 05), I also changed the ti-soc-thermal
driver to use this new API and the omap4430 bandgap DT node,
as an example (this has been tested on panda omap4430); and also changed
the hwmon drivers lm75 and tmp102 to have the option to build
a thermal zone using DT. I haven't touched any dts file using
lm75 or tmp102 because this should come on a need basis.
I believe this code is pretty usable the way it is, but might
need to be revisited, in case the enhancement proposed by Durga
get in. This is because representing virtual thermal zones
built out of several sensors may need a different representation
in DT.
[1] - RFC of this work:
http://comments.gmane.org/gmane.linux.power-management.general/35874
Changes from RFC:
- Added a way to load cpufreq cooling device out of DT
- Added a way to bind with upper and lower limits using bind_params
- Added a way to specify upper and lower binding limits on DT
- Added DT thermal builder support to lm75 and tmp102 hwmon drivers
- Changed ERANGE to EDOM
- Added thermal constants for zone type and bind limit, so that
dts file could be more readable
Tested on panda omap4430 (3.11-rc1 with minor changes for getting cpufreq working)
BR,
Eduardo Valentin (9):
cpufreq: cpufreq-cpu0: add dt node parsing for 'needs-cooling'
thermal: hwmon: move hwmon support to single file
thermal: thermal_core: allow binding with limits on bind_params
arm: dts: flag omap4430 with needs-cooling for cpu node
thermal: introduce device tree parser
thermal: ti-soc-thermal: use thermal DT infrastructure
arm: dts: add omap4430 thermal data
hwmon: lm75: expose to thermal fw via DT nodes
hwmon: tmp102: expose to thermal fw via DT nodes
.../devicetree/bindings/cpufreq/cpufreq-cpu0.txt | 3 +
.../devicetree/bindings/thermal/thermal.txt | 133 ++++++
Documentation/thermal/sysfs-api.txt | 7 +
arch/arm/boot/dts/omap443x.dtsi | 32 +-
drivers/cpufreq/cpufreq-cpu0.c | 8 +
drivers/hwmon/lm75.c | 29 ++
drivers/hwmon/tmp102.c | 25 ++
drivers/thermal/Kconfig | 22 +
drivers/thermal/Makefile | 4 +
drivers/thermal/thermal_core.c | 274 +-----------
drivers/thermal/thermal_dt.c | 458 +++++++++++++++++++++
drivers/thermal/thermal_hwmon.c | 269 ++++++++++++
drivers/thermal/thermal_hwmon.h | 49 +++
drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 46 ++-
include/dt-bindings/thermal/thermal.h | 27 ++
include/linux/thermal.h | 13 +
16 files changed, 1129 insertions(+), 270 deletions(-)
create mode 100644 Documentation/devicetree/bindings/thermal/thermal.txt
create mode 100644 drivers/thermal/thermal_dt.c
create mode 100644 drivers/thermal/thermal_hwmon.c
create mode 100644 drivers/thermal/thermal_hwmon.h
create mode 100644 include/dt-bindings/thermal/thermal.h
--
1.8.2.1.342.gfa7285d
OMAP4430 devices can reach high temperatures and thus
needs to have cpufreq-cooling on systems running on it.
This patch adds the flag so that cpufreq-cpu0 driver
loads the cooling device to use cpufreq on OMAP4430.
Cc: "Benoît Cousson" <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Eduardo Valentin <[email protected]>
---
arch/arm/boot/dts/omap443x.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/omap443x.dtsi b/arch/arm/boot/dts/omap443x.dtsi
index bcf455e..4a4dcc3 100644
--- a/arch/arm/boot/dts/omap443x.dtsi
+++ b/arch/arm/boot/dts/omap443x.dtsi
@@ -22,6 +22,7 @@
1008000 1375000
>;
clock-latency = <300000>; /* From legacy driver */
+ needs-cooling; /* make sure we have cpufreq-cooling */
};
};
--
1.8.2.1.342.gfa7285d
This patch changes the cpufreq-cpu0 driver to consider if
a cpu needs cooling (with cpufreq). In case the cooling is needed,
it can be flagged at the cpu0 device tree node, with the boolean
property 'needs-cooling'.
In case this boolean is present, the driver will
load a cpufreq cooling device in the system. The cpufreq-cpu0
driver is not interested in determining how the system should
be using the cooling device. The driver is responsible
only of loading the cooling device.
Describing how the cooling device will be used can be
accomplished by setting up a thermal zone that references
and is composed by the cpufreq cooling device.
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Eduardo Valentin <[email protected]>
---
Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt | 3 +++
drivers/cpufreq/cpufreq-cpu0.c | 8 ++++++++
2 files changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
index 051f764..e8ff916 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
@@ -15,6 +15,8 @@ Optional properties:
- clock-latency: Specify the possible maximum transition latency for clock,
in unit of nanoseconds.
- voltage-tolerance: Specify the CPU voltage tolerance in percentage.
+- needs-cooling: The generic cpu cooling (freq clipping) is loaded by the
+generic cpufreq-cpu0 driver in case the device tree node has this boolean.
Examples:
@@ -33,6 +35,7 @@ cpus {
198000 850000
>;
clock-latency = <61036>; /* two CLK32 periods */
+ needs-cooling;
};
cpu@1 {
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index ad1fde2..4a8747a 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -20,6 +20,9 @@
#include <linux/platform_device.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
+#include <linux/thermal.h>
+#include <linux/cpu_cooling.h>
+#include <linux/cpumask.h>
static unsigned int transition_latency;
static unsigned int voltage_tolerance; /* in percentage */
@@ -28,6 +31,7 @@ static struct device *cpu_dev;
static struct clk *cpu_clk;
static struct regulator *cpu_reg;
static struct cpufreq_frequency_table *freq_table;
+static struct thermal_cooling_device *cdev;
static int cpu0_verify_speed(struct cpufreq_policy *policy)
{
@@ -268,6 +272,9 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
goto out_free_table;
}
+ if (of_property_read_bool(np, "needs-cooling"))
+ cdev = cpufreq_cooling_register(cpu_present_mask);
+
of_node_put(np);
of_node_put(parent);
return 0;
@@ -283,6 +290,7 @@ out_put_parent:
static int cpu0_cpufreq_remove(struct platform_device *pdev)
{
+ cpufreq_cooling_unregister(cdev);
cpufreq_unregister_driver(&cpu0_cpufreq_driver);
opp_free_cpufreq_table(cpu_dev, &freq_table);
--
1.8.2.1.342.gfa7285d
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. Also, some minor changes
in the code were required, as recommended by checkpatch.pl.
Cc: Zhang Rui <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Eduardo Valentin <[email protected]>
---
drivers/thermal/Kconfig | 9 ++
drivers/thermal/Makefile | 3 +
drivers/thermal/thermal_core.c | 255 +------------------------------------
drivers/thermal/thermal_hwmon.c | 269 ++++++++++++++++++++++++++++++++++++++++
drivers/thermal/thermal_hwmon.h | 49 ++++++++
5 files changed, 331 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..ab52a96
--- /dev/null
+++ b/drivers/thermal/thermal_hwmon.c
@@ -0,0 +1,269 @@
+/*
+ * 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 <[email protected]>
+ * Copyright (C) 2008 Sujith Thomas <[email protected]>
+ *
+ * Copyright (C) 2013 Texas Instruments
+ * Copyright (C) 2013 Eduardo Valentin <[email protected]>
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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>
+#include "thermal_hwmon.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(*hwmon), 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(*hwmon), 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 <[email protected]>
+ * Copyright (C) 2008 Sujith Thomas <[email protected]>
+ *
+ * Copyright (C) 2013 Texas Instruments
+ * Copyright (C) 2013 Eduardo Valentin <[email protected]>
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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
This patch changes the dtsi entry on omap4430 to contain
the thermal data. This data will enable the passive
cooling with CPUfreq cooling device at 100C and the
system will do a thermal shutdown at 125C.
Cc: "Benoît Cousson" <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Eduardo Valentin <[email protected]>
---
arch/arm/boot/dts/omap443x.dtsi | 31 +++++++++++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/omap443x.dtsi b/arch/arm/boot/dts/omap443x.dtsi
index 4a4dcc3..27f0e0f 100644
--- a/arch/arm/boot/dts/omap443x.dtsi
+++ b/arch/arm/boot/dts/omap443x.dtsi
@@ -8,6 +8,7 @@
* kind, whether express or implied.
*/
+#include <dt-bindings/thermal/thermal.h>
#include "omap4.dtsi"
/ {
@@ -27,8 +28,34 @@
};
bandgap {
- reg = <0x4a002260 0x4
- 0x4a00232C 0x4>;
+ reg = <0x4a002260 0x4 0x4a00232C 0x4>;
compatible = "ti,omap4430-bandgap";
+ 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 = <2000>; /* milliCelsius */
+ type = <THERMAL_TRIP_PASSIVE>;
+ };
+ crit@125000{
+ temperature = <125000>; /* milliCelsius */
+ hysteresis = <2000>; /* milliCelsius */
+ type = <THERMAL_TRIP_CRITICAL>;
+ };
+ };
+ bind_params {
+ action@0{
+ cooling_device = "thermal-cpufreq";
+ weight = <100>; /* percentage */
+ mask = <0x01>;
+ /* no limits, using defaults */
+ };
+ };
+ };
};
};
--
1.8.2.1.342.gfa7285d
This patch improves the ti-soc-thermal driver by adding the
support to build the thermal zones based on DT nodes.
The driver will have two options now to build the thermal
zones. The first option is the zones originally coded
in this driver. So, the driver behavior will be same
if there is no DT node describing the zones. The second
option, when it is found a DT node with thermal data,
will used the common infrastructure to build the thermal
zone and bind its cooling devices.
In case the driver loads thermal data using the legacy
mode, this driver still adds to the system
a cpufreq cooling device. Loading the thermal data from
DT, the driver assumes someone else will add the cpufreq
cooling device, like the cpufreq driver.
Cc: Zhang Rui <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Eduardo Valentin <[email protected]>
---
drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 46 +++++++++++++++++-----
1 file changed, 36 insertions(+), 10 deletions(-)
diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index 4c5f55c37..de4e138 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -31,6 +31,7 @@
#include <linux/cpufreq.h>
#include <linux/cpumask.h>
#include <linux/cpu_cooling.h>
+#include <linux/of.h>
#include "ti-thermal.h"
#include "ti-bandgap.h"
@@ -75,11 +76,10 @@ static inline int ti_thermal_hotspot_temperature(int t, int s, int c)
/* thermal zone ops */
/* Get temperature callback function for thermal zone*/
-static inline int ti_thermal_get_temp(struct thermal_zone_device *thermal,
- unsigned long *temp)
+static inline int __ti_thermal_get_temp(void *devdata, unsigned long *temp)
{
struct thermal_zone_device *pcb_tz = NULL;
- struct ti_thermal_data *data = thermal->devdata;
+ struct ti_thermal_data *data = devdata;
struct ti_bandgap *bgp;
const struct ti_temp_sensor *s;
int ret, tmp, slope, constant;
@@ -117,6 +117,14 @@ static inline int ti_thermal_get_temp(struct thermal_zone_device *thermal,
return ret;
}
+static inline int ti_thermal_get_temp(struct thermal_zone_device *thermal,
+ unsigned long *temp)
+{
+ struct ti_thermal_data *data = thermal->devdata;
+
+ return __ti_thermal_get_temp(data, temp);
+}
+
/* Bind callback functions for thermal zone */
static int ti_thermal_bind(struct thermal_zone_device *thermal,
struct thermal_cooling_device *cdev)
@@ -302,17 +310,27 @@ int ti_thermal_expose_sensor(struct ti_bandgap *bgp, int id,
if (!data)
return -EINVAL;
- /* Create thermal zone */
- data->ti_thermal = thermal_zone_device_register(domain,
+ /* in case this is specified by DT */
+ if (of_find_node_by_name(bgp->dev->of_node, "thermal_zone")) {
+ data->ti_thermal = thermal_zone_of_device_register(bgp->dev,
+ data, __ti_thermal_get_temp);
+ if (IS_ERR(data->ti_thermal)) {
+ dev_err(bgp->dev, "failed to build of thermal zone\n");
+ return PTR_ERR(data->ti_thermal);
+ }
+ } else {
+ /* Create thermal zone */
+ data->ti_thermal = thermal_zone_device_register(domain,
OMAP_TRIP_NUMBER, 0, data, &ti_thermal_ops,
NULL, FAST_TEMP_MONITORING_RATE,
FAST_TEMP_MONITORING_RATE);
- if (IS_ERR(data->ti_thermal)) {
- dev_err(bgp->dev, "thermal zone device is NULL\n");
- return PTR_ERR(data->ti_thermal);
+ if (IS_ERR(data->ti_thermal)) {
+ dev_err(bgp->dev, "thermal zone device is NULL\n");
+ return PTR_ERR(data->ti_thermal);
+ }
+ data->ti_thermal->polling_delay = FAST_TEMP_MONITORING_RATE;
+ ti_bandgap_set_sensor_data(bgp, id, data);
}
- data->ti_thermal->polling_delay = FAST_TEMP_MONITORING_RATE;
- ti_bandgap_set_sensor_data(bgp, id, data);
return 0;
}
@@ -343,6 +361,14 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
{
struct ti_thermal_data *data;
+ /*
+ * We are assuming here that if one deploys the zone
+ * using DT, then it must be aware that the cooling device
+ * loading has to happen via cpufreq driver.
+ */
+ if (of_find_node_by_name(bgp->dev->of_node, "thermal_zone"))
+ return 0;
+
data = ti_bandgap_get_sensor_data(bgp, id);
if (!data || IS_ERR(data))
data = ti_thermal_build_data(bgp, id);
--
1.8.2.1.342.gfa7285d
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 tree nodes to use
this infrastructure.
Cc: Zhang Rui <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Eduardo Valentin <[email protected]>
---
.../devicetree/bindings/thermal/thermal.txt | 133 ++++++
drivers/thermal/Kconfig | 13 +
drivers/thermal/Makefile | 1 +
drivers/thermal/thermal_dt.c | 458 +++++++++++++++++++++
include/dt-bindings/thermal/thermal.h | 27 ++
include/linux/thermal.h | 3 +
6 files changed, 635 insertions(+)
create mode 100644 Documentation/devicetree/bindings/thermal/thermal.txt
create mode 100644 drivers/thermal/thermal_dt.c
create mode 100644 include/dt-bindings/thermal/thermal.h
diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
new file mode 100644
index 0000000..ac0c7b9
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/thermal.txt
@@ -0,0 +1,133 @@
+----------------------------------------
+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
+ THERMAL_TRIP_PASSIVE
+ THERMAL_TRIP_HOT
+ THERMAL_TRIP_CRITICAL
+
+**********
+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.
+- limits: An array integers consisting of 2-tuples items, and each item means
+ lower and upper state limits, like <min-state max-state>.
+
+************
+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 = <THERMAL_TRIP_PASSIVE>;
+ };
+ crit@125000{
+ temperature = <125000>; /* milliCelsius */
+ hysteresis = <0>; /* milliCelsius */
+ type = <THERMAL_TRIP_CRITICAL>;
+ };
+ };
+ bind_params {
+ action@0{
+ cooling_device = "thermal-cpufreq";
+ weight = <100>; /* percentage */
+ mask = <0x01>;
+ limits = <
+ /* lower-state upper-state */
+ 1 THERMAL_NO_LIMITS
+ THERMAL_NO_LIMITS THERMAL_NO_LIMITS
+ >;
+ };
+ };
+};
+
+***************
+cpufreq-cooling
+***************
+
+The generic cpu cooling (freq clipping) can be loaded by the
+generic cpufreq-cpu0 driver in case the device tree node informs
+this need.
+
+In order to load the cpufreq cooling device, it is possible to
+inform that the cpu needs cooling by adding the boolean flag
+'needs-cooling' at the cpu0 phandle.
+
+Example:
+ cpus {
+ cpu@0 {
+ operating-points = <
+ /* kHz uV */
+ 800000 1313000
+ 1008000 1375000
+ >;
+ needs-cooling; /* make sure we have cpufreq-cooling */
+ };
+ ...
+ };
+
+The above will cause the cpufreq-cpu0 driver to understand that
+the cpu will need passive cooling and while probing that node it will
+also load the cpufreq cpu cooling device in that system.
+
+However, be advised that this flag will not describe what needs to
+be done or how the cpufreq cooling device will be used. In order to
+accomplish this, one needs to write a phandle for a thermal zone, as
+described in the section 'thermal_zone'.
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..980b795
--- /dev/null
+++ b/drivers/thermal/thermal_dt.c
@@ -0,0 +1,458 @@
+/*
+ * thermal_dt.c - Generic Thermal Management device tree support.
+ *
+ * Copyright (C) 2013 Texas Instruments
+ * Copyright (C) 2013 Eduardo Valentin <[email protected]>
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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/types.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,
+ int ntrips)
+{
+ const char *cdev_name;
+ u32 *limits;
+ 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));
+
+ limits = kzalloc(ntrips * sizeof(*limits) * 2, GFP_KERNEL);
+ if (!limits) {
+ dev_err(dev, "not enough mem for reading limits\n");
+ return -ENOMEM;
+ }
+ ret = of_property_read_u32_array(node, "limits", limits, 2 * ntrips);
+ if (!ret) {
+ int i = ntrips;
+
+ tbp->binding_limits = devm_kzalloc(dev,
+ ntrips * 2 *
+ sizeof(*tbp->binding_limits),
+ GFP_KERNEL);
+ if (!tbp->binding_limits) {
+ dev_err(dev, "not enough mem for storing limits\n");
+ return -ENOMEM;
+ }
+ while (i--)
+ tbp->binding_limits[i] = limits[i];
+ }
+ kfree(limits);
+
+ 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],
+ tz->ntrips);
+ 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 -EDOM;
+
+ *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 -EDOM;
+
+ *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 -EDOM;
+
+ /* 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 -EDOM;
+
+ *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 -EDOM;
+
+ /* 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,
+};
+
+/**
+ * thermal_zone_of_device_register() - registers a thermal zone based on DT
+ * @device: a valid struct device pointer containing device tree node
+ * with thermal_zone descriptor.
+ * @data: a private pointer (owned by the caller) that will be passed
+ * back, when a temperature reading is needed.
+ * @get_temp: a pointer to a function that returns the thermal zone
+ * temperature.
+ *
+ * This function will parse the device tree node associated with the
+ * @dev struct and will attempt to build a thermal zone device based
+ * upon the thermal data provided and described by DT.
+ *
+ * The thermal zone temperature is provided by the @get_temp function
+ * pointer. When called, it will have the private pointer @data back.
+ *
+ * Return: On success returns a valid struct thermal_zone_device,
+ * otherwise, it returns a corresponding ERR_PTR(). Caller must
+ * check the return value with help of IS_ERR() helper.
+ */
+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 ERR_CAST(tz);
+ }
+ ops = devm_kzalloc(dev, sizeof(*ops), GFP_KERNEL);
+ if (!ops) {
+ dev_err(dev, "no memory available for thermal ops\n");
+ return ERR_PTR(-ENOMEM);
+ }
+ 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/include/dt-bindings/thermal/thermal.h b/include/dt-bindings/thermal/thermal.h
new file mode 100644
index 0000000..6dd6ccd
--- /dev/null
+++ b/include/dt-bindings/thermal/thermal.h
@@ -0,0 +1,27 @@
+/*
+ * This header provides constants for most thermal bindings.
+ *
+ * Copyright (C) 2013 Texas Instruments
+ * Eduardo Valentin <[email protected]>
+ *
+ * GPLv2 only
+ */
+
+#ifndef _DT_BINDINGS_THERMAL_THERMAL_H
+#define _DT_BINDINGS_THERMAL_THERMAL_H
+
+/*
+ * Here are the thermal trip types. This must
+ * match with enum thermal_trip_type at
+ * include/linux/thermal.h
+ */
+#define THERMAL_TRIP_ACTIVE 0
+#define THERMAL_TRIP_PASSIVE 1
+#define THERMAL_TRIP_HOT 2
+#define THERMAL_TRIP_CRITICAL 3
+
+/* On cooling devices upper and lower limits */
+#define THERMAL_NO_LIMIT (-1UL)
+
+#endif
+
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 39575eb..22a3250 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -234,6 +234,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
This patch adds to tmp102 temperature sensor the possibility
to expose itself as thermal zone device, registered on the
thermal framework.
The thermal zone is built only if a device tree node
describing a thermal zone for this sensor is present
inside the tmp102 DT node. Otherwise, the driver behavior
will be the same.
Cc: Jean Delvare <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Eduardo Valentin <[email protected]>
---
drivers/hwmon/tmp102.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/hwmon/tmp102.c b/drivers/hwmon/tmp102.c
index d7b47ab..621093b 100644
--- a/drivers/hwmon/tmp102.c
+++ b/drivers/hwmon/tmp102.c
@@ -27,6 +27,8 @@
#include <linux/mutex.h>
#include <linux/device.h>
#include <linux/jiffies.h>
+#include <linux/thermal.h>
+#include <linux/of.h>
#define DRIVER_NAME "tmp102"
@@ -50,6 +52,7 @@
struct tmp102 {
struct device *hwmon_dev;
+ struct thermal_zone_device *tz;
struct mutex lock;
u16 config_orig;
unsigned long last_update;
@@ -93,6 +96,15 @@ static struct tmp102 *tmp102_update_device(struct i2c_client *client)
return tmp102;
}
+static int tmp102_read_temp(void *dev, unsigned long *temp)
+{
+ struct tmp102 *tmp102 = tmp102_update_device(to_i2c_client(dev));
+
+ *temp = tmp102->temp[0];
+
+ return 0;
+}
+
static ssize_t tmp102_show_temp(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -204,10 +216,22 @@ static int tmp102_probe(struct i2c_client *client,
goto fail_remove_sysfs;
}
+ if (of_find_node_by_name(client->dev.of_node, "thermal_zone")) {
+ tmp102->tz = thermal_zone_of_device_register(&client->dev,
+ &client->dev,
+ tmp102_read_temp);
+ if (IS_ERR(tmp102->tz)) {
+ status = PTR_ERR(tmp102->tz);
+ goto exit_hwmon;
+ }
+ }
+
dev_info(&client->dev, "initialized\n");
return 0;
+exit_hwmon:
+ hwmon_device_unregister(tmp102->hwmon_dev);
fail_remove_sysfs:
sysfs_remove_group(&client->dev.kobj, &tmp102_attr_group);
fail_restore_config:
@@ -220,6 +244,7 @@ static int tmp102_remove(struct i2c_client *client)
{
struct tmp102 *tmp102 = i2c_get_clientdata(client);
+ thermal_zone_device_unregister(tmp102->tz);
hwmon_device_unregister(tmp102->hwmon_dev);
sysfs_remove_group(&client->dev.kobj, &tmp102_attr_group);
--
1.8.2.1.342.gfa7285d
This patch adds to lm75 temperature sensor the possibility
to expose itself as thermal zone device, registered on the
thermal framework.
The thermal zone is built only if a device tree node
describing a thermal zone for this sensor is present
inside the lm75 DT node. Otherwise, the driver behavior
will be the same.
Cc: Jean Delvare <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Eduardo Valentin <[email protected]>
---
drivers/hwmon/lm75.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index c03b490..0aa5e28 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -27,6 +27,8 @@
#include <linux/hwmon-sysfs.h>
#include <linux/err.h>
#include <linux/mutex.h>
+#include <linux/thermal.h>
+#include <linux/of.h>
#include "lm75.h"
@@ -70,6 +72,7 @@ static const u8 LM75_REG_TEMP[3] = {
/* Each client has this additional data */
struct lm75_data {
struct device *hwmon_dev;
+ struct thermal_zone_device *tz;
struct mutex update_lock;
u8 orig_conf;
u8 resolution; /* In bits, between 9 and 12 */
@@ -92,6 +95,19 @@ static struct lm75_data *lm75_update_device(struct device *dev);
/* sysfs attributes for hwmon */
+static int lm75_read_temp(void *dev, unsigned long *temp)
+{
+ struct lm75_data *data = lm75_update_device(dev);
+
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+
+ *temp = ((data->temp[0] >> (16 - data->resolution)) * 1000) >>
+ (data->resolution - 8);
+
+ return 0;
+}
+
static ssize_t show_temp(struct device *dev, struct device_attribute *da,
char *buf)
{
@@ -271,11 +287,23 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
goto exit_remove;
}
+ if (of_find_node_by_name(client->dev.of_node, "thermal_zone")) {
+ data->tz = thermal_zone_of_device_register(&client->dev,
+ &client->dev,
+ lm75_read_temp);
+ if (IS_ERR(data->tz)) {
+ status = PTR_ERR(data->tz);
+ goto exit_hwmon;
+ }
+ }
+
dev_info(&client->dev, "%s: sensor '%s'\n",
dev_name(data->hwmon_dev), client->name);
return 0;
+exit_hwmon:
+ hwmon_device_unregister(data->hwmon_dev);
exit_remove:
sysfs_remove_group(&client->dev.kobj, &lm75_group);
return status;
@@ -285,6 +313,7 @@ static int lm75_remove(struct i2c_client *client)
{
struct lm75_data *data = i2c_get_clientdata(client);
+ thermal_zone_device_unregister(data->tz);
hwmon_device_unregister(data->hwmon_dev);
sysfs_remove_group(&client->dev.kobj, &lm75_group);
lm75_write_value(client, LM75_REG_CONF, data->orig_conf);
--
1.8.2.1.342.gfa7285d
When registering a thermal zone device using platform information
via bind_params, the thermal framework will always perform the
cdev binding using the lowest and highest limits (THERMAL_NO_LIMIT).
This patch changes the data structures so that it is possible
to inform what are the desired limits for each trip point
inside a bind_param. The way the binding is performed is also
changed so that it uses the new data structure.
Cc: Zhang Rui <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Eduardo Valentin <[email protected]>
---
Documentation/thermal/sysfs-api.txt | 7 +++++++
drivers/thermal/thermal_core.c | 19 +++++++++++++++----
include/linux/thermal.h | 10 ++++++++++
3 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index a71bd5b..2ad50e7 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -134,6 +134,13 @@ temperature) and throttle appropriate devices.
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.
+ .limits: This is an array of cooling state limits. Must have exactly
+ 2 * thermal_zone.number_of_trip_points. It is an array consisting
+ of tuples <lower-state upper-state> of state limits. Each trip
+ will be associated with one state limit tuple when binding.
+ A NULL pointer means <THERMAL_NO_LIMITS THERMAL_NO_LIMITS>
+ on all trips. These limits are used when binding a cdev to a
+ trip point.
.match: This call back returns success(0) if the 'tz and cdev' need to
be bound, as per platform data.
1.4.2 struct thermal_zone_params
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 247528b..096c8be 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -202,14 +202,23 @@ static void print_bind_err_msg(struct thermal_zone_device *tz,
}
static void __bind(struct thermal_zone_device *tz, int mask,
- struct thermal_cooling_device *cdev)
+ struct thermal_cooling_device *cdev,
+ unsigned long *limits)
{
int i, ret;
for (i = 0; i < tz->trips; i++) {
if (mask & (1 << i)) {
+ unsigned long upper, lower;
+
+ upper = THERMAL_NO_LIMIT;
+ lower = THERMAL_NO_LIMIT;
+ if (limits) {
+ lower = limits[i * 2];
+ upper = limits[i * 2 + 1];
+ }
ret = thermal_zone_bind_cooling_device(tz, i, cdev,
- THERMAL_NO_LIMIT, THERMAL_NO_LIMIT);
+ upper, lower);
if (ret)
print_bind_err_msg(tz, cdev, ret);
}
@@ -254,7 +263,8 @@ static void bind_cdev(struct thermal_cooling_device *cdev)
if (tzp->tbp[i].match(pos, cdev))
continue;
tzp->tbp[i].cdev = cdev;
- __bind(pos, tzp->tbp[i].trip_mask, cdev);
+ __bind(pos, tzp->tbp[i].trip_mask, cdev,
+ tzp->tbp[i].binding_limits);
}
}
@@ -292,7 +302,8 @@ static void bind_tz(struct thermal_zone_device *tz)
if (tzp->tbp[i].match(tz, pos))
continue;
tzp->tbp[i].cdev = pos;
- __bind(tz, tzp->tbp[i].trip_mask, pos);
+ __bind(tz, tzp->tbp[i].trip_mask, pos,
+ tzp->tbp[i].binding_limits);
}
}
exit:
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index a386a1c..39575eb 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -207,6 +207,16 @@ struct thermal_bind_params {
* See Documentation/thermal/sysfs-api.txt for more information.
*/
int trip_mask;
+
+ /*
+ * This is an array of cooling state limits. Must have exactly
+ * 2 * thermal_zone.number_of_trip_points. It is an array consisting
+ * of tuples <lower-state upper-state> of state limits. Each trip
+ * will be associated with one state limit tuple when binding.
+ * A NULL pointer means <THERMAL_NO_LIMITS THERMAL_NO_LIMITS>
+ * on all trips.
+ */
+ unsigned long *binding_limits;
int (*match) (struct thermal_zone_device *tz,
struct thermal_cooling_device *cdev);
};
--
1.8.2.1.342.gfa7285d
> -----Original Message-----
> From: [email protected] [mailto:lm-sensors-bounces@lm-
> sensors.org] On Behalf Of Eduardo Valentin
> Sent: Wednesday, July 17, 2013 8:47 PM
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; Eduardo Valentin; Zhang, Rui;
> [email protected]
> Subject: [lm-sensors] [RESEND PATCH V1 3/9] thermal: thermal_core: allow
> binding with limits on bind_params
>
> When registering a thermal zone device using platform information
> via bind_params, the thermal framework will always perform the
> cdev binding using the lowest and highest limits (THERMAL_NO_LIMIT).
>
> This patch changes the data structures so that it is possible
> to inform what are the desired limits for each trip point
> inside a bind_param. The way the binding is performed is also
> changed so that it uses the new data structure.
>
> Cc: Zhang Rui <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Eduardo Valentin <[email protected]>
This patch looks good to me.
Acked-by: Durgadoss R <[email protected]>
Thanks,
Durga
> ---
> Documentation/thermal/sysfs-api.txt | 7 +++++++
> drivers/thermal/thermal_core.c | 19 +++++++++++++++----
> include/linux/thermal.h | 10 ++++++++++
> 3 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/thermal/sysfs-api.txt
> b/Documentation/thermal/sysfs-api.txt
> index a71bd5b..2ad50e7 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -134,6 +134,13 @@ temperature) and throttle appropriate devices.
> 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.
> + .limits: This is an array of cooling state limits. Must have exactly
> + 2 * thermal_zone.number_of_trip_points. It is an array consisting
> + of tuples <lower-state upper-state> of state limits. Each trip
> + will be associated with one state limit tuple when binding.
> + A NULL pointer means <THERMAL_NO_LIMITS THERMAL_NO_LIMITS>
> + on all trips. These limits are used when binding a cdev to a
> + trip point.
> .match: This call back returns success(0) if the 'tz and cdev' need to
> be bound, as per platform data.
> 1.4.2 struct thermal_zone_params
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 247528b..096c8be 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -202,14 +202,23 @@ static void print_bind_err_msg(struct
> thermal_zone_device *tz,
> }
>
> static void __bind(struct thermal_zone_device *tz, int mask,
> - struct thermal_cooling_device *cdev)
> + struct thermal_cooling_device *cdev,
> + unsigned long *limits)
> {
> int i, ret;
>
> for (i = 0; i < tz->trips; i++) {
> if (mask & (1 << i)) {
> + unsigned long upper, lower;
> +
> + upper = THERMAL_NO_LIMIT;
> + lower = THERMAL_NO_LIMIT;
> + if (limits) {
> + lower = limits[i * 2];
> + upper = limits[i * 2 + 1];
> + }
> ret = thermal_zone_bind_cooling_device(tz, i, cdev,
> - THERMAL_NO_LIMIT,
> THERMAL_NO_LIMIT);
> + upper, lower);
> if (ret)
> print_bind_err_msg(tz, cdev, ret);
> }
> @@ -254,7 +263,8 @@ static void bind_cdev(struct thermal_cooling_device
> *cdev)
> if (tzp->tbp[i].match(pos, cdev))
> continue;
> tzp->tbp[i].cdev = cdev;
> - __bind(pos, tzp->tbp[i].trip_mask, cdev);
> + __bind(pos, tzp->tbp[i].trip_mask, cdev,
> + tzp->tbp[i].binding_limits);
> }
> }
>
> @@ -292,7 +302,8 @@ static void bind_tz(struct thermal_zone_device *tz)
> if (tzp->tbp[i].match(tz, pos))
> continue;
> tzp->tbp[i].cdev = pos;
> - __bind(tz, tzp->tbp[i].trip_mask, pos);
> + __bind(tz, tzp->tbp[i].trip_mask, pos,
> + tzp->tbp[i].binding_limits);
> }
> }
> exit:
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index a386a1c..39575eb 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -207,6 +207,16 @@ struct thermal_bind_params {
> * See Documentation/thermal/sysfs-api.txt for more information.
> */
> int trip_mask;
> +
> + /*
> + * This is an array of cooling state limits. Must have exactly
> + * 2 * thermal_zone.number_of_trip_points. It is an array consisting
> + * of tuples <lower-state upper-state> of state limits. Each trip
> + * will be associated with one state limit tuple when binding.
> + * A NULL pointer means <THERMAL_NO_LIMITS THERMAL_NO_LIMITS>
> + * on all trips.
> + */
> + unsigned long *binding_limits;
> int (*match) (struct thermal_zone_device *tz,
> struct thermal_cooling_device *cdev);
> };
> --
> 1.8.2.1.342.gfa7285d
>
>
> _______________________________________________
> lm-sensors mailing list
> [email protected]
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
> -----Original Message-----
> From: [email protected] [mailto:lm-sensors-bounces@lm-
> sensors.org] On Behalf Of Eduardo Valentin
> Sent: Wednesday, July 17, 2013 8:47 PM
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; Eduardo Valentin; Zhang, Rui;
> [email protected]
> Subject: [lm-sensors] [RESEND PATCH V1 2/9] 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.
>
> In order to do this move, the hwmon list head is now
> using its own locking. Before, the list used
> the global thermal locking. Also, some minor changes
> in the code were required, as recommended by checkpatch.pl.
>
> Cc: Zhang Rui <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Eduardo Valentin <[email protected]>
Went through the patch, looks fine. Nice clean up Eduardo!!
Acked-by: Durgadoss R <[email protected]>
Thanks,
Durga
> ---
> drivers/thermal/Kconfig | 9 ++
> drivers/thermal/Makefile | 3 +
> drivers/thermal/thermal_core.c | 255 +------------------------------------
> drivers/thermal/thermal_hwmon.c | 269
> ++++++++++++++++++++++++++++++++++++++++
> drivers/thermal/thermal_hwmon.h | 49 ++++++++
> 5 files changed, 331 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..ab52a96
> --- /dev/null
> +++ b/drivers/thermal/thermal_hwmon.c
> @@ -0,0 +1,269 @@
> +/*
> + * 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 <[email protected]>
> + * Copyright (C) 2008 Sujith Thomas <[email protected]>
> + *
> + * Copyright (C) 2013 Texas Instruments
> + * Copyright (C) 2013 Eduardo Valentin <[email protected]>
> + *
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~
> + *
> + * 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>
> +#include "thermal_hwmon.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(*hwmon), 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(*hwmon), 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 <[email protected]>
> + * Copyright (C) 2008 Sujith Thomas <[email protected]>
> + *
> + * Copyright (C) 2013 Texas Instruments
> + * Copyright (C) 2013 Eduardo Valentin <[email protected]>
> + *
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~
> + *
> + * 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
>
>
> _______________________________________________
> lm-sensors mailing list
> [email protected]
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin wrote:
> Hello all,
>
> As you noticed, I am working in a way to represent thermal data
> using device tree [1]. Essentially, this should be a way to say
> what to do with a sensor and how to associate (cooling) actions
> with it.
>
Seems to me that goes way beyond the supposed scope of devicetree data.
Devicetree data is supposed to describe hardware, not its configuration or use.
This is clearly a use case.
Guenter
> The motivation to create such infrastructure is:
> (i) - to reuse the existing temperature sensor code base;
> (ii) - have a way to easily describe thermal data across different
> boards for the same sensor. Say you have an i2c temp sensor,
> which is placed close to your battery on board A but on
> board B, another instance of this same senor is placed
> close to your display, for instance.
>
> This series introduces then a DT parser. The data expected in
> DT must contain the needed properties to build a thermal zone
> out of the desired sensor. All properties are documented and
> they are derived from the existing requirements of current
> thermal API.
>
> In order to perform a binding with cooling devices,
> the new thermal zone built using DT nodes will use
> the existing thermal API that uses binding parameters. This is
> the current proposed way to register thermal zones with platform
> information, written by Durga.
>
> There are some virtual concepts that are pushed to device tree,
> I know. But I believe using device tree to do this makes sense
> because we are still describing the HW and how they are related
> to each other. Things like cooling devices are not represented
> in device tree though, as I believe that will cause a lot of
> confusion with real devices (as already does).
>
> So the series is short but I think it makes sense to describe
> how it is organized, as it touches several places. First four
> patches are a preparation for this parser. There is a change
> on cpufreq-cpu0, so that it knows now how to load its
> corresponding cooling device. There is a change on thermal_core
> to split its hwmon code, because I think we may need to improve
> this code base when we start to integrate better with hwmon
> temperature sensors. Then, another needed preparation is to
> improve the bind_params, so that we are able to bind with
> upper and lower limits. The remaining patches are the changes
> with the proposed DT parser. A part from the DT thermal zone
> builder itself (patch 05), I also changed the ti-soc-thermal
> driver to use this new API and the omap4430 bandgap DT node,
> as an example (this has been tested on panda omap4430); and also changed
> the hwmon drivers lm75 and tmp102 to have the option to build
> a thermal zone using DT. I haven't touched any dts file using
> lm75 or tmp102 because this should come on a need basis.
>
> I believe this code is pretty usable the way it is, but might
> need to be revisited, in case the enhancement proposed by Durga
> get in. This is because representing virtual thermal zones
> built out of several sensors may need a different representation
> in DT.
>
> [1] - RFC of this work:
> http://comments.gmane.org/gmane.linux.power-management.general/35874
>
> Changes from RFC:
> - Added a way to load cpufreq cooling device out of DT
> - Added a way to bind with upper and lower limits using bind_params
> - Added a way to specify upper and lower binding limits on DT
> - Added DT thermal builder support to lm75 and tmp102 hwmon drivers
> - Changed ERANGE to EDOM
> - Added thermal constants for zone type and bind limit, so that
> dts file could be more readable
>
> Tested on panda omap4430 (3.11-rc1 with minor changes for getting cpufreq working)
>
> BR,
>
> Eduardo Valentin (9):
> cpufreq: cpufreq-cpu0: add dt node parsing for 'needs-cooling'
> thermal: hwmon: move hwmon support to single file
> thermal: thermal_core: allow binding with limits on bind_params
> arm: dts: flag omap4430 with needs-cooling for cpu node
> thermal: introduce device tree parser
> thermal: ti-soc-thermal: use thermal DT infrastructure
> arm: dts: add omap4430 thermal data
> hwmon: lm75: expose to thermal fw via DT nodes
> hwmon: tmp102: expose to thermal fw via DT nodes
>
> .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt | 3 +
> .../devicetree/bindings/thermal/thermal.txt | 133 ++++++
> Documentation/thermal/sysfs-api.txt | 7 +
> arch/arm/boot/dts/omap443x.dtsi | 32 +-
> drivers/cpufreq/cpufreq-cpu0.c | 8 +
> drivers/hwmon/lm75.c | 29 ++
> drivers/hwmon/tmp102.c | 25 ++
> drivers/thermal/Kconfig | 22 +
> drivers/thermal/Makefile | 4 +
> drivers/thermal/thermal_core.c | 274 +-----------
> drivers/thermal/thermal_dt.c | 458 +++++++++++++++++++++
> drivers/thermal/thermal_hwmon.c | 269 ++++++++++++
> drivers/thermal/thermal_hwmon.h | 49 +++
> drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 46 ++-
> include/dt-bindings/thermal/thermal.h | 27 ++
> include/linux/thermal.h | 13 +
> 16 files changed, 1129 insertions(+), 270 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/thermal/thermal.txt
> create mode 100644 drivers/thermal/thermal_dt.c
> create mode 100644 drivers/thermal/thermal_hwmon.c
> create mode 100644 drivers/thermal/thermal_hwmon.h
> create mode 100644 include/dt-bindings/thermal/thermal.h
>
> --
> 1.8.2.1.342.gfa7285d
>
>
> _______________________________________________
> lm-sensors mailing list
> [email protected]
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
>
On 07/17/2013 11:17 PM, Eduardo Valentin wrote:
> This patch adds to lm75 temperature sensor the possibility
> to expose itself as thermal zone device, registered on the
> thermal framework.
>
> The thermal zone is built only if a device tree node
> describing a thermal zone for this sensor is present
> inside the lm75 DT node. Otherwise, the driver behavior
> will be the same.
>
> Cc: Jean Delvare <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Eduardo Valentin <[email protected]>
> ---
> drivers/hwmon/lm75.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index c03b490..0aa5e28 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -27,6 +27,8 @@
> #include <linux/hwmon-sysfs.h>
> #include <linux/err.h>
> #include <linux/mutex.h>
> +#include <linux/thermal.h>
> +#include <linux/of.h>
> #include "lm75.h"
>
>
> @@ -70,6 +72,7 @@ static const u8 LM75_REG_TEMP[3] = {
> /* Each client has this additional data */
> struct lm75_data {
> struct device *hwmon_dev;
> + struct thermal_zone_device *tz;
> struct mutex update_lock;
> u8 orig_conf;
> u8 resolution; /* In bits, between 9 and 12 */
> @@ -92,6 +95,19 @@ static struct lm75_data *lm75_update_device(struct device *dev);
>
> /* sysfs attributes for hwmon */
>
> +static int lm75_read_temp(void *dev, unsigned long *temp)
> +{
> + struct lm75_data *data = lm75_update_device(dev);
> +
> + if (IS_ERR(data))
> + return PTR_ERR(data);
> +
> + *temp = ((data->temp[0] >> (16 - data->resolution)) * 1000) >>
> + (data->resolution - 8);
> +
> + return 0;
> +}
> +
> static ssize_t show_temp(struct device *dev, struct device_attribute *da,
> char *buf)
> {
> @@ -271,11 +287,23 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
> goto exit_remove;
> }
>
> + if (of_find_node_by_name(client->dev.of_node, "thermal_zone")) {
> + data->tz = thermal_zone_of_device_register(&client->dev,
> + &client->dev,
> + lm75_read_temp);
Hi, Eduardo
I have two questions:
1. As we know, after register to the thermal framework, it will have
duplicate hwmon devices. I think lm-sensor maintainer would not like this.
How about to add a flag to indicate it, which I talk about with
Durgadoss in your [RFC 1/4]patch.
2. I'm also trying to use your codes on lm90. The LM90 serial has more
then one sensors in one chip, local sensor, remote sensor and may have
remote2 sensor, so it mean there may have more than one thermal_zone
under the lm90 device node, will you consider it?
Thanks.
Wei.
> + if (IS_ERR(data->tz)) {
> + status = PTR_ERR(data->tz);
> + goto exit_hwmon;
> + }
> + }
> +
> dev_info(&client->dev, "%s: sensor '%s'\n",
> dev_name(data->hwmon_dev), client->name);
>
> return 0;
>
> +exit_hwmon:
> + hwmon_device_unregister(data->hwmon_dev);
> exit_remove:
> sysfs_remove_group(&client->dev.kobj, &lm75_group);
> return status;
> @@ -285,6 +313,7 @@ static int lm75_remove(struct i2c_client *client)
> {
> struct lm75_data *data = i2c_get_clientdata(client);
>
> + thermal_zone_device_unregister(data->tz);
> hwmon_device_unregister(data->hwmon_dev);
> sysfs_remove_group(&client->dev.kobj, &lm75_group);
> lm75_write_value(client, LM75_REG_CONF, data->orig_conf);
>
On Wed, Jul 17, 2013 at 11:17:27AM -0400, Eduardo Valentin wrote:
> This patch adds to lm75 temperature sensor the possibility
> to expose itself as thermal zone device, registered on the
> thermal framework.
>
> The thermal zone is built only if a device tree node
> describing a thermal zone for this sensor is present
> inside the lm75 DT node. Otherwise, the driver behavior
> will be the same.
>
As mentioned before, your usage of devicetree information is clearly
outside the scope for devicetree data.
One clear guideline for devicetree data is that it is supposed to be usable
across multiple operating systems. Your proposal is clearly operating system
and even subsystem specific. It does _not_ describe hardware.
NACK.
Guenter
> Cc: Jean Delvare <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Eduardo Valentin <[email protected]>
> ---
> drivers/hwmon/lm75.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index c03b490..0aa5e28 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -27,6 +27,8 @@
> #include <linux/hwmon-sysfs.h>
> #include <linux/err.h>
> #include <linux/mutex.h>
> +#include <linux/thermal.h>
> +#include <linux/of.h>
> #include "lm75.h"
>
>
> @@ -70,6 +72,7 @@ static const u8 LM75_REG_TEMP[3] = {
> /* Each client has this additional data */
> struct lm75_data {
> struct device *hwmon_dev;
> + struct thermal_zone_device *tz;
> struct mutex update_lock;
> u8 orig_conf;
> u8 resolution; /* In bits, between 9 and 12 */
> @@ -92,6 +95,19 @@ static struct lm75_data *lm75_update_device(struct device *dev);
>
> /* sysfs attributes for hwmon */
>
> +static int lm75_read_temp(void *dev, unsigned long *temp)
> +{
> + struct lm75_data *data = lm75_update_device(dev);
> +
> + if (IS_ERR(data))
> + return PTR_ERR(data);
> +
> + *temp = ((data->temp[0] >> (16 - data->resolution)) * 1000) >>
> + (data->resolution - 8);
> +
> + return 0;
> +}
> +
> static ssize_t show_temp(struct device *dev, struct device_attribute *da,
> char *buf)
> {
> @@ -271,11 +287,23 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
> goto exit_remove;
> }
>
> + if (of_find_node_by_name(client->dev.of_node, "thermal_zone")) {
> + data->tz = thermal_zone_of_device_register(&client->dev,
> + &client->dev,
> + lm75_read_temp);
> + if (IS_ERR(data->tz)) {
> + status = PTR_ERR(data->tz);
> + goto exit_hwmon;
> + }
> + }
> +
> dev_info(&client->dev, "%s: sensor '%s'\n",
> dev_name(data->hwmon_dev), client->name);
>
> return 0;
>
> +exit_hwmon:
> + hwmon_device_unregister(data->hwmon_dev);
> exit_remove:
> sysfs_remove_group(&client->dev.kobj, &lm75_group);
> return status;
> @@ -285,6 +313,7 @@ static int lm75_remove(struct i2c_client *client)
> {
> struct lm75_data *data = i2c_get_clientdata(client);
>
> + thermal_zone_device_unregister(data->tz);
> hwmon_device_unregister(data->hwmon_dev);
> sysfs_remove_group(&client->dev.kobj, &lm75_group);
> lm75_write_value(client, LM75_REG_CONF, data->orig_conf);
> --
> 1.8.2.1.342.gfa7285d
>
>
On Wed, Jul 17, 2013 at 11:17:28AM -0400, Eduardo Valentin wrote:
> This patch adds to tmp102 temperature sensor the possibility
> to expose itself as thermal zone device, registered on the
> thermal framework.
>
> The thermal zone is built only if a device tree node
> describing a thermal zone for this sensor is present
> inside the tmp102 DT node. Otherwise, the driver behavior
> will be the same.
>
NACK, same reason as for the lm75 patch.
Guenter
Hi Wei,
On 18-07-2013 01:33, Wei Ni wrote:
> On 07/17/2013 11:17 PM, Eduardo Valentin wrote:
>> This patch adds to lm75 temperature sensor the possibility
>> to expose itself as thermal zone device, registered on the
>> thermal framework.
>>
>> The thermal zone is built only if a device tree node
>> describing a thermal zone for this sensor is present
>> inside the lm75 DT node. Otherwise, the driver behavior
>> will be the same.
>>
>> Cc: Jean Delvare <[email protected]>
>> Cc: Guenter Roeck <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Eduardo Valentin <[email protected]>
>> ---
>> drivers/hwmon/lm75.c | 29 +++++++++++++++++++++++++++++
>> 1 file changed, 29 insertions(+)
>>
>> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
>> index c03b490..0aa5e28 100644
>> --- a/drivers/hwmon/lm75.c
>> +++ b/drivers/hwmon/lm75.c
>> @@ -27,6 +27,8 @@
>> #include <linux/hwmon-sysfs.h>
>> #include <linux/err.h>
>> #include <linux/mutex.h>
>> +#include <linux/thermal.h>
>> +#include <linux/of.h>
>> #include "lm75.h"
>>
>>
>> @@ -70,6 +72,7 @@ static const u8 LM75_REG_TEMP[3] = {
>> /* Each client has this additional data */
>> struct lm75_data {
>> struct device *hwmon_dev;
>> + struct thermal_zone_device *tz;
>> struct mutex update_lock;
>> u8 orig_conf;
>> u8 resolution; /* In bits, between 9 and 12 */
>> @@ -92,6 +95,19 @@ static struct lm75_data *lm75_update_device(struct device *dev);
>>
>> /* sysfs attributes for hwmon */
>>
>> +static int lm75_read_temp(void *dev, unsigned long *temp)
>> +{
>> + struct lm75_data *data = lm75_update_device(dev);
>> +
>> + if (IS_ERR(data))
>> + return PTR_ERR(data);
>> +
>> + *temp = ((data->temp[0] >> (16 - data->resolution)) * 1000) >>
>> + (data->resolution - 8);
>> +
>> + return 0;
>> +}
>> +
>> static ssize_t show_temp(struct device *dev, struct device_attribute *da,
>> char *buf)
>> {
>> @@ -271,11 +287,23 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
>> goto exit_remove;
>> }
>>
>> + if (of_find_node_by_name(client->dev.of_node, "thermal_zone")) {
>> + data->tz = thermal_zone_of_device_register(&client->dev,
>> + &client->dev,
>> + lm75_read_temp);
>
> Hi, Eduardo
> I have two questions:
> 1. As we know, after register to the thermal framework, it will have
> duplicate hwmon devices. I think lm-sensor maintainer would not like this.
Yes I noticed. You have always the option to disable the
CONFIG_THERMAL_HWMON in your build.
> How about to add a flag to indicate it, which I talk about with
> Durgadoss in your [RFC 1/4]patch.
This would be very much appreciated, but I don't think this is a blocker
for this series. We can of course include this patch on it. Having a
flag to control this thermal fw to hwmon interface is actually a good idea.
>
> 2. I'm also trying to use your codes on lm90. The LM90 serial has more
> then one sensors in one chip, local sensor, remote sensor and may have
> remote2 sensor, so it mean there may have more than one thermal_zone
> under the lm90 device node, will you consider it?
>
I haven't looked lm90 source code. How do you map it? Do you probe each
sensor or do you probe 1 device and expose 3 sensors?
In first case, you can still reuse what is in this series.
Later case, we need to change it slightly.
That is the case which is pretty similar on OMAP5 for instance, where
the device has three sensors. This patch series does not cover for this
case. But it can be simply modified to get around it.
We would need to allow more than one 'thermal_zone' nodes inside a
single device. But then we would need to have a way to determine which
sensor goes to which zone too.
> Thanks.
> Wei.
>
>> + if (IS_ERR(data->tz)) {
>> + status = PTR_ERR(data->tz);
>> + goto exit_hwmon;
>> + }
>> + }
>> +
>> dev_info(&client->dev, "%s: sensor '%s'\n",
>> dev_name(data->hwmon_dev), client->name);
>>
>> return 0;
>>
>> +exit_hwmon:
>> + hwmon_device_unregister(data->hwmon_dev);
>> exit_remove:
>> sysfs_remove_group(&client->dev.kobj, &lm75_group);
>> return status;
>> @@ -285,6 +313,7 @@ static int lm75_remove(struct i2c_client *client)
>> {
>> struct lm75_data *data = i2c_get_clientdata(client);
>>
>> + thermal_zone_device_unregister(data->tz);
>> hwmon_device_unregister(data->hwmon_dev);
>> sysfs_remove_group(&client->dev.kobj, &lm75_group);
>> lm75_write_value(client, LM75_REG_CONF, data->orig_conf);
>>
>
>
>
--
You have got to be excited about what you are doing. (L. Lamport)
Eduardo Valentin
Hello Guenter,
On 17-07-2013 18:09, Guenter Roeck wrote:
> On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin wrote:
>> Hello all,
>>
>> As you noticed, I am working in a way to represent thermal data
>> using device tree [1]. Essentially, this should be a way to say
>> what to do with a sensor and how to associate (cooling) actions
>> with it.
>>
> Seems to me that goes way beyond the supposed scope of devicetree data.
> Devicetree data is supposed to describe hardware, not its configuration or use.
> This is clearly a use case.
Thanks for rising your voice here. It is important to know what hwmon
ppl think about this.
>
> Guenter
As your answers to the series are giving same argument, I chose to
answer on patch 0. I would be happier if you could elaborate a bit more
on your concern, specially if you take hwmon cap here, and give your
view with that perspective.
I also considered that this work could be abusing of DT purposes. But
let me explain why I still think it makes sense to have it.
First thing is that this series intend to describe the thermal data
required for a specific board. That means, considering your board
layout, mechanics, power dissipation and composition of your ICs, etc,
that will impose thermal requirements on your system. That is not
configuration, but part of your board design and non-functional
requirement. To me, configuration would be something like saying you
want to use a specific keyboard layout, or defining your wifi card
channel, or display size, etc.
Here what is described and setup may get confused with configuration,
but it is not because what goes in DT in this case must be actually
derived from your HW needs. Putting a sensor close to your battery has
a strong meaning behind. Same if you put a sensor close to your
processor. For instance, we have cases we need to consider external heat
in order to properly determine our CPU hotspot level, using a board
sensor. That is what I mean by HW requirement/need.
Again, just to refresh our minds, this is about protecting your board
and ICs from operating out of their spec and extending their lifetime.
This is not about configuring something just because user has chosen to.
That is definitely not a configuration.
Being a use case, well, these new DT nodes can still be seen as a use
case, yes. But depends on your understanding of use case. Because a
sensor device may be used on different needs, composing different use
cases. But still here we are talking about HW needs and composition. And
yes, if you take that perspective, there are use cases already described
in DT.
For instance, just because you use an LDO to power a MMC, does it mean
you always will use it to power MMC on all boards. Would that be a use
case? And in that example, because your MMC requires 2.8V, if you have a
DT property to say that, does it mean it is configuration? Well, yes,
but that is based on HW needs, otherwise it wont operate properly. Same
thing here, leave your hw operating out of temperature specs and you
will see what is the outcome.
Similar example would be cpufreq, or OPPs for opp layer. Defining an OPP
in DT could be considered a configuration? Well in theory yes, one can
pick what ever configuration for your (D)PLL then match with a
minimalist voltage level. And in theory, a voltage level can sustain
more than one frequency level. An OPP is still a virtual concept, and we
still describe it in DT. Why? To me is because it is a HW need, because
HW folks have actually validated that configuration of PLL (frequency)
and voltage level. Sometimes they have even optimized it (for low power
consumption for instance), as one may achieve same OPP with different
configuration. Then why thermal data, which is again, a 'HW
configuration' that has been optimized by HW folks, known to be a HW
requirement, cannot be described in DT?
Similar argument goes to the fact that one may think this is subsystem
specific. Again, describing your OPPs is not OPP layer specific?
Besides, one can still read the device tree nodes I have written for
describing thermal data and implement the HW requirement elsewhere, if
wanted (say in user land). I don't see as subsystem specific.
Keep in mind that these very same concepts are actually derived from
ACPI specification. They don't come from nowhere. And just because your
system does not have ACPI support, does not mean it won't have a need to
describe thermal?
So, because with this work (i) we are describing something that is
required for properly usage of your HW (and not choice of the user),
because (ii) same data is used on different specification (that is used
on different OSes too), because (iii) you don't need thermal fw to read
this data and you could implement the HW requirement elsewhere, because
(iv) there are other similar requirements already implemented via DT; I
still think this work is within DT scope. And that is based on evidence
of existing work not because DT is nice and I would want to use it.
Hope that clarifies.
Of course it is always welcome to hear ppl opinion. I would be really
interested to know what ppl from OF think about this topic.
If still, this does not fit DT, I would like to understand a proper
argument. Better than, this is configuration/use case.
>
>> The motivation to create such infrastructure is:
>> (i) - to reuse the existing temperature sensor code base;
>> (ii) - have a way to easily describe thermal data across different
>> boards for the same sensor. Say you have an i2c temp sensor,
>> which is placed close to your battery on board A but on
>> board B, another instance of this same senor is placed
>> close to your display, for instance.
>>
>> This series introduces then a DT parser. The data expected in
>> DT must contain the needed properties to build a thermal zone
>> out of the desired sensor. All properties are documented and
>> they are derived from the existing requirements of current
>> thermal API.
>>
>> In order to perform a binding with cooling devices,
>> the new thermal zone built using DT nodes will use
>> the existing thermal API that uses binding parameters. This is
>> the current proposed way to register thermal zones with platform
>> information, written by Durga.
>>
>> There are some virtual concepts that are pushed to device tree,
>> I know. But I believe using device tree to do this makes sense
>> because we are still describing the HW and how they are related
>> to each other. Things like cooling devices are not represented
>> in device tree though, as I believe that will cause a lot of
>> confusion with real devices (as already does).
>>
>> So the series is short but I think it makes sense to describe
>> how it is organized, as it touches several places. First four
>> patches are a preparation for this parser. There is a change
>> on cpufreq-cpu0, so that it knows now how to load its
>> corresponding cooling device. There is a change on thermal_core
>> to split its hwmon code, because I think we may need to improve
>> this code base when we start to integrate better with hwmon
>> temperature sensors. Then, another needed preparation is to
>> improve the bind_params, so that we are able to bind with
>> upper and lower limits. The remaining patches are the changes
>> with the proposed DT parser. A part from the DT thermal zone
>> builder itself (patch 05), I also changed the ti-soc-thermal
>> driver to use this new API and the omap4430 bandgap DT node,
>> as an example (this has been tested on panda omap4430); and also changed
>> the hwmon drivers lm75 and tmp102 to have the option to build
>> a thermal zone using DT. I haven't touched any dts file using
>> lm75 or tmp102 because this should come on a need basis.
>>
>> I believe this code is pretty usable the way it is, but might
>> need to be revisited, in case the enhancement proposed by Durga
>> get in. This is because representing virtual thermal zones
>> built out of several sensors may need a different representation
>> in DT.
>>
>> [1] - RFC of this work:
>> http://comments.gmane.org/gmane.linux.power-management.general/35874
>>
>> Changes from RFC:
>> - Added a way to load cpufreq cooling device out of DT
>> - Added a way to bind with upper and lower limits using bind_params
>> - Added a way to specify upper and lower binding limits on DT
>> - Added DT thermal builder support to lm75 and tmp102 hwmon drivers
>> - Changed ERANGE to EDOM
>> - Added thermal constants for zone type and bind limit, so that
>> dts file could be more readable
>>
>> Tested on panda omap4430 (3.11-rc1 with minor changes for getting cpufreq working)
>>
>> BR,
>>
>> Eduardo Valentin (9):
>> cpufreq: cpufreq-cpu0: add dt node parsing for 'needs-cooling'
>> thermal: hwmon: move hwmon support to single file
>> thermal: thermal_core: allow binding with limits on bind_params
>> arm: dts: flag omap4430 with needs-cooling for cpu node
>> thermal: introduce device tree parser
>> thermal: ti-soc-thermal: use thermal DT infrastructure
>> arm: dts: add omap4430 thermal data
>> hwmon: lm75: expose to thermal fw via DT nodes
>> hwmon: tmp102: expose to thermal fw via DT nodes
>>
>> .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt | 3 +
>> .../devicetree/bindings/thermal/thermal.txt | 133 ++++++
>> Documentation/thermal/sysfs-api.txt | 7 +
>> arch/arm/boot/dts/omap443x.dtsi | 32 +-
>> drivers/cpufreq/cpufreq-cpu0.c | 8 +
>> drivers/hwmon/lm75.c | 29 ++
>> drivers/hwmon/tmp102.c | 25 ++
>> drivers/thermal/Kconfig | 22 +
>> drivers/thermal/Makefile | 4 +
>> drivers/thermal/thermal_core.c | 274 +-----------
>> drivers/thermal/thermal_dt.c | 458 +++++++++++++++++++++
>> drivers/thermal/thermal_hwmon.c | 269 ++++++++++++
>> drivers/thermal/thermal_hwmon.h | 49 +++
>> drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 46 ++-
>> include/dt-bindings/thermal/thermal.h | 27 ++
>> include/linux/thermal.h | 13 +
>> 16 files changed, 1129 insertions(+), 270 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/thermal/thermal.txt
>> create mode 100644 drivers/thermal/thermal_dt.c
>> create mode 100644 drivers/thermal/thermal_hwmon.c
>> create mode 100644 drivers/thermal/thermal_hwmon.h
>> create mode 100644 include/dt-bindings/thermal/thermal.h
>>
>> --
>> 1.8.2.1.342.gfa7285d
>>
>>
>> _______________________________________________
>> lm-sensors mailing list
>> [email protected]
>> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
>>
>
>
--
You have got to be excited about what you are doing. (L. Lamport)
Eduardo Valentin
On 07/18/2013 07:53 AM, Eduardo Valentin wrote:
> Hello Guenter,
>
> On 17-07-2013 18:09, Guenter Roeck wrote:
>> On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin
>> wrote:
>>> Hello all,
>>>
>>> As you noticed, I am working in a way to represent thermal
>>> data using device tree [1]. Essentially, this should be a way
>>> to say what to do with a sensor and how to associate (cooling)
>>> actions with it.
>>>
>> Seems to me that goes way beyond the supposed scope of devicetree
>> data. Devicetree data is supposed to describe hardware, not its
>> configuration or use. This is clearly a use case.
>
> Thanks for rising your voice here. It is important to know what
> hwmon ppl think about this.
I meant to find time to read Guenter's original email where he
initially objected to putting data into DT, and determine exactly what
was being objected to. I still haven't:-( However, the arguments that
Eduardo stated in his email do make sense to me; I agree that
temperature limits really are a description of HW. Details of which
cooling methods to invoke when certain temperature limits are reached
is also part of the HW/system design, and hence I would tend to agree
that they're appropriate to include in DT. Anyway, that's just my 2
cents on the matter:-)
On Thu, Jul 18, 2013 at 09:53:05AM -0400, Eduardo Valentin wrote:
> Hello Guenter,
>
> On 17-07-2013 18:09, Guenter Roeck wrote:
> > On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin wrote:
> >> Hello all,
> >>
> >> As you noticed, I am working in a way to represent thermal data
> >> using device tree [1]. Essentially, this should be a way to say
> >> what to do with a sensor and how to associate (cooling) actions
> >> with it.
> >>
> > Seems to me that goes way beyond the supposed scope of devicetree data.
> > Devicetree data is supposed to describe hardware, not its configuration or use.
> > This is clearly a use case.
>
> Thanks for rising your voice here. It is important to know what hwmon
> ppl think about this.
>
Sorry, I don't know what ppl stands for.
> >
> > Guenter
>
> As your answers to the series are giving same argument, I chose to
> answer on patch 0. I would be happier if you could elaborate a bit more
> on your concern, specially if you take hwmon cap here, and give your
> view with that perspective.
>
> I also considered that this work could be abusing of DT purposes. But
> let me explain why I still think it makes sense to have it.
>
Ultimately, you are making my point here. If you considered it, did you ask
devicetree experts for an opinion ? Did you discuss the subject on the
devicetree-discuss mailing list ? If so, what was the result ?
> First thing is that this series intend to describe the thermal data
> required for a specific board. That means, considering your board
> layout, mechanics, power dissipation and composition of your ICs, etc,
> that will impose thermal requirements on your system. That is not
> configuration, but part of your board design and non-functional
> requirement. To me, configuration would be something like saying you
> want to use a specific keyboard layout, or defining your wifi card
> channel, or display size, etc.
>
> Here what is described and setup may get confused with configuration,
> but it is not because what goes in DT in this case must be actually
> derived from your HW needs. Putting a sensor close to your battery has
> a strong meaning behind. Same if you put a sensor close to your
> processor. For instance, we have cases we need to consider external heat
> in order to properly determine our CPU hotspot level, using a board
> sensor. That is what I mean by HW requirement/need.
>
> Again, just to refresh our minds, this is about protecting your board
> and ICs from operating out of their spec and extending their lifetime.
> This is not about configuring something just because user has chosen to.
> That is definitely not a configuration.
>
> Being a use case, well, these new DT nodes can still be seen as a use
> case, yes. But depends on your understanding of use case. Because a
> sensor device may be used on different needs, composing different use
> cases. But still here we are talking about HW needs and composition. And
> yes, if you take that perspective, there are use cases already described
> in DT.
>
> For instance, just because you use an LDO to power a MMC, does it mean
> you always will use it to power MMC on all boards. Would that be a use
> case? And in that example, because your MMC requires 2.8V, if you have a
> DT property to say that, does it mean it is configuration? Well, yes,
> but that is based on HW needs, otherwise it wont operate properly. Same
> thing here, leave your hw operating out of temperature specs and you
> will see what is the outcome.
>
> Similar example would be cpufreq, or OPPs for opp layer. Defining an OPP
> in DT could be considered a configuration? Well in theory yes, one can
> pick what ever configuration for your (D)PLL then match with a
> minimalist voltage level. And in theory, a voltage level can sustain
> more than one frequency level. An OPP is still a virtual concept, and we
> still describe it in DT. Why? To me is because it is a HW need, because
> HW folks have actually validated that configuration of PLL (frequency)
> and voltage level. Sometimes they have even optimized it (for low power
> consumption for instance), as one may achieve same OPP with different
> configuration. Then why thermal data, which is again, a 'HW
> configuration' that has been optimized by HW folks, known to be a HW
> requirement, cannot be described in DT?
>
> Similar argument goes to the fact that one may think this is subsystem
> specific. Again, describing your OPPs is not OPP layer specific?
> Besides, one can still read the device tree nodes I have written for
> describing thermal data and implement the HW requirement elsewhere, if
> wanted (say in user land). I don't see as subsystem specific.
>
> Keep in mind that these very same concepts are actually derived from
> ACPI specification. They don't come from nowhere. And just because your
> system does not have ACPI support, does not mean it won't have a need to
> describe thermal?
>
> So, because with this work (i) we are describing something that is
> required for properly usage of your HW (and not choice of the user),
> because (ii) same data is used on different specification (that is used
> on different OSes too), because (iii) you don't need thermal fw to read
> this data and you could implement the HW requirement elsewhere, because
> (iv) there are other similar requirements already implemented via DT; I
> still think this work is within DT scope. And that is based on evidence
> of existing work not because DT is nice and I would want to use it.
>
> Hope that clarifies.
>
> Of course it is always welcome to hear ppl opinion. I would be really
> interested to know what ppl from OF think about this topic.
>
Yes, me too, or more widely the devicetree community. This is exactly
why I brought it up.
> If still, this does not fit DT, I would like to understand a proper
> argument. Better than, this is configuration/use case.
>
I am not a devicetree expert. One of the complaints by the devicetree
folks is that much is added to devicetree which should not be there.
I find this use case questionable. Maybe I am wrong and it isn't,
which may well be. But you thought about it as well, so I don't think
I am that far off track.
This needs to be discussed and determined by devicetree experts, not me.
I'll be happy with your patch series if you get an agreement or an Ack
by the devicetree maintainer for your patch series.
Guenter
On Thu, Jul 18, 2013 at 11:18:05AM -0600, Stephen Warren wrote:
> On 07/18/2013 07:53 AM, Eduardo Valentin wrote:
> > Hello Guenter,
> >
> > On 17-07-2013 18:09, Guenter Roeck wrote:
> >> On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin
> >> wrote:
> >>> Hello all,
> >>>
> >>> As you noticed, I am working in a way to represent thermal
> >>> data using device tree [1]. Essentially, this should be a way
> >>> to say what to do with a sensor and how to associate (cooling)
> >>> actions with it.
> >>>
> >> Seems to me that goes way beyond the supposed scope of devicetree
> >> data. Devicetree data is supposed to describe hardware, not its
> >> configuration or use. This is clearly a use case.
> >
> > Thanks for rising your voice here. It is important to know what
> > hwmon ppl think about this.
>
> I meant to find time to read Guenter's original email where he
> initially objected to putting data into DT, and determine exactly what
> was being objected to. I still haven't:-( However, the arguments that
> Eduardo stated in his email do make sense to me; I agree that
> temperature limits really are a description of HW. Details of which
> cooling methods to invoke when certain temperature limits are reached
> is also part of the HW/system design, and hence I would tend to agree
> that they're appropriate to include in DT. Anyway, that's just my 2
> cents on the matter:-)
Many systems have multiple profiles for various use cases (high performance,
low power etc), and limits are different based on the use case. If that means
you are going to have multiple devicetree variants based on the profile,
I would argue that you crossed the line. With thermal profiles it gets even more
complicated, as those parameters may be played around with and changed
multiple times to find the best settings to achieve optimal cooling.
Does this describe hardware ? I don't think so, but, as I mentioned before,
maybe I am wrong.
Guenter
On 07/18/2013 09:12 PM, Eduardo Valentin wrote:
> * PGP Signed: 07/18/2013 at 06:12:01 AM
>
> Hi Wei,
>
>
> On 18-07-2013 01:33, Wei Ni wrote:
>> On 07/17/2013 11:17 PM, Eduardo Valentin wrote:
>>> This patch adds to lm75 temperature sensor the possibility
>>> to expose itself as thermal zone device, registered on the
>>> thermal framework.
>>>
>>> The thermal zone is built only if a device tree node
>>> describing a thermal zone for this sensor is present
>>> inside the lm75 DT node. Otherwise, the driver behavior
>>> will be the same.
>>>
>>> Cc: Jean Delvare <[email protected]>
>>> Cc: Guenter Roeck <[email protected]>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Signed-off-by: Eduardo Valentin <[email protected]>
>>> ---
>>> drivers/hwmon/lm75.c | 29 +++++++++++++++++++++++++++++
>>> 1 file changed, 29 insertions(+)
>>>
>>> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
>>> index c03b490..0aa5e28 100644
>>> --- a/drivers/hwmon/lm75.c
>>> +++ b/drivers/hwmon/lm75.c
>>> @@ -27,6 +27,8 @@
>>> #include <linux/hwmon-sysfs.h>
>>> #include <linux/err.h>
>>> #include <linux/mutex.h>
>>> +#include <linux/thermal.h>
>>> +#include <linux/of.h>
>>> #include "lm75.h"
>>>
>>>
>>> @@ -70,6 +72,7 @@ static const u8 LM75_REG_TEMP[3] = {
>>> /* Each client has this additional data */
>>> struct lm75_data {
>>> struct device *hwmon_dev;
>>> + struct thermal_zone_device *tz;
>>> struct mutex update_lock;
>>> u8 orig_conf;
>>> u8 resolution; /* In bits, between 9 and 12 */
>>> @@ -92,6 +95,19 @@ static struct lm75_data *lm75_update_device(struct device *dev);
>>>
>>> /* sysfs attributes for hwmon */
>>>
>>> +static int lm75_read_temp(void *dev, unsigned long *temp)
>>> +{
>>> + struct lm75_data *data = lm75_update_device(dev);
>>> +
>>> + if (IS_ERR(data))
>>> + return PTR_ERR(data);
>>> +
>>> + *temp = ((data->temp[0] >> (16 - data->resolution)) * 1000) >>
>>> + (data->resolution - 8);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static ssize_t show_temp(struct device *dev, struct device_attribute *da,
>>> char *buf)
>>> {
>>> @@ -271,11 +287,23 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
>>> goto exit_remove;
>>> }
>>>
>>> + if (of_find_node_by_name(client->dev.of_node, "thermal_zone")) {
>>> + data->tz = thermal_zone_of_device_register(&client->dev,
>>> + &client->dev,
>>> + lm75_read_temp);
>>
>> Hi, Eduardo
>> I have two questions:
>> 1. As we know, after register to the thermal framework, it will have
>> duplicate hwmon devices. I think lm-sensor maintainer would not like this.
>
> Yes I noticed. You have always the option to disable the
> CONFIG_THERMAL_HWMON in your build.
Yes, we can disable this config, but it will disable all hwmon interface
which registered by the thermal fw, it may affect other driver.
>
>> How about to add a flag to indicate it, which I talk about with
>> Durgadoss in your [RFC 1/4]patch.
>
> This would be very much appreciated, but I don't think this is a blocker
> for this series. We can of course include this patch on it. Having a
> flag to control this thermal fw to hwmon interface is actually a good idea.
Ok, I think I can post it as a separate patches.
>
>>
>> 2. I'm also trying to use your codes on lm90. The LM90 serial has more
>> then one sensors in one chip, local sensor, remote sensor and may have
>> remote2 sensor, so it mean there may have more than one thermal_zone
>> under the lm90 device node, will you consider it?
>>
>
> I haven't looked lm90 source code. How do you map it? Do you probe each
> sensor or do you probe 1 device and expose 3 sensors?
lm90.c is the driver for all LM90 register compatible chips, it probe 1
device, and normally it can expose local and remote sensor, and for
max669x chip, it also expose remote2 sensor.
>
> In first case, you can still reuse what is in this series.
>
> Later case, we need to change it slightly.
>
> That is the case which is pretty similar on OMAP5 for instance, where
> the device has three sensors. This patch series does not cover for this
> case. But it can be simply modified to get around it.
>
> We would need to allow more than one 'thermal_zone' nodes inside a
> single device. But then we would need to have a way to determine which
> sensor goes to which zone too.
Yes, agreed, we need a way to determine it.
I have tentative suggestions, how about following:
lm90 {
...
thermal_zone@0 {
type = "local_sensor";
...
}
thermal_zone@1 {
type = "remote_sensor";
}
}
And in the driver's probe routine, we can parse the thermal_zone node,
and pass it to thermal_zone_of_device_register() with the right .get_temp().
Wei.
>
>
>> Thanks.
>> Wei.
>>
>>> + if (IS_ERR(data->tz)) {
>>> + status = PTR_ERR(data->tz);
>>> + goto exit_hwmon;
>>> + }
>>> + }
>>> +
>>> dev_info(&client->dev, "%s: sensor '%s'\n",
>>> dev_name(data->hwmon_dev), client->name);
>>>
>>> return 0;
>>>
>>> +exit_hwmon:
>>> + hwmon_device_unregister(data->hwmon_dev);
>>> exit_remove:
>>> sysfs_remove_group(&client->dev.kobj, &lm75_group);
>>> return status;
>>> @@ -285,6 +313,7 @@ static int lm75_remove(struct i2c_client *client)
>>> {
>>> struct lm75_data *data = i2c_get_clientdata(client);
>>>
>>> + thermal_zone_device_unregister(data->tz);
>>> hwmon_device_unregister(data->hwmon_dev);
>>> sysfs_remove_group(&client->dev.kobj, &lm75_group);
>>> lm75_write_value(client, LM75_REG_CONF, data->orig_conf);
>>>
>>
>>
>>
>
>
On 18-07-2013 17:21, Guenter Roeck wrote:
> On Thu, Jul 18, 2013 at 11:18:05AM -0600, Stephen Warren wrote:
>> On 07/18/2013 07:53 AM, Eduardo Valentin wrote:
>>> Hello Guenter,
>>>
>>> On 17-07-2013 18:09, Guenter Roeck wrote:
>>>> On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin
>>>> wrote:
>>>>> Hello all,
>>>>>
>>>>> As you noticed, I am working in a way to represent thermal
>>>>> data using device tree [1]. Essentially, this should be a way
>>>>> to say what to do with a sensor and how to associate (cooling)
>>>>> actions with it.
>>>>>
>>>> Seems to me that goes way beyond the supposed scope of devicetree
>>>> data. Devicetree data is supposed to describe hardware, not its
>>>> configuration or use. This is clearly a use case.
>>>
>>> Thanks for rising your voice here. It is important to know what
>>> hwmon ppl think about this.
>>
>> I meant to find time to read Guenter's original email where he
>> initially objected to putting data into DT, and determine exactly what
>> was being objected to. I still haven't:-( However, the arguments that
>> Eduardo stated in his email do make sense to me; I agree that
>> temperature limits really are a description of HW. Details of which
>> cooling methods to invoke when certain temperature limits are reached
>> is also part of the HW/system design, and hence I would tend to agree
>> that they're appropriate to include in DT. Anyway, that's just my 2
>> cents on the matter:-)
>
> Many systems have multiple profiles for various use cases (high performance,
> low power etc), and limits are different based on the use case. If that means
> you are going to have multiple devicetree variants based on the profile,
No, definitely this patch series is *not* about mapping multiples
profiles for various use cases on device tree! This series is about
mapping *hw thermal limits* on device tree.
> I would argue that you crossed the line. With thermal profiles it gets even more
> complicated, as those parameters may be played around with and changed
> multiple times to find the best settings to achieve optimal cooling.
> Does this describe hardware ? I don't think so, but, as I mentioned before,
> maybe I am wrong.
Again, this is about describing points and actions to avoid your hw
degradation. May be also useful to avoid end user harm.
This series is not about describing performance profiles.
>
> Guenter
>
>
--
You have got to be excited about what you are doing. (L. Lamport)
Eduardo Valentin
On 18-07-2013 17:11, Guenter Roeck wrote:
> On Thu, Jul 18, 2013 at 09:53:05AM -0400, Eduardo Valentin wrote:
>> Hello Guenter,
>>
>> On 17-07-2013 18:09, Guenter Roeck wrote:
>>> On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin wrote:
>>>> Hello all,
>>>>
>>>> As you noticed, I am working in a way to represent thermal data
>>>> using device tree [1]. Essentially, this should be a way to say
>>>> what to do with a sensor and how to associate (cooling) actions
>>>> with it.
>>>>
>>> Seems to me that goes way beyond the supposed scope of devicetree data.
>>> Devicetree data is supposed to describe hardware, not its configuration or use.
>>> This is clearly a use case.
>>
>> Thanks for rising your voice here. It is important to know what hwmon
>> ppl think about this.
>>
> Sorry, I don't know what ppl stands for.
>
>>>
>>> Guenter
>>
>> As your answers to the series are giving same argument, I chose to
>> answer on patch 0. I would be happier if you could elaborate a bit more
>> on your concern, specially if you take hwmon cap here, and give your
>> view with that perspective.
>>
>> I also considered that this work could be abusing of DT purposes. But
>> let me explain why I still think it makes sense to have it.
>>
> Ultimately, you are making my point here. If you considered it, did you ask
> devicetree experts for an opinion ? Did you discuss the subject on the
> devicetree-discuss mailing list ? If so, what was the result ?
Although I have asked, I didn't get any feedback.
https://lkml.org/lkml/2013/4/11/760
But now I am requesting feedback in a formal (patch) way.
Consider this patch series as official request for (devicetree experts
and everyone involved) opinions.
>
>> First thing is that this series intend to describe the thermal data
>> required for a specific board. That means, considering your board
>> layout, mechanics, power dissipation and composition of your ICs, etc,
>> that will impose thermal requirements on your system. That is not
>> configuration, but part of your board design and non-functional
>> requirement. To me, configuration would be something like saying you
>> want to use a specific keyboard layout, or defining your wifi card
>> channel, or display size, etc.
>>
>> Here what is described and setup may get confused with configuration,
>> but it is not because what goes in DT in this case must be actually
>> derived from your HW needs. Putting a sensor close to your battery has
>> a strong meaning behind. Same if you put a sensor close to your
>> processor. For instance, we have cases we need to consider external heat
>> in order to properly determine our CPU hotspot level, using a board
>> sensor. That is what I mean by HW requirement/need.
>>
>> Again, just to refresh our minds, this is about protecting your board
>> and ICs from operating out of their spec and extending their lifetime.
>> This is not about configuring something just because user has chosen to.
>> That is definitely not a configuration.
>>
>> Being a use case, well, these new DT nodes can still be seen as a use
>> case, yes. But depends on your understanding of use case. Because a
>> sensor device may be used on different needs, composing different use
>> cases. But still here we are talking about HW needs and composition. And
>> yes, if you take that perspective, there are use cases already described
>> in DT.
>>
>> For instance, just because you use an LDO to power a MMC, does it mean
>> you always will use it to power MMC on all boards. Would that be a use
>> case? And in that example, because your MMC requires 2.8V, if you have a
>> DT property to say that, does it mean it is configuration? Well, yes,
>> but that is based on HW needs, otherwise it wont operate properly. Same
>> thing here, leave your hw operating out of temperature specs and you
>> will see what is the outcome.
>>
>> Similar example would be cpufreq, or OPPs for opp layer. Defining an OPP
>> in DT could be considered a configuration? Well in theory yes, one can
>> pick what ever configuration for your (D)PLL then match with a
>> minimalist voltage level. And in theory, a voltage level can sustain
>> more than one frequency level. An OPP is still a virtual concept, and we
>> still describe it in DT. Why? To me is because it is a HW need, because
>> HW folks have actually validated that configuration of PLL (frequency)
>> and voltage level. Sometimes they have even optimized it (for low power
>> consumption for instance), as one may achieve same OPP with different
>> configuration. Then why thermal data, which is again, a 'HW
>> configuration' that has been optimized by HW folks, known to be a HW
>> requirement, cannot be described in DT?
>>
>> Similar argument goes to the fact that one may think this is subsystem
>> specific. Again, describing your OPPs is not OPP layer specific?
>> Besides, one can still read the device tree nodes I have written for
>> describing thermal data and implement the HW requirement elsewhere, if
>> wanted (say in user land). I don't see as subsystem specific.
>>
>> Keep in mind that these very same concepts are actually derived from
>> ACPI specification. They don't come from nowhere. And just because your
>> system does not have ACPI support, does not mean it won't have a need to
>> describe thermal?
>>
>> So, because with this work (i) we are describing something that is
>> required for properly usage of your HW (and not choice of the user),
>> because (ii) same data is used on different specification (that is used
>> on different OSes too), because (iii) you don't need thermal fw to read
>> this data and you could implement the HW requirement elsewhere, because
>> (iv) there are other similar requirements already implemented via DT; I
>> still think this work is within DT scope. And that is based on evidence
>> of existing work not because DT is nice and I would want to use it.
>>
>> Hope that clarifies.
>>
>> Of course it is always welcome to hear ppl opinion. I would be really
>> interested to know what ppl from OF think about this topic.
>>
> Yes, me too, or more widely the devicetree community. This is exactly
> why I brought it up.
And that is why I copied them.
>
>> If still, this does not fit DT, I would like to understand a proper
>> argument. Better than, this is configuration/use case.
>>
>
> I am not a devicetree expert. One of the complaints by the devicetree
> folks is that much is added to devicetree which should not be there.
> I find this use case questionable. Maybe I am wrong and it isn't,
ok..
> which may well be. But you thought about it as well, so I don't think
> I am that far off track.
Well, what I meant was more like, yes, I considered DT requirements
before writing this code.
>
> This needs to be discussed and determined by devicetree experts, not me.
> I'll be happy with your patch series if you get an agreement or an Ack
> by the devicetree maintainer for your patch series.
Ok. Thanks for your review and taking the time to express your judgment,
Guenter. Let s wait for Grant and dt folks to express their too.
>
> Guenter
>
>
--
You have got to be excited about what you are doing. (L. Lamport)
Eduardo Valentin
On 07/19/2013 07:38 AM, Eduardo Valentin wrote:
> On 18-07-2013 17:11, Guenter Roeck wrote:
>> On Thu, Jul 18, 2013 at 09:53:05AM -0400, Eduardo Valentin
>> wrote:
>>> Hello Guenter,
>>>
>>> On 17-07-2013 18:09, Guenter Roeck wrote:
>>>> On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin
>>>> wrote:
>>>>> Hello all,
>>>>>
>>>>> As you noticed, I am working in a way to represent thermal
>>>>> data using device tree [1]. Essentially, this should be a
>>>>> way to say what to do with a sensor and how to associate
>>>>> (cooling) actions with it.
>>>>>
>>>> Seems to me that goes way beyond the supposed scope of
>>>> devicetree data. Devicetree data is supposed to describe
>>>> hardware, not its configuration or use. This is clearly a use
>>>> case.
>>>
>>> Thanks for rising your voice here. It is important to know what
>>> hwmon ppl think about this.
>>>
>> Sorry, I don't know what ppl stands for.
>>
>>>>
>>>> Guenter
>>>
>>> As your answers to the series are giving same argument, I chose
>>> to answer on patch 0. I would be happier if you could elaborate
>>> a bit more on your concern, specially if you take hwmon cap
>>> here, and give your view with that perspective.
>>>
>>> I also considered that this work could be abusing of DT
>>> purposes. But let me explain why I still think it makes sense
>>> to have it.
>>>
>> Ultimately, you are making my point here. If you considered it,
>> did you ask devicetree experts for an opinion ? Did you discuss
>> the subject on the devicetree-discuss mailing list ? If so, what
>> was the result ?
>
> Although I have asked, I didn't get any feedback.
> https://lkml.org/lkml/2013/4/11/760
>
> But now I am requesting feedback in a formal (patch) way.
>
> Consider this patch series as official request for (devicetree
> experts and everyone involved) opinions.
I might suggest (a) sending the email "To" the DT maintainer, rather
than just CC'ing him, (b) perhaps start a new thread just to present
the proposed DT binding, and get feedback on that. A thread with a new
subject like "[RFC] DT binding for thermal zones" might get more
attention than a patch submission; the subject line of this patch
doesn't stand much (since it implies to me it's more about build
issues than DT bindings even though it does mention DT).
On 07/18/2013 03:21 PM, Guenter Roeck wrote:
> On Thu, Jul 18, 2013 at 11:18:05AM -0600, Stephen Warren wrote:
>> On 07/18/2013 07:53 AM, Eduardo Valentin wrote:
>>> Hello Guenter,
>>>
>>> On 17-07-2013 18:09, Guenter Roeck wrote:
>>>> On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin
>>>> wrote:
>>>>> Hello all,
>>>>>
>>>>> As you noticed, I am working in a way to represent thermal
>>>>> data using device tree [1]. Essentially, this should be a way
>>>>> to say what to do with a sensor and how to associate (cooling)
>>>>> actions with it.
>>>>>
>>>> Seems to me that goes way beyond the supposed scope of devicetree
>>>> data. Devicetree data is supposed to describe hardware, not its
>>>> configuration or use. This is clearly a use case.
>>>
>>> Thanks for rising your voice here. It is important to know what
>>> hwmon ppl think about this.
>>
>> I meant to find time to read Guenter's original email where he
>> initially objected to putting data into DT, and determine exactly what
>> was being objected to. I still haven't:-( However, the arguments that
>> Eduardo stated in his email do make sense to me; I agree that
>> temperature limits really are a description of HW. Details of which
>> cooling methods to invoke when certain temperature limits are reached
>> is also part of the HW/system design, and hence I would tend to agree
>> that they're appropriate to include in DT. Anyway, that's just my 2
>> cents on the matter:-)
>
> Many systems have multiple profiles for various use cases (high performance,
> low power etc), and limits are different based on the use case. If that means
> you are going to have multiple devicetree variants based on the profile,
> I would argue that you crossed the line.
Yes, I can see that argument. However, a counter-point:
* I believe we do need a DT binding to describe the absolute thermal
limits of a system, for safety/correctness of system operation.
* We need to define a syntax/schema to represent that.
* If we then want to implement additional profiles with stricter limits,
do we really want to invent a different syntax/schema to represent
those? Representing them in the same way seems like good use of the
design, mind-share, etc.
* Perhaps that doesn't mean that the additional profiles have to be in
DT though; just that we somehow make any other representation of those
profiles as close to the DT representation in syntax/structure as we
can, to get maximum re-use.
> With thermal profiles it gets even more
> complicated, as those parameters may be played around with and changed
> multiple times to find the best settings to achieve optimal cooling.
To me, that sounds more like fixing a bug in the initial data, rather
than something which fundamentally affects how the data should be
represented.
> Does this describe hardware ? I don't think so, but, as I mentioned before,
> maybe I am wrong.
On 19-07-2013 14:45, Stephen Warren wrote:
> On 07/19/2013 07:38 AM, Eduardo Valentin wrote:
>> On 18-07-2013 17:11, Guenter Roeck wrote:
>>> On Thu, Jul 18, 2013 at 09:53:05AM -0400, Eduardo Valentin
>>> wrote:
>>>> Hello Guenter,
>>>>
>>>> On 17-07-2013 18:09, Guenter Roeck wrote:
>>>>> On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin
>>>>> wrote:
>>>>>> Hello all,
>>>>>>
>>>>>> As you noticed, I am working in a way to represent thermal
>>>>>> data using device tree [1]. Essentially, this should be a
>>>>>> way to say what to do with a sensor and how to associate
>>>>>> (cooling) actions with it.
>>>>>>
>>>>> Seems to me that goes way beyond the supposed scope of
>>>>> devicetree data. Devicetree data is supposed to describe
>>>>> hardware, not its configuration or use. This is clearly a use
>>>>> case.
>>>>
>>>> Thanks for rising your voice here. It is important to know what
>>>> hwmon ppl think about this.
>>>>
>>> Sorry, I don't know what ppl stands for.
>>>
>>>>>
>>>>> Guenter
>>>>
>>>> As your answers to the series are giving same argument, I chose
>>>> to answer on patch 0. I would be happier if you could elaborate
>>>> a bit more on your concern, specially if you take hwmon cap
>>>> here, and give your view with that perspective.
>>>>
>>>> I also considered that this work could be abusing of DT
>>>> purposes. But let me explain why I still think it makes sense
>>>> to have it.
>>>>
>>> Ultimately, you are making my point here. If you considered it,
>>> did you ask devicetree experts for an opinion ? Did you discuss
>>> the subject on the devicetree-discuss mailing list ? If so, what
>>> was the result ?
>>
>> Although I have asked, I didn't get any feedback.
>> https://lkml.org/lkml/2013/4/11/760
>>
>> But now I am requesting feedback in a formal (patch) way.
>>
>> Consider this patch series as official request for (devicetree
>> experts and everyone involved) opinions.
>
> I might suggest (a) sending the email "To" the DT maintainer, rather
> than just CC'ing him, (b) perhaps start a new thread just to present
> the proposed DT binding, and get feedback on that. A thread with a new
> subject like "[RFC] DT binding for thermal zones" might get more
> attention than a patch submission; the subject line of this patch
> doesn't stand much (since it implies to me it's more about build
> issues than DT bindings even though it does mention DT).
>
OK. I will do that. Sounds reasonable. Resending this series as RFC
again, but now addressed to DT folks.
>
--
You have got to be excited about what you are doing. (L. Lamport)
Eduardo Valentin
On Fri, Jul 19, 2013 at 02:56:19PM -0400, Eduardo Valentin wrote:
> On 19-07-2013 14:45, Stephen Warren wrote:
> > On 07/19/2013 07:38 AM, Eduardo Valentin wrote:
> >> On 18-07-2013 17:11, Guenter Roeck wrote:
> >>> On Thu, Jul 18, 2013 at 09:53:05AM -0400, Eduardo Valentin
> >>> wrote:
> >>>> Hello Guenter,
> >>>>
> >>>> On 17-07-2013 18:09, Guenter Roeck wrote:
> >>>>> On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin
> >>>>> wrote:
> >>>>>> Hello all,
> >>>>>>
> >>>>>> As you noticed, I am working in a way to represent thermal
> >>>>>> data using device tree [1]. Essentially, this should be a
> >>>>>> way to say what to do with a sensor and how to associate
> >>>>>> (cooling) actions with it.
> >>>>>>
> >>>>> Seems to me that goes way beyond the supposed scope of
> >>>>> devicetree data. Devicetree data is supposed to describe
> >>>>> hardware, not its configuration or use. This is clearly a use
> >>>>> case.
> >>>>
> >>>> Thanks for rising your voice here. It is important to know what
> >>>> hwmon ppl think about this.
> >>>>
> >>> Sorry, I don't know what ppl stands for.
> >>>
> >>>>>
> >>>>> Guenter
> >>>>
> >>>> As your answers to the series are giving same argument, I chose
> >>>> to answer on patch 0. I would be happier if you could elaborate
> >>>> a bit more on your concern, specially if you take hwmon cap
> >>>> here, and give your view with that perspective.
> >>>>
> >>>> I also considered that this work could be abusing of DT
> >>>> purposes. But let me explain why I still think it makes sense
> >>>> to have it.
> >>>>
> >>> Ultimately, you are making my point here. If you considered it,
> >>> did you ask devicetree experts for an opinion ? Did you discuss
> >>> the subject on the devicetree-discuss mailing list ? If so, what
> >>> was the result ?
> >>
> >> Although I have asked, I didn't get any feedback.
> >> https://lkml.org/lkml/2013/4/11/760
> >>
> >> But now I am requesting feedback in a formal (patch) way.
> >>
> >> Consider this patch series as official request for (devicetree
> >> experts and everyone involved) opinions.
> >
> > I might suggest (a) sending the email "To" the DT maintainer, rather
> > than just CC'ing him, (b) perhaps start a new thread just to present
> > the proposed DT binding, and get feedback on that. A thread with a new
> > subject like "[RFC] DT binding for thermal zones" might get more
> > attention than a patch submission; the subject line of this patch
> > doesn't stand much (since it implies to me it's more about build
> > issues than DT bindings even though it does mention DT).
> >
>
> OK. I will do that. Sounds reasonable. Resending this series as RFC
> again, but now addressed to DT folks.
>
It might help to not just send the series, but to start a thread to discuss
the proposed bindings.
Guenter
On Fri, Jul 19, 2013 at 12:48:39PM -0600, Stephen Warren wrote:
> On 07/18/2013 03:21 PM, Guenter Roeck wrote:
> > On Thu, Jul 18, 2013 at 11:18:05AM -0600, Stephen Warren wrote:
> >> On 07/18/2013 07:53 AM, Eduardo Valentin wrote:
> >>> Hello Guenter,
> >>>
> >>> On 17-07-2013 18:09, Guenter Roeck wrote:
> >>>> On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin
> >>>> wrote:
> >>>>> Hello all,
> >>>>>
> >>>>> As you noticed, I am working in a way to represent thermal
> >>>>> data using device tree [1]. Essentially, this should be a way
> >>>>> to say what to do with a sensor and how to associate (cooling)
> >>>>> actions with it.
> >>>>>
> >>>> Seems to me that goes way beyond the supposed scope of devicetree
> >>>> data. Devicetree data is supposed to describe hardware, not its
> >>>> configuration or use. This is clearly a use case.
> >>>
> >>> Thanks for rising your voice here. It is important to know what
> >>> hwmon ppl think about this.
> >>
> >> I meant to find time to read Guenter's original email where he
> >> initially objected to putting data into DT, and determine exactly what
> >> was being objected to. I still haven't:-( However, the arguments that
> >> Eduardo stated in his email do make sense to me; I agree that
> >> temperature limits really are a description of HW. Details of which
> >> cooling methods to invoke when certain temperature limits are reached
> >> is also part of the HW/system design, and hence I would tend to agree
> >> that they're appropriate to include in DT. Anyway, that's just my 2
> >> cents on the matter:-)
> >
> > Many systems have multiple profiles for various use cases (high performance,
> > low power etc), and limits are different based on the use case. If that means
> > you are going to have multiple devicetree variants based on the profile,
> > I would argue that you crossed the line.
>
> Yes, I can see that argument. However, a counter-point:
>
> * I believe we do need a DT binding to describe the absolute thermal
> limits of a system, for safety/correctness of system operation.
>
> * We need to define a syntax/schema to represent that.
>
> * If we then want to implement additional profiles with stricter limits,
> do we really want to invent a different syntax/schema to represent
> those? Representing them in the same way seems like good use of the
> design, mind-share, etc.
>
> * Perhaps that doesn't mean that the additional profiles have to be in
> DT though; just that we somehow make any other representation of those
> profiles as close to the DT representation in syntax/structure as we
> can, to get maximum re-use.
>
> > With thermal profiles it gets even more
> > complicated, as those parameters may be played around with and changed
> > multiple times to find the best settings to achieve optimal cooling.
>
> To me, that sounds more like fixing a bug in the initial data, rather
> than something which fundamentally affects how the data should be
> represented.
>
For me, a bug in devicetree data would be if a wrong gpio pin is assigned ... if
a wrong clock frequency is specified, or a wrong clock input. I would not expect
a clock rate or pin assignment to be played around with to find the optimal
setting. Either it works or it doesn't.
Note that we don't really talk about pure thermal limits here. We are talking
about association between thermal sensors and cooling devices, which is much
more abstract and not as well defined as clock pin assignments. In many case,
more than one cooling device exists, and there is no well defined means to cool
the system. It may be cooled with a fan, or by reducing the CPU clock speed.
There may be multiple fans interacting with each other, and not all may be
present at all times. There may be secondary cooling options if primary cooling
fails (eg reduce CPU speed or increase speed of a secondary fan if a primary fan
fails).
Yes, I understand you need a means to describe thermal profiles. Putting one of
those into DT and declaring it to be the "absolute thermal limit", however, is
really just asking for putting other thermal profiles into DT as well ..
if you want a different thermal profile, just load a different devicetree, and
possibly make it configurable in BIOS/ROMMON. Of course you can declare that not
to be "intentional", but what do you think would happen in the real world ?
Do you really think anyone would bother putting one profile into DT and others
into some other database ? Not really, especially if the kernel would not even
support reading that other database. And if it would, you would not need the
DT data in the first place.
This is completely different to the intended use of devicetree information.
An instance of devicetree data should be tied to a specific piece of hardware.
Changing it may enable or disable pieces of that hardware, but change its
operation, as in "optimize for performance" or "optimize for low power
consumption" ? Yes, again, I understand you are saying that this is not the
_intent_, but that is what it enables.
Other but related subject .. from a thermal / hwmon driver's perspective, if
such a driver supports thermal subsystem, it should just register itself as
thermal sensor device, because that is what it is. If and how it is tied to
cooling devices should be part of the thermal subsystem and be decided there.
I don't think it is necessary for a thermal sensor to read devicetree data
in order to determine if it is supposed to register as thermal device or not.
That _is_ configuration and does not describe hardware - the mere fact that
a thermal sensor exists in the system already identifies it as thermal sensor
device, not anything in devicetree data. This would be similar to, say, a clock
device. A clock is a clock, and registers itself with the clock subsystem even
if there is no consumer. Sure, its interconnection with clock consumers would
be determined by devicetree data, but that is a different subject. So, even if
devicetree maintainers accept your definition of thermal profiles as hardware,
I think your approach is still wrong, and a thermal sensor device should
register itself as thermal device independently of its association with
cooling devices.
Thanks,
Guenter
On 07/21/2013 04:08 AM, Guenter Roeck wrote:
> ... [a bunch of good points re: why DT shouldn't describe thermal
> profiles]
Yes, lots of good arguments there.
So, where/how in your opinion should thermal profiles be defined, and
how should they get into the kernel? The nice thing about DT is that
it's a single place that describes the platform, with a well-defined
method of getting that information into the kernel. What alternatives exist?
> Other but related subject .. from a thermal / hwmon driver's perspective, if
> such a driver supports thermal subsystem, it should just register itself as
> thermal sensor device, because that is what it is. If and how it is tied to
> cooling devices should be part of the thermal subsystem and be decided there.
For audio, we have individual DT nodes that represent individual
audio-related components such as audio controllers, audio CODECs, etc.
We also have a "virtual" node that describes how those components
interact and create a complete sound card. Would it make sense to do
something similar with thermal sensors and cooling devices; represent
them all individually, have them register themselves with the
thermal/hwmon subsystem as you describe, but then have another "system
level" node that describes how the system designer intended them to
interact?
If you don't think so, how would the kernel represent and gain that
higher-level knowledge?
On Mon, Jul 22, 2013 at 12:43:43PM -0700, Stephen Warren wrote:
> On 07/21/2013 04:08 AM, Guenter Roeck wrote:
> > ... [a bunch of good points re: why DT shouldn't describe thermal
> > profiles]
>
> Yes, lots of good arguments there.
>
> So, where/how in your opinion should thermal profiles be defined, and
> how should they get into the kernel? The nice thing about DT is that
> it's a single place that describes the platform, with a well-defined
> method of getting that information into the kernel. What alternatives exist?
>
Excellent but quite different question. Quite frankly, I would love to be able
to use DT to describe a platform and its configuration and use instead of just
its hardware. Just like with thermal data, it would be great if I could use
devicetree information to describe limits for hwmon devices and configuration
information for all kinds of devices. This would simplify my life a lot.
Unfortunately, that is not how it works, so one ends up with pre-configuring
devices on ROMMON or secondary methods such as sensors.conf for hwmon devices.
Sure, that all works, but for embedded devices it means I have to re-build the
root file system each time a limit is changed. It _would_ be much simpler to
just be able to change the limit(s) in devicetree data and be done.
So, no, I don't have a good other solution. I have seen attempts to implement a
user-space 'parallel' devicetree, or the above mentioned sensors.conf file, but
nothing comprehensive. If you find a good generic solution, let me know.
Note that ACPI is an attempt to do that, but its scope is not limited to
describing the hardware, and it is not available on non-Intel embedded systems.
Also, it is typically controlled by another entity in a company (the BIOS
engineers), who tend to be highly resistive to changes. Modifying ACPI
data tends to be way more difficult than modifying devicetree data.
> > Other but related subject .. from a thermal / hwmon driver's perspective, if
> > such a driver supports thermal subsystem, it should just register itself as
> > thermal sensor device, because that is what it is. If and how it is tied to
> > cooling devices should be part of the thermal subsystem and be decided there.
>
> For audio, we have individual DT nodes that represent individual
> audio-related components such as audio controllers, audio CODECs, etc.
> We also have a "virtual" node that describes how those components
> interact and create a complete sound card. Would it make sense to do
> something similar with thermal sensors and cooling devices; represent
> them all individually, have them register themselves with the
> thermal/hwmon subsystem as you describe, but then have another "system
> level" node that describes how the system designer intended them to
> interact?
>
For device registration itself, definitely. An instance of a sensor driver
should not have to know how (or if) it is used by the system, just like a gpio
driver does not have to know if and how the gpio pins it provides are used.
Binding is the tricky question here, as it is all virtual and, as you said
yourself, describes the interaction and thus use of the various sensors and
cooling devices to accomplish the ultimate goal of keeping the system in an
acceptable temperature range. Hopefully you'll get an answer from the
devicetree experts if your intended devicetree usage of it is acceptable.
Guenter
On Wednesday, July 17, 2013 11:17:20 AM Eduardo Valentin wrote:
> This patch changes the cpufreq-cpu0 driver to consider if
> a cpu needs cooling (with cpufreq). In case the cooling is needed,
> it can be flagged at the cpu0 device tree node, with the boolean
> property 'needs-cooling'.
>
> In case this boolean is present, the driver will
> load a cpufreq cooling device in the system. The cpufreq-cpu0
> driver is not interested in determining how the system should
> be using the cooling device. The driver is responsible
> only of loading the cooling device.
>
> Describing how the cooling device will be used can be
> accomplished by setting up a thermal zone that references
> and is composed by the cpufreq cooling device.
>
> Cc: "Rafael J. Wysocki" <[email protected]>
FWIW, this change is fine by me, but I guess it should be handled along with
the entire series.
> Cc: Viresh Kumar <[email protected]>
> Cc: Grant Likely <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Eduardo Valentin <[email protected]>
> ---
> Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt | 3 +++
> drivers/cpufreq/cpufreq-cpu0.c | 8 ++++++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> index 051f764..e8ff916 100644
> --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> @@ -15,6 +15,8 @@ Optional properties:
> - clock-latency: Specify the possible maximum transition latency for clock,
> in unit of nanoseconds.
> - voltage-tolerance: Specify the CPU voltage tolerance in percentage.
> +- needs-cooling: The generic cpu cooling (freq clipping) is loaded by the
> +generic cpufreq-cpu0 driver in case the device tree node has this boolean.
>
> Examples:
>
> @@ -33,6 +35,7 @@ cpus {
> 198000 850000
> >;
> clock-latency = <61036>; /* two CLK32 periods */
> + needs-cooling;
> };
>
> cpu@1 {
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> index ad1fde2..4a8747a 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -20,6 +20,9 @@
> #include <linux/platform_device.h>
> #include <linux/regulator/consumer.h>
> #include <linux/slab.h>
> +#include <linux/thermal.h>
> +#include <linux/cpu_cooling.h>
> +#include <linux/cpumask.h>
>
> static unsigned int transition_latency;
> static unsigned int voltage_tolerance; /* in percentage */
> @@ -28,6 +31,7 @@ static struct device *cpu_dev;
> static struct clk *cpu_clk;
> static struct regulator *cpu_reg;
> static struct cpufreq_frequency_table *freq_table;
> +static struct thermal_cooling_device *cdev;
>
> static int cpu0_verify_speed(struct cpufreq_policy *policy)
> {
> @@ -268,6 +272,9 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
> goto out_free_table;
> }
>
> + if (of_property_read_bool(np, "needs-cooling"))
> + cdev = cpufreq_cooling_register(cpu_present_mask);
> +
> of_node_put(np);
> of_node_put(parent);
> return 0;
> @@ -283,6 +290,7 @@ out_put_parent:
>
> static int cpu0_cpufreq_remove(struct platform_device *pdev)
> {
> + cpufreq_cooling_unregister(cdev);
> cpufreq_unregister_driver(&cpu0_cpufreq_driver);
> opp_free_cpufreq_table(cpu_dev, &freq_table);
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On 25-07-2013 19:28, Rafael J. Wysocki wrote:
> On Wednesday, July 17, 2013 11:17:20 AM Eduardo Valentin wrote:
>> This patch changes the cpufreq-cpu0 driver to consider if
>> a cpu needs cooling (with cpufreq). In case the cooling is needed,
>> it can be flagged at the cpu0 device tree node, with the boolean
>> property 'needs-cooling'.
>>
>> In case this boolean is present, the driver will
>> load a cpufreq cooling device in the system. The cpufreq-cpu0
>> driver is not interested in determining how the system should
>> be using the cooling device. The driver is responsible
>> only of loading the cooling device.
>>
>> Describing how the cooling device will be used can be
>> accomplished by setting up a thermal zone that references
>> and is composed by the cpufreq cooling device.
>>
>> Cc: "Rafael J. Wysocki" <[email protected]>
>
> FWIW, this change is fine by me, but I guess it should be handled along with
> the entire series.
Raphael, thanks for your review.
Yeah, this change must be handled along with the other changes in this
series. There is still ongoing discussion in DT mailing list with
respect to thermal bindings. Thus, for now, this patch is on hold.
>
>> Cc: Viresh Kumar <[email protected]>
>> Cc: Grant Likely <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Eduardo Valentin <[email protected]>
>> ---
>> Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt | 3 +++
>> drivers/cpufreq/cpufreq-cpu0.c | 8 ++++++++
>> 2 files changed, 11 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> index 051f764..e8ff916 100644
>> --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> @@ -15,6 +15,8 @@ Optional properties:
>> - clock-latency: Specify the possible maximum transition latency for clock,
>> in unit of nanoseconds.
>> - voltage-tolerance: Specify the CPU voltage tolerance in percentage.
>> +- needs-cooling: The generic cpu cooling (freq clipping) is loaded by the
>> +generic cpufreq-cpu0 driver in case the device tree node has this boolean.
>>
>> Examples:
>>
>> @@ -33,6 +35,7 @@ cpus {
>> 198000 850000
>> >;
>> clock-latency = <61036>; /* two CLK32 periods */
>> + needs-cooling;
>> };
>>
>> cpu@1 {
>> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
>> index ad1fde2..4a8747a 100644
>> --- a/drivers/cpufreq/cpufreq-cpu0.c
>> +++ b/drivers/cpufreq/cpufreq-cpu0.c
>> @@ -20,6 +20,9 @@
>> #include <linux/platform_device.h>
>> #include <linux/regulator/consumer.h>
>> #include <linux/slab.h>
>> +#include <linux/thermal.h>
>> +#include <linux/cpu_cooling.h>
>> +#include <linux/cpumask.h>
>>
>> static unsigned int transition_latency;
>> static unsigned int voltage_tolerance; /* in percentage */
>> @@ -28,6 +31,7 @@ static struct device *cpu_dev;
>> static struct clk *cpu_clk;
>> static struct regulator *cpu_reg;
>> static struct cpufreq_frequency_table *freq_table;
>> +static struct thermal_cooling_device *cdev;
>>
>> static int cpu0_verify_speed(struct cpufreq_policy *policy)
>> {
>> @@ -268,6 +272,9 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
>> goto out_free_table;
>> }
>>
>> + if (of_property_read_bool(np, "needs-cooling"))
>> + cdev = cpufreq_cooling_register(cpu_present_mask);
>> +
>> of_node_put(np);
>> of_node_put(parent);
>> return 0;
>> @@ -283,6 +290,7 @@ out_put_parent:
>>
>> static int cpu0_cpufreq_remove(struct platform_device *pdev)
>> {
>> + cpufreq_cooling_unregister(cdev);
>> cpufreq_unregister_driver(&cpu0_cpufreq_driver);
>> opp_free_cpufreq_table(cpu_dev, &freq_table);
>>
>>
--
You have got to be excited about what you are doing. (L. Lamport)
Eduardo Valentin