LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v6] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-04-23  2:25 Yuantian Tang
  2019-05-10  3:13 ` Shawn Guo
  0 siblings, 1 reply; 7+ messages in thread
From: Yuantian Tang @ 2019-04-23  2:25 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>
---
v6:
        - add cooling device map to cpu0-7 in platform node.
 arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi |   43 +++++++++++++++++++++--
 1 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
index 661137f..a697a82 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,42 @@
 
 			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>,
+						<&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>;
+				};
+			};
+		};
+
+		platform {
+			polling-delay-passive = <1000>;
+			polling-delay = <5000>;
+			thermal-sensors = <&tmu 1>;
+
+			trips {
+				platform_alert: platform-alert {
+					temperature = <85000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+
+				platform_crit: platform-crit {
+					temperature = <95000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+
+			cooling-maps {
+				map0 {
+					trip = <&platform_alert>;
 					cooling-device =
 						<&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
 						<&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-- 
1.7.1


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

* Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal zone node
  2019-04-23  2:25 [PATCH v6] arm64: dts: ls1088a: add one more thermal zone node Yuantian Tang
@ 2019-05-10  3:13 ` Shawn Guo
  2019-05-10  3:40   ` [EXT] " Andy Tang
  0 siblings, 1 reply; 7+ messages in thread
From: Shawn Guo @ 2019-05-10  3:13 UTC (permalink / raw)
  To: Yuantian Tang
  Cc: leoyang.li, robh+dt, mark.rutland, linux-arm-kernel, devicetree,
	linux-kernel, linux-pm, daniel.lezcano, rui.zhang, edubezval

On Tue, Apr 23, 2019 at 10:25:07AM +0800, 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>
> ---
> v6:
>         - add cooling device map to cpu0-7 in platform node.

@Daniel, are you fine with this version?

Shawn

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

* RE: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal zone node
  2019-05-10  3:13 ` Shawn Guo
@ 2019-05-10  3:40   ` Andy Tang
  2019-05-10  7:16     ` Daniel Lezcano
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Tang @ 2019-05-10  3:40 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Leo Li, robh+dt, mark.rutland, linux-arm-kernel, devicetree,
	linux-kernel, linux-pm, daniel.lezcano, rui.zhang, edubezval

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

> -----Original Message-----
> From: Shawn Guo <shawnguo@kernel.org>
> Sent: 2019年5月10日 11:14
> To: Andy Tang <andy.tang@nxp.com>
> 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; daniel.lezcano@linaro.org; rui.zhang@intel.com;
> edubezval@gmail.com
> Subject: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal
> zone node
> 
> Caution: EXT Email
> 
> On Tue, Apr 23, 2019 at 10:25:07AM +0800, 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>
> > ---
> > v6:
> >         - add cooling device map to cpu0-7 in platform node.
I like to explain a little. I think it makes sense that multiple thermal zone map to same cooling device. 
In this way, no matter which thermal zone raises a temp alarm, it can call cooling device to chill out.
I also asked cpufreq maintainer about the cooling map issue, he think it would be fine.
I have tested and no issue found. 

Daniel, what's your thought?

Thanks,
Andy
> 
> @Daniel, are you fine with this version?
> 
> Shawn

[-- Attachment #2: Type: message/rfc822, Size: 8886 bytes --]

From: Viresh Kumar <viresh.kumar@linaro.org>
To: Andy Tang <andy.tang@nxp.com>
Subject: [EXT] Re: Ask for help about cooling device map
Date: Mon, 22 Apr 2019 08:13:23 +0000
Message-ID: <20190422081323.lkgwbso36sshafbf@vireshk-i7>

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 22-04-19, 07:09, Andy Tang wrote:
> Hi Viresh,
>
> Sorry to bother you. I have a question, hope I can get you help.
> Here it is:
>
> I want to add multiple "Thermal Zone" support in dts ( driver is ready).
> The final dts looks like below:
>
>         thermal-zones {
>                 cpu_thermal: cpu-thermal {
>                         polling-delay-passive = <1000>;
>                         polling-delay = <5000>;
>                         thermal-sensors = <&tmu 0>;
>
>                         trips {
>                                 ccu_alert: ccu-alert {
>                                         temperature = <85000>;
>                                         hysteresis = <2000>;
>                                         type = "passive";
>                                 };
>                                 ccu_crit: ccu-crit {
>                                         temperature = <95000>;
>                                         hysteresis = <2000>;
>                                         type = "critical";
>                         cooling-maps {
>                                 map0 {
>                                         trip = <&ccu_alert>;
>                                         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>;
>                                 };
>                         };
>                 };
>                 platform {
>                         polling-delay-passive = <1000>;
>                         polling-delay = <5000>;
>                         thermal-sensors = <&tmu 1>;
>                         trips {
>                                 plt_alert: plt-alert {
>                                         temperature = <85000>;
>                                         hysteresis = <2000>;
>                                         type = "passive";
>                         };
>                                 plt_crit: plt-crit {
>                                         temperature = <95000>;
>                                         hysteresis = <2000>;
>                                         type = "critical";
>                                 };
>                         };
>                         cooling-maps {
>                                 map0 {
>                                         trip = <&plt_alert>;
>                                         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>;
>                         }
> }
>
> Here we have 2 thermal zones, but both map cpu0-7 as cooling device.  I have tested and didn't find any issues.
> My query is: is it allowed to map same device as "cooling device" to more than one "thermal zone"?

That should be fine IMO.

--
viresh

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

* Re: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal zone node
  2019-05-10  3:40   ` [EXT] " Andy Tang
@ 2019-05-10  7:16     ` Daniel Lezcano
  2019-05-10  8:47       ` Andy Tang
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Lezcano @ 2019-05-10  7:16 UTC (permalink / raw)
  To: Andy Tang, Shawn Guo
  Cc: Leo Li, robh+dt, mark.rutland, linux-arm-kernel, devicetree,
	linux-kernel, linux-pm, rui.zhang, edubezval

On 10/05/2019 05:40, Andy Tang wrote:
>> -----Original Message-----
>> From: Shawn Guo <shawnguo@kernel.org>
>> Sent: 2019年5月10日 11:14
>> To: Andy Tang <andy.tang@nxp.com>
>> 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; daniel.lezcano@linaro.org; rui.zhang@intel.com;
>> edubezval@gmail.com
>> Subject: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal
>> zone node
>>
>> Caution: EXT Email
>>
>> On Tue, Apr 23, 2019 at 10:25:07AM +0800, 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>
>>> ---
>>> v6:
>>>         - add cooling device map to cpu0-7 in platform node.
> I like to explain a little. I think it makes sense that multiple thermal zone map to same cooling device. 
> In this way, no matter which thermal zone raises a temp alarm, it can call cooling device to chill out.
> I also asked cpufreq maintainer about the cooling map issue, he think it would be fine.
> I have tested and no issue found. 
> 
> Daniel, what's your thought?

If there are multiple thermal zones, they will be managed by different
instances of a thermal governor. Each instances will act on the shared
cooling device and will collide in their decisions:

 - If the sensors are closed, their behavior will be similar regarding
the temperature. The governors may take the same decision for the
cooling device. But in such case having just one thermal zone managed is
enough.

 - If the sensors are not closed, their behavior will be different
regarding the temperature. The governors will take different decision
regarding the cooling device (one will decrease the freq, other will
increase the freq).

As the thermal governors are not able to manage several thermal zones
and there is one cooling device (the cpu cooling device), this setup
won't work as expected IMO.

The setup making sense is having a thermal zone per 'cluster' and a
cooling device per 'cluster'. That means the platform has one clock line
per 'cluster'. The thermal management happens in a self-contained
thermal zone (one cooling device - one governor - one thermal zone).

In the case of HMP, other combinations are possible to be optimal.



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

* RE: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal zone node
  2019-05-10  7:16     ` Daniel Lezcano
@ 2019-05-10  8:47       ` Andy Tang
  2019-05-10 10:12         ` Viresh Kumar
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Tang @ 2019-05-10  8:47 UTC (permalink / raw)
  To: Daniel Lezcano, Shawn Guo, viresh.kumar
  Cc: Leo Li, robh+dt, mark.rutland, linux-arm-kernel, devicetree,
	linux-kernel, linux-pm, rui.zhang, edubezval

+ Viresh for help.

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: 2019年5月10日 15:17
> To: Andy Tang <andy.tang@nxp.com>; Shawn Guo <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 v6] arm64: dts: ls1088a: add one more thermal
> zone node
> 
> Caution: EXT Email
> 
> On 10/05/2019 05:40, Andy Tang wrote:
> >> -----Original Message-----
> >> From: Shawn Guo <shawnguo@kernel.org>
> >> Sent: 2019年5月10日 11:14
> >> To: Andy Tang <andy.tang@nxp.com>
> >> 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; daniel.lezcano@linaro.org;
> >> rui.zhang@intel.com; edubezval@gmail.com
> >> Subject: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more
> >> thermal zone node
> >>
> >> Caution: EXT Email
> >>
> >> On Tue, Apr 23, 2019 at 10:25:07AM +0800, 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>
> >>> ---
> >>> v6:
> >>>         - add cooling device map to cpu0-7 in platform node.
> > I like to explain a little. I think it makes sense that multiple thermal zone
> map to same cooling device.
> > In this way, no matter which thermal zone raises a temp alarm, it can call
> cooling device to chill out.
> > I also asked cpufreq maintainer about the cooling map issue, he think it
> would be fine.
> > I have tested and no issue found.
> >
> > Daniel, what's your thought?
> 
> If there are multiple thermal zones, they will be managed by different
> instances of a thermal governor. Each instances will act on the shared cooling
> device and will collide in their decisions:
> 
>  - If the sensors are closed, their behavior will be similar regarding the
> temperature. The governors may take the same decision for the cooling
> device. But in such case having just one thermal zone managed is enough.
> 
>  - If the sensors are not closed, their behavior will be different regarding the
> temperature. The governors will take different decision regarding the cooling
> device (one will decrease the freq, other will increase the freq).
> 
> As the thermal governors are not able to manage several thermal zones and
> there is one cooling device (the cpu cooling device), this setup won't work as
> expected IMO.
> 
> The setup making sense is having a thermal zone per 'cluster' and a cooling
> device per 'cluster'. That means the platform has one clock line per 'cluster'.
> The thermal management happens in a self-contained thermal zone (one
> cooling device - one governor - one thermal zone).
> 
> In the case of HMP, other combinations are possible to be optimal.
Hi Viresh,

I want to map multiple thermal zones to the same cooling device. The above is the discussion about it.
It seems reasonable. But I am not expert on this. Could you please provide some thoughts? Thanks.

BR,
Andy
> 
> 
> 
> --
> 
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> linaro.org%2F&amp;data=02%7C01%7Candy.tang%40nxp.com%7C935b7a08
> 31cc466da40808d6d5176f8e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636930693965540189&amp;sdata=WK9NDBVwzhCC9WMkmjLObJ
> OBQwRG%2Fboed%2FKx18xiBNQ%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.
> facebook.com%2Fpages%2FLinaro&amp;data=02%7C01%7Candy.tang%40nx
> p.com%7C935b7a0831cc466da40808d6d5176f8e%7C686ea1d3bc2b4c6fa92c
> d99c5c301635%7C0%7C0%7C636930693965550202&amp;sdata=O0mWO76
> 9sK2ZMGX9AxgLGYNVfkFHBD4ZIGCclttvyPI%3D&amp;reserved=0>
> Facebook |
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitte
> r.com%2F%23!%2Flinaroorg&amp;data=02%7C01%7Candy.tang%40nxp.com
> %7C935b7a0831cc466da40808d6d5176f8e%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C636930693965550202&amp;sdata=tV%2Bi8Bk3OqO
> h%2FZHpBr2NQDACVvtGi8KNGQt5dZaTeyg%3D&amp;reserved=0> Twitter |
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> linaro.org%2Flinaro-blog%2F&amp;data=02%7C01%7Candy.tang%40nxp.co
> m%7C935b7a0831cc466da40808d6d5176f8e%7C686ea1d3bc2b4c6fa92cd99
> c5c301635%7C0%7C0%7C636930693965550202&amp;sdata=0E8a938xEt7l2
> MBLnMETsCQKfhJMgmzFNtuCKPXf5SY%3D&amp;reserved=0> Blog


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

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

On 10-05-19, 08:47, Andy Tang wrote:
> + Viresh for help.
> 
> > -----Original Message-----
> > From: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Sent: 2019年5月10日 15:17
> > To: Andy Tang <andy.tang@nxp.com>; Shawn Guo <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 v6] arm64: dts: ls1088a: add one more thermal
> > zone node
> > 
> > Caution: EXT Email
> > 
> > On 10/05/2019 05:40, Andy Tang wrote:
> > >> -----Original Message-----
> > >> From: Shawn Guo <shawnguo@kernel.org>
> > >> Sent: 2019年5月10日 11:14
> > >> To: Andy Tang <andy.tang@nxp.com>
> > >> 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; daniel.lezcano@linaro.org;
> > >> rui.zhang@intel.com; edubezval@gmail.com
> > >> Subject: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more
> > >> thermal zone node
> > >>
> > >> Caution: EXT Email
> > >>
> > >> On Tue, Apr 23, 2019 at 10:25:07AM +0800, 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>
> > >>> ---
> > >>> v6:
> > >>>         - add cooling device map to cpu0-7 in platform node.
> > > I like to explain a little. I think it makes sense that multiple thermal zone
> > map to same cooling device.
> > > In this way, no matter which thermal zone raises a temp alarm, it can call
> > cooling device to chill out.
> > > I also asked cpufreq maintainer about the cooling map issue, he think it
> > would be fine.

Yes, you asked me and I said it should be okay.

> > > I have tested and no issue found.
> > >
> > > Daniel, what's your thought?
> > 
> > If there are multiple thermal zones, they will be managed by different
> > instances of a thermal governor. Each instances will act on the shared cooling
> > device and will collide in their decisions:
> > 
> >  - If the sensors are closed, their behavior will be similar regarding the
> > temperature. The governors may take the same decision for the cooling
> > device. But in such case having just one thermal zone managed is enough.
> > 
> >  - If the sensors are not closed, their behavior will be different regarding the
> > temperature. The governors will take different decision regarding the cooling
> > device (one will decrease the freq, other will increase the freq).
> > 
> > As the thermal governors are not able to manage several thermal zones and
> > there is one cooling device (the cpu cooling device), this setup won't work as
> > expected IMO.
> > 
> > The setup making sense is having a thermal zone per 'cluster' and a cooling
> > device per 'cluster'. That means the platform has one clock line per 'cluster'.
> > The thermal management happens in a self-contained thermal zone (one
> > cooling device - one governor - one thermal zone).
> > 
> > In the case of HMP, other combinations are possible to be optimal.

But not sure how I missed the obvious, though I do remember thinking about this.

So the problem is that the cpu_cooling driver will get requests in parallel to
set different max frequencies and the last call will always win and may result
in undesired outcome.

Sorry about creating the confusion.

-- 
viresh

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

* RE: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal zone node
  2019-05-10 10:12         ` Viresh Kumar
@ 2019-05-11  4:04           ` Andy Tang
  0 siblings, 0 replies; 7+ messages in thread
From: Andy Tang @ 2019-05-11  4:04 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Daniel Lezcano, Shawn Guo, Leo Li, robh+dt, mark.rutland,
	linux-arm-kernel, devicetree, linux-kernel, linux-pm, rui.zhang,
	edubezval

Thanks Viresh for your explanation.

BR,
Andy
> -----Original Message-----
> From: Viresh Kumar <viresh.kumar@linaro.org>
> Sent: 2019年5月10日 18:12
> To: Andy Tang <andy.tang@nxp.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>; Shawn Guo
> <shawnguo@kernel.org>; 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 v6] arm64: dts: ls1088a: add one more thermal
> zone node
> 
> Caution: EXT Email
> 
> On 10-05-19, 08:47, Andy Tang wrote:
> > + Viresh for help.
> >
> > > -----Original Message-----
> > > From: Daniel Lezcano <daniel.lezcano@linaro.org>
> > > Sent: 2019年5月10日 15:17
> > > To: Andy Tang <andy.tang@nxp.com>; Shawn Guo
> <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 v6] arm64: dts: ls1088a: add one more
> > > thermal zone node
> > >
> > > Caution: EXT Email
> > >
> > > On 10/05/2019 05:40, Andy Tang wrote:
> > > >> -----Original Message-----
> > > >> From: Shawn Guo <shawnguo@kernel.org>
> > > >> Sent: 2019年5月10日 11:14
> > > >> To: Andy Tang <andy.tang@nxp.com>
> > > >> 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; daniel.lezcano@linaro.org;
> > > >> rui.zhang@intel.com; edubezval@gmail.com
> > > >> Subject: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more
> > > >> thermal zone node
> > > >>
> > > >> Caution: EXT Email
> > > >>
> > > >> On Tue, Apr 23, 2019 at 10:25:07AM +0800, 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>
> > > >>> ---
> > > >>> v6:
> > > >>>         - add cooling device map to cpu0-7 in platform node.
> > > > I like to explain a little. I think it makes sense that multiple
> > > > thermal zone
> > > map to same cooling device.
> > > > In this way, no matter which thermal zone raises a temp alarm, it
> > > > can call
> > > cooling device to chill out.
> > > > I also asked cpufreq maintainer about the cooling map issue, he
> > > > think it
> > > would be fine.
> 
> Yes, you asked me and I said it should be okay.
> 
> > > > I have tested and no issue found.
> > > >
> > > > Daniel, what's your thought?
> > >
> > > If there are multiple thermal zones, they will be managed by
> > > different instances of a thermal governor. Each instances will act
> > > on the shared cooling device and will collide in their decisions:
> > >
> > >  - If the sensors are closed, their behavior will be similar
> > > regarding the temperature. The governors may take the same decision
> > > for the cooling device. But in such case having just one thermal zone
> managed is enough.
> > >
> > >  - If the sensors are not closed, their behavior will be different
> > > regarding the temperature. The governors will take different
> > > decision regarding the cooling device (one will decrease the freq, other
> will increase the freq).
> > >
> > > As the thermal governors are not able to manage several thermal
> > > zones and there is one cooling device (the cpu cooling device), this
> > > setup won't work as expected IMO.
> > >
> > > The setup making sense is having a thermal zone per 'cluster' and a
> > > cooling device per 'cluster'. That means the platform has one clock line
> per 'cluster'.
> > > The thermal management happens in a self-contained thermal zone (one
> > > cooling device - one governor - one thermal zone).
> > >
> > > In the case of HMP, other combinations are possible to be optimal.
> 
> But not sure how I missed the obvious, though I do remember thinking about
> this.
> 
> So the problem is that the cpu_cooling driver will get requests in parallel to
> set different max frequencies and the last call will always win and may result
> in undesired outcome.
> 
> Sorry about creating the confusion.
> 
> --
> viresh

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

end of thread, other threads:[~2019-05-11  4:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-23  2:25 [PATCH v6] arm64: dts: ls1088a: add one more thermal zone node Yuantian Tang
2019-05-10  3:13 ` Shawn Guo
2019-05-10  3:40   ` [EXT] " Andy Tang
2019-05-10  7:16     ` Daniel Lezcano
2019-05-10  8:47       ` Andy Tang
2019-05-10 10:12         ` Viresh Kumar
2019-05-11  4:04           ` Andy Tang

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