linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] dt: thermal: Fix broken cooling-maps
@ 2018-07-05  5:09 Viresh Kumar
  2018-07-05  5:09 ` [PATCH 1/2] dt-bindings: thermal: Allow multiple devices to share cooling map Viresh Kumar
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Viresh Kumar @ 2018-07-05  5:09 UTC (permalink / raw)
  To: Zhang Rui, Eduardo Valentin, robh, Wei Xu
  Cc: Viresh Kumar, linux-pm, Vincent Guittot, Daniel Lezcano,
	devicetree, olof, linux-arm-kernel, linux-kernel

Hi,

This is an attempt to fix the broken or partially defined DT bindings
for cooling-maps. We should list every device that participates in
cooling down at a certain trip point, instead of just the first in the
list as that depends on certain ordering of events to work properly.

The first patch extends the binding to allow a list of phandles in
"cooling-device" property and the second patch fixes one of the
platform's DT.

This will be followed up by fixing all platform DT bindings that have
these issues after this set is accepted.

The kernel also requires some changes to handle the phandle list, but
wouldn't break with these changes as it reads the first phandle in the
list for now. We can update that separately.

--
viresh

Viresh Kumar (2):
  dt-bindings: thermal: Allow multiple devices to share cooling map
  arm64: dts: hi6220: Add all CPUs in cooling maps

 Documentation/devicetree/bindings/thermal/thermal.txt | 11 +++--------
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi             |  9 ++++++++-
 2 files changed, 11 insertions(+), 9 deletions(-)

-- 
2.18.0.rc1.242.g61856ae69a2c


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

* [PATCH 1/2] dt-bindings: thermal: Allow multiple devices to share cooling map
  2018-07-05  5:09 [PATCH 0/2] dt: thermal: Fix broken cooling-maps Viresh Kumar
@ 2018-07-05  5:09 ` Viresh Kumar
  2018-07-16  4:34   ` Viresh Kumar
  2018-07-16 22:02   ` Rob Herring
  2018-07-05  5:09 ` [PATCH 2/2] arm64: dts: hi6220: Add all CPUs in cooling maps Viresh Kumar
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 17+ messages in thread
From: Viresh Kumar @ 2018-07-05  5:09 UTC (permalink / raw)
  To: Zhang Rui, Eduardo Valentin, robh
  Cc: Viresh Kumar, linux-pm, Vincent Guittot, Daniel Lezcano,
	devicetree, olof, linux-kernel

Allow cooling devices sharing same trip point with same contribution
value to share the cooling map as well. Otherwise the same information
will be duplicated for each device sharing the trip point.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 Documentation/devicetree/bindings/thermal/thermal.txt | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
index cc553f0952c5..eb7ee91556a5 100644
--- a/Documentation/devicetree/bindings/thermal/thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/thermal.txt
@@ -97,8 +97,8 @@ get assigned to trip points of the zone. The cooling devices are expected
 to be loaded in the target system.
 
 Required properties:
-- cooling-device:	A phandle of a cooling device with its specifier,
-  Type: phandle +	referring to which cooling device is used in this
+- cooling-device:	A list of phandles of cooling devices with their specifiers,
+  Type: phandle +	referring to which cooling devices are used in this
     cooling specifier	binding. In the cooling specifier, the first cell
 			is the minimum cooling state and the second cell
 			is the maximum cooling state used in this map.
@@ -276,12 +276,7 @@ thermal-zones {
 			};
 			map1 {
 				trip = <&cpu_alert1>;
-				cooling-device = <&fan0 5 THERMAL_NO_LIMIT>;
-			};
-			map2 {
-				trip = <&cpu_alert1>;
-				cooling-device =
-				    <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				cooling-device = <&fan0 5 THERMAL_NO_LIMIT>, <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
 			};
 		};
 	};
-- 
2.18.0.rc1.242.g61856ae69a2c


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

* [PATCH 2/2] arm64: dts: hi6220: Add all CPUs in cooling maps
  2018-07-05  5:09 [PATCH 0/2] dt: thermal: Fix broken cooling-maps Viresh Kumar
  2018-07-05  5:09 ` [PATCH 1/2] dt-bindings: thermal: Allow multiple devices to share cooling map Viresh Kumar
@ 2018-07-05  5:09 ` Viresh Kumar
  2018-07-05  8:44   ` Daniel Lezcano
  2018-07-17 10:54 ` [PATCH] of: thermal: Allow multiple devices to share cooling map Viresh Kumar
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Viresh Kumar @ 2018-07-05  5:09 UTC (permalink / raw)
  To: Zhang Rui, Eduardo Valentin, robh, Wei Xu
  Cc: Viresh Kumar, linux-pm, Vincent Guittot, Daniel Lezcano,
	devicetree, olof, linux-arm-kernel, linux-kernel

Each CPU can (and does) participate in cooling down the system but the
DT only captures the CPU0 in the cooling maps. Things work by chance as
under normal circumstances its the CPU0 which is used by the operating
systems to probe the cooling devices. But as soon as that ordering
changes and any other CPU is used to bring up the cooling device, we
will start seeing errors.

On the other hand, the hardware is partially defined in DT in these
cases and we must do a better job by capturing all devices.

Add all devices (CPUs here) in the cooling maps which are also affected
by the trip point.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 247024df714f..919d36b91bf3 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -886,7 +886,14 @@
 				cooling-maps {
 					map0 {
 						trip = <&target>;
-						cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+						cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+								 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+								 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+								 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+								 <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+								 <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+								 <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+								 <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
 					};
 				};
 			};
-- 
2.18.0.rc1.242.g61856ae69a2c


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

* Re: [PATCH 2/2] arm64: dts: hi6220: Add all CPUs in cooling maps
  2018-07-05  5:09 ` [PATCH 2/2] arm64: dts: hi6220: Add all CPUs in cooling maps Viresh Kumar
@ 2018-07-05  8:44   ` Daniel Lezcano
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Lezcano @ 2018-07-05  8:44 UTC (permalink / raw)
  To: Viresh Kumar, Zhang Rui, Eduardo Valentin, robh, Wei Xu
  Cc: linux-pm, Vincent Guittot, devicetree, olof, linux-arm-kernel,
	linux-kernel

On 05/07/2018 07:09, Viresh Kumar wrote:
> Each CPU can (and does) participate in cooling down the system but the
> DT only captures the CPU0 in the cooling maps. Things work by chance as
> under normal circumstances its the CPU0 which is used by the operating
> systems to probe the cooling devices. But as soon as that ordering
> changes and any other CPU is used to bring up the cooling device, we
> will start seeing errors.
> 
> On the other hand, the hardware is partially defined in DT in these
> cases and we must do a better job by capturing all devices.
> 
> Add all devices (CPUs here) in the cooling maps which are also affected
> by the trip point.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> index 247024df714f..919d36b91bf3 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> @@ -886,7 +886,14 @@
>  				cooling-maps {
>  					map0 {
>  						trip = <&target>;
> -						cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +						cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +								 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +								 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +								 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +								 <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +								 <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +								 <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +								 <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>  					};
>  				};
>  			};
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


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

* Re: [PATCH 1/2] dt-bindings: thermal: Allow multiple devices to share cooling map
  2018-07-05  5:09 ` [PATCH 1/2] dt-bindings: thermal: Allow multiple devices to share cooling map Viresh Kumar
@ 2018-07-16  4:34   ` Viresh Kumar
  2018-07-16 22:02   ` Rob Herring
  1 sibling, 0 replies; 17+ messages in thread
From: Viresh Kumar @ 2018-07-16  4:34 UTC (permalink / raw)
  To: Zhang Rui, Eduardo Valentin, robh
  Cc: linux-pm, Vincent Guittot, Daniel Lezcano, devicetree, olof,
	linux-kernel

On 05-07-18, 10:39, Viresh Kumar wrote:
> Allow cooling devices sharing same trip point with same contribution
> value to share the cooling map as well. Otherwise the same information
> will be duplicated for each device sharing the trip point.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  Documentation/devicetree/bindings/thermal/thermal.txt | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
> index cc553f0952c5..eb7ee91556a5 100644
> --- a/Documentation/devicetree/bindings/thermal/thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
> @@ -97,8 +97,8 @@ get assigned to trip points of the zone. The cooling devices are expected
>  to be loaded in the target system.
>  
>  Required properties:
> -- cooling-device:	A phandle of a cooling device with its specifier,
> -  Type: phandle +	referring to which cooling device is used in this
> +- cooling-device:	A list of phandles of cooling devices with their specifiers,
> +  Type: phandle +	referring to which cooling devices are used in this
>      cooling specifier	binding. In the cooling specifier, the first cell
>  			is the minimum cooling state and the second cell
>  			is the maximum cooling state used in this map.
> @@ -276,12 +276,7 @@ thermal-zones {
>  			};
>  			map1 {
>  				trip = <&cpu_alert1>;
> -				cooling-device = <&fan0 5 THERMAL_NO_LIMIT>;
> -			};
> -			map2 {
> -				trip = <&cpu_alert1>;
> -				cooling-device =
> -				    <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +				cooling-device = <&fan0 5 THERMAL_NO_LIMIT>, <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>  			};
>  		};
>  	};

Any objections to this ? Can you guys provide Acks ?

-- 
viresh

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

* Re: [PATCH 1/2] dt-bindings: thermal: Allow multiple devices to share cooling map
  2018-07-05  5:09 ` [PATCH 1/2] dt-bindings: thermal: Allow multiple devices to share cooling map Viresh Kumar
  2018-07-16  4:34   ` Viresh Kumar
@ 2018-07-16 22:02   ` Rob Herring
  1 sibling, 0 replies; 17+ messages in thread
From: Rob Herring @ 2018-07-16 22:02 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Zhang Rui, Eduardo Valentin, linux-pm, Vincent Guittot,
	Daniel Lezcano, devicetree, olof, linux-kernel

On Thu, Jul 05, 2018 at 10:39:23AM +0530, Viresh Kumar wrote:
> Allow cooling devices sharing same trip point with same contribution
> value to share the cooling map as well. Otherwise the same information
> will be duplicated for each device sharing the trip point.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  Documentation/devicetree/bindings/thermal/thermal.txt | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

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

* [PATCH] of: thermal: Allow multiple devices to share cooling map
  2018-07-05  5:09 [PATCH 0/2] dt: thermal: Fix broken cooling-maps Viresh Kumar
  2018-07-05  5:09 ` [PATCH 1/2] dt-bindings: thermal: Allow multiple devices to share cooling map Viresh Kumar
  2018-07-05  5:09 ` [PATCH 2/2] arm64: dts: hi6220: Add all CPUs in cooling maps Viresh Kumar
@ 2018-07-17 10:54 ` Viresh Kumar
  2018-08-06 18:05   ` Eduardo Valentin
  2018-08-08  7:08   ` [PATCH V2] " Viresh Kumar
  2018-07-18 15:34 ` [PATCH 0/2] dt: thermal: Fix broken cooling-maps Wei Xu
  2018-07-31  4:51 ` Viresh Kumar
  4 siblings, 2 replies; 17+ messages in thread
From: Viresh Kumar @ 2018-07-17 10:54 UTC (permalink / raw)
  To: Zhang Rui, Eduardo Valentin
  Cc: Viresh Kumar, linux-pm, Vincent Guittot, Daniel Lezcano, linux-kernel

A cooling map entry may now contain a list of phandles and their
arguments representing multiple devices which share the trip point.

This patch updates the thermal OF core to parse them properly.

Tested on Hikey960.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
This is a follow up patch to the DT bindings patchset:

https://lkml.kernel.org/r/cover.1530766981.git.viresh.kumar@linaro.org

 drivers/thermal/of-thermal.c | 140 +++++++++++++++++++++++++----------
 1 file changed, 102 insertions(+), 38 deletions(-)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index 977a8307fbb1..79c06c4c861b 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -19,22 +19,33 @@
 /***   Private data structures to represent thermal device tree data ***/
 
 /**
- * struct __thermal_bind_param - a match between trip and cooling device
+ * struct __thermal_cooling_bind_param - a cooling device for a trip point
  * @cooling_device: a pointer to identify the referred cooling device
- * @trip_id: the trip point index
- * @usage: the percentage (from 0 to 100) of cooling contribution
  * @min: minimum cooling state used at this trip point
  * @max: maximum cooling state used at this trip point
  */
 
-struct __thermal_bind_params {
+struct __thermal_cooling_bind_param {
 	struct device_node *cooling_device;
-	unsigned int trip_id;
-	unsigned int usage;
 	unsigned long min;
 	unsigned long max;
 };
 
+/**
+ * struct __thermal_bind_param - a match between trip and cooling device
+ * @tcbp: a pointer to an array of cooling devices
+ * @count: number of elements in array
+ * @trip_id: the trip point index
+ * @usage: the percentage (from 0 to 100) of cooling contribution
+ */
+
+struct __thermal_bind_params {
+	struct __thermal_cooling_bind_param *tcbp;
+	unsigned int count;
+	unsigned int trip_id;
+	unsigned int usage;
+};
+
 /**
  * struct __thermal_zone - internal representation of a thermal zone
  * @mode: current thermal zone device mode (enabled/disabled)
@@ -192,25 +203,31 @@ static int of_thermal_bind(struct thermal_zone_device *thermal,
 			   struct thermal_cooling_device *cdev)
 {
 	struct __thermal_zone *data = thermal->devdata;
-	int i;
+	struct __thermal_bind_params *tbp;
+	struct __thermal_cooling_bind_param *tcbp;
+	int i, j;
 
 	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;
+		tbp = data->tbps + i;
 
-		if (tbp->cooling_device == cdev->np) {
-			int ret;
+		for (j = 0; j < tbp->count; j++) {
+			tcbp = tbp->tcbp + j;
 
-			ret = thermal_zone_bind_cooling_device(thermal,
+			if (tcbp->cooling_device == cdev->np) {
+				int ret;
+
+				ret = thermal_zone_bind_cooling_device(thermal,
 						tbp->trip_id, cdev,
-						tbp->max,
-						tbp->min,
+						tcbp->max,
+						tcbp->min,
 						tbp->usage);
-			if (ret)
-				return ret;
+				if (ret)
+					return ret;
+			}
 		}
 	}
 
@@ -221,22 +238,28 @@ static int of_thermal_unbind(struct thermal_zone_device *thermal,
 			     struct thermal_cooling_device *cdev)
 {
 	struct __thermal_zone *data = thermal->devdata;
-	int i;
+	struct __thermal_bind_params *tbp;
+	struct __thermal_cooling_bind_param *tcbp;
+	int i, j;
 
 	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;
+		tbp = data->tbps + i;
+
+		for (j = 0; j < tbp->count; j++) {
+			tcbp = tbp->tcbp + j;
 
-		if (tbp->cooling_device == cdev->np) {
-			int ret;
+			if (tcbp->cooling_device == cdev->np) {
+				int ret;
 
-			ret = thermal_zone_unbind_cooling_device(thermal,
-						tbp->trip_id, cdev);
-			if (ret)
-				return ret;
+				ret = thermal_zone_unbind_cooling_device(thermal,
+							tbp->trip_id, cdev);
+				if (ret)
+					return ret;
+			}
 		}
 	}
 
@@ -652,8 +675,9 @@ static int thermal_of_populate_bind_params(struct device_node *np,
 					   int ntrips)
 {
 	struct of_phandle_args cooling_spec;
+	struct __thermal_cooling_bind_param *__tcbp;
 	struct device_node *trip;
-	int ret, i;
+	int ret, i, count;
 	u32 prop;
 
 	/* Default weight. Usage is optional */
@@ -680,20 +704,44 @@ static int thermal_of_populate_bind_params(struct device_node *np,
 		goto end;
 	}
 
-	ret = of_parse_phandle_with_args(np, "cooling-device", "#cooling-cells",
-					 0, &cooling_spec);
-	if (ret < 0) {
+	count = of_count_phandle_with_args(np, "cooling-device",
+					   "#cooling-cells");
+	if (!count) {
 		pr_err("missing cooling_device property\n");
 		goto end;
 	}
-	__tbp->cooling_device = cooling_spec.np;
-	if (cooling_spec.args_count >= 2) { /* at least min and max */
-		__tbp->min = cooling_spec.args[0];
-		__tbp->max = cooling_spec.args[1];
-	} else {
-		pr_err("wrong reference to cooling device, missing limits\n");
+
+	__tcbp = kcalloc(count, sizeof(*__tcbp), GFP_KERNEL);
+	if (!__tcbp)
+		goto end;
+
+	for (i = 0; i < count; i++) {
+		ret = of_parse_phandle_with_args(np, "cooling-device",
+				"#cooling-cells", i, &cooling_spec);
+		if (ret < 0) {
+			pr_err("missing cooling_device property\n");
+			goto free_tcbp;
+		}
+
+		__tcbp[i].cooling_device = cooling_spec.np;
+
+		if (cooling_spec.args_count >= 2) { /* at least min and max */
+			__tcbp[i].min = cooling_spec.args[0];
+			__tcbp[i].max = cooling_spec.args[1];
+		} else {
+			pr_err("wrong reference to cooling device, missing limits\n");
+		}
 	}
 
+	__tbp->tcbp= __tcbp;
+	__tbp->count = count;
+
+	goto end;
+
+free_tcbp:
+	for (i = i - 1; i >= 0; i--)
+		of_node_put(__tcbp[i].cooling_device);
+	kfree(__tcbp);
 end:
 	of_node_put(trip);
 
@@ -900,8 +948,16 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
 	return tz;
 
 free_tbps:
-	for (i = i - 1; i >= 0; i--)
-		of_node_put(tz->tbps[i].cooling_device);
+	for (i = i - 1; i >= 0; i--) {
+		struct __thermal_bind_params *tbp = tz->tbps + i;
+		int j;
+
+		for (j = 0; j < tbp->count; j++)
+			of_node_put(tbp->tcbp[j].cooling_device);
+
+		kfree(tbp->tcbp);
+	}
+
 	kfree(tz->tbps);
 free_trips:
 	for (i = 0; i < tz->ntrips; i++)
@@ -917,10 +973,18 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
 
 static inline void of_thermal_free_zone(struct __thermal_zone *tz)
 {
-	int i;
+	struct __thermal_bind_params *tbp;
+	int i, j;
+
+	for (i = 0; i < tz->num_tbps; i++) {
+		tbp = tz->tbps + i;
+
+		for (j = 0; j < tbp->count; j++)
+			of_node_put(tbp->tcbp[j].cooling_device);
+
+		kfree(tbp->tcbp);
+	}
 
-	for (i = 0; i < tz->num_tbps; i++)
-		of_node_put(tz->tbps[i].cooling_device);
 	kfree(tz->tbps);
 	for (i = 0; i < tz->ntrips; i++)
 		of_node_put(tz->trips[i].np);
-- 
2.18.0.rc1.242.g61856ae69a2c


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

* Re: [PATCH 0/2] dt: thermal: Fix broken cooling-maps
  2018-07-05  5:09 [PATCH 0/2] dt: thermal: Fix broken cooling-maps Viresh Kumar
                   ` (2 preceding siblings ...)
  2018-07-17 10:54 ` [PATCH] of: thermal: Allow multiple devices to share cooling map Viresh Kumar
@ 2018-07-18 15:34 ` Wei Xu
  2018-07-19  2:40   ` Viresh Kumar
  2018-07-31  4:51 ` Viresh Kumar
  4 siblings, 1 reply; 17+ messages in thread
From: Wei Xu @ 2018-07-18 15:34 UTC (permalink / raw)
  To: Viresh Kumar, Zhang Rui, Eduardo Valentin, robh
  Cc: linux-pm, Vincent Guittot, Daniel Lezcano, devicetree, olof,
	linux-arm-kernel, linux-kernel

Hi Viresh,

On 2018/7/5 6:09, Viresh Kumar wrote:
> Hi,
> 
> This is an attempt to fix the broken or partially defined DT bindings
> for cooling-maps. We should list every device that participates in
> cooling down at a certain trip point, instead of just the first in the
> list as that depends on certain ordering of events to work properly.
> 
> The first patch extends the binding to allow a list of phandles in
> "cooling-device" property and the second patch fixes one of the
> platform's DT.
> 
> This will be followed up by fixing all platform DT bindings that have
> these issues after this set is accepted.
> 
> The kernel also requires some changes to handle the phandle list, but
> wouldn't break with these changes as it reads the first phandle in the
> list for now. We can update that separately.
> 
> --
> viresh
> 
> Viresh Kumar (2):
>   dt-bindings: thermal: Allow multiple devices to share cooling map
>   arm64: dts: hi6220: Add all CPUs in cooling maps
> 
>  Documentation/devicetree/bindings/thermal/thermal.txt | 11 +++--------
>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi             |  9 ++++++++-
>  2 files changed, 11 insertions(+), 9 deletions(-)
> 

Thanks!
Applied both to the hisilicon dt tree.

Best Regards,
Wei


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

* Re: [PATCH 0/2] dt: thermal: Fix broken cooling-maps
  2018-07-18 15:34 ` [PATCH 0/2] dt: thermal: Fix broken cooling-maps Wei Xu
@ 2018-07-19  2:40   ` Viresh Kumar
  2018-07-19  9:54     ` Wei Xu
  0 siblings, 1 reply; 17+ messages in thread
From: Viresh Kumar @ 2018-07-19  2:40 UTC (permalink / raw)
  To: Wei Xu
  Cc: Zhang Rui, Eduardo Valentin, robh, linux-pm, Vincent Guittot,
	Daniel Lezcano, devicetree, olof, linux-arm-kernel, linux-kernel

On 18-07-18, 16:34, Wei Xu wrote:
> Hi Viresh,
> 
> On 2018/7/5 6:09, Viresh Kumar wrote:
> > Hi,
> > 
> > This is an attempt to fix the broken or partially defined DT bindings
> > for cooling-maps. We should list every device that participates in
> > cooling down at a certain trip point, instead of just the first in the
> > list as that depends on certain ordering of events to work properly.
> > 
> > The first patch extends the binding to allow a list of phandles in
> > "cooling-device" property and the second patch fixes one of the
> > platform's DT.
> > 
> > This will be followed up by fixing all platform DT bindings that have
> > these issues after this set is accepted.
> > 
> > The kernel also requires some changes to handle the phandle list, but
> > wouldn't break with these changes as it reads the first phandle in the
> > list for now. We can update that separately.
> > 
> > --
> > viresh
> > 
> > Viresh Kumar (2):
> >   dt-bindings: thermal: Allow multiple devices to share cooling map
> >   arm64: dts: hi6220: Add all CPUs in cooling maps
> > 
> >  Documentation/devicetree/bindings/thermal/thermal.txt | 11 +++--------
> >  arch/arm64/boot/dts/hisilicon/hi6220.dtsi             |  9 ++++++++-
> >  2 files changed, 11 insertions(+), 9 deletions(-)
> > 
> 
> Thanks!
> Applied both to the hisilicon dt tree.

Hi Wei,

I expected the first patch to go via the thermal tree as it is for the
thermal core maintainers. Second patch can very well go from your
tree, but only after the 1st one is applied by thermal maintainers.

-- 
viresh

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

* Re: [PATCH 0/2] dt: thermal: Fix broken cooling-maps
  2018-07-19  2:40   ` Viresh Kumar
@ 2018-07-19  9:54     ` Wei Xu
  0 siblings, 0 replies; 17+ messages in thread
From: Wei Xu @ 2018-07-19  9:54 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Zhang Rui, Eduardo Valentin, robh, linux-pm, Vincent Guittot,
	Daniel Lezcano, devicetree, olof, linux-arm-kernel, linux-kernel

Hi Viresh,

On 2018/7/19 3:40, Viresh Kumar wrote:
> On 18-07-18, 16:34, Wei Xu wrote:
>> Hi Viresh,
>>
>> On 2018/7/5 6:09, Viresh Kumar wrote:
>>> Hi,
>>>
>>> This is an attempt to fix the broken or partially defined DT bindings
>>> for cooling-maps. We should list every device that participates in
>>> cooling down at a certain trip point, instead of just the first in the
>>> list as that depends on certain ordering of events to work properly.
>>>
>>> The first patch extends the binding to allow a list of phandles in
>>> "cooling-device" property and the second patch fixes one of the
>>> platform's DT.
>>>
>>> This will be followed up by fixing all platform DT bindings that have
>>> these issues after this set is accepted.
>>>
>>> The kernel also requires some changes to handle the phandle list, but
>>> wouldn't break with these changes as it reads the first phandle in the
>>> list for now. We can update that separately.
>>>
>>> --
>>> viresh
>>>
>>> Viresh Kumar (2):
>>>   dt-bindings: thermal: Allow multiple devices to share cooling map
>>>   arm64: dts: hi6220: Add all CPUs in cooling maps
>>>
>>>  Documentation/devicetree/bindings/thermal/thermal.txt | 11 +++--------
>>>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi             |  9 ++++++++-
>>>  2 files changed, 11 insertions(+), 9 deletions(-)
>>>
>>
>> Thanks!
>> Applied both to the hisilicon dt tree.
> 
> Hi Wei,
> 
> I expected the first patch to go via the thermal tree as it is for the
> thermal core maintainers. Second patch can very well go from your
> tree, but only after the 1st one is applied by thermal maintainers.
> 

OK. I will drop them in the pull request and apply the dts patch later.

Best Regards,
Wei



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

* Re: [PATCH 0/2] dt: thermal: Fix broken cooling-maps
  2018-07-05  5:09 [PATCH 0/2] dt: thermal: Fix broken cooling-maps Viresh Kumar
                   ` (3 preceding siblings ...)
  2018-07-18 15:34 ` [PATCH 0/2] dt: thermal: Fix broken cooling-maps Wei Xu
@ 2018-07-31  4:51 ` Viresh Kumar
  2018-07-31  6:00   ` Zhang Rui
  4 siblings, 1 reply; 17+ messages in thread
From: Viresh Kumar @ 2018-07-31  4:51 UTC (permalink / raw)
  To: Zhang Rui, Eduardo Valentin, robh, Wei Xu
  Cc: linux-pm, Vincent Guittot, Daniel Lezcano, devicetree, olof,
	linux-arm-kernel, linux-kernel

On 05-07-18, 10:39, Viresh Kumar wrote:
> Hi,
> 
> This is an attempt to fix the broken or partially defined DT bindings
> for cooling-maps. We should list every device that participates in
> cooling down at a certain trip point, instead of just the first in the
> list as that depends on certain ordering of events to work properly.
> 
> The first patch extends the binding to allow a list of phandles in
> "cooling-device" property and the second patch fixes one of the
> platform's DT.
> 
> This will be followed up by fixing all platform DT bindings that have
> these issues after this set is accepted.
> 
> The kernel also requires some changes to handle the phandle list, but
> wouldn't break with these changes as it reads the first phandle in the
> list for now. We can update that separately.

@Zhang: Are you going to apply this for 4.19-rc1 ? There are lot of patches that
I am holding up until this gets merged.

-- 
viresh

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

* Re: [PATCH 0/2] dt: thermal: Fix broken cooling-maps
  2018-07-31  4:51 ` Viresh Kumar
@ 2018-07-31  6:00   ` Zhang Rui
  2018-08-03  8:40     ` Viresh Kumar
  0 siblings, 1 reply; 17+ messages in thread
From: Zhang Rui @ 2018-07-31  6:00 UTC (permalink / raw)
  To: Viresh Kumar, Eduardo Valentin, robh, Wei Xu
  Cc: linux-pm, Vincent Guittot, Daniel Lezcano, devicetree, olof,
	linux-arm-kernel, linux-kernel

On 二, 2018-07-31 at 10:21 +0530, Viresh Kumar wrote:
> On 05-07-18, 10:39, Viresh Kumar wrote:
> > 
> > Hi,
> > 
> > This is an attempt to fix the broken or partially defined DT
> > bindings
> > for cooling-maps. We should list every device that participates in
> > cooling down at a certain trip point, instead of just the first in
> > the
> > list as that depends on certain ordering of events to work
> > properly.
> > 
> > The first patch extends the binding to allow a list of phandles in
> > "cooling-device" property and the second patch fixes one of the
> > platform's DT.
> > 
> > This will be followed up by fixing all platform DT bindings that
> > have
> > these issues after this set is accepted.
> > 
> > The kernel also requires some changes to handle the phandle list,
> > but
> > wouldn't break with these changes as it reads the first phandle in
> > the
> > list for now. We can update that separately.
> @Zhang: Are you going to apply this for 4.19-rc1 ? There are lot of
> patches that
> I am holding up until this gets merged,
> 
I suppose this patch should go via Eduardo' tree.
Eduardo, can you please take a look at this patch set?

thanks,
rui

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

* Re: [PATCH 0/2] dt: thermal: Fix broken cooling-maps
  2018-07-31  6:00   ` Zhang Rui
@ 2018-08-03  8:40     ` Viresh Kumar
  2018-08-06  6:29       ` Zhang Rui
  0 siblings, 1 reply; 17+ messages in thread
From: Viresh Kumar @ 2018-08-03  8:40 UTC (permalink / raw)
  To: Zhang Rui
  Cc: Eduardo Valentin, robh, Wei Xu, linux-pm, Vincent Guittot,
	Daniel Lezcano, devicetree, olof, linux-arm-kernel, linux-kernel

On 31-07-18, 14:00, Zhang Rui wrote:
> I suppose this patch should go via Eduardo' tree.
> Eduardo, can you please take a look at this patch set?

Zhang,

Since Eduardo isn't replying, will it be possible for you to pick it up as I
don't want to miss 4.19-rc1.

-- 
viresh

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

* Re: [PATCH 0/2] dt: thermal: Fix broken cooling-maps
  2018-08-03  8:40     ` Viresh Kumar
@ 2018-08-06  6:29       ` Zhang Rui
  0 siblings, 0 replies; 17+ messages in thread
From: Zhang Rui @ 2018-08-06  6:29 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Eduardo Valentin, robh, Wei Xu, linux-pm, Vincent Guittot,
	Daniel Lezcano, devicetree, olof, linux-arm-kernel, linux-kernel

On 五, 2018-08-03 at 14:10 +0530, Viresh Kumar wrote:
> On 31-07-18, 14:00, Zhang Rui wrote:
> > 
> > I suppose this patch should go via Eduardo' tree.
> > Eduardo, can you please take a look at this patch set?
> Zhang,
> 
> Since Eduardo isn't replying, will it be possible for you to pick it
> up as I
> don't want to miss 4.19-rc1.
> 
Okay, I will keep queue it in my tree first.

thanks,
rui

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

* Re: [PATCH] of: thermal: Allow multiple devices to share cooling map
  2018-07-17 10:54 ` [PATCH] of: thermal: Allow multiple devices to share cooling map Viresh Kumar
@ 2018-08-06 18:05   ` Eduardo Valentin
  2018-08-08  7:08   ` [PATCH V2] " Viresh Kumar
  1 sibling, 0 replies; 17+ messages in thread
From: Eduardo Valentin @ 2018-08-06 18:05 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Zhang Rui, linux-pm, Vincent Guittot, Daniel Lezcano, linux-kernel

Hello,

On Tue, Jul 17, 2018 at 04:24:16PM +0530, Viresh Kumar wrote:
> A cooling map entry may now contain a list of phandles and their
> arguments representing multiple devices which share the trip point.
> 
> This patch updates the thermal OF core to parse them properly.

I am mostly fine with the patch idea, specially because we wont be
breaking existing DT blobs out there.

See comment below.

> 
> Tested on Hikey960.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> This is a follow up patch to the DT bindings patchset:
> 
> https://lkml.kernel.org/r/cover.1530766981.git.viresh.kumar@linaro.org
> 
>  drivers/thermal/of-thermal.c | 140 +++++++++++++++++++++++++----------
>  1 file changed, 102 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index 977a8307fbb1..79c06c4c861b 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -19,22 +19,33 @@
>  /***   Private data structures to represent thermal device tree data ***/
>  
>  /**
> - * struct __thermal_bind_param - a match between trip and cooling device
> + * struct __thermal_cooling_bind_param - a cooling device for a trip point
>   * @cooling_device: a pointer to identify the referred cooling device
> - * @trip_id: the trip point index
> - * @usage: the percentage (from 0 to 100) of cooling contribution
>   * @min: minimum cooling state used at this trip point
>   * @max: maximum cooling state used at this trip point
>   */
>  
> -struct __thermal_bind_params {
> +struct __thermal_cooling_bind_param {
>  	struct device_node *cooling_device;
> -	unsigned int trip_id;
> -	unsigned int usage;
>  	unsigned long min;
>  	unsigned long max;
>  };
>  
> +/**
> + * struct __thermal_bind_param - a match between trip and cooling device
> + * @tcbp: a pointer to an array of cooling devices
> + * @count: number of elements in array
> + * @trip_id: the trip point index
> + * @usage: the percentage (from 0 to 100) of cooling contribution
> + */
> +
> +struct __thermal_bind_params {
> +	struct __thermal_cooling_bind_param *tcbp;
> +	unsigned int count;
> +	unsigned int trip_id;
> +	unsigned int usage;
> +};

Do you mind elaborating why you needed to do this split and adding a new
struct? More important, please describe in your commit message.

> +
>  /**
>   * struct __thermal_zone - internal representation of a thermal zone
>   * @mode: current thermal zone device mode (enabled/disabled)
> @@ -192,25 +203,31 @@ static int of_thermal_bind(struct thermal_zone_device *thermal,
>  			   struct thermal_cooling_device *cdev)
>  {
>  	struct __thermal_zone *data = thermal->devdata;
> -	int i;
> +	struct __thermal_bind_params *tbp;
> +	struct __thermal_cooling_bind_param *tcbp;
> +	int i, j;
>  
>  	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;
> +		tbp = data->tbps + i;
>  
> -		if (tbp->cooling_device == cdev->np) {
> -			int ret;
> +		for (j = 0; j < tbp->count; j++) {
> +			tcbp = tbp->tcbp + j;
>  
> -			ret = thermal_zone_bind_cooling_device(thermal,
> +			if (tcbp->cooling_device == cdev->np) {
> +				int ret;
> +
> +				ret = thermal_zone_bind_cooling_device(thermal,
>  						tbp->trip_id, cdev,
> -						tbp->max,
> -						tbp->min,
> +						tcbp->max,
> +						tcbp->min,
>  						tbp->usage);
> -			if (ret)
> -				return ret;
> +				if (ret)
> +					return ret;
> +			}
>  		}
>  	}
>  
> @@ -221,22 +238,28 @@ static int of_thermal_unbind(struct thermal_zone_device *thermal,
>  			     struct thermal_cooling_device *cdev)
>  {
>  	struct __thermal_zone *data = thermal->devdata;
> -	int i;
> +	struct __thermal_bind_params *tbp;
> +	struct __thermal_cooling_bind_param *tcbp;
> +	int i, j;
>  
>  	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;
> +		tbp = data->tbps + i;
> +
> +		for (j = 0; j < tbp->count; j++) {
> +			tcbp = tbp->tcbp + j;
>  
> -		if (tbp->cooling_device == cdev->np) {
> -			int ret;
> +			if (tcbp->cooling_device == cdev->np) {
> +				int ret;
>  
> -			ret = thermal_zone_unbind_cooling_device(thermal,
> -						tbp->trip_id, cdev);
> -			if (ret)
> -				return ret;
> +				ret = thermal_zone_unbind_cooling_device(thermal,
> +							tbp->trip_id, cdev);
> +				if (ret)
> +					return ret;
> +			}
>  		}
>  	}
>  
> @@ -652,8 +675,9 @@ static int thermal_of_populate_bind_params(struct device_node *np,
>  					   int ntrips)
>  {
>  	struct of_phandle_args cooling_spec;
> +	struct __thermal_cooling_bind_param *__tcbp;
>  	struct device_node *trip;
> -	int ret, i;
> +	int ret, i, count;
>  	u32 prop;
>  
>  	/* Default weight. Usage is optional */
> @@ -680,20 +704,44 @@ static int thermal_of_populate_bind_params(struct device_node *np,
>  		goto end;
>  	}
>  
> -	ret = of_parse_phandle_with_args(np, "cooling-device", "#cooling-cells",
> -					 0, &cooling_spec);
> -	if (ret < 0) {
> +	count = of_count_phandle_with_args(np, "cooling-device",
> +					   "#cooling-cells");
> +	if (!count) {
>  		pr_err("missing cooling_device property\n");

Probably the above error message deserves a better fit to the current
situation. Maybe:
+  		pr_err("Add a cooling_device property with at least one device\n");

>  		goto end;
>  	}
> -	__tbp->cooling_device = cooling_spec.np;
> -	if (cooling_spec.args_count >= 2) { /* at least min and max */
> -		__tbp->min = cooling_spec.args[0];
> -		__tbp->max = cooling_spec.args[1];
> -	} else {
> -		pr_err("wrong reference to cooling device, missing limits\n");
> +
> +	__tcbp = kcalloc(count, sizeof(*__tcbp), GFP_KERNEL);
> +	if (!__tcbp)
> +		goto end;
> +
> +	for (i = 0; i < count; i++) {
> +		ret = of_parse_phandle_with_args(np, "cooling-device",
> +				"#cooling-cells", i, &cooling_spec);
> +		if (ret < 0) {
> +			pr_err("missing cooling_device property\n");

Here is a wrongly written cooling_device phandle, no?

> +			goto free_tcbp;
> +		}
> +
> +		__tcbp[i].cooling_device = cooling_spec.np;
> +
> +		if (cooling_spec.args_count >= 2) { /* at least min and max */
> +			__tcbp[i].min = cooling_spec.args[0];
> +			__tcbp[i].max = cooling_spec.args[1];
> +		} else {
> +			pr_err("wrong reference to cooling device, missing limits\n");
> +		}
>  	}
>  
> +	__tbp->tcbp= __tcbp;
> +	__tbp->count = count;
> +
> +	goto end;
> +
> +free_tcbp:
> +	for (i = i - 1; i >= 0; i--)
> +		of_node_put(__tcbp[i].cooling_device);
> +	kfree(__tcbp);
>  end:
>  	of_node_put(trip);
>  
> @@ -900,8 +948,16 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>  	return tz;
>  
>  free_tbps:
> -	for (i = i - 1; i >= 0; i--)
> -		of_node_put(tz->tbps[i].cooling_device);
> +	for (i = i - 1; i >= 0; i--) {
> +		struct __thermal_bind_params *tbp = tz->tbps + i;
> +		int j;
> +
> +		for (j = 0; j < tbp->count; j++)
> +			of_node_put(tbp->tcbp[j].cooling_device);
> +
> +		kfree(tbp->tcbp);
> +	}
> +
>  	kfree(tz->tbps);
>  free_trips:
>  	for (i = 0; i < tz->ntrips; i++)
> @@ -917,10 +973,18 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>  
>  static inline void of_thermal_free_zone(struct __thermal_zone *tz)
>  {
> -	int i;
> +	struct __thermal_bind_params *tbp;
> +	int i, j;
> +
> +	for (i = 0; i < tz->num_tbps; i++) {
> +		tbp = tz->tbps + i;
> +
> +		for (j = 0; j < tbp->count; j++)
> +			of_node_put(tbp->tcbp[j].cooling_device);
> +
> +		kfree(tbp->tcbp);
> +	}
>  
> -	for (i = 0; i < tz->num_tbps; i++)
> -		of_node_put(tz->tbps[i].cooling_device);
>  	kfree(tz->tbps);
>  	for (i = 0; i < tz->ntrips; i++)
>  		of_node_put(tz->trips[i].np);
> -- 
> 2.18.0.rc1.242.g61856ae69a2c
> 

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

* [PATCH V2] of: thermal: Allow multiple devices to share cooling map
  2018-07-17 10:54 ` [PATCH] of: thermal: Allow multiple devices to share cooling map Viresh Kumar
  2018-08-06 18:05   ` Eduardo Valentin
@ 2018-08-08  7:08   ` Viresh Kumar
  2018-08-24 23:14     ` Eduardo Valentin
  1 sibling, 1 reply; 17+ messages in thread
From: Viresh Kumar @ 2018-08-08  7:08 UTC (permalink / raw)
  To: Zhang Rui, Eduardo Valentin
  Cc: Viresh Kumar, Vincent Guittot, Daniel Lezcano, linux-pm, linux-kernel

A cooling map entry may now contain a list of phandles and their
arguments representing multiple devices which share the trip point.

This patch updates the thermal OF core to parse them properly. The trip
point and contribution value is shared by multiple cooling devices now
and so a new structure is created, struct __thermal_cooling_bind_param,
which represents a cooling device and its min/max states and the
existing struct __thermal_bind_params now contains an array of this new
cooling device structure.

Tested on Hikey960.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V2:
- Updated commit log to include new structures information
- Updated two error messages to give better error log

 drivers/thermal/of-thermal.c | 142 +++++++++++++++++++++++++----------
 1 file changed, 103 insertions(+), 39 deletions(-)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index 977a8307fbb1..16b890a4fb50 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -19,22 +19,33 @@
 /***   Private data structures to represent thermal device tree data ***/
 
 /**
- * struct __thermal_bind_param - a match between trip and cooling device
+ * struct __thermal_cooling_bind_param - a cooling device for a trip point
  * @cooling_device: a pointer to identify the referred cooling device
- * @trip_id: the trip point index
- * @usage: the percentage (from 0 to 100) of cooling contribution
  * @min: minimum cooling state used at this trip point
  * @max: maximum cooling state used at this trip point
  */
 
-struct __thermal_bind_params {
+struct __thermal_cooling_bind_param {
 	struct device_node *cooling_device;
-	unsigned int trip_id;
-	unsigned int usage;
 	unsigned long min;
 	unsigned long max;
 };
 
+/**
+ * struct __thermal_bind_param - a match between trip and cooling device
+ * @tcbp: a pointer to an array of cooling devices
+ * @count: number of elements in array
+ * @trip_id: the trip point index
+ * @usage: the percentage (from 0 to 100) of cooling contribution
+ */
+
+struct __thermal_bind_params {
+	struct __thermal_cooling_bind_param *tcbp;
+	unsigned int count;
+	unsigned int trip_id;
+	unsigned int usage;
+};
+
 /**
  * struct __thermal_zone - internal representation of a thermal zone
  * @mode: current thermal zone device mode (enabled/disabled)
@@ -192,25 +203,31 @@ static int of_thermal_bind(struct thermal_zone_device *thermal,
 			   struct thermal_cooling_device *cdev)
 {
 	struct __thermal_zone *data = thermal->devdata;
-	int i;
+	struct __thermal_bind_params *tbp;
+	struct __thermal_cooling_bind_param *tcbp;
+	int i, j;
 
 	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;
+		tbp = data->tbps + i;
 
-		if (tbp->cooling_device == cdev->np) {
-			int ret;
+		for (j = 0; j < tbp->count; j++) {
+			tcbp = tbp->tcbp + j;
 
-			ret = thermal_zone_bind_cooling_device(thermal,
+			if (tcbp->cooling_device == cdev->np) {
+				int ret;
+
+				ret = thermal_zone_bind_cooling_device(thermal,
 						tbp->trip_id, cdev,
-						tbp->max,
-						tbp->min,
+						tcbp->max,
+						tcbp->min,
 						tbp->usage);
-			if (ret)
-				return ret;
+				if (ret)
+					return ret;
+			}
 		}
 	}
 
@@ -221,22 +238,28 @@ static int of_thermal_unbind(struct thermal_zone_device *thermal,
 			     struct thermal_cooling_device *cdev)
 {
 	struct __thermal_zone *data = thermal->devdata;
-	int i;
+	struct __thermal_bind_params *tbp;
+	struct __thermal_cooling_bind_param *tcbp;
+	int i, j;
 
 	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;
+		tbp = data->tbps + i;
+
+		for (j = 0; j < tbp->count; j++) {
+			tcbp = tbp->tcbp + j;
 
-		if (tbp->cooling_device == cdev->np) {
-			int ret;
+			if (tcbp->cooling_device == cdev->np) {
+				int ret;
 
-			ret = thermal_zone_unbind_cooling_device(thermal,
-						tbp->trip_id, cdev);
-			if (ret)
-				return ret;
+				ret = thermal_zone_unbind_cooling_device(thermal,
+							tbp->trip_id, cdev);
+				if (ret)
+					return ret;
+			}
 		}
 	}
 
@@ -652,8 +675,9 @@ static int thermal_of_populate_bind_params(struct device_node *np,
 					   int ntrips)
 {
 	struct of_phandle_args cooling_spec;
+	struct __thermal_cooling_bind_param *__tcbp;
 	struct device_node *trip;
-	int ret, i;
+	int ret, i, count;
 	u32 prop;
 
 	/* Default weight. Usage is optional */
@@ -680,20 +704,44 @@ static int thermal_of_populate_bind_params(struct device_node *np,
 		goto end;
 	}
 
-	ret = of_parse_phandle_with_args(np, "cooling-device", "#cooling-cells",
-					 0, &cooling_spec);
-	if (ret < 0) {
-		pr_err("missing cooling_device property\n");
+	count = of_count_phandle_with_args(np, "cooling-device",
+					   "#cooling-cells");
+	if (!count) {
+		pr_err("Add a cooling_device property with at least one device\n");
 		goto end;
 	}
-	__tbp->cooling_device = cooling_spec.np;
-	if (cooling_spec.args_count >= 2) { /* at least min and max */
-		__tbp->min = cooling_spec.args[0];
-		__tbp->max = cooling_spec.args[1];
-	} else {
-		pr_err("wrong reference to cooling device, missing limits\n");
+
+	__tcbp = kcalloc(count, sizeof(*__tcbp), GFP_KERNEL);
+	if (!__tcbp)
+		goto end;
+
+	for (i = 0; i < count; i++) {
+		ret = of_parse_phandle_with_args(np, "cooling-device",
+				"#cooling-cells", i, &cooling_spec);
+		if (ret < 0) {
+			pr_err("Invalid cooling-device entry\n");
+			goto free_tcbp;
+		}
+
+		__tcbp[i].cooling_device = cooling_spec.np;
+
+		if (cooling_spec.args_count >= 2) { /* at least min and max */
+			__tcbp[i].min = cooling_spec.args[0];
+			__tcbp[i].max = cooling_spec.args[1];
+		} else {
+			pr_err("wrong reference to cooling device, missing limits\n");
+		}
 	}
 
+	__tbp->tcbp= __tcbp;
+	__tbp->count = count;
+
+	goto end;
+
+free_tcbp:
+	for (i = i - 1; i >= 0; i--)
+		of_node_put(__tcbp[i].cooling_device);
+	kfree(__tcbp);
 end:
 	of_node_put(trip);
 
@@ -900,8 +948,16 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
 	return tz;
 
 free_tbps:
-	for (i = i - 1; i >= 0; i--)
-		of_node_put(tz->tbps[i].cooling_device);
+	for (i = i - 1; i >= 0; i--) {
+		struct __thermal_bind_params *tbp = tz->tbps + i;
+		int j;
+
+		for (j = 0; j < tbp->count; j++)
+			of_node_put(tbp->tcbp[j].cooling_device);
+
+		kfree(tbp->tcbp);
+	}
+
 	kfree(tz->tbps);
 free_trips:
 	for (i = 0; i < tz->ntrips; i++)
@@ -917,10 +973,18 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
 
 static inline void of_thermal_free_zone(struct __thermal_zone *tz)
 {
-	int i;
+	struct __thermal_bind_params *tbp;
+	int i, j;
+
+	for (i = 0; i < tz->num_tbps; i++) {
+		tbp = tz->tbps + i;
+
+		for (j = 0; j < tbp->count; j++)
+			of_node_put(tbp->tcbp[j].cooling_device);
+
+		kfree(tbp->tcbp);
+	}
 
-	for (i = 0; i < tz->num_tbps; i++)
-		of_node_put(tz->tbps[i].cooling_device);
 	kfree(tz->tbps);
 	for (i = 0; i < tz->ntrips; i++)
 		of_node_put(tz->trips[i].np);
-- 
2.18.0.rc1.242.g61856ae69a2c


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

* Re: [PATCH V2] of: thermal: Allow multiple devices to share cooling map
  2018-08-08  7:08   ` [PATCH V2] " Viresh Kumar
@ 2018-08-24 23:14     ` Eduardo Valentin
  0 siblings, 0 replies; 17+ messages in thread
From: Eduardo Valentin @ 2018-08-24 23:14 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Zhang Rui, Vincent Guittot, Daniel Lezcano, linux-pm, linux-kernel

On Wed, Aug 08, 2018 at 12:38:14PM +0530, Viresh Kumar wrote:
> A cooling map entry may now contain a list of phandles and their
> arguments representing multiple devices which share the trip point.
> 
> This patch updates the thermal OF core to parse them properly. The trip
> point and contribution value is shared by multiple cooling devices now
> and so a new structure is created, struct __thermal_cooling_bind_param,
> which represents a cooling device and its min/max states and the
> existing struct __thermal_bind_params now contains an array of this new
> cooling device structure.
> 
> Tested on Hikey960.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> V2:
> - Updated commit log to include new structures information
> - Updated two error messages to give better error log
> 
>  drivers/thermal/of-thermal.c | 142 +++++++++++++++++++++++++----------
>  1 file changed, 103 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index 977a8307fbb1..16b890a4fb50 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -19,22 +19,33 @@
>  /***   Private data structures to represent thermal device tree data ***/
>  
>  /**
> - * struct __thermal_bind_param - a match between trip and cooling device
> + * struct __thermal_cooling_bind_param - a cooling device for a trip point
>   * @cooling_device: a pointer to identify the referred cooling device
> - * @trip_id: the trip point index
> - * @usage: the percentage (from 0 to 100) of cooling contribution
>   * @min: minimum cooling state used at this trip point
>   * @max: maximum cooling state used at this trip point
>   */
>  
> -struct __thermal_bind_params {
> +struct __thermal_cooling_bind_param {
>  	struct device_node *cooling_device;
> -	unsigned int trip_id;
> -	unsigned int usage;
>  	unsigned long min;
>  	unsigned long max;
>  };
>  
> +/**
> + * struct __thermal_bind_param - a match between trip and cooling device
> + * @tcbp: a pointer to an array of cooling devices
> + * @count: number of elements in array
> + * @trip_id: the trip point index
> + * @usage: the percentage (from 0 to 100) of cooling contribution
> + */
> +
> +struct __thermal_bind_params {
> +	struct __thermal_cooling_bind_param *tcbp;
> +	unsigned int count;
> +	unsigned int trip_id;
> +	unsigned int usage;
> +};
> +
>  /**
>   * struct __thermal_zone - internal representation of a thermal zone
>   * @mode: current thermal zone device mode (enabled/disabled)
> @@ -192,25 +203,31 @@ static int of_thermal_bind(struct thermal_zone_device *thermal,
>  			   struct thermal_cooling_device *cdev)
>  {
>  	struct __thermal_zone *data = thermal->devdata;
> -	int i;
> +	struct __thermal_bind_params *tbp;
> +	struct __thermal_cooling_bind_param *tcbp;
> +	int i, j;
>  
>  	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;
> +		tbp = data->tbps + i;
>  
> -		if (tbp->cooling_device == cdev->np) {
> -			int ret;
> +		for (j = 0; j < tbp->count; j++) {
> +			tcbp = tbp->tcbp + j;
>  
> -			ret = thermal_zone_bind_cooling_device(thermal,
> +			if (tcbp->cooling_device == cdev->np) {
> +				int ret;
> +
> +				ret = thermal_zone_bind_cooling_device(thermal,
>  						tbp->trip_id, cdev,
> -						tbp->max,
> -						tbp->min,
> +						tcbp->max,
> +						tcbp->min,
>  						tbp->usage);
> -			if (ret)
> -				return ret;
> +				if (ret)
> +					return ret;
> +			}
>  		}
>  	}
>  
> @@ -221,22 +238,28 @@ static int of_thermal_unbind(struct thermal_zone_device *thermal,
>  			     struct thermal_cooling_device *cdev)
>  {
>  	struct __thermal_zone *data = thermal->devdata;
> -	int i;
> +	struct __thermal_bind_params *tbp;
> +	struct __thermal_cooling_bind_param *tcbp;
> +	int i, j;
>  
>  	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;
> +		tbp = data->tbps + i;
> +
> +		for (j = 0; j < tbp->count; j++) {
> +			tcbp = tbp->tcbp + j;
>  
> -		if (tbp->cooling_device == cdev->np) {
> -			int ret;
> +			if (tcbp->cooling_device == cdev->np) {
> +				int ret;
>  
> -			ret = thermal_zone_unbind_cooling_device(thermal,
> -						tbp->trip_id, cdev);
> -			if (ret)
> -				return ret;
> +				ret = thermal_zone_unbind_cooling_device(thermal,
> +							tbp->trip_id, cdev);
> +				if (ret)
> +					return ret;
> +			}
>  		}
>  	}
>  
> @@ -652,8 +675,9 @@ static int thermal_of_populate_bind_params(struct device_node *np,
>  					   int ntrips)
>  {
>  	struct of_phandle_args cooling_spec;
> +	struct __thermal_cooling_bind_param *__tcbp;
>  	struct device_node *trip;
> -	int ret, i;
> +	int ret, i, count;
>  	u32 prop;
>  
>  	/* Default weight. Usage is optional */
> @@ -680,20 +704,44 @@ static int thermal_of_populate_bind_params(struct device_node *np,
>  		goto end;
>  	}
>  
> -	ret = of_parse_phandle_with_args(np, "cooling-device", "#cooling-cells",
> -					 0, &cooling_spec);
> -	if (ret < 0) {
> -		pr_err("missing cooling_device property\n");
> +	count = of_count_phandle_with_args(np, "cooling-device",
> +					   "#cooling-cells");
> +	if (!count) {
> +		pr_err("Add a cooling_device property with at least one device\n");
>  		goto end;
>  	}
> -	__tbp->cooling_device = cooling_spec.np;
> -	if (cooling_spec.args_count >= 2) { /* at least min and max */
> -		__tbp->min = cooling_spec.args[0];
> -		__tbp->max = cooling_spec.args[1];
> -	} else {
> -		pr_err("wrong reference to cooling device, missing limits\n");
> +
> +	__tcbp = kcalloc(count, sizeof(*__tcbp), GFP_KERNEL);
> +	if (!__tcbp)
> +		goto end;
> +
> +	for (i = 0; i < count; i++) {
> +		ret = of_parse_phandle_with_args(np, "cooling-device",
> +				"#cooling-cells", i, &cooling_spec);
> +		if (ret < 0) {
> +			pr_err("Invalid cooling-device entry\n");
> +			goto free_tcbp;
> +		}
> +
> +		__tcbp[i].cooling_device = cooling_spec.np;
> +
> +		if (cooling_spec.args_count >= 2) { /* at least min and max */
> +			__tcbp[i].min = cooling_spec.args[0];
> +			__tcbp[i].max = cooling_spec.args[1];
> +		} else {
> +			pr_err("wrong reference to cooling device, missing limits\n");
> +		}
>  	}
>  
> +	__tbp->tcbp= __tcbp;

Applying but manually fixing the above style issue.

> +	__tbp->count = count;
> +
> +	goto end;
> +
> +free_tcbp:
> +	for (i = i - 1; i >= 0; i--)
> +		of_node_put(__tcbp[i].cooling_device);
> +	kfree(__tcbp);
>  end:
>  	of_node_put(trip);
>  
> @@ -900,8 +948,16 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>  	return tz;
>  
>  free_tbps:
> -	for (i = i - 1; i >= 0; i--)
> -		of_node_put(tz->tbps[i].cooling_device);
> +	for (i = i - 1; i >= 0; i--) {
> +		struct __thermal_bind_params *tbp = tz->tbps + i;
> +		int j;
> +
> +		for (j = 0; j < tbp->count; j++)
> +			of_node_put(tbp->tcbp[j].cooling_device);
> +
> +		kfree(tbp->tcbp);
> +	}
> +
>  	kfree(tz->tbps);
>  free_trips:
>  	for (i = 0; i < tz->ntrips; i++)
> @@ -917,10 +973,18 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>  
>  static inline void of_thermal_free_zone(struct __thermal_zone *tz)
>  {
> -	int i;
> +	struct __thermal_bind_params *tbp;
> +	int i, j;
> +
> +	for (i = 0; i < tz->num_tbps; i++) {
> +		tbp = tz->tbps + i;
> +
> +		for (j = 0; j < tbp->count; j++)
> +			of_node_put(tbp->tcbp[j].cooling_device);
> +
> +		kfree(tbp->tcbp);
> +	}
>  
> -	for (i = 0; i < tz->num_tbps; i++)
> -		of_node_put(tz->tbps[i].cooling_device);
>  	kfree(tz->tbps);
>  	for (i = 0; i < tz->ntrips; i++)
>  		of_node_put(tz->trips[i].np);
> -- 
> 2.18.0.rc1.242.g61856ae69a2c
> 

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

end of thread, other threads:[~2018-08-24 23:14 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-05  5:09 [PATCH 0/2] dt: thermal: Fix broken cooling-maps Viresh Kumar
2018-07-05  5:09 ` [PATCH 1/2] dt-bindings: thermal: Allow multiple devices to share cooling map Viresh Kumar
2018-07-16  4:34   ` Viresh Kumar
2018-07-16 22:02   ` Rob Herring
2018-07-05  5:09 ` [PATCH 2/2] arm64: dts: hi6220: Add all CPUs in cooling maps Viresh Kumar
2018-07-05  8:44   ` Daniel Lezcano
2018-07-17 10:54 ` [PATCH] of: thermal: Allow multiple devices to share cooling map Viresh Kumar
2018-08-06 18:05   ` Eduardo Valentin
2018-08-08  7:08   ` [PATCH V2] " Viresh Kumar
2018-08-24 23:14     ` Eduardo Valentin
2018-07-18 15:34 ` [PATCH 0/2] dt: thermal: Fix broken cooling-maps Wei Xu
2018-07-19  2:40   ` Viresh Kumar
2018-07-19  9:54     ` Wei Xu
2018-07-31  4:51 ` Viresh Kumar
2018-07-31  6:00   ` Zhang Rui
2018-08-03  8:40     ` Viresh Kumar
2018-08-06  6:29       ` Zhang Rui

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).