On 11.12.2024 4:37 PM, Pengyu Luo wrote: > Add an initial devicetree for the Huawei Matebook E Go, which is based on > sc8280xp. > > There are 3 variants, Huawei released first 2 at the same time. > Huawei Matebook E Go LTE(sc8180x), codename should be gaokun2. > Huawei Matebook E Go(sc8280xp@3.0GHz), codename is gaokun3. > Huawei Matebook E Go 2023(sc8280xp@2.69GHz). > > We add support for the latter two variants. > > This work started by Tianyu Gao and Xuecong Chen, they made the > devicetree based on existing work(i.e. the Lenovo X13s and the > Qualcomm CRD), it can boot with framebuffer. > > Original work: https://github.com/matalama80td3l/matebook-e-go-boot-works/blob/main/dts/sc8280xp-huawei-matebook-e-go.dts > > Later, I got my device, I continue their work. > > Supported features: > - adsp > - bluetooth (connect issue) > - charge (with a lower power) > - framebuffer > - gpu > - keyboard (via internal USB) > - pcie devices (wifi and nvme, no modem) > - speakers and microphones > - tablet mode switch > - touchscreen > - usb > - volume key and power key > > Some key features not supported yet: > - battery and charger information report (EC driver required) > - built-in display (cannot enable backlight yet) > - charging thresholds control (EC driver required) > - camera > - LID switch detection (EC driver required) > - USB Type-C altmode (EC driver required) > - USB Type-C PD (EC driver required) Does qcom_battmgr / pmic_glink not work? > > I have finished the EC driver, once this series are upstreamed, > I will submit a series of patches to enable EC support. > > Signed-off-by: Tianyu Gao <gty0622@xxxxxxxxx> > Signed-off-by: Xuecong Chen <chenxuecong2009@xxxxxxxxxxx> Your commit message suggests Co-developed-by: tags would also be fitting here [...] > + chosen { > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + framebuffer0: framebuffer@c6200000 { > + compatible = "simple-framebuffer"; > + reg = <0x0 0xC6200000 0x0 0x02400000>; > + width = <1600>; > + height = <2560>; > + stride = <(1600 * 4)>; > + format = "a8r8g8b8"; > + }; > + }; This should be redundant, as you should have efifb [...] > + > + wcd938x: audio-codec { > + compatible = "qcom,wcd9380-codec"; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&wcd_default>; Please follow this order throughout the file: property-n property-names [...] > + > + 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>; > + }; > + > + gpio-keys { > + compatible = "gpio-keys"; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&mode_pin_active>, <&vol_up_n>; > + > + switch-mode { > + gpios = <&tlmm 26 GPIO_ACTIVE_HIGH>; This could use a label too - "Tablet Mode Switch", perhaps? > + linux,input-type = <EV_SW>; > + linux,code = <SW_TABLET_MODE>; > + debounce-interval = <10>; > + wakeup-source; > + }; > + > + key-vol-up { Please place this node above the switch-mode one to preserve alphabetical ordering (see [1]) > + label = "Volume Up"; > + gpios = <&pmc8280_1_gpios 6 GPIO_ACTIVE_LOW>; > + linux,code = <KEY_VOLUMEUP>; > + debounce-interval = <15>; > + linux,can-disable; > + wakeup-source; > + }; > + Stray newline [...] > + > + /* s2c, s3c */ Please remove such comments [...] > + > + /* /lib/firmware/ath11k/WCN6855/hw2.1/board-2.bin > + * there is no calibrate data for huawei, > + * but they have the same subsystem-device id > + */ > + qcom,ath11k-calibration-variant = "LE_X13S"; Oh, this can be taken care of! See [2], [3]. [...] > + > +&sound { > + compatible = "qcom,sc8280xp-sndcard"; > + model = "SC8280XP-HUAWEI-MATEBOOKEGO"; > + audio-routing = > + "SpkrLeft IN", "WSA_SPK1 OUT", Please remove the line break and > + "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 { Please insert a newline between the last property and subnodes. Otherwise looks fairly good! Konrad [1] https://docs.kernel.org/devicetree/bindings/dts-coding-style.html [2] https://lore.kernel.org/ath11k/ZwR1hu-B0bGe4zG7@localhost.localdomain/ [3] https://git.codelinaro.org/clo/ath-firmware/ath11k-firmware
