Linux-mtd Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2]  Add octal DTR support for Macronix flash
@ 2023-07-27  9:16 Jaime Liao
  2023-07-27  9:16 ` [PATCH v2 1/2] mtd: spi-nor: add Octal " Jaime Liao
  2023-07-27  9:16 ` [PATCH v2 2/2] mtd: spi-nor: add support for Macronix Octal flash Jaime Liao
  0 siblings, 2 replies; 16+ messages in thread
From: Jaime Liao @ 2023-07-27  9:16 UTC (permalink / raw)
  To: linux-mtd, tudor.ambarus, pratyush, michael, miquel.raynal
  Cc: leoyu, JaimeLiao

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

This series add method for Macronix Octal DTR Eable/Disable
and add Macronix Octal flash support.

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 (2):
  mtd: spi-nor: add Octal DTR support for Macronix flash
  mtd: spi-nor: add support for Macronix Octal flash

 drivers/mtd/spi-nor/macronix.c | 123 ++++++++++++++++++++++++++++++++-
 1 file changed, 121 insertions(+), 2 deletions(-)

-- 
2.25.1


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

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

* [PATCH v2 1/2] mtd: spi-nor: add Octal DTR support for Macronix flash
  2023-07-27  9:16 [PATCH v2 0/2] Add octal DTR support for Macronix flash Jaime Liao
@ 2023-07-27  9:16 ` Jaime Liao
  2023-07-27 10:00   ` Tudor Ambarus
  2023-07-27  9:16 ` [PATCH v2 2/2] mtd: spi-nor: add support for Macronix Octal flash Jaime Liao
  1 sibling, 1 reply; 16+ messages in thread
From: Jaime Liao @ 2023-07-27  9:16 UTC (permalink / raw)
  To: linux-mtd, tudor.ambarus, pratyush, michael, miquel.raynal
  Cc: leoyu, JaimeLiao

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.

For enable Macronix flashes in Octal DTR mode, Configuration
Register2 DOPI bit should be set and DC bit setting for dummy
cycles.

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 04888258e891..b397fd274868 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -8,6 +8,22 @@
 
 #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	/* For setting octal DTR mode */
+#define SPINOR_REG_MXIC_CR2_DC		0x00000300	/* 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_DC_20		0x0		/* Setting dummy cycles to 20 */
+#define SPINOR_REG_MXIC_ADDR_BYTES	4		/* Fixed R/W volatile address bytes to 4 */
+
+/* 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,9 +121,90 @@ 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, i;
+	int ret;
+
+	/* Use 20 dummy cycles for memory array reads. */
+	buf[0] = SPINOR_REG_MXIC_DC_20;
+	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;
+	}
+
+	/* Macronix SPI-NOR flash 8D-8D-8D read ID would get 6 bytes data A-A-B-B-C-C */
+	for (i = 0; i < nor->info->id_len; i++)
+		if (buf[i * 2] != nor->info->id[i])
+			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_octal_dtr_enable(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;
+	nor->params->octal_dtr_enable = macronix_nor_octal_dtr_enable;
 }
 
 static void macronix_nor_late_init(struct spi_nor *nor)
-- 
2.25.1


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

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

* [PATCH v2 2/2] mtd: spi-nor: add support for Macronix Octal flash
  2023-07-27  9:16 [PATCH v2 0/2] Add octal DTR support for Macronix flash Jaime Liao
  2023-07-27  9:16 ` [PATCH v2 1/2] mtd: spi-nor: add Octal " Jaime Liao
@ 2023-07-27  9:16 ` Jaime Liao
  2023-07-27  9:54   ` Tudor Ambarus
  1 sibling, 1 reply; 16+ messages in thread
From: Jaime Liao @ 2023-07-27  9:16 UTC (permalink / raw)
  To: linux-mtd, tudor.ambarus, pratyush, michael, miquel.raynal
  Cc: leoyu, JaimeLiao

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

MX25UM51245G : 512Mb flash, with Word mode data output format when Octal DTR read
MX25UM51345G : 512Mb flash, with Byte mode data output format when Octal DTR read

Macronix Octal flash with two types, "byte mode" and "word mode".
Because of word mode flash with byte swap issue when Octal DTR read,
adding byte mode flash in this patchset only.

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
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
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
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
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 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
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 | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index b397fd274868..a5aee10590b8 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -99,8 +99,8 @@ static const struct flash_info macronix_nor_parts[] = {
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
 		FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
 	{ "mx25uw51245g", INFOB(0xc2813a, 0, 0, 0, 4)
-		PARSE_SFDP
-		FLAGS(SPI_NOR_RWW) },
+		FLAGS(SPI_NOR_RWW)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
 	{ "mx25v8035f",  INFO(0xc22314, 0, 64 * 1024,  16)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
 			      SPI_NOR_QUAD_READ) },
