Re: [PATCH v2] arm64: dts: qcom: sm8650-ayaneo-pocket-s2: add display nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The Goodix GT911 touchscreen node specifies `reset-gpios` but omits the required `irq-gpios` property, silently disabling the driver's hardware reset and power management logic.
- [High] The device tree relies on hardware components (SGM3804 regulator and SY7758 backlight) that currently have no supporting driver or device tree binding in the kernel.
- [Medium] The `mdp_vsync_active` and `mdp_vsync_suspend` pinctrl states are defined but never referenced in the display or MDSS nodes, preventing the hardware VSYNC signal from being routed.
- [Low] The backlight node is named `sy7758@2e` instead of a generic name like `backlight@2e`.
--

commit a7774645b5e43100828950906f34539f8e7aadef
Author: KancyJoe <kancy2333@outlook.com>

arm64: dts: qcom: sm8650-ayaneo-pocket-s2: add display nodes

This commit adds device tree nodes for the dual DSI panel, the SGM3804
regulator, and the SY7758 backlight controller. It also enables the GPU
and the touch controller to provide full display support.

> diff --git a/arch/arm64/boot/dts/qcom/sm8650-ayaneo-pocket-s2.dts b/arch/arm64/boot/dts/qcom/sm8650-ayaneo-pocket-s2.dts
> index 0dc994f4e48d9..af890e76813b7 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650-ayaneo-pocket-s2.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8650-ayaneo-pocket-s2.dts
[ ... ]
> @@ -1016,6 +1040,30 @@ wcd_usbss_sbu_mux: endpoint {
>  	};
>  };
>  
> +&i2c4 {
> +	clock-frequency = <400000>;
> +
> +	status = "okay";
> +
> +	touchscreen@5d {
> +		compatible = "goodix,gt911";
> +		reg = <0x5d>;
> +
> +		interrupt-parent = <&tlmm>;
> +		interrupts = <162 IRQ_TYPE_EDGE_FALLING>;
> +
> +		reset-gpios = <&tlmm 161 GPIO_ACTIVE_HIGH>;

[Severity: High]
Is the irq-gpios property missing from this node?

The Goodix driver requires retrieving the interrupt GPIO via the irq-gpios
property to execute its hardware reset sequence, as the INT pin state
determines the I2C address.

When irq-gpios is omitted, the driver's irq_pin_access_method falls back
to IRQ_PIN_ACCESS_NONE. This completely bypasses the goodix_reset()
function during probe.

Additionally, goodix_suspend() and goodix_resume() will return early without
issuing the deep sleep command, which can cause power drain during system
suspend.

> +		VDDIO-supply = <&vreg_l14b_3p2>;
> +		AVDD28-supply = <&vreg_l14b_3p2>;
> +
> +		touchscreen-size-x = <1440>;
> +		touchscreen-size-y = <2560>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&ts_reset_default>, <&ts_irq_default>;
> +	};
> +};
> +
[ ... ]
> @@ -1053,6 +1101,49 @@ redriver_ss_in: endpoint {
>  	};
>  };
>  
> +&i2c9 {
> +	status = "okay";
> +
> +	/* Screen power */
> +	regulator@3e {
> +		compatible = "sgmicro,sgm3804";

[Severity: High]
Do these components currently have supporting drivers and device tree bindings
upstream?

Because there are no existing drivers for sgmicro,sgm3804 or silergy,sy7758
in the kernel, they will not probe.

When the panel driver attempts to acquire its essential supplies (vsp-supply,
vsn-supply, and backlight), it will hit a permanent -EPROBE_DEFER, preventing
the entire display subsystem from initializing.

> +		reg = <0x3e>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&sgm3804_default>;
> +
> +		vin-supply = <&vph_pwr>;
> +
[ ... ]
> +	/* Backlight */
> +	sy7758_backlight: sy7758@2e {

[Severity: Low]
Could this node be named using a generic class like backlight@2e?

The Devicetree Specification recommends using generic node names that reflect
the function of the device rather than the specific model or IC name.
Non-generic names can trigger validation errors during dt_binding_check.

> +		compatible = "silergy,sy7758";
> +		reg = <0x2e>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&sy7758_default>;
> +
> +		vddio-supply = <&sy7758_vdd33_reg>;
> +		enable-gpios = <&tlmm 164 GPIO_ACTIVE_HIGH>;
> +	};
> +};
[ ... ]
> @@ -1411,6 +1603,20 @@ fan_int: fan-int-state {
>  		bias-pull-up;
>  	};
>  
> +	mdp_vsync_active: mdp-vsync-active-state {
> +		pins = "gpio86";
> +		function = "mdp_vsync";
> +		drive-strength = <2>;
> +		bias-pull-down;
> +	};
> +
> +	mdp_vsync_suspend: mdp-vsync-suspend-state {
> +		pins = "gpio86";
> +		function = "mdp_vsync";
> +		drive-strength = <2>;
> +		bias-pull-down;
> +	};
> +

[Severity: Medium]
Are these mdp_vsync pinctrl states supposed to be referenced in the panel or
MDSS nodes?

They appear to be defined here but are never assigned to the pinctrl-0 or
pinctrl-1 properties of the display components. Command-mode panels typically
rely on this Tearing Effect (TE) signal to synchronize frame updates.

Without routing the signal, the display controller may fall back to software
synchronization or timeout waiting for TE interrupts, which can result in
frame drops or tearing.

>  	upd720201_avdd33: upd720201-avdd33-state {
>  		pins = "gpio123";
>  		function = "gpio";

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-topic-sm8650-ayaneo-pocket-s2-display-dt-v2-1-cdd4b70e5a16@linaro.org?part=1