2022-08-16 08:26:47

by Siddharth Vadapalli

[permalink] [raw]
Subject: [PATCH v4 1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J7200 CPSW5G

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 <[email protected]>
---
.../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

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
--
2.25.1


2022-08-16 09:31:37

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J7200 CPSW5G

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 <[email protected]>
> ---
> .../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

2022-08-17 06:07:25

by Siddharth Vadapalli

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J7200 CPSW5G

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 <[email protected]>
>> ---
>> .../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.

2022-08-17 06:45:08

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J7200 CPSW5G

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

2022-08-17 08:31:47

by Siddharth Vadapalli

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J7200 CPSW5G

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.

2022-08-19 10:54:50

by Siddharth Vadapalli

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J7200 CPSW5G

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.

2022-08-19 11:02:37

by Siddharth Vadapalli

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J7200 CPSW5G

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.

2022-08-19 12:16:39

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J7200 CPSW5G

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

2022-08-22 04:24:42

by Siddharth Vadapalli

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J7200 CPSW5G

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.