[PATCH v4 1/5] can: flexcan: mark TX mailbox as TX_INACTIVE

[PATCH v4 1/5] can: flexcan: mark TX mailbox as TX_INACTIVE

[Marc Kleine-Budde]

This patch fixes the initialization of the TX mailbox. It is now correctly
initialized as TX_INACTIVE not RX_EMPTY.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Changes since v3:
- new

 drivers/net/can/flexcan.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 630c7bf..76dcbca 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -136,6 +136,17 @@
 
 /* FLEXCAN message buffers */
 #define FLEXCAN_MB_CNT_CODE(x)		(((x) & 0xf) << 24)
+#define FLEXCAN_MB_CODE_RX_INACTIVE	(0x0 << 24)
+#define FLEXCAN_MB_CODE_RX_EMPTY	(0x4 << 24)
+#define FLEXCAN_MB_CODE_RX_FULL		(0x2 << 24)
+#define FLEXCAN_MB_CODE_RX_OVERRRUN	(0x6 << 24)
+#define FLEXCAN_MB_CODE_RX_RANSWER	(0xa << 24)
+
+#define FLEXCAN_MB_CODE_TX_INACTIVE	(0x8 << 24)
+#define FLEXCAN_MB_CODE_TX_ABORT	(0x9 << 24)
+#define FLEXCAN_MB_CODE_TX_DATA		(0xc << 24)
+#define FLEXCAN_MB_CODE_TX_TANSWER	(0xe << 24)
+
 #define FLEXCAN_MB_CNT_SRR		BIT(22)
 #define FLEXCAN_MB_CNT_IDE		BIT(21)
 #define FLEXCAN_MB_CNT_RTR		BIT(20)
@@ -867,8 +878,8 @@ static int flexcan_chip_start(struct net_device *dev)
 	netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
 	flexcan_write(reg_ctrl, &regs->ctrl);
 
-	/* Abort any pending TX, mark Mailbox as INACTIVE */
-	flexcan_write(FLEXCAN_MB_CNT_CODE(0x4),
+	/* mark TX mailbox as INACTIVE */
+	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
 		      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
 
 	/* acceptance mask/acceptance code (accept everything) */
-- 
2.1.0

[PATCH v4 2/5] can: flexcan: correctly initialize mailboxes

[Marc Kleine-Budde]

From: David Jander <david@protonic.nl>

Apparently mailboxes may contain random data at startup, causing some of them
being prepared for message reception. This causes overruns being missed or even
confusing the IRQ check for trasmitted messages, increasing the transmit
counter instead of the error counter.

This patch initializes all mailboxes after the FIFO as RX_INACTIVE.

Signed-off-by: David Jander <david@protonic.nl>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---

Changes since v3:
- make use of FLEXCAN_MB_CODE_RX_INACTIVE instead of using magic numbers

 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 76dcbca..fc076952 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -812,6 +812,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	struct flexcan_regs __iomem *regs = priv->base;
 	int err;
 	u32 reg_mcr, reg_ctrl;
+	int i;
 
 	/* enable module */
 	err = flexcan_chip_enable(priv);
@@ -878,6 +879,12 @@ static int flexcan_chip_start(struct net_device *dev)
 	netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
 	flexcan_write(reg_ctrl, &regs->ctrl);
 
+	/* clear and invalidate all mailboxes first */
+	for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->cantxfg); i++) {
+		flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE,
+			      &regs->cantxfg[i].can_ctrl);
+	}
+
 	/* mark TX mailbox as INACTIVE */
 	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
 		      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
-- 
2.1.0

[PATCH v4 3/5] can: flexcan: implement workaround for errata ERR005829

[Marc Kleine-Budde]

From: David Jander <david@protonic.nl>

This patch implements the workaround mentioned in ERR005829:

    ERR005829: FlexCAN: FlexCAN does not transmit a message that is enabled to
    be transmitted in a specific moment during the arbitration process.

Workaround: The workaround consists of two extra steps after setting up a
message for transmission:

Step 8: Reserve the first valid mailbox as an inactive mailbox (CODE=0b1000).
If RX FIFO is disabled, this mailbox must be message buffer 0. Otherwise, the
first valid mailbox can be found using the "RX FIFO filters" table in the
FlexCAN chapter of the chip reference manual.

Step 9: Write twice INACTIVE code (0b1000) into the first valid mailbox.

Signed-off-by: David Jander <david@protonic.nl>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---

changes since v3:
- renamed mailbox affected by errata to FLEXCAN_TX_BUF_RESERVED
- make use of FLEXCAN_MB_CODE_TX_INACTIVE
- initialize FLEXCAN_TX_BUF_RESERVED as FLEXCAN_MB_CODE_TX_INACTIVE in
  chip_start, too

 drivers/net/can/flexcan.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index fc076952..54061c4 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -125,7 +125,9 @@
 	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
 
 /* FLEXCAN interrupt flag register (IFLAG) bits */
-#define FLEXCAN_TX_BUF_ID		8
+/* Errata ERR005829 step7: Reserve first valid MB */
+#define FLEXCAN_TX_BUF_RESERVED		8
+#define FLEXCAN_TX_BUF_ID		9
 #define FLEXCAN_IFLAG_BUF(x)		BIT(x)
 #define FLEXCAN_IFLAG_RX_FIFO_OVERFLOW	BIT(7)
 #define FLEXCAN_IFLAG_RX_FIFO_WARN	BIT(6)
@@ -439,6 +441,14 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	flexcan_write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
 	flexcan_write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
 
+	/* Errata ERR005829 step8:
+	 * Write twice INACTIVE(0x8) code to first MB.
+	 */
+	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+		      &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
+	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+		      &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
+
 	return NETDEV_TX_OK;
 }
 
