Received-SPF: pass (google.com: domain of linux-wireless+bounces-3365-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <0eba8b05e38c5767caf291a77bd88f5306e8a89d.camel@egauge.net>
Subject: Re: [PATCH] wifi: wilc1000: correct CRC7 calculation
From: David Mosberger-Tang <davidm@egauge.net>
To: Ajay.Kathat@microchip.com, linux-wireless@vger.kernel.org
Cc: alexis.lothore@bootlin.com, kvalo@kernel.org
Date: Thu, 08 Feb 2024 17:37:26 -0700
In-Reply-To: <563535fa-964c-480d-8e6d-1678de5e057c@microchip.com>
References: <20240207050736.2717641-1-davidm@egauge.net>
	 <563535fa-964c-480d-8e6d-1678de5e057c@microchip.com>
Organization: eGauge Systems LLC
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu2 
Precedence: bulk
MIME-Version: 1.0

Ajay,

On Thu, 2024-02-08 at 20:57 +0000, Ajay.Kathat@microchip.com wrote:
> Hi David,
>=20
>=20
> 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
> >=20
> > Document
> >=20
> >     ATWILC1000/ATWILC3000
> >     Baremetal Wi-Fi/BLE Link Controller Software Design Guide
> >=20
> >     https://tinyurl.com/yer2xhyc
> >=20
> > says that bit 0 of the CRC7 code must always be a 1.
> >=20
> > 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.
> >=20
>=20
> 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