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

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

[Alexandru Gagniuc]

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

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

[Alexandru Gagniuc]

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

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 976 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 499 bytes --]

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. 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

[Alex Gagniuc]

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

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 179 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]

On Wed, Oct 26, 2016 at 10:55:28AM +0200, Maxime Ripard wrote:
> On Wed, Oct 26, 2016 at 12:00:30AM -0700, Alexandru Gagniuc wrote:

> > 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.

Without knowing what the previous discussion was it's hard to comment,
it sounds like some prior review comments are just being ignored here
but since I'm not turning up anything with this subject line I've no
idea what that might have been (and that's very concerning in itself
given that this is apparently v7...).  I'm also concerned that there
isn't a version of this for sun6i, it's going to make all the
cut'n'pasting between the two drivers harder if we make changes in one
and not the other.

If the concern from the previous reviews to do with not using DMA is
there some reason it's hard to do DMA?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2132 bytes --]

Hi Mark,

On Thu, Oct 27, 2016 at 12:14:19PM +0100, Mark Brown wrote:
> On Wed, Oct 26, 2016 at 10:55:28AM +0200, Maxime Ripard wrote:
> > On Wed, Oct 26, 2016 at 12:00:30AM -0700, Alexandru Gagniuc wrote:
> 
> > > 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.
> 
> Without knowing what the previous discussion was it's hard to comment,
> it sounds like some prior review comments are just being ignored here
> but since I'm not turning up anything with this subject line I've no
> idea what that might have been (and that's very concerning in itself
> given that this is apparently v7...).

v4 was here: https://patchwork.kernel.org/patch/3893371/
v5: https://patchwork.kernel.org/patch/5455381/
v6: https://patchwork.kernel.org/patch/6975871/

So basically, I really have no idea why, but it really seems like it
was just falling through the cracks, repeatedly (I'm not puting the
blame on anyone though, it just happened). Maybe it was just because
of the lack of comments :)

> I'm also concerned that there isn't a version of this for sun6i,
> it's going to make all the cut'n'pasting between the two drivers
> harder if we make changes in one and not the other.

I think I'll give reg_field a shot though, and try to merge the sun6i
driver into this one and see the results. If it can help your
decision.

> If the concern from the previous reviews to do with not using DMA is
> there some reason it's hard to do DMA?

I think just like Alexandru that it is orthogonal. But to really
answer, no, it's not difficult. There's just been some fundamental
disagreement on whether DMA was supposed to be optional or not that
stalled everything I guess.

Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1899 bytes --]

On Thu, Oct 27, 2016 at 11:27:27PM +0200, Maxime Ripard wrote:
> On Thu, Oct 27, 2016 at 12:14:19PM +0100, Mark Brown wrote:

> > but since I'm not turning up anything with this subject line I've no
> > idea what that might have been (and that's very concerning in itself
> > given that this is apparently v7...).

> v4 was here: https://patchwork.kernel.org/patch/3893371/
> v5: https://patchwork.kernel.org/patch/5455381/
> v6: https://patchwork.kernel.org/patch/6975871/

> So basically, I really have no idea why, but it really seems like it
> was just falling through the cracks, repeatedly (I'm not puting the
> blame on anyone though, it just happened). Maybe it was just because
> of the lack of comments :)

Oh, those subject lines were all starting ARM: rather than spi: -
there's a good chance I didn't look at the patches if I was busy
thinking they were changes for arch/arm rather than the SPI driver.

> > I'm also concerned that there isn't a version of this for sun6i,
> > it's going to make all the cut'n'pasting between the two drivers
> > harder if we make changes in one and not the other.

> I think I'll give reg_field a shot though, and try to merge the sun6i
> driver into this one and see the results. If it can help your
> decision.

It would definitely be nice given the level of duplication.

> > If the concern from the previous reviews to do with not using DMA is
> > there some reason it's hard to do DMA?

> I think just like Alexandru that it is orthogonal. But to really
> answer, no, it's not difficult. There's just been some fundamental
> disagreement on whether DMA was supposed to be optional or not that
> stalled everything I guess.

Oh, I seem to remember some patches adding DMA support that were doing
some strange special snowflake thing with ignoring errors now that I
think about it but that's not this one...  why did nobody ever follow up
on those?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Applied "spi: sun4i: Allow transfers larger than FIFO size" to the spi tree

[Mark Brown]

The patch

   spi: sun4i: Allow transfers larger than FIFO size

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 196737912da5eab055489e0635662a3b7adef6eb Mon Sep 17 00:00:00 2001
From: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Date: Wed, 26 Oct 2016 00:00:31 -0700
Subject: [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>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 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 4969dc10684a..c5cd635c28f3 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.10.1

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 933 bytes --]

Hi Mark,

On Thu, Oct 27, 2016 at 11:47:12PM +0100, Mark Brown wrote:
> > > If the concern from the previous reviews to do with not using DMA is
> > > there some reason it's hard to do DMA?
> 
> > I think just like Alexandru that it is orthogonal. But to really
> > answer, no, it's not difficult. There's just been some fundamental
> > disagreement on whether DMA was supposed to be optional or not that
> > stalled everything I guess.
> 
> Oh, I seem to remember some patches adding DMA support that were doing
> some strange special snowflake thing with ignoring errors now that I
> think about it but that's not this one...  why did nobody ever follow up
> on those?

I have no idea. I think last time we discussed this was last
July. Especially with this in now, there's probably no rush anyway.

Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]