[PATCH v4 0/7] mtd: spi-nor: add xSPI Octal DTR support

[PATCH v4 0/7] mtd: spi-nor: add xSPI Octal DTR support

[Mason Yang]

Hello,

JESD216C has defined specification for Octal 8S-8S-8S and 8D-8D-8D.
Based on JEDEC216C Basic Flash Parameter Table (BFPT) driver extract
DWORD-18: command and command extension type.
DWORD-20: Maximum operation speed of device in Octal mode.

xSPI profile 1.0 table:
DWORD-1: Read Fast command, the number of dummy cycles and address nbytes
	 for Read Status Register command.
DWORD-2: Read/Write volatile Register command for CFG Reg2.
DWORD-4 and DWORD-5: dummy cycles used for various frequencies based on
maximum speed of device from BFPT 20th DWORD.

Ccommand sequences to change to octal DTR mode:
The length of each command sequence is 8 per byte for single SPI mode and
patching driver to parse and execute these sequences for octal DTR mode.

By Vignesh's comments, this patches set is dependency on Pratyush's
[1][2][5][7][8] and [10][12].

Tested on Macronix's Zynq PicoZed board with Macronix's SPI controller
(spi-mxic.c) driver patched on mx25uw51245g Octal flash.

[1] https://patchwork.ozlabs.org/project/linux-mtd/patch/20200525091544.17270-2-p.yadav@ti.com/
[2] https://patchwork.ozlabs.org/project/linux-mtd/patch/20200525091544.17270-3-p.yadav@ti.com/
[5] https://patchwork.ozlabs.org/project/linux-mtd/patch/20200525091544.17270-6-p.yadav@ti.com/
[7] https://patchwork.ozlabs.org/project/linux-mtd/patch/20200525091544.17270-8-p.yadav@ti.com/
[8] https://patchwork.ozlabs.org/project/linux-mtd/patch/20200525091544.17270-9-p.yadav@ti.com/
[10] https://patchwork.ozlabs.org/project/linux-mtd/patch/20200525091544.17270-11-p.yadav@ti.com/
[12] https://patchwork.ozlabs.org/project/linux-mtd/patch/20200525091544.17270-13-p.yadav@ti.com/

Summary of change log
v4:
Removed Pratyush's patches set part.

v3:
Add support command sequences to change octal DTR mode and based on
part of Pratyush's patches set.

v2: 
Parse BFPT & xSPI table for Octal 8D-8D-8D mode parameters and enable Octal
mode in spi_nor_late_init_params().
Using Macros in spi_nor_spimem_read_data, spi_nor_spimem_write_data and
so on by Vignesh comments.

v1:
Without parsing BFPT & xSPI profile 1.0 table and enter Octal 8D-8D-8D
mode directly in spi_nor_fixups hooks.


thnaks for your time and review.
best regards,
Mason

Mason Yang (7):
  mtd: spi-nor: sfdp: get octal mode maximum speed from BFPT
  mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table
  mtd: spi-nor: sfdp: parse command sequences to change octal DTR mode
  mtd: spi-nor: core: add configuration register 2 read & write support
  mtd: spi-nor: core: execute command sequences to change octal DTR mode
  spi: mxic: patch for octal DTR mode support
  mtd: spi-nor: macronix: Add Octal 8D-8D-8D supports for Macronix
    mx25uw51245g

 drivers/mtd/spi-nor/core.c     | 151 ++++++++++++++++++++++++-
 drivers/mtd/spi-nor/core.h     |  41 +++++++
 drivers/mtd/spi-nor/macronix.c |  55 +++++++++
 drivers/mtd/spi-nor/sfdp.c     | 246 +++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/spi-nor/sfdp.h     |   4 +
 drivers/spi/spi-mxic.c         |  98 ++++++++++------
 6 files changed, 562 insertions(+), 33 deletions(-)

-- 
1.9.1

[PATCH v4 1/7] mtd: spi-nor: sfdp: get octal mode maximum speed from BFPT

[Mason Yang]

Get maximum operation speed of device in octal mode from
BFPT 20th DWORD.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/mtd/spi-nor/core.h |  2 ++
 drivers/mtd/spi-nor/sfdp.c | 36 ++++++++++++++++++++++++++++++++++++
 drivers/mtd/spi-nor/sfdp.h |  4 ++++
 3 files changed, 42 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 6f2f6b2..7a36b22 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -190,6 +190,7 @@ struct spi_nor_locking_ops {
  *
  * @size:		the flash memory density in bytes.
  * @page_size:		the page size of the SPI NOR flash memory.
+ * @octal_max_speed:	maximum operation speed of device in octal mode.
  * @hwcaps:		describes the read and page program hardware
  *			capabilities.
  * @reads:		read capabilities ordered by priority: the higher index
@@ -212,6 +213,7 @@ struct spi_nor_locking_ops {
 struct spi_nor_flash_parameter {
 	u64				size;
 	u32				page_size;
+	u16				octal_max_speed;
 
 	struct spi_nor_hwcaps		hwcaps;
 	struct spi_nor_read_command	reads[SNOR_CMD_READ_MAX];
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index f6038d3..4d13f66 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -4,6 +4,7 @@
  * Copyright (C) 2014, Freescale Semiconductor, Inc.
  */
 
+#include <linux/bitfield.h>
 #include <linux/slab.h>
 #include <linux/sort.h>
 #include <linux/mtd/spi-nor.h>
@@ -26,6 +27,12 @@
 #define SFDP_JESD216A_MINOR	5
 #define SFDP_JESD216B_MINOR	6
 
+/* Basic Flash Parameter Table 20th DWORD, Max operation speed of device */
+struct octal_max_speed {
+	u8 idx; /* Bits value */
+	u16 hz; /* MHz */
+};
+
 struct sfdp_header {
 	u32		signature; /* Ox50444653U <=> "SFDP" */
 	u8		minor;
@@ -440,6 +447,22 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
 	u32 addr;
 	u16 half;
 	u8 erase_mask;
+	static const struct octal_max_speed max_hz[] = {
+		/* Bits value, MHz */
+		{ 0x0c, 400 },
+		{ 0x0b, 333 },
+		{ 0x0a, 266 },
+		{ 0x09, 250 },
+		{ 0x08, 200 },
+		{ 0x07, 166 },
+		{ 0x06, 133 },
+		{ 0x05, 100 },
+		{ 0x04,  80 },
+		{ 0x03,  66 },
+		{ 0x02,  50 },
+		{ 0x01,  33 },
+	};
+	u8 idx;
 
 	/* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs. */
 	if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
@@ -604,6 +627,19 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
 		return -EINVAL;
 	}
 
+	/* Octal mode max speed */
+	idx = max(FIELD_GET(BFPT_DWORD20_OCTAL_DTR_MAX_SPEED,
+			    bfpt.dwords[BFPT_DWORD(20)]),
+		  FIELD_GET(BFPT_DWORD20_OCTAL_STR_MAX_SPEED,
+			    bfpt.dwords[BFPT_DWORD(20)]));
+
+	for (i = 0; i < ARRAY_SIZE(max_hz); i++) {
+		if (max_hz[i].idx == idx) {
+			params->octal_max_speed = max_hz[i].hz;
+			break;
+		}
+	}
+
 	return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt, params);
 }
 
diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
index e0a8ded..8ae6d9a 100644
--- a/drivers/mtd/spi-nor/sfdp.h
+++ b/drivers/mtd/spi-nor/sfdp.h
@@ -83,6 +83,10 @@ struct sfdp_bfpt {
 #define BFPT_DWORD15_QER_SR2_BIT1_NO_RD		(0x4UL << 20)
 #define BFPT_DWORD15_QER_SR2_BIT1		(0x5UL << 20) /* Spansion */
 
+#define BFPT_DWORD20_OCTAL_MAX_SPEED_MASK       GENMASK(31, 16)
+#define BFPT_DWORD20_OCTAL_DTR_MAX_SPEED        GENMASK(31, 28)
+#define BFPT_DWORD20_OCTAL_STR_MAX_SPEED        GENMASK(19, 16)
+
 struct sfdp_parameter_header {
 	u8		id_lsb;
 	u8		minor;
-- 
1.9.1

[PATCH v4 2/7] mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table

[Mason Yang]

JESD251, xSPI profile 1.0 table supports octal DTR mode.
Extract information like the fast read opcode, dummy cycles for various
frequencies, the number of dummy cycles needed for a Read Status
Register command, the number of address bytes needed for a Read
Status Register command, read volatile register command and write
volatile register command.

According to BFPT 20th DWORD of octal maximum speed, driver get it's
specific dummy cycles from profile 1.0 table and then could update
it to device by their fixup hooks.

Since driver get octal DTR read opcode and then set read settings,
expose spi_nor_set_read_settings() in core.h.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/mtd/spi-nor/core.c |   2 +-
 drivers/mtd/spi-nor/core.h |  16 +++++++
 drivers/mtd/spi-nor/sfdp.c | 106 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 1ab4386..3799417 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2204,7 +2204,7 @@ static int spi_nor_check(struct spi_nor *nor)
 	return 0;
 }
 
-static void
+void
 spi_nor_set_read_settings(struct spi_nor_read_command *read,
 			  u8 num_mode_clocks,
 			  u8 num_wait_states,
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 7a36b22..a33f807 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -191,6 +191,12 @@ struct spi_nor_locking_ops {
  * @size:		the flash memory density in bytes.
  * @page_size:		the page size of the SPI NOR flash memory.
  * @octal_max_speed:	maximum operation speed of device in octal mode.
+ * @rdsr_dummy:		dummy cycles needed for Read Status Register command.
+ * @rdsr_addr_nbytes:	dummy address bytes needed for Read Status Register
+ *			command.
+ * @rd_reg_cmd:		read volatile register command for xSPI device.
+ * @wr_reg_cmd:		write volatile register command for xSPI device.
+ * @dummy_cycles:	dummy cycles used for various frequencies
  * @hwcaps:		describes the read and page program hardware
  *			capabilities.
  * @reads:		read capabilities ordered by priority: the higher index
@@ -214,6 +220,11 @@ struct spi_nor_flash_parameter {
 	u64				size;
 	u32				page_size;
 	u16				octal_max_speed;
+	u8				rdsr_dummy;
+	u8				rdsr_addr_nbytes;
+	u8				rd_reg_cmd;
+	u8				wr_reg_cmd;
+	u8				dummy_cycles;
 
 	struct spi_nor_hwcaps		hwcaps;
 	struct spi_nor_read_command	reads[SNOR_CMD_READ_MAX];
@@ -420,6 +431,11 @@ ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
 
 int spi_nor_hwcaps_read2cmd(u32 hwcaps);
 u8 spi_nor_convert_3to4_read(u8 opcode);
+void spi_nor_set_read_settings(struct spi_nor_read_command *read,
+			       u8 num_mode_clocks,
+			       u8 num_wait_states,
+			       u8 opcode,
+			       enum spi_nor_protocol proto);
 void spi_nor_set_pp_settings(struct spi_nor_pp_command *pp, u8 opcode,
 			     enum spi_nor_protocol proto);
 
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 4d13f66..27a4de4 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -20,6 +20,7 @@
 #define SFDP_BFPT_ID		0xff00	/* Basic Flash Parameter Table */
 #define SFDP_SECTOR_MAP_ID	0xff81	/* Sector Map Table */
 #define SFDP_4BAIT_ID		0xff84  /* 4-byte Address Instruction Table */
+#define SFDP_PROFILE1_ID	0xff05	/* xSPI Profile 1.0 table. */
 
 #define SFDP_SIGNATURE		0x50444653U
 #define SFDP_JESD216_MAJOR	1
@@ -27,6 +28,27 @@
 #define SFDP_JESD216A_MINOR	5
 #define SFDP_JESD216B_MINOR	6
 
+/* xSPI Profile 1.0 table (from JESD216D.01). */
+#define XSPI_PF1_DWORD1_RD_CMD                  GENMASK(15, 8)
+#define XSPI_PF1_DWORD1_RDSR_ADDR_BYTES         BIT(29)
+#define XSPI_PF1_DWORD1_RDSR_DUMMY_CYCLES       BIT(28)
+#define XSPI_PF1_DWORD2_RD_REG_CMD              GENMASK(31, 24)
+#define XSPI_PF1_DWORD2_WR_REG_CMD              GENMASK(15, 8)
+#define XSPI_PF1_DUMMY_CYCLES_DEFAULT		20
+#define XSPI_DWORD(x)   ((x) - 1)
+#define XSPI_DWORD_MAX  5
+
+struct sfdp_xspi {
+	u32 dwords[XSPI_DWORD_MAX];
+};
+
+struct xspi_dummy_cycles {
+	u16 speed_hz;	/* Speed MHz */
+	u8 dwords;	/* Dwords index */
+	u32 mask;	/* Mask */
+	u8 shift;	/* Bit shift */
+};
+
 /* Basic Flash Parameter Table 20th DWORD, Max operation speed of device */
 struct octal_max_speed {
 	u8 idx; /* Bits value */
@@ -1117,6 +1139,86 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
 }
 
 /**
+ * spi_nor_parse_profile1() - parse the xSPI Profile 1.0 table
+ * @nor:		pointer to a 'struct spi_nor'
+ * @param_header:	xspi profile 1 parameter table header
+ * @params:		pointer to the 'struct spi_nor_flash_parameter' to be.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_parse_profile1(struct spi_nor *nor,
+				  const struct sfdp_parameter_header *header,
+				  struct spi_nor_flash_parameter *params)
+{
+	struct sfdp_xspi pfile1;
+	u8 opcode;
+	u32 i, addr;
+	size_t len;
+	int ret;
+	static const struct xspi_dummy_cycles dummy[] = {
+		/* {MHz, Dwords index, Mask, Bit shift} */
+		{ 200, 4, GENMASK(11, 7),   7},
+		{ 166, 5, GENMASK(31, 27), 27},
+		{ 133, 5, GENMASK(21, 17), 17},
+		{ 100, 5, GENMASK(11, 7),   7},
+	};
+
+	if (header->major != SFDP_JESD216_MAJOR ||
+	    header->length < XSPI_DWORD_MAX)
+		return -EINVAL;
+
+	len = min_t(size_t, sizeof(pfile1),
+		    header->length * sizeof(u32));
+
+	memset(&pfile1, 0, sizeof(pfile1));
+
+	addr = SFDP_PARAM_HEADER_PTP(header);
+	ret = spi_nor_read_sfdp(nor, addr, len, &pfile1);
+	if (ret)
+		goto out;
+
+	/* Fix endianness of the xSPI 1.0 DWORDs. */
+	le32_to_cpu_array(pfile1.dwords, XSPI_DWORD_MAX);
+
+	/* Get 8D-8D-8D fast read opcode and dummy cycles. */
+	opcode = FIELD_GET(XSPI_PF1_DWORD1_RD_CMD,
+			   pfile1.dwords[XSPI_DWORD(1)]);
+
+	if (pfile1.dwords[XSPI_DWORD(1)] & XSPI_PF1_DWORD1_RDSR_ADDR_BYTES)
+		params->rdsr_addr_nbytes = 4;
+	else
+		params->rdsr_addr_nbytes = 0;
+
+	if (pfile1.dwords[XSPI_DWORD(1)] & XSPI_PF1_DWORD1_RDSR_DUMMY_CYCLES)
+		params->rdsr_dummy = 8;
+	else
+		params->rdsr_dummy = 4;
+
+	params->rd_reg_cmd = FIELD_GET(XSPI_PF1_DWORD2_RD_REG_CMD,
+				       pfile1.dwords[XSPI_DWORD(2)]);
+	params->wr_reg_cmd = FIELD_GET(XSPI_PF1_DWORD2_WR_REG_CMD,
+				       pfile1.dwords[XSPI_DWORD(2)]);
+
+	/* according to BFPT 20th DOWRD to get devices dummy cycles */
+	for (i = 0; i < ARRAY_SIZE(dummy); i++) {
+		if (params->octal_max_speed == dummy[i].speed_hz) {
+			params->dummy_cycles = (dummy[i].mask &
+				pfile1.dwords[XSPI_DWORD(dummy[i].dwords)]) >>
+				dummy[i].shift;
+			break;
+		}
+	}
+	if (i == ARRAY_SIZE(dummy))
+		params->dummy_cycles = XSPI_PF1_DUMMY_CYCLES_DEFAULT;
+
+	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_8_8_8_DTR],
+				  0, params->dummy_cycles,
+				  opcode, SNOR_PROTO_8_8_8_DTR);
+out:
+	return ret;
+}
+
+/**
  * spi_nor_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
  * @nor:		pointer to a 'struct spi_nor'
  * @params:		pointer to the 'struct spi_nor_flash_parameter' to be
@@ -1217,6 +1319,10 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
 			err = spi_nor_parse_4bait(nor, param_header, params);
 			break;
 
+		case SFDP_PROFILE1_ID:
+			err = spi_nor_parse_profile1(nor, param_header, params);
+			break;
+
 		default:
 			break;
 		}
-- 
1.9.1

[PATCH v4 3/7] mtd: spi-nor: sfdp: parse command sequences to change octal DTR mode

[Mason Yang]

A set of simple command sequences is provided which can be executed
directly by the host controller to enable octal DTR mode.

Each command sequence is 8 per byte for single SPI mode.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/mtd/spi-nor/core.h |  20 +++++++++
 drivers/mtd/spi-nor/sfdp.c | 104 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index a33f807..8de7f53 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -183,6 +183,23 @@ struct spi_nor_locking_ops {
 };
 
 /**
+ * struct cmd_seq_octal_dtr - command sequences to change to octal DTR mode
+ * Each command sequence is 8 per byte for single SPI mode.
+ * @len:	commmand length of each command sequence.
+ * @opcode:	command code.
+ * @addr:	address offset to device.
+ * @data:	data write to device.
+ */
+struct cmd_seq_octal_dtr {
+	u8 len;
+	u8 opcode;
+	u32 addr;
+	u8 data;
+};
+
+#define CMD_SEQ_NUM	4
+
+/**
  * struct spi_nor_flash_parameter - SPI NOR flash parameters and settings.
  * Includes legacy flash parameters and settings that can be overwritten
  * by the spi_nor_fixups hooks, or dynamically when parsing the JESD216
@@ -205,6 +222,7 @@ struct spi_nor_locking_ops {
  *                      higher index in the array, the higher priority.
  * @erase_map:		the erase map parsed from the SFDP Sector Map Parameter
  *                      Table.
+ * @cmd_seq:		command sequence to change to octal DTR mode.
  * @quad_enable:	enables SPI NOR quad mode.
  * @set_4byte_addr_mode: puts the SPI NOR in 4 byte addressing mode.
  * @convert_addr:	converts an absolute address into something the flash
@@ -232,6 +250,8 @@ struct spi_nor_flash_parameter {
 
 	struct spi_nor_erase_map        erase_map;
 
+	struct cmd_seq_octal_dtr	cmd_seq[CMD_SEQ_NUM];
+
 	int (*quad_enable)(struct spi_nor *nor);
 	int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
 	u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 27a4de4..ef19290 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -21,6 +21,7 @@
 #define SFDP_SECTOR_MAP_ID	0xff81	/* Sector Map Table */
 #define SFDP_4BAIT_ID		0xff84  /* 4-byte Address Instruction Table */
 #define SFDP_PROFILE1_ID	0xff05	/* xSPI Profile 1.0 table. */
+#define SFDP_CMD_TO_8DTR_ID	0xff0a  /* Command Sequences to Octal DTR */
 
 #define SFDP_SIGNATURE		0x50444653U
 #define SFDP_JESD216_MAJOR	1
@@ -49,6 +50,19 @@ struct xspi_dummy_cycles {
 	u8 shift;	/* Bit shift */
 };
 
+#define CMD_TO_8DTR_LEN			GENMASK(31, 24)
+#define CMD_TO_8DTR_OPCODE		GENMASK(23, 16)
+#define CMD_TO_8DTR_1_ADDR		GENMASK(15, 8)
+#define CMD_TO_8DTR_1_ADDR_DATA		GENMASK(7, 0)
+#define CMD_TO_8DTR_4_ADDR_MSB		GENMASK(15, 0)
+#define CMD_TO_8DTR_4_ADDR_LSB		GENMASK(31, 16)
+#define CMD_TO_8DTR_4_ADDR_DATA		GENMASK(15, 8)
+#define CMD_TO_8DTR_SIZE_MAX		8
+
+struct sfdp_cmd_to_8dtr {
+	u32	dwords[CMD_TO_8DTR_SIZE_MAX];
+};
+
 /* Basic Flash Parameter Table 20th DWORD, Max operation speed of device */
 struct octal_max_speed {
 	u8 idx; /* Bits value */
@@ -1219,6 +1233,91 @@ static int spi_nor_parse_profile1(struct spi_nor *nor,
 }
 
 /**
+ * spi_nor_parse_cmd_to_8dtr() - parse the command sequence to octal DTR
+ * @nor:		pointer to a 'struct spi_nor'
+ * @param_header:	command sequence to octal DTR parameter table header.
+ * @params:		pointer to the 'struct spi_nor_flash_parameter' to be.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_parse_cmd_to_8dtr(struct spi_nor *nor,
+				     const struct sfdp_parameter_header *header,
+				     struct spi_nor_flash_parameter *params)
+{
+	struct sfdp_cmd_to_8dtr cmd_seq;
+	u32 i, j, addr;
+	size_t len;
+	int ret;
+
+	if (header->major != SFDP_JESD216_MAJOR ||
+	    header->length < CMD_TO_8DTR_SIZE_MAX)
+		return -EINVAL;
+
+	len = min_t(size_t, sizeof(cmd_seq),
+		    header->length * sizeof(u32));
+
+	memset(&cmd_seq, 0, sizeof(cmd_seq));
+
+	addr = SFDP_PARAM_HEADER_PTP(header);
+	ret = spi_nor_read_sfdp(nor, addr, len, &cmd_seq);
+	if (ret)
+		goto out;
+
+	/* Fix endianness of the Command Sequences to octal DTR. */
+	le32_to_cpu_array(cmd_seq.dwords, CMD_TO_8DTR_SIZE_MAX);
+
+	memset(params->cmd_seq, 0, sizeof(params->cmd_seq[CMD_SEQ_NUM]));
+
+	for (i = 0, j = 0;
+	     i < CMD_SEQ_NUM && j < CMD_TO_8DTR_SIZE_MAX; i++, j += 2) {
+		params->cmd_seq[i].len = FIELD_GET(CMD_TO_8DTR_LEN,
+						   cmd_seq.dwords[j]);
+		if (!params->cmd_seq[i].len)
+			break;
+
+		switch (params->cmd_seq[i].len) {
+		case 1:
+			params->cmd_seq[i].opcode =
+				FIELD_GET(CMD_TO_8DTR_OPCODE,
+					  cmd_seq.dwords[j]);
+			break;
+
+		case 3:
+			params->cmd_seq[i].opcode =
+				FIELD_GET(CMD_TO_8DTR_OPCODE,
+					  cmd_seq.dwords[j]);
+			params->cmd_seq[i].addr =
+				FIELD_GET(CMD_TO_8DTR_1_ADDR,
+					  cmd_seq.dwords[j]);
+			params->cmd_seq[i].data =
+				FIELD_GET(CMD_TO_8DTR_1_ADDR_DATA,
+					  cmd_seq.dwords[j]);
+			break;
+
+		case 6:
+			params->cmd_seq[i].opcode =
+				FIELD_GET(CMD_TO_8DTR_OPCODE,
+					  cmd_seq.dwords[j]);
+			params->cmd_seq[i].addr =
+				FIELD_GET(CMD_TO_8DTR_4_ADDR_MSB,
+					  cmd_seq.dwords[j]) << 16 |
+				FIELD_GET(CMD_TO_8DTR_4_ADDR_LSB,
+					  cmd_seq.dwords[j + 1]);
+			params->cmd_seq[i].data =
+				FIELD_GET(CMD_TO_8DTR_4_ADDR_DATA,
+					  cmd_seq.dwords[j + 1]);
+			break;
+
+		default:
+			break;
+		}
+	}
+
+out:
+	return ret;
+}
+
+/**
  * spi_nor_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
  * @nor:		pointer to a 'struct spi_nor'
  * @params:		pointer to the 'struct spi_nor_flash_parameter' to be
@@ -1323,6 +1422,11 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
 			err = spi_nor_parse_profile1(nor, param_header, params);
 			break;
 
+		case SFDP_CMD_TO_8DTR_ID:
+			err = spi_nor_parse_cmd_to_8dtr(nor,
+							param_header, params);
+			break;
+
 		default:
 			break;
 		}
-- 
1.9.1

[PATCH v4 4/7] mtd: spi-nor: core: add configuration register 2 read & write support

[Mason Yang]

Configuration register 2 is to set the device operation condition like
STR or DTR mode at address offset 0 and DQS mode at address offset 0x200.

Each device has various address offset for it's specific operatoin
setting.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/mtd/spi-nor/core.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/spi-nor/core.h |  2 ++
 2 files changed, 80 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 3799417..fed6236 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -269,6 +269,84 @@ int spi_nor_write_disable(struct spi_nor *nor)
 }
 
 /**
+ * spi_nor_read_cr2() - Read the Configuration Register 2.
+ * @nor:	pointer to 'struct spi_nor'.
+ * @addr:	offset address to read.
+ * @cr2:	pointer to a DMA-able buffer where the value of the
+ *              Configuration Register 2  will be written.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+int spi_nor_read_cr2(struct spi_nor *nor, u32 addr, u8 *cr2)
+{
+	int ret;
+	u8 cmd = nor->params->rd_reg_cmd;
+
+	if (nor->spimem) {
+		struct spi_mem_op op =
+			SPI_MEM_OP(SPI_MEM_OP_CMD(cmd, 1),
+				   SPI_MEM_OP_ADDR(4, addr, 1),
+				   SPI_MEM_OP_DUMMY(4, 1),
+				   SPI_MEM_OP_DATA_IN(1, cr2, 1));
+
+		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		if (spi_nor_protocol_is_dtr(nor->reg_proto))
+			ret = -ENOTSUPP;
+		else
+			ret = nor->controller_ops->read_reg(nor, cmd, cr2, 1);
+	}
+
+	if (ret)
+		dev_dbg(nor->dev, "error %d reading CR2\n", ret);
+
+	return ret;
+}
+
+/**
+ * spi_nor_write_cr2() - Write the Configuration Register 2.
+ * @nor:	pointer to 'struct spi_nor'.
+ * @addr:	offset address to write.
+ * @cr2:	pointer to a DMA-able buffer where the value of the
+ *              Configuratin Register 2 will be read.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+int spi_nor_write_cr2(struct spi_nor *nor, u32 addr, u8 *cr2)
+{
+	int ret;
+	u8 cmd = nor->params->wr_reg_cmd;
+
+	ret = spi_nor_write_enable(nor);
+	if (ret)
+		return ret;
+
+	if (nor->spimem) {
+		struct spi_mem_op op =
+			SPI_MEM_OP(SPI_MEM_OP_CMD(cmd, 1),
+				   SPI_MEM_OP_ADDR(4, addr, 1),
+				   SPI_MEM_OP_NO_DUMMY,
+				   SPI_MEM_OP_DATA_OUT(1, cr2, 1));
+
+		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		if (spi_nor_protocol_is_dtr(nor->reg_proto))
+			ret = -ENOTSUPP;
+		else
+			ret = nor->controller_ops->write_reg(nor, cmd, cr2, 1);
+	}
+
+	if (ret)
+		dev_dbg(nor->dev, "error %d write CFG Reg 2\n", ret);
+
+	return ret;
+}
+
+/**
  * spi_nor_read_sr() - Read the Status Register.
  * @nor:	pointer to 'struct spi_nor'.
  * @sr:		pointer to a DMA-able buffer where the value of the
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 8de7f53..0eb07ca 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -434,6 +434,8 @@ struct spi_nor_manufacturer {
 
 int spi_nor_write_enable(struct spi_nor *nor);
 int spi_nor_write_disable(struct spi_nor *nor);
+int spi_nor_read_cr2(struct spi_nor *nor, u32 addr, u8 *cr2);
+int spi_nor_write_cr2(struct spi_nor *nor, u32 addr, u8 *cr2);
 int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable);
 int spi_nor_write_ear(struct spi_nor *nor, u8 ear);
 int spi_nor_wait_till_ready(struct spi_nor *nor);
-- 
1.9.1

[PATCH v4 5/7] mtd: spi-nor: core: execute command sequences to change octal DTR mode

[Mason Yang]

Execute command sequences to change octal DTR mode.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/mtd/spi-nor/core.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/spi-nor/core.h |  1 +
 2 files changed, 72 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index fed6236..c8cd0c6 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -347,6 +347,77 @@ int spi_nor_write_cr2(struct spi_nor *nor, u32 addr, u8 *cr2)
 }
 
 /**
+ * spi_nor_cmd_seq_octal_dtr() - command sequences to change to octal DTR mode
+ * @nor:	pointer to 'struct spi_nor'.
+ * @enable:	enable Octal DTR.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+int spi_nor_cmd_seq_octal_dtr(struct spi_nor *nor, bool enable)
+{
+	struct spi_nor_flash_parameter *p = nor->params;
+	struct cmd_seq_octal_dtr *cs = p->cmd_seq;
+	int i, ret;
+	struct spi_mem_op op;
+
+	if (!nor->spimem || !p->cmd_seq[0].len)
+		return -ENOTSUPP;
+
+	if (!enable)
+		return 0;
+
+	for (i = 0; i < CMD_SEQ_NUM; i++) {
+		switch (p->cmd_seq[i].len) {
+		case 1:
+			op = (struct spi_mem_op)
+				SPI_MEM_OP(SPI_MEM_OP_CMD(cs[i].opcode, 1),
+					   SPI_MEM_OP_NO_ADDR,
+					   SPI_MEM_OP_NO_DUMMY,
+					   SPI_MEM_OP_NO_DATA);
+
+			ret = spi_mem_exec_op(nor->spimem, &op);
+			if (ret)
+				return ret;
+			break;
+
+		case 3:
+			op = (struct spi_mem_op)
+				SPI_MEM_OP(SPI_MEM_OP_CMD(cs[i].opcode, 1),
+					   SPI_MEM_OP_ADDR(1, cs[i].addr, 1),
+					   SPI_MEM_OP_NO_DUMMY,
+					   SPI_MEM_OP_DATA_OUT(1,
+							       &cs[i].data, 1));
+
+			ret = spi_mem_exec_op(nor->spimem, &op);
+			if (ret)
+				return ret;
+			break;
+
+		case 6:
+			op = (struct spi_mem_op)
+				SPI_MEM_OP(SPI_MEM_OP_CMD(cs[i].opcode, 1),
+					   SPI_MEM_OP_ADDR(4, cs[i].addr, 1),
+					   SPI_MEM_OP_NO_DUMMY,
+					   SPI_MEM_OP_DATA_OUT(1,
+							       &cs[i].data, 1));
+
+			ret = spi_mem_exec_op(nor->spimem, &op);
+			if (ret)
+				return ret;
+			break;
+
+		default:
+			dev_err(nor->dev,
+				"Error %d sequences to Octal DTR\n",
+				p->cmd_seq[i].len);
+			break;
+		}
+	}
+
+	return ret;
+}
+
+/**
  * spi_nor_read_sr() - Read the Status Register.
  * @nor:	pointer to 'struct spi_nor'.
  * @sr:		pointer to a DMA-able buffer where the value of the
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 0eb07ca..e4cf20a 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -436,6 +436,7 @@ struct spi_nor_manufacturer {
 int spi_nor_write_disable(struct spi_nor *nor);
 int spi_nor_read_cr2(struct spi_nor *nor, u32 addr, u8 *cr2);
 int spi_nor_write_cr2(struct spi_nor *nor, u32 addr, u8 *cr2);
+int spi_nor_cmd_seq_octal_dtr(struct spi_nor *nor, bool enable);
 int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable);
 int spi_nor_write_ear(struct spi_nor *nor, u8 ear);
 int spi_nor_wait_till_ready(struct spi_nor *nor);
-- 
1.9.1

[PATCH v4 6/7] spi: mxic: patch for octal DTR mode support

[Mason Yang]

Driver patch for octal 8D-8D-8D mode support.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/spi/spi-mxic.c | 98 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 66 insertions(+), 32 deletions(-)

diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
index 69491f3..a9b3817 100644
--- a/drivers/spi/spi-mxic.c
+++ b/drivers/spi/spi-mxic.c
@@ -280,10 +280,55 @@ static void mxic_spi_hw_init(struct mxic_spi *mxic)
 	       mxic->regs + HC_CFG);
 }
 
+static u32 mxic_spi_mem_prep_op_cfg(const struct spi_mem_op *op)
+{
+	u32 cfg =  OP_CMD_BYTES(op->cmd.nbytes) |
+		   OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) |
+		   (op->cmd.dtr ? OP_CMD_DDR : 0);
+
+	if (op->addr.nbytes)
+		cfg |= OP_ADDR_BYTES(op->addr.nbytes) |
+		       OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) |
+		       (op->addr.dtr ? OP_ADDR_DDR : 0);
+
+	if (op->dummy.nbytes)
+		cfg |= OP_DUMMY_CYC(op->dummy.nbytes);
+
+	if (op->data.nbytes) {
+		cfg |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) |
+		      (op->data.dtr ? OP_DATA_DDR : 0);
+		if (op->data.dir == SPI_MEM_DATA_IN)
+			cfg |= OP_READ;
+	}
+
+	return cfg;
+}
+
+static void mxic_spi_set_hc_cfg(struct spi_device *spi, u32 flags)
+{
+	struct mxic_spi *mxic = spi_master_get_devdata(spi->master);
+	int nio = 1;
+
+	if (spi->mode & (SPI_RX_OCTAL | SPI_TX_OCTAL))
+		nio = 8;
+	else if (spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD))
+		nio = 4;
+	else if (spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL))
+		nio = 2;
+
+	writel(flags | HC_CFG_NIO(nio) |
+	       HC_CFG_TYPE(spi->chip_select, HC_CFG_TYPE_SPI_NOR) |
+	       HC_CFG_SLV_ACT(spi->chip_select) | HC_CFG_IDLE_SIO_LVL(1),
+	       mxic->regs + HC_CFG);
+}
+
 static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
 			      void *rxbuf, unsigned int len)
 {
 	unsigned int pos = 0;
+	bool dtr_enabled;
+
+	dtr_enabled = (readl(mxic->regs + SS_CTRL(0)) & OP_DATA_DDR);
 
 	while (pos < len) {
 		unsigned int nbytes = len - pos;
@@ -302,6 +347,9 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
 		if (ret)
 			return ret;
 
+		if (dtr_enabled && len & 0x1)
+			nbytes++;
+
 		writel(data, mxic->regs + TXD(nbytes % 4));
 
 		if (rxbuf) {
@@ -319,6 +367,8 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
 
 			data = readl(mxic->regs + RXD);
 			data >>= (8 * (4 - nbytes));
+			if (dtr_enabled && len & 0x1)
+				nbytes++;
 			memcpy(rxbuf + pos, &data, nbytes);
 			WARN_ON(readl(mxic->regs + INT_STS) & INT_RX_NOT_EMPTY);
 		} else {
@@ -335,8 +385,8 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
 static bool mxic_spi_mem_supports_op(struct spi_mem *mem,
 				     const struct spi_mem_op *op)
 {
-	if (op->data.buswidth > 4 || op->addr.buswidth > 4 ||
-	    op->dummy.buswidth > 4 || op->cmd.buswidth > 4)
+	if (op->data.buswidth > 8 || op->addr.buswidth > 8 ||
+	    op->dummy.buswidth > 8 || op->cmd.buswidth > 8)
 		return false;
 
 	if (op->data.nbytes && op->dummy.nbytes &&
@@ -346,6 +396,9 @@ static bool mxic_spi_mem_supports_op(struct spi_mem *mem,
 	if (op->addr.nbytes > 7)
 		return false;
 
+	if (op->cmd.buswidth == 8 && op->cmd.nbytes == 2)
+		return true;
+
 	return spi_mem_default_supports_op(mem, op);
 }
 
@@ -353,47 +406,27 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem,
 				const struct spi_mem_op *op)
 {
 	struct mxic_spi *mxic = spi_master_get_devdata(mem->spi->master);
-	int nio = 1, i, ret;
-	u32 ss_ctrl;
-	u8 addr[8];
+	int i, ret;
+	u8 addr[8], cmd[2];
 
 	ret = mxic_spi_set_freq(mxic, mem->spi->max_speed_hz);
 	if (ret)
 		return ret;
 
-	if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD))
-		nio = 4;
-	else if (mem->spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL))
-		nio = 2;
+	mxic_spi_set_hc_cfg(mem->spi, HC_CFG_MAN_CS_EN);
 
-	writel(HC_CFG_NIO(nio) |
-	       HC_CFG_TYPE(mem->spi->chip_select, HC_CFG_TYPE_SPI_NOR) |
-	       HC_CFG_SLV_ACT(mem->spi->chip_select) | HC_CFG_IDLE_SIO_LVL(1) |
-	       HC_CFG_MAN_CS_EN,
-	       mxic->regs + HC_CFG);
 	writel(HC_EN_BIT, mxic->regs + HC_EN);
 
-	ss_ctrl = OP_CMD_BYTES(1) | OP_CMD_BUSW(fls(op->cmd.buswidth) - 1);
-
-	if (op->addr.nbytes)
-		ss_ctrl |= OP_ADDR_BYTES(op->addr.nbytes) |
-			   OP_ADDR_BUSW(fls(op->addr.buswidth) - 1);
-
-	if (op->dummy.nbytes)
-		ss_ctrl |= OP_DUMMY_CYC(op->dummy.nbytes);
-
-	if (op->data.nbytes) {
-		ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1);
-		if (op->data.dir == SPI_MEM_DATA_IN)
-			ss_ctrl |= OP_READ;
-	}
-
-	writel(ss_ctrl, mxic->regs + SS_CTRL(mem->spi->chip_select));
+	writel(mxic_spi_mem_prep_op_cfg(op),
+	       mxic->regs + SS_CTRL(mem->spi->chip_select));
 
 	writel(readl(mxic->regs + HC_CFG) | HC_CFG_MAN_CS_ASSERT,
 	       mxic->regs + HC_CFG);
 
-	ret = mxic_spi_data_xfer(mxic, &op->cmd.opcode, NULL, 1);
+	for (i = 0; i < op->cmd.nbytes; i++)
+		cmd[i] = op->cmd.opcode >> (8 * (op->cmd.nbytes - i - 1));
+
+	ret = mxic_spi_data_xfer(mxic, cmd, NULL, op->cmd.nbytes);
 	if (ret)
 		goto out;
 
@@ -566,7 +599,8 @@ static int mxic_spi_probe(struct platform_device *pdev)
 	master->bits_per_word_mask = SPI_BPW_MASK(8);
 	master->mode_bits = SPI_CPOL | SPI_CPHA |
 			SPI_RX_DUAL | SPI_TX_DUAL |
-			SPI_RX_QUAD | SPI_TX_QUAD;
+			SPI_RX_QUAD | SPI_TX_QUAD |
+			SPI_RX_OCTAL | SPI_TX_OCTAL;
 
 	mxic_spi_hw_init(mxic);
 
-- 
1.9.1

[PATCH v4 7/7] mtd: spi-nor: macronix: Add Octal 8D-8D-8D supports for Macronix mx25uw51245g

[Mason Yang]

Macronix mx25uw51245g is a SPI NOR that supports 1-1-1/8-8-8 mode.

Correct the dummy cycles to device for various frequencies
after xSPI profile 1.0 table parsed.

Enable mx25uw51245g to Octal DTR mode by executing the command sequences
to change to octal DTR mode.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/mtd/spi-nor/macronix.c | 55 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 96735d8..6c9a24c 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -8,6 +8,57 @@
 
 #include "core.h"
 
+#define MXIC_CR2_DUMMY_SET_ADDR 0x300
+
+/* Fixup the dummy cycles to device and setup octa_dtr_enable() */
+static void mx25uw51245g_post_sfdp_fixups(struct spi_nor *nor)
+{
+	struct spi_nor_flash_parameter *params = nor->params;
+	int ret;
+	u8 rdc, wdc;
+
+	ret = spi_nor_read_cr2(nor, MXIC_CR2_DUMMY_SET_ADDR, &rdc);
+	if (ret)
+		return;
+
+	/* Refer to dummy cycle and frequency table(MHz) */
+	switch (params->dummy_cycles) {
+	case 10:	/* 10 dummy cycles for 104 MHz */
+		wdc = 5;
+		break;
+	case 12:	/* 12 dummy cycles for 133 MHz */
+		wdc = 4;
+		break;
+	case 16:	/* 16 dummy cycles for 166 MHz */
+		wdc = 2;
+		break;
+	case 18:	/* 18 dummy cycles for 173 MHz */
+		wdc = 1;
+		break;
+	case 20:	/* 20 dummy cycles for 200 MHz */
+	default:
+		wdc = 0;
+	}
+
+	if (rdc != wdc)
+		spi_nor_write_cr2(nor, MXIC_CR2_DUMMY_SET_ADDR, &wdc);
+
+	if (params->cmd_seq[0].len) {
+		params->octal_dtr_enable = spi_nor_cmd_seq_octal_dtr;
+		params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR;
+		params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR;
+
+	} else {
+		params->octal_dtr_enable = NULL;
+		params->hwcaps.mask &= ~SNOR_HWCAPS_READ_8_8_8_DTR;
+		params->hwcaps.mask &= ~SNOR_HWCAPS_PP_8_8_8_DTR;
+	}
+}
+
+static struct spi_nor_fixups mx25uw51245g_fixups = {
+	.post_sfdp = mx25uw51245g_post_sfdp_fixups,
+};
+
 static int
 mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
 			    const struct sfdp_parameter_header *bfpt_header,
@@ -84,6 +135,10 @@
 			      SPI_NOR_QUAD_READ) },
 	{ "mx66l1g55g",  INFO(0xc2261b, 0, 64 * 1024, 2048,
 			      SPI_NOR_QUAD_READ) },
