[PATCH 02/15] can: flexcan: rename crl2 -> ctrl2

[David Jander]

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

Signed-off-by: Marc Kleine-Budde <***@pengutronix.de>
---
drivers/net/can/flexcan.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 2e70a27..b287367 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -93,13 +93,13 @@
(FLEXCAN_CTRL_ERR_BUS | FLEXCAN_CTRL_ERR_STATE)

/* FLEXCAN control register 2 (CTRL2) bits */
-#define FLEXCAN_CRL2_ECRWRE BIT(29)
-#define FLEXCAN_CRL2_WRMFRZ BIT(28)
-#define FLEXCAN_CRL2_RFFN(x) (((x) & 0x0f) << 24)
-#define FLEXCAN_CRL2_TASD(x) (((x) & 0x1f) << 19)
-#define FLEXCAN_CRL2_MRP BIT(18)
-#define FLEXCAN_CRL2_RRS BIT(17)
-#define FLEXCAN_CRL2_EACEN BIT(16)
+#define FLEXCAN_CTRL2_ECRWRE BIT(29)
+#define FLEXCAN_CTRL2_WRMFRZ BIT(28)
+#define FLEXCAN_CTRL2_RFFN(x) (((x) & 0x0f) << 24)
+#define FLEXCAN_CTRL2_TASD(x) (((x) & 0x1f) << 19)
+#define FLEXCAN_CTRL2_MRP BIT(18)
+#define FLEXCAN_CTRL2_RRS BIT(17)
+#define FLEXCAN_CTRL2_EACEN BIT(16)

/* FLEXCAN memory error control register (MECR) bits */
#define FLEXCAN_MECR_ECRWRDIS BIT(31)
@@ -221,7 +221,7 @@ struct flexcan_regs {
u32 imask1; /* 0x28 */
u32 iflag2; /* 0x2c */
u32 iflag1; /* 0x30 */
- u32 crl2; /* 0x34 */
+ u32 ctrl2; /* 0x34 */
u32 esr2; /* 0x38 */
u32 imeur; /* 0x3c */
u32 lrfr; /* 0x40 */
@@ -893,7 +893,7 @@ static int flexcan_chip_start(struct net_device *dev)
struct flexcan_priv *priv = netdev_priv(dev);
struct flexcan_regs __iomem *regs = priv->base;
int err;
- u32 reg_mcr, reg_ctrl, reg_crl2, reg_mecr;
+ u32 reg_mcr, reg_ctrl, reg_ctrl2, reg_mecr;
int i;

/* enable module */
@@ -996,9 +996,9 @@ static int flexcan_chip_start(struct net_device *dev)
* and Correction of Memory Errors" to write to
* MECR register
*/
- reg_crl2 = flexcan_read(&regs->crl2);
- reg_crl2 |= FLEXCAN_CRL2_ECRWRE;
- flexcan_write(reg_crl2, &regs->crl2);
+ reg_ctrl2 = flexcan_read(&regs->ctrl2);
+ reg_ctrl2 |= FLEXCAN_CTRL2_ECRWRE;
+ flexcan_write(reg_ctrl2, &regs->ctrl2);

reg_mecr = flexcan_read(&regs->mecr);
reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS;

[David Jander]

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

Signed-off-by: Marc Kleine-Budde <***@pengutronix.de>
---
drivers/net/can/dev.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/can/dev.h | 25 ++++++++++
2 files changed, 145 insertions(+)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 02492d2..c1e53e9 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -273,6 +273,126 @@ static int can_get_bittiming(struct net_device *dev, struct can_bittiming *bt,
return err;
}

+static bool can_rx_fifo_ge(struct can_rx_fifo *fifo, unsigned int a, unsigned int b)
+{
+ if (fifo->inc)
+ return a >= b;
+ else
+ return a <= b;
+}
+
+static unsigned int can_rx_fifo_inc(struct can_rx_fifo *fifo, unsigned int *val)
+{
+ if (fifo->inc)
+ return (*val)++;
+ else
+ return (*val)--;
+}
+
+static u32 can_rx_fifo_mask_low(struct can_rx_fifo *fifo)
+{
+ if (fifo->inc)
+ return ~0U >> (32 + fifo->low_first - fifo->high_first) << fifo->low_first;
+ else
+ return ~0U >> (32 - fifo->low_first + fifo->high_first) << (fifo->high_first + 1);
+}
+
+static u32 can_rx_fifo_mask_high(struct can_rx_fifo *fifo)
+{
+ if (fifo->inc)
+ return ~0U >> (32 + fifo->high_first - fifo->high_last - 1) << fifo->high_first;
+ else
+ return ~0U >> (32 - fifo->high_first + fifo->high_last - 1) << fifo->high_last;
+}
+
+int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)
+{
+ fifo->dev = dev;
+
+ if ((fifo->low_first < fifo->high_first) &&
+ (fifo->high_first < fifo->high_last))
+ fifo->inc = true;
+ else if ((fifo->low_first > fifo->high_first) &&
+ (fifo->high_first > fifo->high_last))
+ fifo->inc = false;
+ else
+ return -EINVAL;
+
+ if (!fifo->read_pending || !fifo->mailbox_enable_mask ||
+ !fifo->mailbox_disable || !fifo->mailbox_receive)
+ return -EINVAL;
+
+ /* init variables */
+ fifo->mask_low = can_rx_fifo_mask_low(fifo);
+ fifo->mask_high = can_rx_fifo_mask_high(fifo);
+ fifo->next = fifo->low_first;
+ fifo->active = fifo->mask_low | fifo->mask_high;
+ fifo->mailbox_enable_mask(fifo, fifo->active);
+
+ netdev_dbg(dev, "%s: low_first=%d, high_first=%d, high_last=%d\n", __func__,
+ fifo->low_first, fifo->high_first, fifo->high_last);
+ netdev_dbg(dev, "%s: mask_low=0x%08x mask_high=0x%08x\n", __func__,
+ fifo->mask_low, fifo->mask_high);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(can_rx_fifo_add);
+
+int can_rx_fifo_poll(struct can_rx_fifo *fifo, int quota)
+{
+ int received = 0;
+ u32 pending;
+ unsigned int mb;
+
+ do {
+ pending = fifo->read_pending(fifo);
+ pending &= fifo->active;
+
+ if (!(pending & BIT(fifo->next))) {
+ /*
+ * Wrap around only if:
+ * - we are in the upper group and
+ * - there is a CAN frame in the first mailbox
+ * of the lower group.
+ */
+ if (can_rx_fifo_ge(fifo, fifo->next, fifo->high_first) &&
+ (pending & BIT(fifo->low_first))) {
+ fifo->next = fifo->low_first;
+
+ fifo->active |= fifo->mask_high;
+ fifo->mailbox_enable_mask(fifo, fifo->mask_high);
+ } else {
+ break;
+ }
+ }
+
+ mb = can_rx_fifo_inc(fifo, &fifo->next);
+
+ /* disable mailbox */
+ fifo->active &= ~BIT(mb);
+ fifo->mailbox_disable(fifo, mb);
+
+ fifo->mailbox_receive(fifo, mb);
+
+ if (fifo->next == fifo->high_first) {
+ fifo->active |= fifo->mask_low;
+ fifo->mailbox_enable_mask(fifo, fifo->mask_low);
+ }
+
+ received++;
+ quota--;
+ } while (quota);
+
+ return received;
+}
+EXPORT_SYMBOL_GPL(can_rx_fifo_poll);
+
+u32 can_rx_fifo_get_active_mb_mask(const struct can_rx_fifo *fifo)
+{
+ return fifo->active;
+}
+EXPORT_SYMBOL_GPL(can_rx_fifo_get_active_mb_mask);
+
/*
* Local echo of CAN messages
*
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 6992afc..e9468d0 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -64,6 +64,27 @@ struct can_priv {
#endif
};

+struct can_rx_fifo {
+ struct net_device *dev;
+
+ unsigned int low_first;
+ unsigned int high_first;
+ unsigned int high_last; /* not needed during runtime */
+
+ u32 (*read_pending)(struct can_rx_fifo *rx_fifo);
+ void (*mailbox_enable_mask)(struct can_rx_fifo *rx_fifo, u32 mask);
+ void (*mailbox_disable)(struct can_rx_fifo *rx_fifo, unsigned int mb);
+ void (*mailbox_receive)(struct can_rx_fifo *rx_fifo, unsigned int mb);
+
+ u32 mask_low;
+ u32 mask_high;
+ u32 active;
+
+ unsigned int next;
+
+ bool inc;
+};
+
/*
* get_can_dlc(value) - helper macro to cast a given data length code (dlc)
* to __u8 and ensure the dlc value to be max. 8 bytes.
@@ -105,6 +126,10 @@ u8 can_dlc2len(u8 can_dlc);
/* map the sanitized data length to an appropriate data length code */
u8 can_len2dlc(u8 len);

+int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo);
+int can_rx_fifo_poll(struct can_rx_fifo *fifo, int quota);
+u32 can_rx_fifo_get_active_mb_mask(const struct can_rx_fifo *fifo);
+
struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max);
void free_candev(struct net_device *dev);

[David Jander]

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

This patch adds a short documentation snippet about the mailbox organization as
it's regularly not correct in freescale's datasheets.

Signed-off-by: Marc Kleine-Budde <***@pengutronix.de>
---
drivers/net/can/flexcan.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 172065c..2e70a27 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -230,6 +230,16 @@ struct flexcan_regs {
u32 rxfir; /* 0x4c */
u32 _reserved3[12]; /* 0x50 */
struct flexcan_mb cantxfg[64]; /* 0x80 */
+ /* FIFO-mode:
+ * MB
+ * 0x080...0x08f 0 RX message buffer
+ * 0x090...0x0df 1-5 reserverd
+ * 0x0e0...0x0ff 6-7 8 entry ID table
+ * (mx25, mx28, mx35, mx53)
+ * 0x0e0...0x2df 6-7..37 8..128 entry ID table
+ * size conf'ed via ctrl2::RFFN
+ * (mx6, vf610)
+ */
u32 _reserved4[408];
u32 mecr; /* 0xae0 */
u32 erriar; /* 0xae4 */

[David Jander]

Signed-off-by: David Jander <***@protonic.nl>
---
drivers/net/can/dev.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 22a3955..3e1160a 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -301,17 +301,21 @@ static unsigned int can_rx_fifo_inc(struct can_rx_fifo *fifo, unsigned int *val)
static u64 can_rx_fifo_mask_low(struct can_rx_fifo *fifo)
{
if (fifo->inc)
- return ~0LLU >> (64 + fifo->low_first - fifo->high_first) << fifo->low_first;
+ return ~0LLU >> (64 + fifo->low_first - fifo->high_first)
+ << fifo->low_first;
else
- return ~0LLU >> (64 - fifo->low_first + fifo->high_first) << (fifo->high_first + 1);
+ return ~0LLU >> (64 - fifo->low_first + fifo->high_first)
+ << (fifo->high_first + 1);
}

static u64 can_rx_fifo_mask_high(struct can_rx_fifo *fifo)
{
if (fifo->inc)
- return ~0LLU >> (64 + fifo->high_first - fifo->high_last - 1) << fifo->high_first;
+ return ~0LLU >> (64 + fifo->high_first - fifo->high_last - 1)
+ << fifo->high_first;
else
- return ~0LLU >> (64 - fifo->high_first + fifo->high_last - 1) << fifo->high_last;
+ return ~0LLU >> (64 - fifo->high_first + fifo->high_last - 1)
+ << fifo->high_last;
}

static int can_rx_fifo_read_napi_frame(struct can_rx_fifo *fifo, int index)

[Marc Kleine-Budde]

Post by David Jander
---
drivers/net/can/dev.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 22a3955..3e1160a 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -301,17 +301,21 @@ static unsigned int can_rx_fifo_inc(struct can_rx_fifo *fifo, unsigned int *val)
static u64 can_rx_fifo_mask_low(struct can_rx_fifo *fifo)
{
if (fifo->inc)
- return ~0LLU >> (64 + fifo->low_first - fifo->high_first) << fifo->low_first;
+ return ~0LLU >> (64 + fifo->low_first - fifo->high_first)
+ << fifo->low_first;

In the kernel, a line usually doesn't start with an operator. Please keep it in
the original line.

Post by David Jander
else
- return ~0LLU >> (64 - fifo->low_first + fifo->high_first) << (fifo->high_first + 1);
+ return ~0LLU >> (64 - fifo->low_first + fifo->high_first)
+ << (fifo->high_first + 1);
}
static u64 can_rx_fifo_mask_high(struct can_rx_fifo *fifo)
{
if (fifo->inc)
- return ~0LLU >> (64 + fifo->high_first - fifo->high_last - 1) << fifo->high_first;
+ return ~0LLU >> (64 + fifo->high_first - fifo->high_last - 1)
+ << fifo->high_first;
else
- return ~0LLU >> (64 - fifo->high_first + fifo->high_last - 1) << fifo->high_last;
+ return ~0LLU >> (64 - fifo->high_first + fifo->high_last - 1)
+ << fifo->high_last;
}
static int can_rx_fifo_read_napi_frame(struct can_rx_fifo *fifo, int index)

Marc

[David Jander]

On Sun, 19 Oct 2014 23:18:03 +0200

Post by Marc Kleine-Budde

Post by David Jander
---
drivers/net/can/dev.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 22a3955..3e1160a 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -301,17 +301,21 @@ static unsigned int can_rx_fifo_inc(struct
can_rx_fifo *fifo, unsigned int *val) static u64
can_rx_fifo_mask_low(struct can_rx_fifo *fifo) {
if (fifo->inc)
- return ~0LLU >> (64 + fifo->low_first - fifo->high_first)
<< fifo->low_first;
+ return ~0LLU >> (64 + fifo->low_first - fifo->high_first)
+ << fifo->low_first;

In the kernel, a line usually doesn't start with an operator. Please keep it
in the original line.

Right. I did this on purpose because I thought it was a lot more readable
like that for this case, but if you prefer coding-style, I'll fix it.

Post by Marc Kleine-Budde

Post by David Jander
else
- return ~0LLU >> (64 - fifo->low_first + fifo->high_first)
<< (fifo->high_first + 1);
+ return ~0LLU >> (64 - fifo->low_first + fifo->high_first)
+ << (fifo->high_first + 1);
}
static u64 can_rx_fifo_mask_high(struct can_rx_fifo *fifo)
{
if (fifo->inc)
- return ~0LLU >> (64 + fifo->high_first - fifo->high_last
- 1) << fifo->high_first;
+ return ~0LLU >> (64 + fifo->high_first - fifo->high_last
- 1)
+ << fifo->high_first;
else
- return ~0LLU >> (64 - fifo->high_first + fifo->high_last
- 1) << fifo->high_last;
+ return ~0LLU >> (64 - fifo->high_first + fifo->high_last
- 1)
+ << fifo->high_last;
}
static int can_rx_fifo_read_napi_frame(struct can_rx_fifo *fifo, int index)

Marc

Best regards,

[David Jander]

This function needs to be called every time the CAN controller is reset
and before interrupts are enabled. Otherwise the irq-offload loop gets
out of sync. Detect this and BUG() to the user if he forgot.

Signed-off-by: David Jander <***@protonic.nl>
---
drivers/net/can/dev.c | 20 ++++++++++++++++++--
include/linux/can/dev.h | 1 +
2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 3e1160a..fc35d28 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -487,8 +487,17 @@ int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo)
fifo->second_first = true;
}

- if (received)
- napi_schedule(&fifo->napi);
+ if (received) {
+ can_rx_fifo_napi_schedule(fifo);
+ } else {
+ /*
+ * This should only happen if the CAN conroller was reset, but
+ * can_rx_fifo_reset() was not called. BUG();
+ */
+ netdev_warn(fifo->dev, "%s: No messages found,"
+ " RX-FIFO out of sync?\n", __func__);
+ BUG();
+ }

