| Message ID | 20240702091655.278974-1-amit.pundir@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: qcom: sm8550-hdk: add the Wifi node | expand |
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
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).
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
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.
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
Hi, 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. > + */ <snip> Now [1] driver & bindings changes were merged, could you resend with the following squashed: [1] https://lore.kernel.org/all/20240709-hci_qca_refactor-v3-0-5f48ca001fed@linaro.org/ ====><===================================== diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts index 0fa0b79de741..01c921602605 100644 --- a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts +++ b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts @@ -284,13 +284,10 @@ wcn7850-pmu { compatible = "qcom,wcn7850-pmu"; pinctrl-names = "default"; - pinctrl-0 = <&wlan_en>, <&pmk8550_sleep_clk>; + pinctrl-0 = <&wlan_en>, <&bt_default>, <&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. - */ + bt-enable-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>; vdd-supply = <&vreg_s5g_0p85>; vddio-supply = <&vreg_l15b_1p8>; @@ -1312,20 +1309,15 @@ &uart14 { bluetooth { compatible = "qcom,wcn7850-bt"; - vddio-supply = <&vreg_l15b_1p8>; - vddaon-supply = <&vreg_s4e_0p95>; - vdddig-supply = <&vreg_s4e_0p95>; - vddrfa0p8-supply = <&vreg_s4e_0p95>; - vddrfa1p2-supply = <&vreg_s4g_1p25>; - vddrfa1p9-supply = <&vreg_s6g_1p86>; + 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>; max-speed = <3200000>; - - enable-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>; - swctrl-gpios = <&tlmm 82 GPIO_ACTIVE_HIGH>; - - pinctrl-0 = <&bt_default>; - pinctrl-names = "default"; }; }; ====><===================================== Thanks, Neil
On Tue, 16 Jul 2024 at 15:10, Neil Armstrong <neil.armstrong@linaro.org> wrote: > > Hi, > > 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. > > + */ > > <snip> > > Now [1] driver & bindings changes were merged, could you resend with the following squashed: Done. Thank you very much Neil. > > [1] https://lore.kernel.org/all/20240709-hci_qca_refactor-v3-0-5f48ca001fed@linaro.org/ > > ====><===================================== > diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts > index 0fa0b79de741..01c921602605 100644 > --- a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts > +++ b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts > @@ -284,13 +284,10 @@ wcn7850-pmu { > compatible = "qcom,wcn7850-pmu"; > > pinctrl-names = "default"; > - pinctrl-0 = <&wlan_en>, <&pmk8550_sleep_clk>; > + pinctrl-0 = <&wlan_en>, <&bt_default>, <&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. > - */ > + bt-enable-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>; > > vdd-supply = <&vreg_s5g_0p85>; > vddio-supply = <&vreg_l15b_1p8>; > @@ -1312,20 +1309,15 @@ &uart14 { > bluetooth { > compatible = "qcom,wcn7850-bt"; > > - vddio-supply = <&vreg_l15b_1p8>; > - vddaon-supply = <&vreg_s4e_0p95>; > - vdddig-supply = <&vreg_s4e_0p95>; > - vddrfa0p8-supply = <&vreg_s4e_0p95>; > - vddrfa1p2-supply = <&vreg_s4g_1p25>; > - vddrfa1p9-supply = <&vreg_s6g_1p86>; > + 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>; > > max-speed = <3200000>; > - > - enable-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>; > - swctrl-gpios = <&tlmm 82 GPIO_ACTIVE_HIGH>; > - > - pinctrl-0 = <&bt_default>; > - pinctrl-names = "default"; > }; > }; > ====><===================================== > > Thanks, > Neil
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 {
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(+)