| Message ID | 20210719143811.2135-2-prabhakar.mahadev-lad.rj@bp.renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/5] dt-bindings: net: can: renesas,rcar-canfd: Document RZ/G2L SoC | expand |
Hi Lad, On Mon, 2021-07-19 at 15:38 +0100, Lad Prabhakar 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> > --- > .../bindings/net/can/renesas,rcar-canfd.yaml | 66 +++++++++++++++++-- > 1 file changed, 60 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml > index 0b33ba9ccb47..4fb6dd370904 100644 > --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml > +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml > @@ -30,13 +30,15 @@ properties: > - renesas,r8a77995-canfd # R-Car D3 > - const: renesas,rcar-gen3-canfd # R-Car Gen3 and RZ/G2 > > + - items: > + - enum: > + - renesas,r9a07g044-canfd # RZ/G2{L,LC} > + - const: renesas,rzg2l-canfd # RZ/G2L family > + > reg: > maxItems: 1 > > - interrupts: > - items: > - - description: Channel interrupt > - - description: Global interrupt > + interrupts: true > > clocks: > maxItems: 3 > @@ -50,8 +52,7 @@ properties: > power-domains: > maxItems: 1 > > - resets: > - maxItems: 1 > + resets: true > > renesas,no-can-fd: > $ref: /schemas/types.yaml#/definitions/flag > @@ -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 > + - const: can0_tx > + - const: can0_tx_rx_fifo_receive_completion > + - const: can1_error > + - const: can1_tx > + - const: can1_tx_rx_fifo_receive_completion > + > + resets: > + items: > + - description: CANFD_RSTP_N > + - description: CANFD_RSTC_N Do you know what the "P" and "C" stands for? It would be nice if the description could tell us what the reset lines are used for. I would prefer if you used these names (or shortened versions, for example "rstp_n", "rstc_n") as "reset-names" and let the driver reference the resets by name instead of by index. regards Philipp
Hi Geert, Thank you for the review. On Tue, Jul 20, 2021 at 11:21 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > 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... > Agreed thanks for the input. > > + > > + 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. > indeed Cheers, Prabhakar > > + > > + 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
Hi Geert, On Tue, Jul 20, 2021 at 4:11 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Tue, Jul 20, 2021 at 4:31 PM Lad, Prabhakar > <prabhakar.csengg@gmail.com> wrote: > > On Tue, Jul 20, 2021 at 11:22 AM Philipp Zabel <p.zabel@pengutronix.de> wrote: > > > On Mon, 2021-07-19 at 15:38 +0100, Lad Prabhakar 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> > > > > > --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml > > > > +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml > > > > > + resets: > > > > + items: > > > > + - description: CANFD_RSTP_N > > > > + - description: CANFD_RSTC_N > > > > > > Do you know what the "P" and "C" stands for? It would be nice if the > > > description could tell us what the reset lines are used for. > > > > > unfortunately the HW manual does not mention anything about "P" and "C" :( > > > > > I would prefer if you used these names (or shortened versions, for > > > example "rstp_n", "rstc_n") as "reset-names" and let the driver > > > reference the resets by name instead of by index. > > > > > OK will do that and maxItems:2 for resets. > > > > @Geert, for R-Car Gen3 does "canfd_rst" (as it's a module reset) > > sounds good for reset-names? Or do you have any other suggestions? > > I wouldn't bother with reset-names on R-Car, as there is only a > single reset. > OK will keep "description: CANFD reset" for R-Car as done in the current patch and just add reset-names only for RZ/G2L SoC. > BTW, does there exist a generally-accepted reset-equivalent of "fck" > ("Functional ClocK")? > None that I am aware of (Couple of binding docs have "rst"), but maybe Philipp could have some suggestions. Cheers, Prabhakar > 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
Hi Prabhakar, On Tue, 2021-07-20 at 15:31 +0100, Lad, Prabhakar wrote: > Hi Philipp, > > Thank you for the review. > > On Tue, Jul 20, 2021 at 11:22 AM Philipp Zabel <p.zabel@pengutronix.de> wrote: > > Hi Lad, Sorry I mixed up your name. > > On Mon, 2021-07-19 at 15:38 +0100, Lad Prabhakar 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> > > > --- > > > .../bindings/net/can/renesas,rcar-canfd.yaml | 66 +++++++++++++++++-- > > > 1 file changed, 60 insertions(+), 6 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml > > > index 0b33ba9ccb47..4fb6dd370904 100644 > > > --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml > > > +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml > > > @@ -30,13 +30,15 @@ properties: > > > - renesas,r8a77995-canfd # R-Car D3 > > > - const: renesas,rcar-gen3-canfd # R-Car Gen3 and RZ/G2 > > > > > > + - items: > > > + - enum: > > > + - renesas,r9a07g044-canfd # RZ/G2{L,LC} > > > + - const: renesas,rzg2l-canfd # RZ/G2L family > > > + > > > reg: > > > maxItems: 1 > > > > > > - interrupts: > > > - items: > > > - - description: Channel interrupt > > > - - description: Global interrupt > > > + interrupts: true > > > > > > clocks: > > > maxItems: 3 > > > @@ -50,8 +52,7 @@ properties: > > > power-domains: > > > maxItems: 1 > > > > > > - resets: > > > - maxItems: 1 > > > + resets: true > > > > > > renesas,no-can-fd: > > > $ref: /schemas/types.yaml#/definitions/flag > > > @@ -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 > > > + - const: can0_tx > > > + - const: can0_tx_rx_fifo_receive_completion > > > + - const: can1_error > > > + - const: can1_tx > > > + - const: can1_tx_rx_fifo_receive_completion > > > + > > > + resets: > > > + items: > > > + - description: CANFD_RSTP_N > > > + - description: CANFD_RSTC_N > > > > Do you know what the "P" and "C" stands for? It would be nice if the > > description could tell us what the reset lines are used for. > > > unfortunately the HW manual does not mention anything about "P" and "C" :( Yes, unfortunately this is all too common. > > I would prefer if you used these names (or shortened versions, for > > example "rstp_n", "rstc_n") as "reset-names" and let the driver > > reference the resets by name instead of by index. > > > OK will do that and maxItems:2 for resets. > > @Geert, for R-Car Gen3 does "canfd_rst" (as it's a module reset) > sounds good for reset-names? Or do you have any other suggestions? I agree with Geert here. Assuming no second reset will be discovered for R-Car Gen3 later, there is no need to invent a name. regards Philipp
diff --git a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml index 0b33ba9ccb47..4fb6dd370904 100644 --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml @@ -30,13 +30,15 @@ properties: - renesas,r8a77995-canfd # R-Car D3 - const: renesas,rcar-gen3-canfd # R-Car Gen3 and RZ/G2 + - items: + - enum: + - renesas,r9a07g044-canfd # RZ/G2{L,LC} + - const: renesas,rzg2l-canfd # RZ/G2L family + reg: maxItems: 1 - interrupts: - items: - - description: Channel interrupt - - description: Global interrupt + interrupts: true clocks: maxItems: 3 @@ -50,8 +52,7 @@ properties: power-domains: maxItems: 1 - resets: - maxItems: 1 + resets: true renesas,no-can-fd: $ref: /schemas/types.yaml#/definitions/flag @@ -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 + - const: can0_tx + - const: can0_tx_rx_fifo_receive_completion + - const: can1_error + - const: can1_tx + - const: can1_tx_rx_fifo_receive_completion + + 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 + + resets: + items: + - description: CANFD reset + unevaluatedProperties: false examples: