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

[Paulraj, Sandeep]

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

It is late. Pull request already sent to Wolfgang
> 
> 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.

Compare it on your platform. I believe you have the OMAP L137.
And post the results.

> 
> 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;
>  }
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot