All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Various fixes to weights in the thermal framework
@ 2015-01-22 17:27 Javi Merino
  2015-01-22 17:27 ` [PATCH 1/5] thermal: of: fix cooling device weights in device tree Javi Merino
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Javi Merino @ 2015-01-22 17:27 UTC (permalink / raw)
  To: linux-pm; +Cc: Punit.Agrawal, Kapileshwar.Singh, Javi Merino

Weights can be specified in the device tree, but the code currently
ignores the value.  Patch 1 fixes that by putting the weight in the
thermal_instance.  Patches 2-3 simplifies the code in the fair share
governor by using the weight in the now populated thermal_instance.
Patch 4 exports the weight to sysfs.  Patch 5 drops the requirement of
all weights needing to add up to a hundred.

Existing thermal zones using this governor should still work, as these
series shouldn't make any functional change for them.  However,
thermal zones that previously weren't able to use the fair share
governor (because they were being register through device tree or
because they weren't populating thermal_bind_params) now populate the
weight properly and are able to use the governor.

Javi Merino (4):
  thermal: fair_share: use the weight from the thermal instance
  thermal: fair_share: fix typo
  thermal: export weight to sysfs
  thermal: fair_share: generalize the weight concept

Kapileshwar Singh (1):
  thermal: of: fix cooling device weights in device tree

 Documentation/thermal/sysfs-api.txt                | 27 ++++++++--
 drivers/thermal/db8500_thermal.c                   |  2 +-
 drivers/thermal/fair_share.c                       | 38 ++++++-------
 drivers/thermal/imx_thermal.c                      |  3 +-
 drivers/thermal/of-thermal.c                       |  5 +-
 drivers/thermal/samsung/exynos_thermal_common.c    |  3 +-
 drivers/thermal/thermal_core.c                     | 62 +++++++++++++++++++---
 drivers/thermal/thermal_core.h                     |  3 ++
 drivers/thermal/ti-soc-thermal/ti-thermal-common.c |  3 +-
 include/linux/thermal.h                            | 19 +++++--
 10 files changed, 126 insertions(+), 39 deletions(-)

-- 
1.9.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/5] thermal: of: fix cooling device weights in device tree
  2015-01-22 17:27 [PATCH 0/5] Various fixes to weights in the thermal framework Javi Merino
@ 2015-01-22 17:27 ` Javi Merino
  2015-01-31  0:00   ` Eduardo Valentin
  2015-01-22 17:27 ` [PATCH 2/5] thermal: fair_share: use the weight from the thermal instance Javi Merino
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Javi Merino @ 2015-01-22 17:27 UTC (permalink / raw)
  To: linux-pm
  Cc: Punit.Agrawal, Kapileshwar.Singh, Kapileshwar Singh, Zhang Rui,
	Eduardo Valentin, Kukjin Kim

From: Kapileshwar Singh <kapileshwar.singh@arm.com>

Currently you can specify the weight of the cooling device in the device
tree but that information is not populated to the
thermal_bind_params where the fair share governor expects it to
be.  The of thermal zone device doesn't have a thermal_bind_params
structure and arguably it's better to pass the weight inside the
thermal_instance as it is specific to the bind of a cooling device to a
thermal zone parameter.

Core thermal code is fixed to populate the weight in the instance from
the thermal_bind_params, so platform code that was passing the weight
inside the thermal_bind_params continue to work seamlessly.

While we are at it, create a default value for the weight parameter for
those thermal zones that currently don't define it and remove the
hardcoded default in of-thermal.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Kukjin Kim <kgene@kernel.org>
Signed-off-by: Kapileshwar Singh <kapileshwar.singh@arm.com>
---

This patch was previously suggested here[0].  During the discussion,
Eduardo suggested that dropping the bind op for of-thermal and using
tbps and their .match() function was preferrable.  We sent a series
that did that in [1].  However, looking at the code we think that this
patch is preferrable as it is simpler and enables the rest of the
patches in these series, specially the simplification in patch 2.
That's why we are reposting it, now properly asking for inclusion.

[0] http://thread.gmane.org/gmane.linux.power-management.general/54622
[1] http://thread.gmane.org/gmane.linux.power-management.general/55339

 Documentation/thermal/sysfs-api.txt                |  4 +++-
 drivers/thermal/db8500_thermal.c                   |  2 +-
 drivers/thermal/fair_share.c                       |  2 +-
 drivers/thermal/imx_thermal.c                      |  3 ++-
 drivers/thermal/of-thermal.c                       |  5 +++--
 drivers/thermal/samsung/exynos_thermal_common.c    |  3 ++-
 drivers/thermal/thermal_core.c                     | 22 ++++++++++++++++------
 drivers/thermal/thermal_core.h                     |  1 +
 drivers/thermal/ti-soc-thermal/ti-thermal-common.c |  3 ++-
 include/linux/thermal.h                            |  6 +++++-
 10 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 87519cb379ee..7ec632ed9769 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -95,7 +95,7 @@ temperature) and throttle appropriate devices.
 1.3 interface for binding a thermal zone device with a thermal cooling device
 1.3.1 int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
 	int trip, struct thermal_cooling_device *cdev,
-	unsigned long upper, unsigned long lower);
+	unsigned long upper, unsigned long lower, unsigned int weight);
 
     This interface function bind a thermal cooling device to the certain trip
     point of a thermal zone device.
@@ -110,6 +110,8 @@ temperature) and throttle appropriate devices.
     lower:the Minimum cooling state can be used for this trip point.
           THERMAL_NO_LIMIT means no lower limit,
 	  and the cooling device can be in cooling state 0.
+    weight: the influence of this cooling device in this thermal
+            zone.  See 1.4.1 below for more information.
 
 1.3.2 int thermal_zone_unbind_cooling_device(struct thermal_zone_device *tz,
 		int trip, struct thermal_cooling_device *cdev);
