[PATCH v2 0/3] can: m_can: stabilise peripheral m_can RX and TX

[PATCH v2 0/3] can: m_can: stabilise peripheral m_can RX and TX

[Torin Cooper-Bennun]

In this patch series, rx-offload is utilised in the m_can driver,
instead of sending skbs directly from a threaded IRQ using
netif_receive_skb() (which is meant to be called only from a softirq
context). This should fix the major stability problems with M_CAN
peripherals (tcan4x5x being the only practical example at the moment).

I have tested this with a Raspberry Pi 3 Model B and a TCAN4550. The
driver no longer crashes outright, and TX works consistently and echoes
correctly. RX is also much more stable, but I think more testing is
required with different hardware to really ensure stability.

There is definitely at least one outstanding issue. In my tests, if the
TCAN4550 encounters bus activity immediately after init, such that RX
FIFO 0 is already full by the time the IRQ handler is first called, then
in the initial round of RX polling, the frames are apparently read out
empty (DLC = 0, ID = 0). After this point, RX works as expected. I
assume there's a data race hidden in the init process somewhere, but I
have failed to find it!

On my hardware setup, I still have occasional incorrectly-read-out
frames (e.g. DLC wrong, or the odd 4 data bytes reading 0), but I am yet
to rule out SPI signal problems, nor Raspberry Pi SPI hardware problems.
In any case, these problems existed in my setup before this patch
series.

I would be grateful if someone were able to test these changes with a
different hardware setup, and also with non-peripheral M_CAN.

Many thanks,
Torin

----- Version changelog -----

v2
 - Use GENMASK, FIELD_PREP, FIELD_GET for new bitmasks
 - Retain existing skb handling for non-peripheral M_CAN as it already
   uses napi for RX
 - Add timestamp param to m_can_tx_update_stats

[PATCH v2 1/3] can: m_can: add infrastructure for internal timestamps

[Torin Cooper-Bennun]

Add infrastucture to allow internal timestamps from the M_CAN to be
configured and retrieved.

Signed-off-by: Torin Cooper-Bennun <torin@maxiluxsystems.com>
---
 drivers/net/can/m_can/m_can.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 3752520a7d4b..ce1fef95d34b 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -8,6 +8,7 @@
  * https://github.com/linux-can/can-doc/tree/master/m_can
  */
 
+#include <linux/bitfield.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
@@ -148,6 +149,16 @@ enum m_can_reg {
 #define NBTP_NTSEG2_SHIFT	0
 #define NBTP_NTSEG2_MASK	(0x7f << NBTP_NTSEG2_SHIFT)
 
+/* Timestamp Counter Configuration Register (TSCC) */
+#define TSCC_TCP_MASK		GENMASK(19, 16)
+#define TSCC_TSS_MASK		GENMASK(1, 0)
+#define TSCC_TSS_DISABLE	0x0
+#define TSCC_TSS_INTERNAL	0x1
+#define TSCC_TSS_EXTERNAL	0x2
+
+/* Timestamp Counter Value Register (TSCV) */
+#define TSCV_TSC_MASK		GENMASK(15, 0)
+
 /* Error Counter Register(ECR) */
 #define ECR_RP			BIT(15)
 #define ECR_REC_SHIFT		8
@@ -302,6 +313,7 @@ enum m_can_reg {
 #define RX_BUF_ANMF		BIT(31)
 #define RX_BUF_FDF		BIT(21)
 #define RX_BUF_BRS		BIT(20)
+#define RX_BUF_RXTS_MASK	GENMASK(15, 0)
 
 /* Tx Buffer Element */
 /* T0 */
@@ -319,6 +331,7 @@ enum m_can_reg {
 /* E1 */
 #define TX_EVENT_MM_SHIFT	TX_BUF_MM_SHIFT
 #define TX_EVENT_MM_MASK	(0xff << TX_EVENT_MM_SHIFT)
+#define TX_EVENT_TXTS_MASK	GENMASK(15, 0)
 
 static inline u32 m_can_read(struct m_can_classdev *cdev, enum m_can_reg reg)
 {
@@ -413,6 +426,20 @@ static inline void m_can_disable_all_interrupts(struct m_can_classdev *cdev)
 	m_can_write(cdev, M_CAN_ILE, 0x0);
 }
 
+/* Retrieve internal timestamp counter from TSCV.TSC, and shift it to 32-bit
+ * width.
+ */
+static u32 m_can_get_timestamp(struct m_can_classdev *cdev)
+{
+	u32 tscv;
+	u32 tsc;
+
+	tscv = m_can_read(cdev, M_CAN_TSCV);
+	tsc = FIELD_GET(TSCV_TSC_MASK, tscv);
+
+	return (tsc << 16);
+}
+
 static void m_can_clean(struct net_device *net)
 {
 	struct m_can_classdev *cdev = netdev_priv(net);
-- 
2.30.1

[PATCH v2 2/3] can: m_can: m_can_chip_config(): enable and configure internal timestamps

[Torin Cooper-Bennun]

This is a prerequisite for transitioning the m_can driver to rx-offload,
which works best with TX and RX timestamps.

The timestamps provided by M_CAN are 16-bit, timed according to the
nominal bit timing, and may be prescaled by a multiplier up to 16. We
choose the highest prescalar so that the timestamp wraps every 2^20 bit
times, or 209 ms at a bus speed of 5 Mbit/s. Timestamps will have a
precision of 16 bit times.

Signed-off-by: Torin Cooper-Bennun <torin@maxiluxsystems.com>
---
 drivers/net/can/m_can/m_can.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index ce1fef95d34b..e61b7e186d80 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1138,6 +1138,7 @@ static int m_can_set_bittiming(struct net_device *dev)
  *		- >= v3.1.x: TX FIFO is used
  * - configure mode
  * - setup bittiming
+ * - configure timestamp generation
  */
 static void m_can_chip_config(struct net_device *dev)
 {
@@ -1249,6 +1250,10 @@ static void m_can_chip_config(struct net_device *dev)
 	/* set bittiming params */
 	m_can_set_bittiming(dev);
 
+	/* enable internal timestamp generation, with a prescalar of 16. The
+	 * prescalar is applied to the nominal bit timing */
+	m_can_write(cdev, M_CAN_TSCC, FIELD_PREP(TSCC_TCP_MASK, 0xf));
+
 	m_can_config_endisable(cdev, false);
 
 	if (cdev->ops->init)
-- 
2.30.1

[PATCH v2 3/3] can: m_can: fix periph RX path: use rx-offload to ensure skbs are sent from softirq context

[Torin Cooper-Bennun]

For peripheral devices, m_can sent skbs directly from a threaded irq
instead of from a softirq context, breaking the tcan4x5x peripheral
driver completely. This patch transitions the driver to use the
rx-offload helper for peripherals, ensuring the skbs are sent from the
correct context, with h/w timestamping to ensure correct ordering.

Signed-off-by: Torin Cooper-Bennun <torin@maxiluxsystems.com>
---
 drivers/net/can/m_can/m_can.c | 109 +++++++++++++++++++++++++++++-----
 drivers/net/can/m_can/m_can.h |   2 +
 2 files changed, 97 insertions(+), 14 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index e61b7e186d80..80519b5cec31 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -457,6 +457,20 @@ static void m_can_clean(struct net_device *net)
 	}
 }
 
+/* For peripherals, pass skb to rx-offload, which will push skb from napi. For
+ * non-peripherals, RX is done in napi already, so push directly. timestamp is
+ * used to ensure good skb ordering in rx-offload and is ignored
+ * for non-peripherals */
+static void m_can_receive_skb(struct m_can_classdev *cdev,
+			      struct sk_buff *skb,
+			      u32 timestamp)
+{
+	if (cdev->is_peripheral)
+		can_rx_offload_queue_sorted(&cdev->offload, skb, timestamp);
+	else
+		netif_receive_skb(skb);
+}
+
 static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
 {
 	struct net_device_stats *stats = &dev->stats;
@@ -464,6 +478,7 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
 	struct canfd_frame *cf;
 	struct sk_buff *skb;
 	u32 id, fgi, dlc;
+	u32 timestamp = 0;
 	int i;
 
 	/* calculate the fifo get index for where to read data */
@@ -512,7 +527,9 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
 	stats->rx_packets++;
 	stats->rx_bytes += cf->len;
 
-	netif_receive_skb(skb);
+	timestamp = FIELD_GET(RX_BUF_RXTS_MASK, dlc);
+
+	m_can_receive_skb(cdev, skb, timestamp);
 }
 
 static int m_can_do_rx_poll(struct net_device *dev, int quota)
@@ -546,9 +563,11 @@ static int m_can_do_rx_poll(struct net_device *dev, int quota)
 
 static int m_can_handle_lost_msg(struct net_device *dev)
 {
+	struct m_can_classdev *cdev = netdev_priv(dev);
 	struct net_device_stats *stats = &dev->stats;
 	struct sk_buff *skb;
 	struct can_frame *frame;
+	u32 timestamp = 0;
 
 	netdev_err(dev, "msg lost in rxf0\n");
 
@@ -562,7 +581,10 @@ static int m_can_handle_lost_msg(struct net_device *dev)
 	frame->can_id |= CAN_ERR_CRTL;
 	frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
 
-	netif_receive_skb(skb);
+	if (cdev->is_peripheral)
+		timestamp = m_can_get_timestamp(cdev);
+
+	m_can_receive_skb(cdev, skb, timestamp);
 
 	return 1;
 }
@@ -574,6 +596,7 @@ static int m_can_handle_lec_err(struct net_device *dev,
 	struct net_device_stats *stats = &dev->stats;
 	struct can_frame *cf;
 	struct sk_buff *skb;
+	u32 timestamp = 0;
 
 	cdev->can.can_stats.bus_error++;
 	stats->rx_errors++;
@@ -619,7 +642,11 @@ static int m_can_handle_lec_err(struct net_device *dev,
 
 	stats->rx_packets++;
 	stats->rx_bytes += cf->len;
-	netif_receive_skb(skb);
+
+	if (cdev->is_peripheral)
+		timestamp = m_can_get_timestamp(cdev);
+
+	m_can_receive_skb(cdev, skb, timestamp);
 
 	return 1;
 }
@@ -677,6 +704,7 @@ static int m_can_handle_state_change(struct net_device *dev,
 	struct sk_buff *skb;
 	struct can_berr_counter bec;
 	unsigned int ecr;
+	u32 timestamp;
 
 	switch (new_state) {
 	case CAN_STATE_ERROR_WARNING:
@@ -738,7 +766,11 @@ static int m_can_handle_state_change(struct net_device *dev,
 
 	stats->rx_packets++;
 	stats->rx_bytes += cf->len;
-	netif_receive_skb(skb);
+
+	if (cdev->is_peripheral)
+		timestamp = m_can_get_timestamp(cdev);
+
+	m_can_receive_skb(cdev, skb, timestamp);
 
 	return 1;
 }
@@ -803,6 +835,7 @@ static int m_can_handle_protocol_error(struct net_device *dev, u32 irqstatus)
 	struct m_can_classdev *cdev = netdev_priv(dev);
 	struct can_frame *cf;
 	struct sk_buff *skb;
+	u32 timestamp;
 
 	/* propagate the error condition to the CAN stack */
 	skb = alloc_can_err_skb(dev, &cf);
@@ -824,7 +857,11 @@ static int m_can_handle_protocol_error(struct net_device *dev, u32 irqstatus)
 		netdev_dbg(dev, "allocation of skb failed\n");
 		return 0;
 	}
-	netif_receive_skb(skb);
+
+	if (cdev->is_peripheral)
+		timestamp = m_can_get_timestamp(cdev);
+
+	m_can_receive_skb(cdev, skb, timestamp);
 
 	return 1;
 }
@@ -925,6 +962,28 @@ static int m_can_poll(struct napi_struct *napi, int quota)
 	return work_done;
 }
 
+/* Echo tx skb and update net stats. Peripherals use rx-offload for echo.
+ * timestamp is used for peripherals to ensure correct ordering by rx-offload,
+ * and is ignored for non-peripherals */
+static void m_can_tx_update_stats(struct m_can_classdev *cdev,
+				  unsigned int msg_mark,
+				  u32 timestamp)
+{
+	struct net_device *dev = cdev->net;
+	struct net_device_stats *stats = &dev->stats;
+
+	if (cdev->is_peripheral)
+		stats->tx_bytes +=
+			can_rx_offload_get_echo_skb(&cdev->offload,
+						    msg_mark,
+						    timestamp,
+						    NULL);
+	else
+		stats->tx_bytes += can_get_echo_skb(dev, msg_mark, NULL);
+
+	stats->tx_packets++;
+}
+
 static void m_can_echo_tx_event(struct net_device *dev)
 {
 	u32 txe_count = 0;
@@ -934,7 +993,6 @@ static void m_can_echo_tx_event(struct net_device *dev)
 	unsigned int msg_mark;
 
 	struct m_can_classdev *cdev = netdev_priv(dev);
-	struct net_device_stats *stats = &dev->stats;
 
 	/* read tx event fifo status */
 	m_can_txefs = m_can_read(cdev, M_CAN_TXEFS);
@@ -944,21 +1002,24 @@ static void m_can_echo_tx_event(struct net_device *dev)
 
 	/* Get and process all sent elements */
 	for (i = 0; i < txe_count; i++) {
+		u32 txe;
+		u32 timestamp;
+
 		/* retrieve get index */
 		fgi = (m_can_read(cdev, M_CAN_TXEFS) & TXEFS_EFGI_MASK) >>
 			TXEFS_EFGI_SHIFT;
 
-		/* get message marker */
-		msg_mark = (m_can_txe_fifo_read(cdev, fgi, 4) &
-			    TX_EVENT_MM_MASK) >> TX_EVENT_MM_SHIFT;
+		/* get message marker, timestamp */
+		txe = m_can_txe_fifo_read(cdev, fgi, 4);
+		msg_mark = (txe & TX_EVENT_MM_MASK) >> TX_EVENT_MM_SHIFT;
+		timestamp = FIELD_GET(TX_EVENT_TXTS_MASK, txe);
 
 		/* ack txe element */
 		m_can_write(cdev, M_CAN_TXEFA, (TXEFA_EFAI_MASK &
 						(fgi << TXEFA_EFAI_SHIFT)));
 
 		/* update stats */
-		stats->tx_bytes += can_get_echo_skb(dev, msg_mark, NULL);
-		stats->tx_packets++;
+		m_can_tx_update_stats(cdev, msg_mark, timestamp);
 	}
 }
 
@@ -966,7 +1027,6 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
 {
 	struct net_device *dev = (struct net_device *)dev_id;
 	struct m_can_classdev *cdev = netdev_priv(dev);
-	struct net_device_stats *stats = &dev->stats;
 	u32 ir;
 
 	if (pm_runtime_suspended(cdev->dev))
@@ -999,8 +1059,12 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
 	if (cdev->version == 30) {
 		if (ir & IR_TC) {
 			/* Transmission Complete Interrupt*/
-			stats->tx_bytes += can_get_echo_skb(dev, 0, NULL);
-			stats->tx_packets++;
+			u32 timestamp = 0;
+
+			if (cdev->is_peripheral)
+				timestamp = m_can_get_timestamp(cdev);
+			m_can_tx_update_stats(cdev, 0, timestamp);
+
 			can_led_event(dev, CAN_LED_EVENT_TX);
 			netif_wake_queue(dev);
 		}
@@ -1461,6 +1525,9 @@ static int m_can_close(struct net_device *dev)
 		cdev->tx_wq = NULL;
 	}
 
+	if (cdev->is_peripheral)
+		can_rx_offload_disable(&cdev->offload);
+
 	close_candev(dev);
 	can_led_event(dev, CAN_LED_EVENT_STOP);
 
@@ -1659,6 +1726,9 @@ static int m_can_open(struct net_device *dev)
 		goto exit_disable_clks;
 	}
 
+	if (cdev->is_peripheral)
+		can_rx_offload_enable(&cdev->offload);
+
 	/* register interrupt handler */
 	if (cdev->is_peripheral) {
 		cdev->tx_skb = NULL;
@@ -1700,6 +1770,8 @@ static int m_can_open(struct net_device *dev)
 	if (cdev->is_peripheral)
 		destroy_workqueue(cdev->tx_wq);
 out_wq_fail:
+	if (cdev->is_peripheral)
+		can_rx_offload_disable(&cdev->offload);
 	close_candev(dev);
 exit_disable_clks:
 	m_can_clk_stop(cdev);
@@ -1853,6 +1925,13 @@ int m_can_class_register(struct m_can_classdev *cdev)
 			return ret;
 	}
 
+	if (cdev->is_peripheral) {
+		ret = can_rx_offload_add_manual(cdev->net, &cdev->offload,
+						M_CAN_NAPI_WEIGHT);
+		if (ret)
+			goto clk_disable;
+	}
+
 	ret = m_can_dev_setup(cdev);
 	if (ret)
 		goto clk_disable;
@@ -1883,6 +1962,8 @@ EXPORT_SYMBOL_GPL(m_can_class_register);
 
 void m_can_class_unregister(struct m_can_classdev *cdev)
 {
+	if (cdev->is_peripheral)
+		can_rx_offload_del(&cdev->offload);
 	unregister_candev(cdev->net);
 }
 EXPORT_SYMBOL_GPL(m_can_class_unregister);
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index 3fda84cef351..ace071c3e58c 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -8,6 +8,7 @@
 
 #include <linux/can/core.h>
 #include <linux/can/led.h>
+#include <linux/can/rx-offload.h>
 #include <linux/completion.h>
 #include <linux/device.h>
 #include <linux/dma-mapping.h>
@@ -71,6 +72,7 @@ struct m_can_ops {
 
 struct m_can_classdev {
 	struct can_priv can;
+	struct can_rx_offload offload;
 	struct napi_struct napi;
 	struct net_device *net;
 	struct device *dev;
-- 
2.30.1

Re: [PATCH v2 3/3] can: m_can: fix periph RX path: use rx-offload to ensure skbs are sent from softirq context

[Marc Kleine-Budde]

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

On 08.03.2021 10:24:28, Torin Cooper-Bennun wrote:
> For peripheral devices, m_can sent skbs directly from a threaded irq
> instead of from a softirq context, breaking the tcan4x5x peripheral
> driver completely. This patch transitions the driver to use the
> rx-offload helper for peripherals, ensuring the skbs are sent from the
> correct context, with h/w timestamping to ensure correct ordering.
> 
> Signed-off-by: Torin Cooper-Bennun <torin@maxiluxsystems.com>
> ---
>  drivers/net/can/m_can/m_can.c | 109 +++++++++++++++++++++++++++++-----
>  drivers/net/can/m_can/m_can.h |   2 +
>  2 files changed, 97 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index e61b7e186d80..80519b5cec31 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -457,6 +457,20 @@ static void m_can_clean(struct net_device *net)
>  	}
>  }
>  
> +/* For peripherals, pass skb to rx-offload, which will push skb from napi. For
> + * non-peripherals, RX is done in napi already, so push directly. timestamp is
> + * used to ensure good skb ordering in rx-offload and is ignored
> + * for non-peripherals */

nitpick:
networking subsystem multiline comments have the closing "*/" in a
separate line. fixed.

> +static void m_can_receive_skb(struct m_can_classdev *cdev,
> +			      struct sk_buff *skb,
> +			      u32 timestamp)
> +{
> +	if (cdev->is_peripheral)
> +		can_rx_offload_queue_sorted(&cdev->offload, skb, timestamp);
> +	else
> +		netif_receive_skb(skb);
> +}
> +
>  static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
>  {
>  	struct net_device_stats *stats = &dev->stats;
> @@ -464,6 +478,7 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
>  	struct canfd_frame *cf;
>  	struct sk_buff *skb;
>  	u32 id, fgi, dlc;
> +	u32 timestamp = 0;
>  	int i;
>  
>  	/* calculate the fifo get index for where to read data */
> @@ -512,7 +527,9 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
>  	stats->rx_packets++;
>  	stats->rx_bytes += cf->len;
>  
> -	netif_receive_skb(skb);
> +	timestamp = FIELD_GET(RX_BUF_RXTS_MASK, dlc);
> +
> +	m_can_receive_skb(cdev, skb, timestamp);
>  }
>  
>  static int m_can_do_rx_poll(struct net_device *dev, int quota)
> @@ -546,9 +563,11 @@ static int m_can_do_rx_poll(struct net_device *dev, int quota)
>  
>  static int m_can_handle_lost_msg(struct net_device *dev)
>  {
> +	struct m_can_classdev *cdev = netdev_priv(dev);
>  	struct net_device_stats *stats = &dev->stats;
>  	struct sk_buff *skb;
>  	struct can_frame *frame;
> +	u32 timestamp = 0;
>  
>  	netdev_err(dev, "msg lost in rxf0\n");
>  
> @@ -562,7 +581,10 @@ static int m_can_handle_lost_msg(struct net_device *dev)
>  	frame->can_id |= CAN_ERR_CRTL;
>  	frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
>  
> -	netif_receive_skb(skb);
> +	if (cdev->is_peripheral)
> +		timestamp = m_can_get_timestamp(cdev);
> +
> +	m_can_receive_skb(cdev, skb, timestamp);
>  
>  	return 1;
>  }
> @@ -574,6 +596,7 @@ static int m_can_handle_lec_err(struct net_device *dev,
>  	struct net_device_stats *stats = &dev->stats;
>  	struct can_frame *cf;
>  	struct sk_buff *skb;
> +	u32 timestamp = 0;
>  
>  	cdev->can.can_stats.bus_error++;
>  	stats->rx_errors++;
> @@ -619,7 +642,11 @@ static int m_can_handle_lec_err(struct net_device *dev,
>  
>  	stats->rx_packets++;
>  	stats->rx_bytes += cf->len;
> -	netif_receive_skb(skb);
> +
> +	if (cdev->is_peripheral)
> +		timestamp = m_can_get_timestamp(cdev);
> +
> +	m_can_receive_skb(cdev, skb, timestamp);
>  
>  	return 1;
>  }
> @@ -677,6 +704,7 @@ static int m_can_handle_state_change(struct net_device *dev,
>  	struct sk_buff *skb;
>  	struct can_berr_counter bec;
>  	unsigned int ecr;
> +	u32 timestamp;

Better initialize = 0 here.

>  
>  	switch (new_state) {
>  	case CAN_STATE_ERROR_WARNING:
> @@ -738,7 +766,11 @@ static int m_can_handle_state_change(struct net_device *dev,
>  
>  	stats->rx_packets++;
>  	stats->rx_bytes += cf->len;
> -	netif_receive_skb(skb);
> +
> +	if (cdev->is_peripheral)
> +		timestamp = m_can_get_timestamp(cdev);
> +
> +	m_can_receive_skb(cdev, skb, timestamp);
>  
>  	return 1;
>  }
> @@ -803,6 +835,7 @@ static int m_can_handle_protocol_error(struct net_device *dev, u32 irqstatus)
>  	struct m_can_classdev *cdev = netdev_priv(dev);
>  	struct can_frame *cf;
>  	struct sk_buff *skb;
> +	u32 timestamp;

Here, too.

>  
>  	/* propagate the error condition to the CAN stack */
>  	skb = alloc_can_err_skb(dev, &cf);
> @@ -824,7 +857,11 @@ static int m_can_handle_protocol_error(struct net_device *dev, u32 irqstatus)
>  		netdev_dbg(dev, "allocation of skb failed\n");
>  		return 0;
>  	}
> -	netif_receive_skb(skb);
> +
> +	if (cdev->is_peripheral)
> +		timestamp = m_can_get_timestamp(cdev);
> +
> +	m_can_receive_skb(cdev, skb, timestamp);
>  
>  	return 1;
>  }
> @@ -925,6 +962,28 @@ static int m_can_poll(struct napi_struct *napi, int quota)
>  	return work_done;
>  }
>  
> +/* Echo tx skb and update net stats. Peripherals use rx-offload for echo.
> + * timestamp is used for peripherals to ensure correct ordering by rx-offload,
> + * and is ignored for non-peripherals */

nitpick about net comment style :)

> +static void m_can_tx_update_stats(struct m_can_classdev *cdev,
> +				  unsigned int msg_mark,
> +				  u32 timestamp)
> +{
> +	struct net_device *dev = cdev->net;
> +	struct net_device_stats *stats = &dev->stats;
> +
> +	if (cdev->is_peripheral)
> +		stats->tx_bytes +=
> +			can_rx_offload_get_echo_skb(&cdev->offload,
> +						    msg_mark,
> +						    timestamp,
> +						    NULL);
> +	else
> +		stats->tx_bytes += can_get_echo_skb(dev, msg_mark, NULL);
> +
> +	stats->tx_packets++;
> +}
> +
>  static void m_can_echo_tx_event(struct net_device *dev)
>  {
>  	u32 txe_count = 0;
> @@ -934,7 +993,6 @@ static void m_can_echo_tx_event(struct net_device *dev)
>  	unsigned int msg_mark;
>  
>  	struct m_can_classdev *cdev = netdev_priv(dev);
> -	struct net_device_stats *stats = &dev->stats;
>  
>  	/* read tx event fifo status */
>  	m_can_txefs = m_can_read(cdev, M_CAN_TXEFS);
> @@ -944,21 +1002,24 @@ static void m_can_echo_tx_event(struct net_device *dev)
>  
>  	/* Get and process all sent elements */
>  	for (i = 0; i < txe_count; i++) {
> +		u32 txe;
> +		u32 timestamp;

Here, too.

> +
>  		/* retrieve get index */
>  		fgi = (m_can_read(cdev, M_CAN_TXEFS) & TXEFS_EFGI_MASK) >>
>  			TXEFS_EFGI_SHIFT;
>  
> -		/* get message marker */
> -		msg_mark = (m_can_txe_fifo_read(cdev, fgi, 4) &
> -			    TX_EVENT_MM_MASK) >> TX_EVENT_MM_SHIFT;
> +		/* get message marker, timestamp */
> +		txe = m_can_txe_fifo_read(cdev, fgi, 4);
> +		msg_mark = (txe & TX_EVENT_MM_MASK) >> TX_EVENT_MM_SHIFT;
> +		timestamp = FIELD_GET(TX_EVENT_TXTS_MASK, txe);
>  
>  		/* ack txe element */
>  		m_can_write(cdev, M_CAN_TXEFA, (TXEFA_EFAI_MASK &
>  						(fgi << TXEFA_EFAI_SHIFT)));
>  
>  		/* update stats */
> -		stats->tx_bytes += can_get_echo_skb(dev, msg_mark, NULL);
> -		stats->tx_packets++;
> +		m_can_tx_update_stats(cdev, msg_mark, timestamp);
>  	}
>  }
>  
> @@ -966,7 +1027,6 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
>  {
>  	struct net_device *dev = (struct net_device *)dev_id;
>  	struct m_can_classdev *cdev = netdev_priv(dev);
> -	struct net_device_stats *stats = &dev->stats;
>  	u32 ir;
>  
>  	if (pm_runtime_suspended(cdev->dev))
> @@ -999,8 +1059,12 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
>  	if (cdev->version == 30) {
>  		if (ir & IR_TC) {
>  			/* Transmission Complete Interrupt*/
> -			stats->tx_bytes += can_get_echo_skb(dev, 0, NULL);
> -			stats->tx_packets++;
> +			u32 timestamp = 0;
> +
> +			if (cdev->is_peripheral)
> +				timestamp = m_can_get_timestamp(cdev);
> +			m_can_tx_update_stats(cdev, 0, timestamp);
> +
>  			can_led_event(dev, CAN_LED_EVENT_TX);
>  			netif_wake_queue(dev);
>  		}
> @@ -1461,6 +1525,9 @@ static int m_can_close(struct net_device *dev)
>  		cdev->tx_wq = NULL;
>  	}
>  
> +	if (cdev->is_peripheral)
> +		can_rx_offload_disable(&cdev->offload);
> +
>  	close_candev(dev);
>  	can_led_event(dev, CAN_LED_EVENT_STOP);
>  
> @@ -1659,6 +1726,9 @@ static int m_can_open(struct net_device *dev)
>  		goto exit_disable_clks;
>  	}
>  
> +	if (cdev->is_peripheral)
> +		can_rx_offload_enable(&cdev->offload);
> +
>  	/* register interrupt handler */
>  	if (cdev->is_peripheral) {
>  		cdev->tx_skb = NULL;
> @@ -1700,6 +1770,8 @@ static int m_can_open(struct net_device *dev)
>  	if (cdev->is_peripheral)
>  		destroy_workqueue(cdev->tx_wq);
>  out_wq_fail:
> +	if (cdev->is_peripheral)
> +		can_rx_offload_disable(&cdev->offload);
>  	close_candev(dev);
>  exit_disable_clks:
>  	m_can_clk_stop(cdev);
> @@ -1853,6 +1925,13 @@ int m_can_class_register(struct m_can_classdev *cdev)
>  			return ret;
>  	}
>  
> +	if (cdev->is_peripheral) {
> +		ret = can_rx_offload_add_manual(cdev->net, &cdev->offload,
> +						M_CAN_NAPI_WEIGHT);
> +		if (ret)
> +			goto clk_disable;
> +	}
> +
>  	ret = m_can_dev_setup(cdev);
>  	if (ret)
>  		goto clk_disable;

You need to update the error handling here. I've done this while
applying.

> @@ -1883,6 +1962,8 @@ EXPORT_SYMBOL_GPL(m_can_class_register);
>  
>  void m_can_class_unregister(struct m_can_classdev *cdev)
>  {
> +	if (cdev->is_peripheral)
> +		can_rx_offload_del(&cdev->offload);
>  	unregister_candev(cdev->net);
>  }
>  EXPORT_SYMBOL_GPL(m_can_class_unregister);
> diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
> index 3fda84cef351..ace071c3e58c 100644
> --- a/drivers/net/can/m_can/m_can.h
> +++ b/drivers/net/can/m_can/m_can.h
> @@ -8,6 +8,7 @@
>  
>  #include <linux/can/core.h>
>  #include <linux/can/led.h>
> +#include <linux/can/rx-offload.h>
>  #include <linux/completion.h>
>  #include <linux/device.h>
>  #include <linux/dma-mapping.h>
> @@ -71,6 +72,7 @@ struct m_can_ops {
>  
>  struct m_can_classdev {
>  	struct can_priv can;
> +	struct can_rx_offload offload;
>  	struct napi_struct napi;
>  	struct net_device *net;
>  	struct device *dev;
> -- 
> 2.30.1

I'll send a v3.

regards,
Marc

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]