linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode
@ 2019-11-15  8:58 Mason Yang
  2019-11-15  8:58 ` [PATCH 1/4] spi: spi-mem: " Mason Yang
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Mason Yang @ 2019-11-15  8:58 UTC (permalink / raw)
  To: broonie, miquel.raynal, richard, marek.vasut, dwmw2,
	computersforpeace, vigneshr, bbrezillon, tudor.ambarus
  Cc: juliensu, linux-kernel, linux-mtd, linux-spi, Mason Yang

Hello,

This is repost of patchset from Boris Brezillon's
[RFC,00/18] mtd: spi-nor: Proposal for 8-8-8 mode support [1].

Background from cover letter for RFC[1].

The trend has been around Octal NOR Flash lately and the latest mainline
already supports 1-1-8 and 1-8-8 modes.

Boris opened a discussion on how we should support stateful modes (X-X-X
and XD-XD-XD, where X is the bus width and D means Double Transfer Rate).

JESD216C has defined specification for Octal 8-8-8 and 8D-8D-8D.
It defined command and command extension in
JEDEC Basic Flash Parameter Table(18th DWORD) as well as how to
enable 8-8-8/8D-8D-8D mode sequences (Write CFG Reg 2).

The first set of patches is according to JESD216C adding Double Transfer
Rate(DTR) fields, extension command and command bytes number to the
spi_mem_op struct.

The second set of patches define the relevant macrons and enum in spi-nor
layer for Octal 8-8-8 and 8D-8D-8D mode operation.

The last set of patches in the series are modifying spi_nor_fixups hook to
tweak flash parameters for spi_nor_read/pp_setting() and then in a
chip-specific way to enter 8-8-8 or 8D-8D-8D modes on a Macronix chip.

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

[1] https://patchwork.ozlabs.org/cover/982926/

thnaks for your time and review.
best regards,
Mason


Mason Yang (4):
  spi: spi-mem: Add support for Octal 8D-8D-8D mode
  mtd: spi-nor: Add support for Octal 8D-8D-8D mode
  mtd: spi-nor: Add Octal 8D-8D-8D mode support for Macronix
    mx25uw51245g
  spi: mxic: Add support for Octal 8D-8D-8D mode

 drivers/mtd/spi-nor/spi-nor.c | 273 +++++++++++++++++++++++++++++++++++++++++-
 drivers/spi/spi-mem.c         |   8 +-
 drivers/spi/spi-mxic.c        |  98 ++++++++++-----
 include/linux/mtd/spi-nor.h   |  61 +++++++++-
 include/linux/spi/spi-mem.h   |  13 ++
 5 files changed, 410 insertions(+), 43 deletions(-)

-- 
1.9.1


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

* [PATCH 1/4] spi: spi-mem: Add support for Octal 8D-8D-8D mode
  2019-11-15  8:58 [PATCH 0/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode Mason Yang
@ 2019-11-15  8:58 ` Mason Yang
  2019-11-15  8:58 ` [PATCH 2/4] mtd: spi-nor: " Mason Yang
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Mason Yang @ 2019-11-15  8:58 UTC (permalink / raw)
  To: broonie, miquel.raynal, richard, marek.vasut, dwmw2,
	computersforpeace, vigneshr, bbrezillon, tudor.ambarus
  Cc: juliensu, linux-kernel, linux-mtd, linux-spi, Mason Yang,
	Boris Brezillon

According to JESD216C SPI NORs are using 2 bytes opcodes
when operated in OPI (Octal Peripheral Interface).

To add extension command, command bytes number and
Double Transfer Rate(DTR) fields to the spi_mem_op struct.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/spi/spi-mem.c       |  8 +++++++-
 include/linux/spi/spi-mem.h | 13 +++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index 9f0fa9f..eb33dd85 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -154,6 +154,12 @@ bool spi_mem_default_supports_op(struct spi_mem *mem,
 				   op->data.dir == SPI_MEM_DATA_OUT))
 		return false;
 
+	if (op->cmd.dtr || op->addr.dtr || op->dummy.dtr || op->data.dtr)
+		return false;
+
+	if (op->cmd.nbytes != 1)
+		return false;
+
 	return true;
 }
 EXPORT_SYMBOL_GPL(spi_mem_default_supports_op);
@@ -168,7 +174,7 @@ static bool spi_mem_buswidth_is_valid(u8 buswidth)
 
 static int spi_mem_check_op(const struct spi_mem_op *op)
 {
-	if (!op->cmd.buswidth)
+	if (!op->cmd.buswidth || op->cmd.nbytes < 1 || op->cmd.nbytes > 2)
 		return -EINVAL;
 
 	if ((op->addr.nbytes && !op->addr.buswidth) ||
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index af9ff2f..bf54079 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -17,6 +17,7 @@
 	{							\
 		.buswidth = __buswidth,				\
 		.opcode = __opcode,				\
+		.nbytes = 1,					\
 	}
 
 #define SPI_MEM_OP_ADDR(__nbytes, __val, __buswidth)		\
@@ -69,11 +70,15 @@ enum spi_mem_data_dir {
 
 /**
  * struct spi_mem_op - describes a SPI memory operation
+ * @cmd.nbytes: number of opcode bytes (only 1 or 2 are valid)
  * @cmd.buswidth: number of IO lines used to transmit the command
+ * @cmd.dtr: set true to transfer opcode in double transfer rate mode
  * @cmd.opcode: operation opcode
+ * @cmd.ext_opcode: extension operation opcode
  * @addr.nbytes: number of address bytes to send. Can be zero if the operation
  *		 does not need to send an address
  * @addr.buswidth: number of IO lines used to transmit the address cycles
+ * @addr.dtr: set true to transfer address bytes in double transfer rate mode
  * @addr.val: address value. This value is always sent MSB first on the bus.
  *	      Note that only @addr.nbytes are taken into account in this
  *	      address value, so users should make sure the value fits in the
@@ -81,34 +86,42 @@ enum spi_mem_data_dir {
  * @dummy.nbytes: number of dummy bytes to send after an opcode or address. Can
  *		  be zero if the operation does not require dummy bytes
  * @dummy.buswidth: number of IO lanes used to transmit the dummy bytes
+ * @dummy.dtr: set true to transfer dummy bytes in double transfer rate mode
  * @data.buswidth: number of IO lanes used to send/receive the data
  * @data.dir: direction of the transfer
  * @data.nbytes: number of data bytes to send/receive. Can be zero if the
  *		 operation does not involve transferring data
+ * @data.dtr: set true to transfer data bytes in double transfer rate mode
  * @data.buf.in: input buffer (must be DMA-able)
  * @data.buf.out: output buffer (must be DMA-able)
  */
 struct spi_mem_op {
 	struct {
+		u8 nbytes;
 		u8 buswidth;
+		bool dtr;
 		u8 opcode;
+		u8 ext_opcode;
 	} cmd;
 
 	struct {
 		u8 nbytes;
 		u8 buswidth;
+		bool dtr;
 		u64 val;
 	} addr;
 
 	struct {
 		u8 nbytes;
 		u8 buswidth;
+		bool dtr;
 	} dummy;
 
 	struct {
 		u8 buswidth;
 		enum spi_mem_data_dir dir;
 		unsigned int nbytes;
+		bool dtr;
 		union {
 			void *in;
 			const void *out;
-- 
1.9.1


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

* [PATCH 2/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode
  2019-11-15  8:58 [PATCH 0/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode Mason Yang
  2019-11-15  8:58 ` [PATCH 1/4] spi: spi-mem: " Mason Yang
@ 2019-11-15  8:58 ` Mason Yang
  2019-11-15 12:30   ` kbuild test robot
                     ` (2 more replies)
  2019-11-15  8:58 ` [PATCH 3/4] mtd: spi-nor: Add Octal 8D-8D-8D mode support for Macronix mx25uw51245g Mason Yang
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 17+ messages in thread
From: Mason Yang @ 2019-11-15  8:58 UTC (permalink / raw)
  To: broonie, miquel.raynal, richard, marek.vasut, dwmw2,
	computersforpeace, vigneshr, bbrezillon, tudor.ambarus
  Cc: juliensu, linux-kernel, linux-mtd, linux-spi, Mason Yang,
	Boris Brezillon

According to JESD216C (JEDEC Basic Flash Parameter Table 18th DWORD)
Octal DTR(8D-8D-8D) command and command extension (00b: same, 01b: inverse)
to add extension command mode in spi_nor struct and to add write_cr2
(Write CFG Reg 2) for 8-8-8/8D-8D-8D mode sequences enable.

Define the relevant macrons and enum to add such modes and make sure
op->xxx.dtr fields, command nbytes and extension command are properly
filled and unmask DTR and X-X-X modes in spi_nor_spimem_adjust_hwcaps()
so that DTR and X-X-X support detection is done through
spi_mem_supports_op().

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/mtd/spi-nor/spi-nor.c | 159 ++++++++++++++++++++++++++++++++++++++++--
 include/linux/mtd/spi-nor.h   |  58 +++++++++++++--
 2 files changed, 206 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 7acf4a9..4cdf52d 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -320,6 +320,23 @@ static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from,
 	/* convert the dummy cycles to the number of bytes */
 	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
 
+	if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
+		op.cmd.nbytes = 2;
+
+		if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+			op.cmd.ext_opcode = ~nor->read_opcode;
+		else
+			op.cmd.ext_opcode = nor->read_opcode;
+
+		if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
+			op.dummy.nbytes *= 2;
+			op.cmd.dtr = true;
+			op.addr.dtr = true;
+			op.dummy.dtr = true;
+			op.data.dtr = true;
+		}
+	}
+
 	return spi_nor_spimem_xfer_data(nor, &op);
 }
 
@@ -367,6 +384,21 @@ static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t to,
 	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
 		op.addr.nbytes = 0;
 
+	if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
+		op.cmd.nbytes = 2;
+
+		if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+			op.cmd.ext_opcode = ~nor->program_opcode;
+		else
+			op.cmd.ext_opcode = nor->program_opcode;
+
+		if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto)) {
+			op.cmd.dtr = true;
+			op.addr.dtr = true;
+			op.data.dtr = true;
+		}
+	}
+
 	return spi_nor_spimem_xfer_data(nor, &op);
 }
 
@@ -404,6 +436,30 @@ static int read_sr(struct spi_nor *nor)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
 
+		if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
+			op.cmd.buswidth = 8;
+			op.addr.buswidth = 8;
+			op.dummy.buswidth = 8;
+			op.data.buswidth = 8;
+			op.cmd.nbytes = 2;
+			op.addr.nbytes = 4;
+			op.dummy.nbytes = 4;
+			op.addr.val = 0;
+
+			if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+				op.cmd.ext_opcode = ~SPINOR_OP_RDSR;
+			else
+				op.cmd.ext_opcode = SPINOR_OP_RDSR;
+
+			if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
+				op.dummy.nbytes *= 2;
+				op.cmd.dtr = true;
+				op.addr.dtr = true;
+				op.dummy.dtr = true;
+				op.data.dtr = true;
+			}
+		}
+
 		ret = spi_mem_exec_op(nor->spimem, &op);
 	} else {
 		ret = nor->read_reg(nor, SPINOR_OP_RDSR, nor->bouncebuf, 1);
@@ -508,6 +564,19 @@ static int write_enable(struct spi_nor *nor)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_NO_DATA);
 
+		if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
+			op.cmd.buswidth = 8;
+			op.cmd.nbytes = 2;
+
+			if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
+				op.cmd.dtr = true;
+
+			if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+				op.cmd.ext_opcode = ~SPINOR_OP_WREN;
+			else
+				op.cmd.ext_opcode = SPINOR_OP_WREN;
+		}
+
 		return spi_mem_exec_op(nor->spimem, &op);
 	}
 
@@ -526,12 +595,65 @@ static int write_disable(struct spi_nor *nor)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_NO_DATA);
 
+		if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
+			op.cmd.buswidth = 8;
+			op.cmd.nbytes = 2;
+
+			if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
+				op.cmd.dtr = true;
+
+			if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+				op.cmd.ext_opcode = ~SPINOR_OP_WRDI;
+			else
+				op.cmd.ext_opcode = SPINOR_OP_WRDI;
+		}
+
 		return spi_mem_exec_op(nor->spimem, &op);
 	}
 
 	return nor->write_reg(nor, SPINOR_OP_WRDI, NULL, 0);
 }
 
+/*
+ * Write configuration register 2 one byte
+ * Returns negative if error occurred.
+ */
+static int write_cr2(struct spi_nor *nor, u32 addr, u8 val)
+{
+	write_enable(nor);
+
+	nor->bouncebuf[0] = val;
+	if (nor->spimem) {
+		struct spi_mem_op op =
+			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRCR2, 1),
+				   SPI_MEM_OP_ADDR(4, addr, 1),
+				   SPI_MEM_OP_NO_DUMMY,
+				   SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
+
+		if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
+			op.cmd.buswidth = 8;
+			op.addr.buswidth = 8;
+			op.data.buswidth = 8;
+			op.cmd.nbytes = 2;
+
+			if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto)) {
+				op.cmd.dtr = true;
+				op.addr.dtr = true;
+				op.data.dtr = true;
+			}
+
+			if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+				op.cmd.ext_opcode = ~SPINOR_OP_WRCR2;
+			else
+				op.cmd.ext_opcode = SPINOR_OP_WRCR2;
+		}
+
+		return spi_mem_exec_op(nor->spimem, &op);
+	}
+
+	return -ENOTSUPP;
+}
+
 static struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
 {
 	return mtd->priv;
@@ -868,6 +990,19 @@ static int erase_chip(struct spi_nor *nor)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_NO_DATA);
 
+		if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
+			op.cmd.buswidth = 8;
+			op.cmd.nbytes = 2;
+
+			if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
+				op.cmd.dtr = true;
+
+			if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+				op.cmd.ext_opcode = ~SPINOR_OP_CHIP_ERASE;
+			else
+				op.cmd.ext_opcode = SPINOR_OP_CHIP_ERASE;
+		}
+
 		return spi_mem_exec_op(nor->spimem, &op);
 	}
 
@@ -945,6 +1080,22 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_NO_DATA);
 
+		if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
+			op.cmd.buswidth = 8;
+			op.addr.buswidth = 8;
+			op.cmd.nbytes = 2;
+
+			if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto)) {
+				op.cmd.dtr = true;
+				op.addr.dtr = true;
+			}
+
+			if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+				op.cmd.ext_opcode = ~nor->erase_opcode;
+			else
+				op.cmd.ext_opcode = nor->erase_opcode;
+		}
+
 		return spi_mem_exec_op(nor->spimem, &op);
 	}
 
@@ -2825,6 +2976,7 @@ static int spi_nor_hwcaps_read2cmd(u32 hwcaps)
 		{ SNOR_HWCAPS_READ_1_8_8,	SNOR_CMD_READ_1_8_8 },
 		{ SNOR_HWCAPS_READ_8_8_8,	SNOR_CMD_READ_8_8_8 },
 		{ SNOR_HWCAPS_READ_1_8_8_DTR,	SNOR_CMD_READ_1_8_8_DTR },
+		{ SNOR_HWCAPS_READ_8_8_8_DTR,	SNOR_CMD_READ_8D_8D_8D },
 	};
 
 	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_read2cmd,
@@ -2841,6 +2993,7 @@ static int spi_nor_hwcaps_pp2cmd(u32 hwcaps)
 		{ SNOR_HWCAPS_PP_1_1_8,		SNOR_CMD_PP_1_1_8 },
 		{ SNOR_HWCAPS_PP_1_8_8,		SNOR_CMD_PP_1_8_8 },
 		{ SNOR_HWCAPS_PP_8_8_8,		SNOR_CMD_PP_8_8_8 },
+		{ SNOR_HWCAPS_PP_8_8_8_DTR,	SNOR_CMD_PP_8D_8D_8D },
 	};
 
 	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_pp2cmd,
@@ -3010,12 +3163,6 @@ static int spi_nor_spimem_check_pp(struct spi_nor *nor,
 	struct spi_nor_flash_parameter *params =  &nor->params;
 	unsigned int cap;
 
-	/* DTR modes are not supported yet, mask them all. */
-	*hwcaps &= ~SNOR_HWCAPS_DTR;
-
-	/* X-X-X modes are not supported yet, mask them all. */
-	*hwcaps &= ~SNOR_HWCAPS_X_X_X;
-
 	for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
 		int rdidx, ppidx;
 
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index fc0b4b1..2e720ca 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -84,6 +84,9 @@
 #define SPINOR_OP_BE_4K_4B	0x21	/* Erase 4KiB block */
 #define SPINOR_OP_BE_32K_4B	0x5c	/* Erase 32KiB block */
 #define SPINOR_OP_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
+#define SPINOR_OP_READ_8_8_8	SPINOR_OP_READ_1_4_4_4B
+#define SPINOR_OP_PP_8_8_8	SPINOR_OP_PP_4B
+#define SPINOR_OP_PP_8D_8D_8D	SPINOR_OP_PP_4B
 
 /* Double Transfer Rate opcodes - defined in JEDEC JESD216B. */
 #define SPINOR_OP_READ_1_1_1_DTR	0x0d
@@ -93,6 +96,7 @@
 #define SPINOR_OP_READ_1_1_1_DTR_4B	0x0e
 #define SPINOR_OP_READ_1_2_2_DTR_4B	0xbe
 #define SPINOR_OP_READ_1_4_4_DTR_4B	0xee
+#define SPINOR_OP_READ_8D_8D_8D		SPINOR_OP_READ_1_4_4_DTR_4B
 
 /* Used for SST flashes only. */
 #define SPINOR_OP_BP		0x02	/* Byte program */
@@ -107,6 +111,8 @@
 #define XSR_PAGESIZE		BIT(0)	/* Page size in Po2 or Linear */
 #define XSR_RDY			BIT(7)	/* Ready */
 
+/* Write CFG Reg 2 - defined in JEDEC JESD216C. */
+#define SPINOR_OP_WRCR2		0x72	/* Write configuration register 2 */
 
 /* Used for Macronix and Winbond flashes. */
 #define SPINOR_OP_EN4B		0xb7	/* Enter 4-byte mode */
@@ -150,6 +156,13 @@
 /* Status Register 2 bits. */
 #define SR2_QUAD_EN_BIT7	BIT(7)
 
+/* Configuration register 2, offset 0 */
+#define CR2_REG0		0x0
+#define CR2_REG0_MODE_MASK	GENMASK(1, 0)
+#define CR2_REG0_MODE_SPI	0
+#define CR2_REG0_MODE_OPI_STR	1
+#define CR2_REG0_MODE_OPI_DTR	2
+
 /* Supported SPI protocols */
 #define SNOR_PROTO_INST_MASK	GENMASK(23, 16)
 #define SNOR_PROTO_INST_SHIFT	16
@@ -170,6 +183,7 @@
 	 SNOR_PROTO_DATA_MASK)
 
 #define SNOR_PROTO_IS_DTR	BIT(24)	/* Double Transfer Rate */
+#define SNOR_PROTO_IS_8D_8D_8D	BIT(25) /* Full Octal DTR */
 
 #define SNOR_PROTO_STR(_inst_nbits, _addr_nbits, _data_nbits)	\
 	(SNOR_PROTO_INST(_inst_nbits) |				\
@@ -179,6 +193,10 @@
 	(SNOR_PROTO_IS_DTR |					\
 	 SNOR_PROTO_STR(_inst_nbits, _addr_nbits, _data_nbits))
 
+#define SNOR_PROTO_8D_8D_8D(_nbits)				\
+	(SNOR_PROTO_IS_8D_8D_8D |				\
+	 SNOR_PROTO_STR(_nbits, _nbits, _nbits))
+
 enum spi_nor_protocol {
 	SNOR_PROTO_1_1_1 = SNOR_PROTO_STR(1, 1, 1),
 	SNOR_PROTO_1_1_2 = SNOR_PROTO_STR(1, 1, 2),
@@ -195,6 +213,7 @@ enum spi_nor_protocol {
 	SNOR_PROTO_1_2_2_DTR = SNOR_PROTO_DTR(1, 2, 2),
 	SNOR_PROTO_1_4_4_DTR = SNOR_PROTO_DTR(1, 4, 4),
 	SNOR_PROTO_1_8_8_DTR = SNOR_PROTO_DTR(1, 8, 8),
+	SNOR_PROTO_8_8_8_DTR = SNOR_PROTO_8D_8D_8D(8),
 };
 
 static inline bool spi_nor_protocol_is_dtr(enum spi_nor_protocol proto)
@@ -202,6 +221,16 @@ static inline bool spi_nor_protocol_is_dtr(enum spi_nor_protocol proto)
 	return !!(proto & SNOR_PROTO_IS_DTR);
 }
 
+static inline bool spi_nor_protocol_is_8_8_8(enum spi_nor_protocol proto)
+{
+	return !!(proto & SNOR_PROTO_8_8_8);
+}
+
+static inline bool spi_nor_protocol_is_8D_8D_8D(enum spi_nor_protocol proto)
+{
+	return !!(proto & SNOR_PROTO_IS_8D_8D_8D);
+}
+
 static inline u8 spi_nor_get_protocol_inst_nbits(enum spi_nor_protocol proto)
 {
 	return ((unsigned long)(proto & SNOR_PROTO_INST_MASK)) >>
@@ -349,7 +378,7 @@ struct spi_nor_hwcaps {
  * then Quad SPI protocols before Dual SPI protocols, Fast Read and lastly
  * (Slow) Read.
  */
-#define SNOR_HWCAPS_READ_MASK		GENMASK(14, 0)
+#define SNOR_HWCAPS_READ_MASK		GENMASK(15, 0)
 #define SNOR_HWCAPS_READ		BIT(0)
 #define SNOR_HWCAPS_READ_FAST		BIT(1)
 #define SNOR_HWCAPS_READ_1_1_1_DTR	BIT(2)
@@ -366,11 +395,12 @@ struct spi_nor_hwcaps {
 #define SNOR_HWCAPS_READ_4_4_4		BIT(9)
 #define SNOR_HWCAPS_READ_1_4_4_DTR	BIT(10)
 
-#define SNOR_HWCAPS_READ_OCTAL		GENMASK(14, 11)
+#define SNOR_HWCAPS_READ_OCTAL		GENMASK(15, 11)
 #define SNOR_HWCAPS_READ_1_1_8		BIT(11)
 #define SNOR_HWCAPS_READ_1_8_8		BIT(12)
 #define SNOR_HWCAPS_READ_8_8_8		BIT(13)
 #define SNOR_HWCAPS_READ_1_8_8_DTR	BIT(14)
+#define SNOR_HWCAPS_READ_8_8_8_DTR	BIT(15)
 
 /*
  * Page Program capabilities.
@@ -381,7 +411,7 @@ struct spi_nor_hwcaps {
  * JEDEC/SFDP standard to define them. Also at this moment no SPI flash memory
  * implements such commands.
  */
-#define SNOR_HWCAPS_PP_MASK	GENMASK(22, 16)
+#define SNOR_HWCAPS_PP_MASK	GENMASK(23, 16)
 #define SNOR_HWCAPS_PP		BIT(16)
 
 #define SNOR_HWCAPS_PP_QUAD	GENMASK(19, 17)
@@ -389,10 +419,17 @@ struct spi_nor_hwcaps {
 #define SNOR_HWCAPS_PP_1_4_4	BIT(18)
 #define SNOR_HWCAPS_PP_4_4_4	BIT(19)
 
-#define SNOR_HWCAPS_PP_OCTAL	GENMASK(22, 20)
+#define SNOR_HWCAPS_PP_OCTAL	GENMASK(23, 20)
 #define SNOR_HWCAPS_PP_1_1_8	BIT(20)
 #define SNOR_HWCAPS_PP_1_8_8	BIT(21)
 #define SNOR_HWCAPS_PP_8_8_8	BIT(22)
+#define SNOR_HWCAPS_PP_8_8_8_DTR	BIT(23)
+
+#define SNOR_HWCAPS_OPI_FULL_STR	(SNOR_HWCAPS_READ_8_8_8 | \
+					 SNOR_HWCAPS_PP_8_8_8)
+
+#define SNOR_HWCAPS_OPI_FULL_DTR	(SNOR_HWCAPS_READ_8_8_8_DTR | \
+					 SNOR_HWCAPS_PP_8_8_8_DTR)
 
 #define SNOR_HWCAPS_X_X_X	(SNOR_HWCAPS_READ_2_2_2 |	\
 				 SNOR_HWCAPS_READ_4_4_4 |	\
@@ -403,7 +440,9 @@ struct spi_nor_hwcaps {
 #define SNOR_HWCAPS_DTR		(SNOR_HWCAPS_READ_1_1_1_DTR |	\
 				 SNOR_HWCAPS_READ_1_2_2_DTR |	\
 				 SNOR_HWCAPS_READ_1_4_4_DTR |	\
-				 SNOR_HWCAPS_READ_1_8_8_DTR)
+				 SNOR_HWCAPS_READ_1_8_8_DTR |	\
+				 SNOR_HWCAPS_READ_8_8_8_DTR |	\
+				 SNOR_HWCAPS_PP_8_8_8_DTR)
 
 #define SNOR_HWCAPS_ALL		(SNOR_HWCAPS_READ_MASK |	\
 				 SNOR_HWCAPS_PP_MASK)
@@ -442,6 +481,7 @@ enum spi_nor_read_command_index {
 	SNOR_CMD_READ_1_8_8,
 	SNOR_CMD_READ_8_8_8,
 	SNOR_CMD_READ_1_8_8_DTR,
+	SNOR_CMD_READ_8D_8D_8D,
 
 	SNOR_CMD_READ_MAX
 };
@@ -458,6 +498,7 @@ enum spi_nor_pp_command_index {
 	SNOR_CMD_PP_1_1_8,
 	SNOR_CMD_PP_1_8_8,
 	SNOR_CMD_PP_8_8_8,
+	SNOR_CMD_PP_8D_8D_8D,
 
 	SNOR_CMD_PP_MAX
 };
@@ -528,6 +569,11 @@ struct spi_nor_flash_parameter {
  */
 struct flash_info;
 
+enum extension_cmd_mode {
+	EXT_CMD_IS_CMD,
+	EXT_CMD_IS_INVERSE,
+};
+
 /**
  * struct spi_nor - Structure for defining a the SPI NOR layer
  * @mtd:		point to a mtd_info structure
@@ -537,6 +583,7 @@ struct spi_nor_flash_parameter {
  * @bouncebuf:		bounce buffer used when the buffer passed by the MTD
  *                      layer is not DMA-able
  * @bouncebuf_size:	size of the bounce buffer
+ * @ext_cmd_mode:	extension command mode, 0: same, 1: inverse
  * @info:		spi-nor part JDEC MFR id and other info
  * @page_size:		the page size of the SPI NOR
  * @addr_width:		number of address bytes
@@ -575,6 +622,7 @@ struct spi_nor {
 	struct spi_mem		*spimem;
 	u8			*bouncebuf;
 	size_t			bouncebuf_size;
+	enum extension_cmd_mode	ext_cmd_mode;
 	const struct flash_info	*info;
 	u32			page_size;
 	u8			addr_width;
-- 
1.9.1


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

* [PATCH 3/4] mtd: spi-nor: Add Octal 8D-8D-8D mode support for Macronix mx25uw51245g
  2019-11-15  8:58 [PATCH 0/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode Mason Yang
  2019-11-15  8:58 ` [PATCH 1/4] spi: spi-mem: " Mason Yang
  2019-11-15  8:58 ` [PATCH 2/4] mtd: spi-nor: " Mason Yang
@ 2019-11-15  8:58 ` Mason Yang
  2019-12-04 13:03   ` Vignesh Raghavendra
  2019-11-15  8:58 ` [PATCH 4/4] spi: mxic: Add support for Octal 8D-8D-8D mode Mason Yang
  2019-12-10 17:00 ` [PATCH 0/4] mtd: spi-nor: " Tudor.Ambarus
  4 siblings, 1 reply; 17+ messages in thread
From: Mason Yang @ 2019-11-15  8:58 UTC (permalink / raw)
  To: broonie, miquel.raynal, richard, marek.vasut, dwmw2,
	computersforpeace, vigneshr, bbrezillon, tudor.ambarus
  Cc: juliensu, linux-kernel, linux-mtd, linux-spi, Mason Yang,
	Boris Brezillon

mx25uw51245g is a SPI NOR that supports 1-1-1/8-8-8 mode and the SFDPRD
command but returns an empty SFDP page. This forces us to add a new entry
in the flash_info table and patch spi_nor_fixups hooks to tweak flash
parameters for spi_nor_read/pp_setting() and then to enter
Octal 8D-8D-8D modes.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/mtd/spi-nor/spi-nor.c | 114 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spi-nor.h   |   3 ++
 2 files changed, 117 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 4cdf52d..9013590 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -615,6 +615,56 @@ static int write_disable(struct spi_nor *nor)
 }
 
 /*
+ * Read configuration register 2, returning its value in the
+ * location. Return the configuration register 2 value.
+ * Returns negative if error occurred.
+ */
+static int read_cr2(struct spi_nor *nor, u32 addr)
+{
+	int ret;
+
+	if (nor->spimem) {
+		struct spi_mem_op op =
+			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDCR2, 1),
+				   SPI_MEM_OP_ADDR(4, addr, 1),
+				   SPI_MEM_OP_DUMMY(4, 1),
+				   SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
+
+		if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
+			op.cmd.buswidth = 8;
+			op.addr.buswidth = 8;
+			op.dummy.buswidth = 8;
+			op.data.buswidth = 8;
+			op.cmd.nbytes = 2;
+
+			if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
+				op.dummy.nbytes *= 2;
+				op.cmd.dtr = true;
+				op.addr.dtr = true;
+				op.dummy.dtr = true;
+				op.data.dtr = true;
+			}
+
+			if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+				op.cmd.ext_opcode = ~SPINOR_OP_RDCR2;
+			else
+				op.cmd.ext_opcode = SPINOR_OP_RDCR2;
+		}
+
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = -ENOTSUPP;
+	}
+
+	if (ret < 0) {
+		dev_err(nor->dev, "error %d reading CR\n", ret);
+		return ret;
+	}
+
+	return nor->bouncebuf[0];
+}
+
+/*
  * Write configuration register 2 one byte
  * Returns negative if error occurred.
  */
