From: David Mosberger-Tang <davidm@egauge.net>
To: linux-wireless@vger.kernel.org
Cc: Ajay Singh <ajay.kathat@microchip.com>,
Claudiu Beznea <claudiu.beznea@microchip.com>,
davidm@egauge.net
Subject: [PATCH 4/4] wilc1000: Add support for enabling CRC
Date: Wed, 24 Feb 2021 05:52:00 +0000 (UTC) [thread overview]
Message-ID: <20210224055135.1509200-4-davidm@egauge.net> (raw)
In-Reply-To: <20210224055135.1509200-1-davidm@egauge.net>
The driver so far has always disabled CRC protection. This means any
data corruption that occurred during the SPI transfers could
potentially go unnoticed. This patch adds the macros ENABLE_CRC7 and
ENABLE_CRC16 to allow compile-time selection of whether or not CRC7
and CRC16, respectively, should be enabled.
The default configuration remains unchanged, with both CRC7 and CRC16
off.
Signed-off-by: David Mosberger-Tang <davidm@egauge.net>
---
.../net/wireless/microchip/wilc1000/Kconfig | 1 +
drivers/net/wireless/microchip/wilc1000/spi.c | 151 +++++++++++++-----
2 files changed, 108 insertions(+), 44 deletions(-)
diff --git a/drivers/net/wireless/microchip/wilc1000/Kconfig b/drivers/net/wireless/microchip/wilc1000/Kconfig
index 7f15e42602dd..62cfcdc9aacc 100644
--- a/drivers/net/wireless/microchip/wilc1000/Kconfig
+++ b/drivers/net/wireless/microchip/wilc1000/Kconfig
@@ -27,6 +27,7 @@ config WILC1000_SPI
depends on CFG80211 && INET && SPI
select WILC1000
select CRC7
+ select CRC_ITU_T
help
This module adds support for the SPI interface of adapters using
WILC1000 chipset. The Atmel WILC1000 has a Serial Peripheral
diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c b/drivers/net/wireless/microchip/wilc1000/spi.c
index b0e096a03a28..c745a440d273 100644
--- a/drivers/net/wireless/microchip/wilc1000/spi.c
+++ b/drivers/net/wireless/microchip/wilc1000/spi.c
@@ -7,10 +7,23 @@
#include <linux/clk.h>
#include <linux/spi/spi.h>
#include <linux/crc7.h>
+#include <linux/crc-itu-t.h>
#include "netdev.h"
#include "cfg80211.h"
+/**
+ * Establish the driver's desired CRC configuration. CRC7 is used for
+ * command transfers which have no other protection against corruption
+ * during the SPI transfer. Commands are short so CRC7 is relatively
+ * cheap. CRC16 is used for data transfers, including network packet
+ * transfers. Since those transfers can be large, CRC16 is relatively
+ * expensive. CRC16 is also often redundant as network packets
+ * typically are protected by their own, higher-level checksum.
+ */
+#define ENABLE_CRC7 0 /* set to 1 to protect SPI commands with CRC7 */
+#define ENABLE_CRC16 0 /* set to 1 to protect SPI data with CRC16 */
+
/*
* For CMD_SINGLE_READ and CMD_INTERNAL_READ, WILC may insert one or
* more zero bytes between the command response and the DATA Start tag
@@ -22,7 +35,8 @@
#define WILC_SPI_RSP_HDR_EXTRA_DATA 8
struct wilc_spi {
- int crc_off;
+ bool crc7_enabled;
+ bool crc16_enabled;
};
static const struct wilc_hif_func wilc_hif_spi;
@@ -123,6 +137,10 @@ static int wilc_bus_probe(struct spi_device *spi)
if (!spi_priv)
return -ENOMEM;
+ /* WILC chip resets to both CRCs enabled: */
+ spi_priv->crc7_enabled = true;
+ spi_priv->crc16_enabled = true;
+
ret = wilc_cfg80211_init(&wilc, &spi->dev, WILC_HIF_SPI, &wilc_hif_spi);
if (ret) {
kfree(spi_priv);
@@ -303,7 +321,8 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
struct wilc_spi *spi_priv = wilc->bus_data;
int ix, nbytes;
int result = 0;
- u8 cmd, order, crc[2] = {0};
+ u8 cmd, order, crc[2];
+ u16 crc_calc;
/*
* Data
@@ -345,9 +364,12 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
}
/*
- * Write Crc
+ * Write CRC
*/
- if (!spi_priv->crc_off) {
+ if (spi_priv->crc16_enabled) {
+ crc_calc = crc_itu_t(0xffff, &b[ix], nbytes);
+ crc[0] = crc_calc >> 8;
+ crc[1] = crc_calc;
if (wilc_spi_tx(wilc, crc, 2)) {
dev_err(&spi->dev, "Failed data block crc write, bus error...\n");
result = -EINVAL;
@@ -381,11 +403,11 @@ static int wilc_spi_single_read(struct wilc *wilc, u8 cmd, u32 adr, void *b,
struct spi_device *spi = to_spi_device(wilc->dev);
struct wilc_spi *spi_priv = wilc->bus_data;
u8 wb[32], rb[32];
- u8 crc[2];
int cmd_len, resp_len, i;
+ u16 crc_calc, crc_recv;
struct wilc_spi_cmd *c;
- struct wilc_spi_read_rsp_data *r_data;
struct wilc_spi_rsp_data *r;
+ struct wilc_spi_read_rsp_data *r_data;
memset(wb, 0x0, sizeof(wb));
memset(rb, 0x0, sizeof(rb));
@@ -409,7 +431,7 @@ static int wilc_spi_single_read(struct wilc *wilc, u8 cmd, u32 adr, void *b,
cmd_len = offsetof(struct wilc_spi_cmd, u.simple_cmd.crc);
resp_len = sizeof(*r) + sizeof(*r_data) + WILC_SPI_RSP_HDR_EXTRA_DATA;
- if (!spi_priv->crc_off) {
+ if (spi_priv->crc7_enabled) {
c->u.simple_cmd.crc[0] = wilc_get_crc7(wb, cmd_len);
cmd_len += 1;
resp_len += 2;
@@ -455,8 +477,16 @@ static int wilc_spi_single_read(struct wilc *wilc, u8 cmd, u32 adr, void *b,
if (b)
memcpy(b, r_data->data, 4);
- if (!spi_priv->crc_off)
- memcpy(crc, r_data->crc, 2);
+ if (!clockless && spi_priv->crc16_enabled) {
+ crc_recv = (r_data->crc[0] << 8) | r_data->crc[1];
+ crc_calc = crc_itu_t(0xffff, r_data->data, 4);
+ if (crc_recv != crc_calc) {
+ dev_err(&spi->dev, "%s: bad CRC 0x%04x "
+ "(calculated 0x%04x)\n", __func__,
+ crc_recv, crc_calc);
+ return -EINVAL;
+ }
+ }
return 0;
}
@@ -483,7 +513,7 @@ static int wilc_spi_write_cmd(struct wilc *wilc, u8 cmd, u32 adr, u32 data,
c->u.internal_w_cmd.addr[1] = adr;
c->u.internal_w_cmd.data = cpu_to_be32(data);
cmd_len = offsetof(struct wilc_spi_cmd, u.internal_w_cmd.crc);
- if (!spi_priv->crc_off)
+ if (spi_priv->crc7_enabled)
c->u.internal_w_cmd.crc[0] = wilc_get_crc7(wb, cmd_len);
} else if (cmd == CMD_SINGLE_WRITE) {
c->u.w_cmd.addr[0] = adr >> 16;
@@ -491,14 +521,14 @@ static int wilc_spi_write_cmd(struct wilc *wilc, u8 cmd, u32 adr, u32 data,
c->u.w_cmd.addr[2] = adr;
c->u.w_cmd.data = cpu_to_be32(data);
cmd_len = offsetof(struct wilc_spi_cmd, u.w_cmd.crc);
- if (!spi_priv->crc_off)
+ if (spi_priv->crc7_enabled)
c->u.w_cmd.crc[0] = wilc_get_crc7(wb, cmd_len);
} else {
dev_err(&spi->dev, "write cmd [%x] not supported\n", cmd);
return -EINVAL;
}
- if (!spi_priv->crc_off)
+ if (spi_priv->crc7_enabled)
cmd_len += 1;
resp_len = sizeof(*r);
@@ -536,6 +566,7 @@ static int wilc_spi_dma_rw(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz)
{
struct spi_device *spi = to_spi_device(wilc->dev);
struct wilc_spi *spi_priv = wilc->bus_data;
+ u16 crc_recv, crc_calc;
u8 wb[32], rb[32];
int cmd_len, resp_len;
int retry, ix = 0;
@@ -554,7 +585,7 @@ static int wilc_spi_dma_rw(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz)
c->u.dma_cmd.size[0] = sz >> 8;
c->u.dma_cmd.size[1] = sz;
cmd_len = offsetof(struct wilc_spi_cmd, u.dma_cmd.crc);
- if (!spi_priv->crc_off)
+ if (spi_priv->crc7_enabled)
c->u.dma_cmd.crc[0] = wilc_get_crc7(wb, cmd_len);
} else if (cmd == CMD_DMA_EXT_WRITE || cmd == CMD_DMA_EXT_READ) {
c->u.dma_cmd_ext.addr[0] = adr >> 16;
@@ -564,14 +595,14 @@ static int wilc_spi_dma_rw(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz)
c->u.dma_cmd_ext.size[1] = sz >> 8;
c->u.dma_cmd_ext.size[2] = sz;
cmd_len = offsetof(struct wilc_spi_cmd, u.dma_cmd_ext.crc);
- if (!spi_priv->crc_off)
+ if (spi_priv->crc7_enabled)
c->u.dma_cmd_ext.crc[0] = wilc_get_crc7(wb, cmd_len);
} else {
dev_err(&spi->dev, "dma read write cmd [%x] not supported\n",
cmd);
return -EINVAL;
}
- if (!spi_priv->crc_off)
+ if (spi_priv->crc7_enabled)
cmd_len += 1;
resp_len = sizeof(*r);
@@ -637,12 +668,35 @@ static int wilc_spi_dma_rw(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz)
}
/*
- * Read Crc
+ * Read CRC
*/
- if (!spi_priv->crc_off && wilc_spi_rx(wilc, crc, 2)) {
- dev_err(&spi->dev,
- "Failed block crc read, bus err\n");
- return -EINVAL;
+ if (spi_priv->crc16_enabled) {
+ if (wilc_spi_rx(wilc, crc, 2)) {
+ dev_err(&spi->dev,
+ "Failed block crc read, bus err\n");
+ return -EINVAL;
+ }
+ crc_recv = (crc[0] << 8) | crc[1];
+ crc_calc = crc_itu_t(0xffff, &b[ix], nbytes);
+ if (crc_recv != crc_calc) {
+ dev_err(&spi->dev, "%s: bad CRC 0x%04x "
+ "(calculated 0x%04x)\n", __func__,
+ crc_recv, crc_calc);
+
+ {
+ u8 byte;
+ int i;
+
+ for (i = 0; i < 16384; ++i) {
+ byte = 0;
+ wilc_spi_rx(wilc, &byte, 1);
+ if (!byte)
+ break;
+ }
+ }
+
+ return -EINVAL;
+ }
}
ix += nbytes;
@@ -871,43 +925,52 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
ret = spi_internal_read(wilc, WILC_SPI_PROTOCOL_OFFSET, ®);
if (ret) {
/*
- * Read failed. Try with CRC off. This might happen when module
- * is removed but chip isn't reset
+ * Read failed. Try with CRC7 off. This might happen
+ * when module is removed but chip isn't reset.
*/
- spi_priv->crc_off = 1;
+ spi_priv->crc7_enabled = false;
dev_err(&spi->dev,
- "Failed read with CRC on, retrying with CRC off\n");
+ "Failed read with CRC7 on, retrying with CRC7 off\n");
ret = spi_internal_read(wilc, WILC_SPI_PROTOCOL_OFFSET, ®);
if (ret) {
/*
- * Read failed with both CRC on and off,
+ * Read failed with both CRC7 on and off,
* something went bad
*/
dev_err(&spi->dev, "Failed internal read protocol\n");
return ret;
}
}
- if (spi_priv->crc_off == 0) {
- /* disable crc checking: */
- reg &= ~(PROTOCOL_REG_CRC7_MASK | PROTOCOL_REG_CRC16_MASK);
-
- /* set the data packet size: */
- BUILD_BUG_ON(DATA_PKT_LOG_SZ < DATA_PKT_LOG_SZ_MIN
- || DATA_PKT_LOG_SZ > DATA_PKT_LOG_SZ_MAX);
- reg &= ~PROTOCOL_REG_PKT_SZ_MASK;
- reg |= FIELD_PREP(PROTOCOL_REG_PKT_SZ_MASK,
- DATA_PKT_LOG_SZ - DATA_PKT_LOG_SZ_MIN);
-
- ret = spi_internal_write(wilc, WILC_SPI_PROTOCOL_OFFSET, reg);
- if (ret) {
- dev_err(&spi->dev,
- "[wilc spi %d]: Failed internal write reg\n",
- __LINE__);
- return ret;
- }
- spi_priv->crc_off = 1;
+
+ /* set up the desired CRC configuration: */
+ reg &= ~(PROTOCOL_REG_CRC7_MASK | PROTOCOL_REG_CRC16_MASK);
+#if ENABLE_CRC7
+ reg |= PROTOCOL_REG_CRC7_MASK;
+#endif
+#if ENABLE_CRC16
+ reg |= PROTOCOL_REG_CRC16_MASK;
+#endif
+
+ /* set up the data packet size: */
+ BUILD_BUG_ON(DATA_PKT_LOG_SZ < DATA_PKT_LOG_SZ_MIN
+ || DATA_PKT_LOG_SZ > DATA_PKT_LOG_SZ_MAX);
+ reg &= ~PROTOCOL_REG_PKT_SZ_MASK;
+ reg |= FIELD_PREP(PROTOCOL_REG_PKT_SZ_MASK,
+ DATA_PKT_LOG_SZ - DATA_PKT_LOG_SZ_MIN);
+
+ /* establish the new setup: */
+ ret = spi_internal_write(wilc, WILC_SPI_PROTOCOL_OFFSET, reg);
+ if (ret) {
+ dev_err(&spi->dev,
+ "[wilc spi %d]: Failed internal write reg\n",
+ __LINE__);
+ return ret;
}
+ /* now that new set up is established, update our state to match: */
+ spi_priv->crc7_enabled = ENABLE_CRC7;
+ spi_priv->crc16_enabled = ENABLE_CRC16;
+
/*
* make sure can read back chip id correctly
*/
--
2.25.1
next prev parent reply other threads:[~2021-02-24 5:53 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-24 5:51 [PATCH 1/4] wilc1000: Make SPI transfers work at 48MHz David Mosberger-Tang
2021-02-24 5:51 ` [PATCH 2/4] wilc1000: Introduce symbolic names for SPI protocol register David Mosberger-Tang
2021-02-24 5:51 ` [PATCH 3/4] wilc1000: Check for errors at end of DMA write David Mosberger-Tang
2021-02-25 8:27 ` Kalle Valo
2021-02-25 18:10 ` David Mosberger-Tang
2021-02-24 5:52 ` David Mosberger-Tang [this message]
2021-02-24 10:01 ` [PATCH 4/4] wilc1000: Add support for enabling CRC Ajay.Kathat
2021-02-24 16:25 ` David Mosberger-Tang
2021-02-25 4:50 ` Ajay.Kathat
2021-02-24 13:35 ` Ajay.Kathat
2021-02-24 15:47 ` David Mosberger-Tang
2021-02-25 4:58 ` Ajay.Kathat
2021-02-25 8:25 ` Kalle Valo
2021-02-24 21:19 ` Julian Calaby
2021-02-24 23:36 ` David Mosberger-Tang
2021-02-25 5:56 ` Ajay.Kathat
2021-02-25 8:22 ` Kalle Valo
2021-02-25 11:03 ` Ajay.Kathat
2021-02-26 7:09 ` Kalle Valo
2021-02-27 17:29 ` [PATCH v2 1/4] wilc1000: Make SPI transfers work at 48MHz David Mosberger-Tang
2021-04-17 17:48 ` Kalle Valo
2021-02-27 17:29 ` [PATCH v2 2/4] wilc1000: Introduce symbolic names for SPI protocol register David Mosberger-Tang
2021-02-27 17:29 ` [PATCH v2 3/4] wilc1000: Check for errors at end of DMA write David Mosberger-Tang
2021-02-27 17:31 ` [PATCH v2 4/4] wilc1000: Add support for enabling CRC David Mosberger-Tang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210224055135.1509200-4-davidm@egauge.net \
--to=davidm@egauge.net \
--cc=ajay.kathat@microchip.com \
--cc=claudiu.beznea@microchip.com \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).