[PATCH] wifi: wilc1000: correct CRC7 calculation

[PATCH] wifi: wilc1000: correct CRC7 calculation

[David Mosberger-Tang]

Document

    ATWILC1000/ATWILC3000
    Baremetal Wi-Fi/BLE Link Controller Software Design Guide

    https://tinyurl.com/yer2xhyc

says that bit 0 of the CRC7 code must always be a 1.

I confirmed that today with a logic analyzer: setting bit 0 causes
wilc1000 to accept a command with CRC7 enabled, whereas clearing bit 0
causes wilc1000 to reject the command with a CRC error.

Signed-off-by: David Mosberger-Tang <davidm@egauge.net>
---
 drivers/net/wireless/microchip/wilc1000/spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c b/drivers/net/wireless/microchip/wilc1000/spi.c
index 7eb0f8a421a3..b9ed88d9c33d 100644
--- a/drivers/net/wireless/microchip/wilc1000/spi.c
+++ b/drivers/net/wireless/microchip/wilc1000/spi.c
@@ -472,7 +472,7 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
  ********************************************/
 static u8 wilc_get_crc7(u8 *buffer, u32 len)
 {
-	return crc7_be(0xfe, buffer, len);
+	return crc7_be(0xfe, buffer, len) | 0x01;
 }
 
 static int wilc_spi_single_read(struct wilc *wilc, u8 cmd, u32 adr, void *b,
-- 
2.34.1

Re: [PATCH] wifi: wilc1000: correct CRC7 calculation

[Ajay.Kathat]

Hi David,


On 2/6/24 22:07, David Mosberger-Tang wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Document
> 
>     ATWILC1000/ATWILC3000
>     Baremetal Wi-Fi/BLE Link Controller Software Design Guide
> 
>     https://tinyurl.com/yer2xhyc
> 
> says that bit 0 of the CRC7 code must always be a 1.
> 
> I confirmed that today with a logic analyzer: setting bit 0 causes
> wilc1000 to accept a command with CRC7 enabled, whereas clearing bit 0
> causes wilc1000 to reject the command with a CRC error.
> 

The change looks okay to me. Just curious, if the command CRC7 failure
is observed during the wifi operation or it was created by explicitly
clear bit0 in the code. Often I have tested with enable_crc7 enabled but
didn't observe into any command failure.


Regards,
Ajay

Re: [PATCH] wifi: wilc1000: correct CRC7 calculation

[David Mosberger-Tang]

Ajay,

On Thu, 2024-02-08 at 20:57 +0000, Ajay.Kathat@microchip.com wrote:
> Hi David,
> 
> 
> On 2/6/24 22:07, David Mosberger-Tang wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Document
> > 
> >     ATWILC1000/ATWILC3000
> >     Baremetal Wi-Fi/BLE Link Controller Software Design Guide
> > 
> >     https://tinyurl.com/yer2xhyc
> > 
> > says that bit 0 of the CRC7 code must always be a 1.
> > 
> > I confirmed that today with a logic analyzer: setting bit 0 causes
> > wilc1000 to accept a command with CRC7 enabled, whereas clearing bit 0
> > causes wilc1000 to reject the command with a CRC error.
> > 
> 
> The change looks okay to me. Just curious, if the command CRC7 failure
> is observed during the wifi operation or it was created by explicitly
> clear bit0 in the code. Often I have tested with enable_crc7 enabled but
> didn't observe into any command failure.

Yeah, the module doesn't seem to always reject CRC7 sums with bit 0
cleared - otherwise I think we'd have noticed this error sooner.  I'm
not sure what causes the WILC1000 to either ignore or pay attention to
bit 0, but I have definitely traces captured where CRC7s with bit 0
cleared resulted in status 0x03 (bad CRC) whereas it always seems to
work with bit 0 set.  Since the documentation also says that it should
b set, it's probably safer to go with that.

  --david

Re: [PATCH] wifi: wilc1000: correct CRC7 calculation

[Kalle Valo]

David Mosberger-Tang <davidm@egauge.net> wrote:

> Document
> 
>     ATWILC1000/ATWILC3000
>     Baremetal Wi-Fi/BLE Link Controller Software Design Guide
> 
>     https://tinyurl.com/yer2xhyc
> 
> says that bit 0 of the CRC7 code must always be a 1.
> 
> I confirmed that today with a logic analyzer: setting bit 0 causes
> wilc1000 to accept a command with CRC7 enabled, whereas clearing bit 0
> causes wilc1000 to reject the command with a CRC error.
> 
> Signed-off-by: David Mosberger-Tang <davidm@egauge.net>

Patch applied to wireless-next.git, thanks.

c08a986344a5 wifi: wilc1000: correct CRC7 calculation

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20240207050736.2717641-1-davidm@egauge.net/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches