linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-04-11  8:32 Yuantian Tang
  2019-04-11 19:14 ` Daniel Lezcano
  0 siblings, 1 reply; 6+ messages in thread
From: Yuantian Tang @ 2019-04-11  8:32 UTC (permalink / raw)
  To: shawnguo
  Cc: leoyang.li, robh+dt, mark.rutland, linux-arm-kernel, devicetree,
	linux-kernel, linux-pm, daniel.lezcano, rui.zhang, edubezval,
	Yuantian Tang

Ls1088a has 2 thermal sensors, core cluster and SoC platform. Core cluster
sensor is used to monitor the temperature of core and SoC platform is for
platform. The current dts only support the first sensor.
This patch adds the second sensor node to dts to enable it.

Signed-off-by: Yuantian Tang <andy.tang@nxp.com>
---
v5:
	- update the thermal zone name due to the length limitation
	- remove cooling map in platform zone
v4:
	- use hyphen instead of underscore in node name
v3:
	- use more descriptive name for each zone
v2:
	- Add more information about sensors to description
 arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi |   28 ++++++++++++++++++++---
 1 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
index de93b42..de39672 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
@@ -129,19 +129,19 @@
 	};
 
 	thermal-zones {
-		cpu_thermal: cpu-thermal {
+		core-cluster {
 			polling-delay-passive = <1000>;
 			polling-delay = <5000>;
 			thermal-sensors = <&tmu 0>;
 
 			trips {
-				cpu_alert: cpu-alert {
+				core_cluster_alert: core-cluster-alert {
 					temperature = <85000>;
 					hysteresis = <2000>;
 					type = "passive";
 				};
 
-				cpu_crit: cpu-crit {
+				core_cluster_crit: core-cluster-crit {
 					temperature = <95000>;
 					hysteresis = <2000>;
 					type = "critical";
@@ -150,7 +150,7 @@
 
 			cooling-maps {
 				map0 {
-					trip = <&cpu_alert>;
+					trip = <&core_cluster_alert>;
 					cooling-device =
 						<&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
 						<&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
@@ -163,6 +163,26 @@
 				};
 			};
 		};
+
+		platform {
+			polling-delay-passive = <1000>;
+			polling-delay = <5000>;
+			thermal-sensors = <&tmu 1>;
+
+			trips {
+				platform-alert {
+					temperature = <85000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+
+				platform-crit {
+					temperature = <95000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+		};
 	};
 
 	timer {
-- 
1.7.1


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

* Re: [PATCH v5] arm64: dts: ls1088a: add one more thermal zone node
  2019-04-11  8:32 [PATCH v5] arm64: dts: ls1088a: add one more thermal zone node Yuantian Tang
@ 2019-04-11 19:14 ` Daniel Lezcano
  2019-04-12  7:47   ` [EXT] " Andy Tang
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Lezcano @ 2019-04-11 19:14 UTC (permalink / raw)
  To: Yuantian Tang, shawnguo
  Cc: leoyang.li, robh+dt, mark.rutland, linux-arm-kernel, devicetree,
	linux-kernel, linux-pm, rui.zhang, edubezval

On 11/04/2019 10:32, Yuantian Tang wrote:
> Ls1088a has 2 thermal sensors, core cluster and SoC platform. Core cluster
> sensor is used to monitor the temperature of core and SoC platform is for
> platform. The current dts only support the first sensor.
> This patch adds the second sensor node to dts to enable it.
> 
> Signed-off-by: Yuantian Tang <andy.tang@nxp.com>
> ---
> v5:
> 	- update the thermal zone name due to the length limitation
> 	- remove cooling map in platform zone
> v4:
> 	- use hyphen instead of underscore in node name
> v3:
> 	- use more descriptive name for each zone
> v2:
> 	- Add more information about sensors to description
>  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi |   28 ++++++++++++++++++++---
>  1 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> index de93b42..de39672 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> @@ -129,19 +129,19 @@
>  	};
>  
>  	thermal-zones {
> -		cpu_thermal: cpu-thermal {
> +		core-cluster {
>  			polling-delay-passive = <1000>;
>  			polling-delay = <5000>;
>  			thermal-sensors = <&tmu 0>;
>  
>  			trips {
> -				cpu_alert: cpu-alert {
> +				core_cluster_alert: core-cluster-alert {
>  					temperature = <85000>;
>  					hysteresis = <2000>;
>  					type = "passive";
>  				};
>  
> -				cpu_crit: cpu-crit {
> +				core_cluster_crit: core-cluster-crit {
>  					temperature = <95000>;
>  					hysteresis = <2000>;
>  					type = "critical";
> @@ -150,7 +150,7 @@
>  
>  			cooling-maps {
>  				map0 {
> -					trip = <&cpu_alert>;
> +					trip = <&core_cluster_alert>;
>  					cooling-device =
>  						<&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>  						<&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> @@ -163,6 +163,26 @@
>  				};
>  			};
>  		};
> +
> +		platform {
> +			polling-delay-passive = <1000>;
> +			polling-delay = <5000>;
> +			thermal-sensors = <&tmu 1>;
> +
> +			trips {
> +				platform-alert {
> +					temperature = <85000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				platform-crit {
> +					temperature = <95000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
> +		};


Unfortunately, the documentation says the thermal zone node must contain
a "cooling-maps" entry.

It makes sense as if there is no cooling device, there is no action to
be taken thus polling the temperature just add noise to the system.



>  	};
>  
>  	timer {
> 


-- 
 <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] 6+ messages in thread

* RE: [EXT] Re: [PATCH v5] arm64: dts: ls1088a: add one more thermal zone node
  2019-04-11 19:14 ` Daniel Lezcano
@ 2019-04-12  7:47   ` Andy Tang
  2019-04-12 12:19     ` Daniel Lezcano
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Tang @ 2019-04-12  7:47 UTC (permalink / raw)
  To: Daniel Lezcano, shawnguo
  Cc: Leo Li, robh+dt, mark.rutland, linux-arm-kernel, devicetree,
	linux-kernel, linux-pm, rui.zhang, edubezval


> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: 2019年4月12日 3:15
> To: Andy Tang <andy.tang@nxp.com>; shawnguo@kernel.org
> Cc: Leo Li <leoyang.li@nxp.com>; robh+dt@kernel.org; mark.rutland@arm.com;
> linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-pm@vger.kernel.org; rui.zhang@intel.com;
> edubezval@gmail.com
> Subject: [EXT] Re: [PATCH v5] arm64: dts: ls1088a: add one more thermal zone
> node
> 
> WARNING: This email was created outside of NXP. DO NOT CLICK links or
> attachments unless you recognize the sender and know the content is safe.
> 
> 
> 
> On 11/04/2019 10:32, Yuantian Tang wrote:
> > Ls1088a has 2 thermal sensors, core cluster and SoC platform. Core
> > cluster sensor is used to monitor the temperature of core and SoC
> > platform is for platform. The current dts only support the first sensor.
> > This patch adds the second sensor node to dts to enable it.
> >
> > Signed-off-by: Yuantian Tang <andy.tang@nxp.com>
> > ---
> > v5:
> >       - update the thermal zone name due to the length limitation
> >       - remove cooling map in platform zone
> > v4:
> >       - use hyphen instead of underscore in node name
> > v3:
> >       - use more descriptive name for each zone
> > v2:
> >       - Add more information about sensors to description
> >  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi |   28
> ++++++++++++++++++++---
> >  1 files changed, 24 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> > b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> > index de93b42..de39672 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> > @@ -129,19 +129,19 @@
> >       };
> >
> >       thermal-zones {
> > -             cpu_thermal: cpu-thermal {
> > +             core-cluster {
> >                       polling-delay-passive = <1000>;
> >                       polling-delay = <5000>;
> >                       thermal-sensors = <&tmu 0>;
> >
> >                       trips {
> > -                             cpu_alert: cpu-alert {
> > +                             core_cluster_alert: core-cluster-alert {
> >                                       temperature = <85000>;
> >                                       hysteresis = <2000>;
> >                                       type = "passive";
> >                               };
> >
> > -                             cpu_crit: cpu-crit {
> > +                             core_cluster_crit: core-cluster-crit {
> >                                       temperature = <95000>;
> >                                       hysteresis = <2000>;
> >                                       type = "critical"; @@ -150,7
> > +150,7 @@
> >
> >                       cooling-maps {
> >                               map0 {
> > -                                     trip = <&cpu_alert>;
> > +                                     trip = <&core_cluster_alert>;
> >                                       cooling-device =
> >                                               <&cpu0
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> >                                               <&cpu1
> THERMAL_NO_LIMIT
> > THERMAL_NO_LIMIT>, @@ -163,6 +163,26 @@
> >                               };
> >                       };
> >               };
> > +
> > +             platform {
> > +                     polling-delay-passive = <1000>;
> > +                     polling-delay = <5000>;
> > +                     thermal-sensors = <&tmu 1>;
> > +
> > +                     trips {
> > +                             platform-alert {
> > +                                     temperature = <85000>;
> > +                                     hysteresis = <2000>;
> > +                                     type = "passive";
> > +                             };
> > +
> > +                             platform-crit {
> > +                                     temperature = <95000>;
> > +                                     hysteresis = <2000>;
> > +                                     type = "critical";
> > +                             };
> > +                     };
> > +             };
> 
> 
> Unfortunately, the documentation says the thermal zone node must contain a
> "cooling-maps" entry.
That's a question.
If I add "cooling-maps", then multiple zones will use same cooling device.
If I don't, documentation says it is a " Required properties".
I am wondering how it was dealt with on other platform supporting multiple zones.

Hi Shawn,
What's your opinions about this?

BR,
Andy

> 
> It makes sense as if there is no cooling device, there is no action to be taken
> thus polling the temperature just add noise to the system.
> 
> 
> 
> >       };
> >
> >       timer {
> >
> 
> 
> --
> 
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.lin
> aro.org%2F&amp;data=02%7C01%7Candy.tang%40nxp.com%7Cda3cde05ae944
> d7fdb5508d6beb1f69a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
> 636906068893110033&amp;sdata=2KAlALDq3%2FR0W5jGf1E4DR1GHTEpyO2w
> YwIXZT7ZS8E%3D&amp;reserved=0> Linaro.org │ Open source software for
> ARM SoCs
> 
> Follow Linaro:
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.fac
> ebook.com%2Fpages%2FLinaro&amp;data=02%7C01%7Candy.tang%40nxp.com
> %7Cda3cde05ae944d7fdb5508d6beb1f69a%7C686ea1d3bc2b4c6fa92cd99c5c30
> 1635%7C0%7C0%7C636906068893110033&amp;sdata=2aINIFXnw6nNehmU5xK
> SgpeekA7Pv6YJVVB7gA5FxYU%3D&amp;reserved=0> Facebook |
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitter.c
> om%2F%23!%2Flinaroorg&amp;data=02%7C01%7Candy.tang%40nxp.com%7Cd
> a3cde05ae944d7fdb5508d6beb1f69a%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0%7C0%7C636906068893110033&amp;sdata=X6eDvdATzokwP7VVKx02yge
> 6Akebd89%2BWRsYRE5rgf8%3D&amp;reserved=0> Twitter |
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.lin
> aro.org%2Flinaro-blog%2F&amp;data=02%7C01%7Candy.tang%40nxp.com%7C
> da3cde05ae944d7fdb5508d6beb1f69a%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0%7C0%7C636906068893110033&amp;sdata=0F6G6GKdBDQAWZ50OjVcvt
> 1cvksBlk1jLngW1m4mDEw%3D&amp;reserved=0> Blog


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

* Re: [EXT] Re: [PATCH v5] arm64: dts: ls1088a: add one more thermal zone node
  2019-04-12  7:47   ` [EXT] " Andy Tang
@ 2019-04-12 12:19     ` Daniel Lezcano
  2019-04-18  3:28       ` Andy Tang
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Lezcano @ 2019-04-12 12:19 UTC (permalink / raw)
  To: Andy Tang, shawnguo
  Cc: Leo Li, robh+dt, mark.rutland, linux-arm-kernel, devicetree,
	linux-kernel, linux-pm, rui.zhang, edubezval

On 12/04/2019 09:47, Andy Tang wrote:
> 
>> -----Original Message-----
>> From: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Sent: 2019年4月12日 3:15
>> To: Andy Tang <andy.tang@nxp.com>; shawnguo@kernel.org
>> Cc: Leo Li <leoyang.li@nxp.com>; robh+dt@kernel.org; mark.rutland@arm.com;
>> linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
>> linux-kernel@vger.kernel.org; linux-pm@vger.kernel.org; rui.zhang@intel.com;
>> edubezval@gmail.com
>> Subject: [EXT] Re: [PATCH v5] arm64: dts: ls1088a: add one more thermal zone
>> node
>>
>> WARNING: This email was created outside of NXP. DO NOT CLICK links or
>> attachments unless you recognize the sender and know the content is safe.
>>
>>
>>
>> On 11/04/2019 10:32, Yuantian Tang wrote:
>>> Ls1088a has 2 thermal sensors, core cluster and SoC platform. Core
>>> cluster sensor is used to monitor the temperature of core and SoC
>>> platform is for platform. The current dts only support the first sensor.
>>> This patch adds the second sensor node to dts to enable it.
>>>
>>> Signed-off-by: Yuantian Tang <andy.tang@nxp.com>
>>> ---
>>> v5:
>>>       - update the thermal zone name due to the length limitation
>>>       - remove cooling map in platform zone
>>> v4:
>>>       - use hyphen instead of underscore in node name
>>> v3:
>>>       - use more descriptive name for each zone
>>> v2:
>>>       - Add more information about sensors to description
>>>  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi |   28
>> ++++++++++++++++++++---
>>>  1 files changed, 24 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
>>> b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
>>> index de93b42..de39672 100644
>>> --- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
>>> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
>>> @@ -129,19 +129,19 @@
>>>       };
>>>
>>>       thermal-zones {
>>> -             cpu_thermal: cpu-thermal {
>>> +             core-cluster {
>>>                       polling-delay-passive = <1000>;
>>>                       polling-delay = <5000>;
>>>                       thermal-sensors = <&tmu 0>;
>>>
>>>                       trips {
>>> -                             cpu_alert: cpu-alert {
>>> +                             core_cluster_alert: core-cluster-alert {
>>>                                       temperature = <85000>;
>>>                                       hysteresis = <2000>;
>>>                                       type = "passive";
>>>                               };
>>>
>>> -                             cpu_crit: cpu-crit {
>>> +                             core_cluster_crit: core-cluster-crit {
>>>                                       temperature = <95000>;
>>>                                       hysteresis = <2000>;
>>>                                       type = "critical"; @@ -150,7
>>> +150,7 @@
>>>
>>>                       cooling-maps {
>>>                               map0 {
>>> -                                     trip = <&cpu_alert>;
>>> +                                     trip = <&core_cluster_alert>;
>>>                                       cooling-device =
>>>                                               <&cpu0
>> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>>>                                               <&cpu1
>> THERMAL_NO_LIMIT
>>> THERMAL_NO_LIMIT>, @@ -163,6 +163,26 @@
>>>                               };
>>>                       };
>>>               };
>>> +
>>> +             platform {
>>> +                     polling-delay-passive = <1000>;
>>> +                     polling-delay = <5000>;
>>> +                     thermal-sensors = <&tmu 1>;
>>> +
>>> +                     trips {
>>> +                             platform-alert {
>>> +                                     temperature = <85000>;
>>> +                                     hysteresis = <2000>;
>>> +                                     type = "passive";
>>> +                             };
>>> +
>>> +                             platform-crit {
>>> +                                     temperature = <95000>;
>>> +                                     hysteresis = <2000>;
>>> +                                     type = "critical";
>>> +                             };
>>> +                     };
>>> +             };
>>
>>
>> Unfortunately, the documentation says the thermal zone node must contain a
>> "cooling-maps" entry.
> That's a question.
> If I add "cooling-maps", then multiple zones will use same cooling device.
> If I don't, documentation says it is a " Required properties".
> I am wondering how it was dealt with on other platform supporting multiple zones.

I think the problem comes from the fact there are no governors capable
of handling multiple thermal zones.

The additional cooling device is the devfreq but as the mali driver is
not upstream, the cooling map for this device does not appear. We can
find the definitions and the driver in the AOSP [1].

Moreover, the power is usually managed by the IPA governor, but this one
can not handle multiple thermal zones, so the cooling devices are joined
together into a single thermal zone with their contribution to the
cooling effect.

Other definitions I have seen upstream are incomplete or broken.

[1]
https://android.googlesource.com/kernel/hikey-linaro/+/refs/heads/android-hikey-linaro-4.14/arch/arm64/boot/dts/hisilicon/hi3660.dtsi#1340

> Hi Shawn,
> What's your opinions about this?



-- 
 <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] 6+ messages in thread

* RE: [EXT] Re: [PATCH v5] arm64: dts: ls1088a: add one more thermal zone node
  2019-04-12 12:19     ` Daniel Lezcano
@ 2019-04-18  3:28       ` Andy Tang
  2019-04-21  6:45         ` Shawn Guo
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Tang @ 2019-04-18  3:28 UTC (permalink / raw)
  To: Daniel Lezcano, shawnguo
  Cc: Leo Li, robh+dt, mark.rutland, linux-arm-kernel, devicetree,
	linux-kernel, linux-pm, rui.zhang, edubezval


> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: 2019年4月12日 20:19
> To: Andy Tang <andy.tang@nxp.com>; shawnguo@kernel.org
> Cc: Leo Li <leoyang.li@nxp.com>; robh+dt@kernel.org; mark.rutland@arm.com;
> linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-pm@vger.kernel.org; rui.zhang@intel.com;
> edubezval@gmail.com
> Subject: Re: [EXT] Re: [PATCH v5] arm64: dts: ls1088a: add one more thermal
> zone node
> 
> WARNING: This email was created outside of NXP. DO NOT CLICK links or
> attachments unless you recognize the sender and know the content is safe.
> 
> 
> 
> On 12/04/2019 09:47, Andy Tang wrote:
> >
> >> -----Original Message-----
> >> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> >> Sent: 2019年4月12日 3:15
> >> To: Andy Tang <andy.tang@nxp.com>; shawnguo@kernel.org
> >> Cc: Leo Li <leoyang.li@nxp.com>; robh+dt@kernel.org;
> >> mark.rutland@arm.com; linux-arm-kernel@lists.infradead.org;
> >> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> linux-pm@vger.kernel.org; rui.zhang@intel.com; edubezval@gmail.com
> >> Subject: [EXT] Re: [PATCH v5] arm64: dts: ls1088a: add one more
> >> thermal zone node
> >>
> >> WARNING: This email was created outside of NXP. DO NOT CLICK links or
> >> attachments unless you recognize the sender and know the content is safe.
> >>
> >>
> >>
> >> On 11/04/2019 10:32, Yuantian Tang wrote:
> >>> Ls1088a has 2 thermal sensors, core cluster and SoC platform. Core
> >>> cluster sensor is used to monitor the temperature of core and SoC
> >>> platform is for platform. The current dts only support the first sensor.
> >>> This patch adds the second sensor node to dts to enable it.
> >>>
> >>> Signed-off-by: Yuantian Tang <andy.tang@nxp.com>
> >>> ---
> >>> v5:
> >>>       - update the thermal zone name due to the length limitation
> >>>       - remove cooling map in platform zone
> >>> v4:
> >>>       - use hyphen instead of underscore in node name
> >>> v3:
> >>>       - use more descriptive name for each zone
> >>> v2:
> >>>       - Add more information about sensors to description
> >>>  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi |   28
> >> ++++++++++++++++++++---
> >>>  1 files changed, 24 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> >>> b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> >>> index de93b42..de39672 100644
> >>> --- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> >>> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> >>> @@ -129,19 +129,19 @@
> >>>       };
> >>>
> >>>       thermal-zones {
> >>> -             cpu_thermal: cpu-thermal {
> >>> +             core-cluster {
> >>>                       polling-delay-passive = <1000>;
> >>>                       polling-delay = <5000>;
> >>>                       thermal-sensors = <&tmu 0>;
> >>>
> >>>                       trips {
> >>> -                             cpu_alert: cpu-alert {
> >>> +                             core_cluster_alert: core-cluster-alert
> >>> + {
> >>>                                       temperature = <85000>;
> >>>                                       hysteresis = <2000>;
> >>>                                       type = "passive";
> >>>                               };
> >>>
> >>> -                             cpu_crit: cpu-crit {
> >>> +                             core_cluster_crit: core-cluster-crit {
> >>>                                       temperature = <95000>;
> >>>                                       hysteresis = <2000>;
> >>>                                       type = "critical"; @@ -150,7
> >>> +150,7 @@
> >>>
> >>>                       cooling-maps {
> >>>                               map0 {
> >>> -                                     trip = <&cpu_alert>;
> >>> +                                     trip = <&core_cluster_alert>;
> >>>                                       cooling-device =
> >>>                                               <&cpu0
> >> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> >>>                                               <&cpu1
> >> THERMAL_NO_LIMIT
> >>> THERMAL_NO_LIMIT>, @@ -163,6 +163,26 @@
> >>>                               };
> >>>                       };
> >>>               };
> >>> +
> >>> +             platform {
> >>> +                     polling-delay-passive = <1000>;
> >>> +                     polling-delay = <5000>;
> >>> +                     thermal-sensors = <&tmu 1>;
> >>> +
> >>> +                     trips {
> >>> +                             platform-alert {
> >>> +                                     temperature = <85000>;
> >>> +                                     hysteresis = <2000>;
> >>> +                                     type = "passive";
> >>> +                             };
> >>> +
> >>> +                             platform-crit {
> >>> +                                     temperature = <95000>;
> >>> +                                     hysteresis = <2000>;
> >>> +                                     type = "critical";
> >>> +                             };
> >>> +                     };
> >>> +             };
> >>
> >>
> >> Unfortunately, the documentation says the thermal zone node must
> >> contain a "cooling-maps" entry.
> > That's a question.
> > If I add "cooling-maps", then multiple zones will use same cooling device.
> > If I don't, documentation says it is a " Required properties".
> > I am wondering how it was dealt with on other platform supporting multiple
> zones.
> 
> I think the problem comes from the fact there are no governors capable of
> handling multiple thermal zones.
> 
> The additional cooling device is the devfreq but as the mali driver is not
> upstream, the cooling map for this device does not appear. We can find the
> definitions and the driver in the AOSP [1].
> 
> Moreover, the power is usually managed by the IPA governor, but this one can
> not handle multiple thermal zones, so the cooling devices are joined together
> into a single thermal zone with their contribution to the cooling effect.
> 
> Other definitions I have seen upstream are incomplete or broken.
> 
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fandroid.g
> ooglesource.com%2Fkernel%2Fhikey-linaro%2F%2B%2Frefs%2Fheads%2Fandr
> oid-hikey-linaro-4.14%2Farch%2Farm64%2Fboot%2Fdts%2Fhisilicon%2Fhi3660.
> dtsi%231340&amp;data=02%7C01%7Candy.tang%40nxp.com%7C559218b1622e
> 4603251908d6bf410e4b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C636906683466836985&amp;sdata=952xIcrr11g%2B0nl2lgKrG18SlRVgGV6L%2
> BMBL3U6UaiQ%3D&amp;reserved=0
Interesting. I think we have to accept the imperfect for now.

Hi Shawn,
Any comments?

BR,
Andy
> 
> > Hi Shawn,
> > What's your opinions about this?
> 
> 
> 
> --
> 
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.lin
> aro.org%2F&amp;data=02%7C01%7Candy.tang%40nxp.com%7C559218b1622e4
> 603251908d6bf410e4b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
> 636906683466836985&amp;sdata=AJvIX8XJ7j1JkpWgD1GuEcP2keO0R1muAzt9
> n2qgU3A%3D&amp;reserved=0> Linaro.org │ Open source software for ARM
> SoCs
> 
> Follow Linaro:
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.fac
> ebook.com%2Fpages%2FLinaro&amp;data=02%7C01%7Candy.tang%40nxp.com
> %7C559218b1622e4603251908d6bf410e4b%7C686ea1d3bc2b4c6fa92cd99c5c30
> 1635%7C0%7C0%7C636906683466836985&amp;sdata=cSUB5H7sqpUP%2B0EP
> HgXEgaRZxYit%2FGN9Vt6XrQuy0DA%3D&amp;reserved=0> Facebook |
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitter.c
> om%2F%23!%2Flinaroorg&amp;data=02%7C01%7Candy.tang%40nxp.com%7C5
> 59218b1622e4603251908d6bf410e4b%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0%7C0%7C636906683466836985&amp;sdata=OH7w3%2FUwMEAuBS5%2
> BwPfJl9USz9gT6LpTIw1ORmMeaXA%3D&amp;reserved=0> Twitter |
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.lin
> aro.org%2Flinaro-blog%2F&amp;data=02%7C01%7Candy.tang%40nxp.com%7C
> 559218b1622e4603251908d6bf410e4b%7C686ea1d3bc2b4c6fa92cd99c5c30163
> 5%7C0%7C0%7C636906683466836985&amp;sdata=LtyQn%2BVliawr2tuRhCB84
> WH1RMEio3aFQ2ExCfKn7ok%3D&amp;reserved=0> Blog


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

* Re: [EXT] Re: [PATCH v5] arm64: dts: ls1088a: add one more thermal zone node
  2019-04-18  3:28       ` Andy Tang
@ 2019-04-21  6:45         ` Shawn Guo
  0 siblings, 0 replies; 6+ messages in thread
From: Shawn Guo @ 2019-04-21  6:45 UTC (permalink / raw)
  To: Andy Tang
  Cc: Daniel Lezcano, Leo Li, robh+dt, mark.rutland, linux-arm-kernel,
	devicetree, linux-kernel, linux-pm, rui.zhang, edubezval

On Thu, Apr 18, 2019 at 03:28:56AM +0000, Andy Tang wrote:
> 
> > -----Original Message-----
> > From: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Sent: 2019年4月12日 20:19
> > To: Andy Tang <andy.tang@nxp.com>; shawnguo@kernel.org
> > Cc: Leo Li <leoyang.li@nxp.com>; robh+dt@kernel.org; mark.rutland@arm.com;
> > linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
> > linux-kernel@vger.kernel.org; linux-pm@vger.kernel.org; rui.zhang@intel.com;
> > edubezval@gmail.com
> > Subject: Re: [EXT] Re: [PATCH v5] arm64: dts: ls1088a: add one more thermal
> > zone node
> > 
> > WARNING: This email was created outside of NXP. DO NOT CLICK links or
> > attachments unless you recognize the sender and know the content is safe.
> > 
> > 
> > 
> > On 12/04/2019 09:47, Andy Tang wrote:
> > >
> > >> -----Original Message-----
> > >> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> > >> Sent: 2019年4月12日 3:15
> > >> To: Andy Tang <andy.tang@nxp.com>; shawnguo@kernel.org
> > >> Cc: Leo Li <leoyang.li@nxp.com>; robh+dt@kernel.org;
> > >> mark.rutland@arm.com; linux-arm-kernel@lists.infradead.org;
> > >> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> > >> linux-pm@vger.kernel.org; rui.zhang@intel.com; edubezval@gmail.com
> > >> Subject: [EXT] Re: [PATCH v5] arm64: dts: ls1088a: add one more
> > >> thermal zone node
> > >>
> > >> WARNING: This email was created outside of NXP. DO NOT CLICK links or
> > >> attachments unless you recognize the sender and know the content is safe.
> > >>
> > >>
> > >>
> > >> On 11/04/2019 10:32, Yuantian Tang wrote:
> > >>> Ls1088a has 2 thermal sensors, core cluster and SoC platform. Core
> > >>> cluster sensor is used to monitor the temperature of core and SoC
> > >>> platform is for platform. The current dts only support the first sensor.
> > >>> This patch adds the second sensor node to dts to enable it.
> > >>>
> > >>> Signed-off-by: Yuantian Tang <andy.tang@nxp.com>
> > >>> ---
> > >>> v5:
> > >>>       - update the thermal zone name due to the length limitation
> > >>>       - remove cooling map in platform zone
> > >>> v4:
> > >>>       - use hyphen instead of underscore in node name
> > >>> v3:
> > >>>       - use more descriptive name for each zone
> > >>> v2:
> > >>>       - Add more information about sensors to description
> > >>>  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi |   28
> > >> ++++++++++++++++++++---
> > >>>  1 files changed, 24 insertions(+), 4 deletions(-)
> > >>>
> > >>> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> > >>> b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> > >>> index de93b42..de39672 100644
> > >>> --- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> > >>> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> > >>> @@ -129,19 +129,19 @@
> > >>>       };
> > >>>
> > >>>       thermal-zones {
> > >>> -             cpu_thermal: cpu-thermal {
> > >>> +             core-cluster {
> > >>>                       polling-delay-passive = <1000>;
> > >>>                       polling-delay = <5000>;
> > >>>                       thermal-sensors = <&tmu 0>;
> > >>>
> > >>>                       trips {
> > >>> -                             cpu_alert: cpu-alert {
> > >>> +                             core_cluster_alert: core-cluster-alert
> > >>> + {
> > >>>                                       temperature = <85000>;
> > >>>                                       hysteresis = <2000>;
> > >>>                                       type = "passive";
> > >>>                               };
> > >>>
> > >>> -                             cpu_crit: cpu-crit {
> > >>> +                             core_cluster_crit: core-cluster-crit {
> > >>>                                       temperature = <95000>;
> > >>>                                       hysteresis = <2000>;
> > >>>                                       type = "critical"; @@ -150,7
> > >>> +150,7 @@
> > >>>
> > >>>                       cooling-maps {
> > >>>                               map0 {
> > >>> -                                     trip = <&cpu_alert>;
> > >>> +                                     trip = <&core_cluster_alert>;
> > >>>                                       cooling-device =
> > >>>                                               <&cpu0
> > >> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > >>>                                               <&cpu1
> > >> THERMAL_NO_LIMIT
> > >>> THERMAL_NO_LIMIT>, @@ -163,6 +163,26 @@
> > >>>                               };
> > >>>                       };
> > >>>               };
> > >>> +
> > >>> +             platform {
> > >>> +                     polling-delay-passive = <1000>;
> > >>> +                     polling-delay = <5000>;
> > >>> +                     thermal-sensors = <&tmu 1>;
> > >>> +
> > >>> +                     trips {
> > >>> +                             platform-alert {
> > >>> +                                     temperature = <85000>;
> > >>> +                                     hysteresis = <2000>;
> > >>> +                                     type = "passive";
> > >>> +                             };
> > >>> +
> > >>> +                             platform-crit {
> > >>> +                                     temperature = <95000>;
> > >>> +                                     hysteresis = <2000>;
> > >>> +                                     type = "critical";
> > >>> +                             };
> > >>> +                     };
> > >>> +             };
> > >>
> > >>
> > >> Unfortunately, the documentation says the thermal zone node must
> > >> contain a "cooling-maps" entry.
> > > That's a question.
> > > If I add "cooling-maps", then multiple zones will use same cooling device.
> > > If I don't, documentation says it is a " Required properties".
> > > I am wondering how it was dealt with on other platform supporting multiple
> > zones.
> > 
> > I think the problem comes from the fact there are no governors capable of
> > handling multiple thermal zones.
> > 
> > The additional cooling device is the devfreq but as the mali driver is not
> > upstream, the cooling map for this device does not appear. We can find the
> > definitions and the driver in the AOSP [1].
> > 
> > Moreover, the power is usually managed by the IPA governor, but this one can
> > not handle multiple thermal zones, so the cooling devices are joined together
> > into a single thermal zone with their contribution to the cooling effect.
> > 
> > Other definitions I have seen upstream are incomplete or broken.
> > 
> > [1]
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fandroid.g
> > ooglesource.com%2Fkernel%2Fhikey-linaro%2F%2B%2Frefs%2Fheads%2Fandr
> > oid-hikey-linaro-4.14%2Farch%2Farm64%2Fboot%2Fdts%2Fhisilicon%2Fhi3660.
> > dtsi%231340&amp;data=02%7C01%7Candy.tang%40nxp.com%7C559218b1622e
> > 4603251908d6bf410e4b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> > C636906683466836985&amp;sdata=952xIcrr11g%2B0nl2lgKrG18SlRVgGV6L%2
> > BMBL3U6UaiQ%3D&amp;reserved=0
> Interesting. I think we have to accept the imperfect for now.
> 
> Hi Shawn,
> Any comments?

Sorry.  I'm not going to take a DTS change that contradicts with
bindings.  If you think bindings is not reasonable, change the bindings
first.

Shawn

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

end of thread, other threads:[~2019-04-21  6:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-11  8:32 [PATCH v5] arm64: dts: ls1088a: add one more thermal zone node Yuantian Tang
2019-04-11 19:14 ` Daniel Lezcano
2019-04-12  7:47   ` [EXT] " Andy Tang
2019-04-12 12:19     ` Daniel Lezcano
2019-04-18  3:28       ` Andy Tang
2019-04-21  6:45         ` Shawn Guo

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