Re: [PATCH v5 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] dts: duplicate gpio-reserved-ranges entry for GPIO 65 in tlmm node
- [Low] dts: subnodes of pmk8550_vadc are not sorted alphanumerically by unit address
--

commit b2ae087bb3abb0aa89d56fa01c7818e7b495008d
Author: Aleksandrs Vinarskis <alex@vinarskis.com>

arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC

This commit describes the embedded controller, its interrupt and required
thermal zones. It also adds the EC's reset GPIO to the reserved range to
prevent device operation from triggering an unrecoverable state.

> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
> index f48e6bc350063..6d99dc47eb163 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts

[ ... ]

> +&pmk8550_vadc {
> +	sys_therm0: channel@14c {
> +		reg = <ADC5_GEN3_AMUX3_GPIO_100K_PU(1)>;
> +		qcom,hw-settle-time = <200>;
> +		qcom,ratiometric;
> +		label = "lpddr5_therm";
> +	};
> +
> +	sys_therm1: channel@14d {
> +		reg = <ADC5_GEN3_AMUX4_GPIO_100K_PU(1)>;
> +		qcom,hw-settle-time = <200>;
> +		qcom,ratiometric;
> +		label = "usb_charger_left_therm";
> +	};
> +
> +	sys_therm2: channel@144 {
> +		reg = <ADC5_GEN3_AMUX1_THM_100K_PU(1)>;

[Severity: Low]
Are these subnodes ordered correctly? The newly added channel@... subnodes
under &pmk8550_vadc are sorted sequentially by their labels (sys_therm0,
sys_therm1, sys_therm2, etc.) rather than by their unit addresses.

As a result, the unit addresses appear out of order (@14c, @14d, @144).
Standard Devicetree style guidelines strictly require that child nodes be
sorted alphanumerically by their unit address.

[ ... ]

>  &tlmm {
>  	gpio-reserved-ranges = <44 4>,  /* SPI11 (TPM) */
> +			       <65 1>,  /* EC Reset, accessible but yields system unusable */
>  			       <76 4>,  /* SPI19 (TZ Protected) */
>  			       <65 1>, /* EC reset */
>  			       <238 1>; /* UFS Reset */

[Severity: Low]
Is the duplicate <65 1> entry here intentional? The patch adds a <65 1> entry
for the EC Reset pin to the gpio-reserved-ranges array in the &tlmm node,
but an identical <65 1> entry for the EC reset is already present
immediately after the <76 4> entry.

This creates a duplicate, overlapping reserved range in the Devicetree
configuration.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011-dell-xps-9345-ec-v5-0-2ab1258b84c1@vinarskis.com?part=3