diff mbox series

[1/3] dt-bindings: can: fsl,flexcan: add S32G2/S32G3 SoC support

Message ID 20241119081053.4175940-2-ciprianmarian.costea@oss.nxp.com (mailing list archive)
State Superseded
Headers show
Series add FlexCAN support for S32G2/S32G3 SoCs | expand

Commit Message

Ciprian Marian Costea Nov. 19, 2024, 8:10 a.m. UTC
From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>

Add S32G2/S32G3 SoCs compatible strings.

A particularity for these SoCs is the presence of separate interrupts for
state change, bus errors, MBs 0-7 and MBs 8-127 respectively.

Increase maxItems of 'interrupts' to 4 for S32G based SoCs and keep the
same restriction for other SoCs.

Also, as part of this commit, move the 'allOf' after the required
properties to make the documentation easier to read.

Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
---
 .../bindings/net/can/fsl,flexcan.yaml         | 25 ++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

Comments

Frank Li Nov. 19, 2024, 7:49 p.m. UTC | #1
On Tue, Nov 19, 2024 at 10:10:51AM +0200, Ciprian Costea wrote:
> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>
> Add S32G2/S32G3 SoCs compatible strings.
>
> A particularity for these SoCs is the presence of separate interrupts for
> state change, bus errors, MBs 0-7 and MBs 8-127 respectively.
>
> Increase maxItems of 'interrupts' to 4 for S32G based SoCs and keep the
> same restriction for other SoCs.
>
> Also, as part of this commit, move the 'allOf' after the required
> properties to make the documentation easier to read.
>
> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  .../bindings/net/can/fsl,flexcan.yaml         | 25 ++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
> index 97dd1a7c5ed2..cb7204c06acf 100644
> --- a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
> +++ b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
> @@ -10,9 +10,6 @@ title:
>  maintainers:
>    - Marc Kleine-Budde <mkl@pengutronix.de>
>
> -allOf:
> -  - $ref: can-controller.yaml#
> -
>  properties:
>    compatible:
>      oneOf:
> @@ -28,6 +25,7 @@ properties:
>            - fsl,vf610-flexcan
>            - fsl,ls1021ar2-flexcan
>            - fsl,lx2160ar1-flexcan
> +          - nxp,s32g2-flexcan
>        - items:
>            - enum:
>                - fsl,imx53-flexcan
> @@ -43,6 +41,10 @@ properties:
>            - enum:
>                - fsl,ls1028ar1-flexcan
>            - const: fsl,lx2160ar1-flexcan
> +      - items:
> +          - enum:
> +              - nxp,s32g3-flexcan
> +          - const: nxp,s32g2-flexcan
>
>    reg:
>      maxItems: 1
> @@ -136,6 +138,23 @@ required:
>    - reg
>    - interrupts
>
> +allOf:
> +  - $ref: can-controller.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: nxp,s32g2-flexcan
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 4
> +          maxItems: 4
> +    else:
> +      properties:
> +        interrupts:
> +          maxItems: 1
> +
>  additionalProperties: false
>
>  examples:
> --
> 2.45.2
>
Krzysztof Kozlowski Nov. 20, 2024, 8:45 a.m. UTC | #2
On Tue, Nov 19, 2024 at 10:10:51AM +0200, Ciprian Costea wrote:
>    reg:
>      maxItems: 1
> @@ -136,6 +138,23 @@ required:
>    - reg
>    - interrupts
>  
> +allOf:
> +  - $ref: can-controller.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: nxp,s32g2-flexcan
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 4
> +          maxItems: 4

Top level says max is 1. You need to keep there widest constraints.

> +    else:
> +      properties:
> +        interrupts:
> +          maxItems: 1

Best regards,
Krzysztof
Marc Kleine-Budde Nov. 20, 2024, 8:49 a.m. UTC | #3
On 19.11.2024 10:10:51, Ciprian Costea wrote:
> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> 
> Add S32G2/S32G3 SoCs compatible strings.
> 
> A particularity for these SoCs is the presence of separate interrupts for
> state change, bus errors, MBs 0-7 and MBs 8-127 respectively.
> 
> Increase maxItems of 'interrupts' to 4 for S32G based SoCs and keep the
> same restriction for other SoCs.

Can you add an "interrupt-names" property?

regards,
Marc
Ciprian Marian Costea Nov. 20, 2024, 9:04 a.m. UTC | #4
On 11/20/2024 10:49 AM, Marc Kleine-Budde wrote:
> On 19.11.2024 10:10:51, Ciprian Costea wrote:
>> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>
>> Add S32G2/S32G3 SoCs compatible strings.
>>
>> A particularity for these SoCs is the presence of separate interrupts for
>> state change, bus errors, MBs 0-7 and MBs 8-127 respectively.
>>
>> Increase maxItems of 'interrupts' to 4 for S32G based SoCs and keep the
>> same restriction for other SoCs.
> 
> Can you add an "interrupt-names" property?
> 
> regards,
> Marc
> 

Yes, I will add "interrupt-names" property under the if condition from 
'allOf' in V2.

Best Regards,
Ciprian
Krzysztof Kozlowski Nov. 20, 2024, 9:12 a.m. UTC | #5
On 20/11/2024 09:45, Krzysztof Kozlowski wrote:
> On Tue, Nov 19, 2024 at 10:10:51AM +0200, Ciprian Costea wrote:
>>    reg:
>>      maxItems: 1
>> @@ -136,6 +138,23 @@ required:
>>    - reg
>>    - interrupts
>>  
>> +allOf:
>> +  - $ref: can-controller.yaml#
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: nxp,s32g2-flexcan
>> +    then:
>> +      properties:
>> +        interrupts:
>> +          minItems: 4
>> +          maxItems: 4
> 
> Top level says max is 1. You need to keep there widest constraints.
And list items here instead...

Best regards,
Krzysztof
Ciprian Marian Costea Nov. 20, 2024, 10:33 a.m. UTC | #6
On 11/20/2024 11:12 AM, Krzysztof Kozlowski wrote:
> On 20/11/2024 09:45, Krzysztof Kozlowski wrote:
>> On Tue, Nov 19, 2024 at 10:10:51AM +0200, Ciprian Costea wrote:
>>>     reg:
>>>       maxItems: 1
>>> @@ -136,6 +138,23 @@ required:
>>>     - reg
>>>     - interrupts
>>>   
>>> +allOf:
>>> +  - $ref: can-controller.yaml#
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: nxp,s32g2-flexcan
>>> +    then:
>>> +      properties:
>>> +        interrupts:
>>> +          minItems: 4
>>> +          maxItems: 4
>>
>> Top level says max is 1. You need to keep there widest constraints.
> And list items here instead...
> 
> Best regards,
> Krzysztof

Hello Krzysztof,

Just to confirm before making any changes:
Are you referring to directly change 'maxItems' to value 4 ? Instead of 
using this 'if' condition under 'allOf' ?

Best Regards,
Ciprian
Krzysztof Kozlowski Nov. 20, 2024, 1:29 p.m. UTC | #7
On 20/11/2024 11:33, Ciprian Marian Costea wrote:
> On 11/20/2024 11:12 AM, Krzysztof Kozlowski wrote:
>> On 20/11/2024 09:45, Krzysztof Kozlowski wrote:
>>> On Tue, Nov 19, 2024 at 10:10:51AM +0200, Ciprian Costea wrote:
>>>>     reg:
>>>>       maxItems: 1
>>>> @@ -136,6 +138,23 @@ required:
>>>>     - reg
>>>>     - interrupts
>>>>   
>>>> +allOf:
>>>> +  - $ref: can-controller.yaml#
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            const: nxp,s32g2-flexcan
>>>> +    then:
>>>> +      properties:
>>>> +        interrupts:
>>>> +          minItems: 4
>>>> +          maxItems: 4
>>>
>>> Top level says max is 1. You need to keep there widest constraints.
>> And list items here instead...
>>
>> Best regards,
>> Krzysztof
> 
> Hello Krzysztof,
> 
> Just to confirm before making any changes:
> Are you referring to directly change 'maxItems' to value 4 ? Instead of 

No, I want you to create a list here. List the items. Nothing about
"maxItems" in my message above (unless you quote earlier but then
respond under proper quote). Just like other bindings are doing.

https://elixir.bootlin.com/linux/v6.11-rc6/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L127

> using this 'if' condition under 'allOf' ?

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
index 97dd1a7c5ed2..cb7204c06acf 100644
--- a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
+++ b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
@@ -10,9 +10,6 @@  title:
 maintainers:
   - Marc Kleine-Budde <mkl@pengutronix.de>
 
-allOf:
-  - $ref: can-controller.yaml#
-
 properties:
   compatible:
     oneOf:
@@ -28,6 +25,7 @@  properties:
           - fsl,vf610-flexcan
           - fsl,ls1021ar2-flexcan
           - fsl,lx2160ar1-flexcan
+          - nxp,s32g2-flexcan
       - items:
           - enum:
               - fsl,imx53-flexcan
@@ -43,6 +41,10 @@  properties:
           - enum:
               - fsl,ls1028ar1-flexcan
           - const: fsl,lx2160ar1-flexcan
+      - items:
+          - enum:
+              - nxp,s32g3-flexcan
+          - const: nxp,s32g2-flexcan
 
   reg:
     maxItems: 1
@@ -136,6 +138,23 @@  required:
   - reg
   - interrupts
 
+allOf:
+  - $ref: can-controller.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: nxp,s32g2-flexcan
+    then:
+      properties:
+        interrupts:
+          minItems: 4
+          maxItems: 4
+    else:
+      properties:
+        interrupts:
+          maxItems: 1
+
 additionalProperties: false
 
 examples: