| Message ID | 20220816060139.111934-2-s-vadapalli@ti.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | J7200: CPSW5G: Add support for QSGMII mode to am65-cpsw driver | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 10 of 10 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 41 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On 16/08/2022 09:01, Siddharth Vadapalli wrote: > Update bindings for TI K3 J7200 SoC which contains 5 ports (4 external > ports) CPSW5G module and add compatible for it. > > Changes made: > - Add new compatible ti,j7200-cpswxg-nuss for CPSW5G. > - Extend pattern properties for new compatible. > - Change maximum number of CPSW ports to 4 for new compatible. > > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> > --- > .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > 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 b8281d8be940..5366a367c387 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,j721e-cpsw-nuss > - ti,am642-cpsw-nuss > + - ti,j7200-cpswxg-nuss Keep some order in the list, so maybe before j721e. > > reg: > maxItems: 1 > @@ -110,7 +111,7 @@ properties: > const: 0 > > patternProperties: > - port@[1-2]: > + "^port@[1-4]$": > type: object > description: CPSWxG NUSS external ports > > @@ -119,7 +120,7 @@ properties: > properties: > reg: > minimum: 1 > - maximum: 2 > + maximum: 4 > description: CPSW port number > > phys: > @@ -151,6 +152,18 @@ properties: > > additionalProperties: false > > +if: This goes under allOf just before unevaluated/additionalProperties:false Best regards, Krzysztof
Hello Krzysztof, On 16/08/22 13:14, Krzysztof Kozlowski wrote: > On 16/08/2022 09:01, Siddharth Vadapalli wrote: >> Update bindings for TI K3 J7200 SoC which contains 5 ports (4 external >> ports) CPSW5G module and add compatible for it. >> >> Changes made: >> - Add new compatible ti,j7200-cpswxg-nuss for CPSW5G. >> - Extend pattern properties for new compatible. >> - Change maximum number of CPSW ports to 4 for new compatible. >> >> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> >> --- >> .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 17 +++++++++++++++-- >> 1 file changed, 15 insertions(+), 2 deletions(-) >> >> 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 b8281d8be940..5366a367c387 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,j721e-cpsw-nuss >> - ti,am642-cpsw-nuss >> + - ti,j7200-cpswxg-nuss > > Keep some order in the list, so maybe before j721e. Thank you for reviewing the patch. I will move ti,j7200-cpswxg-nuss above ti,j721e-cpsw-nuss in the v5 series. > >> >> reg: >> maxItems: 1 >> @@ -110,7 +111,7 @@ properties: >> const: 0 >> >> patternProperties: >> - port@[1-2]: >> + "^port@[1-4]$": >> type: object >> description: CPSWxG NUSS external ports >> >> @@ -119,7 +120,7 @@ properties: >> properties: >> reg: >> minimum: 1 >> - maximum: 2 >> + maximum: 4 >> description: CPSW port number >> >> phys: >> @@ -151,6 +152,18 @@ properties: >> >> additionalProperties: false >> >> +if: > > This goes under allOf just before unevaluated/additionalProperties:false allOf was added by me in v3 series patch and it is not present in the file. I removed it in v4 after Rob Herring's suggestion. Please let me know if simply moving the if-then statements to the line above additionalProperties:false would be fine. Regards, Siddharth.
On 17/08/2022 08:14, Siddharth Vadapalli wrote: >>> - port@[1-2]: >>> + "^port@[1-4]$": >>> type: object >>> description: CPSWxG NUSS external ports >>> >>> @@ -119,7 +120,7 @@ properties: >>> properties: >>> reg: >>> minimum: 1 >>> - maximum: 2 >>> + maximum: 4 >>> description: CPSW port number >>> >>> phys: >>> @@ -151,6 +152,18 @@ properties: >>> >>> additionalProperties: false >>> >>> +if: >> >> This goes under allOf just before unevaluated/additionalProperties:false > > allOf was added by me in v3 series patch and it is not present in the > file. I removed it in v4 after Rob Herring's suggestion. Please let me > know if simply moving the if-then statements to the line above > additionalProperties:false would be fine. I think Rob's comment was focusing not on using or not-using allOf, but on format of your entire if-then-else. Your v3 was huge and included allOf in wrong place). Now you add if-then in proper place, but it is still advisable to put it with allOf, so if ever you grow the if-then by new entry, you do not have to change the indentation. Anyway the location is not correct. Regardless if this is if-then or allOf-if-then, put it just like example schema is suggesting. Best regards, Krzysztof
Hello Krzysztof, On 17/08/22 11:20, Krzysztof Kozlowski wrote: > On 17/08/2022 08:14, Siddharth Vadapalli wrote: > >>>> - port@[1-2]: >>>> + "^port@[1-4]$": >>>> type: object >>>> description: CPSWxG NUSS external ports >>>> >>>> @@ -119,7 +120,7 @@ properties: >>>> properties: >>>> reg: >>>> minimum: 1 >>>> - maximum: 2 >>>> + maximum: 4 >>>> description: CPSW port number >>>> >>>> phys: >>>> @@ -151,6 +152,18 @@ properties: >>>> >>>> additionalProperties: false >>>> >>>> +if: >>> >>> This goes under allOf just before unevaluated/additionalProperties:false >> >> allOf was added by me in v3 series patch and it is not present in the >> file. I removed it in v4 after Rob Herring's suggestion. Please let me >> know if simply moving the if-then statements to the line above >> additionalProperties:false would be fine. > > I think Rob's comment was focusing not on using or not-using allOf, but > on format of your entire if-then-else. Your v3 was huge and included > allOf in wrong place). > > Now you add if-then in proper place, but it is still advisable to put it > with allOf, so if ever you grow the if-then by new entry, you do not > have to change the indentation. > > Anyway the location is not correct. Regardless if this is if-then or > allOf-if-then, put it just like example schema is suggesting. I will move the if-then statements to the lines above the "additionalProperties: false" line. Also, I will add an allOf for this single if-then statement in the v5 series. Regards, Siddharth.
Hello Krzysztof, On 17/08/22 13:11, Siddharth Vadapalli wrote: > Hello Krzysztof, > > On 17/08/22 11:20, Krzysztof Kozlowski wrote: >> On 17/08/2022 08:14, Siddharth Vadapalli wrote: >> >>>>> - port@[1-2]: >>>>> + "^port@[1-4]$": >>>>> type: object >>>>> description: CPSWxG NUSS external ports >>>>> >>>>> @@ -119,7 +120,7 @@ properties: >>>>> properties: >>>>> reg: >>>>> minimum: 1 >>>>> - maximum: 2 >>>>> + maximum: 4 >>>>> description: CPSW port number >>>>> >>>>> phys: >>>>> @@ -151,6 +152,18 @@ properties: >>>>> >>>>> additionalProperties: false >>>>> >>>>> +if: >>>> >>>> This goes under allOf just before unevaluated/additionalProperties:false >>> >>> allOf was added by me in v3 series patch and it is not present in the >>> file. I removed it in v4 after Rob Herring's suggestion. Please let me >>> know if simply moving the if-then statements to the line above >>> additionalProperties:false would be fine. >> >> I think Rob's comment was focusing not on using or not-using allOf, but >> on format of your entire if-then-else. Your v3 was huge and included >> allOf in wrong place). >> >> Now you add if-then in proper place, but it is still advisable to put it >> with allOf, so if ever you grow the if-then by new entry, you do not >> have to change the indentation. >> >> Anyway the location is not correct. Regardless if this is if-then or >> allOf-if-then, put it just like example schema is suggesting. > > I will move the if-then statements to the lines above the > "additionalProperties: false" line. Also, I will add an allOf for this I had a look at the example at [1] and it uses allOf after the "additionalProperties: false" line. Would it be fine then for me to add allOf and the single if-then statement below the "additionalProperties: false" line? Please let me know. [1] -> https://github.com/devicetree-org/dt-schema/blob/mai/test/schemas/conditionals-allof-example.yaml Regards, Siddharth.
On 19/08/22 15:59, Siddharth Vadapalli wrote: > Hello Krzysztof, > > On 17/08/22 13:11, Siddharth Vadapalli wrote: >> Hello Krzysztof, >> >> On 17/08/22 11:20, Krzysztof Kozlowski wrote: >>> On 17/08/2022 08:14, Siddharth Vadapalli wrote: >>> >>>>>> - port@[1-2]: >>>>>> + "^port@[1-4]$": >>>>>> type: object >>>>>> description: CPSWxG NUSS external ports >>>>>> >>>>>> @@ -119,7 +120,7 @@ properties: >>>>>> properties: >>>>>> reg: >>>>>> minimum: 1 >>>>>> - maximum: 2 >>>>>> + maximum: 4 >>>>>> description: CPSW port number >>>>>> >>>>>> phys: >>>>>> @@ -151,6 +152,18 @@ properties: >>>>>> >>>>>> additionalProperties: false >>>>>> >>>>>> +if: >>>>> >>>>> This goes under allOf just before unevaluated/additionalProperties:false >>>> >>>> allOf was added by me in v3 series patch and it is not present in the >>>> file. I removed it in v4 after Rob Herring's suggestion. Please let me >>>> know if simply moving the if-then statements to the line above >>>> additionalProperties:false would be fine. >>> >>> I think Rob's comment was focusing not on using or not-using allOf, but >>> on format of your entire if-then-else. Your v3 was huge and included >>> allOf in wrong place). >>> >>> Now you add if-then in proper place, but it is still advisable to put it >>> with allOf, so if ever you grow the if-then by new entry, you do not >>> have to change the indentation. >>> >>> Anyway the location is not correct. Regardless if this is if-then or >>> allOf-if-then, put it just like example schema is suggesting. >> >> I will move the if-then statements to the lines above the >> "additionalProperties: false" line. Also, I will add an allOf for this > > I had a look at the example at [1] and it uses allOf after the > "additionalProperties: false" line. Would it be fine then for me to add > allOf and the single if-then statement below the "additionalProperties: > false" line? Please let me know. > > [1] -> https://github.com/devicetree-org/dt-schema/blob/mai/test/schemas/conditionals-allof-example.yaml Sorry, the correct link is: https://github.com/devicetree-org/dt-schema/blob/main/test/schemas/conditionals-allof-example.yaml Regards, Siddharth.
On 19/08/2022 13:43, Siddharth Vadapalli wrote: >>>> Anyway the location is not correct. Regardless if this is if-then or >>>> allOf-if-then, put it just like example schema is suggesting. >>> >>> I will move the if-then statements to the lines above the >>> "additionalProperties: false" line. Also, I will add an allOf for this >> >> I had a look at the example at [1] and it uses allOf after the >> "additionalProperties: false" line. Would it be fine then for me to add >> allOf and the single if-then statement below the "additionalProperties: >> false" line? Please let me know. >> >> [1] -> https://github.com/devicetree-org/dt-schema/blob/mai/test/schemas/conditionals-allof-example.yaml > > Sorry, the correct link is: > https://github.com/devicetree-org/dt-schema/blob/main/test/schemas/conditionals-allof-example.yaml You are referring to tests? I did not suggest that. Please put it in place like example schema is suggesting: https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml Best regards, Krzysztof
Hello Krzysztof, On 19/08/22 17:34, Krzysztof Kozlowski wrote: > On 19/08/2022 13:43, Siddharth Vadapalli wrote: > >>>>> Anyway the location is not correct. Regardless if this is if-then or >>>>> allOf-if-then, put it just like example schema is suggesting. >>>> >>>> I will move the if-then statements to the lines above the >>>> "additionalProperties: false" line. Also, I will add an allOf for this >>> >>> I had a look at the example at [1] and it uses allOf after the >>> "additionalProperties: false" line. Would it be fine then for me to add >>> allOf and the single if-then statement below the "additionalProperties: >>> false" line? Please let me know. >>> >>> [1] -> https://github.com/devicetree-org/dt-schema/blob/mai/test/schemas/conditionals-allof-example.yaml >> >> Sorry, the correct link is: >> https://github.com/devicetree-org/dt-schema/blob/main/test/schemas/conditionals-allof-example.yaml > > You are referring to tests? I did not suggest that. Please put it in > place like example schema is suggesting: > > https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml Thank you for the clarification. I will follow this schema and add the allOf and the single if-then statement just above the "additionalProperties: false" line. Regards, Siddharth.
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 b8281d8be940..5366a367c387 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,j721e-cpsw-nuss - ti,am642-cpsw-nuss + - ti,j7200-cpswxg-nuss reg: maxItems: 1 @@ -110,7 +111,7 @@ properties: const: 0 patternProperties: - port@[1-2]: + "^port@[1-4]$": type: object description: CPSWxG NUSS external ports @@ -119,7 +120,7 @@ properties: properties: reg: minimum: 1 - maximum: 2 + maximum: 4 description: CPSW port number phys: @@ -151,6 +152,18 @@ properties: additionalProperties: false +if: + not: + properties: + compatible: + contains: + const: ti,j7200-cpswxg-nuss +then: + properties: + ethernet-ports: + patternProperties: + "^port@[3-4]$": false + patternProperties: "^mdio@[0-9a-f]+$": type: object
Update bindings for TI K3 J7200 SoC which contains 5 ports (4 external ports) CPSW5G module and add compatible for it. Changes made: - Add new compatible ti,j7200-cpswxg-nuss for CPSW5G. - Extend pattern properties for new compatible. - Change maximum number of CPSW ports to 4 for new compatible. Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> --- .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)