[RFC 00/10] mtd: spi-nor: Update spi-nor framework

[RFC 00/10] mtd: spi-nor: Update spi-nor framework

[Prabhakar Kushwaha]

SPI-NOR framework currently supports-
 - (1-1-1, 1-1-2, 1-1-4) read protocols
 - read latency(dummy cycles) are hardcoded with the assumption
 that the flash would support it.
 - Number of mode cycles/bits is also hardcoded, but the mode bits
 may be different for different Flashes and they also differ for
 different commands.
 - Additionally the existing framework selects the slowest read protocol
 combination supported by the controller and the Flash.

This patch set add support of 1-2-2, 1-4-4 read protocols. 
It choose the fastest available read protocol. It adds a provision to
configure the read latency (dummy cycles) based on the Flash memory
device. 

This patch also adds support of S25FS-S flash family with required
mode bits(required number of mode cycles). To allow dummy cycle
configuration in flash, new APIs have been added to read/write any register.

Finally it update fsl-quadspi binding to support different modes
(Single, dual, quad) of controller and update its capability
structure. This capability register is used by spi-nor framework
in defining right supported flash configurations.


Prabhakar Kushwaha (10):
  mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ protocols
  mtd: spi-nor: Use the highest supported READ protocol
  mtd: spi-nor: Configure read latency for read commands
  mtd: spi-nor: Configure read latency for micron flashes
  Documentation: binding: Update fsl-quadspi binding
  mtd: spi-nor: Update hwcap based on mode of fsl-quadspi
  mtd: spi-nor: Add support of read/write any reg commands
  mtd: spi-nor: Add support of SPANSION S25FS-S flash
  mtd: spi-nor: Disable hybrid mode for SPANSION S25FS-S family
  mtd: spi-nor: Implement anyreg functions for fsl-quadspi

 .../devicetree/bindings/mtd/fsl-quadspi.txt        |   4 +
 drivers/mtd/spi-nor/fsl-quadspi.c                  |  47 ++++-
 drivers/mtd/spi-nor/spi-nor.c                      | 196 +++++++++++++++++++--
 include/linux/mtd/spi-nor.h                        |  19 ++
 4 files changed, 247 insertions(+), 19 deletions(-)

-- 
2.7.4

[RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ protocols

[Prabhakar Kushwaha]

IO READ protocols transfers both address and data on multiple
data bits. 1-2-2(DUAL IO), 1-4-4(QUAD IO) transfer address on 2
data bits or 4 bits per rising edge of SCK respectively.

This patch update spi_nor_flash_parameter->spi_nor_read_command
array based on DUAL or QUAD IO flag enabled in flash_info for a flash.

Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 19c00072..7d3b52f 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -61,7 +61,7 @@ struct flash_info {
 	u16		page_size;
 	u16		addr_width;
 
-	u16		flags;
+	u32		flags;
 #define SECT_4K			BIT(0)	/* SPINOR_OP_BE_4K works uniformly */
 #define SPI_NOR_NO_ERASE	BIT(1)	/* No erase command needed */
 #define SST_WRITE		BIT(2)	/* use SST byte programming */
@@ -89,6 +89,8 @@ struct flash_info {
 #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip erase */
 #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
 #define USE_CLSR		BIT(14)	/* use CLSR command */
+#define SPI_NOR_DUAL_IO_READ	BIT(15)	/* Flash supports Dual IO Read */
+#define SPI_NOR_QUAD_IO_READ	BIT(16)	/* Flash supports Quad IO Read */
 };
 
 #define JEDEC_MFR(info)	((info)->id[0])
@@ -2399,6 +2401,13 @@ static int spi_nor_init_params(struct spi_nor *nor,
 					  SNOR_PROTO_1_1_2);
 	}
 
+	if (info->flags & SPI_NOR_DUAL_IO_READ) {
+		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
+		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_2_2],
+					  0, 8, SPINOR_OP_READ_1_2_2,
+					  SNOR_PROTO_1_2_2);
+	}
+
 	if (info->flags & SPI_NOR_QUAD_READ) {
 		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
 		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_4],
@@ -2406,6 +2415,14 @@ static int spi_nor_init_params(struct spi_nor *nor,
 					  SNOR_PROTO_1_1_4);
 	}
 
+	if (info->flags & SPI_NOR_QUAD_IO_READ) {
+		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_4_4;
+		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_4_4],
+					  0, 10, SPINOR_OP_READ_1_4_4,
+					  SNOR_PROTO_1_4_4);
+	}
+
+
 	/* Page Program settings. */
 	params->hwcaps.mask |= SNOR_HWCAPS_PP;
 	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP],
@@ -2432,7 +2449,8 @@ static int spi_nor_init_params(struct spi_nor *nor,
 	/* Override the parameters with data read from SFDP tables. */
 	nor->addr_width = 0;
 	nor->mtd.erasesize = 0;
-	if ((info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)) &&
+	if ((info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
+	    SPI_NOR_DUAL_IO_READ | SPI_NOR_QUAD_IO_READ)) &&
 	    !(info->flags & SPI_NOR_SKIP_SFDP)) {
 		struct spi_nor_flash_parameter sfdp_params;
 
-- 
2.7.4

[RFC 02/10] mtd: spi-nor: Use the highest supported READ protocol

[Prabhakar Kushwaha]

HWCAP read capabilities defines higher bit position as the higher
priority. i.e. use Octo SPI protocols first, then Quad SPI protocols
before Dual SPI protocols, Fast Read and lastly (Slow) Read.

Currently implementation chose the last set bit i.e. slowest read.
This patch select the highest available read capability.

Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 7d3b52f..01898e1 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2521,7 +2521,7 @@ static int spi_nor_select_read(struct spi_nor *nor,
 			       const struct spi_nor_flash_parameter *params,
 			       u32 shared_hwcaps)
 {
-	int cmd, best_match = fls(shared_hwcaps & SNOR_HWCAPS_READ_MASK) - 1;
+	int cmd, best_match = ffs(shared_hwcaps & SNOR_HWCAPS_READ_MASK) - 1;
 	const struct spi_nor_read_command *read;
 
 	if (best_match < 0)
-- 
2.7.4

[RFC 03/10] mtd: spi-nor: Configure read latency for read commands

[Prabhakar Kushwaha]

All read commands have read latency (dummy cycle). It defines a
period between the end of address or mode and the beginning of
read data returning to the host. Flashes have default read latency.
This default read latency may not match with the selected latency.

So, selected latency needs to be programmed in flash via
volatile configuration register. Considering flashes have different
types of configuration registers/bits depending upon vendor/family.

This patch provides a way define function pointer per flash vendor
to configure volatile configuration register. function pointer has
flash_info parameter to take care of difference within the family of
a vendor.

Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 01898e1..7d94874 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1695,6 +1695,9 @@ struct spi_nor_read_command {
 	u8			num_wait_states;
 	u8			opcode;
 	enum spi_nor_protocol	proto;
+
+	int (*dummy_config)(struct spi_nor *nor, u8 num_wait_states,
+			    const struct flash_info *info);
 };
 
 struct spi_nor_pp_command {
@@ -1760,12 +1763,19 @@ spi_nor_set_read_settings(struct spi_nor_read_command *read,
 			  u8 num_mode_clocks,
 			  u8 num_wait_states,
 			  u8 opcode,
-			  enum spi_nor_protocol proto)
+			  enum spi_nor_protocol proto,
+			  const struct flash_info *info)
 {
 	read->num_mode_clocks = num_mode_clocks;
 	read->num_wait_states = num_wait_states;
 	read->opcode = opcode;
 	read->proto = proto;
+
+	switch (JEDEC_MFR(info)) {
+	default:
+		read->dummy_config = NULL;
+		break;
+	}
 }
 
 static void
@@ -2385,41 +2395,41 @@ static int spi_nor_init_params(struct spi_nor *nor,
 	params->hwcaps.mask |= SNOR_HWCAPS_READ;
 	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ],
 				  0, 0, SPINOR_OP_READ,
-				  SNOR_PROTO_1_1_1);
+				  SNOR_PROTO_1_1_1, info);
 
 	if (!(info->flags & SPI_NOR_NO_FR)) {
 		params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
 		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST],
 					  0, 8, SPINOR_OP_READ_FAST,
-					  SNOR_PROTO_1_1_1);
+					  SNOR_PROTO_1_1_1, info);
 	}
 
 	if (info->flags & SPI_NOR_DUAL_READ) {
 		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
 		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_2],
 					  0, 8, SPINOR_OP_READ_1_1_2,
-					  SNOR_PROTO_1_1_2);
+					  SNOR_PROTO_1_1_2, info);
 	}
 
 	if (info->flags & SPI_NOR_DUAL_IO_READ) {
 		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
 		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_2_2],
 					  0, 8, SPINOR_OP_READ_1_2_2,
-					  SNOR_PROTO_1_2_2);
+					  SNOR_PROTO_1_2_2, info);
 	}
 
 	if (info->flags & SPI_NOR_QUAD_READ) {
 		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
 		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_4],
 					  0, 8, SPINOR_OP_READ_1_1_4,
-					  SNOR_PROTO_1_1_4);
+					  SNOR_PROTO_1_1_4, info);
 	}
 
 	if (info->flags & SPI_NOR_QUAD_IO_READ) {
 		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_4_4;
 		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_4_4],
 					  0, 10, SPINOR_OP_READ_1_4_4,
-					  SNOR_PROTO_1_4_4);
+					  SNOR_PROTO_1_4_4, info);
 	}
 
 
@@ -2519,7 +2529,8 @@ static int spi_nor_hwcaps_pp2cmd(u32 hwcaps)
 
 static int spi_nor_select_read(struct spi_nor *nor,
 			       const struct spi_nor_flash_parameter *params,
-			       u32 shared_hwcaps)
+			       u32 shared_hwcaps,
+			       const struct flash_info *info)
 {
 	int cmd, best_match = ffs(shared_hwcaps & SNOR_HWCAPS_READ_MASK) - 1;
 	const struct spi_nor_read_command *read;
@@ -2535,6 +2546,10 @@ static int spi_nor_select_read(struct spi_nor *nor,
 	nor->read_opcode = read->opcode;
 	nor->read_proto = read->proto;
 
+	if (read->dummy_config &&
+	    !read->dummy_config(nor, read->num_wait_states, info))
+		return -EINVAL;
+
 	/*
 	 * In the spi-nor framework, we don't need to make the difference
 	 * between mode clock cycles and wait state clock cycles.
@@ -2546,6 +2561,7 @@ static int spi_nor_select_read(struct spi_nor *nor,
 	 * into the so called dummy clock cycles.
 	 */
 	nor->read_dummy = read->num_mode_clocks + read->num_wait_states;
+
 	return 0;
 }
 
@@ -2622,7 +2638,7 @@ static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info,
 	}
 
 	/* Select the (Fast) Read command. */
