All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories
@ 2016-04-13 17:23 Cyrille Pitchen
  2016-04-13 17:23 ` [PATCH RFC 1/8] mtd: spi-nor: add an alternative method to support memory >16MiB Cyrille Pitchen
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Cyrille Pitchen @ 2016-04-13 17:23 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

Hi all,

this series is RFC but has already been tested on a sama5d2x xplained
board with the Atmel QSPI controller + Micron n25q128a13:
compatible = "micron,n25q128a13", "jedec,spi-nor";

This first 3 patches of the series are stable and have already been
submitted to linux-mtd. They are required as a base for the later
patches.

Support of Micron memories has been implemented as an example, other
memory entries should be updated as needed in the spi_nor_ids[] table.

This new way to support Quad SPI memories is inspired by the JEDEC
SFDP standard. However SFDP tables are not provided by all memories and
some of them badly implement the standard. Also the standard itself
can tell whether the memory supports the 2-2-2 mode but doesn't provide
the procedure to enter/leave this mode as provided for the 4-4-4 mode.


Please note that some commit messages are missing but a review might be
really helpfull! :)

Almost all the actual rework is done in patch 4.

Best regards,

Cyrille

Cyrille Pitchen (8):
  mtd: spi-nor: add an alternative method to support memory >16MiB
  mtd: spi-nor: allow different flash_info entries to share the same
    JEDEC ID
  mtd: spi-nor: add entry for Macronix mx25l25673g
  mtd: spi-nor: fix support of Dual (x-y-2) and Quad (x-y-4) SPI
    protocols
  mtd: m25p80: add support of dual and quad spi protocols to all
    commands
  mtd: spi-nor: add support for Micron Dual and Quad SPI memories
  Documentation: atmel-quadspi: add binding file for Atmel QSPI driver
  mtd: atmel-quadspi: add driver for Atmel QSPI controller

 .../devicetree/bindings/mtd/atmel-quadspi.txt      |  32 +
 drivers/mtd/devices/m25p80.c                       | 212 ++++--
 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                | 812 +++++++++++++++++++++
 drivers/mtd/spi-nor/fsl-quadspi.c                  |   9 +-
 drivers/mtd/spi-nor/mtk-quadspi.c                  |  17 +-
 drivers/mtd/spi-nor/nxp-spifi.c                    |  22 +-
 drivers/mtd/spi-nor/spi-nor.c                      | 635 +++++++++++++---
 include/linux/mtd/spi-nor.h                        | 160 +++-
 12 files changed, 1750 insertions(+), 194 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] 15+ messages in thread

* [PATCH RFC 1/8] mtd: spi-nor: add an alternative method to support memory >16MiB
  2016-04-13 17:23 [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories Cyrille Pitchen
@ 2016-04-13 17:23 ` Cyrille Pitchen
  2016-04-13 17:23 ` [PATCH RFC 2/8] mtd: spi-nor: allow different flash_info entries to share the same JEDEC ID Cyrille Pitchen
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Cyrille Pitchen @ 2016-04-13 17:23 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           | 103 +++++++++++++++++++++++++-------
 include/linux/mtd/spi-nor.h             |  21 +++++--
 4 files changed, 111 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 157841dc3e99..4606eac237fe 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,80 @@ 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_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)
@@ -1421,27 +1499,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 3c36113a88e1..a91e95756a48 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -42,9 +42,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 */
@@ -55,11 +59,16 @@
 #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_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
 
 /* Used for SST flashes only. */
-- 
1.8.2.2

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

* [PATCH RFC 2/8] mtd: spi-nor: allow different flash_info entries to share the same JEDEC ID
  2016-04-13 17:23 [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories Cyrille Pitchen
  2016-04-13 17:23 ` [PATCH RFC 1/8] mtd: spi-nor: add an alternative method to support memory >16MiB Cyrille Pitchen
@ 2016-04-13 17:23 ` Cyrille Pitchen
  2016-04-13 17:23 ` [PATCH RFC 3/8] mtd: spi-nor: add entry for Macronix mx25l25673g Cyrille Pitchen
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Cyrille Pitchen @ 2016-04-13 17:23 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

Some SPI memories like Macronix MX25L25635E and MX25L25673G share the very
same JEDEC ID with no ext ID but provide different hardware capabilities.
For instance, the 35E revision doesn't support the dedicated 4byte address
opcodes for (Fast) Read, Page Program and Sector Erase operations whereas
the 73G does.

The 'name' argument of spi_nor_scan() is used by spi_nor_match_id() to look
the right entry up. Later, spi_nor_read_id() is called to check whether the
actual JEDEC ID read from the hardware matches the one associated with the
struct flash_info pointer returned by spi_nor_match_id().
However this check was done by comparing the jinfo and info struct
flash_info pointers. Since these pointer values might be different now,
the updated code checks the values of the id_len and id fields, which
should be the same for all entries associated to the same JEDEC ID.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 4606eac237fe..aac291a590e1 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1359,7 +1359,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 		jinfo = spi_nor_read_id(nor);
 		if (IS_ERR(jinfo)) {
 			return PTR_ERR(jinfo);
-		} else if (jinfo != info) {
+		} else if (jinfo->id_len != info->id_len ||
+			   memcmp(jinfo->id, info->id, info->id_len)) {
 			/*
 			 * JEDEC knows better, so overwrite platform ID. We
 			 * can't trust partitions any longer, but we'll let
-- 
1.8.2.2

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

* [PATCH RFC 3/8] mtd: spi-nor: add entry for Macronix mx25l25673g
  2016-04-13 17:23 [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories Cyrille Pitchen
  2016-04-13 17:23 ` [PATCH RFC 1/8] mtd: spi-nor: add an alternative method to support memory >16MiB Cyrille Pitchen
  2016-04-13 17:23 ` [PATCH RFC 2/8] mtd: spi-nor: allow different flash_info entries to share the same JEDEC ID Cyrille Pitchen
@ 2016-04-13 17:23 ` Cyrille Pitchen
  2016-04-13 17:23 ` [PATCH RFC 4/8] mtd: spi-nor: fix support of Dual (x-y-2) and Quad (x-y-4) SPI protocols Cyrille Pitchen
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Cyrille Pitchen @ 2016-04-13 17:23 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

The Macronix MX25L25635E and MX25L25673G share the same JEDEC ID, C22019,
with no extended ID to differenciate them at runtime.

However, the 73G supports dedicated 4byte address op code for (Fast) Read,
Page Program and Sectore Erase Operation whereas the 35E doesn't.

So this patch adds a specific entry to support the 73G revision.

For backward compatibility purpose, this new entry is inserted AFTER the
legacy "mx25l25635e" entry so this later entry is still the first one
found hence returned by spi_nor_read_id().

Then using the new entry requires the "mx25l25673g" string to be given
as the 'name' argument of spi_nor_scan().

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index aac291a590e1..54115aface9f 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -933,6 +933,7 @@ static const struct flash_info spi_nor_ids[] = {
 	{ "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, 0) },
 	{ "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
 	{ "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, 0) },
+	{ "mx25l25673g", INFO(0xc22019, 0, 64 * 1024, 512, SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
 	{ "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) },
 	{ "mx66l51235l", INFO(0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_QUAD_READ) },
 	{ "mx66l1g55g",  INFO(0xc2261b, 0, 64 * 1024, 2048, SPI_NOR_QUAD_READ) },
-- 
1.8.2.2

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

* [PATCH RFC 4/8] mtd: spi-nor: fix support of Dual (x-y-2) and Quad (x-y-4) SPI protocols
  2016-04-13 17:23 [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories Cyrille Pitchen
                   ` (2 preceding siblings ...)
  2016-04-13 17:23 ` [PATCH RFC 3/8] mtd: spi-nor: add entry for Macronix mx25l25673g Cyrille Pitchen
@ 2016-04-13 17:23 ` Cyrille Pitchen
  2016-04-15 22:09   ` Marek Vasut
  2016-04-24  5:06   ` R, Vignesh
  2016-04-13 17:23 ` [PATCH RFC 5/8] mtd: m25p80: add support of dual and quad spi protocols to all commands Cyrille Pitchen
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 15+ messages in thread
From: Cyrille Pitchen @ 2016-04-13 17:23 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

The quad (or dual) mode of a spi-nor memory may be enabled at boot time by
non-volatile bits in some setting register. Also such a mode may have
already been enabled at early stage by some boot loader.

Hence, we should not guess the spi-nor memory is always configured for the
regular SPI 1-1-1 protocol.

When other SPI modes such as SPI 2-2-2 or SPI 4-4-4 are already enabled,
some manufacturers (Micron, Macronix, ...) require to use a new dedicated
op code AFh to read the JEDEC ID.

Moreover, the procedures to enter/leave the 4-4-4 or 2-2-2 modes or to
enable the Quad Enable bits also depend on the memory's manufacturer.
Here again, some manufucturers have updated/fixed their procedure, hence
some procedures might not always be the same for a given manufacturer.

Also, depending on the SPI protocols for (Fast) Read, Page Program or
Sector Erase operations, the op codes might not be the same between
manufacturers and in some cases between memory parts all sharing the
same JEDEC ID (Micron n25q256*).

So to deal with all those issues, this patch extends the spi-nor framework
so basic flash parameters can be provided for each entry in the
spi_nor_ids[] table.

Finally, the spi_nor_scan() prototype has been changed: the enum read_mode
has been replaced by a struct spi_nor_modes so each SPI controller driver
can precisely declare the lists of SPI modes it supports for (Fast) Read
and Page Program operations. Equivalent lists are also available at the
memory side so the best match is selected.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/mtd/devices/m25p80.c      |  18 +--
 drivers/mtd/spi-nor/fsl-quadspi.c |   9 +-
 drivers/mtd/spi-nor/mtk-quadspi.c |  17 ++-
 drivers/mtd/spi-nor/nxp-spifi.c   |  22 ++--
 drivers/mtd/spi-nor/spi-nor.c     | 253 +++++++++++++++++++++++++++++---------
 include/linux/mtd/spi-nor.h       | 138 +++++++++++++++++++--
 6 files changed, 365 insertions(+), 92 deletions(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 9d6854467651..12112f7ae1a4 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -105,10 +105,10 @@ static void 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;
@@ -184,7 +184,11 @@ 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 = {
+		.id_modes = SNOR_MODE_1_1_1,
+		.rd_modes = SNOR_MODE_SLOW,
+		.wr_modes = SNOR_MODE_1_1_1,
+	};
 	char *flash_name;
 	int ret;
 
@@ -210,9 +214,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;
@@ -229,7 +233,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/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 9ab2b51d54b8..2eaf60711013 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -979,6 +979,13 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 	struct spi_nor *nor;
 	struct mtd_info *mtd;
 	int ret, i = 0;
+	struct spi_nor_modes modes = {
+		.id_modes = SNOR_MODE_1_1_1,
+		.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)
@@ -1080,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 8bed1a4cb79c..928031520d73 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 +
@@ -376,6 +376,13 @@ static int mtk_nor_init(struct mt8173_nor *mt8173_nor,
 {
 	int ret;
 	struct spi_nor *nor;
+	struct spi_nor_modes modes = {
+		.id_modes = SNOR_MODE_1_1_1,
+		.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);
@@ -392,7 +399,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 ae428cb0e04b..c0254c37f198 100644
--- a/drivers/mtd/spi-nor/nxp-spifi.c
+++ b/drivers/mtd/spi-nor/nxp-spifi.c
@@ -237,13 +237,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:
@@ -271,7 +270,11 @@ 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 = {
+		.id_modes = SNOR_MODE_1_1_1,
+		.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;
@@ -305,13 +308,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)) {
@@ -348,7 +350,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 54115aface9f..3573cc708b16 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -79,6 +79,8 @@ struct flash_info {
 					 * Use dedicated 4byte address op codes
 					 * to support memory size above 128Mib.
 					 */
+
+	const struct spi_nor_basic_flash_parameter *params;
 };
 
 #define JEDEC_MFR(info)	((info)->id[0])
