From: Andy Tang <andy.tang@nxp.com>
To: Shawn Guo <shawnguo@kernel.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>,
"mark.rutland@arm.com" <mark.rutland@arm.com>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Leo Li <leoyang.li@nxp.com>,
"edubezval@gmail.com" <edubezval@gmail.com>,
"robh+dt@kernel.org" <robh+dt@kernel.org>,
"rui.zhang@intel.com" <rui.zhang@intel.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: RE: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
Date: Wed, 20 Mar 2019 08:44:18 +0000 [thread overview]
Message-ID: <VI1PR04MB433351C20A9863EEF21A2504F3410@VI1PR04MB4333.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20190320081853.GE4980@dragon>
Hi Shawn,
> -----Original Message-----
> From: Shawn Guo <shawnguo@kernel.org>
> Sent: 2019年3月20日 16:19
> To: Andy Tang <andy.tang@nxp.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>; mark.rutland@arm.com;
> devicetree@vger.kernel.org; linux-pm@vger.kernel.org;
> linux-kernel@vger.kernel.org; Leo Li <leoyang.li@nxp.com>;
> edubezval@gmail.com; robh+dt@kernel.org; rui.zhang@intel.com;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
>
> On Fri, Mar 08, 2019 at 09:57:09AM +0000, Andy Tang wrote:
> >
> >
> > > -----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.
>
> Here is how I would understand Daniel's comments:
>
> CPU = cores + caches
> physical chip = SoC = CPU + GPU + peripherals ...
Agree.
>
> > 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.
>
> So using CPU is appropriate for me, less confusing, more consistent with other
> platforms.
What about core cluster? We can't name it cpu0, cpu1 etc I think.
BR,
Andy
>
> Shawn
next prev parent reply other threads:[~2019-03-20 8:44 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-04 3:21 [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node Yuantian Tang
2019-03-04 6:21 ` Shawn Guo
2019-03-04 6:46 ` Andy Tang
2019-03-04 6:53 ` Shawn Guo
2019-03-07 9:14 ` Daniel Lezcano
2019-03-08 2:07 ` Andy Tang
2019-03-08 9:28 ` Daniel Lezcano
2019-03-08 9:57 ` Andy Tang
2019-03-20 8:18 ` Shawn Guo
2019-03-20 8:44 ` Andy Tang [this message]
2019-03-20 14:49 ` Shawn Guo
2019-03-21 1:05 ` Andy Tang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=VI1PR04MB433351C20A9863EEF21A2504F3410@VI1PR04MB4333.eurprd04.prod.outlook.com \
--to=andy.tang@nxp.com \
--cc=daniel.lezcano@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=edubezval@gmail.com \
--cc=leoyang.li@nxp.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=robh+dt@kernel.org \
--cc=rui.zhang@intel.com \
--cc=shawnguo@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).