+	{ "mx25uw51245g", INFO(0xc2813a, 0, 64 * 1024, 1024,
+			      SECT_4K | SPI_NOR_4B_OPCODES |
+			      SPI_NOR_OCTAL_DTR_READ)
+			      .fixups = &mx25uw51245g_fixups },
 };
 
 static void macronix_default_init(struct spi_nor *nor)
-- 
1.9.1

Re: [PATCH v4 0/7] mtd: spi-nor: add xSPI Octal DTR support

[Pratyush Yadav]

Hi Mason,

On 29/05/20 03:36PM, Mason Yang wrote:
> Mason Yang (7):
>   mtd: spi-nor: sfdp: get octal mode maximum speed from BFPT
>   mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table

I think you've misunderstood what Boris meant. The idea of specifying a 
dependency on a patch set is assuming that entire series is already 
applied to the main branch, and then creating patches on top of those.

So, for example, you can checkout the 'master' branch of linux-next. 
Then apply all 19 of my patches on top of that branch by using 'git am'. 
Then you add your work there and separate it out on a set of commits.

If you'd to that, you won't need to add a separate patch for parsing the 
Profile 1.0 table because I've already done that in my series. All you 
need to do is add the extra things, like adding parsing of a DWORD that 
my series doesn't, or adding support for your Macronix flash.

