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

[Andy Tang <andy.tang@nxp.com>]

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: 2019年3月8日 17:28
> 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: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
> 
> On 08/03/2019 03:07, Andy Tang wrote:
> >
> >
> >> -----Original Message-----
> >> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> >> Sent: 2019年3月7日 17:15
> >> 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: [PATCH v2] arm64: dts: ls1088a: add one more thermal
> >> zone node
> >>
> >>>>> PS: In order to keep consistency to the first thermal-zone node,
> >>>>> there will be "WARNING: line over 80 characters" warnings.
> >>>>>
> >>>>>  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..9f52bc9 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 {
> >>>>> +		ccu {
> >>>>
> >>>> Is this change really necessary?  What does 'ccu' stand for?
> >>> I think so. ccu stands for core cluster unit. cpu is too general.
> >>> On some platforms, there are more than one core clusters.
> >>> At least we should change it to "core cluster" if short form is not
> appropriate.
> >>
> >> If the sensor is a the cluster level, 'cluster' is enough. IMHO, no
> >> need to give a description of what contains the cluster, otherwise
> >> you will end up with a 'core-gpu-cluster-l2' name.
> > If cluster is specific to core, we can use cluster instead. But I don't think so.
> > Cluster may refer to "core cluster", "GPU cluster" etc.
> > So, I think "core-cluster" is ok.
> > If core was divided to several clusters, we can name it as "core-cluster1",
> "core-cluster2" etc.
> > If GPU was divided to several clusters we can name it as "gpu-cluster1",
> "gpu-cluster2" etc.
> 
> 
> Hi Andy,
> 
> I think there is a confusion around the 'cpu' term and 'cluster'.
> 
> ARM would like to see the 'cluster' word to disappear, so whenever possible we
> should avoid it.
> 
> From the hardware side, 'CPU' is usually used to describe the physical chip
> containing the cores+cache.
> 
> From the software side, 'CPU' is usually used to describe the logical process
> unit, aka a core or a hyper-thread.
> 
> As we are in the DT, so describing the hardware, the CPU refers to the group
> cores+caches.
> 
> From my POV, using 'cpu' for the group of cores and 'gpu' for the graphic
> sounds ok, and so far that is what is used for the other platforms.
> 
> If you change the name, that may give the feeling there is something special
> with those thermal zones.

Thanks Daniel for your detailed explanations.

But as you said 'CPU' is usually used to describe the physical chip.
So if we name it as CPU, it sounds like this temperature sensor is monitoring the whole chip.
That's not true in our case.

Take ls2088a for example:
In ls2088a SoC, there are 7 temperature sensors. Please note that they are all located in SoC.
The placement of the temperature sensors are showed below:

Sensor ID		placement
1			DDR controller 1
2			DDR controller 2
3			DDR controller 3
4			core cluster 1
5 			core cluster 2
6 			core cluster 3
7 			core cluster 4

Apparently using CPU or CPU-cluster is not appropriate. Core-cluster is better.

What do you think?

BR,
Andy
> 
> 
> 
> 
> --
> 
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.l
> inaro.org%2F&amp;data=02%7C01%7Candy.tang%40nxp.com%7C0619d2c87e5
> a412d88d108d6a3a8650e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C636876340979527503&amp;sdata=jXdIKhCIwPxmTlmKoVe4iHCpvpMxLVFZ
> UbLB39iTRZ8%3D&amp;reserved=0> Linaro.org │ Open source software for
> ARM SoCs
> 
> Follow Linaro:
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> facebook.com%2Fpages%2FLinaro&amp;data=02%7C01%7Candy.tang%40nxp.c
> om%7C0619d2c87e5a412d88d108d6a3a8650e%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C636876340979527503&amp;sdata=ynZrr4frMPXXwegQ
> wJ0kASxsK8QjhCfB7Ok2D5Z1yEc%3D&amp;reserved=0> Facebook |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitte
> r.com%2F%23!%2Flinaroorg&amp;data=02%7C01%7Candy.tang%40nxp.com%7
> C0619d2c87e5a412d88d108d6a3a8650e%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C636876340979527503&amp;sdata=qPQFMpnzhgYdS1IXbP0Fc
> wJoM9r5q0L1BX8JLTKWWo8%3D&amp;reserved=0> Twitter |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.l
> inaro.org%2Flinaro-blog%2F&amp;data=02%7C01%7Candy.tang%40nxp.com%7
> C0619d2c87e5a412d88d108d6a3a8650e%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C636876340979527503&amp;sdata=U5H0Wc2hwkHE0oKTA6H
> VbQ4QdEmBfZ%2FC%2FHl2it0bzNU%3D&amp;reserved=0> Blog