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

On Wed, Aug 22, 2018 at 03:12:26PM +0000, Chris Paterson wrote:

Hello Simon,

quoted

From: Simon Horman <horms@verge.net.au>
Sent: 22 August 2018 11:49

On Fri, Aug 17, 2018 at 03:38:23PM +0100, Fabrizio Castro wrote:

quoted

Document RZ/G2M (r8a774a1) SoC bindings.

Signed-off-by: Fabrizio Castro <redacted>
Reviewed-by: Biju Das <redacted>

Reviewed-by: Simon Horman <redacted>

Thank you.

quoted

quoted

---
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.

quoted

 	      "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)

Yes, I think that sounds reasonable.

But in that case we should pre-emptively not use renesas,rcar-gen3-can for
RZ/G2.