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

[mrnuke]

From c541c363b339d145f326747db5a3b0fabce2780a Mon Sep 17 00:00:00 2001
From: Alexandru Gagniuc <mr.nu...@gmail.com>
Date: Mon, 17 Mar 2014 20:08:05 -0500
Subject: [PATCH] NOTFORMERGE: ARM: sun4i: spi: 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.

In the long term, we'll be better off handling this as DMA transfers,
but for now, this enables some userspace software, such as flashrom,
to use the A10 SPI controller via the spidev interfac without receing
-EINVAL.

This patch should NOT be merged, as it only handles long Rx transfers.
Long Tx transfers have not been tested, and they will most likely fail
miserably. There is currently no refreshing of Tx FIFOs when they run
empty on long transfers.

I'm, watching the bus with a logic analyzer, and at low speeds (2 MHz
or less), the bursts are continuous. However, on higher speeds, the
bursts seem to happen in 64-byte micro-bursts. My assumption is that
we aren't servicing the 3/4 FIFO interrupts fast enough, the FIFOs get
filled, and the controller pauses communication. At higher speeds,
there seem to be weird effects, such as the transmission pausing mid-
byte for a long period of time, but I suspect this to be due to my
logic analyzer being too slow and/or too much impedance in the
connecting wires (logic analyzer missing SCLK transitions).

This patch is currently tested on top of sunxi-devel, but should apply
cleanly to sunxi-next as well.

Signed-off-by: Alexandru Gagniuc <mr.nu...@gmail.com>
---
drivers/spi/spi-sun4i.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 3f82705..f3d06bb 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -47,6 +47,7 @@
#define SUN4I_CTL_TP BIT(18)

#define SUN4I_INT_CTL_REG 0x0c
+#define SUN4I_INT_CTL_RF_F34 BIT(4)
#define SUN4I_INT_CTL_TC BIT(16)

#define SUN4I_INT_STA_REG 0x10
@@ -68,6 +69,8 @@
#define SUN4I_XMIT_CNT_REG 0x24
#define SUN4I_XMIT_CNT(cnt) ((cnt) & 0xffffff)

+#define SUN4I_MAX_XFER_SIZE 0xffffff
+
#define SUN4I_FIFO_STA_REG 0x28
#define SUN4I_FIFO_STA_RF_CNT_MASK 0x7f
#define SUN4I_FIFO_STA_RF_CNT_BITS 0
@@ -175,8 +178,7 @@ static int sun4i_spi_transfer_one(struct spi_master
*master,
int ret = 0;
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 -EINVAL;

reinit_completion(&sspi->done);
@@ -274,7 +276,8 @@ static int sun4i_spi_transfer_one(struct spi_master
*master,
sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);

/* Enable the interrupts */
- sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, SUN4I_INT_CTL_TC);
+ sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, SUN4I_INT_CTL_TC |
+ SUN4I_INT_CTL_RF_F34);

/* Start the transfer */
reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
@@ -287,7 +290,7 @@ static int sun4i_spi_transfer_one(struct spi_master
*master,
goto out;
}

- sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
+ /* FIFO is drained during the interrupt handler */

out:
sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0);
@@ -300,9 +303,18 @@ static irqreturn_t sun4i_spi_handler(int irq, void
*dev_id)
struct sun4i_spi *sspi = dev_id;
u32 status = sun4i_spi_read(sspi, SUN4I_INT_STA_REG);

+ /* 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;
+ }
+
/* 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;
}
--
1.8.5.3

[mrnuke]

On Monday, March 17, 2014 08:28:26 PM mrnuke wrote:

I re-ran some captures after a clean reset.

> However, on higher speeds, the
> bursts seem to happen in 64-byte micro-bursts. My assumption is that
> we aren't servicing the 3/4 FIFO interrupts fast enough, the FIFOs get
> filled, and the controller pauses communication.

Up to 10MHz SCLK, bursts are continuous with this patch.

> At higher speeds,
> there seem to be weird effects, such as the transmission pausing mid-
> byte for a long period of time, but I suspect this to be due to my
> logic analyzer being too slow and/or too much impedance in the
> connecting wires (logic analyzer missing SCLK transitions).
>

My logic analyzer cannot reliably capture higher clocks, so any results at
speeds higher than 10MHz are completely bogus.

Alex

[Maxime Ripard]

Hi,

On Mon, Mar 17, 2014 at 08:28:26PM -0500, mrnuke wrote:
> From c541c363b339d145f326747db5a3b0fabce2780a Mon Sep 17 00:00:00 2001
> From: Alexandru Gagniuc <mr.nu...@gmail.com>
> Date: Mon, 17 Mar 2014 20:08:05 -0500
> Subject: [PATCH] NOTFORMERGE: ARM: sun4i: spi: 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.

Cool. I started to work on this for the A31, but got nowhere since I
couldn't figure out how to restart a transfer when the fifo was
depleted. I assumed it wouldn't just work on the older SoCs too, but
it seems like I was wrong :)

> 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.
>
> In the long term, we'll be better off handling this as DMA transfers,

Yep.

> but for now, this enables some userspace software, such as flashrom,
> to use the A10 SPI controller via the spidev interfac without receing
> -EINVAL.
>
> This patch should NOT be merged, as it only handles long Rx transfers.
> Long Tx transfers have not been tested, and they will most likely fail
> miserably. There is currently no refreshing of Tx FIFOs when they run
> empty on long transfers.

I'm not sure about what to do with this patch then :)

Could you work on making it working for TX FIFOs too?

Thanks!
Maxime

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

[mrnuke]

On Tuesday, March 18, 2014 10:43:28 AM Maxime Ripard wrote:
> > This patch should NOT be merged, as it only handles long Rx transfers.
> > Long Tx transfers have not been tested, and they will most likely fail
> > miserably. There is currently no refreshing of Tx FIFOs when they run
> > empty on long transfers.
>
> I'm not sure about what to do with this patch then :)

Bikeshed :). I wanted to make sure the approach is not wrong.

> Could you work on making it working for TX FIFOs too?

Sure. That's the plan.

Alex

[Alexandru Gagniuc]

This is the second iteration of the patches sent yesterday. Per
Maxime's request, I have squashed them into one single patch, and
applied most of the suggestion he made.

As far as only clearing the interrupt _after_ draining the FIFO, I
found that we usually get one extra Rx interrupt per burst if we do
this, so I left the comment in.

>> /* Enable the interrupts */
>> - sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, SUN4I_INT_CTL_TC |

>> - SUN4I_INT_CTL_RF_F34);
>> + reg = 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)
>> + reg |= SUN4I_INT_CTL_TF_E34;
>> + sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, reg);
>
> I'd be much dumber than that. Why don't you just enable both
> interrupts all the time if we need larger transfers ?

As I've pointed out previously, this interrupt is PITA, as it triggers
an IRQ storm if enabled without much thought. I found that
(tx_len > SUN4I_FIFO_DEPTH) is the best check for when it's safe to
enable this interrupt.

Alexandru Gagniuc (1):

ARM: sun4i: spi: Allow transfers larger than FIFO size

drivers/spi/spi-sun4i.c | 68 +++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 63 insertions(+), 5 deletions(-)

--
1.8.5.3