@@ -143,24 +145,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.
  */
@@ -1073,13 +1057,19 @@ static const struct flash_info spi_nor_ids[] = {
 	{ },
 };
 
-static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
+static const struct flash_info *
+spi_nor_read_id(struct spi_nor *nor,
+		const struct spi_nor_basic_flash_parameter *params,
+		u32 id_modes)
 {
 	int			tmp;
 	u8			id[SPI_NOR_MAX_ID_LEN];
 	const struct flash_info	*info;
 
-	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
+	if (params && params->read_id)
+		tmp = params->read_id(nor, id, sizeof(id), id_modes);
+	else
+		tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, sizeof(id));
 	if (tmp < 0) {
 		dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
 		return ERR_PTR(tmp);
@@ -1329,8 +1319,159 @@ 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 int spi_nor_midx2proto(int midx, enum spi_nor_protocol *proto)
 {
+	switch (midx) {
+	case SNOR_MIDX_SLOW:
+	case SNOR_MIDX_1_1_1:
+		*proto = SNOR_PROTO_1_1_1;
+		break;
+
+	case SNOR_MIDX_1_1_2:
+		*proto = SNOR_PROTO_1_1_2;
+		break;
+
+	case SNOR_MIDX_1_2_2:
+		*proto = SNOR_PROTO_1_2_2;
+		break;
+
+	case SNOR_MIDX_2_2_2:
+		*proto = SNOR_PROTO_2_2_2;
+		break;
+
+	case SNOR_MIDX_1_1_4:
+		*proto = SNOR_PROTO_1_1_4;
+		break;
+
+	case SNOR_MIDX_1_4_4:
+		*proto = SNOR_PROTO_1_4_4;
+		break;
+
+	case SNOR_MIDX_4_4_4:
+		*proto = SNOR_PROTO_4_4_4;
+		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, enable_4_4_4, enable_2_2_2;
+	u32 rd_modes, wr_modes, cmd_modes, mask;
+	const struct spi_nor_erase_type *erase_type;
+	const struct spi_nor_read *read;
+	int rd_midx, wr_midx, err = 0;
+
+	/* 2-2-2 or 4-4-4 modes must be supported by BOTH read and write */
+	mask = (SNOR_MODE_2_2_2 | SNOR_MODE_4_4_4);
+	cmd_modes = (modes->rd_modes & modes->wr_modes) & mask;
+	rd_modes = (modes->rd_modes & ~mask) | cmd_modes;
+	wr_modes = (modes->wr_modes & ~mask) | cmd_modes;
+
+	/* Setup read operation. */
+	rd_midx = fls(params->rd_modes & rd_modes) - 1;
+	if (spi_nor_midx2proto(rd_midx, &nor->read_proto)) {
+		dev_err(nor->dev, "invalid (fast) read\n");
+		return -EINVAL;
+	}
+	read = &params->reads[rd_midx];
+	nor->read_opcode = read->opcode;
+	nor->read_dummy = read->num_mode_clocks + read->num_wait_states;
+
+	/* Set page program op code and protocol. */
+	wr_midx = fls(params->wr_modes & wr_modes) - 1;
+	if (spi_nor_midx2proto(wr_midx, &nor->write_proto)) {
+		dev_err(nor->dev, "invalid page program\n");
+		return -EINVAL;
+	}
+	nor->program_opcode = params->page_programs[wr_midx];
+
+	/* Set sector erase op code and size. */
+	erase_type = &params->erase_types[0];
+#ifdef CONFIG_MTD_SPI_NOR_USE_4K_SECTORS
+	for (i = 1; i < SNOR_MAX_ERASE_TYPES; ++i)
+		if (params->erase_types[i].size == 0x0c)
+			erase_type = &params->erase_types[i];
+#endif
+	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_4_4_4 = (nor->read_proto == SNOR_PROTO_4_4_4);
+	enable_2_2_2 = (nor->read_proto == SNOR_PROTO_2_2_2);
+
+	/* Enable Quad I/O if needed. */
+	if ((enable_quad_io || enable_4_4_4) &&
+	    params->enable_quad_io &&
+	    nor->reg_proto != SNOR_PROTO_4_4_4) {
+		err = params->enable_quad_io(nor, true);
+		if (err) {
+			dev_err(nor->dev,
+				"failed to enable the Quad I/O mode\n");
+			return err;
+		}
+	}
+
+	/* Enter/Leave 2-2-2 or 4-4-4 if needed. */
+	if (enable_2_2_2 && params->enable_2_2_2 &&
+	    nor->reg_proto != SNOR_PROTO_2_2_2)
+		err = params->enable_2_2_2(nor, true);
+	else if (enable_4_4_4 && params->enable_4_4_4 &&
+		 nor->reg_proto != SNOR_PROTO_4_4_4)
+		err = params->enable_4_4_4(nor, true);
+	else if (!enable_2_2_2 && params->enable_2_2_2 &&
+		 nor->reg_proto == SNOR_PROTO_2_2_2)
+		err = params->enable_2_2_2(nor, false);
+	else if (!enable_4_4_4 && params->enable_4_4_4 &&
+		 nor->reg_proto == SNOR_PROTO_4_4_4)
+		err = params->enable_4_4_4(nor, false);
+	if (err)
+		return err;
+
+	/*
+	 * Fix erase protocol if needed, read and write protocols should
+	 * already be valid.
+	 */
+	switch (nor->reg_proto) {
+	case SNOR_PROTO_4_4_4:
+		nor->erase_proto = SNOR_PROTO_4_4_4;
+		break;
+
+	case SNOR_PROTO_2_2_2:
+		nor->erase_proto = SNOR_PROTO_2_2_2;
+		break;
+
+	default:
+		nor->erase_proto = SNOR_PROTO_1_1_1;
+		break;
+	}
+
+	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=%zu\n",
+		nor->erase_opcode, nor->erase_proto, nor->mtd.erasesize);
+
+	return 0;
+}
+
+int spi_nor_scan(struct spi_nor *nor, const char *name,
+		 struct spi_nor_modes *modes)
+{
+	const struct spi_nor_basic_flash_parameter *params = NULL;
 	const struct flash_info *info = NULL;
 	struct device *dev = nor->dev;
 	struct mtd_info *mtd = &nor->mtd;
@@ -1342,11 +1483,17 @@ 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->erase_proto = SNOR_PROTO_1_1_1;
+	nor->read_proto = SNOR_PROTO_1_1_1;
+	nor->write_proto = SNOR_PROTO_1_1_1;
+	nor->reg_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 */
 	if (!info)
-		info = spi_nor_read_id(nor);
+		info = spi_nor_read_id(nor, NULL, 0);
 	if (IS_ERR_OR_NULL(info))
 		return -ENOENT;
 
@@ -1357,7 +1504,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	if (name && info->id_len) {
 		const struct flash_info *jinfo;
 
-		jinfo = spi_nor_read_id(nor);
+		jinfo = spi_nor_read_id(nor, info->params, modes->id_modes);
 		if (IS_ERR(jinfo)) {
 			return PTR_ERR(jinfo);
 		} else if (jinfo->id_len != info->id_len ||
@@ -1374,6 +1521,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 			info = jinfo;
 		}
 	}
+	params = info->params;
 
 	mutex_init(&nor->lock);
 
@@ -1451,51 +1599,42 @@ 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;
+			modes->rd_modes |= SNOR_MODE_1_1_1;
 		else
-			nor->flash_read = SPI_NOR_NORMAL;
+			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;
+		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;
+		modes->rd_modes &= ~SNOR_MODE_1_1_1;
 
-	/* 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");
+	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).
+	 * - enter/leave the 4-4-4 or 2-2-2 modes if needed.
+	 */
+	if (params) {
+		ret = spi_nor_setup(nor, info, params, modes);
+		if (ret)
 			return ret;
+	} else {
+		if (modes->rd_modes & SNOR_MODE_1_1_1) {
+			nor->read_opcode = SPINOR_OP_READ_FAST;
+			nor->read_dummy = 8;
+		} else {
+			nor->read_opcode = SPINOR_OP_READ;
+			nor->read_dummy = 0;
 		}
-		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;
-	}
-
-	nor->program_opcode = SPINOR_OP_PP;
-
 	if (info->addr_width)
 		nor->addr_width = info->addr_width;
 	else if (mtd->size > 0x1000000) {
@@ -1516,8 +1655,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 a91e95756a48..770ea84370d0 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -84,8 +84,9 @@
 #define SPINOR_OP_BRWR		0x17	/* Bank register write */
 
 /* Used for Micron flashes only. */
-#define SPINOR_OP_RD_EVCR      0x65    /* Read EVCR register */
-#define SPINOR_OP_WD_EVCR      0x61    /* Write EVCR register */
+#define SPINOR_OP_MIO_RDID	0xaf	/* Multiple I/O Read JEDEC ID */
+#define SPINOR_OP_RD_EVCR	0x65    /* Read EVCR register */
+#define SPINOR_OP_WD_EVCR	0x61    /* Write EVCR register */
 
 /* Status Register bits. */
 #define SR_WIP			BIT(0)	/* Write in progress */
@@ -108,11 +109,119 @@
 /* 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 modes */
+enum spi_nor_mode_index {
+	/* Sorted by ascending priority order */
+	SNOR_MIDX_SLOW = 0,
+	SNOR_MIDX_1_1_1,
+	SNOR_MIDX_1_1_2,
+	SNOR_MIDX_1_2_2,
+	SNOR_MIDX_2_2_2,
+	SNOR_MIDX_1_1_4,
+	SNOR_MIDX_1_4_4,
+	SNOR_MIDX_4_4_4,
+
+	SNOR_MIDX_MAX
+};
+
+#define SNOR_MODE_SLOW		BIT(SNOR_MIDX_SLOW)
+#define SNOR_MODE_1_1_1		BIT(SNOR_MIDX_1_1_1)
+#define SNOR_MODE_1_1_2		BIT(SNOR_MIDX_1_1_2)
+#define SNOR_MODE_1_2_2		BIT(SNOR_MIDX_1_2_2)
+#define SNOR_MODE_2_2_2		BIT(SNOR_MIDX_2_2_2)
+#define SNOR_MODE_1_1_4		BIT(SNOR_MIDX_1_1_4)
+#define SNOR_MODE_1_4_4		BIT(SNOR_MIDX_1_4_4)
+#define SNOR_MODE_4_4_4		BIT(SNOR_MIDX_4_4_4)
+
+struct spi_nor_modes {
+	u32	id_modes;	/* supported SPI modes for Read JEDEC ID */
+	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;
+};
+
+#define SNOR_OP_READ(_num_mode_clocks, _num_wait_states, _opcode) \
+	{							  \
+		.num_wait_states = _num_wait_states,		  \
+		.num_mode_clocks = _num_mode_clocks,		  \
+		.opcode = _opcode,				  \
+	}
+
+struct spi_nor_erase_type {
+	u8	size;	/* specifies 'N' so erase size = 2^N */
+	u8	opcode;
+};
+
+#define SNOR_OP_ERASE(_size, _opcode) { .size = _size, .opcode = _opcode }
+#define SNOR_OP_ERASE_64K(_opcode) SNOR_OP_ERASE(0x10, _opcode)
+#define SNOR_OP_ERASE_32K(_opcode) SNOR_OP_ERASE(0x0f, _opcode)
+#define SNOR_OP_ERASE_4K(_opcode)  SNOR_OP_ERASE(0x0c, _opcode)
+
+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_MIDX_MAX];
+
+	/* Page Program settings */
+	u32				wr_modes;
+	u8				page_programs[SNOR_MIDX_MAX];
+
+	/* Sector Erase settings */
+	struct spi_nor_erase_type	erase_types[SNOR_MAX_ERASE_TYPES];
+
+	int (*read_id)(struct spi_nor *nor, u8 *id, size_t id_len,
+		       u32 id_modes);
+	int (*enable_quad_io)(struct spi_nor *nor, bool enable);
+	int (*enable_4_4_4)(struct spi_nor *nor, bool enable);
+	int (*enable_2_2_2)(struct spi_nor *nor, bool enable);
+};
+
+
+#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
@@ -140,9 +249,12 @@ 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_*)
+ * @erase_proto:	the SPI protocol used by erase operations
+ * @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
@@ -171,7 +283,10 @@ struct spi_nor {
 	u8			read_opcode;
 	u8			read_dummy;
 	u8			program_opcode;
-	enum read_mode		flash_read;
+	enum spi_nor_protocol	erase_proto;
+	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];
@@ -209,7 +324,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
@@ -219,6 +334,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,
+		 struct spi_nor_modes *modes);
 
 #endif
-- 
1.8.2.2

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

* [PATCH RFC 5/8] mtd: m25p80: add support of dual and quad spi protocols to all commands
  2016-04-13 17:23 [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories Cyrille Pitchen
                   ` (3 preceding siblings ...)
  2016-04-13 17:23 ` [PATCH RFC 4/8] mtd: spi-nor: fix support of Dual (x-y-2) and Quad (x-y-4) SPI protocols Cyrille Pitchen
@ 2016-04-13 17:23 ` Cyrille Pitchen
  2016-04-13 17:23 ` [PATCH RFC 6/8] mtd: spi-nor: add support for Micron Dual and Quad SPI memories Cyrille Pitchen
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Cyrille Pitchen @ 2016-04-13 17:23 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.

However once their Quad mode enabled, Micron and Macronix spi-nor memories
expect all commands to use the SPI 4-4-4 protocol.

Also, once their Dual mode enabled, Micron spi-nor memories expect all
commands to use the SPI-2-2-2 protocol.

So 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 | 200 +++++++++++++++++++++++++++++++++----------
 1 file changed, 155 insertions(+), 45 deletions(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 12112f7ae1a4..d4a3bc0e4c3f 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 void m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
@@ -78,43 +150,54 @@ static void 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);
-
-	spi_message_init(&m);
+	int ret, cmd_sz = m25p_cmdsz(nor);
 
 	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) {
+		*retlen = 0;
+		return;
+	}
 
-	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);
 	spi_sync(spi, &m);
 
 	*retlen += m.actual_length - cmd_sz;
 }
 
-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.
@@ -124,13 +207,24 @@ static int 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;
+	struct spi_transfer xfers[3];
+	struct spi_message m;
+	int ret;
+
+	/* Get transfer protocols. */
+	ret = m25p80_proto2nbits(nor->read_proto,
+				 &code_nbits, &addr_nbits, &data_nbits);
+	if (ret < 0) {
+		*retlen = 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;
 		int ret;
@@ -143,31 +237,47 @@ static int 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);
 		*retlen = msg.retlen;
 		return ret;
 	}
 
-	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 = len;
-	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);
 	spi_sync(spi, &m);
 
 	*retlen = m.actual_length - m25p_cmdsz(nor) - dummy;
-- 
1.8.2.2

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

* [PATCH RFC 6/8] mtd: spi-nor: add support for Micron Dual and Quad SPI memories
  2016-04-13 17:23 [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories Cyrille Pitchen
                   ` (4 preceding siblings ...)
  2016-04-13 17:23 ` [PATCH RFC 5/8] mtd: m25p80: add support of dual and quad spi protocols to all commands Cyrille Pitchen
@ 2016-04-13 17:23 ` Cyrille Pitchen
  2016-04-13 17:23 ` [PATCH RFC 7/8] Documentation: atmel-quadspi: add binding file for Atmel QSPI driver Cyrille Pitchen
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Cyrille Pitchen @ 2016-04-13 17:23 UTC (permalink / raw)
  To: computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, marex, linux-kernel, Cyrille Pitchen

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 3573cc708b16..260ba5b9d010 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -807,6 +807,214 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 	return ret;
 }
 
+struct spi_nor_read_id {
+	u32			id_mode;
+	u8			opcode;
+};
+
+static int spi_nor_read_id_multi_io(struct spi_nor *nor,
+				    u8 *id, size_t id_len,
+				    u32 id_modes,
+				    const struct spi_nor_read_id *configs,
+				    size_t num_configs,
+				    u8 mfr_id);
+
+static int spi_nor_micron_read_id(struct spi_nor *nor, u8 *id, size_t id_len,
+				  u32 id_modes)
+{
+	static const struct spi_nor_read_id configs[] = {
+		{SNOR_MODE_1_1_1, SPINOR_OP_RDID},
+		{SNOR_MODE_4_4_4, SPINOR_OP_MIO_RDID},
+		{SNOR_MODE_2_2_2, SPINOR_OP_MIO_RDID},
+	};
+
+	return spi_nor_read_id_multi_io(nor, id, id_len, id_modes,
+					configs, ARRAY_SIZE(configs),
+					SNOR_MFR_MICRON);
+}
+
+static int micron_set_protocol(struct spi_nor *nor, u8 mask, u8 val,
+			       enum spi_nor_protocol proto)
+{
+	u8 evcr;
+	int ret;
+
+	/* Read the Enhanced Volatile Configuration Register (EVCR). */
+	ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &evcr, 1);
+	if (ret < 0) {
+		dev_err(nor->dev, "error while reading EVCR register\n");
+		return ret;
+	}
+
+	/* Check whether we need to update the protocol bits. */
+	if ((evcr & mask) == val)
+		return 0;
+
+	/* Set EVCR protocol bits. */
+	write_enable(nor);
+	evcr = (evcr & ~mask) | val;
+	ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, &evcr, 1);
+	if (ret < 0) {
+		dev_err(nor->dev, "error while writing EVCR register\n");
+		return ret;
+	}
+
+	/* Switch reg protocol now before accessing any other registers. */
+	nor->reg_proto = proto;
+
+	ret = spi_nor_wait_till_ready(nor);
+	if (ret)
+		return ret;
+
+	/* Read EVCR and check it. */
+	ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &evcr, 1);
+	if (ret < 0 || (evcr & mask) != val) {
+		dev_err(nor->dev, "Micron EVCR protocol bits not updated\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int micron_set_quad_mode(struct spi_nor *nor)
+{
+	int ret;
+
+	/* Clear Quad bit to select the Quad SPI mode */
+	ret = micron_set_protocol(nor,
+				  EVCR_QUAD_EN_MICRON, 0,
+				  SNOR_PROTO_4_4_4);
+	if (ret) {
+		dev_err(nor->dev, "Failed to set Micron Quad SPI mode\n");
+		return ret;
+	}
+
+	nor->read_proto  = SNOR_PROTO_4_4_4;
+	nor->write_proto = SNOR_PROTO_4_4_4;
+	nor->erase_proto = SNOR_PROTO_4_4_4;
+	return 0;
+}
+
+static int micron_set_dual_mode(struct  spi_nor *nor)
+{
+	int ret;
+
+	/* Set Quad/Dual bits to 10 to select the Dual SPI mode */
+	ret = micron_set_protocol(nor,
+				  EVCR_QUAD_EN_MICRON | EVCR_DUAL_EN_MICRON,
+				  EVCR_QUAD_EN_MICRON,
+				  SNOR_PROTO_2_2_2);
+	if (ret) {
+		dev_err(nor->dev, "Failed to set Micron Dual SPI mode\n");
+		return ret;
+	}
+
+	nor->read_proto  = SNOR_PROTO_2_2_2;
+	nor->write_proto = SNOR_PROTO_2_2_2;
+	nor->erase_proto = SNOR_PROTO_2_2_2;
+	return 0;
+}
+
+static int micron_set_extended_spi_mode(struct spi_nor *nor)
+{
+	int ret;
+
+	/* Set Quad/Dual bits to 11 to select the Extended SPI mode */
+	ret = micron_set_protocol(nor,
+				  EVCR_QUAD_EN_MICRON | EVCR_DUAL_EN_MICRON,
+				  EVCR_QUAD_EN_MICRON | EVCR_DUAL_EN_MICRON,
+				  SNOR_PROTO_1_1_1);
+	if (ret) {
+		dev_err(nor->dev, "Failed to set Micron Extended SPI mode\n");
+		return ret;
+	}
+
+	nor->write_proto = SNOR_PROTO_1_1_1;
+	nor->erase_proto = SNOR_PROTO_1_1_1;
+	return 0;
+}
+
+static int spi_nor_micron_enable_4_4_4(struct spi_nor *nor, bool enable)
+{
+	if (enable)
+		return micron_set_quad_mode(nor);
+	return micron_set_extended_spi_mode(nor);
+}
+
+static int spi_nor_micron_enable_2_2_2(struct spi_nor *nor, bool enable)
+{
+	if (enable)
+		return micron_set_dual_mode(nor);
+	return micron_set_extended_spi_mode(nor);
+}
+
+#define SNOR_MICRON_RD_MODES			\
+	(SNOR_MODE_SLOW |			\
+	 SNOR_MODE_1_1_1 |			\
+	 SNOR_MODE_1_1_2 |			\
+	 SNOR_MODE_1_2_2 |			\
+	 SNOR_MODE_2_2_2 |			\
+	 SNOR_MODE_1_1_4 |			\
+	 SNOR_MODE_1_4_4 |			\
+	 SNOR_MODE_4_4_4)
+
+#define SNOR_MICRON_WR_MODES			\
+	(SNOR_MODE_1_1_1 |			\
+	 SNOR_MODE_2_2_2 |			\
+	 SNOR_MODE_1_1_4 |			\
+	 SNOR_MODE_4_4_4)
+
+static const struct spi_nor_basic_flash_parameter micron_params = {
+	.rd_modes		= SNOR_MICRON_RD_MODES,
+	.reads[SNOR_MIDX_SLOW]	= SNOR_OP_READ(0, 0, SPINOR_OP_READ),
+	.reads[SNOR_MIDX_1_1_1]	= SNOR_OP_READ(0, 8, SPINOR_OP_READ_FAST),
+	.reads[SNOR_MIDX_1_1_2]	= SNOR_OP_READ(0, 8, SPINOR_OP_READ_1_1_2),
+	.reads[SNOR_MIDX_1_2_2]	= SNOR_OP_READ(1, 7, SPINOR_OP_READ_1_2_2),
+	.reads[SNOR_MIDX_2_2_2]	= SNOR_OP_READ(1, 7, SPINOR_OP_READ_1_2_2),
+	.reads[SNOR_MIDX_1_1_4]	= SNOR_OP_READ(1, 7, SPINOR_OP_READ_1_1_4),
+	.reads[SNOR_MIDX_1_4_4]	= SNOR_OP_READ(1, 9, SPINOR_OP_READ_1_4_4),
+	.reads[SNOR_MIDX_4_4_4]	= SNOR_OP_READ(1, 9, SPINOR_OP_READ_1_4_4),
+
+	.wr_modes		= SNOR_MICRON_WR_MODES,
+	.page_programs[SNOR_MIDX_1_1_1]	= SPINOR_OP_PP,
+	.page_programs[SNOR_MIDX_2_2_2]	= SPINOR_OP_PP,
+	.page_programs[SNOR_MIDX_1_1_4]	= SPINOR_OP_PP_1_1_4,
+	.page_programs[SNOR_MIDX_4_4_4]	= SPINOR_OP_PP_1_1_4,
+
+	.erase_types[0]		= SNOR_OP_ERASE_64K(SPINOR_OP_SE),
+
+	.read_id		= spi_nor_micron_read_id,
+	.enable_4_4_4		= spi_nor_micron_enable_4_4_4,
+	.enable_2_2_2		= spi_nor_micron_enable_2_2_2,
+};
+
+static const struct spi_nor_basic_flash_parameter micron_4k_params = {
+	.rd_modes		= SNOR_MICRON_RD_MODES,
+	.reads[SNOR_MIDX_SLOW]	= SNOR_OP_READ(0, 0, SPINOR_OP_READ),
+	.reads[SNOR_MIDX_1_1_1]	= SNOR_OP_READ(0, 8, SPINOR_OP_READ_FAST),
+	.reads[SNOR_MIDX_1_1_2]	= SNOR_OP_READ(0, 8, SPINOR_OP_READ_1_1_2),
+	.reads[SNOR_MIDX_1_2_2]	= SNOR_OP_READ(1, 7, SPINOR_OP_READ_1_2_2),
+	.reads[SNOR_MIDX_2_2_2]	= SNOR_OP_READ(1, 7, SPINOR_OP_READ_1_2_2),
+	.reads[SNOR_MIDX_1_1_4]	= SNOR_OP_READ(1, 7, SPINOR_OP_READ_1_1_4),
+	.reads[SNOR_MIDX_1_4_4]	= SNOR_OP_READ(1, 9, SPINOR_OP_READ_1_4_4),
+	.reads[SNOR_MIDX_4_4_4]	= SNOR_OP_READ(1, 9, SPINOR_OP_READ_1_4_4),
+
+	.wr_modes		= SNOR_MICRON_WR_MODES,
+	.page_programs[SNOR_MIDX_1_1_1]	= SPINOR_OP_PP,
+	.page_programs[SNOR_MIDX_2_2_2]	= SPINOR_OP_PP,
+	.page_programs[SNOR_MIDX_1_1_4]	= SPINOR_OP_PP_1_1_4,
+	.page_programs[SNOR_MIDX_4_4_4]	= SPINOR_OP_PP_1_1_4,
+
+	.erase_types[0]		= SNOR_OP_ERASE_64K(SPINOR_OP_SE),
+	.erase_types[1]		= SNOR_OP_ERASE_4K(SPINOR_OP_BE_4K),
+
+	.read_id		= spi_nor_micron_read_id,
+	.enable_4_4_4		= spi_nor_micron_enable_4_4_4,
+	.enable_2_2_2		= spi_nor_micron_enable_2_2_2,
+};
+
+#define PARAMS(_name) .params = &_name##_params
+
 /* Used when the "_ext_id" is two bytes at most */
 #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
 		.id = {							\
@@ -820,7 +1028,7 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 		.sector_size = (_sector_size),				\
 		.n_sectors = (_n_sectors),				\
 		.page_size = 256,					\
-		.flags = (_flags),
+		.flags = (_flags)
 
 #define INFO6(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
 		.id = {							\
@@ -923,16 +1131,16 @@ static const struct flash_info spi_nor_ids[] = {
 	{ "mx66l1g55g",  INFO(0xc2261b, 0, 64 * 1024, 2048, SPI_NOR_QUAD_READ) },
 
 	/* Micron */
-	{ "n25q032",	 INFO(0x20ba16, 0, 64 * 1024,   64, SPI_NOR_QUAD_READ) },
-	{ "n25q032a",	 INFO(0x20bb16, 0, 64 * 1024,   64, SPI_NOR_QUAD_READ) },
-	{ "n25q064",     INFO(0x20ba17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
-	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
-	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
-	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
-	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
-	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
-	{ "n25q512ax3",  INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
-	{ "n25q00",      INFO(0x20ba21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
+	{ "n25q032",     INFO(0x20ba16, 0, 64 * 1024,   64, 0), PARAMS(micron) },
+	{ "n25q032a",    INFO(0x20bb16, 0, 64 * 1024,   64, 0), PARAMS(micron) },
+	{ "n25q064",     INFO(0x20ba17, 0, 64 * 1024,  128, 0), PARAMS(micron_4k) },
+	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, 0), PARAMS(micron_4k) },
+	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, 0), PARAMS(micron_4k) },
+	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, 0), PARAMS(micron_4k) },
+	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, 0), PARAMS(micron_4k) },
+	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, USE_FSR), PARAMS(micron_4k) },
+	{ "n25q512ax3",  INFO(0x20ba20, 0, 64 * 1024, 1024, USE_FSR), PARAMS(micron_4k) },
+	{ "n25q00",      INFO(0x20ba21, 0, 64 * 1024, 2048, USE_FSR), PARAMS(micron_4k) },
 
 	/* PMC */
 	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2, SECT_4K_PMC) },
@@ -1358,6 +1566,51 @@ static int spi_nor_midx2proto(int midx, enum spi_nor_protocol *proto)
 	return 0;
 }
 
+static int spi_nor_read_id_multi_io(struct spi_nor *nor,
+				    u8 *id, size_t id_len,
+				    u32 id_modes,
+				    const struct spi_nor_read_id *configs,
+				    size_t num_configs,
+				    u8 mfr_id)
+{
+	size_t i;
+	int err;
+
+	memset(id, 0, id_len);
+	for (i = 0; i < num_configs; ++i) {
+		const struct spi_nor_read_id *config = &configs[i];
+		enum spi_nor_protocol proto;
+		int id_midx;
+
+		/* Skip unsupported modes. */
+		if (!(config->id_mode & id_modes))
+			continue;
+
+		/* Set SPI protocols */
+		id_midx = fls(config->id_mode) - 1;
+		if (spi_nor_midx2proto(id_midx, &proto)) {
+			dev_err(nor->dev,
+				"Invalid spi-nor mode to read the JEDEC ID\n");
+			return -EINVAL;
+		}
+		nor->reg_proto   = proto;
+		nor->read_proto  = proto;
+		nor->write_proto = proto;
+		nor->erase_proto = proto;
+
+		err = nor->read_reg(nor, config->opcode, id, id_len);
+		if (err)
+			return err;
+
+		if (id[0] == mfr_id)
+			break;
+
+		memset(id, 0, id_len);
+	}
+
+	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)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 770ea84370d0..d857f628b5ed 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -101,6 +101,7 @@
 #define SR_QUAD_EN_MX		BIT(6)	/* Macronix Quad I/O */
 
 /* Enhanced Volatile Configuration Register bits */
+#define EVCR_DUAL_EN_MICRON	BIT(6)	/* Micron Dual I/O */
 #define EVCR_QUAD_EN_MICRON	BIT(7)	/* Micron Quad I/O */
 
 /* Flag Status Register bits */
-- 
1.8.2.2

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