>   mtd: spi-nor: sfdp: parse command sequences to change octal DTR mode
>   mtd: spi-nor: core: add configuration register 2 read & write support
>   mtd: spi-nor: core: execute command sequences to change octal DTR mode
>   spi: mxic: patch for octal DTR mode support
>   mtd: spi-nor: macronix: Add Octal 8D-8D-8D supports for Macronix
>     mx25uw51245g
> 
>  drivers/mtd/spi-nor/core.c     | 151 ++++++++++++++++++++++++-
>  drivers/mtd/spi-nor/core.h     |  41 +++++++
>  drivers/mtd/spi-nor/macronix.c |  55 +++++++++
>  drivers/mtd/spi-nor/sfdp.c     | 246 +++++++++++++++++++++++++++++++++++++++++
>  drivers/mtd/spi-nor/sfdp.h     |   4 +
>  drivers/spi/spi-mxic.c         |  98 ++++++++++------
>  6 files changed, 562 insertions(+), 33 deletions(-)
> 

-- 
Regards,
Pratyush Yadav

Re: [PATCH v4 1/7] mtd: spi-nor: sfdp: get octal mode maximum speed from BFPT

[Pratyush Yadav]

On 29/05/20 03:36PM, Mason Yang wrote:
> Get maximum operation speed of device in octal mode from
> BFPT 20th DWORD.

I don't like the idea of getting the maximum operation speed from BFPT 
when the Profile 1.0 table already tells us that. For example, the 
200MHz operation dummy cycles field in the 4th DWORD says that a value 
of 0 means the frequency is not supported. So the Profile 1.0 table 
already tells us what frequencies the flash can run at in xSPI Profile 
1.0 mode.

So IMO we should use the Profile 1.0 table for this instead because
it will be a localized change which is easier to maintain. I also get 
the feeling it will be less prone to mis-interpretations.
 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/core.h |  2 ++
>  drivers/mtd/spi-nor/sfdp.c | 36 ++++++++++++++++++++++++++++++++++++
>  drivers/mtd/spi-nor/sfdp.h |  4 ++++
>  3 files changed, 42 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 6f2f6b2..7a36b22 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -190,6 +190,7 @@ struct spi_nor_locking_ops {
>   *
>   * @size:		the flash memory density in bytes.
>   * @page_size:		the page size of the SPI NOR flash memory.
> + * @octal_max_speed:	maximum operation speed of device in octal mode.
>   * @hwcaps:		describes the read and page program hardware
>   *			capabilities.
>   * @reads:		read capabilities ordered by priority: the higher index
> @@ -212,6 +213,7 @@ struct spi_nor_locking_ops {
>  struct spi_nor_flash_parameter {
>  	u64				size;
>  	u32				page_size;
> +	u16				octal_max_speed;
>  
>  	struct spi_nor_hwcaps		hwcaps;
>  	struct spi_nor_read_command	reads[SNOR_CMD_READ_MAX];
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index f6038d3..4d13f66 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -4,6 +4,7 @@
>   * Copyright (C) 2014, Freescale Semiconductor, Inc.
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/slab.h>
>  #include <linux/sort.h>
>  #include <linux/mtd/spi-nor.h>
> @@ -26,6 +27,12 @@
>  #define SFDP_JESD216A_MINOR	5
>  #define SFDP_JESD216B_MINOR	6
>  
> +/* Basic Flash Parameter Table 20th DWORD, Max operation speed of device */
> +struct octal_max_speed {
> +	u8 idx; /* Bits value */
> +	u16 hz; /* MHz */
> +};
> +
>  struct sfdp_header {
>  	u32		signature; /* Ox50444653U <=> "SFDP" */
>  	u8		minor;
> @@ -440,6 +447,22 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  	u32 addr;
>  	u16 half;
>  	u8 erase_mask;
> +	static const struct octal_max_speed max_hz[] = {
> +		/* Bits value, MHz */
> +		{ 0x0c, 400 },
> +		{ 0x0b, 333 },
> +		{ 0x0a, 266 },
> +		{ 0x09, 250 },
> +		{ 0x08, 200 },
> +		{ 0x07, 166 },
> +		{ 0x06, 133 },
> +		{ 0x05, 100 },
> +		{ 0x04,  80 },
> +		{ 0x03,  66 },
> +		{ 0x02,  50 },
> +		{ 0x01,  33 },
> +	};
> +	u8 idx;
>  
>  	/* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs. */
>  	if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
> @@ -604,6 +627,19 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  		return -EINVAL;
>  	}
>  
> +	/* Octal mode max speed */
> +	idx = max(FIELD_GET(BFPT_DWORD20_OCTAL_DTR_MAX_SPEED,
> +			    bfpt.dwords[BFPT_DWORD(20)]),
> +		  FIELD_GET(BFPT_DWORD20_OCTAL_STR_MAX_SPEED,
> +			    bfpt.dwords[BFPT_DWORD(20)]));
> +
> +	for (i = 0; i < ARRAY_SIZE(max_hz); i++) {
> +		if (max_hz[i].idx == idx) {
> +			params->octal_max_speed = max_hz[i].hz;
> +			break;
> +		}
> +	}
> +
>  	return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt, params);
>  }
>  
> diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
> index e0a8ded..8ae6d9a 100644
> --- a/drivers/mtd/spi-nor/sfdp.h
> +++ b/drivers/mtd/spi-nor/sfdp.h
> @@ -83,6 +83,10 @@ struct sfdp_bfpt {
>  #define BFPT_DWORD15_QER_SR2_BIT1_NO_RD		(0x4UL << 20)
>  #define BFPT_DWORD15_QER_SR2_BIT1		(0x5UL << 20) /* Spansion */
>  
> +#define BFPT_DWORD20_OCTAL_MAX_SPEED_MASK       GENMASK(31, 16)
> +#define BFPT_DWORD20_OCTAL_DTR_MAX_SPEED        GENMASK(31, 28)
> +#define BFPT_DWORD20_OCTAL_STR_MAX_SPEED        GENMASK(19, 16)
> +
>  struct sfdp_parameter_header {
>  	u8		id_lsb;
>  	u8		minor;

-- 
Regards,
Pratyush Yadav

Re: [PATCH v4 2/7] mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table

[Pratyush Yadav]

On 29/05/20 03:36PM, Mason Yang wrote:
> JESD251, xSPI profile 1.0 table supports octal DTR mode.
> Extract information like the fast read opcode, dummy cycles for various
> frequencies, the number of dummy cycles needed for a Read Status
> Register command, the number of address bytes needed for a Read
> Status Register command, read volatile register command and write
> volatile register command.

Like I said before, please don't re-introduce the functions. My patch 
series already parses the table. Add whatever new feature you want on 
top of it.
 
> According to BFPT 20th DWORD of octal maximum speed, driver get it's
> specific dummy cycles from profile 1.0 table and then could update
> it to device by their fixup hooks.

My patch [0] already find out the dummy cycles from the table as you 
suggested, though it doesn't consult the BFPT. Instead, it uses the 
dummy cycle fields themselves as indication that a speed is supported or 
not.
 
[0] https://lore.kernel.org/linux-mtd/20200525091544.17270-10-p.yadav@ti.com/

-- 
Regards,
Pratyush Yadav

Re: [PATCH v4 7/7] mtd: spi-nor: macronix: Add Octal 8D-8D-8D supports for Macronix mx25uw51245g

[Pratyush Yadav]

On 29/05/20 03:36PM, Mason Yang wrote:
> Macronix mx25uw51245g is a SPI NOR that supports 1-1-1/8-8-8 mode.
> 
> Correct the dummy cycles to device for various frequencies
> after xSPI profile 1.0 table parsed.
> 
> Enable mx25uw51245g to Octal DTR mode by executing the command sequences
> to change to octal DTR mode.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/macronix.c | 55 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 96735d8..6c9a24c 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -8,6 +8,57 @@
>  
>  #include "core.h"
>  
> +#define MXIC_CR2_DUMMY_SET_ADDR 0x300
> +
> +/* Fixup the dummy cycles to device and setup octa_dtr_enable() */
> +static void mx25uw51245g_post_sfdp_fixups(struct spi_nor *nor)
> +{
> +	struct spi_nor_flash_parameter *params = nor->params;
> +	int ret;
> +	u8 rdc, wdc;
> +
> +	ret = spi_nor_read_cr2(nor, MXIC_CR2_DUMMY_SET_ADDR, &rdc);
> +	if (ret)
> +		return;
> +
> +	/* Refer to dummy cycle and frequency table(MHz) */
> +	switch (params->dummy_cycles) {
> +	case 10:	/* 10 dummy cycles for 104 MHz */
> +		wdc = 5;
> +		break;
> +	case 12:	/* 12 dummy cycles for 133 MHz */
> +		wdc = 4;
> +		break;
> +	case 16:	/* 16 dummy cycles for 166 MHz */
> +		wdc = 2;
> +		break;
> +	case 18:	/* 18 dummy cycles for 173 MHz */
> +		wdc = 1;
> +		break;
> +	case 20:	/* 20 dummy cycles for 200 MHz */
> +	default:
> +		wdc = 0;
> +	}

I don't get the point of this. You already know the fastest the 
mx25uw51245g flash can run at. Why not just use the maximum dummy 
cycles? SPI NOR doesn't know the speed the controller is running at so 
the best it can do is use the maximum dummy cycles possible so it never 
falls short. Sure, it will be _slightly_ less performance, but we will 
be sure to read the correct data, which is much much more important.

Is it possible to have two chips which have _exactly_ the same ID but 
one supports say 200MHz frequency but the other doesn't? Without that, 
we can just enable the maximum and move on.

> +
> +	if (rdc != wdc)
> +		spi_nor_write_cr2(nor, MXIC_CR2_DUMMY_SET_ADDR, &wdc);
> +
> +	if (params->cmd_seq[0].len) {
> +		params->octal_dtr_enable = spi_nor_cmd_seq_octal_dtr;
> +		params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR;
> +		params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR;

Same comment as above. We are in the mx25uw51245g fixup hook. We already 
know if the flash supports 8D mode or not from the datasheet. What is 
the need to discover it from SFDP?

> +
> +	} else {
> +		params->octal_dtr_enable = NULL;
> +		params->hwcaps.mask &= ~SNOR_HWCAPS_READ_8_8_8_DTR;
> +		params->hwcaps.mask &= ~SNOR_HWCAPS_PP_8_8_8_DTR;
> +	}
> +}
> +
> +static struct spi_nor_fixups mx25uw51245g_fixups = {
> +	.post_sfdp = mx25uw51245g_post_sfdp_fixups,
> +};
> +
>  static int
>  mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
>  			    const struct sfdp_parameter_header *bfpt_header,
> @@ -84,6 +135,10 @@
>  			      SPI_NOR_QUAD_READ) },
>  	{ "mx66l1g55g",  INFO(0xc2261b, 0, 64 * 1024, 2048,
>  			      SPI_NOR_QUAD_READ) },
> +	{ "mx25uw51245g", INFO(0xc2813a, 0, 64 * 1024, 1024,
> +			      SECT_4K | SPI_NOR_4B_OPCODES |
> +			      SPI_NOR_OCTAL_DTR_READ)
> +			      .fixups = &mx25uw51245g_fixups },
>  };
>  
>  static void macronix_default_init(struct spi_nor *nor)

