can: mcp25xxfd: optionally enable SPI crc commands for reads

Allow the (optional) use of SPI CRC read commands that can get enabled
via a module parameter.

In the future it may become necessary to enable this for register reads
(or at least some specific register reads) as there is a HW bug that
may result in glitches on the spi bus.

Possible variants here are:
* just retry the read
* do not use some fifos (7, 15, 23, 31) at all
* test for bit flips of specific bits and retest CRC

--
Changelog:
  V7 -> V8: added in V8

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
(cherry picked from commit 1e37e00f473bc9b9f192234a40e4b0ffac1d27ad)

7 files changed, 182 insertions, 10 deletions

diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.c b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.c
index 362b1a152b08..3625567ffa15 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.c
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.c
@@ -627,7 +627,12 @@ irqreturn_t mcp25xxfd_can_int(int irq, void *dev_id)
 					      MCP25XXFD_CAN_INT,
 					      &cpriv->status.intf,
 					      sizeof(cpriv->status));
-		if (ret) {
+		switch (ret) {
+		case 0: /* no errors, so process */
+			break;
+		case -EILSEQ: /* a crc error, so run the loop again */
+			continue;
+		default: /* all other error cases */
 			netdev_err(cpriv->can.dev,
 				   "reading can registers returned with error code %i\n",
 				   ret);
@@ -642,10 +647,15 @@ irqreturn_t mcp25xxfd_can_int(int irq, void *dev_id)
 		       cpriv->status.intf) == 0)
 			break;
 
-		/* handle the status */
+		/* handle the interrupts for real */
 		ret = mcp25xxfd_can_int_handle_status(cpriv);
-		if (ret)
+		switch (ret) {
+		case 0: /* no errors, so process */
+		case -EILSEQ: /* a crc error, so run the loop again */
+			break;
+		default: /* all other error cases */
 			goto fail;
+		}
 
 		/* allow voluntarily rescheduling every so often to avoid
 		 * long CS lows at the end of a transfer on low power CPUs
@@ -664,6 +674,9 @@ fail:
  	netdev_err(cpriv->can.dev,
 		   "experienced unexpected error %i in interrupt handler - disabling interrupts\n",
  		   ret);
+	/* note that if we experienced an spi error,
+	 * then this would produce another error
+	 */
 	mcp25xxfd_int_enable(cpriv->priv, false);
 
 	/* we could also put the driver in bus-off mode */
diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.c b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.c
index f8bd7b792412..6d895613111e 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.c
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.c
@@ -5,13 +5,20 @@
  * Copyright 2019 Martin Sperl <kernel@martin.sperl.org>
  */
 
+#include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
 
 #include "mcp25xxfd_cmd.h"
+#include "mcp25xxfd_crc.h"
 #include "mcp25xxfd_priv.h"
 
+/* module parameter */
+static bool use_spi_crc;
+module_param(use_spi_crc, bool, 0664);
+MODULE_PARM_DESC(use_spi_crc, "Use SPI CRC instruction\n");
+
 /* SPI helper */
 
 /* wrapper arround spi_sync, that sets speed_hz */
@@ -92,15 +99,18 @@ static int mcp25xxfd_cmd_write_then_read(struct spi_device *spi,
 					 const void *tx_buf,
 					 unsigned int tx_len,
 					 void *rx_buf,
-					 unsigned int rx_len)
+					 unsigned int rx_len,
+					 void *crc_buf)
 {
+	int crc_len = crc_buf ? 2 : 0;
 	struct spi_transfer xfer[2];
 	u8 *spi_tx, *spi_rx;
 	int xfers;
 	int ret;
 
 	/* get pointer to buffers */
-	ret = mcp25xxfd_cmd_alloc_buf(spi, tx_len + rx_len, &spi_tx, &spi_rx);
+	ret = mcp25xxfd_cmd_alloc_buf(spi, tx_len + rx_len + crc_len,
+				      &spi_tx, &spi_rx);
 	if (ret)
 		return ret;
 
@@ -114,10 +124,10 @@ static int mcp25xxfd_cmd_write_then_read(struct spi_device *spi,
 		xfer[0].len = tx_len;
 		/* the offset for rx_buf needs to get aligned */
 		xfer[1].rx_buf = spi_rx + tx_len;
-		xfer[1].len = rx_len;
+		xfer[1].len = rx_len + crc_len;
 	} else {
 		xfers = 1;
-		xfer[0].len = tx_len + rx_len;
+		xfer[0].len = tx_len + rx_len + crc_len;
 		xfer[0].tx_buf = spi_tx;
 		xfer[0].rx_buf = spi_rx;
 	}
@@ -132,6 +142,8 @@ static int mcp25xxfd_cmd_write_then_read(struct spi_device *spi,
 
 	/* copy result back */
 	memcpy(rx_buf, xfer[0].rx_buf + tx_len, rx_len);
+	if (crc_buf)
+		memcpy(crc_buf, xfer[0].rx_buf + tx_len + rx_len, crc_len);
 
 out:
 	mcp25xxfd_cmd_release_buf(spi, spi_tx, spi_rx);
@@ -182,10 +194,81 @@ int mcp25xxfd_cmd_readn(struct spi_device *spi, u32 reg,
 
 	mcp25xxfd_cmd_calc(MCP25XXFD_INSTRUCTION_READ, reg, cmd);
 
-	ret = mcp25xxfd_cmd_write_then_read(spi, &cmd, 2, data, n);
+	ret = mcp25xxfd_cmd_write_then_read(spi, &cmd, 2, data, n, NULL);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static u16 _mcp25xxfd_cmd_compute_crc(u8 *cmd, u8 *data, int n)
+{
+	u16 crc = 0xffff;
+
+	crc = mcp25xxfd_crc(crc, cmd, 3);
+	crc = mcp25xxfd_crc(crc, data, n);
+
+	return crc;
+}
+
+static int _mcp25xxfd_cmd_readn_crc(struct spi_device *spi, u32 reg,
+				    void *data, int n)
+{
+	u8 cmd[3], crcd[2];
+	u16 crcc, crcr;
+	int ret;
+
+	/* prepare command */
+	mcp25xxfd_cmd_calc(MCP25XXFD_INSTRUCTION_READ_CRC, reg, cmd);
+	/* count depends on word (=RAM) or byte access (Registers) */
+	if (reg < MCP25XXFD_SRAM_ADDR(0) ||
+	    reg >= MCP25XXFD_SRAM_ADDR(MCP25XXFD_SRAM_SIZE))
+		cmd[2] = n;
+	else
+		cmd[2] = n / 4;
+
+	/* now read for real */
+	ret = mcp25xxfd_cmd_write_then_read(spi, &cmd, 3, data, n, crcd);
 	if (ret)
 		return ret;
 
+	/* the received crc */
+	crcr = (crcd[0] << 8) + crcd[1];
+
+	/* compute the crc */
+	crcc = _mcp25xxfd_cmd_compute_crc(cmd, data, n);
+
+	/* if it matches, then return */
+	if (crcc == crcr)
+		return 0;
+
+	/* here possibly handle crc variants with a single bit7 flips */
+
+	/* return with error and rate limited */
+	dev_err_ratelimited(&spi->dev,
+			    "CRC read error: computed: %04x received: %04x - data: %*ph %*ph%s\n",
+			    crcc, crcr, 3, cmd, min_t(int, 64, n), data,
+			    (n > 64) ? "..." : "");
+	return -EILSEQ;
+}
+
+static int mcp25xxfd_cmd_readn_crc(struct spi_device *spi, u32 reg,
+				   void *data, int n)
+{
+	struct mcp25xxfd_priv *priv = spi_get_drvdata(spi);
+	int ret;
+
+	for (; n > 0; n -= 254, reg += 254, data += 254) {
+#if defined(CONFIG_DEBUG_FS)
+		priv->stats.spi_crc_read++;
+		if (n > 254)
+			priv->stats.spi_crc_read_split++;
+#endif
+		ret = _mcp25xxfd_cmd_readn_crc(spi, reg, data, n);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -283,8 +366,13 @@ int mcp25xxfd_cmd_read_regs(struct spi_device *spi, u32 reg,
 {
 	int ret;
 
-	/* read it */
-	ret = mcp25xxfd_cmd_readn(spi, reg, data, bytes);
+	/* read it using crc */
+	if ((use_spi_crc) || (reg & MCP25XXFD_ADDRESS_WITH_CRC))
+		ret = mcp25xxfd_cmd_readn_crc(spi,
+					      reg & MCP25XXFD_ADDRESS_MASK,
+					      data, bytes);
+	else
+		ret = mcp25xxfd_cmd_readn(spi, reg, data, bytes);
 
 	/* and convert it to cpu format */
 	mcp25xxfd_cmd_convert_to_cpu((u32 *)data, bytes / sizeof(bytes));
diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.h b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.h
index c9b8ae4db151..595b85294452 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.h
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.h
@@ -20,6 +20,8 @@
 #define MCP25XXFD_INSTRUCTION_WRITE_SAVE	0xC000
 
 #define MCP25XXFD_ADDRESS_MASK			0x0fff
+/* a bit to use CRC commands if possible */
+#define MCP25XXFD_ADDRESS_WITH_CRC		BIT(31)
 
 static inline void mcp25xxfd_cmd_convert_to_cpu(u32 *data, int n)
 {
diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.c b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.c
index b893d8009448..466f989845dd 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.c
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.c
@@ -12,6 +12,63 @@
 #include "mcp25xxfd_regs.h"
 #include "mcp25xxfd_priv.h"
 
+/* the standard crc16 in linux/crc16.h is unfortunately not
+ * computing the correct results (left shift vs. right shift)
+ * so here an implementation with a table generated by:
+ * http://lkml.iu.edu/hypermail/linux/kernel/0508.1/1085.html
+ *
+ * if someone has a better idea how to make crc16 produce the expected
+ * result, then please come forward...
+ */
+static const u16 _mcp25xxfd_crc_table[256] = {
+	0x0000, 0x8005, 0x800f, 0x000a, 0x801b, 0x001e, 0x0014, 0x8011,
+	0x8033, 0x0036, 0x003c, 0x8039, 0x0028, 0x802d, 0x8027, 0x0022,
+	0x8063, 0x0066, 0x006c, 0x8069, 0x0078, 0x807d, 0x8077, 0x0072,
+	0x0050, 0x8055, 0x805f, 0x005a, 0x804b, 0x004e, 0x0044, 0x8041,
+	0x80c3, 0x00c6, 0x00cc, 0x80c9, 0x00d8, 0x80dd, 0x80d7, 0x00d2,
+	0x00f0, 0x80f5, 0x80ff, 0x00fa, 0x80eb, 0x00ee, 0x00e4, 0x80e1,
+	0x00a0, 0x80a5, 0x80af, 0x00aa, 0x80bb, 0x00be, 0x00b4, 0x80b1,
+	0x8093, 0x0096, 0x009c, 0x8099, 0x0088, 0x808d, 0x8087, 0x0082,
+	0x8183, 0x0186, 0x018c, 0x8189, 0x0198, 0x819d, 0x8197, 0x0192,
+	0x01b0, 0x81b5, 0x81bf, 0x01ba, 0x81ab, 0x01ae, 0x01a4, 0x81a1,
+	0x01e0, 0x81e5, 0x81ef, 0x01ea, 0x81fb, 0x01fe, 0x01f4, 0x81f1,
+	0x81d3, 0x01d6, 0x01dc, 0x81d9, 0x01c8, 0x81cd, 0x81c7, 0x01c2,
+	0x0140, 0x8145, 0x814f, 0x014a, 0x815b, 0x015e, 0x0154, 0x8151,
+	0x8173, 0x0176, 0x017c, 0x8179, 0x0168, 0x816d, 0x8167, 0x0162,
+	0x8123, 0x0126, 0x012c, 0x8129, 0x0138, 0x813d, 0x8137, 0x0132,
+	0x0110, 0x8115, 0x811f, 0x011a, 0x810b, 0x010e, 0x0104, 0x8101,
+	0x8303, 0x0306, 0x030c, 0x8309, 0x0318, 0x831d, 0x8317, 0x0312,
+	0x0330, 0x8335, 0x833f, 0x033a, 0x832b, 0x032e, 0x0324, 0x8321,
+	0x0360, 0x8365, 0x836f, 0x036a, 0x837b, 0x037e, 0x0374, 0x8371,
+	0x8353, 0x0356, 0x035c, 0x8359, 0x0348, 0x834d, 0x8347, 0x0342,
+	0x03c0, 0x83c5, 0x83cf, 0x03ca, 0x83db, 0x03de, 0x03d4, 0x83d1,
+	0x83f3, 0x03f6, 0x03fc, 0x83f9, 0x03e8, 0x83ed, 0x83e7, 0x03e2,
+	0x83a3, 0x03a6, 0x03ac, 0x83a9, 0x03b8, 0x83bd, 0x83b7, 0x03b2,
+	0x0390, 0x8395, 0x839f, 0x039a, 0x838b, 0x038e, 0x0384, 0x8381,
+	0x0280, 0x8285, 0x828f, 0x028a, 0x829b, 0x029e, 0x0294, 0x8291,
+	0x82b3, 0x02b6, 0x02bc, 0x82b9, 0x02a8, 0x82ad, 0x82a7, 0x02a2,
+	0x82e3, 0x02e6, 0x02ec, 0x82e9, 0x02f8, 0x82fd, 0x82f7, 0x02f2,
+	0x02d0, 0x82d5, 0x82df, 0x02da, 0x82cb, 0x02ce, 0x02c4, 0x82c1,
+	0x8243, 0x0246, 0x024c, 0x8249, 0x0258, 0x825d, 0x8257, 0x0252,
+	0x0270, 0x8275, 0x827f, 0x027a, 0x826b, 0x026e, 0x0264, 0x8261,
+	0x0220, 0x8225, 0x822f, 0x022a, 0x823b, 0x023e, 0x0234, 0x8231,
+	0x8213, 0x0216, 0x021c, 0x8219, 0x0208, 0x820d, 0x8207, 0x0202
+};
+
+static inline u16 mcp25xxfd_crc_byte(u16 crc, const u8 data)
+{
+	u8 index = (crc >> 8) ^ data;
+
+	return (crc << 8) ^ _mcp25xxfd_crc_table[index];
+}
+
+u16 mcp25xxfd_crc(u16 crc, u8 const *buffer, size_t len)
+{
+	while (len--)
+		crc = mcp25xxfd_crc_byte(crc, *buffer++);
+	return crc;
+}
+
 int mcp25xxfd_crc_enable_int(struct mcp25xxfd_priv *priv, bool enable)
 {
 	u32 mask = MCP25XXFD_CRC_CRCERRIE | MCP25XXFD_CRC_FERRIE;
diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.h b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.h
index 6e42fe0fad0f..25db96b0b9c9 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.h
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.h
@@ -12,4 +12,6 @@
 int mcp25xxfd_crc_enable_int(struct mcp25xxfd_priv *priv, bool enable);
 int mcp25xxfd_crc_clear_int(struct mcp25xxfd_priv *priv);
 
+u16 mcp25xxfd_crc(u16 crc, u8 const *buffer, size_t len);
+
 #endif /* __MCP25XXFD_CRC_H */
diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_debugfs.c b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_debugfs.c
index 33770e80f7f8..303bcba27919 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_debugfs.c
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_debugfs.c
@@ -76,6 +76,12 @@ static void mcp25xxfd_debugfs_mod_setup(struct mcp25xxfd_priv *priv)
 			   &priv->spi_use_speed_hz);
 	debugfs_create_u32("clk_user_mask", 0444, root, &priv->clk_user_mask);
 
+	/* some statistics */
+	debugfs_create_u64("spi_crc_read", 0444, root,
+			   &priv->stats.spi_crc_read);
+	debugfs_create_u64("spi_crc_read_split", 0444, root,
+			   &priv->stats.spi_crc_read_split);
+
 	/* expose the system registers */
 	priv->debugfs_regs_dir = debugfs_create_dir("regs", root);
 	regs = priv->debugfs_regs_dir;
diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_priv.h b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_priv.h
index ad0d5e4b18ec..89bd39779b28 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_priv.h
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_priv.h
@@ -81,6 +81,10 @@ struct mcp25xxfd_priv {
 #if defined(CONFIG_DEBUG_FS)
 	struct dentry *debugfs_dir;
 	struct dentry *debugfs_regs_dir;
+	struct {
+		u64 spi_crc_read;
+		u64 spi_crc_read_split;
+	} stats;
 #endif
 };