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

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] TUSB320 Type-C controller interrupt is incorrectly configured with a rising edge trigger, causing missed events.
- [Medium] Incorrect touchscreen reset GPIO polarity prevents touchscreen initialization.
- [Medium] WLED node is not enabled, contradicting the commit message claims.
--

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

arm64: dts: qcom: Add LeEco Le2

Add a devicetree for the LeEco Le2 smartphone, which is based
on the Eldarion/MSM8976 SoC. This adds 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
> --- /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: Medium]
Is this GPIO polarity correct for the Synaptics RMI4 controller? The hardware
typically uses an active-low reset. 

If configured as active-high, the driver will drive the line low during normal
operation, which might hold the touchscreen controller in a perpetual reset
state and prevent initialization.

> +
> +		vdd-supply = <&pm8950_l17>;
> +		vio-supply = <&pm8950_l6>;

[ ... ]

> +&blsp2_i2c2 {
> +	status = "okay";
> +
> +	haptics: haptics@5a {
> +		compatible = "ti,drv2604";
> +		reg = <0x5a>;
> +		enable-gpios = <&tlmm 109 GPIO_ACTIVE_HIGH>;
> +		mode = <DRV260X_LRA_MODE>;
> +		library-sel = <DRV260X_LIB_LRA>;
> +	};
> +
> +	tiusb320: typec@47 {
> +		compatible = "ti,tusb320";
> +		reg = <0x47>;
> +		interrupts-extended = <&tlmm 21 IRQ_TYPE_EDGE_RISING>;

[Severity: High]
Will this rising edge trigger work for the TUSB320 hardware? 

The TUSB320 uses an open-drain, active-low INT_N signal that is pulled low
when an event occurs and remains low until serviced. 

If configured as a rising edge, the interrupt handler might not run when the
signal is asserted. This could prevent the interrupt status from being read,
which would mean the chip never releases the line, deadlocking the interrupt
mechanism and missing attach/detach events.

> +
> +		pinctrl-0 = <&tusb320_int_active>;
> +		pinctrl-names = "default";
> +	};
> +};

[ ... ]

> +&pm8950_resin {
> +	linux,code = <KEY_VOLUMEDOWN>;
> +
> +	status = "okay";
> +};
> +
> +&pmi8950_wled {
> +	qcom,current-limit-microamp = <20000>;
> +	qcom,enabled-strings = <0 1>;
> +	qcom,ovp-millivolt = <29500>;
> +};

[Severity: Medium]
Does this node need a status = "okay"; property added here? 

The commit message mentions WLED support is included in this initial
submission, but the pmi8950_wled node is usually defined as disabled in the
base pmi8950.dtsi file. Without an explicit okay status, this hardware
functionality might remain disabled.

> +
> +&rpm_requests {

[ ... ]

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