@@ -2275,10 +2325,72 @@ static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
 	return 0;
 }
 
+static 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);
+
+static void
+spi_nor_set_pp_settings(struct spi_nor_pp_command *pp,
+			u8 opcode,
+			enum spi_nor_protocol proto);
+
+static void
+mx25uw51245g_default_init(struct spi_nor *nor)
+{
+	struct spi_nor_flash_parameter *params = &nor->params;
+
+	if (!(nor->spimem->spi->mode & (SPI_RX_OCTAL | SPI_TX_OCTAL)))
+		return;
+
+	/* Octal 8S-8S-8S mode */
+	params->hwcaps.mask |= SNOR_HWCAPS_OPI_FULL_STR;
+	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_8_8_8],
+				  0, 20, SPINOR_OP_READ_8_8_8,
+				  SNOR_PROTO_8_8_8);
+
+	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_8_8_8],
+				SPINOR_OP_PP_8_8_8, SNOR_PROTO_8_8_8);
+
+	/* Octal 8D-8D-8D mode */
+	params->hwcaps.mask |= SNOR_HWCAPS_OPI_FULL_DTR;
+	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_8D_8D_8D],
+				  0, 20, SPINOR_OP_READ_8D_8D_8D,
+				  SNOR_PROTO_8_8_8_DTR);
+
+	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_8D_8D_8D],
+				SPINOR_OP_PP_8D_8D_8D, SNOR_PROTO_8_8_8_DTR);
+
+	nor->ext_cmd_mode = EXT_CMD_IS_INVERSE;
+}
+
+static void
+mx25uw51245g_post_sfdp_fixups(struct spi_nor *nor)
+{
+	struct spi_nor_flash_parameter *params = &nor->params;
+	u8 cr2;
+
+	cr2 = read_cr2(nor, CR2_REG0) & CR2_REG0_MODE_MASK;
+
+	if (params->hwcaps.mask & SNOR_HWCAPS_OPI_FULL_DTR)
+		cr2 |= CR2_REG0_MODE_OPI_DTR;
+	else if (params->hwcaps.mask & SNOR_HWCAPS_OPI_FULL_STR)
+		cr2 |= CR2_REG0_MODE_OPI_STR;
+
+	write_cr2(nor, CR2_REG0, cr2);
+}
+
 static struct spi_nor_fixups mx25l25635_fixups = {
 	.post_bfpt = mx25l25635_post_bfpt_fixups,
 };
 
+static struct spi_nor_fixups mx25uw51245g_fixups = {
+	.default_init = mx25uw51245g_default_init,
+	.post_sfdp = mx25uw51245g_post_sfdp_fixups,
+};
+
 static void gd25q256_default_init(struct spi_nor *nor)
 {
 	/*
@@ -2442,6 +2554,8 @@ static void gd25q256_default_init(struct spi_nor *nor)
 			 SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
 			 .fixups = &mx25l25635_fixups },
 	{ "mx25u25635f", INFO(0xc22539, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_4B_OPCODES) },
+	{ "mx25uw51245g", INFO(0xc2813a, 0, 64 * 1024, 1024, SECT_4K | SPI_NOR_4B_OPCODES)
+				.fixups = &mx25uw51245g_fixups},
 	{ "mx25v8035f",  INFO(0xc22314, 0, 64 * 1024,  16,
 			 SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) },
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 2e720ca..3aa54dd 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -114,6 +114,9 @@
 /* Write CFG Reg 2 - defined in JEDEC JESD216C. */
 #define SPINOR_OP_WRCR2		0x72	/* Write configuration register 2 */
 
+/* Used for Macronix flashes only */
+#define SPINOR_OP_RDCR2		0x71	/* Read configuration register 2 */
+
 /* Used for Macronix and Winbond flashes. */
 #define SPINOR_OP_EN4B		0xb7	/* Enter 4-byte mode */
 #define SPINOR_OP_EX4B		0xe9	/* Exit 4-byte mode */
-- 
1.9.1


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

* [PATCH 4/4] spi: mxic: Add support for Octal 8D-8D-8D mode
  2019-11-15  8:58 [PATCH 0/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode Mason Yang
                   ` (2 preceding siblings ...)
  2019-11-15  8:58 ` [PATCH 3/4] mtd: spi-nor: Add Octal 8D-8D-8D mode support for Macronix mx25uw51245g Mason Yang
@ 2019-11-15  8:58 ` Mason Yang
  2019-11-15 14:39   ` kbuild test robot
  2019-11-16  7:20   ` kbuild test robot
  2019-12-10 17:00 ` [PATCH 0/4] mtd: spi-nor: " Tudor.Ambarus
  4 siblings, 2 replies; 17+ messages in thread
From: Mason Yang @ 2019-11-15  8:58 UTC (permalink / raw)
  To: broonie, miquel.raynal, richard, marek.vasut, dwmw2,
	computersforpeace, vigneshr, bbrezillon, tudor.ambarus
  Cc: juliensu, linux-kernel, linux-mtd, linux-spi, Mason Yang

patch driver for 8-8-8 and 8D-8D-8D mode support.

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

diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
index f48563c..50e2055 100644
--- a/drivers/spi/spi-mxic.c
+++ b/drivers/spi/spi-mxic.c
@@ -280,10 +280,58 @@ 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;
+			if (op->data.dtr == OP_DATA_DDR)
+				cfg |= OP_DQS_EN;
+		}
+	}
+
+	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 +350,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 +370,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 +388,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 &&
@@ -353,47 +406,29 @@ 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;
+	int i, ret;
 	u8 addr[8];
+	u8 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);
+	cmd[0] = op->cmd.opcode;
+	if (op->cmd.nbytes == 2)
+		cmd[1] = op->cmd.ext_opcode;
+
+	ret = mxic_spi_data_xfer(mxic, cmd, NULL, op->cmd.nbytes);
 	if (ret)
 		goto out;
 
@@ -566,7 +601,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


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

* Re: [PATCH 2/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode
  2019-11-15  8:58 ` [PATCH 2/4] mtd: spi-nor: " Mason Yang
@ 2019-11-15 12:30   ` kbuild test robot
  2019-11-15 13:42   ` kbuild test robot
  2019-12-04 12:46   ` Vignesh Raghavendra
  2 siblings, 0 replies; 17+ messages in thread
From: kbuild test robot @ 2019-11-15 12:30 UTC (permalink / raw)
  To: Mason Yang
  Cc: kbuild-all, broonie, miquel.raynal, richard, marek.vasut, dwmw2,
	computersforpeace, vigneshr, bbrezillon, tudor.ambarus, juliensu,
	linux-kernel, linux-mtd, linux-spi, Mason Yang, Boris Brezillon

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

Hi Mason,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on spi/for-next]
[also build test WARNING on v5.4-rc7]
[cannot apply to next-20191114]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Mason-Yang/mtd-spi-nor-Add-support-for-Octal-8D-8D-8D-mode/20191115-170233
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: c6x-allyesconfig (attached as .config)
compiler: c6x-elf-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=c6x 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers//mtd/spi-nor/spi-nor.c: In function 'erase_chip':
>> drivers//mtd/spi-nor/spi-nor.c:1001:25: warning: large integer implicitly truncated to unsigned type [-Woverflow]
        op.cmd.ext_opcode = ~SPINOR_OP_CHIP_ERASE;
                            ^
   At top level:
   drivers//mtd/spi-nor/spi-nor.c:621:12: warning: 'write_cr2' defined but not used [-Wunused-function]
    static int write_cr2(struct spi_nor *nor, u32 addr, u8 val)
               ^~~~~~~~~

vim +1001 drivers//mtd/spi-nor/spi-nor.c

   976	
   977	/*
   978	 * Erase the whole flash memory
   979	 *
   980	 * Returns 0 if successful, non-zero otherwise.
   981	 */
   982	static int erase_chip(struct spi_nor *nor)
   983	{
   984		dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10));
   985	
   986		if (nor->spimem) {
   987			struct spi_mem_op op =
   988				SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CHIP_ERASE, 1),
   989					   SPI_MEM_OP_NO_ADDR,
   990					   SPI_MEM_OP_NO_DUMMY,
   991					   SPI_MEM_OP_NO_DATA);
   992	
   993			if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
   994				op.cmd.buswidth = 8;
   995				op.cmd.nbytes = 2;
   996	
   997				if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
   998					op.cmd.dtr = true;
   999	
  1000				if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> 1001					op.cmd.ext_opcode = ~SPINOR_OP_CHIP_ERASE;
  1002				else
  1003					op.cmd.ext_opcode = SPINOR_OP_CHIP_ERASE;
  1004			}
  1005	
  1006			return spi_mem_exec_op(nor->spimem, &op);
  1007		}
  1008	
  1009		return nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0);
  1010	}
  1011	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50138 bytes --]

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

