can: add Renesas R-Car CAN driver

Commit Message

Sergei Shtylyov Sept. 27, 2013, 10:11 p.m. UTC

Add support for the CAN controller found in Renesas R-Car SoCs. 

Signed-off-by: Sergei Shtylyov <[e-mail]>

---
The patch is against the 'linux-can-next.git' repo.

 drivers/net/can/Kconfig               |    9 
 drivers/net/can/Makefile              |    1 
 drivers/net/can/rcar_can.c            |  898 ++++++++++++++++++++++++++++++++++
 include/linux/can/platform/rcar_can.h |   15 
 4 files changed, 923 insertions(+)

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Joe Perches Sept. 28, 2013, 12:40 a.m. UTC | #1

On Sat, 2013-09-28 at 02:11 +0400, Sergei Shtylyov wrote:
> Add support for the CAN controller found in Renesas R-Car SoCs. 

trivia:

> +++ linux-can-next/drivers/net/can/rcar_can.c
[]
> +static void rcar_can_rx_pkt(struct rcar_can_priv *priv, int mbx)
> +{
> +       struct net_device_stats *stats = &priv->ndev->stats;
> +       struct can_frame *cf;
> +       struct sk_buff *skb;
> +       u32 data;
> +       u8 dlc;
> +
> +       skb = alloc_can_skb(priv->ndev, &cf);
> +       if (!skb) {
> +               stats->rx_dropped++;
> +               if (printk_ratelimit())
> +                       netdev_err(priv->ndev,
> +                                  "%s: alloc_can_skb() failed\n", __func__);

This logging message is unnecessary because it's
redundant to the generic OOM error in __alloc_skb.
It already gets a dump_stack();




--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Stephen Hemminger Sept. 28, 2013, 12:45 a.m. UTC | #2

On Sat, 28 Sep 2013 02:11:38 +0400
Sergei Shtylyov <[e-mail]> wrote:

> +	stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static irqreturn_t rcar_can_interrupt(int irq, void *dev_id)
> +{
> +	struct net_device *ndev = (struct net_device *)dev_id;
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	u8 isr;
> +
> +	isr = rcar_can_readb(priv, RCAR_CAN_ISR);
> +	if (isr & ISR_ERSF)
> +		rcar_can_error(ndev);

IRQ handler should return IRQ_NONE if not for this device.
Most devices this is true if isr is 0.
Also should check for hot-plug case where device interrupt occurs
but see's all-ones on the bus.

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sergei Shtylyov Sept. 28, 2013, 12:52 a.m. UTC | #3

Hello.

On 09/28/2013 04:45 AM, Stephen Hemminger wrote:

>> +	stats->rx_bytes += cf->can_dlc;
>> +}
>> +
>> +static irqreturn_t rcar_can_interrupt(int irq, void *dev_id)
>> +{
>> +	struct net_device *ndev = (struct net_device *)dev_id;
>> +	struct rcar_can_priv *priv = netdev_priv(ndev);
>> +	struct net_device_stats *stats = &ndev->stats;
>> +	u8 isr;
>> +
>> +	isr = rcar_can_readb(priv, RCAR_CAN_ISR);
>> +	if (isr & ISR_ERSF)
>> +		rcar_can_error(ndev);

> IRQ handler should return IRQ_NONE if not for this device.
> Most devices this is true if isr is 0.

    Hm, totally overlooked that... :-/

> Also should check for hot-plug case where device interrupt occurs
> but see's all-ones on the bus.

    Hotplug is not possible with this SoC device.

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Marc Kleine-Budde Sept. 29, 2013, 7:03 p.m. UTC | #4

On 09/28/2013 12:11 AM, Sergei Shtylyov wrote:
> Add support for the CAN controller found in Renesas R-Car SoCs.

Is there a public available datasheet for the CAN core? What
architecture are the Renesas R-Car SoCs? They're ARM, aren't they?
What's R-Car's status on device tree conversion?

More comments inline

> Signed-off-by: Sergei Shtylyov <[e-mail]>
> 
> ---
> The patch is against the 'linux-can-next.git' repo.
> 
>  drivers/net/can/Kconfig               |    9 
>  drivers/net/can/Makefile              |    1 
>  drivers/net/can/rcar_can.c            |  898 ++++++++++++++++++++++++++++++++++
>  include/linux/can/platform/rcar_can.h |   15 
>  4 files changed, 923 insertions(+)
> 
> Index: linux-can-next/drivers/net/can/Kconfig
> ===================================================================
> --- linux-can-next.orig/drivers/net/can/Kconfig
> +++ linux-can-next/drivers/net/can/Kconfig
> @@ -125,6 +125,15 @@ config CAN_GRCAN
>  	  endian syntheses of the cores would need some modifications on
>  	  the hardware level to work.
>  
> +config CAN_RCAR
> +	tristate "Renesas R-Car CAN controller"
> +	---help---
> +	  Say Y here if you want to use CAN controller found on Renesas R-Car
> +	  SoCs.
> +
> +	  To compile this driver as a module, choose M here: the module will
> +	  be called rcar_can.
> +
>  source "drivers/net/can/mscan/Kconfig"
>  
>  source "drivers/net/can/sja1000/Kconfig"
> Index: linux-can-next/drivers/net/can/Makefile
> ===================================================================
> --- linux-can-next.orig/drivers/net/can/Makefile
> +++ linux-can-next/drivers/net/can/Makefile
> @@ -25,5 +25,6 @@ obj-$(CONFIG_CAN_JANZ_ICAN3)	+= janz-ica
>  obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
>  obj-$(CONFIG_PCH_CAN)		+= pch_can.o
>  obj-$(CONFIG_CAN_GRCAN)		+= grcan.o
> +obj-$(CONFIG_CAN_RCAR)		+= rcar_can.o
>  
>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> Index: linux-can-next/drivers/net/can/rcar_can.c
> ===================================================================
> --- /dev/null
> +++ linux-can-next/drivers/net/can/rcar_can.c
> @@ -0,0 +1,898 @@
> +/*
> + * Renesas R-Car CAN device driver
> + *
> + * Copyright (C) 2013 Cogent Embedded, Inc. <[e-mail]>
> + * Copyright (C) 2013 Renesas Solutions 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/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/can/platform/rcar_can.h>
> +
> +#define DRV_NAME	"rcar_can"
> +
> +#define RCAR_CAN_MIER1	0x42C	/* CANi Mailbox Interrupt Enable Register 1 */
> +#define RCAR_CAN_MKR(n)	((n) < 2 ? 0x430 + 4 * (n) : 0x400 + 4 * ((n) - 2))
> +				/* CANi Mask Register */
> +#define RCAR_CAN_MKIVLR0 0x438	/* CANi Mask Invalid Register 0 */
> +#define RCAR_CAN_MIER0	 0x43C	/* CANi Mailbox Interrupt Enable Register 0 */
> +
> +#define RCAR_CAN_MCTL(n) (0x800 + (n)) /* CANi Message Control Register */
> +#define RCAR_CAN_CTLR	0x840	/* CANi Control Register */
> +#define RCAR_CAN_STR	0x842	/* CANi Status Register */
> +#define RCAR_CAN_BCR	0x844	/* CANi Bit Configuration Register */
> +#define RCAR_CAN_CLKR	0x847	/* CANi Clock Select Register */
> +#define RCAR_CAN_EIER	0x84C	/* CANi Error Interrupt Enable Register */
> +#define RCAR_CAN_EIFR	0x84D	/* CANi Err Interrupt Factor Judge Register */
> +#define RCAR_CAN_RECR	0x84E	/* CANi Receive Error Count Register */
> +#define RCAR_CAN_TECR	0x84F	/* CANi Transmit Error Count Register */
> +#define RCAR_CAN_ECSR	0x850	/* CANi Error Code Store Register */
> +#define RCAR_CAN_MSSR	0x852	/* CANi Mailbox Search Status Register */
> +#define RCAR_CAN_MSMR	0x853	/* CANi Mailbox Search Mode Register */
> +#define RCAR_CAN_TCR	0x858	/* CANi Test Control Register */
> +#define RCAR_CAN_IER	0x860	/* CANi Interrupt Enable Register */
> +#define RCAR_CAN_ISR	0x861	/* CANi Interrupt Status Register */

You might consider using an enum for the register offsets.

> +
> +/* Offsets of RCAR_CAN Mailbox Registers */
> +#define MBX_HDR_OFFSET	0x0
> +#define MBX_DLC_OFFSET	0x5
> +#define MBX_DATA_OFFSET	0x6

can you please add the RCAR_ prefix to all defines.

> +
> +#define RCAR_CAN_MBX_SIZE 0x10
> +
> +/* Control Register bits */
> +#define CTLR_SLPM	BIT(10)
> +#define CTLR_HALT	BIT(9)
> +#define CTLR_RESET	BIT(8)
> +#define CTLR_FORCE_RESET (3 << 8)
> +#define CTLR_TPM	BIT(4)	/* Transmission Priority Mode Select Bit */
> +#define CTLR_IDFM_MIXED	BIT(2)	/* Mixed ID mode */
> +
> +/* Message Control Register bits */
> +#define MCTL_TRMREQ	BIT(7)
> +#define MCTL_RECREQ	BIT(6)
> +#define MCTL_ONESHOT	BIT(4)
> +#define MCTL_SENTDATA	BIT(0)
> +#define MCTL_NEWDATA	BIT(0)
> +
> +#define N_RX_MKREGS	2	/* Number of mask registers */
> +				/* for Rx mailboxes 0-31 */
> +
> +/* Bit Configuration Register settings */
> +#define BCR_TSEG1(x)	(((x) & 0x0f) << 28)
> +#define BCR_BPR(x)	(((x) & 0x3ff) << 16)
> +#define BCR_SJW(x)	(((x) & 0x3) << 12)
> +#define BCR_TSEG2(x)	(((x) & 0x07) << 8)
> +
> +/* Mailbox and Mask Registers bits */
> +#define RCAR_CAN_IDE	BIT(31)
> +#define RCAR_CAN_RTR	BIT(30)
> +#define RCAR_CAN_SID_SHIFT 18
> +
> +/* Interrupt Enable Register bits */
> +#define IER_ERSIE	BIT(5)	/* Error (ERS) Interrupt Enable Bit */
> +#define IER_RXM0IE	BIT(2)	/* Mailbox 0 Successful Reception (RXM0) */
> +				/* Interrupt Enable Bit */
> +#define IER_RXM1IE	BIT(1)	/* Mailbox 1 Successful Reception (RXM0) */
> +				/* Interrupt Enable Bit */
> +#define IER_TXMIE	BIT(0)	/* Mailbox 32 to 63 Successful Tx */
> +				/* Interrupt Enable Bit */
> +
> +/* Interrupt Status Register bits */
> +#define ISR_ERSF	BIT(5)	/* Error (ERS) Interrupt Status Bit */
> +#define ISR_RXM0F	BIT(2)	/* Mailbox 0 Successful Reception (RXM0) */
> +				/* Interrupt Status Bit */
> +#define ISR_RXM1F	BIT(1)	/* Mailbox 1 to 63 Successful Reception */
> +				/* (RXM1) Interrupt Status Bit */
> +#define ISR_TXMF	BIT(0)	/* Mailbox 32 to 63 Successful Transmission */
> +				/* (TXM) Interrupt Status Bit */
> +
> +/* Error Interrupt Enable Register bits */
> +#define EIER_BLIE	BIT(7)	/* Bus Lock Interrupt Enable */
> +#define EIER_OLIE	BIT(6)	/* Overload Frame Transmit Interrupt Enable */
> +#define EIER_ORIE	BIT(5)	/* Receive Overrun Interrupt Enable */
> +#define EIER_BORIE	BIT(4)	/* Bus-Off Recovery Interrupt Enable */
> +
> +#define EIER_BOEIE	BIT(3)	/* Bus-Off Entry Interrupt Enable */
> +#define EIER_EPIE	BIT(2)	/* Error Passive Interrupt Enable */
> +#define EIER_EWIE	BIT(1)	/* Error Warning Interrupt Enable */
> +#define EIER_BEIE	BIT(0)	/* Bus Error Interrupt Enable */
> +
> +/* Error Interrupt Factor Judge Register bits */
> +#define EIFR_BLIF	BIT(7)	/* Bus Lock Detect Flag */
> +#define EIFR_OLIF	BIT(6)	/* Overload Frame Transmission Detect Flag */
> +#define EIFR_ORIF	BIT(5)	/* Receive Overrun Detect Flag */
> +#define EIFR_BORIF	BIT(4)	/* Bus-Off Recovery Detect Flag */
> +#define EIFR_BOEIF	BIT(3)	/* Bus-Off Entry Detect Flag */
> +#define EIFR_EPIF	BIT(2)	/* Error Passive Detect Flag */
> +#define EIFR_EWIF	BIT(1)	/* Error Warning Detect Flag */
> +#define EIFR_BEIF	BIT(0)	/* Bus Error Detect Flag */
> +
> +/* Error Code Store Register bits */
> +#define ECSR_EDPM	BIT(7)	/* Error Display Mode Select Bit */
> +#define ECSR_ADEF	BIT(6)	/* ACK Delimiter Error Flag */
> +#define ECSR_BE0F	BIT(5)	/* Bit Error (dominant) Flag */
> +#define ECSR_BE1F	BIT(4)	/* Bit Error (recessive) Flag */
> +#define ECSR_CEF	BIT(3)	/* CRC Error Flag */
> +#define ECSR_AEF	BIT(2)	/* ACK Error Flag */
> +#define ECSR_FEF	BIT(1)	/* Form Error Flag */
> +#define ECSR_SEF	BIT(0)	/* Stuff Error Flag */
> +
> +/* Mailbox Search Status Register bits */
> +#define MSSR_SEST	BIT(7)	/* Search Result Status Bit */
> +#define MSSR_MBNST	0x3f	/* Search Result Mailbox Number Status mask */
> +
> +/* Mailbox Search Mode Register values */
> +#define MSMR_TXMB	1	/* Transmit mailbox search mode */
> +#define MSMR_RXMB	0	/* Receive mailbox search mode */
> +
> +/* Mailbox configuration:
> + * mailbox 0 - not used
> + * mailbox 1-31 - Rx
> + * mailbox 32-63 - Tx
> + * no FIFO mailboxes
> + */
> +#define N_MBX		64
> +#define FIRST_TX_MB	32
> +#define RX_MBX_MASK	0xFFFFFFFE
> +
> +#define RCAR_CAN_NAPI_WEIGHT (FIRST_TX_MB - 1)
> +
> +struct rcar_can_priv {
> +	struct can_priv can;	/* Must be the first member! */
> +	struct net_device *ndev;
> +	struct napi_struct napi;
> +	void __iomem *reg_base;
> +	struct clk *clk;
> +	spinlock_t mier_lock;
> +	u8 clock_select;
> +};
> +
> +static const struct can_bittiming_const rcar_can_bittiming_const = {
> +	.name = DRV_NAME,
> +	.tseg1_min = 4,
> +	.tseg1_max = 16,
> +	.tseg2_min = 2,
> +	.tseg2_max = 8,
> +	.sjw_max = 4,
> +	.brp_min = 1,
> +	.brp_max = 1024,
> +	.brp_inc = 1,
> +};
> +

If you use use an enum for the register offsets you can change the int
to that enum in the accessor functions.

> +static inline u32 rcar_can_readl(struct rcar_can_priv *priv, int reg)
> +{
> +	return readl(priv->reg_base + reg);
> +}
> +
> +static inline u16 rcar_can_readw(struct rcar_can_priv *priv, int reg)
> +{
> +	return readw(priv->reg_base + reg);
> +}
> +
> +static inline u8 rcar_can_readb(struct rcar_can_priv *priv, int reg)
> +{
> +	return readb(priv->reg_base + reg);
> +}
> +
> +static inline void rcar_can_writel(struct rcar_can_priv *priv, int reg, u32 val)
> +{
> +	writel(val, priv->reg_base + reg);
> +}
> +
> +static inline void rcar_can_writew(struct rcar_can_priv *priv, int reg, u16 val)
> +{
> +	writew(val, priv->reg_base + reg);
> +}
> +
> +static inline void rcar_can_writeb(struct rcar_can_priv *priv, int reg, u8 val)
> +{
> +	writeb(val, priv->reg_base + reg);
> +}
> +
> +static inline u32 rcar_can_mbx_readl(struct rcar_can_priv *priv,
> +				     u32 mbxno, u8 offset)
> +{
> +	return rcar_can_readl(priv, RCAR_CAN_MBX_SIZE * mbxno + offset);

Can you define hide the RCAR_CAN_MBX_SIZE * mbxno into a macro?
Something like:

    RCAR_CAN_MBX(mbxno)

> +}
> +
> +static inline u8 rcar_can_mbx_readb(struct rcar_can_priv *priv,
> +				    u32 mbxno, u8 offset)
> +{
> +	return rcar_can_readb(priv, RCAR_CAN_MBX_SIZE * mbxno + offset);
> +}
> +
> +static inline void rcar_can_mbx_writel(struct rcar_can_priv *priv, u32 mbxno,
> +				       u8 offset, u32 val)
> +{
> +	rcar_can_writel(priv, RCAR_CAN_MBX_SIZE * mbxno + offset, val);
> +}
> +
> +static inline void rcar_can_mbx_writeb(struct rcar_can_priv *priv, u32 mbxno,
> +				       u8 offset, u8 val)
> +{
> +	rcar_can_writeb(priv, RCAR_CAN_MBX_SIZE * mbxno + offset, val);
> +}
> +
> +static void rcar_can_error(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	u8 eifr;
> +
> +	/* Propagate the error condition to the CAN stack */
> +	skb = alloc_can_err_skb(ndev, &cf);
> +	if (!skb) {
> +		if (printk_ratelimit())
> +			netdev_err(priv->ndev,
> +				   "%s: alloc_can_err_skb() failed\n",
> +				   __func__);
> +		return;
> +	}
> +
> +	eifr = rcar_can_readb(priv, RCAR_CAN_EIFR);
> +	if (eifr & EIFR_EWIF) {
> +		netdev_dbg(priv->ndev, "Error warning interrupt\n");
> +		priv->can.state = CAN_STATE_ERROR_WARNING;
> +		priv->can.can_stats.error_warning++;
> +		cf->can_id |= CAN_ERR_CRTL;
> +		if (rcar_can_readb(priv, RCAR_CAN_TECR) > 96)
> +			cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
> +		if (rcar_can_readb(priv, RCAR_CAN_RECR) > 96)
> +			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> +		/* Clear interrupt condition */
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EWIF);

The cast is not needed.

> +	}
> +	if (eifr & EIFR_EPIF) {
> +		netdev_dbg(priv->ndev, "Error passive interrupt\n");
> +		priv->can.state = CAN_STATE_ERROR_PASSIVE;
> +		priv->can.can_stats.error_passive++;
> +		cf->can_id |= CAN_ERR_CRTL;
> +		if (rcar_can_readb(priv, RCAR_CAN_TECR) > 127)
> +			cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
> +		if (rcar_can_readb(priv, RCAR_CAN_RECR) > 127)
> +			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
> +		/* Clear interrupt condition */
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EPIF);
> +	}
> +	if (eifr & EIFR_BOEIF) {
> +		netdev_dbg(priv->ndev, "Bus-off entry interrupt\n");
> +		priv->can.state = CAN_STATE_BUS_OFF;
> +		cf->can_id |= CAN_ERR_BUSOFF;
> +		/* Clear interrupt condition */
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BOEIF);
> +		/* Disable all interrupts in bus-off to avoid int hog */
> +		rcar_can_writeb(priv, RCAR_CAN_EIER, 0);
> +		rcar_can_writeb(priv, RCAR_CAN_IER, 0);
> +		can_bus_off(ndev);

How does the rcan recover from bus off?

> +	}
> +	if (eifr & EIFR_BEIF) {
> +		int rx_errors = 0, tx_errors = 0, bus_errors = 0;
> +		u8 ecsr;
> +
> +		netdev_dbg(priv->ndev, "Bus error interrupt:\n");
> +		cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
> +		cf->data[2] = CAN_ERR_PROT_UNSPEC;
> +
> +		ecsr = rcar_can_readb(priv, RCAR_CAN_ECSR);
> +		if (ecsr & ECSR_ADEF) {
> +			netdev_dbg(priv->ndev, "ACK Delimiter Error\n");
> +			cf->data[3] |= CAN_ERR_PROT_LOC_ACK_DEL;
> +			bus_errors++;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_ADEF);
> +		}
> +		if (ecsr & ECSR_BE0F) {
> +			netdev_dbg(priv->ndev, "Bit Error (dominant)\n");
> +			cf->data[2] |= CAN_ERR_PROT_BIT0;
> +			bus_errors++;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_BE0F);
> +		}
> +		if (ecsr & ECSR_BE1F) {
> +			netdev_dbg(priv->ndev, "Bit Error (recessive)\n");
> +			cf->data[2] |= CAN_ERR_PROT_BIT1;
> +			bus_errors++;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_BE1F);
> +		}
> +		if (ecsr & ECSR_CEF) {
> +			netdev_dbg(priv->ndev, "CRC Error\n");
> +			cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
> +			bus_errors++;
> +			rx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_CEF);
> +		}
> +		if (ecsr & ECSR_AEF) {
> +			netdev_dbg(priv->ndev, "ACK Error\n");
> +			cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
> +			bus_errors++;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_AEF);
> +		}
> +		if (ecsr & ECSR_FEF) {
> +			netdev_dbg(priv->ndev, "Form Error\n");
> +			cf->data[2] |= CAN_ERR_PROT_FORM;
> +			bus_errors++;
> +			rx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_FEF);
> +		}
> +		if (ecsr & ECSR_SEF) {
> +			netdev_dbg(priv->ndev, "Stuff Error\n");
> +			cf->data[2] |= CAN_ERR_PROT_STUFF;
> +			bus_errors++;
> +			rx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_SEF);
> +		}
> +
> +		priv->can.can_stats.bus_error += bus_errors;
> +		ndev->stats.rx_errors += rx_errors;
> +		ndev->stats.tx_errors += tx_errors;
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BEIF);
> +	}
> +	if (eifr & EIFR_ORIF) {
> +		netdev_dbg(priv->ndev, "Receive overrun error interrupt\n");
> +		cf->can_id |= CAN_ERR_CRTL;
> +		cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> +		ndev->stats.rx_over_errors++;
> +		ndev->stats.rx_errors++;
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_ORIF);
> +	}
> +	if (eifr & EIFR_OLIF) {
> +		netdev_dbg(priv->ndev,
> +			   "Overload Frame Transmission error interrupt\n");
> +		cf->can_id |= CAN_ERR_PROT;
> +		cf->data[2] |= CAN_ERR_PROT_OVERLOAD;
> +		ndev->stats.rx_over_errors++;
> +		ndev->stats.rx_errors++;
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_OLIF);
> +	}
> +
> +	netif_rx(skb);
> +	stats->rx_packets++;
> +	stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static irqreturn_t rcar_can_interrupt(int irq, void *dev_id)
> +{
> +	struct net_device *ndev = (struct net_device *)dev_id;
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	u8 isr;
> +
> +	isr = rcar_can_readb(priv, RCAR_CAN_ISR);
> +	if (isr & ISR_ERSF)
> +		rcar_can_error(ndev);
> +
> +	if (isr & ISR_TXMF) {
> +		u32 ie_mask = 0;
> +
> +		/* Set Transmit Mailbox Search Mode */
> +		rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_TXMB);

Can you please outline how to mailbox search mode works? What happens if
you activate tx mailbox search mode here and rx mailbox search mode below?

> +		while (1) {

I presonally don't like while (1) loops in an interrupt handler, can you
rearange the code, so that you have an explizid loop termination?

> +			u8 mctl, mbx;
> +
> +			mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
> +			if (mbx & MSSR_SEST)
> +				break;
> +			mbx &= MSSR_MBNST;
> +			mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
> +			/* Bits SENTDATA and TRMREQ cannot be
> +			 * set to 0 simultaneously
> +			 */
> +			mctl &= ~MCTL_TRMREQ;
> +			rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
> +			mctl &= ~MCTL_SENTDATA;
> +			/* Clear interrupt */
> +			rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);

Can you combine both writes to RCAR_CAN_MCTL(mbx) or does the hardware
require two separate writes?

> +			ie_mask |= BIT(mbx - FIRST_TX_MB);
> +			stats->tx_bytes += can_get_echo_skb(ndev,
> +							    mbx - FIRST_TX_MB);

Can you guarantee that you call can_get_echo_skb in the same order the
frames get send?

> +			stats->tx_packets++;
> +			can_led_event(ndev, CAN_LED_EVENT_TX);
> +		}
> +		/* Set receive mailbox search mode */
> +		rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_RXMB);
> +		/* Disable mailbox interrupt, mark mailbox as free */
> +		if (ie_mask) {
> +			u32 mier1;
> +
> +			spin_lock(&priv->mier_lock);
> +			mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
> +			rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 & ~ie_mask);
> +			spin_unlock(&priv->mier_lock);
> +			if (unlikely(netif_queue_stopped(ndev)))
> +				netif_wake_queue(ndev);

You can call netif_wake_queue() unconditionally, it does the check for
queue stopped anyway.

> +		}
> +	}
> +	if (isr & ISR_RXM1F) {
> +		if (napi_schedule_prep(&priv->napi)) {
> +			/* Disable Rx interrupts */
> +			rcar_can_writeb(priv, RCAR_CAN_IER,
> +					rcar_can_readb(priv, RCAR_CAN_IER) &
> +						       ~IER_RXM1IE);
> +			__napi_schedule(&priv->napi);
> +		}
> +	}
> +	return IRQ_HANDLED;

Do not return IRQ_HANDLED unconditionally. You should return IRQ_HANDLED
only if there really was a interrupt for your device.

