2024-06-06 13:31:04

by Kalle Valo

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

Bartosz Golaszewski <[email protected]> writes:

> From: Bartosz Golaszewski <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Bartosz Golaszewski <[email protected]>

[...]

> +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


2024-06-06 13:36:06

by Bartosz Golaszewski

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

On Thu, Jun 6, 2024 at 3:30 PM Kalle Valo <[email protected]> wrote:
>
> Bartosz Golaszewski <[email protected]> writes:
>
> > From: Bartosz Golaszewski <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Bartosz Golaszewski <[email protected]>
>
> [...]
>
> > +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

2024-06-06 14:02:10

by Kalle Valo

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

Bartosz Golaszewski <[email protected]> writes:

> On Thu, Jun 6, 2024 at 3:30 PM Kalle Valo <[email protected]> wrote:
>
>>
>> Bartosz Golaszewski <[email protected]> writes:
>>
>> > From: Bartosz Golaszewski <[email protected]>
>> >
>> > 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 <[email protected]>
>> > Signed-off-by: Bartosz Golaszewski <[email protected]>
>>
>> [...]
>>
>> > +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