2013-08-23 22:03:27

by Eduardo Valentin

[permalink] [raw]
Subject: [PATCH 0/5] drivers: thermal: several fixes

Hello all,

This is simple patch set with fixes on core thermal framework.
These fixes are a set of changes that I find needed that I
found during the process of attempting to describe thermal
data using device tree.

These patches touch several drivers, as there is an API change.
I have only compiled tested on other drivers. I have done
a runtime test on OMAP and DRA devices though.

Please feel free to perform your own test and report issues
if you find some.

All best,

Eduardo Valentin (5):
thermal: hwmon: move hwmon support to single file
drivers: thermal: parent virtual hwmon with thermal zone
drivers: thermal: make usage of CONFIG_THERMAL_HWMON optional
thermal: thermal_core: allow binding with limits on bind_params
drivers: thermal: add check when unregistering cpu cooling

Documentation/thermal/sysfs-api.txt | 11 +-
drivers/acpi/thermal.c | 6 +-
drivers/platform/x86/acerhdf.c | 3 +-
drivers/platform/x86/intel_mid_thermal.c | 2 +-
drivers/power/power_supply_core.c | 2 +-
drivers/thermal/Kconfig | 9 +
drivers/thermal/Makefile | 3 +
drivers/thermal/armada_thermal.c | 2 +-
drivers/thermal/cpu_cooling.c | 6 +-
drivers/thermal/db8500_thermal.c | 2 +-
drivers/thermal/dove_thermal.c | 2 +-
drivers/thermal/exynos_thermal.c | 2 +-
drivers/thermal/kirkwood_thermal.c | 2 +-
drivers/thermal/rcar_thermal.c | 2 +-
drivers/thermal/spear_thermal.c | 2 +-
drivers/thermal/thermal_core.c | 287 ++-------------------
drivers/thermal/thermal_hwmon.c | 269 +++++++++++++++++++
drivers/thermal/thermal_hwmon.h | 49 ++++
drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 2 +-
drivers/thermal/x86_pkg_temp_thermal.c | 2 +-
include/linux/thermal.h | 12 +-
21 files changed, 398 insertions(+), 279 deletions(-)
create mode 100644 drivers/thermal/thermal_hwmon.c
create mode 100644 drivers/thermal/thermal_hwmon.h

--
1.8.2.1.342.gfa7285d


2013-08-23 22:03:33

by Eduardo Valentin

[permalink] [raw]
Subject: [PATCH 1/5] 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]
Acked-by: Durgadoss R <[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..fdb0719
--- /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(*temp), GFP_KERNEL);
+ if (!temp) {
+ result = -ENOMEM;
+ goto unregister_name;
+ }
+
+ temp->tz = tz;
+ hwmon->count++;
+
+ snprintf(temp->temp_input.name, sizeof(temp->temp_input.name),
+ "temp%d_input", hwmon->count);
+ temp->temp_input.attr.attr.name = temp->temp_input.name;
+ temp->temp_input.attr.attr.mode = 0444;
+ temp->temp_input.attr.show = temp_input_show;
+ sysfs_attr_init(&temp->temp_input.attr.attr);
+ result = device_create_file(hwmon->device, &temp->temp_input.attr);
+ if (result)
+ goto free_temp_mem;
+
+ if (tz->ops->get_crit_temp) {
+ unsigned long temperature;
+ if (!tz->ops->get_crit_temp(tz, &temperature)) {
+ snprintf(temp->temp_crit.name,
+ sizeof(temp->temp_crit.name),
+ "temp%d_crit", hwmon->count);
+ temp->temp_crit.attr.attr.name = temp->temp_crit.name;
+ temp->temp_crit.attr.attr.mode = 0444;
+ temp->temp_crit.attr.show = temp_crit_show;
+ sysfs_attr_init(&temp->temp_crit.attr.attr);
+ result = device_create_file(hwmon->device,
+ &temp->temp_crit.attr);
+ if (result)
+ goto unregister_input;
+ }
+ }
+
+ mutex_lock(&thermal_hwmon_list_lock);
+ if (new_hwmon_device)
+ list_add_tail(&hwmon->node, &thermal_hwmon_list);
+ list_add_tail(&temp->hwmon_node, &hwmon->tz_list);
+ mutex_unlock(&thermal_hwmon_list_lock);
+
+ return 0;
+
+ unregister_input:
+ device_remove_file(hwmon->device, &temp->temp_input.attr);
+ free_temp_mem:
+ kfree(temp);
+ unregister_name:
+ if (new_hwmon_device) {
+ device_remove_file(hwmon->device, &dev_attr_name);
+ hwmon_device_unregister(hwmon->device);
+ }
+ free_mem:
+ if (new_hwmon_device)
+ kfree(hwmon);
+
+ return result;
+}
+
+void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+ struct thermal_hwmon_device *hwmon;
+ struct thermal_hwmon_temp *temp;
+
+ hwmon = thermal_hwmon_lookup_by_type(tz);
+ if (unlikely(!hwmon)) {
+ /* Should never happen... */
+ dev_dbg(&tz->device, "hwmon device lookup failed!\n");
+ return;
+ }
+
+ temp = thermal_hwmon_lookup_temp(hwmon, tz);
+ if (unlikely(!temp)) {
+ /* Should never happen... */
+ dev_dbg(&tz->device, "temperature input lookup failed!\n");
+ return;
+ }
+
+ device_remove_file(hwmon->device, &temp->temp_input.attr);
+ if (tz->ops->get_crit_temp)
+ device_remove_file(hwmon->device, &temp->temp_crit.attr);
+
+ mutex_lock(&thermal_hwmon_list_lock);
+ list_del(&temp->hwmon_node);
+ kfree(temp);
+ if (!list_empty(&hwmon->tz_list)) {
+ mutex_unlock(&thermal_hwmon_list_lock);
+ return;
+ }
+ list_del(&hwmon->node);
+ mutex_unlock(&thermal_hwmon_list_lock);
+
+ device_remove_file(hwmon->device, &dev_attr_name);
+ hwmon_device_unregister(hwmon->device);
+ kfree(hwmon);
+}
diff --git a/drivers/thermal/thermal_hwmon.h b/drivers/thermal/thermal_hwmon.h
new file mode 100644
index 0000000..c798fdb
--- /dev/null
+++ b/drivers/thermal/thermal_hwmon.h
@@ -0,0 +1,49 @@
+/*
+ * thermal_hwmon.h - Generic Thermal Management hwmon support.
+ *
+ * Code based on Intel thermal_core.c. Copyrights of the original code:
+ * Copyright (C) 2008 Intel Corp
+ * Copyright (C) 2008 Zhang Rui <[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

2013-08-23 22:03:38

by Eduardo Valentin

[permalink] [raw]
Subject: [PATCH 2/5] drivers: thermal: parent virtual hwmon with thermal zone

When creating virtual hwmon devices based out of thermal
zone devices, the virtual devices won't have parents.

This patch changes the code so that the parent of virtual
hwmon devices is the thermal zone device that they are
based of.

Cc: Zhang Rui <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Eduardo Valentin <[email protected]>
---
drivers/thermal/thermal_hwmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
index fdb0719..eeef0e2 100644
--- a/drivers/thermal/thermal_hwmon.c
+++ b/drivers/thermal/thermal_hwmon.c
@@ -159,7 +159,7 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)

INIT_LIST_HEAD(&hwmon->tz_list);
strlcpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH);
- hwmon->device = hwmon_device_register(NULL);
+ hwmon->device = hwmon_device_register(&tz->device);
if (IS_ERR(hwmon->device)) {
result = PTR_ERR(hwmon->device);
goto free_mem;
--
1.8.2.1.342.gfa7285d

2013-08-23 22:03:54

by Eduardo Valentin

[permalink] [raw]
Subject: [PATCH 4/5] 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]>
---
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 4b4a052..86e75a6 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -136,6 +136,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 d949ab0..e9545a7 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 88148b9..7c2769c 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

2013-08-23 22:03:59

by Eduardo Valentin

[permalink] [raw]
Subject: [PATCH 5/5] drivers: thermal: add check when unregistering cpu cooling

This patch avoids NULL pointer accesses while unregistering
cpu cooling devices, in case a NULL pointer is received.

Cc: Zhang Rui <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Eduardo Valentin <[email protected]>
---
drivers/thermal/cpu_cooling.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 82e15db..429a98c 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -496,8 +496,12 @@ EXPORT_SYMBOL_GPL(cpufreq_cooling_register);
*/
void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
{
- struct cpufreq_cooling_device *cpufreq_dev = cdev->devdata;
+ struct cpufreq_cooling_device *cpufreq_dev;

+ if (!cdev)
+ return;
+
+ cpufreq_dev = cdev->devdata;
mutex_lock(&cooling_cpufreq_lock);
cpufreq_dev_count--;

--
1.8.2.1.342.gfa7285d

2013-08-23 22:04:58

by Eduardo Valentin

[permalink] [raw]
Subject: [PATCH 3/5] drivers: thermal: make usage of CONFIG_THERMAL_HWMON optional

When registering a new thermal_device, the thermal framework
will always add a hwmon sysfs interface.

This patch adds a flag to make this behavior optional. Now
when registering a new thermal device, the caller needs
to say if the hwmon interface is required.

In order to keep same behavior as of today, all current
calls will by default create the hwmon interface.

Cc: Anton Vorontsov <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: [email protected]
Cc: Grant Likely <[email protected]>
Cc: Kukjin Kim <[email protected]>
Cc: Len Brown <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Matthew Garrett <[email protected]>
Cc: Peter Feuerer <[email protected]>
Cc: [email protected]
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Zhang Rui <[email protected]>
Suggested-by: Wei Ni <[email protected]>
Signed-off-by: Eduardo Valentin <[email protected]>
---
Documentation/thermal/sysfs-api.txt | 4 +++-
drivers/acpi/thermal.c | 6 ++++--
drivers/platform/x86/acerhdf.c | 3 ++-
drivers/platform/x86/intel_mid_thermal.c | 2 +-
drivers/power/power_supply_core.c | 2 +-
drivers/thermal/armada_thermal.c | 2 +-
drivers/thermal/db8500_thermal.c | 2 +-
drivers/thermal/dove_thermal.c | 2 +-
drivers/thermal/exynos_thermal.c | 2 +-
drivers/thermal/kirkwood_thermal.c | 2 +-
drivers/thermal/rcar_thermal.c | 2 +-
drivers/thermal/spear_thermal.c | 2 +-
drivers/thermal/thermal_core.c | 13 +++++++++----
drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 2 +-
drivers/thermal/x86_pkg_temp_thermal.c | 2 +-
include/linux/thermal.h | 2 +-
16 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index a71bd5b..4b4a052 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -64,7 +64,9 @@ temperature) and throttle appropriate devices.
performing passive cooling.
polling_delay: number of milliseconds to wait between polls when checking
whether trip points have been crossed (0 for interrupt driven systems).
-
+ add_hwmon: a boolean to indicate if the thermal to hwmon sysfs interface
+ is required. When add_hwmon == true, a hwmon sysfs interface
+ will be created. When add_hwmon == false, nothing will be done

1.1.2 void thermal_zone_device_unregister(struct thermal_zone_device *tz)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index a33821c..4d542b4 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -916,12 +916,14 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
thermal_zone_device_register("acpitz", trips, 0, tz,
&acpi_thermal_zone_ops, NULL,
tz->trips.passive.tsp*100,
- tz->polling_frequency*100);
+ tz->polling_frequency*100,
+ true);
else
tz->thermal_zone =
thermal_zone_device_register("acpitz", trips, 0, tz,
&acpi_thermal_zone_ops, NULL,
- 0, tz->polling_frequency*100);
+ 0, tz->polling_frequency*100,
+ true);
if (IS_ERR(tz->thermal_zone))
return -ENODEV;

diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index f94467c..004d9ba0 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -663,7 +663,8 @@ static int acerhdf_register_thermal(void)

thz_dev = thermal_zone_device_register("acerhdf", 1, 0, NULL,
&acerhdf_dev_ops, NULL, 0,
- (kernelmode) ? interval*1000 : 0);
+ (kernelmode) ? interval*1000 : 0,
+ true);
if (IS_ERR(thz_dev))
return -EINVAL;

diff --git a/drivers/platform/x86/intel_mid_thermal.c b/drivers/platform/x86/intel_mid_thermal.c
index 81c491e..efea0bf 100644
--- a/drivers/platform/x86/intel_mid_thermal.c
+++ b/drivers/platform/x86/intel_mid_thermal.c
@@ -502,7 +502,7 @@ static int mid_thermal_probe(struct platform_device *pdev)
goto err;
}
pinfo->tzd[i] = thermal_zone_device_register(name[i],
- 0, 0, td_info, &tzd_ops, NULL, 0, 0);
+ 0, 0, td_info, &tzd_ops, NULL, 0, 0, true);
if (IS_ERR(pinfo->tzd[i])) {
kfree(td_info);
ret = PTR_ERR(pinfo->tzd[i]);
diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 3b2d5df..5f39914 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -359,7 +359,7 @@ static int psy_register_thermal(struct power_supply *psy)
for (i = 0; i < psy->num_properties; i++) {
if (psy->properties[i] == POWER_SUPPLY_PROP_TEMP) {
psy->tzd = thermal_zone_device_register(psy->name, 0, 0,
- psy, &psy_tzd_ops, NULL, 0, 0);
+ psy, &psy_tzd_ops, NULL, 0, 0, true);
if (IS_ERR(psy->tzd))
return PTR_ERR(psy->tzd);
break;
diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 5e53212..bc54c80 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -182,7 +182,7 @@ static int armada_thermal_probe(struct platform_device *pdev)
priv->ops->init_sensor(priv);

thermal = thermal_zone_device_register("armada_thermal", 0, 0,
- priv, &ops, NULL, 0, 0);
+ priv, &ops, NULL, 0, 0, true);
if (IS_ERR(thermal)) {
dev_err(&pdev->dev,
"Failed to register thermal zone device\n");
diff --git a/drivers/thermal/db8500_thermal.c b/drivers/thermal/db8500_thermal.c
index 1e3b3bf..439a854 100644
--- a/drivers/thermal/db8500_thermal.c
+++ b/drivers/thermal/db8500_thermal.c
@@ -447,7 +447,7 @@ static int db8500_thermal_probe(struct platform_device *pdev)
}

pzone->therm_dev = thermal_zone_device_register("db8500_thermal_zone",
- ptrips->num_trips, 0, pzone, &thdev_ops, NULL, 0, 0);
+ ptrips->num_trips, 0, pzone, &thdev_ops, NULL, 0, 0, true);

if (IS_ERR(pzone->therm_dev)) {
dev_err(&pdev->dev, "Register thermal zone device failed.\n");
diff --git a/drivers/thermal/dove_thermal.c b/drivers/thermal/dove_thermal.c
index 828f5e3..61f2247 100644
--- a/drivers/thermal/dove_thermal.c
+++ b/drivers/thermal/dove_thermal.c
@@ -155,7 +155,7 @@ static int dove_thermal_probe(struct platform_device *pdev)
}

thermal = thermal_zone_device_register("dove_thermal", 0, 0,
- priv, &ops, NULL, 0, 0);
+ priv, &ops, NULL, 0, 0, true);
if (IS_ERR(thermal)) {
dev_err(&pdev->dev,
"Failed to register thermal zone device\n");
diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c
index 9af4b93..6ffadd3 100644
--- a/drivers/thermal/exynos_thermal.c
+++ b/drivers/thermal/exynos_thermal.c
@@ -469,7 +469,7 @@ static int exynos_register_thermal(struct thermal_sensor_conf *sensor_conf)
th_zone->therm_dev = thermal_zone_device_register(sensor_conf->name,
EXYNOS_ZONE_COUNT, 0, NULL, &exynos_dev_ops, NULL, 0,
sensor_conf->trip_data.trigger_falling ?
- 0 : IDLE_INTERVAL);
+ 0 : IDLE_INTERVAL, true);

if (IS_ERR(th_zone->therm_dev)) {
pr_err("Failed to register thermal zone device\n");
diff --git a/drivers/thermal/kirkwood_thermal.c b/drivers/thermal/kirkwood_thermal.c
index 3b034a0..fb8853e 100644
--- a/drivers/thermal/kirkwood_thermal.c
+++ b/drivers/thermal/kirkwood_thermal.c
@@ -85,7 +85,7 @@ static int kirkwood_thermal_probe(struct platform_device *pdev)
return PTR_ERR(priv->sensor);

thermal = thermal_zone_device_register("kirkwood_thermal", 0, 0,
- priv, &ops, NULL, 0, 0);
+ priv, &ops, NULL, 0, 0, true);
if (IS_ERR(thermal)) {
dev_err(&pdev->dev,
"Failed to register thermal zone device\n");
diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index 88f92e1..64ae530 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -439,7 +439,7 @@ static int rcar_thermal_probe(struct platform_device *pdev)
priv->zone = thermal_zone_device_register("rcar_thermal",
1, 0, priv,
&rcar_thermal_zone_ops, NULL, 0,
- idle);
+ idle, true);
if (IS_ERR(priv->zone)) {
dev_err(dev, "can't register thermal zone\n");
ret = PTR_ERR(priv->zone);
diff --git a/drivers/thermal/spear_thermal.c b/drivers/thermal/spear_thermal.c
index ab79ea4..ec0c44d 100644
--- a/drivers/thermal/spear_thermal.c
+++ b/drivers/thermal/spear_thermal.c
@@ -140,7 +140,7 @@ static int spear_thermal_probe(struct platform_device *pdev)
writel_relaxed(stdev->flags, stdev->thermal_base);

spear_thermal = thermal_zone_device_register("spear_thermal", 0, 0,
- stdev, &ops, NULL, 0, 0);
+ stdev, &ops, NULL, 0, 0, true);
if (IS_ERR(spear_thermal)) {
dev_err(&pdev->dev, "thermal zone device is NULL\n");
ret = PTR_ERR(spear_thermal);
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 247528b..d949ab0 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1344,6 +1344,9 @@ static void remove_trip_attrs(struct thermal_zone_device *tz)
* @polling_delay: number of milliseconds to wait between polls when checking
* whether trip points have been crossed (0 for interrupt
* driven systems)
+ * @add_hwmon: a boolean to indicate if the thermal to hwmon sysfs interface
+ * is required. When add_hwmon == true, a hwmon sysfs interface
+ * will be created. When add_hwmon == false, nothing will be done
*
* This interface function adds a new thermal zone device (sensor) to
* /sys/class/thermal folder as thermal_zone[0-*]. It tries to bind all the
@@ -1359,7 +1362,7 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
int trips, int mask, void *devdata,
const struct thermal_zone_device_ops *ops,
const struct thermal_zone_params *tzp,
- int passive_delay, int polling_delay)
+ int passive_delay, int polling_delay, bool add_hwmon)
{
struct thermal_zone_device *tz;
enum thermal_trip_type trip_type;
@@ -1462,9 +1465,11 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,

mutex_unlock(&thermal_governor_lock);

- result = thermal_add_hwmon_sysfs(tz);
- if (result)
- goto unregister;
+ if (add_hwmon) {
+ result = thermal_add_hwmon_sysfs(tz);
+ if (result)
+ goto unregister;
+ }

mutex_lock(&thermal_list_lock);
list_add_tail(&tz->node, &thermal_tz_list);
diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index 4c5f55c37..5ab613a 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -306,7 +306,7 @@ int ti_thermal_expose_sensor(struct ti_bandgap *bgp, int id,
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);
+ FAST_TEMP_MONITORING_RATE, true);
if (IS_ERR(data->ti_thermal)) {
dev_err(bgp->dev, "thermal zone device is NULL\n");
return PTR_ERR(data->ti_thermal);
diff --git a/drivers/thermal/x86_pkg_temp_thermal.c b/drivers/thermal/x86_pkg_temp_thermal.c
index f36950e..74eb4c0 100644
--- a/drivers/thermal/x86_pkg_temp_thermal.c
+++ b/drivers/thermal/x86_pkg_temp_thermal.c
@@ -444,7 +444,7 @@ static int pkg_temp_thermal_device_add(unsigned int cpu)
thres_count,
(thres_count == MAX_NUMBER_OF_TRIPS) ?
0x03 : 0x01,
- phy_dev_entry, &tzone_ops, NULL, 0, 0);
+ phy_dev_entry, &tzone_ops, NULL, 0, 0, true);
if (IS_ERR(phy_dev_entry->tzone)) {
err = PTR_ERR(phy_dev_entry->tzone);
goto err_ret_free;
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index a386a1c..88148b9 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -226,7 +226,7 @@ struct thermal_genl_event {
/* Function declarations */
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);
+ const struct thermal_zone_params *, int, int, bool);
void thermal_zone_device_unregister(struct thermal_zone_device *);

int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
--
1.8.2.1.342.gfa7285d

2013-08-23 22:57:47

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 3/5] drivers: thermal: make usage of CONFIG_THERMAL_HWMON optional

On Friday, August 23, 2013 06:03:14 PM Eduardo Valentin wrote:
> When registering a new thermal_device, the thermal framework
> will always add a hwmon sysfs interface.
>
> This patch adds a flag to make this behavior optional. Now
> when registering a new thermal device, the caller needs
> to say if the hwmon interface is required.
>
> In order to keep same behavior as of today, all current
> calls will by default create the hwmon interface.

Well, instead of modifying all of the callers this way, why don't
you add new versions taking the additional argument as, for example,

thermal_zone_device_register_full()

and redefine the old ones as static inline wrappers, for example

static inline struct thermal_zone_device *thermal_zone_device_register(args)
{
return thermal_zone_device_register_full(args, true);
}

?

That'd reduce the size of this patch a bit I suppose (and the number of
subsystems involved at the same time).

Thanks,
Rafael


> Cc: Anton Vorontsov <[email protected]>
> Cc: David Woodhouse <[email protected]>
> Cc: [email protected]
> Cc: Grant Likely <[email protected]>
> Cc: Kukjin Kim <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: Matthew Garrett <[email protected]>
> Cc: Peter Feuerer <[email protected]>
> Cc: [email protected]
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Zhang Rui <[email protected]>
> Suggested-by: Wei Ni <[email protected]>
> Signed-off-by: Eduardo Valentin <[email protected]>
> ---
> Documentation/thermal/sysfs-api.txt | 4 +++-
> drivers/acpi/thermal.c | 6 ++++--
> drivers/platform/x86/acerhdf.c | 3 ++-
> drivers/platform/x86/intel_mid_thermal.c | 2 +-
> drivers/power/power_supply_core.c | 2 +-
> drivers/thermal/armada_thermal.c | 2 +-
> drivers/thermal/db8500_thermal.c | 2 +-
> drivers/thermal/dove_thermal.c | 2 +-
> drivers/thermal/exynos_thermal.c | 2 +-
> drivers/thermal/kirkwood_thermal.c | 2 +-
> drivers/thermal/rcar_thermal.c | 2 +-
> drivers/thermal/spear_thermal.c | 2 +-
> drivers/thermal/thermal_core.c | 13 +++++++++----
> drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 2 +-
> drivers/thermal/x86_pkg_temp_thermal.c | 2 +-
> include/linux/thermal.h | 2 +-
> 16 files changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index a71bd5b..4b4a052 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -64,7 +64,9 @@ temperature) and throttle appropriate devices.
> performing passive cooling.
> polling_delay: number of milliseconds to wait between polls when checking
> whether trip points have been crossed (0 for interrupt driven systems).
> -
> + add_hwmon: a boolean to indicate if the thermal to hwmon sysfs interface
> + is required. When add_hwmon == true, a hwmon sysfs interface
> + will be created. When add_hwmon == false, nothing will be done
>
> 1.1.2 void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index a33821c..4d542b4 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -916,12 +916,14 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
> thermal_zone_device_register("acpitz", trips, 0, tz,
> &acpi_thermal_zone_ops, NULL,
> tz->trips.passive.tsp*100,
> - tz->polling_frequency*100);
> + tz->polling_frequency*100,
> + true);
> else
> tz->thermal_zone =
> thermal_zone_device_register("acpitz", trips, 0, tz,
> &acpi_thermal_zone_ops, NULL,
> - 0, tz->polling_frequency*100);
> + 0, tz->polling_frequency*100,
> + true);
> if (IS_ERR(tz->thermal_zone))
> return -ENODEV;
>
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index f94467c..004d9ba0 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -663,7 +663,8 @@ static int acerhdf_register_thermal(void)
>
> thz_dev = thermal_zone_device_register("acerhdf", 1, 0, NULL,
> &acerhdf_dev_ops, NULL, 0,
> - (kernelmode) ? interval*1000 : 0);
> + (kernelmode) ? interval*1000 : 0,
> + true);
> if (IS_ERR(thz_dev))
> return -EINVAL;
>
> diff --git a/drivers/platform/x86/intel_mid_thermal.c b/drivers/platform/x86/intel_mid_thermal.c
> index 81c491e..efea0bf 100644
> --- a/drivers/platform/x86/intel_mid_thermal.c
> +++ b/drivers/platform/x86/intel_mid_thermal.c
> @@ -502,7 +502,7 @@ static int mid_thermal_probe(struct platform_device *pdev)
> goto err;
> }
> pinfo->tzd[i] = thermal_zone_device_register(name[i],
> - 0, 0, td_info, &tzd_ops, NULL, 0, 0);
> + 0, 0, td_info, &tzd_ops, NULL, 0, 0, true);
> if (IS_ERR(pinfo->tzd[i])) {
> kfree(td_info);
> ret = PTR_ERR(pinfo->tzd[i]);
> diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
> index 3b2d5df..5f39914 100644
> --- a/drivers/power/power_supply_core.c
> +++ b/drivers/power/power_supply_core.c
> @@ -359,7 +359,7 @@ static int psy_register_thermal(struct power_supply *psy)
> for (i = 0; i < psy->num_properties; i++) {
> if (psy->properties[i] == POWER_SUPPLY_PROP_TEMP) {
> psy->tzd = thermal_zone_device_register(psy->name, 0, 0,
> - psy, &psy_tzd_ops, NULL, 0, 0);
> + psy, &psy_tzd_ops, NULL, 0, 0, true);
> if (IS_ERR(psy->tzd))
> return PTR_ERR(psy->tzd);
> break;
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 5e53212..bc54c80 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -182,7 +182,7 @@ static int armada_thermal_probe(struct platform_device *pdev)
> priv->ops->init_sensor(priv);
>
> thermal = thermal_zone_device_register("armada_thermal", 0, 0,
> - priv, &ops, NULL, 0, 0);
> + priv, &ops, NULL, 0, 0, true);
> if (IS_ERR(thermal)) {
> dev_err(&pdev->dev,
> "Failed to register thermal zone device\n");
> diff --git a/drivers/thermal/db8500_thermal.c b/drivers/thermal/db8500_thermal.c
> index 1e3b3bf..439a854 100644
> --- a/drivers/thermal/db8500_thermal.c
> +++ b/drivers/thermal/db8500_thermal.c
> @@ -447,7 +447,7 @@ static int db8500_thermal_probe(struct platform_device *pdev)
> }
>
> pzone->therm_dev = thermal_zone_device_register("db8500_thermal_zone",
> - ptrips->num_trips, 0, pzone, &thdev_ops, NULL, 0, 0);
> + ptrips->num_trips, 0, pzone, &thdev_ops, NULL, 0, 0, true);
>
> if (IS_ERR(pzone->therm_dev)) {
> dev_err(&pdev->dev, "Register thermal zone device failed.\n");
> diff --git a/drivers/thermal/dove_thermal.c b/drivers/thermal/dove_thermal.c
> index 828f5e3..61f2247 100644
> --- a/drivers/thermal/dove_thermal.c
> +++ b/drivers/thermal/dove_thermal.c
> @@ -155,7 +155,7 @@ static int dove_thermal_probe(struct platform_device *pdev)
> }
>
> thermal = thermal_zone_device_register("dove_thermal", 0, 0,
> - priv, &ops, NULL, 0, 0);
> + priv, &ops, NULL, 0, 0, true);
> if (IS_ERR(thermal)) {
> dev_err(&pdev->dev,
> "Failed to register thermal zone device\n");
> diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c
> index 9af4b93..6ffadd3 100644
> --- a/drivers/thermal/exynos_thermal.c
> +++ b/drivers/thermal/exynos_thermal.c
> @@ -469,7 +469,7 @@ static int exynos_register_thermal(struct thermal_sensor_conf *sensor_conf)
> th_zone->therm_dev = thermal_zone_device_register(sensor_conf->name,
> EXYNOS_ZONE_COUNT, 0, NULL, &exynos_dev_ops, NULL, 0,
> sensor_conf->trip_data.trigger_falling ?
> - 0 : IDLE_INTERVAL);
> + 0 : IDLE_INTERVAL, true);
>
> if (IS_ERR(th_zone->therm_dev)) {
> pr_err("Failed to register thermal zone device\n");
> diff --git a/drivers/thermal/kirkwood_thermal.c b/drivers/thermal/kirkwood_thermal.c
> index 3b034a0..fb8853e 100644
> --- a/drivers/thermal/kirkwood_thermal.c
> +++ b/drivers/thermal/kirkwood_thermal.c
> @@ -85,7 +85,7 @@ static int kirkwood_thermal_probe(struct platform_device *pdev)
> return PTR_ERR(priv->sensor);
>
> thermal = thermal_zone_device_register("kirkwood_thermal", 0, 0,
> - priv, &ops, NULL, 0, 0);
> + priv, &ops, NULL, 0, 0, true);
> if (IS_ERR(thermal)) {
> dev_err(&pdev->dev,
> "Failed to register thermal zone device\n");
> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index 88f92e1..64ae530 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -439,7 +439,7 @@ static int rcar_thermal_probe(struct platform_device *pdev)
> priv->zone = thermal_zone_device_register("rcar_thermal",
> 1, 0, priv,
> &rcar_thermal_zone_ops, NULL, 0,
> - idle);
> + idle, true);
> if (IS_ERR(priv->zone)) {
> dev_err(dev, "can't register thermal zone\n");
> ret = PTR_ERR(priv->zone);
> diff --git a/drivers/thermal/spear_thermal.c b/drivers/thermal/spear_thermal.c
> index ab79ea4..ec0c44d 100644
> --- a/drivers/thermal/spear_thermal.c
> +++ b/drivers/thermal/spear_thermal.c
> @@ -140,7 +140,7 @@ static int spear_thermal_probe(struct platform_device *pdev)
> writel_relaxed(stdev->flags, stdev->thermal_base);
>
> spear_thermal = thermal_zone_device_register("spear_thermal", 0, 0,
> - stdev, &ops, NULL, 0, 0);
> + stdev, &ops, NULL, 0, 0, true);
> if (IS_ERR(spear_thermal)) {
> dev_err(&pdev->dev, "thermal zone device is NULL\n");
> ret = PTR_ERR(spear_thermal);
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 247528b..d949ab0 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1344,6 +1344,9 @@ static void remove_trip_attrs(struct thermal_zone_device *tz)
> * @polling_delay: number of milliseconds to wait between polls when checking
> * whether trip points have been crossed (0 for interrupt
> * driven systems)
> + * @add_hwmon: a boolean to indicate if the thermal to hwmon sysfs interface
> + * is required. When add_hwmon == true, a hwmon sysfs interface
> + * will be created. When add_hwmon == false, nothing will be done
> *
> * This interface function adds a new thermal zone device (sensor) to
> * /sys/class/thermal folder as thermal_zone[0-*]. It tries to bind all the
> @@ -1359,7 +1362,7 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
> int trips, int mask, void *devdata,
> const struct thermal_zone_device_ops *ops,
> const struct thermal_zone_params *tzp,
> - int passive_delay, int polling_delay)
> + int passive_delay, int polling_delay, bool add_hwmon)
> {
> struct thermal_zone_device *tz;
> enum thermal_trip_type trip_type;
> @@ -1462,9 +1465,11 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
>
> mutex_unlock(&thermal_governor_lock);
>
> - result = thermal_add_hwmon_sysfs(tz);
> - if (result)
> - goto unregister;
> + if (add_hwmon) {
> + result = thermal_add_hwmon_sysfs(tz);
> + if (result)
> + goto unregister;
> + }
>
> mutex_lock(&thermal_list_lock);
> list_add_tail(&tz->node, &thermal_tz_list);
> diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
> index 4c5f55c37..5ab613a 100644
> --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
> +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
> @@ -306,7 +306,7 @@ int ti_thermal_expose_sensor(struct ti_bandgap *bgp, int id,
> 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);
> + FAST_TEMP_MONITORING_RATE, true);
> if (IS_ERR(data->ti_thermal)) {
> dev_err(bgp->dev, "thermal zone device is NULL\n");
> return PTR_ERR(data->ti_thermal);
> diff --git a/drivers/thermal/x86_pkg_temp_thermal.c b/drivers/thermal/x86_pkg_temp_thermal.c
> index f36950e..74eb4c0 100644
> --- a/drivers/thermal/x86_pkg_temp_thermal.c
> +++ b/drivers/thermal/x86_pkg_temp_thermal.c
> @@ -444,7 +444,7 @@ static int pkg_temp_thermal_device_add(unsigned int cpu)
> thres_count,
> (thres_count == MAX_NUMBER_OF_TRIPS) ?
> 0x03 : 0x01,
> - phy_dev_entry, &tzone_ops, NULL, 0, 0);
> + phy_dev_entry, &tzone_ops, NULL, 0, 0, true);
> if (IS_ERR(phy_dev_entry->tzone)) {
> err = PTR_ERR(phy_dev_entry->tzone);
> goto err_ret_free;
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index a386a1c..88148b9 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -226,7 +226,7 @@ struct thermal_genl_event {
> /* Function declarations */
> 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);
> + const struct thermal_zone_params *, int, int, bool);
> void thermal_zone_device_unregister(struct thermal_zone_device *);
>
> int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2013-08-27 18:28:15

by Eduardo Valentin

[permalink] [raw]
Subject: Re: [PATCH 3/5] drivers: thermal: make usage of CONFIG_THERMAL_HWMON optional

On 23-08-2013 19:08, Rafael J. Wysocki wrote:
> On Friday, August 23, 2013 06:03:14 PM Eduardo Valentin wrote:
>> When registering a new thermal_device, the thermal framework
>> will always add a hwmon sysfs interface.
>>
>> This patch adds a flag to make this behavior optional. Now
>> when registering a new thermal device, the caller needs
>> to say if the hwmon interface is required.
>>
>> In order to keep same behavior as of today, all current
>> calls will by default create the hwmon interface.
>
> Well, instead of modifying all of the callers this way, why don't
> you add new versions taking the additional argument as, for example,
>
> thermal_zone_device_register_full()
>
> and redefine the old ones as static inline wrappers, for example
>
> static inline struct thermal_zone_device *thermal_zone_device_register(args)
> {
> return thermal_zone_device_register_full(args, true);
> }
>
> ?

Yeah, that is another way to go and I thought of doing it like that. I
just could not come out with a good API naming:

thermal_zone_device_register_full(all args)

thermal_zone_device_register(args) /* on hwmon == true */
thermal_zone_device_register_no_hwmon(args) /* on hwmon == false */

Would this sound reasonable naming?


>
> That'd reduce the size of this patch a bit I suppose (and the number of
> subsystems involved at the same time).

I see your point.

>
> Thanks,
> Rafael
>
>
>> Cc: Anton Vorontsov <[email protected]>
>> Cc: David Woodhouse <[email protected]>
>> Cc: [email protected]
>> Cc: Grant Likely <[email protected]>
>> Cc: Kukjin Kim <[email protected]>
>> Cc: Len Brown <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: Matthew Garrett <[email protected]>
>> Cc: Peter Feuerer <[email protected]>
>> Cc: [email protected]
>> Cc: "Rafael J. Wysocki" <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: Zhang Rui <[email protected]>
>> Suggested-by: Wei Ni <[email protected]>
>> Signed-off-by: Eduardo Valentin <[email protected]>
>> ---
>> Documentation/thermal/sysfs-api.txt | 4 +++-
>> drivers/acpi/thermal.c | 6 ++++--
>> drivers/platform/x86/acerhdf.c | 3 ++-
>> drivers/platform/x86/intel_mid_thermal.c | 2 +-
>> drivers/power/power_supply_core.c | 2 +-
>> drivers/thermal/armada_thermal.c | 2 +-
>> drivers/thermal/db8500_thermal.c | 2 +-
>> drivers/thermal/dove_thermal.c | 2 +-
>> drivers/thermal/exynos_thermal.c | 2 +-
>> drivers/thermal/kirkwood_thermal.c | 2 +-
>> drivers/thermal/rcar_thermal.c | 2 +-
>> drivers/thermal/spear_thermal.c | 2 +-
>> drivers/thermal/thermal_core.c | 13 +++++++++----
>> drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 2 +-
>> drivers/thermal/x86_pkg_temp_thermal.c | 2 +-
>> include/linux/thermal.h | 2 +-
>> 16 files changed, 30 insertions(+), 20 deletions(-)
>>
>> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
>> index a71bd5b..4b4a052 100644
>> --- a/Documentation/thermal/sysfs-api.txt
>> +++ b/Documentation/thermal/sysfs-api.txt
>> @@ -64,7 +64,9 @@ temperature) and throttle appropriate devices.
>> performing passive cooling.
>> polling_delay: number of milliseconds to wait between polls when checking
>> whether trip points have been crossed (0 for interrupt driven systems).
>> -
>> + add_hwmon: a boolean to indicate if the thermal to hwmon sysfs interface
>> + is required. When add_hwmon == true, a hwmon sysfs interface
>> + will be created. When add_hwmon == false, nothing will be done
>>
>> 1.1.2 void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>>
>> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
>> index a33821c..4d542b4 100644
>> --- a/drivers/acpi/thermal.c
>> +++ b/drivers/acpi/thermal.c
>> @@ -916,12 +916,14 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
>> thermal_zone_device_register("acpitz", trips, 0, tz,
>> &acpi_thermal_zone_ops, NULL,
>> tz->trips.passive.tsp*100,
>> - tz->polling_frequency*100);
>> + tz->polling_frequency*100,
>> + true);
>> else
>> tz->thermal_zone =
>> thermal_zone_device_register("acpitz", trips, 0, tz,
>> &acpi_thermal_zone_ops, NULL,
>> - 0, tz->polling_frequency*100);
>> + 0, tz->polling_frequency*100,
>> + true);
>> if (IS_ERR(tz->thermal_zone))
>> return -ENODEV;
>>
>> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
>> index f94467c..004d9ba0 100644
>> --- a/drivers/platform/x86/acerhdf.c
>> +++ b/drivers/platform/x86/acerhdf.c
>> @@ -663,7 +663,8 @@ static int acerhdf_register_thermal(void)
>>
>> thz_dev = thermal_zone_device_register("acerhdf", 1, 0, NULL,
>> &acerhdf_dev_ops, NULL, 0,
>> - (kernelmode) ? interval*1000 : 0);
>> + (kernelmode) ? interval*1000 : 0,
>> + true);
>> if (IS_ERR(thz_dev))
>> return -EINVAL;
>>
>> diff --git a/drivers/platform/x86/intel_mid_thermal.c b/drivers/platform/x86/intel_mid_thermal.c
>> index 81c491e..efea0bf 100644
>> --- a/drivers/platform/x86/intel_mid_thermal.c
>> +++ b/drivers/platform/x86/intel_mid_thermal.c
>> @@ -502,7 +502,7 @@ static int mid_thermal_probe(struct platform_device *pdev)
>> goto err;
>> }
>> pinfo->tzd[i] = thermal_zone_device_register(name[i],
>> - 0, 0, td_info, &tzd_ops, NULL, 0, 0);
>> + 0, 0, td_info, &tzd_ops, NULL, 0, 0, true);
>> if (IS_ERR(pinfo->tzd[i])) {
>> kfree(td_info);
>> ret = PTR_ERR(pinfo->tzd[i]);
>> diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
>> index 3b2d5df..5f39914 100644
>> --- a/drivers/power/power_supply_core.c
>> +++ b/drivers/power/power_supply_core.c
>> @@ -359,7 +359,7 @@ static int psy_register_thermal(struct power_supply *psy)
>> for (i = 0; i < psy->num_properties; i++) {
>> if (psy->properties[i] == POWER_SUPPLY_PROP_TEMP) {
>> psy->tzd = thermal_zone_device_register(psy->name, 0, 0,
>> - psy, &psy_tzd_ops, NULL, 0, 0);
>> + psy, &psy_tzd_ops, NULL, 0, 0, true);
>> if (IS_ERR(psy->tzd))
>> return PTR_ERR(psy->tzd);
>> break;
>> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
>> index 5e53212..bc54c80 100644
>> --- a/drivers/thermal/armada_thermal.c
>> +++ b/drivers/thermal/armada_thermal.c
>> @@ -182,7 +182,7 @@ static int armada_thermal_probe(struct platform_device *pdev)
>> priv->ops->init_sensor(priv);
>>
>> thermal = thermal_zone_device_register("armada_thermal", 0, 0,
>> - priv, &ops, NULL, 0, 0);
>> + priv, &ops, NULL, 0, 0, true);
>> if (IS_ERR(thermal)) {
>> dev_err(&pdev->dev,
>> "Failed to register thermal zone device\n");
>> diff --git a/drivers/thermal/db8500_thermal.c b/drivers/thermal/db8500_thermal.c
>> index 1e3b3bf..439a854 100644
>> --- a/drivers/thermal/db8500_thermal.c
>> +++ b/drivers/thermal/db8500_thermal.c
>> @@ -447,7 +447,7 @@ static int db8500_thermal_probe(struct platform_device *pdev)
>> }
>>
>> pzone->therm_dev = thermal_zone_device_register("db8500_thermal_zone",
>> - ptrips->num_trips, 0, pzone, &thdev_ops, NULL, 0, 0);
>> + ptrips->num_trips, 0, pzone, &thdev_ops, NULL, 0, 0, true);
>>
>> if (IS_ERR(pzone->therm_dev)) {
>> dev_err(&pdev->dev, "Register thermal zone device failed.\n");
>> diff --git a/drivers/thermal/dove_thermal.c b/drivers/thermal/dove_thermal.c
>> index 828f5e3..61f2247 100644
>> --- a/drivers/thermal/dove_thermal.c
>> +++ b/drivers/thermal/dove_thermal.c
>> @@ -155,7 +155,7 @@ static int dove_thermal_probe(struct platform_device *pdev)
>> }
>>
>> thermal = thermal_zone_device_register("dove_thermal", 0, 0,
>> - priv, &ops, NULL, 0, 0);
>> + priv, &ops, NULL, 0, 0, true);
>> if (IS_ERR(thermal)) {
>> dev_err(&pdev->dev,
>> "Failed to register thermal zone device\n");
>> diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c
>> index 9af4b93..6ffadd3 100644
>> --- a/drivers/thermal/exynos_thermal.c
>> +++ b/drivers/thermal/exynos_thermal.c
>> @@ -469,7 +469,7 @@ static int exynos_register_thermal(struct thermal_sensor_conf *sensor_conf)
>> th_zone->therm_dev = thermal_zone_device_register(sensor_conf->name,
>> EXYNOS_ZONE_COUNT, 0, NULL, &exynos_dev_ops, NULL, 0,
>> sensor_conf->trip_data.trigger_falling ?
>> - 0 : IDLE_INTERVAL);
>> + 0 : IDLE_INTERVAL, true);
>>
>> if (IS_ERR(th_zone->therm_dev)) {
>> pr_err("Failed to register thermal zone device\n");
>> diff --git a/drivers/thermal/kirkwood_thermal.c b/drivers/thermal/kirkwood_thermal.c
>> index 3b034a0..fb8853e 100644
>> --- a/drivers/thermal/kirkwood_thermal.c
>> +++ b/drivers/thermal/kirkwood_thermal.c
>> @@ -85,7 +85,7 @@ static int kirkwood_thermal_probe(struct platform_device *pdev)
>> return PTR_ERR(priv->sensor);
>>
>> thermal = thermal_zone_device_register("kirkwood_thermal", 0, 0,
>> - priv, &ops, NULL, 0, 0);
>> + priv, &ops, NULL, 0, 0, true);
>> if (IS_ERR(thermal)) {
>> dev_err(&pdev->dev,
>> "Failed to register thermal zone device\n");
>> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
>> index 88f92e1..64ae530 100644
>> --- a/drivers/thermal/rcar_thermal.c
>> +++ b/drivers/thermal/rcar_thermal.c
>> @@ -439,7 +439,7 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>> priv->zone = thermal_zone_device_register("rcar_thermal",
>> 1, 0, priv,
>> &rcar_thermal_zone_ops, NULL, 0,
>> - idle);
>> + idle, true);
>> if (IS_ERR(priv->zone)) {
>> dev_err(dev, "can't register thermal zone\n");
>> ret = PTR_ERR(priv->zone);
>> diff --git a/drivers/thermal/spear_thermal.c b/drivers/thermal/spear_thermal.c
>> index ab79ea4..ec0c44d 100644
>> --- a/drivers/thermal/spear_thermal.c
>> +++ b/drivers/thermal/spear_thermal.c
>> @@ -140,7 +140,7 @@ static int spear_thermal_probe(struct platform_device *pdev)
>> writel_relaxed(stdev->flags, stdev->thermal_base);
>>
>> spear_thermal = thermal_zone_device_register("spear_thermal", 0, 0,
>> - stdev, &ops, NULL, 0, 0);
>> + stdev, &ops, NULL, 0, 0, true);
>> if (IS_ERR(spear_thermal)) {
>> dev_err(&pdev->dev, "thermal zone device is NULL\n");
>> ret = PTR_ERR(spear_thermal);
>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>> index 247528b..d949ab0 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -1344,6 +1344,9 @@ static void remove_trip_attrs(struct thermal_zone_device *tz)
>> * @polling_delay: number of milliseconds to wait between polls when checking
>> * whether trip points have been crossed (0 for interrupt
>> * driven systems)
>> + * @add_hwmon: a boolean to indicate if the thermal to hwmon sysfs interface
>> + * is required. When add_hwmon == true, a hwmon sysfs interface
>> + * will be created. When add_hwmon == false, nothing will be done
>> *
>> * This interface function adds a new thermal zone device (sensor) to
>> * /sys/class/thermal folder as thermal_zone[0-*]. It tries to bind all the
>> @@ -1359,7 +1362,7 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
>> int trips, int mask, void *devdata,
>> const struct thermal_zone_device_ops *ops,
>> const struct thermal_zone_params *tzp,
>> - int passive_delay, int polling_delay)
>> + int passive_delay, int polling_delay, bool add_hwmon)
>> {
>> struct thermal_zone_device *tz;
>> enum thermal_trip_type trip_type;
>> @@ -1462,9 +1465,11 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
>>
>> mutex_unlock(&thermal_governor_lock);
>>
>> - result = thermal_add_hwmon_sysfs(tz);
>> - if (result)
>> - goto unregister;
>> + if (add_hwmon) {
>> + result = thermal_add_hwmon_sysfs(tz);
>> + if (result)
>> + goto unregister;
>> + }
>>
>> mutex_lock(&thermal_list_lock);
>> list_add_tail(&tz->node, &thermal_tz_list);
>> diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>> index 4c5f55c37..5ab613a 100644
>> --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>> +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>> @@ -306,7 +306,7 @@ int ti_thermal_expose_sensor(struct ti_bandgap *bgp, int id,
>> 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);
>> + FAST_TEMP_MONITORING_RATE, true);
>> if (IS_ERR(data->ti_thermal)) {
>> dev_err(bgp->dev, "thermal zone device is NULL\n");
>> return PTR_ERR(data->ti_thermal);
>> diff --git a/drivers/thermal/x86_pkg_temp_thermal.c b/drivers/thermal/x86_pkg_temp_thermal.c
>> index f36950e..74eb4c0 100644
>> --- a/drivers/thermal/x86_pkg_temp_thermal.c
>> +++ b/drivers/thermal/x86_pkg_temp_thermal.c
>> @@ -444,7 +444,7 @@ static int pkg_temp_thermal_device_add(unsigned int cpu)
>> thres_count,
>> (thres_count == MAX_NUMBER_OF_TRIPS) ?
>> 0x03 : 0x01,
>> - phy_dev_entry, &tzone_ops, NULL, 0, 0);
>> + phy_dev_entry, &tzone_ops, NULL, 0, 0, true);
>> if (IS_ERR(phy_dev_entry->tzone)) {
>> err = PTR_ERR(phy_dev_entry->tzone);
>> goto err_ret_free;
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index a386a1c..88148b9 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -226,7 +226,7 @@ struct thermal_genl_event {
>> /* Function declarations */
>> 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);
>> + const struct thermal_zone_params *, int, int, bool);
>> void thermal_zone_device_unregister(struct thermal_zone_device *);
>>
>> int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
>>


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

