On 17.04.2024 15:43:55, 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 overrite of LAT0/LAT1.
This change doesn't use single byte write instructions.
> Never write byte 2 of IOCON register to avoid clearing of LAT0/LAT1.
I discovered that erratum, it's described in
mcp251xfd_chip_rx_int_enable():
/* Configure GPIOs:
* - PIN0: GPIO Input
* - PIN1: GPIO Input/RX Interrupt
*
* PIN1 must be Input, otherwise there is a glitch on the
* rx-INT line. It happens between setting the PIN as output
* (in the first byte of the SPI transfer) and configuring the
* PIN as interrupt (in the last byte of the SPI transfer).
*/
The problem is that the SPI writes 1 byte at a time, starting at the
lower address. The chip updates the GPIO pin's status after each written
byte.
This may leads to a glitch if you have an external pull up. The power on
default auf the chip is GPIO/input, the GPIO line is not driven by the
chip and with the external pull up this will result in a high level.
If you configure the GPIO as an output/high, the driver first writes
bits 0...7, which results in the GPIO line being configured as an
output; the subsequent bits 8...15 configure the level of the GPIO
line.
This change doesn't take care of this.
I'm not sure, if it's better to have 2 dedicated writes to IOCON in the
driver or try to hide it here in the regmap.
> Signed-off-by: Gregor Herburger <gregor.herburger@xxxxxxxxxxxxxxx>
> ---
> drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c | 35 +++++++++++++++++++++++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
> index 92b7bc7f14b9..ab4e372baffb 100644
> --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
> +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
> @@ -229,14 +229,47 @@ 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, size_t count)
> +{
> + const size_t data_offset = sizeof(__be16) +
> + mcp251xfd_regmap_crc.pad_bits / BITS_PER_BYTE;
> + u16 reg = *(u16 *)data;
> +
> + /* 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
> + * 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 cleraing of LAT0/LAT1.
> + */
> +
> + /* Write IOCON[15:0] */
> + mcp251xfd_regmap_crc_gather_write(context, ®, 1,
> + data + data_offset, 2);
You write 15:0 in 1 go here.
> + reg += 3;
> + /* Write IOCON[31:24] */
> + mcp251xfd_regmap_crc_gather_write(context, ®, 1,
> + data + data_offset + 3, 1);
> +
> + 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, count);
> + else
> + return mcp251xfd_regmap_crc_gather_write(context,
> data, data_offset,
> data + data_offset,
> count - data_offset);
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 |
Attachment:
signature.asc
Description: PGP signature