[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.nu...@gmail.com>
---

drivers/spi/spi-sun4i.c | 68 +++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 63 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 3f82705..a12da6d 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -47,6 +47,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

@@ -68,6 +70,8 @@

#define SUN4I_XMIT_CNT_REG 0x24
#define SUN4I_XMIT_CNT(cnt) ((cnt) & 0xffffff)

+#define SUN4I_MAX_XFER_SIZE 0xffffff
+
#define SUN4I_FIFO_STA_REG 0x28
#define SUN4I_FIFO_STA_RF_CNT_MASK 0x7f
#define SUN4I_FIFO_STA_RF_CNT_BITS 0

@@ -97,6 +101,28 @@ 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;
+ 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;
@@ -119,8 +145,15 @@ 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;

+ /* See how much data we can fit */
+ cnt = SUN4I_FIFO_DEPTH - sun4i_spi_get_tx_fifo_count(sspi);
+
+ if (len > cnt)
+ len = cnt;
+
if (len > sspi->len)
len = sspi->len;

@@ -175,8 +208,7 @@ static int sun4i_spi_transfer_one(struct spi_master *master,

int ret = 0;
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 -EINVAL;

reinit_completion(&sspi->done);

@@ -274,7 +306,11 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);

/* Enable the interrupts */

- sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, SUN4I_INT_CTL_TC);

+ reg = 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)
+ reg |= SUN4I_INT_CTL_TF_E34;
+ sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, reg);

/* Start the transfer */
reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);

@@ -287,8 +323,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);

@@ -299,14 +333,38 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)

{
struct sun4i_spi *sspi = dev_id;
u32 status = sun4i_spi_read(sspi, SUN4I_INT_STA_REG);

+ u32 cnt;

/* 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;
}

--
1.8.5.3

[Alexandru Gagniuc]

[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.nu...@gmail.com>
---

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

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 3f82705..9dd55d3 100644

/* Enable the interrupts */

- sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, SUN4I_INT_CTL_TC);

+ reg = 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)
+ reg |= SUN4I_INT_CTL_TF_E34;
+ sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, reg);

/* Start the transfer */
reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
@@ -287,8 +323,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);

@@ -303,10 +337,33 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)

[Maxime Ripard]

I'd rather define the mask only once here, and reuse it in the macro
just above.

A comment here about why you're doing so would be nice

> return -EINVAL;
>
> reinit_completion(&sspi->done);
> @@ -274,7 +306,11 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
> sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);
>
> /* Enable the interrupts */
> - sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, SUN4I_INT_CTL_TC);
> + reg = 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)
> + reg |= SUN4I_INT_CTL_TF_E34;
> + sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, reg);

You probably can use the _enable_interrupt function you just defined here :)

[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.nu...@gmail.com>
---

drivers/spi/spi-sun4i.c | 72 ++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 65 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 3f82705..1557528 100644

--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -47,6 +47,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

@@ -62,11 +64,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

@@ -97,6 +102,28 @@ static inline void sun4i_spi_write(struct sun4i_spi *sspi, u32 reg, u32 value)

@@ -119,8 +146,15 @@ 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;

+ /* See how much data we can fit */
+ cnt = SUN4I_FIFO_DEPTH - sun4i_spi_get_tx_fifo_count(sspi);
+
+ if (len > cnt)
+ len = cnt;
+
if (len > sspi->len)
len = sspi->len;

@@ -175,8 +209,8 @@ static int sun4i_spi_transfer_one(struct spi_master *master,

int ret = 0;
u32 reg;

- /* We don't support transfer larger than the FIFO */
- if (tfr->len > SUN4I_FIFO_DEPTH)

+ /* This is the maximum SPI burst size supported by the hardware */

+ if (tfr->len > SUN4I_MAX_XFER_SIZE)

return -EINVAL;

reinit_completion(&sspi->done);
@@ -274,7 +308,10 @@ static int sun4i_spi_transfer_one(struct spi_master *master,

sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);

/* 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);

@@ -287,8 +324,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);

@@ -303,10 +338,33 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)

[Maxime Ripard]

Hi,

Thanks for respinning this patch.

You seem to assign the reg variables when declaring it in other part
of your code. It would be great to be consistent.

You probably want to use min3 here

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;
> }
>
> --
> 1.8.5.3
>

Once these minor issues fixed, you can add my
Acked-by: Maxime Ripard <maxime...@free-electrons.com>

Thanks!

[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.nu...@gmail.com>

Acked-by: Maxime Ripard <maxime...@free-electrons.com>
---
drivers/spi/spi-sun4i.c | 71 ++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 62 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 3f82705..d80ceba 100644

--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -47,6 +47,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
@@ -62,11 +64,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

@@ -97,6 +102,27 @@ 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;

@@ -119,10 +145,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;
@@ -175,8 +204,8 @@ static int sun4i_spi_transfer_one(struct spi_master *master,

int ret = 0;
u32 reg;

- /* We don't support transfer larger than the FIFO */
- if (tfr->len > SUN4I_FIFO_DEPTH)
+ /* This is the maximum SPI burst size supported by the hardware */
+ if (tfr->len > SUN4I_MAX_XFER_SIZE)
return -EINVAL;

reinit_completion(&sspi->done);

@@ -274,7 +303,10 @@ static int sun4i_spi_transfer_one(struct spi_master *master,

sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);

/* 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);

@@ -287,8 +319,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);

@@ -303,10 +333,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)

[Maxime Ripard]

On Mon, Mar 31, 2014 at 09:16:51PM -0500, Alexandru Gagniuc wrote:
> 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.nu...@gmail.com>
> Acked-by: Maxime Ripard <maxime...@free-electrons.com>

Hmmm, you forgot to Cc Mark Brown. Please resubmit it Ccing whoever
shows up using get_maintainer, otherwise, your patch won't be applied.

You also forgot the changes between this version and the previous one.

Thanks!
Maxime

[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.nu...@gmail.com>
Acked-by: Maxime Ripard <maxime...@free-electrons.com>

---

Changes from v4:
* use min3() instead of two if statements in sun4i_spi_fill_fifo()
* fix trivial whitespace issue in if statement in sun4i_spi_handler()
* use consistent style in assigning 'reg' in the added functions.

[Olliver Schinagl]

Hey all,

Alexandru Gagniuc <mr.nuke.me@...> writes:

>
> 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@...>
> Acked-by: Maxime Ripard <maxime.ripard@...>

This patch was posted more then a year ago and I think several tree's carry it.

I've used this patch in combination with the mmc-spi driver and found no
problems with it in the past year.

So have my Tested-by: Olliver Schinagl <oli...@schinagl.nl>

and lets get this patch merged!

P.S. I have the original patch and can resend it if needed.
<snip>

[Olliver Schinagl]

Heh, that didn't go as I hoped,

I used gmane's interface hoping it would bump all the original lists and
users, since i don't have the original mail anymore I don't think :S

So sorry for the noise and l'll try to bump this more properly :)

[Olliver Schinagl]

Alexandru sent this patch over a year and a half ago, and I believe several
tree's have been carrying it since. We've been using this patch on an
Olimex OLinuxIno Lime1 and Lime2 using the mmc-spi driver to access SD cards
without problems. So bumping this and adding my

Tested-by: Olliver Schinagl <oli...@schinagl.nl>

Changes from v5as warned by checkpatch. No functional changes.
* Added some newlines to make checkpatch happy. No functional changes.

Changes from v4:
* use min3() instead of two if statements in sun4i_spi_fill_fifo()
* fix trivial whitespace issue in if statement in sun4i_spi_handler()
* use consistent style in assigning 'reg' in the added functions.

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

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

--
2.1.4

[Olliver Schinagl]

From: Alexandru Gagniuc <mr.nu...@gmail.com>

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.nu...@gmail.com>
Acked-by: Maxime Ripard <maxime...@free-electrons.com>

Signed-off-by: Olliver Schinagl <o.sch...@ultimaker.com>
---

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

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index fbb0a4d..9a30e49 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)

@@ -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;

@@ -174,8 +207,8 @@ static int sun4i_spi_transfer_one(struct spi_master *master,

int ret = 0;
u32 reg;

- /* We don't support transfer larger than the FIFO */
- if (tfr->len > SUN4I_FIFO_DEPTH)
+ /* This is the maximum SPI burst size supported by the hardware */
+ if (tfr->len > SUN4I_MAX_XFER_SIZE)
return -EINVAL;

reinit_completion(&sspi->done);

@@ -273,7 +306,11 @@ static int sun4i_spi_transfer_one(struct spi_master *master,

sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);

/* 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);

@@ -286,8 +323,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);

@@ -302,10 +337,33 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)

2.1.4

[Michal Suchanek]

Hello,

There are basically two issues with this

1) it conflicts with DMA support. Do we want this when there is DMA
support almost complete?

2) it adds support on sun4i but not sun6i. Unfortunately, I do not
have any sun6i hardware. sun6i has dmaengine in mainline but not the
dma support in spi.

That aside,

have you tested this for small transfers that would not trigger the
fifo 3/4 full interrupt?

The sun4i_spi_drain_fifo has moved from sun4i_spi_transfer_one to
sun4i_spi_handler. Presumably completing the transfer will also
trigger the handler but previously sun4i_spi_drain_fifo was called
unconditionally.

FWIW I have megred the DMA support with this but have not tested it so far.

old https://github.com/hramrach/linux-sunxi/commits/sunxi-spi-dma
new https://github.com/hramrach/linux-sunxi/commits/sunxi-spi-pio-dma

Thanks

Michal

[Olliver Schinagl]

Hey Michal,

Well this would affect PIO mode in all cases, so do we want this? I
would think yet in case DMA is not available/disabled?

As without this patch, we cannot use something like mmc-spi due to the
restriction of packet size iirc.

>
> 2) it adds support on sun4i but not sun6i. Unfortunately, I do not
> have any sun6i hardware. sun6i has dmaengine in mainline but not the
> dma support in spi.

Well yeah, I don't have sun6i either so we can't merge something nobody
has tested yet ;)

>
> That aside,
>
> have you tested this for small transfers that would not trigger the
> fifo 3/4 full interrupt?

Well the mmc-spi would also have small transfers, no? I mean it's a
little bit of everything I imagine?

>
> The sun4i_spi_drain_fifo has moved from sun4i_spi_transfer_one to
> sun4i_spi_handler. Presumably completing the transfer will also
> trigger the handler but previously sun4i_spi_drain_fifo was called
> unconditionally.

That's something Alexandru would have to verify/check.

[Michal Suchanek]

When will that happen?

>
> As without this patch, we cannot use something like mmc-spi due to the
> restriction of packet size iirc.

With DMA we can. And since DMA will be enabled most of the time the
non-DMA code will receive little or no testing.

>>
>>
>> 2) it adds support on sun4i but not sun6i. Unfortunately, I do not
>> have any sun6i hardware. sun6i has dmaengine in mainline but not the
>> dma support in spi.
>
> Well yeah, I don't have sun6i either so we can't merge something nobody has
> tested yet ;)
>>
>>
>> That aside,
>>
>> have you tested this for small transfers that would not trigger the
>> fifo 3/4 full interrupt?
>
> Well the mmc-spi would also have small transfers, no? I mean it's a little
> bit of everything I imagine?

I have no idea about mmc spi tbh. I have some mmc sockets lying around
but did not try using them and did not look at the protocol.

Thanks

Michal

[Alex G.]

On 08/10/2015 07:54 AM, Michal Suchanek wrote:
> On 10 August 2015 at 15:44, Olliver Schinagl <olive...@schinagl.nl> wrote:
>> Hey Michal,
>>

>> As without this patch, we cannot use something like mmc-spi due to the
>> restriction of packet size iirc.
>
> With DMA we can. And since DMA will be enabled most of the time the
> non-DMA code will receive little or no testing.
>

That was also an objection over a year ago, when this patch was first
ready, and still, no DMA support is implemented. It would be amazing to
have a way to run larger transfers _today_ :) .

>>>
>>> have you tested this for small transfers that would not trigger the
>>> fifo 3/4 full interrupt?
>>

It worked as expected with any size transfer when I tested it (logic
analyzer confirmed TM). With that being said, I haven't looked at this
patch in a long time, but the underlying logic should still behave the same.


Alex

[Michal Suchanek]

Hello,

On 14 August 2015 at 01:08, Alex G. <mr.nu...@gmail.com> wrote:
> On 08/10/2015 07:54 AM, Michal Suchanek wrote:
>> On 10 August 2015 at 15:44, Olliver Schinagl <olive...@schinagl.nl> wrote:
>>> Hey Michal,
>>>
>>> As without this patch, we cannot use something like mmc-spi due to the
>>> restriction of packet size iirc.
>>
>> With DMA we can. And since DMA will be enabled most of the time the
>> non-DMA code will receive little or no testing.
>>
> That was also an objection over a year ago, when this patch was first
> ready, and still, no DMA support is implemented. It would be amazing to
> have a way to run larger transfers _today_ :) .

The dmaengine patch should get into 4.3 probably.

Thanks

Michal

[Iru Cai]

Thanks for this patch, and I can use flashrom in Cubieboard with the patch v6 and mainline kernel.

However, there's still some problems. I flashed a chip off the board and it works well, but when I read a chip on a mainboard twice, the result is different. Is there something wrong with the SPI driver, or is it a hardware issue?

在 2014年3月18日星期二 UTC+8上午9:28:26，mrnuke写道：

[Priit Laes]

On Sun, 2016-03-27 at 02:10 -0700, Iru Cai wrote:
> Thanks for this patch, and I can use flashrom in Cubieboard with the
> patch v6 and mainline kernel.
>
> However, there's still some problems. I flashed a chip off the board
> and it works well, but when I read a chip on a mainboard twice, the
> result is different. Is there something wrong with the SPI driver, or
> is it a hardware issue?

Could you also provide link (or send a new letter with attachment) to
patch v6?

[Iru Cai]

在 2016年3月27日星期日 UTC+8下午8:46:51，Priit Laes写道：

On Sun, 2016-03-27 at 02:10 -0700, Iru Cai wrote:
> Thanks for this patch, and I can use flashrom in Cubieboard with the
> patch v6 and mainline kernel.
>
> However, there's still some problems. I flashed a chip off the board
> and it works well, but when I read a chip on a mainboard twice, the
> result is different. Is there something wrong with the SPI driver, or
> is it a hardware issue?

Could you also provide link (or send a new letter with attachment) to
patch v6?

Oh, just in this thread. https://groups.google.com/d/msg/linux-sunxi/M8ZSFzWKjfY/DhIuWxWjAgAJ
 

[Alex G.]

On 03/27/2016 02:10 AM, Iru Cai wrote:
> Thanks for this patch, and I can use flashrom in Cubieboard with the
> patch v6 and mainline kernel.
>
> However, there's still some problems. I flashed a chip off the board and
> it works well, but when I read a chip on a mainboard twice, the result
> is different. Is there something wrong with the SPI driver, or is it a
> hardware issue?

This worked for both reads and writes a few years back when I wrote it
against 3.1x, but it is possible that regressions have since been
introduced. If you want to rule out hardware issues, try to probe the
chip out-of-circuit (but make sure you pull WP and HOLD pins appropriately).

Personally, I think there's some other device on the board interfering
with the SPI master.

Alex

> 在 2014年3月18日星期二 UTC+8上午9:28:26，mrnuke写道：
>
> From c541c363b339d145f326747db5a3b0fabce2780a Mon Sep 17 00:00:00 2001

> From: Alexandru Gagniuc <mr.nu...@gmail.com <javascript:>>

> Signed-off-by: Alexandru Gagniuc <mr.nu...@gmail.com <javascript:>>

[Michael Weiser]

Hi all,

On Sat, Aug 08, 2015 at 09:41:51PM +0200, Olliver Schinagl wrote:

> Alexandru sent this patch over a year and a half ago, and I believe several
> tree's have been carrying it since. We've been using this patch on an
> Olimex OLinuxIno Lime1 and Lime2 using the mmc-spi driver to access SD cards
> without problems. So bumping this and adding my

I also have a need for this addition since it makes an ENC28J60 SPI
ethernet controller work on the Cubieboard2. I've successfully tested it
with 4.6.3 where it still applies cleanly.

(There's is a very minor conflict when applying against current Linus
master (EINVAL was changed to EMSGSIZE). I can supply a rebased version
if so desired.)

What is keeping the patch from being merged (i.e. into mainline)?

Thanks!
Michael

> Tested-by: Olliver Schinagl <oli...@schinagl.nl>

Tested-by: Michael Weiser <michael...@gmx.de>

> Changes from v5as warned by checkpatch. No functional changes.
> * Added some newlines to make checkpatch happy. No functional changes.

> Changes from v4:
> * use min3() instead of two if statements in sun4i_spi_fill_fifo()
> * fix trivial whitespace issue in if statement in sun4i_spi_handler()
> * use consistent style in assigning 'reg' in the added functions.

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

> drivers/spi/spi-sun4i.c | 76 +++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 67 insertions(+), 9 deletions(-)
--

Thanks,
Michael

[Mark Brown]

On Thu, Jul 21, 2016 at 01:27:01PM +0200, Michael Weiser wrote:

> What is keeping the patch from being merged (i.e. into mainline)?

Someone needs to address whatever review comments there were on the last
version and submit it.

[Michael Weiser]

Hi Mark,

On Thu, Jul 21, 2016 at 05:31:53PM +0100, Mark Brown wrote:

> > What is keeping the patch from being merged (i.e. into mainline)?
> Someone needs to address whatever review comments there were on the last
> version and submit it.

That's my point: There don't seem to be any.

v6 was resend with fixes to checkpatch niggles on v5:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-August/363073.html

v5 addressed comments by Maxime on v4:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/244305.html
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/243995.html

I can't find any comments on v5 or v6.
--
Thanks,
Michael

[Olliver Schinagl]

As i recall, some claimed it was needed as we have dma now, but i think this patch still scratches the same itch ...

Sent from my Android device with K-9 Mail. Please excuse my brevity.

[Mark Brown]

On Thu, Jul 21, 2016 at 08:03:16PM +0200, Olliver Schinagl wrote:
> As i recall, some claimed it was needed as we have dma now, but i think this patch still scratches the same itch ...

Please don't top post, reply in line with needed context. This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns. Doing this makes your messages much
easier to read and reply to.

[Mark Brown]

On Thu, Jul 21, 2016 at 07:27:12PM +0200, Michael Weiser wrote:
> On Thu, Jul 21, 2016 at 05:31:53PM +0100, Mark Brown wrote:

> > > What is keeping the patch from being merged (i.e. into mainline)?
> > Someone needs to address whatever review comments there were on the last
> > version and submit it.

> That's my point: There don't seem to be any.

> v6 was resend with fixes to checkpatch niggles on v5:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-August/363073.html

I can't tell if that was even submitted, it's got a subject line of
"ARM: sun4i" so there's every chance that even if it was sent to me it
got deleted unread. Whatever was going on it needs to be submited.

[Alex Gagniuc]

And here we are again, over one year later, up to 4.9, and still
limited to 63 byte transfers.

A very smart person just brought this up to me today:

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

Alex

> Thanks
>
> Michal

[Michal Suchanek]

The patches are out there - eg here

https://github.com/hramrach/linux-sunxi/tree/sunxi-spi-dma-merged


I don't have time to haggle with maintainers which variant is
acceptable atm but you are free to use the thing and/or pick it up and
submit to linux.


Thanks

Michal