diff mbox series

[v2,net-next,2/2] net: dsa: sja1105: adapt to a SPI controller with a limited max transfer size

Message ID 20210520200223.3375421-3-olteanv@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Adapt the sja1105 DSA driver to the SPI controller's transfer limits | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: line length of 82 exceeds 80 columns
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Vladimir Oltean May 20, 2021, 8:02 p.m. UTC 
From: Vladimir Oltean <vladimir.oltean@nxp.com>

The static config of the sja1105 switch is a long stream of bytes which
is programmed to the hardware in chunks (portions with the chip select
continuously asserted) of max 256 bytes each.

Only that certain SPI controllers, such as the spi-sc18is602 I2C-to-SPI
bridge, cannot keep the chip select asserted for that long.
The spi_max_transfer_size() and spi_max_message_size() functions are how
the controller can impose its hardware limitations upon the SPI
peripheral driver.

The sja1105 sends its static config to the SPI master in chunks, and
each chunk is a spi_message composed of 2 spi_transfers: the buffer with
the data and a preceding buffer with the SPI access header. Both buffers
must be smaller than the transfer limit, and their sum must be smaller
than the message limit.

Regression-tested on a switch connected to a controller with no
limitations (spi-fsl-dspi) as well as with one with caps for both
max_transfer_size and max_message_size (spi-sc18is602).

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_spi.c | 30 ++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

Comments

Vladimir Oltean May 20, 2021, 8:35 p.m. UTC | #1 
On Thu, May 20, 2021 at 11:02:23PM +0300, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The static config of the sja1105 switch is a long stream of bytes which
> is programmed to the hardware in chunks (portions with the chip select
> continuously asserted) of max 256 bytes each.
> 
> Only that certain SPI controllers, such as the spi-sc18is602 I2C-to-SPI
> bridge, cannot keep the chip select asserted for that long.
> The spi_max_transfer_size() and spi_max_message_size() functions are how
> the controller can impose its hardware limitations upon the SPI
> peripheral driver.
> 
> The sja1105 sends its static config to the SPI master in chunks, and
> each chunk is a spi_message composed of 2 spi_transfers: the buffer with
> the data and a preceding buffer with the SPI access header. Both buffers
> must be smaller than the transfer limit, and their sum must be smaller
> than the message limit.
> 
> Regression-tested on a switch connected to a controller with no
> limitations (spi-fsl-dspi) as well as with one with caps for both
> max_transfer_size and max_message_size (spi-sc18is602).
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/dsa/sja1105/sja1105_spi.c | 30 ++++++++++++++++++++-------
>  1 file changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
> index 8746e3f158a0..7bcf2e419037 100644
> --- a/drivers/net/dsa/sja1105/sja1105_spi.c
> +++ b/drivers/net/dsa/sja1105/sja1105_spi.c
> @@ -40,19 +40,35 @@ static int sja1105_xfer(const struct sja1105_private *priv,
>  			size_t len, struct ptp_system_timestamp *ptp_sts)
>  {
>  	u8 hdr_buf[SJA1105_SIZE_SPI_MSG_HEADER] = {0};
> -	struct sja1105_chunk chunk = {
> -		.len = min_t(size_t, len, SJA1105_SIZE_SPI_MSG_MAXLEN),
> -		.reg_addr = reg_addr,
> -		.buf = buf,
> -	};
>  	struct spi_device *spi = priv->spidev;
>  	struct spi_transfer xfers[2] = {0};
>  	struct spi_transfer *chunk_xfer;
>  	struct spi_transfer *hdr_xfer;
> +	struct sja1105_chunk chunk;
> +	ssize_t xfer_len;
>  	int num_chunks;
>  	int rc, i = 0;
>  
> -	num_chunks = DIV_ROUND_UP(len, SJA1105_SIZE_SPI_MSG_MAXLEN);
> +	/* One spi_message is composed of two spi_transfers: a small one for
> +	 * the message header and another one for the current chunk of the
> +	 * packed buffer.
> +	 * Check that the restrictions imposed by the SPI controller are
> +	 * respected: the chunk buffer is smaller than the max transfer size,
> +	 * and the total length of the chunk plus its message header is smaller
> +	 * than the max message size.
> +	 */
> +	xfer_len = min_t(ssize_t, SJA1105_SIZE_SPI_MSG_MAXLEN,
> +			 spi_max_transfer_size(spi));
> +	xfer_len = min_t(ssize_t, SJA1105_SIZE_SPI_MSG_MAXLEN,
> +			 spi_max_message_size(spi) - SJA1105_SIZE_SPI_MSG_HEADER);
> +	if (xfer_len < 0)
> +		return -ERANGE;

I've introduced a bug here when spi_max_message_size returns SIZE_MAX
which is of the unsigned size_t type. Converted to ssize_t it's negative,
so it triggers the negative check...

Please wait until I send a v3 with this fixed. Thanks.

> +
> +	num_chunks = DIV_ROUND_UP(len, xfer_len);
> +
> +	chunk.reg_addr = reg_addr;
> +	chunk.buf = buf;
> +	chunk.len = min_t(size_t, len, xfer_len);
>  
>  	hdr_xfer = &xfers[0];
>  	chunk_xfer = &xfers[1];
> @@ -104,7 +120,7 @@ static int sja1105_xfer(const struct sja1105_private *priv,
>  		chunk.buf += chunk.len;
>  		chunk.reg_addr += chunk.len / 4;
>  		chunk.len = min_t(size_t, (ptrdiff_t)(buf + len - chunk.buf),
> -				  SJA1105_SIZE_SPI_MSG_MAXLEN);
> +				  xfer_len);
>  
>  		rc = spi_sync_transfer(spi, xfers, 2);
>  		if (rc < 0) {
> -- 
> 2.25.1
>
diff mbox series

Patch

diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
index 8746e3f158a0..7bcf2e419037 100644
--- a/drivers/net/dsa/sja1105/sja1105_spi.c
+++ b/drivers/net/dsa/sja1105/sja1105_spi.c
@@ -40,19 +40,35 @@  static int sja1105_xfer(const struct sja1105_private *priv,
 			size_t len, struct ptp_system_timestamp *ptp_sts)
 {
 	u8 hdr_buf[SJA1105_SIZE_SPI_MSG_HEADER] = {0};
-	struct sja1105_chunk chunk = {
-		.len = min_t(size_t, len, SJA1105_SIZE_SPI_MSG_MAXLEN),
-		.reg_addr = reg_addr,
-		.buf = buf,
-	};
 	struct spi_device *spi = priv->spidev;
 	struct spi_transfer xfers[2] = {0};
 	struct spi_transfer *chunk_xfer;
 	struct spi_transfer *hdr_xfer;
+	struct sja1105_chunk chunk;
+	ssize_t xfer_len;
 	int num_chunks;
 	int rc, i = 0;
 
-	num_chunks = DIV_ROUND_UP(len, SJA1105_SIZE_SPI_MSG_MAXLEN);
+	/* One spi_message is composed of two spi_transfers: a small one for
+	 * the message header and another one for the current chunk of the
+	 * packed buffer.
+	 * Check that the restrictions imposed by the SPI controller are
+	 * respected: the chunk buffer is smaller than the max transfer size,
+	 * and the total length of the chunk plus its message header is smaller
+	 * than the max message size.
+	 */
+	xfer_len = min_t(ssize_t, SJA1105_SIZE_SPI_MSG_MAXLEN,
+			 spi_max_transfer_size(spi));
+	xfer_len = min_t(ssize_t, SJA1105_SIZE_SPI_MSG_MAXLEN,
+			 spi_max_message_size(spi) - SJA1105_SIZE_SPI_MSG_HEADER);
+	if (xfer_len < 0)
+		return -ERANGE;
+
+	num_chunks = DIV_ROUND_UP(len, xfer_len);
+
+	chunk.reg_addr = reg_addr;
+	chunk.buf = buf;
+	chunk.len = min_t(size_t, len, xfer_len);
 
 	hdr_xfer = &xfers[0];
 	chunk_xfer = &xfers[1];
@@ -104,7 +120,7 @@  static int sja1105_xfer(const struct sja1105_private *priv,
 		chunk.buf += chunk.len;
 		chunk.reg_addr += chunk.len / 4;
 		chunk.len = min_t(size_t, (ptrdiff_t)(buf + len - chunk.buf),
-				  SJA1105_SIZE_SPI_MSG_MAXLEN);
+				  xfer_len);
 
 		rc = spi_sync_transfer(spi, xfers, 2);
 		if (rc < 0) {