Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1434205 > unrolled thread

[PATCH 3/4] spi: Add option to insert delay between transactions

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 3/4] spi: Add option to insert delay between transactions apronin@chromium.org - 2016-06-30 06:00 +0200
    Re: [PATCH 3/4] spi: Add option to insert delay between transactions Doug Anderson <dianders@chromium.org> - 2016-07-01 06:50 +0200
    Re: [PATCH 3/4] spi: Add option to insert delay between transactions Mark Brown <broonie@kernel.org> - 2016-07-01 11:10 +0200

#1434205 — [PATCH 3/4] spi: Add option to insert delay between transactions

From: Andrey Pronin <apronin@chromium.org>

Some devices may need CS to be deasserted for some time
between transactions. Added a new capability to guarantee
a delay between SPI transactions for the device.

Signed-off-by: Andrey Pronin <apronin@chromium.org>
---
 drivers/spi/spi.c       | 3 +++
 include/linux/spi/spi.h | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index c51c864..639c7bd 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -971,6 +971,9 @@ static int spi_transfer_one_message(struct spi_master *master,
 		spi_reset_cs_wake_timer(msg->spi);
 	}
 
+	if (msg->spi->xfer_delay)
+		mdelay(msg->spi->xfer_delay);
+
 	spi_set_cs(msg->spi, true);
 
 	SPI_STATISTICS_INCREMENT_FIELD(statm, messages);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 4b06ba6..4b1aa13 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -137,6 +137,8 @@ void spi_statistics_add_transfer_stats(struct spi_statistics *stats,
  * @cs_wake_timer: Timer measuring the delay before the device goes to
  *	sleep after the last SPI transaction.
  *
+ * @xfer_delay: Delay between SPI transactions (msec).
+ *
  * @statistics: statistics for the spi_device
  *
  * A @spi_device is used to interchange data between an SPI slave
@@ -183,6 +185,8 @@ struct spi_device {
 	bool			cs_wake_needed;
 	struct timer_list	cs_wake_timer;
 
+	u32			xfer_delay;
+
 	/* the statistics */
 	struct spi_statistics	statistics;
 
-- 
2.6.6

[toc] | [next] | [standalone]

#1435013

Hi,

On Wed, Jun 29, 2016 at 8:54 PM,  <apronin@chromium.org> wrote:
> From: Andrey Pronin <apronin@chromium.org>
>
> Some devices may need CS to be deasserted for some time
> between transactions. Added a new capability to guarantee
> a delay between SPI transactions for the device.
>
> Signed-off-by: Andrey Pronin <apronin@chromium.org>
> ---
>  drivers/spi/spi.c       | 3 +++
>  include/linux/spi/spi.h | 4 ++++
>  2 files changed, 7 insertions(+)
>
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index c51c864..639c7bd 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -971,6 +971,9 @@ static int spi_transfer_one_message(struct spi_master *master,
>                 spi_reset_cs_wake_timer(msg->spi);
>         }
>
> +       if (msg->spi->xfer_delay)
> +               mdelay(msg->spi->xfer_delay);
> +

Sounds like you're generalizing "EC_SPI_RECOVERY_TIME_NS" from the
cros EC code.  That code actually keeps track of the end of the last
transfer and only delays as much as needed.

Also note that time is in NS, which seems like a more appropriate time
scale.  Even if the hardware you're talking to needs many milliseconds
to recover, there might be some that need only a few ns.


>         spi_set_cs(msg->spi, true);
>
>         SPI_STATISTICS_INCREMENT_FIELD(statm, messages);
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index 4b06ba6..4b1aa13 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -137,6 +137,8 @@ void spi_statistics_add_transfer_stats(struct spi_statistics *stats,
>   * @cs_wake_timer: Timer measuring the delay before the device goes to
>   *     sleep after the last SPI transaction.
>   *
> + * @xfer_delay: Delay between SPI transactions (msec).
> + *
>   * @statistics: statistics for the spi_device
>   *
>   * A @spi_device is used to interchange data between an SPI slave
> @@ -183,6 +185,8 @@ struct spi_device {
>         bool                    cs_wake_needed;
>         struct timer_list       cs_wake_timer;
>
> +       u32                     xfer_delay;
> +
>         /* the statistics */
>         struct spi_statistics   statistics;

As mentioned in the other patch, some of your code is missing and is
in patch #1.

-Doug

[toc] | [prev] | [next] | [standalone]

#1435176

[Multipart message — attachments visible in raw view] — view raw

On Wed, Jun 29, 2016 at 08:54:26PM -0700, apronin@chromium.org wrote:

> Some devices may need CS to be deasserted for some time
> between transactions. Added a new capability to guarantee
> a delay between SPI transactions for the device.

This seems like even more of a per device thing - it's a very rare
requirement (I'm guessing for some SPI controller coprocessor) and
there's such a wide range of potential patterns that might be needed
by different devices.  

> +       if (msg->spi->xfer_delay)
> +               mdelay(msg->spi->xfer_delay);
> +
>         spi_set_cs(msg->spi, true);

This isn't a delay between messages, it's a delay before asserting
chip select which will happen every single time we do anything
regardless of if there was any activity immediately before or not.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web