linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] thermal: of: Fix for cooling devices
@ 2015-01-13 17:51 Kapileshwar Singh
  2015-01-13 17:51 ` [PATCH 1/2] thermal: of: Match function to pass bindparam index Kapileshwar Singh
  2015-01-13 17:51 ` [PATCH 2/2] thermal: of: Match function for of-thermal Kapileshwar Singh
  0 siblings, 2 replies; 5+ messages in thread
From: Kapileshwar Singh @ 2015-01-13 17:51 UTC (permalink / raw)
  To: linux-pm; +Cc: edubezval, rui.zhang, javi.merino, punit.agrawal

This set of patches aims at solving the problem 
due to which the thermal_zone_params are not populated in 
of-thermal. This results in the weight parameter not being 
propagated to the governor correctly.

We solve this problem by introducing a match function to 
replace the ops->bind and also by correctly populating the
thermal_bind_params and thermal_zone_params which involves
parsing the __thermal_bind_params and creating a parsed version
as an internal data structure) which optimizes the match callback.

Kapileshwar Singh (2):
  thermal: of: Match function to pass bindparam index
  thermal: of: Match function for of-thermal

 drivers/thermal/of-thermal.c   |  340 ++++++++++++++++++++++++++++++++--------
 drivers/thermal/thermal_core.c |    4 +-
 include/linux/thermal.h        |    2 +-
 3 files changed, 281 insertions(+), 65 deletions(-)

-- 
1.7.9.5



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

* [PATCH 1/2] thermal: of: Match function to pass bindparam index
  2015-01-13 17:51 [PATCH 0/2] thermal: of: Fix for cooling devices Kapileshwar Singh
@ 2015-01-13 17:51 ` Kapileshwar Singh
  2015-01-20 13:44   ` Eduardo Valentin
  2015-01-13 17:51 ` [PATCH 2/2] thermal: of: Match function for of-thermal Kapileshwar Singh
  1 sibling, 1 reply; 5+ messages in thread
From: Kapileshwar Singh @ 2015-01-13 17:51 UTC (permalink / raw)
  To: linux-pm; +Cc: edubezval, rui.zhang, javi.merino, punit.agrawal

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

The match function should pass the index of the binding parameters
which the cooling device needs to be matched against. There are currently
no implementations of match function within the kernel. A successful
match requires:

	trip_mask == expected_trip_mask;
	weight == expected_weight;
	binding_limits == expected_binding_limits;
	thermal_zone == expected_thermal_zone;

Signed-off-by: Kapileshwar Singh <kapileshwar.singh@arm.com>
---
 drivers/thermal/thermal_core.c |    4 ++--
 include/linux/thermal.h        |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 87e0b0782023..db4d6407c1ec 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -276,7 +276,7 @@ static void bind_cdev(struct thermal_cooling_device *cdev)
 		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))
+			if (tzp->tbp[i].match(pos, cdev, i))
 				continue;
 			tzp->tbp[i].cdev = cdev;
 			__bind(pos, tzp->tbp[i].trip_mask, cdev,
@@ -315,7 +315,7 @@ static void bind_tz(struct thermal_zone_device *tz)
 		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))
+			if (tzp->tbp[i].match(tz, pos, i))
 				continue;
 			tzp->tbp[i].cdev = pos;
 			__bind(tz, tzp->tbp[i].trip_mask, pos,
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index fc52e307efab..dc8cf6dc59e5 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -237,7 +237,7 @@ struct thermal_bind_params {
 	 */
 	unsigned long *binding_limits;
 	int (*match) (struct thermal_zone_device *tz,
-			struct thermal_cooling_device *cdev);
+		      struct thermal_cooling_device *cdev, int index);
 };
 
 /* Structure to define Thermal Zone parameters */
-- 
1.7.9.5



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

* [PATCH 2/2] thermal: of: Match function for of-thermal
  2015-01-13 17:51 [PATCH 0/2] thermal: of: Fix for cooling devices Kapileshwar Singh
  2015-01-13 17:51 ` [PATCH 1/2] thermal: of: Match function to pass bindparam index Kapileshwar Singh
@ 2015-01-13 17:51 ` Kapileshwar Singh
  2015-01-20 13:57   ` Eduardo Valentin
  1 sibling, 1 reply; 5+ messages in thread
From: Kapileshwar Singh @ 2015-01-13 17:51 UTC (permalink / raw)
  To: linux-pm; +Cc: edubezval, rui.zhang, javi.merino, punit.agrawal

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

This patch introduces the following changes:

* Creation of a parsed_bind_params data structure to
  uniquely identify the bind parameters per coolig
  device and optimize the match callback

* Adding a match call-back to of-thermal to replace
  the specific bind operation

* In the previous implementation the thermal_zone_params
  and thermal_bind_params are not populated and the weight
  parameter which is read from the device tree
  (as contribution) does not get propagated to the governor

