[U-Boot] [PATCH V2] spi: exynos: Minimise access to SPI FIFO level

[U-Boot] [PATCH V2] spi: exynos: Minimise access to SPI FIFO level

[Rajeshwari Shinde]

Accessing SPI registers is slow, but access to the FIFO level register
in particular seems to be extraordinarily expensive (I measure up to
600ns). Perhaps it is required to synchronise with the SPI byte output
logic which might run at 1/8th of the 40MHz SPI speed (just a guess).

Reduce access to this register by filling up and emptying FIFOs
more completely, rather than just one word each time around the inner
loop.

Since the rxfifo value will now likely be much greater that what we read
before we fill the txfifo, we only fill the txfifo halfway. This is
because if the txfifo is empty, but the rxfifo has data in it, then writing
too much data to the txfifo may overflow the rxfifo as data arrives.

This speeds up SPI flash reading from about 1MB/s to about 2MB/s on snow.

Based on "[PATCH 0/2 V3] exynos: Support a delay after deactivate for SPI"

Signed-off-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com>
---
Changes in V2:
	- Rebased on "[PATCH 0/2 V5] spi: Enable SPI_PREAMBLE Mode" 
 drivers/spi/exynos_spi.c |   27 +++++++++++++++------------
 1 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c
index deb32bd..bcca3d6 100644
--- a/drivers/spi/exynos_spi.c
+++ b/drivers/spi/exynos_spi.c
@@ -259,24 +259,27 @@ static int spi_rx_tx(struct exynos_spi_slave *spi_slave, int todo,
 
 		/* Keep the fifos full/empty. */
 		spi_get_fifo_levels(regs, &rx_lvl, &tx_lvl);
-		if (tx_lvl < spi_slave->fifo_size && out_bytes) {
+		while (tx_lvl < spi_slave->fifo_size/2 && out_bytes) {
 			temp = txp ? *txp++ : 0xff;
 			writel(temp, &regs->tx_data);
 			out_bytes--;
+			tx_lvl++;
 		}
 		if (rx_lvl > 0) {
-			temp = readl(&regs->rx_data);
-			if (spi_slave->skip_preamble) {
-				if (temp == SPI_PREAMBLE_END_BYTE) {
-					spi_slave->skip_preamble = 0;
-					stopping = 0;
+			while (rx_lvl > 0) {
+				temp = readl(&regs->rx_data);
+				if (spi_slave->skip_preamble) {
+					if (temp == SPI_PREAMBLE_END_BYTE) {
+						spi_slave->skip_preamble = 0;
+						stopping = 0;
+					}
+				} else {
+					if (rxp || stopping)
+						*rxp++ = temp;
+					in_bytes--;
 				}
-			} else {
-				if (rxp || stopping)
-					*rxp++ = temp;
-				in_bytes--;
-			}
-			toread--;
+				toread--;
+				rx_lvl--;
 		} else if (!toread) {
 			/*
 			 * We have run out of input data, but haven't read
-- 
1.7.4.4

[U-Boot] [PATCH V2] spi: exynos: Minimise access to SPI FIFO level

[Jagan Teki]

On 30-05-2013 10:57, Rajeshwari Shinde wrote:
> Accessing SPI registers is slow, but access to the FIFO level register
> in particular seems to be extraordinarily expensive (I measure up to
> 600ns). Perhaps it is required to synchronise with the SPI byte output
> logic which might run at 1/8th of the 40MHz SPI speed (just a guess).
>
> Reduce access to this register by filling up and emptying FIFOs
> more completely, rather than just one word each time around the inner
> loop.
>
> Since the rxfifo value will now likely be much greater that what we read
> before we fill the txfifo, we only fill the txfifo halfway. This is
> because if the txfifo is empty, but the rxfifo has data in it, then writing
> too much data to the txfifo may overflow the rxfifo as data arrives.
>
> This speeds up SPI flash reading from about 1MB/s to about 2MB/s on snow.
>
> Based on "[PATCH 0/2 V3] exynos: Support a delay after deactivate for SPI"

Please use the format i suggested on earlier patch
http://patchwork.ozlabs.org/patch/247451/

Thanks,
Jagan.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com>
> ---
> Changes in V2:
> 	- Rebased on "[PATCH 0/2 V5] spi: Enable SPI_PREAMBLE Mode"
>   drivers/spi/exynos_spi.c |   27 +++++++++++++++------------
>   1 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c
> index deb32bd..bcca3d6 100644
> --- a/drivers/spi/exynos_spi.c
> +++ b/drivers/spi/exynos_spi.c
> @@ -259,24 +259,27 @@ static int spi_rx_tx(struct exynos_spi_slave *spi_slave, int todo,
>
>   		/* Keep the fifos full/empty. */
>   		spi_get_fifo_levels(regs, &rx_lvl, &tx_lvl);
> -		if (tx_lvl < spi_slave->fifo_size && out_bytes) {
> +		while (tx_lvl < spi_slave->fifo_size/2 && out_bytes) {
>   			temp = txp ? *txp++ : 0xff;
>   			writel(temp, &regs->tx_data);
>   			out_bytes--;
> +			tx_lvl++;
>   		}
>   		if (rx_lvl > 0) {
> -			temp = readl(&regs->rx_data);
> -			if (spi_slave->skip_preamble) {
> -				if (temp == SPI_PREAMBLE_END_BYTE) {
> -					spi_slave->skip_preamble = 0;
> -					stopping = 0;
> +			while (rx_lvl > 0) {
> +				temp = readl(&regs->rx_data);
> +				if (spi_slave->skip_preamble) {
> +					if (temp == SPI_PREAMBLE_END_BYTE) {
> +						spi_slave->skip_preamble = 0;
> +						stopping = 0;
> +					}
> +				} else {
> +					if (rxp || stopping)
> +						*rxp++ = temp;
> +					in_bytes--;
>   				}
> -			} else {
> -				if (rxp || stopping)
> -					*rxp++ = temp;
> -				in_bytes--;
> -			}
> -			toread--;
> +				toread--;
> +				rx_lvl--;
>   		} else if (!toread) {
>   			/*
>   			 * We have run out of input data, but haven't read
>

[U-Boot] [PATCH V2] spi: exynos: Minimise access to SPI FIFO level

[Jagan Teki]

Hi,

On Thu, Jun 13, 2013 at 1:29 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> On 30-05-2013 10:57, Rajeshwari Shinde wrote:
>>
>> Accessing SPI registers is slow, but access to the FIFO level register
>> in particular seems to be extraordinarily expensive (I measure up to
>> 600ns). Perhaps it is required to synchronise with the SPI byte output
>> logic which might run at 1/8th of the 40MHz SPI speed (just a guess).
>>
>> Reduce access to this register by filling up and emptying FIFOs
>> more completely, rather than just one word each time around the inner
>> loop.
>>
>> Since the rxfifo value will now likely be much greater that what we read
>> before we fill the txfifo, we only fill the txfifo halfway. This is
>> because if the txfifo is empty, but the rxfifo has data in it, then
>> writing
>> too much data to the txfifo may overflow the rxfifo as data arrives.
>>
>> This speeds up SPI flash reading from about 1MB/s to about 2MB/s on snow.
>>
>> Based on "[PATCH 0/2 V3] exynos: Support a delay after deactivate for SPI"
>
>
> Please use the format i suggested on earlier patch
> http://patchwork.ozlabs.org/patch/247451/
>
> Thanks,
> Jagan.
>
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com>
>> ---
>> Changes in V2:
>>         - Rebased on "[PATCH 0/2 V5] spi: Enable SPI_PREAMBLE Mode"
>>   drivers/spi/exynos_spi.c |   27 +++++++++++++++------------
>>   1 files changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c
>> index deb32bd..bcca3d6 100644
>> --- a/drivers/spi/exynos_spi.c
>> +++ b/drivers/spi/exynos_spi.c
>> @@ -259,24 +259,27 @@ static int spi_rx_tx(struct exynos_spi_slave
>> *spi_slave, int todo,
>>
>>                 /* Keep the fifos full/empty. */
>>                 spi_get_fifo_levels(regs, &rx_lvl, &tx_lvl);
>> -               if (tx_lvl < spi_slave->fifo_size && out_bytes) {
>> +               while (tx_lvl < spi_slave->fifo_size/2 && out_bytes) {
>>                         temp = txp ? *txp++ : 0xff;
>>                         writel(temp, &regs->tx_data);
>>                         out_bytes--;
>> +                       tx_lvl++;
>>                 }
>>                 if (rx_lvl > 0) {
>> -                       temp = readl(&regs->rx_data);
>> -                       if (spi_slave->skip_preamble) {
>> -                               if (temp == SPI_PREAMBLE_END_BYTE) {
>> -                                       spi_slave->skip_preamble = 0;
>> -                                       stopping = 0;
>> +                       while (rx_lvl > 0) {
>> +                               temp = readl(&regs->rx_data);
>> +                               if (spi_slave->skip_preamble) {
>> +                                       if (temp == SPI_PREAMBLE_END_BYTE)
>> {
>> +                                               spi_slave->skip_preamble =
>> 0;
>> +                                               stopping = 0;
>> +                                       }
>> +                               } else {
>> +                                       if (rxp || stopping)
>> +                                               *rxp++ = temp;
>> +                                       in_bytes--;
>>                                 }
>> -                       } else {
>> -                               if (rxp || stopping)
>> -                                       *rxp++ = temp;
>> -                               in_bytes--;
>> -                       }
>> -                       toread--;
>> +                               toread--;
>> +                               rx_lvl--;
>>                 } else if (!toread) {
>>                         /*
>>                          * We have run out of input data, but haven't read
>>
>

Any plan to send this patch?
Please send by fixing few comments on last review.

Also see the patches in:
http://patchwork.ozlabs.org/patch/247457/
http://patchwork.ozlabs.org/patch/247461/

Please let me know for any concerns.

--
Thanks,
Jagan.