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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Medium] Framebuffer broken by mdss overriding and incorrect splash-region node name.
- [Medium] SD card detect GPIO has inverted polarity in the Devicetree.
- [Medium] The aw99706 backlight node uses the property 'enable-gpio' instead of 'enable-gpios'.
- [Medium] Missing 'vdd-s4-supply' for populated smps4 regulator in regulators-3.
- [Low] DWC3 port@2 mapping is invalid and unsupported by the role switch framework.
- [Low] Invalid 'data-lanes' configuration for CPHY mode in mdss_dsi endpoints.
--

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

This commit adds the initial devicetree for the Lenovo Legion Y700 Gen4
tablet, which is based on the sm8750-qrd platform. It supports basic
functionality including USB, framebuffer, buttons, and battery charging.

> 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..6e4f8e9015fee
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
[ ... ]
> +	reserved-memory {
> +		/*
> +		 * Use this to tell the BL to initialize the display related
> +		 * resources, so we can use efifb. Please 's/-/_/'
> +		 */
> +		splash-region@fc800000 {

[Severity: Medium]
Does the hyphen in this node name prevent the ABL bootloader from matching
the splash region memory node? Should this be renamed to
splash_region@fc800000?

[ ... ]
> +	regulators-3 {
> +		compatible = "qcom,pm8550ve-rpmh-regulators";
> +
> +		vdd-l1-supply = <&vreg_s1d_0p97>;
> +		vdd-l2-supply = <&vreg_s3g_1p8>;
> +		vdd-l3-supply = <&vreg_s7i_1p2>;
> +		vdd-s1-supply = <&vph_pwr>;
> +		vdd-s3-supply = <&vph_pwr>;
> +
> +		qcom,pmic-id = "g";

[Severity: Medium]
Since vreg_s4g_0p75 is defined further down in regulators-3, is a
vdd-s4-supply mapping missing here alongside vdd-s1-supply and
vdd-s3-supply?

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

[Severity: Medium]
Should this property be named enable-gpios instead of enable-gpio?

[ ... ]
> +&mdss {
> +	status = "okay";
> +};

[Severity: Medium]
The commit message states "- Framebuffer (Require mdss and dispcc to be
disabled)". Does enabling the mdss node here override and break the
simple-framebuffer fallback intended by the splash_region setup?

[ ... ]
> +&mdss_dsi0_out {
> +	remote-endpoint = <&panel_in_0>;
> +	data-lanes = <0 1 2 3>;
> +};
> +
> +&mdss_dsi0_phy {
> +	vdds-supply = <&vreg_l3i_0p88>;
> +	phy-type = <PHY_TYPE_CPHY>;

[Severity: Low]
Is it correct to specify 4 data lanes for mdss_dsi0_out when the PHY type is
set to PHY_TYPE_CPHY, which natively supports up to 3 trios? Should
data-lanes be updated to <0 1 2>?

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

[Severity: Medium]
Looking at sdc2_card_det_n, gpio55 is configured with bias-pull-up. Shouldn't
cd-gpios use GPIO_ACTIVE_LOW instead of GPIO_ACTIVE_HIGH to correctly handle
the physical pull-down upon SD card insertion?

[ ... ]
> +&usb {
> +	dr_mode = "otg";
> +	maximum-speed = "super-speed-plus-gen2x1";
> +
> +	status = "okay";
> +
> +	ports {
> +		/*
> +		 * USB2.0 is shared for two ports, set pinctrl eusb_mux_default
> +		 * to select the repeter. Plugging out one of them will reset
> +		 * the DWC3, which causes both of them to be disconnected.
> +		 */
> +		port@0 {
> +			reg = <0>;
> +
> +			usb_dwc3_hs: endpoint {
> +				remote-endpoint = <&pmic_glink_hs_in>;
> +			};
> +		};
> +
> +		port@2 {
> +			reg = <2>;
> +
> +			usb_dwc3_hs2: endpoint {
> +				remote-endpoint = <&pmic_glink_hs_c2_in>;
> +			};
> +		};
> +	};
> +};

[Severity: Low]
Is port@2 mapping valid for the dwc3 node? The role switch framework does not
typically support port 2, which could cause issues with Type-C connector
multiplexing. Should port@2 be removed?

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