diff mbox series

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

Message ID 20250211-x1e80100-pwrseq-qcp-v2-1-c4349ca974ab@linaro.org (mailing list archive)
State Superseded
Headers show
Series [v2] arm64: dts: qcom: x1e80100-qcp: Add WiFi/BT pwrseq | expand

Commit Message

Stephan Gerhold Feb. 11, 2025, 3:01 p.m. UTC 
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(+)


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

Best regards,

Comments

Konrad Dybcio Feb. 11, 2025, 3:13 p.m. UTC | #1 
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
Johan Hovold Feb. 11, 2025, 3:49 p.m. UTC | #2 
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
Bartosz Golaszewski Feb. 11, 2025, 5:51 p.m. UTC | #3 
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/
Stephan Gerhold Feb. 11, 2025, 6:21 p.m. UTC | #4 
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/
Bartosz Golaszewski Feb. 11, 2025, 7:34 p.m. UTC | #5 
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
diff mbox series

Patch

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 {