diff mbox series

[v6,05/16] dt-bindings: net: wireless: describe the ath12k PCI module

Message ID 20240325131624.26023-6-brgl@bgdev.pl
State Superseded
Headers show
Series power: sequencing: implement the subsystem and add first users | expand

Commit Message

Bartosz Golaszewski March 25, 2024, 1:16 p.m. UTC
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

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

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

Comments

Krzysztof Kozlowski March 27, 2024, 6:21 p.m. UTC | #1
On 25/03/2024 14:16, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Add device-tree bindings for the ATH12K module found in the WCN7850
> package.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  .../bindings/net/wireless/qcom,ath12k.yaml    | 100 ++++++++++++++++++
>  1 file changed, 100 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> 

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Bartosz Golaszewski April 3, 2024, 2:56 p.m. UTC | #2
On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote:
>
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > +
> > +maintainers:
> > +  - Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> IMHO it would be better to have just driver maintainers listed here.
>

Why? What's wrong with having the author of the bindings in the Cc list?

> > +required:
> > +  - compatible
> > +  - reg
> > +  - vddaon-supply
> > +  - vddwlcx-supply
> > +  - vddwlmx-supply
> > +  - vddrfacmn-supply
> > +  - vddrfa0p8-supply
> > +  - vddrfa1p2-supply
> > +  - vddrfa1p8-supply
> > +  - vddpcie0p9-supply
> > +  - vddpcie1p8-supply
>
> Same comment here as in patch 4. There are also ath12k PCI devices which
> don't need DT at all. I don't know if that should be reflected in the
> bindings doc but I want to point out this.
>

But DT bindings don't apply to devices that don't have DT nodes. This
isn't an issue at all.

Bart
Kalle Valo April 5, 2024, 9:33 a.m. UTC | #3
Bartosz Golaszewski <brgl@bgdev.pl> writes:

> On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote:
>>
>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>
>> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> >
>> > +
>> > +maintainers:
>> > +  - Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>
>> IMHO it would be better to have just driver maintainers listed here.
>>
>
> Why? What's wrong with having the author of the bindings in the Cc list?

If you want follow the ath12k development and review patches then you
can join the ath12k list. I'm not fond of having too many maintainers,
it's not really helping anything and just extra work to periodically
cleanup the silent maintainers.

I would ask the opposite question: why add you as the maintainer?
There's not even a single ath12k patch from you, nor I haven't seen you
doing any patch review or otherwise helping others related to ath12k.
Don't get me wrong, I value the work you do with this important powerseq
feature and hopefully we get it into the tree soon. But I don't see
adding you as a maintainer at this point.
Krzysztof Kozlowski April 5, 2024, 9:37 a.m. UTC | #4
On 05/04/2024 11:33, Kalle Valo wrote:
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
> 
>> On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote:
>>>
>>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>>
>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>
>>>> +
>>>> +maintainers:
>>>> +  - Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>
>>> IMHO it would be better to have just driver maintainers listed here.
>>>
>>
>> Why? What's wrong with having the author of the bindings in the Cc list?
> 
> If you want follow the ath12k development and review patches then you
> can join the ath12k list. I'm not fond of having too many maintainers,
> it's not really helping anything and just extra work to periodically
> cleanup the silent maintainers.
> 
> I would ask the opposite question: why add you as the maintainer?
> There's not even a single ath12k patch from you, nor I haven't seen you
> doing any patch review or otherwise helping others related to ath12k.
> Don't get me wrong, I value the work you do with this important powerseq
> feature and hopefully we get it into the tree soon. But I don't see
> adding you as a maintainer at this point.

This is not a maintainer of driver. This is maintainer of bindings, so
someone who has hardware, datasheets, knowledge and/or interest in
keeping the bindings accurate.

All your arguments above suggest you talk about the driver. This is not
the point here.

Best regards,
Krzysztof
Krzysztof Kozlowski April 5, 2024, 9:41 a.m. UTC | #5
On 05/04/2024 11:37, Krzysztof Kozlowski wrote:
> On 05/04/2024 11:33, Kalle Valo wrote:
>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>
>>> On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote:
>>>>
>>>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>>>
>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>>
>>>>> +
>>>>> +maintainers:
>>>>> +  - Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>
>>>> IMHO it would be better to have just driver maintainers listed here.
>>>>
>>>
>>> Why? What's wrong with having the author of the bindings in the Cc list?
>>
>> If you want follow the ath12k development and review patches then you
>> can join the ath12k list. I'm not fond of having too many maintainers,
>> it's not really helping anything and just extra work to periodically
>> cleanup the silent maintainers.
>>
>> I would ask the opposite question: why add you as the maintainer?
>> There's not even a single ath12k patch from you, nor I haven't seen you
>> doing any patch review or otherwise helping others related to ath12k.
>> Don't get me wrong, I value the work you do with this important powerseq
>> feature and hopefully we get it into the tree soon. But I don't see
>> adding you as a maintainer at this point.
> 
> This is not a maintainer of driver. This is maintainer of bindings, so
> someone who has hardware, datasheets, knowledge and/or interest in
> keeping the bindings accurate.
> 
> All your arguments above suggest you talk about the driver. This is not
> the point here.
> 

And to clarify: I do not have opinion whether Bartosz is a suitable
person here and whether driver maintainers should be instead or not. I
only want to clarify the purpose of the binding maintainer.

Best regards,
Krzysztof
Bartosz Golaszewski April 5, 2024, 9:52 a.m. UTC | #6
On Fri, Apr 5, 2024 at 11:34 AM Kalle Valo <kvalo@kernel.org> wrote:
>
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>
> > On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote:
> >>
> >> Bartosz Golaszewski <brgl@bgdev.pl> writes:
> >>
> >> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >> >
> >> > +
> >> > +maintainers:
> >> > +  - Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >>
> >> IMHO it would be better to have just driver maintainers listed here.
> >>
> >
> > Why? What's wrong with having the author of the bindings in the Cc list?
>
> If you want follow the ath12k development and review patches then you
> can join the ath12k list. I'm not fond of having too many maintainers,
> it's not really helping anything and just extra work to periodically
> cleanup the silent maintainers.
>
> I would ask the opposite question: why add you as the maintainer?
> There's not even a single ath12k patch from you, nor I haven't seen you
> doing any patch review or otherwise helping others related to ath12k.
> Don't get me wrong, I value the work you do with this important powerseq
> feature and hopefully we get it into the tree soon. But I don't see
> adding you as a maintainer at this point.
>

In addition to what Krzysztof already said about you seamingly
confusing the maintenance of the driver vs maintenance of the
device-tree bindings (IOW: structured hardware description) and in
response to your question: I don't see any functional change to any
dt-bindings neither from you nor from Jeff. Are you convinced you can
maintain and properly review any changes?

If so, I don't really care, I can drop myself and have less work.

Bartosz

> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Jeff Johnson April 5, 2024, 3:44 p.m. UTC | #7
On 4/5/2024 2:52 AM, Bartosz Golaszewski wrote:
> In addition to what Krzysztof already said about you seamingly
> confusing the maintenance of the driver vs maintenance of the
> device-tree bindings (IOW: structured hardware description) and in
> response to your question: I don't see any functional change to any
> dt-bindings neither from you nor from Jeff. Are you convinced you can
> maintain and properly review any changes?

Speaking just for myself, I know Bartosz is far more capable in this regard
than I am, so I'd expect him to be listed as a maintainer before me.

/jeff
diff mbox series

Patch

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..c0aad4815953
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
@@ -0,0 +1,100 @@ 
+# 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:
+  - Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
+  - Jeff Johnson <quic_jjohnson@quicinc.com>
+  - Kalle Valo <kvalo@kernel.org>
+
+description:
+  Qualcomm Technologies IEEE 802.11ax 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>;
+            };
+        };
+    };