2020-06-10 07:49:43

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

Hello,

This series adds CAN network driver support for Microchip MCP25XXFD CAN
Controller with MCP2517FD as the target controller version. This series is
mostly inspired (or taken) from the previous iterations posted by Martin Sperl.
I've trimmed down the parts which are not necessary for the initial version
to ease review. Still the series is relatively huge but I hope to get some
reviews (post -rcX ofc!).

Link to the origial series posted by Martin:
https://www.spinics.net/lists/devicetree/msg284462.html

I've not changed the functionality much but done some considerable amount of
cleanups and also preserved the authorship of Martin for all the patches he has
posted earlier. This series has been tested on 96Boards RB3 platform by myself
and Martin has tested the previous version on Rpi3 with external MCP2517FD
controller.

Thanks,
Mani

Manivannan Sadhasivam (1):
MAINTAINERS: Add entry for Microchip MCP25XXFD CAN network driver

Martin Sperl (5):
dt-bindings: can: Document devicetree bindings for MCP25XXFD
can: mcp25xxfd: Add Microchip MCP25XXFD CAN-FD driver infrastructure
can: mcp25xxfd: Add support for CAN reception
can: mcp25xxfd: Add CAN transmission support
can: mcp25xxfd: Optimize TEF read by avoiding unnecessary SPI
transfers

.../bindings/net/can/microchip,mcp25xxfd.yaml | 82 +++
MAINTAINERS | 8 +
drivers/net/can/spi/Kconfig | 2 +
drivers/net/can/spi/Makefile | 2 +
drivers/net/can/spi/mcp25xxfd/Kconfig | 5 +
drivers/net/can/spi/mcp25xxfd/Makefile | 11 +
.../net/can/spi/mcp25xxfd/mcp25xxfd_base.c | 177 +++++
.../net/can/spi/mcp25xxfd/mcp25xxfd_base.h | 14 +
drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.c | 538 ++++++++++++++
drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.h | 52 ++
.../can/spi/mcp25xxfd/mcp25xxfd_can_fifo.c | 305 ++++++++
.../can/spi/mcp25xxfd/mcp25xxfd_can_fifo.h | 16 +
.../net/can/spi/mcp25xxfd/mcp25xxfd_can_id.h | 69 ++
.../net/can/spi/mcp25xxfd/mcp25xxfd_can_int.c | 674 ++++++++++++++++++
.../net/can/spi/mcp25xxfd/mcp25xxfd_can_int.h | 18 +
.../can/spi/mcp25xxfd/mcp25xxfd_can_priv.h | 144 ++++
.../net/can/spi/mcp25xxfd/mcp25xxfd_can_rx.c | 233 ++++++
.../net/can/spi/mcp25xxfd/mcp25xxfd_can_rx.h | 18 +
.../net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.c | 653 +++++++++++++++++
.../net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.h | 86 +++
drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.c | 226 ++++++
drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.h | 84 +++
drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.c | 31 +
drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.h | 15 +
drivers/net/can/spi/mcp25xxfd/mcp25xxfd_ecc.c | 74 ++
drivers/net/can/spi/mcp25xxfd/mcp25xxfd_ecc.h | 16 +
drivers/net/can/spi/mcp25xxfd/mcp25xxfd_int.c | 71 ++
drivers/net/can/spi/mcp25xxfd/mcp25xxfd_int.h | 15 +
.../net/can/spi/mcp25xxfd/mcp25xxfd_priv.h | 50 ++
.../net/can/spi/mcp25xxfd/mcp25xxfd_regs.h | 661 +++++++++++++++++
30 files changed, 4350 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/can/microchip,mcp25xxfd.yaml
create mode 100644 drivers/net/can/spi/mcp25xxfd/Kconfig
create mode 100644 drivers/net/can/spi/mcp25xxfd/Makefile
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_base.c
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_base.h
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.c
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.h
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_fifo.c
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_fifo.h
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_id.h
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.c
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.h
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_priv.h
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_rx.c
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_rx.h
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.c
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.h
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.c
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.h
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.c
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.h
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_ecc.c
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_ecc.h
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_int.c
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_int.h
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_priv.h
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_regs.h

--
2.17.1


2020-06-10 07:51:42

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH 4/6] can: mcp25xxfd: Add CAN transmission support

From: Martin Sperl <[email protected]>

Add un-optimized CAN2.0 and CAN-FD transmission support.

On a Rpi3 we can saturate the CAN bus at 1MHz transmitting CAN2.0
frames with DLC=0 for the number of configured tx-fifos.
Afterwards we need some time to recover the state before we can
fill in the fifos again.

With 7 tx fifos we can: send those 7 frames in 0.33ms and then we
wait for 0.26ms so that is a 56% duty cycle.

With 24 tx fifos this changes to: 1.19ms for 24 frames and then we
wait for 0.52ms so that is a 70% duty cycle.

Signed-off-by: Martin Sperl <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/net/can/spi/mcp25xxfd/Makefile | 1 +
drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.c | 10 +
drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.h | 3 +-
.../can/spi/mcp25xxfd/mcp25xxfd_can_fifo.c | 93 ++-
.../net/can/spi/mcp25xxfd/mcp25xxfd_can_int.c | 21 +-
.../can/spi/mcp25xxfd/mcp25xxfd_can_priv.h | 15 +-
.../net/can/spi/mcp25xxfd/mcp25xxfd_can_rx.c | 2 +-
.../net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.c | 620 ++++++++++++++++++
.../net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.h | 86 +++
9 files changed, 839 insertions(+), 12 deletions(-)
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.c
create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.h

diff --git a/drivers/net/can/spi/mcp25xxfd/Makefile b/drivers/net/can/spi/mcp25xxfd/Makefile
index 5787bdd57a9d..a586b2555ff9 100644
--- a/drivers/net/can/spi/mcp25xxfd/Makefile
+++ b/drivers/net/can/spi/mcp25xxfd/Makefile
@@ -4,6 +4,7 @@ mcp25xxfd-objs += mcp25xxfd_can.o
mcp25xxfd-objs += mcp25xxfd_can_fifo.o
mcp25xxfd-objs += mcp25xxfd_can_int.o
mcp25xxfd-objs += mcp25xxfd_can_rx.o
+mcp25xxfd-objs += mcp25xxfd_can_tx.o
mcp25xxfd-objs += mcp25xxfd_cmd.o
mcp25xxfd-objs += mcp25xxfd_crc.o
mcp25xxfd-objs += mcp25xxfd_ecc.o
diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.c b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.c
index 2ac78024c171..1417f1a22d6e 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.c
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.c
@@ -20,6 +20,7 @@
#include "mcp25xxfd_can_fifo.h"
#include "mcp25xxfd_can_int.h"
#include "mcp25xxfd_can_priv.h"
+#include "mcp25xxfd_can_tx.h"
#include "mcp25xxfd_can.h"
#include "mcp25xxfd_cmd.h"
#include "mcp25xxfd_int.h"
@@ -395,6 +396,10 @@ static int mcp25xxfd_can_open(struct net_device *net)
if (ret)
goto out_int;

+ /* start the tx_queue */
+ mcp25xxfd_can_tx_queue_manage(cpriv,
+ MCP25XXFD_CAN_TX_QUEUE_STATE_STARTED);
+
return 0;

out_int:
@@ -425,6 +430,10 @@ static int mcp25xxfd_can_stop(struct net_device *net)
struct mcp25xxfd_priv *priv = cpriv->priv;
struct spi_device *spi = priv->spi;

+ /* stop transmit queue */
+ mcp25xxfd_can_tx_queue_manage(cpriv,
+ MCP25XXFD_CAN_TX_QUEUE_STATE_STOPPED);
+
/* shutdown the can controller */
mcp25xxfd_can_shutdown(cpriv);

@@ -448,6 +457,7 @@ static int mcp25xxfd_can_stop(struct net_device *net)
static const struct net_device_ops mcp25xxfd_netdev_ops = {
.ndo_open = mcp25xxfd_can_open,
.ndo_stop = mcp25xxfd_can_stop,
+ .ndo_start_xmit = mcp25xxfd_can_tx_start_xmit,
.ndo_change_mtu = can_change_mtu,
};

diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.h b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.h
index 4b18b5bb3d45..8ec20827f099 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.h
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.h
@@ -22,12 +22,13 @@ int mcp25xxfd_can_targetmode(struct mcp25xxfd_can_priv *cpriv)

static inline
void mcp25xxfd_can_queue_frame(struct mcp25xxfd_can_priv *cpriv,
- s32 fifo, u16 ts)
+ s32 fifo, u16 ts, bool is_rx)
{
int idx = cpriv->fifos.submit_queue_count;

cpriv->fifos.submit_queue[idx].fifo = fifo;
cpriv->fifos.submit_queue[idx].ts = ts;
+ cpriv->fifos.submit_queue[idx].is_rx = is_rx;

cpriv->fifos.submit_queue_count++;
}
diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_fifo.c b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_fifo.c
index 4bd776772d2d..4a1ad250bdaf 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_fifo.c
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_fifo.c
@@ -5,8 +5,6 @@
* Copyright 2019 Martin Sperl <[email protected]>
*/

-/* here we define and configure the fifo layout */
-
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/spi/spi.h>
@@ -14,6 +12,7 @@
#include "mcp25xxfd_can.h"
#include "mcp25xxfd_can_fifo.h"
#include "mcp25xxfd_can_priv.h"
+#include "mcp25xxfd_can_tx.h"
#include "mcp25xxfd_cmd.h"

