Re: [PATCH net-next 1/2] net: dsa: mxl862xx: add CRC for MDIO communication

[Andrew Lunn]

> +/* Firmware CRC error codes (outside normal Zephyr errno range). */
> +#define MXL862XX_FW_CRC6_ERR		(-1024)
> +#define MXL862XX_FW_CRC16_ERR		(-1023)
> +
> +/* 3GPP CRC-6 lookup table (polynomial 0x6F).
> + * Matches the firmware's default CRC-6 implementation.
> + */
> +static const u8 mxl862xx_crc6_table[256] = {
> +	0x00, 0x2f, 0x31, 0x1e, 0x0d, 0x22, 0x3c, 0x13,
> +	0x1a, 0x35, 0x2b, 0x04, 0x17, 0x38, 0x26, 0x09,

I had a quick look around and count not find this table anywhere
else. Mellanox has a different CRC-6 polynomial. Until somebody else
needs this, i think it is O.K. here. But sometime in the future it
might be moved into lib/crc.

> +	ret = mxl862xx_crc6_verify(ctrl_enc, len_enc, &fw_result);
> +	if (ret < 0) {
> +		dev_err(&priv->mdiodev->dev,
> +			"CRC-6 error on response to CMD %04x\n", cmd);
> +		return -EIO;
> +	}

This is the question, what to do when you see a checksum failure? The
basic assumption is MDIO is reliable. PHYs don't do any sort of
checksums, nor any other switches using MDIO. Why would you want
checksums?

To detect the hardware is broken? If so, is returning EIO sufficient?
Would it not be better to admin down all the interfaces?

To allow the MDIO clock to run at a higher frequency, at the limit of
the bus, so you get occasionally failures? If so, should you not
retry? But are the commands idempotent? Can you safely retry?

	Andrew