* Re: [PATCH 2/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode
  2019-11-15  8:58 ` [PATCH 2/4] mtd: spi-nor: " Mason Yang
  2019-11-15 12:30   ` kbuild test robot
@ 2019-11-15 13:42   ` kbuild test robot
  2019-12-04 12:46   ` Vignesh Raghavendra
  2 siblings, 0 replies; 17+ messages in thread
From: kbuild test robot @ 2019-11-15 13:42 UTC (permalink / raw)
  To: Mason Yang
  Cc: kbuild-all, broonie, miquel.raynal, richard, marek.vasut, dwmw2,
	computersforpeace, vigneshr, bbrezillon, tudor.ambarus, juliensu,
	linux-kernel, linux-mtd, linux-spi, Mason Yang, Boris Brezillon

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

Hi Mason,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on spi/for-next]
[also build test WARNING on v5.4-rc7]
[cannot apply to next-20191115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Mason-Yang/mtd-spi-nor-Add-support-for-Octal-8D-8D-8D-mode/20191115-170233
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: nds32-allyesconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.1.0 make.cross ARCH=nds32 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/mtd/spi-nor/spi-nor.c: In function 'erase_chip':
>> drivers/mtd/spi-nor/spi-nor.c:1001:25: warning: unsigned conversion from 'int' to 'u8' {aka 'unsigned char'} changes value from '-200' to '56' [-Woverflow]
        op.cmd.ext_opcode = ~SPINOR_OP_CHIP_ERASE;
                            ^
   At top level:
   drivers/mtd/spi-nor/spi-nor.c:621:12: warning: 'write_cr2' defined but not used [-Wunused-function]
    static int write_cr2(struct spi_nor *nor, u32 addr, u8 val)
               ^~~~~~~~~

vim +1001 drivers/mtd/spi-nor/spi-nor.c

   976	
   977	/*
   978	 * Erase the whole flash memory
   979	 *
   980	 * Returns 0 if successful, non-zero otherwise.
   981	 */
   982	static int erase_chip(struct spi_nor *nor)
   983	{
   984		dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10));
   985	
   986		if (nor->spimem) {
   987			struct spi_mem_op op =
   988				SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CHIP_ERASE, 1),
   989					   SPI_MEM_OP_NO_ADDR,
   990					   SPI_MEM_OP_NO_DUMMY,
   991					   SPI_MEM_OP_NO_DATA);
   992	
   993			if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
   994				op.cmd.buswidth = 8;
   995				op.cmd.nbytes = 2;
   996	
   997				if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
   998					op.cmd.dtr = true;
   999	
  1000				if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> 1001					op.cmd.ext_opcode = ~SPINOR_OP_CHIP_ERASE;
  1002				else
  1003					op.cmd.ext_opcode = SPINOR_OP_CHIP_ERASE;
  1004			}
  1005	
  1006			return spi_mem_exec_op(nor->spimem, &op);
  1007		}
  1008	
  1009		return nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0);
  1010	}
  1011	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 53259 bytes --]

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

* Re: [PATCH 4/4] spi: mxic: Add support for Octal 8D-8D-8D mode
  2019-11-15  8:58 ` [PATCH 4/4] spi: mxic: Add support for Octal 8D-8D-8D mode Mason Yang
@ 2019-11-15 14:39   ` kbuild test robot
  2019-11-16  7:20   ` kbuild test robot
  1 sibling, 0 replies; 17+ messages in thread
From: kbuild test robot @ 2019-11-15 14:39 UTC (permalink / raw)
  To: Mason Yang
  Cc: kbuild-all, broonie, miquel.raynal, richard, marek.vasut, dwmw2,
	computersforpeace, vigneshr, bbrezillon, tudor.ambarus, juliensu,
	linux-kernel, linux-mtd, linux-spi, Mason Yang

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

Hi Mason,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on spi/for-next]
[also build test WARNING on v5.4-rc7]
[cannot apply to next-20191115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Mason-Yang/mtd-spi-nor-Add-support-for-Octal-8D-8D-8D-mode/20191115-170233
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: c6x-allyesconfig (attached as .config)
compiler: c6x-elf-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=c6x 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers//spi/spi-mxic.c: In function 'mxic_spi_mem_prep_op_cfg':
>> drivers//spi/spi-mxic.c:302:21: warning: comparison of constant '256' with boolean expression is always false [-Wbool-compare]
       if (op->data.dtr == OP_DATA_DDR)
                        ^~

vim +/256 +302 drivers//spi/spi-mxic.c

   282	
   283	static u32 mxic_spi_mem_prep_op_cfg(const struct spi_mem_op *op)
   284	{
   285		u32 cfg =  OP_CMD_BYTES(op->cmd.nbytes) |
   286			   OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) |
   287			   (op->cmd.dtr ? OP_CMD_DDR : 0);
   288	
   289		if (op->addr.nbytes)
   290			cfg |= OP_ADDR_BYTES(op->addr.nbytes) |
   291			       OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) |
   292			       (op->addr.dtr ? OP_ADDR_DDR : 0);
   293	
   294		if (op->dummy.nbytes)
   295			cfg |= OP_DUMMY_CYC(op->dummy.nbytes);
   296	
   297		if (op->data.nbytes) {
   298			cfg |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) |
   299			      (op->data.dtr ? OP_DATA_DDR : 0);
   300			if (op->data.dir == SPI_MEM_DATA_IN) {
   301				cfg |= OP_READ;
 > 302				if (op->data.dtr == OP_DATA_DDR)
   303					cfg |= OP_DQS_EN;
   304			}
   305		}
   306	
   307		return cfg;
   308	}
   309	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50138 bytes --]

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

* Re: [PATCH 4/4] spi: mxic: Add support for Octal 8D-8D-8D mode
  2019-11-15  8:58 ` [PATCH 4/4] spi: mxic: Add support for Octal 8D-8D-8D mode Mason Yang
  2019-11-15 14:39   ` kbuild test robot
@ 2019-11-16  7:20   ` kbuild test robot
  1 sibling, 0 replies; 17+ messages in thread
From: kbuild test robot @ 2019-11-16  7:20 UTC (permalink / raw)
  To: Mason Yang
  Cc: kbuild-all, broonie, miquel.raynal, richard, marek.vasut, dwmw2,
	computersforpeace, vigneshr, bbrezillon, tudor.ambarus, juliensu,
	linux-kernel, linux-mtd, linux-spi, Mason Yang

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

Hi Mason,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on spi/for-next]
[also build test WARNING on v5.4-rc7]
[cannot apply to next-20191115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Mason-Yang/mtd-spi-nor-Add-support-for-Octal-8D-8D-8D-mode/20191115-170233
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: x86_64-randconfig-a003-20191115 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/err.h:5:0,
                    from include/linux/clk.h:12,
                    from drivers//spi/spi-mxic.c:11:
   drivers//spi/spi-mxic.c: In function 'mxic_spi_mem_prep_op_cfg':
   drivers//spi/spi-mxic.c:302:21: warning: comparison of constant '256' with boolean expression is always false [-Wbool-compare]
       if (op->data.dtr == OP_DATA_DDR)
                        ^
   include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
    #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                                       ^~~~
>> drivers//spi/spi-mxic.c:302:4: note: in expansion of macro 'if'
       if (op->data.dtr == OP_DATA_DDR)
       ^~
   drivers//spi/spi-mxic.c:302:21: warning: comparison of constant '256' with boolean expression is always false [-Wbool-compare]
       if (op->data.dtr == OP_DATA_DDR)
                        ^
   include/linux/compiler.h:58:61: note: in definition of macro '__trace_if_var'
    #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                                                ^~~~
>> drivers//spi/spi-mxic.c:302:4: note: in expansion of macro 'if'
       if (op->data.dtr == OP_DATA_DDR)
       ^~
   drivers//spi/spi-mxic.c:302:21: warning: comparison of constant '256' with boolean expression is always false [-Wbool-compare]
       if (op->data.dtr == OP_DATA_DDR)
                        ^
   include/linux/compiler.h:69:3: note: in definition of macro '__trace_if_value'
     (cond) ?     \
      ^~~~
   include/linux/compiler.h:56:28: note: in expansion of macro '__trace_if_var'
    #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                               ^~~~~~~~~~~~~~
>> drivers//spi/spi-mxic.c:302:4: note: in expansion of macro 'if'
       if (op->data.dtr == OP_DATA_DDR)
       ^~
   Cyclomatic Complexity 1 include/linux/err.h:PTR_ERR
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls
   Cyclomatic Complexity 1 include/linux/ktime.h:ktime_add_us
   Cyclomatic Complexity 1 arch/x86/include/asm/io.h:readl
   Cyclomatic Complexity 1 arch/x86/include/asm/io.h:writel
   Cyclomatic Complexity 1 include/linux/device.h:dev_get_drvdata
   Cyclomatic Complexity 1 include/linux/device.h:dev_set_drvdata
   Cyclomatic Complexity 1 include/linux/platform_device.h:platform_get_drvdata
   Cyclomatic Complexity 1 include/linux/platform_device.h:platform_set_drvdata
   Cyclomatic Complexity 1 include/linux/spi/spi.h:spi_controller_get_devdata
   Cyclomatic Complexity 1 drivers//spi/spi-mxic.c:mxic_spi_set_input_delay_dqs
   Cyclomatic Complexity 1 drivers//spi/spi-mxic.c:mxic_spi_hw_init
   Cyclomatic Complexity 1 drivers//spi/spi-mxic.c:mxic_spi_driver_init
   Cyclomatic Complexity 7 include/linux/ktime.h:ktime_compare
   Cyclomatic Complexity 4 drivers//spi/spi-mxic.c:mxic_spi_set_cs
   Cyclomatic Complexity 10 drivers//spi/spi-mxic.c:mxic_spi_set_hc_cfg
   Cyclomatic Complexity 16 drivers//spi/spi-mxic.c:mxic_spi_mem_prep_op_cfg
   Cyclomatic Complexity 7 include/linux/clk.h:clk_prepare_enable
   Cyclomatic Complexity 1 include/linux/clk.h:clk_disable_unprepare
   Cyclomatic Complexity 7 drivers//spi/spi-mxic.c:mxic_spi_clk_enable
   Cyclomatic Complexity 4 drivers//spi/spi-mxic.c:mxic_spi_runtime_resume
   Cyclomatic Complexity 1 drivers//spi/spi-mxic.c:mxic_spi_clk_disable
   Cyclomatic Complexity 1 drivers//spi/spi-mxic.c:mxic_spi_runtime_suspend
   Cyclomatic Complexity 1 include/linux/pm_runtime.h:pm_runtime_disable
   Cyclomatic Complexity 1 drivers//spi/spi-mxic.c:mxic_spi_remove
   Cyclomatic Complexity 10 drivers//spi/spi-mxic.c:mxic_spi_clk_setup
   Cyclomatic Complexity 10 drivers//spi/spi-mxic.c:mxic_spi_set_freq
   Cyclomatic Complexity 79 drivers//spi/spi-mxic.c:mxic_spi_data_xfer
   Cyclomatic Complexity 42 drivers//spi/spi-mxic.c:mxic_spi_transfer_one
   Cyclomatic Complexity 19 drivers//spi/spi-mxic.c:mxic_spi_mem_exec_op
   Cyclomatic Complexity 1 include/linux/err.h:IS_ERR
   Cyclomatic Complexity 29 drivers//spi/spi-mxic.c:mxic_spi_mem_supports_op
   Cyclomatic Complexity 1 include/linux/spi/spi.h:spi_alloc_master
   Cyclomatic Complexity 4 include/linux/spi/spi.h:spi_controller_put
   Cyclomatic Complexity 15 drivers//spi/spi-mxic.c:mxic_spi_probe
   Cyclomatic Complexity 1 drivers//spi/spi-mxic.c:mxic_spi_driver_exit

vim +/if +302 drivers//spi/spi-mxic.c

   282	
   283	static u32 mxic_spi_mem_prep_op_cfg(const struct spi_mem_op *op)
   284	{
   285		u32 cfg =  OP_CMD_BYTES(op->cmd.nbytes) |
   286			   OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) |
   287			   (op->cmd.dtr ? OP_CMD_DDR : 0);
   288	
   289		if (op->addr.nbytes)
   290			cfg |= OP_ADDR_BYTES(op->addr.nbytes) |
   291			       OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) |
   292			       (op->addr.dtr ? OP_ADDR_DDR : 0);
   293	
   294		if (op->dummy.nbytes)
   295			cfg |= OP_DUMMY_CYC(op->dummy.nbytes);
   296	
   297		if (op->data.nbytes) {
   298			cfg |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) |
   299			      (op->data.dtr ? OP_DATA_DDR : 0);
   300			if (op->data.dir == SPI_MEM_DATA_IN) {
   301				cfg |= OP_READ;
 > 302				if (op->data.dtr == OP_DATA_DDR)
   303					cfg |= OP_DQS_EN;
   304			}
   305		}
   306	
   307		return cfg;
   308	}
   309	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39159 bytes --]

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

