[PATCH v2 0/5] Microsoft Surface Pro 9 5G support

[PATCH v2 0/5] Microsoft Surface Pro 9 5G support

[Jérôme de Bretagne]

This series brings support for the SC8280XP-based Microsoft Surface
Pro 9 5G.

Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
---
Changes in v2:
- Style fixes
- Rename the devicetree to sc8280xp-microsoft-arcata.dts
- Update the audio nodes to use codec@ instead of wcd9380-rx or -tx
- Document which node corresponds to which physical USB Type-C port
- Reordering to get property-0 followed by property-names consistently
- Add a dedicated Wi-Fi calibration variant
- Remove gpio-reserved-ranges in tlmm node as not needed to boot
- Add support for Surface Pro 9 5G in surface_aggregator_registry.c
- Switch to the upstream "microsoft,surface-sam" binding
- Fix uart18 clock from S1 to S2
- Include Acked-by: and Reviewed-by: tags
- Link to v1: https://lore.kernel.org/all/20240903224252.6207-1-jerome.debretagne@gmail.com/

---
Jérôme de Bretagne (5):
  dt-bindings: arm: qcom: Document Microsoft Surface Pro 9 5G
  firmware: qcom: scm: Allow QSEECOM on Microsoft Surface Pro 9 5G
  platform/surface: aggregator_registry: Add Surface Pro 9 5G
  arm64: dts: qcom: sc8280xp: Add uart18
  arm64: dts: qcom: sc8280xp: Add Microsoft Surface Pro 9 5G

 .../devicetree/bindings/arm/qcom.yaml         |    1 +
 arch/arm64/boot/dts/qcom/Makefile             |    1 +
 .../dts/qcom/sc8280xp-microsoft-arcata.dts    | 1032 +++++++++++++++++
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi        |   48 +
 drivers/firmware/qcom/qcom_scm.c              |    1 +
 .../surface/surface_aggregator_registry.c     |   17 +
 6 files changed, 1100 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp-microsoft-arcata.dts

-- 
2.45.2

[PATCH v2 1/5] dt-bindings: arm: qcom: Document Microsoft Surface Pro 9 5G

[Jérôme de Bretagne]

Add compatible for the SC8280XP-based Microsoft Surface Pro 9 5G,
using its Arcata codename.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
---
 Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index f08e13b61172..114c7f5dc402 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -810,6 +810,7 @@ properties:
       - items:
           - enum:
               - lenovo,thinkpad-x13s
+              - microsoft,arcata
               - qcom,sc8280xp-crd
               - qcom,sc8280xp-qrd
           - const: qcom,sc8280xp
-- 
2.45.2

[PATCH v2 2/5] firmware: qcom: scm: Allow QSEECOM on Microsoft Surface Pro 9 5G

[Jérôme de Bretagne]

Add the SC8280XP-based Microsoft Surface Pro 9 5G to the allowlist.

Reviewed-by: Konrad Dybcio <konradybcio@kernel.org>
Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
---
 drivers/firmware/qcom/qcom_scm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 00c379a3cceb..5c8d8d75fb0a 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1725,6 +1725,7 @@ EXPORT_SYMBOL_GPL(qcom_scm_qseecom_app_send);
 static const struct of_device_id qcom_scm_qseecom_allowlist[] __maybe_unused = {
 	{ .compatible = "lenovo,flex-5g" },
 	{ .compatible = "lenovo,thinkpad-x13s", },
+	{ .compatible = "microsoft,arcata", },
 	{ .compatible = "qcom,sc8180x-primus" },
 	{ .compatible = "qcom,x1e80100-crd" },
 	{ .compatible = "qcom,x1e80100-qcp" },
-- 
2.45.2

[PATCH v2 3/5] platform/surface: aggregator_registry: Add Surface Pro 9 5G

[Jérôme de Bretagne]

Add SAM client device nodes for the Surface Pro 9 5G, with the usual
battery/AC and HID nodes for keyboard and touchpad support.

Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
---
 .../surface/surface_aggregator_registry.c       | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index 25c8aa2131d6..8b34d7e465c2 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -390,6 +390,21 @@ static const struct software_node *ssam_node_group_sp9[] = {
 	NULL,
 };
 
+/* Devices for Surface Pro 9 5G. */
+static const struct software_node *ssam_node_group_sp9_5G[] = {
+	&ssam_node_root,
+	&ssam_node_hub_kip,
+	&ssam_node_bat_ac,
+	&ssam_node_bat_main,
+	&ssam_node_tmp_sensors,
+	&ssam_node_hid_kip_keyboard,
+	&ssam_node_hid_kip_penstash,
+	&ssam_node_hid_kip_touchpad,
+	&ssam_node_hid_kip_fwupd,
+	&ssam_node_hid_sam_sensors,
+	&ssam_node_kip_tablet_switch,
+	NULL,
+};
 
 /* -- SSAM platform/meta-hub driver. ---------------------------------------- */
 
@@ -462,6 +477,8 @@ static const struct acpi_device_id ssam_platform_hub_acpi_match[] = {
 MODULE_DEVICE_TABLE(acpi, ssam_platform_hub_acpi_match);
 
 static const struct of_device_id ssam_platform_hub_of_match[] __maybe_unused = {
+	/* Surface Pro 9 5G */
+	{ .compatible = "microsoft,arcata", (void *)ssam_node_group_sp9_5G },
 	/* Surface Laptop 7 */
 	{ .compatible = "microsoft,romulus13", (void *)ssam_node_group_sl7 },
 	{ .compatible = "microsoft,romulus15", (void *)ssam_node_group_sl7 },
-- 
2.45.2

[PATCH v2 4/5] arm64: dts: qcom: sc8280xp: Add uart18

[Jérôme de Bretagne]

Add the node describing uart18 for sc8280xp devices.

Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
---
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 48 ++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 14c3b1d6ad47..1e520113db07 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -1013,6 +1013,24 @@ spi18: spi@888000 {
 				status = "disabled";
 			};
 
+			uart18: serial@888000 {
+				compatible = "qcom,geni-uart";
+				reg = <0 0x00888000 0 0x4000>;
+				clocks = <&gcc GCC_QUPV3_WRAP2_S2_CLK>;
+				clock-names = "se";
+				interrupts = <GIC_SPI 584 IRQ_TYPE_LEVEL_HIGH>;
+				operating-points-v2 = <&qup_opp_table_100mhz>;
+				power-domains = <&rpmhpd SC8280XP_CX>;
+				interconnects = <&clk_virt MASTER_QUP_CORE_2 0 &clk_virt SLAVE_QUP_CORE_2 0>,
+						<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_QUP_2 0>;
+				interconnect-names = "qup-core", "qup-config";
+
+				pinctrl-0 = <&qup_uart18_default>;
+				pinctrl-names = "default";
+
+				status = "disabled";
+			};
+
 			i2c19: i2c@88c000 {
 				compatible = "qcom,geni-i2c";
 				reg = <0 0x0088c000 0 0x4000>;
@@ -4957,6 +4975,36 @@ cci3_i2c1_sleep: cci3-i2c1-sleep-pins {
 					bias-pull-down;
 				};
 			};
+
+			qup_uart18_default: qup-uart18-default-state {
+				cts-pins {
+					pins = "gpio66";
+					function = "qup18";
+					drive-strength = <2>;
+					bias-disable;
+				};
+
+				rts-pins {
+					pins = "gpio67";
+					function = "qup18";
+					drive-strength = <2>;
+					bias-disable;
+				};
+
+				tx-pins {
+					pins = "gpio68";
+					function = "qup18";
+					drive-strength = <2>;
+					bias-disable;
+				};
+
+				rx-pins {
+					pins = "gpio69";
+					function = "qup18";
+					drive-strength = <2>;
+					bias-disable;
+				};
+			};
 		};
 
 		apps_smmu: iommu@15000000 {
-- 
2.45.2

[PATCH v2 5/5] arm64: dts: qcom: sc8280xp: Add Microsoft Surface Pro 9 5G

[Jérôme de Bretagne]

Add an initial devicetree for the Microsoft Surface Pro 9 5G, based
on SC8280XP.

It enables the support for Wi-Fi, NVMe, the two USB Type-C ports,
Bluetooth, 5G cellular modem, audio output (via Bluetooth headsets
or USB audio), external display via DisplayPort over Type-C (only
the bottom USB Type-C port is working so far), charging, the Surface
Aggregator Module (SAM) to get keyboard and touchpad working with
Surface Type Cover accessories.

Some key features not supported yet:
- built-in display (but software fallback is working with efifb
  when blacklisting the msm module)
- built-in display touchscreen
- external display with the top USB Type-C port
- speakers and microphones
- physical volume up and down keys
- LID switch detection

This devicetree is based on the other SC8280XP ones, for the Lenovo
ThinkPad X13s and the Qualcomm CRD.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
---
 arch/arm64/boot/dts/qcom/Makefile             |    1 +
 .../dts/qcom/sc8280xp-microsoft-arcata.dts    | 1032 +++++++++++++++++
 2 files changed, 1033 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp-microsoft-arcata.dts

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 0e5c810304fb..feebd1af5fe6 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -184,6 +184,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sc8180x-lenovo-flex-5g.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc8180x-primus.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc8280xp-crd.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc8280xp-lenovo-thinkpad-x13s.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc8280xp-microsoft-arcata.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sda660-inforce-ifc6560.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm450-lenovo-tbx605f.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm450-motorola-ali.dtb
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-microsoft-arcata.dts b/arch/arm64/boot/dts/qcom/sc8280xp-microsoft-arcata.dts
new file mode 100644
index 000000000000..a31742471f51
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-microsoft-arcata.dts
@@ -0,0 +1,1032 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2024, Jérôme de Bretagne <jerome.debretagne@gmail.com>
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+
+#include "sc8280xp.dtsi"
+#include "sc8280xp-pmics.dtsi"
+
+/ {
+	model = "Microsoft Surface Pro 9 5G";
+	compatible = "microsoft,arcata", "qcom,sc8280xp";
+
+	aliases {
+		serial0 = &uart18;
+		serial1 = &uart2;
+	};
+
+	wcd938x: audio-codec {
+		compatible = "qcom,wcd9380-codec";
+
+		pinctrl-0 = <&wcd_default>;
+		pinctrl-names = "default";
+
+		reset-gpios = <&tlmm 106 GPIO_ACTIVE_LOW>;
+
+		vdd-buck-supply = <&vreg_s10b>;
+		vdd-rxtx-supply = <&vreg_s10b>;
+		vdd-io-supply = <&vreg_s10b>;
+		vdd-mic-bias-supply = <&vreg_bob>;
+
+		qcom,micbias1-microvolt = <1800000>;
+		qcom,micbias2-microvolt = <1800000>;
+		qcom,micbias3-microvolt = <1800000>;
+		qcom,micbias4-microvolt = <1800000>;
+		qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
+		qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
+		qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
+		qcom,rx-device = <&wcd_rx>;
+		qcom,tx-device = <&wcd_tx>;
+
+		#sound-dai-cells = <1>;
+	};
+
+	pmic-glink {
+		compatible = "qcom,sc8280xp-pmic-glink", "qcom,pmic-glink";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		/* Left-side top port */
+		connector@0 {
+			compatible = "usb-c-connector";
+			reg = <0>;
+			power-role = "dual";
+			data-role = "dual";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+
+					pmic_glink_con0_hs: endpoint {
+						remote-endpoint = <&usb_0_dwc3_hs>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+
+					pmic_glink_con0_ss: endpoint {
+						remote-endpoint = <&usb_0_qmpphy_out>;
+					};
+				};
+
+				port@2 {
+					reg = <2>;
+
+					pmic_glink_con0_sbu: endpoint {
+						remote-endpoint = <&usb0_sbu_mux>;
+					};
+				};
+			};
+		};
+
+		/* Left-side bottom port */
+		connector@1 {
+			compatible = "usb-c-connector";
+			reg = <1>;
+			power-role = "dual";
+			data-role = "dual";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+
+					pmic_glink_con1_hs: endpoint {
+						remote-endpoint = <&usb_1_dwc3_hs>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+
+					pmic_glink_con1_ss: endpoint {
+						remote-endpoint = <&usb_1_qmpphy_out>;
+					};
+				};
+
+				port@2 {
+					reg = <2>;
+
+					pmic_glink_con1_sbu: endpoint {
+						remote-endpoint = <&usb1_sbu_mux>;
+					};
+				};
+			};
+		};
+	};
+
+	vreg_nvme: regulator-nvme {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VCC3_SSD";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&tlmm 135 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-0 = <&nvme_reg_en>;
+		pinctrl-names = "default";
+
+		regulator-boot-on;
+	};
+
+	vreg_vph_pwr: regulator-vph-pwr {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VPH_VCC3R9";
+		regulator-min-microvolt = <3900000>;
+		regulator-max-microvolt = <3900000>;
+
+		regulator-always-on;
+	};
+
+	vreg_wlan: regulator-wlan {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VCC_WLAN_3R9";
+		regulator-min-microvolt = <3900000>;
+		regulator-max-microvolt = <3900000>;
+
+		gpio = <&pmr735a_gpios 1 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-0 = <&hastings_reg_en>;
+		pinctrl-names = "default";
+
+		regulator-boot-on;
+	};
+
+	vreg_wwan: regulator-wwan {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VCC3B_WAN";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&pmc8280_2_gpios 1 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-0 = <&wwan_sw_en>;
+		pinctrl-names = "default";
+
+		regulator-boot-on;
+	};
+
+	reserved-memory {
+		gpu_mem: gpu-mem@8bf00000 {
+			reg = <0 0x8bf00000 0 0x2000>;
+			no-map;
+		};
+
+		linux,cma {
+			compatible = "shared-dma-pool";
+			size = <0x0 0x8000000>;
+			reusable;
+			linux,cma-default;
+		};
+	};
+
+	thermal-zones {
+		skin-temp-thermal {
+			polling-delay-passive = <250>;
+
+			thermal-sensors = <&pmk8280_adc_tm 5>;
+
+			trips {
+				skin_temp_alert0: trip-point0 {
+					temperature = <55000>;
+					hysteresis = <1000>;
+					type = "passive";
+				};
+
+				skin_temp_alert1: trip-point1 {
+					temperature = <58000>;
+					hysteresis = <1000>;
+					type = "passive";
+				};
+
+				skin-temp-crit {
+					temperature = <73000>;
+					hysteresis = <1000>;
+					type = "critical";
+				};
+			};
+
+			cooling-maps {
+				map0 {
+					trip = <&skin_temp_alert0>;
+					cooling-device = <&CPU4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&CPU5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&CPU6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&CPU7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				};
+
+				map1 {
+					trip = <&skin_temp_alert1>;
+					cooling-device = <&CPU4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&CPU5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&CPU6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&CPU7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				};
+			};
+		};
+	};
+
+	usb0-sbu-mux {
+		compatible = "pericom,pi3usb102", "gpio-sbu-mux";
+
+		enable-gpios = <&tlmm 101 GPIO_ACTIVE_LOW>;
+		select-gpios = <&tlmm 164 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-0 = <&usb0_sbu_default>;
+		pinctrl-names = "default";
+
+		mode-switch;
+		orientation-switch;
+
+		port {
+			usb0_sbu_mux: endpoint {
+				remote-endpoint = <&pmic_glink_con0_sbu>;
+			};
+		};
+	};
+
+	usb1-sbu-mux {
+		compatible = "pericom,pi3usb102", "gpio-sbu-mux";
+
+		enable-gpios = <&tlmm 48 GPIO_ACTIVE_LOW>;
+		select-gpios = <&tlmm 47 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-0 = <&usb1_sbu_default>;
+		pinctrl-names = "default";
+
+		mode-switch;
+		orientation-switch;
+
+		port {
+			usb1_sbu_mux: endpoint {
+				remote-endpoint = <&pmic_glink_con1_sbu>;
+			};
+		};
+	};
+};
+
+&apps_rsc {
+	regulators-0 {
+		compatible = "qcom,pm8350-rpmh-regulators";
+		qcom,pmic-id = "b";
+
+		vdd-l1-l4-supply = <&vreg_s12b>;
+		vdd-l2-l7-supply = <&vreg_bob>;
+		vdd-l3-l5-supply = <&vreg_s11b>;
+		vdd-l6-l9-l10-supply = <&vreg_s12b>;
+		vdd-l8-supply = <&vreg_s12b>;
+
+		vreg_s10b: smps10 {
+			regulator-name = "vreg_s10b";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+			regulator-always-on;
+		};
+
+		vreg_s11b: smps11 {
+			regulator-name = "vreg_s11b";
+			regulator-min-microvolt = <1272000>;
+			regulator-max-microvolt = <1272000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+			regulator-always-on;
+		};
+
+		vreg_s12b: smps12 {
+			regulator-name = "vreg_s12b";
+			regulator-min-microvolt = <984000>;
+			regulator-max-microvolt = <984000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+			regulator-always-on;
+		};
+
+		vreg_l3b: ldo3 {
+			regulator-name = "vreg_l3b";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+			regulator-boot-on;
+		};
+
+		vreg_l4b: ldo4 {
+			regulator-name = "vreg_l4b";
+			regulator-min-microvolt = <912000>;
+			regulator-max-microvolt = <912000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l6b: ldo6 {
+			regulator-name = "vreg_l6b";
+			regulator-min-microvolt = <880000>;
+			regulator-max-microvolt = <880000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+			regulator-boot-on;
+			regulator-always-on;	// FIXME: VDD_A_EDP_0_0P9
+		};
+	};
+
+	regulators-1 {
+		compatible = "qcom,pm8350c-rpmh-regulators";
+		qcom,pmic-id = "c";
+
+		vdd-bob-supply = <&vreg_vph_pwr>;
+		vdd-l1-l12-supply = <&vreg_s1c>;
+		vdd-l2-l8-supply = <&vreg_s1c>;
+		vdd-l3-l4-l5-l7-l13-supply = <&vreg_bob>;
+		vdd-l6-l9-l11-supply = <&vreg_bob>;
+		vdd-l10-supply = <&vreg_s11b>;
+
+		vreg_s1c: smps1 {
+			regulator-name = "vreg_s1c";
+			regulator-min-microvolt = <1880000>;
+			regulator-max-microvolt = <1900000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+			regulator-always-on;
+		};
+
+		vreg_l1c: ldo1 {
+			regulator-name = "vreg_l1c";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l12c: ldo12 {
+			regulator-name = "vreg_l12c";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l13c: ldo13 {
+			regulator-name = "vreg_l13c";
+			regulator-min-microvolt = <3072000>;
+			regulator-max-microvolt = <3072000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_bob: bob {
+			regulator-name = "vreg_bob";
+			regulator-min-microvolt = <3008000>;
+			regulator-max-microvolt = <3960000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
+			regulator-always-on;
+		};
+	};
+
+	regulators-2 {
+		compatible = "qcom,pm8350-rpmh-regulators";
+		qcom,pmic-id = "d";
+
+		vdd-l1-l4-supply = <&vreg_s11b>;
+		vdd-l2-l7-supply = <&vreg_bob>;
+		vdd-l3-l5-supply = <&vreg_s11b>;
+		vdd-l6-l9-l10-supply = <&vreg_s12b>;
+		vdd-l8-supply = <&vreg_s12b>;
+
+		vreg_l3d: ldo3 {
+			regulator-name = "vreg_l3d";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l4d: ldo4 {
+			regulator-name = "vreg_l4d";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l6d: ldo6 {
+			regulator-name = "vreg_l6d";
+			regulator-min-microvolt = <880000>;
+			regulator-max-microvolt = <880000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l7d: ldo7 {
+			regulator-name = "vreg_l7d";
+			regulator-min-microvolt = <3072000>;
+			regulator-max-microvolt = <3072000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l9d: ldo9 {
+			regulator-name = "vreg_l9d";
+			regulator-min-microvolt = <912000>;
+			regulator-max-microvolt = <912000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+	};
+};
+
+&dispcc0 {
+	status = "okay";
+};
+
+&dispcc1 {
+	status = "okay";
+};
+
+&gpu {
+	status = "okay";
+
+	zap-shader {
+		memory-region = <&gpu_mem>;
+		firmware-name = "qcom/sc8280xp/MICROSOFT/SurfacePro9/qcdxkmsuc8280.mbn";
+	};
+};
+
+&mdss0 {
+	status = "okay";
+};
+
+&mdss0_dp0 {
+	status = "okay";
+};
+
+&mdss0_dp0_out {
+	data-lanes = <0 1>;
+	remote-endpoint = <&usb_0_qmpphy_dp_in>;
+};
+
+&mdss0_dp1 {
+	status = "okay";
+};
+
+&mdss0_dp1_out {
+	data-lanes = <0 1>;
+	remote-endpoint = <&usb_1_qmpphy_dp_in>;
+};
+
+&pcie2a {
+	perst-gpios = <&tlmm 143 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 145 GPIO_ACTIVE_LOW>;
+
+	vddpe-3v3-supply = <&vreg_nvme>;
+
+	pinctrl-0 = <&pcie2a_default>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
+
+&pcie2a_phy {
+	vdda-phy-supply = <&vreg_l6d>;
+	vdda-pll-supply = <&vreg_l4d>;
+
+	status = "okay";
+};
+
+&pcie3a {
+	perst-gpios = <&tlmm 151 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
+
+	vddpe-3v3-supply = <&vreg_wwan>;
+
+	pinctrl-0 = <&pcie3a_default>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
+
+&pcie3a_phy {
+	vdda-phy-supply = <&vreg_l6d>;
+	vdda-pll-supply = <&vreg_l4d>;
+
+	status = "okay";
+};
+
+&pcie4 {
+	max-link-speed = <2>;
+
+	perst-gpios = <&tlmm 141 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 139 GPIO_ACTIVE_LOW>;
+
+	vddpe-3v3-supply = <&vreg_wlan>;
+
+	pinctrl-0 = <&pcie4_default>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
+
+&pcie4_port0 {
+	wifi@0 {
+		compatible = "pci17cb,1103";
+		reg = <0x10000 0x0 0x0 0x0 0x0>;
+
+		qcom,ath11k-calibration-variant = "MS_SP9_5G";
+	};
+};
+
+&pcie4_phy {
+	vdda-phy-supply = <&vreg_l6d>;
+	vdda-pll-supply = <&vreg_l4d>;
+
+	status = "okay";
+};
+
+&pmk8280_pon_pwrkey {
+	status = "okay";
+};
+
+&pmk8280_pon_resin {
+	status = "okay";
+};
+
+&pmk8280_rtc {
+	nvmem-cells = <&rtc_offset>;
+	nvmem-cell-names = "offset";
+
+	status = "okay";
+};
+
+&pmk8280_sdam_6 {
+	status = "okay";
+
+	rtc_offset: rtc-offset@bc {
+		reg = <0xbc 0x4>;
+	};
+};
+
+&qup0 {
+	status = "okay";
+};
+
+&qup1 {
+	status = "okay";
+};
+
+&qup2 {
+	status = "okay";
+};
+
+&remoteproc_adsp {
+	firmware-name = "qcom/sc8280xp/MICROSOFT/SurfacePro9/qcadsp8280.mbn";
+
+	status = "okay";
+};
+
+&remoteproc_nsp0 {
+	firmware-name = "qcom/sc8280xp/MICROSOFT/SurfacePro9/qccdsp8280.mbn";
+
+	status = "okay";
+};
+
+&rxmacro {
+	status = "okay";
+};
+
+&sound {
+	compatible = "qcom,sc8280xp-sndcard";
+	model = "SC8280XP-MICROSOFT-SURFACE-PRO-9-5G";
+	audio-routing = "SpkrLeft IN", "WSA_SPK1 OUT",
+			"SpkrRight IN", "WSA_SPK2 OUT",
+			"IN1_HPHL", "HPHL_OUT",
+			"IN2_HPHR", "HPHR_OUT",
+			"AMIC2", "MIC BIAS2",
+			"VA DMIC0", "MIC BIAS1",
+			"VA DMIC1", "MIC BIAS1",
+			"VA DMIC2", "MIC BIAS3",
+			"VA DMIC0", "VA MIC BIAS1",
+			"VA DMIC1", "VA MIC BIAS1",
+			"VA DMIC2", "VA MIC BIAS3",
+			"TX SWR_ADC1", "ADC2_OUTPUT";
+
+	wcd-playback-dai-link {
+		link-name = "WCD Playback";
+
+		cpu {
+			sound-dai = <&q6apmbedai RX_CODEC_DMA_RX_0>;
+		};
+
+		codec {
+			sound-dai = <&wcd938x 0>, <&swr1 0>, <&rxmacro 0>;
+		};
+
+		platform {
+			sound-dai = <&q6apm>;
+		};
+	};
+
+	wcd-capture-dai-link {
+		link-name = "WCD Capture";
+
+		cpu {
+			sound-dai = <&q6apmbedai TX_CODEC_DMA_TX_3>;
+		};
+
+		codec {
+			sound-dai = <&wcd938x 1>, <&swr2 0>, <&txmacro 0>;
+		};
+
+		platform {
+			sound-dai = <&q6apm>;
+		};
+	};
+
+	wsa-dai-link {
+		link-name = "WSA Playback";
+
+		cpu {
+			sound-dai = <&q6apmbedai WSA_CODEC_DMA_RX_0>;
+		};
+
+		codec {
+			sound-dai = <&swr0 0>, <&wsamacro 0>;
+		};
+
+		platform {
+			sound-dai = <&q6apm>;
+		};
+	};
+
+	va-dai-link {
+		link-name = "VA Capture";
+
+		cpu {
+			sound-dai = <&q6apmbedai VA_CODEC_DMA_TX_0>;
+		};
+
+		platform {
+			sound-dai = <&q6apm>;
+		};
+
+		codec {
+			sound-dai = <&vamacro 0>;
+		};
+	};
+};
+
+&swr0 {
+	status = "okay";
+};
+
+&swr1 {
+	status = "okay";
+
+	wcd_rx: codec@0,4 {
+		compatible = "sdw20217010d00";
+		reg = <0 4>;
+		qcom,rx-port-mapping = <1 2 3 4 5>;
+	};
+};
+
+&swr2 {
+	status = "okay";
+
+	wcd_tx: codec@0,3 {
+		compatible = "sdw20217010d00";
+		reg = <0 3>;
+		qcom,tx-port-mapping = <1 1 2 3>;
+	};
+};
+
+&txmacro {
+	status = "okay";
+};
+
+&uart2 {
+	pinctrl-0 = <&uart2_default>;
+	pinctrl-names = "default";
+
+	status = "okay";
+
+	bluetooth {
+		compatible = "qcom,wcn6855-bt";
+
+		vddio-supply = <&vreg_s10b>;
+		vddbtcxmx-supply = <&vreg_s12b>;
+		vddrfacmn-supply = <&vreg_s12b>;
+		vddrfa0p8-supply = <&vreg_s12b>;
+		vddrfa1p2-supply = <&vreg_s11b>;
+		vddrfa1p7-supply = <&vreg_s1c>;
+
+		max-speed = <3200000>;
+
+		enable-gpios = <&tlmm 133 GPIO_ACTIVE_HIGH>;
+		swctrl-gpios = <&tlmm 132 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-0 = <&bt_default>;
+		pinctrl-names = "default";
+	};
+};
+
+&uart18 {
+	status = "okay";
+
+	embedded-controller {
+		compatible = "microsoft,surface-sam";
+
+		interrupts-extended = <&tlmm 85 IRQ_TYPE_EDGE_RISING>;
+
+		current-speed = <4000000>;
+
+		pinctrl-0 = <&ssam_state>;
+		pinctrl-names = "default";
+	};
+};
+
+&usb_0 {
+	status = "okay";
+};
+
+&usb_0_dwc3 {
+	dr_mode = "host";
+};
+
+&usb_0_dwc3_hs {
+	remote-endpoint = <&pmic_glink_con0_hs>;
+};
+
+&usb_0_hsphy {
+	vdda-pll-supply = <&vreg_l9d>;
+	vdda18-supply = <&vreg_l1c>;
+	vdda33-supply = <&vreg_l7d>;
+
+	status = "okay";
+};
+
+&usb_0_qmpphy {
+	vdda-phy-supply = <&vreg_l9d>;
+	vdda-pll-supply = <&vreg_l4d>;
+
+	orientation-switch;
+
+	status = "okay";
+};
+
+&usb_0_qmpphy_dp_in {
+	remote-endpoint = <&mdss0_dp0_out>;
+};
+
+&usb_0_qmpphy_out {
+	remote-endpoint = <&pmic_glink_con0_ss>;
+};
+
+&usb_1 {
+	status = "okay";
+};
+
+&usb_1_dwc3 {
+	dr_mode = "host";
+};
+
+&usb_1_dwc3_hs {
+	remote-endpoint = <&pmic_glink_con1_hs>;
+};
+
+&usb_1_hsphy {
+	vdda-pll-supply = <&vreg_l4b>;
+	vdda18-supply = <&vreg_l1c>;
+	vdda33-supply = <&vreg_l13c>;
+
+	status = "okay";
+};
+
+&usb_1_qmpphy {
+	vdda-phy-supply = <&vreg_l4b>;
+	vdda-pll-supply = <&vreg_l3b>;
+
+	orientation-switch;
+
+	status = "okay";
+};
+
+&usb_1_qmpphy_dp_in {
+	remote-endpoint = <&mdss0_dp1_out>;
+};
+
+&usb_1_qmpphy_out {
+	remote-endpoint = <&pmic_glink_con1_ss>;
+};
+
+&vamacro {
+	pinctrl-0 = <&dmic01_default>, <&dmic23_default>;
+	pinctrl-names = "default";
+
+	vdd-micb-supply = <&vreg_s10b>;
+
+	qcom,dmic-sample-rate = <4800000>;
+
+	status = "okay";
+};
+
+&wsamacro {
+	status = "okay";
+};
+
+&xo_board_clk {
+	clock-frequency = <38400000>;
+};
+
+/* PINCTRL */
+
+&lpass_tlmm {
+	status = "okay";
+};
+
+&pmc8280_2_gpios {
+	wwan_sw_en: wwan-sw-en-state {
+		pins = "gpio1";
+		function = "normal";
+	};
+};
+
+&pmr735a_gpios {
+	hastings_reg_en: hastings-reg-en-state {
+		pins = "gpio1";
+		function = "normal";
+	};
+};
+
+&tlmm {
+	bt_default: bt-default-state {
+		hstp-bt-en-pins {
+			pins = "gpio133";
+			function = "gpio";
+			drive-strength = <16>;
+			bias-disable;
+		};
+
+		hstp-sw-ctrl-pins {
+			pins = "gpio132";
+			function = "gpio";
+			bias-pull-down;
+		};
+	};
+
+	nvme_reg_en: nvme-reg-en-state {
+		pins = "gpio135";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	pcie2a_default: pcie2a-default-state {
+		clkreq-n-pins {
+			pins = "gpio142";
+			function = "pcie2a_clkreq";
+			drive-strength = <2>;
+			bias-pull-up;
+		};
+
+		perst-n-pins {
+			pins = "gpio143";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-disable;
+		};
+
+		wake-n-pins {
+			pins = "gpio145";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-pull-up;
+	       };
+	};
+
+	pcie3a_default: pcie3a-default-state {
+		clkreq-n-pins {
+			pins = "gpio150";
+			function = "pcie3a_clkreq";
+			drive-strength = <2>;
+			bias-pull-up;
+		};
+
+		perst-n-pins {
+			pins = "gpio151";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-disable;
+		};
+
+		wake-n-pins {
+			pins = "gpio148";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-pull-up;
+		};
+	};
+
+	pcie4_default: pcie4-default-state {
+		clkreq-n-pins {
+			pins = "gpio140";
+			function = "pcie4_clkreq";
+			drive-strength = <2>;
+			bias-pull-up;
+		};
+
+		perst-n-pins {
+			pins = "gpio141";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-disable;
+		};
+
+		wake-n-pins {
+			pins = "gpio139";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-pull-up;
+		};
+	};
+
+
+	ssam_state: ssam-state-state {
+		pins = "gpio85";
+		function = "gpio";
+		bias-disable;
+	};
+
+	uart2_default: uart2-default-state {
+		cts-pins {
+			pins = "gpio121";
+			function = "qup2";
+			bias-bus-hold;
+		};
+
+		rts-pins {
+			pins = "gpio122";
+			function = "qup2";
+			drive-strength = <2>;
+			bias-disable;
+		};
+
+		rx-pins {
+			pins = "gpio124";
+			function = "qup2";
+			bias-pull-up;
+		};
+
+		tx-pins {
+			pins = "gpio123";
+			function = "qup2";
+			drive-strength = <2>;
+			bias-disable;
+		};
+	};
+
+	usb0_sbu_default: usb0-sbu-state {
+		oe-n-pins {
+			pins = "gpio101";
+			function = "gpio";
+			bias-disable;
+			drive-strength = <16>;
+			output-high;
+		};
+
+		sel-pins {
+			pins = "gpio164";
+			function = "gpio";
+			bias-disable;
+			drive-strength = <16>;
+		};
+	};
+
+	usb1_sbu_default: usb1-sbu-state {
+		oe-n-pins {
+			pins = "gpio48";
+			function = "gpio";
+			bias-disable;
+			drive-strength = <16>;
+			output-high;
+		};
+
+		sel-pins {
+			pins = "gpio47";
+			function = "gpio";
+			bias-disable;
+			drive-strength = <16>;
+		};
+	};
+
+	wcd_default: wcd-default-state {
+		reset-pins {
+			pins = "gpio106";
+			function = "gpio";
+			bias-disable;
+		};
+	};
+};
-- 
2.45.2

Re: [PATCH v2 2/5] firmware: qcom: scm: Allow QSEECOM on Microsoft Surface Pro 9 5G

[Maximilian Luz]

On 9/9/24 12:35 AM, Jérôme de Bretagne wrote:
> Add the SC8280XP-based Microsoft Surface Pro 9 5G to the allowlist.
> 
> Reviewed-by: Konrad Dybcio <konradybcio@kernel.org>
> Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
> ---
>   drivers/firmware/qcom/qcom_scm.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 00c379a3cceb..5c8d8d75fb0a 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -1725,6 +1725,7 @@ EXPORT_SYMBOL_GPL(qcom_scm_qseecom_app_send);
>   static const struct of_device_id qcom_scm_qseecom_allowlist[] __maybe_unused = {
>   	{ .compatible = "lenovo,flex-5g" },
>   	{ .compatible = "lenovo,thinkpad-x13s", },
> +	{ .compatible = "microsoft,arcata", },
>   	{ .compatible = "qcom,sc8180x-primus" },
>   	{ .compatible = "qcom,x1e80100-crd" },
>   	{ .compatible = "qcom,x1e80100-qcp" },

Looks good to me.

Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>

Re: [PATCH v2 3/5] platform/surface: aggregator_registry: Add Surface Pro 9 5G

[Maximilian Luz]

Looks good. Two very small nit-picks below, if this goes for a v3:

On 9/9/24 12:35 AM, Jérôme de Bretagne wrote:
> Add SAM client device nodes for the Surface Pro 9 5G, with the usual
> battery/AC and HID nodes for keyboard and touchpad support.
> 
> Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
> ---
>   .../surface/surface_aggregator_registry.c       | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
> index 25c8aa2131d6..8b34d7e465c2 100644
> --- a/drivers/platform/surface/surface_aggregator_registry.c
> +++ b/drivers/platform/surface/surface_aggregator_registry.c
> @@ -390,6 +390,21 @@ static const struct software_node *ssam_node_group_sp9[] = {
>   	NULL,
>   };
>   
> +/* Devices for Surface Pro 9 5G. */

Would be nice if you could change the comment on the SP9 node group to
"Surface Pro 9 (Intel/x86)" and the comment here to "Surface Pro 9 5G
(ARM/QCOM)" or something along those lines to make things a bit more
clear.

> +static const struct software_node *ssam_node_group_sp9_5G[] = {

(This is really just me being a bit obsessive:) It would be nice to have
all-lowercase variable names (regarding the 5G).

> +	&ssam_node_root,
> +	&ssam_node_hub_kip,
> +	&ssam_node_bat_ac,
> +	&ssam_node_bat_main,
> +	&ssam_node_tmp_sensors,
> +	&ssam_node_hid_kip_keyboard,
> +	&ssam_node_hid_kip_penstash,
> +	&ssam_node_hid_kip_touchpad,
> +	&ssam_node_hid_kip_fwupd,
> +	&ssam_node_hid_sam_sensors,
> +	&ssam_node_kip_tablet_switch,
> +	NULL,
> +};
>   
>   /* -- SSAM platform/meta-hub driver. ---------------------------------------- */
>   
> @@ -462,6 +477,8 @@ static const struct acpi_device_id ssam_platform_hub_acpi_match[] = {
>   MODULE_DEVICE_TABLE(acpi, ssam_platform_hub_acpi_match);
>   
>   static const struct of_device_id ssam_platform_hub_of_match[] __maybe_unused = {
> +	/* Surface Pro 9 5G */
> +	{ .compatible = "microsoft,arcata", (void *)ssam_node_group_sp9_5G },
>   	/* Surface Laptop 7 */
>   	{ .compatible = "microsoft,romulus13", (void *)ssam_node_group_sl7 },
>   	{ .compatible = "microsoft,romulus15", (void *)ssam_node_group_sl7 },

Thanks!

Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>

On a related note: I'm wondering whether we should also already add the
ACPI IDs for the as-of-now ARM-only devices. Technically, things won't
work with ACPI right now, but it's not because of the SAM driver stack.
Or maybe at least add them to the comments as a way of documenting it.

Best regards,
Max

Re: (subset) [PATCH v2 0/5] Microsoft Surface Pro 9 5G support

[Bjorn Andersson]

On Mon, 09 Sep 2024 00:35:00 +0200, Jérôme de Bretagne wrote:
> This series brings support for the SC8280XP-based Microsoft Surface
> Pro 9 5G.
> 
> 

Applied, thanks!

[1/5] dt-bindings: arm: qcom: Document Microsoft Surface Pro 9 5G
      commit: e221af165910b8967f7c2788c34826f332f2cc7e
[4/5] arm64: dts: qcom: sc8280xp: Add uart18
      commit: 1e70551123d014b3a1c4b85da54d247243750e7c
[5/5] arm64: dts: qcom: sc8280xp: Add Microsoft Surface Pro 9 5G
      commit: f6231a2eefd430b8b8798911f023891ea51d1d09

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

Re: (subset) [PATCH v2 0/5] Microsoft Surface Pro 9 5G support

[Jérôme de Bretagne]

On Mon, 07 Oct 2024 16:26, Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Mon, 09 Sep 2024 00:35:00 +0200, Jérôme de Bretagne wrote:
> > This series brings support for the SC8280XP-based Microsoft Surface
> > Pro 9 5G.
> >
>
> Applied, thanks!

Thanks, Bjorn!

> [1/5] dt-bindings: arm: qcom: Document Microsoft Surface Pro 9 5G
>       commit: e221af165910b8967f7c2788c34826f332f2cc7e
> [4/5] arm64: dts: qcom: sc8280xp: Add uart18
>       commit: 1e70551123d014b3a1c4b85da54d247243750e7c
> [5/5] arm64: dts: qcom: sc8280xp: Add Microsoft Surface Pro 9 5G
>       commit: f6231a2eefd430b8b8798911f023891ea51d1d09
>
> Best regards,
> --
> Bjorn Andersson <andersson@kernel.org>

Re: [PATCH v2 3/5] platform/surface: aggregator_registry: Add Surface Pro 9 5G

[Jérôme de Bretagne]

Hi,

I'm replying with Hans and Ilpo, who I initially forgot for this
patch, sorry about that.

Le mar. 10 sept. 2024 à 23:29, Maximilian Luz
<luzmaximilian@gmail.com> a écrit :
>
> Looks good. Two very small nit-picks below, if this goes for a v3:

Atm I'm not planning for a v3 as Bjorn has applied the other v2
patches earlier today.
Feel free to include the 2 small suggestions when applying this patch maybe?

> On 9/9/24 12:35 AM, Jérôme de Bretagne wrote:
> > Add SAM client device nodes for the Surface Pro 9 5G, with the usual
> > battery/AC and HID nodes for keyboard and touchpad support.
> >
> > Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
> > ---
> >   .../surface/surface_aggregator_registry.c       | 17 +++++++++++++++++
> >   1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
> > index 25c8aa2131d6..8b34d7e465c2 100644
> > --- a/drivers/platform/surface/surface_aggregator_registry.c
> > +++ b/drivers/platform/surface/surface_aggregator_registry.c
> > @@ -390,6 +390,21 @@ static const struct software_node *ssam_node_group_sp9[] = {
> >       NULL,
> >   };
> >
> > +/* Devices for Surface Pro 9 5G. */
>
> Would be nice if you could change the comment on the SP9 node group to
> "Surface Pro 9 (Intel/x86)" and the comment here to "Surface Pro 9 5G
> (ARM/QCOM)" or something along those lines to make things a bit more
> clear.
>
> > +static const struct software_node *ssam_node_group_sp9_5G[] = {
>
> (This is really just me being a bit obsessive:) It would be nice to have
> all-lowercase variable names (regarding the 5G).

:)

> > +     &ssam_node_root,
> > +     &ssam_node_hub_kip,
> > +     &ssam_node_bat_ac,
> > +     &ssam_node_bat_main,
> > +     &ssam_node_tmp_sensors,
> > +     &ssam_node_hid_kip_keyboard,
> > +     &ssam_node_hid_kip_penstash,
> > +     &ssam_node_hid_kip_touchpad,
> > +     &ssam_node_hid_kip_fwupd,
> > +     &ssam_node_hid_sam_sensors,
> > +     &ssam_node_kip_tablet_switch,
> > +     NULL,
> > +};
> >
> >   /* -- SSAM platform/meta-hub driver. ---------------------------------------- */
> >
> > @@ -462,6 +477,8 @@ static const struct acpi_device_id ssam_platform_hub_acpi_match[] = {
> >   MODULE_DEVICE_TABLE(acpi, ssam_platform_hub_acpi_match);
> >
> >   static const struct of_device_id ssam_platform_hub_of_match[] __maybe_unused = {
> > +     /* Surface Pro 9 5G */
> > +     { .compatible = "microsoft,arcata", (void *)ssam_node_group_sp9_5G },
> >       /* Surface Laptop 7 */
> >       { .compatible = "microsoft,romulus13", (void *)ssam_node_group_sl7 },
> >       { .compatible = "microsoft,romulus15", (void *)ssam_node_group_sl7 },
>
> Thanks!
>
> Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>

Thanks for your review and all the work about SSAM for Surface owners!

Regards,
Jérôme

> On a related note: I'm wondering whether we should also already add the
> ACPI IDs for the as-of-now ARM-only devices. Technically, things won't
> work with ACPI right now, but it's not because of the SAM driver stack.
> Or maybe at least add them to the comments as a way of documenting it.
>
> Best regards,
> Max

Re: [PATCH v2 3/5] platform/surface: aggregator_registry: Add Surface Pro 9 5G

[Hans de Goede]

Hi Jérôme,

On 7-Oct-24 8:44 PM, Jérôme de Bretagne wrote:
> Hi,
> 
> I'm replying with Hans and Ilpo, who I initially forgot for this
> patch, sorry about that.

No worries thank you for forwarding Maximilian's review.

> Le mar. 10 sept. 2024 à 23:29, Maximilian Luz
> <luzmaximilian@gmail.com> a écrit :
>>
>> Looks good. Two very small nit-picks below, if this goes for a v3:
> 
> Atm I'm not planning for a v3 as Bjorn has applied the other v2
> patches earlier today.
> Feel free to include the 2 small suggestions when applying this patch maybe?
> 
>> On 9/9/24 12:35 AM, Jérôme de Bretagne wrote:
>>> Add SAM client device nodes for the Surface Pro 9 5G, with the usual
>>> battery/AC and HID nodes for keyboard and touchpad support.
>>>
>>> Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
>>> ---
>>>   .../surface/surface_aggregator_registry.c       | 17 +++++++++++++++++
>>>   1 file changed, 17 insertions(+)
>>>
>>> diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
>>> index 25c8aa2131d6..8b34d7e465c2 100644
>>> --- a/drivers/platform/surface/surface_aggregator_registry.c
>>> +++ b/drivers/platform/surface/surface_aggregator_registry.c
>>> @@ -390,6 +390,21 @@ static const struct software_node *ssam_node_group_sp9[] = {
>>>       NULL,
>>>   };
>>>
>>> +/* Devices for Surface Pro 9 5G. */
>>
>> Would be nice if you could change the comment on the SP9 node group to
>> "Surface Pro 9 (Intel/x86)" and the comment here to "Surface Pro 9 5G
>> (ARM/QCOM)" or something along those lines to make things a bit more
>> clear.
>>
>>> +static const struct software_node *ssam_node_group_sp9_5G[] = {
>>
>> (This is really just me being a bit obsessive:) It would be nice to have
>> all-lowercase variable names (regarding the 5G).
> 
> :)
> 
>>> +     &ssam_node_root,
>>> +     &ssam_node_hub_kip,
>>> +     &ssam_node_bat_ac,
>>> +     &ssam_node_bat_main,
>>> +     &ssam_node_tmp_sensors,
>>> +     &ssam_node_hid_kip_keyboard,
>>> +     &ssam_node_hid_kip_penstash,
>>> +     &ssam_node_hid_kip_touchpad,
>>> +     &ssam_node_hid_kip_fwupd,
>>> +     &ssam_node_hid_sam_sensors,
>>> +     &ssam_node_kip_tablet_switch,
>>> +     NULL,
>>> +};
>>>
>>>   /* -- SSAM platform/meta-hub driver. ---------------------------------------- */
>>>
>>> @@ -462,6 +477,8 @@ static const struct acpi_device_id ssam_platform_hub_acpi_match[] = {
>>>   MODULE_DEVICE_TABLE(acpi, ssam_platform_hub_acpi_match);
>>>
>>>   static const struct of_device_id ssam_platform_hub_of_match[] __maybe_unused = {
>>> +     /* Surface Pro 9 5G */
>>> +     { .compatible = "microsoft,arcata", (void *)ssam_node_group_sp9_5G },
>>>       /* Surface Laptop 7 */
>>>       { .compatible = "microsoft,romulus13", (void *)ssam_node_group_sl7 },
>>>       { .compatible = "microsoft,romulus15", (void *)ssam_node_group_sl7 },
>>
>> Thanks!
>>
>> Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>
> 
> Thanks for your review and all the work about SSAM for Surface owners!

FWIW I agree with Maximilian's remarks and I would really like to
see these applied to clearly differentiate the x86 and ARM versions.

Normally I would pick up a patch like this which just adds hw-ids as
a fix for 6.12-rc# and squash in the suggested changes.

But looking at the test of the series this is more 6.13 material
since the rest is landing in 6.13, right ?

Patches for linux-next / 6.13 are managed by Ilpo this cycle.

So I'll leave it up to Ilpo if he will squash in the suggested changes
or if he wants a new version (of just this patch, no need for a v3
of the already applied patches).

Regards,

Hans

Re: [PATCH v2 3/5] platform/surface: aggregator_registry: Add Surface Pro 9 5G

[Jérôme de Bretagne]

Le lun. 7 oct. 2024 à 20:54, Hans de Goede <hdegoede@redhat.com> a écrit :
>
> Hi Jérôme,
>
> On 7-Oct-24 8:44 PM, Jérôme de Bretagne wrote:
> > Hi,
> >
> > I'm replying with Hans and Ilpo, who I initially forgot for this
> > patch, sorry about that.
>
> No worries thank you for forwarding Maximilian's review.
>
> > Le mar. 10 sept. 2024 à 23:29, Maximilian Luz
> > <luzmaximilian@gmail.com> a écrit :
> >>
> >> Looks good. Two very small nit-picks below, if this goes for a v3:
> >
> > Atm I'm not planning for a v3 as Bjorn has applied the other v2
> > patches earlier today.
> > Feel free to include the 2 small suggestions when applying this patch maybe?
> >
> >> On 9/9/24 12:35 AM, Jérôme de Bretagne wrote:
> >>> Add SAM client device nodes for the Surface Pro 9 5G, with the usual
> >>> battery/AC and HID nodes for keyboard and touchpad support.
> >>>
> >>> Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
> >>> ---
> >>>   .../surface/surface_aggregator_registry.c       | 17 +++++++++++++++++
> >>>   1 file changed, 17 insertions(+)
> >>>
> >>> diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
> >>> index 25c8aa2131d6..8b34d7e465c2 100644
> >>> --- a/drivers/platform/surface/surface_aggregator_registry.c
> >>> +++ b/drivers/platform/surface/surface_aggregator_registry.c
> >>> @@ -390,6 +390,21 @@ static const struct software_node *ssam_node_group_sp9[] = {
> >>>       NULL,
> >>>   };
> >>>
> >>> +/* Devices for Surface Pro 9 5G. */
> >>
> >> Would be nice if you could change the comment on the SP9 node group to
> >> "Surface Pro 9 (Intel/x86)" and the comment here to "Surface Pro 9 5G
> >> (ARM/QCOM)" or something along those lines to make things a bit more
> >> clear.
> >>
> >>> +static const struct software_node *ssam_node_group_sp9_5G[] = {
> >>
> >> (This is really just me being a bit obsessive:) It would be nice to have
> >> all-lowercase variable names (regarding the 5G).
> >
> > :)
> >
> >>> +     &ssam_node_root,
> >>> +     &ssam_node_hub_kip,
> >>> +     &ssam_node_bat_ac,
> >>> +     &ssam_node_bat_main,
> >>> +     &ssam_node_tmp_sensors,
> >>> +     &ssam_node_hid_kip_keyboard,
> >>> +     &ssam_node_hid_kip_penstash,
> >>> +     &ssam_node_hid_kip_touchpad,
> >>> +     &ssam_node_hid_kip_fwupd,
> >>> +     &ssam_node_hid_sam_sensors,
> >>> +     &ssam_node_kip_tablet_switch,
> >>> +     NULL,
> >>> +};
> >>>
> >>>   /* -- SSAM platform/meta-hub driver. ---------------------------------------- */
> >>>
> >>> @@ -462,6 +477,8 @@ static const struct acpi_device_id ssam_platform_hub_acpi_match[] = {
> >>>   MODULE_DEVICE_TABLE(acpi, ssam_platform_hub_acpi_match);
> >>>
> >>>   static const struct of_device_id ssam_platform_hub_of_match[] __maybe_unused = {
> >>> +     /* Surface Pro 9 5G */
> >>> +     { .compatible = "microsoft,arcata", (void *)ssam_node_group_sp9_5G },
> >>>       /* Surface Laptop 7 */
> >>>       { .compatible = "microsoft,romulus13", (void *)ssam_node_group_sl7 },
> >>>       { .compatible = "microsoft,romulus15", (void *)ssam_node_group_sl7 },
> >>
> >> Thanks!
> >>
> >> Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>
> >
> > Thanks for your review and all the work about SSAM for Surface owners!
>
> FWIW I agree with Maximilian's remarks and I would really like to
> see these applied to clearly differentiate the x86 and ARM versions.

I stuck to the official names but they can be confusing as-is.

> Normally I would pick up a patch like this which just adds hw-ids as
> a fix for 6.12-rc# and squash in the suggested changes.
>
> But looking at the test of the series this is more 6.13 material
> since the rest is landing in 6.13, right ?

The rest is in arm64-for-6.13 and drivers-for-6.13 in Bjorn's tree indeed.

> Patches for linux-next / 6.13 are managed by Ilpo this cycle.
>
> So I'll leave it up to Ilpo if he will squash in the suggested changes
> or if he wants a new version (of just this patch, no need for a v3
> of the already applied patches).
>
> Regards,
>
> Hans

Sure, let's see what Ilpo prefers.

Regards,
Jérôme

Re: [PATCH v2 5/5] arm64: dts: qcom: sc8280xp: Add Microsoft Surface Pro 9 5G

[Johan Hovold]

On Mon, Sep 09, 2024 at 12:35:05AM +0200, Jérôme de Bretagne wrote:
> Add an initial devicetree for the Microsoft Surface Pro 9 5G, based
> on SC8280XP.
> 
> It enables the support for Wi-Fi, NVMe, the two USB Type-C ports,
> Bluetooth, 5G cellular modem, audio output (via Bluetooth headsets
> or USB audio), external display via DisplayPort over Type-C (only
> the bottom USB Type-C port is working so far), charging, the Surface
> Aggregator Module (SAM) to get keyboard and touchpad working with
> Surface Type Cover accessories.
> 
> Some key features not supported yet:
> - built-in display (but software fallback is working with efifb
>   when blacklisting the msm module)
> - built-in display touchscreen
> - external display with the top USB Type-C port
> - speakers and microphones
> - physical volume up and down keys
> - LID switch detection
> 
> This devicetree is based on the other SC8280XP ones, for the Lenovo
> ThinkPad X13s and the Qualcomm CRD.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>

> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2024, Jérôme de Bretagne <jerome.debretagne@gmail.com>
> + */

You mention it in the commit message, but since a lot of this is copied
from the X13s (and CRD) devicetrees you should probably make that clear
here as well and include the relevant copyright notices. For example by
adding:

	Based on ...

	Copyright ...

Johan

Re: [PATCH v2 5/5] arm64: dts: qcom: sc8280xp: Add Microsoft Surface Pro 9 5G

[Jérôme de Bretagne]

Le jeu. 10 oct. 2024 à 10:52, Johan Hovold <johan@kernel.org> a écrit :
>
> On Mon, Sep 09, 2024 at 12:35:05AM +0200, Jérôme de Bretagne wrote:
> > Add an initial devicetree for the Microsoft Surface Pro 9 5G, based
> > on SC8280XP.
> >
> > It enables the support for Wi-Fi, NVMe, the two USB Type-C ports,
> > Bluetooth, 5G cellular modem, audio output (via Bluetooth headsets
> > or USB audio), external display via DisplayPort over Type-C (only
> > the bottom USB Type-C port is working so far), charging, the Surface
> > Aggregator Module (SAM) to get keyboard and touchpad working with
> > Surface Type Cover accessories.
> >
> > Some key features not supported yet:
> > - built-in display (but software fallback is working with efifb
> >   when blacklisting the msm module)
> > - built-in display touchscreen
> > - external display with the top USB Type-C port
> > - speakers and microphones
> > - physical volume up and down keys
> > - LID switch detection
> >
> > This devicetree is based on the other SC8280XP ones, for the Lenovo
> > ThinkPad X13s and the Qualcomm CRD.
> >
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
>
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +/*
> > + * Copyright (c) 2024, Jérôme de Bretagne <jerome.debretagne@gmail.com>
> > + */
>
> You mention it in the commit message, but since a lot of this is copied
> from the X13s (and CRD) devicetrees you should probably make that clear
> here as well and include the relevant copyright notices. For example by
> adding:
>
>         Based on ...
>
>         Copyright ...
>
> Johan

Thank you for the suggestion, so it would look like this?

+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Based on sc8280xp-crd.dts and sc8280xp-lenovo-thinkpad-x13s.dts
+ *
+ * Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2022, Linaro Limited
+ * Copyright (c) 2024, Jérôme de Bretagne <...>
+ */

Bjorn,

How do you want me to add this to the patchset?
Should I send a separate fix-up patch?
Or should I create a newer v3 patchset?

Thank you,
Jérôme

Re: [PATCH v2 5/5] arm64: dts: qcom: sc8280xp: Add Microsoft Surface Pro 9 5G

[Johan Hovold]

On Thu, Oct 10, 2024 at 11:17:35PM +0200, Jérôme de Bretagne wrote:
> Le jeu. 10 oct. 2024 à 10:52, Johan Hovold <johan@kernel.org> a écrit :
> > On Mon, Sep 09, 2024 at 12:35:05AM +0200, Jérôme de Bretagne wrote:

> > You mention it in the commit message, but since a lot of this is copied
> > from the X13s (and CRD) devicetrees you should probably make that clear
> > here as well and include the relevant copyright notices. For example by
> > adding:
> >
> >         Based on ...
> >
> >         Copyright ...

> Thank you for the suggestion, so it would look like this?

You can still keep your copyright notice first, and I meant something
like:

	/*
	 * Microsoft Surface Pro 9 5G devicetree
	 *
	 * Copyright (c) 2024, Jérôme de Bretagne <...>
	 *
	 * Based on sc8280xp-crd.dts and sc8280xp-lenovo-thinkpad-x13s.dts:
	 *
	 * Copyright (c) 2021, The Linux Foundation. All rights reserved.
	 * Copyright (c) 2022, Linaro Limited
	 */

> How do you want me to add this to the patchset?
> Should I send a separate fix-up patch?
> Or should I create a newer v3 patchset?

Ah, sorry, I missed that Bjorn had already picked this one up. Either
leave it as is, or send an incremental (fix-up) patch.

Johan

Re: [PATCH v2 3/5] platform/surface: aggregator_registry: Add Surface Pro 9 5G

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 4160 bytes --]

On Mon, 7 Oct 2024, Hans de Goede wrote:

> Hi Jérôme,
> 
> On 7-Oct-24 8:44 PM, Jérôme de Bretagne wrote:
> > Hi,
> > 
> > I'm replying with Hans and Ilpo, who I initially forgot for this
> > patch, sorry about that.
> 
> No worries thank you for forwarding Maximilian's review.
> 
> > Le mar. 10 sept. 2024 à 23:29, Maximilian Luz
> > <luzmaximilian@gmail.com> a écrit :
> >>
> >> Looks good. Two very small nit-picks below, if this goes for a v3:
> > 
> > Atm I'm not planning for a v3 as Bjorn has applied the other v2
> > patches earlier today.
> > Feel free to include the 2 small suggestions when applying this patch maybe?
> > 
> >> On 9/9/24 12:35 AM, Jérôme de Bretagne wrote:
> >>> Add SAM client device nodes for the Surface Pro 9 5G, with the usual
> >>> battery/AC and HID nodes for keyboard and touchpad support.
> >>>
> >>> Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>
> >>> ---
> >>>   .../surface/surface_aggregator_registry.c       | 17 +++++++++++++++++
> >>>   1 file changed, 17 insertions(+)
> >>>
> >>> diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
> >>> index 25c8aa2131d6..8b34d7e465c2 100644
> >>> --- a/drivers/platform/surface/surface_aggregator_registry.c
> >>> +++ b/drivers/platform/surface/surface_aggregator_registry.c
> >>> @@ -390,6 +390,21 @@ static const struct software_node *ssam_node_group_sp9[] = {
> >>>       NULL,
> >>>   };
> >>>
> >>> +/* Devices for Surface Pro 9 5G. */
> >>
> >> Would be nice if you could change the comment on the SP9 node group to
> >> "Surface Pro 9 (Intel/x86)" and the comment here to "Surface Pro 9 5G
> >> (ARM/QCOM)" or something along those lines to make things a bit more
> >> clear.
> >>
> >>> +static const struct software_node *ssam_node_group_sp9_5G[] = {
> >>
> >> (This is really just me being a bit obsessive:) It would be nice to have
> >> all-lowercase variable names (regarding the 5G).
> > 
> > :)
> > 
> >>> +     &ssam_node_root,
> >>> +     &ssam_node_hub_kip,
> >>> +     &ssam_node_bat_ac,
> >>> +     &ssam_node_bat_main,
> >>> +     &ssam_node_tmp_sensors,
> >>> +     &ssam_node_hid_kip_keyboard,
> >>> +     &ssam_node_hid_kip_penstash,
> >>> +     &ssam_node_hid_kip_touchpad,
> >>> +     &ssam_node_hid_kip_fwupd,
> >>> +     &ssam_node_hid_sam_sensors,
> >>> +     &ssam_node_kip_tablet_switch,
> >>> +     NULL,
> >>> +};
> >>>
> >>>   /* -- SSAM platform/meta-hub driver. ---------------------------------------- */
> >>>
> >>> @@ -462,6 +477,8 @@ static const struct acpi_device_id ssam_platform_hub_acpi_match[] = {
> >>>   MODULE_DEVICE_TABLE(acpi, ssam_platform_hub_acpi_match);
> >>>
> >>>   static const struct of_device_id ssam_platform_hub_of_match[] __maybe_unused = {
> >>> +     /* Surface Pro 9 5G */
> >>> +     { .compatible = "microsoft,arcata", (void *)ssam_node_group_sp9_5G },
> >>>       /* Surface Laptop 7 */
> >>>       { .compatible = "microsoft,romulus13", (void *)ssam_node_group_sl7 },
> >>>       { .compatible = "microsoft,romulus15", (void *)ssam_node_group_sl7 },
> >>
> >> Thanks!
> >>
> >> Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>
> > 
> > Thanks for your review and all the work about SSAM for Surface owners!
> 
> FWIW I agree with Maximilian's remarks and I would really like to
> see these applied to clearly differentiate the x86 and ARM versions.
> 
> Normally I would pick up a patch like this which just adds hw-ids as
> a fix for 6.12-rc# and squash in the suggested changes.
> 
> But looking at the test of the series this is more 6.13 material
> since the rest is landing in 6.13, right ?
> 
> Patches for linux-next / 6.13 are managed by Ilpo this cycle.
> 
> So I'll leave it up to Ilpo if he will squash in the suggested changes
> or if he wants a new version (of just this patch, no need for a v3
> of the already applied patches).

Hi all,

I've now applied patch 3 to review-ilpo branch in pdx86 repo.

I'd appreciate if somebody confirms I got those comment edits right.

-- 
 i.

Re: [PATCH v2 3/5] platform/surface: aggregator_registry: Add Surface Pro 9 5G

[Maximilian Luz]

On 10/22/24 10:56 AM, Ilpo Järvinen wrote:

[...]

> Hi all,
> 
> I've now applied patch 3 to review-ilpo branch in pdx86 repo.
> 
> I'd appreciate if somebody confirms I got those comment edits right.
> 

Hi Ilpo,

looks good to me. Thanks for fixing this up!

Best regards,
Max

Re: [PATCH v2 3/5] platform/surface: aggregator_registry: Add Surface Pro 9 5G

[Jérôme de Bretagne]

On 10/23/24 7:25 PM, Maximilian Luz wrote:
>
> On 10/22/24 10:56 AM, Ilpo Järvinen wrote:
>
> [...]
>
> > Hi all,
> >
> > I've now applied patch 3 to review-ilpo branch in pdx86 repo.
> >
> > I'd appreciate if somebody confirms I got those comment edits right.
> >
>
> Hi Ilpo,
>
> looks good to me. Thanks for fixing this up!
>
> Best regards,
> Max

Hi Ilpo, hi Max,

It looks good to me too.

Thank you both,
Jérôme

Re: [PATCH v2 3/5] platform/surface: aggregator_registry: Add Surface Pro 9 5G

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 397 bytes --]

On Wed, 23 Oct 2024, Maximilian Luz wrote:

> On 10/22/24 10:56 AM, Ilpo Järvinen wrote:
> 
> [...]
> 
> > Hi all,
> > 
> > I've now applied patch 3 to review-ilpo branch in pdx86 repo.
> > 
> > I'd appreciate if somebody confirms I got those comment edits right.
> > 
> 
> Hi Ilpo,
> 
> looks good to me. Thanks for fixing this up!

Great! Thanks for confirmation.

-- 
 i.