diff --git a/drivers/thermal/db8500_thermal.c b/drivers/thermal/db8500_thermal.c
index 20adfbe27df1..2fb273c4baa9 100644
--- a/drivers/thermal/db8500_thermal.c
+++ b/drivers/thermal/db8500_thermal.c
@@ -76,7 +76,7 @@ static int db8500_cdev_bind(struct thermal_zone_device *thermal,
 		upper = lower = i > max_state ? max_state : i;
 
 		ret = thermal_zone_bind_cooling_device(thermal, i, cdev,
-			upper, lower);
+			upper, lower, THERMAL_WEIGHT_DEFAULT);
 
 		dev_info(&cdev->device, "%s bind to %d: %d-%s\n", cdev->type,
 			i, ret, ret ? "fail" : "succeed");
diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
index 6e0a3fbfae86..c3b25187b467 100644
--- a/drivers/thermal/fair_share.c
+++ b/drivers/thermal/fair_share.c
@@ -109,7 +109,7 @@ static int fair_share_throttle(struct thermal_zone_device *tz, int trip)
 			continue;
 
 		instance->target = get_target_state(tz, cdev,
-					tzp->tbp[i].weight, cur_trip_level);
+					instance->weight, cur_trip_level);
 
 		instance->cdev->updated = false;
 		thermal_cdev_update(cdev);
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 2ccbc0788353..36579c05605f 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -306,7 +306,8 @@ static int imx_bind(struct thermal_zone_device *tz,
 
 	ret = thermal_zone_bind_cooling_device(tz, IMX_TRIP_PASSIVE, cdev,
 					       THERMAL_NO_LIMIT,
-					       THERMAL_NO_LIMIT);
+					       THERMAL_NO_LIMIT
+					       THERMAL_WEIGHT_DEFAULT);
 	if (ret) {
 		dev_err(&tz->device,
 			"binding zone %s with cdev %s failed:%d\n",
diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index d717f3dab6f1..60e3a21461e7 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -227,7 +227,8 @@ static int of_thermal_bind(struct thermal_zone_device *thermal,
 			ret = thermal_zone_bind_cooling_device(thermal,
 						tbp->trip_id, cdev,
 						tbp->max,
-						tbp->min);
+						tbp->min,
+						tbp->usage);
 			if (ret)
 				return ret;
 		}
@@ -578,7 +579,7 @@ static int thermal_of_populate_bind_params(struct device_node *np,
 	u32 prop;
 
 	/* Default weight. Usage is optional */
-	__tbp->usage = 0;
+	__tbp->usage = THERMAL_WEIGHT_DEFAULT;
 	ret = of_property_read_u32(np, "contribution", &prop);
 	if (ret == 0)
 		__tbp->usage = prop;
diff --git a/drivers/thermal/samsung/exynos_thermal_common.c b/drivers/thermal/samsung/exynos_thermal_common.c
index 6dc3815cc73f..03f2ebb1e5c3 100644
--- a/drivers/thermal/samsung/exynos_thermal_common.c
+++ b/drivers/thermal/samsung/exynos_thermal_common.c
@@ -163,7 +163,8 @@ static int exynos_bind(struct thermal_zone_device *thermal,
 		case MONITOR_ZONE:
 		case WARN_ZONE:
 			if (thermal_zone_bind_cooling_device(thermal, i, cdev,
-								level, 0)) {
+						level, 0,
+						THERMAL_WEIGHT_DEFAULT)) {
 				dev_err(data->dev,
 					"error unbinding cdev inst=%d\n", i);
 				ret = -EINVAL;
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 87e0b0782023..9a424a66c83a 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -218,7 +218,8 @@ static void print_bind_err_msg(struct thermal_zone_device *tz,
 
 static void __bind(struct thermal_zone_device *tz, int mask,
 			struct thermal_cooling_device *cdev,
-			unsigned long *limits)
+			unsigned long *limits,
+			unsigned int weight)
 {
 	int i, ret;
 
@@ -233,7 +234,8 @@ static void __bind(struct thermal_zone_device *tz, int mask,
 				upper = limits[i * 2 + 1];
 			}
 			ret = thermal_zone_bind_cooling_device(tz, i, cdev,
-							       upper, lower);
+							       upper, lower,
+							       weight);
 			if (ret)
 				print_bind_err_msg(tz, cdev, ret);
 		}
@@ -280,7 +282,8 @@ static void bind_cdev(struct thermal_cooling_device *cdev)
 				continue;
 			tzp->tbp[i].cdev = cdev;
 			__bind(pos, tzp->tbp[i].trip_mask, cdev,
-			       tzp->tbp[i].binding_limits);
+			       tzp->tbp[i].binding_limits,
+			       tzp->tbp[i].weight);
 		}
 	}
 
@@ -319,7 +322,8 @@ static void bind_tz(struct thermal_zone_device *tz)
 				continue;
 			tzp->tbp[i].cdev = pos;
 			__bind(tz, tzp->tbp[i].trip_mask, pos,
-			       tzp->tbp[i].binding_limits);
+			       tzp->tbp[i].binding_limits,
+			       tzp->tbp[i].weight);
 		}
 	}
 exit:
@@ -711,7 +715,8 @@ passive_store(struct device *dev, struct device_attribute *attr,
 				thermal_zone_bind_cooling_device(tz,
 						THERMAL_TRIPS_NONE, cdev,
 						THERMAL_NO_LIMIT,
-						THERMAL_NO_LIMIT);
+						THERMAL_NO_LIMIT,
+						THERMAL_WEIGHT_DEFAULT);
 		}
 		mutex_unlock(&thermal_list_lock);
 		if (!tz->passive_delay)
