Re: [PATCH 4/4] wilc1000: Add support for enabling CRC

[David Mosberger-Tang <davidm@xxxxxxxxxx>]

On Wed, 2021-02-24 at 10:01 +0000, Ajay.Kathat@xxxxxxxxxxxxx wrote:

> This patch series also looks okay to me. I just have one input which is
> captured below.
> 
> We need to disable both crc7 and crc16 while retrying on failure attempt
> by adding below line
> 
> spi_priv->crc16_enabled = false;

Ah, you're right.  We can always probe with CRC16 off since the chip
returns valid register data regardless of whether crc16_enabled is
true or false.  I'm thinking something like this:

	spi_priv->crc16_enabled = false; // ignore CRC16 during CRC-
detection
	for (crc7 = 1; crc7 >= 0; --crc7) {
		spi_priv->crc7_enabled = crc7;
		ret = spi_internal_read(wilc, WILC_SPI_PROTOCOL_OFFSET,
&reg);
		if (ret == 0)
			break;
	}
	if (ret) {
		dev_err(&spi->dev, "Failed with all possible CRC
settings.\n");
		return ret;
	}

> By default the CRC checks are disabled, so if the kernel module is
> reloaded it should reattempt with both disabled.

Are you sure about that?  My test devices resets into:

 PROTOCOL_REG = 0x2e

which should be CRC7 and CRC16 on, right?

  --david