Signed-off-by: Kapileshwar Singh <kapileshwar.singh@arm.com>
---
 drivers/thermal/of-thermal.c |  340 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 278 insertions(+), 62 deletions(-)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index e145b66df444..82f7e4b48845 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -31,6 +31,7 @@
 #include <linux/export.h>
 #include <linux/string.h>
 #include <linux/thermal.h>
+#include <linux/bitops.h>
 
 #include "thermal_core.h"
 
@@ -54,6 +55,26 @@ struct __thermal_bind_params {
 };
 
 /**
+ * struct parsed_bind_params - parsed bind parameters from device tree
+ * @cdev_node: a pointer to identify the device tree node of the cdev
+ * @trip_mask: a mask of all the trips the cdev is to be bound to
+ * @weight: the percentage (0 to 100) of cooling conrtibution
+ * @binding_limits: the max and min limits for each trip point
+ *
+ * The struct __thermal_bind_params is a raw representation of the
+ * data read from the device tree. This is then parsed into this
+ * struct such that there is only on param per cooling device
+ * and can be correlated efficiently with thermal_bind_params.
+ */
+
+struct parsed_bind_params {
+	struct device_node *cdev_node;
+	unsigned long trip_mask;
+	unsigned int weight;
+	unsigned long *binding_limits;
+};
+
+/**
  * struct __thermal_zone - internal representation of a thermal zone
  * @mode: current thermal zone device mode (enabled/disabled)
  * @passive_delay: polling interval while passive cooling is activated
@@ -78,6 +99,8 @@ struct __thermal_zone {
 	/* cooling binding data */
 	int num_tbps;
 	struct __thermal_bind_params *tbps;
+	struct parsed_bind_params *pbs;
+	int num_parsed_tbps;
 
 	/* sensor interface */
 	void *sensor_data;
@@ -208,60 +231,6 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip,
 	return 0;
 }
 
-static int of_thermal_bind(struct thermal_zone_device *thermal,
-			   struct thermal_cooling_device *cdev)
-{
-	struct __thermal_zone *data = thermal->devdata;
-	int i;
-
-	if (!data || IS_ERR(data))
-		return -ENODEV;
-
-	/* find where to bind */
-	for (i = 0; i < data->num_tbps; i++) {
-		struct __thermal_bind_params *tbp = data->tbps + i;
-
-		if (tbp->cooling_device == cdev->np) {
-			int ret;
-
-			ret = thermal_zone_bind_cooling_device(thermal,
-						tbp->trip_id, cdev,
-						tbp->max,
-						tbp->min);
-			if (ret)
-				return ret;
-		}
-	}
-
-	return 0;
-}
-
-static int of_thermal_unbind(struct thermal_zone_device *thermal,
-			     struct thermal_cooling_device *cdev)
-{
-	struct __thermal_zone *data = thermal->devdata;
-	int i;
-
-	if (!data || IS_ERR(data))
-		return -ENODEV;
-
-	/* find where to unbind */
-	for (i = 0; i < data->num_tbps; i++) {
-		struct __thermal_bind_params *tbp = data->tbps + i;
-
-		if (tbp->cooling_device == cdev->np) {
-			int ret;
-
-			ret = thermal_zone_unbind_cooling_device(thermal,
-						tbp->trip_id, cdev);
-			if (ret)
-				return ret;
-		}
-	}
-
-	return 0;
-}
-
 static int of_thermal_get_mode(struct thermal_zone_device *tz,
 			       enum thermal_device_mode *mode)
 {
@@ -384,9 +353,6 @@ static struct thermal_zone_device_ops of_thermal_ops = {
 	.get_trip_hyst = of_thermal_get_trip_hyst,
 	.set_trip_hyst = of_thermal_set_trip_hyst,
 	.get_crit_temp = of_thermal_get_crit_temp,
-
-	.bind = of_thermal_bind,
-	.unbind = of_thermal_unbind,
 };
 
 /***   sensor API   ***/
@@ -621,6 +587,237 @@ static int thermal_of_populate_bind_params(struct device_node *np,
 	return ret;
 }
 
+int of_thermal_match_bind_param(struct thermal_zone_device *tz,
+				struct thermal_cooling_device *cdev,
+				int index)
+{
+	struct __thermal_zone *__tz;
+	struct parsed_bind_params *pbs;
+	struct thermal_bind_params *tbp;
+	int i;
+
+	if (!tz->devdata)
+		return 1;
+
+	__tz = (struct __thermal_zone *)tz->devdata;
+
+	if (!__tz->pbs || !__tz->num_parsed_tbps)
+		return 1;
+
+	pbs = __tz->pbs;
+	tbp = tz->tzp->tbp;
+
+	for (i = 0; i < __tz->num_parsed_tbps; i++) {
+		if (pbs[i].cdev_node == cdev->np) {
+			if (tbp[index].trip_mask != pbs[i].trip_mask)
+				return 1;
+			if (tbp[index].binding_limits != pbs[i].binding_limits)
+				return 1;
+			if (tbp[index].weight != pbs[i].weight)
+				return 1;
+			return 0;
+		}
+	}
+	return 1;
+}
+
+/**
+ * get_unique_mask - return a mask indicating repeated cdevs
+ * @__tbp: __thermal_bind_params internal data structre
+ * @num_tbps: total number of cdev<->trip bindings
+ *
+ * A cooling device may be bound to more than one
+ * thermal trips. These multiple bindings are populated as
+ * separate entries in the @__tbp params internal data structure
+ * from the device tree. The unique mask identifies the location
+ * of re-ocurring cooling devices which is further used to
+ * populate the thermal_bind_params external data structre.
+ *
+ * Return: the calculated bitmask (long)
+	   (set bit means a non-unique cdev at that index)
+ */
+static unsigned long get_unique_mask(struct __thermal_bind_params *__tbp,
+			       unsigned int num_tbps)
+{
+	unsigned long unique_mask = 0;
+	int i, j;
+	/*
+	 * A bit set in the mask means that the cooling device
+	 * at that position is not unique
+	 */
+	for (i = 0; i < num_tbps; i++)
+		for (j = i + 1; j < num_tbps; j++)
+			if (!test_bit(j, &unique_mask) &&
+			   (__tbp[i].cooling_device == __tbp[j].cooling_device))
+				set_bit(j, &unique_mask);
+
+	return  unique_mask;
+}
+
+static void fill_parsed_params(struct parsed_bind_params *pbs,
+				struct __thermal_bind_params *__tbp,
+				int unique)
+{
+	pbs->binding_limits[2 * __tbp->trip_id] = __tbp->min;
+	pbs->binding_limits[2 * __tbp->trip_id + 1] = __tbp->max;
+
+	if (unique) {
+		pbs->weight = __tbp->usage;
+		pbs->trip_mask = (1  << __tbp->trip_id);
+		pbs->cdev_node = __tbp->cooling_device;
+	} else {
+		if (pbs->weight != __tbp->usage)
+			pr_err("Multiple weights (%u, %u) sepcified for cdev %s",
+			       pbs->weight, __tbp->usage, pbs->cdev_node->name);
+		pbs->trip_mask |= (1 << __tbp->trip_id);
+	}
+}
+
+/**
+ * of_thermal_parse_bind_params - parse the populated bind params
+ * @__tz: __thermal_zone private device data for of-thermal
+ *
+ * This function creates a reflection of the thermal_bind_params
+ * data structure and condenses the cooling-map populated from the
+ * device tree. This structure is then used when the match
+ * callback of_thermal_match_bind_param is invoked.
+ *
+ * Return: parsed_bind_parameters structure
+ */
+static struct parsed_bind_params*
+of_thermal_parse_bind_params(struct __thermal_zone *__tz)
+{
+	struct parsed_bind_params *pbs;
+	struct __thermal_bind_params *__tbp = __tz->tbps;
+	unsigned long unique_mask;
+	unsigned long *limits;
+	unsigned int num_parsed;
+	int i, j, err;
+	int bind_count = 0;
+
+	unique_mask = get_unique_mask(__tbp, __tz->num_tbps);
+	num_parsed = __tz->num_tbps - hweight_long(unique_mask);
+	__tz->num_parsed_tbps = num_parsed;
+
+	pbs = kcalloc(num_parsed, sizeof(*pbs), GFP_KERNEL);
+	if (!pbs) {
+		err = -ENOMEM;
+		goto err_exit;
+	}
+
+	/* We have a number of trips in tz */
+	for (i = 0; i < __tz->num_tbps; i++) {
+
+		/*
+		 * We have two cases here :
+		 * First occurence of the cooling device
+		 * In this case we need to allocate a new binding_limits array
+		 * and assign the limits ( __tbp[i].min and __tbp[i].max )
+		 * and set the bit in the trip mask
+		 *
+		 * Repeated occurence of the cooling device:
+		 * In this case we need to find the previously allocated
+		 * binding_param and update the binding_limits and trip_mask.
+		 */
+
+		int unique = !test_bit(i, &unique_mask);
+
+		if (unique) {
+			pbs[bind_count].weight = __tbp[i].usage;
+			limits = kcalloc(2 * __tz->ntrips, sizeof(*limits),
+					 GFP_KERNEL);
+			if (!limits) {
+				err = -ENOMEM;
+				goto free_bp;
+			}
+			pbs[bind_count].binding_limits = limits;
+			fill_parsed_params(&pbs[bind_count], &__tbp[i],
+					     unique);
+			bind_count++;
+		} else {
+			struct device_node *curr;
+			struct device_node *prev;
+
+			for (j = 0; j < bind_count; j++) {
+				curr = __tbp[i].cooling_device;
+				prev = pbs[j].cdev_node;
+				if (curr == prev) {
+					fill_parsed_params(&pbs[j],
+							   &__tbp[i],
+							   unique);
+					break;
+				}
+			}
+		}
+
+	}
+
+	return pbs;
+
+free_bp:
+	for (i = 0; i < num_parsed; i++)
+		kfree(pbs[i].binding_limits);
+	kfree(pbs);
+
+err_exit:
+	return ERR_PTR(err);
+
+}
+
+/**
+ * of_thermal_populate_tzp - populate the thermal zone params
+ * @__tz: __thermal_zone private device data for of-thermal
+ *
+ * This function populates the thermal_zone_params and also the
+ * tzp->tbp based on the parsed_bind_params (__tz->pbs)
+ *
+ * Return: struct thermal_zone params
+ */
+static struct thermal_zone_params*
+of_thermal_populate_tzp(struct __thermal_zone *__tz)
+{
+
+	struct thermal_zone_params *tzp;
+	struct parsed_bind_params *pbs = __tz->pbs;
+	struct thermal_bind_params *tbp;
+	int err;
+	int i;
+
+	tzp = kzalloc(sizeof(*tzp), GFP_KERNEL);
+	if (!tzp)
+		return ERR_PTR(-ENOMEM);
+
+	if (!pbs || !__tz->num_parsed_tbps) {
+		err = -ENODEV;
+		goto free_tzp;
+	}
+
+	tbp = kcalloc(__tz->num_parsed_tbps, sizeof(*tbp), GFP_KERNEL);
+	if (!tbp) {
+		err = -ENOMEM;
+		goto free_tzp;
+	 }
+
+	for (i = 0; i < __tz->num_parsed_tbps; i++) {
+		tbp[i].weight = pbs[i].weight;
+		tbp[i].binding_limits = pbs[i].binding_limits;
+		tbp[i].trip_mask = pbs[i].trip_mask;
+		tbp[i].match = of_thermal_match_bind_param;
+	}
+
+	tzp->tbp = tbp;
+	tzp->num_tbps = __tz->num_parsed_tbps;
+
+	/* No hwmon because there might be hwmon drivers registering */
+	tzp->no_hwmon = true;
+
+	return tzp;
+
+free_tzp:
+	kfree(tzp);
+	return ERR_PTR(err);
+}
+
 /**
  * It maps 'enum thermal_trip_type' found in include/linux/thermal.h
  * into the device tree binding of 'trip', property type.
@@ -800,6 +997,12 @@ thermal_of_build_thermal_zone(struct device_node *np)
 			goto free_tbps;
 	}
 
+	tz->pbs = of_thermal_parse_bind_params(tz);
+	if (IS_ERR(tz->pbs)) {
+		ret = -ENOMEM;
+		goto free_tbps;
+	}
+
 finish:
 	of_node_put(child);
 	tz->mode = THERMAL_DEVICE_DISABLED;
@@ -829,6 +1032,8 @@ static inline void of_thermal_free_zone(struct __thermal_zone *tz)
 	for (i = 0; i < tz->num_tbps; i++)
 		of_node_put(tz->tbps[i].cooling_device);
 	kfree(tz->tbps);
+	/* Free the parsed_bind_params */
+	kfree(tz->pbs);
 	for (i = 0; i < tz->ntrips; i++)
 		of_node_put(tz->trips[i].np);
 	kfree(tz->trips);
@@ -879,15 +1084,14 @@ int __init of_parse_thermal_zones(void)
 		if (!ops)
 			goto exit_free;
 
-		tzp = kzalloc(sizeof(*tzp), GFP_KERNEL);
-		if (!tzp) {
+
+		tzp = of_thermal_populate_tzp(tz);
+
+		if (IS_ERR(tzp)) {
 			kfree(ops);
 			goto exit_free;
 		}
 
-		/* No hwmon because there might be hwmon drivers registering */
-		tzp->no_hwmon = true;
-
 		zone = thermal_zone_device_register(child->name, tz->ntrips,
 						    0, tz,
 						    ops, tzp,
@@ -936,6 +1140,7 @@ void of_thermal_destroy_zones(void)
 
 	for_each_child_of_node(np, child) {
 		struct thermal_zone_device *zone;
+		int i;
 
 		/* Check whether child is enabled or not */
 		if (!of_device_is_available(child))
@@ -946,6 +1151,17 @@ void of_thermal_destroy_zones(void)
 			continue;
 
 		thermal_zone_device_unregister(zone);
+		/*
+		 * free the binding_limits
+		 * free the thermal_bind_params
+		 */
+		if (zone->tzp && zone->tzp->tbp) {
+			const struct thermal_zone_params *tzp = zone->tzp;
+
+			for (i = 0; i < tzp->num_tbps; i++)
+				kfree(tzp->tbp[i].binding_limits);
+			kfree(tzp->tbp);
+		}
 		kfree(zone->tzp);
 		kfree(zone->ops);
 		of_thermal_free_zone(zone->devdata);
-- 
1.7.9.5



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

* Re: [PATCH 1/2] thermal: of: Match function to pass bindparam index
  2015-01-13 17:51 ` [PATCH 1/2] thermal: of: Match function to pass bindparam index Kapileshwar Singh
@ 2015-01-20 13:44   ` Eduardo Valentin
  0 siblings, 0 replies; 5+ messages in thread
From: Eduardo Valentin @ 2015-01-20 13:44 UTC (permalink / raw)
  To: Kapileshwar Singh; +Cc: linux-pm, rui.zhang, javi.merino, punit.agrawal

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

On Tue, Jan 13, 2015 at 05:51:21PM +0000, Kapileshwar Singh wrote:
> From: KP Singh <kapileshwar.singh@arm.com>

The encoding still seams to be scrambled to me.

> 
> The match function should pass the index of the binding parameters
> which the cooling device needs to be matched against. There are currently
> no implementations of match function within the kernel. A successful
> match requires:
> 
> 	trip_mask == expected_trip_mask;
> 	weight == expected_weight;
> 	binding_limits == expected_binding_limits;
> 	thermal_zone == expected_thermal_zone;

ok. The .match callback is documented under
Documentation/thermal/sysfs-api.txt. Can you please also make sure the
documentation is sane after your change?

> 
> Signed-off-by: Kapileshwar Singh <kapileshwar.singh@arm.com>
> ---
>  drivers/thermal/thermal_core.c |    4 ++--
>  include/linux/thermal.h        |    2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 87e0b0782023..db4d6407c1ec 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -276,7 +276,7 @@ static void bind_cdev(struct thermal_cooling_device *cdev)
>  		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))
> +			if (tzp->tbp[i].match(pos, cdev, i))
>  				continue;
>  			tzp->tbp[i].cdev = cdev;
>  			__bind(pos, tzp->tbp[i].trip_mask, cdev,
> @@ -315,7 +315,7 @@ static void bind_tz(struct thermal_zone_device *tz)
>  		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))
> +			if (tzp->tbp[i].match(tz, pos, i))
>  				continue;
>  			tzp->tbp[i].cdev = pos;
>  			__bind(tz, tzp->tbp[i].trip_mask, pos,
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index fc52e307efab..dc8cf6dc59e5 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -237,7 +237,7 @@ struct thermal_bind_params {
>  	 */
>  	unsigned long *binding_limits;
>  	int (*match) (struct thermal_zone_device *tz,
> -			struct thermal_cooling_device *cdev);
> +		      struct thermal_cooling_device *cdev, int index);
>  };
>  
>  /* Structure to define Thermal Zone parameters */
> -- 
> 1.7.9.5
> 
> 

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

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

* Re: [PATCH 2/2] thermal: of: Match function for of-thermal
  2015-01-13 17:51 ` [PATCH 2/2] thermal: of: Match function for of-thermal Kapileshwar Singh
@ 2015-01-20 13:57   ` Eduardo Valentin
  0 siblings, 0 replies; 5+ messages in thread
From: Eduardo Valentin @ 2015-01-20 13:57 UTC (permalink / raw)
  To: Kapileshwar Singh; +Cc: linux-pm, rui.zhang, javi.merino, punit.agrawal

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

Same here, enconding is not helping.

On Tue, Jan 13, 2015 at 05:51:22PM +0000, Kapileshwar Singh wrote:
> From: KP Singh <kapileshwar.singh@arm.com>
> 
> This patch introduces the following changes:
> 
> * Creation of a parsed_bind_params data structure to
>   uniquely identify the bind parameters per coolig

s/coolig/cooling

>   device and optimize the match callback
> 
> * Adding a match call-back to of-thermal to replace
>   the specific bind operation
> 
> * In the previous implementation the thermal_zone_params
>   and thermal_bind_params are not populated and the weight
>   parameter which is read from the device tree
>   (as contribution) does not get propagated to the governor

Is it possible to break this patch into smaller parts?


> 
> Signed-off-by: Kapileshwar Singh <kapileshwar.singh@arm.com>
> ---
>  drivers/thermal/of-thermal.c |  340 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 278 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index e145b66df444..82f7e4b48845 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -31,6 +31,7 @@
>  #include <linux/export.h>
>  #include <linux/string.h>
>  #include <linux/thermal.h>
> +#include <linux/bitops.h>
>  
>  #include "thermal_core.h"
>  
> @@ -54,6 +55,26 @@ struct __thermal_bind_params {
>  };
>  
>  /**
> + * struct parsed_bind_params - parsed bind parameters from device tree
> + * @cdev_node: a pointer to identify the device tree node of the cdev
> + * @trip_mask: a mask of all the trips the cdev is to be bound to
> + * @weight: the percentage (0 to 100) of cooling conrtibution
> + * @binding_limits: the max and min limits for each trip point
> + *
> + * The struct __thermal_bind_params is a raw representation of the
> + * data read from the device tree. This is then parsed into this
> + * struct such that there is only on param per cooling device

s/on/one

> + * and can be correlated efficiently with thermal_bind_params.
> + */
> +
> +struct parsed_bind_params {
> +	struct device_node *cdev_node;
> +	unsigned long trip_mask;
> +	unsigned int weight;
> +	unsigned long *binding_limits;
> +};
> +
> +/**
>   * struct __thermal_zone - internal representation of a thermal zone
>   * @mode: current thermal zone device mode (enabled/disabled)
>   * @passive_delay: polling interval while passive cooling is activated
> @@ -78,6 +99,8 @@ struct __thermal_zone {
>  	/* cooling binding data */
>  	int num_tbps;
>  	struct __thermal_bind_params *tbps;
> +	struct parsed_bind_params *pbs;
> +	int num_parsed_tbps;
>  
>  	/* sensor interface */
>  	void *sensor_data;
> @@ -208,60 +231,6 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip,
>  	return 0;
>  }
>  
> -static int of_thermal_bind(struct thermal_zone_device *thermal,
> -			   struct thermal_cooling_device *cdev)
> -{
> -	struct __thermal_zone *data = thermal->devdata;
> -	int i;
> -
> -	if (!data || IS_ERR(data))
> -		return -ENODEV;
> -
> -	/* find where to bind */
> -	for (i = 0; i < data->num_tbps; i++) {
> -		struct __thermal_bind_params *tbp = data->tbps + i;
> -
> -		if (tbp->cooling_device == cdev->np) {
> -			int ret;
> -
> -			ret = thermal_zone_bind_cooling_device(thermal,
> -						tbp->trip_id, cdev,
> -						tbp->max,
> -						tbp->min);
> -			if (ret)
> -				return ret;
> -		}
> -	}
> -
> -	return 0;
> -}
> -
> -static int of_thermal_unbind(struct thermal_zone_device *thermal,
> -			     struct thermal_cooling_device *cdev)
> -{
> -	struct __thermal_zone *data = thermal->devdata;
> -	int i;
> -
> -	if (!data || IS_ERR(data))
> -		return -ENODEV;
> -
> -	/* find where to unbind */
> -	for (i = 0; i < data->num_tbps; i++) {
> -		struct __thermal_bind_params *tbp = data->tbps + i;
> -
> -		if (tbp->cooling_device == cdev->np) {
> -			int ret;
> -
> -			ret = thermal_zone_unbind_cooling_device(thermal,
> -						tbp->trip_id, cdev);
> -			if (ret)
> -				return ret;
> -		}
> -	}
> -
> -	return 0;
> -}
> -
>  static int of_thermal_get_mode(struct thermal_zone_device *tz,
>  			       enum thermal_device_mode *mode)
>  {
> @@ -384,9 +353,6 @@ static struct thermal_zone_device_ops of_thermal_ops = {
>  	.get_trip_hyst = of_thermal_get_trip_hyst,
>  	.set_trip_hyst = of_thermal_set_trip_hyst,
>  	.get_crit_temp = of_thermal_get_crit_temp,
> -
> -	.bind = of_thermal_bind,
> -	.unbind = of_thermal_unbind,
>  };
>  
>  /***   sensor API   ***/
> @@ -621,6 +587,237 @@ static int thermal_of_populate_bind_params(struct device_node *np,
>  	return ret;
>  }
>  
> +int of_thermal_match_bind_param(struct thermal_zone_device *tz,
> +				struct thermal_cooling_device *cdev,
> +				int index)
> +{
> +	struct __thermal_zone *__tz;
> +	struct parsed_bind_params *pbs;
> +	struct thermal_bind_params *tbp;
> +	int i;
> +
> +	if (!tz->devdata)
> +		return 1;
> +
> +	__tz = (struct __thermal_zone *)tz->devdata;
> +
> +	if (!__tz->pbs || !__tz->num_parsed_tbps)
> +		return 1;
> +
> +	pbs = __tz->pbs;
> +	tbp = tz->tzp->tbp;
> +
> +	for (i = 0; i < __tz->num_parsed_tbps; i++) {
> +		if (pbs[i].cdev_node == cdev->np) {
> +			if (tbp[index].trip_mask != pbs[i].trip_mask)
> +				return 1;
> +			if (tbp[index].binding_limits != pbs[i].binding_limits)
> +				return 1;
> +			if (tbp[index].weight != pbs[i].weight)
> +				return 1;
> +			return 0;
> +		}
> +	}
> +	return 1;
> +}
> +
> +/**
> + * get_unique_mask - return a mask indicating repeated cdevs
> + * @__tbp: __thermal_bind_params internal data structre
> + * @num_tbps: total number of cdev<->trip bindings
> + *
> + * A cooling device may be bound to more than one
> + * thermal trips. These multiple bindings are populated as
> + * separate entries in the @__tbp params internal data structure
> + * from the device tree. The unique mask identifies the location
> + * of re-ocurring cooling devices which is further used to
> + * populate the thermal_bind_params external data structre.
> + *
> + * Return: the calculated bitmask (long)
> +	   (set bit means a non-unique cdev at that index)
> + */
> +static unsigned long get_unique_mask(struct __thermal_bind_params *__tbp,
> +			       unsigned int num_tbps)
> +{
> +	unsigned long unique_mask = 0;
> +	int i, j;
> +	/*
> +	 * A bit set in the mask means that the cooling device
> +	 * at that position is not unique
> +	 */
> +	for (i = 0; i < num_tbps; i++)
> +		for (j = i + 1; j < num_tbps; j++)
> +			if (!test_bit(j, &unique_mask) &&
> +			   (__tbp[i].cooling_device == __tbp[j].cooling_device))
> +				set_bit(j, &unique_mask);
> +
> +	return  unique_mask;
> +}
> +
> +static void fill_parsed_params(struct parsed_bind_params *pbs,
> +				struct __thermal_bind_params *__tbp,
> +				int unique)
> +{
> +	pbs->binding_limits[2 * __tbp->trip_id] = __tbp->min;
> +	pbs->binding_limits[2 * __tbp->trip_id + 1] = __tbp->max;
> +
> +	if (unique) {
> +		pbs->weight = __tbp->usage;
> +		pbs->trip_mask = (1  << __tbp->trip_id);
> +		pbs->cdev_node = __tbp->cooling_device;
> +	} else {
> +		if (pbs->weight != __tbp->usage)
> +			pr_err("Multiple weights (%u, %u) sepcified for cdev %s",
> +			       pbs->weight, __tbp->usage, pbs->cdev_node->name);
> +		pbs->trip_mask |= (1 << __tbp->trip_id);
> +	}
> +}
> +
> +/**
> + * of_thermal_parse_bind_params - parse the populated bind params
> + * @__tz: __thermal_zone private device data for of-thermal
> + *
> + * This function creates a reflection of the thermal_bind_params
> + * data structure and condenses the cooling-map populated from the
> + * device tree. This structure is then used when the match
> + * callback of_thermal_match_bind_param is invoked.
> + *
> + * Return: parsed_bind_parameters structure
> + */
> +static struct parsed_bind_params*
> +of_thermal_parse_bind_params(struct __thermal_zone *__tz)
> +{
> +	struct parsed_bind_params *pbs;
> +	struct __thermal_bind_params *__tbp = __tz->tbps;
> +	unsigned long unique_mask;
> +	unsigned long *limits;
> +	unsigned int num_parsed;
> +	int i, j, err;
> +	int bind_count = 0;
> +
> +	unique_mask = get_unique_mask(__tbp, __tz->num_tbps);
> +	num_parsed = __tz->num_tbps - hweight_long(unique_mask);
> +	__tz->num_parsed_tbps = num_parsed;
> +
> +	pbs = kcalloc(num_parsed, sizeof(*pbs), GFP_KERNEL);
> +	if (!pbs) {
> +		err = -ENOMEM;
> +		goto err_exit;
> +	}
> +
> +	/* We have a number of trips in tz */
> +	for (i = 0; i < __tz->num_tbps; i++) {
> +
> +		/*
> +		 * We have two cases here :
> +		 * First occurence of the cooling device
> +		 * In this case we need to allocate a new binding_limits array
> +		 * and assign the limits ( __tbp[i].min and __tbp[i].max )
> +		 * and set the bit in the trip mask
> +		 *
> +		 * Repeated occurence of the cooling device:
> +		 * In this case we need to find the previously allocated
> +		 * binding_param and update the binding_limits and trip_mask.
> +		 */
> +
> +		int unique = !test_bit(i, &unique_mask);
> +
> +		if (unique) {
> +			pbs[bind_count].weight = __tbp[i].usage;
> +			limits = kcalloc(2 * __tz->ntrips, sizeof(*limits),
> +					 GFP_KERNEL);
> +			if (!limits) {
> +				err = -ENOMEM;
> +				goto free_bp;
> +			}
> +			pbs[bind_count].binding_limits = limits;
> +			fill_parsed_params(&pbs[bind_count], &__tbp[i],
> +					     unique);
> +			bind_count++;
> +		} else {
> +			struct device_node *curr;
> +			struct device_node *prev;
> +
> +			for (j = 0; j < bind_count; j++) {
> +				curr = __tbp[i].cooling_device;
> +				prev = pbs[j].cdev_node;
> +				if (curr == prev) {
> +					fill_parsed_params(&pbs[j],
> +							   &__tbp[i],
> +							   unique);
> +					break;
> +				}
> +			}
> +		}
> +
> +	}
> +
> +	return pbs;
> +
> +free_bp:
> +	for (i = 0; i < num_parsed; i++)
> +		kfree(pbs[i].binding_limits);
> +	kfree(pbs);
> +
> +err_exit:
> +	return ERR_PTR(err);
> +
> +}
> +
> +/**
> + * of_thermal_populate_tzp - populate the thermal zone params
> + * @__tz: __thermal_zone private device data for of-thermal
> + *
> + * This function populates the thermal_zone_params and also the
> + * tzp->tbp based on the parsed_bind_params (__tz->pbs)
> + *
> + * Return: struct thermal_zone params
> + */
> +static struct thermal_zone_params*
> +of_thermal_populate_tzp(struct __thermal_zone *__tz)
> +{
> +
> +	struct thermal_zone_params *tzp;
> +	struct parsed_bind_params *pbs = __tz->pbs;
> +	struct thermal_bind_params *tbp;
> +	int err;
> +	int i;
> +
> +	tzp = kzalloc(sizeof(*tzp), GFP_KERNEL);
> +	if (!tzp)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (!pbs || !__tz->num_parsed_tbps) {
> +		err = -ENODEV;
> +		goto free_tzp;
> +	}
> +
> +	tbp = kcalloc(__tz->num_parsed_tbps, sizeof(*tbp), GFP_KERNEL);
> +	if (!tbp) {
> +		err = -ENOMEM;
> +		goto free_tzp;
> +	 }
> +
> +	for (i = 0; i < __tz->num_parsed_tbps; i++) {
> +		tbp[i].weight = pbs[i].weight;
> +		tbp[i].binding_limits = pbs[i].binding_limits;
> +		tbp[i].trip_mask = pbs[i].trip_mask;
> +		tbp[i].match = of_thermal_match_bind_param;
> +	}
> +
> +	tzp->tbp = tbp;
> +	tzp->num_tbps = __tz->num_parsed_tbps;
> +
> +	/* No hwmon because there might be hwmon drivers registering */
> +	tzp->no_hwmon = true;
> +
> +	return tzp;
> +
> +free_tzp:
> +	kfree(tzp);
> +	return ERR_PTR(err);
> +}
> +
>  /**
>   * It maps 'enum thermal_trip_type' found in include/linux/thermal.h
>   * into the device tree binding of 'trip', property type.
> @@ -800,6 +997,12 @@ thermal_of_build_thermal_zone(struct device_node *np)
>  			goto free_tbps;
>  	}
>  
> +	tz->pbs = of_thermal_parse_bind_params(tz);
> +	if (IS_ERR(tz->pbs)) {
> +		ret = -ENOMEM;
> +		goto free_tbps;
> +	}
> +
>  finish:
>  	of_node_put(child);
>  	tz->mode = THERMAL_DEVICE_DISABLED;
> @@ -829,6 +1032,8 @@ static inline void of_thermal_free_zone(struct __thermal_zone *tz)
>  	for (i = 0; i < tz->num_tbps; i++)
>  		of_node_put(tz->tbps[i].cooling_device);
>  	kfree(tz->tbps);
> +	/* Free the parsed_bind_params */
> +	kfree(tz->pbs);
>  	for (i = 0; i < tz->ntrips; i++)
>  		of_node_put(tz->trips[i].np);
>  	kfree(tz->trips);
> @@ -879,15 +1084,14 @@ int __init of_parse_thermal_zones(void)
>  		if (!ops)
>  			goto exit_free;
>  
> -		tzp = kzalloc(sizeof(*tzp), GFP_KERNEL);
> -		if (!tzp) {
> +
> +		tzp = of_thermal_populate_tzp(tz);
> +
> +		if (IS_ERR(tzp)) {
>  			kfree(ops);
>  			goto exit_free;
>  		}
>  
> -		/* No hwmon because there might be hwmon drivers registering */
> -		tzp->no_hwmon = true;
> -
>  		zone = thermal_zone_device_register(child->name, tz->ntrips,
>  						    0, tz,
>  						    ops, tzp,
> @@ -936,6 +1140,7 @@ void of_thermal_destroy_zones(void)
>  
>  	for_each_child_of_node(np, child) {
>  		struct thermal_zone_device *zone;
> +		int i;
>  
>  		/* Check whether child is enabled or not */
>  		if (!of_device_is_available(child))
> @@ -946,6 +1151,17 @@ void of_thermal_destroy_zones(void)
>  			continue;
>  
>  		thermal_zone_device_unregister(zone);
> +		/*
> +		 * free the binding_limits
> +		 * free the thermal_bind_params
> +		 */
> +		if (zone->tzp && zone->tzp->tbp) {
> +			const struct thermal_zone_params *tzp = zone->tzp;
> +
> +			for (i = 0; i < tzp->num_tbps; i++)
> +				kfree(tzp->tbp[i].binding_limits);
> +			kfree(tzp->tbp);
> +		}
>  		kfree(zone->tzp);
>  		kfree(zone->ops);
>  		of_thermal_free_zone(zone->devdata);
> -- 
> 1.7.9.5
> 
> 

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

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

end of thread, other threads:[~2015-01-20 13:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-13 17:51 [PATCH 0/2] thermal: of: Fix for cooling devices Kapileshwar Singh
2015-01-13 17:51 ` [PATCH 1/2] thermal: of: Match function to pass bindparam index Kapileshwar Singh
2015-01-20 13:44   ` Eduardo Valentin
2015-01-13 17:51 ` [PATCH 2/2] thermal: of: Match function for of-thermal Kapileshwar Singh
2015-01-20 13:57   ` Eduardo Valentin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).