With the help of Thomas, we found out that the chip has a time window
after receiving a CAN frame where the content of the RX FIFO STA
register (containing the RX FIFO head index) is not read correctly. In
the bad case, we observed too large head indices.
As the FIFO is implemented as a ring buffer, a too large head index
results in handling old CAN frames. To work around this issue, keep a
per FIFO timestamp [1] of the last valid received CAN frame and
compare against the timestamp of every received CAN frame. If an old
CAN frame is detected abort the iteration and mark the number of valid
CAN frames are processed in the chip by incrementing the FIFO's tail
index.
[1] As the raw timestamp overflows every 107 seconds (at the usual
clock rate of 40 MHz) convert it to nanoseconds with the
timecounter framework and use this to detect stale CAN frames.
Link: https://lore.kernel.org/all/FR0P281MB1966273C216630B120ABB6E197E89@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
Reported-by: Stefan Althöfer <Stefan.Althoefer@xxxxxxxxxxx>
Tested-by: Stefan Althöfer <Stefan.Althoefer@xxxxxxxxxxx>
Tested-by: Thomas Kopp <thomas.kopp@xxxxxxxxxxxxx>
Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
---
.../net/can/spi/mcp251xfd/mcp251xfd-ring.c | 1 +
drivers/net/can/spi/mcp251xfd/mcp251xfd-rx.c | 28 +++++++++++++++++--
drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 3 ++
3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c
index 591ca0686df4..f69d5fc8c9af 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c
@@ -196,6 +196,7 @@ mcp251xfd_ring_init_rx(struct mcp251xfd_priv *priv, u16 *base, u8 *fifo_nr)
int i, j;
mcp251xfd_for_each_rx_ring(priv, rx_ring, i) {
+ rx_ring->last_valid = timecounter_read(&priv->tc);
rx_ring->head = 0;
rx_ring->tail = 0;
rx_ring->base = *base;
diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-rx.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-rx.c
index 5c6cf2a3e2c2..9ccca8d7b58d 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-rx.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-rx.c
@@ -175,8 +175,6 @@ mcp251xfd_hw_rx_obj_to_skb(const struct mcp251xfd_priv *priv,
if (!(hw_rx_obj->flags & MCP251XFD_OBJ_FLAGS_RTR))
memcpy(cfd->data, hw_rx_obj->data, cfd->len);
-
- mcp251xfd_skb_set_timestamp_raw(priv, skb, hw_rx_obj->ts);
}
static int
@@ -187,8 +185,23 @@ mcp251xfd_handle_rxif_one(struct mcp251xfd_priv *priv,
struct net_device_stats *stats = &priv->ndev->stats;
struct sk_buff *skb;
struct canfd_frame *cfd;
+ u64 timestamp;
int err;
+ /* Due to an erratum the RX FIFO head index might too big and
+ * we're processing past the FIFO's head into old CAN frames.
+ * Compared timestamp of current handled CAN frame against
+ * last valid received one. Abort with -EBADMSG in case we
+ * encounter an old CAN frame.
+ */
+ timestamp = timecounter_cyc2time(&priv->tc, hw_rx_obj->ts);
+ if (timestamp <= ring->last_valid ) {
+ stats->rx_fifo_errors++;
+
+ return -EBADMSG;
+ }
+ ring->last_valid = timestamp;
+
if (hw_rx_obj->flags & MCP251XFD_OBJ_FLAGS_FDF)
skb = alloc_canfd_skb(priv->ndev, &cfd);
else
@@ -199,6 +212,7 @@ mcp251xfd_handle_rxif_one(struct mcp251xfd_priv *priv,
return 0;
}
+ mcp251xfd_skb_set_timestamp(skb, timestamp);
mcp251xfd_hw_rx_obj_to_skb(priv, hw_rx_obj, skb);
err = can_rx_offload_queue_timestamp(&priv->offload, skb, hw_rx_obj->ts);
if (err)
@@ -281,7 +295,15 @@ mcp251xfd_handle_rxif_ring(struct mcp251xfd_priv *priv,
err = mcp251xfd_handle_rxif_one(priv, ring,
(void *)hw_rx_obj +
i * ring->obj_size);
- if (err)
+ /* -EBADMSG means we're affected by an
+ * erratum, i.e. the timestamp in the RX
+ * object is older that the last valid
+ * received CAN frame. Don't process any
+ * further.
+ */
+ if (err == -EBADMSG)
+ return mcp251xfd_handle_rxif_ring_uinc(priv, ring, i);
+ else if (err)
return err;
}
diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
index 91cbc2451c8b..6008d38810e9 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
@@ -546,6 +546,9 @@ struct mcp251xfd_rx_ring {
unsigned int head;
unsigned int tail;
+ /* timestamp of the last valid received CAN frame */
+ u64 last_valid;
+
u16 base;
u8 nr;
u8 fifo_nr;
--
2.39.0