> +}
> +
> +static int rcar_can_set_bittiming(struct net_device *dev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(dev);
> +	struct can_bittiming *bt = &priv->can.bittiming;
> +	u32 bcr;
> +	u16 ctlr;
> +	u8 clkr;
> +
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	if (ctlr & CTLR_SLPM) {
> +		/* Write to BCR in CAN reset mode or CAN halt mode */
> +		return -EBUSY;

The framework guarantees that set_bittiming is only called when the
interface is down.

> +	}
> +	/* Don't overwrite CLKR with 32-bit BCR access */
> +	/* CLKR has 8-bit access */
> +	clkr = rcar_can_readb(priv, RCAR_CAN_CLKR);
> +	bcr = BCR_TSEG1(bt->phase_seg1 + bt->prop_seg - 1) |
> +	      BCR_BPR(bt->brp - 1) | BCR_SJW(bt->sjw - 1) |
> +	      BCR_TSEG2(bt->phase_seg2 - 1);
> +	rcar_can_writel(priv, RCAR_CAN_BCR, bcr);
> +	rcar_can_writeb(priv, RCAR_CAN_CLKR, clkr);
> +	return 0;
> +}
> +
> +static void rcar_can_start(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr, n;
> +
> +	/* Set controller to known mode:
> +	 * - normal mailbox mode (no FIFO);

Does the controller support FIFO?

> +	 * - accept all messages (no filter).
> +	 * CAN is in sleep mode after MCU hardware or software reset.
> +	 */
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	ctlr &= ~CTLR_SLPM;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	/* Go to reset mode */
> +	ctlr |= CTLR_FORCE_RESET;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	ctlr |= CTLR_IDFM_MIXED;	/* Select mixed ID mode */
> +	ctlr &= ~CTLR_TPM;		/* Set ID priority transmit mode */
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +
> +	rcar_can_writeb(priv, RCAR_CAN_CLKR, priv->clock_select);
> +
> +	/* Accept all SID and EID */
> +	for (n = 0; n < N_RX_MKREGS; n++)
> +		rcar_can_writel(priv, RCAR_CAN_MKR(n), 0);
> +	rcar_can_writel(priv, RCAR_CAN_MKIVLR0, 0);
> +
> +	rcar_can_set_bittiming(ndev);
> +
> +	/* Initial value of MIER1 undefined.  Mark all Tx mailboxes as free. */
> +	rcar_can_writel(priv, RCAR_CAN_MIER1, 0);
> +
> +	rcar_can_writeb(priv, RCAR_CAN_IER, IER_TXMIE | IER_ERSIE | IER_RXM1IE);
> +
> +	/* Accumulate error codes */
> +	rcar_can_writeb(priv, RCAR_CAN_ECSR, ECSR_EDPM);
> +	/* Enable error interrupts */
> +	rcar_can_writeb(priv, RCAR_CAN_EIER,
> +			EIER_EWIE | EIER_EPIE | EIER_BOEIE | EIER_BEIE |
> +			EIER_ORIE | EIER_OLIE);
> +	/* Enable interrupts for RX mailboxes */
> +	rcar_can_writel(priv, RCAR_CAN_MIER0, RX_MBX_MASK);
> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> +	/* Write to the CiMCTLj register in CAN
> +	 * operation mode or CAN halt mode.
> +	 * Configure mailboxes 0-31 as Rx mailboxes.
> +	 * Configure mailboxes 32-63 as Tx mailboxes.
> +	 */
> +	/* Go to halt mode */
> +	ctlr |= CTLR_HALT;
> +	ctlr &= ~CTLR_RESET;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	for (n = 0; n < FIRST_TX_MB; n++) {
> +		/* According to documentation we should clear MCTL
> +		 * register before configuring mailbox.
> +		 */
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(n), 0);
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(n), MCTL_RECREQ);
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(FIRST_TX_MB + n), 0);
> +	}
> +	/* Go to operation mode */
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr & ~CTLR_FORCE_RESET);
> +}
> +
> +static int rcar_can_open(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	int err;
> +
> +	err = open_candev(ndev);
> +	if (err) {
> +		netdev_err(ndev, "open_candev() failed %d\n", err);
> +		goto out;
> +	}
> +	napi_enable(&priv->napi);
> +	err = request_irq(ndev->irq, rcar_can_interrupt, 0, ndev->name, ndev);
> +	if (err) {
> +		netdev_err(ndev, "error requesting interrupt %x\n", ndev->irq);
> +		goto out_close;
> +	}
> +	can_led_event(ndev, CAN_LED_EVENT_OPEN);
> +	rcar_can_start(ndev);
> +	netif_start_queue(ndev);
> +	return 0;
> +out_close:
> +	napi_disable(&priv->napi);
> +	close_candev(ndev);
> +out:
> +	return err;
> +}
> +
> +static void rcar_can_stop(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr;
> +
> +	/* Go to (force) reset mode */
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr | CTLR_FORCE_RESET);
> +	rcar_can_writel(priv, RCAR_CAN_MIER0, 0);
> +	rcar_can_writel(priv, RCAR_CAN_MIER1, 0);
> +	rcar_can_writeb(priv, RCAR_CAN_IER, 0);
> +	rcar_can_writeb(priv, RCAR_CAN_EIER, 0);
> +	priv->can.state = CAN_STATE_STOPPED;
> +}
> +
> +static int rcar_can_close(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +
> +	netif_stop_queue(ndev);
> +	rcar_can_stop(ndev);
> +	free_irq(ndev->irq, ndev);
> +	napi_disable(&priv->napi);
> +	close_candev(ndev);
> +	can_led_event(ndev, CAN_LED_EVENT_STOP);
> +	return 0;
> +}
> +
> +static netdev_tx_t rcar_can_start_xmit(struct sk_buff *skb,
> +				       struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	struct can_frame *cf = (struct can_frame *)skb->data;
> +	u32 data, mier1, mbxno, i;
> +	unsigned long flags;
> +	u8 mctl;
> +
> +	if (can_dropped_invalid_skb(ndev, skb))
> +		return NETDEV_TX_OK;
> +
> +	spin_lock_irqsave(&priv->mier_lock, flags);
> +	mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
> +	if (unlikely(mier1 == ~0U)) {
> +		spin_unlock_irqrestore(&priv->mier_lock, flags);
> +		netif_stop_queue(ndev);
> +		return NETDEV_TX_BUSY;
> +	}
> +	rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 | (mier1 + 1));
> +	spin_unlock_irqrestore(&priv->mier_lock, flags);
> +	mbxno = ffz(mier1) + FIRST_TX_MB;

This smells fishy, for several reasons:
The driver should guarantee that the frames are send in the same order
as this function is called. If you pick the first free mailbox I suspect
the hardware send the first mailbox ready to send. I don't know the
hardware, but several CAN controllers I've worked with, function in that
way.
Then you should rethink your flow control. You should call
netif_stop_queue if all your hardware buffers are full, then the above
NETDEV_TX_BUSY should not happen.

> +
> +	if (cf->can_id & CAN_EFF_FLAG) {
> +		/* Extended frame format */
> +		data = (cf->can_id & CAN_EFF_MASK) | RCAR_CAN_IDE;
> +	} else {
> +		/* Standard frame format */
> +		data = (cf->can_id & CAN_SFF_MASK) << RCAR_CAN_SID_SHIFT;
> +	}
> +	if (cf->can_id & CAN_RTR_FLAG) {
> +		/* Remote transmission request */
> +		data |= RCAR_CAN_RTR;
> +	}
> +	rcar_can_mbx_writel(priv, mbxno, MBX_HDR_OFFSET, data);
> +
> +	rcar_can_mbx_writeb(priv, mbxno, MBX_DLC_OFFSET, cf->can_dlc);
> +
> +	for (i = 0; i < cf->can_dlc; i++)
> +		rcar_can_mbx_writeb(priv, mbxno,
> +				    MBX_DATA_OFFSET + i, cf->data[i]);
> +
> +	can_put_echo_skb(skb, ndev, mbxno - FIRST_TX_MB);
> +
> +	rcar_can_writeb(priv, RCAR_CAN_IER,
> +			rcar_can_readb(priv, RCAR_CAN_IER) | IER_TXMIE);
> +
> +	mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbxno));

Do you have to read mctl here?

> +	if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> +		mctl |= MCTL_ONESHOT;
> +	else
> +		mctl &= ~MCTL_ONESHOT;
> +	/* Start TX */
> +	mctl |= MCTL_TRMREQ;
> +	rcar_can_writeb(priv, RCAR_CAN_MCTL(mbxno), mctl);
> +	return NETDEV_TX_OK;
> +}
> +
> +static const struct net_device_ops rcar_can_netdev_ops = {
> +	.ndo_open = rcar_can_open,
> +	.ndo_stop = rcar_can_close,
> +	.ndo_start_xmit = rcar_can_start_xmit,
> +};
> +
> +static void rcar_can_rx_pkt(struct rcar_can_priv *priv, int mbx)
> +{
> +	struct net_device_stats *stats = &priv->ndev->stats;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	u32 data;
> +	u8 dlc;
> +
> +	skb = alloc_can_skb(priv->ndev, &cf);
> +	if (!skb) {
> +		stats->rx_dropped++;
> +		if (printk_ratelimit())
> +			netdev_err(priv->ndev,
> +				   "%s: alloc_can_skb() failed\n", __func__);
> +		return;
> +	}
> +
> +	data = rcar_can_mbx_readl(priv, mbx, MBX_HDR_OFFSET);
> +	if (data & RCAR_CAN_IDE)
> +		cf->can_id = (data & CAN_EFF_MASK) | CAN_EFF_FLAG;
> +	else
> +		cf->can_id = (data >> RCAR_CAN_SID_SHIFT) & CAN_SFF_MASK;
> +	if (data & RCAR_CAN_RTR)
> +		cf->can_id |= CAN_RTR_FLAG;
> +
> +	dlc = rcar_can_mbx_readb(priv, mbx, MBX_DLC_OFFSET);
> +	cf->can_dlc = get_can_dlc(dlc);

Please don't copy data on received RTR frames.

> +	for (dlc = 0; dlc < cf->can_dlc; dlc++)
> +		cf->data[dlc] = rcar_can_mbx_readb(priv, mbx,
> +						   MBX_DATA_OFFSET + dlc);
> +
> +	can_led_event(priv->ndev, CAN_LED_EVENT_RX);
> +
> +	netif_receive_skb(skb);
> +	stats->rx_bytes += cf->can_dlc;
> +	stats->rx_packets++;
> +}
> +
> +static int rcar_can_rx_poll(struct napi_struct *napi, int quota)
> +{
> +	struct rcar_can_priv *priv = container_of(napi,
> +						  struct rcar_can_priv, napi);
> +	u32 num_pkts = 0;

please make it an int
> +
> +	/* Find mailbox */
> +	while (1) {

please put the quota check into the while()

> +		u8 mctl, mbx;
> +
> +		mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
> +		if (mbx & MSSR_SEST || num_pkts >= quota)
> +			break;
> +		mbx &= MSSR_MBNST;
> +		mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
> +		/* Clear interrupt */
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx),
> +				mctl & ~MCTL_NEWDATA);
> +		rcar_can_rx_pkt(priv, mbx);

Which instruction reenables the mailbox?

> +		++num_pkts;
> +	}
> +	/* All packets processed */
> +	if (num_pkts < quota) {
> +		u8 ier;
> +
> +		napi_complete(napi);
> +		ier = rcar_can_readb(priv, RCAR_CAN_IER);
> +		rcar_can_writeb(priv, RCAR_CAN_IER, ier | IER_RXM1IE);

I the hardware doesn't modify the IER register, you can work on a shadow
copy and only write, but never need to read from the hardware.

> +	}
> +	return num_pkts;
> +}
> +
> +static int rcar_can_do_set_mode(struct net_device *ndev, enum can_mode mode)
> +{
> +	switch (mode) {
> +	case CAN_MODE_START:
> +		rcar_can_start(ndev);
> +		netif_wake_queue(ndev);
> +		return 0;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int rcar_can_get_berr_counter(const struct net_device *dev,
> +				     struct can_berr_counter *bec)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(dev);
> +
> +	bec->txerr = rcar_can_readb(priv, RCAR_CAN_TECR);
> +	bec->rxerr = rcar_can_readb(priv, RCAR_CAN_RECR);
> +	return 0;
> +}
> +
> +static int rcar_can_probe(struct platform_device *pdev)
> +{
> +	struct rcar_can_platform_data *pdata;
> +	struct rcar_can_priv *priv;
> +	struct net_device *ndev;
> +	struct resource *mem;
> +	void __iomem *addr;
> +	int err = -ENODEV;
> +	int irq;
> +
> +	pdata = pdev->dev.platform_data;

please use dev_get_platdata()

> +	if (!pdata) {
> +		dev_err(&pdev->dev, "No platform data provided!\n");
> +		goto fail;
> +	}
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (!irq) {
> +		dev_err(&pdev->dev, "No IRQ resource\n");
> +		goto fail;
> +	}
> +
> +	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;
> +	}
> +
> +	ndev = alloc_candev(sizeof(struct rcar_can_priv), N_MBX - FIRST_TX_MB);
> +	if (!ndev) {
> +		dev_err(&pdev->dev, "alloc_candev failed\n");
> +		err = -ENOMEM;
> +		goto fail;
> +	}
> +
> +	priv = netdev_priv(ndev);
> +
> +	priv->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(priv->clk)) {
> +		err = PTR_ERR(priv->clk);
> +		dev_err(&pdev->dev, "cannot get clock: %d\n", err);
> +		goto fail_clk;
> +	}
> +	clk_enable(priv->clk);

You are missing the clock's prepare step. You should call
clk_prepare_enable(). Please move clock_prepare_enable() to the open()
(and the disable_unprepare() to the close() function) so tha the core is
not powered as long as the interface is down. You might have to enable
the clock in the rcar_can_get_berr_counter() as it may be called if the
interface is still down.

> +
> +	ndev->netdev_ops = &rcar_can_netdev_ops;
> +	ndev->irq = irq;
> +	ndev->flags |= IFF_ECHO;
> +	priv->ndev = ndev;
> +	priv->reg_base = addr;
> +	priv->clock_select = pdata->clock_select;
> +	priv->can.clock.freq = clk_get_rate(priv->clk);
> +	priv->can.bittiming_const = &rcar_can_bittiming_const;
> +	priv->can.do_set_bittiming = rcar_can_set_bittiming;

No need to set this callback, as you're calling rcar_can_set_bittiming
explizidly.

> +	priv->can.do_set_mode = rcar_can_do_set_mode;
> +	priv->can.do_get_berr_counter = rcar_can_get_berr_counter;
> +	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES |
> +				       CAN_CTRLMODE_ONE_SHOT;

You don't handle 3_SAMPLES. Have you tested your driver in ONE_SHOT
mode? How does the controller react if a frame cannot be send? Do you
clear the skb that has previously been can_put_echo_skb()?


> +	platform_set_drvdata(pdev, ndev);
> +	SET_NETDEV_DEV(ndev, &pdev->dev);
> +	spin_lock_init(&priv->mier_lock);
> +
> +	netif_napi_add(ndev, &priv->napi, rcar_can_rx_poll,
> +		       RCAR_CAN_NAPI_WEIGHT);
> +	err = register_candev(ndev);
> +	if (err) {
> +		dev_err(&pdev->dev, "register_candev() failed\n");
> +		goto fail_candev;
> +	}
> +
> +	devm_can_led_init(ndev);
> +
> +	dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%u)\n",
> +		 priv->reg_base, ndev->irq);
> +
> +	return 0;
> +fail_candev:
> +	netif_napi_del(&priv->napi);
> +fail_clk:
> +	free_candev(ndev);
> +fail:
> +	return err;
> +}
> +
> +static int rcar_can_remove(struct platform_device *pdev)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr;
> +
> +	unregister_candev(ndev);
> +	netif_napi_del(&priv->napi);
> +	/* Go to sleep mode */
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr | CTLR_SLPM);

You should put the controller in to sleep mode in close()

> +	clk_disable(priv->clk);
> +	free_candev(ndev);
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int rcar_can_suspend(struct device *dev)
> +{
> +	struct net_device *ndev = dev_get_drvdata(dev);
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr;
> +
> +	if (netif_running(ndev)) {
> +		netif_stop_queue(ndev);
> +		netif_device_detach(ndev);
> +	}
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	ctlr |= CTLR_HALT;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	ctlr |= CTLR_SLPM;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	priv->can.state = CAN_STATE_SLEEPING;
> +
> +	clk_disable(priv->clk);
> +	return 0;
> +}
> +
> +static int rcar_can_resume(struct device *dev)
> +{
> +	struct net_device *ndev = dev_get_drvdata(dev);
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr;
> +
> +	clk_enable(priv->clk);
> +
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	ctlr &= ~CTLR_SLPM;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	ctlr &= ~CTLR_FORCE_RESET;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> +	if (netif_running(ndev)) {
> +		netif_device_attach(ndev);
> +		netif_start_queue(ndev);
> +	}
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(rcar_can_pm_ops, rcar_can_suspend, rcar_can_resume);
> +
> +static struct platform_driver rcar_can_driver = {
> +	.driver = {
> +		.name = DRV_NAME,
> +		.owner = THIS_MODULE,
> +		.pm = &rcar_can_pm_ops,
> +	},
> +	.probe = rcar_can_probe,
> +	.remove = rcar_can_remove,
> +};
> +
> +module_platform_driver(rcar_can_driver);
> +
> +MODULE_AUTHOR("Cogent Embedded, Inc.");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("CAN driver for Renesas R-Car SoC");
> +MODULE_ALIAS("platform:" DRV_NAME);
> Index: linux-can-next/include/linux/can/platform/rcar_can.h
> ===================================================================
> --- /dev/null
> +++ linux-can-next/include/linux/can/platform/rcar_can.h
> @@ -0,0 +1,15 @@
> +#ifndef _CAN_PLATFORM_RCAR_CAN_H_
> +#define _CAN_PLATFORM_RCAR_CAN_H_
> +
> +#include <linux/types.h>
> +
> +/* Clock Select Register settings */
> +#define CLKR_CLKEXT	3	/* Externally input clock */
> +#define CLKR_CLKP2	1	/* Peripheral clock (clkp2) */
> +#define CLKR_CLKP1	0	/* Peripheral clock (clkp1) */

Can this be handled by the clock framework and or Device Tree?

> +
> +struct rcar_can_platform_data {
> +	u8 clock_select;	/* Clock source select */
> +};
> +
> +#endif	/* !_CAN_PLATFORM_RCAR_CAN_H_ */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Marc

Wolfgang Grandegger Oct. 2, 2013, 6:09 a.m. UTC | #5

Hallo Sergei,

thanks for your contribution. The patch looks already quite good. Before
I find time for a detailed review could you please check error handling
and bus-off recovery by reporting the output of "$ candump -td -e
any,0:0,#FFFFFFFF" while sending messages to the device ...

1. ... without cable connected
2. ... with short-circuited CAN high and low and doing some time later
       a manual recovery with "ip link set can0 type can restart"

I also wonder if the messages are always sent in order. You could use
the program "canfdtest" [1] from the can-utils for validation.

Support for CAN_CTRLMODE_BERR_REPORTING would also be nice, especially
if the bus errors reporting can easily be switched off (via interrupt mask).

There is also the RCAN-TL1 (Renesas CAN Time Trigger Level 1) module
e.g. available on the SH7203 Group of processors. Is this CAN controller
compatible to some extend with the R-Car CAN or a completely different
implementation?


[1] https://gitorious.org/linux-can/can-utils/source/canfdtest.c


Wolfgang.

On 09/28/2013 12:11 AM, Sergei Shtylyov wrote:
> Add support for the CAN controller found in Renesas R-Car SoCs. 
> 
> Signed-off-by: Sergei Shtylyov <[e-mail]>
> 
> ---
> The patch is against the 'linux-can-next.git' repo.
> 
>  drivers/net/can/Kconfig               |    9 
>  drivers/net/can/Makefile              |    1 
>  drivers/net/can/rcar_can.c            |  898 ++++++++++++++++++++++++++++++++++
>  include/linux/can/platform/rcar_can.h |   15 
>  4 files changed, 923 insertions(+)
> 
> Index: linux-can-next/drivers/net/can/Kconfig
> ===================================================================
> --- linux-can-next.orig/drivers/net/can/Kconfig
> +++ linux-can-next/drivers/net/can/Kconfig
> @@ -125,6 +125,15 @@ config CAN_GRCAN
>  	  endian syntheses of the cores would need some modifications on
>  	  the hardware level to work.
>  
> +config CAN_RCAR
> +	tristate "Renesas R-Car CAN controller"
> +	---help---
> +	  Say Y here if you want to use CAN controller found on Renesas R-Car
> +	  SoCs.
> +
> +	  To compile this driver as a module, choose M here: the module will
> +	  be called rcar_can.
> +
>  source "drivers/net/can/mscan/Kconfig"
>  
>  source "drivers/net/can/sja1000/Kconfig"
> Index: linux-can-next/drivers/net/can/Makefile
> ===================================================================
> --- linux-can-next.orig/drivers/net/can/Makefile
> +++ linux-can-next/drivers/net/can/Makefile
> @@ -25,5 +25,6 @@ obj-$(CONFIG_CAN_JANZ_ICAN3)	+= janz-ica
>  obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
>  obj-$(CONFIG_PCH_CAN)		+= pch_can.o
>  obj-$(CONFIG_CAN_GRCAN)		+= grcan.o
> +obj-$(CONFIG_CAN_RCAR)		+= rcar_can.o
>  
>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> Index: linux-can-next/drivers/net/can/rcar_can.c
> ===================================================================
> --- /dev/null
> +++ linux-can-next/drivers/net/can/rcar_can.c
> @@ -0,0 +1,898 @@
> +/*
> + * Renesas R-Car CAN device driver
> + *
> + * Copyright (C) 2013 Cogent Embedded, Inc. <[e-mail]>
> + * Copyright (C) 2013 Renesas Solutions 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/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/can/platform/rcar_can.h>
> +
> +#define DRV_NAME	"rcar_can"
> +
> +#define RCAR_CAN_MIER1	0x42C	/* CANi Mailbox Interrupt Enable Register 1 */
> +#define RCAR_CAN_MKR(n)	((n) < 2 ? 0x430 + 4 * (n) : 0x400 + 4 * ((n) - 2))
> +				/* CANi Mask Register */
> +#define RCAR_CAN_MKIVLR0 0x438	/* CANi Mask Invalid Register 0 */
> +#define RCAR_CAN_MIER0	 0x43C	/* CANi Mailbox Interrupt Enable Register 0 */
> +
> +#define RCAR_CAN_MCTL(n) (0x800 + (n)) /* CANi Message Control Register */
> +#define RCAR_CAN_CTLR	0x840	/* CANi Control Register */
> +#define RCAR_CAN_STR	0x842	/* CANi Status Register */
> +#define RCAR_CAN_BCR	0x844	/* CANi Bit Configuration Register */
> +#define RCAR_CAN_CLKR	0x847	/* CANi Clock Select Register */
> +#define RCAR_CAN_EIER	0x84C	/* CANi Error Interrupt Enable Register */
> +#define RCAR_CAN_EIFR	0x84D	/* CANi Err Interrupt Factor Judge Register */
> +#define RCAR_CAN_RECR	0x84E	/* CANi Receive Error Count Register */
> +#define RCAR_CAN_TECR	0x84F	/* CANi Transmit Error Count Register */
> +#define RCAR_CAN_ECSR	0x850	/* CANi Error Code Store Register */
> +#define RCAR_CAN_MSSR	0x852	/* CANi Mailbox Search Status Register */
> +#define RCAR_CAN_MSMR	0x853	/* CANi Mailbox Search Mode Register */
> +#define RCAR_CAN_TCR	0x858	/* CANi Test Control Register */
> +#define RCAR_CAN_IER	0x860	/* CANi Interrupt Enable Register */
> +#define RCAR_CAN_ISR	0x861	/* CANi Interrupt Status Register */
> +
> +/* Offsets of RCAR_CAN Mailbox Registers */
> +#define MBX_HDR_OFFSET	0x0
> +#define MBX_DLC_OFFSET	0x5
> +#define MBX_DATA_OFFSET	0x6
> +
> +#define RCAR_CAN_MBX_SIZE 0x10
> +
> +/* Control Register bits */
> +#define CTLR_SLPM	BIT(10)
> +#define CTLR_HALT	BIT(9)
> +#define CTLR_RESET	BIT(8)
> +#define CTLR_FORCE_RESET (3 << 8)
> +#define CTLR_TPM	BIT(4)	/* Transmission Priority Mode Select Bit */
> +#define CTLR_IDFM_MIXED	BIT(2)	/* Mixed ID mode */
> +
> +/* Message Control Register bits */
> +#define MCTL_TRMREQ	BIT(7)
> +#define MCTL_RECREQ	BIT(6)
> +#define MCTL_ONESHOT	BIT(4)
> +#define MCTL_SENTDATA	BIT(0)
> +#define MCTL_NEWDATA	BIT(0)
> +
> +#define N_RX_MKREGS	2	/* Number of mask registers */
> +				/* for Rx mailboxes 0-31 */
> +
> +/* Bit Configuration Register settings */
> +#define BCR_TSEG1(x)	(((x) & 0x0f) << 28)
> +#define BCR_BPR(x)	(((x) & 0x3ff) << 16)
> +#define BCR_SJW(x)	(((x) & 0x3) << 12)
> +#define BCR_TSEG2(x)	(((x) & 0x07) << 8)
> +
> +/* Mailbox and Mask Registers bits */
> +#define RCAR_CAN_IDE	BIT(31)
> +#define RCAR_CAN_RTR	BIT(30)
> +#define RCAR_CAN_SID_SHIFT 18
> +
> +/* Interrupt Enable Register bits */
> +#define IER_ERSIE	BIT(5)	/* Error (ERS) Interrupt Enable Bit */
> +#define IER_RXM0IE	BIT(2)	/* Mailbox 0 Successful Reception (RXM0) */
> +				/* Interrupt Enable Bit */
> +#define IER_RXM1IE	BIT(1)	/* Mailbox 1 Successful Reception (RXM0) */
> +				/* Interrupt Enable Bit */
> +#define IER_TXMIE	BIT(0)	/* Mailbox 32 to 63 Successful Tx */
> +				/* Interrupt Enable Bit */
> +
> +/* Interrupt Status Register bits */
> +#define ISR_ERSF	BIT(5)	/* Error (ERS) Interrupt Status Bit */
> +#define ISR_RXM0F	BIT(2)	/* Mailbox 0 Successful Reception (RXM0) */
> +				/* Interrupt Status Bit */
> +#define ISR_RXM1F	BIT(1)	/* Mailbox 1 to 63 Successful Reception */
> +				/* (RXM1) Interrupt Status Bit */
> +#define ISR_TXMF	BIT(0)	/* Mailbox 32 to 63 Successful Transmission */
> +				/* (TXM) Interrupt Status Bit */
> +
> +/* Error Interrupt Enable Register bits */
> +#define EIER_BLIE	BIT(7)	/* Bus Lock Interrupt Enable */
> +#define EIER_OLIE	BIT(6)	/* Overload Frame Transmit Interrupt Enable */
> +#define EIER_ORIE	BIT(5)	/* Receive Overrun Interrupt Enable */
> +#define EIER_BORIE	BIT(4)	/* Bus-Off Recovery Interrupt Enable */
> +
> +#define EIER_BOEIE	BIT(3)	/* Bus-Off Entry Interrupt Enable */
> +#define EIER_EPIE	BIT(2)	/* Error Passive Interrupt Enable */
> +#define EIER_EWIE	BIT(1)	/* Error Warning Interrupt Enable */
> +#define EIER_BEIE	BIT(0)	/* Bus Error Interrupt Enable */
> +
> +/* Error Interrupt Factor Judge Register bits */
> +#define EIFR_BLIF	BIT(7)	/* Bus Lock Detect Flag */
> +#define EIFR_OLIF	BIT(6)	/* Overload Frame Transmission Detect Flag */
> +#define EIFR_ORIF	BIT(5)	/* Receive Overrun Detect Flag */
> +#define EIFR_BORIF	BIT(4)	/* Bus-Off Recovery Detect Flag */
> +#define EIFR_BOEIF	BIT(3)	/* Bus-Off Entry Detect Flag */
> +#define EIFR_EPIF	BIT(2)	/* Error Passive Detect Flag */
> +#define EIFR_EWIF	BIT(1)	/* Error Warning Detect Flag */
> +#define EIFR_BEIF	BIT(0)	/* Bus Error Detect Flag */
> +
> +/* Error Code Store Register bits */
> +#define ECSR_EDPM	BIT(7)	/* Error Display Mode Select Bit */
> +#define ECSR_ADEF	BIT(6)	/* ACK Delimiter Error Flag */
> +#define ECSR_BE0F	BIT(5)	/* Bit Error (dominant) Flag */
> +#define ECSR_BE1F	BIT(4)	/* Bit Error (recessive) Flag */
> +#define ECSR_CEF	BIT(3)	/* CRC Error Flag */
> +#define ECSR_AEF	BIT(2)	/* ACK Error Flag */
> +#define ECSR_FEF	BIT(1)	/* Form Error Flag */
> +#define ECSR_SEF	BIT(0)	/* Stuff Error Flag */
> +
> +/* Mailbox Search Status Register bits */
> +#define MSSR_SEST	BIT(7)	/* Search Result Status Bit */
> +#define MSSR_MBNST	0x3f	/* Search Result Mailbox Number Status mask */
> +
> +/* Mailbox Search Mode Register values */
> +#define MSMR_TXMB	1	/* Transmit mailbox search mode */
> +#define MSMR_RXMB	0	/* Receive mailbox search mode */
> +
> +/* Mailbox configuration:
> + * mailbox 0 - not used
> + * mailbox 1-31 - Rx
> + * mailbox 32-63 - Tx
> + * no FIFO mailboxes
> + */
> +#define N_MBX		64
> +#define FIRST_TX_MB	32
> +#define RX_MBX_MASK	0xFFFFFFFE
> +
> +#define RCAR_CAN_NAPI_WEIGHT (FIRST_TX_MB - 1)
> +
> +struct rcar_can_priv {
> +	struct can_priv can;	/* Must be the first member! */
> +	struct net_device *ndev;
> +	struct napi_struct napi;
> +	void __iomem *reg_base;
> +	struct clk *clk;
> +	spinlock_t mier_lock;
> +	u8 clock_select;
> +};
> +
> +static const struct can_bittiming_const rcar_can_bittiming_const = {
> +	.name = DRV_NAME,
> +	.tseg1_min = 4,
> +	.tseg1_max = 16,
> +	.tseg2_min = 2,
> +	.tseg2_max = 8,
> +	.sjw_max = 4,
> +	.brp_min = 1,
> +	.brp_max = 1024,
> +	.brp_inc = 1,
> +};
> +
> +static inline u32 rcar_can_readl(struct rcar_can_priv *priv, int reg)
> +{
> +	return readl(priv->reg_base + reg);
> +}
> +
> +static inline u16 rcar_can_readw(struct rcar_can_priv *priv, int reg)
> +{
> +	return readw(priv->reg_base + reg);
> +}
> +
> +static inline u8 rcar_can_readb(struct rcar_can_priv *priv, int reg)
> +{
> +	return readb(priv->reg_base + reg);
> +}
> +
> +static inline void rcar_can_writel(struct rcar_can_priv *priv, int reg, u32 val)
> +{
> +	writel(val, priv->reg_base + reg);
> +}
> +
> +static inline void rcar_can_writew(struct rcar_can_priv *priv, int reg, u16 val)
> +{
> +	writew(val, priv->reg_base + reg);
> +}
> +
> +static inline void rcar_can_writeb(struct rcar_can_priv *priv, int reg, u8 val)
> +{
> +	writeb(val, priv->reg_base + reg);
> +}
> +
> +static inline u32 rcar_can_mbx_readl(struct rcar_can_priv *priv,
> +				     u32 mbxno, u8 offset)
> +{
> +	return rcar_can_readl(priv, RCAR_CAN_MBX_SIZE * mbxno + offset);
> +}
> +
> +static inline u8 rcar_can_mbx_readb(struct rcar_can_priv *priv,
> +				    u32 mbxno, u8 offset)
> +{
> +	return rcar_can_readb(priv, RCAR_CAN_MBX_SIZE * mbxno + offset);
> +}
> +
> +static inline void rcar_can_mbx_writel(struct rcar_can_priv *priv, u32 mbxno,
> +				       u8 offset, u32 val)
> +{
> +	rcar_can_writel(priv, RCAR_CAN_MBX_SIZE * mbxno + offset, val);
> +}
> +
> +static inline void rcar_can_mbx_writeb(struct rcar_can_priv *priv, u32 mbxno,
> +				       u8 offset, u8 val)
> +{
> +	rcar_can_writeb(priv, RCAR_CAN_MBX_SIZE * mbxno + offset, val);
> +}
> +
> +static void rcar_can_error(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	u8 eifr;
> +
> +	/* Propagate the error condition to the CAN stack */
> +	skb = alloc_can_err_skb(ndev, &cf);
> +	if (!skb) {
> +		if (printk_ratelimit())
> +			netdev_err(priv->ndev,
> +				   "%s: alloc_can_err_skb() failed\n",
> +				   __func__);
> +		return;
> +	}
> +
> +	eifr = rcar_can_readb(priv, RCAR_CAN_EIFR);
> +	if (eifr & EIFR_EWIF) {
> +		netdev_dbg(priv->ndev, "Error warning interrupt\n");
> +		priv->can.state = CAN_STATE_ERROR_WARNING;
> +		priv->can.can_stats.error_warning++;
> +		cf->can_id |= CAN_ERR_CRTL;
> +		if (rcar_can_readb(priv, RCAR_CAN_TECR) > 96)
> +			cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
> +		if (rcar_can_readb(priv, RCAR_CAN_RECR) > 96)
> +			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> +		/* Clear interrupt condition */
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EWIF);
> +	}
> +	if (eifr & EIFR_EPIF) {
> +		netdev_dbg(priv->ndev, "Error passive interrupt\n");
> +		priv->can.state = CAN_STATE_ERROR_PASSIVE;
> +		priv->can.can_stats.error_passive++;
> +		cf->can_id |= CAN_ERR_CRTL;
> +		if (rcar_can_readb(priv, RCAR_CAN_TECR) > 127)
> +			cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
> +		if (rcar_can_readb(priv, RCAR_CAN_RECR) > 127)
> +			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
> +		/* Clear interrupt condition */
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EPIF);
> +	}
> +	if (eifr & EIFR_BOEIF) {
> +		netdev_dbg(priv->ndev, "Bus-off entry interrupt\n");
> +		priv->can.state = CAN_STATE_BUS_OFF;
> +		cf->can_id |= CAN_ERR_BUSOFF;
> +		/* Clear interrupt condition */
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BOEIF);
> +		/* Disable all interrupts in bus-off to avoid int hog */
> +		rcar_can_writeb(priv, RCAR_CAN_EIER, 0);
> +		rcar_can_writeb(priv, RCAR_CAN_IER, 0);
> +		can_bus_off(ndev);
> +	}
> +	if (eifr & EIFR_BEIF) {
> +		int rx_errors = 0, tx_errors = 0, bus_errors = 0;
> +		u8 ecsr;
> +
> +		netdev_dbg(priv->ndev, "Bus error interrupt:\n");
> +		cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
> +		cf->data[2] = CAN_ERR_PROT_UNSPEC;
> +
> +		ecsr = rcar_can_readb(priv, RCAR_CAN_ECSR);
> +		if (ecsr & ECSR_ADEF) {
> +			netdev_dbg(priv->ndev, "ACK Delimiter Error\n");
> +			cf->data[3] |= CAN_ERR_PROT_LOC_ACK_DEL;
> +			bus_errors++;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_ADEF);
> +		}
> +		if (ecsr & ECSR_BE0F) {
> +			netdev_dbg(priv->ndev, "Bit Error (dominant)\n");
> +			cf->data[2] |= CAN_ERR_PROT_BIT0;
> +			bus_errors++;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_BE0F);
> +		}
> +		if (ecsr & ECSR_BE1F) {
> +			netdev_dbg(priv->ndev, "Bit Error (recessive)\n");
> +			cf->data[2] |= CAN_ERR_PROT_BIT1;
> +			bus_errors++;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_BE1F);
> +		}
> +		if (ecsr & ECSR_CEF) {
> +			netdev_dbg(priv->ndev, "CRC Error\n");
> +			cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
> +			bus_errors++;
> +			rx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_CEF);
> +		}
> +		if (ecsr & ECSR_AEF) {
> +			netdev_dbg(priv->ndev, "ACK Error\n");
> +			cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
> +			bus_errors++;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_AEF);
> +		}
> +		if (ecsr & ECSR_FEF) {
> +			netdev_dbg(priv->ndev, "Form Error\n");
> +			cf->data[2] |= CAN_ERR_PROT_FORM;
> +			bus_errors++;
> +			rx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_FEF);
> +		}
> +		if (ecsr & ECSR_SEF) {
> +			netdev_dbg(priv->ndev, "Stuff Error\n");
> +			cf->data[2] |= CAN_ERR_PROT_STUFF;
> +			bus_errors++;
> +			rx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_SEF);
> +		}
> +
> +		priv->can.can_stats.bus_error += bus_errors;
> +		ndev->stats.rx_errors += rx_errors;
> +		ndev->stats.tx_errors += tx_errors;
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BEIF);
> +	}
> +	if (eifr & EIFR_ORIF) {
> +		netdev_dbg(priv->ndev, "Receive overrun error interrupt\n");
> +		cf->can_id |= CAN_ERR_CRTL;
> +		cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> +		ndev->stats.rx_over_errors++;
> +		ndev->stats.rx_errors++;
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_ORIF);
> +	}
> +	if (eifr & EIFR_OLIF) {
> +		netdev_dbg(priv->ndev,
> +			   "Overload Frame Transmission error interrupt\n");
> +		cf->can_id |= CAN_ERR_PROT;
> +		cf->data[2] |= CAN_ERR_PROT_OVERLOAD;
> +		ndev->stats.rx_over_errors++;
> +		ndev->stats.rx_errors++;
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_OLIF);
> +	}
> +
> +	netif_rx(skb);
> +	stats->rx_packets++;
> +	stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static irqreturn_t rcar_can_interrupt(int irq, void *dev_id)
> +{
> +	struct net_device *ndev = (struct net_device *)dev_id;
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	u8 isr;
> +
> +	isr = rcar_can_readb(priv, RCAR_CAN_ISR);
> +	if (isr & ISR_ERSF)
> +		rcar_can_error(ndev);
> +
> +	if (isr & ISR_TXMF) {
> +		u32 ie_mask = 0;
> +
> +		/* Set Transmit Mailbox Search Mode */
> +		rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_TXMB);
> +		while (1) {
> +			u8 mctl, mbx;
> +
> +			mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
> +			if (mbx & MSSR_SEST)
> +				break;
> +			mbx &= MSSR_MBNST;
> +			mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
> +			/* Bits SENTDATA and TRMREQ cannot be
> +			 * set to 0 simultaneously
> +			 */
> +			mctl &= ~MCTL_TRMREQ;
> +			rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
> +			mctl &= ~MCTL_SENTDATA;
> +			/* Clear interrupt */
> +			rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
> +			ie_mask |= BIT(mbx - FIRST_TX_MB);
> +			stats->tx_bytes += can_get_echo_skb(ndev,
> +							    mbx - FIRST_TX_MB);
> +			stats->tx_packets++;
> +			can_led_event(ndev, CAN_LED_EVENT_TX);
> +		}
> +		/* Set receive mailbox search mode */
> +		rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_RXMB);
> +		/* Disable mailbox interrupt, mark mailbox as free */
> +		if (ie_mask) {
> +			u32 mier1;
> +
> +			spin_lock(&priv->mier_lock);
> +			mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
> +			rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 & ~ie_mask);
> +			spin_unlock(&priv->mier_lock);
> +			if (unlikely(netif_queue_stopped(ndev)))
> +				netif_wake_queue(ndev);
> +		}
> +	}
> +	if (isr & ISR_RXM1F) {
> +		if (napi_schedule_prep(&priv->napi)) {
> +			/* Disable Rx interrupts */
> +			rcar_can_writeb(priv, RCAR_CAN_IER,
> +					rcar_can_readb(priv, RCAR_CAN_IER) &
> +						       ~IER_RXM1IE);
> +			__napi_schedule(&priv->napi);
> +		}
> +	}
> +	return IRQ_HANDLED;
> +}
> +
> +static int rcar_can_set_bittiming(struct net_device *dev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(dev);
> +	struct can_bittiming *bt = &priv->can.bittiming;
> +	u32 bcr;
> +	u16 ctlr;
> +	u8 clkr;
> +
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	if (ctlr & CTLR_SLPM) {
> +		/* Write to BCR in CAN reset mode or CAN halt mode */
> +		return -EBUSY;
> +	}
> +	/* Don't overwrite CLKR with 32-bit BCR access */
> +	/* CLKR has 8-bit access */
> +	clkr = rcar_can_readb(priv, RCAR_CAN_CLKR);
> +	bcr = BCR_TSEG1(bt->phase_seg1 + bt->prop_seg - 1) |
> +	      BCR_BPR(bt->brp - 1) | BCR_SJW(bt->sjw - 1) |
> +	      BCR_TSEG2(bt->phase_seg2 - 1);
> +	rcar_can_writel(priv, RCAR_CAN_BCR, bcr);
> +	rcar_can_writeb(priv, RCAR_CAN_CLKR, clkr);
> +	return 0;
> +}
> +
> +static void rcar_can_start(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr, n;
> +
> +	/* Set controller to known mode:
> +	 * - normal mailbox mode (no FIFO);
> +	 * - accept all messages (no filter).
> +	 * CAN is in sleep mode after MCU hardware or software reset.
> +	 */
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	ctlr &= ~CTLR_SLPM;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	/* Go to reset mode */
> +	ctlr |= CTLR_FORCE_RESET;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	ctlr |= CTLR_IDFM_MIXED;	/* Select mixed ID mode */
> +	ctlr &= ~CTLR_TPM;		/* Set ID priority transmit mode */
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +
> +	rcar_can_writeb(priv, RCAR_CAN_CLKR, priv->clock_select);
> +
> +	/* Accept all SID and EID */
> +	for (n = 0; n < N_RX_MKREGS; n++)
> +		rcar_can_writel(priv, RCAR_CAN_MKR(n), 0);
> +	rcar_can_writel(priv, RCAR_CAN_MKIVLR0, 0);
> +
> +	rcar_can_set_bittiming(ndev);
> +
> +	/* Initial value of MIER1 undefined.  Mark all Tx mailboxes as free. */
> +	rcar_can_writel(priv, RCAR_CAN_MIER1, 0);
> +
> +	rcar_can_writeb(priv, RCAR_CAN_IER, IER_TXMIE | IER_ERSIE | IER_RXM1IE);
> +
> +	/* Accumulate error codes */
> +	rcar_can_writeb(priv, RCAR_CAN_ECSR, ECSR_EDPM);
> +	/* Enable error interrupts */
> +	rcar_can_writeb(priv, RCAR_CAN_EIER,
> +			EIER_EWIE | EIER_EPIE | EIER_BOEIE | EIER_BEIE |
> +			EIER_ORIE | EIER_OLIE);
> +	/* Enable interrupts for RX mailboxes */
> +	rcar_can_writel(priv, RCAR_CAN_MIER0, RX_MBX_MASK);
> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> +	/* Write to the CiMCTLj register in CAN
> +	 * operation mode or CAN halt mode.
> +	 * Configure mailboxes 0-31 as Rx mailboxes.
> +	 * Configure mailboxes 32-63 as Tx mailboxes.
> +	 */
> +	/* Go to halt mode */
> +	ctlr |= CTLR_HALT;
> +	ctlr &= ~CTLR_RESET;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	for (n = 0; n < FIRST_TX_MB; n++) {
> +		/* According to documentation we should clear MCTL
> +		 * register before configuring mailbox.
> +		 */
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(n), 0);
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(n), MCTL_RECREQ);
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(FIRST_TX_MB + n), 0);
> +	}
> +	/* Go to operation mode */
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr & ~CTLR_FORCE_RESET);
> +}
> +
> +static int rcar_can_open(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	int err;
> +
> +	err = open_candev(ndev);
> +	if (err) {
> +		netdev_err(ndev, "open_candev() failed %d\n", err);
> +		goto out;
> +	}
> +	napi_enable(&priv->napi);
> +	err = request_irq(ndev->irq, rcar_can_interrupt, 0, ndev->name, ndev);
> +	if (err) {
> +		netdev_err(ndev, "error requesting interrupt %x\n", ndev->irq);
> +		goto out_close;
> +	}
> +	can_led_event(ndev, CAN_LED_EVENT_OPEN);
> +	rcar_can_start(ndev);
> +	netif_start_queue(ndev);
> +	return 0;
> +out_close:
> +	napi_disable(&priv->napi);
> +	close_candev(ndev);
> +out:
> +	return err;
> +}
> +
> +static void rcar_can_stop(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr;
> +
> +	/* Go to (force) reset mode */
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr | CTLR_FORCE_RESET);
> +	rcar_can_writel(priv, RCAR_CAN_MIER0, 0);
> +	rcar_can_writel(priv, RCAR_CAN_MIER1, 0);
> +	rcar_can_writeb(priv, RCAR_CAN_IER, 0);
> +	rcar_can_writeb(priv, RCAR_CAN_EIER, 0);
> +	priv->can.state = CAN_STATE_STOPPED;
> +}
> +
> +static int rcar_can_close(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +
> +	netif_stop_queue(ndev);
> +	rcar_can_stop(ndev);
> +	free_irq(ndev->irq, ndev);
> +	napi_disable(&priv->napi);
> +	close_candev(ndev);
> +	can_led_event(ndev, CAN_LED_EVENT_STOP);
> +	return 0;
> +}
> +
> +static netdev_tx_t rcar_can_start_xmit(struct sk_buff *skb,
> +				       struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	struct can_frame *cf = (struct can_frame *)skb->data;
> +	u32 data, mier1, mbxno, i;
> +	unsigned long flags;
> +	u8 mctl;
> +
> +	if (can_dropped_invalid_skb(ndev, skb))
> +		return NETDEV_TX_OK;
> +
> +	spin_lock_irqsave(&priv->mier_lock, flags);
> +	mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
> +	if (unlikely(mier1 == ~0U)) {
> +		spin_unlock_irqrestore(&priv->mier_lock, flags);
> +		netif_stop_queue(ndev);
> +		return NETDEV_TX_BUSY;
> +	}
> +	rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 | (mier1 + 1));
> +	spin_unlock_irqrestore(&priv->mier_lock, flags);
> +	mbxno = ffz(mier1) + FIRST_TX_MB;
> +
> +	if (cf->can_id & CAN_EFF_FLAG) {
> +		/* Extended frame format */
> +		data = (cf->can_id & CAN_EFF_MASK) | RCAR_CAN_IDE;
> +	} else {
> +		/* Standard frame format */
> +		data = (cf->can_id & CAN_SFF_MASK) << RCAR_CAN_SID_SHIFT;
> +	}
> +	if (cf->can_id & CAN_RTR_FLAG) {
> +		/* Remote transmission request */
> +		data |= RCAR_CAN_RTR;
> +	}
> +	rcar_can_mbx_writel(priv, mbxno, MBX_HDR_OFFSET, data);
> +
> +	rcar_can_mbx_writeb(priv, mbxno, MBX_DLC_OFFSET, cf->can_dlc);
> +
> +	for (i = 0; i < cf->can_dlc; i++)
> +		rcar_can_mbx_writeb(priv, mbxno,
> +				    MBX_DATA_OFFSET + i, cf->data[i]);
> +
> +	can_put_echo_skb(skb, ndev, mbxno - FIRST_TX_MB);
> +
> +	rcar_can_writeb(priv, RCAR_CAN_IER,
> +			rcar_can_readb(priv, RCAR_CAN_IER) | IER_TXMIE);
> +
> +	mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbxno));
> +	if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> +		mctl |= MCTL_ONESHOT;
> +	else
> +		mctl &= ~MCTL_ONESHOT;
> +	/* Start TX */
> +	mctl |= MCTL_TRMREQ;
> +	rcar_can_writeb(priv, RCAR_CAN_MCTL(mbxno), mctl);
> +	return NETDEV_TX_OK;
> +}
> +
> +static const struct net_device_ops rcar_can_netdev_ops = {
> +	.ndo_open = rcar_can_open,
> +	.ndo_stop = rcar_can_close,
> +	.ndo_start_xmit = rcar_can_start_xmit,
> +};
> +
> +static void rcar_can_rx_pkt(struct rcar_can_priv *priv, int mbx)
> +{
> +	struct net_device_stats *stats = &priv->ndev->stats;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	u32 data;
> +	u8 dlc;
> +
> +	skb = alloc_can_skb(priv->ndev, &cf);
> +	if (!skb) {
> +		stats->rx_dropped++;
> +		if (printk_ratelimit())
> +			netdev_err(priv->ndev,
> +				   "%s: alloc_can_skb() failed\n", __func__);
> +		return;
> +	}
> +
> +	data = rcar_can_mbx_readl(priv, mbx, MBX_HDR_OFFSET);
> +	if (data & RCAR_CAN_IDE)
> +		cf->can_id = (data & CAN_EFF_MASK) | CAN_EFF_FLAG;
> +	else
> +		cf->can_id = (data >> RCAR_CAN_SID_SHIFT) & CAN_SFF_MASK;
> +	if (data & RCAR_CAN_RTR)
> +		cf->can_id |= CAN_RTR_FLAG;
> +
> +	dlc = rcar_can_mbx_readb(priv, mbx, MBX_DLC_OFFSET);
> +	cf->can_dlc = get_can_dlc(dlc);
> +	for (dlc = 0; dlc < cf->can_dlc; dlc++)
> +		cf->data[dlc] = rcar_can_mbx_readb(priv, mbx,
> +						   MBX_DATA_OFFSET + dlc);
> +
> +	can_led_event(priv->ndev, CAN_LED_EVENT_RX);
> +
> +	netif_receive_skb(skb);
> +	stats->rx_bytes += cf->can_dlc;
> +	stats->rx_packets++;
> +}
> +
> +static int rcar_can_rx_poll(struct napi_struct *napi, int quota)
> +{
> +	struct rcar_can_priv *priv = container_of(napi,
> +						  struct rcar_can_priv, napi);
> +	u32 num_pkts = 0;
> +
> +	/* Find mailbox */
> +	while (1) {
> +		u8 mctl, mbx;
> +
> +		mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
> +		if (mbx & MSSR_SEST || num_pkts >= quota)
> +			break;
> +		mbx &= MSSR_MBNST;
> +		mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
> +		/* Clear interrupt */
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx),
> +				mctl & ~MCTL_NEWDATA);
> +		rcar_can_rx_pkt(priv, mbx);
> +		++num_pkts;
> +	}
> +	/* All packets processed */
> +	if (num_pkts < quota) {
> +		u8 ier;
> +
> +		napi_complete(napi);
> +		ier = rcar_can_readb(priv, RCAR_CAN_IER);
> +		rcar_can_writeb(priv, RCAR_CAN_IER, ier | IER_RXM1IE);
> +	}
> +	return num_pkts;
> +}
> +
> +static int rcar_can_do_set_mode(struct net_device *ndev, enum can_mode mode)
> +{
> +	switch (mode) {
> +	case CAN_MODE_START:
> +		rcar_can_start(ndev);
> +		netif_wake_queue(ndev);
> +		return 0;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int rcar_can_get_berr_counter(const struct net_device *dev,
> +				     struct can_berr_counter *bec)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(dev);
> +
> +	bec->txerr = rcar_can_readb(priv, RCAR_CAN_TECR);
> +	bec->rxerr = rcar_can_readb(priv, RCAR_CAN_RECR);
> +	return 0;
> +}
> +
> +static int rcar_can_probe(struct platform_device *pdev)
> +{
> +	struct rcar_can_platform_data *pdata;
> +	struct rcar_can_priv *priv;
> +	struct net_device *ndev;
> +	struct resource *mem;
> +	void __iomem *addr;
> +	int err = -ENODEV;
> +	int irq;
> +
> +	pdata = pdev->dev.platform_data;
> +	if (!pdata) {
> +		dev_err(&pdev->dev, "No platform data provided!\n");
> +		goto fail;
> +	}
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (!irq) {
> +		dev_err(&pdev->dev, "No IRQ resource\n");
> +		goto fail;
> +	}
> +
> +	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;
> +	}
> +
> +	ndev = alloc_candev(sizeof(struct rcar_can_priv), N_MBX - FIRST_TX_MB);
> +	if (!ndev) {
> +		dev_err(&pdev->dev, "alloc_candev failed\n");
> +		err = -ENOMEM;
> +		goto fail;
> +	}
> +
> +	priv = netdev_priv(ndev);
> +
> +	priv->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(priv->clk)) {
> +		err = PTR_ERR(priv->clk);
> +		dev_err(&pdev->dev, "cannot get clock: %d\n", err);
> +		goto fail_clk;
> +	}
> +	clk_enable(priv->clk);
> +
> +	ndev->netdev_ops = &rcar_can_netdev_ops;
> +	ndev->irq = irq;
> +	ndev->flags |= IFF_ECHO;
> +	priv->ndev = ndev;
> +	priv->reg_base = addr;
> +	priv->clock_select = pdata->clock_select;
> +	priv->can.clock.freq = clk_get_rate(priv->clk);
> +	priv->can.bittiming_const = &rcar_can_bittiming_const;
> +	priv->can.do_set_bittiming = rcar_can_set_bittiming;
> +	priv->can.do_set_mode = rcar_can_do_set_mode;
> +	priv->can.do_get_berr_counter = rcar_can_get_berr_counter;
> +	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES |
> +				       CAN_CTRLMODE_ONE_SHOT;
> +	platform_set_drvdata(pdev, ndev);
> +	SET_NETDEV_DEV(ndev, &pdev->dev);
> +	spin_lock_init(&priv->mier_lock);
> +
> +	netif_napi_add(ndev, &priv->napi, rcar_can_rx_poll,
> +		       RCAR_CAN_NAPI_WEIGHT);
> +	err = register_candev(ndev);
> +	if (err) {
> +		dev_err(&pdev->dev, "register_candev() failed\n");
> +		goto fail_candev;
> +	}
> +
> +	devm_can_led_init(ndev);
> +
> +	dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%u)\n",
> +		 priv->reg_base, ndev->irq);
> +
> +	return 0;
> +fail_candev:
> +	netif_napi_del(&priv->napi);
> +fail_clk:
> +	free_candev(ndev);
> +fail:
> +	return err;
> +}
> +
> +static int rcar_can_remove(struct platform_device *pdev)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr;
> +
> +	unregister_candev(ndev);
> +	netif_napi_del(&priv->napi);
> +	/* Go to sleep mode */
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr | CTLR_SLPM);
> +	clk_disable(priv->clk);
> +	free_candev(ndev);
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int rcar_can_suspend(struct device *dev)
> +{
> +	struct net_device *ndev = dev_get_drvdata(dev);
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr;
> +
> +	if (netif_running(ndev)) {
> +		netif_stop_queue(ndev);
> +		netif_device_detach(ndev);
> +	}
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	ctlr |= CTLR_HALT;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	ctlr |= CTLR_SLPM;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	priv->can.state = CAN_STATE_SLEEPING;
> +
> +	clk_disable(priv->clk);
> +	return 0;
> +}
> +
> +static int rcar_can_resume(struct device *dev)
> +{
> +	struct net_device *ndev = dev_get_drvdata(dev);
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr;
> +
> +	clk_enable(priv->clk);
> +
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	ctlr &= ~CTLR_SLPM;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	ctlr &= ~CTLR_FORCE_RESET;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> +	if (netif_running(ndev)) {
> +		netif_device_attach(ndev);
> +		netif_start_queue(ndev);
> +	}
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(rcar_can_pm_ops, rcar_can_suspend, rcar_can_resume);
> +
> +static struct platform_driver rcar_can_driver = {
> +	.driver = {
> +		.name = DRV_NAME,
> +		.owner = THIS_MODULE,
> +		.pm = &rcar_can_pm_ops,
> +	},
> +	.probe = rcar_can_probe,
> +	.remove = rcar_can_remove,
> +};
> +
> +module_platform_driver(rcar_can_driver);
> +
> +MODULE_AUTHOR("Cogent Embedded, Inc.");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("CAN driver for Renesas R-Car SoC");
> +MODULE_ALIAS("platform:" DRV_NAME);
> Index: linux-can-next/include/linux/can/platform/rcar_can.h
> ===================================================================
> --- /dev/null
> +++ linux-can-next/include/linux/can/platform/rcar_can.h
> @@ -0,0 +1,15 @@
> +#ifndef _CAN_PLATFORM_RCAR_CAN_H_
> +#define _CAN_PLATFORM_RCAR_CAN_H_
> +
> +#include <linux/types.h>
> +
> +/* Clock Select Register settings */
> +#define CLKR_CLKEXT	3	/* Externally input clock */
> +#define CLKR_CLKP2	1	/* Peripheral clock (clkp2) */
> +#define CLKR_CLKP1	0	/* Peripheral clock (clkp1) */
> +
> +struct rcar_can_platform_data {
> +	u8 clock_select;	/* Clock source select */
> +};
> +
> +#endif	/* !_CAN_PLATFORM_RCAR_CAN_H_ */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wolfgang Grandegger Oct. 5, 2013, 5:57 p.m. UTC | #6

On 09/28/2013 12:11 AM, Sergei Shtylyov wrote:
> Add support for the CAN controller found in Renesas R-Car SoCs. 
> 
> Signed-off-by: Sergei Shtylyov <[e-mail]>
> 
> ---
> The patch is against the 'linux-can-next.git' repo.
> 
>  drivers/net/can/Kconfig               |    9 
>  drivers/net/can/Makefile              |    1 
>  drivers/net/can/rcar_can.c            |  898 ++++++++++++++++++++++++++++++++++
>  include/linux/can/platform/rcar_can.h |   15 
>  4 files changed, 923 insertions(+)
> 
> Index: linux-can-next/drivers/net/can/Kconfig
> ===================================================================
> --- linux-can-next.orig/drivers/net/can/Kconfig
> +++ linux-can-next/drivers/net/can/Kconfig
> @@ -125,6 +125,15 @@ config CAN_GRCAN
>  	  endian syntheses of the cores would need some modifications on
>  	  the hardware level to work.
>  
> +config CAN_RCAR
> +	tristate "Renesas R-Car CAN controller"
> +	---help---
> +	  Say Y here if you want to use CAN controller found on Renesas R-Car
> +	  SoCs.
> +
> +	  To compile this driver as a module, choose M here: the module will
> +	  be called rcar_can.
> +
>  source "drivers/net/can/mscan/Kconfig"
>  
>  source "drivers/net/can/sja1000/Kconfig"
> Index: linux-can-next/drivers/net/can/Makefile
> ===================================================================
> --- linux-can-next.orig/drivers/net/can/Makefile
> +++ linux-can-next/drivers/net/can/Makefile
> @@ -25,5 +25,6 @@ obj-$(CONFIG_CAN_JANZ_ICAN3)	+= janz-ica
>  obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
>  obj-$(CONFIG_PCH_CAN)		+= pch_can.o
>  obj-$(CONFIG_CAN_GRCAN)		+= grcan.o
> +obj-$(CONFIG_CAN_RCAR)		+= rcar_can.o
>  
>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> Index: linux-can-next/drivers/net/can/rcar_can.c
> ===================================================================
> --- /dev/null
> +++ linux-can-next/drivers/net/can/rcar_can.c
> @@ -0,0 +1,898 @@
> +/*
> + * Renesas R-Car CAN device driver
> + *
> + * Copyright (C) 2013 Cogent Embedded, Inc. <[e-mail]>
> + * Copyright (C) 2013 Renesas Solutions 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/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/can/platform/rcar_can.h>
> +
> +#define DRV_NAME	"rcar_can"
> +
> +#define RCAR_CAN_MIER1	0x42C	/* CANi Mailbox Interrupt Enable Register 1 */
> +#define RCAR_CAN_MKR(n)	((n) < 2 ? 0x430 + 4 * (n) : 0x400 + 4 * ((n) - 2))
> +				/* CANi Mask Register */
> +#define RCAR_CAN_MKIVLR0 0x438	/* CANi Mask Invalid Register 0 */
> +#define RCAR_CAN_MIER0	 0x43C	/* CANi Mailbox Interrupt Enable Register 0 */
> +
> +#define RCAR_CAN_MCTL(n) (0x800 + (n)) /* CANi Message Control Register */
> +#define RCAR_CAN_CTLR	0x840	/* CANi Control Register */
> +#define RCAR_CAN_STR	0x842	/* CANi Status Register */
> +#define RCAR_CAN_BCR	0x844	/* CANi Bit Configuration Register */
> +#define RCAR_CAN_CLKR	0x847	/* CANi Clock Select Register */
> +#define RCAR_CAN_EIER	0x84C	/* CANi Error Interrupt Enable Register */
> +#define RCAR_CAN_EIFR	0x84D	/* CANi Err Interrupt Factor Judge Register */
> +#define RCAR_CAN_RECR	0x84E	/* CANi Receive Error Count Register */
> +#define RCAR_CAN_TECR	0x84F	/* CANi Transmit Error Count Register */
> +#define RCAR_CAN_ECSR	0x850	/* CANi Error Code Store Register */
> +#define RCAR_CAN_MSSR	0x852	/* CANi Mailbox Search Status Register */
> +#define RCAR_CAN_MSMR	0x853	/* CANi Mailbox Search Mode Register */
> +#define RCAR_CAN_TCR	0x858	/* CANi Test Control Register */
> +#define RCAR_CAN_IER	0x860	/* CANi Interrupt Enable Register */
> +#define RCAR_CAN_ISR	0x861	/* CANi Interrupt Status Register */

CANi on each line!? I would drop it.

> +/* Offsets of RCAR_CAN Mailbox Registers */
> +#define MBX_HDR_OFFSET	0x0
> +#define MBX_DLC_OFFSET	0x5
> +#define MBX_DATA_OFFSET	0x6

I see some advantages in using structs to describe the register layout
here. Better readability, type checking and the macros below will be
obsolete.

> +#define RCAR_CAN_MBX_SIZE 0x10
> +
> +/* Control Register bits */
> +#define CTLR_SLPM	BIT(10)
> +#define CTLR_HALT	BIT(9)
> +#define CTLR_RESET	BIT(8)
> +#define CTLR_FORCE_RESET (3 << 8)
> +#define CTLR_TPM	BIT(4)	/* Transmission Priority Mode Select Bit */
> +#define CTLR_IDFM_MIXED	BIT(2)	/* Mixed ID mode */
> +
> +/* Message Control Register bits */
> +#define MCTL_TRMREQ	BIT(7)
> +#define MCTL_RECREQ	BIT(6)
> +#define MCTL_ONESHOT	BIT(4)
> +#define MCTL_SENTDATA	BIT(0)
> +#define MCTL_NEWDATA	BIT(0)
> +
> +#define N_RX_MKREGS	2	/* Number of mask registers */
> +				/* for Rx mailboxes 0-31 */
> +
> +/* Bit Configuration Register settings */
> +#define BCR_TSEG1(x)	(((x) & 0x0f) << 28)
> +#define BCR_BPR(x)	(((x) & 0x3ff) << 16)
> +#define BCR_SJW(x)	(((x) & 0x3) << 12)
> +#define BCR_TSEG2(x)	(((x) & 0x07) << 8)
> +
> +/* Mailbox and Mask Registers bits */
> +#define RCAR_CAN_IDE	BIT(31)
> +#define RCAR_CAN_RTR	BIT(30)
> +#define RCAR_CAN_SID_SHIFT 18
> +
> +/* Interrupt Enable Register bits */
> +#define IER_ERSIE	BIT(5)	/* Error (ERS) Interrupt Enable Bit */
> +#define IER_RXM0IE	BIT(2)	/* Mailbox 0 Successful Reception (RXM0) */
> +				/* Interrupt Enable Bit */
> +#define IER_RXM1IE	BIT(1)	/* Mailbox 1 Successful Reception (RXM0) */
> +				/* Interrupt Enable Bit */
> +#define IER_TXMIE	BIT(0)	/* Mailbox 32 to 63 Successful Tx */
> +				/* Interrupt Enable Bit */
> +
> +/* Interrupt Status Register bits */
> +#define ISR_ERSF	BIT(5)	/* Error (ERS) Interrupt Status Bit */
> +#define ISR_RXM0F	BIT(2)	/* Mailbox 0 Successful Reception (RXM0) */
> +				/* Interrupt Status Bit */
> +#define ISR_RXM1F	BIT(1)	/* Mailbox 1 to 63 Successful Reception */
> +				/* (RXM1) Interrupt Status Bit */
> +#define ISR_TXMF	BIT(0)	/* Mailbox 32 to 63 Successful Transmission */
> +				/* (TXM) Interrupt Status Bit */
> +
> +/* Error Interrupt Enable Register bits */
> +#define EIER_BLIE	BIT(7)	/* Bus Lock Interrupt Enable */
> +#define EIER_OLIE	BIT(6)	/* Overload Frame Transmit Interrupt Enable */
> +#define EIER_ORIE	BIT(5)	/* Receive Overrun Interrupt Enable */
> +#define EIER_BORIE	BIT(4)	/* Bus-Off Recovery Interrupt Enable */
> +
> +#define EIER_BOEIE	BIT(3)	/* Bus-Off Entry Interrupt Enable */
> +#define EIER_EPIE	BIT(2)	/* Error Passive Interrupt Enable */
> +#define EIER_EWIE	BIT(1)	/* Error Warning Interrupt Enable */
> +#define EIER_BEIE	BIT(0)	/* Bus Error Interrupt Enable */
> +
> +/* Error Interrupt Factor Judge Register bits */
> +#define EIFR_BLIF	BIT(7)	/* Bus Lock Detect Flag */
> +#define EIFR_OLIF	BIT(6)	/* Overload Frame Transmission Detect Flag */
> +#define EIFR_ORIF	BIT(5)	/* Receive Overrun Detect Flag */
> +#define EIFR_BORIF	BIT(4)	/* Bus-Off Recovery Detect Flag */
> +#define EIFR_BOEIF	BIT(3)	/* Bus-Off Entry Detect Flag */
> +#define EIFR_EPIF	BIT(2)	/* Error Passive Detect Flag */
> +#define EIFR_EWIF	BIT(1)	/* Error Warning Detect Flag */
> +#define EIFR_BEIF	BIT(0)	/* Bus Error Detect Flag */
> +
> +/* Error Code Store Register bits */
> +#define ECSR_EDPM	BIT(7)	/* Error Display Mode Select Bit */
> +#define ECSR_ADEF	BIT(6)	/* ACK Delimiter Error Flag */
> +#define ECSR_BE0F	BIT(5)	/* Bit Error (dominant) Flag */
> +#define ECSR_BE1F	BIT(4)	/* Bit Error (recessive) Flag */
> +#define ECSR_CEF	BIT(3)	/* CRC Error Flag */
> +#define ECSR_AEF	BIT(2)	/* ACK Error Flag */
> +#define ECSR_FEF	BIT(1)	/* Form Error Flag */
> +#define ECSR_SEF	BIT(0)	/* Stuff Error Flag */
> +
> +/* Mailbox Search Status Register bits */
> +#define MSSR_SEST	BIT(7)	/* Search Result Status Bit */
> +#define MSSR_MBNST	0x3f	/* Search Result Mailbox Number Status mask */
> +
> +/* Mailbox Search Mode Register values */
> +#define MSMR_TXMB	1	/* Transmit mailbox search mode */
> +#define MSMR_RXMB	0	/* Receive mailbox search mode */
> +
> +/* Mailbox configuration:
> + * mailbox 0 - not used
> + * mailbox 1-31 - Rx
> + * mailbox 32-63 - Tx
> + * no FIFO mailboxes
> + */
> +#define N_MBX		64
> +#define FIRST_TX_MB	32
> +#define RX_MBX_MASK	0xFFFFFFFE
> +
> +#define RCAR_CAN_NAPI_WEIGHT (FIRST_TX_MB - 1)
> +
> +struct rcar_can_priv {
> +	struct can_priv can;	/* Must be the first member! */
> +	struct net_device *ndev;
> +	struct napi_struct napi;
> +	void __iomem *reg_base;
> +	struct clk *clk;
> +	spinlock_t mier_lock;
> +	u8 clock_select;
> +};
> +
> +static const struct can_bittiming_const rcar_can_bittiming_const = {
> +	.name = DRV_NAME,
> +	.tseg1_min = 4,
> +	.tseg1_max = 16,
> +	.tseg2_min = 2,
> +	.tseg2_max = 8,
> +	.sjw_max = 4,
> +	.brp_min = 1,
> +	.brp_max = 1024,
> +	.brp_inc = 1,
> +};
> +
> +static inline u32 rcar_can_readl(struct rcar_can_priv *priv, int reg)
> +{
> +	return readl(priv->reg_base + reg);
> +}
> +
> +static inline u16 rcar_can_readw(struct rcar_can_priv *priv, int reg)
> +{
> +	return readw(priv->reg_base + reg);
> +}
> +
> +static inline u8 rcar_can_readb(struct rcar_can_priv *priv, int reg)
> +{
> +	return readb(priv->reg_base + reg);
> +}
> +
> +static inline void rcar_can_writel(struct rcar_can_priv *priv, int reg, u32 val)
> +{
> +	writel(val, priv->reg_base + reg);
> +}
> +
> +static inline void rcar_can_writew(struct rcar_can_priv *priv, int reg, u16 val)
> +{
> +	writew(val, priv->reg_base + reg);
> +}
> +
> +static inline void rcar_can_writeb(struct rcar_can_priv *priv, int reg, u8 val)
> +{
> +	writeb(val, priv->reg_base + reg);
> +}
> +
> +static inline u32 rcar_can_mbx_readl(struct rcar_can_priv *priv,
> +				     u32 mbxno, u8 offset)
> +{
> +	return rcar_can_readl(priv, RCAR_CAN_MBX_SIZE * mbxno + offset);
> +}
> +
> +static inline u8 rcar_can_mbx_readb(struct rcar_can_priv *priv,
> +				    u32 mbxno, u8 offset)
> +{
> +	return rcar_can_readb(priv, RCAR_CAN_MBX_SIZE * mbxno + offset);
> +}
> +
> +static inline void rcar_can_mbx_writel(struct rcar_can_priv *priv, u32 mbxno,
> +				       u8 offset, u32 val)
> +{
> +	rcar_can_writel(priv, RCAR_CAN_MBX_SIZE * mbxno + offset, val);
> +}
> +
> +static inline void rcar_can_mbx_writeb(struct rcar_can_priv *priv, u32 mbxno,
> +				       u8 offset, u8 val)
> +{
> +	rcar_can_writeb(priv, RCAR_CAN_MBX_SIZE * mbxno + offset, val);
> +}

See my comment above. Reading the DLC from the mailbox could then be
done with:

	writeb(val, &priv->regs.mb[mbxno].dlc]);