static int mcp25xxfd_can_fifo_get_address(struct mcp25xxfd_can_priv *cpriv)
@@ -55,6 +54,15 @@ static int mcp25xxfd_can_fifo_setup_config(struct mcp25xxfd_can_priv *cpriv,
c > 0; i++, f++, p--, c--) {
val = (c > 1) ? flags : flags_last;

+ /* are we in tx mode? */
+ if (flags & MCP25XXFD_CAN_FIFOCON_TXEN) {
+ cpriv->fifos.info[f].is_rx = false;
+ cpriv->fifos.info[f].priority = p;
+ val |= (p << MCP25XXFD_CAN_FIFOCON_TXPRI_SHIFT);
+ } else {
+ cpriv->fifos.info[f].is_rx = true;
+ }
+
/* write the config to the controller in one go */
ret = mcp25xxfd_cmd_write(cpriv->priv->spi,
MCP25XXFD_CAN_FIFOCON(f), val);
@@ -65,6 +73,27 @@ static int mcp25xxfd_can_fifo_setup_config(struct mcp25xxfd_can_priv *cpriv,
return 0;
}

+static int mcp25xxfd_can_fifo_setup_tx(struct mcp25xxfd_can_priv *cpriv)
+{
+ u32 tx_flags = MCP25XXFD_CAN_FIFOCON_FRESET | /* reset FIFO */
+ MCP25XXFD_CAN_FIFOCON_TXEN | /* a tx FIFO */
+ MCP25XXFD_CAN_FIFOCON_TXATIE | /* state in txatif */
+ (cpriv->fifos.payload_mode <<
+ MCP25XXFD_CAN_FIFOCON_PLSIZE_SHIFT) | /* paylod size */
+ (0 << MCP25XXFD_CAN_FIFOCON_FSIZE_SHIFT); /* 1 FIFO deep */
+
+ /* handle oneshot/three-shot */
+ if (cpriv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
+ tx_flags |= MCP25XXFD_CAN_FIFOCON_TXAT_ONE_SHOT <<
+ MCP25XXFD_CAN_FIFOCON_TXAT_SHIFT;
+ else
+ tx_flags |= MCP25XXFD_CAN_FIFOCON_TXAT_UNLIMITED <<
+ MCP25XXFD_CAN_FIFOCON_TXAT_SHIFT;
+
+ return mcp25xxfd_can_fifo_setup_config(cpriv, &cpriv->fifos.tx,
+ tx_flags, tx_flags);
+}
+
static int mcp25xxfd_can_fifo_setup_rx(struct mcp25xxfd_can_priv *cpriv)
{
u32 rx_flags = MCP25XXFD_CAN_FIFOCON_FRESET | /* reset FIFO */
@@ -106,7 +135,7 @@ static int mcp25xxfd_can_fifo_setup_rxfilter(struct mcp25xxfd_can_priv *cpriv)

static int mcp25xxfd_can_fifo_compute(struct mcp25xxfd_can_priv *cpriv)
{
- int rx_memory_available;
+ int tef_memory_used, tx_memory_used, rx_memory_available;

switch (cpriv->can.dev->mtu) {
case CAN_MTU:
@@ -114,23 +143,48 @@ static int mcp25xxfd_can_fifo_compute(struct mcp25xxfd_can_priv *cpriv)
cpriv->fifos.payload_size = 8;
cpriv->fifos.payload_mode = MCP25XXFD_CAN_TXQCON_PLSIZE_8;

+ /* 7 tx fifos */
+ cpriv->fifos.tx.count = 7;
+
break;
case CANFD_MTU:
/* MTU is 64 */
cpriv->fifos.payload_size = 64;
cpriv->fifos.payload_mode = MCP25XXFD_CAN_TXQCON_PLSIZE_64;

+ /* 7 tx fifos */
+ cpriv->fifos.tx.count = 7;
+
break;
default:
return -EINVAL;
}

/* compute effective sizes */
+ cpriv->fifos.tef.size = sizeof(struct mcp25xxfd_can_obj_tef);
+ cpriv->fifos.tx.size = sizeof(struct mcp25xxfd_can_obj_tx) +
+ cpriv->fifos.payload_size;
cpriv->fifos.rx.size = sizeof(struct mcp25xxfd_can_obj_rx) +
cpriv->fifos.payload_size;

+ /* set tef fifos to the number of tx fifos */
+ cpriv->fifos.tef.count = cpriv->fifos.tx.count;
+
+ /* compute size of the tx fifos and TEF */
+ tx_memory_used = cpriv->fifos.tx.count * cpriv->fifos.tx.size;
+ tef_memory_used = cpriv->fifos.tef.count * cpriv->fifos.tef.size;
+
/* calculate evailable memory for RX_fifos */
- rx_memory_available = MCP25XXFD_SRAM_SIZE;
+ rx_memory_available = MCP25XXFD_SRAM_SIZE - tx_memory_used -
+ tef_memory_used;
+
+ /* we need at least one RX Frame */
+ if (rx_memory_available < cpriv->fifos.rx.size) {
+ netdev_err(cpriv->can.dev,
+ "Configured %i tx-fifos exceeds available memory already\n",
+ cpriv->fifos.tx.count);
+ return -EINVAL;
+ }

/* calculate possible amount of RX fifos */
cpriv->fifos.rx.count = rx_memory_available / cpriv->fifos.rx.size;
@@ -138,10 +192,11 @@ static int mcp25xxfd_can_fifo_compute(struct mcp25xxfd_can_priv *cpriv)
/* now calculate effective number of rx-fifos. There are only 31 fifos
* available in total, so we need to limit ourselves
*/
- if (cpriv->fifos.rx.count > 31)
- cpriv->fifos.rx.count = 31;
+ if (cpriv->fifos.rx.count + cpriv->fifos.tx.count > 31)
+ cpriv->fifos.rx.count = 31 - cpriv->fifos.tx.count;

- cpriv->fifos.rx.start = 1;
+ cpriv->fifos.tx.start = 1;
+ cpriv->fifos.rx.start = cpriv->fifos.tx.start + cpriv->fifos.tx.count;

return 0;
}
@@ -170,6 +225,7 @@ static int mcp25xxfd_can_fifo_clear(struct mcp25xxfd_can_priv *cpriv)
int ret;

memset(&cpriv->fifos.info, 0, sizeof(cpriv->fifos.info));
+ memset(&cpriv->fifos.tx, 0, sizeof(cpriv->fifos.tx));
memset(&cpriv->fifos.rx, 0, sizeof(cpriv->fifos.rx));

/* clear FIFO config */
@@ -196,16 +252,31 @@ int mcp25xxfd_can_fifo_setup(struct mcp25xxfd_can_priv *cpriv)
if (ret)
return ret;

- cpriv->regs.tefcon = 0;
+ /* configure TEF */
+ if (cpriv->fifos.tef.count)
+ cpriv->regs.tefcon =
+ MCP25XXFD_CAN_TEFCON_FRESET |
+ MCP25XXFD_CAN_TEFCON_TEFNEIE |
+ MCP25XXFD_CAN_TEFCON_TEFTSEN |
+ ((cpriv->fifos.tef.count - 1) <<
+ MCP25XXFD_CAN_TEFCON_FSIZE_SHIFT);
+ else
+ cpriv->regs.tefcon = 0;
ret = mcp25xxfd_cmd_write(cpriv->priv->spi, MCP25XXFD_CAN_TEFCON,
cpriv->regs.tefcon);
if (ret)
return ret;

+ /* TXQueue disabled */
ret = mcp25xxfd_cmd_write(cpriv->priv->spi, MCP25XXFD_CAN_TXQCON, 0);
if (ret)
return ret;

+ /* configure FIFOS themselves */
+ ret = mcp25xxfd_can_fifo_setup_tx(cpriv);
+ if (ret)
+ return ret;
+
ret = mcp25xxfd_can_fifo_setup_rx(cpriv);
if (ret)
return ret;
@@ -219,10 +290,16 @@ int mcp25xxfd_can_fifo_setup(struct mcp25xxfd_can_priv *cpriv)
if (ret)
return ret;

+ /* setup tx_fifo_queue */
+ ret = mcp25xxfd_can_tx_queue_alloc(cpriv);
+ if (ret)
+ return ret;
+
return 0;
}

void mcp25xxfd_can_fifo_release(struct mcp25xxfd_can_priv *cpriv)
{
+ mcp25xxfd_can_tx_queue_free(cpriv);
mcp25xxfd_can_fifo_clear(cpriv);
}
diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.c b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.c
index 83656b2604df..b3cf3e77c299 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.c
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.c
@@ -23,6 +23,7 @@
#include "mcp25xxfd_can_int.h"
#include "mcp25xxfd_can_priv.h"
#include "mcp25xxfd_can_rx.h"
+#include "mcp25xxfd_can_tx.h"
#include "mcp25xxfd_cmd.h"
#include "mcp25xxfd_ecc.h"

@@ -80,7 +81,9 @@ static int mcp25xxfd_can_int_submit_frames(struct mcp25xxfd_can_priv *cpriv)
/* now submit the fifos */
for (i = 0; i < count; i++) {
fifo = queue[i].fifo;
- ret = mcp25xxfd_can_rx_submit_frame(cpriv, fifo);
+ ret = (queue[i].is_rx) ?
+ mcp25xxfd_can_rx_submit_frame(cpriv, fifo) :
+ mcp25xxfd_can_tx_submit_frame(cpriv, fifo);
if (ret)
return ret;
}
@@ -100,6 +103,9 @@ static int mcp25xxfd_can_int_submit_frames(struct mcp25xxfd_can_priv *cpriv)
}

out:
+ /* enable tx_queue if necessary */
+ mcp25xxfd_can_tx_queue_restart(cpriv);
+
return 0;
}

@@ -496,6 +502,9 @@ static int mcp25xxfd_can_int_error_handling(struct mcp25xxfd_can_priv *cpriv)
cpriv->can.can_stats.bus_off++;
can_bus_off(cpriv->can.dev);
}
+ } else {
+ /* restart the tx queue if needed */
+ mcp25xxfd_can_tx_queue_restart(cpriv);
}

return 0;
@@ -533,6 +542,16 @@ static int mcp25xxfd_can_int_handle_status(struct mcp25xxfd_can_priv *cpriv)
if (ret)
return ret;

+ /* handle aborted TX FIFOs */
+ ret = mcp25xxfd_can_tx_handle_int_txatif(cpriv);
+ if (ret)
+ return ret;
+
+ /* handle the TEF */
+ ret = mcp25xxfd_can_tx_handle_int_tefif(cpriv);
+ if (ret)
+ return ret;
+
/* handle error interrupt flags */
ret = mcp25xxfd_can_rx_handle_int_rxovif(cpriv);
if (ret)
diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_priv.h b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_priv.h
index e043b262a868..99c3ef6d08e0 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_priv.h
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_priv.h
@@ -26,10 +26,12 @@ struct mcp25xxfd_fifo {
struct mcp25xxfd_obj_ts {
s32 ts; /* using signed to handle rollover correctly when sorting */
u16 fifo;
+ s16 is_rx;
};

/* general info on each fifo */
struct mcp25xxfd_fifo_info {
+ u32 is_rx;
u32 offset;
u32 priority;
};
@@ -95,10 +97,18 @@ struct mcp25xxfd_can_priv {

/* infos on fifo layout */

+ /* TEF */
+ struct {
+ u32 count;
+ u32 size;
+ u32 index;
+ } tef;
+
/* info on each fifo */
struct mcp25xxfd_fifo_info info[32];

- /* extra info on rx fifo groups */
+ /* extra info on rx/tx fifo groups */
+ struct mcp25xxfd_fifo tx;
struct mcp25xxfd_fifo rx;

/* queue of can frames that need to get submitted
@@ -109,6 +119,9 @@ struct mcp25xxfd_can_priv {
*/
struct mcp25xxfd_obj_ts submit_queue[32];
int submit_queue_count;
+
+ /* the tx queue of spi messages */
+ struct mcp25xxfd_tx_spi_message_queue *tx_queue;
} fifos;

/* bus state */
diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_rx.c b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_rx.c
index 5e3f706e7a3f..da99145e0c94 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_rx.c
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_rx.c
@@ -163,7 +163,7 @@ static int mcp25xxfd_can_rx_read_frame(struct mcp25xxfd_can_priv *cpriv,
memset(rx->data + len, 0, ((net->mtu == CANFD_MTU) ? 64 : 8) - len);

/* add the fifo to the process queues */
- mcp25xxfd_can_queue_frame(cpriv, fifo, rx->ts);
+ mcp25xxfd_can_queue_frame(cpriv, fifo, rx->ts, true);

/* and clear the interrupt flag for that fifo */
return mcp25xxfd_cmd_write_mask(spi, MCP25XXFD_CAN_FIFOCON(fifo),
diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.c b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.c
new file mode 100644
index 000000000000..5ea1e525e776
--- /dev/null
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.c
@@ -0,0 +1,620 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/* CAN bus driver for Microchip 25XXFD CAN Controller with SPI Interface
+ *
+ * Copyright 2019 Martin Sperl <[email protected]>
+ *
+ * Based on Microchip MCP251x CAN controller driver written by
+ * David Vrabel, Copyright 2006 Arcom Control Systems Ltd.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/can/core.h>
+#include <linux/can/dev.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/netdevice.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+
+#include "mcp25xxfd_can.h"
+#include "mcp25xxfd_can_id.h"
+#include "mcp25xxfd_can_tx.h"
+#include "mcp25xxfd_cmd.h"
+#include "mcp25xxfd_regs.h"
+
+static struct mcp25xxfd_tx_spi_message *
+mcp25xxfd_can_tx_queue_first_spi_message(struct mcp25xxfd_tx_spi_message_queue *
+ queue, u32 *bitmap)
+{
+ u32 first = ffs(*bitmap);
+
+ if (!first)
+ return NULL;
+
+ return queue->fifo2message[first - 1];
+}
+
+static void mcp25xxfd_can_tx_queue_remove_spi_message(u32 *bitmap, int fifo)
+{
+ *bitmap &= ~BIT(fifo);
+}
+
+static void mcp25xxfd_can_tx_queue_add_spi_message(u32 *bitmap, int fifo)
+{
+ *bitmap |= BIT(fifo);
+}
+
+static void mcp25xxfd_can_tx_queue_move_spi_message(u32 *src, u32 *dest,
+ int fifo)
+{
+ mcp25xxfd_can_tx_queue_remove_spi_message(src, fifo);
+ mcp25xxfd_can_tx_queue_add_spi_message(dest, fifo);
+}
+
+static void mcp25xxfd_can_tx_spi_message_fill_fifo_complete(void *context)
+{
+ struct mcp25xxfd_tx_spi_message *msg = context;
+ struct mcp25xxfd_can_priv *cpriv = msg->cpriv;
+ struct mcp25xxfd_tx_spi_message_queue *q = cpriv->fifos.tx_queue;
+ unsigned long flags;
+
+ /* reset transfer length to without data (DLC = 0) */
+ msg->fill_fifo.xfer.len = sizeof(msg->fill_fifo.data.cmd) +
+ sizeof(msg->fill_fifo.data.header);
+
+ spin_lock_irqsave(&cpriv->fifos.tx_queue->lock, flags);
+
+ /* move to in_trigger_fifo_transfer */
+ mcp25xxfd_can_tx_queue_move_spi_message(&q->in_fill_fifo_transfer,
+ &q->in_trigger_fifo_transfer,
+ msg->fifo);
+
+ spin_unlock_irqrestore(&cpriv->fifos.tx_queue->lock, flags);
+}
+
+static void mcp25xxfd_can_tx_spi_message_trigger_fifo_complete(void *context)
+{
+ struct mcp25xxfd_tx_spi_message *msg = context;
+ struct mcp25xxfd_can_priv *cpriv = msg->cpriv;
+ struct mcp25xxfd_tx_spi_message_queue *q = cpriv->fifos.tx_queue;
+ unsigned long flags;
+
+ spin_lock_irqsave(&cpriv->fifos.tx_queue->lock, flags);
+
+ /* move to can_transfer */
+ mcp25xxfd_can_tx_queue_move_spi_message(&q->in_trigger_fifo_transfer,
+ &q->in_can_transfer,
+ msg->fifo);
+
+ spin_unlock_irqrestore(&cpriv->fifos.tx_queue->lock, flags);
+}
+
+static
+void mcp25xxfd_can_tx_message_init(struct mcp25xxfd_can_priv *cpriv,
+ struct mcp25xxfd_tx_spi_message *msg,
+ int fifo)
+{
+ const u32 trigger = MCP25XXFD_CAN_FIFOCON_TXREQ |
+ MCP25XXFD_CAN_FIFOCON_UINC;
+ const int first_byte = mcp25xxfd_cmd_first_byte(trigger);
+ u32 addr;
+
+ msg->cpriv = cpriv;
+ msg->fifo = fifo;
+
+ /* init fill_fifo */
+ spi_message_init(&msg->fill_fifo.msg);
+ msg->fill_fifo.msg.complete =
+ mcp25xxfd_can_tx_spi_message_fill_fifo_complete;
+ msg->fill_fifo.msg.context = msg;
+
+ msg->fill_fifo.xfer.tx_buf = msg->fill_fifo.data.cmd;
+ msg->fill_fifo.xfer.len = sizeof(msg->fill_fifo.data.cmd) +
+ sizeof(msg->fill_fifo.data.header);
+ spi_message_add_tail(&msg->fill_fifo.xfer, &msg->fill_fifo.msg);
+
+ addr = MCP25XXFD_SRAM_ADDR(cpriv->fifos.info[fifo].offset);
+ mcp25xxfd_cmd_calc(MCP25XXFD_INSTRUCTION_WRITE, addr,
+ msg->fill_fifo.data.cmd);
+
+ /* init trigger_fifo */
+ spi_message_init(&msg->trigger_fifo.msg);
+ msg->trigger_fifo.msg.complete =
+ mcp25xxfd_can_tx_spi_message_trigger_fifo_complete;
+ msg->trigger_fifo.msg.context = msg;
+
+ msg->trigger_fifo.xfer.tx_buf = msg->trigger_fifo.data.cmd;
+ msg->trigger_fifo.xfer.len = sizeof(msg->trigger_fifo.data.cmd) +
+ sizeof(msg->trigger_fifo.data.data);
+ spi_message_add_tail(&msg->trigger_fifo.xfer, &msg->trigger_fifo.msg);
+
+ mcp25xxfd_cmd_calc(MCP25XXFD_INSTRUCTION_WRITE,
+ MCP25XXFD_CAN_FIFOCON(fifo) + first_byte,
+ msg->trigger_fifo.data.cmd);
+ msg->trigger_fifo.data.data = trigger >> (8 * first_byte);
+
+ /* add to idle tx transfers */
+ mcp25xxfd_can_tx_queue_add_spi_message(&cpriv->fifos.tx_queue->idle,
+ fifo);
+}
+
+static
+void mcp25xxfd_can_tx_queue_manage_nolock(struct mcp25xxfd_can_priv *cpriv,
+ int state)
+{
+ struct net_device *net = cpriv->can.dev;
+
+ if (state == cpriv->fifos.tx_queue->state)
+ return;
+
+ /* start/stop netif_queue if necessary */
+ switch (cpriv->fifos.tx_queue->state) {
+ case MCP25XXFD_CAN_TX_QUEUE_STATE_RUNABLE:
+ switch (state) {
+ case MCP25XXFD_CAN_TX_QUEUE_STATE_RESTART:
+ case MCP25XXFD_CAN_TX_QUEUE_STATE_STARTED:
+ netif_wake_queue(net);
+ cpriv->fifos.tx_queue->state =
+ MCP25XXFD_CAN_TX_QUEUE_STATE_STARTED;
+ break;
+ }
+ break;
+ case MCP25XXFD_CAN_TX_QUEUE_STATE_STOPPED:
+ switch (state) {
+ case MCP25XXFD_CAN_TX_QUEUE_STATE_STARTED:
+ netif_wake_queue(net);
+ cpriv->fifos.tx_queue->state = state;
+ break;
+ }
+ break;
+ case MCP25XXFD_CAN_TX_QUEUE_STATE_STARTED:
+ switch (state) {
+ case MCP25XXFD_CAN_TX_QUEUE_STATE_RUNABLE:
+ case MCP25XXFD_CAN_TX_QUEUE_STATE_STOPPED:
+ netif_stop_queue(net);
+ cpriv->fifos.tx_queue->state = state;
+ break;
+ }
+ break;
+ default:
+ netdev_err(cpriv->can.dev, "Unsupported tx_queue state: %i\n",
+ cpriv->fifos.tx_queue->state);
+ break;
+ }
+}
+
+void mcp25xxfd_can_tx_queue_manage(struct mcp25xxfd_can_priv *cpriv, int state)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&cpriv->fifos.tx_queue->lock, flags);
+
+ mcp25xxfd_can_tx_queue_manage_nolock(cpriv, state);
+
+ spin_unlock_irqrestore(&cpriv->fifos.tx_queue->lock, flags);
+}
+
+void mcp25xxfd_can_tx_queue_restart(struct mcp25xxfd_can_priv *cpriv)
+{
+ u32 state = MCP25XXFD_CAN_TX_QUEUE_STATE_RESTART;
+ unsigned long flags;
+ u32 mask;
+
+ spin_lock_irqsave(&cpriv->fifos.tx_queue->lock, flags);
+
+ /* only move if there is nothing pending or idle */
+ mask = cpriv->fifos.tx_queue->idle |
+ cpriv->fifos.tx_queue->in_fill_fifo_transfer |
+ cpriv->fifos.tx_queue->in_trigger_fifo_transfer |
+ cpriv->fifos.tx_queue->in_can_transfer;
+ if (mask)
+ goto out;
+
+ /* move all items from transferred to idle */
+ cpriv->fifos.tx_queue->idle |= cpriv->fifos.tx_queue->transferred;
+ cpriv->fifos.tx_queue->transferred = 0;
+
+ /* and enable queue */
+ mcp25xxfd_can_tx_queue_manage_nolock(cpriv, state);
+out:
+ spin_unlock_irqrestore(&cpriv->fifos.tx_queue->lock, flags);
+}
+
+static
+int mcp25xxfd_can_tx_handle_int_tefif_fifo(struct mcp25xxfd_can_priv *cpriv)
+{
+ u32 tef_offset = cpriv->fifos.tef.index * cpriv->fifos.tef.size;
+ struct mcp25xxfd_can_obj_tef *tef =
+ (struct mcp25xxfd_can_obj_tef *)(cpriv->sram + tef_offset);
+ int fifo, ret;
+ unsigned long flags;
+
+ /* read the next TEF entry to get the transmit timestamp and fifo */
+ ret = mcp25xxfd_cmd_read_regs(cpriv->priv->spi,
+ MCP25XXFD_SRAM_ADDR(tef_offset),
+ &tef->id, sizeof(*tef));
+ if (ret)
+ return ret;
+
+ /* get the fifo from tef */
+ fifo = FIELD_GET(MCP25XXFD_CAN_OBJ_FLAGS_SEQ_MASK, tef->flags);
+
+ /* check that the fifo is valid */
+ spin_lock_irqsave(&cpriv->fifos.tx_queue->lock, flags);
+ if ((cpriv->fifos.tx_queue->in_can_transfer & BIT(fifo)) == 0)
+ netdev_err(cpriv->can.dev,
+ "tefif: fifo %i not pending - tef data: id: %08x flags: %08x, ts: %08x - this may be a problem with spi signal quality- try reducing spi-clock speed if this can get reproduced",
+ fifo, tef->id, tef->flags, tef->ts);
+ spin_unlock_irqrestore(&cpriv->fifos.tx_queue->lock, flags);
+
+ /* now we can schedule the fifo for echo submission */
+ mcp25xxfd_can_queue_frame(cpriv, fifo, tef->ts, false);
+
+ /* increment the tef index with wraparound */
+ cpriv->fifos.tef.index++;
+ if (cpriv->fifos.tef.index >= cpriv->fifos.tef.count)
+ cpriv->fifos.tef.index = 0;
+
+ /* finally just increment the TEF pointer */
+ return mcp25xxfd_cmd_write_mask(cpriv->priv->spi, MCP25XXFD_CAN_TEFCON,
+ MCP25XXFD_CAN_TEFCON_UINC,
+ MCP25XXFD_CAN_TEFCON_UINC);
+}
+
+static int
+mcp25xxfd_can_tx_handle_int_tefif_conservative(struct mcp25xxfd_can_priv *cpriv)
+{
+ u32 tefsta;
+ int ret;
+
+ /* read the TEF status */
+ ret = mcp25xxfd_cmd_read_mask(cpriv->priv->spi, MCP25XXFD_CAN_TEFSTA,
+ &tefsta, MCP25XXFD_CAN_TEFSTA_TEFNEIF);
+ if (ret)
+ return ret;
+
+ /* read the tef in an inefficient loop */
+ while (tefsta & MCP25XXFD_CAN_TEFSTA_TEFNEIF) {
+ /* read one tef */
+ ret = mcp25xxfd_can_tx_handle_int_tefif_fifo(cpriv);
+ if (ret)
+ return ret;
+
+ /* read the TEF status */
+ ret = mcp25xxfd_cmd_read_mask(cpriv->priv->spi,
+ MCP25XXFD_CAN_TEFSTA, &tefsta,
+ MCP25XXFD_CAN_TEFSTA_TEFNEIF);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+int mcp25xxfd_can_tx_handle_int_tefif(struct mcp25xxfd_can_priv *cpriv)
+{
+ unsigned long flags;
+ u32 finished;
+
+ if (!(cpriv->status.intf & MCP25XXFD_CAN_INT_TEFIF))
+ return 0;
+
+ spin_lock_irqsave(&cpriv->fifos.tx_queue->lock, flags);
+
+ /* compute finished fifos and clear them immediately */
+ finished = (cpriv->fifos.tx_queue->in_can_transfer ^
+ cpriv->status.txreq) &
+ cpriv->fifos.tx_queue->in_can_transfer;
+
+ spin_unlock_irqrestore(&cpriv->fifos.tx_queue->lock, flags);
+
+ return mcp25xxfd_can_tx_handle_int_tefif_conservative(cpriv);
+}
+
+static
+void mcp25xxfd_can_tx_fill_fifo_common(struct mcp25xxfd_can_priv *cpriv,
+ struct mcp25xxfd_tx_spi_message *smsg,
+ struct mcp25xxfd_can_obj_tx *tx,
+ int dlc, u8 *data)
+{
+ int len = can_dlc2len(dlc);
+
+ /* add fifo number as seq */
+ tx->flags |= smsg->fifo << MCP25XXFD_CAN_OBJ_FLAGS_SEQ_SHIFT;
+
+ /* copy data to tx->data for future reference */
+ memcpy(tx->data, data, len);
+
+ /* transform header to controller format */
+ mcp25xxfd_cmd_convert_from_cpu(&tx->id, sizeof(*tx) / sizeof(u32));
+
+ /* copy header + data to final location - we are not aligned */
+ memcpy(smsg->fill_fifo.data.header, &tx->id, sizeof(*tx) + len);
+
+ /* transfers to sram should be a multiple of 4 and be zero padded */
+ for (; len & 3; len++)
+ *(smsg->fill_fifo.data.header + sizeof(*tx) + len) = 0;
+
+ /* convert it back to CPU format */
+ mcp25xxfd_cmd_convert_to_cpu(&tx->id, sizeof(*tx) / sizeof(u32));
+
+ /* set up size of transfer */
+ smsg->fill_fifo.xfer.len = sizeof(smsg->fill_fifo.data.cmd) +
+ sizeof(smsg->fill_fifo.data.header) + len;
+}
+
+static
+void mcp25xxfd_can_tx_fill_fifo_fd(struct mcp25xxfd_can_priv *cpriv,
+ struct canfd_frame *frame,
+ struct mcp25xxfd_tx_spi_message *smsg,
+ struct mcp25xxfd_can_obj_tx *tx)
+{
+ int dlc = can_len2dlc(frame->len);
+
+ /* compute can id */
+ mcp25xxfd_can_id_to_mcp25xxfd(frame->can_id, &tx->id, &tx->flags);
+
+ /* setup flags */
+ tx->flags |= dlc << MCP25XXFD_CAN_OBJ_FLAGS_DLC_SHIFT;
+ tx->flags |= (frame->can_id & CAN_EFF_FLAG) ?
+ MCP25XXFD_CAN_OBJ_FLAGS_IDE : 0;
+ tx->flags |= (frame->can_id & CAN_RTR_FLAG) ?
+ MCP25XXFD_CAN_OBJ_FLAGS_RTR : 0;
+ if (frame->flags & CANFD_BRS)
+ tx->flags |= MCP25XXFD_CAN_OBJ_FLAGS_BRS;
+
+ tx->flags |= (frame->flags & CANFD_ESI) ?
+ MCP25XXFD_CAN_OBJ_FLAGS_ESI : 0;
+ tx->flags |= MCP25XXFD_CAN_OBJ_FLAGS_FDF;
+
+ /* and do common processing */
+ mcp25xxfd_can_tx_fill_fifo_common(cpriv, smsg, tx, dlc, frame->data);
+}
+
+static
+void mcp25xxfd_can_tx_fill_fifo(struct mcp25xxfd_can_priv *cpriv,
+ struct can_frame *frame,
+ struct mcp25xxfd_tx_spi_message *smsg,
+ struct mcp25xxfd_can_obj_tx *tx)
+{
+ /* set frame to valid dlc */
+ if (frame->can_dlc > 8)
+ frame->can_dlc = 8;
+
+ /* compute can id */
+ mcp25xxfd_can_id_to_mcp25xxfd(frame->can_id, &tx->id, &tx->flags);
+
+ /* setup flags */
+ tx->flags |= frame->can_dlc << MCP25XXFD_CAN_OBJ_FLAGS_DLC_SHIFT;
+ tx->flags |= (frame->can_id & CAN_EFF_FLAG) ?
+ MCP25XXFD_CAN_OBJ_FLAGS_IDE : 0;
+ tx->flags |= (frame->can_id & CAN_RTR_FLAG) ?
+ MCP25XXFD_CAN_OBJ_FLAGS_RTR : 0;
+
+ /* and do common processing */
+ mcp25xxfd_can_tx_fill_fifo_common(cpriv, smsg, tx, frame->can_dlc,
+ frame->data);
+}
+
+static struct mcp25xxfd_tx_spi_message *
+mcp25xxfd_can_tx_queue_get_next_fifo(struct mcp25xxfd_can_priv *cpriv)
+{
+ u32 state = MCP25XXFD_CAN_TX_QUEUE_STATE_RUNABLE;
+ struct mcp25xxfd_tx_spi_message_queue *q = cpriv->fifos.tx_queue;
+ struct mcp25xxfd_tx_spi_message *smsg;
+ unsigned long flags;
+
+ spin_lock_irqsave(&q->lock, flags);
+
+ /* get the first entry from idle */
+ smsg = mcp25xxfd_can_tx_queue_first_spi_message(q, &q->idle);
+ if (!smsg)
+ goto out_busy;
+
+ /* and move the fifo to next stage */
+ mcp25xxfd_can_tx_queue_move_spi_message(&q->idle,
+ &q->in_fill_fifo_transfer,
+ smsg->fifo);
+
+ /* if queue is empty then stop the network queue immediately */
+ if (!q->idle)
+ mcp25xxfd_can_tx_queue_manage_nolock(cpriv, state);
+out_busy:
+ spin_unlock_irqrestore(&q->lock, flags);
+
+ return smsg;
+}
+
+/* submit the can message to the can-bus */
+netdev_tx_t mcp25xxfd_can_tx_start_xmit(struct sk_buff *skb,
+ struct net_device *net)
+{
+ u32 state = MCP25XXFD_CAN_TX_QUEUE_STATE_STOPPED;
+ struct mcp25xxfd_can_priv *cpriv = netdev_priv(net);
+ struct mcp25xxfd_tx_spi_message_queue *q = cpriv->fifos.tx_queue;
+ struct mcp25xxfd_priv *priv = cpriv->priv;
+ struct spi_device *spi = priv->spi;
+ struct mcp25xxfd_tx_spi_message *smsg;
+ struct mcp25xxfd_can_obj_tx *tx;
+ unsigned long flags;
+ int ret;
+
+ /* invalid skb we can ignore */
+ if (can_dropped_invalid_skb(net, skb))
+ return NETDEV_TX_OK;
+
+ spin_lock_irqsave(&q->spi_lock, flags);
+
+ /* get the fifo message structure to process now */
+ smsg = mcp25xxfd_can_tx_queue_get_next_fifo(cpriv);
+ if (!smsg)
+ goto out_busy;
+
+ /* compute the fifo in sram */
+ tx = (struct mcp25xxfd_can_obj_tx *)
+ (cpriv->sram + cpriv->fifos.info[smsg->fifo].offset);
+
+ /* fill in message from skb->data depending on can2.0 or canfd */
+ if (can_is_canfd_skb(skb))
+ mcp25xxfd_can_tx_fill_fifo_fd(cpriv,
+ (struct canfd_frame *)skb->data,
+ smsg, tx);
+ else
+ mcp25xxfd_can_tx_fill_fifo(cpriv,
+ (struct can_frame *)skb->data,
+ smsg, tx);
+
+ /* submit the two messages asyncronously
+ * the reason why we separate transfers into two spi_messages is:
+ * * because the spi framework (currently) does add a 10us delay
+ * between 2 spi_transfers in a single spi_message when
+ * change_cs is set - 2 consecutive spi messages show a shorter
+ * cs disable phase increasing bus utilization
+ * (code reduction with a fix in spi core would be aprox.50 lines)
+ * * this allows the interrupt handler to start spi messages earlier
+ * so reducing latencies a bit and to allow for better concurrency
+ * * this separation - in the future - may get used to fill fifos
+ * early and reduce the delay on "rollover"
+ */
+ ret = spi_async(spi, &smsg->fill_fifo.msg);
+ if (ret)
+ goto out_async_failed;
+
+ ret = spi_async(spi, &smsg->trigger_fifo.msg);
+ if (ret)
+ goto out_async_failed;
+
+ spin_unlock_irqrestore(&q->spi_lock, flags);
+
+ can_put_echo_skb(skb, net, smsg->fifo);
+
+ return NETDEV_TX_OK;
+
+out_async_failed:
+ netdev_err(net, "spi_async submission of fifo %i failed - %i\n",
+ smsg->fifo, ret);
+
+out_busy:
+ mcp25xxfd_can_tx_queue_manage_nolock(cpriv, state);
+ spin_unlock_irqrestore(&q->spi_lock, flags);
+
+ return NETDEV_TX_BUSY;
+}
+
+/* submit the fifo back to the network stack */
+int mcp25xxfd_can_tx_submit_frame(struct mcp25xxfd_can_priv *cpriv, int fifo)
+{
+ struct mcp25xxfd_tx_spi_message_queue *q = cpriv->fifos.tx_queue;
+ struct mcp25xxfd_can_obj_tx *tx = (struct mcp25xxfd_can_obj_tx *)
+ (cpriv->sram + cpriv->fifos.info[fifo].offset);
+ int dlc = (tx->flags & MCP25XXFD_CAN_OBJ_FLAGS_DLC_MASK) >>
+ MCP25XXFD_CAN_OBJ_FLAGS_DLC_SHIFT;
+ unsigned long flags;
+
+ /* update counters */
+ cpriv->can.dev->stats.tx_packets++;
+ cpriv->can.dev->stats.tx_bytes += can_dlc2len(dlc);
+
+ spin_lock_irqsave(&cpriv->fifos.tx_queue->lock, flags);
+
+ /* release the echo buffer */
+ can_get_echo_skb(cpriv->can.dev, fifo);
+
+ /* move from in_can_transfer to transferred */
+ mcp25xxfd_can_tx_queue_move_spi_message(&q->in_can_transfer,
+ &q->transferred, fifo);
+
+ spin_unlock_irqrestore(&cpriv->fifos.tx_queue->lock, flags);
+
+ return 0;
+}
+
+static int
+mcp25xxfd_can_tx_handle_int_txatif_fifo(struct mcp25xxfd_can_priv *cpriv,
+ int fifo)
+{
+ struct mcp25xxfd_tx_spi_message_queue *q = cpriv->fifos.tx_queue;
+ unsigned long flags;
+ u32 val;
+ int ret;
+
+ ret = mcp25xxfd_cmd_read(cpriv->priv->spi,
+ MCP25XXFD_CAN_FIFOSTA(fifo), &val);
+ if (ret)
+ return ret;
+
+ ret = mcp25xxfd_cmd_write_mask(cpriv->priv->spi,
+ MCP25XXFD_CAN_FIFOSTA(fifo), 0,
+ MCP25XXFD_CAN_FIFOSTA_TXABT |
+ MCP25XXFD_CAN_FIFOSTA_TXLARB |
+ MCP25XXFD_CAN_FIFOSTA_TXERR |
+ MCP25XXFD_CAN_FIFOSTA_TXATIF);
+ if (ret)
+ return ret;
+
+ spin_lock_irqsave(&q->lock, flags);
+
+ can_get_echo_skb(cpriv->can.dev, fifo);
+ mcp25xxfd_can_tx_queue_move_spi_message(&q->in_can_transfer,
+ &q->transferred, fifo);
+
+ spin_unlock_irqrestore(&q->lock, flags);
+
+ cpriv->status.txif &= ~BIT(fifo);
+ cpriv->can.dev->stats.tx_aborted_errors++;
+
+ return 0;
+}
+
+int mcp25xxfd_can_tx_handle_int_txatif(struct mcp25xxfd_can_priv *cpriv)
+{
+ int i, f, ret;
+
+ if (!cpriv->status.txatif)
+ return 0;
+
+ /* process all the fifos with txatif flag set */
+ for (i = 0, f = cpriv->fifos.tx.start; i < cpriv->fifos.tx.count;
+ i++, f++) {
+ if (cpriv->status.txatif & BIT(f)) {
+ ret = mcp25xxfd_can_tx_handle_int_txatif_fifo(cpriv, f);
+ if (ret)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+int mcp25xxfd_can_tx_queue_alloc(struct mcp25xxfd_can_priv *cpriv)
+{
+ struct mcp25xxfd_tx_spi_message *msg;
+ size_t size = sizeof(struct mcp25xxfd_tx_spi_message_queue) +
+ cpriv->fifos.tx.count * sizeof(*msg);
+ int i, f;
+
+ cpriv->fifos.tx_queue = kzalloc(size, GFP_KERNEL);
+ if (!cpriv->fifos.tx_queue)
+ return -ENOMEM;
+
+ spin_lock_init(&cpriv->fifos.tx_queue->lock);
+ spin_lock_init(&cpriv->fifos.tx_queue->spi_lock);
+
+ /* initialize the individual spi_message structures */
+ for (i = 0, f = cpriv->fifos.tx.start; i < cpriv->fifos.tx.count;
+ i++, f++) {
+ msg = &cpriv->fifos.tx_queue->message[i];
+ cpriv->fifos.tx_queue->fifo2message[f] = msg;
+ mcp25xxfd_can_tx_message_init(cpriv, msg, f);
+ }
+
+ return 0;
+}
+
+void mcp25xxfd_can_tx_queue_free(struct mcp25xxfd_can_priv *cpriv)
+{
+ kfree(cpriv->fifos.tx_queue);
+ cpriv->fifos.tx_queue = NULL;
+}
diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.h b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.h
new file mode 100644
index 000000000000..1947b3420d58
--- /dev/null
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.h
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/* CAN bus driver for Microchip 25XXFD CAN Controller with SPI Interface
+ *
+ * Copyright 2019 Martin Sperl <[email protected]>
+ */
+
+#ifndef __MCP25XXFD_CAN_TX_H
+#define __MCP25XXFD_CAN_TX_H
+
+#include <linux/spinlock.h>
+#include <linux/spi/spi.h>
+
+#include "mcp25xxfd_can_priv.h"
+
+/* structure of a spi message that is prepared and can get submitted quickly */
+struct mcp25xxfd_tx_spi_message {
+ /* the network device this is related to */
+ struct mcp25xxfd_can_priv *cpriv;
+ /* the fifo this fills */
+ u32 fifo;
+ /* the xfer to fill in the fifo data */
+ struct {
+ struct spi_message msg;
+ struct spi_transfer xfer;
+ struct {
+ u8 cmd[2];
+ u8 header[sizeof(struct mcp25xxfd_can_obj_tx)];
+ u8 data[64];
+ } data;
+ } fill_fifo;
+ /* the xfer to enable transmission on the can bus */
+ struct {
+ struct spi_message msg;
+ struct spi_transfer xfer;
+ struct {
+ u8 cmd[2];
+ u8 data;
+ } data;
+ } trigger_fifo;
+};
+
+struct mcp25xxfd_tx_spi_message_queue {
+ /* spinlock protecting the bitmaps
+ * as well as state and the skb_echo_* functions
+ */
+ spinlock_t lock;
+ /* bitmap of which fifo is in which stage */
+ u32 idle;
+ u32 in_fill_fifo_transfer;
+ u32 in_trigger_fifo_transfer;
+ u32 in_can_transfer;
+ u32 transferred;
+
+ /* the queue state as seen per controller */
+ int state;
+#define MCP25XXFD_CAN_TX_QUEUE_STATE_STOPPED 0
+#define MCP25XXFD_CAN_TX_QUEUE_STATE_STARTED 1
+#define MCP25XXFD_CAN_TX_QUEUE_STATE_RUNABLE 2
+#define MCP25XXFD_CAN_TX_QUEUE_STATE_RESTART 3
+
+ /* spinlock protecting spi submission order */
+ spinlock_t spi_lock;
+
+ /* map each fifo to a mcp25xxfd_tx_spi_message */
+ struct mcp25xxfd_tx_spi_message *fifo2message[32];
+
+ /* the individual messages */
+ struct mcp25xxfd_tx_spi_message message[];
+};
+
+int mcp25xxfd_can_tx_submit_frame(struct mcp25xxfd_can_priv *cpriv, int fifo);
+void mcp25xxfd_can_tx_queue_restart(struct mcp25xxfd_can_priv *cpriv);
+
+int mcp25xxfd_can_tx_handle_int_txatif(struct mcp25xxfd_can_priv *cpriv);
+int mcp25xxfd_can_tx_handle_int_tefif(struct mcp25xxfd_can_priv *cpriv);
+
+netdev_tx_t mcp25xxfd_can_tx_start_xmit(struct sk_buff *skb,
+ struct net_device *net);
+
+void mcp25xxfd_can_tx_queue_manage(struct mcp25xxfd_can_priv *cpriv, int state);
+
+int mcp25xxfd_can_tx_queue_alloc(struct mcp25xxfd_can_priv *cpriv);
+void mcp25xxfd_can_tx_queue_free(struct mcp25xxfd_can_priv *cpriv);
+
+#endif /* __MCP25XXFD_CAN_TX_H */
--
2.17.1

2020-06-11 16:30:56

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On 6/10/20 9:44 AM, Manivannan Sadhasivam wrote:
> Hello,
>
> This series adds CAN network driver support for Microchip MCP25XXFD CAN
> Controller with MCP2517FD as the target controller version. This series is
> mostly inspired (or taken) from the previous iterations posted by Martin Sperl.
> I've trimmed down the parts which are not necessary for the initial version
> to ease review. Still the series is relatively huge but I hope to get some
> reviews (post -rcX ofc!).
>
> Link to the origial series posted by Martin:
> https://www.spinics.net/lists/devicetree/msg284462.html
>
> I've not changed the functionality much but done some considerable amount of
> cleanups and also preserved the authorship of Martin for all the patches he has
> posted earlier. This series has been tested on 96Boards RB3 platform by myself
> and Martin has tested the previous version on Rpi3 with external MCP2517FD
> controller.

I initially started looking at Martin's driver and it was not using several
modern CAN driver infrastructures. I then posted some cleanup patches but Martin
was not working on the driver any more. Then I decided to rewrite the driver,
that is the one I'm hoping to mainline soon.

Can you give it a try?

https://github.com/marckleinebudde/linux/commits/v5.6-rpi/mcp25xxfd-20200607-41

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 |

2020-06-11 20:32:53

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On 6/11/20 6:26 PM, Marc Kleine-Budde wrote:
> I initially started looking at Martin's driver and it was not using several
> modern CAN driver infrastructures. I then posted some cleanup patches but Martin
> was not working on the driver any more. Then I decided to rewrite the driver,
> that is the one I'm hoping to mainline soon.

Seems the driver still suffers from the same problems the last time I looked at it:

I'm on a raspi4 with a mcp2518fd connected to spi0 cs0 and cs1.

Running a canfdtest -vg can0; canfdtest -v can1. It runs into this problem on a
unloaded system and scaling_governor=performance within minutes:

> Jun 11 21:27:08 rpi4 kernel: mcp25xxfd spi0.0 can1: Something is wrong - we got a TEF interrupt but we were not able to detect a finished fifo
> Jun 11 21:27:08 rpi4 kernel: mcp25xxfd spi0.0 can1: Something is wrong - we got a TEF interrupt but we were not able to detect a finished fifo
> Jun 11 21:27:08 rpi4 kernel: mcp25xxfd spi0.0 can1: tefif: fifo 6 not pending - tef data: id: 00000078 flags: 00000c08, ts: 0addbed3 - this may be a problem with spi signal quality- try reducing spi-clock speed if this can get reproduced
> Jun 11 21:27:08 rpi4 kernel: mcp25xxfd spi0.0 can1: tefif: fifo 7 not pending - tef data: id: 00000078 flags: 00000e08, ts: 0addc2e5 - this may be a problem with spi signal quality- try reducing spi-clock speed if this can get reproduced
> Jun 11 21:27:08 rpi4 kernel: mcp25xxfd spi0.0 can1: tefif: fifo 1 not pending - tef data: id: 00000078 flags: 00000208, ts: 0addc701 - this may be a problem with spi signal quality- try reducing spi-clock speed if this can get reproduced
> Jun 11 21:27:08 rpi4 kernel: mcp25xxfd spi0.0 can1: tefif: fifo 2 not pending - tef data: id: 00000078 flags: 00000408, ts: 0addcb1b - this may be a problem with spi signal quality- try reducing spi-clock speed if this can get reproduced

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 |

2020-06-12 11:29:56

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On 6/11/20 10:30 PM, Marc Kleine-Budde wrote:
> On 6/11/20 6:26 PM, Marc Kleine-Budde wrote:
>> I initially started looking at Martin's driver and it was not using several
>> modern CAN driver infrastructures. I then posted some cleanup patches but Martin
>> was not working on the driver any more. Then I decided to rewrite the driver,
>> that is the one I'm hoping to mainline soon.
>
> Seems the driver still suffers from the same problems the last time I looked at it:
>
> I'm on a raspi4 with a mcp2518fd connected to spi0 cs0 and cs1.
>
> Running a canfdtest -vg can0; canfdtest -v can1. It runs into this problem on a
> unloaded system and scaling_governor=performance within minutes:
>
>> Jun 11 21:27:08 rpi4 kernel: mcp25xxfd spi0.0 can1: Something is wrong - we got a TEF interrupt but we were not able to detect a finished fifo
>> Jun 11 21:27:08 rpi4 kernel: mcp25xxfd spi0.0 can1: Something is wrong - we got a TEF interrupt but we were not able to detect a finished fifo
>> Jun 11 21:27:08 rpi4 kernel: mcp25xxfd spi0.0 can1: tefif: fifo 6 not pending - tef data: id: 00000078 flags: 00000c08, ts: 0addbed3 - this may be a problem with spi signal quality- try reducing spi-clock speed if this can get reproduced
>> Jun 11 21:27:08 rpi4 kernel: mcp25xxfd spi0.0 can1: tefif: fifo 7 not pending - tef data: id: 00000078 flags: 00000e08, ts: 0addc2e5 - this may be a problem with spi signal quality- try reducing spi-clock speed if this can get reproduced
>> Jun 11 21:27:08 rpi4 kernel: mcp25xxfd spi0.0 can1: tefif: fifo 1 not pending - tef data: id: 00000078 flags: 00000208, ts: 0addc701 - this may be a problem with spi signal quality- try reducing spi-clock speed if this can get reproduced
>> Jun 11 21:27:08 rpi4 kernel: mcp25xxfd spi0.0 can1: tefif: fifo 2 not pending - tef data: id: 00000078 flags: 00000408, ts: 0addcb1b - this may be a problem with spi signal quality- try reducing spi-clock speed if this can get reproduced

Further two mcp2517 on the rpi4, unloaded system, performance dovernor running
cansequence -v can0 -p; cansequence can1 -v; gives:

> Jun 12 12:17:53 rpi4 kernel: mcp25xxfd spi1.0: ECC single bit error at 800
> Jun 12 12:17:53 rpi4 kernel: mcp25xxfd spi1.0: ECC single bit error at 800
> Jun 12 12:17:54 rpi4 kernel: mcp25xxfd spi1.0: ECC double bit error at 800
> Jun 12 12:17:54 rpi4 kernel: mcp25xxfd spi1.0: unidentified system interrupt - intf = b91a1118
> Jun 12 12:17:54 rpi4 kernel: mcp25xxfd spi1.0: ECC double bit error at 800

I don't see these problems with my driver.

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 |

2020-06-17 17:14:21

by Kurt Van Dijck

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On do, 11 jun 2020 18:26:19 +0200, Marc Kleine-Budde wrote:
> On 6/10/20 9:44 AM, Manivannan Sadhasivam wrote:
> > Hello,
> >
> > This series adds CAN network driver support for Microchip MCP25XXFD CAN
> > Controller with MCP2517FD as the target controller version. This series is
> > mostly inspired (or taken) from the previous iterations posted by Martin Sperl.
> > I've trimmed down the parts which are not necessary for the initial version
> > to ease review. Still the series is relatively huge but I hope to get some
> > reviews (post -rcX ofc!).
> >
> > Link to the origial series posted by Martin:
> > https://www.spinics.net/lists/devicetree/msg284462.html
> >
> > I've not changed the functionality much but done some considerable amount of
> > cleanups and also preserved the authorship of Martin for all the patches he has
> > posted earlier. This series has been tested on 96Boards RB3 platform by myself
> > and Martin has tested the previous version on Rpi3 with external MCP2517FD
> > controller.
>
> I initially started looking at Martin's driver and it was not using several
> modern CAN driver infrastructures. I then posted some cleanup patches but Martin
> was not working on the driver any more. Then I decided to rewrite the driver,
> that is the one I'm hoping to mainline soon.
>
> Can you give it a try?
>
> https://github.com/marckleinebudde/linux/commits/v5.6-rpi/mcp25xxfd-20200607-41

Hi Marc,

I'm in the process of getting a Variscite imx8m mini SOM online, with
MCP2517FD. The 4.19 kernel that comes with it, has a driver that is
clearly inspired by the one of Martin Sperl (not investigated too much
yet). I have problems of probing the chip when the bus is under full
load (under not full load, the probing only occasionally fails) due to
the modeswitch test.

Is there a real difference in yours between the rpi and sunxi branches?
Is there much evolution since -36 or would you mind to backport your
latest -42 to 4.19?

I will work on this the upcoming days. I can't do extensive tests, but
rather a works-for-me test that includes bitrate probe.
I only have 1 such board right now, and no other FD hardware.

Kurt
>
> Marc

2020-06-17 22:39:22

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On 6/17/20 6:59 PM, Kurt Van Dijck wrote:
> I'm in the process of getting a Variscite imx8m mini SOM online, with

Have your heard about the imx8m plus? It has CAN cores! We have a board in the
office to play with. :)

> MCP2517FD. The 4.19 kernel that comes with it, has a driver that is

You shall not start projects with 1,5 years old kernel.
And you probably shall not use vendor kernel for new projects.
:D

> clearly inspired by the one of Martin Sperl (not investigated too much
> yet). I have problems of probing the chip when the bus is under full
> load (under not full load, the probing only occasionally fails) due to
> the modeswitch test.
>
> Is there a real difference in yours between the rpi and sunxi branches?

The sunxi branch has some sunxi SPI driver improvements, the rpi branch some for
the raspi SPI drivers. All branches with the same -xx should have the same
mcp25xxfd driver.

With the exception the latest version is v5.6-rpi/mcp25xxfd-20200607-41, which
is cleaned up for mainlining (it has the logging and dump stuff removed).

> Is there much evolution since -36 or would you mind to backport your
> latest -42 to 4.19?

Not much, some bus off cleanups, however I've backported all changes to
v4.19-rpi/mcp25xxfd-20200429-41 (debug and log is still included).

When you port this to your mx8 take all from (including)

097701d1ea4f can: dev: avoid long lines
to
v4.19-rpi/mcp25xxfd-20200429-41

> I will work on this the upcoming days. I can't do extensive tests, but
> rather a works-for-me test that includes bitrate probe.
> I only have 1 such board right now, and no other FD hardware.

Great, looking for feedback! Especially how the driver copes with your fully
loaded bus test.

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 |

2020-06-18 10:01:21

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

Hi,

On 0611, Marc Kleine-Budde wrote:
> On 6/10/20 9:44 AM, Manivannan Sadhasivam wrote:
> > Hello,
> >
> > This series adds CAN network driver support for Microchip MCP25XXFD CAN
> > Controller with MCP2517FD as the target controller version. This series is
> > mostly inspired (or taken) from the previous iterations posted by Martin Sperl.
> > I've trimmed down the parts which are not necessary for the initial version
> > to ease review. Still the series is relatively huge but I hope to get some
> > reviews (post -rcX ofc!).
> >
> > Link to the origial series posted by Martin:
> > https://www.spinics.net/lists/devicetree/msg284462.html
> >
> > I've not changed the functionality much but done some considerable amount of
> > cleanups and also preserved the authorship of Martin for all the patches he has
> > posted earlier. This series has been tested on 96Boards RB3 platform by myself
> > and Martin has tested the previous version on Rpi3 with external MCP2517FD
> > controller.
>
> I initially started looking at Martin's driver and it was not using several
> modern CAN driver infrastructures. I then posted some cleanup patches but Martin
> was not working on the driver any more. Then I decided to rewrite the driver,
> that is the one I'm hoping to mainline soon.
>

So how should we proceed from here? It is okay for me to work on adding some
features and also fixing the issues you've reported so far. But I want to reach
a consensus before moving forward.

If you think that it makes sense to go with your set of patches, then I need an
estimate on when you'll post the first revision.

> Can you give it a try?
>
> https://github.com/marckleinebudde/linux/commits/v5.6-rpi/mcp25xxfd-20200607-41
>

Sure thing. Will do.

Thanks,
Mani

> 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 |

2020-06-18 12:11:57

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

Hi,

On 0611, Marc Kleine-Budde wrote:
> On 6/10/20 9:44 AM, Manivannan Sadhasivam wrote:
> > Hello,
> >
> > This series adds CAN network driver support for Microchip MCP25XXFD CAN
> > Controller with MCP2517FD as the target controller version. This series is
> > mostly inspired (or taken) from the previous iterations posted by Martin Sperl.
> > I've trimmed down the parts which are not necessary for the initial version
> > to ease review. Still the series is relatively huge but I hope to get some
> > reviews (post -rcX ofc!).
> >
> > Link to the origial series posted by Martin:
> > https://www.spinics.net/lists/devicetree/msg284462.html
> >
> > I've not changed the functionality much but done some considerable amount of
> > cleanups and also preserved the authorship of Martin for all the patches he has
> > posted earlier. This series has been tested on 96Boards RB3 platform by myself
> > and Martin has tested the previous version on Rpi3 with external MCP2517FD
> > controller.
>
> I initially started looking at Martin's driver and it was not using several
> modern CAN driver infrastructures. I then posted some cleanup patches but Martin
> was not working on the driver any more. Then I decided to rewrite the driver,
> that is the one I'm hoping to mainline soon.
>
> Can you give it a try?
>
> https://github.com/marckleinebudde/linux/commits/v5.6-rpi/mcp25xxfd-20200607-41
>

Tested this version on my board with MCP2518FD and it works fine. I'm okay with
moving forward with your version and would like to contribute. Please let me
know how we can move forward.

Thanks,
Mani

> 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 |

2020-06-18 12:35:39

by Kurt Van Dijck

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On do, 18 jun 2020 00:36:29 +0200, Marc Kleine-Budde wrote:
> On 6/17/20 6:59 PM, Kurt Van Dijck wrote:
> > I'm in the process of getting a Variscite imx8m mini SOM online, with
>
> Have your heard about the imx8m plus? It has CAN cores! We have a board in the
> office to play with. :)
>
> > MCP2517FD. The 4.19 kernel that comes with it, has a driver that is
>
> You shall not start projects with 1,5 years old kernel.
> And you probably shall not use vendor kernel for new projects.
> :D

your rpi kernel starts of v4.19.119 (or so), where the variscite starts
of v4.19.35.
The result was quite some list of merge conflicts, on top of a vendor
kernel, so I took your advise and switched to the latest and greatest
5.7.3.
Luckily, we need no sound (yet) and no video. :-)

>
> Not much, some bus off cleanups, however I've backported all changes to
> v4.19-rpi/mcp25xxfd-20200429-41 (debug and log is still included).
>
> When you port this to your mx8 take all from (including)
>
> 097701d1ea4f can: dev: avoid long lines
> to
> v4.19-rpi/mcp25xxfd-20200429-41
>

I'll merge one of your latest ...

Kurt

2020-06-18 12:38:10

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On 6/18/20 2:30 PM, Kurt Van Dijck wrote:
> On do, 18 jun 2020 00:36:29 +0200, Marc Kleine-Budde wrote:
>> On 6/17/20 6:59 PM, Kurt Van Dijck wrote:
>>> I'm in the process of getting a Variscite imx8m mini SOM online, with
>>
>> Have your heard about the imx8m plus? It has CAN cores! We have a board in the
>> office to play with. :)
>>
>>> MCP2517FD. The 4.19 kernel that comes with it, has a driver that is
>>
>> You shall not start projects with 1,5 years old kernel.
>> And you probably shall not use vendor kernel for new projects.
>> :D
>
> your rpi kernel starts of v4.19.119 (or so), where the variscite starts
> of v4.19.35.

You're missing some stable backports for the kernel then.

> The result was quite some list of merge conflicts, on top of a vendor
> kernel, so I took your advise and switched to the latest and greatest
> 5.7.3.

\o/

> Luckily, we need no sound (yet) and no video. :-)

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 |

2020-06-20 05:05:09

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On Thu, Jun 18, 2020 at 02:25:33PM +0530, Manivannan Sadhasivam wrote:
> Hi,
>
> On 0611, Marc Kleine-Budde wrote:
> > On 6/10/20 9:44 AM, Manivannan Sadhasivam wrote:
> > > Hello,
> > >
> > > This series adds CAN network driver support for Microchip MCP25XXFD CAN
> > > Controller with MCP2517FD as the target controller version. This series is
> > > mostly inspired (or taken) from the previous iterations posted by Martin Sperl.
> > > I've trimmed down the parts which are not necessary for the initial version
> > > to ease review. Still the series is relatively huge but I hope to get some
> > > reviews (post -rcX ofc!).
> > >
> > > Link to the origial series posted by Martin:
> > > https://www.spinics.net/lists/devicetree/msg284462.html
> > >
> > > I've not changed the functionality much but done some considerable amount of
> > > cleanups and also preserved the authorship of Martin for all the patches he has
> > > posted earlier. This series has been tested on 96Boards RB3 platform by myself
> > > and Martin has tested the previous version on Rpi3 with external MCP2517FD
> > > controller.
> >
> > I initially started looking at Martin's driver and it was not using several
> > modern CAN driver infrastructures. I then posted some cleanup patches but Martin
> > was not working on the driver any more. Then I decided to rewrite the driver,
> > that is the one I'm hoping to mainline soon.
> >
>
> So how should we proceed from here? It is okay for me to work on adding some
> features and also fixing the issues you've reported so far. But I want to reach
> a consensus before moving forward.
>
> If you think that it makes sense to go with your set of patches, then I need an
> estimate on when you'll post the first revision.
>

Ping!

> > Can you give it a try?
> >
> > https://github.com/marckleinebudde/linux/commits/v5.6-rpi/mcp25xxfd-20200607-41
> >
>
> Sure thing. Will do.
>
> Thanks,
> Mani
>
> > 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 |

2020-06-22 10:28:38

by Kurt Van Dijck

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

Hey Marc,

On do, 18 jun 2020 14:35:28 +0200, Marc Kleine-Budde wrote:
> On 6/18/20 2:30 PM, Kurt Van Dijck wrote:
> > On do, 18 jun 2020 00:36:29 +0200, Marc Kleine-Budde wrote:
> >> On 6/17/20 6:59 PM, Kurt Van Dijck wrote:
> >>> I'm in the process of getting a Variscite imx8m mini SOM online, with
> >>
> >> Have your heard about the imx8m plus? It has CAN cores! We have a board in the
> >> office to play with. :)
> >>
> >>> MCP2517FD. The 4.19 kernel that comes with it, has a driver that is
> >>
> >> You shall not start projects with 1,5 years old kernel.
> >> And you probably shall not use vendor kernel for new projects.
> >> :D
> >
> > your rpi kernel starts of v4.19.119 (or so), where the variscite starts
> > of v4.19.35.
>
> You're missing some stable backports for the kernel then.
>
> > The result was quite some list of merge conflicts, on top of a vendor
> > kernel, so I took your advise and switched to the latest and greatest
> > 5.7.3.
>
> \o/

