[U-Boot] [PATCH 0/2] spi: ti_qspi: Increase write speed

[U-Boot] [PATCH 0/2] spi: ti_qspi: Increase write speed

[Vignesh R]

This patch series tries to improve QSPI write speed by writing 16 bytes
at once whenever possible. Also remove unnecessary 100us delay for
AM437x.

Tested on AM437x SK, DRA74 and DRA72 EVM.

Vignesh R (2):
  spi: ti_qspi: use 128 bit transfer mode when writing to flash
  spi: ti_qspi: Remove unnecessary udelay for AM437x

 drivers/spi/ti_qspi.c | 44 +++++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 13 deletions(-)

-- 
2.9.3

[U-Boot] [PATCH 1/2] spi: ti_qspi: use 128 bit transfer mode when writing to flash

[Vignesh R]

TI QSPI has four 32 bit data registers which can be used to transfer 16
bytes of data at once. The register group QSPI_SPI_DATA_REG_3,
QSPI_SPI_DATA_REG_2, QSPI_SPI_DATA_REG_1 and QSPI_SPI_DATA_REG is
treated as a single 128-bit word for shifting data in and out. The bit
at QSPI_SPI_DATA_REG_3[31] position is the first bit to be shifted out
in case of 128 bit transfer mode. Therefore the first byte to be written
to flash should be at QSPI_SPI_DATA_REG_3[31-25] position.
Instead of writing 1 byte at a time when interacting with SPI NOR flash,
make use of all the four registers so that 16 bytes can be transferred
in one go.

With this patch, the flash write speed increases from ~250KBs/ to
~650KB/s on DRA74 EVM.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/spi/ti_qspi.c | 37 +++++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
index bb72cb03ec24..fe2a280cc7ae 100644
--- a/drivers/spi/ti_qspi.c
+++ b/drivers/spi/ti_qspi.c
@@ -23,6 +23,8 @@ DECLARE_GLOBAL_DATA_PTR;
 #define QSPI_TIMEOUT                    2000000
 #define QSPI_FCLK			192000000
 #define QSPI_DRA7XX_FCLK                76800000
+#define QSPI_WLEN_MAX_BITS		128
+#define QSPI_WLEN_MAX_BYTES		(QSPI_WLEN_MAX_BITS >> 3)
 /* clock control */
 #define QSPI_CLK_EN                     BIT(31)
 #define QSPI_CLK_DIV_MAX                0xffff
