On 4.09.2024 12:42 AM, 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), > external display via DisplayPort over Type-C (only the bottom USB > Type-C port is working so far, corresponding to the usb1 / dp1 nodes), > charging, the Surface Aggregator Module (SAM) to get keyboard and > touchpad working with the 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. > > Signed-off-by: Jérôme de Bretagne <jerome.debretagne@xxxxxxxxx> > --- [...] > + > + pmic-glink { > + compatible = "qcom,sc8280xp-pmic-glink", "qcom,pmic-glink"; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + 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>; > + }; > + }; > + }; > + }; > + > + connector@1 { > + compatible = "usb-c-connector"; > + reg = <1>; > + power-role = "dual"; > + data-role = "dual"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@0 { Missing newline above Could you add a comment explaining which port is which (physically)? See x1e device trees for an example [...] > + usb0-sbu-mux { > + compatible = "pericom,pi3usb102", "gpio-sbu-mux"; > + > + enable-gpios = <&tlmm 101 GPIO_ACTIVE_LOW>; > + select-gpios = <&tlmm 164 GPIO_ACTIVE_HIGH>; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&usb0_sbu_default>; property-n property-names please, all throughout the file [...] > +&pcie4_port0 { > + wifi@0 { > + compatible = "pci17cb,1103"; > + reg = <0x10000 0x0 0x0 0x0 0x0>; > + > + qcom,ath11k-calibration-variant = "LE_X13S"; This is most likely not a x13s, please add a new calibration variant [...] > +&sound { > + compatible = "qcom,sc8280xp-sndcard"; > + model = "SC8280XP-MICROSOFT-SURFACE-PRO-9-5G"; > + audio-routing = > + "SpkrLeft IN", "WSA_SPK1 OUT", Drop the newline after = > + "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 { Add a newline between the last property and subnodes [...] > + > +&uart18 { > + status = "okay"; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&uart18_state>; > + > + surface-aggregator { > + compatible = "surface,aggregator"; This wasn't tested against the upstream driver (see Documentation/devicetree/bindings/platform/microsoft,surface-sam.yaml) [...] > + > +&tlmm { > + gpio-reserved-ranges = <70 2>, <74 6>, <125 2>, <128 2>, <154 4>; Please check that all of these entries are necessary (if they aren't, removing them will break booting) Konrad