RE: [PATCH] dt-bindings: can: rcar_can: Add r8a774a1 support

[Chris Paterson]

Hello Simon,

> From: Simon Horman <horms@xxxxxxxxxxxx>
> Sent: 22 August 2018 11:49
> 
> On Fri, Aug 17, 2018 at 03:38:23PM +0100, Fabrizio Castro wrote:
> > Document RZ/G2M (r8a774a1) SoC bindings.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@xxxxxxxxxxxxxx>
> > Reviewed-by: Biju Das <biju.das@xxxxxxxxxxxxxx>
> 
> Reviewed-by: Simon Horman <horms+renesas@xxxxxxxxxxxx>

Thank you.

> 
> > ---
> > This patch applies on top of next-20180817
> >
> >  Documentation/devicetree/bindings/net/can/rcar_can.txt | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt
> > b/Documentation/devicetree/bindings/net/can/rcar_can.txt
> > index 94a7f33..84afc78 100644
> > --- a/Documentation/devicetree/bindings/net/can/rcar_can.txt
> > +++ b/Documentation/devicetree/bindings/net/can/rcar_can.txt
> > @@ -4,6 +4,7 @@ Renesas R-Car CAN controller Device Tree Bindings
> > Required properties:
> >  - compatible: "renesas,can-r8a7743" if CAN controller is a part of R8A7743
> SoC.
> >  	      "renesas,can-r8a7745" if CAN controller is a part of R8A7745 SoC.
> > +	      "renesas,can-r8a774a1" if CAN controller is a part of R8A774A1 SoC.
> >  	      "renesas,can-r8a7778" if CAN controller is a part of R8A7778 SoC.
> >  	      "renesas,can-r8a7779" if CAN controller is a part of R8A7779 SoC.
> >  	      "renesas,can-r8a7790" if CAN controller is a part of R8A7790 SoC.
> > @@ -16,7 +17,8 @@ Required properties:
> >  	      "renesas,rcar-gen1-can" for a generic R-Car Gen1 compatible device.
> >  	      "renesas,rcar-gen2-can" for a generic R-Car Gen2 or RZ/G1
> >  	      compatible device.
> > -	      "renesas,rcar-gen3-can" for a generic R-Car Gen3 compatible device.
> > +	      "renesas,rcar-gen3-can" for a generic R-Car Gen3 or RZ/G2
> > +	      compatible device.
> >  	      When compatible with the generic version, nodes must list the
> >  	      SoC-specific version corresponding to the platform first
> >  	      followed by the generic version.
> > @@ -24,7 +26,9 @@ Required properties:
> >  - reg: physical base address and size of the R-Car CAN register map.
> >  - interrupts: interrupt specifier for the sole interrupt.
> >  - clocks: phandles and clock specifiers for 3 CAN clock inputs.
> > -- clock-names: 3 clock input name strings: "clkp1", "clkp2", "can_clk".
> > +- clock-names: 2 clock input name strings for RZ/G2: "clkp1",
> > +"can_clk", and
> 
> Minor comment: Personally I would start a new sentence at "and".
> 
> Question: Is a driver update required so support 2-clock SoCs?

The driver will work okay as-is.

In theory CAN could be broken if renesas,can-clock-select is set to 0x1 (clkp2) in the DT, as this value will be written to the CAN Clock Select Register. However if the documentation is followed there will be no problems.

We should probably update the driver to fix this though, which will be a change specific to all RZ/G2 devices, so perhaps we should also be adding a "renesas,rzg-gen2-can" family compatible string as well? (to driver and documentation)

Kind regards, Chris

> 
> > +	       3 clock input name strings for every other SoC: "clkp1", "clkp2",
> > +	       "can_clk".
> >  - pinctrl-0: pin control group to be used for this controller.
> >  - pinctrl-names: must be "default".
> >
> > --
> > 2.7.4
> >