On 03/03/2016 04:38 PM, Ramesh Shanmugasundaram wrote:
> 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 
> <ramesh.shanmugasunda...@bp.renesas.com>
> ---
> 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 
> ++++++++++++++++++++
>  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.
> +
> +  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.
> +
> +Example
> +-------
> +
> +SoC common .dtsi file:
> +
> +             canfd: canfd@e66c0000 {
> +                     compatible = "renesas,r8a7795-canfd",
> +                                  "renesas,rcar-gen3-canfd";
> +                     reg = <0 0xe66c0000 0 0x8000>;
> +                     interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>,
> +                                <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
> +                     clocks = <&cpg CPG_MOD 914>,
> +                            <&cpg CPG_CORE R8A7795_CLK_CANFD>,
> +                            <&can_clk>;
> +                     clock-names = "fck", "canfd", "can_clk";
> +                     assigned-clocks = <&cpg CPG_CORE R8A7795_CLK_CANFD>;
> +                     assigned-clock-rates = <40000000>;
> +                     power-domains = <&cpg>;
> +                     status = "disabled";
> +
> +                     channel0 {
> +                             status = "disabled";
> +                     };
> +
> +                     channel1 {
> +                             status = "disabled";
> +                     };
> +             };
> +
> +Board specific .dts file:
> +
> +E.g. below enables Channel 1 alone in the board.
> +
> +&canfd {
> +        pinctrl-0 = <&canfd1_pins>;
> +        pinctrl-names = "default";
> +        status = "okay";
> +
> +     channel1 {
> +             status = "okay";
> +     };
> +};
> +
> +E.g. below enables Channel 0 alone in the board using External clock
> +as fCAN clock.
> +
> +&canfd {
> +        pinctrl-0 = <&canfd0_pins &can_clk_pins>;
> +        pinctrl-names = "default";
> +        status = "okay";
> +
> +     channel0 {
> +             status = "okay";
> +     };
> +};
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index 0d40aef..0ecb4fe 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -114,6 +114,16 @@ config CAN_RCAR
>         To compile this driver as a module, choose M here: the module will
>         be called rcar_can.
>  
> +config CANFD_RCAR
> +     tristate "Renesas R-Car CAN FD controller"
> +     depends on ARCH_RENESAS || ARM
> +     ---help---
> +       Say Y here if you want to use CAN FD controller found on
> +       Renesas R-Car SoCs.
> +
> +       To compile this driver as a module, choose M here: the module will
> +       be called rcar_canfd.
> +
>  config CAN_SUN4I
>       tristate "Allwinner A10 CAN controller"
>       depends on MACH_SUN4I || MACH_SUN7I || COMPILE_TEST
> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index e3db0c8..401b150 100644
> --- a/drivers/net/can/Makefile
> +++ b/drivers/net/can/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_CAN_JANZ_ICAN3)        += janz-ican3.o
>  obj-$(CONFIG_CAN_MSCAN)              += mscan/
>  obj-$(CONFIG_CAN_M_CAN)              += m_can/
>  obj-$(CONFIG_CAN_RCAR)               += rcar_can.o
> +obj-$(CONFIG_CANFD_RCAR)     += rcar_canfd.o
>  obj-$(CONFIG_CAN_SJA1000)    += sja1000/
>  obj-$(CONFIG_CAN_SUN4I)              += sun4i_can.o
>  obj-$(CONFIG_CAN_TI_HECC)    += ti_hecc.o
> diff --git a/drivers/net/can/rcar_canfd.c b/drivers/net/can/rcar_canfd.c
> new file mode 100644
> index 0000000..56e089d
> --- /dev/null
> +++ b/drivers/net/can/rcar_canfd.c
> @@ -0,0 +1,1624 @@
> +/* Renesas R-Car CAN FD device driver
> + *
> + * Copyright (C) 2015 Renesas Electronics Corp.
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <linux/errno.h>
> +#include <linux/netdevice.h>
> +#include <linux/platform_device.h>
> +#include <linux/can/led.h>
> +#include <linux/can/dev.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/bitmap.h>
> +#include <linux/bitops.h>
> +#include <linux/iopoll.h>
> +
> +#define RCANFD_DRV_NAME                      "rcar_canfd"
> +
> +#define RCANFD_FIFO_DEPTH            8       /* Tx FIFO depth */
> +#define RCANFD_NAPI_WEIGHT           8       /* Rx poll quota */
> +
> +#define RCANFD_NUM_CHANNELS          2
> +#define RCANFD_CHANNELS_MASK         0x3     /* Two channels max */

