On 07.12.2021 16:53:12, Modilaynen, Pavel wrote:

Hello Marc,

We observe the very similar issue with MCP2517FD.

quoted

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 ?

ACK - although there are newer versions available. The newest erratum
for the mcp2517fd is the "C" issue:

| https://ww1.microchip.com/downloads/en/DeviceDoc/MCP2517FD-Silicon-Errata-and-Data-Sheet-Clarification-DS80000792C.pdf

The mpc2518fd can be downloaded here:

| https://ww1.microchip.com/downloads/en/DeviceDoc/MCP2518FD-Silicon-Errata-and-Data-Sheet-Clarification-DS80000789C.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]....

Can you send a patch that updates the workaround and description.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |