* [PATCHv2 00/14] Thermal Framework Enhancements
@ 2012-08-27 4:28 Durgadoss R
2012-08-27 4:28 ` [PATCHv2 01/14] Thermal: Refactor thermal.h file Durgadoss R
` (13 more replies)
0 siblings, 14 replies; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch series is a v2 of the series submitted here:
http://www.spinics.net/lists/linux-acpi/msg37375.html
These patches are based on Rui's tree here: (branch - thermal)
git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git
Changes since v1:
* Created thermal_core.h
* Removed get_cdev_by_name API, as it is no longer needed
* Introduced thermal_bind_params inside thermal_zone_params
* Use a single arbitrator by making thermal_cdev_update global
* Added more documentation on EXPORT_SYMBOL APIs in thermal_sys.c
* Various minor fixes based on comments from Rui and Eduardo.
The function pointer based implementation for various throttling
policies is not done here. I tried and it altered the structure of
the patch set a lot, and hence I shall take it up after this code
gets in.
Durgadoss R (14):
Thermal: Refactor thermal.h file
Thermal: Move thermal_instance to thermal_core.h
Thermal: Add get trend, get instance API's to thermal_sys
Thermal: Add platform level information to thermal.h
Thermal: Obtain platform data for thermal zone
Thermal: Add a policy sysfs attribute
Thermal: Update binding logic based on platform data
Thermal: Make thermal_cdev_update as a global function
Thermal: Introduce fair_share thermal governor
Thermal: Introduce a step_wise thermal governor
Thermal: Remove throttling logic out of thermal_sys.c
Thermal: Add a notification API
Thermal: Add documentation for platform layer data
Thermal: Platform layer changes to provide thermal data
Documentation/thermal/sysfs-api.txt | 51 +++
arch/x86/platform/mrst/mrst.c | 49 +++
drivers/thermal/Kconfig | 12 +
drivers/thermal/Makefile | 4 +-
drivers/thermal/fair_share.c | 113 ++++++
drivers/thermal/step_wise.c | 173 ++++++++++
drivers/thermal/thermal_core.h | 53 +++
drivers/thermal/thermal_sys.c | 647 ++++++++++++++++++++++-------------
include/linux/thermal.h | 145 ++++++--
9 files changed, 969 insertions(+), 278 deletions(-)
create mode 100644 drivers/thermal/fair_share.c
create mode 100644 drivers/thermal/step_wise.c
create mode 100644 drivers/thermal/thermal_core.h
--
1.7.9.5
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv2 01/14] Thermal: Refactor thermal.h file
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
@ 2012-08-27 4:28 ` Durgadoss R
2012-08-27 4:28 ` [PATCHv2 02/14] Thermal: Move thermal_instance to thermal_core.h Durgadoss R
` (12 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch rearranges the code in thermal.h file,
in the following order, so that it is easy to
read/maintain.
1. All #defines
2. All enums
3. All fops structures
4. All device structures
5. All function declarations
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
include/linux/thermal.h | 78 ++++++++++++++++++++++++++---------------------
1 file changed, 43 insertions(+), 35 deletions(-)
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 91b3481..8611e3e 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -29,6 +29,23 @@
#include <linux/device.h>
#include <linux/workqueue.h>
+#define THERMAL_TRIPS_NONE -1
+#define THERMAL_MAX_TRIPS 12
+#define THERMAL_NAME_LENGTH 20
+
+/* No upper/lower limit requirement */
+#define THERMAL_NO_LIMIT -1UL
+
+/* Unit conversion macros */
+#define KELVIN_TO_CELSIUS(t) (long)(((long)t-2732 >= 0) ? \
+ ((long)t-2732+5)/10 : ((long)t-2732-5)/10)
+#define CELSIUS_TO_KELVIN(t) ((t)*10+2732)
+
+/* Adding event notification support elements */
+#define THERMAL_GENL_FAMILY_NAME "thermal_event"
+#define THERMAL_GENL_VERSION 0x01
+#define THERMAL_GENL_MCAST_GROUP_NAME "thermal_mc_group"
+
struct thermal_zone_device;
struct thermal_cooling_device;
@@ -50,6 +67,30 @@ enum thermal_trend {
THERMAL_TREND_DROPPING, /* temperature is dropping */
};
+/* Events supported by Thermal Netlink */
+enum events {
+ THERMAL_AUX0,
+ THERMAL_AUX1,
+ THERMAL_CRITICAL,
+ THERMAL_DEV_FAULT,
+};
+
+/* attributes of thermal_genl_family */
+enum {
+ THERMAL_GENL_ATTR_UNSPEC,
+ THERMAL_GENL_ATTR_EVENT,
+ __THERMAL_GENL_ATTR_MAX,
+};
+#define THERMAL_GENL_ATTR_MAX (__THERMAL_GENL_ATTR_MAX - 1)
+
+/* commands supported by the thermal_genl_family */
+enum {
+ THERMAL_GENL_CMD_UNSPEC,
+ THERMAL_GENL_CMD_EVENT,
+ __THERMAL_GENL_CMD_MAX,
+};
+#define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)
+
struct thermal_zone_device_ops {
int (*bind) (struct thermal_zone_device *,
struct thermal_cooling_device *);
@@ -83,11 +124,6 @@ struct thermal_cooling_device_ops {
int (*set_cur_state) (struct thermal_cooling_device *, unsigned long);
};
-#define THERMAL_NO_LIMIT -1UL /* no upper/lower limit requirement */
-
-#define THERMAL_TRIPS_NONE -1
-#define THERMAL_MAX_TRIPS 12
-#define THERMAL_NAME_LENGTH 20
struct thermal_cooling_device {
int id;
char type[THERMAL_NAME_LENGTH];
@@ -100,10 +136,6 @@ struct thermal_cooling_device {
struct list_head node;
};
-#define KELVIN_TO_CELSIUS(t) (long)(((long)t-2732 >= 0) ? \
- ((long)t-2732+5)/10 : ((long)t-2732-5)/10)
-#define CELSIUS_TO_KELVIN(t) ((t)*10+2732)
-
struct thermal_attr {
struct device_attribute attr;
char name[THERMAL_NAME_LENGTH];
@@ -131,38 +163,13 @@ struct thermal_zone_device {
struct list_head node;
struct delayed_work poll_queue;
};
-/* Adding event notification support elements */
-#define THERMAL_GENL_FAMILY_NAME "thermal_event"
-#define THERMAL_GENL_VERSION 0x01
-#define THERMAL_GENL_MCAST_GROUP_NAME "thermal_mc_group"
-
-enum events {
- THERMAL_AUX0,
- THERMAL_AUX1,
- THERMAL_CRITICAL,
- THERMAL_DEV_FAULT,
-};
struct thermal_genl_event {
u32 orig;
enum events event;
};
-/* attributes of thermal_genl_family */
-enum {
- THERMAL_GENL_ATTR_UNSPEC,
- THERMAL_GENL_ATTR_EVENT,
- __THERMAL_GENL_ATTR_MAX,
-};
-#define THERMAL_GENL_ATTR_MAX (__THERMAL_GENL_ATTR_MAX - 1)
-
-/* commands supported by the thermal_genl_family */
-enum {
- THERMAL_GENL_CMD_UNSPEC,
- THERMAL_GENL_CMD_EVENT,
- __THERMAL_GENL_CMD_MAX,
-};
-#define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)
+/* Function declarations */
struct thermal_zone_device *thermal_zone_device_register(const char *, int, int,
void *, const struct thermal_zone_device_ops *, int, int);
void thermal_zone_device_unregister(struct thermal_zone_device *);
@@ -173,6 +180,7 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
int thermal_zone_unbind_cooling_device(struct thermal_zone_device *, int,
struct thermal_cooling_device *);
void thermal_zone_device_update(struct thermal_zone_device *);
+
struct thermal_cooling_device *thermal_cooling_device_register(char *, void *,
const struct thermal_cooling_device_ops *);
void thermal_cooling_device_unregister(struct thermal_cooling_device *);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv2 02/14] Thermal: Move thermal_instance to thermal_core.h
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
2012-08-27 4:28 ` [PATCHv2 01/14] Thermal: Refactor thermal.h file Durgadoss R
@ 2012-08-27 4:28 ` Durgadoss R
2012-08-27 4:28 ` [PATCHv2 03/14] Thermal: Add get trend, get instance API's to thermal_sys Durgadoss R
` (11 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch creates a thermal_core.h file which can contain
all defines used by the core thermal framework files. For
now, move the thermal_instance structure to thermal_core.h
This structure is used by files under drivers/thermal/.
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
drivers/thermal/thermal_core.h | 53 ++++++++++++++++++++++++++++++++++++++++
drivers/thermal/thermal_sys.c | 23 ++---------------
2 files changed, 55 insertions(+), 21 deletions(-)
create mode 100644 drivers/thermal/thermal_core.h
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
new file mode 100644
index 0000000..0d3205a
--- /dev/null
+++ b/drivers/thermal/thermal_core.h
@@ -0,0 +1,53 @@
+/*
+ * thermal_core.h
+ *
+ * Copyright (C) 2012 Intel Corp
+ * Author: Durgadoss R <durgadoss.r@intel.com>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#ifndef __THERMAL_CORE_H__
+#define __THERMAL_CORE_H__
+
+#include <linux/device.h>
+#include <linux/thermal.h>
+
+/* Initial state of a cooling device during binding */
+#define THERMAL_NO_TARGET -1UL
+
+/*
+ * This structure is used to describe the behavior of
+ * a certain cooling device on a certain trip point
+ * in a certain thermal zone
+ */
+struct thermal_instance {
+ int id;
+ char name[THERMAL_NAME_LENGTH];
+ struct thermal_zone_device *tz;
+ struct thermal_cooling_device *cdev;
+ int trip;
+ unsigned long upper; /* Highest cooling state for this trip point */
+ unsigned long lower; /* Lowest cooling state for this trip point */
+ unsigned long target; /* expected cooling state */
+ char attr_name[THERMAL_NAME_LENGTH];
+ struct device_attribute attr;
+ struct list_head tz_node; /* node in tz->thermal_instances */
+ struct list_head cdev_node; /* node in cdev->thermal_instances */
+};
+
+#endif /* __THERMAL_CORE_H__ */
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 5be8728..0e71b00 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -37,31 +37,12 @@
#include <net/netlink.h>
#include <net/genetlink.h>
+#include "thermal_core.h"
+
MODULE_AUTHOR("Zhang Rui");
MODULE_DESCRIPTION("Generic thermal management sysfs support");
MODULE_LICENSE("GPL");
-#define THERMAL_NO_TARGET -1UL
-/*
- * This structure is used to describe the behavior of
- * a certain cooling device on a certain trip point
- * in a certain thermal zone
- */
-struct thermal_instance {
- int id;
- char name[THERMAL_NAME_LENGTH];
- struct thermal_zone_device *tz;
- struct thermal_cooling_device *cdev;
- int trip;
- unsigned long upper; /* Highest cooling state for this trip point */
- unsigned long lower; /* Lowest cooling state for this trip point */
- unsigned long target; /* expected cooling state */
- char attr_name[THERMAL_NAME_LENGTH];
- struct device_attribute attr;
- struct list_head tz_node; /* node in tz->thermal_instances */
- struct list_head cdev_node; /* node in cdev->thermal_instances */
-};
-
static DEFINE_IDR(thermal_tz_idr);
static DEFINE_IDR(thermal_cdev_idr);
static DEFINE_MUTEX(thermal_idr_lock);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv2 03/14] Thermal: Add get trend, get instance API's to thermal_sys
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
2012-08-27 4:28 ` [PATCHv2 01/14] Thermal: Refactor thermal.h file Durgadoss R
2012-08-27 4:28 ` [PATCHv2 02/14] Thermal: Move thermal_instance to thermal_core.h Durgadoss R
@ 2012-08-27 4:28 ` Durgadoss R
2012-08-27 8:11 ` Zhang, Rui
2012-08-27 4:28 ` [PATCHv2 04/14] Thermal: Add platform level information to thermal.h Durgadoss R
` (10 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch adds the following API's to thermal_sys.c, that
can be used by other Thermal drivers.
* get_tz_trend: obtain the trend of the given thermal zone
* get_thermal_instance: obtain the instance corresponding
to the given tz, cdev and the trip point.
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
drivers/thermal/thermal_sys.c | 40 ++++++++++++++++++++++++++++++++++++++++
include/linux/thermal.h | 4 ++++
2 files changed, 44 insertions(+)
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 0e71b00..5e141b5 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -82,6 +82,46 @@ static void release_idr(struct idr *idr, struct mutex *lock, int id)
mutex_unlock(lock);
}
+int get_tz_trend(struct thermal_zone_device *tz, int trip)
+{
+ enum thermal_trend trend;
+
+ if (!tz->ops->get_trend || tz->ops->get_trend(tz, trip, &trend)) {
+ if (tz->temperature > tz->last_temperature)
+ trend = THERMAL_TREND_RAISING;
+ else if (tz->temperature < tz->last_temperature)
+ trend = THERMAL_TREND_DROPPING;
+ else
+ trend = THERMAL_TREND_STABLE;
+ }
+
+ return trend;
+}
+EXPORT_SYMBOL(get_tz_trend);
+
+struct thermal_instance *get_thermal_instance(struct thermal_zone_device *tz,
+ struct thermal_cooling_device *cdev, int trip)
+{
+ struct thermal_instance *pos = NULL;
+ struct thermal_instance *target_instance = NULL;
+
+ mutex_lock(&tz->lock);
+ mutex_lock(&cdev->lock);
+
+ list_for_each_entry(pos, &tz->thermal_instances, tz_node) {
+ if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) {
+ target_instance = pos;
+ break;
+ }
+ }
+
+ mutex_unlock(&cdev->lock);
+ mutex_unlock(&tz->lock);
+
+ return target_instance;
+}
+EXPORT_SYMBOL(get_thermal_instance);
+
/* sys I/F for thermal zone */
#define to_thermal_zone(_dev) \
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 8611e3e..32af124 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -185,6 +185,10 @@ struct thermal_cooling_device *thermal_cooling_device_register(char *, void *,
const struct thermal_cooling_device_ops *);
void thermal_cooling_device_unregister(struct thermal_cooling_device *);
+int get_tz_trend(struct thermal_zone_device *, int);
+struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
+ struct thermal_cooling_device *, int);
+
#ifdef CONFIG_NET
extern int thermal_generate_netlink_event(u32 orig, enum events event);
#else
--
1.7.9.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv2 04/14] Thermal: Add platform level information to thermal.h
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
` (2 preceding siblings ...)
2012-08-27 4:28 ` [PATCHv2 03/14] Thermal: Add get trend, get instance API's to thermal_sys Durgadoss R
@ 2012-08-27 4:28 ` Durgadoss R
2012-08-27 8:27 ` Zhang, Rui
2012-08-27 4:28 ` [PATCHv2 05/14] Thermal: Obtain platform data for thermal zone Durgadoss R
` (9 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch creates two structures; one to hold bind
bind parameters for a thermal zone, and another to
store platform layer data for a thermal zone, and
defines an extern function to retrieve these
parameters from thermal_sys.c. This patch also
defines an enum that describes various policies
for thermal throttling.
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
drivers/thermal/thermal_sys.c | 3 +++
include/linux/thermal.h | 46 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+)
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 5e141b5..92a187c 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -51,6 +51,9 @@ static LIST_HEAD(thermal_tz_list);
static LIST_HEAD(thermal_cdev_list);
static DEFINE_MUTEX(thermal_list_lock);
+int (*get_platform_thermal_params)(struct thermal_zone_device *);
+EXPORT_SYMBOL(get_platform_thermal_params);
+
static int get_idr(struct idr *idr, struct mutex *lock, int *id)
{
int err;
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 32af124..b644b8e 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -67,6 +67,12 @@ enum thermal_trend {
THERMAL_TREND_DROPPING, /* temperature is dropping */
};
+enum thermal_throttle_policy {
+ THERMAL_USER_SPACE,
+ THERMAL_FAIR_SHARE,
+ THERMAL_STEP_WISE,
+};
+
/* Events supported by Thermal Netlink */
enum events {
THERMAL_AUX0,
@@ -162,6 +168,37 @@ struct thermal_zone_device {
struct mutex lock; /* protect thermal_instances list */
struct list_head node;
struct delayed_work poll_queue;
+ struct thermal_zone_params *tzp;
+};
+
+/* Structure that holds binding parameters for a zone */
+struct thermal_bind_params {
+ struct thermal_cooling_device *cdev;
+
+ /*
+ * This is a measure of 'how effectively these devices can
+ * cool 'this' thermal zone. The shall be determined by platform
+ * characterization. This is on a 'percentage' scale.
+ * See Documentation/thermal/sysfs-api.txt for more information.
+ */
+ int weight;
+
+ /*
+ * This is a bit mask that gives the binding relation between this
+ * thermal zone and cdev, for a particular trip point.
+ * See Documentation/thermal/sysfs-api.txt for more information.
+ */
+ int trip_mask;
+ int (*match) (struct thermal_zone_device *tz,
+ struct thermal_cooling_device *cdev);
+};
+
+/* Structure to define Thermal Zone parameters */
+struct thermal_zone_params {
+ const char *zone_name;
+ enum thermal_throttle_policy throttle_policy;
+ int num_tbps; /* Number of tbp entries */
+ struct thermal_bind_params *tbp;
};
struct thermal_genl_event {
@@ -188,6 +225,15 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *);
int get_tz_trend(struct thermal_zone_device *, int);
struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
struct thermal_cooling_device *, int);
+/*
+ * The platform layer shall define a 'function' that provides the
+ * parameters for all thermal zones in the platform. This pointer
+ * should point to that 'function'.
+ *
+ * In thermal_zone_device_register() we update the parameters
+ * for the particular thermal zone.
+ */
+extern int (*get_platform_thermal_params)(struct thermal_zone_device *);
#ifdef CONFIG_NET
extern int thermal_generate_netlink_event(u32 orig, enum events event);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv2 05/14] Thermal: Obtain platform data for thermal zone
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
` (3 preceding siblings ...)
2012-08-27 4:28 ` [PATCHv2 04/14] Thermal: Add platform level information to thermal.h Durgadoss R
@ 2012-08-27 4:28 ` Durgadoss R
2012-08-27 8:29 ` Zhang, Rui
2012-08-27 4:28 ` [PATCHv2 06/14] Thermal: Add a policy sysfs attribute Durgadoss R
` (8 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch retrieves the platform level data for
a zone during its registration. It is not an error
to not have any platform data.
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
drivers/thermal/thermal_sys.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 92a187c..6adda39 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -1339,6 +1339,22 @@ static void remove_trip_attrs(struct thermal_zone_device *tz)
kfree(tz->trip_hyst_attrs);
}
+static int retrieve_zone_params(struct thermal_zone_device *tz)
+{
+ int ret;
+
+ /* Check whether the platform data pointer is defined */
+ if (!get_platform_thermal_params)
+ return 0;
+
+ /* It is not an error to not have any platform data */
+ ret = get_platform_thermal_params(tz);
+ if (ret)
+ tz->tzp = NULL;
+
+ return 0;
+}
+
/**
* thermal_zone_device_register - register a new thermal zone device
* @type: the thermal zone device type
@@ -1443,6 +1459,11 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
if (result)
goto unregister;
+ /* Retrieve platform level parameters for this zone */
+ result = retrieve_zone_params(tz);
+ if (result)
+ goto unregister;
+
mutex_lock(&thermal_list_lock);
list_add_tail(&tz->node, &thermal_tz_list);
if (ops->bind)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv2 06/14] Thermal: Add a policy sysfs attribute
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
` (4 preceding siblings ...)
2012-08-27 4:28 ` [PATCHv2 05/14] Thermal: Obtain platform data for thermal zone Durgadoss R
@ 2012-08-27 4:28 ` Durgadoss R
2012-08-27 8:31 ` Zhang, Rui
2012-08-27 9:35 ` Zhang, Rui
2012-08-27 4:28 ` [PATCHv2 07/14] Thermal: Update binding logic based on platform data Durgadoss R
` (7 subsequent siblings)
13 siblings, 2 replies; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch adds a policy sysfs attribute to a thermal zone.
This attribute will give us the throttling policy used
for the zone. This is a RO attribute.
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
drivers/thermal/thermal_sys.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 6adda39..8aa4200a6 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -378,10 +378,32 @@ passive_show(struct device *dev, struct device_attribute *attr,
return sprintf(buf, "%d\n", tz->forced_passive);
}
+static ssize_t
+policy_show(struct device *dev, struct device_attribute *devattr, char *buf)
+{
+ struct thermal_zone_device *tz = to_thermal_zone(dev);
+ struct thermal_zone_params *tzp = tz->tzp;
+
+ if (!tzp)
+ return sprintf(buf, "step_wise(default)\n");
+
+ switch (tzp->throttle_policy) {
+ case THERMAL_FAIR_SHARE:
+ return sprintf(buf, "fair_share\n");
+ case THERMAL_STEP_WISE:
+ return sprintf(buf, "step_wise\n");
+ case THERMAL_USER_SPACE:
+ return sprintf(buf, "user_space\n");
+ default:
+ return sprintf(buf, "unknown\n");
+ }
+}
+
static DEVICE_ATTR(type, 0444, type_show, NULL);
static DEVICE_ATTR(temp, 0444, temp_show, NULL);
static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, passive_store);
+static DEVICE_ATTR(throttle_policy, S_IRUGO, policy_show, NULL);
/* sys I/F for cooling device */
#define to_cooling_device(_dev) \
@@ -1349,10 +1371,16 @@ static int retrieve_zone_params(struct thermal_zone_device *tz)
/* It is not an error to not have any platform data */
ret = get_platform_thermal_params(tz);
- if (ret)
+ if (ret) {
tz->tzp = NULL;
+ return 0;
+ }
- return 0;
+ ret = device_create_file(&tz->device, &dev_attr_throttle_policy);
+ if (ret)
+ dev_err(&tz->device, "creating policy attr failed:%d\n", ret);
+
+ return ret;
}
/**
--
1.7.9.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv2 07/14] Thermal: Update binding logic based on platform data
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
` (5 preceding siblings ...)
2012-08-27 4:28 ` [PATCHv2 06/14] Thermal: Add a policy sysfs attribute Durgadoss R
@ 2012-08-27 4:28 ` Durgadoss R
2012-08-27 9:50 ` Zhang, Rui
2012-08-27 4:28 ` [PATCHv2 08/14] Thermal: Make thermal_cdev_update as a global function Durgadoss R
` (6 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch updates the binding logic in thermal_sys.c
It uses the platform layer data to bind a thermal zone
to a cdev for a particular trip point.
* If we do not have platform data and do not have
.bind defined, do not bind.
* If we do not have platform data but .bind is
defined, then use tz->ops->bind.
* If we have platform data, use it to create binding.
The same logic sequence is followed for unbind also.
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
drivers/thermal/thermal_sys.c | 184 +++++++++++++++++++++++++++++++++++------
1 file changed, 158 insertions(+), 26 deletions(-)
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 8aa4200a6..5d38501 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -125,6 +125,111 @@ struct thermal_instance *get_thermal_instance(struct thermal_zone_device *tz,
}
EXPORT_SYMBOL(get_thermal_instance);
+static void print_bind_err_msg(struct thermal_zone_device *tz,
+ struct thermal_cooling_device *cdev, int ret)
+{
+ dev_err(&tz->device, "binding zone %s with cdev %s failed:%d\n",
+ tz->type, cdev->type, ret);
+}
+
+static void __bind(struct thermal_zone_device *tz, int mask,
+ struct thermal_cooling_device *cdev)
+{
+ int i, ret;
+
+ for (i = 0; i < tz->trips; i++) {
+ if (mask & (1 << i)) {
+ ret = thermal_zone_bind_cooling_device(tz, i, cdev,
+ THERMAL_NO_LIMIT, THERMAL_NO_LIMIT);
+ if (ret)
+ print_bind_err_msg(tz, cdev, ret);
+ }
+ }
+}
+
+static void __unbind(struct thermal_zone_device *tz, int mask,
+ struct thermal_cooling_device *cdev)
+{
+ int i;
+
+ for (i = 0; i < tz->trips; i++)
+ if (mask & (1 << i))
+ thermal_zone_unbind_cooling_device(tz, i, cdev);
+}
+
+static void update_bind_info(struct thermal_cooling_device *cdev)
+{
+ int i, ret;
+ struct thermal_zone_params *tzp;
+ struct thermal_zone_device *pos = NULL;
+
+ mutex_lock(&thermal_list_lock);
+
+ list_for_each_entry(pos, &thermal_tz_list, node) {
+ if (!pos->tzp && !pos->ops->bind)
+ continue;
+
+ if (!pos->tzp && pos->ops->bind) {
+ ret = pos->ops->bind(pos, cdev);
+ if (ret)
+ print_bind_err_msg(pos, cdev, ret);
+ }
+
+ tzp = pos->tzp;
+ if (!tzp->tbp)
+ return;
+
+ for (i = 0; i < tzp->num_tbps; i++) {
+ if (tzp->tbp[i].cdev || !tzp->tbp[i].match)
+ continue;
+ if (tzp->tbp[i].match(pos, cdev))
+ continue;
+ tzp->tbp[i].cdev = cdev;
+ __bind(pos, tzp->tbp[i].trip_mask, cdev);
+ }
+ }
+
+ mutex_unlock(&thermal_list_lock);
+}
+
+static void do_binding(struct thermal_zone_device *tz)
+{
+ int i, ret;
+ struct thermal_cooling_device *pos = NULL;
+ struct thermal_zone_params *tzp = tz->tzp;
+
+ if (!tzp && !tz->ops->bind)
+ return;
+
+ mutex_lock(&thermal_list_lock);
+
+ /* If there is no platform data, try to use ops->bind */
+ if (!tzp && tz->ops->bind) {
+ list_for_each_entry(pos, &thermal_cdev_list, node) {
+ ret = tz->ops->bind(tz, pos);
+ if (ret)
+ print_bind_err_msg(tz, pos, ret);
+ }
+ goto exit;
+ }
+
+ if (!tzp->tbp)
+ goto exit;
+
+ list_for_each_entry(pos, &thermal_cdev_list, node) {
+ for (i = 0; i < tzp->num_tbps; i++) {
+ if (tzp->tbp[i].cdev || !tzp->tbp[i].match)
+ continue;
+ if (tzp->tbp[i].match(tz, pos))
+ continue;
+ tzp->tbp[i].cdev = pos;
+ __bind(tz, tzp->tbp[i].trip_mask, pos);
+ }
+ }
+exit:
+ mutex_unlock(&thermal_list_lock);
+}
+
/* sys I/F for thermal zone */
#define to_thermal_zone(_dev) \
@@ -943,7 +1048,6 @@ thermal_cooling_device_register(char *type, void *devdata,
const struct thermal_cooling_device_ops *ops)
{
struct thermal_cooling_device *cdev;
- struct thermal_zone_device *pos;
int result;
if (strlen(type) >= THERMAL_NAME_LENGTH)
@@ -993,20 +1097,15 @@ thermal_cooling_device_register(char *type, void *devdata,
if (result)
goto unregister;
+ /* Add 'this' new cdev to the global cdev list */
mutex_lock(&thermal_list_lock);
list_add(&cdev->node, &thermal_cdev_list);
- list_for_each_entry(pos, &thermal_tz_list, node) {
- if (!pos->ops->bind)
- continue;
- result = pos->ops->bind(pos, cdev);
- if (result)
- break;
-
- }
mutex_unlock(&thermal_list_lock);
- if (!result)
- return cdev;
+ /* Update binding information for 'this' new cdev */
+ update_bind_info(cdev);
+
+ return cdev;
unregister:
release_idr(&thermal_cdev_idr, &thermal_idr_lock, cdev->id);
@@ -1022,10 +1121,10 @@ EXPORT_SYMBOL(thermal_cooling_device_register);
* thermal_cooling_device_unregister() must be called when the device is no
* longer needed.
*/
-void thermal_cooling_device_unregister(struct
- thermal_cooling_device
- *cdev)
+void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
{
+ int i;
+ struct thermal_zone_params *tzp;
struct thermal_zone_device *tz;
struct thermal_cooling_device *pos = NULL;
@@ -1042,12 +1141,28 @@ void thermal_cooling_device_unregister(struct
return;
}
list_del(&cdev->node);
+
+ /* Unbind all thermal zones associated with 'this' cdev */
list_for_each_entry(tz, &thermal_tz_list, node) {
- if (!tz->ops->unbind)
+ if (tz->ops->unbind) {
+ tz->ops->unbind(tz, cdev);
continue;
- tz->ops->unbind(tz, cdev);
+ }
+
+ if (!tz->tzp || !tz->tzp->tbp)
+ continue;
+
+ tzp = tz->tzp;
+ for (i = 0; i < tzp->num_tbps; i++) {
+ if (tzp->tbp[i].cdev == cdev) {
+ __unbind(tz, tzp->tbp[i].trip_mask, cdev);
+ tzp->tbp[i].cdev = NULL;
+ }
+ }
}
+
mutex_unlock(&thermal_list_lock);
+
if (cdev->type[0])
device_remove_file(&cdev->device, &dev_attr_cdev_type);
device_remove_file(&cdev->device, &dev_attr_max_state);
@@ -1405,7 +1520,6 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
int passive_delay, int polling_delay)
{
struct thermal_zone_device *tz;
- struct thermal_cooling_device *pos;
enum thermal_trip_type trip_type;
int result;
int count;
@@ -1494,14 +1608,12 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
mutex_lock(&thermal_list_lock);
list_add_tail(&tz->node, &thermal_tz_list);
- if (ops->bind)
- list_for_each_entry(pos, &thermal_cdev_list, node) {
- result = ops->bind(tz, pos);
- if (result)
- break;
- }
mutex_unlock(&thermal_list_lock);
+ /* Bind cooling devices for this zone */
+ do_binding(tz);
+
+
INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check);
thermal_zone_device_update(tz);
@@ -1522,12 +1634,16 @@ EXPORT_SYMBOL(thermal_zone_device_register);
*/
void thermal_zone_device_unregister(struct thermal_zone_device *tz)
{
+ int i;
+ struct thermal_zone_params *tzp;
struct thermal_cooling_device *cdev;
struct thermal_zone_device *pos = NULL;
if (!tz)
return;
+ tzp = tz->tzp;
+
mutex_lock(&thermal_list_lock);
list_for_each_entry(pos, &thermal_tz_list, node)
if (pos == tz)
@@ -1538,9 +1654,25 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
return;
}
list_del(&tz->node);
- if (tz->ops->unbind)
- list_for_each_entry(cdev, &thermal_cdev_list, node)
- tz->ops->unbind(tz, cdev);
+
+ /* Unbind all cdevs associated with 'this' thermal zone */
+ list_for_each_entry(cdev, &thermal_cdev_list, node) {
+ if (tz->ops->unbind) {
+ tz->ops->unbind(tz, cdev);
+ continue;
+ }
+
+ if (!tzp || !tzp->tbp)
+ break;
+
+ for (i = 0; i < tzp->num_tbps; i++) {
+ if (tzp->tbp[i].cdev == cdev) {
+ __unbind(tz, tzp->tbp[i].trip_mask, cdev);
+ tzp->tbp[i].cdev = NULL;
+ }
+ }
+ }
+
mutex_unlock(&thermal_list_lock);
thermal_zone_device_set_polling(tz, 0);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv2 08/14] Thermal: Make thermal_cdev_update as a global function
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
` (6 preceding siblings ...)
2012-08-27 4:28 ` [PATCHv2 07/14] Thermal: Update binding logic based on platform data Durgadoss R
@ 2012-08-27 4:28 ` Durgadoss R
2012-08-27 4:28 ` [PATCHv2 09/14] Thermal: Introduce fair_share thermal governor Durgadoss R
` (5 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch makes the thermal_cdev_update function as a
global one, so that other files can use it. This function
serves as a single arbitrator to set the state of a cooling
device.
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
drivers/thermal/thermal_sys.c | 5 +++--
include/linux/thermal.h | 2 ++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 5d38501..768458b 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -1174,7 +1174,7 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
}
EXPORT_SYMBOL(thermal_cooling_device_unregister);
-static void thermal_cdev_do_update(struct thermal_cooling_device *cdev)
+void thermal_cdev_update(struct thermal_cooling_device *cdev)
{
struct thermal_instance *instance;
unsigned long target = 0;
@@ -1195,13 +1195,14 @@ static void thermal_cdev_do_update(struct thermal_cooling_device *cdev)
cdev->ops->set_cur_state(cdev, target);
cdev->updated = true;
}
+EXPORT_SYMBOL(thermal_cdev_update);
static void thermal_zone_do_update(struct thermal_zone_device *tz)
{
struct thermal_instance *instance;
list_for_each_entry(instance, &tz->thermal_instances, tz_node)
- thermal_cdev_do_update(instance->cdev);
+ thermal_cdev_update(instance->cdev);
}
/*
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index b644b8e..591f350 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -225,6 +225,8 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *);
int get_tz_trend(struct thermal_zone_device *, int);
struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
struct thermal_cooling_device *, int);
+void thermal_cdev_update(struct thermal_cooling_device *);
+
/*
* The platform layer shall define a 'function' that provides the
* parameters for all thermal zones in the platform. This pointer
--
1.7.9.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv2 09/14] Thermal: Introduce fair_share thermal governor
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
` (7 preceding siblings ...)
2012-08-27 4:28 ` [PATCHv2 08/14] Thermal: Make thermal_cdev_update as a global function Durgadoss R
@ 2012-08-27 4:28 ` Durgadoss R
2012-08-27 4:28 ` [PATCHv2 10/14] Thermal: Introduce a step_wise " Durgadoss R
` (4 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch introduces a simple 'weight' based
governor named fair_share governor. Whenever the
thermal framework gets notified of the trip point
violation, this governor (if configured), throttles
the cooling devices associated with a thermal zone.
This mapping between a thermal zone and a cooling device
and the effectiveness of cooling are provided in the
platform layer.
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
drivers/thermal/Kconfig | 6 +++
drivers/thermal/Makefile | 3 +-
drivers/thermal/fair_share.c | 113 ++++++++++++++++++++++++++++++++++++++++++
include/linux/thermal.h | 9 ++++
4 files changed, 130 insertions(+), 1 deletion(-)
create mode 100644 drivers/thermal/fair_share.c
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 3ab2bd5..f5110c0 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -27,3 +27,9 @@ config SPEAR_THERMAL
help
Enable this to plug the SPEAr thermal sensor driver into the Linux
thermal framework
+
+config FAIR_SHARE
+ bool "Fair-share thermal governor"
+ depends on THERMAL
+ help
+ Enable this to manage platform thermals using fair-share governor.
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index a9fff0b..4ffe1a8 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -3,4 +3,5 @@
#
obj-$(CONFIG_THERMAL) += thermal_sys.o
-obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
\ No newline at end of file
+obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
+obj-$(CONFIG_FAIR_SHARE) += fair_share.o
diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
new file mode 100644
index 0000000..a57e40d
--- /dev/null
+++ b/drivers/thermal/fair_share.c
@@ -0,0 +1,113 @@
+/*
+ * fair_share.c - A simple weight based Thermal governor
+ *
+ * Copyright (C) 2012 Intel Corp
+ * Copyright (C) 2012 Durgadoss R <durgadoss.r@intel.com>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/thermal.h>
+
+#include "thermal_core.h"
+
+/**
+ * get_trip_level: - obtains the current trip level for a zone
+ * @tz: thermal zone device
+ */
+static int get_trip_level(struct thermal_zone_device *tz)
+{
+ int count = 0;
+ unsigned long trip_temp;
+
+ if (tz->trips == 0 || !tz->ops->get_trip_temp)
+ return 0;
+
+ for (count = 0; count < tz->trips; count++) {
+ tz->ops->get_trip_temp(tz, count, &trip_temp);
+ if (tz->temperature < trip_temp)
+ break;
+ }
+ return count;
+}
+
+static long get_target_state(struct thermal_zone_device *tz,
+ struct thermal_cooling_device *cdev, int weight, int level)
+{
+ unsigned long max_state;
+
+ cdev->ops->get_max_state(cdev, &max_state);
+
+ return (long)(weight * level * max_state) / (100 * tz->trips);
+}
+
+/**
+ * fair_share_throttle - throttles devices asscciated with the given zone
+ * @tz - thermal_zone_device
+ *
+ * Throttling Logic: This uses three parameters to calculate the new
+ * throttle state of the cooling devices associated with the given zone.
+ *
+ * Parameters used for Throttling:
+ * P1. max_state: Maximum throttle state exposed by the cooling device.
+ * P2. weight[i]/100:
+ * How 'effective' the 'i'th device is, in cooling the given zone.
+ * P3. cur_trip_level/max_no_of_trips:
+ * This describes the extent to which the devices should be throttled.
+ * We do not want to throttle too much when we trip a lower temperature,
+ * whereas the throttling is at full swing if we trip critical levels.
+ * (Heavily assumes the trip points are in ascending order)
+ * new_state of cooling device = P3 * P2 * P1
+ */
+void fair_share_throttle(struct thermal_zone_device *tz, int trip)
+{
+ struct thermal_zone_params *tzp;
+ struct thermal_cooling_device *cdev;
+ struct thermal_instance *instance;
+ int i;
+ int cur_trip_level = get_trip_level(tz);
+
+ if (!tz->tzp || !tz->tzp->tbp)
+ return;
+
+ tzp = tz->tzp;
+
+ for (i = 0; i < tzp->num_tbps; i++) {
+ if (!tzp->tbp[i].cdev)
+ continue;
+
+ cdev = tzp->tbp[i].cdev;
+ instance = get_thermal_instance(tz, cdev, trip);
+ if (!instance)
+ continue;
+
+ instance->target = get_target_state(tz, cdev,
+ tzp->tbp[i].weight, cur_trip_level);
+
+ instance->cdev->updated = false;
+ thermal_cdev_update(cdev);
+ }
+}
+EXPORT_SYMBOL(fair_share_throttle);
+
+MODULE_AUTHOR("Durgadoss R");
+MODULE_DESCRIPTION("A simple weight based thermal throttling governor");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 591f350..472beed 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -237,6 +237,15 @@ void thermal_cdev_update(struct thermal_cooling_device *);
*/
extern int (*get_platform_thermal_params)(struct thermal_zone_device *);
+#ifdef CONFIG_FAIR_SHARE
+extern void fair_share_throttle(struct thermal_zone_device *tz, int trip);
+#else
+static inline void fair_share_throttle(struct thermal_zone_device *tz, int trip)
+{
+ return;
+}
+#endif
+
#ifdef CONFIG_NET
extern int thermal_generate_netlink_event(u32 orig, enum events event);
#else
--
1.7.9.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv2 10/14] Thermal: Introduce a step_wise thermal governor
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
` (8 preceding siblings ...)
2012-08-27 4:28 ` [PATCHv2 09/14] Thermal: Introduce fair_share thermal governor Durgadoss R
@ 2012-08-27 4:28 ` Durgadoss R
2012-08-27 4:28 ` [PATCHv2 11/14] Thermal: Remove throttling logic out of thermal_sys.c Durgadoss R
` (3 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch adds a simple step_wise governor to the
generic thermal layer. This algorithm throttles the
cooling devices in a linear fashion. If the 'trend'
is heating, it throttles by one step. And if the
thermal trend is cooling it de-throttles by one step.
This actually moves the throttling logic from thermal_sys.c
and puts inside step_wise.c, without any change.
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
drivers/thermal/Kconfig | 6 ++
drivers/thermal/Makefile | 1 +
drivers/thermal/step_wise.c | 173 +++++++++++++++++++++++++++++++++++++++++++
include/linux/thermal.h | 9 +++
4 files changed, 189 insertions(+)
create mode 100644 drivers/thermal/step_wise.c
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index f5110c0..0401cdf 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -33,3 +33,9 @@ config FAIR_SHARE
depends on THERMAL
help
Enable this to manage platform thermals using fair-share governor.
+
+config STEP_WISE
+ bool "Step_wise thermal governor"
+ depends on THERMAL
+ help
+ Enable this to manage platform thermals using a simple linear
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 4ffe1a8..c2c0ce0 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -5,3 +5,4 @@
obj-$(CONFIG_THERMAL) += thermal_sys.o
obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
obj-$(CONFIG_FAIR_SHARE) += fair_share.o
+obj-$(CONFIG_STEP_WISE) += step_wise.o
diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
new file mode 100644
index 0000000..1161974
--- /dev/null
+++ b/drivers/thermal/step_wise.c
@@ -0,0 +1,173 @@
+/*
+ * step_wise.c - A step-by-step Thermal throttling governor
+ *
+ * Copyright (C) 2012 Intel Corp
+ * Copyright (C) 2012 Durgadoss R <durgadoss.r@intel.com>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/thermal.h>
+
+#include "thermal_core.h"
+
+/*
+ * If the temperature is higher than a trip point,
+ * a. if the trend is THERMAL_TREND_RAISING, use higher cooling
+ * state for this trip point
+ * b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
+ * state for this trip point
+ */
+static unsigned long get_target_state(struct thermal_instance *instance,
+ enum thermal_trend trend)
+{
+ struct thermal_cooling_device *cdev = instance->cdev;
+ unsigned long cur_state;
+
+ cdev->ops->get_cur_state(cdev, &cur_state);
+
+ if (trend == THERMAL_TREND_RAISING) {
+ cur_state = cur_state < instance->upper ?
+ (cur_state + 1) : instance->upper;
+ } else if (trend == THERMAL_TREND_DROPPING) {
+ cur_state = cur_state > instance->lower ?
+ (cur_state - 1) : instance->lower;
+ }
+
+ return cur_state;
+}
+
+static void update_passive_instance(struct thermal_zone_device *tz,
+ enum thermal_trip_type type, int value)
+{
+ /*
+ * If value is +1, activate a passive instance.
+ * If value is -1, deactivate a passive instance.
+ */
+ if (type == THERMAL_TRIP_PASSIVE || type == THERMAL_TRIPS_NONE)
+ tz->passive += value;
+}
+
+static void update_instance_for_throttle(struct thermal_zone_device *tz,
+ int trip, enum thermal_trip_type trip_type,
+ enum thermal_trend trend)
+{
+ struct thermal_instance *instance;
+
+ list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
+ if (instance->trip != trip)
+ continue;
+
+ instance->target = get_target_state(instance, trend);
+
+ /* Activate a passive thermal instance */
+ if (instance->target == THERMAL_NO_TARGET)
+ update_passive_instance(tz, trip_type, 1);
+
+ instance->cdev->updated = false; /* cdev needs update */
+ }
+}
+
+static void update_instance_for_dethrottle(struct thermal_zone_device *tz,
+ int trip, enum thermal_trip_type trip_type)
+{
+ struct thermal_instance *instance;
+ struct thermal_cooling_device *cdev;
+ unsigned long cur_state;
+
+ list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
+ if (instance->trip != trip ||
+ instance->target == THERMAL_NO_TARGET)
+ continue;
+
+ cdev = instance->cdev;
+ cdev->ops->get_cur_state(cdev, &cur_state);
+
+ instance->target = cur_state > instance->lower ?
+ (cur_state - 1) : THERMAL_NO_TARGET;
+
+ /* Deactivate a passive thermal instance */
+ if (instance->target == THERMAL_NO_TARGET)
+ update_passive_instance(tz, trip_type, -1);
+
+ cdev->updated = false; /* cdev needs update */
+ }
+}
+
+static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
+{
+ long trip_temp;
+ enum thermal_trip_type trip_type;
+ enum thermal_trend trend;
+
+ if (trip == THERMAL_TRIPS_NONE) {
+ trip_temp = tz->forced_passive;
+ trip_type = THERMAL_TRIPS_NONE;
+ } else {
+ tz->ops->get_trip_temp(tz, trip, &trip_temp);
+ tz->ops->get_trip_type(tz, trip, &trip_type);
+ }
+
+ trend = get_tz_trend(tz, trip);
+
+ mutex_lock(&tz->lock);
+
+ if (tz->temperature >= trip_temp)
+ update_instance_for_throttle(tz, trip, trip_type, trend);
+ else
+ update_instance_for_dethrottle(tz, trip, trip_type);
+
+ mutex_unlock(&tz->lock);
+}
+
+/**
+ * step_wise_throttle - throttles devices asscciated with the given zone
+ * @tz - thermal_zone_device
+ * @trip - the trip point
+ * @trip_type - type of the trip point
+ *
+ * Throttling Logic: This uses the trend of the thermal zone to throttle.
+ * If the thermal zone is 'heating up' this throttles all the cooling
+ * devices associated with the zone and its particular trip point, by one
+ * step. If the zone is 'cooling down' it brings back the performance of
+ * the devices by one step.
+ */
+void step_wise_throttle(struct thermal_zone_device *tz, int trip)
+{
+ struct thermal_instance *instance;
+
+ thermal_zone_trip_update(tz, trip);
+
+ if (tz->forced_passive)
+ thermal_zone_trip_update(tz, THERMAL_TRIPS_NONE);
+
+ mutex_lock(&tz->lock);
+
+ list_for_each_entry(instance, &tz->thermal_instances, tz_node)
+ thermal_cdev_update(instance->cdev);
+
+ mutex_unlock(&tz->lock);
+}
+EXPORT_SYMBOL(step_wise_throttle);
+
+MODULE_AUTHOR("Durgadoss R");
+MODULE_DESCRIPTION("A step-by-step thermal throttling governor");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 472beed..c7e0a4d 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -246,6 +246,15 @@ static inline void fair_share_throttle(struct thermal_zone_device *tz, int trip)
}
#endif
+#ifdef CONFIG_STEP_WISE
+extern void step_wise_throttle(struct thermal_zone_device *tz, int trip);
+#else
+static inline void step_wise_throttle(struct thermal_zone_device *tz, int trip)
+{
+ return;
+}
+#endif
+
#ifdef CONFIG_NET
extern int thermal_generate_netlink_event(u32 orig, enum events event);
#else
--
1.7.9.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv2 11/14] Thermal: Remove throttling logic out of thermal_sys.c
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
` (9 preceding siblings ...)
2012-08-27 4:28 ` [PATCHv2 10/14] Thermal: Introduce a step_wise " Durgadoss R
@ 2012-08-27 4:28 ` Durgadoss R
2012-08-27 4:28 ` [PATCHv2 12/14] Thermal: Add a notification API Durgadoss R
` (2 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch removes the throttling logic out of
thermal_sys.c; also refactors the code into smaller
functions so that are easy to read/maintain.
* Seperates the handling of critical and non-critical trips
* Re-arranges the set_polling and device_check methods, so
that all related functions are arranged in one place.
* Removes the 'do_update' and 'trip_update' method, as part
of moving the throttling logic out of thermal_sys.c
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
drivers/thermal/thermal_sys.c | 337 +++++++++++++++++------------------------
1 file changed, 136 insertions(+), 201 deletions(-)
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 768458b..3f96f3e 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -230,6 +230,142 @@ exit:
mutex_unlock(&thermal_list_lock);
}
+static void thermal_zone_device_set_polling(struct thermal_zone_device *tz,
+ int delay)
+{
+ cancel_delayed_work(&(tz->poll_queue));
+
+ if (!delay)
+ return;
+
+ if (delay > 1000)
+ queue_delayed_work(system_freezable_wq, &(tz->poll_queue),
+ round_jiffies(msecs_to_jiffies(delay)));
+ else
+ queue_delayed_work(system_freezable_wq, &(tz->poll_queue),
+ msecs_to_jiffies(delay));
+}
+
+static void monitor_thermal_zone(struct thermal_zone_device *tz)
+{
+ mutex_lock(&tz->lock);
+
+ if (tz->passive)
+ thermal_zone_device_set_polling(tz, tz->passive_delay);
+ else if (tz->polling_delay)
+ thermal_zone_device_set_polling(tz, tz->polling_delay);
+ else
+ thermal_zone_device_set_polling(tz, 0);
+
+ mutex_unlock(&tz->lock);
+}
+
+static void notify_user_space(struct thermal_zone_device *tz, int trip)
+{
+ mutex_lock(&tz->lock);
+
+ kobject_uevent(&tz->device.kobj, KOBJ_CHANGE);
+
+ mutex_unlock(&tz->lock);
+}
+
+static void handle_non_critical_trips(struct thermal_zone_device *tz,
+ int trip, enum thermal_trip_type trip_type)
+{
+ int throttle_policy = THERMAL_STEP_WISE;
+
+ if (tz->tzp)
+ throttle_policy = tz->tzp->throttle_policy;
+
+ switch (throttle_policy) {
+ case THERMAL_FAIR_SHARE:
+ fair_share_throttle(tz, trip);
+ break;
+ case THERMAL_STEP_WISE:
+ step_wise_throttle(tz, trip);
+ break;
+ case THERMAL_USER_SPACE:
+ notify_user_space(tz, trip);
+ break;
+ }
+}
+
+static void handle_critical_trips(struct thermal_zone_device *tz,
+ int trip, enum thermal_trip_type trip_type)
+{
+ long trip_temp;
+
+ tz->ops->get_trip_temp(tz, trip, &trip_temp);
+
+ /* If we have not crossed the trip_temp, we do not care. */
+ if (tz->temperature < trip_temp)
+ return;
+
+ if (tz->ops->notify)
+ tz->ops->notify(tz, trip, trip_type);
+
+ if (trip_type == THERMAL_TRIP_CRITICAL) {
+ pr_emerg("Critical temperature reached(%d C),shutting down\n",
+ tz->temperature / 1000);
+ orderly_poweroff(true);
+ }
+}
+
+static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
+{
+ enum thermal_trip_type type;
+
+ tz->ops->get_trip_type(tz, trip, &type);
+
+ if (type == THERMAL_TRIP_CRITICAL || type == THERMAL_TRIP_HOT)
+ handle_critical_trips(tz, trip, type);
+ else
+ handle_non_critical_trips(tz, trip, type);
+ /*
+ * Alright, we handled this trip successfully.
+ * So, start monitoring again.
+ */
+ monitor_thermal_zone(tz);
+}
+
+static void update_temperature(struct thermal_zone_device *tz)
+{
+ long temp;
+ int ret;
+
+ mutex_lock(&tz->lock);
+
+ ret = tz->ops->get_temp(tz, &temp);
+ if (ret) {
+ pr_warn("failed to read out thermal zone %d\n", tz->id);
+ return;
+ }
+
+ tz->last_temperature = tz->temperature;
+ tz->temperature = temp;
+
+ mutex_unlock(&tz->lock);
+}
+
+void thermal_zone_device_update(struct thermal_zone_device *tz)
+{
+ int count;
+
+ update_temperature(tz);
+
+ for (count = 0; count < tz->trips; count++)
+ handle_thermal_trip(tz, count);
+}
+EXPORT_SYMBOL(thermal_zone_device_update);
+
+static void thermal_zone_device_check(struct work_struct *work)
+{
+ struct thermal_zone_device *tz = container_of(work, struct
+ thermal_zone_device,
+ poll_queue.work);
+ thermal_zone_device_update(tz);
+}
+
/* sys I/F for thermal zone */
#define to_thermal_zone(_dev) \
@@ -850,30 +986,6 @@ thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
}
#endif
-static void thermal_zone_device_set_polling(struct thermal_zone_device *tz,
- int delay)
-{
- cancel_delayed_work(&(tz->poll_queue));
-
- if (!delay)
- return;
-
- if (delay > 1000)
- queue_delayed_work(system_freezable_wq, &(tz->poll_queue),
- round_jiffies(msecs_to_jiffies(delay)));
- else
- queue_delayed_work(system_freezable_wq, &(tz->poll_queue),
- msecs_to_jiffies(delay));
-}
-
-static void thermal_zone_device_check(struct work_struct *work)
-{
- struct thermal_zone_device *tz = container_of(work, struct
- thermal_zone_device,
- poll_queue.work);
- thermal_zone_device_update(tz);
-}
-
/**
* thermal_zone_bind_cooling_device - bind a cooling device to a thermal zone
* @tz: thermal zone device
@@ -1197,183 +1309,6 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev)
}
EXPORT_SYMBOL(thermal_cdev_update);
-static void thermal_zone_do_update(struct thermal_zone_device *tz)
-{
- struct thermal_instance *instance;
-
- list_for_each_entry(instance, &tz->thermal_instances, tz_node)
- thermal_cdev_update(instance->cdev);
-}
-
-/*
- * Cooling algorithm for both active and passive cooling
- *
- * 1. if the temperature is higher than a trip point,
- * a. if the trend is THERMAL_TREND_RAISING, use higher cooling
- * state for this trip point
- * b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
- * state for this trip point
- *
- * 2. if the temperature is lower than a trip point, use lower
- * cooling state for this trip point
- *
- * Note that this behaves the same as the previous passive cooling
- * algorithm.
- */
-
-static void thermal_zone_trip_update(struct thermal_zone_device *tz,
- int trip, long temp)
-{
- struct thermal_instance *instance;
- struct thermal_cooling_device *cdev = NULL;
- unsigned long cur_state, max_state;
- long trip_temp;
- enum thermal_trip_type trip_type;
- enum thermal_trend trend;
-
- if (trip == THERMAL_TRIPS_NONE) {
- trip_temp = tz->forced_passive;
- trip_type = THERMAL_TRIPS_NONE;
- } else {
- tz->ops->get_trip_temp(tz, trip, &trip_temp);
- tz->ops->get_trip_type(tz, trip, &trip_type);
- }
-
- if (!tz->ops->get_trend || tz->ops->get_trend(tz, trip, &trend)) {
- /*
- * compare the current temperature and previous temperature
- * to get the thermal trend, if no special requirement
- */
- if (tz->temperature > tz->last_temperature)
- trend = THERMAL_TREND_RAISING;
- else if (tz->temperature < tz->last_temperature)
- trend = THERMAL_TREND_DROPPING;
- else
- trend = THERMAL_TREND_STABLE;
- }
-
- if (temp >= trip_temp) {
- list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
- if (instance->trip != trip)
- continue;
-
- cdev = instance->cdev;
-
- cdev->ops->get_cur_state(cdev, &cur_state);
- cdev->ops->get_max_state(cdev, &max_state);
-
- if (trend == THERMAL_TREND_RAISING) {
- cur_state = cur_state < instance->upper ?
- (cur_state + 1) : instance->upper;
- } else if (trend == THERMAL_TREND_DROPPING) {
- cur_state = cur_state > instance->lower ?
- (cur_state - 1) : instance->lower;
- }
-
- /* activate a passive thermal instance */
- if ((trip_type == THERMAL_TRIP_PASSIVE ||
- trip_type == THERMAL_TRIPS_NONE) &&
- instance->target == THERMAL_NO_TARGET)
- tz->passive++;
-
- instance->target = cur_state;
- cdev->updated = false; /* cooling device needs update */
- }
- } else { /* below trip */
- list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
- if (instance->trip != trip)
- continue;
-
- /* Do not use the inactive thermal instance */
- if (instance->target == THERMAL_NO_TARGET)
- continue;
- cdev = instance->cdev;
- cdev->ops->get_cur_state(cdev, &cur_state);
-
- cur_state = cur_state > instance->lower ?
- (cur_state - 1) : THERMAL_NO_TARGET;
-
- /* deactivate a passive thermal instance */
- if ((trip_type == THERMAL_TRIP_PASSIVE ||
- trip_type == THERMAL_TRIPS_NONE) &&
- cur_state == THERMAL_NO_TARGET)
- tz->passive--;
- instance->target = cur_state;
- cdev->updated = false; /* cooling device needs update */
- }
- }
-
- return;
-}
-/**
- * thermal_zone_device_update - force an update of a thermal zone's state
- * @ttz: the thermal zone to update
- */
-
-void thermal_zone_device_update(struct thermal_zone_device *tz)
-{
- int count, ret = 0;
- long temp, trip_temp;
- enum thermal_trip_type trip_type;
-
- mutex_lock(&tz->lock);
-
- if (tz->ops->get_temp(tz, &temp)) {
- /* get_temp failed - retry it later */
- pr_warn("failed to read out thermal zone %d\n", tz->id);
- goto leave;
- }
-
- tz->last_temperature = tz->temperature;
- tz->temperature = temp;
-
- for (count = 0; count < tz->trips; count++) {
- tz->ops->get_trip_type(tz, count, &trip_type);
- tz->ops->get_trip_temp(tz, count, &trip_temp);
-
- switch (trip_type) {
- case THERMAL_TRIP_CRITICAL:
- if (temp >= trip_temp) {
- if (tz->ops->notify)
- ret = tz->ops->notify(tz, count,
- trip_type);
- if (!ret) {
- pr_emerg("Critical temperature reached (%ld C), shutting down\n",
- temp/1000);
- orderly_poweroff(true);
- }
- }
- break;
- case THERMAL_TRIP_HOT:
- if (temp >= trip_temp)
- if (tz->ops->notify)
- tz->ops->notify(tz, count, trip_type);
- break;
- case THERMAL_TRIP_ACTIVE:
- thermal_zone_trip_update(tz, count, temp);
- break;
- case THERMAL_TRIP_PASSIVE:
- if (temp >= trip_temp || tz->passive)
- thermal_zone_trip_update(tz, count, temp);
- break;
- }
- }
-
- if (tz->forced_passive)
- thermal_zone_trip_update(tz, THERMAL_TRIPS_NONE, temp);
- thermal_zone_do_update(tz);
-
-leave:
- if (tz->passive)
- thermal_zone_device_set_polling(tz, tz->passive_delay);
- else if (tz->polling_delay)
- thermal_zone_device_set_polling(tz, tz->polling_delay);
- else
- thermal_zone_device_set_polling(tz, 0);
- mutex_unlock(&tz->lock);
-}
-EXPORT_SYMBOL(thermal_zone_device_update);
-
/**
* create_trip_attrs - create attributes for trip points
* @tz: the thermal zone device
--
1.7.9.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv2 12/14] Thermal: Add a notification API
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
` (10 preceding siblings ...)
2012-08-27 4:28 ` [PATCHv2 11/14] Thermal: Remove throttling logic out of thermal_sys.c Durgadoss R
@ 2012-08-27 4:28 ` Durgadoss R
2012-08-27 4:28 ` [PATCHv2 13/14] Thermal: Add documentation for platform layer data Durgadoss R
2012-08-27 4:28 ` [PATCHv2 14/14] Thermal: Platform layer changes to provide thermal data Durgadoss R
13 siblings, 0 replies; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch adds a notification API which the sensor drivers'
can use to notify the framework. The framework then takes
care of the throttling according to the configured policy.
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
drivers/thermal/thermal_sys.c | 18 ++++++++++++++++++
include/linux/thermal.h | 1 +
2 files changed, 19 insertions(+)
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 3f96f3e..0f4cc04 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -1310,6 +1310,24 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev)
EXPORT_SYMBOL(thermal_cdev_update);
/**
+ * notify_thermal_framework - Sensor drivers use this API to notify framework
+ * @tz: thermal zone device
+ * @trip: indicates which trip point has been crossed
+ *
+ * This function handles the trip events from sensor drivers. It starts
+ * throttling the cooling devices according to the policy configured.
+ * For CRITICAL and HOT trip points, this notifies the respective drivers,
+ * and does actual throttling for other trip points i.e ACTIVE and PASSIVE.
+ * The throttling policy is based on the configured platform data; if no
+ * platform data is provided, this uses the step_wise throttling policy.
+ */
+void notify_thermal_framework(struct thermal_zone_device *tz, int trip)
+{
+ handle_thermal_trip(tz, trip);
+}
+EXPORT_SYMBOL(notify_thermal_framework);
+
+/**
* create_trip_attrs - create attributes for trip points
* @tz: the thermal zone device
* @mask: Writeable trip point bitmap.
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index c7e0a4d..273ab0d 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -226,6 +226,7 @@ int get_tz_trend(struct thermal_zone_device *, int);
struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
struct thermal_cooling_device *, int);
void thermal_cdev_update(struct thermal_cooling_device *);
+void notify_thermal_framework(struct thermal_zone_device *, int);
/*
* The platform layer shall define a 'function' that provides the
--
1.7.9.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv2 13/14] Thermal: Add documentation for platform layer data
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
` (11 preceding siblings ...)
2012-08-27 4:28 ` [PATCHv2 12/14] Thermal: Add a notification API Durgadoss R
@ 2012-08-27 4:28 ` Durgadoss R
2012-08-27 4:28 ` [PATCHv2 14/14] Thermal: Platform layer changes to provide thermal data Durgadoss R
13 siblings, 0 replies; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch adds documentation for thermal_bind_params
and thermal_zone_params structures. Also, adds details
on EXPORT_SYMBOL APIs.
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
Documentation/thermal/sysfs-api.txt | 51 +++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index ca1a1a3..650f41b 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -112,6 +112,30 @@ temperature) and throttle appropriate devices.
trip: indicates which trip point the cooling devices is associated with
in this thermal zone.
+1.4 Thermal Zone Parameters
+1.4.1 struct thermal_bind_params
+ This structure defines the following parameters that are used to bind
+ a zone with a cooling device for a particular trip point.
+ .cdev: The cooling device pointer
+ .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.
+1.4.2 struct thermal_zone_params
+ This structure defines the platform level parameters for a thermal zone.
+ This data, for each thermal zone should come from the platform layer.
+ This is an optional feature where some platforms can choose not to
+ provide this data.
+ .throttle_policy: One of the throttling policies from enum
+ thermal_throttle_policy
+ .zone_name: Name of the thermal zone, for which these parameters
+ are being defined.
+ .num_tbps: Number of thermal_bind_params entries for this zone
+ .tbp: thermal_bind_params entries
+
2. sysfs attributes structure
RO read only value
@@ -305,3 +329,30 @@ to a thermal_zone_device when it registers itself with the framework. The
event will be one of:{THERMAL_AUX0, THERMAL_AUX1, THERMAL_CRITICAL,
THERMAL_DEV_FAULT}. Notification can be sent when the current temperature
crosses any of the configured thresholds.
+
+5. Export Symbol APIs:
+
+5.1: get_tz_trend:
+This function returns the trend of a thermal zone, i.e the rate of change
+of temperature of the thermal zone. Ideally, the thermal sensor drivers
+are supposed to implement the callback. If they don't, the thermal
+framework calculated the trend by comparing the previous and the current
+temperature values.
+
+5.2:get_thermal_instance:
+This function returns the thermal_instance corresponding to a given
+{thermal_zone, cooling_device, trip_point} combination. Returns NULL
+if such an instance does not exist.
+
+5.3:notify_thermal_framework:
+This function handles the trip events from sensor drivers. It starts
+throttling the cooling devices according to the policy configured.
+For CRITICAL and HOT trip points, this notifies the respective drivers,
+and does actual throttling for other trip points i.e ACTIVE and PASSIVE.
+The throttling policy is based on the configured platform data; if no
+platform data is provided, this uses the step_wise throttling policy.
+
+5.4:thermal_cdev_update:
+This function serves as an arbitrator to set the state of a cooling
+device. It sets the cooling device to the deepest cooling state if
+possible.
--
1.7.9.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv2 14/14] Thermal: Platform layer changes to provide thermal data
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
` (12 preceding siblings ...)
2012-08-27 4:28 ` [PATCHv2 13/14] Thermal: Add documentation for platform layer data Durgadoss R
@ 2012-08-27 4:28 ` Durgadoss R
13 siblings, 0 replies; 34+ messages in thread
From: Durgadoss R @ 2012-08-27 4:28 UTC (permalink / raw
To: lenb, rui.zhang; +Cc: linux-acpi, eduardo.valentin, Durgadoss R
This patch shows how can we add platform specific thermal data
required by the thermal framework. This is just an example
patch, and _not_ for merge.
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
arch/x86/platform/mrst/mrst.c | 49 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/arch/x86/platform/mrst/mrst.c b/arch/x86/platform/mrst/mrst.c
index fd41a92..91587bf 100644
--- a/arch/x86/platform/mrst/mrst.c
+++ b/arch/x86/platform/mrst/mrst.c
@@ -30,6 +30,7 @@
#include <linux/mfd/intel_msic.h>
#include <linux/gpio.h>
#include <linux/i2c/tc35876x.h>
+#include <linux/thermal.h>
#include <asm/setup.h>
#include <asm/mpspec_def.h>
@@ -78,6 +79,37 @@ struct sfi_rtc_table_entry sfi_mrtc_array[SFI_MRTC_MAX];
EXPORT_SYMBOL_GPL(sfi_mrtc_array);
int sfi_mrtc_num;
+static int thermal_match(struct thermal_zone_device *tz,
+ struct thermal_cooling_device *cdev)
+{
+ if (!strcmp(tz->type, "CPU")) {
+ if (!strcmp(cdev->type, "CPU") ||
+ !strcmp(cdev->type, "Battery"))
+ return 0;
+ }
+ return 1;
+}
+
+static struct thermal_bind_params tz_cpu_params[2] = {
+ {.cdev = NULL,
+ .weight = 80,
+ .trip_mask = 0x0F,
+ .match = thermal_match, },
+
+ { .cdev = NULL,
+ .weight = 80,
+ .trip_mask = 0x0F,
+ .match = thermal_match, },
+ };
+
+#define MRST_THERMAL_ZONES 1
+struct thermal_zone_params tzp[MRST_THERMAL_ZONES] = {
+ { .zone_name = "CPU",
+ .throttle_policy = THERMAL_FAIR_SHARE,
+ .num_tbps = 2,
+ .tbp = tz_cpu_params, }
+};
+
static void mrst_power_off(void)
{
}
@@ -983,10 +1015,27 @@ static int __init sfi_parse_devs(struct sfi_table_header *table)
return 0;
}
+static int mrst_get_thermal_params(struct thermal_zone_device *tz)
+{
+ int i;
+
+ for (i = 0; i < MRST_THERMAL_ZONES; i++) {
+ if (!strcmp(tzp[i].thermal_zone_name, tz->type)) {
+ tz->tzp = &tzp[i];
+ return 0;
+ }
+ }
+ return -ENODEV;
+}
+
static int __init mrst_platform_init(void)
{
sfi_table_parse(SFI_SIG_GPIO, NULL, NULL, sfi_parse_gpio);
sfi_table_parse(SFI_SIG_DEVS, NULL, NULL, sfi_parse_devs);
+
+ /* Set platform thermal data pointer */
+ get_platform_thermal_params = mrst_get_thermal_params;
+
return 0;
}
arch_initcall(mrst_platform_init);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* RE: [PATCHv2 03/14] Thermal: Add get trend, get instance API's to thermal_sys
2012-08-27 4:28 ` [PATCHv2 03/14] Thermal: Add get trend, get instance API's to thermal_sys Durgadoss R
@ 2012-08-27 8:11 ` Zhang, Rui
2012-08-27 8:47 ` R, Durgadoss
0 siblings, 1 reply; 34+ messages in thread
From: Zhang, Rui @ 2012-08-27 8:11 UTC (permalink / raw
To: R, Durgadoss, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
> -----Original Message-----
> From: R, Durgadoss
> Sent: Monday, August 27, 2012 7:28 AM
> To: lenb@kernel.org; Zhang, Rui
> Cc: linux-acpi@vger.kernel.org; eduardo.valentin@ti.com; R, Durgadoss
> Subject: [PATCHv2 03/14] Thermal: Add get trend, get instance API's to
> thermal_sys
> Importance: High
>
> This patch adds the following API's to thermal_sys.c, that can be used
> by other Thermal drivers.
> * get_tz_trend: obtain the trend of the given thermal zone
> * get_thermal_instance: obtain the instance corresponding
> to the given tz, cdev and the trip point.
>
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
> drivers/thermal/thermal_sys.c | 40
> ++++++++++++++++++++++++++++++++++++++++
> include/linux/thermal.h | 4 ++++
> 2 files changed, 44 insertions(+)
>
> diff --git a/drivers/thermal/thermal_sys.c
> b/drivers/thermal/thermal_sys.c index 0e71b00..5e141b5 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -82,6 +82,46 @@ static void release_idr(struct idr *idr, struct
> mutex *lock, int id)
> mutex_unlock(lock);
> }
>
> +int get_tz_trend(struct thermal_zone_device *tz, int trip) {
> + enum thermal_trend trend;
> +
> + if (!tz->ops->get_trend || tz->ops->get_trend(tz, trip, &trend))
> {
> + if (tz->temperature > tz->last_temperature)
> + trend = THERMAL_TREND_RAISING;
> + else if (tz->temperature < tz->last_temperature)
> + trend = THERMAL_TREND_DROPPING;
> + else
> + trend = THERMAL_TREND_STABLE;
> + }
> +
> + return trend;
> +}
> +EXPORT_SYMBOL(get_tz_trend);
> +
> +struct thermal_instance *get_thermal_instance(struct
> thermal_zone_device *tz,
> + struct thermal_cooling_device *cdev, int trip) {
> + struct thermal_instance *pos = NULL;
> + struct thermal_instance *target_instance = NULL;
> +
> + mutex_lock(&tz->lock);
> + mutex_lock(&cdev->lock);
> +
> + list_for_each_entry(pos, &tz->thermal_instances, tz_node) {
> + if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev)
> {
> + target_instance = pos;
> + break;
> + }
> + }
> +
> + mutex_unlock(&cdev->lock);
> + mutex_unlock(&tz->lock);
> +
> + return target_instance;
> +}
> +EXPORT_SYMBOL(get_thermal_instance);
> +
> /* sys I/F for thermal zone */
>
> #define to_thermal_zone(_dev) \
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h index
> 8611e3e..32af124 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -185,6 +185,10 @@ struct thermal_cooling_device
> *thermal_cooling_device_register(char *, void *,
> const struct thermal_cooling_device_ops *); void
> thermal_cooling_device_unregister(struct thermal_cooling_device *);
>
> +int get_tz_trend(struct thermal_zone_device *, int); struct
Coding style.
Thanks,
rui
> +thermal_instance *get_thermal_instance(struct thermal_zone_device *,
> + struct thermal_cooling_device *, int);
> +
> #ifdef CONFIG_NET
> extern int thermal_generate_netlink_event(u32 orig, enum events event);
> #else
> --
> 1.7.9.5
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 04/14] Thermal: Add platform level information to thermal.h
2012-08-27 4:28 ` [PATCHv2 04/14] Thermal: Add platform level information to thermal.h Durgadoss R
@ 2012-08-27 8:27 ` Zhang, Rui
2012-08-27 8:57 ` R, Durgadoss
0 siblings, 1 reply; 34+ messages in thread
From: Zhang, Rui @ 2012-08-27 8:27 UTC (permalink / raw
To: R, Durgadoss, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
> -----Original Message-----
> From: R, Durgadoss
> Sent: Monday, August 27, 2012 7:28 AM
> To: lenb@kernel.org; Zhang, Rui
> Cc: linux-acpi@vger.kernel.org; eduardo.valentin@ti.com; R, Durgadoss
> Subject: [PATCHv2 04/14] Thermal: Add platform level information to
> thermal.h
> Importance: High
>
> This patch creates two structures; one to hold bind bind parameters for
> a thermal zone, and another to store platform layer data for a thermal
> zone, and defines an extern function to retrieve these parameters from
> thermal_sys.c. This patch also defines an enum that describes various
> policies for thermal throttling.
>
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
> drivers/thermal/thermal_sys.c | 3 +++
> include/linux/thermal.h | 46
> +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 49 insertions(+)
>
> diff --git a/drivers/thermal/thermal_sys.c
> b/drivers/thermal/thermal_sys.c index 5e141b5..92a187c 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -51,6 +51,9 @@ static LIST_HEAD(thermal_tz_list); static
> LIST_HEAD(thermal_cdev_list); static DEFINE_MUTEX(thermal_list_lock);
>
> +int (*get_platform_thermal_params)(struct thermal_zone_device *);
> +EXPORT_SYMBOL(get_platform_thermal_params);
> +
If this function is used by the thermal layer, and provided by the platform thermal driver, why not make it mandatory when registering a thermal zone?
Say,
+/* Structure to define Thermal Zone parameters */ struct
+thermal_zone_params {
+ int trips,
+ int mask,
+ struct thermal_zone_device_ops *ops;
+ enum thermal_throttle_policy throttle_policy;
+ int num_tbps; /* Number of tbp entries */
+ struct thermal_bind_params *tbp;
};
And modify thermal_zone_device_register to
Struct thermal_zone_device *thermal_zone_device_register(const char *type, struct thermal_zone_params *params);
The first 3 fields are necessary for registering a zone, the thermal_bind_params can either be filled by platform thermal driver, or be NULL and filled by thermal layer later, when user invokes thermal_zone_bind_cooling_devices.
In this way, we do not need this API at all.
Thanks,
rui
> static int get_idr(struct idr *idr, struct mutex *lock, int *id) {
> int err;
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h index
> 32af124..b644b8e 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -67,6 +67,12 @@ enum thermal_trend {
> THERMAL_TREND_DROPPING, /* temperature is dropping */ };
>
> +enum thermal_throttle_policy {
> + THERMAL_USER_SPACE,
> + THERMAL_FAIR_SHARE,
> + THERMAL_STEP_WISE,
> +};
> +
> /* Events supported by Thermal Netlink */ enum events {
> THERMAL_AUX0,
> @@ -162,6 +168,37 @@ struct thermal_zone_device {
> struct mutex lock; /* protect thermal_instances list */
> struct list_head node;
> struct delayed_work poll_queue;
> + struct thermal_zone_params *tzp;
> +};
> +
> +/* Structure that holds binding parameters for a zone */ struct
> +thermal_bind_params {
> + struct thermal_cooling_device *cdev;
> +
> + /*
> + * This is a measure of 'how effectively these devices can
> + * cool 'this' thermal zone. The shall be determined by platform
> + * characterization. This is on a 'percentage' scale.
> + * See Documentation/thermal/sysfs-api.txt for more information.
> + */
> + int weight;
> +
> + /*
> + * This is a bit mask that gives the binding relation between
> this
> + * thermal zone and cdev, for a particular trip point.
> + * See Documentation/thermal/sysfs-api.txt for more information.
> + */
> + int trip_mask;
> + int (*match) (struct thermal_zone_device *tz,
> + struct thermal_cooling_device *cdev); };
You should start a new line here.
> +
> +/* Structure to define Thermal Zone parameters */ struct
> +thermal_zone_params {
> + const char *zone_name;
What is this zone_name used for?
> + enum thermal_throttle_policy throttle_policy;
> + int num_tbps; /* Number of tbp entries */
> + struct thermal_bind_params *tbp;
> };
>
> struct thermal_genl_event {
> @@ -188,6 +225,15 @@ void thermal_cooling_device_unregister(struct
> thermal_cooling_device *); int get_tz_trend(struct thermal_zone_device
> *, int); struct thermal_instance *get_thermal_instance(struct
> thermal_zone_device *,
> struct thermal_cooling_device *, int);
> +/*
> + * The platform layer shall define a 'function' that provides the
> + * parameters for all thermal zones in the platform. This pointer
> + * should point to that 'function'.
> + *
> + * In thermal_zone_device_register() we update the parameters
> + * for the particular thermal zone.
> + */
> +extern int (*get_platform_thermal_params)(struct thermal_zone_device
> +*);
>
> #ifdef CONFIG_NET
> extern int thermal_generate_netlink_event(u32 orig, enum events event);
> --
> 1.7.9.5
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 05/14] Thermal: Obtain platform data for thermal zone
2012-08-27 4:28 ` [PATCHv2 05/14] Thermal: Obtain platform data for thermal zone Durgadoss R
@ 2012-08-27 8:29 ` Zhang, Rui
2012-08-27 8:59 ` R, Durgadoss
0 siblings, 1 reply; 34+ messages in thread
From: Zhang, Rui @ 2012-08-27 8:29 UTC (permalink / raw
To: R, Durgadoss, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
> -----Original Message-----
> From: R, Durgadoss
> Sent: Monday, August 27, 2012 7:28 AM
> To: lenb@kernel.org; Zhang, Rui
> Cc: linux-acpi@vger.kernel.org; eduardo.valentin@ti.com; R, Durgadoss
> Subject: [PATCHv2 05/14] Thermal: Obtain platform data for thermal zone
> Importance: High
>
> This patch retrieves the platform level data for a zone during its
> registration. It is not an error to not have any platform data.
>
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
> drivers/thermal/thermal_sys.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/thermal/thermal_sys.c
> b/drivers/thermal/thermal_sys.c index 92a187c..6adda39 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -1339,6 +1339,22 @@ static void remove_trip_attrs(struct
> thermal_zone_device *tz)
> kfree(tz->trip_hyst_attrs);
> }
>
> +static int retrieve_zone_params(struct thermal_zone_device *tz) {
> + int ret;
> +
> + /* Check whether the platform data pointer is defined */
> + if (!get_platform_thermal_params)
> + return 0;
> +
> + /* It is not an error to not have any platform data */
> + ret = get_platform_thermal_params(tz);
> + if (ret)
> + tz->tzp = NULL;
> +
> + return 0;
> +}
> +
> /**
> * thermal_zone_device_register - register a new thermal zone device
> * @type: the thermal zone device type
> @@ -1443,6 +1459,11 @@ struct thermal_zone_device
> *thermal_zone_device_register(const char *type,
> if (result)
> goto unregister;
>
> + /* Retrieve platform level parameters for this zone */
> + result = retrieve_zone_params(tz);
> + if (result)
> + goto unregister;
> +
As I said, if we need it, we should ask user to provide it during registration. What do you think?
Thanks,
rui
> mutex_lock(&thermal_list_lock);
> list_add_tail(&tz->node, &thermal_tz_list);
> if (ops->bind)
> --
> 1.7.9.5
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 06/14] Thermal: Add a policy sysfs attribute
2012-08-27 4:28 ` [PATCHv2 06/14] Thermal: Add a policy sysfs attribute Durgadoss R
@ 2012-08-27 8:31 ` Zhang, Rui
2012-08-27 9:02 ` R, Durgadoss
2012-08-27 9:35 ` Zhang, Rui
1 sibling, 1 reply; 34+ messages in thread
From: Zhang, Rui @ 2012-08-27 8:31 UTC (permalink / raw
To: R, Durgadoss, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
> -----Original Message-----
> From: R, Durgadoss
> Sent: Monday, August 27, 2012 7:28 AM
> To: lenb@kernel.org; Zhang, Rui
> Cc: linux-acpi@vger.kernel.org; eduardo.valentin@ti.com; R, Durgadoss
> Subject: [PATCHv2 06/14] Thermal: Add a policy sysfs attribute
> Importance: High
>
> This patch adds a policy sysfs attribute to a thermal zone.
> This attribute will give us the throttling policy used for the zone.
> This is a RO attribute.
>
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
> drivers/thermal/thermal_sys.c | 32 ++++++++++++++++++++++++++++++--
> 1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/thermal_sys.c
> b/drivers/thermal/thermal_sys.c index 6adda39..8aa4200a6 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -378,10 +378,32 @@ passive_show(struct device *dev, struct
> device_attribute *attr,
> return sprintf(buf, "%d\n", tz->forced_passive); }
>
> +static ssize_t
> +policy_show(struct device *dev, struct device_attribute *devattr, char
> +*buf) {
> + struct thermal_zone_device *tz = to_thermal_zone(dev);
> + struct thermal_zone_params *tzp = tz->tzp;
> +
> + if (!tzp)
> + return sprintf(buf, "step_wise(default)\n");
> +
> + switch (tzp->throttle_policy) {
> + case THERMAL_FAIR_SHARE:
> + return sprintf(buf, "fair_share\n");
> + case THERMAL_STEP_WISE:
> + return sprintf(buf, "step_wise\n");
> + case THERMAL_USER_SPACE:
> + return sprintf(buf, "user_space\n");
> + default:
> + return sprintf(buf, "unknown\n");
> + }
> +}
> +
> static DEVICE_ATTR(type, 0444, type_show, NULL); static
> DEVICE_ATTR(temp, 0444, temp_show, NULL); static DEVICE_ATTR(mode,
> 0644, mode_show, mode_store); static DEVICE_ATTR(passive, S_IRUGO |
> S_IWUSR, passive_show, passive_store);
> +static DEVICE_ATTR(throttle_policy, S_IRUGO, policy_show, NULL);
>
> /* sys I/F for cooling device */
> #define to_cooling_device(_dev) \
> @@ -1349,10 +1371,16 @@ static int retrieve_zone_params(struct
> thermal_zone_device *tz)
>
> /* It is not an error to not have any platform data */
> ret = get_platform_thermal_params(tz);
> - if (ret)
> + if (ret) {
> tz->tzp = NULL;
> + return 0;
> + }
>
> - return 0;
> + ret = device_create_file(&tz->device, &dev_attr_throttle_policy);
> + if (ret)
> + dev_err(&tz->device, "creating policy attr failed:%d\n",
> ret);
> +
> + return ret;
> }
What does this mean?
We will not create "policy" attributes if there is no thermal_zone_params?
What if platform thermal drivers chooses to provide .bind() and invoke thermal_zone_bind_cooling_device manually?
Thanks,
rui
>
> /**
> --
> 1.7.9.5
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 03/14] Thermal: Add get trend, get instance API's to thermal_sys
2012-08-27 8:11 ` Zhang, Rui
@ 2012-08-27 8:47 ` R, Durgadoss
2012-08-27 9:19 ` Zhang, Rui
0 siblings, 1 reply; 34+ messages in thread
From: R, Durgadoss @ 2012-08-27 8:47 UTC (permalink / raw
To: Zhang, Rui, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
Hi Rui,
> >
> > +int get_tz_trend(struct thermal_zone_device *tz, int trip) {
> > + enum thermal_trend trend;
> > +
> > + if (!tz->ops->get_trend || tz->ops->get_trend(tz, trip, &trend))
> > {
> > + if (tz->temperature > tz->last_temperature)
> > + trend = THERMAL_TREND_RAISING;
> > + else if (tz->temperature < tz->last_temperature)
> > + trend = THERMAL_TREND_DROPPING;
> > + else
> > + trend = THERMAL_TREND_STABLE;
> > + }
> > +
> > + return trend;
> > +}
> > +EXPORT_SYMBOL(get_tz_trend);
> > +
> > +struct thermal_instance *get_thermal_instance(struct
> > thermal_zone_device *tz,
> > + struct thermal_cooling_device *cdev, int trip) {
> > + struct thermal_instance *pos = NULL;
> > + struct thermal_instance *target_instance = NULL;
> > +
> > + mutex_lock(&tz->lock);
> > + mutex_lock(&cdev->lock);
> > +
> > + list_for_each_entry(pos, &tz->thermal_instances, tz_node) {
> > + if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev)
> > {
> > + target_instance = pos;
> > + break;
> > + }
> > + }
> > +
> > + mutex_unlock(&cdev->lock);
> > + mutex_unlock(&tz->lock);
> > +
> > + return target_instance;
> > +}
> > +EXPORT_SYMBOL(get_thermal_instance);
> > +
> > /* sys I/F for thermal zone */
> >
> > #define to_thermal_zone(_dev) \
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h index
> > 8611e3e..32af124 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -185,6 +185,10 @@ struct thermal_cooling_device
> > *thermal_cooling_device_register(char *, void *,
> > const struct thermal_cooling_device_ops *); void
> > thermal_cooling_device_unregister(struct thermal_cooling_device *);
> >
> > +int get_tz_trend(struct thermal_zone_device *, int); struct
>
> Coding style.
Not sure what you meant here. Checkpatch did not complain either.
Thanks,
Durga
>
> Thanks,
> rui
> > +thermal_instance *get_thermal_instance(struct thermal_zone_device *,
> > + struct thermal_cooling_device *, int);
> > +
> > #ifdef CONFIG_NET
> > extern int thermal_generate_netlink_event(u32 orig, enum events
> event);
> > #else
> > --
> > 1.7.9.5
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 04/14] Thermal: Add platform level information to thermal.h
2012-08-27 8:27 ` Zhang, Rui
@ 2012-08-27 8:57 ` R, Durgadoss
2012-08-27 9:22 ` Zhang, Rui
0 siblings, 1 reply; 34+ messages in thread
From: R, Durgadoss @ 2012-08-27 8:57 UTC (permalink / raw
To: Zhang, Rui, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
Hi Rui,
[cut.]
> > +int (*get_platform_thermal_params)(struct thermal_zone_device *);
> > +EXPORT_SYMBOL(get_platform_thermal_params);
> > +
> If this function is used by the thermal layer, and provided by the platform
> thermal driver, why not make it mandatory when registering a thermal zone?
>
> Say,
>
> +/* Structure to define Thermal Zone parameters */ struct
> +thermal_zone_params {
> + int trips,
> + int mask,
> + struct thermal_zone_device_ops *ops;
> + enum thermal_throttle_policy throttle_policy;
> + int num_tbps; /* Number of tbp entries */
> + struct thermal_bind_params *tbp;
> };
> And modify thermal_zone_device_register to
> Struct thermal_zone_device *thermal_zone_device_register(const char
> *type, struct thermal_zone_params *params);
>
> The first 3 fields are necessary for registering a zone, the
> thermal_bind_params can either be filled by platform thermal driver, or be
> NULL and filled by thermal layer later, when user invokes
> thermal_zone_bind_cooling_devices.
>
> In this way, we do not need this API at all.
We can do it either ways. In this case, we need to modify all the tzd_register
calls. If we are Ok doing that, I am happy to change.
Just that we are adding one more to the already existing 7 args :-)
> > static int get_idr(struct idr *idr, struct mutex *lock, int *id) {
> > int err;
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h index
> > 32af124..b644b8e 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -67,6 +67,12 @@ enum thermal_trend {
> > THERMAL_TREND_DROPPING, /* temperature is dropping */ };
> >
> > +enum thermal_throttle_policy {
> > + THERMAL_USER_SPACE,
> > + THERMAL_FAIR_SHARE,
> > + THERMAL_STEP_WISE,
> > +};
> > +
> > /* Events supported by Thermal Netlink */ enum events {
> > THERMAL_AUX0,
> > @@ -162,6 +168,37 @@ struct thermal_zone_device {
> > struct mutex lock; /* protect thermal_instances list */
> > struct list_head node;
> > struct delayed_work poll_queue;
> > + struct thermal_zone_params *tzp;
> > +};
> > +
> > +/* Structure that holds binding parameters for a zone */ struct
> > +thermal_bind_params {
> > + struct thermal_cooling_device *cdev;
> > +
> > + /*
> > + * This is a measure of 'how effectively these devices can
> > + * cool 'this' thermal zone. The shall be determined by platform
> > + * characterization. This is on a 'percentage' scale.
> > + * See Documentation/thermal/sysfs-api.txt for more information.
> > + */
> > + int weight;
> > +
> > + /*
> > + * This is a bit mask that gives the binding relation between
> > this
> > + * thermal zone and cdev, for a particular trip point.
> > + * See Documentation/thermal/sysfs-api.txt for more information.
> > + */
> > + int trip_mask;
> > + int (*match) (struct thermal_zone_device *tz,
> > + struct thermal_cooling_device *cdev); };
>
> You should start a new line here.
Again, not sure what you meant here. The new line is already there.
> > +/* Structure to define Thermal Zone parameters */ struct
> > +thermal_zone_params {
> > + const char *zone_name;
>
>
> What is this zone_name used for?
This is required when we retrieve platform data from framework layer.
Now, that we make it as an argument in tzd_register, we don't need this.
Thanks,
Durga
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 05/14] Thermal: Obtain platform data for thermal zone
2012-08-27 8:29 ` Zhang, Rui
@ 2012-08-27 8:59 ` R, Durgadoss
0 siblings, 0 replies; 34+ messages in thread
From: R, Durgadoss @ 2012-08-27 8:59 UTC (permalink / raw
To: Zhang, Rui, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
Hi Rui,
> > /**
> > * thermal_zone_device_register - register a new thermal zone device
> > * @type: the thermal zone device type
> > @@ -1443,6 +1459,11 @@ struct thermal_zone_device
> > *thermal_zone_device_register(const char *type,
> > if (result)
> > goto unregister;
> >
> > + /* Retrieve platform level parameters for this zone */
> > + result = retrieve_zone_params(tz);
> > + if (result)
> > + goto unregister;
> > +
>
> As I said, if we need it, we should ask user to provide it during registration.
> What do you think?
That's Fine, we can provide it through tzd_register function call.
Thanks,
Durga
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 06/14] Thermal: Add a policy sysfs attribute
2012-08-27 8:31 ` Zhang, Rui
@ 2012-08-27 9:02 ` R, Durgadoss
2012-08-27 9:25 ` Zhang, Rui
0 siblings, 1 reply; 34+ messages in thread
From: R, Durgadoss @ 2012-08-27 9:02 UTC (permalink / raw
To: Zhang, Rui, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
Hi Rui,
> > +static ssize_t
> > +policy_show(struct device *dev, struct device_attribute *devattr, char
> > +*buf) {
> > + struct thermal_zone_device *tz = to_thermal_zone(dev);
> > + struct thermal_zone_params *tzp = tz->tzp;
> > +
> > + if (!tzp)
> > + return sprintf(buf, "step_wise(default)\n");
> > +
> > + switch (tzp->throttle_policy) {
> > + case THERMAL_FAIR_SHARE:
> > + return sprintf(buf, "fair_share\n");
> > + case THERMAL_STEP_WISE:
> > + return sprintf(buf, "step_wise\n");
> > + case THERMAL_USER_SPACE:
> > + return sprintf(buf, "user_space\n");
> > + default:
> > + return sprintf(buf, "unknown\n");
> > + }
> > +}
> > +
> > static DEVICE_ATTR(type, 0444, type_show, NULL); static
> > DEVICE_ATTR(temp, 0444, temp_show, NULL); static DEVICE_ATTR(mode,
> > 0644, mode_show, mode_store); static DEVICE_ATTR(passive, S_IRUGO |
> > S_IWUSR, passive_show, passive_store);
> > +static DEVICE_ATTR(throttle_policy, S_IRUGO, policy_show, NULL);
> >
> > /* sys I/F for cooling device */
> > #define to_cooling_device(_dev) \
> > @@ -1349,10 +1371,16 @@ static int retrieve_zone_params(struct
> > thermal_zone_device *tz)
> >
> > /* It is not an error to not have any platform data */
> > ret = get_platform_thermal_params(tz);
> > - if (ret)
> > + if (ret) {
> > tz->tzp = NULL;
> > + return 0;
> > + }
> >
> > - return 0;
> > + ret = device_create_file(&tz->device, &dev_attr_throttle_policy);
> > + if (ret)
> > + dev_err(&tz->device, "creating policy attr failed:%d\n",
> > ret);
> > +
> > + return ret;
> > }
>
> What does this mean?
> We will not create "policy" attributes if there is no thermal_zone_params?
Yes, that's what I thought initially. Because if there is no 'throttle_policy'
we assume that it is (by default) step_wise.
But, if we make tz_params be provided through tzd_register function call,
it makes sense for this to be a mandatory attribute, showing 'step_wise"
if there is no thermal_zone_params.
This needs a clean fix, will make it in v3.
Thanks,
Durga
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 03/14] Thermal: Add get trend, get instance API's to thermal_sys
2012-08-27 8:47 ` R, Durgadoss
@ 2012-08-27 9:19 ` Zhang, Rui
2012-08-27 9:24 ` R, Durgadoss
0 siblings, 1 reply; 34+ messages in thread
From: Zhang, Rui @ 2012-08-27 9:19 UTC (permalink / raw
To: R, Durgadoss, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
> -----Original Message-----
> From: R, Durgadoss
> Sent: Monday, August 27, 2012 11:47 AM
> To: Zhang, Rui; lenb@kernel.org
> Cc: linux-acpi@vger.kernel.org; eduardo.valentin@ti.com
> Subject: RE: [PATCHv2 03/14] Thermal: Add get trend, get instance API's
> to thermal_sys
> Importance: High
>
> Hi Rui,
>
> > >
> > > +int get_tz_trend(struct thermal_zone_device *tz, int trip) {
> > > + enum thermal_trend trend;
> > > +
> > > + if (!tz->ops->get_trend || tz->ops->get_trend(tz, trip, &trend))
> > > {
> > > + if (tz->temperature > tz->last_temperature)
> > > + trend = THERMAL_TREND_RAISING;
> > > + else if (tz->temperature < tz->last_temperature)
> > > + trend = THERMAL_TREND_DROPPING;
> > > + else
> > > + trend = THERMAL_TREND_STABLE;
> > > + }
> > > +
> > > + return trend;
> > > +}
> > > +EXPORT_SYMBOL(get_tz_trend);
> > > +
> > > +struct thermal_instance *get_thermal_instance(struct
> > > thermal_zone_device *tz,
> > > + struct thermal_cooling_device *cdev, int trip) {
> > > + struct thermal_instance *pos = NULL;
> > > + struct thermal_instance *target_instance = NULL;
> > > +
> > > + mutex_lock(&tz->lock);
> > > + mutex_lock(&cdev->lock);
> > > +
> > > + list_for_each_entry(pos, &tz->thermal_instances, tz_node) {
> > > + if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev)
> > > {
> > > + target_instance = pos;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + mutex_unlock(&cdev->lock);
> > > + mutex_unlock(&tz->lock);
> > > +
> > > + return target_instance;
> > > +}
> > > +EXPORT_SYMBOL(get_thermal_instance);
> > > +
> > > /* sys I/F for thermal zone */
> > >
> > > #define to_thermal_zone(_dev) \
> > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index
> > > 8611e3e..32af124 100644
> > > --- a/include/linux/thermal.h
> > > +++ b/include/linux/thermal.h
> > > @@ -185,6 +185,10 @@ struct thermal_cooling_device
> > > *thermal_cooling_device_register(char *, void *,
> > > const struct thermal_cooling_device_ops *); void
> > > thermal_cooling_device_unregister(struct thermal_cooling_device *);
> > >
> > > +int get_tz_trend(struct thermal_zone_device *, int); struct
> >
> > Coding style.
>
> Not sure what you meant here. Checkpatch did not complain either.
>
you should start a new line before "struct".
Thanks,
rui
> Thanks,
> Durga
>
> >
> > Thanks,
> > rui
> > > +thermal_instance *get_thermal_instance(struct thermal_zone_device
> *,
> > > + struct thermal_cooling_device *, int);
> > > +
> > > #ifdef CONFIG_NET
> > > extern int thermal_generate_netlink_event(u32 orig, enum events
> > event);
> > > #else
> > > --
> > > 1.7.9.5
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 04/14] Thermal: Add platform level information to thermal.h
2012-08-27 8:57 ` R, Durgadoss
@ 2012-08-27 9:22 ` Zhang, Rui
2012-08-27 9:25 ` R, Durgadoss
0 siblings, 1 reply; 34+ messages in thread
From: Zhang, Rui @ 2012-08-27 9:22 UTC (permalink / raw
To: R, Durgadoss, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
> -----Original Message-----
> From: R, Durgadoss
> Sent: Monday, August 27, 2012 11:57 AM
> To: Zhang, Rui; lenb@kernel.org
> Cc: linux-acpi@vger.kernel.org; eduardo.valentin@ti.com
> Subject: RE: [PATCHv2 04/14] Thermal: Add platform level information to
> thermal.h
> Importance: High
>
> Hi Rui,
>
> [cut.]
>
> > > +int (*get_platform_thermal_params)(struct thermal_zone_device *);
> > > +EXPORT_SYMBOL(get_platform_thermal_params);
> > > +
> > If this function is used by the thermal layer, and provided by the
> > platform thermal driver, why not make it mandatory when registering a
> thermal zone?
> >
> > Say,
> >
> > +/* Structure to define Thermal Zone parameters */ struct
> > +thermal_zone_params {
> > + int trips,
> > + int mask,
> > + struct thermal_zone_device_ops *ops;
> > + enum thermal_throttle_policy throttle_policy;
> > + int num_tbps; /* Number of tbp entries */
> > + struct thermal_bind_params *tbp;
> > };
> > And modify thermal_zone_device_register to Struct thermal_zone_device
> > *thermal_zone_device_register(const char *type, struct
> > thermal_zone_params *params);
> >
> > The first 3 fields are necessary for registering a zone, the
> > thermal_bind_params can either be filled by platform thermal driver,
> > or be NULL and filled by thermal layer later, when user invokes
> > thermal_zone_bind_cooling_devices.
> >
> > In this way, we do not need this API at all.
>
> We can do it either ways. In this case, we need to modify all the
> tzd_register calls. If we are Ok doing that, I am happy to change.
>
> Just that we are adding one more to the already existing 7 args :-)
No, we are trying to reducing the args by moving them into thermal_zone_params.
>
> > > static int get_idr(struct idr *idr, struct mutex *lock, int *id)
> {
> > > int err;
> > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index
> > > 32af124..b644b8e 100644
> > > --- a/include/linux/thermal.h
> > > +++ b/include/linux/thermal.h
> > > @@ -67,6 +67,12 @@ enum thermal_trend {
> > > THERMAL_TREND_DROPPING, /* temperature is dropping */ };
> > >
> > > +enum thermal_throttle_policy {
> > > + THERMAL_USER_SPACE,
> > > + THERMAL_FAIR_SHARE,
> > > + THERMAL_STEP_WISE,
> > > +};
> > > +
> > > /* Events supported by Thermal Netlink */ enum events {
> > > THERMAL_AUX0,
> > > @@ -162,6 +168,37 @@ struct thermal_zone_device {
> > > struct mutex lock; /* protect thermal_instances list */
> > > struct list_head node;
> > > struct delayed_work poll_queue;
> > > + struct thermal_zone_params *tzp;
> > > +};
> > > +
> > > +/* Structure that holds binding parameters for a zone */ struct
> > > +thermal_bind_params {
> > > + struct thermal_cooling_device *cdev;
> > > +
> > > + /*
> > > + * This is a measure of 'how effectively these devices can
> > > + * cool 'this' thermal zone. The shall be determined by platform
> > > + * characterization. This is on a 'percentage' scale.
> > > + * See Documentation/thermal/sysfs-api.txt for more information.
> > > + */
> > > + int weight;
> > > +
> > > + /*
> > > + * This is a bit mask that gives the binding relation between
> > > this
> > > + * thermal zone and cdev, for a particular trip point.
> > > + * See Documentation/thermal/sysfs-api.txt for more information.
> > > + */
> > > + int trip_mask;
> > > + int (*match) (struct thermal_zone_device *tz,
> > > + struct thermal_cooling_device *cdev); };
> >
> > You should start a new line here.
>
> Again, not sure what you meant here. The new line is already there.
>
struct thermal_cooling_device *cdev);
};
I'm not sure what it is in your original patch, but I see something like
"struct thermal_cooling_device *cdev); };"
May be this is because I'm using outlook?
thanks,
rui
> > > +/* Structure to define Thermal Zone parameters */ struct
> > > +thermal_zone_params {
> > > + const char *zone_name;
> >
> >
> > What is this zone_name used for?
>
> This is required when we retrieve platform data from framework layer.
> Now, that we make it as an argument in tzd_register, we don't need this.
>
> Thanks,
> Durga
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 03/14] Thermal: Add get trend, get instance API's to thermal_sys
2012-08-27 9:19 ` Zhang, Rui
@ 2012-08-27 9:24 ` R, Durgadoss
0 siblings, 0 replies; 34+ messages in thread
From: R, Durgadoss @ 2012-08-27 9:24 UTC (permalink / raw
To: Zhang, Rui, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
Hi Rui,
> > index
> > > > 8611e3e..32af124 100644
> > > > --- a/include/linux/thermal.h
> > > > +++ b/include/linux/thermal.h
> > > > @@ -185,6 +185,10 @@ struct thermal_cooling_device
> > > > *thermal_cooling_device_register(char *, void *,
> > > > const struct thermal_cooling_device_ops *); void
> > > > thermal_cooling_device_unregister(struct thermal_cooling_device *);
> > > >
> > > > +int get_tz_trend(struct thermal_zone_device *, int); struct
> > >
> > > Coding style.
> >
> > Not sure what you meant here. Checkpatch did not complain either.
> >
> you should start a new line before "struct".
Agreed :-) But in the actual patch it is that way,
I think it is your mailer which is playing things here :-)
Kindly check..
Thanks,
Durga
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 06/14] Thermal: Add a policy sysfs attribute
2012-08-27 9:02 ` R, Durgadoss
@ 2012-08-27 9:25 ` Zhang, Rui
2012-08-27 10:23 ` R, Durgadoss
0 siblings, 1 reply; 34+ messages in thread
From: Zhang, Rui @ 2012-08-27 9:25 UTC (permalink / raw
To: R, Durgadoss, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
> -----Original Message-----
> From: R, Durgadoss
> Sent: Monday, August 27, 2012 12:02 PM
> To: Zhang, Rui; lenb@kernel.org
> Cc: linux-acpi@vger.kernel.org; eduardo.valentin@ti.com
> Subject: RE: [PATCHv2 06/14] Thermal: Add a policy sysfs attribute
> Importance: High
>
> Hi Rui,
>
> > > +static ssize_t
> > > +policy_show(struct device *dev, struct device_attribute *devattr,
> > > +char
> > > +*buf) {
> > > + struct thermal_zone_device *tz = to_thermal_zone(dev);
> > > + struct thermal_zone_params *tzp = tz->tzp;
> > > +
> > > + if (!tzp)
> > > + return sprintf(buf, "step_wise(default)\n");
> > > +
> > > + switch (tzp->throttle_policy) {
> > > + case THERMAL_FAIR_SHARE:
> > > + return sprintf(buf, "fair_share\n");
> > > + case THERMAL_STEP_WISE:
> > > + return sprintf(buf, "step_wise\n");
> > > + case THERMAL_USER_SPACE:
> > > + return sprintf(buf, "user_space\n");
> > > + default:
> > > + return sprintf(buf, "unknown\n");
> > > + }
> > > +}
> > > +
> > > static DEVICE_ATTR(type, 0444, type_show, NULL); static
> > > DEVICE_ATTR(temp, 0444, temp_show, NULL); static DEVICE_ATTR(mode,
> > > 0644, mode_show, mode_store); static DEVICE_ATTR(passive, S_IRUGO
> |
> > > S_IWUSR, passive_show, passive_store);
> > > +static DEVICE_ATTR(throttle_policy, S_IRUGO, policy_show, NULL);
> > >
> > > /* sys I/F for cooling device */
> > > #define to_cooling_device(_dev) \
> > > @@ -1349,10 +1371,16 @@ static int retrieve_zone_params(struct
> > > thermal_zone_device *tz)
> > >
> > > /* It is not an error to not have any platform data */
> > > ret = get_platform_thermal_params(tz);
> > > - if (ret)
> > > + if (ret) {
> > > tz->tzp = NULL;
> > > + return 0;
> > > + }
> > >
> > > - return 0;
> > > + ret = device_create_file(&tz->device, &dev_attr_throttle_policy);
> > > + if (ret)
> > > + dev_err(&tz->device, "creating policy attr failed:%d\n",
> > > ret);
> > > +
> > > + return ret;
> > > }
> >
> > What does this mean?
> > We will not create "policy" attributes if there is no
> thermal_zone_params?
>
> Yes, that's what I thought initially. Because if there is no
> 'throttle_policy'
> we assume that it is (by default) step_wise.
>
> But, if we make tz_params be provided through tzd_register function
> call, it makes sense for this to be a mandatory attribute, showing
> 'step_wise"
> if there is no thermal_zone_params.
>
IMO, every thermal zone should have a policy. And they can be changed anytime if user wants to.
Thanks,
rui
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 04/14] Thermal: Add platform level information to thermal.h
2012-08-27 9:22 ` Zhang, Rui
@ 2012-08-27 9:25 ` R, Durgadoss
0 siblings, 0 replies; 34+ messages in thread
From: R, Durgadoss @ 2012-08-27 9:25 UTC (permalink / raw
To: Zhang, Rui, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
> > > And modify thermal_zone_device_register to Struct
> thermal_zone_device
> > > *thermal_zone_device_register(const char *type, struct
> > > thermal_zone_params *params);
> > >
> > > The first 3 fields are necessary for registering a zone, the
> > > thermal_bind_params can either be filled by platform thermal driver,
> > > or be NULL and filled by thermal layer later, when user invokes
> > > thermal_zone_bind_cooling_devices.
> > >
> > > In this way, we do not need this API at all.
> >
> > We can do it either ways. In this case, we need to modify all the
> > tzd_register calls. If we are Ok doing that, I am happy to change.
> >
> > Just that we are adding one more to the already existing 7 args :-)
>
> No, we are trying to reducing the args by moving them into
> thermal_zone_params.
>
> >
> > > > static int get_idr(struct idr *idr, struct mutex *lock, int *id)
> > {
> > > > int err;
> > > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index
> > > > 32af124..b644b8e 100644
> > > > --- a/include/linux/thermal.h
> > > > +++ b/include/linux/thermal.h
> > > > @@ -67,6 +67,12 @@ enum thermal_trend {
> > > > THERMAL_TREND_DROPPING, /* temperature is dropping */ };
> > > >
> > > > +enum thermal_throttle_policy {
> > > > + THERMAL_USER_SPACE,
> > > > + THERMAL_FAIR_SHARE,
> > > > + THERMAL_STEP_WISE,
> > > > +};
> > > > +
> > > > /* Events supported by Thermal Netlink */ enum events {
> > > > THERMAL_AUX0,
> > > > @@ -162,6 +168,37 @@ struct thermal_zone_device {
> > > > struct mutex lock; /* protect thermal_instances list */
> > > > struct list_head node;
> > > > struct delayed_work poll_queue;
> > > > + struct thermal_zone_params *tzp;
> > > > +};
> > > > +
> > > > +/* Structure that holds binding parameters for a zone */ struct
> > > > +thermal_bind_params {
> > > > + struct thermal_cooling_device *cdev;
> > > > +
> > > > + /*
> > > > + * This is a measure of 'how effectively these devices can
> > > > + * cool 'this' thermal zone. The shall be determined by platform
> > > > + * characterization. This is on a 'percentage' scale.
> > > > + * See Documentation/thermal/sysfs-api.txt for more information.
> > > > + */
> > > > + int weight;
> > > > +
> > > > + /*
> > > > + * This is a bit mask that gives the binding relation between
> > > > this
> > > > + * thermal zone and cdev, for a particular trip point.
> > > > + * See Documentation/thermal/sysfs-api.txt for more information.
> > > > + */
> > > > + int trip_mask;
> > > > + int (*match) (struct thermal_zone_device *tz,
> > > > + struct thermal_cooling_device *cdev); };
> > >
> > > You should start a new line here.
> >
> > Again, not sure what you meant here. The new line is already there.
> >
> struct thermal_cooling_device *cdev);
> };
>
> I'm not sure what it is in your original patch, but I see something like
> "struct thermal_cooling_device *cdev); };"
> May be this is because I'm using outlook?
Looks like it is !!
Thanks,
Durga
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 06/14] Thermal: Add a policy sysfs attribute
2012-08-27 4:28 ` [PATCHv2 06/14] Thermal: Add a policy sysfs attribute Durgadoss R
2012-08-27 8:31 ` Zhang, Rui
@ 2012-08-27 9:35 ` Zhang, Rui
2012-08-27 10:25 ` R, Durgadoss
1 sibling, 1 reply; 34+ messages in thread
From: Zhang, Rui @ 2012-08-27 9:35 UTC (permalink / raw
To: R, Durgadoss, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
> -----Original Message-----
> From: R, Durgadoss
> Sent: Monday, August 27, 2012 7:28 AM
> To: lenb@kernel.org; Zhang, Rui
> Cc: linux-acpi@vger.kernel.org; eduardo.valentin@ti.com; R, Durgadoss
> Subject: [PATCHv2 06/14] Thermal: Add a policy sysfs attribute
> Importance: High
>
> This patch adds a policy sysfs attribute to a thermal zone.
> This attribute will give us the throttling policy used for the zone.
> This is a RO attribute.
>
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
> drivers/thermal/thermal_sys.c | 32 ++++++++++++++++++++++++++++++--
> 1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/thermal_sys.c
> b/drivers/thermal/thermal_sys.c index 6adda39..8aa4200a6 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -378,10 +378,32 @@ passive_show(struct device *dev, struct
> device_attribute *attr,
> return sprintf(buf, "%d\n", tz->forced_passive); }
>
> +static ssize_t
> +policy_show(struct device *dev, struct device_attribute *devattr, char
> +*buf) {
> + struct thermal_zone_device *tz = to_thermal_zone(dev);
> + struct thermal_zone_params *tzp = tz->tzp;
> +
> + if (!tzp)
> + return sprintf(buf, "step_wise(default)\n");
> +
> + switch (tzp->throttle_policy) {
> + case THERMAL_FAIR_SHARE:
> + return sprintf(buf, "fair_share\n");
> + case THERMAL_STEP_WISE:
> + return sprintf(buf, "step_wise\n");
> + case THERMAL_USER_SPACE:
> + return sprintf(buf, "user_space\n");
> + default:
> + return sprintf(buf, "unknown\n");
> + }
> +}
> +
> static DEVICE_ATTR(type, 0444, type_show, NULL); static
> DEVICE_ATTR(temp, 0444, temp_show, NULL); static DEVICE_ATTR(mode,
> 0644, mode_show, mode_store); static DEVICE_ATTR(passive, S_IRUGO |
> S_IWUSR, passive_show, passive_store);
> +static DEVICE_ATTR(throttle_policy, S_IRUGO, policy_show, NULL);
>
> /* sys I/F for cooling device */
> #define to_cooling_device(_dev) \
> @@ -1349,10 +1371,16 @@ static int retrieve_zone_params(struct
> thermal_zone_device *tz)
>
> /* It is not an error to not have any platform data */
> ret = get_platform_thermal_params(tz);
> - if (ret)
> + if (ret) {
> tz->tzp = NULL;
> + return 0;
> + }
>
> - return 0;
> + ret = device_create_file(&tz->device, &dev_attr_throttle_policy);
> + if (ret)
> + dev_err(&tz->device, "creating policy attr failed:%d\n",
> ret);
> +
> + return ret;
> }
>
> /**
We should remove this attribute in thermal_zone_device_unregister();
Thanks.
rui
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 07/14] Thermal: Update binding logic based on platform data
2012-08-27 4:28 ` [PATCHv2 07/14] Thermal: Update binding logic based on platform data Durgadoss R
@ 2012-08-27 9:50 ` Zhang, Rui
2012-08-27 10:29 ` R, Durgadoss
0 siblings, 1 reply; 34+ messages in thread
From: Zhang, Rui @ 2012-08-27 9:50 UTC (permalink / raw
To: R, Durgadoss, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
> -----Original Message-----
> From: R, Durgadoss
> Sent: Monday, August 27, 2012 7:28 AM
> To: lenb@kernel.org; Zhang, Rui
> Cc: linux-acpi@vger.kernel.org; eduardo.valentin@ti.com; R, Durgadoss
> Subject: [PATCHv2 07/14] Thermal: Update binding logic based on
> platform data
> Importance: High
>
> This patch updates the binding logic in thermal_sys.c It uses the
> platform layer data to bind a thermal zone to a cdev for a particular
> trip point.
>
> * If we do not have platform data and do not have
> .bind defined, do not bind.
> * If we do not have platform data but .bind is
> defined, then use tz->ops->bind.
> * If we have platform data, use it to create binding.
>
> The same logic sequence is followed for unbind also.
>
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
> drivers/thermal/thermal_sys.c | 184
> +++++++++++++++++++++++++++++++++++------
> 1 file changed, 158 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/thermal/thermal_sys.c
> b/drivers/thermal/thermal_sys.c index 8aa4200a6..5d38501 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -125,6 +125,111 @@ struct thermal_instance
> *get_thermal_instance(struct thermal_zone_device *tz, }
> EXPORT_SYMBOL(get_thermal_instance);
>
> +static void print_bind_err_msg(struct thermal_zone_device *tz,
> + struct thermal_cooling_device *cdev, int ret) {
> + dev_err(&tz->device, "binding zone %s with cdev %s failed:%d\n",
> + tz->type, cdev->type, ret);
> +}
> +
> +static void __bind(struct thermal_zone_device *tz, int mask,
> + struct thermal_cooling_device *cdev) {
> + int i, ret;
> +
> + for (i = 0; i < tz->trips; i++) {
> + if (mask & (1 << i)) {
> + ret = thermal_zone_bind_cooling_device(tz, i, cdev,
> + THERMAL_NO_LIMIT, THERMAL_NO_LIMIT);
> + if (ret)
> + print_bind_err_msg(tz, cdev, ret);
> + }
> + }
> +}
> +
> +static void __unbind(struct thermal_zone_device *tz, int mask,
> + struct thermal_cooling_device *cdev) {
> + int i;
> +
> + for (i = 0; i < tz->trips; i++)
> + if (mask & (1 << i))
> + thermal_zone_unbind_cooling_device(tz, i, cdev); }
> +
> +static void update_bind_info(struct thermal_cooling_device *cdev) {
I do not think this function name is accurate.
Update_bind_info is called when a new cooling device comes
and do_binding is called when a new zone is registered, right?
I think we should reflect this in the function name.
Maybe something like this:
Int do_bind(tz, cdev) {
If (tz->ops->bind)
Tz->ops->bind(tz, cdev);
Else {
Blabla
...
}
}
Bind_cdev(cdev) {
List_for_each_entry(tz, thermal_tz_list) {
Do_bind(tz, cdev);
}
}
Bind_tz(tz) {
List_for_each_entry(cdev, thermal_cdev_list) {
Do_bind(tz, cdev);
}
}
Thanks,
rui
> + int i, ret;
> + struct thermal_zone_params *tzp;
> + struct thermal_zone_device *pos = NULL;
> +
> + mutex_lock(&thermal_list_lock);
> +
> + list_for_each_entry(pos, &thermal_tz_list, node) {
> + if (!pos->tzp && !pos->ops->bind)
> + continue;
> +
> + if (!pos->tzp && pos->ops->bind) {
> + ret = pos->ops->bind(pos, cdev);
> + if (ret)
> + print_bind_err_msg(pos, cdev, ret);
> + }
> +
> + tzp = pos->tzp;
> + if (!tzp->tbp)
> + return;
> +
> + for (i = 0; i < tzp->num_tbps; i++) {
> + if (tzp->tbp[i].cdev || !tzp->tbp[i].match)
> + continue;
> + if (tzp->tbp[i].match(pos, cdev))
> + continue;
> + tzp->tbp[i].cdev = cdev;
> + __bind(pos, tzp->tbp[i].trip_mask, cdev);
> + }
> + }
> +
> + mutex_unlock(&thermal_list_lock);
> +}
> +
> +static void do_binding(struct thermal_zone_device *tz) {
> + int i, ret;
> + struct thermal_cooling_device *pos = NULL;
> + struct thermal_zone_params *tzp = tz->tzp;
> +
> + if (!tzp && !tz->ops->bind)
> + return;
> +
> + mutex_lock(&thermal_list_lock);
> +
> + /* If there is no platform data, try to use ops->bind */
> + if (!tzp && tz->ops->bind) {
> + list_for_each_entry(pos, &thermal_cdev_list, node) {
> + ret = tz->ops->bind(tz, pos);
> + if (ret)
> + print_bind_err_msg(tz, pos, ret);
> + }
> + goto exit;
> + }
> +
> + if (!tzp->tbp)
> + goto exit;
> +
> + list_for_each_entry(pos, &thermal_cdev_list, node) {
> + for (i = 0; i < tzp->num_tbps; i++) {
> + if (tzp->tbp[i].cdev || !tzp->tbp[i].match)
> + continue;
> + if (tzp->tbp[i].match(tz, pos))
> + continue;
> + tzp->tbp[i].cdev = pos;
> + __bind(tz, tzp->tbp[i].trip_mask, pos);
> + }
> + }
> +exit:
> + mutex_unlock(&thermal_list_lock);
> +}
> +
> /* sys I/F for thermal zone */
>
> #define to_thermal_zone(_dev) \
> @@ -943,7 +1048,6 @@ thermal_cooling_device_register(char *type, void
> *devdata,
> const struct thermal_cooling_device_ops *ops)
> {
> struct thermal_cooling_device *cdev;
> - struct thermal_zone_device *pos;
> int result;
>
> if (strlen(type) >= THERMAL_NAME_LENGTH) @@ -993,20 +1097,15 @@
> thermal_cooling_device_register(char *type, void *devdata,
> if (result)
> goto unregister;
>
> + /* Add 'this' new cdev to the global cdev list */
> mutex_lock(&thermal_list_lock);
> list_add(&cdev->node, &thermal_cdev_list);
> - list_for_each_entry(pos, &thermal_tz_list, node) {
> - if (!pos->ops->bind)
> - continue;
> - result = pos->ops->bind(pos, cdev);
> - if (result)
> - break;
> -
> - }
> mutex_unlock(&thermal_list_lock);
>
> - if (!result)
> - return cdev;
> + /* Update binding information for 'this' new cdev */
> + update_bind_info(cdev);
> +
> + return cdev;
>
> unregister:
> release_idr(&thermal_cdev_idr, &thermal_idr_lock, cdev->id); @@ -
> 1022,10 +1121,10 @@ EXPORT_SYMBOL(thermal_cooling_device_register);
> * thermal_cooling_device_unregister() must be called when the device
> is no
> * longer needed.
> */
> -void thermal_cooling_device_unregister(struct
> - thermal_cooling_device
> - *cdev)
> +void thermal_cooling_device_unregister(struct thermal_cooling_device
> +*cdev)
> {
> + int i;
> + struct thermal_zone_params *tzp;
> struct thermal_zone_device *tz;
> struct thermal_cooling_device *pos = NULL;
>
> @@ -1042,12 +1141,28 @@ void thermal_cooling_device_unregister(struct
> return;
> }
> list_del(&cdev->node);
> +
> + /* Unbind all thermal zones associated with 'this' cdev */
> list_for_each_entry(tz, &thermal_tz_list, node) {
> - if (!tz->ops->unbind)
> + if (tz->ops->unbind) {
> + tz->ops->unbind(tz, cdev);
> continue;
> - tz->ops->unbind(tz, cdev);
> + }
> +
> + if (!tz->tzp || !tz->tzp->tbp)
> + continue;
> +
> + tzp = tz->tzp;
> + for (i = 0; i < tzp->num_tbps; i++) {
> + if (tzp->tbp[i].cdev == cdev) {
> + __unbind(tz, tzp->tbp[i].trip_mask, cdev);
> + tzp->tbp[i].cdev = NULL;
> + }
> + }
> }
> +
> mutex_unlock(&thermal_list_lock);
> +
> if (cdev->type[0])
> device_remove_file(&cdev->device, &dev_attr_cdev_type);
> device_remove_file(&cdev->device, &dev_attr_max_state); @@ -
> 1405,7 +1520,6 @@ struct thermal_zone_device
> *thermal_zone_device_register(const char *type,
> int passive_delay, int polling_delay)
> {
> struct thermal_zone_device *tz;
> - struct thermal_cooling_device *pos;
> enum thermal_trip_type trip_type;
> int result;
> int count;
> @@ -1494,14 +1608,12 @@ struct thermal_zone_device
> *thermal_zone_device_register(const char *type,
>
> mutex_lock(&thermal_list_lock);
> list_add_tail(&tz->node, &thermal_tz_list);
> - if (ops->bind)
> - list_for_each_entry(pos, &thermal_cdev_list, node) {
> - result = ops->bind(tz, pos);
> - if (result)
> - break;
> - }
> mutex_unlock(&thermal_list_lock);
>
> + /* Bind cooling devices for this zone */
> + do_binding(tz);
> +
> +
> INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check);
>
> thermal_zone_device_update(tz);
> @@ -1522,12 +1634,16 @@ EXPORT_SYMBOL(thermal_zone_device_register);
> */
> void thermal_zone_device_unregister(struct thermal_zone_device *tz) {
> + int i;
> + struct thermal_zone_params *tzp;
> struct thermal_cooling_device *cdev;
> struct thermal_zone_device *pos = NULL;
>
> if (!tz)
> return;
>
> + tzp = tz->tzp;
> +
> mutex_lock(&thermal_list_lock);
> list_for_each_entry(pos, &thermal_tz_list, node)
> if (pos == tz)
> @@ -1538,9 +1654,25 @@ void thermal_zone_device_unregister(struct
> thermal_zone_device *tz)
> return;
> }
> list_del(&tz->node);
> - if (tz->ops->unbind)
> - list_for_each_entry(cdev, &thermal_cdev_list, node)
> - tz->ops->unbind(tz, cdev);
> +
> + /* Unbind all cdevs associated with 'this' thermal zone */
> + list_for_each_entry(cdev, &thermal_cdev_list, node) {
> + if (tz->ops->unbind) {
> + tz->ops->unbind(tz, cdev);
> + continue;
> + }
> +
> + if (!tzp || !tzp->tbp)
> + break;
> +
> + for (i = 0; i < tzp->num_tbps; i++) {
> + if (tzp->tbp[i].cdev == cdev) {
> + __unbind(tz, tzp->tbp[i].trip_mask, cdev);
> + tzp->tbp[i].cdev = NULL;
> + }
> + }
> + }
> +
> mutex_unlock(&thermal_list_lock);
>
> thermal_zone_device_set_polling(tz, 0);
> --
> 1.7.9.5
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 06/14] Thermal: Add a policy sysfs attribute
2012-08-27 9:25 ` Zhang, Rui
@ 2012-08-27 10:23 ` R, Durgadoss
2012-08-27 10:44 ` Zhang, Rui
0 siblings, 1 reply; 34+ messages in thread
From: R, Durgadoss @ 2012-08-27 10:23 UTC (permalink / raw
To: Zhang, Rui, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
> > > > #define to_cooling_device(_dev) \
> > > > @@ -1349,10 +1371,16 @@ static int retrieve_zone_params(struct
> > > > thermal_zone_device *tz)
> > > >
> > > > /* It is not an error to not have any platform data */
> > > > ret = get_platform_thermal_params(tz);
> > > > - if (ret)
> > > > + if (ret) {
> > > > tz->tzp = NULL;
> > > > + return 0;
> > > > + }
> > > >
> > > > - return 0;
> > > > + ret = device_create_file(&tz->device, &dev_attr_throttle_policy);
> > > > + if (ret)
> > > > + dev_err(&tz->device, "creating policy attr failed:%d\n",
> > > > ret);
> > > > +
> > > > + return ret;
> > > > }
> > >
> > > What does this mean?
> > > We will not create "policy" attributes if there is no
> > thermal_zone_params?
> >
> > Yes, that's what I thought initially. Because if there is no
> > 'throttle_policy'
> > we assume that it is (by default) step_wise.
> >
> > But, if we make tz_params be provided through tzd_register function
> > call, it makes sense for this to be a mandatory attribute, showing
> > 'step_wise"
> > if there is no thermal_zone_params.
> >
>
> IMO, every thermal zone should have a policy. And they can be changed
> anytime if user wants to.
Agree with you on the first part. Not sure if we want this to be writable.
Thanks,
Durga
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 06/14] Thermal: Add a policy sysfs attribute
2012-08-27 9:35 ` Zhang, Rui
@ 2012-08-27 10:25 ` R, Durgadoss
0 siblings, 0 replies; 34+ messages in thread
From: R, Durgadoss @ 2012-08-27 10:25 UTC (permalink / raw
To: Zhang, Rui, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
> > +static ssize_t
> > +policy_show(struct device *dev, struct device_attribute *devattr, char
> > +*buf) {
> > + struct thermal_zone_device *tz = to_thermal_zone(dev);
> > + struct thermal_zone_params *tzp = tz->tzp;
> > +
> > + if (!tzp)
> > + return sprintf(buf, "step_wise(default)\n");
> > +
> > + switch (tzp->throttle_policy) {
> > + case THERMAL_FAIR_SHARE:
> > + return sprintf(buf, "fair_share\n");
> > + case THERMAL_STEP_WISE:
> > + return sprintf(buf, "step_wise\n");
> > + case THERMAL_USER_SPACE:
> > + return sprintf(buf, "user_space\n");
> > + default:
> > + return sprintf(buf, "unknown\n");
> > + }
> > +}
> > +
> > static DEVICE_ATTR(type, 0444, type_show, NULL); static
> > DEVICE_ATTR(temp, 0444, temp_show, NULL); static DEVICE_ATTR(mode,
> > 0644, mode_show, mode_store); static DEVICE_ATTR(passive, S_IRUGO |
> > S_IWUSR, passive_show, passive_store);
> > +static DEVICE_ATTR(throttle_policy, S_IRUGO, policy_show, NULL);
> >
> > /* sys I/F for cooling device */
> > #define to_cooling_device(_dev) \
> > @@ -1349,10 +1371,16 @@ static int retrieve_zone_params(struct
> > thermal_zone_device *tz)
> >
> > /* It is not an error to not have any platform data */
> > ret = get_platform_thermal_params(tz);
> > - if (ret)
> > + if (ret) {
> > tz->tzp = NULL;
> > + return 0;
> > + }
> >
> > - return 0;
> > + ret = device_create_file(&tz->device, &dev_attr_throttle_policy);
> > + if (ret)
> > + dev_err(&tz->device, "creating policy attr failed:%d\n",
> > ret);
> > +
> > + return ret;
> > }
> >
> > /**
>
> We should remove this attribute in thermal_zone_device_unregister();
Oh yes, missed it :-(
Will fix in v3..
Thanks for the catch,
Durga
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 07/14] Thermal: Update binding logic based on platform data
2012-08-27 9:50 ` Zhang, Rui
@ 2012-08-27 10:29 ` R, Durgadoss
0 siblings, 0 replies; 34+ messages in thread
From: R, Durgadoss @ 2012-08-27 10:29 UTC (permalink / raw
To: Zhang, Rui, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
> > +
> > +static void __unbind(struct thermal_zone_device *tz, int mask,
> > + struct thermal_cooling_device *cdev) {
> > + int i;
> > +
> > + for (i = 0; i < tz->trips; i++)
> > + if (mask & (1 << i))
> > + thermal_zone_unbind_cooling_device(tz, i, cdev); }
> > +
> > +static void update_bind_info(struct thermal_cooling_device *cdev) {
>
> I do not think this function name is accurate.
>
> Update_bind_info is called when a new cooling device comes
> and do_binding is called when a new zone is registered, right?
> I think we should reflect this in the function name.
> Maybe something like this:
> Int do_bind(tz, cdev) {
> If (tz->ops->bind)
> Tz->ops->bind(tz, cdev);
> Else {
> Blabla
> ...
> }
> }
>
> Bind_cdev(cdev) {
> List_for_each_entry(tz, thermal_tz_list) {
> Do_bind(tz, cdev);
> }
> }
>
> Bind_tz(tz) {
> List_for_each_entry(cdev, thermal_cdev_list) {
> Do_bind(tz, cdev);
> }
> }
>
Will see how I can rename/make this patch cleaner..
Thanks,
Durga
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCHv2 06/14] Thermal: Add a policy sysfs attribute
2012-08-27 10:23 ` R, Durgadoss
@ 2012-08-27 10:44 ` Zhang, Rui
0 siblings, 0 replies; 34+ messages in thread
From: Zhang, Rui @ 2012-08-27 10:44 UTC (permalink / raw
To: R, Durgadoss, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, eduardo.valentin@ti.com
> -----Original Message-----
> From: R, Durgadoss
> Sent: Monday, August 27, 2012 1:24 PM
> To: Zhang, Rui; lenb@kernel.org
> Cc: linux-acpi@vger.kernel.org; eduardo.valentin@ti.com
> Subject: RE: [PATCHv2 06/14] Thermal: Add a policy sysfs attribute
> Importance: High
>
> > > > > #define to_cooling_device(_dev) \
> > > > > @@ -1349,10 +1371,16 @@ static int retrieve_zone_params(struct
> > > > > thermal_zone_device *tz)
> > > > >
> > > > > /* It is not an error to not have any platform data */
> > > > > ret = get_platform_thermal_params(tz);
> > > > > - if (ret)
> > > > > + if (ret) {
> > > > > tz->tzp = NULL;
> > > > > + return 0;
> > > > > + }
> > > > >
> > > > > - return 0;
> > > > > + ret = device_create_file(&tz->device,
> &dev_attr_throttle_policy);
> > > > > + if (ret)
> > > > > + dev_err(&tz->device, "creating policy attr
> failed:%d\n",
> > > > > ret);
> > > > > +
> > > > > + return ret;
> > > > > }
> > > >
> > > > What does this mean?
> > > > We will not create "policy" attributes if there is no
> > > thermal_zone_params?
> > >
> > > Yes, that's what I thought initially. Because if there is no
> > > 'throttle_policy'
> > > we assume that it is (by default) step_wise.
> > >
> > > But, if we make tz_params be provided through tzd_register function
> > > call, it makes sense for this to be a mandatory attribute, showing
> > > 'step_wise"
> > > if there is no thermal_zone_params.
> > >
> >
> > IMO, every thermal zone should have a policy. And they can be changed
> > anytime if user wants to.
>
> Agree with you on the first part. Not sure if we want this to be
> writable.
>
Say, what if a user space application is loaded and want to take control of the thermal management from kernel?
It should set the policy to "userspace" to stop the kernel actions first.
BTW, just like the cpufreq governors, they can be changed any time.
> Thanks,
> Durga
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2012-08-27 10:44 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-27 4:28 [PATCHv2 00/14] Thermal Framework Enhancements Durgadoss R
2012-08-27 4:28 ` [PATCHv2 01/14] Thermal: Refactor thermal.h file Durgadoss R
2012-08-27 4:28 ` [PATCHv2 02/14] Thermal: Move thermal_instance to thermal_core.h Durgadoss R
2012-08-27 4:28 ` [PATCHv2 03/14] Thermal: Add get trend, get instance API's to thermal_sys Durgadoss R
2012-08-27 8:11 ` Zhang, Rui
2012-08-27 8:47 ` R, Durgadoss
2012-08-27 9:19 ` Zhang, Rui
2012-08-27 9:24 ` R, Durgadoss
2012-08-27 4:28 ` [PATCHv2 04/14] Thermal: Add platform level information to thermal.h Durgadoss R
2012-08-27 8:27 ` Zhang, Rui
2012-08-27 8:57 ` R, Durgadoss
2012-08-27 9:22 ` Zhang, Rui
2012-08-27 9:25 ` R, Durgadoss
2012-08-27 4:28 ` [PATCHv2 05/14] Thermal: Obtain platform data for thermal zone Durgadoss R
2012-08-27 8:29 ` Zhang, Rui
2012-08-27 8:59 ` R, Durgadoss
2012-08-27 4:28 ` [PATCHv2 06/14] Thermal: Add a policy sysfs attribute Durgadoss R
2012-08-27 8:31 ` Zhang, Rui
2012-08-27 9:02 ` R, Durgadoss
2012-08-27 9:25 ` Zhang, Rui
2012-08-27 10:23 ` R, Durgadoss
2012-08-27 10:44 ` Zhang, Rui
2012-08-27 9:35 ` Zhang, Rui
2012-08-27 10:25 ` R, Durgadoss
2012-08-27 4:28 ` [PATCHv2 07/14] Thermal: Update binding logic based on platform data Durgadoss R
2012-08-27 9:50 ` Zhang, Rui
2012-08-27 10:29 ` R, Durgadoss
2012-08-27 4:28 ` [PATCHv2 08/14] Thermal: Make thermal_cdev_update as a global function Durgadoss R
2012-08-27 4:28 ` [PATCHv2 09/14] Thermal: Introduce fair_share thermal governor Durgadoss R
2012-08-27 4:28 ` [PATCHv2 10/14] Thermal: Introduce a step_wise " Durgadoss R
2012-08-27 4:28 ` [PATCHv2 11/14] Thermal: Remove throttling logic out of thermal_sys.c Durgadoss R
2012-08-27 4:28 ` [PATCHv2 12/14] Thermal: Add a notification API Durgadoss R
2012-08-27 4:28 ` [PATCHv2 13/14] Thermal: Add documentation for platform layer data Durgadoss R
2012-08-27 4:28 ` [PATCHv2 14/14] Thermal: Platform layer changes to provide thermal data Durgadoss R
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.