[PATCH v3 1/9] mtd:fsl-quadspi:use the property fields of SPI-NOR

[PATCH v3 1/9] mtd:fsl-quadspi:use the property fields of SPI-NOR

[Yunhui Cui]

We can get the read/write/erase opcode from the spi nor framework
directly. This patch uses the information stored in the SPI-NOR to
remove the hardcode in the fsl_qspi_init_lut().

Signed-off-by: Yunhui Cui <B56489@freescale.com>
Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 40 ++++++++++++---------------------------
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 5c82e4e..5ad6402 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -373,9 +373,13 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	void __iomem *base = q->iobase;
 	int rxfifo = q->devtype_data->rxfifo;
 	u32 lut_base;
-	u8 cmd, addrlen, dummy;
 	int i;
 
+	struct spi_nor *nor = &q->nor[0];
+	u8 addrlen = (nor->addr_width == 3) ? ADDR24BIT : ADDR32BIT;
+	u8 read_op = nor->read_opcode;
+	u8 read_dm = nor->read_dummy;
+
 	fsl_qspi_unlock_lut(q);
 
 	/* Clear all the LUT table */
@@ -385,20 +389,10 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	/* Quad Read */
 	lut_base = SEQID_QUAD_READ * 4;
 
-	if (q->nor_size <= SZ_16M) {
-		cmd = SPINOR_OP_READ_1_1_4;
-		addrlen = ADDR24BIT;
-		dummy = 8;
-	} else {
-		/* use the 4-byte address */
-		cmd = SPINOR_OP_READ_1_1_4;
-		addrlen = ADDR32BIT;
-		dummy = 8;
-	}
-
-	qspi_writel(q, LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
+	qspi_writel(q, LUT0(CMD, PAD1, read_op) | LUT1(ADDR, PAD1, addrlen),
 			base + QUADSPI_LUT(lut_base));
-	qspi_writel(q, LUT0(DUMMY, PAD1, dummy) | LUT1(FSL_READ, PAD4, rxfifo),
+	qspi_writel(q, LUT0(DUMMY, PAD1, read_dm) |
+		    LUT1(FSL_READ, PAD4, rxfifo),
 			base + QUADSPI_LUT(lut_base + 1));
 
 	/* Write enable */
@@ -409,16 +403,8 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	/* Page Program */
 	lut_base = SEQID_PP * 4;
 
-	if (q->nor_size <= SZ_16M) {
-		cmd = SPINOR_OP_PP;
-		addrlen = ADDR24BIT;
-	} else {
-		/* use the 4-byte address */
-		cmd = SPINOR_OP_PP;
-		addrlen = ADDR32BIT;
-	}
-
-	qspi_writel(q, LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
+	qspi_writel(q, LUT0(CMD, PAD1, nor->program_opcode) |
+		    LUT1(ADDR, PAD1, addrlen),
 			base + QUADSPI_LUT(lut_base));
 	qspi_writel(q, LUT0(FSL_WRITE, PAD1, 0),
 			base + QUADSPI_LUT(lut_base + 1));
@@ -432,10 +418,8 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	/* Erase a sector */
 	lut_base = SEQID_SE * 4;
 
-	cmd = q->nor[0].erase_opcode;
-	addrlen = q->nor_size <= SZ_16M ? ADDR24BIT : ADDR32BIT;
-
-	qspi_writel(q, LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
+	qspi_writel(q, LUT0(CMD, PAD1, nor->erase_opcode) |
+		    LUT1(ADDR, PAD1, addrlen),
 			base + QUADSPI_LUT(lut_base));
 
 	/* Erase the whole chip */
-- 
2.1.0.27.g96db324

[PATCH v3 2/9] mtd: fsl-quadspi: Rename SEQID_QUAD_READ to SEQID_READ

[Yunhui Cui]

There are some read modes for flash, such as NORMAL, FAST,
QUAD, DDR QUAD. These modes will use the identical lut table base
So rename SEQID_QUAD_READ to SEQID_READ.

Signed-off-by: Yunhui Cui <B56489@freescale.com>
Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 5ad6402..21c77e8 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -193,7 +193,7 @@
 #define QUADSPI_LUT_NUM		64
 
 /* SEQID -- we can have 16 seqids at most. */
-#define SEQID_QUAD_READ		0
+#define SEQID_READ		0
 #define SEQID_WREN		1
 #define SEQID_WRDI		2
 #define SEQID_RDSR		3
@@ -386,8 +386,8 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	for (i = 0; i < QUADSPI_LUT_NUM; i++)
 		qspi_writel(q, 0, base + QUADSPI_LUT_BASE + i * 4);
 
-	/* Quad Read */
-	lut_base = SEQID_QUAD_READ * 4;
+	/* Read */
+	lut_base = SEQID_READ * 4;
 
 	qspi_writel(q, LUT0(CMD, PAD1, read_op) | LUT1(ADDR, PAD1, addrlen),
 			base + QUADSPI_LUT(lut_base));
@@ -468,7 +468,7 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
 {
 	switch (cmd) {
 	case SPINOR_OP_READ_1_1_4:
-		return SEQID_QUAD_READ;
+		return SEQID_READ;
 	case SPINOR_OP_WREN:
 		return SEQID_WREN;
 	case SPINOR_OP_WRDI:
-- 
2.1.0.27.g96db324

Re: [PATCH v3 2/9] mtd: fsl-quadspi: Rename SEQID_QUAD_READ to SEQID_READ

[Han Xu]

On Thu, Aug 18, 2016 at 2:37 AM, Yunhui Cui <B56489@freescale.com> wrote:
> There are some read modes for flash, such as NORMAL, FAST,
> QUAD, DDR QUAD. These modes will use the identical lut table base
> So rename SEQID_QUAD_READ to SEQID_READ.
>
> Signed-off-by: Yunhui Cui <B56489@freescale.com>
> Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 5ad6402..21c77e8 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -193,7 +193,7 @@
>  #define QUADSPI_LUT_NUM                64
>
>  /* SEQID -- we can have 16 seqids at most. */
> -#define SEQID_QUAD_READ                0
> +#define SEQID_READ             0
>  #define SEQID_WREN             1
>  #define SEQID_WRDI             2
>  #define SEQID_RDSR             3
> @@ -386,8 +386,8 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>         for (i = 0; i < QUADSPI_LUT_NUM; i++)
>                 qspi_writel(q, 0, base + QUADSPI_LUT_BASE + i * 4);
>
> -       /* Quad Read */
> -       lut_base = SEQID_QUAD_READ * 4;
> +       /* Read */
> +       lut_base = SEQID_READ * 4;
>
>         qspi_writel(q, LUT0(CMD, PAD1, read_op) | LUT1(ADDR, PAD1, addrlen),
>                         base + QUADSPI_LUT(lut_base));
> @@ -468,7 +468,7 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
>  {
>         switch (cmd) {
>         case SPINOR_OP_READ_1_1_4:
> -               return SEQID_QUAD_READ;
> +               return SEQID_READ;
>         case SPINOR_OP_WREN:
>                 return SEQID_WREN;
>         case SPINOR_OP_WRDI:
> --
> 2.1.0.27.g96db324
>
>
Acked-by: Han xu <han.xu@nxp.com>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

Re: [PATCH v3 2/9] mtd: fsl-quadspi: Rename SEQID_QUAD_READ to SEQID_READ

[Cyrille Pitchen]

Le 18/08/2016 à 09:37, Yunhui Cui a écrit :
> There are some read modes for flash, such as NORMAL, FAST,
> QUAD, DDR QUAD. These modes will use the identical lut table base
> So rename SEQID_QUAD_READ to SEQID_READ.
> 
> Signed-off-by: Yunhui Cui <B56489@freescale.com>
> Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>

Applied to the spi-nor tree.

Thanks!
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 5ad6402..21c77e8 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -193,7 +193,7 @@
>  #define QUADSPI_LUT_NUM		64
>  
>  /* SEQID -- we can have 16 seqids at most. */
> -#define SEQID_QUAD_READ		0
> +#define SEQID_READ		0
>  #define SEQID_WREN		1
>  #define SEQID_WRDI		2
>  #define SEQID_RDSR		3
> @@ -386,8 +386,8 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  	for (i = 0; i < QUADSPI_LUT_NUM; i++)
>  		qspi_writel(q, 0, base + QUADSPI_LUT_BASE + i * 4);
>  
> -	/* Quad Read */
> -	lut_base = SEQID_QUAD_READ * 4;
> +	/* Read */
> +	lut_base = SEQID_READ * 4;
>  
>  	qspi_writel(q, LUT0(CMD, PAD1, read_op) | LUT1(ADDR, PAD1, addrlen),
>  			base + QUADSPI_LUT(lut_base));
> @@ -468,7 +468,7 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
>  {
>  	switch (cmd) {
>  	case SPINOR_OP_READ_1_1_4:
> -		return SEQID_QUAD_READ;
> +		return SEQID_READ;
>  	case SPINOR_OP_WREN:
>  		return SEQID_WREN;
>  	case SPINOR_OP_WRDI:
> 

[PATCH v3 3/9] mtd: spi-nor: fsl-quadspi: add fast-read mode support

[Yunhui Cui]

From: Yunhui Cui <yunhui.cui@nxp.com>

The qspi driver add generic fast-read mode for different
flash venders. There are some different board flash work on
different mode, such fast-read, quad-mode.

Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 21c77e8..b0a74b8 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;
-- 
2.1.0.27.g96db324

Re: [PATCH v3 3/9] mtd: spi-nor: fsl-quadspi: add fast-read mode support

[Han Xu]

On Thu, Aug 18, 2016 at 2:37 AM, Yunhui Cui <B56489@freescale.com> wrote:
> From: Yunhui Cui <yunhui.cui@nxp.com>
>
> The qspi driver add generic fast-read mode for different
> flash venders. There are some different board flash work on
> different mode, such fast-read, quad-mode.
>
> Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 21c77e8..b0a74b8 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;
> --
> 2.1.0.27.g96db324
>
>
Acked-by: Han xu <han.xu@nxp.com>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

[PATCH v3 4/9] mtd: spi-nor: fsl-quadspi: extend support for some special requerment.

[Yunhui Cui]

From: Yunhui Cui <yunhui.cui@nxp.com>

Add extra info in LUT table to support some special requerments.
Spansion S25FS-S family flash need some special operations.

Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 44 +++++++++++++++++++++++++++++++++++++--
 include/linux/mtd/spi-nor.h       |  4 ++++
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index b0a74b8..2521370 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -205,6 +205,9 @@
 #define SEQID_RDCR		9
 #define SEQID_EN4B		10
 #define SEQID_BRWR		11
+#define SEQID_RDAR		12
+#define SEQID_WRAR		13
+
 
 #define QUADSPI_MIN_IOMAP SZ_4M
 
@@ -470,6 +473,28 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_BRWR),
 			base + QUADSPI_LUT(lut_base));
 
+	/*
+	 * Read any device register.
+	 * Used for Spansion S25FS-S family flash only.
+	 */
+	lut_base = SEQID_RDAR * 4;
+	qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_SPANSION_RDAR) |
+			LUT1(ADDR, PAD1, ADDR24BIT),
+			base + QUADSPI_LUT(lut_base));
+	qspi_writel(q, LUT0(DUMMY, PAD1, 8) | LUT1(FSL_READ, PAD1, 1),
+			base + QUADSPI_LUT(lut_base + 1));
+
+	/*
+	 * Write any device register.
+	 * Used for Spansion S25FS-S family flash only.
+	 */
+	lut_base = SEQID_WRAR * 4;
+	qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_SPANSION_WRAR) |
+			LUT1(ADDR, PAD1, ADDR24BIT),
+			base + QUADSPI_LUT(lut_base));
+	qspi_writel(q, LUT0(FSL_WRITE, PAD1, 1),
+			base + QUADSPI_LUT(lut_base + 1));
+
 	fsl_qspi_lock_lut(q);
 }
 
@@ -477,9 +502,15 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
 {
 	switch (cmd) {
+	case SPINOR_OP_READ4_1_1_4:
 	case SPINOR_OP_READ_1_1_4:
 	case SPINOR_OP_READ_FAST:
+	case SPINOR_OP_READ4_FAST:
 		return SEQID_READ;
+	case SPINOR_OP_SPANSION_RDAR:
+		return SEQID_RDAR;
+	case SPINOR_OP_SPANSION_WRAR:
+		return SEQID_WRAR;
 	case SPINOR_OP_WREN:
 		return SEQID_WREN;
 	case SPINOR_OP_WRDI:
@@ -491,6 +522,7 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
 	case SPINOR_OP_CHIP_ERASE:
 		return SEQID_CHIP_ERASE;
 	case SPINOR_OP_PP:
+	case SPINOR_OP_PP_4B:
 		return SEQID_PP;
 	case SPINOR_OP_RDID:
 		return SEQID_RDID;
@@ -830,8 +862,12 @@ static int fsl_qspi_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 {
 	int ret;
 	struct fsl_qspi *q = nor->priv;
+	u32 to = 0;
+
+	if (opcode == SPINOR_OP_SPANSION_RDAR)
+		memcpy(&to, nor->cmd_buf, 4);
 
-	ret = fsl_qspi_runcmd(q, opcode, 0, len);
+	ret = fsl_qspi_runcmd(q, opcode, to, len);
 	if (ret)
 		return ret;
 
@@ -843,9 +879,13 @@ static int fsl_qspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 {
 	struct fsl_qspi *q = nor->priv;
 	int ret;
+	u32 to = 0;
+
+	if (opcode == SPINOR_OP_SPANSION_WRAR)
+		memcpy(&to, nor->cmd_buf, 4);
 
 	if (!buf) {
-		ret = fsl_qspi_runcmd(q, opcode, 0, 1);
+		ret = fsl_qspi_runcmd(q, opcode, to, 1);
 		if (ret)
 			return ret;
 
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index c425c7b..db3fe42 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -75,6 +75,10 @@
 /* Used for Spansion flashes only. */
 #define SPINOR_OP_BRWR		0x17	/* Bank register write */
 
+/* Used for Spansion S25FS-S family flash only. */
+#define SPINOR_OP_SPANSION_RDAR	0x65	/* Read any device register */
+#define SPINOR_OP_SPANSION_WRAR	0x71	/* Write any device register */
+
 /* Used for Micron flashes only. */
 #define SPINOR_OP_RD_EVCR      0x65    /* Read EVCR register */
 #define SPINOR_OP_WD_EVCR      0x61    /* Write EVCR register */
-- 
2.1.0.27.g96db324

Re: [PATCH v3 4/9] mtd: spi-nor: fsl-quadspi: extend support for some special requerment.

[Han Xu]

On Thu, Aug 18, 2016 at 2:37 AM, Yunhui Cui <B56489@freescale.com> wrote:
> From: Yunhui Cui <yunhui.cui@nxp.com>
>
> Add extra info in LUT table to support some special requerments.
> Spansion S25FS-S family flash need some special operations.
>
> Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 44 +++++++++++++++++++++++++++++++++++++--
>  include/linux/mtd/spi-nor.h       |  4 ++++
>  2 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index b0a74b8..2521370 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -205,6 +205,9 @@
>  #define SEQID_RDCR             9
>  #define SEQID_EN4B             10
>  #define SEQID_BRWR             11
> +#define SEQID_RDAR             12
> +#define SEQID_WRAR             13
> +
>
>  #define QUADSPI_MIN_IOMAP SZ_4M
>
> @@ -470,6 +473,28 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>         qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_BRWR),
>                         base + QUADSPI_LUT(lut_base));
>
> +       /*
> +        * Read any device register.
> +        * Used for Spansion S25FS-S family flash only.
> +        */
> +       lut_base = SEQID_RDAR * 4;
> +       qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_SPANSION_RDAR) |
> +                       LUT1(ADDR, PAD1, ADDR24BIT),
> +                       base + QUADSPI_LUT(lut_base));
> +       qspi_writel(q, LUT0(DUMMY, PAD1, 8) | LUT1(FSL_READ, PAD1, 1),
> +                       base + QUADSPI_LUT(lut_base + 1));
> +
> +       /*
> +        * Write any device register.
> +        * Used for Spansion S25FS-S family flash only.
> +        */
> +       lut_base = SEQID_WRAR * 4;
> +       qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_SPANSION_WRAR) |
> +                       LUT1(ADDR, PAD1, ADDR24BIT),
> +                       base + QUADSPI_LUT(lut_base));
> +       qspi_writel(q, LUT0(FSL_WRITE, PAD1, 1),
> +                       base + QUADSPI_LUT(lut_base + 1));
> +
>         fsl_qspi_lock_lut(q);
>  }
>
> @@ -477,9 +502,15 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
>  {
>         switch (cmd) {
> +       case SPINOR_OP_READ4_1_1_4:
>         case SPINOR_OP_READ_1_1_4:
>         case SPINOR_OP_READ_FAST:
> +       case SPINOR_OP_READ4_FAST:
>                 return SEQID_READ;
> +       case SPINOR_OP_SPANSION_RDAR:
> +               return SEQID_RDAR;
> +       case SPINOR_OP_SPANSION_WRAR:
> +               return SEQID_WRAR;
>         case SPINOR_OP_WREN:
>                 return SEQID_WREN;
>         case SPINOR_OP_WRDI:
> @@ -491,6 +522,7 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
>         case SPINOR_OP_CHIP_ERASE:
>                 return SEQID_CHIP_ERASE;
>         case SPINOR_OP_PP:
> +       case SPINOR_OP_PP_4B:
>                 return SEQID_PP;
>         case SPINOR_OP_RDID:
>                 return SEQID_RDID;
> @@ -830,8 +862,12 @@ static int fsl_qspi_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
>  {
>         int ret;
>         struct fsl_qspi *q = nor->priv;
> +       u32 to = 0;
> +
> +       if (opcode == SPINOR_OP_SPANSION_RDAR)
> +               memcpy(&to, nor->cmd_buf, 4);
>
> -       ret = fsl_qspi_runcmd(q, opcode, 0, len);
> +       ret = fsl_qspi_runcmd(q, opcode, to, len);
>         if (ret)
>                 return ret;
>
> @@ -843,9 +879,13 @@ static int fsl_qspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
>  {
>         struct fsl_qspi *q = nor->priv;
>         int ret;
> +       u32 to = 0;
> +
> +       if (opcode == SPINOR_OP_SPANSION_WRAR)
> +               memcpy(&to, nor->cmd_buf, 4);
>
>         if (!buf) {
> -               ret = fsl_qspi_runcmd(q, opcode, 0, 1);
> +               ret = fsl_qspi_runcmd(q, opcode, to, 1);
>                 if (ret)
>                         return ret;
>
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index c425c7b..db3fe42 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -75,6 +75,10 @@
>  /* Used for Spansion flashes only. */
>  #define SPINOR_OP_BRWR         0x17    /* Bank register write */
>
> +/* Used for Spansion S25FS-S family flash only. */
> +#define SPINOR_OP_SPANSION_RDAR        0x65    /* Read any device register */
> +#define SPINOR_OP_SPANSION_WRAR        0x71    /* Write any device register */
> +
>  /* Used for Micron flashes only. */
>  #define SPINOR_OP_RD_EVCR      0x65    /* Read EVCR register */
>  #define SPINOR_OP_WD_EVCR      0x61    /* Write EVCR register */
> --
> 2.1.0.27.g96db324
>
>
Acked-by: Han xu <han.xu@nxp.com>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

[PATCH v3 5/9] mtd: spi-nor: fsl-quadspi:Support qspi for ls2080a

[Yunhui Cui]

There is a hardware feature that qspi_amba_base is added
internally by SOC design on ls2080a. So as to software, the driver
need support to the feature.

Signed-off-by: Yunhui Cui <B56489@freescale.com>
Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 2521370..57eed3c 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -41,6 +41,8 @@
 #define QUADSPI_QUIRK_TKT253890		(1 << 2)
 /* Controller cannot wake up from wait mode, TKT245618 */
 #define QUADSPI_QUIRK_TKT245618         (1 << 3)
+/* QSPI_AMBA_BASE is internally added by SOC design */
+#define QUADSPI_AMBA_BASE_INTERNAL	(0x10000)
 
 /* The registers */
 #define QUADSPI_MCR			0x00
@@ -217,6 +219,7 @@ enum fsl_qspi_devtype {
 	FSL_QUADSPI_IMX7D,
 	FSL_QUADSPI_IMX6UL,
 	FSL_QUADSPI_LS1021A,
+	FSL_QUADSPI_LS2080A,
 };
 
 struct fsl_qspi_devtype_data {
@@ -270,6 +273,14 @@ static struct fsl_qspi_devtype_data ls1021a_data = {
 	.driver_data = 0,
 };
 
+static struct fsl_qspi_devtype_data ls2080a_data = {
+	.devtype = FSL_QUADSPI_LS2080A,
+	.rxfifo = 128,
+	.txfifo = 64,
+	.ahb_buf_size = 1024,
+	.driver_data = QUADSPI_AMBA_BASE_INTERNAL,
+};
+
 #define FSL_QSPI_MAX_CHIP	4
 struct fsl_qspi {
 	struct spi_nor nor[FSL_QSPI_MAX_CHIP];
@@ -312,6 +323,11 @@ static inline int needs_wakeup_wait_mode(struct fsl_qspi *q)
 	return q->devtype_data->driver_data & QUADSPI_QUIRK_TKT245618;
 }
 
+static inline int has_added_amba_base_internal(struct fsl_qspi *q)
+{
+	return q->devtype_data->driver_data & QUADSPI_AMBA_BASE_INTERNAL;
+}
+
 /*
  * R/W functions for big- or little-endian registers:
  * The qSPI controller's endian is independent of the CPU core's endian.
@@ -558,8 +574,11 @@ fsl_qspi_runcmd(struct fsl_qspi *q, u8 cmd, unsigned int addr, int len)
 	/* save the reg */
 	reg = qspi_readl(q, base + QUADSPI_MCR);
 
-	qspi_writel(q, q->memmap_phy + q->chip_base_addr + addr,
-			base + QUADSPI_SFAR);
+	if (has_added_amba_base_internal(q))
+		qspi_writel(q, q->chip_base_addr + addr, base + QUADSPI_SFAR);
+	else
+		qspi_writel(q, q->memmap_phy + q->chip_base_addr + addr,
+			    base + QUADSPI_SFAR);
 	qspi_writel(q, QUADSPI_RBCT_WMRK_MASK | QUADSPI_RBCT_RXBRD_USEIPS,
 			base + QUADSPI_RBCT);
 	qspi_writel(q, reg | QUADSPI_MCR_CLR_RXF_MASK, base + QUADSPI_MCR);
@@ -849,6 +868,7 @@ static const struct of_device_id fsl_qspi_dt_ids[] = {
 	{ .compatible = "fsl,imx7d-qspi", .data = (void *)&imx7d_data, },
 	{ .compatible = "fsl,imx6ul-qspi", .data = (void *)&imx6ul_data, },
 	{ .compatible = "fsl,ls1021a-qspi", .data = (void *)&ls1021a_data, },
+	{ .compatible = "fsl,ls2080a-qspi", .data = (void *)&ls2080a_data, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, fsl_qspi_dt_ids);
-- 
2.1.0.27.g96db324

Re: [PATCH v3 5/9] mtd: spi-nor: fsl-quadspi:Support qspi for ls2080a

[Han Xu]

On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui <B56489@freescale.com> wrote:
> There is a hardware feature that qspi_amba_base is added
> internally by SOC design on ls2080a. So as to software, the driver
> need support to the feature.
>
> Signed-off-by: Yunhui Cui <B56489@freescale.com>
> Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 2521370..57eed3c 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -41,6 +41,8 @@
>  #define QUADSPI_QUIRK_TKT253890                (1 << 2)
>  /* Controller cannot wake up from wait mode, TKT245618 */
>  #define QUADSPI_QUIRK_TKT245618         (1 << 3)
> +/* QSPI_AMBA_BASE is internally added by SOC design */
> +#define QUADSPI_AMBA_BASE_INTERNAL     (0x10000)
>
>  /* The registers */
>  #define QUADSPI_MCR                    0x00
> @@ -217,6 +219,7 @@ enum fsl_qspi_devtype {
>         FSL_QUADSPI_IMX7D,
>         FSL_QUADSPI_IMX6UL,
>         FSL_QUADSPI_LS1021A,
> +       FSL_QUADSPI_LS2080A,
>  };
>
>  struct fsl_qspi_devtype_data {
> @@ -270,6 +273,14 @@ static struct fsl_qspi_devtype_data ls1021a_data = {
>         .driver_data = 0,
>  };
>
> +static struct fsl_qspi_devtype_data ls2080a_data = {
> +       .devtype = FSL_QUADSPI_LS2080A,
> +       .rxfifo = 128,
> +       .txfifo = 64,
> +       .ahb_buf_size = 1024,
> +       .driver_data = QUADSPI_AMBA_BASE_INTERNAL,
> +};
> +
>  #define FSL_QSPI_MAX_CHIP      4
>  struct fsl_qspi {
>         struct spi_nor nor[FSL_QSPI_MAX_CHIP];
> @@ -312,6 +323,11 @@ static inline int needs_wakeup_wait_mode(struct fsl_qspi *q)
>         return q->devtype_data->driver_data & QUADSPI_QUIRK_TKT245618;
>  }
>
> +static inline int has_added_amba_base_internal(struct fsl_qspi *q)
> +{
> +       return q->devtype_data->driver_data & QUADSPI_AMBA_BASE_INTERNAL;
> +}
> +
>  /*
>   * R/W functions for big- or little-endian registers:
>   * The qSPI controller's endian is independent of the CPU core's endian.
> @@ -558,8 +574,11 @@ fsl_qspi_runcmd(struct fsl_qspi *q, u8 cmd, unsigned int addr, int len)
>         /* save the reg */
>         reg = qspi_readl(q, base + QUADSPI_MCR);
>
> -       qspi_writel(q, q->memmap_phy + q->chip_base_addr + addr,
> -                       base + QUADSPI_SFAR);
> +       if (has_added_amba_base_internal(q))
> +               qspi_writel(q, q->chip_base_addr + addr, base + QUADSPI_SFAR);
> +       else
> +               qspi_writel(q, q->memmap_phy + q->chip_base_addr + addr,
> +                           base + QUADSPI_SFAR);
>         qspi_writel(q, QUADSPI_RBCT_WMRK_MASK | QUADSPI_RBCT_RXBRD_USEIPS,
>                         base + QUADSPI_RBCT);
>         qspi_writel(q, reg | QUADSPI_MCR_CLR_RXF_MASK, base + QUADSPI_MCR);
> @@ -849,6 +868,7 @@ static const struct of_device_id fsl_qspi_dt_ids[] = {
>         { .compatible = "fsl,imx7d-qspi", .data = (void *)&imx7d_data, },
>         { .compatible = "fsl,imx6ul-qspi", .data = (void *)&imx6ul_data, },
>         { .compatible = "fsl,ls1021a-qspi", .data = (void *)&ls1021a_data, },
> +       { .compatible = "fsl,ls2080a-qspi", .data = (void *)&ls2080a_data, },
>         { /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, fsl_qspi_dt_ids);
> --
> 2.1.0.27.g96db324
>
>
Acked-by: Han xu <han.xu@nxp.com>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

[PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Yunhui Cui]

From: Yunhui Cui <yunhui.cui@nxp.com>

With the physical sectors combination, S25FS-S family flash
requires some special operations for read/write functions.

Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 56 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index d0fc165..495d0bb 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -39,6 +39,10 @@
 
 #define SPI_NOR_MAX_ID_LEN	6
 #define SPI_NOR_MAX_ADDR_WIDTH	4
+/* Added for S25FS-S family flash */
+#define SPINOR_CONFIG_REG3_OFFSET      0x800004
+#define CR3V_4KB_ERASE_UNABLE  0x8
+#define SPINOR_S25FS_FAMILY_EXT_JEDEC	0x81
 
 struct flash_info {
 	char		*name;
@@ -78,6 +82,7 @@ struct flash_info {
 };
 
 #define JEDEC_MFR(info)	((info)->id[0])
+#define EXT_JEDEC(info)	((info)->id[5])
 
 static const struct flash_info *spi_nor_match_id(const char *name);
 
@@ -899,6 +904,7 @@ static const struct flash_info spi_nor_ids[] = {
 	 */
 	{ "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,  64, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+	{ "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024, 512, 0)},
 	{ "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
 	{ "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
@@ -1036,6 +1042,50 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
 	return ERR_PTR(-ENODEV);
 }
 
+/*
+ * The S25FS-S family physical sectors may be configured as a
+ * hybrid combination of eight 4-kB parameter sectors
+ * at the top or bottom of the address space with all
+ * but one of the remaining sectors being uniform size.
+ * The Parameter Sector Erase commands (20h or 21h) must
+ * be used to erase the 4-kB parameter sectors individually.
+ * The Sector (uniform sector) Erase commands (D8h or DCh)
+ * must be used to erase any of the remaining
+ * sectors, including the portion of highest or lowest address
+ * sector that is not overlaid by the parameter sectors.
+ * The uniform sector erase command has no effect on parameter sectors.
+ */
+static int spansion_s25fs_disable_4kb_erase(struct spi_nor *nor)
+{
+	u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
+	u8 cr3v = 0x0;
+	int ret = 0x0;
+
+	nor->cmd_buf[2] = cr3v_addr >> 16;
+	nor->cmd_buf[1] = cr3v_addr >> 8;
+	nor->cmd_buf[0] = cr3v_addr >> 0;
+
+	ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
+	if (ret)
+		return ret;
+	if (cr3v & CR3V_4KB_ERASE_UNABLE)
+		return 0;
+	ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
+	if (ret)
+		return ret;
+	cr3v = CR3V_4KB_ERASE_UNABLE;
+	nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
+	nor->write(nor, cr3v_addr, 1, &cr3v);
+
+	ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
+	if (ret)
+		return ret;
+	if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
+		return -EPERM;
+
+	return 0;
+}
+
 static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
 			size_t *retlen, u_char *buf)
 {
@@ -1361,6 +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 		spi_nor_wait_till_ready(nor);
 	}
 
+	if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC) {
+		ret = spansion_s25fs_disable_4kb_erase(nor);
+		if (ret)
+			return ret;
+	}
+
 	if (!mtd->name)
 		mtd->name = dev_name(dev);
 	mtd->priv = nor;
-- 
2.1.0.27.g96db324

Re: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Han Xu]

On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui <B56489@freescale.com> wrote:
> From: Yunhui Cui <yunhui.cui@nxp.com>
>
> With the physical sectors combination, S25FS-S family flash
> requires some special operations for read/write functions.
>
> Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 56 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index d0fc165..495d0bb 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -39,6 +39,10 @@
>
>  #define SPI_NOR_MAX_ID_LEN     6
>  #define SPI_NOR_MAX_ADDR_WIDTH 4
> +/* Added for S25FS-S family flash */
> +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
> +#define CR3V_4KB_ERASE_UNABLE  0x8
> +#define SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
>
>  struct flash_info {
>         char            *name;
> @@ -78,6 +82,7 @@ struct flash_info {
>  };
>
>  #define JEDEC_MFR(info)        ((info)->id[0])
> +#define EXT_JEDEC(info)        ((info)->id[5])
>
>  static const struct flash_info *spi_nor_match_id(const char *name);
>
> @@ -899,6 +904,7 @@ static const struct flash_info spi_nor_ids[] = {
>          */
>         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,  64, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024, 512, 0)},
>         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
>         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>         { "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> @@ -1036,6 +1042,50 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
>         return ERR_PTR(-ENODEV);
>  }
>
> +/*
> + * The S25FS-S family physical sectors may be configured as a
> + * hybrid combination of eight 4-kB parameter sectors
> + * at the top or bottom of the address space with all
> + * but one of the remaining sectors being uniform size.
> + * The Parameter Sector Erase commands (20h or 21h) must
> + * be used to erase the 4-kB parameter sectors individually.
> + * The Sector (uniform sector) Erase commands (D8h or DCh)
> + * must be used to erase any of the remaining
> + * sectors, including the portion of highest or lowest address
> + * sector that is not overlaid by the parameter sectors.
> + * The uniform sector erase command has no effect on parameter sectors.
> + */
> +static int spansion_s25fs_disable_4kb_erase(struct spi_nor *nor)
> +{
> +       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
> +       u8 cr3v = 0x0;
> +       int ret = 0x0;
> +
> +       nor->cmd_buf[2] = cr3v_addr >> 16;
> +       nor->cmd_buf[1] = cr3v_addr >> 8;
> +       nor->cmd_buf[0] = cr3v_addr >> 0;
> +
> +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
> +       if (ret)
> +               return ret;
> +       if (cr3v & CR3V_4KB_ERASE_UNABLE)
> +               return 0;
> +       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> +       if (ret)
> +               return ret;
> +       cr3v = CR3V_4KB_ERASE_UNABLE;
> +       nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
> +       nor->write(nor, cr3v_addr, 1, &cr3v);
> +
> +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
> +       if (ret)
> +               return ret;
> +       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> +               return -EPERM;
> +
> +       return 0;
> +}
> +
>  static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
>                         size_t *retlen, u_char *buf)
>  {
> @@ -1361,6 +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>                 spi_nor_wait_till_ready(nor);
>         }
>
> +       if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC) {
> +               ret = spansion_s25fs_disable_4kb_erase(nor);
> +               if (ret)
> +                       return ret;
> +       }
> +
>         if (!mtd->name)
>                 mtd->name = dev_name(dev);
>         mtd->priv = nor;
> --
> 2.1.0.27.g96db324
>
>
Hi Brian, I will ack this change but still feel it's kind of hacking code.

Acked-by: Han xu <han.xu@nxp.com>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

Hello,

> -----Original Message-----
> From: linux-mtd [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf
> Of Han Xu
> Sent: Wednesday, September 14, 2016 9:49 PM
> To: Yunhui Cui <B56489@freescale.com>
> Cc: Yunhui Cui <yunhui.cui@nxp.com>; David Woodhouse
> <dwmw2@infradead.org>; linux-kernel@vger.kernel.org; linux-
> mtd@lists.infradead.org; han.xu@freescale.com; Brian Norris
> <computersforpeace@gmail.com>; jagannadh.teki@gmail.com; linux-arm-
> kernel@lists.infradead.org; Yao Yuan <yao.yuan@nxp.com>
> Subject: Re: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family
> flash
> 
> On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui <B56489@freescale.com>
> wrote:
> > From: Yunhui Cui <yunhui.cui@nxp.com>
> >
> > With the physical sectors combination, S25FS-S family flash requires
> > some special operations for read/write functions.
> >
> > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c | 56
> > +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 56 insertions(+)
> >
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -39,6 +39,10 @@
> >
> >  #define SPI_NOR_MAX_ID_LEN     6
> >  #define SPI_NOR_MAX_ADDR_WIDTH 4
> > +/* Added for S25FS-S family flash */
> > +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
> > +#define CR3V_4KB_ERASE_UNABLE  0x8
> > +#define SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
> >
> >  struct flash_info {
> >         char            *name;
> > @@ -78,6 +82,7 @@ struct flash_info {
> >  };
> >
> >  #define JEDEC_MFR(info)        ((info)->id[0])
> > +#define EXT_JEDEC(info)        ((info)->id[5])
> >
> >  static const struct flash_info *spi_nor_match_id(const char *name);
> >
> > @@ -899,6 +904,7 @@ static const struct flash_info spi_nor_ids[] = {
> >          */
> >         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,  64,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> >         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128,
> > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024, 512, 0)},
> >         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
> >         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> >         { "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256,
> > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -1036,6 +1042,50
> @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
> >         return ERR_PTR(-ENODEV);
> >  }
> >
> > +/*
> > + * The S25FS-S family physical sectors may be configured as a
> > + * hybrid combination of eight 4-kB parameter sectors
> > + * at the top or bottom of the address space with all
> > + * but one of the remaining sectors being uniform size.
> > + * The Parameter Sector Erase commands (20h or 21h) must
> > + * be used to erase the 4-kB parameter sectors individually.
> > + * The Sector (uniform sector) Erase commands (D8h or DCh)
> > + * must be used to erase any of the remaining
> > + * sectors, including the portion of highest or lowest address
> > + * sector that is not overlaid by the parameter sectors.
> > + * The uniform sector erase command has no effect on parameter
> sectors.
> > + */
> > +static int spansion_s25fs_disable_4kb_erase(struct spi_nor *nor) {
> > +       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
> > +       u8 cr3v = 0x0;
> > +       int ret = 0x0;
> > +
> > +       nor->cmd_buf[2] = cr3v_addr >> 16;
> > +       nor->cmd_buf[1] = cr3v_addr >> 8;
> > +       nor->cmd_buf[0] = cr3v_addr >> 0;
> > +
> > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
> > +       if (ret)
> > +               return ret;
> > +       if (cr3v & CR3V_4KB_ERASE_UNABLE)
> > +               return 0;
> > +       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> > +       if (ret)
> > +               return ret;
> > +       cr3v = CR3V_4KB_ERASE_UNABLE;
> > +       nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
> > +       nor->write(nor, cr3v_addr, 1, &cr3v);
> > +
> > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
> > +       if (ret)
> > +               return ret;
> > +       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> > +               return -EPERM;
> > +
> > +       return 0;
> > +}
> > +
> >  static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
> >                         size_t *retlen, u_char *buf)  { @@ -1361,6
> > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> enum read_mode mode)
> >                 spi_nor_wait_till_ready(nor);
> >         }
> >
> > +       if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC) {
> > +               ret = spansion_s25fs_disable_4kb_erase(nor);
> > +               if (ret)
> > +                       return ret;
> > +       }
> > +
> >         if (!mtd->name)
> >                 mtd->name = dev_name(dev);
> >         mtd->priv = nor;
> > --
> > 2.1.0.27.g96db324
> >
> >
> Hi Brian, I will ack this change but still feel it's kind of hacking code.
> 
> Acked-by: Han xu <han.xu@nxp.com>

I am new on the list so I am not sure if this topic has been discussed.
Generally our product functionality relay on those 4KiB sectors.
I know that this hack is already in u-boot, but if you mainstream this
you will force users of those 4KiB sectors to do hack the hack...
I believe the proper solution here is to use erase regions functionality,
I send and RFS about that some time ago.

Thanks,
Marcin

> > ______________________________________________________
> > Linux MTD discussion mailing list
> > http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
> 
> 
> --
> Sincerely,
> 
> Han XU
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Han Xu]

On Thu, Sep 15, 2016 at 06:50:55AM +0000, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
> Hello,
> 
> > -----Original Message-----
> > From: linux-mtd [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf
> > Of Han Xu
> > Sent: Wednesday, September 14, 2016 9:49 PM
> > To: Yunhui Cui <B56489@freescale.com>
> > Cc: Yunhui Cui <yunhui.cui@nxp.com>; David Woodhouse
> > <dwmw2@infradead.org>; linux-kernel@vger.kernel.org; linux-
> > mtd@lists.infradead.org; han.xu@freescale.com; Brian Norris
> > <computersforpeace@gmail.com>; jagannadh.teki@gmail.com; linux-arm-
> > kernel@lists.infradead.org; Yao Yuan <yao.yuan@nxp.com>
> > Subject: Re: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family
> > flash
> > 
> > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui <B56489@freescale.com>
> > wrote:
> > > From: Yunhui Cui <yunhui.cui@nxp.com>
> > >
> > > With the physical sectors combination, S25FS-S family flash requires
> > > some special operations for read/write functions.
> > >
> > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> > > ---
> > >  drivers/mtd/spi-nor/spi-nor.c | 56
> > > +++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 56 insertions(+)
> > >
> > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb 100644
> > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > @@ -39,6 +39,10 @@
> > >
> > >  #define SPI_NOR_MAX_ID_LEN     6
> > >  #define SPI_NOR_MAX_ADDR_WIDTH 4
> > > +/* Added for S25FS-S family flash */
> > > +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
> > > +#define CR3V_4KB_ERASE_UNABLE  0x8
> > > +#define SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
> > >
> > >  struct flash_info {
> > >         char            *name;
> > > @@ -78,6 +82,7 @@ struct flash_info {
> > >  };
> > >
> > >  #define JEDEC_MFR(info)        ((info)->id[0])
> > > +#define EXT_JEDEC(info)        ((info)->id[5])
> > >
> > >  static const struct flash_info *spi_nor_match_id(const char *name);
> > >
> > > @@ -899,6 +904,7 @@ static const struct flash_info spi_nor_ids[] = {
> > >          */
> > >         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,  64,
> > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > >         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128,
> > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024, 512, 0)},
> > >         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
> > >         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512,
> > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > >         { "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256,
> > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -1036,6 +1042,50
> > @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
> > >         return ERR_PTR(-ENODEV);
> > >  }
> > >
> > > +/*
> > > + * The S25FS-S family physical sectors may be configured as a
> > > + * hybrid combination of eight 4-kB parameter sectors
> > > + * at the top or bottom of the address space with all
> > > + * but one of the remaining sectors being uniform size.
> > > + * The Parameter Sector Erase commands (20h or 21h) must
> > > + * be used to erase the 4-kB parameter sectors individually.
> > > + * The Sector (uniform sector) Erase commands (D8h or DCh)
> > > + * must be used to erase any of the remaining
> > > + * sectors, including the portion of highest or lowest address
> > > + * sector that is not overlaid by the parameter sectors.
> > > + * The uniform sector erase command has no effect on parameter
> > sectors.
> > > + */
> > > +static int spansion_s25fs_disable_4kb_erase(struct spi_nor *nor) {
> > > +       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
> > > +       u8 cr3v = 0x0;
> > > +       int ret = 0x0;
> > > +
> > > +       nor->cmd_buf[2] = cr3v_addr >> 16;
> > > +       nor->cmd_buf[1] = cr3v_addr >> 8;
> > > +       nor->cmd_buf[0] = cr3v_addr >> 0;
> > > +
> > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
> > > +       if (ret)
> > > +               return ret;
> > > +       if (cr3v & CR3V_4KB_ERASE_UNABLE)
> > > +               return 0;
> > > +       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> > > +       if (ret)
> > > +               return ret;
> > > +       cr3v = CR3V_4KB_ERASE_UNABLE;
> > > +       nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
> > > +       nor->write(nor, cr3v_addr, 1, &cr3v);
> > > +
> > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
> > > +       if (ret)
> > > +               return ret;
> > > +       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> > > +               return -EPERM;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
> > >                         size_t *retlen, u_char *buf)  { @@ -1361,6
> > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> > enum read_mode mode)
> > >                 spi_nor_wait_till_ready(nor);
> > >         }
> > >
> > > +       if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC) {
> > > +               ret = spansion_s25fs_disable_4kb_erase(nor);
> > > +               if (ret)
> > > +                       return ret;
> > > +       }
> > > +
> > >         if (!mtd->name)
> > >                 mtd->name = dev_name(dev);
> > >         mtd->priv = nor;
> > > --
> > > 2.1.0.27.g96db324
> > >
> > >
> > Hi Brian, I will ack this change but still feel it's kind of hacking code.
> > 
> > Acked-by: Han xu <han.xu@nxp.com>
> 
> I am new on the list so I am not sure if this topic has been discussed.
> Generally our product functionality relay on those 4KiB sectors.
> I know that this hack is already in u-boot, but if you mainstream this
> you will force users of those 4KiB sectors to do hack the hack...
> I believe the proper solution here is to use erase regions functionality,
> I send and RFS about that some time ago.

Do you mind to send me a link for reference?

Sincerely,
Han XU

> 
> Thanks,
> Marcin
> 
> > > ______________________________________________________
> > > Linux MTD discussion mailing list
> > > http://lists.infradead.org/mailman/listinfo/linux-mtd/
> > 
> > 
> > 
> > --
> > Sincerely,
> > 
> > Han XU
> > 
> > ______________________________________________________
> > Linux MTD discussion mailing list
> > http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

> -----Original Message-----
> From: Han Xu [mailto:xhnjupt@gmail.com]
> Sent: Wednesday, November 16, 2016 6:11 PM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <marcin.krzeminski@nokia.com>
> Cc: Yunhui Cui <B56489@freescale.com>; Yunhui Cui <yunhui.cui@nxp.com>;
> David Woodhouse <dwmw2@infradead.org>; linux-kernel@vger.kernel.org;
> linux-mtd@lists.infradead.org; han.xu@freescale.com; Brian Norris
> <computersforpeace@gmail.com>; jagannadh.teki@gmail.com; linux-arm-
> kernel@lists.infradead.org; Yao Yuan <yao.yuan@nxp.com>
> Subject: Re: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family
> flash
> 
> On Thu, Sep 15, 2016 at 06:50:55AM +0000, Krzeminski, Marcin (Nokia -
> PL/Wroclaw) wrote:
> > Hello,
> >
> > > -----Original Message-----
> > > From: linux-mtd [mailto:linux-mtd-bounces@lists.infradead.org] On
> > > Behalf Of Han Xu
> > > Sent: Wednesday, September 14, 2016 9:49 PM
> > > To: Yunhui Cui <B56489@freescale.com>
> > > Cc: Yunhui Cui <yunhui.cui@nxp.com>; David Woodhouse
> > > <dwmw2@infradead.org>; linux-kernel@vger.kernel.org; linux-
> > > mtd@lists.infradead.org; han.xu@freescale.com; Brian Norris
> > > <computersforpeace@gmail.com>; jagannadh.teki@gmail.com; linux-
> arm-
> > > kernel@lists.infradead.org; Yao Yuan <yao.yuan@nxp.com>
> > > Subject: Re: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S
> > > family flash
> > >
> > > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui <B56489@freescale.com>
> > > wrote:
> > > > From: Yunhui Cui <yunhui.cui@nxp.com>
> > > >
> > > > With the physical sectors combination, S25FS-S family flash
> > > > requires some special operations for read/write functions.
> > > >
> > > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> > > > ---
> > > >  drivers/mtd/spi-nor/spi-nor.c | 56
> > > > +++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 56 insertions(+)
> > > >
> > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb 100644
> > > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > > @@ -39,6 +39,10 @@
> > > >
> > > >  #define SPI_NOR_MAX_ID_LEN     6
> > > >  #define SPI_NOR_MAX_ADDR_WIDTH 4
> > > > +/* Added for S25FS-S family flash */
> > > > +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
> > > > +#define CR3V_4KB_ERASE_UNABLE  0x8 #define
> > > > +SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
> > > >
> > > >  struct flash_info {
> > > >         char            *name;
> > > > @@ -78,6 +82,7 @@ struct flash_info {  };
> > > >
> > > >  #define JEDEC_MFR(info)        ((info)->id[0])
> > > > +#define EXT_JEDEC(info)        ((info)->id[5])
> > > >
> > > >  static const struct flash_info *spi_nor_match_id(const char
> > > > *name);
> > > >
> > > > @@ -899,6 +904,7 @@ static const struct flash_info spi_nor_ids[] = {
> > > >          */
> > > >         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,  64,
> > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > >         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128,
> > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024, 512,
> > > > + 0)},
> > > >         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
> > > >         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512,
> > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > >         { "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256,
> > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -1036,6
> +1042,50
> > > @@ static const struct flash_info *spi_nor_read_id(struct spi_nor
> > > *nor)
> > > >         return ERR_PTR(-ENODEV);
> > > >  }
> > > >
> > > > +/*
> > > > + * The S25FS-S family physical sectors may be configured as a
> > > > + * hybrid combination of eight 4-kB parameter sectors
> > > > + * at the top or bottom of the address space with all
> > > > + * but one of the remaining sectors being uniform size.
> > > > + * The Parameter Sector Erase commands (20h or 21h) must
> > > > + * be used to erase the 4-kB parameter sectors individually.
> > > > + * The Sector (uniform sector) Erase commands (D8h or DCh)
> > > > + * must be used to erase any of the remaining
> > > > + * sectors, including the portion of highest or lowest address
> > > > + * sector that is not overlaid by the parameter sectors.
> > > > + * The uniform sector erase command has no effect on parameter
> > > sectors.
> > > > + */
> > > > +static int spansion_s25fs_disable_4kb_erase(struct spi_nor *nor) {
> > > > +       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
> > > > +       u8 cr3v = 0x0;
> > > > +       int ret = 0x0;
> > > > +
> > > > +       nor->cmd_buf[2] = cr3v_addr >> 16;
> > > > +       nor->cmd_buf[1] = cr3v_addr >> 8;
> > > > +       nor->cmd_buf[0] = cr3v_addr >> 0;
> > > > +
> > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
> > > > +       if (ret)
> > > > +               return ret;
> > > > +       if (cr3v & CR3V_4KB_ERASE_UNABLE)
> > > > +               return 0;
> > > > +       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> > > > +       if (ret)
> > > > +               return ret;
> > > > +       cr3v = CR3V_4KB_ERASE_UNABLE;
> > > > +       nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
> > > > +       nor->write(nor, cr3v_addr, 1, &cr3v);
> > > > +
> > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
> > > > +       if (ret)
> > > > +               return ret;
> > > > +       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> > > > +               return -EPERM;
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > >  static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
> > > >                         size_t *retlen, u_char *buf)  { @@ -1361,6
> > > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const char
> > > > +*name,
> > > enum read_mode mode)
> > > >                 spi_nor_wait_till_ready(nor);
> > > >         }
> > > >
> > > > +       if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC) {
> > > > +               ret = spansion_s25fs_disable_4kb_erase(nor);
> > > > +               if (ret)
> > > > +                       return ret;
> > > > +       }
> > > > +
> > > >         if (!mtd->name)
> > > >                 mtd->name = dev_name(dev);
> > > >         mtd->priv = nor;
> > > > --
> > > > 2.1.0.27.g96db324
> > > >
> > > >
> > > Hi Brian, I will ack this change but still feel it's kind of hacking code.
> > >
> > > Acked-by: Han xu <han.xu@nxp.com>
> >
> > I am new on the list so I am not sure if this topic has been discussed.
> > Generally our product functionality relay on those 4KiB sectors.
> > I know that this hack is already in u-boot, but if you mainstream this
> > you will force users of those 4KiB sectors to do hack the hack...
> > I believe the proper solution here is to use erase regions
> > functionality, I send and RFS about that some time ago.
> 
> Do you mind to send me a link for reference?
> 
Han,

Sorry, It seem I have not posted erase region changes (only those regarding DUAL/QUAD I/O).
Generally, in this flash you need to create 3 erase regions (because in FS-S family support only
 4KiB erase on parameters sector - eg. 1.2.2.4 in S25FS512S). In my case regions are:
1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21)
2. 32 - 256 - SE_CMD (0xd8/0xdc)
3. Rest of the flash SE_CMD (0xd8/0xdc)

To erase whole flash you can also use CHIP_ERASE_CMD (0x60/0xC7) command, you just
need to add one more mtd partition that will cover whole flash.

Thanks,
Marcin

> Sincerely,
> Han XU
> 
> >
> > Thanks,
> > Marcin
> >
> > > > ______________________________________________________
> > > > Linux MTD discussion mailing list
> > > > http://lists.infradead.org/mailman/listinfo/linux-mtd/
> > >
> > >
> > >
> > > --
> > > Sincerely,
> > >
> > > Han XU
> > >
> > > ______________________________________________________
> > > Linux MTD discussion mailing list
> > > http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Yao Yuan]

On Thu, Nov 17, 2016 at 06:50:55AM +0000, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
> > > > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui <B56489@freescale.com>
> > > > wrote:
> > > > > From: Yunhui Cui <yunhui.cui@nxp.com>
> > > > >
> > > > > With the physical sectors combination, S25FS-S family flash
> > > > > requires some special operations for read/write functions.
> > > > >
> > > > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > ---
> > > > >  drivers/mtd/spi-nor/spi-nor.c | 56
> > > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 56 insertions(+)
> > > > >
> > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb 100644
> > > > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > > > @@ -39,6 +39,10 @@
> > > > >
> > > > >  #define SPI_NOR_MAX_ID_LEN     6
> > > > >  #define SPI_NOR_MAX_ADDR_WIDTH 4
> > > > > +/* Added for S25FS-S family flash */
> > > > > +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
> > > > > +#define CR3V_4KB_ERASE_UNABLE  0x8 #define
> > > > > +SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
> > > > >
> > > > >  struct flash_info {
> > > > >         char            *name;
> > > > > @@ -78,6 +82,7 @@ struct flash_info {  };
> > > > >
> > > > >  #define JEDEC_MFR(info)        ((info)->id[0])
> > > > > +#define EXT_JEDEC(info)        ((info)->id[5])
> > > > >
> > > > >  static const struct flash_info *spi_nor_match_id(const char
> > > > > *name);
> > > > >
> > > > > @@ -899,6 +904,7 @@ static const struct flash_info spi_nor_ids[] = {
> > > > >          */
> > > > >         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,  64,
> > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > >         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128,
> > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024,
> > > > > + 512, 0)},
> > > > >         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
> > > > >         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512,
> > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > >         { "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256,
> > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -1036,6
> > +1042,50
> > > > @@ static const struct flash_info *spi_nor_read_id(struct spi_nor
> > > > *nor)
> > > > >         return ERR_PTR(-ENODEV);  }
> > > > >
> > > > > +/*
> > > > > + * The S25FS-S family physical sectors may be configured as a
> > > > > + * hybrid combination of eight 4-kB parameter sectors
> > > > > + * at the top or bottom of the address space with all
> > > > > + * but one of the remaining sectors being uniform size.
> > > > > + * The Parameter Sector Erase commands (20h or 21h) must
> > > > > + * be used to erase the 4-kB parameter sectors individually.
> > > > > + * The Sector (uniform sector) Erase commands (D8h or DCh)
> > > > > + * must be used to erase any of the remaining
> > > > > + * sectors, including the portion of highest or lowest address
> > > > > + * sector that is not overlaid by the parameter sectors.
> > > > > + * The uniform sector erase command has no effect on parameter
> > > > sectors.
> > > > > + */
> > > > > +static int spansion_s25fs_disable_4kb_erase(struct spi_nor *nor) {
> > > > > +       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
> > > > > +       u8 cr3v = 0x0;
> > > > > +       int ret = 0x0;
> > > > > +
> > > > > +       nor->cmd_buf[2] = cr3v_addr >> 16;
> > > > > +       nor->cmd_buf[1] = cr3v_addr >> 8;
> > > > > +       nor->cmd_buf[0] = cr3v_addr >> 0;
> > > > > +
> > > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
> > > > > +       if (ret)
> > > > > +               return ret;
> > > > > +       if (cr3v & CR3V_4KB_ERASE_UNABLE)
> > > > > +               return 0;
> > > > > +       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> > > > > +       if (ret)
> > > > > +               return ret;
> > > > > +       cr3v = CR3V_4KB_ERASE_UNABLE;
> > > > > +       nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
> > > > > +       nor->write(nor, cr3v_addr, 1, &cr3v);
> > > > > +
> > > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
> > > > > +       if (ret)
> > > > > +               return ret;
> > > > > +       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> > > > > +               return -EPERM;
> > > > > +
> > > > > +       return 0;
> > > > > +}
> > > > > +
> > > > >  static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
> > > > >                         size_t *retlen, u_char *buf)  { @@
> > > > > -1361,6
> > > > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const char
> > > > > +*name,
> > > > enum read_mode mode)
> > > > >                 spi_nor_wait_till_ready(nor);
> > > > >         }
> > > > >
> > > > > +       if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC) {
> > > > > +               ret = spansion_s25fs_disable_4kb_erase(nor);
> > > > > +               if (ret)
> > > > > +                       return ret;
> > > > > +       }
> > > > > +
> > > > >         if (!mtd->name)
> > > > >                 mtd->name = dev_name(dev);
> > > > >         mtd->priv = nor;
> > > > > --
> > > > > 2.1.0.27.g96db324
> > > > >
> > > > >
> > > > Hi Brian, I will ack this change but still feel it's kind of hacking code.
> > > >
> > > > Acked-by: Han xu <han.xu@nxp.com>
> > >
> > > I am new on the list so I am not sure if this topic has been discussed.
> > > Generally our product functionality relay on those 4KiB sectors.
> > > I know that this hack is already in u-boot, but if you mainstream
> > > this you will force users of those 4KiB sectors to do hack the hack...
> > > I believe the proper solution here is to use erase regions
> > > functionality, I send and RFS about that some time ago.
> >
> > Do you mind to send me a link for reference?
> >
> Han,
> 
> Sorry, It seem I have not posted erase region changes (only those regarding
> DUAL/QUAD I/O).
> Generally, in this flash you need to create 3 erase regions (because in FS-S
> family support only  4KiB erase on parameters sector - eg. 1.2.2.4 in S25FS512S).
> In my case regions are:
> 1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21) 2. 32 - 256 - SE_CMD (0xd8/0xdc) 3.
> Rest of the flash SE_CMD (0xd8/0xdc)
> 
> To erase whole flash you can also use CHIP_ERASE_CMD (0x60/0xC7) command,
> you just need to add one more mtd partition that will cover whole flash.
> 

Hi Krzeminski,

Do you think is there any great advantages for enable 4KB?
Because for NXP(Freescale) QSPI controller, there is only support max to 16 groups command.

So It's hard to give 3 groups command just for erase (0x21,0xdc and 0xc7).
So we have to disable the 4kb erase and only use 256kbytes in this patch.

RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

> -----Original Message-----
> From: Yao Yuan [mailto:yao.yuan@nxp.com]
> Sent: Thursday, November 17, 2016 10:14 AM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <marcin.krzeminski@nokia.com>; Han Xu <xhnjupt@gmail.com>
> Cc: David Woodhouse <dwmw2@infradead.org>; linux-
> kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> han.xu@freescale.com; Brian Norris <computersforpeace@gmail.com>;
> jagannadh.teki@gmail.com; linux-arm-kernel@lists.infradead.org
> Subject: RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family
> flash
> 
> On Thu, Nov 17, 2016 at 06:50:55AM +0000, Krzeminski, Marcin (Nokia -
> PL/Wroclaw) wrote:
> > > > > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui
> > > > > <B56489@freescale.com>
> > > > > wrote:
> > > > > > From: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > >
> > > > > > With the physical sectors combination, S25FS-S family flash
> > > > > > requires some special operations for read/write functions.
> > > > > >
> > > > > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > ---
> > > > > >  drivers/mtd/spi-nor/spi-nor.c | 56
> > > > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > > >  1 file changed, 56 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb 100644
> > > > > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > > > > @@ -39,6 +39,10 @@
> > > > > >
> > > > > >  #define SPI_NOR_MAX_ID_LEN     6
> > > > > >  #define SPI_NOR_MAX_ADDR_WIDTH 4
> > > > > > +/* Added for S25FS-S family flash */
> > > > > > +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
> > > > > > +#define CR3V_4KB_ERASE_UNABLE  0x8 #define
> > > > > > +SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
> > > > > >
> > > > > >  struct flash_info {
> > > > > >         char            *name;
> > > > > > @@ -78,6 +82,7 @@ struct flash_info {  };
> > > > > >
> > > > > >  #define JEDEC_MFR(info)        ((info)->id[0])
> > > > > > +#define EXT_JEDEC(info)        ((info)->id[5])
> > > > > >
> > > > > >  static const struct flash_info *spi_nor_match_id(const char
> > > > > > *name);
> > > > > >
> > > > > > @@ -899,6 +904,7 @@ static const struct flash_info spi_nor_ids[] = {
> > > > > >          */
> > > > > >         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,
> > > > > > 64,
> > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > >         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024,
> > > > > > 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024,
> > > > > > + 512, 0)},
> > > > > >         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
> > > > > >         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024,
> > > > > > 512,
> > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > >         { "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024,
> > > > > > 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -1036,6
> > > +1042,50
> > > > > @@ static const struct flash_info *spi_nor_read_id(struct
> > > > > spi_nor
> > > > > *nor)
> > > > > >         return ERR_PTR(-ENODEV);  }
> > > > > >
> > > > > > +/*
> > > > > > + * The S25FS-S family physical sectors may be configured as a
> > > > > > + * hybrid combination of eight 4-kB parameter sectors
> > > > > > + * at the top or bottom of the address space with all
> > > > > > + * but one of the remaining sectors being uniform size.
> > > > > > + * The Parameter Sector Erase commands (20h or 21h) must
> > > > > > + * be used to erase the 4-kB parameter sectors individually.
> > > > > > + * The Sector (uniform sector) Erase commands (D8h or DCh)
> > > > > > + * must be used to erase any of the remaining
> > > > > > + * sectors, including the portion of highest or lowest
> > > > > > +address
> > > > > > + * sector that is not overlaid by the parameter sectors.
> > > > > > + * The uniform sector erase command has no effect on
> > > > > > +parameter
> > > > > sectors.
> > > > > > + */
> > > > > > +static int spansion_s25fs_disable_4kb_erase(struct spi_nor *nor) {
> > > > > > +       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
> > > > > > +       u8 cr3v = 0x0;
> > > > > > +       int ret = 0x0;
> > > > > > +
> > > > > > +       nor->cmd_buf[2] = cr3v_addr >> 16;
> > > > > > +       nor->cmd_buf[1] = cr3v_addr >> 8;
> > > > > > +       nor->cmd_buf[0] = cr3v_addr >> 0;
> > > > > > +
> > > > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR,
> &cr3v, 1);
> > > > > > +       if (ret)
> > > > > > +               return ret;
> > > > > > +       if (cr3v & CR3V_4KB_ERASE_UNABLE)
> > > > > > +               return 0;
> > > > > > +       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> > > > > > +       if (ret)
> > > > > > +               return ret;
> > > > > > +       cr3v = CR3V_4KB_ERASE_UNABLE;
> > > > > > +       nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
> > > > > > +       nor->write(nor, cr3v_addr, 1, &cr3v);
> > > > > > +
> > > > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR,
> &cr3v, 1);
> > > > > > +       if (ret)
> > > > > > +               return ret;
> > > > > > +       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> > > > > > +               return -EPERM;
> > > > > > +
> > > > > > +       return 0;
> > > > > > +}
> > > > > > +
> > > > > >  static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t
> len,
> > > > > >                         size_t *retlen, u_char *buf)  { @@
> > > > > > -1361,6
> > > > > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const char
> > > > > > +*name,
> > > > > enum read_mode mode)
> > > > > >                 spi_nor_wait_till_ready(nor);
> > > > > >         }
> > > > > >
> > > > > > +       if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC) {
> > > > > > +               ret = spansion_s25fs_disable_4kb_erase(nor);
> > > > > > +               if (ret)
> > > > > > +                       return ret;
> > > > > > +       }
> > > > > > +
> > > > > >         if (!mtd->name)
> > > > > >                 mtd->name = dev_name(dev);
> > > > > >         mtd->priv = nor;
> > > > > > --
> > > > > > 2.1.0.27.g96db324
> > > > > >
> > > > > >
> > > > > Hi Brian, I will ack this change but still feel it's kind of hacking code.
> > > > >
> > > > > Acked-by: Han xu <han.xu@nxp.com>
> > > >
> > > > I am new on the list so I am not sure if this topic has been discussed.
> > > > Generally our product functionality relay on those 4KiB sectors.
> > > > I know that this hack is already in u-boot, but if you mainstream
> > > > this you will force users of those 4KiB sectors to do hack the hack...
> > > > I believe the proper solution here is to use erase regions
> > > > functionality, I send and RFS about that some time ago.
> > >
> > > Do you mind to send me a link for reference?
> > >
> > Han,
> >
> > Sorry, It seem I have not posted erase region changes (only those
> > regarding DUAL/QUAD I/O).
> > Generally, in this flash you need to create 3 erase regions (because
> > in FS-S family support only  4KiB erase on parameters sector - eg. 1.2.2.4 in
> S25FS512S).
> > In my case regions are:
> > 1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21) 2. 32 - 256 - SE_CMD
> (0xd8/0xdc) 3.
> > Rest of the flash SE_CMD (0xd8/0xdc)
> >
> > To erase whole flash you can also use CHIP_ERASE_CMD (0x60/0xC7)
> > command, you just need to add one more mtd partition that will cover
> whole flash.
> >
> 
> Hi Krzeminski,
> 
> Do you think is there any great advantages for enable 4KB?
> Because for NXP(Freescale) QSPI controller, there is only support max to 16
> groups command.
> 
> So It's hard to give 3 groups command just for erase (0x21,0xdc and 0xc7).
> So we have to disable the 4kb erase and only use 256kbytes in this patch.
> 
Yes, if you disable parameters sector in spi-nor framework you will disable it
for all spi-nor clients not only for NXP QSPI controller. There are users (at
least me) that relay on parameters sector functionality. This patch will brake it.

Thanks,
Marcin

RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Yao Yuan]

On Thu, Nov 17, 2016 at 10:14:55AM +0000, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
> > On Thu, Nov 17, 2016 at 06:50:55AM +0000, Krzeminski, Marcin (Nokia -
> > PL/Wroclaw) wrote:
> > > > > > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui
> > > > > > <B56489@freescale.com>
> > > > > > wrote:
> > > > > > > From: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > >
> > > > > > > With the physical sectors combination, S25FS-S family flash
> > > > > > > requires some special operations for read/write functions.
> > > > > > >
> > > > > > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > > ---
> > > > > > >  drivers/mtd/spi-nor/spi-nor.c | 56
> > > > > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > > > >  1 file changed, 56 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb
> > > > > > > 100644
> > > > > > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > @@ -39,6 +39,10 @@
> > > > > > >
> > > > > > >  #define SPI_NOR_MAX_ID_LEN     6
> > > > > > >  #define SPI_NOR_MAX_ADDR_WIDTH 4
> > > > > > > +/* Added for S25FS-S family flash */
> > > > > > > +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
> > > > > > > +#define CR3V_4KB_ERASE_UNABLE  0x8 #define
> > > > > > > +SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
> > > > > > >
> > > > > > >  struct flash_info {
> > > > > > >         char            *name;
> > > > > > > @@ -78,6 +82,7 @@ struct flash_info {  };
> > > > > > >
> > > > > > >  #define JEDEC_MFR(info)        ((info)->id[0])
> > > > > > > +#define EXT_JEDEC(info)        ((info)->id[5])
> > > > > > >
> > > > > > >  static const struct flash_info *spi_nor_match_id(const char
> > > > > > > *name);
> > > > > > >
> > > > > > > @@ -899,6 +904,7 @@ static const struct flash_info spi_nor_ids[] = {
> > > > > > >          */
> > > > > > >         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,
> > > > > > > 64,
> > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > >         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024,
> > > > > > > 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024,
> > > > > > > + 512, 0)},
> > > > > > >         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
> > > > > > >         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024,
> > > > > > > 512,
> > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > >         { "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024,
> > > > > > > 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -1036,6
> > > > +1042,50
> > > > > > @@ static const struct flash_info *spi_nor_read_id(struct
> > > > > > spi_nor
> > > > > > *nor)
> > > > > > >         return ERR_PTR(-ENODEV);  }
> > > > > > >
> > > > > > > +/*
> > > > > > > + * The S25FS-S family physical sectors may be configured as
> > > > > > > +a
> > > > > > > + * hybrid combination of eight 4-kB parameter sectors
> > > > > > > + * at the top or bottom of the address space with all
> > > > > > > + * but one of the remaining sectors being uniform size.
> > > > > > > + * The Parameter Sector Erase commands (20h or 21h) must
> > > > > > > + * be used to erase the 4-kB parameter sectors individually.
> > > > > > > + * The Sector (uniform sector) Erase commands (D8h or DCh)
> > > > > > > + * must be used to erase any of the remaining
> > > > > > > + * sectors, including the portion of highest or lowest
> > > > > > > +address
> > > > > > > + * sector that is not overlaid by the parameter sectors.
> > > > > > > + * The uniform sector erase command has no effect on
> > > > > > > +parameter
> > > > > > sectors.
> > > > > > > + */
> > > > > > > +static int spansion_s25fs_disable_4kb_erase(struct spi_nor *nor) {
> > > > > > > +       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
> > > > > > > +       u8 cr3v = 0x0;
> > > > > > > +       int ret = 0x0;
> > > > > > > +
> > > > > > > +       nor->cmd_buf[2] = cr3v_addr >> 16;
> > > > > > > +       nor->cmd_buf[1] = cr3v_addr >> 8;
> > > > > > > +       nor->cmd_buf[0] = cr3v_addr >> 0;
> > > > > > > +
> > > > > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR,
> > &cr3v, 1);
> > > > > > > +       if (ret)
> > > > > > > +               return ret;
> > > > > > > +       if (cr3v & CR3V_4KB_ERASE_UNABLE)
> > > > > > > +               return 0;
> > > > > > > +       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> > > > > > > +       if (ret)
> > > > > > > +               return ret;
> > > > > > > +       cr3v = CR3V_4KB_ERASE_UNABLE;
> > > > > > > +       nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
> > > > > > > +       nor->write(nor, cr3v_addr, 1, &cr3v);
> > > > > > > +
> > > > > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR,
> > &cr3v, 1);
> > > > > > > +       if (ret)
> > > > > > > +               return ret;
> > > > > > > +       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> > > > > > > +               return -EPERM;
> > > > > > > +
> > > > > > > +       return 0;
> > > > > > > +}
> > > > > > > +
> > > > > > >  static int spi_nor_read(struct mtd_info *mtd, loff_t from,
> > > > > > > size_t
> > len,
> > > > > > >                         size_t *retlen, u_char *buf)  { @@
> > > > > > > -1361,6
> > > > > > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const char
> > > > > > > +*name,
> > > > > > enum read_mode mode)
> > > > > > >                 spi_nor_wait_till_ready(nor);
> > > > > > >         }
> > > > > > >
> > > > > > > +       if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC) {
> > > > > > > +               ret = spansion_s25fs_disable_4kb_erase(nor);
> > > > > > > +               if (ret)
> > > > > > > +                       return ret;
> > > > > > > +       }
> > > > > > > +
> > > > > > >         if (!mtd->name)
> > > > > > >                 mtd->name = dev_name(dev);
> > > > > > >         mtd->priv = nor;
> > > > > > > --
> > > > > > > 2.1.0.27.g96db324
> > > > > > >
> > > > > > >
> > > > > > Hi Brian, I will ack this change but still feel it's kind of hacking code.
> > > > > >
> > > > > > Acked-by: Han xu <han.xu@nxp.com>
> > > > >
> > > > > I am new on the list so I am not sure if this topic has been discussed.
> > > > > Generally our product functionality relay on those 4KiB sectors.
> > > > > I know that this hack is already in u-boot, but if you
> > > > > mainstream this you will force users of those 4KiB sectors to do hack the
> hack...
> > > > > I believe the proper solution here is to use erase regions
> > > > > functionality, I send and RFS about that some time ago.
> > > >
> > > > Do you mind to send me a link for reference?
> > > >
> > > Han,
> > >
> > > Sorry, It seem I have not posted erase region changes (only those
> > > regarding DUAL/QUAD I/O).
> > > Generally, in this flash you need to create 3 erase regions (because
> > > in FS-S family support only  4KiB erase on parameters sector - eg.
> > > 1.2.2.4 in
> > S25FS512S).
> > > In my case regions are:
> > > 1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21) 2. 32 - 256 - SE_CMD
> > (0xd8/0xdc) 3.
> > > Rest of the flash SE_CMD (0xd8/0xdc)
> > >
> > > To erase whole flash you can also use CHIP_ERASE_CMD (0x60/0xC7)
> > > command, you just need to add one more mtd partition that will cover
> > whole flash.
> > >
> >
> > Hi Krzeminski,
> >
> > Do you think is there any great advantages for enable 4KB?
> > Because for NXP(Freescale) QSPI controller, there is only support max
> > to 16 groups command.
> >
> > So It's hard to give 3 groups command just for erase (0x21,0xdc and 0xc7).
> > So we have to disable the 4kb erase and only use 256kbytes in this patch.
> >
> Yes, if you disable parameters sector in spi-nor framework you will disable it for
> all spi-nor clients not only for NXP QSPI controller. There are users (at least me)
> that relay on parameters sector functionality. This patch will brake it.
> 
> Thanks,

Hi Krzeminski,

Get it.
So do you think how about that I add a flag in dts to select it?
The user want's disable 4kb, he can add the flag.

In spi-nor.c:
if (of_property_read_bool(np, "spi-nor, disable-4kb")) {
	spansion_s25fs_disable_4kb_erase();
}
                else
...

In dts:

&qspi {
        num-cs = <2>;
        bus-num = <0>;
        status = "okay";

        qflash0: s25fs512s@0 {
                compatible = "spansion, s25fs512s";
	 spi-nor, disable-4kb
                #address-cells = <1>;
                #size-cells = <1>;
                spi-max-frequency = <20000000>;
                reg = <0>;
        };

I think it should be a better way.

How about your think?

Thanks.

RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

> -----Original Message-----
> From: Yao Yuan [mailto:yao.yuan@nxp.com]
> Sent: Friday, November 18, 2016 5:20 AM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <marcin.krzeminski@nokia.com>; Han Xu <xhnjupt@gmail.com>
> Cc: David Woodhouse <dwmw2@infradead.org>; linux-
> kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> han.xu@freescale.com; Brian Norris <computersforpeace@gmail.com>;
> jagannadh.teki@gmail.com; linux-arm-kernel@lists.infradead.org
> Subject: RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family
> flash
> 
> On Thu, Nov 17, 2016 at 10:14:55AM +0000, Krzeminski, Marcin (Nokia -
> PL/Wroclaw) wrote:
> > > On Thu, Nov 17, 2016 at 06:50:55AM +0000, Krzeminski, Marcin (Nokia
> > > -
> > > PL/Wroclaw) wrote:
> > > > > > > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui
> > > > > > > <B56489@freescale.com>
> > > > > > > wrote:
> > > > > > > > From: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > > >
> > > > > > > > With the physical sectors combination, S25FS-S family
> > > > > > > > flash requires some special operations for read/write functions.
> > > > > > > >
> > > > > > > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > > > ---
> > > > > > > >  drivers/mtd/spi-nor/spi-nor.c | 56
> > > > > > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > > > > >  1 file changed, 56 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb
> > > > > > > > 100644
> > > > > > > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > @@ -39,6 +39,10 @@
> > > > > > > >
> > > > > > > >  #define SPI_NOR_MAX_ID_LEN     6
> > > > > > > >  #define SPI_NOR_MAX_ADDR_WIDTH 4
> > > > > > > > +/* Added for S25FS-S family flash */
> > > > > > > > +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
> > > > > > > > +#define CR3V_4KB_ERASE_UNABLE  0x8 #define
> > > > > > > > +SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
> > > > > > > >
> > > > > > > >  struct flash_info {
> > > > > > > >         char            *name;
> > > > > > > > @@ -78,6 +82,7 @@ struct flash_info {  };
> > > > > > > >
> > > > > > > >  #define JEDEC_MFR(info)        ((info)->id[0])
> > > > > > > > +#define EXT_JEDEC(info)        ((info)->id[5])
> > > > > > > >
> > > > > > > >  static const struct flash_info *spi_nor_match_id(const
> > > > > > > > char *name);
> > > > > > > >
> > > > > > > > @@ -899,6 +904,7 @@ static const struct flash_info
> spi_nor_ids[] = {
> > > > > > > >          */
> > > > > > > >         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,
> > > > > > > > 64,
> > > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > >         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024,
> > > > > > > > 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > > +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 *
> > > > > > > > + 1024, 512, 0)},
> > > > > > > >         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
> > > > > > > >         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024,
> > > > > > > > 512,
> > > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > >         { "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024,
> > > > > > > > 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -
> 1036,6
> > > > > +1042,50
> > > > > > > @@ static const struct flash_info *spi_nor_read_id(struct
> > > > > > > spi_nor
> > > > > > > *nor)
> > > > > > > >         return ERR_PTR(-ENODEV);  }
> > > > > > > >
> > > > > > > > +/*
> > > > > > > > + * The S25FS-S family physical sectors may be configured
> > > > > > > > +as a
> > > > > > > > + * hybrid combination of eight 4-kB parameter sectors
> > > > > > > > + * at the top or bottom of the address space with all
> > > > > > > > + * but one of the remaining sectors being uniform size.
> > > > > > > > + * The Parameter Sector Erase commands (20h or 21h) must
> > > > > > > > + * be used to erase the 4-kB parameter sectors individually.
> > > > > > > > + * The Sector (uniform sector) Erase commands (D8h or
> > > > > > > > +DCh)
> > > > > > > > + * must be used to erase any of the remaining
> > > > > > > > + * sectors, including the portion of highest or lowest
> > > > > > > > +address
> > > > > > > > + * sector that is not overlaid by the parameter sectors.
> > > > > > > > + * The uniform sector erase command has no effect on
> > > > > > > > +parameter
> > > > > > > sectors.
> > > > > > > > + */
> > > > > > > > +static int spansion_s25fs_disable_4kb_erase(struct spi_nor
> *nor) {
> > > > > > > > +       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
> > > > > > > > +       u8 cr3v = 0x0;
> > > > > > > > +       int ret = 0x0;
> > > > > > > > +
> > > > > > > > +       nor->cmd_buf[2] = cr3v_addr >> 16;
> > > > > > > > +       nor->cmd_buf[1] = cr3v_addr >> 8;
> > > > > > > > +       nor->cmd_buf[0] = cr3v_addr >> 0;
> > > > > > > > +
> > > > > > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR,
> > > &cr3v, 1);
> > > > > > > > +       if (ret)
> > > > > > > > +               return ret;
> > > > > > > > +       if (cr3v & CR3V_4KB_ERASE_UNABLE)
> > > > > > > > +               return 0;
> > > > > > > > +       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> > > > > > > > +       if (ret)
> > > > > > > > +               return ret;
> > > > > > > > +       cr3v = CR3V_4KB_ERASE_UNABLE;
> > > > > > > > +       nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
> > > > > > > > +       nor->write(nor, cr3v_addr, 1, &cr3v);
> > > > > > > > +
> > > > > > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR,
> > > &cr3v, 1);
> > > > > > > > +       if (ret)
> > > > > > > > +               return ret;
> > > > > > > > +       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> > > > > > > > +               return -EPERM;
> > > > > > > > +
> > > > > > > > +       return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > >  static int spi_nor_read(struct mtd_info *mtd, loff_t
> > > > > > > > from, size_t
> > > len,
> > > > > > > >                         size_t *retlen, u_char *buf)  { @@
> > > > > > > > -1361,6
> > > > > > > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const
> > > > > > > > +char *name,
> > > > > > > enum read_mode mode)
> > > > > > > >                 spi_nor_wait_till_ready(nor);
> > > > > > > >         }
> > > > > > > >
> > > > > > > > +       if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC)
> {
> > > > > > > > +               ret = spansion_s25fs_disable_4kb_erase(nor);
> > > > > > > > +               if (ret)
> > > > > > > > +                       return ret;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > >         if (!mtd->name)
> > > > > > > >                 mtd->name = dev_name(dev);
> > > > > > > >         mtd->priv = nor;
> > > > > > > > --
> > > > > > > > 2.1.0.27.g96db324
> > > > > > > >
> > > > > > > >
> > > > > > > Hi Brian, I will ack this change but still feel it's kind of hacking code.
> > > > > > >
> > > > > > > Acked-by: Han xu <han.xu@nxp.com>
> > > > > >
> > > > > > I am new on the list so I am not sure if this topic has been discussed.
> > > > > > Generally our product functionality relay on those 4KiB sectors.
> > > > > > I know that this hack is already in u-boot, but if you
> > > > > > mainstream this you will force users of those 4KiB sectors to
> > > > > > do hack the
> > hack...
> > > > > > I believe the proper solution here is to use erase regions
> > > > > > functionality, I send and RFS about that some time ago.
> > > > >
> > > > > Do you mind to send me a link for reference?
> > > > >
> > > > Han,
> > > >
> > > > Sorry, It seem I have not posted erase region changes (only those
> > > > regarding DUAL/QUAD I/O).
> > > > Generally, in this flash you need to create 3 erase regions
> > > > (because in FS-S family support only  4KiB erase on parameters sector -
> eg.
> > > > 1.2.2.4 in
> > > S25FS512S).
> > > > In my case regions are:
> > > > 1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21) 2. 32 - 256 - SE_CMD
> > > (0xd8/0xdc) 3.
> > > > Rest of the flash SE_CMD (0xd8/0xdc)
> > > >
> > > > To erase whole flash you can also use CHIP_ERASE_CMD (0x60/0xC7)
> > > > command, you just need to add one more mtd partition that will
> > > > cover
> > > whole flash.
> > > >
> > >
> > > Hi Krzeminski,
> > >
> > > Do you think is there any great advantages for enable 4KB?
> > > Because for NXP(Freescale) QSPI controller, there is only support
> > > max to 16 groups command.
> > >
> > > So It's hard to give 3 groups command just for erase (0x21,0xdc and 0xc7).
> > > So we have to disable the 4kb erase and only use 256kbytes in this patch.
> > >
> > Yes, if you disable parameters sector in spi-nor framework you will
> > disable it for all spi-nor clients not only for NXP QSPI controller.
> > There are users (at least me) that relay on parameters sector functionality.
> This patch will brake it.
> >
> > Thanks,
> 
> Hi Krzeminski,
> 
> Get it.
> So do you think how about that I add a flag in dts to select it?
> The user want's disable 4kb, he can add the flag.
> 
> In spi-nor.c:
> if (of_property_read_bool(np, "spi-nor, disable-4kb")) {
> 	spansion_s25fs_disable_4kb_erase();
> }
>                 else
> ...
> 
> In dts:
> 
> &qspi {
>         num-cs = <2>;
>         bus-num = <0>;
>         status = "okay";
> 
>         qflash0: s25fs512s@0 {
>                 compatible = "spansion, s25fs512s";
> 	 spi-nor, disable-4kb
>                 #address-cells = <1>;
>                 #size-cells = <1>;
>                 spi-max-frequency = <20000000>;
>                 reg = <0>;
>         };
> 
> I think it should be a better way.
> 
> How about your think?

This looks much better - at least for me.
There are some parameters in JESD216 standard regarding parameters sector,
but unfortunately I have not investigated that. You can take a look at Cyrille series,
that adds support for JESD216  standard.

Thanks,
Marcin

> 
> Thanks.
> 

RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Yao Yuan]

On Thu, Nov 18, 2016 at 07:00 PM +0000, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
> > -----Original Message-----
> > From: Yao Yuan [mailto:yao.yuan@nxp.com]
> > Sent: Friday, November 18, 2016 5:20 AM
> > To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> > <marcin.krzeminski@nokia.com>; Han Xu <xhnjupt@gmail.com>
> > Cc: David Woodhouse <dwmw2@infradead.org>; linux-
> > kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> > han.xu@freescale.com; Brian Norris <computersforpeace@gmail.com>;
> > jagannadh.teki@gmail.com; linux-arm-kernel@lists.infradead.org
> > Subject: RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S
> > family flash
> >
> > On Thu, Nov 17, 2016 at 10:14:55AM +0000, Krzeminski, Marcin (Nokia -
> > PL/Wroclaw) wrote:
> > > > On Thu, Nov 17, 2016 at 06:50:55AM +0000, Krzeminski, Marcin
> > > > (Nokia
> > > > -
> > > > PL/Wroclaw) wrote:
> > > > > > > > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui
> > > > > > > > <B56489@freescale.com>
> > > > > > > > wrote:
> > > > > > > > > From: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > > > >
> > > > > > > > > With the physical sectors combination, S25FS-S family
> > > > > > > > > flash requires some special operations for read/write functions.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > > > > ---
> > > > > > > > >  drivers/mtd/spi-nor/spi-nor.c | 56
> > > > > > > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > > > > > >  1 file changed, 56 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb
> > > > > > > > > 100644
> > > > > > > > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > > @@ -39,6 +39,10 @@
> > > > > > > > >
> > > > > > > > >  #define SPI_NOR_MAX_ID_LEN     6
> > > > > > > > >  #define SPI_NOR_MAX_ADDR_WIDTH 4
> > > > > > > > > +/* Added for S25FS-S family flash */
> > > > > > > > > +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
> > > > > > > > > +#define CR3V_4KB_ERASE_UNABLE  0x8 #define
> > > > > > > > > +SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
> > > > > > > > >
> > > > > > > > >  struct flash_info {
> > > > > > > > >         char            *name;
> > > > > > > > > @@ -78,6 +82,7 @@ struct flash_info {  };
> > > > > > > > >
> > > > > > > > >  #define JEDEC_MFR(info)        ((info)->id[0])
> > > > > > > > > +#define EXT_JEDEC(info)        ((info)->id[5])
> > > > > > > > >
> > > > > > > > >  static const struct flash_info *spi_nor_match_id(const
> > > > > > > > > char *name);
> > > > > > > > >
> > > > > > > > > @@ -899,6 +904,7 @@ static const struct flash_info
> > spi_nor_ids[] = {
> > > > > > > > >          */
> > > > > > > > >         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 *
> > > > > > > > > 1024, 64,
> > > > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > > >         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 *
> > > > > > > > > 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > > > +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 *
> > > > > > > > > + 1024, 512, 0)},
> > > > > > > > >         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
> > > > > > > > >         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 *
> > > > > > > > > 1024, 512,
> > > > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > > >         { "s25fl512s",  INFO(0x010220, 0x4d00, 256 *
> > > > > > > > > 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> @@
> > > > > > > > > -
> > 1036,6
> > > > > > +1042,50
> > > > > > > > @@ static const struct flash_info *spi_nor_read_id(struct
> > > > > > > > spi_nor
> > > > > > > > *nor)
> > > > > > > > >         return ERR_PTR(-ENODEV);  }
> > > > > > > > >
> > > > > > > > > +/*
> > > > > > > > > + * The S25FS-S family physical sectors may be
> > > > > > > > > +configured as a
> > > > > > > > > + * hybrid combination of eight 4-kB parameter sectors
> > > > > > > > > + * at the top or bottom of the address space with all
> > > > > > > > > + * but one of the remaining sectors being uniform size.
> > > > > > > > > + * The Parameter Sector Erase commands (20h or 21h)
> > > > > > > > > +must
> > > > > > > > > + * be used to erase the 4-kB parameter sectors individually.
> > > > > > > > > + * The Sector (uniform sector) Erase commands (D8h or
> > > > > > > > > +DCh)
> > > > > > > > > + * must be used to erase any of the remaining
> > > > > > > > > + * sectors, including the portion of highest or lowest
> > > > > > > > > +address
> > > > > > > > > + * sector that is not overlaid by the parameter sectors.
> > > > > > > > > + * The uniform sector erase command has no effect on
> > > > > > > > > +parameter
> > > > > > > > sectors.
> > > > > > > > > + */
> > > > > > > > > +static int spansion_s25fs_disable_4kb_erase(struct
> > > > > > > > > +spi_nor
> > *nor) {
> > > > > > > > > +       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
> > > > > > > > > +       u8 cr3v = 0x0;
> > > > > > > > > +       int ret = 0x0;
> > > > > > > > > +
> > > > > > > > > +       nor->cmd_buf[2] = cr3v_addr >> 16;
> > > > > > > > > +       nor->cmd_buf[1] = cr3v_addr >> 8;
> > > > > > > > > +       nor->cmd_buf[0] = cr3v_addr >> 0;
> > > > > > > > > +
> > > > > > > > > +       ret = nor->read_reg(nor,
> > > > > > > > > + SPINOR_OP_SPANSION_RDAR,
> > > > &cr3v, 1);
> > > > > > > > > +       if (ret)
> > > > > > > > > +               return ret;
> > > > > > > > > +       if (cr3v & CR3V_4KB_ERASE_UNABLE)
> > > > > > > > > +               return 0;
> > > > > > > > > +       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> > > > > > > > > +       if (ret)
> > > > > > > > > +               return ret;
> > > > > > > > > +       cr3v = CR3V_4KB_ERASE_UNABLE;
> > > > > > > > > +       nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
> > > > > > > > > +       nor->write(nor, cr3v_addr, 1, &cr3v);
> > > > > > > > > +
> > > > > > > > > +       ret = nor->read_reg(nor,
> > > > > > > > > + SPINOR_OP_SPANSION_RDAR,
> > > > &cr3v, 1);
> > > > > > > > > +       if (ret)
> > > > > > > > > +               return ret;
> > > > > > > > > +       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> > > > > > > > > +               return -EPERM;
> > > > > > > > > +
> > > > > > > > > +       return 0;
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > >  static int spi_nor_read(struct mtd_info *mtd, loff_t
> > > > > > > > > from, size_t
> > > > len,
> > > > > > > > >                         size_t *retlen, u_char *buf)  {
> > > > > > > > > @@
> > > > > > > > > -1361,6
> > > > > > > > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const
> > > > > > > > > +char *name,
> > > > > > > > enum read_mode mode)
> > > > > > > > >                 spi_nor_wait_till_ready(nor);
> > > > > > > > >         }
> > > > > > > > >
> > > > > > > > > +       if (EXT_JEDEC(info) ==
> > > > > > > > > + SPINOR_S25FS_FAMILY_EXT_JEDEC)
> > {
> > > > > > > > > +               ret = spansion_s25fs_disable_4kb_erase(nor);
> > > > > > > > > +               if (ret)
> > > > > > > > > +                       return ret;
> > > > > > > > > +       }
> > > > > > > > > +
> > > > > > > > >         if (!mtd->name)
> > > > > > > > >                 mtd->name = dev_name(dev);
> > > > > > > > >         mtd->priv = nor;
> > > > > > > > > --
> > > > > > > > > 2.1.0.27.g96db324
> > > > > > > > >
> > > > > > > > >
> > > > > > > > Hi Brian, I will ack this change but still feel it's kind of hacking code.
> > > > > > > >
> > > > > > > > Acked-by: Han xu <han.xu@nxp.com>
> > > > > > >
> > > > > > > I am new on the list so I am not sure if this topic has been discussed.
> > > > > > > Generally our product functionality relay on those 4KiB sectors.
> > > > > > > I know that this hack is already in u-boot, but if you
> > > > > > > mainstream this you will force users of those 4KiB sectors
> > > > > > > to do hack the
> > > hack...
> > > > > > > I believe the proper solution here is to use erase regions
> > > > > > > functionality, I send and RFS about that some time ago.
> > > > > >
> > > > > > Do you mind to send me a link for reference?
> > > > > >
> > > > > Han,
> > > > >
> > > > > Sorry, It seem I have not posted erase region changes (only
> > > > > those regarding DUAL/QUAD I/O).
> > > > > Generally, in this flash you need to create 3 erase regions
> > > > > (because in FS-S family support only  4KiB erase on parameters
> > > > > sector -
> > eg.
> > > > > 1.2.2.4 in
> > > > S25FS512S).
> > > > > In my case regions are:
> > > > > 1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21) 2. 32 - 256 - SE_CMD
> > > > (0xd8/0xdc) 3.
> > > > > Rest of the flash SE_CMD (0xd8/0xdc)
> > > > >
> > > > > To erase whole flash you can also use CHIP_ERASE_CMD (0x60/0xC7)
> > > > > command, you just need to add one more mtd partition that will
> > > > > cover
> > > > whole flash.
> > > > >
> > > >
> > > > Hi Krzeminski,
> > > >
> > > > Do you think is there any great advantages for enable 4KB?
> > > > Because for NXP(Freescale) QSPI controller, there is only support
> > > > max to 16 groups command.
> > > >
> > > > So It's hard to give 3 groups command just for erase (0x21,0xdc and 0xc7).
> > > > So we have to disable the 4kb erase and only use 256kbytes in this patch.
> > > >
> > > Yes, if you disable parameters sector in spi-nor framework you will
> > > disable it for all spi-nor clients not only for NXP QSPI controller.
> > > There are users (at least me) that relay on parameters sector functionality.
> > This patch will brake it.
> > >
> > > Thanks,
> >
> > Hi Krzeminski,
> >
> > Get it.
> > So do you think how about that I add a flag in dts to select it?
> > The user want's disable 4kb, he can add the flag.
> >
> > In spi-nor.c:
> > if (of_property_read_bool(np, "spi-nor, disable-4kb")) {
> > 	spansion_s25fs_disable_4kb_erase();
> > }
> >                 else
> > ...
> >
> > In dts:
> >
> > &qspi {
> >         num-cs = <2>;
> >         bus-num = <0>;
> >         status = "okay";
> >
> >         qflash0: s25fs512s@0 {
> >                 compatible = "spansion, s25fs512s";
> > 	 spi-nor, disable-4kb
> >                 #address-cells = <1>;
> >                 #size-cells = <1>;
> >                 spi-max-frequency = <20000000>;
> >                 reg = <0>;
> >         };
> >
> > I think it should be a better way.
> >
> > How about your think?
> 
> This looks much better - at least for me.
> There are some parameters in JESD216 standard regarding parameters sector,
> but unfortunately I have not investigated that. You can take a look at Cyrille
> series, that adds support for JESD216  standard.
> 

Ok, I will resend v4 for add this.

BTW, the 4-kytes block for S25FS is just only the first 8 blocks, the other block is 256kytes.
Do out SPI-NOR support erase those specific combination?

If not, do you have any plan for add it?
It seems I can't fine the support in spi-nor.

Thanks.

RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

> -----Original Message-----
> From: Yao Yuan [mailto:yao.yuan@nxp.com]
> Sent: Monday, November 21, 2016 7:28 AM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <marcin.krzeminski@nokia.com>; Han Xu <xhnjupt@gmail.com>
> Cc: David Woodhouse <dwmw2@infradead.org>; linux-
> kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> han.xu@freescale.com; Brian Norris <computersforpeace@gmail.com>;
> jagannadh.teki@gmail.com; linux-arm-kernel@lists.infradead.org; Cyrille
> Pitchen <cyrille.pitchen@atmel.com>
> Subject: RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family
> flash
> 
> On Thu, Nov 18, 2016 at 07:00 PM +0000, Krzeminski, Marcin (Nokia -
> PL/Wroclaw) wrote:
> > > -----Original Message-----
> > > From: Yao Yuan [mailto:yao.yuan@nxp.com]
> > > Sent: Friday, November 18, 2016 5:20 AM
> > > To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> > > <marcin.krzeminski@nokia.com>; Han Xu <xhnjupt@gmail.com>
> > > Cc: David Woodhouse <dwmw2@infradead.org>; linux-
> > > kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> > > han.xu@freescale.com; Brian Norris <computersforpeace@gmail.com>;
> > > jagannadh.teki@gmail.com; linux-arm-kernel@lists.infradead.org
> > > Subject: RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S
> > > family flash
> > >
> > > On Thu, Nov 17, 2016 at 10:14:55AM +0000, Krzeminski, Marcin (Nokia
> > > -
> > > PL/Wroclaw) wrote:
> > > > > On Thu, Nov 17, 2016 at 06:50:55AM +0000, Krzeminski, Marcin
> > > > > (Nokia
> > > > > -
> > > > > PL/Wroclaw) wrote:
> > > > > > > > > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui
> > > > > > > > > <B56489@freescale.com>
> > > > > > > > > wrote:
> > > > > > > > > > From: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > > > > >
> > > > > > > > > > With the physical sectors combination, S25FS-S family
> > > > > > > > > > flash requires some special operations for read/write
> functions.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > > > > > ---
> > > > > > > > > >  drivers/mtd/spi-nor/spi-nor.c | 56
> > > > > > > > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > > > > > > >  1 file changed, 56 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb
> > > > > > > > > > 100644
> > > > > > > > > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > > > @@ -39,6 +39,10 @@
> > > > > > > > > >
> > > > > > > > > >  #define SPI_NOR_MAX_ID_LEN     6
> > > > > > > > > >  #define SPI_NOR_MAX_ADDR_WIDTH 4
> > > > > > > > > > +/* Added for S25FS-S family flash */
> > > > > > > > > > +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
> > > > > > > > > > +#define CR3V_4KB_ERASE_UNABLE  0x8 #define
> > > > > > > > > > +SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
> > > > > > > > > >
> > > > > > > > > >  struct flash_info {
> > > > > > > > > >         char            *name;
> > > > > > > > > > @@ -78,6 +82,7 @@ struct flash_info {  };
> > > > > > > > > >
> > > > > > > > > >  #define JEDEC_MFR(info)        ((info)->id[0])
> > > > > > > > > > +#define EXT_JEDEC(info)        ((info)->id[5])
> > > > > > > > > >
> > > > > > > > > >  static const struct flash_info
> > > > > > > > > > *spi_nor_match_id(const char *name);
> > > > > > > > > >
> > > > > > > > > > @@ -899,6 +904,7 @@ static const struct flash_info
> > > spi_nor_ids[] = {
> > > > > > > > > >          */
> > > > > > > > > >         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 *
> > > > > > > > > > 1024, 64,
> > > > > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > > > >         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 *
> > > > > > > > > > 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> },
> > > > > > > > > > +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 *
> > > > > > > > > > + 1024, 512, 0)},
> > > > > > > > > >         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128,
> 0) },
> > > > > > > > > >         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 *
> > > > > > > > > > 1024, 512,
> > > > > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > > > >         { "s25fl512s",  INFO(0x010220, 0x4d00, 256 *
> > > > > > > > > > 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> },
> > @@
> > > > > > > > > > -
> > > 1036,6
> > > > > > > +1042,50
> > > > > > > > > @@ static const struct flash_info
> > > > > > > > > *spi_nor_read_id(struct spi_nor
> > > > > > > > > *nor)
> > > > > > > > > >         return ERR_PTR(-ENODEV);  }
> > > > > > > > > >
> > > > > > > > > > +/*
> > > > > > > > > > + * The S25FS-S family physical sectors may be
> > > > > > > > > > +configured as a
> > > > > > > > > > + * hybrid combination of eight 4-kB parameter sectors
> > > > > > > > > > + * at the top or bottom of the address space with all
> > > > > > > > > > + * but one of the remaining sectors being uniform size.
> > > > > > > > > > + * The Parameter Sector Erase commands (20h or 21h)
> > > > > > > > > > +must
> > > > > > > > > > + * be used to erase the 4-kB parameter sectors individually.
> > > > > > > > > > + * The Sector (uniform sector) Erase commands (D8h or
> > > > > > > > > > +DCh)
> > > > > > > > > > + * must be used to erase any of the remaining
> > > > > > > > > > + * sectors, including the portion of highest or
> > > > > > > > > > +lowest address
> > > > > > > > > > + * sector that is not overlaid by the parameter sectors.
> > > > > > > > > > + * The uniform sector erase command has no effect on
> > > > > > > > > > +parameter
> > > > > > > > > sectors.
> > > > > > > > > > + */
> > > > > > > > > > +static int spansion_s25fs_disable_4kb_erase(struct
> > > > > > > > > > +spi_nor
> > > *nor) {
> > > > > > > > > > +       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
> > > > > > > > > > +       u8 cr3v = 0x0;
> > > > > > > > > > +       int ret = 0x0;
> > > > > > > > > > +
> > > > > > > > > > +       nor->cmd_buf[2] = cr3v_addr >> 16;
> > > > > > > > > > +       nor->cmd_buf[1] = cr3v_addr >> 8;
> > > > > > > > > > +       nor->cmd_buf[0] = cr3v_addr >> 0;
> > > > > > > > > > +
> > > > > > > > > > +       ret = nor->read_reg(nor,
> > > > > > > > > > + SPINOR_OP_SPANSION_RDAR,
> > > > > &cr3v, 1);
> > > > > > > > > > +       if (ret)
> > > > > > > > > > +               return ret;
> > > > > > > > > > +       if (cr3v & CR3V_4KB_ERASE_UNABLE)
> > > > > > > > > > +               return 0;
> > > > > > > > > > +       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> > > > > > > > > > +       if (ret)
> > > > > > > > > > +               return ret;
> > > > > > > > > > +       cr3v = CR3V_4KB_ERASE_UNABLE;
> > > > > > > > > > +       nor->program_opcode =
> SPINOR_OP_SPANSION_WRAR;
> > > > > > > > > > +       nor->write(nor, cr3v_addr, 1, &cr3v);
> > > > > > > > > > +
> > > > > > > > > > +       ret = nor->read_reg(nor,
> > > > > > > > > > + SPINOR_OP_SPANSION_RDAR,
> > > > > &cr3v, 1);
> > > > > > > > > > +       if (ret)
> > > > > > > > > > +               return ret;
> > > > > > > > > > +       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> > > > > > > > > > +               return -EPERM;
> > > > > > > > > > +
> > > > > > > > > > +       return 0;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > >  static int spi_nor_read(struct mtd_info *mtd, loff_t
> > > > > > > > > > from, size_t
> > > > > len,
> > > > > > > > > >                         size_t *retlen, u_char *buf)
> > > > > > > > > > { @@
> > > > > > > > > > -1361,6
> > > > > > > > > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor,
> > > > > > > > > > +const char *name,
> > > > > > > > > enum read_mode mode)
> > > > > > > > > >                 spi_nor_wait_till_ready(nor);
> > > > > > > > > >         }
> > > > > > > > > >
> > > > > > > > > > +       if (EXT_JEDEC(info) ==
> > > > > > > > > > + SPINOR_S25FS_FAMILY_EXT_JEDEC)
> > > {
> > > > > > > > > > +               ret = spansion_s25fs_disable_4kb_erase(nor);
> > > > > > > > > > +               if (ret)
> > > > > > > > > > +                       return ret;
> > > > > > > > > > +       }
> > > > > > > > > > +
> > > > > > > > > >         if (!mtd->name)
> > > > > > > > > >                 mtd->name = dev_name(dev);
> > > > > > > > > >         mtd->priv = nor;
> > > > > > > > > > --
> > > > > > > > > > 2.1.0.27.g96db324
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > Hi Brian, I will ack this change but still feel it's kind of hacking
> code.
> > > > > > > > >
> > > > > > > > > Acked-by: Han xu <han.xu@nxp.com>
> > > > > > > >
> > > > > > > > I am new on the list so I am not sure if this topic has been
> discussed.
> > > > > > > > Generally our product functionality relay on those 4KiB sectors.
> > > > > > > > I know that this hack is already in u-boot, but if you
> > > > > > > > mainstream this you will force users of those 4KiB sectors
> > > > > > > > to do hack the
> > > > hack...
> > > > > > > > I believe the proper solution here is to use erase regions
> > > > > > > > functionality, I send and RFS about that some time ago.
> > > > > > >
> > > > > > > Do you mind to send me a link for reference?
> > > > > > >
> > > > > > Han,
> > > > > >
> > > > > > Sorry, It seem I have not posted erase region changes (only
> > > > > > those regarding DUAL/QUAD I/O).
> > > > > > Generally, in this flash you need to create 3 erase regions
> > > > > > (because in FS-S family support only  4KiB erase on parameters
> > > > > > sector -
> > > eg.
> > > > > > 1.2.2.4 in
> > > > > S25FS512S).
> > > > > > In my case regions are:
> > > > > > 1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21) 2. 32 - 256 -
> > > > > > SE_CMD
> > > > > (0xd8/0xdc) 3.
> > > > > > Rest of the flash SE_CMD (0xd8/0xdc)
> > > > > >
> > > > > > To erase whole flash you can also use CHIP_ERASE_CMD
> > > > > > (0x60/0xC7) command, you just need to add one more mtd
> > > > > > partition that will cover
> > > > > whole flash.
> > > > > >
> > > > >
> > > > > Hi Krzeminski,
> > > > >
> > > > > Do you think is there any great advantages for enable 4KB?
> > > > > Because for NXP(Freescale) QSPI controller, there is only
> > > > > support max to 16 groups command.
> > > > >
> > > > > So It's hard to give 3 groups command just for erase (0x21,0xdc and
> 0xc7).
> > > > > So we have to disable the 4kb erase and only use 256kbytes in this
> patch.
> > > > >
> > > > Yes, if you disable parameters sector in spi-nor framework you
> > > > will disable it for all spi-nor clients not only for NXP QSPI controller.
> > > > There are users (at least me) that relay on parameters sector
> functionality.
> > > This patch will brake it.
> > > >
> > > > Thanks,
> > >
> > > Hi Krzeminski,
> > >
> > > Get it.
> > > So do you think how about that I add a flag in dts to select it?
> > > The user want's disable 4kb, he can add the flag.
> > >
> > > In spi-nor.c:
> > > if (of_property_read_bool(np, "spi-nor, disable-4kb")) {
> > > 	spansion_s25fs_disable_4kb_erase();
> > > }
> > >                 else
> > > ...
> > >
> > > In dts:
> > >
> > > &qspi {
> > >         num-cs = <2>;
> > >         bus-num = <0>;
> > >         status = "okay";
> > >
> > >         qflash0: s25fs512s@0 {
> > >                 compatible = "spansion, s25fs512s";
> > > 	 spi-nor, disable-4kb
> > >                 #address-cells = <1>;
> > >                 #size-cells = <1>;
> > >                 spi-max-frequency = <20000000>;
> > >                 reg = <0>;
> > >         };
> > >
> > > I think it should be a better way.
> > >
> > > How about your think?
> >
> > This looks much better - at least for me.
> > There are some parameters in JESD216 standard regarding parameters
> > sector, but unfortunately I have not investigated that. You can take a
> > look at Cyrille series, that adds support for JESD216  standard.
> >
> 
> Ok, I will resend v4 for add this.
> 
> BTW, the 4-kytes block for S25FS is just only the first 8 blocks, the other block
> is 256kytes.
> Do out SPI-NOR support erase those specific combination?
> 
> If not, do you have any plan for add it?
> It seems I can't fine the support in spi-nor.
> 
Those erase regions are solution for such flash, current upstream version does not
have this. My solution is not universal, so probably I will not add it.

Regards,
Marcin

> Thanks.

RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Yao Yuan]

On Thu, Nov 21, 2016 at 03:15 PM +0800, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
> > -----Original Message-----
> > From: Yao Yuan [mailto:yao.yuan@nxp.com]
> > Sent: Monday, November 21, 2016 7:28 AM
> > To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> > <marcin.krzeminski@nokia.com>; Han Xu <xhnjupt@gmail.com>
> > Cc: David Woodhouse <dwmw2@infradead.org>; linux-
> > kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> > han.xu@freescale.com; Brian Norris <computersforpeace@gmail.com>;
> > jagannadh.teki@gmail.com; linux-arm-kernel@lists.infradead.org;
> > Cyrille Pitchen <cyrille.pitchen@atmel.com>
> > Subject: RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S
> > family flash
> >
> > On Thu, Nov 18, 2016 at 07:00 PM +0000, Krzeminski, Marcin (Nokia -
> > PL/Wroclaw) wrote:
> > > > -----Original Message-----
> > > > From: Yao Yuan [mailto:yao.yuan@nxp.com]
> > > > Sent: Friday, November 18, 2016 5:20 AM
> > > > To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> > > > <marcin.krzeminski@nokia.com>; Han Xu <xhnjupt@gmail.com>
> > > > Cc: David Woodhouse <dwmw2@infradead.org>; linux-
> > > > kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> > > > han.xu@freescale.com; Brian Norris <computersforpeace@gmail.com>;
> > > > jagannadh.teki@gmail.com; linux-arm-kernel@lists.infradead.org
> > > > Subject: RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S
> > > > family flash
> > > >
> > > > On Thu, Nov 17, 2016 at 10:14:55AM +0000, Krzeminski, Marcin
> > > > (Nokia
> > > > -
> > > > PL/Wroclaw) wrote:
> > > > > > On Thu, Nov 17, 2016 at 06:50:55AM +0000, Krzeminski, Marcin
> > > > > > (Nokia
> > > > > > -
> > > > > > PL/Wroclaw) wrote:
> > > > > > > > > > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui
> > > > > > > > > > <B56489@freescale.com>
> > > > > > > > > > wrote:
> > > > > > > > > > > From: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > > > > > >
> > > > > > > > > > > With the physical sectors combination, S25FS-S
> > > > > > > > > > > family flash requires some special operations for
> > > > > > > > > > > read/write
> > functions.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > > > > > > ---
> > > > > > > > > > >  drivers/mtd/spi-nor/spi-nor.c | 56
> > > > > > > > > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > > > > > > > >  1 file changed, 56 insertions(+)
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > > > > b/drivers/mtd/spi-nor/spi-nor.c index
> > > > > > > > > > > d0fc165..495d0bb
> > > > > > > > > > > 100644
> > > > > > > > > > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > > > > @@ -39,6 +39,10 @@
> > > > > > > > > > >
> > > > > > > > > > >  #define SPI_NOR_MAX_ID_LEN     6
> > > > > > > > > > >  #define SPI_NOR_MAX_ADDR_WIDTH 4
> > > > > > > > > > > +/* Added for S25FS-S family flash */
> > > > > > > > > > > +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
> > > > > > > > > > > +#define CR3V_4KB_ERASE_UNABLE  0x8 #define
> > > > > > > > > > > +SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
> > > > > > > > > > >
> > > > > > > > > > >  struct flash_info {
> > > > > > > > > > >         char            *name;
> > > > > > > > > > > @@ -78,6 +82,7 @@ struct flash_info {  };
> > > > > > > > > > >
> > > > > > > > > > >  #define JEDEC_MFR(info)        ((info)->id[0])
> > > > > > > > > > > +#define EXT_JEDEC(info)        ((info)->id[5])
> > > > > > > > > > >
> > > > > > > > > > >  static const struct flash_info
> > > > > > > > > > > *spi_nor_match_id(const char *name);
> > > > > > > > > > >
> > > > > > > > > > > @@ -899,6 +904,7 @@ static const struct flash_info
> > > > spi_nor_ids[] = {
> > > > > > > > > > >          */
> > > > > > > > > > >         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 *
> > > > > > > > > > > 1024, 64,
> > > > > > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > > > > >         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 *
> > > > > > > > > > > 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> > },
> > > > > > > > > > > +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64
> > > > > > > > > > > + * 1024, 512, 0)},
> > > > > > > > > > >         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 *
> > > > > > > > > > > 1024, 128,
> > 0) },
> > > > > > > > > > >         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 *
> > > > > > > > > > > 1024, 512,
> > > > > > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > > > > >         { "s25fl512s",  INFO(0x010220, 0x4d00, 256 *
> > > > > > > > > > > 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> > },
> > > @@
> > > > > > > > > > > -
> > > > 1036,6
> > > > > > > > +1042,50
> > > > > > > > > > @@ static const struct flash_info
> > > > > > > > > > *spi_nor_read_id(struct spi_nor
> > > > > > > > > > *nor)
> > > > > > > > > > >         return ERR_PTR(-ENODEV);  }
> > > > > > > > > > >
> > > > > > > > > > > +/*
> > > > > > > > > > > + * The S25FS-S family physical sectors may be
> > > > > > > > > > > +configured as a
> > > > > > > > > > > + * hybrid combination of eight 4-kB parameter
> > > > > > > > > > > +sectors
> > > > > > > > > > > + * at the top or bottom of the address space with
> > > > > > > > > > > +all
> > > > > > > > > > > + * but one of the remaining sectors being uniform size.
> > > > > > > > > > > + * The Parameter Sector Erase commands (20h or 21h)
> > > > > > > > > > > +must
> > > > > > > > > > > + * be used to erase the 4-kB parameter sectors individually.
> > > > > > > > > > > + * The Sector (uniform sector) Erase commands (D8h
> > > > > > > > > > > +or
> > > > > > > > > > > +DCh)
> > > > > > > > > > > + * must be used to erase any of the remaining
> > > > > > > > > > > + * sectors, including the portion of highest or
> > > > > > > > > > > +lowest address
> > > > > > > > > > > + * sector that is not overlaid by the parameter sectors.
> > > > > > > > > > > + * The uniform sector erase command has no effect
> > > > > > > > > > > +on parameter
> > > > > > > > > > sectors.
> > > > > > > > > > > + */
> > > > > > > > > > > +static int spansion_s25fs_disable_4kb_erase(struct
> > > > > > > > > > > +spi_nor
> > > > *nor) {
> > > > > > > > > > > +       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
> > > > > > > > > > > +       u8 cr3v = 0x0;
> > > > > > > > > > > +       int ret = 0x0;
> > > > > > > > > > > +
> > > > > > > > > > > +       nor->cmd_buf[2] = cr3v_addr >> 16;
> > > > > > > > > > > +       nor->cmd_buf[1] = cr3v_addr >> 8;
> > > > > > > > > > > +       nor->cmd_buf[0] = cr3v_addr >> 0;
> > > > > > > > > > > +
> > > > > > > > > > > +       ret = nor->read_reg(nor,
> > > > > > > > > > > + SPINOR_OP_SPANSION_RDAR,
> > > > > > &cr3v, 1);
> > > > > > > > > > > +       if (ret)
> > > > > > > > > > > +               return ret;
> > > > > > > > > > > +       if (cr3v & CR3V_4KB_ERASE_UNABLE)
> > > > > > > > > > > +               return 0;
> > > > > > > > > > > +       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> > > > > > > > > > > +       if (ret)
> > > > > > > > > > > +               return ret;
> > > > > > > > > > > +       cr3v = CR3V_4KB_ERASE_UNABLE;
> > > > > > > > > > > +       nor->program_opcode =
> > SPINOR_OP_SPANSION_WRAR;
> > > > > > > > > > > +       nor->write(nor, cr3v_addr, 1, &cr3v);
> > > > > > > > > > > +
> > > > > > > > > > > +       ret = nor->read_reg(nor,
> > > > > > > > > > > + SPINOR_OP_SPANSION_RDAR,
> > > > > > &cr3v, 1);
> > > > > > > > > > > +       if (ret)
> > > > > > > > > > > +               return ret;
> > > > > > > > > > > +       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> > > > > > > > > > > +               return -EPERM;
> > > > > > > > > > > +
> > > > > > > > > > > +       return 0;
> > > > > > > > > > > +}
> > > > > > > > > > > +
> > > > > > > > > > >  static int spi_nor_read(struct mtd_info *mtd,
> > > > > > > > > > > loff_t from, size_t
> > > > > > len,
> > > > > > > > > > >                         size_t *retlen, u_char *buf)
> > > > > > > > > > > { @@
> > > > > > > > > > > -1361,6
> > > > > > > > > > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor,
> > > > > > > > > > > +const char *name,
> > > > > > > > > > enum read_mode mode)
> > > > > > > > > > >                 spi_nor_wait_till_ready(nor);
> > > > > > > > > > >         }
> > > > > > > > > > >
> > > > > > > > > > > +       if (EXT_JEDEC(info) ==
> > > > > > > > > > > + SPINOR_S25FS_FAMILY_EXT_JEDEC)
> > > > {
> > > > > > > > > > > +               ret = spansion_s25fs_disable_4kb_erase(nor);
> > > > > > > > > > > +               if (ret)
> > > > > > > > > > > +                       return ret;
> > > > > > > > > > > +       }
> > > > > > > > > > > +
> > > > > > > > > > >         if (!mtd->name)
> > > > > > > > > > >                 mtd->name = dev_name(dev);
> > > > > > > > > > >         mtd->priv = nor;
> > > > > > > > > > > --
> > > > > > > > > > > 2.1.0.27.g96db324
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > Hi Brian, I will ack this change but still feel it's
> > > > > > > > > > kind of hacking
> > code.
> > > > > > > > > >
> > > > > > > > > > Acked-by: Han xu <han.xu@nxp.com>
> > > > > > > > >
> > > > > > > > > I am new on the list so I am not sure if this topic has
> > > > > > > > > been
> > discussed.
> > > > > > > > > Generally our product functionality relay on those 4KiB sectors.
> > > > > > > > > I know that this hack is already in u-boot, but if you
> > > > > > > > > mainstream this you will force users of those 4KiB
> > > > > > > > > sectors to do hack the
> > > > > hack...
> > > > > > > > > I believe the proper solution here is to use erase
> > > > > > > > > regions functionality, I send and RFS about that some time ago.
> > > > > > > >
> > > > > > > > Do you mind to send me a link for reference?
> > > > > > > >
> > > > > > > Han,
> > > > > > >
> > > > > > > Sorry, It seem I have not posted erase region changes (only
> > > > > > > those regarding DUAL/QUAD I/O).
> > > > > > > Generally, in this flash you need to create 3 erase regions
> > > > > > > (because in FS-S family support only  4KiB erase on
> > > > > > > parameters sector -
> > > > eg.
> > > > > > > 1.2.2.4 in
> > > > > > S25FS512S).
> > > > > > > In my case regions are:
> > > > > > > 1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21) 2. 32 - 256 -
> > > > > > > SE_CMD
> > > > > > (0xd8/0xdc) 3.
> > > > > > > Rest of the flash SE_CMD (0xd8/0xdc)
> > > > > > >
> > > > > > > To erase whole flash you can also use CHIP_ERASE_CMD
> > > > > > > (0x60/0xC7) command, you just need to add one more mtd
> > > > > > > partition that will cover
> > > > > > whole flash.
> > > > > > >
> > > > > >
> > > > > > Hi Krzeminski,
> > > > > >
> > > > > > Do you think is there any great advantages for enable 4KB?
> > > > > > Because for NXP(Freescale) QSPI controller, there is only
> > > > > > support max to 16 groups command.
> > > > > >
> > > > > > So It's hard to give 3 groups command just for erase
> > > > > > (0x21,0xdc and
> > 0xc7).
> > > > > > So we have to disable the 4kb erase and only use 256kbytes in
> > > > > > this
> > patch.
> > > > > >
> > > > > Yes, if you disable parameters sector in spi-nor framework you
> > > > > will disable it for all spi-nor clients not only for NXP QSPI controller.
> > > > > There are users (at least me) that relay on parameters sector
> > functionality.
> > > > This patch will brake it.
> > > > >
> > > > > Thanks,
> > > >
> > > > Hi Krzeminski,
> > > >
> > > > Get it.
> > > > So do you think how about that I add a flag in dts to select it?
> > > > The user want's disable 4kb, he can add the flag.
> > > >
> > > > In spi-nor.c:
> > > > if (of_property_read_bool(np, "spi-nor, disable-4kb")) {
> > > > 	spansion_s25fs_disable_4kb_erase();
> > > > }
> > > >                 else
> > > > ...
> > > >
> > > > In dts:
> > > >
> > > > &qspi {
> > > >         num-cs = <2>;
> > > >         bus-num = <0>;
> > > >         status = "okay";
> > > >
> > > >         qflash0: s25fs512s@0 {
> > > >                 compatible = "spansion, s25fs512s";
> > > > 	 spi-nor, disable-4kb
> > > >                 #address-cells = <1>;
> > > >                 #size-cells = <1>;
> > > >                 spi-max-frequency = <20000000>;
> > > >                 reg = <0>;
> > > >         };
> > > >
> > > > I think it should be a better way.
> > > >
> > > > How about your think?
> > >
> > > This looks much better - at least for me.
> > > There are some parameters in JESD216 standard regarding parameters
> > > sector, but unfortunately I have not investigated that. You can take
> > > a look at Cyrille series, that adds support for JESD216  standard.
> > >
> >
> > Ok, I will resend v4 for add this.
> >
> > BTW, the 4-kytes block for S25FS is just only the first 8 blocks, the
> > other block is 256kytes.
> > Do out SPI-NOR support erase those specific combination?
> >
> > If not, do you have any plan for add it?
> > It seems I can't fine the support in spi-nor.
> >
> Those erase regions are solution for such flash, current upstream version does
> not have this. My solution is not universal, so probably I will not add it.
> 

Ok, Get it.
We will not use the specific 4kb combination mode for S25FS also.
So I will add the patch to disable 4kb for S25FS. But I will add a flag in dts that
the user can do the choice.

Thanks.

Re: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Han Xu]

On Thu, Nov 17, 2016 at 3:14 AM, Yao Yuan <yao.yuan@nxp.com> wrote:
> On Thu, Nov 17, 2016 at 06:50:55AM +0000, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
>> > > > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui <B56489@freescale.com>
>> > > > wrote:
>> > > > > From: Yunhui Cui <yunhui.cui@nxp.com>
>> > > > >
>> > > > > With the physical sectors combination, S25FS-S family flash
>> > > > > requires some special operations for read/write functions.
>> > > > >
>> > > > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
>> > > > > ---
>> > > > >  drivers/mtd/spi-nor/spi-nor.c | 56
>> > > > > +++++++++++++++++++++++++++++++++++++++++++
>> > > > >  1 file changed, 56 insertions(+)
>> > > > >
>> > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
>> > > > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb 100644
>> > > > > --- a/drivers/mtd/spi-nor/spi-nor.c
>> > > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
>> > > > > @@ -39,6 +39,10 @@
>> > > > >
>> > > > >  #define SPI_NOR_MAX_ID_LEN     6
>> > > > >  #define SPI_NOR_MAX_ADDR_WIDTH 4
>> > > > > +/* Added for S25FS-S family flash */
>> > > > > +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
>> > > > > +#define CR3V_4KB_ERASE_UNABLE  0x8 #define
>> > > > > +SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
>> > > > >
>> > > > >  struct flash_info {
>> > > > >         char            *name;
>> > > > > @@ -78,6 +82,7 @@ struct flash_info {  };
>> > > > >
>> > > > >  #define JEDEC_MFR(info)        ((info)->id[0])
>> > > > > +#define EXT_JEDEC(info)        ((info)->id[5])
>> > > > >
>> > > > >  static const struct flash_info *spi_nor_match_id(const char
>> > > > > *name);
>> > > > >
>> > > > > @@ -899,6 +904,7 @@ static const struct flash_info spi_nor_ids[] = {
>> > > > >          */
>> > > > >         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,  64,
>> > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>> > > > >         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128,
>> > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>> > > > > +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024,
>> > > > > + 512, 0)},
>> > > > >         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
>> > > > >         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512,
>> > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>> > > > >         { "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256,
>> > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -1036,6
>> > +1042,50
>> > > > @@ static const struct flash_info *spi_nor_read_id(struct spi_nor
>> > > > *nor)
>> > > > >         return ERR_PTR(-ENODEV);  }
>> > > > >
>> > > > > +/*
>> > > > > + * The S25FS-S family physical sectors may be configured as a
>> > > > > + * hybrid combination of eight 4-kB parameter sectors
>> > > > > + * at the top or bottom of the address space with all
>> > > > > + * but one of the remaining sectors being uniform size.
>> > > > > + * The Parameter Sector Erase commands (20h or 21h) must
>> > > > > + * be used to erase the 4-kB parameter sectors individually.
>> > > > > + * The Sector (uniform sector) Erase commands (D8h or DCh)
>> > > > > + * must be used to erase any of the remaining
>> > > > > + * sectors, including the portion of highest or lowest address
>> > > > > + * sector that is not overlaid by the parameter sectors.
>> > > > > + * The uniform sector erase command has no effect on parameter
>> > > > sectors.
>> > > > > + */
>> > > > > +static int spansion_s25fs_disable_4kb_erase(struct spi_nor *nor) {
>> > > > > +       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
>> > > > > +       u8 cr3v = 0x0;
>> > > > > +       int ret = 0x0;
>> > > > > +
>> > > > > +       nor->cmd_buf[2] = cr3v_addr >> 16;
>> > > > > +       nor->cmd_buf[1] = cr3v_addr >> 8;
>> > > > > +       nor->cmd_buf[0] = cr3v_addr >> 0;
>> > > > > +
>> > > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
>> > > > > +       if (ret)
>> > > > > +               return ret;
>> > > > > +       if (cr3v & CR3V_4KB_ERASE_UNABLE)
>> > > > > +               return 0;
>> > > > > +       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
>> > > > > +       if (ret)
>> > > > > +               return ret;
>> > > > > +       cr3v = CR3V_4KB_ERASE_UNABLE;
>> > > > > +       nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
>> > > > > +       nor->write(nor, cr3v_addr, 1, &cr3v);
>> > > > > +
>> > > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
>> > > > > +       if (ret)
>> > > > > +               return ret;
>> > > > > +       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
>> > > > > +               return -EPERM;
>> > > > > +
>> > > > > +       return 0;
>> > > > > +}
>> > > > > +
>> > > > >  static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
>> > > > >                         size_t *retlen, u_char *buf)  { @@
>> > > > > -1361,6
>> > > > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const char
>> > > > > +*name,
>> > > > enum read_mode mode)
>> > > > >                 spi_nor_wait_till_ready(nor);
>> > > > >         }
>> > > > >
>> > > > > +       if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC) {
>> > > > > +               ret = spansion_s25fs_disable_4kb_erase(nor);
>> > > > > +               if (ret)
>> > > > > +                       return ret;
>> > > > > +       }
>> > > > > +
>> > > > >         if (!mtd->name)
>> > > > >                 mtd->name = dev_name(dev);
>> > > > >         mtd->priv = nor;
>> > > > > --
>> > > > > 2.1.0.27.g96db324
>> > > > >
>> > > > >
>> > > > Hi Brian, I will ack this change but still feel it's kind of hacking code.
>> > > >
>> > > > Acked-by: Han xu <han.xu@nxp.com>
>> > >
>> > > I am new on the list so I am not sure if this topic has been discussed.
>> > > Generally our product functionality relay on those 4KiB sectors.
>> > > I know that this hack is already in u-boot, but if you mainstream
>> > > this you will force users of those 4KiB sectors to do hack the hack...
>> > > I believe the proper solution here is to use erase regions
>> > > functionality, I send and RFS about that some time ago.
>> >
>> > Do you mind to send me a link for reference?
>> >
>> Han,
>>
>> Sorry, It seem I have not posted erase region changes (only those regarding
>> DUAL/QUAD I/O).
>> Generally, in this flash you need to create 3 erase regions (because in FS-S
>> family support only  4KiB erase on parameters sector - eg. 1.2.2.4 in S25FS512S).
>> In my case regions are:
>> 1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21) 2. 32 - 256 - SE_CMD (0xd8/0xdc) 3.
>> Rest of the flash SE_CMD (0xd8/0xdc)
>>
>> To erase whole flash you can also use CHIP_ERASE_CMD (0x60/0xC7) command,
>> you just need to add one more mtd partition that will cover whole flash.
>>
>
> Hi Krzeminski,
>
> Do you think is there any great advantages for enable 4KB?
> Because for NXP(Freescale) QSPI controller, there is only support max to 16 groups command.

If it's really necessary to support all command groups, you can try
apply my dynamic lut patch first.
https://patchwork.ozlabs.org/patch/676791/

>
> So It's hard to give 3 groups command just for erase (0x21,0xdc and 0xc7).
> So we have to disable the 4kb erase and only use 256kbytes in this patch.
>
>



-- 
Sincerely,

Han XU

RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Yao Yuan]

On Thu, Nov 18, 2016 at 12:31 PM, Han Xu wrote:
> On Thu, Nov 17, 2016 at 3:14 AM, Yao Yuan <yao.yuan@nxp.com> wrote:
> > On Thu, Nov 17, 2016 at 06:50:55AM +0000, Krzeminski, Marcin (Nokia -
> PL/Wroclaw) wrote:
> >> > > > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui
> >> > > > <B56489@freescale.com>
> >> > > > wrote:
> >> > > > > From: Yunhui Cui <yunhui.cui@nxp.com>
> >> > > > >
> >> > > > > With the physical sectors combination, S25FS-S family flash
> >> > > > > requires some special operations for read/write functions.
> >> > > > >
> >> > > > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> >> > > > > ---
> >> > > > >  drivers/mtd/spi-nor/spi-nor.c | 56
> >> > > > > +++++++++++++++++++++++++++++++++++++++++++
> >> > > > >  1 file changed, 56 insertions(+)
> >> > > > >
> >> > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> >> > > > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb 100644
> >> > > > > --- a/drivers/mtd/spi-nor/spi-nor.c
> >> > > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> >> > > > > @@ -39,6 +39,10 @@
> >> > > > >
> >> > > > >  #define SPI_NOR_MAX_ID_LEN     6
> >> > > > >  #define SPI_NOR_MAX_ADDR_WIDTH 4
> >> > > > > +/* Added for S25FS-S family flash */
> >> > > > > +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
> >> > > > > +#define CR3V_4KB_ERASE_UNABLE  0x8 #define
> >> > > > > +SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
> >> > > > >
> >> > > > >  struct flash_info {
> >> > > > >         char            *name;
> >> > > > > @@ -78,6 +82,7 @@ struct flash_info {  };
> >> > > > >
> >> > > > >  #define JEDEC_MFR(info)        ((info)->id[0])
> >> > > > > +#define EXT_JEDEC(info)        ((info)->id[5])
> >> > > > >
> >> > > > >  static const struct flash_info *spi_nor_match_id(const char
> >> > > > > *name);
> >> > > > >
> >> > > > > @@ -899,6 +904,7 @@ static const struct flash_info spi_nor_ids[] = {
> >> > > > >          */
> >> > > > >         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,
> >> > > > > 64,
> >> > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> >> > > > >         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024,
> >> > > > > 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> >> > > > > +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024,
> >> > > > > + 512, 0)},
> >> > > > >         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
> >> > > > >         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024,
> >> > > > > 512,
> >> > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> >> > > > >         { "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024,
> >> > > > > 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -1036,6
> >> > +1042,50
> >> > > > @@ static const struct flash_info *spi_nor_read_id(struct
> >> > > > spi_nor
> >> > > > *nor)
> >> > > > >         return ERR_PTR(-ENODEV);  }
> >> > > > >
> >> > > > > +/*
> >> > > > > + * The S25FS-S family physical sectors may be configured as
> >> > > > > +a
> >> > > > > + * hybrid combination of eight 4-kB parameter sectors
> >> > > > > + * at the top or bottom of the address space with all
> >> > > > > + * but one of the remaining sectors being uniform size.
> >> > > > > + * The Parameter Sector Erase commands (20h or 21h) must
> >> > > > > + * be used to erase the 4-kB parameter sectors individually.
> >> > > > > + * The Sector (uniform sector) Erase commands (D8h or DCh)
> >> > > > > + * must be used to erase any of the remaining
> >> > > > > + * sectors, including the portion of highest or lowest
> >> > > > > +address
> >> > > > > + * sector that is not overlaid by the parameter sectors.
> >> > > > > + * The uniform sector erase command has no effect on
> >> > > > > +parameter
> >> > > > sectors.
> >> > > > > + */
> >> > > > > +static int spansion_s25fs_disable_4kb_erase(struct spi_nor *nor) {
> >> > > > > +       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
> >> > > > > +       u8 cr3v = 0x0;
> >> > > > > +       int ret = 0x0;
> >> > > > > +
> >> > > > > +       nor->cmd_buf[2] = cr3v_addr >> 16;
> >> > > > > +       nor->cmd_buf[1] = cr3v_addr >> 8;
> >> > > > > +       nor->cmd_buf[0] = cr3v_addr >> 0;
> >> > > > > +
> >> > > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v,
> 1);
> >> > > > > +       if (ret)
> >> > > > > +               return ret;
> >> > > > > +       if (cr3v & CR3V_4KB_ERASE_UNABLE)
> >> > > > > +               return 0;
> >> > > > > +       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> >> > > > > +       if (ret)
> >> > > > > +               return ret;
> >> > > > > +       cr3v = CR3V_4KB_ERASE_UNABLE;
> >> > > > > +       nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
> >> > > > > +       nor->write(nor, cr3v_addr, 1, &cr3v);
> >> > > > > +
> >> > > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v,
> 1);
> >> > > > > +       if (ret)
> >> > > > > +               return ret;
> >> > > > > +       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> >> > > > > +               return -EPERM;
> >> > > > > +
> >> > > > > +       return 0;
> >> > > > > +}
> >> > > > > +
> >> > > > >  static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
> >> > > > >                         size_t *retlen, u_char *buf)  { @@
> >> > > > > -1361,6
> >> > > > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const char
> >> > > > > +*name,
> >> > > > enum read_mode mode)
> >> > > > >                 spi_nor_wait_till_ready(nor);
> >> > > > >         }
> >> > > > >
> >> > > > > +       if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC) {
> >> > > > > +               ret = spansion_s25fs_disable_4kb_erase(nor);
> >> > > > > +               if (ret)
> >> > > > > +                       return ret;
> >> > > > > +       }
> >> > > > > +
> >> > > > >         if (!mtd->name)
> >> > > > >                 mtd->name = dev_name(dev);
> >> > > > >         mtd->priv = nor;
> >> > > > > --
> >> > > > > 2.1.0.27.g96db324
> >> > > > >
> >> > > > >
> >> > > > Hi Brian, I will ack this change but still feel it's kind of hacking code.
> >> > > >
> >> > > > Acked-by: Han xu <han.xu@nxp.com>
> >> > >
> >> > > I am new on the list so I am not sure if this topic has been discussed.
> >> > > Generally our product functionality relay on those 4KiB sectors.
> >> > > I know that this hack is already in u-boot, but if you mainstream
> >> > > this you will force users of those 4KiB sectors to do hack the hack...
> >> > > I believe the proper solution here is to use erase regions
> >> > > functionality, I send and RFS about that some time ago.
> >> >
> >> > Do you mind to send me a link for reference?
> >> >
> >> Han,
> >>
> >> Sorry, It seem I have not posted erase region changes (only those
> >> regarding DUAL/QUAD I/O).
> >> Generally, in this flash you need to create 3 erase regions (because
> >> in FS-S family support only  4KiB erase on parameters sector - eg. 1.2.2.4 in
> S25FS512S).
> >> In my case regions are:
> >> 1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21) 2. 32 - 256 - SE_CMD (0xd8/0xdc)
> 3.
> >> Rest of the flash SE_CMD (0xd8/0xdc)
> >>
> >> To erase whole flash you can also use CHIP_ERASE_CMD (0x60/0xC7)
> >> command, you just need to add one more mtd partition that will cover
> whole flash.
> >>
> >
> > Hi Krzeminski,
> >
> > Do you think is there any great advantages for enable 4KB?
> > Because for NXP(Freescale) QSPI controller, there is only support max to 16
> groups command.
> 
> If it's really necessary to support all command groups, you can try apply my
> dynamic lut patch first.
> https://patchwork.ozlabs.org/patch/676791/
> 

I think it will be helpful, I will test and check it.

Thanks,

Re: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash

[Jagan Teki]

On Thu, Aug 18, 2016 at 1:08 PM, Yunhui Cui <B56489@freescale.com> wrote:
> From: Yunhui Cui <yunhui.cui@nxp.com>
>
> With the physical sectors combination, S25FS-S family flash
> requires some special operations for read/write functions.
>
> Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 56 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index d0fc165..495d0bb 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -39,6 +39,10 @@
>
>  #define SPI_NOR_MAX_ID_LEN     6
>  #define SPI_NOR_MAX_ADDR_WIDTH 4
> +/* Added for S25FS-S family flash */
> +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
> +#define CR3V_4KB_ERASE_UNABLE  0x8
> +#define SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
>
>  struct flash_info {
>         char            *name;
> @@ -78,6 +82,7 @@ struct flash_info {
>  };
>
>  #define JEDEC_MFR(info)        ((info)->id[0])
> +#define EXT_JEDEC(info)        ((info)->id[5])
>
>  static const struct flash_info *spi_nor_match_id(const char *name);
>
> @@ -899,6 +904,7 @@ static const struct flash_info spi_nor_ids[] = {
>          */
>         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,  64, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024, 512, 0)},

Handling normal and parameter sectors on specific chip part look quite
messy, and this patch[1] will erase SECT_4K by seeing whole flash as
normal sectors.

[1] https://patchwork.kernel.org/patch/9266541/

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

[PATCH v3 7/9] mtd: fsl-quadspi: Solve Micron Spansion flash command conflict

[Yunhui Cui]

From: Yunhui Cui <yunhui.cui@nxp.com>

Add some lut_tables to support quad mode for flash n25q128
on the board ls1021a-twr and solve flash Spansion and Micron
command conflict.
In switch {}, The value of command SPINOR_OP_RD_EVCR and
SPINOR_OP_SPANSION_RDAR is the same. They have to share
the same seq_id: SEQID_RDAR_OR_RD_EVCR.

Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 47 ++++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 57eed3c..f9a7d4b 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -207,9 +207,9 @@
 #define SEQID_RDCR		9
 #define SEQID_EN4B		10
 #define SEQID_BRWR		11
-#define SEQID_RDAR		12
+#define SEQID_RDAR_OR_RD_EVCR	12
 #define SEQID_WRAR		13
-
+#define SEQID_WD_EVCR           14
 
 #define QUADSPI_MIN_IOMAP SZ_4M
 
@@ -393,6 +393,7 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	int rxfifo = q->devtype_data->rxfifo;
 	u32 lut_base;
 	int i;
+	const struct fsl_qspi_devtype_data *devtype_data = q->devtype_data;
 
 	struct spi_nor *nor = &q->nor[0];
 	u8 addrlen = (nor->addr_width == 3) ? ADDR24BIT : ADDR32BIT;
@@ -489,16 +490,26 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_BRWR),
 			base + QUADSPI_LUT(lut_base));
 
+
 	/*
-	 * Read any device register.
-	 * Used for Spansion S25FS-S family flash only.
+	 * Flash Micron and Spansion command confilict
+	 * use the same value 0x65. But it indicates different meaning.
 	 */
-	lut_base = SEQID_RDAR * 4;
-	qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_SPANSION_RDAR) |
-			LUT1(ADDR, PAD1, ADDR24BIT),
-			base + QUADSPI_LUT(lut_base));
-	qspi_writel(q, LUT0(DUMMY, PAD1, 8) | LUT1(FSL_READ, PAD1, 1),
-			base + QUADSPI_LUT(lut_base + 1));
+	lut_base = SEQID_RDAR_OR_RD_EVCR * 4;
+	if (devtype_data->devtype == FSL_QUADSPI_LS2080A) {
+		/*
+		* Read any device register.
+		* Used for Spansion S25FS-S family flash only.
+		*/
+		qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_SPANSION_RDAR) |
+			    LUT1(ADDR, PAD1, ADDR24BIT),
+			    base + QUADSPI_LUT(lut_base));
+		qspi_writel(q, LUT0(DUMMY, PAD1, 8) | LUT1(FSL_READ, PAD1, 1),
+			    base + QUADSPI_LUT(lut_base + 1));
+	} else {
+		qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_RD_EVCR),
+			    base + QUADSPI_LUT(lut_base));
+	}
 
 	/*
 	 * Write any device register.
@@ -511,6 +522,11 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	qspi_writel(q, LUT0(FSL_WRITE, PAD1, 1),
 			base + QUADSPI_LUT(lut_base + 1));
 
+	/* Write EVCR register */
+	lut_base = SEQID_WD_EVCR * 4;
+	qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_WD_EVCR),
+		    base + QUADSPI_LUT(lut_base));
+
 	fsl_qspi_lock_lut(q);
 }
 
@@ -523,8 +539,15 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
 	case SPINOR_OP_READ_FAST:
 	case SPINOR_OP_READ4_FAST:
 		return SEQID_READ;
+	/*
+	 * Spansion & Micron use the same command value 0x65
+	 * Spansion: SPINOR_OP_SPANSION_RDAR, read any register.
+	 * Micron: SPINOR_OP_RD_EVCR,
+	 * read enhanced volatile configuration register.
+	 * case SPINOR_OP_RD_EVCR:
+	 */
 	case SPINOR_OP_SPANSION_RDAR:
-		return SEQID_RDAR;
+		return SEQID_RDAR_OR_RD_EVCR;
 	case SPINOR_OP_SPANSION_WRAR:
 		return SEQID_WRAR;
 	case SPINOR_OP_WREN:
@@ -550,6 +573,8 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
 		return SEQID_EN4B;
 	case SPINOR_OP_BRWR:
 		return SEQID_BRWR;
+	case SPINOR_OP_WD_EVCR:
+		return SEQID_WD_EVCR;
 	default:
 		if (cmd == q->nor[0].erase_opcode)
 			return SEQID_SE;
-- 
2.1.0.27.g96db324

Re: [PATCH v3 7/9] mtd: fsl-quadspi: Solve Micron Spansion flash command conflict

[Han Xu]

On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui <B56489@freescale.com> wrote:
> From: Yunhui Cui <yunhui.cui@nxp.com>
>
> Add some lut_tables to support quad mode for flash n25q128
> on the board ls1021a-twr and solve flash Spansion and Micron
> command conflict.
> In switch {}, The value of command SPINOR_OP_RD_EVCR and
> SPINOR_OP_SPANSION_RDAR is the same. They have to share
> the same seq_id: SEQID_RDAR_OR_RD_EVCR.
>
> Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 47 ++++++++++++++++++++++++++++++---------
>  1 file changed, 36 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 57eed3c..f9a7d4b 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -207,9 +207,9 @@
>  #define SEQID_RDCR             9
>  #define SEQID_EN4B             10
>  #define SEQID_BRWR             11
> -#define SEQID_RDAR             12
> +#define SEQID_RDAR_OR_RD_EVCR  12
>  #define SEQID_WRAR             13
> -
> +#define SEQID_WD_EVCR           14
>
>  #define QUADSPI_MIN_IOMAP SZ_4M
>
> @@ -393,6 +393,7 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>         int rxfifo = q->devtype_data->rxfifo;
>         u32 lut_base;
>         int i;
> +       const struct fsl_qspi_devtype_data *devtype_data = q->devtype_data;
>
>         struct spi_nor *nor = &q->nor[0];
>         u8 addrlen = (nor->addr_width == 3) ? ADDR24BIT : ADDR32BIT;
> @@ -489,16 +490,26 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>         qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_BRWR),
>                         base + QUADSPI_LUT(lut_base));
>
> +
>         /*
> -        * Read any device register.
> -        * Used for Spansion S25FS-S family flash only.
> +        * Flash Micron and Spansion command confilict
> +        * use the same value 0x65. But it indicates different meaning.
>          */
> -       lut_base = SEQID_RDAR * 4;
> -       qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_SPANSION_RDAR) |
> -                       LUT1(ADDR, PAD1, ADDR24BIT),
> -                       base + QUADSPI_LUT(lut_base));
> -       qspi_writel(q, LUT0(DUMMY, PAD1, 8) | LUT1(FSL_READ, PAD1, 1),
> -                       base + QUADSPI_LUT(lut_base + 1));
> +       lut_base = SEQID_RDAR_OR_RD_EVCR * 4;
> +       if (devtype_data->devtype == FSL_QUADSPI_LS2080A) {
> +               /*
> +               * Read any device register.
> +               * Used for Spansion S25FS-S family flash only.
> +               */
> +               qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_SPANSION_RDAR) |
> +                           LUT1(ADDR, PAD1, ADDR24BIT),
> +                           base + QUADSPI_LUT(lut_base));
> +               qspi_writel(q, LUT0(DUMMY, PAD1, 8) | LUT1(FSL_READ, PAD1, 1),
> +                           base + QUADSPI_LUT(lut_base + 1));
> +       } else {
> +               qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_RD_EVCR),
> +                           base + QUADSPI_LUT(lut_base));
> +       }
>
>         /*
>          * Write any device register.
> @@ -511,6 +522,11 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>         qspi_writel(q, LUT0(FSL_WRITE, PAD1, 1),
>                         base + QUADSPI_LUT(lut_base + 1));
>
> +       /* Write EVCR register */
> +       lut_base = SEQID_WD_EVCR * 4;
> +       qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_WD_EVCR),
> +                   base + QUADSPI_LUT(lut_base));
> +
>         fsl_qspi_lock_lut(q);
>  }
>
> @@ -523,8 +539,15 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
>         case SPINOR_OP_READ_FAST:
>         case SPINOR_OP_READ4_FAST:
>                 return SEQID_READ;
> +       /*
> +        * Spansion & Micron use the same command value 0x65
> +        * Spansion: SPINOR_OP_SPANSION_RDAR, read any register.
> +        * Micron: SPINOR_OP_RD_EVCR,
> +        * read enhanced volatile configuration register.
> +        * case SPINOR_OP_RD_EVCR:
> +        */
>         case SPINOR_OP_SPANSION_RDAR:
> -               return SEQID_RDAR;
> +               return SEQID_RDAR_OR_RD_EVCR;
>         case SPINOR_OP_SPANSION_WRAR:
>                 return SEQID_WRAR;
>         case SPINOR_OP_WREN:
> @@ -550,6 +573,8 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
>                 return SEQID_EN4B;
>         case SPINOR_OP_BRWR:
>                 return SEQID_BRWR;
> +       case SPINOR_OP_WD_EVCR:
> +               return SEQID_WD_EVCR;
>         default:
>                 if (cmd == q->nor[0].erase_opcode)
>                         return SEQID_SE;
> --
> 2.1.0.27.g96db324
>
>
Acked-by: Han xu <han.xu@nxp.com>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

[PATCH v3 8/9] mtd: fsl-quadspi: disable AHB buffer prefetch

[Yunhui Cui]

From: Yunhui Cui <yunhui.cui@nxp.com>

A-009282: QuadSPI: QuadSPI data pre-fetch can result in incorrect data
Affects: QuadSPI
Description: With AHB buffer prefetch enabled, the QuadSPI may return
incorrect data on the AHB
interface. The buffer pre-fetch is enabled if the fetch size as
configured either in the LUT or in
the BUFxCR register is greater than 8 bytes.
Impact: Only 64 bit read allowed.
Workaround: Keep the read data size to 64 bits (8 Bytes), which disables
the prefetch on the AHB buffer,
and prevents this issue from occurring.

Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index f9a7d4b..193e81b 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -41,6 +41,8 @@
 #define QUADSPI_QUIRK_TKT253890		(1 << 2)
 /* Controller cannot wake up from wait mode, TKT245618 */
 #define QUADSPI_QUIRK_TKT245618         (1 << 3)
+/*Errata A-009282: disable the AHB buffer prefetch */
+#define QUADSPI_QUIRK_ADASZ_8BYTE_LIMIT (1 << 4)
 /* QSPI_AMBA_BASE is internally added by SOC design */
 #define QUADSPI_AMBA_BASE_INTERNAL	(0x10000)
 
@@ -270,7 +272,7 @@ static struct fsl_qspi_devtype_data ls1021a_data = {
 	.rxfifo = 128,
 	.txfifo = 64,
 	.ahb_buf_size = 1024,
-	.driver_data = 0,
+	.driver_data = QUADSPI_QUIRK_ADASZ_8BYTE_LIMIT,
 };
 
 static struct fsl_qspi_devtype_data ls2080a_data = {
@@ -278,7 +280,8 @@ static struct fsl_qspi_devtype_data ls2080a_data = {
 	.rxfifo = 128,
 	.txfifo = 64,
 	.ahb_buf_size = 1024,
-	.driver_data = QUADSPI_AMBA_BASE_INTERNAL,
+	.driver_data = QUADSPI_AMBA_BASE_INTERNAL
+		       | QUADSPI_QUIRK_ADASZ_8BYTE_LIMIT,
 };
 
 #define FSL_QSPI_MAX_CHIP	4
@@ -328,6 +331,11 @@ static inline int has_added_amba_base_internal(struct fsl_qspi *q)
 	return q->devtype_data->driver_data & QUADSPI_AMBA_BASE_INTERNAL;
 }
 
+static inline int needs_disable_ahb_prefetch(struct fsl_qspi *q)
+{
+	return q->devtype_data->driver_data & QUADSPI_QUIRK_ADASZ_8BYTE_LIMIT;
+}
+
 /*
  * R/W functions for big- or little-endian registers:
  * The qSPI controller's endian is independent of the CPU core's endian.
@@ -757,14 +765,21 @@ static void fsl_qspi_init_abh_read(struct fsl_qspi *q)
 	qspi_writel(q, QUADSPI_BUFXCR_INVALID_MSTRID, base + QUADSPI_BUF0CR);
 	qspi_writel(q, QUADSPI_BUFXCR_INVALID_MSTRID, base + QUADSPI_BUF1CR);
 	qspi_writel(q, QUADSPI_BUFXCR_INVALID_MSTRID, base + QUADSPI_BUF2CR);
-	/*
-	 * Set ADATSZ with the maximum AHB buffer size to improve the
-	 * read performance.
-	 */
-	qspi_writel(q, QUADSPI_BUF3CR_ALLMST_MASK |
-			((q->devtype_data->ahb_buf_size / 8)
-			<< QUADSPI_BUF3CR_ADATSZ_SHIFT),
+
+	if (needs_disable_ahb_prefetch(q)) {
+		qspi_writel(q, QUADSPI_BUF3CR_ALLMST_MASK |
+			(1 << QUADSPI_BUF3CR_ADATSZ_SHIFT),
 			base + QUADSPI_BUF3CR);
+	} else {
+		/*
+		 * Set ADATSZ with the maximum AHB buffer size to improve the
+		 * read performance.
+		*/
+		qspi_writel(q, QUADSPI_BUF3CR_ALLMST_MASK |
+				((q->devtype_data->ahb_buf_size / 8)
+				<< QUADSPI_BUF3CR_ADATSZ_SHIFT),
+				base + QUADSPI_BUF3CR);
+	}
 
 	/* We only use the buffer3 */
 	qspi_writel(q, 0, base + QUADSPI_BUF0IND);
-- 
2.1.0.27.g96db324

Re: [PATCH v3 8/9] mtd: fsl-quadspi: disable AHB buffer prefetch

[Han Xu]

On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui <B56489@freescale.com> wrote:
> From: Yunhui Cui <yunhui.cui@nxp.com>
>
> A-009282: QuadSPI: QuadSPI data pre-fetch can result in incorrect data
> Affects: QuadSPI
> Description: With AHB buffer prefetch enabled, the QuadSPI may return
> incorrect data on the AHB
> interface. The buffer pre-fetch is enabled if the fetch size as
> configured either in the LUT or in
> the BUFxCR register is greater than 8 bytes.
> Impact: Only 64 bit read allowed.
> Workaround: Keep the read data size to 64 bits (8 Bytes), which disables
> the prefetch on the AHB buffer,
> and prevents this issue from occurring.
>
> Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index f9a7d4b..193e81b 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -41,6 +41,8 @@
>  #define QUADSPI_QUIRK_TKT253890                (1 << 2)
>  /* Controller cannot wake up from wait mode, TKT245618 */
>  #define QUADSPI_QUIRK_TKT245618         (1 << 3)
> +/*Errata A-009282: disable the AHB buffer prefetch */
> +#define QUADSPI_QUIRK_ADASZ_8BYTE_LIMIT (1 << 4)
>  /* QSPI_AMBA_BASE is internally added by SOC design */
>  #define QUADSPI_AMBA_BASE_INTERNAL     (0x10000)
>
> @@ -270,7 +272,7 @@ static struct fsl_qspi_devtype_data ls1021a_data = {
>         .rxfifo = 128,
>         .txfifo = 64,
>         .ahb_buf_size = 1024,
> -       .driver_data = 0,
> +       .driver_data = QUADSPI_QUIRK_ADASZ_8BYTE_LIMIT,
>  };
>
>  static struct fsl_qspi_devtype_data ls2080a_data = {
> @@ -278,7 +280,8 @@ static struct fsl_qspi_devtype_data ls2080a_data = {
>         .rxfifo = 128,
>         .txfifo = 64,
>         .ahb_buf_size = 1024,
> -       .driver_data = QUADSPI_AMBA_BASE_INTERNAL,
> +       .driver_data = QUADSPI_AMBA_BASE_INTERNAL
> +                      | QUADSPI_QUIRK_ADASZ_8BYTE_LIMIT,
>  };
>
>  #define FSL_QSPI_MAX_CHIP      4
> @@ -328,6 +331,11 @@ static inline int has_added_amba_base_internal(struct fsl_qspi *q)
>         return q->devtype_data->driver_data & QUADSPI_AMBA_BASE_INTERNAL;
>  }
>
> +static inline int needs_disable_ahb_prefetch(struct fsl_qspi *q)
> +{
> +       return q->devtype_data->driver_data & QUADSPI_QUIRK_ADASZ_8BYTE_LIMIT;
> +}
> +
>  /*
>   * R/W functions for big- or little-endian registers:
>   * The qSPI controller's endian is independent of the CPU core's endian.
> @@ -757,14 +765,21 @@ static void fsl_qspi_init_abh_read(struct fsl_qspi *q)
>         qspi_writel(q, QUADSPI_BUFXCR_INVALID_MSTRID, base + QUADSPI_BUF0CR);
>         qspi_writel(q, QUADSPI_BUFXCR_INVALID_MSTRID, base + QUADSPI_BUF1CR);
>         qspi_writel(q, QUADSPI_BUFXCR_INVALID_MSTRID, base + QUADSPI_BUF2CR);
> -       /*
> -        * Set ADATSZ with the maximum AHB buffer size to improve the
> -        * read performance.
> -        */
> -       qspi_writel(q, QUADSPI_BUF3CR_ALLMST_MASK |
> -                       ((q->devtype_data->ahb_buf_size / 8)
> -                       << QUADSPI_BUF3CR_ADATSZ_SHIFT),
> +
> +       if (needs_disable_ahb_prefetch(q)) {
> +               qspi_writel(q, QUADSPI_BUF3CR_ALLMST_MASK |
> +                       (1 << QUADSPI_BUF3CR_ADATSZ_SHIFT),
>                         base + QUADSPI_BUF3CR);
> +       } else {
> +               /*
> +                * Set ADATSZ with the maximum AHB buffer size to improve the
> +                * read performance.
> +               */
> +               qspi_writel(q, QUADSPI_BUF3CR_ALLMST_MASK |
> +                               ((q->devtype_data->ahb_buf_size / 8)
> +                               << QUADSPI_BUF3CR_ADATSZ_SHIFT),
> +                               base + QUADSPI_BUF3CR);
> +       }
>
>         /* We only use the buffer3 */
>         qspi_writel(q, 0, base + QUADSPI_BUF0IND);
> --
> 2.1.0.27.g96db324
>
>
Acked-by: Han xu <han.xu@nxp.com>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

[PATCH v3 9/9] mtd: fsl-quadspi: add multi flash chip R/W on ls2080a

[Yunhui Cui]

From: Yunhui Cui <yunhui.cui@nxp.com>

There is a hardware feature that qspi_amba_base is added
internally by SOC design on ls2080a. so memmap_phy need not
be added in driver. If memmap_phy is added, the flash A1
addr space is [0, memmap_phy] which far more than flash size.
The AMBA memory will be divided into four parts and assign to
every chipselect. Every channel will has two valid chipselects.

Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 193e81b..8c9746c 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -736,11 +736,17 @@ static void fsl_qspi_set_map_addr(struct fsl_qspi *q)
 {
 	int nor_size = q->nor_size;
 	void __iomem *base = q->iobase;
+	u32 mem_base;
 
-	qspi_writel(q, nor_size + q->memmap_phy, base + QUADSPI_SFA1AD);
-	qspi_writel(q, nor_size * 2 + q->memmap_phy, base + QUADSPI_SFA2AD);
-	qspi_writel(q, nor_size * 3 + q->memmap_phy, base + QUADSPI_SFB1AD);
-	qspi_writel(q, nor_size * 4 + q->memmap_phy, base + QUADSPI_SFB2AD);
+	if (has_added_amba_base_internal(q))
+		mem_base = 0x0;
+	else
+		mem_base = q->memmap_phy;
+
+	qspi_writel(q, nor_size + mem_base, base + QUADSPI_SFA1AD);
+	qspi_writel(q, nor_size * 2 + mem_base, base + QUADSPI_SFA2AD);
+	qspi_writel(q, nor_size * 3 + mem_base, base + QUADSPI_SFB1AD);
+	qspi_writel(q, nor_size * 4 + mem_base, base + QUADSPI_SFB2AD);
 }
 
 /*
-- 
2.1.0.27.g96db324

Re: [PATCH v3 9/9] mtd: fsl-quadspi: add multi flash chip R/W on ls2080a

[Han Xu]

On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui <B56489@freescale.com> wrote:
> From: Yunhui Cui <yunhui.cui@nxp.com>
>
> There is a hardware feature that qspi_amba_base is added
> internally by SOC design on ls2080a. so memmap_phy need not
> be added in driver. If memmap_phy is added, the flash A1
> addr space is [0, memmap_phy] which far more than flash size.
> The AMBA memory will be divided into four parts and assign to
> every chipselect. Every channel will has two valid chipselects.
>
> Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 193e81b..8c9746c 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -736,11 +736,17 @@ static void fsl_qspi_set_map_addr(struct fsl_qspi *q)
>  {
>         int nor_size = q->nor_size;
>         void __iomem *base = q->iobase;
> +       u32 mem_base;
>
> -       qspi_writel(q, nor_size + q->memmap_phy, base + QUADSPI_SFA1AD);
> -       qspi_writel(q, nor_size * 2 + q->memmap_phy, base + QUADSPI_SFA2AD);
> -       qspi_writel(q, nor_size * 3 + q->memmap_phy, base + QUADSPI_SFB1AD);
> -       qspi_writel(q, nor_size * 4 + q->memmap_phy, base + QUADSPI_SFB2AD);
> +       if (has_added_amba_base_internal(q))
> +               mem_base = 0x0;
> +       else
> +               mem_base = q->memmap_phy;
> +
> +       qspi_writel(q, nor_size + mem_base, base + QUADSPI_SFA1AD);
> +       qspi_writel(q, nor_size * 2 + mem_base, base + QUADSPI_SFA2AD);
> +       qspi_writel(q, nor_size * 3 + mem_base, base + QUADSPI_SFB1AD);
> +       qspi_writel(q, nor_size * 4 + mem_base, base + QUADSPI_SFB2AD);
>  }
>
>  /*
> --
> 2.1.0.27.g96db324
>
>
Acked-by: Han xu <han.xu@nxp.com>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

Re: [PATCH v3 1/9] mtd:fsl-quadspi:use the property fields of SPI-NOR

[Han Xu]

On Thu, Aug 18, 2016 at 2:37 AM, Yunhui Cui <B56489@freescale.com> wrote:
> We can get the read/write/erase opcode from the spi nor framework
> directly. This patch uses the information stored in the SPI-NOR to
> remove the hardcode in the fsl_qspi_init_lut().
>
> Signed-off-by: Yunhui Cui <B56489@freescale.com>
> Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 40 ++++++++++++---------------------------
>  1 file changed, 12 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 5c82e4e..5ad6402 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -373,9 +373,13 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>         void __iomem *base = q->iobase;
>         int rxfifo = q->devtype_data->rxfifo;
>         u32 lut_base;
> -       u8 cmd, addrlen, dummy;
>         int i;
>
> +       struct spi_nor *nor = &q->nor[0];
> +       u8 addrlen = (nor->addr_width == 3) ? ADDR24BIT : ADDR32BIT;
> +       u8 read_op = nor->read_opcode;
> +       u8 read_dm = nor->read_dummy;
> +
>         fsl_qspi_unlock_lut(q);
>
>         /* Clear all the LUT table */
> @@ -385,20 +389,10 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>         /* Quad Read */
>         lut_base = SEQID_QUAD_READ * 4;
>
> -       if (q->nor_size <= SZ_16M) {
> -               cmd = SPINOR_OP_READ_1_1_4;
> -               addrlen = ADDR24BIT;
> -               dummy = 8;
> -       } else {
> -               /* use the 4-byte address */
> -               cmd = SPINOR_OP_READ_1_1_4;
> -               addrlen = ADDR32BIT;
> -               dummy = 8;
> -       }
> -
> -       qspi_writel(q, LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
> +       qspi_writel(q, LUT0(CMD, PAD1, read_op) | LUT1(ADDR, PAD1, addrlen),
>                         base + QUADSPI_LUT(lut_base));
> -       qspi_writel(q, LUT0(DUMMY, PAD1, dummy) | LUT1(FSL_READ, PAD4, rxfifo),
> +       qspi_writel(q, LUT0(DUMMY, PAD1, read_dm) |
> +                   LUT1(FSL_READ, PAD4, rxfifo),
>                         base + QUADSPI_LUT(lut_base + 1));
>
>         /* Write enable */
> @@ -409,16 +403,8 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>         /* Page Program */
>         lut_base = SEQID_PP * 4;
>
> -       if (q->nor_size <= SZ_16M) {
> -               cmd = SPINOR_OP_PP;
> -               addrlen = ADDR24BIT;
> -       } else {
> -               /* use the 4-byte address */
> -               cmd = SPINOR_OP_PP;
> -               addrlen = ADDR32BIT;
> -       }
> -
> -       qspi_writel(q, LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
> +       qspi_writel(q, LUT0(CMD, PAD1, nor->program_opcode) |
> +                   LUT1(ADDR, PAD1, addrlen),
>                         base + QUADSPI_LUT(lut_base));
>         qspi_writel(q, LUT0(FSL_WRITE, PAD1, 0),
>                         base + QUADSPI_LUT(lut_base + 1));
> @@ -432,10 +418,8 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>         /* Erase a sector */
>         lut_base = SEQID_SE * 4;
>
> -       cmd = q->nor[0].erase_opcode;
> -       addrlen = q->nor_size <= SZ_16M ? ADDR24BIT : ADDR32BIT;
> -
> -       qspi_writel(q, LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
> +       qspi_writel(q, LUT0(CMD, PAD1, nor->erase_opcode) |
> +                   LUT1(ADDR, PAD1, addrlen),
>                         base + QUADSPI_LUT(lut_base));
>
>         /* Erase the whole chip */
> --
> 2.1.0.27.g96db324
>
>
Acked-by: Han xu <han.xu@nxp.com>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

Re: [PATCH v3 1/9] mtd:fsl-quadspi:use the property fields of SPI-NOR

[Cyrille Pitchen]

Hi Yunhui,

Indeed it's a really good thing to update the Freescale QSPI controller driver
so it relies more on the settings chosen in spi-nor.c: it avoids duplicating
the code. spi-nor.c is supposed to select all the instruction codes and memory
settings according to both the (Q)SPI memory and the controller capabilities.
The controller driver should only be responsible for sending the SPI commands
but should neither modify those commands nor be aware of the actual memory
connected to it.

So the idea of this patch is great :)

Best regards,

Cyrille

Le 18/08/2016 à 09:37, Yunhui Cui a écrit :
> We can get the read/write/erase opcode from the spi nor framework
> directly. This patch uses the information stored in the SPI-NOR to
> remove the hardcode in the fsl_qspi_init_lut().
> 
> Signed-off-by: Yunhui Cui <B56489@freescale.com>
> Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
Reviewed-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 40 ++++++++++++---------------------------
>  1 file changed, 12 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 5c82e4e..5ad6402 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -373,9 +373,13 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  	void __iomem *base = q->iobase;
>  	int rxfifo = q->devtype_data->rxfifo;
>  	u32 lut_base;
> -	u8 cmd, addrlen, dummy;
>  	int i;
>  
> +	struct spi_nor *nor = &q->nor[0];
> +	u8 addrlen = (nor->addr_width == 3) ? ADDR24BIT : ADDR32BIT;
> +	u8 read_op = nor->read_opcode;
> +	u8 read_dm = nor->read_dummy;
> +
>  	fsl_qspi_unlock_lut(q);
>  
>  	/* Clear all the LUT table */
> @@ -385,20 +389,10 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  	/* Quad Read */
>  	lut_base = SEQID_QUAD_READ * 4;
>  
> -	if (q->nor_size <= SZ_16M) {
> -		cmd = SPINOR_OP_READ_1_1_4;
> -		addrlen = ADDR24BIT;
> -		dummy = 8;
> -	} else {
> -		/* use the 4-byte address */
> -		cmd = SPINOR_OP_READ_1_1_4;
> -		addrlen = ADDR32BIT;
> -		dummy = 8;
> -	}
> -
> -	qspi_writel(q, LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
> +	qspi_writel(q, LUT0(CMD, PAD1, read_op) | LUT1(ADDR, PAD1, addrlen),
>  			base + QUADSPI_LUT(lut_base));
> -	qspi_writel(q, LUT0(DUMMY, PAD1, dummy) | LUT1(FSL_READ, PAD4, rxfifo),
> +	qspi_writel(q, LUT0(DUMMY, PAD1, read_dm) |
> +		    LUT1(FSL_READ, PAD4, rxfifo),
>  			base + QUADSPI_LUT(lut_base + 1));
>  
>  	/* Write enable */
> @@ -409,16 +403,8 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  	/* Page Program */
>  	lut_base = SEQID_PP * 4;
>  
> -	if (q->nor_size <= SZ_16M) {
> -		cmd = SPINOR_OP_PP;
> -		addrlen = ADDR24BIT;
> -	} else {
> -		/* use the 4-byte address */
> -		cmd = SPINOR_OP_PP;
> -		addrlen = ADDR32BIT;
> -	}
> -
> -	qspi_writel(q, LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
> +	qspi_writel(q, LUT0(CMD, PAD1, nor->program_opcode) |
> +		    LUT1(ADDR, PAD1, addrlen),
>  			base + QUADSPI_LUT(lut_base));
>  	qspi_writel(q, LUT0(FSL_WRITE, PAD1, 0),
>  			base + QUADSPI_LUT(lut_base + 1));
> @@ -432,10 +418,8 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  	/* Erase a sector */
>  	lut_base = SEQID_SE * 4;
>  
> -	cmd = q->nor[0].erase_opcode;
> -	addrlen = q->nor_size <= SZ_16M ? ADDR24BIT : ADDR32BIT;
> -
> -	qspi_writel(q, LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
> +	qspi_writel(q, LUT0(CMD, PAD1, nor->erase_opcode) |
> +		    LUT1(ADDR, PAD1, addrlen),
>  			base + QUADSPI_LUT(lut_base));
>  
>  	/* Erase the whole chip */
> 

Re: [PATCH v3 1/9] mtd:fsl-quadspi:use the property fields of SPI-NOR

[Cyrille Pitchen]

Le 18/08/2016 à 09:37, Yunhui Cui a écrit :
> We can get the read/write/erase opcode from the spi nor framework
> directly. This patch uses the information stored in the SPI-NOR to
> remove the hardcode in the fsl_qspi_init_lut().
> 
> Signed-off-by: Yunhui Cui <B56489@freescale.com>
> Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>

Applied to the spi-nor tree

Thanks!
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 40 ++++++++++++---------------------------
>  1 file changed, 12 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 5c82e4e..5ad6402 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -373,9 +373,13 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  	void __iomem *base = q->iobase;
>  	int rxfifo = q->devtype_data->rxfifo;
>  	u32 lut_base;
> -	u8 cmd, addrlen, dummy;
>  	int i;
>  
> +	struct spi_nor *nor = &q->nor[0];
> +	u8 addrlen = (nor->addr_width == 3) ? ADDR24BIT : ADDR32BIT;
> +	u8 read_op = nor->read_opcode;
> +	u8 read_dm = nor->read_dummy;
> +
>  	fsl_qspi_unlock_lut(q);
>  
>  	/* Clear all the LUT table */
> @@ -385,20 +389,10 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  	/* Quad Read */
>  	lut_base = SEQID_QUAD_READ * 4;
>  
> -	if (q->nor_size <= SZ_16M) {
> -		cmd = SPINOR_OP_READ_1_1_4;
> -		addrlen = ADDR24BIT;
> -		dummy = 8;
> -	} else {
> -		/* use the 4-byte address */
> -		cmd = SPINOR_OP_READ_1_1_4;
> -		addrlen = ADDR32BIT;
> -		dummy = 8;
> -	}
> -
> -	qspi_writel(q, LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
> +	qspi_writel(q, LUT0(CMD, PAD1, read_op) | LUT1(ADDR, PAD1, addrlen),
>  			base + QUADSPI_LUT(lut_base));
> -	qspi_writel(q, LUT0(DUMMY, PAD1, dummy) | LUT1(FSL_READ, PAD4, rxfifo),
> +	qspi_writel(q, LUT0(DUMMY, PAD1, read_dm) |
> +		    LUT1(FSL_READ, PAD4, rxfifo),
>  			base + QUADSPI_LUT(lut_base + 1));
>  
>  	/* Write enable */
> @@ -409,16 +403,8 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  	/* Page Program */
>  	lut_base = SEQID_PP * 4;
>  
> -	if (q->nor_size <= SZ_16M) {
> -		cmd = SPINOR_OP_PP;
> -		addrlen = ADDR24BIT;
> -	} else {
> -		/* use the 4-byte address */
> -		cmd = SPINOR_OP_PP;
> -		addrlen = ADDR32BIT;
> -	}
> -
> -	qspi_writel(q, LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
> +	qspi_writel(q, LUT0(CMD, PAD1, nor->program_opcode) |
> +		    LUT1(ADDR, PAD1, addrlen),
>  			base + QUADSPI_LUT(lut_base));
>  	qspi_writel(q, LUT0(FSL_WRITE, PAD1, 0),
>  			base + QUADSPI_LUT(lut_base + 1));
> @@ -432,10 +418,8 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  	/* Erase a sector */
>  	lut_base = SEQID_SE * 4;
>  
> -	cmd = q->nor[0].erase_opcode;
> -	addrlen = q->nor_size <= SZ_16M ? ADDR24BIT : ADDR32BIT;
> -
> -	qspi_writel(q, LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
> +	qspi_writel(q, LUT0(CMD, PAD1, nor->erase_opcode) |
> +		    LUT1(ADDR, PAD1, addrlen),
>  			base + QUADSPI_LUT(lut_base));
>  
>  	/* Erase the whole chip */
>