On 24/02/21 9:55 pm, David Mosberger-Tang wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > 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) { Its better to first try with crc7 disable and then with enable because our default configuration has both disabled so it will return success immediatly on reattempt for default configuration. for (crc7 = 0; crc7 <= 1; ++crc7) { > spi_priv->crc7_enabled = crc7; > ret = spi_internal_read(wilc, WILC_SPI_PROTOCOL_OFFSET, > ®); > 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? The workaround was added for scenario when kernel module is loaded without CRC enabled in WILC1000 chip. The scenario was observed during the kernel module unload(rmmod) then reload(insmod/modprobe). Mostly the driver will read with CRC ON but if it fails then read attempt should be tried with CRC OFF. Regards, Ajay