[PATCH v9 0/2] dt-bindings: describe the ath1X modules on QCom BT/WLAN chipsets

[PATCH v9 0/2] dt-bindings: describe the ath1X modules on QCom BT/WLAN chipsets

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Here are the two dt-binding patches from the power-sequencing series
targeting the wireless subsystem. To keep the cover-letter short, I
won't repeat all the details, they can be found in the cover-letter for
v8. Please consider picking them up into your tree, they were reviewed
by Krzysztof earlier.

Changelog:

Since v8:
- split out the wireless bindings into their own series
- Link to v8: https://lore.kernel.org/r/20240528-pwrseq-v8-0-d354d52b763c@linaro.org

Since v7:
- added DTS changes for sm8650-hdk
- added circular dependency detection for pwrseq units
- fixed a KASAN reported use-after-free error in remove path
- improve Kconfig descriptions
- fix typos in bindings and Kconfig
- fixed issues reported by smatch
- fix the unbind path in PCI pwrctl
- lots of minor improvements to the pwrseq core

Since v6:
- kernel doc fixes
- drop myself from the DT bindings maintainers list for ath12k
- wait until the PCI bridge device is fully added before creating the
  PCI pwrctl platform devices for its sub-nodes, otherwise we may see
  sysfs and procfs attribute failures (due to duplication, we're
  basically trying to probe the same device twice at the same time)
- I kept the regulators for QCA6390's ath11k as required as they only
  apply to this specific Qualcomm package

Since v5:
- unify the approach to modelling the WCN WLAN/BT chips by always exposing
  the PMU node on the device tree and making the WLAN and BT nodes become
  consumers of its power outputs; this includes a major rework of the DT
  sources, bindings and driver code; there's no more a separate PCI
  pwrctl driver for WCN7850, instead its power-up sequence was moved
  into the pwrseq driver common for all WCN chips
- don't set load_uA from new regulator consumers
- fix reported kerneldoc issues
- drop voltage ranges for PMU outputs from DT
- many minor tweaks and reworks

v1: Original RFC:

https://lore.kernel.org/lkml/20240104130123.37115-1-brgl@bgdev.pl/T/

v2: First real patch series (should have been PATCH v2) adding what I
    referred to back then as PCI power sequencing:

https://lore.kernel.org/linux-arm-kernel/2024021413-grumbling-unlivable-c145@gregkh/T/

v3: RFC for the DT representation of the PMU supplying the WLAN and BT
    modules inside the QCA6391 package (was largely separate from the
    series but probably should have been called PATCH or RFC v3):

https://lore.kernel.org/all/CAMRc=Mc+GNoi57eTQg71DXkQKjdaoAmCpB=h2ndEpGnmdhVV-Q@mail.gmail.com/T/

v4: Second attempt at the full series with changed scope (introduction of
    the pwrseq subsystem, should have been RFC v4)

https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/

v5: Two different ways of handling QCA6390 and WCN7850:

https://lore.kernel.org/lkml/20240216203215.40870-1-brgl@bgdev.pl/

Bartosz Golaszewski (2):
  dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on
    QCA6390
  dt-bindings: net: wireless: describe the ath12k PCI module

 .../net/wireless/qcom,ath11k-pci.yaml         | 46 +++++++++
 .../bindings/net/wireless/qcom,ath12k.yaml    | 99 +++++++++++++++++++
 2 files changed, 145 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml

-- 
2.40.1

[PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Add a PCI compatible for the ATH11K module on QCA6390 and describe the
power inputs from the PMU that it consumes.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 .../net/wireless/qcom,ath11k-pci.yaml         | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k-pci.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k-pci.yaml
index 41d023797d7d..8675d7d0215c 100644
--- a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k-pci.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k-pci.yaml
@@ -17,6 +17,7 @@ description: |
 properties:
   compatible:
     enum:
+      - pci17cb,1101  # QCA6390
       - pci17cb,1103  # WCN6855
 
   reg:
@@ -28,10 +29,55 @@ properties:
       string to uniquely identify variant of the calibration data for designs
       with colliding bus and device ids
 
+  vddrfacmn-supply:
+    description: VDD_RFA_CMN supply regulator handle
+
+  vddaon-supply:
+    description: VDD_AON supply regulator handle
+
+  vddwlcx-supply:
+    description: VDD_WL_CX supply regulator handle
+
+  vddwlmx-supply:
+    description: VDD_WL_MX supply regulator handle
+
+  vddrfa0p8-supply:
+    description: VDD_RFA_0P8 supply regulator handle
+
+  vddrfa1p2-supply:
+    description: VDD_RFA_1P2 supply regulator handle
+
+  vddrfa1p7-supply:
+    description: VDD_RFA_1P7 supply regulator handle
+
+  vddpcie0p9-supply:
+    description: VDD_PCIE_0P9 supply regulator handle
+
+  vddpcie1p8-supply:
+    description: VDD_PCIE_1P8 supply regulator handle
+
 required:
   - compatible
   - reg
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: pci17cb,1101
+    then:
+      required:
+        - vddrfacmn-supply
+        - vddaon-supply
+        - vddwlcx-supply
+        - vddwlmx-supply
+        - vddrfa0p8-supply
+        - vddrfa1p2-supply
+        - vddrfa1p7-supply
+        - vddpcie0p9-supply
+        - vddpcie1p8-supply
+
 additionalProperties: false
 
 examples:
-- 
2.40.1

[PATCH v9 2/2] dt-bindings: net: wireless: describe the ath12k PCI module

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Add device-tree bindings for the ATH12K module found in the WCN7850
package.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 .../bindings/net/wireless/qcom,ath12k.yaml    | 99 +++++++++++++++++++
 1 file changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml

diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
new file mode 100644
index 000000000000..1b5884015b15
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2024 Linaro Limited
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/wireless/qcom,ath12k.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies ath12k wireless devices (PCIe)
+
+maintainers:
+  - Jeff Johnson <quic_jjohnson@quicinc.com>
+  - Kalle Valo <kvalo@kernel.org>
+
+description:
+  Qualcomm Technologies IEEE 802.11be PCIe devices.
+
+properties:
+  compatible:
+    enum:
+      - pci17cb,1107  # WCN7850
+
+  reg:
+    maxItems: 1
+
+  vddaon-supply:
+    description: VDD_AON supply regulator handle
+
+  vddwlcx-supply:
+    description: VDD_WLCX supply regulator handle
+
+  vddwlmx-supply:
+    description: VDD_WLMX supply regulator handle
+
+  vddrfacmn-supply:
+    description: VDD_RFA_CMN supply regulator handle
+
+  vddrfa0p8-supply:
+    description: VDD_RFA_0P8 supply regulator handle
+
+  vddrfa1p2-supply:
+    description: VDD_RFA_1P2 supply regulator handle
+
+  vddrfa1p8-supply:
+    description: VDD_RFA_1P8 supply regulator handle
+
+  vddpcie0p9-supply:
+    description: VDD_PCIE_0P9 supply regulator handle
+
+  vddpcie1p8-supply:
+    description: VDD_PCIE_1P8 supply regulator handle
+
+required:
+  - compatible
+  - reg
+  - vddaon-supply
+  - vddwlcx-supply
+  - vddwlmx-supply
+  - vddrfacmn-supply
+  - vddrfa0p8-supply
+  - vddrfa1p2-supply
+  - vddrfa1p8-supply
+  - vddpcie0p9-supply
+  - vddpcie1p8-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,rpmh.h>
+    #include <dt-bindings/gpio/gpio.h>
+    pcie {
+        #address-cells = <3>;
+        #size-cells = <2>;
+
+        pcie@0 {
+            device_type = "pci";
+            reg = <0x0 0x0 0x0 0x0 0x0>;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            ranges;
+
+            bus-range = <0x01 0xff>;
+
+            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>;
+            };
+        };
+    };
-- 
2.40.1

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Kalle Valo]