@@ -230,13 +232,33 @@ static int __ti_qspi_xfer(struct ti_qspi_priv *priv, unsigned int bitlen,
 #ifdef CONFIG_AM43XX
 	udelay(100);
 #endif
-	while (words--) {
+	while (words) {
+		u8 xfer_len = 0;
+
 		if (txp) {
-			debug("tx cmd %08x dc %08x data %02x\n",
-			      priv->cmd | QSPI_WR_SNGL, priv->dc, *txp);
-			writel(*txp++, &priv->base->data);
-			writel(priv->cmd | QSPI_WR_SNGL,
-			       &priv->base->cmd);
+			u32 cmd = priv->cmd;
+
+			if (words >= QSPI_WLEN_MAX_BYTES) {
+				u32 *txbuf = (u32 *)txp;
+				u32 data;
+
+				data = cpu_to_be32(*txbuf++);
+				writel(data, &priv->base->data3);
+				data = cpu_to_be32(*txbuf++);
+				writel(data, &priv->base->data2);
+				data = cpu_to_be32(*txbuf++);
+				writel(data, &priv->base->data1);
+				data = cpu_to_be32(*txbuf++);
+				writel(data, &priv->base->data);
+				cmd |= QSPI_WLEN(QSPI_WLEN_MAX_BITS);
+				xfer_len = QSPI_WLEN_MAX_BYTES;
+			} else {
+				writeb(*txp, &priv->base->data);
+				xfer_len = 1;
+			}
+			debug("tx cmd %08x dc %08x\n",
+			      cmd | QSPI_WR_SNGL, priv->dc);
+			writel(cmd | QSPI_WR_SNGL, &priv->base->cmd);
 			status = readl(&priv->base->status);
 			timeout = QSPI_TIMEOUT;
 			while ((status & QSPI_WC_BUSY) != QSPI_XFER_DONE) {
@@ -246,6 +268,7 @@ static int __ti_qspi_xfer(struct ti_qspi_priv *priv, unsigned int bitlen,
 				}
 				status = readl(&priv->base->status);
 			}
+			txp += xfer_len;
 			debug("tx done, status %08x\n", status);
 		}
 		if (rxp) {
@@ -262,9 +285,11 @@ static int __ti_qspi_xfer(struct ti_qspi_priv *priv, unsigned int bitlen,
 				status = readl(&priv->base->status);
 			}
 			*rxp++ = readl(&priv->base->data);
+			xfer_len = 1;
 			debug("rx done, status %08x, read %02x\n",
 			      status, *(rxp-1));
 		}
+		words -= xfer_len;
 	}
 
 	/* Terminate frame */
-- 
2.9.3

[U-Boot] [PATCH 2/2] spi: ti_qspi: Remove unnecessary udelay for AM437x

[Vignesh R]

This udelay() was added as an HACK and is no longer required. All
read/write/erase operations work fine even without this delay. Hence,
remove the udelay() call.

Tested read/write/erase operation on AM437x SK. Also tested QSPI Boot.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/spi/ti_qspi.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
index fe2a280cc7ae..1c215a0f89e2 100644
--- a/drivers/spi/ti_qspi.c
+++ b/drivers/spi/ti_qspi.c
@@ -225,13 +225,6 @@ static int __ti_qspi_xfer(struct ti_qspi_priv *priv, unsigned int bitlen,
 		priv->cmd |= QSPI_3_PIN;
 	priv->cmd |= 0xfff;
 
-/* FIXME: This delay is required for successfull
- * completion of read/write/erase. Once its root
- * caused, it will be remove from the driver.
- */
-#ifdef CONFIG_AM43XX
-	udelay(100);
-#endif
 	while (words) {
 		u8 xfer_len = 0;
 
-- 
2.9.3

[U-Boot] [PATCH 1/2] spi: ti_qspi: use 128 bit transfer mode when writing to flash

[Tom Rini]

On Thu, Sep 01, 2016 at 01:24:39PM +0530, Vignesh R wrote:

> TI QSPI has four 32 bit data registers which can be used to transfer 16
> bytes of data at once. The register group QSPI_SPI_DATA_REG_3,
> QSPI_SPI_DATA_REG_2, QSPI_SPI_DATA_REG_1 and QSPI_SPI_DATA_REG is
> treated as a single 128-bit word for shifting data in and out. The bit
> at QSPI_SPI_DATA_REG_3[31] position is the first bit to be shifted out
> in case of 128 bit transfer mode. Therefore the first byte to be written
> to flash should be at QSPI_SPI_DATA_REG_3[31-25] position.
> Instead of writing 1 byte at a time when interacting with SPI NOR flash,
> make use of all the four registers so that 16 bytes can be transferred
> in one go.
> 
> With this patch, the flash write speed increases from ~250KBs/ to
> ~650KB/s on DRA74 EVM.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160902/5180472f/attachment.sig>

[U-Boot] [PATCH 2/2] spi: ti_qspi: Remove unnecessary udelay for AM437x

[Tom Rini]

On Thu, Sep 01, 2016 at 01:24:40PM +0530, Vignesh R wrote:

> This udelay() was added as an HACK and is no longer required. All
> read/write/erase operations work fine even without this delay. Hence,
> remove the udelay() call.
> 
> Tested read/write/erase operation on AM437x SK. Also tested QSPI Boot.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160902/afb44dd1/attachment.sig>

[U-Boot] [PATCH 2/2] spi: ti_qspi: Remove unnecessary udelay for AM437x

[Jagan Teki]

On Fri, Sep 2, 2016 at 8:24 PM, Tom Rini <trini@konsulko.com> wrote:
> On Thu, Sep 01, 2016 at 01:24:40PM +0530, Vignesh R wrote:
>
>> This udelay() was added as an HACK and is no longer required. All
>> read/write/erase operations work fine even without this delay. Hence,
>> remove the udelay() call.
>>
>> Tested read/write/erase operation on AM437x SK. Also tested QSPI Boot.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Reviewed-by: Jagan Teki <jteki@openedev.com>

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH 1/2] spi: ti_qspi: use 128 bit transfer mode when writing to flash

[Jagan Teki]

On Thu, Sep 1, 2016 at 1:24 PM, Vignesh R <vigneshr@ti.com> wrote:
> TI QSPI has four 32 bit data registers which can be used to transfer 16
> bytes of data at once. The register group QSPI_SPI_DATA_REG_3,
> QSPI_SPI_DATA_REG_2, QSPI_SPI_DATA_REG_1 and QSPI_SPI_DATA_REG is
> treated as a single 128-bit word for shifting data in and out. The bit
> at QSPI_SPI_DATA_REG_3[31] position is the first bit to be shifted out
> in case of 128 bit transfer mode. Therefore the first byte to be written
> to flash should be at QSPI_SPI_DATA_REG_3[31-25] position.
> Instead of writing 1 byte at a time when interacting with SPI NOR flash,
> make use of all the four registers so that 16 bytes can be transferred
> in one go.
>
> With this patch, the flash write speed increases from ~250KBs/ to
> ~650KB/s on DRA74 EVM.
>
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>  drivers/spi/ti_qspi.c | 37 +++++++++++++++++++++++++++++++------
>  1 file changed, 31 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
> index bb72cb03ec24..fe2a280cc7ae 100644
> --- a/drivers/spi/ti_qspi.c
> +++ b/drivers/spi/ti_qspi.c
> @@ -23,6 +23,8 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define QSPI_TIMEOUT                    2000000
>  #define QSPI_FCLK                      192000000
>  #define QSPI_DRA7XX_FCLK                76800000
> +#define QSPI_WLEN_MAX_BITS             128
> +#define QSPI_WLEN_MAX_BYTES            (QSPI_WLEN_MAX_BITS >> 3)
>  /* clock control */
>  #define QSPI_CLK_EN                     BIT(31)
>  #define QSPI_CLK_DIV_MAX                0xffff
> @@ -230,13 +232,33 @@ static int __ti_qspi_xfer(struct ti_qspi_priv *priv, unsigned int bitlen,
>  #ifdef CONFIG_AM43XX
>         udelay(100);
>  #endif
> -       while (words--) {
> +       while (words) {
> +               u8 xfer_len = 0;
> +
>                 if (txp) {
> -                       debug("tx cmd %08x dc %08x data %02x\n",
> -                             priv->cmd | QSPI_WR_SNGL, priv->dc, *txp);
> -                       writel(*txp++, &priv->base->data);
> -                       writel(priv->cmd | QSPI_WR_SNGL,
> -                              &priv->base->cmd);
> +                       u32 cmd = priv->cmd;

Don't we require cmd mask for WLEN_MAX_BITS?

thanks!
-- 
Jagan.

[U-Boot] [PATCH 1/2] spi: ti_qspi: use 128 bit transfer mode when writing to flash

[Vignesh R]

Hi,

On Sunday 04 September 2016 07:51 PM, Jagan Teki wrote:
> On Thu, Sep 1, 2016 at 1:24 PM, Vignesh R <vigneshr@ti.com> wrote:
[...]
>> @@ -23,6 +23,8 @@ DECLARE_GLOBAL_DATA_PTR;
>>  #define QSPI_TIMEOUT                    2000000
>>  #define QSPI_FCLK                      192000000
>>  #define QSPI_DRA7XX_FCLK                76800000
>> +#define QSPI_WLEN_MAX_BITS             128
>> +#define QSPI_WLEN_MAX_BYTES            (QSPI_WLEN_MAX_BITS >> 3)
>>  /* clock control */
>>  #define QSPI_CLK_EN                     BIT(31)
>>  #define QSPI_CLK_DIV_MAX                0xffff
>> @@ -230,13 +232,33 @@ static int __ti_qspi_xfer(struct ti_qspi_priv *priv, unsigned int bitlen,
>>  #ifdef CONFIG_AM43XX
>>         udelay(100);
>>  #endif
>> -       while (words--) {
>> +       while (words) {
>> +               u8 xfer_len = 0;
>> +
>>                 if (txp) {
>> -                       debug("tx cmd %08x dc %08x data %02x\n",
>> -                             priv->cmd | QSPI_WR_SNGL, priv->dc, *txp);
>> -                       writel(*txp++, &priv->base->data);
>> -                       writel(priv->cmd | QSPI_WR_SNGL,
>> -                              &priv->base->cmd);
>> +                       u32 cmd = priv->cmd;
> 
> Don't we require cmd mask for WLEN_MAX_BITS?
> 

Its not quite necessary. priv->cmd always has WLEN field set to 0x7
(8bit) where as for 128bit write WLEN is to be set to 0x7F. Therefore
the logic

+				cmd |= QSPI_WLEN(QSPI_WLEN_MAX_BITS);

will still work w/o needing to clear WLEN field using a mask. But
anyways, to avoid confusion, I will add a mask for WLEN_MAX_BITS.

-- 
Regards
Vignesh