Re: [PATCH v4 11/12] arm64: dts: qcom: sm6350-lena: Add PM6150L thermals

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] arm64: dts: qcom: sm6350-lena: handle used in Author and
  Signed-off-by
- [Medium] arm64: dts: qcom: sm6350-lena: inconsistent RFC_CAM_THERM
  pinctrl and ADC setup
- [Low] arm64: dts: qcom: sm6350-lena: thermal-sensor-msm ordered by
  label not name
--

commit e42ff4c503ef1827973e7803bdff8b39d54aacf7
Author: erebion <erebion@erebion.eu>

> arm64: dts: qcom: sm6350-lena: Add PM6150L thermals
>
> Configure the thermals for the PA_THERM1, MSM_THERM, PA_THERM0,
> RFC_CAM_THERM, CAM_FLASH_THERM and QUIET_THERM thermistors connected to
> PM6150L.
>
> Due to hardware constraints we can only register 4 zones with
> pm6150l_adc_tm, the other 2 we can register via generic-adc-thermal.
>
> The trip points can really only be considered as placeholders, more
> configuration with cooling etc. can be added later.
>
> Signed-off-by: erebion <erebion@erebion.eu>

[Severity: High]
Per Documentation/process/submitting-patches.rst, the Author header and
Signed-off-by trailer must use a real human name rather than an account
handle ("erebion").

> diff --git a/arch/arm64/boot/dts/qcom/sm6350-sony-xperia-lena-pdx213.dts b/arch/arm64/boot/dts/qcom/sm6350-sony-xperia-lena-pdx213.dts
> index 79097ae409bab..d7a5df04a296c 100644
> --- a/arch/arm64/boot/dts/qcom/sm6350-sony-xperia-lena-pdx213.dts
> +++ b/arch/arm64/boot/dts/qcom/sm6350-sony-xperia-lena-pdx213.dts
> @@ -61,6 +61,13 @@ key-volume-down {
>  		};
>  	};
>  
> +	msm_therm_sensor: thermal-sensor-msm {
> +		compatible = "generic-adc-thermal";
> +		#thermal-sensor-cells = <0>;
> +		io-channels = <&pm6150l_adc ADC5_AMUX_THM2_100K_PU>;
> +		io-channel-names = "sensor-channel";
> +	};
> +
>  	touch_en_vreg: regulator-touch-en {

[Severity: Low]
This isn't a bug, but devicetree coding style orders nodes without unit
addresses alphabetically by node name rather than label, so
thermal-sensor-msm should be placed after reserved-memory and before
thermal-zones.

[ ... ]

> @@ -392,6 +467,83 @@ &mpss {
>  	status = "okay";
>  };
>  
> +&pm6150l_adc {
> +	pinctrl-0 = <&pm6150l_adc_default>;
> +	pinctrl-names = "default";
> +
> +	channel@4d {
> +		reg = <ADC5_AMUX_THM1_100K_PU>;
> +		label = "pa_therm1";
> +		qcom,hw-settle-time = <200>;
> +		qcom,pre-scaling = <1 1>;
> +		qcom,ratiometric;
> +	};
> +
> +	channel@4e {
> +		reg = <ADC5_AMUX_THM2_100K_PU>;
> +		label = "msm_therm";
> +		qcom,hw-settle-time = <200>;
> +		qcom,pre-scaling = <1 1>;
> +		qcom,ratiometric;
> +	};
> +
> +	channel@4f {
> +		reg = <ADC5_AMUX_THM3_100K_PU>;
> +		label = "pa_therm0";
> +		qcom,hw-settle-time = <200>;
> +		qcom,pre-scaling = <1 1>;
> +		qcom,ratiometric;
> +	};
> +
> +	channel@54 {
> +		reg = <ADC5_GPIO3_100K_PU>;
> +		label = "rear_cam_flash_therm";
> +		qcom,hw-settle-time = <200>;
> +		qcom,pre-scaling = <1 1>;
> +		qcom,ratiometric;
> +	};
> +
> +	channel@55 {
> +		reg = <ADC5_GPIO4_100K_PU>;
> +		label = "quiet_therm";
> +		qcom,hw-settle-time = <200>;
> +		qcom,pre-scaling = <1 1>;
> +		qcom,ratiometric;
> +	};
> +};

[ ... ]

> @@ -405,6 +557,14 @@ led-0 {
>  	};
>  };
>  
> +&pm6150l_gpios {
> +	pm6150l_adc_default: adc-default-state {
> +		pins = "gpio6", "gpio7", "gpio10";
> +		function = PMIC_GPIO_FUNC_NORMAL;
> +		bias-high-impedance;
> +	};
> +};

[Severity: Medium]
Is the sixth thermistor (for rear camera) missing from pm6150l_adc, or
should gpio6 be dropped from pm6150l_adc_default?

The commit message describes six thermistors with two registered via
generic-adc-thermal, and pm6150l_adc_default configures three pins (gpio6,
gpio7, and gpio10) into high-impedance mode.  However, pm6150l_adc only
defines five channels (with only two gpio-backed inputs, channel@54 and
channel@55, omitting channel@53) and only one generic-adc-thermal sensor
(msm_therm_sensor) is registered.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-pdx213-upstream-3-v4-0-4b01c50a821c@erebion.eu?part=11