@@ -913,6 +918,9 @@ thermal_cooling_device_trip_point_show(struct device *dev,
  * @lower:	the Minimum cooling state can be used for this trip point.
  *		THERMAL_NO_LIMIT means no lower limit,
  *		and the cooling device can be in cooling state 0.
+ * @weight:	The weight of the cooling device to be bound to the
+ *		thermal zone. Use THERMAL_WEIGHT_DEFAULT for the
+ *		default value
  *
  * This interface function bind a thermal cooling device to the certain trip
  * point of a thermal zone device.
@@ -923,7 +931,8 @@ thermal_cooling_device_trip_point_show(struct device *dev,
 int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
 				     int trip,
 				     struct thermal_cooling_device *cdev,
-				     unsigned long upper, unsigned long lower)
+				     unsigned long upper, unsigned long lower,
+				     unsigned int weight)
 {
 	struct thermal_instance *dev;
 	struct thermal_instance *pos;
@@ -968,6 +977,7 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
 	dev->upper = upper;
 	dev->lower = lower;
 	dev->target = THERMAL_NO_TARGET;
+	dev->weight = weight;
 
 	result = get_idr(&tz->idr, &tz->lock, &dev->id);
 	if (result)
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 0531c752fbbb..7a465e9d456c 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -48,6 +48,7 @@ struct thermal_instance {
 	struct device_attribute attr;
 	struct list_head tz_node; /* node in tz->thermal_instances */
 	struct list_head cdev_node; /* node in cdev->thermal_instances */
+	unsigned int weight; /* The weight of the cooling device */
 };
 
 int thermal_register_governor(struct thermal_governor *);
diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index 3fb054a10f6a..4638718fabf6 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -146,7 +146,8 @@ static int ti_thermal_bind(struct thermal_zone_device *thermal,
 	return thermal_zone_bind_cooling_device(thermal, 0, cdev,
 	/* bind with min and max states defined by cpu_cooling */
 						THERMAL_NO_LIMIT,
-						THERMAL_NO_LIMIT);
+						THERMAL_NO_LIMIT,
+						THERMAL_WEIGHT_DEFAULT);
 }
 
 /* Unbind callback functions for thermal zone */
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index fc52e307efab..2ed7062fac1d 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -40,6 +40,9 @@
 /* No upper/lower limit requirement */
 #define THERMAL_NO_LIMIT	((u32)~0)
 
+/* Default weight of a bound cooling device */
+#define THERMAL_WEIGHT_DEFAULT 0
+
 /* Unit conversion macros */
 #define KELVIN_TO_CELSIUS(t)	(long)(((long)t-2732 >= 0) ?	\
 				((long)t-2732+5)/10 : ((long)t-2732-5)/10)
@@ -321,7 +324,8 @@ void thermal_zone_device_unregister(struct thermal_zone_device *);
 
 int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
 				     struct thermal_cooling_device *,
-				     unsigned long, unsigned long);
+				     unsigned long, unsigned long,
+				     unsigned 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 *);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/5] thermal: fair_share: use the weight from the thermal instance
  2015-01-22 17:27 [PATCH 0/5] Various fixes to weights in the thermal framework Javi Merino
  2015-01-22 17:27 ` [PATCH 1/5] thermal: of: fix cooling device weights in device tree Javi Merino
@ 2015-01-22 17:27 ` Javi Merino
  2015-01-22 17:27 ` [PATCH 3/5] thermal: fair_share: fix typo Javi Merino
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Javi Merino @ 2015-01-22 17:27 UTC (permalink / raw)
  To: linux-pm
  Cc: Punit.Agrawal, Kapileshwar.Singh, Javi Merino, Zhang Rui,
	Eduardo Valentin

The fair share governor is not usable with thermal zones that use the
bind op and don't populate thermal_zone_parameters, the majority of
them.  Now that the weight is in the thermal instance, we can use that
in the fair share governor to allow every thermal zone to trivially use
this governor.  Furthermore, this simplifies the code.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---
 drivers/thermal/fair_share.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
index c3b25187b467..9e392d34ac9f 100644
--- a/drivers/thermal/fair_share.c
+++ b/drivers/thermal/fair_share.c
@@ -88,24 +88,13 @@ static long get_target_state(struct thermal_zone_device *tz,
  */
 static int fair_share_throttle(struct thermal_zone_device *tz, int trip)
 {
-	const 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 -EINVAL;
+	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
+		struct thermal_cooling_device *cdev = instance->cdev;
 
-	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)
+		if (instance->trip != trip)
 			continue;
 
 		instance->target = get_target_state(tz, cdev,
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/5] thermal: fair_share: fix typo
  2015-01-22 17:27 [PATCH 0/5] Various fixes to weights in the thermal framework Javi Merino
  2015-01-22 17:27 ` [PATCH 1/5] thermal: of: fix cooling device weights in device tree Javi Merino
  2015-01-22 17:27 ` [PATCH 2/5] thermal: fair_share: use the weight from the thermal instance Javi Merino
@ 2015-01-22 17:27 ` Javi Merino
  2015-01-22 17:27 ` [PATCH 4/5] thermal: export weight to sysfs Javi Merino
  2015-01-22 17:27 ` [PATCH 5/5] thermal: fair_share: generalize the weight concept Javi Merino
  4 siblings, 0 replies; 8+ messages in thread
From: Javi Merino @ 2015-01-22 17:27 UTC (permalink / raw)
  To: linux-pm
  Cc: Punit.Agrawal, Kapileshwar.Singh, Javi Merino, Zhang Rui,
	Eduardo Valentin

s/asscciated/associated/

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---
 drivers/thermal/fair_share.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