Bartosz Golaszewski <brgl@bgdev.pl> writes:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Add a PCI compatible for the ATH11K module on QCA6390 and describe the
> power inputs from the PMU that it consumes.
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

[...]

> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: pci17cb,1101
> +    then:
> +      required:
> +        - vddrfacmn-supply
> +        - vddaon-supply
> +        - vddwlcx-supply
> +        - vddwlmx-supply
> +        - vddrfa0p8-supply
> +        - vddrfa1p2-supply
> +        - vddrfa1p7-supply
> +        - vddpcie0p9-supply
> +        - vddpcie1p8-supply

Not sure if we discussed this before, but based on this I understand
that there can't be an DT entry for device pci17cb,1101 without all the
supply properties? But there are QCA6390 devices with PCI id 17cb:1101
which do not need these supplies and already work. For example, my Dell
XPS 13 x86 laptop is one. Or anyone who manually installs QCA6390 board
to their PCI slot and some of them might want to use DT, for example
setting qcom,ath11k-calibration-variant.

This is not a blocker for me, just making sure that we are not breaking
any existing setups.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v9 2/2] dt-bindings: net: wireless: describe the ath12k PCI module

[Kalle Valo]

Bartosz Golaszewski <brgl@bgdev.pl> writes:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Add device-tree bindings for the ATH12K module found in the WCN7850
> package.
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  .../bindings/net/wireless/qcom,ath12k.yaml    | 99 +++++++++++++++++++
>  1 file changed, 99 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> new file mode 100644
> index 000000000000..1b5884015b15
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (c) 2024 Linaro Limited
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/wireless/qcom,ath12k.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Technologies ath12k wireless devices (PCIe)
> +
> +maintainers:
> +  - Jeff Johnson <quic_jjohnson@quicinc.com>
> +  - Kalle Valo <kvalo@kernel.org>
> +
> +description:
> +  Qualcomm Technologies IEEE 802.11be PCIe devices.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - pci17cb,1107  # WCN7850
> +
> +  reg:
> +    maxItems: 1
> +
> +  vddaon-supply:
> +    description: VDD_AON supply regulator handle
> +
> +  vddwlcx-supply:
> +    description: VDD_WLCX supply regulator handle
> +
> +  vddwlmx-supply:
> +    description: VDD_WLMX supply regulator handle
> +
> +  vddrfacmn-supply:
> +    description: VDD_RFA_CMN supply regulator handle
> +
> +  vddrfa0p8-supply:
> +    description: VDD_RFA_0P8 supply regulator handle
> +
> +  vddrfa1p2-supply:
> +    description: VDD_RFA_1P2 supply regulator handle
> +
> +  vddrfa1p8-supply:
> +    description: VDD_RFA_1P8 supply regulator handle
> +
> +  vddpcie0p9-supply:
> +    description: VDD_PCIE_0P9 supply regulator handle
> +
> +  vddpcie1p8-supply:
> +    description: VDD_PCIE_1P8 supply regulator handle
> +
> +required:
> +  - compatible
> +  - reg
> +  - vddaon-supply
> +  - vddwlcx-supply
> +  - vddwlmx-supply
> +  - vddrfacmn-supply
> +  - vddrfa0p8-supply
> +  - vddrfa1p2-supply
> +  - vddrfa1p8-supply
> +  - vddpcie0p9-supply
> +  - vddpcie1p8-supply

Same comment here. To my understanding there are ath12k PCI devices
which do not need the supplies. But I guess we change this to optional
later?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v9 0/2] dt-bindings: describe the ath1X modules on QCom BT/WLAN chipsets

[Kalle Valo]

Bartosz Golaszewski <brgl@bgdev.pl> writes:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Here are the two dt-binding patches from the power-sequencing series
> targeting the wireless subsystem. To keep the cover-letter short, I
> won't repeat all the details, they can be found in the cover-letter for
> v8. Please consider picking them up into your tree, they were reviewed
> by Krzysztof earlier.

Is it ok for everyone that I'll take these to our ath.git tree?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Bartosz Golaszewski]

On Thu, Jun 6, 2024 at 3:30 PM Kalle Valo <kvalo@kernel.org> wrote:
>
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Add a PCI compatible for the ATH11K module on QCA6390 and describe the
> > power inputs from the PMU that it consumes.
> >
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> [...]
>
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: pci17cb,1101
> > +    then:
> > +      required:
> > +        - vddrfacmn-supply
> > +        - vddaon-supply
> > +        - vddwlcx-supply
> > +        - vddwlmx-supply
> > +        - vddrfa0p8-supply
> > +        - vddrfa1p2-supply
> > +        - vddrfa1p7-supply
> > +        - vddpcie0p9-supply
> > +        - vddpcie1p8-supply
>
> Not sure if we discussed this before, but based on this I understand
> that there can't be an DT entry for device pci17cb,1101 without all the
> supply properties? But there are QCA6390 devices with PCI id 17cb:1101
> which do not need these supplies and already work. For example, my Dell
> XPS 13 x86 laptop is one. Or anyone who manually installs QCA6390 board
> to their PCI slot and some of them might want to use DT, for example
> setting qcom,ath11k-calibration-variant.
>
> This is not a blocker for me, just making sure that we are not breaking
> any existing setups.
>

If they are already powered up without the need for the PCI pwrctl
driver to do it, then they will work alright. Bindings don't affect
functionality. But if you have a QCA6390 then you have its PMU too and
the bindings model the real-world hardware.

IOW: your laptop should be alright but the supplies are really there
which warrants adding them to the bindings.

Bart

> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v9 2/2] dt-bindings: net: wireless: describe the ath12k PCI module

[Bartosz Golaszewski]

