Re: [net-next 6/6] can: mcp251xfd: mcp251xfd_regmap_crc_read(): work around broken CRC on TBC register

From: "Modilaynen, Pavel" <pavel.modilaynen@volvocars.com>
To: "mkl@pengutronix.de" <mkl@pengutronix.de>
Cc: "drew@beagleboard.org" <drew@beagleboard.org>,
	"linux-can@vger.kernel.org" <linux-can@vger.kernel.org>,
	"menschel.p@posteo.de" <menschel.p@posteo.de>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"will@macchina.cc" <will@macchina.cc>
Subject: Re: [net-next 6/6] can: mcp251xfd: mcp251xfd_regmap_crc_read(): work around broken CRC on TBC register
Date: Tue, 7 Dec 2021 16:53:12 +0000	[thread overview]
Message-ID: <PR3P174MB0112D073D0E5E080FAAE8510846E9@PR3P174MB0112.EURP174.PROD.OUTLOOK.COM> (raw)

Hello Marc,

We observe the very similar issue with MCP2517FD.

>The CRC errors the patch works around are CRC errors introduced by a
>chip erratum, not by electromagnetic interference. In my observation

Are you referring this errata doc
https://datasheet.octopart.com/MCP2517FD-H-JHA-Microchip-datasheet-136609045.pdf ?

We have the similar CRC read errors but
the lowest byte is not 0x00 and 0x80, it's actually 0x0x or 0x8x, e.g.

mcp251xfd spi0.0 can0: CRC read error at address 0x0010 (length=4, data=82 d1 fa 6c, CRC=0xd9c2) retrying.

0xb0 0x10 0x04 0x82 0xd1 0xfa 0x6c => 0x59FD (not matching)

but if I flip the first received bit  (highest bit in the lowest byte):
0xb0 0x10 0x04 0x02 0xd1 0xfa 0x6c => 0xD9C2 (matching!)

So, your fix covers only the case of 0x00 and 0x80, 
do you think that the workaround should be extended so check
     (buf_rx->data[0] == 0x0 || buf_rx->data[0] == 0x80)) {
turns into 
     ((buf_rx->data[0] & 0xf0) == 0x0 || (buf_rx->data[0] & 0xf0) == 0x80)) {

Errata, actually says
"Only bits 7/15/23/31 of the following registers can be affected:"

So, we could basically, in simplest case flip bit 31 and re-check CRC without any check of 
rx->data[0]....

Regards,
Pavel