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

Groups > linux.kernel > #1508859 > unrolled thread

[PATCH v7] spi: sun4i: Allow transfers larger than FIFO size

Back to article view | Back to linux.kernel

Contents

  [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size Alexandru Gagniuc <mr.nuke.me@gmail.com> - 2016-10-26 09:10 +0200
    [PATCH] spi: sun4i: Allow transfers larger than FIFO size Alexandru Gagniuc <mr.nuke.me@gmail.com> - 2016-10-26 09:10 +0200
    Re: [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size Mark Brown <broonie@kernel.org> - 2016-10-26 12:10 +0200
      Re: [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size Alex Gagniuc <mr.nuke.me@gmail.com> - 2016-10-26 18:10 +0200
        Re: [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size Mark Brown <broonie@kernel.org> - 2016-10-26 18:50 +0200
    Re: [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-27 00:20 +0200

#1508859 — [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size

This is the seventh attempt to get this patch in. I was prompted to look
into this again as someone recently remarked:

" cool, to bad spi does not work properly on allwinner..."

Two and a half years ago,  we were told this will all be handled with DMA.
A year or so ago, we were told this will all be handled with DMA.
A week or so ago, we were told this will all be handled with DMA.

See the pattern?
When DMA finally takes over, this fallback path is not mutually exclusive.

Changes since V6:
Rebased to make sure it applies on top of 4.9-rc.
Also tested on actual hardware.

Alexandru Gagniuc (1):
  spi: sun4i: Allow transfers larger than FIFO size

 drivers/spi/spi-sun4i.c | 75 +++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 67 insertions(+), 8 deletions(-)

--
2.7.4

[toc] | [next] | [standalone]

#1508861 — [PATCH] spi: sun4i: Allow transfers larger than FIFO size

SPI transfers were limited to one FIFO depth, which is 64 bytes.
This was an artificial limitation, however, as the hardware can handle
much larger bursts. To accommodate this, we enable the interrupt when
the Rx FIFO is 3/4 full, and drain the FIFO within the interrupt
handler. The 3/4 ratio was chosen arbitrarily, with the intention to
reduce the potential number of interrupts.

Since the SUN4I_CTL_TP bit is set, the hardware will pause
transmission whenever the FIFO is full, so there is no risk of losing
data if we can't service the interrupt in time.

For the Tx side, enable and use the Tx FIFO 3/4 empty interrupt to
replenish the FIFO on large SPI bursts. This requires more care in
when the interrupt is left enabled, as this interrupt will continually
trigger when the FIFO is less than 1/4 full, even though we
acknowledge it.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
---
 drivers/spi/spi-sun4i.c | 75 +++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 67 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 4969dc1..c5cd635 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -46,6 +46,8 @@
 #define SUN4I_CTL_TP				BIT(18)
 
 #define SUN4I_INT_CTL_REG		0x0c
+#define SUN4I_INT_CTL_RF_F34			BIT(4)
+#define SUN4I_INT_CTL_TF_E34			BIT(12)
 #define SUN4I_INT_CTL_TC			BIT(16)
 
 #define SUN4I_INT_STA_REG		0x10
@@ -61,11 +63,14 @@
 #define SUN4I_CLK_CTL_CDR1(div)			(((div) & SUN4I_CLK_CTL_CDR1_MASK) << 8)
 #define SUN4I_CLK_CTL_DRS			BIT(12)
 
+#define SUN4I_MAX_XFER_SIZE			0xffffff
+
 #define SUN4I_BURST_CNT_REG		0x20
-#define SUN4I_BURST_CNT(cnt)			((cnt) & 0xffffff)
+#define SUN4I_BURST_CNT(cnt)			((cnt) & SUN4I_MAX_XFER_SIZE)
 
 #define SUN4I_XMIT_CNT_REG		0x24
-#define SUN4I_XMIT_CNT(cnt)			((cnt) & 0xffffff)
+#define SUN4I_XMIT_CNT(cnt)			((cnt) & SUN4I_MAX_XFER_SIZE)
+
 
 #define SUN4I_FIFO_STA_REG		0x28
 #define SUN4I_FIFO_STA_RF_CNT_MASK		0x7f
@@ -96,6 +101,31 @@ static inline void sun4i_spi_write(struct sun4i_spi *sspi, u32 reg, u32 value)
 	writel(value, sspi->base_addr + reg);
 }
 
+static inline u32 sun4i_spi_get_tx_fifo_count(struct sun4i_spi *sspi)
+{
+	u32 reg = sun4i_spi_read(sspi, SUN4I_FIFO_STA_REG);
+
+	reg >>= SUN4I_FIFO_STA_TF_CNT_BITS;
+
+	return reg & SUN4I_FIFO_STA_TF_CNT_MASK;
+}
+
+static inline void sun4i_spi_enable_interrupt(struct sun4i_spi *sspi, u32 mask)
+{
+	u32 reg = sun4i_spi_read(sspi, SUN4I_INT_CTL_REG);
+
+	reg |= mask;
+	sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, reg);
+}
+
+static inline void sun4i_spi_disable_interrupt(struct sun4i_spi *sspi, u32 mask)
+{
+	u32 reg = sun4i_spi_read(sspi, SUN4I_INT_CTL_REG);
+
+	reg &= ~mask;
+	sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, reg);
+}
+
 static inline void sun4i_spi_drain_fifo(struct sun4i_spi *sspi, int len)
 {
 	u32 reg, cnt;
@@ -118,10 +148,13 @@ static inline void sun4i_spi_drain_fifo(struct sun4i_spi *sspi, int len)
 
 static inline void sun4i_spi_fill_fifo(struct sun4i_spi *sspi, int len)
 {
+	u32 cnt;
 	u8 byte;
 
-	if (len > sspi->len)
-		len = sspi->len;
+	/* See how much data we can fit */
+	cnt = SUN4I_FIFO_DEPTH - sun4i_spi_get_tx_fifo_count(sspi);
+
+	len = min3(len, (int)cnt, sspi->len);
 
 	while (len--) {
 		byte = sspi->tx_buf ? *sspi->tx_buf++ : 0;
@@ -184,10 +217,10 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 	u32 reg;
 
 	/* We don't support transfer larger than the FIFO */
-	if (tfr->len > SUN4I_FIFO_DEPTH)
+	if (tfr->len > SUN4I_MAX_XFER_SIZE)
 		return -EMSGSIZE;
 
-	if (tfr->tx_buf && tfr->len >= SUN4I_FIFO_DEPTH)
+	if (tfr->tx_buf && tfr->len >= SUN4I_MAX_XFER_SIZE)
 		return -EMSGSIZE;
 
 	reinit_completion(&sspi->done);
@@ -286,7 +319,11 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 	sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH - 1);
 
 	/* Enable the interrupts */
-	sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, SUN4I_INT_CTL_TC);
+	sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TC |
+					 SUN4I_INT_CTL_RF_F34);
+	/* Only enable Tx FIFO interrupt if we really need it */
+	if (tx_len > SUN4I_FIFO_DEPTH)
+		sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TF_E34);
 
 	/* Start the transfer */
 	reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
@@ -306,7 +343,6 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 		goto out;
 	}
 
-	sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
 
 out:
 	sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0);
@@ -322,10 +358,33 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)
 	/* Transfer complete */
 	if (status & SUN4I_INT_CTL_TC) {
 		sun4i_spi_write(sspi, SUN4I_INT_STA_REG, SUN4I_INT_CTL_TC);
+		sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
 		complete(&sspi->done);
 		return IRQ_HANDLED;
 	}
 
+	/* Receive FIFO 3/4 full */
+	if (status & SUN4I_INT_CTL_RF_F34) {
+		sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
+		/* Only clear the interrupt _after_ draining the FIFO */
+		sun4i_spi_write(sspi, SUN4I_INT_STA_REG, SUN4I_INT_CTL_RF_F34);
+		return IRQ_HANDLED;
+	}
+
+	/* Transmit FIFO 3/4 empty */
+	if (status & SUN4I_INT_CTL_TF_E34) {
+		sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);
+
+		if (!sspi->len)
+			/* nothing left to transmit */
+			sun4i_spi_disable_interrupt(sspi, SUN4I_INT_CTL_TF_E34);
+
+		/* Only clear the interrupt _after_ re-seeding the FIFO */
+		sun4i_spi_write(sspi, SUN4I_INT_STA_REG, SUN4I_INT_CTL_TF_E34);
+
+		return IRQ_HANDLED;
+	}
+
 	return IRQ_NONE;
 }
 
-- 
2.7.4

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

#1509023

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

On Wed, Oct 26, 2016 at 12:00:30AM -0700, Alexandru Gagniuc wrote:
> This is the seventh attempt to get this patch in. I was prompted to look
> into this again as someone recently remarked:

Please don't send cover letters for single patches, if there is anything
that needs saying put it in the changelog of the patch or after the ---
if it's administrative stuff.  This reduces mail volume and ensures that 
any important information is recorded in the changelog rather than being
lost. 

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

#1509559

On Wed, Oct 26, 2016 at 3:07 AM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Oct 26, 2016 at 12:00:30AM -0700, Alexandru Gagniuc wrote:
>> This is the seventh attempt to get this patch in. I was prompted to look
>> into this again as someone recently remarked:
>
> Please don't send cover letters for single patches, if there is anything
> that needs saying put it in the changelog of the patch or after the ---
> if it's administrative stuff.  This reduces mail volume and ensures that
> any important information is recorded in the changelog rather than being
> lost.

Thanks for the heads up! Sorry about that. Do you want me to resend
this as a single mail?

Alex

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

#1509591

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

On Wed, Oct 26, 2016 at 09:06:31AM -0700, Alex Gagniuc wrote:

> Thanks for the heads up! Sorry about that. Do you want me to resend
> this as a single mail?

It's fine, no need.

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

#1509901

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

On Wed, Oct 26, 2016 at 12:00:30AM -0700, Alexandru Gagniuc wrote:
> This is the seventh attempt to get this patch in. I was prompted to look
> into this again as someone recently remarked:
> 
> " cool, to bad spi does not work properly on allwinner..."
> 
> Two and a half years ago,  we were told this will all be handled with DMA.
> A year or so ago, we were told this will all be handled with DMA.
> A week or so ago, we were told this will all be handled with DMA.
> 
> See the pattern?
> When DMA finally takes over, this fallback path is not mutually exclusive.

I definitely agree, and we had this patch in the CHIP kernel for quite
some time, working like a charm.

I was planning to respin it in the next few days, glad to see you took
care of it :)

Mark, any comments on this? For the record, it already has my Acked-by.

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web