diff mbox series

[net-next,v6,1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: Add J721e CPSW9G support

Message ID 20230104103432.1126403-2-s-vadapalli@ti.com (mailing list archive)
State New, archived
Headers show
Series Add support for QSGMII mode for J721e CPSW9G to am65-cpsw driver | expand

Commit Message

s-vadapalli Jan. 4, 2023, 10:34 a.m. UTC
Update bindings for TI K3 J721e SoC which contains 9 ports (8 external
ports) CPSW9G module and add compatible for it.

Changes made:
    - Add new compatible ti,j721e-cpswxg-nuss for CPSW9G.
    - Extend pattern properties for new compatible.
    - Change maximum number of CPSW ports to 8 for new compatible.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   | 33 ++++++++++++++++---
 1 file changed, 29 insertions(+), 4 deletions(-)

Comments

Geert Uytterhoeven Jan. 17, 2023, 1:45 p.m. UTC | #1
Hi Siddharth,

On Wed, Jan 4, 2023 at 11:37 AM Siddharth Vadapalli <s-vadapalli@ti.com> wrote:
> Update bindings for TI K3 J721e SoC which contains 9 ports (8 external
> ports) CPSW9G module and add compatible for it.
>
> Changes made:
>     - Add new compatible ti,j721e-cpswxg-nuss for CPSW9G.
>     - Extend pattern properties for new compatible.
>     - Change maximum number of CPSW ports to 8 for new compatible.
>
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Thanks for your patch, which is now commit c85b53e32c8ecfe6
("dt-bindings: net: ti: k3-am654-cpsw-nuss: Add J721e CPSW9G
support") in net-next.

You forgot to document the presence of the new optional
"serdes-phy" PHY.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Krzysztof Kozlowski Jan. 17, 2023, 5:17 p.m. UTC | #2
On 17/01/2023 14:45, Geert Uytterhoeven wrote:
> Hi Siddharth,
> 
> On Wed, Jan 4, 2023 at 11:37 AM Siddharth Vadapalli <s-vadapalli@ti.com> wrote:
>> Update bindings for TI K3 J721e SoC which contains 9 ports (8 external
>> ports) CPSW9G module and add compatible for it.
>>
>> Changes made:
>>     - Add new compatible ti,j721e-cpswxg-nuss for CPSW9G.
>>     - Extend pattern properties for new compatible.
>>     - Change maximum number of CPSW ports to 8 for new compatible.
>>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> Reviewed-by: Rob Herring <robh@kernel.org>
> 
> Thanks for your patch, which is now commit c85b53e32c8ecfe6
> ("dt-bindings: net: ti: k3-am654-cpsw-nuss: Add J721e CPSW9G
> support") in net-next.
> 
> You forgot to document the presence of the new optional
> "serdes-phy" PHY.

I think we should start rejecting most of bindings without DTS, because
submitters really like to forget to make complete bindings. Having a DTS
with such undocumented property gives a bit bigger chance it will get an
attention. :(

Best regards,
Krzysztof
Vignesh Raghavendra Jan. 18, 2023, 5:21 p.m. UTC | #3
On 1/17/2023 10:47 PM, Krzysztof Kozlowski wrote:
> On 17/01/2023 14:45, Geert Uytterhoeven wrote:
>> Hi Siddharth,
>>
>> On Wed, Jan 4, 2023 at 11:37 AM Siddharth Vadapalli <s-vadapalli@ti.com> wrote:
>>> Update bindings for TI K3 J721e SoC which contains 9 ports (8 external
>>> ports) CPSW9G module and add compatible for it.
>>>
>>> Changes made:
>>>     - Add new compatible ti,j721e-cpswxg-nuss for CPSW9G.
>>>     - Extend pattern properties for new compatible.
>>>     - Change maximum number of CPSW ports to 8 for new compatible.
>>>
>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>
>> Thanks for your patch, which is now commit c85b53e32c8ecfe6
>> ("dt-bindings: net: ti: k3-am654-cpsw-nuss: Add J721e CPSW9G
>> support") in net-next.
>>
>> You forgot to document the presence of the new optional
>> "serdes-phy" PHY.
> 
> I think we should start rejecting most of bindings without DTS, because
> submitters really like to forget to make complete bindings. Having a DTS
> with such undocumented property gives a bit bigger chance it will get an
> attention. :(
> 

Agree, bindings should have been better tested against real DTS.

But for reviewers, this been a bit of chicken-egg problem. Bindings and
driver changes have to go in first and via "subsystem" trees while DTS
patches have to go via "arch" tree. So, they get posted separately.

One may not see DTS patches (and thus user of the bindings) until
bindings reach Torvalds' tree. So, user of bindings will only appear in
the next kernel release cycle (at which time they do get flagged due to
failing make dtbs_check but its bit late). Wondering how others are
managing the same ?


Regards
Vignesh
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
index 821974815dec..900063411a20 100644
--- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
+++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
@@ -57,6 +57,7 @@  properties:
       - ti,am654-cpsw-nuss
       - ti,j7200-cpswxg-nuss
       - ti,j721e-cpsw-nuss
+      - ti,j721e-cpswxg-nuss
       - ti,am642-cpsw-nuss
 
   reg:
@@ -111,7 +112,7 @@  properties:
         const: 0
 
     patternProperties:
-      "^port@[1-4]$":
+      "^port@[1-8]$":
         type: object
         description: CPSWxG NUSS external ports
 
@@ -121,7 +122,7 @@  properties:
         properties:
           reg:
             minimum: 1
-            maximum: 4
+            maximum: 8
             description: CPSW port number
 
           phys:
@@ -186,12 +187,36 @@  allOf:
         properties:
           compatible:
             contains:
-              const: ti,j7200-cpswxg-nuss
+              const: ti,j721e-cpswxg-nuss
     then:
       properties:
         ethernet-ports:
           patternProperties:
-            "^port@[3-4]$": false
+            "^port@[5-8]$": false
+            "^port@[1-4]$":
+              properties:
+                reg:
+                  minimum: 1
+                  maximum: 4
+
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              enum:
+                - ti,j721e-cpswxg-nuss
+                - ti,j7200-cpswxg-nuss
+    then:
+      properties:
+        ethernet-ports:
+          patternProperties:
+            "^port@[3-8]$": false
+            "^port@[1-2]$":
+              properties:
+                reg:
+                  minimum: 1
+                  maximum: 2
 
 additionalProperties: false