-	err = spi_nor_select_read(nor, params, shared_mask);
+	err = spi_nor_select_read(nor, params, shared_mask, info);
 	if (err) {
 		dev_err(nor->dev,
 			"can't select read settings supported by both the SPI controller and memory.\n");
-- 
2.7.4

[RFC 04/10] mtd: spi-nor: Configure read latency for micron flashes

[Prabhakar Kushwaha]

Volatile Configuration Register[4:7] defines Number of read latency
i.e. dummy clock cycles. This patch program volatile configuration
register using READ VOLATILE CONFIGURATION REGISTER(85h) and
WRITE VOLATILE CONFIGURATION REGISTER(81h) command

Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 29 +++++++++++++++++++++++++++++
 include/linux/mtd/spi-nor.h   |  2 ++
 2 files changed, 31 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 7d94874..dd1a771 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1638,6 +1638,31 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
+static int micron_dummy_config(struct spi_nor *nor, u8 num_wait_states,
+			       const struct flash_info *info)
+{
+	int ret;
+	u8 val;
+
+	ret = nor->read_reg(nor, SPINOR_OP_RD_VCR, &val, 1);
+	if (ret < 0) {
+		dev_err(nor->dev, "error %d reading CR\n", ret);
+		return ret;
+	}
+
+	val &= 0x0F;
+	val |= num_wait_states << 4;
+
+	ret = nor->write_reg(nor, SPINOR_OP_WD_VCR, &val, 1);
+	if (ret < 0) {
+		dev_err(nor->dev,
+			"error while writing configuration register\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int spi_nor_check(struct spi_nor *nor)
 {
 	if (!nor->dev || !nor->read || !nor->write ||
@@ -1772,6 +1797,10 @@ spi_nor_set_read_settings(struct spi_nor_read_command *read,
 	read->proto = proto;
 
 	switch (JEDEC_MFR(info)) {
+	case SNOR_MFR_MICRON:
+		read->dummy_config = micron_dummy_config;
+		break;
+
 	default:
 		read->dummy_config = NULL;
 		break;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 1f0a7fc..6c62aff 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -110,6 +110,8 @@
 /* Used for Micron flashes only. */
 #define SPINOR_OP_RD_EVCR      0x65    /* Read EVCR register */
 #define SPINOR_OP_WD_EVCR      0x61    /* Write EVCR register */
+#define SPINOR_OP_RD_VCR       0x85     /* Read VCR register */
+#define SPINOR_OP_WD_VCR       0x81     /* Write VCR register */
 
 /* Status Register bits. */
 #define SR_WIP			BIT(0)	/* Write in progress */
-- 
2.7.4

[RFC 05/10] Documentation: binding: Update fsl-quadspi binding

[Prabhakar Kushwaha]

QuadSPI controller supports Single, dual, quad modes of operation.
Mode is controlled via either RCW or the on-board connection to flash.

spi-tx-bus-width, spi-rx-bus-width binding added to define mode
of operation. Definition of these binding are standard as defined
in Documentation/devicetree/bindings/spi/spi-bus.txt.

Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
---
 Documentation/devicetree/bindings/mtd/fsl-quadspi.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/fsl-quadspi.txt b/Documentation/devicetree/bindings/mtd/fsl-quadspi.txt
index c34aa6f..13a903e 100644
--- a/Documentation/devicetree/bindings/mtd/fsl-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/fsl-quadspi.txt
@@ -23,6 +23,10 @@ Optional properties:
 			      bus, you should enable this property.
 			      (Please check the board's schematic.)
   - big-endian : That means the IP register is big endian
+  - spi-tx-bus-width - Definition as per
+                     Documentation/devicetree/bindings/spi/spi-bus.txt
+  - spi-rx-bus-width - Definition as per
+                     Documentation/devicetree/bindings/spi/spi-bus.txt
 
 Example:
 
-- 
2.7.4

[RFC 06/10] mtd: spi-nor: Update hwcap based on mode of fsl-quadspi

[Prabhakar Kushwaha]

FSL QuadSPI controller supports Single, dual, quad modes of operation.
Mode information is available via "spi-tx-bus-width" and
"spi-rx-bus-width" nodes of device tree.

Update read hwcap capability by reading "spi-rx-bus-width".
1_1_1 (SNOR_HWCAPS_PP) is selected as default write hwcap capability (PP).

Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index f17d224..5477d78 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -957,9 +957,8 @@ static void fsl_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
 
 static int fsl_qspi_probe(struct platform_device *pdev)
 {
-	const struct spi_nor_hwcaps hwcaps = {
-		.mask = SNOR_HWCAPS_READ_1_1_4 |
-			SNOR_HWCAPS_PP,
+	struct spi_nor_hwcaps hwcaps = {
+		.mask = SNOR_HWCAPS_PP,
 	};
 	struct device_node *np = pdev->dev.of_node;
 	struct device *dev = &pdev->dev;
@@ -967,7 +966,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct spi_nor *nor;
 	struct mtd_info *mtd;
-	int ret, i = 0;
+	int ret, i = 0, value;
 
 	q = devm_kzalloc(dev, sizeof(*q), GFP_KERNEL);
 	if (!q)
@@ -1015,6 +1014,28 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 		goto clk_failed;
 	}
 
+	if (!of_property_read_u32(np, "spi-rx-bus-width", &value)) {
+		switch (value) {
+		case 1:
+			hwcaps.mask |= SNOR_HWCAPS_READ | SNOR_HWCAPS_READ_FAST;
+			break;
+		case 2:
+			hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2 |
+				SNOR_HWCAPS_READ_1_2_2 | SNOR_HWCAPS_READ_2_2_2;
+			break;
+		case 4:
+			hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4 |
+				SNOR_HWCAPS_READ_1_4_4 | SNOR_HWCAPS_READ_4_4_4;
+			break;
+		default:
+			dev_err(dev,
+				"spi-rx-bus-width %d not supported\n",
+				value);
+			break;
+		}
+	} else
+		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
+
 	/* find the irq */
 	ret = platform_get_irq(pdev, 0);
 	if (ret < 0) {
-- 
2.7.4

[RFC 07/10] mtd: spi-nor: Add support of read/write any reg commands

[Prabhakar Kushwaha]

Spansion flash provide support of Read Any Register and Write Any
Register commands.  These commands provides a way to read or write
all device registers - non-volatile and volatile.

These commands require special signature and handing from underlying
flash controller.

Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
---
 include/linux/mtd/spi-nor.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 6c62aff..b6dfa25 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -256,6 +256,8 @@ enum spi_nor_option_flags {
  *			read/write/erase/lock/unlock operations
  * @read_reg:		[DRIVER-SPECIFIC] read out the register
  * @write_reg:		[DRIVER-SPECIFIC] write data to the register
+ * @read_anyreg:	[DRIVER-SPECIFIC] read out from any register
+ * @write_anyreg	[DRIVER-SPECIFIC] write data to any register
  * @read:		[DRIVER-SPECIFIC] read data from the SPI NOR
  * @write:		[DRIVER-SPECIFIC] write data to the SPI NOR
  * @erase:		[DRIVER-SPECIFIC] erase a sector of the SPI NOR
@@ -288,6 +290,10 @@ struct spi_nor {
 	void (*unprepare)(struct spi_nor *nor, enum spi_nor_ops ops);
 	int (*read_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len);
 	int (*write_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len);
+	int (*read_anyreg)(struct spi_nor *nor, u8 opcode, u32 offset,
+			   u8 *buf, int len);
+	int (*write_anyreg)(struct spi_nor *nor, u8 opcode, u32 offset,
+			    u8 *buf, int len);
 
 	ssize_t (*read)(struct spi_nor *nor, loff_t from,
 			size_t len, u_char *read_buf);
-- 
2.7.4

[RFC 08/10] mtd: spi-nor: Add support of SPANSION S25FS-S flash

[Prabhakar Kushwaha]

S25FS512S is a 512 Mbit, 1.8 V Serial Peripheral Interface with
Multi-I/O Flash. It provide support of read (Fast, Dual I/O,
Quad I/O, DDR Quad I/O), program and erase(hybrid, uniform sector).

Read commands require latency cycles which are configured via volatile
configurations register 2 (CR2V). Dual I/O and Quad I/O read require
mode cycles.

This patch add support SPANSION S25FS-S flash family and update
required structure, registers.

Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 68 ++++++++++++++++++++++++++++++++++++++-----
 include/linux/mtd/spi-nor.h   | 10 +++++++
 2 files changed, 71 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index dd1a771..58c37f6f 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -93,7 +93,8 @@ struct flash_info {
 #define SPI_NOR_QUAD_IO_READ	BIT(16)	/* Flash supports Quad IO Read */
 };
 
-#define JEDEC_MFR(info)	((info)->id[0])
+#define JEDEC_MFR(info)		((info)->id[0])
+#define JEDEC_EXT_ID(info)	((info)->id[5])
 
 static const struct flash_info *spi_nor_match_id(const char *name);
 
@@ -1056,6 +1057,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) },
+	{ "s25fs512s",  INFO6(0x010220, 0x4d0081, 256 * 1024, 256, SPI_NOR_DUAL_IO_READ | SPI_NOR_QUAD_IO_READ) },
 	{ "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) },
 	{ "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
 	{ "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
@@ -1663,6 +1665,33 @@ static int micron_dummy_config(struct spi_nor *nor, u8 num_wait_states,
 	return 0;
 }
 
+static int spansion_dummy_config(struct spi_nor *nor, u8 num_wait_states,
+				 const struct flash_info *info)
+{
+	int ret;
+	u8 val;
+
+	if (JEDEC_EXT_ID(info) == SPINOR_S25FS_FAMILY_ID) {
+		ret = nor->read_anyreg(nor, SPINOR_OP_RDAR, SPANSION_CR2V_OFF,
+				       &val, 1);
+		if (ret < 0) {
+			dev_err(nor->dev, "error %d reading CR2V\n", ret);
+			return ret;
+		}
+		val &= 0xF0;
+		val |= num_wait_states;
+
+		ret = nor->write_anyreg(nor, SPINOR_OP_WRAR, SPANSION_CR2V_OFF,
+					&val, 1);
+		if (ret < 0) {
+			dev_err(nor->dev, "error %d writing CR2V\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 static int spi_nor_check(struct spi_nor *nor)
 {
 	if (!nor->dev || !nor->read || !nor->write ||
@@ -1801,6 +1830,10 @@ spi_nor_set_read_settings(struct spi_nor_read_command *read,
 		read->dummy_config = micron_dummy_config;
 		break;
 
+	case SNOR_MFR_SPANSION:
+		read->dummy_config = spansion_dummy_config;
+		break;
+
 	default:
 		read->dummy_config = NULL;
 		break;
@@ -2413,6 +2446,8 @@ static int spi_nor_init_params(struct spi_nor *nor,
 			       const struct flash_info *info,
 			       struct spi_nor_flash_parameter *params)
 {
+	u8 mode = 0;
+
 	/* Set legacy flash parameters as default. */
 	memset(params, 0, sizeof(*params));
 
@@ -2423,41 +2458,60 @@ static int spi_nor_init_params(struct spi_nor *nor,
 	/* (Fast) Read settings. */
 	params->hwcaps.mask |= SNOR_HWCAPS_READ;
 	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ],
-				  0, 0, SPINOR_OP_READ,
+				  mode, 0, SPINOR_OP_READ,
 				  SNOR_PROTO_1_1_1, info);
 
 	if (!(info->flags & SPI_NOR_NO_FR)) {
 		params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
 		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST],
-					  0, 8, SPINOR_OP_READ_FAST,
+					  mode, 8, SPINOR_OP_READ_FAST,
 					  SNOR_PROTO_1_1_1, info);
 	}
 
 	if (info->flags & SPI_NOR_DUAL_READ) {
 		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
 		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_2],
-					  0, 8, SPINOR_OP_READ_1_1_2,
+					  mode, 8, SPINOR_OP_READ_1_1_2,
 					  SNOR_PROTO_1_1_2, info);
 	}
 
 	if (info->flags & SPI_NOR_DUAL_IO_READ) {
 		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
+
+		switch (JEDEC_EXT_ID(info)) {
+		case SPINOR_S25FS_FAMILY_ID:
+			mode = 4;
+			break;
+		default:
+			mode = 0;
+			break;
+		}
 		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_2_2],
-					  0, 8, SPINOR_OP_READ_1_2_2,
+					  mode, 8, SPINOR_OP_READ_1_2_2,
 					  SNOR_PROTO_1_2_2, info);
 	}
 
 	if (info->flags & SPI_NOR_QUAD_READ) {
 		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
 		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_4],
-					  0, 8, SPINOR_OP_READ_1_1_4,
+					  mode, 8, SPINOR_OP_READ_1_1_4,
 					  SNOR_PROTO_1_1_4, info);
 	}
 
 	if (info->flags & SPI_NOR_QUAD_IO_READ) {
 		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_4_4;
+
+		switch (JEDEC_EXT_ID(info)) {
+		case SPINOR_S25FS_FAMILY_ID:
+			mode = 2;
+			break;
+		default:
+			mode = 0;
+			break;
+		}
+
 		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_4_4],
-					  0, 10, SPINOR_OP_READ_1_4_4,
+					  mode, 10, SPINOR_OP_READ_1_4_4,
 					  SNOR_PROTO_1_4_4, info);
 	}
 
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index b6dfa25..9ff96cb 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -29,6 +29,8 @@
 #define SNOR_MFR_SST		CFI_MFR_SST
 #define SNOR_MFR_WINBOND	0xef /* Also used by some Spansion */
 
+#define SPINOR_S25FS_FAMILY_ID	0x81
+
 /*
  * Note on opcode nomenclature: some opcodes have a format like
  * SPINOR_OP_FUNCTION{4,}_x_y_z. The numbers x, y, and z stand for the number
@@ -106,6 +108,14 @@
 /* Used for Spansion flashes only. */
 #define SPINOR_OP_BRWR		0x17	/* Bank register write */
 #define SPINOR_OP_CLSR		0x30	/* Clear status register 1 */
+#define SPINOR_OP_RDAR		0x65	/* Read any register */
+#define SPINOR_OP_WRAR		0x71	/* Write any register */
+#define SPANSION_SR1V_OFF	0x00800000 /* SR1V offset */
+#define SPANSION_SR2V_OFF	0x00800001 /* SR2V offset */
+#define SPANSION_CR1V_OFF	0x00800002 /* CR1V offset */
+#define SPANSION_CR2V_OFF	0x00800003 /* CR2V offset */
+#define SPANSION_CR3V_OFF	0x00800004 /* CR3V offset */
+
 
 /* Used for Micron flashes only. */
 #define SPINOR_OP_RD_EVCR      0x65    /* Read EVCR register */
-- 
2.7.4

[RFC 09/10] mtd: spi-nor: Disable hybrid mode for SPANSION S25FS-S family

[Prabhakar Kushwaha]

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 default status of the flash is the hybrid architecture.
The parameter sectors and the uniform sectors have different erase
commands.

This patch disables the hybrid sector architecture which makes the
flash have uniform sector size and uniform erase command.

Signed-off-by: Rajat Srivastava <rajat.srivastava@nxp.com>
Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 49 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spi-nor.h   |  1 +
 2 files changed, 50 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 58c37f6f..ca2ae44 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1692,6 +1692,33 @@ static int spansion_dummy_config(struct spi_nor *nor, u8 num_wait_states,
 	return 0;
 }
 
+static int spansion_s25fs_disable_hybrid_mode(struct spi_nor *nor)
+{
+	u8 cr3v = 0x0;
+	int ret = 0x0;
+
+	ret = nor->read_anyreg(nor, SPINOR_OP_RDAR, SPANSION_CR3V_OFF,
+			       &cr3v, 1);
+	if (ret < 0) {
+		dev_err(nor->dev, "error %d reading CR3V\n", ret);
+		return ret;
+	}
+
+	/* CR3V bit3: 4-KB Erase */
+	if (cr3v & SPANSION_CR3V_4KB_ERASE_DISABLE)
+		return 0;
+
+	cr3v |= SPANSION_CR3V_4KB_ERASE_DISABLE;
+	ret = nor->write_anyreg(nor, SPINOR_OP_RDAR, SPANSION_CR3V_OFF,
+				&cr3v, 1);
+	if (ret < 0) {
+		dev_err(nor->dev, "error %d writing CR3V\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int spi_nor_check(struct spi_nor *nor)
 {
 	if (!nor->dev || !nor->read || !nor->write ||
@@ -2939,6 +2966,28 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 			return ret;
 	}
 
+	/*
+	 * 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.
+	 * The following code removes the 4-kB parameter sectors from the
+	 * address map i.e. it disables the hybrid mode so that all sectors are
+	 * uniform size.
+	 */
+	if (JEDEC_EXT_ID(info) == SPINOR_S25FS_FAMILY_ID) {
+		ret = spansion_s25fs_disable_hybrid_mode(nor);
+		if (ret)
+			return ret;
+	}
+
 	dev_info(dev, "%s (%lld Kbytes)\n", info->name,
 			(long long)mtd->size >> 10);
 
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 9ff96cb..6c34e00 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -115,6 +115,7 @@
 #define SPANSION_CR1V_OFF	0x00800002 /* CR1V offset */
 #define SPANSION_CR2V_OFF	0x00800003 /* CR2V offset */
 #define SPANSION_CR3V_OFF	0x00800004 /* CR3V offset */
+#define SPANSION_CR3V_4KB_ERASE_DISABLE	0x8
 
 
 /* Used for Micron flashes only. */
-- 
2.7.4

[RFC 10/10] mtd: spi-nor: Implement anyreg functions for fsl-quadspi

[Prabhakar Kushwaha]

Read Any Register and Write Any Register commands provides a way to
read or write all device registers - non-volatile and volatile.
These commands are followed by a 3- or 4-byte address, followed by
a number of latency (dummy) cycles set by volatile configuration
registers. Then the selected register contents are returned or written.

This patch update quadspi controller driver to implement above behavior.

Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 5477d78..5164eab 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -854,6 +854,22 @@ static int fsl_qspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 	return ret;
 }
 
+static int fsl_qspi_read_anyreg(struct spi_nor *nor, u8 opcode, u32 offset,
+				u8 *buf, int len)
+{
+	/* TODO: It will be updated during controller changes*/
+
+	return 0;
+}
+
+static int fsl_qspi_write_anyreg(struct spi_nor *nor, u8 opcode, u32 offset,
+				 u8 *buf, int len)
+{
+	/* TODO: It will be updated during controller changes*/
+
+	return 0;
+}
+
 static ssize_t fsl_qspi_write(struct spi_nor *nor, loff_t to,
 			      size_t len, const u_char *buf)
 {
@@ -1075,6 +1091,8 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 		/* fill the hooks */
 		nor->read_reg = fsl_qspi_read_reg;
 		nor->write_reg = fsl_qspi_write_reg;
+		nor->read_anyreg = fsl_qspi_read_anyreg;
+		nor->write_anyreg = fsl_qspi_write_anyreg;
 		nor->read = fsl_qspi_read;
 		nor->write = fsl_qspi_write;
 		nor->erase = fsl_qspi_erase;
-- 
2.7.4

Re: [RFC 02/10] mtd: spi-nor: Use the highest supported READ protocol

[Cyrille Pitchen]

Hi Prabhakar,

Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
> HWCAP read capabilities defines higher bit position as the higher
> priority. i.e. use Octo SPI protocols first, then Quad SPI protocols
> before Dual SPI protocols, Fast Read and lastly (Slow) Read.
> 
> Currently implementation chose the last set bit i.e. slowest read.
> This patch select the highest available read capability.
> 
> Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 7d3b52f..01898e1 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2521,7 +2521,7 @@ static int spi_nor_select_read(struct spi_nor *nor,
>  			       const struct spi_nor_flash_parameter *params,
>  			       u32 shared_hwcaps)
>  {
> -	int cmd, best_match = fls(shared_hwcaps & SNOR_HWCAPS_READ_MASK) - 1;
> +	int cmd, best_match = ffs(shared_hwcaps & SNOR_HWCAPS_READ_MASK) - 1;

I'm pretty sure this patch is wrong and fls() - find last
(most-significant) bit set - is actually what we want to select the best match.

Is the implementation of fls() correct on your platform ?

Best regards,

Cyrille

>  	const struct spi_nor_read_command *read;
>  
>  	if (best_match < 0)
> 

Re: [RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ protocols

[Cyrille Pitchen]

Hi Prabhakar,

Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
> IO READ protocols transfers both address and data on multiple
> data bits. 1-2-2(DUAL IO), 1-4-4(QUAD IO) transfer address on 2
> data bits or 4 bits per rising edge of SCK respectively.
> 
> This patch update spi_nor_flash_parameter->spi_nor_read_command
> array based on DUAL or QUAD IO flag enabled in flash_info for a flash.
> 
> Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 19c00072..7d3b52f 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -61,7 +61,7 @@ struct flash_info {
>  	u16		page_size;
>  	u16		addr_width;
>  
> -	u16		flags;
> +	u32		flags;
>  #define SECT_4K			BIT(0)	/* SPINOR_OP_BE_4K works uniformly */
>  #define SPI_NOR_NO_ERASE	BIT(1)	/* No erase command needed */
>  #define SST_WRITE		BIT(2)	/* use SST byte programming */
> @@ -89,6 +89,8 @@ struct flash_info {
>  #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip erase */
>  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
>  #define USE_CLSR		BIT(14)	/* use CLSR command */
> +#define SPI_NOR_DUAL_IO_READ	BIT(15)	/* Flash supports Dual IO Read */
> +#define SPI_NOR_QUAD_IO_READ	BIT(16)	/* Flash supports Quad IO Read */
>  };
>  
>  #define JEDEC_MFR(info)	((info)->id[0])
> @@ -2399,6 +2401,13 @@ static int spi_nor_init_params(struct spi_nor *nor,
>  					  SNOR_PROTO_1_1_2);
>  	}
>  
> +	if (info->flags & SPI_NOR_DUAL_IO_READ) {
> +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
> +		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_2_2],
> +					  0, 8, SPINOR_OP_READ_1_2_2,
> +					  SNOR_PROTO_1_2_2);
> +	}
> +
>  	if (info->flags & SPI_NOR_QUAD_READ) {
>  		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
>  		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_4],
> @@ -2406,6 +2415,14 @@ static int spi_nor_init_params(struct spi_nor *nor,
>  					  SNOR_PROTO_1_1_4);
>  	}
>  
> +	if (info->flags & SPI_NOR_QUAD_IO_READ) {
> +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_4_4;
> +		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_4_4],
> +					  0, 10, SPINOR_OP_READ_1_4_4,

The actual number of mode and wait state clock cycles depend on the
manufacturer and the memory part number.

Here are few examples of factory settings for Fast Read 1-4-4
(mode/wait states):

Micron: 1/9
Macronix: 2/4
Spansion: 2/8

AFAIK, 0/10 is not used by any manufacturer and doesn't work with Macronix.

For Fast Read 1-1-1, 1-1-2 and 1-1-4, all memories I know use 0/8. I guess
those numbers were chosen for 1-1-2 and 1-1-4 too to be backward compatible
with older memories supporting only Fast Read 1-1-1, which always uses no
node cycle and 8 wait-states.

However with the introduction of Fast Read 1-2-2 and 1-4-4, things became
messy and every manufacturer used its own settings. That's why we didn't
introduce SPI_NOR_DUAL_IO_READ of SPI_NOR_QUAD_IO_READ flags: because we
can associate them settings that would apply to all manufacturers.

We can still use Fast Read 1-2-2 and 1-4-4 when supported by both the
memory and the SPI controller by parsing the memory SFDP tables and filling
the 'struct spi_nor_flash_parameter' accordingly.

We (MTD maintainers) don't want to add more hard-coded data in the
'struct flash_info' and in the spi_nor_ids[] array when those data can
actually be retrieved from the SFDP tables.

Latest SPI NOR memories almost all support SFDP tables (JEDEC JESD216B).
I had a quick overview of you series and it seems that you have been
testing with both Micron and Spansion memory parts: both should support
SFDP.

So sorry but I will reject this patch: as I said unlike Fast Read 1-1-z
protocols which always use 0 mode cycle and 8 wait states, there are no
settings for Fast Read 1-4-4 or 1-2-2 that could be valid for all
memory manufacturers.

Best regards,

Cyrille

> +					  SNOR_PROTO_1_4_4);
> +	}
> +
> +
>  	/* Page Program settings. */
>  	params->hwcaps.mask |= SNOR_HWCAPS_PP;
>  	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP],
> @@ -2432,7 +2449,8 @@ static int spi_nor_init_params(struct spi_nor *nor,
>  	/* Override the parameters with data read from SFDP tables. */
>  	nor->addr_width = 0;
>  	nor->mtd.erasesize = 0;
> -	if ((info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)) &&
> +	if ((info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> +	    SPI_NOR_DUAL_IO_READ | SPI_NOR_QUAD_IO_READ)) &&
>  	    !(info->flags & SPI_NOR_SKIP_SFDP)) {
>  		struct spi_nor_flash_parameter sfdp_params;
>  
> 

Re: [RFC 03/10] mtd: spi-nor: Configure read latency for read commands

[Cyrille Pitchen]

Hi Prabhakar,

Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
> All read commands have read latency (dummy cycle). It defines a
> period between the end of address or mode and the beginning of
> read data returning to the host. Flashes have default read latency.
> This default read latency may not match with the selected latency.
>

The read latency is either hard-coded (0 mode cycles and 8 wait-states)
for Fast Read 1-1-z operations or read from the Basic Flash Parameter
Table programmed as one mandatory table of the JEDEC JESD126B
specification when SFDP tables are supported by the memory parts.

In all cases, those values are factory settings. I don't understand why
there would be a mismatch between the default read latency and the
selected latency if any boot-loader or flash programming tool has ever
changed the memory settings.

Could you please explain in which case you found a mismatch ?

Best regards,

Cyrille
 
> So, selected latency needs to be programmed in flash via
> volatile configuration register. Considering flashes have different
> types of configuration registers/bits depending upon vendor/family.
> 
> This patch provides a way define function pointer per flash vendor
> to configure volatile configuration register. function pointer has
> flash_info parameter to take care of difference within the family of
> a vendor.
> 
> Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 01898e1..7d94874 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1695,6 +1695,9 @@ struct spi_nor_read_command {
>  	u8			num_wait_states;
>  	u8			opcode;
>  	enum spi_nor_protocol	proto;
> +
> +	int (*dummy_config)(struct spi_nor *nor, u8 num_wait_states,
> +			    const struct flash_info *info);
>  };
>  
>  struct spi_nor_pp_command {
> @@ -1760,12 +1763,19 @@ spi_nor_set_read_settings(struct spi_nor_read_command *read,
>  			  u8 num_mode_clocks,
>  			  u8 num_wait_states,
>  			  u8 opcode,
> -			  enum spi_nor_protocol proto)
> +			  enum spi_nor_protocol proto,
> +			  const struct flash_info *info)
>  {
>  	read->num_mode_clocks = num_mode_clocks;
>  	read->num_wait_states = num_wait_states;
>  	read->opcode = opcode;
>  	read->proto = proto;
> +
> +	switch (JEDEC_MFR(info)) {
> +	default:
> +		read->dummy_config = NULL;
> +		break;
> +	}
>  }
>  
>  static void
> @@ -2385,41 +2395,41 @@ static int spi_nor_init_params(struct spi_nor *nor,
>  	params->hwcaps.mask |= SNOR_HWCAPS_READ;
>  	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ],
>  				  0, 0, SPINOR_OP_READ,
> -				  SNOR_PROTO_1_1_1);
> +				  SNOR_PROTO_1_1_1, info);
>  
>  	if (!(info->flags & SPI_NOR_NO_FR)) {
>  		params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
>  		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST],
>  					  0, 8, SPINOR_OP_READ_FAST,
> -					  SNOR_PROTO_1_1_1);
> +					  SNOR_PROTO_1_1_1, info);
>  	}
>  
>  	if (info->flags & SPI_NOR_DUAL_READ) {
>  		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
>  		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_2],
>  					  0, 8, SPINOR_OP_READ_1_1_2,
> -					  SNOR_PROTO_1_1_2);
> +					  SNOR_PROTO_1_1_2, info);
>  	}
>  
>  	if (info->flags & SPI_NOR_DUAL_IO_READ) {
>  		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
>  		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_2_2],
>  					  0, 8, SPINOR_OP_READ_1_2_2,
> -					  SNOR_PROTO_1_2_2);
> +					  SNOR_PROTO_1_2_2, info);
>  	}
>  
>  	if (info->flags & SPI_NOR_QUAD_READ) {
>  		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
>  		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_4],
>  					  0, 8, SPINOR_OP_READ_1_1_4,
> -					  SNOR_PROTO_1_1_4);
> +					  SNOR_PROTO_1_1_4, info);
>  	}
>  
>  	if (info->flags & SPI_NOR_QUAD_IO_READ) {
>  		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_4_4;
>  		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_4_4],
>  					  0, 10, SPINOR_OP_READ_1_4_4,
> -					  SNOR_PROTO_1_4_4);
> +					  SNOR_PROTO_1_4_4, info);
>  	}
>  
>  
> @@ -2519,7 +2529,8 @@ static int spi_nor_hwcaps_pp2cmd(u32 hwcaps)
>  
>  static int spi_nor_select_read(struct spi_nor *nor,
>  			       const struct spi_nor_flash_parameter *params,
> -			       u32 shared_hwcaps)
> +			       u32 shared_hwcaps,
> +			       const struct flash_info *info)
>  {
>  	int cmd, best_match = ffs(shared_hwcaps & SNOR_HWCAPS_READ_MASK) - 1;
>  	const struct spi_nor_read_command *read;
> @@ -2535,6 +2546,10 @@ static int spi_nor_select_read(struct spi_nor *nor,
>  	nor->read_opcode = read->opcode;
>  	nor->read_proto = read->proto;
>  
> +	if (read->dummy_config &&
> +	    !read->dummy_config(nor, read->num_wait_states, info))
> +		return -EINVAL;
> +
>  	/*
>  	 * In the spi-nor framework, we don't need to make the difference
>  	 * between mode clock cycles and wait state clock cycles.
> @@ -2546,6 +2561,7 @@ static int spi_nor_select_read(struct spi_nor *nor,
>  	 * into the so called dummy clock cycles.
>  	 */
>  	nor->read_dummy = read->num_mode_clocks + read->num_wait_states;
> +
>  	return 0;
>  }
>  
> @@ -2622,7 +2638,7 @@ static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info,
>  	}
>  
>  	/* Select the (Fast) Read command. */
> -	err = spi_nor_select_read(nor, params, shared_mask);
> +	err = spi_nor_select_read(nor, params, shared_mask, info);
>  	if (err) {
>  		dev_err(nor->dev,
>  			"can't select read settings supported by both the SPI controller and memory.\n");
> 

Re: [RFC 09/10] mtd: spi-nor: Disable hybrid mode for SPANSION S25FS-S family

[Cyrille Pitchen]

Hi Prabhakar,

Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
> 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 default status of the flash is the hybrid architecture.
> The parameter sectors and the uniform sectors have different erase
> commands.
> 
> This patch disables the hybrid sector architecture which makes the
> flash have uniform sector size and uniform erase command.
>

This issue should be addressed in a generic way, not Spansion specific,
by adding support of the optional Sector Map Parameter Table (SFDP).
In case of non uniform memories, like this Spansion S25FS-S family or
SST26, the Sector Map Parameter Table becomes mandatory as a SFDP table.

>From the JESD216B specification, about the JEDEC Sector Map Parameter Table:
"""
This table is required when a memory device:
* Has sectors of more than one size
* Or, does not allow all Erase Type commands to be applied to all sectors.
"""

Best regards,

Cyrille
 
> Signed-off-by: Rajat Srivastava <rajat.srivastava@nxp.com>
> Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 49 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/spi-nor.h   |  1 +
>  2 files changed, 50 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 58c37f6f..ca2ae44 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1692,6 +1692,33 @@ static int spansion_dummy_config(struct spi_nor *nor, u8 num_wait_states,
>  	return 0;
>  }
>  
> +static int spansion_s25fs_disable_hybrid_mode(struct spi_nor *nor)
> +{
> +	u8 cr3v = 0x0;
> +	int ret = 0x0;
> +
> +	ret = nor->read_anyreg(nor, SPINOR_OP_RDAR, SPANSION_CR3V_OFF,
> +			       &cr3v, 1);
> +	if (ret < 0) {
> +		dev_err(nor->dev, "error %d reading CR3V\n", ret);
> +		return ret;
> +	}
> +
> +	/* CR3V bit3: 4-KB Erase */
> +	if (cr3v & SPANSION_CR3V_4KB_ERASE_DISABLE)
> +		return 0;
> +
> +	cr3v |= SPANSION_CR3V_4KB_ERASE_DISABLE;
> +	ret = nor->write_anyreg(nor, SPINOR_OP_RDAR, SPANSION_CR3V_OFF,
> +				&cr3v, 1);
> +	if (ret < 0) {
> +		dev_err(nor->dev, "error %d writing CR3V\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int spi_nor_check(struct spi_nor *nor)
>  {
>  	if (!nor->dev || !nor->read || !nor->write ||
> @@ -2939,6 +2966,28 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  			return ret;
>  	}
>  
> +	/*
> +	 * 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.
> +	 * The following code removes the 4-kB parameter sectors from the
> +	 * address map i.e. it disables the hybrid mode so that all sectors are
> +	 * uniform size.
> +	 */
> +	if (JEDEC_EXT_ID(info) == SPINOR_S25FS_FAMILY_ID) {
> +		ret = spansion_s25fs_disable_hybrid_mode(nor);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	dev_info(dev, "%s (%lld Kbytes)\n", info->name,
>  			(long long)mtd->size >> 10);
>  
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 9ff96cb..6c34e00 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -115,6 +115,7 @@
>  #define SPANSION_CR1V_OFF	0x00800002 /* CR1V offset */
>  #define SPANSION_CR2V_OFF	0x00800003 /* CR2V offset */
>  #define SPANSION_CR3V_OFF	0x00800004 /* CR3V offset */
> +#define SPANSION_CR3V_4KB_ERASE_DISABLE	0x8
>  
>  
>  /* Used for Micron flashes only. */
> 

RE: [RFC 02/10] mtd: spi-nor: Use the highest supported READ protocol

[Prabhakar Kushwaha]

Regards,
Prabhakar

> -----Original Message-----
> From: Cyrille Pitchen [mailto:cyrille.pitchen@wedev4u.fr]
> Sent: Wednesday, December 06, 2017 3:24 PM
> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; linux-
> mtd@lists.infradead.org
> Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com;
> dedekind1@gmail.com
> Subject: Re: [RFC 02/10] mtd: spi-nor: Use the highest supported READ protocol
> 
> Hi Prabhakar,
> 
> Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
> > HWCAP read capabilities defines higher bit position as the higher
> > priority. i.e. use Octo SPI protocols first, then Quad SPI protocols
> > before Dual SPI protocols, Fast Read and lastly (Slow) Read.
> >
> > Currently implementation chose the last set bit i.e. slowest read.
> > This patch select the highest available read capability.
> >
> > Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> > index 7d3b52f..01898e1 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -2521,7 +2521,7 @@ static int spi_nor_select_read(struct spi_nor *nor,
> >  			       const struct spi_nor_flash_parameter *params,
> >  			       u32 shared_hwcaps)
> >  {
> > -	int cmd, best_match = fls(shared_hwcaps &
> SNOR_HWCAPS_READ_MASK) - 1;
> > +	int cmd, best_match = ffs(shared_hwcaps &
> SNOR_HWCAPS_READ_MASK) - 1;
> 
> I'm pretty sure this patch is wrong and fls() - find last
> (most-significant) bit set - is actually what we want to select the best match.
> 

I misunderstood below line.   Thanks for correcting me. 

/*
 * fls() returns zero if the input is zero, otherwise returns the bit
 * position of the last set bit, where the LSB is 1 and MSB is 32.
 */

--pk

RE: [RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ protocols

[Prabhakar Kushwaha]

Hi Cyrille,


> -----Original Message-----
> From: Cyrille Pitchen [mailto:cyrille.pitchen@wedev4u.fr]
> Sent: Wednesday, December 06, 2017 4:04 PM
> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; linux-
> mtd@lists.infradead.org
> Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com;
> dedekind1@gmail.com
> Subject: Re: [RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ
> protocols
> 
> Hi Prabhakar,
> 
> Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
> > IO READ protocols transfers both address and data on multiple
> > data bits. 1-2-2(DUAL IO), 1-4-4(QUAD IO) transfer address on 2
> > data bits or 4 bits per rising edge of SCK respectively.
> >
> > This patch update spi_nor_flash_parameter->spi_nor_read_command
> > array based on DUAL or QUAD IO flag enabled in flash_info for a flash.
> >
> > Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c | 22 ++++++++++++++++++++--
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> > index 19c00072..7d3b52f 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -61,7 +61,7 @@ struct flash_info {
> >  	u16		page_size;
> >  	u16		addr_width;
> >
> > -	u16		flags;
> > +	u32		flags;
> >  #define SECT_4K			BIT(0)	/* SPINOR_OP_BE_4K works
> uniformly */
> >  #define SPI_NOR_NO_ERASE	BIT(1)	/* No erase command needed */
> >  #define SST_WRITE		BIT(2)	/* use SST byte programming */
> > @@ -89,6 +89,8 @@ struct flash_info {
> >  #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip
> erase */
> >  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
> >  #define USE_CLSR		BIT(14)	/* use CLSR command */
> > +#define SPI_NOR_DUAL_IO_READ	BIT(15)	/* Flash supports Dual IO Read
> */
> > +#define SPI_NOR_QUAD_IO_READ	BIT(16)	/* Flash supports Quad IO
> Read */
> >  };
> >
> >  #define JEDEC_MFR(info)	((info)->id[0])
> > @@ -2399,6 +2401,13 @@ static int spi_nor_init_params(struct spi_nor *nor,
> >  					  SNOR_PROTO_1_1_2);
> >  	}
> >
> > +	if (info->flags & SPI_NOR_DUAL_IO_READ) {
> > +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
> > +		spi_nor_set_read_settings(&params-
> >reads[SNOR_CMD_READ_1_2_2],
> > +					  0, 8, SPINOR_OP_READ_1_2_2,
> > +					  SNOR_PROTO_1_2_2);
> > +	}
> > +
> >  	if (info->flags & SPI_NOR_QUAD_READ) {
> >  		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
> >  		spi_nor_set_read_settings(&params-
> >reads[SNOR_CMD_READ_1_1_4],
> > @@ -2406,6 +2415,14 @@ static int spi_nor_init_params(struct spi_nor *nor,
> >  					  SNOR_PROTO_1_1_4);
> >  	}
> >
> > +	if (info->flags & SPI_NOR_QUAD_IO_READ) {
> > +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_4_4;
> > +		spi_nor_set_read_settings(&params-
> >reads[SNOR_CMD_READ_1_4_4],
> > +					  0, 10, SPINOR_OP_READ_1_4_4,
> 
> The actual number of mode and wait state clock cycles depend on the
> manufacturer and the memory part number.
> 
> Here are few examples of factory settings for Fast Read 1-4-4
> (mode/wait states):
> 
> Micron: 1/9
> Macronix: 2/4
> Spansion: 2/8
> 
> AFAIK, 0/10 is not used by any manufacturer and doesn't work with Macronix.
> 
> For Fast Read 1-1-1, 1-1-2 and 1-1-4, all memories I know use 0/8. I guess
> those numbers were chosen for 1-1-2 and 1-1-4 too to be backward compatible
> with older memories supporting only Fast Read 1-1-1, which always uses no
> node cycle and 8 wait-states.
> 
> However with the introduction of Fast Read 1-2-2 and 1-4-4, things became
> messy and every manufacturer used its own settings. That's why we didn't
> introduce SPI_NOR_DUAL_IO_READ of SPI_NOR_QUAD_IO_READ flags: because
> we
> can associate them settings that would apply to all manufacturers.
> 
> We can still use Fast Read 1-2-2 and 1-4-4 when supported by both the
> memory and the SPI controller by parsing the memory SFDP tables and filling
> the 'struct spi_nor_flash_parameter' accordingly.
> 
> We (MTD maintainers) don't want to add more hard-coded data in the
> 'struct flash_info' and in the spi_nor_ids[] array when those data can
> actually be retrieved from the SFDP tables.
> 
> Latest SPI NOR memories almost all support SFDP tables (JEDEC JESD216B).
> I had a quick overview of you series and it seems that you have been
> testing with both Micron and Spansion memory parts: both should support
> SFDP.
> 
> So sorry but I will reject this patch: as I said unlike Fast Read 1-1-z
> protocols which always use 0 mode cycle and 8 wait states, there are no
> settings for Fast Read 1-4-4 or 1-2-2 that could be valid for all
> memory manufacturers.
> 

I understand your point. Let me explore SFDP.

--pk

RE: [RFC 03/10] mtd: spi-nor: Configure read latency for read commands

[Prabhakar Kushwaha]

Hi Cyrille,

> -----Original Message-----
> From: Cyrille Pitchen [mailto:cyrille.pitchen@wedev4u.fr]
> Sent: Wednesday, December 06, 2017 4:16 PM
> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; linux-
> mtd@lists.infradead.org
> Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com;
> dedekind1@gmail.com
> Subject: Re: [RFC 03/10] mtd: spi-nor: Configure read latency for read commands
> 
> Hi Prabhakar,
> 
> Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
> > All read commands have read latency (dummy cycle). It defines a
> > period between the end of address or mode and the beginning of
> > read data returning to the host. Flashes have default read latency.
> > This default read latency may not match with the selected latency.
> >
> 
> The read latency is either hard-coded (0 mode cycles and 8 wait-states)
> for Fast Read 1-1-z operations or read from the Basic Flash Parameter
> Table programmed as one mandatory table of the JEDEC JESD126B
> specification when SFDP tables are supported by the memory parts.
> 
> In all cases, those values are factory settings. I don't understand why
> there would be a mismatch between the default read latency and the
> selected latency if any boot-loader or flash programming tool has ever
> changed the memory settings.
> 
> Could you please explain in which case you found a mismatch ?
> 
If we only talk about 1-1-z than yes. The Default value is always "0/8". 

This mismatch start to come when we see 1-2-2 or 1-4-4 read protocols.
For eg. S25FL128S/S25FL256S default dummy latency is 00. Which is 4 for  1-2-2 or 1-4-4.

I saw your review comments of using SFDP for 1-2-2 and 1-4-4. 
Let me explore this path if we really require this patch..

Thanks for guidance!!

--pk

RE: [RFC 09/10] mtd: spi-nor: Disable hybrid mode for SPANSION S25FS-S family

[Prabhakar Kushwaha]

> -----Original Message-----
> From: Cyrille Pitchen [mailto:cyrille.pitchen@wedev4u.fr]
> Sent: Wednesday, December 06, 2017 4:28 PM
> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; linux-
> mtd@lists.infradead.org
> Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com; Rajat
> Srivastava <rajat.srivastava@nxp.com>; dedekind1@gmail.com
> Subject: Re: [RFC 09/10] mtd: spi-nor: Disable hybrid mode for SPANSION S25FS-
> S family
> 
> Hi Prabhakar,
> 
> Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
> > 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 default status of the flash is the hybrid architecture.
> > The parameter sectors and the uniform sectors have different erase
> > commands.
> >
> > This patch disables the hybrid sector architecture which makes the
> > flash have uniform sector size and uniform erase command.
> >
> 
> This issue should be addressed in a generic way, not Spansion specific,
> by adding support of the optional Sector Map Parameter Table (SFDP).
> In case of non uniform memories, like this Spansion S25FS-S family or
> SST26, the Sector Map Parameter Table becomes mandatory as a SFDP table.
> 
> From the JESD216B specification, about the JEDEC Sector Map Parameter Table:
> """
> This table is required when a memory device:
> * Has sectors of more than one size
> * Or, does not allow all Erase Type commands to be applied to all sectors.
> """
> 

Thanks for the guidance. Let me explore Sector Map Parameter Table (SFDP)

--pk.

RE: [RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ protocols

[Prabhakar Kushwaha]

Thanks Cyrille for the pointers,

I have few queries on SFDP.

> -----Original Message-----
> From: linux-mtd [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of
> Prabhakar Kushwaha
> Sent: Thursday, December 07, 2017 2:15 PM
> To: Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>; linux-mtd@lists.infradead.org
> Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com;
> dedekind1@gmail.com
> Subject: RE: [RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ
> protocols
> 
> Hi Cyrille,
> 
> 
> > -----Original Message-----
> > From: Cyrille Pitchen [mailto:cyrille.pitchen@wedev4u.fr]
> > Sent: Wednesday, December 06, 2017 4:04 PM
> > To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; linux-
> > mtd@lists.infradead.org
> > Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com;
> > dedekind1@gmail.com
> > Subject: Re: [RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ
> > protocols
> >
> > Hi Prabhakar,
> >
> > Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
> > > IO READ protocols transfers both address and data on multiple
> > > data bits. 1-2-2(DUAL IO), 1-4-4(QUAD IO) transfer address on 2
> > > data bits or 4 bits per rising edge of SCK respectively.
> > >
> > > This patch update spi_nor_flash_parameter->spi_nor_read_command
> > > array based on DUAL or QUAD IO flag enabled in flash_info for a flash.
> > >
> > > Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> > > ---
> > >  drivers/mtd/spi-nor/spi-nor.c | 22 ++++++++++++++++++++--
> > >  1 file changed, 20 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> > > index 19c00072..7d3b52f 100644
> > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > @@ -61,7 +61,7 @@ struct flash_info {
> > >  	u16		page_size;
> > >  	u16		addr_width;
> > >
> > > -	u16		flags;
> > > +	u32		flags;
> > >  #define SECT_4K			BIT(0)	/* SPINOR_OP_BE_4K works
> > uniformly */
> > >  #define SPI_NOR_NO_ERASE	BIT(1)	/* No erase command needed */
> > >  #define SST_WRITE		BIT(2)	/* use SST byte programming */
> > > @@ -89,6 +89,8 @@ struct flash_info {
> > >  #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip
> > erase */
> > >  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
> > >  #define USE_CLSR		BIT(14)	/* use CLSR command */
> > > +#define SPI_NOR_DUAL_IO_READ	BIT(15)	/* Flash supports Dual IO Read
> > */
> > > +#define SPI_NOR_QUAD_IO_READ	BIT(16)	/* Flash supports Quad IO
> > Read */
> > >  };
> > >
> > >  #define JEDEC_MFR(info)	((info)->id[0])
> > > @@ -2399,6 +2401,13 @@ static int spi_nor_init_params(struct spi_nor
> *nor,
> > >  					  SNOR_PROTO_1_1_2);
> > >  	}
> > >
> > > +	if (info->flags & SPI_NOR_DUAL_IO_READ) {
> > > +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
> > > +		spi_nor_set_read_settings(&params-
> > >reads[SNOR_CMD_READ_1_2_2],
> > > +					  0, 8, SPINOR_OP_READ_1_2_2,
> > > +					  SNOR_PROTO_1_2_2);
> > > +	}
> > > +
> > >  	if (info->flags & SPI_NOR_QUAD_READ) {
> > >  		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
> > >  		spi_nor_set_read_settings(&params-
> > >reads[SNOR_CMD_READ_1_1_4],
> > > @@ -2406,6 +2415,14 @@ static int spi_nor_init_params(struct spi_nor
> *nor,
> > >  					  SNOR_PROTO_1_1_4);
> > >  	}
> > >
> > > +	if (info->flags & SPI_NOR_QUAD_IO_READ) {
> > > +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_4_4;
> > > +		spi_nor_set_read_settings(&params-
> > >reads[SNOR_CMD_READ_1_4_4],
> > > +					  0, 10, SPINOR_OP_READ_1_4_4,
> >
> > The actual number of mode and wait state clock cycles depend on the
> > manufacturer and the memory part number.
> >
> > Here are few examples of factory settings for Fast Read 1-4-4
> > (mode/wait states):
> >
> > Micron: 1/9
> > Macronix: 2/4
> > Spansion: 2/8
> >
> > AFAIK, 0/10 is not used by any manufacturer and doesn't work with Macronix.
> >
> > For Fast Read 1-1-1, 1-1-2 and 1-1-4, all memories I know use 0/8. I guess
> > those numbers were chosen for 1-1-2 and 1-1-4 too to be backward
> compatible
> > with older memories supporting only Fast Read 1-1-1, which always uses no
> > node cycle and 8 wait-states.
> >
> > However with the introduction of Fast Read 1-2-2 and 1-4-4, things became
> > messy and every manufacturer used its own settings. That's why we didn't
> > introduce SPI_NOR_DUAL_IO_READ of SPI_NOR_QUAD_IO_READ flags:
> because
> > we
> > can associate them settings that would apply to all manufacturers.
> >
> > We can still use Fast Read 1-2-2 and 1-4-4 when supported by both the
> > memory and the SPI controller by parsing the memory SFDP tables and filling
> > the 'struct spi_nor_flash_parameter' accordingly.
> >
> > We (MTD maintainers) don't want to add more hard-coded data in the
> > 'struct flash_info' and in the spi_nor_ids[] array when those data can
> > actually be retrieved from the SFDP tables.
> >
> > Latest SPI NOR memories almost all support SFDP tables (JEDEC JESD216B).
> > I had a quick overview of you series and it seems that you have been
> > testing with both Micron and Spansion memory parts: both should support
> > SFDP.
> >
> > So sorry but I will reject this patch: as I said unlike Fast Read 1-1-z
> > protocols which always use 0 mode cycle and 8 wait states, there are no
> > settings for Fast Read 1-4-4 or 1-2-2 that could be valid for all
> > memory manufacturers.
> >
> 
> I understand your point. Let me explore SFDP.
> 

I tried to understand the SFDP. 

Looks like struct sfdp_bfpt_read needs to be update to support mode bit. Other info(dummy cycle) is already part of current SFDP implementation. 

-pk

Re: [RFC 03/10] mtd: spi-nor: Configure read latency for read commands

[Cyrille Pitchen]

Le 07/12/2017 à 10:03, Prabhakar Kushwaha a écrit :
> Hi Cyrille,
> 
>> -----Original Message-----
>> From: Cyrille Pitchen [mailto:cyrille.pitchen@wedev4u.fr]
>> Sent: Wednesday, December 06, 2017 4:16 PM
>> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; linux-
>> mtd@lists.infradead.org
>> Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com;
>> dedekind1@gmail.com
>> Subject: Re: [RFC 03/10] mtd: spi-nor: Configure read latency for read commands
>>
>> Hi Prabhakar,
>>
>> Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
>>> All read commands have read latency (dummy cycle). It defines a
>>> period between the end of address or mode and the beginning of
>>> read data returning to the host. Flashes have default read latency.
>>> This default read latency may not match with the selected latency.
>>>
>>
>> The read latency is either hard-coded (0 mode cycles and 8 wait-states)
>> for Fast Read 1-1-z operations or read from the Basic Flash Parameter
>> Table programmed as one mandatory table of the JEDEC JESD126B
>> specification when SFDP tables are supported by the memory parts.
>>
>> In all cases, those values are factory settings. I don't understand why
>> there would be a mismatch between the default read latency and the
>> selected latency if any boot-loader or flash programming tool has ever
>> changed the memory settings.
>>
>> Could you please explain in which case you found a mismatch ?
>>
> If we only talk about 1-1-z than yes. The Default value is always "0/8". 
> 
> This mismatch start to come when we see 1-2-2 or 1-4-4 read protocols.
> For eg. S25FL128S/S25FL256S default dummy latency is 00. Which is 4 for  1-2-2 or 1-4-4.
>

If the memory is still configured to use its factory settings, for instance
latency code 00 for Spansion memories, and if this memory is complient with
the JEDEC JESD216 speficition, then we read from the Basic Flash Parameter
Table (BFPT), the only always mandatory SFDP table, to retrieve the number
of mode cycles and wait-states to be used with Fast Read 1-2-2 and 1-4-4:

- Fast Read 1-2-2 settings are written in the 4th DWORD of the BFPT.
- Fast Read 1-4-4 settings are written in the 3rd DWORD of the BFPT.

Please refer to the JESD216 specification and to the sfdp_bfpt_reads[]
array.

Hence, as long as you don't change the latency code of your Spansion memory
and keep it in its factory state, a proper value is set in nor->read_dummy
based on what have been read from the SFDP table (BFPT).

You should not have any mismatch between what was set in nor->read_dummy
and what you memory expects.
 
> I saw your review comments of using SFDP for 1-2-2 and 1-4-4. 
> Let me explore this path if we really require this patch..
> 
> Thanks for guidance!!
> 
> --pk
> 
> 
> 
> 

Re: [RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ protocols

[Cyrille Pitchen]

Le 07/12/2017 à 12:04, Prabhakar Kushwaha a écrit :
> Thanks Cyrille for the pointers,
> 
> I have few queries on SFDP.
> 
>> -----Original Message-----
>> From: linux-mtd [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of
>> Prabhakar Kushwaha
>> Sent: Thursday, December 07, 2017 2:15 PM
>> To: Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>; linux-mtd@lists.infradead.org
>> Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com;
>> dedekind1@gmail.com
>> Subject: RE: [RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ
>> protocols
>>
>> Hi Cyrille,
>>
>>
>>> -----Original Message-----
>>> From: Cyrille Pitchen [mailto:cyrille.pitchen@wedev4u.fr]
>>> Sent: Wednesday, December 06, 2017 4:04 PM
>>> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; linux-
>>> mtd@lists.infradead.org
>>> Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com;
>>> dedekind1@gmail.com
>>> Subject: Re: [RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ
>>> protocols
>>>
>>> Hi Prabhakar,
>>>
>>> Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
>>>> IO READ protocols transfers both address and data on multiple
>>>> data bits. 1-2-2(DUAL IO), 1-4-4(QUAD IO) transfer address on 2
>>>> data bits or 4 bits per rising edge of SCK respectively.
>>>>
>>>> This patch update spi_nor_flash_parameter->spi_nor_read_command
>>>> array based on DUAL or QUAD IO flag enabled in flash_info for a flash.
>>>>
>>>> Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
>>>> ---
>>>>  drivers/mtd/spi-nor/spi-nor.c | 22 ++++++++++++++++++++--
>>>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>>> index 19c00072..7d3b52f 100644
>>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>>> @@ -61,7 +61,7 @@ struct flash_info {
>>>>  	u16		page_size;
>>>>  	u16		addr_width;
>>>>
>>>> -	u16		flags;
>>>> +	u32		flags;
>>>>  #define SECT_4K			BIT(0)	/* SPINOR_OP_BE_4K works
>>> uniformly */
>>>>  #define SPI_NOR_NO_ERASE	BIT(1)	/* No erase command needed */
>>>>  #define SST_WRITE		BIT(2)	/* use SST byte programming */
>>>> @@ -89,6 +89,8 @@ struct flash_info {
>>>>  #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip
>>> erase */
>>>>  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
>>>>  #define USE_CLSR		BIT(14)	/* use CLSR command */
>>>> +#define SPI_NOR_DUAL_IO_READ	BIT(15)	/* Flash supports Dual IO Read
>>> */
>>>> +#define SPI_NOR_QUAD_IO_READ	BIT(16)	/* Flash supports Quad IO
>>> Read */
>>>>  };
>>>>
>>>>  #define JEDEC_MFR(info)	((info)->id[0])
>>>> @@ -2399,6 +2401,13 @@ static int spi_nor_init_params(struct spi_nor
>> *nor,
>>>>  					  SNOR_PROTO_1_1_2);
>>>>  	}
>>>>
>>>> +	if (info->flags & SPI_NOR_DUAL_IO_READ) {
>>>> +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
>>>> +		spi_nor_set_read_settings(&params-
>>>> reads[SNOR_CMD_READ_1_2_2],
>>>> +					  0, 8, SPINOR_OP_READ_1_2_2,
>>>> +					  SNOR_PROTO_1_2_2);
>>>> +	}
>>>> +
>>>>  	if (info->flags & SPI_NOR_QUAD_READ) {
>>>>  		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
>>>>  		spi_nor_set_read_settings(&params-
>>>> reads[SNOR_CMD_READ_1_1_4],
>>>> @@ -2406,6 +2415,14 @@ static int spi_nor_init_params(struct spi_nor
>> *nor,
>>>>  					  SNOR_PROTO_1_1_4);
>>>>  	}
>>>>
>>>> +	if (info->flags & SPI_NOR_QUAD_IO_READ) {
>>>> +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_4_4;
>>>> +		spi_nor_set_read_settings(&params-
>>>> reads[SNOR_CMD_READ_1_4_4],
>>>> +					  0, 10, SPINOR_OP_READ_1_4_4,
>>>
>>> The actual number of mode and wait state clock cycles depend on the
>>> manufacturer and the memory part number.
>>>
>>> Here are few examples of factory settings for Fast Read 1-4-4
>>> (mode/wait states):
>>>
>>> Micron: 1/9
>>> Macronix: 2/4
>>> Spansion: 2/8
>>>
>>> AFAIK, 0/10 is not used by any manufacturer and doesn't work with Macronix.
>>>
>>> For Fast Read 1-1-1, 1-1-2 and 1-1-4, all memories I know use 0/8. I guess
>>> those numbers were chosen for 1-1-2 and 1-1-4 too to be backward
>> compatible
>>> with older memories supporting only Fast Read 1-1-1, which always uses no
>>> node cycle and 8 wait-states.
>>>
>>> However with the introduction of Fast Read 1-2-2 and 1-4-4, things became
>>> messy and every manufacturer used its own settings. That's why we didn't
>>> introduce SPI_NOR_DUAL_IO_READ of SPI_NOR_QUAD_IO_READ flags:
>> because
>>> we
>>> can associate them settings that would apply to all manufacturers.
>>>
>>> We can still use Fast Read 1-2-2 and 1-4-4 when supported by both the
>>> memory and the SPI controller by parsing the memory SFDP tables and filling
>>> the 'struct spi_nor_flash_parameter' accordingly.
>>>
>>> We (MTD maintainers) don't want to add more hard-coded data in the
>>> 'struct flash_info' and in the spi_nor_ids[] array when those data can
>>> actually be retrieved from the SFDP tables.
>>>
>>> Latest SPI NOR memories almost all support SFDP tables (JEDEC JESD216B).
>>> I had a quick overview of you series and it seems that you have been
>>> testing with both Micron and Spansion memory parts: both should support
>>> SFDP.
>>>
>>> So sorry but I will reject this patch: as I said unlike Fast Read 1-1-z
>>> protocols which always use 0 mode cycle and 8 wait states, there are no
>>> settings for Fast Read 1-4-4 or 1-2-2 that could be valid for all
>>> memory manufacturers.
>>>
>>
>> I understand your point. Let me explore SFDP.
>>
> 
> I tried to understand the SFDP. 
> 
> Looks like struct sfdp_bfpt_read needs to be update to support mode bit. Other info(dummy cycle) is already part of current SFDP implementation. 
> 
> -pk
>

>From spi-nor.c: spi_nor_select_read():
	/*
	 * In the spi-nor framework, we don't need to make the difference
	 * between mode clock cycles and wait state clock cycles.
	 * Indeed, the value of the mode clock cycles is used by a QSPI
	 * flash memory to know whether it should enter or leave its 0-4-4
	 * (Continuous Read / XIP) mode.
	 * eXecution In Place is out of the scope of the mtd sub-system.
	 * Hence we choose to merge both mode and wait state clock cycles
	 * into the so called dummy clock cycles.
	 */
	nor->read_dummy = read->num_mode_clocks + read->num_wait_states;

So we don't care about the mode bit: just write the 0xFF value during the
2 mode cycles(or 0xF if only one mode cycle) and the spi-nor memory won't
enter its Continuous Read / XIP mode. This value works for all
manufacturers (based on the JESD216 rev B specification describing the 16
DWORDs of the BFPT table).

>From m25p80.c: m25p80_read():

	/*
	 * Set all dummy/mode cycle bits to avoid sending some manufacturer
	 * specific pattern, which might make the memory enter its Continuous
	 * Read mode by mistake.
	 * Based on the different mode cycle bit patterns listed and described
	 * in the JESD216B specification, the 0xff value works for all memories
	 * and all manufacturers.
	 */
	cmd_sz = t[0].len;
	memset(flash->command + cmd_sz - dummy, 0xff, dummy);

 
> 
> 
> 

RE: [RFC 09/10] mtd: spi-nor: Disable hybrid mode for SPANSION S25FS-S family

[Prabhakar Kushwaha]

Hi Cyrille,

> -----Original Message-----
> From: Cyrille Pitchen [mailto:cyrille.pitchen@wedev4u.fr]
> Sent: Wednesday, December 06, 2017 4:28 PM
> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; linux-
> mtd@lists.infradead.org
> Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com; Rajat
> Srivastava <rajat.srivastava@nxp.com>; dedekind1@gmail.com
> Subject: Re: [RFC 09/10] mtd: spi-nor: Disable hybrid mode for SPANSION S25FS -
> S family
> 
> Hi Prabhakar,
> 
> Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
> > 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 default status of the flash is the hybrid architecture.
> > The parameter sectors and the uniform sectors have different erase
> > commands.
> >
> > This patch disables the hybrid sector architecture which makes the
> > flash have uniform sector size and uniform erase command.
> >
> 
> This issue should be addressed in a generic way, not Spansion specific,
> by adding support of the optional Sector Map Parameter Table (SFDP).
> In case of non uniform memories, like this Spansion S25FS-S family or
> SST26, the Sector Map Parameter Table becomes mandatory as a SFDP table.
> 
> From the JESD216B specification, about the JEDEC Sector Map Parameter Table:
> """
> This table is required when a memory device:
> * Has sectors of more than one size
> * Or, does not allow all Erase Type commands to be applied to all sectors.
> """
> 

As I understand from Sector Map Parameter Table, It tell about the flash layout and which region support which erase type.

Flash layout can be configured using volatile or non-volatile registers. For e.g. Spansion S25FS has following combination
Device    CR3NV[3]    CR1NV[2]    CR3NV[1]     Index Value      Description
FS512S       0                  0                    1                01h                      4-kB sectors at bottom with remainder 256-kB sectors
                    0                  1                    1                03h                      4-kB sectors at top with remainder 256-kB sectors
                    1                  0                    1                05h                      Uniform 256-kB sectors
Once one of the combination programmed.  Sector Map Parameter Table help with type of erase supported. 

My requirement is for setting flash in particular configuration i.e. "05" --> Uniform 256-kb. This requirement is getting met with existing patch. 
I don’t want "uniform 256-kb" as default  in spi-nor.c as different user may have different requirements. 
How to achieve this? Device tree ??

--pk

RE: [RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ protocols

[Prabhakar Kushwaha]

Hi Cyrille


> -----Original Message-----
> From: Cyrille Pitchen [mailto:cyrille.pitchen@wedev4u.fr]
> Sent: Thursday, December 07, 2017 11:27 PM
> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; linux-
> mtd@lists.infradead.org
> Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com;
> dedekind1@gmail.com; Suresh Gupta <suresh.gupta@nxp.com>; Poonam
> Aggrwal <poonam.aggrwal@nxp.com>
> Subject: Re: [RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ
> protocols
> 
> Le 07/12/2017 à 12:04, Prabhakar Kushwaha a écrit :
> > Thanks Cyrille for the pointers,
> >
> > I have few queries on SFDP.
> >
> >> -----Original Message-----
> >> From: linux-mtd [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of
> >> Prabhakar Kushwaha
> >> Sent: Thursday, December 07, 2017 2:15 PM
> >> To: Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>; linux-
> mtd@lists.infradead.org
> >> Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com;
> >> dedekind1@gmail.com
> >> Subject: RE: [RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ
> >> protocols
> >>
> >> Hi Cyrille,
> >>
> >>
> >>> -----Original Message-----
> >>> From: Cyrille Pitchen [mailto:cyrille.pitchen@wedev4u.fr]
> >>> Sent: Wednesday, December 06, 2017 4:04 PM
> >>> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; linux-
> >>> mtd@lists.infradead.org
> >>> Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com;
> >>> dedekind1@gmail.com
> >>> Subject: Re: [RFC 01/10] mtd: spi-nor: Add support of 1-2-2, 1-4-4 IO READ
> >>> protocols
> >>>
> >>> Hi Prabhakar,
> >>>
> >>> Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
> >>>> IO READ protocols transfers both address and data on multiple
> >>>> data bits. 1-2-2(DUAL IO), 1-4-4(QUAD IO) transfer address on 2
> >>>> data bits or 4 bits per rising edge of SCK respectively.
> >>>>
> >>>> This patch update spi_nor_flash_parameter->spi_nor_read_command
> >>>> array based on DUAL or QUAD IO flag enabled in flash_info for a flash.
> >>>>
> >>>> Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> >>>> ---
> >>>>  drivers/mtd/spi-nor/spi-nor.c | 22 ++++++++++++++++++++--
> >>>>  1 file changed, 20 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> >>>> index 19c00072..7d3b52f 100644
> >>>> --- a/drivers/mtd/spi-nor/spi-nor.c
> >>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
> >>>> @@ -61,7 +61,7 @@ struct flash_info {
> >>>>  	u16		page_size;
> >>>>  	u16		addr_width;
> >>>>
> >>>> -	u16		flags;
> >>>> +	u32		flags;
> >>>>  #define SECT_4K			BIT(0)	/* SPINOR_OP_BE_4K works
> >>> uniformly */
> >>>>  #define SPI_NOR_NO_ERASE	BIT(1)	/* No erase command needed
> */
> >>>>  #define SST_WRITE		BIT(2)	/* use SST byte programming
> */
> >>>> @@ -89,6 +89,8 @@ struct flash_info {
> >>>>  #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip
> >>> erase */
> >>>>  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables
> */
> >>>>  #define USE_CLSR		BIT(14)	/* use CLSR command */
> >>>> +#define SPI_NOR_DUAL_IO_READ	BIT(15)	/* Flash supports Dual IO Read
> >>> */
> >>>> +#define SPI_NOR_QUAD_IO_READ	BIT(16)	/* Flash supports Quad IO
> >>> Read */
> >>>>  };
> >>>>
> >>>>  #define JEDEC_MFR(info)	((info)->id[0])
> >>>> @@ -2399,6 +2401,13 @@ static int spi_nor_init_params(struct spi_nor
> >> *nor,
> >>>>  					  SNOR_PROTO_1_1_2);
> >>>>  	}
> >>>>
> >>>> +	if (info->flags & SPI_NOR_DUAL_IO_READ) {
> >>>> +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
> >>>> +		spi_nor_set_read_settings(&params-
> >>>> reads[SNOR_CMD_READ_1_2_2],
> >>>> +					  0, 8, SPINOR_OP_READ_1_2_2,
> >>>> +					  SNOR_PROTO_1_2_2);
> >>>> +	}
> >>>> +
> >>>>  	if (info->flags & SPI_NOR_QUAD_READ) {
> >>>>  		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
> >>>>  		spi_nor_set_read_settings(&params-
> >>>> reads[SNOR_CMD_READ_1_1_4],
> >>>> @@ -2406,6 +2415,14 @@ static int spi_nor_init_params(struct spi_nor
> >> *nor,
> >>>>  					  SNOR_PROTO_1_1_4);
> >>>>  	}
> >>>>
> >>>> +	if (info->flags & SPI_NOR_QUAD_IO_READ) {
> >>>> +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_4_4;
> >>>> +		spi_nor_set_read_settings(&params-
> >>>> reads[SNOR_CMD_READ_1_4_4],
> >>>> +					  0, 10, SPINOR_OP_READ_1_4_4,
> >>>
> >>> The actual number of mode and wait state clock cycles depend on the
> >>> manufacturer and the memory part number.
> >>>
> >>> Here are few examples of factory settings for Fast Read 1-4-4
> >>> (mode/wait states):
> >>>
> >>> Micron: 1/9
> >>> Macronix: 2/4
> >>> Spansion: 2/8
> >>>
> >>> AFAIK, 0/10 is not used by any manufacturer and doesn't work with
> Macronix.
> >>>
> >>> For Fast Read 1-1-1, 1-1-2 and 1-1-4, all memories I know use 0/8. I guess
> >>> those numbers were chosen for 1-1-2 and 1-1-4 too to be backward
> >> compatible
> >>> with older memories supporting only Fast Read 1-1-1, which always uses no
> >>> node cycle and 8 wait-states.
> >>>
> >>> However with the introduction of Fast Read 1-2-2 and 1-4-4, things became
> >>> messy and every manufacturer used its own settings. That's why we didn't
> >>> introduce SPI_NOR_DUAL_IO_READ of SPI_NOR_QUAD_IO_READ flags:
> >> because
> >>> we
> >>> can associate them settings that would apply to all manufacturers.
> >>>
> >>> We can still use Fast Read 1-2-2 and 1-4-4 when supported by both the
> >>> memory and the SPI controller by parsing the memory SFDP tables and filling
> >>> the 'struct spi_nor_flash_parameter' accordingly.
> >>>
> >>> We (MTD maintainers) don't want to add more hard-coded data in the
> >>> 'struct flash_info' and in the spi_nor_ids[] array when those data can
> >>> actually be retrieved from the SFDP tables.
> >>>
> >>> Latest SPI NOR memories almost all support SFDP tables (JEDEC JESD216B).
> >>> I had a quick overview of you series and it seems that you have been
> >>> testing with both Micron and Spansion memory parts: both should support
> >>> SFDP.
> >>>
> >>> So sorry but I will reject this patch: as I said unlike Fast Read 1-1-z
> >>> protocols which always use 0 mode cycle and 8 wait states, there are no
> >>> settings for Fast Read 1-4-4 or 1-2-2 that could be valid for all
> >>> memory manufacturers.
> >>>
> >>
> >> I understand your point. Let me explore SFDP.
> >>
> >
> > I tried to understand the SFDP.
> >
> > Looks like struct sfdp_bfpt_read needs to be update to support mode bit. Other
> info(dummy cycle) is already part of current SFDP implementation.
> >
> > -pk
> >
> 
> From spi-nor.c: spi_nor_select_read():
> 	/*
> 	 * In the spi-nor framework, we don't need to make the difference
> 	 * between mode clock cycles and wait state clock cycles.
> 	 * Indeed, the value of the mode clock cycles is used by a QSPI
> 	 * flash memory to know whether it should enter or leave its 0-4-4
> 	 * (Continuous Read / XIP) mode.
> 	 * eXecution In Place is out of the scope of the mtd sub-system.
> 	 * Hence we choose to merge both mode and wait state clock cycles
> 	 * into the so called dummy clock cycles.
> 	 */
> 	nor->read_dummy = read->num_mode_clocks + read-
> >num_wait_states;
> 
> So we don't care about the mode bit: just write the 0xFF value during the
> 2 mode cycles(or 0xF if only one mode cycle) and the spi-nor memory won't
> enter its Continuous Read / XIP mode. This value works for all
> manufacturers (based on the JESD216 rev B specification describing the 16
> DWORDs of the BFPT table).
> 
> From m25p80.c: m25p80_read():
> 
> 	/*
> 	 * Set all dummy/mode cycle bits to avoid sending some manufacturer
> 	 * specific pattern, which might make the memory enter its Continuous
> 	 * Read mode by mistake.
> 	 * Based on the different mode cycle bit patterns listed and described
> 	 * in the JESD216B specification, the 0xff value works for all memories
> 	 * and all manufacturers.
> 	 */
> 	cmd_sz = t[0].len;
> 	memset(flash->command + cmd_sz - dummy, 0xff, dummy);
> 

As current framework deals with number of clock cycles for both mode and dummy. 
It is responsibility of low level controller driver to either use clock or convert into bytes as per requirement.
Is my understanding correct?

--pk 

RE: [RFC 09/10] mtd: spi-nor: Disable hybrid mode for SPANSION S25FS-S family

[Prabhakar Kushwaha]

Hi Cyrille,


> -----Original Message-----
> From: linux-mtd [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of
> Prabhakar Kushwaha
> Sent: Saturday, December 09, 2017 10:46 PM
> To: Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>; linux-mtd@lists.infradead.org
> Cc: Poonam Aggrwal <poonam.aggrwal@nxp.com>; boris.brezillon@free-
> electrons.com; dedekind1@gmail.com; Rajat Srivastava
> <rajat.srivastava@nxp.com>; Suresh Gupta <suresh.gupta@nxp.com>;
> computersforpeace@gmail.com
> Subject: RE: [RFC 09/10] mtd: spi-nor: Disable hybrid mode for SPANSION S25FS-
> S family
> 
> Hi Cyrille,
> 
> > -----Original Message-----
> > From: Cyrille Pitchen [mailto:cyrille.pitchen@wedev4u.fr]
> > Sent: Wednesday, December 06, 2017 4:28 PM
> > To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; linux-
> > mtd@lists.infradead.org
> > Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com;
> Rajat
> > Srivastava <rajat.srivastava@nxp.com>; dedekind1@gmail.com
> > Subject: Re: [RFC 09/10] mtd: spi-nor: Disable hybrid mode for SPANSION S25FS
> -
> > S family
> >
> > Hi Prabhakar,
> >
> > Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
> > > 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 default status of the flash is the hybrid architecture.
> > > The parameter sectors and the uniform sectors have different erase
> > > commands.
> > >
> > > This patch disables the hybrid sector architecture which makes the
> > > flash have uniform sector size and uniform erase command.
> > >
> >
> > This issue should be addressed in a generic way, not Spansion specific,
> > by adding support of the optional Sector Map Parameter Table (SFDP).
> > In case of non uniform memories, like this Spansion S25FS-S family or
> > SST26, the Sector Map Parameter Table becomes mandatory as a SFDP table.
> >
> > From the JESD216B specification, about the JEDEC Sector Map Parameter
> Table:
> > """
> > This table is required when a memory device:
> > * Has sectors of more than one size
> > * Or, does not allow all Erase Type commands to be applied to all sectors.
> > """
> >
> 
> As I understand from Sector Map Parameter Table, It tell about the flash layout
> and which region support which erase type.
> 
> Flash layout can be configured using volatile or non-volatile registers. For e.g.
> Spansion S25FS has following combination
> Device    CR3NV[3]    CR1NV[2]    CR3NV[1]     Index Value      Description
> FS512S       0                  0                    1                01h                      4-kB sectors at
> bottom with remainder 256-kB sectors
>                     0                  1                    1                03h                      4-kB sectors at top
> with remainder 256-kB sectors
>                     1                  0                    1                05h                      Uniform 256-kB
> sectors
> Once one of the combination programmed.  Sector Map Parameter Table help
> with type of erase supported.
> 
> My requirement is for setting flash in particular configuration i.e. "05" -->
> Uniform 256-kb. This requirement is getting met with existing patch.
> I don’t want "uniform 256-kb" as default  in spi-nor.c as different user may have
> different requirements.
> How to achieve this? Device tree ??
> 

Flashes has different layout based on configuration. For e.g. Spansion S25FS supports 3 layout. 
Same has been explained in previous mail.

I have requirement of selecting a particular layout of flash. I don’t want to fix/hardcode flash layout in spi-nor.c. 
This means layout info should come from defconfig or device tree. 

Do we have any such device tree binding available for flash.

--pk

Re: [RFC 09/10] mtd: spi-nor: Disable hybrid mode for SPANSION S25FS-S family

[Cyrille Pitchen]

Hi Prabhakar,

Le 21/12/2017 à 13:41, Prabhakar Kushwaha a écrit :
> Hi Cyrille,
> 
> 
>> -----Original Message-----
>> From: linux-mtd [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of
>> Prabhakar Kushwaha
>> Sent: Saturday, December 09, 2017 10:46 PM
>> To: Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>; linux-mtd@lists.infradead.org
>> Cc: Poonam Aggrwal <poonam.aggrwal@nxp.com>; boris.brezillon@free-
>> electrons.com; dedekind1@gmail.com; Rajat Srivastava
>> <rajat.srivastava@nxp.com>; Suresh Gupta <suresh.gupta@nxp.com>;
>> computersforpeace@gmail.com
>> Subject: RE: [RFC 09/10] mtd: spi-nor: Disable hybrid mode for SPANSION S25FS-
>> S family
>>
>> Hi Cyrille,
>>
>>> -----Original Message-----
>>> From: Cyrille Pitchen [mailto:cyrille.pitchen@wedev4u.fr]
>>> Sent: Wednesday, December 06, 2017 4:28 PM
>>> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; linux-
>>> mtd@lists.infradead.org
>>> Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com;
>> Rajat
>>> Srivastava <rajat.srivastava@nxp.com>; dedekind1@gmail.com
>>> Subject: Re: [RFC 09/10] mtd: spi-nor: Disable hybrid mode for SPANSION S25FS
>> -
>>> S family
>>>
>>> Hi Prabhakar,
>>>
>>> Le 06/12/2017 à 09:15, Prabhakar Kushwaha a écrit :
>>>> 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 default status of the flash is the hybrid architecture.
>>>> The parameter sectors and the uniform sectors have different erase
>>>> commands.
>>>>
>>>> This patch disables the hybrid sector architecture which makes the
>>>> flash have uniform sector size and uniform erase command.
>>>>
>>>
>>> This issue should be addressed in a generic way, not Spansion specific,
>>> by adding support of the optional Sector Map Parameter Table (SFDP).
>>> In case of non uniform memories, like this Spansion S25FS-S family or
>>> SST26, the Sector Map Parameter Table becomes mandatory as a SFDP table.
>>>
>>> From the JESD216B specification, about the JEDEC Sector Map Parameter
>> Table:
>>> """
>>> This table is required when a memory device:
>>> * Has sectors of more than one size
>>> * Or, does not allow all Erase Type commands to be applied to all sectors.
>>> """
>>>
>>
>> As I understand from Sector Map Parameter Table, It tell about the flash layout
>> and which region support which erase type.
>>
>> Flash layout can be configured using volatile or non-volatile registers. For e.g.
>> Spansion S25FS has following combination
>> Device    CR3NV[3]    CR1NV[2]    CR3NV[1]     Index Value      Description
>> FS512S       0                  0                    1                01h                      4-kB sectors at
>> bottom with remainder 256-kB sectors
>>                     0                  1                    1                03h                      4-kB sectors at top
>> with remainder 256-kB sectors
>>                     1                  0                    1                05h                      Uniform 256-kB
>> sectors
>> Once one of the combination programmed.  Sector Map Parameter Table help
>> with type of erase supported.
>>
>> My requirement is for setting flash in particular configuration i.e. "05" -->
>> Uniform 256-kb. This requirement is getting met with existing patch.
>> I don’t want "uniform 256-kb" as default  in spi-nor.c as different user may have
>> different requirements.
>> How to achieve this? Device tree ??
>>
> 
> Flashes has different layout based on configuration. For e.g. Spansion S25FS supports 3 layout. 
> Same has been explained in previous mail.
> 
> I have requirement of selecting a particular layout of flash. I don’t want to fix/hardcode flash layout in spi-nor.c. 
> This means layout info should come from defconfig or device tree. 
> 
> Do we have any such device tree binding available for flash.
> 

AFAIK, there is currently no existing DT property. So we may introduce a
generic one like "sector-map-index":

sector-map-index = <5>;

However, now I'm reading again the JESD216B specification about the sector maps.
I'm looking at the "configuration detection command descriptor (command)".

"""
The Sector Map table is built from a sequence of descriptors. There are two
types of descriptors: 
• A configuration detection command descriptor (command)
• A configuration sector map descriptor (map)

The command descriptors are optional. If there is a single configuration
then no command descriptors are needed. If there are more than two
configurations, more than one command descriptor is needed. If command
descriptors are provided, they always precede map descriptors in the
table.

Each configuration detection command is described by two Dwords. These
Dwords provide: 
• A bit indicating that the descriptor is a configuration detection command,
• a bit indicating whether this descriptor is the last command descriptor,
• the instruction code for the command,
• the number of address bytes for the command,
• the number of read latency cycles between the last address byte and the read data byte,
• a mask to select the bit of interest in the returned data byte,
• and the address value for the command.

It is assumed that each command is reading a configuration register with a
single byte of return data and that this type of command does not provide
mode bits. Each command selects a single bit from the byte of returned
data. There will be a separate command descriptor and command sent for each
bit of configuration selecting information that is needed to select the
current Sector Map Configuration that is in use.
"""

So it looks like only instruction op codes are provided for reading the
selected sector map configuration, but not for updating it.
If there is no standard to describe a generic way to select the wanted
sector map configuration, then I'm no so eager to introduce support of all
manufacturer's custom procedures with all their quirks.

Besides, I guess there is no point updating the selection of the sector map
configuration at each boot or SPI NOR memory initialization. I think this
selection should be done once for all, likely when programming the memory
for the first time in the factory, and/or could be done by some custom boot
loader.

I'd rather focus on implementing standards like JEDEC/JESD216B than adding
support to all memory manufacturer quirks when we can avoid them.

The reason? I used to do this in the past, implementing manufacturer
specific procedures, for instance to modify the number of dummy clock
cycles used by Fast Read commands. Then time showed that it was a mistake
because manufacturers don't seem to be consistent and don't hesitate to
change their procedures breaking the compatibility with their previous
memory parts. So it was a real pain to add support to new memory parts,
especially because the work I did was in some read-only boot-loader (ROM
Code), hence that can be updated without producing new chips.

My point is that manufacturer specific pieces of source code *in a generic
framework like spi-nor* are not so reliable whereas sticking to standards
as mush as possible has showed up to be a better choice over the time.
I agree this is a little bit a personal opinion but based on practical
experience.

Of course it doesn't mean that manufacturer specific pieces of source code
are forbidden, of course not! It means that this is not the recommended way
hence should be avoided. Also you will have to justify your position if you
think this should be done in Linux anyway :)

So I'm still open to the discussion then don't hesitate to defend your
position.

For now, adding support to the non-uniform erase sector map based on the
dedicated SFDP table sounds like a good idea to *read* the current
configuration but *updating* the selection from Linux might not be worth.

Best regards,

Cyrille


> --pk
> 
> 
>