[PATCH v2 00/12] can: rx-offload: add implmentation and switch flexcan driver to use it

[PATCH v2 00/12] can: rx-offload: add implmentation and switch flexcan driver to use it

[Marc Kleine-Budde]

Hello,

this patch takes up the idea to read the CAN frames in IRQ context and send
them later in NAPI. The first two patches add each an offloading scheme.

The first one is for hardware FIFO based cores, like the flexcan in FIFO mode.
The second one requires mailboxes with timestamps. The mailboxes are read and
sorted by timestamp in IRQ context, sending is done later in NAPI aswell.

The remaining patches modify the flexcan driver to make use of it. imx6 and
vf610 SoCs can make use of the 64 mailbox software FIFO, while older SoCs still
use flexcan's 6 mailbox deep hardware FIFO.

Testing on any flexcan core is highly appreciated.

regards,
Marc

[PATCH v2 01/12] can: rx-offload: Add support for HW fifo based irq offloading

[Marc Kleine-Budde]

From: David Jander <david@protonic.nl>

Some CAN controllers have a usable FIFO already but can still benefit
from off-loading the CAN controller FIFO. The CAN frames of the FIFO are
read and put into a skb queue during interrupt and then transmitted in a
NAPI context.

Signed-off-by: David Jander <david@protonic.nl>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/Makefile       |   3 +-
 drivers/net/can/rx-offload.c   | 163 +++++++++++++++++++++++++++++++++++++++++
 include/linux/can/rx-offload.h |  56 ++++++++++++++
 3 files changed, 221 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/can/rx-offload.c
 create mode 100644 include/linux/can/rx-offload.h

diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 26ba4b794a0b..04621f202352 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -6,7 +6,8 @@ obj-$(CONFIG_CAN_VCAN)		+= vcan.o
 obj-$(CONFIG_CAN_SLCAN)		+= slcan.o
 
 obj-$(CONFIG_CAN_DEV)		+= can-dev.o
-can-dev-y			:= dev.o
+can-dev-y			+= dev.o
+can-dev-y			+= rx-offload.o
 
 can-dev-$(CONFIG_CAN_LEDS)	+= led.o
 
diff --git a/drivers/net/can/rx-offload.c b/drivers/net/can/rx-offload.c
new file mode 100644
index 000000000000..db360ddeec97
--- /dev/null
+++ b/drivers/net/can/rx-offload.c
@@ -0,0 +1,163 @@
+/*
+ * Copyright (c) 2014 David Jander, Protonic Holland
+ * Copyright (C) 2014-2016 Pengutronix, Marc Kleine-Budde <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the version 2 of the GNU General Public License
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/can/dev.h>
+#include <linux/can/rx-offload.h>
+
+static int can_rx_offload_napi_poll(struct napi_struct *napi, int quota)
+{
+	struct can_rx_offload *offload = container_of(napi, struct can_rx_offload, napi);
+	struct net_device *dev = offload->dev;
+	struct net_device_stats *stats = &dev->stats;
+	struct sk_buff *skb;
+	int work_done = 0;
+
+	if (offload->poll_pre_read && work_done < quota)
+		work_done += offload->poll_pre_read(offload);
+
+	while ((work_done < quota) &&
+	       (skb = skb_dequeue(&offload->skb_queue))) {
+		struct can_frame *cf = (struct can_frame *)skb->data;
+
+		work_done++;
+		stats->rx_packets++;
+		stats->rx_bytes += cf->can_dlc;
+		netif_receive_skb(skb);
+	}
+
+	if (offload->poll_post_read && work_done < quota)
+		work_done += offload->poll_post_read(offload);
+
+	if (work_done < quota) {
+		napi_complete(napi);
+
+		/* Check if there was another interrupt */
+		if ((!skb_queue_empty(&offload->skb_queue) || offload->poll_errors) &&
+		    napi_reschedule(&offload->napi)) {
+			offload->poll_errors = false;
+		}
+
+		if (offload->poll_error_interrupts_enable)
+			offload->poll_error_interrupts_enable(offload);
+	}
+
+	can_led_event(offload->dev, CAN_LED_EVENT_RX);
+
+	return work_done;
+}
+
+static struct sk_buff *can_rx_offload_offload_one(struct can_rx_offload *offload, unsigned int n)
+{
+	struct sk_buff *skb = NULL;
+	struct can_frame *cf;
+	int ret;
+
+	/* If queue is full or skb not available, read to discard mailbox */
+	if (likely(skb_queue_len(&offload->skb_queue) <=
+		   offload->skb_queue_len_max))
+		skb = alloc_can_skb(offload->dev, &cf);
+
+	if (!skb) {
+		struct can_frame cf_overflow;
+
+		ret = offload->mailbox_read(offload, &cf_overflow, n);
+		if (ret)
+			offload->dev->stats.rx_dropped++;
+
+		return NULL;
+	}
+
+	ret = offload->mailbox_read(offload, cf, n);
+	if (!ret) {
+		kfree_skb(skb);
+		return NULL;
+	}
+
+	return skb;
+}
+
+int can_rx_offload_irq_offload_fifo(struct can_rx_offload *offload)
+{
+	struct sk_buff *skb;
+	int received = 0;
+
+	while ((skb = can_rx_offload_offload_one(offload, 0))) {
+		skb_queue_tail(&offload->skb_queue, skb);
+		received++;
+	}
+
+	if (received)
+		can_rx_offload_schedule(offload);
+
+	return received;
+}
+EXPORT_SYMBOL_GPL(can_rx_offload_irq_offload_fifo);
+
+static int can_rx_offload_init_queue(struct net_device *dev, struct can_rx_offload *offload, unsigned int weight)
+{
+	offload->dev = dev;
+
+	/* Limit queue len to 4x the weight (rounted to next power of two) */
+	offload->skb_queue_len_max = 2 << fls(weight);
+	offload->skb_queue_len_max *= 4;
+	skb_queue_head_init(&offload->skb_queue);
+
+	can_rx_offload_reset(offload);
+	netif_napi_add(dev, &offload->napi, can_rx_offload_napi_poll, weight);
+
+	dev_dbg(dev->dev.parent, "%s: mb_first=%d mb_last=%d queue_len_max=%d\n",
+		__func__, offload->mb_first, offload->mb_last,
+		offload->skb_queue_len_max);
+
+	return 0;
+}
+
+int can_rx_offload_add_fifo(struct net_device *dev, struct can_rx_offload *offload, unsigned int weight)
+{
+	if (!offload->mailbox_read)
+		return -EINVAL;
+
+	return can_rx_offload_init_queue(dev, offload, weight);
+}
+EXPORT_SYMBOL_GPL(can_rx_offload_add_fifo);
+
+void can_rx_offload_enable(struct can_rx_offload *offload)
+{
+	can_rx_offload_reset(offload);
+	napi_enable(&offload->napi);
+}
+EXPORT_SYMBOL_GPL(can_rx_offload_enable);
+
+void can_rx_offload_irq_error(struct can_rx_offload *offload)
+{
+	offload->poll_errors = true;
+	can_rx_offload_schedule(offload);
+}
+EXPORT_SYMBOL_GPL(can_rx_offload_irq_error);
+
+void can_rx_offload_del(struct can_rx_offload *offload)
+{
+	netif_napi_del(&offload->napi);
+	skb_queue_purge(&offload->skb_queue);
+}
+EXPORT_SYMBOL_GPL(can_rx_offload_del);
+
+void can_rx_offload_reset(struct can_rx_offload *offload)
+{
+	offload->poll_errors = false;
+}
+EXPORT_SYMBOL_GPL(can_rx_offload_reset);
diff --git a/include/linux/can/rx-offload.h b/include/linux/can/rx-offload.h
new file mode 100644
index 000000000000..a0ebc49aef2a
--- /dev/null
+++ b/include/linux/can/rx-offload.h
@@ -0,0 +1,56 @@
+/*
+ * linux/can/rx-offload.h
+ *
+ * Copyright (c) 2014 David Jander, Protonic Holland
+ * Copyright (c) 2014-2016 Pengutronix, Marc Kleine-Budde <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the version 2 of the GNU General Public License
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _CAN_RX_OFFLOAD_H
+#define _CAN_RX_OFFLOAD_H
+
+#include <linux/netdevice.h>
+#include <linux/can.h>
+
+struct can_rx_offload {
+	struct net_device *dev;
+
+	void (*poll_error_interrupts_enable)(struct can_rx_offload *offload);
+	unsigned int (*mailbox_read)(struct can_rx_offload *offload, struct can_frame *cf, unsigned int mb);
+	unsigned int (*poll_pre_read)(struct can_rx_offload *offload);
+	unsigned int (*poll_post_read)(struct can_rx_offload *offload);
+
+	struct sk_buff_head skb_queue;
+	u32 skb_queue_len_max;
+
+	struct napi_struct napi;
+
+	bool poll_errors;
+};
+
+int can_rx_offload_add_fifo(struct net_device *dev, struct can_rx_offload *offload, unsigned int weight);
+int can_rx_offload_irq_offload_fifo(struct can_rx_offload *offload);
+void can_rx_offload_irq_error(struct can_rx_offload *offload);
+void can_rx_offload_reset(struct can_rx_offload *offload);
+void can_rx_offload_del(struct can_rx_offload *offload);
+void can_rx_offload_enable(struct can_rx_offload *offload);
+
+static inline void can_rx_offload_schedule(struct can_rx_offload *offload)
+{
+	napi_schedule(&offload->napi);
+}
+
+static inline void can_rx_offload_disable(struct can_rx_offload *offload)
+{
+	napi_disable(&offload->napi);
+}
+
+#endif /* !_CAN_RX_OFFLOAD_H */
-- 
2.8.1

[PATCH v2 02/12] can: rx-offload: Add support for timestamp based irq offloading

[Marc Kleine-Budde]

Some CAN controllers don't implement a FIFO in hardware, but fill their
mailboxes in a particular order (from lowest to highest or highest to lowest).
This makes problems to read the frames in the correct order from the hardware,
as new frames might be filled into just read (low) mailboxes. This gets worse,
when following new frames are received into not read (higher) mailboxes.

On the bright side some these CAN controllers put a timestamp on each received
CAN frame. This patch adds support to offload CAN frames in interrupt context,
order them by timestamp and then transmitted in a NAPI context.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/rx-offload.c   | 135 ++++++++++++++++++++++++++++++++++++++++-
 include/linux/can/rx-offload.h |   9 ++-
 2 files changed, 141 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/rx-offload.c b/drivers/net/can/rx-offload.c
index db360ddeec97..74bde3992cb4 100644
--- a/drivers/net/can/rx-offload.c
+++ b/drivers/net/can/rx-offload.c
@@ -18,6 +18,33 @@
 #include <linux/can/dev.h>
 #include <linux/can/rx-offload.h>
 
+struct can_rx_offload_cb {
+	u32 timestamp;
+};
+
+static inline struct can_rx_offload_cb *can_rx_offload_get_cb(struct sk_buff *skb)
+{
+	BUILD_BUG_ON(sizeof(struct can_rx_offload_cb) > sizeof(skb->cb));
+
+	return (struct can_rx_offload_cb *)skb->cb;
+}
+
+static inline bool can_rx_offload_le(struct can_rx_offload *offload, unsigned int a, unsigned int b)
+{
+	if (offload->inc)
+		return a <= b;
+	else
+		return a >= b;
+}
+
+static inline unsigned int can_rx_offload_inc(struct can_rx_offload *offload, unsigned int *val)
+{
+	if (offload->inc)
+		return (*val)++;
+	else
+		return (*val)--;
+}
+
 static int can_rx_offload_napi_poll(struct napi_struct *napi, int quota)
 {
 	struct can_rx_offload *offload = container_of(napi, struct can_rx_offload, napi);
@@ -60,9 +87,47 @@ static int can_rx_offload_napi_poll(struct napi_struct *napi, int quota)
 	return work_done;
 }
 
+static inline void __skb_queue_add_sort(struct sk_buff_head *head, struct sk_buff *new,
+					int (*compare)(struct sk_buff *a, struct sk_buff *b))
+{
+	struct sk_buff *pos, *insert = (struct sk_buff *)head;
+
+	skb_queue_reverse_walk(head, pos) {
+		const struct can_rx_offload_cb *cb_pos, *cb_new;
+
+		cb_pos = can_rx_offload_get_cb(pos);
+		cb_new = can_rx_offload_get_cb(new);
+
+		netdev_dbg(new->dev,
+			   "%s: pos=0x%08x, new=0x%08x, diff=%10d, queue_len=%d\n",
+			   __func__,
+			   cb_pos->timestamp, cb_new->timestamp,
+			   cb_new->timestamp - cb_pos->timestamp,
+			   skb_queue_len(head));
+
+		if (compare(pos, new) < 0)
+			continue;
+		insert = pos;
+		break;
+	}
+
+	__skb_queue_after(head, insert, new);
+}
+
+static int can_rx_offload_compare(struct sk_buff *a, struct sk_buff *b)
+{
+	const struct can_rx_offload_cb *cb_a, *cb_b;
+
+	cb_a = can_rx_offload_get_cb(a);
+	cb_b = can_rx_offload_get_cb(b);
+
+	return cb_b->timestamp - cb_a->timestamp;
+}
+
 static struct sk_buff *can_rx_offload_offload_one(struct can_rx_offload *offload, unsigned int n)
 {
 	struct sk_buff *skb = NULL;
+	struct can_rx_offload_cb *cb;
 	struct can_frame *cf;
 	int ret;
 
@@ -73,15 +138,18 @@ static struct sk_buff *can_rx_offload_offload_one(struct can_rx_offload *offload
 
 	if (!skb) {
 		struct can_frame cf_overflow;
+		u32 timestamp;
 
-		ret = offload->mailbox_read(offload, &cf_overflow, n);
+		ret = offload->mailbox_read(offload, &cf_overflow,
+					    &timestamp, n);
 		if (ret)
 			offload->dev->stats.rx_dropped++;
 
 		return NULL;
 	}
 
-	ret = offload->mailbox_read(offload, cf, n);
+	cb = can_rx_offload_get_cb(skb);
+	ret = offload->mailbox_read(offload, cf, &cb->timestamp, n);
 	if (!ret) {
 		kfree_skb(skb);
 		return NULL;
@@ -90,6 +158,50 @@ static struct sk_buff *can_rx_offload_offload_one(struct can_rx_offload *offload
 	return skb;
 }
 
+int can_rx_offload_irq_offload_timestamp(struct can_rx_offload *offload, u64 pending)
+{
+	struct sk_buff_head skb_queue;
+	int received = 0;
+	unsigned int i;
+
+	__skb_queue_head_init(&skb_queue);
+
+	for (i = offload->mb_first;
+	     can_rx_offload_le(offload, i, offload->mb_last);
+	     can_rx_offload_inc(offload, &i)) {
+		struct sk_buff *skb;
+
+		if (!(pending & BIT_ULL(i)))
+			continue;
+
+		skb = can_rx_offload_offload_one(offload, i);
+		if (!skb)
+			break;
+
+		__skb_queue_add_sort(&skb_queue, skb, can_rx_offload_compare);
+		received++;
+	}
+
+	if (received) {
+		unsigned long flags;
+		u32 queue_len;
+
+		spin_lock_irqsave(&offload->skb_queue.lock, flags);
+		skb_queue_splice_tail(&skb_queue, &offload->skb_queue);
+		spin_unlock_irqrestore(&offload->skb_queue.lock, flags);
+
+		if ((queue_len = skb_queue_len(&offload->skb_queue)) >
+		    (offload->skb_queue_len_max / 8))
+			netdev_dbg(offload->dev, "%s: queue_len=%d\n",
+				   __func__, queue_len);
+
+		can_rx_offload_schedule(offload);
+	}
+
+	return received;
+}
+EXPORT_SYMBOL_GPL(can_rx_offload_irq_offload_timestamp);
+
 int can_rx_offload_irq_offload_fifo(struct can_rx_offload *offload)
 {
 	struct sk_buff *skb;
@@ -126,6 +238,25 @@ static int can_rx_offload_init_queue(struct net_device *dev, struct can_rx_offlo
 	return 0;
 }
 
+int can_rx_offload_add_timestamp(struct net_device *dev, struct can_rx_offload *offload)
+{
+	unsigned int weight;
+
+	if (!offload->mailbox_read)
+		return -EINVAL;
+
+	if (offload->mb_first < offload->mb_last) {
+		offload->inc = true;
+		weight = offload->mb_last - offload->mb_first;
+	} else {
+		offload->inc = false;
+		weight = offload->mb_first - offload->mb_last;
+	}
+
+	return can_rx_offload_init_queue(dev, offload, weight);;
+}
+EXPORT_SYMBOL_GPL(can_rx_offload_add_timestamp);
+
 int can_rx_offload_add_fifo(struct net_device *dev, struct can_rx_offload *offload, unsigned int weight)
 {
 	if (!offload->mailbox_read)
diff --git a/include/linux/can/rx-offload.h b/include/linux/can/rx-offload.h
index a0ebc49aef2a..4ef5203be934 100644
--- a/include/linux/can/rx-offload.h
+++ b/include/linux/can/rx-offload.h
@@ -24,19 +24,26 @@ struct can_rx_offload {
 	struct net_device *dev;
 
 	void (*poll_error_interrupts_enable)(struct can_rx_offload *offload);
-	unsigned int (*mailbox_read)(struct can_rx_offload *offload, struct can_frame *cf, unsigned int mb);
+	unsigned int (*mailbox_read)(struct can_rx_offload *offload, struct can_frame *cf,
+				     u32 *timestamp, unsigned int mb);
 	unsigned int (*poll_pre_read)(struct can_rx_offload *offload);
 	unsigned int (*poll_post_read)(struct can_rx_offload *offload);
 
 	struct sk_buff_head skb_queue;
 	u32 skb_queue_len_max;
 
+	unsigned int mb_first;
+	unsigned int mb_last;
+
 	struct napi_struct napi;
 
+	bool inc;
 	bool poll_errors;
 };
 
+int can_rx_offload_add_timestamp(struct net_device *dev, struct can_rx_offload *offload);
 int can_rx_offload_add_fifo(struct net_device *dev, struct can_rx_offload *offload, unsigned int weight);
+int can_rx_offload_irq_offload_timestamp(struct can_rx_offload *offload, u64 reg);
 int can_rx_offload_irq_offload_fifo(struct can_rx_offload *offload);
 void can_rx_offload_irq_error(struct can_rx_offload *offload);
 void can_rx_offload_reset(struct can_rx_offload *offload);
-- 
2.8.1

Re: [PATCH v2 02/12] can: rx-offload: Add support for timestamp based irq offloading

[Andri Yngvason]

Quoting Marc Kleine-Budde (2016-07-04 18:32:07)
> +static int can_rx_offload_compare(struct sk_buff *a, struct sk_buff *b)
> +{
> +       const struct can_rx_offload_cb *cb_a, *cb_b;
> +
> +       cb_a = can_rx_offload_get_cb(a);
> +       cb_b = can_rx_offload_get_cb(b);
> +
> +       return cb_b->timestamp - cb_a->timestamp;
> +}

Hi Marc,

An int is not large enough to store the difference between two u32.

E.g. INT_MAX - (-1) = INT_MIN

They're unlikely to be very far apart, but you can never be too cautious. ;)

See
http://stackoverflow.com/questions/10996418/efficient-integer-compare-function

Best regards,
Andri

Re: [PATCH v2 02/12] can: rx-offload: Add support for timestamp based irq offloading

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1656 bytes --]

On 07/04/2016 08:49 PM, Andri Yngvason wrote:
> Quoting Marc Kleine-Budde (2016-07-04 18:32:07)
>> +static int can_rx_offload_compare(struct sk_buff *a, struct sk_buff *b)
>> +{
>> +       const struct can_rx_offload_cb *cb_a, *cb_b;
>> +
>> +       cb_a = can_rx_offload_get_cb(a);
>> +       cb_b = can_rx_offload_get_cb(b);
>> +
>> +       return cb_b->timestamp - cb_a->timestamp;
>> +}
> 
> Hi Marc,
> 
> An int is not large enough to store the difference between two u32.
> 
> E.g. INT_MAX - (-1) = INT_MIN
> 
> They're unlikely to be very far apart, but you can never be too cautious. ;)

Actually I want to have the difference between two timestamps in order
to sort . The timestamps are monotonically increasing and will
eventually roll over. As the flexcan core only has a 16 bit timer, I
shift the timestamp by 16 bit (cb = reg_ts << 16), to fill the whole 32
bit. This way I don't have to take care about the overflow, right?

> main: a=0xfffffff0 b=0xfffffffc diff= 12
> main: a=0xfffffff0 b=0xfffffffd diff= 13
> main: a=0xfffffff0 b=0xfffffffe diff= 14
> main: a=0xfffffff0 b=0xffffffff diff= 15
> main: a=0xfffffff0 b=0x00000000 diff= 16
> main: a=0xfffffff0 b=0x00000001 diff= 17
> main: a=0xfffffff0 b=0x00000002 diff= 18
> main: a=0xfffffff0 b=0x00000003 diff= 19
> main: a=0xfffffff0 b=0x00000004 diff= 20

regards,
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: 455 bytes --]

Re: [PATCH v2 02/12] can: rx-offload: Add support for timestamp based irq offloading

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1671 bytes --]

On 07/04/2016 10:59 PM, Marc Kleine-Budde wrote:
> Actually I want to have the difference between two timestamps in order
> to sort . The timestamps are monotonically increasing and will
> eventually roll over. As the flexcan core only has a 16 bit timer, I
> shift the timestamp by 16 bit (cb = reg_ts << 16), to fill the whole 32
> bit. This way I don't have to take care about the overflow, right?
> 
>> main: a=0xfffffff0 b=0xfffffffc diff= 12
>> main: a=0xfffffff0 b=0xfffffffd diff= 13
>> main: a=0xfffffff0 b=0xfffffffe diff= 14
>> main: a=0xfffffff0 b=0xffffffff diff= 15
>> main: a=0xfffffff0 b=0x00000000 diff= 16
>> main: a=0xfffffff0 b=0x00000001 diff= 17
>> main: a=0xfffffff0 b=0x00000002 diff= 18
>> main: a=0xfffffff0 b=0x00000003 diff= 19
>> main: a=0xfffffff0 b=0x00000004 diff= 20

It's also working when the difference is negative:

> main: a=0xfffffffc b=0xfffffff0 diff=-12
> main: a=0xfffffffd b=0xfffffff0 diff=-13
> main: a=0xfffffffe b=0xfffffff0 diff=-14
> main: a=0xffffffff b=0xfffffff0 diff=-15
> main: a=0x00000000 b=0xfffffff0 diff=-16
> main: a=0x00000001 b=0xfffffff0 diff=-17
> main: a=0x00000002 b=0xfffffff0 diff=-18
> main: a=0x00000003 b=0xfffffff0 diff=-19

It will get "problematic" if the difference between the two u32 is
around 2^31. But then it's hard to tell which of the mailboxes holds the
older timestamp.

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

Re: [PATCH v2 02/12] can: rx-offload: Add support for timestamp based irq offloading

[Andri Yngvason]

On Tue, Jul 05, 2016 at 12:30:32AM +0200, Marc Kleine-Budde wrote:
> On 07/04/2016 10:59 PM, Marc Kleine-Budde wrote:
> > Actually I want to have the difference between two timestamps in order
> > to sort . The timestamps are monotonically increasing and will
> > eventually roll over. As the flexcan core only has a 16 bit timer, I
> > shift the timestamp by 16 bit (cb = reg_ts << 16), to fill the whole 32
> > bit. This way I don't have to take care about the overflow, right?
> > 
Hmmm, yes, that is quite clever. If I understand correctly, this means that if
you have three timestamps on the roll-over boundary: 0xfffe0000, 0xffff0000 and
0x0001000, the sorting algorithm will sort them in this particular order because
	(int)(0xfffe0000 - 0xffff0000) < 0
and
	(int)(0xffff0000 - 0x00010000) < 0

Perhaps there should be a comment explaining this?

> >> main: a=0xfffffff0 b=0xfffffffc diff= 12
> >> main: a=0xfffffff0 b=0xfffffffd diff= 13
> >> main: a=0xfffffff0 b=0xfffffffe diff= 14
> >> main: a=0xfffffff0 b=0xffffffff diff= 15
> >> main: a=0xfffffff0 b=0x00000000 diff= 16
> >> main: a=0xfffffff0 b=0x00000001 diff= 17
> >> main: a=0xfffffff0 b=0x00000002 diff= 18
> >> main: a=0xfffffff0 b=0x00000003 diff= 19
> >> main: a=0xfffffff0 b=0x00000004 diff= 20
> 
> It's also working when the difference is negative:
> 
> > main: a=0xfffffffc b=0xfffffff0 diff=-12
> > main: a=0xfffffffd b=0xfffffff0 diff=-13
> > main: a=0xfffffffe b=0xfffffff0 diff=-14
> > main: a=0xffffffff b=0xfffffff0 diff=-15
> > main: a=0x00000000 b=0xfffffff0 diff=-16
> > main: a=0x00000001 b=0xfffffff0 diff=-17
> > main: a=0x00000002 b=0xfffffff0 diff=-18
> > main: a=0x00000003 b=0xfffffff0 diff=-19
> 
> It will get "problematic" if the difference between the two u32 is
> around 2^31. But then it's hard to tell which of the mailboxes holds the
> older timestamp.
> 
Yes, it's also safe to assume that if you ever get this much of a difference
between timestamps in the mailbox, something else must be wrong before that
happens.

I've run into bugs involving this kind of a comparison function where the
difference actually can be large enough to cause problems, so this was a red
flag for me, but it turns out to be OK.

Best regards,
Andri

Re: [PATCH v2 02/12] can: rx-offload: Add support for timestamp based irq offloading

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 2996 bytes --]

On 07/05/2016 01:58 PM, Andri Yngvason wrote:
> On Tue, Jul 05, 2016 at 12:30:32AM +0200, Marc Kleine-Budde wrote:
>> On 07/04/2016 10:59 PM, Marc Kleine-Budde wrote:
>>> Actually I want to have the difference between two timestamps in order
>>> to sort . The timestamps are monotonically increasing and will
>>> eventually roll over. As the flexcan core only has a 16 bit timer, I
>>> shift the timestamp by 16 bit (cb = reg_ts << 16), to fill the whole 32
>>> bit. This way I don't have to take care about the overflow, right?
>>>
> Hmmm, yes, that is quite clever. If I understand correctly, this means that if
> you have three timestamps on the roll-over boundary: 0xfffe0000, 0xffff0000 and
> 0x0001000, the sorting algorithm will sort them in this particular order because
> 	(int)(0xfffe0000 - 0xffff0000) < 0
> and
> 	(int)(0xffff0000 - 0x00010000) < 0

ACK. The queue_add_sort will walk the skb_queue reverse (from oldest to
newest) optimizing for the case that frames are read in-order. So if I
read the frames in order (even over a roll-over) there will be a maximum
of one compare for each frame. Only the frames that are read in one
can_rx_offload_irq_offload_timestamp() call will be sorted.

> Perhaps there should be a comment explaining this?

ACK

>>>> main: a=0xfffffff0 b=0xfffffffc diff= 12
>>>> main: a=0xfffffff0 b=0xfffffffd diff= 13
>>>> main: a=0xfffffff0 b=0xfffffffe diff= 14
>>>> main: a=0xfffffff0 b=0xffffffff diff= 15
>>>> main: a=0xfffffff0 b=0x00000000 diff= 16
>>>> main: a=0xfffffff0 b=0x00000001 diff= 17
>>>> main: a=0xfffffff0 b=0x00000002 diff= 18
>>>> main: a=0xfffffff0 b=0x00000003 diff= 19
>>>> main: a=0xfffffff0 b=0x00000004 diff= 20
>>
>> It's also working when the difference is negative:
>>
>>> main: a=0xfffffffc b=0xfffffff0 diff=-12
>>> main: a=0xfffffffd b=0xfffffff0 diff=-13
>>> main: a=0xfffffffe b=0xfffffff0 diff=-14
>>> main: a=0xffffffff b=0xfffffff0 diff=-15
>>> main: a=0x00000000 b=0xfffffff0 diff=-16
>>> main: a=0x00000001 b=0xfffffff0 diff=-17
>>> main: a=0x00000002 b=0xfffffff0 diff=-18
>>> main: a=0x00000003 b=0xfffffff0 diff=-19
>>
>> It will get "problematic" if the difference between the two u32 is
>> around 2^31. But then it's hard to tell which of the mailboxes holds the
>> older timestamp.
>>
> Yes, it's also safe to assume that if you ever get this much of a difference
> between timestamps in the mailbox, something else must be wrong before that
> happens.

ACK

> I've run into bugs involving this kind of a comparison function where the
> difference actually can be large enough to cause problems, so this was a red
> flag for me, but it turns out to be OK.

That

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

Re: [PATCH v2 02/12] can: rx-offload: Add support for timestamp based irq offloading

[Alexander Stein]

On Monday 04 July 2016 20:32:07, Marc Kleine-Budde wrote:
> Some CAN controllers don't implement a FIFO in hardware, but fill their
> mailboxes in a particular order (from lowest to highest or highest to
> lowest). This makes problems to read the frames in the correct order from
> the hardware, as new frames might be filled into just read (low) mailboxes.
> This gets worse, when following new frames are received into not read
> (higher) mailboxes.
> 
> On the bright side some these CAN controllers put a timestamp on each
> received CAN frame. This patch adds support to offload CAN frames in
> interrupt context, order them by timestamp and then transmitted in a NAPI
> context.
> 
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

How is this supposed to work with CAN controllers without tmestamps, e.g. 
CCAN/DCAN? Do you need to set the timestamp manually using ktime_get() or 
similar when reading each mailbox? IMHO especially those controllers are prone 
to ordering issues under heavy load as the hardware doesn't ensure ordering by 
itself and there is no hardware based timestamp.

Best regards,
Alexander

Re: [PATCH v2 02/12] can: rx-offload: Add support for timestamp based irq offloading

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1866 bytes --]

On 07/05/2016 07:46 AM, Alexander Stein wrote:
> On Monday 04 July 2016 20:32:07, Marc Kleine-Budde wrote:
>> Some CAN controllers don't implement a FIFO in hardware, but fill their
>> mailboxes in a particular order (from lowest to highest or highest to
>> lowest). This makes problems to read the frames in the correct order from
>> the hardware, as new frames might be filled into just read (low) mailboxes.
>> This gets worse, when following new frames are received into not read
>> (higher) mailboxes.
>>
>> On the bright side some these CAN controllers put a timestamp on each
>> received CAN frame. This patch adds support to offload CAN frames in
>> interrupt context, order them by timestamp and then transmitted in a NAPI
>> context.
>>
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> 
> How is this supposed to work with CAN controllers without tmestamps, e.g. 
> CCAN/DCAN? Do you need to set the timestamp manually using ktime_get() or 
> similar when reading each mailbox? IMHO especially those controllers are prone 
> to ordering issues under heavy load as the hardware doesn't ensure ordering by 
> itself and there is no hardware based timestamp.

The algorithm added in this patch only supports controllers that add
timestamps to the mailbox on reception in hardware (like flexcan, at91
and hecc). However the other algorithm we implemented didn't make use of
timestamps and targeted controllers that fill mailboxes in a particular
order. It can be ported to the current framework and the c_can/d_can
driver to it.

regards,
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: 455 bytes --]

Re: [PATCH v2 02/12] can: rx-offload: Add support for timestamp based irq offloading

[David Jander]

On Tue, 5 Jul 2016 08:19:31 +0200
Marc Kleine-Budde <mkl@pengutronix.de> wrote:

> On 07/05/2016 07:46 AM, Alexander Stein wrote:
> > On Monday 04 July 2016 20:32:07, Marc Kleine-Budde wrote:  
> >> Some CAN controllers don't implement a FIFO in hardware, but fill their
> >> mailboxes in a particular order (from lowest to highest or highest to
> >> lowest). This makes problems to read the frames in the correct order from
> >> the hardware, as new frames might be filled into just read (low) mailboxes.
> >> This gets worse, when following new frames are received into not read
> >> (higher) mailboxes.
> >>
> >> On the bright side some these CAN controllers put a timestamp on each
> >> received CAN frame. This patch adds support to offload CAN frames in
> >> interrupt context, order them by timestamp and then transmitted in a NAPI
> >> context.
> >>
> >> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>  
> > 
> > How is this supposed to work with CAN controllers without tmestamps, e.g. 
> > CCAN/DCAN? Do you need to set the timestamp manually using ktime_get() or 
> > similar when reading each mailbox? IMHO especially those controllers are prone 
> > to ordering issues under heavy load as the hardware doesn't ensure ordering by 
> > itself and there is no hardware based timestamp.  
> 
> The algorithm added in this patch only supports controllers that add
> timestamps to the mailbox on reception in hardware (like flexcan, at91
> and hecc). However the other algorithm we implemented didn't make use of
> timestamps and targeted controllers that fill mailboxes in a particular
> order. It can be ported to the current framework and the c_can/d_can
> driver to it.

I originally wrote order-based FIFO simulation for the flexcan, that didn't
use the timestamps. It turned out that flexcan has some hardware issues that
prevented this implementation from working reliably on the flexcan controller.
In fact, the whole ordered reception mechanism is useless on flexcan without
timestamps.
Marc: did you throw away the original implementation for good, or are you
planning to add it again, in order to use it for CCAN/DCAN and similar
controllers?

Best regards,

-- 
David Jander
Protonic Holland.

Re: [PATCH v2 02/12] can: rx-offload: Add support for timestamp based irq offloading

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 2736 bytes --]

On 07/05/2016 08:31 AM, David Jander wrote:
> On Tue, 5 Jul 2016 08:19:31 +0200
> Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> 
>> On 07/05/2016 07:46 AM, Alexander Stein wrote:
>>> On Monday 04 July 2016 20:32:07, Marc Kleine-Budde wrote:  
>>>> Some CAN controllers don't implement a FIFO in hardware, but fill their
>>>> mailboxes in a particular order (from lowest to highest or highest to
>>>> lowest). This makes problems to read the frames in the correct order from
>>>> the hardware, as new frames might be filled into just read (low) mailboxes.
>>>> This gets worse, when following new frames are received into not read
>>>> (higher) mailboxes.
>>>>
>>>> On the bright side some these CAN controllers put a timestamp on each
>>>> received CAN frame. This patch adds support to offload CAN frames in
>>>> interrupt context, order them by timestamp and then transmitted in a NAPI
>>>> context.
>>>>
>>>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>  
>>>
>>> How is this supposed to work with CAN controllers without tmestamps, e.g. 
>>> CCAN/DCAN? Do you need to set the timestamp manually using ktime_get() or 
>>> similar when reading each mailbox? IMHO especially those controllers are prone 
>>> to ordering issues under heavy load as the hardware doesn't ensure ordering by 
>>> itself and there is no hardware based timestamp.  
>>
>> The algorithm added in this patch only supports controllers that add
>> timestamps to the mailbox on reception in hardware (like flexcan, at91
>> and hecc). However the other algorithm we implemented didn't make use of
>> timestamps and targeted controllers that fill mailboxes in a particular
>> order. It can be ported to the current framework and the c_can/d_can
>> driver to it.
> 
> I originally wrote order-based FIFO simulation for the flexcan, that didn't
> use the timestamps. It turned out that flexcan has some hardware issues that
> prevented this implementation from working reliably on the flexcan controller.
> In fact, the whole ordered reception mechanism is useless on flexcan without
> timestamps.
> Marc: did you throw away the original implementation for good, or are you
> planning to add it again, in order to use it for CCAN/DCAN and similar
> controllers?

My focus was on flexcan for now, but the old order-based FIFO simulation
can be ported to the new infrastructure and probably to the c_can/d_can
driver.

regards,
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: 455 bytes --]

[PATCH v2 03/12] can: flexcan: remove write-only member pdata of struct flexcan_priv

[Marc Kleine-Budde]

This patch removes the write only member pdata from the struct
flexcan_priv.

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

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 41c0fc9f3b14..dea2bf27cfc4 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -257,7 +257,6 @@ struct flexcan_priv {
 
 	struct clk *clk_ipg;
 	struct clk *clk_per;
-	struct flexcan_platform_data *pdata;
 	const struct flexcan_devtype_data *devtype_data;
 	struct regulator *reg_xceiver;
 };
@@ -1223,7 +1222,6 @@ static int flexcan_probe(struct platform_device *pdev)
 	priv->regs = regs;
 	priv->clk_ipg = clk_ipg;
 	priv->clk_per = clk_per;
-	priv->pdata = dev_get_platdata(&pdev->dev);
 	priv->devtype_data = devtype_data;
 	priv->reg_xceiver = reg_xceiver;
 
-- 
2.8.1

[PATCH v2 04/12] can: flexcan: make declaration of devtype_data const

[Marc Kleine-Budde]

This patch changes the declaration of the devtype data to const.

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

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index dea2bf27cfc4..617e723de909 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -261,17 +261,17 @@ struct flexcan_priv {
 	struct regulator *reg_xceiver;
 };
 
-static struct flexcan_devtype_data fsl_p1010_devtype_data = {
+static const struct flexcan_devtype_data fsl_p1010_devtype_data = {
 	.quirks = FLEXCAN_QUIRK_BROKEN_ERR_STATE,
 };
 
-static struct flexcan_devtype_data fsl_imx28_devtype_data;
+static const struct flexcan_devtype_data fsl_imx28_devtype_data;
 
-static struct flexcan_devtype_data fsl_imx6q_devtype_data = {
+static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
 	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG,
 };
 
-static struct flexcan_devtype_data fsl_vf610_devtype_data = {
+static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
 	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_DISABLE_MECR,
 };
 
-- 
2.8.1

[PATCH v2 05/12] can: flexcan: calculate default value for imask1 during runtime

[Marc Kleine-Budde]

This patch converts the define FLEXCAN_IFLAG_DEFAULT into the runtime
calculated value priv->reg_imask1_default. This is a preparation patch to make
the TX mailbox selectable during runtime, too.

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

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 617e723de909..bcdad95a5749 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -149,9 +149,6 @@
 #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_RX_INACTIVE	(0x0 << 24)
@@ -254,6 +251,7 @@ struct flexcan_priv {
 	struct flexcan_regs __iomem *regs;
 	u32 reg_esr;
 	u32 reg_ctrl_default;
+	u32 reg_imask1_default;
 
 	struct clk *clk_ipg;
 	struct clk *clk_per;
@@ -704,7 +702,7 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 	if (work_done < quota) {
 		napi_complete(napi);
 		/* enable IRQs */
-		flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+		flexcan_write(priv->reg_imask1_default, &regs->imask1);
 		flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
 	}
 
@@ -738,7 +736,7 @@ 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_write(priv->reg_imask1_default &
 			      ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
 		flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
 			      &regs->ctrl);
@@ -941,7 +939,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	/* enable interrupts atomically */
 	disable_irq(dev->irq);
 	flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
-	flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+	flexcan_write(priv->reg_imask1_default, &regs->imask1);
 	enable_irq(dev->irq);
 
 	/* print chip status */
@@ -1225,6 +1223,10 @@ static int flexcan_probe(struct platform_device *pdev)
 	priv->devtype_data = devtype_data;
 	priv->reg_xceiver = reg_xceiver;
 
+	priv->reg_imask1_default = FLEXCAN_IFLAG_RX_FIFO_OVERFLOW |
+		FLEXCAN_IFLAG_RX_FIFO_AVAILABLE |
+		FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID);
+
 	netif_napi_add(dev, &priv->napi, flexcan_poll, FLEXCAN_NAPI_WEIGHT);
 
 	platform_set_drvdata(pdev, dev);
-- 
2.8.1

[PATCH v2 06/12] can: flexcan: make TX mailbox selectable during runtime

[Marc Kleine-Budde]

This patch makes the TX mailbox selectable duing runtime. This is a preparation
patch to use of the hardware FIFO selectable via runtime. As the TX mailbox
number is different in HW FIFO and normal mode.

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

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index bcdad95a5749..f3e67e7c0f49 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -143,9 +143,9 @@
 
 /* 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_IFLAG_BUF(x)		BIT(x)
+#define FLEXCAN_TX_MB_RESERVED_HW_FIFO	8
+#define FLEXCAN_TX_MB_HW_FIFO		9
+#define FLEXCAN_IFLAG_MB(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)
@@ -249,6 +249,9 @@ struct flexcan_priv {
 	struct napi_struct napi;
 
 	struct flexcan_regs __iomem *regs;
+	struct flexcan_mb __iomem *tx_mb;
+	struct flexcan_mb __iomem *tx_mb_reserved;
+	u8 tx_mb_idx;
 	u32 reg_esr;
 	u32 reg_ctrl_default;
 	u32 reg_imask1_default;
@@ -465,7 +468,6 @@ static int flexcan_get_berr_counter(const struct net_device *dev,
 static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	const struct flexcan_priv *priv = netdev_priv(dev);
-	struct flexcan_regs __iomem *regs = priv->regs;
 	struct can_frame *cf = (struct can_frame *)skb->data;
 	u32 can_id;
 	u32 data;
@@ -488,25 +490,25 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	if (cf->can_dlc > 0) {
 		data = be32_to_cpup((__be32 *)&cf->data[0]);
-		flexcan_write(data, &regs->mb[FLEXCAN_TX_BUF_ID].data[0]);
+		flexcan_write(data, &priv->tx_mb->data[0]);
 	}
 	if (cf->can_dlc > 3) {
 		data = be32_to_cpup((__be32 *)&cf->data[4]);
-		flexcan_write(data, &regs->mb[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->mb[FLEXCAN_TX_BUF_ID].can_id);
-	flexcan_write(ctrl, &regs->mb[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->mb[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
+		      &priv->tx_mb_reserved->can_ctrl);
 	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
-		      &regs->mb[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
+		      &priv->tx_mb_reserved->can_ctrl);
 
 	return NETDEV_TX_OK;
 }
@@ -751,15 +753,15 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	}
 
 	/* transmission complete interrupt */
-	if (reg_iflag1 & (1 << FLEXCAN_TX_BUF_ID)) {
+	if (reg_iflag1 & FLEXCAN_IFLAG_MB(priv->tx_mb_idx)) {
 		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 MB is in RX mode */
 		flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
-			      &regs->mb[FLEXCAN_TX_BUF_ID].can_ctrl);
-		flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
+			      &priv->tx_mb->can_ctrl);
+		flexcan_write(FLEXCAN_IFLAG_MB(priv->tx_mb_idx), &regs->iflag1);
 		netif_wake_queue(dev);
 	}
 
@@ -843,7 +845,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
 	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN | FLEXCAN_MCR_SRX_DIS |
-		FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
+		FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
 	netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
 	flexcan_write(reg_mcr, &regs->mcr);
 
@@ -881,18 +883,18 @@ static int flexcan_chip_start(struct net_device *dev)
 	flexcan_write(reg_ctrl, &regs->ctrl);
 
 	/* clear and invalidate all mailboxes first */
-	for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->mb); i++) {
+	for (i = priv->tx_mb_idx; i < ARRAY_SIZE(regs->mb); i++) {
 		flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE,
 			      &regs->mb[i].can_ctrl);
 	}
 
 	/* Errata ERR005829: mark first TX mailbox as INACTIVE */
 	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
-		      &regs->mb[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
+		      &priv->tx_mb_reserved->can_ctrl);
 
 	/* mark TX mailbox as INACTIVE */
 	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
-		      &regs->mb[FLEXCAN_TX_BUF_ID].can_ctrl);
+		      &priv->tx_mb->can_ctrl);
 
 	/* acceptance mask/acceptance code (accept everything) */
 	flexcan_write(0x0, &regs->rxgmask);
@@ -1223,9 +1225,13 @@ static int flexcan_probe(struct platform_device *pdev)
 	priv->devtype_data = devtype_data;
 	priv->reg_xceiver = reg_xceiver;
 
+	priv->tx_mb_idx = FLEXCAN_TX_MB_HW_FIFO;
+	priv->tx_mb_reserved = &regs->mb[FLEXCAN_TX_MB_RESERVED_HW_FIFO];
+	priv->tx_mb = &regs->mb[priv->tx_mb_idx];
+
 	priv->reg_imask1_default = FLEXCAN_IFLAG_RX_FIFO_OVERFLOW |
 		FLEXCAN_IFLAG_RX_FIFO_AVAILABLE |
-		FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID);
+		FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
 
 	netif_napi_add(dev, &priv->napi, flexcan_poll, FLEXCAN_NAPI_WEIGHT);
 
-- 
2.8.1

[PATCH v2 07/12] can: flexcan: make use of rx-offload's irq_offload_fifo

[Marc Kleine-Budde]

This patch converts the flexcan driver to make use of the rx-offload
can_rx_offload_irq_offload_fifo() helper function. The idea is to read
the CAN frames already in the interrupt context, as the depth of the
flexcan HW FIFO is too shallow, resulting in too many missed frames.
During a normal NAPI poll the frames are the pushed into the upper
layers.

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

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index f3e67e7c0f49..adad38d17b0e 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -24,6 +24,7 @@
 #include <linux/can/dev.h>
 #include <linux/can/error.h>
 #include <linux/can/led.h>
+#include <linux/can/rx-offload.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
@@ -143,8 +144,8 @@
 
 /* FLEXCAN interrupt flag register (IFLAG) bits */
 /* Errata ERR005829 step7: Reserve first valid MB */
-#define FLEXCAN_TX_MB_RESERVED_HW_FIFO	8
-#define FLEXCAN_TX_MB_HW_FIFO		9
+#define FLEXCAN_TX_MB_RESERVED_OFF_FIFO	8
+#define FLEXCAN_TX_MB_OFF_FIFO		9
 #define FLEXCAN_IFLAG_MB(x)		BIT(x)
 #define FLEXCAN_IFLAG_RX_FIFO_OVERFLOW	BIT(7)
 #define FLEXCAN_IFLAG_RX_FIFO_WARN	BIT(6)
@@ -246,13 +247,14 @@ struct flexcan_devtype_data {
 
 struct flexcan_priv {
 	struct can_priv can;
-	struct napi_struct napi;
+	struct can_rx_offload offload;
 
 	struct flexcan_regs __iomem *regs;
 	struct flexcan_mb __iomem *tx_mb;
 	struct flexcan_mb __iomem *tx_mb_reserved;
 	u8 tx_mb_idx;
 	u32 reg_esr;
+	u32 poll_esr;			/* used in flexcan_poll_bus_err */
 	u32 reg_ctrl_default;
 	u32 reg_imask1_default;
 
@@ -513,6 +515,11 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	return NETDEV_TX_OK;
 }
 
+static inline struct flexcan_priv *rx_offload_to_priv(struct can_rx_offload *offload)
+{
+	return container_of(offload, struct flexcan_priv, offload);
+}
+
 static void do_bus_err(struct net_device *dev,
 		       struct can_frame *cf, u32 reg_esr)
 {
@@ -561,16 +568,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_offload *offload)
 {
+	struct flexcan_priv *priv = rx_offload_to_priv(offload);
+	struct net_device *dev = offload->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);
 
 	dev->stats.rx_packets++;
 	dev->stats.rx_bytes += cf->can_dlc;
@@ -579,14 +591,21 @@ static int flexcan_poll_bus_err(struct net_device *dev, u32 reg_esr)
 	return 1;
 }
 
-static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
+static unsigned int flexcan_poll_state(struct can_rx_offload *offload)
 {
-	struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_priv *priv = rx_offload_to_priv(offload);
+	struct net_device *dev = offload->dev;
+	struct flexcan_regs __iomem *regs = priv->regs;
 	struct sk_buff *skb;
 	struct can_frame *cf;
 	enum can_state new_state = 0, rx_state = 0, tx_state = 0;
 	int flt;
 	struct can_berr_counter bec;
+	u32 reg_esr;
+
+	/* esr bits are clear-on-read, so save them for flexcan_poll_bus_err() */
+	priv->poll_esr = priv->reg_esr | flexcan_read(&regs->esr);
+	reg_esr = priv->poll_esr;
 
 	flt = reg_esr & FLEXCAN_ESR_FLT_CONF_MASK;
 	if (likely(flt == FLEXCAN_ESR_FLT_CONF_ACTIVE)) {
@@ -623,15 +642,23 @@ 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 unsigned int flexcan_mailbox_read(struct can_rx_offload *offload,
+					 struct can_frame *cf,
+					 u32 *timestamp, unsigned int n)
 {
-	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_priv *priv = rx_offload_to_priv(offload);
 	struct flexcan_regs __iomem *regs = priv->regs;
-	struct flexcan_mb __iomem *mb = &regs->mb[0];
-	u32 reg_ctrl, reg_id;
+	struct flexcan_mb __iomem *mb = &regs->mb[n];
+	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);
+	/* increase timstamp to full 32 bit */
+	*timestamp = reg_ctrl << 16;
+
 	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;
@@ -648,67 +675,16 @@ static void flexcan_read_fifo(const struct net_device *dev,
 	/* 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);
-
-	stats->rx_packets++;
-	stats->rx_bytes += cf->can_dlc;
-	netif_receive_skb(skb);
-
-	can_led_event(dev, CAN_LED_EVENT_RX);
 
 	return 1;
 }
 
-static int flexcan_poll(struct napi_struct *napi, int quota)
+static void flexcan_poll_error_interrupts_enable(struct can_rx_offload *offload)
 {
-	struct net_device *dev = napi->dev;
-	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_priv *priv = rx_offload_to_priv(offload);
 	struct flexcan_regs __iomem *regs = priv->regs;
-	u32 reg_iflag1, reg_esr;
-	int work_done = 0;
-
-	/* 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 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);
-	}
-
-	/* 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);
-		/* enable IRQs */
-		flexcan_write(priv->reg_imask1_default, &regs->imask1);
-		flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
-	}
 
-	return work_done;
+	flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
 }
 
 static irqreturn_t flexcan_irq(int irq, void *dev_id)
@@ -721,30 +697,27 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 
 	reg_iflag1 = flexcan_read(&regs->iflag1);
 	reg_esr = flexcan_read(&regs->esr);
+	/* The error bits are cleared on read,
+	 * save them for later use.
+	 */
+	priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
 
 	/* 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);
 
-	/* 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.
-		 */
-		priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
-		flexcan_write(priv->reg_imask1_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_offload_irq_error(&priv->offload);
 	}
 
+	/* reception interrupt */
+	if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE)
+		can_rx_offload_irq_offload_fifo(&priv->offload);
+
 	/* FIFO overflow */
 	if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
 		flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
@@ -1007,7 +980,7 @@ static int flexcan_open(struct net_device *dev)
 
 	can_led_event(dev, CAN_LED_EVENT_OPEN);
 
-	napi_enable(&priv->napi);
+	can_rx_offload_enable(&priv->offload);
 	netif_start_queue(dev);
 
 	return 0;
@@ -1029,7 +1002,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_offload_disable(&priv->offload);
 	flexcan_chip_stop(dev);
 
 	free_irq(dev->irq, dev);
@@ -1207,6 +1180,9 @@ static int flexcan_probe(struct platform_device *pdev)
 	if (!dev)
 		return -ENOMEM;
 
+	platform_set_drvdata(pdev, dev);
+	SET_NETDEV_DEV(dev, &pdev->dev);
+
 	dev->netdev_ops = &flexcan_netdev_ops;
 	dev->irq = irq;
 	dev->flags |= IFF_ECHO;
@@ -1225,18 +1201,23 @@ static int flexcan_probe(struct platform_device *pdev)
 	priv->devtype_data = devtype_data;
 	priv->reg_xceiver = reg_xceiver;
 
-	priv->tx_mb_idx = FLEXCAN_TX_MB_HW_FIFO;
-	priv->tx_mb_reserved = &regs->mb[FLEXCAN_TX_MB_RESERVED_HW_FIFO];
+	priv->tx_mb_idx = FLEXCAN_TX_MB_OFF_FIFO;
+	priv->tx_mb_reserved = &regs->mb[FLEXCAN_TX_MB_RESERVED_OFF_FIFO];
 	priv->tx_mb = &regs->mb[priv->tx_mb_idx];
 
 	priv->reg_imask1_default = FLEXCAN_IFLAG_RX_FIFO_OVERFLOW |
 		FLEXCAN_IFLAG_RX_FIFO_AVAILABLE |
 		FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
 
-	netif_napi_add(dev, &priv->napi, flexcan_poll, FLEXCAN_NAPI_WEIGHT);
+	priv->offload.poll_pre_read = flexcan_poll_state;
+	priv->offload.poll_post_read = flexcan_poll_bus_err;
+	priv->offload.poll_error_interrupts_enable =
+		flexcan_poll_error_interrupts_enable;
+	priv->offload.mailbox_read = flexcan_mailbox_read;
 
-	platform_set_drvdata(pdev, dev);
-	SET_NETDEV_DEV(dev, &pdev->dev);
+	err = can_rx_offload_add_fifo(dev, &priv->offload, FLEXCAN_NAPI_WEIGHT);
+	if (err)
+		goto failed_offload;
 
 	err = register_flexcandev(dev);
 	if (err) {
@@ -1251,6 +1232,7 @@ static int flexcan_probe(struct platform_device *pdev)
 
 	return 0;
 
+ failed_offload:
  failed_register:
 	free_candev(dev);
 	return err;
@@ -1262,7 +1244,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_offload_del(&priv->offload);
 	free_candev(dev);
 
 	return 0;
-- 
2.8.1

[PATCH v2 08/12] can: flexcan: add missing register definitions

[Marc Kleine-Budde]

This patch adds some missing register definitions, which are needed in an
upcoming patch.

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

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index adad38d17b0e..ae16add589a8 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -56,7 +56,7 @@
 #define FLEXCAN_MCR_WAK_SRC		BIT(19)
 #define FLEXCAN_MCR_DOZE		BIT(18)
 #define FLEXCAN_MCR_SRX_DIS		BIT(17)
-#define FLEXCAN_MCR_BCC			BIT(16)
+#define FLEXCAN_MCR_IRMQ		BIT(16)
 #define FLEXCAN_MCR_LPRIO_EN		BIT(13)
 #define FLEXCAN_MCR_AEN			BIT(12)
 #define FLEXCAN_MCR_MAXMB(x)		((x) & 0x7f)
@@ -211,7 +211,10 @@ struct flexcan_regs {
 	u32 imask1;		/* 0x28 */
 	u32 iflag2;		/* 0x2c */
 	u32 iflag1;		/* 0x30 */
-	u32 ctrl2;		/* 0x34 */
+	union {			/* 0x34 */
+		u32 gfwr_mx28;	/* MX28, MX53 */
+		u32 ctrl2;	/* MX6, VF610 */
+	};
 	u32 esr2;		/* 0x38 */
 	u32 imeur;		/* 0x3c */
 	u32 lrfr;		/* 0x40 */
@@ -230,7 +233,11 @@ struct flexcan_regs {
 	 *				size conf'ed via ctrl2::RFFN
 	 *				(mx6, vf610)
 	 */
-	u32 _reserved4[408];
+	u32 _reserved4[256];	/* 0x480 */
+	u32 rximr[64];		/* 0x880 */
+	u32 _reserved5[24];	/* 0x980 */
+	u32 gfwr_mx6;		/* 0x9e0 - MX6 */
+	u32 _reserved6[63];	/* 0x9e4 */
 	u32 mecr;		/* 0xae0 */
 	u32 erriar;		/* 0xae4 */
 	u32 erridpr;		/* 0xae8 */
-- 
2.8.1

[PATCH v2 09/12] can: flexcan: activate individual RX masking and initialize reg_rximr

[Marc Kleine-Budde]

Modern flexcan IP cores support two RX modes. One is using the 6 fames deep
hardware FIFO, the other is using up to 64 mailboxes (in non FIFO mode). For
now only the HW FIFO mode is activated.

In order to make use of the RX mailboxes the individual RX masking feature has
to be activated, otherwise matching mailboxes are overwritten during the
reception process. This however switches on the individual RX masking, which
uses reg_rximr registers for masking.

This patch activates the individual RX masking feature unconditionally and
initializes the mask registers (reg_rximr) with 0x0 == "don't care", which
switches off any filtering.

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

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index ae16add589a8..793be68e02b7 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -818,6 +818,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	 * only supervisor access
 	 * enable warning int
 	 * disable local echo
+	 * enable individual RX masking
 	 * choose format C
 	 * set max mailbox number
 	 */
@@ -825,7 +826,8 @@ static int flexcan_chip_start(struct net_device *dev)
 	reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
 	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN | FLEXCAN_MCR_SRX_DIS |
-		FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
+		FLEXCAN_MCR_IRMQ | FLEXCAN_MCR_IDAM_C |
+		FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
 	netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
 	flexcan_write(reg_mcr, &regs->mcr);
 
@@ -884,6 +886,10 @@ static int flexcan_chip_start(struct net_device *dev)
 	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_DISABLE_RXFG)
 		flexcan_write(0x0, &regs->rxfgmask);
 
+	/* clear acceptance filters */
+	for (i = 0; i < ARRAY_SIZE(regs->mb); i++)
+		flexcan_write(0, &regs->rximr[i]);
+
 	/* On Vybrid, disable memory error detection interrupts
 	 * and freeze mode.
 	 * This also works around errata e5295 which generates
-- 
2.8.1

[PATCH v2 10/12] can: flexcan: add quirk FLEXCAN_QUIRK_ENABLE_EACEN_RRS

[Marc Kleine-Budde]

In order to receive RTR frames in the non HW FIFO mode the RSS and EACEN bits
of the reg_ctrl2 have to be activated. As this has no side effect in the FIFO
mode, we do this unconditionally on cores with the reg_ctrl2.

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

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 793be68e02b7..c94a65dbb1c2 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -187,7 +187,8 @@
  */
 #define FLEXCAN_QUIRK_BROKEN_ERR_STATE	BIT(1) /* [TR]WRN_INT not connected */
 #define FLEXCAN_QUIRK_DISABLE_RXFG	BIT(2) /* Disable RX FIFO Global mask */
-#define FLEXCAN_QUIRK_DISABLE_MECR	BIT(3) /* Disble Memory error detection */
+#define FLEXCAN_QUIRK_ENABLE_EACEN_RRS	BIT(3) /* Enable EACEN and RRS bit in ctrl2 */
+#define FLEXCAN_QUIRK_DISABLE_MECR	BIT(4) /* Disble Memory error detection */
 
 /* Structure of the message buffer */
 struct flexcan_mb {
@@ -278,11 +279,12 @@ static const struct flexcan_devtype_data fsl_p1010_devtype_data = {
 static const struct flexcan_devtype_data fsl_imx28_devtype_data;
 
 static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
-	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG,
+	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS,
 };
 
 static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
-	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_DISABLE_MECR,
+	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
+		FLEXCAN_QUIRK_DISABLE_MECR,
 };
 
 static const struct can_bittiming_const flexcan_bittiming_const = {
@@ -864,6 +866,12 @@ static int flexcan_chip_start(struct net_device *dev)
 	netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
 	flexcan_write(reg_ctrl, &regs->ctrl);
 
+	if ((priv->devtype_data->quirks & FLEXCAN_QUIRK_ENABLE_EACEN_RRS)) {
+		reg_ctrl2 = flexcan_read(&regs->ctrl2);
+		reg_ctrl2 |= FLEXCAN_CTRL2_EACEN | FLEXCAN_CTRL2_RRS;
+		flexcan_write(reg_ctrl2, &regs->ctrl2);
+	}
+
 	/* clear and invalidate all mailboxes first */
 	for (i = priv->tx_mb_idx; i < ARRAY_SIZE(regs->mb); i++) {
 		flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE,
-- 
2.8.1

[PATCH v2 11/12] can: flexcan: add support for timestamp based rx-offload

[Marc Kleine-Budde]

The flexcan IP core has 64 mailboxes. For now they are configured for
RX as a hardware FIFO. This FIFO has a fixed depth of 6 CAN frames. In
some high load scenarios it turns out thas this buffer is too small.

In order to have a buffer larger than the 6 frames FIFO, this patch adds
support for timestamp based offloading via the generic rx-offload
infrastructure.

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

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index c94a65dbb1c2..9867a5c9c9d4 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -3,7 +3,8 @@
  *
  * Copyright (c) 2005-2006 Varma Electronics Oy
  * Copyright (c) 2009 Sascha Hauer, Pengutronix
- * Copyright (c) 2010 Marc Kleine-Budde, Pengutronix
+ * Copyright (c) 2010-2016 Pengutronix, Marc Kleine-Budde <kernel@pengutronix.de>
+ * Copyright (c) 2014 David Jander, Protonic Holland
  *
  * Based on code originally by Andrey Volkov <avolkov@varma-el.com>
  *
@@ -59,6 +60,7 @@
 #define FLEXCAN_MCR_IRMQ		BIT(16)
 #define FLEXCAN_MCR_LPRIO_EN		BIT(13)
 #define FLEXCAN_MCR_AEN			BIT(12)
+/* MCR_MAXMB: maximum used MBs is MAXMB + 1 */
 #define FLEXCAN_MCR_MAXMB(x)		((x) & 0x7f)
 #define FLEXCAN_MCR_IDAM_A		(0x0 << 8)
 #define FLEXCAN_MCR_IDAM_B		(0x1 << 8)
@@ -146,12 +148,18 @@
 /* Errata ERR005829 step7: Reserve first valid MB */
 #define FLEXCAN_TX_MB_RESERVED_OFF_FIFO	8
 #define FLEXCAN_TX_MB_OFF_FIFO		9
+#define FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP	0
+#define FLEXCAN_TX_MB_OFF_TIMESTAMP		1
+#define FLEXCAN_RX_MB_OFF_TIMESTAMP_FIRST	(FLEXCAN_TX_MB_OFF_TIMESTAMP + 1)
+#define FLEXCAN_RX_MB_OFF_TIMESTAMP_LAST	63
 #define FLEXCAN_IFLAG_MB(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)
 
 /* 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)
@@ -189,6 +197,7 @@
 #define FLEXCAN_QUIRK_DISABLE_RXFG	BIT(2) /* Disable RX FIFO Global mask */
 #define FLEXCAN_QUIRK_ENABLE_EACEN_RRS	BIT(3) /* Enable EACEN and RRS bit in ctrl2 */
 #define FLEXCAN_QUIRK_DISABLE_MECR	BIT(4) /* Disble Memory error detection */
+#define FLEXCAN_QUIRK_USE_OFF_TIMESTAMP	BIT(5) /* Use timestamp based offloading */
 
 /* Structure of the message buffer */
 struct flexcan_mb {
@@ -265,6 +274,7 @@ struct flexcan_priv {
 	u32 poll_esr;			/* used in flexcan_poll_bus_err */
 	u32 reg_ctrl_default;
 	u32 reg_imask1_default;
+	u32 reg_imask2_default;
 
 	struct clk *clk_ipg;
 	struct clk *clk_per;
@@ -660,11 +670,32 @@ static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload,
 	struct flexcan_mb __iomem *mb = &regs->mb[n];
 	u32 reg_ctrl, reg_id, reg_iflag1;
 
-	reg_iflag1 = flexcan_read(&regs->iflag1);
-	if (!(reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE))
-		return 0;
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
+		u32 code;
+
+		do {
+			reg_ctrl = flexcan_read(&mb->can_ctrl);
+		} while (reg_ctrl & FLEXCAN_MB_CODE_RX_BUSY_BIT);
+
+		/* is this MB empty? */
+		code = reg_ctrl & FLEXCAN_MB_CODE_MASK;
+		if ((code != FLEXCAN_MB_CODE_RX_FULL) &&
+		    (code != FLEXCAN_MB_CODE_RX_OVERRUN))
+			return 0;
+
+		if (code == FLEXCAN_MB_CODE_RX_OVERRUN) {
+			/* This MB was overrun, we lost data */
+			offload->dev->stats.rx_over_errors++;
+			offload->dev->stats.rx_errors++;
+		}
+	} else {
+		reg_iflag1 = flexcan_read(&regs->iflag1);
+		if (!(reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE))
+			return 0;
+
+		reg_ctrl = flexcan_read(&mb->can_ctrl);
+	}
 
-	reg_ctrl = flexcan_read(&mb->can_ctrl);
 	/* increase timstamp to full 32 bit */
 	*timestamp = reg_ctrl << 16;
 
@@ -682,8 +713,16 @@ static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload,
 	*(__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);
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
+		/* Clear IRQ */
+		if (n < 32)
+			flexcan_write(BIT(n), &regs->iflag1);
+		else
+			flexcan_write(BIT(n - 32), &regs->iflag2);
+	} else {
+		flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
+		flexcan_read(&regs->timer);
+	}
 
 	return 1;
 }
@@ -696,6 +735,18 @@ static void flexcan_poll_error_interrupts_enable(struct can_rx_offload *offload)
 	flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
 }
 
+static inline u64 flexcan_read_reg_iflag_rx(struct flexcan_priv *priv)
+{
+	struct flexcan_regs __iomem *regs = priv->regs;
+	u32 iflag1, iflag2;
+
+	iflag2 = flexcan_read(&regs->iflag2) & priv->reg_imask2_default;
+	iflag1 = flexcan_read(&regs->iflag1) & priv->reg_imask1_default &
+		~FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
+
+	return (u64)iflag2 << 32 | iflag1;
+}
+
 static irqreturn_t flexcan_irq(int irq, void *dev_id)
 {
 	struct net_device *dev = dev_id;
@@ -724,14 +775,26 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	}
 
 	/* reception interrupt */
-	if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE)
-		can_rx_offload_irq_offload_fifo(&priv->offload);
-
-	/* 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++;
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
+		u64 reg_iflag;
+		int ret;
+
+		while ((reg_iflag = flexcan_read_reg_iflag_rx(priv))) {
+			ret = can_rx_offload_irq_offload_timestamp(&priv->offload,
+								   reg_iflag);
+			if (!ret)
+				break;
+		}
+	} else {
+		if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE)
+			can_rx_offload_irq_offload_fifo(&priv->offload);
+
+		/* 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 */
@@ -826,10 +889,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 |
-		FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN | FLEXCAN_MCR_SRX_DIS |
-		FLEXCAN_MCR_IRMQ | FLEXCAN_MCR_IDAM_C |
-		FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
+	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | FLEXCAN_MCR_SUPV |
+		FLEXCAN_MCR_WRN_EN | FLEXCAN_MCR_SRX_DIS | FLEXCAN_MCR_IRMQ |
+		FLEXCAN_MCR_IDAM_C;
+
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
+		reg_mcr &= ~FLEXCAN_MCR_FEN;
+		reg_mcr |= FLEXCAN_MCR_MAXMB(priv->offload.mb_last);
+	} else {
+		reg_mcr |= FLEXCAN_MCR_FEN |
+			FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
+	}
 	netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
 	flexcan_write(reg_mcr, &regs->mcr);
 