(BIT(RCANFD_NUM_CHANNELS) - 1

> +
> +/* Rx FIFO is a global resource of the controller. There are 8 such FIFOs
> + * available. Each channel gets a dedicated Rx FIFO (i.e.) the channel
> + * number is added to RFFIFO index.
> + */
> +#define RCANFD_RFFIFO_IDX            0
> +
> +/* Tx/Rx or Common FIFO is a per channel resource. Each channel has 3 Common
> + * FIFOs dedicated to them. Use the first (index 0) FIFO out of the 3 for Tx.
> + */
> +#define RCANFD_CFFIFO_IDX            0
> +
> +/* Global register bits */
> +#define RCANFD_GINTF_CANFD           BIT(0)
> +
> +#define RCANFD_GCFG_TPRI             BIT(0)
> +#define RCANFD_GCFG_DCE                      BIT(1)
> +#define RCANFD_GCFG_DCS                      BIT(4)
> +#define RCANFD_GCFG_CMPOC            BIT(5)
> +#define RCANFD_GCFG_EEFE             BIT(6)
> +
> +#define RCANFD_GCTR_SLPR             BIT(2)
> +#define RCANFD_GCTR_MODEMASK         (0x3)
> +#define RCANFD_GCTR_GOPM             (0x0)
> +#define RCANFD_GCTR_GRESET           (0x1)
> +#define RCANFD_GCTR_GHLT             (0x2)
> +
> +#define RCANFD_GCTR_DEIE             BIT(8)
> +#define RCANFD_GCTR_MEIE             BIT(9)
> +#define RCANFD_GCTR_THLEIE           BIT(10)
> +#define RCANFD_GCTR_CFMPOFIE         BIT(11)
> +#define RCANFD_GCTR_TSRST            BIT(16)
> +
> +#define RCANFD_GSTS_RAMINIT          BIT(3)
> +#define RCANFD_GSTS_SLP                      BIT(2)
> +#define RCANFD_GSTS_HLT                      BIT(1)
> +#define RCANFD_GSTS_RESET            BIT(0)
> +
> +#define RCANFD_GSTS_GNOPM            (BIT(0) | BIT(1) | BIT(2) | BIT(3))
> +
> +/* Channel register bits */
> +#define RCANFD_CCTR_CSLPR            BIT(2)
> +#define RCANFD_CCTR_MODEMASK         (0x3)
> +#define RCANFD_CCTR_COPM             (0x0)
> +#define RCANFD_CCTR_CRESET           (0x1)
> +#define RCANFD_CCTR_CHLT             (0x2)
> +#define RCANFD_CCTR_CTMASK           (0x3 << 25)
> +#define RCANFD_CCTR_CTMS_ILB         (0x3 << 25)
> +#define RCANFD_CCTR_CTME             BIT(24)
> +#define RCANFD_CCTR_ERRD             BIT(23)
> +#define RCANFD_CCTR_BOMMASK          (0x3 << 21)
> +#define RCANFD_CCTR_BOM_ENTRY                (0x1 << 21)
> +#define RCANFD_CCTR_TDCVFIE          BIT(19)
> +#define RCANFD_CCTR_SOCOIE           BIT(18)
> +#define RCANFD_CCTR_EOCOIE           BIT(17)
> +#define RCANFD_CCTR_TAIE             BIT(16)
> +#define RCANFD_CCTR_ALIE             BIT(15)
> +#define RCANFD_CCTR_BLIE             BIT(14)
> +#define RCANFD_CCTR_OLIE             BIT(13)
> +#define RCANFD_CCTR_BORIE            BIT(12)
> +#define RCANFD_CCTR_BOEIE            BIT(11)
> +#define RCANFD_CCTR_EPIE             BIT(10)
> +#define RCANFD_CCTR_EWIE             BIT(9)
> +#define RCANFD_CCTR_BEIE             BIT(8)
> +
> +#define RCANFD_CSTS_COM                      BIT(7)
> +#define RCANFD_CSTS_REC                      BIT(6)
> +#define RCANFD_CSTS_TRM                      BIT(5)
> +#define RCANFD_CSTS_BO                       BIT(4)
> +#define RCANFD_CSTS_EP                       BIT(3)
> +#define RCANFD_CSTS_SLP                      BIT(2)
> +#define RCANFD_CSTS_HALT             BIT(1)
> +#define RCANFD_CSTS_RESET            BIT(0)
> +
> +#define RCANFD_CSTS_TECCNT(x)                (((x) >> 24) & 0xff)
> +#define RCANFD_CSTS_RECCNT(x)                (((x) >> 16) & 0xff)
> +
> +/* Bit Configuration register */
> +#define RCANFD_BRP(x)                        (((x) & 0x3ff) << 0)
> +#define RCANFD_SJW(x)                        (((x) & 0x3) << 24)
> +#define RCANFD_TSEG1(x)                      (((x) & 0xf) << 16)
> +#define RCANFD_TSEG2(x)                      (((x) & 0x7) << 20)
> +
> +#define RCANFD_NR_BRP(x)             (((x) & 0x3ff) << 0)
> +#define RCANFD_NR_SJW(x)             (((x) & 0x1f) << 11)
> +#define RCANFD_NR_TSEG1(x)           (((x) & 0x7f) << 16)
> +#define RCANFD_NR_TSEG2(x)           (((x) & 0x1f) << 24)
> +
> +#define RCANFD_DR_BRP(x)             (((x) & 0xff) << 0)
> +#define RCANFD_DR_SJW(x)             (((x) & 0x7) << 24)
> +#define RCANFD_DR_TSEG1(x)           (((x) & 0xf) << 16)
> +#define RCANFD_DR_TSEG2(x)           (((x) & 0x7) << 20)
> +
> +/* Global Error flag bits */
> +#define RCANFD_GERFL_EEF1            BIT(17)
> +#define RCANFD_GERFL_EEF0            BIT(16)
> +#define RCANFD_GERFL_CMPOF           BIT(3)
> +#define RCANFD_GERFL_THLES           BIT(2)
> +#define RCANFD_GERFL_MES             BIT(1)
> +#define RCANFD_GERFL_DEF             BIT(0)
> +
> +#define RCANFD_GERFL_ERR(x)          ((x) & (RCANFD_GERFL_EEF1 |\
> +                                             RCANFD_GERFL_EEF0 |\
> +                                             RCANFD_GERFL_MES |\
> +                                             RCANFD_GERFL_CMPOF))
> +
> +/* Channel Error flag bits */
> +#define RCANFD_CERFL_ADEF            BIT(14)
> +#define RCANFD_CERFL_B0EF            BIT(13)
> +#define RCANFD_CERFL_B1EF            BIT(12)
> +#define RCANFD_CERFL_CEF             BIT(11)
> +#define RCANFD_CERFL_AEF             BIT(10)
> +#define RCANFD_CERFL_FEF             BIT(9)
> +#define RCANFD_CERFL_SEF             BIT(8)
> +#define RCANFD_CERFL_ALEF            BIT(7)
> +#define RCANFD_CERFL_BLEF            BIT(6)
> +#define RCANFD_CERFL_OLEF            BIT(5)
> +#define RCANFD_CERFL_BOREF           BIT(4)
> +#define RCANFD_CERFL_BOEEF           BIT(3)
> +#define RCANFD_CERFL_EPEF            BIT(2)
> +#define RCANFD_CERFL_EWEF            BIT(1)
> +#define RCANFD_CERFL_BEF             BIT(0)
> +
> +#define RCANFD_CERFL_ERR(x)          ((x) & (0x7fff)) /* above bits 14:0 */
> +
> +/* CAN FD specific registers */
> +#define RCANFD_DCFG_TDCE             BIT(9)
> +#define RCANFD_DCFG_TDCOC            BIT(8)
> +#define RCANFD_DCFG_TDCO(x)          (((x) & 0x7f) >> 16)
> +
> +#define RCANFD_DCSTS_TDCR(x)         (((x) >> 0) & 0x7f)
> +#define RCANFD_DCSTS_SOCCNT(x)               (((x) >> 24) & 0xff)
> +#define RCANFD_DCSTS_EOCCNT(x)               (((x) >> 16) & 0xff)
> +
> +#define RCANFD_DCSTS_TDCVF           BIT(7)
> +#define RCANFD_DCSTS_EOCO            BIT(8)
> +#define RCANFD_DCSTS_SOCO            BIT(9)
> +
> +/* Rx FIFO bits */
> +#define RCANFD_RFFIFO_RFIF           BIT(3)
> +#define RCANFD_RFFIFO_RFMLT          BIT(2)
> +#define RCANFD_RFFIFO_RFFLL          BIT(1)
> +#define RCANFD_RFFIFO_RFEMP          BIT(0)
> +
> +#define RCANFD_RFFIFO_RFESI          BIT(0)
> +#define RCANFD_RFFIFO_RFBRS          BIT(1)
> +#define RCANFD_RFFIFO_RFFDF          BIT(2)
> +
> +#define RCANFD_RFFIFO_RFIDE          BIT(31)
> +#define RCANFD_RFFIFO_RFRTR          BIT(30)
> +
> +#define RCANFD_RFFIFO_RFDLC(x)               (((x) >> 28) & 0xf)
> +#define RCANFD_RFFIFO_RFPTR(x)               (((x) >> 16) & 0xfff)
> +#define RCANFD_RFFIFO_RFTS(x)                (((x) >> 0) & 0xff)
> +
> +#define RCANFD_RFFIFO_RFIM           BIT(12)
> +#define RCANFD_RFFIFO_RFDC(x)                (((x) & 0x7) << 8)
> +#define RCANFD_RFFIFO_RFPLS(x)               (((x) & 0x7) << 4)
> +#define RCANFD_RFFIFO_RFIE           BIT(1)
> +#define RCANFD_RFFIFO_RFE            BIT(0)
> +
> +/* Common FIFO bits */
> +#define RCANFD_CMFIFO_TML(x)         (((x) & 0xf) << 20)
> +#define RCANFD_CMFIFO_M(x)           (((x) & 0x3) << 16)
> +#define RCANFD_CMFIFO_CFIM           BIT(12)
> +#define RCANFD_CMFIFO_DC(x)          (((x) & 0x7) << 8)
> +#define RCANFD_CMFIFO_PLS(x)         (((x) & 0x7) << 4)
> +#define RCANFD_CMFIFO_CFTXIE         BIT(2)
> +#define RCANFD_CMFIFO_CFE            BIT(0)
> +
> +#define RCANFD_CMFIFO_CFTXIF         BIT(4)
> +#define RCANFD_CMFIFO_CFMLT          BIT(2)
> +#define RCANFD_CMFIFO_CFFLL          BIT(1)
> +#define RCANFD_CMFIFO_CFEMP          BIT(0)
> +#define RCANFD_CMFIFO_CFMC(x)                (((x) >> 8) & 0xff)
> +
> +#define RCANFD_CMFIFO_CFESI          BIT(0)
> +#define RCANFD_CMFIFO_CFBRS          BIT(1)
> +#define RCANFD_CMFIFO_CFFDF          BIT(2)
> +
> +#define RCANFD_CMFIFO_CFIDE          BIT(31)
> +#define RCANFD_CMFIFO_CFRTR          BIT(30)
> +#define RCANFD_CMFIFO_CFID(x)                ((x) & 0x1fffffff)
> +
> +#define RCANFD_CMFIFO_CFDLC(x)               (((x) & 0xf) << 28)
> +#define RCANFD_CMFIFO_CFPTR(x)               (((x) & 0xfff) << 16)
> +#define RCANFD_CMFIFO_CFTS(x)                (((x) & 0xff) << 0)
> +
> +/* Global Test Config register */
> +#define RCANFD_GTSTCFG_C0CBCE                BIT(0)
> +#define RCANFD_GTSTCFG_C1CBCE                BIT(1)
> +
> +#define RCANFD_GTSTCTR_ICBCTME               BIT(0)
> +
> +/* AFL Rx rules registers */
> +#define RCANFD_AFLCFG_SETRNC0(x)     (((x) & 0xff) << 24)
> +#define RCANFD_AFLCFG_SETRNC1(x)     (((x) & 0xff) << 16)

What about something like:

#define RCANFD_AFLCFG_SETRNC(n, x)      (((x) & 0xff) << (24 - n * 8))

This will save some if()s in the code