index 9e392d34ac9f..692f4053f08b 100644
--- a/drivers/thermal/fair_share.c
+++ b/drivers/thermal/fair_share.c
@@ -69,7 +69,7 @@ static long get_target_state(struct thermal_zone_device *tz,
 }
 
 /**
- * fair_share_throttle - throttles devices asscciated with the given zone
+ * fair_share_throttle - throttles devices associated with the given zone
  * @tz - thermal_zone_device
  *
  * Throttling Logic: This uses three parameters to calculate the new
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/5] thermal: export weight to sysfs
  2015-01-22 17:27 [PATCH 0/5] Various fixes to weights in the thermal framework Javi Merino
                   ` (2 preceding siblings ...)
  2015-01-22 17:27 ` [PATCH 3/5] thermal: fair_share: fix typo Javi Merino
@ 2015-01-22 17:27 ` Javi Merino
  2015-01-22 17:27 ` [PATCH 5/5] thermal: fair_share: generalize the weight concept Javi Merino
  4 siblings, 0 replies; 8+ messages in thread
From: Javi Merino @ 2015-01-22 17:27 UTC (permalink / raw)
  To: linux-pm
  Cc: Punit.Agrawal, Kapileshwar.Singh, Javi Merino, Zhang Rui,
	Eduardo Valentin

It's useful to have access to the weights for the cooling devices for
thermal zones and change them if needed.  Export them to sysfs.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---
 Documentation/thermal/sysfs-api.txt | 15 +++++++++++++-
 drivers/thermal/thermal_core.c      | 40 +++++++++++++++++++++++++++++++++++++
 drivers/thermal/thermal_core.h      |  2 ++
 3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 7ec632ed9769..3625453ceef6 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -194,6 +194,8 @@ thermal_zone_bind_cooling_device/thermal_zone_unbind_cooling_device.
 /sys/class/thermal/thermal_zone[0-*]:
     |---cdev[0-*]:		[0-*]th cooling device in current thermal zone
     |---cdev[0-*]_trip_point:	Trip point that cdev[0-*] is associated with
+    |---cdev[0-*]_weight:       Influence of the cooling device in
+                                this thermal zone
 
 Besides the thermal zone device sysfs I/F and cooling device sysfs I/F,
 the generic thermal driver also creates a hwmon sysfs I/F for each _type_
@@ -267,6 +269,14 @@ cdev[0-*]_trip_point
 	point.
 	RO, Optional
 
+cdev[0-*]_weight
+        The influence of cdev[0-*] in this thermal zone. This value
+        is relative to the rest of cooling devices in the thermal
+        zone. For example, if a cooling device has a weight double
+        than that of other, it's twice as effective in cooling the
+        thermal zone.
+        RW, Optional
+
 passive
 	Attribute is only present for zones in which the passive cooling
 	policy is not supported by native thermal driver. Default is zero
@@ -320,7 +330,8 @@ passive, active. If an ACPI thermal zone supports critical, passive,
 active[0] and active[1] at the same time, it may register itself as a
 thermal_zone_device (thermal_zone1) with 4 trip points in all.
 It has one processor and one fan, which are both registered as
-thermal_cooling_device.
+thermal_cooling_device. Both are considered to have the same
+effectiveness in cooling the thermal zone.
 
 If the processor is listed in _PSL method, and the fan is listed in _AL0
 method, the sys I/F structure will be built like this:
@@ -342,8 +353,10 @@ method, the sys I/F structure will be built like this:
     |---trip_point_3_type:	active1
     |---cdev0:			--->/sys/class/thermal/cooling_device0
     |---cdev0_trip_point:	1	/* cdev0 can be used for passive */
+    |---cdev0_weight:           1024
     |---cdev1:			--->/sys/class/thermal/cooling_device3
     |---cdev1_trip_point:	2	/* cdev1 can be used for active[0]*/
+    |---cdev1_weight:           1024
 
 |cooling_device0:
     |---type:			Processor
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 9a424a66c83a..eaa704043e51 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -904,6 +904,34 @@ thermal_cooling_device_trip_point_show(struct device *dev,
 		return sprintf(buf, "%d\n", instance->trip);
 }
 
+static ssize_t
+thermal_cooling_device_weight_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct thermal_instance *instance;
+
+	instance = container_of(attr, struct thermal_instance, weight_attr);
+
+	return sprintf(buf, "%d\n", instance->weight);
+}
+
+static ssize_t
+thermal_cooling_device_weight_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	struct thermal_instance *instance;
+	int ret, weight;
+
+	ret = kstrtoint(buf, 0, &weight);
+	if (ret)
+		return ret;
+
+	instance = container_of(attr, struct thermal_instance, weight_attr);
+	instance->weight = weight;
+
+	return count;
+}
 /* Device management */
 
 /**
@@ -998,6 +1026,16 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
 	if (result)
 		goto remove_symbol_link;
 
+	sprintf(dev->weight_attr_name, "cdev%d_weight", dev->id);
+	sysfs_attr_init(&dev->weight_attr.attr);
+	dev->weight_attr.attr.name = dev->weight_attr_name;
+	dev->weight_attr.attr.mode = S_IWUSR | S_IRUGO;
+	dev->weight_attr.show = thermal_cooling_device_weight_show;
+	dev->weight_attr.store = thermal_cooling_device_weight_store;
+	result = device_create_file(&tz->device, &dev->weight_attr);
+	if (result)
+		goto remove_trip_file;
+
 	mutex_lock(&tz->lock);
 	mutex_lock(&cdev->lock);
 	list_for_each_entry(pos, &tz->thermal_instances, tz_node)
@@ -1015,6 +1053,8 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
 	if (!result)
 		return 0;
 
+	device_remove_file(&tz->device, &dev->weight_attr);
+remove_trip_file:
 	device_remove_file(&tz->device, &dev->attr);
 remove_symbol_link:
 	sysfs_remove_link(&tz->device.kobj, dev->name);
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 7a465e9d456c..faebe881f062 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -46,6 +46,8 @@ struct thermal_instance {
 	unsigned long target;	/* expected cooling state */
 	char attr_name[THERMAL_NAME_LENGTH];
 	struct device_attribute attr;
+	char weight_attr_name[THERMAL_NAME_LENGTH];
+	struct device_attribute weight_attr;
 	struct list_head tz_node; /* node in tz->thermal_instances */
 	struct list_head cdev_node; /* node in cdev->thermal_instances */
 	unsigned int weight; /* The weight of the cooling device */
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/5] thermal: fair_share: generalize the weight concept
  2015-01-22 17:27 [PATCH 0/5] Various fixes to weights in the thermal framework Javi Merino
                   ` (3 preceding siblings ...)
  2015-01-22 17:27 ` [PATCH 4/5] thermal: export weight to sysfs Javi Merino
@ 2015-01-22 17:27 ` Javi Merino
  4 siblings, 0 replies; 8+ messages in thread
From: Javi Merino @ 2015-01-22 17:27 UTC (permalink / raw)
  To: linux-pm
  Cc: Punit.Agrawal, Kapileshwar.Singh, Javi Merino, Zhang Rui,
	Eduardo Valentin

The fair share governor has the concept of weights, which is the
influence of each cooling device in a thermal zone.  The current
implementation forces the weights of all cooling devices in a thermal
zone to add up to a 100.  This complicates setups, as you need to know
in advance how many cooling devices you are going to have.  If you bind a
new cooling device, you have to modify all the other cooling devices
weights, which is error prone.  Furthermore, you can't specify a
"default" weight for platforms since that default value depends on the
number of cooling devices in the platform.

This patch generalizes the concept of weight by allowing any number to
be a "weight".  Weights are now relative to each other.  Platforms that
don't specify weights get the same default value for all their cooling
devices, so all their cdevs are considered to be equally influential.

It's important to note that previous users of the weights don't need to
alter the code: percentages continue to work as they used to.  This
patch just removes the constraint of all the weights in a thermal zone
having to add up to a 100.  If they do, you get the same behavior as
before.  If they don't, fair share now works for that platform.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---
 Documentation/thermal/sysfs-api.txt |  8 +++++---
 drivers/thermal/fair_share.c        | 23 ++++++++++++++++++-----
 include/linux/thermal.h             | 17 ++++++++++++-----
 3 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 3625453ceef6..c9fd014f0c21 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -129,9 +129,11 @@ temperature) and throttle appropriate devices.
     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.
+    .weight: The 'influence' of a particular cooling device on this
+             zone. This is relative to the rest of the cooling
+             devices. For example, if all cooling devices have a
+             weight of 1, then they all contribute the same. You can
+             use percentages if you want, but it's not mandatory.
     .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
diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
index 692f4053f08b..5cd6ff1d0a4c 100644
--- a/drivers/thermal/fair_share.c
+++ b/drivers/thermal/fair_share.c
@@ -59,13 +59,13 @@ static int get_trip_level(struct thermal_zone_device *tz)
 }
 
 static long get_target_state(struct thermal_zone_device *tz,
-		struct thermal_cooling_device *cdev, int weight, int level)
+		struct thermal_cooling_device *cdev, int percentage, int level)
 {
 	unsigned long max_state;
 
 	cdev->ops->get_max_state(cdev, &max_state);
 
-	return (long)(weight * level * max_state) / (100 * tz->trips);
+	return (long)(percentage * level * max_state) / (100 * tz->trips);
 }
 
 /**
@@ -77,7 +77,7 @@ static long get_target_state(struct thermal_zone_device *tz,
  *
  * Parameters used for Throttling:
  * P1. max_state: Maximum throttle state exposed by the cooling device.
- * P2. weight[i]/100:
+ * P2. percentage[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.
@@ -89,16 +89,29 @@ static long get_target_state(struct thermal_zone_device *tz,
 static int fair_share_throttle(struct thermal_zone_device *tz, int trip)
 {
 	struct thermal_instance *instance;
+	int total_weight = 0;
 	int cur_trip_level = get_trip_level(tz);
 
 	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
+		if (instance->trip != trip)
+			continue;
+
+		total_weight += instance->weight;
+	}
+
+	if (!total_weight)
+		return -EINVAL;
+
+	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
+		int percentage;
 		struct thermal_cooling_device *cdev = instance->cdev;
 
 		if (instance->trip != trip)
 			continue;
 
-		instance->target = get_target_state(tz, cdev,
-					instance->weight, cur_trip_level);
+		percentage = (instance->weight * 100) / total_weight;
+		instance->target = get_target_state(tz, cdev, percentage,
+						    cur_trip_level);
 
 		instance->cdev->updated = false;
 		thermal_cdev_update(cdev);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 2ed7062fac1d..2426426731ca 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -40,8 +40,12 @@
 /* No upper/lower limit requirement */
 #define THERMAL_NO_LIMIT	((u32)~0)
 
-/* Default weight of a bound cooling device */
-#define THERMAL_WEIGHT_DEFAULT 0
+/*
+ * Default weight of a bound cooling device.  By setting it to 1024 we
+ * give developers a range so that they can specify cooling devices
+ * that are less or more "influential" than the default
+ */
+#define THERMAL_WEIGHT_DEFAULT 1024
 
 /* Unit conversion macros */
 #define KELVIN_TO_CELSIUS(t)	(long)(((long)t-2732 >= 0) ?	\
@@ -217,9 +221,12 @@ struct thermal_bind_params {
 
 	/*
 	 * 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.
+	 * cool 'this' thermal zone. It shall be determined by
+	 * platform characterization. This value is relative to the
+	 * rest of the weights so a cooling device whose weight is
+	 * double that of another cooling device is twice as
+	 * effective. See Documentation/thermal/sysfs-api.txt for more
+	 * information.
 	 */
 	int weight;
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/5] thermal: of: fix cooling device weights in device tree
  2015-01-22 17:27 ` [PATCH 1/5] thermal: of: fix cooling device weights in device tree Javi Merino
@ 2015-01-31  0:00   ` Eduardo Valentin
  2015-02-01 12:21     ` Javi Merino
  0 siblings, 1 reply; 8+ messages in thread
From: Eduardo Valentin @ 2015-01-31  0:00 UTC (permalink / raw)
  To: Javi Merino
  Cc: linux-pm, Punit.Agrawal, Kapileshwar.Singh, Zhang Rui, Kukjin Kim

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

On Thu, Jan 22, 2015 at 05:27:01PM +0000, Javi Merino wrote:
> From: Kapileshwar Singh <kapileshwar.singh@arm.com>
> 
> Currently you can specify the weight of the cooling device in the device
> tree but that information is not populated to the
> thermal_bind_params where the fair share governor expects it to
> be.  The of thermal zone device doesn't have a thermal_bind_params
> structure and arguably it's better to pass the weight inside the
> thermal_instance as it is specific to the bind of a cooling device to a
> thermal zone parameter.
> 
> Core thermal code is fixed to populate the weight in the instance from
> the thermal_bind_params, so platform code that was passing the weight
> inside the thermal_bind_params continue to work seamlessly.
> 
> While we are at it, create a default value for the weight parameter for
> those thermal zones that currently don't define it and remove the
> hardcoded default in of-thermal.
> 
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Kukjin Kim <kgene@kernel.org>
> Signed-off-by: Kapileshwar Singh <kapileshwar.singh@arm.com>
> ---
> 
> This patch was previously suggested here[0].  During the discussion,
> Eduardo suggested that dropping the bind op for of-thermal and using
> tbps and their .match() function was preferrable.  We sent a series
> that did that in [1].  However, looking at the code we think that this
> patch is preferrable as it is simpler and enables the rest of the
> patches in these series, specially the simplification in patch 2.
> That's why we are reposting it, now properly asking for inclusion.
> 
> [0] http://thread.gmane.org/gmane.linux.power-management.general/54622
> [1] http://thread.gmane.org/gmane.linux.power-management.general/55339

Despite going one way or another, this patch breaks existing users of
the thermal_zone_bind_cooling_device API.

> 
>  Documentation/thermal/sysfs-api.txt                |  4 +++-
>  drivers/thermal/db8500_thermal.c                   |  2 +-
>  drivers/thermal/fair_share.c                       |  2 +-
>  drivers/thermal/imx_thermal.c                      |  3 ++-
>  drivers/thermal/of-thermal.c                       |  5 +++--
>  drivers/thermal/samsung/exynos_thermal_common.c    |  3 ++-
>  drivers/thermal/thermal_core.c                     | 22 ++++++++++++++++------
>  drivers/thermal/thermal_core.h                     |  1 +
>  drivers/thermal/ti-soc-thermal/ti-thermal-common.c |  3 ++-
>  include/linux/thermal.h                            |  6 +++++-
>  10 files changed, 36 insertions(+), 15 deletions(-)

$ git grep thermal_zone_bind_cooling_device
Documentation/thermal/sysfs-api.txt:1.3.1 int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
Documentation/thermal/sysfs-api.txt:thermal_zone_bind_cooling_device/thermal_zone_unbind_cooling_device.
drivers/acpi/thermal.c:					thermal_zone_bind_cooling_device
drivers/acpi/thermal.c:				result = thermal_zone_bind_cooling_device
drivers/acpi/thermal.c:				result = thermal_zone_bind_cooling_device

We are not supposed to break ACPI code base :-)

