From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Thompson Date: Wed, 23 Jun 2010 09:14:55 +0100 Subject: [U-Boot] [PATCH] Davinci: SPI performance enhancements In-Reply-To: <0554BEF07D437848AF01B9C9B5F0BC5D9EB78B58@dlee01.ent.ti.com> References: <4C1F3082.8070108@ge.com> <0554BEF07D437848AF01B9C9B5F0BC5D9EB78B58@dlee01.ent.ti.com> Message-ID: <4C21C27F.9080008@ge.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 22/06/10 16:27, Paulraj, Sandeep wrote: > >> The following restructuring and optimisations increase the SPI >> read performance from 1.3MiB/s (on da850) to 2.87MiB/s (on da830): >> >> Remove continual revaluation of driver state from the core of the >> copy loop. State can not change during the copy loop, so it is >> possible to move these evaluations to before the copy loop. >> >> Cost is more code space as loop variants are required for each set >> of possible configurations. The loops are simpler however, so the >> extra is only 128bytes on da830 with CONFIG_SPI_HALF_DUPLEX >> defined. >> >> Unrolling the first copy loop iteration allows the TX buffer to be >> pre-loaded reducing SPI clock starvation. >> >> Unrolling the last copy loop iteration removes testing for the >> final loop iteration every time round the loop. >> >> Using the RX buffer empty flag as a transfer throttle allows the >> assumption that it is always safe to write to the TX buffer, so >> polling of TX buffer full flag can be removed. >> >> Signed-off-by: Nick Thompson >> --- >> da850 and da830 are similar devices. The SPI module is common to >> both, but da850 uses DDR and da830 uses SDRAM. The EVM's might >> not actually be comparable, but they appear to be at least similar. >> >> The speed was tested with a 8MiB transfer from SPI FLASH using: >> >> sf read 0xc0008000 0 0x800000 >> >> drivers/spi/davinci_spi.c | 195 +++++++++++++++++++++++++++++----------- > > Applied to u-boot-ti/next Sandeep, You're too quick for me :-) I was going to do a v2 patch to cover a couple of Delio's points. They are not critical issues though, so I think we can let the pull request go through and I will submit a new patch once it hits u-boot/next: On 21/06/10 19:38, Delio Brignoli wrote: >> + if (!dout) >> + return davinci_spi_read(slave, len, din, flags); >> + else if (!din) >> + return davinci_spi_write(slave, len, dout, flags); >> +#ifndef CONFIG_SPI_HALF_DUPLEX >> + else >> + return davinci_spi_read_write(slave, len, din, dout, flags); >> +#endif > > I think there should always be an else branch at the end even if > CONFIG_SPI_HALF_DUPLEX is not defined. Something like: > > #else > flags |= SPI_XFER_END; > #endif > > to terminate the transfer instead of failing silently. > In fact it should signal the error condition somehow, but > I do not know enough about u-boot to provide an advice on this. Its a non issue if the driver is called correctly in the first place, but we can't be sure of that. Nick.