> +static void rcar_can_error(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	u8 eifr;
> +
> +	/* Propagate the error condition to the CAN stack */
> +	skb = alloc_can_err_skb(ndev, &cf);
> +	if (!skb) {
> +		if (printk_ratelimit())
> +			netdev_err(priv->ndev,
> +				   "%s: alloc_can_err_skb() failed\n",
> +				   __func__);
> +		return;
> +	}

IIRC, the policy is to return silently here. I only see the ti_hecc
driver is using printk_ratelimit(). Marc?

> +	eifr = rcar_can_readb(priv, RCAR_CAN_EIFR);
> +	if (eifr & EIFR_EWIF) {
> +		netdev_dbg(priv->ndev, "Error warning interrupt\n");
> +		priv->can.state = CAN_STATE_ERROR_WARNING;
> +		priv->can.can_stats.error_warning++;
> +		cf->can_id |= CAN_ERR_CRTL;
> +		if (rcar_can_readb(priv, RCAR_CAN_TECR) > 96)
> +			cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
> +		if (rcar_can_readb(priv, RCAR_CAN_RECR) > 96)
> +			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> +		/* Clear interrupt condition */
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EWIF);

Please avoid casts. I think they are not needed anyway (here and below).

> +	}
> +	if (eifr & EIFR_EPIF) {
> +		netdev_dbg(priv->ndev, "Error passive interrupt\n");
> +		priv->can.state = CAN_STATE_ERROR_PASSIVE;
> +		priv->can.can_stats.error_passive++;
> +		cf->can_id |= CAN_ERR_CRTL;
> +		if (rcar_can_readb(priv, RCAR_CAN_TECR) > 127)
> +			cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
> +		if (rcar_can_readb(priv, RCAR_CAN_RECR) > 127)
> +			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
> +		/* Clear interrupt condition */
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EPIF);
> +	}
> +	if (eifr & EIFR_BOEIF) {
> +		netdev_dbg(priv->ndev, "Bus-off entry interrupt\n");
> +		priv->can.state = CAN_STATE_BUS_OFF;
> +		cf->can_id |= CAN_ERR_BUSOFF;
> +		/* Clear interrupt condition */
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BOEIF);
> +		/* Disable all interrupts in bus-off to avoid int hog */
> +		rcar_can_writeb(priv, RCAR_CAN_EIER, 0);
> +		rcar_can_writeb(priv, RCAR_CAN_IER, 0);
> +		can_bus_off(ndev);
> +	}
> +	if (eifr & EIFR_BEIF) {
> +		int rx_errors = 0, tx_errors = 0, bus_errors = 0;
> +		u8 ecsr;
> +
> +		netdev_dbg(priv->ndev, "Bus error interrupt:\n");
> +		cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
> +		cf->data[2] = CAN_ERR_PROT_UNSPEC;

I think you can increment bus errors here for all types.:

		bus_errors++;

> +
> +		ecsr = rcar_can_readb(priv, RCAR_CAN_ECSR);
> +		if (ecsr & ECSR_ADEF) {
> +			netdev_dbg(priv->ndev, "ACK Delimiter Error\n");
> +			cf->data[3] |= CAN_ERR_PROT_LOC_ACK_DEL;
> +			bus_errors++;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_ADEF);
> +		}
> +		if (ecsr & ECSR_BE0F) {
> +			netdev_dbg(priv->ndev, "Bit Error (dominant)\n");
> +			cf->data[2] |= CAN_ERR_PROT_BIT0;
> +			bus_errors++;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_BE0F);
> +		}
> +		if (ecsr & ECSR_BE1F) {
> +			netdev_dbg(priv->ndev, "Bit Error (recessive)\n");
> +			cf->data[2] |= CAN_ERR_PROT_BIT1;
> +			bus_errors++;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_BE1F);
> +		}
> +		if (ecsr & ECSR_CEF) {
> +			netdev_dbg(priv->ndev, "CRC Error\n");
> +			cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
> +			bus_errors++;
> +			rx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_CEF);
> +		}
> +		if (ecsr & ECSR_AEF) {
> +			netdev_dbg(priv->ndev, "ACK Error\n");
> +			cf->data[3] |= CAN_ERR_PROT_LOC_ACK;

Please add here:

		        cf->can_id |= CAN_ERR_ACK;

> +			bus_errors++;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_AEF);
> +		}
> +		if (ecsr & ECSR_FEF) {
> +			netdev_dbg(priv->ndev, "Form Error\n");
> +			cf->data[2] |= CAN_ERR_PROT_FORM;
> +			bus_errors++;
> +			rx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_FEF);
> +		}
> +		if (ecsr & ECSR_SEF) {
> +			netdev_dbg(priv->ndev, "Stuff Error\n");
> +			cf->data[2] |= CAN_ERR_PROT_STUFF;
> +			bus_errors++;
> +			rx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_SEF);
> +		}
> +
> +		priv->can.can_stats.bus_error += bus_errors;
> +		ndev->stats.rx_errors += rx_errors;
> +		ndev->stats.tx_errors += tx_errors;
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BEIF);
> +	}
> +	if (eifr & EIFR_ORIF) {
> +		netdev_dbg(priv->ndev, "Receive overrun error interrupt\n");
> +		cf->can_id |= CAN_ERR_CRTL;
> +		cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> +		ndev->stats.rx_over_errors++;
> +		ndev->stats.rx_errors++;
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_ORIF);
> +	}
> +	if (eifr & EIFR_OLIF) {
> +		netdev_dbg(priv->ndev,
> +			   "Overload Frame Transmission error interrupt\n");
> +		cf->can_id |= CAN_ERR_PROT;
> +		cf->data[2] |= CAN_ERR_PROT_OVERLOAD;
> +		ndev->stats.rx_over_errors++;
> +		ndev->stats.rx_errors++;
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_OLIF);
> +	}
> +
> +	netif_rx(skb);
> +	stats->rx_packets++;
> +	stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static irqreturn_t rcar_can_interrupt(int irq, void *dev_id)
> +{
> +	struct net_device *ndev = (struct net_device *)dev_id;
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	u8 isr;
> +
> +	isr = rcar_can_readb(priv, RCAR_CAN_ISR);
> +	if (isr & ISR_ERSF)
> +		rcar_can_error(ndev);
> +
> +	if (isr & ISR_TXMF) {
> +		u32 ie_mask = 0;
> +
> +		/* Set Transmit Mailbox Search Mode */
> +		rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_TXMB);
> +		while (1) {
> +			u8 mctl, mbx;
> +
> +			mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
> +			if (mbx & MSSR_SEST)
> +				break;
> +			mbx &= MSSR_MBNST;
> +			mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
> +			/* Bits SENTDATA and TRMREQ cannot be
> +			 * set to 0 simultaneously
> +			 */
> +			mctl &= ~MCTL_TRMREQ;
> +			rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
> +			mctl &= ~MCTL_SENTDATA;
> +			/* Clear interrupt */
> +			rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
> +			ie_mask |= BIT(mbx - FIRST_TX_MB);
> +			stats->tx_bytes += can_get_echo_skb(ndev,
> +							    mbx - FIRST_TX_MB);

can_get_echo_skb() may return 0 because local loopback can be disabled
by the user.

> +			stats->tx_packets++;
> +			can_led_event(ndev, CAN_LED_EVENT_TX);
> +		}
> +		/* Set receive mailbox search mode */
> +		rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_RXMB);
> +		/* Disable mailbox interrupt, mark mailbox as free */
> +		if (ie_mask) {
> +			u32 mier1;
> +
> +			spin_lock(&priv->mier_lock);
> +			mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
> +			rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 & ~ie_mask);
> +			spin_unlock(&priv->mier_lock);
> +			if (unlikely(netif_queue_stopped(ndev)))
> +				netif_wake_queue(ndev);
> +		}
> +	}
> +	if (isr & ISR_RXM1F) {
> +		if (napi_schedule_prep(&priv->napi)) {
> +			/* Disable Rx interrupts */
> +			rcar_can_writeb(priv, RCAR_CAN_IER,
> +					rcar_can_readb(priv, RCAR_CAN_IER) &
> +						       ~IER_RXM1IE);
> +			__napi_schedule(&priv->napi);
> +		}
> +	}
> +	return IRQ_HANDLED;
> +}
> +
> +static int rcar_can_set_bittiming(struct net_device *dev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(dev);
> +	struct can_bittiming *bt = &priv->can.bittiming;
> +	u32 bcr;
> +	u16 ctlr;
> +	u8 clkr;
> +
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	if (ctlr & CTLR_SLPM) {
> +		/* Write to BCR in CAN reset mode or CAN halt mode */
> +		return -EBUSY;
> +	}
> +	/* Don't overwrite CLKR with 32-bit BCR access */
> +	/* CLKR has 8-bit access */
> +	clkr = rcar_can_readb(priv, RCAR_CAN_CLKR);
> +	bcr = BCR_TSEG1(bt->phase_seg1 + bt->prop_seg - 1) |
> +	      BCR_BPR(bt->brp - 1) | BCR_SJW(bt->sjw - 1) |
> +	      BCR_TSEG2(bt->phase_seg2 - 1);
> +	rcar_can_writel(priv, RCAR_CAN_BCR, bcr);
> +	rcar_can_writeb(priv, RCAR_CAN_CLKR, clkr);
> +	return 0;

What about support for CAN_CTRLMODE_3_SAMPLES?

> +}
> +
> +static void rcar_can_start(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr, n;
> +
> +	/* Set controller to known mode:
> +	 * - normal mailbox mode (no FIFO);
> +	 * - accept all messages (no filter).
> +	 * CAN is in sleep mode after MCU hardware or software reset.
> +	 */
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	ctlr &= ~CTLR_SLPM;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	/* Go to reset mode */
> +	ctlr |= CTLR_FORCE_RESET;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	ctlr |= CTLR_IDFM_MIXED;	/* Select mixed ID mode */
> +	ctlr &= ~CTLR_TPM;		/* Set ID priority transmit mode */
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +
> +	rcar_can_writeb(priv, RCAR_CAN_CLKR, priv->clock_select);
> +
> +	/* Accept all SID and EID */
> +	for (n = 0; n < N_RX_MKREGS; n++)
> +		rcar_can_writel(priv, RCAR_CAN_MKR(n), 0);
> +	rcar_can_writel(priv, RCAR_CAN_MKIVLR0, 0);
> +
> +	rcar_can_set_bittiming(ndev);
> +
> +	/* Initial value of MIER1 undefined.  Mark all Tx mailboxes as free. */
> +	rcar_can_writel(priv, RCAR_CAN_MIER1, 0);
> +
> +	rcar_can_writeb(priv, RCAR_CAN_IER, IER_TXMIE | IER_ERSIE | IER_RXM1IE);
> +
> +	/* Accumulate error codes */
> +	rcar_can_writeb(priv, RCAR_CAN_ECSR, ECSR_EDPM);
> +	/* Enable error interrupts */
> +	rcar_can_writeb(priv, RCAR_CAN_EIER,
> +			EIER_EWIE | EIER_EPIE | EIER_BOEIE | EIER_BEIE |
> +			EIER_ORIE | EIER_OLIE);

Bus error reporting might result in heavy system load, especially
because they are handled in the irq context. For this purpose we have
the CAN_CTRLMODE_BERR_REPORTING flag. Shoudl not be a big deal to
support it here (if the hardware is working properly):


	rcar_can_writeb(priv, RCAR_CAN_EIER,
			EIER_EWIE | EIER_EPIE | EIER_BOEIE |
			(priv->can.ctrlmode &
			 CAN_CTRLMODE_BERR_REPORTING ? EIER_BEIE : 0) |
			EIER_ORIE | EIER_OLIE);

> +	/* Enable interrupts for RX mailboxes */
> +	rcar_can_writel(priv, RCAR_CAN_MIER0, RX_MBX_MASK);
> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> +	/* Write to the CiMCTLj register in CAN
> +	 * operation mode or CAN halt mode.
> +	 * Configure mailboxes 0-31 as Rx mailboxes.
> +	 * Configure mailboxes 32-63 as Tx mailboxes.
> +	 */
> +	/* Go to halt mode */
> +	ctlr |= CTLR_HALT;
> +	ctlr &= ~CTLR_RESET;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	for (n = 0; n < FIRST_TX_MB; n++) {
> +		/* According to documentation we should clear MCTL
> +		 * register before configuring mailbox.
> +		 */
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(n), 0);
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(n), MCTL_RECREQ);
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(FIRST_TX_MB + n), 0);
> +	}
> +	/* Go to operation mode */
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr & ~CTLR_FORCE_RESET);
> +}
> +
> +static int rcar_can_open(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	int err;
> +
> +	err = open_candev(ndev);
> +	if (err) {
> +		netdev_err(ndev, "open_candev() failed %d\n", err);
> +		goto out;
> +	}
> +	napi_enable(&priv->napi);
> +	err = request_irq(ndev->irq, rcar_can_interrupt, 0, ndev->name, ndev);
> +	if (err) {
> +		netdev_err(ndev, "error requesting interrupt %x\n", ndev->irq);
> +		goto out_close;
> +	}
> +	can_led_event(ndev, CAN_LED_EVENT_OPEN);
> +	rcar_can_start(ndev);
> +	netif_start_queue(ndev);
> +	return 0;
> +out_close:
> +	napi_disable(&priv->napi);
> +	close_candev(ndev);
> +out:
> +	return err;
> +}
> +
> +static void rcar_can_stop(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr;
> +
> +	/* Go to (force) reset mode */
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr | CTLR_FORCE_RESET);
> +	rcar_can_writel(priv, RCAR_CAN_MIER0, 0);
> +	rcar_can_writel(priv, RCAR_CAN_MIER1, 0);
> +	rcar_can_writeb(priv, RCAR_CAN_IER, 0);
> +	rcar_can_writeb(priv, RCAR_CAN_EIER, 0);
> +	priv->can.state = CAN_STATE_STOPPED;
> +}
> +
> +static int rcar_can_close(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +
> +	netif_stop_queue(ndev);
> +	rcar_can_stop(ndev);
> +	free_irq(ndev->irq, ndev);
> +	napi_disable(&priv->napi);
> +	close_candev(ndev);
> +	can_led_event(ndev, CAN_LED_EVENT_STOP);
> +	return 0;
> +}
> +
> +static netdev_tx_t rcar_can_start_xmit(struct sk_buff *skb,
> +				       struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	struct can_frame *cf = (struct can_frame *)skb->data;
> +	u32 data, mier1, mbxno, i;
> +	unsigned long flags;
> +	u8 mctl;
> +
> +	if (can_dropped_invalid_skb(ndev, skb))
> +		return NETDEV_TX_OK;
> +
> +	spin_lock_irqsave(&priv->mier_lock, flags);
> +	mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
> +	if (unlikely(mier1 == ~0U)) {
> +		spin_unlock_irqrestore(&priv->mier_lock, flags);
> +		netif_stop_queue(ndev);
> +		return NETDEV_TX_BUSY;
> +	}

I think it is possible the avoid the "return NETDEV_TX_BUSY" here in
most cases if you stop the queue at the end of the previous message.

> +	rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 | (mier1 + 1));
> +	spin_unlock_irqrestore(&priv->mier_lock, flags);
> +	mbxno = ffz(mier1) + FIRST_TX_MB;

