[PATCH v2] arm64: dts: qcom: x1e80100-qcp: Add WiFi/BT pwrseq

Stephan Gerhold posted 1 patch 12 months ago
There is a newer version of this series
arch/arm64/boot/dts/qcom/x1e80100-qcp.dts | 144 ++++++++++++++++++++++++++++++
1 file changed, 144 insertions(+)
[PATCH v2] arm64: dts: qcom: x1e80100-qcp: Add WiFi/BT pwrseq
Posted by Stephan Gerhold 12 months ago
Add the WiFi/BT nodes for QCP and describe the regulators for the WCN7850
combo chip using the new power sequencing bindings. All voltages are
derived from chained fixed regulators controlled using a single GPIO.

The same setup also works for CRD (and likely most of the other X1E80100
laptops). However, unlike the QCP they use soldered or removable M.2 cards
supplied by a single 3.3V fixed regulator. The other necessary voltages are
then derived inside the M.2 card. Describing this properly requires
new bindings, so this commit only adds QCP for now.

Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
Changes in v2:
- Rebase on qcom for-next, patch 1-2 were applied already
- Mention dummy regulator warning
- Link to v1: https://lore.kernel.org/r/20241007-x1e80100-pwrseq-qcp-v1-0-f7166510ab17@linaro.org
---
The Linux driver currently warns about a missing regulator supply:

  pwrseq-qcom_wcn wcn7850-pmu: supply vddio1p2 not found, using dummy regulator

This supply exists on the WCN7850 chip, but nothing is connected there on
the QCP. Discussion is still open how to hide this warning in the driver,
but since the DT is correct and the same setup is already used on SM8550
upstream, this shouldn't block this patch.
---
 arch/arm64/boot/dts/qcom/x1e80100-qcp.dts | 144 ++++++++++++++++++++++++++++++
 1 file changed, 144 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
index ec594628304a9ab9fe2dd7cdc0467953cd82dc1f..4240c608a4087d8173c1e92565e3c729fd15bed6 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
@@ -17,6 +17,7 @@ / {
 
 	aliases {
 		serial0 = &uart21;
+		serial1 = &uart14;
 	};
 
 	wcd938x: audio-codec {
@@ -337,6 +338,101 @@ usb_1_ss2_sbu_mux: endpoint {
 			};
 		};
 	};
+
+	vreg_wcn_3p3: regulator-wcn-3p3 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VREG_WCN_3P3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&tlmm 214 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-0 = <&wcn_sw_en>;
+		pinctrl-names = "default";
+
+		regulator-boot-on;
+	};
+
+	vreg_wcn_0p95: regulator-wcn-0p95 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VREG_WCN_0P95";
+		regulator-min-microvolt = <950000>;
+		regulator-max-microvolt = <950000>;
+
+		vin-supply = <&vreg_wcn_3p3>;
+	};
+
+	vreg_wcn_1p9: regulator-wcn-1p9 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VREG_WCN_1P9";
+		regulator-min-microvolt = <1900000>;
+		regulator-max-microvolt = <1900000>;
+
+		vin-supply = <&vreg_wcn_3p3>;
+	};
+
+	wcn7850-pmu {
+		compatible = "qcom,wcn7850-pmu";
+
+		vdd-supply = <&vreg_wcn_0p95>;
+		vddio-supply = <&vreg_l15b_1p8>;
+		vddaon-supply = <&vreg_wcn_0p95>;
+		vdddig-supply = <&vreg_wcn_0p95>;
+		vddrfa1p2-supply = <&vreg_wcn_1p9>;
+		vddrfa1p8-supply = <&vreg_wcn_1p9>;
+
+		wlan-enable-gpios = <&tlmm 117 GPIO_ACTIVE_HIGH>;
+		bt-enable-gpios = <&tlmm 116 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-0 = <&wcn_wlan_bt_en>;
+		pinctrl-names = "default";
+
+		regulators {
+			vreg_pmu_rfa_cmn: ldo0 {
+				regulator-name = "vreg_pmu_rfa_cmn";
+			};
+
+			vreg_pmu_aon_0p59: ldo1 {
+				regulator-name = "vreg_pmu_aon_0p59";
+			};
+
+			vreg_pmu_wlcx_0p8: ldo2 {
+				regulator-name = "vreg_pmu_wlcx_0p8";
+			};
+
+			vreg_pmu_wlmx_0p85: ldo3 {
+				regulator-name = "vreg_pmu_wlmx_0p85";
+			};
+
+			vreg_pmu_btcmx_0p85: ldo4 {
+				regulator-name = "vreg_pmu_btcmx_0p85";
+			};
+
+			vreg_pmu_rfa_0p8: ldo5 {
+				regulator-name = "vreg_pmu_rfa_0p8";
+			};
+
+			vreg_pmu_rfa_1p2: ldo6 {
+				regulator-name = "vreg_pmu_rfa_1p2";
+			};
+
+			vreg_pmu_rfa_1p8: ldo7 {
+				regulator-name = "vreg_pmu_rfa_1p8";
+			};
+
+			vreg_pmu_pcie_0p9: ldo8 {
+				regulator-name = "vreg_pmu_pcie_0p9";
+			};
+
+			vreg_pmu_pcie_1p8: ldo9 {
+				regulator-name = "vreg_pmu_pcie_1p8";
+			};
+		};
+	};
 };
 
 &apps_rsc {
@@ -825,6 +921,23 @@ &pcie4_phy {
 	status = "okay";
 };
 
+&pcie4_port0 {
+	wifi@0 {
+		compatible = "pci17cb,1107";
+		reg = <0x10000 0x0 0x0 0x0 0x0>;
+
+		vddaon-supply = <&vreg_pmu_aon_0p59>;
+		vddwlcx-supply = <&vreg_pmu_wlcx_0p8>;
+		vddwlmx-supply = <&vreg_pmu_wlmx_0p85>;
+		vddrfacmn-supply = <&vreg_pmu_rfa_cmn>;
+		vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>;
+		vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>;
+		vddrfa1p8-supply = <&vreg_pmu_rfa_1p8>;
+		vddpcie0p9-supply = <&vreg_pmu_pcie_0p9>;
+		vddpcie1p8-supply = <&vreg_pmu_pcie_1p8>;
+	};
+};
+
 &pcie6a {
 	perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
@@ -1135,6 +1248,37 @@ wcd_default: wcd-reset-n-active-state {
 		bias-disable;
 		output-low;
 	};
+
+	wcn_wlan_bt_en: wcn-wlan-bt-en-state {
+		pins = "gpio116", "gpio117";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	wcn_sw_en: wcn-sw-en-state {
+		pins = "gpio214";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+};
+
+&uart14 {
+	status = "okay";
+
+	bluetooth {
+		compatible = "qcom,wcn7850-bt";
+		max-speed = <3200000>;
+
+		vddaon-supply = <&vreg_pmu_aon_0p59>;
+		vddwlcx-supply = <&vreg_pmu_wlcx_0p8>;
+		vddwlmx-supply = <&vreg_pmu_wlmx_0p85>;
+		vddrfacmn-supply = <&vreg_pmu_rfa_cmn>;
+		vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>;
+		vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>;
+		vddrfa1p8-supply = <&vreg_pmu_rfa_1p8>;
+	};
 };
 
 &uart21 {

---
base-commit: 18f0a0ac2430a17949aa3b393ee22f7ad0de37e0
change-id: 20241007-x1e80100-pwrseq-qcp-2bf898c60353

Best regards,
-- 
Stephan Gerhold <stephan.gerhold@linaro.org>
Re: [PATCH v2] arm64: dts: qcom: x1e80100-qcp: Add WiFi/BT pwrseq
Posted by Konrad Dybcio 12 months ago
On 11.02.2025 4:01 PM, Stephan Gerhold wrote:
> Add the WiFi/BT nodes for QCP and describe the regulators for the WCN7850
> combo chip using the new power sequencing bindings. All voltages are
> derived from chained fixed regulators controlled using a single GPIO.
> 
> The same setup also works for CRD (and likely most of the other X1E80100
> laptops). However, unlike the QCP they use soldered or removable M.2 cards
> supplied by a single 3.3V fixed regulator. The other necessary voltages are
> then derived inside the M.2 card. Describing this properly requires
> new bindings, so this commit only adds QCP for now.
> 
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
> Changes in v2:
> - Rebase on qcom for-next, patch 1-2 were applied already
> - Mention dummy regulator warning
> - Link to v1: https://lore.kernel.org/r/20241007-x1e80100-pwrseq-qcp-v1-0-f7166510ab17@linaro.org
> ---
> The Linux driver currently warns about a missing regulator supply:
> 
>   pwrseq-qcom_wcn wcn7850-pmu: supply vddio1p2 not found, using dummy regulator
> 
> This supply exists on the WCN7850 chip, but nothing is connected there on
> the QCP. Discussion is still open how to hide this warning in the driver,
> but since the DT is correct and the same setup is already used on SM8550
> upstream, this shouldn't block this patch.
> ---

Even with this warning:

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad
Re: [PATCH v2] arm64: dts: qcom: x1e80100-qcp: Add WiFi/BT pwrseq
Posted by Johan Hovold 12 months ago
On Tue, Feb 11, 2025 at 04:01:56PM +0100, Stephan Gerhold wrote:
> Add the WiFi/BT nodes for QCP and describe the regulators for the WCN7850
> combo chip using the new power sequencing bindings. All voltages are
> derived from chained fixed regulators controlled using a single GPIO.
> 
> The same setup also works for CRD (and likely most of the other X1E80100
> laptops). However, unlike the QCP they use soldered or removable M.2 cards
> supplied by a single 3.3V fixed regulator. The other necessary voltages are
> then derived inside the M.2 card. Describing this properly requires
> new bindings, so this commit only adds QCP for now.
> 
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
> Changes in v2:
> - Rebase on qcom for-next, patch 1-2 were applied already
> - Mention dummy regulator warning
> - Link to v1: https://lore.kernel.org/r/20241007-x1e80100-pwrseq-qcp-v1-0-f7166510ab17@linaro.org
> ---
> The Linux driver currently warns about a missing regulator supply:
> 
>   pwrseq-qcom_wcn wcn7850-pmu: supply vddio1p2 not found, using dummy regulator
>
> This supply exists on the WCN7850 chip, but nothing is connected there on
> the QCP. Discussion is still open how to hide this warning in the driver,
> but since the DT is correct and the same setup is already used on SM8550
> upstream, this shouldn't block this patch.

I thought Bartosz was gonna fix his driver...

> @@ -337,6 +338,101 @@ usb_1_ss2_sbu_mux: endpoint {
>  			};
>  		};
>  	};
> +
> +	vreg_wcn_3p3: regulator-wcn-3p3 {

Please keep the nodes sorted by name. These should all go above the usb
muxes.

Johan
Re: [PATCH v2] arm64: dts: qcom: x1e80100-qcp: Add WiFi/BT pwrseq
Posted by Bartosz Golaszewski 12 months ago
On Tue, Feb 11, 2025 at 4:49 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Tue, Feb 11, 2025 at 04:01:56PM +0100, Stephan Gerhold wrote:
> > Add the WiFi/BT nodes for QCP and describe the regulators for the WCN7850
> > combo chip using the new power sequencing bindings. All voltages are
> > derived from chained fixed regulators controlled using a single GPIO.
> >
> > The same setup also works for CRD (and likely most of the other X1E80100
> > laptops). However, unlike the QCP they use soldered or removable M.2 cards
> > supplied by a single 3.3V fixed regulator. The other necessary voltages are
> > then derived inside the M.2 card. Describing this properly requires
> > new bindings, so this commit only adds QCP for now.
> >
> > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > ---
> > Changes in v2:
> > - Rebase on qcom for-next, patch 1-2 were applied already
> > - Mention dummy regulator warning
> > - Link to v1: https://lore.kernel.org/r/20241007-x1e80100-pwrseq-qcp-v1-0-f7166510ab17@linaro.org
> > ---
> > The Linux driver currently warns about a missing regulator supply:
> >
> >   pwrseq-qcom_wcn wcn7850-pmu: supply vddio1p2 not found, using dummy regulator
> >
> > This supply exists on the WCN7850 chip, but nothing is connected there on
> > the QCP. Discussion is still open how to hide this warning in the driver,
> > but since the DT is correct and the same setup is already used on SM8550
> > upstream, this shouldn't block this patch.
>
> I thought Bartosz was gonna fix his driver...
>

This is not the same issue. The one you're thinking about[1] was fixed
by commit ad783b9f8e78 ("PCI/pwrctl: Abandon QCom WCN probe on
pre-pwrseq device-trees").

This warning comes from the PMU driver, not the PCI pwrctrl one for
the WLAN module. One solution would be to make this supply optional in
bindings and use regulator_get_optional for the ones we know may be
unconnected. Does it sound correct?

Bartosz

[1] https://lore.kernel.org/all/Zv565olMDDGHyYVt@hovoldconsulting.com/
Re: [PATCH v2] arm64: dts: qcom: x1e80100-qcp: Add WiFi/BT pwrseq
Posted by Stephan Gerhold 12 months ago
On Tue, Feb 11, 2025 at 06:51:02PM +0100, Bartosz Golaszewski wrote:
> On Tue, Feb 11, 2025 at 4:49 PM Johan Hovold <johan@kernel.org> wrote:
> >
> > On Tue, Feb 11, 2025 at 04:01:56PM +0100, Stephan Gerhold wrote:
> > > Add the WiFi/BT nodes for QCP and describe the regulators for the WCN7850
> > > combo chip using the new power sequencing bindings. All voltages are
> > > derived from chained fixed regulators controlled using a single GPIO.
> > >
> > > The same setup also works for CRD (and likely most of the other X1E80100
> > > laptops). However, unlike the QCP they use soldered or removable M.2 cards
> > > supplied by a single 3.3V fixed regulator. The other necessary voltages are
> > > then derived inside the M.2 card. Describing this properly requires
> > > new bindings, so this commit only adds QCP for now.
> > >
> > > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > > ---
> > > Changes in v2:
> > > - Rebase on qcom for-next, patch 1-2 were applied already
> > > - Mention dummy regulator warning
> > > - Link to v1: https://lore.kernel.org/r/20241007-x1e80100-pwrseq-qcp-v1-0-f7166510ab17@linaro.org
> > > ---
> > > The Linux driver currently warns about a missing regulator supply:
> > >
> > >   pwrseq-qcom_wcn wcn7850-pmu: supply vddio1p2 not found, using dummy regulator
> > >
> > > This supply exists on the WCN7850 chip, but nothing is connected there on
> > > the QCP. Discussion is still open how to hide this warning in the driver,
> > > but since the DT is correct and the same setup is already used on SM8550
> > > upstream, this shouldn't block this patch.
> >
> > I thought Bartosz was gonna fix his driver...
> >
> 
> This is not the same issue. The one you're thinking about[1] was fixed
> by commit ad783b9f8e78 ("PCI/pwrctl: Abandon QCom WCN probe on
> pre-pwrseq device-trees").
> 
> This warning comes from the PMU driver, not the PCI pwrctrl one for
> the WLAN module. One solution would be to make this supply optional in
> bindings and use regulator_get_optional for the ones we know may be
> unconnected. Does it sound correct?
> 

The supply is optional already in the bindings. It's not optional in the
driver though, because that one uses the bulk regulator API and that
currently provides no way to mark an individual regulator as optional.

We did discuss this on v1 of this patch. I think you did not get back to
Mark's last message yet [2]. :-)

Thanks,
Stephan

[2]: https://lore.kernel.org/linux-arm-msm/f125c7d5-5f85-4ff6-999b-2098ff3103f9@sirena.org.uk/
Re: [PATCH v2] arm64: dts: qcom: x1e80100-qcp: Add WiFi/BT pwrseq
Posted by Bartosz Golaszewski 12 months ago
On Tue, Feb 11, 2025 at 7:21 PM Stephan Gerhold
<stephan.gerhold@linaro.org> wrote:
>
> On Tue, Feb 11, 2025 at 06:51:02PM +0100, Bartosz Golaszewski wrote:
> > On Tue, Feb 11, 2025 at 4:49 PM Johan Hovold <johan@kernel.org> wrote:
> > >
> > > On Tue, Feb 11, 2025 at 04:01:56PM +0100, Stephan Gerhold wrote:
> > > > Add the WiFi/BT nodes for QCP and describe the regulators for the WCN7850
> > > > combo chip using the new power sequencing bindings. All voltages are
> > > > derived from chained fixed regulators controlled using a single GPIO.
> > > >
> > > > The same setup also works for CRD (and likely most of the other X1E80100
> > > > laptops). However, unlike the QCP they use soldered or removable M.2 cards
> > > > supplied by a single 3.3V fixed regulator. The other necessary voltages are
> > > > then derived inside the M.2 card. Describing this properly requires
> > > > new bindings, so this commit only adds QCP for now.
> > > >
> > > > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > > > ---
> > > > Changes in v2:
> > > > - Rebase on qcom for-next, patch 1-2 were applied already
> > > > - Mention dummy regulator warning
> > > > - Link to v1: https://lore.kernel.org/r/20241007-x1e80100-pwrseq-qcp-v1-0-f7166510ab17@linaro.org
> > > > ---
> > > > The Linux driver currently warns about a missing regulator supply:
> > > >
> > > >   pwrseq-qcom_wcn wcn7850-pmu: supply vddio1p2 not found, using dummy regulator
> > > >
> > > > This supply exists on the WCN7850 chip, but nothing is connected there on
> > > > the QCP. Discussion is still open how to hide this warning in the driver,
> > > > but since the DT is correct and the same setup is already used on SM8550
> > > > upstream, this shouldn't block this patch.
> > >
> > > I thought Bartosz was gonna fix his driver...
> > >
> >
> > This is not the same issue. The one you're thinking about[1] was fixed
> > by commit ad783b9f8e78 ("PCI/pwrctl: Abandon QCom WCN probe on
> > pre-pwrseq device-trees").
> >
> > This warning comes from the PMU driver, not the PCI pwrctrl one for
> > the WLAN module. One solution would be to make this supply optional in
> > bindings and use regulator_get_optional for the ones we know may be
> > unconnected. Does it sound correct?
> >
>
> The supply is optional already in the bindings. It's not optional in the
> driver though, because that one uses the bulk regulator API and that
> currently provides no way to mark an individual regulator as optional.
>
> We did discuss this on v1 of this patch. I think you did not get back to
> Mark's last message yet [2]. :-)
>
> Thanks,
> Stephan
>
> [2]: https://lore.kernel.org/linux-arm-msm/f125c7d5-5f85-4ff6-999b-2098ff3103f9@sirena.org.uk/

Indeed, thanks for reminding me. I'll respond tomorrow.

Bartosz