Eduardo Valentin


Attachments:
signature.asc (295.00 B)
OpenPGP digital signature

2013-08-27 21:07:03

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 3/5] drivers: thermal: make usage of CONFIG_THERMAL_HWMON optional

On Tuesday, August 27, 2013 02:26:41 PM Eduardo Valentin wrote:
> On 23-08-2013 19:08, Rafael J. Wysocki wrote:
> > On Friday, August 23, 2013 06:03:14 PM Eduardo Valentin wrote:
> >> When registering a new thermal_device, the thermal framework
> >> will always add a hwmon sysfs interface.
> >>
> >> This patch adds a flag to make this behavior optional. Now
> >> when registering a new thermal device, the caller needs
> >> to say if the hwmon interface is required.
> >>
> >> In order to keep same behavior as of today, all current
> >> calls will by default create the hwmon interface.
> >
> > Well, instead of modifying all of the callers this way, why don't
> > you add new versions taking the additional argument as, for example,
> >
> > thermal_zone_device_register_full()
> >
> > and redefine the old ones as static inline wrappers, for example
> >
> > static inline struct thermal_zone_device *thermal_zone_device_register(args)
> > {
> > return thermal_zone_device_register_full(args, true);
> > }
> >
> > ?
>
> Yeah, that is another way to go and I thought of doing it like that. I
> just could not come out with a good API naming:
>
> thermal_zone_device_register_full(all args)
>
> thermal_zone_device_register(args) /* on hwmon == true */
> thermal_zone_device_register_no_hwmon(args) /* on hwmon == false */
>
> Would this sound reasonable naming?

Yeah, sounds good to me.

Thanks,
Rafael

2013-08-28 00:12:41

by Zhang, Rui

[permalink] [raw]
Subject: Re: [PATCH 3/5] drivers: thermal: make usage of CONFIG_THERMAL_HWMON optional

On 二, 2013-08-27 at 23:17 +0200, Rafael J. Wysocki wrote:
> On Tuesday, August 27, 2013 02:26:41 PM Eduardo Valentin wrote:
> > On 23-08-2013 19:08, Rafael J. Wysocki wrote:
> > > On Friday, August 23, 2013 06:03:14 PM Eduardo Valentin wrote:
> > >> When registering a new thermal_device, the thermal framework
> > >> will always add a hwmon sysfs interface.
> > >>
> > >> This patch adds a flag to make this behavior optional. Now
> > >> when registering a new thermal device, the caller needs
> > >> to say if the hwmon interface is required.
> > >>
> > >> In order to keep same behavior as of today, all current
> > >> calls will by default create the hwmon interface.
> > >
> > > Well, instead of modifying all of the callers this way, why don't
> > > you add new versions taking the additional argument as, for example,
> > >
> > > thermal_zone_device_register_full()
> > >
> > > and redefine the old ones as static inline wrappers, for example
> > >
> > > static inline struct thermal_zone_device *thermal_zone_device_register(args)
> > > {
> > > return thermal_zone_device_register_full(args, true);
> > > }
> > >
> > > ?
> >
> > Yeah, that is another way to go and I thought of doing it like that. I
> > just could not come out with a good API naming:
> >
> > thermal_zone_device_register_full(all args)
> >
> > thermal_zone_device_register(args) /* on hwmon == true */
> > thermal_zone_device_register_no_hwmon(args) /* on hwmon == false */
> >
> > Would this sound reasonable naming?
>
> Yeah, sounds good to me.
>
Well, actually, Tianyu is working on a patch set to move all the
thermal_zone_device_register() parameters to struct thermal_zone_params.

But with that patch set done, what you need to do next is just to
introduce a new field in struct thermal_zone_params and make sure 0
means registering hwmon sysfs I/F.

thanks,
rui

> Thanks,
> Rafael
>

2013-08-28 13:17:08

by Eduardo Valentin

[permalink] [raw]
Subject: Re: [PATCH 3/5] drivers: thermal: make usage of CONFIG_THERMAL_HWMON optional

On 27-08-2013 20:13, Zhang Rui wrote:
> On 二, 2013-08-27 at 23:17 +0200, Rafael J. Wysocki wrote:
>> On Tuesday, August 27, 2013 02:26:41 PM Eduardo Valentin wrote:
>>> On 23-08-2013 19:08, Rafael J. Wysocki wrote:
>>>> On Friday, August 23, 2013 06:03:14 PM Eduardo Valentin wrote:
>>>>> When registering a new thermal_device, the thermal framework
>>>>> will always add a hwmon sysfs interface.
>>>>>
>>>>> This patch adds a flag to make this behavior optional. Now
>>>>> when registering a new thermal device, the caller needs
>>>>> to say if the hwmon interface is required.
>>>>>
>>>>> In order to keep same behavior as of today, all current
>>>>> calls will by default create the hwmon interface.
>>>>
>>>> Well, instead of modifying all of the callers this way, why don't
>>>> you add new versions taking the additional argument as, for example,
>>>>
>>>> thermal_zone_device_register_full()
>>>>
>>>> and redefine the old ones as static inline wrappers, for example
>>>>
>>>> static inline struct thermal_zone_device *thermal_zone_device_register(args)
>>>> {
>>>> return thermal_zone_device_register_full(args, true);
>>>> }
>>>>
>>>> ?
>>>
>>> Yeah, that is another way to go and I thought of doing it like that. I
>>> just could not come out with a good API naming:
>>>
>>> thermal_zone_device_register_full(all args)
>>>
>>> thermal_zone_device_register(args) /* on hwmon == true */
>>> thermal_zone_device_register_no_hwmon(args) /* on hwmon == false */
>>>
>>> Would this sound reasonable naming?
>>
>> Yeah, sounds good to me.
>>
> Well, actually, Tianyu is working on a patch set to move all the
> thermal_zone_device_register() parameters to struct thermal_zone_params.
>
> But with that patch set done, what you need to do next is just to
> introduce a new field in struct thermal_zone_params and make sure 0
> means registering hwmon sysfs I/F.