@@ -878,6 +948,12 @@ static int flexcan_chip_start(struct net_device *dev)
 			      &regs->mb[i].can_ctrl);
 	}
 
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
+		for (i = priv->offload.mb_first; i <= priv->offload.mb_last; i++)
+			flexcan_write(FLEXCAN_MB_CODE_RX_EMPTY,
+				      &regs->mb[i].can_ctrl);
+	}
+
 	/* Errata ERR005829: mark first TX mailbox as INACTIVE */
 	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
 		      &priv->tx_mb_reserved->can_ctrl);
@@ -936,6 +1012,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	disable_irq(dev->irq);
 	flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
 	flexcan_write(priv->reg_imask1_default, &regs->imask1);
+	flexcan_write(priv->reg_imask2_default, &regs->imask2);
 	enable_irq(dev->irq);
 
 	/* print chip status */
@@ -965,6 +1042,7 @@ static void flexcan_chip_stop(struct net_device *dev)
 	flexcan_chip_disable(priv);
 
 	/* Disable all interrupts */
+	flexcan_write(0, &regs->imask2);
 	flexcan_write(0, &regs->imask1);
 	flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
 		      &regs->ctrl);
@@ -1097,8 +1175,9 @@ static int register_flexcandev(struct net_device *dev)
 	flexcan_write(reg, &regs->mcr);
 
 	/* Currently we only support newer versions of this core
-	 * featuring a RX FIFO. Older cores found on some Coldfire
-	 * derivates are not yet supported.
+	 * featuring a RX hardware FIFO (although this driver doesn't
+	 * make use of it on some cores). Older cores, found on some
+	 * Coldfire derivates are not tested.
 	 */
 	reg = flexcan_read(&regs->mcr);
 	if (!(reg & FLEXCAN_MCR_FEN)) {
@@ -1222,13 +1301,17 @@ static int flexcan_probe(struct platform_device *pdev)
 	priv->devtype_data = devtype_data;
 	priv->reg_xceiver = reg_xceiver;
 
-	priv->tx_mb_idx = FLEXCAN_TX_MB_OFF_FIFO;
-	priv->tx_mb_reserved = &regs->mb[FLEXCAN_TX_MB_RESERVED_OFF_FIFO];
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
+		priv->tx_mb_idx = FLEXCAN_TX_MB_OFF_TIMESTAMP;
+		priv->tx_mb_reserved = &regs->mb[FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP];
+	} else {
+		priv->tx_mb_idx = FLEXCAN_TX_MB_OFF_FIFO;
+		priv->tx_mb_reserved = &regs->mb[FLEXCAN_TX_MB_RESERVED_OFF_FIFO];
+	}
 	priv->tx_mb = &regs->mb[priv->tx_mb_idx];
 
-	priv->reg_imask1_default = FLEXCAN_IFLAG_RX_FIFO_OVERFLOW |
-		FLEXCAN_IFLAG_RX_FIFO_AVAILABLE |
-		FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
+	priv->reg_imask1_default = FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
+	priv->reg_imask2_default = 0;
 
 	priv->offload.poll_pre_read = flexcan_poll_state;
 	priv->offload.poll_post_read = flexcan_poll_bus_err;
@@ -1236,7 +1319,23 @@ static int flexcan_probe(struct platform_device *pdev)
 		flexcan_poll_error_interrupts_enable;
 	priv->offload.mailbox_read = flexcan_mailbox_read;
 
-	err = can_rx_offload_add_fifo(dev, &priv->offload, FLEXCAN_NAPI_WEIGHT);
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
+		u64 imask;
+
+		priv->offload.mb_first = FLEXCAN_RX_MB_OFF_TIMESTAMP_FIRST;
+		priv->offload.mb_last = FLEXCAN_RX_MB_OFF_TIMESTAMP_LAST;
+
+		imask = ~0LLU >> (64 + priv->offload.mb_first - priv->offload.mb_last - 1)
+			      << priv->offload.mb_first;
+		priv->reg_imask1_default |= imask;
+		priv->reg_imask2_default |= imask >> 32;
+
+		err = can_rx_offload_add_timestamp(dev, &priv->offload);
+	} else {
+		priv->reg_imask1_default |= FLEXCAN_IFLAG_RX_FIFO_OVERFLOW |
+			FLEXCAN_IFLAG_RX_FIFO_AVAILABLE;
+		err = can_rx_offload_add_fifo(dev, &priv->offload, FLEXCAN_NAPI_WEIGHT);
+	}
 	if (err)
 		goto failed_offload;
 
-- 
2.8.1

[PATCH v2 12/12] can: flexcan: switch imx6 and vf610 to timestamp based offloading

[Marc Kleine-Budde]

This patch switches the imx6 and vf610 based SoCs from the hardware FIFO
to the timestamp based rx offloading.

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

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 9867a5c9c9d4..d020cc357adf 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -289,12 +289,13 @@ static const struct flexcan_devtype_data fsl_p1010_devtype_data = {
 static const struct flexcan_devtype_data fsl_imx28_devtype_data;
 
 static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
-	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS,
+	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
+		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP,
 };
 
 static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
 	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