drivers/platform/x86/acerhdf.c:	if (thermal_zone_bind_cooling_device(thermal, 0, cdev,

You may want to add acerhdf to your patch.

drivers/thermal/db8500_thermal.c:		ret = thermal_zone_bind_cooling_device(thermal, i, cdev,
drivers/thermal/imx_thermal.c:	ret = thermal_zone_bind_cooling_device(tz, IMX_TRIP_PASSIVE, cdev,
drivers/thermal/of-thermal.c:			ret = thermal_zone_bind_cooling_device(thermal,
drivers/thermal/thermal_core.c:			ret = thermal_zone_bind_cooling_device(tz, i, cdev,
drivers/thermal/thermal_core.c:				thermal_zone_bind_cooling_device(tz,
drivers/thermal/thermal_core.c: * thermal_zone_bind_cooling_device() - bind a cooling device to a thermal zone
drivers/thermal/thermal_core.c:int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
drivers/thermal/thermal_core.c:EXPORT_SYMBOL_GPL(thermal_zone_bind_cooling_device);
drivers/thermal/ti-soc-thermal/ti-thermal-common.c:	return thermal_zone_bind_cooling_device(thermal, 0, cdev,
include/linux/thermal.h:int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,

> 
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index 87519cb379ee..7ec632ed9769 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -95,7 +95,7 @@ temperature) and throttle appropriate devices.
>  1.3 interface for binding a thermal zone device with a thermal cooling device
>  1.3.1 int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
>  	int trip, struct thermal_cooling_device *cdev,
> -	unsigned long upper, unsigned long lower);
> +	unsigned long upper, unsigned long lower, unsigned int weight);
>  
>      This interface function bind a thermal cooling device to the certain trip
>      point of a thermal zone device.
> @@ -110,6 +110,8 @@ temperature) and throttle appropriate devices.
>      lower:the Minimum cooling state can be used for this trip point.
>            THERMAL_NO_LIMIT means no lower limit,
>  	  and the cooling device can be in cooling state 0.
> +    weight: the influence of this cooling device in this thermal
> +            zone.  See 1.4.1 below for more information.
>  
>  1.3.2 int thermal_zone_unbind_cooling_device(struct thermal_zone_device *tz,
>  		int trip, struct thermal_cooling_device *cdev);
> diff --git a/drivers/thermal/db8500_thermal.c b/drivers/thermal/db8500_thermal.c
> index 20adfbe27df1..2fb273c4baa9 100644
> --- a/drivers/thermal/db8500_thermal.c
> +++ b/drivers/thermal/db8500_thermal.c
> @@ -76,7 +76,7 @@ static int db8500_cdev_bind(struct thermal_zone_device *thermal,
>  		upper = lower = i > max_state ? max_state : i;
>  
>  		ret = thermal_zone_bind_cooling_device(thermal, i, cdev,
> -			upper, lower);
> +			upper, lower, THERMAL_WEIGHT_DEFAULT);
>  
>  		dev_info(&cdev->device, "%s bind to %d: %d-%s\n", cdev->type,
>  			i, ret, ret ? "fail" : "succeed");
> diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
> index 6e0a3fbfae86..c3b25187b467 100644
> --- a/drivers/thermal/fair_share.c
> +++ b/drivers/thermal/fair_share.c
> @@ -109,7 +109,7 @@ static int fair_share_throttle(struct thermal_zone_device *tz, int trip)
>  			continue;
>  
>  		instance->target = get_target_state(tz, cdev,
> -					tzp->tbp[i].weight, cur_trip_level);
> +					instance->weight, cur_trip_level);
>  
>  		instance->cdev->updated = false;
>  		thermal_cdev_update(cdev);
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 2ccbc0788353..36579c05605f 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -306,7 +306,8 @@ static int imx_bind(struct thermal_zone_device *tz,
>  
>  	ret = thermal_zone_bind_cooling_device(tz, IMX_TRIP_PASSIVE, cdev,
>  					       THERMAL_NO_LIMIT,
> -					       THERMAL_NO_LIMIT);
> +					       THERMAL_NO_LIMIT
> +					       THERMAL_WEIGHT_DEFAULT);
>  	if (ret) {
>  		dev_err(&tz->device,
>  			"binding zone %s with cdev %s failed:%d\n",
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index d717f3dab6f1..60e3a21461e7 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -227,7 +227,8 @@ static int of_thermal_bind(struct thermal_zone_device *thermal,
>  			ret = thermal_zone_bind_cooling_device(thermal,
>  						tbp->trip_id, cdev,
>  						tbp->max,
> -						tbp->min);
> +						tbp->min,
> +						tbp->usage);
>  			if (ret)
>  				return ret;
>  		}
> @@ -578,7 +579,7 @@ static int thermal_of_populate_bind_params(struct device_node *np,
>  	u32 prop;
>  
>  	/* Default weight. Usage is optional */
> -	__tbp->usage = 0;
> +	__tbp->usage = THERMAL_WEIGHT_DEFAULT;
>  	ret = of_property_read_u32(np, "contribution", &prop);
>  	if (ret == 0)
>  		__tbp->usage = prop;
> diff --git a/drivers/thermal/samsung/exynos_thermal_common.c b/drivers/thermal/samsung/exynos_thermal_common.c
> index 6dc3815cc73f..03f2ebb1e5c3 100644
> --- a/drivers/thermal/samsung/exynos_thermal_common.c
> +++ b/drivers/thermal/samsung/exynos_thermal_common.c
> @@ -163,7 +163,8 @@ static int exynos_bind(struct thermal_zone_device *thermal,
>  		case MONITOR_ZONE:
>  		case WARN_ZONE:
>  			if (thermal_zone_bind_cooling_device(thermal, i, cdev,
> -								level, 0)) {
> +						level, 0,
> +						THERMAL_WEIGHT_DEFAULT)) {
>  				dev_err(data->dev,
>  					"error unbinding cdev inst=%d\n", i);
>  				ret = -EINVAL;
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 87e0b0782023..9a424a66c83a 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -218,7 +218,8 @@ static void print_bind_err_msg(struct thermal_zone_device *tz,
>  
>  static void __bind(struct thermal_zone_device *tz, int mask,
>  			struct thermal_cooling_device *cdev,
> -			unsigned long *limits)
> +			unsigned long *limits,
> +			unsigned int weight)
>  {
>  	int i, ret;
>  
> @@ -233,7 +234,8 @@ static void __bind(struct thermal_zone_device *tz, int mask,
>  				upper = limits[i * 2 + 1];
>  			}
>  			ret = thermal_zone_bind_cooling_device(tz, i, cdev,
> -							       upper, lower);
> +							       upper, lower,
> +							       weight);
>  			if (ret)
>  				print_bind_err_msg(tz, cdev, ret);
>  		}
> @@ -280,7 +282,8 @@ static void bind_cdev(struct thermal_cooling_device *cdev)
>  				continue;
>  			tzp->tbp[i].cdev = cdev;
>  			__bind(pos, tzp->tbp[i].trip_mask, cdev,
> -			       tzp->tbp[i].binding_limits);
> +			       tzp->tbp[i].binding_limits,
> +			       tzp->tbp[i].weight);
>  		}
>  	}
>  
> @@ -319,7 +322,8 @@ static void bind_tz(struct thermal_zone_device *tz)
>  				continue;
>  			tzp->tbp[i].cdev = pos;
>  			__bind(tz, tzp->tbp[i].trip_mask, pos,
> -			       tzp->tbp[i].binding_limits);
> +			       tzp->tbp[i].binding_limits,
> +			       tzp->tbp[i].weight);
>  		}
>  	}
>  exit:
> @@ -711,7 +715,8 @@ passive_store(struct device *dev, struct device_attribute *attr,
>  				thermal_zone_bind_cooling_device(tz,
>  						THERMAL_TRIPS_NONE, cdev,
>  						THERMAL_NO_LIMIT,
> -						THERMAL_NO_LIMIT);
> +						THERMAL_NO_LIMIT,
> +						THERMAL_WEIGHT_DEFAULT);
>  		}
>  		mutex_unlock(&thermal_list_lock);
>  		if (!tz->passive_delay)
> @@ -913,6 +918,9 @@ thermal_cooling_device_trip_point_show(struct device *dev,
>   * @lower:	the Minimum cooling state can be used for this trip point.
>   *		THERMAL_NO_LIMIT means no lower limit,
>   *		and the cooling device can be in cooling state 0.
> + * @weight:	The weight of the cooling device to be bound to the
> + *		thermal zone. Use THERMAL_WEIGHT_DEFAULT for the
> + *		default value
>   *
>   * This interface function bind a thermal cooling device to the certain trip
>   * point of a thermal zone device.
> @@ -923,7 +931,8 @@ thermal_cooling_device_trip_point_show(struct device *dev,
>  int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
>  				     int trip,
>  				     struct thermal_cooling_device *cdev,
> -				     unsigned long upper, unsigned long lower)
> +				     unsigned long upper, unsigned long lower,
> +				     unsigned int weight)
>  {
>  	struct thermal_instance *dev;
>  	struct thermal_instance *pos;
> @@ -968,6 +977,7 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
>  	dev->upper = upper;
>  	dev->lower = lower;
>  	dev->target = THERMAL_NO_TARGET;
> +	dev->weight = weight;
>  
>  	result = get_idr(&tz->idr, &tz->lock, &dev->id);
>  	if (result)
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index 0531c752fbbb..7a465e9d456c 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -48,6 +48,7 @@ struct thermal_instance {
>  	struct device_attribute attr;
>  	struct list_head tz_node; /* node in tz->thermal_instances */
>  	struct list_head cdev_node; /* node in cdev->thermal_instances */
> +	unsigned int weight; /* The weight of the cooling device */
>  };
>  
>  int thermal_register_governor(struct thermal_governor *);
> diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
> index 3fb054a10f6a..4638718fabf6 100644
> --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
> +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
> @@ -146,7 +146,8 @@ static int ti_thermal_bind(struct thermal_zone_device *thermal,
>  	return thermal_zone_bind_cooling_device(thermal, 0, cdev,
>  	/* bind with min and max states defined by cpu_cooling */
>  						THERMAL_NO_LIMIT,
> -						THERMAL_NO_LIMIT);
> +						THERMAL_NO_LIMIT,
> +						THERMAL_WEIGHT_DEFAULT);
>  }
>  
>  /* Unbind callback functions for thermal zone */
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index fc52e307efab..2ed7062fac1d 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -40,6 +40,9 @@
>  /* No upper/lower limit requirement */
>  #define THERMAL_NO_LIMIT	((u32)~0)
>  
> +/* Default weight of a bound cooling device */
> +#define THERMAL_WEIGHT_DEFAULT 0
> +
>  /* Unit conversion macros */
>  #define KELVIN_TO_CELSIUS(t)	(long)(((long)t-2732 >= 0) ?	\
>  				((long)t-2732+5)/10 : ((long)t-2732-5)/10)
> @@ -321,7 +324,8 @@ void thermal_zone_device_unregister(struct thermal_zone_device *);
>  
>  int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
>  				     struct thermal_cooling_device *,
> -				     unsigned long, unsigned long);
> +				     unsigned long, unsigned long,
> +				     unsigned 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 *);
> -- 
> 1.9.1
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/5] thermal: of: fix cooling device weights in device tree
  2015-01-31  0:00   ` Eduardo Valentin
@ 2015-02-01 12:21     ` Javi Merino
  0 siblings, 0 replies; 8+ messages in thread
From: Javi Merino @ 2015-02-01 12:21 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: linux-pm, Punit Agrawal, Kapileshwar Singh, Zhang Rui, Kukjin Kim

Hi Eduardo,

On Sat, Jan 31, 2015 at 12:00:19AM +0000, Eduardo Valentin wrote:
> On Thu, Jan 22, 2015 at 05:27:01PM +0000, Javi Merino wrote:
> > From: Kapileshwar Singh <kapileshwar.singh@arm.com>
> > 
> > Currently you can specify the weight of the cooling device in the device
> > tree but that information is not populated to the
> > thermal_bind_params where the fair share governor expects it to
> > be.  The of thermal zone device doesn't have a thermal_bind_params
> > structure and arguably it's better to pass the weight inside the
> > thermal_instance as it is specific to the bind of a cooling device to a
> > thermal zone parameter.
> > 
> > Core thermal code is fixed to populate the weight in the instance from
> > the thermal_bind_params, so platform code that was passing the weight
> > inside the thermal_bind_params continue to work seamlessly.
> > 
> > While we are at it, create a default value for the weight parameter for
> > those thermal zones that currently don't define it and remove the
> > hardcoded default in of-thermal.
> > 
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Eduardo Valentin <edubezval@gmail.com>
> > Cc: Kukjin Kim <kgene@kernel.org>
> > Signed-off-by: Kapileshwar Singh <kapileshwar.singh@arm.com>
> > ---
> > 
> > This patch was previously suggested here[0].  During the discussion,
> > Eduardo suggested that dropping the bind op for of-thermal and using
> > tbps and their .match() function was preferrable.  We sent a series
> > that did that in [1].  However, looking at the code we think that this
> > patch is preferrable as it is simpler and enables the rest of the
> > patches in these series, specially the simplification in patch 2.
> > That's why we are reposting it, now properly asking for inclusion.
> > 
> > [0] http://thread.gmane.org/gmane.linux.power-management.general/54622
> > [1] http://thread.gmane.org/gmane.linux.power-management.general/55339
> 
> Despite going one way or another, this patch breaks existing users of
> the thermal_zone_bind_cooling_device API.
> 
> > 
> >  Documentation/thermal/sysfs-api.txt                |  4 +++-
> >  drivers/thermal/db8500_thermal.c                   |  2 +-
> >  drivers/thermal/fair_share.c                       |  2 +-
> >  drivers/thermal/imx_thermal.c                      |  3 ++-
> >  drivers/thermal/of-thermal.c                       |  5 +++--
> >  drivers/thermal/samsung/exynos_thermal_common.c    |  3 ++-
> >  drivers/thermal/thermal_core.c                     | 22 ++++++++++++++++------
> >  drivers/thermal/thermal_core.h                     |  1 +
> >  drivers/thermal/ti-soc-thermal/ti-thermal-common.c |  3 ++-
> >  include/linux/thermal.h                            |  6 +++++-
> >  10 files changed, 36 insertions(+), 15 deletions(-)
> 
> $ git grep thermal_zone_bind_cooling_device
> Documentation/thermal/sysfs-api.txt:1.3.1 int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
> Documentation/thermal/sysfs-api.txt:thermal_zone_bind_cooling_device/thermal_zone_unbind_cooling_device.
> drivers/acpi/thermal.c:					thermal_zone_bind_cooling_device
> drivers/acpi/thermal.c:				result = thermal_zone_bind_cooling_device
> drivers/acpi/thermal.c:				result = thermal_zone_bind_cooling_device
> 
> We are not supposed to break ACPI code base :-)
> 
> drivers/platform/x86/acerhdf.c:	if (thermal_zone_bind_cooling_device(thermal, 0, cdev,
> 
> You may want to add acerhdf to your patch.
 
Yes, we'll send a v2 with ACPI and acerhdf.

Cheers,
Javi

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-02-01 12:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-22 17:27 [PATCH 0/5] Various fixes to weights in the thermal framework Javi Merino
2015-01-22 17:27 ` [PATCH 1/5] thermal: of: fix cooling device weights in device tree Javi Merino
2015-01-31  0:00   ` Eduardo Valentin
2015-02-01 12:21     ` Javi Merino
2015-01-22 17:27 ` [PATCH 2/5] thermal: fair_share: use the weight from the thermal instance Javi Merino
2015-01-22 17:27 ` [PATCH 3/5] thermal: fair_share: fix typo Javi Merino
2015-01-22 17:27 ` [PATCH 4/5] thermal: export weight to sysfs Javi Merino
2015-01-22 17:27 ` [PATCH 5/5] thermal: fair_share: generalize the weight concept Javi Merino

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.