On Thu, Jun 6, 2024 at 3:34 PM Kalle Valo <kvalo@kernel.org> wrote:
>
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Add device-tree bindings for the ATH12K module found in the WCN7850
> > package.
> >
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> >  .../bindings/net/wireless/qcom,ath12k.yaml    | 99 +++++++++++++++++++
> >  1 file changed, 99 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> > new file mode 100644
> > index 000000000000..1b5884015b15
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> > @@ -0,0 +1,99 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (c) 2024 Linaro Limited
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/wireless/qcom,ath12k.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm Technologies ath12k wireless devices (PCIe)
> > +
> > +maintainers:
> > +  - Jeff Johnson <quic_jjohnson@quicinc.com>
> > +  - Kalle Valo <kvalo@kernel.org>
> > +
> > +description:
> > +  Qualcomm Technologies IEEE 802.11be PCIe devices.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - pci17cb,1107  # WCN7850
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  vddaon-supply:
> > +    description: VDD_AON supply regulator handle
> > +
> > +  vddwlcx-supply:
> > +    description: VDD_WLCX supply regulator handle
> > +
> > +  vddwlmx-supply:
> > +    description: VDD_WLMX supply regulator handle
> > +
> > +  vddrfacmn-supply:
> > +    description: VDD_RFA_CMN supply regulator handle
> > +
> > +  vddrfa0p8-supply:
> > +    description: VDD_RFA_0P8 supply regulator handle
> > +
> > +  vddrfa1p2-supply:
> > +    description: VDD_RFA_1P2 supply regulator handle
> > +
> > +  vddrfa1p8-supply:
> > +    description: VDD_RFA_1P8 supply regulator handle
> > +
> > +  vddpcie0p9-supply:
> > +    description: VDD_PCIE_0P9 supply regulator handle
> > +
> > +  vddpcie1p8-supply:
> > +    description: VDD_PCIE_1P8 supply regulator handle
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - vddaon-supply
> > +  - vddwlcx-supply
> > +  - vddwlmx-supply
> > +  - vddrfacmn-supply
> > +  - vddrfa0p8-supply
> > +  - vddrfa1p2-supply
> > +  - vddrfa1p8-supply
> > +  - vddpcie0p9-supply
> > +  - vddpcie1p8-supply
>
> Same comment here. To my understanding there are ath12k PCI devices
> which do not need the supplies. But I guess we change this to optional
> later?
>

Sure we can. But "pci17cb,1107" refers to a QCom variant and it always
comes powered by a PMU so these supplies really are required.

Bart

> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Kalle Valo]

Bartosz Golaszewski <brgl@bgdev.pl> writes:

> On Thu, Jun 6, 2024 at 3:30 PM Kalle Valo <kvalo@kernel.org> wrote:
>
>>
>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>
>> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> >
>> > Add a PCI compatible for the ATH11K module on QCA6390 and describe the
>> > power inputs from the PMU that it consumes.
>> >
>> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>
>> [...]
>>
>> > +allOf:
>> > +  - if:
>> > +      properties:
>> > +        compatible:
>> > +          contains:
>> > +            const: pci17cb,1101
>> > +    then:
>> > +      required:
>> > +        - vddrfacmn-supply
>> > +        - vddaon-supply
>> > +        - vddwlcx-supply
>> > +        - vddwlmx-supply
>> > +        - vddrfa0p8-supply
>> > +        - vddrfa1p2-supply
>> > +        - vddrfa1p7-supply
>> > +        - vddpcie0p9-supply
>> > +        - vddpcie1p8-supply
>>
>> Not sure if we discussed this before, but based on this I understand
>> that there can't be an DT entry for device pci17cb,1101 without all the
>> supply properties? But there are QCA6390 devices with PCI id 17cb:1101
>> which do not need these supplies and already work. For example, my Dell
>> XPS 13 x86 laptop is one. Or anyone who manually installs QCA6390 board
>> to their PCI slot and some of them might want to use DT, for example
>> setting qcom,ath11k-calibration-variant.
>>
>> This is not a blocker for me, just making sure that we are not breaking
>> any existing setups.
>>
>
> If they are already powered up without the need for the PCI pwrctl
> driver to do it, then they will work alright. Bindings don't affect
> functionality.

Sure, I'm not worried about functionality. I'm worried that if I
there's, for example, an ARM based setup which uses DT and wants to use
a similar QCA6390 board that I have, and set
qcom,ath11k-calibration-variant in DT. In other words, I'm worried if
you are looking at this only for Snapdragon family of boards?

Again, I don't see this as a blocker. I just want to understand how this
should work for all types of devices there are out there.

> But if you have a QCA6390 then you have its PMU too and the bindings
> model the real-world hardware.
>
> IOW: your laptop should be alright but the supplies are really there
> which warrants adding them to the bindings.

Sorry, not following here. Can you clarify your comment "the supplies
are really there"? You mean inside the PCI board? But that's not visible
to the kernel in anyway, the PCI board just works after I plug it in.
It's like a regular PCI device. So I don't understand why that should be
visible in DT, but I can very well be missing something.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Bartosz Golaszewski]

On Thu, Jun 6, 2024 at 4:02 PM Kalle Valo <kvalo@kernel.org> wrote:
>
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>
> > On Thu, Jun 6, 2024 at 3:30 PM Kalle Valo <kvalo@kernel.org> wrote:
> >
> >>
> >> Bartosz Golaszewski <brgl@bgdev.pl> writes:
> >>
> >> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >> >
> >> > Add a PCI compatible for the ATH11K module on QCA6390 and describe the
> >> > power inputs from the PMU that it consumes.
> >> >
> >> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >>
> >> [...]
> >>
> >> > +allOf:
> >> > +  - if:
> >> > +      properties:
> >> > +        compatible:
> >> > +          contains:
> >> > +            const: pci17cb,1101
> >> > +    then:
> >> > +      required:
> >> > +        - vddrfacmn-supply
> >> > +        - vddaon-supply
> >> > +        - vddwlcx-supply
> >> > +        - vddwlmx-supply
> >> > +        - vddrfa0p8-supply
> >> > +        - vddrfa1p2-supply
> >> > +        - vddrfa1p7-supply
> >> > +        - vddpcie0p9-supply
> >> > +        - vddpcie1p8-supply
> >>
> >> Not sure if we discussed this before, but based on this I understand
> >> that there can't be an DT entry for device pci17cb,1101 without all the
> >> supply properties? But there are QCA6390 devices with PCI id 17cb:1101
> >> which do not need these supplies and already work. For example, my Dell
> >> XPS 13 x86 laptop is one. Or anyone who manually installs QCA6390 board
> >> to their PCI slot and some of them might want to use DT, for example
> >> setting qcom,ath11k-calibration-variant.
> >>
> >> This is not a blocker for me, just making sure that we are not breaking
> >> any existing setups.
> >>
> >
> > If they are already powered up without the need for the PCI pwrctl
> > driver to do it, then they will work alright. Bindings don't affect
> > functionality.
>
> Sure, I'm not worried about functionality. I'm worried that if I
> there's, for example, an ARM based setup which uses DT and wants to use
> a similar QCA6390 board that I have, and set
> qcom,ath11k-calibration-variant in DT. In other words, I'm worried if
> you are looking at this only for Snapdragon family of boards?
>

No, what I'm looking at is the entire QCA6390 package. That means WLAN
*and* Bluetooth *and* the PMU that manages power.

If you're using the QCA6390 on a device-tree system then you should
probably model at least the WLAN node and the PMU and the problem with
supplies is fixed. But if you don't have the supplies, that's alright
for downstream.

> Again, I don't see this as a blocker. I just want to understand how this
> should work for all types of devices there are out there.
>
> > But if you have a QCA6390 then you have its PMU too and the bindings
> > model the real-world hardware.
> >
> > IOW: your laptop should be alright but the supplies are really there
> > which warrants adding them to the bindings.
>
> Sorry, not following here. Can you clarify your comment "the supplies
> are really there"? You mean inside the PCI board? But that's not visible
> to the kernel in anyway, the PCI board just works after I plug it in.
> It's like a regular PCI device. So I don't understand why that should be
> visible in DT, but I can very well be missing something.
>

I think you're thinking about some kind of detachable PCIe board with
this chipset on it. I refer to the QCA6390 chipset itself which is
also more than just PCI. The Bluetooth interface doesn't use PCI at
all. On the boards I'm working on, the chipset is just soldered to the
main board. If your detachable board "just works" then it must be
wired in a way that enables WLAN the moment it's plugged in but this
doesn't happen over PCI. The chipset has a power input and GPIOs to
enable each module.

Also: I doubt you need DT for your detachable board?

Bart

> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Kalle Valo]

Bartosz Golaszewski <brgl@bgdev.pl> writes:

> On Thu, Jun 6, 2024 at 4:02 PM Kalle Valo <kvalo@kernel.org> wrote:
>
>>
>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>
>> > On Thu, Jun 6, 2024 at 3:30 PM Kalle Valo <kvalo@kernel.org> wrote:
>> >
>> >>
>> >> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>> >>
>> >> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> >> >
>> >> > Add a PCI compatible for the ATH11K module on QCA6390 and describe the
>> >> > power inputs from the PMU that it consumes.
>> >> >
>> >> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> >> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> >>
>> >> [...]
>> >>
>> >> > +allOf:
>> >> > +  - if:
>> >> > +      properties:
>> >> > +        compatible:
>> >> > +          contains:
>> >> > +            const: pci17cb,1101
>> >> > +    then:
>> >> > +      required:
>> >> > +        - vddrfacmn-supply
>> >> > +        - vddaon-supply
>> >> > +        - vddwlcx-supply
>> >> > +        - vddwlmx-supply
>> >> > +        - vddrfa0p8-supply
>> >> > +        - vddrfa1p2-supply
>> >> > +        - vddrfa1p7-supply
>> >> > +        - vddpcie0p9-supply
>> >> > +        - vddpcie1p8-supply
>> >>
>> >> Not sure if we discussed this before, but based on this I understand
>> >> that there can't be an DT entry for device pci17cb,1101 without all the
>> >> supply properties? But there are QCA6390 devices with PCI id 17cb:1101
>> >> which do not need these supplies and already work. For example, my Dell
>> >> XPS 13 x86 laptop is one. Or anyone who manually installs QCA6390 board
>> >> to their PCI slot and some of them might want to use DT, for example
>> >> setting qcom,ath11k-calibration-variant.
>> >>
>> >> This is not a blocker for me, just making sure that we are not breaking
>> >> any existing setups.
>> >>
>> >
>> > If they are already powered up without the need for the PCI pwrctl
>> > driver to do it, then they will work alright. Bindings don't affect
>> > functionality.
>>
>> Sure, I'm not worried about functionality. I'm worried that if I
>> there's, for example, an ARM based setup which uses DT and wants to use
>> a similar QCA6390 board that I have, and set
>> qcom,ath11k-calibration-variant in DT. In other words, I'm worried if
>> you are looking at this only for Snapdragon family of boards?
>>
>
> No, what I'm looking at is the entire QCA6390 package. That means WLAN
> *and* Bluetooth *and* the PMU that manages power.

I think we are just looking at this from different point of views. You
are looking at a datasheet (most likely for a Snapdragon based system)
and I'm looking what actual devices there are out in the field.

> If you're using the QCA6390 on a device-tree system then you should
> probably model at least the WLAN node and the PMU and the problem with
> supplies is fixed.

But why? If there are boards out there who don't need any of this why
would they still need to model all this in DT?

Based on the discussions I have heard only Snapdragon systems who
require all this configuration you describe. Of course there can be
other systems but I have not heard about those.

> But if you don't have the supplies, that's alright for downstream.

What do you mean downstream in this context?

>> Again, I don't see this as a blocker. I just want to understand how this
>> should work for all types of devices there are out there.
>>
>> > But if you have a QCA6390 then you have its PMU too and the bindings
>> > model the real-world hardware.
>> >
>> > IOW: your laptop should be alright but the supplies are really there
>> > which warrants adding them to the bindings.
>>
>> Sorry, not following here. Can you clarify your comment "the supplies
>> are really there"? You mean inside the PCI board? But that's not visible
>> to the kernel in anyway, the PCI board just works after I plug it in.
>> It's like a regular PCI device. So I don't understand why that should be
>> visible in DT, but I can very well be missing something.
>>
>
> I think you're thinking about some kind of detachable PCIe board with
> this chipset on it.

Exactly, a lot of WLAN boards are like this.

> I refer to the QCA6390 chipset itself which is also more than just
> PCI. The Bluetooth interface doesn't use PCI at all. On the boards I'm
> working on, the chipset is just soldered to the main board.

And I guess you are looking at Snapdragon boards only?

> If your detachable board "just works" then it must be wired in a way
> that enables WLAN the moment it's plugged in but this doesn't happen
> over PCI. The chipset has a power input and GPIOs to enable each
> module.

I don't know how the boards are implemented but it could be so. But from
host system point of view it's just a regular PCI device.

> Also: I doubt you need DT for your detachable board?

Sure, I don't need DT but that's not my point. My point is why require
these supplies for _all_ devices having PCI id 17cb:1101 (ie. QCA6390)
then clearly there are such devices which don't need it? To me that's
bad design and, if I'm understanding correctly, prevents use of
qcom,ath11k-calibration-variant property. To me having the supplies
optional in DT is more approriate.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Bartosz Golaszewski]

On Thu, Jun 6, 2024 at 6:16 PM Kalle Valo <kvalo@kernel.org> wrote:
>
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>
> > On Thu, Jun 6, 2024 at 4:02 PM Kalle Valo <kvalo@kernel.org> wrote:
> >
> >>
> >> Bartosz Golaszewski <brgl@bgdev.pl> writes:
> >>
> >> > On Thu, Jun 6, 2024 at 3:30 PM Kalle Valo <kvalo@kernel.org> wrote:
> >> >
> >> >>
> >> >> Bartosz Golaszewski <brgl@bgdev.pl> writes:
> >> >>
> >> >> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >> >> >
> >> >> > Add a PCI compatible for the ATH11K module on QCA6390 and describe the
> >> >> > power inputs from the PMU that it consumes.
> >> >> >
> >> >> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> >> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >> >>
> >> >> [...]
> >> >>
> >> >> > +allOf:
> >> >> > +  - if:
> >> >> > +      properties:
> >> >> > +        compatible:
> >> >> > +          contains:
> >> >> > +            const: pci17cb,1101
> >> >> > +    then:
> >> >> > +      required:
> >> >> > +        - vddrfacmn-supply
> >> >> > +        - vddaon-supply
> >> >> > +        - vddwlcx-supply
> >> >> > +        - vddwlmx-supply
> >> >> > +        - vddrfa0p8-supply
> >> >> > +        - vddrfa1p2-supply
> >> >> > +        - vddrfa1p7-supply
> >> >> > +        - vddpcie0p9-supply
> >> >> > +        - vddpcie1p8-supply
> >> >>
> >> >> Not sure if we discussed this before, but based on this I understand
> >> >> that there can't be an DT entry for device pci17cb,1101 without all the
> >> >> supply properties? But there are QCA6390 devices with PCI id 17cb:1101
> >> >> which do not need these supplies and already work. For example, my Dell
> >> >> XPS 13 x86 laptop is one. Or anyone who manually installs QCA6390 board
> >> >> to their PCI slot and some of them might want to use DT, for example
> >> >> setting qcom,ath11k-calibration-variant.
> >> >>
> >> >> This is not a blocker for me, just making sure that we are not breaking
> >> >> any existing setups.
> >> >>
> >> >
> >> > If they are already powered up without the need for the PCI pwrctl
> >> > driver to do it, then they will work alright. Bindings don't affect
> >> > functionality.
> >>
> >> Sure, I'm not worried about functionality. I'm worried that if I
> >> there's, for example, an ARM based setup which uses DT and wants to use
> >> a similar QCA6390 board that I have, and set
> >> qcom,ath11k-calibration-variant in DT. In other words, I'm worried if
> >> you are looking at this only for Snapdragon family of boards?
> >>
> >
> > No, what I'm looking at is the entire QCA6390 package. That means WLAN
> > *and* Bluetooth *and* the PMU that manages power.
>
> I think we are just looking at this from different point of views. You
> are looking at a datasheet (most likely for a Snapdragon based system)
> and I'm looking what actual devices there are out in the field.
>
> > If you're using the QCA6390 on a device-tree system then you should
> > probably model at least the WLAN node and the PMU and the problem with
> > supplies is fixed.
>
> But why? If there are boards out there who don't need any of this why
> would they still need to model all this in DT?
>

Because this is what is there? The goal of the device tree is to
describe the hardware. The fact we didn't describe it before doesn't
make it correct.

> Based on the discussions I have heard only Snapdragon systems who
> require all this configuration you describe. Of course there can be
> other systems but I have not heard about those.
>

DT is not configuration, it is description of actual hardware. It
doesn't matter if Snapdragon systems are the only ones that actually
*require* this description to make WLAN/BT functional upstream. The
chipset would be the same on any PCIe board, it's just that the host
systems wouldn't need to take care with its power sequence. But for a
dynamic board like this, you don't need DT.

> > But if you don't have the supplies, that's alright for downstream.
>
> What do you mean downstream in this context?
>

I mean: if you wanted to upstream the DT sources, then they should
include the supplies AND the PMU node. But if you just want to make
the WLAN run on some vendor kernel then you don't need to think about
it, it will work.

> >> Again, I don't see this as a blocker. I just want to understand how this
> >> should work for all types of devices there are out there.
> >>
> >> > But if you have a QCA6390 then you have its PMU too and the bindings
> >> > model the real-world hardware.
> >> >
> >> > IOW: your laptop should be alright but the supplies are really there
> >> > which warrants adding them to the bindings.
> >>
> >> Sorry, not following here. Can you clarify your comment "the supplies
> >> are really there"? You mean inside the PCI board? But that's not visible
> >> to the kernel in anyway, the PCI board just works after I plug it in.
> >> It's like a regular PCI device. So I don't understand why that should be
> >> visible in DT, but I can very well be missing something.
> >>
> >
> > I think you're thinking about some kind of detachable PCIe board with
> > this chipset on it.
>
> Exactly, a lot of WLAN boards are like this.
>
> > I refer to the QCA6390 chipset itself which is also more than just
> > PCI. The Bluetooth interface doesn't use PCI at all. On the boards I'm
> > working on, the chipset is just soldered to the main board.
>
> And I guess you are looking at Snapdragon boards only?
>

But what is your point?

> > If your detachable board "just works" then it must be wired in a way
> > that enables WLAN the moment it's plugged in but this doesn't happen
> > over PCI. The chipset has a power input and GPIOs to enable each
> > module.
>
> I don't know how the boards are implemented but it could be so. But from
> host system point of view it's just a regular PCI device.
>

And you don't need DT anyway for this type of devices.

> > Also: I doubt you need DT for your detachable board?
>
> Sure, I don't need DT but that's not my point. My point is why require
> these supplies for _all_ devices having PCI id 17cb:1101 (ie. QCA6390)
> then clearly there are such devices which don't need it? To me that's
> bad design and, if I'm understanding correctly, prevents use of
> qcom,ath11k-calibration-variant property. To me having the supplies
> optional in DT is more approriate.
>

We require them because *they are physically there*.

Bart

> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v9 0/2] dt-bindings: describe the ath1X modules on QCom BT/WLAN chipsets

[Jeff Johnson]

On 6/6/2024 6:35 AM, Kalle Valo wrote:
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
> 
>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>
>> Here are the two dt-binding patches from the power-sequencing series
>> targeting the wireless subsystem. To keep the cover-letter short, I
>> won't repeat all the details, they can be found in the cover-letter for
>> v8. Please consider picking them up into your tree, they were reviewed
>> by Krzysztof earlier.
> 
> Is it ok for everyone that I'll take these to our ath.git tree?
> 

Kalle, if you take through your tree can you:
s/quic_jjohnson@quicinc.com/jjohnson@kernel.org/

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Krzysztof Kozlowski]

On 06/06/2024 20:08, Bartosz Golaszewski wrote:
> On Thu, Jun 6, 2024 at 6:16 PM Kalle Valo <kvalo@kernel.org> wrote:
>>
>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>
>>> On Thu, Jun 6, 2024 at 4:02 PM Kalle Valo <kvalo@kernel.org> wrote:
>>>
>>>>
>>>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>>>
>>>>> On Thu, Jun 6, 2024 at 3:30 PM Kalle Valo <kvalo@kernel.org> wrote:
>>>>>
>>>>>>
>>>>>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>>>>>
>>>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>>>>
>>>>>>> Add a PCI compatible for the ATH11K module on QCA6390 and describe the
>>>>>>> power inputs from the PMU that it consumes.
>>>>>>>
>>>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>>> +allOf:
>>>>>>> +  - if:
>>>>>>> +      properties:
>>>>>>> +        compatible:
>>>>>>> +          contains:
>>>>>>> +            const: pci17cb,1101
>>>>>>> +    then:
>>>>>>> +      required:
>>>>>>> +        - vddrfacmn-supply
>>>>>>> +        - vddaon-supply
>>>>>>> +        - vddwlcx-supply
>>>>>>> +        - vddwlmx-supply
>>>>>>> +        - vddrfa0p8-supply
>>>>>>> +        - vddrfa1p2-supply
>>>>>>> +        - vddrfa1p7-supply
>>>>>>> +        - vddpcie0p9-supply
>>>>>>> +        - vddpcie1p8-supply
>>>>>>
>>>>>> Not sure if we discussed this before, but based on this I understand
>>>>>> that there can't be an DT entry for device pci17cb,1101 without all the
>>>>>> supply properties? But there are QCA6390 devices with PCI id 17cb:1101
>>>>>> which do not need these supplies and already work. For example, my Dell
>>>>>> XPS 13 x86 laptop is one. Or anyone who manually installs QCA6390 board
>>>>>> to their PCI slot and some of them might want to use DT, for example
>>>>>> setting qcom,ath11k-calibration-variant.
>>>>>>
>>>>>> This is not a blocker for me, just making sure that we are not breaking
>>>>>> any existing setups.
>>>>>>
>>>>>
>>>>> If they are already powered up without the need for the PCI pwrctl
>>>>> driver to do it, then they will work alright. Bindings don't affect
>>>>> functionality.
>>>>
>>>> Sure, I'm not worried about functionality. I'm worried that if I
>>>> there's, for example, an ARM based setup which uses DT and wants to use
>>>> a similar QCA6390 board that I have, and set
>>>> qcom,ath11k-calibration-variant in DT. In other words, I'm worried if
>>>> you are looking at this only for Snapdragon family of boards?
>>>>
>>>
>>> No, what I'm looking at is the entire QCA6390 package. That means WLAN
>>> *and* Bluetooth *and* the PMU that manages power.
>>
>> I think we are just looking at this from different point of views. You
>> are looking at a datasheet (most likely for a Snapdragon based system)
>> and I'm looking what actual devices there are out in the field.
>>
>>> If you're using the QCA6390 on a device-tree system then you should
>>> probably model at least the WLAN node and the PMU and the problem with
>>> supplies is fixed.
>>
>> But why? If there are boards out there who don't need any of this why
>> would they still need to model all this in DT?
>>
> 
> Because this is what is there? The goal of the device tree is to
> describe the hardware. The fact we didn't describe it before doesn't
> make it correct.

Correct.

Kalle,
All of the devices out there need these supplies, but they are sometimes
provided by generic PCI supply and on-board regulators. Basically your
PCI adapter is not the same as QCA6390 chip on Snapdragon board.


> 
>> Based on the discussions I have heard only Snapdragon systems who
>> require all this configuration you describe. Of course there can be
>> other systems but I have not heard about those.
>>
> 
> DT is not configuration, it is description of actual hardware. It
> doesn't matter if Snapdragon systems are the only ones that actually
> *require* this description to make WLAN/BT functional upstream. The
> chipset would be the same on any PCIe board, it's just that the host
> systems wouldn't need to take care with its power sequence. But for a
> dynamic board like this, you don't need DT.
> 

Correct.

...

> 
>>> If your detachable board "just works" then it must be wired in a way
>>> that enables WLAN the moment it's plugged in but this doesn't happen
>>> over PCI. The chipset has a power input and GPIOs to enable each
>>> module.
>>
>> I don't know how the boards are implemented but it could be so. But from
>> host system point of view it's just a regular PCI device.
>>
> 
> And you don't need DT anyway for this type of devices.

Detechable board, like PCI adapter, derives these supplies from generic
PCI whatever-3.3v through additional regulators. All these supplies are
there - on the board.

> 
>>> Also: I doubt you need DT for your detachable board?
>>
>> Sure, I don't need DT but that's not my point. My point is why require
>> these supplies for _all_ devices having PCI id 17cb:1101 (ie. QCA6390)
>> then clearly there are such devices which don't need it? To me that's
>> bad design and, if I'm understanding correctly, prevents use of
>> qcom,ath11k-calibration-variant property. To me having the supplies
>> optional in DT is more approriate.
>>
> 
> We require them because *they are physically there*.

I understand that for all known DT QCA6390 hardware, the supplies should
be provided thus they should be required. If in the future we have
different design or we represent some pluggable PCI card, then:
1. Probably that PCI card does not need power sequencing, thus no DT
description,
2. If still needs power sequencing, you can always amend bindings and
un-require the supplies.


Best regards,
Krzysztof

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Bartosz Golaszewski]

On Fri, Jun 7, 2024 at 8:40 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> Kalle,
> All of the devices out there need these supplies, but they are sometimes
> provided by generic PCI supply and on-board regulators. Basically your
> PCI adapter is not the same as QCA6390 chip on Snapdragon board.
>
>
> >
> >> Based on the discussions I have heard only Snapdragon systems who
> >> require all this configuration you describe. Of course there can be
> >> other systems but I have not heard about those.
> >>
> >
> > DT is not configuration, it is description of actual hardware. It
> > doesn't matter if Snapdragon systems are the only ones that actually
> > *require* this description to make WLAN/BT functional upstream. The
> > chipset would be the same on any PCIe board, it's just that the host
> > systems wouldn't need to take care with its power sequence. But for a
> > dynamic board like this, you don't need DT.
> >
>
> Correct.
>
> ...
>
> >
> >>> If your detachable board "just works" then it must be wired in a way
> >>> that enables WLAN the moment it's plugged in but this doesn't happen
> >>> over PCI. The chipset has a power input and GPIOs to enable each
> >>> module.
> >>
> >> I don't know how the boards are implemented but it could be so. But from
> >> host system point of view it's just a regular PCI device.
> >>
> >
> > And you don't need DT anyway for this type of devices.
>
> Detechable board, like PCI adapter, derives these supplies from generic
> PCI whatever-3.3v through additional regulators. All these supplies are
> there - on the board.
>
> >
> >>> Also: I doubt you need DT for your detachable board?
> >>
> >> Sure, I don't need DT but that's not my point. My point is why require
> >> these supplies for _all_ devices having PCI id 17cb:1101 (ie. QCA6390)
> >> then clearly there are such devices which don't need it? To me that's
> >> bad design and, if I'm understanding correctly, prevents use of
> >> qcom,ath11k-calibration-variant property. To me having the supplies
> >> optional in DT is more approriate.
> >>
> >
> > We require them because *they are physically there*.
>
> I understand that for all known DT QCA6390 hardware, the supplies should
> be provided thus they should be required. If in the future we have
> different design or we represent some pluggable PCI card, then:
> 1. Probably that PCI card does not need power sequencing, thus no DT
> description,
> 2. If still needs power sequencing, you can always amend bindings and
> un-require the supplies.
>
>
> Best regards,
> Krzysztof
>

Kalle, does the above answer your questions? Are these bindings good to go?

Bart

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Kalle Valo]

Bartosz Golaszewski <brgl@bgdev.pl> writes:

> On Thu, Jun 6, 2024 at 6:16 PM Kalle Valo <kvalo@kernel.org> wrote:
>
>>
>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>
>> > On Thu, Jun 6, 2024 at 4:02 PM Kalle Valo <kvalo@kernel.org> wrote:
>> >
>> >> Sure, I'm not worried about functionality. I'm worried that if I
>> >> there's, for example, an ARM based setup which uses DT and wants to use
>> >> a similar QCA6390 board that I have, and set
>> >> qcom,ath11k-calibration-variant in DT. In other words, I'm worried if
>> >> you are looking at this only for Snapdragon family of boards?
>> >>
>> >
>> > No, what I'm looking at is the entire QCA6390 package. That means WLAN
>> > *and* Bluetooth *and* the PMU that manages power.
>>
>> I think we are just looking at this from different point of views. You
>> are looking at a datasheet (most likely for a Snapdragon based system)
>> and I'm looking what actual devices there are out in the field.
>>
>> > If you're using the QCA6390 on a device-tree system then you should
>> > probably model at least the WLAN node and the PMU and the problem with
>> > supplies is fixed.
>>
>> But why? If there are boards out there who don't need any of this why
>> would they still need to model all this in DT?
>>
>
> Because this is what is there? The goal of the device tree is to
> describe the hardware. The fact we didn't describe it before doesn't
> make it correct.
>
>> Based on the discussions I have heard only Snapdragon systems who
>> require all this configuration you describe. Of course there can be
>> other systems but I have not heard about those.
>>
>
> DT is not configuration, it is description of actual hardware. It
> doesn't matter if Snapdragon systems are the only ones that actually
> *require* this description to make WLAN/BT functional upstream. The
> chipset would be the same on any PCIe board, it's just that the host
> systems wouldn't need to take care with its power sequence. But for a
> dynamic board like this, you don't need DT.
>
>> > But if you don't have the supplies, that's alright for downstream.
>>
>> What do you mean downstream in this context?
>>
>
> I mean: if you wanted to upstream the DT sources, then they should
> include the supplies AND the PMU node. But if you just want to make
> the WLAN run on some vendor kernel then you don't need to think about
> it, it will work.
>
>> >> Again, I don't see this as a blocker. I just want to understand how this
>> >> should work for all types of devices there are out there.
>> >>
>> >> > But if you have a QCA6390 then you have its PMU too and the bindings
>> >> > model the real-world hardware.
>> >> >
>> >> > IOW: your laptop should be alright but the supplies are really there
>> >> > which warrants adding them to the bindings.
>> >>
>> >> Sorry, not following here. Can you clarify your comment "the supplies
>> >> are really there"? You mean inside the PCI board? But that's not visible
>> >> to the kernel in anyway, the PCI board just works after I plug it in.
>> >> It's like a regular PCI device. So I don't understand why that should be
>> >> visible in DT, but I can very well be missing something.
>> >>
>> >
>> > I think you're thinking about some kind of detachable PCIe board with
>> > this chipset on it.
>>
>> Exactly, a lot of WLAN boards are like this.
>>
>> > I refer to the QCA6390 chipset itself which is also more than just
>> > PCI. The Bluetooth interface doesn't use PCI at all. On the boards I'm
>> > working on, the chipset is just soldered to the main board.
>>
>> And I guess you are looking at Snapdragon boards only?
>>
>
> But what is your point?

My point (again) is that to me it look likes that you are looking this
only for Snapdragon type of devices and ignoring the rest. I am looking
at this to support _all_ type of devices and I want to make sure that we
don't have any artificial restrictions to use ath11k or ath12k devices
in upstream Linux.

I could not find a public example of a QCA6390 M.2 board like I have, but
here's one for QCA2066:

https://compex.com.sg/shop/wifi-module/wlt206h-wifi6-ble5-1-11ax-qca2062-qca2065/

QCA2066 is a mobile chipset supported by ath11k, similarly like QCA6390.
It's just newer and different features, and with a different PCI id. In
the past using these kind of M.2 boards for Wi-Fi has been quite common
but don't know how commit it is nowadays.

>> > If your detachable board "just works" then it must be wired in a way
>> > that enables WLAN the moment it's plugged in but this doesn't happen
>> > over PCI. The chipset has a power input and GPIOs to enable each
>> > module.
>>
>> I don't know how the boards are implemented but it could be so. But from
>> host system point of view it's just a regular PCI device.
>>
>
> And you don't need DT anyway for this type of devices.

I wish we wouldn't need to use DT for such M.2 boards, but we do need to
use qcom,ath11k-calibration-variant in some cases when the device (or
the firmware) doesn't provide unique enough identifier to choose the
correct board file automatically. I already mentioned the property in my
earlier emails.

I hope this clears up what I'm trying to say.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Kalle Valo]

Bartosz Golaszewski <brgl@bgdev.pl> writes:

>> >> Sure, I don't need DT but that's not my point. My point is why require
>> >> these supplies for _all_ devices having PCI id 17cb:1101 (ie. QCA6390)
>> >> then clearly there are such devices which don't need it? To me that's
>> >> bad design and, if I'm understanding correctly, prevents use of
>> >> qcom,ath11k-calibration-variant property. To me having the supplies
>> >> optional in DT is more approriate.
>> >>
>> >
>> > We require them because *they are physically there*.
>>
>> I understand that for all known DT QCA6390 hardware, the supplies should
>> be provided thus they should be required. If in the future we have
>> different design or we represent some pluggable PCI card, then:
>> 1. Probably that PCI card does not need power sequencing, thus no DT
>> description,
>> 2. If still needs power sequencing, you can always amend bindings and
>> un-require the supplies.
>>
>>
>> Best regards,
>> Krzysztof
>>
>
> Kalle, does the above answer your questions? Are these bindings good to go?

To me most important is that we are on the same page that in some cases
(eg. with M.2 boards) the supplies can be optional and we can update the
bindings doc once such need arises (but we don't make any changes right
now). Based on point 2 from Krzysztof I think we all agree, right?

Just making sure: if we later change the supplies optional does that
create any problems with backwards compatibility? It's important that
updates go smoothly.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Bartosz Golaszewski]

On Wed, Jun 12, 2024 at 2:49 PM Kalle Valo <kvalo@kernel.org> wrote:
>
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>
> >> >> Sure, I don't need DT but that's not my point. My point is why require
> >> >> these supplies for _all_ devices having PCI id 17cb:1101 (ie. QCA6390)
> >> >> then clearly there are such devices which don't need it? To me that's
> >> >> bad design and, if I'm understanding correctly, prevents use of
> >> >> qcom,ath11k-calibration-variant property. To me having the supplies
> >> >> optional in DT is more approriate.
> >> >>
> >> >
> >> > We require them because *they are physically there*.
> >>
> >> I understand that for all known DT QCA6390 hardware, the supplies should
> >> be provided thus they should be required. If in the future we have
> >> different design or we represent some pluggable PCI card, then:
> >> 1. Probably that PCI card does not need power sequencing, thus no DT
> >> description,
> >> 2. If still needs power sequencing, you can always amend bindings and
> >> un-require the supplies.
> >>
> >>
> >> Best regards,
> >> Krzysztof
> >>
> >
> > Kalle, does the above answer your questions? Are these bindings good to go?
>
> To me most important is that we are on the same page that in some cases
> (eg. with M.2 boards) the supplies can be optional and we can update the
> bindings doc once such need arises (but we don't make any changes right
> now). Based on point 2 from Krzysztof I think we all agree, right?
>
> Just making sure: if we later change the supplies optional does that
> create any problems with backwards compatibility? It's important that
> updates go smoothly.

No, you can always relax the requirements alright. It's only when you
make them more strict that you'll run into backward compatibility
issues.

Bart

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Bartosz Golaszewski]

