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

[PATCH V2 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 V2 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 V2 2/5] net: can: flexcan: disable error interrupts in non ERR-Active state

[Varka Bhadram]

On 07/28/2014 12:04 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.
> 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.
> +		 */

networking comment style...?


-- 
Regards,
Varka Bhadram.

[PATCH V2 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

[PATCH V2 4/5] can: flexcan: fix transition from and to freeze mode in chip_{,un}freeze

[Matthias Klein]

From: Marc Kleine-Budde <mkl@pengutronix.de>

This patch factors out freeze and unfreeze of the CAN core into seperate
functions. Experiments have shown that the transition from and to freeze mode
may take several microseconds, especially the time entering the freeze mode
depends on the current bitrate.

This patch adds a while loop which polls the Freeze Mode ACK bit (FRZ_ACK) that
indicates a successfull mode change. If the function runs into a timeout a
error value is returned.

Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Matthias Klein <matthias.klein@optimeas.de>
---
 drivers/net/can/flexcan.c | 60 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 11 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index f6f95ae..b77f1da 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -322,6 +322,44 @@ static int flexcan_chip_disable(struct flexcan_priv *priv)
 	return 0;
 }
 
+static int flexcan_chip_freeze(struct flexcan_priv *priv)
+{
+	struct flexcan_regs __iomem *regs = priv->base;
+	unsigned int timeout = 1000 * 1000 * 10 / priv->can.bittiming.bitrate;
+	u32 reg;
+
+	reg = flexcan_read(&regs->mcr);
+	reg |= FLEXCAN_MCR_HALT;
+	flexcan_write(reg, &regs->mcr);
+
+	while (timeout-- && !(flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
+		usleep_range(100, 200);
+
+	if (!(flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int flexcan_chip_unfreeze(struct flexcan_priv *priv)
+{
+	struct flexcan_regs __iomem *regs = priv->base;
+	unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
+	u32 reg;
+
+	reg = flexcan_read(&regs->mcr);
+	reg &= ~FLEXCAN_MCR_HALT;
+	flexcan_write(reg, &regs->mcr);
+
+	while (timeout-- && (flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
+		usleep_range(10, 20);
+
+	if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
 static int flexcan_get_berr_counter(const struct net_device *dev,
 				    struct can_berr_counter *bec)
 {
@@ -776,7 +814,7 @@ static int flexcan_chip_start(struct net_device *dev)
 		netdev_err(dev, "Failed to softreset can module (mcr=0x%08x)\n",
 			   reg_mcr);
 		err = -ENODEV;
-		goto out;
+		goto out_chip_disable;
 	}
 
 	flexcan_set_bittiming(dev);
@@ -846,12 +884,12 @@ static int flexcan_chip_start(struct net_device *dev)
 
 	err = flexcan_transceiver_enable(priv);
 	if (err)
-		goto out;
+		goto out_chip_disable;
 
 	/* synchronize with the can bus */
-	reg_mcr = flexcan_read(&regs->mcr);
-	reg_mcr &= ~FLEXCAN_MCR_HALT;
-	flexcan_write(reg_mcr, &regs->mcr);
+	err = flexcan_chip_unfreeze(priv);
+	if (err)
+		goto out_transceiver_disable;
 
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
@@ -864,7 +902,9 @@ static int flexcan_chip_start(struct net_device *dev)
 
 	return 0;
 
- out:
+ out_transceiver_disable:
+	flexcan_transceiver_disable(priv);
+ out_chip_disable:
 	flexcan_chip_disable(priv);
 	return err;
 }
@@ -879,12 +919,10 @@ static void flexcan_chip_stop(struct net_device *dev)
 {
 	struct flexcan_priv *priv = netdev_priv(dev);
 	struct flexcan_regs __iomem *regs = priv->base;
-	u32 reg;
 
-	/* Disable + halt module */
-	reg = flexcan_read(&regs->mcr);
-	reg |= FLEXCAN_MCR_MDIS | FLEXCAN_MCR_HALT;
-	flexcan_write(reg, &regs->mcr);
+	/* freeze + disable module */
+	flexcan_chip_freeze(priv);
+	flexcan_chip_disable(priv);
 
 	/* Disable all interrupts */
 	flexcan_write(0, &regs->imask1);
-- 
2.0.1

Re: [PATCH V2 4/5] can: flexcan: fix transition from and to freeze mode in chip_{,un}freeze

[Marc Kleine-Budde]

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

On 07/28/2014 08:34 AM, Matthias Klein wrote:
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> 
> This patch factors out freeze and unfreeze of the CAN core into seperate
> functions. Experiments have shown that the transition from and to freeze mode
> may take several microseconds, especially the time entering the freeze mode
> depends on the current bitrate.
> 
> This patch adds a while loop which polls the Freeze Mode ACK bit (FRZ_ACK) that
> indicates a successfull mode change. If the function runs into a timeout a
> error value is returned.
> 
> Cc: linux-stable <stable@vger.kernel.org>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Matthias Klein <matthias.klein@optimeas.de>

This patch is already mainline, please make your series based on the
latest net-next/master tree.

Marc

> ---
>  drivers/net/can/flexcan.c | 60 ++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 49 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index f6f95ae..b77f1da 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -322,6 +322,44 @@ static int flexcan_chip_disable(struct flexcan_priv *priv)
>  	return 0;
>  }
>  
> +static int flexcan_chip_freeze(struct flexcan_priv *priv)
> +{
> +	struct flexcan_regs __iomem *regs = priv->base;
> +	unsigned int timeout = 1000 * 1000 * 10 / priv->can.bittiming.bitrate;
> +	u32 reg;
> +
> +	reg = flexcan_read(&regs->mcr);
> +	reg |= FLEXCAN_MCR_HALT;
> +	flexcan_write(reg, &regs->mcr);
> +
> +	while (timeout-- && !(flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
> +		usleep_range(100, 200);
> +
> +	if (!(flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
> +static int flexcan_chip_unfreeze(struct flexcan_priv *priv)
> +{
> +	struct flexcan_regs __iomem *regs = priv->base;
> +	unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
> +	u32 reg;
> +
> +	reg = flexcan_read(&regs->mcr);
> +	reg &= ~FLEXCAN_MCR_HALT;
> +	flexcan_write(reg, &regs->mcr);
> +
> +	while (timeout-- && (flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
> +		usleep_range(10, 20);
> +
> +	if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
>  static int flexcan_get_berr_counter(const struct net_device *dev,
>  				    struct can_berr_counter *bec)
>  {
> @@ -776,7 +814,7 @@ static int flexcan_chip_start(struct net_device *dev)
>  		netdev_err(dev, "Failed to softreset can module (mcr=0x%08x)\n",
>  			   reg_mcr);
>  		err = -ENODEV;
> -		goto out;
> +		goto out_chip_disable;
>  	}
>  
>  	flexcan_set_bittiming(dev);
> @@ -846,12 +884,12 @@ static int flexcan_chip_start(struct net_device *dev)
>  
>  	err = flexcan_transceiver_enable(priv);
>  	if (err)
> -		goto out;
> +		goto out_chip_disable;
>  
>  	/* synchronize with the can bus */
> -	reg_mcr = flexcan_read(&regs->mcr);
> -	reg_mcr &= ~FLEXCAN_MCR_HALT;
> -	flexcan_write(reg_mcr, &regs->mcr);
> +	err = flexcan_chip_unfreeze(priv);
> +	if (err)
> +		goto out_transceiver_disable;
>  
>  	priv->can.state = CAN_STATE_ERROR_ACTIVE;
>  
> @@ -864,7 +902,9 @@ static int flexcan_chip_start(struct net_device *dev)
>  
>  	return 0;
>  
> - out:
> + out_transceiver_disable:
> +	flexcan_transceiver_disable(priv);
> + out_chip_disable:
>  	flexcan_chip_disable(priv);
>  	return err;
>  }
> @@ -879,12 +919,10 @@ static void flexcan_chip_stop(struct net_device *dev)
>  {
>  	struct flexcan_priv *priv = netdev_priv(dev);
>  	struct flexcan_regs __iomem *regs = priv->base;
> -	u32 reg;
>  
> -	/* Disable + halt module */
> -	reg = flexcan_read(&regs->mcr);
> -	reg |= FLEXCAN_MCR_MDIS | FLEXCAN_MCR_HALT;
> -	flexcan_write(reg, &regs->mcr);
> +	/* freeze + disable module */
> +	flexcan_chip_freeze(priv);
> +	flexcan_chip_disable(priv);
>  
>  	/* Disable all interrupts */
>  	flexcan_write(0, &regs->imask1);
> 


-- 
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 V2 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 | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index b77f1da..f9fefcb 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -479,6 +479,34 @@ static int flexcan_poll_bus_err(struct net_device *dev, u32 reg_esr)
 	return 1;
 }
 
+static void flexcan_reset_err_reg(struct flexcan_priv *priv)
+{
+	struct flexcan_regs __iomem *regs = priv->base;
+
+	/*
+	 * 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.
+	 */
+
+	flexcan_chip_freeze(priv);
+
+	flexcan_write(0, &regs->ecr);
+
+	flexcan_chip_unfreeze(priv);
+}
+
 static void do_state(struct net_device *dev,
 		     struct can_frame *cf, enum can_state new_state)
 {
@@ -537,6 +565,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(priv);
 		break;
 	case CAN_STATE_ERROR_ACTIVE:
 		netdev_dbg(dev, "Error Active\n");
-- 
2.0.1

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

[Varka Bhadram]

On 07/28/2014 12:04 PM, Matthias Klein wrote:
> 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 | 30 ++++++++++++++++++++++++++++++
>   1 file changed, 30 insertions(+)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index b77f1da..f9fefcb 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -479,6 +479,34 @@ static int flexcan_poll_bus_err(struct net_device *dev, u32 reg_esr)
>   	return 1;
>   }
>   
> +static void flexcan_reset_err_reg(struct flexcan_priv *priv)
> +{
> +	struct flexcan_regs __iomem *regs = priv->base;
> +
> +	/*
> +	 * 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.
> +	 */
> +

networking comment style...?

/* bla bla
  * bla
  */


-- 
Regards,
Varka Bhadram.

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

[Marc Kleine-Budde]

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

On 07/28/2014 08:34 AM, Matthias Klein wrote:
> 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.

My original comment still applies, I don't see a problem with sending
CAN frames in error passive mode.

The only thing that violates the specs is IMHO, that the TX error
counter is incremented > 127 when the bus is open, although is does not
reach 255.

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

Why do you this? The expected behaviour is that each successfully
transmitted CAN frame decreases the TX error counter. So it will take a
few frames until you reach a TX error counter of 0 when coming from
error passive.

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 --]

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

[Sebastian Andrzej Siewior]

On 07/28/2014 09:05 AM, Marc Kleine-Budde wrote:
> On 07/28/2014 08:34 AM, Matthias Klein wrote:
>> 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.
> 
> My original comment still applies, I don't see a problem with sending
> CAN frames in error passive mode.
> 
> The only thing that violates the specs is IMHO, that the TX error
> counter is incremented > 127 when the bus is open, although is does not
> reach 255.

According to the spec it is frozen and never will be incremented.

>> This fix sets the TX error count back to zero when entering the
>> ERROR-WARNING state (after the first retransfered packet is acknowledged).
> 
> Why do you this? The expected behaviour is that each successfully
> transmitted CAN frame decreases the TX error counter. So it will take a
> few frames until you reach a TX error counter of 0 when coming from
> error passive.

How does the user learn that he is able to send a can frame?

> 
> Marc

Sebastian

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

[Varka Bhadram]

On 07/28/2014 12:04 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.
> 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;

bool instead if int ...?

>   
>   	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)


-- 
Regards,
Varka Bhadram.