[PATCH V4 00/14] CAN: Add rx-fifo support and port flexcan to it

[PATCH V4 00/14] CAN: Add rx-fifo support and port flexcan to it

[David Jander]

This is the whole patch series again, adding the original rx-fifo framework
from Marc Kleine-Budde as well as the morphing into IRQ-offload mode and a
sample implementation using the flexcan driver.

The new flexcan driver supports full fifo abstraction mode for V10 and newer
peripherals, and simple irq-offloading for older parts.

Changes from V3:
 - Moved rx-fifo patches to beginning of series
 - Fixed accessing cf->can_dlc after buffer had been freed in
   can_rx_fifo_read_napi_frame()
 - Call can_led_event() only once per NAPI invocation.
 - Fixed missing { } on else branch in can_rx_fifo_add().
 - Documented ring buffer and overflow mailbox mechanism.
 - Remove NULL check before calling kfree() in can_rx_fifo_del().
 - Renamed second_first to scan_high_first.
 - Documented struct can_rx_fifo.
 - Replaced the BUG() in can_rx_fifo_irq_offload() with WARN().
 - Documented the difference between poll_can_state() and poll_bus_error().
 - Enabled support for leaving poll_*() handlers NULL if not needed.
 - Squashed two flexcan patches to avoid having a half-broken driver in
   between.
 - Consolidated can_rx_fifo_add_simple() and can_rx_fifo_irq_offload_simple()
   with a flag in struct can_rx_fifo, removing the need for those two
   functions.

David Jander (10):
  can: rx-fifo: Increase MB size limit from 32 to 64
  can: rx-fifo: Change to do controller off-load in interrupt and NAPI
    poll
  can: rx-fifo: fix long lines
  can: rx-fifo: Add can_rx_fifo_reset() function
  can: rx-fifo: remove obsolete comment
  can: rx-fifo: Add support for can state tracking and error polling
  can: rx-fifo: Add support for simple irq offloading
  can: rx-fifo: Add documentation to struct can_rx_fifo.
  can: flexcan: Add support for RX-FIFO
  can: flexcan: Add MB/Fifo specific column to comment table of IP
    versions

Marc Kleine-Budde (4):
  can: dev: add preliminary rx-fifo
  can: flexcan: add documentation about mailbox organizaiton
  can: flexcan: rename crl2 -> ctrl2
  can: flexcan: replace open coded mailbox code by proper defines

 drivers/net/can/dev.c     | 329 +++++++++++++++++++++++++++++++++
 drivers/net/can/flexcan.c | 452 ++++++++++++++++++++++++++++++++--------------
 include/linux/can/dev.h   |  90 +++++++++
 3 files changed, 736 insertions(+), 135 deletions(-)

-- 
1.9.1

[PATCH 01/14] can: dev: add preliminary rx-fifo

[David Jander]

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

Signed-off-by: Marc Kleine-Budde <mkl@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);
 
-- 
1.9.1

[PATCH 02/14] can: rx-fifo: Increase MB size limit from 32 to 64

[David Jander]

Signed-off-by: David Jander <david@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);
-- 
1.9.1

[PATCH 03/14] can: rx-fifo: Change to do controller off-load in interrupt and NAPI poll

[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 <david@protonic.nl>
---
 drivers/net/can/dev.c   | 213 +++++++++++++++++++++++++++++++++++++-----------
 include/linux/can/dev.h |  29 +++++--
 2 files changed, 188 insertions(+), 54 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 930b9f4..9b17592 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,101 @@ 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));
+
+	stats->rx_packets++;
+	stats->rx_bytes += cf->can_dlc;
+
+	netif_receive_skb(skb);
+
+	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;
+	}
+
+	can_led_event(fifo->dev, CAN_LED_EVENT_RX);
+
+	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->scan_high_first = false;
 	fifo->active = fifo->mask_low | fifo->mask_high;
 	fifo->mailbox_enable_mask(fifo, fifo->active);
 
@@ -338,60 +421,94 @@ 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->scan_high_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->high_first)
+		netdev_warn(fifo->dev, "%s: RX order cannot be guaranteed."
+			" (count=%d)\n", __func__, i);
 
