[PATCH v4 0/6] Add octal DTR support for Macronix flash

[PATCH v4 0/6] Add octal DTR support for Macronix flash

[Jaime Liao]

From: JaimeLiao <jaimeliao@mxic.com.tw>

Add method for manufacturer read id function.
Add method for Macronix Octal DTR Eable/Disable.
Merge Tudor's patch "Allow specifying the byte order in DTR mode"

v4:
  Add patch for adding manufacturer read id function.
  remove patch "hook manufacturer by checking first byte id"

v3:
  Add patch for hook manufacturer by comparing ID 1st byte.
  Add patches for specifying the byte order in DTR mode by merging
  Tudor's patch.

v2:
  Following exsting rules to re-create Macronix specify Octal DTR method.
  change signature to jaimeliao@mxic.com.tw
  Clear sector size information in flash INFO.

JaimeLiao (6):
  mtd: spi-nor: Add manufacturer read id function
  mtd: spi-nor: add Octal DTR support for Macronix flash
  mtd: spi-nor: add support for Macronix Octal flash
  spi: spi-mem: Allow specifying the byte order in DTR mode
  mtd: spi-nor: core: Allow specifying the byte order in DTR mode
  mtd: spi-nor: sfdp: Get the 8D-8D-8D byte order from BFPT

 drivers/mtd/spi-nor/core.c     |  38 +++++++-
 drivers/mtd/spi-nor/core.h     |   7 ++
 drivers/mtd/spi-nor/macronix.c | 160 +++++++++++++++++++++++++++++++++
 drivers/mtd/spi-nor/sfdp.c     |   4 +
 drivers/mtd/spi-nor/sfdp.h     |   1 +
 drivers/spi/spi-mem.c          |   4 +
 include/linux/spi/spi-mem.h    |   6 ++
 7 files changed, 218 insertions(+), 2 deletions(-)

-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[Jaime Liao]

From: JaimeLiao <jaimeliao@mxic.com.tw>

Add manufacturer read id function because of some flash
may change data format when read id in octal dtr mode.

Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
---
 drivers/mtd/spi-nor/core.c     | 30 ++++++++++++++++++++++++++++--
 drivers/mtd/spi-nor/core.h     |  6 ++++++
 drivers/mtd/spi-nor/macronix.c | 18 ++++++++++++++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 1b0c6770c14e..7ee624b16e17 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -408,7 +408,7 @@ int spi_nor_write_disable(struct spi_nor *nor)
 }
 
 /**
- * spi_nor_read_id() - Read the JEDEC ID.
+ * spi_nor_default_read_id() - Read the JEDEC ID.
  * @nor:	pointer to 'struct spi_nor'.
  * @naddr:	number of address bytes to send. Can be zero if the operation
  *		does not need to send an address.
@@ -420,7 +420,7 @@ int spi_nor_write_disable(struct spi_nor *nor)
  *
  * Return: 0 on success, -errno otherwise.
  */
-int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
+int spi_nor_default_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
 		    enum spi_nor_protocol proto)
 {
 	int ret;
@@ -438,6 +438,32 @@ int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
 	return ret;
 }
 
+/**
+ * spi_nor_read_id() - Read ID by manufacturer read id function.
+ * @nor:	pointer to 'struct spi_nor'.
+ * @naddr:	number of address bytes to send. Can be zero if the operation
+ *		does not need to send an address.
+ * @ndummy:	number of dummy bytes to send after an opcode or address. Can
+ *		be zero if the operation does not require dummy bytes.
+ * @id:		pointer to a DMA-able buffer where the value of the JEDEC ID
+ *		will be written.
+ * @proto:	the SPI protocol for register operation.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
+		    enum spi_nor_protocol proto)
+{
+	int ret;
+
+	if (nor->manufacturer && nor->manufacturer->fixups && nor->manufacturer->fixups->read_id)
+		ret = nor->manufacturer->fixups->read_id(nor, naddr, ndummy, id, proto);
+	else
+		ret = spi_nor_default_read_id(nor, naddr, ndummy, id, proto);
+
+	return ret;
+}
+
 /**
  * spi_nor_read_sr() - Read the Status Register.
  * @nor:	pointer to 'struct spi_nor'.
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 9217379b9cfe..92cbc2d3f7fe 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -424,6 +424,8 @@ struct spi_nor_flash_parameter {
  * @late_init: used to initialize flash parameters that are not declared in the
  *             JESD216 SFDP standard, or where SFDP tables not defined at all.
  *             Will replace the default_init() hook.
+ * @read_id:   used to read id which may change format after enter into
+	       octal dtr mode.
  *
  * Those hooks can be used to tweak the SPI NOR configuration when the SFDP
  * table is broken or not available.
@@ -435,6 +437,8 @@ struct spi_nor_fixups {
 			 const struct sfdp_bfpt *bfpt);
 	int (*post_sfdp)(struct spi_nor *nor);
 	int (*late_init)(struct spi_nor *nor);
+	int (*read_id)(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
+		       enum spi_nor_protocol reg_proto);
 };
 
 /**
@@ -667,6 +671,8 @@ void spi_nor_unlock_and_unprep(struct spi_nor *nor);
 int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
 int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor);
 int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor);
+int spi_nor_default_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
+			    enum spi_nor_protocol reg_proto);
 int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
 		    enum spi_nor_protocol reg_proto);
 int spi_nor_read_sr(struct spi_nor *nor, u8 *sr);
diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index eb149e517c1f..8ab47691dfbb 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -118,9 +118,27 @@ static int macronix_nor_late_init(struct spi_nor *nor)
 	return 0;
 }
 
+static int macronix_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
+				enum spi_nor_protocol proto)
+{
+	int i, ret;
+
+	ret = spi_nor_default_read_id(nor, naddr, ndummy, id, proto);
+	/* Retrieve odd array and re-sort id because of read id format will be A-A-B-B-C-C
+	 * after enter into octal dtr mode for Macronix flashes.
+	 */
+	if (proto == SNOR_PROTO_8_8_8_DTR) {
+		for (i = 0; i < nor->info->id_len; i++)
+			id[i] = id[i*2];
+	}
+
+	return ret;
+}
+
 static const struct spi_nor_fixups macronix_nor_fixups = {
 	.default_init = macronix_nor_default_init,
 	.late_init = macronix_nor_late_init,
+	.read_id = macronix_nor_read_id,
 };
 
 const struct spi_nor_manufacturer spi_nor_macronix = {
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 2/6] mtd: spi-nor: add Octal DTR support for Macronix flash

[Jaime Liao]

From: JaimeLiao <jaimeliao@mxic.com.tw>

Create Macronix specify method for enable Octal DTR mode and
set 20 dummy cycles to allow running at the maximum supported
frequency for Macronix Octal flash.

Use number of dummy cycles which is parse by SFDP then convert
it to bit pattern and set in CR2 register.
Set CR2 register for enable octal dtr mode.

Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
Co-developed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/mtd/spi-nor/macronix.c | 97 ++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 8ab47691dfbb..28c49c98503a 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -8,6 +8,24 @@
 
 #include "core.h"
 
+#define SPINOR_OP_MXIC_RD_ANY_REG	0x71		/* Read volatile configuration register 2 */
+#define SPINOR_OP_MXIC_WR_ANY_REG	0x72		/* Write volatile configuration register 2 */
+#define SPINOR_REG_MXIC_CR2_MODE	0x00000000	/* CR2 address for setting octal DTR mode */
+#define SPINOR_REG_MXIC_CR2_DC		0x00000300	/* CR2 address for setting dummy cycles */
+#define SPINOR_REG_MXIC_OPI_DTR_EN	0x2		/* Enable Octal DTR */
+#define SPINOR_REG_MXIC_SPI_EN		0x0		/* Enable SPI */
+#define SPINOR_REG_MXIC_ADDR_BYTES	4		/* Fixed R/W volatile address bytes to 4 */
+/* Convert dummy cycles to bit pattern */
+#define SPINOR_REG_MXIC_DC(p) \
+		((20 - p)/2)
+
+/* Macronix SPI NOR flash operations. */
+#define MXIC_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf)		\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_MXIC_WR_ANY_REG, 0),		\
+		   SPI_MEM_OP_ADDR(naddr, addr, 0),			\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_DATA_OUT(ndata, buf, 0))
+
 static int
 mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
 			    const struct sfdp_parameter_header *bfpt_header,
@@ -105,6 +123,84 @@ static const struct flash_info macronix_nor_parts[] = {
 		FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
 };
 
