Re: [PATCH V3 3/3] arm64: dts: qcom: SC7280: Add thermal zone support

From: Matthias Kaehlcke <mka@chromium.org>
To: Rajeshwari <rkambl@codeaurora.org>
Cc: amitk@kernel.org, thara.gopinath@linaro.org, agross@kernel.org,
	bjorn.andersson@linaro.org, rui.zhang@intel.com,
	daniel.lezcano@linaro.org, robh+dt@kernel.org,
	linux-pm@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	sanm@codeaurora.org, manafm@codeaurora.org
Subject: Re: [PATCH V3 3/3] arm64: dts: qcom: SC7280: Add thermal zone support
Date: Mon, 3 May 2021 10:56:09 -0700	[thread overview]
Message-ID: <YJA5ORG5eR5f5nXc@google.com> (raw)
In-Reply-To: <1619778592-8112-4-git-send-email-rkambl@codeaurora.org>

On Fri, Apr 30, 2021 at 03:59:52PM +0530, Rajeshwari wrote:
> Adding thermal zone, cooling maps support and changing hysteresis value for critical trip point in SC7280.
> Hysteresis is not needed for critical trip point, when it reaches critical threshold system
> will go for shutdown.

This patch doesn't change any hysteresis values, it adds them. As
commented on v2, the sentence about the hysteresis doesn't add any
value IMO, just remove it, or at least explain in a reply why you
think it is valuable.

> 
> Signed-off-by: Rajeshwari <rkambl@codeaurora.org>
> ---

patches with v > 1 should include a changelog

>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 852 +++++++++++++++++++++++++++++++++++
>  1 file changed, 852 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 275113c..d722081 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>
>  
>  / {
>
> ...
>
> +	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 = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu0_alert1: trip-point1 {
> +					temperature = <95000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu0_crit: cpu_crit {

The node name should use dashes as separator, not underscores, i.e. it
should be 'cpu-crit'. Applicable to all critical trip points.