-		fifo->mailbox_receive(fifo, mb);
+	fifo->scan_high_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->scan_high_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;
+	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..66b0228 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -71,18 +71,33 @@ 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 scan_high_first;
 
 	bool inc;
+
+	/* CAN frame ring buffer. Will be allocated to an appropriate size */
+	struct can_frame *ring;
+
+	/*
+	 * Overflow buffer: This will work sort of as /dev/null if the ring-
+	 * buffer is full. We don't want to bother the user with taking care
+	 * of that situation, so we just pass it the overflow buffer instead.
+	 */
+	struct can_frame overflow;
+
+	size_t ring_size;
+	unsigned int ring_head;
+	unsigned int ring_tail;
+	struct napi_struct napi;
 };
 
 /*
@@ -127,8 +142,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);
-- 
1.9.1

[PATCH 04/14] can: rx-fifo: fix long lines

[David Jander]

Signed-off-by: David Jander <david@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 9b17592..3e32ec3 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)
-- 
1.9.1

[PATCH 05/14] can: rx-fifo: Add can_rx_fifo_reset() function

[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 WARN() to the user if he forgot.

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

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 3e32ec3..186ce54 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -488,8 +488,18 @@ int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo)
 		fifo->scan_high_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. WARN() the user and try
+		 * to recover. This may fail and the system may hang though.
+		 */
+		WARN(true, "%s: No messages found,"
+			    " RX-FIFO out of sync?\n", __func__);
+		can_rx_fifo_reset(fifo);
+	}
 
 	return received;
 }
@@ -507,6 +517,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->scan_high_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)
 {
 	kfree(fifo->ring);
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 66b0228..334626c 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -145,6 +145,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);
-- 
1.9.1

[PATCH 06/14] can: rx-fifo: remove obsolete comment

[David Jander]

Signed-off-by: David Jander <david@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 334626c..24a88f4 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);
-- 
1.9.1

[PATCH 07/14] can: rx-fifo: Add support for can state tracking and error polling

[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. Both handlers can be NULL if they are not needed.
poll_bus_error() should handle the stored bus-error flags and generate
error frames as needed, while poll_can_state() should handle the CAN-bus
state and generate state-change error-frames if needed.

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

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 186ce54..ffa4f60 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -350,6 +350,9 @@ static int can_rx_fifo_napi_poll(struct napi_struct *napi, int quota)
 	unsigned int tail;
 
 restart_poll:
+	if ((work_done < quota) && fifo->poll_can_state)
+		work_done += fifo->poll_can_state(fifo);
+
 	/* handle mailboxes */
 	head = smp_load_acquire(&fifo->ring_head);
 	tail = fifo->ring_tail;
@@ -361,14 +364,19 @@ restart_poll:
 		smp_store_release(&fifo->ring_tail, tail);
 	}
 
+	if ((work_done < quota) && fifo->poll_bus_error)
+		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;
+		}
 	}
 
 	can_led_event(fifo->dev, CAN_LED_EVENT_RX);
@@ -413,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->scan_high_first = false;
+	fifo->poll_errors = false;
 	fifo->active = fifo->mask_low | fifo->mask_high;
 	fifo->mailbox_enable_mask(fifo, fifo->active);
 
@@ -505,6 +514,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 24a88f4..a44108d 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 scan_high_first;
+	bool poll_errors;
 
 	bool inc;
 
@@ -100,6 +103,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.
@@ -143,6 +151,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);
-- 
1.9.1

[PATCH 08/14] can: rx-fifo: Add support for simple irq offloading