I got my board up with a 5.7, despite device-tree problems completely
unrelated to CAN.
It seems to work well with a fully-loaded CAN bus (cangen -g0 ...).
So that is a real improvement.
I will need to add the listen-only mode soon.

Kurt

2020-06-22 10:57:59

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On 6/22/20 12:25 PM, Kurt Van Dijck wrote:
> I got my board up with a 5.7, despite device-tree problems completely
> unrelated to CAN.

\o/

> It seems to work well with a fully-loaded CAN bus (cangen -g0 ...).
> So that is a real improvement.

Can I add your Tested-by?

> I will need to add the listen-only mode soon.

Patches are always welcome!

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 |

2020-06-22 11:45:48

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On 6/18/20 2:06 PM, Manivannan Sadhasivam wrote:
>> https://github.com/marckleinebudde/linux/commits/v5.6-rpi/mcp25xxfd-20200607-41
>>
>
> Tested this version on my board with MCP2518FD and it works fine. I'm okay with
> moving forward with your version and would like to contribute. Please let me
> know how we can move forward.

Can I add your Tested-by?

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 |

2020-06-22 11:50:26

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On 6/18/20 10:55 AM, Manivannan Sadhasivam wrote:
> So how should we proceed from here? It is okay for me to work on adding some
> features and also fixing the issues you've reported so far. But I want to reach
> a consensus before moving forward.
>
> If you think that it makes sense to go with your set of patches, then I need an
> estimate on when you'll post the first revision.

