[PATCH v6] arm64: dts: ls1088a: add one more thermal zone node

[PATCH v6] arm64: dts: ls1088a: add one more thermal zone node

[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

Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal zone node

[Shawn Guo]

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

RE: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal zone node

[Andy Tang]

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

Re: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal zone node

[Daniel Lezcano]

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

RE: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal zone node

[Andy Tang]

+ 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

Re: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal zone node

[Viresh Kumar]

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

RE: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal zone node

[Andy Tang]

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