Rui,

Do you have a pointer to his patch?

>
> thanks,
> rui
>
>> Thanks,
>> Rafael
>>
>
>
>
>


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

Eduardo Valentin


Attachments:
signature.asc (295.00 B)
OpenPGP digital signature

2013-08-29 20:13:22

by Eduardo Valentin

[permalink] [raw]
Subject: [PATCHv2 3/5] drivers: thermal: make usage of CONFIG_THERMAL_HWMON optional

When registering a new thermal_device, the thermal framework
will always add a hwmon sysfs interface.

This patch adds a flag to make this behavior optional. Now
when registering a new thermal device, the caller can
optionally inform if hwmon interface is desirable. This can
be done by means of passing a thermal_zone_params.no_hwmon == true.

In order to keep same behavior as of today, all current
calls will by default create the hwmon interface.

Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Zhang Rui <[email protected]>
Suggested-by: Wei Ni <[email protected]>
Signed-off-by: Eduardo Valentin <[email protected]>
---
Documentation/thermal/sysfs-api.txt | 5 +++++
drivers/thermal/thermal_core.c | 8 +++++---
include/linux/thermal.h | 8 ++++++++
3 files changed, 18 insertions(+), 3 deletions(-)
---

Hello all,

So, here is then a different way of implementing same feature.
As requested by Rui, now the flag goes via thermal_zone_params,
because this seams to be the preferred way, at least in near future.

Using this method the amount of subsystems involved is reduced to 1:
thermal, as requested by Raphael. I just made is a negative flag
in order to keep the same behavior of the existing drivers, without
changing them. The flag is documented though.

All best,


diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index a71bd5b..37c5486 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -142,6 +142,11 @@ temperature) and throttle appropriate devices.
This is an optional feature where some platforms can choose not to
provide this data.
.governor_name: Name of the thermal governor used for this zone
+ .no_hwmon: a boolean to indicate if the thermal to hwmon sysfs interface
+ is required. when no_hwmon == false, a hwmon sysfs interface
+ will be created. when no_hwmon == true, nothing will be done.
+ In case the thermal_zone_params is NULL, the hwmon interface
+ will be created (for backward compatibility).
.num_tbps: Number of thermal_bind_params entries for this zone
.tbp: thermal_bind_params entries

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 247528b..51648bf 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1462,9 +1462,11 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,

mutex_unlock(&thermal_governor_lock);

- result = thermal_add_hwmon_sysfs(tz);
- if (result)
- goto unregister;
+ if (!tz->tzp || !tz->tzp->no_hwmon) {
+ result = thermal_add_hwmon_sysfs(tz);
+ if (result)
+ goto unregister;
+ }

mutex_lock(&thermal_list_lock);
list_add_tail(&tz->node, &thermal_tz_list);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index a386a1c..b4a975f 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -214,6 +214,14 @@ struct thermal_bind_params {
/* Structure to define Thermal Zone parameters */
struct thermal_zone_params {
char governor_name[THERMAL_NAME_LENGTH];
+
+ /*
+ * a boolean to indicate if the thermal to hwmon sysfs interface
+ * is required. when no_hwmon == false, a hwmon sysfs interface
+ * will be created. when no_hwmon == true, nothing will be done
+ */
+ bool no_hwmon;
+
int num_tbps; /* Number of tbp entries */
struct thermal_bind_params *tbp;
};
--
1.8.2.1.342.gfa7285d

2013-09-02 14:09:50

by Eduardo Valentin

[permalink] [raw]
Subject: Re: [PATCHv2 3/5] drivers: thermal: make usage of CONFIG_THERMAL_HWMON optional

Hi,

On 29-08-2013 16:12, Eduardo Valentin wrote:
> When registering a new thermal_device, the thermal framework
> will always add a hwmon sysfs interface.
>
> This patch adds a flag to make this behavior optional. Now
> when registering a new thermal device, the caller can
> optionally inform if hwmon interface is desirable. This can
> be done by means of passing a thermal_zone_params.no_hwmon == true.
>
> In order to keep same behavior as of today, all current
> calls will by default create the hwmon interface.
>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: Zhang Rui <[email protected]>
> Suggested-by: Wei Ni <[email protected]>
> Signed-off-by: Eduardo Valentin <[email protected]>
> ---
> Documentation/thermal/sysfs-api.txt | 5 +++++
> drivers/thermal/thermal_core.c | 8 +++++---
> include/linux/thermal.h | 8 ++++++++
> 3 files changed, 18 insertions(+), 3 deletions(-)
> ---
>
> Hello all,
>
> So, here is then a different way of implementing same feature.
> As requested by Rui, now the flag goes via thermal_zone_params,
> because this seams to be the preferred way, at least in near future.
>
> Using this method the amount of subsystems involved is reduced to 1:
> thermal, as requested by Raphael. I just made is a negative flag
> in order to keep the same behavior of the existing drivers, without
> changing them. The flag is documented though.
>

Objections on this way of adding this flag?

Rui,

If you are OK, I can add this series to my tree too.


> All best,
>
>
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index a71bd5b..37c5486 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -142,6 +142,11 @@ temperature) and throttle appropriate devices.
> This is an optional feature where some platforms can choose not to
> provide this data.
> .governor_name: Name of the thermal governor used for this zone
> + .no_hwmon: a boolean to indicate if the thermal to hwmon sysfs interface
> + is required. when no_hwmon == false, a hwmon sysfs interface
> + will be created. when no_hwmon == true, nothing will be done.
> + In case the thermal_zone_params is NULL, the hwmon interface
> + will be created (for backward compatibility).
> .num_tbps: Number of thermal_bind_params entries for this zone
> .tbp: thermal_bind_params entries
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 247528b..51648bf 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1462,9 +1462,11 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
>
> mutex_unlock(&thermal_governor_lock);
>
> - result = thermal_add_hwmon_sysfs(tz);
> - if (result)
> - goto unregister;
> + if (!tz->tzp || !tz->tzp->no_hwmon) {
> + result = thermal_add_hwmon_sysfs(tz);
> + if (result)
> + goto unregister;
> + }
>
> mutex_lock(&thermal_list_lock);
> list_add_tail(&tz->node, &thermal_tz_list);
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index a386a1c..b4a975f 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -214,6 +214,14 @@ struct thermal_bind_params {
> /* Structure to define Thermal Zone parameters */
> struct thermal_zone_params {
> char governor_name[THERMAL_NAME_LENGTH];
> +
> + /*
> + * a boolean to indicate if the thermal to hwmon sysfs interface
> + * is required. when no_hwmon == false, a hwmon sysfs interface
> + * will be created. when no_hwmon == true, nothing will be done
> + */
> + bool no_hwmon;
> +
> int num_tbps; /* Number of tbp entries */
> struct thermal_bind_params *tbp;
> };
>




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

Eduardo Valentin


Attachments:
signature.asc (295.00 B)
OpenPGP digital signature