On Wed, Jun 12, 2024 at 2:52 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Wed, Jun 12, 2024 at 2:49 PM Kalle Valo <kvalo@kernel.org> wrote:
> >
> > Bartosz Golaszewski <brgl@bgdev.pl> writes:
> >
> > >> >> Sure, I don't need DT but that's not my point. My point is why require
> > >> >> these supplies for _all_ devices having PCI id 17cb:1101 (ie. QCA6390)
> > >> >> then clearly there are such devices which don't need it? To me that's
> > >> >> bad design and, if I'm understanding correctly, prevents use of
> > >> >> qcom,ath11k-calibration-variant property. To me having the supplies
> > >> >> optional in DT is more approriate.
> > >> >>
> > >> >
> > >> > We require them because *they are physically there*.
> > >>
> > >> I understand that for all known DT QCA6390 hardware, the supplies should
> > >> be provided thus they should be required. If in the future we have
> > >> different design or we represent some pluggable PCI card, then:
> > >> 1. Probably that PCI card does not need power sequencing, thus no DT
> > >> description,
> > >> 2. If still needs power sequencing, you can always amend bindings and
> > >> un-require the supplies.
> > >>
> > >>
> > >> Best regards,
> > >> Krzysztof
> > >>
> > >
> > > Kalle, does the above answer your questions? Are these bindings good to go?
> >
> > To me most important is that we are on the same page that in some cases
> > (eg. with M.2 boards) the supplies can be optional and we can update the
> > bindings doc once such need arises (but we don't make any changes right
> > now). Based on point 2 from Krzysztof I think we all agree, right?
> >
> > Just making sure: if we later change the supplies optional does that
> > create any problems with backwards compatibility? It's important that
> > updates go smoothly.
>
> No, you can always relax the requirements alright. It's only when you
> make them more strict that you'll run into backward compatibility
> issues.
>
> Bart

Kalle,

Is that ok with you? Can we get that queued to avoid the new
check_dtbs warnings in next when the DTS changes land?

Bart

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Kalle Valo]

Bartosz Golaszewski <brgl@bgdev.pl> writes:

> On Wed, Jun 12, 2024 at 2:52 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>
>> On Wed, Jun 12, 2024 at 2:49 PM Kalle Valo <kvalo@kernel.org> wrote:
>> >
>> > Bartosz Golaszewski <brgl@bgdev.pl> writes:
>> >
>> > >> >> Sure, I don't need DT but that's not my point. My point is why require
>> > >> >> these supplies for _all_ devices having PCI id 17cb:1101 (ie. QCA6390)
>> > >> >> then clearly there are such devices which don't need it? To me that's
>> > >> >> bad design and, if I'm understanding correctly, prevents use of
>> > >> >> qcom,ath11k-calibration-variant property. To me having the supplies
>> > >> >> optional in DT is more approriate.
>> > >> >>
>> > >> >
>> > >> > We require them because *they are physically there*.
>> > >>
>> > >> I understand that for all known DT QCA6390 hardware, the supplies should
>> > >> be provided thus they should be required. If in the future we have
>> > >> different design or we represent some pluggable PCI card, then:
>> > >> 1. Probably that PCI card does not need power sequencing, thus no DT
>> > >> description,
>> > >> 2. If still needs power sequencing, you can always amend bindings and
>> > >> un-require the supplies.
>> > >>
>> > >>
>> > >> Best regards,
>> > >> Krzysztof
>> > >>
>> > >
>> > > Kalle, does the above answer your questions? Are these bindings good to go?
>> >
>> > To me most important is that we are on the same page that in some cases
>> > (eg. with M.2 boards) the supplies can be optional and we can update the
>> > bindings doc once such need arises (but we don't make any changes right
>> > now). Based on point 2 from Krzysztof I think we all agree, right?
>> >
>> > Just making sure: if we later change the supplies optional does that
>> > create any problems with backwards compatibility? It's important that
>> > updates go smoothly.
>>
>> No, you can always relax the requirements alright. It's only when you
>> make them more strict that you'll run into backward compatibility
>> issues.
>>
>> Bart
>
> Kalle,
>
> Is that ok with you? Can we get that queued to avoid the new
> check_dtbs warnings in next when the DTS changes land?

Yes, this patchset is already on our pending branch and should be
applied soon. I was on a long weekend hence the delay.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Kalle Valo]

Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> Add a PCI compatible for the ATH11K module on QCA6390 and describe the
> power inputs from the PMU that it consumes.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

2 patches applied to ath-next branch of ath.git, thanks.

71839a929d9e dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
aa17d384971b dt-bindings: net: wireless: describe the ath12k PCI module

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20240605122106.23818-2-brgl@bgdev.pl/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Krzysztof Kozlowski]

On 17/06/2024 13:09, Kalle Valo wrote:
> Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> 
>> Add a PCI compatible for the ATH11K module on QCA6390 and describe the
>> power inputs from the PMU that it consumes.
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
> 
> 2 patches applied to ath-next branch of ath.git, thanks.

Hi Kalle,

Are you sure your tree is properly fed to linux-next? I cannot find
these patches in linux-next and above repo is not listed in Next/Trees.

Every maintainer tree should (IMHO: *MUST*) be fed to linux-next.

You will also get LKP coverage for free, unless your tree is there due
to scanning korg.

Please look at slides here and implement at least linux-next (although I
also encourage to opt-in to transparency log and LKP):
https://lpc.events/event/17/contributions/1498/

Best regards,
Krzysztof

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Kalle Valo]

Krzysztof Kozlowski <krzk@kernel.org> writes:

> On 17/06/2024 13:09, Kalle Valo wrote:
>> Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> 
>>> Add a PCI compatible for the ATH11K module on QCA6390 and describe the
>>> power inputs from the PMU that it consumes.
>>>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
>> 
>> 2 patches applied to ath-next branch of ath.git, thanks.
>
> Hi Kalle,
>
> Are you sure your tree is properly fed to linux-next? I cannot find
> these patches in linux-next and above repo is not listed in Next/Trees.

ath.git is not included in linux-next builds. To my knowledge wireless
and wireless-next trees are the only trees from the wireless subsystem
which are included, all driver trees are not. But Jeff and I are talking
about including ath.git to linux-next.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v9 1/2] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

[Krzysztof Kozlowski]

On 24/06/2024 11:15, Kalle Valo wrote:
> Krzysztof Kozlowski <krzk@kernel.org> writes:
> 
>> On 17/06/2024 13:09, Kalle Valo wrote:
>>> Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>>
>>>> Add a PCI compatible for the ATH11K module on QCA6390 and describe the
>>>> power inputs from the PMU that it consumes.
>>>>
>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
>>>
>>> 2 patches applied to ath-next branch of ath.git, thanks.
>>
>> Hi Kalle,
>>
>> Are you sure your tree is properly fed to linux-next? I cannot find
>> these patches in linux-next and above repo is not listed in Next/Trees.
> 
> ath.git is not included in linux-next builds. To my knowledge wireless
> and wireless-next trees are the only trees from the wireless subsystem
> which are included, all driver trees are not. But Jeff and I are talking
> about including ath.git to linux-next.

Thanks for confirming. There is not much to "talk". Just send one email.
There is no single issue stopping you from being in linux-next. The only
work/caveat is when wireless-next cherry-picks your patches instead of
git pull, but even then you can arrange with Stephen to opt-out from
emails about duplicated commits.

Of course this is not specific to Ath - as you said - all wireless
sub-trees should be fixed. It's really odd that these are not in linux-next.

Best regards,
Krzysztof