Message ID | 20210520200223.3375421-3-olteanv@gmail.com |
---|---|
State | New |
Headers | show |
Series | None | expand |
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 --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) {