@@ -885,6 +895,10 @@ static int flexcan_chip_start(struct net_device *dev)
 			      &regs->cantxfg[i].can_ctrl);
 	}
 
+	/* Errata ERR005829: mark first TX mailbox as INACTIVE */
+	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+		      &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
+
 	/* mark TX mailbox as INACTIVE */
 	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
 		      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
-- 
2.1.0

[PATCH v4 4/5] can: flexcan: put TX mailbox into TX_INACTIVE mode after tx-complete

[Marc Kleine-Budde]

After sending a RTR frame the TX mailbox becomes a RX_EMPTY mailbox. To avoid
side effects when the RX-FIFO is full, this patch puts the TX mailbox into
TX_INACTIVE mode after the transmission has been completed.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---

Changes since v3:
- new

 drivers/net/can/flexcan.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 54061c4..c17ae9e 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -765,6 +765,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 		stats->tx_bytes += can_get_echo_skb(dev, 0);
 		stats->tx_packets++;
 		can_led_event(dev, CAN_LED_EVENT_TX);
+		/* after sending a RTR frame mailbox is in RX mode */
+		flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+			      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
 		flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
 		netif_wake_queue(dev);
 	}
-- 
2.1.0

Re: [PATCH v4 4/5] can: flexcan: put TX mailbox into TX_INACTIVE mode after tx-complete

[David Jander]

On Tue, 16 Sep 2014 16:20:33 +0200
Marc Kleine-Budde <mkl@pengutronix.de> wrote:

> After sending a RTR frame the TX mailbox becomes a RX_EMPTY mailbox. To avoid
> side effects when the RX-FIFO is full, this patch puts the TX mailbox into
> TX_INACTIVE mode after the transmission has been completed.
> 
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> 
> Changes since v3:
> - new
> 
>  drivers/net/can/flexcan.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 54061c4..c17ae9e 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -765,6 +765,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  		stats->tx_bytes += can_get_echo_skb(dev, 0);
>  		stats->tx_packets++;
>  		can_led_event(dev, CAN_LED_EVENT_TX);
> +		/* after sending a RTR frame mailbox is in RX mode */
> +		flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> +			      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
>  		flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
>  		netif_wake_queue(dev);
>  	}

Good one. I hadn't notice that case yet.
Since you do this in the IRQ, I assume the expected race-condition window is
minimal, but AFAICS there is stil a (very) small chance of a race if an
overflow message arrives before this code in the IRQ handler is reached.
Both chance of occurring and impact is so small that it will not likely be a
problem ever... besides the fact that it cannot be avoided. Should this be
mentioned?

Best regards,

-- 
David Jander
Protonic Holland.

Re: [PATCH v4 4/5] can: flexcan: put TX mailbox into TX_INACTIVE mode after tx-complete

[Marc Kleine-Budde]

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

On 09/16/2014 05:18 PM, David Jander wrote:
> On Tue, 16 Sep 2014 16:20:33 +0200
> Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> 
>> After sending a RTR frame the TX mailbox becomes a RX_EMPTY mailbox. To avoid
>> side effects when the RX-FIFO is full, this patch puts the TX mailbox into
>> TX_INACTIVE mode after the transmission has been completed.
>>
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> ---
>>
>> Changes since v3:
>> - new
>>
>>  drivers/net/can/flexcan.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>> index 54061c4..c17ae9e 100644
>> --- a/drivers/net/can/flexcan.c
>> +++ b/drivers/net/can/flexcan.c
>> @@ -765,6 +765,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>>  		stats->tx_bytes += can_get_echo_skb(dev, 0);
>>  		stats->tx_packets++;
>>  		can_led_event(dev, CAN_LED_EVENT_TX);
>> +		/* after sending a RTR frame mailbox is in RX mode */
>> +		flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
>> +			      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
>>  		flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
>>  		netif_wake_queue(dev);
>>  	}
> 
> Good one. I hadn't notice that case yet.

Just stumbled over it, the reference manual is a bit vague, it this
mailbox will receive any CAN frame or just a CAN frame with the same ID.

> Since you do this in the IRQ, I assume the expected race-condition window is
> minimal, but AFAICS there is stil a (very) small chance of a race if an

Yes, there's a race window.

> overflow message arrives before this code in the IRQ handler is reached.
> Both chance of occurring and impact is so small that it will not likely be a
> problem ever... besides the fact that it cannot be avoided. Should this be
> mentioned?

Good point, I'll add a comment to the code.

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: 181 bytes --]

[PATCH v4 5/5] can: flexcan: increase FLEXCAN_MCR_MAXMB() macro to 7 bits

[Marc Kleine-Budde]

This patch increases the mask in the FLEXCAN_MCR_MAXMB() to 7 bits as in the
newer flexcan cores the MAXMB field is 7 bits wide.

Reported-by: David Jander <david@protonic.nl>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/flexcan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index c17ae9e..6586309 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -62,7 +62,7 @@
 #define FLEXCAN_MCR_BCC			BIT(16)
 #define FLEXCAN_MCR_LPRIO_EN		BIT(13)
 #define FLEXCAN_MCR_AEN			BIT(12)
-#define FLEXCAN_MCR_MAXMB(x)		((x) & 0x1f)
+#define FLEXCAN_MCR_MAXMB(x)		((x) & 0x7f)
 #define FLEXCAN_MCR_IDAM_A		(0 << 8)
 #define FLEXCAN_MCR_IDAM_B		(1 << 8)
 #define FLEXCAN_MCR_IDAM_C		(2 << 8)
-- 
2.1.0