> +#define RCANFD_AFLCFG_GETRNC0(x)     (((x) >> 24) & 0xff)
> +#define RCANFD_AFLCFG_GETRNC1(x)     (((x) >> 16) & 0xff)
> +
> +#define RCANFD_AFL_PAGENUM(entry)    ((entry) / 16)
> +
> +#define RCANFD_AFLCTR_AFLDAE         BIT(8)
> +#define RCANFD_AFLCTR_AFLPN(x)               ((x) & 0x1f)
> +#define RCANFD_CHANNEL_NUMRULES              1       /* only one rule per 
> channel */
> +#define RCANFD_AFLID_GAFLLB          BIT(29)
> +#define RCANFD_AFLPTR1_RFFIFO(x)     (1 << (x))
> +
> +/* Common register map offsets */
> +
> +/* Nominal rate registers */
> +#define RCANFD_CCFG(m)                       (0x0000 + (0x10 * (m)))
> +#define RCANFD_CCTR(m)                       (0x0004 + (0x10 * (m)))
> +#define RCANFD_CSTS(m)                       (0x0008 + (0x10 * (m)))
> +#define RCANFD_CERFL(m)                      (0x000C + (0x10 * (m)))
> +
> +/* Global registers */
> +#define RCANFD_GCFG                  (0x0084)
> +#define RCANFD_GCTR                  (0x0088)
> +#define RCANFD_GSTS                  (0x008c)
> +#define RCANFD_GERFL                 (0x0090)
> +#define RCANFD_GTSC                  (0x0094)
> +#define RCANFD_GAFLECTR                      (0x0098)
> +#define RCANFD_GAFLCFG0                      (0x009c)
> +#define RCANFD_GAFLCFG1                      (0x00a0)
> +#define RCANFD_RMNB                  (0x00a4)
> +
> +#define RCANFD_RMND(y)                       (0x00a8 + (0x04 * (y)))
> +
> +/* Rx FIFO Control registers */
> +#define RCANFD_RFCC(x)                       (0x00b8 + (0x04 * (x)))
> +#define RCANFD_RFSTS(x)                      (0x00d8 + (0x04 * (x)))
> +#define RCANFD_RFPCTR(x)             (0x00f8 + (0x04 * (x)))
> +
> +/* Common FIFO Control registers */
> +#define RCANFD_CFCC(ch, idx)         (0x0118 + (0x0c * (ch)) + \
> +                                      (0x04 * (idx)))
> +#define RCANFD_CFSTS(ch, idx)                (0x0178 + (0x0c * (ch)) + \
> +                                      (0x04 * (idx)))
> +#define RCANFD_CFPCTR(ch, idx)               (0x01d8 + (0x0c * (ch)) + \
> +                                      (0x04 * (idx)))
> +
> +#define RCANFD_FESTS                 (0x0238)
> +#define RCANFD_FFSTS                 (0x023c)
> +#define RCANFD_FMSTS                 (0x0240)
> +#define RCANFD_RFISTS                        (0x0244)
> +#define RCANFD_CFRISTS                       (0x0248)
> +#define RCANFD_CFTISTS                       (0x024c)
> +
> +#define RCANFD_TMC(p)                        (0x0250 + (0x01 * (p)))
> +#define RCANFD_TMSTS(p)                      (0x02d0 + (0x01 * (p)))
> +
> +#define RCANFD_TMTRSTS(y)            (0x0350 + (0x04 * (y)))
> +#define RCANFD_TMTARSTS(y)           (0x0360 + (0x04 * (y)))
> +#define RCANFD_TMTCSTS(y)            (0x0370 + (0x04 * (y)))
> +#define RCANFD_TMTASTS(y)            (0x0380 + (0x04 * (y)))
> +#define RCANFD_TMIEC(y)                      (0x0390 + (0x04 * (y)))
> +
> +#define RCANFD_TXQCC(m)                      (0x03a0 + (0x04 * (m)))
> +#define RCANFD_TXQSTS(m)             (0x03c0 + (0x04 * (m)))
> +#define RCANFD_TXQPCTR(m)            (0x03e0 + (0x04 * (m)))
> +
> +#define RCANFD_THLCC(m)                      (0x0400 + (0x04 * (m)))
> +#define RCANFD_THLSTS(m)             (0x0420 + (0x04 * (m)))
> +#define RCANFD_THLPCTR(m)            (0x0440 + (0x04 * (m)))
> +
> +#define RCANFD_GTINTSTS0             (0x0460)
> +#define RCANFD_GTINTSTS1             (0x0464)
> +#define RCANFD_GTSTCFG                       (0x0468)
> +#define RCANFD_GTSTCTR                       (0x046c)
> +#define RCANFD_GLOCKK                        (0x047c)
> +#define RCANFD_GRMCFG                        (0x04fc)
> +
> +/* Receive rule registers */
> +#define RCANFD_GAFLID(offset, j)     ((offset) + (0x10 * (j)))
> +#define RCANFD_GAFLM(offset, j)              ((offset) + 0x04 + (0x10 * (j)))
> +#define RCANFD_GAFLP0(offset, j)     ((offset) + 0x08 + (0x10 * (j)))
> +#define RCANFD_GAFLP1(offset, j)     ((offset) + 0x0c + (0x10 * (j)))
> +
> +/* CAN FD mode specific regsiter map */
> +
> +/* Data bitrate registers */
> +#define RCANFD_F_CDFG(m)             (0x0500 + (0x20 * (m)))
> +#define RCANFD_F_CFDCFG(m)           (0x0504 + (0x20 * (m)))
> +#define RCANFD_F_CFDCTR(m)           (0x0508 + (0x20 * (m)))
> +#define RCANFD_F_CFDSTS(m)           (0x050c + (0x20 * (m)))
> +#define RCANFD_F_CFDCRC(m)           (0x0510 + (0x20 * (m)))
> +
> +#define RCANFD_F_GAFL_OFFSET         (0x1000)
> +
> +#define RCANFD_F_RMID(q)             (0x2000 + (0x10 * (q)))
> +#define RCANFD_F_RMPTR(q)            (0x2004 + (0x10 * (q)))
> +#define RCANFD_F_RMFDSTS(q)          (0x2008 + (0x10 * (q)))
> +#define RCANFD_F_RMDF(q, b)          (0x200c + (0x04 * (b)) + (0x20 * (q)))
> +
> +/* Rx FIFO data registers */
> +#define RCANFD_F_RFOFFSET            (0x3000)
> +#define RCANFD_F_RFID(x)             (RCANFD_F_RFOFFSET + (0x80 * (x)))
> +#define RCANFD_F_RFPTR(x)            (RCANFD_F_RFOFFSET + 0x04 + \
> +                                      (0x80 * (x)))
> +#define RCANFD_F_RFFDSTS(x)          (RCANFD_F_RFOFFSET + 0x08 + \
> +                                      (0x80 * (x)))
> +#define RCANFD_F_RFDF(x, df)         (RCANFD_F_RFOFFSET + 0x0c + \
> +                                      (0x80 * (x)) + (0x04 * (df)))
> +
> +/* Common FIFO data registers */
> +#define RCANFD_F_CFOFFSET            (0x3400)
> +#define RCANFD_F_CFID(ch, idx)               (RCANFD_F_CFOFFSET + (0x180 * 
> (ch)) + \
> +                                      (0x80 * (idx)))
> +#define RCANFD_F_CFPTR(ch, idx)              (RCANFD_F_CFOFFSET + 0x04 + \
> +                                      (0x180 * (ch)) + (0x80 * (idx)))
> +#define RCANFD_F_CFFDCSTS(ch, idx)   (RCANFD_F_CFOFFSET + 0x08 + \
> +                                      (0x180 * (ch)) + (0x80 * (idx)))
> +#define RCANFD_F_CFDF(ch, idx, df)   (RCANFD_F_CFOFFSET + 0x0c + \
> +                                      (0x180 * (ch)) + (0x80 * (idx)) + \
> +                                      (0x04 * (df)))
> +
> +#define RCANFD_F_TMID(p)             (0x4000 + (0x20 * (p)))
> +#define RCANFD_F_TMPTR(p)            (0x4004 + (0x20 * (p)))
> +#define RCANFD_F_TMFDCTR(p)          (0x4008 + (0x20 * (p)))
> +#define RCANFD_F_TMDF(p, b)          (0x400c + (0x20 * (p)) + (0x04 * (b)))
> +
> +#define RCANFD_F_THLACC(m)           (0x6000 + (0x04 * (m)))
> +#define RCANFD_F_RPGACC(r)           (0x6400 + (0x04 * (r)))
> +
> +struct rcar_canfd_global;
> +
> +/* Channel priv data */
> +struct rcar_canfd_channel {
> +     struct can_priv can;                    /* Must be the first member */
> +     struct net_device *ndev;
> +     struct rcar_canfd_global *gpriv;        /* Controller reference */
> +     void __iomem *base;                     /* Register base address */
> +#ifdef CONFIG_DEBUG_FS
> +     struct dentry *dev_root;
> +#endif /* CONFIG_DEBUG_FS */
> +     struct napi_struct napi;
> +     u8  tx_len[RCANFD_FIFO_DEPTH];          /* For net stats */
> +     u32 tx_head;                            /* Incremented on xmit */
> +     u32 tx_tail;                            /* Incremented on xmit done */
> +     u32 channel;                            /* Channel number */
> +     spinlock_t tx_lock;                     /* To protect tx path */
> +};
> +
> +/* Global priv data */
> +struct rcar_canfd_global {
> +     struct rcar_canfd_channel *ch[RCANFD_NUM_CHANNELS];
> +     void __iomem *base;             /* Register base address */
> +     struct platform_device *pdev;   /* Respective platform device */
> +     struct clk *clkp;               /* Peripheral clock */
> +     struct clk *can_clk;            /* fCAN clock */
> +     int clock_select;               /* CANFD or Ext clock */
        ^^^
please give the corresponding enum a proper name and use it here.

> +     unsigned long channels_mask;    /* Enabled channels mask */
> +     u32 freq;                       /* fCAN clock frequency in Hz */

This value is not used outside of the probe function. You can pass the
freq to the individual channels via an argument.

> +};
> +
> +/* CAN FD mode nominal rate constants */
> +static const struct can_bittiming_const rcar_canfd_nom_bittiming_const = {
> +     .name = RCANFD_DRV_NAME,
> +     .tseg1_min = 2,
> +     .tseg1_max = 128,
> +     .tseg2_min = 2,
> +     .tseg2_max = 32,
> +     .sjw_max = 32,
> +     .brp_min = 1,
> +     .brp_max = 1024,
> +     .brp_inc = 1,
> +};
> +
> +/* CAN FD mode data rate constants */
> +static const struct can_bittiming_const rcar_canfd_data_bittiming_const = {
> +     .name = RCANFD_DRV_NAME,
> +     .tseg1_min = 2,
> +     .tseg1_max = 16,
> +     .tseg2_min = 2,
> +     .tseg2_max = 8,
> +     .sjw_max = 8,
> +     .brp_min = 1,
> +     .brp_max = 256,
> +     .brp_inc = 1,
> +};
> +
> +/* fCAN clock select register settings */
> +enum {
> +     RCANFD_CANFDCLK = 0,    /* CANFD clock */
> +     RCANFD_EXTCLK,          /* Externally input clock */
> +};
> +
> +/* Helper functions */
> +static inline void rcar_canfd_update(u32 mask, u32 val, u32 __iomem *reg)
> +{
> +     u32 data = readl(reg);
> +
> +     data &= ~mask;
> +     data |= (val & mask);
> +     writel(data, reg);
> +}
> +
> +#define rcar_canfd_read(priv, offset)                        \
> +     readl(priv->base + (offset))
> +#define rcar_canfd_write(priv, offset, val)          \
> +     writel(val, priv->base + (offset))
> +#define rcar_canfd_set_bit(priv, reg, val)           \
> +     rcar_canfd_update(val, val, priv->base + (reg))
> +#define rcar_canfd_clear_bit(priv, reg, val)         \
> +     rcar_canfd_update(val, 0, priv->base + (reg))
> +#define rcar_canfd_update_bit(priv, reg, mask, val)  \
> +     rcar_canfd_update(mask, val, priv->base + (reg))

please use static inline functions instead of defines.

> +
> +static void rcar_canfd_get_data(struct canfd_frame *cf,
> +                             struct rcar_canfd_channel *priv, u32 off)

Please use "struct rcar_canfd_channel *priv" as first argument, struct
canfd_frame *cf as second. Remove off, as the offset is already defined
by the channel.

> +{
> +     u32 i, lwords;
> +
> +     lwords = cf->len / sizeof(u32);
> +     if (cf->len % sizeof(u32))
> +             lwords++;

Use DIV_ROUND_UP() instead of open coding it.

> +     for (i = 0; i < lwords; i++)
> +             *((u32 *)cf->data + i) =
> +                     rcar_canfd_read(priv, off + (i * sizeof(u32)));
> +}
> +
> +static void rcar_canfd_put_data(struct canfd_frame *cf,
> +                             struct rcar_canfd_channel *priv, u32 off)

same here

