| Message ID | 20240813190639.154983-2-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: qcom: sc8280xp: enable WLAN and Bluetooth | expand |
On Tue, Aug 13, 2024 at 09:06:36PM +0200, Bartosz Golaszewski wrote: > From: Konrad Dybcio <konradybcio@kernel.org> > > Add nodes for the WCN6855 PMU, the WLAN module and relevant regulators > and pin functions to enable wifi support on sc8280xp-crd. What are you guys smoking? The Wi-Fi has been enabled on the CRD since 6.2 and commit d907fe5acbf1 ("arm64: dts: qcom: sc8280xp-crd: enable WiFi controller"). > Signed-off-by: Konrad Dybcio <konradybcio@kernel.org> > [Bartosz: > - write the commit message, > - rebase Konrad's commit, > - fix one of the supplies' name] > Co-developed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- > arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 112 ++++++++++++++++++++++ > 1 file changed, 112 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts > index 6020582b0a59..57efeefbc89e 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts > @@ -177,6 +177,17 @@ vreg_misc_3p3: regulator-misc-3p3 { > regulator-always-on; > }; > > + vreg_s10b: regulator-s10b { > + compatible = "regulator-fixed"; I don't think this is a fixed regulator. > + > + 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"; > > +&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 = "LE_X13S"; This is not the right calibration variant either. In fact, Qualcomm has not yet released any calibration data for this CRD yet: https://bugzilla.kernel.org/show_bug.cgi?id=216036 I use a patch like this locally, but we shouldn't merge this upstream. > + wlan_en: wlan-en-state { > + pins = "gpio134"; > + function = "gpio"; > + drive-strength = <8>; Why increase the drive strength? > + bias-pull-down; > + }; Johan
On 22.08.2024 4:56 PM, Johan Hovold wrote: > On Tue, Aug 13, 2024 at 09:06:36PM +0200, Bartosz Golaszewski wrote: >> From: Konrad Dybcio <konradybcio@kernel.org> >> >> Add nodes for the WCN6855 PMU, the WLAN module and relevant regulators >> and pin functions to enable wifi support on sc8280xp-crd. > > What are you guys smoking? The Wi-Fi has been enabled on the CRD since > 6.2 and commit d907fe5acbf1 ("arm64: dts: qcom: sc8280xp-crd: enable > WiFi controller"). Smells like copypasta.. > >> Signed-off-by: Konrad Dybcio <konradybcio@kernel.org> >> [Bartosz: >> - write the commit message, >> - rebase Konrad's commit, >> - fix one of the supplies' name] >> Co-developed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >> --- >> arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 112 ++++++++++++++++++++++ >> 1 file changed, 112 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts >> index 6020582b0a59..57efeefbc89e 100644 >> --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts >> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts >> @@ -177,6 +177,17 @@ vreg_misc_3p3: regulator-misc-3p3 { >> regulator-always-on; >> }; >> >> + vreg_s10b: regulator-s10b { >> + compatible = "regulator-fixed"; > > I don't think this is a fixed regulator. It effectively is > >> + >> + 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"; >> > >> +&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 = "LE_X13S"; > > This is not the right calibration variant either. In fact, Qualcomm has > not yet released any calibration data for this CRD yet: > > https://bugzilla.kernel.org/show_bug.cgi?id=216036 > > I use a patch like this locally, but we shouldn't merge this upstream. Agreed > >> + wlan_en: wlan-en-state { >> + pins = "gpio134"; >> + function = "gpio"; >> + drive-strength = <8>; > > Why increase the drive strength? This is what's used on Windows, for lack of a better idea, not sure if this is actually necessary Konrad
On Tue, Aug 27, 2024 at 01:26:40PM +0200, Konrad Dybcio wrote: > On 22.08.2024 4:56 PM, Johan Hovold wrote: > > On Tue, Aug 13, 2024 at 09:06:36PM +0200, Bartosz Golaszewski wrote: > >> From: Konrad Dybcio <konradybcio@kernel.org> > >> + vreg_s10b: regulator-s10b { > >> + compatible = "regulator-fixed"; > > > > I don't think this is a fixed regulator. > > It effectively is If it's an output of one of the pmic as the name suggests and as it is on the X13s, I think it should be described as such. > >> + > >> + regulator-name = "VREG_S10B"; > >> + regulator-min-microvolt = <1800000>; > >> + regulator-max-microvolt = <1800000>; > >> + > >> + regulator-always-on; > >> + regulator-boot-on; > >> + }; > >> + wlan_en: wlan-en-state { > >> + pins = "gpio134"; > >> + function = "gpio"; > >> + drive-strength = <8>; > > > > Why increase the drive strength? > > This is what's used on Windows, for lack of a better idea, not sure > if this is actually necessary If this indeed is what the fw configures it as then I guess it's fine to keep as-is for now. Johan
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts index 6020582b0a59..57efeefbc89e 100644 --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts @@ -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,67 @@ 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>; + 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 { @@ -276,6 +348,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 +383,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 +669,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 = "LE_X13S"; + }; +}; + &pmc8280c_lpg { status = "okay"; }; @@ -829,6 +934,13 @@ reset-pins { }; }; + wlan_en: wlan-en-state { + pins = "gpio134"; + function = "gpio"; + drive-strength = <8>; + bias-pull-down; + }; + nvme_reg_en: nvme-reg-en-state { pins = "gpio135"; function = "gpio";