diff mbox series

arm64: dts: qcom: sm8550-hdk: add the Wifi node

Message ID 20240702091655.278974-1-amit.pundir@linaro.org (mailing list archive)
State Changes Requested
Headers show
Series arm64: dts: qcom: sm8550-hdk: add the Wifi node | expand

Commit Message

Amit Pundir July 2, 2024, 9:16 a.m. UTC
Describe the ath12k WLAN on-board the WCN7850 module present on the
board.

Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
---
Kanged verbatim from 490812872449 ("arm64: dts: qcom: sm8550-qrd: add the Wifi node").

 arch/arm64/boot/dts/qcom/sm8550-hdk.dts | 97 +++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

Comments

Krzysztof Kozlowski July 2, 2024, 10:42 a.m. UTC | #1
On 02/07/2024 11:16, Amit Pundir wrote:
> Describe the ath12k WLAN on-board the WCN7850 module present on the
> board.
> 
> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> ---
> Kanged verbatim from 490812872449 ("arm64: dts: qcom: sm8550-qrd: add the Wifi node").
> 
>  arch/arm64/boot/dts/qcom/sm8550-hdk.dts | 97 +++++++++++++++++++++++++
>  1 file changed, 97 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> index 12d60a0ee095..c453d081a2df 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> @@ -279,6 +279,68 @@ platform {
>  			};
>  		};
>  	};
> +
> +	wcn7850-pmu {
> +		compatible = "qcom,wcn7850-pmu";
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&wlan_en>, <&pmk8550_sleep_clk>;
> +
> +		wlan-enable-gpios = <&tlmm 80 GPIO_ACTIVE_HIGH>;
> +		/*
> +		 * TODO Add bt-enable-gpios once the Bluetooth driver is
> +		 * converted to using the power sequencer.

I don't understand why hardware description should depend on the driver.
Either you have this GPIO or not. If you have it, what does it matter if
there is no driver who can play with it?

Best regards,
Krzysztof
Dmitry Baryshkov July 2, 2024, 11:10 a.m. UTC | #2
On Tue, Jul 02, 2024 at 12:42:02PM GMT, Krzysztof Kozlowski wrote:
> On 02/07/2024 11:16, Amit Pundir wrote:
> > Describe the ath12k WLAN on-board the WCN7850 module present on the
> > board.
> > 
> > Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> > ---
> > Kanged verbatim from 490812872449 ("arm64: dts: qcom: sm8550-qrd: add the Wifi node").
> > 
> >  arch/arm64/boot/dts/qcom/sm8550-hdk.dts | 97 +++++++++++++++++++++++++
> >  1 file changed, 97 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> > index 12d60a0ee095..c453d081a2df 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> > +++ b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> > @@ -279,6 +279,68 @@ platform {
> >  			};
> >  		};
> >  	};
> > +
> > +	wcn7850-pmu {
> > +		compatible = "qcom,wcn7850-pmu";
> > +
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&wlan_en>, <&pmk8550_sleep_clk>;
> > +
> > +		wlan-enable-gpios = <&tlmm 80 GPIO_ACTIVE_HIGH>;
> > +		/*
> > +		 * TODO Add bt-enable-gpios once the Bluetooth driver is
> > +		 * converted to using the power sequencer.
> 
> I don't understand why hardware description should depend on the driver.
> Either you have this GPIO or not. If you have it, what does it matter if
> there is no driver who can play with it?

Then there is a conflict between BT and PMU, which both will try to
access the gpio (or at least the pinctrl).
Bartosz Golaszewski July 2, 2024, 11:13 a.m. UTC | #3
On Tue, Jul 2, 2024 at 1:10 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Tue, Jul 02, 2024 at 12:42:02PM GMT, Krzysztof Kozlowski wrote:
> > On 02/07/2024 11:16, Amit Pundir wrote:
> > > Describe the ath12k WLAN on-board the WCN7850 module present on the
> > > board.
> > >
> > > Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> > > ---
> > > Kanged verbatim from 490812872449 ("arm64: dts: qcom: sm8550-qrd: add the Wifi node").
> > >
> > >  arch/arm64/boot/dts/qcom/sm8550-hdk.dts | 97 +++++++++++++++++++++++++
> > >  1 file changed, 97 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> > > index 12d60a0ee095..c453d081a2df 100644
> > > --- a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> > > +++ b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> > > @@ -279,6 +279,68 @@ platform {
> > >                     };
> > >             };
> > >     };
> > > +
> > > +   wcn7850-pmu {
> > > +           compatible = "qcom,wcn7850-pmu";
> > > +
> > > +           pinctrl-names = "default";
> > > +           pinctrl-0 = <&wlan_en>, <&pmk8550_sleep_clk>;
> > > +
> > > +           wlan-enable-gpios = <&tlmm 80 GPIO_ACTIVE_HIGH>;
> > > +           /*
> > > +            * TODO Add bt-enable-gpios once the Bluetooth driver is
> > > +            * converted to using the power sequencer.
> >
> > I don't understand why hardware description should depend on the driver.
> > Either you have this GPIO or not. If you have it, what does it matter if
> > there is no driver who can play with it?
>
> Then there is a conflict between BT and PMU, which both will try to
> access the gpio (or at least the pinctrl).

Ah, so this slipped through the cracks. Amit merely copied it from existing dts.

Yes, there's a conflict but Krzysztof is right - we should handle this
in the driver, not in dts.

However for that we need a patch for the PMU pwrseq driver first. Let
me cook something up.

Bart
Dmitry Baryshkov July 2, 2024, 11:32 a.m. UTC | #4
On Tue, Jul 02, 2024 at 01:13:09PM GMT, Bartosz Golaszewski wrote:
> On Tue, Jul 2, 2024 at 1:10 PM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Tue, Jul 02, 2024 at 12:42:02PM GMT, Krzysztof Kozlowski wrote:
> > > On 02/07/2024 11:16, Amit Pundir wrote:
> > > > Describe the ath12k WLAN on-board the WCN7850 module present on the
> > > > board.
> > > >
> > > > Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> > > > ---
> > > > Kanged verbatim from 490812872449 ("arm64: dts: qcom: sm8550-qrd: add the Wifi node").
> > > >
> > > >  arch/arm64/boot/dts/qcom/sm8550-hdk.dts | 97 +++++++++++++++++++++++++
> > > >  1 file changed, 97 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> > > > index 12d60a0ee095..c453d081a2df 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> > > > +++ b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> > > > @@ -279,6 +279,68 @@ platform {
> > > >                     };
> > > >             };
> > > >     };
> > > > +
> > > > +   wcn7850-pmu {
> > > > +           compatible = "qcom,wcn7850-pmu";
> > > > +
> > > > +           pinctrl-names = "default";
> > > > +           pinctrl-0 = <&wlan_en>, <&pmk8550_sleep_clk>;
> > > > +
> > > > +           wlan-enable-gpios = <&tlmm 80 GPIO_ACTIVE_HIGH>;
> > > > +           /*
> > > > +            * TODO Add bt-enable-gpios once the Bluetooth driver is
> > > > +            * converted to using the power sequencer.
> > >
> > > I don't understand why hardware description should depend on the driver.
> > > Either you have this GPIO or not. If you have it, what does it matter if
> > > there is no driver who can play with it?
> >
> > Then there is a conflict between BT and PMU, which both will try to
> > access the gpio (or at least the pinctrl).
> 
> Ah, so this slipped through the cracks. Amit merely copied it from existing dts.
> 
> Yes, there's a conflict but Krzysztof is right - we should handle this
> in the driver, not in dts.
> 
> However for that we need a patch for the PMU pwrseq driver first. Let
> me cook something up.

Or for the BT driver, which might be more futureproof.
Bartosz Golaszewski July 2, 2024, 11:38 a.m. UTC | #5
On Tue, Jul 2, 2024 at 1:32 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Tue, Jul 02, 2024 at 01:13:09PM GMT, Bartosz Golaszewski wrote:
> > On Tue, Jul 2, 2024 at 1:10 PM Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> > >
> > > On Tue, Jul 02, 2024 at 12:42:02PM GMT, Krzysztof Kozlowski wrote:
> > > > On 02/07/2024 11:16, Amit Pundir wrote:
> > > > > Describe the ath12k WLAN on-board the WCN7850 module present on the
> > > > > board.
> > > > >
> > > > > Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> > > > > ---
> > > > > Kanged verbatim from 490812872449 ("arm64: dts: qcom: sm8550-qrd: add the Wifi node").
> > > > >
> > > > >  arch/arm64/boot/dts/qcom/sm8550-hdk.dts | 97 +++++++++++++++++++++++++
> > > > >  1 file changed, 97 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> > > > > index 12d60a0ee095..c453d081a2df 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> > > > > +++ b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> > > > > @@ -279,6 +279,68 @@ platform {
> > > > >                     };
> > > > >             };
> > > > >     };
> > > > > +
> > > > > +   wcn7850-pmu {
> > > > > +           compatible = "qcom,wcn7850-pmu";
> > > > > +
> > > > > +           pinctrl-names = "default";
> > > > > +           pinctrl-0 = <&wlan_en>, <&pmk8550_sleep_clk>;
> > > > > +
> > > > > +           wlan-enable-gpios = <&tlmm 80 GPIO_ACTIVE_HIGH>;
> > > > > +           /*
> > > > > +            * TODO Add bt-enable-gpios once the Bluetooth driver is
> > > > > +            * converted to using the power sequencer.
> > > >
> > > > I don't understand why hardware description should depend on the driver.
> > > > Either you have this GPIO or not. If you have it, what does it matter if
> > > > there is no driver who can play with it?
> > >
> > > Then there is a conflict between BT and PMU, which both will try to
> > > access the gpio (or at least the pinctrl).
> >
> > Ah, so this slipped through the cracks. Amit merely copied it from existing dts.
> >
> > Yes, there's a conflict but Krzysztof is right - we should handle this
> > in the driver, not in dts.
> >
> > However for that we need a patch for the PMU pwrseq driver first. Let
> > me cook something up.
>
> Or for the BT driver, which might be more futureproof.

The problem here is that we have DT bindings that define the Bluetooth
node properties (including the bt-enable GPIO) for wcn7850 and so are
committed to supporting existing device-trees. Any change to the
handling of this model (unlike QCA6390) must retain backward
compatibility. I don't know yet how to approach it so for now I
propose to add a quirk to the pwrseq driver and revisit later.

Bart
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
index 12d60a0ee095..c453d081a2df 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
+++ b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
@@ -279,6 +279,68 @@  platform {
 			};
 		};
 	};
+
+	wcn7850-pmu {
+		compatible = "qcom,wcn7850-pmu";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&wlan_en>, <&pmk8550_sleep_clk>;
+
+		wlan-enable-gpios = <&tlmm 80 GPIO_ACTIVE_HIGH>;
+		/*
+		 * TODO Add bt-enable-gpios once the Bluetooth driver is
+		 * converted to using the power sequencer.
+		 */
+
+		vdd-supply = <&vreg_s5g_0p85>;
+		vddio-supply = <&vreg_l15b_1p8>;
+		vddaon-supply = <&vreg_s2g_0p85>;
+		vdddig-supply = <&vreg_s4e_0p95>;
+		vddrfa1p2-supply = <&vreg_s4g_1p25>;
+		vddrfa1p8-supply = <&vreg_s6g_1p86>;
+
+		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 {
@@ -954,6 +1016,23 @@  &pcie0 {
 	status = "okay";
 };
 
+&pcieport0 {
+	wifi@0 {
+		compatible = "pci17cb,1107";
+		reg = <0x10000 0x0 0x0 0x0 0x0>;
+
+		vddrfacmn-supply = <&vreg_pmu_rfa_cmn>;
+		vddaon-supply = <&vreg_pmu_aon_0p59>;
+		vddwlcx-supply = <&vreg_pmu_wlcx_0p8>;
+		vddwlmx-supply = <&vreg_pmu_wlmx_0p85>;
+		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>;
+	};
+};
+
 &pcie0_phy {
 	vdda-phy-supply = <&vreg_l1e_0p88>;
 	vdda-pll-supply = <&vreg_l3e_1p2>;
@@ -1046,6 +1125,17 @@  &pon_resin {
 	status = "okay";
 };
 
+&pmk8550_gpios {
+	pmk8550_sleep_clk: sleep-clk-state {
+		pins = "gpio3";
+		function = "func1";
+		input-disable;
+		output-enable;
+		bias-disable;
+		power-source = <0>;
+	};
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };
@@ -1206,6 +1296,13 @@  wcd_default: wcd-reset-n-active-state {
 		bias-disable;
 		output-low;
 	};
+
+	wlan_en: wlan-en-state {
+		pins = "gpio80";
+		function = "gpio";
+		drive-strength = <8>;
+		bias-pull-down;
+	};
 };
 
 &uart7 {