Picking up this old thread....

On 21.12.2021 22:24:52, Thomas.Kopp@microchip.com wrote:
> Thanks for the data. I've looked into this and it seems that the
> second bit being set in your case does not depend on the SPI-Rate (or
> the quirks for that matter) but it seems to be hardware setup related.
> 
> I'm fine with changing the driver so that it ignores set LSBs but
> would limit it to 2 or 3 bits:

> (buf_rx->data[0] == 0x0 || buf_rx->data[0] == 0x80))
> becomes
> ((buf_rx->data[0] & 0xf8) == 0x0 || (buf_rx->data[0] & 0xf8) == 0x80)) {
> 
> The action also needs to be changed and the flip back of the bit needs
> to be removed. In this case the flipped databit that produces a
> matching CRC is actually  correct (i.e. consistent with the 7 LSBs in
> that byte.)
> 
> A patch could look like this (I'm currently not close to a setup where
> I can compile/test this.)

Thomas, can I have your Signed-off-by for this patch?

Marc

> diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
> index 297491516a26..e5bc897f37e8 100644
> --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
> +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
> @@ -332,12 +332,10 @@ mcp251xfd_regmap_crc_read(void *context,
>                  *
>                  * If the highest bit in the lowest byte is flipped
>                  * the transferred CRC matches the calculated one. We
> -                * assume for now the CRC calculation in the chip
> -                * works on wrong data and the transferred data is
> -                * correct.
> +                * assume for now the CRC operates on the correct data.
>                  */
>                 if (reg == MCP251XFD_REG_TBC &&
> -                   (buf_rx->data[0] == 0x0 || buf_rx->data[0] == 0x80)) {
> +                   ((buf_rx->data[0] & 0xF8) == 0x0 || (buf_rx->data[0] & 0xF8) == 0x80)) {
>                         /* Flip highest bit in lowest byte of le32 */
>                         buf_rx->data[0] ^= 0x80;
> 
> @@ -347,10 +345,8 @@ mcp251xfd_regmap_crc_read(void *context,
>                                                                   val_len);
>                         if (!err) {
>                                 /* If CRC is now correct, assume
> -                                * transferred data was OK, flip bit
> -                                * back to original value.
> +                                * flipped data was OK.
>                                  */
> -                               buf_rx->data[0] ^= 0x80;
>                                 goto out;
>                         }
>                 }
> 
> Thanks,
> Thomas
> 

-- 
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 |