All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 1/2] mtd: spi-nor: core: Introduce SPI_NOR_SOFT_RESET flash_info fixup_flag
@ 2021-12-17 18:06 ` Tudor Ambarus
  0 siblings, 0 replies; 10+ messages in thread
From: Tudor Ambarus @ 2021-12-17 18:06 UTC (permalink / raw)
  To: p.yadav, michael
  Cc: miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel,
	nicolas.ferre, Tudor Ambarus

The Soft Reset and Rescue Sequence Support is defined in BFPT_DWORD(16)
starting with JESD216A. The first version of SFDP, JESD216 (April 2011),
defines just the first 9 BFPT DWORDS, thus it does not contain information
about the Software Reset and Rescue Support. Since this support can not
be discovered by parsing the first SFDP version, introduce a flash_info
fixup_flag that will be used either by flashes that define
JESD216 (April 2011) or by flashes that do not define SFDP at all.
In case a flash defines BFPT_DWORD(16) but with wrong values, one should
instead use a post_bfpt() hook and set SNOR_F_SOFT_RESET.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
---
v5: no changes
v4: no changes
v3: collect R-b
v2: no changes
 drivers/mtd/spi-nor/core.c | 3 +++
 drivers/mtd/spi-nor/core.h | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 2e21d5ac0e2d..32d80fdaa2a2 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2699,6 +2699,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor)
 
 	if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE)
 		nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
+
+	if (fixup_flags & SPI_NOR_SOFT_RESET)
+		nor->flags |= SNOR_F_SOFT_RESET;
 }
 
 /**
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 2afb610853a9..70c6bb7f5f04 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -373,6 +373,8 @@ struct spi_nor_fixups {
  *                            memory size above 128Mib.
  *   SPI_NOR_IO_MODE_EN_VOLATILE: flash enables the best available I/O mode
  *                            via a volatile bit.
+ *   SPI_NOR_SOFT_RESET:      flash supports software reset enable, reset
+ *                            sequence.
  * @mfr_flags:      manufacturer private flags. Used in the manufacturer fixup
  *                  hooks to differentiate support between flashes of the same
  *                  manufacturer.
@@ -416,6 +418,7 @@ struct flash_info {
 	u8 fixup_flags;
 #define SPI_NOR_4B_OPCODES		BIT(0)
 #define SPI_NOR_IO_MODE_EN_VOLATILE	BIT(1)
+#define SPI_NOR_SOFT_RESET		BIT(2)
 
 	u8 mfr_flags;
 
-- 
2.25.1


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

* [PATCH v5 1/2] mtd: spi-nor: core: Introduce SPI_NOR_SOFT_RESET flash_info fixup_flag
@ 2021-12-17 18:06 ` Tudor Ambarus
  0 siblings, 0 replies; 10+ messages in thread
From: Tudor Ambarus @ 2021-12-17 18:06 UTC (permalink / raw)
  To: p.yadav, michael
  Cc: miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel,
	nicolas.ferre, Tudor Ambarus

The Soft Reset and Rescue Sequence Support is defined in BFPT_DWORD(16)
starting with JESD216A. The first version of SFDP, JESD216 (April 2011),
defines just the first 9 BFPT DWORDS, thus it does not contain information
about the Software Reset and Rescue Support. Since this support can not
be discovered by parsing the first SFDP version, introduce a flash_info
fixup_flag that will be used either by flashes that define
JESD216 (April 2011) or by flashes that do not define SFDP at all.
In case a flash defines BFPT_DWORD(16) but with wrong values, one should
instead use a post_bfpt() hook and set SNOR_F_SOFT_RESET.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
---
v5: no changes
v4: no changes
v3: collect R-b
v2: no changes
 drivers/mtd/spi-nor/core.c | 3 +++
 drivers/mtd/spi-nor/core.h | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 2e21d5ac0e2d..32d80fdaa2a2 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2699,6 +2699,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor)
 
 	if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE)
 		nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
