Re: [PATCH 3/3] arm64: dts: qcom: Add Lenovo IdeaPad Slim 5x 14Q8X9

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 9/6/26 2:22 PM, Jens Glathe via B4 Relay wrote:
> From: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
> 
> Add a common DTSI plus LCD and OLED board files for the Lenovo
> IdeaPad Slim 5 14Q8X9 (machine type 83HL) on X1P42100. In some
> regions this SKU is sold as "IdeaPad Slim 5x"; the "x" is the
> Qualcomm suffix in Lenovo's current IdeaPad naming (i = Intel,
> a = AMD, x = Qualcomm).

[...]

> +	hdmi-connector {
> +		compatible = "hdmi-connector";
> +		type = "a";
> +
> +		port {
> +			hdmi_con: endpoint {
> +				remote-endpoint = <&hdmi_bridge_tmds_out>;
> +			};
> +		};
> +	};
> +
> +	gpio-keys {

nit: 'g' < 'h'

[...]

> +	sound {
> +		compatible = "qcom,x1e80100-sndcard";
> +		model = "X1E80100-LENOVO-Ideapad-5";
> +		audio-routing = "SpkrLeft IN", "WSA WSA_SPK1 OUT",
> +				"SpkrRight IN", "WSA WSA_SPK2 OUT",
> +				"IN1_HPHL", "HPHL_OUT",
> +				"IN2_HPHR", "HPHR_OUT",
> +				"AMIC2", "MIC BIAS2",
> +				"VA DMIC0", "MIC BIAS1",
> +				"VA DMIC1", "MIC BIAS1",
> +				"VA DMIC0", "VA MIC BIAS1",
> +				"VA DMIC1", "VA MIC BIAS1",

https://lore.kernel.org/linux-arm-msm/20241203-x1e80100-va-mic-bias-v1-2-0dfd4d9b492c@linaro.org/

[...]

> +		vreg_l4b_1p8: ldo4 {
> +			regulator-name = "vreg_l4b_1p8";
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +
> +			regulator-state-mem {
> +				regulator-on-in-suspend;
> +			};

Why is this necessary?

[...]

> +&i2c1 {
> +	clock-frequency = <400000>;
> +
> +	status = "okay";
> +
> +	/* 3d PS883x */

Any reason not to wire it up now?

[...]

> +&iris {
> +	firmware-name = "qcom/x1p42100/LENOVO/83HL/qcvss8380.mbn";
> +	status = "okay";

nit: \n before status, please

[...]

> +&spi10 {
> +	status = "disabled";
> +
> +	/* Unknown device */
> +};

Either enable it, or remove it, I think

> +
> +&swr0 {
> +	status = "okay";
> +
> +	pinctrl-0 = <&wsa_swr_active>, <&spkr_01_sd_n_active>;
> +	pinctrl-names = "default";

'status' should go last

[...]

> +&tlmm {
> +	gpio-reserved-ranges = <34 2>, /* Unused */
> +		<44 4>,
> +		<72 2>; /* Secure EC I2C connection (?) */

Please align the '<'s

> +
> +	eusb3_reset_n: eusb3-reset-n-state {
> +		pins = "gpio6";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +		output-low;

You don't need the output- properties on the TLMM pins

[...]

> +&usb_1_ss2 {
> +	dr_mode = "host";
> +	maximum-speed = "high-speed";
> +	phys = <&usb_1_ss2_hsphy>;
> +	phy-names = "usb2-phy";
> +	qcom,select-utmi-as-pipe-clk;
> +	status = "okay";
> +	/delete-property/ port@1;

nit: status last, please

Konrad