Done, I've posted my current version, which is -41.

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 |

2020-06-22 12:35:43

by Kurt Van Dijck

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On ma, 22 jun 2020 12:55:41 +0200, Marc Kleine-Budde wrote:
> Date: Mon, 22 Jun 2020 12:55:41 +0200
> From: Marc Kleine-Budde <[email protected]>
> To: Manivannan Sadhasivam <[email protected]>,
> [email protected], [email protected], [email protected],
> [email protected], [email protected]
> Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver
> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
> Thunderbird/68.9.0
>
> On 6/22/20 12:25 PM, Kurt Van Dijck wrote:
> > I got my board up with a 5.7, despite device-tree problems completely
> > unrelated to CAN.
>
> \o/
>
> > It seems to work well with a fully-loaded CAN bus (cangen -g0 ...).
> > So that is a real improvement.
>
> Can I add your Tested-by?
yes.
>
> > I will need to add the listen-only mode soon.
>
> Patches are always welcome!
>
> 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 |

2020-06-22 12:46:27

by Kurt Van Dijck

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

Marc,

I get RX-0: FIFO overflows in listen-only mode (back-to-back burst of
the single other node).
The SPI peripheral does not use DMA :-(.
Do you have, by accident, some freescale SPI fixes lying around?

It's not the biggest problem on my side, but is proves the system not
being guarded against load.

Kurt

On ma, 22 jun 2020 14:30:31 +0200, Kurt Van Dijck wrote:
>
> On ma, 22 jun 2020 12:55:41 +0200, Marc Kleine-Budde wrote:
> > On 6/22/20 12:25 PM, Kurt Van Dijck wrote:
> > > I got my board up with a 5.7, despite device-tree problems completely
> > > unrelated to CAN.
> >
> > \o/
> >
> > > It seems to work well with a fully-loaded CAN bus (cangen -g0 ...).
> > > So that is a real improvement.
> >
> > Can I add your Tested-by?
> yes.
> >
> > > I will need to add the listen-only mode soon.
> >
> > Patches are always welcome!
> >
> > regards,
> > Marc

2020-06-22 12:56:33

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On 6/22/20 2:43 PM, Kurt Van Dijck wrote:
> I get RX-0: FIFO overflows in listen-only mode (back-to-back burst of
> the single other node).

Single other node? Who's ACKing the CAN frames?

> The SPI peripheral does not use DMA :-(.

The SPI messages are quite small, so DMA wont help either. Getting rid of the
IRQ and polling for completion is the way to go.

> Do you have, by accident, some freescale SPI fixes lying around?

nope

> It's not the biggest problem on my side, but is proves the system not
> being guarded against load.

Do you have freq scaling activated?

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 |

2020-06-22 13:31:23

by Kurt Van Dijck

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On ma, 22 jun 2020 14:54:15 +0200, Marc Kleine-Budde wrote:
> On 6/22/20 2:43 PM, Kurt Van Dijck wrote:
> > I get RX-0: FIFO overflows in listen-only mode (back-to-back burst of
> > the single other node).
>
> Single other node? Who's ACKing the CAN frames?

hence the back-to-back burst.

>
> > The SPI peripheral does not use DMA :-(.
>
> The SPI messages are quite small, so DMA wont help either. Getting rid of the
> IRQ and polling for completion is the way to go.
>
> > Do you have, by accident, some freescale SPI fixes lying around?
>
> nope
>
> > It's not the biggest problem on my side, but is proves the system not
> > being guarded against load.
>
> Do you have freq scaling activated?

Not yet.

The device tree needs upgrading ... grrr

>
> 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 |

2020-06-22 13:43:13

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver

On 6/22/20 3:26 PM, Kurt Van Dijck wrote:
> On ma, 22 jun 2020 14:54:15 +0200, Marc Kleine-Budde wrote:
>> On 6/22/20 2:43 PM, Kurt Van Dijck wrote:
>>> I get RX-0: FIFO overflows in listen-only mode (back-to-back burst of
>>> the single other node).
>>
>> Single other node? Who's ACKing the CAN frames?
>
> hence the back-to-back burst.

Just wanted to be sure if I understood correctly. Nice testcase btw!

>>> The SPI peripheral does not use DMA :-(.
>>
>> The SPI messages are quite small, so DMA wont help either. Getting rid of the
>> IRQ and polling for completion is the way to go.
>>
>>> Do you have, by accident, some freescale SPI fixes lying around?
>>
>> nope
>>
>>> It's not the biggest problem on my side, but is proves the system not
>>> being guarded against load.
>>
>> Do you have freq scaling activated?
>
> Not yet.
>
> The device tree needs upgrading ... grrr

Without freq scaling the imx core is supposed to run at full speed, which is
better for bursty SPI traffic than starting clocked down into a burst....

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 |