* [PATCH RFC 7/8] Documentation: atmel-quadspi: add binding file for Atmel QSPI driver
  2016-04-13 17:23 [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories Cyrille Pitchen
                   ` (5 preceding siblings ...)
  2016-04-13 17:23 ` [PATCH RFC 6/8] mtd: spi-nor: add support for Micron Dual and Quad SPI memories Cyrille Pitchen
@ 2016-04-13 17:23 ` Cyrille Pitchen
  2016-04-13 17:23 ` [PATCH RFC 8/8] mtd: atmel-quadspi: add driver for Atmel QSPI controller Cyrille Pitchen
  2016-04-15 21:48 ` [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories Marek Vasut
  8 siblings, 0 replies; 15+ messages in thread
From: Cyrille Pitchen @ 2016-04-13 17:23 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] 15+ messages in thread

* [PATCH RFC 8/8] mtd: atmel-quadspi: add driver for Atmel QSPI controller
  2016-04-13 17:23 [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories Cyrille Pitchen
                   ` (6 preceding siblings ...)
  2016-04-13 17:23 ` [PATCH RFC 7/8] Documentation: atmel-quadspi: add binding file for Atmel QSPI driver Cyrille Pitchen
@ 2016-04-13 17:23 ` Cyrille Pitchen
  2016-04-15 22:17   ` Marek Vasut
  2016-04-15 21:48 ` [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories Marek Vasut
  8 siblings, 1 reply; 15+ messages in thread
From: Cyrille Pitchen @ 2016-04-13 17:23 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 | 812 ++++++++++++++++++++++++++++++++++++
 3 files changed, 822 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..948a74c170ed
--- /dev/null
+++ b/drivers/mtd/spi-nor/atmel-quadspi.c
@@ -0,0 +1,812 @@
+/*
+ * 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;
+
+#ifdef DEBUG
+	u8			last_instruction;
+#endif
+};
+
+struct atmel_qspi_command {
+	u32	ifr;
+	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)
+{
+	u8 cmd_buf[SPI_NOR_MAX_CMD_SIZE];
+	size_t len = 0;
+	int i;
+
+	if (cmd->enable.bits.instruction) {
+		if (aq->last_instruction == cmd->instruction)
+			return;
+		aq->last_instruction = cmd->instruction;
+	}
+
+	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 (cmd->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)
+#endif
+
+static int atmel_qspi_run_command(struct atmel_qspi *aq,
+				  const struct atmel_qspi_command *cmd)
+{
+	u32 iar, icr, ifr, sr;
+	int err = 0;
+
+	iar = 0;
+	icr = 0;
+	ifr = cmd->ifr;
+
+	/* 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);
+	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_command_set_ifr(struct atmel_qspi_command *cmd,
+				      u32 ifr_tfrtyp,
+				      enum spi_nor_protocol proto)
+{
+	cmd->ifr = ifr_tfrtyp;
+
+	switch (proto) {
+	case SNOR_PROTO_1_1_1:
+		cmd->ifr |= QSPI_IFR_WIDTH_SINGLE_BIT_SPI;
+		break;
+	case SNOR_PROTO_1_1_2:
+		cmd->ifr |= QSPI_IFR_WIDTH_DUAL_OUTPUT;
+		break;
+	case SNOR_PROTO_1_1_4:
+		cmd->ifr |= QSPI_IFR_WIDTH_QUAD_OUTPUT;
+		break;
+	case SNOR_PROTO_1_2_2:
+		cmd->ifr |= QSPI_IFR_WIDTH_DUAL_IO;
+		break;
+	case SNOR_PROTO_1_4_4:
+		cmd->ifr |= QSPI_IFR_WIDTH_QUAD_IO;
+		break;
+	case SNOR_PROTO_2_2_2:
+		cmd->ifr |= QSPI_IFR_WIDTH_DUAL_CMD;
+		break;
+	case SNOR_PROTO_4_4_4:
+		cmd->ifr |= QSPI_IFR_WIDTH_QUAD_CMD;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+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;
+	int ret;
+
+	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;
+
+	ret = atmel_qspi_command_set_ifr(&cmd,
+					 QSPI_IFR_TFRTYP_TRSFR_READ,
+					 nor->reg_proto);
+	if (ret)
+		return ret;
+
+	return atmel_qspi_run_command(aq, &cmd);
+}
+
+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;
+	int ret;
+
+	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;
+
+	ret = atmel_qspi_command_set_ifr(&cmd,
+					 QSPI_IFR_TFRTYP_TRSFR_WRITE,
+					 nor->reg_proto);
+	if (ret)
+		return ret;
+
+	return atmel_qspi_run_command(aq, &cmd);
+}
+
+static void atmel_qspi_write(struct spi_nor *nor, loff_t to, size_t len,
+			     size_t *retlen, const u_char *write_buf)
+{
+	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.enable.bits.data = 1;
+	cmd.instruction = nor->program_opcode;
+	cmd.address = (u32)to;
+	cmd.tx_buf = write_buf;
+	cmd.buf_len = len;
+
+	if (atmel_qspi_command_set_ifr(&cmd,
+				       QSPI_IFR_TFRTYP_TRSFR_WRITE_MEM,
+				       nor->write_proto))
+		return;
+
+	if (!atmel_qspi_run_command(aq, &cmd))
+		*retlen += len;
+}
+
+static int atmel_qspi_erase(struct spi_nor *nor, loff_t offs)
+{
+	struct atmel_qspi *aq = nor->priv;
+	struct atmel_qspi_command cmd;
+	int ret;
+
+	dev_dbg(nor->dev, "%dKiB at 0x%08x\n",
+		nor->mtd.erasesize / 1024, (u32)offs);
+
+	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;
+
+	ret = atmel_qspi_command_set_ifr(&cmd,
+					 QSPI_IFR_TFRTYP_TRSFR_WRITE,
+					 nor->erase_proto);
+	if (ret)
+		return ret;
+
+	return atmel_qspi_run_command(aq, &cmd);
+}
+
+static int atmel_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
+			   size_t *retlen, u_char *read_buf)
+{
+	struct atmel_qspi *aq = nor->priv;
+	struct atmel_qspi_command cmd;
+	int ret;
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.enable.bits.instruction = 1;
+	cmd.enable.bits.address = nor->addr_width;
+	cmd.enable.bits.dummy = (nor->read_dummy > 0);
+	cmd.enable.bits.data = 1;
+	cmd.instruction = nor->read_opcode;
+	cmd.address = (u32)from;
+	cmd.num_dummy_cycles = nor->read_dummy;
+	cmd.rx_buf = read_buf;
+	cmd.buf_len = len;
+
+	ret = atmel_qspi_command_set_ifr(&cmd,
+					 QSPI_IFR_TFRTYP_TRSFR_READ_MEM,
+					 nor->read_proto);
+	if (ret)
+		return ret;
+
+	ret = atmel_qspi_run_command(aq, &cmd);
+	if (ret)
+		return ret;
+
+	*retlen += len;
+	return 0;
+}
+
+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;
+	struct spi_nor_modes modes = {
+		.id_modes = (SNOR_MODE_1_1_1 |
+			     SNOR_MODE_2_2_2 |
+			     SNOR_MODE_4_4_4),
+		.rd_modes = (SNOR_MODE_SLOW |
+			     SNOR_MODE_1_1_1 |
+			     SNOR_MODE_1_1_2 |
+			     SNOR_MODE_1_2_2 |
+			     SNOR_MODE_2_2_2 |
+			     SNOR_MODE_1_1_4 |
+			     SNOR_MODE_1_4_4 |
+			     SNOR_MODE_4_4_4),
+		.wr_modes = (SNOR_MODE_1_1_1 |
+			     SNOR_MODE_1_1_2 |
+			     SNOR_MODE_1_2_2 |
+			     SNOR_MODE_2_2_2 |
+			     SNOR_MODE_1_1_4 |
+			     SNOR_MODE_1_4_4 |
+			     SNOR_MODE_4_4_4),
+	};
+	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, &modes);
+	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] 15+ messages in thread

* Re: [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories
  2016-04-13 17:23 [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories Cyrille Pitchen
                   ` (7 preceding siblings ...)
  2016-04-13 17:23 ` [PATCH RFC 8/8] mtd: atmel-quadspi: add driver for Atmel QSPI controller Cyrille Pitchen
@ 2016-04-15 21:48 ` Marek Vasut
  8 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2016-04-15 21:48 UTC (permalink / raw)
  To: Cyrille Pitchen, computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, linux-kernel

On 04/13/2016 07:23 PM, Cyrille Pitchen wrote:
> Hi all,
> 
> this series is RFC but has already been tested on a sama5d2x xplained
> board with the Atmel QSPI controller + Micron n25q128a13:
> compatible = "micron,n25q128a13", "jedec,spi-nor";
> 
> This first 3 patches of the series are stable and have already been
> submitted to linux-mtd. They are required as a base for the later
> patches.
> 
> Support of Micron memories has been implemented as an example, other
> memory entries should be updated as needed in the spi_nor_ids[] table.
> 
> This new way to support Quad SPI memories is inspired by the JEDEC
> SFDP standard. However SFDP tables are not provided by all memories and
> some of them badly implement the standard. Also the standard itself
> can tell whether the memory supports the 2-2-2 mode but doesn't provide
> the procedure to enter/leave this mode as provided for the 4-4-4 mode.
> 
> 
> Please note that some commit messages are missing but a review might be
> really helpfull! :)
> 
> Almost all the actual rework is done in patch 4.

For Altera SoCFPGA , Cadence QSPI NOR controller , Terasic SoCkit board:
Tested-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

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

* Re: [PATCH RFC 4/8] mtd: spi-nor: fix support of Dual (x-y-2) and Quad (x-y-4) SPI protocols
  2016-04-13 17:23 ` [PATCH RFC 4/8] mtd: spi-nor: fix support of Dual (x-y-2) and Quad (x-y-4) SPI protocols Cyrille Pitchen
@ 2016-04-15 22:09   ` Marek Vasut
  2016-04-25 12:01     ` Cyrille Pitchen
  2016-04-24  5:06   ` R, Vignesh
  1 sibling, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2016-04-15 22:09 UTC (permalink / raw)
  To: Cyrille Pitchen, computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, linux-kernel

On 04/13/2016 07:23 PM, Cyrille Pitchen wrote:

[...]

Hi!

[...]

> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 9d6854467651..12112f7ae1a4 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -105,10 +105,10 @@ static void 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:

I have to say, I am not a big fan of replacing a macro with numeric
constant, but that might be a matter of taste.

>  		return 2;
> -	case SPI_NOR_QUAD:
> +	case 4:
>  		return 4;
>  	default:
>  		return 0;

[...]

> -int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
> +static int spi_nor_midx2proto(int midx, enum spi_nor_protocol *proto)

It is entirely inobvious what this function does from it's name.
I had to scroll through the code to figure out what midx means
and that it's "mode index". Either add a comment and rename the
function. I would be in favor of the later.

>  {
> +	switch (midx) {
> +	case SNOR_MIDX_SLOW:
> +	case SNOR_MIDX_1_1_1:
> +		*proto = SNOR_PROTO_1_1_1;

Can you not unify SNOR_PROTO_* and SNOR_MIDX_* , so this odd switch
would go away entirely ? If not, make this into a lookup table at
least.

> +		break;
> +
> +	case SNOR_MIDX_1_1_2:
> +		*proto = SNOR_PROTO_1_1_2;
> +		break;
> +
> +	case SNOR_MIDX_1_2_2:
> +		*proto = SNOR_PROTO_1_2_2;
> +		break;
> +
> +	case SNOR_MIDX_2_2_2:
> +		*proto = SNOR_PROTO_2_2_2;
> +		break;
> +
> +	case SNOR_MIDX_1_1_4:
> +		*proto = SNOR_PROTO_1_1_4;
> +		break;
> +
> +	case SNOR_MIDX_1_4_4:
> +		*proto = SNOR_PROTO_1_4_4;
> +		break;
> +
> +	case SNOR_MIDX_4_4_4:
> +		*proto = SNOR_PROTO_4_4_4;
> +		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, enable_4_4_4, enable_2_2_2;
> +	u32 rd_modes, wr_modes, cmd_modes, mask;
> +	const struct spi_nor_erase_type *erase_type;
> +	const struct spi_nor_read *read;
> +	int rd_midx, wr_midx, err = 0;
> +
> +	/* 2-2-2 or 4-4-4 modes must be supported by BOTH read and write */
> +	mask = (SNOR_MODE_2_2_2 | SNOR_MODE_4_4_4);
> +	cmd_modes = (modes->rd_modes & modes->wr_modes) & mask;
> +	rd_modes = (modes->rd_modes & ~mask) | cmd_modes;
> +	wr_modes = (modes->wr_modes & ~mask) | cmd_modes;

This is a little cryptic, but it's indeed correct.

> +	/* Setup read operation. */
> +	rd_midx = fls(params->rd_modes & rd_modes) - 1;
> +	if (spi_nor_midx2proto(rd_midx, &nor->read_proto)) {
> +		dev_err(nor->dev, "invalid (fast) read\n");

The error spit could certainly be more descriptive.

> +		return -EINVAL;
> +	}
> +	read = &params->reads[rd_midx];
> +	nor->read_opcode = read->opcode;
> +	nor->read_dummy = read->num_mode_clocks + read->num_wait_states;
> +
> +	/* Set page program op code and protocol. */
> +	wr_midx = fls(params->wr_modes & wr_modes) - 1;
> +	if (spi_nor_midx2proto(wr_midx, &nor->write_proto)) {
> +		dev_err(nor->dev, "invalid page program\n");
> +		return -EINVAL;
> +	}
> +	nor->program_opcode = params->page_programs[wr_midx];
> +
> +	/* Set sector erase op code and size. */
> +	erase_type = &params->erase_types[0];
> +#ifdef CONFIG_MTD_SPI_NOR_USE_4K_SECTORS
> +	for (i = 1; i < SNOR_MAX_ERASE_TYPES; ++i)
> +		if (params->erase_types[i].size == 0x0c)
> +			erase_type = &params->erase_types[i];
> +#endif
> +	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);

What about dual IO? Shouldn't the code implement the same check ?

> +	enable_4_4_4 = (nor->read_proto == SNOR_PROTO_4_4_4);
> +	enable_2_2_2 = (nor->read_proto == SNOR_PROTO_2_2_2);
> +
> +	/* Enable Quad I/O if needed. */
> +	if ((enable_quad_io || enable_4_4_4) &&
> +	    params->enable_quad_io &&
> +	    nor->reg_proto != SNOR_PROTO_4_4_4) {
> +		err = params->enable_quad_io(nor, true);
> +		if (err) {
> +			dev_err(nor->dev,
> +				"failed to enable the Quad I/O mode\n");
> +			return err;
> +		}
> +	}
> +
> +	/* Enter/Leave 2-2-2 or 4-4-4 if needed. */
> +	if (enable_2_2_2 && params->enable_2_2_2 &&
> +	    nor->reg_proto != SNOR_PROTO_2_2_2)
> +		err = params->enable_2_2_2(nor, true);
> +	else if (enable_4_4_4 && params->enable_4_4_4 &&
> +		 nor->reg_proto != SNOR_PROTO_4_4_4)
> +		err = params->enable_4_4_4(nor, true);
> +	else if (!enable_2_2_2 && params->enable_2_2_2 &&
> +		 nor->reg_proto == SNOR_PROTO_2_2_2)
> +		err = params->enable_2_2_2(nor, false);
> +	else if (!enable_4_4_4 && params->enable_4_4_4 &&
> +		 nor->reg_proto == SNOR_PROTO_4_4_4)
> +		err = params->enable_4_4_4(nor, false);
> +	if (err)
> +		return err;
> +
> +	/*
> +	 * Fix erase protocol if needed, read and write protocols should
> +	 * already be valid.
> +	 */
> +	switch (nor->reg_proto) {
> +	case SNOR_PROTO_4_4_4:
> +		nor->erase_proto = SNOR_PROTO_4_4_4;
> +		break;
> +
> +	case SNOR_PROTO_2_2_2:
> +		nor->erase_proto = SNOR_PROTO_2_2_2;
> +		break;
> +
> +	default:
> +		nor->erase_proto = SNOR_PROTO_1_1_1;
> +		break;
> +	}
> +
> +	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=%zu\n",
> +		nor->erase_opcode, nor->erase_proto, nor->mtd.erasesize);
> +
> +	return 0;
> +}

[...]

> +/* Supported modes */
> +enum spi_nor_mode_index {
> +	/* Sorted by ascending priority order */
> +	SNOR_MIDX_SLOW = 0,
> +	SNOR_MIDX_1_1_1,
> +	SNOR_MIDX_1_1_2,
> +	SNOR_MIDX_1_2_2,
> +	SNOR_MIDX_2_2_2,
> +	SNOR_MIDX_1_1_4,
> +	SNOR_MIDX_1_4_4,
> +	SNOR_MIDX_4_4_4,
> +
> +	SNOR_MIDX_MAX
> +};
> +
> +#define SNOR_MODE_SLOW		BIT(SNOR_MIDX_SLOW)
> +#define SNOR_MODE_1_1_1		BIT(SNOR_MIDX_1_1_1)
> +#define SNOR_MODE_1_1_2		BIT(SNOR_MIDX_1_1_2)
> +#define SNOR_MODE_1_2_2		BIT(SNOR_MIDX_1_2_2)
> +#define SNOR_MODE_2_2_2		BIT(SNOR_MIDX_2_2_2)
> +#define SNOR_MODE_1_1_4		BIT(SNOR_MIDX_1_1_4)
> +#define SNOR_MODE_1_4_4		BIT(SNOR_MIDX_1_4_4)
> +#define SNOR_MODE_4_4_4		BIT(SNOR_MIDX_4_4_4)

This is something I was pondering about, but why don't you encode this
SNOR mode as a 32-bit number, which would contain 8 bits for each
command/addr/data field and possibly 8 remaining bits for flags ?
This would allow for easy extraction of each component from it without
the need for all the switch() {} statements.

> +struct spi_nor_modes {
> +	u32	id_modes;	/* supported SPI modes for Read JEDEC ID */
> +	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;
> +};
> +
> +#define SNOR_OP_READ(_num_mode_clocks, _num_wait_states, _opcode) \
> +	{							  \
> +		.num_wait_states = _num_wait_states,		  \
> +		.num_mode_clocks = _num_mode_clocks,		  \
> +		.opcode = _opcode,				  \
> +	}
> +
> +struct spi_nor_erase_type {
> +	u8	size;	/* specifies 'N' so erase size = 2^N */
> +	u8	opcode;
> +};
> +
> +#define SNOR_OP_ERASE(_size, _opcode) { .size = _size, .opcode = _opcode }

Use parentheses around (_size) and (_opcode) in the macro to avoid issues.

> +#define SNOR_OP_ERASE_64K(_opcode) SNOR_OP_ERASE(0x10, _opcode)
> +#define SNOR_OP_ERASE_32K(_opcode) SNOR_OP_ERASE(0x0f, _opcode)
> +#define SNOR_OP_ERASE_4K(_opcode)  SNOR_OP_ERASE(0x0c, _opcode)

DTTO above for (_opcode)

> +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_MIDX_MAX];
> +
> +	/* Page Program settings */
> +	u32				wr_modes;
> +	u8				page_programs[SNOR_MIDX_MAX];
> +
> +	/* Sector Erase settings */
> +	struct spi_nor_erase_type	erase_types[SNOR_MAX_ERASE_TYPES];
> +
> +	int (*read_id)(struct spi_nor *nor, u8 *id, size_t id_len,
> +		       u32 id_modes);
> +	int (*enable_quad_io)(struct spi_nor *nor, bool enable);
> +	int (*enable_4_4_4)(struct spi_nor *nor, bool enable);
> +	int (*enable_2_2_2)(struct spi_nor *nor, bool enable);
> +};

[...]

-- 
Best regards,
Marek Vasut

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

* Re: [PATCH RFC 8/8] mtd: atmel-quadspi: add driver for Atmel QSPI controller
  2016-04-13 17:23 ` [PATCH RFC 8/8] mtd: atmel-quadspi: add driver for Atmel QSPI controller Cyrille Pitchen
@ 2016-04-15 22:17   ` Marek Vasut
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2016-04-15 22:17 UTC (permalink / raw)
  To: Cyrille Pitchen, computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, linux-kernel

On 04/13/2016 07:23 PM, Cyrille Pitchen wrote:
> 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>

[...]

> +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!) */

My first reaction when reading this comment was "HUH?" . Can you explain
the problem a bit better please ?

> +	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);

Why the leading underscore ?

> +	return 0;
> +}

[...]

> +static int atmel_qspi_probe(struct platform_device *pdev)
> +{
> +	struct device_node *child, *np = pdev->dev.of_node;
> +	struct spi_nor_modes modes = {
> +		.id_modes = (SNOR_MODE_1_1_1 |
> +			     SNOR_MODE_2_2_2 |
> +			     SNOR_MODE_4_4_4),
> +		.rd_modes = (SNOR_MODE_SLOW |
> +			     SNOR_MODE_1_1_1 |
> +			     SNOR_MODE_1_1_2 |
> +			     SNOR_MODE_1_2_2 |
> +			     SNOR_MODE_2_2_2 |
> +			     SNOR_MODE_1_1_4 |
> +			     SNOR_MODE_1_4_4 |
> +			     SNOR_MODE_4_4_4),
> +		.wr_modes = (SNOR_MODE_1_1_1 |
> +			     SNOR_MODE_1_1_2 |
> +			     SNOR_MODE_1_2_2 |
> +			     SNOR_MODE_2_2_2 |
> +			     SNOR_MODE_1_1_4 |
> +			     SNOR_MODE_1_4_4 |
> +			     SNOR_MODE_4_4_4),
> +	};
> +	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 */

Can you extract this into a separate function? I think this "setup spi
nor" part is starting to turn into a nice boilerplate code :) It'd be
nice to have it readily isolated and prepared for factoring out once
the time comes.

Does this controller support multiple SPI NORs ?

> +	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, &modes);
> +	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" },

Oh neat :)

> +	{ /* 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");
> 


-- 
Best regards,
Marek Vasut

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

* Re: [PATCH RFC 4/8] mtd: spi-nor: fix support of Dual (x-y-2) and Quad (x-y-4) SPI protocols
  2016-04-13 17:23 ` [PATCH RFC 4/8] mtd: spi-nor: fix support of Dual (x-y-2) and Quad (x-y-4) SPI protocols Cyrille Pitchen
  2016-04-15 22:09   ` Marek Vasut
@ 2016-04-24  5:06   ` R, Vignesh
  2016-04-25  9:34     ` Cyrille Pitchen
  1 sibling, 1 reply; 15+ messages in thread
From: R, Vignesh @ 2016-04-24  5:06 UTC (permalink / raw)
  To: Cyrille Pitchen, computersforpeace, linux-mtd
  Cc: marex, boris.brezillon, nicolas.ferre, linux-kernel

Hi Cyrille,

On 4/13/2016 10:53 PM, Cyrille Pitchen wrote:
[...]

> +
> +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, enable_4_4_4, enable_2_2_2;
> +	u32 rd_modes, wr_modes, cmd_modes, mask;
> +	const struct spi_nor_erase_type *erase_type;
> +	const struct spi_nor_read *read;
> +	int rd_midx, wr_midx, err = 0;
> +
> +	/* 2-2-2 or 4-4-4 modes must be supported by BOTH read and write */
> +	mask = (SNOR_MODE_2_2_2 | SNOR_MODE_4_4_4);
> +	cmd_modes = (modes->rd_modes & modes->wr_modes) & mask;
> +	rd_modes = (modes->rd_modes & ~mask) | cmd_modes;
> +	wr_modes = (modes->wr_modes & ~mask) | cmd_modes;
> +
> +	/* Setup read operation. */
> +	rd_midx = fls(params->rd_modes & rd_modes) - 1;
> +	if (spi_nor_midx2proto(rd_midx, &nor->read_proto)) {
> +		dev_err(nor->dev, "invalid (fast) read\n");
> +		return -EINVAL;
> +	}
> +	read = &params->reads[rd_midx];
> +	nor->read_opcode = read->opcode;
> +	nor->read_dummy = read->num_mode_clocks + read->num_wait_states;
> +
> +	/* Set page program op code and protocol. */
> +	wr_midx = fls(params->wr_modes & wr_modes) - 1;
> +	if (spi_nor_midx2proto(wr_midx, &nor->write_proto)) {
> +		dev_err(nor->dev, "invalid page program\n");
> +		return -EINVAL;
> +	}
> +	nor->program_opcode = params->page_programs[wr_midx];
> +
> +	/* Set sector erase op code and size. */
> +	erase_type = &params->erase_types[0];
> +#ifdef CONFIG_MTD_SPI_NOR_USE_4K_SECTORS
> +	for (i = 1; i < SNOR_MAX_ERASE_TYPES; ++i)

^^^ 'i' is undefined here.

> +		if (params->erase_types[i].size == 0x0c)
> +			erase_type = &params->erase_types[i];
> +#endif
> +	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_4_4_4 = (nor->read_proto == SNOR_PROTO_4_4_4);
> +	enable_2_2_2 = (nor->read_proto == SNOR_PROTO_2_2_2);
> +
> +	/* Enable Quad I/O if needed. */
> +	if ((enable_quad_io || enable_4_4_4) &&
> +	    params->enable_quad_io &&
> +	    nor->reg_proto != SNOR_PROTO_4_4_4) {
> +		err = params->enable_quad_io(nor, true);
> +		if (err) {
> +			dev_err(nor->dev,
> +				"failed to enable the Quad I/O mode\n");
> +			return err;
> +		}
> +	}
> +
> +	/* Enter/Leave 2-2-2 or 4-4-4 if needed. */
> +	if (enable_2_2_2 && params->enable_2_2_2 &&
> +	    nor->reg_proto != SNOR_PROTO_2_2_2)
> +		err = params->enable_2_2_2(nor, true);
> +	else if (enable_4_4_4 && params->enable_4_4_4 &&
> +		 nor->reg_proto != SNOR_PROTO_4_4_4)
> +		err = params->enable_4_4_4(nor, true);
> +	else if (!enable_2_2_2 && params->enable_2_2_2 &&
> +		 nor->reg_proto == SNOR_PROTO_2_2_2)
> +		err = params->enable_2_2_2(nor, false);
> +	else if (!enable_4_4_4 && params->enable_4_4_4 &&
> +		 nor->reg_proto == SNOR_PROTO_4_4_4)
> +		err = params->enable_4_4_4(nor, false);
> +	if (err)
> +		return err;
> +
> +	/*
> +	 * Fix erase protocol if needed, read and write protocols should
> +	 * already be valid.
> +	 */
> +	switch (nor->reg_proto) {
> +	case SNOR_PROTO_4_4_4:
> +		nor->erase_proto = SNOR_PROTO_4_4_4;
> +		break;
> +
> +	case SNOR_PROTO_2_2_2:
> +		nor->erase_proto = SNOR_PROTO_2_2_2;
> +		break;
> +
> +	default:
> +		nor->erase_proto = SNOR_PROTO_1_1_1;
> +		break;
> +	}
> +
> +	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=%zu\n",
> +		nor->erase_opcode, nor->erase_proto, nor->mtd.erasesize);
> +
> +	return 0;
> +}
> +
> +int spi_nor_scan(struct spi_nor *nor, const char *name,
> +		 struct spi_nor_modes *modes)
> +{
> +	const struct spi_nor_basic_flash_parameter *params = NULL;
>  	const struct flash_info *info = NULL;
>  	struct device *dev = nor->dev;
>  	struct mtd_info *mtd = &nor->mtd;
> @@ -1342,11 +1483,17 @@ 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->erase_proto = SNOR_PROTO_1_1_1;
> +	nor->read_proto = SNOR_PROTO_1_1_1;
> +	nor->write_proto = SNOR_PROTO_1_1_1;
> +	nor->reg_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 */
>  	if (!info)
> -		info = spi_nor_read_id(nor);
> +		info = spi_nor_read_id(nor, NULL, 0);
>  	if (IS_ERR_OR_NULL(info))
>  		return -ENOENT;
>  
> @@ -1357,7 +1504,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>  	if (name && info->id_len) {
>  		const struct flash_info *jinfo;
>  
> -		jinfo = spi_nor_read_id(nor);
> +		jinfo = spi_nor_read_id(nor, info->params, modes->id_modes);
>  		if (IS_ERR(jinfo)) {
>  			return PTR_ERR(jinfo);
>  		} else if (jinfo->id_len != info->id_len ||
> @@ -1374,6 +1521,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>  			info = jinfo;
>  		}
>  	}
> +	params = info->params;
>  
>  	mutex_init(&nor->lock);
>  
> @@ -1451,51 +1599,42 @@ 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;
> +			modes->rd_modes |= SNOR_MODE_1_1_1;
>  		else
> -			nor->flash_read = SPI_NOR_NORMAL;
> +			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;
> +		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;
> +		modes->rd_modes &= ~SNOR_MODE_1_1_1;
>  
> -	/* 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");

Now that the set_quad_mode() is removed, could you explain how spansion
flash enters SNOR_MODE_1_1_4 mode? Is it bootloader's responsibility to
flash's set quad enable bit?

Regards
Vignesh

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

* Re: [PATCH RFC 4/8] mtd: spi-nor: fix support of Dual (x-y-2) and Quad (x-y-4) SPI protocols
  2016-04-24  5:06   ` R, Vignesh
@ 2016-04-25  9:34     ` Cyrille Pitchen
  0 siblings, 0 replies; 15+ messages in thread
From: Cyrille Pitchen @ 2016-04-25  9:34 UTC (permalink / raw)
  To: R, Vignesh, computersforpeace, linux-mtd
  Cc: marex, boris.brezillon, nicolas.ferre, linux-kernel

Hi Vignesh,

Le 24/04/2016 07:06, R, Vignesh a écrit :
> Hi Cyrille,
> 
> On 4/13/2016 10:53 PM, Cyrille Pitchen wrote:
> [...]
>> @@ -1451,51 +1599,42 @@ 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;
>> +			modes->rd_modes |= SNOR_MODE_1_1_1;
>>  		else
>> -			nor->flash_read = SPI_NOR_NORMAL;
>> +			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;
>> +		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;
>> +		modes->rd_modes &= ~SNOR_MODE_1_1_1;
>>  
>> -	/* 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");
> 
> Now that the set_quad_mode() is removed, could you explain how spansion
> flash enters SNOR_MODE_1_1_4 mode? Is it bootloader's responsibility to
> flash's set quad enable bit?
> 

The proposed mechanics relies on the struct spi_nor_basic_flash_parameter.
This structure provides the same kind of parameters that those we could find
in JEDEC SFDP (Serial Flash Discoverable Parameters):
- (Fast) Read op codes and protocols supported by the memory and the associated
  number of dummy cycles. More precisely the number of mode clock and wait
  state cycle.
- Page Program op code and protocols.
- Sector/Block Erase op codes and the associated erase size.
- The "Quad Enable Requirements (QER)" which tells us "whether the device 
  contains a Quad Enable (QE) bit used to enable 1-1-4 and 1-4-4 quad read or
  quad program operations" as found in the JESD216B specification and
  implemented here through the optional .enable_quad_io() hook.
- The "4-4-4 mode enable and disable sequences", implemented by the optional
  .enable_4_4_4() hook.
- The optional .enable_2_2_2() hook: the JESD216B specification misses to
  describe this procedure, only used by Micron AFAIK.

For this RFC series, I've only provided a patch (patch 6) for Micron as an
example of the new way to support QSPI memories.
I've already written another patch for the support of Macronix memories but I
didn't publish it yet.

I was waiting for comments on the series to know whether I'm on the good way
before spending much time on writing support to other manufacturers.

For Spansion, the .enable_4_4_4() and .enable_2_2_2() would not be implemented
as Spansion QSPI memories only supports 1-1-2, 1-2-2, 1-1-4 and 1-4-4
protocols. Hence only the .enable_quad_io() hook is needed. This hook should
be implemented using something very closed to the spansion_quad_enable()
function.

So at one side the struct spi_nor_basic_flash_parameter describes the SPI
memory hardware capabilities and SPI protocols it supports.
On the other side, the struct spi_nor_modes allows the caller of spi_nor_scan()
to describe the hardware capabilities of the SPI controller; which SPI
protocols the controller supports and which SPI protocols the user wants to
allow or disable.

As an example, if a QSPI memory has already entered its 4-4-4 mode, you can
still probe the memory from Linux by reading the JEDEC ID as long as you set
the SNOR_MODE_4_4_4 bit in modes.id_modes.
Then, if you don't set the SNOR_MODE_4_4_4 bit in modes.rd_modes but only the
SNOR_MODE_1_1_4 bit, spi_nor_scan() first makes the SPI exit its 4-4-4 mode
then select the SPI 1-1-4 (if supported by the memory) for Fast Read
operations. You can do so if for some reason you'd rather use the SPI 1-1-4
protocol instead of the 4-4-4.

So it's now up to the caller of spi_nor_scan() to tell the framework what
protocols it supports and what protocols it wants to use. The spi-nor framework
will select the best match between the memory and controller hardware
capabilities.

Anyway, thanks for the review! :)

> Regards
> Vignesh
> 

Best regards,

Cyrille

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

* Re: [PATCH RFC 4/8] mtd: spi-nor: fix support of Dual (x-y-2) and Quad (x-y-4) SPI protocols
  2016-04-15 22:09   ` Marek Vasut
@ 2016-04-25 12:01     ` Cyrille Pitchen
  0 siblings, 0 replies; 15+ messages in thread
From: Cyrille Pitchen @ 2016-04-25 12:01 UTC (permalink / raw)
  To: Marek Vasut, computersforpeace, linux-mtd
  Cc: nicolas.ferre, boris.brezillon, linux-kernel

Hi Marek,

Le 16/04/2016 00:09, Marek Vasut a écrit :
> On 04/13/2016 07:23 PM, Cyrille Pitchen wrote:
> 
> [...]
> 
> Hi!
> 
> [...]
> 
>> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
>> index 9d6854467651..12112f7ae1a4 100644
>> --- a/drivers/mtd/devices/m25p80.c
>> +++ b/drivers/mtd/devices/m25p80.c
>> @@ -105,10 +105,10 @@ static void 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:
> 
> I have to say, I am not a big fan of replacing a macro with numeric
> constant, but that might be a matter of taste.
> 

I guess here I could use the SPI_NBITS_{SINGLE, DUAL, QUAD} macros as defined
in include/linux/spi/spi.h
>>  		return 2;
>> -	case SPI_NOR_QUAD:
>> +	case 4:
>>  		return 4;
>>  	default:
>>  		return 0;
> 
> [...]
> 
>> -int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>> +static int spi_nor_midx2proto(int midx, enum spi_nor_protocol *proto)
> 
> It is entirely inobvious what this function does from it's name.
> I had to scroll through the code to figure out what midx means
> and that it's "mode index". Either add a comment and rename the
> function. I would be in favor of the later.
> 

I'm thinking about a way to simply remove the enum spi_nor_mode_index.
1 - I need to build bit masks for struct spi_nor_modes and struct
    spi_nor_basic_flash_parameter so I can easily select the best match
    between the memory (mask1) and the controller (mask2) capabilities:
    fls(mask1 & mask2)
2 - I need to easily extract the three widths x, y and z from SPI x-y-z
    protocol (x = code, y = addr, z = data).

So I plan to use another way to encode the widths:

/* 2 bits are enough to encode the protocol class */
enum spi_nor_protocol_class {
	SNOR_PCLASS_1_1_N,
	SNOR_PCLASS_1_N_N,
	SNOR_PCLASS_N_N_N,

	SNOR_PCLASS_MAX
};

/* 3 bits are enough to encode widths up to 2^7 = 128 */
enum spi_nor_protocol_width {
	SNOR_PWIDTH_1,
	SNOR_PWIDTH_2,
	SNOR_PWIDTH_4,
	SNOR_PWDITH_8,
};

#define SNOR_PROTO(pclass, pwidth) \
	((((pwidth) & 0x7) << 2) | ((pclass) & 0x3))

#define SNOR_PROTO2CLASS(proto)	\
	((enum spi_nor_protocol_class)((proto) & 0x3))

#define SNOR_PROTO2WIDTH(proto) \
	((enum spi_nor_protocol_width)(((proto) >> 2) & 0x7))

enum spi_nor_protocol {
	SNOR_PROTO_1_1_1 = SNOR_PROTO(SNOR_PCLASS_1_1_N, SNOR_PWIDTH_1)
	SNOR_PROTO_1_1_4 = SNOR_PROTO(SNOR_PCLASS_1_1_N, SNOR_PWIDTH_4)
	SNOR_PROTO_1_2_2 = SNOR_PROTO(SNOR_PCLASS_1_N_N, SNOR_PWDITH_2)
	SNOR_PROTO_4_4_4 = SNOR_PROTO(SNOR_PCLASS_N_N_N, SNOR_PWIDTH_4)
[...]
};

/*
 * 1 - The higher pwidth, the higher protocol priority.
 * 2 - For a given pwidth, the higher pclass, the higher protocol priority.
 */
#define SNOR_PROTO_BIT(pclass, pwidth) \
	BIT((pwidth) * SNOR_PCLASS_MAX + (pclass))

/* Helper conversion functions */

/*
 * spi_nor_protocol_*_width() functions are used at runtime for instance
 * by m25p80_read().
 */
static inline u8 spi_nor_protocol_code_width(enum spi_nor_protocol proto)
{
	enum spi_nor_protocol_class pclass = SNOR_PROTO2CLASS(proto);
	enum spi_nor_protocol_width pwidth = SNOR_PROTO2WIDTH(proto);

	return (pclass == SNOR_PCLASS_N_N_N) ? (1 << pwidth) : 1;
}

static inline u8 spi_nor_protocol_addr_width(enum spi_nor_protocol proto)
{
	enum spi_nor_protocol_class pclass = SNOR_PROTO2CLASS(proto);
	enum spi_nor_protocol_width pwidth = SNOR_PROTO2WIDTH(proto);

	return (pclass == SNOR_PCLASS_1_1_N) ? 1 : (1 << pwidth);
}

static inline u8 spi_nor_protocol_data_width(enum spi_nor_protocol proto)
{
	enum spi_nor_protocol_width pwidth = SNOR_PROTO2WIDTH(proto);

	return (1 << pwdith);
}

/* Only used once from spi_nor_scan() */
static inline int spi_nor_bitmask2proto(uint32_t bitmask,
					enum spi_nor_protocol *proto)
{
	int pclass, pwidth, index = fls(bitmask) - 1;

	if (unlikely(index < 0))
		return -EINVAL;

	pclass = index % SNOR_PCLASS_MAX;
	pwidth = index / SNOR_PCLASS_MAX;
	*proto = SNOR_PROTO(pclass, pwidth);

	return 0;
}

>>  {
>> +	switch (midx) {
>> +	case SNOR_MIDX_SLOW:
>> +	case SNOR_MIDX_1_1_1:
>> +		*proto = SNOR_PROTO_1_1_1;
> 
> Can you not unify SNOR_PROTO_* and SNOR_MIDX_* , so this odd switch
> would go away entirely ? If not, make this into a lookup table at
> least.
> 

With the spi_nor_bitmask2proto() function described above I could get rid of
this switch statement. I just need to find out a mean to encode support
of Read vs Fast Read.

pclass belongs to {0, 1, 2} and pwidth to {0, 1, 2, .., 7} so the maximum
index is 7 * 3 + 2 = 23. BIT(24) up to BIT(31) are available for flags such
as "support slow read", "support fast read".

>> +		break;
>> +
>> +	case SNOR_MIDX_1_1_2:
>> +		*proto = SNOR_PROTO_1_1_2;
>> +		break;
>> +
>> +	case SNOR_MIDX_1_2_2:
>> +		*proto = SNOR_PROTO_1_2_2;
>> +		break;
>> +
>> +	case SNOR_MIDX_2_2_2:
>> +		*proto = SNOR_PROTO_2_2_2;
>> +		break;
>> +
>> +	case SNOR_MIDX_1_1_4:
>> +		*proto = SNOR_PROTO_1_1_4;
>> +		break;
>> +
>> +	case SNOR_MIDX_1_4_4:
>> +		*proto = SNOR_PROTO_1_4_4;
>> +		break;
>> +
>> +	case SNOR_MIDX_4_4_4:
>> +		*proto = SNOR_PROTO_4_4_4;
>> +		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, enable_4_4_4, enable_2_2_2;
>> +	u32 rd_modes, wr_modes, cmd_modes, mask;
>> +	const struct spi_nor_erase_type *erase_type;
>> +	const struct spi_nor_read *read;
>> +	int rd_midx, wr_midx, err = 0;
>> +
>> +	/* 2-2-2 or 4-4-4 modes must be supported by BOTH read and write */
>> +	mask = (SNOR_MODE_2_2_2 | SNOR_MODE_4_4_4);
>> +	cmd_modes = (modes->rd_modes & modes->wr_modes) & mask;
>> +	rd_modes = (modes->rd_modes & ~mask) | cmd_modes;
>> +	wr_modes = (modes->wr_modes & ~mask) | cmd_modes;
> 
> This is a little cryptic, but it's indeed correct.
> 
Maybe I can develop the commit some more: the SNOR_MODE_4_4_4 bit must be set
in both rd_modes and wr_modes bitmask, otherwise it's not relevant so I clear
it. I do the same for the SNOR_MODE_2_2_2 bit.

>> +	/* Setup read operation. */
>> +	rd_midx = fls(params->rd_modes & rd_modes) - 1;
>> +	if (spi_nor_midx2proto(rd_midx, &nor->read_proto)) {
>> +		dev_err(nor->dev, "invalid (fast) read\n");
> 
> The error spit could certainly be more descriptive.
> 
>> +		return -EINVAL;
>> +	}
>> +	read = &params->reads[rd_midx];
>> +	nor->read_opcode = read->opcode;
>> +	nor->read_dummy = read->num_mode_clocks + read->num_wait_states;
>> +
>> +	/* Set page program op code and protocol. */
>> +	wr_midx = fls(params->wr_modes & wr_modes) - 1;
>> +	if (spi_nor_midx2proto(wr_midx, &nor->write_proto)) {
>> +		dev_err(nor->dev, "invalid page program\n");
>> +		return -EINVAL;
>> +	}
>> +	nor->program_opcode = params->page_programs[wr_midx];
>> +
>> +	/* Set sector erase op code and size. */
>> +	erase_type = &params->erase_types[0];
>> +#ifdef CONFIG_MTD_SPI_NOR_USE_4K_SECTORS
>> +	for (i = 1; i < SNOR_MAX_ERASE_TYPES; ++i)
>> +		if (params->erase_types[i].size == 0x0c)
>> +			erase_type = &params->erase_types[i];
>> +#endif
>> +	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);
> 
> What about dual IO? Shouldn't the code implement the same check ?
> 
Dual I/O doesn't exit: the SPI 1-1-1 protocol use MOSI/IO0 to send data to the
memory and MISO/IO1 to receive data from the memory.
The SPI 1-1-2, 1-1-2 and 2-2-2 protocols are also limited to IO0 and IO1 to
send and receive data.

The issue comes with Quad SPI protocols (SPI 1-1-4, 1-4-4 and 4-4-4). Those
protocol require two additional I/O lines, IO2 and IO3, which didn't exist
in the legacy SPI protocol. This is why most manufacturers provide a mean
to reassign 2 pins to other function:
- #Write Protect <-> IO2
- #Reset or #Hold <-> IO3

If you want to use Quad SPI protocols, you need IO2 and IO3 but you are likely
not to be able to use the write protect, reset and hold features any longer.
Actually it might also depends on the package: some packages have enough pins
so #Write Protect and IO2 don't share the same pin for instance.

>> +	enable_4_4_4 = (nor->read_proto == SNOR_PROTO_4_4_4);
>> +	enable_2_2_2 = (nor->read_proto == SNOR_PROTO_2_2_2);
>> +
>> +	/* Enable Quad I/O if needed. */
>> +	if ((enable_quad_io || enable_4_4_4) &&
>> +	    params->enable_quad_io &&
>> +	    nor->reg_proto != SNOR_PROTO_4_4_4) {
>> +		err = params->enable_quad_io(nor, true);
>> +		if (err) {
>> +			dev_err(nor->dev,
>> +				"failed to enable the Quad I/O mode\n");
>> +			return err;
>> +		}
>> +	}
>> +
>> +	/* Enter/Leave 2-2-2 or 4-4-4 if needed. */
>> +	if (enable_2_2_2 && params->enable_2_2_2 &&
>> +	    nor->reg_proto != SNOR_PROTO_2_2_2)
>> +		err = params->enable_2_2_2(nor, true);
>> +	else if (enable_4_4_4 && params->enable_4_4_4 &&
>> +		 nor->reg_proto != SNOR_PROTO_4_4_4)
>> +		err = params->enable_4_4_4(nor, true);
>> +	else if (!enable_2_2_2 && params->enable_2_2_2 &&
>> +		 nor->reg_proto == SNOR_PROTO_2_2_2)
>> +		err = params->enable_2_2_2(nor, false);
>> +	else if (!enable_4_4_4 && params->enable_4_4_4 &&
>> +		 nor->reg_proto == SNOR_PROTO_4_4_4)
>> +		err = params->enable_4_4_4(nor, false);
>> +	if (err)
>> +		return err;
>> +
>> +	/*
>> +	 * Fix erase protocol if needed, read and write protocols should
>> +	 * already be valid.
>> +	 */
>> +	switch (nor->reg_proto) {
>> +	case SNOR_PROTO_4_4_4:
>> +		nor->erase_proto = SNOR_PROTO_4_4_4;
>> +		break;
>> +
>> +	case SNOR_PROTO_2_2_2:
>> +		nor->erase_proto = SNOR_PROTO_2_2_2;
>> +		break;
>> +
>> +	default:
>> +		nor->erase_proto = SNOR_PROTO_1_1_1;
>> +		break;
>> +	}
>> +
>> +	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=%zu\n",
>> +		nor->erase_opcode, nor->erase_proto, nor->mtd.erasesize);
>> +
>> +	return 0;
>> +}
> 
> [...]
> 
>> +/* Supported modes */
>> +enum spi_nor_mode_index {
>> +	/* Sorted by ascending priority order */
>> +	SNOR_MIDX_SLOW = 0,
>> +	SNOR_MIDX_1_1_1,
>> +	SNOR_MIDX_1_1_2,
>> +	SNOR_MIDX_1_2_2,
>> +	SNOR_MIDX_2_2_2,
>> +	SNOR_MIDX_1_1_4,
>> +	SNOR_MIDX_1_4_4,
>> +	SNOR_MIDX_4_4_4,
>> +
>> +	SNOR_MIDX_MAX
>> +};
>> +
>> +#define SNOR_MODE_SLOW		BIT(SNOR_MIDX_SLOW)
>> +#define SNOR_MODE_1_1_1		BIT(SNOR_MIDX_1_1_1)
>> +#define SNOR_MODE_1_1_2		BIT(SNOR_MIDX_1_1_2)
>> +#define SNOR_MODE_1_2_2		BIT(SNOR_MIDX_1_2_2)
>> +#define SNOR_MODE_2_2_2		BIT(SNOR_MIDX_2_2_2)
>> +#define SNOR_MODE_1_1_4		BIT(SNOR_MIDX_1_1_4)
>> +#define SNOR_MODE_1_4_4		BIT(SNOR_MIDX_1_4_4)
>> +#define SNOR_MODE_4_4_4		BIT(SNOR_MIDX_4_4_4)
> 
> This is something I was pondering about, but why don't you encode this
> SNOR mode as a 32-bit number, which would contain 8 bits for each
> command/addr/data field and possibly 8 remaining bits for flags ?
> This would allow for easy extraction of each component from it without
> the need for all the switch() {} statements.
> 

This is what I did with SNOR_PROTO(code, addr, data) macro below in the very
same file: I use 4 bits per code, addr, and data hence 12 bits.
The SNOR_PROTO() macro is used to set the values in the enum spi_nor_protocol.
This encoding was chosen so it's easy to extract the code, address and data
widths from an enum spi_nor_protocol such as nor->read_proto.

However this encoding was not suited to build bitmask used by the struct
spi_nor_modes. Hence the new encoding I proposed earlier with pclass and
pwidth.

>> +struct spi_nor_modes {
>> +	u32	id_modes;	/* supported SPI modes for Read JEDEC ID */
>> +	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;
>> +};
>> +
>> +#define SNOR_OP_READ(_num_mode_clocks, _num_wait_states, _opcode) \
>> +	{							  \
>> +		.num_wait_states = _num_wait_states,		  \
>> +		.num_mode_clocks = _num_mode_clocks,		  \
>> +		.opcode = _opcode,				  \
>> +	}
>> +
>> +struct spi_nor_erase_type {
>> +	u8	size;	/* specifies 'N' so erase size = 2^N */
>> +	u8	opcode;
>> +};
>> +
>> +#define SNOR_OP_ERASE(_size, _opcode) { .size = _size, .opcode = _opcode }
> 
> Use parentheses around (_size) and (_opcode) in the macro to avoid issues.
> 
You're right, I forgot them :)

>> +#define SNOR_OP_ERASE_64K(_opcode) SNOR_OP_ERASE(0x10, _opcode)
>> +#define SNOR_OP_ERASE_32K(_opcode) SNOR_OP_ERASE(0x0f, _opcode)
>> +#define SNOR_OP_ERASE_4K(_opcode)  SNOR_OP_ERASE(0x0c, _opcode)
> 
> DTTO above for (_opcode)
> 
>> +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_MIDX_MAX];
>> +
>> +	/* Page Program settings */
>> +	u32				wr_modes;
>> +	u8				page_programs[SNOR_MIDX_MAX];
>> +
>> +	/* Sector Erase settings */
>> +	struct spi_nor_erase_type	erase_types[SNOR_MAX_ERASE_TYPES];
>> +
>> +	int (*read_id)(struct spi_nor *nor, u8 *id, size_t id_len,
>> +		       u32 id_modes);
>> +	int (*enable_quad_io)(struct spi_nor *nor, bool enable);
>> +	int (*enable_4_4_4)(struct spi_nor *nor, bool enable);
>> +	int (*enable_2_2_2)(struct spi_nor *nor, bool enable);
>> +};
> 
> [...]
> 

Thanks for your review,

Best regards,

Cyrille

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

end of thread, other threads:[~2016-04-25 12:01 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-13 17:23 [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories Cyrille Pitchen
2016-04-13 17:23 ` [PATCH RFC 1/8] mtd: spi-nor: add an alternative method to support memory >16MiB Cyrille Pitchen
2016-04-13 17:23 ` [PATCH RFC 2/8] mtd: spi-nor: allow different flash_info entries to share the same JEDEC ID Cyrille Pitchen
2016-04-13 17:23 ` [PATCH RFC 3/8] mtd: spi-nor: add entry for Macronix mx25l25673g Cyrille Pitchen
2016-04-13 17:23 ` [PATCH RFC 4/8] mtd: spi-nor: fix support of Dual (x-y-2) and Quad (x-y-4) SPI protocols Cyrille Pitchen
2016-04-15 22:09   ` Marek Vasut
2016-04-25 12:01     ` Cyrille Pitchen
2016-04-24  5:06   ` R, Vignesh
2016-04-25  9:34     ` Cyrille Pitchen
2016-04-13 17:23 ` [PATCH RFC 5/8] mtd: m25p80: add support of dual and quad spi protocols to all commands Cyrille Pitchen
2016-04-13 17:23 ` [PATCH RFC 6/8] mtd: spi-nor: add support for Micron Dual and Quad SPI memories Cyrille Pitchen
2016-04-13 17:23 ` [PATCH RFC 7/8] Documentation: atmel-quadspi: add binding file for Atmel QSPI driver Cyrille Pitchen
2016-04-13 17:23 ` [PATCH RFC 8/8] mtd: atmel-quadspi: add driver for Atmel QSPI controller Cyrille Pitchen
2016-04-15 22:17   ` Marek Vasut
2016-04-15 21:48 ` [PATCH RFC 0/8] mtd: spi-nor: fix support of Quad SPI memories Marek Vasut

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.