return received;
}
@@ -506,6 +515,13 @@ void can_rx_fifo_napi_disable(struct can_rx_fifo *fifo)
}
EXPORT_SYMBOL_GPL(can_rx_fifo_napi_disable);

+void can_rx_fifo_reset(struct can_rx_fifo *fifo)
+{
+ fifo->second_first = false;
+ fifo->active = fifo->mask_low | fifo->mask_high;
+}
+EXPORT_SYMBOL_GPL(can_rx_fifo_reset);
+
void can_rx_fifo_del(struct can_rx_fifo *fifo)
{
if (fifo->ring)
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 64a8de3..ada324c 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -137,6 +137,7 @@ int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo);
int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo);
void can_rx_fifo_napi_enable(struct can_rx_fifo *fifo);
void can_rx_fifo_napi_disable(struct can_rx_fifo *fifo);
+void can_rx_fifo_reset(struct can_rx_fifo *fifo);
void can_rx_fifo_del(struct can_rx_fifo *fifo);

struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max);

[David Jander]

The FlexCAN controller has a RX FIFO that is only 6 messages deep, and a
mailbox space capable of holding up to 63 messages.

This space was largely unused, limiting the permissible latency from interrupt
to NAPI to only 6 messages. This patch uses all available MBs for message
reception and frees the MBs in the IRQ handler to greatly decrease the
likelihood of receive overruns.

Signed-off-by: David Jander <***@protonic.nl>
---
drivers/net/can/flexcan.c | 322 ++++++++++++++++++++++++++++++++++------------
1 file changed, 241 insertions(+), 81 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 20f5c0e..8118f35 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -4,6 +4,7 @@
* Copyright (c) 2005-2006 Varma Electronics Oy
* Copyright (c) 2009 Sascha Hauer, Pengutronix
* Copyright (c) 2010 Marc Kleine-Budde, Pengutronix
+ * Copyright (c) 2014 David Jander, Protonic Holland
*
* Based on code originally by Andrey Volkov <***@varma-el.com>
*
@@ -24,6 +25,7 @@
#include <linux/can/dev.h>
#include <linux/can/error.h>
#include <linux/can/led.h>
+#include <linux/circ_buf.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/if_arp.h>
@@ -40,8 +42,8 @@

#define DRV_NAME "flexcan"

-/* 8 for RX fifo and 2 error handling */
-#define FLEXCAN_NAPI_WEIGHT (8 + 2)
+/* 64 MB's */
+#define FLEXCAN_NAPI_WEIGHT (64)

/* FLEXCAN module configuration register (CANMCR) bits */
#define FLEXCAN_MCR_MDIS BIT(31)
@@ -63,6 +65,18 @@
#define FLEXCAN_MCR_LPRIO_EN BIT(13)
#define FLEXCAN_MCR_AEN BIT(12)
#define FLEXCAN_MCR_MAXMB(x) ((x) & 0x7f)
+/*
+ * MCR_MAXMB:
+ * This field is 7 bits wide on V10 and newer flexcan cores, but the
+ * documentation of older versions states this field as being only 6 bits wide.
+ * This contradicts the fact that those cores still have 64 MB's, because with
+ * a 6-bit field the MAXMB value can never be higher than 63 (0x3f) which would
+ * implicate that the last MB could never be used. Since we cannot be sure
+ * whether just the documentation was wrong or older versions of the flexcan
+ * core can indeed not use the last MB, we chose the safest action and make
+ * this driver only use 63 MB's in every case for simplicity.
+ */
+#define FLEXCAN_MCR_MAXMB_USED (0x3f)
#define FLEXCAN_MCR_IDAM_A (0 << 8)
#define FLEXCAN_MCR_IDAM_B (1 << 8)
#define FLEXCAN_MCR_IDAM_C (2 << 8)
@@ -113,6 +127,9 @@
#define FLEXCAN_MECR_ECCDIS BIT(8)
#define FLEXCAN_MECR_NCEFAFRZ BIT(7)

+/* FLEXCAN control register 2 (CTRL2) bits */
+#define FLEXCAN_CTRL2_EACEN BIT(16)
+
/* FLEXCAN error and status register (ESR) bits */
#define FLEXCAN_ESR_TWRN_INT BIT(17)
#define FLEXCAN_ESR_RWRN_INT BIT(16)
@@ -147,17 +164,16 @@

/* FLEXCAN interrupt flag register (IFLAG) bits */
/* Errata ERR005829 step7: Reserve first valid MB */
-#define FLEXCAN_TX_BUF_RESERVED 8
-#define FLEXCAN_TX_BUF_ID 9
+#define FLEXCAN_TX_BUF_RESERVED 0
+#define FLEXCAN_TX_BUF_ID 1
+#define FLEXCAN_RX_AREA_START (FLEXCAN_TX_BUF_ID + 1)
#define FLEXCAN_IFLAG_BUF(x) BIT(x)
-#define FLEXCAN_IFLAG_RX_FIFO_OVERFLOW BIT(7)
-#define FLEXCAN_IFLAG_RX_FIFO_WARN BIT(6)
-#define FLEXCAN_IFLAG_RX_FIFO_AVAILABLE BIT(5)
-#define FLEXCAN_IFLAG_DEFAULT \
- (FLEXCAN_IFLAG_RX_FIFO_OVERFLOW | FLEXCAN_IFLAG_RX_FIFO_AVAILABLE | \
- FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID))
+#define FLEXCAN_IFLAG1_DEFAULT (0xfffffffe)
+#define FLEXCAN_IFLAG2_DEFAULT (0xffffffff)

/* FLEXCAN message buffers */
+#define FLEXCAN_MB_CODE_MASK (0xf << 24)
+#define FLEXCAN_MB_CODE_RX_BUSY_BIT (0x1 << 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)
@@ -175,8 +191,6 @@
#define FLEXCAN_MB_CNT_LENGTH(x) (((x) & 0xf) << 16)
#define FLEXCAN_MB_CNT_TIMESTAMP(x) ((x) & 0xffff)

-#define FLEXCAN_MB_CODE_MASK (0xf0ffffff)
-
#define FLEXCAN_TIMEOUT_US (50)

/*
@@ -239,7 +253,9 @@ struct flexcan_regs {
* size conf'ed via ctrl2::RFFN
* (mx6, vf610)
*/
- u32 _reserved4[408];
+ u32 _reserved4[256]; /* 0x480 */
+ u32 rximr[64]; /* 0x880 */
+ u32 _reserved5[88]; /* 0x980 */
u32 mecr; /* 0xae0 */
u32 erriar; /* 0xae4 */
u32 erridpr; /* 0xae8 */
@@ -268,6 +284,10 @@ struct flexcan_priv {
struct flexcan_platform_data *pdata;
const struct flexcan_devtype_data *devtype_data;
struct regulator *reg_xceiver;
+ struct flexcan_mb cantxfg_copy[FLEXCAN_NAPI_WEIGHT];
+ bool second_first;
+ unsigned int rx_head;
+ unsigned int rx_tail;
};

static struct flexcan_devtype_data fsl_p1010_devtype_data = {
@@ -697,16 +717,37 @@ static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
return 1;
}

-static void flexcan_read_fifo(const struct net_device *dev,
- struct can_frame *cf)
+static int flexcan_read_frame(struct net_device *dev, int n)
{
- const struct flexcan_priv *priv = netdev_priv(dev);
- struct flexcan_regs __iomem *regs = priv->base;
- struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
+ struct net_device_stats *stats = &dev->stats;
+ struct flexcan_priv *priv = netdev_priv(dev);
+ struct flexcan_mb *mb = &priv->cantxfg_copy[n];
+ struct can_frame *cf;
+ struct sk_buff *skb;
u32 reg_ctrl, reg_id;
+ u32 code;

- reg_ctrl = flexcan_read(&mb->can_ctrl);
- reg_id = flexcan_read(&mb->can_id);
+ reg_ctrl = ACCESS_ONCE(mb->can_ctrl);
+ code = reg_ctrl & FLEXCAN_MB_CODE_MASK;
+
+ /* is this MB empty? */
+ if ((code != FLEXCAN_MB_CODE_RX_FULL) &&
+ (code != FLEXCAN_MB_CODE_RX_OVERRRUN))
+ return 0;
+
+ if (code == FLEXCAN_MB_CODE_RX_OVERRRUN) {
+ /* This MB was overrun, we lost data */
+ priv->dev->stats.rx_over_errors++;
+ priv->dev->stats.rx_errors++;
+ }
+
+ skb = alloc_can_skb(dev, &cf);
+ if (unlikely(!skb)) {
+ stats->rx_dropped++;
+ return 0;
+ }
+
+ reg_id = mb->can_id;
if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
else
@@ -716,27 +757,9 @@ static void flexcan_read_fifo(const struct net_device *dev,
cf->can_id |= CAN_RTR_FLAG;
cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);

- *(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb->data[0]));
- *(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb->data[1]));
+ *(__be32 *)(cf->data + 0) = cpu_to_be32(mb->data[0]);
+ *(__be32 *)(cf->data + 4) = cpu_to_be32(mb->data[1]);

- /* mark as read */
- flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
- flexcan_read(&regs->timer);
-}
-
-static int flexcan_read_frame(struct net_device *dev)
-{
- struct net_device_stats *stats = &dev->stats;
- struct can_frame *cf;
- struct sk_buff *skb;
-
- skb = alloc_can_skb(dev, &cf);
- if (unlikely(!skb)) {
- stats->rx_dropped++;
- return 0;
- }
-
- flexcan_read_fifo(dev, cf);
netif_receive_skb(skb);

stats->rx_packets++;
@@ -750,10 +773,13 @@ static int flexcan_read_frame(struct net_device *dev)
static int flexcan_poll(struct napi_struct *napi, int quota)
{
struct net_device *dev = napi->dev;
- const struct flexcan_priv *priv = netdev_priv(dev);
+ struct flexcan_priv *priv = netdev_priv(dev);
struct flexcan_regs __iomem *regs = priv->base;
- u32 reg_iflag1, reg_esr;
+ u32 reg_esr;
int work_done = 0;
+ int ret;
+ unsigned int head;
+ unsigned int tail;

/*
* The error bits are cleared on read,
@@ -764,38 +790,173 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
/* handle state changes */
work_done += flexcan_poll_state(dev, reg_esr);

- /* handle RX-FIFO */
- reg_iflag1 = flexcan_read(&regs->iflag1);
- while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
- work_done < quota) {
- work_done += flexcan_read_frame(dev);
- reg_iflag1 = flexcan_read(&regs->iflag1);
+ /* handle mailboxes */
+ head = smp_load_acquire(&priv->rx_head);
+ tail = priv->rx_tail;
+ while ((CIRC_CNT(head, tail, ARRAY_SIZE(priv->cantxfg_copy)) >= 1) &&
+ (work_done < quota)) {
+ ret = flexcan_read_frame(dev, tail);
+ work_done += ret;
+ tail = (tail + 1) & (ARRAY_SIZE(priv->cantxfg_copy) -1);
+ smp_store_release(&priv->rx_tail, tail);
}

/* report bus errors */
if (flexcan_has_and_handle_berr(priv, reg_esr) && work_done < quota)
work_done += flexcan_poll_bus_err(dev, reg_esr);

- if (work_done < quota) {
+ if (work_done < quota)
napi_complete(napi);
- /* enable IRQs */
- flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
- flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
- }

return work_done;
}

+static u32 flexcan_copy_one_rxmb(struct flexcan_priv *priv, int i)
+{
+ struct flexcan_regs __iomem *regs = priv->base;
+ struct flexcan_mb __iomem *mb;
+ struct flexcan_mb *dst;
+ u32 reg_ctrl;
+ u32 code;
+ unsigned int head;
+ unsigned int tail;
+
+ mb = &regs->cantxfg[i];
+ reg_ctrl = flexcan_read(&mb->can_ctrl);
+ code = reg_ctrl & FLEXCAN_MB_CODE_MASK;
+
+ while (code & FLEXCAN_MB_CODE_RX_BUSY_BIT) {
+ /* MB busy, shouldn't take long */
+ reg_ctrl = flexcan_read(&mb->can_ctrl);
+ code = reg_ctrl & FLEXCAN_MB_CODE_MASK;
+ }
+
+ /* No need to process if neither FULL nor OVERRRUN */
+ if ((code != FLEXCAN_MB_CODE_RX_FULL) &&
+ (code != FLEXCAN_MB_CODE_RX_OVERRRUN))
+ return code;
+
+ head = priv->rx_head;
+ tail = ACCESS_ONCE(priv->rx_tail);
+ if (CIRC_SPACE(head, tail, ARRAY_SIZE(priv->cantxfg_copy)) >= 1) {
+ /* Copy contents */
+ dst = &priv->cantxfg_copy[priv->rx_head];
+ dst->can_id = flexcan_read(&mb->can_id);
+ dst->data[0] = flexcan_read(&mb->data[0]);
+ dst->data[1] = flexcan_read(&mb->data[1]);
+ dst->can_ctrl = reg_ctrl;
+
+ smp_store_release(&priv->rx_head,
+ (head + 1) & (ARRAY_SIZE(priv->cantxfg_copy) - 1));
+ } else {
+ /* Buffer space overrun, we lost data */
+ priv->dev->stats.rx_over_errors++;
+ priv->dev->stats.rx_errors++;
+ }
+
+ /* Clear the interrupt flag and lock MB */
+ if (i < 32)
+ flexcan_write(BIT(i), &regs->iflag1);
+ else
+ flexcan_write(BIT(i - 32), &regs->iflag2);
+ flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE, &mb->can_ctrl);
+
+ return code;
+}
+
+static void flexcan_unlock_if_locked(struct flexcan_priv *priv, int i)
+{
+ struct flexcan_regs __iomem *regs = priv->base;
+ struct flexcan_mb __iomem *mb;
+ u32 reg_ctrl;
+ u32 code;
+
+ mb = &regs->cantxfg[i];
+ reg_ctrl = flexcan_read(&mb->can_ctrl);
+ code = reg_ctrl & FLEXCAN_MB_CODE_MASK;
+ if (code == FLEXCAN_MB_CODE_RX_INACTIVE)
+ flexcan_write(FLEXCAN_MB_CODE_RX_EMPTY, &mb->can_ctrl);
+}
+
+/*
+ * flexcan_copy_rxmbs
+ *
+ * This function is called from interrupt and needs to make a safe copy
+ * of the MB area to offload the chip immediately to avoid loosing
+ * messages due to latency.
+ * To avoid loosing messages due to race-conditions while reading the MB's,
+ * we need to make use of the locking mechanism of FlexCAN. Unfortunately,
+ * in this mode, automatic locking applies _only_ to MBs that are _not_ EMPTY.
+ * This means we could identify a MB as EMPTY while it is about to get filled.
+ * To avoid this situation from disturbing our queue ordering, we do the
+ * following:
+ * We split the MB area into two halves:
+ * The lower 32 (-2 due to one TX-MB and errata ERR005829) and the upper 32.
+ * We start with all MBs in EMPTY state and all filters disabled (don't care).
+ * FlexCAN will start filling from the lowest MB. Once this function is called,
+ * we copy and disable in an atomic operation all FULL MBs. The first EMPTY one
+ * we encounter may be about to get filled so we stop there. Next time FlexCAN
+ * will have filled more MBs. Once there are no EMPTY MBs in the lower half, we
+ * EMPTY all FULL or locked MBs again.
+ * Next time we have the following situation:
+ * If there is a FULL MB in the upper half, it is because it was about to get
+ * filled when we scanned last time, or got filled just before emptying the
+ * lowest MB, so this will be the first MB we need to copy now. If there are
+ * no EMPTY MBs in the lower half at this time, it means we cannot guarantee
+ * ordering anymore. It also means latency exceeded 30 messages.
+ */
+static void flexcan_copy_rxmbs(struct flexcan_priv *priv, u32 iflag1, u32 iflag2)
+{
+ struct flexcan_regs __iomem *regs = priv->base;
+ int i;
+ unsigned int code;
+
+ if (priv->second_first) {
+ /* Begin with second half */
+ for(i = 32; i < 64; i++) {
+ flexcan_copy_one_rxmb(priv, i);
+ flexcan_unlock_if_locked(priv, i);
+ }
+ }
+
+ /* Copy and disable FULL MBs */
+ for(i = FLEXCAN_RX_AREA_START; i < 64; i++) {
+ code = flexcan_copy_one_rxmb(priv, i);
+ if (code == FLEXCAN_MB_CODE_RX_EMPTY)
+ break;
+ }
+
+ if ((i >= 32) && priv->second_first)
+ netdev_warn(priv->dev, "RX order cannot be guaranteed. count=%d\n", i);
+
+ priv->second_first = false;
+
+ /* No EMPTY MB in first half? */
+ if (i >= 32) {
+ /* Re-enable all disabled MBs */
+ for(i = FLEXCAN_RX_AREA_START; i < 64; i++)
+ flexcan_unlock_if_locked(priv, i);
+
+ /* Next time we need to check the second half first */
+ priv->second_first = true;
+ }
+
+ /* Unlock the last locked MB if any */
+ flexcan_read(&regs->timer);
+}
+
static irqreturn_t flexcan_irq(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
struct net_device_stats *stats = &dev->stats;
struct flexcan_priv *priv = netdev_priv(dev);
struct flexcan_regs __iomem *regs = priv->base;
- u32 reg_iflag1, reg_esr;
+ u32 reg_iflag1, reg_iflag2, reg_esr;

reg_iflag1 = flexcan_read(&regs->iflag1);
+ reg_iflag2 = flexcan_read(&regs->iflag2);
reg_esr = flexcan_read(&regs->esr);
+
/* ACK all bus error and state change IRQ sources */
if (reg_esr & FLEXCAN_ESR_ALL_INT)
flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
@@ -806,7 +967,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
* - state change IRQ
* - bus error IRQ and bus error reporting is activated
*/
- if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
+ if ((reg_iflag1 & ~(1 << FLEXCAN_TX_BUF_ID)) || reg_iflag2 ||
(reg_esr & FLEXCAN_ESR_ERR_STATE) ||
flexcan_has_and_handle_berr(priv, reg_esr)) {
/*
@@ -814,20 +975,10 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
* save them for later use.
*/
priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
- flexcan_write(FLEXCAN_IFLAG_DEFAULT &
- ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
- flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
- &regs->ctrl);
+ flexcan_copy_rxmbs(priv, reg_iflag1, reg_iflag2);
napi_schedule(&priv->napi);
}

- /* FIFO overflow */
- if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
- flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
- dev->stats.rx_over_errors++;
- dev->stats.rx_errors++;
- }
-
/* transmission complete interrupt */
if (reg_iflag1 & (1 << FLEXCAN_TX_BUF_ID)) {
stats->tx_bytes += can_get_echo_skb(dev, 0);
@@ -920,11 +1071,11 @@ static int flexcan_chip_start(struct net_device *dev)
*
*/
reg_mcr = flexcan_read(&regs->mcr);
- reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
- reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
+ reg_mcr &= ~(FLEXCAN_MCR_MAXMB(0xff) | FLEXCAN_MCR_FEN);
+ reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS |
- FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
+ FLEXCAN_MCR_BCC | FLEXCAN_MCR_MAXMB(FLEXCAN_MCR_MAXMB_USED);
netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
flexcan_write(reg_mcr, &regs->mcr);

@@ -960,28 +1111,36 @@ 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);
+ if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) {
+ /* CTRL2: Enable EACEN */
+ reg_ctrl2 = flexcan_read(&regs->ctrl2);
+ reg_ctrl2 |= FLEXCAN_CTRL2_EACEN | FLEXCAN_CTRL2_RRS;
+ flexcan_write(reg_ctrl2, &regs->ctrl2);
}

