linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] can: mcp251xfd: optimizing transfer size for CRC transfers size 1
@ 2023-01-27 12:42 Thomas Kopp
  2023-02-02 14:10 ` Marc Kleine-Budde
  0 siblings, 1 reply; 2+ messages in thread
From: Thomas Kopp @ 2023-01-27 12:42 UTC (permalink / raw)
  To: linux-can, mkl; +Cc: mani, thomas.kopp

For CRC transfers with size 1 it is more efficient to use the write_safe command instead of the write_crc command. This saves the length byte on the SPI transfer.

Signed-off-by: Thomas Kopp <thomas.kopp@microchip.com>
---
 .../net/can/spi/mcp251xfd/mcp251xfd-ring.c    | 31 ++++++++++++-------
 drivers/net/can/spi/mcp251xfd/mcp251xfd.h     | 26 +++++++++++++---
 2 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c
index f69d5fc8c9afd..3c3bc9be1f295 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c
@@ -30,22 +30,31 @@ mcp251xfd_cmd_prepare_write_reg(const struct mcp251xfd_priv *priv,
 	last_byte = mcp251xfd_last_byte_set(mask);
 	len = last_byte - first_byte + 1;
 
-	data = mcp251xfd_spi_cmd_write(priv, write_reg_buf, reg + first_byte);
+	data = mcp251xfd_spi_cmd_write(priv, write_reg_buf, reg + first_byte, len);
 	val_le32 = cpu_to_le32(val >> BITS_PER_BYTE * first_byte);
 	memcpy(data, &val_le32, len);
 
 	if (priv->devtype_data.quirks & MCP251XFD_QUIRK_CRC_REG) {
 		u16 crc;
-
-		mcp251xfd_spi_cmd_crc_set_len_in_reg(&write_reg_buf->crc.cmd,
-						     len);
-		/* CRC */
-		len += sizeof(write_reg_buf->crc.cmd);
-		crc = mcp251xfd_crc16_compute(&write_reg_buf->crc, len);
-		put_unaligned_be16(crc, (void *)write_reg_buf + len);
-
-		/* Total length */
-		len += sizeof(write_reg_buf->crc.crc);
+		if (len == 1) {
+			/* CRC */
+			len += sizeof(write_reg_buf->safe.cmd);
+			crc = mcp251xfd_crc16_compute(&write_reg_buf->safe, len);
+			put_unaligned_be16(crc, (void *)write_reg_buf + len);
+
+			/* Total length */
+			len += sizeof(write_reg_buf->safe.crc);
+		} else {
+			mcp251xfd_spi_cmd_crc_set_len_in_reg(&write_reg_buf->crc.cmd,
+							     len);
+			/* CRC */
+			len += sizeof(write_reg_buf->crc.cmd);
+			crc = mcp251xfd_crc16_compute(&write_reg_buf->crc, len);
+			put_unaligned_be16(crc, (void *)write_reg_buf + len);
+
+			/* Total length */
+			len += sizeof(write_reg_buf->crc.crc);
+		}
 	} else {
 		len += sizeof(write_reg_buf->nocrc.cmd);
 	}
diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
index 6008d38810e98..5d82eb2676182 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
@@ -504,6 +504,11 @@ union mcp251xfd_write_reg_buf {
 		u8 data[4];
 		__be16 crc;
 	} crc;
+	struct __packed {
+		struct mcp251xfd_buf_cmd cmd;
+		u8 data[1];
+		__be16 crc;
+	} safe;
 } ____cacheline_aligned;
 
 struct mcp251xfd_tx_obj {
@@ -762,6 +767,13 @@ mcp251xfd_spi_cmd_write_crc_set_addr(struct mcp251xfd_buf_cmd_crc *cmd,
 	cmd->cmd = cpu_to_be16(MCP251XFD_SPI_INSTRUCTION_WRITE_CRC | addr);
 }
 
+static inline void
+mcp251xfd_spi_cmd_write_safe_set_addr(struct mcp251xfd_buf_cmd *cmd,
+				     u16 addr)
+{
+	cmd->cmd = cpu_to_be16(MCP251XFD_SPI_INSTRUCTION_WRITE_CRC_SAFE | addr);
+}
+
 static inline void
 mcp251xfd_spi_cmd_write_crc(struct mcp251xfd_buf_cmd_crc *cmd,
 			    u16 addr, u16 len)
@@ -773,14 +785,20 @@ mcp251xfd_spi_cmd_write_crc(struct mcp251xfd_buf_cmd_crc *cmd,
 static inline u8 *
 mcp251xfd_spi_cmd_write(const struct mcp251xfd_priv *priv,
 			union mcp251xfd_write_reg_buf *write_reg_buf,
-			u16 addr)
+			u16 addr, u8 len)
 {
 	u8 *data;
 
 	if (priv->devtype_data.quirks & MCP251XFD_QUIRK_CRC_REG) {
-		mcp251xfd_spi_cmd_write_crc_set_addr(&write_reg_buf->crc.cmd,
-						     addr);
-		data = write_reg_buf->crc.data;
+		if (len == 1) {
+			mcp251xfd_spi_cmd_write_safe_set_addr(&write_reg_buf->safe.cmd,
+							     addr);
+			data = write_reg_buf->safe.data;
+		} else {
+			mcp251xfd_spi_cmd_write_crc_set_addr(&write_reg_buf->crc.cmd,
+							     addr);
+			data = write_reg_buf->crc.data;
+		}
 	} else {
 		mcp251xfd_spi_cmd_write_nocrc(&write_reg_buf->nocrc.cmd,
 					      addr);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [PATCH] can: mcp251xfd: optimizing transfer size for CRC transfers size 1
  2023-01-27 12:42 [PATCH] can: mcp251xfd: optimizing transfer size for CRC transfers size 1 Thomas Kopp
@ 2023-02-02 14:10 ` Marc Kleine-Budde
  0 siblings, 0 replies; 2+ messages in thread
