From 069c7ba69461c9ad0c7d3a1fe9c574e1b261fa3d Mon Sep 17 00:00:00 2001 From: Martin Sperl Date: Thu, 28 Feb 2019 18:39:01 +0000 Subject: 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 (cherry picked from commit 1e37e00f473bc9b9f192234a40e4b0ffac1d27ad) --- drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.c | 19 +++- drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.c | 102 ++++++++++++++++++++-- drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.h | 2 + drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.c | 57 ++++++++++++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.h | 2 + drivers/net/can/spi/mcp25xxfd/mcp25xxfd_debugfs.c | 6 ++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_priv.h | 4 + 7 files changed, 182 insertions(+), 10 deletions(-) (limited to 'drivers/net') 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 */ +#include #include #include #include #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 }; -- cgit v1.2.3