Re: [PATCH v4 5/7] arm64: dts: qcom: Add LeEco Le2

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The `vdd_l9_l10_l13_l14_l15_l18` property in `qcom,rpm-pm8950-regulators` is missing the mandatory `-supply` suffix.
- [High] The `&otg` node configures an `extcon` but lacks `status = "okay";`, meaning USB will remain disabled.
- [Low] The Synaptics RMI4 touchscreen `reset-gpios` is incorrectly specified as active-high (`GPIO_ACTIVE_HIGH`), contradicting the hardware's active-low requirement.
--

--- Patch [5]: [PATCH v4 5/7] arm64: dts: qcom: Add LeEco Le2 ---
commit c3c3413469ad8891e9f97013b16ab8831b5677fe
Author: Adam Skladowski <[email protected]>

arm64: dts: qcom: Add LeEco Le2

This patch adds a devicetree for the LeEco Le2 smartphone based on the
Eldarion/MSM8976 SoC. It includes initial support for I2C peripherals,
regulators, WLED, remoteprocs, and GPU.

> diff --git a/arch/arm64/boot/dts/qcom/msm8976-leeco-s2.dts b/arch/arm64/boot/dts/qcom/msm8976-leeco-s2.dts
> new file mode 100644
> index 0000000000000..e64335162c34b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8976-leeco-s2.dts
[ ... ]
> +&blsp1_i2c4 {
> +	status = "okay";
> +
> +	touchscreen@4b {
> +		compatible = "syna,rmi4-i2c";
> +		reg = <0x4b>;
> +		interrupts-extended = <&tlmm 65 IRQ_TYPE_EDGE_FALLING>;
> +
> +		pinctrl-0 = <&touch_default_state>;
> +		pinctrl-names = "default";
> +
> +		reset-gpios = <&tlmm 64 GPIO_ACTIVE_HIGH>;

[Severity: Low]
Is the reset-gpios polarity correct here? 

Looking further down in the patch at touch_default_state, a bias-pull-up is
defined for this pin. This suggests the inactive state is physically high,
which would mean the reset should be active-low. While the syna,rmi4-i2c
driver may ignore this property currently, if driver support is added later,
the GPIO framework might drive the pin low when de-asserting it, keeping
the device permanently in reset.

[ ... ]
> +&mdss_dsi0_phy {
> +	vddio-supply = <&pm8950_l6>;
> +};
> +
> +&otg {
> +	extcon = <&tiusb320>;
> +};

[Severity: High]
Should this node include a status = "okay" property? 

Without it, the OTG port will remain in the disabled state defined by the
base SoC inclusion, which prevents the USB controller driver from probing
and leaves USB/OTG functionality broken.

[ ... ]
> +&rpm_requests {
> +	regulators {
> +		compatible = "qcom,rpm-pm8950-regulators";
> +
> +		vdd_s1-supply = <&vph_pwr>;
> +		vdd_s2-supply = <&vph_pwr>;
> +		vdd_s3-supply = <&vph_pwr>;
> +		vdd_s4-supply = <&vph_pwr>;
> +		vdd_s6-supply = <&vph_pwr>;
> +		vdd_l1_l19-supply = <&pm8950_s3>;
> +		vdd_l2_l23-supply = <&pm8950_s3>;
> +		vdd_l3-supply = <&pm8950_s3>;
> +		vdd_l5_l6_l7_l16-supply = <&pm8950_s4>;
> +		vdd_l8_l11_l12_l17_l22-supply = <&vph_pwr>;
> +		vdd_l9_l10_l13_l14_l15_l18 = <&vph_pwr>;

[Severity: High]
Is this property missing the mandatory -supply suffix?

The regulator framework relies on the -supply suffix to establish parent-child
relationships. Without it, the framework will fail to resolve the parent
supply, leaving regulators L9, L10, L13, L14, L15, and L18 unmapped.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5