[RFC PATCH 00/14] consolidate and unify state change and bus-off handling

[RFC PATCH 00/14] consolidate and unify state change and bus-off handling

[Wolfgang Grandegger]

As already announced, here is my first patch series to consolidate the
handling of CAN error state changes and bus-off.

I have also updated the Flexcan driver to behave like the other drivers,
especially bus error reporting is now enabled via interrupt source. This
works on my i.MX28 SOC. Marc, would be nice if you could test it on
other SOCs as well (or somebody else). Hopefully we will not need to
handle different hardware variants of the Flexcan controller.

I separated "consolidate error state handling" and "consolidate bus-off
handling" to make the difference clear. The latter one is more delicate
as it relys on the "back to error active" change to arrive to re-enable
the TX queue.

I have also pushed these patches to my cloned Gitrorious repository
"wg-linux-can-next". See:

https://gitorious.org/~wgrandegger/linux-can/wg-linux-can-next/commits/master

Any comments are appreciated.

Wolfgang.

Wolfgang Grandegger (14):
  can: flexcan: fix irq flooding by clearing all interrupt sources
  can: replace the dev_dbg/info/err/... with the new netdev_xxx macros
  can: flexcan: only for bus error reporting enable berr interrupt
  can: bfin_can/ti_hecc/mscan: add missing do_get_berr_counter callback
  can: add error counters to the data fields of any CAN error message
  can: dev: consolidate error state change handling
  can: flexcan: consolidate error state handling
  can: flexcan: consolidate bus-off handling
  can: sja1000: consolidate error state handling
  can: sja1000: consolidate bus-off handling
  can: mscan: consolidate error state handling
  can: mscan: consolidate bus-off handling
  can: cc770: consolidate error state handling
  can: cc770: consolidate bus-off handling

 drivers/net/can/bfin_can.c        |   36 +++++---
 drivers/net/can/cc770/cc770.c     |   57 ++++++-----
 drivers/net/can/dev.c             |  103 +++++++++++++++++---
 drivers/net/can/flexcan.c         |  194 ++++++++++++-------------------------
 drivers/net/can/mcp251x.c         |    3 +-
 drivers/net/can/mscan/mscan.c     |  129 ++++++++++++-------------
 drivers/net/can/sja1000/sja1000.c |   67 ++++++-------
 drivers/net/can/ti_hecc.c         |   32 ++++--
 drivers/net/can/usb/ems_usb.c     |   57 +++++-------
 drivers/net/can/usb/esd_usb2.c    |   27 +++---
 include/linux/can/dev.h           |   11 ++
 include/linux/can/error.h         |    2 +
 12 files changed, 369 insertions(+), 349 deletions(-)

-- 
1.7.4.1

[RFC PATCH 01/14] can: flexcan: fix irq flooding by clearing all interrupt sources

[Wolfgang Grandegger]

As pointed out by Reuben Dowle, the TWRN_INT, RWRN_INT, BOFF_INT
interrupt sources need to be cleared as well to avoid interrupt
flooding, at least for the Flexcan on i.MX28 SOCs.

CC: Reuben Dowle <Reuben.Dowle@navico.com>
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/flexcan.c |   67 ++++++++++++++++-----------------------------
 1 files changed, 24 insertions(+), 43 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 165a4c7..feb8e64 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -118,6 +118,9 @@
 	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | FLEXCAN_ESR_BOFF_INT)
 #define FLEXCAN_ESR_ERR_ALL \
 	(FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE)
+#define FLEXCAN_ESR_ALL_INT \
+	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
+	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
 
 /* FLEXCAN interrupt flag register (IFLAG) bits */
 #define FLEXCAN_TX_BUF_ID		8
@@ -224,13 +227,6 @@ static void flexcan_transceiver_switch(const struct flexcan_priv *priv, int on)
 		priv->pdata->transceiver_switch(on);
 }
 