-- 
Regards,
Pratyush Yadav

Re: [PATCH v4 1/7] mtd: spi-nor: sfdp: get octal mode maximum speed from BFPT

[masonccyang]

Hi Pratyush,

> 
> Subject
> 
> Re: [PATCH v4 1/7] mtd: spi-nor: sfdp: get octal mode maximum speed from 
BFPT
> 
> On 29/05/20 03:36PM, Mason Yang wrote:
> > Get maximum operation speed of device in octal mode from
> > BFPT 20th DWORD.
> 
> I don't like the idea of getting the maximum operation speed from BFPT 
> when the Profile 1.0 table already tells us that. For example, the 
> 200MHz operation dummy cycles field in the 4th DWORD says that a value 
> of 0 means the frequency is not supported. So the Profile 1.0 table 
> already tells us what frequencies the flash can run at in xSPI Profile 
> 1.0 mode.

As JEDEC spec. mentioned that 
Operation faster than 200MHz is not part of the current xSPI Spec. 
However, this does not prevent vendors from making devices that operate at 
higher speed.

In addition, current xSPP profile 1.0 table only defined up to 200MHz.

> 
> So IMO we should use the Profile 1.0 table for this instead because
> it will be a localized change which is easier to maintain. I also get 
> the feeling it will be less prone to mis-interpretations.
> 
> > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> > ---

thanks for your time & comments.
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

Re: [PATCH v4 7/7] mtd: spi-nor: macronix: Add Octal 8D-8D-8D supports for Macronix mx25uw51245g

[masonccyang]

Hi Pratyush,

 
> Subject
> 
> Re: [PATCH v4 7/7] mtd: spi-nor: macronix: Add Octal 8D-8D-8D supports 
for 
> Macronix mx25uw51245g
> 
> On 29/05/20 03:36PM, Mason Yang wrote:
> > Macronix mx25uw51245g is a SPI NOR that supports 1-1-1/8-8-8 mode.
> > 
> > Correct the dummy cycles to device for various frequencies
> > after xSPI profile 1.0 table parsed.
> > 
> > Enable mx25uw51245g to Octal DTR mode by executing the command 
sequences
> > to change to octal DTR mode.
> > 
> > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> > ---
> >  drivers/mtd/spi-nor/macronix.c | 55 
++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 55 insertions(+)
> > 
> > diff --git a/drivers/mtd/spi-nor/macronix.c 
b/drivers/mtd/spi-nor/macronix.c
> > index 96735d8..6c9a24c 100644
> > --- a/drivers/mtd/spi-nor/macronix.c
> > +++ b/drivers/mtd/spi-nor/macronix.c
> > @@ -8,6 +8,57 @@
> > 
> >  #include "core.h"
> > 
> > +#define MXIC_CR2_DUMMY_SET_ADDR 0x300
> > +
> > +/* Fixup the dummy cycles to device and setup octa_dtr_enable() */
> > +static void mx25uw51245g_post_sfdp_fixups(struct spi_nor *nor)
> > +{
> > +   struct spi_nor_flash_parameter *params = nor->params;
> > +   int ret;
> > +   u8 rdc, wdc;
> > +
> > +   ret = spi_nor_read_cr2(nor, MXIC_CR2_DUMMY_SET_ADDR, &rdc);
> > +   if (ret)
> > +      return;
> > +
> > +   /* Refer to dummy cycle and frequency table(MHz) */
> > +   switch (params->dummy_cycles) {
> > +   case 10:   /* 10 dummy cycles for 104 MHz */
> > +      wdc = 5;
> > +      break;
> > +   case 12:   /* 12 dummy cycles for 133 MHz */
> > +      wdc = 4;
> > +      break;
> > +   case 16:   /* 16 dummy cycles for 166 MHz */
> > +      wdc = 2;
> > +      break;
> > +   case 18:   /* 18 dummy cycles for 173 MHz */
> > +      wdc = 1;
> > +      break;
> > +   case 20:   /* 20 dummy cycles for 200 MHz */
> > +   default:
> > +      wdc = 0;
> > +   }
> 
> I don't get the point of this. You already know the fastest the 
> mx25uw51245g flash can run at. Why not just use the maximum dummy 
> cycles? SPI NOR doesn't know the speed the controller is running at so 
> the best it can do is use the maximum dummy cycles possible so it never 
> falls short. Sure, it will be _slightly_ less performance, but we will 
> be sure to read the correct data, which is much much more important.

In general, 200MHz needs 20 dummy cycles but some powerful device may only 

needs 18 dummy cycles or less.

Set a proper dummy cycles for a better performance.

> 
> Is it possible to have two chips which have _exactly_ the same ID but 
> one supports say 200MHz frequency but the other doesn't? Without that, 
> we can just enable the maximum and move on.
> 

thanks for your time & comments.
Mason



CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

Re: [PATCH v4 7/7] mtd: spi-nor: macronix: Add Octal 8D-8D-8D supports for Macronix mx25uw51245g

[Pratyush Yadav]

On 02/06/20 02:44PM, masonccyang@mxic.com.tw wrote:
> 
> Hi Pratyush,
> 
>  
> > Subject
> > 
> > Re: [PATCH v4 7/7] mtd: spi-nor: macronix: Add Octal 8D-8D-8D supports 
> for 
> > Macronix mx25uw51245g
> > 
> > On 29/05/20 03:36PM, Mason Yang wrote:
> > > Macronix mx25uw51245g is a SPI NOR that supports 1-1-1/8-8-8 mode.
> > > 
> > > Correct the dummy cycles to device for various frequencies
> > > after xSPI profile 1.0 table parsed.
> > > 
> > > Enable mx25uw51245g to Octal DTR mode by executing the command 
> sequences
> > > to change to octal DTR mode.
> > > 
> > > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> > > ---
> > >  drivers/mtd/spi-nor/macronix.c | 55 
> ++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 55 insertions(+)
> > > 
> > > diff --git a/drivers/mtd/spi-nor/macronix.c 
> b/drivers/mtd/spi-nor/macronix.c
> > > index 96735d8..6c9a24c 100644
> > > --- a/drivers/mtd/spi-nor/macronix.c
> > > +++ b/drivers/mtd/spi-nor/macronix.c
> > > @@ -8,6 +8,57 @@
> > > 
> > >  #include "core.h"
> > > 
> > > +#define MXIC_CR2_DUMMY_SET_ADDR 0x300
> > > +
> > > +/* Fixup the dummy cycles to device and setup octa_dtr_enable() */
> > > +static void mx25uw51245g_post_sfdp_fixups(struct spi_nor *nor)
> > > +{
> > > +   struct spi_nor_flash_parameter *params = nor->params;
> > > +   int ret;
> > > +   u8 rdc, wdc;
> > > +
> > > +   ret = spi_nor_read_cr2(nor, MXIC_CR2_DUMMY_SET_ADDR, &rdc);
> > > +   if (ret)
> > > +      return;
> > > +
> > > +   /* Refer to dummy cycle and frequency table(MHz) */
> > > +   switch (params->dummy_cycles) {
> > > +   case 10:   /* 10 dummy cycles for 104 MHz */
> > > +      wdc = 5;
> > > +      break;
> > > +   case 12:   /* 12 dummy cycles for 133 MHz */
> > > +      wdc = 4;
> > > +      break;
> > > +   case 16:   /* 16 dummy cycles for 166 MHz */
> > > +      wdc = 2;
> > > +      break;
> > > +   case 18:   /* 18 dummy cycles for 173 MHz */
> > > +      wdc = 1;
> > > +      break;
> > > +   case 20:   /* 20 dummy cycles for 200 MHz */
> > > +   default:
> > > +      wdc = 0;
> > > +   }
> > 
> > I don't get the point of this. You already know the fastest the 
> > mx25uw51245g flash can run at. Why not just use the maximum dummy 
> > cycles? SPI NOR doesn't know the speed the controller is running at so 
> > the best it can do is use the maximum dummy cycles possible so it never 
> > falls short. Sure, it will be _slightly_ less performance, but we will 
> > be sure to read the correct data, which is much much more important.
> 
> In general, 200MHz needs 20 dummy cycles but some powerful device may only 
> 
> needs 18 dummy cycles or less.

Yes, but do different mx25uw51245g chips have different dummy cycle 
requirements? Shouldn't all the chips with the same ID have same 
performance?

This is a fixup hook for mx25uw51245g, so you should already know how 
many cycles are needed for this specific device. Is there any need for 
generic code here?
 
> Set a proper dummy cycles for a better performance.
> 
> > 
> > Is it possible to have two chips which have _exactly_ the same ID but 
> > one supports say 200MHz frequency but the other doesn't? Without that, 
> > we can just enable the maximum and move on.
> > 
> 

-- 
Regards,
Pratyush Yadav

Re: [PATCH v4 7/7] mtd: spi-nor: macronix: Add Octal 8D-8D-8D supports for Macronix mx25uw51245g

[masonccyang]

> > > > 
> > > > +#define MXIC_CR2_DUMMY_SET_ADDR 0x300
> > > > +
> > > > +/* Fixup the dummy cycles to device and setup octa_dtr_enable() 
*/
> > > > +static void mx25uw51245g_post_sfdp_fixups(struct spi_nor *nor)
> > > > +{
> > > > +   struct spi_nor_flash_parameter *params = nor->params;
> > > > +   int ret;
> > > > +   u8 rdc, wdc;
> > > > +
> > > > +   ret = spi_nor_read_cr2(nor, MXIC_CR2_DUMMY_SET_ADDR, &rdc);
> > > > +   if (ret)
> > > > +      return;
> > > > +
> > > > +   /* Refer to dummy cycle and frequency table(MHz) */
> > > > +   switch (params->dummy_cycles) {
> > > > +   case 10:   /* 10 dummy cycles for 104 MHz */
> > > > +      wdc = 5;
> > > > +      break;
> > > > +   case 12:   /* 12 dummy cycles for 133 MHz */
> > > > +      wdc = 4;
> > > > +      break;
> > > > +   case 16:   /* 16 dummy cycles for 166 MHz */
> > > > +      wdc = 2;
> > > > +      break;
> > > > +   case 18:   /* 18 dummy cycles for 173 MHz */
> > > > +      wdc = 1;
> > > > +      break;
> > > > +   case 20:   /* 20 dummy cycles for 200 MHz */
> > > > +   default:
> > > > +      wdc = 0;
> > > > +   }
> > > 
> > > I don't get the point of this. You already know the fastest the 
> > > mx25uw51245g flash can run at. Why not just use the maximum dummy 
> > > cycles? SPI NOR doesn't know the speed the controller is running at 
so 
> > > the best it can do is use the maximum dummy cycles possible so it 
never 
> > > falls short. Sure, it will be _slightly_ less performance, but we 
will 
> > > be sure to read the correct data, which is much much more important.
> > 
> > In general, 200MHz needs 20 dummy cycles but some powerful device may 
only 
> > 
> > needs 18 dummy cycles or less.
> 
> Yes, but do different mx25uw51245g chips have different dummy cycle 
> requirements? Shouldn't all the chips with the same ID have same 
> performance?
> 

Same chip ID but different grade,
i.e., commercial or industrial grade. 

thanks & best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

Re: [PATCH v4 7/7] mtd: spi-nor: macronix: Add Octal 8D-8D-8D supports for Macronix mx25uw51245g

[Pratyush Yadav]

On 05/06/20 10:53AM, masonccyang@mxic.com.tw wrote:
> 
> > > > > 
> > > > > +#define MXIC_CR2_DUMMY_SET_ADDR 0x300
> > > > > +
> > > > > +/* Fixup the dummy cycles to device and setup octa_dtr_enable() 
> */
> > > > > +static void mx25uw51245g_post_sfdp_fixups(struct spi_nor *nor)
> > > > > +{
> > > > > +   struct spi_nor_flash_parameter *params = nor->params;
> > > > > +   int ret;
> > > > > +   u8 rdc, wdc;
> > > > > +
> > > > > +   ret = spi_nor_read_cr2(nor, MXIC_CR2_DUMMY_SET_ADDR, &rdc);
> > > > > +   if (ret)
> > > > > +      return;
> > > > > +
> > > > > +   /* Refer to dummy cycle and frequency table(MHz) */
> > > > > +   switch (params->dummy_cycles) {
> > > > > +   case 10:   /* 10 dummy cycles for 104 MHz */
> > > > > +      wdc = 5;
> > > > > +      break;
> > > > > +   case 12:   /* 12 dummy cycles for 133 MHz */
> > > > > +      wdc = 4;
> > > > > +      break;
> > > > > +   case 16:   /* 16 dummy cycles for 166 MHz */
> > > > > +      wdc = 2;
> > > > > +      break;
> > > > > +   case 18:   /* 18 dummy cycles for 173 MHz */
> > > > > +      wdc = 1;
> > > > > +      break;
> > > > > +   case 20:   /* 20 dummy cycles for 200 MHz */
> > > > > +   default:
> > > > > +      wdc = 0;
> > > > > +   }
> > > > 
> > > > I don't get the point of this. You already know the fastest the 
> > > > mx25uw51245g flash can run at. Why not just use the maximum dummy 
> > > > cycles? SPI NOR doesn't know the speed the controller is running at 
> so 
> > > > the best it can do is use the maximum dummy cycles possible so it 
> never 
> > > > falls short. Sure, it will be _slightly_ less performance, but we 
> will 
> > > > be sure to read the correct data, which is much much more important.
> > > 
> > > In general, 200MHz needs 20 dummy cycles but some powerful device may 
> only 
> > > 
> > > needs 18 dummy cycles or less.
> > 
> > Yes, but do different mx25uw51245g chips have different dummy cycle 
> > requirements? Shouldn't all the chips with the same ID have same 
> > performance?
> > 
> 
> Same chip ID but different grade,
> i.e., commercial or industrial grade. 

Ok. In that case it makes sense.

-- 
Regards,
Pratyush Yadav

Re: [PATCH v4 0/7] mtd: spi-nor: add xSPI Octal DTR support

[masonccyang]

+ YC Lin in loop,

YC(ycllin@mxic.com.tw)will take over this patch set.


> Subject
> 
> [PATCH v4 0/7] mtd: spi-nor: add xSPI Octal DTR support
> 
> Hello,
> 
> JESD216C has defined specification for Octal 8S-8S-8S and 8D-8D-8D.
> Based on JEDEC216C Basic Flash Parameter Table (BFPT) driver extract
> DWORD-18: command and command extension type.
> DWORD-20: Maximum operation speed of device in Octal mode.
> 
> xSPI profile 1.0 table:
> DWORD-1: Read Fast command, the number of dummy cycles and address 
nbytes
>     for Read Status Register command.
> DWORD-2: Read/Write volatile Register command for CFG Reg2.
> DWORD-4 and DWORD-5: dummy cycles used for various frequencies based on
> maximum speed of device from BFPT 20th DWORD.
> 
> Ccommand sequences to change to octal DTR mode:
> The length of each command sequence is 8 per byte for single SPI mode 
and
> patching driver to parse and execute these sequences for octal DTR mode.
> 
> By Vignesh's comments, this patches set is dependency on Pratyush's
> [1][2][5][7][8] and [10][12].
> 
> Tested on Macronix's Zynq PicoZed board with Macronix's SPI controller
> (spi-mxic.c) driver patched on mx25uw51245g Octal flash.
> 


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

Re: [PATCH v4 1/7] mtd: spi-nor: sfdp: get octal mode maximum speed from BFPT

[masonccyang]

+ YC Lin in loop,

> 
> Subject
> 
> [PATCH v4 1/7] mtd: spi-nor: sfdp: get octal mode maximum speed from 
BFPT
> 
> Get maximum operation speed of device in octal mode from
> BFPT 20th DWORD.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

Re: [PATCH v4 2/7] mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table

[masonccyang]

+ YC Lin in loop,

--
> 
> Subject
> 
> [PATCH v4 2/7] mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table
> 
> JESD251, xSPI profile 1.0 table supports octal DTR mode.
> Extract information like the fast read opcode, dummy cycles for various
> frequencies, the number of dummy cycles needed for a Read Status
> Register command, the number of address bytes needed for a Read
> Status Register command, read volatile register command and write
> volatile register command.
> 
> According to BFPT 20th DWORD of octal maximum speed, driver get it's
> specific dummy cycles from profile 1.0 table and then could update
> it to device by their fixup hooks.
> 
> Since driver get octal DTR read opcode and then set read settings,
> expose spi_nor_set_read_settings() in core.h.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

Re: [PATCH v4 3/7] mtd: spi-nor: sfdp: parse command sequences to change octal DTR mode

[masonccyang]

+ YC Lin in loop,

--
> Subject
> 
> [PATCH v4 3/7] mtd: spi-nor: sfdp: parse command sequences to change 
octal DTR mode
> 
> A set of simple command sequences is provided which can be executed
> directly by the host controller to enable octal DTR mode.
> 
> Each command sequence is 8 per byte for single SPI mode.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

Re: [PATCH v4 4/7] mtd: spi-nor: core: add configuration register 2 read & write support

[masonccyang]

+ YC Lin in loop,

--
> 
> Subject
> 
> [PATCH v4 4/7] mtd: spi-nor: core: add configuration register 2 read & 
write support
> 
> Configuration register 2 is to set the device operation condition like
> STR or DTR mode at address offset 0 and DQS mode at address offset 
0x200.
> 
> Each device has various address offset for it's specific operatoin
> setting.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

Re: [PATCH v4 5/7] mtd: spi-nor: core: execute command sequences to change octal DTR mode

[masonccyang]

+ YC Lin in loop,

--
> 
> Subject
> 
> [PATCH v4 5/7] mtd: spi-nor: core: execute command sequences to change 
octal DTR mode
> 
> Execute command sequences to change octal DTR mode.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

Re: [PATCH v4 6/7] spi: mxic: patch for octal DTR mode support

[masonccyang]

+ YC Lin in loop,

--
> 
> Subject
> 
> [PATCH v4 6/7] spi: mxic: patch for octal DTR mode support
> 
> Driver patch for octal 8D-8D-8D mode support.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

Re: [PATCH v4 7/7] mtd: spi-nor: macronix: Add Octal 8D-8D-8D supports for Macronix mx25uw51245g

[masonccyang]

+ YC Lin in loop,

--
> 
> Subject
> 
> [PATCH v4 7/7] mtd: spi-nor: macronix: Add Octal 8D-8D-8D supports for 
> Macronix mx25uw51245g
> 
> Macronix mx25uw51245g is a SPI NOR that supports 1-1-1/8-8-8 mode.
> 
> Correct the dummy cycles to device for various frequencies
> after xSPI profile 1.0 table parsed.
> 
> Enable mx25uw51245g to Octal DTR mode by executing the command sequences
> to change to octal DTR mode.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

Re: [PATCH v4 1/7] mtd: spi-nor: sfdp: get octal mode maximum speed from BFPT

[Tudor.Ambarus]

Hi, Mason, YC Lin,

On 5/29/20 10:36 AM, Mason Yang wrote:
> Get maximum operation speed of device in octal mode from
> BFPT 20th DWORD.
> 

I would like to understand how would we use the max speed value
at the SPI NOR level. The maximum operation speed is typically used
to determine the number of dummy cycles, which is described in xSPI
Spec for speeds of 200 MHz or less. Even if BFPT[dword20] describes
supported speeds up to 400 MHz, it doesn't indicate the number of
required dummy cycles. What number of dummy cycles would we use for
speeds higher than 200 MHz?

We may be tempted however to pass the max_speed_hz to the SPIMEM layer,
so that the controller can sync with the memory to choose the best
available speed.

Cheers,
ta

> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/core.h |  2 ++
>  drivers/mtd/spi-nor/sfdp.c | 36 ++++++++++++++++++++++++++++++++++++
>  drivers/mtd/spi-nor/sfdp.h |  4 ++++
>  3 files changed, 42 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 6f2f6b2..7a36b22 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -190,6 +190,7 @@ struct spi_nor_locking_ops {
>   *
>   * @size:		the flash memory density in bytes.
>   * @page_size:		the page size of the SPI NOR flash memory.
> + * @octal_max_speed:	maximum operation speed of device in octal mode.
>   * @hwcaps:		describes the read and page program hardware
>   *			capabilities.
>   * @reads:		read capabilities ordered by priority: the higher index
> @@ -212,6 +213,7 @@ struct spi_nor_locking_ops {
>  struct spi_nor_flash_parameter {
>  	u64				size;
>  	u32				page_size;
> +	u16				octal_max_speed;
>  
>  	struct spi_nor_hwcaps		hwcaps;
>  	struct spi_nor_read_command	reads[SNOR_CMD_READ_MAX];
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index f6038d3..4d13f66 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -4,6 +4,7 @@
>   * Copyright (C) 2014, Freescale Semiconductor, Inc.
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/slab.h>
>  #include <linux/sort.h>
>  #include <linux/mtd/spi-nor.h>
> @@ -26,6 +27,12 @@
>  #define SFDP_JESD216A_MINOR	5
>  #define SFDP_JESD216B_MINOR	6
>  
> +/* Basic Flash Parameter Table 20th DWORD, Max operation speed of device */
> +struct octal_max_speed {
> +	u8 idx; /* Bits value */
> +	u16 hz; /* MHz */
> +};
> +
>  struct sfdp_header {
>  	u32		signature; /* Ox50444653U <=> "SFDP" */
>  	u8		minor;
> @@ -440,6 +447,22 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  	u32 addr;
>  	u16 half;
>  	u8 erase_mask;
> +	static const struct octal_max_speed max_hz[] = {
> +		/* Bits value, MHz */
> +		{ 0x0c, 400 },
> +		{ 0x0b, 333 },
> +		{ 0x0a, 266 },
> +		{ 0x09, 250 },
> +		{ 0x08, 200 },
> +		{ 0x07, 166 },
> +		{ 0x06, 133 },
> +		{ 0x05, 100 },
> +		{ 0x04,  80 },
> +		{ 0x03,  66 },
> +		{ 0x02,  50 },
> +		{ 0x01,  33 },
> +	};
> +	u8 idx;
>  
>  	/* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs. */
>  	if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
> @@ -604,6 +627,19 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  		return -EINVAL;
>  	}
>  
> +	/* Octal mode max speed */
> +	idx = max(FIELD_GET(BFPT_DWORD20_OCTAL_DTR_MAX_SPEED,
> +			    bfpt.dwords[BFPT_DWORD(20)]),
> +		  FIELD_GET(BFPT_DWORD20_OCTAL_STR_MAX_SPEED,
> +			    bfpt.dwords[BFPT_DWORD(20)]));
> +
> +	for (i = 0; i < ARRAY_SIZE(max_hz); i++) {
> +		if (max_hz[i].idx == idx) {
> +			params->octal_max_speed = max_hz[i].hz;
> +			break;
> +		}
> +	}
> +
>  	return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt, params);
>  }
>  
> diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
> index e0a8ded..8ae6d9a 100644
> --- a/drivers/mtd/spi-nor/sfdp.h
> +++ b/drivers/mtd/spi-nor/sfdp.h
> @@ -83,6 +83,10 @@ struct sfdp_bfpt {
>  #define BFPT_DWORD15_QER_SR2_BIT1_NO_RD		(0x4UL << 20)
>  #define BFPT_DWORD15_QER_SR2_BIT1		(0x5UL << 20) /* Spansion */
>  
> +#define BFPT_DWORD20_OCTAL_MAX_SPEED_MASK       GENMASK(31, 16)
> +#define BFPT_DWORD20_OCTAL_DTR_MAX_SPEED        GENMASK(31, 28)
> +#define BFPT_DWORD20_OCTAL_STR_MAX_SPEED        GENMASK(19, 16)
> +
>  struct sfdp_parameter_header {
>  	u8		id_lsb;
>  	u8		minor;
> 

Re: [PATCH v4 2/7] mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table

[Tudor.Ambarus]

Hi, Mason, YC Lin,

On 5/29/20 10:36 AM, Mason Yang wrote:
> JESD251, xSPI profile 1.0 table supports octal DTR mode.
> Extract information like the fast read opcode, dummy cycles for various
> frequencies, the number of dummy cycles needed for a Read Status
> Register command, the number of address bytes needed for a Read
> Status Register command, read volatile register command and write
> volatile register command.
> 
> According to BFPT 20th DWORD of octal maximum speed, driver get it's
> specific dummy cycles from profile 1.0 table and then could update
> it to device by their fixup hooks.
> 
> Since driver get octal DTR read opcode and then set read settings,
> expose spi_nor_set_read_settings() in core.h.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/core.c |   2 +-
>  drivers/mtd/spi-nor/core.h |  16 +++++++
>  drivers/mtd/spi-nor/sfdp.c | 106 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 123 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 1ab4386..3799417 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2204,7 +2204,7 @@ static int spi_nor_check(struct spi_nor *nor)
>  	return 0;
>  }
>  
> -static void
> +void
>  spi_nor_set_read_settings(struct spi_nor_read_command *read,
>  			  u8 num_mode_clocks,
>  			  u8 num_wait_states,
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 7a36b22..a33f807 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -191,6 +191,12 @@ struct spi_nor_locking_ops {
>   * @size:		the flash memory density in bytes.
>   * @page_size:		the page size of the SPI NOR flash memory.
>   * @octal_max_speed:	maximum operation speed of device in octal mode.
> + * @rdsr_dummy:		dummy cycles needed for Read Status Register command.
> + * @rdsr_addr_nbytes:	dummy address bytes needed for Read Status Register
> + *			command.
> + * @rd_reg_cmd:		read volatile register command for xSPI device.
> + * @wr_reg_cmd:		write volatile register command for xSPI device.

Can all the volatile registers be accessed with these commands?
The profile1.0 table lacks description.

> + * @dummy_cycles:	dummy cycles used for various frequencies
>   * @hwcaps:		describes the read and page program hardware
>   *			capabilities.
>   * @reads:		read capabilities ordered by priority: the higher index
> @@ -214,6 +220,11 @@ struct spi_nor_flash_parameter {
>  	u64				size;
>  	u32				page_size;
>  	u16				octal_max_speed;
> +	u8				rdsr_dummy;
> +	u8				rdsr_addr_nbytes;
> +	u8				rd_reg_cmd;
> +	u8				wr_reg_cmd;
> +	u8				dummy_cycles;
>  
>  	struct spi_nor_hwcaps		hwcaps;
>  	struct spi_nor_read_command	reads[SNOR_CMD_READ_MAX];
> @@ -420,6 +431,11 @@ ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
>  
>  int spi_nor_hwcaps_read2cmd(u32 hwcaps);
>  u8 spi_nor_convert_3to4_read(u8 opcode);
> +void spi_nor_set_read_settings(struct spi_nor_read_command *read,
> +			       u8 num_mode_clocks,
> +			       u8 num_wait_states,
> +			       u8 opcode,
> +			       enum spi_nor_protocol proto);
>  void spi_nor_set_pp_settings(struct spi_nor_pp_command *pp, u8 opcode,
>  			     enum spi_nor_protocol proto);
>  
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index 4d13f66..27a4de4 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -20,6 +20,7 @@
>  #define SFDP_BFPT_ID		0xff00	/* Basic Flash Parameter Table */
>  #define SFDP_SECTOR_MAP_ID	0xff81	/* Sector Map Table */
>  #define SFDP_4BAIT_ID		0xff84  /* 4-byte Address Instruction Table */
> +#define SFDP_PROFILE1_ID	0xff05	/* xSPI Profile 1.0 table. */
>  
>  #define SFDP_SIGNATURE		0x50444653U
>  #define SFDP_JESD216_MAJOR	1
> @@ -27,6 +28,27 @@
>  #define SFDP_JESD216A_MINOR	5
>  #define SFDP_JESD216B_MINOR	6
>  
> +/* xSPI Profile 1.0 table (from JESD216D.01). */
> +#define XSPI_PF1_DWORD1_RD_CMD                  GENMASK(15, 8)
> +#define XSPI_PF1_DWORD1_RDSR_ADDR_BYTES         BIT(29)
> +#define XSPI_PF1_DWORD1_RDSR_DUMMY_CYCLES       BIT(28)
> +#define XSPI_PF1_DWORD2_RD_REG_CMD              GENMASK(31, 24)
> +#define XSPI_PF1_DWORD2_WR_REG_CMD              GENMASK(15, 8)
> +#define XSPI_PF1_DUMMY_CYCLES_DEFAULT		20
> +#define XSPI_DWORD(x)   ((x) - 1)
> +#define XSPI_DWORD_MAX  5
> +
> +struct sfdp_xspi {
> +	u32 dwords[XSPI_DWORD_MAX];
> +};
> +
> +struct xspi_dummy_cycles {
> +	u16 speed_hz;	/* Speed MHz */
> +	u8 dwords;	/* Dwords index */
> +	u32 mask;	/* Mask */
> +	u8 shift;	/* Bit shift */
> +};
> +
>  /* Basic Flash Parameter Table 20th DWORD, Max operation speed of device */
>  struct octal_max_speed {
>  	u8 idx; /* Bits value */
> @@ -1117,6 +1139,86 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
>  }
>  
>  /**
> + * spi_nor_parse_profile1() - parse the xSPI Profile 1.0 table
> + * @nor:		pointer to a 'struct spi_nor'
> + * @param_header:	xspi profile 1 parameter table header
> + * @params:		pointer to the 'struct spi_nor_flash_parameter' to be.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_parse_profile1(struct spi_nor *nor,
> +				  const struct sfdp_parameter_header *header,
> +				  struct spi_nor_flash_parameter *params)
> +{
> +	struct sfdp_xspi pfile1;
> +	u8 opcode;
> +	u32 i, addr;
> +	size_t len;
> +	int ret;
> +	static const struct xspi_dummy_cycles dummy[] = {
> +		/* {MHz, Dwords index, Mask, Bit shift} */
> +		{ 200, 4, GENMASK(11, 7),   7},
> +		{ 166, 5, GENMASK(31, 27), 27},
> +		{ 133, 5, GENMASK(21, 17), 17},
> +		{ 100, 5, GENMASK(11, 7),   7},
> +	};
> +
> +	if (header->major != SFDP_JESD216_MAJOR ||
> +	    header->length < XSPI_DWORD_MAX)
> +		return -EINVAL;
> +
> +	len = min_t(size_t, sizeof(pfile1),
> +		    header->length * sizeof(u32));
> +
> +	memset(&pfile1, 0, sizeof(pfile1));
> +
> +	addr = SFDP_PARAM_HEADER_PTP(header);
> +	ret = spi_nor_read_sfdp(nor, addr, len, &pfile1);
> +	if (ret)
> +		goto out;
> +
> +	/* Fix endianness of the xSPI 1.0 DWORDs. */
> +	le32_to_cpu_array(pfile1.dwords, XSPI_DWORD_MAX);
> +
> +	/* Get 8D-8D-8D fast read opcode and dummy cycles. */
> +	opcode = FIELD_GET(XSPI_PF1_DWORD1_RD_CMD,
> +			   pfile1.dwords[XSPI_DWORD(1)]);
> +
> +	if (pfile1.dwords[XSPI_DWORD(1)] & XSPI_PF1_DWORD1_RDSR_ADDR_BYTES)
> +		params->rdsr_addr_nbytes = 4;
> +	else
> +		params->rdsr_addr_nbytes = 0;
> +
> +	if (pfile1.dwords[XSPI_DWORD(1)] & XSPI_PF1_DWORD1_RDSR_DUMMY_CYCLES)
> +		params->rdsr_dummy = 8;
> +	else
> +		params->rdsr_dummy = 4;
> +
> +	params->rd_reg_cmd = FIELD_GET(XSPI_PF1_DWORD2_RD_REG_CMD,
> +				       pfile1.dwords[XSPI_DWORD(2)]);
> +	params->wr_reg_cmd = FIELD_GET(XSPI_PF1_DWORD2_WR_REG_CMD,
> +				       pfile1.dwords[XSPI_DWORD(2)]);
> +
> +	/* according to BFPT 20th DOWRD to get devices dummy cycles */
> +	for (i = 0; i < ARRAY_SIZE(dummy); i++) {
> +		if (params->octal_max_speed == dummy[i].speed_hz) {

Why do you introduced this restriction? The dummy cycles fields themselves
can be used to determine if a speed is supported or not. A value different
than zero for the dummy cycles fields means that the corresponding frequency
is supported.

Cheers,
ta


> +			params->dummy_cycles = (dummy[i].mask &
> +				pfile1.dwords[XSPI_DWORD(dummy[i].dwords)]) >>
> +				dummy[i].shift;
> +			break;
> +		}
> +	}
> +	if (i == ARRAY_SIZE(dummy))
> +		params->dummy_cycles = XSPI_PF1_DUMMY_CYCLES_DEFAULT;
> +
> +	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_8_8_8_DTR],
> +				  0, params->dummy_cycles,
> +				  opcode, SNOR_PROTO_8_8_8_DTR);
> +out:
> +	return ret;
> +}
> +
> +/**
>   * spi_nor_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
>   * @nor:		pointer to a 'struct spi_nor'
>   * @params:		pointer to the 'struct spi_nor_flash_parameter' to be
> @@ -1217,6 +1319,10 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
>  			err = spi_nor_parse_4bait(nor, param_header, params);
>  			break;
>  
> +		case SFDP_PROFILE1_ID:
> +			err = spi_nor_parse_profile1(nor, param_header, params);
> +			break;
> +
>  		default:
>  			break;
>  		}
> 

Re: [PATCH v4 3/7] mtd: spi-nor: sfdp: parse command sequences to change octal DTR mode

[Tudor.Ambarus]

Hi, Mason, YC Lin,

On 5/29/20 10:36 AM, Mason Yang wrote:
> A set of simple command sequences is provided which can be executed
> directly by the host controller to enable octal DTR mode.
> 
> Each command sequence is 8 per byte for single SPI mode.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/core.h |  20 +++++++++
>  drivers/mtd/spi-nor/sfdp.c | 104 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 124 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index a33f807..8de7f53 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -183,6 +183,23 @@ struct spi_nor_locking_ops {
>  };
>  
>  /**
> + * struct cmd_seq_octal_dtr - command sequences to change to octal DTR mode
> + * Each command sequence is 8 per byte for single SPI mode.
> + * @len:	commmand length of each command sequence.
> + * @opcode:	command code.
> + * @addr:	address offset to device.
> + * @data:	data write to device.
> + */
> +struct cmd_seq_octal_dtr {
> +	u8 len;
> +	u8 opcode;
> +	u32 addr;
> +	u8 data;
> +};

You define seven bytes for the command sequence, while the table defines
eight bytes: first byte indicates the length of the sequence, and the
following seven bytes are values to be output by the controller.
How did you choose this cmd seq organization? The standard refers to the
7-byte sequence as "byte values to be output by the controller", it doesn't
indicate at which offset the opcode, addr and data are.

> +
> +#define CMD_SEQ_NUM	4
> +
> +/**
>   * struct spi_nor_flash_parameter - SPI NOR flash parameters and settings.
>   * Includes legacy flash parameters and settings that can be overwritten
>   * by the spi_nor_fixups hooks, or dynamically when parsing the JESD216
> @@ -205,6 +222,7 @@ struct spi_nor_locking_ops {
>   *                      higher index in the array, the higher priority.
>   * @erase_map:		the erase map parsed from the SFDP Sector Map Parameter
>   *                      Table.
> + * @cmd_seq:		command sequence to change to octal DTR mode.
>   * @quad_enable:	enables SPI NOR quad mode.
>   * @set_4byte_addr_mode: puts the SPI NOR in 4 byte addressing mode.
>   * @convert_addr:	converts an absolute address into something the flash
> @@ -232,6 +250,8 @@ struct spi_nor_flash_parameter {
>  
>  	struct spi_nor_erase_map        erase_map;
>  
> +	struct cmd_seq_octal_dtr	cmd_seq[CMD_SEQ_NUM];
> +
>  	int (*quad_enable)(struct spi_nor *nor);
>  	int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
>  	u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index 27a4de4..ef19290 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -21,6 +21,7 @@
>  #define SFDP_SECTOR_MAP_ID	0xff81	/* Sector Map Table */
>  #define SFDP_4BAIT_ID		0xff84  /* 4-byte Address Instruction Table */
>  #define SFDP_PROFILE1_ID	0xff05	/* xSPI Profile 1.0 table. */
> +#define SFDP_CMD_TO_8DTR_ID	0xff0a  /* Command Sequences to Octal DTR */
>  
>  #define SFDP_SIGNATURE		0x50444653U
>  #define SFDP_JESD216_MAJOR	1
> @@ -49,6 +50,19 @@ struct xspi_dummy_cycles {
>  	u8 shift;	/* Bit shift */
>  };
>  
> +#define CMD_TO_8DTR_LEN			GENMASK(31, 24)
> +#define CMD_TO_8DTR_OPCODE		GENMASK(23, 16)
> +#define CMD_TO_8DTR_1_ADDR		GENMASK(15, 8)
> +#define CMD_TO_8DTR_1_ADDR_DATA		GENMASK(7, 0)
> +#define CMD_TO_8DTR_4_ADDR_MSB		GENMASK(15, 0)
> +#define CMD_TO_8DTR_4_ADDR_LSB		GENMASK(31, 16)
> +#define CMD_TO_8DTR_4_ADDR_DATA		GENMASK(15, 8)
> +#define CMD_TO_8DTR_SIZE_MAX		8
> +
> +struct sfdp_cmd_to_8dtr {
> +	u32	dwords[CMD_TO_8DTR_SIZE_MAX];
> +};
> +
>  /* Basic Flash Parameter Table 20th DWORD, Max operation speed of device */
>  struct octal_max_speed {
>  	u8 idx; /* Bits value */
> @@ -1219,6 +1233,91 @@ static int spi_nor_parse_profile1(struct spi_nor *nor,
>  }
>  
>  /**
> + * spi_nor_parse_cmd_to_8dtr() - parse the command sequence to octal DTR
> + * @nor:		pointer to a 'struct spi_nor'
> + * @param_header:	command sequence to octal DTR parameter table header.
> + * @params:		pointer to the 'struct spi_nor_flash_parameter' to be.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_parse_cmd_to_8dtr(struct spi_nor *nor,
> +				     const struct sfdp_parameter_header *header,
> +				     struct spi_nor_flash_parameter *params)
> +{
> +	struct sfdp_cmd_to_8dtr cmd_seq;

you'll need to kmalloc cmd_seq because spi_nor_read_sfdp expects a DMA-able
buffer.

> +	u32 i, j, addr;
> +	size_t len;
> +	int ret;
> +
> +	if (header->major != SFDP_JESD216_MAJOR ||
> +	    header->length < CMD_TO_8DTR_SIZE_MAX)
> +		return -EINVAL;
> +
> +	len = min_t(size_t, sizeof(cmd_seq),
> +		    header->length * sizeof(u32));
> +
> +	memset(&cmd_seq, 0, sizeof(cmd_seq));

no need to set the contents to zero, cmd_seq will be overwritten with
the values read from sfdp.

> +
> +	addr = SFDP_PARAM_HEADER_PTP(header);
> +	ret = spi_nor_read_sfdp(nor, addr, len, &cmd_seq);
> +	if (ret)
> +		goto out;
> +
> +	/* Fix endianness of the Command Sequences to octal DTR. */
> +	le32_to_cpu_array(cmd_seq.dwords, CMD_TO_8DTR_SIZE_MAX);
> +
> +	memset(params->cmd_seq, 0, sizeof(params->cmd_seq[CMD_SEQ_NUM]));

params->cmd_seq is already filled with zeroes, no need to memset its
contents to zero

> +
> +	for (i = 0, j = 0;
> +	     i < CMD_SEQ_NUM && j < CMD_TO_8DTR_SIZE_MAX; i++, j += 2) {
> +		params->cmd_seq[i].len = FIELD_GET(CMD_TO_8DTR_LEN,
> +						   cmd_seq.dwords[j]);
> +		if (!params->cmd_seq[i].len)
> +			break;
> +
> +		switch (params->cmd_seq[i].len) {
> +		case 1:
> +			params->cmd_seq[i].opcode =
> +				FIELD_GET(CMD_TO_8DTR_OPCODE,
> +					  cmd_seq.dwords[j]);
> +			break;
> +
> +		case 3:
> +			params->cmd_seq[i].opcode =
> +				FIELD_GET(CMD_TO_8DTR_OPCODE,
> +					  cmd_seq.dwords[j]);
> +			params->cmd_seq[i].addr =
> +				FIELD_GET(CMD_TO_8DTR_1_ADDR,
> +					  cmd_seq.dwords[j]);
> +			params->cmd_seq[i].data =
> +				FIELD_GET(CMD_TO_8DTR_1_ADDR_DATA,
> +					  cmd_seq.dwords[j]);
> +			break;
> +
> +		case 6:
> +			params->cmd_seq[i].opcode =
> +				FIELD_GET(CMD_TO_8DTR_OPCODE,
> +					  cmd_seq.dwords[j]);
> +			params->cmd_seq[i].addr =
> +				FIELD_GET(CMD_TO_8DTR_4_ADDR_MSB,
> +					  cmd_seq.dwords[j]) << 16 |
> +				FIELD_GET(CMD_TO_8DTR_4_ADDR_LSB,
> +					  cmd_seq.dwords[j + 1]);
> +			params->cmd_seq[i].data =
> +				FIELD_GET(CMD_TO_8DTR_4_ADDR_DATA,
> +					  cmd_seq.dwords[j + 1]);
> +			break;

how should we treat case 2, 5 and 7? From where the assumptions on how
data is organized in the cmd sequence?

Thanks,
ta

> +
> +		default:
> +			break;
> +		}
> +	}
> +
> +out:
> +	return ret;
> +}
> +
> +/**
>   * spi_nor_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
>   * @nor:		pointer to a 'struct spi_nor'
>   * @params:		pointer to the 'struct spi_nor_flash_parameter' to be
> @@ -1323,6 +1422,11 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
>  			err = spi_nor_parse_profile1(nor, param_header, params);
>  			break;
>  
> +		case SFDP_CMD_TO_8DTR_ID:
> +			err = spi_nor_parse_cmd_to_8dtr(nor,
> +							param_header, params);
> +			break;
> +
>  		default:
>  			break;
>  		}
> 

Re: [PATCH v4 4/7] mtd: spi-nor: core: add configuration register 2 read & write support

[Tudor.Ambarus]

Hi, Mason, YC Lin,

On 5/29/20 10:36 AM, Mason Yang wrote:
> Configuration register 2 is to set the device operation condition like
> STR or DTR mode at address offset 0 and DQS mode at address offset 0x200.
> 
> Each device has various address offset for it's specific operatoin
> setting.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/core.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/mtd/spi-nor/core.h |  2 ++
>  2 files changed, 80 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 3799417..fed6236 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -269,6 +269,84 @@ int spi_nor_write_disable(struct spi_nor *nor)
>  }
>  
>  /**
> + * spi_nor_read_cr2() - Read the Configuration Register 2.
> + * @nor:	pointer to 'struct spi_nor'.
> + * @addr:	offset address to read.
> + * @cr2:	pointer to a DMA-able buffer where the value of the
> + *              Configuration Register 2  will be written.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +int spi_nor_read_cr2(struct spi_nor *nor, u32 addr, u8 *cr2)

spi_nor_read_volatile_reg() please

do we need to pass len as an argument?

s/cr2/buf

why do we need addr for register interactions?

> +{
> +	int ret;
> +	u8 cmd = nor->params->rd_reg_cmd;

you'll need to redefine to rd_vreg_cmd.

> +
> +	if (nor->spimem) {
> +		struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(cmd, 1),
> +				   SPI_MEM_OP_ADDR(4, addr, 1),

nor->addr_width?

> +				   SPI_MEM_OP_DUMMY(4, 1),

nor->read_dummy?

> +				   SPI_MEM_OP_DATA_IN(1, cr2, 1));

SPI_MEM_OP_DATA_IN(len, cr2, 0)?
> +
> +		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> +
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		if (spi_nor_protocol_is_dtr(nor->reg_proto))
> +			ret = -ENOTSUPP;
> +		else
> +			ret = nor->controller_ops->read_reg(nor, cmd, cr2, 1);
> +	}
> +
> +	if (ret)
> +		dev_dbg(nor->dev, "error %d reading CR2\n", ret);
> +
> +	return ret;
> +}
> +
> +/**
> + * spi_nor_write_cr2() - Write the Configuration Register 2.
> + * @nor:	pointer to 'struct spi_nor'.
> + * @addr:	offset address to write.
> + * @cr2:	pointer to a DMA-able buffer where the value of the
> + *              Configuratin Register 2 will be read.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +int spi_nor_write_cr2(struct spi_nor *nor, u32 addr, u8 *cr2)

spi_nor_write_volatile_reg() please

Cheers,
ta

Re: [PATCH v4 7/7] mtd: spi-nor: macronix: Add Octal 8D-8D-8D supports for Macronix mx25uw51245g

[Tudor.Ambarus]

Hi, Mason, YC Lin,

On 5/29/20 10:36 AM, Mason Yang wrote:
> Macronix mx25uw51245g is a SPI NOR that supports 1-1-1/8-8-8 mode.
> 
> Correct the dummy cycles to device for various frequencies
> after xSPI profile 1.0 table parsed.
> 
> Enable mx25uw51245g to Octal DTR mode by executing the command sequences
> to change to octal DTR mode.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/macronix.c | 55 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 96735d8..6c9a24c 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -8,6 +8,57 @@
>  
>  #include "core.h"
>  
> +#define MXIC_CR2_DUMMY_SET_ADDR 0x300
> +
> +/* Fixup the dummy cycles to device and setup octa_dtr_enable() */
> +static void mx25uw51245g_post_sfdp_fixups(struct spi_nor *nor)
> +{
> +	struct spi_nor_flash_parameter *params = nor->params;
> +	int ret;
> +	u8 rdc, wdc;
> +
> +	ret = spi_nor_read_cr2(nor, MXIC_CR2_DUMMY_SET_ADDR, &rdc);

nor->bouncebuf is DMA-able, use it instead of rdc

> +	if (ret)
> +		return;
> +
> +	/* Refer to dummy cycle and frequency table(MHz) */
> +	switch (params->dummy_cycles) {
> +	case 10:	/* 10 dummy cycles for 104 MHz */
> +		wdc = 5;
> +		break;
> +	case 12:	/* 12 dummy cycles for 133 MHz */
> +		wdc = 4;
> +		break;
> +	case 16:	/* 16 dummy cycles for 166 MHz */
> +		wdc = 2;
> +		break;
> +	case 18:	/* 18 dummy cycles for 173 MHz */
> +		wdc = 1;
> +		break;
> +	case 20:	/* 20 dummy cycles for 200 MHz */
> +	default:
> +		wdc = 0;
> +	}
> +
> +	if (rdc != wdc)
> +		spi_nor_write_cr2(nor, MXIC_CR2_DUMMY_SET_ADDR, &wdc);
> +
> +	if (params->cmd_seq[0].len) {
> +		params->octal_dtr_enable = spi_nor_cmd_seq_octal_dtr;
> +		params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR;
> +		params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR;
> +
> +	} else {
> +		params->octal_dtr_enable = NULL;
> +		params->hwcaps.mask &= ~SNOR_HWCAPS_READ_8_8_8_DTR;
> +		params->hwcaps.mask &= ~SNOR_HWCAPS_PP_8_8_8_DTR;
> +	}
> +}
> +
> +static struct spi_nor_fixups mx25uw51245g_fixups = {
> +	.post_sfdp = mx25uw51245g_post_sfdp_fixups,
> +};
> +
>  static int
>  mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
>  			    const struct sfdp_parameter_header *bfpt_header,
> @@ -84,6 +135,10 @@
>  			      SPI_NOR_QUAD_READ) },
>  	{ "mx66l1g55g",  INFO(0xc2261b, 0, 64 * 1024, 2048,
>  			      SPI_NOR_QUAD_READ) },
> +	{ "mx25uw51245g", INFO(0xc2813a, 0, 64 * 1024, 1024,
> +			      SECT_4K | SPI_NOR_4B_OPCODES |
> +			      SPI_NOR_OCTAL_DTR_READ)

octal dtr page program is supported?

> +			      .fixups = &mx25uw51245g_fixups },
>  };
>  
>  static void macronix_default_init(struct spi_nor *nor)
> 

Cheers,
ta

Re: [PATCH v4 0/7] mtd: spi-nor: add xSPI Octal DTR support

[Tudor.Ambarus]

Hi, Mason, YC Lin,

We'll have to figure out how we can best use the "Command Sequences
to Change to Octal DDR" table.

Would be great if you continue to work on this. One has to
rebase this series on top of v5.10-rc1 with Pratyush's series [1]
applied in advance. Please let me know about your plans with this
series.

Cheers,
ta

[1] https://patchwork.ozlabs.org/project/linux-mtd/cover/20201005153138.6437-1-p.yadav@ti.com/