* [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-04 3:21 ` Yuantian Tang
0 siblings, 0 replies; 30+ messages in thread
From: Yuantian Tang @ 2019-03-04 3:21 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>
---
v2:
- Add more information about sensors to description
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 {
polling-delay-passive = <1000>;
polling-delay = <5000>;
thermal-sensors = <&tmu 0>;
trips {
- cpu_alert: cpu-alert {
+ ccu_alert: ccu-alert {
temperature = <85000>;
hysteresis = <2000>;
type = "passive";
};
- cpu_crit: cpu-crit {
+ ccu_crit: ccu-crit {
temperature = <95000>;
hysteresis = <2000>;
type = "critical";
@@ -150,7 +150,42 @@
cooling-maps {
map0 {
- trip = <&cpu_alert>;
+ 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>;
+ };
+ };
+ };
+
+ plt {
+ 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>,
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-04 3:21 ` Yuantian Tang
0 siblings, 0 replies; 30+ messages in thread
From: Yuantian Tang @ 2019-03-04 3:21 UTC (permalink / raw)
To: shawnguo
Cc: mark.rutland, devicetree, Yuantian Tang, linux-pm,
daniel.lezcano, linux-kernel, leoyang.li, edubezval, robh+dt,
rui.zhang, linux-arm-kernel
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>
---
v2:
- Add more information about sensors to description
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 {
polling-delay-passive = <1000>;
polling-delay = <5000>;
thermal-sensors = <&tmu 0>;
trips {
- cpu_alert: cpu-alert {
+ ccu_alert: ccu-alert {
temperature = <85000>;
hysteresis = <2000>;
type = "passive";
};
- cpu_crit: cpu-crit {
+ ccu_crit: ccu-crit {
temperature = <95000>;
hysteresis = <2000>;
type = "critical";
@@ -150,7 +150,42 @@
cooling-maps {
map0 {
- trip = <&cpu_alert>;
+ 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>;
+ };
+ };
+ };
+
+ plt {
+ 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>,
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-04 3:21 ` Yuantian Tang
0 siblings, 0 replies; 30+ messages in thread
From: Yuantian Tang @ 2019-03-04 3:21 UTC (permalink / raw)
To: shawnguo
Cc: mark.rutland, devicetree, Yuantian Tang, linux-pm,
daniel.lezcano, linux-kernel, leoyang.li, edubezval, robh+dt,
rui.zhang, linux-arm-kernel
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>
---
v2:
- Add more information about sensors to description
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 {
polling-delay-passive = <1000>;
polling-delay = <5000>;
thermal-sensors = <&tmu 0>;
trips {
- cpu_alert: cpu-alert {
+ ccu_alert: ccu-alert {
temperature = <85000>;
hysteresis = <2000>;
type = "passive";
};
- cpu_crit: cpu-crit {
+ ccu_crit: ccu-crit {
temperature = <95000>;
hysteresis = <2000>;
type = "critical";
@@ -150,7 +150,42 @@
cooling-maps {
map0 {
- trip = <&cpu_alert>;
+ 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>;
+ };
+ };
+ };
+
+ plt {
+ 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>,
--
1.7.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
2019-03-04 3:21 ` Yuantian Tang
(?)
@ 2019-03-04 6:21 ` Shawn Guo
-1 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2019-03-04 6:21 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 Mon, Mar 04, 2019 at 11:21:11AM +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>
> ---
> v2:
> - Add more information about sensors to description
> 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?
> polling-delay-passive = <1000>;
> polling-delay = <5000>;
> thermal-sensors = <&tmu 0>;
>
> trips {
> - cpu_alert: cpu-alert {
> + ccu_alert: ccu-alert {
> temperature = <85000>;
> hysteresis = <2000>;
> type = "passive";
> };
>
> - cpu_crit: cpu-crit {
> + ccu_crit: ccu-crit {
> temperature = <95000>;
> hysteresis = <2000>;
> type = "critical";
> @@ -150,7 +150,42 @@
>
> cooling-maps {
> map0 {
> - trip = <&cpu_alert>;
> + 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>;
> + };
> + };
> + };
> +
> + plt {
What about 'platform-thermal' for node name, platform-alert and
platform-crit for trip nodes below?
Shawn
> + 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>,
> --
> 1.7.1
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-04 6:21 ` Shawn Guo
0 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2019-03-04 6:21 UTC (permalink / raw)
To: Yuantian Tang
Cc: mark.rutland, devicetree, linux-pm, daniel.lezcano, linux-kernel,
leoyang.li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
On Mon, Mar 04, 2019 at 11:21:11AM +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>
> ---
> v2:
> - Add more information about sensors to description
> 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?
> polling-delay-passive = <1000>;
> polling-delay = <5000>;
> thermal-sensors = <&tmu 0>;
>
> trips {
> - cpu_alert: cpu-alert {
> + ccu_alert: ccu-alert {
> temperature = <85000>;
> hysteresis = <2000>;
> type = "passive";
> };
>
> - cpu_crit: cpu-crit {
> + ccu_crit: ccu-crit {
> temperature = <95000>;
> hysteresis = <2000>;
> type = "critical";
> @@ -150,7 +150,42 @@
>
> cooling-maps {
> map0 {
> - trip = <&cpu_alert>;
> + 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>;
> + };
> + };
> + };
> +
> + plt {
What about 'platform-thermal' for node name, platform-alert and
platform-crit for trip nodes below?
Shawn
> + 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>,
> --
> 1.7.1
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-04 6:21 ` Shawn Guo
0 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2019-03-04 6:21 UTC (permalink / raw)
To: Yuantian Tang
Cc: mark.rutland, devicetree, linux-pm, daniel.lezcano, linux-kernel,
leoyang.li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
On Mon, Mar 04, 2019 at 11:21:11AM +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>
> ---
> v2:
> - Add more information about sensors to description
> 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?
> polling-delay-passive = <1000>;
> polling-delay = <5000>;
> thermal-sensors = <&tmu 0>;
>
> trips {
> - cpu_alert: cpu-alert {
> + ccu_alert: ccu-alert {
> temperature = <85000>;
> hysteresis = <2000>;
> type = "passive";
> };
>
> - cpu_crit: cpu-crit {
> + ccu_crit: ccu-crit {
> temperature = <95000>;
> hysteresis = <2000>;
> type = "critical";
> @@ -150,7 +150,42 @@
>
> cooling-maps {
> map0 {
> - trip = <&cpu_alert>;
> + 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>;
> + };
> + };
> + };
> +
> + plt {
What about 'platform-thermal' for node name, platform-alert and
platform-crit for trip nodes below?
Shawn
> + 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>,
> --
> 1.7.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
2019-03-04 6:21 ` Shawn Guo
@ 2019-03-04 6:46 ` Andy Tang
-1 siblings, 0 replies; 30+ messages in thread
From: Andy Tang @ 2019-03-04 6:46 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
> -----Original Message-----
> From: Shawn Guo <shawnguo@kernel.org>
> Sent: 2019年3月4日 14:21
> 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: Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
>
> On Mon, Mar 04, 2019 at 11:21:11AM +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>
> > ---
> > v2:
> > - Add more information about sensors to description
> > 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.
>
> > polling-delay-passive = <1000>;
> > polling-delay = <5000>;
> > thermal-sensors = <&tmu 0>;
> >
> > trips {
> > - cpu_alert: cpu-alert {
> > + ccu_alert: ccu-alert {
> > temperature = <85000>;
> > hysteresis = <2000>;
> > type = "passive";
> > };
> >
> > - cpu_crit: cpu-crit {
> > + ccu_crit: ccu-crit {
> > temperature = <95000>;
> > hysteresis = <2000>;
> > type = "critical";
> > @@ -150,7 +150,42 @@
> >
> > cooling-maps {
> > map0 {
> > - trip = <&cpu_alert>;
> > + 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>;
> > + };
> > + };
> > + };
> > +
> > + plt {
>
> What about 'platform-thermal' for node name, platform-alert and platform-crit
> for trip nodes below?
OK, will use long name form.
BR,
Andy
>
> Shawn
>
> > + 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>,
> > --
> > 1.7.1
> >
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-04 6:46 ` Andy Tang
0 siblings, 0 replies; 30+ messages in thread
From: Andy Tang @ 2019-03-04 6:46 UTC (permalink / raw)
To: Shawn Guo
Cc: mark.rutland, devicetree, linux-pm, daniel.lezcano, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
> -----Original Message-----
> From: Shawn Guo <shawnguo@kernel.org>
> Sent: 2019年3月4日 14:21
> 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: Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
>
> On Mon, Mar 04, 2019 at 11:21:11AM +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>
> > ---
> > v2:
> > - Add more information about sensors to description
> > 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.
>
> > polling-delay-passive = <1000>;
> > polling-delay = <5000>;
> > thermal-sensors = <&tmu 0>;
> >
> > trips {
> > - cpu_alert: cpu-alert {
> > + ccu_alert: ccu-alert {
> > temperature = <85000>;
> > hysteresis = <2000>;
> > type = "passive";
> > };
> >
> > - cpu_crit: cpu-crit {
> > + ccu_crit: ccu-crit {
> > temperature = <95000>;
> > hysteresis = <2000>;
> > type = "critical";
> > @@ -150,7 +150,42 @@
> >
> > cooling-maps {
> > map0 {
> > - trip = <&cpu_alert>;
> > + 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>;
> > + };
> > + };
> > + };
> > +
> > + plt {
>
> What about 'platform-thermal' for node name, platform-alert and platform-crit
> for trip nodes below?
OK, will use long name form.
BR,
Andy
>
> Shawn
>
> > + 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>,
> > --
> > 1.7.1
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
2019-03-04 6:46 ` Andy Tang
(?)
@ 2019-03-04 6:53 ` Shawn Guo
-1 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2019-03-04 6:53 UTC (permalink / raw)
To: Andy Tang
Cc: mark.rutland, devicetree, linux-pm, daniel.lezcano, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
On Mon, Mar 04, 2019 at 06:46:32AM +0000, Andy Tang wrote:
>
>
> > -----Original Message-----
> > From: Shawn Guo <shawnguo@kernel.org>
> > Sent: 2019年3月4日 14:21
> > 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: Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
> >
> > On Mon, Mar 04, 2019 at 11:21:11AM +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>
> > > ---
> > > v2:
> > > - Add more information about sensors to description
> > > 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.
Yes, please give it a more descriptive name.
Shawn
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-04 6:53 ` Shawn Guo
0 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2019-03-04 6:53 UTC (permalink / raw)
To: Andy Tang
Cc: mark.rutland, devicetree, linux-pm, daniel.lezcano, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
On Mon, Mar 04, 2019 at 06:46:32AM +0000, Andy Tang wrote:
>
>
> > -----Original Message-----
> > From: Shawn Guo <shawnguo@kernel.org>
> > Sent: 2019年3月4日 14:21
> > 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: Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
> >
> > On Mon, Mar 04, 2019 at 11:21:11AM +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>
> > > ---
> > > v2:
> > > - Add more information about sensors to description
> > > 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.
Yes, please give it a more descriptive name.
Shawn
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-04 6:53 ` Shawn Guo
0 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2019-03-04 6:53 UTC (permalink / raw)
To: Andy Tang
Cc: mark.rutland, devicetree, linux-pm, daniel.lezcano, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
On Mon, Mar 04, 2019 at 06:46:32AM +0000, Andy Tang wrote:
>
>
> > -----Original Message-----
> > From: Shawn Guo <shawnguo@kernel.org>
> > Sent: 2019年3月4日 14:21
> > 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: Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
> >
> > On Mon, Mar 04, 2019 at 11:21:11AM +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>
> > > ---
> > > v2:
> > > - Add more information about sensors to description
> > > 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.
Yes, please give it a more descriptive name.
Shawn
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
2019-03-04 6:46 ` Andy Tang
@ 2019-03-07 9:14 ` Daniel Lezcano
-1 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2019-03-07 9:14 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 04/03/2019 07:46, Andy Tang wrote:
>
>
>> -----Original Message-----
>> From: Shawn Guo <shawnguo@kernel.org>
>> Sent: 2019年3月4日 14:21
>> 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: Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
>>
>> On Mon, Mar 04, 2019 at 11:21:11AM +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>
>>> ---
>>> v2:
>>> - Add more information about sensors to description
>>> 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.
>>
>>> polling-delay-passive = <1000>;
>>> polling-delay = <5000>;
>>> thermal-sensors = <&tmu 0>;
>>>
>>> trips {
>>> - cpu_alert: cpu-alert {
>>> + ccu_alert: ccu-alert {
>>> temperature = <85000>;
>>> hysteresis = <2000>;
>>> type = "passive";
>>> };
>>>
>>> - cpu_crit: cpu-crit {
>>> + ccu_crit: ccu-crit {
>>> temperature = <95000>;
>>> hysteresis = <2000>;
>>> type = "critical";
>>> @@ -150,7 +150,42 @@
>>>
>>> cooling-maps {
>>> map0 {
>>> - trip = <&cpu_alert>;
>>> + 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>;
>>> + };
>>> + };
>>> + };
>>> +
>>> + plt {
>>
>> What about 'platform-thermal' for node name, platform-alert and platform-crit
>> for trip nodes below?
> OK, will use long name form.
>
> BR,
> Andy
>>
>> Shawn
>>
>>> + 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>,
>>> --
>>> 1.7.1
>>>
--
<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] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-07 9:14 ` Daniel Lezcano
0 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2019-03-07 9:14 UTC (permalink / raw)
To: Andy Tang, Shawn Guo
Cc: mark.rutland, devicetree, linux-pm, linux-kernel, Leo Li,
edubezval, robh+dt, rui.zhang, linux-arm-kernel
On 04/03/2019 07:46, Andy Tang wrote:
>
>
>> -----Original Message-----
>> From: Shawn Guo <shawnguo@kernel.org>
>> Sent: 2019年3月4日 14:21
>> 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: Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
>>
>> On Mon, Mar 04, 2019 at 11:21:11AM +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>
>>> ---
>>> v2:
>>> - Add more information about sensors to description
>>> 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.
>>
>>> polling-delay-passive = <1000>;
>>> polling-delay = <5000>;
>>> thermal-sensors = <&tmu 0>;
>>>
>>> trips {
>>> - cpu_alert: cpu-alert {
>>> + ccu_alert: ccu-alert {
>>> temperature = <85000>;
>>> hysteresis = <2000>;
>>> type = "passive";
>>> };
>>>
>>> - cpu_crit: cpu-crit {
>>> + ccu_crit: ccu-crit {
>>> temperature = <95000>;
>>> hysteresis = <2000>;
>>> type = "critical";
>>> @@ -150,7 +150,42 @@
>>>
>>> cooling-maps {
>>> map0 {
>>> - trip = <&cpu_alert>;
>>> + 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>;
>>> + };
>>> + };
>>> + };
>>> +
>>> + plt {
>>
>> What about 'platform-thermal' for node name, platform-alert and platform-crit
>> for trip nodes below?
> OK, will use long name form.
>
> BR,
> Andy
>>
>> Shawn
>>
>>> + 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>,
>>> --
>>> 1.7.1
>>>
--
<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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
2019-03-07 9:14 ` Daniel Lezcano
@ 2019-03-08 2:07 ` Andy Tang
-1 siblings, 0 replies; 30+ messages in thread
From: Andy Tang @ 2019-03-08 2:07 UTC (permalink / raw)
To: Daniel Lezcano, Shawn Guo
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年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.
BR,
Andy
>
> >>
> >>> polling-delay-passive = <1000>;
> >>> polling-delay = <5000>;
> >>> thermal-sensors = <&tmu 0>;
> >>>
> >>> trips {
> >>> - cpu_alert: cpu-alert {
> >>> + ccu_alert: ccu-alert {
> >>> temperature = <85000>;
> >>> hysteresis = <2000>;
> >>> type = "passive";
> >>> };
> >>>
> >>> - cpu_crit: cpu-crit {
> >>> + ccu_crit: ccu-crit {
> >>> temperature = <95000>;
> >>> hysteresis = <2000>;
> >>> type = "critical";
> >>> @@ -150,7 +150,42 @@
> >>>
> >>> cooling-maps {
> >>> map0 {
> >>> - trip = <&cpu_alert>;
> >>> + 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>;
> >>> + };
> >>> + };
> >>> + };
> >>> +
> >>> + plt {
> >>
> >> What about 'platform-thermal' for node name, platform-alert and
> >> platform-crit for trip nodes below?
> > OK, will use long name form.
> >
> > BR,
> > Andy
> >>
> >> Shawn
> >>
> >>> + 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>,
> >>> --
> >>> 1.7.1
> >>>
>
>
> --
>
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.l
> inaro.org%2F&data=02%7C01%7Candy.tang%40nxp.com%7C59738105e23
> 2436e0c8808d6a2dd583b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C636875468882120357&sdata=jxYXYhc3Tc407a10WgOcbyIX7OCl1eScoJ
> deutX%2Btnk%3D&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&data=02%7C01%7Candy.tang%40nxp.c
> om%7C59738105e232436e0c8808d6a2dd583b%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C636875468882120357&sdata=vikEm7YrV%2F8RvQ
> %2FGsXh3O361HZR3RqDcdvq4uTgB58w%3D&reserved=0> Facebook |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitte
> r.com%2F%23!%2Flinaroorg&data=02%7C01%7Candy.tang%40nxp.com%7
> C59738105e232436e0c8808d6a2dd583b%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C636875468882120357&sdata=dBdCgFfsO6uFS%2FLl5lNH
> aYB2mOredm9Wdtf1aWT8Si8%3D&reserved=0> Twitter |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.l
> inaro.org%2Flinaro-blog%2F&data=02%7C01%7Candy.tang%40nxp.com%7
> C59738105e232436e0c8808d6a2dd583b%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C636875468882120357&sdata=FCbdkbwdUPVyCjPdxcU5U
> gUfFVZzMExdaOpsZD3SCmQ%3D&reserved=0> Blog
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-08 2:07 ` Andy Tang
0 siblings, 0 replies; 30+ messages in thread
From: Andy Tang @ 2019-03-08 2:07 UTC (permalink / raw)
To: Daniel Lezcano, Shawn Guo
Cc: mark.rutland, devicetree, linux-pm, linux-kernel, Leo Li,
edubezval, robh+dt, rui.zhang, linux-arm-kernel
> -----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.
BR,
Andy
>
> >>
> >>> polling-delay-passive = <1000>;
> >>> polling-delay = <5000>;
> >>> thermal-sensors = <&tmu 0>;
> >>>
> >>> trips {
> >>> - cpu_alert: cpu-alert {
> >>> + ccu_alert: ccu-alert {
> >>> temperature = <85000>;
> >>> hysteresis = <2000>;
> >>> type = "passive";
> >>> };
> >>>
> >>> - cpu_crit: cpu-crit {
> >>> + ccu_crit: ccu-crit {
> >>> temperature = <95000>;
> >>> hysteresis = <2000>;
> >>> type = "critical";
> >>> @@ -150,7 +150,42 @@
> >>>
> >>> cooling-maps {
> >>> map0 {
> >>> - trip = <&cpu_alert>;
> >>> + 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>;
> >>> + };
> >>> + };
> >>> + };
> >>> +
> >>> + plt {
> >>
> >> What about 'platform-thermal' for node name, platform-alert and
> >> platform-crit for trip nodes below?
> > OK, will use long name form.
> >
> > BR,
> > Andy
> >>
> >> Shawn
> >>
> >>> + 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>,
> >>> --
> >>> 1.7.1
> >>>
>
>
> --
>
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.l
> inaro.org%2F&data=02%7C01%7Candy.tang%40nxp.com%7C59738105e23
> 2436e0c8808d6a2dd583b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C636875468882120357&sdata=jxYXYhc3Tc407a10WgOcbyIX7OCl1eScoJ
> deutX%2Btnk%3D&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&data=02%7C01%7Candy.tang%40nxp.c
> om%7C59738105e232436e0c8808d6a2dd583b%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C636875468882120357&sdata=vikEm7YrV%2F8RvQ
> %2FGsXh3O361HZR3RqDcdvq4uTgB58w%3D&reserved=0> Facebook |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitte
> r.com%2F%23!%2Flinaroorg&data=02%7C01%7Candy.tang%40nxp.com%7
> C59738105e232436e0c8808d6a2dd583b%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C636875468882120357&sdata=dBdCgFfsO6uFS%2FLl5lNH
> aYB2mOredm9Wdtf1aWT8Si8%3D&reserved=0> Twitter |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.l
> inaro.org%2Flinaro-blog%2F&data=02%7C01%7Candy.tang%40nxp.com%7
> C59738105e232436e0c8808d6a2dd583b%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C636875468882120357&sdata=FCbdkbwdUPVyCjPdxcU5U
> gUfFVZzMExdaOpsZD3SCmQ%3D&reserved=0> Blog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
2019-03-08 2:07 ` Andy Tang
@ 2019-03-08 9:28 ` Daniel Lezcano
-1 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2019-03-08 9:28 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 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.
--
<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] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-08 9:28 ` Daniel Lezcano
0 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2019-03-08 9:28 UTC (permalink / raw)
To: Andy Tang, Shawn Guo
Cc: mark.rutland, devicetree, linux-pm, linux-kernel, Leo Li,
edubezval, robh+dt, rui.zhang, linux-arm-kernel
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.
--
<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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
2019-03-08 9:28 ` Daniel Lezcano
@ 2019-03-08 9:57 ` Andy Tang
-1 siblings, 0 replies; 30+ messages in thread
From: Andy Tang @ 2019-03-08 9:57 UTC (permalink / raw)
To: Daniel Lezcano, Shawn Guo
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年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&data=02%7C01%7Candy.tang%40nxp.com%7C0619d2c87e5
> a412d88d108d6a3a8650e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C636876340979527503&sdata=jXdIKhCIwPxmTlmKoVe4iHCpvpMxLVFZ
> UbLB39iTRZ8%3D&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&data=02%7C01%7Candy.tang%40nxp.c
> om%7C0619d2c87e5a412d88d108d6a3a8650e%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C636876340979527503&sdata=ynZrr4frMPXXwegQ
> wJ0kASxsK8QjhCfB7Ok2D5Z1yEc%3D&reserved=0> Facebook |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitte
> r.com%2F%23!%2Flinaroorg&data=02%7C01%7Candy.tang%40nxp.com%7
> C0619d2c87e5a412d88d108d6a3a8650e%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C636876340979527503&sdata=qPQFMpnzhgYdS1IXbP0Fc
> wJoM9r5q0L1BX8JLTKWWo8%3D&reserved=0> Twitter |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.l
> inaro.org%2Flinaro-blog%2F&data=02%7C01%7Candy.tang%40nxp.com%7
> C0619d2c87e5a412d88d108d6a3a8650e%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C636876340979527503&sdata=U5H0Wc2hwkHE0oKTA6H
> VbQ4QdEmBfZ%2FC%2FHl2it0bzNU%3D&reserved=0> Blog
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-08 9:57 ` Andy Tang
0 siblings, 0 replies; 30+ messages in thread
From: Andy Tang @ 2019-03-08 9:57 UTC (permalink / raw)
To: Daniel Lezcano, Shawn Guo
Cc: mark.rutland, devicetree, linux-pm, linux-kernel, Leo Li,
edubezval, robh+dt, rui.zhang, linux-arm-kernel
> -----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&data=02%7C01%7Candy.tang%40nxp.com%7C0619d2c87e5
> a412d88d108d6a3a8650e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C636876340979527503&sdata=jXdIKhCIwPxmTlmKoVe4iHCpvpMxLVFZ
> UbLB39iTRZ8%3D&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&data=02%7C01%7Candy.tang%40nxp.c
> om%7C0619d2c87e5a412d88d108d6a3a8650e%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C636876340979527503&sdata=ynZrr4frMPXXwegQ
> wJ0kASxsK8QjhCfB7Ok2D5Z1yEc%3D&reserved=0> Facebook |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitte
> r.com%2F%23!%2Flinaroorg&data=02%7C01%7Candy.tang%40nxp.com%7
> C0619d2c87e5a412d88d108d6a3a8650e%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C636876340979527503&sdata=qPQFMpnzhgYdS1IXbP0Fc
> wJoM9r5q0L1BX8JLTKWWo8%3D&reserved=0> Twitter |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.l
> inaro.org%2Flinaro-blog%2F&data=02%7C01%7Candy.tang%40nxp.com%7
> C0619d2c87e5a412d88d108d6a3a8650e%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C636876340979527503&sdata=U5H0Wc2hwkHE0oKTA6H
> VbQ4QdEmBfZ%2FC%2FHl2it0bzNU%3D&reserved=0> Blog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
2019-03-08 9:57 ` Andy Tang
(?)
@ 2019-03-20 8:18 ` Shawn Guo
-1 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2019-03-20 8:18 UTC (permalink / raw)
To: Andy Tang
Cc: Daniel Lezcano, mark.rutland, devicetree, linux-pm, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
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 ...
> 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.
Shawn
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-20 8:18 ` Shawn Guo
0 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2019-03-20 8:18 UTC (permalink / raw)
To: Andy Tang
Cc: Daniel Lezcano, mark.rutland, devicetree, linux-pm, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
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 ...
> 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.
Shawn
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-20 8:18 ` Shawn Guo
0 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2019-03-20 8:18 UTC (permalink / raw)
To: Andy Tang
Cc: mark.rutland, devicetree, linux-pm, Daniel Lezcano, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
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 ...
> 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.
Shawn
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
2019-03-20 8:18 ` Shawn Guo
(?)
@ 2019-03-20 8:44 ` Andy Tang
-1 siblings, 0 replies; 30+ messages in thread
From: Andy Tang @ 2019-03-20 8:44 UTC (permalink / raw)
To: Shawn Guo
Cc: Daniel Lezcano, mark.rutland, devicetree, linux-pm, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-20 8:44 ` Andy Tang
0 siblings, 0 replies; 30+ messages in thread
From: Andy Tang @ 2019-03-20 8:44 UTC (permalink / raw)
To: Shawn Guo
Cc: Daniel Lezcano, mark.rutland, devicetree, linux-pm, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-20 8:44 ` Andy Tang
0 siblings, 0 replies; 30+ messages in thread
From: Andy Tang @ 2019-03-20 8:44 UTC (permalink / raw)
To: Shawn Guo
Cc: mark.rutland, devicetree, linux-pm, Daniel Lezcano, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
2019-03-20 8:44 ` Andy Tang
(?)
@ 2019-03-20 14:49 ` Shawn Guo
-1 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2019-03-20 14:49 UTC (permalink / raw)
To: Andy Tang
Cc: Daniel Lezcano, mark.rutland, devicetree, linux-pm, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
On Wed, Mar 20, 2019 at 08:44:18AM +0000, Andy Tang wrote:
> > > 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.
Hmm, yes, that would be even more confusing. What about cpu-thermal-1,
cpu-thermal-2 ...?
Shawn
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-20 14:49 ` Shawn Guo
0 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2019-03-20 14:49 UTC (permalink / raw)
To: Andy Tang
Cc: Daniel Lezcano, mark.rutland, devicetree, linux-pm, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
On Wed, Mar 20, 2019 at 08:44:18AM +0000, Andy Tang wrote:
> > > 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.
Hmm, yes, that would be even more confusing. What about cpu-thermal-1,
cpu-thermal-2 ...?
Shawn
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-20 14:49 ` Shawn Guo
0 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2019-03-20 14:49 UTC (permalink / raw)
To: Andy Tang
Cc: mark.rutland, devicetree, linux-pm, Daniel Lezcano, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
On Wed, Mar 20, 2019 at 08:44:18AM +0000, Andy Tang wrote:
> > > 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.
Hmm, yes, that would be even more confusing. What about cpu-thermal-1,
cpu-thermal-2 ...?
Shawn
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
2019-03-20 14:49 ` Shawn Guo
@ 2019-03-21 1:05 ` Andy Tang
-1 siblings, 0 replies; 30+ messages in thread
From: Andy Tang @ 2019-03-21 1:05 UTC (permalink / raw)
To: Shawn Guo
Cc: Daniel Lezcano, mark.rutland, devicetree, linux-pm, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
> -----Original Message-----
> From: Shawn Guo <shawnguo@kernel.org>
> Sent: 2019年3月20日 22:49
> 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 Wed, Mar 20, 2019 at 08:44:18AM +0000, Andy Tang wrote:
> > > > 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.
>
> Hmm, yes, that would be even more confusing. What about cpu-thermal-1,
> cpu-thermal-2 ...?
Cpu-thermal-x can't change anything better than cpuX. It can't reflect the concept of CLUSTER.
I prefer to use core-cluster. It is a wild accepted term in ARM ecosystem.
BR,
Andy
>
> Shawn
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
@ 2019-03-21 1:05 ` Andy Tang
0 siblings, 0 replies; 30+ messages in thread
From: Andy Tang @ 2019-03-21 1:05 UTC (permalink / raw)
To: Shawn Guo
Cc: mark.rutland, devicetree, linux-pm, Daniel Lezcano, linux-kernel,
Leo Li, edubezval, robh+dt, rui.zhang, linux-arm-kernel
> -----Original Message-----
> From: Shawn Guo <shawnguo@kernel.org>
> Sent: 2019年3月20日 22:49
> 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 Wed, Mar 20, 2019 at 08:44:18AM +0000, Andy Tang wrote:
> > > > 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.
>
> Hmm, yes, that would be even more confusing. What about cpu-thermal-1,
> cpu-thermal-2 ...?
Cpu-thermal-x can't change anything better than cpuX. It can't reflect the concept of CLUSTER.
I prefer to use core-cluster. It is a wild accepted term in ARM ecosystem.
BR,
Andy
>
> Shawn
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2019-03-21 1:05 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-04 3:21 [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node Yuantian Tang
2019-03-04 3:21 ` Yuantian Tang
2019-03-04 3:21 ` Yuantian Tang
2019-03-04 6:21 ` Shawn Guo
2019-03-04 6:21 ` Shawn Guo
2019-03-04 6:21 ` Shawn Guo
2019-03-04 6:46 ` Andy Tang
2019-03-04 6:46 ` Andy Tang
2019-03-04 6:53 ` Shawn Guo
2019-03-04 6:53 ` Shawn Guo
2019-03-04 6:53 ` Shawn Guo
2019-03-07 9:14 ` Daniel Lezcano
2019-03-07 9:14 ` Daniel Lezcano
2019-03-08 2:07 ` Andy Tang
2019-03-08 2:07 ` Andy Tang
2019-03-08 9:28 ` Daniel Lezcano
2019-03-08 9:28 ` Daniel Lezcano
2019-03-08 9:57 ` Andy Tang
2019-03-08 9:57 ` Andy Tang
2019-03-20 8:18 ` Shawn Guo
2019-03-20 8:18 ` Shawn Guo
2019-03-20 8:18 ` Shawn Guo
2019-03-20 8:44 ` Andy Tang
2019-03-20 8:44 ` Andy Tang
2019-03-20 8:44 ` Andy Tang
2019-03-20 14:49 ` Shawn Guo
2019-03-20 14:49 ` Shawn Guo
2019-03-20 14:49 ` Shawn Guo
2019-03-21 1:05 ` Andy Tang
2019-03-21 1:05 ` Andy Tang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.