-static inline int flexcan_has_and_handle_berr(const struct flexcan_priv *priv,
-					      u32 reg_esr)
-{
-	return (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
-		(reg_esr & FLEXCAN_ESR_ERR_BUS);
-}
-
 static inline void flexcan_chip_enable(struct flexcan_priv *priv)
 {
 	struct flexcan_regs __iomem *regs = priv->base;
@@ -358,24 +354,6 @@ static void do_bus_err(struct net_device *dev,
 		dev->stats.tx_errors++;
 }
 
-static int flexcan_poll_bus_err(struct net_device *dev, u32 reg_esr)
-{
-	struct sk_buff *skb;
-	struct can_frame *cf;
-
-	skb = alloc_can_err_skb(dev, &cf);
-	if (unlikely(!skb))
-		return 0;
-
-	do_bus_err(dev, cf, reg_esr);
-	netif_receive_skb(skb);
-
-	dev->stats.rx_packets++;
-	dev->stats.rx_bytes += cf->can_dlc;
-
-	return 1;
-}
-
 static void do_state(struct net_device *dev,
 		     struct can_frame *cf, enum can_state new_state)
 {
@@ -442,7 +420,7 @@ static void do_state(struct net_device *dev,
 	}
 }
 
-static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
+static int flexcan_poll_error_state(struct net_device *dev, u32 reg_esr)
 {
 	struct flexcan_priv *priv = netdev_priv(dev);
 	struct sk_buff *skb;
@@ -462,16 +440,23 @@ static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
 	else
 		new_state = CAN_STATE_BUS_OFF;
 
-	/* state hasn't changed */
-	if (likely(new_state == priv->can.state))
+	/* has state changed or are there bus errros? */
+	if (likely(new_state == priv->can.state &&
+		  !(reg_esr & FLEXCAN_ESR_ERR_BUS)))
 		return 0;
 
 	skb = alloc_can_err_skb(dev, &cf);
 	if (unlikely(!skb))
 		return 0;
 
-	do_state(dev, cf, new_state);
-	priv->can.state = new_state;
+	if (new_state != priv->can.state) {
+		do_state(dev, cf, new_state);
+		priv->can.state = new_state;
+	}
+
+	if (reg_esr & FLEXCAN_ESR_ERR_BUS)
+		do_bus_err(dev, cf, reg_esr);
+
 	netif_receive_skb(skb);
 
 	dev->stats.rx_packets++;
@@ -540,10 +525,10 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 	 * The error bits are cleared on read,
 	 * use saved value from irq handler.
 	 */
-	reg_esr = flexcan_read(&regs->esr) | priv->reg_esr;
+	reg_esr = priv->reg_esr;
 
-	/* handle state changes */
-	work_done += flexcan_poll_state(dev, reg_esr);
+	/* handle bus error amd state changes */
+	work_done += flexcan_poll_error_state(dev, reg_esr);
 
 	/* handle RX-FIFO */
 	reg_iflag1 = flexcan_read(&regs->iflag1);
@@ -553,10 +538,6 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 		reg_iflag1 = flexcan_read(&regs->iflag1);
 	}
 
-	/* report bus errors */
-	if (flexcan_has_and_handle_berr(priv, reg_esr) && work_done < quota)
-		work_done += flexcan_poll_bus_err(dev, reg_esr);
-
 	if (work_done < quota) {
 		napi_complete(napi);
 		/* enable IRQs */
@@ -577,7 +558,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 
 	reg_iflag1 = flexcan_read(&regs->iflag1);
 	reg_esr = flexcan_read(&regs->esr);
-	flexcan_write(FLEXCAN_ESR_ERR_INT, &regs->esr);	/* ACK err IRQ */
+	/* ACK all bus error and state change IRQ sources */
+	if (reg_esr & FLEXCAN_ESR_ALL_INT)
+		flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
 
 	/*
 	 * schedule NAPI in case of:
@@ -586,13 +569,11 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	 * - bus error IRQ and bus error reporting is activated
 	 */
 	if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
-	    (reg_esr & FLEXCAN_ESR_ERR_STATE) ||
-	    flexcan_has_and_handle_berr(priv, reg_esr)) {
+	    (reg_esr & FLEXCAN_ESR_ALL_INT)) {
 		/*
-		 * The error bits are cleared on read,
-		 * save them for later use.
+		 * save esr at interrupt time for later use in poll function
 		 */
-		priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
+		priv->reg_esr = reg_esr;
 		flexcan_write(FLEXCAN_IFLAG_DEFAULT &
 			~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
 		flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
-- 
1.7.4.1

Re: [RFC PATCH 01/14] can: flexcan: fix irq flooding by clearing all interrupt sources

[Wolfgang Grandegger]

On 12/07/2011 03:55 PM, Wolfgang Grandegger wrote:
> As pointed out by Reuben Dowle, the TWRN_INT, RWRN_INT, BOFF_INT
> interrupt sources need to be cleared as well to avoid interrupt
> flooding, at least for the Flexcan on i.MX28 SOCs.
> 
> CC: Reuben Dowle <Reuben.Dowle-LjHtMTQxemvQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

Argh, I just realize that the commit messages have be swapped. Please
take the one from 

[RFC PATCH 03/14] can: flexcan: only for bus error reporting enable berr interrupt

So far, the bus error (berr) interrupt source is always enabled,
even if bus error reporting is not requested (via ctrlmode flag
CAN_CTRLMODE_BERR_REPORTING). This is not necessay, at least on
the Flexcan of the i.MX28 SOC and it avoids flooding with with
bus error interrupts. State changes interrupts do still arrive
as documented. The function flexcan_has_and_handle_berr() has
been removed and error state changes and bus errors are now
packed into one error message by a common poll function, like
for other CAN controllers.

Puh, managing 14 patches is not a trivial task, at least for me!

Wolfgang.

> ---
>  drivers/net/can/flexcan.c |   67 ++++++++++++++++-----------------------------
>  1 files changed, 24 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 165a4c7..feb8e64 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -118,6 +118,9 @@
>  	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | FLEXCAN_ESR_BOFF_INT)
>  #define FLEXCAN_ESR_ERR_ALL \
>  	(FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE)
> +#define FLEXCAN_ESR_ALL_INT \
> +	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
> +	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
>  
>  /* FLEXCAN interrupt flag register (IFLAG) bits */
>  #define FLEXCAN_TX_BUF_ID		8
> @@ -224,13 +227,6 @@ static void flexcan_transceiver_switch(const struct flexcan_priv *priv, int on)
>  		priv->pdata->transceiver_switch(on);
>  }
>  
> -static inline int flexcan_has_and_handle_berr(const struct flexcan_priv *priv,
> -					      u32 reg_esr)
> -{
> -	return (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
> -		(reg_esr & FLEXCAN_ESR_ERR_BUS);
> -}
> -
>  static inline void flexcan_chip_enable(struct flexcan_priv *priv)
>  {
>  	struct flexcan_regs __iomem *regs = priv->base;
> @@ -358,24 +354,6 @@ static void do_bus_err(struct net_device *dev,
>  		dev->stats.tx_errors++;
>  }
>  
> -static int flexcan_poll_bus_err(struct net_device *dev, u32 reg_esr)
> -{
> -	struct sk_buff *skb;
> -	struct can_frame *cf;
> -
> -	skb = alloc_can_err_skb(dev, &cf);
> -	if (unlikely(!skb))
> -		return 0;
> -
> -	do_bus_err(dev, cf, reg_esr);
> -	netif_receive_skb(skb);
> -
> -	dev->stats.rx_packets++;
> -	dev->stats.rx_bytes += cf->can_dlc;
> -
> -	return 1;
> -}
> -
>  static void do_state(struct net_device *dev,
>  		     struct can_frame *cf, enum can_state new_state)
>  {
> @@ -442,7 +420,7 @@ static void do_state(struct net_device *dev,
>  	}
>  }
>  
> -static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
> +static int flexcan_poll_error_state(struct net_device *dev, u32 reg_esr)
>  {
>  	struct flexcan_priv *priv = netdev_priv(dev);
>  	struct sk_buff *skb;
> @@ -462,16 +440,23 @@ static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
>  	else
>  		new_state = CAN_STATE_BUS_OFF;
>  
> -	/* state hasn't changed */
> -	if (likely(new_state == priv->can.state))
> +	/* has state changed or are there bus errros? */
> +	if (likely(new_state == priv->can.state &&
> +		  !(reg_esr & FLEXCAN_ESR_ERR_BUS)))
>  		return 0;
>  
>  	skb = alloc_can_err_skb(dev, &cf);
>  	if (unlikely(!skb))
>  		return 0;
>  
> -	do_state(dev, cf, new_state);
> -	priv->can.state = new_state;
> +	if (new_state != priv->can.state) {
> +		do_state(dev, cf, new_state);
> +		priv->can.state = new_state;
> +	}
> +
> +	if (reg_esr & FLEXCAN_ESR_ERR_BUS)
> +		do_bus_err(dev, cf, reg_esr);
> +
>  	netif_receive_skb(skb);
>  
>  	dev->stats.rx_packets++;
> @@ -540,10 +525,10 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
>  	 * The error bits are cleared on read,
>  	 * use saved value from irq handler.
>  	 */
> -	reg_esr = flexcan_read(&regs->esr) | priv->reg_esr;
> +	reg_esr = priv->reg_esr;
>  
> -	/* handle state changes */
> -	work_done += flexcan_poll_state(dev, reg_esr);
> +	/* handle bus error amd state changes */
> +	work_done += flexcan_poll_error_state(dev, reg_esr);
>  
>  	/* handle RX-FIFO */
>  	reg_iflag1 = flexcan_read(&regs->iflag1);
> @@ -553,10 +538,6 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
>  		reg_iflag1 = flexcan_read(&regs->iflag1);
>  	}
>  
> -	/* report bus errors */
> -	if (flexcan_has_and_handle_berr(priv, reg_esr) && work_done < quota)
> -		work_done += flexcan_poll_bus_err(dev, reg_esr);
> -
>  	if (work_done < quota) {
>  		napi_complete(napi);
>  		/* enable IRQs */
> @@ -577,7 +558,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  
>  	reg_iflag1 = flexcan_read(&regs->iflag1);
>  	reg_esr = flexcan_read(&regs->esr);
> -	flexcan_write(FLEXCAN_ESR_ERR_INT, &regs->esr);	/* ACK err IRQ */
> +	/* ACK all bus error and state change IRQ sources */
> +	if (reg_esr & FLEXCAN_ESR_ALL_INT)
> +		flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
>  
>  	/*
>  	 * schedule NAPI in case of:
> @@ -586,13 +569,11 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  	 * - bus error IRQ and bus error reporting is activated
>  	 */
>  	if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
> -	    (reg_esr & FLEXCAN_ESR_ERR_STATE) ||
> -	    flexcan_has_and_handle_berr(priv, reg_esr)) {
> +	    (reg_esr & FLEXCAN_ESR_ALL_INT)) {
>  		/*
> -		 * The error bits are cleared on read,
> -		 * save them for later use.
> +		 * save esr at interrupt time for later use in poll function
>  		 */
> -		priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
> +		priv->reg_esr = reg_esr;
>  		flexcan_write(FLEXCAN_IFLAG_DEFAULT &
>  			~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
>  		flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,

[RFC PATCH 02/14] can: replace the dev_dbg/info/err/... with the new netdev_xxx macros

[Wolfgang Grandegger]

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/bfin_can.c        |   21 +++++--------
 drivers/net/can/dev.c             |   23 +++++++--------
 drivers/net/can/flexcan.c         |   44 +++++++++++++---------------
 drivers/net/can/mcp251x.c         |    3 +-
 drivers/net/can/mscan/mscan.c     |   17 +++++------
 drivers/net/can/sja1000/sja1000.c |   19 ++++++------
 drivers/net/can/ti_hecc.c         |   20 ++++++------
 drivers/net/can/usb/ems_usb.c     |   57 +++++++++++++++----------------------
 drivers/net/can/usb/esd_usb2.c    |   27 ++++++++----------
 9 files changed, 103 insertions(+), 128 deletions(-)

diff --git a/drivers/net/can/bfin_can.c b/drivers/net/can/bfin_can.c
index 349e0fa..9cf4ddb 100644
--- a/drivers/net/can/bfin_can.c
+++ b/drivers/net/can/bfin_can.c
@@ -82,8 +82,7 @@ static int bfin_can_set_bittiming(struct net_device *dev)
 	bfin_write(&reg->clock, clk);
 	bfin_write(&reg->timing, timing);
 
-	dev_info(dev->dev.parent, "setting CLOCK=0x%04x TIMING=0x%04x\n",
-			clk, timing);
+	netdev_info(dev, "setting CLOCK=0x%04x TIMING=0x%04x\n", clk, timing);
 
 	return 0;
 }
@@ -108,8 +107,7 @@ static void bfin_can_set_reset_mode(struct net_device *dev)
 	while (!(bfin_read(&reg->control) & CCA)) {
 		udelay(10);
 		if (--timeout == 0) {
-			dev_err(dev->dev.parent,
-					"fail to enter configuration mode\n");
+			netdev_err(dev, "fail to enter configuration mode\n");
 			BUG();
 		}
 	}
@@ -165,8 +163,7 @@ static void bfin_can_set_normal_mode(struct net_device *dev)
 	while (bfin_read(&reg->status) & CCA) {
 		udelay(10);
 		if (--timeout == 0) {
-			dev_err(dev->dev.parent,
-					"fail to leave configuration mode\n");
+			netdev_err(dev, "fail to leave configuration mode\n");
 			BUG();
 		}
 	}
@@ -331,7 +328,7 @@ static int bfin_can_err(struct net_device *dev, u16 isrc, u16 status)
 
 	if (isrc & RMLIS) {
 		/* data overrun interrupt */
-		dev_dbg(dev->dev.parent, "data overrun interrupt\n");
+		netdev_dbg(dev, "data overrun interrupt\n");
 		cf->can_id |= CAN_ERR_CRTL;
 		cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
 		stats->rx_over_errors++;
@@ -339,7 +336,7 @@ static int bfin_can_err(struct net_device *dev, u16 isrc, u16 status)
 	}
 
 	if (isrc & BOIS) {
-		dev_dbg(dev->dev.parent, "bus-off mode interrupt\n");
+		netdev_dbg(dev, "bus-off mode interrupt\n");
 		state = CAN_STATE_BUS_OFF;
 		cf->can_id |= CAN_ERR_BUSOFF;
 		can_bus_off(dev);
@@ -347,13 +344,12 @@ static int bfin_can_err(struct net_device *dev, u16 isrc, u16 status)
 
 	if (isrc & EPIS) {
 		/* error passive interrupt */
-		dev_dbg(dev->dev.parent, "error passive interrupt\n");
+		netdev_dbg(dev, "error passive interrupt\n");
 		state = CAN_STATE_ERROR_PASSIVE;
 	}
 
 	if ((isrc & EWTIS) || (isrc & EWRIS)) {
-		dev_dbg(dev->dev.parent,
-				"Error Warning Transmit/Receive Interrupt\n");
+		netdev_dbg(dev, "Error Warning Transmit/Receive Interrupt\n");
 		state = CAN_STATE_ERROR_WARNING;
 	}
 
@@ -636,8 +632,7 @@ static int bfin_can_suspend(struct platform_device *pdev, pm_message_t mesg)
 		while (!(bfin_read(&reg->intr) & SMACK)) {
 			udelay(10);
 			if (--timeout == 0) {
-				dev_err(dev->dev.parent,
-						"fail to enter sleep mode\n");
+				netdev_err(dev, "fail to enter sleep mode\n");
 				BUG();
 			}
 		}
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 120f1ab..ac4ac92 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -130,13 +130,13 @@ static int can_calc_bittiming(struct net_device *dev, struct can_bittiming *bt)
 		/* Error in one-tenth of a percent */
 		error = (best_error * 1000) / bt->bitrate;
 		if (error > CAN_CALC_MAX_ERROR) {
-			dev_err(dev->dev.parent,
-				"bitrate error %ld.%ld%% too high\n",
-				error / 10, error % 10);
+			netdev_err(dev,
+				   "bitrate error %ld.%ld%% too high\n",
+				   error / 10, error % 10);
 			return -EDOM;
 		} else {
-			dev_warn(dev->dev.parent, "bitrate error %ld.%ld%%\n",
-				 error / 10, error % 10);
+			netdev_warn(dev, "bitrate error %ld.%ld%%\n",
+				    error / 10, error % 10);
 		}
 	}
 
@@ -172,7 +172,7 @@ static int can_calc_bittiming(struct net_device *dev, struct can_bittiming *bt)
 #else /* !CONFIG_CAN_CALC_BITTIMING */
 static int can_calc_bittiming(struct net_device *dev, struct can_bittiming *bt)
 {
-	dev_err(dev->dev.parent, "bit-timing calculation not available\n");
+	netdev_err(dev, "bit-timing calculation not available\n");
 	return -EINVAL;
 }
 #endif /* CONFIG_CAN_CALC_BITTIMING */
@@ -313,8 +313,7 @@ void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
 		priv->echo_skb[idx] = skb;
 	} else {
 		/* locking problem with netif_stop_queue() ?? */
-		dev_err(dev->dev.parent, "%s: BUG! echo_skb is occupied!\n",
-			__func__);
+		netdev_err(dev, "%s: BUG! echo_skb is occupied!\n", __func__);
 		kfree_skb(skb);
 	}
 }
@@ -392,7 +391,7 @@ void can_restart(unsigned long data)
 	stats->rx_bytes += cf->can_dlc;
 
 restart:
-	dev_dbg(dev->dev.parent, "restarted\n");
+	netdev_dbg(dev, "restarted\n");
 	priv->can_stats.restarts++;
 
 	/* Now restart the device */
@@ -400,7 +399,7 @@ restart:
 
 	netif_carrier_on(dev);
 	if (err)
-		dev_err(dev->dev.parent, "Error %d during restart", err);
+		netdev_err(dev, "Error %d during restart", err);
 }
 
 int can_restart_now(struct net_device *dev)
@@ -433,7 +432,7 @@ void can_bus_off(struct net_device *dev)
 {
 	struct can_priv *priv = netdev_priv(dev);
 
-	dev_dbg(dev->dev.parent, "bus-off\n");
+	netdev_dbg(dev, "bus-off\n");
 
 	netif_carrier_off(dev);
 	priv->can_stats.bus_off++;
@@ -545,7 +544,7 @@ int open_candev(struct net_device *dev)
 	struct can_priv *priv = netdev_priv(dev);
 
 	if (!priv->bittiming.tq && !priv->bittiming.bitrate) {
-		dev_err(dev->dev.parent, "bit-timing not yet defined\n");
+		netdev_err(dev, "bit-timing not yet defined\n");
 		return -EINVAL;
 	}
 
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index feb8e64..0ce914a 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -315,34 +315,34 @@ static void do_bus_err(struct net_device *dev,
 	cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
 
 	if (reg_esr & FLEXCAN_ESR_BIT1_ERR) {
-		dev_dbg(dev->dev.parent, "BIT1_ERR irq\n");
+		netdev_dbg(dev, "BIT1_ERR irq\n");
 		cf->data[2] |= CAN_ERR_PROT_BIT1;
 		tx_errors = 1;
 	}
 	if (reg_esr & FLEXCAN_ESR_BIT0_ERR) {
-		dev_dbg(dev->dev.parent, "BIT0_ERR irq\n");
+		netdev_dbg(dev, "BIT0_ERR irq\n");
 		cf->data[2] |= CAN_ERR_PROT_BIT0;
 		tx_errors = 1;
 	}
 	if (reg_esr & FLEXCAN_ESR_ACK_ERR) {
-		dev_dbg(dev->dev.parent, "ACK_ERR irq\n");
+		netdev_dbg(dev, "ACK_ERR irq\n");
 		cf->can_id |= CAN_ERR_ACK;
 		cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
 		tx_errors = 1;
 	}
 	if (reg_esr & FLEXCAN_ESR_CRC_ERR) {
-		dev_dbg(dev->dev.parent, "CRC_ERR irq\n");
+		netdev_dbg(dev, "CRC_ERR irq\n");
 		cf->data[2] |= CAN_ERR_PROT_BIT;
 		cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
 		rx_errors = 1;
 	}
 	if (reg_esr & FLEXCAN_ESR_FRM_ERR) {
-		dev_dbg(dev->dev.parent, "FRM_ERR irq\n");
+		netdev_dbg(dev, "FRM_ERR irq\n");
 		cf->data[2] |= CAN_ERR_PROT_FORM;
 		rx_errors = 1;
 	}
 	if (reg_esr & FLEXCAN_ESR_STF_ERR) {
-		dev_dbg(dev->dev.parent, "STF_ERR irq\n");
+		netdev_dbg(dev, "STF_ERR irq\n");
 		cf->data[2] |= CAN_ERR_PROT_STUFF;
 		rx_errors = 1;
 	}
@@ -371,7 +371,7 @@ static void do_state(struct net_device *dev,
 		 */
 		if (new_state >= CAN_STATE_ERROR_WARNING &&
 		    new_state <= CAN_STATE_BUS_OFF) {
-			dev_dbg(dev->dev.parent, "Error Warning IRQ\n");
+			netdev_dbg(dev, "Error Warning IRQ\n");
 			priv->can.can_stats.error_warning++;
 
 			cf->can_id |= CAN_ERR_CRTL;
@@ -387,7 +387,7 @@ static void do_state(struct net_device *dev,
 		 */
 		if (new_state >= CAN_STATE_ERROR_PASSIVE &&
 		    new_state <= CAN_STATE_BUS_OFF) {
-			dev_dbg(dev->dev.parent, "Error Passive IRQ\n");
+			netdev_dbg(dev, "Error Passive IRQ\n");
 			priv->can.can_stats.error_passive++;
 
 			cf->can_id |= CAN_ERR_CRTL;
@@ -397,8 +397,8 @@ static void do_state(struct net_device *dev,
 		}
 		break;
 	case CAN_STATE_BUS_OFF:
-		dev_err(dev->dev.parent,
-			"BUG! hardware recovered automatically from BUS_OFF\n");
+		netdev_err(dev, "BUG! "
+			   "hardware recovered automatically from BUS_OFF\n");
 		break;
 	default:
 		break;
@@ -407,7 +407,7 @@ static void do_state(struct net_device *dev,
 	/* process state changes depending on the new state */
 	switch (new_state) {
 	case CAN_STATE_ERROR_ACTIVE:
-		dev_dbg(dev->dev.parent, "Error Active\n");
+		netdev_dbg(dev, "Error Active\n");
 		cf->can_id |= CAN_ERR_PROT;
 		cf->data[2] = CAN_ERR_PROT_ACTIVE;
 		break;
@@ -629,12 +629,12 @@ static void flexcan_set_bittiming(struct net_device *dev)
 	if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
 		reg |= FLEXCAN_CTRL_SMP;
 
-	dev_info(dev->dev.parent, "writing ctrl=0x%08x\n", reg);
+	netdev_info(dev, "writing ctrl=0x%08x\n", reg);
 	flexcan_write(reg, &regs->ctrl);
 
 	/* print chip status */
-	dev_dbg(dev->dev.parent, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
-		flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
+	netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
+		   flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
 }
 
 /*
@@ -660,9 +660,8 @@ static int flexcan_chip_start(struct net_device *dev)
 
 	reg_mcr = flexcan_read(&regs->mcr);
 	if (reg_mcr & FLEXCAN_MCR_SOFTRST) {
-		dev_err(dev->dev.parent,
-			"Failed to softreset can module (mcr=0x%08x)\n",
-			reg_mcr);
+		netdev_err(dev, "Failed to softreset can module (mcr=0x%08x)\n",
+			   reg_mcr);
 		err = -ENODEV;
 		goto out;
 	}
@@ -684,7 +683,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
 		FLEXCAN_MCR_IDAM_C;
-	dev_dbg(dev->dev.parent, "%s: writing mcr=0x%08x", __func__, reg_mcr);
+	netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
 	flexcan_write(reg_mcr, &regs->mcr);
 
 	/*
@@ -710,7 +709,7 @@ static int flexcan_chip_start(struct net_device *dev)
 
 	/* save for later use */
 	priv->reg_ctrl_default = reg_ctrl;
-	dev_dbg(dev->dev.parent, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
+	netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
 	flexcan_write(reg_ctrl, &regs->ctrl);
 
 	for (i = 0; i < ARRAY_SIZE(regs->cantxfg); i++) {
@@ -742,8 +741,8 @@ static int flexcan_chip_start(struct net_device *dev)
 	flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
 
 	/* print chip status */
-	dev_dbg(dev->dev.parent, "%s: reading mcr=0x%08x ctrl=0x%08x\n",
-		__func__, flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
+	netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__,
+		   flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
 
 	return 0;
 
@@ -881,8 +880,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
 	 */
 	reg = flexcan_read(&regs->mcr);
 	if (!(reg & FLEXCAN_MCR_FEN)) {
-		dev_err(dev->dev.parent,
-			"Could not enable RX FIFO, unsupported core\n");
+		netdev_err(dev, "Could not enable RX FIFO, unsupported core\n");
 		err = -ENODEV;
 		goto out;
 	}
diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
index 330140e..346785c 100644
--- a/drivers/net/can/mcp251x.c
+++ b/drivers/net/can/mcp251x.c
@@ -712,8 +712,7 @@ static void mcp251x_error_skb(struct net_device *net, int can_id, int data1)
 		frame->data[1] = data1;
 		netif_rx_ni(skb);
 	} else {
-		dev_err(&net->dev,
-			"cannot allocate error skb\n");
+		netdev_err(net, "cannot allocate error skb\n");
 	}
 }
 
diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
index 1c82dd8..ad8e687 100644
--- a/drivers/net/can/mscan/mscan.c
+++ b/drivers/net/can/mscan/mscan.c
@@ -95,9 +95,9 @@ static int mscan_set_mode(struct net_device *dev, u8 mode)
 			 * any, at once.
 			 */
 			if (i >= MSCAN_SET_MODE_RETRIES)
-				dev_dbg(dev->dev.parent,
-					"device failed to enter sleep mode. "
-					"We proceed anyhow.\n");
+				netdev_dbg(dev,
+					   "device failed to enter sleep mode. "
+					   "We proceed anyhow.\n");
 			else
 				priv->can.state = CAN_STATE_SLEEPING;
 		}
@@ -213,7 +213,7 @@ static netdev_tx_t mscan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	switch (hweight8(i)) {
 	case 0:
 		netif_stop_queue(dev);
-		dev_err(dev->dev.parent, "Tx Ring full when queue awake!\n");
+		netdev_err(dev, "Tx Ring full when queue awake!\n");
 		return NETDEV_TX_BUSY;
 	case 1:
 		/*
@@ -352,7 +352,7 @@ static void mscan_get_err_frame(struct net_device *dev, struct can_frame *frame,
 	struct net_device_stats *stats = &dev->stats;
 	enum can_state old_state;
 
-	dev_dbg(dev->dev.parent, "error interrupt (canrflg=%#x)\n", canrflg);
+	netdev_dbg(dev, "error interrupt (canrflg=%#x)\n", canrflg);
 	frame->can_id = CAN_ERR_FLAG;
 
 	if (canrflg & MSCAN_OVRIF) {
@@ -427,7 +427,7 @@ static int mscan_rx_poll(struct napi_struct *napi, int quota)
 		skb = alloc_can_skb(dev, &frame);
 		if (!skb) {
 			if (printk_ratelimit())
-				dev_notice(dev->dev.parent, "packet dropped\n");
+				netdev_notice(dev, "packet dropped\n");
 			stats->rx_dropped++;
 			out_8(&regs->canrflg, canrflg);
 			continue;
@@ -551,8 +551,7 @@ static int mscan_do_set_bittiming(struct net_device *dev)
 		BTR1_SET_TSEG2(bt->phase_seg2) |
 		BTR1_SET_SAM(priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES));
 
-	dev_info(dev->dev.parent, "setting BTR0=0x%02x BTR1=0x%02x\n",
-		btr0, btr1);
+	netdev_info(dev, "setting BTR0=0x%02x BTR1=0x%02x\n", btr0, btr1);
 
 	out_8(&regs->canbtr0, btr0);
 	out_8(&regs->canbtr1, btr1);
@@ -575,7 +574,7 @@ static int mscan_open(struct net_device *dev)
 
 	ret = request_irq(dev->irq, mscan_isr, 0, dev->name, dev);
 	if (ret < 0) {
-		dev_err(dev->dev.parent, "failed to attach interrupt\n");
+		netdev_err(dev, "failed to attach interrupt\n");
 		goto exit_napi_disable;
 	}
 
diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
index 04a3f1b..ebbcfca 100644
--- a/drivers/net/can/sja1000/sja1000.c
+++ b/drivers/net/can/sja1000/sja1000.c
@@ -128,7 +128,7 @@ static void set_reset_mode(struct net_device *dev)
 		status = priv->read_reg(priv, REG_MOD);
 	}
 
-	dev_err(dev->dev.parent, "setting SJA1000 into reset mode failed!\n");
+	netdev_err(dev, "setting SJA1000 into reset mode failed!\n");
 }
 
 static void set_normal_mode(struct net_device *dev)
@@ -156,7 +156,7 @@ static void set_normal_mode(struct net_device *dev)
 		status = priv->read_reg(priv, REG_MOD);
 	}
 
-	dev_err(dev->dev.parent, "setting SJA1000 into normal mode failed!\n");
+	netdev_err(dev, "setting SJA1000 into normal mode failed!\n");
 }
 
 static void sja1000_start(struct net_device *dev)
@@ -209,8 +209,7 @@ static int sja1000_set_bittiming(struct net_device *dev)
 	if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
 		btr1 |= 0x80;
 
-	dev_info(dev->dev.parent,
-		 "setting BTR0=0x%02x BTR1=0x%02x\n", btr0, btr1);
+	netdev_info(dev, "setting BTR0=0x%02x BTR1=0x%02x\n", btr0, btr1);
 
 	priv->write_reg(priv, REG_BTR0, btr0);
 	priv->write_reg(priv, REG_BTR1, btr1);
@@ -378,7 +377,7 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
 
 	if (isrc & IRQ_DOI) {
 		/* data overrun interrupt */
-		dev_dbg(dev->dev.parent, "data overrun interrupt\n");
+		netdev_dbg(dev, "data overrun interrupt\n");
 		cf->can_id |= CAN_ERR_CRTL;
 		cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
 		stats->rx_over_errors++;
@@ -388,7 +387,7 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
 
 	if (isrc & IRQ_EI) {
 		/* error warning interrupt */
-		dev_dbg(dev->dev.parent, "error warning interrupt\n");
+		netdev_dbg(dev, "error warning interrupt\n");
 
 		if (status & SR_BS) {
 			state = CAN_STATE_BUS_OFF;
@@ -429,7 +428,7 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
 	}
 	if (isrc & IRQ_EPI) {
 		/* error passive interrupt */
-		dev_dbg(dev->dev.parent, "error passive interrupt\n");
+		netdev_dbg(dev, "error passive interrupt\n");
 		if (status & SR_ES)
 			state = CAN_STATE_ERROR_PASSIVE;
 		else
@@ -437,7 +436,7 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
 	}
 	if (isrc & IRQ_ALI) {
 		/* arbitration lost interrupt */
-		dev_dbg(dev->dev.parent, "arbitration lost interrupt\n");
+		netdev_dbg(dev, "arbitration lost interrupt\n");
 		alc = priv->read_reg(priv, REG_ALC);
 		priv->can.can_stats.arbitration_lost++;
 		stats->tx_errors++;
@@ -495,7 +494,7 @@ irqreturn_t sja1000_interrupt(int irq, void *dev_id)
 		status = priv->read_reg(priv, REG_SR);
 
 		if (isrc & IRQ_WUI)
-			dev_warn(dev->dev.parent, "wakeup interrupt\n");
+			netdev_warn(dev, "wakeup interrupt\n");
 
 		if (isrc & IRQ_TI) {
 			/* transmission complete interrupt */
@@ -522,7 +521,7 @@ irqreturn_t sja1000_interrupt(int irq, void *dev_id)
 		priv->post_irq(priv);
 
 	if (n >= SJA1000_MAX_IRQ)
-		dev_dbg(dev->dev.parent, "%d messages handled in ISR", n);
+		netdev_dbg(dev, "%d messages handled in ISR", n);
 
 	return (n) ? IRQ_HANDLED : IRQ_NONE;
 }
diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index df809e3..d50e5e0 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -306,7 +306,7 @@ static int ti_hecc_set_btc(struct ti_hecc_priv *priv)
 		if (bit_timing->brp > 4)
 			can_btc |= HECC_CANBTC_SAM;
 		else
-			dev_warn(priv->ndev->dev.parent, "WARN: Triple" \
+			netdev_warn(priv->ndev, "WARN: Triple"
 				"sampling not set due to h/w limitations");
 	}
 	can_btc |= ((bit_timing->sjw - 1) & 0x3) << 8;
@@ -315,7 +315,7 @@ static int ti_hecc_set_btc(struct ti_hecc_priv *priv)
 	/* ERM being set to 0 by default meaning resync at falling edge */
 
 	hecc_write(priv, HECC_CANBTC, can_btc);
-	dev_info(priv->ndev->dev.parent, "setting CANBTC=%#x\n", can_btc);
+	netdev_info(priv->ndev, "setting CANBTC=%#x\n", can_btc);
 
 	return 0;
 }
@@ -332,7 +332,7 @@ static void ti_hecc_reset(struct net_device *ndev)
 	u32 cnt;
 	struct ti_hecc_priv *priv = netdev_priv(ndev);
 
-	dev_dbg(ndev->dev.parent, "resetting hecc ...\n");
+	netdev_dbg(ndev, "resetting hecc ...\n");
 	hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_SRES);
 
 	/* Set change control request and wait till enabled */
@@ -496,7 +496,7 @@ static netdev_tx_t ti_hecc_xmit(struct sk_buff *skb, struct net_device *ndev)
 	if (unlikely(hecc_read(priv, HECC_CANME) & mbx_mask)) {
 		spin_unlock_irqrestore(&priv->mbx_lock, flags);
 		netif_stop_queue(ndev);
-		dev_err(priv->ndev->dev.parent,
+		dev_err(priv->ndev,
 			"BUG: TX mbx not ready tx_head=%08X, tx_tail=%08X\n",
 			priv->tx_head, priv->tx_tail);
 		return NETDEV_TX_BUSY;
@@ -550,7 +550,7 @@ static int ti_hecc_rx_pkt(struct ti_hecc_priv *priv, int mbxno)
 	skb = alloc_can_skb(priv->ndev, &cf);
 	if (!skb) {
 		if (printk_ratelimit())
-			dev_err(priv->ndev->dev.parent,
+			netdev_err(priv->ndev,
 				"ti_hecc_rx_pkt: alloc_can_skb() failed\n");
 		return -ENOMEM;
 	}
@@ -668,7 +668,7 @@ static int ti_hecc_error(struct net_device *ndev, int int_status,
 	skb = alloc_can_err_skb(ndev, &cf);
 	if (!skb) {
 		if (printk_ratelimit())
-			dev_err(priv->ndev->dev.parent,
+			netdev_err(priv->ndev,
 				"ti_hecc_error: alloc_can_err_skb() failed\n");
 		return -ENOMEM;
 	}
@@ -684,7 +684,7 @@ static int ti_hecc_error(struct net_device *ndev, int int_status,
 				cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
 		}
 		hecc_set_bit(priv, HECC_CANES, HECC_CANES_EW);
-		dev_dbg(priv->ndev->dev.parent, "Error Warning interrupt\n");
+		netdev_dbg(priv->ndev, "Error Warning interrupt\n");
 		hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_CCR);
 	}
 
@@ -699,7 +699,7 @@ static int ti_hecc_error(struct net_device *ndev, int int_status,
 				cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
 		}
 		hecc_set_bit(priv, HECC_CANES, HECC_CANES_EP);
-		dev_dbg(priv->ndev->dev.parent, "Error passive interrupt\n");
+		netdev_dbg(priv->ndev, "Error passive interrupt\n");
 		hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_CCR);
 	}
 
@@ -824,7 +824,7 @@ static int ti_hecc_open(struct net_device *ndev)
 	err = request_irq(ndev->irq, ti_hecc_interrupt, IRQF_SHARED,
 			ndev->name, ndev);
 	if (err) {
-		dev_err(ndev->dev.parent, "error requesting interrupt\n");
+		netdev_err(ndev, "error requesting interrupt\n");
 		return err;
 	}
 
@@ -833,7 +833,7 @@ static int ti_hecc_open(struct net_device *ndev)
 	/* Open common can device */
 	err = open_candev(ndev);
 	if (err) {
-		dev_err(ndev->dev.parent, "open_candev() failed %d\n", err);
+		netdev_err(ndev, "open_candev() failed %d\n", err);
 		ti_hecc_transceiver_switch(priv, 0);
 		free_irq(ndev->irq, ndev);
 		return err;
diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
index a72c7bf..bf088b5 100644
--- a/drivers/net/can/usb/ems_usb.c
+++ b/drivers/net/can/usb/ems_usb.c
@@ -288,8 +288,7 @@ static void ems_usb_read_interrupt_callback(struct urb *urb)
 		return;
 
 	default:
-		dev_info(netdev->dev.parent, "Rx interrupt aborted %d\n",
-			 urb->status);
+		netdev_info(netdev, "Rx interrupt aborted %d\n", urb->status);
 		break;
 	}
 
@@ -298,8 +297,7 @@ static void ems_usb_read_interrupt_callback(struct urb *urb)
 	if (err == -ENODEV)
 		netif_device_detach(netdev);
 	else if (err)
-		dev_err(netdev->dev.parent,
-			"failed resubmitting intr urb: %d\n", err);
+		netdev_err(netdev, "failed resubmitting intr urb: %d\n", err);
 }
 
 static void ems_usb_rx_can_msg(struct ems_usb *dev, struct ems_cpc_msg *msg)
@@ -431,8 +429,7 @@ static void ems_usb_read_bulk_callback(struct urb *urb)
 		return;
 
 	default:
-		dev_info(netdev->dev.parent, "Rx URB aborted (%d)\n",
-			 urb->status);
+		netdev_info(netdev, "Rx URB aborted (%d)\n", urb->status);
 		goto resubmit_urb;
 	}
 
@@ -477,7 +474,7 @@ static void ems_usb_read_bulk_callback(struct urb *urb)
 			msg_count--;
 
 			if (start > urb->transfer_buffer_length) {
-				dev_err(netdev->dev.parent, "format error\n");
+				netdev_err(netdev, "format error\n");
 				break;
 			}
 		}
@@ -493,8 +490,8 @@ resubmit_urb:
 	if (retval == -ENODEV)
 		netif_device_detach(netdev);
 	else if (retval)
-		dev_err(netdev->dev.parent,
-			"failed resubmitting read bulk urb: %d\n", retval);
+		netdev_err(netdev,
+			   "failed resubmitting read bulk urb: %d\n", retval);
 }
 
 /*
@@ -521,8 +518,7 @@ static void ems_usb_write_bulk_callback(struct urb *urb)
 		return;
 
 	if (urb->status)
-		dev_info(netdev->dev.parent, "Tx URB aborted (%d)\n",
-			 urb->status);
+		netdev_info(netdev, "Tx URB aborted (%d)\n", urb->status);
 
 	netdev->trans_start = jiffies;
 
@@ -605,16 +601,14 @@ static int ems_usb_start(struct ems_usb *dev)
 		/* create a URB, and a buffer for it */
 		urb = usb_alloc_urb(0, GFP_KERNEL);
 		if (!urb) {
-			dev_err(netdev->dev.parent,
-				"No memory left for URBs\n");
+			netdev_err(netdev, "No memory left for URBs\n");
 			return -ENOMEM;
 		}
 
 		buf = usb_alloc_coherent(dev->udev, RX_BUFFER_SIZE, GFP_KERNEL,
 					 &urb->transfer_dma);
 		if (!buf) {
-			dev_err(netdev->dev.parent,
-				"No memory left for USB buffer\n");
+			netdev_err(netdev, "No memory left for USB buffer\n");
 			usb_free_urb(urb);
 			return -ENOMEM;
 		}
@@ -642,13 +636,13 @@ static int ems_usb_start(struct ems_usb *dev)
 
 	/* Did we submit any URBs */
 	if (i == 0) {
-		dev_warn(netdev->dev.parent, "couldn't setup read URBs\n");
+		netdev_warn(netdev, "couldn't setup read URBs\n");
 		return err;
 	}
 
 	/* Warn if we've couldn't transmit all the URBs */
 	if (i < MAX_RX_URBS)
-		dev_warn(netdev->dev.parent, "rx performance may be slow\n");
+		netdev_warn(netdev, "rx performance may be slow\n");
 
 	/* Setup and start interrupt URB */
 	usb_fill_int_urb(dev->intr_urb, dev->udev,
@@ -662,8 +656,7 @@ static int ems_usb_start(struct ems_usb *dev)
 		if (err == -ENODEV)
 			netif_device_detach(dev->netdev);
 
-		dev_warn(netdev->dev.parent, "intr URB submit failed: %d\n",
-			 err);
+		netdev_warn(netdev, "intr URB submit failed: %d\n", err);
 
 		return err;
 	}
@@ -695,7 +688,7 @@ failed:
 	if (err == -ENODEV)
 		netif_device_detach(dev->netdev);
 
-	dev_warn(netdev->dev.parent, "couldn't submit control: %d\n", err);
+	netdev_warn(netdev, "couldn't submit control: %d\n", err);
 
 	return err;
 }
@@ -735,8 +728,7 @@ static int ems_usb_open(struct net_device *netdev)
 		if (err == -ENODEV)
 			netif_device_detach(dev->netdev);
 
-		dev_warn(netdev->dev.parent, "couldn't start device: %d\n",
-			 err);
+		netdev_warn(netdev, "couldn't start device: %d\n", err);
 
 		close_candev(netdev);
 
@@ -769,13 +761,13 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff *skb, struct net_device *ne
 	/* create a URB, and a buffer for it, and copy the data to the URB */
 	urb = usb_alloc_urb(0, GFP_ATOMIC);
 	if (!urb) {
-		dev_err(netdev->dev.parent, "No memory left for URBs\n");
+		netdev_err(netdev, "No memory left for URBs\n");
 		goto nomem;
 	}
 
 	buf = usb_alloc_coherent(dev->udev, size, GFP_ATOMIC, &urb->transfer_dma);
 	if (!buf) {
-		dev_err(netdev->dev.parent, "No memory left for USB buffer\n");
+		netdev_err(netdev, "No memory left for USB buffer\n");
 		usb_free_urb(urb);
 		goto nomem;
 	}
@@ -818,7 +810,7 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff *skb, struct net_device *ne
 		usb_unanchor_urb(urb);
 		usb_free_coherent(dev->udev, size, buf, urb->transfer_dma);
 
-		dev_warn(netdev->dev.parent, "couldn't find free context\n");
+		netdev_warn(netdev, "couldn't find free context\n");
 
 		return NETDEV_TX_BUSY;
 	}
@@ -849,7 +841,7 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff *skb, struct net_device *ne
 		if (err == -ENODEV) {
 			netif_device_detach(netdev);
 		} else {
-			dev_warn(netdev->dev.parent, "failed tx_urb %d\n", err);
+			netdev_warn(netdev, "failed tx_urb %d\n", err);
 
 			stats->tx_dropped++;
 		}
@@ -889,7 +881,7 @@ static int ems_usb_close(struct net_device *netdev)
 
 	/* Set CAN controller to reset mode */
 	if (ems_usb_write_mode(dev, SJA1000_MOD_RM))
-		dev_warn(netdev->dev.parent, "couldn't stop device");
+		netdev_warn(netdev, "couldn't stop device");
 
 	close_candev(netdev);
 
@@ -926,7 +918,7 @@ static int ems_usb_set_mode(struct net_device *netdev, enum can_mode mode)
 	switch (mode) {
 	case CAN_MODE_START:
 		if (ems_usb_write_mode(dev, SJA1000_MOD_NORMAL))
-			dev_warn(netdev->dev.parent, "couldn't start device");
+			netdev_warn(netdev, "couldn't start device");
 
 		if (netif_queue_stopped(netdev))
 			netif_wake_queue(netdev);
@@ -951,8 +943,7 @@ static int ems_usb_set_bittiming(struct net_device *netdev)
 	if (dev->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
 		btr1 |= 0x80;
 
-	dev_info(netdev->dev.parent, "setting BTR0=0x%02x BTR1=0x%02x\n",
-		 btr0, btr1);
+	netdev_info(netdev, "setting BTR0=0x%02x BTR1=0x%02x\n", btr0, btr1);
 
 	dev->active_params.msg.can_params.cc_params.sja1000.btr0 = btr0;
 	dev->active_params.msg.can_params.cc_params.sja1000.btr1 = btr1;
@@ -1057,15 +1048,13 @@ static int ems_usb_probe(struct usb_interface *intf,
 
 	err = ems_usb_command_msg(dev, &dev->active_params);
 	if (err) {
-		dev_err(netdev->dev.parent,
-			"couldn't initialize controller: %d\n", err);
+		netdev_err(netdev, "couldn't initialize controller: %d\n", err);
 		goto cleanup_tx_msg_buffer;
 	}
 
 	err = register_candev(netdev);
 	if (err) {
-		dev_err(netdev->dev.parent,
-			"couldn't register CAN device: %d\n", err);
+		netdev_err(netdev, "couldn't register CAN device: %d\n", err);
 		goto cleanup_tx_msg_buffer;
 	}
 
diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
index eb8b0e6..6f3fafb 100644
--- a/drivers/net/can/usb/esd_usb2.c
+++ b/drivers/net/can/usb/esd_usb2.c
@@ -470,8 +470,7 @@ static void esd_usb2_write_bulk_callback(struct urb *urb)
 		return;
 
 	if (urb->status)
-		dev_info(netdev->dev.parent, "Tx URB aborted (%d)\n",
-			 urb->status);
+		netdev_info(netdev, "Tx URB aborted (%d)\n", urb->status);
 
 	netdev->trans_start = jiffies;
 }
@@ -651,7 +650,7 @@ failed:
 	if (err == -ENODEV)
 		netif_device_detach(netdev);
 
-	dev_err(netdev->dev.parent, "couldn't start device: %d\n", err);
+	netdev_err(netdev, "couldn't start device: %d\n", err);
 
 	return err;
 }
@@ -687,8 +686,7 @@ static int esd_usb2_open(struct net_device *netdev)
 	/* finally start device */
 	err = esd_usb2_start(priv);
 	if (err) {
-		dev_warn(netdev->dev.parent,
-			 "couldn't start device: %d\n", err);
+		netdev_warn(netdev, "couldn't start device: %d\n", err);
 		close_candev(netdev);
 		return err;
 	}
@@ -721,7 +719,7 @@ static netdev_tx_t esd_usb2_start_xmit(struct sk_buff *skb,
 	/* create a URB, and a buffer for it, and copy the data to the URB */
 	urb = usb_alloc_urb(0, GFP_ATOMIC);
 	if (!urb) {
-		dev_err(netdev->dev.parent, "No memory left for URBs\n");
+		netdev_err(netdev, "No memory left for URBs\n");
 		stats->tx_dropped++;
 		dev_kfree_skb(skb);
 		goto nourbmem;
@@ -730,7 +728,7 @@ static netdev_tx_t esd_usb2_start_xmit(struct sk_buff *skb,
 	buf = usb_alloc_coherent(dev->udev, size, GFP_ATOMIC,
 				 &urb->transfer_dma);
 	if (!buf) {
-		dev_err(netdev->dev.parent, "No memory left for USB buffer\n");
+		netdev_err(netdev, "No memory left for USB buffer\n");
 		stats->tx_dropped++;
 		dev_kfree_skb(skb);
 		goto nobufmem;
@@ -766,7 +764,7 @@ static netdev_tx_t esd_usb2_start_xmit(struct sk_buff *skb,
 	 * This may never happen.
 	 */
 	if (!context) {
-		dev_warn(netdev->dev.parent, "couldn't find free context\n");
+		netdev_warn(netdev, "couldn't find free context\n");
 		ret = NETDEV_TX_BUSY;
 		goto releasebuf;
 	}
@@ -806,7 +804,7 @@ static netdev_tx_t esd_usb2_start_xmit(struct sk_buff *skb,
 		if (err == -ENODEV)
 			netif_device_detach(netdev);
 		else
-			dev_warn(netdev->dev.parent, "failed tx_urb %d\n", err);
+			netdev_warn(netdev, "failed tx_urb %d\n", err);
 
 		goto releasebuf;
 	}
@@ -845,7 +843,7 @@ static int esd_usb2_close(struct net_device *netdev)
 	for (i = 0; i <= ESD_MAX_ID_SEGMENT; i++)
 		msg.msg.filter.mask[i] = 0;
 	if (esd_usb2_send_msg(priv->usb2, &msg) < 0)
-		dev_err(netdev->dev.parent, "sending idadd message failed\n");
+		netdev_err(netdev, "sending idadd message failed\n");
 
 	/* set CAN controller to reset mode */
 	msg.msg.hdr.len = 2;
@@ -854,7 +852,7 @@ static int esd_usb2_close(struct net_device *netdev)
 	msg.msg.setbaud.rsvd = 0;
 	msg.msg.setbaud.baud = cpu_to_le32(ESD_USB2_NO_BAUDRATE);
 	if (esd_usb2_send_msg(priv->usb2, &msg) < 0)
-		dev_err(netdev->dev.parent, "sending setbaud message failed\n");
+		netdev_err(netdev, "sending setbaud message failed\n");
 
 	priv->can.state = CAN_STATE_STOPPED;
 
@@ -910,7 +908,7 @@ static int esd_usb2_set_bittiming(struct net_device *netdev)
 	msg.msg.setbaud.rsvd = 0;
 	msg.msg.setbaud.baud = cpu_to_le32(canbtr);
 
-	dev_info(netdev->dev.parent, "setting BTR=%#x\n", canbtr);
+	netdev_info(netdev, "setting BTR=%#x\n", canbtr);
 
 	return esd_usb2_send_msg(priv->usb2, &msg);
 }
@@ -988,15 +986,14 @@ static int esd_usb2_probe_one_net(struct usb_interface *intf, int index)
 
 	err = register_candev(netdev);
 	if (err) {
-		dev_err(&intf->dev,
-			"couldn't register CAN device: %d\n", err);
+		dev_err(&intf->dev, "couldn't register CAN device: %d\n", err);
 		free_candev(netdev);
 		err = -ENOMEM;
 		goto done;
 	}
 
 	dev->nets[index] = priv;
-	dev_info(netdev->dev.parent, "device %s registered\n", netdev->name);
+	netdev_info(netdev, "device %s registered\n", netdev->name);
 
 done:
 	return err;
-- 
1.7.4.1

[RFC PATCH 03/14] can: flexcan: only for bus error reporting enable berr interrupt

[Wolfgang Grandegger]

So far, the bus error (berr) interrupt source is always enabled,
even if bus error reporting is not requested (via ctrlmode flag
CAN_CTRLMODE_BERR_REPORTING). This is not necessay, at least on
the Flexcan of the i.MX28 SOC and it avoids flooding with with
bus error interrupts. State changes interrupts do still arrive
as documented. The function flexcan_has_and_handle_berr() has
been removed and error state changes and bus errors are now
packed into one error message by a common poll function, like
for other CAN controllers.

CC: Reuben Dowle <Reuben.Dowle@navico.com>
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/flexcan.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 0ce914a..095b74b 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -705,7 +705,10 @@ static int flexcan_chip_start(struct net_device *dev)
 	reg_ctrl = flexcan_read(&regs->ctrl);
 	reg_ctrl &= ~FLEXCAN_CTRL_TSYN;
 	reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF |
-		FLEXCAN_CTRL_ERR_STATE | FLEXCAN_CTRL_ERR_MSK;
+		FLEXCAN_CTRL_ERR_STATE;
+
+	if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)
+		reg_ctrl |= FLEXCAN_CTRL_ERR_MSK;
 
 	/* save for later use */
 	priv->reg_ctrl_default = reg_ctrl;
-- 
1.7.4.1

Re: [RFC PATCH 03/14] can: flexcan: only for bus error reporting enable berr interrupt

[Wolfgang Grandegger]

On 12/07/2011 03:55 PM, Wolfgang Grandegger wrote:
> So far, the bus error (berr) interrupt source is always enabled,
> even if bus error reporting is not requested (via ctrlmode flag
> CAN_CTRLMODE_BERR_REPORTING). This is not necessay, at least on
> the Flexcan of the i.MX28 SOC and it avoids flooding with with
> bus error interrupts. State changes interrupts do still arrive
> as documented. The function flexcan_has_and_handle_berr() has
> been removed and error state changes and bus errors are now
> packed into one error message by a common poll function, like
> for other CAN controllers.
> 
> CC: Reuben Dowle <Reuben.Dowle@navico.com>
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>

... and here the subject and commit message should be:

[RFC PATCH 01/14] can: flexcan: fix irq flooding by clearing all interrupt sources

As pointed out by Reuben Dowle, the TWRN_INT, RWRN_INT, BOFF_INT
interrupt sources need to be cleared as well to avoid interrupt
flooding, at least for the Flexcan on i.MX28 SOCs.

CC: Reuben Dowle <Reuben.Dowle@navico.com>
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>

Sorry for the noise.

Wolfgang.


> ---
>  drivers/net/can/flexcan.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 0ce914a..095b74b 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -705,7 +705,10 @@ static int flexcan_chip_start(struct net_device *dev)
>  	reg_ctrl = flexcan_read(&regs->ctrl);
>  	reg_ctrl &= ~FLEXCAN_CTRL_TSYN;
>  	reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF |
> -		FLEXCAN_CTRL_ERR_STATE | FLEXCAN_CTRL_ERR_MSK;
> +		FLEXCAN_CTRL_ERR_STATE;
> +
> +	if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)
> +		reg_ctrl |= FLEXCAN_CTRL_ERR_MSK;
>  
>  	/* save for later use */
>  	priv->reg_ctrl_default = reg_ctrl;

[RFC PATCH 04/14] can: bfin_can/ti_hecc/mscan: add missing do_get_berr_counter callback

[Wolfgang Grandegger]

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/bfin_can.c    |   15 +++++++++++++++
 drivers/net/can/mscan/mscan.c |   16 +++++++++++++++-
 drivers/net/can/ti_hecc.c     |   12 ++++++++++++
 3 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/bfin_can.c b/drivers/net/can/bfin_can.c
index 9cf4ddb..3f88473 100644
--- a/drivers/net/can/bfin_can.c
+++ b/drivers/net/can/bfin_can.c
@@ -221,6 +221,20 @@ static int bfin_can_set_mode(struct net_device *dev, enum can_mode mode)
 	return 0;
 }
 
+static int bfin_can_get_berr_counter(const struct net_device *dev,
+				     struct can_berr_counter *bec)
+{
+	struct bfin_can_priv *priv = netdev_priv(dev);
+	struct bfin_can_regs __iomem *reg = priv->membase;
+
+	u16 cec = bfin_read(&reg->cec);
+
+	bec->txerr = cec >> 8;
+	bec->rxerr = cec;
+
+	return 0;
+}
+
 static int bfin_can_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct bfin_can_priv *priv = netdev_priv(dev);
@@ -505,6 +519,7 @@ struct net_device *alloc_bfin_candev(void)
 	priv->can.bittiming_const = &bfin_can_bittiming_const;
 	priv->can.do_set_bittiming = bfin_can_set_bittiming;
 	priv->can.do_set_mode = bfin_can_set_mode;
+	priv->can.do_get_berr_counter = bfin_can_get_berr_counter;
 	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
 
 	return dev;
diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
index ad8e687..41a2a2d 100644
--- a/drivers/net/can/mscan/mscan.c
+++ b/drivers/net/can/mscan/mscan.c
@@ -559,6 +559,18 @@ static int mscan_do_set_bittiming(struct net_device *dev)
 	return 0;
 }
 
+static int mscan_get_berr_counter(const struct net_device *dev,
+				  struct can_berr_counter *bec)
+{
+	struct mscan_priv *priv = netdev_priv(dev);
+	struct mscan_regs __iomem *regs = priv->reg_base;
+
+	bec->txerr = in_8(&regs->cantxerr);
+	bec->rxerr = in_8(&regs->canrxerr);
+
+	return 0;
+}
+
 static int mscan_open(struct net_device *dev)
 {
 	int ret;
@@ -638,8 +650,10 @@ int register_mscandev(struct net_device *dev, int mscan_clksrc)
 	else
 		ctl1 &= ~MSCAN_CLKSRC;
 
-	if (priv->type == MSCAN_TYPE_MPC5121)
+	if (priv->type == MSCAN_TYPE_MPC5121) {
+		priv->can.do_get_berr_counter = mscan_get_berr_counter;
 		ctl1 |= MSCAN_BORM; /* bus-off recovery upon request */
+	}
 
 	ctl1 |= MSCAN_CANE;
 	out_8(&regs->canctl1, ctl1);
diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index d50e5e0..ebaee3c 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -458,6 +458,17 @@ static int ti_hecc_do_set_mode(struct net_device *ndev, enum can_mode mode)
 	return ret;
 }
 
+static int ti_hecc_get_berr_counter(const struct net_device *dev,
+					struct can_berr_counter *bec)
+{
+	struct ti_hecc_priv *priv = netdev_priv(ndev);
+
+	bec->txerr = hecc_read(priv, HECC_CANTEC);
+	bec->rxerr = hecc_read(priv, HECC_CANREC);
+
+	return 0;
+}
+
 /*
  * ti_hecc_xmit: HECC Transmit
  *
@@ -922,6 +933,7 @@ static int ti_hecc_probe(struct platform_device *pdev)
 	priv->can.bittiming_const = &ti_hecc_bittiming_const;
 	priv->can.do_set_mode = ti_hecc_do_set_mode;
 	priv->can.do_get_state = ti_hecc_get_state;
+	priv->can.do_get_berr_counter = ti_hecc_get_berr_counter;
 	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
 
 	spin_lock_init(&priv->mbx_lock);
-- 
1.7.4.1

[RFC PATCH 05/14] can: add error counters to the data fields of any CAN error message

[Wolfgang Grandegger]

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/dev.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index ac4ac92..400d871 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -476,6 +476,7 @@ EXPORT_SYMBOL_GPL(alloc_can_skb);
 
 struct sk_buff *alloc_can_err_skb(struct net_device *dev, struct can_frame **cf)
 {
+	struct can_priv *priv = netdev_priv(dev);
 	struct sk_buff *skb;
 
 	skb = alloc_can_skb(dev, cf);
@@ -485,6 +486,16 @@ struct sk_buff *alloc_can_err_skb(struct net_device *dev, struct can_frame **cf)
 	(*cf)->can_id = CAN_ERR_FLAG;
 	(*cf)->can_dlc = CAN_ERR_DLC;
 
+	/* For each message, add TX and RX error counters */
+	if (priv->do_get_berr_counter) {
+		struct can_berr_counter bec;
+
+		if (!priv->do_get_berr_counter(dev, &bec)) {
+			(*cf)->data[6] = bec.txerr;
+			(*cf)->data[7] = bec.rxerr;
+		}
+	}
+
 	return skb;
 }
 EXPORT_SYMBOL_GPL(alloc_can_err_skb);
-- 
1.7.4.1

[RFC PATCH 06/14] can: dev: consolidate error state change handling

[Wolfgang Grandegger]

This patch provides the core support to allow state changes going down
including "back to error active".

For any state change the CAN_ERR_STATE_CHANGE bit will be set in the
can_id. If the state gets worse, the CAN_ERR_CRTL bis is set as usual
also for backward compatibility. If the state is back to error active,
the data[1] field will be set to CAN_ERR_CRTL_ACTIVE. The state change
management will be done by a common "can_change_state()" function doing
all the necessary bit settings and counter increments.

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/dev.c     |   69 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/can/dev.h   |   11 +++++++
 include/linux/can/error.h |    2 +
 3 files changed, 82 insertions(+), 0 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 400d871..d8e4ebb 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -242,6 +242,75 @@ static int can_get_bittiming(struct net_device *dev, struct can_bittiming *bt)
 	return 0;
 }
 
+void can_change_state(struct net_device *dev, struct can_frame *cf,
+		      enum can_state state, unsigned int err_dir)
+{
+	struct can_priv *priv = netdev_priv(dev);
+
+	/* Function should only be called in case of real state changes */
+	if (state == priv->state) {
+		netdev_warn(dev, "%s: oops, state did not change", __func__);
+		return;
+	}
+
+	/* This message reports CAN error state changes */
+	cf->can_id |= CAN_ERR_STATE_CHANGE;
+	/* If it get's worse, we set the controller problem bit */
+	if (state > priv->state && state != CAN_STATE_BUS_OFF)
+		cf->can_id |= CAN_ERR_CRTL;
+
+	switch (state) {
+	case CAN_STATE_ERROR_WARNING:
+	case CAN_STATE_ERROR_PASSIVE:
+		/* State change triggered by TX or RX errors? */
+		if (err_dir == CAN_ERR_DIR_UNKNOWN &&
+		    priv->do_get_berr_counter) {
+			if (cf->data[6] > cf->data[7])
+				err_dir = CAN_ERR_DIR_TX;
+			else if (cf->data[7] > cf->data[6])
+				err_dir = CAN_ERR_DIR_RX;
+		}
+
+		if (state == CAN_STATE_ERROR_WARNING) {
+			priv->can_stats.error_warning++;
+			if (err_dir == CAN_ERR_DIR_TX)
+				cf->data[1] = CAN_ERR_CRTL_TX_WARNING;
+			else if (err_dir == CAN_ERR_DIR_RX)
+				cf->data[1] = CAN_ERR_CRTL_RX_WARNING;
+			else
+				cf->data[1] = CAN_ERR_CRTL_TX_WARNING |
+					CAN_ERR_CRTL_RX_WARNING;
+		} else {
+			priv->can_stats.error_passive++;
+			if (err_dir == CAN_ERR_DIR_TX)
+				cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE;
+			else if (err_dir == CAN_ERR_DIR_RX)
+				cf->data[1] = CAN_ERR_CRTL_RX_PASSIVE;
+			else
+				cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
+					CAN_ERR_CRTL_RX_PASSIVE;
+		}
+		break;
+
+	case CAN_STATE_ERROR_ACTIVE:
+		cf->data[1] = CAN_ERR_CRTL_ACTIVE; /* back to error active */
+		break;
+
+	case CAN_STATE_BUS_OFF:
+		priv->can_stats.bus_off++;
+		cf->can_id |= CAN_ERR_BUSOFF;
+		break;
+
+	default:
+		break;
+
+	}
+
+	/* Finally set new state */
+	priv->state = state;
+}
+EXPORT_SYMBOL_GPL(can_change_state);
+
 /*
  * Local echo of CAN messages
  *
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index a0969fc..a50862f 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -27,6 +27,13 @@ enum can_mode {
 };
 
 /*
+ * CAN error direction
+ */
+#define CAN_ERR_DIR_UNKNOWN	0x0
+#define CAN_ERR_DIR_RX		0x1
+#define CAN_ERR_DIR_TX		0x2
+
+/*
  * CAN common private data
  */
 struct can_priv {
@@ -90,6 +97,9 @@ void unregister_candev(struct net_device *dev);
 int can_restart_now(struct net_device *dev);
 void can_bus_off(struct net_device *dev);
 
+void can_change_state(struct net_device *dev, struct can_frame *cf,
+		      enum can_state state, unsigned int err_dir);
+
 void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
 		      unsigned int idx);
 void can_get_echo_skb(struct net_device *dev, unsigned int idx);
@@ -99,4 +109,5 @@ struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf);
 struct sk_buff *alloc_can_err_skb(struct net_device *dev,
 				  struct can_frame **cf);
 
+
 #endif /* CAN_DEV_H */
diff --git a/include/linux/can/error.h b/include/linux/can/error.h
index 63e855e..44c893c 100644
--- a/include/linux/can/error.h
+++ b/include/linux/can/error.h
@@ -24,6 +24,7 @@
 #define CAN_ERR_BUSOFF       0x00000040U /* bus off */
 #define CAN_ERR_BUSERROR     0x00000080U /* bus error (may flood!) */
 #define CAN_ERR_RESTARTED    0x00000100U /* controller restarted */
+#define CAN_ERR_STATE_CHANGE 0x00000200U /* CAN error state change / data[1] */
 
 /* arbitration lost in bit ... / data[0] */
 #define CAN_ERR_LOSTARB_UNSPEC   0x00 /* unspecified */
@@ -39,6 +40,7 @@
 #define CAN_ERR_CRTL_TX_PASSIVE  0x20 /* reached error passive status TX */
 				      /* (at least one error counter exceeds */
 				      /* the protocol-defined level of 127)  */
+#define CAN_ERR_CRTL_ACTIVE      0x40 /* recovered to error active state */
 
 /* error in CAN protocol (type) / data[2] */
 #define CAN_ERR_PROT_UNSPEC      0x00 /* unspecified */
-- 
1.7.4.1

[RFC PATCH 07/14] can: flexcan: consolidate error state handling

[Wolfgang Grandegger]

Allow state to decrease bus-off->passive->warning->active.
Use the common function can_chage_state().

Here is an example output of "candump -candump -td -e any,0:0,#FFFFFFFF"
for a recovery from error passive state due to no ack/cable (reconnect
after 5s) for a Flexcan controller on the i.MX28 SOC:

 (000.202225)  can0   07  [8] 07 B4 4F 67 3A 90 D9 E4
 (000.200425)  can0  200002AC  [8] 00 08 00 19 00 00 65 00   ERRORFRAME
        controller-problem{tx-error-warning}
        protocol-violation{{}{acknowledge-slot}}
        no-acknowledgement-on-tx
        bus-error
        state-change{tx-error-warning}
        error-counter-tx-rx{{101}{0}}
 (004.869425)  can0  200002AC  [8] 00 20 00 19 00 00 84 00   ERRORFRAME
        controller-problem{tx-error-passive}
        protocol-violation{{}{acknowledge-slot}}
        no-acknowledgement-on-tx
        bus-error
        state-change{tx-error-passive}
        error-counter-tx-rx{{132}{0}}
 (000.000156)  can0    8  [2] C7 6A
 ...
 (000.000187)  can0  20000200  [8] 00 08 00 00 00 00 7F 00   ERRORFRAME
        state-change{tx-error-warning}
        error-counter-tx-rx{{127}{0}}
 (000.000032)  can0    D  [2] 66 5B
 ...
 (000.200487)  can0   3A  [8] CA BC FF 2C C3 C2 1C 63
 (000.200458)  can0  20000200  [8] 00 40 00 00 00 00 5F 00   ERRORFRAME
        state-change{back-to-error-active}
        error-counter-tx-rx{{95}{0}}
 (000.000125)  can0   3B  [8] 6D 40 BC 19 EA 35 0C 58

The state machine of the Flexcan is somehow buggy. The state change to
error passive is not reported in time but when message sending
restarted.

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/flexcan.c |   86 ++++++--------------------------------------
 1 files changed, 12 insertions(+), 74 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 095b74b..73e0597 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -354,72 +354,6 @@ static void do_bus_err(struct net_device *dev,
 		dev->stats.tx_errors++;
 }
 
-static void do_state(struct net_device *dev,
-		     struct can_frame *cf, enum can_state new_state)
-{
-	struct flexcan_priv *priv = netdev_priv(dev);
-	struct can_berr_counter bec;
-
-	flexcan_get_berr_counter(dev, &bec);
-
-	switch (priv->can.state) {
-	case CAN_STATE_ERROR_ACTIVE:
-		/*
-		 * from: ERROR_ACTIVE
-		 * to  : ERROR_WARNING, ERROR_PASSIVE, BUS_OFF
-		 * =>  : there was a warning int
-		 */
-		if (new_state >= CAN_STATE_ERROR_WARNING &&
-		    new_state <= CAN_STATE_BUS_OFF) {
-			netdev_dbg(dev, "Error Warning IRQ\n");
-			priv->can.can_stats.error_warning++;
-
-			cf->can_id |= CAN_ERR_CRTL;
-			cf->data[1] = (bec.txerr > bec.rxerr) ?
-				CAN_ERR_CRTL_TX_WARNING :
-				CAN_ERR_CRTL_RX_WARNING;
-		}
-	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
-		/*
-		 * from: ERROR_ACTIVE, ERROR_WARNING
-		 * to  : ERROR_PASSIVE, BUS_OFF
-		 * =>  : error passive int
-		 */
-		if (new_state >= CAN_STATE_ERROR_PASSIVE &&
-		    new_state <= CAN_STATE_BUS_OFF) {
-			netdev_dbg(dev, "Error Passive IRQ\n");
-			priv->can.can_stats.error_passive++;
-
-			cf->can_id |= CAN_ERR_CRTL;
-			cf->data[1] = (bec.txerr > bec.rxerr) ?
-				CAN_ERR_CRTL_TX_PASSIVE :
-				CAN_ERR_CRTL_RX_PASSIVE;
-		}
-		break;
-	case CAN_STATE_BUS_OFF:
-		netdev_err(dev, "BUG! "
-			   "hardware recovered automatically from BUS_OFF\n");
-		break;
-	default:
-		break;
-	}
-
-	/* process state changes depending on the new state */
-	switch (new_state) {
-	case CAN_STATE_ERROR_ACTIVE:
-		netdev_dbg(dev, "Error Active\n");
-		cf->can_id |= CAN_ERR_PROT;
-		cf->data[2] = CAN_ERR_PROT_ACTIVE;
-		break;
-	case CAN_STATE_BUS_OFF:
-		cf->can_id |= CAN_ERR_BUSOFF;
-		can_bus_off(dev);
-		break;
-	default:
-		break;
-	}
-}
-
 static int flexcan_poll_error_state(struct net_device *dev, u32 reg_esr)
 {
 	struct flexcan_priv *priv = netdev_priv(dev);
@@ -431,14 +365,19 @@ static int flexcan_poll_error_state(struct net_device *dev, u32 reg_esr)
 	flt = reg_esr & FLEXCAN_ESR_FLT_CONF_MASK;
 	if (likely(flt == FLEXCAN_ESR_FLT_CONF_ACTIVE)) {
 		if (likely(!(reg_esr & (FLEXCAN_ESR_TX_WRN |
-					FLEXCAN_ESR_RX_WRN))))
+					FLEXCAN_ESR_RX_WRN)))) {
 			new_state = CAN_STATE_ERROR_ACTIVE;
-		else
+			if (priv->can.state == CAN_STATE_BUS_OFF)
+				netif_start_queue(dev);
+		} else {
 			new_state = CAN_STATE_ERROR_WARNING;
-	} else if (unlikely(flt == FLEXCAN_ESR_FLT_CONF_PASSIVE))
+		}
+	} else if (unlikely(flt == FLEXCAN_ESR_FLT_CONF_PASSIVE)) {
 		new_state = CAN_STATE_ERROR_PASSIVE;
-	else
+	} else {
 		new_state = CAN_STATE_BUS_OFF;
+		can_bus_off(dev);
+	}
 
 	/* has state changed or are there bus errros? */
 	if (likely(new_state == priv->can.state &&
@@ -449,10 +388,8 @@ static int flexcan_poll_error_state(struct net_device *dev, u32 reg_esr)
 	if (unlikely(!skb))
 		return 0;
 
-	if (new_state != priv->can.state) {
-		do_state(dev, cf, new_state);
-		priv->can.state = new_state;
-	}
+	if (new_state != priv->can.state)
+		can_change_state(dev, cf, new_state, CAN_ERR_DIR_UNKNOWN);
 
 	if (reg_esr & FLEXCAN_ESR_ERR_BUS)
 		do_bus_err(dev, cf, reg_esr);
@@ -839,6 +776,7 @@ static int flexcan_set_mode(struct net_device *dev, enum can_mode mode)
 			return err;
 
 		netif_wake_queue(dev);
+
 		break;
 
 	default:
-- 
1.7.4.1

[RFC PATCH 08/14] can: flexcan: consolidate bus-off handling

[Wolfgang Grandegger]

The netdev queue is restarted not before the CAN error state has
returned to error active.

Here is an example output of "candump -candump -td -e any,0:0,#FFFFFFFF"
for a bus-off recovery due to a short circiut (restart after 5s) for a
Flexcan controller on the i.MX28 SOC:

 (000.200433)  can0    D  [4] B5 97 EB 52
 (000.200401)  can0  2000028C  [8] 00 20 08 00 00 00 91 00   ERRORFRAME
        controller-problem{tx-error-passive}
        protocol-violation{{tx-dominant-bit-error}{}}
        bus-error
        state-change{tx-error-passive}
        error-counter-tx-rx{{145}{0}}
 (000.001000)  can0  200002C8  [8] 00 00 08 00 00 00 0E 00   ERRORFRAME
        protocol-violation{{tx-dominant-bit-error}{}}
        bus-off
        bus-error
        state-change{}
        error-counter-tx-rx{{14}{0}}
 (005.006823)  can0  20000100  [8] 00 00 00 00 00 00 00 00   ERRORFRAME
        restarted-after-bus-off
 (000.001063)  can0  20000200  [8] 00 40 00 00 00 00 00 00   ERRORFRAME
        state-change{back-to-error-active}
 (000.006218)  can0   27  [2] 72 F3

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/flexcan.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 73e0597..86b697c 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -376,6 +376,7 @@ static int flexcan_poll_error_state(struct net_device *dev, u32 reg_esr)
 		new_state = CAN_STATE_ERROR_PASSIVE;
 	} else {
 		new_state = CAN_STATE_BUS_OFF;
+		netif_stop_queue(dev);
 		can_bus_off(dev);
 	}
 
@@ -675,7 +676,9 @@ static int flexcan_chip_start(struct net_device *dev)
 	reg_mcr &= ~FLEXCAN_MCR_HALT;
 	flexcan_write(reg_mcr, &regs->mcr);
 
-	priv->can.state = CAN_STATE_ERROR_ACTIVE;
+	/* If from bus-off, wait for state change to error active */
+	if (priv->can.state != CAN_STATE_BUS_OFF)
+		priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
 	/* enable FIFO interrupts */
 	flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
@@ -767,6 +770,7 @@ static int flexcan_close(struct net_device *dev)
 
 static int flexcan_set_mode(struct net_device *dev, enum can_mode mode)
 {
+	struct flexcan_priv *priv = netdev_priv(dev);
 	int err;
 
 	switch (mode) {
@@ -775,7 +779,9 @@ static int flexcan_set_mode(struct net_device *dev, enum can_mode mode)
 		if (err)
 			return err;
 
-		netif_wake_queue(dev);
+		/* If from bus-off, wait for state change to error active */
+		if (priv->can.state != CAN_STATE_BUS_OFF)
+			netif_wake_queue(dev);
 
 		break;
 
-- 
1.7.4.1

[RFC PATCH 09/14] can: sja1000: consolidate error state handling

[Wolfgang Grandegger]

Allow state to decrease bus-off->passive->warning->active.
Use the common function can_chage_state().

Here is an example output of "candump -candump -td -e any,0:0,#FFFFFFFF"
for a recovery from error passive state due to no ack/cable (reconnect
after 5s) for a SJA1000 on an on EMS PCI card:

 (000.201913)  can0   1C  [0]
 (000.212241)  can0  20000204  [8] 00 08 00 00 00 00 60 00   ERRORFRAME
        controller-problem{tx-error-warning}
        state-change{tx-error-warning}
        error-counter-tx-rx{{96}{0}}
 (000.003544)  can0  20000204  [8] 00 20 00 00 00 00 80 00   ERRORFRAME
        controller-problem{tx-error-passive}
        state-change{tx-error-passive}
        error-counter-tx-rx{{128}{0}}
 (004.901842)  can0   1D  [7] 1D F6 33 52 31 4B DE
 (000.000116)  can0  20000200  [8] 00 08 00 00 00 00 7F 00   ERRORFRAME
        state-change{tx-error-warning}
        error-counter-tx-rx{{127}{0}}
 (000.000678)  can0   1E  [6] 42 05 14 82 23 B6
 ...
 (000.201927)  can0   49  [4] 2F 1A 97 25
 (000.000096)  can0  20000200  [8] 00 40 00 00 00 00 5F 00   ERRORFRAME
        state-change{back-to-error-active}
        error-counter-tx-rx{{95}{0}}
 (000.202184)  can0   4A  [8] 7F 87 0E FE 03 BA 78 91

This is how it should be on other CAN controllers as well.

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/sja1000/sja1000.c |   39 ++++++++++++------------------------
 1 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
index ebbcfca..f6a1c74 100644
--- a/drivers/net/can/sja1000/sja1000.c
+++ b/drivers/net/can/sja1000/sja1000.c
@@ -371,6 +371,8 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
 	enum can_state state = priv->can.state;
 	uint8_t ecc, alc;
 
+	netdev_dbg(dev, "%s: ir=0x%02x sr=0x%02x", __func__, isrc, status);
+
 	skb = alloc_can_err_skb(dev, &cf);
 	if (skb == NULL)
 		return -ENOMEM;
@@ -391,12 +393,12 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
 
 		if (status & SR_BS) {
 			state = CAN_STATE_BUS_OFF;
-			cf->can_id |= CAN_ERR_BUSOFF;
 			can_bus_off(dev);
 		} else if (status & SR_ES) {
 			state = CAN_STATE_ERROR_WARNING;
-		} else
+		} else {
 			state = CAN_STATE_ERROR_ACTIVE;
+		}
 	}
 	if (isrc & IRQ_BEI) {
 		/* bus error interrupt */
@@ -429,10 +431,14 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
 	if (isrc & IRQ_EPI) {
 		/* error passive interrupt */
 		netdev_dbg(dev, "error passive interrupt\n");
-		if (status & SR_ES)
-			state = CAN_STATE_ERROR_PASSIVE;
-		else
+		if (status & SR_ES) {
+			if (state == CAN_STATE_ERROR_WARNING)
+				state = CAN_STATE_ERROR_PASSIVE;
+			else
+				state = CAN_STATE_ERROR_WARNING;
+		} else {
 			state = CAN_STATE_ERROR_ACTIVE;
+		}
 	}
 	if (isrc & IRQ_ALI) {
 		/* arbitration lost interrupt */
@@ -444,27 +450,8 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
 		cf->data[0] = alc & 0x1f;
 	}
 
-	if (state != priv->can.state && (state == CAN_STATE_ERROR_WARNING ||
-					 state == CAN_STATE_ERROR_PASSIVE)) {
-		uint8_t rxerr = priv->read_reg(priv, REG_RXERR);
-		uint8_t txerr = priv->read_reg(priv, REG_TXERR);
-		cf->can_id |= CAN_ERR_CRTL;
-		if (state == CAN_STATE_ERROR_WARNING) {
-			priv->can.can_stats.error_warning++;
-			cf->data[1] = (txerr > rxerr) ?
-				CAN_ERR_CRTL_TX_WARNING :
-				CAN_ERR_CRTL_RX_WARNING;
-		} else {
-			priv->can.can_stats.error_passive++;
-			cf->data[1] = (txerr > rxerr) ?
-				CAN_ERR_CRTL_TX_PASSIVE :
-				CAN_ERR_CRTL_RX_PASSIVE;
-		}
-		cf->data[6] = txerr;
-		cf->data[7] = rxerr;
-	}
-
-	priv->can.state = state;
+	if (state != priv->can.state)
+		can_change_state(dev, cf, state, CAN_ERR_DIR_UNKNOWN);
 
 	netif_rx(skb);
 
-- 
1.7.4.1

[RFC PATCH 10/14] can: sja1000: consolidate bus-off handling

[Wolfgang Grandegger]

A real bus off recovery cycle is triggered by re-setting the init
bit and the netif queue is restarted not before the CAN error state
has returned to error active.

Here is an example output of "candump -candump -td -e any,0:0,#FFFFFFFF"
for a bus-off recovery due to a short circiut (restart after 5s) for a
SJA1000 on an EMS PCI card:

 (000.202225)  can0   5F  [8] 07 B4 4F 67 3A 90 D9 E4
 (000.201285)  can0  20000204  [8] 00 08 00 00 00 00 78 00   ERRORFRAME
        controller-problem{tx-error-warning}
        state-change{tx-error-warning}
        error-counter-tx-rx{{120}{0}}
 (000.003249)  can0  20000240  [8] 00 00 00 00 00 00 00 00   ERRORFRAME
        bus-off
        state-change{}
 (005.015531)  can0  20000100  [8] 00 00 00 00 00 00 7F 00   ERRORFRAME
        restarted-after-bus-off
        error-counter-tx-rx{{127}{0}}
 (000.000133)  can0  20000200  [8] 00 40 00 00 00 00 00 00   ERRORFRAME
        state-change{back-to-error-active}
 (000.031612)  can0   79  [5] 79 85 71 34 7A

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/sja1000/sja1000.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
index f6a1c74..29af651 100644
--- a/drivers/net/can/sja1000/sja1000.c
+++ b/drivers/net/can/sja1000/sja1000.c
@@ -163,6 +163,13 @@ static void sja1000_start(struct net_device *dev)
 {
 	struct sja1000_priv *priv = netdev_priv(dev);
 
+	if (priv->can.state == CAN_STATE_BUS_OFF) {
+		/* trigger bus-off recovery and restart queue */
+		priv->write_reg(priv, REG_MOD, 0x00);
+		netif_wake_queue(dev);
+		return;
+	}
+
 	/* leave reset mode */
 	if (priv->can.state != CAN_STATE_STOPPED)
 		set_reset_mode(dev);
@@ -393,6 +400,7 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
 
 		if (status & SR_BS) {
 			state = CAN_STATE_BUS_OFF;
+			netif_stop_queue(dev);
 			can_bus_off(dev);
 		} else if (status & SR_ES) {
 			state = CAN_STATE_ERROR_WARNING;
-- 
1.7.4.1

[RFC PATCH 11/14] can: mscan: consolidate error state handling

[Wolfgang Grandegger]

Allow state to decrease bus-off->passive->warning->active.
Use the common function can_chage_state().

Here is an example output of "candump -candump -td -e any,0:0,#FFFFFFFF"
for a recovery from error passive state due to no ack/cable (reconnect
after 5s) for a MSCAN on a MPC5200 SOC:

 (000.204135)  can2    6  [8] 25 84 EF 0F 3F 3D B7 3E
 (000.206547)  can2  20000204  [8] 00 08 00 00 00 00 00 00   ERRORFRAME
        controller-problem{tx-error-warning}
        state-change{tx-error-warning}
 (000.000909)  can2  20000204  [8] 00 20 00 00 00 00 00 00   ERRORFRAME
        controller-problem{tx-error-passive}
        state-change{tx-error-passive}
 (004.928785)  can2    7  [7] 4D BA 4C 90 60 BB D6
 (000.000204)  can2  20000200  [8] 00 08 00 00 00 00 00 00   ERRORFRAME
        state-change{tx-error-warning}
 (000.000000)  can2    8  [1] 6F
 (000.000239)  can2    9  [8] 24 CD C3 9A 62 16 37 30
 ...
 (000.204000)  can2   33  [8] 58 DC 54 02 4D CF 61 BF
 (000.000152)  can2  20000200  [8] 00 40 00 00 00 00 00 00   ERRORFRAME
        state-change{back-to-error-active}
 (000.203725)  can2   34  [0]
 (000.204074)  can2   35  [5] 71 E9 59 E5 2E

And for a MSCAN on a MPC5121 SOC:

 (000.203997)  can0    6  [8] 0F 21 AA BD 6C F5 2F 43
 (000.206737)  can0  20000204  [8] 00 08 00 00 00 00 60 00   ERRORFRAME
        controller-problem{tx-error-warning}
        state-change{tx-error-warning}
        error-counter-tx-rx{{96}{0}}
 (000.000962)  can0  20000204  [8] 00 20 00 00 00 00 80 00   ERRORFRAME
        controller-problem{tx-error-passive}
        state-change{tx-error-passive}
        error-counter-tx-rx{{128}{0}}
 (004.929359)  can0    7  [8] 64 7E BF 12 15 4A 22 E4
 (000.000117)  can0  20000200  [8] 00 08 00 00 00 00 7F 00   ERRORFRAME
        state-change{tx-error-warning}
        error-counter-tx-rx{{127}{0}}
 (000.000163)  can0    8  [1] 1C
 ...
 (000.204030)  can0   35  [8] 2D 8F 0E CA 26 4D 0D 37
 (000.000123)  can0  20000200  [8] 00 40 00 00 00 00 5F 00   ERRORFRAME
        state-change{back-to-error-active}
        error-counter-tx-rx{{95}{0}}
 (000.203879)  can0   36  [8] 6F C9 05 BA 73 9A A5 E2

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/mscan/mscan.c |   89 ++++++++++++++++-------------------------
 1 files changed, 34 insertions(+), 55 deletions(-)

diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
index 41a2a2d..e9e6b14 100644
--- a/drivers/net/can/mscan/mscan.c
+++ b/drivers/net/can/mscan/mscan.c
@@ -290,20 +290,6 @@ static netdev_tx_t mscan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	return NETDEV_TX_OK;
 }
 
-/* This function returns the old state to see where we came from */
-static enum can_state check_set_state(struct net_device *dev, u8 canrflg)
-{
-	struct mscan_priv *priv = netdev_priv(dev);
-	enum can_state state, old_state = priv->can.state;
-
-	if (canrflg & MSCAN_CSCIF && old_state <= CAN_STATE_BUS_OFF) {
-		state = state_map[max(MSCAN_STATE_RX(canrflg),
-				      MSCAN_STATE_TX(canrflg))];
-		priv->can.state = state;
-	}
-	return old_state;
-}
-
 static void mscan_get_rx_frame(struct net_device *dev, struct can_frame *frame)
 {
 	struct mscan_priv *priv = netdev_priv(dev);
@@ -350,60 +336,50 @@ static void mscan_get_err_frame(struct net_device *dev, struct can_frame *frame,
 	struct mscan_priv *priv = netdev_priv(dev);
 	struct mscan_regs __iomem *regs = priv->reg_base;
 	struct net_device_stats *stats = &dev->stats;
-	enum can_state old_state;
 
 	netdev_dbg(dev, "error interrupt (canrflg=%#x)\n", canrflg);
-	frame->can_id = CAN_ERR_FLAG;
 
 	if (canrflg & MSCAN_OVRIF) {
 		frame->can_id |= CAN_ERR_CRTL;
-		frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
+		frame->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
 		stats->rx_over_errors++;
 		stats->rx_errors++;
 	} else {
 		frame->data[1] = 0;
 	}
 
-	old_state = check_set_state(dev, canrflg);
-	/* State changed */
-	if (old_state != priv->can.state) {
-		switch (priv->can.state) {
-		case CAN_STATE_ERROR_WARNING:
-			frame->can_id |= CAN_ERR_CRTL;
-			priv->can.can_stats.error_warning++;
-			if ((priv->shadow_statflg & MSCAN_RSTAT_MSK) <
-			    (canrflg & MSCAN_RSTAT_MSK))
-				frame->data[1] |= CAN_ERR_CRTL_RX_WARNING;
-			if ((priv->shadow_statflg & MSCAN_TSTAT_MSK) <
+	if (canrflg & MSCAN_CSCIF) {
+		enum can_state state = state_map[max(MSCAN_STATE_RX(canrflg),
+						     MSCAN_STATE_TX(canrflg))];
+		unsigned int err_dir = CAN_ERR_DIR_UNKNOWN;
+
+		if (state != priv->can.state) {
+
+			if ((priv->shadow_statflg & MSCAN_TSTAT_MSK) !=
 			    (canrflg & MSCAN_TSTAT_MSK))
-				frame->data[1] |= CAN_ERR_CRTL_TX_WARNING;
-			break;
-		case CAN_STATE_ERROR_PASSIVE:
-			frame->can_id |= CAN_ERR_CRTL;
-			priv->can.can_stats.error_passive++;
-			frame->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
-			break;
-		case CAN_STATE_BUS_OFF:
-			frame->can_id |= CAN_ERR_BUSOFF;
-			/*
-			 * The MSCAN on the MPC5200 does recover from bus-off
-			 * automatically. To avoid that we stop the chip doing
-			 * a light-weight stop (we are in irq-context).
-			 */
-			if (priv->type != MSCAN_TYPE_MPC5121) {
-				out_8(&regs->cantier, 0);
-				out_8(&regs->canrier, 0);
-				setbits8(&regs->canctl0,
-					 MSCAN_SLPRQ | MSCAN_INITRQ);
+				err_dir |= CAN_ERR_DIR_TX;
+			if ((priv->shadow_statflg & MSCAN_RSTAT_MSK) !=
+			    (canrflg & MSCAN_RSTAT_MSK))
+				err_dir |= CAN_ERR_DIR_RX;
+			if (state == CAN_STATE_BUS_OFF) {
+				/*
+				 * The MSCAN on the MPC5200 does recover from
+				 * bus-off automatically. To avoid that we
+				 * stop the chip doing a light-weight stop
+				 * (we  are in irq-context).
+				 */
+				if (priv->type != MSCAN_TYPE_MPC5121) {
+					out_8(&regs->cantier, 0);
+					out_8(&regs->canrier, 0);
+					setbits8(&regs->canctl0,
+						 MSCAN_SLPRQ | MSCAN_INITRQ);
+				}
+				can_bus_off(dev);
 			}
-			can_bus_off(dev);
-			break;
-		default:
-			break;
+			can_change_state(dev, frame, state, err_dir);
 		}
+		priv->shadow_statflg = canrflg & MSCAN_STAT_MSK;
 	}
-	priv->shadow_statflg = canrflg & MSCAN_STAT_MSK;
-	frame->can_dlc = CAN_ERR_DLC;
 	out_8(&regs->canrflg, MSCAN_ERR_IF);
 }
 
@@ -424,7 +400,10 @@ static int mscan_rx_poll(struct napi_struct *napi, int quota)
 		if (!(canrflg & (MSCAN_RXF | MSCAN_ERR_IF)))
 			break;
 
-		skb = alloc_can_skb(dev, &frame);
+		if (canrflg & MSCAN_RXF)
+			skb = alloc_can_skb(dev, &frame);
+		else /* MSCAN_ERR_IF */
+			skb = alloc_can_err_skb(dev, &frame);
 		if (!skb) {
 			if (printk_ratelimit())
 				netdev_notice(dev, "packet dropped\n");
@@ -435,7 +414,7 @@ static int mscan_rx_poll(struct napi_struct *napi, int quota)
 
 		if (canrflg & MSCAN_RXF)
 			mscan_get_rx_frame(dev, frame);
-		else if (canrflg & MSCAN_ERR_IF)
+		else /* MSCAN_ERR_IF */
 			mscan_get_err_frame(dev, frame, canrflg);
 
 		stats->rx_packets++;
-- 
1.7.4.1

[RFC PATCH 12/14] can: mscan: consolidate bus-off handling

[Wolfgang Grandegger]

The netif queue is restarted not before the CAN error state
has returned below bus-off (passive, warning or active).

Here is an example output of "candump -candump -td -e any,0:0,#FFFFFFFF"
for a recovery from error passive state due to no ack/cable (reconnect
after 5s) for a MSCAN on a MPC5200 SOC:

 (000.203990)  can2   31  [1] 5D
 (000.204007)  can2  20000204  [8] 00 08 00 00 00 00 00 00   ERRORFRAME
        controller-problem{tx-error-warning}
        state-change{tx-error-warning}
 (000.000842)  can2  20000240  [8] 00 00 00 00 00 00 00 00   ERRORFRAME
        bus-off
        state-change{}
 (005.010926)  can2  20000100  [8] 00 00 00 00 00 00 00 00   ERRORFRAME
        restarted-after-bus-off
 (000.002880)  can2  20000200  [8] 00 40 00 00 00 00 00 00   ERRORFRAME
        state-change{back-to-error-active}
 (000.085464)  can2   4B  [8] 14 EF 5E D3 43 84 7C 87

And for a MSCAN on a MPC5121 SOC:

 (000.204131)  can0   36  [8] 3B ED 98 76 48 33 1A 13
 (000.203927)  can0  20000204  [8] 00 20 00 00 00 00 94 00   ERRORFRAME
        controller-problem{tx-error-passive}
        state-change{tx-error-passive}
        error-counter-tx-rx{{148}{0}}
 (000.000828)  can0  20000240  [8] 00 00 00 00 00 00 7F 00   ERRORFRAME
        bus-off
        state-change{}
        error-counter-tx-rx{{127}{0}}
 (005.006905)  can0  20000100  [8] 00 00 00 00 00 00 7F 00   ERRORFRAME
        restarted-after-bus-off
        error-counter-tx-rx{{127}{0}}
 (000.003043)  can0  20000200  [8] 00 40 00 00 00 00 00 00   ERRORFRAME
        state-change{back-to-error-active}
 (000.089262)  can0   50  [6] 12 3E 5B 23 15 68

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/mscan/mscan.c     |    7 +++++--
 drivers/net/can/sja1000/sja1000.c |    7 +++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
index e9e6b14..0762093 100644
--- a/drivers/net/can/mscan/mscan.c
+++ b/drivers/net/can/mscan/mscan.c
@@ -180,7 +180,6 @@ static int mscan_restart(struct net_device *dev)
 	if (priv->type == MSCAN_TYPE_MPC5121) {
 		struct mscan_regs __iomem *regs = priv->reg_base;
 
-		priv->can.state = CAN_STATE_ERROR_ACTIVE;
 		WARN(!(in_8(&regs->canmisc) & MSCAN_BOHOLD),
 		     "bus-off state expected\n");
 		out_8(&regs->canmisc, MSCAN_BOHOLD);
@@ -374,8 +373,12 @@ static void mscan_get_err_frame(struct net_device *dev, struct can_frame *frame,
 					setbits8(&regs->canctl0,
 						 MSCAN_SLPRQ | MSCAN_INITRQ);
 				}
+				netif_stop_queue(dev);
 				can_bus_off(dev);
 			}
+			/* Restart TX queue after recovered from bus-off */
+			if (priv->can.state == CAN_STATE_BUS_OFF)
+				netif_wake_queue(dev);
 			can_change_state(dev, frame, state, err_dir);
 		}
 		priv->shadow_statflg = canrflg & MSCAN_STAT_MSK;
@@ -507,7 +510,7 @@ static int mscan_do_set_mode(struct net_device *dev, enum can_mode mode)
 		ret = mscan_restart(dev);
 		if (ret)
 			break;
-		if (netif_queue_stopped(dev))
+		if (priv->can.state != CAN_STATE_BUS_OFF)
 			netif_wake_queue(dev);
 		break;
 
diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
index 29af651..e70a966 100644
--- a/drivers/net/can/sja1000/sja1000.c
+++ b/drivers/net/can/sja1000/sja1000.c
@@ -166,7 +166,6 @@ static void sja1000_start(struct net_device *dev)
 	if (priv->can.state == CAN_STATE_BUS_OFF) {
 		/* trigger bus-off recovery and restart queue */
 		priv->write_reg(priv, REG_MOD, 0x00);
-		netif_wake_queue(dev);
 		return;
 	}
 
@@ -458,8 +457,12 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
 		cf->data[0] = alc & 0x1f;
 	}
 
-	if (state != priv->can.state)
+	if (state != priv->can.state) {
+		/* Restart TX queue after recovered from bus-off */
+		if (priv->can.state == CAN_STATE_BUS_OFF)
+			netif_wake_queue(dev);
 		can_change_state(dev, cf, state, CAN_ERR_DIR_UNKNOWN);
+	}
 
 	netif_rx(skb);
 
-- 
1.7.4.1

[RFC PATCH 13/14] can: cc770: consolidate error state handling

[Wolfgang Grandegger]

Allow state to decrease bus-off->passive->warning->active.
Use the common function can_chage_state().

Here is an example output of "candump -candump -td -e any,0:0,#FFFFFFFF"
for a recovery from error passive state due to no ack/cable (reconnect
after 5s) for an Intel 82527 on a TQM855L board:

 (000.200637)  can0   58  [4] 51 C0 12 C4
 (000.202869)  can0  20000224  [8] 00 0C 00 00 00 00 00 00   ERRORFRAME
        controller-problem{rx-error-warning,tx-error-warning}
        no-acknowledgement-on-tx
        state-change{rx-error-warning,tx-error-warning}
 (004.928599)  can0   59  [8] 50 69 F9 1F 6A 88 4B 3A
 (000.000429)  can0   5A  [8] 29 1C 44 49 06 8C F8 07
 ...
 (000.200789)  can0   8A  [8] 38 6B F1 09 0B A2 6D 80
 (000.200831)  can0  20000200  [8] 00 40 00 00 00 00 00 00   ERRORFRAME
        state-change{back-to-error-active}
 (000.000021)  can0   8B  [7] 4B 9A 8A E7 5B C5 CD

The Intel 82527 does not report state changes to error passive.

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/cc770/cc770.c |   37 ++++++++++++-------------------------
 1 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/drivers/net/can/cc770/cc770.c b/drivers/net/can/cc770/cc770.c
index 7668967..905a3ca 100644
--- a/drivers/net/can/cc770/cc770.c
+++ b/drivers/net/can/cc770/cc770.c
@@ -349,8 +349,8 @@ static int cc770_set_mode(struct net_device *dev, enum can_mode mode)
 {
 	switch (mode) {
 	case CAN_MODE_START:
-		cc770_start(dev);
 		netif_wake_queue(dev);
+		cc770_start(dev);
 		break;
 
 	default:
@@ -513,6 +513,7 @@ static int cc770_err(struct net_device *dev, u8 status)
 {
 	struct cc770_priv *priv = netdev_priv(dev);
 	struct net_device_stats *stats = &dev->stats;
+	enum can_state new_state;
 	struct can_frame *cf;
 	struct sk_buff *skb;
 	u8 lec;
@@ -523,38 +524,25 @@ static int cc770_err(struct net_device *dev, u8 status)
 	if (!skb)
 		return -ENOMEM;
 
-	/* Use extended functions of the CC770 */
-	if (priv->control_normal_mode & CTRL_EAF) {
-		cf->data[6] = cc770_read_reg(priv, tx_error_counter);
-		cf->data[7] = cc770_read_reg(priv, rx_error_counter);
-	}
-
+	/* Handle state changes */
 	if (status & STAT_BOFF) {
 		/* Disable interrupts */
 		cc770_write_reg(priv, control, CTRL_INI);
-		cf->can_id |= CAN_ERR_BUSOFF;
-		priv->can.state = CAN_STATE_BUS_OFF;
+		new_state = CAN_STATE_BUS_OFF;
 		can_bus_off(dev);
 	} else if (status & STAT_WARN) {
-		cf->can_id |= CAN_ERR_CRTL;
 		/* Only the CC770 does show error passive */
-		if (cf->data[7] > 127) {
-			cf->data[1] = CAN_ERR_CRTL_RX_PASSIVE |
-				CAN_ERR_CRTL_TX_PASSIVE;
-			priv->can.state = CAN_STATE_ERROR_PASSIVE;
-			priv->can.can_stats.error_passive++;
-		} else {
-			cf->data[1] = CAN_ERR_CRTL_RX_WARNING |
-				CAN_ERR_CRTL_TX_WARNING;
-			priv->can.state = CAN_STATE_ERROR_WARNING;
-			priv->can.can_stats.error_warning++;
-		}
+		if (priv->control_normal_mode & CTRL_EAF &&
+		    cc770_read_reg(priv, rx_error_counter) > 127)
+			new_state = CAN_STATE_ERROR_PASSIVE;
+		else
+			new_state = CAN_STATE_ERROR_WARNING;
 	} else {
 		/* Back to error avtive */
-		cf->can_id |= CAN_ERR_PROT;
-		cf->data[2] = CAN_ERR_PROT_ACTIVE;
-		priv->can.state = CAN_STATE_ERROR_ACTIVE;
+		new_state = CAN_STATE_ERROR_ACTIVE;
 	}
+	if (new_state != priv->can.state)
+		can_change_state(dev, cf, new_state, CAN_ERR_DIR_UNKNOWN);
 
 	lec = status & STAT_LEC_MASK;
 	if (lec < 7 && lec > 0) {
@@ -770,7 +758,6 @@ static int cc770_open(struct net_device *dev)
 	cc770_start(dev);
 
 	netif_start_queue(dev);
-
 	return 0;
 }
 
-- 
1.7.4.1

[RFC PATCH 14/14] can: cc770: consolidate bus-off handling

[Wolfgang Grandegger]

A real bus off recovery cycle is triggered by re-setting the init
bit and the netif queue is restarted not before the CAN error
has returned below bus-off (passive, warning or active).

Here is an example output of "candump -candump -td -e any,0:0,#FFFFFFFF"
for a bus-off recovery due to a short circiut (restart after 5s) for an
Intel 82527 on a TQM855L board:

 (000.201120)  can0   88  [8] 31 33 FE 45 58 C2 08 FE
 (000.200637)  can0  2000020C  [8] 00 0C 08 00 00 00 00 00   ERRORFRAME
        controller-problem{rx-error-warning,tx-error-warning}
        protocol-violation{{tx-dominant-bit-error}{}}
        state-change{rx-error-warning,tx-error-warning}
 (000.001016)  can0  20000248  [8] 00 00 08 00 00 00 00 00   ERRORFRAME
        protocol-violation{{tx-dominant-bit-error}{}}
        bus-off
        state-change{}
 (005.007273)  can0  20000100  [8] 00 00 00 00 00 00 00 00   ERRORFRAME
        restarted-after-bus-off
 (000.003323)  can0  20000208  [8] 00 40 08 00 00 00 00 00   ERRORFRAME
        protocol-violation{{tx-dominant-bit-error}{}}
        state-change{back-to-error-active}
 (000.005255)  can0   A2  [3] 50 22 B6

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/cc770/cc770.c |   24 ++++++++++++++++++++----
 1 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/cc770/cc770.c b/drivers/net/can/cc770/cc770.c
index 905a3ca..6d77bbc 100644
--- a/drivers/net/can/cc770/cc770.c
+++ b/drivers/net/can/cc770/cc770.c
@@ -337,7 +337,16 @@ static void cc770_start(struct net_device *dev)
 {
 	struct cc770_priv *priv = netdev_priv(dev);
 
-	/* leave reset mode */
+	if (priv->can.state == CAN_STATE_BUS_OFF) {
+		/*
+		 * Start bus-off recovery cycle by clearing the init
+		 * bit and re-enabling the interrupts
+		 */
+		cc770_write_reg(priv, control, priv->control_normal_mode);
+		return;
+	}
+
+	/* enter reset mode ? */
 	if (priv->can.state != CAN_STATE_STOPPED)
 		set_reset_mode(dev);
 
@@ -347,10 +356,13 @@ static void cc770_start(struct net_device *dev)
 
 static int cc770_set_mode(struct net_device *dev, enum can_mode mode)
 {
+	struct cc770_priv *priv = netdev_priv(dev);
+
 	switch (mode) {
 	case CAN_MODE_START:
-		netif_wake_queue(dev);
 		cc770_start(dev);
+		if (priv->can.state != CAN_STATE_BUS_OFF)
+			netif_wake_queue(dev);
 		break;
 
 	default:
@@ -529,6 +541,7 @@ static int cc770_err(struct net_device *dev, u8 status)
 		/* Disable interrupts */
 		cc770_write_reg(priv, control, CTRL_INI);
 		new_state = CAN_STATE_BUS_OFF;
+		netif_stop_queue(dev);
 		can_bus_off(dev);
 	} else if (status & STAT_WARN) {
 		/* Only the CC770 does show error passive */
@@ -541,8 +554,12 @@ static int cc770_err(struct net_device *dev, u8 status)
 		/* Back to error avtive */
 		new_state = CAN_STATE_ERROR_ACTIVE;
 	}
-	if (new_state != priv->can.state)
+	if (new_state != priv->can.state) {
+		/* Have we recovered from bus-off? */
+		if (priv->can.state == CAN_STATE_BUS_OFF)
+			netif_wake_queue(dev);
 		can_change_state(dev, cf, new_state, CAN_ERR_DIR_UNKNOWN);
+	}
 
 	lec = status & STAT_LEC_MASK;
 	if (lec < 7 && lec > 0) {
@@ -757,7 +774,6 @@ static int cc770_open(struct net_device *dev)
 	/* init and start chip */
 	cc770_start(dev);
 
-	netif_start_queue(dev);
 	return 0;
 }
 
-- 
1.7.4.1

Re: [RFC PATCH 00/14] consolidate and unify state change and bus-off handling

[Wolfgang Grandegger]

On 12/07/2011 03:55 PM, Wolfgang Grandegger wrote:
> As already announced, here is my first patch series to consolidate the
> handling of CAN error state changes and bus-off.
> 
> I have also updated the Flexcan driver to behave like the other drivers,
> especially bus error reporting is now enabled via interrupt source. This
> works on my i.MX28 SOC. Marc, would be nice if you could test it on
> other SOCs as well (or somebody else). Hopefully we will not need to
> handle different hardware variants of the Flexcan controller.
> 
> I separated "consolidate error state handling" and "consolidate bus-off
> handling" to make the difference clear. The latter one is more delicate
> as it relys on the "back to error active" change to arrive to re-enable
> the TX queue.
> 
> I have also pushed these patches to my cloned Gitrorious repository
> "wg-linux-can-next". See:
> 
> https://gitorious.org/~wgrandegger/linux-can/wg-linux-can-next/commits/master

I have now recreated the repository and push the revised patches to the
"devel" branch:

https://gitorious.org/~wgrandegger/linux-can/wg-linux-can-next/commits/devel

Be aware that I will regularly rebase that branch.

Wolfgang,