Re: [PATCH 2/2] arm64: dts: qcom: sm8750: Add Lenovo Legion Y700 Gen4

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] SD card detection GPIO polarity is inverted.
- [Medium] Unresolved `splash-region` node name breaks bootloader framebuffer setup.
- [Medium] The SBU mux (`fsa4480_hs_sbu_mux`) for the short edge Type-C connector is improperly mapped to `port@1` instead of `port@2`.
- [Low] Invalid scalar property `enable-gpio` used for the Awinic backlight driver instead of `enable-gpios`.
--

--- Patch [2]: [PATCH 2/2] arm64: dts: qcom: sm8750: Add Lenovo Legion Y700 Gen4 ---
commit fd69339b25380307f4c2239ad2b73d115e6d80d8
Author: Pengyu Luo <mitltlatltl@gmail.com>

arm64: dts: qcom: sm8750: Add Lenovo Legion Y700 Gen4

Add an initial devicetree for the Lenovo Legion Y700 Gen4, which is
based on sm8750-qrd.

Link: https://lore.kernel.org/dri-devel/20260709142846.12463-1-mitltlatltl@gmail.com
Link: https://github.com/GUF296/linux/commits/codex/tb321fu-full-remediation-20260715
Link: https://github.com/linux-msm/hexagonrpc

> diff --git a/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
> new file mode 100644
> index 0000000000000..602e7322b921b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts

[ ... ]

> +		/* Short edge */
> +		connector@1 {
> +			compatible = "usb-c-connector";
> +			reg = <1>;
> +
> +			power-role = "dual";
> +			data-role = "dual";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;

[ ... ]

> +				port@1 {
> +					reg = <1>;
> +
> +					pmic_glink_hs_sbu: endpoint {
> +						remote-endpoint = <&fsa4480_hs_sbu_mux>;
> +					};
> +				};
> +			};
> +		};
> +	};

[Severity: Medium]
Is this SBU mux mapped to the correct port? According to the usb-connector
schema, port 1 is reserved for SuperSpeed, while port 2 is reserved for SBU.
Will mapping this to port 1 cause the Type-C subsystem and pmic_glink_altmode
driver to misidentify the SBU mux as a SuperSpeed switch and break routing?

> +	reserved-memory {
> +		/*
> +		 * Use this to tell the BL to initialize the display related
> +		 * resources, so we can use efifb. Please 's/-/_/'
> +		 */
> +		splash-region {
> +			reg = <0 0xfc800000 0 0x2b00000>;
> +			no-map;
> +		};
> +	};

[Severity: Medium]
Will this node name break out-of-the-box framebuffer initialization?
Since the comment indicates the bootloader expects an underscore, could we
provide the correctly formatted splash_region here directly rather than
relying on a manual string replacement during compilation or deployment?

[ ... ]

> +	backlight: backlight@76 {
> +		compatible = "awinic,aw99706";
> +		reg = <0x76>;
> +
> +		enable-gpio = <&tlmm 88 GPIO_ACTIVE_HIGH>;

[Severity: Low]
Should this be enable-gpios instead of enable-gpio? While gpiolib
transparently handles the singular fallback at runtime, using the singular
form will likely cause dtbs_check to fail since the awinic,aw99706 binding
requires the plural property name.

[ ... ]

> +&sdhc_2 {
> +	cd-gpios = <&tlmm 55 GPIO_ACTIVE_HIGH>;

[Severity: Medium]
Is the polarity for cd-gpios correct here? The sdc2_card_det_n pinctrl
node further down defines a bias-pull-up, meaning a floating (empty) slot
will pull high.

With GPIO_ACTIVE_HIGH logic, will this cause the driver to incorrectly
interpret an empty slot as occupied?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719093226.197740-1-mitltlatltl@gmail.com?part=2