RE: [PATCH v2] can: rcar_canfd: Add Renesas R-Car CAN FD driver

Hi Rob,

Thanks for the review comments.

On Thu, Mar 03, 2016 at 03:38:35PM +0000, Ramesh Shanmugasundaram wrote:

quoted

This patch adds support for the CAN FD controller found in Renesas
R-Car SoCs. The controller operates in CAN FD mode by default.

CAN FD mode supports both Classical CAN & CAN FD frame formats. The
controller supports ISO 11898-1:2015 CAN FD format only.

This controller supports two channels and the driver can enable either
or both of the channels.

Driver uses Rx FIFOs (one per channel) for reception & Common FIFOs
(one per channel) for transmission. Rx filter rules are configured to
the minimum (one per channel) and it accepts Standard, Extended, Data
& Remote Frame combinations.

Note: There are few documentation errors in R-Car Gen3 Hardware User
Manual v0.5E with respect to CAN FD controller. They are listed below:

1. CAN FD interrupt numbers 29 & 30 are listed as per channel
interrupts. However, they are common to both channels (i.e.) they are
global and channel interrupts respectively.

2. CANFD clock is derived from PLL1. This is not documented.

3. CANFD clock is further divided by (1/2) within the CAN FD controller.
This is not documented.

4. The minimum value of NTSEG1 in RSCFDnCFDCmNCFG register is 2 Tq. It
is mentioned 4 Tq in the manual.

5. The maximum number of message RAM area the controller can use is
3584 bytes. It is specified 10752 bytes in the manual.

Signed-off-by: Ramesh Shanmugasundaram
[off-list ref]
---
Changes since v1:
	* Removed testmodes & debugfs code (suggested by Oliver H)
	* Fixed tx path race issue by introducing lock (suggested by Marc K)
	* Removed __maybe_unused attribute of rcar_canfd_of_table

Thanks,
Ramesh
---
 .../devicetree/bindings/net/can/rcar_canfd.txt     |   86 ++
 drivers/net/can/Kconfig                            |   10 +
 drivers/net/can/Makefile                           |    1 +
 drivers/net/can/rcar_canfd.c                       | 1624

++++++++++++++++++++

quoted

 4 files changed, 1721 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/net/can/rcar_canfd.txt
 create mode 100644 drivers/net/can/rcar_canfd.c

diff --git a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt

b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
new file mode 100644
index 0000000..4299bd8

--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt

@@ -0,0 +1,86 @@
+Renesas R-Car CAN FD controller Device Tree Bindings
+----------------------------------------------------
+
+Required properties:
+- compatible: Must contain one or more of the following:
+  - "renesas,rcar-gen3-canfd" for R-Car Gen3 compatible controller.
+  - "renesas,r8a7795-canfd" for R8A7795 (R-Car H3) compatible

controller.

quoted

+
+  When compatible with the generic version, nodes must list the
+ SoC-specific version corresponding to the platform first, followed
+ by the  family-specific and/or generic versions.
+
+- reg: physical base address and size of the R-Car CAN FD register map.
+- interrupts: interrupt specifier for the Global & Channel interrupts
+- clocks: phandles and clock specifiers for 3 clock inputs.
+- clock-names: 3 clock input name strings: "fck", "canfd", "can_clk".
+- pinctrl-0: pin control group to be used for this controller.
+- pinctrl-names: must be "default".
+
+Required properties for "renesas,r8a7795-canfd" compatible:
+In R8A7795 SoC, canfd clock is a div6 clock and can be used by both
+CAN and CAN FD controller at the same time. It needs to be scaled to
+maximum frequency if any of these controllers use it. This is done
+using the below properties.
+
+- assigned-clocks: phandle of canfd clock.
+- assigned-clock-rates: maximum frequency of this clock.
+
+Each channel is represented as a child node. They can be
+enabled/disabled using "status" property.

How many channels? Required or optional?

Two channels and it is a required property. I will update as below in the next revision.

Required child node:
The controller supports two channels and each is represented as a child
node. The names of the child nodes are "channel0" and "channel1" respectively.
Each child node supports the "status" property only, which is used to
enable/disable the respective channel.

quoted

+
+Example
+-------
+
+SoC common .dtsi file:
+
+		canfd: canfd@e66c0000 {

can@e66c0000

Thanks. I will update.

Thanks,
Ramesh