All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] mtd: spi-nor: parse SFDP tables as defined by JESD216B
@ 2016-06-20 16:49 Cyrille Pitchen
  2016-06-20 16:50 ` [PATCH 1/9] mtd: spi-nor: improve macronix_quad_enable() Cyrille Pitchen
                   ` (8 more replies)
  0 siblings, 9 replies; 26+ messages in thread
From: Cyrille Pitchen @ 2016-06-20 16:49 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

Hi all,

This series of patches adds support SPI x-y-z protocols other than
SPI 1-1-1, 1-1-2 and 1-1-4. When available, the Serial Flash Discoverable
Parameter (SFDP) tables are parsed to dynamically configure the SPI
protocols, op codes, number of dummy cycles or erase block size used
during Fast Read, Page Program and Sector Erase operation.
Otherwise, when SFDP tables are not available, the legacy settings are
used: we only use SPI 1-1-1, 1-1-2 or 1-1-4 protocols.

Also the 3rd parameter of spi_nor_scan() is changed so the caller can
provide the spi-nor framework with a more accurate list of SPI protocols
supported by the SPI controller. Using both this list and the SFDP
settings, the spi-nor framework can now select the best match for SPI
protocols supported by both the memory and the controller.


This series of patches is based onto next-20160620.
It was tested on a sama5d2 xplained board + Macronix mx25l25673g.

Best regards,

Cyrille

Cyrille Pitchen (9):
  mtd: spi-nor: improve macronix_quad_enable()
  mtd: spi-nor: add an alternative method to support memory >16MiB
  Documentation: atmel-quadspi: add binding file for Atmel QSPI driver
  mtd: atmel-quadspi: add driver for Atmel QSPI controller
  mtd: spi-nor: add support of SPI protocols like SPI 1-2-2 and SPI
    1-4-4
  mtd: spi-nor: remove unused set_quad_mode() function
  mtd: m25p80: add support of dual and quad spi protocols to all
    commands
  mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables
  mtd: spi-nor: parse SFDP 4-byte Address Instruction Table

 .../devicetree/bindings/mtd/atmel-quadspi.txt      |  32 +
 drivers/mtd/devices/m25p80.c                       | 204 ++++-
 drivers/mtd/devices/serial_flash_cmds.h            |   7 -
 drivers/mtd/devices/st_spi_fsm.c                   |  28 +-
 drivers/mtd/spi-nor/Kconfig                        |   9 +
 drivers/mtd/spi-nor/Makefile                       |   1 +
 drivers/mtd/spi-nor/atmel-quadspi.c                | 770 +++++++++++++++++
 drivers/mtd/spi-nor/fsl-quadspi.c                  |   8 +-
 drivers/mtd/spi-nor/mtk-quadspi.c                  |  16 +-
 drivers/mtd/spi-nor/nxp-spifi.c                    |  21 +-
 drivers/mtd/spi-nor/spi-nor.c                      | 962 +++++++++++++++++++--
 include/linux/mtd/spi-nor.h                        | 159 +++-
 12 files changed, 2021 insertions(+), 196 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/atmel-quadspi.txt
 create mode 100644 drivers/mtd/spi-nor/atmel-quadspi.c

-- 
1.8.2.2

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/9] mtd: spi-nor: improve macronix_quad_enable()
  2016-06-20 16:49 [PATCH 0/9] mtd: spi-nor: parse SFDP tables as defined by JESD216B Cyrille Pitchen
@ 2016-06-20 16:50 ` Cyrille Pitchen
  2016-06-20 16:50 ` [PATCH 2/9] mtd: spi-nor: add an alternative method to support memory >16MiB Cyrille Pitchen
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 26+ messages in thread
From: Cyrille Pitchen @ 2016-06-20 16:50 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

The patch checks whether the Quad Enable bit is already set in the Status
Register. If so, the function exits immediately with a successful return
code. Otherwise, a message is now printed telling we're setting the
non-volatile bit.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index a63922ed6385..86a555e43eb1 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1194,6 +1194,11 @@ static int macronix_quad_enable(struct spi_nor *nor)
 	val = read_sr(nor);
 	if (val < 0)
 		return val;
+	if (val & SR_QUAD_EN_MX)
+		return 0;
+
+	/* Update the Quad Enable bit. */
+	dev_info(nor->dev, "setting Macronix Quad Enable (non-volatile) bit\n");
 	write_enable(nor);
 
 	write_sr(nor, val | SR_QUAD_EN_MX);
-- 
1.8.2.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/9] mtd: spi-nor: add an alternative method to support memory >16MiB
  2016-06-20 16:49 [PATCH 0/9] mtd: spi-nor: parse SFDP tables as defined by JESD216B Cyrille Pitchen
  2016-06-20 16:50 ` [PATCH 1/9] mtd: spi-nor: improve macronix_quad_enable() Cyrille Pitchen
@ 2016-06-20 16:50 ` Cyrille Pitchen
  2016-06-20 16:50 ` [PATCH 3/9] Documentation: atmel-quadspi: add binding file for Atmel QSPI driver Cyrille Pitchen
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 26+ messages in thread
From: Cyrille Pitchen @ 2016-06-20 16:50 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

This patch provides an alternative mean to support memory above 16MiB
(128Mib) by replacing 3byte address op codes by their associated 4byte
address versions.

Using the dedicated 4byte address op codes doesn't change the internal
state of the SPI NOR memory as opposed to using other means such as
updating a Base Address Register (BAR) and sending command to enter/leave
the 4byte mode.

Hence when a CPU reset occurs, early bootloaders don't need to be aware
of BAR value or 4byte mode being enabled: they can still access the first
16MiB of the SPI NOR memory using the regular 3byte address op codes.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/mtd/devices/serial_flash_cmds.h |   7 ---
 drivers/mtd/devices/st_spi_fsm.c        |  28 ++++-----
 drivers/mtd/spi-nor/spi-nor.c           | 104 +++++++++++++++++++++++++-------
 include/linux/mtd/spi-nor.h             |  22 +++++--
 4 files changed, 113 insertions(+), 48 deletions(-)

diff --git a/drivers/mtd/devices/serial_flash_cmds.h b/drivers/mtd/devices/serial_flash_cmds.h
index f59a125295d0..8b81e15105dd 100644
--- a/drivers/mtd/devices/serial_flash_cmds.h
+++ b/drivers/mtd/devices/serial_flash_cmds.h
@@ -18,19 +18,12 @@
 #define SPINOR_OP_RDVCR		0x85
 
 /* JEDEC Standard - Serial Flash Discoverable Parmeters (SFDP) Commands */
-#define SPINOR_OP_READ_1_2_2	0xbb	/* DUAL I/O READ */
-#define SPINOR_OP_READ_1_4_4	0xeb	/* QUAD I/O READ */
-
 #define SPINOR_OP_WRITE		0x02	/* PAGE PROGRAM */
 #define SPINOR_OP_WRITE_1_1_2	0xa2	/* DUAL INPUT PROGRAM */
 #define SPINOR_OP_WRITE_1_2_2	0xd2	/* DUAL INPUT EXT PROGRAM */
 #define SPINOR_OP_WRITE_1_1_4	0x32	/* QUAD INPUT PROGRAM */
 #define SPINOR_OP_WRITE_1_4_4	0x12	/* QUAD INPUT EXT PROGRAM */
 
-/* READ commands with 32-bit addressing */
-#define SPINOR_OP_READ4_1_2_2	0xbc
-#define SPINOR_OP_READ4_1_4_4	0xec
-
 /* Configuration flags */
 #define FLASH_FLAG_SINGLE	0x000000ff
 #define FLASH_FLAG_READ_WRITE	0x00000001
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 5454b4113589..804313a33f2b 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -507,13 +507,13 @@ static struct seq_rw_config n25q_read3_configs[] = {
  *	- 'FAST' variants configured for 8 dummy cycles (see note above.)
  */
 static struct seq_rw_config n25q_read4_configs[] = {
-	{FLASH_FLAG_READ_1_4_4, SPINOR_OP_READ4_1_4_4,	0, 4, 4, 0x00, 0, 8},
-	{FLASH_FLAG_READ_1_1_4, SPINOR_OP_READ4_1_1_4,	0, 1, 4, 0x00, 0, 8},
-	{FLASH_FLAG_READ_1_2_2, SPINOR_OP_READ4_1_2_2,	0, 2, 2, 0x00, 0, 8},
-	{FLASH_FLAG_READ_1_1_2, SPINOR_OP_READ4_1_1_2,	0, 1, 2, 0x00, 0, 8},
-	{FLASH_FLAG_READ_FAST,	SPINOR_OP_READ4_FAST,	0, 1, 1, 0x00, 0, 8},
-	{FLASH_FLAG_READ_WRITE, SPINOR_OP_READ4,	0, 1, 1, 0x00, 0, 0},
-	{0x00,			0,			0, 0, 0, 0x00, 0, 0},
+	{FLASH_FLAG_READ_1_4_4, SPINOR_OP_READ_1_4_4_4B, 0, 4, 4, 0x00, 0, 8},
+	{FLASH_FLAG_READ_1_1_4, SPINOR_OP_READ_1_1_4_4B, 0, 1, 4, 0x00, 0, 8},
+	{FLASH_FLAG_READ_1_2_2, SPINOR_OP_READ_1_2_2_4B, 0, 2, 2, 0x00, 0, 8},
+	{FLASH_FLAG_READ_1_1_2, SPINOR_OP_READ_1_1_2_4B, 0, 1, 2, 0x00, 0, 8},
+	{FLASH_FLAG_READ_FAST,	SPINOR_OP_READ_FAST_4B,  0, 1, 1, 0x00, 0, 8},
+	{FLASH_FLAG_READ_WRITE, SPINOR_OP_READ_4B,       0, 1, 1, 0x00, 0, 0},
+	{0x00,			0,                       0, 0, 0, 0x00, 0, 0},
 };
 
 /*
@@ -553,13 +553,13 @@ static int stfsm_mx25_en_32bit_addr_seq(struct stfsm_seq *seq)
  * entering a state that is incompatible with the SPIBoot Controller.
  */
 static struct seq_rw_config stfsm_s25fl_read4_configs[] = {
-	{FLASH_FLAG_READ_1_4_4,  SPINOR_OP_READ4_1_4_4,  0, 4, 4, 0x00, 2, 4},
-	{FLASH_FLAG_READ_1_1_4,  SPINOR_OP_READ4_1_1_4,  0, 1, 4, 0x00, 0, 8},
-	{FLASH_FLAG_READ_1_2_2,  SPINOR_OP_READ4_1_2_2,  0, 2, 2, 0x00, 4, 0},
-	{FLASH_FLAG_READ_1_1_2,  SPINOR_OP_READ4_1_1_2,  0, 1, 2, 0x00, 0, 8},
-	{FLASH_FLAG_READ_FAST,   SPINOR_OP_READ4_FAST,   0, 1, 1, 0x00, 0, 8},
-	{FLASH_FLAG_READ_WRITE,  SPINOR_OP_READ4,        0, 1, 1, 0x00, 0, 0},
-	{0x00,                   0,                      0, 0, 0, 0x00, 0, 0},
+	{FLASH_FLAG_READ_1_4_4,  SPINOR_OP_READ_1_4_4_4B,  0, 4, 4, 0x00, 2, 4},
+	{FLASH_FLAG_READ_1_1_4,  SPINOR_OP_READ_1_1_4_4B,  0, 1, 4, 0x00, 0, 8},
+	{FLASH_FLAG_READ_1_2_2,  SPINOR_OP_READ_1_2_2_4B,  0, 2, 2, 0x00, 4, 0},
+	{FLASH_FLAG_READ_1_1_2,  SPINOR_OP_READ_1_1_2_4B,  0, 1, 2, 0x00, 0, 8},
+	{FLASH_FLAG_READ_FAST,   SPINOR_OP_READ_FAST_4B,   0, 1, 1, 0x00, 0, 8},
+	{FLASH_FLAG_READ_WRITE,  SPINOR_OP_READ_4B,        0, 1, 1, 0x00, 0, 0},
+	{0x00,                   0,                        0, 0, 0, 0x00, 0, 0},
 };
 
 static struct seq_rw_config stfsm_s25fl_write4_configs[] = {
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 86a555e43eb1..e484bfe54c36 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -75,6 +75,10 @@ struct flash_info {
 					 * bit. Must be used with
 					 * SPI_NOR_HAS_LOCK.
 					 */
+#define SPI_NOR_4B_OPCODES	BIT(10)	/*
+					 * Use dedicated 4byte address op codes
+					 * to support memory size above 128Mib.
+					 */
 };
 
 #define JEDEC_MFR(info)	((info)->id[0])
@@ -188,6 +192,81 @@ static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
 	return mtd->priv;
 }
 
+
+struct spi_nor_address_entry {
+	u8	src_opcode;
+	u8	dst_opcode;
+};
+
+static u8 spi_nor_convert_opcode(u8 opcode,
+				 const struct spi_nor_address_entry *entries,
+				 size_t num_entries)
+{
+	int min, max;
+
+	min = 0;
+	max = num_entries - 1;
+	while (min <= max) {
+		int mid = (min + max) >> 1;
+		const struct spi_nor_address_entry *entry = &entries[mid];
+
+		if (opcode == entry->src_opcode)
+			return entry->dst_opcode;
+
+		if (opcode < entry->src_opcode)
+			max = mid - 1;
+		else
+			min = mid + 1;
+	}
+
+	/* No conversion found */
+	return opcode;
+}
+
+static u8 spi_nor_3to4_opcode(u8 opcode)
+{
+	/* MUST be sorted by 3byte opcode */
+#define ENTRY_3TO4(_opcode)	{ _opcode, _opcode##_4B }
+	static const struct spi_nor_address_entry spi_nor_3to4_table[] = {
+		ENTRY_3TO4(SPINOR_OP_PP),		/* 0x02 */
+		ENTRY_3TO4(SPINOR_OP_READ),		/* 0x03 */
+		ENTRY_3TO4(SPINOR_OP_READ_FAST),	/* 0x0b */
+		ENTRY_3TO4(SPINOR_OP_BE_4K),		/* 0x20 */
+		ENTRY_3TO4(SPINOR_OP_PP_1_1_4),		/* 0x32 */
+		ENTRY_3TO4(SPINOR_OP_PP_1_4_4),		/* 0x38 */
+		ENTRY_3TO4(SPINOR_OP_READ_1_1_2),	/* 0x3b */
+		ENTRY_3TO4(SPINOR_OP_BE_32K),		/* 0x52 */
+		ENTRY_3TO4(SPINOR_OP_READ_1_1_4),	/* 0x6b */
+		ENTRY_3TO4(SPINOR_OP_READ_1_2_2),	/* 0xbb */
+		ENTRY_3TO4(SPINOR_OP_SE),		/* 0xd8 */
+		ENTRY_3TO4(SPINOR_OP_READ_1_4_4),	/* 0xeb */
+	};
+#undef ENTRY_3TO4
+
+	return spi_nor_convert_opcode(opcode, spi_nor_3to4_table,
+				      ARRAY_SIZE(spi_nor_3to4_table));
+}
+
+static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
+				      const struct flash_info *info)
+{
+	/* Do some manufacturer fixups first */
+	switch (JEDEC_MFR(info)) {
+	case SNOR_MFR_SPANSION:
+		/* No small sector erase for 4-byte command set */
+		nor->erase_opcode = SPINOR_OP_SE;
+		nor->mtd.erasesize = info->sector_size;
+		break;
+
+	default:
+		break;
+	}
+
+	nor->read_opcode	= spi_nor_3to4_opcode(nor->read_opcode);
+	nor->program_opcode	= spi_nor_3to4_opcode(nor->program_opcode);
+	nor->erase_opcode	= spi_nor_3to4_opcode(nor->erase_opcode);
+}
+
 /* Enable/disable 4-byte addressing mode. */
 static inline int set_4byte(struct spi_nor *nor, const struct flash_info *info,
 			    int enable)
@@ -1459,27 +1538,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	else if (mtd->size > 0x1000000) {
 		/* enable 4-byte addressing if the device exceeds 16MiB */
 		nor->addr_width = 4;
-		if (JEDEC_MFR(info) == SNOR_MFR_SPANSION) {
-			/* Dedicated 4-byte command set */
-			switch (nor->flash_read) {
-			case SPI_NOR_QUAD:
-				nor->read_opcode = SPINOR_OP_READ4_1_1_4;
-				break;
-			case SPI_NOR_DUAL:
-				nor->read_opcode = SPINOR_OP_READ4_1_1_2;
-				break;
-			case SPI_NOR_FAST:
-				nor->read_opcode = SPINOR_OP_READ4_FAST;
-				break;
-			case SPI_NOR_NORMAL:
-				nor->read_opcode = SPINOR_OP_READ4;
-				break;
-			}
-			nor->program_opcode = SPINOR_OP_PP_4B;
-			/* No small sector erase for 4-byte command set */
-			nor->erase_opcode = SPINOR_OP_SE_4B;
-			mtd->erasesize = info->sector_size;
-		} else
+		if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
+		    info->flags & SPI_NOR_4B_OPCODES)
+			spi_nor_set_4byte_opcodes(nor, info);
+		else
 			set_4byte(nor, info, 1);
 	} else {
 		nor->addr_width = 3;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index c425c7b4c2a0..8b02fd7864d0 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -43,9 +43,13 @@
 #define SPINOR_OP_WRSR		0x01	/* Write status register 1 byte */
 #define SPINOR_OP_READ		0x03	/* Read data bytes (low frequency) */
 #define SPINOR_OP_READ_FAST	0x0b	/* Read data bytes (high frequency) */
-#define SPINOR_OP_READ_1_1_2	0x3b	/* Read data bytes (Dual SPI) */
-#define SPINOR_OP_READ_1_1_4	0x6b	/* Read data bytes (Quad SPI) */
+#define SPINOR_OP_READ_1_1_2	0x3b	/* Read data bytes (Dual Output SPI) */
+#define SPINOR_OP_READ_1_2_2	0xbb	/* Read data bytes (Dual I/O SPI) */
+#define SPINOR_OP_READ_1_1_4	0x6b	/* Read data bytes (Quad Output SPI) */
+#define SPINOR_OP_READ_1_4_4	0xeb	/* Read data bytes (Quad I/O SPI) */
 #define SPINOR_OP_PP		0x02	/* Page program (up to 256 bytes) */
+#define SPINOR_OP_PP_1_1_4	0x32	/* Quad page program */
+#define SPINOR_OP_PP_1_4_4	0x38	/* Quad page program */
 #define SPINOR_OP_BE_4K		0x20	/* Erase 4KiB block */
 #define SPINOR_OP_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips */
 #define SPINOR_OP_BE_32K	0x52	/* Erase 32KiB block */
@@ -56,11 +60,17 @@
 #define SPINOR_OP_RDFSR		0x70	/* Read flag status register */
 
 /* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
-#define SPINOR_OP_READ4		0x13	/* Read data bytes (low frequency) */
-#define SPINOR_OP_READ4_FAST	0x0c	/* Read data bytes (high frequency) */
-#define SPINOR_OP_READ4_1_1_2	0x3c	/* Read data bytes (Dual SPI) */
-#define SPINOR_OP_READ4_1_1_4	0x6c	/* Read data bytes (Quad SPI) */
+#define SPINOR_OP_READ_4B	0x13	/* Read data bytes (low frequency) */
+#define SPINOR_OP_READ_FAST_4B	0x0c	/* Read data bytes (high frequency) */
+#define SPINOR_OP_READ_1_1_2_4B	0x3c	/* Read data bytes (Dual Output SPI) */
+#define SPINOR_OP_READ_1_2_2_4B	0xbc	/* Read data bytes (Dual I/O SPI) */
+#define SPINOR_OP_READ_1_1_4_4B	0x6c	/* Read data bytes (Quad Output SPI) */
+#define SPINOR_OP_READ_1_4_4_4B	0xec	/* Read data bytes (Quad I/O SPI) */
 #define SPINOR_OP_PP_4B		0x12	/* Page program (up to 256 bytes) */
+#define SPINOR_OP_PP_1_1_4_4B	0x34	/* Quad page program */
+#define SPINOR_OP_PP_1_4_4_4B	0x3e	/* Quad page program */
+#define SPINOR_OP_BE_4K_4B	0x21	/* Erase 4KiB block */
+#define SPINOR_OP_BE_32K_4B	0x5c	/* Erase 32KiB block */
 #define SPINOR_OP_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
 
 /* Used for SST flashes only. */
-- 
1.8.2.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/9] Documentation: atmel-quadspi: add binding file for Atmel QSPI driver
  2016-06-20 16:49 [PATCH 0/9] mtd: spi-nor: parse SFDP tables as defined by JESD216B Cyrille Pitchen
  2016-06-20 16:50 ` [PATCH 1/9] mtd: spi-nor: improve macronix_quad_enable() Cyrille Pitchen
  2016-06-20 16:50 ` [PATCH 2/9] mtd: spi-nor: add an alternative method to support memory >16MiB Cyrille Pitchen
@ 2016-06-20 16:50 ` Cyrille Pitchen
  2016-06-20 16:50 ` [PATCH 4/9] mtd: atmel-quadspi: add driver for Atmel QSPI controller Cyrille Pitchen
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 26+ messages in thread
From: Cyrille Pitchen @ 2016-06-20 16:50 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

This patch documents the DT bindings for the driver of the Atmel QSPI
controller embedded inside sama5d2x SoCs.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 .../devicetree/bindings/mtd/atmel-quadspi.txt      | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/atmel-quadspi.txt

diff --git a/Documentation/devicetree/bindings/mtd/atmel-quadspi.txt b/Documentation/devicetree/bindings/mtd/atmel-quadspi.txt
new file mode 100644
index 000000000000..489807005eda
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/atmel-quadspi.txt
@@ -0,0 +1,32 @@
+* Atmel Quad Serial Peripheral Interface (QSPI)
+
+Required properties:
+- compatible:     Should be "atmel,sama5d2-qspi".
+- reg:            Should contain the locations and lengths of the base registers
+                  and the mapped memory.
+- reg-names:      Should contain the resource reg names:
+                  - qspi_base: configuration register address space
+                  - qspi_mmap: memory mapped address space
+- interrupts:     Should contain the interrupt for the device.
+- clocks:         The phandle of the clock needed by the QSPI controller.
+- #address-cells: Should be <1>.
+- #size-cells:    Should be <0>.
+
+Example:
+
+spi@f0020000 {
+	compatible = "atmel,sama5d2-qspi";
+	reg = <0xf0020000 0x100>, <0xd0000000 0x8000000>;
+	reg-names = "qspi_base", "qspi_mmap";
+	interrupts = <52 IRQ_TYPE_LEVEL_HIGH 7>;
+	clocks = <&spi0_clk>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_spi0_default>;
+	status = "okay";
+
+	m25p80@0 {
+		...
+	};
+};
-- 
1.8.2.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 4/9] mtd: atmel-quadspi: add driver for Atmel QSPI controller
  2016-06-20 16:49 [PATCH 0/9] mtd: spi-nor: parse SFDP tables as defined by JESD216B Cyrille Pitchen
                   ` (2 preceding siblings ...)
  2016-06-20 16:50 ` [PATCH 3/9] Documentation: atmel-quadspi: add binding file for Atmel QSPI driver Cyrille Pitchen
@ 2016-06-20 16:50 ` Cyrille Pitchen
  2016-06-20 16:50 ` [PATCH 5/9] mtd: spi-nor: add support of SPI protocols like SPI 1-2-2 and SPI 1-4-4 Cyrille Pitchen
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 26+ messages in thread
From: Cyrille Pitchen @ 2016-06-20 16:50 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

This driver add support to the new Atmel QSPI controller embedded into
sama5d2x SoCs. It expects a NOR memory to be connected to the QSPI
controller.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/mtd/spi-nor/Kconfig         |   9 +
 drivers/mtd/spi-nor/Makefile        |   1 +
 drivers/mtd/spi-nor/atmel-quadspi.c | 741 ++++++++++++++++++++++++++++++++++++
 3 files changed, 751 insertions(+)
 create mode 100644 drivers/mtd/spi-nor/atmel-quadspi.c

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index d42c98e1f581..c546efd1357b 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -38,6 +38,15 @@ config SPI_FSL_QUADSPI
 	  This controller does not support generic SPI. It only supports
 	  SPI NOR.
 
+config SPI_ATMEL_QUADSPI
+	tristate "Atmel Quad SPI Controller"
+	depends on ARCH_AT91 || (ARM && COMPILE_TEST)
+	depends on OF && HAS_IOMEM
+	help
+	  This enables support for the Quad SPI controller in master mode.
+	  This driver does not support generic SPI. The implementation only
+	  supports SPI NOR.
+
 config SPI_NXP_SPIFI
 	tristate "NXP SPI Flash Interface (SPIFI)"
 	depends on OF && (ARCH_LPC18XX || COMPILE_TEST)
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 0bf3a7f81675..1525913698ad 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_MTD_SPI_NOR)	+= spi-nor.o
 obj-$(CONFIG_SPI_FSL_QUADSPI)	+= fsl-quadspi.o
 obj-$(CONFIG_MTD_MT81xx_NOR)    += mtk-quadspi.o
 obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
+obj-$(CONFIG_SPI_ATMEL_QUADSPI)	+= atmel-quadspi.o
diff --git a/drivers/mtd/spi-nor/atmel-quadspi.c b/drivers/mtd/spi-nor/atmel-quadspi.c
new file mode 100644
index 000000000000..06d1bf276dd0
--- /dev/null
+++ b/drivers/mtd/spi-nor/atmel-quadspi.c
@@ -0,0 +1,741 @@
+/*
+ * Driver for Atmel QSPI Controller
+ *
+ * Copyright (C) 2015 Atmel Corporation
+ *
+ * Author: Cyrille Pitchen <cyrille.pitchen@atmel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * This driver is based on drivers/mtd/spi-nor/fsl-quadspi.c from Freescale.
+ */
+
+#include <linux/kernel.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/platform_data/atmel.h>
+#include <linux/of.h>
+
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/pinctrl/consumer.h>
+
+/* QSPI register offsets */
+#define QSPI_CR      0x0000  /* Control Register */
+#define QSPI_MR      0x0004  /* Mode Register */
+#define QSPI_RD      0x0008  /* Receive Data Register */
+#define QSPI_TD      0x000c  /* Transmit Data Register */
+#define QSPI_SR      0x0010  /* Status Register */
+#define QSPI_IER     0x0014  /* Interrupt Enable Register */
+#define QSPI_IDR     0x0018  /* Interrupt Disable Register */
+#define QSPI_IMR     0x001c  /* Interrupt Mask Register */
+#define QSPI_SCR     0x0020  /* Serial Clock Register */
+
+#define QSPI_IAR     0x0030  /* Instruction Address Register */
+#define QSPI_ICR     0x0034  /* Instruction Code Register */
+#define QSPI_IFR     0x0038  /* Instruction Frame Register */
+
+#define QSPI_SMR     0x0040  /* Scrambling Mode Register */
+#define QSPI_SKR     0x0044  /* Scrambling Key Register */
+
+#define QSPI_WPMR    0x00E4  /* Write Protection Mode Register */
+#define QSPI_WPSR    0x00E8  /* Write Protection Status Register */
+
+#define QSPI_VERSION 0x00FC  /* Version Register */
+
+
+/* Bitfields in QSPI_CR (Control Register) */
+#define QSPI_CR_QSPIEN                  BIT(0)
+#define QSPI_CR_QSPIDIS                 BIT(1)
+#define QSPI_CR_SWRST                   BIT(7)
+#define QSPI_CR_LASTXFER                BIT(24)
+
+/* Bitfields in QSPI_MR (Mode Register) */
+#define QSPI_MR_SSM                     BIT(0)
+#define QSPI_MR_LLB                     BIT(1)
+#define QSPI_MR_WDRBT                   BIT(2)
+#define QSPI_MR_SMRM                    BIT(3)
+#define QSPI_MR_CSMODE_MASK             GENMASK(5, 4)
+#define QSPI_MR_CSMODE_NOT_RELOADED     (0 << 4)
+#define QSPI_MR_CSMODE_LASTXFER         (1 << 4)
+#define QSPI_MR_CSMODE_SYSTEMATICALLY   (2 << 4)
+#define QSPI_MR_NBBITS_MASK             GENMASK(11, 8)
+#define QSPI_MR_NBBITS(n)               ((((n) - 8) << 8) & QSPI_MR_NBBITS_MASK)
+#define QSPI_MR_DLYBCT_MASK             GENMASK(23, 16)
+#define QSPI_MR_DLYBCT(n)               (((n) << 16) & QSPI_MR_DLYBCT_MASK)
+#define QSPI_MR_DLYCS_MASK              GENMASK(31, 24)
+#define QSPI_MR_DLYCS(n)                (((n) << 24) & QSPI_MR_DLYCS_MASK)
+
+/* Bitfields in QSPI_SR/QSPI_IER/QSPI_IDR/QSPI_IMR  */
+#define QSPI_SR_RDRF                    BIT(0)
+#define QSPI_SR_TDRE                    BIT(1)
+#define QSPI_SR_TXEMPTY                 BIT(2)
+#define QSPI_SR_OVRES                   BIT(3)
+#define QSPI_SR_CSR                     BIT(8)
+#define QSPI_SR_CSS                     BIT(9)
+#define QSPI_SR_INSTRE                  BIT(10)
+#define QSPI_SR_QSPIENS                 BIT(24)
+
+#define QSPI_SR_CMD_COMPLETED	(QSPI_SR_INSTRE | QSPI_SR_CSR)
+
+/* Bitfields in QSPI_SCR (Serial Clock Register) */
+#define QSPI_SCR_CPOL                   BIT(0)
+#define QSPI_SCR_CPHA                   BIT(1)
+#define QSPI_SCR_SCBR_MASK              GENMASK(15, 8)
+#define QSPI_SCR_SCBR(n)                (((n) << 8) & QSPI_SCR_SCBR_MASK)
+#define QSPI_SCR_DLYBS_MASK             GENMASK(23, 16)
+#define QSPI_SCR_DLYBS(n)               (((n) << 16) & QSPI_SCR_DLYBS_MASK)
+
+/* Bitfields in QSPI_ICR (Instruction Code Register) */
+#define QSPI_ICR_INST_MASK              GENMASK(7, 0)
+#define QSPI_ICR_INST(inst)             (((inst) << 0) & QSPI_ICR_INST_MASK)
+#define QSPI_ICR_OPT_MASK               GENMASK(23, 16)
+#define QSPI_ICR_OPT(opt)               (((opt) << 16) & QSPI_ICR_OPT_MASK)
+
+/* Bitfields in QSPI_IFR (Instruction Frame Register) */
+#define QSPI_IFR_WIDTH_MASK             GENMASK(2, 0)
+#define QSPI_IFR_WIDTH_SINGLE_BIT_SPI   (0 << 0)
+#define QSPI_IFR_WIDTH_DUAL_OUTPUT      (1 << 0)
+#define QSPI_IFR_WIDTH_QUAD_OUTPUT      (2 << 0)
+#define QSPI_IFR_WIDTH_DUAL_IO          (3 << 0)
+#define QSPI_IFR_WIDTH_QUAD_IO          (4 << 0)
+#define QSPI_IFR_WIDTH_DUAL_CMD         (5 << 0)
+#define QSPI_IFR_WIDTH_QUAD_CMD         (6 << 0)
+#define QSPI_IFR_INSTEN                 BIT(4)
+#define QSPI_IFR_ADDREN                 BIT(5)
+#define QSPI_IFR_OPTEN                  BIT(6)
+#define QSPI_IFR_DATAEN                 BIT(7)
+#define QSPI_IFR_OPTL_MASK              GENMASK(9, 8)
+#define QSPI_IFR_OPTL_1BIT              (0 << 8)
+#define QSPI_IFR_OPTL_2BIT              (1 << 8)
+#define QSPI_IFR_OPTL_4BIT              (2 << 8)
+#define QSPI_IFR_OPTL_8BIT              (3 << 8)
+#define QSPI_IFR_ADDRL                  BIT(10)
+#define QSPI_IFR_TFRTYP_MASK            GENMASK(13, 12)
+#define QSPI_IFR_TFRTYP_TRSFR_READ      (0 << 12)
+#define QSPI_IFR_TFRTYP_TRSFR_READ_MEM  (1 << 12)
+#define QSPI_IFR_TFRTYP_TRSFR_WRITE     (2 << 12)
+#define QSPI_IFR_TFRTYP_TRSFR_WRITE_MEM (3 << 13)
+#define QSPI_IFR_CRM                    BIT(14)
+#define QSPI_IFR_NBDUM_MASK             GENMASK(20, 16)
+#define QSPI_IFR_NBDUM(n)               (((n) << 16) & QSPI_IFR_NBDUM_MASK)
+
+/* Bitfields in QSPI_SMR (Scrambling Mode Register) */
+#define QSPI_SMR_SCREN                  BIT(0)
+#define QSPI_SMR_RVDIS                  BIT(1)
+
+/* Bitfields in QSPI_WPMR (Write Protection Mode Register) */
+#define QSPI_WPMR_WPEN                  BIT(0)
+#define QSPI_WPMR_WPKEY_MASK            GENMASK(31, 8)
+#define QSPI_WPMR_WPKEY(wpkey)          (((wpkey) << 8) & QSPI_WPMR_WPKEY_MASK)
+
+/* Bitfields in QSPI_WPSR (Write Protection Status Register) */
+#define QSPI_WPSR_WPVS                  BIT(0)
+#define QSPI_WPSR_WPVSRC_MASK           GENMASK(15, 8)
+#define QSPI_WPSR_WPVSRC(src)           (((src) << 8) & QSPI_WPSR_WPVSRC)
+
+
+struct atmel_qspi {
+	void __iomem		*regs;
+	void __iomem		*mem;
+	struct clk		*clk;
+	struct platform_device	*pdev;
+	u32			pending;
+
+	struct spi_nor		nor;
+	u32			clk_rate;
+	struct completion	cmd_completion;
+};
+
+struct atmel_qspi_command {
+	union {
+		struct {
+			u32	instruction:1;
+			u32	address:3;
+			u32	mode:1;
+			u32	dummy:1;
+			u32	data:1;
+			u32	reserved:25;
+		}		bits;
+		u32	word;
+	}	enable;
+	u8	instruction;
+	u8	mode;
+	u8	num_mode_cycles;
+	u8	num_dummy_cycles;
+	u32	address;
+
+	size_t		buf_len;
+	const void	*tx_buf;
+	void		*rx_buf;
+};
+
+/* Register access functions */
+static inline u32 qspi_readl(struct atmel_qspi *aq, u32 reg)
+{
+	return readl_relaxed(aq->regs + reg);
+}
+
+static inline void qspi_writel(struct atmel_qspi *aq, u32 reg, u32 value)
+{
+	writel_relaxed(value, aq->regs + reg);
+}
+
+static int atmel_qspi_run_transfer(struct atmel_qspi *aq,
+				   const struct atmel_qspi_command *cmd)
+{
+	void __iomem *ahb_mem;
+
+	/* Then fallback to a PIO transfer (memcpy() DOES NOT work!) */
+	ahb_mem = aq->mem;
+	if (cmd->enable.bits.address)
+		ahb_mem += cmd->address;
+	if (cmd->tx_buf)
+		_memcpy_toio(ahb_mem, cmd->tx_buf, cmd->buf_len);
+	else
+		_memcpy_fromio(cmd->rx_buf, ahb_mem, cmd->buf_len);
+
+	return 0;
+}
+
+#ifdef DEBUG
+static void atmel_qspi_debug_command(struct atmel_qspi *aq,
+				     const struct atmel_qspi_command *cmd,
+				     u32 ifr)
+{
+	u8 cmd_buf[SPI_NOR_MAX_CMD_SIZE];
+	size_t len = 0;
+	int i;
+
+	if (cmd->enable.bits.instruction)
+		cmd_buf[len++] = cmd->instruction;
+
+	for (i = cmd->enable.bits.address-1; i >= 0; --i)
+		cmd_buf[len++] = (cmd->address >> (i << 3)) & 0xff;
+
+	if (cmd->enable.bits.mode)
+		cmd_buf[len++] = cmd->mode;
+
+	if (cmd->enable.bits.dummy) {
+		int num = cmd->num_dummy_cycles;
+
+		switch (ifr & QSPI_IFR_WIDTH_MASK) {
+		case QSPI_IFR_WIDTH_SINGLE_BIT_SPI:
+		case QSPI_IFR_WIDTH_DUAL_OUTPUT:
+		case QSPI_IFR_WIDTH_QUAD_OUTPUT:
+			num >>= 3;
+			break;
+		case QSPI_IFR_WIDTH_DUAL_IO:
+		case QSPI_IFR_WIDTH_DUAL_CMD:
+			num >>= 2;
+			break;
+		case QSPI_IFR_WIDTH_QUAD_IO:
+		case QSPI_IFR_WIDTH_QUAD_CMD:
+			num >>= 1;
+			break;
+		default:
+			return;
+		}
+
+		for (i = 0; i < num; ++i)
+			cmd_buf[len++] = 0;
+	}
+
+	/* Dump the SPI command */
+	print_hex_dump(KERN_DEBUG, "qspi cmd: ", DUMP_PREFIX_NONE,
+		       32, 1, cmd_buf, len, false);
+
+#ifdef VERBOSE_DEBUG
+	/* If verbose debug is enabled, also dump the TX data */
+	if (cmd->enable.bits.data && cmd->tx_buf)
+		print_hex_dump(KERN_DEBUG, "qspi tx : ", DUMP_PREFIX_NONE,
+			       32, 1, cmd->tx_buf, cmd->buf_len, false);
+#endif
+}
+#else
+#define atmel_qspi_debug_command(aq, cmd, ifr)
+#endif
+
+static int atmel_qspi_run_command(struct atmel_qspi *aq,
+				  const struct atmel_qspi_command *cmd,
+				  u32 ifr_tfrtyp, u32 ifr_width)
+{
+	u32 iar, icr, ifr, sr;
+	int err = 0;
+
+	iar = 0;
+	icr = 0;
+	ifr = ifr_tfrtyp | ifr_width;
+
+	/* Compute instruction parameters */
+	if (cmd->enable.bits.instruction) {
+		icr |= QSPI_ICR_INST(cmd->instruction);
+		ifr |= QSPI_IFR_INSTEN;
+	}
+
+	/* Compute address parameters */
+	switch (cmd->enable.bits.address) {
+	case 4:
+		ifr |= QSPI_IFR_ADDRL;
+		/* fall through to the 24bit (3 byte) address case. */
+	case 3:
+		iar = (cmd->enable.bits.data) ? 0 : cmd->address;
+		ifr |= QSPI_IFR_ADDREN;
+		break;
+	case 0:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Compute option parameters */
+	if (cmd->enable.bits.mode && cmd->num_mode_cycles) {
+		u32 mode_cycle_bits, mode_bits;
+
+		icr |= QSPI_ICR_OPT(cmd->mode);
+		ifr |= QSPI_IFR_OPTEN;
+
+		switch (ifr & QSPI_IFR_WIDTH_MASK) {
+		case QSPI_IFR_WIDTH_SINGLE_BIT_SPI:
+		case QSPI_IFR_WIDTH_DUAL_OUTPUT:
+		case QSPI_IFR_WIDTH_QUAD_OUTPUT:
+			mode_cycle_bits = 1;
+			break;
+		case QSPI_IFR_WIDTH_DUAL_IO:
+		case QSPI_IFR_WIDTH_DUAL_CMD:
+			mode_cycle_bits = 2;
+			break;
+		case QSPI_IFR_WIDTH_QUAD_IO:
+		case QSPI_IFR_WIDTH_QUAD_CMD:
+			mode_cycle_bits = 4;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		mode_bits = cmd->num_mode_cycles * mode_cycle_bits;
+		switch (mode_bits) {
+		case 1:
+			ifr |= QSPI_IFR_OPTL_1BIT;
+			break;
+
+		case 2:
+			ifr |= QSPI_IFR_OPTL_2BIT;
+			break;
+
+		case 4:
+			ifr |= QSPI_IFR_OPTL_4BIT;
+			break;
+
+		case 8:
+			ifr |= QSPI_IFR_OPTL_8BIT;
+			break;
+
+		default:
+			return -EINVAL;
+		}
+	}
+
+	/* Set number of dummy cycles */
+	if (cmd->enable.bits.dummy)
+		ifr |= QSPI_IFR_NBDUM(cmd->num_dummy_cycles);
+
+	/* Set data enable */
+	if (cmd->enable.bits.data) {
+		ifr |= QSPI_IFR_DATAEN;
+
+		/* Special case for Continuous Read Mode */
+		if (!cmd->tx_buf && !cmd->rx_buf)
+			ifr |= QSPI_IFR_CRM;
+	}
+
+	/* Clear pending interrupts */
+	(void)qspi_readl(aq, QSPI_SR);
+
+	/* Set QSPI Instruction Frame registers */
+	atmel_qspi_debug_command(aq, cmd, ifr);
+	qspi_writel(aq, QSPI_IAR, iar);
+	qspi_writel(aq, QSPI_ICR, icr);
+	qspi_writel(aq, QSPI_IFR, ifr);
+
+	/* Skip to the final steps if there is no data */
+	if (!cmd->enable.bits.data)
+		goto no_data;
+
+	/* Dummy read of QSPI_IFR to synchronize APB and AHB accesses */
+	(void)qspi_readl(aq, QSPI_IFR);
+
+	/* Stop here for continuous read */
+	if (!cmd->tx_buf && !cmd->rx_buf)
+		return 0;
+	/* Send/Receive data */
+	err = atmel_qspi_run_transfer(aq, cmd);
+
+	/* Release the chip-select */
+	qspi_writel(aq, QSPI_CR, QSPI_CR_LASTXFER);
+
+	if (err)
+		return err;
+
+#if defined(DEBUG) && defined(VERBOSE_DEBUG)
+	/*
+	 * If verbose debug is enabled, also dump the RX data in addition to
+	 * the SPI command previously dumped by atmel_qspi_debug_command()
+	 */
+	if (cmd->rx_buf)
+		print_hex_dump(KERN_DEBUG, "qspi rx : ", DUMP_PREFIX_NONE,
+			       32, 1, cmd->rx_buf, cmd->buf_len, false);
+#endif
+no_data:
+	/* Poll INSTRuction End status */
+	sr = qspi_readl(aq, QSPI_SR);
+	if ((sr & QSPI_SR_CMD_COMPLETED) == QSPI_SR_CMD_COMPLETED)
+		return err;
+
+	/* Wait for INSTRuction End interrupt */
+	reinit_completion(&aq->cmd_completion);
+	aq->pending = sr & QSPI_SR_CMD_COMPLETED;
+	qspi_writel(aq, QSPI_IER, QSPI_SR_CMD_COMPLETED);
+	if (!wait_for_completion_timeout(&aq->cmd_completion,
+					 msecs_to_jiffies(1000)))
+		err = -ETIMEDOUT;
+	qspi_writel(aq, QSPI_IDR, QSPI_SR_CMD_COMPLETED);
+
+	return err;
+}
+
+static int atmel_qspi_read_reg(struct spi_nor *nor, u8 opcode,
+			       u8 *buf, int len)
+{
+	struct atmel_qspi *aq = nor->priv;
+	struct atmel_qspi_command cmd;
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.enable.bits.instruction = 1;
+	cmd.enable.bits.data = 1;
+	cmd.instruction = opcode;
+	cmd.rx_buf = buf;
+	cmd.buf_len = len;
+	return atmel_qspi_run_command(aq, &cmd, QSPI_IFR_TFRTYP_TRSFR_READ,
+				      QSPI_IFR_WIDTH_SINGLE_BIT_SPI);
+}
+
+static int atmel_qspi_write_reg(struct spi_nor *nor, u8 opcode,
+				u8 *buf, int len)
+{
+	struct atmel_qspi *aq = nor->priv;
+	struct atmel_qspi_command cmd;
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.enable.bits.instruction = 1;
+	cmd.enable.bits.data = (buf != NULL && len > 0);
+	cmd.instruction = opcode;
+	cmd.tx_buf = buf;
+	cmd.buf_len = len;
+	return atmel_qspi_run_command(aq, &cmd, QSPI_IFR_TFRTYP_TRSFR_WRITE,
+				      QSPI_IFR_WIDTH_SINGLE_BIT_SPI);
+}
+
+static ssize_t atmel_qspi_write(struct spi_nor *nor, loff_t to, size_t len,
+				const u_char *write_buf)
+{
+	struct atmel_qspi *aq = nor->priv;
+	struct atmel_qspi_command cmd;
+	ssize_t ret;
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.enable.bits.instruction = 1;
+	cmd.enable.bits.address = nor->addr_width;
+	cmd.enable.bits.data = 1;
+	cmd.instruction = nor->program_opcode;
+	cmd.address = (u32)to;
+	cmd.tx_buf = write_buf;
+	cmd.buf_len = len;
+	ret = atmel_qspi_run_command(aq, &cmd, QSPI_IFR_TFRTYP_TRSFR_WRITE_MEM,
+				     QSPI_IFR_WIDTH_SINGLE_BIT_SPI);
+	return (ret < 0) ? ret : len;
+}
+
+static int atmel_qspi_erase(struct spi_nor *nor, loff_t offs)
+{
+	struct atmel_qspi *aq = nor->priv;
+	struct atmel_qspi_command cmd;
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.enable.bits.instruction = 1;
+	cmd.enable.bits.address = nor->addr_width;
+	cmd.instruction = nor->erase_opcode;
+	cmd.address = (u32)offs;
+	return atmel_qspi_run_command(aq, &cmd, QSPI_IFR_TFRTYP_TRSFR_WRITE,
+				      QSPI_IFR_WIDTH_SINGLE_BIT_SPI);
+}
+
+static ssize_t atmel_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
+			       u_char *read_buf)
+{
+	struct atmel_qspi *aq = nor->priv;
+	struct atmel_qspi_command cmd;
+	u8 num_mode_cycles, num_dummy_cycles;
+	u32 ifr_width;
+	ssize_t ret;
+
+	switch (nor->flash_read) {
+	case SPI_NOR_NORMAL:
+	case SPI_NOR_FAST:
+		ifr_width = QSPI_IFR_WIDTH_SINGLE_BIT_SPI;
+		break;
+
+	case SPI_NOR_DUAL:
+		ifr_width = QSPI_IFR_WIDTH_DUAL_OUTPUT;
+		break;
+
+	case SPI_NOR_QUAD:
+		ifr_width = QSPI_IFR_WIDTH_QUAD_OUTPUT;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	if (nor->read_dummy >= 2) {
+		num_mode_cycles = 2;
+		num_dummy_cycles = nor->read_dummy - 2;
+	} else {
+		num_mode_cycles = nor->read_dummy;
+		num_dummy_cycles = 0;
+	}
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.enable.bits.instruction = 1;
+	cmd.enable.bits.address = nor->addr_width;
+	cmd.enable.bits.mode = (num_mode_cycles > 0);
+	cmd.enable.bits.dummy = (num_dummy_cycles > 0);
+	cmd.enable.bits.data = 1;
+	cmd.instruction = nor->read_opcode;
+	cmd.address = (u32)from;
+	cmd.mode = 0xff; /* This value prevents from entering the 0-4-4 mode */
+	cmd.num_mode_cycles = num_mode_cycles;
+	cmd.num_dummy_cycles = num_dummy_cycles;
+	cmd.rx_buf = read_buf;
+	cmd.buf_len = len;
+	ret = atmel_qspi_run_command(aq, &cmd, QSPI_IFR_TFRTYP_TRSFR_READ_MEM,
+				     ifr_width);
+	return (ret < 0) ? ret : len;
+}
+
+static int atmel_qspi_init(struct atmel_qspi *aq)
+{
+	unsigned long src_rate;
+	u32 mr, scr, scbr;
+
+	/* Reset the QSPI controller */
+	qspi_writel(aq, QSPI_CR, QSPI_CR_SWRST);
+
+	/* Set the QSPI controller in Serial Memory Mode */
+	mr = QSPI_MR_NBBITS(8) | QSPI_MR_SSM;
+	qspi_writel(aq, QSPI_MR, mr);
+
+	src_rate = clk_get_rate(aq->clk);
+	if (!src_rate)
+		return -EINVAL;
+
+	/* Compute the QSPI baudrate */
+	scbr = DIV_ROUND_UP(src_rate, aq->clk_rate);
+	if (scbr > 0)
+		scbr--;
+	scr = QSPI_SCR_SCBR(scbr);
+	qspi_writel(aq, QSPI_SCR, scr);
+
+	/* Enable the QSPI controller */
+	qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIEN);
+
+	return 0;
+}
+
+static irqreturn_t atmel_qspi_interrupt(int irq, void *dev_id)
+{
+	struct atmel_qspi *aq = (struct atmel_qspi *)dev_id;
+	u32 status, mask, pending;
+
+	status = qspi_readl(aq, QSPI_SR);
+	mask = qspi_readl(aq, QSPI_IMR);
+	pending = status & mask;
+
+	if (!pending)
+		return IRQ_NONE;
+
+	aq->pending |= pending;
+	if ((aq->pending & QSPI_SR_CMD_COMPLETED) == QSPI_SR_CMD_COMPLETED)
+		complete(&aq->cmd_completion);
+
+	return IRQ_HANDLED;
+}
+
+static int atmel_qspi_probe(struct platform_device *pdev)
+{
+	struct device_node *child, *np = pdev->dev.of_node;
+	char modalias[SPI_NAME_SIZE];
+	const char *name = NULL;
+	struct atmel_qspi *aq;
+	struct resource *res;
+	struct spi_nor *nor;
+	struct mtd_info *mtd;
+	int irq, err = 0;
+
+	if (of_get_child_count(np) != 1)
+		return -ENODEV;
+	child = of_get_next_child(np, NULL);
+
+	aq = devm_kzalloc(&pdev->dev, sizeof(*aq), GFP_KERNEL);
+	if (!aq) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	platform_set_drvdata(pdev, aq);
+	init_completion(&aq->cmd_completion);
+	aq->pdev = pdev;
+
+	/* Map the registers */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi_base");
+	aq->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(aq->regs)) {
+		dev_err(&pdev->dev, "missing registers\n");
+		err = PTR_ERR(aq->regs);
+		goto exit;
+	}
+
+	/* Map the AHB memory */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi_mmap");
+	aq->mem = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(aq->mem)) {
+		dev_err(&pdev->dev, "missing AHB memory\n");
+		err = PTR_ERR(aq->mem);
+		goto exit;
+	}
+
+	/* Get the peripheral clock */
+	aq->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(aq->clk)) {
+		dev_err(&pdev->dev, "missing peripheral clock\n");
+		err = PTR_ERR(aq->clk);
+		goto exit;
+	}
+
+	/* Enable the peripheral clock */
+	err = clk_prepare_enable(aq->clk);
+	if (err) {
+		dev_err(&pdev->dev, "failed to enable the peripheral clock\n");
+		goto exit;
+	}
+
+	/* Request the IRQ */
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "missing IRQ\n");
+		err = irq;
+		goto disable_clk;
+	}
+	err = devm_request_irq(&pdev->dev, irq, atmel_qspi_interrupt,
+			       0, dev_name(&pdev->dev), aq);
+	if (err)
+		goto disable_clk;
+
+	/* Setup the spi-nor */
+	nor = &aq->nor;
+	mtd = &nor->mtd;
+
+	nor->dev = &pdev->dev;
+	spi_nor_set_flash_node(nor, child);
+	nor->priv = aq;
+	mtd->priv = nor;
+
+	nor->read_reg = atmel_qspi_read_reg;
+	nor->write_reg = atmel_qspi_write_reg;
+	nor->read = atmel_qspi_read;
+	nor->write = atmel_qspi_write;
+	nor->erase = atmel_qspi_erase;
+
+	err = of_modalias_node(child, modalias, sizeof(modalias));
+	if (err < 0)
+		goto disable_clk;
+
+	if (strcmp(modalias, "spi-nor"))
+		name = modalias;
+
+	err = of_property_read_u32(child, "spi-max-frequency", &aq->clk_rate);
+	if (err < 0)
+		goto disable_clk;
+
+	err = atmel_qspi_init(aq);
+	if (err)
+		goto disable_clk;
+
+	err = spi_nor_scan(nor, name, SPI_NOR_QUAD);
+	if (err)
+		goto disable_clk;
+
+	err = mtd_device_register(mtd, NULL, 0);
+	if (err)
+		goto disable_clk;
+
+	of_node_put(child);
+
+	return 0;
+
+disable_clk:
+	clk_disable_unprepare(aq->clk);
+exit:
+	of_node_put(child);
+
+	return err;
+}
+
+static int atmel_qspi_remove(struct platform_device *pdev)
+{
+	struct atmel_qspi *aq = platform_get_drvdata(pdev);
+
+	mtd_device_unregister(&aq->nor.mtd);
+	qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIDIS);
+	clk_disable_unprepare(aq->clk);
+	return 0;
+}
+
+
+static const struct of_device_id atmel_qspi_dt_ids[] = {
+	{ .compatible = "atmel,sama5d2-qspi" },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, atmel_qspi_dt_ids);
+
+static struct platform_driver atmel_qspi_driver = {
+	.driver = {
+		.name	= "atmel_qspi",
+		.of_match_table	= atmel_qspi_dt_ids,
+	},
+	.probe		= atmel_qspi_probe,
+	.remove		= atmel_qspi_remove,
+};
+module_platform_driver(atmel_qspi_driver);
+
+MODULE_AUTHOR("Cyrille Pitchen <cyrille.pitchen@atmel.com>");
+MODULE_DESCRIPTION("Atmel QSPI Controller driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.2.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 5/9] mtd: spi-nor: add support of SPI protocols like SPI 1-2-2 and SPI 1-4-4
  2016-06-20 16:49 [PATCH 0/9] mtd: spi-nor: parse SFDP tables as defined by JESD216B Cyrille Pitchen
                   ` (3 preceding siblings ...)
  2016-06-20 16:50 ` [PATCH 4/9] mtd: atmel-quadspi: add driver for Atmel QSPI controller Cyrille Pitchen
@ 2016-06-20 16:50 ` Cyrille Pitchen
  2016-06-20 16:50 ` [PATCH 6/9] mtd: spi-nor: remove unused set_quad_mode() function Cyrille Pitchen
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 26+ messages in thread
From: Cyrille Pitchen @ 2016-06-20 16:50 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

This patch changes the prototype of spi_nor_scan(): its 3rd parameter
is replaced by a const struct spi_nor_modes pointer, which tells the
spi-nor framework about which SPI protocols are supported by the SPI
controller.

Besides, this patch also introduces a new spi_nor_basic_flash_parameter
structure telling the spi-nor framework about the SPI protocols supported
by the SPI memory and the needed op codes to use these SPI protocols.

Currently the struct spi_nor_basic_flash_parameter is filled with legacy
values but a later patch will allow to fill it dynamically by reading the
JESD216 Serial Flash Discoverable Parameter (SFDP) tables from the SPI
memory.

With both structures, the spi-nor framework can now compute the best
match between SPI protocols supported by both the (Q)SPI memory and
controller hence selecting the relevant op codes for (Fast) Read, Page
Program and Sector Erase operations.

The spi_nor_basic_flash_parameter structure also provides the spi-nor
framework with the number of dummy cycles to be used with each Fast Read
commands and the erase block size associated to the erase block op codes.

Finally the spi_nor_basic_flash_parameter structure, through the optional
.enable_quad_io() hook, tells the spi-nor framework how to set the Quad
Enable (QE) bit of the QSPI memory to enable its Quad SPI features.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/mtd/devices/m25p80.c        |  17 ++-
 drivers/mtd/spi-nor/atmel-quadspi.c |  83 +++++++----
 drivers/mtd/spi-nor/fsl-quadspi.c   |   8 +-
 drivers/mtd/spi-nor/mtk-quadspi.c   |  16 ++-
 drivers/mtd/spi-nor/nxp-spifi.c     |  21 +--
 drivers/mtd/spi-nor/spi-nor.c       | 278 ++++++++++++++++++++++++++++--------
 include/linux/mtd/spi-nor.h         | 136 ++++++++++++++++--
 7 files changed, 444 insertions(+), 115 deletions(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 9cf7fcd28034..f0a55c01406b 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -111,10 +111,10 @@ static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
 
 static inline unsigned int m25p80_rx_nbits(struct spi_nor *nor)
 {
-	switch (nor->flash_read) {
-	case SPI_NOR_DUAL:
+	switch (SNOR_PROTO_DATA_FROM_PROTO(nor->read_proto)) {
+	case 2:
 		return 2;
-	case SPI_NOR_QUAD:
+	case 4:
 		return 4;
 	default:
 		return 0;
@@ -195,7 +195,10 @@ static int m25p_probe(struct spi_device *spi)
 	struct flash_platform_data	*data;
 	struct m25p *flash;
 	struct spi_nor *nor;
-	enum read_mode mode = SPI_NOR_NORMAL;
+	struct spi_nor_modes modes = {
+		.rd_modes = SNOR_MODE_SLOW,
+		.wr_modes = SNOR_MODE_1_1_1,
+	};
 	char *flash_name;
 	int ret;
 
@@ -221,9 +224,9 @@ static int m25p_probe(struct spi_device *spi)
 	flash->spi = spi;
 
 	if (spi->mode & SPI_RX_QUAD)
-		mode = SPI_NOR_QUAD;
+		modes.rd_modes |= SNOR_MODE_1_1_4;
 	else if (spi->mode & SPI_RX_DUAL)
-		mode = SPI_NOR_DUAL;
+		modes.rd_modes |= SNOR_MODE_1_1_2;
 
 	if (data && data->name)
 		nor->mtd.name = data->name;
@@ -240,7 +243,7 @@ static int m25p_probe(struct spi_device *spi)
 	else
 		flash_name = spi->modalias;
 
-	ret = spi_nor_scan(nor, flash_name, mode);
+	ret = spi_nor_scan(nor, flash_name, &modes);
 	if (ret)
 		return ret;
 
diff --git a/drivers/mtd/spi-nor/atmel-quadspi.c b/drivers/mtd/spi-nor/atmel-quadspi.c
index 06d1bf276dd0..d51c3db2110f 100644
--- a/drivers/mtd/spi-nor/atmel-quadspi.c
+++ b/drivers/mtd/spi-nor/atmel-quadspi.c
@@ -275,14 +275,48 @@ static void atmel_qspi_debug_command(struct atmel_qspi *aq,
 
 static int atmel_qspi_run_command(struct atmel_qspi *aq,
 				  const struct atmel_qspi_command *cmd,
-				  u32 ifr_tfrtyp, u32 ifr_width)
+				  u32 ifr_tfrtyp, enum spi_nor_protocol proto)
 {
 	u32 iar, icr, ifr, sr;
 	int err = 0;
 
 	iar = 0;
 	icr = 0;
-	ifr = ifr_tfrtyp | ifr_width;
+	ifr = ifr_tfrtyp;
+
+	/* Set the SPI protocol */
+	switch (proto) {
+	case SNOR_PROTO_1_1_1:
+		ifr |= QSPI_IFR_WIDTH_SINGLE_BIT_SPI;
+		break;
+
+	case SNOR_PROTO_1_1_2:
+		ifr |= QSPI_IFR_WIDTH_DUAL_OUTPUT;
+		break;
+
+	case SNOR_PROTO_1_1_4:
+		ifr |= QSPI_IFR_WIDTH_QUAD_OUTPUT;
+		break;
+
+	case SNOR_PROTO_1_2_2:
+		ifr |= QSPI_IFR_WIDTH_DUAL_IO;
+		break;
+
+	case SNOR_PROTO_1_4_4:
+		ifr |= QSPI_IFR_WIDTH_QUAD_IO;
+		break;
+
+	case SNOR_PROTO_2_2_2:
+		ifr |= QSPI_IFR_WIDTH_DUAL_CMD;
+		break;
+
+	case SNOR_PROTO_4_4_4:
+		ifr |= QSPI_IFR_WIDTH_QUAD_CMD;
+		break;
+
+	default:
+		return -EINVAL;
+	}
 
 	/* Compute instruction parameters */
 	if (cmd->enable.bits.instruction) {
@@ -434,7 +468,7 @@ static int atmel_qspi_read_reg(struct spi_nor *nor, u8 opcode,
 	cmd.rx_buf = buf;
 	cmd.buf_len = len;
 	return atmel_qspi_run_command(aq, &cmd, QSPI_IFR_TFRTYP_TRSFR_READ,
-				      QSPI_IFR_WIDTH_SINGLE_BIT_SPI);
+				      nor->reg_proto);
 }
 
 static int atmel_qspi_write_reg(struct spi_nor *nor, u8 opcode,
@@ -450,7 +484,7 @@ static int atmel_qspi_write_reg(struct spi_nor *nor, u8 opcode,
 	cmd.tx_buf = buf;
 	cmd.buf_len = len;
 	return atmel_qspi_run_command(aq, &cmd, QSPI_IFR_TFRTYP_TRSFR_WRITE,
-				      QSPI_IFR_WIDTH_SINGLE_BIT_SPI);
+				      nor->reg_proto);
 }
 
 static ssize_t atmel_qspi_write(struct spi_nor *nor, loff_t to, size_t len,
@@ -469,7 +503,7 @@ static ssize_t atmel_qspi_write(struct spi_nor *nor, loff_t to, size_t len,
 	cmd.tx_buf = write_buf;
 	cmd.buf_len = len;
 	ret = atmel_qspi_run_command(aq, &cmd, QSPI_IFR_TFRTYP_TRSFR_WRITE_MEM,
-				     QSPI_IFR_WIDTH_SINGLE_BIT_SPI);
+				     nor->write_proto);
 	return (ret < 0) ? ret : len;
 }
 
@@ -484,7 +518,7 @@ static int atmel_qspi_erase(struct spi_nor *nor, loff_t offs)
 	cmd.instruction = nor->erase_opcode;
 	cmd.address = (u32)offs;
 	return atmel_qspi_run_command(aq, &cmd, QSPI_IFR_TFRTYP_TRSFR_WRITE,
-				      QSPI_IFR_WIDTH_SINGLE_BIT_SPI);
+				      nor->reg_proto);
 }
 
 static ssize_t atmel_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
@@ -493,27 +527,8 @@ static ssize_t atmel_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
 	struct atmel_qspi *aq = nor->priv;
 	struct atmel_qspi_command cmd;
 	u8 num_mode_cycles, num_dummy_cycles;
-	u32 ifr_width;
 	ssize_t ret;
 
-	switch (nor->flash_read) {
-	case SPI_NOR_NORMAL:
-	case SPI_NOR_FAST:
-		ifr_width = QSPI_IFR_WIDTH_SINGLE_BIT_SPI;
-		break;
-
-	case SPI_NOR_DUAL:
-		ifr_width = QSPI_IFR_WIDTH_DUAL_OUTPUT;
-		break;
-
-	case SPI_NOR_QUAD:
-		ifr_width = QSPI_IFR_WIDTH_QUAD_OUTPUT;
-		break;
-
-	default:
-		return -EINVAL;
-	}
-
 	if (nor->read_dummy >= 2) {
 		num_mode_cycles = 2;
 		num_dummy_cycles = nor->read_dummy - 2;
@@ -536,7 +551,7 @@ static ssize_t atmel_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
 	cmd.rx_buf = read_buf;
 	cmd.buf_len = len;
 	ret = atmel_qspi_run_command(aq, &cmd, QSPI_IFR_TFRTYP_TRSFR_READ_MEM,
-				     ifr_width);
+				     nor->read_proto);
 	return (ret < 0) ? ret : len;
 }
 
@@ -598,6 +613,20 @@ static int atmel_qspi_probe(struct platform_device *pdev)
 	struct spi_nor *nor;
 	struct mtd_info *mtd;
 	int irq, err = 0;
+	struct spi_nor_modes modes = {
+		.rd_modes = (SNOR_MODE_SLOW |
+			     SNOR_MODE_1_1_1 |
+			     SNOR_MODE_1_1_2 |
+			     SNOR_MODE_1_1_4 |
+			     SNOR_MODE_1_2_2 |
+			     SNOR_MODE_1_4_4),
+		.wr_modes = (SNOR_MODE_SLOW |
+			     SNOR_MODE_1_1_1 |
+			     SNOR_MODE_1_1_2 |
+			     SNOR_MODE_1_1_4 |
+			     SNOR_MODE_1_2_2 |
+			     SNOR_MODE_1_4_4),
+	};
 
 	if (of_get_child_count(np) != 1)
 		return -ENODEV;
@@ -688,7 +717,7 @@ static int atmel_qspi_probe(struct platform_device *pdev)
 	if (err)
 		goto disable_clk;
 
-	err = spi_nor_scan(nor, name, SPI_NOR_QUAD);
+	err = spi_nor_scan(nor, name, &modes);
 	if (err)
 		goto disable_clk;
 
diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 5c82e4ef1904..01e7356d6623 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -980,6 +980,12 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 	struct spi_nor *nor;
 	struct mtd_info *mtd;
 	int ret, i = 0;
+	static const struct spi_nor_modes modes = {
+		.rd_modes = (SNOR_MODE_SLOW |
+			     SNOR_MODE_1_1_1 |
+			     SNOR_MODE_1_1_4),
+		.wr_modes = SNOR_MODE_1_1_1,
+	};
 
 	q = devm_kzalloc(dev, sizeof(*q), GFP_KERNEL);
 	if (!q)
@@ -1081,7 +1087,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 		/* set the chip address for READID */
 		fsl_qspi_set_base_addr(q, nor);
 
-		ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
+		ret = spi_nor_scan(nor, NULL, &modes);
 		if (ret)
 			goto mutex_failed;
 
diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
index 21c31b373cd3..c07cc2bd3fd7 100644
--- a/drivers/mtd/spi-nor/mtk-quadspi.c
+++ b/drivers/mtd/spi-nor/mtk-quadspi.c
@@ -122,20 +122,20 @@ static void mt8173_nor_set_read_mode(struct mt8173_nor *mt8173_nor)
 {
 	struct spi_nor *nor = &mt8173_nor->nor;
 
-	switch (nor->flash_read) {
-	case SPI_NOR_FAST:
+	switch (SNOR_PROTO_DATA_FROM_PROTO(nor->read_proto)) {
+	case 1:
 		writeb(nor->read_opcode, mt8173_nor->base +
 		       MTK_NOR_PRGDATA3_REG);
 		writeb(MTK_NOR_FAST_READ, mt8173_nor->base +
 		       MTK_NOR_CFG1_REG);
 		break;
-	case SPI_NOR_DUAL:
+	case 2:
 		writeb(nor->read_opcode, mt8173_nor->base +
 		       MTK_NOR_PRGDATA3_REG);
 		writeb(MTK_NOR_DUAL_READ_EN, mt8173_nor->base +
 		       MTK_NOR_DUAL_REG);
 		break;
-	case SPI_NOR_QUAD:
+	case 4:
 		writeb(nor->read_opcode, mt8173_nor->base +
 		       MTK_NOR_PRGDATA4_REG);
 		writeb(MTK_NOR_QUAD_READ_EN, mt8173_nor->base +
@@ -384,6 +384,12 @@ static int mtk_nor_init(struct mt8173_nor *mt8173_nor,
 {
 	int ret;
 	struct spi_nor *nor;
+	static const struct spi_nor_modes modes = {
+		.rd_modes = (SNOR_MODE_SLOW |
+			     SNOR_MODE_1_1_1 |
+			     SNOR_MODE_1_1_2),
+		.wr_modes = SNOR_MODE_1_1_1,
+	};
 
 	/* initialize controller to accept commands */
 	writel(MTK_NOR_ENABLE_SF_CMD, mt8173_nor->base + MTK_NOR_WRPROT_REG);
@@ -400,7 +406,7 @@ static int mtk_nor_init(struct mt8173_nor *mt8173_nor,
 	nor->write_reg = mt8173_nor_write_reg;
 	nor->mtd.name = "mtk_nor";
 	/* initialized with NULL */
-	ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL);
+	ret = spi_nor_scan(nor, NULL, &modes);
 	if (ret)
 		return ret;
 
diff --git a/drivers/mtd/spi-nor/nxp-spifi.c b/drivers/mtd/spi-nor/nxp-spifi.c
index 73a14f40928b..327c5b5fe9da 100644
--- a/drivers/mtd/spi-nor/nxp-spifi.c
+++ b/drivers/mtd/spi-nor/nxp-spifi.c
@@ -240,13 +240,12 @@ static int nxp_spifi_erase(struct spi_nor *nor, loff_t offs)
 
 static int nxp_spifi_setup_memory_cmd(struct nxp_spifi *spifi)
 {
-	switch (spifi->nor.flash_read) {
-	case SPI_NOR_NORMAL:
-	case SPI_NOR_FAST:
+	switch (SNOR_PROTO_DATA_FROM_PROTO(spifi->nor.read_proto)) {
+	case 1:
 		spifi->mcmd = SPIFI_CMD_FIELDFORM_ALL_SERIAL;
 		break;
-	case SPI_NOR_DUAL:
-	case SPI_NOR_QUAD:
+	case 2:
+	case 4:
 		spifi->mcmd = SPIFI_CMD_FIELDFORM_QUAD_DUAL_DATA;
 		break;
 	default:
@@ -274,7 +273,10 @@ static void nxp_spifi_dummy_id_read(struct spi_nor *nor)
 static int nxp_spifi_setup_flash(struct nxp_spifi *spifi,
 				 struct device_node *np)
 {
-	enum read_mode flash_read;
+	struct spi_nor_modes modes = {
+		.rd_modes = (SNOR_MODE_SLOW | SNOR_MODE_1_1_1),
+		.wr_modes = SNOR_MODE_1_1_1,
+	};
 	u32 ctrl, property;
 	u16 mode = 0;
 	int ret;
@@ -308,13 +310,12 @@ static int nxp_spifi_setup_flash(struct nxp_spifi *spifi,
 
 	if (mode & SPI_RX_DUAL) {
 		ctrl |= SPIFI_CTRL_DUAL;
-		flash_read = SPI_NOR_DUAL;
+		modes.rd_modes |= SNOR_MODE_1_1_2;
 	} else if (mode & SPI_RX_QUAD) {
 		ctrl &= ~SPIFI_CTRL_DUAL;
-		flash_read = SPI_NOR_QUAD;
+		modes.rd_modes |= SNOR_MODE_1_1_4;
 	} else {
 		ctrl |= SPIFI_CTRL_DUAL;
-		flash_read = SPI_NOR_NORMAL;
 	}
 
 	switch (mode & (SPI_CPHA | SPI_CPOL)) {
@@ -351,7 +352,7 @@ static int nxp_spifi_setup_flash(struct nxp_spifi *spifi,
 	 */
 	nxp_spifi_dummy_id_read(&spifi->nor);
 
-	ret = spi_nor_scan(&spifi->nor, NULL, flash_read);
+	ret = spi_nor_scan(&spifi->nor, NULL, &modes);
 	if (ret) {
 		dev_err(spifi->dev, "device scan failed\n");
 		return ret;
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index e484bfe54c36..29ba0e665fac 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -143,24 +143,6 @@ static int read_cr(struct spi_nor *nor)
 }
 
 /*
- * Dummy Cycle calculation for different type of read.
- * It can be used to support more commands with
- * different dummy cycle requirements.
- */
-static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
-{
-	switch (nor->flash_read) {
-	case SPI_NOR_FAST:
-	case SPI_NOR_DUAL:
-	case SPI_NOR_QUAD:
-		return 8;
-	case SPI_NOR_NORMAL:
-		return 0;
-	}
-	return 0;
-}
-
-/*
  * Write status register 1 byte
  * Returns negative if error occurred.
  */
@@ -1367,8 +1349,204 @@ static int spi_nor_check(struct spi_nor *nor)
 	return 0;
 }
 
-int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
+static inline void spi_nor_set_read_settings(struct spi_nor_read *read,
+					     u8 num_mode_clocks,
+					     u8 num_wait_states,
+					     u8 opcode)
+{
+	read->num_mode_clocks = num_mode_clocks;
+	read->num_wait_states = num_wait_states;
+	read->opcode = opcode;
+}
+
+static inline void spi_nor_set_erase_settings(struct spi_nor_erase_type *erase,
+					      u8 size, u8 opcode)
+{
+	erase->size = size;
+	erase->opcode = opcode;
+}
+
+static int spi_nor_init_params(struct spi_nor *nor,
+			       const struct flash_info *info,
+			       struct spi_nor_basic_flash_parameter *params)
+{
+	// TODO: parse SFDP table
+
+	/* If SFDP tables are not available, use legacy settings. */
+	memset(params, 0, sizeof(*params));
+
+	/* (Fast) Read settings. */
+	params->rd_modes = SNOR_MODE_1_1_1;
+	spi_nor_set_read_settings(&params->reads[SNOR_PINDEX_1_1_1],
+				  0, 8, SPINOR_OP_READ_FAST);
+	if (!(info->flags & SPI_NOR_NO_FR)) {
+		params->rd_modes |= SNOR_MODE_SLOW;
+		spi_nor_set_read_settings(&params->reads[SNOR_PINDEX_SLOW],
+					  0, 0, SPINOR_OP_READ);
+	}
+	if (info->flags & SPI_NOR_DUAL_READ) {
+		params->rd_modes |= SNOR_MODE_1_1_2;
+		spi_nor_set_read_settings(&params->reads[SNOR_PINDEX_1_1_2],
+					  0, 8, SPINOR_OP_READ_1_1_2);
+	}
+	if (info->flags & SPI_NOR_QUAD_READ) {
+		params->rd_modes|= SNOR_MODE_1_1_4;
+		spi_nor_set_read_settings(&params->reads[SNOR_PINDEX_1_1_4],
+					  0, 8, SPINOR_OP_READ_1_1_4);
+	}
+
+	/* Page Program settings. */
+	params->wr_modes = SNOR_MODE_1_1_1;
+	params->page_programs[SNOR_PINDEX_1_1_1] = SPINOR_OP_PP;
+
+	/* Sector Erase settings. */
+	spi_nor_set_erase_settings(&params->erase_types[0],
+				   SNOR_ERASE_64K, SPINOR_OP_SE);
+	if (info->flags & SECT_4K)
+		spi_nor_set_erase_settings(&params->erase_types[1],
+					   SNOR_ERASE_4K, SPINOR_OP_BE_4K);
+	else if (info->flags & SECT_4K_PMC)
+		spi_nor_set_erase_settings(&params->erase_types[1],
+					   SNOR_ERASE_4K, SPINOR_OP_BE_4K_PMC);
+
+	/* Select the procedure to set the Quad Enable bit. */
+	if (params->rd_modes & (SNOR_MODE_1_1_4 |
+				SNOR_MODE_1_4_4 |
+				SNOR_MODE_4_4_4)) {
+		switch (JEDEC_MFR(info)) {
+		case SNOR_MFR_MACRONIX:
+			params->enable_quad_io = macronix_quad_enable;
+			break;
+
+		case SNOR_MFR_MICRON:
+			break;
+
+		default:
+			params->enable_quad_io = spansion_quad_enable;
+			break;
+		}
+	}
+
+	return 0;
+}
+
+static int spi_nor_pindex2proto(int pindex, enum spi_nor_protocol *proto)
+{
+	enum spi_nor_protocol_width pwidth;
+	enum spi_nor_protocol_class pclass;
+	uint8_t width;
+
+	if (pindex < 0)
+		return -EINVAL;
+
+	pwidth = (enum spi_nor_protocol_width)(pindex / SNOR_PCLASS_MAX);
+	pclass = (enum spi_nor_protocol_class)(pindex % SNOR_PCLASS_MAX);
+
+	width = (1 << pwidth) & 0xf;
+	if (!width)
+		return -EINVAL;
+
+	switch (pclass) {
+	case SNOR_PCLASS_1_1_N:
+		*proto = SNOR_PROTO(1, 1, width);
+		break;
+
+	case SNOR_PCLASS_1_N_N:
+		*proto = SNOR_PROTO(1, width, width);
+		break;
+
+	case SNOR_PCLASS_N_N_N:
+		*proto = SNOR_PROTO(width, width, width);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info,
+			 const struct spi_nor_basic_flash_parameter *params,
+			 const struct spi_nor_modes *modes)
 {
+	bool enable_quad_io;
+	u32 rd_modes, wr_modes, mask;
+	const struct spi_nor_erase_type *erase_type = NULL;
+	const struct spi_nor_read *read;
+	int rd_pindex, wr_pindex, i, err = 0;
+	u8 erase_size = SNOR_ERASE_64K;
+
+	/* 2-2-2 or 4-4-4 modes are not supported yet. */
+	mask = (SNOR_MODE_2_2_2 | SNOR_MODE_4_4_4);
+	rd_modes = modes->rd_modes & ~mask;
+	wr_modes = modes->wr_modes & ~mask;
+
+	/* Setup read operation. */
+	rd_pindex = fls(params->rd_modes & rd_modes) - 1;
+	if (spi_nor_pindex2proto(rd_pindex, &nor->read_proto)) {
+		dev_err(nor->dev, "invalid (fast) read\n");
+		return -EINVAL;
+	}
+	read = &params->reads[rd_pindex];
+	nor->read_opcode = read->opcode;
+	nor->read_dummy = read->num_mode_clocks + read->num_wait_states;
+
+	/* Set page program op code and protocol. */
+	wr_pindex = fls(params->wr_modes & wr_modes) - 1;
+	if (spi_nor_pindex2proto(wr_pindex, &nor->write_proto)) {
+		dev_err(nor->dev, "invalid page program\n");
+		return -EINVAL;
+	}
+	nor->program_opcode = params->page_programs[wr_pindex];
+
+	/* Set sector erase op code and size. */
+	erase_type = &params->erase_types[0];
+#ifdef CONFIG_MTD_SPI_NOR_USE_4K_SECTORS
+	erase_size = SNOR_ERASE_4K;
+#endif
+	for (i = 0; i < SNOR_MAX_ERASE_TYPES; ++i) {
+		if (params->erase_types[i].size == erase_size) {
+			erase_type = &params->erase_types[i];
+			break;
+		}
+	}
+	nor->erase_opcode = erase_type->opcode;
+	nor->mtd.erasesize = (1 << erase_type->size);
+
+
+	enable_quad_io = (SNOR_PROTO_DATA_FROM_PROTO(nor->read_proto) == 4 ||
+			  SNOR_PROTO_DATA_FROM_PROTO(nor->write_proto) == 4);
+
+	/* Enable Quad I/O if needed. */
+	if (enable_quad_io && params->enable_quad_io) {
+		err = params->enable_quad_io(nor);
+		if (err) {
+			dev_err(nor->dev,
+				"failed to enable the Quad I/O mode\n");
+			return err;
+		}
+	}
+
+	dev_dbg(nor->dev,
+		"(Fast) Read:  opcode=%02Xh, protocol=%03x, mode=%u, wait=%u\n",
+		nor->read_opcode, nor->read_proto,
+		read->num_mode_clocks, read->num_wait_states);
+	dev_dbg(nor->dev,
+		"Page Program: opcode=%02Xh, protocol=%03x\n",
+		nor->program_opcode, nor->write_proto);
+	dev_dbg(nor->dev,
+		"Sector Erase: opcode=%02Xh, protocol=%03x, sector size=%u\n",
+		nor->erase_opcode, nor->reg_proto, (u32)nor->mtd.erasesize);
+
+	return 0;
+}
+
+int spi_nor_scan(struct spi_nor *nor, const char *name,
+		 const struct spi_nor_modes *modes)
+{
+	struct spi_nor_basic_flash_parameter params;
+	struct spi_nor_modes fixed_modes = *modes;
 	const struct flash_info *info = NULL;
 	struct device *dev = nor->dev;
 	struct mtd_info *mtd = &nor->mtd;
@@ -1380,6 +1558,11 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	if (ret)
 		return ret;
 
+	/* Reset SPI protocol for all commands */
+	nor->reg_proto = SNOR_PROTO_1_1_1;
+	nor->read_proto = SNOR_PROTO_1_1_1;
+	nor->write_proto = SNOR_PROTO_1_1_1;
+
 	if (name)
 		info = spi_nor_match_id(name);
 	/* Try to auto-detect if chip name wasn't specified or not found */
@@ -1412,6 +1595,11 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 		}
 	}
 
+	/* Parse the Serial Flash Discoverable Parameters table */
+	ret = spi_nor_init_params(nor, info, &params);
+	if (ret)
+		return ret;
+
 	mutex_init(&nor->lock);
 
 	/*
@@ -1488,51 +1676,31 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	if (np) {
 		/* If we were instantiated by DT, use it */
 		if (of_property_read_bool(np, "m25p,fast-read"))
-			nor->flash_read = SPI_NOR_FAST;
+			fixed_modes.rd_modes |= SNOR_MODE_1_1_1;
 		else
-			nor->flash_read = SPI_NOR_NORMAL;
+			fixed_modes.rd_modes &= ~SNOR_MODE_1_1_1;
 	} else {
 		/* If we weren't instantiated by DT, default to fast-read */
-		nor->flash_read = SPI_NOR_FAST;
+		fixed_modes.rd_modes |= SNOR_MODE_1_1_1;
 	}
 
 	/* Some devices cannot do fast-read, no matter what DT tells us */
 	if (info->flags & SPI_NOR_NO_FR)
-		nor->flash_read = SPI_NOR_NORMAL;
-
-	/* Quad/Dual-read mode takes precedence over fast/normal */
-	if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
-		ret = set_quad_mode(nor, info);
-		if (ret) {
-			dev_err(dev, "quad mode not supported\n");
-			return ret;
-		}
-		nor->flash_read = SPI_NOR_QUAD;
-	} else if (mode == SPI_NOR_DUAL && info->flags & SPI_NOR_DUAL_READ) {
-		nor->flash_read = SPI_NOR_DUAL;
-	}
-
-	/* Default commands */
-	switch (nor->flash_read) {
-	case SPI_NOR_QUAD:
-		nor->read_opcode = SPINOR_OP_READ_1_1_4;
-		break;
-	case SPI_NOR_DUAL:
-		nor->read_opcode = SPINOR_OP_READ_1_1_2;
-		break;
-	case SPI_NOR_FAST:
-		nor->read_opcode = SPINOR_OP_READ_FAST;
-		break;
-	case SPI_NOR_NORMAL:
-		nor->read_opcode = SPINOR_OP_READ;
-		break;
-	default:
-		dev_err(dev, "No Read opcode defined\n");
-		return -EINVAL;
-	}
+		fixed_modes.rd_modes &= ~SNOR_MODE_1_1_1;
 
 	nor->program_opcode = SPINOR_OP_PP;
 
+	/*
+	 * Configure the SPI memory:
+	 * - select op codes for (Fast) Read, Page Program and Sector Erase.
+	 * - set the number of dummy cycles (mode cycles + wait states).
+	 * - set the SPI protocols for register and memory accesses.
+	 * - set the Quad Enable bit if needed (required by SPI x-y-4 protos).
+	 */
+	ret = spi_nor_setup(nor, info, &params, &fixed_modes);
+	if (ret)
+		return ret;
+
 	if (info->addr_width)
 		nor->addr_width = info->addr_width;
 	else if (mtd->size > 0x1000000) {
@@ -1553,8 +1721,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 		return -EINVAL;
 	}
 
-	nor->read_dummy = spi_nor_read_dummy_cycles(nor);
-
 	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 8b02fd7864d0..730f33dce1ea 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -110,11 +110,124 @@
 /* Configuration Register bits. */
 #define CR_QUAD_EN_SPAN		BIT(1)	/* Spansion Quad I/O */
 
-enum read_mode {
-	SPI_NOR_NORMAL = 0,
-	SPI_NOR_FAST,
-	SPI_NOR_DUAL,
-	SPI_NOR_QUAD,
+
+/* Supported SPI protocols */
+enum spi_nor_protocol_class {
+	SNOR_PCLASS_1_1_N,
+	SNOR_PCLASS_1_N_N,
+	SNOR_PCLASS_N_N_N,
+
+	SNOR_PCLASS_MAX
+};
+
+enum spi_nor_protocol_width {
+	SNOR_PWIDTH_1,
+	SNOR_PWIDTH_2,
+	SNOR_PWIDTH_4,
+	SNOR_PWIDTH_8,
+};
+
+/* The encoding is chosen so the higher index the higher priority */
+#define SNOR_PINDEX(pwidth, pclass) \
+	((pwidth) * SNOR_PCLASS_MAX + (pclass))
+enum spi_nor_protocol_index {
+	SNOR_PINDEX_SLOW  = SNOR_PINDEX(SNOR_PWIDTH_1, SNOR_PCLASS_1_1_N),
+	/* Little trick to make the difference between Read and Fast Read commands. */
+	SNOR_PINDEX_1_1_1 = SNOR_PINDEX(SNOR_PWIDTH_1, SNOR_PCLASS_1_N_N),
+	SNOR_PINDEX_1_1_2 = SNOR_PINDEX(SNOR_PWIDTH_2, SNOR_PCLASS_1_1_N),
+	SNOR_PINDEX_1_2_2 = SNOR_PINDEX(SNOR_PWIDTH_2, SNOR_PCLASS_1_N_N),
+	SNOR_PINDEX_2_2_2 = SNOR_PINDEX(SNOR_PWIDTH_2, SNOR_PCLASS_N_N_N),
+	SNOR_PINDEX_1_1_4 = SNOR_PINDEX(SNOR_PWIDTH_4, SNOR_PCLASS_1_1_N),
+	SNOR_PINDEX_1_4_4 = SNOR_PINDEX(SNOR_PWIDTH_4, SNOR_PCLASS_1_N_N),
+	SNOR_PINDEX_4_4_4 = SNOR_PINDEX(SNOR_PWIDTH_4, SNOR_PCLASS_N_N_N),
+
+	SNOR_PINDEX_MAX
+};
+
+#define SNOR_MODE_SLOW		BIT(SNOR_PINDEX_SLOW)
+#define SNOR_MODE_1_1_1		BIT(SNOR_PINDEX_1_1_1)
+#define SNOR_MODE_1_1_2		BIT(SNOR_PINDEX_1_1_2)
+#define SNOR_MODE_1_2_2		BIT(SNOR_PINDEX_1_2_2)
+#define SNOR_MODE_2_2_2		BIT(SNOR_PINDEX_2_2_2)
+#define SNOR_MODE_1_1_4		BIT(SNOR_PINDEX_1_1_4)
+#define SNOR_MODE_1_4_4		BIT(SNOR_PINDEX_1_4_4)
+#define SNOR_MODE_4_4_4		BIT(SNOR_PINDEX_4_4_4)
+
+struct spi_nor_modes {
+	u32	rd_modes;	/* supported SPI modes for (Fast) Read */
+	u32	wr_modes;	/* supported SPI modes for Page Program */
+};
+
+
+struct spi_nor_read {
+	u8	num_wait_states:5;
+	u8	num_mode_clocks:3;
+	u8	opcode;
+};
+
+struct spi_nor_erase_type {
+	u8	size;	/* specifies 'N' so erase size = 2^N */
+	u8	opcode;
+};
+
+#define SNOR_ERASE_64K		0x10
+#define SNOR_ERASE_32K		0x0f
+#define SNOR_ERASE_4K		0x0c
+
+struct spi_nor;
+
+#define SNOR_MAX_ERASE_TYPES	4
+
+struct spi_nor_basic_flash_parameter {
+	/* Fast Read settings */
+	u32				rd_modes;
+	struct spi_nor_read		reads[SNOR_PINDEX_MAX];
+
+	/* Page Program settings */
+	u32				wr_modes;
+	u8				page_programs[SNOR_PINDEX_MAX];
+
+	/* Sector Erase settings */
+	struct spi_nor_erase_type	erase_types[SNOR_MAX_ERASE_TYPES];
+
+	int (*enable_quad_io)(struct spi_nor *nor);
+};
+
+
+#define SNOR_PROTO_CODE_OFF	8
+#define SNOR_PROTO_CODE_MASK	GENMASK(11, 8)
+#define SNOR_PROTO_CODE_TO_PROTO(code) \
+	(((code) << SNOR_PROTO_CODE_OFF) & SNOR_PROTO_CODE_MASK)
+#define SNOR_PROTO_CODE_FROM_PROTO(proto) \
+	((((u32)(proto)) & SNOR_PROTO_CODE_MASK) >> SNOR_PROTO_CODE_OFF)
+
+#define SNOR_PROTO_ADDR_OFF	4
+#define SNOR_PROTO_ADDR_MASK	GENMASK(7, 4)
+#define SNOR_PROTO_ADDR_TO_PROTO(addr) \
+	(((addr) << SNOR_PROTO_ADDR_OFF) & SNOR_PROTO_ADDR_MASK)
+#define SNOR_PROTO_ADDR_FROM_PROTO(proto) \
+	((((u32)(proto)) & SNOR_PROTO_ADDR_MASK) >> SNOR_PROTO_ADDR_OFF)
+
+#define SNOR_PROTO_DATA_OFF	0
+#define SNOR_PROTO_DATA_MASK	GENMASK(3, 0)
+#define SNOR_PROTO_DATA_TO_PROTO(data) \
+	(((data) << SNOR_PROTO_DATA_OFF) & SNOR_PROTO_DATA_MASK)
+#define SNOR_PROTO_DATA_FROM_PROTO(proto) \
+	((((u32)(proto)) & SNOR_PROTO_DATA_MASK) >> SNOR_PROTO_DATA_OFF)
+
+#define SNOR_PROTO(code, addr, data)	  \
+	(SNOR_PROTO_CODE_TO_PROTO(code) |   \
+	 SNOR_PROTO_ADDR_TO_PROTO(addr) | \
+	 SNOR_PROTO_DATA_TO_PROTO(data))
+
+enum spi_nor_protocol {
+	SNOR_PROTO_1_1_1 = SNOR_PROTO(1, 1, 1),	/* SPI */
+	SNOR_PROTO_1_1_2 = SNOR_PROTO(1, 1, 2),	/* Dual Output */
+	SNOR_PROTO_1_1_4 = SNOR_PROTO(1, 1, 4),	/* Quad Output */
+	SNOR_PROTO_1_2_2 = SNOR_PROTO(1, 2, 2),	/* Dual IO */
+	SNOR_PROTO_1_4_4 = SNOR_PROTO(1, 4, 4),	/* Quad IO */
+	SNOR_PROTO_2_2_2 = SNOR_PROTO(2, 2, 2),	/* Dual Command */
+	SNOR_PROTO_4_4_4 = SNOR_PROTO(4, 4, 4),	/* Quad Command */
 };
 
 #define SPI_NOR_MAX_CMD_SIZE	8
@@ -142,9 +255,11 @@ enum spi_nor_option_flags {
  * @read_opcode:	the read opcode
  * @read_dummy:		the dummy needed by the read operation
  * @program_opcode:	the program opcode
- * @flash_read:		the mode of the read
  * @sst_write_second:	used by the SST write operation
  * @flags:		flag options for the current SPI-NOR (SNOR_F_*)
+ * @read_proto:		the SPI protocol used by read operations
+ * @write_proto:	the SPI protocol used by write operations
+ * @reg_proto		the SPI protocol used by read_reg/write_reg operations
  * @cmd_buf:		used by the write_reg
  * @prepare:		[OPTIONAL] do some preparations for the
  *			read/write/erase/lock/unlock operations
@@ -173,7 +288,9 @@ struct spi_nor {
 	u8			read_opcode;
 	u8			read_dummy;
 	u8			program_opcode;
-	enum read_mode		flash_read;
+	enum spi_nor_protocol	read_proto;
+	enum spi_nor_protocol	write_proto;
+	enum spi_nor_protocol	reg_proto;
 	bool			sst_write_second;
 	u32			flags;
 	u8			cmd_buf[SPI_NOR_MAX_CMD_SIZE];
@@ -211,7 +328,7 @@ static inline struct device_node *spi_nor_get_flash_node(struct spi_nor *nor)
  * spi_nor_scan() - scan the SPI NOR
  * @nor:	the spi_nor structure
  * @name:	the chip type name
- * @mode:	the read mode supported by the driver
+ * @modes:	the SPI modes supported by the controller driver
  *
  * The drivers can use this fuction to scan the SPI NOR.
  * In the scanning, it will try to get all the necessary information to
@@ -221,6 +338,7 @@ static inline struct device_node *spi_nor_get_flash_node(struct spi_nor *nor)
  *
  * Return: 0 for success, others for failure.
  */
-int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode);
+int spi_nor_scan(struct spi_nor *nor, const char *name,
+		 const struct spi_nor_modes *modes);
 
 #endif
-- 
1.8.2.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 6/9] mtd: spi-nor: remove unused set_quad_mode() function
  2016-06-20 16:49 [PATCH 0/9] mtd: spi-nor: parse SFDP tables as defined by JESD216B Cyrille Pitchen
                   ` (4 preceding siblings ...)
  2016-06-20 16:50 ` [PATCH 5/9] mtd: spi-nor: add support of SPI protocols like SPI 1-2-2 and SPI 1-4-4 Cyrille Pitchen
@ 2016-06-20 16:50 ` Cyrille Pitchen
  2016-06-20 16:50 ` [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands Cyrille Pitchen
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 26+ messages in thread
From: Cyrille Pitchen @ 2016-06-20 16:50 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

The set_quad_mode() function is no longer used, so we remove it.

This patch is not squashed into the previous patch on purpose for
readiness issue and to ease the review process of the whole series.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 29ba0e665fac..43f104fc7fac 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1314,30 +1314,6 @@ static int spansion_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
-static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
-{
-	int status;
-
-	switch (JEDEC_MFR(info)) {
-	case SNOR_MFR_MACRONIX:
-		status = macronix_quad_enable(nor);
-		if (status) {
-			dev_err(nor->dev, "Macronix quad-read not enabled\n");
-			return -EINVAL;
-		}
-		return status;
-	case SNOR_MFR_MICRON:
-		return 0;
-	default:
-		status = spansion_quad_enable(nor);
-		if (status) {
-			dev_err(nor->dev, "Spansion quad-read not enabled\n");
-			return -EINVAL;
-		}
-		return status;
-	}
-}
-
 static int spi_nor_check(struct spi_nor *nor)
 {
 	if (!nor->dev || !nor->read || !nor->write ||
-- 
1.8.2.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands
  2016-06-20 16:49 [PATCH 0/9] mtd: spi-nor: parse SFDP tables as defined by JESD216B Cyrille Pitchen
                   ` (5 preceding siblings ...)
  2016-06-20 16:50 ` [PATCH 6/9] mtd: spi-nor: remove unused set_quad_mode() function Cyrille Pitchen
@ 2016-06-20 16:50 ` Cyrille Pitchen
  2016-06-23 20:35   ` Michal Suchanek
  2016-06-20 16:50 ` [PATCH 8/9] mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables Cyrille Pitchen
  2016-06-20 16:50 ` [PATCH 9/9] mtd: spi-nor: parse SFDP 4-byte Address Instruction Table Cyrille Pitchen
  8 siblings, 1 reply; 26+ messages in thread
From: Cyrille Pitchen @ 2016-06-20 16:50 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

Before this patch, m25p80_read() supported few SPI protocols:
- regular SPI 1-1-1
- SPI Dual Output 1-1-2
- SPI Quad Output 1-1-4
On the other hand, all other m25p80_*() hooks only supported SPI 1-1-1.

This patch adds support to all currently existing SPI protocols to
cover as many protocols as possible.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/mtd/devices/m25p80.c | 193 +++++++++++++++++++++++++++++++++----------
 1 file changed, 149 insertions(+), 44 deletions(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index f0a55c01406b..38778eac6c21 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -27,22 +27,64 @@
 #include <linux/spi/flash.h>
 #include <linux/mtd/spi-nor.h>
 
-#define	MAX_CMD_SIZE		6
+#define	MAX_CMD_SIZE		16
 struct m25p {
 	struct spi_device	*spi;
 	struct spi_nor		spi_nor;
 	u8			command[MAX_CMD_SIZE];
 };
 
+static inline int m25p80_proto2nbits(enum spi_nor_protocol proto,
+				     unsigned *code_nbits,
+				     unsigned *addr_nbits,
+				     unsigned *data_nbits)
+{
+	if (code_nbits)
+		*code_nbits = SNOR_PROTO_CODE_FROM_PROTO(proto);
+	if (addr_nbits)
+		*addr_nbits = SNOR_PROTO_ADDR_FROM_PROTO(proto);
+	if (data_nbits)
+		*data_nbits = SNOR_PROTO_DATA_FROM_PROTO(proto);
+
+	return 0;
+}
+
 static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
 {
 	struct m25p *flash = nor->priv;
 	struct spi_device *spi = flash->spi;
+	unsigned code_nbits, data_nbits;
+	struct spi_transfer xfers[2];
 	int ret;
 
-	ret = spi_write_then_read(spi, &code, 1, val, len);
+	/* Check the total length of command op code and data. */
+	if (len + 1 > MAX_CMD_SIZE)
+		return -EINVAL;
+
+	/* Get transfer protocols (addr_nbits is not relevant here). */
+	ret = m25p80_proto2nbits(nor->reg_proto,
+				 &code_nbits, NULL, &data_nbits);
+	if (ret < 0)
+		return ret;
+
+	/* Set up transfers. */
+	memset(xfers, 0, sizeof(xfers));
+
+	flash->command[0] = code;
+	xfers[0].len = 1;
+	xfers[0].tx_buf = flash->command;
+	xfers[0].tx_nbits = code_nbits;
+
+	xfers[1].len = len;
+	xfers[1].rx_buf = &flash->command[1];
+	xfers[1].rx_nbits = data_nbits;
+
+	/* Process command. */
+	ret = spi_sync_transfer(spi, xfers, 2);
 	if (ret < 0)
 		dev_err(&spi->dev, "error %d reading %x\n", ret, code);
+	else
+		memcpy(val, &flash->command[1], len);
 
 	return ret;
 }
@@ -65,12 +107,42 @@ static int m25p80_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 {
 	struct m25p *flash = nor->priv;
 	struct spi_device *spi = flash->spi;
+	unsigned code_nbits, data_nbits, num_xfers = 1;
+	struct spi_transfer xfers[2];
+	int ret;
+
+	/* Check the total length of command op code and data. */
+	if (buf && (len + 1 > MAX_CMD_SIZE))
+		return -EINVAL;
+
+	/* Get transfer protocols (addr_nbits is not relevant here). */
+	ret = m25p80_proto2nbits(nor->reg_proto,
+				 &code_nbits, NULL, &data_nbits);
+	if (ret < 0)
+		return ret;
+
+	/* Set up transfer(s). */
+	memset(xfers, 0, sizeof(xfers));
 
 	flash->command[0] = opcode;
-	if (buf)
+	xfers[0].len = 1;
+	xfers[0].tx_buf = flash->command;
+	xfers[0].tx_nbits = code_nbits;
+
+	if (buf) {
 		memcpy(&flash->command[1], buf, len);
+		if (data_nbits == code_nbits) {
+			xfers[0].len += len;
+		} else {
+			xfers[1].len = len;
+			xfers[1].tx_buf = &flash->command[1];
+			xfers[1].tx_nbits = data_nbits;
+			num_xfers++;
+		}
+	}
 
-	return spi_write(spi, flash->command, len + 1);
+	/* Process command. */
+	return spi_sync_transfer(spi, xfers, num_xfers);
 }
 
 static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
@@ -78,27 +150,48 @@ static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
 {
 	struct m25p *flash = nor->priv;
 	struct spi_device *spi = flash->spi;
-	struct spi_transfer t[2] = {};
+	unsigned code_nbits, addr_nbits, data_nbits, num_xfers = 1;
+	struct spi_transfer xfers[3];
 	struct spi_message m;
 	int cmd_sz = m25p_cmdsz(nor);
 	ssize_t ret;
 
-	spi_message_init(&m);
-
 	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
 		cmd_sz = 1;
 
-	flash->command[0] = nor->program_opcode;
-	m25p_addr2cmd(nor, to, flash->command);
+	/* Get transfer protocols. */
+	ret = m25p80_proto2nbits(nor->write_proto,
+				 &code_nbits, &addr_nbits, &data_nbits);
+	if (ret < 0)
+		return ret;
 
-	t[0].tx_buf = flash->command;
-	t[0].len = cmd_sz;
-	spi_message_add_tail(&t[0], &m);
+	/* Set up transfers. */
+	memset(xfers, 0, sizeof(xfers));
+
+	flash->command[0] = nor->program_opcode;
+	xfers[0].len = 1;
+	xfers[0].tx_buf = flash->command;
+	xfers[0].tx_nbits = code_nbits;
+
+	if (cmd_sz > 1) {
+		m25p_addr2cmd(nor, to, flash->command);
+		if (addr_nbits == code_nbits) {
+			xfers[0].len += nor->addr_width;
+		} else {
+			xfers[1].len = nor->addr_width;
+			xfers[1].tx_buf = &flash->command[1];
+			xfers[1].tx_nbits = addr_nbits;
+			num_xfers++;
+		}
+	}
 
-	t[1].tx_buf = buf;
-	t[1].len = len;
-	spi_message_add_tail(&t[1], &m);
+	xfers[num_xfers].len = len;
+	xfers[num_xfers].tx_buf = buf;
+	xfers[num_xfers].tx_nbits = data_nbits;
+	num_xfers++;
 
+	/* Process command. */
+	spi_message_init_with_transfers(&m, xfers, num_xfers);
 	ret = spi_sync(spi, &m);
 	if (ret)
 		return ret;
@@ -109,18 +202,6 @@ static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
 	return ret;
 }
 
-static inline unsigned int m25p80_rx_nbits(struct spi_nor *nor)
-{
-	switch (SNOR_PROTO_DATA_FROM_PROTO(nor->read_proto)) {
-	case 2:
-		return 2;
-	case 4:
-		return 4;
-	default:
-		return 0;
-	}
-}
-
 /*
  * Read an address range from the nor chip.  The address range
  * may be any size provided it is within the physical boundaries.
@@ -130,14 +211,22 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
 {
 	struct m25p *flash = nor->priv;
 	struct spi_device *spi = flash->spi;
-	struct spi_transfer t[2];
-	struct spi_message m;
+	unsigned code_nbits, addr_nbits, data_nbits, num_xfers = 1;
 	unsigned int dummy = nor->read_dummy;
 	ssize_t ret;
+	struct spi_transfer xfers[3];
+	struct spi_message m;
+
+	/* Get transfer protocols. */
+	ret = m25p80_proto2nbits(nor->read_proto,
+				 &code_nbits, &addr_nbits, &data_nbits);
+	if (ret < 0)
+		return ret;
 
 	/* convert the dummy cycles to the number of bytes */
-	dummy /= 8;
+	dummy = (dummy * addr_nbits) / 8;
 
+	/* Use the SPI flash API if supported. */
 	if (spi_flash_read_supported(spi)) {
 		struct spi_flash_read_message msg;
 
@@ -149,10 +238,9 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
 		msg.read_opcode = nor->read_opcode;
 		msg.addr_width = nor->addr_width;
 		msg.dummy_bytes = dummy;
-		/* TODO: Support other combinations */
-		msg.opcode_nbits = SPI_NBITS_SINGLE;
-		msg.addr_nbits = SPI_NBITS_SINGLE;
-		msg.data_nbits = m25p80_rx_nbits(nor);
+		msg.opcode_nbits = code_nbits;
+		msg.addr_nbits = addr_nbits;
+		msg.data_nbits = data_nbits;
 
 		ret = spi_flash_read(spi, &msg);
 		if (ret < 0)
@@ -160,21 +248,38 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
 		return msg.retlen;
 	}
 
-	spi_message_init(&m);
-	memset(t, 0, (sizeof t));
+	/* Set up transfers. */
+	memset(xfers, 0, sizeof(xfers));
 
 	flash->command[0] = nor->read_opcode;
-	m25p_addr2cmd(nor, from, flash->command);
+	xfers[0].len = 1;
+	xfers[0].tx_buf = flash->command;
+	xfers[0].tx_nbits = code_nbits;
 
-	t[0].tx_buf = flash->command;
-	t[0].len = m25p_cmdsz(nor) + dummy;
-	spi_message_add_tail(&t[0], &m);
+	m25p_addr2cmd(nor, from, flash->command);
+	/*
+	 * Clear all dummy/mode cycle bits to avoid sending some manufacturer
+	 * specific pattern, which might make the memory enter its Continuous
+	 * Read mode by mistake.
+	 */
+	memset(flash->command + 1 + nor->addr_width, 0, dummy);
+
+	if (addr_nbits == code_nbits) {
+		xfers[0].len += nor->addr_width + dummy;
+	} else {
+		xfers[1].len = nor->addr_width + dummy;
+		xfers[1].tx_buf = &flash->command[1];
+		xfers[1].tx_nbits = addr_nbits;
+		num_xfers++;
+	}
 
-	t[1].rx_buf = buf;
-	t[1].rx_nbits = m25p80_rx_nbits(nor);
-	t[1].len = min(len, spi_max_transfer_size(spi));
-	spi_message_add_tail(&t[1], &m);
+	xfers[num_xfers].len = len;
+	xfers[num_xfers].rx_buf = buf;
+	xfers[num_xfers].rx_nbits = data_nbits;
+	num_xfers++;
 
+	/* Process command. */
+	spi_message_init_with_transfers(&m, xfers, num_xfers);
 	ret = spi_sync(spi, &m);
 	if (ret)
 		return ret;
-- 
1.8.2.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 8/9] mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables
  2016-06-20 16:49 [PATCH 0/9] mtd: spi-nor: parse SFDP tables as defined by JESD216B Cyrille Pitchen
                   ` (6 preceding siblings ...)
  2016-06-20 16:50 ` [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands Cyrille Pitchen
@ 2016-06-20 16:50 ` Cyrille Pitchen
  2016-06-20 16:50 ` [PATCH 9/9] mtd: spi-nor: parse SFDP 4-byte Address Instruction Table Cyrille Pitchen
  8 siblings, 0 replies; 26+ messages in thread
From: Cyrille Pitchen @ 2016-06-20 16:50 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

This patch adds support the the JESD216B standard and parse the SFDP
tables to dynamically initialize the spi_nor_basic_flash_parameter
structure.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 423 +++++++++++++++++++++++++++++++++++++++++-
 include/linux/mtd/spi-nor.h   |   1 +
 2 files changed, 423 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 43f104fc7fac..2d53c65f5795 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -17,6 +17,7 @@
 #include <linux/mutex.h>
 #include <linux/math64.h>
 #include <linux/sizes.h>
+#include <linux/slab.h>
 
 #include <linux/mtd/mtd.h>
 #include <linux/of_platform.h>
@@ -79,6 +80,7 @@ struct flash_info {
 					 * Use dedicated 4byte address op codes
 					 * to support memory size above 128Mib.
 					 */
+#define SPI_NOR_SKIP_SFDP	BIT(11) /* Skip read of SFDP tables */
 };
 
 #define JEDEC_MFR(info)	((info)->id[0])
@@ -1314,6 +1316,53 @@ static int spansion_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
+static int spansion_new_quad_enable(struct spi_nor *nor)
+{
+	u8 sr_cr[2];
+	int ret;
+
+	/* Check current Quad Enable bit value. */
+	ret = read_cr(nor);
+	if (ret < 0) {
+		dev_err(nor->dev,
+			"error while reading configuration register\n");
+		return -EINVAL;
+	}
+	sr_cr[1] = ret;
+	if (sr_cr[1] & CR_QUAD_EN_SPAN)
+		return 0;
+
+	dev_info(nor->dev, "setting Spansion Quad Enable (non-volatile) bit\n");
+
+	/* Keep the current value of the Status Register. */
+	ret = read_sr(nor);
+	if (ret < 0) {
+		dev_err(nor->dev,
+			"error while reading status register\n");
+		return -EINVAL;
+	}
+	sr_cr[0] = ret;
+	sr_cr[1] |= CR_QUAD_EN_SPAN;
+
+	write_enable(nor);
+
+	ret = nor->write_reg(nor, SPINOR_OP_WRSR, sr_cr, 2);
+	if (ret < 0) {
+		dev_err(nor->dev,
+			"error while writing configuration register\n");
+		return -EINVAL;
+	}
+
+	/* read back and check it */
+	ret = read_cr(nor);
+	if (!(ret > 0 && (ret & CR_QUAD_EN_SPAN))) {
+		dev_err(nor->dev, "Spansion Quad bit not set\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int spi_nor_check(struct spi_nor *nor)
 {
 	if (!nor->dev || !nor->read || !nor->write ||
@@ -1342,11 +1391,382 @@ static inline void spi_nor_set_erase_settings(struct spi_nor_erase_type *erase,
 	erase->opcode = opcode;
 }
 
+static int spi_nor_read_sfdp(struct spi_nor *nor, u32 addr, size_t len, void *buf)
+{
+	u8 addr_width, read_opcode, read_dummy;
+	int ret;
+
+	read_opcode = nor->read_opcode;
+	addr_width = nor->addr_width;
+	read_dummy = nor->read_dummy;
+
+	nor->read_opcode = SPINOR_OP_RDSFDP;
+	nor->addr_width = 3;
+	nor->read_dummy = 8;
+
+	ret = nor->read(nor, addr, len, (u8 *)buf);
+
+	nor->read_opcode = read_opcode;
+	nor->addr_width = addr_width;
+	nor->read_dummy = read_dummy;
+
+	return (ret < 0) ? ret : 0;
+}
+
+struct sfdp_parameter_header {
+	u8		id_lsb;
+	u8		minor;
+	u8		major;
+	u8		length; /* in double words */
+	u8		parameter_table_pointer[3]; /* byte address */
+	u8		id_msb;
+};
+
+#define SFDP_PARAM_HEADER_ID(p)	((u16)(((p)->id_msb << 8) | (p)->id_lsb))
+#define SFDP_PARAM_HEADER_PTP(p) \
+	((u32)(((p)->parameter_table_pointer[2] << 16) | \
+	       ((p)->parameter_table_pointer[1] <<  8) | \
+	       ((p)->parameter_table_pointer[0] <<  0)))
+
+
+#define SFDP_BFPT_ID		0xff00u	/* Basic Flash Parameter Table */
+
+#define SFDP_SIGNATURE		0x50444653u
+#define SFDP_JESD216_MAJOR	1
+#define SFDP_JESD216_MINOR	0
+#define SFDP_JESD216A_MINOR	5
+#define SFDP_JESD216B_MINOR	6
+
+struct sfdp_header {
+	u32		signature; /* Ox50444653 <=> "SFDP" */
+	u8		minor;
+	u8		major;
+	u8		nph; /* 0-base number of parameter headers */
+	u8		unused;
+
+	/* Basic Flash Parameter Table. */
+	struct sfdp_parameter_header	bfpt_header;
+};
+
+/* Basic Flash Parameter Table */
+
+/* 1st DWORD. */
+#define BFPT_WORD0_FAST_READ_1_1_2	BIT(16)
+#define BFPT_WORD0_ADDRESS_BYTES_MASK	GENMASK(18, 17)
+#define BFPT_WORD0_ADDRESS_BYTES_3_ONLY	(0u << 17)
+#define BFPT_WORD0_ADDRESS_BYTES_3_OR_4	(1u << 17)
+#define BFPT_WORD0_ADDRESS_BYTES_4_ONLY	(2u << 17)
+#define BFPT_WORD0_DTR			BIT(19)
+#define BFPT_WORD0_FAST_READ_1_2_2	BIT(20)
+#define BFPT_WORD0_FAST_READ_1_4_4	BIT(21)
+#define BFPT_WORD0_FAST_READ_1_1_4	BIT(22)
+
+/* 15th DWORD. */
+
+/*
+ * (from JESD216B)
+ * Quad Enable Requirements (QER):
+ * - 000b: Device does not have a QE bit. Device detects 1-1-4 and 1-4-4
+ *         reads based on instruction. DQ3/HOLD# functions are hold during
+ *         instruction pahse.
+ * - 001b: QE is bit 1 of status register 2. It is set via Write Status with
+ *         two data bytes where bit 1 of the second byte is one.
+ *         [...]
+ *         Writing only one byte to the status register has the side-effect of
+ *         clearing status register 2, including the QE bit. The 100b code is
+ *         used if writing one byte to the status register does not modify
+ *         status register 2.
+ * - 010b: QE is bit 6 of status register 1. It is set via Write Status with
+ *         one data byte where bit 6 is one.
+ *         [...]
+ * - 011b: QE is bit 7 of status register 2. It is set via Write status
+ *         register 2 instruction 3Eh with one data byte where bit 7 is one.
+ *         [...]
+ *         The status register 2 is read using instruction 3Fh.
+ * - 100b: QE is bit 1 of status register 2. It is set via Write Status with
+ *         two data bytes where bit 1 of the second byte is one.
+ *         [...]
+ *         In contrast to the 001b code, writing one byte to the status
+ *         register does not modify status register 2.
+ * - 101b: QE is bit 1 of status register 2. Status register 1 is read using
+ *         Read Status instruction 05h. Status register2 is read using
+ *         instruction 35h. QE is set via Writ Status instruction 01h with
+ *         two data bytes where bit 1 of the second byte is one.
+ *         [...]
+ */
+#define BFPT_WORD14_QER_MASK		GENMASK(22, 20)
+#define BFPT_WORD14_QER_NONE		(0 << 20) /* Micron */
+#define BFPT_WORD14_QER_SR2_BIT1_BUGGY	(1 << 20)
+#define BFPT_WORD14_QER_SR1_BIT6	(2 << 20) /* Macronix */
+#define BFPT_WORD14_QER_SR2_BIT7	(3 << 20)
+#define BFPT_WORD14_QER_SR2_BIT1_NO_RD	(4 << 20)
+#define BFPT_WORD14_QER_SR2_BIT1	(5 << 20) /* Spansion */
+
+/* JESD216B defines a Basic Flash Parameter Table of 16 words. */
+#define SFDP_BFPT_MAX_WORDS	16
+
+struct sfdp_bfpt {
+	u32	word[SFDP_BFPT_MAX_WORDS];
+};
+
+/* Fast Read settings. */
+#define BFPT_FAST_READ_WAIT_STATES(half)	((((u16)(half)) >> 0) & 0x1f)
+#define BFPT_FAST_READ_MODE_CLOCKS(half)	((((u16)(half)) >> 5) & 0x07)
+#define BFPT_FAST_READ_OP_CODE(half)		((((u16)(half)) >> 8) & 0xff)
+
+struct sfdp_read {
+	/* The protocol index of SPI x-y-z. */
+	enum spi_nor_protocol_index	pindex;
+
+	/*
+	 * The bit <wbit> in DWORD<windex> tells us whether the
+	 * Fast Read x-y-z command is supported.
+	 */
+	int				windex;
+	int				wbit;
+
+	/*
+	 * The half-word at offset <hshift> in DWORD<hindex> encodes the
+	 * op code, the number of mode clocks and the number of wait states
+	 * to be used by Fast Read x-y-z commands.
+	 */
+	int				hindex;
+	int				hshift;
+};
+
+static const struct sfdp_read sfdp_reads[] = {
+	/* Supported: DWORD0 bit 16,  Settings: DWORD3 bit[15:0] */
+	{SNOR_PINDEX_1_1_2, 0, 16, 3,  0},
+
+	/* Supported: DWORD0 bit 20,  Settings: DWORD3 bit[31:16] */
+	{SNOR_PINDEX_1_2_2, 0, 20, 3, 16},
+
+	/* Supported: DWORD4 bit  0,  Settings: DWORD5 bit[31:16] */
+	{SNOR_PINDEX_2_2_2, 4,  0, 5, 16},
+
+	/* Supported: DWORD0 bit 22,  Settings: DWORD2 bit[31:16] */
+	{SNOR_PINDEX_1_1_4, 0, 22, 2, 16},
+
+	/* Supported: DWORD0 bit 21,  Settings: DWORD2 bit[15:0] */
+	{SNOR_PINDEX_1_4_4, 0, 21, 2, 0},
+
+	/* Supported: DWORD4 bit  4,  Settings: DWORD6 bit[31:16] */
+	{SNOR_PINDEX_4_4_4, 4, 4, 6, 16},
+};
+
+
+/* Sector Erase settings. */
+#define BFPT_ERASE_SIZE(half)		((((u16)(half)) >> 0) & 0xff)
+#define BFPT_ERASE_OP_CODE(half)	((((u16)(half)) >> 8) & 0xff)
+
+struct sfdp_erase {
+	/*
+	 * The half-word at offset <hshift> in DWORD<hindex> encodes the
+	 * op code and erase sector size to be used by Sector Erase commands.
+	 */
+	int		hindex;
+	int		hshift;
+};
+
+static const struct sfdp_erase sfdp_erases[SNOR_MAX_ERASE_TYPES] = {
+	/* Erase Type 1 in DWORD7 bits[15:0] */
+	{7, 0},
+
+	/* Erase Type 2 in DWORD7 bits[31:16] */
+	{7, 16},
+
+	/* Erase Type 3 in DWORD8 bits[15:0] */
+	{8, 0},
+
+	/* Erase Type 4: in DWORD8 bits[31:16] */
+	{8, 16},
+};
+
+
+static int spi_nor_parse_bfpt(struct spi_nor *nor,
+			      const struct flash_info *info,
+			      const struct sfdp_parameter_header *bfpt_header,
+			      struct spi_nor_basic_flash_parameter *params)
+{
+	struct sfdp_bfpt bfpt;
+	size_t len;
+	int i, err;
+	u32 addr;
+	u16 half;
+
+	/* JESD216 Basic Flash Parameter Table length is at least 9 words. */
+	if (bfpt_header->length < 9)
+		return -EINVAL;
+
+	/* Read the Basic Flash Parameter Table. */
+	len = min_t(size_t, sizeof(bfpt),
+		    bfpt_header->length * sizeof(uint32_t));
+	addr = SFDP_PARAM_HEADER_PTP(bfpt_header);
+	memset(&bfpt, 0, sizeof(bfpt));
+	err = spi_nor_read_sfdp(nor,  addr, len, &bfpt);
+	if (err)
+		return err;
+
+	for (i = 0; i < SFDP_BFPT_MAX_WORDS; ++i)
+		bfpt.word[i] = le32_to_cpu(bfpt.word[i]);
+
+	memset(params, 0, sizeof(*params));
+
+	/* Fast Read settings. */
+	params->rd_modes = (SNOR_MODE_SLOW | SNOR_MODE_1_1_1);
+	spi_nor_set_read_settings(&params->reads[SNOR_PINDEX_SLOW],
+				  0, 0, SPINOR_OP_READ);
+	spi_nor_set_read_settings(&params->reads[SNOR_PINDEX_1_1_1],
+				  0, 8, SPINOR_OP_READ_FAST);
+
+	for (i = 0; i < ARRAY_SIZE(sfdp_reads); ++i) {
+		const struct sfdp_read *rd_info = &sfdp_reads[i];
+		struct spi_nor_read *read = &params->reads[rd_info->pindex];
+
+		if (!(bfpt.word[rd_info->windex] & BIT(rd_info->wbit)))
+			continue;
+
+		params->rd_modes |= BIT(rd_info->pindex);
+		half = bfpt.word[rd_info->hindex] >> rd_info->hshift;
+		read->num_mode_clocks = BFPT_FAST_READ_MODE_CLOCKS(half);
+		read->num_wait_states = BFPT_FAST_READ_WAIT_STATES(half);
+		read->opcode = BFPT_FAST_READ_OP_CODE(half);
+	}
+
+	/* Page Program settings. */
+	params->wr_modes = SNOR_MODE_1_1_1;
+	params->page_programs[SNOR_PINDEX_1_1_1] = SPINOR_OP_PP;
+
+	/* Sector Erase settings. */
+	for (i = 0; i < SNOR_MAX_ERASE_TYPES; ++i) {
+		const struct sfdp_erase *er_info = &sfdp_erases[i];
+		struct spi_nor_erase_type *erase = &params->erase_types[i];
+
+		half = bfpt.word[er_info->hindex] >> er_info->hshift;
+		erase->size = BFPT_ERASE_SIZE(half);
+		erase->opcode = BFPT_ERASE_OP_CODE(half);
+	}
+
+	/* Stop here if not JESD216 rev B or later. */
+	if (bfpt_header->length < 15)
+		return 0;
+
+	/* Enable Quad I/O. */
+	switch (bfpt.word[14] & BFPT_WORD14_QER_MASK) {
+	default:
+	case BFPT_WORD14_QER_NONE:
+		break;
+
+	case BFPT_WORD14_QER_SR2_BIT1_BUGGY:
+	case BFPT_WORD14_QER_SR2_BIT1_NO_RD:
+		params->enable_quad_io = spansion_quad_enable;
+		break;
+
+	case BFPT_WORD14_QER_SR1_BIT6:
+		params->enable_quad_io = macronix_quad_enable;
+		break;
+
+	case BFPT_WORD14_QER_SR2_BIT7:
+		// TODO:
+		break;
+
+	case BFPT_WORD14_QER_SR2_BIT1:
+		params->enable_quad_io = spansion_new_quad_enable;
+		break;
+	}
+
+	return 0;
+}
+
+static int spi_nor_parse_sfdp(struct spi_nor *nor,
+			      const struct flash_info *info,
+			      struct spi_nor_basic_flash_parameter *params)
+{
+	const struct sfdp_parameter_header *param_header, *bfpt_header;
+	struct sfdp_parameter_header *param_headers = NULL;
+	struct sfdp_header header;
+	size_t psize;
+	int i, err;
+
+	/* Get the SFDP header. */
+	err = spi_nor_read_sfdp(nor, 0, sizeof(header), &header);
+	if (err)
+		return err;
+
+	/* Check the SFDP header version. */
+	if (le32_to_cpu(header.signature) != SFDP_SIGNATURE ||
+	    header.major != SFDP_JESD216_MAJOR ||
+	    header.minor < SFDP_JESD216_MINOR)
+		return -EINVAL;
+
+	/*
+	 * Verify that the first and only mandatory parameter header is a
+	 * Basic Flash Parameter Table header as specified in JESD216.
+	 */
+	bfpt_header = &header.bfpt_header;
+	if (SFDP_PARAM_HEADER_ID(bfpt_header) != SFDP_BFPT_ID ||
+	    bfpt_header->major != SFDP_JESD216_MAJOR)
+		return -EINVAL;
+
+	/* Allocate memory for parameter headers. */
+	if (header.nph) {
+		psize = header.nph * sizeof(*param_headers);
+
+		param_headers = kmalloc(psize, GFP_KERNEL);
+		if (!param_headers) {
+			dev_err(nor->dev,
+				"failed to allocate memory for SFDP parameter headers\n");
+			return -ENOMEM;
+		}
+
+		err = spi_nor_read_sfdp(nor, sizeof(header),
+					psize, param_headers);
+		if (err) {
+			dev_err(nor->dev,
+				"failed to read SFDP parameter headers\n");
+			goto exit;
+		}
+	}
+
+	/*
+	 * Check other parameter headers to get the latest revision of
+	 * the basic flash parameter table.
+	 */
+	for (i = 0; i < header.nph; ++i) {
+		param_header = &param_headers[i];
+
+		if (SFDP_PARAM_HEADER_ID(param_header) == SFDP_BFPT_ID &&
+		    param_header->major == SFDP_JESD216_MAJOR &&
+		    (param_header->minor > bfpt_header->minor ||
+		     (param_header->minor == bfpt_header->minor &&
+		      param_header->length > bfpt_header->length)))
+			bfpt_header = param_header;
+	}
+	err = spi_nor_parse_bfpt(nor, info, bfpt_header, params);
+	if (err)
+		goto exit;
+
+exit:
+	kfree(param_headers);
+	return (err) ? err : bfpt_header->minor;
+}
+
 static int spi_nor_init_params(struct spi_nor *nor,
 			       const struct flash_info *info,
 			       struct spi_nor_basic_flash_parameter *params)
 {
-	// TODO: parse SFDP table
+	int jesd216_minor = -EINVAL;
+
+	/* First trying to parse SFDP tables. */
+	if (!(info->flags & SPI_NOR_SKIP_SFDP)) {
+		jesd216_minor = spi_nor_parse_sfdp(nor, info, params);
+
+		if (jesd216_minor >= SFDP_JESD216B_MINOR)
+			return 0;
+
+		if (jesd216_minor >= SFDP_JESD216_MINOR)
+			goto set_enable_quad_io;
+	}
 
 	/* If SFDP tables are not available, use legacy settings. */
 	memset(params, 0, sizeof(*params));
@@ -1385,6 +1805,7 @@ static int spi_nor_init_params(struct spi_nor *nor,
 		spi_nor_set_erase_settings(&params->erase_types[1],
 					   SNOR_ERASE_4K, SPINOR_OP_BE_4K_PMC);
 
+set_enable_quad_io:
 	/* Select the procedure to set the Quad Enable bit. */
 	if (params->rd_modes & (SNOR_MODE_1_1_4 |
 				SNOR_MODE_1_4_4 |
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 730f33dce1ea..4fcb4225097e 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -56,6 +56,7 @@
 #define SPINOR_OP_CHIP_ERASE	0xc7	/* Erase whole flash chip */
 #define SPINOR_OP_SE		0xd8	/* Sector erase (usually 64KiB) */
 #define SPINOR_OP_RDID		0x9f	/* Read JEDEC ID */
+#define SPINOR_OP_RDSFDP	0x5a	/* Read SFDP */
 #define SPINOR_OP_RDCR		0x35	/* Read configuration register */
 #define SPINOR_OP_RDFSR		0x70	/* Read flag status register */
 
-- 
1.8.2.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 9/9] mtd: spi-nor: parse SFDP 4-byte Address Instruction Table
  2016-06-20 16:49 [PATCH 0/9] mtd: spi-nor: parse SFDP tables as defined by JESD216B Cyrille Pitchen
                   ` (7 preceding siblings ...)
  2016-06-20 16:50 ` [PATCH 8/9] mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables Cyrille Pitchen
@ 2016-06-20 16:50 ` Cyrille Pitchen
  8 siblings, 0 replies; 26+ messages in thread
From: Cyrille Pitchen @ 2016-06-20 16:50 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

This patch adds supports for SFDP (JESD216B) 4-byte Address Instruction
Table. This table is optional but when available, we parse it to get the
4-byte address op codes supported by the memory.
Using these op codes is stateless as opposed to entering the 4-byte
address mode or setting the Base Address Register (BAR).

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 142 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 141 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 2d53c65f5795..b6ef5716d46f 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1430,6 +1430,7 @@ struct sfdp_parameter_header {
 
 
 #define SFDP_BFPT_ID		0xff00u	/* Basic Flash Parameter Table */
+#define SFDP_4BAIT_ID		0xff84u	/* 4-byte Address Instruction Table */
 
 #define SFDP_SIGNATURE		0x50444653u
 #define SFDP_JESD216_MAJOR	1
@@ -1678,6 +1679,125 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
 	return 0;
 }
 
+struct sfdp_4bait {
+	enum spi_nor_protocol_index	pindex;
+	int				wbit;
+};
+
+static int spi_nor_parse_4bait(struct spi_nor *nor,
+			      const struct flash_info *info,
+			      const struct sfdp_parameter_header *param_header,
+			      struct spi_nor_basic_flash_parameter *params)
+{
+	static const struct sfdp_4bait reads[] = {
+		{SNOR_PINDEX_SLOW,  0},	/* 0x13 */
+		{SNOR_PINDEX_1_1_1, 1}, /* 0x0c */
+		{SNOR_PINDEX_1_1_2, 2}, /* 0x3c */
+		{SNOR_PINDEX_1_2_2, 3},	/* 0xbc */
+		{SNOR_PINDEX_1_1_4, 4},	/* 0x6c */
+		{SNOR_PINDEX_1_4_4, 5},	/* 0xec */
+	};
+	static const struct sfdp_4bait programs[] = {
+		{SNOR_PINDEX_1_1_1, 6}, /* 0x12 */
+		{SNOR_PINDEX_1_1_4, 7}, /* 0x34 */
+		{SNOR_PINDEX_1_4_4, 8}, /* 0x3e */
+	};
+	static const struct sfdp_4bait erases[SNOR_MAX_ERASE_TYPES] = {
+		{SNOR_PINDEX_1_1_1,  9},
+		{SNOR_PINDEX_1_1_1, 10},
+		{SNOR_PINDEX_1_1_1, 11},
+		{SNOR_PINDEX_1_1_1, 12},
+	};
+	u32 word[2], addr, rd_modes, wr_modes, erase_modes;
+	int i, err;
+
+	if (param_header->major != SFDP_JESD216_MAJOR ||
+	    param_header->length < 2)
+		return -EINVAL;
+
+	/* Read the 4-byte Address Instruction Table. */
+	addr = SFDP_PARAM_HEADER_PTP(param_header);
+	err = spi_nor_read_sfdp(nor, addr, sizeof(word), word);
+	if (err)
+		return err;
+
+	for (i = 0; i < 2; ++i)
+		word[i] = le32_to_cpu(word[i]);
+
+	/*
+	 * Compute the subset of (Fast) Read commands for which the 4-byte
+	 * version is supported.
+	 */
+	rd_modes = 0;
+	for (i = 0; i < ARRAY_SIZE(reads); ++i) {
+		const struct sfdp_4bait *read = &reads[i];
+
+		if ((params->rd_modes & BIT(read->pindex)) &&
+		    (word[0] & BIT(read->wbit)))
+			rd_modes |= BIT(read->pindex);
+	}
+
+	/*
+	 * Compute the subset of Page Program commands for which the 4-byte
+	 * version is supported.
+	 */
+	wr_modes = 0;
+	for (i = 0; i < ARRAY_SIZE(programs); ++i) {
+		const struct sfdp_4bait *program = &programs[i];
+
+		if ((params->wr_modes & BIT(program->pindex)) &&
+		    (word[0] & BIT(program->wbit)))
+			wr_modes |= BIT(program->pindex);
+	}
+
+	/*
+	 * Compute the subet of Sector Erase commands for which the 4-byte
+	 * version is supported.
+	 */
+	erase_modes = 0;
+	for (i = 0; i < SNOR_MAX_ERASE_TYPES; ++i) {
+		const struct sfdp_4bait *erase = &erases[i];
+
+		if ((params->erase_types[i].size > 0) &&
+		    (word[0] & BIT(erase->wbit)))
+			erase_modes |= BIT(i);
+	}
+
+	/*
+	 * We need at least one 4-byte op code per read, program and erase
+	 * operation; the .read(), .write() and .erase() hooks share the
+	 * nor->addr_width value.
+	 */
+	if (!rd_modes || !wr_modes || !erase_modes)
+		return 0;
+
+	/*
+	 * Discard all operations from the 4-byte instruction set which are
+	 * not supported by this memory.
+	 */
+	params->rd_modes = rd_modes;
+	params->wr_modes = wr_modes;
+	for (i = 0; i < SNOR_MAX_ERASE_TYPES; ++i)
+		if (!(erase_modes & BIT(i)))
+			params->erase_types[i].size = 0;
+
+	/* Use the 4-byte address instruction set. */
+	params->reads[SNOR_PINDEX_SLOW].opcode  = SPINOR_OP_READ_4B;
+	params->reads[SNOR_PINDEX_1_1_1].opcode = SPINOR_OP_READ_FAST_4B;
+	params->reads[SNOR_PINDEX_1_1_2].opcode = SPINOR_OP_READ_1_1_2_4B;
+	params->reads[SNOR_PINDEX_1_2_2].opcode = SPINOR_OP_READ_1_2_2_4B;
+	params->reads[SNOR_PINDEX_1_1_4].opcode = SPINOR_OP_READ_1_1_4_4B;
+	params->reads[SNOR_PINDEX_1_4_4].opcode = SPINOR_OP_READ_1_4_4_4B;
+	params->page_programs[SNOR_PINDEX_1_1_1] = SPINOR_OP_PP_4B;
+	params->page_programs[SNOR_PINDEX_1_1_4] = SPINOR_OP_PP_1_1_4_4B;
+	params->page_programs[SNOR_PINDEX_1_4_4] = SPINOR_OP_PP_1_4_4_4B;
+	for (i = 0; i < SNOR_MAX_ERASE_TYPES; ++i)
+		params->erase_types[i].opcode = (word[1] >> (i * 8)) & 0xff;
+
+	nor->addr_width = 4;
+	return 0;
+}
+
 static int spi_nor_parse_sfdp(struct spi_nor *nor,
 			      const struct flash_info *info,
 			      struct spi_nor_basic_flash_parameter *params)
@@ -1746,6 +1866,24 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
 	if (err)
 		goto exit;
 
+	/* Parse other parameter headers. */
+	for (i = 0; i < header.nph; ++i) {
+		param_header = &param_headers[i];
+
+		switch (SFDP_PARAM_HEADER_ID(param_header)) {
+		case SFDP_4BAIT_ID:
+			err = spi_nor_parse_4bait(nor, info, param_header,
+						  params);
+			break;
+
+		default:
+			break;
+		}
+
+		if (err)
+			goto exit;
+	}
+
 exit:
 	kfree(param_headers);
 	return (err) ? err : bfpt_header->minor;
@@ -2098,7 +2236,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 	if (ret)
 		return ret;
 
-	if (info->addr_width)
+	if (nor->addr_width)
+		; /* already configured by spi_nor_setup(). */
+	else if (info->addr_width)
 		nor->addr_width = info->addr_width;
 	else if (mtd->size > 0x1000000) {
 		/* enable 4-byte addressing if the device exceeds 16MiB */
-- 
1.8.2.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands
  2016-06-20 16:50 ` [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands Cyrille Pitchen
@ 2016-06-23 20:35   ` Michal Suchanek
  2016-06-23 20:46     ` Marek Vasut
  2016-06-27  9:52     ` Cyrille Pitchen
  0 siblings, 2 replies; 26+ messages in thread
From: Michal Suchanek @ 2016-06-23 20:35 UTC (permalink / raw)
  To: Cyrille Pitchen
  Cc: Brian Norris, MTD Maling List, Marek Vašut, Boris Brezillon,
	nicolas.ferre, Linux Kernel Mailing List

Hello,

this patch is kind of awesome.

I have a few practical concerns however.

On 20 June 2016 at 18:50, Cyrille Pitchen <cyrille.pitchen@atmel.com> wrote:
> Before this patch, m25p80_read() supported few SPI protocols:
> - regular SPI 1-1-1
> - SPI Dual Output 1-1-2
> - SPI Quad Output 1-1-4
> On the other hand, all other m25p80_*() hooks only supported SPI 1-1-1.

Under typical use my estimate is that huge majority of data is
transferred in _read() seconded by _write().

As I understand it the n-n-n means how many bits you transfer in
parallel when sending command-address-data.

In _read() the command and data overhead is negligible when you can
read kilobytes at once. So difference between 1-1-4 and 4-4-4 is not
meaningful performance-wise. Are there flash chips that support one
but not the other?

For _write() the benefits are even harder to assess. You can
presumably write at n-n-4 or n-n-2 if your controller and flash
supports it transferring the page faster. And then spend possibly
large amount of time waiting for the flash to get ready again. If the
programming time is fixed transferring the page faster may or may not
have benefits. It may at least free the bus for other devices to use.

The _reg_ stuff is probably negligible altogether,

Lastly the faster transfers of address bytes seem to be achieved with
increasingly longer command codes given how much the maximum command
length increased. So even in a page write where the address is a few %
of the transfer the benefit of these extra modes is dubious.

Overall I wonder how much it is worthwhile to complicate the code to
get all these modes in every single function.

Thanks

Michal

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands
  2016-06-23 20:35   ` Michal Suchanek
@ 2016-06-23 20:46     ` Marek Vasut
  2016-06-23 21:58       ` Michal Suchanek
  2016-06-27  9:52     ` Cyrille Pitchen
  1 sibling, 1 reply; 26+ messages in thread
From: Marek Vasut @ 2016-06-23 20:46 UTC (permalink / raw)
  To: Michal Suchanek, Cyrille Pitchen
  Cc: Brian Norris, MTD Maling List, Boris Brezillon, nicolas.ferre,
	Linux Kernel Mailing List

On 06/23/2016 10:35 PM, Michal Suchanek wrote:
> Hello,

Hi,

> this patch is kind of awesome.
> 
> I have a few practical concerns however.
> 
> On 20 June 2016 at 18:50, Cyrille Pitchen <cyrille.pitchen@atmel.com> wrote:
>> Before this patch, m25p80_read() supported few SPI protocols:
>> - regular SPI 1-1-1
>> - SPI Dual Output 1-1-2
>> - SPI Quad Output 1-1-4
>> On the other hand, all other m25p80_*() hooks only supported SPI 1-1-1.
> 
> Under typical use my estimate is that huge majority of data is
> transferred in _read() seconded by _write().
> 
> As I understand it the n-n-n means how many bits you transfer in
> parallel when sending command-address-data.
> 
> In _read() the command and data overhead is negligible when you can
> read kilobytes at once. So difference between 1-1-4 and 4-4-4 is not
> meaningful performance-wise. Are there flash chips that support one
> but not the other?

That's quite unlikely.

> For _write() the benefits are even harder to assess.

The page program usually works on 256B pages, so the math is rather easy.

> You can
> presumably write at n-n-4 or n-n-2 if your controller and flash
> supports it transferring the page faster. And then spend possibly
> large amount of time waiting for the flash to get ready again. If the
> programming time is fixed transferring the page faster may or may not
> have benefits. It may at least free the bus for other devices to use.
> 
> The _reg_ stuff is probably negligible altogether,
> 
> Lastly the faster transfers of address bytes seem to be achieved with
> increasingly longer command codes given how much the maximum command
> length increased. So even in a page write where the address is a few %
> of the transfer the benefit of these extra modes is dubious.
> 
> Overall I wonder how much it is worthwhile to complicate the code to
> get all these modes in every single function.

In my opinion, 1-1-x makes sense as it is supported by most flashes,
while n-m-x where n,m>1 does not make sense as it often requires some
stateful change to non-volatile register with little gain.

> Thanks
> 
> Michal
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands
  2016-06-23 20:46     ` Marek Vasut
@ 2016-06-23 21:58       ` Michal Suchanek
  2016-06-23 22:14         ` Marek Vasut
  0 siblings, 1 reply; 26+ messages in thread
From: Michal Suchanek @ 2016-06-23 21:58 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Cyrille Pitchen, Brian Norris, MTD Maling List, Boris Brezillon,
	nicolas.ferre, Linux Kernel Mailing List

On 23 June 2016 at 22:46, Marek Vasut <marex@denx.de> wrote:
> On 06/23/2016 10:35 PM, Michal Suchanek wrote:
>> Hello,
>
> Hi,
>
>> this patch is kind of awesome.
>>
>> I have a few practical concerns however.
>>
>> On 20 June 2016 at 18:50, Cyrille Pitchen <cyrille.pitchen@atmel.com> wrote:
>>> Before this patch, m25p80_read() supported few SPI protocols:
>>> - regular SPI 1-1-1
>>> - SPI Dual Output 1-1-2
>>> - SPI Quad Output 1-1-4
>>> On the other hand, all other m25p80_*() hooks only supported SPI 1-1-1.
>>
>> Under typical use my estimate is that huge majority of data is
>> transferred in _read() seconded by _write().
>>
>> As I understand it the n-n-n means how many bits you transfer in
>> parallel when sending command-address-data.
>>
>> In _read() the command and data overhead is negligible when you can
>> read kilobytes at once. So difference between 1-1-4 and 4-4-4 is not
>> meaningful performance-wise. Are there flash chips that support one
>> but not the other?
>
> That's quite unlikely.
>
>> For _write() the benefits are even harder to assess.
>
> The page program usually works on 256B pages, so the math is rather easy.
>
>> You can
>> presumably write at n-n-4 or n-n-2 if your controller and flash
>> supports it transferring the page faster. And then spend possibly
>> large amount of time waiting for the flash to get ready again. If the
>> programming time is fixed transferring the page faster may or may not
>> have benefits. It may at least free the bus for other devices to use.
>>
>> The _reg_ stuff is probably negligible altogether,
>>
>> Lastly the faster transfers of address bytes seem to be achieved with
>> increasingly longer command codes given how much the maximum command
>> length increased. So even in a page write where the address is a few %
>> of the transfer the benefit of these extra modes is dubious.
>>
>> Overall I wonder how much it is worthwhile to complicate the code to
>> get all these modes in every single function.
>
> In my opinion, 1-1-x makes sense as it is supported by most flashes,
> while n-m-x where n,m>1 does not make sense as it often requires some
> stateful change to non-volatile register with little gain.
>

There is actually one thing that x-x-x modes make easier. If I were to
implement dual mode switch on my SPI master controller it would be
probably set for whole message and would not change mid-transfer.
Still you can probably simulate x-x-x with 1-1-x by scattering the
1-1-x command bits across more bytes.

Thanks

Michal

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands
  2016-06-23 21:58       ` Michal Suchanek
@ 2016-06-23 22:14         ` Marek Vasut
  2016-06-23 22:43           ` Michal Suchanek
  0 siblings, 1 reply; 26+ messages in thread
From: Marek Vasut @ 2016-06-23 22:14 UTC (permalink / raw)
  To: Michal Suchanek
  Cc: Cyrille Pitchen, Brian Norris, MTD Maling List, Boris Brezillon,
	nicolas.ferre, Linux Kernel Mailing List

On 06/23/2016 11:58 PM, Michal Suchanek wrote:
> On 23 June 2016 at 22:46, Marek Vasut <marex@denx.de> wrote:
>> On 06/23/2016 10:35 PM, Michal Suchanek wrote:
>>> Hello,
>>
>> Hi,
>>
>>> this patch is kind of awesome.
>>>
>>> I have a few practical concerns however.
>>>
>>> On 20 June 2016 at 18:50, Cyrille Pitchen <cyrille.pitchen@atmel.com> wrote:
>>>> Before this patch, m25p80_read() supported few SPI protocols:
>>>> - regular SPI 1-1-1
>>>> - SPI Dual Output 1-1-2
>>>> - SPI Quad Output 1-1-4
>>>> On the other hand, all other m25p80_*() hooks only supported SPI 1-1-1.
>>>
>>> Under typical use my estimate is that huge majority of data is
>>> transferred in _read() seconded by _write().
>>>
>>> As I understand it the n-n-n means how many bits you transfer in
>>> parallel when sending command-address-data.
>>>
>>> In _read() the command and data overhead is negligible when you can
>>> read kilobytes at once. So difference between 1-1-4 and 4-4-4 is not
>>> meaningful performance-wise. Are there flash chips that support one
>>> but not the other?
>>
>> That's quite unlikely.
>>
>>> For _write() the benefits are even harder to assess.
>>
>> The page program usually works on 256B pages, so the math is rather easy.
>>
>>> You can
>>> presumably write at n-n-4 or n-n-2 if your controller and flash
>>> supports it transferring the page faster. And then spend possibly
>>> large amount of time waiting for the flash to get ready again. If the
>>> programming time is fixed transferring the page faster may or may not
>>> have benefits. It may at least free the bus for other devices to use.
>>>
>>> The _reg_ stuff is probably negligible altogether,
>>>
>>> Lastly the faster transfers of address bytes seem to be achieved with
>>> increasingly longer command codes given how much the maximum command
>>> length increased. So even in a page write where the address is a few %
>>> of the transfer the benefit of these extra modes is dubious.
>>>
>>> Overall I wonder how much it is worthwhile to complicate the code to
>>> get all these modes in every single function.
>>
>> In my opinion, 1-1-x makes sense as it is supported by most flashes,
>> while n-m-x where n,m>1 does not make sense as it often requires some
>> stateful change to non-volatile register with little gain.
>>
> 
> There is actually one thing that x-x-x modes make easier. If I were to
> implement dual mode switch on my SPI master controller it would be
> probably set for whole message and would not change mid-transfer.

Your IP would not sell as customers would like to use it with SPI
flashes which can only do 1-1-x modes. These flashes are on the market,
today, and thus used and thus you have to support them if you want to
make profit.

In fact, the SPI flash starts in 1-1-1 mode anyway, thus you need to
support that mode. To support other modes, you need to implement simple
switch in the hardware which either shifts out a bit a time, two bits on
two lines at a time or whatever else ; selecting which one it is must be
done synchronous to input clock and on a byte boundary, which is trivial
to implement in hardware.

> Still you can probably simulate x-x-x with 1-1-x by scattering the
> 1-1-x command bits across more bytes.

That's not how you usually implement it. It's quite often a shift register.

> Thanks
> 
> Michal
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands
  2016-06-23 22:14         ` Marek Vasut
@ 2016-06-23 22:43           ` Michal Suchanek
  2016-06-23 22:50             ` Marek Vasut
  0 siblings, 1 reply; 26+ messages in thread
From: Michal Suchanek @ 2016-06-23 22:43 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Cyrille Pitchen, Brian Norris, MTD Maling List, Boris Brezillon,
	nicolas.ferre, Linux Kernel Mailing List

On 24 June 2016 at 00:14, Marek Vasut <marex@denx.de> wrote:
> On 06/23/2016 11:58 PM, Michal Suchanek wrote:
>> On 23 June 2016 at 22:46, Marek Vasut <marex@denx.de> wrote:
>>> On 06/23/2016 10:35 PM, Michal Suchanek wrote:
>>>> Hello,
>>>
>>> Hi,
>>>
>>>> this patch is kind of awesome.
>>>>
>>>> I have a few practical concerns however.
>>>>
>>>> On 20 June 2016 at 18:50, Cyrille Pitchen <cyrille.pitchen@atmel.com> wrote:
>>>>> Before this patch, m25p80_read() supported few SPI protocols:
>>>>> - regular SPI 1-1-1
>>>>> - SPI Dual Output 1-1-2
>>>>> - SPI Quad Output 1-1-4
>>>>> On the other hand, all other m25p80_*() hooks only supported SPI 1-1-1.
>>>>
>>>> Under typical use my estimate is that huge majority of data is
>>>> transferred in _read() seconded by _write().
>>>>
>>>> As I understand it the n-n-n means how many bits you transfer in
>>>> parallel when sending command-address-data.
>>>>
>>>> In _read() the command and data overhead is negligible when you can
>>>> read kilobytes at once. So difference between 1-1-4 and 4-4-4 is not
>>>> meaningful performance-wise. Are there flash chips that support one
>>>> but not the other?
>>>
>>> That's quite unlikely.
>>>
>>>> For _write() the benefits are even harder to assess.
>>>
>>> The page program usually works on 256B pages, so the math is rather easy.
>>>
>>>> You can
>>>> presumably write at n-n-4 or n-n-2 if your controller and flash
>>>> supports it transferring the page faster. And then spend possibly
>>>> large amount of time waiting for the flash to get ready again. If the
>>>> programming time is fixed transferring the page faster may or may not
>>>> have benefits. It may at least free the bus for other devices to use.
>>>>
>>>> The _reg_ stuff is probably negligible altogether,
>>>>
>>>> Lastly the faster transfers of address bytes seem to be achieved with
>>>> increasingly longer command codes given how much the maximum command
>>>> length increased. So even in a page write where the address is a few %
>>>> of the transfer the benefit of these extra modes is dubious.
>>>>
>>>> Overall I wonder how much it is worthwhile to complicate the code to
>>>> get all these modes in every single function.
>>>
>>> In my opinion, 1-1-x makes sense as it is supported by most flashes,
>>> while n-m-x where n,m>1 does not make sense as it often requires some
>>> stateful change to non-volatile register with little gain.
>>>
>>
>> There is actually one thing that x-x-x modes make easier. If I were to
>> implement dual mode switch on my SPI master controller it would be
>> probably set for whole message and would not change mid-transfer.
>

>
>> Still you can probably simulate x-x-x with 1-1-x by scattering the
>> 1-1-x command bits across more bytes.
>
> That's not how you usually implement it. It's quite often a shift register.
>

Checking the manual there is a bit in a register that switches the
master controller to dual mode receive (only). So the master
controller can do 1-1-2 read (only). I don't use that feature because
afaict there is no code in m25p80 which does the switch and as pointed
out the reg_read commands are done in 1-1-1.

If there was similar bit for write you could do 2-2-2 write but any
other option would be quite challenging.

Thanks

Michal

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands
  2016-06-23 22:43           ` Michal Suchanek
@ 2016-06-23 22:50             ` Marek Vasut
  2016-06-23 23:04               ` Michal Suchanek
  0 siblings, 1 reply; 26+ messages in thread
From: Marek Vasut @ 2016-06-23 22:50 UTC (permalink / raw)
  To: Michal Suchanek
  Cc: Cyrille Pitchen, Brian Norris, MTD Maling List, Boris Brezillon,
	nicolas.ferre, Linux Kernel Mailing List

On 06/24/2016 12:43 AM, Michal Suchanek wrote:
> On 24 June 2016 at 00:14, Marek Vasut <marex@denx.de> wrote:
>> On 06/23/2016 11:58 PM, Michal Suchanek wrote:
>>> On 23 June 2016 at 22:46, Marek Vasut <marex@denx.de> wrote:
>>>> On 06/23/2016 10:35 PM, Michal Suchanek wrote:
>>>>> Hello,
>>>>
>>>> Hi,
>>>>
>>>>> this patch is kind of awesome.
>>>>>
>>>>> I have a few practical concerns however.
>>>>>
>>>>> On 20 June 2016 at 18:50, Cyrille Pitchen <cyrille.pitchen@atmel.com> wrote:
>>>>>> Before this patch, m25p80_read() supported few SPI protocols:
>>>>>> - regular SPI 1-1-1
>>>>>> - SPI Dual Output 1-1-2
>>>>>> - SPI Quad Output 1-1-4
>>>>>> On the other hand, all other m25p80_*() hooks only supported SPI 1-1-1.
>>>>>
>>>>> Under typical use my estimate is that huge majority of data is
>>>>> transferred in _read() seconded by _write().
>>>>>
>>>>> As I understand it the n-n-n means how many bits you transfer in
>>>>> parallel when sending command-address-data.
>>>>>
>>>>> In _read() the command and data overhead is negligible when you can
>>>>> read kilobytes at once. So difference between 1-1-4 and 4-4-4 is not
>>>>> meaningful performance-wise. Are there flash chips that support one
>>>>> but not the other?
>>>>
>>>> That's quite unlikely.
>>>>
>>>>> For _write() the benefits are even harder to assess.
>>>>
>>>> The page program usually works on 256B pages, so the math is rather easy.
>>>>
>>>>> You can
>>>>> presumably write at n-n-4 or n-n-2 if your controller and flash
>>>>> supports it transferring the page faster. And then spend possibly
>>>>> large amount of time waiting for the flash to get ready again. If the
>>>>> programming time is fixed transferring the page faster may or may not
>>>>> have benefits. It may at least free the bus for other devices to use.
>>>>>
>>>>> The _reg_ stuff is probably negligible altogether,
>>>>>
>>>>> Lastly the faster transfers of address bytes seem to be achieved with
>>>>> increasingly longer command codes given how much the maximum command
>>>>> length increased. So even in a page write where the address is a few %
>>>>> of the transfer the benefit of these extra modes is dubious.
>>>>>
>>>>> Overall I wonder how much it is worthwhile to complicate the code to
>>>>> get all these modes in every single function.
>>>>
>>>> In my opinion, 1-1-x makes sense as it is supported by most flashes,
>>>> while n-m-x where n,m>1 does not make sense as it often requires some
>>>> stateful change to non-volatile register with little gain.
>>>>
>>>
>>> There is actually one thing that x-x-x modes make easier. If I were to
>>> implement dual mode switch on my SPI master controller it would be
>>> probably set for whole message and would not change mid-transfer.
>>
> 
>>
>>> Still you can probably simulate x-x-x with 1-1-x by scattering the
>>> 1-1-x command bits across more bytes.
>>
>> That's not how you usually implement it. It's quite often a shift register.
>>
> 
> Checking the manual there is a bit in a register that switches the
> master controller to dual mode receive (only). So the master
> controller can do 1-1-2 read (only). I don't use that feature because
> afaict there is no code in m25p80 which does the switch and as pointed
> out the reg_read commands are done in 1-1-1.

I don't think I understand. Are you talking about some specific
controller now ?

> If there was similar bit for write you could do 2-2-2 write but any
> other option would be quite challenging.
> 
> Thanks
> 
> Michal
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands
  2016-06-23 22:50             ` Marek Vasut
@ 2016-06-23 23:04               ` Michal Suchanek
  0 siblings, 0 replies; 26+ messages in thread
From: Michal Suchanek @ 2016-06-23 23:04 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Cyrille Pitchen, Brian Norris, MTD Maling List, Boris Brezillon,
	nicolas.ferre, Linux Kernel Mailing List

On 24 June 2016 at 00:50, Marek Vasut <marex@denx.de> wrote:
> On 06/24/2016 12:43 AM, Michal Suchanek wrote:
>> On 24 June 2016 at 00:14, Marek Vasut <marex@denx.de> wrote:
>>> On 06/23/2016 11:58 PM, Michal Suchanek wrote:
>>>> On 23 June 2016 at 22:46, Marek Vasut <marex@denx.de> wrote:
>>>>> On 06/23/2016 10:35 PM, Michal Suchanek wrote:
>>>>>> Hello,
>>>>>
>>>>> Hi,
>>>>>
>>>>>> this patch is kind of awesome.
>>>>>>
>>>>>> I have a few practical concerns however.
>>>>>>
>>>>>> On 20 June 2016 at 18:50, Cyrille Pitchen <cyrille.pitchen@atmel.com> wrote:
>>>>>>> Before this patch, m25p80_read() supported few SPI protocols:
>>>>>>> - regular SPI 1-1-1
>>>>>>> - SPI Dual Output 1-1-2
>>>>>>> - SPI Quad Output 1-1-4
>>>>>>> On the other hand, all other m25p80_*() hooks only supported SPI 1-1-1.
>>>>>>
>>>>>> Under typical use my estimate is that huge majority of data is
>>>>>> transferred in _read() seconded by _write().
>>>>>>
>>>>>> As I understand it the n-n-n means how many bits you transfer in
>>>>>> parallel when sending command-address-data.
>>>>>>
>>>>>> In _read() the command and data overhead is negligible when you can
>>>>>> read kilobytes at once. So difference between 1-1-4 and 4-4-4 is not
>>>>>> meaningful performance-wise. Are there flash chips that support one
>>>>>> but not the other?
>>>>>
>>>>> That's quite unlikely.
>>>>>
>>>>>> For _write() the benefits are even harder to assess.
>>>>>
>>>>> The page program usually works on 256B pages, so the math is rather easy.
>>>>>
>>>>>> You can
>>>>>> presumably write at n-n-4 or n-n-2 if your controller and flash
>>>>>> supports it transferring the page faster. And then spend possibly
>>>>>> large amount of time waiting for the flash to get ready again. If the
>>>>>> programming time is fixed transferring the page faster may or may not
>>>>>> have benefits. It may at least free the bus for other devices to use.
>>>>>>
>>>>>> The _reg_ stuff is probably negligible altogether,
>>>>>>
>>>>>> Lastly the faster transfers of address bytes seem to be achieved with
>>>>>> increasingly longer command codes given how much the maximum command
>>>>>> length increased. So even in a page write where the address is a few %
>>>>>> of the transfer the benefit of these extra modes is dubious.
>>>>>>
>>>>>> Overall I wonder how much it is worthwhile to complicate the code to
>>>>>> get all these modes in every single function.
>>>>>
>>>>> In my opinion, 1-1-x makes sense as it is supported by most flashes,
>>>>> while n-m-x where n,m>1 does not make sense as it often requires some
>>>>> stateful change to non-volatile register with little gain.
>>>>>
>>>>
>>>> There is actually one thing that x-x-x modes make easier. If I were to
>>>> implement dual mode switch on my SPI master controller it would be
>>>> probably set for whole message and would not change mid-transfer.
>>>
>>
>>>
>>>> Still you can probably simulate x-x-x with 1-1-x by scattering the
>>>> 1-1-x command bits across more bytes.
>>>
>>> That's not how you usually implement it. It's quite often a shift register.
>>>
>>
>> Checking the manual there is a bit in a register that switches the
>> master controller to dual mode receive (only). So the master
>> controller can do 1-1-2 read (only). I don't use that feature because
>> afaict there is no code in m25p80 which does the switch and as pointed
>> out the reg_read commands are done in 1-1-1.
>
> I don't think I understand. Are you talking about some specific
> controller now ?
>

Yes, the sunxi spi controller can do dual read according to manual but
not sure if anyone has tired that, At least the A31 variant can.

Thanks

Michal

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands
  2016-06-23 20:35   ` Michal Suchanek
  2016-06-23 20:46     ` Marek Vasut
@ 2016-06-27  9:52     ` Cyrille Pitchen
  2016-06-27 12:37       ` Cyrille Pitchen
  1 sibling, 1 reply; 26+ messages in thread
From: Cyrille Pitchen @ 2016-06-27  9:52 UTC (permalink / raw)
  To: Michal Suchanek
  Cc: Brian Norris, MTD Maling List, Marek Vašut, Boris Brezillon,
	nicolas.ferre, Linux Kernel Mailing List

Le 23/06/2016 22:35, Michal Suchanek a écrit :
> Hello,
> 
> this patch is kind of awesome.
> 
> I have a few practical concerns however.
> 
> On 20 June 2016 at 18:50, Cyrille Pitchen <cyrille.pitchen@atmel.com> wrote:
>> Before this patch, m25p80_read() supported few SPI protocols:
>> - regular SPI 1-1-1
>> - SPI Dual Output 1-1-2
>> - SPI Quad Output 1-1-4
>> On the other hand, all other m25p80_*() hooks only supported SPI 1-1-1.
> 
> Under typical use my estimate is that huge majority of data is
> transferred in _read() seconded by _write().
> 
> As I understand it the n-n-n means how many bits you transfer in
> parallel when sending command-address-data.
> 
> In _read() the command and data overhead is negligible when you can
> read kilobytes at once. So difference between 1-1-4 and 4-4-4 is not
> meaningful performance-wise. Are there flash chips that support one
> but not the other?
> 
> For _write() the benefits are even harder to assess. You can
> presumably write at n-n-4 or n-n-2 if your controller and flash
> supports it transferring the page faster. And then spend possibly
> large amount of time waiting for the flash to get ready again. If the
> programming time is fixed transferring the page faster may or may not
> have benefits. It may at least free the bus for other devices to use.
> 

This series of patches is not supposed to improve performances. Lets take
a Macronix mx25l25673g as an example with a 512-byte read (3-byte address):

1 - Fast Read 1-1-4 (0 mode cycle, 8 wait states)
command: 8 cycles
address/mode/dummy: (3 * 8) + 0 + 8 = 32 cycles
data: (512 * 8) / 4 = 1024 cycles
total: 8 + 32 + 1024 = 1064 cycles

2 - Fast Read 1-4-4 (2 mode cycles, 4 wait states)
command: 8 cycles
address/mode/dummy: (3 * 8) / 4 + 2 + 4 = 12 cycles
data: (512 * 8) / 4 = 1024 cycles
total: 8 + 12 + 1024 = 1044 cycles

theoretical benefit: < 2%


The real purpose of the whole series is to provide solutions to some known
issues.

A - Currently the spi-nor force use the B7h op code to enter the 4-byte
address mode of > 16MB memories. This mode is state-full; it changes the
internal state of the SPI memory. Once in its 4-byte mode, the regular 3-byte
Fast Read, Page Program and Sector Erase op codes are expected to be followed
by a 4-byte address. Then if a spurious reset occurs, some bootloaders might
be stuck as they still expect to use 3-byte addresses.

On the other hand, when available, the 4-byte address instruction set is
stateless. So we'd rather use it whenever possible. It's the purpose of patch
2. However adding the SPI_NOR_4B_OPCODES on some memory entry is not always
possible. Again, let's take the Macronix mx25l25673g memory as an example:
it shares the very same JEDEC ID with the older mx25l25635e. The 73g supports
the 4-byte address instruction set whereas the 35e doesn't. Hence we can't
add the SPI_NOR_4B_OPCODES on the 35e entry without introducing a bug.

My first approach was to patch the spi-nor framework to allow two or more
memory entries to share the same JEDEC ID and to select the right entry
according to the compatible DT string. This solution was rejected.

Then my latest solution is to rely on the SFDP tables: the 73g supports the
optional 4-byte Address Instruction Table whereas the 35e doesn't support SFDP
table at all. Hence, when this table is successfully parsed on the 73g, we know
we can safely use the 4-byte op codes. For the 35e case, we still enter the
4-byte address mode as before since there is no other solution...
So with this series of patches, no regression with 35e memories but a solution
to properly use 73g memories without changing their internal state: the
bootloader is now happy.


B - Setting the old 'mode' argument of spi_nor_read() to SPI_NOR_QUAD just told
the SPI controller supports Fast Read 1-1-4 but nothing about whether it also
supports Page Program x-y-4 commands. It is interesting to make the difference
between read and write hardware capabilities. Indeed we cannot assume that if
a controller can do Fast Read 1-1-4 operations then it can also perform
Page Program 1-1-4. I'm pretty sure this statement is false. So let's the SPI
controller explicitly declares its read and write hardware capabilities.

Once again, I'm not targeting performance improvement with Page Program.
However, using other SPI protocols when available helps to deal with some
memory quirks.

This time, let's take the case of Micron n25q512*. Those memory
are > 16MB so we fall into the same issue as described in A.
For those Micron memories, depending on the part number (telling us whether the
Reset pin is available...) the 12h op code stands for two different operations:
- either 3-byte address Page Program x-4-4 (the standard 38h op code for this
  operation is not available and there is no op code for 4-byte Page Program
  1-1-1)
- either 4-byte address Page Program 1-1-1 (the standard command associated to
  the 12h op code)

Since they are different part numbers of the same memory family, all those
memories once again share the very same JEDEC ID and there no mean to 
dynamically discover the actual part number (or I didn't find such a mean yet).
Anyway, even knowing the part number, depending on the result, the 4-byte Page
Program 1-1-1 operation is simply not supported.

Hopefully, for all part numbers, 4-byte Page Program 1-1-4 is always supported
by the 34h op code, which is the standard hope code for this operation.

Then with SPI controllers which explicitly supports Page Program 1-1-4, we 
could implement something to fix the Page Program operation on Micron memory
above 16MB. Hence we have a solution!

There are just examples, I guess we could find others. My point is that
currently we can't use some QPSI memories properly and its a blocking issue.
The whole series should be see as a bug fixes enabler rather than a performance
improvement.


> The _reg_ stuff is probably negligible altogether,
> 
> Lastly the faster transfers of address bytes seem to be achieved with
> increasingly longer command codes given how much the maximum command
> length increased. So even in a page write where the address is a few %
> of the transfer the benefit of these extra modes is dubious.
I'm not sure to understand this point but I guess you refer to:
-#define	MAX_CMD_SIZE		6
+#define	MAX_CMD_SIZE		16

If so, the actual command size doesn't change at all, the increase of this
macro value is justified by another reason. Indeed, let's have a look into
the m25p80_read_reg(): before the patch it was implemented using
spi_write_then_read(). This later function uses an intermediate buffer,
which might be kmalloc() allocated, to transfer data with spi_sync() and calls
memcpy() to copy data from/to this imtermediate buffer to/from buffers
provided as function parameters (txbuf and rxbuf).
As the comment says, the purpose of this intermediate buffer is to be
"DMA-safe".

Then, after the patch, spi_write_then_read() is no longer used but we still
need a DMA-safe buffer to perform the data transfer with spi_sync(). This can
be achieve with the flash->command[] buffer; we just need to increase its size
some more since it is now also filled with the read data whereas it was only
filled with the command/address/dummy data before.

> 
> Overall I wonder how much it is worthwhile to complicate the code to
> get all these modes in every single function.
> 
> Thanks
> 
> Michal
> 

Best regards,

Cyrille

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands
  2016-06-27  9:52     ` Cyrille Pitchen
@ 2016-06-27 12:37       ` Cyrille Pitchen
  2016-06-28  7:57           ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
  0 siblings, 1 reply; 26+ messages in thread
From: Cyrille Pitchen @ 2016-06-27 12:37 UTC (permalink / raw)
  To: Michal Suchanek
  Cc: Brian Norris, MTD Maling List, Marek Vašut, Boris Brezillon,
	nicolas.ferre, Linux Kernel Mailing List, marcin.krzeminski

Hi all,

+ Marcin,

According to Marcin's series of patches
"mtd: spi-nor: DUAL/QUAD I/O read modes implementation"
and the Spansion/Cypress S25FS512S datasheet, new Cypress QSPI memories
no longer supports Fast Read 1-1-2 or Fast Read 1-1-4 but only Fast Read x-2-2
and Fast Read x-4-4.

Hence one more example of the need to extend the spi-nor framework to support
QSPI memories correctly.

Also, looking at the datasheets of s25fL512s vs s25fS512s, the default
factory settings for the number of (mode + wait state) cycles has changed for
Fast Read 1-4-4 (EBh /ECh).

default 	mode 	wait state
L512s: 		2	4
S512s:		2	8

Those factory default settings are provided by the Basic Flash Parameter Table
from the Serial Flash Discoverable Parameters (JESD216). SFDP tables seem to
be supported by both families.

Hence this series of patches actually adds support to the new Spansion/Cypress
QSPI memories.

We will just have to care about setting a proper value in the mode cycles to
avoid entering the continuous read mode by mistake.
Looking at the JESD216 specification the 0xff value is likely to be a common
value for all manufacturers to tell the QSPI memory not to enter (or leave)
its continuous read/performance enhancement mode.

Entering the continuous read mode tells the QSPI memory that the op code value
will be implicit in the following command, which then will start from the
address cycles. The continuous mode is mainly (only ?) use for XIP application.

Adding support to the continuous read mode might be interesting but should be
discussed in a dedicated thread.

Best regards,

Cyrille

Le 27/06/2016 11:52, Cyrille Pitchen a écrit :
> Le 23/06/2016 22:35, Michal Suchanek a écrit :
>> Hello,
>>
>> this patch is kind of awesome.
>>
>> I have a few practical concerns however.
>>
>> On 20 June 2016 at 18:50, Cyrille Pitchen <cyrille.pitchen@atmel.com> wrote:
>>> Before this patch, m25p80_read() supported few SPI protocols:
>>> - regular SPI 1-1-1
>>> - SPI Dual Output 1-1-2
>>> - SPI Quad Output 1-1-4
>>> On the other hand, all other m25p80_*() hooks only supported SPI 1-1-1.
>>
>> Under typical use my estimate is that huge majority of data is
>> transferred in _read() seconded by _write().
>>
>> As I understand it the n-n-n means how many bits you transfer in
>> parallel when sending command-address-data.
>>
>> In _read() the command and data overhead is negligible when you can
>> read kilobytes at once. So difference between 1-1-4 and 4-4-4 is not
>> meaningful performance-wise. Are there flash chips that support one
>> but not the other?
>>
>> For _write() the benefits are even harder to assess. You can
>> presumably write at n-n-4 or n-n-2 if your controller and flash
>> supports it transferring the page faster. And then spend possibly
>> large amount of time waiting for the flash to get ready again. If the
>> programming time is fixed transferring the page faster may or may not
>> have benefits. It may at least free the bus for other devices to use.
>>
> 
> This series of patches is not supposed to improve performances. Lets take
> a Macronix mx25l25673g as an example with a 512-byte read (3-byte address):
> 
> 1 - Fast Read 1-1-4 (0 mode cycle, 8 wait states)
> command: 8 cycles
> address/mode/dummy: (3 * 8) + 0 + 8 = 32 cycles
> data: (512 * 8) / 4 = 1024 cycles
> total: 8 + 32 + 1024 = 1064 cycles
> 
> 2 - Fast Read 1-4-4 (2 mode cycles, 4 wait states)
> command: 8 cycles
> address/mode/dummy: (3 * 8) / 4 + 2 + 4 = 12 cycles
> data: (512 * 8) / 4 = 1024 cycles
> total: 8 + 12 + 1024 = 1044 cycles
> 
> theoretical benefit: < 2%
> 
> 
> The real purpose of the whole series is to provide solutions to some known
> issues.
> 
> A - Currently the spi-nor force use the B7h op code to enter the 4-byte
> address mode of > 16MB memories. This mode is state-full; it changes the
> internal state of the SPI memory. Once in its 4-byte mode, the regular 3-byte
> Fast Read, Page Program and Sector Erase op codes are expected to be followed
> by a 4-byte address. Then if a spurious reset occurs, some bootloaders might
> be stuck as they still expect to use 3-byte addresses.
> 
> On the other hand, when available, the 4-byte address instruction set is
> stateless. So we'd rather use it whenever possible. It's the purpose of patch
> 2. However adding the SPI_NOR_4B_OPCODES on some memory entry is not always
> possible. Again, let's take the Macronix mx25l25673g memory as an example:
> it shares the very same JEDEC ID with the older mx25l25635e. The 73g supports
> the 4-byte address instruction set whereas the 35e doesn't. Hence we can't
> add the SPI_NOR_4B_OPCODES on the 35e entry without introducing a bug.
> 
> My first approach was to patch the spi-nor framework to allow two or more
> memory entries to share the same JEDEC ID and to select the right entry
> according to the compatible DT string. This solution was rejected.
> 
> Then my latest solution is to rely on the SFDP tables: the 73g supports the
> optional 4-byte Address Instruction Table whereas the 35e doesn't support SFDP
> table at all. Hence, when this table is successfully parsed on the 73g, we know
> we can safely use the 4-byte op codes. For the 35e case, we still enter the
> 4-byte address mode as before since there is no other solution...
> So with this series of patches, no regression with 35e memories but a solution
> to properly use 73g memories without changing their internal state: the
> bootloader is now happy.
> 
> 
> B - Setting the old 'mode' argument of spi_nor_read() to SPI_NOR_QUAD just told
> the SPI controller supports Fast Read 1-1-4 but nothing about whether it also
> supports Page Program x-y-4 commands. It is interesting to make the difference
> between read and write hardware capabilities. Indeed we cannot assume that if
> a controller can do Fast Read 1-1-4 operations then it can also perform
> Page Program 1-1-4. I'm pretty sure this statement is false. So let's the SPI
> controller explicitly declares its read and write hardware capabilities.
> 
> Once again, I'm not targeting performance improvement with Page Program.
> However, using other SPI protocols when available helps to deal with some
> memory quirks.
> 
> This time, let's take the case of Micron n25q512*. Those memory
> are > 16MB so we fall into the same issue as described in A.
> For those Micron memories, depending on the part number (telling us whether the
> Reset pin is available...) the 12h op code stands for two different operations:
> - either 3-byte address Page Program x-4-4 (the standard 38h op code for this
>   operation is not available and there is no op code for 4-byte Page Program
>   1-1-1)
> - either 4-byte address Page Program 1-1-1 (the standard command associated to
>   the 12h op code)
> 
> Since they are different part numbers of the same memory family, all those
> memories once again share the very same JEDEC ID and there no mean to 
> dynamically discover the actual part number (or I didn't find such a mean yet).
> Anyway, even knowing the part number, depending on the result, the 4-byte Page
> Program 1-1-1 operation is simply not supported.
> 
> Hopefully, for all part numbers, 4-byte Page Program 1-1-4 is always supported
> by the 34h op code, which is the standard hope code for this operation.
> 
> Then with SPI controllers which explicitly supports Page Program 1-1-4, we 
> could implement something to fix the Page Program operation on Micron memory
> above 16MB. Hence we have a solution!
> 
> There are just examples, I guess we could find others. My point is that
> currently we can't use some QPSI memories properly and its a blocking issue.
> The whole series should be see as a bug fixes enabler rather than a performance
> improvement.
> 
> 
>> The _reg_ stuff is probably negligible altogether,
>>
>> Lastly the faster transfers of address bytes seem to be achieved with
>> increasingly longer command codes given how much the maximum command
>> length increased. So even in a page write where the address is a few %
>> of the transfer the benefit of these extra modes is dubious.
> I'm not sure to understand this point but I guess you refer to:
> -#define	MAX_CMD_SIZE		6
> +#define	MAX_CMD_SIZE		16
> 
> If so, the actual command size doesn't change at all, the increase of this
> macro value is justified by another reason. Indeed, let's have a look into
> the m25p80_read_reg(): before the patch it was implemented using
> spi_write_then_read(). This later function uses an intermediate buffer,
> which might be kmalloc() allocated, to transfer data with spi_sync() and calls
> memcpy() to copy data from/to this imtermediate buffer to/from buffers
> provided as function parameters (txbuf and rxbuf).
> As the comment says, the purpose of this intermediate buffer is to be
> "DMA-safe".
> 
> Then, after the patch, spi_write_then_read() is no longer used but we still
> need a DMA-safe buffer to perform the data transfer with spi_sync(). This can
> be achieve with the flash->command[] buffer; we just need to increase its size
> some more since it is now also filled with the read data whereas it was only
> filled with the command/address/dummy data before.
> 
>>
>> Overall I wonder how much it is worthwhile to complicate the code to
>> get all these modes in every single function.
>>
>> Thanks
>>
>> Michal
>>
> 
> Best regards,
> 
> Cyrille
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands
  2016-06-27 12:37       ` Cyrille Pitchen
@ 2016-06-28  7:57           ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
  0 siblings, 0 replies; 26+ messages in thread
From: Krzeminski, Marcin (Nokia - PL/Wroclaw) @ 2016-06-28  7:57 UTC (permalink / raw)
  To: Cyrille Pitchen, Michal Suchanek
  Cc: Brian Norris, MTD Maling List, Marek Vašut, Boris Brezillon,
	nicolas.ferre, Linux Kernel Mailing List

Hello,

> -----Original Message-----
> From: Cyrille Pitchen [mailto:cyrille.pitchen@atmel.com]
> Sent: Monday, June 27, 2016 2:38 PM
> To: Michal Suchanek <hramrach@gmail.com>
> Cc: Brian Norris <computersforpeace@gmail.com>; MTD Maling List <linux-
> mtd@lists.infradead.org>; Marek Vašut <marex@denx.de>; Boris Brezillon
> <boris.brezillon@free-electrons.com>; nicolas.ferre@atmel.com; Linux
> Kernel Mailing List <linux-kernel@vger.kernel.org>; Krzeminski, Marcin
> (Nokia - PL/Wroclaw) <marcin.krzeminski@nokia.com>
> Subject: Re: [PATCH 7/9] mtd: m25p80: add support of dual and quad spi
> protocols to all commands
> 
> Hi all,
> 
> + Marcin,
> 
> According to Marcin's series of patches
> "mtd: spi-nor: DUAL/QUAD I/O read modes implementation"
> and the Spansion/Cypress S25FS512S datasheet, new Cypress QSPI
> memories no longer supports Fast Read 1-1-2 or Fast Read 1-1-4 but only Fast
> Read x-2-2 and Fast Read x-4-4.
> 
> Hence one more example of the need to extend the spi-nor framework to
> support QSPI memories correctly.
> 
> Also, looking at the datasheets of s25fL512s vs s25fS512s, the default factory
> settings for the number of (mode + wait state) cycles has changed for Fast
> Read 1-4-4 (EBh /ECh).
> 
> default 	mode 	wait state
> L512s: 		2	4
> S512s:		2	8
> 
> Those factory default settings are provided by the Basic Flash Parameter
> Table from the Serial Flash Discoverable Parameters (JESD216). SFDP tables
> seem to be supported by both families.
> 
Hello,

It is also possible to read number of dummy cycles (wait states) from the flash itself.
On this Spansion and Microns(eg. N25Q512) user can change and program
dummy cycles count. If we will use default (or hardcoded values like in spi-nor),
we will fail fast reads in this case. I will probably implement this in spi-nor for Spansion,
since I have use case with low SPI frequency, and dummy cycles count  could be 0.

> Hence this series of patches actually adds support to the new
> Spansion/Cypress QSPI memories.
> 
> We will just have to care about setting a proper value in the mode cycles to
> avoid entering the continuous read mode by mistake.
> Looking at the JESD216 specification the 0xff value is likely to be a common
> value for all manufacturers to tell the QSPI memory not to enter (or leave) its
> continuous read/performance enhancement mode.
> 
> Entering the continuous read mode tells the QSPI memory that the op code
> value will be implicit in the following command, which then will start from the
> address cycles. The continuous mode is mainly (only ?) use for XIP
> application.
> 
> Adding support to the continuous read mode might be interesting but should
> be discussed in a dedicated thread.
> 
> Best regards,
> 
> Cyrille
There is one more think in fs512 and fl512 family. Those devices can have 8x4kB
sectors  at the beginning or end of the flash. Those sectors can not be erased with
erase sector cmd(0xd8), but need to be erased with erase_4k(0x20).
To allow erasing those sector mtd eraseregions are needed. According to fs512s
datasheet conforming to JESD216 allows to read flash map, so it could
be possible to create regions automatically. 

Best regards,
Marcin
> 
> Le 27/06/2016 11:52, Cyrille Pitchen a écrit :
> > Le 23/06/2016 22:35, Michal Suchanek a écrit :
> >> Hello,
> >>
> >> this patch is kind of awesome.
> >>
> >> I have a few practical concerns however.
> >>
> >> On 20 June 2016 at 18:50, Cyrille Pitchen <cyrille.pitchen@atmel.com>
> wrote:
> >>> Before this patch, m25p80_read() supported few SPI protocols:
> >>> - regular SPI 1-1-1
> >>> - SPI Dual Output 1-1-2
> >>> - SPI Quad Output 1-1-4
> >>> On the other hand, all other m25p80_*() hooks only supported SPI 1-1-1.
> >>
> >> Under typical use my estimate is that huge majority of data is
> >> transferred in _read() seconded by _write().
> >>
> >> As I understand it the n-n-n means how many bits you transfer in
> >> parallel when sending command-address-data.
> >>
> >> In _read() the command and data overhead is negligible when you can
> >> read kilobytes at once. So difference between 1-1-4 and 4-4-4 is not
> >> meaningful performance-wise. Are there flash chips that support one
> >> but not the other?
> >>
> >> For _write() the benefits are even harder to assess. You can
> >> presumably write at n-n-4 or n-n-2 if your controller and flash
> >> supports it transferring the page faster. And then spend possibly
> >> large amount of time waiting for the flash to get ready again. If the
> >> programming time is fixed transferring the page faster may or may not
> >> have benefits. It may at least free the bus for other devices to use.
> >>
> >
> > This series of patches is not supposed to improve performances. Lets
> > take a Macronix mx25l25673g as an example with a 512-byte read (3-byte
> address):
> >
> > 1 - Fast Read 1-1-4 (0 mode cycle, 8 wait states)
> > command: 8 cycles
> > address/mode/dummy: (3 * 8) + 0 + 8 = 32 cycles
> > data: (512 * 8) / 4 = 1024 cycles
> > total: 8 + 32 + 1024 = 1064 cycles
> >
> > 2 - Fast Read 1-4-4 (2 mode cycles, 4 wait states)
> > command: 8 cycles
> > address/mode/dummy: (3 * 8) / 4 + 2 + 4 = 12 cycles
> > data: (512 * 8) / 4 = 1024 cycles
> > total: 8 + 12 + 1024 = 1044 cycles
> >
> > theoretical benefit: < 2%
> >
> >
> > The real purpose of the whole series is to provide solutions to some
> > known issues.
> >
> > A - Currently the spi-nor force use the B7h op code to enter the
> > 4-byte address mode of > 16MB memories. This mode is state-full; it
> > changes the internal state of the SPI memory. Once in its 4-byte mode,
> > the regular 3-byte Fast Read, Page Program and Sector Erase op codes
> > are expected to be followed by a 4-byte address. Then if a spurious
> > reset occurs, some bootloaders might be stuck as they still expect to use 3-
> byte addresses.
> >
> > On the other hand, when available, the 4-byte address instruction set
> > is stateless. So we'd rather use it whenever possible. It's the
> > purpose of patch 2. However adding the SPI_NOR_4B_OPCODES on some
> > memory entry is not always possible. Again, let's take the Macronix
> mx25l25673g memory as an example:
> > it shares the very same JEDEC ID with the older mx25l25635e. The 73g
> > supports the 4-byte address instruction set whereas the 35e doesn't.
> > Hence we can't add the SPI_NOR_4B_OPCODES on the 35e entry without
> introducing a bug.
> >
> > My first approach was to patch the spi-nor framework to allow two or
> > more memory entries to share the same JEDEC ID and to select the right
> > entry according to the compatible DT string. This solution was rejected.
> >
> > Then my latest solution is to rely on the SFDP tables: the 73g
> > supports the optional 4-byte Address Instruction Table whereas the 35e
> > doesn't support SFDP table at all. Hence, when this table is
> > successfully parsed on the 73g, we know we can safely use the 4-byte
> > op codes. For the 35e case, we still enter the 4-byte address mode as
> before since there is no other solution...
> > So with this series of patches, no regression with 35e memories but a
> > solution to properly use 73g memories without changing their internal
> > state: the bootloader is now happy.
> >
> >
> > B - Setting the old 'mode' argument of spi_nor_read() to SPI_NOR_QUAD
> > just told the SPI controller supports Fast Read 1-1-4 but nothing
> > about whether it also supports Page Program x-y-4 commands. It is
> > interesting to make the difference between read and write hardware
> > capabilities. Indeed we cannot assume that if a controller can do Fast
> > Read 1-1-4 operations then it can also perform Page Program 1-1-4. I'm
> > pretty sure this statement is false. So let's the SPI controller explicitly
> declares its read and write hardware capabilities.
> >
> > Once again, I'm not targeting performance improvement with Page
> Program.
> > However, using other SPI protocols when available helps to deal with
> > some memory quirks.
> >
> > This time, let's take the case of Micron n25q512*. Those memory are >
> > 16MB so we fall into the same issue as described in A.
> > For those Micron memories, depending on the part number (telling us
> > whether the Reset pin is available...) the 12h op code stands for two
> different operations:
> > - either 3-byte address Page Program x-4-4 (the standard 38h op code for
> this
> >   operation is not available and there is no op code for 4-byte Page Program
> >   1-1-1)
> > - either 4-byte address Page Program 1-1-1 (the standard command
> associated to
> >   the 12h op code)
> >
> > Since they are different part numbers of the same memory family, all
> > those memories once again share the very same JEDEC ID and there no
> > mean to dynamically discover the actual part number (or I didn't find such a
> mean yet).
> > Anyway, even knowing the part number, depending on the result, the
> > 4-byte Page Program 1-1-1 operation is simply not supported.
> >
> > Hopefully, for all part numbers, 4-byte Page Program 1-1-4 is always
> > supported by the 34h op code, which is the standard hope code for this
> operation.
> >
> > Then with SPI controllers which explicitly supports Page Program
> > 1-1-4, we could implement something to fix the Page Program operation
> > on Micron memory above 16MB. Hence we have a solution!
> >
> > There are just examples, I guess we could find others. My point is
> > that currently we can't use some QPSI memories properly and its a blocking
> issue.
> > The whole series should be see as a bug fixes enabler rather than a
> > performance improvement.
> >
> >
> >> The _reg_ stuff is probably negligible altogether,
> >>
> >> Lastly the faster transfers of address bytes seem to be achieved with
> >> increasingly longer command codes given how much the maximum
> command
> >> length increased. So even in a page write where the address is a few
> >> % of the transfer the benefit of these extra modes is dubious.
> > I'm not sure to understand this point but I guess you refer to:
> > -#define	MAX_CMD_SIZE		6
> > +#define	MAX_CMD_SIZE		16
> >
> > If so, the actual command size doesn't change at all, the increase of
> > this macro value is justified by another reason. Indeed, let's have a
> > look into the m25p80_read_reg(): before the patch it was implemented
> > using spi_write_then_read(). This later function uses an intermediate
> > buffer, which might be kmalloc() allocated, to transfer data with
> > spi_sync() and calls
> > memcpy() to copy data from/to this imtermediate buffer to/from buffers
> > provided as function parameters (txbuf and rxbuf).
> > As the comment says, the purpose of this intermediate buffer is to be
> > "DMA-safe".
> >
> > Then, after the patch, spi_write_then_read() is no longer used but we
> > still need a DMA-safe buffer to perform the data transfer with
> > spi_sync(). This can be achieve with the flash->command[] buffer; we
> > just need to increase its size some more since it is now also filled
> > with the read data whereas it was only filled with the
> command/address/dummy data before.
> >
> >>
> >> Overall I wonder how much it is worthwhile to complicate the code to
> >> get all these modes in every single function.
> >>
> >> Thanks
> >>
> >> Michal
> >>
> >
> > Best regards,
> >
> > Cyrille
> >

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands
@ 2016-06-28  7:57           ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
  0 siblings, 0 replies; 26+ messages in thread
From: Krzeminski, Marcin (Nokia - PL/Wroclaw) @ 2016-06-28  7:57 UTC (permalink / raw)
  To: Cyrille Pitchen, Michal Suchanek
  Cc: Brian Norris, MTD Maling List, Marek Vašut, Boris Brezillon,
	nicolas.ferre, Linux Kernel Mailing List

Hello,

> -----Original Message-----
> From: Cyrille Pitchen [mailto:cyrille.pitchen@atmel.com]
> Sent: Monday, June 27, 2016 2:38 PM
> To: Michal Suchanek <hramrach@gmail.com>
> Cc: Brian Norris <computersforpeace@gmail.com>; MTD Maling List <linux-
> mtd@lists.infradead.org>; Marek Vašut <marex@denx.de>; Boris Brezillon
> <boris.brezillon@free-electrons.com>; nicolas.ferre@atmel.com; Linux
> Kernel Mailing List <linux-kernel@vger.kernel.org>; Krzeminski, Marcin
> (Nokia - PL/Wroclaw) <marcin.krzeminski@nokia.com>
> Subject: Re: [PATCH 7/9] mtd: m25p80: add support of dual and quad spi
> protocols to all commands
> 
> Hi all,
> 
> + Marcin,
> 
> According to Marcin's series of patches
> "mtd: spi-nor: DUAL/QUAD I/O read modes implementation"
> and the Spansion/Cypress S25FS512S datasheet, new Cypress QSPI
> memories no longer supports Fast Read 1-1-2 or Fast Read 1-1-4 but only Fast
> Read x-2-2 and Fast Read x-4-4.
> 
> Hence one more example of the need to extend the spi-nor framework to
> support QSPI memories correctly.
> 
> Also, looking at the datasheets of s25fL512s vs s25fS512s, the default factory
> settings for the number of (mode + wait state) cycles has changed for Fast
> Read 1-4-4 (EBh /ECh).
> 
> default 	mode 	wait state
> L512s: 		2	4
> S512s:		2	8
> 
> Those factory default settings are provided by the Basic Flash Parameter
> Table from the Serial Flash Discoverable Parameters (JESD216). SFDP tables
> seem to be supported by both families.
> 
Hello,

It is also possible to read number of dummy cycles (wait states) from the flash itself.
On this Spansion and Microns(eg. N25Q512) user can change and program
dummy cycles count. If we will use default (or hardcoded values like in spi-nor),
we will fail fast reads in this case. I will probably implement this in spi-nor for Spansion,
since I have use case with low SPI frequency, and dummy cycles count  could be 0.

> Hence this series of patches actually adds support to the new
> Spansion/Cypress QSPI memories.
> 
> We will just have to care about setting a proper value in the mode cycles to
> avoid entering the continuous read mode by mistake.
> Looking at the JESD216 specification the 0xff value is likely to be a common
> value for all manufacturers to tell the QSPI memory not to enter (or leave) its
> continuous read/performance enhancement mode.
> 
> Entering the continuous read mode tells the QSPI memory that the op code
> value will be implicit in the following command, which then will start from the
> address cycles. The continuous mode is mainly (only ?) use for XIP
> application.
> 
> Adding support to the continuous read mode might be interesting but should
> be discussed in a dedicated thread.
> 
> Best regards,
> 
> Cyrille
There is one more think in fs512 and fl512 family. Those devices can have 8x4kB
sectors  at the beginning or end of the flash. Those sectors can not be erased with
erase sector cmd(0xd8), but need to be erased with erase_4k(0x20).
To allow erasing those sector mtd eraseregions are needed. According to fs512s
datasheet conforming to JESD216 allows to read flash map, so it could
be possible to create regions automatically. 

Best regards,
Marcin
> 
> Le 27/06/2016 11:52, Cyrille Pitchen a écrit :
> > Le 23/06/2016 22:35, Michal Suchanek a écrit :
> >> Hello,
> >>
> >> this patch is kind of awesome.
> >>
> >> I have a few practical concerns however.
> >>
> >> On 20 June 2016 at 18:50, Cyrille Pitchen <cyrille.pitchen@atmel.com>
> wrote:
> >>> Before this patch, m25p80_read() supported few SPI protocols:
> >>> - regular SPI 1-1-1
> >>> - SPI Dual Output 1-1-2
> >>> - SPI Quad Output 1-1-4
> >>> On the other hand, all other m25p80_*() hooks only supported SPI 1-1-1.
> >>
> >> Under typical use my estimate is that huge majority of data is
> >> transferred in _read() seconded by _write().
> >>
> >> As I understand it the n-n-n means how many bits you transfer in
> >> parallel when sending command-address-data.
> >>
> >> In _read() the command and data overhead is negligible when you can
> >> read kilobytes at once. So difference between 1-1-4 and 4-4-4 is not
> >> meaningful performance-wise. Are there flash chips that support one
> >> but not the other?
> >>
> >> For _write() the benefits are even harder to assess. You can
> >> presumably write at n-n-4 or n-n-2 if your controller and flash
> >> supports it transferring the page faster. And then spend possibly
> >> large amount of time waiting for the flash to get ready again. If the
> >> programming time is fixed transferring the page faster may or may not
> >> have benefits. It may at least free the bus for other devices to use.
> >>
> >
> > This series of patches is not supposed to improve performances. Lets
> > take a Macronix mx25l25673g as an example with a 512-byte read (3-byte
> address):
> >
> > 1 - Fast Read 1-1-4 (0 mode cycle, 8 wait states)
> > command: 8 cycles
> > address/mode/dummy: (3 * 8) + 0 + 8 = 32 cycles
> > data: (512 * 8) / 4 = 1024 cycles
> > total: 8 + 32 + 1024 = 1064 cycles
> >
> > 2 - Fast Read 1-4-4 (2 mode cycles, 4 wait states)
> > command: 8 cycles
> > address/mode/dummy: (3 * 8) / 4 + 2 + 4 = 12 cycles
> > data: (512 * 8) / 4 = 1024 cycles
> > total: 8 + 12 + 1024 = 1044 cycles
> >
> > theoretical benefit: < 2%
> >
> >
> > The real purpose of the whole series is to provide solutions to some
> > known issues.
> >
> > A - Currently the spi-nor force use the B7h op code to enter the
> > 4-byte address mode of > 16MB memories. This mode is state-full; it
> > changes the internal state of the SPI memory. Once in its 4-byte mode,
> > the regular 3-byte Fast Read, Page Program and Sector Erase op codes
> > are expected to be followed by a 4-byte address. Then if a spurious
> > reset occurs, some bootloaders might be stuck as they still expect to use 3-
> byte addresses.
> >
> > On the other hand, when available, the 4-byte address instruction set
> > is stateless. So we'd rather use it whenever possible. It's the
> > purpose of patch 2. However adding the SPI_NOR_4B_OPCODES on some
> > memory entry is not always possible. Again, let's take the Macronix
> mx25l25673g memory as an example:
> > it shares the very same JEDEC ID with the older mx25l25635e. The 73g
> > supports the 4-byte address instruction set whereas the 35e doesn't.
> > Hence we can't add the SPI_NOR_4B_OPCODES on the 35e entry without
> introducing a bug.
> >
> > My first approach was to patch the spi-nor framework to allow two or
> > more memory entries to share the same JEDEC ID and to select the right
> > entry according to the compatible DT string. This solution was rejected.
> >
> > Then my latest solution is to rely on the SFDP tables: the 73g
> > supports the optional 4-byte Address Instruction Table whereas the 35e
> > doesn't support SFDP table at all. Hence, when this table is
> > successfully parsed on the 73g, we know we can safely use the 4-byte
> > op codes. For the 35e case, we still enter the 4-byte address mode as
> before since there is no other solution...
> > So with this series of patches, no regression with 35e memories but a
> > solution to properly use 73g memories without changing their internal
> > state: the bootloader is now happy.
> >
> >
> > B - Setting the old 'mode' argument of spi_nor_read() to SPI_NOR_QUAD
> > just told the SPI controller supports Fast Read 1-1-4 but nothing
> > about whether it also supports Page Program x-y-4 commands. It is
> > interesting to make the difference between read and write hardware
> > capabilities. Indeed we cannot assume that if a controller can do Fast
> > Read 1-1-4 operations then it can also perform Page Program 1-1-4. I'm
> > pretty sure this statement is false. So let's the SPI controller explicitly
> declares its read and write hardware capabilities.
> >
> > Once again, I'm not targeting performance improvement with Page
> Program.
> > However, using other SPI protocols when available helps to deal with
> > some memory quirks.
> >
> > This time, let's take the case of Micron n25q512*. Those memory are >
> > 16MB so we fall into the same issue as described in A.
> > For those Micron memories, depending on the part number (telling us
> > whether the Reset pin is available...) the 12h op code stands for two
> different operations:
> > - either 3-byte address Page Program x-4-4 (the standard 38h op code for
> this
> >   operation is not available and there is no op code for 4-byte Page Program
> >   1-1-1)
> > - either 4-byte address Page Program 1-1-1 (the standard command
> associated to
> >   the 12h op code)
> >
> > Since they are different part numbers of the same memory family, all
> > those memories once again share the very same JEDEC ID and there no
> > mean to dynamically discover the actual part number (or I didn't find such a
> mean yet).
> > Anyway, even knowing the part number, depending on the result, the
> > 4-byte Page Program 1-1-1 operation is simply not supported.
> >
> > Hopefully, for all part numbers, 4-byte Page Program 1-1-4 is always
> > supported by the 34h op code, which is the standard hope code for this
> operation.
> >
> > Then with SPI controllers which explicitly supports Page Program
> > 1-1-4, we could implement something to fix the Page Program operation
> > on Micron memory above 16MB. Hence we have a solution!
> >
> > There are just examples, I guess we could find others. My point is
> > that currently we can't use some QPSI memories properly and its a blocking
> issue.
> > The whole series should be see as a bug fixes enabler rather than a
> > performance improvement.
> >
> >
> >> The _reg_ stuff is probably negligible altogether,
> >>
> >> Lastly the faster transfers of address bytes seem to be achieved with
> >> increasingly longer command codes given how much the maximum
> command
> >> length increased. So even in a page write where the address is a few
> >> % of the transfer the benefit of these extra modes is dubious.
> > I'm not sure to understand this point but I guess you refer to:
> > -#define	MAX_CMD_SIZE		6
> > +#define	MAX_CMD_SIZE		16
> >
> > If so, the actual command size doesn't change at all, the increase of
> > this macro value is justified by another reason. Indeed, let's have a
> > look into the m25p80_read_reg(): before the patch it was implemented
> > using spi_write_then_read(). This later function uses an intermediate
> > buffer, which might be kmalloc() allocated, to transfer data with
> > spi_sync() and calls
> > memcpy() to copy data from/to this imtermediate buffer to/from buffers
> > provided as function parameters (txbuf and rxbuf).
> > As the comment says, the purpose of this intermediate buffer is to be
> > "DMA-safe".
> >
> > Then, after the patch, spi_write_then_read() is no longer used but we
> > still need a DMA-safe buffer to perform the data transfer with
> > spi_sync(). This can be achieve with the flash->command[] buffer; we
> > just need to increase its size some more since it is now also filled
> > with the read data whereas it was only filled with the
> command/address/dummy data before.
> >
> >>
> >> Overall I wonder how much it is worthwhile to complicate the code to
> >> get all these modes in every single function.
> >>
> >> Thanks
> >>
> >> Michal
> >>
> >
> > Best regards,
> >
> > Cyrille
> >


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/9] mtd: spi-nor: add an alternative method to support memory >16MiB
  2016-10-05 12:53     ` Cyrille Pitchen
@ 2016-10-06 10:03         ` Vignesh R
  0 siblings, 0 replies; 26+ messages in thread
From: Vignesh R @ 2016-10-06 10:03 UTC (permalink / raw)
  To: Cyrille Pitchen, computersforpeace, linux-mtd
  Cc: marex, boris.brezillon, nicolas.ferre, linux-kernel



On Wednesday 05 October 2016 06:23 PM, Cyrille Pitchen wrote:
> Hi Vignesh,
> 
> Le 05/10/2016 à 14:12, Vignesh R a écrit :
>> Hi,
>>
>>
>> On Tuesday 04 October 2016 10:07 PM, Cyrille Pitchen wrote:
>> [...]
>>>  static struct seq_rw_config stfsm_s25fl_write4_configs[] = {
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>> index 5c87b2d99507..423448c1c7a8 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -75,6 +75,10 @@ struct flash_info {
>>>  					 * bit. Must be used with
>>>  					 * SPI_NOR_HAS_LOCK.
>>>  					 */
>>> +#define SPI_NOR_4B_OPCODES	BIT(10)	/*
>>> +					 * Use dedicated 4byte address op codes
>>> +					 * to support memory size above 128Mib.
>>> +					 */
>>
>>
>> I don't see this flag being added to any flash_info data for any m25p80
>> compatible flashes? Is that part of a separate series?
>>
> 
> Indeed, this flag is not used yet by any entry in the spi_nor_ids[] array.
> At first, I tested this patch with a Macronix mx25l25673g memory.
> This memory shares the very same JEDEC ID as the older mx25l25635e, so
> I added the SPI_NOR_4B_OPCODES flag in the mx25l25635e entry during my test.
> However I didn't commit this change since the actual mx25l25635e memory
> doesn't support The 4-byte address instruction test.
> 
> Also the mx25l25673g provides the optional 4-byte Address Instruction Table,
> as described in the JESD216 rev B specification, so patches 6 and 7 allow
> to use the 4-byte address op codes without setting the SPI_NOR_4B_OPCODES flag.
> 
> Hence the SPI_NOR_4B_OPCODES, will be useful for memories which don't
> provide the JESD216B 4-byte Address Instruction Table but still support
> the associated op code. It will have to be tested for each individual entry
> in the spi_nor_ids[] array, which can be done later.
> 
> Anyway, the 4-byte address op codes introduced by this patch are also used
> by patch 7 (parse SFDP 4-byte Address Instruction Table).

OK, thanks for the explanation!


-- 
Regards
Vignesh

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/9] mtd: spi-nor: add an alternative method to support memory >16MiB
@ 2016-10-06 10:03         ` Vignesh R
  0 siblings, 0 replies; 26+ messages in thread
From: Vignesh R @ 2016-10-06 10:03 UTC (permalink / raw)
  To: Cyrille Pitchen, computersforpeace, linux-mtd
  Cc: marex, boris.brezillon, nicolas.ferre, linux-kernel



On Wednesday 05 October 2016 06:23 PM, Cyrille Pitchen wrote:
> Hi Vignesh,
> 
> Le 05/10/2016 à 14:12, Vignesh R a écrit :
>> Hi,
>>
>>
>> On Tuesday 04 October 2016 10:07 PM, Cyrille Pitchen wrote:
>> [...]
>>>  static struct seq_rw_config stfsm_s25fl_write4_configs[] = {
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>> index 5c87b2d99507..423448c1c7a8 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -75,6 +75,10 @@ struct flash_info {
>>>  					 * bit. Must be used with
>>>  					 * SPI_NOR_HAS_LOCK.
>>>  					 */
>>> +#define SPI_NOR_4B_OPCODES	BIT(10)	/*
>>> +					 * Use dedicated 4byte address op codes
>>> +					 * to support memory size above 128Mib.
>>> +					 */
>>
>>
>> I don't see this flag being added to any flash_info data for any m25p80
>> compatible flashes? Is that part of a separate series?
>>
> 
> Indeed, this flag is not used yet by any entry in the spi_nor_ids[] array.
> At first, I tested this patch with a Macronix mx25l25673g memory.
> This memory shares the very same JEDEC ID as the older mx25l25635e, so
> I added the SPI_NOR_4B_OPCODES flag in the mx25l25635e entry during my test.
> However I didn't commit this change since the actual mx25l25635e memory
> doesn't support The 4-byte address instruction test.
> 
> Also the mx25l25673g provides the optional 4-byte Address Instruction Table,
> as described in the JESD216 rev B specification, so patches 6 and 7 allow
> to use the 4-byte address op codes without setting the SPI_NOR_4B_OPCODES flag.
> 
> Hence the SPI_NOR_4B_OPCODES, will be useful for memories which don't
> provide the JESD216B 4-byte Address Instruction Table but still support
> the associated op code. It will have to be tested for each individual entry
> in the spi_nor_ids[] array, which can be done later.
> 
> Anyway, the 4-byte address op codes introduced by this patch are also used
> by patch 7 (parse SFDP 4-byte Address Instruction Table).

OK, thanks for the explanation!


-- 
Regards
Vignesh

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/9] mtd: spi-nor: add an alternative method to support memory >16MiB
  2016-10-05 12:12   ` Vignesh R
@ 2016-10-05 12:53     ` Cyrille Pitchen
  2016-10-06 10:03         ` Vignesh R
  0 siblings, 1 reply; 26+ messages in thread
From: Cyrille Pitchen @ 2016-10-05 12:53 UTC (permalink / raw)
  To: Vignesh R, computersforpeace, linux-mtd
  Cc: marex, boris.brezillon, nicolas.ferre, linux-kernel

Hi Vignesh,

Le 05/10/2016 à 14:12, Vignesh R a écrit :
> Hi,
> 
> 
> On Tuesday 04 October 2016 10:07 PM, Cyrille Pitchen wrote:
> [...]
>>  static struct seq_rw_config stfsm_s25fl_write4_configs[] = {
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 5c87b2d99507..423448c1c7a8 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -75,6 +75,10 @@ struct flash_info {
>>  					 * bit. Must be used with
>>  					 * SPI_NOR_HAS_LOCK.
>>  					 */
>> +#define SPI_NOR_4B_OPCODES	BIT(10)	/*
>> +					 * Use dedicated 4byte address op codes
>> +					 * to support memory size above 128Mib.
>> +					 */
> 
> 
> I don't see this flag being added to any flash_info data for any m25p80
> compatible flashes? Is that part of a separate series?
> 

Indeed, this flag is not used yet by any entry in the spi_nor_ids[] array.
At first, I tested this patch with a Macronix mx25l25673g memory.
This memory shares the very same JEDEC ID as the older mx25l25635e, so
I added the SPI_NOR_4B_OPCODES flag in the mx25l25635e entry during my test.
However I didn't commit this change since the actual mx25l25635e memory
doesn't support The 4-byte address instruction test.

Also the mx25l25673g provides the optional 4-byte Address Instruction Table,
as described in the JESD216 rev B specification, so patches 6 and 7 allow
to use the 4-byte address op codes without setting the SPI_NOR_4B_OPCODES flag.

Hence the SPI_NOR_4B_OPCODES, will be useful for memories which don't
provide the JESD216B 4-byte Address Instruction Table but still support
the associated op code. It will have to be tested for each individual entry
in the spi_nor_ids[] array, which can be done later.

Anyway, the 4-byte address op codes introduced by this patch are also used
by patch 7 (parse SFDP 4-byte Address Instruction Table).

Best regards,

Cyrille

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/9] mtd: spi-nor: add an alternative method to support memory >16MiB
  2016-10-04 16:37 ` [PATCH 2/9] mtd: spi-nor: add an alternative method to support memory >16MiB Cyrille Pitchen
@ 2016-10-05 12:12   ` Vignesh R
  2016-10-05 12:53     ` Cyrille Pitchen
  0 siblings, 1 reply; 26+ messages in thread
From: Vignesh R @ 2016-10-05 12:12 UTC (permalink / raw)
  To: Cyrille Pitchen, computersforpeace, linux-mtd
  Cc: marex, boris.brezillon, nicolas.ferre, linux-kernel

Hi,


On Tuesday 04 October 2016 10:07 PM, Cyrille Pitchen wrote:
[...]
>  static struct seq_rw_config stfsm_s25fl_write4_configs[] = {
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 5c87b2d99507..423448c1c7a8 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -75,6 +75,10 @@ struct flash_info {
>  					 * bit. Must be used with
>  					 * SPI_NOR_HAS_LOCK.
>  					 */
> +#define SPI_NOR_4B_OPCODES	BIT(10)	/*
> +					 * Use dedicated 4byte address op codes
> +					 * to support memory size above 128Mib.
> +					 */


I don't see this flag being added to any flash_info data for any m25p80
compatible flashes? Is that part of a separate series?

-- 
Regards
Vignesh

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 2/9] mtd: spi-nor: add an alternative method to support memory >16MiB
  2016-10-04 16:37 [PATCH 0/9] mtd: spi-nor: parse SFDP tables to setup (Q)SPI memories Cyrille Pitchen
@ 2016-10-04 16:37 ` Cyrille Pitchen
  2016-10-05 12:12   ` Vignesh R
  0 siblings, 1 reply; 26+ messages in thread
From: Cyrille Pitchen @ 2016-10-04 16:37 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

This patch provides an alternative mean to support memory above 16MiB
(128Mib) by replacing 3byte address op codes by their associated 4byte
address versions.

Using the dedicated 4byte address op codes doesn't change the internal
state of the SPI NOR memory as opposed to using other means such as
updating a Base Address Register (BAR) and sending command to enter/leave
the 4byte mode.

Hence when a CPU reset occurs, early bootloaders don't need to be aware
of BAR value or 4byte mode being enabled: they can still access the first
16MiB of the SPI NOR memory using the regular 3byte address op codes.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/mtd/devices/serial_flash_cmds.h |   7 ---
 drivers/mtd/devices/st_spi_fsm.c        |  28 ++++-----
 drivers/mtd/spi-nor/spi-nor.c           | 104 +++++++++++++++++++++++++-------
 include/linux/mtd/spi-nor.h             |  22 +++++--
 4 files changed, 113 insertions(+), 48 deletions(-)

diff --git a/drivers/mtd/devices/serial_flash_cmds.h b/drivers/mtd/devices/serial_flash_cmds.h
index f59a125295d0..8b81e15105dd 100644
--- a/drivers/mtd/devices/serial_flash_cmds.h
+++ b/drivers/mtd/devices/serial_flash_cmds.h
@@ -18,19 +18,12 @@
 #define SPINOR_OP_RDVCR		0x85
 
 /* JEDEC Standard - Serial Flash Discoverable Parmeters (SFDP) Commands */
-#define SPINOR_OP_READ_1_2_2	0xbb	/* DUAL I/O READ */
-#define SPINOR_OP_READ_1_4_4	0xeb	/* QUAD I/O READ */
-
 #define SPINOR_OP_WRITE		0x02	/* PAGE PROGRAM */
 #define SPINOR_OP_WRITE_1_1_2	0xa2	/* DUAL INPUT PROGRAM */
 #define SPINOR_OP_WRITE_1_2_2	0xd2	/* DUAL INPUT EXT PROGRAM */
 #define SPINOR_OP_WRITE_1_1_4	0x32	/* QUAD INPUT PROGRAM */
 #define SPINOR_OP_WRITE_1_4_4	0x12	/* QUAD INPUT EXT PROGRAM */
 
-/* READ commands with 32-bit addressing */
-#define SPINOR_OP_READ4_1_2_2	0xbc
-#define SPINOR_OP_READ4_1_4_4	0xec
-
 /* Configuration flags */
 #define FLASH_FLAG_SINGLE	0x000000ff
 #define FLASH_FLAG_READ_WRITE	0x00000001
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 5454b4113589..804313a33f2b 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -507,13 +507,13 @@ static struct seq_rw_config n25q_read3_configs[] = {
  *	- 'FAST' variants configured for 8 dummy cycles (see note above.)
  */
 static struct seq_rw_config n25q_read4_configs[] = {
-	{FLASH_FLAG_READ_1_4_4, SPINOR_OP_READ4_1_4_4,	0, 4, 4, 0x00, 0, 8},
-	{FLASH_FLAG_READ_1_1_4, SPINOR_OP_READ4_1_1_4,	0, 1, 4, 0x00, 0, 8},
-	{FLASH_FLAG_READ_1_2_2, SPINOR_OP_READ4_1_2_2,	0, 2, 2, 0x00, 0, 8},
-	{FLASH_FLAG_READ_1_1_2, SPINOR_OP_READ4_1_1_2,	0, 1, 2, 0x00, 0, 8},
-	{FLASH_FLAG_READ_FAST,	SPINOR_OP_READ4_FAST,	0, 1, 1, 0x00, 0, 8},
-	{FLASH_FLAG_READ_WRITE, SPINOR_OP_READ4,	0, 1, 1, 0x00, 0, 0},
-	{0x00,			0,			0, 0, 0, 0x00, 0, 0},
+	{FLASH_FLAG_READ_1_4_4, SPINOR_OP_READ_1_4_4_4B, 0, 4, 4, 0x00, 0, 8},
+	{FLASH_FLAG_READ_1_1_4, SPINOR_OP_READ_1_1_4_4B, 0, 1, 4, 0x00, 0, 8},
+	{FLASH_FLAG_READ_1_2_2, SPINOR_OP_READ_1_2_2_4B, 0, 2, 2, 0x00, 0, 8},
+	{FLASH_FLAG_READ_1_1_2, SPINOR_OP_READ_1_1_2_4B, 0, 1, 2, 0x00, 0, 8},
+	{FLASH_FLAG_READ_FAST,	SPINOR_OP_READ_FAST_4B,  0, 1, 1, 0x00, 0, 8},
+	{FLASH_FLAG_READ_WRITE, SPINOR_OP_READ_4B,       0, 1, 1, 0x00, 0, 0},
+	{0x00,			0,                       0, 0, 0, 0x00, 0, 0},
 };
 
 /*
@@ -553,13 +553,13 @@ static int stfsm_mx25_en_32bit_addr_seq(struct stfsm_seq *seq)
  * entering a state that is incompatible with the SPIBoot Controller.
  */
 static struct seq_rw_config stfsm_s25fl_read4_configs[] = {
-	{FLASH_FLAG_READ_1_4_4,  SPINOR_OP_READ4_1_4_4,  0, 4, 4, 0x00, 2, 4},
-	{FLASH_FLAG_READ_1_1_4,  SPINOR_OP_READ4_1_1_4,  0, 1, 4, 0x00, 0, 8},
-	{FLASH_FLAG_READ_1_2_2,  SPINOR_OP_READ4_1_2_2,  0, 2, 2, 0x00, 4, 0},
-	{FLASH_FLAG_READ_1_1_2,  SPINOR_OP_READ4_1_1_2,  0, 1, 2, 0x00, 0, 8},
-	{FLASH_FLAG_READ_FAST,   SPINOR_OP_READ4_FAST,   0, 1, 1, 0x00, 0, 8},
-	{FLASH_FLAG_READ_WRITE,  SPINOR_OP_READ4,        0, 1, 1, 0x00, 0, 0},
-	{0x00,                   0,                      0, 0, 0, 0x00, 0, 0},
+	{FLASH_FLAG_READ_1_4_4,  SPINOR_OP_READ_1_4_4_4B,  0, 4, 4, 0x00, 2, 4},
+	{FLASH_FLAG_READ_1_1_4,  SPINOR_OP_READ_1_1_4_4B,  0, 1, 4, 0x00, 0, 8},
+	{FLASH_FLAG_READ_1_2_2,  SPINOR_OP_READ_1_2_2_4B,  0, 2, 2, 0x00, 4, 0},
+	{FLASH_FLAG_READ_1_1_2,  SPINOR_OP_READ_1_1_2_4B,  0, 1, 2, 0x00, 0, 8},
+	{FLASH_FLAG_READ_FAST,   SPINOR_OP_READ_FAST_4B,   0, 1, 1, 0x00, 0, 8},
+	{FLASH_FLAG_READ_WRITE,  SPINOR_OP_READ_4B,        0, 1, 1, 0x00, 0, 0},
+	{0x00,                   0,                        0, 0, 0, 0x00, 0, 0},
 };
 
 static struct seq_rw_config stfsm_s25fl_write4_configs[] = {
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 5c87b2d99507..423448c1c7a8 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -75,6 +75,10 @@ struct flash_info {
 					 * bit. Must be used with
 					 * SPI_NOR_HAS_LOCK.
 					 */
+#define SPI_NOR_4B_OPCODES	BIT(10)	/*
+					 * Use dedicated 4byte address op codes
+					 * to support memory size above 128Mib.
+					 */
 };
 
 #define JEDEC_MFR(info)	((info)->id[0])
@@ -188,6 +192,81 @@ static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
 	return mtd->priv;
 }
 
+
+struct spi_nor_address_entry {
+	u8	src_opcode;
+	u8	dst_opcode;
+};
+
+static u8 spi_nor_convert_opcode(u8 opcode,
+				 const struct spi_nor_address_entry *entries,
+				 size_t num_entries)
+{
+	int min, max;
+
+	min = 0;
+	max = num_entries - 1;
+	while (min <= max) {
+		int mid = (min + max) >> 1;
+		const struct spi_nor_address_entry *entry = &entries[mid];
+
+		if (opcode == entry->src_opcode)
+			return entry->dst_opcode;
+
+		if (opcode < entry->src_opcode)
+			max = mid - 1;
+		else
+			min = mid + 1;
+	}
+
+	/* No conversion found */
+	return opcode;
+}
+
+static u8 spi_nor_3to4_opcode(u8 opcode)
+{
+	/* MUST be sorted by 3byte opcode */
+#define ENTRY_3TO4(_opcode)	{ _opcode, _opcode##_4B }
+	static const struct spi_nor_address_entry spi_nor_3to4_table[] = {
+		ENTRY_3TO4(SPINOR_OP_PP),		/* 0x02 */
+		ENTRY_3TO4(SPINOR_OP_READ),		/* 0x03 */
+		ENTRY_3TO4(SPINOR_OP_READ_FAST),	/* 0x0b */
+		ENTRY_3TO4(SPINOR_OP_BE_4K),		/* 0x20 */
+		ENTRY_3TO4(SPINOR_OP_PP_1_1_4),		/* 0x32 */
+		ENTRY_3TO4(SPINOR_OP_PP_1_4_4),		/* 0x38 */
+		ENTRY_3TO4(SPINOR_OP_READ_1_1_2),	/* 0x3b */
+		ENTRY_3TO4(SPINOR_OP_BE_32K),		/* 0x52 */
+		ENTRY_3TO4(SPINOR_OP_READ_1_1_4),	/* 0x6b */
+		ENTRY_3TO4(SPINOR_OP_READ_1_2_2),	/* 0xbb */
+		ENTRY_3TO4(SPINOR_OP_SE),		/* 0xd8 */
+		ENTRY_3TO4(SPINOR_OP_READ_1_4_4),	/* 0xeb */
+	};
+#undef ENTRY_3TO4
+
+	return spi_nor_convert_opcode(opcode, spi_nor_3to4_table,
+				      ARRAY_SIZE(spi_nor_3to4_table));
+}
+
+static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
+				      const struct flash_info *info)
+{
+	/* Do some manufacturer fixups first */
+	switch (JEDEC_MFR(info)) {
+	case SNOR_MFR_SPANSION:
+		/* No small sector erase for 4-byte command set */
+		nor->erase_opcode = SPINOR_OP_SE;
+		nor->mtd.erasesize = info->sector_size;
+		break;
+
+	default:
+		break;
+	}
+
+	nor->read_opcode	= spi_nor_3to4_opcode(nor->read_opcode);
+	nor->program_opcode	= spi_nor_3to4_opcode(nor->program_opcode);
+	nor->erase_opcode	= spi_nor_3to4_opcode(nor->erase_opcode);
+}
+
 /* Enable/disable 4-byte addressing mode. */
 static inline int set_4byte(struct spi_nor *nor, const struct flash_info *info,
 			    int enable)
@@ -1476,27 +1555,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	else if (mtd->size > 0x1000000) {
 		/* enable 4-byte addressing if the device exceeds 16MiB */
 		nor->addr_width = 4;
-		if (JEDEC_MFR(info) == SNOR_MFR_SPANSION) {
-			/* Dedicated 4-byte command set */
-			switch (nor->flash_read) {
-			case SPI_NOR_QUAD:
-				nor->read_opcode = SPINOR_OP_READ4_1_1_4;
-				break;
-			case SPI_NOR_DUAL:
-				nor->read_opcode = SPINOR_OP_READ4_1_1_2;
-				break;
-			case SPI_NOR_FAST:
-				nor->read_opcode = SPINOR_OP_READ4_FAST;
-				break;
-			case SPI_NOR_NORMAL:
-				nor->read_opcode = SPINOR_OP_READ4;
-				break;
-			}
-			nor->program_opcode = SPINOR_OP_PP_4B;
-			/* No small sector erase for 4-byte command set */
-			nor->erase_opcode = SPINOR_OP_SE_4B;
-			mtd->erasesize = info->sector_size;
-		} else
+		if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
+		    info->flags & SPI_NOR_4B_OPCODES)
+			spi_nor_set_4byte_opcodes(nor, info);
+		else
 			set_4byte(nor, info, 1);
 	} else {
 		nor->addr_width = 3;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index c425c7b4c2a0..8b02fd7864d0 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -43,9 +43,13 @@
 #define SPINOR_OP_WRSR		0x01	/* Write status register 1 byte */
 #define SPINOR_OP_READ		0x03	/* Read data bytes (low frequency) */
 #define SPINOR_OP_READ_FAST	0x0b	/* Read data bytes (high frequency) */
-#define SPINOR_OP_READ_1_1_2	0x3b	/* Read data bytes (Dual SPI) */
-#define SPINOR_OP_READ_1_1_4	0x6b	/* Read data bytes (Quad SPI) */
+#define SPINOR_OP_READ_1_1_2	0x3b	/* Read data bytes (Dual Output SPI) */
+#define SPINOR_OP_READ_1_2_2	0xbb	/* Read data bytes (Dual I/O SPI) */
+#define SPINOR_OP_READ_1_1_4	0x6b	/* Read data bytes (Quad Output SPI) */
+#define SPINOR_OP_READ_1_4_4	0xeb	/* Read data bytes (Quad I/O SPI) */
 #define SPINOR_OP_PP		0x02	/* Page program (up to 256 bytes) */
+#define SPINOR_OP_PP_1_1_4	0x32	/* Quad page program */
+#define SPINOR_OP_PP_1_4_4	0x38	/* Quad page program */
 #define SPINOR_OP_BE_4K		0x20	/* Erase 4KiB block */
 #define SPINOR_OP_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips */
 #define SPINOR_OP_BE_32K	0x52	/* Erase 32KiB block */
@@ -56,11 +60,17 @@
 #define SPINOR_OP_RDFSR		0x70	/* Read flag status register */
 
 /* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
-#define SPINOR_OP_READ4		0x13	/* Read data bytes (low frequency) */
-#define SPINOR_OP_READ4_FAST	0x0c	/* Read data bytes (high frequency) */
-#define SPINOR_OP_READ4_1_1_2	0x3c	/* Read data bytes (Dual SPI) */
-#define SPINOR_OP_READ4_1_1_4	0x6c	/* Read data bytes (Quad SPI) */
+#define SPINOR_OP_READ_4B	0x13	/* Read data bytes (low frequency) */
+#define SPINOR_OP_READ_FAST_4B	0x0c	/* Read data bytes (high frequency) */
+#define SPINOR_OP_READ_1_1_2_4B	0x3c	/* Read data bytes (Dual Output SPI) */
+#define SPINOR_OP_READ_1_2_2_4B	0xbc	/* Read data bytes (Dual I/O SPI) */
+#define SPINOR_OP_READ_1_1_4_4B	0x6c	/* Read data bytes (Quad Output SPI) */
+#define SPINOR_OP_READ_1_4_4_4B	0xec	/* Read data bytes (Quad I/O SPI) */
 #define SPINOR_OP_PP_4B		0x12	/* Page program (up to 256 bytes) */
+#define SPINOR_OP_PP_1_1_4_4B	0x34	/* Quad page program */
+#define SPINOR_OP_PP_1_4_4_4B	0x3e	/* Quad page program */
+#define SPINOR_OP_BE_4K_4B	0x21	/* Erase 4KiB block */
+#define SPINOR_OP_BE_32K_4B	0x5c	/* Erase 32KiB block */
 #define SPINOR_OP_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
 
 /* Used for SST flashes only. */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2016-10-06 10:07 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-20 16:49 [PATCH 0/9] mtd: spi-nor: parse SFDP tables as defined by JESD216B Cyrille Pitchen
2016-06-20 16:50 ` [PATCH 1/9] mtd: spi-nor: improve macronix_quad_enable() Cyrille Pitchen
2016-06-20 16:50 ` [PATCH 2/9] mtd: spi-nor: add an alternative method to support memory >16MiB Cyrille Pitchen
2016-06-20 16:50 ` [PATCH 3/9] Documentation: atmel-quadspi: add binding file for Atmel QSPI driver Cyrille Pitchen
2016-06-20 16:50 ` [PATCH 4/9] mtd: atmel-quadspi: add driver for Atmel QSPI controller Cyrille Pitchen
2016-06-20 16:50 ` [PATCH 5/9] mtd: spi-nor: add support of SPI protocols like SPI 1-2-2 and SPI 1-4-4 Cyrille Pitchen
2016-06-20 16:50 ` [PATCH 6/9] mtd: spi-nor: remove unused set_quad_mode() function Cyrille Pitchen
2016-06-20 16:50 ` [PATCH 7/9] mtd: m25p80: add support of dual and quad spi protocols to all commands Cyrille Pitchen
2016-06-23 20:35   ` Michal Suchanek
2016-06-23 20:46     ` Marek Vasut
2016-06-23 21:58       ` Michal Suchanek
2016-06-23 22:14         ` Marek Vasut
2016-06-23 22:43           ` Michal Suchanek
2016-06-23 22:50             ` Marek Vasut
2016-06-23 23:04               ` Michal Suchanek
2016-06-27  9:52     ` Cyrille Pitchen
2016-06-27 12:37       ` Cyrille Pitchen
2016-06-28  7:57         ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
2016-06-28  7:57           ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
2016-06-20 16:50 ` [PATCH 8/9] mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables Cyrille Pitchen
2016-06-20 16:50 ` [PATCH 9/9] mtd: spi-nor: parse SFDP 4-byte Address Instruction Table Cyrille Pitchen
2016-10-04 16:37 [PATCH 0/9] mtd: spi-nor: parse SFDP tables to setup (Q)SPI memories Cyrille Pitchen
2016-10-04 16:37 ` [PATCH 2/9] mtd: spi-nor: add an alternative method to support memory >16MiB Cyrille Pitchen
2016-10-05 12:12   ` Vignesh R
2016-10-05 12:53     ` Cyrille Pitchen
2016-10-06 10:03       ` Vignesh R
2016-10-06 10:03         ` Vignesh R

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.