@@ -119,6 +119,28 @@ 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) },
+	{ "mx66uw2g345g", INFOB(0xc2843c, 0, 0, 0, 4)
+		FLAGS(SPI_NOR_RWW)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP) },
+	{ "mx66uw2g345gx0", INFOB(0xc2943c, 0, 0, 0, 4)
+		FLAGS(SPI_NOR_RWW)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP) },
+	{ "mx25uw51345g", INFOB(0xc2843a, 0, 0, 0, 4)
+		FLAGS(SPI_NOR_RWW)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
+	{ "mx25um25345g", INFO(0xc28339, 0, 0, 0)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
+	{ "mx25uw25345g", INFOB(0xc28439, 0, 0, 0, 4)
+		FLAGS(SPI_NOR_RWW)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
+	{ "mx25uw12345g", INFOB(0xc28438, 0, 0, 0, 4)
+		FLAGS(SPI_NOR_RWW)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
+	{ "mx25uw6345g", INFOB(0xc28437, 0, 0, 0, 4)
+		FLAGS(SPI_NOR_RWW)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
 };
 
 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/

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

* Re: [PATCH v2 2/2] mtd: spi-nor: add support for Macronix Octal flash
  2023-07-27  9:16 ` [PATCH v2 2/2] mtd: spi-nor: add support for Macronix Octal flash Jaime Liao
@ 2023-07-27  9:54   ` Tudor Ambarus
  2023-07-27 10:04     ` Michael Walle
  0 siblings, 1 reply; 16+ messages in thread
From: Tudor Ambarus @ 2023-07-27  9:54 UTC (permalink / raw)
  To: Jaime Liao, linux-mtd, pratyush, michael, miquel.raynal; +Cc: leoyu, JaimeLiao



On 7/27/23 10:16, 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
> 
> MX25UM51245G : 512Mb flash, with Word mode data output format when Octal DTR read
> MX25UM51345G : 512Mb flash, with Byte mode data output format when Octal DTR read

split this in multiple patches and send a patch for each family please.
> 
> Macronix Octal flash with two types, "byte mode" and "word mode".
> Because of word mode flash with byte swap issue when Octal DTR read,
> adding byte mode flash in this patchset only.

Please reformulate this paragraph. So all these flashes that you're adding
don't swap bytes on a 2 byte boundary?
> 
> 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.

remove this paragraph.
> 
> As below are the SFDP table dump.

I'd like you to do some sanity checks as well, using mtd_debug. There
was a script that I advertised, please search ml for other flash updates
to see how to do it.

> 
> 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
> 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
> 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
> 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
> 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 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
> 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 | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index b397fd274868..a5aee10590b8 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -99,8 +99,8 @@ static const struct flash_info macronix_nor_parts[] = {
>  		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>  		FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
>  	{ "mx25uw51245g", INFOB(0xc2813a, 0, 0, 0, 4)
> -		PARSE_SFDP
> -		FLAGS(SPI_NOR_RWW) },
> +		FLAGS(SPI_NOR_RWW)
> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },

why? Here you don't add a new flash, but change an existing one.
Add a dedicated patch for this. Why can't you use SFDP?

>  	{ "mx25v8035f",  INFO(0xc22314, 0, 64 * 1024,  16)
>  		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
>  			      SPI_NOR_QUAD_READ) },
> @@ -119,6 +119,28 @@ 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) },
> +	{ "mx66uw2g345g", INFOB(0xc2843c, 0, 0, 0, 4)
> +		FLAGS(SPI_NOR_RWW)
> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ |
> +			      SPI_NOR_OCTAL_DTR_PP) },
PARSE_SFDP instead.

> +	{ "mx66uw2g345gx0", INFOB(0xc2943c, 0, 0, 0, 4)
> +		FLAGS(SPI_NOR_RWW)
> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ |
> +			      SPI_NOR_OCTAL_DTR_PP) },
> +	{ "mx25uw51345g", INFOB(0xc2843a, 0, 0, 0, 4)
> +		FLAGS(SPI_NOR_RWW)
> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
> +	{ "mx25um25345g", INFO(0xc28339, 0, 0, 0)
> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
> +	{ "mx25uw25345g", INFOB(0xc28439, 0, 0, 0, 4)
> +		FLAGS(SPI_NOR_RWW)
> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
> +	{ "mx25uw12345g", INFOB(0xc28438, 0, 0, 0, 4)
> +		FLAGS(SPI_NOR_RWW)
> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
> +	{ "mx25uw6345g", INFOB(0xc28437, 0, 0, 0, 4)
> +		FLAGS(SPI_NOR_RWW)
> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },

PARSE_SFDP for all the flashes please.
>  };
>  
>  static int macronix_nor_octal_dtr_en(struct spi_nor *nor)

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

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

* Re: [PATCH v2 1/2] mtd: spi-nor: add Octal DTR support for Macronix flash
  2023-07-27  9:16 ` [PATCH v2 1/2] mtd: spi-nor: add Octal " Jaime Liao
@ 2023-07-27 10:00   ` Tudor Ambarus
  2023-07-27 10:04     ` Tudor Ambarus
                       ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Tudor Ambarus @ 2023-07-27 10:00 UTC (permalink / raw)
  To: Jaime Liao, linux-mtd, pratyush, michael, miquel.raynal; +Cc: leoyu, JaimeLiao



On 7/27/23 10:16, 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.

20 dummy cycles is particular for a specific operating frequency.

If you test the flash at different speeds, let's say at 30 MHz, and
at their highest frequency (133, 200 MHZ?) does the flash work with
current settings?
> 
> For enable Macronix flashes in Octal DTR mode, Configuration
> Register2 DOPI bit should be set and DC bit setting for dummy
> cycles.

Use imperative.
> 
> 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 04888258e891..b397fd274868 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -8,6 +8,22 @@
>  
>  #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	/* For setting octal DTR mode */
> +#define SPINOR_REG_MXIC_CR2_DC		0x00000300	/* 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_DC_20		0x0		/* Setting dummy cycles to 20 */
> +#define SPINOR_REG_MXIC_ADDR_BYTES	4		/* Fixed R/W volatile address bytes to 4 */
> +
> +/* 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,9 +121,90 @@ 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, i;
> +	int ret;
> +
> +	/* Use 20 dummy cycles for memory array reads. */
> +	buf[0] = SPINOR_REG_MXIC_DC_20;
> +	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;
> +	}
> +
> +	/* Macronix SPI-NOR flash 8D-8D-8D read ID would get 6 bytes data A-A-B-B-C-C */
> +	for (i = 0; i < nor->info->id_len; i++)
> +		if (buf[i * 2] != nor->info->id[i])
> +			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_octal_dtr_enable(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;
> +	nor->params->octal_dtr_enable = macronix_nor_octal_dtr_enable;

this must be done in the late_init hook, I'd like to get rid of the
default_init hook.
>  }
>  
>  static void macronix_nor_late_init(struct spi_nor *nor)

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

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

* Re: [PATCH v2 2/2] mtd: spi-nor: add support for Macronix Octal flash
  2023-07-27  9:54   ` Tudor Ambarus
@ 2023-07-27 10:04     ` Michael Walle
  2023-07-27 10:10       ` Tudor Ambarus
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Walle @ 2023-07-27 10:04 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: Jaime Liao, linux-mtd, pratyush, miquel.raynal, leoyu, JaimeLiao

Hi,

>> +	{ "mx66uw2g345gx0", INFOB(0xc2943c, 0, 0, 0, 4)
>> +		FLAGS(SPI_NOR_RWW)
>> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ 
>> |
>> +			      SPI_NOR_OCTAL_DTR_PP) },
>> +	{ "mx25uw51345g", INFOB(0xc2843a, 0, 0, 0, 4)
>> +		FLAGS(SPI_NOR_RWW)
>> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | 
>> SPI_NOR_OCTAL_DTR_PP) },
>> +	{ "mx25um25345g", INFO(0xc28339, 0, 0, 0)
>> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | 
>> SPI_NOR_OCTAL_DTR_PP) },
>> +	{ "mx25uw25345g", INFOB(0xc28439, 0, 0, 0, 4)
>> +		FLAGS(SPI_NOR_RWW)
>> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | 
>> SPI_NOR_OCTAL_DTR_PP) },
>> +	{ "mx25uw12345g", INFOB(0xc28438, 0, 0, 0, 4)
>> +		FLAGS(SPI_NOR_RWW)
>> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | 
>> SPI_NOR_OCTAL_DTR_PP) },
>> +	{ "mx25uw6345g", INFOB(0xc28437, 0, 0, 0, 4)
>> +		FLAGS(SPI_NOR_RWW)
>> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | 
>> SPI_NOR_OCTAL_DTR_PP) },
> 
> PARSE_SFDP for all the flashes please.

And no NO_SFDP_FLAGS() as discussed.

-michael

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

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

* Re: [PATCH v2 1/2] mtd: spi-nor: add Octal DTR support for Macronix flash
  2023-07-27 10:00   ` Tudor Ambarus
@ 2023-07-27 10:04     ` Tudor Ambarus
  2023-07-27 10:11     ` Michael Walle
  2023-08-01  7:24     ` liao jaime
  2 siblings, 0 replies; 16+ messages in thread
From: Tudor Ambarus @ 2023-07-27 10:04 UTC (permalink / raw)
  To: Jaime Liao, linux-mtd, pratyush, michael, miquel.raynal; +Cc: leoyu, JaimeLiao



On 7/27/23 11:00, Tudor Ambarus wrote:
> +	/* Macronix SPI-NOR flash 8D-8D-8D read ID would get 6 bytes data A-A-B-B-C-C */
> +	for (i = 0; i < nor->info->id_len; i++)
> +		if (buf[i * 2] != nor->info->id[i])
> +			return -EINVAL;

check all the bytes, not just some

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

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

* Re: [PATCH v2 2/2] mtd: spi-nor: add support for Macronix Octal flash
  2023-07-27 10:04     ` Michael Walle
@ 2023-07-27 10:10       ` Tudor Ambarus
  2023-07-27 10:12         ` Michael Walle
  0 siblings, 1 reply; 16+ messages in thread
From: Tudor Ambarus @ 2023-07-27 10:10 UTC (permalink / raw)
  To: Michael Walle
  Cc: Jaime Liao, linux-mtd, pratyush, miquel.raynal, leoyu, JaimeLiao



On 7/27/23 11:04, Michael Walle wrote:
> Hi,
> 
>>> +    { "mx66uw2g345gx0", INFOB(0xc2943c, 0, 0, 0, 4)
>>> +        FLAGS(SPI_NOR_RWW)
>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ |
>>> +                  SPI_NOR_OCTAL_DTR_PP) },
>>> +    { "mx25uw51345g", INFOB(0xc2843a, 0, 0, 0, 4)
>>> +        FLAGS(SPI_NOR_RWW)
>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
>>> +    { "mx25um25345g", INFO(0xc28339, 0, 0, 0)
>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
>>> +    { "mx25uw25345g", INFOB(0xc28439, 0, 0, 0, 4)
>>> +        FLAGS(SPI_NOR_RWW)
>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
>>> +    { "mx25uw12345g", INFOB(0xc28438, 0, 0, 0, 4)
>>> +        FLAGS(SPI_NOR_RWW)
>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
>>> +    { "mx25uw6345g", INFOB(0xc28437, 0, 0, 0, 4)
>>> +        FLAGS(SPI_NOR_RWW)
>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
>>
>> PARSE_SFDP for all the flashes please.
> 
> And no NO_SFDP_FLAGS() as discussed.
> 

Are there flavors of these flashes that don't support SFDP,
or why did you ask for NO_SFDP_FLAGS? Do each of these flashes
have SFDP and no-SFDP flavors?

If that's the case, I'm not yet sure if we want to mix PARSE_SFDP
with NO_SFDP_FLAGS().

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

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

* Re: [PATCH v2 1/2] mtd: spi-nor: add Octal DTR support for Macronix flash
  2023-07-27 10:00   ` Tudor Ambarus
  2023-07-27 10:04     ` Tudor Ambarus
@ 2023-07-27 10:11     ` Michael Walle
  2023-07-28  9:46       ` Michael Walle
  2023-08-01  7:24     ` liao jaime
  2 siblings, 1 reply; 16+ messages in thread
From: Michael Walle @ 2023-07-27 10:11 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: Jaime Liao, linux-mtd, pratyush, miquel.raynal, leoyu, JaimeLiao

Hi,

>> +	/* Macronix SPI-NOR flash 8D-8D-8D read ID would get 6 bytes data 
>> A-A-B-B-C-C */
>> +	for (i = 0; i < nor->info->id_len; i++)
>> +		if (buf[i * 2] != nor->info->id[i])
>> +			return -EINVAL;

We need some kind of per flash rdid override. I guess rdid won't work
in octal mode then. Are any other commands also affected?

I had something like a per flash .rdid op in mind where for this flash
you do like

rdid() {
    if (octal mode) {
        buf = spi_xfer(2 * SPI_NOR_MAX_ID_LEN);
        strip_every_other_byte(buf);
        return buf;
    } else {
       spi_nor_default_read_id();
    }
}

rename spi_nor_read_id() to spi_nor_default_read_id()

spi_nor_read_id() {
    if (nor->info->rdid)
       return nor->info->rdid();
    return spi_nor_default_read_id();
}

And also move the readid and compare code to a common helper, as
discussed.

-michael

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

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

* Re: [PATCH v2 2/2] mtd: spi-nor: add support for Macronix Octal flash
  2023-07-27 10:10       ` Tudor Ambarus
@ 2023-07-27 10:12         ` Michael Walle
  2023-07-27 10:14           ` Tudor Ambarus
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Walle @ 2023-07-27 10:12 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: Jaime Liao, linux-mtd, pratyush, miquel.raynal, leoyu, JaimeLiao

Am 2023-07-27 12:10, schrieb Tudor Ambarus:
> On 7/27/23 11:04, Michael Walle wrote:
>> Hi,
>> 
>>>> +    { "mx66uw2g345gx0", INFOB(0xc2943c, 0, 0, 0, 4)
>>>> +        FLAGS(SPI_NOR_RWW)
>>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ | 
>>>> SPI_NOR_OCTAL_DTR_READ |
>>>> +                  SPI_NOR_OCTAL_DTR_PP) },
>>>> +    { "mx25uw51345g", INFOB(0xc2843a, 0, 0, 0, 4)
>>>> +        FLAGS(SPI_NOR_RWW)
>>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | 
>>>> SPI_NOR_OCTAL_DTR_PP) },
>>>> +    { "mx25um25345g", INFO(0xc28339, 0, 0, 0)
>>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | 
>>>> SPI_NOR_OCTAL_DTR_PP) },
>>>> +    { "mx25uw25345g", INFOB(0xc28439, 0, 0, 0, 4)
>>>> +        FLAGS(SPI_NOR_RWW)
>>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | 
>>>> SPI_NOR_OCTAL_DTR_PP) },
>>>> +    { "mx25uw12345g", INFOB(0xc28438, 0, 0, 0, 4)
>>>> +        FLAGS(SPI_NOR_RWW)
>>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | 
>>>> SPI_NOR_OCTAL_DTR_PP) },
>>>> +    { "mx25uw6345g", INFOB(0xc28437, 0, 0, 0, 4)
>>>> +        FLAGS(SPI_NOR_RWW)
>>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | 
>>>> SPI_NOR_OCTAL_DTR_PP) },
>>> 
>>> PARSE_SFDP for all the flashes please.
>> 
>> And no NO_SFDP_FLAGS() as discussed.
>> 
> 
> Are there flavors of these flashes that don't support SFDP,
> or why did you ask for NO_SFDP_FLAGS? Do each of these flashes
> have SFDP and no-SFDP flavors?

I asked to drop NO_SFDP_FLAGS().

-michael

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

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

* Re: [PATCH v2 2/2] mtd: spi-nor: add support for Macronix Octal flash
  2023-07-27 10:12         ` Michael Walle
@ 2023-07-27 10:14           ` Tudor Ambarus
  0 siblings, 0 replies; 16+ messages in thread
From: Tudor Ambarus @ 2023-07-27 10:14 UTC (permalink / raw)
  To: Michael Walle
  Cc: Jaime Liao, linux-mtd, pratyush, miquel.raynal, leoyu, JaimeLiao



On 7/27/23 11:12, Michael Walle wrote:
> Am 2023-07-27 12:10, schrieb Tudor Ambarus:
>> On 7/27/23 11:04, Michael Walle wrote:
>>> Hi,
>>>
>>>>> +    { "mx66uw2g345gx0", INFOB(0xc2943c, 0, 0, 0, 4)
>>>>> +        FLAGS(SPI_NOR_RWW)
>>>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ |
>>>>> +                  SPI_NOR_OCTAL_DTR_PP) },
>>>>> +    { "mx25uw51345g", INFOB(0xc2843a, 0, 0, 0, 4)
>>>>> +        FLAGS(SPI_NOR_RWW)
>>>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
>>>>> +    { "mx25um25345g", INFO(0xc28339, 0, 0, 0)
>>>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
>>>>> +    { "mx25uw25345g", INFOB(0xc28439, 0, 0, 0, 4)
>>>>> +        FLAGS(SPI_NOR_RWW)
>>>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
>>>>> +    { "mx25uw12345g", INFOB(0xc28438, 0, 0, 0, 4)
>>>>> +        FLAGS(SPI_NOR_RWW)
>>>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
>>>>> +    { "mx25uw6345g", INFOB(0xc28437, 0, 0, 0, 4)
>>>>> +        FLAGS(SPI_NOR_RWW)
>>>>> +        NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) },
>>>>
>>>> PARSE_SFDP for all the flashes please.
>>>
>>> And no NO_SFDP_FLAGS() as discussed.
>>>
>>
>> Are there flavors of these flashes that don't support SFDP,
>> or why did you ask for NO_SFDP_FLAGS? Do each of these flashes
>> have SFDP and no-SFDP flavors?
> 
> I asked to drop NO_SFDP_FLAGS().
> 

Oh, yes, that's what I meant too. There was a previous comment
which said "PARSE_SFDP instead".

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

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

* Re: [PATCH v2 1/2] mtd: spi-nor: add Octal DTR support for Macronix flash
  2023-07-27 10:11     ` Michael Walle
@ 2023-07-28  9:46       ` Michael Walle
  2023-07-28 11:01         ` Tudor Ambarus
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Walle @ 2023-07-28  9:46 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: Tudor Ambarus, Jaime Liao, linux-mtd, pratyush, miquel.raynal,
	leoyu, JaimeLiao

Hi,

>>> +	/* Macronix SPI-NOR flash 8D-8D-8D read ID would get 6 bytes data 
>>> A-A-B-B-C-C */
>>> +	for (i = 0; i < nor->info->id_len; i++)
>>> +		if (buf[i * 2] != nor->info->id[i])
>>> +			return -EINVAL;
> 
> We need some kind of per flash rdid override. I guess rdid won't work
> in octal mode then. Are any other commands also affected?

Jaime was kind enough to send me some datasheets and it looks like
some commands will return the response bytes in STR mode. So,
technically, the correct thing to do here would be to use
8D-8D-8S mode. Other commands will also respond in STR mode, but
I guess we don't notice because most of them are just one byte.
But I think we should get this correct in the core.

-michael

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

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

* Re: [PATCH v2 1/2] mtd: spi-nor: add Octal DTR support for Macronix flash
  2023-07-28  9:46       ` Michael Walle
@ 2023-07-28 11:01         ` Tudor Ambarus
  0 siblings, 0 replies; 16+ messages in thread
From: Tudor Ambarus @ 2023-07-28 11:01 UTC (permalink / raw)
  To: Michael Walle
  Cc: Jaime Liao, linux-mtd, pratyush, miquel.raynal, leoyu, JaimeLiao



On 7/28/23 10:46, Michael Walle wrote:
> Hi,
> 
>>>> +    /* Macronix SPI-NOR flash 8D-8D-8D read ID would get 6 bytes data A-A-B-B-C-C */
>>>> +    for (i = 0; i < nor->info->id_len; i++)
>>>> +        if (buf[i * 2] != nor->info->id[i])
>>>> +            return -EINVAL;
>>
>> We need some kind of per flash rdid override. I guess rdid won't work
>> in octal mode then. Are any other commands also affected?
> 
> Jaime was kind enough to send me some datasheets and it looks like
> some commands will return the response bytes in STR mode. So,
> technically, the correct thing to do here would be to use
> 8D-8D-8S mode. Other commands will also respond in STR mode, but
> I guess we don't notice because most of them are just one byte.
> But I think we should get this correct in the core.
> 

happy to get patches for review!

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

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

* Re: [PATCH v2 1/2] mtd: spi-nor: add Octal DTR support for Macronix flash
  2023-07-27 10:00   ` Tudor Ambarus
  2023-07-27 10:04     ` Tudor Ambarus
  2023-07-27 10:11     ` Michael Walle
@ 2023-08-01  7:24     ` liao jaime
  2023-08-01  8:16       ` Tudor Ambarus
  2 siblings, 1 reply; 16+ messages in thread
From: liao jaime @ 2023-08-01  7:24 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: linux-mtd, pratyush, michael, miquel.raynal, leoyu, JaimeLiao

Hi Tudor

>
>
>
> On 7/27/23 10:16, 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.
>
> 20 dummy cycles is particular for a specific operating frequency.
>
> If you test the flash at different speeds, let's say at 30 MHz, and
> at their highest frequency (133, 200 MHZ?) does the flash work with
> current settings?
Yes, datasheet include "Dummy Cycle and Frequency Table" for checking.
For Macronix spi-nor flash, 20 or 18 dummy cycles should be use at 200MHz.
It alse means that DC 20 is suit for other frequency(<=200MHz).

> >
> > For enable Macronix flashes in Octal DTR mode, Configuration
> > Register2 DOPI bit should be set and DC bit setting for dummy
> > cycles.
>
> Use imperative.
I didn't get it.

> >
> > 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 04888258e891..b397fd274868 100644
> > --- a/drivers/mtd/spi-nor/macronix.c
> > +++ b/drivers/mtd/spi-nor/macronix.c
> > @@ -8,6 +8,22 @@
> >
> >  #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      /* For setting octal DTR mode */
> > +#define SPINOR_REG_MXIC_CR2_DC               0x00000300      /* 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_DC_20                0x0             /* Setting dummy cycles to 20 */
> > +#define SPINOR_REG_MXIC_ADDR_BYTES   4               /* Fixed R/W volatile address bytes to 4 */
> > +
> > +/* 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,9 +121,90 @@ 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, i;
> > +     int ret;
> > +
> > +     /* Use 20 dummy cycles for memory array reads. */
> > +     buf[0] = SPINOR_REG_MXIC_DC_20;
> > +     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;
> > +     }
> > +
> > +     /* Macronix SPI-NOR flash 8D-8D-8D read ID would get 6 bytes data A-A-B-B-C-C */
> > +     for (i = 0; i < nor->info->id_len; i++)
> > +             if (buf[i * 2] != nor->info->id[i])
> > +                     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_octal_dtr_enable(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;
> > +     nor->params->octal_dtr_enable = macronix_nor_octal_dtr_enable;
>
> this must be done in the late_init hook, I'd like to get rid of the
> default_init hook.
nor->params->quad_enable should be move in late_init or
nor->params->octal_dtr_enable only?

> >  }
> >
> >  static void macronix_nor_late_init(struct spi_nor *nor)

Thanks
Jaime

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

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

* Re: [PATCH v2 1/2] mtd: spi-nor: add Octal DTR support for Macronix flash
  2023-08-01  7:24     ` liao jaime
@ 2023-08-01  8:16       ` Tudor Ambarus
  2023-08-01  9:33         ` liao jaime
  0 siblings, 1 reply; 16+ messages in thread
From: Tudor Ambarus @ 2023-08-01  8:16 UTC (permalink / raw)
  To: liao jaime; +Cc: linux-mtd, pratyush, michael, miquel.raynal, leoyu, JaimeLiao



On 8/1/23 08:24, liao jaime wrote:
> Hi Tudor
> 
>>
>>
>>
>> On 7/27/23 10:16, 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.
>>
>> 20 dummy cycles is particular for a specific operating frequency.
>>
>> If you test the flash at different speeds, let's say at 30 MHz, and
>> at their highest frequency (133, 200 MHZ?) does the flash work with
>> current settings?
> Yes, datasheet include "Dummy Cycle and Frequency Table" for checking.
> For Macronix spi-nor flash, 20 or 18 dummy cycles should be use at 200MHz.
> It alse means that DC 20 is suit for other frequency(<=200MHz).
> 

You didn't answer my question. Does the flash work at 30 MHz by using 20
dummy cycles?
>>>
>>> For enable Macronix flashes in Octal DTR mode, Configuration
>>> Register2 DOPI bit should be set and DC bit setting for dummy
>>> cycles.
>>
>> Use imperative.
> I didn't get it.

Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
to do frotz", as if you are giving orders to the codebase to change
its behaviour. Please read:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html

> 
>>>
>>> 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 04888258e891..b397fd274868 100644
>>> --- a/drivers/mtd/spi-nor/macronix.c
>>> +++ b/drivers/mtd/spi-nor/macronix.c
>>> @@ -8,6 +8,22 @@
>>>
>>>  #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      /* For setting octal DTR mode */
>>> +#define SPINOR_REG_MXIC_CR2_DC               0x00000300      /* 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_DC_20                0x0             /* Setting dummy cycles to 20 */
>>> +#define SPINOR_REG_MXIC_ADDR_BYTES   4               /* Fixed R/W volatile address bytes to 4 */
>>> +
>>> +/* 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,9 +121,90 @@ 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, i;
>>> +     int ret;
>>> +
>>> +     /* Use 20 dummy cycles for memory array reads. */
>>> +     buf[0] = SPINOR_REG_MXIC_DC_20;
>>> +     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;
>>> +     }
>>> +
>>> +     /* Macronix SPI-NOR flash 8D-8D-8D read ID would get 6 bytes data A-A-B-B-C-C */
>>> +     for (i = 0; i < nor->info->id_len; i++)
>>> +             if (buf[i * 2] != nor->info->id[i])
>>> +                     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_octal_dtr_enable(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;
>>> +     nor->params->octal_dtr_enable = macronix_nor_octal_dtr_enable;
>>
>> this must be done in the late_init hook, I'd like to get rid of the
>> default_init hook.
> nor->params->quad_enable should be move in late_init or
> nor->params->octal_dtr_enable only?

quad_enable should be parsed from sfdp, thus we have to get rid of
setting quad_enable in a dedicated hook. Remove it entirely, but in a
dedicated patch. Of course, I expect you test the change and verify
that quad still works on all the affected flashes.

octal_dtr_enable should be set in late_init as we don't retrieve the
octal dtr enable from SFDP yet.
> 
>>>  }
>>>
>>>  static void macronix_nor_late_init(struct spi_nor *nor)
> 
> Thanks
> Jaime

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

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

* Re: [PATCH v2 1/2] mtd: spi-nor: add Octal DTR support for Macronix flash
  2023-08-01  8:16       ` Tudor Ambarus
@ 2023-08-01  9:33         ` liao jaime
  0 siblings, 0 replies; 16+ messages in thread
From: liao jaime @ 2023-08-01  9:33 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: linux-mtd, pratyush, michael, miquel.raynal, leoyu, JaimeLiao

Hi Tudor

>
>
>
> On 8/1/23 08:24, liao jaime wrote:
> > Hi Tudor
> >
> >>
> >>
> >>
> >> On 7/27/23 10:16, 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.
> >>
> >> 20 dummy cycles is particular for a specific operating frequency.
> >>
> >> If you test the flash at different speeds, let's say at 30 MHz, and
> >> at their highest frequency (133, 200 MHZ?) does the flash work with
> >> current settings?
> > Yes, datasheet include "Dummy Cycle and Frequency Table" for checking.
> > For Macronix spi-nor flash, 20 or 18 dummy cycles should be use at 200MHz.
> > It alse means that DC 20 is suit for other frequency(<=200MHz).
> >
>
> You didn't answer my question. Does the flash work at 30 MHz by using 20
> dummy cycles?
Yes.

> >>>
> >>> For enable Macronix flashes in Octal DTR mode, Configuration
> >>> Register2 DOPI bit should be set and DC bit setting for dummy
> >>> cycles.
> >>
> >> Use imperative.
> > I didn't get it.
>
> Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
> instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
> to do frotz", as if you are giving orders to the codebase to change
> its behaviour. Please read:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
Got it, thanks.

>
> >
> >>>
> >>> 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 04888258e891..b397fd274868 100644
> >>> --- a/drivers/mtd/spi-nor/macronix.c
> >>> +++ b/drivers/mtd/spi-nor/macronix.c
> >>> @@ -8,6 +8,22 @@
> >>>
> >>>  #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      /* For setting octal DTR mode */
> >>> +#define SPINOR_REG_MXIC_CR2_DC               0x00000300      /* 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_DC_20                0x0             /* Setting dummy cycles to 20 */
> >>> +#define SPINOR_REG_MXIC_ADDR_BYTES   4               /* Fixed R/W volatile address bytes to 4 */
> >>> +
> >>> +/* 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,9 +121,90 @@ 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, i;
> >>> +     int ret;
> >>> +
> >>> +     /* Use 20 dummy cycles for memory array reads. */
> >>> +     buf[0] = SPINOR_REG_MXIC_DC_20;
> >>> +     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;
> >>> +     }
> >>> +
> >>> +     /* Macronix SPI-NOR flash 8D-8D-8D read ID would get 6 bytes data A-A-B-B-C-C */
> >>> +     for (i = 0; i < nor->info->id_len; i++)
> >>> +             if (buf[i * 2] != nor->info->id[i])
> >>> +                     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_octal_dtr_enable(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;
> >>> +     nor->params->octal_dtr_enable = macronix_nor_octal_dtr_enable;
> >>
> >> this must be done in the late_init hook, I'd like to get rid of the
> >> default_init hook.
> > nor->params->quad_enable should be move in late_init or
> > nor->params->octal_dtr_enable only?
>
> quad_enable should be parsed from sfdp, thus we have to get rid of
> setting quad_enable in a dedicated hook. Remove it entirely, but in a
> dedicated patch. Of course, I expect you test the change and verify
> that quad still works on all the affected flashes.
>
> octal_dtr_enable should be set in late_init as we don't retrieve the
> octal dtr enable from SFDP yet.
Ok, I will move it to late_init.

> >
> >>>  }
> >>>
> >>>  static void macronix_nor_late_init(struct spi_nor *nor)
> >
> > Thanks
> > Jaime

Thanks
Jaime

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

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

end of thread, other threads:[~2023-08-01  9:55 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-27  9:16 [PATCH v2 0/2] Add octal DTR support for Macronix flash Jaime Liao
2023-07-27  9:16 ` [PATCH v2 1/2] mtd: spi-nor: add Octal " Jaime Liao
2023-07-27 10:00   ` Tudor Ambarus
2023-07-27 10:04     ` Tudor Ambarus
2023-07-27 10:11     ` Michael Walle
2023-07-28  9:46       ` Michael Walle
2023-07-28 11:01         ` Tudor Ambarus
2023-08-01  7:24     ` liao jaime
2023-08-01  8:16       ` Tudor Ambarus
2023-08-01  9:33         ` liao jaime
2023-07-27  9:16 ` [PATCH v2 2/2] mtd: spi-nor: add support for Macronix Octal flash Jaime Liao
2023-07-27  9:54   ` Tudor Ambarus
2023-07-27 10:04     ` Michael Walle
2023-07-27 10:10       ` Tudor Ambarus
2023-07-27 10:12         ` Michael Walle
2023-07-27 10:14           ` Tudor Ambarus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox