Re: [PATCH v2 4/4] arm64: dts: qcom: sdm845: mtp: Add vadc channels and thermal zones

[Matthias Kaehlcke <mka@chromium.org>]

On Thu, Sep 23, 2021 at 02:23:11PM -0700, Bjorn Andersson wrote:
> Downstream defines four ADC channels related to thermal sensors external
> to the PM8998 and two channels for internal voltage measurements.
> 
> Add these to the upstream SDM845 MTP, describe the thermal monitor
> channels and add thermal_zones for these.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> In addition to the iio channels exposed by v1, Daniel wanted thermal_zones...
> 
> Changes since v1:
> - Enable the pm8998_adc_tm and describe the ADC channels
> - Add thermal-zones for the new channels
> 
>  arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 128 ++++++++++++++++++++++++
>  1 file changed, 128 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> index 52dd7a858231..e3b40daef801 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -10,6 +10,8 @@
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>  #include "sdm845.dtsi"
> +#include "pm8998.dtsi"
> +#include "pmi8998.dtsi"
>  
>  / {
>  	model = "Qualcomm Technologies, Inc. SDM845 MTP";
> @@ -46,6 +48,68 @@ vreg_s4a_1p8: pm8998-smps4 {
>  
>  		vin-supply = <&vph_pwr>;
>  	};
> +
> +	thermal-zones {
> +		xo_thermal: xo-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&pm8998_adc_tm 1>;
> +
> +			trips {
> +				trip-point {
> +					temperature = <125000>;
> +					hysteresis = <10000>;
> +					type = "passive";
> +				};
> +			};
> +		};
> +
> +		msm_thermal: msm-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&pm8998_adc_tm 2>;
> +
> +			trips {
> +				trip-point {
> +					temperature = <125000>;
> +					hysteresis = <10000>;
> +					type = "passive";
> +				};
> +			};
> +		};
> +
> +		pa_thermal: pa-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&pm8998_adc_tm 3>;
> +
> +			trips {
> +				trip-point {
> +					temperature = <125000>;
> +					hysteresis = <10000>;
> +					type = "passive";
> +				};
> +			};
> +		};
> +
> +		quiet_thermal: quiet-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&pm8998_adc_tm 4>;
> +
> +			trips {
> +				trip-point {
> +					temperature = <125000>;
> +					hysteresis = <10000>;
> +					type = "passive";
> +				};
> +			};
> +		};
> +	};
>  };
>  
>  &adsp_pas {
> @@ -469,6 +533,70 @@ &mss_pil {
>  	firmware-name = "qcom/sdm845/mba.mbn", "qcom/sdm845/modem.mbn";
>  };
>  
> +&pm8998_adc {
> +	adc-chan@4c {
> +		reg = <ADC5_XO_THERM_100K_PU>;
> +		label = "xo_therm";
> +	};
> +
> +	adc-chan@4d {
> +		reg = <ADC5_AMUX_THM1_100K_PU>;
> +		label = "msm_therm";
> +	};
> +
> +	adc-chan@4f {
> +		reg = <ADC5_AMUX_THM3_100K_PU>;
> +		label = "pa_therm1";
> +	};
> +
> +	adc-chan@51 {
> +		reg = <ADC5_AMUX_THM5_100K_PU>;
> +		label = "quiet_therm";
> +	};
> +
> +	adc-chan@83 {
> +		reg = <ADC5_VPH_PWR>;
> +		label = "vph_pwr";
> +	};
> +
> +	adc-chan@85 {
> +		reg = <ADC5_VCOIN>;
> +		label = "vcoin";
> +	};
> +};
> +
> +&pm8998_adc_tm {
> +	status = "okay";
> +
> +	xo-thermistor@1 {
> +		reg = <1>;
> +		io-channels = <&pm8998_adc ADC5_XO_THERM_100K_PU>;
> +		qcom,ratiometric;
> +		qcom,hw-settle-time-us = <200>;
> +	};
> +
> +	msm-thermistor@2 {
> +		reg = <2>;
> +		io-channels = <&pm8998_adc ADC5_AMUX_THM1_100K_PU>;
> +		qcom,ratiometric;
> +		qcom,hw-settle-time-us = <200>;
> +	};
> +
> +	pa-thermistor@3 {
> +		reg = <3>;
> +		io-channels = <&pm8998_adc ADC5_AMUX_THM3_100K_PU>;
> +		qcom,ratiometric;
> +		qcom,hw-settle-time-us = <200>;
> +	};
> +
> +	quiet-thermistor@4 {
> +		reg = <4>;
> +		io-channels = <&pm8998_adc ADC5_AMUX_THM5_100K_PU>;
> +		qcom,ratiometric;
> +		qcom,hw-settle-time-us = <200>;
> +	};
> +};
> +

The example in the 'qcom,spmi-adc-tm5' binding specifies 'qcom,ratiometric'
and 'qcom,hw-settle-time-us' for both the ADC and the thermal monitor, so do
several board files (e.g. sm8250-mtp.dts and qrb5165-rb5.dts). This apparent
redundancy bothered me earlier, it's not really clear to me whether it's
needed/recommended or not. Do you happen to have any insights on this?