diff mbox series

[v3,2/6] dt-bindings: can: nxp,sja1000: Document RZ/N1{D,S} support

Message ID 20220704075032.383700-3-biju.das.jz@bp.renesas.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Add support for RZ/N1 SJA1000 CAN controller | expand

Checks

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 fail Errors and warnings before: 5 this patch: 5
netdev/cc_maintainers success CCed 11 of 11 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, 56 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Biju Das July 4, 2022, 7:50 a.m. UTC
Add CAN binding documentation for Renesas RZ/N1 SoC.

The SJA1000 CAN controller on RZ/N1 SoC has some differences compared
to others like it has no clock divider register (CDR) support and it has
no HW loopback (HW doesn't see tx messages on rx), so introduced a new
compatible 'renesas,rzn1-sja1000' to handle these differences.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * Added reg-io-width is required property for renesas,rzn1-sja1000.
v1->v2:
 * Updated commit description.
 * Added an example for RZ/N1D SJA1000 usage
---
 .../bindings/net/can/nxp,sja1000.yaml         | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

Comments

Krzysztof Kozlowski July 4, 2022, 8:48 a.m. UTC | #1
On 04/07/2022 09:50, Biju Das wrote:
> Add CAN binding documentation for Renesas RZ/N1 SoC.
> 
> The SJA1000 CAN controller on RZ/N1 SoC has some differences compared
> to others like it has no clock divider register (CDR) support and it has
> no HW loopback (HW doesn't see tx messages on rx), so introduced a new
> compatible 'renesas,rzn1-sja1000' to handle these differences.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v2->v3:
>  * Added reg-io-width is required property for renesas,rzn1-sja1000.
> v1->v2:
>  * Updated commit description.
>  * Added an example for RZ/N1D SJA1000 usage
> ---
>  .../bindings/net/can/nxp,sja1000.yaml         | 35 +++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml b/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
> index d34060226e4e..16786475eae3 100644
> --- a/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
> +++ b/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
> @@ -19,6 +19,16 @@ allOf:
>      then:
>        required:
>          - reg-io-width
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,rzn1-sja1000
> +    then:
> +      required:
> +        - clocks
> +        - clock-names
> +        - reg-io-width
>  
>  properties:
>    compatible:
> @@ -27,6 +37,12 @@ properties:
>          const: nxp,sja1000
>        - description: Technologic Systems SJA1000 CAN Controller
>          const: technologic,sja1000
> +      - description: Renesas RZ/N1 SJA1000 CAN Controller
> +        items:
> +          - enum:
> +              - renesas,r9a06g032-sja1000 # RZ/N1D
> +              - renesas,r9a06g033-sja1000 # RZ/N1S
> +          - const: renesas,rzn1-sja1000 # RZ/N1

This explains usage of oneOf, but still earlier entries should be just
an enum.

>  
>    reg:
>      maxItems: 1
> @@ -34,6 +50,12 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: can_clk

Skip entire clock-names. Does not bring any information, especially that
name is obvious.

> +
>    reg-io-width:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      description: I/O register width (in bytes) implemented by this device
> @@ -101,3 +123,16 @@ examples:
>              nxp,tx-output-config = <0x06>;
>              nxp,external-clock-frequency = <24000000>;
>      };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/r9a06g032-sysctrl.h>
> +
> +    can@52104000 {
> +            compatible = "renesas,r9a06g032-sja1000","renesas,rzn1-sja1000";

Missing space after ,

Wrong indentation.

> +            reg = <0x52104000 0x800>;
> +            reg-io-width = <4>;
> +            interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&sysctrl R9A06G032_HCLK_CAN0>;
> +            clock-names = "can_clk";
> +    };


Best regards,
Krzysztof
Biju Das July 4, 2022, 9:07 a.m. UTC | #2
Hi Krzysztof,

Thanks for the feedback.

> Subject: Re: [PATCH v3 2/6] dt-bindings: can: nxp,sja1000: Document
> RZ/N1{D,S} support
> 
> On 04/07/2022 09:50, Biju Das wrote:
> > Add CAN binding documentation for Renesas RZ/N1 SoC.
> >
> > The SJA1000 CAN controller on RZ/N1 SoC has some differences compared
> > to others like it has no clock divider register (CDR) support and it
> > has no HW loopback (HW doesn't see tx messages on rx), so introduced a
> > new compatible 'renesas,rzn1-sja1000' to handle these differences.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v2->v3:
> >  * Added reg-io-width is required property for renesas,rzn1-sja1000.
> > v1->v2:
> >  * Updated commit description.
> >  * Added an example for RZ/N1D SJA1000 usage
> > ---
> >  .../bindings/net/can/nxp,sja1000.yaml         | 35 +++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
> > b/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
> > index d34060226e4e..16786475eae3 100644
> > --- a/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
> > +++ b/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
> > @@ -19,6 +19,16 @@ allOf:
> >      then:
> >        required:
> >          - reg-io-width
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: renesas,rzn1-sja1000
> > +    then:
> > +      required:
> > +        - clocks
> > +        - clock-names
> > +        - reg-io-width
> >
> >  properties:
> >    compatible:
> > @@ -27,6 +37,12 @@ properties:
> >          const: nxp,sja1000
> >        - description: Technologic Systems SJA1000 CAN Controller
> >          const: technologic,sja1000
> > +      - description: Renesas RZ/N1 SJA1000 CAN Controller
> > +        items:
> > +          - enum:
> > +              - renesas,r9a06g032-sja1000 # RZ/N1D
> > +              - renesas,r9a06g033-sja1000 # RZ/N1S
> > +          - const: renesas,rzn1-sja1000 # RZ/N1
> 
> This explains usage of oneOf, but still earlier entries should be just an
> enum.

OK.

> 
> >
> >    reg:
> >      maxItems: 1
> > @@ -34,6 +50,12 @@ properties:
> >    interrupts:
> >      maxItems: 1
> >
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    const: can_clk
> 
> Skip entire clock-names. Does not bring any information, especially that
> name is obvious.

Yes, true for single clock, clock-names doesn't make any value.
Will drop it.

> 
> > +
> >    reg-io-width:
> >      $ref: /schemas/types.yaml#/definitions/uint32
> >      description: I/O register width (in bytes) implemented by this
> > device @@ -101,3 +123,16 @@ examples:
> >              nxp,tx-output-config = <0x06>;
> >              nxp,external-clock-frequency = <24000000>;
> >      };
> > +
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/r9a06g032-sysctrl.h>
> > +
> > +    can@52104000 {
> > +            compatible =
> > + "renesas,r9a06g032-sja1000","renesas,rzn1-sja1000";
> 
> Missing space after ,
OK.

> 
> Wrong indentation.

As you suggested in the previous patch, will make 4 spaces indentation for examples.

Cheers,
Biju
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml b/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
index d34060226e4e..16786475eae3 100644
--- a/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
+++ b/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
@@ -19,6 +19,16 @@  allOf:
     then:
       required:
         - reg-io-width
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,rzn1-sja1000
+    then:
+      required:
+        - clocks
+        - clock-names
+        - reg-io-width
 
 properties:
   compatible:
@@ -27,6 +37,12 @@  properties:
         const: nxp,sja1000
       - description: Technologic Systems SJA1000 CAN Controller
         const: technologic,sja1000
+      - description: Renesas RZ/N1 SJA1000 CAN Controller
+        items:
+          - enum:
+              - renesas,r9a06g032-sja1000 # RZ/N1D
+              - renesas,r9a06g033-sja1000 # RZ/N1S
+          - const: renesas,rzn1-sja1000 # RZ/N1
 
   reg:
     maxItems: 1
@@ -34,6 +50,12 @@  properties:
   interrupts:
     maxItems: 1
 
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: can_clk
+
   reg-io-width:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: I/O register width (in bytes) implemented by this device
@@ -101,3 +123,16 @@  examples:
             nxp,tx-output-config = <0x06>;
             nxp,external-clock-frequency = <24000000>;
     };
+
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/r9a06g032-sysctrl.h>
+
+    can@52104000 {
+            compatible = "renesas,r9a06g032-sja1000","renesas,rzn1-sja1000";
+            reg = <0x52104000 0x800>;
+            reg-io-width = <4>;
+            interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&sysctrl R9A06G032_HCLK_CAN0>;
+            clock-names = "can_clk";
+    };