Re: [PATCH v2 1/5] dt-bindings: net: can: renesas,rcar-canfd: Document RZ/G2L SoC

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Prabhakar,

On Mon, Jul 19, 2021 at 4:39 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add CANFD binding documentation for Renesas RZ/G2L SoC.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Just some bikeshedding on the exact naming below ;-)

> --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> @@ -91,6 +92,59 @@ required:
>    - channel0
>    - channel1
>
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - renesas,rzg2l-canfd
> +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
> +
> +    interrupt-names:
> +      items:
> +        - const: g_error
> +        - const: g_rx_fifo
> +        - const: can0_error

s/error/err/?

> +        - const: can0_tx
> +        - const: can0_tx_rx_fifo_receive_completion
> +        - const: can1_error
> +        - const: can1_tx
> +        - const: can1_tx_rx_fifo_receive_completion

s/receive/rx/?

Some are also a bit long to type.
Perhaps use naming closer to the User's Manual?

INTRCANGERR => g_err
INTRCANGRECC => g_recc
INTRCAN0ERR => ch0_err
INTRCAN0REC => ch0_rec
INTRCAN0TRX => ch0_trx
INTRCAN1ERR => ch1_err
INTRCAN1REC => ch1_rec
INTRCAN1TRX => ch1_trx

These do not have "_int" suffixes...

> +
> +    resets:
> +      items:
> +        - description: CANFD_RSTP_N
> +        - description: CANFD_RSTC_N
> +
> +  required:
> +    - interrupt-names
> +else:
> +  properties:
> +    interrupts:
> +      items:
> +        - description: Channel interrupt
> +        - description: Global interrupt
> +
> +    interrupt-names:
> +      items:
> +        - const: ch_int
> +        - const: g_int

... and these do have "_int" suffixes.

> +
> +    resets:
> +      items:
> +        - description: CANFD reset
> +
>  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