fsl spi questions & patch

fsl spi questions & patch

[Frans Meulenbroeks]

[-- Attachment #1.1: Type: text/plain, Size: 1298 bytes --]

Hi,

I've been playing with spi on mpc8313e and have some things on
spi-fsl-spi.c:

Is QE useful on 8313?
I've tried it (using cpu-qe in my dts file) and see in the boot log that it
is used, but I do not really see any effect when it comes to performance or
cpu usage.

Furthermore:
In the fsl_spi_cpu_irq there is a line:
        /* Clear the events */
        mpc8xxx_spi_write_reg(&reg_base->event, events);

Is this really useful? The 8313 book says NE is cleared upon reading and NF
is cleared upon writing.
(this might apply to fsl_spi_cpm_irq too, I do not have info on cpm.

Next, I noticed some spacing between two spi words being sent. It seems the
transmit buffer is not filled when possible, but only when a word is
received (and the previous word is transmitted). By modifying the code
somewhat I was able to roughly double the effective transfer rate in my
test setup (8 Mhz spi clock, 32 bit transfers). Attached is my changed
code. Feedback on it is appreciated.

This patch also eliminated the spinning until TX is done. (actually I am
not sure if this will happen, I would expect NE and NF to be raised roughly
at the same time.

Thanks for any feedback!

Frans Meulenbroeks

PS:it would probably be nice if in board setup one could also set the
(default) value of bits-per-word.

[-- Attachment #1.2: Type: text/html, Size: 1396 bytes --]

[-- Attachment #2: 0001-spi-fsl-spi.c-use-NF-interrupt-instead-of-NE.patch --]
[-- Type: application/octet-stream, Size: 2799 bytes --]

From fe783ac0fe3d2ea4814d282f436c9670f892bda2 Mon Sep 17 00:00:00 2001
From: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
Date: Wed, 28 Nov 2012 09:42:51 +0100
Subject: [PATCH] spi-fsl-spi.c: use NF interrupt instead of NE

When using NE (not empty) interrupt we get the first interrupt
when the first word is received (and the first one send as
sending and receiving happens simultaneously and sychronously).
In the case of multi-word transmits there is a short delay to
reload the transmit buffer (on my test system 1.5 us).
However it is possible to load the next byte before as there
is a one word hold register.

By using NF (not full) to trigger the interrupt we fill the
register as soon as possible, improving the transmission speed.
(about doubling it with an 8Mhz SPI clock and 32 bit transfers
on MPC8313e)

I also moved the disabling of the interrupt into the _irq
handler (before calling complete() ) instead of after
wait_for_completion().
On my system the NF interrupt kept being raised; this fixed that.

Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
---
 drivers/spi/spi-fsl-spi.c |   27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c
index 6a62934..1f81ccd 100644
--- a/drivers/spi/spi-fsl-spi.c
+++ b/drivers/spi/spi-fsl-spi.c
@@ -410,7 +410,7 @@ static int fsl_spi_cpu_bufs(struct mpc8xxx_spi *mspi,
 	mspi->count = len;
 
 	/* enable rx ints */
-	mpc8xxx_spi_write_reg(&reg_base->mask, SPIM_NE);
+	mpc8xxx_spi_write_reg(&reg_base->mask, SPIM_NF);
 
 	/* transmit word */
 	word = mspi->get_tx(mspi);
@@ -460,9 +460,6 @@ static int fsl_spi_bufs(struct spi_device *spi, struct spi_transfer *t,
 
 	wait_for_completion(&mpc8xxx_spi->done);
 
-	/* disable rx ints */
-	mpc8xxx_spi_write_reg(&reg_base->mask, 0);
-
 	if (mpc8xxx_spi->flags & SPI_CPM_MODE)
 		fsl_spi_cpm_bufs_complete(mpc8xxx_spi);
 
@@ -604,23 +601,19 @@ static void fsl_spi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events)
 			mspi->get_rx(rx_data, mspi);
 	}
 
-	if ((events & SPIE_NF) == 0)
-		/* spin until TX is done */
-		while (((events =
-			mpc8xxx_spi_read_reg(&reg_base->event)) &
-						SPIE_NF) == 0)
-			cpu_relax();
-
 	/* Clear the events */
 	mpc8xxx_spi_write_reg(&reg_base->event, events);
 
-	mspi->count -= 1;
-	if (mspi->count) {
-		u32 word = mspi->get_tx(mspi);
+	if ((events & SPIE_NF) && (mspi->count > 1))
+	{	mspi->count -= 1;
+		if (mspi->count) {
+			u32 word = mspi->get_tx(mspi);
 
-		mpc8xxx_spi_write_reg(&reg_base->transmit, word);
-	} else {
-		complete(&mspi->done);
+			mpc8xxx_spi_write_reg(&reg_base->transmit, word);
+		} else {
+			mpc8xxx_spi_write_reg(&reg_base->mask, 0); /* disable interrupts */
+			complete(&mspi->done);
+		}
 	}
 }
 
-- 
1.7.9.5

Re: fsl spi questions & patch

[Scott Wood]

On 11/28/2012 03:39:22 AM, Frans Meulenbroeks wrote:
> Hi,
>=20
> I've been playing with spi on mpc8313e and have some things on
> spi-fsl-spi.c:
>=20
> Is QE useful on 8313?
> I've tried it (using cpu-qe in my dts file) and see in the boot log =20
> that it
> is used, but I do not really see any effect when it comes to =20
> performance or
> cpu usage.

8313 does not have a QE.  The "effect" when you do have a QE and use it =20
is that you can talk to the peripherals that are connected to the QE.  =20
The effect when you modify the device tree to say the board has =20
hardware that it doesn't have is generally bad.

-Scott=