[PATCH v6 04/16] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

Bartosz Golaszewski posted 16 patches 1 year, 10 months ago
There is a newer version of this series
[PATCH v6 04/16] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
Posted by Bartosz Golaszewski 1 year, 10 months ago
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.

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
Re: [PATCH v6 04/16] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
Posted by Kalle Valo 1 year, 10 months ago
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.
>
> 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

I don't know DT well enough to know what the "required:" above means,
but does this take into account that there are normal "plug&play" type
of QCA6390 boards as well which don't need any DT settings?

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

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v6 04/16] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
Posted by Bartosz Golaszewski 1 year, 10 months ago
On Mon, Mar 25, 2024 at 2:57 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.
> >
> > 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
>
> I don't know DT well enough to know what the "required:" above means,
> but does this take into account that there are normal "plug&play" type
> of QCA6390 boards as well which don't need any DT settings?
>

Do they require a DT node though for some reason?

Bart
Re: [PATCH v6 04/16] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
Posted by Jeff Johnson 1 year, 10 months ago
On 3/25/2024 7:09 AM, Bartosz Golaszewski wrote:
> On Mon, Mar 25, 2024 at 2:57 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.
>>>
>>> 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
>>
>> I don't know DT well enough to know what the "required:" above means,
>> but does this take into account that there are normal "plug&play" type
>> of QCA6390 boards as well which don't need any DT settings?
>>
> 
> Do they require a DT node though for some reason?

I would not expect the "PC" flavor of the card to require DT.
The "mobile" and "automotive" flavors would probably require it.

Re: [PATCH v6 04/16] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
Posted by Kalle Valo 1 year, 10 months ago
Bartosz Golaszewski <brgl@bgdev.pl> writes:

> On Mon, Mar 25, 2024 at 2:57 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.
>> >
>> > 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
>>
>> I don't know DT well enough to know what the "required:" above means,
>> but does this take into account that there are normal "plug&play" type
>> of QCA6390 boards as well which don't need any DT settings?
>
> Do they require a DT node though for some reason?

You can attach the device to any PCI slot, connect the WLAN antenna and
it just works without DT nodes. I'm trying to make sure here that basic
setup still works.

Adding also Johan and ath11k list. For example, I don't know what's the
plan with Lenovo X13s, will it use this framework? I guess in theory we
could have devices which use qcom,ath11k-calibration-variant from DT but
not any of these supply properties?

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

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v6 04/16] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
Posted by Johan Hovold 1 year, 10 months ago
On Mon, Mar 25, 2024 at 04:37:29PM +0200, Kalle Valo wrote:
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
> > On Mon, Mar 25, 2024 at 2:57 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.
> >> >
> >> > 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
> >>
> >> I don't know DT well enough to know what the "required:" above means,
> >> but does this take into account that there are normal "plug&play" type
> >> of QCA6390 boards as well which don't need any DT settings?
> >
> > Do they require a DT node though for some reason?
> 
> You can attach the device to any PCI slot, connect the WLAN antenna and
> it just works without DT nodes. I'm trying to make sure here that basic
> setup still works.
> 
> Adding also Johan and ath11k list. For example, I don't know what's the
> plan with Lenovo X13s, will it use this framework? I guess in theory we
> could have devices which use qcom,ath11k-calibration-variant from DT but
> not any of these supply properties?

In theory we could, but at least the WCN6855 in the X13s has a similar
set of supplies and enable gpios which are currently not fully described
in the devicetree as there has been no support for doing so thus far.
Instead we rely on the bootloader to enable the module.

I haven't had time to look at the latest attempt on adding support for
handling such resources, but eventually we'll need to address this in
some way.

Johan
Re: [PATCH v6 04/16] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
Posted by Bartosz Golaszewski 1 year, 10 months ago
On Mon, Mar 25, 2024 at 3:37 PM Kalle Valo <kvalo@kernel.org> wrote:
>
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>
> > On Mon, Mar 25, 2024 at 2:57 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.
> >> >
> >> > 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
> >>
> >> I don't know DT well enough to know what the "required:" above means,
> >> but does this take into account that there are normal "plug&play" type
> >> of QCA6390 boards as well which don't need any DT settings?
> >
> > Do they require a DT node though for some reason?
>
> You can attach the device to any PCI slot, connect the WLAN antenna and
> it just works without DT nodes. I'm trying to make sure here that basic
> setup still works.
>

Sure, definitely. I there's no DT node, then the binding doesn't apply
and the driver (the platform part of it) will not probe.

> Adding also Johan and ath11k list. For example, I don't know what's the
> plan with Lenovo X13s, will it use this framework? I guess in theory we
> could have devices which use qcom,ath11k-calibration-variant from DT but
> not any of these supply properties?
>

Good point. I will receive the X13s in a month from now. I do plan on
upstreaming correct support for WLAN and BT for it as well.

I guess we can always relax the requirements once a valid use-case appears?

Bart

> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v6 04/16] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
Posted by Kalle Valo 1 year, 10 months ago
Bartosz Golaszewski <brgl@bgdev.pl> writes:

>> >> I don't know DT well enough to know what the "required:" above means,
>> >> but does this take into account that there are normal "plug&play" type
>> >> of QCA6390 boards as well which don't need any DT settings?
>> >
>> > Do they require a DT node though for some reason?
>>
>> You can attach the device to any PCI slot, connect the WLAN antenna and
>> it just works without DT nodes. I'm trying to make sure here that basic
>> setup still works.
>>
>
> Sure, definitely. I there's no DT node, then the binding doesn't apply
> and the driver (the platform part of it) will not probe.
>
>> Adding also Johan and ath11k list. For example, I don't know what's the
>> plan with Lenovo X13s, will it use this framework? I guess in theory we
>> could have devices which use qcom,ath11k-calibration-variant from DT but
>> not any of these supply properties?
>>
>
> Good point. I will receive the X13s in a month from now. I do plan on
> upstreaming correct support for WLAN and BT for it as well.
>
> I guess we can always relax the requirements once a valid use-case appears?

I think we have such cases already now:

$ git grep ath11k-calibration-variant -- arch
arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts:     qcom,ath11k-calibration-variant = "Fairphone_5";
arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts:                     qcom,ath11k-calibration-variant = "LE_X13S";

But please do check that. I'm no DT expert :)

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

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v6 04/16] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
Posted by Bartosz Golaszewski 1 year, 10 months ago
On Tue, Mar 26, 2024 at 4:12 PM Kalle Valo <kvalo@kernel.org> wrote:
>
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>
> >> >> I don't know DT well enough to know what the "required:" above means,
> >> >> but does this take into account that there are normal "plug&play" type
> >> >> of QCA6390 boards as well which don't need any DT settings?
> >> >
> >> > Do they require a DT node though for some reason?
> >>
> >> You can attach the device to any PCI slot, connect the WLAN antenna and
> >> it just works without DT nodes. I'm trying to make sure here that basic
> >> setup still works.
> >>
> >
> > Sure, definitely. I there's no DT node, then the binding doesn't apply
> > and the driver (the platform part of it) will not probe.
> >
> >> Adding also Johan and ath11k list. For example, I don't know what's the
> >> plan with Lenovo X13s, will it use this framework? I guess in theory we
> >> could have devices which use qcom,ath11k-calibration-variant from DT but
> >> not any of these supply properties?
> >>
> >
> > Good point. I will receive the X13s in a month from now. I do plan on
> > upstreaming correct support for WLAN and BT for it as well.
> >
> > I guess we can always relax the requirements once a valid use-case appears?
>
> I think we have such cases already now:
>
> $ git grep ath11k-calibration-variant -- arch
> arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts:     qcom,ath11k-calibration-variant = "Fairphone_5";
> arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts:                     qcom,ath11k-calibration-variant = "LE_X13S";
>
> But please do check that. I'm no DT expert :)
>

You're thinking about making the required: field depend on the value
of qcom,ath11k-calibration-variant? Am I getting this right?

Bart
Re: [PATCH v6 04/16] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
Posted by Johan Hovold 1 year, 10 months ago
On Tue, Mar 26, 2024 at 05:32:55PM +0100, Bartosz Golaszewski wrote:
> On Tue, Mar 26, 2024 at 4:12 PM Kalle Valo <kvalo@kernel.org> wrote:

> > >> Adding also Johan and ath11k list. For example, I don't know what's the
> > >> plan with Lenovo X13s, will it use this framework? I guess in theory we
> > >> could have devices which use qcom,ath11k-calibration-variant from DT but
> > >> not any of these supply properties?
> > >
> > > Good point. I will receive the X13s in a month from now. I do plan on
> > > upstreaming correct support for WLAN and BT for it as well.
> > >
> > > I guess we can always relax the requirements once a valid use-case appears?
> >
> > I think we have such cases already now:
> >
> > $ git grep ath11k-calibration-variant -- arch
> > arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts:     qcom,ath11k-calibration-variant = "Fairphone_5";
> > arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts:                     qcom,ath11k-calibration-variant = "LE_X13S";
> >
> > But please do check that. I'm no DT expert :)
> 
> You're thinking about making the required: field depend on the value
> of qcom,ath11k-calibration-variant? Am I getting this right?

No, I think Kalle is worried about requiring the supply properties for
certain PCI device ids, in case we have existing or future devicetrees
with those ids that did not specify them or that need not specify them
(e.g. any PC modules).

Currently we only have the X13s controller in mainline being described
by a PCIe endpoint node in DT, but it has a different id ("pci17cb,1103"
instead of "pci17cb,1101" which you are adding here).

The Fairphone controller is apparently not a PCI device at all.

Johan