+
+	if (fixup_flags & SPI_NOR_SOFT_RESET)
+		nor->flags |= SNOR_F_SOFT_RESET;
 }
 
 /**
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 2afb610853a9..70c6bb7f5f04 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -373,6 +373,8 @@ struct spi_nor_fixups {
  *                            memory size above 128Mib.
  *   SPI_NOR_IO_MODE_EN_VOLATILE: flash enables the best available I/O mode
  *                            via a volatile bit.
+ *   SPI_NOR_SOFT_RESET:      flash supports software reset enable, reset
+ *                            sequence.
  * @mfr_flags:      manufacturer private flags. Used in the manufacturer fixup
  *                  hooks to differentiate support between flashes of the same
  *                  manufacturer.
@@ -416,6 +418,7 @@ struct flash_info {
 	u8 fixup_flags;
 #define SPI_NOR_4B_OPCODES		BIT(0)
 #define SPI_NOR_IO_MODE_EN_VOLATILE	BIT(1)
+#define SPI_NOR_SOFT_RESET		BIT(2)
 
 	u8 mfr_flags;
 
-- 
2.25.1


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

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

* [PATCH v5 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g
  2021-12-17 18:06 ` Tudor Ambarus
@ 2021-12-17 18:06   ` Tudor Ambarus
  -1 siblings, 0 replies; 10+ messages in thread
From: Tudor Ambarus @ 2021-12-17 18:06 UTC (permalink / raw)
  To: p.yadav, michael
  Cc: miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel,
	nicolas.ferre, Tudor Ambarus

mx66lm1g45g supports just 1-1-1, 8-8-8 and 8D-8D-8D modes. There are
versions of mx66lm1g45g which do not support SFDP, thus use
SPI_NOR_SKIP_SFDP. The RDID command issued through the octal peripheral
interface outputs data always in STR mode for whatever reason. Since
8D-8D-8S is not common, avoid reading the ID when enabling the octal dtr
mode. Instead, read back the CR2 to check if the switch was successful.
Tested in 1-1-1 and 8d-8d-8d modes using sama7g5 QSPI IP.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
v5: drop superfluous "buf" local variable in
spi_nor_macronix_read/write_cr2. buf was used only once, no need for a
dedicated local variable.

v4: macronix requires that undefined register addresses to keep their
value unchanged. The second byte of CR2 is not defined. Read the second
byte of CR2 before disabling Octal DTR mode, so that we don't modify its
value when disabling Octal DTR.

v3:
- drop setting of dummy cycles, use the default value
- avoid odd lengths in octal dtr mode
- s/8d-8d-8d/8D-8D-8D

v2: SPI_NOR_SOFT_RESET as a FIXUP_FLAG

# cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id
c2853b
# cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer
macronix
# cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/partname
mx66lm1g45g
# cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp
cat: can't open '/sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp': No such file or directory
 drivers/mtd/spi-nor/macronix.c | 107 +++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 67aaa83038b6..88b1218f19f4 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -32,6 +32,106 @@ static struct spi_nor_fixups mx25l25635_fixups = {
 	.post_bfpt = mx25l25635_post_bfpt_fixups,
 };
 
+#define SPINOR_OP_READ_CR2		0x71
+#define SPINOR_OP_WRITE_CR2		0x72
+#define SPINOR_OP_MX_DTR_RD		0xee
+
+#define SPINOR_REG_CR2_MODE_ADDR	0
+#define SPINOR_REG_CR2_DTR_OPI_ENABLE	BIT(1)
+#define SPINOR_REG_CR2_SPI		0
+
+static int spi_nor_macronix_read_cr2(struct spi_nor *nor, bool octal_dtr)
+{
+	struct spi_mem_op op =
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_READ_CR2, 1),
+			   SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1),
+			   SPI_MEM_OP_DUMMY(octal_dtr ? 4 : 0, 1),
+			   SPI_MEM_OP_DATA_IN(octal_dtr ? 2 : 1, nor->bouncebuf, 1));
+
+	if (octal_dtr)
+		spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
+	return spi_mem_exec_op(nor->spimem, &op);
+}
+
+static int spi_nor_macronix_write_cr2(struct spi_nor *nor, bool octal_dtr)
+{
+	struct spi_mem_op op =
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRITE_CR2, 1),
+			   SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1),
+			   SPI_MEM_OP_NO_DUMMY,
+			   SPI_MEM_OP_DATA_OUT(octal_dtr ? 2 : 1, nor->bouncebuf, 1));
+	int ret;
+
+	if (octal_dtr)
+		spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
+
+	ret = spi_nor_write_enable(nor);
+	if (ret)
+		return ret;
+	return spi_mem_exec_op(nor->spimem, &op);
+}
+
+static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor, bool enable)
+{
+	u8 *buf = nor->bouncebuf;
+	int ret;
+
+	/* Set/unset the octal and DTR enable bits. */
+	if (enable) {
+		buf[0] = SPINOR_REG_CR2_DTR_OPI_ENABLE;
+	} else {
+		/*
+		 * One byte transactions are not allowed in 8D-8D-8D mode.
+		 * mx66lm1g45g requires that undefined register addresses to
+		 * keep their value unchanged. Its second CR2 byte value is not
+		 * defined. Read the second byte value of CR2 so that we can
+		 * write it back when disabling Octal DTR mode.
+		 */
+		ret = spi_nor_macronix_read_cr2(nor, true);
+		if (ret)
+			return ret;
+		buf[0] = SPINOR_REG_CR2_SPI;
+	}
+	ret = spi_nor_macronix_write_cr2(nor, !enable);
+	if (ret)
+		return ret;
+
+	/* Read back CR2 to make sure the switch was successful. */
+	ret = spi_nor_macronix_read_cr2(nor, enable);
+	if (ret)
+		return ret;
+	if (enable) {
+		if (buf[0] != SPINOR_REG_CR2_DTR_OPI_ENABLE) {
+			dev_dbg(nor->dev, "Failed to enable 8D-8D-8D mode.\n");
+			return -EINVAL;
+		}
+	} else if (buf[0] != SPINOR_REG_CR2_SPI) {
+		dev_dbg(nor->dev, "Failed to disable 8D-8D-8D mode.\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void mx66lm1g45g_late_init(struct spi_nor *nor)
+{
+	nor->params->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable;
+
+	/* Set the Fast Read settings. */
+	nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR;
+	spi_nor_set_read_settings(&nor->params->reads[SNOR_CMD_READ_8_8_8_DTR],
+				  0, 20, SPINOR_OP_MX_DTR_RD,
+				  SNOR_PROTO_8_8_8_DTR);
+
+	nor->cmd_ext_type = SPI_NOR_EXT_INVERT;
+	nor->params->rdsr_dummy = 4;
+	nor->params->rdsr_addr_nbytes = 4;
+}
+
+static struct spi_nor_fixups mx66lm1g45g_fixups = {
+	.late_init = mx66lm1g45g_late_init,
+};
+
 static const struct flash_info macronix_parts[] = {
 	/* Macronix */
 	{ "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1)
@@ -100,6 +200,13 @@ static const struct flash_info macronix_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) },
+	{ "mx66lm1g45g", INFO(0xc2853b, 0, 64 * 1024, 2048)
+		NO_SFDP_FLAGS(SPI_NOR_SKIP_SFDP | SECT_4K |
+			      SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES | SPI_NOR_IO_MODE_EN_VOLATILE |
+			    SPI_NOR_SOFT_RESET)
+		.fixups = &mx66lm1g45g_fixups,
+	},
 };
 
 static void macronix_default_init(struct spi_nor *nor)
-- 
2.25.1


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

* [PATCH v5 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g
@ 2021-12-17 18:06   ` Tudor Ambarus
  0 siblings, 0 replies; 10+ messages in thread
From: Tudor Ambarus @ 2021-12-17 18:06 UTC (permalink / raw)
  To: p.yadav, michael
  Cc: miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel,
	nicolas.ferre, Tudor Ambarus

mx66lm1g45g supports just 1-1-1, 8-8-8 and 8D-8D-8D modes. There are
versions of mx66lm1g45g which do not support SFDP, thus use
SPI_NOR_SKIP_SFDP. The RDID command issued through the octal peripheral
interface outputs data always in STR mode for whatever reason. Since
8D-8D-8S is not common, avoid reading the ID when enabling the octal dtr
mode. Instead, read back the CR2 to check if the switch was successful.
Tested in 1-1-1 and 8d-8d-8d modes using sama7g5 QSPI IP.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
v5: drop superfluous "buf" local variable in
spi_nor_macronix_read/write_cr2. buf was used only once, no need for a
dedicated local variable.

v4: macronix requires that undefined register addresses to keep their
value unchanged. The second byte of CR2 is not defined. Read the second
byte of CR2 before disabling Octal DTR mode, so that we don't modify its
value when disabling Octal DTR.

v3:
- drop setting of dummy cycles, use the default value
- avoid odd lengths in octal dtr mode
- s/8d-8d-8d/8D-8D-8D

v2: SPI_NOR_SOFT_RESET as a FIXUP_FLAG

# cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id
c2853b
# cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer
macronix
# cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/partname
mx66lm1g45g
# cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp
cat: can't open '/sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp': No such file or directory
 drivers/mtd/spi-nor/macronix.c | 107 +++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 67aaa83038b6..88b1218f19f4 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -32,6 +32,106 @@ static struct spi_nor_fixups mx25l25635_fixups = {
 	.post_bfpt = mx25l25635_post_bfpt_fixups,
 };
 
+#define SPINOR_OP_READ_CR2		0x71
+#define SPINOR_OP_WRITE_CR2		0x72
+#define SPINOR_OP_MX_DTR_RD		0xee
+
+#define SPINOR_REG_CR2_MODE_ADDR	0
+#define SPINOR_REG_CR2_DTR_OPI_ENABLE	BIT(1)
+#define SPINOR_REG_CR2_SPI		0
+
+static int spi_nor_macronix_read_cr2(struct spi_nor *nor, bool octal_dtr)
+{
+	struct spi_mem_op op =
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_READ_CR2, 1),
+			   SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1),
+			   SPI_MEM_OP_DUMMY(octal_dtr ? 4 : 0, 1),
+			   SPI_MEM_OP_DATA_IN(octal_dtr ? 2 : 1, nor->bouncebuf, 1));
+
+	if (octal_dtr)
+		spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
+	return spi_mem_exec_op(nor->spimem, &op);
+}
+
+static int spi_nor_macronix_write_cr2(struct spi_nor *nor, bool octal_dtr)
+{
+	struct spi_mem_op op =
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRITE_CR2, 1),
+			   SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1),
+			   SPI_MEM_OP_NO_DUMMY,
+			   SPI_MEM_OP_DATA_OUT(octal_dtr ? 2 : 1, nor->bouncebuf, 1));
+	int ret;
+
+	if (octal_dtr)
+		spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
+
+	ret = spi_nor_write_enable(nor);
+	if (ret)
+		return ret;
+	return spi_mem_exec_op(nor->spimem, &op);
+}
+
+static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor, bool enable)
+{
+	u8 *buf = nor->bouncebuf;
+	int ret;
+
+	/* Set/unset the octal and DTR enable bits. */
+	if (enable) {
+		buf[0] = SPINOR_REG_CR2_DTR_OPI_ENABLE;
+	} else {
+		/*
+		 * One byte transactions are not allowed in 8D-8D-8D mode.
+		 * mx66lm1g45g requires that undefined register addresses to
+		 * keep their value unchanged. Its second CR2 byte value is not
+		 * defined. Read the second byte value of CR2 so that we can
+		 * write it back when disabling Octal DTR mode.
+		 */
+		ret = spi_nor_macronix_read_cr2(nor, true);
+		if (ret)
+			return ret;
+		buf[0] = SPINOR_REG_CR2_SPI;
+	}
+	ret = spi_nor_macronix_write_cr2(nor, !enable);
+	if (ret)
+		return ret;
+
+	/* Read back CR2 to make sure the switch was successful. */
+	ret = spi_nor_macronix_read_cr2(nor, enable);
+	if (ret)
+		return ret;
+	if (enable) {
+		if (buf[0] != SPINOR_REG_CR2_DTR_OPI_ENABLE) {
+			dev_dbg(nor->dev, "Failed to enable 8D-8D-8D mode.\n");
+			return -EINVAL;
+		}
+	} else if (buf[0] != SPINOR_REG_CR2_SPI) {
+		dev_dbg(nor->dev, "Failed to disable 8D-8D-8D mode.\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void mx66lm1g45g_late_init(struct spi_nor *nor)
+{
+	nor->params->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable;
+
+	/* Set the Fast Read settings. */
+	nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR;
+	spi_nor_set_read_settings(&nor->params->reads[SNOR_CMD_READ_8_8_8_DTR],
+				  0, 20, SPINOR_OP_MX_DTR_RD,
+				  SNOR_PROTO_8_8_8_DTR);
+
+	nor->cmd_ext_type = SPI_NOR_EXT_INVERT;
+	nor->params->rdsr_dummy = 4;
+	nor->params->rdsr_addr_nbytes = 4;
+}
+
+static struct spi_nor_fixups mx66lm1g45g_fixups = {
+	.late_init = mx66lm1g45g_late_init,
+};
+
 static const struct flash_info macronix_parts[] = {
 	/* Macronix */
 	{ "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1)
@@ -100,6 +200,13 @@ static const struct flash_info macronix_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) },
+	{ "mx66lm1g45g", INFO(0xc2853b, 0, 64 * 1024, 2048)
+		NO_SFDP_FLAGS(SPI_NOR_SKIP_SFDP | SECT_4K |
+			      SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES | SPI_NOR_IO_MODE_EN_VOLATILE |
+			    SPI_NOR_SOFT_RESET)
+		.fixups = &mx66lm1g45g_fixups,
+	},
 };
 
 static void macronix_default_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] 10+ messages in thread

* Re: [PATCH v5 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g
  2021-12-17 18:06   ` Tudor Ambarus
@ 2021-12-20 10:25     ` Pratyush Yadav
  -1 siblings, 0 replies; 10+ messages in thread
From: Pratyush Yadav @ 2021-12-20 10:25 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: michael, miquel.raynal, richard, vigneshr, linux-mtd,
	linux-kernel, nicolas.ferre

Hi Tudor,

On 17/12/21 08:06PM, Tudor Ambarus wrote:
> mx66lm1g45g supports just 1-1-1, 8-8-8 and 8D-8D-8D modes. There are
> versions of mx66lm1g45g which do not support SFDP, thus use
> SPI_NOR_SKIP_SFDP. The RDID command issued through the octal peripheral
> interface outputs data always in STR mode for whatever reason. Since
> 8D-8D-8S is not common, avoid reading the ID when enabling the octal dtr
> mode. Instead, read back the CR2 to check if the switch was successful.

I replied to your v2 just now about this.

> Tested in 1-1-1 and 8d-8d-8d modes using sama7g5 QSPI IP.

Link to datasheet in the commit message would be nice.

As discussed on IRC, this flash reverses byte order in 8D-8D-8D mode. So 
the data you write in 1S-1S-1S mode will be have byte order reversed 
when reading in 8D-8D-8D mode. Do you have any plans on doing something 
for this? Or do we just leave it to the user to figure it out?

> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> v5: drop superfluous "buf" local variable in
> spi_nor_macronix_read/write_cr2. buf was used only once, no need for a
> dedicated local variable.
> 
> v4: macronix requires that undefined register addresses to keep their
> value unchanged. The second byte of CR2 is not defined. Read the second
> byte of CR2 before disabling Octal DTR mode, so that we don't modify its
> value when disabling Octal DTR.
> 
> v3:
> - drop setting of dummy cycles, use the default value
> - avoid odd lengths in octal dtr mode
> - s/8d-8d-8d/8D-8D-8D
> 
> v2: SPI_NOR_SOFT_RESET as a FIXUP_FLAG
> 
> # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id
> c2853b
> # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer
> macronix
> # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/partname
> mx66lm1g45g
> # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp
> cat: can't open '/sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp': No such file or directory
>  drivers/mtd/spi-nor/macronix.c | 107 +++++++++++++++++++++++++++++++++
>  1 file changed, 107 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 67aaa83038b6..88b1218f19f4 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -32,6 +32,106 @@ static struct spi_nor_fixups mx25l25635_fixups = {
>  	.post_bfpt = mx25l25635_post_bfpt_fixups,
>  };
>  
> +#define SPINOR_OP_READ_CR2		0x71
> +#define SPINOR_OP_WRITE_CR2		0x72
> +#define SPINOR_OP_MX_DTR_RD		0xee
> +
> +#define SPINOR_REG_CR2_MODE_ADDR	0
> +#define SPINOR_REG_CR2_DTR_OPI_ENABLE	BIT(1)
> +#define SPINOR_REG_CR2_SPI		0
> +
> +static int spi_nor_macronix_read_cr2(struct spi_nor *nor, bool octal_dtr)
> +{
> +	struct spi_mem_op op =
> +		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_READ_CR2, 1),
> +			   SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1),
> +			   SPI_MEM_OP_DUMMY(octal_dtr ? 4 : 0, 1),
> +			   SPI_MEM_OP_DATA_IN(octal_dtr ? 2 : 1, nor->bouncebuf, 1));

Set buswidths to 0 and call spi_nor_spimem_setup_op() for both 1S-1S-1S 
and 8D-8D-8D.

> +
> +	if (octal_dtr)
> +		spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
> +	return spi_mem_exec_op(nor->spimem, &op);
> +}
> +
> +static int spi_nor_macronix_write_cr2(struct spi_nor *nor, bool octal_dtr)
> +{
> +	struct spi_mem_op op =
> +		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRITE_CR2, 1),
> +			   SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1),
> +			   SPI_MEM_OP_NO_DUMMY,
> +			   SPI_MEM_OP_DATA_OUT(octal_dtr ? 2 : 1, nor->bouncebuf, 1));
> +	int ret;
> +
> +	if (octal_dtr)
> +		spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);

Same as above.

> +
> +	ret = spi_nor_write_enable(nor);
> +	if (ret)
> +		return ret;
> +	return spi_mem_exec_op(nor->spimem, &op);
> +}
> +
> +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor, bool enable)
> +{
> +	u8 *buf = nor->bouncebuf;
> +	int ret;
> +
> +	/* Set/unset the octal and DTR enable bits. */
> +	if (enable) {
> +		buf[0] = SPINOR_REG_CR2_DTR_OPI_ENABLE;
> +	} else {
> +		/*
> +		 * One byte transactions are not allowed in 8D-8D-8D mode.
> +		 * mx66lm1g45g requires that undefined register addresses to
> +		 * keep their value unchanged. Its second CR2 byte value is not
> +		 * defined. Read the second byte value of CR2 so that we can
> +		 * write it back when disabling Octal DTR mode.
> +		 */
> +		ret = spi_nor_macronix_read_cr2(nor, true);
> +		if (ret)
> +			return ret;
> +		buf[0] = SPINOR_REG_CR2_SPI;

The fact that spi_nor_macronix_read_cr2() fills buf[1] is not very 
obvious. Can you pass buf as a argument to make this relationship a bit 
more clear?

> +	}
> +	ret = spi_nor_macronix_write_cr2(nor, !enable);
> +	if (ret)
> +		return ret;
> +
> +	/* Read back CR2 to make sure the switch was successful. */
> +	ret = spi_nor_macronix_read_cr2(nor, enable);
> +	if (ret)
> +		return ret;
> +	if (enable) {
> +		if (buf[0] != SPINOR_REG_CR2_DTR_OPI_ENABLE) {
> +			dev_dbg(nor->dev, "Failed to enable 8D-8D-8D mode.\n");
> +			return -EINVAL;
> +		}
> +	} else if (buf[0] != SPINOR_REG_CR2_SPI) {
> +		dev_dbg(nor->dev, "Failed to disable 8D-8D-8D mode.\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static void mx66lm1g45g_late_init(struct spi_nor *nor)
> +{
> +	nor->params->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable;
> +
> +	/* Set the Fast Read settings. */
> +	nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR;
> +	spi_nor_set_read_settings(&nor->params->reads[SNOR_CMD_READ_8_8_8_DTR],
> +				  0, 20, SPINOR_OP_MX_DTR_RD,
> +				  SNOR_PROTO_8_8_8_DTR);
> +
> +	nor->cmd_ext_type = SPI_NOR_EXT_INVERT;
> +	nor->params->rdsr_dummy = 4;
> +	nor->params->rdsr_addr_nbytes = 4;
> +}
> +
> +static struct spi_nor_fixups mx66lm1g45g_fixups = {
> +	.late_init = mx66lm1g45g_late_init,
> +};
> +
>  static const struct flash_info macronix_parts[] = {
>  	/* Macronix */
>  	{ "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1)
> @@ -100,6 +200,13 @@ static const struct flash_info macronix_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) },
> +	{ "mx66lm1g45g", INFO(0xc2853b, 0, 64 * 1024, 2048)
> +		NO_SFDP_FLAGS(SPI_NOR_SKIP_SFDP | SECT_4K |
> +			      SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP)
> +		FIXUP_FLAGS(SPI_NOR_4B_OPCODES | SPI_NOR_IO_MODE_EN_VOLATILE |
> +			    SPI_NOR_SOFT_RESET)
> +		.fixups = &mx66lm1g45g_fixups,
> +	},
>  };
>  
>  static void macronix_default_init(struct spi_nor *nor)
> -- 
> 2.25.1
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

* Re: [PATCH v5 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g
@ 2021-12-20 10:25     ` Pratyush Yadav
  0 siblings, 0 replies; 10+ messages in thread
From: Pratyush Yadav @ 2021-12-20 10:25 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: michael, miquel.raynal, richard, vigneshr, linux-mtd,
	linux-kernel, nicolas.ferre

Hi Tudor,

On 17/12/21 08:06PM, Tudor Ambarus wrote:
> mx66lm1g45g supports just 1-1-1, 8-8-8 and 8D-8D-8D modes. There are
> versions of mx66lm1g45g which do not support SFDP, thus use
> SPI_NOR_SKIP_SFDP. The RDID command issued through the octal peripheral
> interface outputs data always in STR mode for whatever reason. Since
> 8D-8D-8S is not common, avoid reading the ID when enabling the octal dtr
> mode. Instead, read back the CR2 to check if the switch was successful.

I replied to your v2 just now about this.

> Tested in 1-1-1 and 8d-8d-8d modes using sama7g5 QSPI IP.

Link to datasheet in the commit message would be nice.

As discussed on IRC, this flash reverses byte order in 8D-8D-8D mode. So 
the data you write in 1S-1S-1S mode will be have byte order reversed 
when reading in 8D-8D-8D mode. Do you have any plans on doing something 
for this? Or do we just leave it to the user to figure it out?

> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> v5: drop superfluous "buf" local variable in
> spi_nor_macronix_read/write_cr2. buf was used only once, no need for a
> dedicated local variable.
> 
> v4: macronix requires that undefined register addresses to keep their
> value unchanged. The second byte of CR2 is not defined. Read the second
> byte of CR2 before disabling Octal DTR mode, so that we don't modify its
> value when disabling Octal DTR.
> 
> v3:
> - drop setting of dummy cycles, use the default value
> - avoid odd lengths in octal dtr mode
> - s/8d-8d-8d/8D-8D-8D
> 
> v2: SPI_NOR_SOFT_RESET as a FIXUP_FLAG
> 
> # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id
> c2853b
> # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer
> macronix
> # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/partname
> mx66lm1g45g
> # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp
> cat: can't open '/sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp': No such file or directory
>  drivers/mtd/spi-nor/macronix.c | 107 +++++++++++++++++++++++++++++++++
>  1 file changed, 107 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 67aaa83038b6..88b1218f19f4 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -32,6 +32,106 @@ static struct spi_nor_fixups mx25l25635_fixups = {
>  	.post_bfpt = mx25l25635_post_bfpt_fixups,
>  };
>  
> +#define SPINOR_OP_READ_CR2		0x71
> +#define SPINOR_OP_WRITE_CR2		0x72
> +#define SPINOR_OP_MX_DTR_RD		0xee
> +
> +#define SPINOR_REG_CR2_MODE_ADDR	0
> +#define SPINOR_REG_CR2_DTR_OPI_ENABLE	BIT(1)
> +#define SPINOR_REG_CR2_SPI		0
> +
> +static int spi_nor_macronix_read_cr2(struct spi_nor *nor, bool octal_dtr)
> +{
> +	struct spi_mem_op op =
> +		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_READ_CR2, 1),
> +			   SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1),
> +			   SPI_MEM_OP_DUMMY(octal_dtr ? 4 : 0, 1),
> +			   SPI_MEM_OP_DATA_IN(octal_dtr ? 2 : 1, nor->bouncebuf, 1));

Set buswidths to 0 and call spi_nor_spimem_setup_op() for both 1S-1S-1S 
and 8D-8D-8D.

> +
> +	if (octal_dtr)
> +		spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
> +	return spi_mem_exec_op(nor->spimem, &op);
> +}
> +
> +static int spi_nor_macronix_write_cr2(struct spi_nor *nor, bool octal_dtr)
> +{
> +	struct spi_mem_op op =
> +		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRITE_CR2, 1),
> +			   SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1),
> +			   SPI_MEM_OP_NO_DUMMY,
> +			   SPI_MEM_OP_DATA_OUT(octal_dtr ? 2 : 1, nor->bouncebuf, 1));
> +	int ret;
> +
> +	if (octal_dtr)
> +		spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);

Same as above.

> +
> +	ret = spi_nor_write_enable(nor);
> +	if (ret)
> +		return ret;
> +	return spi_mem_exec_op(nor->spimem, &op);
> +}
> +
> +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor, bool enable)
> +{
> +	u8 *buf = nor->bouncebuf;
> +	int ret;
> +
> +	/* Set/unset the octal and DTR enable bits. */
> +	if (enable) {
> +		buf[0] = SPINOR_REG_CR2_DTR_OPI_ENABLE;
> +	} else {
> +		/*
> +		 * One byte transactions are not allowed in 8D-8D-8D mode.
> +		 * mx66lm1g45g requires that undefined register addresses to
> +		 * keep their value unchanged. Its second CR2 byte value is not
> +		 * defined. Read the second byte value of CR2 so that we can
> +		 * write it back when disabling Octal DTR mode.
> +		 */
> +		ret = spi_nor_macronix_read_cr2(nor, true);
> +		if (ret)
> +			return ret;
> +		buf[0] = SPINOR_REG_CR2_SPI;

The fact that spi_nor_macronix_read_cr2() fills buf[1] is not very 
obvious. Can you pass buf as a argument to make this relationship a bit 
more clear?

> +	}
> +	ret = spi_nor_macronix_write_cr2(nor, !enable);
> +	if (ret)
> +		return ret;
> +
> +	/* Read back CR2 to make sure the switch was successful. */
> +	ret = spi_nor_macronix_read_cr2(nor, enable);
> +	if (ret)
> +		return ret;
> +	if (enable) {
> +		if (buf[0] != SPINOR_REG_CR2_DTR_OPI_ENABLE) {
> +			dev_dbg(nor->dev, "Failed to enable 8D-8D-8D mode.\n");
> +			return -EINVAL;
> +		}
> +	} else if (buf[0] != SPINOR_REG_CR2_SPI) {
> +		dev_dbg(nor->dev, "Failed to disable 8D-8D-8D mode.\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static void mx66lm1g45g_late_init(struct spi_nor *nor)
> +{
> +	nor->params->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable;
> +
> +	/* Set the Fast Read settings. */
> +	nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR;
> +	spi_nor_set_read_settings(&nor->params->reads[SNOR_CMD_READ_8_8_8_DTR],
> +				  0, 20, SPINOR_OP_MX_DTR_RD,
> +				  SNOR_PROTO_8_8_8_DTR);
> +
> +	nor->cmd_ext_type = SPI_NOR_EXT_INVERT;
> +	nor->params->rdsr_dummy = 4;
> +	nor->params->rdsr_addr_nbytes = 4;
> +}
> +
> +static struct spi_nor_fixups mx66lm1g45g_fixups = {
> +	.late_init = mx66lm1g45g_late_init,
> +};
> +
>  static const struct flash_info macronix_parts[] = {
>  	/* Macronix */
>  	{ "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1)
> @@ -100,6 +200,13 @@ static const struct flash_info macronix_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) },
> +	{ "mx66lm1g45g", INFO(0xc2853b, 0, 64 * 1024, 2048)
> +		NO_SFDP_FLAGS(SPI_NOR_SKIP_SFDP | SECT_4K |
> +			      SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP)
> +		FIXUP_FLAGS(SPI_NOR_4B_OPCODES | SPI_NOR_IO_MODE_EN_VOLATILE |
> +			    SPI_NOR_SOFT_RESET)
> +		.fixups = &mx66lm1g45g_fixups,
> +	},
>  };
>  
>  static void macronix_default_init(struct spi_nor *nor)
> -- 
> 2.25.1
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

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

* Re: [PATCH v5 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g
  2021-12-20 10:25     ` Pratyush Yadav
@ 2021-12-20 11:10       ` Tudor.Ambarus
  -1 siblings, 0 replies; 10+ messages in thread
From: Tudor.Ambarus @ 2021-12-20 11:10 UTC (permalink / raw)
  To: p.yadav
  Cc: michael, miquel.raynal, richard, vigneshr, linux-mtd,
	linux-kernel, Nicolas.Ferre

On 12/20/21 12:25 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor,
> 
> On 17/12/21 08:06PM, Tudor Ambarus wrote:
>> mx66lm1g45g supports just 1-1-1, 8-8-8 and 8D-8D-8D modes. There are
>> versions of mx66lm1g45g which do not support SFDP, thus use
>> SPI_NOR_SKIP_SFDP. The RDID command issued through the octal peripheral
>> interface outputs data always in STR mode for whatever reason. Since
>> 8D-8D-8S is not common, avoid reading the ID when enabling the octal dtr
>> mode. Instead, read back the CR2 to check if the switch was successful.
> 
> I replied to your v2 just now about this.
> 
>> Tested in 1-1-1 and 8d-8d-8d modes using sama7g5 QSPI IP.
> 
> Link to datasheet in the commit message would be nice.

Do you know if there's a standardized way to add a link to a datasheet
in the commit message, i.e. should I use the Link tag or just a simple
link will do?
> 
> As discussed on IRC, this flash reverses byte order in 8D-8D-8D mode. So
> the data you write in 1S-1S-1S mode will be have byte order reversed
> when reading in 8D-8D-8D mode. Do you have any plans on doing something
> for this? Or do we just leave it to the user to figure it out?

I don't think we should amend this in software. Reading/writing in
8D-8D-8D will give sane results, the problem is just when you
use STR for write and DTR for read, or viceversa. This is just a bad
design and we should leave it as it is.

I'll address all your other comments in a v6. Thanks.
ta

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

* Re: [PATCH v5 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g
@ 2021-12-20 11:10       ` Tudor.Ambarus
  0 siblings, 0 replies; 10+ messages in thread
From: Tudor.Ambarus @ 2021-12-20 11:10 UTC (permalink / raw)
  To: p.yadav
  Cc: michael, miquel.raynal, richard, vigneshr, linux-mtd,
	linux-kernel, Nicolas.Ferre

On 12/20/21 12:25 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor,
> 
> On 17/12/21 08:06PM, Tudor Ambarus wrote:
>> mx66lm1g45g supports just 1-1-1, 8-8-8 and 8D-8D-8D modes. There are
>> versions of mx66lm1g45g which do not support SFDP, thus use
>> SPI_NOR_SKIP_SFDP. The RDID command issued through the octal peripheral
>> interface outputs data always in STR mode for whatever reason. Since
>> 8D-8D-8S is not common, avoid reading the ID when enabling the octal dtr
>> mode. Instead, read back the CR2 to check if the switch was successful.
> 
> I replied to your v2 just now about this.
> 
>> Tested in 1-1-1 and 8d-8d-8d modes using sama7g5 QSPI IP.
> 
> Link to datasheet in the commit message would be nice.

Do you know if there's a standardized way to add a link to a datasheet
in the commit message, i.e. should I use the Link tag or just a simple
link will do?
> 
> As discussed on IRC, this flash reverses byte order in 8D-8D-8D mode. So
> the data you write in 1S-1S-1S mode will be have byte order reversed
> when reading in 8D-8D-8D mode. Do you have any plans on doing something
> for this? Or do we just leave it to the user to figure it out?

I don't think we should amend this in software. Reading/writing in
8D-8D-8D will give sane results, the problem is just when you
use STR for write and DTR for read, or viceversa. This is just a bad
design and we should leave it as it is.

I'll address all your other comments in a v6. Thanks.
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v5 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g
  2021-12-20 11:10       ` Tudor.Ambarus
@ 2021-12-20 11:18         ` Pratyush Yadav
  -1 siblings, 0 replies; 10+ messages in thread
From: Pratyush Yadav @ 2021-12-20 11:18 UTC (permalink / raw)
  To: Tudor.Ambarus
  Cc: michael, miquel.raynal, richard, vigneshr, linux-mtd,
	linux-kernel, Nicolas.Ferre

On 20/12/21 11:10AM, Tudor.Ambarus@microchip.com wrote:
> On 12/20/21 12:25 PM, Pratyush Yadav wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Hi Tudor,
> > 
> > On 17/12/21 08:06PM, Tudor Ambarus wrote:
> >> mx66lm1g45g supports just 1-1-1, 8-8-8 and 8D-8D-8D modes. There are
> >> versions of mx66lm1g45g which do not support SFDP, thus use
> >> SPI_NOR_SKIP_SFDP. The RDID command issued through the octal peripheral
> >> interface outputs data always in STR mode for whatever reason. Since
> >> 8D-8D-8S is not common, avoid reading the ID when enabling the octal dtr
> >> mode. Instead, read back the CR2 to check if the switch was successful.
> > 
> > I replied to your v2 just now about this.
> > 
> >> Tested in 1-1-1 and 8d-8d-8d modes using sama7g5 QSPI IP.
> > 
> > Link to datasheet in the commit message would be nice.
> 
> Do you know if there's a standardized way to add a link to a datasheet
> in the commit message, i.e. should I use the Link tag or just a simple
> link will do?

I don't know if there is. Something like "Datasheet: <link>" should be 
fine I think.

> > 
> > As discussed on IRC, this flash reverses byte order in 8D-8D-8D mode. So
> > the data you write in 1S-1S-1S mode will be have byte order reversed
> > when reading in 8D-8D-8D mode. Do you have any plans on doing something
> > for this? Or do we just leave it to the user to figure it out?
> 
> I don't think we should amend this in software. Reading/writing in
> 8D-8D-8D will give sane results, the problem is just when you
> use STR for write and DTR for read, or viceversa. This is just a bad
> design and we should leave it as it is.

Sounds good to me.

> 
> I'll address all your other comments in a v6. Thanks.
> ta

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

* Re: [PATCH v5 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g
@ 2021-12-20 11:18         ` Pratyush Yadav
  0 siblings, 0 replies; 10+ messages in thread
From: Pratyush Yadav @ 2021-12-20 11:18 UTC (permalink / raw)
  To: Tudor.Ambarus
  Cc: michael, miquel.raynal, richard, vigneshr, linux-mtd,
	linux-kernel, Nicolas.Ferre

On 20/12/21 11:10AM, Tudor.Ambarus@microchip.com wrote:
> On 12/20/21 12:25 PM, Pratyush Yadav wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Hi Tudor,
> > 
> > On 17/12/21 08:06PM, Tudor Ambarus wrote:
> >> mx66lm1g45g supports just 1-1-1, 8-8-8 and 8D-8D-8D modes. There are
> >> versions of mx66lm1g45g which do not support SFDP, thus use
> >> SPI_NOR_SKIP_SFDP. The RDID command issued through the octal peripheral
> >> interface outputs data always in STR mode for whatever reason. Since
> >> 8D-8D-8S is not common, avoid reading the ID when enabling the octal dtr
> >> mode. Instead, read back the CR2 to check if the switch was successful.
> > 
> > I replied to your v2 just now about this.
> > 
> >> Tested in 1-1-1 and 8d-8d-8d modes using sama7g5 QSPI IP.
> > 
> > Link to datasheet in the commit message would be nice.
> 
> Do you know if there's a standardized way to add a link to a datasheet
> in the commit message, i.e. should I use the Link tag or just a simple
> link will do?

I don't know if there is. Something like "Datasheet: <link>" should be 
fine I think.

> > 
> > As discussed on IRC, this flash reverses byte order in 8D-8D-8D mode. So
> > the data you write in 1S-1S-1S mode will be have byte order reversed
> > when reading in 8D-8D-8D mode. Do you have any plans on doing something
> > for this? Or do we just leave it to the user to figure it out?
> 
> I don't think we should amend this in software. Reading/writing in
> 8D-8D-8D will give sane results, the problem is just when you
> use STR for write and DTR for read, or viceversa. This is just a bad
> design and we should leave it as it is.

Sounds good to me.

> 
> I'll address all your other comments in a v6. Thanks.
> ta

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

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

end of thread, other threads:[~2021-12-20 11:26 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-17 18:06 [PATCH v5 1/2] mtd: spi-nor: core: Introduce SPI_NOR_SOFT_RESET flash_info fixup_flag Tudor Ambarus
2021-12-17 18:06 ` Tudor Ambarus
2021-12-17 18:06 ` [PATCH v5 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g Tudor Ambarus
2021-12-17 18:06   ` Tudor Ambarus
2021-12-20 10:25   ` Pratyush Yadav
2021-12-20 10:25     ` Pratyush Yadav
2021-12-20 11:10     ` Tudor.Ambarus
2021-12-20 11:10       ` Tudor.Ambarus
2021-12-20 11:18       ` Pratyush Yadav
2021-12-20 11:18         ` Pratyush Yadav

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.