[PATCH linux-next v2 00/14] mtd: spi-nor: add driver for Atmel QSPI controller

[PATCH linux-next v2 00/14] 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

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 QSPI + Spansion s25fl512 (atmel-quadspi.c driver)

SPI 1-1-4: default settings. Early developpements on a FPGA board.

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

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


Winbond and Macronix memories were NOT tested. The patches were written
only based on the manufacturers' datasheets.


Best regards,

Cyrille


ChangeLog:

v1 -> v2:
- I've totally reworked the whole series. Especially I have split the
  former patch 2 to ease the code review. The Single/Dual/Quad SPI mode
  support fixes are now done by manufacturer.
- fix support and probing of Winbond memories.
- m25p80: compute the number of dummy bytes from both the number of dummy
  clock cycles and the number of I/O lines used to send them.
- fix Kconfig dependencies for the Atmel QSPI controller driver (it should
  fix the kbuild test robot warnings).
- add more comments.


Cyrille Pitchen (14):
  mtd: spi-nor: remove micron_quad_enable()
  mtd: spi-nor: properly detect the memory when it boots in Quad or Dual
    mode
  mtd: spi-nor: select op codes and SPI NOR protocols by manufacturer
  mtd: spi-nor: fix support of Macronix memories
  mtd: spi-nor: fix support of Winbond memories
  mtd: spi-nor: fix support of Micron memories
  mtd: spi-nor: fix support of Spansion memories
  mtd: spi-nor: configure the number of dummy clock cycles by
    manufacturer
  mtd: spi-nor: configure the number of dummy clock cycles on Micron
    memories
  mtd: spi-nor: configure the number of dummy clock cycles on Macronix
    memories
  mtd: spi-nor: configure the number of dummy clock cycles on Spansion
    memories
  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                       | 192 ++++-
 drivers/mtd/spi-nor/Kconfig                        |   9 +
 drivers/mtd/spi-nor/Makefile                       |   1 +
 drivers/mtd/spi-nor/atmel-quadspi.c                | 877 +++++++++++++++++++
 drivers/mtd/spi-nor/spi-nor.c                      | 928 +++++++++++++++++++--
 include/linux/mtd/spi-nor.h                        |  71 +-
 7 files changed, 1985 insertions(+), 125 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 v2 01/14] mtd: spi-nor: remove micron_quad_enable()

[Cyrille Pitchen]

This patch remove the micron_quad_enable() function which force the Quad
SPI mode. However, once this mode is enabled, the Micron memory expect ALL
commands to use the SPI 4-4-4 protocol. Hence a failure does occur when
calling spi_nor_wait_till_ready() right after the update of the Enhanced
Volatile Configuration Register (EVCR) in the micron_quad_enable() as
the SPI controller driver is not aware about the protocol change.

Since there is almost no performance increase using Fast Read 4-4-4
commands instead of Fast Read 1-1-4 commands, we rather keep on using the
Extended SPI mode than enabling the Quad SPI mode.

Let's take the example of the pretty standard use of 8 dummy cycles during
Fast Read operations on 64KB erase sectors:

Fast Read 1-1-4 requires 8 cycles for the command, then 24 cycles for the
3byte address followed by 8 dummy clock cycles and finally 65536*2 cycles
for the read data; so 131112 clock cycles.

On the other hand the Fast Read 4-4-4 would require 2 cycles for the
command, then 6 cycles for the 3byte address followed by 8 dummy clock
cycles and finally 65536*2 cycles for the read data. So 131088 clock
cycles. The theorical bandwidth increase is 0.0%.

Now using Fast Read operations on 512byte pages:
Fast Read 1-1-4 needs 8+24+8+(512*2) = 1064 clock cycles whereas Fast
Read 4-4-4 would requires 2+6+8+(512*2) = 1040 clock cycles. Hence the
theorical bandwidth increase is 2.3%.
Consecutive reads for non sequential pages is not a relevant use case so
The Quad SPI mode is not worth it.

mtd_speedtest seems to confirm these figures.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Fixes: 548cd3ab54da ("mtd: spi-nor: Add quad I/O support for Micron SPI NOR")
---
 drivers/mtd/spi-nor/spi-nor.c | 46 +------------------------------------------
 1 file changed, 1 insertion(+), 45 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index ed0c19c558b5..3028c06547c1 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1100,45 +1100,6 @@ static int spansion_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
-static int micron_quad_enable(struct spi_nor *nor)
-{
-	int ret;
-	u8 val;
-
-	ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
-	if (ret < 0) {
-		dev_err(nor->dev, "error %d reading EVCR\n", ret);
-		return ret;
-	}
-
-	write_enable(nor);
-
-	/* 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);
-	if (ret < 0) {
-		dev_err(nor->dev, "error while writing EVCR register\n");
-		return ret;
-	}
-
-	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, &val, 1);
-	if (ret < 0) {
-		dev_err(nor->dev, "error %d reading EVCR\n", ret);
-		return ret;
-	}
-	if (val & EVCR_QUAD_EN_MICRON) {
-		dev_err(nor->dev, "Micron EVCR Quad bit not clear\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
 {
 	int status;
@@ -1152,12 +1113,7 @@ static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
 		}
 		return status;
 	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 0;
 	default:
 		status = spansion_quad_enable(nor);
 		if (status) {
-- 
1.8.2.2

[PATCH linux-next v2 02/14] 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.

Winbond memories, once their Quad mode enabled, expect ALL commands to use
the SPI 4-4-4 protocol. Unlike Micron and Macronix memories, they still
reply to the regular JEDEC Read ID (0x9f) command.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 3028c06547c1..8967319ea7da 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -73,6 +73,12 @@ struct flash_info {
 
 #define JEDEC_MFR(info)	((info)->id[0])
 
+struct read_id_config {
+	enum read_mode		mode;
+	enum spi_nor_protocol	proto;
+	u8			opcode;
+};
+
 static const struct flash_info *spi_nor_match_id(const char *name);
 
 /*
@@ -879,11 +885,22 @@ 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[] = {
+		/* Winbond QPI mode */
+		{SPI_NOR_QUAD, SNOR_PROTO_4_4_4, SPINOR_OP_RDID},
+
+		/* Micron Quad mode & Macronix QPI mode */
+		{SPI_NOR_QUAD, SNOR_PROTO_4_4_4, SPINOR_OP_MIO_RDID},
+
+		/* Micron Dual mode */
+		{SPI_NOR_DUAL, SNOR_PROTO_2_2_2, SPINOR_OP_MIO_RDID}
+	};
 
 	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
 	if (tmp < 0) {
@@ -891,6 +908,58 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
 		return ERR_PTR(tmp);
 	}
 
+	/*
+	 * Check whether the SPI NOR memory has already been configured (at
+	 * reset or by some bootloader) to use a protocol other than SPI 1-1-1.
+	 */
+	for (i = 0; i < ARRAY_SIZE(configs); ++i) {
+		int len = SPI_NOR_MAX_ID_LEN;
+		bool is_multi = false;
+
+		/*
+		 * Check the latest read Manufacturer ID + Device ID (3 bytes):
+		 * if they are different from both 0x000000 and 0xffffff, we
+		 * assume that we succeeded in reading a valid JEDEC ID so we
+		 * don't need to try other SPI protocols.
+		 * Indeed when either the protocol or the op code are not valid,
+		 * the SPI NOR memory should not reply to the command. Hence the
+		 * SPI I/O lines remain in their default state: 1 when connected
+		 * to pull-up resistors or 0 with pull-down.
+		 */
+		if (!((id[0] == 0xff && id[1] == 0xff && id[2] == 0xff) ||
+		      (id[0] == 0x00 && id[1] == 0x00 && id[2] == 0x00)))
+			break;
+
+		/* Only try protocols supported by the user. */
+		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.
+		 */
+		if (configs[i].opcode == SPINOR_OP_MIO_RDID) {
+			is_multi = true;
+			len = 3;
+			memset(id + len, 0, sizeof(id) - len);
+		}
+
+		tmp = nor->read_reg(nor, configs[i].opcode, id, len);
+		if (tmp < 0) {
+			dev_dbg(nor->dev,
+				"error %d reading JEDEC ID%s\n",
+				tmp, (is_multi ? " Multi I/O" : ""));
+			return ERR_PTR(tmp);
+		}
+	}
+
 	for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
 		info = &spi_nor_ids[tmp];
 		if (info->id_len) {
@@ -1148,11 +1217,17 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	if (ret)
 		return ret;
 
+	/* Reset SPI protocol for all commands */
+	nor->erase_proto = SNOR_PROTO_1_1_1;
+	nor->read_proto = SNOR_PROTO_1_1_1;
+	nor->write_proto = SNOR_PROTO_1_1_1;
+	nor->reg_proto = SNOR_PROTO_1_1_1;
+
 	if (name)
 		info = spi_nor_match_id(name);
 	/* Try to auto-detect if chip name wasn't specified or not found */
 	if (!info)
-		info = spi_nor_read_id(nor);
+		info = spi_nor_read_id(nor, mode);
 	if (IS_ERR_OR_NULL(info))
 		return -ENOENT;
 
@@ -1163,7 +1238,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 62356d50815b..53932c87bcf2 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,43 @@ enum read_mode {
 	SPI_NOR_QUAD,
 };
 
+
+#define SNOR_PROTO_CMD_OFF	8
+#define SNOR_PROTO_CMD_MASK	GENMASK(11, 8)
+#define SNOR_PROTO_CMD_TO_PROTO(cmd) \
+	(((cmd) << SNOR_PROTO_CMD_OFF) & SNOR_PROTO_CMD_MASK)
+#define SNOR_PROTO_CMD_FROM_PROTO(proto) \
+	((((u32)(proto)) & SNOR_PROTO_CMD_MASK) >> SNOR_PROTO_CMD_OFF)
+
+#define SNOR_PROTO_ADDR_OFF	4
+#define SNOR_PROTO_ADDR_MASK	GENMASK(7, 4)
+#define SNOR_PROTO_ADDR_TO_PROTO(addr) \
+	(((addr) << SNOR_PROTO_ADDR_OFF) & SNOR_PROTO_ADDR_MASK)
+#define SNOR_PROTO_ADDR_FROM_PROTO(proto) \
+	((((u32)(proto)) & SNOR_PROTO_ADDR_MASK) >> SNOR_PROTO_ADDR_OFF)
+
+#define SNOR_PROTO_DATA_OFF	0
+#define SNOR_PROTO_DATA_MASK	GENMASK(3, 0)
+#define SNOR_PROTO_DATA_TO_PROTO(data) \
+	(((data) << SNOR_PROTO_DATA_OFF) & SNOR_PROTO_DATA_MASK)
+#define SNOR_PROTO_DATA_FROM_PROTO(proto) \
+	((((u32)(proto)) & SNOR_PROTO_DATA_MASK) >> SNOR_PROTO_DATA_OFF)
+
+#define SNOR_PROTO(cmd, addr, data)	  \
+	(SNOR_PROTO_CMD_TO_PROTO(cmd) |   \
+	 SNOR_PROTO_ADDR_TO_PROTO(addr) | \
+	 SNOR_PROTO_DATA_TO_PROTO(data))
+
+enum spi_nor_protocol {
+	SNOR_PROTO_1_1_1 = SNOR_PROTO(1, 1, 1),	/* SPI */
+	SNOR_PROTO_1_1_2 = SNOR_PROTO(1, 1, 2),	/* Dual Output */
+	SNOR_PROTO_1_1_4 = SNOR_PROTO(1, 1, 4),	/* Quad Output */
+	SNOR_PROTO_1_2_2 = SNOR_PROTO(1, 2, 2),	/* Dual IO */
+	SNOR_PROTO_1_4_4 = SNOR_PROTO(1, 4, 4),	/* Quad IO */
+	SNOR_PROTO_2_2_2 = SNOR_PROTO(2, 2, 2),	/* Dual Command */
+	SNOR_PROTO_4_4_4 = SNOR_PROTO(4, 4, 4),	/* Quad Command */
+};
+
 #define SPI_NOR_MAX_CMD_SIZE	8
 enum spi_nor_ops {
 	SPI_NOR_OPS_READ = 0,
@@ -132,6 +170,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 +202,10 @@ struct spi_nor {
 	u8			read_opcode;
 	u8			read_dummy;
 	u8			program_opcode;
+	enum spi_nor_protocol	erase_proto;
+	enum spi_nor_protocol	read_proto;
+	enum spi_nor_protocol	write_proto;
+	enum spi_nor_protocol	reg_proto;
 	enum read_mode		flash_read;
 	bool			sst_write_second;
 	u32			flags;
-- 
1.8.2.2

[PATCH linux-next v2 03/14] mtd: spi-nor: select op codes and SPI NOR protocols by manufacturer

[Cyrille Pitchen]

This is a transitional patch to prepare the split by Manufacturer of the
support of Single/Dual/Quad SPI modes.

Indeed every QSPI NOR manufacturer (Spansion, Micron, Macronix, Winbond)
supports Dual or Quad SPI modes on its way. Especially the Fast Read op
code and the SPI NOR protocols to use are not quite the same depending on
the manufacturer.

For instance Quad commands can use either the SPI 1-1-4, 1-4-4 or 4-4-4
protocol.

This is why this patch will be completed by fixes for each manufacturer.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 8967319ea7da..8793cebbe5a9 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1180,17 +1180,61 @@ static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
 			dev_err(nor->dev, "Macronix quad-read not enabled\n");
 			return -EINVAL;
 		}
-		return status;
+		/* Check whether Macronix QPI mode is enabled. */
+		if (nor->read_proto != SNOR_PROTO_4_4_4)
+			nor->read_proto = SNOR_PROTO_1_1_4;
+		break;
+
 	case SNOR_MFR_MICRON:
-		return 0;
-	default:
+		/* Check whether Micron Quad mode is enabled. */
+		if (nor->read_proto != SNOR_PROTO_4_4_4)
+			nor->read_proto = SNOR_PROTO_1_1_4;
+		break;
+
+	case SNOR_MFR_SPANSION:
 		status = spansion_quad_enable(nor);
 		if (status) {
 			dev_err(nor->dev, "Spansion quad-read not enabled\n");
 			return -EINVAL;
 		}
-		return status;
+		nor->read_proto = SNOR_PROTO_1_1_4;
+		break;
+
+	default:
+		return -EINVAL;
 	}
+
+	nor->read_opcode = SPINOR_OP_READ_1_1_4;
+	return 0;
+}
+
+static int set_dual_mode(struct spi_nor *nor, const struct flash_info *info)
+{
+	switch (JEDEC_MFR(info)) {
+	case SNOR_MFR_MICRON:
+		/* Check whether Micron Dual mode is enabled. */
+		if (nor->read_proto != SNOR_PROTO_2_2_2)
+			nor->read_proto = SNOR_PROTO_1_1_2;
+		break;
+
+	default:
+		nor->read_proto = SNOR_PROTO_1_1_2;
+		break;
+	}
+
+	nor->read_opcode = SPINOR_OP_READ_1_1_2;
+	return 0;
+}
+
+static int set_single_mode(struct spi_nor *nor, const struct flash_info *info)
+{
+	switch (JEDEC_MFR(info)) {
+	default:
+		nor->read_proto = SNOR_PROTO_1_1_1;
+		break;
+	}
+
+	return 0;
 }
 
 static int spi_nor_check(struct spi_nor *nor)
@@ -1338,7 +1382,30 @@ 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 */
+	if (nor->flash_read == SPI_NOR_NORMAL)
+		nor->read_opcode = SPINOR_OP_READ;
+	else
+		nor->read_opcode = SPINOR_OP_READ_FAST;
+
+	nor->program_opcode = SPINOR_OP_PP;
+
+	/*
+	 * Quad/Dual-read mode takes precedence over fast/normal.
+	 *
+	 * Manufacturer specific modes are discovered when reading the JEDEC ID
+	 * and are reported by the nor->read_proto value:
+	 *  - SNOR_PROTO_4_4_4 is either:
+	 *    + Micron Quad mode enabled
+	 *    + Macronix/Winbond QPI mode enabled
+	 *  - SNOR_PROTO_2_2_2 is either:
+	 *    + Micron Dual mode enabled
+	 *
+	 * The opcodes and the protocols 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) {
@@ -1347,30 +1414,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) {
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 53932c87bcf2..89e3228ec1d0 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) */
 
-- 
1.8.2.2

[PATCH linux-next v2 04/14] mtd: spi-nor: fix support of Macronix memories

[Cyrille Pitchen]

This patch fixes the support of Macronix memories. Especially we avoid
updating the Status Register when not needed as the Quad Enable (QE) bit
is a non-volatile bit.

Also we add comments to explain why we use some Fast Read op codes rather
than others.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 8793cebbe5a9..042ac49d6188 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1115,6 +1115,11 @@ static int macronix_quad_enable(struct spi_nor *nor)
 	val = read_sr(nor);
 	if (val < 0)
 		return val;
+
+	if (likely(val & SR_QUAD_EN_MX))
+		return 0;
+	dev_warn(nor->dev, "Macronix Quad mode disabled, enable it\n");
+
 	write_enable(nor);
 
 	write_sr(nor, val | SR_QUAD_EN_MX);