+static int macronix_nor_octal_dtr_en(struct spi_nor *nor)
+{
+	struct spi_mem_op op;
+	u8 *buf = nor->bouncebuf;
+	int ret;
+
+	/* Use dummy cycles which is parse by SFDP and convert to bit pattern. */
+	buf[0] = SPINOR_REG_MXIC_DC(nor->params->reads[SNOR_CMD_READ_8_8_8_DTR].num_wait_states);
+	op = (struct spi_mem_op)
+		MXIC_NOR_WR_ANY_REG_OP(SPINOR_REG_MXIC_ADDR_BYTES,
+				       SPINOR_REG_MXIC_CR2_DC, 1, buf);
+
+	ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
+	if (ret)
+		return ret;
+
+	/* Set the octal and DTR enable bits. */
+	buf[0] = SPINOR_REG_MXIC_OPI_DTR_EN;
+	op = (struct spi_mem_op)
+		MXIC_NOR_WR_ANY_REG_OP(SPINOR_REG_MXIC_ADDR_BYTES,
+				       SPINOR_REG_MXIC_CR2_MODE, 1, buf);
+	ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
+	if (ret)
+		return ret;
+
+	/* Read flash ID to make sure the switch was successful. */
+	ret = spi_nor_read_id(nor, 4, 4, buf, SNOR_PROTO_8_8_8_DTR);
+	if (ret) {
+		dev_dbg(nor->dev, "error %d reading JEDEC ID after enabling 8D-8D-8D mode\n", ret);
+		return ret;
+	}
+
+	if (memcmp(buf, nor->info->id, nor->info->id_len))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int macronix_nor_octal_dtr_dis(struct spi_nor *nor)
+{
+	struct spi_mem_op op;
+	u8 *buf = nor->bouncebuf;
+	int ret;
+
+	/*
+	 * The register is 1-byte wide, but 1-byte transactions are not
+	 * allowed in 8D-8D-8D mode. Since there is no register at the
+	 * next location, just initialize the value to 0 and let the
+	 * transaction go on.
+	 */
+	buf[0] = SPINOR_REG_MXIC_SPI_EN;
+	buf[1] = 0x0;
+	op = (struct spi_mem_op)
+		MXIC_NOR_WR_ANY_REG_OP(SPINOR_REG_MXIC_ADDR_BYTES,
+				       SPINOR_REG_MXIC_CR2_MODE, 2, buf);
+	ret = spi_nor_write_any_volatile_reg(nor, &op, SNOR_PROTO_8_8_8_DTR);
+	if (ret)
+		return ret;
+
+	/* Read flash ID to make sure the switch was successful. */
+	ret = spi_nor_read_id(nor, 0, 0, buf, SNOR_PROTO_1_1_1);
+	if (ret) {
+		dev_dbg(nor->dev, "error %d reading JEDEC ID after disabling 8D-8D-8D mode\n", ret);
+		return ret;
+	}
+
+	if (memcmp(buf, nor->info->id, nor->info->id_len))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int macronix_nor_set_octal_dtr(struct spi_nor *nor, bool enable)
+{
+	return enable ? macronix_nor_octal_dtr_en(nor) :
+			macronix_nor_octal_dtr_dis(nor);
+}
+
 static void macronix_nor_default_init(struct spi_nor *nor)
 {
 	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
@@ -114,6 +210,7 @@ static int macronix_nor_late_init(struct spi_nor *nor)
 {
 	if (!nor->params->set_4byte_addr_mode)
 		nor->params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode_en4b_ex4b;
+	nor->params->set_octal_dtr = macronix_nor_set_octal_dtr;
 
 	return 0;
 }
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 3/6] mtd: spi-nor: add support for Macronix Octal flash

[Jaime Liao]

From: JaimeLiao <jaimeliao@mxic.com.tw>

Adding Macronix Octal flash for Octal DTR support.

The octaflash series can be divided into the following types:

MX25 series : Serial NOR Flash.
MX66 series : Serial NOR Flash with stacked die.(Size larger than 1Gb)
LM/UM series : Up to 250MHz clock frequency with both DTR/STR operation.
LW/UW series : Support simultaneous Read-while-Write operation in multiple
               bank architecture. Read-while-write feature which means read
               data one bank while another bank is programing or erasing.

MX25LM : 3.0V Octal I/O
 -https://www.mxic.com.tw/Lists/Datasheet/Attachments/8729/MX25LM51245G,%203V,%20512Mb,%20v1.1.pdf

MX25UM : 1.8V Octal I/O
 -https://www.mxic.com.tw/Lists/Datasheet/Attachments/8967/MX25UM51245G,%201.8V,%20512Mb,%20v1.5.pdf

MX66LM : 3.0V Octal I/O with stacked die
 -https://www.mxic.com.tw/Lists/Datasheet/Attachments/8748/MX66LM1G45G,%203V,%201Gb,%20v1.1.pdf

MX66UM : 1.8V Octal I/O with stacked die
 -https://www.mxic.com.tw/Lists/Datasheet/Attachments/8711/MX66UM1G45G,%201.8V,%201Gb,%20v1.1.pdf

MX25LW : 3.0V Octal I/O with Read-while-Write
MX25UW : 1.8V Octal I/O with Read-while-Write
MX66LW : 3.0V Octal I/O with Read-while-Write and stack die
MX66UW : 1.8V Octal I/O with Read-while-Write and stack die

About LW/UW series, please contact us freely if you have any
questions. For adding Octal NOR Flash IDs, we have validated
each Flash on plateform zynq-picozed.

As below are the SFDP table dump.

zynq> cat jedec_id
c2943c
zynq> cat manufacturer
macronix
zynq> cat partname
mx66uw2g345gx0
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff7f00ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff87790100841200e2cc04674630b030b0f4bdd55c
000000ff101000200000000000007c234800000000008888000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
000a00001445988043061f0021dcffff
zynq> md5sum sfdp
839ad44d1e758bea30bd9917ba763ba6  sfdp

zynq> cat jedec_id
c2853b
zynq> cat manufacturer
macronix
zynq> cat partname
mx66lm1g45g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff3f00ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff87690100821200e2cc02673830b030b0f4bdd55c
000000ff101000200000000000007ca34800000000006666000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
0000000014351c0043060f0021dcffff
zynq> md5sum sfdp
7b46113b529d58a6335531a10f14a76e  sfdp

zynq> cat jedec_id
c2853a
zynq> cat manufacturer
macronix
zynq> cat partname
mx25lm51245g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff1f00ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff897901008d1200e2cc02674430b030b0f4bdd55c
000000ff101000200000000000007ca34800000000006666000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
0000000014351c0043060f0021dcffff
zynq> md5sum sfdp
214868617d74e6bfb2c45444d5d6fff0  sfdp

zynq> cat jedec_id
c2863a
zynq> cat manufacturer
macronix
zynq> cat partname
mx25lw51245g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff000000000000000000000000
00000000e5208affffffff1f00ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff8b7901008f1200e2cc04674630b030b0f4bdd55c
000000ff101000200000000000007ca34800000000006666000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
0000000014351c0043060f0021dcffff
zynq> md5sum sfdp
bb32ccaca6814f3104b985ac91bd65ac  sfdp

zynq> cat jedec_id
c28539
zynq> cat manufacturer
macronix
zynq> cat partname
mx25lm25645g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff0f00ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff87690100821200d2cc02673830b030b0f4bdd55c
000000ff101000200000000000007ca34800000000006666000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
0000000014351c0043060f0021dcffff
zynq> md5sum sfdp
ec258f831ac737454c7eb9f6a8a4495a  sfdp

zynq> cat jedec_id
c2843c
zynq> cat manufacturer
macronix
zynq> cat partname
mx66uw2g345g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff7f00ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff87790100841200e2cc04674630b030b0f4bdd55c
000000ff101000200000147c00007c234800000000007777000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
000000001445988043061f0021dcffff
zynq> md5sum sfdp
00447475e039e67c256a8d75d5885ae8  sfdp

zynq> cat jedec_id
c2803b
zynq> cat manufacturer
macronix
zynq> cat partname
mx66um1g45g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff3f00ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff897901008d1200e2cc02674430b030b0f4bdd55c
000000ff101000200000000000007ca34800000000008888000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
000a000014359c8043060f0021dcffff
zynq> md5sum sfdp
eea09d64679e64f627402b39a177e356  sfdp

zynq> cat jedec_id
c2813b
zynq> cat manufacturer
macronix
zynq> cat partname
mx66uw1g45g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff3f00ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff8b7901008f1200e2cc04674630b030b0f4bdd55c
000000ff101000200000000000007ca34800000000008888000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
000a00001445988043060f0021dcffff
zynq> md5sum sfdp
b89a53266007fce06ba7cc4c0956f917  sfdp

zynq> cat jedec_id
c2843a
zynq> cat manufacturer
macronix
zynq> cat partname
mx25uw51345g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff1f00ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff8b7901008f1200e2cc04674630b030b0f4bdd55c
000000ff101000200000000000007c234800000000008888000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
000a00001445988043060f0021dcffff
zynq> md5sum sfdp
cdccbfad3c384e77f3a5f7847b57b148  sfdp

zynq> cat jedec_id
c28039
zynq> cat manufacturer
macronix
zynq> cat partname
mx25um25645g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff0f00ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff87790100841200d2cc02673830b030b0f4bdd55c
000000ff101000200000000000007ca34800000000008888000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
000a000014359c8043060f0021dcffff
zynq> md5sum sfdp
d652779f17770dc833cd96262cb2a620  sfdp

zynq> cat jedec_id
c28139
zynq> cat manufacturer
macronix
zynq> cat partname
mx25uw25645g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff0f00ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff897901008d1200d2cc04674630b030b0f4bdd55c
000000ff101000200000000000007ca34800000000008888000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
000a00001445988043060f0021dcffff
zynq> md5sum sfdp
e43ab2dbcbcf99cebc74964c5dcf3ee2  sfdp

zynq> cat jedec_id
c28339
zynq> cat manufacturer
macronix
zynq> cat partname
mx25um25345g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff0f00ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff87690100821200d2cc02673830b030b0f4bdd55c
000000ff101000200000000000007c234800000000008888000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
040900001445988043060f0021dcffff
zynq> md5sum sfdp
950e623745a002e1747008592e6dbdf9  sfdp

zynq> cat jedec_id
c28439
zynq> cat manufacturer
macronix
zynq> cat partname
mx25uw25345g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff0f00ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff87790100841200d2cc04674630b030b0f4bdd55c
000000ff101000200000000000007c234800000000008888000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
000a00001445988043060f0021dcffff
zynq> md5sum sfdp
f7712440f8ce0adb538dfa0c10579c79  sfdp

zynq> cat jedec_id
c28138
zynq> cat manufacturer
macronix
zynq> cat partname
mx25uw12845g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff000000000000000000000000
00000000e5208affffffff0700ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff8b7901008f1200c9cc04674630b030b0f4bdd55c
000000ff101000200000000000007ca34800000000008888000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
000a00001445988043060f0021dcffff
zynq> md5sum sfdp
9eacff90d7aa7cf737b970e0f2a7f2c6  sfdp

zynq> cat jedec_id
c28438
zynq> cat manufacturer
macronix
zynq> cat partname
mx25uw12345g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff0700ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff8b7901008f1200c9cc04674630b030b0f4bdd55c
000000ff101000200000000000007c234800000000008888000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
000a00001445988043060f0021dcffff
zynq> md5sum sfdp
a3eb609c08894c84270ad06efc03766c  sfdp

zynq> cat jedec_id
c28137
zynq> cat manufacturer
macronix
zynq> cat partname
mx25uw6445g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff0300ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff897901008d1200c4cc04674630b030b0f4bdd55c
000000ff101000200000000000007ca34800000000008888000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
000a00001445988043060f0021dcffff
zynq> md5sum sfdp
b09aeedb0cfd0f77adc7e08592d295a9  sfdp

zynq> cat jedec_id
c28437
zynq> cat manufacturer
macronix
zynq> cat partname
mx25uw6345g
zynq> xxd -p sfdp
53464450080104fd00070114400000ff8701011c900000ff0a0001080001
00ff05000105200100ff84000102340100ff0000000000000000ffffffff
ffffffffe5208affffffff0300ff00ff00ff00ffeeffffffffff00ffffff
00ff0c2010d800ff00ff8b7901008f1200c4cc04674630b030b0f4bdd55c
000000ff101000200000000000007c234800000000008888000000000000
00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
727103b80000000090a3188200c069960000000000000000727100987271
00b8727100990000000072710098727100f872710099727100f900000000
00000000011501d0727106d8000086500000060100000000020001030002
00000000060100000000000072060002000000eec0697272717100d8f7f6
000a00001445988043060f0021dcffff
zynq> md5sum sfdp
c6fb57b8fdd4c35b5f0dacc4a1f7d4f4  sfdp

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

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 28c49c98503a..1ccea31ae64a 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -121,6 +121,51 @@ static const struct flash_info macronix_nor_parts[] = {
 	{ "mx66u2g45g",	 INFO(0xc2253c, 0, 64 * 1024, 4096)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
 		FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
+	{ "mx66uw2g345gx0", INFOB(0xc2943c, 0, 0, 0, 4)
+		PARSE_SFDP
+		FLAGS(SPI_NOR_RWW) },
+	{ "mx66lm1g45g", INFOB(0xc2853b, 0, 0, 0, 4)
+		PARSE_SFDP },
+	{ "mx25lm51245g", INFOB(0xc2853a, 0, 0, 0, 4)
+		PARSE_SFDP },
+	{ "mx25lw51245g", INFOB(0xc2863a, 0, 0, 0, 4)
+		PARSE_SFDP
+		FLAGS(SPI_NOR_RWW) },
+	{ "mx25lm25645g", INFOB(0xc28539, 0, 0, 0, 4)
+		PARSE_SFDP },
+	{ "mx66uw2g345g", INFOB(0xc2843c, 0, 0, 0, 4)
+		PARSE_SFDP
+		FLAGS(SPI_NOR_RWW) },
+	{ "mx66um1g45g", INFOB(0xc2803b, 0, 0, 0, 4)
+		PARSE_SFDP },
+	{ "mx66uw1g45g", INFOB(0xc2813b, 0, 0, 0, 4)
+		PARSE_SFDP
+		FLAGS(SPI_NOR_RWW) },
+	{ "mx25uw51345g", INFOB(0xc2843a, 0, 0, 0, 4)
+		PARSE_SFDP
+		FLAGS(SPI_NOR_RWW) },
+	{ "mx25um25645g", INFO(0xc28039, 0, 0, 0)
+		PARSE_SFDP },
+	{ "mx25uw25645g", INFO(0xc28139, 0, 0, 0)
+		PARSE_SFDP
+		FLAGS(SPI_NOR_RWW) },
+	{ "mx25um25345g", INFO(0xc28339, 0, 0, 0)
+		PARSE_SFDP },
+	{ "mx25uw25345g", INFOB(0xc28439, 0, 0, 0, 4)
+		PARSE_SFDP
+		FLAGS(SPI_NOR_RWW) },
+	{ "mx25uw12845g", INFOB(0xc28138, 0, 0, 0, 4)
+		PARSE_SFDP
+		FLAGS(SPI_NOR_RWW) },
+	{ "mx25uw12345g", INFOB(0xc28438, 0, 0, 0, 4)
+		PARSE_SFDP
+		FLAGS(SPI_NOR_RWW) },
+	{ "mx25uw6445g", INFOB(0xc28137, 0, 0, 0, 4)
+		PARSE_SFDP
+		FLAGS(SPI_NOR_RWW) },
+	{ "mx25uw6345g", INFOB(0xc28437, 0, 0, 0, 4)
+		PARSE_SFDP
+		FLAGS(SPI_NOR_RWW) },
 };
 
 static int macronix_nor_octal_dtr_en(struct spi_nor *nor)
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 4/6] spi: spi-mem: Allow specifying the byte order in DTR mode

[Jaime Liao]

From: JaimeLiao <jaimeliao@mxic.com.tw>

There are NOR flashes (Macronix) that swap the bytes on a 16-bit
boundary when configured in Octal DTR mode. The byte order of
16-bit words is swapped when read or written in Octal Double
Transfer Rate (DTR) mode compared to Single Transfer Rate (STR)
modes. If one writes D0 D1 D2 D3 bytes using 1-1-1 mode, and uses
8D-8D-8D SPI mode for reading, it will read back D1 D0 D3 D2.
Swapping the bytes may introduce some endianness problems. It can
affect the boot sequence if the entire boot sequence is not handled
in either 8D-8D-8D mode or 1-1-1 mode. So we must swap the bytes
back to have the same byte order as in STR modes. Fortunately there
are controllers that could swap the bytes back at runtime,
addressing the flash's endiannesses requirements. Provide a way for
the upper layers to specify the byte order in Octal DTR mode.

Merge Tudor's patch and add modifications for suiting newer version
of Linux kernel.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
---
 drivers/spi/spi-mem.c       | 4 ++++
 include/linux/spi/spi-mem.h | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index edd7430d4c05..9c03b5617fff 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -172,6 +172,10 @@ bool spi_mem_default_supports_op(struct spi_mem *mem,
 		if (!spi_mem_controller_is_capable(ctlr, dtr))
 			return false;
 
+		if (op->data.dtr_swab16 &&
+		    !(spi_mem_controller_is_capable(ctlr, dtr_swab16)))
+			return false;
+
 		if (op->cmd.nbytes != 2)
 			return false;
 	} else {
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index 6b0a7dc48a4b..d4935c5c3c7a 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -89,6 +89,8 @@ enum spi_mem_data_dir {
  * @dummy.dtr: whether the dummy bytes should be sent in DTR mode or not
  * @data.buswidth: number of IO lanes used to send/receive the data
  * @data.dtr: whether the data should be sent in DTR mode or not
+ * @data.dtr_swab16: whether the byte order of 16-bit words is swapped when read
+ *		     or written in Octal DTR mode compared to STR mode.
  * @data.ecc: whether error correction is required or not
  * @data.dir: direction of the transfer
  * @data.nbytes: number of data bytes to send/receive. Can be zero if the
@@ -123,6 +125,7 @@ struct spi_mem_op {
 	struct {
 		u8 buswidth;
 		u8 dtr : 1;
+		u8 dtr_swab16 : 1;
 		u8 ecc : 1;
 		u8 __pad : 6;
 		enum spi_mem_data_dir dir;
@@ -294,10 +297,13 @@ struct spi_controller_mem_ops {
 /**
  * struct spi_controller_mem_caps - SPI memory controller capabilities
  * @dtr: Supports DTR operations
+ * @dtr_swab16: Supports swapping bytes on a 16 bit boundary when configured in
+ *		Octal DTR
  * @ecc: Supports operations with error correction
  */
 struct spi_controller_mem_caps {
 	bool dtr;
+	bool dtr_swab16;
 	bool ecc;
 };
 
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 5/6] mtd: spi-nor: core: Allow specifying the byte order in DTR mode

[Jaime Liao]

From: JaimeLiao <jaimeliao@mxic.com.tw>

Macronix swaps bytes on a 16-bit boundary when configured in Octal DTR.
The byte order of 16-bit words is swapped when read or written in 8D-8D-8D
mode compared to STR modes. Allow operations to specify the byte order in
DTR mode, so that controllers can swap the bytes back at run-time to
address the flash's endianness requirements, if they are capable. If the
controllers are not capable of swapping the bytes, the protocol is
downgrade via spi_nor_spimem_adjust_hwcaps(). When available, the swapping
of the bytes is always done regardless if it's a data or register access,
so that we comply with the JESD216 requirements: "Byte order of 16-bit
words is swapped when read in 8D-8D-8D mode compared to 1-1-1".

Merge Tudor's patch and add modifications for suiting newer version
of Linux kernel.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
---
 drivers/mtd/spi-nor/core.c | 8 ++++++++
 drivers/mtd/spi-nor/core.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 7ee624b16e17..83e7b7ff89ee 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -70,6 +70,13 @@ static u8 spi_nor_get_cmd_ext(const struct spi_nor *nor,
 	}
 }
 
+static inline bool spi_nor_is_octal_dtr_swab16(const struct spi_nor *nor,
+						enum spi_nor_protocol proto)
+{
+	return (proto == SNOR_PROTO_8_8_8_DTR) &&
+		(nor->flags & SNOR_F_DTR_SWAB16);
+}
+
 /**
  * spi_nor_spimem_setup_op() - Set up common properties of a spi-mem op.
  * @nor:		pointer to a 'struct spi_nor'
@@ -105,6 +112,7 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
 		op->addr.dtr = true;
 		op->dummy.dtr = true;
 		op->data.dtr = true;
+		op->data.dtr_swab16 = spi_nor_is_octal_dtr_swab16(nor, proto);
 
 		/* 2 bytes per clock cycle in DTR mode. */
 		op->dummy.nbytes *= 2;
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 92cbc2d3f7fe..7ff4c96fe21e 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -133,6 +133,7 @@ enum spi_nor_option_flags {
 	SNOR_F_RWW		= BIT(14),
 	SNOR_F_ECC		= BIT(15),
 	SNOR_F_NO_WP		= BIT(16),
+	SNOR_F_DTR_SWAB16       = BIT(17),
 };
 
 struct spi_nor_read_command {
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 6/6] mtd: spi-nor: sfdp: Get the 8D-8D-8D byte order from BFPT

[Jaime Liao]

From: JaimeLiao <jaimeliao@mxic.com.tw>

Parse BFPT in order to retrieve the byte order in 8D-8D-8D mode.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
---
 drivers/mtd/spi-nor/sfdp.c | 4 ++++
 drivers/mtd/spi-nor/sfdp.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index b3b11dfed789..2241207556bf 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -650,6 +650,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
 		return -EOPNOTSUPP;
 	}
 
+	/* Byte order in 8D-8D-8D mode */
+	if (bfpt.dwords[SFDP_DWORD(18)] & BFPT_DWORD18_BYTE_ORDER_SWAPPED)
+		nor->flags |= SNOR_F_DTR_SWAB16;
+
 	return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt);
 }
 
diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
index 6eb99e1cdd61..eba760941d43 100644
--- a/drivers/mtd/spi-nor/sfdp.h
+++ b/drivers/mtd/spi-nor/sfdp.h
@@ -123,6 +123,7 @@ struct sfdp_bfpt {
 #define BFPT_DWORD18_CMD_EXT_INV		(0x1UL << 29) /* Invert */
 #define BFPT_DWORD18_CMD_EXT_RES		(0x2UL << 29) /* Reserved */
 #define BFPT_DWORD18_CMD_EXT_16B		(0x3UL << 29) /* 16-bit opcode */
+#define BFPT_DWORD18_BYTE_ORDER_SWAPPED		BIT(31) /* Byte order of 16-bit words in 8D-8D-8D mode */
 
 struct sfdp_parameter_header {
 	u8		id_lsb;
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[Tudor Ambarus]

Hi, Jaime,

Thanks for the patch! I think we are getting closer to have this
integrated, but I have some concerns that hopefully with your help we'll
address and move forward.

On 08.09.2023 09:42, Jaime Liao wrote:
> From: JaimeLiao <jaimeliao@mxic.com.tw>
> 
> Add manufacturer read id function because of some flash
> may change data format when read id in octal dtr mode.

I'm not convinced such a method is really needed, would you please
elaborate the explanation why it's needed?

I'm looking at the mx66lm1g45g datasheet. From what I see in "Figure 13.
Read Identification (RDID) Sequence (DTR-OPI Mode)" looks like even if
the flash is operated in 8d-8d-8d mode, what the flash actually uses is
a 8d-8d-8s mode for the read id. Each ID byte is sent twice on both
rising and falling edge of the clock, thus behaving like a 8d-8d-8s
protocol.

I see this flash supports 1-1-1, 8-8-8, and 8d-8d-8d, there are no mixed
modes supported, thus a 8d-8d-8s mode seems just like a hardware bug to
me. So my proposal is to leave the core away and to handle the read id
hack just in the macronix driver.

> 
> Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/core.c     | 30 ++++++++++++++++++++++++++++--
>  drivers/mtd/spi-nor/core.h     |  6 ++++++
>  drivers/mtd/spi-nor/macronix.c | 18 ++++++++++++++++++
>  3 files changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 1b0c6770c14e..7ee624b16e17 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -408,7 +408,7 @@ int spi_nor_write_disable(struct spi_nor *nor)
>  }
>  
>  /**
> - * spi_nor_read_id() - Read the JEDEC ID.
> + * spi_nor_default_read_id() - Read the JEDEC ID.
>   * @nor:	pointer to 'struct spi_nor'.
>   * @naddr:	number of address bytes to send. Can be zero if the operation
>   *		does not need to send an address.
> @@ -420,7 +420,7 @@ int spi_nor_write_disable(struct spi_nor *nor)
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> -int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
> +int spi_nor_default_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
>  		    enum spi_nor_protocol proto)
>  {
>  	int ret;
> @@ -438,6 +438,32 @@ int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
>  	return ret;
>  }
>  
> +/**
> + * spi_nor_read_id() - Read ID by manufacturer read id function.
> + * @nor:	pointer to 'struct spi_nor'.
> + * @naddr:	number of address bytes to send. Can be zero if the operation
> + *		does not need to send an address.
> + * @ndummy:	number of dummy bytes to send after an opcode or address. Can
> + *		be zero if the operation does not require dummy bytes.
> + * @id:		pointer to a DMA-able buffer where the value of the JEDEC ID
> + *		will be written.
> + * @proto:	the SPI protocol for register operation.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
> +		    enum spi_nor_protocol proto)
> +{
> +	int ret;
> +
> +	if (nor->manufacturer && nor->manufacturer->fixups && nor->manufacturer->fixups->read_id)
> +		ret = nor->manufacturer->fixups->read_id(nor, naddr, ndummy, id, proto);
> +	else
> +		ret = spi_nor_default_read_id(nor, naddr, ndummy, id, proto);
> +
> +	return ret;
> +}
> +
>  /**
>   * spi_nor_read_sr() - Read the Status Register.
>   * @nor:	pointer to 'struct spi_nor'.
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 9217379b9cfe..92cbc2d3f7fe 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -424,6 +424,8 @@ struct spi_nor_flash_parameter {
>   * @late_init: used to initialize flash parameters that are not declared in the
>   *             JESD216 SFDP standard, or where SFDP tables not defined at all.
>   *             Will replace the default_init() hook.
> + * @read_id:   used to read id which may change format after enter into
> +	       octal dtr mode.
>   *
>   * Those hooks can be used to tweak the SPI NOR configuration when the SFDP
>   * table is broken or not available.
> @@ -435,6 +437,8 @@ struct spi_nor_fixups {
>  			 const struct sfdp_bfpt *bfpt);
>  	int (*post_sfdp)(struct spi_nor *nor);
>  	int (*late_init)(struct spi_nor *nor);
> +	int (*read_id)(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
> +		       enum spi_nor_protocol reg_proto);
>  };
>  
>  /**
> @@ -667,6 +671,8 @@ void spi_nor_unlock_and_unprep(struct spi_nor *nor);
>  int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
>  int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor);
>  int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor);
> +int spi_nor_default_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
> +			    enum spi_nor_protocol reg_proto);
>  int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
>  		    enum spi_nor_protocol reg_proto);
>  int spi_nor_read_sr(struct spi_nor *nor, u8 *sr);
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index eb149e517c1f..8ab47691dfbb 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -118,9 +118,27 @@ static int macronix_nor_late_init(struct spi_nor *nor)
>  	return 0;
>  }
>  
> +static int macronix_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
> +				enum spi_nor_protocol proto)
> +{
> +	int i, ret;
> +
> +	ret = spi_nor_default_read_id(nor, naddr, ndummy, id, proto);
> +	/* Retrieve odd array and re-sort id because of read id format will be A-A-B-B-C-C
> +	 * after enter into octal dtr mode for Macronix flashes.
> +	 */
> +	if (proto == SNOR_PROTO_8_8_8_DTR) {
> +		for (i = 0; i < nor->info->id_len; i++)
> +			id[i] = id[i*2];

why do you overwrite the id? How about just checking that
id[i] == id[i + 1]? why do you care if you print an a-a-b-b-c-c id?
> +	}
> +
> +	return ret;
> +}
> +
>  static const struct spi_nor_fixups macronix_nor_fixups = {
>  	.default_init = macronix_nor_default_init,
>  	.late_init = macronix_nor_late_init,
> +	.read_id = macronix_nor_read_id,
>  };
>  
>  const struct spi_nor_manufacturer spi_nor_macronix = {

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 2/6] mtd: spi-nor: add Octal DTR support for Macronix flash

[Tudor Ambarus]

Hi, Jaime,

On 08.09.2023 09:43, Jaime Liao wrote:
> From: JaimeLiao <jaimeliao@mxic.com.tw>
> 
> Create Macronix specify method for enable Octal DTR mode and
> set 20 dummy cycles to allow running at the maximum supported
> frequency for Macronix Octal flash.

You didn't answer my question. What happens when you specify 20 dummy
cycles, thus you configure the dummy cycles for the maximum flash speed,
but you program the flash to work on 1 MHz for example. Do you still
have reliable results?
> 
> Use number of dummy cycles which is parse by SFDP then convert
> it to bit pattern and set in CR2 register.

What we should do instead is to determine the flash speed at which the
flash is operated and then to set the correct number of dummy cycles
according to what the flash requires. There should be a table somewhere
in the datasheet that specify a required number of dummy cycles for a
particular frequency, isn't it? Yeah, see "9-3-1. Dummy Cycle and
Frequency Table (MHz)" of the mx66lm1g45g datasheet.

Cheers,
ta

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 3/6] mtd: spi-nor: add support for Macronix Octal flash

[Tudor Ambarus]

Hi, Jaime,

I think I already asked to split this in a patch per flash family,
it will become more readable this way.

On 08.09.2023 09:43, Jaime Liao wrote:
> From: JaimeLiao <jaimeliao@mxic.com.tw>
> 
> Adding Macronix Octal flash for Octal DTR support.
> 
> The octaflash series can be divided into the following types:
> 
> MX25 series : Serial NOR Flash.
> MX66 series : Serial NOR Flash with stacked die.(Size larger than 1Gb)
> LM/UM series : Up to 250MHz clock frequency with both DTR/STR operation.
> LW/UW series : Support simultaneous Read-while-Write operation in multiple
>                bank architecture. Read-while-write feature which means read
>                data one bank while another bank is programing or erasing.
> 
> MX25LM : 3.0V Octal I/O
>  -https://www.mxic.com.tw/Lists/Datasheet/Attachments/8729/MX25LM51245G,%203V,%20512Mb,%20v1.1.pdf
> 
> MX25UM : 1.8V Octal I/O
>  -https://www.mxic.com.tw/Lists/Datasheet/Attachments/8967/MX25UM51245G,%201.8V,%20512Mb,%20v1.5.pdf
> 
> MX66LM : 3.0V Octal I/O with stacked die
>  -https://www.mxic.com.tw/Lists/Datasheet/Attachments/8748/MX66LM1G45G,%203V,%201Gb,%20v1.1.pdf
> 
> MX66UM : 1.8V Octal I/O with stacked die
>  -https://www.mxic.com.tw/Lists/Datasheet/Attachments/8711/MX66UM1G45G,%201.8V,%201Gb,%20v1.1.pdf
> 
> MX25LW : 3.0V Octal I/O with Read-while-Write
> MX25UW : 1.8V Octal I/O with Read-while-Write
> MX66LW : 3.0V Octal I/O with Read-while-Write and stack die
> MX66UW : 1.8V Octal I/O with Read-while-Write and stack die
> 
> About LW/UW series, please contact us freely if you have any

this is not the place for such a statement.

> questions. For adding Octal NOR Flash IDs, we have validated
> each Flash on plateform zynq-picozed.
> 
> As below are the SFDP table dump.
> 
> zynq> cat jedec_id
> c2943c
> zynq> cat manufacturer
> macronix
> zynq> cat partname
> mx66uw2g345gx0
> zynq> xxd -p sfdp
> 53464450080104fd00070114400000ff8701011c900000ff0a0001080001
> 00ff05000105200100ff84000102340100ff0000000000000000ffffffff
> ffffffffe5208affffffff7f00ff00ff00ff00ffeeffffffffff00ffffff
> 00ff0c2010d800ff00ff87790100841200e2cc04674630b030b0f4bdd55c
> 000000ff101000200000000000007c234800000000008888000000000000
> 00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
> 727103b80000000090a3188200c069960000000000000000727100987271
> 00b8727100990000000072710098727100f872710099727100f900000000
> 00000000011501d0727106d8000086500000060100000000020001030002
> 00000000060100000000000072060002000000eec0697272717100d8f7f6
> 000a00001445988043061f0021dcffff
> zynq> md5sum sfdp
> 839ad44d1e758bea30bd9917ba763ba6  sfdp

please do the mtd_debug tests as well. I shall update mtd-debug with a
sanity test in the future.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 4/6] spi: spi-mem: Allow specifying the byte order in DTR mode

