lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <YIc2dJlhXtb7NWU2@google.com>
Date:   Mon, 26 Apr 2021 14:53:56 -0700
From:   Matthias Kaehlcke <mka@...omium.org>
To:     Rajeshwari <rkambl@...eaurora.org>
Cc:     amitk@...nel.org, thara.gopinath@...aro.org, agross@...nel.org,
        bjorn.andersson@...aro.org, rui.zhang@...el.com,
        daniel.lezcano@...aro.org, robh+dt@...nel.org,
        linux-pm@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        sanm@...eaurora.org, manafm@...eaurora.org
Subject: Re: [PATCH V2 3/3] ARM: dts: qcom: Add Thermal zone support in
 SC7280.

On Fri, Apr 23, 2021 at 11:52:57PM +0530, Rajeshwari wrote:

> Subject: ARM: dts: qcom: Add Thermal zone support in SC7280

s/ARM/arm64/

same for patch [2/2]

nit: s/Thermal/thermal/

>
> Added Thermal zone support, cooling maps and added hysteresis as zero as
> it is not needed as system is supposed to shut down when critical trip point
> is reached.

nit: s/Thermal/thermal/

I don't think it's necessary to explain the hysteresis values unless
they are really uncommon.

> Signed-off-by: Rajeshwari <rkambl@...eaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 844 +++++++++++++++++++++++++++++++++++
>  1 file changed, 844 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 275113c..80f4ecb 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -12,6 +12,7 @@
>  #include <dt-bindings/power/qcom-aoss-qmp.h>
>  #include <dt-bindings/power/qcom-rpmpd.h>
>  #include <dt-bindings/soc/qcom,rpmh-rsc.h>
> +#include <dt-bindings/thermal/thermal.h>
>  
>  / {
>  	interrupt-parent = <&intc>;
> @@ -1140,6 +1141,849 @@
>  		};
>  	};
>  
> +	thermal_zones: thermal-zones {
> +		cpu0-thermal {
> +			polling-delay-passive = <250>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens0 1>;
> +
> +			trips {
> +				cpu0_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "passive";
> +				};
> +
> +				cpu0_alert1: trip-point1 {
> +					temperature = <95000>;
> +					hysteresis = <0>;
> +					type = "passive";
> +				};
> +
> +				cpu0_crit: cpu_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};

In the review of v1 (https://patchwork.kernel.org/project/linux-arm-msm/patch/1619005442-16485-2-git-send-email-rkambl@codeaurora.org/)
I suggested that a hysteresis isn't needed for the critical trip
points, but this patch also also gets rid of the hysteresis values
for the passive and hot trip points, which isn't correct.

> +			};
> +
> +			cooling-maps {
> +				map0 {
> +					trip = <&cpu0_alert0>;
> +					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>;
> +				};
> +				map1 {
> +					trip = <&cpu0_alert1>;
> +					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>;
> +				};
> +			};
> +		};
>
> ...
>
> +		aoss0-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens0 0>;
> +
> +			trips {
> +				aoss0_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +
> +				aoss0_crit: aoss0_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		aoss1-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens1 0>;
> +
> +			trips {
> +				aoss1_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +
> +				aoss1_crit: aoss1_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		cpuss0-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens0 5>;
> +
> +			trips {
> +				cpuss0_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +				cpuss0_crit: cluster0_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		cpuss1-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens0 6>;
> +
> +			trips {
> +				cpuss1_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +				cpuss1_crit: cluster0_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		gpuss0-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens1 1>;
> +
> +			trips {
> +				gpuss0_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +
> +				gpuss0_crit: gpuss0_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		gpuss1-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens1 2>;
> +
> +			trips {
> +				gpuss1_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +
> +				gpuss1_crit: gpuss1_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		nspss0-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens1 3>;
> +
> +			trips {
> +				nspss0_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +
> +				nspss0_crit: nspss0_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		nspss1-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens1 4>;
> +
> +			trips {
> +				nspss1_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +
> +				nspss1_crit: nspss1_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		video-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens1 5>;
> +
> +			trips {
> +				video_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +
> +				video_crit: video_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		ddr-thermal {

uber-nit: I still think an alphabetical order would be preferable, it
makes it easy to locate the zones, whereas the indices of the TSENS
sensor are mostly irrelevant for the order. Anyway, AFAIK there are
no clear rules or conventions regarding this, so it's ok if yo
prefer to leave it as is.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