[PATCH] can: mcp25xxfd: Add explanation for SPI clk limit, Reference to ECC errata

[PATCH] can: mcp25xxfd: Add explanation for SPI clk limit, Reference to ECC errata

[Thomas Kopp]

Reference the MCP2517FD and MCP2518FD erratasheets and paste the
explanation.
The single error correction does not always work, so just indicate that
a single error occurred.

Signed-off-by: Thomas Kopp <thomas.kopp@microchip.com>
---
 .../net/can/spi/mcp25xxfd/mcp25xxfd-core.c    | 28 ++++++++++++++++---
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c
index 96901fb47bde..5557b2d1e774 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c
@@ -1943,6 +1943,17 @@ mcp25xxfd_handle_eccif_recover(struct mcp25xxfd_priv *priv, u8 nr)
 	return mcp25xxfd_chip_set_normal_mode(priv);
 }
 
+	/* Errata Reference:  DS80000789B, DS80000792C
+	 * ECC single error correction does not work in
+	 * all cases:
+	 * Fix/Work Around:
+	 * Enable single error correction and double error
+	 * detection interrupts by setting SECIE and
+	 * DEDIE. Handle SECIF as a detection interrupt
+	 * and do not rely on the error correction. Instead,
+	 * handle both interrupts as a notification that the
+	 * RAM word at ERRADDR was corrupted.
+	 */
 static int
 mcp25xxfd_handle_eccif(struct mcp25xxfd_priv *priv, bool set_normal_mode)
 {
@@ -1974,7 +1985,7 @@ mcp25xxfd_handle_eccif(struct mcp25xxfd_priv *priv, bool set_normal_mode)
 		return err;
 
 	if (ecc_stat & MCP25XXFD_REG_ECCSTAT_SECIF)
-		msg = "Single ECC Error corrected at address";
+		msg = "Single ECC Error detected at address";
 	else if (ecc_stat & MCP25XXFD_REG_ECCSTAT_DEDIF)
 		msg = "Double ECC Error detected at address";
 	else
@@ -2815,8 +2826,18 @@ static int mcp25xxfd_probe(struct spi_device *spi)
 	/* According to the datasheet the SPI clock must be less or
 	 * equal SYSCLOCK / 2.
 	 *
-	 * It turns out, that the Controller is not stable at this
-	 * rate. Known good and bad combinations are:
+	 * Errata Reference:  DS80000789B, DS80000792C
+	 * The SPI can write corrupted data to the RAM
+	 * at fast SPI speeds:
+	 * Simultaneous activity on the CAN bus while
+	 * writing data to RAM via the SPI interface, with
+	 * high SCK frequency, can lead to corrupted data
+	 * being written to RAM.
+	 * Fix/Work Around:
+	 * Ensure that FSCK is less than or equal to
+	 * 0.85 * (FSYSCLK/2).
+	 *
+	 * Known good and bad combinations are:
 	 *
 	 * MCP	ext-clk	SoC			SPI			SPI-clk		max-clk	parent-clk	Status	config
 	 *
@@ -2829,7 +2850,6 @@ static int mcp25xxfd_probe(struct spi_device *spi)
 	 * 2517 40 MHz	atmel,sama5d27		atmel,at91rm9200-spi	16400000 Hz	 82.00%	 82000000 Hz	good	default
 	 * 2518 40 MHz	atmel,sama5d27		atmel,at91rm9200-spi	16400000 Hz	 82.00%	 82000000 Hz	good	default
 	 *
-	 * Limit SPI clock to 85% of SYSCLOCK / 2 for now.
 	 */
 	priv->spi_max_speed_hz_orig = spi->max_speed_hz;
 	spi->max_speed_hz = min(spi->max_speed_hz, freq / 2 / 1000 * 850);
-- 
2.25.1

Re: [PATCH] can: mcp25xxfd: Add explanation for SPI clk limit, Reference to ECC errata

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1348 bytes --]

On 9/25/20 8:56 AM, Thomas Kopp wrote:
> Reference the MCP2517FD and MCP2518FD erratasheets and paste the
> explanation.
> The single error correction does not always work, so just indicate that
> a single error occurred.
> 
> Signed-off-by: Thomas Kopp <thomas.kopp@microchip.com>
> ---
>  .../net/can/spi/mcp25xxfd/mcp25xxfd-core.c    | 28 ++++++++++++++++---
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c
> index 96901fb47bde..5557b2d1e774 100644
> --- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c
> +++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c
> @@ -1943,6 +1943,17 @@ mcp25xxfd_handle_eccif_recover(struct mcp25xxfd_priv *priv, u8 nr)
>  	return mcp25xxfd_chip_set_normal_mode(priv);
>  }
>  
> +	/* Errata Reference:  DS80000789B, DS80000792C

The "DS80000792C" is the new erratum for the mcp2517fd? Is it already online,
the https://www.microchip.com/wwwproducts/en/MCP2517FD still lists the "B" variant.

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 |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH] can: mcp25xxfd: Add explanation for SPI clk limit, Reference to ECC errata

[Thomas.Kopp]

> The "DS80000792C" is the new erratum for the mcp2517fd? Is it already
> online,
> the https://www.microchip.com/wwwproducts/en/MCP2517FD still lists
> the "B" variant.

Correct, that's the new revision. There was a delay in the signoff process. 
It should go online tomorrow.

Thomas