linux-kernel - Re: [PATCH v2 4/8] arm64: dts: qcom: Add support for Surface Pro 11

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ae91cad5-5a0b-4c83-9eb0-6e7baf6a33d0@oss.qualcomm.com>
Date: Mon, 1 Dec 2025 16:35:24 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Dale Whinham <daleyo@...il.com>, Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>
Cc: Jérôme de Bretagne <jerome.debretagne@...il.com>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/8] arm64: dts: qcom: Add support for Surface Pro 11

On 12/1/25 2:14 AM, Dale Whinham wrote:
> Add device trees for the Qualcomm X1E and X1P-based Microsoft Surface
> Pro 11 machines (codenamed 'Denali').
> 
> This device is very similar to the Surface Laptop 7 ('Romulus').
> 
> Use a similar strategy to x1-asus-zenbook-a14.dtsi so that we can create
> x1e and x1p-specific flavors of the device tree without too much code
> duplication.

[...]

> +		pinctrl-0 = <&hall_int_n_default>;
> +		pinctrl-names = "default";
> +
> +		switch-lid {
> +			gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> +			linux,input-type = <EV_SW>;
> +			linux,code = <SW_LID>;

I.. don't think this device has a lid - what triggers this GPIO?

[...]

> +	/*
> +	 * TODO: These two regulators are actually part of the removable M.2
> +	 * card and not the CRD mainboard. Need to describe this differently.
> +	 * Functionally it works correctly, because all we need to do is to
> +	 * turn on the actual 3.3V supply above.

There's not a M.2 card, the WLAN chip is soldered on board

https://www.ifixit.com/Guide/Microsoft+Surface+Pro+11+Chip+ID/174016#s370945


> +	sound {
> +		compatible = "qcom,x1e80100-sndcard";
> +		model = "X1E80100-Microsoft-Surface-Pro-11";
> +		audio-routing = "SpkrLeft IN", "WSA WSA_SPK1 OUT",
> +				"SpkrRight IN", "WSA WSA_SPK2 OUT",
> +				"VA DMIC0", "vdd-micb",
> +				"VA DMIC1", "vdd-micb";
> +
> +		wsa-dai-link {
> +			link-name = "WSA Playback";
> +
> +			cpu {
> +				sound-dai = <&q6apmbedai WSA_CODEC_DMA_RX_0>;
> +			};
> +
> +			codec {

'co'dec < 'cp'u

please flip the order of the two

[...]

> +&i2c0 {
> +	clock-frequency = <400000>;
> +
> +	status = "disabled";

Is there anything connected to that controller?

If so, let's keep it enabled so that it's accessible through i2c-tools
It'd be even better if you could document (in a comment, like in romulus.dtsi)
what and at what address that is

[...]

> +&lpass_tlmm {
> +	spkr_01_sd_n_active: spkr-01-sd-n-active-state {
> +		pins = "gpio12";
> +		function = "gpio";
> +		drive-strength = <16>;
> +		bias-disable;
> +		output-low;

Please drop output-low from both definitions, the output state is
controlled manually by the WSA driver. Although from the diff below
it looks like spkr_23_sd_n is unused and you only have 2 speakers

[...]

> +&tlmm {
> +	gpio-reserved-ranges = <44 4>, /* SPI (TPM) */
> +						   <238 1>; /* UFS Reset */

Please ensure your tab width is set to 8

[...]
> +	cam_indicator_en: cam-indicator-en-state {
> +		pins = "gpio225";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +
> +	wcn_sw_en: wcn-sw-en-state {
> +			pins = "gpio214";
> +			function = "gpio";
> +			drive-strength = <2>;
> +			bias-disable;
> +	};
> +
> +	wcn_wlan_bt_en: wcn-wlan-bt-en-state {
> +			pins = "gpio116", "gpio117";
> +			function = "gpio";
> +			drive-strength = <2>;
> +			bias-disable;
> +	};

and here (+ these last 2 entries are out of order, GPIO num-wise,
please adjust that )

[...]

> +++ b/arch/arm64/boot/dts/qcom/x1e80100-microsoft-denali-oled.dts
> @@ -0,0 +1,20 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2025 Dale Whinham <daleyo@...il.com>
> + */
> +
> +/dts-v1/;
> +
> +#include "x1e80100.dtsi"
> +#include "x1-microsoft-denali.dtsi"
> +
> +/ {
> +	model = "Microsoft Surface Pro 11th Edition (OLED)";
> +	compatible = "microsoft,denali-oled", "microsoft,denali",
> +	             "qcom,x1e80100";

Are the OLED models always X1E and the LCD ones always based on X1E80100
and LCD models always based on X1P64100?

Konrad

> +};
> +
> +&panel {
> +	compatible = "samsung,atna30dw01", "samsung,atna33xc20";
> +};
> diff --git a/arch/arm64/boot/dts/qcom/x1p64100-microsoft-denali.dts b/arch/arm64/boot/dts/qcom/x1p64100-microsoft-denali.dts
> new file mode 100644
> index 000000000000..7c064ad49395
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/x1p64100-microsoft-denali.dts
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2025 Dale Whinham <daleyo@...il.com>
> + */
> +
> +/dts-v1/;
> +
> +#include "x1e80100.dtsi"
> +#include "x1-microsoft-denali.dtsi"
> +
> +/ {
> +	model = "Microsoft Surface Pro 11th Edition (LCD)";
> +	compatible = "microsoft,denali-lcd", "microsoft,denali",
> +	             "qcom,x1p64100", "qcom,x1e80100";
> +};