+ /* Prepare RX mailboxes */
+ for (i = FLEXCAN_RX_AREA_START; i < ARRAY_SIZE(regs->cantxfg); i++) {
+ flexcan_write(FLEXCAN_MB_CODE_RX_EMPTY,
+ &regs->cantxfg[i].can_ctrl);
+ flexcan_write(0, &regs->rximr[i]); /* Clear filter */
+ }
+
+ /* Prepare TX mailbox */
+ flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+ &regs->cantxfg[FLEXCAN_TX_BUF_ID].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);
+ priv->second_first = false;
+ priv->rx_head = priv->rx_tail = 0;

/* acceptance mask/acceptance code (accept everything) */
flexcan_write(0x0, &regs->rxgmask);
flexcan_write(0x0, &regs->rx14mask);
flexcan_write(0x0, &regs->rx15mask);

- if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES)
- flexcan_write(0x0, &regs->rxfgmask);
-
/*
* On Vybrid, disable memory error detection interrupts
* and freeze mode.
@@ -1018,8 +1177,9 @@ static int flexcan_chip_start(struct net_device *dev)

priv->can.state = CAN_STATE_ERROR_ACTIVE;

- /* enable FIFO interrupts */
- flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+ /* enable all MB interrupts */
+ flexcan_write(FLEXCAN_IFLAG1_DEFAULT, &regs->imask1);
+ flexcan_write(FLEXCAN_IFLAG2_DEFAULT, &regs->imask2);

/* print chip status */
netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__,

[David Jander]

The idea is to use rx-fifo from interrupt context and take away the need
for NAPI polling from the driver. Currently no support for error-handling
is included.

Signed-off-by: David Jander <***@protonic.nl>
---
drivers/net/can/dev.c | 213 +++++++++++++++++++++++++++++++++++++-----------
include/linux/can/dev.h | 21 +++--
2 files changed, 180 insertions(+), 54 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 930b9f4..22a3955 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -26,6 +26,7 @@
#include <linux/can/skb.h>
#include <linux/can/netlink.h>
#include <linux/can/led.h>
+#include <linux/circ_buf.h>
#include <net/rtnetlink.h>

#define MOD_DESC "CAN device driver interface"
@@ -281,6 +282,14 @@ static bool can_rx_fifo_ge(struct can_rx_fifo *fifo, unsigned int a, unsigned in
return a <= b;
}

+static bool can_rx_fifo_le(struct can_rx_fifo *fifo, unsigned int a, unsigned int b)
+{
+ if (fifo->inc)
+ return a <= b;
+ else
+ return a >= b;
+}
+
static unsigned int can_rx_fifo_inc(struct can_rx_fifo *fifo, unsigned int *val)
{
if (fifo->inc)
@@ -305,27 +314,100 @@ static u64 can_rx_fifo_mask_high(struct can_rx_fifo *fifo)
return ~0LLU >> (64 - fifo->high_first + fifo->high_last - 1) << fifo->high_last;
}

+static int can_rx_fifo_read_napi_frame(struct can_rx_fifo *fifo, int index)
+{
+ struct net_device *dev = fifo->dev;
+ struct net_device_stats *stats = &dev->stats;
+ struct sk_buff *skb;
+ struct can_frame *cf;
+
+ skb = alloc_can_skb(dev, &cf);
+ if (unlikely(!skb)) {
+ stats->rx_dropped++;
+ return 0;
+ }
+
+ memcpy(cf, &fifo->ring[index], sizeof(*cf));
+
+ netif_receive_skb(skb);
+
+ stats->rx_packets++;
+ stats->rx_bytes += cf->can_dlc;
+
+ can_led_event(dev, CAN_LED_EVENT_RX);
+
+ return 1;
+}
+
+static int can_rx_fifo_napi_poll(struct napi_struct *napi, int quota)
+{
+ struct can_rx_fifo *fifo = container_of(napi, struct can_rx_fifo, napi);
+ int work_done = 0;
+ int ret;
+ unsigned int head;
+ unsigned int tail;
+
+restart_poll:
+ /* handle mailboxes */
+ head = smp_load_acquire(&fifo->ring_head);
+ tail = fifo->ring_tail;
+ while ((CIRC_CNT(head, tail, fifo->ring_size) >= 1) &&
+ (work_done < quota)) {
+ ret = can_rx_fifo_read_napi_frame(fifo, tail);
+ work_done += ret;
+ tail = (tail + 1) & (fifo->ring_size -1);
+ smp_store_release(&fifo->ring_tail, tail);
+ }
+
+ if (work_done < quota) {
+ napi_complete(napi);
+
+ /* Check if there was another interrupt */
+ head = smp_load_acquire(&fifo->ring_head);
+ if ((CIRC_CNT(head, tail, fifo->ring_size) >= 1) &&
+ napi_reschedule(&fifo->napi))
+ goto restart_poll;
+ }
+
+ return work_done;
+}
+
int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)
{
+ unsigned int weight;
fifo->dev = dev;

if ((fifo->low_first < fifo->high_first) &&
- (fifo->high_first < fifo->high_last))
+ (fifo->high_first < fifo->high_last)) {
fifo->inc = true;
- else if ((fifo->low_first > fifo->high_first) &&
- (fifo->high_first > fifo->high_last))
+ weight = fifo->high_last - fifo->low_first;
+ } else if ((fifo->low_first > fifo->high_first) &&
+ (fifo->high_first > fifo->high_last)) {
fifo->inc = false;
- else
+ weight = fifo->low_first - fifo->high_last;
+ } else
return -EINVAL;

- if (!fifo->read_pending || !fifo->mailbox_enable_mask ||
- !fifo->mailbox_disable || !fifo->mailbox_receive)
+ if (!fifo->mailbox_enable_mask || !fifo->mailbox_move_to_buffer ||
+ !fifo->mailbox_enable)
return -EINVAL;

+ /* Make ring-buffer a sensible size that is a power of 2 */
+ fifo->ring_size = (2 << fls(weight));
+ fifo->ring = kzalloc(sizeof(struct can_frame) * fifo->ring_size,
+ GFP_KERNEL);
+ if (!fifo->ring)
+ return -ENOMEM;
+
+ fifo->ring_head = fifo->ring_tail = 0;
+
+ /* Take care of NAPI handling */
+ netif_napi_add(dev, &fifo->napi, can_rx_fifo_napi_poll, weight);
+
/* init variables */
fifo->mask_low = can_rx_fifo_mask_low(fifo);
fifo->mask_high = can_rx_fifo_mask_high(fifo);
- fifo->next = fifo->low_first;
+ fifo->second_first = false;
fifo->active = fifo->mask_low | fifo->mask_high;
fifo->mailbox_enable_mask(fifo, fifo->active);

@@ -338,60 +420,95 @@ int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)
}
EXPORT_SYMBOL_GPL(can_rx_fifo_add);

-int can_rx_fifo_poll(struct can_rx_fifo *fifo, int quota)
+static unsigned int can_rx_fifo_offload_if_full(struct can_rx_fifo *fifo, unsigned int n)
+{
+ unsigned int head = fifo->ring_head;
+ unsigned int tail = ACCESS_ONCE(fifo->ring_tail);
+ unsigned int ret = 0;
+
+ if (CIRC_SPACE(head, tail, fifo->ring_size) >= 1) {
+ ret = fifo->mailbox_move_to_buffer(fifo, &fifo->ring[head], n);
+ if (ret)
+ smp_store_release(&fifo->ring_head,
+ (head + 1) & (fifo->ring_size - 1));
+ } else {
+ ret = fifo->mailbox_move_to_buffer(fifo, &fifo->overflow, n);
+ if (ret)
+ fifo->dev->stats.rx_dropped++;
+ }
+ return ret;
+}
+
+int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo)
{
- int received = 0;
- u64 pending;
- unsigned int mb;
-
- do {
- pending = fifo->read_pending(fifo);
- pending &= fifo->active;
-
- if (!(pending & BIT_ULL(fifo->next))) {
- /*
- * Wrap around only if:
- * - we are in the upper group and
- * - there is a CAN frame in the first mailbox
- * of the lower group.
- */
- if (can_rx_fifo_ge(fifo, fifo->next, fifo->high_first) &&
- (pending & BIT_ULL(fifo->low_first))) {
- fifo->next = fifo->low_first;
-
- fifo->active |= fifo->mask_high;
- fifo->mailbox_enable_mask(fifo, fifo->mask_high);
- } else {
- break;
- }
+ unsigned int i;
+ unsigned int ret;
+ unsigned int received = 0;
+
+ if (fifo->second_first) {
+ for (i = fifo->high_first;
+ can_rx_fifo_le(fifo, i, fifo->high_last);
+ can_rx_fifo_inc(fifo, &i)) {
+ received += can_rx_fifo_offload_if_full(fifo, i);
+ fifo->active |= BIT_ULL(i);
+ fifo->mailbox_enable(fifo, i);
}
+ }

- mb = can_rx_fifo_inc(fifo, &fifo->next);
+ /* Copy and disable FULL MBs */
+ for (i = fifo->low_first; can_rx_fifo_le(fifo, i, fifo->high_last);
+ can_rx_fifo_inc(fifo, &i)) {
+ if (!(fifo->active & BIT_ULL(i)))
+ continue;
+ ret = can_rx_fifo_offload_if_full(fifo, i);
+ if (!ret)
+ break;
+ received += ret;
+ fifo->active &= ~BIT_ULL(i);
+ }

- /* disable mailbox */
- fifo->active &= ~BIT_ULL(mb);
- fifo->mailbox_disable(fifo, mb);
+ if (can_rx_fifo_ge(fifo, i, fifo->high_first) && fifo->second_first)
+ netdev_warn(fifo->dev, "%s: RX order cannot be guaranteed."
+ " (count=%d)\n", __func__, i);

- fifo->mailbox_receive(fifo, mb);
+ fifo->second_first = false;

- if (fifo->next == fifo->high_first) {
- fifo->active |= fifo->mask_low;
- fifo->mailbox_enable_mask(fifo, fifo->mask_low);
- }
+ /* No EMPTY MB in first half? */
+ if (can_rx_fifo_ge(fifo, i, fifo->high_first)) {
+ /* Re-enable all disabled MBs */
+ fifo->active = fifo->mask_low | fifo->mask_high;
+ fifo->mailbox_enable_mask(fifo, fifo->active);
+
+ /* Next time we need to check the second half first */
+ fifo->second_first = true;
+ }

- received++;
- quota--;
- } while (quota);
+ if (received)
+ napi_schedule(&fifo->napi);

return received;
}
-EXPORT_SYMBOL_GPL(can_rx_fifo_poll);
+EXPORT_SYMBOL_GPL(can_rx_fifo_irq_offload);
+
+void can_rx_fifo_napi_enable(struct can_rx_fifo *fifo)
+{
+ napi_enable(&fifo->napi);
+}
+EXPORT_SYMBOL_GPL(can_rx_fifo_napi_enable);
+
+void can_rx_fifo_napi_disable(struct can_rx_fifo *fifo)
+{
+ napi_disable(&fifo->napi);
+}
+EXPORT_SYMBOL_GPL(can_rx_fifo_napi_disable);

-u64 can_rx_fifo_get_active_mb_mask(const struct can_rx_fifo *fifo)
+void can_rx_fifo_del(struct can_rx_fifo *fifo)
{
- return fifo->active;
+ if (fifo->ring)
+ kfree(fifo->ring);
+ netif_napi_del(&fifo->napi);
}
-EXPORT_SYMBOL_GPL(can_rx_fifo_get_active_mb_mask);
+EXPORT_SYMBOL_GPL(can_rx_fifo_del);

/*
* Local echo of CAN messages
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index ed46f7d..64a8de3 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -71,18 +71,25 @@ struct can_rx_fifo {
unsigned int high_first;
unsigned int high_last; /* not needed during runtime */

- u64 (*read_pending)(struct can_rx_fifo *rx_fifo);
void (*mailbox_enable_mask)(struct can_rx_fifo *rx_fifo, u64 mask);
- void (*mailbox_disable)(struct can_rx_fifo *rx_fifo, unsigned int mb);
- void (*mailbox_receive)(struct can_rx_fifo *rx_fifo, unsigned int mb);
+ void (*mailbox_enable)(struct can_rx_fifo *rx_fifo, unsigned int mb);
+ unsigned int (*mailbox_move_to_buffer)(struct can_rx_fifo *rx_fifo,
+ struct can_frame *frame, unsigned int mb);

u64 mask_low;
u64 mask_high;
u64 active;

- unsigned int next;
+ unsigned int second_first;

bool inc;
+
+ struct can_frame *ring;
+ struct can_frame overflow;
+ size_t ring_size;
+ unsigned int ring_head;
+ unsigned int ring_tail;
+ struct napi_struct napi;
};

/*
@@ -127,8 +134,10 @@ u8 can_dlc2len(u8 can_dlc);
u8 can_len2dlc(u8 len);

int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo);
-int can_rx_fifo_poll(struct can_rx_fifo *fifo, int quota);
-u64 can_rx_fifo_get_active_mb_mask(const struct can_rx_fifo *fifo);
+int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo);
+void can_rx_fifo_napi_enable(struct can_rx_fifo *fifo);
+void can_rx_fifo_napi_disable(struct can_rx_fifo *fifo);
+void can_rx_fifo_del(struct can_rx_fifo *fifo);

struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max);
void free_candev(struct net_device *dev);

[Marc Kleine-Budde]

Post by David Jander
The idea is to use rx-fifo from interrupt context and take away the need
for NAPI polling from the driver. Currently no support for error-handling
is included.

Not a complete review but, at least a start. See comments inline.

Post by David Jander
---
drivers/net/can/dev.c | 213 +++++++++++++++++++++++++++++++++++++-----------
include/linux/can/dev.h | 21 +++--
2 files changed, 180 insertions(+), 54 deletions(-)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 930b9f4..22a3955 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -26,6 +26,7 @@
#include <linux/can/skb.h>
#include <linux/can/netlink.h>
#include <linux/can/led.h>
+#include <linux/circ_buf.h>
#include <net/rtnetlink.h>
#define MOD_DESC "CAN device driver interface"
@@ -281,6 +282,14 @@ static bool can_rx_fifo_ge(struct can_rx_fifo *fifo, unsigned int a, unsigned in
return a <= b;
}
+static bool can_rx_fifo_le(struct can_rx_fifo *fifo, unsigned int a, unsigned int b)
+{
+ if (fifo->inc)
+ return a <= b;
+ else
+ return a >= b;
+}
+
static unsigned int can_rx_fifo_inc(struct can_rx_fifo *fifo, unsigned int *val)
{
if (fifo->inc)
@@ -305,27 +314,100 @@ static u64 can_rx_fifo_mask_high(struct can_rx_fifo *fifo)
return ~0LLU >> (64 - fifo->high_first + fifo->high_last - 1) << fifo->high_last;
}
+static int can_rx_fifo_read_napi_frame(struct can_rx_fifo *fifo, int index)
+{
+ struct net_device *dev = fifo->dev;
+ struct net_device_stats *stats = &dev->stats;
+ struct sk_buff *skb;
+ struct can_frame *cf;
+
+ skb = alloc_can_skb(dev, &cf);
+ if (unlikely(!skb)) {
+ stats->rx_dropped++;
+ return 0;
+ }
+
+ memcpy(cf, &fifo->ring[index], sizeof(*cf));
+
+ netif_receive_skb(skb);
+
+ stats->rx_packets++;
+ stats->rx_bytes += cf->can_dlc;

cf may not be valid after netif_receive_skb() anymore. Please so the stats
before calling it.

Post by David Jander
+
+ can_led_event(dev, CAN_LED_EVENT_RX);

Please call can_led_event only once per napi invocation.

Post by David Jander
+
+ return 1;
+}
+
+static int can_rx_fifo_napi_poll(struct napi_struct *napi, int quota)
+{
+ struct can_rx_fifo *fifo = container_of(napi, struct can_rx_fifo, napi);
+ int work_done = 0;
+ int ret;
+ unsigned int head;
+ unsigned int tail;
+
+ /* handle mailboxes */
+ head = smp_load_acquire(&fifo->ring_head);
+ tail = fifo->ring_tail;
+ while ((CIRC_CNT(head, tail, fifo->ring_size) >= 1) &&
+ (work_done < quota)) {
+ ret = can_rx_fifo_read_napi_frame(fifo, tail);
+ work_done += ret;
+ tail = (tail + 1) & (fifo->ring_size -1);
+ smp_store_release(&fifo->ring_tail, tail);
+ }
+
+ if (work_done < quota) {
+ napi_complete(napi);
+
+ /* Check if there was another interrupt */
+ head = smp_load_acquire(&fifo->ring_head);
+ if ((CIRC_CNT(head, tail, fifo->ring_size) >= 1) &&
+ napi_reschedule(&fifo->napi))
+ goto restart_poll;

Hmmm, this looks a bit strange. If I understand the code correctly you ask that
napi should be started again, but then jump directly to the beginning.

Post by David Jander
+ }
+
+ return work_done;
+}
+
int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)
{
+ unsigned int weight;
fifo->dev = dev;
if ((fifo->low_first < fifo->high_first) &&
- (fifo->high_first < fifo->high_last))
+ (fifo->high_first < fifo->high_last)) {
fifo->inc = true;
- else if ((fifo->low_first > fifo->high_first) &&
- (fifo->high_first > fifo->high_last))
+ weight = fifo->high_last - fifo->low_first;
+ } else if ((fifo->low_first > fifo->high_first) &&
+ (fifo->high_first > fifo->high_last)) {
fifo->inc = false;
- else
+ weight = fifo->low_first - fifo->high_last;
+ } else
return -EINVAL;

Please but { } at every branch of the if else.

Post by David Jander
- if (!fifo->read_pending || !fifo->mailbox_enable_mask ||
- !fifo->mailbox_disable || !fifo->mailbox_receive)
+ if (!fifo->mailbox_enable_mask || !fifo->mailbox_move_to_buffer ||
+ !fifo->mailbox_enable)
return -EINVAL;
+ /* Make ring-buffer a sensible size that is a power of 2 */
+ fifo->ring_size = (2 << fls(weight));
+ fifo->ring = kzalloc(sizeof(struct can_frame) * fifo->ring_size,
+ GFP_KERNEL);
+ if (!fifo->ring)
+ return -ENOMEM;
+
+ fifo->ring_head = fifo->ring_tail = 0;
+
+ /* Take care of NAPI handling */
+ netif_napi_add(dev, &fifo->napi, can_rx_fifo_napi_poll, weight);

I'm not sure, if the rx-fifo should take care of the whole NAPI, I think it's
better to provide helper functions instead.

Post by David Jander
+
/* init variables */
fifo->mask_low = can_rx_fifo_mask_low(fifo);
fifo->mask_high = can_rx_fifo_mask_high(fifo);
- fifo->next = fifo->low_first;
+ fifo->second_first = false;
fifo->active = fifo->mask_low | fifo->mask_high;
fifo->mailbox_enable_mask(fifo, fifo->active);
@@ -338,60 +420,95 @@ int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)
}
EXPORT_SYMBOL_GPL(can_rx_fifo_add);
-int can_rx_fifo_poll(struct can_rx_fifo *fifo, int quota)
+static unsigned int can_rx_fifo_offload_if_full(struct can_rx_fifo *fifo, unsigned int n)
+{
+ unsigned int head = fifo->ring_head;
+ unsigned int tail = ACCESS_ONCE(fifo->ring_tail);
+ unsigned int ret = 0;
+
+ if (CIRC_SPACE(head, tail, fifo->ring_size) >= 1) {
+ ret = fifo->mailbox_move_to_buffer(fifo, &fifo->ring[head], n);
+ if (ret)
+ smp_store_release(&fifo->ring_head,
+ (head + 1) & (fifo->ring_size - 1));
+ } else {
+ ret = fifo->mailbox_move_to_buffer(fifo, &fifo->overflow, n);
+ if (ret)
+ fifo->dev->stats.rx_dropped++;
+ }

That's the purpose of the overflow mailbox? fifo-> overflow seems to be write
only?

Post by David Jander
+ return ret;
+}
+
+int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo)
{
- int received = 0;
- u64 pending;
- unsigned int mb;
-
- do {
- pending = fifo->read_pending(fifo);
- pending &= fifo->active;
-
- if (!(pending & BIT_ULL(fifo->next))) {
- /*
- * - we are in the upper group and
- * - there is a CAN frame in the first mailbox
- * of the lower group.
- */
- if (can_rx_fifo_ge(fifo, fifo->next, fifo->high_first) &&
- (pending & BIT_ULL(fifo->low_first))) {
- fifo->next = fifo->low_first;
-
- fifo->active |= fifo->mask_high;
- fifo->mailbox_enable_mask(fifo, fifo->mask_high);
- } else {
- break;
- }
+ unsigned int i;
+ unsigned int ret;
+ unsigned int received = 0;
+
+ if (fifo->second_first) {
+ for (i = fifo->high_first;
+ can_rx_fifo_le(fifo, i, fifo->high_last);
+ can_rx_fifo_inc(fifo, &i)) {
+ received += can_rx_fifo_offload_if_full(fifo, i);
+ fifo->active |= BIT_ULL(i);
+ fifo->mailbox_enable(fifo, i);
}
+ }
- mb = can_rx_fifo_inc(fifo, &fifo->next);
+ /* Copy and disable FULL MBs */
+ for (i = fifo->low_first; can_rx_fifo_le(fifo, i, fifo->high_last);
+ can_rx_fifo_inc(fifo, &i)) {
+ if (!(fifo->active & BIT_ULL(i)))
+ continue;
+ ret = can_rx_fifo_offload_if_full(fifo, i);
+ if (!ret)
+ break;
+ received += ret;
+ fifo->active &= ~BIT_ULL(i);
+ }
- /* disable mailbox */
- fifo->active &= ~BIT_ULL(mb);
- fifo->mailbox_disable(fifo, mb);
+ if (can_rx_fifo_ge(fifo, i, fifo->high_first) && fifo->second_first)
+ netdev_warn(fifo->dev, "%s: RX order cannot be guaranteed."
+ " (count=%d)\n", __func__, i);
- fifo->mailbox_receive(fifo, mb);
+ fifo->second_first = false;
- if (fifo->next == fifo->high_first) {
- fifo->active |= fifo->mask_low;
- fifo->mailbox_enable_mask(fifo, fifo->mask_low);
- }
+ /* No EMPTY MB in first half? */
+ if (can_rx_fifo_ge(fifo, i, fifo->high_first)) {
+ /* Re-enable all disabled MBs */
+ fifo->active = fifo->mask_low | fifo->mask_high;
+ fifo->mailbox_enable_mask(fifo, fifo->active);
+
+ /* Next time we need to check the second half first */
+ fifo->second_first = true;
+ }
- received++;
- quota--;
- } while (quota);
+ if (received)
+ napi_schedule(&fifo->napi);
return received;
}
-EXPORT_SYMBOL_GPL(can_rx_fifo_poll);
+EXPORT_SYMBOL_GPL(can_rx_fifo_irq_offload);
+
+void can_rx_fifo_napi_enable(struct can_rx_fifo *fifo)
+{
+ napi_enable(&fifo->napi);
+}
+EXPORT_SYMBOL_GPL(can_rx_fifo_napi_enable);
+
+void can_rx_fifo_napi_disable(struct can_rx_fifo *fifo)
+{
+ napi_disable(&fifo->napi);
+}
+EXPORT_SYMBOL_GPL(can_rx_fifo_napi_disable);
-u64 can_rx_fifo_get_active_mb_mask(const struct can_rx_fifo *fifo)
+void can_rx_fifo_del(struct can_rx_fifo *fifo)
{
- return fifo->active;
+ if (fifo->ring)
+ kfree(fifo->ring);

kfree() can be called with NULL

Post by David Jander
+ netif_napi_del(&fifo->napi);
}
-EXPORT_SYMBOL_GPL(can_rx_fifo_get_active_mb_mask);
+EXPORT_SYMBOL_GPL(can_rx_fifo_del);
/*
* Local echo of CAN messages
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index ed46f7d..64a8de3 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -71,18 +71,25 @@ struct can_rx_fifo {
unsigned int high_first;
unsigned int high_last; /* not needed during runtime */
- u64 (*read_pending)(struct can_rx_fifo *rx_fifo);
void (*mailbox_enable_mask)(struct can_rx_fifo *rx_fifo, u64 mask);
- void (*mailbox_disable)(struct can_rx_fifo *rx_fifo, unsigned int mb);
- void (*mailbox_receive)(struct can_rx_fifo *rx_fifo, unsigned int mb);
+ void (*mailbox_enable)(struct can_rx_fifo *rx_fifo, unsigned int mb);
+ unsigned int (*mailbox_move_to_buffer)(struct can_rx_fifo *rx_fifo,
+ struct can_frame *frame, unsigned int mb);
u64 mask_low;
u64 mask_high;
u64 active;
- unsigned int next;
+ unsigned int second_first;

The rest of the code talks about low and high, what about naming this variable,
high_first?

Post by David Jander
bool inc;
+
+ struct can_frame *ring;
+ struct can_frame overflow;
+ size_t ring_size;
+ unsigned int ring_head;
+ unsigned int ring_tail;
+ struct napi_struct napi;
};
/*
@@ -127,8 +134,10 @@ u8 can_dlc2len(u8 can_dlc);
u8 can_len2dlc(u8 len);
int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo);
-int can_rx_fifo_poll(struct can_rx_fifo *fifo, int quota);
-u64 can_rx_fifo_get_active_mb_mask(const struct can_rx_fifo *fifo);
+int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo);
+void can_rx_fifo_napi_enable(struct can_rx_fifo *fifo);
+void can_rx_fifo_napi_disable(struct can_rx_fifo *fifo);
+void can_rx_fifo_del(struct can_rx_fifo *fifo);
struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max);
void free_candev(struct net_device *dev);

Marc

[David Jander]

On Mon, 20 Oct 2014 00:09:09 +0200

Post by Marc Kleine-Budde

Post by David Jander
The idea is to use rx-fifo from interrupt context and take away the need
for NAPI polling from the driver. Currently no support for error-handling
is included.

Not a complete review but, at least a start. See comments inline.

Thanks a lot.

Post by Marc Kleine-Budde

Post by David Jander
---
drivers/net/can/dev.c | 213
+++++++++++++++++++++++++++++++++++++----------- include/linux/can/dev.h
| 21 +++-- 2 files changed, 180 insertions(+), 54 deletions(-)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 930b9f4..22a3955 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -26,6 +26,7 @@
#include <linux/can/skb.h>
#include <linux/can/netlink.h>
#include <linux/can/led.h>
+#include <linux/circ_buf.h>
#include <net/rtnetlink.h>
#define MOD_DESC "CAN device driver interface"
@@ -281,6 +282,14 @@ static bool can_rx_fifo_ge(struct can_rx_fifo *fifo,
unsigned int a, unsigned in return a <= b;
}
+static bool can_rx_fifo_le(struct can_rx_fifo *fifo, unsigned int a,
unsigned int b) +{
+ if (fifo->inc)
+ return a <= b;
+ else
+ return a >= b;
+}
+
static unsigned int can_rx_fifo_inc(struct can_rx_fifo *fifo, unsigned
int *val) {
if (fifo->inc)
@@ -305,27 +314,100 @@ static u64 can_rx_fifo_mask_high(struct can_rx_fifo
*fifo) return ~0LLU >> (64 - fifo->high_first + fifo->high_last - 1) <<
fifo->high_last; }
+static int can_rx_fifo_read_napi_frame(struct can_rx_fifo *fifo, int
index) +{
+ struct net_device *dev = fifo->dev;
+ struct net_device_stats *stats = &dev->stats;
+ struct sk_buff *skb;
+ struct can_frame *cf;
+
+ skb = alloc_can_skb(dev, &cf);
+ if (unlikely(!skb)) {
+ stats->rx_dropped++;
+ return 0;
+ }
+
+ memcpy(cf, &fifo->ring[index], sizeof(*cf));
+
+ netif_receive_skb(skb);
+
+ stats->rx_packets++;
+ stats->rx_bytes += cf->can_dlc;

cf may not be valid after netif_receive_skb() anymore. Please so the stats
before calling it.

Oops, right. Will fix that.

Post by Marc Kleine-Budde

Post by David Jander
+
+ can_led_event(dev, CAN_LED_EVENT_RX);

Please call can_led_event only once per napi invocation.

That is indeed a better idea. Will move the can_led_event() call to the end of
can_rx_fifo_napi_poll().

Post by Marc Kleine-Budde

Post by David Jander
+
+ return 1;
+}
+
+static int can_rx_fifo_napi_poll(struct napi_struct *napi, int quota)
+{
+ struct can_rx_fifo *fifo = container_of(napi, struct can_rx_fifo, napi);
+ int work_done = 0;
+ int ret;
+ unsigned int head;
+ unsigned int tail;
+
+ /* handle mailboxes */
+ head = smp_load_acquire(&fifo->ring_head);
+ tail = fifo->ring_tail;
+ while ((CIRC_CNT(head, tail, fifo->ring_size) >= 1) &&
+ (work_done < quota)) {
+ ret = can_rx_fifo_read_napi_frame(fifo, tail);
+ work_done += ret;
+ tail = (tail + 1) & (fifo->ring_size -1);
+ smp_store_release(&fifo->ring_tail, tail);
+ }
+
+ if (work_done < quota) {
+ napi_complete(napi);
+
+ /* Check if there was another interrupt */
+ head = smp_load_acquire(&fifo->ring_head);
+ if ((CIRC_CNT(head, tail, fifo->ring_size) >= 1) &&
+ napi_reschedule(&fifo->napi))
+ goto restart_poll;

Hmmm, this looks a bit strange. If I understand the code correctly you ask
that napi should be started again, but then jump directly to the beginning.

The documentation seems to say that one should use it like that:

http://www.linuxfoundation.org/collaborate/workgroups/networking/napi

If you still think it is wrong, then tell me how to re-enable napi and
continue correctly. AFAIK it is done like this in order to avoid a race when
the interrupt is called while NAPI polling was underway. napi_schedule() just
sets a flag, and does _not_ add work to a queue...

Post by Marc Kleine-Budde

Post by David Jander
+ }
+
+ return work_done;
+}
+
int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)
{
+ unsigned int weight;
fifo->dev = dev;
if ((fifo->low_first < fifo->high_first) &&
- (fifo->high_first < fifo->high_last))
+ (fifo->high_first < fifo->high_last)) {
fifo->inc = true;
- else if ((fifo->low_first > fifo->high_first) &&
- (fifo->high_first > fifo->high_last))
+ weight = fifo->high_last - fifo->low_first;
+ } else if ((fifo->low_first > fifo->high_first) &&
+ (fifo->high_first > fifo->high_last)) {
fifo->inc = false;
- else
+ weight = fifo->low_first - fifo->high_last;
+ } else
return -EINVAL;

Please but { } at every branch of the if else.

Ok, will do.

Post by Marc Kleine-Budde

Post by David Jander
- if (!fifo->read_pending || !fifo->mailbox_enable_mask ||
- !fifo->mailbox_disable || !fifo->mailbox_receive)
+ if (!fifo->mailbox_enable_mask || !fifo->mailbox_move_to_buffer ||
+ !fifo->mailbox_enable)
return -EINVAL;
+ /* Make ring-buffer a sensible size that is a power of 2 */
+ fifo->ring_size = (2 << fls(weight));
+ fifo->ring = kzalloc(sizeof(struct can_frame) * fifo->ring_size,
+ GFP_KERNEL);
+ if (!fifo->ring)
+ return -ENOMEM;
+
+ fifo->ring_head = fifo->ring_tail = 0;
+
+ /* Take care of NAPI handling */
+ netif_napi_add(dev, &fifo->napi, can_rx_fifo_napi_poll, weight);

I'm not sure, if the rx-fifo should take care of the whole NAPI, I think it's
better to provide helper functions instead.

Why not? We are removing the messages from the CAN controller in the IRQ
already, so why would the CAN driver have to even care about NAPI which
happens _after_ all that? Can you come up with an example where this may not
be desirable?
Can you illustrate your idea with helper functions?

Post by Marc Kleine-Budde

Post by David Jander
+
/* init variables */
fifo->mask_low = can_rx_fifo_mask_low(fifo);
fifo->mask_high = can_rx_fifo_mask_high(fifo);
- fifo->next = fifo->low_first;
+ fifo->second_first = false;
fifo->active = fifo->mask_low | fifo->mask_high;
fifo->mailbox_enable_mask(fifo, fifo->active);
@@ -338,60 +420,95 @@ int can_rx_fifo_add(struct net_device *dev, struct
can_rx_fifo *fifo) }
EXPORT_SYMBOL_GPL(can_rx_fifo_add);
-int can_rx_fifo_poll(struct can_rx_fifo *fifo, int quota)
+static unsigned int can_rx_fifo_offload_if_full(struct can_rx_fifo *fifo,
unsigned int n) +{
+ unsigned int head = fifo->ring_head;
+ unsigned int tail = ACCESS_ONCE(fifo->ring_tail);
+ unsigned int ret = 0;
+
+ if (CIRC_SPACE(head, tail, fifo->ring_size) >= 1) {
+ ret = fifo->mailbox_move_to_buffer(fifo,
&fifo->ring[head], n);
+ if (ret)
+ smp_store_release(&fifo->ring_head,
+ (head + 1) & (fifo->ring_size - 1));
+ } else {
+ ret = fifo->mailbox_move_to_buffer(fifo, &fifo->overflow,
n);
+ if (ret)
+ fifo->dev->stats.rx_dropped++;
+ }

That's the purpose of the overflow mailbox? fifo-> overflow seems to be write
only?

Yes.
The idea is to simplify the code for the user. mailbox_move_to_buffer() should
just move the corresponding can message to the provided buffer and do all
interrupt-flag clearing and stuff the driver needs to do in order to free the
MB. Its just that in the case we don't have space in the circular buffer, I
don't want to complicate things for the driver and tell him that this message
should be discarded. Just think of fifo->overflow as a sort of /dev/null.
Of course I could just pass NULL to that function, but IMHO that's dangerous
because since it happens only very seldom, a missing check for NULL in that
function may bite you when it is already too late (i.e. the driver already hit
mainline).

Post by Marc Kleine-Budde

Post by David Jander
+ return ret;
+}
+
+int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo)
{
- int received = 0;
- u64 pending;
- unsigned int mb;
-
- do {
- pending = fifo->read_pending(fifo);
- pending &= fifo->active;
-
- if (!(pending & BIT_ULL(fifo->next))) {
- /*
- * - we are in the upper group and
- * - there is a CAN frame in the first mailbox
- * of the lower group.
- */
- if (can_rx_fifo_ge(fifo, fifo->next,
fifo->high_first) &&
- (pending & BIT_ULL(fifo->low_first))) {
- fifo->next = fifo->low_first;
-
- fifo->active |= fifo->mask_high;
- fifo->mailbox_enable_mask(fifo,
fifo->mask_high);
- } else {
- break;
- }
+ unsigned int i;
+ unsigned int ret;
+ unsigned int received = 0;
+
+ if (fifo->second_first) {
+ for (i = fifo->high_first;
+ can_rx_fifo_le(fifo, i, fifo->high_last);
+ can_rx_fifo_inc(fifo, &i)) {
+ received += can_rx_fifo_offload_if_full(fifo, i);
+ fifo->active |= BIT_ULL(i);
+ fifo->mailbox_enable(fifo, i);
}
+ }
- mb = can_rx_fifo_inc(fifo, &fifo->next);
+ /* Copy and disable FULL MBs */
+ for (i = fifo->low_first; can_rx_fifo_le(fifo, i,
fifo->high_last);
+ can_rx_fifo_inc(fifo, &i)) {
+ if (!(fifo->active & BIT_ULL(i)))
+ continue;
+ ret = can_rx_fifo_offload_if_full(fifo, i);
+ if (!ret)
+ break;
+ received += ret;
+ fifo->active &= ~BIT_ULL(i);
+ }
- /* disable mailbox */
- fifo->active &= ~BIT_ULL(mb);
- fifo->mailbox_disable(fifo, mb);
+ if (can_rx_fifo_ge(fifo, i, fifo->high_first) &&
fifo->second_first)
+ netdev_warn(fifo->dev, "%s: RX order cannot be
guaranteed."
+ " (count=%d)\n", __func__, i);
- fifo->mailbox_receive(fifo, mb);
+ fifo->second_first = false;
- if (fifo->next == fifo->high_first) {
- fifo->active |= fifo->mask_low;
- fifo->mailbox_enable_mask(fifo, fifo->mask_low);
- }
+ /* No EMPTY MB in first half? */
+ if (can_rx_fifo_ge(fifo, i, fifo->high_first)) {
+ /* Re-enable all disabled MBs */
+ fifo->active = fifo->mask_low | fifo->mask_high;
+ fifo->mailbox_enable_mask(fifo, fifo->active);
+
+ /* Next time we need to check the second half first */
+ fifo->second_first = true;
+ }
- received++;
- quota--;
- } while (quota);
+ if (received)
+ napi_schedule(&fifo->napi);
return received;
}
-EXPORT_SYMBOL_GPL(can_rx_fifo_poll);
+EXPORT_SYMBOL_GPL(can_rx_fifo_irq_offload);
+
+void can_rx_fifo_napi_enable(struct can_rx_fifo *fifo)
+{
+ napi_enable(&fifo->napi);
+}
+EXPORT_SYMBOL_GPL(can_rx_fifo_napi_enable);
+
+void can_rx_fifo_napi_disable(struct can_rx_fifo *fifo)
+{
+ napi_disable(&fifo->napi);
+}
+EXPORT_SYMBOL_GPL(can_rx_fifo_napi_disable);
-u64 can_rx_fifo_get_active_mb_mask(const struct can_rx_fifo *fifo)
+void can_rx_fifo_del(struct can_rx_fifo *fifo)
{
- return fifo->active;
+ if (fifo->ring)
+ kfree(fifo->ring);

kfree() can be called with NULL

Right. Will remove the if().

Post by Marc Kleine-Budde

Post by David Jander
+ netif_napi_del(&fifo->napi);
}
-EXPORT_SYMBOL_GPL(can_rx_fifo_get_active_mb_mask);
+EXPORT_SYMBOL_GPL(can_rx_fifo_del);
/*
* Local echo of CAN messages
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index ed46f7d..64a8de3 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -71,18 +71,25 @@ struct can_rx_fifo {
unsigned int high_first;
unsigned int high_last; /* not needed during
runtime */
- u64 (*read_pending)(struct can_rx_fifo *rx_fifo);
void (*mailbox_enable_mask)(struct can_rx_fifo *rx_fifo, u64 mask);
- void (*mailbox_disable)(struct can_rx_fifo *rx_fifo, unsigned int mb);
- void (*mailbox_receive)(struct can_rx_fifo *rx_fifo, unsigned int mb);
+ void (*mailbox_enable)(struct can_rx_fifo *rx_fifo, unsigned int mb);
+ unsigned int (*mailbox_move_to_buffer)(struct can_rx_fifo
*rx_fifo,
+ struct can_frame *frame, unsigned int mb);
u64 mask_low;
u64 mask_high;
u64 active;
- unsigned int next;
+ unsigned int second_first;

The rest of the code talks about low and high, what about naming this
variable, high_first?

Good idea... it was a leftover of my original patch, and I didn't change the
name. Will do.

Post by Marc Kleine-Budde

Post by David Jander
bool inc;
+
+ struct can_frame *ring;
+ struct can_frame overflow;
+ size_t ring_size;
+ unsigned int ring_head;
+ unsigned int ring_tail;
+ struct napi_struct napi;
};
/*
@@ -127,8 +134,10 @@ u8 can_dlc2len(u8 can_dlc);
u8 can_len2dlc(u8 len);
int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo);
-int can_rx_fifo_poll(struct can_rx_fifo *fifo, int quota);
-u64 can_rx_fifo_get_active_mb_mask(const struct can_rx_fifo *fifo);
+int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo);
+void can_rx_fifo_napi_enable(struct can_rx_fifo *fifo);
+void can_rx_fifo_napi_disable(struct can_rx_fifo *fifo);
+void can_rx_fifo_del(struct can_rx_fifo *fifo);
struct net_device *alloc_candev(int sizeof_priv, unsigned int
echo_skb_max); void free_candev(struct net_device *dev);

Marc

Best regards,

[David Jander]

Flexcan V10 and newer are able to receive RTR frames in a MB. Older
versions are not. Those should use flexcan in FIFO mode.

Signed-off-by: David Jander <***@protonic.nl>
---
drivers/net/can/flexcan.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 5e37ce4..b8d8058 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -197,14 +197,14 @@
* FLEXCAN hardware feature flags
*
* Below is some version info we got:
- * SOC Version IP-Version Glitch- [TR]WRN_INT Memory err
- * Filter? connected? detection
- * MX25 FlexCAN2 03.00.00.00 no no no
- * MX28 FlexCAN2 03.00.04.00 yes yes no
- * MX35 FlexCAN2 03.00.00.00 no no no
- * MX53 FlexCAN2 03.00.00.00 yes no no
- * MX6s FlexCAN3 10.00.12.00 yes yes no
- * VF610 FlexCAN3 ? no yes yes
+ * SOC Version IP-Version Glitch- [TR]WRN_INT Memory err RTR re-
+ * Filter? connected? detection ception in MB
+ * MX25 FlexCAN2 03.00.00.00 no no no no
+ * MX28 FlexCAN2 03.00.04.00 yes yes no no
+ * MX35 FlexCAN2 03.00.00.00 no no no no
+ * MX53 FlexCAN2 03.00.00.00 yes no no no
+ * MX6s FlexCAN3 10.00.12.00 yes yes no yes
+ * VF610 FlexCAN3 ? no yes yes yes?
*
* Some SOCs do not have the RX_WARN & TX_WARN interrupt line connected.
*/

[David Jander]

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

Signed-off-by: Marc Kleine-Budde <***@pengutronix.de>
---
drivers/net/can/flexcan.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index b287367..20f5c0e 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -158,7 +158,6 @@
FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID))

/* 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)
@@ -479,7 +478,7 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
struct flexcan_regs __iomem *regs = priv->base;
struct can_frame *cf = (struct can_frame *)skb->data;
u32 can_id;
- u32 ctrl = FLEXCAN_MB_CNT_CODE(0xc) | (cf->can_dlc << 16);
+ u32 ctrl = FLEXCAN_MB_CODE_TX_DATA | (cf->can_dlc << 16);

if (can_dropped_invalid_skb(dev, skb))
return NETDEV_TX_OK;

[David Jander]

Signed-off-by: David Jander <***@protonic.nl>
---
include/linux/can/dev.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index ada324c..c49c37b 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -69,7 +69,7 @@ struct can_rx_fifo {

unsigned int low_first;
unsigned int high_first;
- unsigned int high_last; /* not needed during runtime */
+ unsigned int high_last;

void (*mailbox_enable_mask)(struct can_rx_fifo *rx_fifo, u64 mask);
void (*mailbox_enable)(struct can_rx_fifo *rx_fifo, unsigned int mb);

[David Jander]

Signed-off-by: David Jander <***@protonic.nl>
---
drivers/net/can/dev.c | 24 ++++++++++++------------
include/linux/can/dev.h | 12 ++++++------
2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index c1e53e9..930b9f4 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -289,20 +289,20 @@ static unsigned int can_rx_fifo_inc(struct can_rx_fifo *fifo, unsigned int *val)
return (*val)--;
}

-static u32 can_rx_fifo_mask_low(struct can_rx_fifo *fifo)
+static u64 can_rx_fifo_mask_low(struct can_rx_fifo *fifo)
{
if (fifo->inc)
- return ~0U >> (32 + fifo->low_first - fifo->high_first) << fifo->low_first;
+ return ~0LLU >> (64 + fifo->low_first - fifo->high_first) << fifo->low_first;
else
- return ~0U >> (32 - fifo->low_first + fifo->high_first) << (fifo->high_first + 1);
+ return ~0LLU >> (64 - fifo->low_first + fifo->high_first) << (fifo->high_first + 1);
}

-static u32 can_rx_fifo_mask_high(struct can_rx_fifo *fifo)
+static u64 can_rx_fifo_mask_high(struct can_rx_fifo *fifo)
{
if (fifo->inc)
- return ~0U >> (32 + fifo->high_first - fifo->high_last - 1) << fifo->high_first;
+ return ~0LLU >> (64 + fifo->high_first - fifo->high_last - 1) << fifo->high_first;
else
- return ~0U >> (32 - fifo->high_first + fifo->high_last - 1) << fifo->high_last;
+ return ~0LLU >> (64 - fifo->high_first + fifo->high_last - 1) << fifo->high_last;
}

int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)
@@ -331,7 +331,7 @@ int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)

netdev_dbg(dev, "%s: low_first=%d, high_first=%d, high_last=%d\n", __func__,
fifo->low_first, fifo->high_first, fifo->high_last);
- netdev_dbg(dev, "%s: mask_low=0x%08x mask_high=0x%08x\n", __func__,
+ netdev_dbg(dev, "%s: mask_low=0x%016llx mask_high=0x%016llx\n", __func__,
fifo->mask_low, fifo->mask_high);

return 0;
@@ -341,14 +341,14 @@ EXPORT_SYMBOL_GPL(can_rx_fifo_add);
int can_rx_fifo_poll(struct can_rx_fifo *fifo, int quota)
{
int received = 0;
- u32 pending;
+ u64 pending;
unsigned int mb;

do {
pending = fifo->read_pending(fifo);
pending &= fifo->active;

- if (!(pending & BIT(fifo->next))) {
+ if (!(pending & BIT_ULL(fifo->next))) {
/*
* Wrap around only if:
* - we are in the upper group and
@@ -356,7 +356,7 @@ int can_rx_fifo_poll(struct can_rx_fifo *fifo, int quota)
* of the lower group.
*/
if (can_rx_fifo_ge(fifo, fifo->next, fifo->high_first) &&
- (pending & BIT(fifo->low_first))) {
+ (pending & BIT_ULL(fifo->low_first))) {
fifo->next = fifo->low_first;

fifo->active |= fifo->mask_high;
@@ -369,7 +369,7 @@ int can_rx_fifo_poll(struct can_rx_fifo *fifo, int quota)
mb = can_rx_fifo_inc(fifo, &fifo->next);

/* disable mailbox */
- fifo->active &= ~BIT(mb);
+ fifo->active &= ~BIT_ULL(mb);
fifo->mailbox_disable(fifo, mb);

fifo->mailbox_receive(fifo, mb);
@@ -387,7 +387,7 @@ int can_rx_fifo_poll(struct can_rx_fifo *fifo, int quota)
}
EXPORT_SYMBOL_GPL(can_rx_fifo_poll);

-u32 can_rx_fifo_get_active_mb_mask(const struct can_rx_fifo *fifo)
+u64 can_rx_fifo_get_active_mb_mask(const struct can_rx_fifo *fifo)
{
return fifo->active;
}
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index e9468d0..ed46f7d 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -71,14 +71,14 @@ struct can_rx_fifo {
unsigned int high_first;
unsigned int high_last; /* not needed during runtime */

- u32 (*read_pending)(struct can_rx_fifo *rx_fifo);
- void (*mailbox_enable_mask)(struct can_rx_fifo *rx_fifo, u32 mask);
+ u64 (*read_pending)(struct can_rx_fifo *rx_fifo);
+ void (*mailbox_enable_mask)(struct can_rx_fifo *rx_fifo, u64 mask);
void (*mailbox_disable)(struct can_rx_fifo *rx_fifo, unsigned int mb);
void (*mailbox_receive)(struct can_rx_fifo *rx_fifo, unsigned int mb);

- u32 mask_low;
- u32 mask_high;
- u32 active;
+ u64 mask_low;
+ u64 mask_high;
+ u64 active;

unsigned int next;

@@ -128,7 +128,7 @@ u8 can_len2dlc(u8 len);

int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo);
int can_rx_fifo_poll(struct can_rx_fifo *fifo, int quota);
-u32 can_rx_fifo_get_active_mb_mask(const struct can_rx_fifo *fifo);
+u64 can_rx_fifo_get_active_mb_mask(const struct can_rx_fifo *fifo);

struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max);
void free_candev(struct net_device *dev);

[Marc Kleine-Budde]

Please move to the beginning of the series, so that I can squash all fifo
related patches into one. So that we have just a single (hopefully perfect)
rx-fifo patch.

regards,
Marc

[David Jander]

On Sun, 19 Oct 2014 23:25:50 +0200

Post by Marc Kleine-Budde
Please move to the beginning of the series, so that I can squash all fifo
related patches into one. So that we have just a single (hopefully perfect)
rx-fifo patch.

Ok, will reorder the patches.

Best regards,

[David Jander]

The interrupt handler should store the error flags if needed and call
can_rx_fifo_irq_error() if there was an error flag set. This will trigger
a napi-poll that will call poll_can_state() and poll_bus_error()
callbacks. Those should handle can state machine and send error frames
as needed.

Signed-off-by: David Jander <***@protonic.nl>
---
drivers/net/can/dev.c | 23 ++++++++++++++++++++---
include/linux/can/dev.h | 9 +++++++++
2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index fc35d28..dac7579 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -352,6 +352,9 @@ static int can_rx_fifo_napi_poll(struct napi_struct *napi, int quota)
unsigned int tail;

restart_poll:
+ if (work_done < quota)
+ work_done += fifo->poll_can_state(fifo);
+
/* handle mailboxes */
head = smp_load_acquire(&fifo->ring_head);
tail = fifo->ring_tail;
@@ -363,14 +366,19 @@ restart_poll:
smp_store_release(&fifo->ring_tail, tail);
}

+ if (work_done < quota)
+ work_done += fifo->poll_bus_error(fifo);
+
if (work_done < quota) {
napi_complete(napi);

/* Check if there was another interrupt */
head = smp_load_acquire(&fifo->ring_head);
- if ((CIRC_CNT(head, tail, fifo->ring_size) >= 1) &&
- napi_reschedule(&fifo->napi))
+ if (((CIRC_CNT(head, tail, fifo->ring_size) >= 1) ||
+ fifo->poll_errors) && napi_reschedule(&fifo->napi)) {
+ fifo->poll_errors = false;
goto restart_poll;
+ }
}

return work_done;
@@ -393,7 +401,8 @@ int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)
return -EINVAL;

if (!fifo->mailbox_enable_mask || !fifo->mailbox_move_to_buffer ||
- !fifo->mailbox_enable)
+ !fifo->mailbox_enable || !fifo->poll_bus_error ||
+ !fifo->poll_can_state)
return -EINVAL;

/* Make ring-buffer a sensible size that is a power of 2 */
@@ -412,6 +421,7 @@ int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)
fifo->mask_low = can_rx_fifo_mask_low(fifo);
fifo->mask_high = can_rx_fifo_mask_high(fifo);
fifo->second_first = false;
+ fifo->poll_errors = false;
fifo->active = fifo->mask_low | fifo->mask_high;
fifo->mailbox_enable_mask(fifo, fifo->active);

@@ -503,6 +513,13 @@ int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo)
}
EXPORT_SYMBOL_GPL(can_rx_fifo_irq_offload);

+void can_rx_fifo_irq_error(struct can_rx_fifo *fifo)
+{
+ fifo->poll_errors = true;
+ can_rx_fifo_napi_schedule(fifo);
+}
+EXPORT_SYMBOL_GPL(can_rx_fifo_irq_error);
+
void can_rx_fifo_napi_enable(struct can_rx_fifo *fifo)
{
napi_enable(&fifo->napi);
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index c49c37b..e1ed6d4 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -75,12 +75,15 @@ struct can_rx_fifo {
void (*mailbox_enable)(struct can_rx_fifo *rx_fifo, unsigned int mb);
unsigned int (*mailbox_move_to_buffer)(struct can_rx_fifo *rx_fifo,
struct can_frame *frame, unsigned int mb);
+ unsigned int (*poll_can_state)(struct can_rx_fifo *rx_fifo);
+ unsigned int (*poll_bus_error)(struct can_rx_fifo *rx_fifo);

u64 mask_low;
u64 mask_high;
u64 active;

unsigned int second_first;
+ bool poll_errors;

bool inc;

@@ -92,6 +95,11 @@ struct can_rx_fifo {
struct napi_struct napi;
};

+static inline void can_rx_fifo_napi_schedule(struct can_rx_fifo *fifo)
+{
+ napi_schedule(&fifo->napi);
+}
+
/*
* get_can_dlc(value) - helper macro to cast a given data length code (dlc)
* to __u8 and ensure the dlc value to be max. 8 bytes.
@@ -135,6 +143,7 @@ u8 can_len2dlc(u8 len);

int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo);
int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo);
+void can_rx_fifo_irq_error(struct can_rx_fifo *fifo);
void can_rx_fifo_napi_enable(struct can_rx_fifo *fifo);
void can_rx_fifo_napi_disable(struct can_rx_fifo *fifo);
void can_rx_fifo_reset(struct can_rx_fifo *fifo);

[David Jander]

Some CAN controllers have a usable FIFO already but can still benefit from
off-loading the CAN controller FIFO in the interrupt into an extra ring-
buffer. Add support for these simpler cases also.

Signed-off-by: David Jander <***@protonic.nl>
---
drivers/net/can/dev.c | 76 ++++++++++++++++++++++++++++++++++++++++---------
include/linux/can/dev.h | 2 ++
2 files changed, 65 insertions(+), 13 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index dac7579..278aea3 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -384,10 +384,32 @@ restart_poll:
return work_done;
}

+static int can_rx_fifo_init_ring(struct net_device *dev,
+ struct can_rx_fifo *fifo, unsigned int weight)
+{
+ fifo->dev = dev;
+
+ /* Make ring-buffer a sensible size that is a power of 2 */
+ fifo->ring_size = (2 << fls(weight));
+ fifo->ring = kzalloc(sizeof(struct can_frame) * fifo->ring_size,
+ GFP_KERNEL);
+ if (!fifo->ring)
+ return -ENOMEM;
+
+ fifo->ring_head = fifo->ring_tail = 0;
+
+ /* Take care of NAPI handling */
+ netif_napi_add(dev, &fifo->napi, can_rx_fifo_napi_poll, weight);
+
+ fifo->poll_errors = false;
+
+ return 0;
+}
+
int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)
{
unsigned int weight;
- fifo->dev = dev;
+ int ret;

if ((fifo->low_first < fifo->high_first) &&
(fifo->high_first < fifo->high_last)) {
@@ -405,23 +427,14 @@ int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)
!fifo->poll_can_state)
return -EINVAL;

- /* Make ring-buffer a sensible size that is a power of 2 */
- fifo->ring_size = (2 << fls(weight));
- fifo->ring = kzalloc(sizeof(struct can_frame) * fifo->ring_size,
- GFP_KERNEL);
- if (!fifo->ring)
- return -ENOMEM;
-
- fifo->ring_head = fifo->ring_tail = 0;
-
- /* Take care of NAPI handling */
- netif_napi_add(dev, &fifo->napi, can_rx_fifo_napi_poll, weight);
+ ret = can_rx_fifo_init_ring(dev, fifo, weight);
+ if (ret)
+ return ret;

/* init variables */
fifo->mask_low = can_rx_fifo_mask_low(fifo);
fifo->mask_high = can_rx_fifo_mask_high(fifo);
fifo->second_first = false;
- fifo->poll_errors = false;
fifo->active = fifo->mask_low | fifo->mask_high;
fifo->mailbox_enable_mask(fifo, fifo->active);

@@ -434,6 +447,26 @@ int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)
}
EXPORT_SYMBOL_GPL(can_rx_fifo_add);

+int can_rx_fifo_add_simple(struct net_device *dev, struct can_rx_fifo *fifo)
+{
+ int ret;
+
+ if (!fifo->mailbox_move_to_buffer || !fifo->poll_bus_error ||
+ !fifo->poll_can_state)
+ return -EINVAL;
+
+ ret = can_rx_fifo_init_ring(dev, fifo, 64);
+ if (ret)
+ return ret;
+
+ /* init variables */
+ fifo->mask_low = 0;
+ fifo->mask_high = 0;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(can_rx_fifo_add_simple);
+
static unsigned int can_rx_fifo_offload_if_full(struct can_rx_fifo *fifo, unsigned int n)
{
unsigned int head = fifo->ring_head;
@@ -513,6 +546,23 @@ int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo)
}
EXPORT_SYMBOL_GPL(can_rx_fifo_irq_offload);

+int can_rx_fifo_irq_offload_simple(struct can_rx_fifo *fifo)
+{
+ unsigned int received = 0;
+ unsigned int ret;
+
+ do {
+ ret = can_rx_fifo_offload_if_full(fifo, 0);
+ received += ret;
+ } while (ret);
+
+ if (received)
+ can_rx_fifo_napi_schedule(fifo);
+
+ return received;
+}
+EXPORT_SYMBOL_GPL(can_rx_fifo_irq_offload_simple);
+
void can_rx_fifo_irq_error(struct can_rx_fifo *fifo)
{
fifo->poll_errors = true;
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index e1ed6d4..18feef3 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -142,7 +142,9 @@ u8 can_dlc2len(u8 can_dlc);
u8 can_len2dlc(u8 len);

int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo);
+int can_rx_fifo_add_simple(struct net_device *dev, struct can_rx_fifo *fifo);
int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo);
+int can_rx_fifo_irq_offload_simple(struct can_rx_fifo *fifo);
void can_rx_fifo_irq_error(struct can_rx_fifo *fifo);
void can_rx_fifo_napi_enable(struct can_rx_fifo *fifo);
void can_rx_fifo_napi_disable(struct can_rx_fifo *fifo);

[David Jander]

This adds support for RX-FIFO simulating a FIFO from a linear array of message-
boxes. This breaks RTR reception on flexcan older than V10.

