Message ID | 20230104103432.1126403-2-s-vadapalli@ti.com (mailing list archive) |
---|---|
State | Accepted |
Commit | c85b53e32c8ecfe6292db702ba28e8a00ca90011 |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | Add support for QSGMII mode for J721e CPSW9G to am65-cpsw driver | expand |
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
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
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 --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