How to you handle "mbxno" wrap around. Are messages then still send out
in order?

> +
> +	if (cf->can_id & CAN_EFF_FLAG) {
> +		/* Extended frame format */
> +		data = (cf->can_id & CAN_EFF_MASK) | RCAR_CAN_IDE;
> +	} else {
> +		/* Standard frame format */
> +		data = (cf->can_id & CAN_SFF_MASK) << RCAR_CAN_SID_SHIFT;
> +	}
> +	if (cf->can_id & CAN_RTR_FLAG) {
> +		/* Remote transmission request */
> +		data |= RCAR_CAN_RTR;
> +	}
> +	rcar_can_mbx_writel(priv, mbxno, MBX_HDR_OFFSET, data);
> +
> +	rcar_can_mbx_writeb(priv, mbxno, MBX_DLC_OFFSET, cf->can_dlc);
> +
> +	for (i = 0; i < cf->can_dlc; i++)
> +		rcar_can_mbx_writeb(priv, mbxno,
> +				    MBX_DATA_OFFSET + i, cf->data[i]);
> +
> +	can_put_echo_skb(skb, ndev, mbxno - FIRST_TX_MB);
> +
> +	rcar_can_writeb(priv, RCAR_CAN_IER,
> +			rcar_can_readb(priv, RCAR_CAN_IER) | IER_TXMIE);
> +
> +	mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbxno));
> +	if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> +		mctl |= MCTL_ONESHOT;
> +	else
> +		mctl &= ~MCTL_ONESHOT;
> +	/* Start TX */
> +	mctl |= MCTL_TRMREQ;
> +	rcar_can_writeb(priv, RCAR_CAN_MCTL(mbxno), mctl);
> +	return NETDEV_TX_OK;
> +}
> +
> +static const struct net_device_ops rcar_can_netdev_ops = {
> +	.ndo_open = rcar_can_open,
> +	.ndo_stop = rcar_can_close,
> +	.ndo_start_xmit = rcar_can_start_xmit,
> +};
> +
> +static void rcar_can_rx_pkt(struct rcar_can_priv *priv, int mbx)
> +{
> +	struct net_device_stats *stats = &priv->ndev->stats;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	u32 data;
> +	u8 dlc;
> +
> +	skb = alloc_can_skb(priv->ndev, &cf);
> +	if (!skb) {
> +		stats->rx_dropped++;
> +		if (printk_ratelimit())
> +			netdev_err(priv->ndev,
> +				   "%s: alloc_can_skb() failed\n", __func__);
> +		return;
> +	}

Same comment as for alloc_can_err_skb().

> +	data = rcar_can_mbx_readl(priv, mbx, MBX_HDR_OFFSET);
> +	if (data & RCAR_CAN_IDE)
> +		cf->can_id = (data & CAN_EFF_MASK) | CAN_EFF_FLAG;
> +	else
> +		cf->can_id = (data >> RCAR_CAN_SID_SHIFT) & CAN_SFF_MASK;
> +	if (data & RCAR_CAN_RTR)
> +		cf->can_id |= CAN_RTR_FLAG;

IIRC, we should not copy data for RTR frames. At least for the SJA1000
we fixed:

  commit 87e9af6cc67d842cd92b52b81f3f14e665e7ab05
  Author: Kurt Van Dijck <[e-mail]>
  Date:   Mon May 2 04:50:48 2011 +0000

    can: fix SJA1000 dlc for RTR packets

    RTR frames do have a valid data length code on CAN.
    The driver for SJA1000 did not handle that situation properly.

But I see that not all drivers do respect that rule.

> +
> +	dlc = rcar_can_mbx_readb(priv, mbx, MBX_DLC_OFFSET);
> +	cf->can_dlc = get_can_dlc(dlc);
> +	for (dlc = 0; dlc < cf->can_dlc; dlc++)
> +		cf->data[dlc] = rcar_can_mbx_readb(priv, mbx,
> +						   MBX_DATA_OFFSET + dlc);
> +
> +	can_led_event(priv->ndev, CAN_LED_EVENT_RX);
> +
> +	netif_receive_skb(skb);
> +	stats->rx_bytes += cf->can_dlc;
> +	stats->rx_packets++;
> +}
> +
> +static int rcar_can_rx_poll(struct napi_struct *napi, int quota)
> +{
> +	struct rcar_can_priv *priv = container_of(napi,
> +						  struct rcar_can_priv, napi);
> +	u32 num_pkts = 0;
> +
> +	/* Find mailbox */
> +	while (1) {
> +		u8 mctl, mbx;
> +
> +		mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
> +		if (mbx & MSSR_SEST || num_pkts >= quota)
> +			break;
> +		mbx &= MSSR_MBNST;
> +		mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
> +		/* Clear interrupt */
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx),
> +				mctl & ~MCTL_NEWDATA);
> +		rcar_can_rx_pkt(priv, mbx);
> +		++num_pkts;
> +	}
> +	/* All packets processed */
> +	if (num_pkts < quota) {
> +		u8 ier;
> +
> +		napi_complete(napi);
> +		ier = rcar_can_readb(priv, RCAR_CAN_IER);
> +		rcar_can_writeb(priv, RCAR_CAN_IER, ier | IER_RXM1IE);
> +	}
> +	return num_pkts;
> +}
> +
> +static int rcar_can_do_set_mode(struct net_device *ndev, enum can_mode mode)
> +{
> +	switch (mode) {
> +	case CAN_MODE_START:
> +		rcar_can_start(ndev);
> +		netif_wake_queue(ndev);
> +		return 0;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int rcar_can_get_berr_counter(const struct net_device *dev,
> +				     struct can_berr_counter *bec)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(dev);
> +
> +	bec->txerr = rcar_can_readb(priv, RCAR_CAN_TECR);
> +	bec->rxerr = rcar_can_readb(priv, RCAR_CAN_RECR);
> +	return 0;
> +}
> +
> +static int rcar_can_probe(struct platform_device *pdev)
> +{
> +	struct rcar_can_platform_data *pdata;
> +	struct rcar_can_priv *priv;
> +	struct net_device *ndev;
> +	struct resource *mem;
> +	void __iomem *addr;
> +	int err = -ENODEV;
> +	int irq;
> +
> +	pdata = pdev->dev.platform_data;
> +	if (!pdata) {
> +		dev_err(&pdev->dev, "No platform data provided!\n");
> +		goto fail;
> +	}
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (!irq) {
> +		dev_err(&pdev->dev, "No IRQ resource\n");
> +		goto fail;
> +	}
> +
> +	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;
> +	}
> +
> +	ndev = alloc_candev(sizeof(struct rcar_can_priv), N_MBX - FIRST_TX_MB);
> +	if (!ndev) {
> +		dev_err(&pdev->dev, "alloc_candev failed\n");
> +		err = -ENOMEM;
> +		goto fail;
> +	}
> +
> +	priv = netdev_priv(ndev);
> +
> +	priv->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(priv->clk)) {
> +		err = PTR_ERR(priv->clk);
> +		dev_err(&pdev->dev, "cannot get clock: %d\n", err);
> +		goto fail_clk;
> +	}
> +	clk_enable(priv->clk);
> +
> +	ndev->netdev_ops = &rcar_can_netdev_ops;
> +	ndev->irq = irq;
> +	ndev->flags |= IFF_ECHO;
> +	priv->ndev = ndev;
> +	priv->reg_base = addr;
> +	priv->clock_select = pdata->clock_select;
> +	priv->can.clock.freq = clk_get_rate(priv->clk);
> +	priv->can.bittiming_const = &rcar_can_bittiming_const;
> +	priv->can.do_set_bittiming = rcar_can_set_bittiming;

rcar_can_set_bittiming() is already called in the open function. Is this
intended?

Wolfgang.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sergei Shtylyov Oct. 17, 2013, 9:54 p.m. UTC | #7

Hello.

On 09/29/2013 11:03 PM, Marc Kleine-Budde wrote:

    Sorry for the belated reply -- was on vacations.

>> Add support for the CAN controller found in Renesas R-Car SoCs.

> Is there a public available datasheet for the CAN core?

    I don't think so.

> What architecture are the Renesas R-Car SoCs? They're ARM, aren't they?

    ARM/SH, to be precise.

> What's R-Car's status on device tree conversion?

    Ongoing, but at quite an early stage yet.

> More comments inline

>> Signed-off-by: Sergei Shtylyov <[e-mail]>

[...]

>> Index: linux-can-next/drivers/net/can/rcar_can.c
>> ===================================================================
>> --- /dev/null
>> +++ linux-can-next/drivers/net/can/rcar_can.c
>> @@ -0,0 +1,898 @@
[...]
>> +#define RCAR_CAN_MIER1	0x42C	/* CANi Mailbox Interrupt Enable Register 1 */
>> +#define RCAR_CAN_MKR(n)	((n) < 2 ? 0x430 + 4 * (n) : 0x400 + 4 * ((n) - 2))
>> +				/* CANi Mask Register */
>> +#define RCAR_CAN_MKIVLR0 0x438	/* CANi Mask Invalid Register 0 */
>> +#define RCAR_CAN_MIER0	 0x43C	/* CANi Mailbox Interrupt Enable Register 0 */
>> +
>> +#define RCAR_CAN_MCTL(n) (0x800 + (n)) /* CANi Message Control Register */
>> +#define RCAR_CAN_CTLR	0x840	/* CANi Control Register */
>> +#define RCAR_CAN_STR	0x842	/* CANi Status Register */
>> +#define RCAR_CAN_BCR	0x844	/* CANi Bit Configuration Register */
>> +#define RCAR_CAN_CLKR	0x847	/* CANi Clock Select Register */
>> +#define RCAR_CAN_EIER	0x84C	/* CANi Error Interrupt Enable Register */
>> +#define RCAR_CAN_EIFR	0x84D	/* CANi Err Interrupt Factor Judge Register */
>> +#define RCAR_CAN_RECR	0x84E	/* CANi Receive Error Count Register */
>> +#define RCAR_CAN_TECR	0x84F	/* CANi Transmit Error Count Register */
>> +#define RCAR_CAN_ECSR	0x850	/* CANi Error Code Store Register */
>> +#define RCAR_CAN_MSSR	0x852	/* CANi Mailbox Search Status Register */
>> +#define RCAR_CAN_MSMR	0x853	/* CANi Mailbox Search Mode Register */
>> +#define RCAR_CAN_TCR	0x858	/* CANi Test Control Register */
>> +#define RCAR_CAN_IER	0x860	/* CANi Interrupt Enable Register */
>> +#define RCAR_CAN_ISR	0x861	/* CANi Interrupt Status Register */

> You might consider using an enum for the register offsets.

    Doesn't seem a good idea to me, given the parametrized macros above 
(corresponding to sequentially numbered registers).

>> +
>> +/* Offsets of RCAR_CAN Mailbox Registers */
>> +#define MBX_HDR_OFFSET	0x0
>> +#define MBX_DLC_OFFSET	0x5
>> +#define MBX_DATA_OFFSET	0x6

> can you please add the RCAR_ prefix to all defines.

    I think they would look clumsy but if you insist on *all* defines, I'd do 
it. I'm getting rid of the #define's above based on Wolfgang's feedback.

[...]

>> +static inline u32 rcar_can_mbx_readl(struct rcar_can_priv *priv,
>> +				     u32 mbxno, u8 offset)
>> +{
>> +	return rcar_can_readl(priv, RCAR_CAN_MBX_SIZE * mbxno + offset);

> Can you define hide the RCAR_CAN_MBX_SIZE * mbxno into a macro?
> Something like:

>      RCAR_CAN_MBX(mbxno)

    Used a structure there based on Wolfgang's suggestion.

[...]
>> +static void rcar_can_error(struct net_device *ndev)
>> +{
[...]
>> +	eifr = rcar_can_readb(priv, RCAR_CAN_EIFR);
>> +	if (eifr & EIFR_EWIF) {
>> +		netdev_dbg(priv->ndev, "Error warning interrupt\n");
>> +		priv->can.state = CAN_STATE_ERROR_WARNING;
>> +		priv->can.can_stats.error_warning++;
>> +		cf->can_id |= CAN_ERR_CRTL;
>> +		if (rcar_can_readb(priv, RCAR_CAN_TECR) > 96)
>> +			cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
>> +		if (rcar_can_readb(priv, RCAR_CAN_RECR) > 96)
>> +			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
>> +		/* Clear interrupt condition */
>> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EWIF);

> The cast is not needed.

    Unfortunately, I get a compiler warning without it.

>> +	}
>> +	if (eifr & EIFR_EPIF) {
>> +		netdev_dbg(priv->ndev, "Error passive interrupt\n");
>> +		priv->can.state = CAN_STATE_ERROR_PASSIVE;
>> +		priv->can.can_stats.error_passive++;
>> +		cf->can_id |= CAN_ERR_CRTL;
>> +		if (rcar_can_readb(priv, RCAR_CAN_TECR) > 127)
>> +			cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
>> +		if (rcar_can_readb(priv, RCAR_CAN_RECR) > 127)
>> +			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
>> +		/* Clear interrupt condition */
>> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EPIF);
>> +	}
>> +	if (eifr & EIFR_BOEIF) {
>> +		netdev_dbg(priv->ndev, "Bus-off entry interrupt\n");
>> +		priv->can.state = CAN_STATE_BUS_OFF;
>> +		cf->can_id |= CAN_ERR_BUSOFF;
>> +		/* Clear interrupt condition */
>> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BOEIF);
>> +		/* Disable all interrupts in bus-off to avoid int hog */
>> +		rcar_can_writeb(priv, RCAR_CAN_EIER, 0);
>> +		rcar_can_writeb(priv, RCAR_CAN_IER, 0);
>> +		can_bus_off(ndev);

> How does the rcan recover from bus off?

    Good question... this needs additional work.

[...]

>> +static irqreturn_t rcar_can_interrupt(int irq, void *dev_id)
>> +{
>> +	struct net_device *ndev = (struct net_device *)dev_id;
>> +	struct rcar_can_priv *priv = netdev_priv(ndev);
>> +	struct net_device_stats *stats = &ndev->stats;
>> +	u8 isr;
>> +
>> +	isr = rcar_can_readb(priv, RCAR_CAN_ISR);
>> +	if (isr & ISR_ERSF)
>> +		rcar_can_error(ndev);
>> +
>> +	if (isr & ISR_TXMF) {
>> +		u32 ie_mask = 0;
>> +
>> +		/* Set Transmit Mailbox Search Mode */
>> +		rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_TXMB);

> Can you please outline how to mailbox search mode works? What happens if
> you activate tx mailbox search mode here and rx mailbox search mode below?

    Work of the search is not well documented. The controller has 4 search 
modes, only one can be enabled at the moment.
    I just don't know what happens if we get TX interrupt while handling RX.
Either we painlessly switch to TX search, finish it and return to RX search,
and it continues where we are interrupted... or we lose some RX packets.

>> +		while (1) {

> I presonally don't like while (1) loops in an interrupt handler, can you
> rearange the code, so that you have an explizid loop termination?

    It was done in order to avoid assignment in the *while* expression.
Although checkpatch.pl didn't complain about it, it does complain about such 
in the *if* expression, so I thought it consistent to avoid this in the 
*while* too.

>> +			u8 mctl, mbx;
>> +
>> +			mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
>> +			if (mbx & MSSR_SEST)
>> +				break;
>> +			mbx &= MSSR_MBNST;
>> +			mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
>> +			/* Bits SENTDATA and TRMREQ cannot be
>> +			 * set to 0 simultaneously
>> +			 */
>> +			mctl &= ~MCTL_TRMREQ;
>> +			rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
>> +			mctl &= ~MCTL_SENTDATA;
>> +			/* Clear interrupt */
>> +			rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);

> Can you combine both writes to RCAR_CAN_MCTL(mbx) or does the hardware
> require two separate writes?

    Yes, it does. Haven't you seen the comment above the writes?

>> +			ie_mask |= BIT(mbx - FIRST_TX_MB);
>> +			stats->tx_bytes += can_get_echo_skb(ndev,
>> +							    mbx - FIRST_TX_MB);

> Can you guarantee that you call can_get_echo_skb in the same order the
> frames get send?

    Another good question...

[...]
>> +		/* Disable mailbox interrupt, mark mailbox as free */
>> +		if (ie_mask) {
>> +			u32 mier1;
>> +
>> +			spin_lock(&priv->mier_lock);
>> +			mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
>> +			rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 & ~ie_mask);
>> +			spin_unlock(&priv->mier_lock);
>> +			if (unlikely(netif_queue_stopped(ndev)))
>> +				netif_wake_queue(ndev);

> You can call netif_wake_queue() unconditionally, it does the check for
> queue stopped anyway.

    Indeed.

>> +		}
>> +	}
>> +	if (isr & ISR_RXM1F) {
>> +		if (napi_schedule_prep(&priv->napi)) {
>> +			/* Disable Rx interrupts */
>> +			rcar_can_writeb(priv, RCAR_CAN_IER,
>> +					rcar_can_readb(priv, RCAR_CAN_IER) &
>> +						       ~IER_RXM1IE);
>> +			__napi_schedule(&priv->napi);
>> +		}
>> +	}
>> +	return IRQ_HANDLED;

> Do not return IRQ_HANDLED unconditionally. You should return IRQ_HANDLED
> only if there really was a interrupt for your device.

    Yes, people have already complained about this. I'll add a check of the 
interrupt status.

>> +static int rcar_can_set_bittiming(struct net_device *dev)
>> +{
>> +	struct rcar_can_priv *priv = netdev_priv(dev);
>> +	struct can_bittiming *bt = &priv->can.bittiming;
>> +	u32 bcr;
>> +	u16 ctlr;
>> +	u8 clkr;
>> +
>> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
>> +	if (ctlr & CTLR_SLPM) {
>> +		/* Write to BCR in CAN reset mode or CAN halt mode */
>> +		return -EBUSY;

> The framework guarantees that set_bittiming is only called when the
> interface is down.

    Exactly. With the current driver code we get rcar_can_set_bittiming() 
called when device is not opened. It is in the sleep mode and we can not touch 
CTLR register in the sleep mode. I call this function when opening device 
explicitly.

[...]
>> +static void rcar_can_start(struct net_device *ndev)
>> +{
>> +	struct rcar_can_priv *priv = netdev_priv(ndev);
>> +	u16 ctlr, n;
>> +
>> +	/* Set controller to known mode:
>> +	 * - normal mailbox mode (no FIFO);

> Does the controller support FIFO?

    Yes.

[...]

>> +static netdev_tx_t rcar_can_start_xmit(struct sk_buff *skb,
>> +				       struct net_device *ndev)
>> +{
[...]
>> +	spin_lock_irqsave(&priv->mier_lock, flags);
>> +	mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
>> +	if (unlikely(mier1 == ~0U)) {
>> +		spin_unlock_irqrestore(&priv->mier_lock, flags);
>> +		netif_stop_queue(ndev);
>> +		return NETDEV_TX_BUSY;
>> +	}
>> +	rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 | (mier1 + 1));
>> +	spin_unlock_irqrestore(&priv->mier_lock, flags);
>> +	mbxno = ffz(mier1) + FIRST_TX_MB;
>
> This smells fishy, for several reasons:
> The driver should guarantee that the frames are send in the same order
> as this function is called. If you pick the first free mailbox I suspect
> the hardware send the first mailbox ready to send. I don't know the
> hardware, but several CAN controllers I've worked with, function in that
> way.

    There are two TX priority modes: ID priority transmit mode and mailbox 
number transmit mode. Currently, ID priority mode is selected -- I didn't 
realize that "the driver should guarantee that the frames are send in the same 
order as this function is called". Still driver can work in the mailbox number 
priority mode.
    ffz(mier1) selects first free mailbox with lowest number. It will have 
higher priority.

> Then you should rethink your flow control. You should call
> netif_stop_queue if all your hardware buffers are full, then the above
> NETDEV_TX_BUSY should not happen.

    You mean that I should check the state of mailboxes at end of this 
function? OK.

[...]
>> +	mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbxno));

> Do you have to read mctl here?

    Hm, possibly not. Need to re-check...

[...]
>> +static void rcar_can_rx_pkt(struct rcar_can_priv *priv, int mbx)
>> +{
[...]
>> +	data = rcar_can_mbx_readl(priv, mbx, MBX_HDR_OFFSET);
>> +	if (data & RCAR_CAN_IDE)
>> +		cf->can_id = (data & CAN_EFF_MASK) | CAN_EFF_FLAG;
>> +	else
>> +		cf->can_id = (data >> RCAR_CAN_SID_SHIFT) & CAN_SFF_MASK;
>> +	if (data & RCAR_CAN_RTR)
>> +		cf->can_id |= CAN_RTR_FLAG;
>> +
>> +	dlc = rcar_can_mbx_readb(priv, mbx, MBX_DLC_OFFSET);
>> +	cf->can_dlc = get_can_dlc(dlc);

> Please don't copy data on received RTR frames.

    OK.

>> +	for (dlc = 0; dlc < cf->can_dlc; dlc++)
>> +		cf->data[dlc] = rcar_can_mbx_readb(priv, mbx,
>> +						   MBX_DATA_OFFSET + dlc);
[...]
>> +static int rcar_can_rx_poll(struct napi_struct *napi, int quota)
>> +{
>> +	struct rcar_can_priv *priv = container_of(napi,
>> +						  struct rcar_can_priv, napi);
>> +	u32 num_pkts = 0;

> please make it an int

    OK, overlooked this.

>> +
>> +	/* Find mailbox */
>> +	while (1) {

> please put the quota check into the while()

    OK.

>> +		u8 mctl, mbx;
>> +
>> +		mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
>> +		if (mbx & MSSR_SEST || num_pkts >= quota)
>> +			break;
>> +		mbx &= MSSR_MBNST;
>> +		mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
>> +		/* Clear interrupt */
>> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx),
>> +				mctl & ~MCTL_NEWDATA);
>> +		rcar_can_rx_pkt(priv, mbx);

> Which instruction reenables the mailbox?

    Good question. It's not clear from documentation. For sure we can toggle 
RECREQ bit. My current understanding is: mailbox is not ready to receive while 
MCTL.NEWDATA is set. If so, need to clear interrupt after receive. May need to 
check MSGLOST flag -- need to find out how to report overwritten messages.

>> +	/* All packets processed */
>> +	if (num_pkts < quota) {
>> +		u8 ier;
>> +
>> +		napi_complete(napi);
>> +		ier = rcar_can_readb(priv, RCAR_CAN_IER);
>> +		rcar_can_writeb(priv, RCAR_CAN_IER, ier | IER_RXM1IE);

> I the hardware doesn't modify the IER register, you can work on a shadow
> copy and only write, but never need to read from the hardware.

    OK.

[...]
>> +static int rcar_can_probe(struct platform_device *pdev)
>> +{
[...]
>> +	pdata = pdev->dev.platform_data;

> please use dev_get_platdata()

    OK. I saw the patches converting the drivers to using this helper.

>> +	clk_enable(priv->clk);

> You are missing the clock's prepare step.

    We are not supporting common clock framework yet, so this step should be a 
NOP AFAIU.

> You should call
> clk_prepare_enable(). Please move clock_prepare_enable() to the open()
> (and the disable_unprepare() to the close() function) so tha the core is
> not powered as long as the interface is down. You might have to enable
> the clock in the rcar_can_get_berr_counter() as it may be called if the
> interface is still down.

    OK, will do.

>> +
>> +	ndev->netdev_ops = &rcar_can_netdev_ops;
>> +	ndev->irq = irq;
>> +	ndev->flags |= IFF_ECHO;
>> +	priv->ndev = ndev;
>> +	priv->reg_base = addr;
>> +	priv->clock_select = pdata->clock_select;
>> +	priv->can.clock.freq = clk_get_rate(priv->clk);
>> +	priv->can.bittiming_const = &rcar_can_bittiming_const;
>> +	priv->can.do_set_bittiming = rcar_can_set_bittiming;

> No need to set this callback, as you're calling rcar_can_set_bittiming
> explizidly.

    OK.

>> +	priv->can.do_set_mode = rcar_can_do_set_mode;
>> +	priv->can.do_get_berr_counter = rcar_can_get_berr_counter;
>> +	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES |
>> +				       CAN_CTRLMODE_ONE_SHOT;

> You don't handle 3_SAMPLES. Have you tested your driver in ONE_SHOT

    I've tested in ONE_SHOT but the controller was forced into that mode.
Didn't find application that sets this mode.

> mode? How does the controller react if a frame cannot be send? Do you
> clear the skb that has previously been can_put_echo_skb()?

    Another thing to look at -- maybe related to bus-off question.

>> +static int rcar_can_remove(struct platform_device *pdev)
>> +{
>> +	struct net_device *ndev = platform_get_drvdata(pdev);
>> +	struct rcar_can_priv *priv = netdev_priv(ndev);
>> +	u16 ctlr;
>> +
>> +	unregister_candev(ndev);
>> +	netif_napi_del(&priv->napi);
>> +	/* Go to sleep mode */
>> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
>> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr | CTLR_SLPM);

> You should put the controller in to sleep mode in close()

    Seems OK to do this.

[...]
>> Index: linux-can-next/include/linux/can/platform/rcar_can.h
>> ===================================================================
>> --- /dev/null
>> +++ linux-can-next/include/linux/can/platform/rcar_can.h
>> @@ -0,0 +1,15 @@
>> +#ifndef _CAN_PLATFORM_RCAR_CAN_H_
>> +#define _CAN_PLATFORM_RCAR_CAN_H_
>> +
>> +#include <linux/types.h>
>> +
>> +/* Clock Select Register settings */
>> +#define CLKR_CLKEXT	3	/* Externally input clock */
>> +#define CLKR_CLKP2	1	/* Peripheral clock (clkp2) */
>> +#define CLKR_CLKP1	0	/* Peripheral clock (clkp1) */

> Can this be handled by the clock framework

    No.

> and or Device Tree?

    Yes.

> Marc

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sergei Shtylyov Oct. 17, 2013, 10:16 p.m. UTC | #8

Hello.

On 10/02/2013 10:09 AM, Wolfgang Grandegger wrote:

    Sorry for the belated reply -- was on vacations.

> thanks for your contribution. The patch looks already quite good. Before
> I find time for a detailed review could you please check error handling
> and bus-off recovery by reporting the output of "$ candump -td -e
> any,0:0,#FFFFFFFF" while sending messages to the device ...

> 1. ... without cable connected

Terminal 1:

root@10.0.0.101:/opt/can-utils# ./cangen -n 1 -g 1 can0
root@10.0.0.101:/opt/can-utils#

Terminal 2:

root@10.0.0.101:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
(000.000000) can0 200000AC [8] 00 08 00 19 00 00 00 00 ERRORFRAME
controller-problem{tx-error-warning}
protocol-violation{{}{acknowledge-slot}}
no-acknowledgement-on-tx
bus-error
(000.004496) can0 20000004 [8] 00 20 00 00 00 00 00 00 ERRORFRAME
controller-problem{tx-error-passive}

So we get and stay in error- passive state:

root@10.0.0.101:/opt/can-utils# ip -details link show can0
2: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN qlen 
10 link/can
can state ERROR-PASSIVE (berr-counter tx 128 rx 0) restart-ms 0
bitrate 297619 sample-point 0.714
tq 480 prop-seg 2 phase-seg1 2 phase-seg2 2 sjw 1
rcar_can: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..1024 brp-inc 1
clock 49999999
root@10.0.0.101:/opt/can-utils#

dmesg:
rcar_can rcar_can.0 can0: bitrate error 0.7%
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: ACK Error
rcar_can rcar_can.0 can0: Error passive interrupt

> 2. ... with short-circuited CAN high and low and doing some time later
>         a manual recovery with "ip link set can0 type can restart"

    Now we have auto recovery only. Manual recovery was tested with the first 
driver version and worked.

Terminal 1:

root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
root@10.0.0.104:/opt/can-utils#

Terminal 2:

root@10.0.0.104:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
(000.000000) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
controller-problem{}
protocol-violation{{tx-dominant-bit-error}{}}
bus-error
(000.021147) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
controller-problem{}
bus-off
restarted-after-bus-off
(011.738522) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
controller-problem{}
protocol-violation{{tx-dominant-bit-error}{}}
bus-error
(000.021163) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
controller-problem{}
bus-off
restarted-after-bus-off
(001.666625) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
controller-problem{}
protocol-violation{{tx-dominant-bit-error}{}}
bus-error
(000.021157) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
controller-problem{}
bus-off
restarted-after-bus-off

dmesg:
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: Bit Error (dominant)
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus-off entry interrupt
rcar_can rcar_can.0 can0: bus-off
rcar_can rcar_can.0 can0: Bus-off recovery interrupt
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: Bit Error (dominant)
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus-off entry interrupt
rcar_can rcar_can.0 can0: bus-off
rcar_can rcar_can.0 can0: Bus-off recovery interrupt
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: Bit Error (dominant)
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus-off entry interrupt
rcar_can rcar_can.0 can0: bus-off
rcar_can rcar_can.0 can0: Bus-off recovery interrupt

> I also wonder if the messages are always sent in order. You could use
> the program "canfdtest" [1] from the can-utils for validation.

    This program is PITA. With the driver workaroung it works:

FULL Duplex test:
on DUT:
root@10.0.0.104:/opt/can-utils# ./canfdtest -v can0
interface = can0, family = 29, type = 3, proto = 1
............................

root@am335x-evm:~# canconfig can0 bitrate 1000000
can0 bitrate: 1000000, sample-point: 0.750
root@am335x-evm:~# canconfig can0 start
[50164.566650] d_can d_can.1: can0: setting CAN BT = 0x2701
can0 state: ERROR-ACTIVE
root@am335x-evm:~# ./canfdtest -v -g can0
interface = can0, family = 29, type = 3, proto = 1
...............................................................................C
Test messages sent and received: 3826567
Exiting...

> Support for CAN_CTRLMODE_BERR_REPORTING would also be nice, especially
> if the bus errors reporting can easily be switched off (via interrupt mask).

   Done.

> There is also the RCAN-TL1 (Renesas CAN Time Trigger Level 1) module
> e.g. available on the SH7203 Group of processors. Is this CAN controller
> compatible to some extend with the R-Car CAN or a completely different
> implementation?

    No, not compatible.

> [1] https://gitorious.org/linux-can/can-utils/source/canfdtest.c

> Wolfgang.

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wolfgang Grandegger Oct. 21, 2013, 7:12 p.m. UTC | #9

Hi Sergei,

On 10/18/2013 12:16 AM, Sergei Shtylyov wrote:
> Hello.
> 
> On 10/02/2013 10:09 AM, Wolfgang Grandegger wrote:
> 
>    Sorry for the belated reply -- was on vacations.
> 
>> thanks for your contribution. The patch looks already quite good. Before
>> I find time for a detailed review could you please check error handling
>> and bus-off recovery by reporting the output of "$ candump -td -e
>> any,0:0,#FFFFFFFF" while sending messages to the device ...
> 
>> 1. ... without cable connected
> 
> Terminal 1:
> 
> root@10.0.0.101:/opt/can-utils# ./cangen -n 1 -g 1 can0
> root@10.0.0.101:/opt/can-utils#
> 
> Terminal 2:
> 
> root@10.0.0.101:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
> (000.000000) can0 200000AC [8] 00 08 00 19 00 00 00 00 ERRORFRAME
> controller-problem{tx-error-warning}
> protocol-violation{{}{acknowledge-slot}}
> no-acknowledgement-on-tx
> bus-error
> (000.004496) can0 20000004 [8] 00 20 00 00 00 00 00 00 ERRORFRAME
> controller-problem{tx-error-passive}
> 
> So we get and stay in error- passive state:

Looks good.

> 
> root@10.0.0.101:/opt/can-utils# ip -details link show can0
> 2: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
> qlen 10 link/can
> can state ERROR-PASSIVE (berr-counter tx 128 rx 0) restart-ms 0
> bitrate 297619 sample-point 0.714

Strange, what bitrate did you configure?

> tq 480 prop-seg 2 phase-seg1 2 phase-seg2 2 sjw 1
> rcar_can: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..1024 brp-inc 1
> clock 49999999

Could you please try if the algorithm works better with 50000000.

> root@10.0.0.101:/opt/can-utils#
> 
> dmesg:
> rcar_can rcar_can.0 can0: bitrate error 0.7%
> rcar_can rcar_can.0 can0: Error warning interrupt
> rcar_can rcar_can.0 can0: Bus error interrupt:
> rcar_can rcar_can.0 can0: ACK Error
> rcar_can rcar_can.0 can0: Error passive interrupt
> 
>> 2. ... with short-circuited CAN high and low and doing some time later
>>         a manual recovery with "ip link set can0 type can restart"
> 
>    Now we have auto recovery only. Manual recovery was tested with the
> first driver version and worked.

What do you mean with "auto recovery"? Auto recovery by the hardware or
via "restart-ms <ms>"? How do you choose between "manual" and "auto"
recovery?

> Terminal 1:
> 
> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
> root@10.0.0.104:/opt/can-utils#
> 
> Terminal 2:
> 
> root@10.0.0.104:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
> (000.000000) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
> controller-problem{}
> protocol-violation{{tx-dominant-bit-error}{}}
> bus-error
> (000.021147) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
> controller-problem{}
> bus-off
> restarted-after-bus-off

Why does it get "restarted" directly after the bus-off?

> (011.738522) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
> controller-problem{}

What controller problem? data[1] is not set for some reasom.

> protocol-violation{{tx-dominant-bit-error}{}}
> bus-error
> (000.021163) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
> controller-problem{}
> bus-off
> restarted-after-bus-off
> (001.666625) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
> controller-problem{}
> protocol-violation{{tx-dominant-bit-error}{}}
> bus-error
> (000.021157) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
> controller-problem{}
> bus-off
> restarted-after-bus-off
> 
> dmesg:
> rcar_can rcar_can.0 can0: Error warning interrupt
> rcar_can rcar_can.0 can0: Error passive interrupt
> rcar_can rcar_can.0 can0: Bus error interrupt:
> rcar_can rcar_can.0 can0: Bit Error (dominant)
> rcar_can rcar_can.0 can0: Error warning interrupt
> rcar_can rcar_can.0 can0: Error passive interrupt

Why are they reported again. You are already in error passive.

> rcar_can rcar_can.0 can0: Bus-off entry interrupt
> rcar_can rcar_can.0 can0: bus-off
> rcar_can rcar_can.0 can0: Bus-off recovery interrupt
> rcar_can rcar_can.0 can0: Error warning interrupt
> rcar_can rcar_can.0 can0: Error passive interrupt
> rcar_can rcar_can.0 can0: Bus error interrupt:
> rcar_can rcar_can.0 can0: Bit Error (dominant)
> rcar_can rcar_can.0 can0: Error warning interrupt
> rcar_can rcar_can.0 can0: Error passive interrupt
> rcar_can rcar_can.0 can0: Bus-off entry interrupt
> rcar_can rcar_can.0 can0: bus-off
> rcar_can rcar_can.0 can0: Bus-off recovery interrupt
> rcar_can rcar_can.0 can0: Error warning interrupt
> rcar_can rcar_can.0 can0: Error passive interrupt
> rcar_can rcar_can.0 can0: Bus error interrupt:
> rcar_can rcar_can.0 can0: Bit Error (dominant)
> rcar_can rcar_can.0 can0: Error warning interrupt
> rcar_can rcar_can.0 can0: Error passive interrupt
> rcar_can rcar_can.0 can0: Bus-off entry interrupt
> rcar_can rcar_can.0 can0: bus-off
> rcar_can rcar_can.0 can0: Bus-off recovery interrupt

>> I also wonder if the messages are always sent in order. You could use
>> the program "canfdtest" [1] from the can-utils for validation.
> 
>    This program is PITA. With the driver workaroung it works:

What workaround?

Wolfgang.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sergei Shtylyov Nov. 9, 2013, 1:02 a.m. UTC | #10

Hello.

On 10/21/2013 11:12 PM, Wolfgang Grandegger wrote:

>>     Sorry for the belated reply -- was on vacations.

    And again sorry, couldn't get to this due to other things.

>>> thanks for your contribution. The patch looks already quite good. Before
>>> I find time for a detailed review could you please check error handling
>>> and bus-off recovery by reporting the output of "$ candump -td -e
>>> any,0:0,#FFFFFFFF" while sending messages to the device ...

[...]

>> root@10.0.0.101:/opt/can-utils# ip -details link show can0
>> 2: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
>> qlen 10 link/can
>> can state ERROR-PASSIVE (berr-counter tx 128 rx 0) restart-ms 0
>> bitrate 297619 sample-point 0.714

> Strange, what bitrate did you configure?

    300000.

>> tq 480 prop-seg 2 phase-seg1 2 phase-seg2 2 sjw 1
>> rcar_can: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..1024 brp-inc 1
>> clock 49999999

> Could you please try if the algorithm works better with 50000000.

    It doesn't. Look at the logs below:

Bitrate setting:
----------------
actual:
300KHz
ip -details link show can0
2: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN qlen 10
link/can
can state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
bitrate 297619 sample-point 0.714
tq 480 prop-seg 2 phase-seg1 2 phase-seg2 2 sjw 1
rcar_can: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..1024 brp-inc 1
clock 49999999

500KHz:
ip -details link show can0
2: can0: <NOARP,ECHO> mtu 16 qdisc pfifo_fast state DOWN qlen 10
link/can
can state STOPPED (berr-counter tx 0 rx 0) restart-ms 0
bitrate 499999 sample-point 0.800
tq 200 prop-seg 3 phase-seg1 4 phase-seg2 2 sjw 1
rcar_can: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..1024 brp-inc 1
clock 49999999

700KHz:
ip -details link show can0
2: can0: <NOARP,ECHO> mtu 16 qdisc pfifo_fast state DOWN qlen 10
link/can
can state STOPPED (berr-counter tx 0 rx 0) restart-ms 0
bitrate 694444 sample-point 0.750
tq 180 prop-seg 2 phase-seg1 3 phase-seg2 2 sjw 1
rcar_can: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..1024 brp-inc 1
clock 49999999

--------------
Forced:
priv->can.clock.freq = 50000000;
300KHz:
ip -details link show can0
2: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN qlen 10
link/can
can state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
bitrate 297619 sample-point 0.714
tq 480 prop-seg 2 phase-seg1 2 phase-seg2 2 sjw 1
rcar_can: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..1024 brp-inc 1
clock 50000000
root@10.0.0.101:/opt#

700KHz:
root@10.0.0.101:/opt# ip -details link show can0
2: can0: <NOARP,ECHO> mtu 16 qdisc pfifo_fast state DOWN qlen 10
link/can
can state STOPPED (berr-counter tx 0 rx 0) restart-ms 0
bitrate 694444 sample-point 0.750
tq 180 prop-seg 2 phase-seg1 3 phase-seg2 2 sjw 1
rcar_can: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..1024 brp-inc 1
clock 50000000

>> root@10.0.0.101:/opt/can-utils#

>> dmesg:
>> rcar_can rcar_can.0 can0: bitrate error 0.7%
>> rcar_can rcar_can.0 can0: Error warning interrupt
>> rcar_can rcar_can.0 can0: Bus error interrupt:
>> rcar_can rcar_can.0 can0: ACK Error
>> rcar_can rcar_can.0 can0: Error passive interrupt

>>> 2. ... with short-circuited CAN high and low and doing some time later
>>>          a manual recovery with "ip link set can0 type can restart"

>>     Now we have auto recovery only. Manual recovery was tested with the
>> first driver version and worked.

> What do you mean with "auto recovery"? Auto recovery by the hardware or
> via "restart-ms <ms>"? How do you choose between "manual" and "auto"
> recovery?

    This exact test was done with hardware auto-recovery only. No "restart-ms" 
was programmed.

>> Terminal 1:

>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>> root@10.0.0.104:/opt/can-utils#

>> Terminal 2:

>> root@10.0.0.104:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
>> (000.000000) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>> controller-problem{}
>> protocol-violation{{tx-dominant-bit-error}{}}
>> bus-error
>> (000.021147) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>> controller-problem{}
>> bus-off
>> restarted-after-bus-off

> Why does it get "restarted" directly after the bus-off?

    Because we have hardware auto-recovery enabled.

>> (011.738522) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>> controller-problem{}

> What controller problem? data[1] is not set for some reasom.

    Not comments. Looking into it.

>> protocol-violation{{tx-dominant-bit-error}{}}
>> bus-error
>> (000.021163) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>> controller-problem{}
>> bus-off
>> restarted-after-bus-off
>> (001.666625) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>> controller-problem{}
>> protocol-violation{{tx-dominant-bit-error}{}}
>> bus-error
>> (000.021157) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>> controller-problem{}
>> bus-off
>> restarted-after-bus-off

>> dmesg:
>> rcar_can rcar_can.0 can0: Error warning interrupt
>> rcar_can rcar_can.0 can0: Error passive interrupt
>> rcar_can rcar_can.0 can0: Bus error interrupt:
>> rcar_can rcar_can.0 can0: Bit Error (dominant)
>> rcar_can rcar_can.0 can0: Error warning interrupt
>> rcar_can rcar_can.0 can0: Error passive interrupt

> Why are they reported again. You are already in error passive.

    Don't know. :-/

[...]

>>> I also wonder if the messages are always sent in order. You could use
>>> the program "canfdtest" [1] from the can-utils for validation.

>>     This program is PITA. With the driver workaroung it works:

> What workaround?

    Doesn't matter already, got rid of it.

> Wolfgang.

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wolfgang Grandegger Nov. 9, 2013, 10:53 a.m. UTC | #11

Hi Sergei,

On 11/09/2013 02:02 AM, Sergei Shtylyov wrote:
> Hello.
> 
> On 10/21/2013 11:12 PM, Wolfgang Grandegger wrote:
> 
>>>     Sorry for the belated reply -- was on vacations.
> 
>    And again sorry, couldn't get to this due to other things.
> 
>>>> thanks for your contribution. The patch looks already quite good.
>>>> Before
>>>> I find time for a detailed review could you please check error handling
>>>> and bus-off recovery by reporting the output of "$ candump -td -e
>>>> any,0:0,#FFFFFFFF" while sending messages to the device ...
> 
> [...]
> 
>>> root@10.0.0.101:/opt/can-utils# ip -details link show can0
>>> 2: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
>>> qlen 10 link/can
>>> can state ERROR-PASSIVE (berr-counter tx 128 rx 0) restart-ms 0
>>> bitrate 297619 sample-point 0.714
> 
>> Strange, what bitrate did you configure?
> 
>    300000.

Ah, OK. It's just a very unusual CAN bitrate. Common are 125k, 250k,
500kB, 800kB and 1 MBit/s. Is it your choice?

>>> tq 480 prop-seg 2 phase-seg1 2 phase-seg2 2 sjw 1
>>> rcar_can: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..1024 brp-inc 1
>>> clock 49999999
> 
>> Could you please try if the algorithm works better with 50000000.
> 
>    It doesn't. Look at the logs below:

OK, I was mainly confused by the bitrate. Anyway, the bitrate algorithim
sometimes does not like exotic clock frequencies or bitrates. Then
manual setting of the bit-timing parameters might be necessary. But that
seem not the case here.

>>>> 2. ... with short-circuited CAN high and low and doing some time later
>>>>          a manual recovery with "ip link set can0 type can restart"
> 
>>>     Now we have auto recovery only. Manual recovery was tested with the
>>> first driver version and worked.
> 
>> What do you mean with "auto recovery"? Auto recovery by the hardware or
>> via "restart-ms <ms>"? How do you choose between "manual" and "auto"
>> recovery?
> 
>    This exact test was done with hardware auto-recovery only. No
> "restart-ms" was programmed.

OK, you already explained that in another mail and your driver does not
use/support hardware auto-recovery any longer.

> 
>>> Terminal 1:
> 
>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>> root@10.0.0.104:/opt/can-utils#
> 
>>> Terminal 2:
> 
>>> root@10.0.0.104:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
>>> (000.000000) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>>> controller-problem{}
>>> protocol-violation{{tx-dominant-bit-error}{}}
>>> bus-error
>>> (000.021147) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>>> controller-problem{}
>>> bus-off
>>> restarted-after-bus-off
> 
>> Why does it get "restarted" directly after the bus-off?
> 
>    Because we have hardware auto-recovery enabled.
> 
>>> (011.738522) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>>> controller-problem{}
> 
>> What controller problem? data[1] is not set for some reasom.
> 
>    Not comments. Looking into it.
> 
>>> protocol-violation{{tx-dominant-bit-error}{}}
>>> bus-error
>>> (000.021163) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>>> controller-problem{}
>>> bus-off
>>> restarted-after-bus-off
>>> (001.666625) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>>> controller-problem{}
>>> protocol-violation{{tx-dominant-bit-error}{}}
>>> bus-error
>>> (000.021157) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>>> controller-problem{}
>>> bus-off
>>> restarted-after-bus-off
> 
>>> dmesg:
>>> rcar_can rcar_can.0 can0: Error warning interrupt
>>> rcar_can rcar_can.0 can0: Error passive interrupt
>>> rcar_can rcar_can.0 can0: Bus error interrupt:
>>> rcar_can rcar_can.0 can0: Bit Error (dominant)
>>> rcar_can rcar_can.0 can0: Error warning interrupt
>>> rcar_can rcar_can.0 can0: Error passive interrupt
> 
>> Why are they reported again. You are already in error passive.
> 
>    Don't know. :-/

The hardware might not be that smart. Then the software should care.

>>>> I also wonder if the messages are always sent in order. You could use
>>>> the program "canfdtest" [1] from the can-utils for validation.
> 
>>>     This program is PITA. With the driver workaroung it works:
> 
>> What workaround?
> 
>    Doesn't matter already, got rid of it.

OK. BTW: I suggest to run "canfdtest" at *1* MB/s with additional system
and I/O load and for much longer than a minute to increase the
probability of an out-of-order transmissions to occur.

Wolfgang.

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wolfgang Grandegger Nov. 9, 2013, 2:47 p.m. UTC | #12

On 11/09/2013 11:53 AM, Wolfgang Grandegger wrote:
> Hi Sergei,
> 
> On 11/09/2013 02:02 AM, Sergei Shtylyov wrote:
>> Hello.
>>
>> On 10/21/2013 11:12 PM, Wolfgang Grandegger wrote:
>>
>>>>     Sorry for the belated reply -- was on vacations.
>>
>>    And again sorry, couldn't get to this due to other things.
>>
>>>>> thanks for your contribution. The patch looks already quite good.
>>>>> Before
>>>>> I find time for a detailed review could you please check error handling
>>>>> and bus-off recovery by reporting the output of "$ candump -td -e
>>>>> any,0:0,#FFFFFFFF" while sending messages to the device ...
>>
>> [...]
>>
>>>> root@10.0.0.101:/opt/can-utils# ip -details link show can0
>>>> 2: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
>>>> qlen 10 link/can
>>>> can state ERROR-PASSIVE (berr-counter tx 128 rx 0) restart-ms 0
>>>> bitrate 297619 sample-point 0.714
>>
>>> Strange, what bitrate did you configure?
>>
>>    300000.
> 
> Ah, OK. It's just a very unusual CAN bitrate. Common are 125k, 250k,
> 500kB, 800kB and 1 MBit/s. Is it your choice?
> 
>>>> tq 480 prop-seg 2 phase-seg1 2 phase-seg2 2 sjw 1
>>>> rcar_can: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..1024 brp-inc 1
>>>> clock 49999999
>>
>>> Could you please try if the algorithm works better with 50000000.
>>
>>    It doesn't. Look at the logs below:
> 
> OK, I was mainly confused by the bitrate. Anyway, the bitrate algorithim
> sometimes does not like exotic clock frequencies or bitrates. Then
> manual setting of the bit-timing parameters might be necessary. But that
> seem not the case here.
> 
>>>>> 2. ... with short-circuited CAN high and low and doing some time later
>>>>>          a manual recovery with "ip link set can0 type can restart"
>>
>>>>     Now we have auto recovery only. Manual recovery was tested with the
>>>> first driver version and worked.
>>
>>> What do you mean with "auto recovery"? Auto recovery by the hardware or
>>> via "restart-ms <ms>"? How do you choose between "manual" and "auto"
>>> recovery?
>>
>>    This exact test was done with hardware auto-recovery only. No
>> "restart-ms" was programmed.
> 
> OK, you already explained that in another mail and your driver does not
> use/support hardware auto-recovery any longer.
> 
>>
>>>> Terminal 1:
>>
>>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>>> root@10.0.0.104:/opt/can-utils#
>>
>>>> Terminal 2:
>>
>>>> root@10.0.0.104:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
>>>> (000.000000) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>>>> controller-problem{}
>>>> protocol-violation{{tx-dominant-bit-error}{}}
>>>> bus-error
>>>> (000.021147) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>>>> controller-problem{}
>>>> bus-off
>>>> restarted-after-bus-off
>>
>>> Why does it get "restarted" directly after the bus-off?
>>
>>    Because we have hardware auto-recovery enabled.
>>
>>>> (011.738522) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>>>> controller-problem{}
>>
>>> What controller problem? data[1] is not set for some reasom.
>>
>>    Not comments. Looking into it.
>>
>>>> protocol-violation{{tx-dominant-bit-error}{}}
>>>> bus-error
>>>> (000.021163) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>>>> controller-problem{}
>>>> bus-off
>>>> restarted-after-bus-off
>>>> (001.666625) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>>>> controller-problem{}
>>>> protocol-violation{{tx-dominant-bit-error}{}}
>>>> bus-error
>>>> (000.021157) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>>>> controller-problem{}
>>>> bus-off
>>>> restarted-after-bus-off
>>
>>>> dmesg:
>>>> rcar_can rcar_can.0 can0: Error warning interrupt
>>>> rcar_can rcar_can.0 can0: Error passive interrupt
>>>> rcar_can rcar_can.0 can0: Bus error interrupt:
>>>> rcar_can rcar_can.0 can0: Bit Error (dominant)
>>>> rcar_can rcar_can.0 can0: Error warning interrupt
>>>> rcar_can rcar_can.0 can0: Error passive interrupt
>>
>>> Why are they reported again. You are already in error passive.
>>
>>    Don't know. :-/
> 
> The hardware might not be that smart. Then the software should care.
> 
>>>>> I also wonder if the messages are always sent in order. You could use
>>>>> the program "canfdtest" [1] from the can-utils for validation.
>>
>>>>     This program is PITA. With the driver workaroung it works:
>>
>>> What workaround?
>>
>>    Doesn't matter already, got rid of it.
> 
> OK. BTW: I suggest to run "canfdtest" at *1* MB/s with additional system
> and I/O load and for much longer than a minute to increase the
> probability of an out-of-order transmissions to occur.

That's probably a wrong assumption because "canfdtest" does do a 1ms
sleep after each generated messages :(. Therefore I would try at 125
KBit/s. Sorry for not providing a reliable tool for out-of-order
validation. There is also the "cansequence" program from the Pengutronix
canutils [1], which might be better suited, also to reveal races.

Wolfgang.

[1] http://git.pengutronix.de/?p=tools/canutils.git;a=summary
    http://www.pengutronix.de/software/socket-can/download

Wolfgang.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sergei Shtylyov Nov. 12, 2013, 9:45 p.m. UTC | #13

Hello.

On 11/09/2013 04:02 AM, Sergei Shtylyov wrote:

>>>> 2. ... with short-circuited CAN high and low and doing some time later
>>>>          a manual recovery with "ip link set can0 type can restart"

>>>     Now we have auto recovery only. Manual recovery was tested with the
>>> first driver version and worked.

>> What do you mean with "auto recovery"? Auto recovery by the hardware or
>> via "restart-ms <ms>"? How do you choose between "manual" and "auto"
>> recovery?

>     This exact test was done with hardware auto-recovery only. No "restart-ms"
> was programmed.

>>> Terminal 1:

>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>> root@10.0.0.104:/opt/can-utils#

>>> Terminal 2:

>>> root@10.0.0.104:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
>>> (000.000000) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>>> controller-problem{}
>>> protocol-violation{{tx-dominant-bit-error}{}}
>>> bus-error
>>> (000.021147) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>>> controller-problem{}
>>> bus-off
>>> restarted-after-bus-off

>> Why does it get "restarted" directly after the bus-off?

>     Because we have hardware auto-recovery enabled.

>>> (011.738522) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>>> controller-problem{}

>> What controller problem? data[1] is not set for some reasom.

>     Not comments. Looking into it.

    Sorry, this has been fixed a while ago. Now the log looks like:

root@10.0.0.104:/opt/can-utils#  ./candump -td -e any,0:0,#FFFFFFFF
  (000.000000)  can0  2000008C   [8]  00 28 08 00 00 00 88 00 ERRORFRAME
         controller-problem{tx-error-warning,tx-error-passive}
         protocol-violation{{tx-dominant-bit-error}{}}
         bus-error
         error-counter-tx-rx{{136}{0}}
  (000.007578)  can0  20000040   [8]  00 00 00 00 00 00 00 00 ERRORFRAME
         bus-off
  (000.091847)  can0  20000100   [8]  00 00 00 00 00 00 00 00 ERRORFRAME
         restarted-after-bus-off
  (056.136722)  can0  2000008C   [8]  00 28 08 00 00 00 88 00 ERRORFRAME
         controller-problem{tx-error-warning,tx-error-passive}
         protocol-violation{{tx-dominant-bit-error}{}}
         bus-error
         error-counter-tx-rx{{136}{0}}

>>> dmesg:
>>> rcar_can rcar_can.0 can0: Error warning interrupt
>>> rcar_can rcar_can.0 can0: Error passive interrupt
>>> rcar_can rcar_can.0 can0: Bus error interrupt:
>>> rcar_can rcar_can.0 can0: Bit Error (dominant)
>>> rcar_can rcar_can.0 can0: Error warning interrupt
>>> rcar_can rcar_can.0 can0: Error passive interrupt

>> Why are they reported again. You are already in error passive.

>     Don't know. :-/

    This also has been dealt with. Here's an example:

rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: ACK Error
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: ACK Error
rcar_can rcar_can.0 can0: Error passive interrupt

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wolfgang Grandegger Nov. 12, 2013, 10:17 p.m. UTC | #14

Hi Sergei,

On 11/12/2013 10:45 PM, Sergei Shtylyov wrote:
> Hello.
> 
> On 11/09/2013 04:02 AM, Sergei Shtylyov wrote:
> 
>>>>> 2. ... with short-circuited CAN high and low and doing some time later
>>>>>          a manual recovery with "ip link set can0 type can restart"
> 
>>>>     Now we have auto recovery only. Manual recovery was tested with the
>>>> first driver version and worked.
> 
>>> What do you mean with "auto recovery"? Auto recovery by the hardware or
>>> via "restart-ms <ms>"? How do you choose between "manual" and "auto"
>>> recovery?
> 
>>     This exact test was done with hardware auto-recovery only. No
>> "restart-ms"
>> was programmed.
> 
>>>> Terminal 1:
> 
>>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>>> root@10.0.0.104:/opt/can-utils#
> 
>>>> Terminal 2:
> 
>>>> root@10.0.0.104:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
>>>> (000.000000) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>>>> controller-problem{}
>>>> protocol-violation{{tx-dominant-bit-error}{}}
>>>> bus-error
>>>> (000.021147) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>>>> controller-problem{}
>>>> bus-off
>>>> restarted-after-bus-off
> 
>>> Why does it get "restarted" directly after the bus-off?
> 
>>     Because we have hardware auto-recovery enabled.
> 
>>>> (011.738522) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>>>> controller-problem{}
> 
>>> What controller problem? data[1] is not set for some reasom.
> 
>>     Not comments. Looking into it.
> 
>    Sorry, this has been fixed a while ago. Now the log looks like:
> 
> root@10.0.0.104:/opt/can-utils#  ./candump -td -e any,0:0,#FFFFFFFF
>  (000.000000)  can0  2000008C   [8]  00 28 08 00 00 00 88 00 ERRORFRAME
>         controller-problem{tx-error-warning,tx-error-passive}

Only the highest level should be reported.

>         protocol-violation{{tx-dominant-bit-error}{}}
>         bus-error
>         error-counter-tx-rx{{136}{0}}
>  (000.007578)  can0  20000040   [8]  00 00 00 00 00 00 00 00 ERRORFRAME
>         bus-off
>  (000.091847)  can0  20000100   [8]  00 00 00 00 00 00 00 00 ERRORFRAME
>         restarted-after-bus-off
>  (056.136722)  can0  2000008C   [8]  00 28 08 00 00 00 88 00 ERRORFRAME
>         controller-problem{tx-error-warning,tx-error-passive}

Ditto.

Wolfgang.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

Index: linux-can-next/drivers/net/can/Kconfig
===================================================================
--- linux-can-next.orig/drivers/net/can/Kconfig
+++ linux-can-next/drivers/net/can/Kconfig
@@ -125,6 +125,15 @@  config CAN_GRCAN
 	  endian syntheses of the cores would need some modifications on
 	  the hardware level to work.
 
+config CAN_RCAR
+	tristate "Renesas R-Car CAN controller"
+	---help---
+	  Say Y here if you want to use CAN controller found on Renesas R-Car
+	  SoCs.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called rcar_can.
+
 source "drivers/net/can/mscan/Kconfig"
 
 source "drivers/net/can/sja1000/Kconfig"
Index: linux-can-next/drivers/net/can/Makefile
===================================================================
--- linux-can-next.orig/drivers/net/can/Makefile
+++ linux-can-next/drivers/net/can/Makefile
@@ -25,5 +25,6 @@  obj-$(CONFIG_CAN_JANZ_ICAN3)	+= janz-ica
 obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
 obj-$(CONFIG_PCH_CAN)		+= pch_can.o
 obj-$(CONFIG_CAN_GRCAN)		+= grcan.o
+obj-$(CONFIG_CAN_RCAR)		+= rcar_can.o
 
 ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
Index: linux-can-next/drivers/net/can/rcar_can.c
===================================================================
--- /dev/null
+++ linux-can-next/drivers/net/can/rcar_can.c
@@ -0,0 +1,898 @@ 
+/*
+ * Renesas R-Car CAN device driver
+ *
+ * Copyright (C) 2013 Cogent Embedded, Inc. <[e-mail]>
+ * Copyright (C) 2013 Renesas Solutions 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/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/can/platform/rcar_can.h>
+
+#define DRV_NAME	"rcar_can"
+
+#define RCAR_CAN_MIER1	0x42C	/* CANi Mailbox Interrupt Enable Register 1 */
+#define RCAR_CAN_MKR(n)	((n) < 2 ? 0x430 + 4 * (n) : 0x400 + 4 * ((n) - 2))
+				/* CANi Mask Register */
+#define RCAR_CAN_MKIVLR0 0x438	/* CANi Mask Invalid Register 0 */
+#define RCAR_CAN_MIER0	 0x43C	/* CANi Mailbox Interrupt Enable Register 0 */
+
+#define RCAR_CAN_MCTL(n) (0x800 + (n)) /* CANi Message Control Register */
+#define RCAR_CAN_CTLR	0x840	/* CANi Control Register */
+#define RCAR_CAN_STR	0x842	/* CANi Status Register */
+#define RCAR_CAN_BCR	0x844	/* CANi Bit Configuration Register */
+#define RCAR_CAN_CLKR	0x847	/* CANi Clock Select Register */
+#define RCAR_CAN_EIER	0x84C	/* CANi Error Interrupt Enable Register */
+#define RCAR_CAN_EIFR	0x84D	/* CANi Err Interrupt Factor Judge Register */
+#define RCAR_CAN_RECR	0x84E	/* CANi Receive Error Count Register */
+#define RCAR_CAN_TECR	0x84F	/* CANi Transmit Error Count Register */
+#define RCAR_CAN_ECSR	0x850	/* CANi Error Code Store Register */
+#define RCAR_CAN_MSSR	0x852	/* CANi Mailbox Search Status Register */
+#define RCAR_CAN_MSMR	0x853	/* CANi Mailbox Search Mode Register */
+#define RCAR_CAN_TCR	0x858	/* CANi Test Control Register */
+#define RCAR_CAN_IER	0x860	/* CANi Interrupt Enable Register */
+#define RCAR_CAN_ISR	0x861	/* CANi Interrupt Status Register */
+
+/* Offsets of RCAR_CAN Mailbox Registers */
+#define MBX_HDR_OFFSET	0x0
+#define MBX_DLC_OFFSET	0x5
+#define MBX_DATA_OFFSET	0x6
+
+#define RCAR_CAN_MBX_SIZE 0x10
+
+/* Control Register bits */
+#define CTLR_SLPM	BIT(10)
+#define CTLR_HALT	BIT(9)
+#define CTLR_RESET	BIT(8)
+#define CTLR_FORCE_RESET (3 << 8)
+#define CTLR_TPM	BIT(4)	/* Transmission Priority Mode Select Bit */
+#define CTLR_IDFM_MIXED	BIT(2)	/* Mixed ID mode */
+
+/* Message Control Register bits */
+#define MCTL_TRMREQ	BIT(7)
+#define MCTL_RECREQ	BIT(6)
+#define MCTL_ONESHOT	BIT(4)
+#define MCTL_SENTDATA	BIT(0)
+#define MCTL_NEWDATA	BIT(0)
+
+#define N_RX_MKREGS	2	/* Number of mask registers */
+				/* for Rx mailboxes 0-31 */
+
+/* Bit Configuration Register settings */
+#define BCR_TSEG1(x)	(((x) & 0x0f) << 28)
+#define BCR_BPR(x)	(((x) & 0x3ff) << 16)
+#define BCR_SJW(x)	(((x) & 0x3) << 12)
+#define BCR_TSEG2(x)	(((x) & 0x07) << 8)
+
+/* Mailbox and Mask Registers bits */
+#define RCAR_CAN_IDE	BIT(31)
+#define RCAR_CAN_RTR	BIT(30)
+#define RCAR_CAN_SID_SHIFT 18
+
+/* Interrupt Enable Register bits */
+#define IER_ERSIE	BIT(5)	/* Error (ERS) Interrupt Enable Bit */
+#define IER_RXM0IE	BIT(2)	/* Mailbox 0 Successful Reception (RXM0) */
+				/* Interrupt Enable Bit */
+#define IER_RXM1IE	BIT(1)	/* Mailbox 1 Successful Reception (RXM0) */
+				/* Interrupt Enable Bit */
+#define IER_TXMIE	BIT(0)	/* Mailbox 32 to 63 Successful Tx */
+				/* Interrupt Enable Bit */
+
+/* Interrupt Status Register bits */
+#define ISR_ERSF	BIT(5)	/* Error (ERS) Interrupt Status Bit */
+#define ISR_RXM0F	BIT(2)	/* Mailbox 0 Successful Reception (RXM0) */
+				/* Interrupt Status Bit */
+#define ISR_RXM1F	BIT(1)	/* Mailbox 1 to 63 Successful Reception */
+				/* (RXM1) Interrupt Status Bit */
+#define ISR_TXMF	BIT(0)	/* Mailbox 32 to 63 Successful Transmission */
+				/* (TXM) Interrupt Status Bit */
+
+/* Error Interrupt Enable Register bits */
+#define EIER_BLIE	BIT(7)	/* Bus Lock Interrupt Enable */
+#define EIER_OLIE	BIT(6)	/* Overload Frame Transmit Interrupt Enable */
+#define EIER_ORIE	BIT(5)	/* Receive Overrun Interrupt Enable */
+#define EIER_BORIE	BIT(4)	/* Bus-Off Recovery Interrupt Enable */
+
+#define EIER_BOEIE	BIT(3)	/* Bus-Off Entry Interrupt Enable */
+#define EIER_EPIE	BIT(2)	/* Error Passive Interrupt Enable */
+#define EIER_EWIE	BIT(1)	/* Error Warning Interrupt Enable */
+#define EIER_BEIE	BIT(0)	/* Bus Error Interrupt Enable */
+
+/* Error Interrupt Factor Judge Register bits */
+#define EIFR_BLIF	BIT(7)	/* Bus Lock Detect Flag */
+#define EIFR_OLIF	BIT(6)	/* Overload Frame Transmission Detect Flag */
+#define EIFR_ORIF	BIT(5)	/* Receive Overrun Detect Flag */
+#define EIFR_BORIF	BIT(4)	/* Bus-Off Recovery Detect Flag */
+#define EIFR_BOEIF	BIT(3)	/* Bus-Off Entry Detect Flag */
+#define EIFR_EPIF	BIT(2)	/* Error Passive Detect Flag */
+#define EIFR_EWIF	BIT(1)	/* Error Warning Detect Flag */
+#define EIFR_BEIF	BIT(0)	/* Bus Error Detect Flag */
+
+/* Error Code Store Register bits */
+#define ECSR_EDPM	BIT(7)	/* Error Display Mode Select Bit */
+#define ECSR_ADEF	BIT(6)	/* ACK Delimiter Error Flag */
+#define ECSR_BE0F	BIT(5)	/* Bit Error (dominant) Flag */
+#define ECSR_BE1F	BIT(4)	/* Bit Error (recessive) Flag */
+#define ECSR_CEF	BIT(3)	/* CRC Error Flag */
+#define ECSR_AEF	BIT(2)	/* ACK Error Flag */
+#define ECSR_FEF	BIT(1)	/* Form Error Flag */
+#define ECSR_SEF	BIT(0)	/* Stuff Error Flag */
+
+/* Mailbox Search Status Register bits */
+#define MSSR_SEST	BIT(7)	/* Search Result Status Bit */
+#define MSSR_MBNST	0x3f	/* Search Result Mailbox Number Status mask */
+
+/* Mailbox Search Mode Register values */
+#define MSMR_TXMB	1	/* Transmit mailbox search mode */
+#define MSMR_RXMB	0	/* Receive mailbox search mode */
+
+/* Mailbox configuration:
+ * mailbox 0 - not used
+ * mailbox 1-31 - Rx
+ * mailbox 32-63 - Tx
+ * no FIFO mailboxes
+ */
+#define N_MBX		64
+#define FIRST_TX_MB	32
+#define RX_MBX_MASK	0xFFFFFFFE
+
+#define RCAR_CAN_NAPI_WEIGHT (FIRST_TX_MB - 1)
+
+struct rcar_can_priv {
+	struct can_priv can;	/* Must be the first member! */
+	struct net_device *ndev;
+	struct napi_struct napi;
+	void __iomem *reg_base;
+	struct clk *clk;
+	spinlock_t mier_lock;
+	u8 clock_select;
+};
+
+static const struct can_bittiming_const rcar_can_bittiming_const = {
+	.name = DRV_NAME,
+	.tseg1_min = 4,
+	.tseg1_max = 16,
+	.tseg2_min = 2,
+	.tseg2_max = 8,
+	.sjw_max = 4,
+	.brp_min = 1,
+	.brp_max = 1024,
+	.brp_inc = 1,
+};
+
+static inline u32 rcar_can_readl(struct rcar_can_priv *priv, int reg)
+{
+	return readl(priv->reg_base + reg);
+}
+
+static inline u16 rcar_can_readw(struct rcar_can_priv *priv, int reg)
+{
+	return readw(priv->reg_base + reg);
+}
+
+static inline u8 rcar_can_readb(struct rcar_can_priv *priv, int reg)
+{
+	return readb(priv->reg_base + reg);
+}
+
+static inline void rcar_can_writel(struct rcar_can_priv *priv, int reg, u32 val)
+{
+	writel(val, priv->reg_base + reg);
+}
+
+static inline void rcar_can_writew(struct rcar_can_priv *priv, int reg, u16 val)
+{
+	writew(val, priv->reg_base + reg);
+}
+
+static inline void rcar_can_writeb(struct rcar_can_priv *priv, int reg, u8 val)
+{
+	writeb(val, priv->reg_base + reg);
+}
+
+static inline u32 rcar_can_mbx_readl(struct rcar_can_priv *priv,
+				     u32 mbxno, u8 offset)
+{
+	return rcar_can_readl(priv, RCAR_CAN_MBX_SIZE * mbxno + offset);
+}
+
+static inline u8 rcar_can_mbx_readb(struct rcar_can_priv *priv,
+				    u32 mbxno, u8 offset)
+{
+	return rcar_can_readb(priv, RCAR_CAN_MBX_SIZE * mbxno + offset);
+}
+
+static inline void rcar_can_mbx_writel(struct rcar_can_priv *priv, u32 mbxno,
+				       u8 offset, u32 val)
+{
+	rcar_can_writel(priv, RCAR_CAN_MBX_SIZE * mbxno + offset, val);
+}
+
+static inline void rcar_can_mbx_writeb(struct rcar_can_priv *priv, u32 mbxno,
+				       u8 offset, u8 val)
+{
+	rcar_can_writeb(priv, RCAR_CAN_MBX_SIZE * mbxno + offset, val);
+}
+
+static void rcar_can_error(struct net_device *ndev)
+{
+	struct rcar_can_priv *priv = netdev_priv(ndev);
+	struct net_device_stats *stats = &ndev->stats;
+	struct can_frame *cf;
+	struct sk_buff *skb;
+	u8 eifr;
+
+	/* Propagate the error condition to the CAN stack */
+	skb = alloc_can_err_skb(ndev, &cf);
+	if (!skb) {
+		if (printk_ratelimit())
+			netdev_err(priv->ndev,
+				   "%s: alloc_can_err_skb() failed\n",
+				   __func__);
+		return;
+	}
+
+	eifr = rcar_can_readb(priv, RCAR_CAN_EIFR);
+	if (eifr & EIFR_EWIF) {
+		netdev_dbg(priv->ndev, "Error warning interrupt\n");
+		priv->can.state = CAN_STATE_ERROR_WARNING;
+		priv->can.can_stats.error_warning++;
+		cf->can_id |= CAN_ERR_CRTL;
+		if (rcar_can_readb(priv, RCAR_CAN_TECR) > 96)
+			cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
+		if (rcar_can_readb(priv, RCAR_CAN_RECR) > 96)
+			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
+		/* Clear interrupt condition */
+		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EWIF);
+	}
+	if (eifr & EIFR_EPIF) {
+		netdev_dbg(priv->ndev, "Error passive interrupt\n");
+		priv->can.state = CAN_STATE_ERROR_PASSIVE;
+		priv->can.can_stats.error_passive++;
+		cf->can_id |= CAN_ERR_CRTL;
+		if (rcar_can_readb(priv, RCAR_CAN_TECR) > 127)
+			cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
+		if (rcar_can_readb(priv, RCAR_CAN_RECR) > 127)
+			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
+		/* Clear interrupt condition */
+		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EPIF);
+	}
+	if (eifr & EIFR_BOEIF) {
+		netdev_dbg(priv->ndev, "Bus-off entry interrupt\n");
+		priv->can.state = CAN_STATE_BUS_OFF;
+		cf->can_id |= CAN_ERR_BUSOFF;
+		/* Clear interrupt condition */
+		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BOEIF);
+		/* Disable all interrupts in bus-off to avoid int hog */
+		rcar_can_writeb(priv, RCAR_CAN_EIER, 0);
+		rcar_can_writeb(priv, RCAR_CAN_IER, 0);
+		can_bus_off(ndev);
+	}
+	if (eifr & EIFR_BEIF) {
+		int rx_errors = 0, tx_errors = 0, bus_errors = 0;
+		u8 ecsr;
+
+		netdev_dbg(priv->ndev, "Bus error interrupt:\n");
+		cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
+		cf->data[2] = CAN_ERR_PROT_UNSPEC;
+
+		ecsr = rcar_can_readb(priv, RCAR_CAN_ECSR);
+		if (ecsr & ECSR_ADEF) {
+			netdev_dbg(priv->ndev, "ACK Delimiter Error\n");
+			cf->data[3] |= CAN_ERR_PROT_LOC_ACK_DEL;
+			bus_errors++;
+			tx_errors++;
+			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_ADEF);
+		}
+		if (ecsr & ECSR_BE0F) {
+			netdev_dbg(priv->ndev, "Bit Error (dominant)\n");
+			cf->data[2] |= CAN_ERR_PROT_BIT0;
+			bus_errors++;
+			tx_errors++;
+			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_BE0F);
+		}
+		if (ecsr & ECSR_BE1F) {
+			netdev_dbg(priv->ndev, "Bit Error (recessive)\n");
+			cf->data[2] |= CAN_ERR_PROT_BIT1;
+			bus_errors++;
+			tx_errors++;
+			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_BE1F);
+		}
+		if (ecsr & ECSR_CEF) {
+			netdev_dbg(priv->ndev, "CRC Error\n");
+			cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
+			bus_errors++;
+			rx_errors++;
+			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_CEF);
+		}
+		if (ecsr & ECSR_AEF) {
+			netdev_dbg(priv->ndev, "ACK Error\n");
+			cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
+			bus_errors++;
+			tx_errors++;
+			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_AEF);
+		}
+		if (ecsr & ECSR_FEF) {
+			netdev_dbg(priv->ndev, "Form Error\n");
+			cf->data[2] |= CAN_ERR_PROT_FORM;
+			bus_errors++;
+			rx_errors++;
+			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_FEF);
+		}
+		if (ecsr & ECSR_SEF) {
+			netdev_dbg(priv->ndev, "Stuff Error\n");
+			cf->data[2] |= CAN_ERR_PROT_STUFF;
+			bus_errors++;
+			rx_errors++;
+			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_SEF);
+		}
+
+		priv->can.can_stats.bus_error += bus_errors;
+		ndev->stats.rx_errors += rx_errors;
+		ndev->stats.tx_errors += tx_errors;
+		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BEIF);
+	}
+	if (eifr & EIFR_ORIF) {
+		netdev_dbg(priv->ndev, "Receive overrun error interrupt\n");
+		cf->can_id |= CAN_ERR_CRTL;
+		cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
+		ndev->stats.rx_over_errors++;
+		ndev->stats.rx_errors++;
+		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_ORIF);
+	}
+	if (eifr & EIFR_OLIF) {
+		netdev_dbg(priv->ndev,
+			   "Overload Frame Transmission error interrupt\n");
+		cf->can_id |= CAN_ERR_PROT;
+		cf->data[2] |= CAN_ERR_PROT_OVERLOAD;
+		ndev->stats.rx_over_errors++;
+		ndev->stats.rx_errors++;
+		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_OLIF);
+	}
+
+	netif_rx(skb);
+	stats->rx_packets++;
+	stats->rx_bytes += cf->can_dlc;
+}
+
+static irqreturn_t rcar_can_interrupt(int irq, void *dev_id)
+{
+	struct net_device *ndev = (struct net_device *)dev_id;
+	struct rcar_can_priv *priv = netdev_priv(ndev);
+	struct net_device_stats *stats = &ndev->stats;
+	u8 isr;
+
+	isr = rcar_can_readb(priv, RCAR_CAN_ISR);
+	if (isr & ISR_ERSF)
+		rcar_can_error(ndev);
+
+	if (isr & ISR_TXMF) {
+		u32 ie_mask = 0;
+
+		/* Set Transmit Mailbox Search Mode */
+		rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_TXMB);
+		while (1) {
+			u8 mctl, mbx;
+
+			mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
+			if (mbx & MSSR_SEST)
+				break;
+			mbx &= MSSR_MBNST;
+			mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
+			/* Bits SENTDATA and TRMREQ cannot be
+			 * set to 0 simultaneously
+			 */
+			mctl &= ~MCTL_TRMREQ;
+			rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
+			mctl &= ~MCTL_SENTDATA;
+			/* Clear interrupt */
+			rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
+			ie_mask |= BIT(mbx - FIRST_TX_MB);
+			stats->tx_bytes += can_get_echo_skb(ndev,
+							    mbx - FIRST_TX_MB);
+			stats->tx_packets++;
+			can_led_event(ndev, CAN_LED_EVENT_TX);
+		}
+		/* Set receive mailbox search mode */
+		rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_RXMB);
+		/* Disable mailbox interrupt, mark mailbox as free */
+		if (ie_mask) {
+			u32 mier1;
+
+			spin_lock(&priv->mier_lock);
+			mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
+			rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 & ~ie_mask);
+			spin_unlock(&priv->mier_lock);
+			if (unlikely(netif_queue_stopped(ndev)))
+				netif_wake_queue(ndev);
+		}
+	}
+	if (isr & ISR_RXM1F) {
+		if (napi_schedule_prep(&priv->napi)) {
+			/* Disable Rx interrupts */
+			rcar_can_writeb(priv, RCAR_CAN_IER,
+					rcar_can_readb(priv, RCAR_CAN_IER) &
+						       ~IER_RXM1IE);
+			__napi_schedule(&priv->napi);
+		}
+	}
+	return IRQ_HANDLED;
+}
+
+static int rcar_can_set_bittiming(struct net_device *dev)
+{
+	struct rcar_can_priv *priv = netdev_priv(dev);
+	struct can_bittiming *bt = &priv->can.bittiming;
+	u32 bcr;
+	u16 ctlr;
+	u8 clkr;
+
+	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
+	if (ctlr & CTLR_SLPM) {
+		/* Write to BCR in CAN reset mode or CAN halt mode */
+		return -EBUSY;
+	}
+	/* Don't overwrite CLKR with 32-bit BCR access */
+	/* CLKR has 8-bit access */
+	clkr = rcar_can_readb(priv, RCAR_CAN_CLKR);
+	bcr = BCR_TSEG1(bt->phase_seg1 + bt->prop_seg - 1) |
+	      BCR_BPR(bt->brp - 1) | BCR_SJW(bt->sjw - 1) |
+	      BCR_TSEG2(bt->phase_seg2 - 1);
+	rcar_can_writel(priv, RCAR_CAN_BCR, bcr);
+	rcar_can_writeb(priv, RCAR_CAN_CLKR, clkr);
+	return 0;
+}
+
+static void rcar_can_start(struct net_device *ndev)
+{
+	struct rcar_can_priv *priv = netdev_priv(ndev);
+	u16 ctlr, n;
+
+	/* Set controller to known mode:
+	 * - normal mailbox mode (no FIFO);
+	 * - accept all messages (no filter).
+	 * CAN is in sleep mode after MCU hardware or software reset.
+	 */
+	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
+	ctlr &= ~CTLR_SLPM;
+	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
+	/* Go to reset mode */
+	ctlr |= CTLR_FORCE_RESET;
+	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
+	ctlr |= CTLR_IDFM_MIXED;	/* Select mixed ID mode */
+	ctlr &= ~CTLR_TPM;		/* Set ID priority transmit mode */
+	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
+
+	rcar_can_writeb(priv, RCAR_CAN_CLKR, priv->clock_select);
+
+	/* Accept all SID and EID */
+	for (n = 0; n < N_RX_MKREGS; n++)
+		rcar_can_writel(priv, RCAR_CAN_MKR(n), 0);
+	rcar_can_writel(priv, RCAR_CAN_MKIVLR0, 0);
+
+	rcar_can_set_bittiming(ndev);
+
+	/* Initial value of MIER1 undefined.  Mark all Tx mailboxes as free. */
+	rcar_can_writel(priv, RCAR_CAN_MIER1, 0);
+
+	rcar_can_writeb(priv, RCAR_CAN_IER, IER_TXMIE | IER_ERSIE | IER_RXM1IE);
+
+	/* Accumulate error codes */
+	rcar_can_writeb(priv, RCAR_CAN_ECSR, ECSR_EDPM);
+	/* Enable error interrupts */
+	rcar_can_writeb(priv, RCAR_CAN_EIER,
+			EIER_EWIE | EIER_EPIE | EIER_BOEIE | EIER_BEIE |
+			EIER_ORIE | EIER_OLIE);
+	/* Enable interrupts for RX mailboxes */
+	rcar_can_writel(priv, RCAR_CAN_MIER0, RX_MBX_MASK);
+	priv->can.state = CAN_STATE_ERROR_ACTIVE;
+
+	/* Write to the CiMCTLj register in CAN
+	 * operation mode or CAN halt mode.
+	 * Configure mailboxes 0-31 as Rx mailboxes.
+	 * Configure mailboxes 32-63 as Tx mailboxes.
+	 */
+	/* Go to halt mode */
+	ctlr |= CTLR_HALT;
+	ctlr &= ~CTLR_RESET;
+	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
+	for (n = 0; n < FIRST_TX_MB; n++) {
+		/* According to documentation we should clear MCTL
+		 * register before configuring mailbox.
+		 */
+		rcar_can_writeb(priv, RCAR_CAN_MCTL(n), 0);
+		rcar_can_writeb(priv, RCAR_CAN_MCTL(n), MCTL_RECREQ);
+		rcar_can_writeb(priv, RCAR_CAN_MCTL(FIRST_TX_MB + n), 0);
+	}
+	/* Go to operation mode */
+	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr & ~CTLR_FORCE_RESET);
+}
+
+static int rcar_can_open(struct net_device *ndev)
+{
+	struct rcar_can_priv *priv = netdev_priv(ndev);
+	int err;
+
+	err = open_candev(ndev);
+	if (err) {
+		netdev_err(ndev, "open_candev() failed %d\n", err);
+		goto out;
+	}
+	napi_enable(&priv->napi);
+	err = request_irq(ndev->irq, rcar_can_interrupt, 0, ndev->name, ndev);
+	if (err) {
+		netdev_err(ndev, "error requesting interrupt %x\n", ndev->irq);
+		goto out_close;
+	}
+	can_led_event(ndev, CAN_LED_EVENT_OPEN);
+	rcar_can_start(ndev);
+	netif_start_queue(ndev);
+	return 0;
+out_close:
+	napi_disable(&priv->napi);
+	close_candev(ndev);
+out:
+	return err;
+}
+
+static void rcar_can_stop(struct net_device *ndev)
+{
+	struct rcar_can_priv *priv = netdev_priv(ndev);
+	u16 ctlr;
+
+	/* Go to (force) reset mode */
+	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
+	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr | CTLR_FORCE_RESET);
+	rcar_can_writel(priv, RCAR_CAN_MIER0, 0);
+	rcar_can_writel(priv, RCAR_CAN_MIER1, 0);
+	rcar_can_writeb(priv, RCAR_CAN_IER, 0);
+	rcar_can_writeb(priv, RCAR_CAN_EIER, 0);
+	priv->can.state = CAN_STATE_STOPPED;
+}
+
+static int rcar_can_close(struct net_device *ndev)
+{
+	struct rcar_can_priv *priv = netdev_priv(ndev);
+
+	netif_stop_queue(ndev);
+	rcar_can_stop(ndev);
+	free_irq(ndev->irq, ndev);
+	napi_disable(&priv->napi);
+	close_candev(ndev);
+	can_led_event(ndev, CAN_LED_EVENT_STOP);
+	return 0;
+}
+
+static netdev_tx_t rcar_can_start_xmit(struct sk_buff *skb,
+				       struct net_device *ndev)
+{
+	struct rcar_can_priv *priv = netdev_priv(ndev);
+	struct can_frame *cf = (struct can_frame *)skb->data;
+	u32 data, mier1, mbxno, i;
+	unsigned long flags;
+	u8 mctl;
+
+	if (can_dropped_invalid_skb(ndev, skb))
+		return NETDEV_TX_OK;
+
+	spin_lock_irqsave(&priv->mier_lock, flags);
+	mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
+	if (unlikely(mier1 == ~0U)) {
+		spin_unlock_irqrestore(&priv->mier_lock, flags);
+		netif_stop_queue(ndev);
+		return NETDEV_TX_BUSY;
+	}
+	rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 | (mier1 + 1));
+	spin_unlock_irqrestore(&priv->mier_lock, flags);
+	mbxno = ffz(mier1) + FIRST_TX_MB;
+
+	if (cf->can_id & CAN_EFF_FLAG) {
+		/* Extended frame format */
+		data = (cf->can_id & CAN_EFF_MASK) | RCAR_CAN_IDE;
+	} else {
+		/* Standard frame format */
+		data = (cf->can_id & CAN_SFF_MASK) << RCAR_CAN_SID_SHIFT;
+	}
+	if (cf->can_id & CAN_RTR_FLAG) {
+		/* Remote transmission request */
+		data |= RCAR_CAN_RTR;
+	}
+	rcar_can_mbx_writel(priv, mbxno, MBX_HDR_OFFSET, data);
+
+	rcar_can_mbx_writeb(priv, mbxno, MBX_DLC_OFFSET, cf->can_dlc);
+
+	for (i = 0; i < cf->can_dlc; i++)
+		rcar_can_mbx_writeb(priv, mbxno,
+				    MBX_DATA_OFFSET + i, cf->data[i]);
+
+	can_put_echo_skb(skb, ndev, mbxno - FIRST_TX_MB);
+
+	rcar_can_writeb(priv, RCAR_CAN_IER,
+			rcar_can_readb(priv, RCAR_CAN_IER) | IER_TXMIE);
+
+	mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbxno));
+	if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
+		mctl |= MCTL_ONESHOT;
+	else
+		mctl &= ~MCTL_ONESHOT;
+	/* Start TX */
+	mctl |= MCTL_TRMREQ;
+	rcar_can_writeb(priv, RCAR_CAN_MCTL(mbxno), mctl);
+	return NETDEV_TX_OK;
+}
+
+static const struct net_device_ops rcar_can_netdev_ops = {
+	.ndo_open = rcar_can_open,
+	.ndo_stop = rcar_can_close,
+	.ndo_start_xmit = rcar_can_start_xmit,
+};
+
+static void rcar_can_rx_pkt(struct rcar_can_priv *priv, int mbx)
+{
+	struct net_device_stats *stats = &priv->ndev->stats;
+	struct can_frame *cf;
+	struct sk_buff *skb;
+	u32 data;
+	u8 dlc;
+
+	skb = alloc_can_skb(priv->ndev, &cf);
+	if (!skb) {
+		stats->rx_dropped++;
+		if (printk_ratelimit())
+			netdev_err(priv->ndev,
+				   "%s: alloc_can_skb() failed\n", __func__);
+		return;
+	}
+
+	data = rcar_can_mbx_readl(priv, mbx, MBX_HDR_OFFSET);
+	if (data & RCAR_CAN_IDE)
+		cf->can_id = (data & CAN_EFF_MASK) | CAN_EFF_FLAG;
+	else
+		cf->can_id = (data >> RCAR_CAN_SID_SHIFT) & CAN_SFF_MASK;
+	if (data & RCAR_CAN_RTR)
+		cf->can_id |= CAN_RTR_FLAG;
+
+	dlc = rcar_can_mbx_readb(priv, mbx, MBX_DLC_OFFSET);
+	cf->can_dlc = get_can_dlc(dlc);
+	for (dlc = 0; dlc < cf->can_dlc; dlc++)
+		cf->data[dlc] = rcar_can_mbx_readb(priv, mbx,
+						   MBX_DATA_OFFSET + dlc);
+
+	can_led_event(priv->ndev, CAN_LED_EVENT_RX);
+
+	netif_receive_skb(skb);
+	stats->rx_bytes += cf->can_dlc;
+	stats->rx_packets++;
+}
+
+static int rcar_can_rx_poll(struct napi_struct *napi, int quota)
+{
+	struct rcar_can_priv *priv = container_of(napi,
+						  struct rcar_can_priv, napi);
+	u32 num_pkts = 0;
+
+	/* Find mailbox */
+	while (1) {
+		u8 mctl, mbx;
+
+		mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
+		if (mbx & MSSR_SEST || num_pkts >= quota)
+			break;
+		mbx &= MSSR_MBNST;
+		mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
+		/* Clear interrupt */
+		rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx),
+				mctl & ~MCTL_NEWDATA);
+		rcar_can_rx_pkt(priv, mbx);
+		++num_pkts;
+	}
+	/* All packets processed */
+	if (num_pkts < quota) {
+		u8 ier;
+
+		napi_complete(napi);
+		ier = rcar_can_readb(priv, RCAR_CAN_IER);
+		rcar_can_writeb(priv, RCAR_CAN_IER, ier | IER_RXM1IE);
+	}
+	return num_pkts;
+}
+
+static int rcar_can_do_set_mode(struct net_device *ndev, enum can_mode mode)
+{
+	switch (mode) {
+	case CAN_MODE_START:
+		rcar_can_start(ndev);
+		netif_wake_queue(ndev);
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int rcar_can_get_berr_counter(const struct net_device *dev,
+				     struct can_berr_counter *bec)
+{
+	struct rcar_can_priv *priv = netdev_priv(dev);
+
+	bec->txerr = rcar_can_readb(priv, RCAR_CAN_TECR);
+	bec->rxerr = rcar_can_readb(priv, RCAR_CAN_RECR);
+	return 0;
+}
+
+static int rcar_can_probe(struct platform_device *pdev)
+{
+	struct rcar_can_platform_data *pdata;
+	struct rcar_can_priv *priv;
+	struct net_device *ndev;
+	struct resource *mem;
+	void __iomem *addr;
+	int err = -ENODEV;
+	int irq;
+
+	pdata = pdev->dev.platform_data;
+	if (!pdata) {
+		dev_err(&pdev->dev, "No platform data provided!\n");
+		goto fail;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (!irq) {
+		dev_err(&pdev->dev, "No IRQ resource\n");
+		goto fail;
+	}
+
+	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;
+	}
+
+	ndev = alloc_candev(sizeof(struct rcar_can_priv), N_MBX - FIRST_TX_MB);
+	if (!ndev) {
+		dev_err(&pdev->dev, "alloc_candev failed\n");
+		err = -ENOMEM;
+		goto fail;
+	}
+
+	priv = netdev_priv(ndev);
+
+	priv->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		err = PTR_ERR(priv->clk);
+		dev_err(&pdev->dev, "cannot get clock: %d\n", err);
+		goto fail_clk;
+	}
+	clk_enable(priv->clk);
+
+	ndev->netdev_ops = &rcar_can_netdev_ops;
+	ndev->irq = irq;
+	ndev->flags |= IFF_ECHO;
+	priv->ndev = ndev;
+	priv->reg_base = addr;
+	priv->clock_select = pdata->clock_select;
+	priv->can.clock.freq = clk_get_rate(priv->clk);
+	priv->can.bittiming_const = &rcar_can_bittiming_const;
+	priv->can.do_set_bittiming = rcar_can_set_bittiming;
+	priv->can.do_set_mode = rcar_can_do_set_mode;
+	priv->can.do_get_berr_counter = rcar_can_get_berr_counter;
+	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES |
+				       CAN_CTRLMODE_ONE_SHOT;
+	platform_set_drvdata(pdev, ndev);
+	SET_NETDEV_DEV(ndev, &pdev->dev);
+	spin_lock_init(&priv->mier_lock);
+
+	netif_napi_add(ndev, &priv->napi, rcar_can_rx_poll,
+		       RCAR_CAN_NAPI_WEIGHT);
+	err = register_candev(ndev);
+	if (err) {
+		dev_err(&pdev->dev, "register_candev() failed\n");
+		goto fail_candev;
+	}
+
+	devm_can_led_init(ndev);
+
+	dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%u)\n",
+		 priv->reg_base, ndev->irq);
+
+	return 0;
+fail_candev:
+	netif_napi_del(&priv->napi);
+fail_clk:
+	free_candev(ndev);
+fail:
+	return err;
+}
+
+static int rcar_can_remove(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct rcar_can_priv *priv = netdev_priv(ndev);
+	u16 ctlr;
+
+	unregister_candev(ndev);
+	netif_napi_del(&priv->napi);
+	/* Go to sleep mode */
+	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
+	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr | CTLR_SLPM);
+	clk_disable(priv->clk);
+	free_candev(ndev);
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int rcar_can_suspend(struct device *dev)
+{
+	struct net_device *ndev = dev_get_drvdata(dev);
+	struct rcar_can_priv *priv = netdev_priv(ndev);
+	u16 ctlr;
+
+	if (netif_running(ndev)) {
+		netif_stop_queue(ndev);
+		netif_device_detach(ndev);
+	}
+	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
+	ctlr |= CTLR_HALT;
+	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
+	ctlr |= CTLR_SLPM;
+	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
+	priv->can.state = CAN_STATE_SLEEPING;
+
+	clk_disable(priv->clk);
+	return 0;
+}
+
+static int rcar_can_resume(struct device *dev)
+{
+	struct net_device *ndev = dev_get_drvdata(dev);
+	struct rcar_can_priv *priv = netdev_priv(ndev);
+	u16 ctlr;
+
+	clk_enable(priv->clk);
+
+	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
+	ctlr &= ~CTLR_SLPM;
+	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
+	ctlr &= ~CTLR_FORCE_RESET;
+	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
+	priv->can.state = CAN_STATE_ERROR_ACTIVE;
+
+	if (netif_running(ndev)) {
+		netif_device_attach(ndev);
+		netif_start_queue(ndev);
+	}
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(rcar_can_pm_ops, rcar_can_suspend, rcar_can_resume);
+
+static struct platform_driver rcar_can_driver = {
+	.driver = {
+		.name = DRV_NAME,
+		.owner = THIS_MODULE,
+		.pm = &rcar_can_pm_ops,
+	},
+	.probe = rcar_can_probe,
+	.remove = rcar_can_remove,
+};
+
+module_platform_driver(rcar_can_driver);
+
+MODULE_AUTHOR("Cogent Embedded, Inc.");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("CAN driver for Renesas R-Car SoC");
+MODULE_ALIAS("platform:" DRV_NAME);
Index: linux-can-next/include/linux/can/platform/rcar_can.h
===================================================================
--- /dev/null
+++ linux-can-next/include/linux/can/platform/rcar_can.h
@@ -0,0 +1,15 @@ 
+#ifndef _CAN_PLATFORM_RCAR_CAN_H_
+#define _CAN_PLATFORM_RCAR_CAN_H_
+
+#include <linux/types.h>
+
+/* Clock Select Register settings */
+#define CLKR_CLKEXT	3	/* Externally input clock */
+#define CLKR_CLKP2	1	/* Peripheral clock (clkp2) */
+#define CLKR_CLKP1	0	/* Peripheral clock (clkp1) */
+
+struct rcar_can_platform_data {
+	u8 clock_select;	/* Clock source select */
+};
+
+#endif	/* !_CAN_PLATFORM_RCAR_CAN_H_ */