[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 <david@protonic.nl>
---
 drivers/net/can/dev.c   | 79 +++++++++++++++++++++++++++++++++++++++++--------
 include/linux/can/dev.h |  2 ++
 2 files changed, 69 insertions(+), 12 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index ffa4f60..2741d42 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -384,10 +384,54 @@ 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;
+}
+
+static 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;
+}
+
 int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)
 {
 	unsigned int weight;
-	fifo->dev = dev;
+	int ret;
+
+	if (fifo->simple_offload)
+		return can_rx_fifo_add_simple(dev, fifo);
 
 	if ((fifo->low_first < fifo->high_first) &&
 	    (fifo->high_first < fifo->high_last)) {
@@ -405,17 +449,9 @@ int can_rx_fifo_add(struct net_device *dev, struct can_rx_fifo *fifo)
 	    !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);
+	ret = can_rx_fifo_init_ring(dev, fifo, weight);
+	if (ret)
+		return ret;
 
 	/* init variables */
 	fifo->mask_low = can_rx_fifo_mask_low(fifo);
@@ -453,12 +489,31 @@ static unsigned int can_rx_fifo_offload_if_full(struct can_rx_fifo *fifo, unsign
 	return ret;
 }
 
+static 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;
+}
+
 int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo)
 {
 	unsigned int i;
 	unsigned int ret;
 	unsigned int received = 0;
 
+	if (fifo->simple_offload)
+		return can_rx_fifo_irq_offload_simple(fifo);
+
 	if (fifo->scan_high_first) {
 		for (i = fifo->high_first;
 		     can_rx_fifo_le(fifo, i, fifo->high_last);
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index a44108d..c8b8519 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -71,6 +71,8 @@ struct can_rx_fifo {
 	unsigned int high_first;
 	unsigned int high_last;
 
+	bool simple_offload;
+
 	void (*mailbox_enable_mask)(struct can_rx_fifo *rx_fifo, u64 mask);
 	void (*mailbox_enable)(struct can_rx_fifo *rx_fifo, unsigned int mb);
 	unsigned int (*mailbox_move_to_buffer)(struct can_rx_fifo *rx_fifo,
-- 
1.9.1

[PATCH 09/14] can: rx-fifo: Add documentation to struct can_rx_fifo.

[David Jander]

Signed-off-by: David Jander <david@protonic.nl>
---
 include/linux/can/dev.h | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index c8b8519..10e0a16 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -64,6 +64,42 @@ struct can_priv {
 #endif
 };
 
+/*
+ * CAN rx-fifo struct:
+ *
+ * User-provided parameters (when calling can_rx_fifo_add()):
+ *
+ *	@low_first:	Number of message-box (MB) that gets filled first by
+ *			CAN controller.
+ *	@high_first:	Number of the MB that gets filled first if the lower
+ *			half is disabled (first MB of the second half).
+ *	@high_last:	Number of the MB that gets filled last by the CAN-
+ *			controller
+ *	@mailbox_enable_mask:
+ *			Enable the corresponding MB for each bit set in 'mask'.
+ *	@mailbox_enable:
+ *			Enable MB number 'mb'.
+ *	@mailbox_move_to_buffer:
+ *			This function should atomically copy one message to the
+ *			provided buffer and lock the MB, so that no more
+ *			messages can be received. This function must also take
+ *			care of clearing interrupt-flags if necessary.
+ *	@poll_can_state:
+ *			This function should take care of bus state tracking.
+ *			It is called by	the NAPI poll handler, before
+ *			mailbox_move_to_buffer(). It should handle and generate
+ *			bus-state error-frames such as error-warning and bus-off.
+ *			Set this to NULL if not needed. Returns 1 if an error-
+ *			frame was generated, 0 otherwise.
+ *	@poll_bus_errors:
+ *			This function should check for (bus)-errors and
+ *			generate the corresponding error frames. It is called
+ *			by the NAPI poll handler, after mailbox_move_to_buffer().
+ *			It should typically check for and generate bit-, frame-,
+ *			stuff-,	crc- and ack error-frames.
+ *			Set this to NULL if not needed. Returns 1 if an error-
+ *			frame was generated, 0 otherwise.
+ */
 struct can_rx_fifo {
 	struct net_device *dev;
 
-- 
1.9.1

[PATCH 10/14] can: flexcan: add documentation about mailbox organizaiton

[David Jander]

From: Marc Kleine-Budde <mkl@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 <mkl@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 */
-- 
1.9.1

[PATCH 11/14] can: flexcan: rename crl2 -> ctrl2

[David Jander]

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

Signed-off-by: Marc Kleine-Budde <mkl@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;
-- 
1.9.1

[PATCH 12/14] can: flexcan: replace open coded mailbox code by proper defines

[David Jander]

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

Signed-off-by: Marc Kleine-Budde <mkl@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;
-- 
1.9.1

[PATCH 13/14] can: flexcan: Add support for RX-FIFO

[David Jander]

Flexcan controllers have a hardware fifo, but that fifo is very small. On top
of that, it is read out from the NAPI-handler (soft IRQ). If we receive back-
to-back CAN frames at high baud-rate, the permissible latency from hard-IRQ to
NAPI is only 6 messages (the depth of the hardware FIFO).

Adding support for RX-FIFO (see can/dev.c) to this driver, improves both of
these aspects:

1. For V10 or newer flexcan IPs the whole 64 MB area is used as a virtual
   RX-FIFO.

2. RX-FIFO takes care of off-loading the flexcan controller directly from
   (hard-) interrupt into the RX-FIFO ring-buffer.

Unfortunately, flexcan IP cores older than V10 (V3.x) do not support receiving
RTR frames in a message-box. Therefor we cannot use the whole 64 MB area for
RX-FIFO in this case, but we still benefit a little from the faster
off-loading and NAPI handling from the RX-FIFO code.

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

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 20f5c0e..304b164 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 <avolkov@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,9 +164,13 @@
 
 /* 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_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)
@@ -158,6 +179,8 @@
 	 FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID))
 
 /* 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 +198,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 +260,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 */
@@ -257,10 +280,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;
@@ -268,6 +291,10 @@ struct flexcan_priv {
 	struct flexcan_platform_data *pdata;
 	const struct flexcan_devtype_data *devtype_data;
 	struct regulator *reg_xceiver;
+
+	struct can_rx_fifo fifo;
+
+	struct flexcan_mb __iomem *tx_mb;
 };
 
 static struct flexcan_devtype_data fsl_p1010_devtype_data = {
@@ -497,29 +524,36 @@ 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;
 }
 
+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)
 {
@@ -568,16 +602,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++;
@@ -659,17 +698,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
@@ -697,15 +741,66 @@ 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 void flexcan_mailbox_enable(struct can_rx_fifo *fifo, unsigned int n)
 {
-	const struct flexcan_priv *priv = netdev_priv(dev);
+	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;
+	struct flexcan_mb __iomem *mb;
+	u32 reg_ctrl;
+	u32 code;
 
+	mb = &regs->cantxfg[n];
 	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_read(&regs->timer);
+}
+
+static void flexcan_mailbox_enable_mask(struct can_rx_fifo *fifo, u64 mask)
+{
+	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;
+	unsigned int n;
+
+	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);
+	}
+	flexcan_read(&regs->timer);
+}
+
+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 *mb = &regs->cantxfg[n];
+	u32 reg_ctrl, reg_id;
+	u32 code;
+
+	do {
+		reg_ctrl = flexcan_read(&mb->can_ctrl);
+		code = reg_ctrl & FLEXCAN_MB_CODE_MASK;
+	} while (code & FLEXCAN_MB_CODE_RX_BUSY_BIT);
+
+	/* 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++;
+	}
+
 	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;
@@ -719,71 +814,95 @@ static void flexcan_read_fifo(const struct net_device *dev,
 	*(__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);
+	/* Clear IRQ and lock MB */
+	if (n < 32)
+		flexcan_write(BIT(n), &regs->iflag1);
+	else
+		flexcan_write(BIT(n - 32), &regs->iflag2);
+
+	flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE, &mb->can_ctrl);
+
+	return 1;
 }
 
-static int flexcan_read_frame(struct net_device *dev)
+static unsigned int flexcan_fifo_move_to_buffer(struct can_rx_fifo *fifo,
+		struct can_frame *cf, unsigned int n)
 {
-	struct net_device_stats *stats = &dev->stats;
-	struct can_frame *cf;
-	struct sk_buff *skb;
+	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;
 
-	skb = alloc_can_skb(dev, &cf);
-	if (unlikely(!skb)) {
-		stats->rx_dropped++;
+	reg_iflag1 = flexcan_read(&regs->iflag1);
+	if (!(reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE))
 		return 0;
-	}
 
-	flexcan_read_fifo(dev, cf);
-	netif_receive_skb(skb);
+	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);
 
-	stats->rx_packets++;
-	stats->rx_bytes += cf->can_dlc;
+	*(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb->data[0]));
+	*(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb->data[1]));
 