-		FLEXCAN_QUIRK_DISABLE_MECR,
+		FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_OFF_TIMESTAMP,
 };
 
 static const struct can_bittiming_const flexcan_bittiming_const = {
-- 
2.8.1

Re: [PATCH v2 00/12] can: rx-offload: add implmentation and switch flexcan driver to use it

[Mirza Krak]

2016-07-04 20:32 GMT+02:00 Marc Kleine-Budde <mkl@pengutronix.de>:
> Hello,
>
> this patch takes up the idea to read the CAN frames in IRQ context and send
> them later in NAPI. The first two patches add each an offloading scheme.
>
> The first one is for hardware FIFO based cores, like the flexcan in FIFO mode.
> The second one requires mailboxes with timestamps. The mailboxes are read and
> sorted by timestamp in IRQ context, sending is done later in NAPI aswell.
>
> The remaining patches modify the flexcan driver to make use of it. imx6 and
> vf610 SoCs can make use of the 64 mailbox software FIFO, while older SoCs still
> use flexcan's 6 mailbox deep hardware FIFO.
>
> Testing on any flexcan core is highly appreciated.

Did a test run on v2 series on my VF610 SOC.

I do not see any problems with order anymore [1] and it recovers fine
from "ERROFRAMES" [2]. No other issues to report.

Tested-by: Mirza Krak <mirza.krak@hostmobility.com>

[1]. https://www.marc.info/?l=linux-can&m=146286890307194&w=2
[2]. https://www.marc.info/?l=linux-can&m=146374389028083&w=2
-- 
Med Vänliga Hälsningar / Best Regards

*******************************************************************
Mirza Krak
Host Mobility AB
mirza.krak@hostmobility.com
Anders Personsgatan 12, 416 64 Göteborg
Sweden
http://www.hostmobility.com
Direct: +46 31 31 32 704
Phone: +46 31 31 32 700
Fax: +46 31 80 67 51
Mobile: +46 730 28 06 22
*******************************************************************

Re: [PATCH v2 00/12] can: rx-offload: add implmentation and switch flexcan driver to use it

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1344 bytes --]

On 07/13/2016 09:28 AM, Mirza Krak wrote:
> 2016-07-04 20:32 GMT+02:00 Marc Kleine-Budde <mkl@pengutronix.de>:
>> Hello,
>>
>> this patch takes up the idea to read the CAN frames in IRQ context and send
>> them later in NAPI. The first two patches add each an offloading scheme.
>>
>> The first one is for hardware FIFO based cores, like the flexcan in FIFO mode.
>> The second one requires mailboxes with timestamps. The mailboxes are read and
>> sorted by timestamp in IRQ context, sending is done later in NAPI aswell.
>>
>> The remaining patches modify the flexcan driver to make use of it. imx6 and
>> vf610 SoCs can make use of the 64 mailbox software FIFO, while older SoCs still
>> use flexcan's 6 mailbox deep hardware FIFO.
>>
>> Testing on any flexcan core is highly appreciated.
> 
> Did a test run on v2 series on my VF610 SOC.
> 
> I do not see any problems with order anymore [1] and it recovers fine
> from "ERROFRAMES" [2]. No other issues to report.
> 
> Tested-by: Mirza Krak <mirza.krak@hostmobility.com>

Thanks.

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

Re: [PATCH v2 00/12] can: rx-offload: add implmentation and switch flexcan driver to use it

[Holger Schurig]

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

Late answer :-)

This driver was tested with Linux 4.7.2 an on i.MX6Q device. Not by me,
I don't even have their exact test software. I just get the reports ...


Using some external tool (maybe Kvaser?) they generated 80% busload at
500 kbps, the driver survived for 3 days without any data loss. So,
compared with the current in-kernel driver, this is a definitive
improvement!



Then they did same CAN ping-ping test with reboots and wrote: "Out of
about 16,000 reboots we had one failure. This failed both CAN interfaces
and the logs show that the PTX-C received all CAN messages and returned
them, but the test machine for some reason did not receive any CAN
messages."

I don't have any idea why Linux thought it was sending but the receiver
didn't receive anything. And I'm no CAN expert, not at all. Wouldn't I
get an error condition on the sending side?   How would a program get
this error, is it a return value to sendto()

Holger

Re: [PATCH v2 00/12] can: rx-offload: add implmentation and switch flexcan driver to use it

[Holger Schurig]

Holger Schurig <holgerschurig@gmail.com> writes:

> Then they did same CAN ping-ping test with reboots and wrote: "Out of
> about 16,000 reboots we had one failure. This failed both CAN interfaces
> and the logs show that the PTX-C received all CAN messages and returned
> them, but the test machine for some reason did not receive any CAN
> messages."

It turned out that the error was not in the FlexCAN + rx-fifo patches,
but at the receiving end.

So you can have a "Tested-By: Holger Schurig <holgerschurig@gmail.com>"
for your patch series on top of 4.7.5.

Re: [PATCH v2 00/12] can: rx-offload: add implmentation and switch flexcan driver to use it

[Alexander Stein]

Hi,

On Monday 04 July 2016 20:32:05, Marc Kleine-Budde wrote:
> this patch takes up the idea to read the CAN frames in IRQ context and send
> them later in NAPI. The first two patches add each an offloading scheme.
> 
> The first one is for hardware FIFO based cores, like the flexcan in FIFO
> mode. The second one requires mailboxes with timestamps. The mailboxes are
> read and sorted by timestamp in IRQ context, sending is done later in NAPI
> aswell.
> 
> The remaining patches modify the flexcan driver to make use of it. imx6 and
> vf610 SoCs can make use of the 64 mailbox software FIFO, while older SoCs
> still use flexcan's 6 mailbox deep hardware FIFO.
> 
> Testing on any flexcan core is highly appreciated.

We did some tests on our custom i.MX35 based board. This means we are stuck at the 6 mailbox deep hardware FIFO.
This is how our test works:
* Send 2 * 250000 CAN frames to the i.MX board, in 2 * 250 burst sizes at 1MBit/s
* Running iperf in parallel

Originally we used 3.10.95-rt102 (RT enabled) and lost CAN frames. The CAN-IRQ-Thread has been prioritized to SCHED_FIFO 91.
Then we updated to 4.6.7-rt13. Without those patches in either case, RT enabled or not, the hardware FIFO still overran.
When this patchset is applied the non-RT configuration successfully received all CAN frames without any drop. Unfortunately when RT is enabled there are still hardware overruns. A first try on the newly released 4.6.7-rt14 improved the situation, there are less overruns, but they still exist.

In summary the non-RT case seems fine now, but I wonder what causes the delays on RT to the CAN-IRQ-Thread which seem to be about 500us (bus time of 6 4Byte CAN frames).

Best regards,
Alexander

Re: [PATCH v2 00/12] can: rx-offload: add implmentation and switch flexcan driver to use it

[David Jander]

On Tue, 04 Oct 2016 13:57:33 +0200
Alexander Stein <alexander.stein@systec-electronic.com> wrote:

> Hi,
> 
> On Monday 04 July 2016 20:32:05, Marc Kleine-Budde wrote:
> > this patch takes up the idea to read the CAN frames in IRQ context and send
> > them later in NAPI. The first two patches add each an offloading scheme.
> > 
> > The first one is for hardware FIFO based cores, like the flexcan in FIFO
> > mode. The second one requires mailboxes with timestamps. The mailboxes are
> > read and sorted by timestamp in IRQ context, sending is done later in NAPI
> > aswell.
> > 
> > The remaining patches modify the flexcan driver to make use of it. imx6 and
> > vf610 SoCs can make use of the 64 mailbox software FIFO, while older SoCs
> > still use flexcan's 6 mailbox deep hardware FIFO.
> > 
> > Testing on any flexcan core is highly appreciated.  
> 
> We did some tests on our custom i.MX35 based board. This means we are stuck
> at the 6 mailbox deep hardware FIFO. This is how our test works:
> * Send 2 * 250000 CAN frames to the i.MX board, in 2 * 250 burst sizes at
> 1MBit/s
> * Running iperf in parallel
> 
> Originally we used 3.10.95-rt102 (RT enabled) and lost CAN frames. The
> CAN-IRQ-Thread has been prioritized to SCHED_FIFO 91. Then we updated to
> 4.6.7-rt13. Without those patches in either case, RT enabled or not, the
> hardware FIFO still overran. When this patchset is applied the non-RT
> configuration successfully received all CAN frames without any drop.
> Unfortunately when RT is enabled there are still hardware overruns. A first
> try on the newly released 4.6.7-rt14 improved the situation, there are less
> overruns, but they still exist.
> 
> In summary the non-RT case seems fine now, but I wonder what causes the
> delays on RT to the CAN-IRQ-Thread which seem to be about 500us (bus time of
> 6 4Byte CAN frames).

Thanks for testing!
Well, in -RT I guess anything with a higher priority than the CAN-IRQ can
cause a delay... but since going from -rt13 to -rt14 improved the situation,
this sounds like bugs in the -rt code. Are you running any code in SCHED_FIFO
that you don't run in the non-RT case?
It's nevertheless very nice to see that for non-RT, you got from losing
messages to not losing any message with this patchset, even on a lowly i.MX35
at 1Mbit/s! This proves that the patchset is certainly worth it.

Best regards,

-- 
David Jander
Protonic Holland.

Re: [PATCH v2 00/12] can: rx-offload: add implmentation and switch flexcan driver to use it

[Alexander Stein]

Hi David,

On Tuesday 04 October 2016 14:33:15, David Jander wrote:
> > In summary the non-RT case seems fine now, but I wonder what causes the
> > delays on RT to the CAN-IRQ-Thread which seem to be about 500us (bus time
> > of 6 4Byte CAN frames).
> 
> Thanks for testing!
> Well, in -RT I guess anything with a higher priority than the CAN-IRQ can
> cause a delay... but since going from -rt13 to -rt14 improved the situation,
> this sounds like bugs in the -rt code. Are you running any code in
> SCHED_FIFO that you don't run in the non-RT case?

AFAIK no userspace process is put into SCHED_FIFO and only the priority of 
CAN-IRQ-Thread is increased to 91. Userspace runs with unchanged priority. So 
yes I guess there is still problem in -rt code.

> It's nevertheless very nice to see that for non-RT, you got from losing
> messages to not losing any message with this patchset, even on a lowly
> i.MX35 at 1Mbit/s! This proves that the patchset is certainly worth it.

I would expect the same from running with RT enabled, especially as the CAN-
IRQ-Thread has SCHED_FIFO wuth priority 91. There shouldn't be much code which 
can delay that thread.

Best regards,
Alexander

Re: [PATCH v2 00/12] can: rx-offload: add implmentation and switch flexcan driver to use it

[Alexander Stein]

On Monday 04 July 2016 20:32:05, Marc Kleine-Budde wrote:
> Hello,
> 
> this patch takes up the idea to read the CAN frames in IRQ context and send
> them later in NAPI. The first two patches add each an offloading scheme.
> 
> The first one is for hardware FIFO based cores, like the flexcan in FIFO
> mode. The second one requires mailboxes with timestamps. The mailboxes are
> read and sorted by timestamp in IRQ context, sending is done later in NAPI
> aswell.
> 
> The remaining patches modify the flexcan driver to make use of it. imx6 and
> vf610 SoCs can make use of the 64 mailbox software FIFO, while older SoCs
> still use flexcan's 6 mailbox deep hardware FIFO.
> 
> Testing on any flexcan core is highly appreciated.

What is the status of this patchset?

Best regards,
Alexander