spi: ti_qspi: Increase write speed

PATCH Superseded Vignesh R 3 participants 8 messages Sep 1, 2016 – Sep 6, 2016 SPI

Share & tools

Vignesh R

author

2016-09-01 07:54 UTC #0 lore raw

cover spi: ti_qspi: Increase write speed

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(-)

Signature · 1 line

2.9.3

Vignesh R

author

2016-09-01 07:54 UTC #1 lore raw

PATCH 1/2 spi: ti_qspi: use 128 bit transfer mode when writing to flash

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 <[email protected]>

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

drivers/spi/ti_qspi.c +31 -6

index bb72cb03ec24..fe2a280cc7ae 100644
@@ -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 */

Signature · 1 line

2.9.3

Tom Rini

2016-09-02 14:54 UTC #2 lore raw

PATCH 1/2 spi: ti_qspi: use 128 bit transfer mode when writing to flash

Quoted 11 lines · 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.

Quoted 4 lines

With this patch, the flash write speed increases from ~250KBs/ to ~650KB/s on DRA74 EVM.
Signed-off-by Vignesh R <[email protected]>

Reviewed-by Tom Rini <[email protected]>

Signature · 8 lines

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>

Jagan Teki

2016-09-04 14:21 UTC #3 lore raw

PATCH 1/2 spi: ti_qspi: use 128 bit transfer mode when writing to flash

In reply to Vignesh R

Quoted 37 lines · 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 <[email protected]>
---
 drivers/spi/ti_qspi.c | 37 +++++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)
drivers/spi/ti_qspi.c +2 -1
index bb72cb03ec24..fe2a280cc7ae 100644
@@ -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--) {

drivers/spi/ti_qspi.c static int __ti_qspi_xfer(struct ti_qspi_priv *priv, unsigned int bitlen, · line 239

Quoted 10 lines

drivers/spi/ti_qspi.c +4 -5

+       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!

Signature · 1 line

Jagan.

Vignesh R

author

2016-09-06 05:09 UTC #4 lore raw

PATCH 1/2 spi: ti_qspi: use 128 bit transfer mode when writing to flash

Hi,

Quoted 1 line · Jagan Teki

On Thu, Sep 1, 2016 at 1:24 PM, Vignesh R <[email protected]> wrote:

[...]

Quoted 18 lines

drivers/spi/ti_qspi.c +5 -1

@@ -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) {

Quoted 9 lines

drivers/spi/ti_qspi.c static int __ti_qspi_xfer(struct ti_qspi_priv *priv, unsigned int bitlen, · line 239

Quoted 6 lines

drivers/spi/ti_qspi.c +1 -5

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

Signature · 2 lines

Regards
Vignesh

Vignesh R

author

2016-09-01 07:54 UTC #5 lore raw

PATCH 2/2 spi: ti_qspi: Remove unnecessary udelay for AM437x

In reply to 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 <[email protected]>

---
 drivers/spi/ti_qspi.c | 7 -------
 1 file changed, 7 deletions(-)

drivers/spi/ti_qspi.c +0 -7

index fe2a280cc7ae..1c215a0f89e2 100644
@@ -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;

Signature · 1 line

2.9.3

Tom Rini

2016-09-02 14:54 UTC #6 lore raw

PATCH 2/2 spi: ti_qspi: Remove unnecessary udelay for AM437x

Quoted 7 lines · 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 <[email protected]>

Reviewed-by Tom Rini <[email protected]>

Signature · 8 lines

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>

Jagan Teki

2016-09-04 13:55 UTC #7 lore raw

PATCH 2/2 spi: ti_qspi: Remove unnecessary udelay for AM437x

Quoted 11 lines · Tom Rini

Quoted 7 lines · 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 <[email protected]>
Reviewed-by Tom Rini <[email protected]>

Reviewed-by Jagan Teki <[email protected]>

thanks!

Signature · 4 lines

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