From: Marc Kleine-Budde @ 2023-02-02 14:10 UTC (permalink / raw)
  To: Thomas Kopp; +Cc: linux-can, mani

[-- Attachment #1: Type: text/plain, Size: 2778 bytes --]

On 27.01.2023 13:42:58, Thomas Kopp wrote:
> For CRC transfers with size 1 it is more efficient to use the
> write_safe command instead of the write_crc command. This saves the
> length byte on the SPI transfer.

Looks good to me. But I cannot measure a difference.

> Signed-off-by: Thomas Kopp <thomas.kopp@microchip.com>
> ---
>  .../net/can/spi/mcp251xfd/mcp251xfd-ring.c    | 31 ++++++++++++-------
>  drivers/net/can/spi/mcp251xfd/mcp251xfd.h     | 26 +++++++++++++---
>  2 files changed, 42 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c
> index f69d5fc8c9afd..3c3bc9be1f295 100644
> --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c
> +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c
> @@ -30,22 +30,31 @@ mcp251xfd_cmd_prepare_write_reg(const struct mcp251xfd_priv *priv,
>  	last_byte = mcp251xfd_last_byte_set(mask);
>  	len = last_byte - first_byte + 1;
>  
> -	data = mcp251xfd_spi_cmd_write(priv, write_reg_buf, reg + first_byte);
> +	data = mcp251xfd_spi_cmd_write(priv, write_reg_buf, reg + first_byte, len);
>  	val_le32 = cpu_to_le32(val >> BITS_PER_BYTE * first_byte);
>  	memcpy(data, &val_le32, len);
>  
>  	if (priv->devtype_data.quirks & MCP251XFD_QUIRK_CRC_REG) {
>  		u16 crc;
> -
> -		mcp251xfd_spi_cmd_crc_set_len_in_reg(&write_reg_buf->crc.cmd,
> -						     len);
> -		/* CRC */
> -		len += sizeof(write_reg_buf->crc.cmd);
> -		crc = mcp251xfd_crc16_compute(&write_reg_buf->crc, len);
> -		put_unaligned_be16(crc, (void *)write_reg_buf + len);
> -
> -		/* Total length */
> -		len += sizeof(write_reg_buf->crc.crc);
> +		if (len == 1) {
> +			/* CRC */
> +			len += sizeof(write_reg_buf->safe.cmd);
> +			crc = mcp251xfd_crc16_compute(&write_reg_buf->safe, len);
> +			put_unaligned_be16(crc, (void *)write_reg_buf + len);
> +
> +			/* Total length */
> +			len += sizeof(write_reg_buf->safe.crc);
> +		} else {
> +			mcp251xfd_spi_cmd_crc_set_len_in_reg(&write_reg_buf->crc.cmd,
> +							     len);
> +			/* CRC */
> +			len += sizeof(write_reg_buf->crc.cmd);
> +			crc = mcp251xfd_crc16_compute(&write_reg_buf->crc, len);
> +			put_unaligned_be16(crc, (void *)write_reg_buf + len);
> +
> +			/* Total length */
> +			len += sizeof(write_reg_buf->crc.crc);
> +		}
>  	} else {
>  		len += sizeof(write_reg_buf->nocrc.cmd);
>  	}

I've moved changed the if() logic a bit, saving 1 level of indention.

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: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-02-02 14:10 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-27 12:42 [PATCH] can: mcp251xfd: optimizing transfer size for CRC transfers size 1 Thomas Kopp
2023-02-02 14:10 ` Marc Kleine-Budde

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).