2024-02-07 05:07:51

by David Mosberger-Tang

[permalink] [raw]
Subject: [PATCH] wifi: wilc1000: correct CRC7 calculation

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 <[email protected]>
---
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



2024-02-08 20:57:39

by Ajay Singh

[permalink] [raw]
Subject: Re: [PATCH] wifi: wilc1000: correct CRC7 calculation

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

2024-02-09 00:37:37

by David Mosberger-Tang

[permalink] [raw]
Subject: Re: [PATCH] wifi: wilc1000: correct CRC7 calculation

Ajay,

On Thu, 2024-02-08 at 20:57 +0000, [email protected] 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


2024-02-12 15:46:36

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] wifi: wilc1000: correct CRC7 calculation

David Mosberger-Tang <[email protected]> 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 <[email protected]>

Patch applied to wireless-next.git, thanks.

c08a986344a5 wifi: wilc1000: correct CRC7 calculation

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

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