arm64: dts: qcom: sc8280xp: enable WLAN and Bluetooth
Subsystems (4)
v4 cover arm64: dts: qcom: sc8280xp: enable WLAN and Bluetooth
From: Bartosz Golaszewski <[email protected]>
This correctly models the WLAN and Bluetooth modules on two boards using the sc8280xp SoC. For the sc8280xp-crd we add the PMU, wifi and bluetooth nodes with the correctly modelled wiring between them. For the X13s, we rework existing nodes so that they align with the new DT bindings contract.
v3 -> v4:
- bind bluetooth pins on X13s in patch 3/3
- only drop the regulator-always-on properties for vreg_s11b and vreg_s12b and fold this change into patch 3/3
v2 -> v3:
- move adding the bt-enable-gpios to the PMU on the CRD to patch 2/4
- add a patch removing the regulator-always-on property from regulators on X13s that no longer need it
v1 -> v2:
- fix commit message in patch 1/3
- drop drive-strength from the wlan enable pin function
- drop the calibration variant property from the wifi node of the CRD
Bartosz Golaszewski (2):
arm64: dts: qcom: sc8280xp-crd: enable bluetooth
arm64: dts: qcom: sc8280xp-x13s: model the PMU of the on-board wcn6855
Konrad Dybcio (1):
arm64: dts: qcom: sc8280xp-crd: model the PMU of the on-board wcn6855
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 168 ++++++++++++++++++ .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 100 +++++++++-- 2 files changed, 254 insertions(+), 14 deletions(-)
Signature · 1 line
2.30.2
PATCH v4 1/3 arm64: dts: qcom: sc8280xp-crd: model the PMU of the on-board wcn6855
From: Konrad Dybcio <[email protected]>
Add nodes for the WCN6855 PMU, the WLAN module and relevant regulators and pin functions to fully describe how the wifi is actually wired on this platform.
[Bartosz:
- write the commit message,
- rebase Konrad's commit,
- fix one of the supplies' name]
--- arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 110 ++++++++++++++++++++++ 1 file changed, 110 insertions(+)
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +110 -0
@@ -177,6 +177,17 @@ vreg_misc_3p3: regulator-misc-3p3 { regulator-always-on; }; + vreg_s10b: regulator-s10b { + compatible = "regulator-fixed"; + + regulator-name = "VREG_S10B"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + + regulator-always-on; + regulator-boot-on; + }; + vreg_wlan: regulator-wlan { compatible = "regulator-fixed";
@@ -260,6 +271,66 @@ usb1_sbu_mux: endpoint { }; }; }; + + wcn6855-pmu { + compatible = "qcom,wcn6855-pmu"; + + pinctrl-0 = <&wlan_en>; + pinctrl-names = "default"; + + wlan-enable-gpios = <&tlmm 134 GPIO_ACTIVE_HIGH>; + + vddio-supply = <&vreg_s10b>; + vddaon-supply = <&vreg_s12b>; + vddpmu-supply = <&vreg_s12b>; + vddrfa0p95-supply = <&vreg_s12b>; + vddrfa1p3-supply = <&vreg_s11b>; + vddrfa1p9-supply = <&vreg_s1c>; + vddpcie1p3-supply = <&vreg_s11b>; + vddpcie1p9-supply = <&vreg_s1c>; + + regulators { + vreg_pmu_rfa_cmn_0p8: ldo0 { + regulator-name = "vreg_pmu_rfa_cmn_0p8"; + }; + + vreg_pmu_aon_0p8: ldo1 { + regulator-name = "vreg_pmu_aon_0p8"; + }; + + vreg_pmu_wlcx_0p8: ldo2 { + regulator-name = "vreg_pmu_wlcx_0p8"; + }; + + vreg_pmu_wlmx_0p8: ldo3 { + regulator-name = "vreg_pmu_wlmx_0p8"; + }; + + vreg_pmu_btcmx_0p8: ldo4 { + regulator-name = "vreg_pmu_btcmx_0p8"; + }; + + vreg_pmu_pcie_1p8: ldo5 { + regulator-name = "vreg_pmu_pcie_1p8"; + }; + + vreg_pmu_pcie_0p9: ldo6 { + regulator-name = "vreg_pmu_pcie_0p9"; + }; + + vreg_pmu_rfa_0p8: ldo7 { + regulator-name = "vreg_pmu_rfa_0p8"; + }; + + vreg_pmu_rfa_1p2: ldo8 { + regulator-name = "vreg_pmu_rfa_1p2"; + }; + + vreg_pmu_rfa_1p7: ldo9 { + regulator-name = "vreg_pmu_rfa_1p7"; + }; + }; + }; }; &apps_rsc {
@@ -276,6 +347,13 @@ vreg_s11b: smps11 { regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; }; + vreg_s12b: smps12 { + regulator-name = "vreg_s12b"; + regulator-min-microvolt = <984000>; + regulator-max-microvolt = <984000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + vreg_l3b: ldo3 { regulator-name = "vreg_l3b"; regulator-min-microvolt = <1200000>;
@@ -304,6 +382,13 @@ regulators-1 { compatible = "qcom,pm8350c-rpmh-regulators"; qcom,pmic-id = "c"; + vreg_s1c: smps1 { + regulator-name = "vreg_s1c"; + regulator-min-microvolt = <1888000>; + regulator-max-microvolt = <1888000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + vreg_l1c: ldo1 { regulator-name = "vreg_l1c"; regulator-min-microvolt = <1800000>;
@@ -583,6 +668,25 @@ &pcie4_phy { status = "okay"; }; +&pcie4_port0 { + wifi@0 { + compatible = "pci17cb,1103"; + reg = <0x10000 0x0 0x0 0x0 0x0>; + + vddrfacmn-supply = <&vreg_pmu_rfa_cmn_0p8>; + vddaon-supply = <&vreg_pmu_aon_0p8>; + vddwlcx-supply = <&vreg_pmu_wlcx_0p8>; + vddwlmx-supply = <&vreg_pmu_wlmx_0p8>; + vddpcie1p8-supply = <&vreg_pmu_pcie_1p8>; + vddpcie0p9-supply = <&vreg_pmu_pcie_0p9>; + vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>; + vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>; + vddrfa1p8-supply = <&vreg_pmu_rfa_1p7>; + + qcom,ath11k-calibration-variant = "QC_8280XP_CRD"; + }; +}; + &pmc8280c_lpg { status = "okay"; };
@@ -829,6 +933,12 @@ reset-pins { }; }; + wlan_en: wlan-en-state { + pins = "gpio134"; + function = "gpio"; + bias-pull-down; + }; + nvme_reg_en: nvme-reg-en-state { pins = "gpio135"; function = "gpio";
Signature · 1 line
2.30.2
Re: [PATCH v4 1/3] arm64: dts: qcom: sc8280xp-crd: model the PMU of the on-board wcn6855
Quoted 6 lines · Bartosz Golaszewski
From: Konrad Dybcio <[email protected]>
Add nodes for the WCN6855 PMU, the WLAN module and relevant regulators and pin functions to fully describe how the wifi is actually wired on this platform.
Quoted 8 lines
Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Konrad Dybcio <[email protected]>[Bartosz:
- write the commit message,
- rebase Konrad's commit,
- fix one of the supplies' name]Co-developed-by Bartosz Golaszewski <[email protected]>Signed-off-by Bartosz Golaszewski <[email protected]>
First, make sure to CC people that help review your patches.
Quoted 12 lines
--- arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 110 ++++++++++++++++++++++ 1 file changed, 110 insertions(+)arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +0
-0@@ -177,6 +177,17 @@ vreg_misc_3p3: regulator-misc-3p3 { regulator-always-on; };
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts vreg_misc_3p3: regulator-misc-3p3 { · line 189
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +10
-0+ vreg_s10b: regulator-s10b { + compatible = "regulator-fixed"; + + regulator-name = "VREG_S10B"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + + regulator-always-on; + regulator-boot-on; + };
Again, this is not a fixed regulator. It's an supply from one of the PMICs that are already described in DT.
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +7
-0+ vreg_wlan: regulator-wlan { compatible = "regulator-fixed"; +&pcie4_port0 { + wifi@0 { + compatible = "pci17cb,1103"; + reg = <0x10000 0x0 0x0 0x0 0x0>; + + vddrfacmn-supply = <&vreg_pmu_rfa_cmn_0p8>;
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts &pcie4_phy { · line 686
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +10
-0+ vddaon-supply = <&vreg_pmu_aon_0p8>; + vddwlcx-supply = <&vreg_pmu_wlcx_0p8>; + vddwlmx-supply = <&vreg_pmu_wlmx_0p8>; + vddpcie1p8-supply = <&vreg_pmu_pcie_1p8>; + vddpcie0p9-supply = <&vreg_pmu_pcie_0p9>; + vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>; + vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>; + vddrfa1p8-supply = <&vreg_pmu_rfa_1p7>; + + qcom,ath11k-calibration-variant = "QC_8280XP_CRD";
Not "SC8280XP"?
Quoted 7 lines
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +3
-0+ }; +}; + &pmc8280c_lpg { status = "okay"; };
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts reset-pins { · line 939
Quoted 7 lines
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +4
-0@@ -829,6 +933,12 @@ reset-pins { }; }; + wlan_en: wlan-en-state { + pins = "gpio134"; + function = "gpio"; + bias-pull-down;
We don't want a pull-down enabled on an active high enable signal.
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts reset-pins { · line 940
Quoted 1 line
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +1
-0+ };
Please keep the nodes sorted by name.
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts reset-pins { · line 944
Quoted 4 lines
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +1
-0+ nvme_reg_en: nvme-reg-en-state { pins = "gpio135"; function = "gpio";
Johan
Re: [PATCH v4 1/3] arm64: dts: qcom: sc8280xp-crd: model the PMU of the on-board wcn6855
Quoted 6 lines · Johan Hovold
Review
Quoted 3 lines
(quoted diff) +2
-0+ + qcom,ath11k-calibration-variant = "QC_8280XP_CRD";Not "SC8280XP"?
This was agreed with Kalle under the previous version.
Bart
PATCH v4 2/3 arm64: dts: qcom: sc8280xp-crd: enable bluetooth
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Add the bluetooth node for sc8280xp-crd and make it consume the outputs from the PMU as per the new DT bindings contract.
--- arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 60 ++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-)
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +59 -1
@@ -20,6 +20,7 @@ aliases { i2c4 = &i2c4; i2c21 = &i2c21; serial0 = &uart17; + serial1 = &uart2; }; backlight: backlight {
@@ -275,10 +276,11 @@ usb1_sbu_mux: endpoint { wcn6855-pmu { compatible = "qcom,wcn6855-pmu"; - pinctrl-0 = <&wlan_en>; + pinctrl-0 = <&bt_en>, <&wlan_en>; pinctrl-names = "default"; wlan-enable-gpios = <&tlmm 134 GPIO_ACTIVE_HIGH>; + bt-enable-gpios = <&tlmm 133 GPIO_ACTIVE_HIGH>; vddio-supply = <&vreg_s10b>; vddaon-supply = <&vreg_s12b>;
@@ -747,6 +749,27 @@ &sdc2 { status = "okay"; }; +&uart2 { + pinctrl-0 = <&uart2_default>; + pinctrl-names = "default"; + + status = "okay"; + + bluetooth { + compatible = "qcom,wcn6855-bt"; + + vddrfacmn-supply = <&vreg_pmu_rfa_cmn_0p8>; + vddaon-supply = <&vreg_pmu_aon_0p8>; + vddwlcx-supply = <&vreg_pmu_wlcx_0p8>; + vddwlmx-supply = <&vreg_pmu_wlmx_0p8>; + vddbtcmx-supply = <&vreg_pmu_btcmx_0p8>; + vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>; + vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>; + vddrfa1p8-supply = <&vreg_pmu_rfa_1p7>; + }; +}; + + &uart17 { compatible = "qcom,geni-debug-uart";
@@ -892,6 +915,13 @@ hastings_reg_en: hastings-reg-en-state { &tlmm { gpio-reserved-ranges = <74 6>, <83 4>, <125 2>, <128 2>, <154 7>; + bt_en: bt-en-state { + pins = "gpio133"; + function = "gpio"; + drive-strength = <16>; + bias-disable; + }; + edp_reg_en: edp-reg-en-state { pins = "gpio25"; function = "gpio";
@@ -1091,6 +1121,34 @@ reset-n-pins { }; }; + 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";
Signature · 1 line
2.30.2
Re: [PATCH v4 2/3] arm64: dts: qcom: sc8280xp-crd: enable bluetooth
Quoted 19 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Add the bluetooth node for sc8280xp-crd and make it consume the outputs from the PMU as per the new DT bindings contract.
Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Bartosz Golaszewski <[email protected]>+&uart2 {
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +10
-0+ pinctrl-0 = <&uart2_default>; + pinctrl-names = "default"; + + status = "okay"; + + bluetooth { + compatible = "qcom,wcn6855-bt"; + + vddrfacmn-supply = <&vreg_pmu_rfa_cmn_0p8>; + vddaon-supply = <&vreg_pmu_aon_0p8>;
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts &sdc2 { · line 772
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +10
-0+ vddwlcx-supply = <&vreg_pmu_wlcx_0p8>; + vddwlmx-supply = <&vreg_pmu_wlmx_0p8>; + vddbtcmx-supply = <&vreg_pmu_btcmx_0p8>; + vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>; + vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>; + vddrfa1p8-supply = <&vreg_pmu_rfa_1p7>; + }; +}; + +
nit: stray newline
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts hastings_reg_en: hastings-reg-en-state { · line 921
Quoted 11 lines
&uart17 {
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +4
-0compatible = "qcom,geni-debug-uart";@@ -892,6 +915,13 @@ hastings_reg_en: hastings-reg-en-state { &tlmm { gpio-reserved-ranges = <74 6>, <83 4>, <125 2>, <128 2>, <154 7>; + bt_en: bt-en-state { + pins = "gpio133"; + function = "gpio"; + drive-strength = <16>;
Why increase the drive strength?
I see the bootfw has configured the wlan_en this way (as we discussed before) even if it should not be needed.
Please add back the drive strength in the previous patch as well even if you want to configure it the same way as the fw did for now.
Review hastings_reg_en: hastings-reg-en-state { · line 923
Quoted 2 lines
(quoted diff) +2
-0+ bias-disable; + };
Johan
PATCH v4 3/3 arm64: dts: qcom: sc8280xp-x13s: model the PMU of the on-board wcn6855
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Add a node for the PMU of the WCN6855 and rework the inputs of the wifi and bluetooth nodes to consume the PMU's outputs.
With this we can drop the regulator-always-on properties from vreg_s11b and vreg_s12b as they will now be enabled by the power sequencing driver.
--- .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 100 +++++++++++++++--- 1 file changed, 86 insertions(+), 14 deletions(-)
arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts +86 -14
@@ -400,6 +400,67 @@ usb1_sbu_mux: endpoint { }; }; }; + + wcn6855-pmu { + compatible = "qcom,wcn6855-pmu"; + + pinctrl-0 = <&bt_default>, <&wlan_en>; + pinctrl-names = "default"; + + wlan-enable-gpios = <&tlmm 134 GPIO_ACTIVE_HIGH>; + bt-enable-gpios = <&tlmm 133 GPIO_ACTIVE_HIGH>; + + vddio-supply = <&vreg_s10b>; + vddaon-supply = <&vreg_s12b>; + vddpmu-supply = <&vreg_s12b>; + vddrfa0p95-supply = <&vreg_s12b>; + vddrfa1p3-supply = <&vreg_s11b>; + vddrfa1p9-supply = <&vreg_s1c>; + vddpcie1p3-supply = <&vreg_s11b>; + vddpcie1p9-supply = <&vreg_s1c>; + + regulators { + vreg_pmu_rfa_cmn_0p8: ldo0 { + regulator-name = "vreg_pmu_rfa_cmn_0p8"; + }; + + vreg_pmu_aon_0p8: ldo1 { + regulator-name = "vreg_pmu_aon_0p8"; + }; + + vreg_pmu_wlcx_0p8: ldo2 { + regulator-name = "vreg_pmu_wlcx_0p8"; + }; + + vreg_pmu_wlmx_0p8: ldo3 { + regulator-name = "vreg_pmu_wlmx_0p8"; + }; + + vreg_pmu_btcmx_0p8: ldo4 { + regulator-name = "vreg_pmu_btcmx_0p8"; + }; + + vreg_pmu_pcie_1p8: ldo5 { + regulator-name = "vreg_pmu_pcie_1p8"; + }; + + vreg_pmu_pcie_0p9: ldo6 { + regulator-name = "vreg_pmu_pcie_0p9"; + }; + + vreg_pmu_rfa_0p8: ldo7 { + regulator-name = "vreg_pmu_rfa_0p8"; + }; + + vreg_pmu_rfa_1p2: ldo8 { + regulator-name = "vreg_pmu_rfa_1p2"; + }; + + vreg_pmu_rfa_1p7: ldo9 { + regulator-name = "vreg_pmu_rfa_1p7"; + }; + }; + }; }; &apps_rsc {
@@ -426,7 +487,6 @@ vreg_s11b: smps11 { regulator-min-microvolt = <1272000>; regulator-max-microvolt = <1272000>; regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; - regulator-always-on; }; vreg_s12b: smps12 {
@@ -434,7 +494,6 @@ vreg_s12b: smps12 { regulator-min-microvolt = <984000>; regulator-max-microvolt = <984000>; regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; - regulator-always-on; }; vreg_l1b: ldo1 {
@@ -927,6 +986,16 @@ wifi@0 { compatible = "pci17cb,1103"; reg = <0x10000 0x0 0x0 0x0 0x0>; + vddrfacmn-supply = <&vreg_pmu_rfa_cmn_0p8>; + vddaon-supply = <&vreg_pmu_aon_0p8>; + vddwlcx-supply = <&vreg_pmu_wlcx_0p8>; + vddwlmx-supply = <&vreg_pmu_wlmx_0p8>; + vddpcie1p8-supply = <&vreg_pmu_pcie_1p8>; + vddpcie0p9-supply = <&vreg_pmu_pcie_0p9>; + vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>; + vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>; + vddrfa1p8-supply = <&vreg_pmu_rfa_1p7>; + qcom,ath11k-calibration-variant = "LE_X13S"; }; };
@@ -1258,20 +1327,16 @@ &uart2 { 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>; + vddrfacmn-supply = <&vreg_pmu_rfa_cmn_0p8>; + vddaon-supply = <&vreg_pmu_aon_0p8>; + vddwlcx-supply = <&vreg_pmu_wlcx_0p8>; + vddwlmx-supply = <&vreg_pmu_wlmx_0p8>; + vddbtcmx-supply = <&vreg_pmu_btcmx_0p8>; + vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>; + vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>; + vddrfa1p8-supply = <&vreg_pmu_rfa_1p7>; max-speed = <3200000>; - - enable-gpios = <&tlmm 133 GPIO_ACTIVE_HIGH>; - swctrl-gpios = <&tlmm 132 GPIO_ACTIVE_HIGH>; - - pinctrl-0 = <&bt_default>; - pinctrl-names = "default"; }; };
@@ -1761,4 +1826,11 @@ reset-pins { bias-disable; }; }; + + wlan_en: wlan-en-state { + pins = "gpio134"; + function = "gpio"; + drive-strength = <8>; + bias-pull-down; + }; };
Signature · 1 line
2.30.2
Re: [PATCH v4 3/3] arm64: dts: qcom: sc8280xp-x13s: model the PMU of the on-board wcn6855
Quoted 11 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Add a node for the PMU of the WCN6855 and rework the inputs of the wifi and bluetooth nodes to consume the PMU's outputs.
With this we can drop the regulator-always-on properties from
vreg_s11bandvreg_s12bas they will now be enabled by the power sequencing driver.Signed-off-by Bartosz Golaszewski <[email protected]>
Without this patch I'm seeing an indefinite probe deferral with 6.12-rc1:
platform 1c00000.pcie:pcie@0:wifi@0: deferred probe pending: pci-pwrctl-pwrseq: Failed to get the power sequencer
Can you please look into that and make sure that the existing DT continues to work without such warnings.
Quoted 35 lines
--- .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 100 +++++++++++++++--- 1 file changed, 86 insertions(+), 14 deletions(-)arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts +12
-6@@ -400,6 +400,67 @@ usb1_sbu_mux: endpoint { }; }; }; + + wcn6855-pmu { + compatible = "qcom,wcn6855-pmu"; + + pinctrl-0 = <&bt_default>, <&wlan_en>; + pinctrl-names = "default"; + + wlan-enable-gpios = <&tlmm 134 GPIO_ACTIVE_HIGH>; + bt-enable-gpios = <&tlmm 133 GPIO_ACTIVE_HIGH>;@@ -1258,20 +1327,16 @@ &uart2 { 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>; + vddrfacmn-supply = <&vreg_pmu_rfa_cmn_0p8>; + vddaon-supply = <&vreg_pmu_aon_0p8>; + vddwlcx-supply = <&vreg_pmu_wlcx_0p8>;
arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts &uart2 { · line 1271
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts +5
-3+ vddwlmx-supply = <&vreg_pmu_wlmx_0p8>; + vddbtcmx-supply = <&vreg_pmu_btcmx_0p8>; + vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>; + vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>; + vddrfa1p8-supply = <&vreg_pmu_rfa_1p7>; max-speed = <3200000>; - - enable-gpios = <&tlmm 133 GPIO_ACTIVE_HIGH>; - swctrl-gpios = <&tlmm 132 GPIO_ACTIVE_HIGH>;
What about swctrl? You're just removing this pin from DT now without any comment on why you think that is the right thing to do.
Should this one also be an input to the PMU block?
Quoted 7 lines
arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts +0
-3- - pinctrl-0 = <&bt_default>; - pinctrl-names = "default"; }; };
arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts reset-pins { · line 1833
Quoted 8 lines
arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts +5
-0@@ -1761,4 +1826,11 @@ reset-pins { bias-disable; }; }; + + wlan_en: wlan-en-state { + pins = "gpio134"; + function = "gpio"; + drive-strength = <8>;
Yet another drive strength? Also from fw config?
arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts reset-pins { · line 1836
Quoted 3 lines
arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts +2
-0+ bias-pull-down; + }; };
Johan
Re: [PATCH v4 3/3] arm64: dts: qcom: sc8280xp-x13s: model the PMU of the on-board wcn6855
Quoted 14 lines · Johan Hovold
Quoted 11 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Add a node for the PMU of the WCN6855 and rework the inputs of the wifi and bluetooth nodes to consume the PMU's outputs.
With this we can drop the regulator-always-on properties from
vreg_s11bandvreg_s12bas they will now be enabled by the power sequencing driver.Signed-off-by Bartosz Golaszewski <[email protected]>
Quoted 8 lines
Without this patch I'm seeing an indefinite probe deferral with 6.12-rc1:
platform 1c00000.pcie:pcie@0:wifi@0: deferred probe pending: pci-pwrctl-pwrseq: Failed to get the power sequencer
Can you please look into that and make sure that the existing DT continues to work without such warnings.
Ah, dammit, I missed the fact that X13s already has this node defined so PCI pwrctl will consume it and try to get the power sequencer handle. I'm wondering how to tackle it though... It will most likely require some kind of a driver quirk where we check if we have the PMU node and if not, then don't try to set up power sequencing. Any other ideas?
Quoted 9 lines
arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts &uart2 { · line 1271
Quoted 3 lines
arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts +0
-3- - enable-gpios = <&tlmm 133 GPIO_ACTIVE_HIGH>; - swctrl-gpios = <&tlmm 132 GPIO_ACTIVE_HIGH>;What about swctrl? You're just removing this pin from DT now without any comment on why you think that is the right thing to do.
Should this one also be an input to the PMU block?
I recently added it to the bindings as an optional property. It's technically an output of the PMU to the host indicating the state of the clock supply to the BT module. We're not really using it but I can keep it here if you prefer.
Bart
Re: [PATCH v4 3/3] arm64: dts: qcom: sc8280xp-x13s: model the PMU of the on-board wcn6855
Quoted 11 lines · Bartosz Golaszewski
Quoted 9 lines · Johan Hovold
Without this patch I'm seeing an indefinite probe deferral with 6.12-rc1:
platform 1c00000.pcie:pcie@0:wifi@0: deferred probe pending: pci-pwrctl-pwrseq: Failed to get the power sequencer
Can you please look into that and make sure that the existing DT continues to work without such warnings.
Quoted 7 lines
Ah, dammit, I missed the fact that X13s already has this node defined so PCI pwrctl will consume it and try to get the power sequencer handle. I'm wondering how to tackle it though... It will most likely require some kind of a driver quirk where we check if we have the PMU node and if not, then don't try to set up power sequencing. Any other ideas?
This is untested but would it make sense?
(quoted diff) +0 -0
b/drivers/pci/pwrctl/pci-pwrctl-pwrseq.c
index a23a4312574b..071ee77c763d 100644
drivers/pci/pwrctl/pci-pwrctl-pwrseq.c +26 -1
@@ -3,6 +3,7 @@* Copyright (C) 2024 Linaro Ltd. */ +#include <linux/cleanup.h> #include <linux/device.h> #include <linux/mod_devicetable.h> #include <linux/module.h>
@@ -87,7 +88,31 @@ static struct platform_driver pci_pwrctl_pwrseq_driver = { }, .probe = pci_pwrctl_pwrseq_probe, }; -module_platform_driver (pci_pwrctl_pwrseq_driver); + +static int __init pci_pwrctl_pwrseq_init (void ) +{ +/* +* Old device trees for the Lenovo X13s have the "pci17cb,1103" node +* defined but don't use power sequencing yet. If we register this +* driver, it will match against this node and lead to emitting of +* a warning in the kernel log when we cannot get the power sequencing +* handle. Let's skip registering the platform driver if we're on X13s +* but don't have the PMU node. +*/ +if (of_machine_is_compatible ("lenovo,thinkpad-x13s" )) { + struct device_node *dn __free(device_node) = +of_find_compatible_node (NULL ,NULL ,"qcom,wcn6855-pmu" ); +if (!dn) +return 0 ; + } + +return platform_driver_register (&pci_pwrctl_pwrseq_driver); +} + +static void __exit pci_pwrctl_pwrseq_exit(void) +{ +platform_driver_unregister (&pci_pwrctl_pwrseq_driver); +}MODULE_AUTHOR ("Bartosz Golaszewski <[email protected]>" ); MODULE_DESCRIPTION("Generic PCI Power Control module for power
sequenced devices");
X13s is the only platform that would use one of the compatibles supported by this driver before power sequencing so it should be a one-off quirk.
Bart
Re: [PATCH v4 3/3] arm64: dts: qcom: sc8280xp-x13s: model the PMU of the on-board wcn6855
Quoted 70 lines · Bartosz Golaszewski
Quoted 11 lines · Bartosz Golaszewski
Quoted 9 lines · Johan Hovold
Without this patch I'm seeing an indefinite probe deferral with 6.12-rc1:
platform 1c00000.pcie:pcie@0:wifi@0: deferred probe pending: pci-pwrctl-pwrseq: Failed to get the power sequencer
Can you please look into that and make sure that the existing DT continues to work without such warnings.
Quoted 7 lines
Ah, dammit, I missed the fact that X13s already has this node defined so PCI pwrctl will consume it and try to get the power sequencer handle. I'm wondering how to tackle it though... It will most likely require some kind of a driver quirk where we check if we have the PMU node and if not, then don't try to set up power sequencing. Any other ideas?
This is untested but would it make sense?
drivers/pci/pwrctl/pci-pwrctl-pwrseq.c +26
-1@@ -3,6 +3,7 @@* Copyright (C) 2024 Linaro Ltd. */ +#include <linux/cleanup.h> #include <linux/device.h> #include <linux/mod_devicetable.h> #include <linux/module.h> @@ -87,7 +88,31 @@ static struct platform_driver pci_pwrctl_pwrseq_driver = { }, .probe = pci_pwrctl_pwrseq_probe, }; -module_platform_driver (pci_pwrctl_pwrseq_driver); + +static int __init pci_pwrctl_pwrseq_init (void ) +{ +/* +* Old device trees for the Lenovo X13s have the "pci17cb,1103" node +* defined but don't use power sequencing yet. If we register this +* driver, it will match against this node and lead to emitting of +* a warning in the kernel log when we cannot get the power sequencing +* handle. Let's skip registering the platform driver if we're on X13s +* but don't have the PMU node. +*/ +if (of_machine_is_compatible ("lenovo,thinkpad-x13s" )) { + struct device_node *dn __free(device_node) = +of_find_compatible_node (NULL ,NULL ,"qcom,wcn6855-pmu" ); +if (!dn) +return 0 ; + } + +return platform_driver_register (&pci_pwrctl_pwrseq_driver); +} + +static void __exit pci_pwrctl_pwrseq_exit(void) +{ +platform_driver_unregister (&pci_pwrctl_pwrseq_driver); +}MODULE_AUTHOR ("Bartosz Golaszewski <[email protected]>" ); MODULE_DESCRIPTION("Generic PCI Power Control module for powersequenced devices");
Quoted 3 lines
X13s is the only platform that would use one of the compatibles supported by this driver before power sequencing so it should be a one-off quirk.
I'm guessing the pci17cb,1107 node in x1e80100-lenovo-yoga-slim7x is also affected?
Maybe you can check if the node has one of the -supply properties and return -ENODEV from pci_pwrctl_pwrseq_probe() otherwise?
Thanks,
Stephan
Re: [PATCH v4 3/3] arm64: dts: qcom: sc8280xp-x13s: model the PMU of the on-board wcn6855
Quoted 79 lines · Stephan Gerhold
Quoted 70 lines · Bartosz Golaszewski
Quoted 11 lines · Bartosz Golaszewski
Quoted 9 lines · Johan Hovold
Without this patch I'm seeing an indefinite probe deferral with 6.12-rc1:
platform 1c00000.pcie:pcie@0:wifi@0: deferred probe pending: pci-pwrctl-pwrseq: Failed to get the power sequencer
Can you please look into that and make sure that the existing DT continues to work without such warnings.
Quoted 7 lines
Ah, dammit, I missed the fact that X13s already has this node defined so PCI pwrctl will consume it and try to get the power sequencer handle. I'm wondering how to tackle it though... It will most likely require some kind of a driver quirk where we check if we have the PMU node and if not, then don't try to set up power sequencing. Any other ideas?
This is untested but would it make sense?
drivers/pci/pwrctl/pci-pwrctl-pwrseq.c +26
-1@@ -3,6 +3,7 @@* Copyright (C) 2024 Linaro Ltd. */ +#include <linux/cleanup.h> #include <linux/device.h> #include <linux/mod_devicetable.h> #include <linux/module.h> @@ -87,7 +88,31 @@ static struct platform_driver pci_pwrctl_pwrseq_driver = { }, .probe = pci_pwrctl_pwrseq_probe, }; -module_platform_driver (pci_pwrctl_pwrseq_driver); + +static int __init pci_pwrctl_pwrseq_init (void ) +{ +/* +* Old device trees for the Lenovo X13s have the "pci17cb,1103" node +* defined but don't use power sequencing yet. If we register this +* driver, it will match against this node and lead to emitting of +* a warning in the kernel log when we cannot get the power sequencing +* handle. Let's skip registering the platform driver if we're on X13s +* but don't have the PMU node. +*/ +if (of_machine_is_compatible ("lenovo,thinkpad-x13s" )) { + struct device_node *dn __free(device_node) = +of_find_compatible_node (NULL ,NULL ,"qcom,wcn6855-pmu" ); +if (!dn) +return 0 ; + } + +return platform_driver_register (&pci_pwrctl_pwrseq_driver); +} + +static void __exit pci_pwrctl_pwrseq_exit(void) +{ +platform_driver_unregister (&pci_pwrctl_pwrseq_driver); +}MODULE_AUTHOR ("Bartosz Golaszewski <[email protected]>" ); MODULE_DESCRIPTION("Generic PCI Power Control module for powersequenced devices");
Quoted 3 lines
X13s is the only platform that would use one of the compatibles supported by this driver before power sequencing so it should be a one-off quirk.
I'm guessing the pci17cb,1107 node in x1e80100-lenovo-yoga-slim7x is also affected?
Quoted 3 lines
Maybe you can check if the node has one of the -supply properties and return
-ENODEVfrompci_pwrctl_pwrseq_probe()otherwise?
Yeah, that may be a better solution I suppose.
Bart