[Tudor Ambarus]

On 08.09.2023 09:43, Jaime Liao wrote:
> From: JaimeLiao <jaimeliao@mxic.com.tw>
> 
> There are NOR flashes (Macronix) that swap the bytes on a 16-bit
> boundary when configured in Octal DTR mode. The byte order of
> 16-bit words is swapped when read or written in Octal Double
> Transfer Rate (DTR) mode compared to Single Transfer Rate (STR)
> modes. If one writes D0 D1 D2 D3 bytes using 1-1-1 mode, and uses
> 8D-8D-8D SPI mode for reading, it will read back D1 D0 D3 D2.
> Swapping the bytes may introduce some endianness problems. It can
> affect the boot sequence if the entire boot sequence is not handled
> in either 8D-8D-8D mode or 1-1-1 mode. So we must swap the bytes
> back to have the same byte order as in STR modes. Fortunately there
> are controllers that could swap the bytes back at runtime,
> addressing the flash's endiannesses requirements. Provide a way for
> the upper layers to specify the byte order in Octal DTR mode.
> 

Jaime,

would you please remind me how this works? Does these flashes always
swap the bytes? Is there a flash configuration register where you can
specify whether to swap the bytes or not?

If it's not configurable I'm thinking of introducing a hook that allows
flashes to program the spi controller to swap the bytes and if the spi
controller does not support byte swapping, to just return an error and
fallback to 8-8-8/1-1-1 mode. We can't break the bootchain.

Cheers,
ta

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 5/6] mtd: spi-nor: core: Allow specifying the byte order in DTR mode

[Tudor Ambarus]

Hi, Jaime,

On 08.09.2023 09:43, Jaime Liao wrote:
> From: JaimeLiao <jaimeliao@mxic.com.tw>
> 
> Macronix swaps bytes on a 16-bit boundary when configured in Octal DTR.
> The byte order of 16-bit words is swapped when read or written in 8D-8D-8D
> mode compared to STR modes. Allow operations to specify the byte order in
> DTR mode, so that controllers can swap the bytes back at run-time to
> address the flash's endianness requirements, if they are capable. If the
> controllers are not capable of swapping the bytes, the protocol is
> downgrade via spi_nor_spimem_adjust_hwcaps(). When available, the swapping
> of the bytes is always done regardless if it's a data or register access,
> so that we comply with the JESD216 requirements: "Byte order of 16-bit
> words is swapped when read in 8D-8D-8D mode compared to 1-1-1".
> 

There's a bit in SFDP that specifies whether the flash swaps the bytes
or not in 8d-8d-8d mode. See BFPT DWORD[18] bit 31. Can we query the
SFDP instead of introducing a new info flag?

Cheers,
ta

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 6/6] mtd: spi-nor: sfdp: Get the 8D-8D-8D byte order from BFPT

[Tudor Ambarus]

On 08.09.2023 09:43, Jaime Liao wrote:
> From: JaimeLiao <jaimeliao@mxic.com.tw>
> 
> Parse BFPT in order to retrieve the byte order in 8D-8D-8D mode.

Oh, here it is. Why do you need the flash info flag then?

> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/sfdp.c | 4 ++++
>  drivers/mtd/spi-nor/sfdp.h | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index b3b11dfed789..2241207556bf 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -650,6 +650,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  		return -EOPNOTSUPP;
>  	}
>  
> +	/* Byte order in 8D-8D-8D mode */
> +	if (bfpt.dwords[SFDP_DWORD(18)] & BFPT_DWORD18_BYTE_ORDER_SWAPPED)
> +		nor->flags |= SNOR_F_DTR_SWAB16;
> +
>  	return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt);
>  }
>  
> diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
> index 6eb99e1cdd61..eba760941d43 100644
> --- a/drivers/mtd/spi-nor/sfdp.h
> +++ b/drivers/mtd/spi-nor/sfdp.h
> @@ -123,6 +123,7 @@ struct sfdp_bfpt {
>  #define BFPT_DWORD18_CMD_EXT_INV		(0x1UL << 29) /* Invert */
>  #define BFPT_DWORD18_CMD_EXT_RES		(0x2UL << 29) /* Reserved */
>  #define BFPT_DWORD18_CMD_EXT_16B		(0x3UL << 29) /* 16-bit opcode */
> +#define BFPT_DWORD18_BYTE_ORDER_SWAPPED		BIT(31) /* Byte order of 16-bit words in 8D-8D-8D mode */
>  
>  struct sfdp_parameter_header {
>  	u8		id_lsb;

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 2/6] mtd: spi-nor: add Octal DTR support for Macronix flash

[Pratyush Yadav]

On Wed, Sep 20 2023, Tudor Ambarus wrote:

> Hi, Jaime,
>
> On 08.09.2023 09:43, Jaime Liao wrote:
>> From: JaimeLiao <jaimeliao@mxic.com.tw>
>> 
>> Create Macronix specify method for enable Octal DTR mode and
>> set 20 dummy cycles to allow running at the maximum supported
>> frequency for Macronix Octal flash.
>
> You didn't answer my question. What happens when you specify 20 dummy
> cycles, thus you configure the dummy cycles for the maximum flash speed,
> but you program the flash to work on 1 MHz for example. Do you still
> have reliable results?

I don't know about this particular flash, but in the past, I have tried
doing this with Spansion and Micron flashes, and they work fine on lower
frequencies even with the maximum dummy cycles set.

When you think about it, the only reason higher frequencies put a
restriction on minimum dummy cycles is because the flash actually has a
restriction on _time_. As time for each cycle gets shorter with higher
frequencies, you need more cycles to wait the same amount of time. Dummy
cycles are just a way to ensure you wait more than the minimum amount of
time needed to get the flash ready to transmit data.

So I see no reason for a flash to break because it waited _longer_ than
the minimum time.

>> 
>> Use number of dummy cycles which is parse by SFDP then convert
>> it to bit pattern and set in CR2 register.
>
> What we should do instead is to determine the flash speed at which the
> flash is operated and then to set the correct number of dummy cycles
> according to what the flash requires. There should be a table somewhere
> in the datasheet that specify a required number of dummy cycles for a
> particular frequency, isn't it? Yeah, see "9-3-1. Dummy Cycle and
> Frequency Table (MHz)" of the mx66lm1g45g datasheet.

Right, most flashes do give such a table, though I don't remember any
more if SFDP has something like this as well.

I remember thinking the same thing when I was adding support for Octal
DTR in SPI NOR. The problem is that SPI NOR currently has no way of
knowing what exact speed the flash will be operated at.

It can look at nor->spimem->spi->max_speed_hz (I am not sure it should
do this directly) which comes from the "spi-max-frequency" DT property,
but that is only the maximum. This can be a good starting point to
decide the maximum number of cycles you would need.

But that is only the maximum. The controller does not guarantee using
the maximum speed. It can use something slower as well. And currently
there is no way for the controller to report that speed back to SPI MEM
or SPI NOR. So if we want to waste the least amount of dummy cycles, we
would need to teach the controller drivers to report back the exact
speed it is going to driver the flash at.

I am not sure this might be worth the trouble though. We should first
quantify how much throughput/latency we stand to gain from doing this.
But I do think looking at "spi-max-frequency" is fairly simple and
should be a good enough start.

-- 
Regards,
Pratyush Yadav

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[Pratyush Yadav]

On Wed, Sep 20 2023, Tudor Ambarus wrote:

> Hi, Jaime,
>
> Thanks for the patch! I think we are getting closer to have this
> integrated, but I have some concerns that hopefully with your help we'll
> address and move forward.
>
> On 08.09.2023 09:42, Jaime Liao wrote:
>> From: JaimeLiao <jaimeliao@mxic.com.tw>
>> 
>> Add manufacturer read id function because of some flash
>> may change data format when read id in octal dtr mode.
>
> I'm not convinced such a method is really needed, would you please
> elaborate the explanation why it's needed?
>
> I'm looking at the mx66lm1g45g datasheet. From what I see in "Figure 13.
> Read Identification (RDID) Sequence (DTR-OPI Mode)" looks like even if
> the flash is operated in 8d-8d-8d mode, what the flash actually uses is
> a 8d-8d-8s mode for the read id. Each ID byte is sent twice on both
> rising and falling edge of the clock, thus behaving like a 8d-8d-8s
> protocol.
>
> I see this flash supports 1-1-1, 8-8-8, and 8d-8d-8d, there are no mixed
> modes supported, thus a 8d-8d-8s mode seems just like a hardware bug to
> me. So my proposal is to leave the core away and to handle the read id
> hack just in the macronix driver.

+1

[...]
>> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
>> index eb149e517c1f..8ab47691dfbb 100644
>> --- a/drivers/mtd/spi-nor/macronix.c
>> +++ b/drivers/mtd/spi-nor/macronix.c
>> @@ -118,9 +118,27 @@ static int macronix_nor_late_init(struct spi_nor *nor)
>>  	return 0;
>>  }
>>  
>> +static int macronix_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
>> +				enum spi_nor_protocol proto)
>> +{
>> +	int i, ret;
>> +
>> +	ret = spi_nor_default_read_id(nor, naddr, ndummy, id, proto);
>> +	/* Retrieve odd array and re-sort id because of read id format will be A-A-B-B-C-C
>> +	 * after enter into octal dtr mode for Macronix flashes.
>> +	 */
>> +	if (proto == SNOR_PROTO_8_8_8_DTR) {
>> +		for (i = 0; i < nor->info->id_len; i++)
>> +			id[i] = id[i*2];
>
> why do you overwrite the id? How about just checking that
> id[i] == id[i + 1]? why do you care if you print an a-a-b-b-c-c id?

Because id_len == 3 so you should only treat the ID as a-a-b. When you
memcmp() the ID after reading it, you would not get a match.

>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>  static const struct spi_nor_fixups macronix_nor_fixups = {
>>  	.default_init = macronix_nor_default_init,
>>  	.late_init = macronix_nor_late_init,
>> +	.read_id = macronix_nor_read_id,
>>  };
>>  
>>  const struct spi_nor_manufacturer spi_nor_macronix = {

-- 
Regards,
Pratyush Yadav

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[Michael Walle]

Hi,

>> I see this flash supports 1-1-1, 8-8-8, and 8d-8d-8d, there are no 
>> mixed
>> modes supported, thus a 8d-8d-8s mode seems just like a hardware bug 
>> to
>> me. So my proposal is to leave the core away and to handle the read id
>> hack just in the macronix driver.
> 
> +1

I've looked at the xspi spec. There is no RDID specified. So I'd argue,
the only pseudo standard is that RDID was only ever used with 1s1s1s. 
But
we added spi_nor_read_id() with parameters suited for the "unusual" 
8d8d8d
case with additional address and dummy cycles. Just for checking whether 
the
octal-dtr switch was successful. Therefore, we've already added 
parameters to
spi_nor_read_id() which are not standard. Then we can just add one more. 
It's
just how macronix is doing it. Again there is no standard.
If we'd only put standard (or for the 9F pseudo standard) things in the 
core,
then spi_nor_read_id() would need to check whether the flash is in 
1s1s1s
mode. And no I wouldn't prefer that ;)

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[Tudor Ambarus]

On 10/5/23 14:27, Michael Walle wrote:
> Am 2023-10-05 15:19, schrieb Tudor Ambarus:
>> On 10/5/23 12:43, Michael Walle wrote:
>>> Hi,
>>>
>> hey
>>
>>>>> I see this flash supports 1-1-1, 8-8-8, and 8d-8d-8d, there are no
>>>>> mixed
>>>>> modes supported, thus a 8d-8d-8s mode seems just like a hardware
>>>>> bug to
>>>>> me. So my proposal is to leave the core away and to handle the read id
>>>>> hack just in the macronix driver.
>>>>
>>>> +1
>>>
>>> I've looked at the xspi spec. There is no RDID specified. So I'd argue,
>>> the only pseudo standard is that RDID was only ever used with 1s1s1s.
>>> But
>>> we added spi_nor_read_id() with parameters suited for the "unusual"
>>> 8d8d8d
>>> case with additional address and dummy cycles. Just for checking whether
>>> the
>>> octal-dtr switch was successful. Therefore, we've already added
>>> parameters to
>>> spi_nor_read_id() which are not standard. Then we can just add one more.
>>> It's
>>> just how macronix is doing it. Again there is no standard.
>>> If we'd only put standard (or for the 9F pseudo standard) things in the
>>> core,
>>> then spi_nor_read_id() would need to check whether the flash is in
>>> 1s1s1s
>>> mode. And no I wouldn't prefer that ;)
>>>
>>
>> If we really want to be catholic, we should switch to 8d-8d-8s mode and
>> then issue the read id and the core will handle the readid correctly.
>> But there is no such a thing, because macronix considers that it is in
>> 8d-8d-8d mode at the time of issuing 8d-8d-8s read id. That's why I say
>> it's a bug on their side. The core is meant to be generic, I don't want
>> to pollute the core with manufacturer specific fixes.
> 
> Then why does spi_nor_read_id() have the following parameters:
> 
>  * @naddr:      number of address bytes to send. Can be zero if the
> operation
>  *              does not need to send an address.
>  * @ndummy:     number of dummy bytes to send after an opcode or
> address. Can
>  *              be zero if the operation does not require dummy bytes.
>  * @proto:      the SPI protocol for register operation.
> 
> They aren't standard either. It's just the way spansion and micron
> transfers
> the ID via RDID in octal DTR mode. And now there's macronix who does it
> slightly
> different. But *neither* of them are standard. Why should one be in the
> core and
> one shouldn't.
> 
> spi_nor_read_id() should just handle proto == SNOR_PROTO_8D_8D_8S.

Yes, it should, but mx is in 8d-8d-8d at the time it issues the read id,
thus it passes the core proto == SNOR_PROTO_8D_8D_8D, isn't it? If you
care about this, please send a patch addressing it, it's better to talk
on code. I don't see yet how you will handle it, but I'm open to review
some code.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[Michael Walle]

Hi,

>>>>>> I see this flash supports 1-1-1, 8-8-8, and 8d-8d-8d, there are no
>>>>>> mixed
>>>>>> modes supported, thus a 8d-8d-8s mode seems just like a hardware
>>>>>> bug to
>>>>>> me. So my proposal is to leave the core away and to handle the 
>>>>>> read id
>>>>>> hack just in the macronix driver.
>>>>> 
>>>>> +1
>>>> 
>>>> I've looked at the xspi spec. There is no RDID specified. So I'd 
>>>> argue,
>>>> the only pseudo standard is that RDID was only ever used with 
>>>> 1s1s1s.
>>>> But
>>>> we added spi_nor_read_id() with parameters suited for the "unusual"
>>>> 8d8d8d
>>>> case with additional address and dummy cycles. Just for checking 
>>>> whether
>>>> the
>>>> octal-dtr switch was successful. Therefore, we've already added
>>>> parameters to
>>>> spi_nor_read_id() which are not standard. Then we can just add one 
>>>> more.
>>>> It's
>>>> just how macronix is doing it. Again there is no standard.
>>>> If we'd only put standard (or for the 9F pseudo standard) things in 
>>>> the
>>>> core,
>>>> then spi_nor_read_id() would need to check whether the flash is in
>>>> 1s1s1s
>>>> mode. And no I wouldn't prefer that ;)
>>>> 
>>> 
>>> If we really want to be catholic, we should switch to 8d-8d-8s mode 
>>> and
>>> then issue the read id and the core will handle the readid correctly.
>>> But there is no such a thing, because macronix considers that it is 
>>> in
>>> 8d-8d-8d mode at the time of issuing 8d-8d-8s read id. That's why I 
>>> say
>>> it's a bug on their side. The core is meant to be generic, I don't 
>>> want
>>> to pollute the core with manufacturer specific fixes.
>> 
>> Then why does spi_nor_read_id() have the following parameters:
>> 
>>  * @naddr:      number of address bytes to send. Can be zero if the
>> operation
>>  *              does not need to send an address.
>>  * @ndummy:     number of dummy bytes to send after an opcode or
>> address. Can
>>  *              be zero if the operation does not require dummy bytes.
>>  * @proto:      the SPI protocol for register operation.
>> 
>> They aren't standard either. It's just the way spansion and micron
>> transfers
>> the ID via RDID in octal DTR mode. And now there's macronix who does 
>> it
>> slightly
>> different. But *neither* of them are standard. Why should one be in 
>> the
>> core and
>> one shouldn't.
>> 
>> spi_nor_read_id() should just handle proto == SNOR_PROTO_8D_8D_8S.
> 
> Yes, it should, but mx is in 8d-8d-8d at the time it issues the read 
> id,
> thus it passes the core proto == SNOR_PROTO_8D_8D_8D, isn't it?

The flash device is in octal dtr mode, which means it will expect the
opcode in octal dtr mode. But the data phase mode depends on the opcode
(and theoretically, the address phase, too). If you use the
(non-standard) RDID on this flash command has to be executed as 8d8d8s.

> If you
> care about this, please send a patch addressing it, it's better to talk
> on code. I don't see yet how you will handle it, but I'm open to review
> some code.

I really don't have time right now. But something along:

spi_nor_read_id(.., proto) {
   bool emulated_8d8d8s;

   op = assemble_op(.., proto);

   /* Emulate 8d8d8s if the controller doesn't support it */
   if (!spi_mem_supports_op(op) && proto == 8d8d8s) {
      op = assemble_op(.., 8d8d8d);
      emulated_8d8d8s = true;
   }
   execute_op()
   if (emulated_8d8d8s) {
      /* discard every other byte of the response */
   }
}

Later (or even now), that could be moved down the callstack into
the spimem core.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[liao jaime]

Hi,

>
> Hi,
>
> >>>>>> I see this flash supports 1-1-1, 8-8-8, and 8d-8d-8d, there are no
> >>>>>> mixed
> >>>>>> modes supported, thus a 8d-8d-8s mode seems just like a hardware
> >>>>>> bug to
> >>>>>> me. So my proposal is to leave the core away and to handle the
> >>>>>> read id
> >>>>>> hack just in the macronix driver.
> >>>>>
> >>>>> +1
> >>>>
> >>>> I've looked at the xspi spec. There is no RDID specified. So I'd
> >>>> argue,
> >>>> the only pseudo standard is that RDID was only ever used with
> >>>> 1s1s1s.
> >>>> But
> >>>> we added spi_nor_read_id() with parameters suited for the "unusual"
> >>>> 8d8d8d
> >>>> case with additional address and dummy cycles. Just for checking
> >>>> whether
> >>>> the
> >>>> octal-dtr switch was successful. Therefore, we've already added
> >>>> parameters to
> >>>> spi_nor_read_id() which are not standard. Then we can just add one
> >>>> more.
> >>>> It's
> >>>> just how macronix is doing it. Again there is no standard.
> >>>> If we'd only put standard (or for the 9F pseudo standard) things in
> >>>> the
> >>>> core,
> >>>> then spi_nor_read_id() would need to check whether the flash is in
> >>>> 1s1s1s
> >>>> mode. And no I wouldn't prefer that ;)
> >>>>
> >>>
> >>> If we really want to be catholic, we should switch to 8d-8d-8s mode
> >>> and
> >>> then issue the read id and the core will handle the readid correctly.
> >>> But there is no such a thing, because macronix considers that it is
> >>> in
> >>> 8d-8d-8d mode at the time of issuing 8d-8d-8s read id. That's why I
> >>> say
> >>> it's a bug on their side. The core is meant to be generic, I don't
> >>> want
> >>> to pollute the core with manufacturer specific fixes.
> >>
> >> Then why does spi_nor_read_id() have the following parameters:
> >>
> >>  * @naddr:      number of address bytes to send. Can be zero if the
> >> operation
> >>  *              does not need to send an address.
> >>  * @ndummy:     number of dummy bytes to send after an opcode or
> >> address. Can
> >>  *              be zero if the operation does not require dummy bytes.
> >>  * @proto:      the SPI protocol for register operation.
> >>
> >> They aren't standard either. It's just the way spansion and micron
> >> transfers
> >> the ID via RDID in octal DTR mode. And now there's macronix who does
> >> it
> >> slightly
> >> different. But *neither* of them are standard. Why should one be in
> >> the
> >> core and
> >> one shouldn't.
> >>
> >> spi_nor_read_id() should just handle proto == SNOR_PROTO_8D_8D_8S.
> >
> > Yes, it should, but mx is in 8d-8d-8d at the time it issues the read
> > id,
> > thus it passes the core proto == SNOR_PROTO_8D_8D_8D, isn't it?
>
> The flash device is in octal dtr mode, which means it will expect the
> opcode in octal dtr mode. But the data phase mode depends on the opcode
> (and theoretically, the address phase, too). If you use the
> (non-standard) RDID on this flash command has to be executed as 8d8d8s.
>
> > If you
> > care about this, please send a patch addressing it, it's better to talk
> > on code. I don't see yet how you will handle it, but I'm open to review
> > some code.
>
> I really don't have time right now. But something along:
>
> spi_nor_read_id(.., proto) {
>    bool emulated_8d8d8s;
>
>    op = assemble_op(.., proto);
>
>    /* Emulate 8d8d8s if the controller doesn't support it */
>    if (!spi_mem_supports_op(op) && proto == 8d8d8s) {
>       op = assemble_op(.., 8d8d8d);
>       emulated_8d8d8s = true;
>    }
>    execute_op()
>    if (emulated_8d8d8s) {
>       /* discard every other byte of the response */
>    }
> }
After checking with Macronix designer, a-a-b-b-c-c is the data arrangement for
read id operation of flash in 8D-8D-8D.
Flash transfer data while DQS raising and falling edge exactlly.
It is not a hardware bug or 8D-8D-8S.
So that I think we only need a method for specific comparison after
read id in 8d-8d-8d.

>
> Later (or even now), that could be moved down the callstack into
> the spimem core.
>
> -michael

Thanks
Jaime

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[Michael Walle]

Hi,

>> >>>>>> I see this flash supports 1-1-1, 8-8-8, and 8d-8d-8d, there are no
>> >>>>>> mixed
>> >>>>>> modes supported, thus a 8d-8d-8s mode seems just like a hardware
>> >>>>>> bug to
>> >>>>>> me. So my proposal is to leave the core away and to handle the
>> >>>>>> read id
>> >>>>>> hack just in the macronix driver.
>> >>>>>
>> >>>>> +1
>> >>>>
>> >>>> I've looked at the xspi spec. There is no RDID specified. So I'd
>> >>>> argue,
>> >>>> the only pseudo standard is that RDID was only ever used with
>> >>>> 1s1s1s.
>> >>>> But
>> >>>> we added spi_nor_read_id() with parameters suited for the "unusual"
>> >>>> 8d8d8d
>> >>>> case with additional address and dummy cycles. Just for checking
>> >>>> whether
>> >>>> the
>> >>>> octal-dtr switch was successful. Therefore, we've already added
>> >>>> parameters to
>> >>>> spi_nor_read_id() which are not standard. Then we can just add one
>> >>>> more.
>> >>>> It's
>> >>>> just how macronix is doing it. Again there is no standard.
>> >>>> If we'd only put standard (or for the 9F pseudo standard) things in
>> >>>> the
>> >>>> core,
>> >>>> then spi_nor_read_id() would need to check whether the flash is in
>> >>>> 1s1s1s
>> >>>> mode. And no I wouldn't prefer that ;)
>> >>>>
>> >>>
>> >>> If we really want to be catholic, we should switch to 8d-8d-8s mode
>> >>> and
>> >>> then issue the read id and the core will handle the readid correctly.
>> >>> But there is no such a thing, because macronix considers that it is
>> >>> in
>> >>> 8d-8d-8d mode at the time of issuing 8d-8d-8s read id. That's why I
>> >>> say
>> >>> it's a bug on their side. The core is meant to be generic, I don't
>> >>> want
>> >>> to pollute the core with manufacturer specific fixes.
>> >>
>> >> Then why does spi_nor_read_id() have the following parameters:
>> >>
>> >>  * @naddr:      number of address bytes to send. Can be zero if the
>> >> operation
>> >>  *              does not need to send an address.
>> >>  * @ndummy:     number of dummy bytes to send after an opcode or
>> >> address. Can
>> >>  *              be zero if the operation does not require dummy bytes.
>> >>  * @proto:      the SPI protocol for register operation.
>> >>
>> >> They aren't standard either. It's just the way spansion and micron
>> >> transfers
>> >> the ID via RDID in octal DTR mode. And now there's macronix who does
>> >> it
>> >> slightly
>> >> different. But *neither* of them are standard. Why should one be in
>> >> the
>> >> core and
>> >> one shouldn't.
>> >>
>> >> spi_nor_read_id() should just handle proto == SNOR_PROTO_8D_8D_8S.
>> >
>> > Yes, it should, but mx is in 8d-8d-8d at the time it issues the read
>> > id,
>> > thus it passes the core proto == SNOR_PROTO_8D_8D_8D, isn't it?
>> 
>> The flash device is in octal dtr mode, which means it will expect the
>> opcode in octal dtr mode. But the data phase mode depends on the 
>> opcode
>> (and theoretically, the address phase, too). If you use the
>> (non-standard) RDID on this flash command has to be executed as 
>> 8d8d8s.
>> 
>> > If you
>> > care about this, please send a patch addressing it, it's better to talk
>> > on code. I don't see yet how you will handle it, but I'm open to review
>> > some code.
>> 
>> I really don't have time right now. But something along:
>> 
>> spi_nor_read_id(.., proto) {
>>    bool emulated_8d8d8s;
>> 
>>    op = assemble_op(.., proto);
>> 
>>    /* Emulate 8d8d8s if the controller doesn't support it */
>>    if (!spi_mem_supports_op(op) && proto == 8d8d8s) {
>>       op = assemble_op(.., 8d8d8d);
>>       emulated_8d8d8s = true;
>>    }
>>    execute_op()
>>    if (emulated_8d8d8s) {
>>       /* discard every other byte of the response */
>>    }
>> }
> After checking with Macronix designer, a-a-b-b-c-c is the data 
> arrangement for
> read id operation of flash in 8D-8D-8D.

Could you please point to any specification? I doubt there is one
and every vendor will do it slightly differently. I mean we already
have some flashes which (apparently) reply to RDID in 8d8d8d.

For example, see the Semper flash datasheet:
https://www.infineon.com/dgdl/Infineon-S28HS256T_S28HS512T_S28HS01GT_S28HL256T_S28HL512T_S28HL01GT_256-Mb_(32-MB)_512-Mb_(64-MB)_1-Gb_(128-MB)_HS-T_(1.8-V)_HL-T_(3.0-V)_Semper_Flash_with_Octal_Interface-DataSheet-v03_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ee6bca96f97&da=t

Have a look at Table 78 (or search for RDIDN_4_0) and Figure 28.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 3/6] mtd: spi-nor: add support for Macronix Octal flash

[liao jaime]

Hi Tudor

>
> Hi, Jaime,
>
> I think I already asked to split this in a patch per flash family,
> it will become more readable this way.
OK.

>
> On 08.09.2023 09:43, Jaime Liao wrote:
> > From: JaimeLiao <jaimeliao@mxic.com.tw>
> >
> > Adding Macronix Octal flash for Octal DTR support.
> >
> > The octaflash series can be divided into the following types:
> >
> > MX25 series : Serial NOR Flash.
> > MX66 series : Serial NOR Flash with stacked die.(Size larger than 1Gb)
> > LM/UM series : Up to 250MHz clock frequency with both DTR/STR operation.
> > LW/UW series : Support simultaneous Read-while-Write operation in multiple
> >                bank architecture. Read-while-write feature which means read
> >                data one bank while another bank is programing or erasing.
> >
> > MX25LM : 3.0V Octal I/O
> >  -https://www.mxic.com.tw/Lists/Datasheet/Attachments/8729/MX25LM51245G,%203V,%20512Mb,%20v1.1.pdf
> >
> > MX25UM : 1.8V Octal I/O
> >  -https://www.mxic.com.tw/Lists/Datasheet/Attachments/8967/MX25UM51245G,%201.8V,%20512Mb,%20v1.5.pdf
> >
> > MX66LM : 3.0V Octal I/O with stacked die
> >  -https://www.mxic.com.tw/Lists/Datasheet/Attachments/8748/MX66LM1G45G,%203V,%201Gb,%20v1.1.pdf
> >
> > MX66UM : 1.8V Octal I/O with stacked die
> >  -https://www.mxic.com.tw/Lists/Datasheet/Attachments/8711/MX66UM1G45G,%201.8V,%201Gb,%20v1.1.pdf
> >
> > MX25LW : 3.0V Octal I/O with Read-while-Write
> > MX25UW : 1.8V Octal I/O with Read-while-Write
> > MX66LW : 3.0V Octal I/O with Read-while-Write and stack die
> > MX66UW : 1.8V Octal I/O with Read-while-Write and stack die
> >
> > About LW/UW series, please contact us freely if you have any
>
> this is not the place for such a statement.
Do you mean above descriptions are not needed?

>
> > questions. For adding Octal NOR Flash IDs, we have validated
> > each Flash on plateform zynq-picozed.
> >
> > As below are the SFDP table dump.
> >
> > zynq> cat jedec_id
> > c2943c
> > zynq> cat manufacturer
> > macronix
> > zynq> cat partname
> > mx66uw2g345gx0
> > zynq> xxd -p sfdp
> > 53464450080104fd00070114400000ff8701011c900000ff0a0001080001
> > 00ff05000105200100ff84000102340100ff0000000000000000ffffffff
> > ffffffffe5208affffffff7f00ff00ff00ff00ffeeffffffffff00ffffff
> > 00ff0c2010d800ff00ff87790100841200e2cc04674630b030b0f4bdd55c
> > 000000ff101000200000000000007c234800000000008888000000000000
> > 00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
> > 727103b80000000090a3188200c069960000000000000000727100987271
> > 00b8727100990000000072710098727100f872710099727100f900000000
> > 00000000011501d0727106d8000086500000060100000000020001030002
> > 00000000060100000000000072060002000000eec0697272717100d8f7f6
> > 000a00001445988043061f0021dcffff
> > zynq> md5sum sfdp
> > 839ad44d1e758bea30bd9917ba763ba6  sfdp
>
> please do the mtd_debug tests as well. I shall update mtd-debug with a
> sanity test in the future.
What information do you need?
Is it enough for programing data and reading back then checking the
hash id for both?

Thanks
Jaime

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[liao jaime]

Hi Michael

>
> Hi,
>
> >> >>>>>> I see this flash supports 1-1-1, 8-8-8, and 8d-8d-8d, there are no
> >> >>>>>> mixed
> >> >>>>>> modes supported, thus a 8d-8d-8s mode seems just like a hardware
> >> >>>>>> bug to
> >> >>>>>> me. So my proposal is to leave the core away and to handle the
> >> >>>>>> read id
> >> >>>>>> hack just in the macronix driver.
> >> >>>>>
> >> >>>>> +1
> >> >>>>
> >> >>>> I've looked at the xspi spec. There is no RDID specified. So I'd
> >> >>>> argue,
> >> >>>> the only pseudo standard is that RDID was only ever used with
> >> >>>> 1s1s1s.
> >> >>>> But
> >> >>>> we added spi_nor_read_id() with parameters suited for the "unusual"
> >> >>>> 8d8d8d
> >> >>>> case with additional address and dummy cycles. Just for checking
> >> >>>> whether
> >> >>>> the
> >> >>>> octal-dtr switch was successful. Therefore, we've already added
> >> >>>> parameters to
> >> >>>> spi_nor_read_id() which are not standard. Then we can just add one
> >> >>>> more.
> >> >>>> It's
> >> >>>> just how macronix is doing it. Again there is no standard.
> >> >>>> If we'd only put standard (or for the 9F pseudo standard) things in
> >> >>>> the
> >> >>>> core,
> >> >>>> then spi_nor_read_id() would need to check whether the flash is in
> >> >>>> 1s1s1s
> >> >>>> mode. And no I wouldn't prefer that ;)
> >> >>>>
> >> >>>
> >> >>> If we really want to be catholic, we should switch to 8d-8d-8s mode
> >> >>> and
> >> >>> then issue the read id and the core will handle the readid correctly.
> >> >>> But there is no such a thing, because macronix considers that it is
> >> >>> in
> >> >>> 8d-8d-8d mode at the time of issuing 8d-8d-8s read id. That's why I
> >> >>> say
> >> >>> it's a bug on their side. The core is meant to be generic, I don't
> >> >>> want
> >> >>> to pollute the core with manufacturer specific fixes.
> >> >>
> >> >> Then why does spi_nor_read_id() have the following parameters:
> >> >>
> >> >>  * @naddr:      number of address bytes to send. Can be zero if the
> >> >> operation
> >> >>  *              does not need to send an address.
> >> >>  * @ndummy:     number of dummy bytes to send after an opcode or
> >> >> address. Can
> >> >>  *              be zero if the operation does not require dummy bytes.
> >> >>  * @proto:      the SPI protocol for register operation.
> >> >>
> >> >> They aren't standard either. It's just the way spansion and micron
> >> >> transfers
> >> >> the ID via RDID in octal DTR mode. And now there's macronix who does
> >> >> it
> >> >> slightly
> >> >> different. But *neither* of them are standard. Why should one be in
> >> >> the
> >> >> core and
> >> >> one shouldn't.
> >> >>
> >> >> spi_nor_read_id() should just handle proto == SNOR_PROTO_8D_8D_8S.
> >> >
> >> > Yes, it should, but mx is in 8d-8d-8d at the time it issues the read
> >> > id,
> >> > thus it passes the core proto == SNOR_PROTO_8D_8D_8D, isn't it?
> >>
> >> The flash device is in octal dtr mode, which means it will expect the
> >> opcode in octal dtr mode. But the data phase mode depends on the
> >> opcode
> >> (and theoretically, the address phase, too). If you use the
> >> (non-standard) RDID on this flash command has to be executed as
> >> 8d8d8s.
> >>
> >> > If you
> >> > care about this, please send a patch addressing it, it's better to talk
> >> > on code. I don't see yet how you will handle it, but I'm open to review
> >> > some code.
> >>
> >> I really don't have time right now. But something along:
> >>
> >> spi_nor_read_id(.., proto) {
> >>    bool emulated_8d8d8s;
> >>
> >>    op = assemble_op(.., proto);
> >>
> >>    /* Emulate 8d8d8s if the controller doesn't support it */
> >>    if (!spi_mem_supports_op(op) && proto == 8d8d8s) {
> >>       op = assemble_op(.., 8d8d8d);
> >>       emulated_8d8d8s = true;
> >>    }
> >>    execute_op()
> >>    if (emulated_8d8d8s) {
> >>       /* discard every other byte of the response */
> >>    }
> >> }
> > After checking with Macronix designer, a-a-b-b-c-c is the data
> > arrangement for
> > read id operation of flash in 8D-8D-8D.
>
> Could you please point to any specification? I doubt there is one
> and every vendor will do it slightly differently. I mean we already
> have some flashes which (apparently) reply to RDID in 8d8d8d.
>
> For example, see the Semper flash datasheet:
> https://www.infineon.com/dgdl/Infineon-S28HS256T_S28HS512T_S28HS01GT_S28HL256T_S28HL512T_S28HL01GT_256-Mb_(32-MB)_512-Mb_(64-MB)_1-Gb_(128-MB)_HS-T_(1.8-V)_HL-T_(3.0-V)_Semper_Flash_with_Octal_Interface-DataSheet-v03_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ee6bca96f97&da=t
>
> Have a look at Table 78 (or search for RDIDN_4_0) and Figure 28.
For Figure 28 in this datasheet, I think it means that data latch
while DS raising and falling edge.
The data arrangement of read id follow the 9.2(Manufacturer and Device ID).
As below are the data arrangement for vendors.
For Infineon, a-b-c-d-e-f
For Micron, a-b-c-d-e-f
For Macronix, a-a-b-b-c-c

Most of vendor a, b and c are manufacturer ID, memory type and memory density.
Some vendors may add extended information in read id like d, e and f.
For Infineon, 03-05h provide extend in formations for flash.
For Micron, 03h-19h(4-20 bytes) are called UID which are factory descriptions.
For Macronix, a, b and c are all informations in read id operation.

>
> -michael

Thanks
Jaime

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[Michael Walle]

Hi,

>> > After checking with Macronix designer, a-a-b-b-c-c is the data
>> > arrangement for
>> > read id operation of flash in 8D-8D-8D.
>> 
>> Could you please point to any specification? I doubt there is one
>> and every vendor will do it slightly differently. I mean we already
>> have some flashes which (apparently) reply to RDID in 8d8d8d.
>> 
>> For example, see the Semper flash datasheet:
>> https://www.infineon.com/dgdl/Infineon-S28HS256T_S28HS512T_S28HS01GT_S28HL256T_S28HL512T_S28HL01GT_256-Mb_(32-MB)_512-Mb_(64-MB)_1-Gb_(128-MB)_HS-T_(1.8-V)_HL-T_(3.0-V)_Semper_Flash_with_Octal_Interface-DataSheet-v03_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ee6bca96f97&da=t
>> 
>> Have a look at Table 78 (or search for RDIDN_4_0) and Figure 28.
> For Figure 28 in this datasheet, I think it means that data latch
> while DS raising and falling edge.
> The data arrangement of read id follow the 9.2(Manufacturer and Device 
> ID).
> As below are the data arrangement for vendors.
> For Infineon, a-b-c-d-e-f
> For Micron, a-b-c-d-e-f
> For Macronix, a-a-b-b-c-c

So there is no standard among vendors, infineon as well as micron is
using 8d8d8d and macronix is using 8d8d8s. And - please correct me if
I'm wrong - the data strobe signal is optional.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[liao jaime]

Hi Michael

>
> Hi,
>
> >> > After checking with Macronix designer, a-a-b-b-c-c is the data
> >> > arrangement for
> >> > read id operation of flash in 8D-8D-8D.
> >>
> >> Could you please point to any specification? I doubt there is one
> >> and every vendor will do it slightly differently. I mean we already
> >> have some flashes which (apparently) reply to RDID in 8d8d8d.
> >>
> >> For example, see the Semper flash datasheet:
> >> https://www.infineon.com/dgdl/Infineon-S28HS256T_S28HS512T_S28HS01GT_S28HL256T_S28HL512T_S28HL01GT_256-Mb_(32-MB)_512-Mb_(64-MB)_1-Gb_(128-MB)_HS-T_(1.8-V)_HL-T_(3.0-V)_Semper_Flash_with_Octal_Interface-DataSheet-v03_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ee6bca96f97&da=t
> >>
> >> Have a look at Table 78 (or search for RDIDN_4_0) and Figure 28.
> > For Figure 28 in this datasheet, I think it means that data latch
> > while DS raising and falling edge.
> > The data arrangement of read id follow the 9.2(Manufacturer and Device
> > ID).
> > As below are the data arrangement for vendors.
> > For Infineon, a-b-c-d-e-f
> > For Micron, a-b-c-d-e-f
> > For Macronix, a-a-b-b-c-c
>
> So there is no standard among vendors, infineon as well as micron is
> using 8d8d8d and macronix is using 8d8d8s. And - please correct me if
Macronix read id operation is not 8d8d8s, it just look like 8d8d8s on datasheet
but flash send 2 bytes data per DQS cycle exactly.
I think whether to use 8d or 8s depends on how many times data is latched
within per cycle.

> I'm wrong - the data strobe signal is optional.
As I know, data strobe signal is needed when 8D-8D-8D mode.
Infineon data strobe signal called "DS".
Macronix and Micron data strobe signal called "DQS"

>
> -michael

Thanks
Jaime

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[Michael Walle]

Hi,

Am 2023-10-13 10:23, schrieb liao jaime:
>> >> > After checking with Macronix designer, a-a-b-b-c-c is the data
>> >> > arrangement for
>> >> > read id operation of flash in 8D-8D-8D.
>> >>
>> >> Could you please point to any specification? I doubt there is one
>> >> and every vendor will do it slightly differently. I mean we already
>> >> have some flashes which (apparently) reply to RDID in 8d8d8d.
>> >>
>> >> For example, see the Semper flash datasheet:
>> >> https://www.infineon.com/dgdl/Infineon-S28HS256T_S28HS512T_S28HS01GT_S28HL256T_S28HL512T_S28HL01GT_256-Mb_(32-MB)_512-Mb_(64-MB)_1-Gb_(128-MB)_HS-T_(1.8-V)_HL-T_(3.0-V)_Semper_Flash_with_Octal_Interface-DataSheet-v03_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ee6bca96f97&da=t
>> >>
>> >> Have a look at Table 78 (or search for RDIDN_4_0) and Figure 28.
>> > For Figure 28 in this datasheet, I think it means that data latch
>> > while DS raising and falling edge.
>> > The data arrangement of read id follow the 9.2(Manufacturer and Device
>> > ID).
>> > As below are the data arrangement for vendors.
>> > For Infineon, a-b-c-d-e-f
>> > For Micron, a-b-c-d-e-f
>> > For Macronix, a-a-b-b-c-c
>> 
>> So there is no standard among vendors, infineon as well as micron is
>> using 8d8d8d and macronix is using 8d8d8s. And - please correct me if
> Macronix read id operation is not 8d8d8s, it just look like 8d8d8s on 
> datasheet
> but flash send 2 bytes data per DQS cycle exactly.

Two DQS edges, yes.

> I think whether to use 8d or 8s depends on how many times data is 
> latched
> within per cycle.

I see and I agree with you. The flash behaves like it's in 8d8d8d mode, 
but
just send every byte of the id twice. Well, then I'd argue, it's a quirk 
of
your flash.

So Tudor and Pratyush want to hide that away in macronix.c. I would like 
to
see this handled in the core, because all flashes do the very same after
switching to octal mode and that is trying to do a rdid and see whether
they got a sane response. And during cleanup I noticed that this code is
pretty much copy and paste among all these flashes and looks very
open-coded.

But to move forward here, keep it in macronix.c.

>> I'm wrong - the data strobe signal is optional.
> As I know, data strobe signal is needed when 8D-8D-8D mode.
> Infineon data strobe signal called "DS".
> Macronix and Micron data strobe signal called "DQS"

Maybe Pratyush can help here. But as far as I know, the data strobe
is optional. E.g. on a layerscape ls1028 you can use an internal dummy
strobe if it's not connected. The timing is then determined by pad
delays.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[liao jaime]

Hi Michael


>
> Hi,
>
> Am 2023-10-13 10:23, schrieb liao jaime:
> >> >> > After checking with Macronix designer, a-a-b-b-c-c is the data
> >> >> > arrangement for
> >> >> > read id operation of flash in 8D-8D-8D.
> >> >>
> >> >> Could you please point to any specification? I doubt there is one
> >> >> and every vendor will do it slightly differently. I mean we already
> >> >> have some flashes which (apparently) reply to RDID in 8d8d8d.
> >> >>
> >> >> For example, see the Semper flash datasheet:
> >> >> https://www.infineon.com/dgdl/Infineon-S28HS256T_S28HS512T_S28HS01GT_S28HL256T_S28HL512T_S28HL01GT_256-Mb_(32-MB)_512-Mb_(64-MB)_1-Gb_(128-MB)_HS-T_(1.8-V)_HL-T_(3.0-V)_Semper_Flash_with_Octal_Interface-DataSheet-v03_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ee6bca96f97&da=t
> >> >>
> >> >> Have a look at Table 78 (or search for RDIDN_4_0) and Figure 28.
> >> > For Figure 28 in this datasheet, I think it means that data latch
> >> > while DS raising and falling edge.
> >> > The data arrangement of read id follow the 9.2(Manufacturer and Device
> >> > ID).
> >> > As below are the data arrangement for vendors.
> >> > For Infineon, a-b-c-d-e-f
> >> > For Micron, a-b-c-d-e-f
> >> > For Macronix, a-a-b-b-c-c
> >>
> >> So there is no standard among vendors, infineon as well as micron is
> >> using 8d8d8d and macronix is using 8d8d8s. And - please correct me if
> > Macronix read id operation is not 8d8d8s, it just look like 8d8d8s on
> > datasheet
> > but flash send 2 bytes data per DQS cycle exactly.
>
> Two DQS edges, yes.
>
> > I think whether to use 8d or 8s depends on how many times data is
> > latched
> > within per cycle.
>
> I see and I agree with you. The flash behaves like it's in 8d8d8d mode,
> but
> just send every byte of the id twice. Well, then I'd argue, it's a quirk
> of
> your flash.
I cannot refute that.

>
> So Tudor and Pratyush want to hide that away in macronix.c. I would like
> to
> see this handled in the core, because all flashes do the very same after
> switching to octal mode and that is trying to do a rdid and see whether
> they got a sane response. And during cleanup I noticed that this code is
> pretty much copy and paste among all these flashes and looks very
> open-coded.
>
> But to move forward here, keep it in macronix.c.
Do you mean that I can keep v3 method as below for comparing ID after enable
8D-8D-8D mode?

for (i = 0; i < nor->info->id_len; i++)
if (buf[i * 2] != nor->info->id[i])
return -EINVAL;

>
> >> I'm wrong - the data strobe signal is optional.
> > As I know, data strobe signal is needed when 8D-8D-8D mode.
> > Infineon data strobe signal called "DS".
> > Macronix and Micron data strobe signal called "DQS"
>
> Maybe Pratyush can help here. But as far as I know, the data strobe
> is optional. E.g. on a layerscape ls1028 you can use an internal dummy
> strobe if it's not connected. The timing is then determined by pad
> delays.
It would be great if Pratyush could provide additional information.

>
> -michael

Thanks
Jaime

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[Michael Walle]

Am 2023-10-13 11:14, schrieb liao jaime:
> Hi Michael
> 
> 
>> 
>> Hi,
>> 
>> Am 2023-10-13 10:23, schrieb liao jaime:
>> >> >> > After checking with Macronix designer, a-a-b-b-c-c is the data
>> >> >> > arrangement for
>> >> >> > read id operation of flash in 8D-8D-8D.
>> >> >>
>> >> >> Could you please point to any specification? I doubt there is one
>> >> >> and every vendor will do it slightly differently. I mean we already
>> >> >> have some flashes which (apparently) reply to RDID in 8d8d8d.
>> >> >>
>> >> >> For example, see the Semper flash datasheet:
>> >> >> https://www.infineon.com/dgdl/Infineon-S28HS256T_S28HS512T_S28HS01GT_S28HL256T_S28HL512T_S28HL01GT_256-Mb_(32-MB)_512-Mb_(64-MB)_1-Gb_(128-MB)_HS-T_(1.8-V)_HL-T_(3.0-V)_Semper_Flash_with_Octal_Interface-DataSheet-v03_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ee6bca96f97&da=t
>> >> >>
>> >> >> Have a look at Table 78 (or search for RDIDN_4_0) and Figure 28.
>> >> > For Figure 28 in this datasheet, I think it means that data latch
>> >> > while DS raising and falling edge.
>> >> > The data arrangement of read id follow the 9.2(Manufacturer and Device
>> >> > ID).
>> >> > As below are the data arrangement for vendors.
>> >> > For Infineon, a-b-c-d-e-f
>> >> > For Micron, a-b-c-d-e-f
>> >> > For Macronix, a-a-b-b-c-c
>> >>
>> >> So there is no standard among vendors, infineon as well as micron is
>> >> using 8d8d8d and macronix is using 8d8d8s. And - please correct me if
>> > Macronix read id operation is not 8d8d8s, it just look like 8d8d8s on
>> > datasheet
>> > but flash send 2 bytes data per DQS cycle exactly.
>> 
>> Two DQS edges, yes.
>> 
>> > I think whether to use 8d or 8s depends on how many times data is
>> > latched
>> > within per cycle.
>> 
>> I see and I agree with you. The flash behaves like it's in 8d8d8d 
>> mode,
>> but
>> just send every byte of the id twice. Well, then I'd argue, it's a 
>> quirk
>> of
>> your flash.
> I cannot refute that.
> 
>> 
>> So Tudor and Pratyush want to hide that away in macronix.c. I would 
>> like
>> to
>> see this handled in the core, because all flashes do the very same 
>> after
>> switching to octal mode and that is trying to do a rdid and see 
>> whether
>> they got a sane response. And during cleanup I noticed that this code 
>> is
>> pretty much copy and paste among all these flashes and looks very
>> open-coded.
>> 
>> But to move forward here, keep it in macronix.c.
> Do you mean that I can keep v3 method as below for comparing ID after 
> enable
> 8D-8D-8D mode?
> 
> for (i = 0; i < nor->info->id_len; i++)
> if (buf[i * 2] != nor->info->id[i])
> return -EINVAL;

Technically, this is wrong. Because we only read SPI_NOR_MAX_ID_LEN.
and nor->info->id_len might be as large as SPI_NOR_MAX_ID_LEN. So you'll
have out of bounds accesses. spi_nor_read_id() isn't really useful here.
You'd need to cook up your own RDID command. And that results in even 
more
open coding...

Aside from that, I'd also check that buf[i] == buf[i+1] for i=0,2,4,...

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/6] mtd: spi-nor: Add manufacturer read id function

[Pratyush Yadav]

On Fri, Oct 13 2023, Michael Walle wrote:

> Hi,
>
> Am 2023-10-13 10:23, schrieb liao jaime:
[...]
>>> I'm wrong - the data strobe signal is optional.
>> As I know, data strobe signal is needed when 8D-8D-8D mode.
>> Infineon data strobe signal called "DS".
>> Macronix and Micron data strobe signal called "DQS"
>
> Maybe Pratyush can help here. But as far as I know, the data strobe
> is optional. E.g. on a layerscape ls1028 you can use an internal dummy
> strobe if it's not connected. The timing is then determined by pad
> delays.

Correct. The data strobe is optional and the controller does not have to
use it. Though for higher frequencies, it is usually needed to sample
the data lines at the right time -- given the time window to do so is
smaller and more precision is required.

For example, see my patch here [0] which enabled DQS sampling for PHY
reads since those are higher frequency. Since that patch has not yet
made it to mainline, currently the Cadence Quadspi driver does not use
DQS and still works fine with 8D-8D-8D mode. It does so based on its
hard-coded delays in device tree [1] (see cdns,read-delay).

[0] https://lore.kernel.org/linux-mtd/20210311191216.7363-5-p.yadav@ti.com/
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/spi/cdns,qspi-nor-peripheral-props.yaml

-- 
Regards,
Pratyush Yadav

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/