[PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller

[PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller

[Cyrille Pitchen]

Hi all,

this series of patches adds support to the Atmel QSPI controller available
on sama5d2 SoCs. It was tested on a sama5d2 xplained ultra board with a 
Micron n25q128a13 QSPI memory and a at25df321a SPI memory.

In order to use the Micron memory in its Quad SPI mode, the spi-nor
framework needed to be patched to fix the support of Quad/Dual SPI
protocols with some memory manufacturers such as Spansion, Micron and
Macronix. There are many comments in the source code to explain the
implementation choices based on the datasheets from memory manufacturers.


This series was based and tested on linux-next-20151207

1 - Atmel QSPI + Micron n25q128a13 (atmel-quadspi.c driver)

SPI 1-1-1: This mode was tested replacing SPI_NOR_QUAD by SPI_NOR_FAST as
           argument to spi_nor_scan() called from atmel_qspi_probe().

SPI 1-1-4: Bootloaders (at91bootstrap/uboot) don't enable the Quad SPI
           mode of the Micron memory. When probed from Linux, the memory
           uses its Extended SPI mode and replies to the regular Read ID
           (0x9f) command.

SPI 4-4-4: The romcode enabled the Quad SPI mode the of Micron memory
           before loading the at91bootstrap. When probed from Linux, the
           memory uses its Quad SPI mode and no longer replies to the
           regular Read ID (0x9f) command but instead to the Read ID
           Multiple I/O (0xaf) command. The memory expects ALL commands
           to use the SPI 4-4-4 protocol.

2 - Atmel SPI + at25df321a (m25p80.c driver)

SPI 1-1-1: tested with the m25p80 driver for non regression purpose.


mtd_speedtest was run with the Atmel QSPI controller + Micron memory to
compare the performances of normal and quad SPI protocols. The SPI
bus clock was configured to 83 MHz.

1 - Fast Read 1-1-1

mtd_speedtest: testing eraseblock write speed
mtd_speedtest: eraseblock read speed is 9319 KiB/s
[...]
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 6649 KiB/s
[...]
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 7757 KiB/s

2 - Fast Read 1-1-4

mtd_speedtest: testing eraseblock read speed
mtd_speedtest: eraseblock read speed is 30117 KiB/s
[...]
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 13096 KiB/s
[...]
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 18224 KiB/s


So the performance improvements are:
eraseblock read speed (65536 bytes) : +223%
page read speed (512 bytes)         :  +97%
2 page read speed (1024 bytes)      : +135%


Best Regards,

Cyrille

Cyrille Pitchen (5):
  mtd: spi-nor: properly detect the memory when it boots in Quad or Dual
    mode
  mtd: spi-nor: fix Quad SPI mode support for Spansion, Micron and
    Macronix
  mtd: m25p80: add support of dual and quad spi protocols to all
    commands
  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                       | 233 +++++-
 drivers/mtd/spi-nor/Kconfig                        |   8 +
 drivers/mtd/spi-nor/Makefile                       |   3 +-
 drivers/mtd/spi-nor/atmel-quadspi.c                | 877 +++++++++++++++++++++
 drivers/mtd/spi-nor/spi-nor.c                      | 835 +++++++++++++++++---
 include/linux/mtd/spi-nor.h                        |  38 +-
 7 files changed, 1880 insertions(+), 146 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

[PATCH linux-next 1/5] mtd: spi-nor: properly detect the memory when it boots in Quad or Dual mode

[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.

Micron and Macronix memories, once their Quad (or dual for Micron) mode
enabled, no longer process the regular JEDEC Read ID (0x9f) command but
instead reply to a new command: JEDEC Read ID Multiple I/O (0xaf).
Besides, in Quad mode both memory manufacturers expect ALL commands to
use the SPI 4-4-4 protocol. For Micron memories, enabling their Dual mode
implies to use the SPI 2-2-2 protocol for ALL commands.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 3b2460efc019..bf17736750c1 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -73,6 +73,11 @@ struct flash_info {
 
 #define JEDEC_MFR(info)	((info)->id[0])
 
+struct read_id_config {
+	enum read_mode		mode;
+	enum spi_protocol	proto;
+};
+
 static const struct flash_info *spi_nor_match_id(const char *name);
 
 /*
@@ -867,11 +872,16 @@ 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,
+						enum read_mode mode)
 {
-	int			tmp;
+	int			i, tmp;
 	u8			id[SPI_NOR_MAX_ID_LEN];
 	const struct flash_info	*info;
+	static const struct read_id_config configs[] = {
+		{SPI_NOR_QUAD, SPI_PROTO_4_4_4},
+		{SPI_NOR_DUAL, SPI_PROTO_2_2_2}
+	};
 
 	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
 	if (tmp < 0) {
@@ -879,6 +889,34 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
 		return ERR_PTR(tmp);
 	}
 
+	/* Special case for Micron/Macronix qspi nor. */
+	if ((id[0] == 0xff && id[1] == 0xff && id[2] == 0xff) ||
+	    (id[0] == 0x00 && id[1] == 0x00 && id[2] == 0x00))
+		for (i = 0; i < ARRAY_SIZE(configs); ++i) {
+			if (configs[i].mode != mode)
+				continue;
+
+			/* Set this protocol for all commands. */
+			nor->reg_proto = configs[i].proto;
+			nor->read_proto = configs[i].proto;
+			nor->write_proto = configs[i].proto;
+			nor->erase_proto = configs[i].proto;
+
+			/*
+			 * Multiple I/O Read ID only returns the Manufacturer ID
+			 * (1 byte) and the Device ID (2 bytes). So we reset the
+			 * remaining bytes.
+			 */
+			memset(id, 0, sizeof(id));
+			tmp = nor->read_reg(nor, SPINOR_OP_MIO_RDID, id, 3);
+			if (tmp < 0) {
+				dev_dbg(nor->dev,
+					"error %d reading JEDEC ID Multi I/O\n",
+					tmp);
+				return ERR_PTR(tmp);
+			}
+		}
+
 	for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
 		info = &spi_nor_ids[tmp];
 		if (info->id_len) {
@@ -1178,11 +1216,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 = SPI_PROTO_1_1_1;
+	nor->read_proto = SPI_PROTO_1_1_1;
+	nor->write_proto = SPI_PROTO_1_1_1;
+	nor->reg_proto = SPI_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, mode);
 	if (IS_ERR_OR_NULL(info))
 		return -ENOENT;
 
@@ -1193,7 +1237,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, mode);
 		if (IS_ERR(jinfo)) {
 			return PTR_ERR(jinfo);
 		} else if (jinfo != info) {
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index fac3f6f53981..c91986a99caf 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -75,8 +75,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 */
@@ -105,6 +106,16 @@ enum read_mode {
 	SPI_NOR_QUAD,
 };
 
+enum spi_protocol {
+	SPI_PROTO_1_1_1,	/* SPI */
+	SPI_PROTO_1_1_2,	/* Dual Output */
+	SPI_PROTO_1_1_4,	/* Quad Output */
+	SPI_PROTO_1_2_2,	/* Dual IO */
+	SPI_PROTO_1_4_4,	/* Quad IO */
+	SPI_PROTO_2_2_2,	/* Dual Command */
+	SPI_PROTO_4_4_4,	/* Quad Command */
+};
+
 #define SPI_NOR_MAX_CMD_SIZE	8
 enum spi_nor_ops {
 	SPI_NOR_OPS_READ = 0,
@@ -132,6 +143,10 @@ enum spi_nor_option_flags {
  * @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
@@ -160,6 +175,10 @@ struct spi_nor {
 	u8			read_opcode;
 	u8			read_dummy;
 	u8			program_opcode;
+	enum spi_protocol	erase_proto;
+	enum spi_protocol	read_proto;
+	enum spi_protocol	write_proto;
+	enum spi_protocol	reg_proto;
 	enum read_mode		flash_read;
 	bool			sst_write_second;
 	u32			flags;
-- 
1.8.2.2

[PATCH linux-next 2/5] mtd: spi-nor: fix Quad SPI mode support for Spansion, Micron and Macronix

[Cyrille Pitchen]

This patch reworks the support of Quad and Dual SPI protocols for Micron,
Spansion and Macronix Quad/Dual capable memories. Indeed, in the best
case, only Spansion memories are correctly supported by the current
spi-nor framework.

1 - Micron:
When their Quad SPI mode is enabled, Micron spi-nor memories expect all
commands to use the SPI 4-4-4 protocol. Also when the Dual SPI mode is
enabled, all commands must use the SPI 2-2-2 protocol.

Before this patch, the spi-nor framework used to always enable the Quad
mode when the mode argument of spi_nor_scan() took the value SPI_NOR_QUAD.
That was not suited with drivers only supporting SPI 1-x-4 protocols but
not the 4-4-4 (e.g. the m25p80 driver). Also the SPI controller was not
notified about which SPI protocol to use to transfert command. We cannot
rely only on the op code: in Extended SPI mode the 0x6b command must use
the SPI 1-1-4 protocol whereas in Quad SPi mode the SPI 4-4-4 protocol
must be use instead.

After this patch, the spi-nor framework uses the result of the
spi_nor_read_id() function to choose the right SPI protocol to be used.
If the reg_proto was set to SPI_PROTO_4_4_4, we already know that the Quad
SPI mode is already enabled and that the SPI controller supports the SPI
4-4-4 protocol (otherwise it would have fail to read the JEDEC ID with the
0xaf op code). For the very same reason, if the reg_proto was set to
SPI_PROTO_2_2_2, we already know that the Dual mode is already enabled and
that the SPI controller supports the SPI 2-2-2 protocol.
Otherwise we switch back to the Extended SPI protocol, which supports at
least the Fast Read commands:
- 1-1-1 (0x0b)
- Dual Output 1-1-2 (0x3b)
- Quad Output 1-1-4 (0x6b)

We also safely set the number of dummy cycles to 8 for Fast Read commands
through the Volatile Configuration Register (VCR): some drivers (m25p80)
or SPI controllers only support a number of dummy cycles multiple of 8.
This number may have previouly been set to an unsupported value by an
early bootloader or at reset thanks to the Non-Volatile Configuration
Register.

Finally the XIP bit is always set in the VCR to disable the Continuous
Read mode as we don't want to care about mode cycles.

2 - Macronix:
When the QPI mode is enabled, all commands must use the SPI 4-4-4 protocol
and only the 0xeb op code is supported for Fast Read commands.
Before this patch, the spi-nor framework used to force the QPI mode but
used the 0x6b op code for Fast Read commands when the SPI controller
claims to support Quad SPI mode.
This patch uses the result of spi_nor_read_id() to guess whether the QPI
mode is both enable and supported by the SPI controller (otherwise it
would have failed to read the JEDEC ID with the 0xaf op code).
When the QPI mode is disabled, Macronix memories still support the
following Fast Read commands:
- 1-1-1 (0x0b)
- Dual Output 1-1-2 (0x3b)
- Quad Output 1-1-4 (0x6b)
So if the QPI mode has not already been enabled, there is not need to
enable it. We also avoid the 0xbb (Dual I/O 1-2-2) and 0xeb (Quad I/O
1-4-4) op codes on purpose as we don't want to care about the value to set
in mode cycles not to enter the Continuous Read (Performance Enhance)
mode.

As for Micron memories, the spi-nor framework now safely sets the number
of dummy cycles to 8 thanks to 2 volatile bits inside the Configuration
Register.

3 - Spansion:
As for Macronix, we avoid the 0xbb (Dual I/O 1-2-2) and 0xeb (Quad I/O
1-4-4) op codes on purpose as we don't want to care about the value to set
in mode cycles not to enter in the Continuous Read mode.

Besides, we only care about the Quad Enable bit inside the Configuration
Register (CR) when using Quad operations. In such a case, we first check
its state before trying to set it. Now we also notify the user about the
update of this non-volatile bit.

We also check the Latency Code (LC) in CR to know the exact number of
dummy cycles to use when performing a Fast Read operation. Currently only
the 0x0b, 0x3b and 0x6b op codes are used to perform Fast Read operation
so the number of dummy cycles is always either 0 or 8. Hence no regression
should be introduced.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index bf17736750c1..30b63ab96410 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -138,24 +138,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.
  */
@@ -1065,39 +1047,238 @@ write_err:
 	return ret;
 }
 
-static int macronix_quad_enable(struct spi_nor *nor)
+/*
+ * Write status Register and configuration register with 2 bytes
+ * The first byte will be written to the status register, while the
+ * second byte will be written to the configuration register.
+ * Return negative if error occured.
+ */
+static int write_sr_cr(struct spi_nor *nor, u16 val)
 {
-	int ret, val;
+	nor->cmd_buf[0] = val & 0xff;
+	nor->cmd_buf[1] = (val >> 8);
 
-	val = read_sr(nor);
-	write_enable(nor);
+	return nor->write_reg(nor, SPINOR_OP_WRSR, nor->cmd_buf, 2);
+}
 
-	write_sr(nor, val | SR_QUAD_EN_MX);
+static int macronix_dummy2code(u8 read_opcode, u8 read_dummy, u8 *dc)
+{
+	switch (read_opcode) {
+	case SPINOR_OP_READ:
+	case SPINOR_OP_READ4:
+		*dc = 0;
+		break;
 
-	if (spi_nor_wait_till_ready(nor))
-		return 1;
+	case SPINOR_OP_READ_FAST:
+	case SPINOR_OP_READ_1_1_2:
+	case SPINOR_OP_READ_1_1_4:
+	case SPINOR_OP_READ4_FAST:
+	case SPINOR_OP_READ4_1_1_2:
+	case SPINOR_OP_READ4_1_1_4:
+		switch (read_dummy) {
+		case 6:
+			*dc = 1;
+			break;
+		case 8:
+			*dc = 0;
+			break;
+		case 10:
+			*dc = 3;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+
+	case SPINOR_OP_READ_1_2_2:
+	case SPINOR_OP_READ4_1_2_2:
+		switch (read_dummy) {
+		case 4:
+			*dc = 0;
+			break;
+		case 6:
+			*dc = 1;
+			break;
+		case 8:
+			*dc = 2;
+			break;
+		case 10:
+			*dc = 3;
+		default:
+			return -EINVAL;
+		}
+		break;
+
+	case SPINOR_OP_READ_1_4_4:
+	case SPINOR_OP_READ4_1_4_4:
+		switch (read_dummy) {
+		case 4:
+			*dc = 1;
+			break;
+		case 6:
+			*dc = 0;
+			break;
+		case 8:
+			*dc = 2;
+			break;
+		case 10:
+			*dc = 3;
+		default:
+			return -EINVAL;
+		}
+		break;
 
-	ret = read_sr(nor);
-	if (!(ret > 0 && (ret & SR_QUAD_EN_MX))) {
-		dev_err(nor->dev, "Macronix Quad bit not set\n");
+	default:
 		return -EINVAL;
 	}
 
 	return 0;
 }
 
-/*
- * Write status Register and configuration register with 2 bytes
- * The first byte will be written to the status register, while the
- * second byte will be written to the configuration register.
- * Return negative if error occured.
- */
-static int write_sr_cr(struct spi_nor *nor, u16 val)
+static int macronix_set_dummy_cycles(struct spi_nor *nor, u8 read_dummy)
 {
-	nor->cmd_buf[0] = val & 0xff;
-	nor->cmd_buf[1] = (val >> 8);
+	int ret, sr, cr, mask, val;
+	u16 sr_cr;
+	u8 dc;
 
-	return nor->write_reg(nor, SPINOR_OP_WRSR, nor->cmd_buf, 2);
+	/* Convert the number of dummy cycles into Macronix DC volatile bits */
+	ret = macronix_dummy2code(nor->read_opcode, read_dummy, &dc);
+	if (ret)
+		return ret;
+
+	mask = GENMASK(7, 6);
+	val = (dc << 6) & mask;
+
+	cr = read_cr(nor);
+	if (cr < 0) {
+		dev_err(nor->dev, "error while reading the config register\n");
+		return cr;
+	}
+
+	if ((cr & mask) == val) {
+		nor->read_dummy = read_dummy;
+		return 0;
+	}
+
+	sr = read_sr(nor);
+	if (sr < 0) {
+		dev_err(nor->dev, "error while reading the status register\n");
+		return sr;
+	}
+
+	cr = (cr & ~mask) | val;
+	sr_cr = (sr & 0xff) | ((cr & 0xff) << 8);
+	write_enable(nor);
+	ret = write_sr_cr(nor, sr_cr);
+	if (ret) {
+		dev_err(nor->dev,
+			"error while writting the SR and CR registers\n");
+		return ret;
+	}
+
+	ret = spi_nor_wait_till_ready(nor);
+	if (ret)
+		return ret;
+
+	cr = read_cr(nor);
+	if (cr < 0 || (cr & mask) != val) {
+		dev_err(nor->dev, "Macronix Dummy Cycle bits not updated\n");
+		return -EINVAL;
+	}
+
+	/* Save the number of dummy cycles to use with Fast Read commands */
+	nor->read_dummy = read_dummy;
+	return 0;
+}
+
+static int macronix_set_quad_mode(struct spi_nor *nor)
+{
+	/* Check whether the QPI mode is enabled. */
+	if (nor->reg_proto == SPI_PROTO_4_4_4) {
+		/* 
+		 * In QPI mode, only the Fast Read Quad I/O (0xeb) command is
+		 * supported by the memory. Also the memory expects ALL commands
+		 * to use the SPI 4-4-4 protocol.
+		 * We already know that the SPI controller supports this
+		 * protocol as we succeeded in reading the JEDEC ID with the
+		 * 0xaf command and SPI-4-4-4 protocol.
+		 * However, using the 0xeb command we must take care about the
+		 * values sent during the dummy cycles as we don't want the
+		 * memory to enter its Continuous Read (Performance Enhance)
+		 * mode.
+		 */
+		nor->erase_proto = SPI_PROTO_4_4_4;
+		nor->write_proto = SPI_PROTO_4_4_4;
+		nor->read_proto = SPI_PROTO_4_4_4;
+		nor->read_opcode = SPINOR_OP_READ_1_4_4;
+		return macronix_set_dummy_cycles(nor, 8);
+	}
+
+	/*
+	 * Use the Fast Read Quad Output 1-1-4 (0x6b) command with 8 dummy
+	 * cycles (up to 133MHz for STR and 66MHz for DTR).
+	 */
+	nor->read_proto = SPI_PROTO_1_1_4;
+	nor->read_opcode = SPINOR_OP_READ_1_1_4;
+	return macronix_set_dummy_cycles(nor, 8);
+}
+
+static int macronix_set_dual_mode(struct spi_nor *nor)
+{
+	/*
+	 * Use the Fast Read Dual Output 1-1-2 (0x3b) command with 8 dummy
+	 * cycles (up to 133MHz for STR and 66MHz for DTR).
+	 */
+	nor->read_proto = SPI_PROTO_1_1_2;
+	nor->read_opcode = SPINOR_OP_READ_1_1_2;
+	return macronix_set_dummy_cycles(nor, 8);
+}
+
+static int macronix_set_single_mode(struct spi_nor *nor)
+{
+	u8 read_dummy;
+
+	/*
+	 * Configure 8 dummy cycles for Fast Read 1-1-1 (0x0b) command (up to
+	 * 133MHz for STR and 66MHz for DTR). The Read 1-1-1 (0x03) command
+	 * expects no dummy cycle.
+	 * read_opcode should not be overridden here!
+	 */
+	switch (nor->read_opcode) {
+	case SPINOR_OP_READ:
+	case SPINOR_OP_READ4:
+		read_dummy = 0;
+		break;
+
+	default:
+		read_dummy = 8;
+		break;
+	}
+
+	nor->read_proto = SPI_PROTO_1_1_1;
+	return macronix_set_dummy_cycles(nor, read_dummy);
+}
+
+static inline int spansion_get_config(struct spi_nor *nor,
+				      bool *quad_enabled,
+				      u8 *latency_code)
+{
+	int cr;
+
+	cr = read_cr(nor);
+	if (cr < 0) {
+		dev_err(nor->dev,
+			"error while reading the configuration register\n");
+		return cr;
+	}
+
+	if (quad_enabled)
+		*quad_enabled = !!(cr & CR_QUAD_EN_SPAN);
+
+	if (latency_code)
+		*latency_code = (u8)((cr & GENMASK(7, 6)) >> 6);
+
+	return 0;
 }
 
 static int spansion_quad_enable(struct spi_nor *nor)
@@ -1124,24 +1305,170 @@ static int spansion_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
-static int micron_quad_enable(struct spi_nor *nor)
+static int spansion_set_dummy_cycles(struct spi_nor *nor, u8 latency_code)
 {
+	/* SDR dummy cycles */
+	switch (nor->read_opcode) {
+	case SPINOR_OP_READ:
+	case SPINOR_OP_READ4:
+		nor->read_dummy = 0;
+		break;
+
+	case SPINOR_OP_READ_FAST:
+	case SPINOR_OP_READ_1_1_2:
+	case SPINOR_OP_READ_1_1_4:
+	case SPINOR_OP_READ4_FAST:
+	case SPINOR_OP_READ4_1_1_2:
+	case SPINOR_OP_READ4_1_1_4:
+		nor->read_dummy = (latency_code == 3) ? 0 : 8;
+		break;
+
+	case SPINOR_OP_READ_1_2_2:
+	case SPINOR_OP_READ4_1_2_2:
+		switch (latency_code) {
+		default:
+		case 0:
+		case 3:
+			nor->read_dummy = 4;
+			break;
+		case 1:
+			nor->read_dummy = 5;
+			break;
+		case 2:
+			nor->read_dummy = 6;
+			break;
+		}
+		break;
+
+
+	case SPINOR_OP_READ_1_4_4:
+	case SPINOR_OP_READ4_1_4_4:
+		switch (latency_code) {
+		default:
+		case 0:
+		case 1:
+			nor->read_dummy = 4;
+			break;
+		case 2:
+			nor->read_dummy = 5;
+			break;
+		case 3:
+			nor->read_dummy = 1;
+			break;
+		}
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int spansion_set_quad_mode(struct spi_nor *nor)
+{
+	bool quad_enabled;
+	u8 latency_code;
 	int ret;
-	u8 val;
 
-	ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+	/*
+	 * The QUAD bit of Configuration Register must be set (CR Bit1=1) for
+	 * using any Quad SPI command.
+	 */
+	ret = spansion_get_config(nor, &quad_enabled, &latency_code);
+	if (ret)
+		return ret;
+
+	/* The Quad mode should be enabled ... */
+	if (!quad_enabled) {
+		/* ... if not try to enable it. */
+		dev_warn(nor->dev, "Spansion Quad mode disabled, enable it\n");
+		ret = spansion_quad_enable(nor);
+		if (ret)
+			return ret;
+	}
+
+	/*
+	 * Don't use the Fast Read Quad I/O (0xeb / 0xec) commands as their
+	 * number of dummy cycles can not be set to a multiple of 8: some SPI
+	 * controllers, especially those relying on the m25p80 driver, expect
+	 * the number of dummy cycles to be a multiple of 8.
+	 * Also when using a Fast Read Quad I/O command, the memory checks the 
+	 * value of the first mode/dummy cycles to decice whether it enters or
+	 * leaves the Countinuous Read mode. We should never enter the
+	 * Countinuous Read mode as the spi-nor framework doesn't support it.
+	 * For all these reason, we'd rather use the Fast Read Quad Output
+	 * 1-1-4 (0x6b / 0x6c) commands instead.
+	 */
+	nor->read_proto = SPI_PROTO_1_1_4;
+	nor->read_opcode = SPINOR_OP_READ_1_1_4;
+	return spansion_set_dummy_cycles(nor, latency_code);
+}
+
+static int spansion_set_dual_mode(struct spi_nor *nor)
+{
+	u8 latency_code;
+	int ret;
+
+	/* We don't care about the quad mode status */
+	ret = spansion_get_config(nor, NULL, &latency_code);
+	if (ret)
+		return ret;
+
+	/*
+	 * Don't use the Fast Read Dual I/O (0xbb / 0xbc) commands as their
+	 * number of dummy cycles can not bet set to a multiple of 8: some SPI
+	 * controllers, especially those relying on the m25p80 driver, expect
+	 * the number of dummy cycles to be a multiple of 8.
+	 * For this reason, w'd rather use the Fast Read Dual Output 1-1-2
+	 * (0x3b / 0x3c) commands instead.
+	 */
+	nor->read_proto = SPI_PROTO_1_1_2;
+	nor->read_opcode = SPINOR_OP_READ_1_1_2;
+	return spansion_set_dummy_cycles(nor, latency_code);
+}
+
+static int spansion_set_single_mode(struct spi_nor *nor)
+{
+	u8 latency_code;
+	int ret;
+
+	/* We don't care about the quad mode status */
+	ret = spansion_get_config(nor, NULL, &latency_code);
+	if (ret)
+		return ret;
+
+	nor->read_proto = SPI_PROTO_1_1_1;
+	return spansion_set_dummy_cycles(nor, latency_code);
+}
+
+static int micron_set_dummy_cycles(struct spi_nor *nor, u8 read_dummy)
+{
+	u8 vcr, val, mask;
+	int ret;
+
+	/* Set bit3 (XIP) to disable the Continuous Read mode */
+	mask = GENMASK(7, 4) | BIT(3);
+	val = ((read_dummy << 4) | BIT(3)) & mask;
+
+	/* Read the Volatile Configuration Register (VCR). */
+	ret = nor->read_reg(nor, SPINOR_OP_RD_VCR, &vcr, 1);
 	if (ret < 0) {
-		dev_err(nor->dev, "error %d reading EVCR\n", ret);
+		dev_err(nor->dev, "error while reading VCR register\n");
 		return ret;
 	}
 
-	write_enable(nor);
+	/* Check whether we need to update the number of dummy cycles. */
+	if ((vcr & mask) == val) {
+		nor->read_dummy = read_dummy;
+		return 0;
+	}
 
-	/* set EVCR, enable quad I/O */
-	nor->cmd_buf[0] = val & ~EVCR_QUAD_EN_MICRON;
-	ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, nor->cmd_buf, 1);
+	/* Update the number of dummy into the VCR. */
+	write_enable(nor);
+	vcr = (vcr & ~mask) | val;
+	ret = nor->write_reg(nor, SPINOR_OP_WR_VCR, &vcr, 1);
 	if (ret < 0) {
-		dev_err(nor->dev, "error while writing EVCR register\n");
+		dev_err(nor->dev, "error while writing VCR register\n");
 		return ret;
 	}
 
@@ -1149,47 +1476,309 @@ static int micron_quad_enable(struct spi_nor *nor)
 	if (ret)
 		return ret;
 
-	/* read EVCR and check it */
-	ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+	/* Read VCR and check it. */
+	ret = nor->read_reg(nor, SPINOR_OP_RD_VCR, &vcr, 1);
+	if (ret < 0 || (vcr & mask) != val) {
+		dev_err(nor->dev, "Micron VCR dummy cycles not updated\n");
+		return -EINVAL;
+	}
+
+	/* Save the number of dummy cycles to use with Fast Read commands */
+	nor->read_dummy = read_dummy;
+	return 0;
+}
+
+static int micron_set_protocol(struct spi_nor *nor, u8 mask, u8 val,
+			       enum spi_protocol proto)
+{
+	u8 evcr;
+	int ret;
+
+	/* Read the Exhanced 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 %d reading EVCR\n", ret);
+		dev_err(nor->dev, "error while writing EVCR register\n");
 		return ret;
 	}
-	if (val & EVCR_QUAD_EN_MICRON) {
-		dev_err(nor->dev, "Micron EVCR Quad bit not clear\n");
+
+	/* 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_protocol(struct spi_nor *nor)
+{
+	int ret;
+
+	/* Set Quad bit to 0 to select the Quad SPI mode. */
+	ret = micron_set_protocol(nor,
+				  EVCR_QUAD_EN_MICRON,
+				  0,
+				  SPI_PROTO_4_4_4);
+	if (ret) {
+		dev_err(nor->dev, "Failed to set Micron Quad SPI mode\n");
+		return ret;
+	}
+
+	nor->read_proto = SPI_PROTO_4_4_4;
+	nor->write_proto = SPI_PROTO_4_4_4;
+	nor->erase_proto = SPI_PROTO_4_4_4;
+	return 0;
+}
+
+static int micron_set_dual_protocol(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,
+				  SPI_PROTO_2_2_2);
+	if (ret) {
+		dev_err(nor->dev, "Failed to set Micron Dual SPI mode\n");
+		return ret;
+	}
+
+	nor->read_proto = SPI_PROTO_2_2_2;
+	nor->write_proto = SPI_PROTO_2_2_2;
+	nor->erase_proto = SPI_PROTO_2_2_2;
+	return 0;
+}
+
+static int micron_set_extended_spi_protocol(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,
+				  SPI_PROTO_1_1_1);
+	if (ret) {
+		dev_err(nor->dev, "Failed to set Micron Extended SPI mode\n");
+		return ret;
+	}
+
+	nor->write_proto = SPI_PROTO_1_1_1;
+	nor->erase_proto = SPI_PROTO_1_1_1;
+	return 0;
+}
+
+static int micron_set_quad_mode(struct spi_nor *nor)
+{
+	int ret;
+
+	/* Check whether the Quad SPI mode is enabled. */
+	if (nor->reg_proto == SPI_PROTO_4_4_4) {
+		/*
+		 * If here, the Quad mode should have already been enabled and
+		 * is supported by the SPI controller since the memory replied
+		 * to the Read ID Multiple I/O (0xaf) command in SPI 4-4-4
+		 * protocol. So it might be enough to only set the read, write
+		 * and erase protocols to SPI 4-4-4 but just in case...
+		 */
+		ret = micron_set_quad_protocol(nor);
+		if (ret)
+			return ret;
+
+		/*
+		 * In Quad mode, the memory doesn't make any difference between
+		 * the Fast Read Quad Output 1-1-4 (0x6b) and Fast Read Quad I/O
+		 * 1-4-4 (0xeb) commands: they are both processed in SPI 4-4-4
+		 * protocol. The 1-4-4 command is chosen here only for debug
+		 * purpose to easily detect the chosen mode when logging
+		 * commands.
+		 */
+		nor->read_opcode = SPINOR_OP_READ_1_4_4;
+		return micron_set_dummy_cycles(nor, 8);
+	}
+
+	/*
+	 * Exit Dual or Quad mode if not done yet: the Fast Read Quad Output
+	 * 1-1-4 (0x6b) command is also supported by the Extended SPI Protocol.
+	 * We can change the mode safely as we write into a volatile register.
+	 */
+	ret = micron_set_extended_spi_protocol(nor);
+	if (ret)
+		return ret;
+
+	/*
+	 * Use the Fast Read Quad Output 1-1-4 command.
+	 * Force the number of dummy cycles to 8 and disable the Continuous Read
+	 * mode to prevent some drivers from using it by mistake (m25p80).
+	 * We can change these settings safely as we write into a volatile
+	 * register.
+	 */
+	nor->read_proto = SPI_PROTO_1_1_4;
+	nor->read_opcode = SPINOR_OP_READ_1_1_4;
+	return micron_set_dummy_cycles(nor, 8);
+}
+
+static int micron_set_dual_mode(struct spi_nor *nor)
+{
+	int ret;
+
+	/* Check whether the Dual SPI mode is enabled. */
+	if (nor->reg_proto == SPI_PROTO_2_2_2) {
+		/*
+		 * If here, the Dual mode should have already been enabled and
+		 * is supported by the SPI controller since the memory replied
+		 * to the Read ID Multiple I/O (0xaf) command in SPI 2-2-2
+		 * protocol. So it might be enough to only set the read, write
+		 * and erase protocols to SPI 2-2-2 but just in case...
+		 */
+		ret = micron_set_dual_protocol(nor);
+		if (ret)
+			return ret;
+
+		/*
+		 * In Dual mode, the memory doesn't make any difference between
+		 * the Fast Read Dual Output 1-1-2 (0x3b) and Fast Read Dual I/O
+		 * 1-2-2 (0xbb) commands: they are both processed in SPI 2-2-2
+		 * protocol. The 1-2-2 command is chosen here only for debug
+		 * purpose to easily detect the chosen mode when logging
+		 * commands.
+		 */
+		nor->read_opcode = SPINOR_OP_READ_1_2_2;
+		return micron_set_dummy_cycles(nor, 8);
+	}
+
+	/*
+	 * Exit Dual or Quad mode if not done yet: the Fast Read Dual Output
+	 * 1-1-2 (0x3b) command is also supported by the Extended SPI Protocol.
+	 * We can change the mode safely as we write into a volatile register.
+	 */
+	ret = micron_set_extended_spi_protocol(nor);
+	if (ret)
+		return ret;
+
+	/*
+	 * Use the Fast Read Dual Output 1-1-2 command.
+	 * Force the number of dummy cycles to 8 and disable the Continuous Read
+	 * mode to prevent some drivers from using it by mistake (m25p80).
+	 * We can change these settings safely as we write into a volatile
+	 * register.
+	 */
+	nor->read_proto = SPI_PROTO_1_1_2;
+	nor->read_opcode = SPINOR_OP_READ_1_1_2;
+	return micron_set_dummy_cycles(nor, 8);
+}
+
+static int micron_set_single_mode(struct spi_nor *nor)
+{
+	u8 read_dummy;
+	int ret;
+
+	/*
+	 * Exit Dual or Quad mode if not done yet.
+	 * We can change the mode safely as we write into a volatile register.
+	 */
+	ret = micron_set_extended_spi_protocol(nor);
+	if (ret)
+		return ret;
+
+	/*
+	 * Force the number of dummy cycles to 8 for Fast Read, 0 for Read,
+	 * and disable the Continuous Read mode to prevent some drivers
+	 * from using it by mistake (m25p80).
+	 * We can change these settings safely as we write into a volatile
+	 * register.
+	 */
+	switch (nor->read_opcode) {
+	case SPINOR_OP_READ:
+	case SPINOR_OP_READ4:
+		read_dummy = 0;
+		break;
+
+	default:
+		read_dummy = 8;
+		break;
+	}
+	nor->read_proto = SPI_PROTO_1_1_1;
+	return micron_set_dummy_cycles(nor, read_dummy);
+}
+
 static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
 {
-	int status;
+	switch (JEDEC_MFR(info)) {
+	case SNOR_MFR_MACRONIX:
+		return macronix_set_quad_mode(nor);
 
+	case SNOR_MFR_MICRON:
+		return micron_set_quad_mode(nor);
+
+	case SNOR_MFR_SPANSION:
+		return spansion_set_quad_mode(nor);
+
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int set_dual_mode(struct spi_nor *nor, const struct flash_info *info)
+{
 	switch (JEDEC_MFR(info)) {
 	case SNOR_MFR_MACRONIX:
-		status = macronix_quad_enable(nor);
-		if (status) {
-			dev_err(nor->dev, "Macronix quad-read not enabled\n");
-			return -EINVAL;
-		}
-		return status;
+		return macronix_set_dual_mode(nor);
+
 	case SNOR_MFR_MICRON:
-		status = micron_quad_enable(nor);
-		if (status) {
-			dev_err(nor->dev, "Micron quad-read not enabled\n");
-			return -EINVAL;
-		}
-		return status;
+		return micron_set_dual_mode(nor);
+
+	case SNOR_MFR_SPANSION:
+		return spansion_set_dual_mode(nor);
+
 	default:
-		status = spansion_quad_enable(nor);
-		if (status) {
-			dev_err(nor->dev, "Spansion quad-read not enabled\n");
-			return -EINVAL;
-		}
-		return status;
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int set_single_mode(struct spi_nor *nor, const struct flash_info *info)
+{
+	switch (JEDEC_MFR(info)) {
+	case SNOR_MFR_MACRONIX:
+		return macronix_set_single_mode(nor);
+
+	case SNOR_MFR_MICRON:
+		return micron_set_single_mode(nor);
+
+	case SNOR_MFR_SPANSION:
+		return spansion_set_single_mode(nor);
+
+	default:
+		break;
 	}
+
+	return -EINVAL;
 }
 
 static int spi_nor_check(struct spi_nor *nor)
@@ -1339,7 +1928,22 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	if (info->flags & SPI_NOR_NO_FR)
 		nor->flash_read = SPI_NOR_NORMAL;
 
-	/* Quad/Dual-read mode takes precedence over fast/normal */
+	/* Default commands and number of dummy cycles */
+	nor->program_opcode = SPINOR_OP_PP;
+	if (nor->flash_read == SPI_NOR_NORMAL) {
+		nor->read_opcode = SPINOR_OP_READ;
+		nor->read_dummy = 0;
+	} else {
+		nor->read_opcode = SPINOR_OP_READ_FAST;
+		nor->read_dummy = 8;
+	}
+
+	/*
+	 * Quad/Dual-read mode takes precedence over fast/normal. The opcodes,
+	 * the protocols and the number of dummy cycles are updated depending
+	 * on the manufacturer. The read opcode and protocol should be updated
+	 * by the relevant function when entering Quad or Dual mode.
+	 */
 	if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
 		ret = set_quad_mode(nor, info);
 		if (ret) {
@@ -1348,30 +1952,21 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 		}
 		nor->flash_read = SPI_NOR_QUAD;
 	} else if (mode == SPI_NOR_DUAL && info->flags & SPI_NOR_DUAL_READ) {
+		ret = set_dual_mode(nor, info);
+		if (ret) {
+			dev_err(dev, "dual mode not supported\n");
+			return ret;
+		}
 		nor->flash_read = SPI_NOR_DUAL;
+	} else if (info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)) {
+		/* We may need to leave a Quad or Dual mode */
+		ret = set_single_mode(nor, info);
+		if (ret) {
+			dev_err(dev, "failed to switch back to single mode\n");
+			return ret;
+		}
 	}
 
-	/* 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) {
@@ -1409,8 +2004,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 c91986a99caf..9f54b01675fd 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -42,8 +42,10 @@
 #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_BE_4K		0x20	/* Erase 4KiB block */
 #define SPINOR_OP_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips */
@@ -57,8 +59,10 @@
 /* 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_READ4_1_1_2	0x3c	/* Read data bytes (Dual Output SPI) */
+#define SPINOR_OP_READ4_1_2_2	0xbc	/* Read data bytes (Dual I/O SPI) */
+#define SPINOR_OP_READ4_1_1_4	0x6c	/* Read data bytes (Quad Output SPI) */
+#define SPINOR_OP_READ4_1_4_4	0xec	/* Read data bytes (Quad I/O SPI) */
 #define SPINOR_OP_PP_4B		0x12	/* Page program (up to 256 bytes) */
 #define SPINOR_OP_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
 
@@ -76,6 +80,8 @@
 
 /* Used for Micron flashes only. */
 #define SPINOR_OP_MIO_RDID	0xaf	/* Multiple I/O Read JEDEC ID */
+#define SPINOR_OP_RD_VCR	0x85	/* Read VCR register */
+#define SPINOR_OP_WR_VCR	0x81	/* Write VCR register */
 #define SPINOR_OP_RD_EVCR	0x65    /* Read EVCR register */
 #define SPINOR_OP_WD_EVCR	0x61    /* Write EVCR register */
 
@@ -92,6 +98,7 @@
 
 /* Enhanced Volatile Configuration Register bits */
 #define EVCR_QUAD_EN_MICRON	BIT(7)	/* Micron Quad I/O */
+#define EVCR_DUAL_EN_MICRON	BIT(6)	/* Micron Dual I/O */
 
 /* Flag Status Register bits */
 #define FSR_READY		BIT(7)
-- 
1.8.2.2

[PATCH linux-next 3/5] mtd: m25p80: add support of dual and quad spi protocols to all commands

[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 | 233 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 193 insertions(+), 40 deletions(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index c9c3b7fa3051..8b09f77eeffb 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -27,22 +27,114 @@
 #include <linux/spi/flash.h>
 #include <linux/mtd/spi-nor.h>
 
-#define	MAX_CMD_SIZE		6
+#define	MAX_CMD_SIZE		8
 struct m25p {
 	struct spi_device	*spi;
 	struct spi_nor		spi_nor;
 	u8			command[MAX_CMD_SIZE];
 };
 
+static inline int m25p80_proto2nbits(enum spi_protocol proto,
+				     unsigned *code_nbits,
+				     unsigned *addr_nbits,
+				     unsigned *data_nbits)
+{
+	unsigned code, addr, data;
+
+	switch (proto) {
+	case SPI_PROTO_1_1_1:
+		code = SPI_NBITS_SINGLE;
+		addr = SPI_NBITS_SINGLE;
+		data = SPI_NBITS_SINGLE;
+		break;
+
+	case SPI_PROTO_1_1_2:
+		code = SPI_NBITS_SINGLE;
+		addr = SPI_NBITS_SINGLE;
+		data = SPI_NBITS_DUAL;
+		break;
+
+	case SPI_PROTO_1_1_4:
+		code = SPI_NBITS_SINGLE;
+		addr = SPI_NBITS_SINGLE;
+		data = SPI_NBITS_QUAD;
+		break;
+
+	case SPI_PROTO_1_2_2:
+		code = SPI_NBITS_SINGLE;
+		addr = SPI_NBITS_DUAL;
+		data = SPI_NBITS_DUAL;
+		break;
+
+	case SPI_PROTO_1_4_4:
+		code = SPI_NBITS_SINGLE;
+		addr = SPI_NBITS_QUAD;
+		data = SPI_NBITS_QUAD;
+		break;
+
+	case SPI_PROTO_2_2_2:
+		code = SPI_NBITS_DUAL;
+		addr = SPI_NBITS_DUAL;
+		data = SPI_NBITS_DUAL;
+		break;
+
+	case SPI_PROTO_4_4_4:
+		code = SPI_NBITS_QUAD;
+		addr = SPI_NBITS_QUAD;
+		data = SPI_NBITS_QUAD;
+		break;
+
+	default:
+		return -EINVAL;
+
+	}
+
+	if (code_nbits)
+		*code_nbits = code;
+	if (addr_nbits)
+		*addr_nbits = addr;
+	if (data_nbits)
+		*data_nbits = data;
+
+	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 +157,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 +200,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));
 
-	t[1].tx_buf = buf;
-	t[1].len = len;
-	spi_message_add_tail(&t[1], &m);
+	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++;
+		}
+	}
+
+	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 (nor->flash_read) {
-	case SPI_NOR_DUAL:
-		return 2;
-	case SPI_NOR_QUAD:
-		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,28 +257,48 @@ 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;
 
-	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);
+	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

[PATCH linux-next 4/5] Documentation: atmel-quadspi: add binding file for Atmel QSPI driver

[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>
---
 .../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..e81f20f9faf1
--- /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 = "qpsi_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

[PATCH linux-next 5/5] mtd: atmel-quadspi: add driver for Atmel QSPI controller

[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>
---
 drivers/mtd/spi-nor/Kconfig         |   8 +
 drivers/mtd/spi-nor/Makefile        |   3 +-
 drivers/mtd/spi-nor/atmel-quadspi.c | 877 ++++++++++++++++++++++++++++++++++++
 3 files changed, 887 insertions(+), 1 deletion(-)
 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 0dc927540b3d..15f45dbbfe0d 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -37,6 +37,14 @@ 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 OF && HAS_DMA && (ARCH_AT91 || COMPILE_TEST)
+	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..0cbed0e4ae8c 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,4 +1,5 @@
 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_MTD_MT81xx_NOR)	+= mtk-quadspi.o
+obj-$(CONFIG_SPI_ATMEL_QUADSPI)	+= atmel-quadspi.o
 obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.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..bdebfdc92842
--- /dev/null
+++ b/drivers/mtd/spi-nor/atmel-quadspi.c
@@ -0,0 +1,877 @@
+/*
+ * 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/dma-mapping.h>
+#include <linux/dmaengine.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/platform_data/dma-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)
+
+/* 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;
+	dma_addr_t		phys_addr;
+	struct dma_chan		*chan;
+	struct clk		*clk;
+	struct platform_device	*pdev;
+	u32			pending;
+
+	struct spi_nor		nor;
+	u32			clk_rate;
+	struct completion	cmd_completion;
+	struct completion	dma_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	dma:1;
+			u32	reserved:24;
+		}		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);
+}
+
+
+#define QSPI_DMA_THRESHOLD	32
+
+static void atmel_qspi_dma_callback(void *arg)
+{
+	struct completion *dma_completion = arg;
+
+	complete(dma_completion);
+}
+
+static int atmel_qspi_run_dma_transfer(struct atmel_qspi *aq,
+				       const struct atmel_qspi_command *cmd)
+{
+	u32 offset = (cmd->enable.bits.address) ? cmd->address : 0;
+	struct dma_chan *chan = aq->chan;
+	struct device *dev = &aq->pdev->dev;
+	enum dma_data_direction direction;
+	dma_addr_t phys_addr, dst, src;
+	struct dma_async_tx_descriptor *desc;
+	dma_cookie_t cookie;
+	int err = 0;
+
+	if (cmd->tx_buf) {
+		direction = DMA_TO_DEVICE;
+		phys_addr = dma_map_single(dev, (void *)cmd->tx_buf,
+					   cmd->buf_len, direction);
+		src = phys_addr;
+		dst = aq->phys_addr + offset;
+	} else {
+		direction = DMA_FROM_DEVICE;
+		phys_addr = dma_map_single(dev, (void *)cmd->rx_buf,
+					   cmd->buf_len, direction);
+		src = aq->phys_addr + offset;
+		dst = phys_addr;
+	}
+	if (dma_mapping_error(dev, phys_addr))
+		return -ENOMEM;
+
+	desc = chan->device->device_prep_dma_memcpy(chan, dst, src,
+						    cmd->buf_len,
+						    DMA_PREP_INTERRUPT);
+	if (!desc) {
+		err = -ENOMEM;
+		goto unmap_single;
+	}
+
+	reinit_completion(&aq->dma_completion);
+	desc->callback = atmel_qspi_dma_callback;
+	desc->callback_param = &aq->dma_completion;
+	cookie = dmaengine_submit(desc);
+	err = dma_submit_error(cookie);
+	if (err)
+		goto unmap_single;
+	dma_async_issue_pending(chan);
+
+	if (!wait_for_completion_timeout(&aq->dma_completion,
+					 msecs_to_jiffies(1000)))
+		err = -ETIMEDOUT;
+
+	if (dma_async_is_tx_complete(chan, cookie, NULL, NULL) != DMA_COMPLETE)
+		err = -ETIMEDOUT;
+
+	if (err)
+		dmaengine_terminate_all(chan);
+unmap_single:
+	dma_unmap_single(dev, phys_addr, cmd->buf_len, direction);
+
+	return err;
+}
+
+static int atmel_qspi_run_transfer(struct atmel_qspi *aq,
+				   const struct atmel_qspi_command *cmd)
+{
+	void __iomem *ahb_mem;
+
+	/* First try a DMA transfer */
+	if (aq->chan && cmd->enable.bits.dma &&
+	    cmd->buf_len >= QSPI_DMA_THRESHOLD)
+		return atmel_qspi_run_dma_transfer(aq, cmd);
+
+	/* 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;
+	}
+
+	/* 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_INSTRE)
+		return err;
+
+	/* Wait for INSTRuction End interrupt */
+	reinit_completion(&aq->cmd_completion);
+	aq->pending = 0;
+	qspi_writel(aq, QSPI_IER, QSPI_SR_INSTRE);
+	if (!wait_for_completion_timeout(&aq->cmd_completion,
+					 msecs_to_jiffies(1000)))
+		err = -ETIMEDOUT;
+	qspi_writel(aq, QSPI_IDR, QSPI_SR_INSTRE);
+
+	return err;
+}
+
+static int atmel_qspi_command_set_ifr(struct atmel_qspi_command *cmd,
+				      u32 ifr_tfrtyp,
+				      enum spi_protocol proto)
+{
+	cmd->ifr = ifr_tfrtyp;
+
+	switch (proto) {
+	case SPI_PROTO_1_1_1:
+		cmd->ifr |= QSPI_IFR_WIDTH_SINGLE_BIT_SPI;
+		break;
+	case SPI_PROTO_1_1_2:
+		cmd->ifr |= QSPI_IFR_WIDTH_DUAL_OUTPUT;
+		break;
+	case SPI_PROTO_1_1_4:
+		cmd->ifr |= QSPI_IFR_WIDTH_QUAD_OUTPUT;
+		break;
+	case SPI_PROTO_1_2_2:
+		cmd->ifr |= QSPI_IFR_WIDTH_DUAL_IO;
+		break;
+	case SPI_PROTO_1_4_4:
+		cmd->ifr |= QSPI_IFR_WIDTH_QUAD_IO;
+		break;
+	case SPI_PROTO_2_2_2:
+		cmd->ifr |= QSPI_IFR_WIDTH_DUAL_CMD;
+		break;
+	case SPI_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.enable.bits.dma = 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.enable.bits.dma = 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_SSM | QSPI_MR_NBBITS(8);
+	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 (pending & QSPI_SR_INSTRE)
+		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 atmel_qspi *aq;
+	struct resource *res;
+	dma_cap_mask_t mask;
+	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);
+	init_completion(&aq->dma_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->regs);
+		goto exit;
+	}
+	aq->phys_addr = (dma_addr_t)res->start;
+
+	/* 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;
+
+	/* Try to get a DMA channel for memcpy() operation */
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_MEMCPY, mask);
+	aq->chan = dma_request_channel(mask, NULL, NULL);
+	if (!aq->chan)
+		dev_warn(&pdev->dev, "no available DMA channel\n");
+
+	/* 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_property_read_u32(child, "spi-max-frequency", &aq->clk_rate);
+	if (err < 0)
+		goto release_channel;
+
+	err = atmel_qspi_init(aq);
+	if (err)
+		goto release_channel;
+
+	err = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
+	if (err)
+		goto release_channel;
+
+	err = mtd_device_register(mtd, NULL, 0);
+	if (err)
+		goto release_channel;
+
+	of_node_put(child);
+
+	return 0;
+
+release_channel:
+	if (aq->chan)
+		dma_release_channel(aq->chan);
+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);
+	if (aq->chan)
+		dma_release_channel(aq->chan);
+	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

Re: [PATCH linux-next 5/5] mtd: atmel-quadspi: add driver for Atmel QSPI controller

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1770 bytes --]

Hi Cyrille,

[auto build test ERROR on next-20151207]

url:    https://github.com/0day-ci/linux/commits/Cyrille-Pitchen/mtd-spi-nor-add-driver-for-Atmel-QSPI-controller/20151207-221330
config: i386-allmodconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/mtd/spi-nor/atmel-quadspi.c: In function 'atmel_qspi_run_transfer':
>> drivers/mtd/spi-nor/atmel-quadspi.c:296:3: error: implicit declaration of function '_memcpy_toio' [-Werror=implicit-function-declaration]
      _memcpy_toio(ahb_mem, cmd->tx_buf, cmd->buf_len);
      ^
>> drivers/mtd/spi-nor/atmel-quadspi.c:298:3: error: implicit declaration of function '_memcpy_fromio' [-Werror=implicit-function-declaration]
      _memcpy_fromio(cmd->rx_buf, ahb_mem, cmd->buf_len);
      ^
   cc1: some warnings being treated as errors

coccinelle warnings: (new ones prefixed by >>)

>> drivers/mtd/spi-nor/atmel-quadspi.c:682:6-17: Move constant to right.
--
>> drivers/mtd/spi-nor/atmel-quadspi.c:758:5-11: inconsistent IS_ERR and PTR_ERR on line 760.

Please review and possibly fold the followup patch.

vim +/_memcpy_toio +296 drivers/mtd/spi-nor/atmel-quadspi.c

   290	
   291		/* Then fallback to a PIO transfer (memcpy() DOES NOT work!) */
   292		ahb_mem = aq->mem;
   293		if (cmd->enable.bits.address)
   294			ahb_mem += cmd->address;
   295		if (cmd->tx_buf)
 > 296			_memcpy_toio(ahb_mem, cmd->tx_buf, cmd->buf_len);
   297		else
 > 298			_memcpy_fromio(cmd->rx_buf, ahb_mem, cmd->buf_len);
   299	
   300		return 0;
   301	}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 53054 bytes --]

[PATCH] mtd: atmel-quadspi: fix odd_ptr_err.cocci warnings

[kbuild test robot]

drivers/mtd/spi-nor/atmel-quadspi.c:758:5-11: inconsistent IS_ERR and PTR_ERR on line 760.

 PTR_ERR should access the value just tested by IS_ERR

Semantic patch information:
 There can be false positives in the patch case, where it is the call to
 IS_ERR that is wrong.

Generated by: scripts/coccinelle/tests/odd_ptr_err.cocci

CC: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

Please take the patch only if it's a positive warning. Thanks!

 atmel-quadspi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/mtd/spi-nor/atmel-quadspi.c
+++ b/drivers/mtd/spi-nor/atmel-quadspi.c
@@ -757,7 +757,7 @@ static int atmel_qspi_probe(struct platf
 	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->regs);
+		err = PTR_ERR(aq->mem);
 		goto exit;
 	}
 	aq->phys_addr = (dma_addr_t)res->start;

[PATCH] mtd: atmel-quadspi: fix compare_const_fl.cocci warnings

[kbuild test robot]

drivers/mtd/spi-nor/atmel-quadspi.c:682:6-17: Move constant to right.

 Move constants to the right of binary operators.

Semantic patch information:
 Depends on personal taste in some cases.

Generated by: scripts/coccinelle/misc/compare_const_fl.cocci

CC: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

Please take the patch only if it's a positive warning. Thanks!

 atmel-quadspi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/mtd/spi-nor/atmel-quadspi.c
+++ b/drivers/mtd/spi-nor/atmel-quadspi.c
@@ -679,7 +679,7 @@ static int atmel_qspi_init(struct atmel_
 	qspi_writel(aq, QSPI_CR, QSPI_CR_SWRST);
 
 	/* Set the QSPI controller in Serial Memory Mode */
-	mr = QSPI_MR_SSM | QSPI_MR_NBBITS(8);
+	mr = QSPI_MR_NBBITS(8) | QSPI_MR_SSM;
 	qspi_writel(aq, QSPI_MR, mr);
 
 	src_rate = clk_get_rate(aq->clk);

Re: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller

[Brian Norris]

+ Bean Huo

Hi Cyrille,

On Mon, Dec 07, 2015 at 03:09:09PM +0100, Cyrille Pitchen wrote:
> Hi all,
> 
> this series of patches adds support to the Atmel QSPI controller available
> on sama5d2 SoCs. It was tested on a sama5d2 xplained ultra board with a 
> Micron n25q128a13 QSPI memory and a at25df321a SPI memory.
> 
> In order to use the Micron memory in its Quad SPI mode, the spi-nor
> framework needed to be patched to fix the support of Quad/Dual SPI
> protocols with some memory manufacturers such as Spansion, Micron and
> Macronix. There are many comments in the source code to explain the
> implementation choices based on the datasheets from memory manufacturers.
> 
> 
> This series was based and tested on linux-next-20151207
> 
> 1 - Atmel QSPI + Micron n25q128a13 (atmel-quadspi.c driver)
> 
> SPI 1-1-1: This mode was tested replacing SPI_NOR_QUAD by SPI_NOR_FAST as
>            argument to spi_nor_scan() called from atmel_qspi_probe().
> 
> SPI 1-1-4: Bootloaders (at91bootstrap/uboot) don't enable the Quad SPI
>            mode of the Micron memory. When probed from Linux, the memory
>            uses its Extended SPI mode and replies to the regular Read ID
>            (0x9f) command.
> 
> SPI 4-4-4: The romcode enabled the Quad SPI mode the of Micron memory
>            before loading the at91bootstrap. When probed from Linux, the
>            memory uses its Quad SPI mode and no longer replies to the
>            regular Read ID (0x9f) command but instead to the Read ID
>            Multiple I/O (0xaf) command. The memory expects ALL commands
>            to use the SPI 4-4-4 protocol.

I'll admit I'm a little fuzzy on the differences between dual and quad
modes on various flash manufacturers. Can you help clear it up for me?
I think some of the comments on patch 2 help too, but I'll just comment
here for now.

It looks like the current driver has problems regarding the non 1-x-y
modes (e.g., 4-4-4), right? But I see that spi-nor.c never tries to
send a 4_4_4 command; it only sets read_opcode to
SPINOR_OP_READ_1_1_{1,2,4}. So is this an oversight in patches like
Bean's patch?

    commit 548cd3ab54da ("mtd: spi-nor: Add quad I/O support for Micron
    SPI NOR")

Why would we even need to enable quad modes like that, if we're not
going to send the 4-4-4 opcodes?

My next question (if my understanding is roughly correct) is, do we need
the 4-4-4 modes, and what risks come with them? I understand we can
shorten the command and address phases, but does that alone yield much
performance benefit? And I think the risk is that a given system might
not be prepared for the flash to be in a 4-4-4 mode, if the boot code
tries to use 1-x-y commands.

Also, I see a lot of good comments in patch 2 about Spansion vs.
Macronix vs. Micron memories. I wonder if previous developers have
completely tested their patches, or if they're just reading the
datasheets... so, what kind have testing have you done? Do you have
samples of all these flash to test?

Regards,
Brian

RE: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller

[Bean Huo 霍斌斌 (beanhuo)]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 5588 bytes --]

> -----Original Message-----
> From: Brian Norris [mailto:computersforpeace@gmail.com]
> Sent: Tuesday, December 08, 2015 3:35 AM
> To: Cyrille Pitchen
> Cc: linux-mtd@lists.infradead.org; nicolas.ferre@atmel.com; marex@denx.de;
> vigneshr@ti.com; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
> robh+dt@kernel.org; pawel.moll@arm.com; mark.rutland@arm.com;
> ijc+devicetree@hellion.org.uk; galak@codeaurora.org; Bean Huo »ô±ó±ó
> (beanhuo)
> Subject: Re: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI
> controller
> 
> + Bean Huo
> 
> Hi Cyrille,
> 
> On Mon, Dec 07, 2015 at 03:09:09PM +0100, Cyrille Pitchen wrote:
> > Hi all,
> >
> > this series of patches adds support to the Atmel QSPI controller
> > available on sama5d2 SoCs. It was tested on a sama5d2 xplained ultra
> > board with a Micron n25q128a13 QSPI memory and a at25df321a SPI
> memory.
> >
> > In order to use the Micron memory in its Quad SPI mode, the spi-nor
> > framework needed to be patched to fix the support of Quad/Dual SPI
> > protocols with some memory manufacturers such as Spansion, Micron and
> > Macronix. There are many comments in the source code to explain the
> > implementation choices based on the datasheets from memory
> manufacturers.
> >
> >
> > This series was based and tested on linux-next-20151207
> >
> > 1 - Atmel QSPI + Micron n25q128a13 (atmel-quadspi.c driver)
> >
> > SPI 1-1-1: This mode was tested replacing SPI_NOR_QUAD by
> SPI_NOR_FAST as
> >            argument to spi_nor_scan() called from atmel_qspi_probe().
> >
> > SPI 1-1-4: Bootloaders (at91bootstrap/uboot) don't enable the Quad SPI
> >            mode of the Micron memory. When probed from Linux, the
> memory
> >            uses its Extended SPI mode and replies to the regular Read ID
> >            (0x9f) command.
> >
> > SPI 4-4-4: The romcode enabled the Quad SPI mode the of Micron memory
> >            before loading the at91bootstrap. When probed from Linux,
> the
> >            memory uses its Quad SPI mode and no longer replies to the
> >            regular Read ID (0x9f) command but instead to the Read ID
> >            Multiple I/O (0xaf) command. The memory expects ALL
> commands
> >            to use the SPI 4-4-4 protocol.
> 
> I'll admit I'm a little fuzzy on the differences between dual and quad modes on
> various flash manufacturers. Can you help clear it up for me?

For Micron SPI NOR spi quad mode, means that Qaud I/O prototocol, it follows I/O
Bus width is command-address-Data 4-4-4, at this time, DQ0,DQ1,DQ2,DQ3
are all used to transfer address/command/data. For this maybe not the same between
different flash manufactures. For example, for Spansion Qspi NOR, its all instructions are
transferred from host to memory as a single bit serial sequence on the DQ0 signal, even under
Quad mode.  Dual mode the same as Qaud mode scenario.

for SPI NOR 1-1-4, means command and address are transferred on the DQ0,
but for data, being transferred on DQ0,DQ1,DQ2,DQ3.For this, it is the same
between different flash manufacturers. Of course, at this moment, SPI NOR
should work under extended I/O mode.

> I think some of the comments on patch 2 help too, but I'll just comment here
> for now.
> It looks like the current driver has problems regarding the non 1-x-y modes
> (e.g., 4-4-4), right? But I see that spi-nor.c never tries to send a 4_4_4
> command; it only sets read_opcode to SPINOR_OP_READ_1_1_{1,2,4}. So is
> this an oversight in patches like Bean's patch?

For SPINOR_OP_READ_1_1_{1,2,4} commands, Spi NOR actually works under
Extended I/O mode, not Quad mode. They just push Spi NOR output data by Quad mode,
Command and address still following extended I/O mode.
For 4-4-4 I/O protocol, SPI NOR should change to Quad mode(just as my patch), 
of course, SPI controller should support this. for Micron Qspi NOR, under quad mode,
all commands/address/data are transferred on DQ0,DQ1,DQ2,DQ3 signals. No matter what
kind of command. 


>     commit 548cd3ab54da ("mtd: spi-nor: Add quad I/O support for Micron
>     SPI NOR")
> 
> Why would we even need to enable quad modes like that, if we're not going
> to send the 4-4-4 opcodes?
I think, in order to high speed SPI NOR, after enable quad mode, 
SPINOR_OP_READ_1_1_{1,2,4} commands don't need any more, normal read command (0x03)
Can implement as them.

> My next question (if my understanding is roughly correct) is, do we need the
> 4-4-4 modes, and what risks come with them? I understand we can shorten
> the command and address phases, but does that alone yield much
> performance benefit? And I think the risk is that a given system might not be
> prepared for the flash to be in a 4-4-4 mode, if the boot code tries to use 1-x-y
> commands.

As far as my current experience and knowledge, this still need to be enabled, especially for
fast boot, and some IOT devices to store info into SPI NOR.
For this patches, my current concern is that host side how to get different I/O protocol changes,
and distinguish between different flash manufacturers I/O mode.

> Also, I see a lot of good comments in patch 2 about Spansion vs.
> Macronix vs. Micron memories. I wonder if previous developers have
> completely tested their patches, or if they're just reading the datasheets... so,
> what kind have testing have you done? Do you have samples of all these flash
> to test?
> Regards,
> Brian
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller

[Bean Huo 霍斌斌 (beanhuo)]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 478 bytes --]

> 
> I'll admit I'm a little fuzzy on the differences between dual and quad modes on
> various flash manufacturers. Can you help clear it up for me?

For SPI NOR, currently, don't have an official standard to define an uniform Quad
I/O mode protocol. So we can see that there are some difference between Flash
Vendor.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller

[Cyrille Pitchen]

Hi Brian,

Le 07/12/2015 20:34, Brian Norris a écrit :
> + Bean Huo
> 
> Hi Cyrille,
> 
> On Mon, Dec 07, 2015 at 03:09:09PM +0100, Cyrille Pitchen wrote:
>> Hi all,
>>
>> this series of patches adds support to the Atmel QSPI controller available
>> on sama5d2 SoCs. It was tested on a sama5d2 xplained ultra board with a 
>> Micron n25q128a13 QSPI memory and a at25df321a SPI memory.
>>
>> In order to use the Micron memory in its Quad SPI mode, the spi-nor
>> framework needed to be patched to fix the support of Quad/Dual SPI
>> protocols with some memory manufacturers such as Spansion, Micron and
>> Macronix. There are many comments in the source code to explain the
>> implementation choices based on the datasheets from memory manufacturers.
>>
>>
>> This series was based and tested on linux-next-20151207
>>
>> 1 - Atmel QSPI + Micron n25q128a13 (atmel-quadspi.c driver)
>>
>> SPI 1-1-1: This mode was tested replacing SPI_NOR_QUAD by SPI_NOR_FAST as
>>            argument to spi_nor_scan() called from atmel_qspi_probe().
>>
>> SPI 1-1-4: Bootloaders (at91bootstrap/uboot) don't enable the Quad SPI
>>            mode of the Micron memory. When probed from Linux, the memory
>>            uses its Extended SPI mode and replies to the regular Read ID
>>            (0x9f) command.
>>
>> SPI 4-4-4: The romcode enabled the Quad SPI mode the of Micron memory
>>            before loading the at91bootstrap. When probed from Linux, the
>>            memory uses its Quad SPI mode and no longer replies to the
>>            regular Read ID (0x9f) command but instead to the Read ID
>>            Multiple I/O (0xaf) command. The memory expects ALL commands
>>            to use the SPI 4-4-4 protocol.
> 
> I'll admit I'm a little fuzzy on the differences between dual and quad
> modes on various flash manufacturers. Can you help clear it up for me?
> I think some of the comments on patch 2 help too, but I'll just comment
> here for now.
> 
> It looks like the current driver has problems regarding the non 1-x-y
> modes (e.g., 4-4-4), right? But I see that spi-nor.c never tries to
> send a 4_4_4 command; it only sets read_opcode to
> SPINOR_OP_READ_1_1_{1,2,4}. So is this an oversight in patches like
> Bean's patch?
> 
>     commit 548cd3ab54da ("mtd: spi-nor: Add quad I/O support for Micron
>     SPI NOR")
> 
> Why would we even need to enable quad modes like that, if we're not
> going to send the 4-4-4 opcodes?

First let me clarify one point. This series focuses on Spansion, Macronix
and Micron because when I wrote its patches few months ago, the spi-nor.c
driver supported QSPI protocols only for these three only manufacturers.
Today I notice that now some Winbond memories also use the
SPI_NOR_QUAD_READ flag. Though I try to deal with all manufacturers on a
equal foot, I currently have no knowledge on Windond memories so my
explanations only apply to Spansion, Macronix and Micron memories.

About the SPI 4-4-4 protocol, it is only supported by Micron and Macronix
memories but not by Spansion. Both Micron and Macronix memories provide us
with a special mode, let's call it the Quad SPI mode. As Bean Huo explained
for Micron, once this mode is enabled, the memory expects all commands to
use the SPI 4-4-4 protocol. Hence even if the spi-nor.c driver uses the Fast
Read Quad Output 1-1-4 (0x6b/0x6c) or the Fast Read Quad I/O 1-4-4 (0xeb/0xec)
op codes, the command is interpreted as a Fast Read Quad Command 4-4-4 so must
use the SPI 4-4-4 protocol. As far as I know, there is no currently existing
op code dedicated to the Fast Read Quad Command 4-4-4.

So the op codes may be confusing but when the Quad SPI mode is enabled we
actually use Fast Read 4-4-4 commands.

> 
> My next question (if my understanding is roughly correct) is, do we need
> the 4-4-4 modes, and what risks come with them? I understand we can
> shorten the command and address phases, but does that alone yield much
> performance benefit? And I think the risk is that a given system might
> not be prepared for the flash to be in a 4-4-4 mode, if the boot code
> tries to use 1-x-y commands.
> 

I did not run comparative tests between Fast Read 1-1-4, Fast Read 1-4-4
and Fast Read 4-4-4 yet. Honestly I don't expect much difference. However
performances were not the main purpose when I wrote these patches.
Actually the Quad SPI mode of Macronix and Micron comes with a side effect.
Once enable, not only the memory now expects all commands to use the
SPI 4-4-4 protocol but it no longer replies to the regular Read JEDEC ID
command (0x9f). Instead it replies to a new command, the Read JEDEC ID
Multiple I/O (0xaf).
Now let's imagine that the Quad SPI mode is either permanently enabled at
reset using non-volatile configuration registers or enabled by a early
bootloader. When the spi-nor driver tries to probe the memory, the 0x9f
command fails: the driver should adapt, that's why it tries other protocols
such as SPI 4-4-4 (Micron and Macronix) and SPI 2-2-2 (Micron only when in
Dual SPI mode) with the 0xaf command. When we finally get a valid JEDEC ID,
we also know the current mode of memory.

Then the global policy of this series of patches is:
1 - try to configure the QSPI memory as asked by the user, ie according to
    the read_mode argument of spi_nor_scan().
2 - use volatile bits in configuration registers as much as possible, try to
    avoid changing non-volatile bits hence bootloaders should drive the
    QSPI memory always in the same state after reset.
3 - check the current state/mode the the QSPI memory and adapt to it in order
    to limit the configuration changes
4 - try not to introduce regression ;)

For instance with Micron memories, about the choice between the Extended
SPI mode and the Quad SPI mode, I've currently chosen to keep on using the
detected mode during the probing phase (Read JEDED ID).
Indeed the Extended SPI mode does support the Fast Read 1-1-4 command,
which should not be very different from Fast Read x-4-4 commands thinking
about performances. So I thought it was safer not to change the mode rather
than forcing the Quad SPI mode. Actually using Fast Read 1-1-4 in Extended 
SPI mode only requires the SPI_RX_QUAD capability from the SPI controller
whereas the Quad SPI mode would also require the SPI_TX_QUAD capability.
Referring to the m25p80 driver, the mode argument of spi_nor_scan() is set
to SPI_NOR_QUAD if and only if the SPI controller has the SPI_RX_QUAD flag.
That's not enough to know whether the SPI controller could support to
enable the Quad SPI mode of the Micron memory.
However, if the Quad SPI mode was already enabled and the spi-nor driver
succeeded in probing the Micron memory with the 0xaf command in SPI 4-4-4
protocol, we know that the SPI controller does support the SPI 4-4-4
protocol then we can safely leave the Quad SPI mode of the Micron memory
enabled. Same thing with Micron Dual SPI mode ant the support of the
SPI 2-2-2 protocol by the SPI controller.


Here is my understanding of the volatile/non-volatile bits in
the configuration registers for each QSPI manufacturer according to their
datasheets. Please note that some manufacturers such as Micron provide
both volatile and non-volatile registers to configure some settings.

                       Micron             Macronix           Spansion
Quad/Dual SPI Mode     volatile capable   persistent only    persistent only

# of dummy cycles      volatile capable   volatile capable   persistent only


I think the only place where I may change a non-volatile bit is in
spansion_set_quad_mode() when setting the QUAD bit in the Configuration
Register 1 (CR1) if the user asked to use a Quad SPI protocol but the QUAD
bit was not set yet. That's why I added a dev_warn() message.

About the number of dummy cycles, the spi-nor framework adapts to the current
setting of a Spansion memory.

> Also, I see a lot of good comments in patch 2 about Spansion vs.
> Macronix vs. Micron memories. I wonder if previous developers have
> completely tested their patches, or if they're just reading the
> datasheets... so, what kind have testing have you done? Do you have
> samples of all these flash to test?
> 

I want to be totally honest on this point: I did NOT test with Macronix
memories yet simply because I have no such memory from this manufacturer.
I guess Atmel planed to purchase some samples because we also need to test
their support in the sama5d2 romcode when booting from QSPI.
So for the Macronix case, the patches are only based on my current
understanding of Macronix datasheets (MX66L1G45G, 3V, 1Gb, v1.0.pdf).

On the other hand I did many tests with both Micron and Spansion memories
with sama5d2 SoCs, either with Linux or with the sama5d2 romcode ("normal"
Fast Read x-y-4 but also eXecution In Place using the Continuous Read mode:
the XIP mode is not relevant for the Linux spi-nor framework but it should
take care of the "dummy cycles" it sends to avoid entering into the
Continuous Read mode by mistake).
I used at least these memories under Linux:
- Micron n25q128a13: sama5d2 xplained ultra + linux-next 20151207 /
  at91 linux 4.1
- Micron n25q256: sama5d2 prototype (FPGA) + at91 linux 3.18
- Spansion s25fl512: sama5d2 prototype (FPGA) + at91 linux 3.18

Maybe Marek also did some tests on his side.


I hope I've answered your questions, please don't hesitate to comment/ask more
questions. I'll do my best to answer them! :)

> Regards,
> Brian
> 

Best Regards,

Cyrille

Re: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller

[Cyrille Pitchen]

Le 08/12/2015 11:25, Cyrille Pitchen a écrit :
> Hi Brian,
> 
> Le 07/12/2015 20:34, Brian Norris a écrit :
[...]
>> Also, I see a lot of good comments in patch 2 about Spansion vs.
>> Macronix vs. Micron memories. I wonder if previous developers have
>> completely tested their patches, or if they're just reading the
>> datasheets... so, what kind have testing have you done? Do you have
>> samples of all these flash to test?
>>
> 
> I want to be totally honest on this point: I did NOT test with Macronix
> memories yet simply because I have no such memory from this manufacturer.
> I guess Atmel planed to purchase some samples because we also need to test
> their support in the sama5d2 romcode when booting from QSPI.
> So for the Macronix case, the patches are only based on my current
> understanding of Macronix datasheets (MX66L1G45G, 3V, 1Gb, v1.0.pdf).
> 
> On the other hand I did many tests with both Micron and Spansion memories
> with sama5d2 SoCs, either with Linux or with the sama5d2 romcode ("normal"
> Fast Read x-y-4 but also eXecution In Place using the Continuous Read mode:
> the XIP mode is not relevant for the Linux spi-nor framework but it should
> take care of the "dummy cycles" it sends to avoid entering into the
> Continuous Read mode by mistake).
> I used at least these memories under Linux:
> - Micron n25q128a13: sama5d2 xplained ultra + linux-next 20151207 /
>   at91 linux 4.1
> - Micron n25q256: sama5d2 prototype (FPGA) + at91 linux 3.18
> - Spansion s25fl512: sama5d2 prototype (FPGA) + at91 linux 3.18
> 

I forgot few points. First, I did all Quad SPI tests using the Atmel QSPI
controller and driver.

I did no Dual SPI protocol test yet, only Quad SPI.

I'm able to test neither Quad nor Dual SPI protocols with the Atmel QSPI
protocol + m25p80 driver: this driver is not really suited for handling
the Atmel QSPI controller in its Serial Memory Mode.

However I did some non regression tests with the m25p80 driver used on the
regular Atmel SPI controller and driver (drivers/spi/spi-atmel.c) to
access a at25df312a memory (SPI 1-1-1 protocol).


Nonetheless the m25p80 driver was taken into account when I wrote the series.
Indeed Micron, Macronix and Spansion's datasheets provide tables giving the
number of dummy cycles to use depending on both the SPI bus clock
frequency and the Fast Read command. Hence in spi-nor.c, patch 2 sets
nor->read_dummy to the relevant number of dummy cycles (not bits).

Taking Spansion memories as an example, for their factory default Latency
Code value of 0, they expect:
- 8 dummy cycles for Fast Read 1-1-1 (0x0b / 0x0c)
- 8 dummy cycles for Fast Read 1-1-2 (0x3b / 0x3c)
- 8 dummy cycles for Fast Read 1-1-4 (0x6b / 0x6c)

Fast Read 1-2-2 and 1-4-4 are not used with Spansion memories since the
number of dummy cycles to be used with one of these op codes is not suited
for the m25p80 driver.

For Micron and Macronix, except when their Quad SPI mode is enabled, the
Fast Read 1-1-4 or Fast Read 1-1-2 commands are preferred to the
Fast Read 1-4-4 or Fast Read 1-2-2 commands. The number of dummy cycles is
set to 0 for Read command (0x03) and 8 for other Fast Read commands since
this setting update can be done safely writing into volatile bits inside
configuration registers. A multiple of 8 dummy cycles is suited for the
m25p80_read() implementation.

Then looking at the m25p80 driver, the original (unchanged) cycles/bytes
conversion from the m25p80_read() hook only works with SPI 1-1-x protocols:

 	/* convert the dummy cycles to the number of bytes */
 	dummy /= 8;

As a matter of fact, 8 dummy cycles stand for:
-  8 bits (1 byte ) for SPI 1-1-z protocols
- 16 bits (2 bytes) for SPI x-2-z protocols
- 32 bits (4 bytes) for SPI x-4-z protocols

So I should also fix the above conversion in patch 3 to cover more SPI
protocols.


Best Regards,

Cyrille

Re: [PATCH linux-next 4/5] Documentation: atmel-quadspi: add binding file for Atmel QSPI driver

[Rob Herring]

On Mon, Dec 07, 2015 at 03:09:13PM +0100, Cyrille Pitchen wrote:
> 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>

> ---
>  .../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..e81f20f9faf1
> --- /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 = "qpsi_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
> 

Re: [PATCH linux-next 4/5] Documentation: atmel-quadspi: add binding file for Atmel QSPI driver

[Nicolas Ferre]

Le 07/12/2015 15:09, Cyrille Pitchen a écrit :
> 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>

The change is very small from previous one and moreover accepted by Rob.
So, for sure:

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..e81f20f9faf1
> --- /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 = "qpsi_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 {
> +		...
> +	};
> +};
> 


-- 
Nicolas Ferre

Re: [PATCH linux-next 5/5] mtd: atmel-quadspi: add driver for Atmel QSPI controller

[Nicolas Ferre]

Le 07/12/2015 15:09, Cyrille Pitchen a écrit :
> 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>

Brian,

This driver has been maturing for months and my only goal is to keep on
supporting it.
As I already gave my Ack this summer, here is it now for sure:

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

I recall that Marek gave his as well and might also agree with the
changes and re-iterate his support.

Together with the current work of Cyrille on the spi-nor Quad SPI
support ("[PATCH linux-next 0/4] mtd: spi-nor: fix Quad SPI memory
support"), I have the feeling that this driver deserves an inclusion in
linux-next.
The infrastructure put in place and then used by this driver will
certainly help other developers to build their driver on top. I had the
impression that at least Marek was considering to use this for his
Cadence driver:

His quote from mid-September:
"
Are there any news on this patch series? I'd like to use that for my own
QSPI driver (the Cadence one), so I'd like to check on the status. Are you
still working on this please ?
"

Can we find a way to give this work even more exposure and consider at
least the spi-nor and m25p80 parts mentioned just above for mainline
inclusion?

Best regards,

> ---
>  drivers/mtd/spi-nor/Kconfig         |   8 +
>  drivers/mtd/spi-nor/Makefile        |   3 +-
>  drivers/mtd/spi-nor/atmel-quadspi.c | 877 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 887 insertions(+), 1 deletion(-)
>  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 0dc927540b3d..15f45dbbfe0d 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -37,6 +37,14 @@ 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 OF && HAS_DMA && (ARCH_AT91 || COMPILE_TEST)
> +	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..0cbed0e4ae8c 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,4 +1,5 @@
>  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_MTD_MT81xx_NOR)	+= mtk-quadspi.o
> +obj-$(CONFIG_SPI_ATMEL_QUADSPI)	+= atmel-quadspi.o
>  obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.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..bdebfdc92842
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/atmel-quadspi.c
> @@ -0,0 +1,877 @@
> +/*
> + * 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/dma-mapping.h>
> +#include <linux/dmaengine.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/platform_data/dma-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)
> +
> +/* 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;
> +	dma_addr_t		phys_addr;
> +	struct dma_chan		*chan;
> +	struct clk		*clk;
> +	struct platform_device	*pdev;
> +	u32			pending;
> +
> +	struct spi_nor		nor;
> +	u32			clk_rate;
> +	struct completion	cmd_completion;
> +	struct completion	dma_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	dma:1;
> +			u32	reserved:24;
> +		}		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);
> +}
> +
> +
> +#define QSPI_DMA_THRESHOLD	32
> +
> +static void atmel_qspi_dma_callback(void *arg)
> +{
> +	struct completion *dma_completion = arg;
> +
> +	complete(dma_completion);
> +}
> +
> +static int atmel_qspi_run_dma_transfer(struct atmel_qspi *aq,
> +				       const struct atmel_qspi_command *cmd)
> +{
> +	u32 offset = (cmd->enable.bits.address) ? cmd->address : 0;
> +	struct dma_chan *chan = aq->chan;
> +	struct device *dev = &aq->pdev->dev;
> +	enum dma_data_direction direction;
> +	dma_addr_t phys_addr, dst, src;
> +	struct dma_async_tx_descriptor *desc;
> +	dma_cookie_t cookie;
> +	int err = 0;
> +
> +	if (cmd->tx_buf) {
> +		direction = DMA_TO_DEVICE;
> +		phys_addr = dma_map_single(dev, (void *)cmd->tx_buf,
> +					   cmd->buf_len, direction);
> +		src = phys_addr;
> +		dst = aq->phys_addr + offset;
> +	} else {
> +		direction = DMA_FROM_DEVICE;
> +		phys_addr = dma_map_single(dev, (void *)cmd->rx_buf,
> +					   cmd->buf_len, direction);
> +		src = aq->phys_addr + offset;
> +		dst = phys_addr;
> +	}
> +	if (dma_mapping_error(dev, phys_addr))
> +		return -ENOMEM;
> +
> +	desc = chan->device->device_prep_dma_memcpy(chan, dst, src,
> +						    cmd->buf_len,
> +						    DMA_PREP_INTERRUPT);
> +	if (!desc) {
> +		err = -ENOMEM;
> +		goto unmap_single;
> +	}
> +
> +	reinit_completion(&aq->dma_completion);
> +	desc->callback = atmel_qspi_dma_callback;
> +	desc->callback_param = &aq->dma_completion;
> +	cookie = dmaengine_submit(desc);
> +	err = dma_submit_error(cookie);
> +	if (err)
> +		goto unmap_single;
> +	dma_async_issue_pending(chan);
> +
> +	if (!wait_for_completion_timeout(&aq->dma_completion,
> +					 msecs_to_jiffies(1000)))
> +		err = -ETIMEDOUT;
> +
> +	if (dma_async_is_tx_complete(chan, cookie, NULL, NULL) != DMA_COMPLETE)
> +		err = -ETIMEDOUT;
> +
> +	if (err)
> +		dmaengine_terminate_all(chan);
> +unmap_single:
> +	dma_unmap_single(dev, phys_addr, cmd->buf_len, direction);
> +
> +	return err;
> +}
> +
> +static int atmel_qspi_run_transfer(struct atmel_qspi *aq,
> +				   const struct atmel_qspi_command *cmd)
> +{
> +	void __iomem *ahb_mem;
> +
> +	/* First try a DMA transfer */
> +	if (aq->chan && cmd->enable.bits.dma &&
> +	    cmd->buf_len >= QSPI_DMA_THRESHOLD)
> +		return atmel_qspi_run_dma_transfer(aq, cmd);
> +
> +	/* 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;
> +	}
> +
> +	/* 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_INSTRE)
> +		return err;
> +
> +	/* Wait for INSTRuction End interrupt */
> +	reinit_completion(&aq->cmd_completion);
> +	aq->pending = 0;
> +	qspi_writel(aq, QSPI_IER, QSPI_SR_INSTRE);
> +	if (!wait_for_completion_timeout(&aq->cmd_completion,
> +					 msecs_to_jiffies(1000)))
> +		err = -ETIMEDOUT;
> +	qspi_writel(aq, QSPI_IDR, QSPI_SR_INSTRE);
> +
> +	return err;
> +}
> +
> +static int atmel_qspi_command_set_ifr(struct atmel_qspi_command *cmd,
> +				      u32 ifr_tfrtyp,
> +				      enum spi_protocol proto)
> +{
> +	cmd->ifr = ifr_tfrtyp;
> +
> +	switch (proto) {
> +	case SPI_PROTO_1_1_1:
> +		cmd->ifr |= QSPI_IFR_WIDTH_SINGLE_BIT_SPI;
> +		break;
> +	case SPI_PROTO_1_1_2:
> +		cmd->ifr |= QSPI_IFR_WIDTH_DUAL_OUTPUT;
> +		break;
> +	case SPI_PROTO_1_1_4:
> +		cmd->ifr |= QSPI_IFR_WIDTH_QUAD_OUTPUT;
> +		break;
> +	case SPI_PROTO_1_2_2:
> +		cmd->ifr |= QSPI_IFR_WIDTH_DUAL_IO;
> +		break;
> +	case SPI_PROTO_1_4_4:
> +		cmd->ifr |= QSPI_IFR_WIDTH_QUAD_IO;
> +		break;
> +	case SPI_PROTO_2_2_2:
> +		cmd->ifr |= QSPI_IFR_WIDTH_DUAL_CMD;
> +		break;
> +	case SPI_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.enable.bits.dma = 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.enable.bits.dma = 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_SSM | QSPI_MR_NBBITS(8);
> +	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 (pending & QSPI_SR_INSTRE)
> +		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 atmel_qspi *aq;
> +	struct resource *res;
> +	dma_cap_mask_t mask;
> +	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);
> +	init_completion(&aq->dma_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->regs);
> +		goto exit;
> +	}
> +	aq->phys_addr = (dma_addr_t)res->start;
> +
> +	/* 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;
> +
> +	/* Try to get a DMA channel for memcpy() operation */
> +	dma_cap_zero(mask);
> +	dma_cap_set(DMA_MEMCPY, mask);
> +	aq->chan = dma_request_channel(mask, NULL, NULL);
> +	if (!aq->chan)
> +		dev_warn(&pdev->dev, "no available DMA channel\n");
> +
> +	/* 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_property_read_u32(child, "spi-max-frequency", &aq->clk_rate);
> +	if (err < 0)
> +		goto release_channel;
> +
> +	err = atmel_qspi_init(aq);
> +	if (err)
> +		goto release_channel;
> +
> +	err = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
> +	if (err)
> +		goto release_channel;
> +
> +	err = mtd_device_register(mtd, NULL, 0);
> +	if (err)
> +		goto release_channel;
> +
> +	of_node_put(child);
> +
> +	return 0;
> +
> +release_channel:
> +	if (aq->chan)
> +		dma_release_channel(aq->chan);
> +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);
> +	if (aq->chan)
> +		dma_release_channel(aq->chan);
> +	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");
> 


-- 
Nicolas Ferre

Re: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller

[Brian Norris]

Hi Bean,

On Tue, Dec 08, 2015 at 06:21:00AM +0000, Bean Huo 霍斌斌 wrote:
> > -----Original Message-----
> > From: Brian Norris [mailto:computersforpeace@gmail.com]
> > 
> > I'll admit I'm a little fuzzy on the differences between dual and quad modes on
> > various flash manufacturers. Can you help clear it up for me?
> 
> For Micron SPI NOR spi quad mode, means that Qaud I/O prototocol, it follows I/O
> Bus width is command-address-Data 4-4-4, at this time, DQ0,DQ1,DQ2,DQ3
> are all used to transfer address/command/data. For this maybe not the same between
> different flash manufactures. For example, for Spansion Qspi NOR, its all instructions are
> transferred from host to memory as a single bit serial sequence on the DQ0 signal, even under
> Quad mode.  Dual mode the same as Qaud mode scenario.
> 
> for SPI NOR 1-1-4, means command and address are transferred on the DQ0,
> but for data, being transferred on DQ0,DQ1,DQ2,DQ3.For this, it is the same
> between different flash manufacturers. Of course, at this moment, SPI NOR
> should work under extended I/O mode.

OK, so to make these statements *much* shorter:

 * Micron "Quad Mode" means putting the flash in a 4/4/4 mode

 * Spansion (and all others?) are using 1/1/4 modes

Correct?

> > I think some of the comments on patch 2 help too, but I'll just comment here
> > for now.
> > It looks like the current driver has problems regarding the non 1-x-y modes
> > (e.g., 4-4-4), right? But I see that spi-nor.c never tries to send a 4_4_4
> > command; it only sets read_opcode to SPINOR_OP_READ_1_1_{1,2,4}. So is
> > this an oversight in patches like Bean's patch?
> 
> For SPINOR_OP_READ_1_1_{1,2,4} commands, Spi NOR actually works under
> Extended I/O mode, not Quad mode. They just push Spi NOR output data by Quad mode,
> Command and address still following extended I/O mode.

The naming is confusing enough here... so in your words, "extended"
means 1/1/{1,2,4} (i.e., command, and maybe address, use 1 line, but
data goes on 4)? And "quad" means 4/4/4?

> For 4-4-4 I/O protocol, SPI NOR should change to Quad mode(just as my patch), 
> of course, SPI controller should support this. for Micron Qspi NOR, under quad mode,
> all commands/address/data are transferred on DQ0,DQ1,DQ2,DQ3 signals. No matter what
> kind of command. 

OK, so I think your patch is broken:

> >     commit 548cd3ab54da ("mtd: spi-nor: Add quad I/O support for Micron
> >     SPI NOR")

How did you test this? Specifically, this can't possibly have worked
with a regular drivers/spi/ controllers, since:

 (a) you're enabling 4/4/4 (i.e., "Quad mode") on the flash but
 (b) m25p80_read() only sets .rx_bits for the data; i.e., it's using
 1/1/4 (i.e., "Extended mode")

I'm tempted to essentially revert that, as it looks essentially
untested. It would be nice to have a cleaner baseline before trying to
extend it with Cyrille's work.

Cyrille, what do you think? Is my analysis at all correct here? (Sorry
if this is addressed elsewhere; there's a lot of text in this
conversation, but I'm getting hung up very early.) And if so, does it
hurt to just drop Micron "Quad mode" (4/4/4)?

(AIUI, this won't exactly be a panacea, since you mention bootloaders
that start us off in quad mode, so we can't use single I/O 0x9f READ
ID.)

> > Why would we even need to enable quad modes like that, if we're not going
> > to send the 4-4-4 opcodes?
> I think, in order to high speed SPI NOR, after enable quad mode, 
> SPINOR_OP_READ_1_1_{1,2,4} commands don't need any more, normal read command (0x03)
> Can implement as them.

OK. That's odd, but I guess it doesn't matter much. It just makes it a
little less obvious what's going on.

> > My next question (if my understanding is roughly correct) is, do we need the
> > 4-4-4 modes, and what risks come with them? I understand we can shorten
> > the command and address phases, but does that alone yield much
> > performance benefit? And I think the risk is that a given system might not be
> > prepared for the flash to be in a 4-4-4 mode, if the boot code tries to use 1-x-y
> > commands.
> 
> As far as my current experience and knowledge, this still need to be enabled, especially for
> fast boot, and some IOT devices to store info into SPI NOR.

Do you have any data to about the speed? And you haven't addressed the
risks. There are definitely risks. Cyrille looks like he's trying to
address the risks (e.g., use volatile modes whenever possible), but it
doesn't seem that you are.

> For this patches, my current concern is that host side how to get different I/O protocol changes,
> and distinguish between different flash manufacturers I/O mode.

Brian

Re: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller

[Brian Norris]

On Thu, Dec 17, 2015 at 04:29:01PM -0800, Brian Norris wrote:
> On Tue, Dec 08, 2015 at 06:21:00AM +0000, Bean Huo 霍斌斌 wrote:

> OK, so I think your patch is broken:
> 
> > >     commit 548cd3ab54da ("mtd: spi-nor: Add quad I/O support for Micron
> > >     SPI NOR")
> 
> How did you test this? Specifically, this can't possibly have worked
> with a regular drivers/spi/ controllers, since:
> 
>  (a) you're enabling 4/4/4 (i.e., "Quad mode") on the flash but
>  (b) m25p80_read() only sets .rx_bits for the data; i.e., it's using
>  1/1/4 (i.e., "Extended mode")
> 
> I'm tempted to essentially revert that, as it looks essentially
> untested. It would be nice to have a cleaner baseline before trying to
> extend it with Cyrille's work.
> 
> Cyrille, what do you think? Is my analysis at all correct here? (Sorry
> if this is addressed elsewhere; there's a lot of text in this
> conversation, but I'm getting hung up very early.) And if so, does it
> hurt to just drop Micron "Quad mode" (4/4/4)?

It looks like you address some of this in patch 2, where you (as I do)
claim that Micron support is broken. It seems to me that it could be
better to kill it than to try to fix it. But maybe that's just the
frustrated maintainer in me speaking.

Brian

Re: [PATCH linux-next 1/5] mtd: spi-nor: properly detect the memory when it boots in Quad or Dual mode

[Brian Norris]

Hi Cyrille,

On Mon, Dec 07, 2015 at 03:09:10PM +0100, Cyrille Pitchen wrote:
> 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.
> 
> Micron and Macronix memories, once their Quad (or dual for Micron) mode
> enabled, no longer process the regular JEDEC Read ID (0x9f) command but
> instead reply to a new command: JEDEC Read ID Multiple I/O (0xaf).
> Besides, in Quad mode both memory manufacturers expect ALL commands to
> use the SPI 4-4-4 protocol. For Micron memories, enabling their Dual mode
> implies to use the SPI 2-2-2 protocol for ALL commands.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 52 +++++++++++++++++++++++++++++++++++++++----
>  include/linux/mtd/spi-nor.h   | 23 +++++++++++++++++--
>  2 files changed, 69 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 3b2460efc019..bf17736750c1 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -73,6 +73,11 @@ struct flash_info {
>  
>  #define JEDEC_MFR(info)	((info)->id[0])
>  
> +struct read_id_config {
> +	enum read_mode		mode;
> +	enum spi_protocol	proto;
> +};
> +
>  static const struct flash_info *spi_nor_match_id(const char *name);
>  
>  /*
> @@ -867,11 +872,16 @@ 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,
> +						enum read_mode mode)

It's unclear what you're trying to do with the 'read_mode' enum now.
(Admittedly it may not be clear in the current code either, given the
confusion we already have over Micron support.)

Would you care to document it better?

>  {
> -	int			tmp;
> +	int			i, tmp;
>  	u8			id[SPI_NOR_MAX_ID_LEN];
>  	const struct flash_info	*info;
> +	static const struct read_id_config configs[] = {
> +		{SPI_NOR_QUAD, SPI_PROTO_4_4_4},
> +		{SPI_NOR_DUAL, SPI_PROTO_2_2_2}
> +	};
>  
>  	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
>  	if (tmp < 0) {
> @@ -879,6 +889,34 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
>  		return ERR_PTR(tmp);
>  	}
>  
> +	/* Special case for Micron/Macronix qspi nor. */
> +	if ((id[0] == 0xff && id[1] == 0xff && id[2] == 0xff) ||
> +	    (id[0] == 0x00 && id[1] == 0x00 && id[2] == 0x00))
> +		for (i = 0; i < ARRAY_SIZE(configs); ++i) {
> +			if (configs[i].mode != mode)
> +				continue;
> +
> +			/* Set this protocol for all commands. */
> +			nor->reg_proto = configs[i].proto;
> +			nor->read_proto = configs[i].proto;
> +			nor->write_proto = configs[i].proto;
> +			nor->erase_proto = configs[i].proto;

Are these all fully independent? Do we really need 4 fields for this?

> +
> +			/*
> +			 * Multiple I/O Read ID only returns the Manufacturer ID
> +			 * (1 byte) and the Device ID (2 bytes). So we reset the
> +			 * remaining bytes.
> +			 */
> +			memset(id, 0, sizeof(id));
> +			tmp = nor->read_reg(nor, SPINOR_OP_MIO_RDID, id, 3);
> +			if (tmp < 0) {
> +				dev_dbg(nor->dev,
> +					"error %d reading JEDEC ID Multi I/O\n",
> +					tmp);
> +				return ERR_PTR(tmp);
> +			}
> +		}
> +
>  	for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
>  		info = &spi_nor_ids[tmp];
>  		if (info->id_len) {
> @@ -1178,11 +1216,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 = SPI_PROTO_1_1_1;
> +	nor->read_proto = SPI_PROTO_1_1_1;
> +	nor->write_proto = SPI_PROTO_1_1_1;
> +	nor->reg_proto = SPI_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, mode);
>  	if (IS_ERR_OR_NULL(info))
>  		return -ENOENT;
>  
> @@ -1193,7 +1237,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, mode);
>  		if (IS_ERR(jinfo)) {
>  			return PTR_ERR(jinfo);
>  		} else if (jinfo != info) {
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index fac3f6f53981..c91986a99caf 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -75,8 +75,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 */
> @@ -105,6 +106,16 @@ enum read_mode {
>  	SPI_NOR_QUAD,
>  };
>  
> +enum spi_protocol {
> +	SPI_PROTO_1_1_1,	/* SPI */
> +	SPI_PROTO_1_1_2,	/* Dual Output */
> +	SPI_PROTO_1_1_4,	/* Quad Output */
> +	SPI_PROTO_1_2_2,	/* Dual IO */
> +	SPI_PROTO_1_4_4,	/* Quad IO */
> +	SPI_PROTO_2_2_2,	/* Dual Command */
> +	SPI_PROTO_4_4_4,	/* Quad Command */

Would it help at all to make this enum into something more like a
bitfield? So in some cases, rather than a bit switch block, we can just
extract the "number of lines" from the integer value? e.g.:

#define SNOR_PROTO(command, addr, data) \
	(((command) << 0) | \
	 ((addr) << 4) | \
	 ((data) << 8)) // or some other kind of macro magic

enum spi_nor_protocol {
	SNOR_PROTO_1_1_1		= SNOR_PROTO(1, 1, 1),
	SNOR_PROTO_1_1_2		= SNOR_PROTO(1, 1, 2),
	...
};

static inline int spi_nor_io_lines_command(enum spi_nor_protocol proto)
{
	return proto & 0xf;
}

(Similar for addr and data phases. Also, my naming might suck. Feel free
to improve!)

I don't think we should stomp on the SPI namespace with the
"SPI_PROTO_*" definitions. That's why I chose SNOR_PROTO_ and spi_nor_
prefixes.

Brian

> +};
> +
>  #define SPI_NOR_MAX_CMD_SIZE	8
>  enum spi_nor_ops {
>  	SPI_NOR_OPS_READ = 0,
> @@ -132,6 +143,10 @@ enum spi_nor_option_flags {
>   * @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
> @@ -160,6 +175,10 @@ struct spi_nor {
>  	u8			read_opcode;
>  	u8			read_dummy;
>  	u8			program_opcode;
> +	enum spi_protocol	erase_proto;
> +	enum spi_protocol	read_proto;
> +	enum spi_protocol	write_proto;
> +	enum spi_protocol	reg_proto;
>  	enum read_mode		flash_read;
>  	bool			sst_write_second;
>  	u32			flags;
> -- 
> 1.8.2.2
> 

Re: [PATCH linux-next 1/5] mtd: spi-nor: properly detect the memory when it boots in Quad or Dual mode

[Brian Norris]

(Hit send too early; a few more comments)

On Mon, Dec 07, 2015 at 03:09:10PM +0100, Cyrille Pitchen wrote:
>  drivers/mtd/spi-nor/spi-nor.c | 52 +++++++++++++++++++++++++++++++++++++++----
>  include/linux/mtd/spi-nor.h   | 23 +++++++++++++++++--
>  2 files changed, 69 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 3b2460efc019..bf17736750c1 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -73,6 +73,11 @@ struct flash_info {
>  
>  #define JEDEC_MFR(info)	((info)->id[0])
>  
> +struct read_id_config {
> +	enum read_mode		mode;
> +	enum spi_protocol	proto;
> +};
> +
>  static const struct flash_info *spi_nor_match_id(const char *name);
>  
>  /*
> @@ -867,11 +872,16 @@ 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,
> +						enum read_mode mode)
>  {
> -	int			tmp;
> +	int			i, tmp;
>  	u8			id[SPI_NOR_MAX_ID_LEN];
>  	const struct flash_info	*info;
> +	static const struct read_id_config configs[] = {
> +		{SPI_NOR_QUAD, SPI_PROTO_4_4_4},
> +		{SPI_NOR_DUAL, SPI_PROTO_2_2_2}
> +	};
>  
>  	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
>  	if (tmp < 0) {
> @@ -879,6 +889,34 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
>  		return ERR_PTR(tmp);
>  	}
>  
> +	/* Special case for Micron/Macronix qspi nor. */
> +	if ((id[0] == 0xff && id[1] == 0xff && id[2] == 0xff) ||
> +	    (id[0] == 0x00 && id[1] == 0x00 && id[2] == 0x00))

Is this specified anywhere, or is this just a heuristic, to guess
whether we're getting valid IDs? Do we know anything about what opcodes
look like ...

> +		for (i = 0; i < ARRAY_SIZE(configs); ++i) {
> +			if (configs[i].mode != mode)
> +				continue;
> +
> +			/* Set this protocol for all commands. */
> +			nor->reg_proto = configs[i].proto;
> +			nor->read_proto = configs[i].proto;
> +			nor->write_proto = configs[i].proto;
> +			nor->erase_proto = configs[i].proto;
> +
> +			/*
> +			 * Multiple I/O Read ID only returns the Manufacturer ID
> +			 * (1 byte) and the Device ID (2 bytes). So we reset the
> +			 * remaining bytes.
> +			 */

Ugh, does that mean we get different IDs returned via the different
modes? That sounds like a disaster. What about all the flash that we
need 5+ bytes to differentiate? Just be glad we haven't come across any
Micron or Macronix like that yet?

> +			memset(id, 0, sizeof(id));
> +			tmp = nor->read_reg(nor, SPINOR_OP_MIO_RDID, id, 3);

Hmm, so you're passing implicit configuration data via the
nor->reg_proto field. So, spi-nor drivers now have to read that field
during every read_reg() invocation? Seems like either this should be:
 (a) a driver hook/callback, so we can just reconfigure a handful of
 times or
 (b) part of a parameter that gets passed as part of the function
 signature

> +			if (tmp < 0) {
> +				dev_dbg(nor->dev,
> +					"error %d reading JEDEC ID Multi I/O\n",
> +					tmp);
> +				return ERR_PTR(tmp);
> +			}
> +		}
> +
>  	for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
>  		info = &spi_nor_ids[tmp];
>  		if (info->id_len) {
[...]

Brian

Re: [PATCH linux-next 2/5] mtd: spi-nor: fix Quad SPI mode support for Spansion, Micron and Macronix

[Brian Norris]

On Mon, Dec 07, 2015 at 03:09:11PM +0100, Cyrille Pitchen wrote:
> This patch reworks the support of Quad and Dual SPI protocols for Micron,
> Spansion and Macronix Quad/Dual capable memories. Indeed, in the best
> case, only Spansion memories are correctly supported by the current
> spi-nor framework.

^^ Ah, so this is what I was struggling with at first. I agree that
Micron looks broken. Quite possibly Macronix too. Unfortunately, I
haven't had great test hardware for some of the quad modes. Especially
not anything that supports generic SPI, and not completely on mainline.

> 1 - Micron:
> When their Quad SPI mode is enabled, Micron spi-nor memories expect all
> commands to use the SPI 4-4-4 protocol. Also when the Dual SPI mode is
> enabled, all commands must use the SPI 2-2-2 protocol.
> 
> Before this patch, the spi-nor framework used to always enable the Quad
> mode when the mode argument of spi_nor_scan() took the value SPI_NOR_QUAD.
> That was not suited with drivers only supporting SPI 1-x-4 protocols but
> not the 4-4-4 (e.g. the m25p80 driver). Also the SPI controller was not
> notified about which SPI protocol to use to transfert command. We cannot
> rely only on the op code: in Extended SPI mode the 0x6b command must use
> the SPI 1-1-4 protocol whereas in Quad SPi mode the SPI 4-4-4 protocol
> must be use instead.
> 
> After this patch, the spi-nor framework uses the result of the
> spi_nor_read_id() function to choose the right SPI protocol to be used.
> If the reg_proto was set to SPI_PROTO_4_4_4, we already know that the Quad
> SPI mode is already enabled and that the SPI controller supports the SPI
> 4-4-4 protocol (otherwise it would have fail to read the JEDEC ID with the
> 0xaf op code). For the very same reason, if the reg_proto was set to
> SPI_PROTO_2_2_2, we already know that the Dual mode is already enabled and
> that the SPI controller supports the SPI 2-2-2 protocol.
> Otherwise we switch back to the Extended SPI protocol, which supports at
> least the Fast Read commands:
> - 1-1-1 (0x0b)
> - Dual Output 1-1-2 (0x3b)
> - Quad Output 1-1-4 (0x6b)
> 
> We also safely set the number of dummy cycles to 8 for Fast Read commands
> through the Volatile Configuration Register (VCR): some drivers (m25p80)
> or SPI controllers only support a number of dummy cycles multiple of 8.
> This number may have previouly been set to an unsupported value by an
> early bootloader or at reset thanks to the Non-Volatile Configuration
> Register.

It's not clear to me how you're being safe with the dummy cycles at all.
It seems like you're introducing new values that may be incompatible
with drivers. That can be OK, but you have to give drivers a chance to
opt-out... Maybe some kind of "host capability" flags?

> Finally the XIP bit is always set in the VCR to disable the Continuous
> Read mode as we don't want to care about mode cycles.
> 
> 2 - Macronix:
> When the QPI mode is enabled, all commands must use the SPI 4-4-4 protocol
> and only the 0xeb op code is supported for Fast Read commands.
> Before this patch, the spi-nor framework used to force the QPI mode but
> used the 0x6b op code for Fast Read commands when the SPI controller
> claims to support Quad SPI mode.
> This patch uses the result of spi_nor_read_id() to guess whether the QPI
> mode is both enable and supported by the SPI controller (otherwise it
> would have failed to read the JEDEC ID with the 0xaf op code).
> When the QPI mode is disabled, Macronix memories still support the
> following Fast Read commands:
> - 1-1-1 (0x0b)
> - Dual Output 1-1-2 (0x3b)
> - Quad Output 1-1-4 (0x6b)
> So if the QPI mode has not already been enabled, there is not need to
> enable it. We also avoid the 0xbb (Dual I/O 1-2-2) and 0xeb (Quad I/O
> 1-4-4) op codes on purpose as we don't want to care about the value to set
> in mode cycles not to enter the Continuous Read (Performance Enhance)
> mode.
> 
> As for Micron memories, the spi-nor framework now safely sets the number
> of dummy cycles to 8 thanks to 2 volatile bits inside the Configuration
> Register.
> 
> 3 - Spansion:
> As for Macronix, we avoid the 0xbb (Dual I/O 1-2-2) and 0xeb (Quad I/O
> 1-4-4) op codes on purpose as we don't want to care about the value to set
> in mode cycles not to enter in the Continuous Read mode.
> 
> Besides, we only care about the Quad Enable bit inside the Configuration
> Register (CR) when using Quad operations. In such a case, we first check
> its state before trying to set it. Now we also notify the user about the
> update of this non-volatile bit.
> 
> We also check the Latency Code (LC) in CR to know the exact number of
> dummy cycles to use when performing a Fast Read operation. Currently only
> the 0x0b, 0x3b and 0x6b op codes are used to perform Fast Read operation
> so the number of dummy cycles is always either 0 or 8. Hence no regression
> should be introduced.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 783 +++++++++++++++++++++++++++++++++++++-----
>  include/linux/mtd/spi-nor.h   |  15 +-
>  2 files changed, 699 insertions(+), 99 deletions(-)

That's quite a lot to do in one patch, both in number of lines of code,
and in number of tasks outlined in the commit description. Can this be
broken down at all to be more modular and incremental?

Snipped the rest of the patch for now.

Brian

Re: [PATCH linux-next 1/5] mtd: spi-nor: properly detect the memory when it boots in Quad or Dual mode

[Cyrille Pitchen]

Hi Brian,

I will be on vacation till 2016 January, 4th.
I will try to answer your questions as soon as possible.

Best regards,

Cyrille

Le 18/12/2015 02:55, Brian Norris a écrit :
> Hi Cyrille,
> 
> On Mon, Dec 07, 2015 at 03:09:10PM +0100, Cyrille Pitchen wrote:
>> 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.
>>
>> Micron and Macronix memories, once their Quad (or dual for Micron) mode
>> enabled, no longer process the regular JEDEC Read ID (0x9f) command but
>> instead reply to a new command: JEDEC Read ID Multiple I/O (0xaf).
>> Besides, in Quad mode both memory manufacturers expect ALL commands to
>> use the SPI 4-4-4 protocol. For Micron memories, enabling their Dual mode
>> implies to use the SPI 2-2-2 protocol for ALL commands.
>>
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 52 +++++++++++++++++++++++++++++++++++++++----
>>  include/linux/mtd/spi-nor.h   | 23 +++++++++++++++++--
>>  2 files changed, 69 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 3b2460efc019..bf17736750c1 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -73,6 +73,11 @@ struct flash_info {
>>  
>>  #define JEDEC_MFR(info)	((info)->id[0])
>>  
>> +struct read_id_config {
>> +	enum read_mode		mode;
>> +	enum spi_protocol	proto;
>> +};
>> +
>>  static const struct flash_info *spi_nor_match_id(const char *name);
>>  
>>  /*
>> @@ -867,11 +872,16 @@ 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,
>> +						enum read_mode mode)
> 
> It's unclear what you're trying to do with the 'read_mode' enum now.
> (Admittedly it may not be clear in the current code either, given the
> confusion we already have over Micron support.)
> 
> Would you care to document it better?
> 
>>  {
>> -	int			tmp;
>> +	int			i, tmp;
>>  	u8			id[SPI_NOR_MAX_ID_LEN];
>>  	const struct flash_info	*info;
>> +	static const struct read_id_config configs[] = {
>> +		{SPI_NOR_QUAD, SPI_PROTO_4_4_4},
>> +		{SPI_NOR_DUAL, SPI_PROTO_2_2_2}
>> +	};
>>  
>>  	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
>>  	if (tmp < 0) {
>> @@ -879,6 +889,34 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
>>  		return ERR_PTR(tmp);
>>  	}
>>  
>> +	/* Special case for Micron/Macronix qspi nor. */
>> +	if ((id[0] == 0xff && id[1] == 0xff && id[2] == 0xff) ||
>> +	    (id[0] == 0x00 && id[1] == 0x00 && id[2] == 0x00))
>> +		for (i = 0; i < ARRAY_SIZE(configs); ++i) {
>> +			if (configs[i].mode != mode)
>> +				continue;
>> +
>> +			/* Set this protocol for all commands. */
>> +			nor->reg_proto = configs[i].proto;
>> +			nor->read_proto = configs[i].proto;
>> +			nor->write_proto = configs[i].proto;
>> +			nor->erase_proto = configs[i].proto;
> 
> Are these all fully independent? Do we really need 4 fields for this?
> 
>> +
>> +			/*
>> +			 * Multiple I/O Read ID only returns the Manufacturer ID
>> +			 * (1 byte) and the Device ID (2 bytes). So we reset the
>> +			 * remaining bytes.
>> +			 */
>> +			memset(id, 0, sizeof(id));
>> +			tmp = nor->read_reg(nor, SPINOR_OP_MIO_RDID, id, 3);
>> +			if (tmp < 0) {
>> +				dev_dbg(nor->dev,
>> +					"error %d reading JEDEC ID Multi I/O\n",
>> +					tmp);
>> +				return ERR_PTR(tmp);
>> +			}
>> +		}
>> +
>>  	for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
>>  		info = &spi_nor_ids[tmp];
>>  		if (info->id_len) {
>> @@ -1178,11 +1216,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 = SPI_PROTO_1_1_1;
>> +	nor->read_proto = SPI_PROTO_1_1_1;
>> +	nor->write_proto = SPI_PROTO_1_1_1;
>> +	nor->reg_proto = SPI_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, mode);
>>  	if (IS_ERR_OR_NULL(info))
>>  		return -ENOENT;
>>  
>> @@ -1193,7 +1237,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, mode);
>>  		if (IS_ERR(jinfo)) {
>>  			return PTR_ERR(jinfo);
>>  		} else if (jinfo != info) {
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>> index fac3f6f53981..c91986a99caf 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -75,8 +75,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 */
>> @@ -105,6 +106,16 @@ enum read_mode {
>>  	SPI_NOR_QUAD,
>>  };
>>  
>> +enum spi_protocol {
>> +	SPI_PROTO_1_1_1,	/* SPI */
>> +	SPI_PROTO_1_1_2,	/* Dual Output */
>> +	SPI_PROTO_1_1_4,	/* Quad Output */
>> +	SPI_PROTO_1_2_2,	/* Dual IO */
>> +	SPI_PROTO_1_4_4,	/* Quad IO */
>> +	SPI_PROTO_2_2_2,	/* Dual Command */
>> +	SPI_PROTO_4_4_4,	/* Quad Command */
> 
> Would it help at all to make this enum into something more like a
> bitfield? So in some cases, rather than a bit switch block, we can just
> extract the "number of lines" from the integer value? e.g.:
> 
> #define SNOR_PROTO(command, addr, data) \
> 	(((command) << 0) | \
> 	 ((addr) << 4) | \
> 	 ((data) << 8)) // or some other kind of macro magic
> 
> enum spi_nor_protocol {
> 	SNOR_PROTO_1_1_1		= SNOR_PROTO(1, 1, 1),
> 	SNOR_PROTO_1_1_2		= SNOR_PROTO(1, 1, 2),
> 	...
> };
> 
> static inline int spi_nor_io_lines_command(enum spi_nor_protocol proto)
> {
> 	return proto & 0xf;
> }
> 
> (Similar for addr and data phases. Also, my naming might suck. Feel free
> to improve!)
> 
> I don't think we should stomp on the SPI namespace with the
> "SPI_PROTO_*" definitions. That's why I chose SNOR_PROTO_ and spi_nor_
> prefixes.
> 
> Brian
> 
>> +};
>> +
>>  #define SPI_NOR_MAX_CMD_SIZE	8
>>  enum spi_nor_ops {
>>  	SPI_NOR_OPS_READ = 0,
>> @@ -132,6 +143,10 @@ enum spi_nor_option_flags {
>>   * @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
>> @@ -160,6 +175,10 @@ struct spi_nor {
>>  	u8			read_opcode;
>>  	u8			read_dummy;
>>  	u8			program_opcode;
>> +	enum spi_protocol	erase_proto;
>> +	enum spi_protocol	read_proto;
>> +	enum spi_protocol	write_proto;
>> +	enum spi_protocol	reg_proto;
>>  	enum read_mode		flash_read;
>>  	bool			sst_write_second;
>>  	u32			flags;
>> -- 
>> 1.8.2.2
>>

Re: [PATCH linux-next 2/5] mtd: spi-nor: fix Quad SPI mode support for Spansion, Micron and Macronix

[Cyrille Pitchen]

Hi Brian,

Le 18/12/2015 03:18, Brian Norris a écrit :
> On Mon, Dec 07, 2015 at 03:09:11PM +0100, Cyrille Pitchen wrote:
>> This patch reworks the support of Quad and Dual SPI protocols for Micron,
>> Spansion and Macronix Quad/Dual capable memories. Indeed, in the best
>> case, only Spansion memories are correctly supported by the current
>> spi-nor framework.
> 
> ^^ Ah, so this is what I was struggling with at first. I agree that
> Micron looks broken. Quite possibly Macronix too. Unfortunately, I
> haven't had great test hardware for some of the quad modes. Especially
> not anything that supports generic SPI, and not completely on mainline.
> 

To explain the origin of this series of patches I would say that at first
I was only supposed to write a driver for the Atmel QSPI controller. I was
using (ans still use) a sama5d2 xplained board with a Micron n25q128a13
memory. Hence, testing the driver, I found that the probe failed inside the
old micron_quad_enable() function.
Indeed nor->write_reg() was successfully called to clear the Quad Enable bit
in the Enhanced Volatile Configuration Register but immediately after
spi_nor_wait_till_ready() failed.
The reason was that the Micron memory was switched to its Quad Mode and then
expected all the following commands to use the SPI 4-4-4 protocol. However
the Atmel QSPI controller driver was not aware of this protocol change and
kept on using the SPI 1-1-1 protocol.

So in the very first version of this series of patches, I inserted a call of
spi_nor_set_protocol(), a new function simply calling an optional hook,
between the nor->write_reg() and spi_nor_wait_till_ready() calls. This way
the (Q)SPI controller driver was now notified about the protocol switch and
can eventually adapt to this change.

After some discussions on the mailing list, it appeared that using such a
hook to handle protocol changes would required to insert calls of the new
spi_nor_set_protocol() function before all the calls of nor->read_reg(),
nor->write_reg(), nor->read(), nor->write() and nor->erase().
Indeed some manufacturers like Spansion use different numbers of I/O lines
depending on the type of operation:
- Fast Read: 1-1-4
- register read/write: 1-1-1

So the addition inside struct spi_nor of the new reg_proto, read_proto,
write_proto and erase_proto fields was prefered to the first
spi_nor_set_protocol() proposal. This fields are set once for all by
spi_nor_scan(). SPI controller drivers have to worry about their values only
if they claim to support Dual or Quad protocols through the 'mode' argument
of spi_nor_scan().

Then about the Micron case, I did test it and it didn't work without patching.
To be cautious, I wondered whether it might have work with a
different configuration/SPI controller because I don't want to break
something working. I wondered about a mean for the SPI controller driver to
detect the protocol change. Maybe parsing the SPI message and checking the
command op code. However it looked a highly inefficient method and also it
simply can not work since the very same op code, for instance 0x6B, is used
by both the 1-1-4 and 4-4-4 protocols.
So my conclusion was it could not have worked before: I don't have to worry
about breaking the support of Quad protocols for Micron memories.

I don't think there is a real need to revert Bean's commit since this series
fixes the issue anyway. However if you feel it would be better to revert it to
start from a cleaner base, I'm fine with it. Just let me know your choice so
I can adapt my series before sending the next version.


About the Macronix case, I still don't have any memory sample to test.
However, reading some memory datasheet (again and again), my understanding
has changed a little bit: setting the Quad Enable (QE) non-volatile bit in
the Status Register doesn't mean enabling the Quad Peripheral Interface (QPI)
as I thought.
Two dedicated op codes are used when sending the required commands to
enable/disable the QPI. Once the QPI enabled, the memory expects ALL commands
to use the SPI 4-4-4 protocol. Enabling the QPI requires the QE bit to bit set
first in the Status Register.
However the QE bit is also required to use the SPI 1-1-4 protocol: QPI must be
disabled in that case.
Setting the QE bit only disables the Write Protect and Hold features: the two
associated pins then become the IO2 and IO3 lines, needed by Quad SPI
protocols.

Finally for the Winbond case, I don't have memory from this manufacturer
yet so once again I refer only to some datasheets: it looks like Winbond
memories use a pattern of behaviour very closed to Macronix' one.
Indeed, some Quad Enable non volatile bit must be set inside the Status
Register. Also two dedicated op codes are used to enable/disable the QPI mode.
The QPI mode requires the QE bit to be set and, once enabled, all commands
must use the SPI 4-4-4 protocol.
However the two op codes to enter/leave the QPI mode are not the same as
those of Macronix and both manufacturers use different offsets for their
QE bits.

What I've planned to do to support Macronix and Winbond memory is to sill
use the spi_nor_read_id() function to guess the current protocol. If the
caller of spi_nor_scan() asks for Quad SPI support through the 'mode'
argument, I will use the SPI 4-4-4 protocol if the QPI mode is already
enabled, the SPI 1-1-4 protocol otherwise: I won't change the state of the
QPI mode. Indeed, if spi_nor_read_id() has succeeded in reading the JEDEC
ID, the current state of QPI mode is supported whatever it is.


>> 1 - Micron:
>> When their Quad SPI mode is enabled, Micron spi-nor memories expect all
>> commands to use the SPI 4-4-4 protocol. Also when the Dual SPI mode is
>> enabled, all commands must use the SPI 2-2-2 protocol.
>>
>> Before this patch, the spi-nor framework used to always enable the Quad
>> mode when the mode argument of spi_nor_scan() took the value SPI_NOR_QUAD.
>> That was not suited with drivers only supporting SPI 1-x-4 protocols but
>> not the 4-4-4 (e.g. the m25p80 driver). Also the SPI controller was not
>> notified about which SPI protocol to use to transfert command. We cannot
>> rely only on the op code: in Extended SPI mode the 0x6b command must use
>> the SPI 1-1-4 protocol whereas in Quad SPi mode the SPI 4-4-4 protocol
>> must be use instead.
>>
>> After this patch, the spi-nor framework uses the result of the
>> spi_nor_read_id() function to choose the right SPI protocol to be used.
>> If the reg_proto was set to SPI_PROTO_4_4_4, we already know that the Quad
>> SPI mode is already enabled and that the SPI controller supports the SPI
>> 4-4-4 protocol (otherwise it would have fail to read the JEDEC ID with the
>> 0xaf op code). For the very same reason, if the reg_proto was set to
>> SPI_PROTO_2_2_2, we already know that the Dual mode is already enabled and
>> that the SPI controller supports the SPI 2-2-2 protocol.
>> Otherwise we switch back to the Extended SPI protocol, which supports at
>> least the Fast Read commands:
>> - 1-1-1 (0x0b)
>> - Dual Output 1-1-2 (0x3b)
>> - Quad Output 1-1-4 (0x6b)
>>
>> We also safely set the number of dummy cycles to 8 for Fast Read commands
>> through the Volatile Configuration Register (VCR): some drivers (m25p80)
>> or SPI controllers only support a number of dummy cycles multiple of 8.
>> This number may have previouly been set to an unsupported value by an
>> early bootloader or at reset thanks to the Non-Volatile Configuration
>> Register.
> 
> It's not clear to me how you're being safe with the dummy cycles at all.
> It seems like you're introducing new values that may be incompatible
> with drivers. That can be OK, but you have to give drivers a chance to
> opt-out... Maybe some kind of "host capability" flags?
> 

Currently not all drivers support numbers of dummy cycles which are not
multiple of 8 bits. Especially this is the case of the m25p80 driver.
Other drivers, those supporting more numbers of dummy cycles still support
multiple of 8 bits. So now the spi-nor framework always  sets to 8 the number
of dummy cycles to be used by updating some Micron *volatile* register.
All drivers can deal with a value of 8. This value is almost always the
value used before. It is large enough to fit the highest SPI bus clock
frequencies but still provides good performances.
Actually, I'm pretty sure it was the only used value before.
And since the driver updates a volatile register, the configuration will
still be the same after reset for bootloaders.
In fact it should not change what worked before, it only gives a chance to
recover from bootloaders which would have used "strange" values of dummy
cycle number.
For instance, the romcode of the sama5d2 sets this number of dummy cycles to
10 for all Micron memories to limit the risk of incompatibility with memory
timing requirements. This romcode cannot be updated and tries to
support as many QSPI memories as possible within a limited code size: we
cannot afford using tables indexed by JEDEC ID but only by manufacturer ID.
However a value of 10 is not such a kind of value as expected by Linux
drivers. A value of 8 gives a chance to use the m25p80 driver to manufacturers
who don't want/can't develop a driver dedicated to their QSPI controller.

As a second thought, another solution is to only read the current value of dummy
cycle number and simply set use this value to initialize nor->read_dummy.
The assumption is now that is the value was fine for the QSPI controller during
the bootloader phase it's still fine under Linux as long as a driver can handle
this QSPI controller.

Please let me know which strategy you prefer for my next series.


>> Finally the XIP bit is always set in the VCR to disable the Continuous
>> Read mode as we don't want to care about mode cycles.
>>
>> 2 - Macronix:
>> When the QPI mode is enabled, all commands must use the SPI 4-4-4 protocol
>> and only the 0xeb op code is supported for Fast Read commands.
>> Before this patch, the spi-nor framework used to force the QPI mode but
>> used the 0x6b op code for Fast Read commands when the SPI controller
>> claims to support Quad SPI mode.
>> This patch uses the result of spi_nor_read_id() to guess whether the QPI
>> mode is both enable and supported by the SPI controller (otherwise it
>> would have failed to read the JEDEC ID with the 0xaf op code).
>> When the QPI mode is disabled, Macronix memories still support the
>> following Fast Read commands:
>> - 1-1-1 (0x0b)
>> - Dual Output 1-1-2 (0x3b)
>> - Quad Output 1-1-4 (0x6b)
>> So if the QPI mode has not already been enabled, there is not need to
>> enable it. We also avoid the 0xbb (Dual I/O 1-2-2) and 0xeb (Quad I/O
>> 1-4-4) op codes on purpose as we don't want to care about the value to set
>> in mode cycles not to enter the Continuous Read (Performance Enhance)
>> mode.
>>
>> As for Micron memories, the spi-nor framework now safely sets the number
>> of dummy cycles to 8 thanks to 2 volatile bits inside the Configuration
>> Register.
>>
>> 3 - Spansion:
>> As for Macronix, we avoid the 0xbb (Dual I/O 1-2-2) and 0xeb (Quad I/O
>> 1-4-4) op codes on purpose as we don't want to care about the value to set
>> in mode cycles not to enter in the Continuous Read mode.
>>
>> Besides, we only care about the Quad Enable bit inside the Configuration
>> Register (CR) when using Quad operations. In such a case, we first check
>> its state before trying to set it. Now we also notify the user about the
>> update of this non-volatile bit.
>>
>> We also check the Latency Code (LC) in CR to know the exact number of
>> dummy cycles to use when performing a Fast Read operation. Currently only
>> the 0x0b, 0x3b and 0x6b op codes are used to perform Fast Read operation
>> so the number of dummy cycles is always either 0 or 8. Hence no regression
>> should be introduced.
>>
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 783 +++++++++++++++++++++++++++++++++++++-----
>>  include/linux/mtd/spi-nor.h   |  15 +-
>>  2 files changed, 699 insertions(+), 99 deletions(-)
> 
> That's quite a lot to do in one patch, both in number of lines of code,
> and in number of tasks outlined in the commit description. Can this be
> broken down at all to be more modular and incremental?

I've already tried to split the series a bit more. I had troubles making
patches smaller without breaking functionnal dependencies but I'll try
to split it again before sending the next series.

> 
> Snipped the rest of the patch for now.
> 
> Brian
> 

Happy new year, all! :)

Best regards,

Cyrille

Re: [PATCH linux-next 1/5] mtd: spi-nor: properly detect the memory when it boots in Quad or Dual mode

[Cyrille Pitchen]

Hi Brian,

Le 18/12/2015 02:55, Brian Norris a écrit :
> Hi Cyrille,
> 
> On Mon, Dec 07, 2015 at 03:09:10PM +0100, Cyrille Pitchen wrote:
[...]
>> +
>> +			/* Set this protocol for all commands. */
>> +			nor->reg_proto = configs[i].proto;
>> +			nor->read_proto = configs[i].proto;
>> +			nor->write_proto = configs[i].proto;
>> +			nor->erase_proto = configs[i].proto;
> 
> Are these all fully independent? Do we really need 4 fields for this?
> 

Currently, for sure reg_proto and read_proto are independent. Let's take
Spansion memories as an example:
- Fast Read Quad Data 0x6B uses SPI 1-1-4
- register accesses (read/write) use SPI 1-1-1

AFAIK, Quad IO write commands are not used yet but if one day they are, for
instance with Macronix memories (QPI mode disabled):
- 4x I/O Page Program 0x38 uses SPI 1-1-4
- register accesses (read/write) uses SPI 1-1-1
- Fast Read Quad I/O 0xEB uses SPI 1-4-4
- Sector Erase 0x20 uses SPI 1-1-1

For now, I don't have any example where erase_proto is different from
reg_proto but for clarity reasons I'd rather keep erase_proto and reg_proto
distinct. Otherwise both field should be renamed as it looks odd to use
reg_proto when implementing the nor->erase() hook, doesn't it?

The names were chosen according to both the *_opcode and hooks from the
struct spi_nor:

hook           op code          protocol
read_reg()     N/A              reg_proto
write_reg()    N/A              reg_proto
read()         read_opcode      read_proto
write()        program_opcode   write_proto
erase()        erase_opcode     erase_proto

I admit following this logic 'program_opcode' should be renamed
'write_opcode'.



[...]
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>> index fac3f6f53981..c91986a99caf 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -75,8 +75,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 */
>> @@ -105,6 +106,16 @@ enum read_mode {
>>  	SPI_NOR_QUAD,
>>  };
>>  
>> +enum spi_protocol {
>> +	SPI_PROTO_1_1_1,	/* SPI */
>> +	SPI_PROTO_1_1_2,	/* Dual Output */
>> +	SPI_PROTO_1_1_4,	/* Quad Output */
>> +	SPI_PROTO_1_2_2,	/* Dual IO */
>> +	SPI_PROTO_1_4_4,	/* Quad IO */
>> +	SPI_PROTO_2_2_2,	/* Dual Command */
>> +	SPI_PROTO_4_4_4,	/* Quad Command */
> 
> Would it help at all to make this enum into something more like a
> bitfield? So in some cases, rather than a bit switch block, we can just
> extract the "number of lines" from the integer value? e.g.:
> 
> #define SNOR_PROTO(command, addr, data) \
> 	(((command) << 0) | \
> 	 ((addr) << 4) | \
> 	 ((data) << 8)) // or some other kind of macro magic
> 
> enum spi_nor_protocol {
> 	SNOR_PROTO_1_1_1		= SNOR_PROTO(1, 1, 1),
> 	SNOR_PROTO_1_1_2		= SNOR_PROTO(1, 1, 2),
> 	...
> };
> 
> static inline int spi_nor_io_lines_command(enum spi_nor_protocol proto)
> {
> 	return proto & 0xf;
> }
> 
> (Similar for addr and data phases. Also, my naming might suck. Feel free
> to improve!)
> 
> I don't think we should stomp on the SPI namespace with the
> "SPI_PROTO_*" definitions. That's why I chose SNOR_PROTO_ and spi_nor_
> prefixes.
> 

It looks good to me so I'll change for that :)

> Brian


Best regards,

Cyrille

RE: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller

[Bean Huo 霍斌斌 (beanhuo)]

Hi, Brian
Sorry for response this too later. Please see my comment below:
 
> Hi Bean,
> 
> On Tue, Dec 08, 2015 at 06:21:00AM +0000, Bean Huo 霍斌斌 wrote:
> > > -----Original Message-----
> > > From: Brian Norris [mailto:computersforpeace@gmail.com]
> > >
> > > I'll admit I'm a little fuzzy on the differences between dual and
> > > quad modes on various flash manufacturers. Can you help clear it up for
> me?
> >
> > For Micron SPI NOR spi quad mode, means that Qaud I/O prototocol, it
> > follows I/O Bus width is command-address-Data 4-4-4, at this time,
> > DQ0,DQ1,DQ2,DQ3 are all used to transfer address/command/data. For
> > this maybe not the same between different flash manufactures. For
> > example, for Spansion Qspi NOR, its all instructions are transferred
> > from host to memory as a single bit serial sequence on the DQ0 signal, even
> under Quad mode.  Dual mode the same as Qaud mode scenario.
> >
> > for SPI NOR 1-1-4, means command and address are transferred on the
> > DQ0, but for data, being transferred on DQ0,DQ1,DQ2,DQ3.For this, it
> > is the same between different flash manufacturers. Of course, at this
> > moment, SPI NOR should work under extended I/O mode.
> 
> OK, so to make these statements *much* shorter:
> 
>  * Micron "Quad Mode" means putting the flash in a 4/4/4 mode
	Yes.
>  * Spansion (and all others?) are using 1/1/4 modes
	Per my experience, Spansion are using 1/4/4 in quad mode,
	But Macronix is the same with Micron, also putting flash into 4/4/4 mode.

> 
> Correct?
> 
> > > I think some of the comments on patch 2 help too, but I'll just
> > > comment here for now.
> > > It looks like the current driver has problems regarding the non
> > > 1-x-y modes (e.g., 4-4-4), right? But I see that spi-nor.c never
> > > tries to send a 4_4_4 command; it only sets read_opcode to
> > > SPINOR_OP_READ_1_1_{1,2,4}. So is this an oversight in patches like
> Bean's patch?
> >
> > For SPINOR_OP_READ_1_1_{1,2,4} commands, Spi NOR actually works
> under
> > Extended I/O mode, not Quad mode. They just push Spi NOR output data
> > by Quad mode, Command and address still following extended I/O mode.
> 
> The naming is confusing enough here... so in your words, "extended"
> means 1/1/{1,2,4} (i.e., command, and maybe address, use 1 line, but data
> goes on 4)? And "quad" means 4/4/4?

Extended mode :
means it is the standard spi interface, command and 
Address use 1 line, and how many lines being used by data, it depends on
Specified command, for example, fast read command, uses 4 lines.


> > For 4-4-4 I/O protocol, SPI NOR should change to Quad mode(just as my
> > patch), of course, SPI controller should support this. for Micron Qspi
> > NOR, under quad mode, all commands/address/data are transferred on
> > DQ0,DQ1,DQ2,DQ3 signals. No matter what kind of command.
> 
> OK, so I think your patch is broken:
> 
> > >     commit 548cd3ab54da ("mtd: spi-nor: Add quad I/O support for
> Micron
> > >     SPI NOR")
> 
> How did you test this? Specifically, this can't possibly have worked with a
> regular drivers/spi/ controllers, since:
> 
>  (a) you're enabling 4/4/4 (i.e., "Quad mode") on the flash but
>  (b) m25p80_read() only sets .rx_bits for the data; i.e., it's using
>  1/1/4 (i.e., "Extended mode")

> I'm tempted to essentially revert that, as it looks essentially untested. It
> would be nice to have a cleaner baseline before trying to extend it with
> Cyrille's work.

Definitely ,for my patch , it is tested and verified OK. But I add a hoot function in m25p80.c
To put spi controller into quad mode as long as spi nor switch into quad mode.
For this hook function patch is an independent patch with my patch, I did not submit.
Because if we don't use m25p80.c driver, use new spi structure(such as : driver/mtd/spi-nor/fsl_quadspi.c)
spi controller driver, This hook function patch is not necessary.

> Cyrille, what do you think? Is my analysis at all correct here? (Sorry if this is
> addressed elsewhere; there's a lot of text in this conversation, but I'm getting
> hung up very early.) And if so, does it hurt to just drop Micron "Quad mode"
> (4/4/4)?
> (AIUI, this won't exactly be a panacea, since you mention bootloaders that
> start us off in quad mode, so we can't use single I/O 0x9f READ
> ID.)
> 
> > > Why would we even need to enable quad modes like that, if we're not
> > > going to send the 4-4-4 opcodes?
> > I think, in order to high speed SPI NOR, after enable quad mode,
> > SPINOR_OP_READ_1_1_{1,2,4} commands don't need any more, normal
> read
> > command (0x03) Can implement as them.
> 
> OK. That's odd, but I guess it doesn't matter much. It just makes it a little less
> obvious what's going on.
> 
> > > My next question (if my understanding is roughly correct) is, do we
> > > need the
> > > 4-4-4 modes, and what risks come with them? I understand we can
> > > shorten the command and address phases, but does that alone yield
> > > much performance benefit? And I think the risk is that a given
> > > system might not be prepared for the flash to be in a 4-4-4 mode, if
> > > the boot code tries to use 1-x-y commands.
> >
> > As far as my current experience and knowledge, this still need to be
> > enabled, especially for fast boot, and some IOT devices to store info into
> SPI NOR.
> 
> Do you have any data to about the speed? And you haven't addressed the
> risks. There are definitely risks. Cyrille looks like he's trying to address the
> risks (e.g., use volatile modes whenever possible), but it doesn't seem that
> you are.
> > For this patches, my current concern is that host side how to get
> > different I/O protocol changes, and distinguish between different flash
> manufacturers I/O mode.
> 
> Brian