-	can_led_event(dev, CAN_LED_EVENT_RX);
+	/* mark as read */
+	flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
+	flexcan_read(&regs->timer);
 
 	return 1;
 }
 
-static int flexcan_poll(struct napi_struct *napi, int quota)
+static irqreturn_t flexcan_v10_irq(int irq, void *dev_id)
 {
-	struct net_device *dev = napi->dev;
-	const struct flexcan_priv *priv = netdev_priv(dev);
+	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;
-	int work_done = 0;
+	u32 reg_iflag1, reg_iflag2, reg_esr;
 
-	/*
-	 * The error bits are cleared on read,
-	 * use saved value from irq handler.
-	 */
-	reg_esr = flexcan_read(&regs->esr) | priv->reg_esr;
+	reg_iflag1 = flexcan_read(&regs->iflag1);
+	reg_iflag2 = flexcan_read(&regs->iflag2);
+	reg_esr = flexcan_read(&regs->esr);
 
-	/* handle state changes */
-	work_done += flexcan_poll_state(dev, reg_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);
 
-	/* 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);
+	/* 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);
 	}
 
-	/* report bus errors */
-	if (flexcan_has_and_handle_berr(priv, reg_esr) && work_done < quota)
-		work_done += flexcan_poll_bus_err(dev, reg_esr);
+	/* reception interrupt */
+	if ((reg_iflag1 & ~(1 << FLEXCAN_V10_TX_BUF_ID)) || reg_iflag2) {
+		can_rx_fifo_irq_offload(&priv->fifo);
+	}
 
-	if (work_done < quota) {
-		napi_complete(napi);
-		/* enable IRQs */
-		flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
-		flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
+	/* 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 work_done;
+	return IRQ_HANDLED;
 }
 
 static irqreturn_t flexcan_irq(int irq, void *dev_id)
@@ -800,25 +919,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 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
-	    (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_write(FLEXCAN_IFLAG_DEFAULT &
-			~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
-		flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
-		       &regs->ctrl);
-		napi_schedule(&priv->napi);
+		can_rx_fifo_irq_error(&priv->fifo);
+	}
+
+	/* reception interrupt */
+	if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) {
+		can_rx_fifo_irq_offload(&priv->fifo);
 	}
 
 	/* FIFO overflow */
@@ -921,10 +1035,17 @@ 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 |
+	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_MAXMB(FLEXCAN_TX_BUF_ID);
+		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);
 
@@ -960,28 +1081,45 @@ 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,
+	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 */
+	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);
+		}
 	}
 
-	/* Errata ERR005829: mark first TX mailbox as INACTIVE */
-	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
-		      &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
+	/* Prepare TX mailbox */
+	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, &priv->tx_mb->can_ctrl);
 
-	/* mark TX mailbox as INACTIVE */
-	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
-		      &regs->cantxfg[FLEXCAN_TX_BUF_ID].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);
 	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.
@@ -1011,16 +1149,25 @@ static int flexcan_chip_start(struct net_device *dev)
 	if (err)
 		goto out_chip_disable;
 
+	priv->can.state = CAN_STATE_ERROR_ACTIVE;
+
+	if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES)
+		can_rx_fifo_reset(&priv->fifo);
+
+	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);
 	if (err)
 		goto out_transceiver_disable;
 
-	priv->can.state = CAN_STATE_ERROR_ACTIVE;
-
-	/* enable FIFO interrupts */
-	flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
-
 	/* print chip status */
 	netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__,
 		   flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
@@ -1064,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)
@@ -1077,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;
 
@@ -1088,7 +1241,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;
@@ -1110,7 +1263,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);
@@ -1234,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;
@@ -1294,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;
@@ -1305,7 +1463,22 @@ 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.poll_can_state = flexcan_poll_state;
+	priv->fifo.poll_bus_error = flexcan_poll_bus_err;
+	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;
+	} else {
+		priv->fifo.simple_offload = true;
+		priv->fifo.mailbox_move_to_buffer = flexcan_fifo_move_to_buffer;
+	}
+	can_rx_fifo_add(dev, &priv->fifo);
 
 	platform_set_drvdata(pdev, dev);
 	SET_NETDEV_DEV(dev, &pdev->dev);
@@ -1334,7 +1507,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;
-- 
1.9.1

[PATCH 14/14] can: flexcan: Add MB/Fifo specific column to comment table of IP versions

[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 <david@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 304b164..717ccb3 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -204,14 +204,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.
  */
-- 
1.9.1

Re: [PATCH 02/14] can: rx-fifo: Increase MB size limit from 32 to 64

[Marc Kleine-Budde]

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

On 11/06/2014 09:34 AM, David Jander wrote:
> Signed-off-by: David Jander <david@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);

Yesterday I stumbled over GENMASK_ULL
http://lxr.free-electrons.com/source/include/linux/bitops.h#L22
Does it make sense to use it here?

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 02/14] can: rx-fifo: Increase MB size limit from 32 to 64

[David Jander]

On Thu, 06 Nov 2014 09:41:59 +0100
Marc Kleine-Budde <mkl@pengutronix.de> wrote:

> On 11/06/2014 09:34 AM, David Jander wrote:
> > Signed-off-by: David Jander <david@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);
> 
> Yesterday I stumbled over GENMASK_ULL
> http://lxr.free-electrons.com/source/include/linux/bitops.h#L22
> Does it make sense to use it here?

Looks like its exactly what is needed here.... nice.

Btw, there is a subtle bug in the line you linked to:

> #define GENMASK_ULL(h, l)       (((U64_C(1) << ((h) - (l) + 1)) - 1) << (l))

The _ULL in the name of the macro seems to hint to "unsigned long long", while
the implementation seems to assume that it means U64_C. This is obviously
wrong, although for most platforms that are currently capable of running
Linux, those are probably equivalent...

Funny.

Best regards,

-- 
David Jander
Protonic Holland.

Re: [PATCH 02/14] can: rx-fifo: Increase MB size limit from 32 to 64

[Marc Kleine-Budde]

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

On 11/06/2014 05:03 PM, David Jander wrote:
> On Thu, 06 Nov 2014 09:41:59 +0100
> Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> 
>> On 11/06/2014 09:34 AM, David Jander wrote:
>>> Signed-off-by: David Jander <david@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);
>>
>> Yesterday I stumbled over GENMASK_ULL
>> http://lxr.free-electrons.com/source/include/linux/bitops.h#L22
>> Does it make sense to use it here?
> 
> Looks like its exactly what is needed here.... nice.
> 
> Btw, there is a subtle bug in the line you linked to:
> 
>> #define GENMASK_ULL(h, l)       (((U64_C(1) << ((h) - (l) + 1)) - 1) << (l))
> 
> The _ULL in the name of the macro seems to hint to "unsigned long long", while
> the implementation seems to assume that it means U64_C. This is obviously
> wrong, although for most platforms that are currently capable of running
> Linux, those are probably equivalent...

#define U64_C(x) x ## ULL

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 02/14] can: rx-fifo: Increase MB size limit from 32 to 64

[David Jander]

On Thu, 06 Nov 2014 17:05:18 +0100
Marc Kleine-Budde <mkl@pengutronix.de> wrote:

> On 11/06/2014 05:03 PM, David Jander wrote:
> > On Thu, 06 Nov 2014 09:41:59 +0100
> > Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> > 
> >> On 11/06/2014 09:34 AM, David Jander wrote:
> >>> Signed-off-by: David Jander <david@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);
> >>
> >> Yesterday I stumbled over GENMASK_ULL
> >> http://lxr.free-electrons.com/source/include/linux/bitops.h#L22
> >> Does it make sense to use it here?
> > 
> > Looks like its exactly what is needed here.... nice.
> > 
> > Btw, there is a subtle bug in the line you linked to:
> > 
> >> #define GENMASK_ULL(h, l)       (((U64_C(1) << ((h) - (l) + 1)) - 1) <<
> >> (l))
> > 
> > The _ULL in the name of the macro seems to hint to "unsigned long long",
> > while the implementation seems to assume that it means U64_C. This is
> > obviously wrong, although for most platforms that are currently capable of
> > running Linux, those are probably equivalent...
> 
> #define U64_C(x) x ## ULL

This line:

http://lxr.free-electrons.com/source/include/asm-generic/int-ll64.h#L4

...seems to indicate, that it may not always be the case :-)
AFAIK, C99 specifies that "long long" should at _least_ have 64 bit length,
but says nothing about when it may be larger.
OTOH, I have yet to find a machine/compiler combination that defines "long
long" as more than 64bit... at least not running Linux.
A correct definition should be trivial though:

#define GENMASK_ULL(h, l)       (((1ULL << ((h) - (l) + 1)) - 1) << (l))

Best regards,

-- 
David Jander
Protonic Holland.

Re: [PATCH 02/14] can: rx-fifo: Increase MB size limit from 32 to 64

[Marc Kleine-Budde]

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

On 11/06/2014 05:20 PM, David Jander wrote:
>>> Btw, there is a subtle bug in the line you linked to:
>>>
>>>> #define GENMASK_ULL(h, l)       (((U64_C(1) << ((h) - (l) + 1)) - 1) <<
>>>> (l))
>>>
>>> The _ULL in the name of the macro seems to hint to "unsigned long long",
>>> while the implementation seems to assume that it means U64_C. This is
>>> obviously wrong, although for most platforms that are currently capable of
>>> running Linux, those are probably equivalent...
>>
>> #define U64_C(x) x ## ULL
> 
> This line:
> 
> http://lxr.free-electrons.com/source/include/asm-generic/int-ll64.h#L4
> 
> ...seems to indicate, that it may not always be the case :-)
> AFAIK, C99 specifies that "long long" should at _least_ have 64 bit length,
> but says nothing about when it may be larger.
> OTOH, I have yet to find a machine/compiler combination that defines "long
> long" as more than 64bit... at least not running Linux.
> A correct definition should be trivial though:
> 
> #define GENMASK_ULL(h, l)       (((1ULL << ((h) - (l) + 1)) - 1) << (l))

That's what it does:

$ cat << EOF | gcc -xc -E -
#define U64_C(x) x ## ULL
#define GENMASK_ULL(h, l)   (((U64_C(1) << ((h) - (l) + 1)) - 1) << (l))
GENMASK_ULL(foo, bar)
EOF

Results in:
(((1ULL << ((foo) - (bar) + 1)) - 1) << (bar))

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 02/14] can: rx-fifo: Increase MB size limit from 32 to 64

[David Jander]

On Fri, 07 Nov 2014 09:40:36 +0100
Marc Kleine-Budde <mkl@pengutronix.de> wrote:

> On 11/06/2014 05:20 PM, David Jander wrote:
> >>> Btw, there is a subtle bug in the line you linked to:
> >>>
> >>>> #define GENMASK_ULL(h, l)       (((U64_C(1) << ((h) - (l) + 1)) - 1) <<
> >>>> (l))
> >>>
> >>> The _ULL in the name of the macro seems to hint to "unsigned long long",
> >>> while the implementation seems to assume that it means U64_C. This is
> >>> obviously wrong, although for most platforms that are currently capable
> >>> of running Linux, those are probably equivalent...
> >>
> >> #define U64_C(x) x ## ULL
> > 
> > This line:
> > 
> > http://lxr.free-electrons.com/source/include/asm-generic/int-ll64.h#L4
> > 
> > ...seems to indicate, that it may not always be the case :-)
> > AFAIK, C99 specifies that "long long" should at _least_ have 64 bit length,
> > but says nothing about when it may be larger.
> > OTOH, I have yet to find a machine/compiler combination that defines "long
> > long" as more than 64bit... at least not running Linux.
> > A correct definition should be trivial though:
> > 
> > #define GENMASK_ULL(h, l)       (((1ULL << ((h) - (l) + 1)) - 1) << (l))
> 
> That's what it does:
> 
> $ cat << EOF | gcc -xc -E -
> #define U64_C(x) x ## ULL
> #define GENMASK_ULL(h, l)   (((U64_C(1) << ((h) - (l) + 1)) - 1) << (l))
> GENMASK_ULL(foo, bar)
> EOF
> 
> Results in:
> (((1ULL << ((foo) - (bar) + 1)) - 1) << (bar))

Yes of course! I am not questioning that.
The problem is that although right now asm-generic/int-ll64.h is the only
place where U64_C is defined, this may not be the case in the future. In the
same way as in C99 an "unsigned long" can be either 32bit or 64bit wide, there
is also no guarantee that an "unsigned long long" will always be 64bits. If
tomorrow we add support for another arch in Linux that happens to define
"unsigned long long" as 128bit for example, on that platform we will have
another version arch/foo/include/asm/int-ll64.h that defines...

#define U64_C(x) x ## UL

On that platform one would assume that something called "GENMASK_ULL" is
supposed to support 128bit masks, which is not true because the definition is
wrong. Of course you can say that in that case one should also re-define
GENMASK_ULL, but why do that when fixing the current definition (replace
U64_C(1) with 1ULL) would always be correct? The point is, that in "C" mixing
type definitions between C-types (int, long, long long, char, etc...) with
fixed-size types (int32_t, int64_t, int8_t, etc...) is semantically wrong,
platform-dependent and should be avoided. That's what stdint.h is for in
user-land.
Btw, on some 16-bit DSP processors, the C-type "char" is 16-bit wide... how
many issues do you think this can cause if one is not careful with type
naming? Not that I intend to run Linux on such a DSP, but you get the idea...

Now, can we please get back on-topic ;-)

I am going to use this macro (although it is semantically slightly broken) in
the next version.

Best regards,

-- 
David Jander
Protonic Holland.

Re: [PATCH 03/14] can: rx-fifo: Change to do controller off-load in interrupt and NAPI poll

[David Jander]

On Thu,  6 Nov 2014 09:34:02 +0100
David Jander <david@protonic.nl> wrote:

> 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 <david@protonic.nl>
> ---
>  drivers/net/can/dev.c   | 213
> +++++++++++++++++++++++++++++++++++++----------- include/linux/can/dev.h |
> 29 +++++-- 2 files changed, 188 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
> index 930b9f4..9b17592 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,101 @@ 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));
> +
> +	stats->rx_packets++;
> +	stats->rx_bytes += cf->can_dlc;
> +
> +	netif_receive_skb(skb);
> +
> +	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;
> +	}
> +
> +	can_led_event(fifo->dev, CAN_LED_EVENT_RX);
> +
> +	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->scan_high_first = false;
>  	fifo->active = fifo->mask_low | fifo->mask_high;
>  	fifo->mailbox_enable_mask(fifo, fifo->active);
>  
> @@ -338,60 +421,94 @@ 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->scan_high_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->high_first)

Arrgh! There is a typo in this line, introduced while constantly renaming the
fifo->scan_high_first variable :-(

This line should read:

+	if (can_rx_fifo_ge(fifo, i, fifo->high_first) && fifo->scan_high_first)

This bug just causes a lot of unnecessary dmesg pollution, sorry for that!

> +		netdev_warn(fifo->dev, "%s: RX order cannot be guaranteed."
> +			" (count=%d)\n", __func__, i);
>  
> -		fifo->mailbox_receive(fifo, mb);
> +	fifo->scan_high_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->scan_high_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;
> +	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..66b0228 100644
> --- a/include/linux/can/dev.h
> +++ b/include/linux/can/dev.h
> @@ -71,18 +71,33 @@ 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 scan_high_first;
>  
>  	bool inc;
> +
> +	/* CAN frame ring buffer. Will be allocated to an appropriate size
> */
> +	struct can_frame *ring;
> +
> +	/*
> +	 * Overflow buffer: This will work sort of as /dev/null if the ring-
> +	 * buffer is full. We don't want to bother the user with taking care
> +	 * of that situation, so we just pass it the overflow buffer
> instead.
> +	 */
> +	struct can_frame overflow;
> +
> +	size_t ring_size;
> +	unsigned int ring_head;
> +	unsigned int ring_tail;
> +	struct napi_struct napi;
>  };
>  
>  /*
> @@ -127,8 +142,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);

Best regards,

-- 
David Jander
Protonic Holland.