* [PATCH v4 0/2] spi: support inter-word delays @ 2019-01-29 8:38 Jonas Bonn 2019-01-29 8:38 ` [PATCH v4 1/2] spi: support inter-word delay requirement for devices Jonas Bonn 2019-01-29 8:38 ` [PATCH v4 2/2] spi-atmel: support inter-word delay Jonas Bonn 0 siblings, 2 replies; 7+ messages in thread From: Jonas Bonn @ 2019-01-29 8:38 UTC (permalink / raw) To: linux-kernel; +Cc: Jonas Bonn Changed in v4: * Rename word_delay to word_delay_us and slot it in _beside_ the existing word_delay parameter in spi_transfer (see commit message for more info). * Add code to __spi_validate to make sure transfer and device align with respect to the word_delay_us parameter Changed in v3: * Drop setting of inter-word delay via device tree Changed in v2: * Fix atmel-spi driver to not unconditionally set minimal delay if no delay is required (erroneous clamping) This short series adds support for SPI inter-word delays and configures the spi-atmel driver to honour the setting. Some SPI slaves are so slow that they are unable to keep up even at the SPI controller's lowest available clock frequency. I have such a configuration where an AVR-based SPI slave is unable to feed the SPI bus fast enough even the SPI master runs at the lowest possible clock speed. Jonas Bonn (2): spi: support inter-word delay requirement for devices spi-atmel: support inter-word delay drivers/spi/spi-atmel.c | 18 +++++++++++++----- drivers/spi/spi.c | 5 +++++ include/linux/spi/spi.h | 6 ++++++ 3 files changed, 24 insertions(+), 5 deletions(-) -- 2.19.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 1/2] spi: support inter-word delay requirement for devices 2019-01-29 8:38 [PATCH v4 0/2] spi: support inter-word delays Jonas Bonn @ 2019-01-29 8:38 ` Jonas Bonn 2019-01-29 18:06 ` Mark Brown 2019-01-29 8:38 ` [PATCH v4 2/2] spi-atmel: support inter-word delay Jonas Bonn 1 sibling, 1 reply; 7+ messages in thread From: Jonas Bonn @ 2019-01-29 8:38 UTC (permalink / raw) To: linux-kernel Cc: Jonas Bonn, Mark Brown, Rob Herring, Mark Rutland, linux-spi, devicetree Some devices are slow and cannot keep up with the SPI bus and therefore require a short delay between words of the SPI transfer. The example of this that I'm looking at is a SAMA5D2 with a minimum SPI clock of 400kHz talking to an AVR-based SPI slave. The AVR cannot put bytes on the bus fast enough to keep up with the SoC's SPI controller even at the lowest bus speed. This patch introduces the ability to specify a required inter-word delay for SPI devices. It is up to the controller driver to configure itself accordingly in order to introduce the requested delay. Note that, for spi_transfer, there is already a field word_delay that provides similar functionality. This field, however, is specified in clock cycles (and worse, SPI controller cycles, not SCK cycles); that makes this value dependent on the master clock instead of the device clock for which the delay is intended to provide some relief. This patch leaves this old word_delay in place and provides a time-based word_delay_us alongside it; the new field fits in the struct padding so struct size is constant. There is only one in-kernel user of the word_delay field and presumably that driver could be reworked to use the time-based value instead. The time-based delay is limited to 8 bits as these delays are intended to be short. The SAMA5D2 that I've tested this on limits delays to a maximum of ~100us, which is already many word-transfer periods even at the minimum transfer speed supported by the controller. Signed-off-by: Jonas Bonn <jonas@norrbonn.se> CC: Mark Brown <broonie@kernel.org> CC: Rob Herring <robh+dt@kernel.org> CC: Mark Rutland <mark.rutland@arm.com> CC: linux-spi@vger.kernel.org CC: devicetree@vger.kernel.org --- drivers/spi/spi.c | 5 +++++ include/linux/spi/spi.h | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 9a7def7c3237..64f13204bcb7 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -2961,6 +2961,8 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) * it is not set for this transfer. * Set transfer tx_nbits and rx_nbits as single transfer default * (SPI_NBITS_SINGLE) if it is not set for this transfer. + * Ensure transfer word_delay is at least as long as that required by + * device itself. */ message->frame_length = 0; list_for_each_entry(xfer, &message->transfers, transfer_list) { @@ -3031,6 +3033,9 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) !(spi->mode & SPI_RX_QUAD)) return -EINVAL; } + + if (xfer->word_delay_us < spi->word_delay_us) + xfer->word_delay_us = spi->word_delay_us; } message->status = -EINPROGRESS; diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 314d922ca607..e04622a01836 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -118,6 +118,8 @@ void spi_statistics_add_transfer_stats(struct spi_statistics *stats, * for driver coldplugging, and in uevents used for hotplugging * @cs_gpio: gpio number of the chipselect line (optional, -ENOENT when * not using a GPIO line) + * @word_delay_us: microsecond delay to be inserted between consecutive + * words of a transfer * * @statistics: statistics for the spi_device * @@ -164,6 +166,7 @@ struct spi_device { char modalias[SPI_NAME_SIZE]; const char *driver_override; int cs_gpio; /* chip select gpio */ + uint8_t word_delay_us; /* inter-word delay */ /* the statistics */ struct spi_statistics statistics; @@ -706,6 +709,8 @@ extern void spi_res_release(struct spi_controller *ctlr, * @delay_usecs: microseconds to delay after this transfer before * (optionally) changing the chipselect status, then starting * the next transfer or completing this @spi_message. + * @word_delay_us: microseconds to inter word delay after each word size + * (set by bits_per_word) transmission. * @word_delay: clock cycles to inter word delay after each word size * (set by bits_per_word) transmission. * @transfer_list: transfers are sequenced through @spi_message.transfers @@ -788,6 +793,7 @@ struct spi_transfer { #define SPI_NBITS_DUAL 0x02 /* 2bits transfer */ #define SPI_NBITS_QUAD 0x04 /* 4bits transfer */ u8 bits_per_word; + u8 word_delay_us; u16 delay_usecs; u32 speed_hz; u16 word_delay; -- 2.19.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/2] spi: support inter-word delay requirement for devices 2019-01-29 8:38 ` [PATCH v4 1/2] spi: support inter-word delay requirement for devices Jonas Bonn @ 2019-01-29 18:06 ` Mark Brown 0 siblings, 0 replies; 7+ messages in thread From: Mark Brown @ 2019-01-29 18:06 UTC (permalink / raw) To: Jonas Bonn; +Cc: linux-kernel, Rob Herring, Mark Rutland, linux-spi, devicetree [-- Attachment #1: Type: text/plain, Size: 278 bytes --] On Tue, Jan 29, 2019 at 09:38:43AM +0100, Jonas Bonn wrote: > Some devices are slow and cannot keep up with the SPI bus and therefore > require a short delay between words of the SPI transfer. This doesn't apply against current code, please rebase against for-next and resend. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 2/2] spi-atmel: support inter-word delay 2019-01-29 8:38 [PATCH v4 0/2] spi: support inter-word delays Jonas Bonn 2019-01-29 8:38 ` [PATCH v4 1/2] spi: support inter-word delay requirement for devices Jonas Bonn @ 2019-01-29 8:38 ` Jonas Bonn 2019-01-29 14:27 ` Nicolas.Ferre 1 sibling, 1 reply; 7+ messages in thread From: Jonas Bonn @ 2019-01-29 8:38 UTC (permalink / raw) To: linux-kernel Cc: Jonas Bonn, Nicolas Ferre, Mark Brown, Alexandre Belloni, Ludovic Desroches, linux-spi, linux-arm-kernel If the SPI slave requires an inter-word delay, configure the DLYBCT register accordingly. Tested on a SAMA5D2 board (derived from SAMA5D2-Xplained reference board). Signed-off-by: Jonas Bonn <jonas@norrbonn.se> CC: Nicolas Ferre <nicolas.ferre@microchip.com> CC: Mark Brown <broonie@kernel.org> CC: Alexandre Belloni <alexandre.belloni@bootlin.com> CC: Ludovic Desroches <ludovic.desroches@microchip.com> CC: linux-spi@vger.kernel.org CC: linux-arm-kernel@lists.infradead.org --- drivers/spi/spi-atmel.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c index 74fddcd3282b..6389a228d2f5 100644 --- a/drivers/spi/spi-atmel.c +++ b/drivers/spi/spi-atmel.c @@ -1209,13 +1209,21 @@ static int atmel_spi_setup(struct spi_device *spi) csr |= SPI_BIT(CSAAT); /* DLYBS is mostly irrelevant since we manage chipselect using GPIOs. - * - * DLYBCT would add delays between words, slowing down transfers. - * It could potentially be useful to cope with DMA bottlenecks, but - * in those cases it's probably best to just use a lower bitrate. */ csr |= SPI_BF(DLYBS, 0); - csr |= SPI_BF(DLYBCT, 0); + + /* DLYBCT adds delays between words. This is useful for slow devices + * that need a bit of time to setup the next transfer. + */ + if (spi->word_delay_us) { + csr |= SPI_BF(DLYBCT, + clamp_t(u8, + (as->spi_clk/1000000*spi->word_delay_us)>>5, + 1, 255)); + } else { + csr |= SPI_BF(DLYBCT, 0); + } + /* chipselect must have been muxed as GPIO (e.g. in board setup) */ npcs_pin = (unsigned long)spi->controller_data; -- 2.19.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4 2/2] spi-atmel: support inter-word delay 2019-01-29 8:38 ` [PATCH v4 2/2] spi-atmel: support inter-word delay Jonas Bonn @ 2019-01-29 14:27 ` Nicolas.Ferre 2019-01-29 14:56 ` Jonas Bonn 0 siblings, 1 reply; 7+ messages in thread From: Nicolas.Ferre @ 2019-01-29 14:27 UTC (permalink / raw) To: jonas, linux-kernel Cc: broonie, alexandre.belloni, Ludovic.Desroches, linux-spi, linux-arm-kernel, Tudor.Ambarus On 29/01/2019 at 09:38, Jonas Bonn wrote: > If the SPI slave requires an inter-word delay, configure the DLYBCT > register accordingly. > > Tested on a SAMA5D2 board (derived from SAMA5D2-Xplained reference > board). > > Signed-off-by: Jonas Bonn <jonas@norrbonn.se> > CC: Nicolas Ferre <nicolas.ferre@microchip.com> > CC: Mark Brown <broonie@kernel.org> > CC: Alexandre Belloni <alexandre.belloni@bootlin.com> > CC: Ludovic Desroches <ludovic.desroches@microchip.com> > CC: linux-spi@vger.kernel.org > CC: linux-arm-kernel@lists.infradead.org > --- > drivers/spi/spi-atmel.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c > index 74fddcd3282b..6389a228d2f5 100644 > --- a/drivers/spi/spi-atmel.c > +++ b/drivers/spi/spi-atmel.c > @@ -1209,13 +1209,21 @@ static int atmel_spi_setup(struct spi_device *spi) > csr |= SPI_BIT(CSAAT); > > /* DLYBS is mostly irrelevant since we manage chipselect using GPIOs. > - * > - * DLYBCT would add delays between words, slowing down transfers. > - * It could potentially be useful to cope with DMA bottlenecks, but > - * in those cases it's probably best to just use a lower bitrate. > */ > csr |= SPI_BF(DLYBS, 0); > - csr |= SPI_BF(DLYBCT, 0); > + > + /* DLYBCT adds delays between words. This is useful for slow devices > + * that need a bit of time to setup the next transfer. > + */ > + if (spi->word_delay_us) { Well... > + csr |= SPI_BF(DLYBCT, > + clamp_t(u8, > + (as->spi_clk/1000000*spi->word_delay_us)>>5, > + 1, 255)); ... why not simplifying to: + 0, 255)); and remove the test altogether, after all? > + } else { > + csr |= SPI_BF(DLYBCT, 0); > + } > + > > /* chipselect must have been muxed as GPIO (e.g. in board setup) */ > npcs_pin = (unsigned long)spi->controller_data; > -- Nicolas Ferre ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 2/2] spi-atmel: support inter-word delay 2019-01-29 14:27 ` Nicolas.Ferre @ 2019-01-29 14:56 ` Jonas Bonn 2019-01-29 15:05 ` Alexandre Belloni 0 siblings, 1 reply; 7+ messages in thread From: Jonas Bonn @ 2019-01-29 14:56 UTC (permalink / raw) To: Nicolas.Ferre, linux-kernel Cc: broonie, alexandre.belloni, Ludovic.Desroches, linux-spi, linux-arm-kernel, Tudor.Ambarus Hi, On 29/01/2019 15:27, Nicolas.Ferre@microchip.com wrote: > On 29/01/2019 at 09:38, Jonas Bonn wrote: >> >> + /* DLYBCT adds delays between words. This is useful for slow devices >> + * that need a bit of time to setup the next transfer. >> + */ >> + if (spi->word_delay_us) { > > Well... > >> + csr |= SPI_BF(DLYBCT, >> + clamp_t(u8, >> + (as->spi_clk/1000000*spi->word_delay_us)>>5, >> + 1, 255)); > > ... why not simplifying to: > + 0, 255)); > and remove the test altogether, after all? Hmm... that seemed too easy! This started out as something else and looking at it now I think even the clamp_t() is unnecessary. The value is already 0-255 and the way SPI_BF works any overflow is already truncated... I'll rework this and resubmit once I get some feedback on the word_delay_us bits. Thanks, /Jonas ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 2/2] spi-atmel: support inter-word delay 2019-01-29 14:56 ` Jonas Bonn @ 2019-01-29 15:05 ` Alexandre Belloni 0 siblings, 0 replies; 7+ messages in thread From: Alexandre Belloni @ 2019-01-29 15:05 UTC (permalink / raw) To: Jonas Bonn Cc: Nicolas.Ferre, linux-kernel, broonie, Ludovic.Desroches, linux-spi, linux-arm-kernel, Tudor.Ambarus Hi, On 29/01/2019 15:56:31+0100, Jonas Bonn wrote: > On 29/01/2019 15:27, Nicolas.Ferre@microchip.com wrote: > > On 29/01/2019 at 09:38, Jonas Bonn wrote: > > > > > > + /* DLYBCT adds delays between words. This is useful for slow devices > > > + * that need a bit of time to setup the next transfer. > > > + */ > > > + if (spi->word_delay_us) { > > > > Well... > > > > > + csr |= SPI_BF(DLYBCT, > > > + clamp_t(u8, > > > + (as->spi_clk/1000000*spi->word_delay_us)>>5, > > > + 1, 255)); > > > > ... why not simplifying to: > > + 0, 255)); > > and remove the test altogether, after all? > > Hmm... that seemed too easy! This started out as something else and looking > at it now I think even the clamp_t() is unnecessary. The value is already > 0-255 and the way SPI_BF works any overflow is already truncated... I'll > rework this and resubmit once I get some feedback on the word_delay_us bits. > While at it, note that you need to add spaces around the operators. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-01-29 18:06 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-29 8:38 [PATCH v4 0/2] spi: support inter-word delays Jonas Bonn 2019-01-29 8:38 ` [PATCH v4 1/2] spi: support inter-word delay requirement for devices Jonas Bonn 2019-01-29 18:06 ` Mark Brown 2019-01-29 8:38 ` [PATCH v4 2/2] spi-atmel: support inter-word delay Jonas Bonn 2019-01-29 14:27 ` Nicolas.Ferre 2019-01-29 14:56 ` Jonas Bonn 2019-01-29 15:05 ` Alexandre Belloni
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).