[PATCH v4 00/11] Add support for RZ/G3E CANFD

[PATCH v4 00/11] Add support for RZ/G3E CANFD

[Biju Das]

The CAN-FD module on RZ/G3E is very similar to the one on both R-Car V4H
and RZ/G2L, but differs in some hardware parameters:
 * No external clock, but instead has ram clock.
 * Support up to 6 channels.
 * 20 interrupts.

This patch series depend upon [1]
[1] https://lore.kernel.org/all/20250220094516.126598-1-biju.das.jz@bp.renesas.com/

v3->v4:
 * Added Rb tag from Rob for patch#2.
 * Added prefix RCANFD_* to enum rcar_canfd_reg_offset_id.
 * Added prefix RCANFD_* to enum rcar_canfd_mask_id.
 * Added prefix RCANFD_* to enum rcar_canfd_shift_id.
v2->v3:
 * Collected tags.
 * Dropped reg_gen4() and is_gen4() by adding mask_table, shift_table,
   regs, ch_interface_mode and shared_can_reg variables to
   struct rcar_canfd_hw_info.
v1->v2:
 * Split the series with fixes patch separately.
 * Added patch for Simplify rcar_canfd_probe() using
   of_get_available_child_by_name() as dependency patch hit on can-next.
 * Added Rb tag from Vincent Mailhol.
 * Dropped redundant comment from commit description for patch#3.

Biju Das (11):
  dt-bindings: can: renesas,rcar-canfd: Simplify the conditional schema
  dt-bindings: can: renesas,rcar-canfd: Document RZ/G3E support
  can: rcar_canfd: Use of_get_available_child_by_name()
  can: rcar_canfd: Add ch_interface_mode variable to struct
    rcar_canfd_hw_info
  can: rcar_canfd: Add shared_can_regs variable to struct
    rcar_canfd_hw_info
  can: rcar_canfd: Add register mapping table to struct
    rcar_canfd_hw_info
  can: rcar_canfd: Add mask table to struct rcar_canfd_hw_info
  can: rcar_canfd: Add shift table to struct rcar_canfd_hw_info
  can: rcar_canfd: Add only_internal_clks variable to struct
    rcar_canfd_hw_info
  can: rcar_canfd: Enhance multi_channel_irqs handling
  can: rcar_canfd: Add RZ/G3E support

 .../bindings/net/can/renesas,rcar-canfd.yaml  | 178 +++++++++----
 drivers/net/can/rcar/rcar_canfd.c             | 237 ++++++++++++++----
 2 files changed, 325 insertions(+), 90 deletions(-)

-- 
2.43.0

[PATCH v4 01/11] dt-bindings: can: renesas,rcar-canfd: Simplify the conditional schema

[Biju Das]

RZ/G3E SoC has 20 interrupts, 2 resets and 6 channels that need more
branching with conditional schema. Simplify the conditional schema with
if statements rather than the complex if-else statements to prepare for
supporting RZ/G3E SoC.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
This patch depend upon [1]
[1] https://lore.kernel.org/all/20250220094516.126598-2-biju.das.jz@bp.renesas.com/
v3->v4:
 * No change.
v2->v3:
 * Collected tags.
v1->v2:
 * No change
---
 .../bindings/net/can/renesas,rcar-canfd.yaml  | 123 ++++++++++++------
 1 file changed, 80 insertions(+), 43 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
index f6884f6e59e7..f87f90f431e5 100644
--- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
+++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
@@ -45,7 +45,35 @@ properties:
   reg:
     maxItems: 1
 
-  interrupts: true
+  interrupts:
+    oneOf:
+      - items:
+          - description: Channel interrupt
+          - description: Global interrupt
+      - items:
+          - description: CAN global error interrupt
+          - description: CAN receive FIFO interrupt
+          - description: CAN0 error interrupt
+          - description: CAN0 transmit interrupt
+          - description: CAN0 transmit/receive FIFO receive completion interrupt
+          - description: CAN1 error interrupt
+          - description: CAN1 transmit interrupt
+          - description: CAN1 transmit/receive FIFO receive completion interrupt
+
+  interrupt-names:
+    oneOf:
+      - items:
+          - const: ch_int
+          - const: g_int
+      - items:
+          - const: g_err
+          - const: g_recc
+          - const: ch0_err
+          - const: ch0_rec
+          - const: ch0_trx
+          - const: ch1_err
+          - const: ch1_rec
+          - const: ch1_trx
 
   clocks:
     maxItems: 3
@@ -117,52 +145,71 @@ allOf:
     then:
       properties:
         interrupts:
-          items:
-            - description: CAN global error interrupt
-            - description: CAN receive FIFO interrupt
-            - description: CAN0 error interrupt
-            - description: CAN0 transmit interrupt
-            - description: CAN0 transmit/receive FIFO receive completion interrupt
-            - description: CAN1 error interrupt
-            - description: CAN1 transmit interrupt
-            - description: CAN1 transmit/receive FIFO receive completion interrupt
+          minItems: 8
+          maxItems: 8
 
         interrupt-names:
-          items:
-            - const: g_err
-            - const: g_recc
-            - const: ch0_err
-            - const: ch0_rec
-            - const: ch0_trx
-            - const: ch1_err
-            - const: ch1_rec
-            - const: ch1_trx
+          minItems: 8
+          maxItems: 8
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,rcar-gen3-canfd
+              - renesas,rcar-gen4-canfd
+    then:
+      properties:
+        interrupts:
+          minItems: 2
+          maxItems: 2
+
+        interrupt-names:
+          minItems: 2
+          maxItems: 2
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,rzg2l-canfd
+    then:
+      properties:
         resets:
+          minItems: 2
           maxItems: 2
 
         reset-names:
-          items:
-            - const: rstp_n
-            - const: rstc_n
+          minItems: 2
+          maxItems: 2
 
       required:
         - reset-names
-    else:
-      properties:
-        interrupts:
-          items:
-            - description: Channel interrupt
-            - description: Global interrupt
-
-        interrupt-names:
-          items:
-            - const: ch_int
-            - const: g_int
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,rcar-gen3-canfd
+              - renesas,rcar-gen4-canfd
+    then:
+      properties:
         resets:
           maxItems: 1
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,rcar-gen3-canfd
+              - renesas,rzg2l-canfd
+    then:
+      patternProperties:
+        "^channel[2-7]$": false
+
   - if:
       properties:
         compatible:
@@ -171,16 +218,6 @@ allOf:
     then:
       patternProperties:
         "^channel[4-7]$": false
-    else:
-      if:
-        not:
-          properties:
-            compatible:
-              contains:
-                const: renesas,rcar-gen4-canfd
-      then:
-        patternProperties:
-          "^channel[2-7]$": false
 
 unevaluatedProperties: false
 
-- 
2.43.0

Re: [PATCH v4 01/11] dt-bindings: can: renesas,rcar-canfd: Simplify the conditional schema

[Geert Uytterhoeven]

Hi Biju,

On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> RZ/G3E SoC has 20 interrupts, 2 resets and 6 channels that need more
> branching with conditional schema. Simplify the conditional schema with
> if statements rather than the complex if-else statements to prepare for
> supporting RZ/G3E SoC.
>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml

> @@ -117,52 +145,71 @@ allOf:
>      then:
>        properties:
>          interrupts:
> -          items:
> -            - description: CAN global error interrupt
> -            - description: CAN receive FIFO interrupt
> -            - description: CAN0 error interrupt
> -            - description: CAN0 transmit interrupt
> -            - description: CAN0 transmit/receive FIFO receive completion interrupt
> -            - description: CAN1 error interrupt
> -            - description: CAN1 transmit interrupt
> -            - description: CAN1 transmit/receive FIFO receive completion interrupt
> +          minItems: 8
> +          maxItems: 8
>
>          interrupt-names:
> -          items:
> -            - const: g_err
> -            - const: g_recc
> -            - const: ch0_err
> -            - const: ch0_rec
> -            - const: ch0_trx
> -            - const: ch1_err
> -            - const: ch1_rec
> -            - const: ch1_trx
> +          minItems: 8
> +          maxItems: 8
>
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - renesas,rcar-gen3-canfd
> +              - renesas,rcar-gen4-canfd
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 2
> +          maxItems: 2
> +
> +        interrupt-names:
> +          minItems: 2
> +          maxItems: 2
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,rzg2l-canfd
> +    then:
> +      properties:
>          resets:
> +          minItems: 2
>            maxItems: 2
>
>          reset-names:
> -          items:
> -            - const: rstp_n
> -            - const: rstc_n
> +          minItems: 2
> +          maxItems: 2
>
>        required:
>          - reset-names
> -    else:
> -      properties:
> -        interrupts:
> -          items:
> -            - description: Channel interrupt
> -            - description: Global interrupt
> -
> -        interrupt-names:
> -          items:
> -            - const: ch_int
> -            - const: g_int
>
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - renesas,rcar-gen3-canfd
> +              - renesas,rcar-gen4-canfd
> +    then:
> +      properties:
>          resets:
>            maxItems: 1

I understand you introduced separate if-blocks for interrupts and
resets because RZ/G3E can reuse only a part of the RZ/G2L rules.
However, at least the rules for R-Car Gen3/4 could be kept together,
reducing the number of lines.

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

RE: [PATCH v4 01/11] dt-bindings: can: renesas,rcar-canfd: Simplify the conditional schema

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 14 March 2025 13:32
> Subject: Re: [PATCH v4 01/11] dt-bindings: can: renesas,rcar-canfd: Simplify the conditional schema
> 
> Hi Biju,
> 
> On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > RZ/G3E SoC has 20 interrupts, 2 resets and 6 channels that need more
> > branching with conditional schema. Simplify the conditional schema
> > with if statements rather than the complex if-else statements to
> > prepare for supporting RZ/G3E SoC.
> >
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > ---
> > a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> > +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yam
> > +++ l
> 
> > @@ -117,52 +145,71 @@ allOf:
> >      then:
> >        properties:
> >          interrupts:
> > -          items:
> > -            - description: CAN global error interrupt
> > -            - description: CAN receive FIFO interrupt
> > -            - description: CAN0 error interrupt
> > -            - description: CAN0 transmit interrupt
> > -            - description: CAN0 transmit/receive FIFO receive completion interrupt
> > -            - description: CAN1 error interrupt
> > -            - description: CAN1 transmit interrupt
> > -            - description: CAN1 transmit/receive FIFO receive completion interrupt
> > +          minItems: 8
> > +          maxItems: 8
> >
> >          interrupt-names:
> > -          items:
> > -            - const: g_err
> > -            - const: g_recc
> > -            - const: ch0_err
> > -            - const: ch0_rec
> > -            - const: ch0_trx
> > -            - const: ch1_err
> > -            - const: ch1_rec
> > -            - const: ch1_trx
> > +          minItems: 8
> > +          maxItems: 8
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - renesas,rcar-gen3-canfd
> > +              - renesas,rcar-gen4-canfd
> > +    then:
> > +      properties:
> > +        interrupts:
> > +          minItems: 2
> > +          maxItems: 2
> > +
> > +        interrupt-names:
> > +          minItems: 2
> > +          maxItems: 2
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: renesas,rzg2l-canfd
> > +    then:
> > +      properties:
> >          resets:
> > +          minItems: 2
> >            maxItems: 2
> >
> >          reset-names:
> > -          items:
> > -            - const: rstp_n
> > -            - const: rstc_n
> > +          minItems: 2
> > +          maxItems: 2
> >
> >        required:
> >          - reset-names
> > -    else:
> > -      properties:
> > -        interrupts:
> > -          items:
> > -            - description: Channel interrupt
> > -            - description: Global interrupt
> > -
> > -        interrupt-names:
> > -          items:
> > -            - const: ch_int
> > -            - const: g_int
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - renesas,rcar-gen3-canfd
> > +              - renesas,rcar-gen4-canfd
> > +    then:
> > +      properties:
> >          resets:
> >            maxItems: 1
> 
> I understand you introduced separate if-blocks for interrupts and resets because RZ/G3E can reuse only
> a part of the RZ/G2L rules.
> However, at least the rules for R-Car Gen3/4 could be kept together, reducing the number of lines.

Yes, the resets can go along with interrupt and interrupt-names, for R-Car Gen3/4.

Cheers,
Biju

[PATCH v4 02/11] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3E support

[Biju Das]

Document support for the CAN-FD Interface on the RZ/G3E (R9A09G047) SoC,
which supports up to six channels.

The CAN-FD module on RZ/G3E is very similar to the one on both R-Car V4H
and RZ/G2L, but differs in some hardware parameters:
 * No external clock, but instead has ram clock.
 * Support up to 6 channels.
 * 20 interrupts.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * Added Rb tag from Rob.
v2->v3:
 * Replaced maxItems->minItems: 20 for RZ/G3E interrupt,s as the list has 20
   elements and for existing platforms dropped minItems and keep maxItems: 8.
v1->v2:
 * No change.
---
 .../bindings/net/can/renesas,rcar-canfd.yaml  | 69 +++++++++++++++++--
 1 file changed, 62 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
index f87f90f431e5..47967e944710 100644
--- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
+++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
@@ -42,6 +42,8 @@ properties:
               - renesas,r9a07g054-canfd    # RZ/V2L
           - const: renesas,rzg2l-canfd     # RZ/G2L family
 
+      - const: renesas,r9a09g047-canfd     # RZ/G3E
+
   reg:
     maxItems: 1
 
@@ -59,6 +61,19 @@ properties:
           - description: CAN1 error interrupt
           - description: CAN1 transmit interrupt
           - description: CAN1 transmit/receive FIFO receive completion interrupt
+          - description: CAN2 error interrupt
+          - description: CAN2 transmit interrupt
+          - description: CAN2 transmit/receive FIFO receive completion interrupt
+          - description: CAN3 error interrupt
+          - description: CAN3 transmit interrupt
+          - description: CAN3 transmit/receive FIFO receive completion interrupt
+          - description: CAN4 error interrupt
+          - description: CAN4 transmit interrupt
+          - description: CAN4 transmit/receive FIFO receive completion interrupt
+          - description: CAN5 error interrupt
+          - description: CAN5 transmit interrupt
+          - description: CAN5 transmit/receive FIFO receive completion interrupt
+        minItems: 8
 
   interrupt-names:
     oneOf:
@@ -74,15 +89,33 @@ properties:
           - const: ch1_err
           - const: ch1_rec
           - const: ch1_trx
+          - const: ch2_err
+          - const: ch2_rec
+          - const: ch2_trx
+          - const: ch3_err
+          - const: ch3_rec
+          - const: ch3_trx
+          - const: ch4_err
+          - const: ch4_rec
+          - const: ch4_trx
+          - const: ch5_err
+          - const: ch5_rec
+          - const: ch5_trx
+        minItems: 8
 
   clocks:
     maxItems: 3
 
   clock-names:
-    items:
-      - const: fck
-      - const: canfd
-      - const: can_clk
+    oneOf:
+      - items:
+          - const: fck
+          - const: canfd
+          - const: can_clk
+      - items:
+          - const: fck
+          - const: ram_clk
+          - const: can_clk
 
   power-domains:
     maxItems: 1
@@ -145,11 +178,9 @@ allOf:
     then:
       properties:
         interrupts:
-          minItems: 8
           maxItems: 8
 
         interrupt-names:
-          minItems: 8
           maxItems: 8
 
   - if:
@@ -173,7 +204,9 @@ allOf:
       properties:
         compatible:
           contains:
-            const: renesas,rzg2l-canfd
+            enum:
+              - renesas,r9a09g047-canfd
+              - renesas,rzg2l-canfd
     then:
       properties:
         resets:
@@ -187,6 +220,19 @@ allOf:
       required:
         - reset-names
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,r9a09g047-canfd
+    then:
+      properties:
+        interrupts:
+          minItems: 20
+
+        interrupt-names:
+          minItems: 20
+
   - if:
       properties:
         compatible:
@@ -219,6 +265,15 @@ allOf:
       patternProperties:
         "^channel[4-7]$": false
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,r9a09g047-canfd
+    then:
+      patternProperties:
+        "^channel[6-7]$": false
+
 unevaluatedProperties: false
 
 examples:
-- 
2.43.0

Re: [PATCH v4 02/11] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3E support

[Geert Uytterhoeven]

Hi Biju,

On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Document support for the CAN-FD Interface on the RZ/G3E (R9A09G047) SoC,
> which supports up to six channels.
>
> The CAN-FD module on RZ/G3E is very similar to the one on both R-Car V4H
> and RZ/G2L, but differs in some hardware parameters:
>  * No external clock, but instead has ram clock.
>  * Support up to 6 channels.
>  * 20 interrupts.
>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml

> @@ -173,7 +204,9 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            const: renesas,rzg2l-canfd
> +            enum:
> +              - renesas,r9a09g047-canfd
> +              - renesas,rzg2l-canfd

Not having this common rule, and keeping interrupts and resets together
would allow you to keep a clear separation between RZ/G2L and RZ/G3E,
at the expense of only a single line.

>      then:
>        properties:
>          resets:
> @@ -187,6 +220,19 @@ allOf:
>        required:
>          - reset-names
>
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,r9a09g047-canfd
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 20
> +
> +        interrupt-names:
> +          minItems: 20
> +
>    - if:
>        properties:
>          compatible:
> @@ -219,6 +265,15 @@ allOf:
>        patternProperties:
>          "^channel[4-7]$": false
>
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,r9a09g047-canfd
> +    then:
> +      patternProperties:
> +        "^channel[6-7]$": false
> +
>  unevaluatedProperties: false

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

RE: [PATCH v4 02/11] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3E support

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 14 March 2025 13:40
> Subject: Re: [PATCH v4 02/11] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3E support
> 
> Hi Biju,
> 
> On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Document support for the CAN-FD Interface on the RZ/G3E (R9A09G047)
> > SoC, which supports up to six channels.
> >
> > The CAN-FD module on RZ/G3E is very similar to the one on both R-Car
> > V4H and RZ/G2L, but differs in some hardware parameters:
> >  * No external clock, but instead has ram clock.
> >  * Support up to 6 channels.
> >  * 20 interrupts.
> >
> > Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > ---
> > a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> > +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yam
> > +++ l
> 
> > @@ -173,7 +204,9 @@ allOf:
> >        properties:
> >          compatible:
> >            contains:
> > -            const: renesas,rzg2l-canfd
> > +            enum:
> > +              - renesas,r9a09g047-canfd
> > +              - renesas,rzg2l-canfd
> 
> Not having this common rule, and keeping interrupts and resets together would allow you to keep a
> clear separation between RZ/G2L and RZ/G3E, at the expense of only a single line.

I agree, this will make clear separation.
 
Rob,Conor, Krzysztof, Marc:
  
The suggested changes are trivial and is just sorting the properties for clear
separation. I will be sending V5 for this.

Cheers,
Biju

[PATCH v4 03/11] can: rcar_canfd: Use of_get_available_child_by_name()

[Biju Das]

Simplify rcar_canfd_probe() using of_get_available_child_by_name().

While at it, move of_node_put(child) inside the if block to avoid
additional check if of_child is NULL.

Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * No change.
v2->v3:
 * Added Rb tag from Geert.
v2:
 * Added to this series as dependency patch hit on can-next.
 * Added Rb tag from Vincent Mailhol
 * Dropped redundant comment from commit description.
---
 drivers/net/can/rcar/rcar_canfd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 270f50d836f5..8b326977ddf7 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -1864,13 +1864,13 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 
 	for (i = 0; i < info->max_channels; ++i) {
 		name[7] = '0' + i;
-		of_child = of_get_child_by_name(dev->of_node, name);
-		if (of_child && of_device_is_available(of_child)) {
+		of_child = of_get_available_child_by_name(dev->of_node, name);
+		if (of_child) {
 			channels_mask |= BIT(i);
 			transceivers[i] = devm_of_phy_optional_get(dev,
 							of_child, NULL);
+			of_node_put(of_child);
 		}
-		of_node_put(of_child);
 		if (IS_ERR(transceivers[i]))
 			return PTR_ERR(transceivers[i]);
 	}
-- 
2.43.0

[PATCH v4 04/11] can: rcar_canfd: Add ch_interface_mode variable to struct rcar_canfd_hw_info

[Biju Das]

R-Car Gen4 has channel specific interface mode bit for setting CAN-FD or
Classical CAN mode whereas on R-Car Gen3 it is global. Add a
ch_interface_mode variable to struct rcar_canfd_hw_info to handle this
difference.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * No change.
v3:
 * New patch.
---
 drivers/net/can/rcar/rcar_canfd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 8b326977ddf7..e21041fe432c 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -516,6 +516,7 @@ struct rcar_canfd_hw_info {
 	/* hardware features */
 	unsigned shared_global_irqs:1;	/* Has shared global irqs */
 	unsigned multi_channel_irqs:1;	/* Has multiple channel irqs */
+	unsigned ch_interface_mode:1;	/* Has channel interface mode */
 };
 
 /* Channel priv data */
@@ -596,6 +597,7 @@ static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
 	.max_channels = 8,
 	.postdiv = 2,
 	.shared_global_irqs = 1,
+	.ch_interface_mode = 1,
 };
 
 static const struct rcar_canfd_hw_info rzg2l_hw_info = {
@@ -683,7 +685,7 @@ static void rcar_canfd_tx_failure_cleanup(struct net_device *ndev)
 
 static void rcar_canfd_set_mode(struct rcar_canfd_global *gpriv)
 {
-	if (is_gen4(gpriv)) {
+	if (gpriv->info->ch_interface_mode) {
 		u32 ch, val = gpriv->fdmode ? RCANFD_GEN4_FDCFG_FDOE
 					    : RCANFD_GEN4_FDCFG_CLOE;
 
-- 
2.43.0

Re: [PATCH v4 04/11] can: rcar_canfd: Add ch_interface_mode variable to struct rcar_canfd_hw_info

[Geert Uytterhoeven]

On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> R-Car Gen4 has channel specific interface mode bit for setting CAN-FD or
> Classical CAN mode whereas on R-Car Gen3 it is global. Add a
> ch_interface_mode variable to struct rcar_canfd_hw_info to handle this
> difference.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

[PATCH v4 05/11] can: rcar_canfd: Add shared_can_regs variable to struct rcar_canfd_hw_info

[Biju Das]

R-Car Gen4 has shared regs for both CAN-FD and Classical CAN operations.
Add shared_can_regs variable to struct rcar_canfd_hw_info to handle this
difference.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * No change.
v3:
 * New patch.
---
 drivers/net/can/rcar/rcar_canfd.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index e21041fe432c..f71725da57ce 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -517,6 +517,7 @@ struct rcar_canfd_hw_info {
 	unsigned shared_global_irqs:1;	/* Has shared global irqs */
 	unsigned multi_channel_irqs:1;	/* Has multiple channel irqs */
 	unsigned ch_interface_mode:1;	/* Has channel interface mode */
+	unsigned shared_can_reg:1;	/* Has shared classical can registers */
 };
 
 /* Channel priv data */
@@ -598,6 +599,7 @@ static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
 	.postdiv = 2,
 	.shared_global_irqs = 1,
 	.ch_interface_mode = 1,
+	.shared_can_reg = 1,
 };
 
 static const struct rcar_canfd_hw_info rzg2l_hw_info = {
@@ -813,7 +815,7 @@ static void rcar_canfd_configure_afl_rules(struct rcar_canfd_global *gpriv,
 	/* Write number of rules for channel */
 	rcar_canfd_set_bit(gpriv->base, RCANFD_GAFLCFG(ch),
 			   RCANFD_GAFLCFG_SETRNC(gpriv, ch, num_rules));
-	if (is_gen4(gpriv))
+	if (gpriv->info->shared_can_reg)
 		offset = RCANFD_GEN4_GAFL_OFFSET;
 	else if (gpriv->fdmode)
 		offset = RCANFD_F_GAFL_OFFSET;
@@ -1349,7 +1351,7 @@ static void rcar_canfd_set_bittiming(struct net_device *dev)
 			   brp, sjw, tseg1, tseg2);
 	} else {
 		/* Classical CAN only mode */
-		if (is_gen4(gpriv)) {
+		if (gpriv->info->shared_can_reg) {
 			cfg = (RCANFD_NCFG_NTSEG1(gpriv, tseg1) |
 			       RCANFD_NCFG_NBRP(brp) |
 			       RCANFD_NCFG_NSJW(gpriv, sjw) |
@@ -1514,7 +1516,7 @@ static netdev_tx_t rcar_canfd_start_xmit(struct sk_buff *skb,
 
 	dlc = RCANFD_CFPTR_CFDLC(can_fd_len2dlc(cf->len));
 
-	if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) || is_gen4(gpriv)) {
+	if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) || gpriv->info->shared_can_reg) {
 		rcar_canfd_write(priv->base,
 				 RCANFD_F_CFID(gpriv, ch, RCANFD_CFFIFO_IDX), id);
 		rcar_canfd_write(priv->base,
@@ -1573,7 +1575,7 @@ static void rcar_canfd_rx_pkt(struct rcar_canfd_channel *priv)
 	u32 ch = priv->channel;
 	u32 ridx = ch + RCANFD_RFFIFO_IDX;
 
-	if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) || is_gen4(gpriv)) {
+	if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) || gpriv->info->shared_can_reg) {
 		id = rcar_canfd_read(priv->base, RCANFD_F_RFID(gpriv, ridx));
 		dlc = rcar_canfd_read(priv->base, RCANFD_F_RFPTR(gpriv, ridx));
 
@@ -1624,7 +1626,7 @@ static void rcar_canfd_rx_pkt(struct rcar_canfd_channel *priv)
 		cf->len = can_cc_dlc2len(RCANFD_RFPTR_RFDLC(dlc));
 		if (id & RCANFD_RFID_RFRTR)
 			cf->can_id |= CAN_RTR_FLAG;
-		else if (is_gen4(gpriv))
+		else if (gpriv->info->shared_can_reg)
 			rcar_canfd_get_data(priv, cf, RCANFD_F_RFDF(gpriv, ridx, 0));
 		else
 			rcar_canfd_get_data(priv, cf, RCANFD_C_RFDF(ridx, 0));
-- 
2.43.0

Re: [PATCH v4 05/11] can: rcar_canfd: Add shared_can_regs variable to struct rcar_canfd_hw_info

[Geert Uytterhoeven]

Hi Biju,

On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> R-Car Gen4 has shared regs for both CAN-FD and Classical CAN operations.
> Add shared_can_regs variable to struct rcar_canfd_hw_info to handle this
> difference.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

One nit below.

> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -517,6 +517,7 @@ struct rcar_canfd_hw_info {
>         unsigned shared_global_irqs:1;  /* Has shared global irqs */
>         unsigned multi_channel_irqs:1;  /* Has multiple channel irqs */
>         unsigned ch_interface_mode:1;   /* Has channel interface mode */
> +       unsigned shared_can_reg:1;      /* Has shared classical can registers */

shared_can_regs?

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

RE: [PATCH v4 05/11] can: rcar_canfd: Add shared_can_regs variable to struct rcar_canfd_hw_info

[Biju Das]

Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 14 March 2025 14:47
> Subject: Re: [PATCH v4 05/11] can: rcar_canfd: Add shared_can_regs variable to struct
> rcar_canfd_hw_info
> 
> Hi Biju,
> 
> On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > R-Car Gen4 has shared regs for both CAN-FD and Classical CAN operations.
> > Add shared_can_regs variable to struct rcar_canfd_hw_info to handle
> > this difference.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> One nit below.
> 
> > --- a/drivers/net/can/rcar/rcar_canfd.c
> > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > @@ -517,6 +517,7 @@ struct rcar_canfd_hw_info {
> >         unsigned shared_global_irqs:1;  /* Has shared global irqs */
> >         unsigned multi_channel_irqs:1;  /* Has multiple channel irqs */
> >         unsigned ch_interface_mode:1;   /* Has channel interface mode */
> > +       unsigned shared_can_reg:1;      /* Has shared classical can registers */
> 
> shared_can_regs?

Will fix this in next version.

Thanks,
Biju

[PATCH v4 06/11] can: rcar_canfd: Add register mapping table to struct rcar_canfd_hw_info

[Biju Das]

R-Car Gen3 and Gen4 has some differences in the register offsets. Add a
mapping table to handle these differences.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * Added prefix RCANFD_* to enum rcar_canfd_reg_offset_id.
v3:
 * New patch.
---
 drivers/net/can/rcar/rcar_canfd.c | 48 ++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index f71725da57ce..f7c78b1a2684 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -305,7 +305,7 @@
 #define RCANFD_RMND(y)			(0x00a8 + (0x04 * (y)))
 
 /* RSCFDnCFDRFCCx / RSCFDnRFCCx */
-#define RCANFD_RFCC(gpriv, x)		(reg_gen4(gpriv, 0x00c0, 0x00b8) + (0x04 * (x)))
+#define RCANFD_RFCC(gpriv, x)		((gpriv)->info->regs[RCANFD_RFCC] + (0x04 * (x)))
 /* RSCFDnCFDRFSTSx / RSCFDnRFSTSx */
 #define RCANFD_RFSTS(gpriv, x)		(RCANFD_RFCC(gpriv, x) + 0x20)
 /* RSCFDnCFDRFPCTRx / RSCFDnRFPCTRx */
@@ -315,13 +315,13 @@
 
 /* RSCFDnCFDCFCCx / RSCFDnCFCCx */
 #define RCANFD_CFCC(gpriv, ch, idx) \
-	(reg_gen4(gpriv, 0x0120, 0x0118) + (0x0c * (ch)) + (0x04 * (idx)))
+	((gpriv)->info->regs[RCANFD_CFCC] + (0x0c * (ch)) + (0x04 * (idx)))
 /* RSCFDnCFDCFSTSx / RSCFDnCFSTSx */
 #define RCANFD_CFSTS(gpriv, ch, idx) \
-	(reg_gen4(gpriv, 0x01e0, 0x0178) + (0x0c * (ch)) + (0x04 * (idx)))
+	((gpriv)->info->regs[RCANFD_CFSTS] + (0x0c * (ch)) + (0x04 * (idx)))
 /* RSCFDnCFDCFPCTRx / RSCFDnCFPCTRx */
 #define RCANFD_CFPCTR(gpriv, ch, idx) \
-	(reg_gen4(gpriv, 0x0240, 0x01d8) + (0x0c * (ch)) + (0x04 * (idx)))
+	((gpriv)->info->regs[RCANFD_CFPCTR] + (0x0c * (ch)) + (0x04 * (idx)))
 
 /* RSCFDnCFDFESTS / RSCFDnFESTS */
 #define RCANFD_FESTS			(0x0238)
@@ -437,7 +437,7 @@
 /* CAN FD mode specific register map */
 
 /* RSCFDnCFDCmXXX -> RCANFD_F_XXX(m) */
-#define RCANFD_F_DCFG(gpriv, m)		(reg_gen4(gpriv, 0x1400, 0x0500) + (0x20 * (m)))
+#define RCANFD_F_DCFG(gpriv, m)		((gpriv)->info->regs[RCANFD_F_DCFG] + (0x20 * (m)))
 #define RCANFD_F_CFDCFG(m)		(0x0504 + (0x20 * (m)))
 #define RCANFD_F_CFDCTR(m)		(0x0508 + (0x20 * (m)))
 #define RCANFD_F_CFDSTS(m)		(0x050c + (0x20 * (m)))
@@ -453,7 +453,7 @@
 #define RCANFD_F_RMDF(q, b)		(0x200c + (0x04 * (b)) + (0x20 * (q)))
 
 /* RSCFDnCFDRFXXx -> RCANFD_F_RFXX(x) */
-#define RCANFD_F_RFOFFSET(gpriv)	reg_gen4(gpriv, 0x6000, 0x3000)
+#define RCANFD_F_RFOFFSET(gpriv)	((gpriv)->info->regs[RCANFD_RFOFFSET])
 #define RCANFD_F_RFID(gpriv, x)		(RCANFD_F_RFOFFSET(gpriv) + (0x80 * (x)))
 #define RCANFD_F_RFPTR(gpriv, x)	(RCANFD_F_RFOFFSET(gpriv) + 0x04 + (0x80 * (x)))
 #define RCANFD_F_RFFDSTS(gpriv, x)	(RCANFD_F_RFOFFSET(gpriv) + 0x08 + (0x80 * (x)))
@@ -461,7 +461,7 @@
 	(RCANFD_F_RFOFFSET(gpriv) + 0x0c + (0x80 * (x)) + (0x04 * (df)))
 
 /* RSCFDnCFDCFXXk -> RCANFD_F_CFXX(ch, k) */
-#define RCANFD_F_CFOFFSET(gpriv)	reg_gen4(gpriv, 0x6400, 0x3400)
+#define RCANFD_F_CFOFFSET(gpriv)	((gpriv)->info->regs[RCANFD_CFOFFSET])
 
 #define RCANFD_F_CFID(gpriv, ch, idx) \
 	(RCANFD_F_CFOFFSET(gpriv) + (0x180 * (ch)) + (0x80 * (idx)))
@@ -508,9 +508,20 @@
  */
 #define RCANFD_CFFIFO_IDX		0
 
+enum rcar_canfd_reg_offset_id {
+	RCANFD_RFCC,		/* RX FIFO Configuration/Control Register */
+	RCANFD_CFCC,		/* Common FIFO Configuration/Control Register */
+	RCANFD_CFSTS,		/* Common FIFO Status Register */
+	RCANFD_CFPCTR,		/* Common FIFO Pointer Control Register */
+	RCANFD_F_DCFG,		/* Global FD Configuration Register */
+	RCANFD_RFOFFSET,	/* Receive FIFO buffer access ID register */
+	RCANFD_CFOFFSET,	/* Transmit/receive FIFO buffer access ID register */
+};
+
 struct rcar_canfd_global;
 
 struct rcar_canfd_hw_info {
+	const u16 *regs;
 	u8 max_channels;
 	u8 postdiv;
 	/* hardware features */
@@ -588,13 +599,35 @@ static const struct can_bittiming_const rcar_canfd_bittiming_const = {
 	.brp_inc = 1,
 };
 
+static const u16 rcar_gen3_regs[] = {
+	[RCANFD_RFCC] = 0x00b8,
+	[RCANFD_CFCC] = 0x0118,
+	[RCANFD_CFSTS] = 0x0178,
+	[RCANFD_CFPCTR] = 0x01d8,
+	[RCANFD_F_DCFG] = 0x0500,
+	[RCANFD_RFOFFSET] = 0x3000,
+	[RCANFD_CFOFFSET] = 0x3400,
+};
+
+static const u16 rcar_gen4_regs[] = {
+	[RCANFD_RFCC] = 0x00c0,
+	[RCANFD_CFCC] = 0x0120,
+	[RCANFD_CFSTS] = 0x01e0,
+	[RCANFD_CFPCTR] = 0x0240,
+	[RCANFD_F_DCFG] = 0x1400,
+	[RCANFD_RFOFFSET] = 0x6000,
+	[RCANFD_CFOFFSET] = 0x6400,
+};
+
 static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
+	.regs = rcar_gen3_regs,
 	.max_channels = 2,
 	.postdiv = 2,
 	.shared_global_irqs = 1,
 };
 
 static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
+	.regs = rcar_gen4_regs,
 	.max_channels = 8,
 	.postdiv = 2,
 	.shared_global_irqs = 1,
@@ -603,6 +636,7 @@ static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
 };
 
 static const struct rcar_canfd_hw_info rzg2l_hw_info = {
+	.regs = rcar_gen3_regs,
 	.max_channels = 2,
 	.postdiv = 1,
 	.multi_channel_irqs = 1,
-- 
2.43.0

Re: [PATCH v4 06/11] can: rcar_canfd: Add register mapping table to struct rcar_canfd_hw_info

[Geert Uytterhoeven]

Hi Biju,

Thanks for your patch!

On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> R-Car Gen3 and Gen4 has some differences in the register offsets. Add a

have

> mapping table to handle these differences.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

RE: [PATCH v4 06/11] can: rcar_canfd: Add register mapping table to struct rcar_canfd_hw_info

[Biju Das]

Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 14 March 2025 14:47
> Subject: Re: [PATCH v4 06/11] can: rcar_canfd: Add register mapping table to struct rcar_canfd_hw_info
> 
> Hi Biju,
> 
> Thanks for your patch!
> 
> On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > R-Car Gen3 and Gen4 has some differences in the register offsets. Add
> > a
> 
> have

OK, will update commit description.

Cheers,
Biju

[PATCH v4 07/11] can: rcar_canfd: Add mask table to struct rcar_canfd_hw_info

[Biju Das]

R-Car Gen3 and Gen4 has some differences in the mask bits. Add a
mask table to handle these differences.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * Added prefix RCANFD_* to enum rcar_canfd_mask_id.
v3:
 * New patch.
---
 drivers/net/can/rcar/rcar_canfd.c | 68 +++++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 13 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index f7c78b1a2684..dd9d73b37d02 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -82,25 +82,24 @@
 #define RCANFD_GERFL_DEF		BIT(0)
 
 #define RCANFD_GERFL_ERR(gpriv, x) \
-	((x) & (reg_gen4(gpriv, RCANFD_GERFL_EEF0_7, \
-			 RCANFD_GERFL_EEF(0) | RCANFD_GERFL_EEF(1)) | \
-		RCANFD_GERFL_MES | \
+	((x) & ((gpriv)->info->mask_table[RCANFD_EEF_MASK] | RCANFD_GERFL_MES | \
 		((gpriv)->fdmode ? RCANFD_GERFL_CMPOF : 0)))
 
 /* AFL Rx rules registers */
 
 /* RSCFDnCFDGAFLCFG0 / RSCFDnGAFLCFG0 */
 #define RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \
-	(((x) & reg_gen4(gpriv, 0x1ff, 0xff)) << \
+	(((x) & (gpriv)->info->mask_table[RCANFD_RNC_MASK]) << \
 	 (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8)))
 
 #define RCANFD_GAFLCFG_GETRNC(gpriv, n, x) \
 	(((x) >> (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8))) & \
-	 reg_gen4(gpriv, 0x1ff, 0xff))
+	 (gpriv)->info->mask_table[RCANFD_RNC_MASK])
 
 /* RSCFDnCFDGAFLECTR / RSCFDnGAFLECTR */
 #define RCANFD_GAFLECTR_AFLDAE		BIT(8)
-#define RCANFD_GAFLECTR_AFLPN(gpriv, x)	((x) & reg_gen4(gpriv, 0x7f, 0x1f))
+#define RCANFD_GAFLECTR_AFLPN(gpriv, x)	((x) & \
+					 (gpriv)->info->mask_table[RCANFD_AFLPN_MASK])
 
 /* RSCFDnCFDGAFLIDj / RSCFDnGAFLIDj */
 #define RCANFD_GAFLID_GAFLLB		BIT(29)
@@ -118,13 +117,13 @@
 
 /* RSCFDnCFDCmNCFG - CAN FD only */
 #define RCANFD_NCFG_NTSEG2(gpriv, x) \
-	(((x) & reg_gen4(gpriv, 0x7f, 0x1f)) << reg_gen4(gpriv, 25, 24))
+	(((x) & (gpriv)->info->mask_table[RCANFD_NTSEG2_MASK]) << reg_gen4(gpriv, 25, 24))
 
 #define RCANFD_NCFG_NTSEG1(gpriv, x) \
-	(((x) & reg_gen4(gpriv, 0xff, 0x7f)) << reg_gen4(gpriv, 17, 16))
+	(((x) & (gpriv)->info->mask_table[RCANFD_NTSEG1_MASK]) << reg_gen4(gpriv, 17, 16))
 
 #define RCANFD_NCFG_NSJW(gpriv, x) \
-	(((x) & reg_gen4(gpriv, 0x7f, 0x1f)) << reg_gen4(gpriv, 10, 11))
+	(((x) & (gpriv)->info->mask_table[RCANFD_NSJW_MASK]) << reg_gen4(gpriv, 10, 11))
 
 #define RCANFD_NCFG_NBRP(x)		(((x) & 0x3ff) << 0)
 
@@ -186,13 +185,13 @@
 #define RCANFD_CERFL_ERR(x)		((x) & (0x7fff)) /* above bits 14:0 */
 
 /* RSCFDnCFDCmDCFG */
-#define RCANFD_DCFG_DSJW(gpriv, x)	(((x) & reg_gen4(gpriv, 0xf, 0x7)) << 24)
+#define RCANFD_DCFG_DSJW(gpriv, x)	(((x) & (gpriv)->info->mask_table[RCANFD_DSJW_MASK]) << 24)
 
 #define RCANFD_DCFG_DTSEG2(gpriv, x) \
-	(((x) & reg_gen4(gpriv, 0x0f, 0x7)) << reg_gen4(gpriv, 16, 20))
+	(((x) & (gpriv)->info->mask_table[RCANFD_DTSEG2_MASK]) << reg_gen4(gpriv, 16, 20))
 
 #define RCANFD_DCFG_DTSEG1(gpriv, x) \
-	(((x) & reg_gen4(gpriv, 0x1f, 0xf)) << reg_gen4(gpriv, 8, 16))
+	(((x) & (gpriv)->info->mask_table[RCANFD_DTSEG1_MASK]) << reg_gen4(gpriv, 8, 16))
 
 #define RCANFD_DCFG_DBRP(x)		(((x) & 0xff) << 0)
 
@@ -234,7 +233,7 @@
 
 /* RSCFDnCFDCFCCk */
 #define RCANFD_CFCC_CFTML(gpriv, x)	\
-	(((x) & reg_gen4(gpriv, 0x1f, 0xf)) << reg_gen4(gpriv, 16, 20))
+	(((x) & (gpriv)->info->mask_table[RCANFD_CFTML_MASK]) << reg_gen4(gpriv, 16, 20))
 #define RCANFD_CFCC_CFM(gpriv, x)	(((x) & 0x3) << reg_gen4(gpriv,  8, 16))
 #define RCANFD_CFCC_CFIM		BIT(12)
 #define RCANFD_CFCC_CFDC(gpriv, x)	(((x) & 0x7) << reg_gen4(gpriv, 21,  8))
@@ -518,9 +517,23 @@ enum rcar_canfd_reg_offset_id {
 	RCANFD_CFOFFSET,	/* Transmit/receive FIFO buffer access ID register */
 };
 
+enum rcar_canfd_mask_id {
+	RCANFD_EEF_MASK,	/* Global Error Flag */
+	RCANFD_RNC_MASK,	/* Rule Number for Channel */
+	RCANFD_AFLPN_MASK,	/* Acceptance Filter List Page Number */
+	RCANFD_NTSEG2_MASK,	/* Nominal Bit Rate Time Segment 2 Control */
+	RCANFD_NTSEG1_MASK,	/* Nominal Bit Rate Time Segment 1 Control */
+	RCANFD_NSJW_MASK,	/* Nominal Bit Rate Resynchronization Jump Width Control */
+	RCANFD_DSJW_MASK,	/* Data Bit Rate Resynchronization Jump Width Control */
+	RCANFD_DTSEG2_MASK,	/* Data Bit Rate Time Segment 2 Control */
+	RCANFD_DTSEG1_MASK,	/* Data Bit Rate Time Segment 1 Control */
+	RCANFD_CFTML_MASK,	/* Common FIFO TX Message Buffer Link */
+};
+
 struct rcar_canfd_global;
 
 struct rcar_canfd_hw_info {
+	const u32 *mask_table;
 	const u16 *regs;
 	u8 max_channels;
 	u8 postdiv;
@@ -619,7 +632,34 @@ static const u16 rcar_gen4_regs[] = {
 	[RCANFD_CFOFFSET] = 0x6400,
 };
 
+static const u32 rcar_gen3_mask_table[] = {
+	[RCANFD_EEF_MASK] = RCANFD_GERFL_EEF(0) | RCANFD_GERFL_EEF(1),
+	[RCANFD_RNC_MASK] = 0xff,
+	[RCANFD_AFLPN_MASK] = 0x1f,
+	[RCANFD_NTSEG2_MASK] = 0x1f,
+	[RCANFD_NTSEG1_MASK] = 0x7f,
+	[RCANFD_NSJW_MASK] = 0x1f,
+	[RCANFD_DSJW_MASK] = 0x7,
+	[RCANFD_DTSEG2_MASK] = 0x7,
+	[RCANFD_DTSEG1_MASK] = 0xf,
+	[RCANFD_CFTML_MASK] = 0xf,
+};
+
+static const u32 rcar_gen4_mask_table[] = {
+	[RCANFD_EEF_MASK] = RCANFD_GERFL_EEF0_7,
+	[RCANFD_RNC_MASK] = 0x1ff,
+	[RCANFD_AFLPN_MASK] = 0x7f,
+	[RCANFD_NTSEG2_MASK] = 0x7f,
+	[RCANFD_NTSEG1_MASK] = 0xff,
+	[RCANFD_NSJW_MASK] = 0x7f,
+	[RCANFD_DSJW_MASK] = 0xf,
+	[RCANFD_DTSEG2_MASK] = 0xf,
+	[RCANFD_DTSEG1_MASK] = 0x1f,
+	[RCANFD_CFTML_MASK] = 0x1f,
+};
+
 static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
+	.mask_table = rcar_gen3_mask_table,
 	.regs = rcar_gen3_regs,
 	.max_channels = 2,
 	.postdiv = 2,
@@ -627,6 +667,7 @@ static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
 };
 
 static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
+	.mask_table = rcar_gen4_mask_table,
 	.regs = rcar_gen4_regs,
 	.max_channels = 8,
 	.postdiv = 2,
@@ -636,6 +677,7 @@ static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
 };
 
 static const struct rcar_canfd_hw_info rzg2l_hw_info = {
+	.mask_table = rcar_gen3_mask_table,
 	.regs = rcar_gen3_regs,
 	.max_channels = 2,
 	.postdiv = 1,
-- 
2.43.0

Re: [PATCH v4 07/11] can: rcar_canfd: Add mask table to struct rcar_canfd_hw_info

[Geert Uytterhoeven]

Hi Biju,

Thanks for your patch!

On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> R-Car Gen3 and Gen4 has some differences in the mask bits. Add a

have

> mask table to handle these differences.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Suggestions for improvements below.

> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -82,25 +82,24 @@
>  #define RCANFD_GERFL_DEF               BIT(0)
>
>  #define RCANFD_GERFL_ERR(gpriv, x) \
> -       ((x) & (reg_gen4(gpriv, RCANFD_GERFL_EEF0_7, \
> -                        RCANFD_GERFL_EEF(0) | RCANFD_GERFL_EEF(1)) | \
> -               RCANFD_GERFL_MES | \
> +       ((x) & ((gpriv)->info->mask_table[RCANFD_EEF_MASK] | RCANFD_GERFL_MES | \
>                 ((gpriv)->fdmode ? RCANFD_GERFL_CMPOF : 0)))

RZ/G3E documents only EFF bits 0-5.
I think this can just be replaced by gpriv->channels_mask << 16,
especially as there is only one user of RCANFD_GERFL_ERR().

>
>  /* AFL Rx rules registers */
>
>  /* RSCFDnCFDGAFLCFG0 / RSCFDnGAFLCFG0 */
>  #define RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \
> -       (((x) & reg_gen4(gpriv, 0x1ff, 0xff)) << \
> +       (((x) & (gpriv)->info->mask_table[RCANFD_RNC_MASK]) << \
>          (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8)))

All other mask values are just the maximum value of a parameter.
E.g. the RNC mask is the number of supported rules - 1.
So perhaps store these as parameters instead?

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

RE: [PATCH v4 07/11] can: rcar_canfd: Add mask table to struct rcar_canfd_hw_info

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 14 March 2025 14:47
> Subject: Re: [PATCH v4 07/11] can: rcar_canfd: Add mask table to struct rcar_canfd_hw_info
> 
> Hi Biju,
> 
> Thanks for your patch!
> 
> On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > R-Car Gen3 and Gen4 has some differences in the mask bits. Add a
> 
> have

OK.

> 
> > mask table to handle these differences.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Suggestions for improvements below.
> 
> > --- a/drivers/net/can/rcar/rcar_canfd.c
> > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > @@ -82,25 +82,24 @@
> >  #define RCANFD_GERFL_DEF               BIT(0)
> >
> >  #define RCANFD_GERFL_ERR(gpriv, x) \
> > -       ((x) & (reg_gen4(gpriv, RCANFD_GERFL_EEF0_7, \
> > -                        RCANFD_GERFL_EEF(0) | RCANFD_GERFL_EEF(1)) | \
> > -               RCANFD_GERFL_MES | \
> > +       ((x) & ((gpriv)->info->mask_table[RCANFD_EEF_MASK] |
> > + RCANFD_GERFL_MES | \
> >                 ((gpriv)->fdmode ? RCANFD_GERFL_CMPOF : 0)))
> 
> RZ/G3E documents only EFF bits 0-5.
> I think this can just be replaced by gpriv->channels_mask << 16, especially as there is only one user
> of RCANFD_GERFL_ERR().

Agreed and will drop RCANFD_GERFL_EEF0_7 as it is unused.

> 
> >
> >  /* AFL Rx rules registers */
> >
> >  /* RSCFDnCFDGAFLCFG0 / RSCFDnGAFLCFG0 */  #define
> > RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \
> > -       (((x) & reg_gen4(gpriv, 0x1ff, 0xff)) << \
> > +       (((x) & (gpriv)->info->mask_table[RCANFD_RNC_MASK]) << \
> >          (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16,
> > 8)))
> 
> All other mask values are just the maximum value of a parameter.
> E.g. the RNC mask is the number of supported rules - 1.
> So perhaps store these as parameters instead?

OK, will introduce num_supported_rules variable in struct rcar_canfd_hw_info.

Cheers,
Biju

[PATCH v4 08/11] can: rcar_canfd: Add shift table to struct rcar_canfd_hw_info

[Biju Das]

R-Car Gen3 and Gen4 has some differences in the shift bits. Add a
shift table to handle these differences. After this drop the unused
functions reg_gen4() and is_gen4().

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * Added prefix RCANFD_* to enum rcar_canfd_shift_id.
v3:
 * New patch.
---
 drivers/net/can/rcar/rcar_canfd.c | 84 +++++++++++++++++++++++--------
 1 file changed, 62 insertions(+), 22 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index dd9d73b37d02..f84b88ccb1de 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -90,11 +90,13 @@
 /* RSCFDnCFDGAFLCFG0 / RSCFDnGAFLCFG0 */
 #define RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \
 	(((x) & (gpriv)->info->mask_table[RCANFD_RNC_MASK]) << \
-	 (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8)))
+	 ((gpriv)->info->shift_table[RCANFD_FIRST_RNC_SH] - ((n) & 1) * \
+	  (gpriv)->info->shift_table[RCANFD_SECOND_RNC_SH]))
 
 #define RCANFD_GAFLCFG_GETRNC(gpriv, n, x) \
-	(((x) >> (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8))) & \
-	 (gpriv)->info->mask_table[RCANFD_RNC_MASK])
+	(((x) >> ((gpriv)->info->shift_table[RCANFD_FIRST_RNC_SH] - ((n) & 1) * \
+		  (gpriv)->info->shift_table[RCANFD_SECOND_RNC_SH])) & \
+		  (gpriv)->info->mask_table[RCANFD_RNC_MASK])
 
 /* RSCFDnCFDGAFLECTR / RSCFDnGAFLECTR */
 #define RCANFD_GAFLECTR_AFLDAE		BIT(8)
@@ -117,13 +119,16 @@
 
 /* RSCFDnCFDCmNCFG - CAN FD only */
 #define RCANFD_NCFG_NTSEG2(gpriv, x) \
-	(((x) & (gpriv)->info->mask_table[RCANFD_NTSEG2_MASK]) << reg_gen4(gpriv, 25, 24))
+	(((x) & (gpriv)->info->mask_table[RCANFD_NTSEG2_MASK]) << \
+	 (gpriv)->info->shift_table[RCANFD_NTSEG2_SH])
 
 #define RCANFD_NCFG_NTSEG1(gpriv, x) \
-	(((x) & (gpriv)->info->mask_table[RCANFD_NTSEG1_MASK]) << reg_gen4(gpriv, 17, 16))
+	(((x) & (gpriv)->info->mask_table[RCANFD_NTSEG1_MASK]) << \
+	 (gpriv)->info->shift_table[RCANFD_NTSEG1_SH])
 
 #define RCANFD_NCFG_NSJW(gpriv, x) \
-	(((x) & (gpriv)->info->mask_table[RCANFD_NSJW_MASK]) << reg_gen4(gpriv, 10, 11))
+	(((x) & (gpriv)->info->mask_table[RCANFD_NSJW_MASK]) << \
+	 (gpriv)->info->shift_table[RCANFD_NSJW_SH])
 
 #define RCANFD_NCFG_NBRP(x)		(((x) & 0x3ff) << 0)
 
@@ -188,10 +193,12 @@
 #define RCANFD_DCFG_DSJW(gpriv, x)	(((x) & (gpriv)->info->mask_table[RCANFD_DSJW_MASK]) << 24)
 
 #define RCANFD_DCFG_DTSEG2(gpriv, x) \
-	(((x) & (gpriv)->info->mask_table[RCANFD_DTSEG2_MASK]) << reg_gen4(gpriv, 16, 20))
+	(((x) & (gpriv)->info->mask_table[RCANFD_DTSEG2_MASK]) << \
+	 (gpriv)->info->shift_table[RCANFD_DTSEG2_SH])
 
 #define RCANFD_DCFG_DTSEG1(gpriv, x) \
-	(((x) & (gpriv)->info->mask_table[RCANFD_DTSEG1_MASK]) << reg_gen4(gpriv, 8, 16))
+	(((x) & (gpriv)->info->mask_table[RCANFD_DTSEG1_MASK]) << \
+	 (gpriv)->info->shift_table[RCANFD_DTSEG1_SH])
 
 #define RCANFD_DCFG_DBRP(x)		(((x) & 0xff) << 0)
 
@@ -233,10 +240,11 @@
 
 /* RSCFDnCFDCFCCk */
 #define RCANFD_CFCC_CFTML(gpriv, x)	\
-	(((x) & (gpriv)->info->mask_table[RCANFD_CFTML_MASK]) << reg_gen4(gpriv, 16, 20))
-#define RCANFD_CFCC_CFM(gpriv, x)	(((x) & 0x3) << reg_gen4(gpriv,  8, 16))
+	(((x) & (gpriv)->info->mask_table[RCANFD_CFTML_MASK]) << \
+	 (gpriv)->info->shift_table[RCANFD_CFTML_SH])
+#define RCANFD_CFCC_CFM(gpriv, x)	(((x) & 0x3) << (gpriv)->info->shift_table[RCANFD_CFM_SH])
 #define RCANFD_CFCC_CFIM		BIT(12)
-#define RCANFD_CFCC_CFDC(gpriv, x)	(((x) & 0x7) << reg_gen4(gpriv, 21,  8))
+#define RCANFD_CFCC_CFDC(gpriv, x)	(((x) & 0x7) << (gpriv)->info->shift_table[RCANFD_CFDC_SH])
 #define RCANFD_CFCC_CFPLS(x)		(((x) & 0x7) << 4)
 #define RCANFD_CFCC_CFTXIE		BIT(2)
 #define RCANFD_CFCC_CFE			BIT(0)
@@ -530,11 +538,25 @@ enum rcar_canfd_mask_id {
 	RCANFD_CFTML_MASK,	/* Common FIFO TX Message Buffer Link */
 };
 
+enum rcar_canfd_shift_id {
+	RCANFD_FIRST_RNC_SH,	/* Rule Number for Channel x */
+	RCANFD_SECOND_RNC_SH,	/* Rule Number for Channel x + 1 */
+	RCANFD_NTSEG2_SH,	/* Nominal Bit Rate Time Segment 2 Control */
+	RCANFD_NTSEG1_SH,	/* Nominal Bit Rate Time Segment 1 Control */
+	RCANFD_NSJW_SH,		/* Nominal Bit Rate Resynchronization Jump Width Control */
+	RCANFD_DTSEG2_SH,	/* Data Bit Rate Time Segment 2 Control */
+	RCANFD_DTSEG1_SH,	/* Data Bit Rate Time Segment 1 Control */
+	RCANFD_CFTML_SH,	/* Common FIFO TX Message Buffer Link */
+	RCANFD_CFM_SH,		/* Common FIFO Mode */
+	RCANFD_CFDC_SH,		/* Common FIFO Depth Configuration */
+};
+
 struct rcar_canfd_global;
 
 struct rcar_canfd_hw_info {
 	const u32 *mask_table;
 	const u16 *regs;
+	const u8 *shift_table;
 	u8 max_channels;
 	u8 postdiv;
 	/* hardware features */
@@ -658,8 +680,35 @@ static const u32 rcar_gen4_mask_table[] = {
 	[RCANFD_CFTML_MASK] = 0x1f,
 };
 
+static const u8 rcar_gen3_shift_table[] = {
+	[RCANFD_FIRST_RNC_SH] = 24,
+	[RCANFD_SECOND_RNC_SH] = 8,
+	[RCANFD_NTSEG2_SH] = 24,
+	[RCANFD_NTSEG1_SH] = 16,
+	[RCANFD_NSJW_SH] = 11,
+	[RCANFD_DTSEG2_SH] = 20,
+	[RCANFD_DTSEG1_SH] = 16,
+	[RCANFD_CFTML_SH] = 20,
+	[RCANFD_CFM_SH] = 16,
+	[RCANFD_CFDC_SH] = 8,
+};
+
+static const u8 rcar_gen4_shift_table[] = {
+	[RCANFD_FIRST_RNC_SH] = 16,
+	[RCANFD_SECOND_RNC_SH] = 16,
+	[RCANFD_NTSEG2_SH] = 25,
+	[RCANFD_NTSEG1_SH] = 17,
+	[RCANFD_NSJW_SH] = 10,
+	[RCANFD_DTSEG2_SH] = 16,
+	[RCANFD_DTSEG1_SH] = 8,
+	[RCANFD_CFTML_SH] = 16,
+	[RCANFD_CFM_SH] = 8,
+	[RCANFD_CFDC_SH] = 21,
+};
+
 static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
 	.mask_table = rcar_gen3_mask_table,
+	.shift_table = rcar_gen3_shift_table,
 	.regs = rcar_gen3_regs,
 	.max_channels = 2,
 	.postdiv = 2,
@@ -668,6 +717,7 @@ static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
 
 static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
 	.mask_table = rcar_gen4_mask_table,
+	.shift_table = rcar_gen4_shift_table,
 	.regs = rcar_gen4_regs,
 	.max_channels = 8,
 	.postdiv = 2,
@@ -678,6 +728,7 @@ static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
 
 static const struct rcar_canfd_hw_info rzg2l_hw_info = {
 	.mask_table = rcar_gen3_mask_table,
+	.shift_table = rcar_gen3_shift_table,
 	.regs = rcar_gen3_regs,
 	.max_channels = 2,
 	.postdiv = 1,
@@ -685,17 +736,6 @@ static const struct rcar_canfd_hw_info rzg2l_hw_info = {
 };
 
 /* Helper functions */
-static inline bool is_gen4(struct rcar_canfd_global *gpriv)
-{
-	return gpriv->info == &rcar_gen4_hw_info;
-}
-
-static inline u32 reg_gen4(struct rcar_canfd_global *gpriv,
-			   u32 gen4, u32 not_gen4)
-{
-	return is_gen4(gpriv) ? gen4 : not_gen4;
-}
-
 static inline void rcar_canfd_update(u32 mask, u32 val, u32 __iomem *reg)
 {
 	u32 data = readl(reg);
-- 
2.43.0

Re: [PATCH v4 08/11] can: rcar_canfd: Add shift table to struct rcar_canfd_hw_info

[Geert Uytterhoeven]

Hi Biju,

Thanks for your patch!

On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> R-Car Gen3 and Gen4 has some differences in the shift bits. Add a

have

> shift table to handle these differences. After this drop the unused
> functions reg_gen4() and is_gen4().
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

A suggestion for improvement below.

> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -90,11 +90,13 @@
>  /* RSCFDnCFDGAFLCFG0 / RSCFDnGAFLCFG0 */
>  #define RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \
>         (((x) & (gpriv)->info->mask_table[RCANFD_RNC_MASK]) << \
> -        (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8)))
> +        ((gpriv)->info->shift_table[RCANFD_FIRST_RNC_SH] - ((n) & 1) * \
> +         (gpriv)->info->shift_table[RCANFD_SECOND_RNC_SH]))

Both shifts are dictated by the field width:
  - R-Car Gen4 packs 2 values in a 32-bit word, using a field width
    of 16 bits,
  - R-Car Gen3 packs up to 4 values in a 32-bit word, using a field
    width of 8 bits.
The odd split in two shifts is due to the MSB-first numbering of
the fields.  Hence I think it would be better to use a field width
parameter, and derive the proper shift value from that (like you have
already done for ICU ;-)

Note that the formula will have to be reworked anyway to support the
third channel on R-Car V3H_2, as "n & "1 will no longer fly here...

>  #define RCANFD_GAFLCFG_GETRNC(gpriv, n, x) \
> -       (((x) >> (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8))) & \
> -        (gpriv)->info->mask_table[RCANFD_RNC_MASK])
> +       (((x) >> ((gpriv)->info->shift_table[RCANFD_FIRST_RNC_SH] - ((n) & 1) * \
> +                 (gpriv)->info->shift_table[RCANFD_SECOND_RNC_SH])) & \
> +                 (gpriv)->info->mask_table[RCANFD_RNC_MASK])

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

RE: [PATCH v4 08/11] can: rcar_canfd: Add shift table to struct rcar_canfd_hw_info

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 14 March 2025 14:48
> Subject: Re: [PATCH v4 08/11] can: rcar_canfd: Add shift table to struct rcar_canfd_hw_info
> 
> Hi Biju,
> 
> Thanks for your patch!
> 
> On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > R-Car Gen3 and Gen4 has some differences in the shift bits. Add a
> 
> have
> 
> > shift table to handle these differences. After this drop the unused
> > functions reg_gen4() and is_gen4().
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> A suggestion for improvement below.
> 
> > --- a/drivers/net/can/rcar/rcar_canfd.c
> > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > @@ -90,11 +90,13 @@
> >  /* RSCFDnCFDGAFLCFG0 / RSCFDnGAFLCFG0 */  #define
> > RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \
> >         (((x) & (gpriv)->info->mask_table[RCANFD_RNC_MASK]) << \
> > -        (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8)))
> > +        ((gpriv)->info->shift_table[RCANFD_FIRST_RNC_SH] - ((n) & 1) * \
> > +         (gpriv)->info->shift_table[RCANFD_SECOND_RNC_SH]))
> 
> Both shifts are dictated by the field width:
>   - R-Car Gen4 packs 2 values in a 32-bit word, using a field width
>     of 16 bits,
>   - R-Car Gen3 packs up to 4 values in a 32-bit word, using a field
>     width of 8 bits.
> The odd split in two shifts is due to the MSB-first numbering of the fields.  Hence I think it would
> be better to use a field width parameter, and derive the proper shift value from that (like you have
> already done for ICU ;-)

What about ((32  - (n + 1)* field_width) ?
 
{24,16,8} for n = {0, 1, 2}

And 

{16,0} for n = {0, 1}

Cheers,
Biju

RE: [PATCH v4 08/11] can: rcar_canfd: Add shift table to struct rcar_canfd_hw_info

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 14 March 2025 14:48
> Subject: Re: [PATCH v4 08/11] can: rcar_canfd: Add shift table to struct rcar_canfd_hw_info
> 
> Hi Biju,
> 
> Thanks for your patch!
> 
> On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > R-Car Gen3 and Gen4 has some differences in the shift bits. Add a
> 
> have
> 
> > shift table to handle these differences. After this drop the unused
> > functions reg_gen4() and is_gen4().
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> A suggestion for improvement below.
> 
> > --- a/drivers/net/can/rcar/rcar_canfd.c
> > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > @@ -90,11 +90,13 @@
> >  /* RSCFDnCFDGAFLCFG0 / RSCFDnGAFLCFG0 */  #define
> > RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \
> >         (((x) & (gpriv)->info->mask_table[RCANFD_RNC_MASK]) << \
> > -        (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8)))
> > +        ((gpriv)->info->shift_table[RCANFD_FIRST_RNC_SH] - ((n) & 1) * \
> > +         (gpriv)->info->shift_table[RCANFD_SECOND_RNC_SH]))
> 
> Both shifts are dictated by the field width:
>   - R-Car Gen4 packs 2 values in a 32-bit word, using a field width
>     of 16 bits,
>   - R-Car Gen3 packs up to 4 values in a 32-bit word, using a field
>     width of 8 bits.
> The odd split in two shifts is due to the MSB-first numbering of the fields.  Hence I think it would
> be better to use a field width parameter, and derive the proper shift value from that (like you have
> already done for ICU ;-)
> 
> Note that the formula will have to be reworked anyway to support the third channel on R-Car V3H_2, as
> "n & "1 will no longer fly here...

Agreed. Will introduce rnc_field_width variable in struct rcar_canfd_hw_info and
Simplify the calculation using the formula (32 - (n + 1) * rnc_field_width).
Which will work for third channel on R-Car V3H_2 as well.

Cheers,
Biju

Re: [PATCH v4 08/11] can: rcar_canfd: Add shift table to struct rcar_canfd_hw_info

[Geert Uytterhoeven]

Hi Biju,

On Sat, 15 Mar 2025 at 18:31, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > R-Car Gen3 and Gen4 has some differences in the shift bits. Add a
> > > shift table to handle these differences. After this drop the unused
> > > functions reg_gen4() and is_gen4().
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > A suggestion for improvement below.
> >
> > > --- a/drivers/net/can/rcar/rcar_canfd.c
> > > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > > @@ -90,11 +90,13 @@
> > >  /* RSCFDnCFDGAFLCFG0 / RSCFDnGAFLCFG0 */  #define
> > > RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \
> > >         (((x) & (gpriv)->info->mask_table[RCANFD_RNC_MASK]) << \
> > > -        (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8)))
> > > +        ((gpriv)->info->shift_table[RCANFD_FIRST_RNC_SH] - ((n) & 1) * \
> > > +         (gpriv)->info->shift_table[RCANFD_SECOND_RNC_SH]))
> >
> > Both shifts are dictated by the field width:
> >   - R-Car Gen4 packs 2 values in a 32-bit word, using a field width
> >     of 16 bits,
> >   - R-Car Gen3 packs up to 4 values in a 32-bit word, using a field
> >     width of 8 bits.
> > The odd split in two shifts is due to the MSB-first numbering of the fields.  Hence I think it would
> > be better to use a field width parameter, and derive the proper shift value from that (like you have
> > already done for ICU ;-)
> >
> > Note that the formula will have to be reworked anyway to support the third channel on R-Car V3H_2, as
> > "n & "1 will no longer fly here...
>
> Agreed. Will introduce rnc_field_width variable in struct rcar_canfd_hw_info and
> Simplify the calculation using the formula (32 - (n + 1) * rnc_field_width).
> Which will work for third channel on R-Car V3H_2 as well.

Note that you still need to mask n, as n can be up to 7 (on R-Car V4H),
while the register holds only 2 or 4 values.

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

RE: [PATCH v4 08/11] can: rcar_canfd: Add shift table to struct rcar_canfd_hw_info

[Biju Das]

Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 16 March 2025 09:53
> Subject: Re: [PATCH v4 08/11] can: rcar_canfd: Add shift table to struct rcar_canfd_hw_info
> 
> Hi Biju,
> 
> On Sat, 15 Mar 2025 at 18:31, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Thu, 6 Mar 2025
> > > at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > R-Car Gen3 and Gen4 has some differences in the shift bits. Add a
> > > > shift table to handle these differences. After this drop the
> > > > unused functions reg_gen4() and is_gen4().
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > >
> > > A suggestion for improvement below.
> > >
> > > > --- a/drivers/net/can/rcar/rcar_canfd.c
> > > > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > > > @@ -90,11 +90,13 @@
> > > >  /* RSCFDnCFDGAFLCFG0 / RSCFDnGAFLCFG0 */  #define
> > > > RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \
> > > >         (((x) & (gpriv)->info->mask_table[RCANFD_RNC_MASK]) << \
> > > > -        (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8)))
> > > > +        ((gpriv)->info->shift_table[RCANFD_FIRST_RNC_SH] - ((n) & 1) * \
> > > > +         (gpriv)->info->shift_table[RCANFD_SECOND_RNC_SH]))
> > >
> > > Both shifts are dictated by the field width:
> > >   - R-Car Gen4 packs 2 values in a 32-bit word, using a field width
> > >     of 16 bits,
> > >   - R-Car Gen3 packs up to 4 values in a 32-bit word, using a field
> > >     width of 8 bits.
> > > The odd split in two shifts is due to the MSB-first numbering of the
> > > fields.  Hence I think it would be better to use a field width
> > > parameter, and derive the proper shift value from that (like you
> > > have already done for ICU ;-)
> > >
> > > Note that the formula will have to be reworked anyway to support the
> > > third channel on R-Car V3H_2, as "n & "1 will no longer fly here...
> >
> > Agreed. Will introduce rnc_field_width variable in struct
> > rcar_canfd_hw_info and Simplify the calculation using the formula (32 - (n + 1) * rnc_field_width).
> > Which will work for third channel on R-Car V3H_2 as well.
> 
> Note that you still need to mask n, as n can be up to 7 (on R-Car V4H), while the register holds only
> 2 or 4 values.

OK, will add separate patch for fixing

-#define RCANFD_GAFLCFG(ch)		(0x009c + (0x04 * ((ch) / 2)))
+#define RCANFD_GAFLCFG(ch, n)		(0x009c + (0x04 * ((ch) / (n))))

Where n is gpriv->info->rnc_per_reg.

and SETRNC is simplified to

#define RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \
 	(((x) & ((gpriv)->info->num_supported_rules - 1)) << \
-	 (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8)))
+	(32 - (((n) % (gpriv)->info->rnc_per_reg + 1) * (gpriv)->info->rnc_field_width)))


Cheers,
Biju

Re: [PATCH v4 08/11] can: rcar_canfd: Add shift table to struct rcar_canfd_hw_info

[Geert Uytterhoeven]

Hi Biju,

On Sun, 16 Mar 2025 at 18:02, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > On Sat, 15 Mar 2025 at 18:31, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Thu, 6 Mar 2025
> > > > at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > > R-Car Gen3 and Gen4 has some differences in the shift bits. Add a
> > > > > shift table to handle these differences. After this drop the
> > > > > unused functions reg_gen4() and is_gen4().
> > > > >
> > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > >
> > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > >
> > > > A suggestion for improvement below.
> > > >
> > > > > --- a/drivers/net/can/rcar/rcar_canfd.c
> > > > > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > > > > @@ -90,11 +90,13 @@
> > > > >  /* RSCFDnCFDGAFLCFG0 / RSCFDnGAFLCFG0 */  #define
> > > > > RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \
> > > > >         (((x) & (gpriv)->info->mask_table[RCANFD_RNC_MASK]) << \
> > > > > -        (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8)))
> > > > > +        ((gpriv)->info->shift_table[RCANFD_FIRST_RNC_SH] - ((n) & 1) * \
> > > > > +         (gpriv)->info->shift_table[RCANFD_SECOND_RNC_SH]))
> > > >
> > > > Both shifts are dictated by the field width:
> > > >   - R-Car Gen4 packs 2 values in a 32-bit word, using a field width
> > > >     of 16 bits,
> > > >   - R-Car Gen3 packs up to 4 values in a 32-bit word, using a field
> > > >     width of 8 bits.
> > > > The odd split in two shifts is due to the MSB-first numbering of the
> > > > fields.  Hence I think it would be better to use a field width
> > > > parameter, and derive the proper shift value from that (like you
> > > > have already done for ICU ;-)
> > > >
> > > > Note that the formula will have to be reworked anyway to support the
> > > > third channel on R-Car V3H_2, as "n & "1 will no longer fly here...
> > >
> > > Agreed. Will introduce rnc_field_width variable in struct
> > > rcar_canfd_hw_info and Simplify the calculation using the formula (32 - (n + 1) * rnc_field_width).
> > > Which will work for third channel on R-Car V3H_2 as well.
> >
> > Note that you still need to mask n, as n can be up to 7 (on R-Car V4H), while the register holds only
> > 2 or 4 values.
>
> OK, will add separate patch for fixing
>
> -#define RCANFD_GAFLCFG(ch)             (0x009c + (0x04 * ((ch) / 2)))
> +#define RCANFD_GAFLCFG(ch, n)          (0x009c + (0x04 * ((ch) / (n))))
>
> Where n is gpriv->info->rnc_per_reg.

I think you better pass gpriv instead of n, i.e. RCANFD_GAFLCFG(gpriv, ch),
for consistency with other macros.

> and SETRNC is simplified to
>
> #define RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \
>         (((x) & ((gpriv)->info->num_supported_rules - 1)) << \
> -        (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8)))
> +       (32 - (((n) % (gpriv)->info->rnc_per_reg + 1) * (gpriv)->info->rnc_field_width)))

FTR, gpriv->info->rnc_per_reg == 32 / gpriv->info->rnc_field_width.

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

RE: [PATCH v4 08/11] can: rcar_canfd: Add shift table to struct rcar_canfd_hw_info

[Biju Das]

Hi Geert,


Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 17 March 2025 09:54
> Subject: Re: [PATCH v4 08/11] can: rcar_canfd: Add shift table to struct rcar_canfd_hw_info
> 
> Hi Biju,
> 
> On Sun, 16 Mar 2025 at 18:02, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Sat, 15 Mar 2025
> > > at 18:31, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Thu, 6 Mar
> > > > > 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > > > R-Car Gen3 and Gen4 has some differences in the shift bits.
> > > > > > Add a shift table to handle these differences. After this drop
> > > > > > the unused functions reg_gen4() and is_gen4().
> > > > > >
> > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > >
> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > >
> > > > > A suggestion for improvement below.
> > > > >
> > > > > > --- a/drivers/net/can/rcar/rcar_canfd.c
> > > > > > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > > > > > @@ -90,11 +90,13 @@
> > > > > >  /* RSCFDnCFDGAFLCFG0 / RSCFDnGAFLCFG0 */  #define
> > > > > > RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \
> > > > > >         (((x) & (gpriv)->info->mask_table[RCANFD_RNC_MASK]) << \
> > > > > > -        (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8)))
> > > > > > +        ((gpriv)->info->shift_table[RCANFD_FIRST_RNC_SH] - ((n) & 1) * \
> > > > > > +         (gpriv)->info->shift_table[RCANFD_SECOND_RNC_SH]))
> > > > >
> > > > > Both shifts are dictated by the field width:
> > > > >   - R-Car Gen4 packs 2 values in a 32-bit word, using a field width
> > > > >     of 16 bits,
> > > > >   - R-Car Gen3 packs up to 4 values in a 32-bit word, using a field
> > > > >     width of 8 bits.
> > > > > The odd split in two shifts is due to the MSB-first numbering of
> > > > > the fields.  Hence I think it would be better to use a field
> > > > > width parameter, and derive the proper shift value from that
> > > > > (like you have already done for ICU ;-)
> > > > >
> > > > > Note that the formula will have to be reworked anyway to support
> > > > > the third channel on R-Car V3H_2, as "n & "1 will no longer fly here...
> > > >
> > > > Agreed. Will introduce rnc_field_width variable in struct
> > > > rcar_canfd_hw_info and Simplify the calculation using the formula (32 - (n + 1) *
> rnc_field_width).
> > > > Which will work for third channel on R-Car V3H_2 as well.
> > >
> > > Note that you still need to mask n, as n can be up to 7 (on R-Car
> > > V4H), while the register holds only
> > > 2 or 4 values.
> >
> > OK, will add separate patch for fixing
> >
> > -#define RCANFD_GAFLCFG(ch)             (0x009c + (0x04 * ((ch) / 2)))
> > +#define RCANFD_GAFLCFG(ch, n)          (0x009c + (0x04 * ((ch) / (n))))
> >
> > Where n is gpriv->info->rnc_per_reg.
> 
> I think you better pass gpriv instead of n, i.e. RCANFD_GAFLCFG(gpriv, ch), for consistency with other
> macros.

Agreed. 

> 
> > and SETRNC is simplified to
> >
> > #define RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \
> >         (((x) & ((gpriv)->info->num_supported_rules - 1)) << \
> > -        (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, 8)))
> > +       (32 - (((n) % (gpriv)->info->rnc_per_reg + 1) *
> > + (gpriv)->info->rnc_field_width)))
> 
> FTR, gpriv->info->rnc_per_reg == 32 / gpriv->info->rnc_field_width.

Agreed. Sure, I will change accordingly.

Cheers,
Biju

[PATCH v4 09/11] can: rcar_canfd: Add only_internal_clks variable to struct rcar_canfd_hw_info

[Biju Das]

All SoCs supports extenal clock, but RZ/G3E has only internal clocks. Add
only_internal_clks to struct rcar_canfd_hw_info to handle this difference.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * No change.
v2->v3:
 * No change
v1->v2:
 * No change.
---
 drivers/net/can/rcar/rcar_canfd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index f84b88ccb1de..0042b1e06c76 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -564,6 +564,7 @@ struct rcar_canfd_hw_info {
 	unsigned multi_channel_irqs:1;	/* Has multiple channel irqs */
 	unsigned ch_interface_mode:1;	/* Has channel interface mode */
 	unsigned shared_can_reg:1;	/* Has shared classical can registers */
+	unsigned only_internal_clks:1;	/* Has only internal clocks */
 };
 
 /* Channel priv data */
@@ -2061,7 +2062,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 		fcan_freq = clk_get_rate(gpriv->can_clk) / info->postdiv;
 	} else {
 		fcan_freq = clk_get_rate(gpriv->can_clk);
-		gpriv->extclk = true;
+		gpriv->extclk = !gpriv->info->only_internal_clks;
 	}
 
 	addr = devm_platform_ioremap_resource(pdev, 0);
-- 
2.43.0

Re: [PATCH v4 09/11] can: rcar_canfd: Add only_internal_clks variable to struct rcar_canfd_hw_info

[Geert Uytterhoeven]

Hi Biju,

Thanks for your patch!

On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> All SoCs supports extenal clock, but RZ/G3E has only internal clocks. Add

existing SoCs .... an external

> only_internal_clks to struct rcar_canfd_hw_info to handle this difference.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

RE: [PATCH v4 09/11] can: rcar_canfd: Add only_internal_clks variable to struct rcar_canfd_hw_info

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 14 March 2025 14:48
> Subject: Re: [PATCH v4 09/11] can: rcar_canfd: Add only_internal_clks variable to struct
> rcar_canfd_hw_info
> 
> Hi Biju,
> 
> Thanks for your patch!
> 
> On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > All SoCs supports extenal clock, but RZ/G3E has only internal clocks.
> > Add
> 
> existing SoCs .... an external

OK, Will use "All existing SoCs supports an external clock"

Cheers,
Biju

Re: [PATCH v4 09/11] can: rcar_canfd: Add only_internal_clks variable to struct rcar_canfd_hw_info

[Geert Uytterhoeven]

Hi Biju,

On Sat, 15 Mar 2025 at 18:33, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > All SoCs supports extenal clock, but RZ/G3E has only internal clocks.
> > > Add
> >
> > existing SoCs .... an external
>
> OK, Will use "All existing SoCs supports an external clock"

support (sorry, missed that before).

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

[PATCH v4 10/11] can: rcar_canfd: Enhance multi_channel_irqs handling

[Biju Das]

Currently multi_channel_irqs has only 2 channels. But RZ/G3E has six
channels. Enhance multi_channel_irqs handling to support more than two
channels.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * No change.
v2->v3:
 * No change.
v1->v2:
 * No change.
---
 drivers/net/can/rcar/rcar_canfd.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 0042b1e06c76..862f06b4b91e 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -1866,16 +1866,19 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
 
 	if (info->multi_channel_irqs) {
 		char *irq_name;
+		char name[10];
 		int err_irq;
 		int tx_irq;
 
-		err_irq = platform_get_irq_byname(pdev, ch == 0 ? "ch0_err" : "ch1_err");
+		scnprintf(name, 10, "ch%u_err", ch);
+		err_irq = platform_get_irq_byname(pdev, name);
 		if (err_irq < 0) {
 			err = err_irq;
 			goto fail;
 		}
 
-		tx_irq = platform_get_irq_byname(pdev, ch == 0 ? "ch0_trx" : "ch1_trx");
+		scnprintf(name, 10, "ch%u_trx", ch);
+		tx_irq = platform_get_irq_byname(pdev, name);
 		if (tx_irq < 0) {
 			err = tx_irq;
 			goto fail;
-- 
2.43.0

Re: [PATCH v4 10/11] can: rcar_canfd: Enhance multi_channel_irqs handling

[Geert Uytterhoeven]

On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Currently multi_channel_irqs has only 2 channels. But RZ/G3E has six
> channels. Enhance multi_channel_irqs handling to support more than two
> channels.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

[PATCH v4 11/11] can: rcar_canfd: Add RZ/G3E support

[Biju Das]

The CAN-FD IP found on the RZ/G3E SoC is similar to R-Car Gen4, but
it has no external clock instead it has clk_ram, it has 6 channels
and supports 20 interrupts. Add support for RZ/G3E CAN-FD driver.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * No change.
v2->v3:
 * Replaced gen4_type entry with mask_table, shift_table, regs,
   ch_interface_mode and shared_can_reg.
v1->v2:
 * No change.
---
 drivers/net/can/rcar/rcar_canfd.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 862f06b4b91e..68b15f3ab14f 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -736,6 +736,18 @@ static const struct rcar_canfd_hw_info rzg2l_hw_info = {
 	.multi_channel_irqs = 1,
 };
 
+static const struct rcar_canfd_hw_info r9a09g047_hw_info = {
+	.mask_table = rcar_gen4_mask_table,
+	.shift_table = rcar_gen4_shift_table,
+	.regs = rcar_gen4_regs,
+	.max_channels = 6,
+	.postdiv = 1,
+	.multi_channel_irqs = 1,
+	.ch_interface_mode = 1,
+	.shared_can_reg = 1,
+	.only_internal_clks = 1,
+};
+
 /* Helper functions */
 static inline void rcar_canfd_update(u32 mask, u32 val, u32 __iomem *reg)
 {
@@ -1979,6 +1991,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	u32 rule_entry = 0;
 	bool fdmode = true;			/* CAN FD only mode - default */
 	char name[9] = "channelX";
+	struct clk *clk_ram;
 	int i;
 
 	info = of_device_get_match_data(dev);
@@ -2068,6 +2081,11 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 		gpriv->extclk = !gpriv->info->only_internal_clks;
 	}
 
+	clk_ram = devm_clk_get_optional_enabled(dev, "ram_clk");
+	if (IS_ERR(clk_ram))
+		return dev_err_probe(dev, PTR_ERR(clk_ram),
+				     "cannot get ram clock\n");
+
 	addr = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(addr)) {
 		err = PTR_ERR(addr);
@@ -2230,6 +2248,7 @@ static SIMPLE_DEV_PM_OPS(rcar_canfd_pm_ops, rcar_canfd_suspend,
 
 static const __maybe_unused struct of_device_id rcar_canfd_of_table[] = {
 	{ .compatible = "renesas,r8a779a0-canfd", .data = &rcar_gen4_hw_info },
+	{ .compatible = "renesas,r9a09g047-canfd", .data = &r9a09g047_hw_info },
 	{ .compatible = "renesas,rcar-gen3-canfd", .data = &rcar_gen3_hw_info },
 	{ .compatible = "renesas,rcar-gen4-canfd", .data = &rcar_gen4_hw_info },
 	{ .compatible = "renesas,rzg2l-canfd", .data = &rzg2l_hw_info },
-- 
2.43.0

Re: [PATCH v4 11/11] can: rcar_canfd: Add RZ/G3E support

[Geert Uytterhoeven]

Hi Biju,

On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> The CAN-FD IP found on the RZ/G3E SoC is similar to R-Car Gen4, but
> it has no external clock instead it has clk_ram, it has 6 channels
> and supports 20 interrupts. Add support for RZ/G3E CAN-FD driver.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -1979,6 +1991,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>         u32 rule_entry = 0;
>         bool fdmode = true;                     /* CAN FD only mode - default */
>         char name[9] = "channelX";
> +       struct clk *clk_ram;
>         int i;
>
>         info = of_device_get_match_data(dev);
> @@ -2068,6 +2081,11 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>                 gpriv->extclk = !gpriv->info->only_internal_clks;
>         }
>
> +       clk_ram = devm_clk_get_optional_enabled(dev, "ram_clk");
> +       if (IS_ERR(clk_ram))
> +               return dev_err_probe(dev, PTR_ERR(clk_ram),
> +                                    "cannot get ram clock\n");
> +

clk_ram is unused.

>         addr = devm_platform_ioremap_resource(pdev, 0);
>         if (IS_ERR(addr)) {
>                 err = PTR_ERR(addr);

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

RE: [PATCH v4 11/11] can: rcar_canfd: Add RZ/G3E support

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 14 March 2025 14:48
> Subject: Re: [PATCH v4 11/11] can: rcar_canfd: Add RZ/G3E support
> 
> Hi Biju,
> 
> On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > The CAN-FD IP found on the RZ/G3E SoC is similar to R-Car Gen4, but it
> > has no external clock instead it has clk_ram, it has 6 channels and
> > supports 20 interrupts. Add support for RZ/G3E CAN-FD driver.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/net/can/rcar/rcar_canfd.c
> > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > @@ -1979,6 +1991,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> >         u32 rule_entry = 0;
> >         bool fdmode = true;                     /* CAN FD only mode - default */
> >         char name[9] = "channelX";
> > +       struct clk *clk_ram;
> >         int i;
> >
> >         info = of_device_get_match_data(dev); @@ -2068,6 +2081,11 @@
> > static int rcar_canfd_probe(struct platform_device *pdev)
> >                 gpriv->extclk = !gpriv->info->only_internal_clks;
> >         }
> >
> > +       clk_ram = devm_clk_get_optional_enabled(dev, "ram_clk");
> > +       if (IS_ERR(clk_ram))
> > +               return dev_err_probe(dev, PTR_ERR(clk_ram),
> > +                                    "cannot get ram clock\n");
> > +
> 
> clk_ram is unused.

clk_ram is local variable and is used for error checking.
devm_clk_get_optional_enabled() is dev-res managed clock
and the will be enabled during probe() and get disabled during unbind 
similar to "fck".

Am I missing anything here?

Please let me know.

Cheers,
Biju

Re: [PATCH v4 11/11] can: rcar_canfd: Add RZ/G3E support

[Geert Uytterhoeven]

Hi Biju,

On Fri, 14 Mar 2025 at 15:59, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > The CAN-FD IP found on the RZ/G3E SoC is similar to R-Car Gen4, but it
> > > has no external clock instead it has clk_ram, it has 6 channels and
> > > supports 20 interrupts. Add support for RZ/G3E CAN-FD driver.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/net/can/rcar/rcar_canfd.c
> > > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > > @@ -1979,6 +1991,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> > >         u32 rule_entry = 0;
> > >         bool fdmode = true;                     /* CAN FD only mode - default */
> > >         char name[9] = "channelX";
> > > +       struct clk *clk_ram;
> > >         int i;
> > >
> > >         info = of_device_get_match_data(dev); @@ -2068,6 +2081,11 @@
> > > static int rcar_canfd_probe(struct platform_device *pdev)
> > >                 gpriv->extclk = !gpriv->info->only_internal_clks;
> > >         }
> > >
> > > +       clk_ram = devm_clk_get_optional_enabled(dev, "ram_clk");
> > > +       if (IS_ERR(clk_ram))
> > > +               return dev_err_probe(dev, PTR_ERR(clk_ram),
> > > +                                    "cannot get ram clock\n");
> > > +
> >
> > clk_ram is unused.
>
> clk_ram is local variable and is used for error checking.
> devm_clk_get_optional_enabled() is dev-res managed clock
> and the will be enabled during probe() and get disabled during unbind
> similar to "fck".

Thanks!

> Am I missing anything here?

No, I had missed the "_enabled" part....

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

Re: [PATCH v4 00/11] Add support for RZ/G3E CANFD

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 790 bytes --]

On 06.03.2025 12:42:39, Biju Das wrote:
> The CAN-FD module on RZ/G3E is very similar to the one on both R-Car V4H
> and RZ/G2L, but differs in some hardware parameters:
>  * No external clock, but instead has ram clock.
>  * Support up to 6 channels.
>  * 20 interrupts.
> 
> This patch series depend upon [1]
> [1] https://lore.kernel.org/all/20250220094516.126598-1-biju.das.jz@bp.renesas.com/

I'll send a PR for linux-can today. Ping me after net is merged to
net-next, so I can take this series.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH v4 00/11] Add support for RZ/G3E CANFD

[Biju Das]

Hi Marc,


> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 14 March 2025 12:38
> Subject: Re: [PATCH v4 00/11] Add support for RZ/G3E CANFD
> 
> On 06.03.2025 12:42:39, Biju Das wrote:
> > The CAN-FD module on RZ/G3E is very similar to the one on both R-Car
> > V4H and RZ/G2L, but differs in some hardware parameters:
> >  * No external clock, but instead has ram clock.
> >  * Support up to 6 channels.
> >  * 20 interrupts.
> >
> > This patch series depend upon [1]
> > [1]
> > https://lore.kernel.org/all/20250220094516.126598-1-biju.das.jz@bp.ren
> > esas.com/
> 
> I'll send a PR for linux-can today. Ping me after net is merged to net-next, so I can take this
> series.

Sure. Will do.

Cheers,
Biju