diff mbox

[3/3] mtd:spi-nor:fsl-quadspi:Add fast-read mode support

Message ID 1454067669-35274-4-git-send-email-B56489@freescale.com
State Superseded
Headers show

Commit Message

Yunhui Cui Jan. 29, 2016, 11:41 a.m. UTC 
The qspi driver add generic fast-read mode for different
flash venders, including Micron family. Also add some special
operations for Micron flash read/write in spi-nor.c.

Signed-off-by: Yunhui Cui <B56489@freescale.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 27 +++++++++++++++++++++------
 drivers/mtd/spi-nor/spi-nor.c     |  6 +++++-
 2 files changed, 26 insertions(+), 7 deletions(-)

Comments

Cyrille Pitchen Jan. 29, 2016, 2:51 p.m. UTC | #1 
Hi Yunhui,

Le 29/01/2016 12:41, Yunhui Cui a écrit :
> The qspi driver add generic fast-read mode for different
> flash venders, including Micron family. Also add some special
> operations for Micron flash read/write in spi-nor.c.
> 
> Signed-off-by: Yunhui Cui <B56489@freescale.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 27 +++++++++++++++++++++------
>  drivers/mtd/spi-nor/spi-nor.c     |  6 +++++-
>  2 files changed, 26 insertions(+), 7 deletions(-)
> 
[...]
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index ed0c19c..79a025c 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -763,7 +763,8 @@ static const struct flash_info spi_nor_ids[] = {
>  	{ "n25q064",     INFO(0x20ba17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
>  	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
>  	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SPI_NOR_QUAD_READ) },
> -	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SPI_NOR_QUAD_READ) },
> +	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SPI_NOR_FAST |
> +			SPI_NOR_QUAD_READ) },
>  	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>  	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
>  	{ "n25q512ax3",  INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
This modification looks wrong.

First SPI_NOR_FAST is part of enum read_mode (from spi-nor.h) and is not
intended to be used as a flag, unlike SPI_NOR_QUAD_READ (defined in spi-nor.c
just below the flags field of struct flash_info).

Also, creating a new flag to choose between the Read and the Fast Read commands
is not needed: see my last comment below.


> @@ -1233,6 +1234,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>  
>  	if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>  	    JEDEC_MFR(info) == SNOR_MFR_INTEL ||
> +	    JEDEC_MFR(info) == SNOR_MFR_MICRON ||
>  	    JEDEC_MFR(info) == SNOR_MFR_SST) {
>  		write_enable(nor);
>  		write_sr(nor, 0);
This update is not related with the support of fast-read mode, it deals with
the write protection. Hence it should be moved into a dedicated patch.

Also be careful as some bits of the Status Register are non-volatile on Micron
memories. This modification may change their values but I guess you only want
to clear the write enable/disable (7) bit.


> @@ -1317,6 +1319,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>  		nor->flash_read = SPI_NOR_QUAD;
>  	} else if (mode == SPI_NOR_DUAL && info->flags & SPI_NOR_DUAL_READ) {
>  		nor->flash_read = SPI_NOR_DUAL;
> +	} else if (mode == SPI_NOR_FAST && info->flags & SPI_NOR_FAST) {
> +		nor->flash_read = SPI_NOR_FAST;
>  	}
>  
>  	/* Default commands */
> 
The spi-nor framework already checks the "m25p,fast-read" DT property to choose
between the Read (0x03) and Fast Read (0x0b) command.

So the fsl-quadspi.c driver should rely on this existing property to choose
which of SPI_NOR_FAST or SPI_NOR_QUAD is used as the mode argument of
spi_nor_scan(). As long as mode is neither SPI_NOR_QUAD nor SPI_NOR_DUAL, the
choice of the read op code is done according to whether the "m25p,fast-read" DT
property is set or not.


Best regards,

Cyrille
Yunhui Cui Feb. 1, 2016, 11:23 a.m. UTC | #2 
Hi Cyrille,

Thanks for your suggestions very much, I'll resend version 2 patch set.

Best Regards
Yunhui

-----Original Message-----
From: Cyrille Pitchen [mailto:cyrille.pitchen@atmel.com] 
Sent: Friday, January 29, 2016 10:51 PM
To: Yunhui Cui; dwmw2@infradead.org; computersforpeace@gmail.com; han.xu@freescale.com
Cc: linux-mtd@lists.infradead.org; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Yao Yuan
Subject: Re: [PATCH 3/3] mtd:spi-nor:fsl-quadspi:Add fast-read mode support

Hi Yunhui,

Le 29/01/2016 12:41, Yunhui Cui a écrit :
> The qspi driver add generic fast-read mode for different flash 
> venders, including Micron family. Also add some special operations for 
> Micron flash read/write in spi-nor.c.
> 
> Signed-off-by: Yunhui Cui <B56489@freescale.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 27 +++++++++++++++++++++------
>  drivers/mtd/spi-nor/spi-nor.c     |  6 +++++-
>  2 files changed, 26 insertions(+), 7 deletions(-)
> 
[...]
> diff --git a/drivers/mtd/spi-nor/spi-nor.c 
> b/drivers/mtd/spi-nor/spi-nor.c index ed0c19c..79a025c 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -763,7 +763,8 @@ static const struct flash_info spi_nor_ids[] = {
>  	{ "n25q064",     INFO(0x20ba17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
>  	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
>  	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SPI_NOR_QUAD_READ) },
> -	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SPI_NOR_QUAD_READ) },
> +	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SPI_NOR_FAST |
> +			SPI_NOR_QUAD_READ) },
>  	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>  	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
>  	{ "n25q512ax3",  INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K | 
> USE_FSR | SPI_NOR_QUAD_READ) },
This modification looks wrong.

First SPI_NOR_FAST is part of enum read_mode (from spi-nor.h) and is not intended to be used as a flag, unlike SPI_NOR_QUAD_READ (defined in spi-nor.c just below the flags field of struct flash_info).

Also, creating a new flag to choose between the Read and the Fast Read commands is not needed: see my last comment below.


> @@ -1233,6 +1234,7 @@ int spi_nor_scan(struct spi_nor *nor, const char 
> *name, enum read_mode mode)
>  
>  	if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>  	    JEDEC_MFR(info) == SNOR_MFR_INTEL ||
> +	    JEDEC_MFR(info) == SNOR_MFR_MICRON ||
>  	    JEDEC_MFR(info) == SNOR_MFR_SST) {
>  		write_enable(nor);
>  		write_sr(nor, 0);
This update is not related with the support of fast-read mode, it deals with the write protection. Hence it should be moved into a dedicated patch.

Also be careful as some bits of the Status Register are non-volatile on Micron memories. This modification may change their values but I guess you only want to clear the write enable/disable (7) bit.


> @@ -1317,6 +1319,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>  		nor->flash_read = SPI_NOR_QUAD;
>  	} else if (mode == SPI_NOR_DUAL && info->flags & SPI_NOR_DUAL_READ) {
>  		nor->flash_read = SPI_NOR_DUAL;
> +	} else if (mode == SPI_NOR_FAST && info->flags & SPI_NOR_FAST) {
> +		nor->flash_read = SPI_NOR_FAST;
>  	}
>  
>  	/* Default commands */
> 
The spi-nor framework already checks the "m25p,fast-read" DT property to choose between the Read (0x03) and Fast Read (0x0b) command.

So the fsl-quadspi.c driver should rely on this existing property to choose which of SPI_NOR_FAST or SPI_NOR_QUAD is used as the mode argument of spi_nor_scan(). As long as mode is neither SPI_NOR_QUAD nor SPI_NOR_DUAL, the choice of the read op code is done according to whether the "m25p,fast-read" DT property is set or not.


Best regards,

Cyrille
diff mbox

Patch

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 9861290..fc4451d 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -389,11 +389,21 @@  static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	/* Read */
 	lut_base = SEQID_READ * 4;
 
-	qspi_writel(q, LUT0(CMD, PAD1, read_op) | LUT1(ADDR, PAD1, addrlen),
-			base + QUADSPI_LUT(lut_base));
-	qspi_writel(q, LUT0(DUMMY, PAD1, read_dm) |
-		    LUT1(FSL_READ, PAD4, rxfifo),
-			base + QUADSPI_LUT(lut_base + 1));
+	if (nor->flash_read == SPI_NOR_FAST) {
+		qspi_writel(q, LUT0(CMD, PAD1, read_op) |
+			    LUT1(ADDR, PAD1, addrlen),
+				base + QUADSPI_LUT(lut_base));
+		qspi_writel(q,  LUT0(DUMMY, PAD1, read_dm) |
+			    LUT1(FSL_READ, PAD1, rxfifo),
+				base + QUADSPI_LUT(lut_base + 1));
+	} else if (nor->flash_read == SPI_NOR_QUAD) {
+		qspi_writel(q, LUT0(CMD, PAD1, read_op) |
+			    LUT1(ADDR, PAD1, addrlen),
+				base + QUADSPI_LUT(lut_base));
+		qspi_writel(q, LUT0(DUMMY, PAD1, read_dm) |
+			    LUT1(FSL_READ, PAD4, rxfifo),
+				base + QUADSPI_LUT(lut_base + 1));
+	}
 
 	/* Write enable */
 	lut_base = SEQID_WREN * 4;
@@ -468,6 +478,7 @@  static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
 {
 	switch (cmd) {
 	case SPINOR_OP_READ_1_1_4:
+	case SPINOR_OP_READ_FAST:
 		return SEQID_READ;
 	case SPINOR_OP_WREN:
 		return SEQID_WREN;
@@ -963,6 +974,7 @@  static int fsl_qspi_probe(struct platform_device *pdev)
 	struct spi_nor *nor;
 	struct mtd_info *mtd;
 	int ret, i = 0;
+	enum read_mode mode = SPI_NOR_QUAD;
 
 	q = devm_kzalloc(dev, sizeof(*q), GFP_KERNEL);
 	if (!q)
@@ -1064,7 +1076,10 @@  static int fsl_qspi_probe(struct platform_device *pdev)
 		/* set the chip address for READID */
 		fsl_qspi_set_base_addr(q, nor);
 
-		ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
+		ret = of_property_read_bool(np, "fast-read");
+		mode = (ret) ? SPI_NOR_FAST : SPI_NOR_QUAD;
+
+		ret = spi_nor_scan(nor, NULL, mode);
 		if (ret)
 			goto mutex_failed;
 
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index ed0c19c..79a025c 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -763,7 +763,8 @@  static const struct flash_info spi_nor_ids[] = {
 	{ "n25q064",     INFO(0x20ba17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
 	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
 	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SPI_NOR_QUAD_READ) },
-	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SPI_NOR_QUAD_READ) },
+	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SPI_NOR_FAST |
+			SPI_NOR_QUAD_READ) },
 	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
 	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
 	{ "n25q512ax3",  INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
@@ -1233,6 +1234,7 @@  int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 
 	if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
 	    JEDEC_MFR(info) == SNOR_MFR_INTEL ||
+	    JEDEC_MFR(info) == SNOR_MFR_MICRON ||
 	    JEDEC_MFR(info) == SNOR_MFR_SST) {
 		write_enable(nor);
 		write_sr(nor, 0);
@@ -1317,6 +1319,8 @@  int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 		nor->flash_read = SPI_NOR_QUAD;
 	} else if (mode == SPI_NOR_DUAL && info->flags & SPI_NOR_DUAL_READ) {
 		nor->flash_read = SPI_NOR_DUAL;
+	} else if (mode == SPI_NOR_FAST && info->flags & SPI_NOR_FAST) {
+		nor->flash_read = SPI_NOR_FAST;
 	}
 
 	/* Default commands */