1. Patchew
  2. linux
  3. [PATCHv2 0/5] wifi: ath9k: add ahb OF support
  4. View patch

[PATCHv2 4/5] dt-bindings: net: wireless: ath9k: add OF bindings

Rosen Penev posted 5 patches 6 months, 3 weeks ago
There is a newer version of this series
[PATCHv2 4/5] dt-bindings: net: wireless: ath9k: add OF bindings
Posted by Rosen Penev 6 months, 3 weeks ago 
Now that support was added to the driver, document it.
---
 .../bindings/net/wireless/qca,ath9k.yaml      | 23 ++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
index 0e5412cff2bc..81d00f257922 100644
--- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
@@ -12,7 +12,7 @@ maintainers:
 description: |
   This node provides properties for configuring the ath9k wireless device.
   The node is expected to be specified as a child node of the PCI controller
-  to which the wireless chip is connected.
+  or AHB bus to which the wireless chip is connected.
 
 allOf:
   - $ref: ieee80211.yaml#
@@ -35,6 +35,12 @@ properties:
       - pci168c,0034  # AR9462
       - pci168c,0036  # AR9565
       - pci168c,0037  # AR1111 and AR9485
+      - qca,ar9130-wmac
+      - qca,ar9330-wmac
+      - qca,ar9340-wmac
+      - qca,qca9530-wmac
+      - qca,qca9550-wmac
+      - qca,qca9560-wmac
 
   reg:
     maxItems: 1
@@ -88,3 +94,18 @@ examples:
         nvmem-cell-names = "mac-address", "calibration";
       };
     };
+  - |
+    apb {
+      compatible = "simple-bus";
+      ranges;
+
+      #address-cells = <1>;
+      #size-cells = <1>;
+
+      wifi@180c0000 {
+        compatible = "qca,ar9130-wmac";
+        reg = <0x180c0000 0x230000>;
+
+        interrupts = <2>;
+      };
+    };
-- 
2.49.0
Re: [PATCHv2 4/5] dt-bindings: net: wireless: ath9k: add OF bindings
Posted by Krzysztof Kozlowski 6 months, 3 weeks ago 
On 22/05/2025 20:45, Rosen Penev wrote:
> Now that support was added to the driver, document it.

That's not appropriate commit msg. Binding must be before the user (see
submitting patches in DT directory). Describe the hardware, what are you
adding here.

Subject: OF bindings is redundant. It duplicates dt-bindings. Instead:
"Add Atheros AR9-foo-bar on AHB bus" or something similar

Missing SoB.

Please run scripts/checkpatch.pl on the patches and fix reported
warnings. After that, run also 'scripts/checkpatch.pl --strict' on the
patches and (probably) fix more warnings. Some warnings can be ignored,
especially from --strict run, but the code here looks like it needs a
fix. Feel free to get in touch if the warning is not clear.



> ---
>  .../bindings/net/wireless/qca,ath9k.yaml      | 23 ++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> index 0e5412cff2bc..81d00f257922 100644
> --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> @@ -12,7 +12,7 @@ maintainers:
>  description: |
>    This node provides properties for configuring the ath9k wireless device.
>    The node is expected to be specified as a child node of the PCI controller
> -  to which the wireless chip is connected.
> +  or AHB bus to which the wireless chip is connected.
>  
>  allOf:
>    - $ref: ieee80211.yaml#
> @@ -35,6 +35,12 @@ properties:
>        - pci168c,0034  # AR9462
>        - pci168c,0036  # AR9565
>        - pci168c,0037  # AR1111 and AR9485
> +      - qca,ar9130-wmac
> +      - qca,ar9330-wmac
> +      - qca,ar9340-wmac
> +      - qca,qca9530-wmac
> +      - qca,qca9550-wmac
> +      - qca,qca9560-wmac
>  
>    reg:
>      maxItems: 1
> @@ -88,3 +94,18 @@ examples:
>          nvmem-cell-names = "mac-address", "calibration";
>        };
>      };
> +  - |
> +    apb {
> +      compatible = "simple-bus";
> +      ranges;
> +

Drop these two.

> +      #address-cells = <1>;
> +      #size-cells = <1>;

Best regards,
Krzysztof
Re: [PATCHv2 4/5] dt-bindings: net: wireless: ath9k: add OF bindings
Posted by Rosen Penev 6 months, 3 weeks ago 
On Thu, May 22, 2025 at 12:54 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 22/05/2025 20:45, Rosen Penev wrote:
> > Now that support was added to the driver, document it.
>
> That's not appropriate commit msg. Binding must be before the user (see
> submitting patches in DT directory). Describe the hardware, what are you
> adding here.
>
> Subject: OF bindings is redundant. It duplicates dt-bindings. Instead:
> "Add Atheros AR9-foo-bar on AHB bus" or something similar
At this point I wonder if my approach is wrong. The other ath drivers
use a qcom, prefix and a -wifi suffix. Might make sense to do the same
here to avoid typing qca twice.

>
> Missing SoB.
>
> Please run scripts/checkpatch.pl on the patches and fix reported
> warnings. After that, run also 'scripts/checkpatch.pl --strict' on the
> patches and (probably) fix more warnings. Some warnings can be ignored,
> especially from --strict run, but the code here looks like it needs a
> fix. Feel free to get in touch if the warning is not clear.
>
>
>
> > ---
> >  .../bindings/net/wireless/qca,ath9k.yaml      | 23 ++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> > index 0e5412cff2bc..81d00f257922 100644
> > --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> > +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> > @@ -12,7 +12,7 @@ maintainers:
> >  description: |
> >    This node provides properties for configuring the ath9k wireless device.
> >    The node is expected to be specified as a child node of the PCI controller
> > -  to which the wireless chip is connected.
> > +  or AHB bus to which the wireless chip is connected.
> >
> >  allOf:
> >    - $ref: ieee80211.yaml#
> > @@ -35,6 +35,12 @@ properties:
> >        - pci168c,0034  # AR9462
> >        - pci168c,0036  # AR9565
> >        - pci168c,0037  # AR1111 and AR9485
> > +      - qca,ar9130-wmac
> > +      - qca,ar9330-wmac
> > +      - qca,ar9340-wmac
> > +      - qca,qca9530-wmac
> > +      - qca,qca9550-wmac
> > +      - qca,qca9560-wmac
> >
> >    reg:
> >      maxItems: 1
> > @@ -88,3 +94,18 @@ examples:
> >          nvmem-cell-names = "mac-address", "calibration";
> >        };
> >      };
> > +  - |
> > +    apb {
> > +      compatible = "simple-bus";
> > +      ranges;
> > +
>
> Drop these two.
>
> > +      #address-cells = <1>;
> > +      #size-cells = <1>;
>
> Best regards,
> Krzysztof
Re: [PATCHv2 4/5] dt-bindings: net: wireless: ath9k: add OF bindings
Posted by Krzysztof Kozlowski 6 months, 3 weeks ago 
On 23/05/2025 02:04, Rosen Penev wrote:
> On Thu, May 22, 2025 at 12:54 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 22/05/2025 20:45, Rosen Penev wrote:
>>> Now that support was added to the driver, document it.
>>
>> That's not appropriate commit msg. Binding must be before the user (see
>> submitting patches in DT directory). Describe the hardware, what are you
>> adding here.
>>
>> Subject: OF bindings is redundant. It duplicates dt-bindings. Instead:
>> "Add Atheros AR9-foo-bar on AHB bus" or something similar
> At this point I wonder if my approach is wrong. The other ath drivers
> use a qcom, prefix and a -wifi suffix. Might make sense to do the same
> here to avoid typing qca twice.



Yes, probably this should be qcom.


Best regards,
Krzysztof

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.