[U-Boot] [PATCH] DaVinci: Improve DaVinci SPI speed.

[Nick Thompson]

On 01/06/10 12:36, Delio Brignoli wrote:
> I have updated this patch based on the comments [1] by Wolfgang Denk and 
> removed unused variables.
> [1][http://lists.denx.de/pipermail/u-boot/2010-May/071728.html]
> 
> Reduce the number of reads per byte transferred on the BUF register from 2 to 1 and
> take advantage of the TX buffer in the SPI module. On LogicPD OMAP-L138 EVM, 
> SPI read throughput goes up from ~0.8Mbyte/s to ~1.3Mbyte/s. Tested with a 2Mbyte image file.
> Remove unused variables in the spi_xfer() function.
> 
> Signed-off-by: Delio Brignoli <dbrignoli at audioscience.com>
> Tested-by: Ben Gardiner <bengardiner at nanometrics.ca>

Sorry, I'm a bit late to the party on this.

I have an alternative patch that tries to be even quicker, but I
don't have the same platform as Delio, so can't compare like with
like.

This diff applies before Delio's patch. If it is any faster I am
prepared to create a proper patch. If nobody can test it for speed
I'll probably just drop it, since it produces a slightly bigger
executable and I don't know that it is actually any faster...

In essence, it splits up read and write operations to avoid testing
pointers in every loop iteration. It also unrolls the last iteration
so that it doesn't have to test for loop ending twice each time
round. Finally it avoids bit setting/clearing on each iteration when
the results would only turn out to be the same anyway.

Here's the diff:

diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c
index 60ba007..a90d2f4 100644
--- a/drivers/spi/davinci_spi.c
+++ b/drivers/spi/davinci_spi.c
@@ -126,16 +126,98 @@ void spi_release_bus(struct spi_slave *slave)
 	writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0);
 }
 
-int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
-		const void *dout, void *din, unsigned long flags)
+static inline u8 davinci_spi_read_data(struct davinci_spi_slave *ds, u32 data)
+{
+	unsigned int	buf_reg_val;
+	/* wait till TXFULL is deasserted */
+	while (readl(&ds->regs->buf) & SPIBUF_TXFULL_MASK)
+		;
+	writel(data, &ds->regs->dat1);
+
+	/* read the data - wait for data availability */
+	while ((buf_reg_val = readl(&ds->regs->buf)) & SPIBUF_RXEMPTY_MASK)
+		;
+	return buf_reg_val & 0xFF;
+}
+
+static int davinci_spi_read(struct spi_slave *slave, unsigned int len,
+			    u8 *rxp, unsigned long flags)
 {
 	struct davinci_spi_slave *ds = to_davinci_spi(slave);
-	unsigned int	len, data1_reg_val = readl(&ds->regs->dat1);
-	int		ret, i;
-	const u8	*txp = dout; /* dout can be NULL for read operation */
-	u8		*rxp = din;  /* din can be NULL for write operation */
+	unsigned int data1_reg_val = readl(&ds->regs->dat1);
+
+	/* do an empty read to clear the current contents */
+	(void)readl(&ds->regs->buf);
+
+	/* enable CS hold */
+	data1_reg_val |= ((1 << SPIDAT1_CSHOLD_SHIFT) |
+			(slave->cs << SPIDAT1_CSNR_SHIFT));
+	data1_reg_val &= ~0xFFFF;
+
+	/* keep writing and reading 1 byte until only 1 byte left to read */
+	while ((len--) > 1) {
+		*rxp++ = davinci_spi_read_data(ds, data1_reg_val);
+	}
 
-	ret = 0;
+	/*
+	 * clear CS hold when we reach the end.
+	 */
+	if (flags & SPI_XFER_END)
+		data1_reg_val &= ~(1 << SPIDAT1_CSHOLD_SHIFT);
+
+	*rxp = davinci_spi_read_data(ds, data1_reg_val);
+
+	return 0;
+}
+
+static inline void davinci_spi_write_data(struct davinci_spi_slave *ds, u32 data)
+{
+	/* wait till TXFULL is deasserted */
+	while (readl(&ds->regs->buf) & SPIBUF_TXFULL_MASK)
+		;
+	writel(data, &ds->regs->dat1);
+
+	/* wait for read data availability */
+	while (readl(&ds->regs->buf) & SPIBUF_RXEMPTY_MASK)
+		;
+}
+
+static int davinci_spi_write(struct spi_slave *slave, unsigned int len,
+		const u8 *txp, unsigned long flags)
+{
+	struct davinci_spi_slave *ds = to_davinci_spi(slave);
+	unsigned int data1_reg_val = readl(&ds->regs->dat1);
+
+	/* do an empty read to clear the current contents */
+	(void)readl(&ds->regs->buf);
+
+	/* enable CS hold */
+	data1_reg_val |= ((1 << SPIDAT1_CSHOLD_SHIFT) |
+			(slave->cs << SPIDAT1_CSNR_SHIFT));
+	data1_reg_val &= ~0xFFFF;
+
+	/* keep writing and reading 1 byte until only 1 byte left to write */
+	while ((len--) > 1) {
+		/* write the data */
+		davinci_spi_write_data(ds, data1_reg_val | *txp++);
+	}
+
+	/*
+	 * clear CS hold when we reach the end.
+	 */
+	if (flags & SPI_XFER_END)
+		data1_reg_val &= ~(1 << SPIDAT1_CSHOLD_SHIFT);
+
+	/* write the data */
+	davinci_spi_write_data(ds, data1_reg_val | *txp);
+
+	return 0;
+}
+
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
+		const void *dout, void *din, unsigned long flags)
+{
+	unsigned int len;
 
 	if (bitlen == 0)
 		/* Finish any previously submitted transfers */
@@ -155,53 +237,15 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
 
 	len = bitlen / 8;
 
-	/* do an empty read to clear the current contents */
-	readl(&ds->regs->buf);
-
-	/* keep writing and reading 1 byte until done */
-	for (i = 0; i < len; i++) {
-		/* wait till TXFULL is asserted */
-		while (readl(&ds->regs->buf) & SPIBUF_TXFULL_MASK);
-
-		/* write the data */
-		data1_reg_val &= ~0xFFFF;
-		if (txp) {
-			data1_reg_val |= *txp;
-			txp++;
-		}
-
-		/*
-		 * Write to DAT1 is required to keep the serial transfer going.
-		 * We just terminate when we reach the end.
-		 */
-		if ((i == (len - 1)) && (flags & SPI_XFER_END)) {
-			/* clear CS hold */
-			writel(data1_reg_val &
-				~(1 << SPIDAT1_CSHOLD_SHIFT), &ds->regs->dat1);
-		} else {
-			/* enable CS hold */
-			data1_reg_val |= ((1 << SPIDAT1_CSHOLD_SHIFT) |
-					(slave->cs << SPIDAT1_CSNR_SHIFT));
-			writel(data1_reg_val, &ds->regs->dat1);
-		}
-
-		/* read the data - wait for data availability */
-		while (readl(&ds->regs->buf) & SPIBUF_RXEMPTY_MASK);
-
-		if (rxp) {
-			*rxp = readl(&ds->regs->buf) & 0xFF;
-			rxp++;
-		} else {
-			/* simply drop the read character */
-			readl(&ds->regs->buf);
-		}
-	}
-	return 0;
+	if (din)
+		return davinci_spi_read(slave, len, din, flags);
+	else
+		return davinci_spi_write(slave, len, dout, flags);
 
 out:
 	if (flags & SPI_XFER_END) {
-		writel(data1_reg_val &
-			~(1 << SPIDAT1_CSHOLD_SHIFT), &ds->regs->dat1);
+		u8 dummy = 0;
+		davinci_spi_write(slave, 1, &dummy, flags);
 	}
 	return 0;
 }