[PATCH 0/2] dt: thermal: Fix broken cooling-maps

[PATCH 0/2] dt: thermal: Fix broken cooling-maps

[Viresh Kumar]

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

[PATCH 1/2] dt-bindings: thermal: Allow multiple devices to share cooling map

[Viresh Kumar]

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

[PATCH 2/2] arm64: dts: hi6220: Add all CPUs in cooling maps

[Viresh Kumar]

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

Re: [PATCH 2/2] arm64: dts: hi6220: Add all CPUs in cooling maps

[Daniel Lezcano]

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

Re: [PATCH 1/2] dt-bindings: thermal: Allow multiple devices to share cooling map

[Viresh Kumar]

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

Re: [PATCH 1/2] dt-bindings: thermal: Allow multiple devices to share cooling map

[Rob Herring]

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>

[PATCH] of: thermal: Allow multiple devices to share cooling map

[Viresh Kumar]

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

Re: [PATCH 0/2] dt: thermal: Fix broken cooling-maps

[Wei Xu]

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

Re: [PATCH 0/2] dt: thermal: Fix broken cooling-maps

[Viresh Kumar]

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

Re: [PATCH 0/2] dt: thermal: Fix broken cooling-maps

[Wei Xu]

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

Re: [PATCH 0/2] dt: thermal: Fix broken cooling-maps

[Viresh Kumar]

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

Re: [PATCH 0/2] dt: thermal: Fix broken cooling-maps

[Zhang Rui]

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

Re: [PATCH 0/2] dt: thermal: Fix broken cooling-maps

[Viresh Kumar]

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

Re: [PATCH 0/2] dt: thermal: Fix broken cooling-maps

[Zhang Rui]

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

Re: [PATCH] of: thermal: Allow multiple devices to share cooling map

[Eduardo Valentin]

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
> 

[PATCH V2] of: thermal: Allow multiple devices to share cooling map

[Viresh Kumar]

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

Re: [PATCH V2] of: thermal: Allow multiple devices to share cooling map

[Eduardo Valentin]

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
>