| Message ID | 20240506-mcp251xfd-gpio-feature-v2-4-615b16fa8789@ew.tq-group.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | can: mcp251xfd: add gpio functionality | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Series ignored based on subject, async |
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@ew.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
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; + + /* 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 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); + + 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); + else + return mcp251xfd_regmap_crc_gather_write(context, data, data_offset, data + data_offset, count - data_offset);
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@ew.tq-group.com> --- drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c | 29 +++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-)