@@ -1169,21 +1174,73 @@ static int spansion_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
+static int macronix_set_quad_mode(struct spi_nor *nor)
+{
+	int status;
+
+	/* Check whether the QPI mode is enabled. */
+	if (nor->read_proto == SNOR_PROTO_4_4_4) {
+		/*
+		 * Since the QPI mode is enabled, the Quad Enabled (QE)
+		 * non-volatile bit is already set.
+		 * However in QPI mode, only the Fast Read 1-4-4 (0xeb)
+		 * op code is supported.
+		 * WARNING: we should take care about the performance
+		 * enhance toggling bits P0-P7 written during the
+		 * dummy/mode cycles to avoid entering the continuous
+		 * read (performance enhance) mode by mistake!
+		 */
+		nor->read_opcode = SPINOR_OP_READ_1_4_4;
+		return 0;
+	}
+
+	/*
+	 * The QPI mode is disabled but we still need to set the QE bit:
+	 * this disables the reset and write protect features and
+	 * frees the associated pins so they can be used as the 3rd
+	 * and 4th I/O lines required by Quad SPI commands.
+	 * Also we'd rather use the Fast Read 1-1-4 (0x6b) op code than
+	 * the Fast Read 1-4-4 (0xeb) op code so we don't care about
+	 * entering the continuous read mode by mistake if some
+	 * performance enhance toggling bits P0-P7 were written during
+	 * dummy/mode cycles.
+	 */
+	status = macronix_quad_enable(nor);
+	if (status) {
+		dev_err(nor->dev, "Macronix quad-read not enabled\n");
+		return status;
+	}
+	nor->read_proto = SNOR_PROTO_1_1_4;
+	nor->read_opcode = SPINOR_OP_READ_1_1_4;
+	return 0;
+}
+
+/*
+ * For both Macronix Dual and Single modes, we don't care about the value of
+ * the Quad Enabled (QE) bit since the memory still replies to Dual or Single
+ * SPI commands.
+ */
+
+static int macronix_set_dual_mode(struct spi_nor *nor)
+{
+	nor->read_proto = SNOR_PROTO_1_1_2;
+	nor->read_opcode = SPINOR_OP_READ_1_1_2;
+	return 0;
+}
+
+static int macronix_set_single_mode(struct spi_nor *nor)
+{
+	nor->read_proto = SNOR_PROTO_1_1_1;
+	return 0;
+}
+
 static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
 {
 	int status;
 
 	switch (JEDEC_MFR(info)) {
 	case SNOR_MFR_MACRONIX:
-		status = macronix_quad_enable(nor);
-		if (status) {
-			dev_err(nor->dev, "Macronix quad-read not enabled\n");
-			return -EINVAL;
-		}
-		/* Check whether Macronix QPI mode is enabled. */
-		if (nor->read_proto != SNOR_PROTO_4_4_4)
-			nor->read_proto = SNOR_PROTO_1_1_4;
-		break;
+		return macronix_set_quad_mode(nor);
 
 	case SNOR_MFR_MICRON:
 		/* Check whether Micron Quad mode is enabled. */
@@ -1211,6 +1268,9 @@ static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
 static int set_dual_mode(struct spi_nor *nor, const struct flash_info *info)
 {
 	switch (JEDEC_MFR(info)) {
+	case SNOR_MFR_MACRONIX:
+		return macronix_set_dual_mode(nor);
+
 	case SNOR_MFR_MICRON:
 		/* Check whether Micron Dual mode is enabled. */
 		if (nor->read_proto != SNOR_PROTO_2_2_2)
@@ -1229,6 +1289,9 @@ static int set_dual_mode(struct spi_nor *nor, const struct flash_info *info)
 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);
+
 	default:
 		nor->read_proto = SNOR_PROTO_1_1_1;
 		break;
-- 
1.8.2.2

[PATCH linux-next v2 05/14] mtd: spi-nor: fix support of Winbond memories

[Cyrille Pitchen]

This patch fixes the support of Winbond memories. Indeed, before
performing any Quad SPI command, the Quad Enable (QE) non-volatile bit
MUST be set in the Status Register 2.

According to the w25q16fw datasheet from Winbond:
"When QE=1, the /WP pin becomes IO2 and /HOLD pin becomes IO3."

Quad SPI instructions requires the bidirectional IO2 and IO3 pins.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 042ac49d6188..f8a4887ca6eb 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1174,6 +1174,40 @@ static int spansion_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
+static int winbond_quad_enable(struct spi_nor *nor)
+{
+	int ret;
+	u8 sr2;
+
+	ret = nor->read_reg(nor, SPINOR_OP_RDSR2_WINB, &sr2, 1);
+	if (ret < 0)
+		return ret;
+
+	if (likely(sr2 & SR2_QUAD_EN_WINB))
+		return 0;
+	dev_warn(nor->dev, "Winbond Quad mode disabled, enable it\n");
+
+	write_enable(nor);
+
+	sr2 |= SR2_QUAD_EN_WINB;
+	ret = nor->write_reg(nor, SPINOR_OP_WRSR2_WINB, &sr2, 1);
+	if (ret < 0)
+		return ret;
+
+	if (spi_nor_wait_till_ready(nor))
+		return -EIO;
+
+	ret = nor->read_reg(nor, SPINOR_OP_RDSR2_WINB, &sr2, 1);
+	if (ret < 0)
+		return ret;
+	if (!(sr2 & SR2_QUAD_EN_WINB)) {
+		dev_err(nor->dev, "Winbond Quad bit not set\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int macronix_set_quad_mode(struct spi_nor *nor)
 {
 	int status;
@@ -1234,6 +1268,63 @@ static int macronix_set_single_mode(struct spi_nor *nor)
 	return 0;
 }
 
+static int winbond_set_quad_mode(struct spi_nor *nor)
+{
+	int status;
+
+	/* Check whether the QPI mode is enabled. */
+	if (nor->read_proto == SNOR_PROTO_4_4_4) {
+		/* Since the QPI mode is enabled, the Quad Enabled (QE)
+		 * non-volatile bit is already set.
+		 * If the Fast Read 1-4-4 (0xeb) were used, we should
+		 * take care about the value M7-M0 written during
+		 * dummy/mode cycles to avoid entering the continuous
+		 * read mode by  mistake.
+		 * Also the Fast Read 1-1-4 (0x6b) op code is not
+		 * supported in QPI mode.
+		 * Hence the Fast Read 1-1-1 (0x0b) op code is chosen.
+		 */
+		nor->read_opcode = SPINOR_OP_READ_FAST;
+		return 0;
+	}
+
+	/*
+	 * The QPI mode is disabled but we still need to set the QE bit:
+	 * when QE=1, the /WP pin becomes IO2 and /HOLD pin becomes IO3.
+	 * If the Fast Read 1-4-4 (0xeb) were used, we should take care
+	 * about the value M7-M0 written during dummy/mode cycles to
+	 * avoid entering the continuous read mode by mistake.
+	 * Hence the Fast Read 1-1-4 (0x6b) op code is preferred.
+	 */
+	status = winbond_quad_enable(nor);
+	if (status) {
+		dev_err(nor->dev, "Winbond quad-read nor enabled\n");
+		return status;
+	}
+	nor->read_proto = SNOR_PROTO_1_1_4;
+	nor->read_opcode = SPINOR_OP_READ_1_1_4;
+	return 0;
+}
+
+/*
+ * For both Winbond Dual and Single modes, we don't care about the value of
+ * the Quad Enabled (QE) bit since the memory still replies to Dual or Single
+ * SPI commands.
+ */
+
+static int winbond_set_dual_mode(struct spi_nor *nor)
+{
+	nor->read_proto = SNOR_PROTO_1_1_2;
+	nor->read_opcode = SPINOR_OP_READ_1_1_2;
+	return 0;
+}
+
+static int winbond_set_single_mode(struct spi_nor *nor)
+{
+	nor->read_proto = SNOR_PROTO_1_1_1;
+	return 0;
+}
+
 static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
 {
 	int status;
@@ -1242,6 +1333,9 @@ static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
 	case SNOR_MFR_MACRONIX:
 		return macronix_set_quad_mode(nor);
 
+	case SNOR_MFR_WINBOND:
+		return winbond_set_quad_mode(nor);
+
 	case SNOR_MFR_MICRON:
 		/* Check whether Micron Quad mode is enabled. */
 		if (nor->read_proto != SNOR_PROTO_4_4_4)
@@ -1271,6 +1365,9 @@ static int set_dual_mode(struct spi_nor *nor, const struct flash_info *info)
 	case SNOR_MFR_MACRONIX:
 		return macronix_set_dual_mode(nor);
 
+	case SNOR_MFR_WINBOND:
+		return winbond_set_dual_mode(nor);
+
 	case SNOR_MFR_MICRON:
 		/* Check whether Micron Dual mode is enabled. */
 		if (nor->read_proto != SNOR_PROTO_2_2_2)
@@ -1292,6 +1389,9 @@ static int set_single_mode(struct spi_nor *nor, const struct flash_info *info)
 	case SNOR_MFR_MACRONIX:
 		return macronix_set_single_mode(nor);
 
+	case SNOR_MFR_WINBOND:
+		return winbond_set_single_mode(nor);
+
 	default:
 		nor->read_proto = SNOR_PROTO_1_1_1;
 		break;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 89e3228ec1d0..46343f5a8162 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -75,6 +75,12 @@
 #define SPINOR_OP_EN4B		0xb7	/* Enter 4-byte mode */
 #define SPINOR_OP_EX4B		0xe9	/* Exit 4-byte mode */
 
+/* Used for Winbond flashes only. */
+#define SPINOR_OP_RDSR2_WINB	0x35	/* Read status register 2 */
+#define SPINOR_OP_WRSR2_WINB	0x31	/* Write status register 2 */
+
+#define SR2_QUAD_EN_WINB	BIT(1)	/* Quad Enable bit */
+
 /* Used for Spansion flashes only. */
 #define SPINOR_OP_BRWR		0x17	/* Bank register write */
 
-- 
1.8.2.2

[PATCH linux-next v2 06/14] mtd: spi-nor: fix support of Micron memories

[Cyrille Pitchen]

This patch adds missing mode transitions. Indeed depending on both the
current memory mode and the new protocol wanted by the user, we may need
to perform a switch back to the Extended SPI mode.

However when the current mode is the Quad mode and the user has asked for
a Quad SPI protocol, we'd rather stay in Quad mode and use the Fast Read
4-4-4 command than switch to the Extended SPI mode and use the Fast Read
1-1-4 command.

Also we'd rather stay in Dual mode than swith to the Extended SPI mode
whenever the user has asked for Dual SPI protocol.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index f8a4887ca6eb..db76af852198 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1325,6 +1325,147 @@ static int winbond_set_single_mode(struct spi_nor *nor)
 	return 0;
 }
 
+static int micron_set_protocol(struct spi_nor *nor, u8 mask, u8 val,
+			       enum spi_nor_protocol proto)
+{
+	u8 evcr;
+	int ret;
+
+	/* Read the Enhanced Volatile Configuration Register (EVCR). */
+	ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &evcr, 1);
+	if (ret < 0) {
+		dev_err(nor->dev, "error while reading EVCR register\n");
+		return ret;
+	}
+
+	/* Check whether we need to update the protocol bits. */
+	if ((evcr & mask) == val)
+		return 0;
+
+	/* Set EVCR protocol bits. */
+	write_enable(nor);
+	evcr = (evcr & ~mask) | val;
+	ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, &evcr, 1);
+	if (ret < 0) {
+		dev_err(nor->dev, "error while writing EVCR register\n");
+		return ret;
+	}
+
+	/* Switch reg protocol now before accessing any other registers. */
+	nor->reg_proto = proto;
+
+	ret = spi_nor_wait_till_ready(nor);
+	if (ret)
+		return ret;
+
+	/* Read EVCR and check it. */
+	ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &evcr, 1);
+	if (ret < 0 || (evcr & mask) != val) {
+		dev_err(nor->dev, "Micron EVCR protocol bits not updated\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int micron_set_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,
+				  SNOR_PROTO_1_1_1);
+	if (ret) {
+		dev_err(nor->dev, "Failed to set Micron Extended SPI mode\n");
+		return ret;
+	}
+
+	nor->write_proto = SNOR_PROTO_1_1_1;
+	nor->erase_proto = SNOR_PROTO_1_1_1;
+	return 0;
+}
+
+static int micron_set_quad_mode(struct spi_nor *nor)
+{
+	/* Check whether the Dual SPI mode is enabled. */
+	if (unlikely(nor->read_proto == SNOR_PROTO_2_2_2)) {
+		int ret;
+
+		/*
+		 * Exit Micron Dual mode and switch to the Extended SPI mode:
+		 * we can change the mode safely as we write into a volatile
+		 * register.
+		 * Also the Quad mode is not worth it for MTD usages: it
+		 * should only be relevant for eXecution In Place (XIP) usages,
+		 * which are out of the scope of the spi-nor framework.
+		 */
+		ret = micron_set_extended_spi_protocol(nor);
+		if (ret)
+			return ret;
+	}
+
+	/*
+	 * Whatever the Quad mode is enabled or not, the
+	 * Fast Read Quad Output 1-1-4 (0x6b) op code is supported.
+	 */
+	if (nor->read_proto != SNOR_PROTO_4_4_4)
+		nor->read_proto = SNOR_PROTO_1_1_4;
+	nor->read_opcode = SPINOR_OP_READ_1_1_4;
+	return 0;
+}
+
+static int micron_set_dual_mode(struct spi_nor *nor)
+{
+	/* Check whether Quad mode is enabled. */
+	if (unlikely(nor->read_proto == SNOR_PROTO_4_4_4)) {
+		int ret;
+
+		/*
+		 * Exit Micron Quad mode and switch to the Extended SPI mode:
+		 * we can change the mode safely as we write into a volatile
+		 * register.
+		 * Also the Dual mode is not worth it for MTD usages: it
+		 * should only be relevant for eXecution In Place (XIP) usages,
+		 * which are out of the scope of the spi-nor framework.
+		 */
+		ret = micron_set_extended_spi_protocol(nor);
+		if (ret)
+			return ret;
+	}
+
+	/*
+	 * Whatever the Dual mode is enabled or not, the
+	 * Fast Read Dual Output 1-1-2 (0x3b) op code is supported.
+	 */
+	if (nor->read_proto != SNOR_PROTO_2_2_2)
+		nor->read_proto = SNOR_PROTO_1_1_2;
+	nor->read_opcode = SPINOR_OP_READ_1_1_2;
+	return 0;
+}
+
+static int micron_set_single_mode(struct spi_nor *nor)
+{
+	/* Check whether either the Dual or Quad mode is enabled. */
+	if (unlikely(nor->read_proto != SNOR_PROTO_1_1_1)) {
+		int ret;
+
+		/*
+		 * Exit Micron Dual or Quad mode and switch to the Extended SPI
+		 * mode: we can change the mode safely as we write into a
+		 * volatile register.
+		 */
+		ret = micron_set_extended_spi_protocol(nor);
+		if (ret)
+			return ret;
+
+		nor->read_proto = SNOR_PROTO_1_1_1;
+	}
+
+	return 0;
+}
+
 static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
 {
 	int status;
@@ -1337,10 +1478,7 @@ static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
 		return winbond_set_quad_mode(nor);
 
 	case SNOR_MFR_MICRON:
-		/* Check whether Micron Quad mode is enabled. */
-		if (nor->read_proto != SNOR_PROTO_4_4_4)
-			nor->read_proto = SNOR_PROTO_1_1_4;
-		break;
+		return micron_set_quad_mode(nor);
 
 	case SNOR_MFR_SPANSION:
 		status = spansion_quad_enable(nor);
@@ -1369,10 +1507,7 @@ static int set_dual_mode(struct spi_nor *nor, const struct flash_info *info)
 		return winbond_set_dual_mode(nor);
 
 	case SNOR_MFR_MICRON:
-		/* Check whether Micron Dual mode is enabled. */
-		if (nor->read_proto != SNOR_PROTO_2_2_2)
-			nor->read_proto = SNOR_PROTO_1_1_2;
-		break;
+		return micron_set_dual_mode(nor);
 
 	default:
 		nor->read_proto = SNOR_PROTO_1_1_2;
@@ -1392,6 +1527,9 @@ static int set_single_mode(struct spi_nor *nor, const struct flash_info *info)
 	case SNOR_MFR_WINBOND:
 		return winbond_set_single_mode(nor);
 
+	case SNOR_MFR_MICRON:
+		return micron_set_single_mode(nor);
+
 	default:
 		nor->read_proto = SNOR_PROTO_1_1_1;
 		break;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 46343f5a8162..d0a6f343a063 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -102,6 +102,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 v2 07/14] mtd: spi-nor: fix support of Spansion memories

[Cyrille Pitchen]

This patch is only a transitional one. It concludes the series of patches
to select op codes and protocols by manufacturer.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index db76af852198..067425c7a0ff 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1466,10 +1466,35 @@ static int micron_set_single_mode(struct spi_nor *nor)
 	return 0;
 }
 
-static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
+static int spansion_set_quad_mode(struct spi_nor *nor)
 {
 	int status;
 
+	status = spansion_quad_enable(nor);
+	if (status) {
+		dev_err(nor->dev, "Spansion quad-read not enabled\n");
+		return -EINVAL;
+	}
+	nor->read_proto = SNOR_PROTO_1_1_4;
+	nor->read_opcode = SPINOR_OP_READ_1_1_4;
+	return 0;
+}
+
+static int spansion_set_dual_mode(struct spi_nor *nor)
+{
+	nor->read_proto = SNOR_PROTO_1_1_2;
+	nor->read_opcode = SPINOR_OP_READ_1_1_2;
+	return 0;
+}
+
+static int spansion_set_single_mode(struct spi_nor *nor)
+{
+	nor->read_proto = SNOR_PROTO_1_1_1;
+	return 0;
+}
+
+static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
+{
 	switch (JEDEC_MFR(info)) {
 	case SNOR_MFR_MACRONIX:
 		return macronix_set_quad_mode(nor);
@@ -1481,20 +1506,13 @@ static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
 		return micron_set_quad_mode(nor);
 
 	case SNOR_MFR_SPANSION:
-		status = spansion_quad_enable(nor);
-		if (status) {
-			dev_err(nor->dev, "Spansion quad-read not enabled\n");
-			return -EINVAL;
-		}
-		nor->read_proto = SNOR_PROTO_1_1_4;
-		break;
+		return spansion_set_quad_mode(nor);
 
 	default:
-		return -EINVAL;
+		break;
 	}
 
-	nor->read_opcode = SPINOR_OP_READ_1_1_4;
-	return 0;
+	return -EINVAL;
 }
 
 static int set_dual_mode(struct spi_nor *nor, const struct flash_info *info)
@@ -1509,13 +1527,14 @@ static int set_dual_mode(struct spi_nor *nor, const struct flash_info *info)
 	case SNOR_MFR_MICRON:
 		return micron_set_dual_mode(nor);
 
+	case SNOR_MFR_SPANSION:
+		return spansion_set_dual_mode(nor);
+
 	default:
-		nor->read_proto = SNOR_PROTO_1_1_2;
 		break;
 	}
 
-	nor->read_opcode = SPINOR_OP_READ_1_1_2;
-	return 0;
+	return -EINVAL;
 }
 
 static int set_single_mode(struct spi_nor *nor, const struct flash_info *info)
@@ -1530,12 +1549,14 @@ static int set_single_mode(struct spi_nor *nor, const struct flash_info *info)
 	case SNOR_MFR_MICRON:
 		return micron_set_single_mode(nor);
 
+	case SNOR_MFR_SPANSION:
+		return spansion_set_single_mode(nor);
+
 	default:
-		nor->read_proto = SNOR_PROTO_1_1_1;
 		break;
 	}
 
-	return 0;
+	return -EINVAL;
 }
 
 static int spi_nor_check(struct spi_nor *nor)
-- 
1.8.2.2

[PATCH linux-next v2 08/14] mtd: spi-nor: configure the number of dummy clock cycles by manufacturer

[Cyrille Pitchen]

This is a transitional patch which let us set the number of dummy clock
cycles by manufacturer.

More patches will follow by manufacturer to actually configure the
relevant number of dummy clock cycles following the dedicated procedure.

For instance, some manufacturers like Spansion configure the number of
dummy clock cycles to be used by Fast Read command through some
non-volatile register. In such a case, we should avoid updating its value
but instead read it then set the nor->read_dummy accordingly.

On the other hand, some manufacturers like Micron use some volatile
register. In this case, we'd rather update this register to use a number
of dummy clock cycles, which is a multiple of 8.
Indeed some drivers, like m25p80, only support writing bytes, hence
multiples of 8 bits.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 067425c7a0ff..353a0f6ac3fe 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -139,24 +139,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.
  */
@@ -1225,6 +1207,7 @@ static int macronix_set_quad_mode(struct spi_nor *nor)
 		 * read (performance enhance) mode by mistake!
 		 */
 		nor->read_opcode = SPINOR_OP_READ_1_4_4;
+		nor->read_dummy = 8;
 		return 0;
 	}
 
@@ -1246,6 +1229,7 @@ static int macronix_set_quad_mode(struct spi_nor *nor)
 	}
 	nor->read_proto = SNOR_PROTO_1_1_4;
 	nor->read_opcode = SPINOR_OP_READ_1_1_4;
+	nor->read_dummy = 8;
 	return 0;
 }
 
@@ -1259,12 +1243,27 @@ static int macronix_set_dual_mode(struct spi_nor *nor)
 {
 	nor->read_proto = SNOR_PROTO_1_1_2;
 	nor->read_opcode = SPINOR_OP_READ_1_1_2;
+	nor->read_dummy = 8;
 	return 0;
 }
 
 static int macronix_set_single_mode(struct spi_nor *nor)
 {
+	u8 read_dummy;
+
+	switch (nor->read_opcode) {
+	case SPINOR_OP_READ:
+	case SPINOR_OP_READ4:
+		read_dummy = 0;
+		break;
+
+	default:
+		read_dummy = 8;
+		break;
+	}
+
 	nor->read_proto = SNOR_PROTO_1_1_1;
+	nor->read_dummy = read_dummy;
 	return 0;
 }
 
@@ -1285,6 +1284,7 @@ static int winbond_set_quad_mode(struct spi_nor *nor)
 		 * Hence the Fast Read 1-1-1 (0x0b) op code is chosen.
 		 */
 		nor->read_opcode = SPINOR_OP_READ_FAST;
+		nor->read_dummy = 8;
 		return 0;
 	}
 
@@ -1303,6 +1303,7 @@ static int winbond_set_quad_mode(struct spi_nor *nor)
 	}
 	nor->read_proto = SNOR_PROTO_1_1_4;
 	nor->read_opcode = SPINOR_OP_READ_1_1_4;
+	nor->read_dummy = 8;
 	return 0;
 }
 
@@ -1316,12 +1317,27 @@ static int winbond_set_dual_mode(struct spi_nor *nor)
 {
 	nor->read_proto = SNOR_PROTO_1_1_2;
 	nor->read_opcode = SPINOR_OP_READ_1_1_2;
+	nor->read_dummy = 8;
 	return 0;
 }
 
 static int winbond_set_single_mode(struct spi_nor *nor)
 {
+	u8 read_dummy;
+
+	switch (nor->read_opcode) {
+	case SPINOR_OP_READ:
+	case SPINOR_OP_READ4:
+		read_dummy = 0;
+		break;
+
+	default:
+		read_dummy = 8;
+		break;
+	}
+
 	nor->read_proto = SNOR_PROTO_1_1_1;
+	nor->read_dummy = read_dummy;
 	return 0;
 }
 
@@ -1413,6 +1429,7 @@ static int micron_set_quad_mode(struct spi_nor *nor)
 	if (nor->read_proto != SNOR_PROTO_4_4_4)
 		nor->read_proto = SNOR_PROTO_1_1_4;
 	nor->read_opcode = SPINOR_OP_READ_1_1_4;
+	nor->read_dummy = 8;
 	return 0;
 }
 
@@ -1442,11 +1459,14 @@ static int micron_set_dual_mode(struct spi_nor *nor)
 	if (nor->read_proto != SNOR_PROTO_2_2_2)
 		nor->read_proto = SNOR_PROTO_1_1_2;
 	nor->read_opcode = SPINOR_OP_READ_1_1_2;
+	nor->read_dummy = 8;
 	return 0;
 }
 
 static int micron_set_single_mode(struct spi_nor *nor)
 {
+	u8 read_dummy;
+
 	/* Check whether either the Dual or Quad mode is enabled. */
 	if (unlikely(nor->read_proto != SNOR_PROTO_1_1_1)) {
 		int ret;
@@ -1463,6 +1483,18 @@ static int micron_set_single_mode(struct spi_nor *nor)
 		nor->read_proto = SNOR_PROTO_1_1_1;
 	}
 
+	/* Force the number of dummy cycles to 8 for Fast Read, 0 for Read. */
+	switch (nor->read_opcode) {
+	case SPINOR_OP_READ:
+	case SPINOR_OP_READ4:
+		read_dummy = 0;
+		break;
+
+	default:
+		read_dummy = 8;
+		break;
+	}
+	nor->read_dummy = read_dummy;
 	return 0;
 }
 
@@ -1477,6 +1509,7 @@ static int spansion_set_quad_mode(struct spi_nor *nor)
 	}
 	nor->read_proto = SNOR_PROTO_1_1_4;
 	nor->read_opcode = SPINOR_OP_READ_1_1_4;
+	nor->read_dummy = 8;
 	return 0;
 }
 
@@ -1484,12 +1517,27 @@ static int spansion_set_dual_mode(struct spi_nor *nor)
 {
 	nor->read_proto = SNOR_PROTO_1_1_2;
 	nor->read_opcode = SPINOR_OP_READ_1_1_2;
+	nor->read_dummy = 8;
 	return 0;
 }
 
 static int spansion_set_single_mode(struct spi_nor *nor)
 {
+	u8 read_dummy;
+
+	switch (nor->read_opcode) {
+	case SPINOR_OP_READ:
+	case SPINOR_OP_READ4:
+		read_dummy = 0;
+		break;
+
+	default:
+		read_dummy = 8;
+		break;
+	}
+
 	nor->read_proto = SNOR_PROTO_1_1_1;
+	nor->read_dummy = read_dummy;
 	return 0;
 }
 
@@ -1704,11 +1752,14 @@ 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;
 
-	/* Default commands */
-	if (nor->flash_read == SPI_NOR_NORMAL)
+	/* Default commands and number of dummy cycles */
+	if (nor->flash_read == SPI_NOR_NORMAL) {
 		nor->read_opcode = SPINOR_OP_READ;
-	else
+		nor->read_dummy = 0;
+	} else {
 		nor->read_opcode = SPINOR_OP_READ_FAST;
+		nor->read_dummy = 8;
+	}
 
 	nor->program_opcode = SPINOR_OP_PP;
 
@@ -1723,8 +1774,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	 *  - SNOR_PROTO_2_2_2 is either:
 	 *    + Micron Dual mode enabled
 	 *
-	 * The opcodes and the protocols are updated depending on the
-	 * manufacturer.
+	 * 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.
 	 */
@@ -1788,8 +1839,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);
 
-- 
1.8.2.2

[PATCH linux-next v2 09/14] mtd: spi-nor: configure the number of dummy clock cycles on Micron memories

[Cyrille Pitchen]

The spi-nor framework currently expects all Fast Read operations to use 8
dummy clock cycles. Especially some drivers like m25p80 can only support
multiple of 8 dummy clock cycles.

On Micron memories, the number of dummy clock cycles to be used by Fast
Read commands can be safely set to 8 by updating the Volatile
Configuration Register (VCR).

Also the XIP bit is set at the same time when updating the VCR so the
Continuous Read mode is disabled: this prevents us from entering it by
mistake.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 353a0f6ac3fe..3f79619aea52 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1341,6 +1341,53 @@ static int winbond_set_single_mode(struct spi_nor *nor)
 	return 0;
 }
 
+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 while reading VCR register\n");
+		return ret;
+	}
+
+	/* Check whether we need to update the number of dummy cycles. */
+	if ((vcr & mask) == val) {
+		nor->read_dummy = read_dummy;
+		return 0;
+	}
+
+	/* 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 VCR register\n");
+		return ret;
+	}
+
+	ret = spi_nor_wait_till_ready(nor);
+	if (ret)
+		return ret;
+
+	/* 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_nor_protocol proto)
 {
@@ -1425,12 +1472,15 @@ static int micron_set_quad_mode(struct spi_nor *nor)
 	/*
 	 * Whatever the Quad mode is enabled or not, the
 	 * Fast Read Quad Output 1-1-4 (0x6b) op code is supported.
+	 * 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.
 	 */
 	if (nor->read_proto != SNOR_PROTO_4_4_4)
 		nor->read_proto = SNOR_PROTO_1_1_4;
 	nor->read_opcode = SPINOR_OP_READ_1_1_4;
-	nor->read_dummy = 8;
-	return 0;
+	return micron_set_dummy_cycles(nor, 8);
 }
 
 static int micron_set_dual_mode(struct spi_nor *nor)
@@ -1455,12 +1505,15 @@ static int micron_set_dual_mode(struct spi_nor *nor)
 	/*
 	 * Whatever the Dual mode is enabled or not, the
 	 * Fast Read Dual Output 1-1-2 (0x3b) op code is supported.
+	 * 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.
 	 */
 	if (nor->read_proto != SNOR_PROTO_2_2_2)
 		nor->read_proto = SNOR_PROTO_1_1_2;
 	nor->read_opcode = SPINOR_OP_READ_1_1_2;
-	nor->read_dummy = 8;
-	return 0;
+	return micron_set_dummy_cycles(nor, 8);
 }
 
 static int micron_set_single_mode(struct spi_nor *nor)
@@ -1483,7 +1536,13 @@ static int micron_set_single_mode(struct spi_nor *nor)
 		nor->read_proto = SNOR_PROTO_1_1_1;
 	}
 
-	/* Force the number of dummy cycles to 8 for Fast Read, 0 for Read. */
+	/*
+	 * 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:
@@ -1494,8 +1553,7 @@ static int micron_set_single_mode(struct spi_nor *nor)
 		read_dummy = 8;
 		break;
 	}
-	nor->read_dummy = read_dummy;
-	return 0;
+	return micron_set_dummy_cycles(nor, read_dummy);
 }
 
 static int spansion_set_quad_mode(struct spi_nor *nor)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index d0a6f343a063..2dc0f8b429ca 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -86,6 +86,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 */
 
-- 
1.8.2.2

[PATCH linux-next v2 10/14] mtd: spi-nor: configure the number of dummy clock cycles on Macronix memories

[Cyrille Pitchen]

The spi-nor framework currently expects all Fast Read operations to use 8
dummy clock cycles. Especially some drivers like m25p80 can only support
multiple of 8 dummy clock cycles.

On Macronix memories, the number of dummy clock cycles to be used by Fast
Read commands can be safely set to 8 by updating the DC0 and DC1 volatile
bits inside the Configuration Register.

According to the mx66l1g45g datasheet from Macronix, using 8 dummy clock
cycles should be enough to set the SPI bus clock frequency up to:
- 133 MHz for Fast Read 1-1-1, 1-1-2, 1-1-4 and 1-2-2 commands in Single
  Transfer Rate (STR)
- 104 MHz for Fast Read 1-4-4 (or 4-4-4 in QPI mode) commands (STR)

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 3f79619aea52..68abae5c72e9 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1190,6 +1190,136 @@ static int winbond_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
+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;
+
+	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;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int macronix_set_dummy_cycles(struct spi_nor *nor, u8 read_dummy)
+{
+	int ret, sr, cr, mask, val;
+	u16 sr_cr;
+	u8 dc;
+
+	/* 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 writing 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)
 {
 	int status;
@@ -1207,8 +1337,7 @@ static int macronix_set_quad_mode(struct spi_nor *nor)
 		 * read (performance enhance) mode by mistake!
 		 */
 		nor->read_opcode = SPINOR_OP_READ_1_4_4;
-		nor->read_dummy = 8;
-		return 0;
+		return macronix_set_dummy_cycles(nor, 8);
 	}
 
 	/*
@@ -1221,6 +1350,9 @@ static int macronix_set_quad_mode(struct spi_nor *nor)
 	 * entering the continuous read mode by mistake if some
 	 * performance enhance toggling bits P0-P7 were written during
 	 * dummy/mode cycles.
+	 *
+	 * Use the Fast Read Quad Output 1-1-4 (0x6b) command with 8 dummy
+	 * cycles (up to 133MHz for STR and 66MHz for DTR).
 	 */
 	status = macronix_quad_enable(nor);
 	if (status) {
@@ -1229,8 +1361,7 @@ static int macronix_set_quad_mode(struct spi_nor *nor)
 	}
 	nor->read_proto = SNOR_PROTO_1_1_4;
 	nor->read_opcode = SPINOR_OP_READ_1_1_4;
-	nor->read_dummy = 8;
-	return 0;
+	return macronix_set_dummy_cycles(nor, 8);
 }
 
 /*
@@ -1241,16 +1372,25 @@ static int macronix_set_quad_mode(struct spi_nor *nor)
 
 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 = SNOR_PROTO_1_1_2;
 	nor->read_opcode = SPINOR_OP_READ_1_1_2;
-	nor->read_dummy = 8;
-	return 0;
+	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:
@@ -1263,8 +1403,7 @@ static int macronix_set_single_mode(struct spi_nor *nor)
 	}
 
 	nor->read_proto = SNOR_PROTO_1_1_1;
-	nor->read_dummy = read_dummy;
-	return 0;
+	return macronix_set_dummy_cycles(nor, read_dummy);
 }
 
 static int winbond_set_quad_mode(struct spi_nor *nor)
-- 
1.8.2.2

[PATCH linux-next v2 11/14] mtd: spi-nor: configure the number of dummy clock cycles on Spansion memories

[Cyrille Pitchen]

On Spansion memories, the number of dummy clock cycles to be used during
Fast Read commands is configured through the 2bit latency code (LC). These
bits are non-volatile inside the Configuration Register.

To avoid breaking the configuration expected at reset by some bootloaders,
we'd rather read the latency code and set the nor->read_dummy value
accordingly than update those non-volatile bits.

Since the Quad Enable non-volatile bit can be read at the same time from
the Control Register, we now check its value to avoid some calls of the
spansion_quad_enable() function when they are not needed.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 68abae5c72e9..9ee271b2e5cb 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1695,47 +1695,162 @@ static int micron_set_single_mode(struct spi_nor *nor)
 	return micron_set_dummy_cycles(nor, read_dummy);
 }
 
-static int spansion_set_quad_mode(struct spi_nor *nor)
+static inline int spansion_get_config(struct spi_nor *nor,
+				      bool *quad_enabled,
+				      u8 *latency_code)
 {
-	int status;
+	int cr;
 
-	status = spansion_quad_enable(nor);
-	if (status) {
-		dev_err(nor->dev, "Spansion quad-read not enabled\n");
+	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_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;
+
+	/*
+	 * 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 = SNOR_PROTO_1_1_4;
 	nor->read_opcode = SPINOR_OP_READ_1_1_4;
-	nor->read_dummy = 8;
-	return 0;
+	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 = SNOR_PROTO_1_1_2;
 	nor->read_opcode = SPINOR_OP_READ_1_1_2;
-	nor->read_dummy = 8;
-	return 0;
+	return spansion_set_dummy_cycles(nor, latency_code);
 }
 
 static int spansion_set_single_mode(struct spi_nor *nor)
 {
-	u8 read_dummy;
-
-	switch (nor->read_opcode) {
-	case SPINOR_OP_READ:
-	case SPINOR_OP_READ4:
-		read_dummy = 0;
-		break;
+	u8 latency_code;
+	int ret;
 
-	default:
-		read_dummy = 8;
-		break;
-	}
+	/* We don't care about the quad mode status */
+	ret = spansion_get_config(nor, NULL, &latency_code);
+	if (ret)
+		return ret;
 
 	nor->read_proto = SNOR_PROTO_1_1_1;
-	nor->read_dummy = read_dummy;
-	return 0;
+	return spansion_set_dummy_cycles(nor, latency_code);
 }
 
 static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
-- 
1.8.2.2

[PATCH linux-next v2 12/14] 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 | 192 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 151 insertions(+), 41 deletions(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index c9c3b7fa3051..a57c7d0e3f39 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -27,22 +27,64 @@
 #include <linux/spi/flash.h>
 #include <linux/mtd/spi-nor.h>
 
-#define	MAX_CMD_SIZE		6
+#define	MAX_CMD_SIZE		16
 struct m25p {
 	struct spi_device	*spi;
 	struct spi_nor		spi_nor;
 	u8			command[MAX_CMD_SIZE];
 };
 
+static inline int m25p80_proto2nbits(enum spi_nor_protocol proto,
+				     unsigned *code_nbits,
+				     unsigned *addr_nbits,
+				     unsigned *data_nbits)
+{
+	if (code_nbits)
+		*code_nbits = SNOR_PROTO_CMD_FROM_PROTO(proto);
+	if (addr_nbits)
+		*addr_nbits = SNOR_PROTO_ADDR_FROM_PROTO(proto);
+	if (data_nbits)
+		*data_nbits = SNOR_PROTO_DATA_FROM_PROTO(proto);
+
+	return 0;
+}
+
 static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
 {
 	struct m25p *flash = nor->priv;
 	struct spi_device *spi = flash->spi;
+	unsigned code_nbits, data_nbits;
+	struct spi_transfer xfers[2];
 	int ret;
 
-	ret = spi_write_then_read(spi, &code, 1, val, len);
+	/* Check the total length of command op code and data. */
+	if (len + 1 > MAX_CMD_SIZE)
+		return -EINVAL;
+
+	/* Get transfer protocols (addr_nbits is not relevant here). */
+	ret = m25p80_proto2nbits(nor->reg_proto,
+				 &code_nbits, NULL, &data_nbits);
+	if (ret < 0)
+		return ret;
+
+	/* Set up transfers. */
+	memset(xfers, 0, sizeof(xfers));
+
+	flash->command[0] = code;
+	xfers[0].len = 1;
+	xfers[0].tx_buf = flash->command;
+	xfers[0].tx_nbits = code_nbits;
+
+	xfers[1].len = len;
+	xfers[1].rx_buf = &flash->command[1];
+	xfers[1].rx_nbits = data_nbits;
+
+	/* Process command. */
+	ret = spi_sync_transfer(spi, xfers, 2);
 	if (ret < 0)
 		dev_err(&spi->dev, "error %d reading %x\n", ret, code);
+	else
+		memcpy(val, &flash->command[1], len);
 
 	return ret;
 }
@@ -65,12 +107,42 @@ static int m25p80_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 {
 	struct m25p *flash = nor->priv;
 	struct spi_device *spi = flash->spi;
+	unsigned code_nbits, data_nbits, num_xfers = 1;
+	struct spi_transfer xfers[2];
+	int ret;
+
+	/* Check the total length of command op code and data. */
+	if (buf && (len + 1 > MAX_CMD_SIZE))
+		return -EINVAL;
+
+	/* Get transfer protocols (addr_nbits is not relevant here). */
+	ret = m25p80_proto2nbits(nor->reg_proto,
+				 &code_nbits, NULL, &data_nbits);
+	if (ret < 0)
+		return ret;
+
+	/* Set up transfer(s). */
+	memset(xfers, 0, sizeof(xfers));
 
 	flash->command[0] = opcode;
-	if (buf)
+	xfers[0].len = 1;
+	xfers[0].tx_buf = flash->command;
+	xfers[0].tx_nbits = code_nbits;
+
+	if (buf) {
 		memcpy(&flash->command[1], buf, len);
+		if (data_nbits == code_nbits) {
+			xfers[0].len += len;
+		} else {
+			xfers[1].len = len;
+			xfers[1].tx_buf = &flash->command[1];
+			xfers[1].tx_nbits = data_nbits;
+			num_xfers++;
+		}
+	}
 
-	return spi_write(spi, flash->command, len + 1);
+	/* Process command. */
+	return spi_sync_transfer(spi, xfers, num_xfers);
 }
 
 static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
@@ -78,43 +150,54 @@ static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
 {
 	struct m25p *flash = nor->priv;
 	struct spi_device *spi = flash->spi;
-	struct spi_transfer t[2] = {};
+	unsigned code_nbits, addr_nbits, data_nbits, num_xfers = 1;
+	struct spi_transfer xfers[3];
 	struct spi_message m;
-	int cmd_sz = m25p_cmdsz(nor);
-
-	spi_message_init(&m);
+	int ret, cmd_sz = m25p_cmdsz(nor);
 
 	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
 		cmd_sz = 1;
 
-	flash->command[0] = nor->program_opcode;
-	m25p_addr2cmd(nor, to, flash->command);
+	/* Get transfer protocols. */
+	ret = m25p80_proto2nbits(nor->write_proto,
+				 &code_nbits, &addr_nbits, &data_nbits);
+	if (ret < 0) {
+		*retlen = 0;
+		return;
+	}
 
-	t[0].tx_buf = flash->command;
-	t[0].len = cmd_sz;
-	spi_message_add_tail(&t[0], &m);
+	/* Set up transfers. */
+	memset(xfers, 0, sizeof(xfers));
+
+	flash->command[0] = nor->program_opcode;
+	xfers[0].len = 1;
+	xfers[0].tx_buf = flash->command;
+	xfers[0].tx_nbits = code_nbits;
+
+	if (cmd_sz > 1) {
+		m25p_addr2cmd(nor, to, flash->command);
+		if (addr_nbits == code_nbits) {
+			xfers[0].len += nor->addr_width;
+		} else {
+			xfers[1].len = nor->addr_width;
+			xfers[1].tx_buf = &flash->command[1];
+			xfers[1].tx_nbits = addr_nbits;
+			num_xfers++;
+		}
+	}
 
-	t[1].tx_buf = buf;
-	t[1].len = len;
-	spi_message_add_tail(&t[1], &m);
+	xfers[num_xfers].len = len;
+	xfers[num_xfers].tx_buf = buf;
+	xfers[num_xfers].tx_nbits = data_nbits;
+	num_xfers++;
 
+	/* Process command. */
+	spi_message_init_with_transfers(&m, xfers, num_xfers);
 	spi_sync(spi, &m);
 
 	*retlen += m.actual_length - cmd_sz;
 }
 
-static inline unsigned int m25p80_rx_nbits(struct spi_nor *nor)
-{
-	switch (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 +207,55 @@ static int m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
 {
 	struct m25p *flash = nor->priv;
 	struct spi_device *spi = flash->spi;
-	struct spi_transfer t[2];
-	struct spi_message m;
+	unsigned code_nbits, addr_nbits, data_nbits, num_xfers = 1;
 	unsigned int dummy = nor->read_dummy;
+	struct spi_transfer xfers[3];
+	struct spi_message m;
+	int ret;
+
+	/* Get transfer protocols. */
+	ret = m25p80_proto2nbits(nor->read_proto,
+				 &code_nbits, &addr_nbits, &data_nbits);
+	if (ret < 0) {
+		*retlen = 0;
+		return ret;
+	}
 
 	/* convert the dummy cycles to the number of bytes */
-	dummy /= 8;
+	dummy = (dummy * addr_nbits) / 8;
 
-	spi_message_init(&m);
-	memset(t, 0, (sizeof t));
+	/* Set up transfers. */
+	memset(xfers, 0, sizeof(xfers));
 
 	flash->command[0] = nor->read_opcode;
-	m25p_addr2cmd(nor, from, flash->command);
+	xfers[0].len = 1;
+	xfers[0].tx_buf = flash->command;
+	xfers[0].tx_nbits = code_nbits;
 
-	t[0].tx_buf = flash->command;
-	t[0].len = m25p_cmdsz(nor) + dummy;
-	spi_message_add_tail(&t[0], &m);
+	m25p_addr2cmd(nor, from, flash->command);
+	/*
+	 * Clear all dummy/mode cycle bits to avoid sending some manufacturer
+	 * specific pattern, which might make the memory enter its Continuous
+	 * Read mode by mistake.
+	 */
+	memset(flash->command + 1 + nor->addr_width, 0, dummy);
+
+	if (addr_nbits == code_nbits) {
+		xfers[0].len += nor->addr_width + dummy;
+	} else {
+		xfers[1].len = nor->addr_width + dummy;
+		xfers[1].tx_buf = &flash->command[1];
+		xfers[1].tx_nbits = addr_nbits;
+		num_xfers++;
+	}
 
-	t[1].rx_buf = buf;
-	t[1].rx_nbits = m25p80_rx_nbits(nor);
-	t[1].len = len;
-	spi_message_add_tail(&t[1], &m);
+	xfers[num_xfers].len = len;
+	xfers[num_xfers].rx_buf = buf;
+	xfers[num_xfers].rx_nbits = data_nbits;
+	num_xfers++;
 
+	/* Process command. */
+	spi_message_init_with_transfers(&m, xfers, num_xfers);
 	spi_sync(spi, &m);
 
 	*retlen = m.actual_length - m25p_cmdsz(nor) - dummy;
-- 
1.8.2.2

[PATCH linux-next v2 13/14] 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>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 .../devicetree/bindings/mtd/atmel-quadspi.txt      | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/atmel-quadspi.txt

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

[PATCH linux-next v2 14/14] 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         |   9 +
 drivers/mtd/spi-nor/Makefile        |   1 +
 drivers/mtd/spi-nor/atmel-quadspi.c | 877 ++++++++++++++++++++++++++++++++++++
 3 files changed, 887 insertions(+)
 create mode 100644 drivers/mtd/spi-nor/atmel-quadspi.c

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 0dc927540b3d..db768d3c5bf9 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -37,6 +37,15 @@ config SPI_FSL_QUADSPI
 	  This controller does not support generic SPI. It only supports
 	  SPI NOR.
 
+config SPI_ATMEL_QUADSPI
+	tristate "Atmel Quad SPI Controller"
+	depends on ARCH_AT91 || (ARM && COMPILE_TEST)
+	depends on OF && HAS_DMA && HAS_IOMEM
+	help
+	  This enables support for the Quad SPI controller in master mode.
+	  This driver does not support generic SPI. The implementation only
+	  supports SPI NOR.
+
 config SPI_NXP_SPIFI
 	tristate "NXP SPI Flash Interface (SPIFI)"
 	depends on OF && (ARCH_LPC18XX || COMPILE_TEST)
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 0bf3a7f81675..1525913698ad 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_MTD_SPI_NOR)	+= spi-nor.o
 obj-$(CONFIG_SPI_FSL_QUADSPI)	+= fsl-quadspi.o
 obj-$(CONFIG_MTD_MT81xx_NOR)    += mtk-quadspi.o
 obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
+obj-$(CONFIG_SPI_ATMEL_QUADSPI)	+= atmel-quadspi.o
diff --git a/drivers/mtd/spi-nor/atmel-quadspi.c b/drivers/mtd/spi-nor/atmel-quadspi.c
new file mode 100644
index 000000000000..3a500d92e413
--- /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_nor_protocol proto)
+{
+	cmd->ifr = ifr_tfrtyp;
+
+	switch (proto) {
+	case SNOR_PROTO_1_1_1:
+		cmd->ifr |= QSPI_IFR_WIDTH_SINGLE_BIT_SPI;
+		break;
+	case SNOR_PROTO_1_1_2:
+		cmd->ifr |= QSPI_IFR_WIDTH_DUAL_OUTPUT;
+		break;
+	case SNOR_PROTO_1_1_4:
+		cmd->ifr |= QSPI_IFR_WIDTH_QUAD_OUTPUT;
+		break;
+	case SNOR_PROTO_1_2_2:
+		cmd->ifr |= QSPI_IFR_WIDTH_DUAL_IO;
+		break;
+	case SNOR_PROTO_1_4_4:
+		cmd->ifr |= QSPI_IFR_WIDTH_QUAD_IO;
+		break;
+	case SNOR_PROTO_2_2_2:
+		cmd->ifr |= QSPI_IFR_WIDTH_DUAL_CMD;
+		break;
+	case SNOR_PROTO_4_4_4:
+		cmd->ifr |= QSPI_IFR_WIDTH_QUAD_CMD;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int atmel_qspi_read_reg(struct spi_nor *nor, u8 opcode,
+			       u8 *buf, int len)
+{
+	struct atmel_qspi *aq = nor->priv;
+	struct atmel_qspi_command cmd;
+	int ret;
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.enable.bits.instruction = 1;
+	cmd.enable.bits.data = 1;
+	cmd.instruction = opcode;
+	cmd.rx_buf = buf;
+	cmd.buf_len = len;
+
+	ret = atmel_qspi_command_set_ifr(&cmd,
+					 QSPI_IFR_TFRTYP_TRSFR_READ,
+					 nor->reg_proto);
+	if (ret)
+		return ret;
+
+	return atmel_qspi_run_command(aq, &cmd);
+}
+
+static int atmel_qspi_write_reg(struct spi_nor *nor, u8 opcode,
+				u8 *buf, int len)
+{
+	struct atmel_qspi *aq = nor->priv;
+	struct atmel_qspi_command cmd;
+	int ret;
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.enable.bits.instruction = 1;
+	cmd.enable.bits.data = (buf != NULL && len > 0);
+	cmd.instruction = opcode;
+	cmd.tx_buf = buf;
+	cmd.buf_len = len;
+
+	ret = atmel_qspi_command_set_ifr(&cmd,
+					 QSPI_IFR_TFRTYP_TRSFR_WRITE,
+					 nor->reg_proto);
+	if (ret)
+		return ret;
+
+	return atmel_qspi_run_command(aq, &cmd);
+}
+
+static void atmel_qspi_write(struct spi_nor *nor, loff_t to, size_t len,
+			     size_t *retlen, const u_char *write_buf)
+{
+	struct atmel_qspi *aq = nor->priv;
+	struct atmel_qspi_command cmd;
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.enable.bits.instruction = 1;
+	cmd.enable.bits.address = nor->addr_width;
+	cmd.enable.bits.data = 1;
+	cmd.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_NBBITS(8) | QSPI_MR_SSM;
+	qspi_writel(aq, QSPI_MR, mr);
+
+	src_rate = clk_get_rate(aq->clk);
+	if (!src_rate)
+		return -EINVAL;
+
+	/* Compute the QSPI baudrate */
+	scbr = DIV_ROUND_UP(src_rate, aq->clk_rate);
+	if (scbr > 0)
+		scbr--;
+	scr = QSPI_SCR_SCBR(scbr);
+	qspi_writel(aq, QSPI_SCR, scr);
+
+	/* Enable the QSPI controller */
+	qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIEN);
+
+	return 0;
+}
+
+static irqreturn_t atmel_qspi_interrupt(int irq, void *dev_id)
+{
+	struct atmel_qspi *aq = (struct atmel_qspi *)dev_id;
+	u32 status, mask, pending;
+
+	status = qspi_readl(aq, QSPI_SR);
+	mask = qspi_readl(aq, QSPI_IMR);
+	pending = status & mask;
+
+	if (!pending)
+		return IRQ_NONE;
+
+	aq->pending |= pending;
+	if (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->mem);
+		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 v2 02/14] mtd: spi-nor: properly detect the memory when it boots in Quad or Dual mode

[Boris Brezillon]

Hi Cyrille,

Sorry for this pretty useless review, but I think you know more about
SPI-NOR than I do. So, consider my comments as nitpicks, which shouldn't
prevent your series from being applied.

On Fri, 8 Jan 2016 17:02:14 +0100
Cyrille Pitchen <cyrille.pitchen@atmel.com> 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.
> 
> Winbond memories, once their Quad mode enabled, expect ALL commands to use
> the SPI 4-4-4 protocol. Unlike Micron and Macronix memories, they still
> reply to the regular JEDEC Read ID (0x9f) command.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 83 ++++++++++++++++++++++++++++++++++++++++---
>  include/linux/mtd/spi-nor.h   | 50 ++++++++++++++++++++++++--
>  2 files changed, 127 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 3028c06547c1..8967319ea7da 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -73,6 +73,12 @@ struct flash_info {
>  
>  #define JEDEC_MFR(info)	((info)->id[0])
>  
> +struct read_id_config {
> +	enum read_mode		mode;
> +	enum spi_nor_protocol	proto;
> +	u8			opcode;
> +};
> +
>  static const struct flash_info *spi_nor_match_id(const char *name);
>  
>  /*
> @@ -879,11 +885,22 @@ 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[] = {
> +		/* Winbond QPI mode */
> +		{SPI_NOR_QUAD, SNOR_PROTO_4_4_4, SPINOR_OP_RDID},
> +
> +		/* Micron Quad mode & Macronix QPI mode */
> +		{SPI_NOR_QUAD, SNOR_PROTO_4_4_4, SPINOR_OP_MIO_RDID},
> +
> +		/* Micron Dual mode */
> +		{SPI_NOR_DUAL, SNOR_PROTO_2_2_2, SPINOR_OP_MIO_RDID}
> +	};
>  
>  	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
>  	if (tmp < 0) {
> @@ -891,6 +908,58 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
>  		return ERR_PTR(tmp);
>  	}
>  
> +	/*
> +	 * Check whether the SPI NOR memory has already been configured (at
> +	 * reset or by some bootloader) to use a protocol other than SPI 1-1-1.
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(configs); ++i) {
> +		int len = SPI_NOR_MAX_ID_LEN;
> +		bool is_multi = false;
> +
> +		/*
> +		 * Check the latest read Manufacturer ID + Device ID (3 bytes):
> +		 * if they are different from both 0x000000 and 0xffffff, we
> +		 * assume that we succeeded in reading a valid JEDEC ID so we
> +		 * don't need to try other SPI protocols.
> +		 * Indeed when either the protocol or the op code are not valid,
> +		 * the SPI NOR memory should not reply to the command. Hence the
> +		 * SPI I/O lines remain in their default state: 1 when connected
> +		 * to pull-up resistors or 0 with pull-down.
> +		 */
> +		if (!((id[0] == 0xff && id[1] == 0xff && id[2] == 0xff) ||
> +		      (id[0] == 0x00 && id[1] == 0x00 && id[2] == 0x00)))
> +			break;

Hopefully, you'll have all the pins configured with pull-up or
pull-down resistors, but what if one of them is configured differently?

Shouldn't we try to find a valid chip definition by iterating over the
spi_nor_ids[] table after each mode change?

> +
> +		/* Only try protocols supported by the user. */
> +		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.
> +		 */
> +		if (configs[i].opcode == SPINOR_OP_MIO_RDID) {
> +			is_multi = true;
> +			len = 3;
> +			memset(id + len, 0, sizeof(id) - len);
> +		}
> +
> +		tmp = nor->read_reg(nor, configs[i].opcode, id, len);
> +		if (tmp < 0) {
> +			dev_dbg(nor->dev,
> +				"error %d reading JEDEC ID%s\n",
> +				tmp, (is_multi ? " Multi I/O" : ""));
> +			return ERR_PTR(tmp);
> +		}
> +	}
> +
>  	for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
>  		info = &spi_nor_ids[tmp];
>  		if (info->id_len) {
> @@ -1148,11 +1217,17 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>  	if (ret)
>  		return ret;
>  
> +	/* Reset SPI protocol for all commands */
> +	nor->erase_proto = SNOR_PROTO_1_1_1;
> +	nor->read_proto = SNOR_PROTO_1_1_1;
> +	nor->write_proto = SNOR_PROTO_1_1_1;
> +	nor->reg_proto = SNOR_PROTO_1_1_1;
> +
>  	if (name)
>  		info = spi_nor_match_id(name);
>  	/* Try to auto-detect if chip name wasn't specified or not found */
>  	if (!info)
> -		info = spi_nor_read_id(nor);
> +		info = spi_nor_read_id(nor, mode);
>  	if (IS_ERR_OR_NULL(info))
>  		return -ENOENT;
>  
> @@ -1163,7 +1238,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 62356d50815b..53932c87bcf2 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,43 @@ enum read_mode {
>  	SPI_NOR_QUAD,
>  };
>  
> +

Extra blank line here ^

> +#define SNOR_PROTO_CMD_OFF	8
> +#define SNOR_PROTO_CMD_MASK	GENMASK(11, 8)
> +#define SNOR_PROTO_CMD_TO_PROTO(cmd) \
> +	(((cmd) << SNOR_PROTO_CMD_OFF) & SNOR_PROTO_CMD_MASK)
> +#define SNOR_PROTO_CMD_FROM_PROTO(proto) \
> +	((((u32)(proto)) & SNOR_PROTO_CMD_MASK) >> SNOR_PROTO_CMD_OFF)
> +
> +#define SNOR_PROTO_ADDR_OFF	4
> +#define SNOR_PROTO_ADDR_MASK	GENMASK(7, 4)
> +#define SNOR_PROTO_ADDR_TO_PROTO(addr) \
> +	(((addr) << SNOR_PROTO_ADDR_OFF) & SNOR_PROTO_ADDR_MASK)
> +#define SNOR_PROTO_ADDR_FROM_PROTO(proto) \
> +	((((u32)(proto)) & SNOR_PROTO_ADDR_MASK) >> SNOR_PROTO_ADDR_OFF)
> +
> +#define SNOR_PROTO_DATA_OFF	0
> +#define SNOR_PROTO_DATA_MASK	GENMASK(3, 0)

Why not directly reserving 8 bits per cycle type. Your end result is
stored on a 32 bit integer anyway, and since you'll always have only 3
cycle types (ADDR, CMD and DATA). This way, you'll be ready for
SPI-8-8-8 :-).

> +#define SNOR_PROTO_DATA_TO_PROTO(data) \
> +	(((data) << SNOR_PROTO_DATA_OFF) & SNOR_PROTO_DATA_MASK)
> +#define SNOR_PROTO_DATA_FROM_PROTO(proto) \
> +	((((u32)(proto)) & SNOR_PROTO_DATA_MASK) >> SNOR_PROTO_DATA_OFF)
> +
> +#define SNOR_PROTO(cmd, addr, data)	  \
> +	(SNOR_PROTO_CMD_TO_PROTO(cmd) |   \
> +	 SNOR_PROTO_ADDR_TO_PROTO(addr) | \
> +	 SNOR_PROTO_DATA_TO_PROTO(data))
> +
> +enum spi_nor_protocol {
> +	SNOR_PROTO_1_1_1 = SNOR_PROTO(1, 1, 1),	/* SPI */
> +	SNOR_PROTO_1_1_2 = SNOR_PROTO(1, 1, 2),	/* Dual Output */
> +	SNOR_PROTO_1_1_4 = SNOR_PROTO(1, 1, 4),	/* Quad Output */
> +	SNOR_PROTO_1_2_2 = SNOR_PROTO(1, 2, 2),	/* Dual IO */
> +	SNOR_PROTO_1_4_4 = SNOR_PROTO(1, 4, 4),	/* Quad IO */
> +	SNOR_PROTO_2_2_2 = SNOR_PROTO(2, 2, 2),	/* Dual Command */
> +	SNOR_PROTO_4_4_4 = SNOR_PROTO(4, 4, 4),	/* Quad Command */
> +};

Do you really need this enum definition? I mean, directly using
SNOR_PROTO(X, Y, Z) is just as easy as using the SNOR_PROTO_X_Y_Z enum
definitions, and you won't have to define new ones if spi-nor vendors
decide to support new combinations.

> +
>  #define SPI_NOR_MAX_CMD_SIZE	8
>  enum spi_nor_ops {
>  	SPI_NOR_OPS_READ = 0,
> @@ -132,6 +170,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 +202,10 @@ struct spi_nor {
>  	u8			read_opcode;
>  	u8			read_dummy;
>  	u8			program_opcode;
> +	enum spi_nor_protocol	erase_proto;
> +	enum spi_nor_protocol	read_proto;
> +	enum spi_nor_protocol	write_proto;
> +	enum spi_nor_protocol	reg_proto;

Should be u32 fields if you decide to drop the enum definition.

>  	enum read_mode		flash_read;
>  	bool			sst_write_second;
>  	u32			flags;



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH linux-next v2 03/14] mtd: spi-nor: select op codes and SPI NOR protocols by manufacturer

[Boris Brezillon]

Hi,

On Fri, 8 Jan 2016 17:02:15 +0100
Cyrille Pitchen <cyrille.pitchen@atmel.com> wrote:

> This is a transitional patch to prepare the split by Manufacturer of the
> support of Single/Dual/Quad SPI modes.
> 
> Indeed every QSPI NOR manufacturer (Spansion, Micron, Macronix, Winbond)
> supports Dual or Quad SPI modes on its way. Especially the Fast Read op
> code and the SPI NOR protocols to use are not quite the same depending on
> the manufacturer.
> 
> For instance Quad commands can use either the SPI 1-1-4, 1-4-4 or 4-4-4
> protocol.
> 
> This is why this patch will be completed by fixes for each manufacturer.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 110 ++++++++++++++++++++++++++++++++----------
>  include/linux/mtd/spi-nor.h   |  12 +++--
>  2 files changed, 92 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 8967319ea7da..8793cebbe5a9 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1180,17 +1180,61 @@ static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
>  			dev_err(nor->dev, "Macronix quad-read not enabled\n");
>  			return -EINVAL;
>  		}
> -		return status;
> +		/* Check whether Macronix QPI mode is enabled. */
> +		if (nor->read_proto != SNOR_PROTO_4_4_4)
> +			nor->read_proto = SNOR_PROTO_1_1_4;
> +		break;
> +
>  	case SNOR_MFR_MICRON:
> -		return 0;
> -	default:
> +		/* Check whether Micron Quad mode is enabled. */
> +		if (nor->read_proto != SNOR_PROTO_4_4_4)
> +			nor->read_proto = SNOR_PROTO_1_1_4;
> +		break;
> +
> +	case SNOR_MFR_SPANSION:

The following comment is not only related to your changes, but after
looking at the spi_nor_scan() function and a few other functions in the
core, I see a lot of vendor specific initialization.

Would it make sense to somehow set a default configuration in
spi_nor_scan() and then overload this default config using a
per-manufacturer ->init() method. Something like that.

struct spi_nor_manufacturer {
	int (*init)(struct spi_nor *nor, const struct flash_info *fi);
}

static const struct spi_nor_manufacturer manufacturers[] = {
	[<manuf-id>] = {
		.init = <manuf-init-callback>
	},
};

Of course you could add other methods if you think this differentiation
is needed after the initialization step.

>  		status = spansion_quad_enable(nor);
>  		if (status) {
>  			dev_err(nor->dev, "Spansion quad-read not enabled\n");
>  			return -EINVAL;
>  		}
> -		return status;
> +		nor->read_proto = SNOR_PROTO_1_1_4;
> +		break;
> +
> +	default:
> +		return -EINVAL;
>  	}
> +
> +	nor->read_opcode = SPINOR_OP_READ_1_1_4;
> +	return 0;
> +}
> +
> +static int set_dual_mode(struct spi_nor *nor, const struct flash_info *info)
> +{
> +	switch (JEDEC_MFR(info)) {
> +	case SNOR_MFR_MICRON:
> +		/* Check whether Micron Dual mode is enabled. */
> +		if (nor->read_proto != SNOR_PROTO_2_2_2)
> +			nor->read_proto = SNOR_PROTO_1_1_2;
> +		break;
> +
> +	default:
> +		nor->read_proto = SNOR_PROTO_1_1_2;
> +		break;
> +	}
> +
> +	nor->read_opcode = SPINOR_OP_READ_1_1_2;
> +	return 0;
> +}
> +
> +static int set_single_mode(struct spi_nor *nor, const struct flash_info *info)
> +{
> +	switch (JEDEC_MFR(info)) {
> +	default:
> +		nor->read_proto = SNOR_PROTO_1_1_1;
> +		break;
> +	}

You can just put

	nor->read_proto = SNOR_PROTO_1_1_1;

until you have a manufacturer specific case.

> +
> +	return 0;
>  }
>  
>  static int spi_nor_check(struct spi_nor *nor)
> @@ -1338,7 +1382,30 @@ 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 */
> +	if (nor->flash_read == SPI_NOR_NORMAL)
> +		nor->read_opcode = SPINOR_OP_READ;
> +	else
> +		nor->read_opcode = SPINOR_OP_READ_FAST;
> +
> +	nor->program_opcode = SPINOR_OP_PP;
> +
> +	/*
> +	 * Quad/Dual-read mode takes precedence over fast/normal.
> +	 *
> +	 * Manufacturer specific modes are discovered when reading the JEDEC ID
> +	 * and are reported by the nor->read_proto value:
> +	 *  - SNOR_PROTO_4_4_4 is either:
> +	 *    + Micron Quad mode enabled
> +	 *    + Macronix/Winbond QPI mode enabled
> +	 *  - SNOR_PROTO_2_2_2 is either:
> +	 *    + Micron Dual mode enabled
> +	 *
> +	 * The opcodes and the protocols 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) {
> @@ -1347,30 +1414,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) {
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 53932c87bcf2..89e3228ec1d0 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) */
>  



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

[Cyrille Pitchen]

Hi Boris,

Le 11/01/2016 11:08, Boris Brezillon a écrit :
> Hi Cyrille,
> 
> Sorry for this pretty useless review, but I think you know more about
> SPI-NOR than I do. So, consider my comments as nitpicks, which shouldn't
> prevent your series from being applied.
> 
> On Fri, 8 Jan 2016 17:02:14 +0100
> Cyrille Pitchen <cyrille.pitchen@atmel.com> 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.
>>
>> Winbond memories, once their Quad mode enabled, expect ALL commands to use
>> the SPI 4-4-4 protocol. Unlike Micron and Macronix memories, they still
>> reply to the regular JEDEC Read ID (0x9f) command.
>>
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 83 ++++++++++++++++++++++++++++++++++++++++---
>>  include/linux/mtd/spi-nor.h   | 50 ++++++++++++++++++++++++--
>>  2 files changed, 127 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 3028c06547c1..8967319ea7da 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -73,6 +73,12 @@ struct flash_info {
>>  
>>  #define JEDEC_MFR(info)	((info)->id[0])
>>  
>> +struct read_id_config {
>> +	enum read_mode		mode;
>> +	enum spi_nor_protocol	proto;
>> +	u8			opcode;
>> +};
>> +
>>  static const struct flash_info *spi_nor_match_id(const char *name);
>>  
>>  /*
>> @@ -879,11 +885,22 @@ 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[] = {
>> +		/* Winbond QPI mode */
>> +		{SPI_NOR_QUAD, SNOR_PROTO_4_4_4, SPINOR_OP_RDID},
>> +
>> +		/* Micron Quad mode & Macronix QPI mode */
>> +		{SPI_NOR_QUAD, SNOR_PROTO_4_4_4, SPINOR_OP_MIO_RDID},
>> +
>> +		/* Micron Dual mode */
>> +		{SPI_NOR_DUAL, SNOR_PROTO_2_2_2, SPINOR_OP_MIO_RDID}
>> +	};
>>  
>>  	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
>>  	if (tmp < 0) {
>> @@ -891,6 +908,58 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
>>  		return ERR_PTR(tmp);
>>  	}
>>  
>> +	/*
>> +	 * Check whether the SPI NOR memory has already been configured (at
>> +	 * reset or by some bootloader) to use a protocol other than SPI 1-1-1.
>> +	 */
>> +	for (i = 0; i < ARRAY_SIZE(configs); ++i) {
>> +		int len = SPI_NOR_MAX_ID_LEN;
>> +		bool is_multi = false;
>> +
>> +		/*
>> +		 * Check the latest read Manufacturer ID + Device ID (3 bytes):
>> +		 * if they are different from both 0x000000 and 0xffffff, we
>> +		 * assume that we succeeded in reading a valid JEDEC ID so we
>> +		 * don't need to try other SPI protocols.
>> +		 * Indeed when either the protocol or the op code are not valid,
>> +		 * the SPI NOR memory should not reply to the command. Hence the
>> +		 * SPI I/O lines remain in their default state: 1 when connected
>> +		 * to pull-up resistors or 0 with pull-down.
>> +		 */
>> +		if (!((id[0] == 0xff && id[1] == 0xff && id[2] == 0xff) ||
>> +		      (id[0] == 0x00 && id[1] == 0x00 && id[2] == 0x00)))
>> +			break;
> 
> Hopefully, you'll have all the pins configured with pull-up or
> pull-down resistors, but what if one of them is configured differently?
> 
> Shouldn't we try to find a valid chip definition by iterating over the
> spi_nor_ids[] table after each mode change?
> 

Honestly I don't know and I didn't test what would happen if no resistors were
used. So maybe you're right, it would be safer to look in the ID table up after
every read attempt.
>> +
>> +		/* Only try protocols supported by the user. */
>> +		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.
>> +		 */
>> +		if (configs[i].opcode == SPINOR_OP_MIO_RDID) {
>> +			is_multi = true;
>> +			len = 3;
>> +			memset(id + len, 0, sizeof(id) - len);
>> +		}
>> +
>> +		tmp = nor->read_reg(nor, configs[i].opcode, id, len);
>> +		if (tmp < 0) {
>> +			dev_dbg(nor->dev,
>> +				"error %d reading JEDEC ID%s\n",
>> +				tmp, (is_multi ? " Multi I/O" : ""));
>> +			return ERR_PTR(tmp);
>> +		}
>> +	}
>> +
>>  	for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
>>  		info = &spi_nor_ids[tmp];
>>  		if (info->id_len) {
>> @@ -1148,11 +1217,17 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>  	if (ret)
>>  		return ret;
>>  
>> +	/* Reset SPI protocol for all commands */
>> +	nor->erase_proto = SNOR_PROTO_1_1_1;
>> +	nor->read_proto = SNOR_PROTO_1_1_1;
>> +	nor->write_proto = SNOR_PROTO_1_1_1;
>> +	nor->reg_proto = SNOR_PROTO_1_1_1;
>> +
>>  	if (name)
>>  		info = spi_nor_match_id(name);
>>  	/* Try to auto-detect if chip name wasn't specified or not found */
>>  	if (!info)
>> -		info = spi_nor_read_id(nor);
>> +		info = spi_nor_read_id(nor, mode);
>>  	if (IS_ERR_OR_NULL(info))
>>  		return -ENOENT;
>>  
>> @@ -1163,7 +1238,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 62356d50815b..53932c87bcf2 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,43 @@ enum read_mode {
>>  	SPI_NOR_QUAD,
>>  };
>>  
>> +
> 
> Extra blank line here ^
> 
>> +#define SNOR_PROTO_CMD_OFF	8
>> +#define SNOR_PROTO_CMD_MASK	GENMASK(11, 8)
>> +#define SNOR_PROTO_CMD_TO_PROTO(cmd) \
>> +	(((cmd) << SNOR_PROTO_CMD_OFF) & SNOR_PROTO_CMD_MASK)
>> +#define SNOR_PROTO_CMD_FROM_PROTO(proto) \
>> +	((((u32)(proto)) & SNOR_PROTO_CMD_MASK) >> SNOR_PROTO_CMD_OFF)
>> +
>> +#define SNOR_PROTO_ADDR_OFF	4
>> +#define SNOR_PROTO_ADDR_MASK	GENMASK(7, 4)
>> +#define SNOR_PROTO_ADDR_TO_PROTO(addr) \
>> +	(((addr) << SNOR_PROTO_ADDR_OFF) & SNOR_PROTO_ADDR_MASK)
>> +#define SNOR_PROTO_ADDR_FROM_PROTO(proto) \
>> +	((((u32)(proto)) & SNOR_PROTO_ADDR_MASK) >> SNOR_PROTO_ADDR_OFF)
>> +
>> +#define SNOR_PROTO_DATA_OFF	0
>> +#define SNOR_PROTO_DATA_MASK	GENMASK(3, 0)
> 
> Why not directly reserving 8 bits per cycle type. Your end result is
> stored on a 32 bit integer anyway, and since you'll always have only 3
> cycle types (ADDR, CMD and DATA). This way, you'll be ready for
> SPI-8-8-8 :-).
> 

I've reserved 4 bits per cycle type so the possible values range into [0 - 15].
I thought it would be enough to anticipate future evolution like SPI 8-8-8.
Also it leaves more bits available for future usage.
Finally, I've chosen 4 bits and this order so when debugging and/or printing
the value in hex format, you can easily decode the corresponding protocol:

SPI 1-1-4 -> 0x00000114
SPI 1-2-2 -> 0x00000122
SPI 4-4-4 -> 0x00000444

Anyway I don't mind using 8 bits instead if it is preferred.

>> +#define SNOR_PROTO_DATA_TO_PROTO(data) \
>> +	(((data) << SNOR_PROTO_DATA_OFF) & SNOR_PROTO_DATA_MASK)
>> +#define SNOR_PROTO_DATA_FROM_PROTO(proto) \
>> +	((((u32)(proto)) & SNOR_PROTO_DATA_MASK) >> SNOR_PROTO_DATA_OFF)
>> +
>> +#define SNOR_PROTO(cmd, addr, data)	  \
>> +	(SNOR_PROTO_CMD_TO_PROTO(cmd) |   \
>> +	 SNOR_PROTO_ADDR_TO_PROTO(addr) | \
>> +	 SNOR_PROTO_DATA_TO_PROTO(data))
>> +
>> +enum spi_nor_protocol {
>> +	SNOR_PROTO_1_1_1 = SNOR_PROTO(1, 1, 1),	/* SPI */
>> +	SNOR_PROTO_1_1_2 = SNOR_PROTO(1, 1, 2),	/* Dual Output */
>> +	SNOR_PROTO_1_1_4 = SNOR_PROTO(1, 1, 4),	/* Quad Output */
>> +	SNOR_PROTO_1_2_2 = SNOR_PROTO(1, 2, 2),	/* Dual IO */
>> +	SNOR_PROTO_1_4_4 = SNOR_PROTO(1, 4, 4),	/* Quad IO */
>> +	SNOR_PROTO_2_2_2 = SNOR_PROTO(2, 2, 2),	/* Dual Command */
>> +	SNOR_PROTO_4_4_4 = SNOR_PROTO(4, 4, 4),	/* Quad Command */
>> +};
> 
> Do you really need this enum definition? I mean, directly using
> SNOR_PROTO(X, Y, Z) is just as easy as using the SNOR_PROTO_X_Y_Z enum
> definitions, and you won't have to define new ones if spi-nor vendors
> decide to support new combinations.
> 

This enum was created only for stronger type checking; Except for that, there
is no strong reason to keep the enum. So I don't mind removing it if other
people agree with you.


>> +
>>  #define SPI_NOR_MAX_CMD_SIZE	8
>>  enum spi_nor_ops {
>>  	SPI_NOR_OPS_READ = 0,
>> @@ -132,6 +170,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 +202,10 @@ struct spi_nor {
>>  	u8			read_opcode;
>>  	u8			read_dummy;
>>  	u8			program_opcode;
>> +	enum spi_nor_protocol	erase_proto;
>> +	enum spi_nor_protocol	read_proto;
>> +	enum spi_nor_protocol	write_proto;
>> +	enum spi_nor_protocol	reg_proto;
> 
> Should be u32 fields if you decide to drop the enum definition.
> 
>>  	enum read_mode		flash_read;
>>  	bool			sst_write_second;
>>  	u32			flags;
> 
> 
> 

Thanks for your review :)

Best regards,

Cyrille

Re: [PATCH linux-next v2 03/14] mtd: spi-nor: select op codes and SPI NOR protocols by manufacturer

[Cyrille Pitchen]

Hi Boris,

Le 11/01/2016 11:24, Boris Brezillon a écrit :
> Hi,
> 
> On Fri, 8 Jan 2016 17:02:15 +0100
> Cyrille Pitchen <cyrille.pitchen@atmel.com> wrote:
> 
>> This is a transitional patch to prepare the split by Manufacturer of the
>> support of Single/Dual/Quad SPI modes.
>>
>> Indeed every QSPI NOR manufacturer (Spansion, Micron, Macronix, Winbond)
>> supports Dual or Quad SPI modes on its way. Especially the Fast Read op
>> code and the SPI NOR protocols to use are not quite the same depending on
>> the manufacturer.
>>
>> For instance Quad commands can use either the SPI 1-1-4, 1-4-4 or 4-4-4
>> protocol.
>>
>> This is why this patch will be completed by fixes for each manufacturer.
>>
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 110 ++++++++++++++++++++++++++++++++----------
>>  include/linux/mtd/spi-nor.h   |  12 +++--
>>  2 files changed, 92 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 8967319ea7da..8793cebbe5a9 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -1180,17 +1180,61 @@ static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
>>  			dev_err(nor->dev, "Macronix quad-read not enabled\n");
>>  			return -EINVAL;
>>  		}
>> -		return status;
>> +		/* Check whether Macronix QPI mode is enabled. */
>> +		if (nor->read_proto != SNOR_PROTO_4_4_4)
>> +			nor->read_proto = SNOR_PROTO_1_1_4;
>> +		break;
>> +
>>  	case SNOR_MFR_MICRON:
>> -		return 0;
>> -	default:
>> +		/* Check whether Micron Quad mode is enabled. */
>> +		if (nor->read_proto != SNOR_PROTO_4_4_4)
>> +			nor->read_proto = SNOR_PROTO_1_1_4;
>> +		break;
>> +
>> +	case SNOR_MFR_SPANSION:
> 
> The following comment is not only related to your changes, but after
> looking at the spi_nor_scan() function and a few other functions in the
> core, I see a lot of vendor specific initialization.
> 
> Would it make sense to somehow set a default configuration in
> spi_nor_scan() and then overload this default config using a
> per-manufacturer ->init() method. Something like that.
> 
> struct spi_nor_manufacturer {
> 	int (*init)(struct spi_nor *nor, const struct flash_info *fi);
> }
> 
> static const struct spi_nor_manufacturer manufacturers[] = {
> 	[<manuf-id>] = {
> 		.init = <manuf-init-callback>
> 	},
> };
> 
> Of course you could add other methods if you think this differentiation
> is needed after the initialization step.
> 

It might be a good idea. I don't mind changing if needed. Brian, could you
please comment on this point?

>>  		status = spansion_quad_enable(nor);
>>  		if (status) {
>>  			dev_err(nor->dev, "Spansion quad-read not enabled\n");
>>  			return -EINVAL;
>>  		}
>> -		return status;
>> +		nor->read_proto = SNOR_PROTO_1_1_4;
>> +		break;
>> +
>> +	default:
>> +		return -EINVAL;
>>  	}
>> +
>> +	nor->read_opcode = SPINOR_OP_READ_1_1_4;
>> +	return 0;
>> +}
>> +
>> +static int set_dual_mode(struct spi_nor *nor, const struct flash_info *info)
>> +{
>> +	switch (JEDEC_MFR(info)) {
>> +	case SNOR_MFR_MICRON:
>> +		/* Check whether Micron Dual mode is enabled. */
>> +		if (nor->read_proto != SNOR_PROTO_2_2_2)
>> +			nor->read_proto = SNOR_PROTO_1_1_2;
>> +		break;
>> +
>> +	default:
>> +		nor->read_proto = SNOR_PROTO_1_1_2;
>> +		break;
>> +	}
>> +
>> +	nor->read_opcode = SPINOR_OP_READ_1_1_2;
>> +	return 0;
>> +}
>> +
>> +static int set_single_mode(struct spi_nor *nor, const struct flash_info *info)
>> +{
>> +	switch (JEDEC_MFR(info)) {
>> +	default:
>> +		nor->read_proto = SNOR_PROTO_1_1_1;
>> +		break;
>> +	}
> 
> You can just put
> 
> 	nor->read_proto = SNOR_PROTO_1_1_1;
> 
> until you have a manufacturer specific case.
>

I did it on purpose: it is a transitional patch which was written only to ease
the split of the QSPI support by manufacturer. So the diff producing the next
patch of this series is a little bit more easy to review: it is just a new
case in a switch statement instead of replacing a 'if' statement by a 'switch'
one.
Also it removes some (not all) constraints on the order the manufacturer
specific patches should be applied, which makes it easier to remove some of
them from the series if needed: the rebase job will be simplified.
Ideally, all manufacturer patches should be totally independent.

[...]

Best regards,

Cyrille

Re: [PATCH linux-next v2 10/14] mtd: spi-nor: configure the number of dummy clock cycles on Macronix memories

[Cyrille Pitchen]

Hi all,

I've found a small issue within this patch when I test with a Macronix
mx25l51245g (JEDEC ID C2201A so reported as "mx66l51235l" by spi-nor.c).
It deals with a wrong op code used when reading the Configuration Register.
See below for more details.

So I will send a new patch to fix this issue.

Otherwise, I tested on a sama5d2 xplained board and it works :)


Le 08/01/2016 17:02, Cyrille Pitchen a écrit :
> The spi-nor framework currently expects all Fast Read operations to use 8
> dummy clock cycles. Especially some drivers like m25p80 can only support
> multiple of 8 dummy clock cycles.
> 
> On Macronix memories, the number of dummy clock cycles to be used by Fast
> Read commands can be safely set to 8 by updating the DC0 and DC1 volatile
> bits inside the Configuration Register.
> 
> According to the mx66l1g45g datasheet from Macronix, using 8 dummy clock
> cycles should be enough to set the SPI bus clock frequency up to:
> - 133 MHz for Fast Read 1-1-1, 1-1-2, 1-1-4 and 1-2-2 commands in Single
>   Transfer Rate (STR)
> - 104 MHz for Fast Read 1-4-4 (or 4-4-4 in QPI mode) commands (STR)
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> ---
[...]
> +static int macronix_set_dummy_cycles(struct spi_nor *nor, u8 read_dummy)
> +{
> +	int ret, sr, cr, mask, val;
> +	u16 sr_cr;
> +	u8 dc;
> +
> +	/* 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);
Macronix memories use the 0x15 op code (not 0x35) to read the Configuration Register.
The 0x35 op code is used to enter the QPI mode. So read_cr() cannot be used here.

> +	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 writing the SR and CR registers\n");
> +		return ret;
> +	}
> +
> +	ret = spi_nor_wait_till_ready(nor);
> +	if (ret)
> +		return ret;
> +
> +	cr = read_cr(nor);
Here again, we must use the 0x15 op code instead of 0x35.

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

Best regards,

Cyrille