[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

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