> +{
> +     u32 i, j, lwords, leftover;
> +     u32 data = 0;
> +
> +     lwords = cf->len / sizeof(u32);
> +     leftover = cf->len % sizeof(u32);
> +     for (i = 0; i < lwords; i++)
> +             rcar_canfd_write(priv, off + (i * sizeof(u32)),
> +                              *((u32 *)cf->data + i));

Here you don't convert the endianess...
> +
> +     if (leftover) {
> +             u8 *p = (u8 *)((u32 *)cf->data + i);
> +
> +             for (j = 0; j < leftover; j++)
> +                     data |= p[j] << (j * 8);

...here you do an implicit endianess conversion. "data" is little
endian, while p[j] is big endian.

> +             rcar_canfd_write(priv, off + (i * sizeof(u32)), data);
> +     }

Have you tested to send CAN frames with len != n*4 against a different
controller?

> +}
> +
> +static void rcar_canfd_tx_failure_cleanup(struct net_device *ndev)
> +{
> +     u32 i;
> +
> +     for (i = 0; i < RCANFD_FIFO_DEPTH; i++)
> +             can_free_echo_skb(ndev, i);
> +}
> +
> +static int rcar_canfd_reset_controller(struct rcar_canfd_global *gpriv)
> +{
> +     u32 sts, ch;
> +     int err;
> +
> +     /* Check RAMINIT flag as CAN RAM initialization takes place
> +      * after the MCU reset
> +      */
> +     err = readl_poll_timeout((gpriv->base + RCANFD_GSTS), sts,
> +                              !(sts & RCANFD_GSTS_RAMINIT), 2, 500000);
> +     if (err) {
> +             dev_dbg(&gpriv->pdev->dev, "global raminit failed\n");
> +             return err;
> +     }
> +
> +     /* Transition to Global Reset mode */
> +     rcar_canfd_clear_bit(gpriv, RCANFD_GCTR, RCANFD_GCTR_SLPR);
> +     rcar_canfd_update_bit(gpriv, RCANFD_GCTR,
> +                           RCANFD_GCTR_MODEMASK, RCANFD_GCTR_GRESET);
> +
> +     /* Ensure Global reset mode */
> +     err = readl_poll_timeout((gpriv->base + RCANFD_GSTS), sts,
> +                              (sts & RCANFD_GSTS_RESET), 2, 500000);
> +     if (err) {
> +             dev_dbg(&gpriv->pdev->dev, "global reset failed\n");
> +             return err;
> +     }
> +
> +     /* Reset Global error flags */
> +     rcar_canfd_write(gpriv, RCANFD_GERFL, 0x0);
> +
> +     /* Set the controller into FD mode */
> +     rcar_canfd_set_bit(gpriv, RCANFD_GRMCFG, RCANFD_GINTF_CANFD);
> +
> +     /* Transition all Channels to reset mode */
> +     for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
> +             rcar_canfd_clear_bit(gpriv, RCANFD_CCTR(ch), RCANFD_CCTR_CSLPR);
> +
> +             rcar_canfd_update_bit(gpriv, RCANFD_CCTR(ch),
> +                                   RCANFD_CCTR_MODEMASK,
> +                                   RCANFD_CCTR_CRESET);
> +
> +             /* Ensure Channel reset mode */
> +             err = readl_poll_timeout((gpriv->base + RCANFD_CSTS(ch)), sts,
> +                                      (sts & RCANFD_CSTS_RESET), 2, 500000);
> +             if (err) {
> +                     dev_dbg(&gpriv->pdev->dev,
> +                             "channel %u reset failed\n", ch);
> +                     return err;
> +             }
> +     }
> +     return 0;
> +}
> +
> +static void rcar_canfd_configure_controller(struct rcar_canfd_global *gpriv)
> +{
> +     u32 cfg, ch;
> +
> +     /* Global Configuration settings */
> +     cfg = RCANFD_GCFG_EEFE;         /* ECC error flag enabled */
> +
> +     /* Set External Clock if selected */
> +     if (gpriv->clock_select != RCANFD_CANFDCLK)
> +             cfg |= RCANFD_GCFG_DCS;
> +
> +     /* Truncate payload to configured message size RFPLS */
> +     cfg |= RCANFD_GCFG_CMPOC;
> +
> +     rcar_canfd_set_bit(gpriv, RCANFD_GCFG, cfg);
> +
> +     /* Channel configuration settings */
> +     for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
> +             rcar_canfd_set_bit(gpriv, RCANFD_CCTR(ch), RCANFD_CCTR_ERRD);
> +             rcar_canfd_update_bit(gpriv, RCANFD_CCTR(ch),
> +                                   RCANFD_CCTR_BOMMASK,
> +                                   RCANFD_CCTR_BOM_ENTRY);
> +     }
> +}
> +
> +static void rcar_canfd_configure_afl_rules(struct rcar_canfd_global *gpriv,
> +                                        u32 ch)
> +{
> +     u32 cfg;
> +     int start, page, num_rules = RCANFD_CHANNEL_NUMRULES;
> +     u32 ridx = ch + RCANFD_RFFIFO_IDX;
> +
> +     if (ch == 0) {
> +             start = 0; /* Start from 0th rule */
> +     } else {
> +             /* Get number of existing rules and adjust */
> +             cfg = rcar_canfd_read(gpriv, RCANFD_GAFLCFG0);
> +             start = RCANFD_AFLCFG_GETRNC0(cfg);
> +     }
> +
> +     /* Enable write access to entry */
> +     page = RCANFD_AFL_PAGENUM(start);
> +     rcar_canfd_set_bit(gpriv, RCANFD_GAFLECTR,
> +                        (RCANFD_AFLCTR_AFLPN(page) | RCANFD_AFLCTR_AFLDAE));
> +
> +     /* Write number of rules for channel */
> +     if (ch == 0)
> +             rcar_canfd_set_bit(gpriv, RCANFD_GAFLCFG0,
> +                                RCANFD_AFLCFG_SETRNC0(num_rules));
> +     else
> +             rcar_canfd_set_bit(gpriv, RCANFD_GAFLCFG0,
> +                                RCANFD_AFLCFG_SETRNC1(num_rules));
> +
> +     /* Accept all IDs */
> +     rcar_canfd_write(gpriv, RCANFD_GAFLID(RCANFD_F_GAFL_OFFSET, start), 0);
> +     /* IDE or RTR is not considered for matching */
> +     rcar_canfd_write(gpriv, RCANFD_GAFLM(RCANFD_F_GAFL_OFFSET, start), 0);
> +     /* Any data length accepted */
> +     rcar_canfd_write(gpriv, RCANFD_GAFLP0(RCANFD_F_GAFL_OFFSET, start), 0);
> +     /* Place the msg in corresponding Rx FIFO entry */
> +     rcar_canfd_write(gpriv, RCANFD_GAFLP1(RCANFD_F_GAFL_OFFSET, start),
> +                      RCANFD_AFLPTR1_RFFIFO(ridx));
> +
> +     /* Disable write access to page */
> +     rcar_canfd_clear_bit(gpriv, RCANFD_GAFLECTR, RCANFD_AFLCTR_AFLDAE);
> +}
> +
> +static void rcar_canfd_configure_rx(struct rcar_canfd_global *gpriv, u32 ch)
> +{
> +     /* Rx FIFO is used for reception */
> +     u32 cfg;
> +     u16 rfdc, rfpls;
> +
> +     /* Select Rx FIFO based on channel */
> +     u32 ridx = ch + RCANFD_RFFIFO_IDX;
> +
> +     rfdc = 2;               /* b010 - 8 messages Rx FIFO depth */
> +     rfpls = 7;              /* b111 - Max 64 bytes payload */
> +
> +     cfg = (RCANFD_RFFIFO_RFIM | RCANFD_RFFIFO_RFDC(rfdc) |
> +             RCANFD_RFFIFO_RFPLS(rfpls) | RCANFD_RFFIFO_RFIE);
> +     rcar_canfd_write(gpriv, RCANFD_RFCC(ridx), cfg);
> +}
> +
> +static void rcar_canfd_configure_tx(struct rcar_canfd_global *gpriv, u32 ch)
> +{
> +     /* Tx/Rx(Common) FIFO configured in Tx mode is
> +      * used for transmission
> +      *
> +      * Each channel has 3 Common FIFO dedicated to them.
> +      * Use the 1st (index 0) out of 3
> +      */
> +     u32 cfg;
> +     u16 cftml, cfm, cfdc, cfpls;
> +
> +     cftml = 0;              /* 0th buffer */
> +     cfm = 1;                /* b01 - Transmit mode */
> +     cfdc = 2;               /* b010 - 8 messages Tx FIFO depth */
> +     cfpls = 7;              /* b111 - Max 64 bytes payload */
> +
> +     cfg = (RCANFD_CMFIFO_TML(cftml) | RCANFD_CMFIFO_M(cfm) |
> +             RCANFD_CMFIFO_CFIM | RCANFD_CMFIFO_DC(cfdc) |
> +             RCANFD_CMFIFO_PLS(cfpls) | RCANFD_CMFIFO_CFTXIE);
> +     rcar_canfd_write(gpriv, RCANFD_CFCC(ch, RCANFD_CFFIFO_IDX), cfg);
> +
> +     /* Clear FD mode specific control/status register */
> +     rcar_canfd_write(gpriv, RCANFD_F_CFFDCSTS(ch, RCANFD_CFFIFO_IDX), 0);
> +}
> +
> +static void rcar_canfd_enable_global_interrupts(struct rcar_canfd_global 
> *gpriv)
> +{
> +     u32 ctr;
> +
> +     /* Clear any stray error interrupt flags */
> +     rcar_canfd_write(gpriv, RCANFD_GERFL, 0);
> +
> +     /* Global interrupts setup */
> +     ctr = RCANFD_GCTR_MEIE;
> +     ctr |= RCANFD_GCTR_CFMPOFIE;
> +
> +     rcar_canfd_set_bit(gpriv, RCANFD_GCTR, ctr);
> +}
> +
> +static void rcar_canfd_disable_global_interrupts(struct rcar_canfd_global
> +                                              *gpriv)
> +{
> +     /* Disable all interrupts */
> +     rcar_canfd_write(gpriv, RCANFD_GCTR, 0);
> +
> +     /* Clear any stray error interrupt flags */
> +     rcar_canfd_write(gpriv, RCANFD_GERFL, 0);
> +}
> +
> +static void rcar_canfd_enable_channel_interrupts(struct rcar_canfd_channel
> +                                              *priv)
> +{
> +     u32 ctr, ch = priv->channel;
> +
> +     /* Clear any stray error flags */
> +     rcar_canfd_write(priv, RCANFD_CERFL(ch), 0);
> +
> +     /* Channel interrupts setup */
> +     ctr = (RCANFD_CCTR_TAIE |
> +            RCANFD_CCTR_ALIE | RCANFD_CCTR_BLIE |
> +            RCANFD_CCTR_OLIE | RCANFD_CCTR_BORIE |
> +            RCANFD_CCTR_BOEIE | RCANFD_CCTR_EPIE |
> +            RCANFD_CCTR_EWIE | RCANFD_CCTR_BEIE);
> +     rcar_canfd_set_bit(priv, RCANFD_CCTR(ch), ctr);
> +}
> +
> +static void rcar_canfd_disable_channel_interrupts(struct rcar_canfd_channel
> +                                               *priv)
> +{
> +     u32 ctr, ch = priv->channel;
> +
> +     ctr = (RCANFD_CCTR_TAIE |
> +            RCANFD_CCTR_ALIE | RCANFD_CCTR_BLIE |
> +            RCANFD_CCTR_OLIE | RCANFD_CCTR_BORIE |
> +            RCANFD_CCTR_BOEIE | RCANFD_CCTR_EPIE |
> +            RCANFD_CCTR_EWIE | RCANFD_CCTR_BEIE);
> +     rcar_canfd_clear_bit(priv, RCANFD_CCTR(ch), ctr);
> +
> +     /* Clear any stray error flags */
> +     rcar_canfd_write(priv, RCANFD_CERFL(ch), 0);
> +}
> +
> +static void rcar_canfd_global_error(struct net_device *ndev)
> +{
> +     struct rcar_canfd_channel *priv = netdev_priv(ndev);
> +     struct net_device_stats *stats = &ndev->stats;
> +     u32 ch = priv->channel;
> +     u32 gerfl, sts;
> +     u32 ridx = ch + RCANFD_RFFIFO_IDX;
> +
> +     gerfl = rcar_canfd_read(priv, RCANFD_GERFL);
> +     if ((gerfl & RCANFD_GERFL_EEF0) && (ch == 0)) {
> +             netdev_dbg(ndev, "Ch0: ECC Error flag\n");
> +             stats->tx_dropped++;
> +     }
> +     if ((gerfl & RCANFD_GERFL_EEF1) && (ch == 1)) {
> +             netdev_dbg(ndev, "Ch1: ECC Error flag\n");
> +             stats->tx_dropped++;
> +     }
> +     if (gerfl & RCANFD_GERFL_MES) {
> +             sts = rcar_canfd_read(priv,
> +                                   RCANFD_CFSTS(ch, RCANFD_CFFIFO_IDX));
> +             if (sts & RCANFD_CMFIFO_CFMLT) {
> +                     netdev_dbg(ndev, "Tx Message Lost flag\n");
> +                     stats->tx_dropped++;
> +                     rcar_canfd_write(priv,
> +                                      RCANFD_CFSTS(ch, RCANFD_CFFIFO_IDX),
> +                                      sts & ~RCANFD_CMFIFO_CFMLT);
> +             }
> +
> +             sts = rcar_canfd_read(priv, RCANFD_RFSTS(ridx));
> +             if (sts & RCANFD_RFFIFO_RFMLT) {
> +                     netdev_dbg(ndev, "Rx Message Lost flag\n");
> +                     stats->rx_dropped++;
> +                     rcar_canfd_write(priv, RCANFD_RFSTS(ridx),
> +                                      sts & ~RCANFD_RFFIFO_RFMLT);
> +             }
> +     }
> +     if (gerfl & RCANFD_GERFL_CMPOF) {
> +             /* Message Lost flag will be set for respective channel
> +              * when this condition happens with counters and flags
> +              * already updated.
> +              */
> +             netdev_dbg(ndev, "global payload overflow interrupt\n");
> +     }
> +
> +     /* Clear all global error interrupts. Only affected channels bits
> +      * get cleared
> +      */
> +     rcar_canfd_write(priv, RCANFD_GERFL, 0);
> +}
> +
> +static void rcar_canfd_error(struct net_device *ndev)
> +{
> +     struct rcar_canfd_channel *priv = netdev_priv(ndev);
> +     struct net_device_stats *stats = &ndev->stats;
> +     struct can_frame *cf;
> +     struct sk_buff *skb;
> +     u32 cerfl, csts;
> +     u32 txerr = 0, rxerr = 0;
> +     u32 ch = priv->channel;
> +
> +     /* Propagate the error condition to the CAN stack */
> +     skb = alloc_can_err_skb(ndev, &cf);
> +     if (!skb) {
> +             stats->rx_dropped++;
> +             return;
> +     }
> +
> +     /* Channel error interrupt */
> +     cerfl = rcar_canfd_read(priv, RCANFD_CERFL(ch));
> +     csts = rcar_canfd_read(priv, RCANFD_CSTS(ch));
> +     txerr = RCANFD_CSTS_TECCNT(csts);
> +     rxerr = RCANFD_CSTS_RECCNT(csts);
> +
> +     netdev_dbg(ndev, "ch erfl %x sts %x txerr %u rxerr %u\n",
> +                cerfl, csts, txerr, rxerr);
> +
> +     if (cerfl & RCANFD_CERFL_BEF) {
> +             netdev_dbg(ndev, "Bus error\n");
> +             cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
> +             cf->data[2] = CAN_ERR_PROT_UNSPEC;
> +             priv->can.can_stats.bus_error++;
> +     }
> +     if (cerfl & RCANFD_CERFL_ADEF) {
> +             netdev_dbg(ndev, "ACK Delimiter Error\n");
> +             stats->tx_errors++;
> +             cf->data[3] |= CAN_ERR_PROT_LOC_ACK_DEL;
> +     }
> +     if (cerfl & RCANFD_CERFL_B0EF) {
> +             netdev_dbg(ndev, "Bit Error (dominant)\n");
> +             stats->tx_errors++;
> +             cf->data[2] |= CAN_ERR_PROT_BIT0;
> +     }
> +     if (cerfl & RCANFD_CERFL_B1EF) {
> +             netdev_dbg(ndev, "Bit Error (recessive)\n");
> +             stats->tx_errors++;
> +             cf->data[2] |= CAN_ERR_PROT_BIT1;
> +     }
> +     if (cerfl & RCANFD_CERFL_CEF) {
> +             netdev_dbg(ndev, "CRC Error\n");
> +             stats->rx_errors++;
> +             cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
> +     }
> +     if (cerfl & RCANFD_CERFL_AEF) {
> +             netdev_dbg(ndev, "ACK Error\n");
> +             stats->tx_errors++;
> +             cf->can_id |= CAN_ERR_ACK;
> +             cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
> +     }
> +     if (cerfl & RCANFD_CERFL_FEF) {
> +             netdev_dbg(ndev, "Form Error\n");
> +             stats->rx_errors++;
> +             cf->data[2] |= CAN_ERR_PROT_FORM;
> +     }
> +     if (cerfl & RCANFD_CERFL_SEF) {
> +             netdev_dbg(ndev, "Stuff Error\n");
> +             stats->rx_errors++;
> +             cf->data[2] |= CAN_ERR_PROT_STUFF;
> +     }
> +     if (cerfl & RCANFD_CERFL_ALEF) {
> +             netdev_dbg(ndev, "Arbitration lost Error\n");
> +             priv->can.can_stats.arbitration_lost++;
> +             cf->can_id |= CAN_ERR_LOSTARB;
> +             cf->data[0] |= CAN_ERR_LOSTARB_UNSPEC;
> +     }
> +     if (cerfl & RCANFD_CERFL_BLEF) {
> +             netdev_dbg(ndev, "Bus Lock Error\n");
> +             stats->rx_errors++;
> +             cf->can_id |= CAN_ERR_BUSERROR;
> +     }
> +     if (cerfl & RCANFD_CERFL_EWEF) {
> +             netdev_dbg(ndev, "Error warning interrupt\n");
> +             priv->can.state = CAN_STATE_ERROR_WARNING;
> +             priv->can.can_stats.error_warning++;
> +             cf->can_id |= CAN_ERR_CRTL;
> +             cf->data[1] = txerr > rxerr ? CAN_ERR_CRTL_TX_WARNING :
> +                     CAN_ERR_CRTL_RX_WARNING;
> +             cf->data[6] = txerr;
> +             cf->data[7] = rxerr;
> +     }
> +     if (cerfl & RCANFD_CERFL_EPEF) {
> +             netdev_dbg(ndev, "Error passive interrupt\n");
> +             priv->can.state = CAN_STATE_ERROR_PASSIVE;
> +             priv->can.can_stats.error_passive++;
> +             cf->can_id |= CAN_ERR_CRTL;
> +             cf->data[1] = txerr > rxerr ? CAN_ERR_CRTL_TX_PASSIVE :
> +                     CAN_ERR_CRTL_RX_PASSIVE;
> +             cf->data[6] = txerr;
> +             cf->data[7] = rxerr;
> +     }
> +     if (cerfl & RCANFD_CERFL_BOEEF) {
> +             netdev_dbg(ndev, "Bus-off entry interrupt\n");
> +             rcar_canfd_tx_failure_cleanup(ndev);
> +             priv->can.state = CAN_STATE_BUS_OFF;
> +             priv->can.can_stats.bus_off++;
> +             can_bus_off(ndev);
> +             cf->can_id |= CAN_ERR_BUSOFF;
> +     }
> +     if (cerfl & RCANFD_CERFL_OLEF) {
> +             netdev_dbg(ndev,
> +                        "Overload Frame Transmission error interrupt\n");
> +             stats->tx_errors++;
> +             cf->can_id |= CAN_ERR_PROT;
> +             cf->data[2] |= CAN_ERR_PROT_OVERLOAD;
> +     }
> +
> +     /* Clear all channel error interrupts */
> +     rcar_canfd_write(priv, RCANFD_CERFL(ch), 0);
> +     stats->rx_packets++;
> +     stats->rx_bytes += cf->can_dlc;
> +     netif_rx(skb);
> +}
> +
> +static void rcar_canfd_tx_done(struct net_device *ndev)
> +{
> +     struct rcar_canfd_channel *priv = netdev_priv(ndev);
> +     struct net_device_stats *stats = &ndev->stats;
> +     u32 sts;
> +     unsigned long flags;
> +     u32 ch = priv->channel;
> +
> +     do {

You should iterare over all pending CAN frames:

>       for (/* nix */; (priv->tx_head - priv->tx_tail) > 0; priv->tx_tail++) {


> +             u8 unsent, sent;
> +
> +             sent = priv->tx_tail % RCANFD_FIFO_DEPTH;

and check here, if that packet has really been tramsitted. Exit the loop
otherweise.

> +             stats->tx_packets++;
> +             stats->tx_bytes += priv->tx_len[sent];
> +             priv->tx_len[sent] = 0;
> +             can_get_echo_skb(ndev, sent);
> +
> +             spin_lock_irqsave(&priv->tx_lock, flags);

What does the tx_lock protect? The tx path is per channel, isn't it?

> +             priv->tx_tail++;
> +             sts = rcar_canfd_read(priv,
> +                                   RCANFD_CFSTS(ch, RCANFD_CFFIFO_IDX));
> +             unsent = RCANFD_CMFIFO_CFMC(sts);
> +
> +             /* Wake producer only when there is room */
> +             if (unsent != RCANFD_FIFO_DEPTH)
> +                     netif_wake_queue(ndev);

Move the netif_wake_queue() out of the loop.

> +
> +             if (priv->tx_head - priv->tx_tail <= unsent) {
> +                     spin_unlock_irqrestore(&priv->tx_lock, flags);
> +                     break;
> +             }
> +             spin_unlock_irqrestore(&priv->tx_lock, flags);
> +
> +     } while (1);
> +
> +     /* Clear interrupt */
> +     rcar_canfd_write(priv, RCANFD_CFSTS(ch, RCANFD_CFFIFO_IDX),
> +                      sts & ~RCANFD_CMFIFO_CFTXIF);
> +     can_led_event(ndev, CAN_LED_EVENT_TX);
> +}
> +
> +static irqreturn_t rcar_canfd_global_interrupt(int irq, void *dev_id)
> +{
> +     struct rcar_canfd_global *gpriv = dev_id;
> +     struct net_device *ndev;
> +     struct rcar_canfd_channel *priv;
> +     u32 sts, gerfl;
> +     u32 ch, ridx;
> +
> +     /* Global error interrupts still indicate a condition specific
> +      * to a channel. RxFIFO interrupt is a global interrupt.
> +      */
> +     for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
> +             priv = gpriv->ch[ch];
> +             ndev = priv->ndev;
> +             ridx = ch + RCANFD_RFFIFO_IDX;
> +
> +             /* Global error interrupts */
> +             gerfl = rcar_canfd_read(priv, RCANFD_GERFL);
> +             if (RCANFD_GERFL_ERR(gerfl))
> +                     rcar_canfd_global_error(ndev);
> +
> +             /* Handle Rx interrupts */
> +             sts = rcar_canfd_read(priv, RCANFD_RFSTS(ridx));
> +             if (sts & RCANFD_RFFIFO_RFIF) {
> +                     if (napi_schedule_prep(&priv->napi)) {
> +                             /* Disable Rx FIFO interrupts */
> +                             rcar_canfd_clear_bit(priv,
> +                                                  RCANFD_RFCC(ridx),
> +                                                  RCANFD_RFFIFO_RFIE);
> +                             __napi_schedule(&priv->napi);
> +                     }
> +             }
> +     }
> +     return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t rcar_canfd_channel_interrupt(int irq, void *dev_id)
> +{
> +     struct rcar_canfd_global *gpriv = dev_id;
> +     struct net_device *ndev;
> +     struct rcar_canfd_channel *priv;
> +     u32 sts, cerfl, ch;
> +
> +     /* Common FIFO is a per channel resource */
> +     for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
> +             priv = gpriv->ch[ch];
> +             ndev = priv->ndev;
> +
> +             /* Channel error interrupts */
> +             cerfl = rcar_canfd_read(priv, RCANFD_CERFL(ch));
> +             if (RCANFD_CERFL_ERR(cerfl))
> +                     rcar_canfd_error(ndev);
> +
> +             /* Handle Tx interrupts */
> +             sts = rcar_canfd_read(priv,
> +                                   RCANFD_CFSTS(ch, RCANFD_CFFIFO_IDX));
> +             if (sts & RCANFD_CMFIFO_CFTXIF)
> +                     rcar_canfd_tx_done(ndev);
> +     }
> +     return IRQ_HANDLED;
> +}
> +
> +static void rcar_canfd_set_bittiming(struct net_device *dev)
> +{
> +     struct rcar_canfd_channel *priv = netdev_priv(dev);
> +     const struct can_bittiming *bt = &priv->can.bittiming;
> +     const struct can_bittiming *dbt = &priv->can.data_bittiming;
> +     u16 brp, sjw, tseg1, tseg2;
> +     u32 cfg;
> +     u32 ch = priv->channel;
> +
> +     /* Nominal bit timing settings */
> +     brp = bt->brp - 1;
> +     sjw = bt->sjw - 1;
> +     tseg1 = bt->prop_seg + bt->phase_seg1 - 1;
> +     tseg2 = bt->phase_seg2 - 1;
> +
> +     cfg = (RCANFD_NR_TSEG1(tseg1) | RCANFD_NR_BRP(brp) |
> +            RCANFD_NR_SJW(sjw) | RCANFD_NR_TSEG2(tseg2));
> +
> +     rcar_canfd_write(priv, RCANFD_CCFG(ch), cfg);
> +     netdev_dbg(priv->ndev, "nrate: brp %u, sjw %u, tseg1 %u, tseg2 %u\n",
> +                brp, sjw, tseg1, tseg2);
> +
> +     /* Data bit timing settings */
> +     brp = dbt->brp - 1;
> +     sjw = dbt->sjw - 1;
> +     tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
> +     tseg2 = dbt->phase_seg2 - 1;
> +
> +     cfg = (RCANFD_DR_TSEG1(tseg1) | RCANFD_DR_BRP(brp) |
> +            RCANFD_DR_SJW(sjw) | RCANFD_DR_TSEG2(tseg2));
> +
> +     rcar_canfd_write(priv, RCANFD_F_CDFG(ch), cfg);
> +     netdev_dbg(priv->ndev, "drate: brp %u, sjw %u, tseg1 %u, tseg2 %u\n",
> +                brp, sjw, tseg1, tseg2);
> +}
> +
> +static int rcar_canfd_start(struct net_device *ndev)
> +{
> +     struct rcar_canfd_channel *priv = netdev_priv(ndev);
> +     int err = -EOPNOTSUPP;
> +     u32 sts, ch = priv->channel;
> +     u32 ridx = ch + RCANFD_RFFIFO_IDX;
> +
> +     /* Ensure channel starts in FD mode */
> +     if (!(priv->can.ctrlmode & CAN_CTRLMODE_FD)) {
> +             netdev_err(ndev, "enable can fd mode for channel %d\n", ch);
> +             goto fail_mode;
> +     }
> +
> +     rcar_canfd_set_bittiming(ndev);
> +
> +     rcar_canfd_enable_channel_interrupts(priv);
> +
> +     /* Set channel to Operational mode */
> +     rcar_canfd_update_bit(priv, RCANFD_CCTR(ch),
> +                           RCANFD_CCTR_MODEMASK, RCANFD_CCTR_COPM);
> +
> +     /* Verify channel mode change */
> +     err = readl_poll_timeout((priv->base + RCANFD_CSTS(ch)), sts,
> +                              (sts & RCANFD_CSTS_COM), 2, 500000);
> +     if (err) {
> +             netdev_err(ndev, "channel %u communication state failed\n", ch);
> +             goto fail_mode_change;
> +     }
> +
> +     /* Enable Common & Rx FIFO */
> +     rcar_canfd_set_bit(priv, RCANFD_CFCC(ch, RCANFD_CFFIFO_IDX),
> +                        RCANFD_CMFIFO_CFE);
> +     rcar_canfd_set_bit(priv, RCANFD_RFCC(ridx), RCANFD_RFFIFO_RFE);
> +
> +     priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +     return 0;
> +
> +fail_mode_change:
> +     rcar_canfd_disable_channel_interrupts(priv);
> +fail_mode:
> +     return err;
> +}
> +
> +static int rcar_canfd_open(struct net_device *ndev)
> +{
> +     struct rcar_canfd_channel *priv = netdev_priv(ndev);
> +     struct rcar_canfd_global *gpriv = priv->gpriv;
> +     int err;
> +
> +     /* Peripheral clock is already enabled in probe */
> +     err = clk_prepare_enable(gpriv->can_clk);
> +     if (err) {
> +             netdev_err(ndev, "failed to enable CAN clock, error %d\n", err);
> +             goto out_clock;
> +     }
> +
> +     err = open_candev(ndev);
> +     if (err) {
> +             netdev_err(ndev, "open_candev() failed, error %d\n", err);
> +             goto out_can_clock;
> +     }
> +
> +     napi_enable(&priv->napi);
> +     err = rcar_canfd_start(ndev);
> +     if (err)
> +             goto out_close;
> +     netif_start_queue(ndev);
> +     can_led_event(ndev, CAN_LED_EVENT_OPEN);
> +     return 0;
> +out_close:
> +     napi_disable(&priv->napi);
> +     close_candev(ndev);
> +out_can_clock:
> +     clk_disable_unprepare(gpriv->can_clk);
> +out_clock:
> +     return err;
> +}
> +
> +static void rcar_canfd_stop(struct net_device *ndev)
> +{
> +     struct rcar_canfd_channel *priv = netdev_priv(ndev);
> +     int err;
> +     u32 sts, ch = priv->channel;
> +     u32 ridx = ch + RCANFD_RFFIFO_IDX;
> +
> +     /* Transition to channel reset mode  */
> +     rcar_canfd_update_bit(priv, RCANFD_CCTR(ch),
> +                           RCANFD_CCTR_MODEMASK, RCANFD_CCTR_CRESET);
> +
> +     /* Check Channel reset mode */
> +     err = readl_poll_timeout((priv->base + RCANFD_CSTS(ch)), sts,
> +                              (sts & RCANFD_CSTS_RESET), 2, 500000);
> +     if (err)
> +             netdev_err(ndev, "channel %u reset failed\n", ch);
> +
> +     rcar_canfd_disable_channel_interrupts(priv);
> +
> +     /* Disable Common & Rx FIFO */
> +     rcar_canfd_clear_bit(priv, RCANFD_CFCC(ch, RCANFD_CFFIFO_IDX),
> +                          RCANFD_CMFIFO_CFE);
> +     rcar_canfd_clear_bit(priv, RCANFD_RFCC(ridx), RCANFD_RFFIFO_RFE);
> +
> +     /* Set the state as STOPPED */
> +     priv->can.state = CAN_STATE_STOPPED;
> +}
> +
> +static int rcar_canfd_close(struct net_device *ndev)
> +{
> +     struct rcar_canfd_channel *priv = netdev_priv(ndev);
> +     struct rcar_canfd_global *gpriv = priv->gpriv;
> +
> +     netif_stop_queue(ndev);
> +     rcar_canfd_stop(ndev);
> +     napi_disable(&priv->napi);
> +     clk_disable_unprepare(gpriv->can_clk);
> +     close_candev(ndev);
> +     can_led_event(ndev, CAN_LED_EVENT_STOP);
> +     return 0;
> +}
> +
> +static netdev_tx_t rcar_canfd_start_xmit(struct sk_buff *skb,
> +                                      struct net_device *ndev)
> +{
> +     struct rcar_canfd_channel *priv = netdev_priv(ndev);
> +     struct canfd_frame *cf = (struct canfd_frame *)skb->data;
> +     u32 sts, id, ptr;
> +     unsigned long flags;
> +     u32 ch = priv->channel;
> +
> +     if (can_dropped_invalid_skb(ndev, skb))
> +             return NETDEV_TX_OK;
> +
> +     if (cf->can_id & CAN_EFF_FLAG) {
> +             id = cf->can_id & CAN_EFF_MASK;
> +             id |= RCANFD_CMFIFO_CFIDE;
> +     } else {
> +             id = cf->can_id & CAN_SFF_MASK;
> +     }
> +
> +     if (cf->can_id & CAN_RTR_FLAG)
> +             id |= RCANFD_CMFIFO_CFRTR;
> +
> +     rcar_canfd_write(priv, RCANFD_F_CFID(ch, RCANFD_CFFIFO_IDX),
> +                      id);
> +     ptr = RCANFD_CMFIFO_CFDLC(can_len2dlc(cf->len));

ptr usually means pointer, better call it dlc.

> +     rcar_canfd_write(priv, RCANFD_F_CFPTR(ch, RCANFD_CFFIFO_IDX),
> +                      ptr);
> +
> +     sts = rcar_canfd_read(priv, RCANFD_F_CFFDCSTS(ch, RCANFD_CFFIFO_IDX));
> +     if (can_is_canfd_skb(skb)) {
> +             /* CAN FD frame format */
> +             sts |= RCANFD_CMFIFO_CFFDF;
> +             if (cf->flags & CANFD_BRS)
> +                     sts |= RCANFD_CMFIFO_CFBRS;
> +             else
> +                     sts &= ~RCANFD_CMFIFO_CFBRS;
> +     } else {
> +             sts &= ~RCANFD_CMFIFO_CFFDF;
> +     }
> +     rcar_canfd_write(priv, RCANFD_F_CFFDCSTS(ch, RCANFD_CFFIFO_IDX), sts);
> +
> +     rcar_canfd_put_data(cf, priv,
> +                         RCANFD_F_CFDF(ch, RCANFD_CFFIFO_IDX, 0));
> +
> +     priv->tx_len[priv->tx_head % RCANFD_FIFO_DEPTH] = cf->len;
> +     can_put_echo_skb(skb, ndev, priv->tx_head % RCANFD_FIFO_DEPTH);
> +
> +     spin_lock_irqsave(&priv->tx_lock, flags);
> +     priv->tx_head++;
> +
> +     /* Start Tx: Write 0xff to CFPC to increment the CPU-side
> +      * pointer for the Common FIFO
> +      */
> +     rcar_canfd_write(priv, RCANFD_CFPCTR(ch, RCANFD_CFFIFO_IDX), 0xff);
> +
> +     /* Stop the queue if we've filled all FIFO entries */
> +     if (priv->tx_head - priv->tx_tail >= RCANFD_FIFO_DEPTH)
> +             netif_stop_queue(ndev);

Please move the check of stop_queue, before starting the send.

> +
> +     spin_unlock_irqrestore(&priv->tx_lock, flags);
> +     return NETDEV_TX_OK;
> +}
> +
> +static void rcar_canfd_rx_pkt(struct rcar_canfd_channel *priv)
> +{
> +     struct net_device_stats *stats = &priv->ndev->stats;
> +     struct canfd_frame *cf;
> +     struct sk_buff *skb;
> +     u32 sts = 0, id, ptr;
> +     u32 ch = priv->channel;
> +     u32 ridx = ch + RCANFD_RFFIFO_IDX;
> +
> +     id = rcar_canfd_read(priv, RCANFD_F_RFID(ridx));
> +     ptr = rcar_canfd_read(priv, RCANFD_F_RFPTR(ridx));
> +
> +     sts = rcar_canfd_read(priv, RCANFD_F_RFFDSTS(ridx));
> +     if (sts & RCANFD_RFFIFO_RFFDF)
> +             skb = alloc_canfd_skb(priv->ndev, &cf);
> +     else
> +             skb = alloc_can_skb(priv->ndev,
> +                                 (struct can_frame **)&cf);
> +
> +     if (!skb) {
> +             stats->rx_dropped++;
> +             return;
> +     }
> +
> +     if (sts & RCANFD_RFFIFO_RFFDF)
> +             cf->len = can_dlc2len(RCANFD_RFFIFO_RFDLC(ptr));
> +     else
> +             cf->len = get_can_dlc(RCANFD_RFFIFO_RFDLC(ptr));
> +
> +     if (sts & RCANFD_RFFIFO_RFESI) {
> +             cf->flags |= CANFD_ESI;
> +             netdev_dbg(priv->ndev, "ESI Error\n");
> +     }
> +
> +     if (id & RCANFD_RFFIFO_RFIDE)
> +             cf->can_id = (id & CAN_EFF_MASK) | CAN_EFF_FLAG;
> +     else
> +             cf->can_id = id & CAN_SFF_MASK;
> +
> +     if (!(sts & RCANFD_RFFIFO_RFFDF) && (id & RCANFD_RFFIFO_RFRTR)) {
> +             cf->can_id |= CAN_RTR_FLAG;
> +     } else {
> +             if (sts & RCANFD_RFFIFO_RFBRS)
> +                     cf->flags |= CANFD_BRS;
> +
> +             rcar_canfd_get_data(cf, priv, RCANFD_F_RFDF(ridx, 0));
> +     }
> +
> +     /* Write 0xff to RFPC to increment the CPU-side
> +      * pointer of the Rx FIFO
> +      */
> +     rcar_canfd_write(priv, RCANFD_RFPCTR(ridx), 0xff);
> +
> +     can_led_event(priv->ndev, CAN_LED_EVENT_RX);
> +
> +     stats->rx_bytes += cf->len;
> +     stats->rx_packets++;
> +     netif_receive_skb(skb);
> +}
> +
> +static int rcar_canfd_rx_poll(struct napi_struct *napi, int quota)
> +{
> +     struct rcar_canfd_channel *priv =
> +             container_of(napi, struct rcar_canfd_channel, napi);
> +     int num_pkts;
> +     u32 sts;
> +     u32 ch = priv->channel;
> +     u32 ridx = ch + RCANFD_RFFIFO_IDX;
> +
> +     for (num_pkts = 0; num_pkts < quota; num_pkts++) {
> +             sts = rcar_canfd_read(priv, RCANFD_RFSTS(ridx));
> +             /* Clear interrupt bit */
> +             if (sts & RCANFD_RFFIFO_RFIF)
> +                     rcar_canfd_write(priv, RCANFD_RFSTS(ridx),
> +                                      sts & ~RCANFD_RFFIFO_RFIF);
> +
> +             /* Check FIFO empty condition */
> +             if (sts & RCANFD_RFFIFO_RFEMP)
> +                     break;
> +
> +             rcar_canfd_rx_pkt(priv);

This sequence looks strange. You first conditionally ack the interrupt
then you check for empty fifo, then read the CAN frame. I would assume
that you first check if there's a CAN frame, read it and then clear the
interrupt.

> +     }
> +
> +     /* All packets processed */
> +     if (num_pkts < quota) {
> +             napi_complete(napi);
> +             /* Enable Rx FIFO interrupts */
> +             rcar_canfd_set_bit(priv, RCANFD_RFCC(ridx), RCANFD_RFFIFO_RFIE);
> +     }
> +     return num_pkts;
> +}
> +
> +static int rcar_canfd_do_set_mode(struct net_device *ndev, enum can_mode 
> mode)
> +{
> +     int err;
> +
> +     switch (mode) {
> +     case CAN_MODE_START:
> +             err = rcar_canfd_start(ndev);
> +             if (err)
> +                     return err;
> +             netif_wake_queue(ndev);
> +             return 0;
> +     default:
> +             return -EOPNOTSUPP;
> +     }
> +}
> +
> +static int rcar_canfd_get_berr_counter(const struct net_device *dev,
> +                                    struct can_berr_counter *bec)
> +{
> +     struct rcar_canfd_channel *priv = netdev_priv(dev);
> +     u32 val, ch = priv->channel;
> +
> +     /* Peripheral clock is already enabled in probe */
> +     val = rcar_canfd_read(priv, RCANFD_CSTS(ch));
> +     bec->txerr = RCANFD_CSTS_TECCNT(val);
> +     bec->rxerr = RCANFD_CSTS_RECCNT(val);
> +     return 0;
> +}
> +
> +static const struct net_device_ops rcar_canfd_netdev_ops = {
> +     .ndo_open = rcar_canfd_open,
> +     .ndo_stop = rcar_canfd_close,
> +     .ndo_start_xmit = rcar_canfd_start_xmit,
> +     .ndo_change_mtu = can_change_mtu,
> +};
> +
> +static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch)
> +{
> +     struct platform_device *pdev = gpriv->pdev;
> +     struct rcar_canfd_channel *priv;
> +     struct net_device *ndev;
> +     int err = -ENODEV;
> +
> +     ndev = alloc_candev(sizeof(*priv), RCANFD_FIFO_DEPTH);
> +     if (!ndev) {
> +             dev_err(&pdev->dev, "alloc_candev() failed\n");
> +             err = -ENOMEM;
> +             goto fail;
> +     }
> +     priv = netdev_priv(ndev);
> +
> +     ndev->netdev_ops = &rcar_canfd_netdev_ops;
> +     ndev->flags |= IFF_ECHO;
> +     priv->ndev = ndev;
> +     priv->base = gpriv->base;
> +     priv->channel = ch;
> +     priv->can.clock.freq = gpriv->freq;
> +     dev_info(&pdev->dev, "can_clk rate is %u\n", priv->can.clock.freq);
> +
> +     priv->can.bittiming_const = &rcar_canfd_nom_bittiming_const;
> +     priv->can.data_bittiming_const =
> +             &rcar_canfd_data_bittiming_const;
> +
> +     /* Controller starts in CAN FD mode, which supports classical CAN and
> +      * CAN FD frames. For classical CAN frames only configurations, data
> +      * bitrate should be configured the same as nominal bitrate.
> +      */
> +     priv->can.ctrlmode_supported = CAN_CTRLMODE_BERR_REPORTING |
> +             CAN_CTRLMODE_FD;
> +
> +     priv->can.do_set_mode = rcar_canfd_do_set_mode;
> +     priv->can.do_get_berr_counter = rcar_canfd_get_berr_counter;
> +     priv->gpriv = gpriv;
> +     SET_NETDEV_DEV(ndev, &pdev->dev);
> +
> +     netif_napi_add(ndev, &priv->napi, rcar_canfd_rx_poll,
> +                    RCANFD_NAPI_WEIGHT);
> +     err = register_candev(ndev);
> +     if (err) {
> +             dev_err(&pdev->dev,
> +                     "register_candev() failed, error %d\n", err);
> +             goto fail_candev;
> +     }
> +     spin_lock_init(&priv->tx_lock);
> +     devm_can_led_init(ndev);
> +     gpriv->ch[priv->channel] = priv;
> +     dev_info(&pdev->dev, "device registered (channel %u)\n", priv->channel);
> +     return 0;
> +
> +fail_candev:
> +     netif_napi_del(&priv->napi);
> +     free_candev(ndev);
> +fail:
> +     return err;
> +}
> +
> +static void rcar_canfd_channel_remove(struct rcar_canfd_global *gpriv, u32 
> ch)
> +{
> +     struct rcar_canfd_channel *priv = gpriv->ch[ch];
> +
> +     if (priv) {
> +             unregister_candev(priv->ndev);
> +             netif_napi_del(&priv->napi);
> +             free_candev(priv->ndev);
> +     }
> +}
> +
> +static int rcar_canfd_probe(struct platform_device *pdev)
> +{
> +     struct resource *mem;
> +     void __iomem *addr;
> +     u32 sts, ch;
> +     struct rcar_canfd_global *gpriv;
> +     struct device_node *of_child;
> +     unsigned long channels_mask = 0;
> +     int err, ch_irq, g_irq;
> +
> +     of_child = of_get_child_by_name(pdev->dev.of_node, "channel0");
> +     if (of_child && of_device_is_available(of_child))
> +             channels_mask |= BIT(0);        /* Channel 0 */
> +
> +     of_child = of_get_child_by_name(pdev->dev.of_node, "channel1");
> +     if (of_child && of_device_is_available(of_child))
> +             channels_mask |= BIT(1);        /* Channel 1 */
> +
> +     ch_irq = platform_get_irq(pdev, 0);
> +     if (ch_irq < 0) {
> +             dev_err(&pdev->dev, "no Channel IRQ resource\n");
> +             err = ch_irq;
> +             goto fail_dev;
> +     }
> +
> +     g_irq = platform_get_irq(pdev, 1);
> +     if (g_irq < 0) {
> +             dev_err(&pdev->dev, "no Global IRQ resource\n");
> +             err = g_irq;
> +             goto fail_dev;
> +     }
> +
> +     /* Global controller context */
> +     gpriv = devm_kzalloc(&pdev->dev, sizeof(*gpriv), GFP_KERNEL);
> +     if (!gpriv) {
> +             err = -ENOMEM;
> +             goto fail_dev;
> +     }
> +     gpriv->pdev = pdev;
> +     gpriv->channels_mask = channels_mask;
> +
> +     /* Peripheral clock */
> +     gpriv->clkp = devm_clk_get(&pdev->dev, "fck");
> +     if (IS_ERR(gpriv->clkp)) {
> +             err = PTR_ERR(gpriv->clkp);
> +             dev_err(&pdev->dev, "cannot get peripheral clock, error %d\n",
> +                     err);
> +             goto fail_dev;
> +     }
> +
> +     /* fCAN clock: Pick External clock. If not available fallback to
> +      * CANFD clock
> +      */
> +     gpriv->can_clk = devm_clk_get(&pdev->dev, "can_clk");
> +     if (IS_ERR(gpriv->can_clk)) {
> +             gpriv->can_clk = devm_clk_get(&pdev->dev, "canfd");
> +             if (IS_ERR(gpriv->can_clk)) {
> +                     err = PTR_ERR(gpriv->can_clk);
> +                     dev_err(&pdev->dev,
> +                             "cannot get canfd clock, error %d\n", err);
> +                     goto fail_dev;
> +             }
> +             gpriv->clock_select = RCANFD_CANFDCLK;
> +
> +     } else {
> +             gpriv->clock_select = RCANFD_EXTCLK;
> +     }
> +     gpriv->freq = clk_get_rate(gpriv->can_clk);
> +
> +     if (gpriv->clock_select == RCANFD_CANFDCLK)
> +             /* CANFD clock is further divided by (1/2) within the IP */
> +             gpriv->freq /= 2;
> +
> +     mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +     addr = devm_ioremap_resource(&pdev->dev, mem);
> +     if (IS_ERR(addr)) {
> +             err = PTR_ERR(addr);
> +             goto fail_dev;
> +     }
> +     gpriv->base = addr;
> +
> +     /* Request IRQ that's common for both channels */
> +     err = devm_request_irq(&pdev->dev, ch_irq,
> +                            rcar_canfd_channel_interrupt, 0,
> +                            "canfd.chn", gpriv);
> +     if (err) {
> +             dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
> +                     ch_irq, err);
> +             goto fail_dev;
> +     }
> +     err = devm_request_irq(&pdev->dev, g_irq,
> +                            rcar_canfd_global_interrupt, 0,
> +                            "canfd.gbl", gpriv);
> +     if (err) {
> +             dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
> +                     g_irq, err);
> +             goto fail_dev;
> +     }
> +
> +     /* Enable peripheral clock for register access */
> +     err = clk_prepare_enable(gpriv->clkp);
> +     if (err) {
> +             dev_err(&pdev->dev,
> +                     "failed to enable peripheral clock, error %d\n", err);
> +             goto fail_dev;
> +     }
> +
> +     err = rcar_canfd_reset_controller(gpriv);
> +     if (err) {
> +             dev_err(&pdev->dev, "reset controller failed\n");
> +             goto fail_clk;
> +     }
> +
> +     /* Controller in Global reset & Channel reset mode */
> +     rcar_canfd_configure_controller(gpriv);
> +
> +     /* Configure per channel attributes */
> +     for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
> +             /* Configure Channel's Rx fifo */
> +             rcar_canfd_configure_rx(gpriv, ch);
> +
> +             /* Configure Channel's Tx (Common) fifo */
> +             rcar_canfd_configure_tx(gpriv, ch);
> +
> +             /* Configure receive rules */
> +             rcar_canfd_configure_afl_rules(gpriv, ch);
> +     }
> +
> +     /* Configure common interrupts */
> +     rcar_canfd_enable_global_interrupts(gpriv);
> +
> +     /* Start Global operation mode */
> +     rcar_canfd_update_bit(gpriv, RCANFD_GCTR, RCANFD_GCTR_MODEMASK,
> +                           RCANFD_GCTR_GOPM);
> +
> +     /* Verify mode change */
> +     err = readl_poll_timeout((gpriv->base + RCANFD_GSTS), sts,
> +                              !(sts & RCANFD_GSTS_GNOPM), 2, 500000);
> +     if (err) {
> +             dev_err(&pdev->dev, "global operational mode failed\n");
> +             goto fail_mode;
> +     }
> +
> +     for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
> +             err = rcar_canfd_channel_probe(gpriv, ch);
> +             if (err)
> +                     goto fail_channel;
> +     }

Should the CAN IP core be clocked the whole time? What about shuting
down the clock and enabling it on ifup?

> +
> +     platform_set_drvdata(pdev, gpriv);
> +     dev_info(&pdev->dev, "global operational state (clk %d)\n",
> +              gpriv->clock_select);
> +     return 0;
> +
> +fail_channel:
> +     for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS)
> +             rcar_canfd_channel_remove(gpriv, ch);
> +fail_mode:
> +     rcar_canfd_disable_global_interrupts(gpriv);
> +fail_clk:
> +     clk_disable_unprepare(gpriv->clkp);
> +fail_dev:
> +     return err;
> +}
> +
> +static int rcar_canfd_remove(struct platform_device *pdev)
> +{
> +     struct rcar_canfd_global *gpriv = platform_get_drvdata(pdev);
> +     struct rcar_canfd_channel *priv;
> +     u32 ch;
> +
> +     rcar_canfd_reset_controller(gpriv);
> +     rcar_canfd_disable_global_interrupts(gpriv);
> +
> +     for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
> +             priv = gpriv->ch[ch];
> +             if (priv) {

This should always be true.

> +                     rcar_canfd_disable_channel_interrupts(priv);
> +                     unregister_candev(priv->ndev);
> +                     netif_napi_del(&priv->napi);
> +                     free_candev(priv->ndev);

Please make use of rcar_canfd_channel_remove(), as you already have the
function.

> +             }
> +     }
> +
> +     /* Enter global sleep mode */
> +     rcar_canfd_set_bit(gpriv, RCANFD_GCTR, RCANFD_GCTR_SLPR);
> +     clk_disable_unprepare(gpriv->clkp);
> +     return 0;
> +}
> +
> +static int __maybe_unused rcar_canfd_suspend(struct device *dev)
> +{
> +     return 0;
> +}
> +
> +static int __maybe_unused rcar_canfd_resume(struct device *dev)
> +{
> +     return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(rcar_canfd_pm_ops, rcar_canfd_suspend,
> +                      rcar_canfd_resume);
> +
> +static const struct of_device_id rcar_canfd_of_table[] = {
> +     { .compatible = "renesas,rcar-gen3-canfd" },
> +     { }
> +};
> +
> +MODULE_DEVICE_TABLE(of, rcar_canfd_of_table);
> +
> +static struct platform_driver rcar_canfd_driver = {
> +     .driver = {
> +             .name = RCANFD_DRV_NAME,
> +             .of_match_table = of_match_ptr(rcar_canfd_of_table),
> +             .pm = &rcar_canfd_pm_ops,
> +     },
> +     .probe = rcar_canfd_probe,
> +     .remove = rcar_canfd_remove,
> +};
> +
> +module_platform_driver(rcar_canfd_driver);
> +
> +MODULE_AUTHOR("Ramesh Shanmugasundaram 
> <ramesh.shanmugasunda...@bp.renesas.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("CAN FD driver for Renesas R-Car SoC");
> +MODULE_ALIAS("platform:" RCANFD_DRV_NAME);

regards,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to