[PATCH 1/5] net: can: flexcan: provide propper return code in ISR

[PATCH 1/5] net: can: flexcan: provide propper return code in ISR

[Matthias Klein]

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

flexcan_irq() always returns IRQ_HANDLED no matter if it actually did
something or not. If the CAN interface is brought up without BERR
reporting on a SoC which has the FLEXCAN_HAS_BROKEN_ERR_STATE feature
then ERR reporting is activated but not really handled. That means on an
open bus one receives a lot of STF_ERR and the only thing that happens
is that the number of interrupts is incremented.
This patch ensures that in such a case the core has a chance to detect
such (or similar) misbehavior and disable the interrupt line.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Matthias Klein <matthias.klein@optimeas.de>
---
 drivers/net/can/flexcan.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index e381142..f677b49 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -641,6 +641,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	struct flexcan_priv *priv = netdev_priv(dev);
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg_iflag1, reg_esr;
+	int worked = 0;
 
 	reg_iflag1 = flexcan_read(&regs->iflag1);
 	reg_esr = flexcan_read(&regs->esr);
@@ -667,6 +668,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 		flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
 		       &regs->ctrl);
 		napi_schedule(&priv->napi);
+		worked = 1;
 	}
 
 	/* FIFO overflow */
@@ -674,6 +676,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 		flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
 		dev->stats.rx_over_errors++;
 		dev->stats.rx_errors++;
+		worked = 1;
 	}
 
 	/* transmission complete interrupt */
@@ -683,9 +686,12 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 		can_led_event(dev, CAN_LED_EVENT_TX);
 		flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
 		netif_wake_queue(dev);
+		worked = 1;
 	}
-
-	return IRQ_HANDLED;
+	if (worked)
+		return IRQ_HANDLED;
+	else
+		return IRQ_NONE;
 }
 
 static void flexcan_set_bittiming(struct net_device *dev)
-- 
2.0.1

[PATCH 2/5] net: can: flexcan: disable error interrupts in non ERR-Active state

[Matthias Klein]

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

On imx53 I receive continuously STF_ERR error interrupts after sending a
CAN frame on an open BUS.
This patch disables error interrupts once we leave the ERR-Active state
since I doubt further error interrupts are of any interest especially if
they render the system unresponsible.
According to the manual in this case the system remains in passive state
and RX-err counter is >127 and won't increment any further and so it
won't enter BUS-Off state. Once the system receives a CAN message, the
RX-error counter should by set to 119 … 127 which brings the CAN module
back to ERR-active state which then should activate error reporting
again.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Matthias Klein <matthias.klein@optimeas.de>
---
 drivers/net/can/flexcan.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index f677b49..2b98da2 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -628,7 +628,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 		napi_complete(napi);
 		/* enable IRQs */
 		flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
-		flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
+		/*
+		 * On an open CAN-bus the iMX51 keeps reporting the STF_ERR
+		 * event after an attempt to send a CAN message. This will
+		 * disable further error reports (or that one that keeps
+		 * nagging) once we leave the ERR-Active state.
+		 */
+		if (reg_esr & FLEXCAN_ESR_FLT_CONF_MASK)
+			flexcan_write(priv->reg_ctrl_default &
+					~FLEXCAN_CTRL_ERR_MSK, &regs->ctrl);
+		else
+			flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
 	}
 
 	return work_done;
@@ -656,8 +666,8 @@ 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_ERR_ALL)) {
+
 		/*
 		 * The error bits are cleared on read,
 		 * save them for later use.
-- 
2.0.1

Re: [PATCH 2/5] net: can: flexcan: disable error interrupts in non ERR-Active state

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 3104 bytes --]

On 07/25/2014 08:16 PM, Matthias Klein wrote:
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> On imx53 I receive continuously STF_ERR error interrupts after sending a
> CAN frame on an open BUS.
> This patch disables error interrupts once we leave the ERR-Active state
> since I doubt further error interrupts are of any interest especially if
> they render the system unresponsible.

For some use cases and users this is important. However I think we all
don't want to have an unresponsive system. Have a look the "can:
berr_limit support" series [1], which introduces a configurable time-out
before the error interrupts are re-enabled.

[1] http://comments.gmane.org/gmane.linux.can/4070

The series needs some polishing and there are some open questions to
address.

Marc

> According to the manual in this case the system remains in passive state
> and RX-err counter is >127 and won't increment any further and so it
> won't enter BUS-Off state. Once the system receives a CAN message, the
> RX-error counter should by set to 119 … 127 which brings the CAN module
> back to ERR-active state which then should activate error reporting
> again.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Matthias Klein <matthias.klein@optimeas.de>
> ---
>  drivers/net/can/flexcan.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index f677b49..2b98da2 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -628,7 +628,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
>  		napi_complete(napi);
>  		/* enable IRQs */
>  		flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> -		flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
> +		/*
> +		 * On an open CAN-bus the iMX51 keeps reporting the STF_ERR
> +		 * event after an attempt to send a CAN message. This will
> +		 * disable further error reports (or that one that keeps
> +		 * nagging) once we leave the ERR-Active state.
> +		 */
> +		if (reg_esr & FLEXCAN_ESR_FLT_CONF_MASK)
> +			flexcan_write(priv->reg_ctrl_default &
> +					~FLEXCAN_CTRL_ERR_MSK, &regs->ctrl);
> +		else
> +			flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
>  	}
>  
>  	return work_done;
> @@ -656,8 +666,8 @@ 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_ERR_ALL)) {
> +
>  		/*
>  		 * The error bits are cleared on read,
>  		 * save them for later use.
> 

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

[PATCH 3/5] net: can: flexcan: handle state passive -> warning transition

[Matthias Klein]

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Once the CAN-bus is open and a packet is sent, the controller switches
into the PASSIVE state. Once the BUS is closed again it goes the back
err-warning. The TX error counter goes 0 -> 0x80 -> 0x7f.
This patch makes sure that the user learns about this state chang
(CAN_STATE_ERROR_WARNING => CAN_STATE_ERROR_PASSIVE)

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Matthias Klein <matthias.klein@optimeas.de>
---
 drivers/net/can/flexcan.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 2b98da2..f6f95ae 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -493,6 +493,13 @@ static void do_state(struct net_device *dev,
 
 	/* process state changes depending on the new state */
 	switch (new_state) {
+	case CAN_STATE_ERROR_WARNING:
+		netdev_dbg(dev, "Error Warning\n");
+		cf->can_id |= CAN_ERR_CRTL;
+		cf->data[1] = (bec.txerr > bec.rxerr) ?
+			CAN_ERR_CRTL_TX_WARNING :
+			CAN_ERR_CRTL_RX_WARNING;
+		break;
 	case CAN_STATE_ERROR_ACTIVE:
 		netdev_dbg(dev, "Error Active\n");
 		cf->can_id |= CAN_ERR_PROT;
-- 
2.0.1

Re: [PATCH 3/5] net: can: flexcan: handle state passive -> warning transition

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 877 bytes --]

On 07/25/2014 08:16 PM, Matthias Klein wrote:
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> Once the CAN-bus is open and a packet is sent, the controller switches
> into the PASSIVE state. Once the BUS is closed again it goes the back
> err-warning. The TX error counter goes 0 -> 0x80 -> 0x7f.
> This patch makes sure that the user learns about this state chang
> (CAN_STATE_ERROR_WARNING => CAN_STATE_ERROR_PASSIVE)
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Matthias Klein <matthias.klein@optimeas.de>

Looks good.

Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

[PATCH 4/5] net: can: flexcan: wait for completion when entering freeze mode

[Matthias Klein]

After requesting Freeze Mode, the user must wait for the FRZ_ACK
bit to be asserted in MCR before executing any other action,
otherwise FLEXCAN may operate in an unpredictable way.

Signed-off-by: Matthias Klein <matthias.klein@optimeas.de>
---
 drivers/net/can/flexcan.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index f6f95ae..0fbc571 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -441,6 +441,24 @@ static int flexcan_poll_bus_err(struct net_device *dev, u32 reg_esr)
 	return 1;
 }
 
+static int flexcan_wait_for_frz(struct net_device *dev,
+				struct flexcan_regs __iomem *regs, int en)
+{
+	u32 reg_mcr;
+	u32 loops = 100;
+
+	do {
+		reg_mcr = flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK;
+		if ((en && reg_mcr) || (!en && !reg_mcr))
+			return 0;
+		loops--;
+		udelay(1);
+	} while (loops);
+
+	netdev_err(dev, "Freeze Timeout\n");
+	return -ETIMEDOUT;
+}
+
 static void do_state(struct net_device *dev,
 		     struct can_frame *cf, enum can_state new_state)
 {
@@ -801,6 +819,7 @@ static int flexcan_chip_start(struct net_device *dev)
 		FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
 	netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
 	flexcan_write(reg_mcr, &regs->mcr);
+	flexcan_wait_for_frz(dev, regs, 1);
 
 	/*
 	 * CTRL
@@ -852,6 +871,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	reg_mcr = flexcan_read(&regs->mcr);
 	reg_mcr &= ~FLEXCAN_MCR_HALT;
 	flexcan_write(reg_mcr, &regs->mcr);
+	flexcan_wait_for_frz(dev, regs, 0);
 
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
@@ -885,6 +905,7 @@ static void flexcan_chip_stop(struct net_device *dev)
 	reg = flexcan_read(&regs->mcr);
 	reg |= FLEXCAN_MCR_MDIS | FLEXCAN_MCR_HALT;
 	flexcan_write(reg, &regs->mcr);
+	flexcan_wait_for_frz(dev, regs, 1);
 
 	/* Disable all interrupts */
 	flexcan_write(0, &regs->imask1);
@@ -1018,6 +1039,7 @@ static int register_flexcandev(struct net_device *dev)
 	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
 	flexcan_write(reg, &regs->mcr);
+	flexcan_wait_for_frz(dev, regs, 1);
 
 	/*
 	 * Currently we only support newer versions of this core
-- 
2.0.1

Re: [PATCH 4/5] net: can: flexcan: wait for completion when entering freeze mode

[Sebastian Andrzej Siewior]

* Matthias Klein | 2014-07-25 20:16:41 [+0200]:

>After requesting Freeze Mode, the user must wait for the FRZ_ACK
>bit to be asserted in MCR before executing any other action,
>otherwise FLEXCAN may operate in an unpredictable way.

NAK on 4 + 5. A replacement is comming.

Sebastian

[PATCH] net: can: flexcan: reset the error counter after leaving passive state

[Sebastian Andrzej Siewior]

The reasoning of why is done what is done is described in the longer
comment which I do not repeat here.
While pleaying with the HALT/Freeze mode/bit I noticed that the manual
says that one has to wait until the core completed the request or
 "otherwise FLEXCAN may operate in an unpredictable way"
therefore flexcan_wait_for_frz() is added to places where the HALT bit
is set / cleared.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/can/flexcan.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index f6f95ae..0ac99c5 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -441,6 +441,61 @@ static int flexcan_poll_bus_err(struct net_device *dev, u32 reg_esr)
 	return 1;
 }
 
+static void flexcan_wait_for_frz(struct net_device *dev, int en)
+{
+	struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg_mcr;
+	u32 loops = 25;
+
+	do {
+		reg_mcr = flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK;
+		if ((en && reg_mcr) || (!en && !reg_mcr))
+			return;
+
+		loops--;
+		udelay(1);
+	} while (loops);
+
+	netdev_err(dev, "Timeout during for the FRZ bit\n");
+}
+
+static void flexcan_reset_err_reg(struct net_device *dev)
+{
+	struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg_mcr;
+
+	/*
+	 * The reset of the error counter is ugly but I don't see any other way.
+	 * Open the CAN bus, send packet => HW goes to ERR-pasive, TX-err
+	 * counter is around 128 (maybe 129). Close the CAN-Bus, the TX-err
+	 * counter drops down to somewhere between 126 … 127, HW goes to
+	 * ERR-Warning, everything is fine.
+	 *
+	 * Now: Repeat the above procedure. What happens is that on send the
+	 * TX-err increases to around 134…136 and on connect it drops to
+	 * around 130. Occording to ESR the HW is still in passive mode _but_ it
+	 * is possible send packets - that means can send packets but has no
+	 * clue about it.
+	 * To get a consistent behavior here, the error counter are reset so we
+	 * fall back to Err-Active mode and the second "can send on open bus"
+	 * behaves just like the first one.
+	 */
+
+	reg_mcr = flexcan_read(&regs->mcr);
+	reg_mcr |= FLEXCAN_MCR_HALT;
+	flexcan_write(reg_mcr, &regs->mcr);
+
+	flexcan_wait_for_frz(dev, 1);
+
+	flexcan_write(0, &regs->ecr);
+
+	reg_mcr &= ~FLEXCAN_MCR_HALT;
+	flexcan_write(reg_mcr, &regs->mcr);
+	flexcan_wait_for_frz(dev, 0);
+}
+
 static void do_state(struct net_device *dev,
 		     struct can_frame *cf, enum can_state new_state)
 {
@@ -499,6 +554,8 @@ static void do_state(struct net_device *dev,
 		cf->data[1] = (bec.txerr > bec.rxerr) ?
 			CAN_ERR_CRTL_TX_WARNING :
 			CAN_ERR_CRTL_RX_WARNING;
+
+		flexcan_reset_err_reg(dev);
 		break;
 	case CAN_STATE_ERROR_ACTIVE:
 		netdev_dbg(dev, "Error Active\n");
@@ -801,6 +858,7 @@ static int flexcan_chip_start(struct net_device *dev)
 		FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
 	netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
 	flexcan_write(reg_mcr, &regs->mcr);
+	flexcan_wait_for_frz(dev, 1);
 
 	/*
 	 * CTRL
@@ -852,6 +910,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	reg_mcr = flexcan_read(&regs->mcr);
 	reg_mcr &= ~FLEXCAN_MCR_HALT;
 	flexcan_write(reg_mcr, &regs->mcr);
+	flexcan_wait_for_frz(dev, 0);
 
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
@@ -885,6 +944,7 @@ static void flexcan_chip_stop(struct net_device *dev)
 	reg = flexcan_read(&regs->mcr);
 	reg |= FLEXCAN_MCR_MDIS | FLEXCAN_MCR_HALT;
 	flexcan_write(reg, &regs->mcr);
+	flexcan_wait_for_frz(dev, 1);
 
 	/* Disable all interrupts */
 	flexcan_write(0, &regs->imask1);
@@ -1018,6 +1078,7 @@ static int register_flexcandev(struct net_device *dev)
 	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
 	flexcan_write(reg, &regs->mcr);
+	flexcan_wait_for_frz(dev, 1);
 
 	/*
 	 * Currently we only support newer versions of this core
-- 
2.0.1

Re: [PATCH] net: can: flexcan: reset the error counter after leaving passive state

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 5257 bytes --]

On 07/25/2014 08:51 PM, Sebastian Andrzej Siewior wrote:
> The reasoning of why is done what is done is described in the longer
> comment which I do not repeat here.
> While pleaying with the HALT/Freeze mode/bit I noticed that the manual
> says that one has to wait until the core completed the request or
>  "otherwise FLEXCAN may operate in an unpredictable way"
> therefore flexcan_wait_for_frz() is added to places where the HALT bit
> is set / cleared.

Please have a look at a recent kernel, especially patch

    b1aa1c7 can: flexcan: fix transition from and to freeze mode
                          in chip_{,un}freeze

> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  drivers/net/can/flexcan.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index f6f95ae..0ac99c5 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -441,6 +441,61 @@ static int flexcan_poll_bus_err(struct net_device *dev, u32 reg_esr)
>  	return 1;
>  }
>  
> +static void flexcan_wait_for_frz(struct net_device *dev, int en)
> +{
> +	struct flexcan_priv *priv = netdev_priv(dev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +	u32 reg_mcr;
> +	u32 loops = 25;
> +
> +	do {
> +		reg_mcr = flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK;
> +		if ((en && reg_mcr) || (!en && !reg_mcr))
> +			return;
> +
> +		loops--;
> +		udelay(1);
> +	} while (loops);
> +
> +	netdev_err(dev, "Timeout during for the FRZ bit\n");
> +}
> +
> +static void flexcan_reset_err_reg(struct net_device *dev)
> +{
> +	struct flexcan_priv *priv = netdev_priv(dev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +	u32 reg_mcr;
> +
> +	/*
> +	 * The reset of the error counter is ugly but I don't see any other way.
> +	 * Open the CAN bus, send packet => HW goes to ERR-pasive, TX-err
> +	 * counter is around 128 (maybe 129). Close the CAN-Bus, the TX-err
> +	 * counter drops down to somewhere between 126 … 127, HW goes to
> +	 * ERR-Warning, everything is fine.
> +	 *
> +	 * Now: Repeat the above procedure. What happens is that on send the
> +	 * TX-err increases to around 134…136 and on connect it drops to
> +	 * around 130. Occording to ESR the HW is still in passive mode _but_ it
> +	 * is possible send packets - that means can send packets but has no
> +	 * clue about it.

As far as I know, it's perfectly legal to send CAN frame while in "error
passive" mode. While in error passive mode you cannot send an active
error flag, just a passive one. I don't have a link to the standard at
hand, but have a look at the CAN wiki:

http://www.can-wiki.info/doku.php?id=can_faq_erors

> +	 * To get a consistent behavior here, the error counter are reset so we
> +	 * fall back to Err-Active mode and the second "can send on open bus"
> +	 * behaves just like the first one.
> +	 */
> +
> +	reg_mcr = flexcan_read(&regs->mcr);
> +	reg_mcr |= FLEXCAN_MCR_HALT;
> +	flexcan_write(reg_mcr, &regs->mcr);
> +
> +	flexcan_wait_for_frz(dev, 1);
> +
> +	flexcan_write(0, &regs->ecr);
> +
> +	reg_mcr &= ~FLEXCAN_MCR_HALT;
> +	flexcan_write(reg_mcr, &regs->mcr);
> +	flexcan_wait_for_frz(dev, 0);
> +}
> +
>  static void do_state(struct net_device *dev,
>  		     struct can_frame *cf, enum can_state new_state)
>  {
> @@ -499,6 +554,8 @@ static void do_state(struct net_device *dev,
>  		cf->data[1] = (bec.txerr > bec.rxerr) ?
>  			CAN_ERR_CRTL_TX_WARNING :
>  			CAN_ERR_CRTL_RX_WARNING;
> +
> +		flexcan_reset_err_reg(dev);
>  		break;
>  	case CAN_STATE_ERROR_ACTIVE:
>  		netdev_dbg(dev, "Error Active\n");
> @@ -801,6 +858,7 @@ static int flexcan_chip_start(struct net_device *dev)
>  		FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
>  	netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
>  	flexcan_write(reg_mcr, &regs->mcr);
> +	flexcan_wait_for_frz(dev, 1);
>  
>  	/*
>  	 * CTRL
> @@ -852,6 +910,7 @@ static int flexcan_chip_start(struct net_device *dev)
>  	reg_mcr = flexcan_read(&regs->mcr);
>  	reg_mcr &= ~FLEXCAN_MCR_HALT;
>  	flexcan_write(reg_mcr, &regs->mcr);
> +	flexcan_wait_for_frz(dev, 0);
>  
>  	priv->can.state = CAN_STATE_ERROR_ACTIVE;
>  
> @@ -885,6 +944,7 @@ static void flexcan_chip_stop(struct net_device *dev)
>  	reg = flexcan_read(&regs->mcr);
>  	reg |= FLEXCAN_MCR_MDIS | FLEXCAN_MCR_HALT;
>  	flexcan_write(reg, &regs->mcr);
> +	flexcan_wait_for_frz(dev, 1);
>  
>  	/* Disable all interrupts */
>  	flexcan_write(0, &regs->imask1);
> @@ -1018,6 +1078,7 @@ static int register_flexcandev(struct net_device *dev)
>  	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
>  		FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
>  	flexcan_write(reg, &regs->mcr);
> +	flexcan_wait_for_frz(dev, 1);
>  
>  	/*
>  	 * Currently we only support newer versions of this core
> 

Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

[PATCH 5/5] net: can: flexcan: fix for wrong TX error count behaviour on i.MX53

[Matthias Klein]

Once the CAN-bus is open and a packet is sent, the controller switches
into the PASSIVE state and the TX error count goes to 0x80. When the
bus is closed and the packet gets acknowledged the controller switches
to ERROR-WARNING state and the TX error counter is decremented to 0x7f.
Everything OK so far.

When now the bus is open again and a packet is sent, the controller
switches into PASSIVE state and sets the TX error count to 0x86.
When now the bus is closed the TX error is decremented to 0x85, but
the state does not change to ERROR-WARNING. Now with each successfully
transfered packet (in PASSIVE state!) the TX error counter is
decremented, and when the TX error counter reaches 0x7f the controller
switched back into ERROR-WARNING state.

This fix sets the TX error count back to zero when entering the
ERROR-WARNING state (after the first retransfered packet is acknowledged).

Signed-off-by: Matthias Klein <matthias.klein@optimeas.de>
---
 drivers/net/can/flexcan.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 0fbc571..a74524d 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -464,6 +464,8 @@ static void do_state(struct net_device *dev,
 {
 	struct flexcan_priv *priv = netdev_priv(dev);
 	struct can_berr_counter bec;
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg_mcr;
 
 	flexcan_get_berr_counter(dev, &bec);
 
@@ -517,6 +519,21 @@ static void do_state(struct net_device *dev,
 		cf->data[1] = (bec.txerr > bec.rxerr) ?
 			CAN_ERR_CRTL_TX_WARNING :
 			CAN_ERR_CRTL_RX_WARNING;
+
+		reg_mcr = flexcan_read(&regs->mcr);
+		reg_mcr |= FLEXCAN_MCR_HALT;
+		flexcan_write(reg_mcr, &regs->mcr);
+
+		flexcan_wait_for_frz(dev, regs, 1);
+
+		flexcan_write(0, &regs->ecr);
+
+		reg_mcr &= ~FLEXCAN_MCR_HALT;
+		flexcan_write(reg_mcr, &regs->mcr);
+		flexcan_wait_for_frz(dev, regs, 0);
+
+		netdev_dbg(dev, "txerr: %x\n",
+			flexcan_read(&regs->ecr));
 		break;
 	case CAN_STATE_ERROR_ACTIVE:
 		netdev_dbg(dev, "Error Active\n");
-- 
2.0.1

Re: [PATCH 1/5] net: can: flexcan: provide propper return code in ISR

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 2904 bytes --]

On 07/25/2014 08:16 PM, Matthias Klein wrote:
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> flexcan_irq() always returns IRQ_HANDLED no matter if it actually did
> something or not. If the CAN interface is brought up without BERR
> reporting on a SoC which has the FLEXCAN_HAS_BROKEN_ERR_STATE feature
> then ERR reporting is activated but not really handled. That means on an
> open bus one receives a lot of STF_ERR and the only thing that happens
> is that the number of interrupts is incremented.

Yes....although the interrupt is ack'ed.

> This patch ensures that in such a case the core has a chance to detect
> such (or similar) misbehavior and disable the interrupt line.

The problem is, without the enabled bus errors, you have no chance to
detect the warning interrupt, as it is not connected on some SoCs.

However, the series berr_limit addresses the problem, too.

> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Matthias Klein <matthias.klein@optimeas.de>
> ---
>  drivers/net/can/flexcan.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index e381142..f677b49 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -641,6 +641,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  	struct flexcan_priv *priv = netdev_priv(dev);
>  	struct flexcan_regs __iomem *regs = priv->base;
>  	u32 reg_iflag1, reg_esr;
> +	int worked = 0;
>  
>  	reg_iflag1 = flexcan_read(&regs->iflag1);
>  	reg_esr = flexcan_read(&regs->esr);
> @@ -667,6 +668,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  		flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
>  		       &regs->ctrl);
>  		napi_schedule(&priv->napi);
> +		worked = 1;
>  	}
>  
>  	/* FIFO overflow */
> @@ -674,6 +676,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  		flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
>  		dev->stats.rx_over_errors++;
>  		dev->stats.rx_errors++;
> +		worked = 1;
>  	}
>  
>  	/* transmission complete interrupt */
> @@ -683,9 +686,12 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  		can_led_event(dev, CAN_LED_EVENT_TX);
>  		flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
>  		netif_wake_queue(dev);
> +		worked = 1;
>  	}
> -
> -	return IRQ_HANDLED;
> +	if (worked)
> +		return IRQ_HANDLED;
> +	else
> +		return IRQ_NONE;
>  }
>  
>  static void flexcan_set_bittiming(struct net_device *dev)
> 

Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]