* Re: [PATCH 2/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode
  2019-11-15  8:58 ` [PATCH 2/4] mtd: spi-nor: " Mason Yang
  2019-11-15 12:30   ` kbuild test robot
  2019-11-15 13:42   ` kbuild test robot
@ 2019-12-04 12:46   ` Vignesh Raghavendra
  2019-12-09  7:56     ` masonccyang
  2 siblings, 1 reply; 17+ messages in thread
From: Vignesh Raghavendra @ 2019-12-04 12:46 UTC (permalink / raw)
  To: Mason Yang, broonie, miquel.raynal, richard, marek.vasut, dwmw2,
	computersforpeace, bbrezillon, tudor.ambarus
  Cc: juliensu, linux-kernel, linux-mtd, linux-spi

Hi Mason,

On 15/11/19 2:28 pm, Mason Yang wrote:
> According to JESD216C (JEDEC Basic Flash Parameter Table 18th DWORD)
> Octal DTR(8D-8D-8D) command and command extension (00b: same, 01b: inverse)
> to add extension command mode in spi_nor struct and to add write_cr2
> (Write CFG Reg 2) for 8-8-8/8D-8D-8D mode sequences enable.
> 

But I don't see any code setting "nor->ext_cmd_mode" based on BFPT?

Any new feature that we add to spi-nor should make use of autodiscovery
feature made possible by SFDP tables. Could you modify the patch to
discover capabilities supported by flash and opcodes to be used from
SFDP table?


> Define the relevant macrons and enum to add such modes and make sure
> op->xxx.dtr fields, command nbytes and extension command are properly
> filled and unmask DTR and X-X-X modes in spi_nor_spimem_adjust_hwcaps()
> so that DTR and X-X-X support detection is done through
> spi_mem_supports_op().
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 159 ++++++++++++++++++++++++++++++++++++++++--
>  include/linux/mtd/spi-nor.h   |  58 +++++++++++++--
>  2 files changed, 206 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 7acf4a9..4cdf52d 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -320,6 +320,23 @@ static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from,
>  	/* convert the dummy cycles to the number of bytes */
>  	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
>  
> +	if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
> +		op.cmd.nbytes = 2;

Can we get this info from SFDP too?

> +
> +		if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> +			op.cmd.ext_opcode = ~nor->read_opcode;
> +		else
> +			op.cmd.ext_opcode = nor->read_opcode;
> +
> +		if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
> +			op.dummy.nbytes *= 2;

> +			op.cmd.dtr = true;
> +			op.addr.dtr = true;
> +			op.dummy.dtr = true;
> +			op.data.dtr = true;

Can we have a macro for this initialization?


> +		}
> +	}
> +
>  	return spi_nor_spimem_xfer_data(nor, &op);
>  }
>  
> @@ -367,6 +384,21 @@ static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t to,
>  	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
>  		op.addr.nbytes = 0;
>  
> +	if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
> +		op.cmd.nbytes = 2;
> +
> +		if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> +			op.cmd.ext_opcode = ~nor->program_opcode;
> +		else
> +			op.cmd.ext_opcode = nor->program_opcode;
> +
> +		if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto)) {
> +			op.cmd.dtr = true;
> +			op.addr.dtr = true;
> +			op.data.dtr = true;
> +		}
> +	}
> +
>  	return spi_nor_spimem_xfer_data(nor, &op);
>  }
>  
> @@ -404,6 +436,30 @@ static int read_sr(struct spi_nor *nor)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
>  

This is not based on the latest tree.

> +		if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
> +			op.cmd.buswidth = 8;
> +			op.addr.buswidth = 8;
> +			op.dummy.buswidth = 8;
> +			op.data.buswidth = 8;
> +			op.cmd.nbytes = 2;
> +			op.addr.nbytes = 4;
> +			op.dummy.nbytes = 4;
> +			op.addr.val = 0;

This is not scalable... There will be bunch of if...else ladders when we
want to support other X-X-X modes... Can't these be derived from
nor->reg_proto? And then borrow the logic from spi_nor_spimem_read_data()?


Could you have a look at Boris's initial submission to add 8-8-8 mode at
https://patchwork.kernel.org/cover/10638055/ ?
You could use that series as the base for your changes/additions.

I am fine if you want to call 2nd byte of opcode as ext_opcode

Regards
Vignesh

> +
> +			if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> +				op.cmd.ext_opcode = ~SPINOR_OP_RDSR;
> +			else
> +				op.cmd.ext_opcode = SPINOR_OP_RDSR;
> +
> +			if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
> +				op.dummy.nbytes *= 2;
> +				op.cmd.dtr = true;
> +				op.addr.dtr = true;
> +				op.dummy.dtr = true;
> +				op.data.dtr = true;
> +			}
> +		}
> +
>  		ret = spi_mem_exec_op(nor->spimem, &op);
>  	} else {
>  		ret = nor->read_reg(nor, SPINOR_OP_RDSR, nor->bouncebuf, 1);
> @@ -508,6 +564,19 @@ static int write_enable(struct spi_nor *nor)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_NO_DATA);
>  
> +		if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
> +			op.cmd.buswidth = 8;
> +			op.cmd.nbytes = 2;
> +
> +			if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
> +				op.cmd.dtr = true;
> +
> +			if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> +				op.cmd.ext_opcode = ~SPINOR_OP_WREN;
> +			else
> +				op.cmd.ext_opcode = SPINOR_OP_WREN;
> +		}
> +
>  		return spi_mem_exec_op(nor->spimem, &op);
>  	}
>  
> @@ -526,12 +595,65 @@ static int write_disable(struct spi_nor *nor)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_NO_DATA);
>  
> +		if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
> +			op.cmd.buswidth = 8;
> +			op.cmd.nbytes = 2;
> +
> +			if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
> +				op.cmd.dtr = true;
> +
> +			if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> +				op.cmd.ext_opcode = ~SPINOR_OP_WRDI;
> +			else
> +				op.cmd.ext_opcode = SPINOR_OP_WRDI;
> +		}
> +
>  		return spi_mem_exec_op(nor->spimem, &op);
>  	}
>  
>  	return nor->write_reg(nor, SPINOR_OP_WRDI, NULL, 0);
>  }
>  
> +/*
> + * Write configuration register 2 one byte
> + * Returns negative if error occurred.
> + */
> +static int write_cr2(struct spi_nor *nor, u32 addr, u8 val)
> +{
> +	write_enable(nor);
> +
> +	nor->bouncebuf[0] = val;
> +	if (nor->spimem) {
> +		struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRCR2, 1),
> +				   SPI_MEM_OP_ADDR(4, addr, 1),
> +				   SPI_MEM_OP_NO_DUMMY,
> +				   SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
> +
> +		if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
> +			op.cmd.buswidth = 8;
> +			op.addr.buswidth = 8;
> +			op.data.buswidth = 8;
> +			op.cmd.nbytes = 2;
> +
> +			if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto)) {
> +				op.cmd.dtr = true;
> +				op.addr.dtr = true;
> +				op.data.dtr = true;
> +			}
> +
> +			if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> +				op.cmd.ext_opcode = ~SPINOR_OP_WRCR2;
> +			else
> +				op.cmd.ext_opcode = SPINOR_OP_WRCR2;
> +		}
> +
> +		return spi_mem_exec_op(nor->spimem, &op);
> +	}
> +
> +	return -ENOTSUPP;
> +}
> +
>  static struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
>  {
>  	return mtd->priv;
> @@ -868,6 +990,19 @@ static int erase_chip(struct spi_nor *nor)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_NO_DATA);
>  
> +		if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
> +			op.cmd.buswidth = 8;
> +			op.cmd.nbytes = 2;
> +
> +			if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
> +				op.cmd.dtr = true;
> +
> +			if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> +				op.cmd.ext_opcode = ~SPINOR_OP_CHIP_ERASE;
> +			else
> +				op.cmd.ext_opcode = SPINOR_OP_CHIP_ERASE;
> +		}
> +
>  		return spi_mem_exec_op(nor->spimem, &op);
>  	}
>  
> @@ -945,6 +1080,22 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_NO_DATA);
>  
> +		if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
> +			op.cmd.buswidth = 8;
> +			op.addr.buswidth = 8;
> +			op.cmd.nbytes = 2;
> +
> +			if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto)) {
> +				op.cmd.dtr = true;
> +				op.addr.dtr = true;
> +			}
> +
> +			if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> +				op.cmd.ext_opcode = ~nor->erase_opcode;
> +			else
> +				op.cmd.ext_opcode = nor->erase_opcode;
> +		}
> +
>  		return spi_mem_exec_op(nor->spimem, &op);
>  	}
>  
> @@ -2825,6 +2976,7 @@ static int spi_nor_hwcaps_read2cmd(u32 hwcaps)
>  		{ SNOR_HWCAPS_READ_1_8_8,	SNOR_CMD_READ_1_8_8 },
>  		{ SNOR_HWCAPS_READ_8_8_8,	SNOR_CMD_READ_8_8_8 },
>  		{ SNOR_HWCAPS_READ_1_8_8_DTR,	SNOR_CMD_READ_1_8_8_DTR },
> +		{ SNOR_HWCAPS_READ_8_8_8_DTR,	SNOR_CMD_READ_8D_8D_8D },
>  	};
>  
>  	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_read2cmd,
> @@ -2841,6 +2993,7 @@ static int spi_nor_hwcaps_pp2cmd(u32 hwcaps)
>  		{ SNOR_HWCAPS_PP_1_1_8,		SNOR_CMD_PP_1_1_8 },
>  		{ SNOR_HWCAPS_PP_1_8_8,		SNOR_CMD_PP_1_8_8 },
>  		{ SNOR_HWCAPS_PP_8_8_8,		SNOR_CMD_PP_8_8_8 },
> +		{ SNOR_HWCAPS_PP_8_8_8_DTR,	SNOR_CMD_PP_8D_8D_8D },
>  	};
>  
>  	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_pp2cmd,
> @@ -3010,12 +3163,6 @@ static int spi_nor_spimem_check_pp(struct spi_nor *nor,
>  	struct spi_nor_flash_parameter *params =  &nor->params;
>  	unsigned int cap;
>  
> -	/* DTR modes are not supported yet, mask them all. */
> -	*hwcaps &= ~SNOR_HWCAPS_DTR;
> -
> -	/* X-X-X modes are not supported yet, mask them all. */
> -	*hwcaps &= ~SNOR_HWCAPS_X_X_X;
> -
>  	for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
>  		int rdidx, ppidx;
>  
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index fc0b4b1..2e720ca 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -84,6 +84,9 @@
>  #define SPINOR_OP_BE_4K_4B	0x21	/* Erase 4KiB block */
>  #define SPINOR_OP_BE_32K_4B	0x5c	/* Erase 32KiB block */
>  #define SPINOR_OP_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
> +#define SPINOR_OP_READ_8_8_8	SPINOR_OP_READ_1_4_4_4B
> +#define SPINOR_OP_PP_8_8_8	SPINOR_OP_PP_4B
> +#define SPINOR_OP_PP_8D_8D_8D	SPINOR_OP_PP_4B
>  
>  /* Double Transfer Rate opcodes - defined in JEDEC JESD216B. */
>  #define SPINOR_OP_READ_1_1_1_DTR	0x0d
> @@ -93,6 +96,7 @@
>  #define SPINOR_OP_READ_1_1_1_DTR_4B	0x0e
>  #define SPINOR_OP_READ_1_2_2_DTR_4B	0xbe
>  #define SPINOR_OP_READ_1_4_4_DTR_4B	0xee
> +#define SPINOR_OP_READ_8D_8D_8D		SPINOR_OP_READ_1_4_4_DTR_4B
>  
>  /* Used for SST flashes only. */
>  #define SPINOR_OP_BP		0x02	/* Byte program */
> @@ -107,6 +111,8 @@
>  #define XSR_PAGESIZE		BIT(0)	/* Page size in Po2 or Linear */
>  #define XSR_RDY			BIT(7)	/* Ready */
>  
> +/* Write CFG Reg 2 - defined in JEDEC JESD216C. */
> +#define SPINOR_OP_WRCR2		0x72	/* Write configuration register 2 */
>  
>  /* Used for Macronix and Winbond flashes. */
>  #define SPINOR_OP_EN4B		0xb7	/* Enter 4-byte mode */
> @@ -150,6 +156,13 @@
>  /* Status Register 2 bits. */
>  #define SR2_QUAD_EN_BIT7	BIT(7)
>  
> +/* Configuration register 2, offset 0 */
> +#define CR2_REG0		0x0
> +#define CR2_REG0_MODE_MASK	GENMASK(1, 0)
> +#define CR2_REG0_MODE_SPI	0
> +#define CR2_REG0_MODE_OPI_STR	1
> +#define CR2_REG0_MODE_OPI_DTR	2
> +
>  /* Supported SPI protocols */
>  #define SNOR_PROTO_INST_MASK	GENMASK(23, 16)
>  #define SNOR_PROTO_INST_SHIFT	16
> @@ -170,6 +183,7 @@
>  	 SNOR_PROTO_DATA_MASK)
>  
>  #define SNOR_PROTO_IS_DTR	BIT(24)	/* Double Transfer Rate */
> +#define SNOR_PROTO_IS_8D_8D_8D	BIT(25) /* Full Octal DTR */
>  
>  #define SNOR_PROTO_STR(_inst_nbits, _addr_nbits, _data_nbits)	\
>  	(SNOR_PROTO_INST(_inst_nbits) |				\
> @@ -179,6 +193,10 @@
>  	(SNOR_PROTO_IS_DTR |					\
>  	 SNOR_PROTO_STR(_inst_nbits, _addr_nbits, _data_nbits))
>  
> +#define SNOR_PROTO_8D_8D_8D(_nbits)				\
> +	(SNOR_PROTO_IS_8D_8D_8D |				\
> +	 SNOR_PROTO_STR(_nbits, _nbits, _nbits))
> +
>  enum spi_nor_protocol {
>  	SNOR_PROTO_1_1_1 = SNOR_PROTO_STR(1, 1, 1),
>  	SNOR_PROTO_1_1_2 = SNOR_PROTO_STR(1, 1, 2),
> @@ -195,6 +213,7 @@ enum spi_nor_protocol {
>  	SNOR_PROTO_1_2_2_DTR = SNOR_PROTO_DTR(1, 2, 2),
>  	SNOR_PROTO_1_4_4_DTR = SNOR_PROTO_DTR(1, 4, 4),
>  	SNOR_PROTO_1_8_8_DTR = SNOR_PROTO_DTR(1, 8, 8),
> +	SNOR_PROTO_8_8_8_DTR = SNOR_PROTO_8D_8D_8D(8),
>  };
>  
>  static inline bool spi_nor_protocol_is_dtr(enum spi_nor_protocol proto)
> @@ -202,6 +221,16 @@ static inline bool spi_nor_protocol_is_dtr(enum spi_nor_protocol proto)
>  	return !!(proto & SNOR_PROTO_IS_DTR);
>  }
>  
> +static inline bool spi_nor_protocol_is_8_8_8(enum spi_nor_protocol proto)
> +{
> +	return !!(proto & SNOR_PROTO_8_8_8);
> +}
> +
> +static inline bool spi_nor_protocol_is_8D_8D_8D(enum spi_nor_protocol proto)
> +{
> +	return !!(proto & SNOR_PROTO_IS_8D_8D_8D);
> +}
> +
>  static inline u8 spi_nor_get_protocol_inst_nbits(enum spi_nor_protocol proto)
>  {
>  	return ((unsigned long)(proto & SNOR_PROTO_INST_MASK)) >>
> @@ -349,7 +378,7 @@ struct spi_nor_hwcaps {
>   * then Quad SPI protocols before Dual SPI protocols, Fast Read and lastly
>   * (Slow) Read.
>   */
> -#define SNOR_HWCAPS_READ_MASK		GENMASK(14, 0)
> +#define SNOR_HWCAPS_READ_MASK		GENMASK(15, 0)
>  #define SNOR_HWCAPS_READ		BIT(0)
>  #define SNOR_HWCAPS_READ_FAST		BIT(1)
>  #define SNOR_HWCAPS_READ_1_1_1_DTR	BIT(2)
> @@ -366,11 +395,12 @@ struct spi_nor_hwcaps {
>  #define SNOR_HWCAPS_READ_4_4_4		BIT(9)
>  #define SNOR_HWCAPS_READ_1_4_4_DTR	BIT(10)
>  
> -#define SNOR_HWCAPS_READ_OCTAL		GENMASK(14, 11)
> +#define SNOR_HWCAPS_READ_OCTAL		GENMASK(15, 11)
>  #define SNOR_HWCAPS_READ_1_1_8		BIT(11)
>  #define SNOR_HWCAPS_READ_1_8_8		BIT(12)
>  #define SNOR_HWCAPS_READ_8_8_8		BIT(13)
>  #define SNOR_HWCAPS_READ_1_8_8_DTR	BIT(14)
> +#define SNOR_HWCAPS_READ_8_8_8_DTR	BIT(15)
>  
>  /*
>   * Page Program capabilities.
> @@ -381,7 +411,7 @@ struct spi_nor_hwcaps {
>   * JEDEC/SFDP standard to define them. Also at this moment no SPI flash memory
>   * implements such commands.
>   */
> -#define SNOR_HWCAPS_PP_MASK	GENMASK(22, 16)
> +#define SNOR_HWCAPS_PP_MASK	GENMASK(23, 16)
>  #define SNOR_HWCAPS_PP		BIT(16)
>  
>  #define SNOR_HWCAPS_PP_QUAD	GENMASK(19, 17)
> @@ -389,10 +419,17 @@ struct spi_nor_hwcaps {
>  #define SNOR_HWCAPS_PP_1_4_4	BIT(18)
>  #define SNOR_HWCAPS_PP_4_4_4	BIT(19)
>  
> -#define SNOR_HWCAPS_PP_OCTAL	GENMASK(22, 20)
> +#define SNOR_HWCAPS_PP_OCTAL	GENMASK(23, 20)
>  #define SNOR_HWCAPS_PP_1_1_8	BIT(20)
>  #define SNOR_HWCAPS_PP_1_8_8	BIT(21)
>  #define SNOR_HWCAPS_PP_8_8_8	BIT(22)
> +#define SNOR_HWCAPS_PP_8_8_8_DTR	BIT(23)
> +
> +#define SNOR_HWCAPS_OPI_FULL_STR	(SNOR_HWCAPS_READ_8_8_8 | \
> +					 SNOR_HWCAPS_PP_8_8_8)
> +
> +#define SNOR_HWCAPS_OPI_FULL_DTR	(SNOR_HWCAPS_READ_8_8_8_DTR | \
> +					 SNOR_HWCAPS_PP_8_8_8_DTR)
>  
>  #define SNOR_HWCAPS_X_X_X	(SNOR_HWCAPS_READ_2_2_2 |	\
>  				 SNOR_HWCAPS_READ_4_4_4 |	\
> @@ -403,7 +440,9 @@ struct spi_nor_hwcaps {
>  #define SNOR_HWCAPS_DTR		(SNOR_HWCAPS_READ_1_1_1_DTR |	\
>  				 SNOR_HWCAPS_READ_1_2_2_DTR |	\
>  				 SNOR_HWCAPS_READ_1_4_4_DTR |	\
> -				 SNOR_HWCAPS_READ_1_8_8_DTR)
> +				 SNOR_HWCAPS_READ_1_8_8_DTR |	\
> +				 SNOR_HWCAPS_READ_8_8_8_DTR |	\
> +				 SNOR_HWCAPS_PP_8_8_8_DTR)
>  
>  #define SNOR_HWCAPS_ALL		(SNOR_HWCAPS_READ_MASK |	\
>  				 SNOR_HWCAPS_PP_MASK)
> @@ -442,6 +481,7 @@ enum spi_nor_read_command_index {
>  	SNOR_CMD_READ_1_8_8,
>  	SNOR_CMD_READ_8_8_8,
>  	SNOR_CMD_READ_1_8_8_DTR,
> +	SNOR_CMD_READ_8D_8D_8D,
>  
>  	SNOR_CMD_READ_MAX
>  };
> @@ -458,6 +498,7 @@ enum spi_nor_pp_command_index {
>  	SNOR_CMD_PP_1_1_8,
>  	SNOR_CMD_PP_1_8_8,
>  	SNOR_CMD_PP_8_8_8,
> +	SNOR_CMD_PP_8D_8D_8D,
>  
>  	SNOR_CMD_PP_MAX
>  };
> @@ -528,6 +569,11 @@ struct spi_nor_flash_parameter {
>   */
>  struct flash_info;
>  
> +enum extension_cmd_mode {
> +	EXT_CMD_IS_CMD,
> +	EXT_CMD_IS_INVERSE,
> +};
> +
>  /**
>   * struct spi_nor - Structure for defining a the SPI NOR layer
>   * @mtd:		point to a mtd_info structure
> @@ -537,6 +583,7 @@ struct spi_nor_flash_parameter {
>   * @bouncebuf:		bounce buffer used when the buffer passed by the MTD
>   *                      layer is not DMA-able
>   * @bouncebuf_size:	size of the bounce buffer
> + * @ext_cmd_mode:	extension command mode, 0: same, 1: inverse
>   * @info:		spi-nor part JDEC MFR id and other info
>   * @page_size:		the page size of the SPI NOR
>   * @addr_width:		number of address bytes
> @@ -575,6 +622,7 @@ struct spi_nor {
>  	struct spi_mem		*spimem;
>  	u8			*bouncebuf;
>  	size_t			bouncebuf_size;
> +	enum extension_cmd_mode	ext_cmd_mode;
>  	const struct flash_info	*info;
>  	u32			page_size;
>  	u8			addr_width;
> 

-- 
Regards
Vignesh

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

* Re: [PATCH 3/4] mtd: spi-nor: Add Octal 8D-8D-8D mode support for Macronix mx25uw51245g
  2019-11-15  8:58 ` [PATCH 3/4] mtd: spi-nor: Add Octal 8D-8D-8D mode support for Macronix mx25uw51245g Mason Yang
@ 2019-12-04 13:03   ` Vignesh Raghavendra
  2019-12-09  6:38     ` masonccyang
  0 siblings, 1 reply; 17+ messages in thread
From: Vignesh Raghavendra @ 2019-12-04 13:03 UTC (permalink / raw)
  To: Mason Yang, broonie, miquel.raynal, richard, marek.vasut, dwmw2,
	computersforpeace, bbrezillon, tudor.ambarus
  Cc: juliensu, linux-kernel, linux-mtd, linux-spi, Boris Brezillon



On 15/11/19 2:28 pm, Mason Yang wrote:
> mx25uw51245g is a SPI NOR that supports 1-1-1/8-8-8 mode and the SFDPRD
> command but returns an empty SFDP page. This forces us to add a new entry
> in the flash_info table and patch spi_nor_fixups hooks to tweak flash
> parameters for spi_nor_read/pp_setting() and then to enter
> Octal 8D-8D-8D modes.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 114 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/spi-nor.h   |   3 ++
>  2 files changed, 117 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 4cdf52d..9013590 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -615,6 +615,56 @@ static int write_disable(struct spi_nor *nor)
>  }
>  
>  /*
> + * Read configuration register 2, returning its value in the
> + * location. Return the configuration register 2 value.
> + * Returns negative if error occurred.
> + */
> +static int read_cr2(struct spi_nor *nor, u32 addr)

Please prefix spi_nor_* for all new functions. 
Also, include manf name if its vendor specific.

> +{
> +	int ret;
> +
> +	if (nor->spimem) {
> +		struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDCR2, 1),
> +				   SPI_MEM_OP_ADDR(4, addr, 1),
> +				   SPI_MEM_OP_DUMMY(4, 1),
> +				   SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
> +
> +		if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
> +			op.cmd.buswidth = 8;
> +			op.addr.buswidth = 8;
> +			op.dummy.buswidth = 8;
> +			op.data.buswidth = 8;
> +			op.cmd.nbytes = 2;
> +
> +			if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
> +				op.dummy.nbytes *= 2;
> +				op.cmd.dtr = true;
> +				op.addr.dtr = true;
> +				op.dummy.dtr = true;
> +				op.data.dtr = true;
> +			}
> +
> +			if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> +				op.cmd.ext_opcode = ~SPINOR_OP_RDCR2;
> +			else
> +				op.cmd.ext_opcode = SPINOR_OP_RDCR2;
> +		}
> +
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = -ENOTSUPP;
> +	}
> +
> +	if (ret < 0) {
> +		dev_err(nor->dev, "error %d reading CR\n", ret);
> +		return ret;
> +	}
> +
> +	return nor->bouncebuf[0];
> +}
> +
> +/*
>   * Write configuration register 2 one byte
>   * Returns negative if error occurred.
>   */
> @@ -2275,10 +2325,72 @@ static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
>  	return 0;
>  }
>  
> +static 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);
> +
> +static void
> +spi_nor_set_pp_settings(struct spi_nor_pp_command *pp,
> +			u8 opcode,
> +			enum spi_nor_protocol proto);
> +
> +static void
> +mx25uw51245g_default_init(struct spi_nor *nor)
> +{
> +	struct spi_nor_flash_parameter *params = &nor->params;
> +
> +	if (!(nor->spimem->spi->mode & (SPI_RX_OCTAL | SPI_TX_OCTAL)))
> +		return;
> +
> +	/* Octal 8S-8S-8S mode */
> +	params->hwcaps.mask |= SNOR_HWCAPS_OPI_FULL_STR;
> +	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_8_8_8],
> +				  0, 20, SPINOR_OP_READ_8_8_8,
> +				  SNOR_PROTO_8_8_8);
> +
> +	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_8_8_8],
> +				SPINOR_OP_PP_8_8_8, SNOR_PROTO_8_8_8);
> +
> +	/* Octal 8D-8D-8D mode */
> +	params->hwcaps.mask |= SNOR_HWCAPS_OPI_FULL_DTR;
> +	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_8D_8D_8D],
> +				  0, 20, SPINOR_OP_READ_8D_8D_8D,
> +				  SNOR_PROTO_8_8_8_DTR);
> +
> +	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_8D_8D_8D],
> +				SPINOR_OP_PP_8D_8D_8D, SNOR_PROTO_8_8_8_DTR);
> +
> +	nor->ext_cmd_mode = EXT_CMD_IS_INVERSE;
> +}

I don't see anything that is macronix specific here.. Can this be moved to
generic code with information parsed from SFDP table?

> +
> +static void
> +mx25uw51245g_post_sfdp_fixups(struct spi_nor *nor)
> +{
> +	struct spi_nor_flash_parameter *params = &nor->params;
> +	u8 cr2;
> +
> +	cr2 = read_cr2(nor, CR2_REG0) & CR2_REG0_MODE_MASK;
> +
> +	if (params->hwcaps.mask & SNOR_HWCAPS_OPI_FULL_DTR)
> +		cr2 |= CR2_REG0_MODE_OPI_DTR;
> +	else if (params->hwcaps.mask & SNOR_HWCAPS_OPI_FULL_STR)
> +		cr2 |= CR2_REG0_MODE_OPI_STR;
> +
> +	write_cr2(nor, CR2_REG0, cr2);
> +}
> +

I see this as a misuse of sfdp_fixups hook:

 * @post_sfdp: called after SFDP has been parsed (is also called for SPI NORs
 *             that do not support RDSFDP). Typically used to tweak various
 *             parameters that could not be extracted by other means (i.e.
 *             when information provided by the SFDP/flash_info tables are
 *             incomplete or wrong).
 *


This should only tweak options parsed by SFDP and not be used to
configure flash to a different mode. Please add a separate function 
to do so. See https://patchwork.kernel.org/patch/10638085/

Also, I guess JESD216C provides a way to discover command sequence 
to enter OPI mode?

>  static struct spi_nor_fixups mx25l25635_fixups = {
>  	.post_bfpt = mx25l25635_post_bfpt_fixups,
>  };
>  
> +static struct spi_nor_fixups mx25uw51245g_fixups = {
> +	.default_init = mx25uw51245g_default_init,
> +	.post_sfdp = mx25uw51245g_post_sfdp_fixups,
> +};
> +
>  static void gd25q256_default_init(struct spi_nor *nor)
>  {
>  	/*
> @@ -2442,6 +2554,8 @@ static void gd25q256_default_init(struct spi_nor *nor)
>  			 SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>  			 .fixups = &mx25l25635_fixups },
>  	{ "mx25u25635f", INFO(0xc22539, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_4B_OPCODES) },
> +	{ "mx25uw51245g", INFO(0xc2813a, 0, 64 * 1024, 1024, SECT_4K | SPI_NOR_4B_OPCODES)
> +				.fixups = &mx25uw51245g_fixups},
>  	{ "mx25v8035f",  INFO(0xc22314, 0, 64 * 1024,  16,
>  			 SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>  	{ "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) },
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 2e720ca..3aa54dd 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -114,6 +114,9 @@
>  /* Write CFG Reg 2 - defined in JEDEC JESD216C. */
>  #define SPINOR_OP_WRCR2		0x72	/* Write configuration register 2 */
>  
> +/* Used for Macronix flashes only */
> +#define SPINOR_OP_RDCR2		0x71	/* Read configuration register 2 */
> +
>  /* Used for Macronix and Winbond flashes. */
>  #define SPINOR_OP_EN4B		0xb7	/* Enter 4-byte mode */
>  #define SPINOR_OP_EX4B		0xe9	/* Exit 4-byte mode */
> 

-- 
Regards
Vignesh

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

* Re: [PATCH 3/4] mtd: spi-nor: Add Octal 8D-8D-8D mode support for Macronix mx25uw51245g
  2019-12-04 13:03   ` Vignesh Raghavendra
@ 2019-12-09  6:38     ` masonccyang
  0 siblings, 0 replies; 17+ messages in thread
From: masonccyang @ 2019-12-09  6:38 UTC (permalink / raw)
  To: Vignesh Raghavendra
  Cc: bbrezillon, Boris Brezillon, broonie, computersforpeace, dwmw2,
	juliensu, linux-kernel, linux-mtd, linux-spi, marek.vasut,
	miquel.raynal, richard, tudor.ambarus


Hi Vignesh,

> > 
> >  /*
> > + * Read configuration register 2, returning its value in the
> > + * location. Return the configuration register 2 value.
> > + * Returns negative if error occurred.
> > + */
> > +static int read_cr2(struct spi_nor *nor, u32 addr)
> 
> Please prefix spi_nor_* for all new functions. 
> Also, include manf name if its vendor specific.

okay, will fix it.

> 
> > +{
> > +   int ret;
> > +
> > +   if (nor->spimem) {
> > +      struct spi_mem_op op =
> > +         SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDCR2, 1),
> > +               SPI_MEM_OP_ADDR(4, addr, 1),
> > +               SPI_MEM_OP_DUMMY(4, 1),
> > +               SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
> > +
> > +      if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
> > +         op.cmd.buswidth = 8;
> > +         op.addr.buswidth = 8;
> > +         op.dummy.buswidth = 8;
> > +         op.data.buswidth = 8;
> > +         op.cmd.nbytes = 2;
> > +
> > +         if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
> > +            op.dummy.nbytes *= 2;
> > +            op.cmd.dtr = true;
> > +            op.addr.dtr = true;
> > +            op.dummy.dtr = true;
> > +            op.data.dtr = true;
> > +         }
> > +
> > +         if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> > +            op.cmd.ext_opcode = ~SPINOR_OP_RDCR2;
> > +         else
> > +            op.cmd.ext_opcode = SPINOR_OP_RDCR2;
> > +      }
> > +
> > +      ret = spi_mem_exec_op(nor->spimem, &op);
> > +   } else {
> > +      ret = -ENOTSUPP;
> > +   }
> > +
> > +   if (ret < 0) {
> > +      dev_err(nor->dev, "error %d reading CR\n", ret);
> > +      return ret;
> > +   }
> > +
> > +   return nor->bouncebuf[0];
> > +}
> > +
> > +/*
> >   * Write configuration register 2 one byte
> >   * Returns negative if error occurred.
> >   */
> > @@ -2275,10 +2325,72 @@ static int spi_nor_spansion_clear_sr_bp(struct 
spi_nor *nor)
> >     return 0;
> >  }
> > 
> > +static 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);
> > +
> > +static void
> > +spi_nor_set_pp_settings(struct spi_nor_pp_command *pp,
> > +         u8 opcode,
> > +         enum spi_nor_protocol proto);
> > +
> > +static void
> > +mx25uw51245g_default_init(struct spi_nor *nor)
> > +{
> > +   struct spi_nor_flash_parameter *params = &nor->params;
> > +
> > +   if (!(nor->spimem->spi->mode & (SPI_RX_OCTAL | SPI_TX_OCTAL)))
> > +      return;
> > +
> > +   /* Octal 8S-8S-8S mode */
> > +   params->hwcaps.mask |= SNOR_HWCAPS_OPI_FULL_STR;
> > +   spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_8_8_8],
> > +              0, 20, SPINOR_OP_READ_8_8_8,
> > +              SNOR_PROTO_8_8_8);
> > +
> > +   spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_8_8_8],
> > +            SPINOR_OP_PP_8_8_8, SNOR_PROTO_8_8_8);
> > +
> > +   /* Octal 8D-8D-8D mode */
> > +   params->hwcaps.mask |= SNOR_HWCAPS_OPI_FULL_DTR;
> > +   spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_8D_8D_8D],
> > +              0, 20, SPINOR_OP_READ_8D_8D_8D,
> > +              SNOR_PROTO_8_8_8_DTR);
> > +
> > + 
spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_8D_8D_8D],
> > +            SPINOR_OP_PP_8D_8D_8D, SNOR_PROTO_8_8_8_DTR);
> > +
> > +   nor->ext_cmd_mode = EXT_CMD_IS_INVERSE;
> > +}
> 
> I don't see anything that is macronix specific here.. Can this be moved 
to
> generic code with information parsed from SFDP table?

This mx25uw51245g device support SFDP command but returns an empty SFDP 
page.

> 
> > +
> > +static void
> > +mx25uw51245g_post_sfdp_fixups(struct spi_nor *nor)
> > +{
> > +   struct spi_nor_flash_parameter *params = &nor->params;
> > +   u8 cr2;
> > +
> > +   cr2 = read_cr2(nor, CR2_REG0) & CR2_REG0_MODE_MASK;
> > +
> > +   if (params->hwcaps.mask & SNOR_HWCAPS_OPI_FULL_DTR)
> > +      cr2 |= CR2_REG0_MODE_OPI_DTR;
> > +   else if (params->hwcaps.mask & SNOR_HWCAPS_OPI_FULL_STR)
> > +      cr2 |= CR2_REG0_MODE_OPI_STR;
> > +
> > +   write_cr2(nor, CR2_REG0, cr2);
> > +}
> > +
> 
> I see this as a misuse of sfdp_fixups hook:
> 
>  * @post_sfdp: called after SFDP has been parsed (is also called for SPI 
NORs
>  *             that do not support RDSFDP). Typically used to tweak 
various
>  *             parameters that could not be extracted by other means 
(i.e.
>  *             when information provided by the SFDP/flash_info tables 
are
>  *             incomplete or wrong).
>  *
> 
> 
> This should only tweak options parsed by SFDP and not be used to
> configure flash to a different mode. Please add a separate function 
> to do so. See https://patchwork.kernel.org/patch/10638085/
> 

okay.
My idea is that device changed to 8D-8D-8D stateful mode after SFDP 
parsed. 
But if SFDP page table is broken in device and driver will just configure 
the device into 8D-8D-8D mode directly.


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.

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


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

* Re: [PATCH 2/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode
  2019-12-04 12:46   ` Vignesh Raghavendra
@ 2019-12-09  7:56     ` masonccyang
  2019-12-09  9:53       ` Vignesh Raghavendra
  0 siblings, 1 reply; 17+ messages in thread
From: masonccyang @ 2019-12-09  7:56 UTC (permalink / raw)
  To: Vignesh Raghavendra
  Cc: bbrezillon, broonie, computersforpeace, dwmw2, juliensu,
	linux-kernel, linux-mtd, linux-spi, marek.vasut, miquel.raynal,
	richard, tudor.ambarus


Hi Vignesh,

> 
> On 15/11/19 2:28 pm, Mason Yang wrote:
> > According to JESD216C (JEDEC Basic Flash Parameter Table 18th DWORD)
> > Octal DTR(8D-8D-8D) command and command extension (00b: same, 01b: 
inverse)
> > to add extension command mode in spi_nor struct and to add write_cr2
> > (Write CFG Reg 2) for 8-8-8/8D-8D-8D mode sequences enable.
> > 
> 
> But I don't see any code setting "nor->ext_cmd_mode" based on BFPT?
> 
> Any new feature that we add to spi-nor should make use of autodiscovery
> feature made possible by SFDP tables. Could you modify the patch to
> discover capabilities supported by flash and opcodes to be used from
> SFDP table?

Got it but our device will return a empty SFDP table.

> 
> 
> > Define the relevant macrons and enum to add such modes and make sure
> > op->xxx.dtr fields, command nbytes and extension command are properly
> > filled and unmask DTR and X-X-X modes in 
spi_nor_spimem_adjust_hwcaps()
> > so that DTR and X-X-X support detection is done through
> > spi_mem_supports_op().
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c | 159 
++++++++++++++++++++++++++++++++++++++++--
> >  include/linux/mtd/spi-nor.h   |  58 +++++++++++++--
> >  2 files changed, 206 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c 
b/drivers/mtd/spi-nor/spi-nor.c
> > index 7acf4a9..4cdf52d 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -320,6 +320,23 @@ static ssize_t spi_nor_spimem_read_data(struct 
spi_nor 
> *nor, loff_t from,
> >     /* convert the dummy cycles to the number of bytes */
> >     op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
> > 
> > +   if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
> > +      op.cmd.nbytes = 2;
> 
> Can we get this info from SFDP too?
> 
> > +
> > +      if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> > +         op.cmd.ext_opcode = ~nor->read_opcode;
> > +      else
> > +         op.cmd.ext_opcode = nor->read_opcode;
> > +
> > +      if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
> > +         op.dummy.nbytes *= 2;
> 
> > +         op.cmd.dtr = true;
> > +         op.addr.dtr = true;
> > +         op.dummy.dtr = true;
> > +         op.data.dtr = true;
> 
> Can we have a macro for this initialization?

okay, will fix it.

> 
> 
> > +      }
> > +   }
> > +
> >     return spi_nor_spimem_xfer_data(nor, &op);
> >  }
> > 
> > @@ -367,6 +384,21 @@ static ssize_t spi_nor_spimem_write_data(struct 
spi_nor
> *nor, loff_t to,
> >     if (nor->program_opcode == SPINOR_OP_AAI_WP && 
nor->sst_write_second)
> >        op.addr.nbytes = 0;
> > 
> > +   if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
> > +      op.cmd.nbytes = 2;
> > +
> > +      if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> > +         op.cmd.ext_opcode = ~nor->program_opcode;
> > +      else
> > +         op.cmd.ext_opcode = nor->program_opcode;
> > +
> > +      if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto)) {
> > +         op.cmd.dtr = true;
> > +         op.addr.dtr = true;
> > +         op.data.dtr = true;
> > +      }
> > +   }
> > +
> >     return spi_nor_spimem_xfer_data(nor, &op);
> >  }
> > 
> > @@ -404,6 +436,30 @@ static int read_sr(struct spi_nor *nor)
> >                 SPI_MEM_OP_NO_DUMMY,
> >                 SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
> > 
> 
> This is not based on the latest tree.
> 
> > +      if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
> > +         op.cmd.buswidth = 8;
> > +         op.addr.buswidth = 8;
> > +         op.dummy.buswidth = 8;
> > +         op.data.buswidth = 8;
> > +         op.cmd.nbytes = 2;
> > +         op.addr.nbytes = 4;
> > +         op.dummy.nbytes = 4;
> > +         op.addr.val = 0;
> 
> This is not scalable... There will be bunch of if...else ladders when we
> want to support other X-X-X modes... Can't these be derived from
> nor->reg_proto? And then borrow the logic from 
spi_nor_spimem_read_data()?
> 

Got it !

> 
> Could you have a look at Boris's initial submission to add 8-8-8 mode at
> https://patchwork.kernel.org/cover/10638055/ ?
> You could use that series as the base for your changes/additions.

Got it.
My idea is to support 8D-8D-8D mode with a minimum patches because 
there is no define for 1D-1D-1D, 2D-2D-2D and 4D-4D-4D mode in JEDEC 
if I am right.

> 
> I am fine if you want to call 2nd byte of opcode as ext_opcode
> 
> Regards
> Vignesh
> 

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.

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


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

* Re: [PATCH 2/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode
  2019-12-09  7:56     ` masonccyang
@ 2019-12-09  9:53       ` Vignesh Raghavendra
  2019-12-10  7:21         ` masonccyang
  0 siblings, 1 reply; 17+ messages in thread
From: Vignesh Raghavendra @ 2019-12-09  9:53 UTC (permalink / raw)
  To: masonccyang
  Cc: bbrezillon, juliensu, richard, tudor.ambarus, linux-kernel,
	linux-spi, marek.vasut, broonie, linux-mtd, miquel.raynal,
	computersforpeace, dwmw2

Hi,

On 09/12/19 1:26 pm, masonccyang@mxic.com.tw wrote:
> 
> Hi Vignesh,
> 
>>
>> On 15/11/19 2:28 pm, Mason Yang wrote:
>>> According to JESD216C (JEDEC Basic Flash Parameter Table 18th DWORD)
>>> Octal DTR(8D-8D-8D) command and command extension (00b: same, 01b: 
> inverse)
>>> to add extension command mode in spi_nor struct and to add write_cr2
>>> (Write CFG Reg 2) for 8-8-8/8D-8D-8D mode sequences enable.
>>>
>>
>> But I don't see any code setting "nor->ext_cmd_mode" based on BFPT?
>>
>> Any new feature that we add to spi-nor should make use of autodiscovery
>> feature made possible by SFDP tables. Could you modify the patch to
>> discover capabilities supported by flash and opcodes to be used from
>> SFDP table?
> 
> Got it but our device will return a empty SFDP table.
> 

If flash you tested on does not support JEDEC 216C then don't mention
about it. Above commit message gives an impression that flash in JEDEC
216C compliant.

>>
>>
>>> Define the relevant macrons and enum to add such modes and make sure
>>> op->xxx.dtr fields, command nbytes and extension command are properly
>>> filled and unmask DTR and X-X-X modes in 
> spi_nor_spimem_adjust_hwcaps()
>>> so that DTR and X-X-X support detection is done through
>>> spi_mem_supports_op().
>>>
[...]
>>> @@ -404,6 +436,30 @@ static int read_sr(struct spi_nor *nor)
>>>                 SPI_MEM_OP_NO_DUMMY,
>>>                 SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
>>>
>>
>> This is not based on the latest tree.
>>
>>> +      if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
>>> +         op.cmd.buswidth = 8;
>>> +         op.addr.buswidth = 8;
>>> +         op.dummy.buswidth = 8;
>>> +         op.data.buswidth = 8;
>>> +         op.cmd.nbytes = 2;
>>> +         op.addr.nbytes = 4;
>>> +         op.dummy.nbytes = 4;
>>> +         op.addr.val = 0;
>>
>> This is not scalable... There will be bunch of if...else ladders when we
>> want to support other X-X-X modes... Can't these be derived from
>> nor->reg_proto? And then borrow the logic from 
> spi_nor_spimem_read_data()?
>>
> 
> Got it !
> 
>>
>> Could you have a look at Boris's initial submission to add 8-8-8 mode at
>> https://patchwork.kernel.org/cover/10638055/ ?
>> You could use that series as the base for your changes/additions.
> 
> Got it.
> My idea is to support 8D-8D-8D mode with a minimum patches because 
> there is no define for 1D-1D-1D, 2D-2D-2D and 4D-4D-4D mode in JEDEC 
> if I am right.
> 

JESD251-A1 does talk about 4S-4D-4D right? Also none of the JEDEC
standards prohibit flash vendors from supporting other X-X-X modes.

I think you haven't thought about bigger picture here. Flash devices
that support other mode exist today and we would need the framework to
be built such that these modes can be added in future.

I suggest you start with Boris's series [1] as base and port it to
latest kernel. Isn't that series alone enough to support Macronix Octal
flashes at least?
If required, you could also always include additional patches adding new
features.

[1] https://patchwork.kernel.org/cover/10638055/

-- 
Regards
Vignesh

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

* Re: [PATCH 2/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode
  2019-12-09  9:53       ` Vignesh Raghavendra
@ 2019-12-10  7:21         ` masonccyang
  0 siblings, 0 replies; 17+ messages in thread
From: masonccyang @ 2019-12-10  7:21 UTC (permalink / raw)
  To: Vignesh Raghavendra
  Cc: bbrezillon, broonie, computersforpeace, dwmw2, juliensu,
	linux-kernel, linux-mtd, linux-spi, marek.vasut, miquel.raynal,
	richard, tudor.ambarus


Hi Vignesh,


> >> On 15/11/19 2:28 pm, Mason Yang wrote:
> >>> According to JESD216C (JEDEC Basic Flash Parameter Table 18th DWORD)
> >>> Octal DTR(8D-8D-8D) command and command extension (00b: same, 01b: 
> > inverse)
> >>> to add extension command mode in spi_nor struct and to add write_cr2
> >>> (Write CFG Reg 2) for 8-8-8/8D-8D-8D mode sequences enable.
> >>>
> >>
> >> But I don't see any code setting "nor->ext_cmd_mode" based on BFPT?
> >>
> >> Any new feature that we add to spi-nor should make use of 
autodiscovery
> >> feature made possible by SFDP tables. Could you modify the patch to
> >> discover capabilities supported by flash and opcodes to be used from
> >> SFDP table?
> > 
> > Got it but our device will return a empty SFDP table.
> > 
> 
> If flash you tested on does not support JEDEC 216C then don't mention
> about it. Above commit message gives an impression that flash in JEDEC
> 216C compliant.
> 

okay, got it.

> >>
> >>
> >>> Define the relevant macrons and enum to add such modes and make sure
> >>> op->xxx.dtr fields, command nbytes and extension command are 
properly
> >>> filled and unmask DTR and X-X-X modes in 
> > spi_nor_spimem_adjust_hwcaps()
> >>> so that DTR and X-X-X support detection is done through
> >>> spi_mem_supports_op().
> >>>
> [...]
> >>> @@ -404,6 +436,30 @@ static int read_sr(struct spi_nor *nor)
> >>>                 SPI_MEM_OP_NO_DUMMY,
> >>>                 SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
> >>>
> >>
> >> This is not based on the latest tree.
> >>
> >>> +      if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
> >>> +         op.cmd.buswidth = 8;
> >>> +         op.addr.buswidth = 8;
> >>> +         op.dummy.buswidth = 8;
> >>> +         op.data.buswidth = 8;
> >>> +         op.cmd.nbytes = 2;
> >>> +         op.addr.nbytes = 4;
> >>> +         op.dummy.nbytes = 4;
> >>> +         op.addr.val = 0;
> >>
> >> This is not scalable... There will be bunch of if...else ladders when 
we
> >> want to support other X-X-X modes... Can't these be derived from
> >> nor->reg_proto? And then borrow the logic from 
> > spi_nor_spimem_read_data()?
> >>
> > 
> > Got it !
> > 
> >>
> >> Could you have a look at Boris's initial submission to add 8-8-8 mode 
at
> >> https://patchwork.kernel.org/cover/10638055/ ?
> >> You could use that series as the base for your changes/additions.
> > 
> > Got it.
> > My idea is to support 8D-8D-8D mode with a minimum patches because 
> > there is no define for 1D-1D-1D, 2D-2D-2D and 4D-4D-4D mode in JEDEC 
> > if I am right.
> > 
> 
> JESD251-A1 does talk about 4S-4D-4D right? Also none of the JEDEC
> standards prohibit flash vendors from supporting other X-X-X modes.
> 
> I think you haven't thought about bigger picture here. Flash devices
> that support other mode exist today and we would need the framework to
> be built such that these modes can be added in future.
> 
> I suggest you start with Boris's series [1] as base and port it to
> latest kernel. Isn't that series alone enough to support Macronix Octal
> flashes at least?
> If required, you could also always include additional patches adding new
> features.

okay.

> 
> [1] https://patchwork.kernel.org/cover/10638055/
> 
> -- 
> Regards
> Vignesh

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.

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


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

* Re: [PATCH 0/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode
  2019-11-15  8:58 [PATCH 0/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode Mason Yang
                   ` (3 preceding siblings ...)
  2019-11-15  8:58 ` [PATCH 4/4] spi: mxic: Add support for Octal 8D-8D-8D mode Mason Yang
@ 2019-12-10 17:00 ` Tudor.Ambarus
  2019-12-11  2:14   ` masonccyang
  4 siblings, 1 reply; 17+ messages in thread
From: Tudor.Ambarus @ 2019-12-10 17:00 UTC (permalink / raw)
  To: masonccyang, broonie, miquel.raynal, richard, marek.vasut, dwmw2,
	computersforpeace, vigneshr, bbrezillon
  Cc: juliensu, linux-kernel, linux-mtd, linux-spi

Hi, Mason,

From the discussion you had with Vignesh, I understand that a v2 will follow. A
nit below.

On 11/15/19 10:58 AM, Mason Yang wrote:
> Hello,
> 
> This is repost of patchset from Boris Brezillon's
> [RFC,00/18] mtd: spi-nor: Proposal for 8-8-8 mode support [1].
> 

[cut]

> Mason Yang (4):

Did you intentionally overwrite Boris's authorship? If yes, would you please
describe what changed from Boris's patch set?

Cheers,
ta

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

* Re: [PATCH 0/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode
  2019-12-10 17:00 ` [PATCH 0/4] mtd: spi-nor: " Tudor.Ambarus
@ 2019-12-11  2:14   ` masonccyang
  0 siblings, 0 replies; 17+ messages in thread
From: masonccyang @ 2019-12-11  2:14 UTC (permalink / raw)
  To: Tudor.Ambarus
  Cc: bbrezillon, broonie, computersforpeace, dwmw2, juliensu,
	linux-kernel, linux-mtd, linux-spi, marek.vasut, miquel.raynal,
	richard, vigneshr


Hi Tudor,

> 
> Re: [PATCH 0/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode
> 
> Hi, Mason,
> 
> From the discussion you had with Vignesh, I understand that a v2 will 
follow. A
> nit below.
> 
> On 11/15/19 10:58 AM, Mason Yang wrote:
> > Hello,
> > 
> > This is repost of patchset from Boris Brezillon's
> > [RFC,00/18] mtd: spi-nor: Proposal for 8-8-8 mode support [1].
> > 
> 
> [cut]
> 
> > Mason Yang (4):
> 
> Did you intentionally overwrite Boris's authorship? If yes, would you 
please
> describe what changed from Boris's patch set?

okay, sure.
I will describe it in v2 patch set.

> 
> Cheers,
> ta

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.

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


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

end of thread, other threads:[~2019-12-11  2:14 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-15  8:58 [PATCH 0/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode Mason Yang
2019-11-15  8:58 ` [PATCH 1/4] spi: spi-mem: " Mason Yang
2019-11-15  8:58 ` [PATCH 2/4] mtd: spi-nor: " Mason Yang
2019-11-15 12:30   ` kbuild test robot
2019-11-15 13:42   ` kbuild test robot
2019-12-04 12:46   ` Vignesh Raghavendra
2019-12-09  7:56     ` masonccyang
2019-12-09  9:53       ` Vignesh Raghavendra
2019-12-10  7:21         ` masonccyang
2019-11-15  8:58 ` [PATCH 3/4] mtd: spi-nor: Add Octal 8D-8D-8D mode support for Macronix mx25uw51245g Mason Yang
2019-12-04 13:03   ` Vignesh Raghavendra
2019-12-09  6:38     ` masonccyang
2019-11-15  8:58 ` [PATCH 4/4] spi: mxic: Add support for Octal 8D-8D-8D mode Mason Yang
2019-11-15 14:39   ` kbuild test robot
2019-11-16  7:20   ` kbuild test robot
2019-12-10 17:00 ` [PATCH 0/4] mtd: spi-nor: " Tudor.Ambarus
2019-12-11  2:14   ` masonccyang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).