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

[Marc Kleine-Budde]

From: David Jander <***@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 <***@protonic.nl>
Signed-off-by: Marc Kleine-Budde <***@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);

[Marc Kleine-Budde]

From: David Jander <***@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 <***@protonic.nl>
Signed-off-by: Marc Kleine-Budde <***@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);

[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 <***@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);
}

[David Jander]

On Tue, 16 Sep 2014 16:20:33 +0200

Post by 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.
---
- 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,

[Marc Kleine-Budde]

Post by David Jander
On Tue, 16 Sep 2014 16:20:33 +0200

Post by 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.
---
- 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.

Post by David Jander
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.

Post by David Jander
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

[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 <***@protonic.nl>
Signed-off-by: Marc Kleine-Budde <***@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)