Signed-off-by: David Jander <***@protonic.nl>
---
drivers/net/can/flexcan.c | 333 +++++++++++++++-------------------------------
1 file changed, 104 insertions(+), 229 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 8118f35..5e37ce4 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -273,10 +273,10 @@ struct flexcan_devtype_data {
struct flexcan_priv {
struct can_priv can;
struct net_device *dev;
- struct napi_struct napi;

void __iomem *base;
u32 reg_esr;
+ u32 poll_esr;
u32 reg_ctrl_default;

struct clk *clk_ipg;
@@ -284,10 +284,8 @@ struct flexcan_priv {
struct flexcan_platform_data *pdata;
const struct flexcan_devtype_data *devtype_data;
struct regulator *reg_xceiver;
- struct flexcan_mb cantxfg_copy[FLEXCAN_NAPI_WEIGHT];
- bool second_first;
- unsigned int rx_head;
- unsigned int rx_tail;
+
+ struct can_rx_fifo fifo;
};

static struct flexcan_devtype_data fsl_p1010_devtype_data = {
@@ -540,6 +538,11 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
return NETDEV_TX_OK;
}

+static inline struct flexcan_priv *rx_fifo_to_priv(struct can_rx_fifo *fifo)
+{
+ return container_of(fifo, struct flexcan_priv, fifo);
+}
+
static void do_bus_err(struct net_device *dev,
struct can_frame *cf, u32 reg_esr)
{
@@ -588,16 +591,21 @@ static void do_bus_err(struct net_device *dev,
dev->stats.tx_errors++;
}

-static int flexcan_poll_bus_err(struct net_device *dev, u32 reg_esr)
+static unsigned int flexcan_poll_bus_err(struct can_rx_fifo *fifo)
{
+ struct flexcan_priv *priv = rx_fifo_to_priv(fifo);
+ struct net_device *dev = priv->dev;
struct sk_buff *skb;
struct can_frame *cf;

+ if (!flexcan_has_and_handle_berr(priv, priv->poll_esr))
+ return 0;
+
skb = alloc_can_err_skb(dev, &cf);
if (unlikely(!skb))
return 0;

- do_bus_err(dev, cf, reg_esr);
+ do_bus_err(dev, cf, priv->poll_esr);
netif_receive_skb(skb);

dev->stats.rx_packets++;
@@ -679,17 +687,22 @@ static void do_state(struct net_device *dev,
}
}

-static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
+static unsigned int flexcan_poll_state(struct can_rx_fifo *fifo)
{
- struct flexcan_priv *priv = netdev_priv(dev);
+ struct flexcan_priv *priv = rx_fifo_to_priv(fifo);
+ struct net_device *dev = priv->dev;
+ struct flexcan_regs __iomem *regs = priv->base;
struct sk_buff *skb;
struct can_frame *cf;
enum can_state new_state;
int flt;

- flt = reg_esr & FLEXCAN_ESR_FLT_CONF_MASK;
+ /* esr bits are clear-on-read, so save them for flexcan_poll_bus_err() */
+ priv->poll_esr = priv->reg_esr | flexcan_read(&regs->esr);
+
+ flt = priv->poll_esr & FLEXCAN_ESR_FLT_CONF_MASK;
if (likely(flt == FLEXCAN_ESR_FLT_CONF_ACTIVE)) {
- if (likely(!(reg_esr & (FLEXCAN_ESR_TX_WRN |
+ if (likely(!(priv->poll_esr & (FLEXCAN_ESR_TX_WRN |
FLEXCAN_ESR_RX_WRN))))
new_state = CAN_STATE_ERROR_ACTIVE;
else
@@ -717,232 +730,88 @@ static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
return 1;
}

-static int flexcan_read_frame(struct net_device *dev, int n)
+static void flexcan_mailbox_enable(struct can_rx_fifo *fifo, unsigned int n)
{
- struct net_device_stats *stats = &dev->stats;
- struct flexcan_priv *priv = netdev_priv(dev);
- struct flexcan_mb *mb = &priv->cantxfg_copy[n];
- struct can_frame *cf;
- struct sk_buff *skb;
- u32 reg_ctrl, reg_id;
+ struct flexcan_priv *priv = rx_fifo_to_priv(fifo);
+ struct flexcan_regs __iomem *regs = priv->base;
+ struct flexcan_mb __iomem *mb;
+ u32 reg_ctrl;
u32 code;

- reg_ctrl = ACCESS_ONCE(mb->can_ctrl);
+ mb = &regs->cantxfg[n];
+ reg_ctrl = flexcan_read(&mb->can_ctrl);
code = reg_ctrl & FLEXCAN_MB_CODE_MASK;
-
- /* is this MB empty? */
- if ((code != FLEXCAN_MB_CODE_RX_FULL) &&
- (code != FLEXCAN_MB_CODE_RX_OVERRRUN))
- return 0;
-
- if (code == FLEXCAN_MB_CODE_RX_OVERRRUN) {
- /* This MB was overrun, we lost data */
- priv->dev->stats.rx_over_errors++;
- priv->dev->stats.rx_errors++;
- }
-
- skb = alloc_can_skb(dev, &cf);
- if (unlikely(!skb)) {
- stats->rx_dropped++;
- return 0;
- }
-
- reg_id = mb->can_id;
- if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
- cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
- else
- cf->can_id = (reg_id >> 18) & CAN_SFF_MASK;
-
- if (reg_ctrl & FLEXCAN_MB_CNT_RTR)
- cf->can_id |= CAN_RTR_FLAG;
- cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
-
- *(__be32 *)(cf->data + 0) = cpu_to_be32(mb->data[0]);
- *(__be32 *)(cf->data + 4) = cpu_to_be32(mb->data[1]);
-
- netif_receive_skb(skb);
-
- stats->rx_packets++;
- stats->rx_bytes += cf->can_dlc;
-
- can_led_event(dev, CAN_LED_EVENT_RX);
-
- return 1;
+ if (code == FLEXCAN_MB_CODE_RX_INACTIVE)
+ flexcan_write(FLEXCAN_MB_CODE_RX_EMPTY, &mb->can_ctrl);
+ flexcan_read(&regs->timer);
}

-static int flexcan_poll(struct napi_struct *napi, int quota)
+static void flexcan_mailbox_enable_mask(struct can_rx_fifo *fifo, u64 mask)
{
- struct net_device *dev = napi->dev;
- struct flexcan_priv *priv = netdev_priv(dev);
+ struct flexcan_priv *priv = rx_fifo_to_priv(fifo);
struct flexcan_regs __iomem *regs = priv->base;
- u32 reg_esr;
- int work_done = 0;
- int ret;
- unsigned int head;
- unsigned int tail;
+ struct flexcan_mb __iomem *mb;
+ u32 reg_ctrl;
+ u32 code;
+ unsigned int n;

- /*
- * The error bits are cleared on read,
- * use saved value from irq handler.
- */
- reg_esr = flexcan_read(&regs->esr) | priv->reg_esr;
-
- /* handle state changes */
- work_done += flexcan_poll_state(dev, reg_esr);
-
- /* handle mailboxes */
- head = smp_load_acquire(&priv->rx_head);
- tail = priv->rx_tail;
- while ((CIRC_CNT(head, tail, ARRAY_SIZE(priv->cantxfg_copy)) >= 1) &&
- (work_done < quota)) {
- ret = flexcan_read_frame(dev, tail);
- work_done += ret;
- tail = (tail + 1) & (ARRAY_SIZE(priv->cantxfg_copy) -1);
- smp_store_release(&priv->rx_tail, tail);
+ for (n = FLEXCAN_RX_AREA_START; n < ARRAY_SIZE(regs->cantxfg); n ++) {
+ mb = &regs->cantxfg[n];
+ reg_ctrl = flexcan_read(&mb->can_ctrl);
+ code = reg_ctrl & FLEXCAN_MB_CODE_MASK;
+ if ((mask & BIT_ULL(n)) && (code == FLEXCAN_MB_CODE_RX_INACTIVE))
+ flexcan_write(FLEXCAN_MB_CODE_RX_EMPTY, &mb->can_ctrl);
}
-
- /* report bus errors */
- if (flexcan_has_and_handle_berr(priv, reg_esr) && work_done < quota)
- work_done += flexcan_poll_bus_err(dev, reg_esr);
-
- if (work_done < quota)
- napi_complete(napi);
-
- return work_done;
+ flexcan_read(&regs->timer);
}

-static u32 flexcan_copy_one_rxmb(struct flexcan_priv *priv, int i)
+static unsigned int flexcan_mailbox_move_to_buffer(struct can_rx_fifo *fifo,
+ struct can_frame *cf, unsigned int n)
{
+ struct flexcan_priv *priv = rx_fifo_to_priv(fifo);
struct flexcan_regs __iomem *regs = priv->base;
- struct flexcan_mb __iomem *mb;
- struct flexcan_mb *dst;
- u32 reg_ctrl;
+ struct flexcan_mb *mb = &regs->cantxfg[n];
+ u32 reg_ctrl, reg_id;
u32 code;
- unsigned int head;
- unsigned int tail;

- mb = &regs->cantxfg[i];
- reg_ctrl = flexcan_read(&mb->can_ctrl);
- code = reg_ctrl & FLEXCAN_MB_CODE_MASK;
-
- while (code & FLEXCAN_MB_CODE_RX_BUSY_BIT) {
- /* MB busy, shouldn't take long */
+ do {
reg_ctrl = flexcan_read(&mb->can_ctrl);
code = reg_ctrl & FLEXCAN_MB_CODE_MASK;
- }
+ } while (code & FLEXCAN_MB_CODE_RX_BUSY_BIT);

- /* No need to process if neither FULL nor OVERRRUN */
+ /* is this MB empty? */
if ((code != FLEXCAN_MB_CODE_RX_FULL) &&
(code != FLEXCAN_MB_CODE_RX_OVERRRUN))
- return code;
-
- head = priv->rx_head;
- tail = ACCESS_ONCE(priv->rx_tail);
- if (CIRC_SPACE(head, tail, ARRAY_SIZE(priv->cantxfg_copy)) >= 1) {
- /* Copy contents */
- dst = &priv->cantxfg_copy[priv->rx_head];
- dst->can_id = flexcan_read(&mb->can_id);
- dst->data[0] = flexcan_read(&mb->data[0]);
- dst->data[1] = flexcan_read(&mb->data[1]);
- dst->can_ctrl = reg_ctrl;
-
- smp_store_release(&priv->rx_head,
- (head + 1) & (ARRAY_SIZE(priv->cantxfg_copy) - 1));
- } else {
- /* Buffer space overrun, we lost data */
+ return 0;
+
+ if (code == FLEXCAN_MB_CODE_RX_OVERRRUN) {
+ /* This MB was overrun, we lost data */
priv->dev->stats.rx_over_errors++;
priv->dev->stats.rx_errors++;
}

- /* Clear the interrupt flag and lock MB */
- if (i < 32)
- flexcan_write(BIT(i), &regs->iflag1);
+ reg_id = flexcan_read(&mb->can_id);
+ if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
+ cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
else
- flexcan_write(BIT(i - 32), &regs->iflag2);
- flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE, &mb->can_ctrl);
-
- return code;
-}
-
-static void flexcan_unlock_if_locked(struct flexcan_priv *priv, int i)
-{
- struct flexcan_regs __iomem *regs = priv->base;
- struct flexcan_mb __iomem *mb;
- u32 reg_ctrl;
- u32 code;
-
- mb = &regs->cantxfg[i];
- reg_ctrl = flexcan_read(&mb->can_ctrl);
- code = reg_ctrl & FLEXCAN_MB_CODE_MASK;
- if (code == FLEXCAN_MB_CODE_RX_INACTIVE)
- flexcan_write(FLEXCAN_MB_CODE_RX_EMPTY, &mb->can_ctrl);
-}
-
-/*
- * flexcan_copy_rxmbs
- *
- * This function is called from interrupt and needs to make a safe copy
- * of the MB area to offload the chip immediately to avoid loosing
- * messages due to latency.
- * To avoid loosing messages due to race-conditions while reading the MB's,
- * we need to make use of the locking mechanism of FlexCAN. Unfortunately,
- * in this mode, automatic locking applies _only_ to MBs that are _not_ EMPTY.
- * This means we could identify a MB as EMPTY while it is about to get filled.
- * To avoid this situation from disturbing our queue ordering, we do the
- * following:
- * We split the MB area into two halves:
- * The lower 32 (-2 due to one TX-MB and errata ERR005829) and the upper 32.
- * We start with all MBs in EMPTY state and all filters disabled (don't care).
- * FlexCAN will start filling from the lowest MB. Once this function is called,
- * we copy and disable in an atomic operation all FULL MBs. The first EMPTY one
- * we encounter may be about to get filled so we stop there. Next time FlexCAN
- * will have filled more MBs. Once there are no EMPTY MBs in the lower half, we
- * EMPTY all FULL or locked MBs again.
- * Next time we have the following situation:
- * If there is a FULL MB in the upper half, it is because it was about to get
- * filled when we scanned last time, or got filled just before emptying the
- * lowest MB, so this will be the first MB we need to copy now. If there are
- * no EMPTY MBs in the lower half at this time, it means we cannot guarantee
- * ordering anymore. It also means latency exceeded 30 messages.
- */
-static void flexcan_copy_rxmbs(struct flexcan_priv *priv, u32 iflag1, u32 iflag2)
-{
- struct flexcan_regs __iomem *regs = priv->base;
- int i;
- unsigned int code;
-
- if (priv->second_first) {
- /* Begin with second half */
- for(i = 32; i < 64; i++) {
- flexcan_copy_one_rxmb(priv, i);
- flexcan_unlock_if_locked(priv, i);
- }
- }
-
- /* Copy and disable FULL MBs */
- for(i = FLEXCAN_RX_AREA_START; i < 64; i++) {
- code = flexcan_copy_one_rxmb(priv, i);
- if (code == FLEXCAN_MB_CODE_RX_EMPTY)
- break;
- }
+ cf->can_id = (reg_id >> 18) & CAN_SFF_MASK;

- if ((i >= 32) && priv->second_first)
- netdev_warn(priv->dev, "RX order cannot be guaranteed. count=%d\n", i);
+ if (reg_ctrl & FLEXCAN_MB_CNT_RTR)
+ cf->can_id |= CAN_RTR_FLAG;
+ cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);

- priv->second_first = false;
+ *(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb->data[0]));
+ *(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb->data[1]));

- /* No EMPTY MB in first half? */
- if (i >= 32) {
- /* Re-enable all disabled MBs */
- for(i = FLEXCAN_RX_AREA_START; i < 64; i++)
- flexcan_unlock_if_locked(priv, i);
+ /* Clear IRQ and lock MB */
+ if (n < 32)
+ flexcan_write(BIT(n), &regs->iflag1);
+ else
+ flexcan_write(BIT(n - 32), &regs->iflag2);

- /* Next time we need to check the second half first */
- priv->second_first = true;
- }
+ flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE, &mb->can_ctrl);

- /* Unlock the last locked MB if any */
- flexcan_read(&regs->timer);
+ return 1;
}

static irqreturn_t flexcan_irq(int irq, void *dev_id)
@@ -961,22 +830,20 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
if (reg_esr & FLEXCAN_ESR_ALL_INT)
flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);

- /*
- * schedule NAPI in case of:
- * - rx IRQ
- * - state change IRQ
- * - bus error IRQ and bus error reporting is activated
- */
- if ((reg_iflag1 & ~(1 << FLEXCAN_TX_BUF_ID)) || reg_iflag2 ||
- (reg_esr & FLEXCAN_ESR_ERR_STATE) ||
+ /* bus error IRQ and bus error reporting is activated */
+ if ((reg_esr & FLEXCAN_ESR_ERR_STATE) ||
flexcan_has_and_handle_berr(priv, reg_esr)) {
/*
* The error bits are cleared on read,
- * save them for later use.
+ * save them for later use and let napi poll get them.
*/
priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
- flexcan_copy_rxmbs(priv, reg_iflag1, reg_iflag2);
- napi_schedule(&priv->napi);
+ can_rx_fifo_irq_error(&priv->fifo);
+ }
+
+ /* reception interrupt */
+ if ((reg_iflag1 & ~(1 << FLEXCAN_TX_BUF_ID)) || reg_iflag2) {
+ can_rx_fifo_irq_offload(&priv->fifo);
}

/* transmission complete interrupt */
@@ -1133,9 +1000,6 @@ static int flexcan_chip_start(struct net_device *dev)
flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
&regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);

- priv->second_first = false;
- priv->rx_head = priv->rx_tail = 0;
-
/* acceptance mask/acceptance code (accept everything) */
flexcan_write(0x0, &regs->rxgmask);
flexcan_write(0x0, &regs->rx14mask);
@@ -1170,17 +1034,19 @@ static int flexcan_chip_start(struct net_device *dev)
if (err)
goto out_chip_disable;

- /* synchronize with the can bus */
- err = flexcan_chip_unfreeze(priv);
- if (err)
- goto out_transceiver_disable;
-
priv->can.state = CAN_STATE_ERROR_ACTIVE;

+ can_rx_fifo_reset(&priv->fifo);
+
/* enable all MB interrupts */
flexcan_write(FLEXCAN_IFLAG1_DEFAULT, &regs->imask1);
flexcan_write(FLEXCAN_IFLAG2_DEFAULT, &regs->imask2);

+ /* synchronize with the can bus */
+ err = flexcan_chip_unfreeze(priv);
+ if (err)
+ goto out_transceiver_disable;
+
/* print chip status */
netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__,
flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
@@ -1248,7 +1114,7 @@ static int flexcan_open(struct net_device *dev)

can_led_event(dev, CAN_LED_EVENT_OPEN);

- napi_enable(&priv->napi);
+ can_rx_fifo_napi_enable(&priv->fifo);
netif_start_queue(dev);

return 0;
@@ -1270,7 +1136,7 @@ static int flexcan_close(struct net_device *dev)
struct flexcan_priv *priv = netdev_priv(dev);

netif_stop_queue(dev);
- napi_disable(&priv->napi);
+ can_rx_fifo_napi_disable(&priv->fifo);
flexcan_chip_stop(dev);

free_irq(dev->irq, dev);
@@ -1465,7 +1331,16 @@ static int flexcan_probe(struct platform_device *pdev)
if (IS_ERR(priv->reg_xceiver))
priv->reg_xceiver = NULL;

- netif_napi_add(dev, &priv->napi, flexcan_poll, FLEXCAN_NAPI_WEIGHT);
+ /* Fill in RX-FIFO */
+ priv->fifo.low_first = FLEXCAN_RX_AREA_START;
+ priv->fifo.high_first = 32; /* FIXME: #define */
+ priv->fifo.high_last = 63; /* FIXME: #define */
+ priv->fifo.mailbox_enable_mask = flexcan_mailbox_enable_mask;
+ priv->fifo.mailbox_enable = flexcan_mailbox_enable;
+ priv->fifo.mailbox_move_to_buffer = flexcan_mailbox_move_to_buffer;
+ priv->fifo.poll_can_state = flexcan_poll_state;
+ priv->fifo.poll_bus_error = flexcan_poll_bus_err;
+ can_rx_fifo_add(dev, &priv->fifo);

platform_set_drvdata(pdev, dev);
SET_NETDEV_DEV(dev, &pdev->dev);
@@ -1494,7 +1369,7 @@ static int flexcan_remove(struct platform_device *pdev)
struct flexcan_priv *priv = netdev_priv(dev);

unregister_flexcandev(dev);
- netif_napi_del(&priv->napi);
+ can_rx_fifo_del(&priv->fifo);
free_candev(dev);

return 0;

[David Jander]

Flexcan IPs prior to V10 need to work in FIFO mode in order to be able
to receive RTR frames, so restore FIFO-mode for older IPs while still
using rx-fifo IRQ offloading.

Signed-off-by: David Jander <***@protonic.nl>
---
drivers/net/can/flexcan.c | 220 +++++++++++++++++++++++++++++++++++++---------
1 file changed, 179 insertions(+), 41 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index b8d8058..9ac89b8 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -164,12 +164,19 @@

/* FLEXCAN interrupt flag register (IFLAG) bits */
/* Errata ERR005829 step7: Reserve first valid MB */
-#define FLEXCAN_TX_BUF_RESERVED 0
-#define FLEXCAN_TX_BUF_ID 1
-#define FLEXCAN_RX_AREA_START (FLEXCAN_TX_BUF_ID + 1)
+#define FLEXCAN_V10_TX_BUF_RESERVED 0
+#define FLEXCAN_V10_TX_BUF_ID 1
+#define FLEXCAN_TX_BUF_ID 8
+#define FLEXCAN_RX_AREA_START (FLEXCAN_V10_TX_BUF_ID + 1)
#define FLEXCAN_IFLAG_BUF(x) BIT(x)
#define FLEXCAN_IFLAG1_DEFAULT (0xfffffffe)
#define FLEXCAN_IFLAG2_DEFAULT (0xffffffff)
+#define FLEXCAN_IFLAG_RX_FIFO_OVERFLOW BIT(7)
+#define FLEXCAN_IFLAG_RX_FIFO_WARN BIT(6)
+#define FLEXCAN_IFLAG_RX_FIFO_AVAILABLE BIT(5)
+#define FLEXCAN_IFLAG_DEFAULT \
+ (FLEXCAN_IFLAG_RX_FIFO_OVERFLOW | FLEXCAN_IFLAG_RX_FIFO_AVAILABLE | \
+ FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID))

/* FLEXCAN message buffers */
#define FLEXCAN_MB_CODE_MASK (0xf << 24)
@@ -286,6 +293,8 @@ struct flexcan_priv {
struct regulator *reg_xceiver;

struct can_rx_fifo fifo;
+
+ struct flexcan_mb __iomem *tx_mb;
};

static struct flexcan_devtype_data fsl_p1010_devtype_data = {
@@ -515,25 +524,27 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)

if (cf->can_dlc > 0) {
u32 data = be32_to_cpup((__be32 *)&cf->data[0]);
- flexcan_write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
+ flexcan_write(data, &priv->tx_mb->data[0]);
}
if (cf->can_dlc > 3) {
u32 data = be32_to_cpup((__be32 *)&cf->data[4]);
- flexcan_write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
+ flexcan_write(data, &priv->tx_mb->data[1]);
}

can_put_echo_skb(skb, dev, 0);

- flexcan_write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
- flexcan_write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
+ flexcan_write(can_id, &priv->tx_mb->can_id);
+ flexcan_write(ctrl, &priv->tx_mb->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);
+ if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) {
+ /* Errata ERR005829 step8:
+ * Write twice INACTIVE(0x8) code to first MB.
+ */
+ flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+ &regs->cantxfg[FLEXCAN_V10_TX_BUF_RESERVED].can_ctrl);
+ flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+ &regs->cantxfg[FLEXCAN_V10_TX_BUF_RESERVED].can_ctrl);
+ }

return NETDEV_TX_OK;
}
@@ -814,7 +825,40 @@ static unsigned int flexcan_mailbox_move_to_buffer(struct can_rx_fifo *fifo,
return 1;
}

-static irqreturn_t flexcan_irq(int irq, void *dev_id)
+static unsigned int flexcan_fifo_move_to_buffer(struct can_rx_fifo *fifo,
+ struct can_frame *cf, unsigned int n)
+{
+ struct flexcan_priv *priv = rx_fifo_to_priv(fifo);
+ struct flexcan_regs __iomem *regs = priv->base;
+ struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
+ u32 reg_ctrl, reg_id, reg_iflag1;
+
+ reg_iflag1 = flexcan_read(&regs->iflag1);
+ if (!(reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE))
+ return 0;
+
+ reg_ctrl = flexcan_read(&mb->can_ctrl);
+ reg_id = flexcan_read(&mb->can_id);
+ if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
+ cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
+ else
+ cf->can_id = (reg_id >> 18) & CAN_SFF_MASK;
+
+ if (reg_ctrl & FLEXCAN_MB_CNT_RTR)
+ cf->can_id |= CAN_RTR_FLAG;
+ cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
+
+ *(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb->data[0]));
+ *(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb->data[1]));
+
+ /* mark as read */
+ flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
+ flexcan_read(&regs->timer);
+
+ return 1;
+}
+
+static irqreturn_t flexcan_v10_irq(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
struct net_device_stats *stats = &dev->stats;
@@ -842,11 +886,63 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
}

/* reception interrupt */
- if ((reg_iflag1 & ~(1 << FLEXCAN_TX_BUF_ID)) || reg_iflag2) {
+ if ((reg_iflag1 & ~(1 << FLEXCAN_V10_TX_BUF_ID)) || reg_iflag2) {
can_rx_fifo_irq_offload(&priv->fifo);
}

/* transmission complete interrupt */
+ if (reg_iflag1 & (1 << FLEXCAN_V10_TX_BUF_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_V10_TX_BUF_ID].can_ctrl);
+ flexcan_write((1 << FLEXCAN_V10_TX_BUF_ID), &regs->iflag1);
+ netif_wake_queue(dev);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t flexcan_irq(int irq, void *dev_id)
+{
+ struct net_device *dev = dev_id;
+ struct net_device_stats *stats = &dev->stats;
+ struct flexcan_priv *priv = netdev_priv(dev);
+ struct flexcan_regs __iomem *regs = priv->base;
+ u32 reg_iflag1, reg_esr;
+
+ reg_iflag1 = flexcan_read(&regs->iflag1);
+ reg_esr = flexcan_read(&regs->esr);
+ /* ACK all bus error and state change IRQ sources */
+ if (reg_esr & FLEXCAN_ESR_ALL_INT)
+ flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
+
+ /* bus error IRQ and bus error reporting is activated */
+ if ((reg_esr & FLEXCAN_ESR_ERR_STATE) ||
+ flexcan_has_and_handle_berr(priv, reg_esr)) {
+ /*
+ * The error bits are cleared on read,
+ * save them for later use and let napi poll get them.
+ */
+ priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
+ can_rx_fifo_irq_error(&priv->fifo);
+ }
+
+ /* reception interrupt */
+ if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) {
+ can_rx_fifo_irq_offload_simple(&priv->fifo);
+ }
+
+ /* FIFO overflow */
+ if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
+ flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
+ dev->stats.rx_over_errors++;
+ dev->stats.rx_errors++;
+ }
+
+ /* transmission complete interrupt */
if (reg_iflag1 & (1 << FLEXCAN_TX_BUF_ID)) {
stats->tx_bytes += can_get_echo_skb(dev, 0);
stats->tx_packets++;
@@ -938,11 +1034,18 @@ static int flexcan_chip_start(struct net_device *dev)
*
*/
reg_mcr = flexcan_read(&regs->mcr);
- reg_mcr &= ~(FLEXCAN_MCR_MAXMB(0xff) | FLEXCAN_MCR_FEN);
+ reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
+ if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) {
+ reg_mcr &= ~FLEXCAN_MCR_FEN;
+ reg_mcr |= FLEXCAN_MCR_BCC |
+ FLEXCAN_MCR_MAXMB(FLEXCAN_MCR_MAXMB_USED);
+ } else {
+ reg_mcr |= FLEXCAN_MCR_FEN |
+ FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
+ }
reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
- FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS |
- FLEXCAN_MCR_BCC | FLEXCAN_MCR_MAXMB(FLEXCAN_MCR_MAXMB_USED);
+ FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS;
netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
flexcan_write(reg_mcr, &regs->mcr);

@@ -986,19 +1089,31 @@ static int flexcan_chip_start(struct net_device *dev)
}

/* Prepare RX mailboxes */
- for (i = FLEXCAN_RX_AREA_START; i < ARRAY_SIZE(regs->cantxfg); i++) {
- flexcan_write(FLEXCAN_MB_CODE_RX_EMPTY,
- &regs->cantxfg[i].can_ctrl);
- flexcan_write(0, &regs->rximr[i]); /* Clear filter */
+ if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) {
+ /* Prepare RX mailboxes */
+ for (i = FLEXCAN_RX_AREA_START; i < ARRAY_SIZE(regs->cantxfg);
+ i++) {
+ flexcan_write(FLEXCAN_MB_CODE_RX_EMPTY,
+ &regs->cantxfg[i].can_ctrl);
+ flexcan_write(0, &regs->rximr[i]); /* Clear filter */
+ }
+ } else {
+ /* 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);
+ }
}

/* Prepare TX mailbox */
- flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
- &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
+ flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, &priv->tx_mb->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);
+ if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) {
+ /* Errata ERR005829: mark first TX mailbox as INACTIVE */
+ flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+ &regs->cantxfg[FLEXCAN_V10_TX_BUF_RESERVED].can_ctrl);
+ }

/* acceptance mask/acceptance code (accept everything) */
flexcan_write(0x0, &regs->rxgmask);
@@ -1036,11 +1151,17 @@ static int flexcan_chip_start(struct net_device *dev)

priv->can.state = CAN_STATE_ERROR_ACTIVE;

- can_rx_fifo_reset(&priv->fifo);
+ if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES)
+ can_rx_fifo_reset(&priv->fifo);

- /* enable all MB interrupts */
- flexcan_write(FLEXCAN_IFLAG1_DEFAULT, &regs->imask1);
- flexcan_write(FLEXCAN_IFLAG2_DEFAULT, &regs->imask2);
+ if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) {
+ /* enable all MB interrupts */
+ flexcan_write(FLEXCAN_IFLAG1_DEFAULT, &regs->imask1);
+ flexcan_write(FLEXCAN_IFLAG2_DEFAULT, &regs->imask2);
+ } else {
+ /* enable FIFO interrupts */
+ flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+ }

/* synchronize with the can bus */
err = flexcan_chip_unfreeze(priv);
@@ -1090,6 +1211,7 @@ static int flexcan_open(struct net_device *dev)
{
struct flexcan_priv *priv = netdev_priv(dev);
int err;
+ irq_handler_t handler;

err = clk_prepare_enable(priv->clk_ipg);
if (err)
@@ -1103,7 +1225,12 @@ static int flexcan_open(struct net_device *dev)
if (err)
goto out_disable_per;

- err = request_irq(dev->irq, flexcan_irq, IRQF_SHARED, dev->name, dev);
+ if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES)
+ handler = flexcan_v10_irq;
+ else
+ handler = flexcan_irq;
+
+ err = request_irq(dev->irq, handler, IRQF_SHARED, dev->name, dev);
if (err)
goto out_close;

@@ -1260,6 +1387,7 @@ static int flexcan_probe(struct platform_device *pdev)
const struct flexcan_devtype_data *devtype_data;
struct net_device *dev;
struct flexcan_priv *priv;
+ struct flexcan_regs __iomem *regs;
struct resource *mem;
struct clk *clk_ipg = NULL, *clk_per = NULL;
void __iomem *base;
@@ -1320,7 +1448,11 @@ static int flexcan_probe(struct platform_device *pdev)
priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
CAN_CTRLMODE_LISTENONLY | CAN_CTRLMODE_3_SAMPLES |
CAN_CTRLMODE_BERR_REPORTING;
- priv->base = base;
+ regs = priv->base = base;
+ if (devtype_data->features & FLEXCAN_HAS_V10_FEATURES)
+ priv->tx_mb = &regs->cantxfg[FLEXCAN_V10_TX_BUF_ID];
+ else
+ priv->tx_mb = &regs->cantxfg[FLEXCAN_TX_BUF_ID];
priv->dev = dev;
priv->clk_ipg = clk_ipg;
priv->clk_per = clk_per;
@@ -1332,15 +1464,21 @@ static int flexcan_probe(struct platform_device *pdev)
priv->reg_xceiver = NULL;

/* Fill in RX-FIFO */
- priv->fifo.low_first = FLEXCAN_RX_AREA_START;
- priv->fifo.high_first = 32; /* FIXME: #define */
- priv->fifo.high_last = 63; /* FIXME: #define */
- priv->fifo.mailbox_enable_mask = flexcan_mailbox_enable_mask;
- priv->fifo.mailbox_enable = flexcan_mailbox_enable;
- priv->fifo.mailbox_move_to_buffer = flexcan_mailbox_move_to_buffer;
priv->fifo.poll_can_state = flexcan_poll_state;
priv->fifo.poll_bus_error = flexcan_poll_bus_err;
- can_rx_fifo_add(dev, &priv->fifo);
+ if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) {
+ priv->fifo.low_first = FLEXCAN_RX_AREA_START;
+ priv->fifo.high_first = 32; /* FIXME: #define */
+ priv->fifo.high_last = 63; /* FIXME: #define */
+ priv->fifo.mailbox_enable_mask = flexcan_mailbox_enable_mask;
+ priv->fifo.mailbox_enable = flexcan_mailbox_enable;
+ priv->fifo.mailbox_move_to_buffer =
+ flexcan_mailbox_move_to_buffer;
+ can_rx_fifo_add(dev, &priv->fifo);
+ } else {
+ priv->fifo.mailbox_move_to_buffer = flexcan_fifo_move_to_buffer;
+ can_rx_fifo_add_simple(dev, &priv->fifo);
+ }

platform_set_drvdata(pdev, dev);
SET_NETDEV_DEV(dev, &pdev->dev);