[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240506-aromatic-dainty-orangutan-f8b57c-mkl@pengutronix.de>
Date: Mon, 6 May 2024 11:18:55 +0200
From: Marc Kleine-Budde <mkl@...gutronix.de>
To: Gregor Herburger <gregor.herburger@...tq-group.com>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
Thomas Kopp <thomas.kopp@...rochip.com>, Vincent Mailhol <mailhol.vincent@...adoo.fr>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, linux-can@...r.kernel.org,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
linux@...tq-group.com
Subject: Re: [PATCH v2 4/6] can: mcp251xfd: mcp251xfd_regmap_crc_write():
workaround for errata 5
On 06.05.2024 07:59:46, Gregor Herburger wrote:
> According to Errata DS80000789E 5 writing IOCON register using one SPI
> write command clears LAT0/LAT1.
>
> Errata Fix/Work Around suggests to write registers with single byte write
> instructions. However, it seems that every write to the second byte
> causes the overwrite of LAT0/LAT1.
>
> Never write byte 2 of IOCON register to avoid clearing of LAT0/LAT1.
>
> Signed-off-by: Gregor Herburger <gregor.herburger@...tq-group.com>
> ---
> drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c | 29 +++++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
> index 65150e762007..43fcf7f50591 100644
> --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
> +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
> @@ -229,14 +229,41 @@ mcp251xfd_regmap_crc_gather_write(void *context,
> return spi_sync_transfer(spi, xfer, ARRAY_SIZE(xfer));
> }
>
> +static int mcp251xfd_regmap_crc_write_iocon(void *context, const void *data)
> +{
> + u16 reg = MCP251XFD_REG_IOCON;
const
> +
> + /* Never write to bits 16..23 of IOCON register to avoid clearing of LAT0/LAT1
> + *
> + * According to Errata DS80000789E 5 writing IOCON register using one
Just for completeness add "mcp2518fd" in front of Errata.
> + * SPI write command clears LAT0/LAT1.
> + *
> + * Errata Fix/Work Around suggests to write registers with single byte
> + * write instructions. However, it seems that the byte at 0xe06(IOCON[23:16])
> + * is for read-only access and writing to it causes the clearing of LAT0/LAT1.
> + */
> +
> + /* Write IOCON[15:0] */
> + mcp251xfd_regmap_crc_gather_write(context, ®, 1, data, 2);
> + reg += 3;
> + /* Write IOCON[31:24] */
> + mcp251xfd_regmap_crc_gather_write(context, ®, 1, data + 3, 1);
Please add error handling.
> +
> + return 0;
> +}
> +
> static int
> mcp251xfd_regmap_crc_write(void *context,
> const void *data, size_t count)
> {
> const size_t data_offset = sizeof(__be16) +
> mcp251xfd_regmap_crc.pad_bits / BITS_PER_BYTE;
> + u16 reg = *(u16 *)data;
>
> - return mcp251xfd_regmap_crc_gather_write(context,
> + if (reg == MCP251XFD_REG_IOCON)
> + return mcp251xfd_regmap_crc_write_iocon(context, data + data_offset);
Please also check that "count" is sizeof(__le32).
> + else
> + return mcp251xfd_regmap_crc_gather_write(context,
> data, data_offset,
> data + data_offset,
> count - data_offset);
>
Also add the workaround for the nocrc regmap.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists