[PATCH v4 0/6] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t

[PATCH v4 0/6] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

The S25HL-T/S25HS-T family is the Cypress Semper Flash with Quad SPI.

The summary datasheets can be found in the following links.
https://www.cypress.com/file/424146/download (256Mb/512Mb/1Gb, single die)
https://www.cypress.com/file/499246/download (2Gb/4Gb, dual/quad die)

The full version can be found in the following links (registration
required).
https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-Semper-Flash-with-Quad-SPI/ta-p/260789?attachment-id=19522
https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-2Gb-MCP-Semper-Flash-with-Quad-SPI/ta-p/260823?attachment-id=29503

Tested on Xilinx Zynq-7000 FPGA board.

Changes in v4:
  - Reword 'legacy' to 'default'
  - Rename spi_nor_read() to spi_nor_default_ready()
  - Fix dummy cycle calculation in spansion_read_any_reg()
  - Modify comment for spansion_write_any_reg()
  - Merge block comments about SMPT in s25hx_t_post_sfdp_fixups()
  - Remove USE_CLSR flags from S25HL02GT and S25HS02GT

Changes in v3:
  - Split into multiple patches
  - Remove S25HL256T and S25HS256T
  - Add S25HL02GT and S25HS02GT 
  - Add support for multi-die package parts support
  - Cleanup Read/Write Any Register implementation
  - Remove erase_map fix for top/split sector layout
  - Set ECC data unit size (16B) to writesize 

Changes in v2:
  - Remove SPI_NOR_SKIP_SFDP flag and clean up related fixups
  - Check CFR3V[4] to determine page_size instead of force 512B
  - Depend on the patchset below to support non-uniform sector layout
    https://lore.kernel.org/linux-mtd/cover.1601612872.git.Takahiro.Kuwano@infineon.com/

Takahiro Kuwano (6):
  mtd: spi-nor: core: Add the ->ready() hook
  mtd: spi-nor: core: Expose spi_nor_clear_sr() to manufacturer drivers
  mtd: spi-nor: spansion: Add support for Read/Write Any  Register
  mtd: spi-nor: spansion: Add support for volatile QE bit
  mtd: spi-nor: spansion: Add status check for multi-die parts
  mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups

 drivers/mtd/spi-nor/core.c     |  10 +-
 drivers/mtd/spi-nor/core.h     |   3 +
 drivers/mtd/spi-nor/spansion.c | 325 +++++++++++++++++++++++++++++++++
 3 files changed, 334 insertions(+), 4 deletions(-)

-- 
2.25.1


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

[PATCH v4 1/6] mtd: spi-nor: core: Add the ->ready() hook

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

This hook can be used for SPI NOR flashes that do not support default
status read method.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
---
Changes in v4:
  - Reword 'legacy' to 'default' in commit description
  - Rename spi_nor_read() to spi_nor_default_ready() 
  
Changes in v3:
  - New in v3
    The purpose is same as the patch introduced by Yaliang Wang.
    https://patchwork.ozlabs.org/project/linux-mtd/patch/20210301142844.1089385-1-yaliang.wang@windriver.com/  

 drivers/mtd/spi-nor/core.c | 8 +++++---
 drivers/mtd/spi-nor/core.h | 2 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 0522304f52fa..5de72322ae32 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -785,12 +785,13 @@ static int spi_nor_fsr_ready(struct spi_nor *nor)
 }
 
 /**
- * spi_nor_ready() - Query the flash to see if it is ready for new commands.
+ * spi_nor_default_ready() - Query the flash to see if it is ready for new
+ * commands.
  * @nor:	pointer to 'struct spi_nor'.
  *
  * Return: 1 if ready, 0 if not ready, -errno on errors.
  */
-static int spi_nor_ready(struct spi_nor *nor)
+static int spi_nor_default_ready(struct spi_nor *nor)
 {
 	int sr, fsr;
 
@@ -826,7 +827,7 @@ static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
 		if (time_after_eq(jiffies, deadline))
 			timeout = 1;
 
-		ret = spi_nor_ready(nor);
+		ret = nor->params->ready(nor);
 		if (ret < 0)
 			return ret;
 		if (ret)
@@ -2920,6 +2921,7 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
 	params->quad_enable = spi_nor_sr2_bit1_quad_enable;
 	params->set_4byte_addr_mode = spansion_set_4byte_addr_mode;
 	params->setup = spi_nor_default_setup;
+	params->ready = spi_nor_default_ready;
 	/* Default to 16-bit Write Status (01h) Command */
 	nor->flags |= SNOR_F_HAS_16BIT_SR;
 
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 4a3f7f150b5d..4d06c27630fe 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -218,6 +218,7 @@ struct spi_nor_locking_ops {
  *                      flashes that have peculiarities to the SPI NOR standard
  *                      e.g. different opcodes, specific address calculation,
  *                      page size, etc.
+ * @ready:		checks if the SPI NOR flash is ready.
  * @locking_ops:	SPI NOR locking methods.
  */
 struct spi_nor_flash_parameter {
@@ -238,6 +239,7 @@ struct spi_nor_flash_parameter {
 	int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
 	u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
 	int (*setup)(struct spi_nor *nor, const struct spi_nor_hwcaps *hwcaps);
+	int (*ready)(struct spi_nor *nor);
 
 	const struct spi_nor_locking_ops *locking_ops;
 };
-- 
2.25.1


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

[PATCH v4 2/6] mtd: spi-nor: core: Expose spi_nor_clear_sr() to manufacturer drivers

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

spi_nor_clear_sr() needs to be called from manufacturer drivers that
implement the ->ready().

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
---
Changes in v4:
  - No change
  
Changes in v3:
  - New in v3

 drivers/mtd/spi-nor/core.c | 2 +-
 drivers/mtd/spi-nor/core.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 5de72322ae32..7ab0225473e1 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -653,7 +653,7 @@ static int spi_nor_xsr_ready(struct spi_nor *nor)
  * spi_nor_clear_sr() - Clear the Status Register.
  * @nor:	pointer to 'struct spi_nor'.
  */
-static void spi_nor_clear_sr(struct spi_nor *nor)
+void spi_nor_clear_sr(struct spi_nor *nor)
 {
 	int ret;
 
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 4d06c27630fe..596480aef924 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -447,6 +447,7 @@ int spi_nor_read_cr(struct spi_nor *nor, u8 *cr);
 int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len);
 int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1);
 
+void spi_nor_clear_sr(struct spi_nor *nor);
 int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr);
 ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t from, size_t len,
 			  u8 *buf);
-- 
2.25.1


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

[PATCH v4 3/6] mtd: spi-nor: spansion: Add support for Read/Write Any Register

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Some of Spansion/Cypress chips support Read/Write Any Register commands.
These commands are mainly used to write volatile registers and access to
the registers in second and subsequent die for multi-die package parts.

The Read Any Register instruction (65h) is followed by register address
and dummy cycles, then the selected register byte is returned.

The Write Any Register instruction (71h) is followed by register address
and register byte to write.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
Changes in v4:
  - Fix dummy cycle calculation in spansion_read_any_reg()
  - Modify comment for spansion_write_any_reg()
  
Changes in v3:
  - Cleanup implementation

 drivers/mtd/spi-nor/spansion.c | 106 +++++++++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index b0c5521c1e27..52a427d0af02 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -19,6 +19,112 @@
 #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS	0
 #define SPINOR_OP_CYPRESS_RD_FAST		0xee
 
+/**
+ * spansion_read_any_reg() - Read Any Register.
+ * @nor:	pointer to a 'struct spi_nor'
+ * @reg_addr:	register address
+ * @reg_dummy:	number of dummy cycles for register read
+ * @reg_val:	pointer to a buffer where the register value is copied into
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spansion_read_any_reg(struct spi_nor *nor, u32 reg_addr,
+				 u8 reg_dummy, u8 *reg_val)
+{
+	ssize_t ret;
+
+	if (nor->spimem) {
+		struct spi_mem_op op =
+			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_ANY_REG, 0),
+				   SPI_MEM_OP_ADDR(nor->addr_width, reg_addr, 0),
+				   SPI_MEM_OP_DUMMY(reg_dummy, 0),
+				   SPI_MEM_OP_DATA_IN(1, reg_val, 0));
+
+		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
+		op.dummy.nbytes = (reg_dummy * op.dummy.buswidth) / 8;
+		if (spi_nor_protocol_is_dtr(nor->reg_proto))
+			op.dummy.nbytes *= 2;
+
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		enum spi_nor_protocol proto = nor->read_proto;
+		u8 opcode = nor->read_opcode;
+		u8 dummy = nor->read_dummy;
+
+		nor->read_opcode = SPINOR_OP_RD_ANY_REG;
+		nor->read_dummy = reg_dummy;
+		nor->read_proto = nor->reg_proto;
+
+		ret = nor->controller_ops->read(nor, reg_addr, 1, reg_val);
+
+		nor->read_opcode = opcode;
+		nor->read_dummy = dummy;
+		nor->read_proto = proto;
+
+		if (ret == 1)
+			return ret;
+		if (ret != 1)
+			return -EIO;
+
+		ret = 0;
+	}
+
+	return ret;
+}
+
+/**
+ * spansion_write_any_reg() - Write Any Register.
+ * @nor:	pointer to a 'struct spi_nor'
+ * @reg_addr:	register address (should be a volatile register)
+ * @reg_val:	register value to be written
+ *
+ * Volatile register write will be effective immediately after the operation so
+ * this function does not poll the status.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spansion_write_any_reg(struct spi_nor *nor, u32 reg_addr, u8 reg_val)
+{
+	ssize_t ret;
+
+	ret = spi_nor_write_enable(nor);
+	if (ret)
+		return ret;
+
+	if (nor->spimem) {
+		struct spi_mem_op op =
+			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 0),
+				   SPI_MEM_OP_ADDR(nor->addr_width, reg_addr, 0),
+				   SPI_MEM_OP_NO_DUMMY,
+				   SPI_MEM_OP_DATA_OUT(1, &reg_val, 0));
+
+		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		enum spi_nor_protocol proto = nor->write_proto;
+		u8 opcode = nor->program_opcode;
+
+		nor->program_opcode = SPINOR_OP_WR_ANY_REG;
+		nor->write_proto = nor->reg_proto;
+
+		ret = nor->controller_ops->write(nor, reg_addr, 1, &reg_val);
+
+		nor->program_opcode = opcode;
+		nor->write_proto = proto;
+
+		if (ret < 0)
+			return ret;
+		if (ret != 1)
+			return -EIO;
+
+		ret = 0;
+	}
+
+	return ret;
+}
+
 /**
  * spi_nor_cypress_octal_dtr_enable() - Enable octal DTR on Cypress flashes.
  * @nor:		pointer to a 'struct spi_nor'
-- 
2.25.1


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

[PATCH v4 4/6] mtd: spi-nor: spansion: Add support for volatile QE bit

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Some of Spansion/Cypress chips support volatile version of configuration
registers and it is recommended to update volatile registers in the field
application due to a risk of the non-volatile registers corruption by
power interrupt. This patch adds a function to set Quad Enable bit in CFR1
volatile. The function supports multi-die package parts that require to
set the Quad Enable bit in each die.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
---
Changes in v4:
  - No change
  
Changes in v3:
  - Add multi-die package parts support

 drivers/mtd/spi-nor/spansion.c | 58 ++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 52a427d0af02..fdbf43bccc21 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -10,6 +10,8 @@
 
 #define SPINOR_OP_RD_ANY_REG			0x65	/* Read any register */
 #define SPINOR_OP_WR_ANY_REG			0x71	/* Write any register */
+#define SPINOR_REG_CYPRESS_CFR1V		0x00800002
+#define SPINOR_REG_CYPRESS_CFR1V_QUAD_EN	BIT(1)	/* Quad Enable */
 #define SPINOR_REG_CYPRESS_CFR2V		0x00800003
 #define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24	0xb
 #define SPINOR_REG_CYPRESS_CFR3V		0x00800004
@@ -125,6 +127,62 @@ static int spansion_write_any_reg(struct spi_nor *nor, u32 reg_addr, u8 reg_val)
 	return ret;
 }
 
+/**
+ * spansion_quad_enable_volatile() - enable Quad I/O mode in volatile register.
+ * @nor:	pointer to a 'struct spi_nor'
+ * @reg_dummy:	number of dummy cycles for register read
+ * @die_size:	size of each die to determine the number of dies
+ *
+ * It is recommended to update volatile registers in the field application due
+ * to a risk of the non-volatile registers corruption by power interrupt. This
+ * function sets Quad Enable bit in CFR1 volatile. If users set the Quad Enable
+ * bit in the CFR1 non-volatile in advance (typically by a Flash programmer
+ * before mounting Flash on PCB), the Quad Enable bit in the CFR1 volatile is
+ * also set during Flash power-up. This function supports multi-die package
+ * parts that require to set the Quad Enable bit in each die.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spansion_quad_enable_volatile(struct spi_nor *nor, u8 reg_dummy,
+					 u32 die_size)
+{
+	int ret;
+	u32 base, reg_addr;
+	u8 cfr1v, cfr1v_written;
+
+	for (base = 0; base < nor->params->size; base += die_size) {
+		reg_addr = base + SPINOR_REG_CYPRESS_CFR1V;
+
+		ret = spansion_read_any_reg(nor, reg_addr, reg_dummy, &cfr1v);
+		if (ret)
+			return ret;
+
+		if (cfr1v & SPINOR_REG_CYPRESS_CFR1V_QUAD_EN)
+			continue;
+
+		/* Update the Quad Enable bit. */
+		cfr1v |= SPINOR_REG_CYPRESS_CFR1V_QUAD_EN;
+
+		ret = spansion_write_any_reg(nor, reg_addr, cfr1v);
+		if (ret)
+			return ret;
+
+		cfr1v_written = cfr1v;
+
+		/* Read back and check it. */
+		ret = spansion_read_any_reg(nor, reg_addr, reg_dummy, &cfr1v);
+		if (ret)
+			return ret;
+
+		if (cfr1v != cfr1v_written) {
+			dev_err(nor->dev, "CFR1: Read back test failed\n");
+			return -EIO;
+		}
+	}
+
+	return 0;
+}
+
 /**
  * spi_nor_cypress_octal_dtr_enable() - Enable octal DTR on Cypress flashes.
  * @nor:		pointer to a 'struct spi_nor'
-- 
2.25.1


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

[PATCH v4 5/6] mtd: spi-nor: spansion: Add status check for multi-die parts

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

The multi-die package parts from Spansion/Cypress require to check
the status register value in each die to detemine the device is
ready. This patch adds a helper that queries status of each die by
Read Any Register command. The device specific ->ready() hook will
call this helper with the number of dummy cycles and die size.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
---
Changes in v4:
  - Reword 'legacy' to 'default' in function header comments

Changes in v3:
  - New in v3

 drivers/mtd/spi-nor/spansion.c | 46 ++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index fdbf43bccc21..04ad8f83dae0 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -10,6 +10,7 @@
 
 #define SPINOR_OP_RD_ANY_REG			0x65	/* Read any register */
 #define SPINOR_OP_WR_ANY_REG			0x71	/* Write any register */
+#define SPINOR_REG_CYPRESS_STR1V		0x00800000
 #define SPINOR_REG_CYPRESS_CFR1V		0x00800002
 #define SPINOR_REG_CYPRESS_CFR1V_QUAD_EN	BIT(1)	/* Quad Enable */
 #define SPINOR_REG_CYPRESS_CFR2V		0x00800003
@@ -183,6 +184,51 @@ static int spansion_quad_enable_volatile(struct spi_nor *nor, u8 reg_dummy,
 	return 0;
 }
 
+/**
+ * spansion_mdp_ready() - Query the Status Register via Read Any Register
+ *                        command for multi-die package parts that do not
+ *                        support default RDSR(05h)
+ * @nor:	pointer to 'struct spi_nor'.
+ * @reg_dummy:	number of dummy cycles for register read
+ * @die_size:	size of each die to determine the number of dies
+ *
+ * Return: 1 if ready, 0 if not ready, -errno on errors.
+ */
+static int spansion_mdp_ready(struct spi_nor *nor, u8 reg_dummy, u32 die_size)
+{
+	int ret;
+	u32 base;
+	u8 sr;
+
+	for (base = 0; base < nor->params->size; base += die_size) {
+		ret = spansion_read_any_reg(nor,
+					    base + SPINOR_REG_CYPRESS_STR1V,
+					    reg_dummy, &sr);
+		if (ret)
+			return ret;
+
+		if (sr & (SR_E_ERR | SR_P_ERR)) {
+			if (sr & SR_E_ERR)
+				dev_err(nor->dev, "Erase Error occurred\n");
+			else
+				dev_err(nor->dev, "Programming Error occurred\n");
+
+			spi_nor_clear_sr(nor);
+
+			ret = spi_nor_write_disable(nor);
+			if (ret)
+				return ret;
+
+			return -EIO;
+		}
+
+		if (sr & SR_WIP)
+			return 0;
+	}
+
+	return 1;
+}
+
 /**
  * spi_nor_cypress_octal_dtr_enable() - Enable octal DTR on Cypress flashes.
  * @nor:		pointer to a 'struct spi_nor'
-- 
2.25.1


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

[PATCH v4 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

The S25HL-T/S25HS-T family is the Cypress Semper Flash with Quad SPI.

For the single-die package parts (512Mb and 1Gb), only bottom 4KB and
uniform sector sizes are supported. For the multi-die package parts (2Gb),
only uniform sector sizes is supprted. This is due to missing or incorrect
entries in SMPT. Fixup for other sector sizes configurations will be
followed up as needed.

Tested on Xilinx Zynq-7000 FPGA board.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
---
Changes in v4:
  - Merge block comments about SMPT in s25hx_t_post_sfdp_fixups()
  - Remove USE_CLSR flags from S25HL02GT and S25HS02GT

Changes in v3:
  - Remove S25HL256T and S25HS256T
  - Add S25HL02GT and S25HS02GT 
  - Add support for multi-die package parts support
  - Remove erase_map fix for top/split sector layout
  - Set ECC data unit size (16B) to writesize 

 drivers/mtd/spi-nor/spansion.c | 115 +++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 04ad8f83dae0..63a10ba28fc1 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -229,6 +229,103 @@ static int spansion_mdp_ready(struct spi_nor *nor, u8 reg_dummy, u32 die_size)
 	return 1;
 }
 
+static int s25hx_t_quad_enable(struct spi_nor *nor)
+{
+	return spansion_quad_enable_volatile(nor, 0, SZ_128M);
+}
+
+static int s25hx_t_mdp_ready(struct spi_nor *nor)
+{
+	return spansion_mdp_ready(nor, 0, SZ_128M);
+}
+
+static int
+s25hx_t_post_bfpt_fixups(struct spi_nor *nor,
+			 const struct sfdp_parameter_header *bfpt_header,
+			 const struct sfdp_bfpt *bfpt,
+			 struct spi_nor_flash_parameter *params)
+{
+	int ret;
+	u32 addr;
+	u8 cfr3v;
+
+	ret = spi_nor_set_4byte_addr_mode(nor, true);
+	if (ret)
+		return ret;
+	nor->addr_width = 4;
+
+	/* Replace Quad Enable with volatile version */
+	params->quad_enable = s25hx_t_quad_enable;
+
+	/*
+	 * The page_size is set to 512B from BFPT, but it actually depends on
+	 * the configuration register. Look up the CFR3V and determine the
+	 * page_size. For multi-die package parts, use 512B only when the all
+	 * dies are configured to 512B buffer.
+	 */
+	for (addr = 0; addr < params->size; addr += SZ_128M) {
+		ret = spansion_read_any_reg(nor,
+					    addr + SPINOR_REG_CYPRESS_CFR3V, 0,
+					    &cfr3v);
+		if (ret)
+			return ret;
+
+		if (!(cfr3v & SPINOR_REG_CYPRESS_CFR3V_PGSZ)) {
+			params->page_size = 256;
+			return 0;
+		}
+	}
+	params->page_size = 512;
+
+	return 0;
+}
+
+void s25hx_t_post_sfdp_fixups(struct spi_nor *nor)
+{
+	/* Fast Read 4B requires mode cycles */
+	nor->params->reads[SNOR_CMD_READ_FAST].num_mode_clocks = 8;
+
+	/* The writesize should be ECC data unit size */
+	nor->params->writesize = 16;
+
+	/*
+	 * For the single-die package parts (512Mb and 1Gb), bottom 4KB and
+	 * uniform sector maps are correctly populated in the erase_map
+	 * structure. The table below shows all possible combinations of related
+	 * register bits and its availability in SMPT.
+	 *
+	 *   CFR3[3] | CFR1[6] | CFR1[2] | Sector Map | Available in SMPT?
+	 *  -------------------------------------------------------------------
+	 *      0    |    0    |    0    | Bottom     | YES
+	 *      0    |    0    |    1    | Top        | NO (decoded as Split)
+	 *      0    |    1    |    0    | Split      | NO
+	 *      0    |    1    |    1    | Split      | NO (decoded as Top)
+	 *      1    |    0    |    0    | Uniform    | YES
+	 *      1    |    0    |    1    | Uniform    | NO
+	 *      1    |    1    |    0    | Uniform    | NO
+	 *      1    |    1    |    1    | Uniform    | NO
+	 *  -------------------------------------------------------------------
+	 *
+	 * For the dual-die package parts (2Gb), SMPT parse fails due to
+	 * incorrect SMPT entries and the erase map is populated as 4K uniform
+	 * that does not supported the parts. So it needs to be rolled back to
+	 * 256K uniform that is the factory default of multi-die package parts.
+	 */
+	if (nor->params->size > SZ_128M) {
+		spi_nor_init_uniform_erase_map(&nor->params->erase_map,
+					       BIT(SNOR_ERASE_TYPE_MAX - 1),
+					       nor->params->size);
+
+		/* Need to check status of each die via RDAR command */
+		nor->params->ready = s25hx_t_mdp_ready;
+	}
+}
+
+static struct spi_nor_fixups s25hx_t_fixups = {
+	.post_bfpt = s25hx_t_post_bfpt_fixups,
+	.post_sfdp = s25hx_t_post_sfdp_fixups
+};
+
 /**
  * spi_nor_cypress_octal_dtr_enable() - Enable octal DTR on Cypress flashes.
  * @nor:		pointer to a 'struct spi_nor'
@@ -479,6 +576,24 @@ static const struct flash_info spansion_parts[] = {
 	{ "s25fl256l",  INFO(0x016019,      0,  64 * 1024, 512,
 			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
 			     SPI_NOR_4B_OPCODES) },
+	{ "s25hl512t",  INFO6(0x342a1a, 0x0f0390, 256 * 1024, 256,
+			     SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR)
+	  .fixups = &s25hx_t_fixups },
+	{ "s25hl01gt",  INFO6(0x342a1b, 0x0f0390, 256 * 1024, 512,
+			     SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR)
+	  .fixups = &s25hx_t_fixups },
+	{ "s25hl02gt",  INFO6(0x342a1c, 0x0f0090, 256 * 1024, 1024,
+			     SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+	  .fixups = &s25hx_t_fixups },
+	{ "s25hs512t",  INFO6(0x342b1a, 0x0f0390, 256 * 1024, 256,
+			     SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR)
+	  .fixups = &s25hx_t_fixups },
+	{ "s25hs01gt",  INFO6(0x342b1b, 0x0f0390, 256 * 1024, 512,
+			     SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR)
+	  .fixups = &s25hx_t_fixups },
+	{ "s25hs02gt",  INFO6(0x342b1c, 0x0f0090, 256 * 1024, 1024,
+			     SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+	  .fixups = &s25hx_t_fixups },
 	{ "cy15x104q",  INFO6(0x042cc2, 0x7f7f7f, 512 * 1024, 1,
 			      SPI_NOR_NO_ERASE) },
 	{ "s28hs512t",   INFO(0x345b1a,      0, 256 * 1024, 256,
-- 
2.25.1


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

Re: [PATCH v4 3/6] mtd: spi-nor: spansion: Add support for Read/Write Any Register

[Pratyush Yadav]

On 19/03/21 03:56PM, tkuw584924@gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> Some of Spansion/Cypress chips support Read/Write Any Register commands.
> These commands are mainly used to write volatile registers and access to
> the registers in second and subsequent die for multi-die package parts.
> 
> The Read Any Register instruction (65h) is followed by register address
> and dummy cycles, then the selected register byte is returned.
> 
> The Write Any Register instruction (71h) is followed by register address
> and register byte to write.
> 
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

Re: [PATCH v4 0/6] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t

[Tudor.Ambarus]

Hi,

On 3/19/21 8:51 AM, tkuw584924@gmail.com wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> The S25HL-T/S25HS-T family is the Cypress Semper Flash with Quad SPI.
> 
> The summary datasheets can be found in the following links.
> https://www.cypress.com/file/424146/download (256Mb/512Mb/1Gb, single die)
> https://www.cypress.com/file/499246/download (2Gb/4Gb, dual/quad die)
> 
> The full version can be found in the following links (registration
> required).
> https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-Semper-Flash-with-Quad-SPI/ta-p/260789?attachment-id=19522
> https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-2Gb-MCP-Semper-Flash-with-Quad-SPI/ta-p/260823?attachment-id=29503

Takahiro, looks like I can't access the dual/quad die full datasheet, not enough rights,
even after creating an account.

How are multi die ops handled when crossing a die boundary?

> 
> Tested on Xilinx Zynq-7000 FPGA board.

Have you tried to erase/read/write multiple dies with a single request?

Cheers,
ta
> 
> Changes in v4:
>   - Reword 'legacy' to 'default'
>   - Rename spi_nor_read() to spi_nor_default_ready()
>   - Fix dummy cycle calculation in spansion_read_any_reg()
>   - Modify comment for spansion_write_any_reg()
>   - Merge block comments about SMPT in s25hx_t_post_sfdp_fixups()
>   - Remove USE_CLSR flags from S25HL02GT and S25HS02GT
> 
> Changes in v3:
>   - Split into multiple patches
>   - Remove S25HL256T and S25HS256T
>   - Add S25HL02GT and S25HS02GT
>   - Add support for multi-die package parts support
>   - Cleanup Read/Write Any Register implementation
>   - Remove erase_map fix for top/split sector layout
>   - Set ECC data unit size (16B) to writesize
> 
> Changes in v2:
>   - Remove SPI_NOR_SKIP_SFDP flag and clean up related fixups
>   - Check CFR3V[4] to determine page_size instead of force 512B
>   - Depend on the patchset below to support non-uniform sector layout
>     https://lore.kernel.org/linux-mtd/cover.1601612872.git.Takahiro.Kuwano@infineon.com/
> 
> Takahiro Kuwano (6):
>   mtd: spi-nor: core: Add the ->ready() hook
>   mtd: spi-nor: core: Expose spi_nor_clear_sr() to manufacturer drivers
>   mtd: spi-nor: spansion: Add support for Read/Write Any  Register
>   mtd: spi-nor: spansion: Add support for volatile QE bit
>   mtd: spi-nor: spansion: Add status check for multi-die parts
>   mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups
> 
>  drivers/mtd/spi-nor/core.c     |  10 +-
>  drivers/mtd/spi-nor/core.h     |   3 +
>  drivers/mtd/spi-nor/spansion.c | 325 +++++++++++++++++++++++++++++++++
>  3 files changed, 334 insertions(+), 4 deletions(-)
> 
> --
> 2.25.1
> 

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

Re: [PATCH v4 0/6] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t

[Takahiro Kuwano]

Hi Tudor,

On 4/1/2021 3:09 PM, Tudor.Ambarus@microchip.com wrote:
> Hi,
> 
> On 3/19/21 8:51 AM, tkuw584924@gmail.com wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>
>> The S25HL-T/S25HS-T family is the Cypress Semper Flash with Quad SPI.
>>
>> The summary datasheets can be found in the following links.
>> https://www.cypress.com/file/424146/download (256Mb/512Mb/1Gb, single die)
>> https://www.cypress.com/file/499246/download (2Gb/4Gb, dual/quad die)
>>
>> The full version can be found in the following links (registration
>> required).
>> https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-Semper-Flash-with-Quad-SPI/ta-p/260789?attachment-id=19522
>> https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-2Gb-MCP-Semper-Flash-with-Quad-SPI/ta-p/260823?attachment-id=29503
> 
> Takahiro, looks like I can't access the dual/quad die full datasheet, not enough rights,
> even after creating an account.
> 
My colleague helped on this. I hope you could access the datasheet.

> How are multi die ops handled when crossing a die boundary?
> 
The existing erase/write ops are done by sector/page size alignment and
never cross a die boundary. The read ops does not care about die boundary
but still works because the Semper dual/quad die parts continue to output
data from next die when crossing a die boundary.

>>
>> Tested on Xilinx Zynq-7000 FPGA board.
> 
> Have you tried to erase/read/write multiple dies with a single request?
> 
Yes, I did 'mtd_debug erase/write/read' with address ranges that involve
two dies. You can find errata info in the datasheet about cross-die read.
My test does not cover the trigger condition of the failure actually.
I will submit another patch that work around the issue as needed.

Best Regards,
Takahiro

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

Re: [PATCH v4 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups

[Tudor.Ambarus]

Hi,

On 3/19/21 8:58 AM, tkuw584924@gmail.com wrote:
> +static int
> +s25hx_t_post_bfpt_fixups(struct spi_nor *nor,
> +                        const struct sfdp_parameter_header *bfpt_header,
> +                        const struct sfdp_bfpt *bfpt,
> +                        struct spi_nor_flash_parameter *params)
> +{
> +       int ret;
> +       u32 addr;
> +       u8 cfr3v;
> +
> +       ret = spi_nor_set_4byte_addr_mode(nor, true);
> +       if (ret)
> +               return ret;
> +       nor->addr_width = 4;

Takahiro, you are bypassing the core functions. spansion_set_4byte_addr_mode()
will be called at set_4byte_addr_mode() time, resulting in an illegal op?

Are we safe to modify the core and do the spi_nor_set_addr_width() and
nor->params->set_4byte_addr_mode() before parsing SFDP?

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

Re: [PATCH v4 0/6] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t

[Tudor.Ambarus]

On 4/2/21 10:13 AM, Takahiro Kuwano wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor,

Hi!

> 
> On 4/1/2021 3:09 PM, Tudor.Ambarus@microchip.com wrote:
>> Hi,
>>
>> On 3/19/21 8:51 AM, tkuw584924@gmail.com wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>>
>>> The S25HL-T/S25HS-T family is the Cypress Semper Flash with Quad SPI.
>>>
>>> The summary datasheets can be found in the following links.
>>> https://www.cypress.com/file/424146/download (256Mb/512Mb/1Gb, single die)
>>> https://www.cypress.com/file/499246/download (2Gb/4Gb, dual/quad die)
>>>
>>> The full version can be found in the following links (registration
>>> required).
>>> https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-Semper-Flash-with-Quad-SPI/ta-p/260789?attachment-id=19522
>>> https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-2Gb-MCP-Semper-Flash-with-Quad-SPI/ta-p/260823?attachment-id=29503
>>
>> Takahiro, looks like I can't access the dual/quad die full datasheet, not enough rights,
>> even after creating an account.
>>
> My colleague helped on this. I hope you could access the datasheet.

Takahiro, I can now access the datasheet, thanks! I see that Erase Chip
requires an address. Is the erase chip with address common to other
manufacturers? We'll have to update the core so that these flashe
benefit of the erase chip opcode. Otherwise we'll have to set
SNOR_F_NO_OP_CHIP_ERASE at flash declaration, which I don't really favor.

> 
>> How are multi die ops handled when crossing a die boundary?
>>
> The existing erase/write ops are done by sector/page size alignment and
> never cross a die boundary. The read ops does not care about die boundary

What do you mean? What will happen if I request to erase 512K starting from
offset (die0 - 256K)?

> but still works because the Semper dual/quad die parts continue to output
> data from next die when crossing a die boundary.

cool

> 
>>>
>>> Tested on Xilinx Zynq-7000 FPGA board.
>>
>> Have you tried to erase/read/write multiple dies with a single request?
>>
> Yes, I did 'mtd_debug erase/write/read' with address ranges that involve
> two dies. You can find errata info in the datasheet about cross-die read.
> My test does not cover the trigger condition of the failure actually.
> I will submit another patch that work around the issue as needed.
> 

How will you differentiate between silicon revisions?

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

Re: [PATCH v4 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups

[Takahiro Kuwano]

Hi Tudor,

On 4/8/2021 2:06 PM, Tudor.Ambarus@microchip.com wrote:
> Hi,
> 
> On 3/19/21 8:58 AM, tkuw584924@gmail.com wrote:
>> +static int
>> +s25hx_t_post_bfpt_fixups(struct spi_nor *nor,
>> +                        const struct sfdp_parameter_header *bfpt_header,
>> +                        const struct sfdp_bfpt *bfpt,
>> +                        struct spi_nor_flash_parameter *params)
>> +{
>> +       int ret;
>> +       u32 addr;
>> +       u8 cfr3v;
>> +
>> +       ret = spi_nor_set_4byte_addr_mode(nor, true);
>> +       if (ret)
>> +               return ret;
>> +       nor->addr_width = 4;
> 
> Takahiro, you are bypassing the core functions. spansion_set_4byte_addr_mode()
> will be called at set_4byte_addr_mode() time, resulting in an illegal op?
> 
Since the spansion_post_sfdp_fixups() adds the SNOR_F_4B_OPCODES flag,
spansion_set_4byte_addr_mode() is not called actually.

> Are we safe to modify the core and do the spi_nor_set_addr_width() and
> nor->params->set_4byte_addr_mode() before parsing SFDP?
> 
It sounds not safe to me as there are some discussions about addr_width.

https://patchwork.ozlabs.org/project/linux-mtd/patch/20201212115817.5122-1-vigneshr@ti.com/
https://patchwork.ozlabs.org/project/linux-mtd/patch/1611740450-47975-2-git-send-email-yangyicong@hisilicon.com/

Best Regards,
Takahiro

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

Re: [PATCH v4 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups

[Tudor.Ambarus]

On 4/8/21 11:21 AM, Takahiro Kuwano wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor,
> 
> On 4/8/2021 2:06 PM, Tudor.Ambarus@microchip.com wrote:
>> Hi,
>>
>> On 3/19/21 8:58 AM, tkuw584924@gmail.com wrote:
>>> +static int
>>> +s25hx_t_post_bfpt_fixups(struct spi_nor *nor,
>>> +                        const struct sfdp_parameter_header *bfpt_header,
>>> +                        const struct sfdp_bfpt *bfpt,
>>> +                        struct spi_nor_flash_parameter *params)
>>> +{
>>> +       int ret;
>>> +       u32 addr;
>>> +       u8 cfr3v;
>>> +
>>> +       ret = spi_nor_set_4byte_addr_mode(nor, true);
>>> +       if (ret)
>>> +               return ret;
>>> +       nor->addr_width = 4;
>>
>> Takahiro, you are bypassing the core functions. spansion_set_4byte_addr_mode()
>> will be called at set_4byte_addr_mode() time, resulting in an illegal op?
>>
> Since the spansion_post_sfdp_fixups() adds the SNOR_F_4B_OPCODES flag,
> spansion_set_4byte_addr_mode() is not called actually.

right. Do we have to undo this somewhere, i.e, call
spi_nor_set_4byte_addr_mode(nor, false); ?

> 
>> Are we safe to modify the core and do the spi_nor_set_addr_width() and
>> nor->params->set_4byte_addr_mode() before parsing SFDP?
>>
> It sounds not safe to me as there are some discussions about addr_width.

oh, yes, of course, because addr width and 4byte addr mode opcodes are
discovered when parsing SFDP. I need to drink my coffee before writing
emails :D.

> 
> https://patchwork.ozlabs.org/project/linux-mtd/patch/20201212115817.5122-1-vigneshr@ti.com/
> https://patchwork.ozlabs.org/project/linux-mtd/patch/1611740450-47975-2-git-send-email-yangyicong@hisilicon.com/
> 
> Best Regards,
> Takahiro
> 

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

Re: [PATCH v4 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups

[Takahiro Kuwano]

On 4/8/2021 7:03 PM, Tudor.Ambarus@microchip.com wrote:
> On 4/8/21 11:21 AM, Takahiro Kuwano wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Hi Tudor,
>>
>> On 4/8/2021 2:06 PM, Tudor.Ambarus@microchip.com wrote:
>>> Hi,
>>>
>>> On 3/19/21 8:58 AM, tkuw584924@gmail.com wrote:
>>>> +static int
>>>> +s25hx_t_post_bfpt_fixups(struct spi_nor *nor,
>>>> +                        const struct sfdp_parameter_header *bfpt_header,
>>>> +                        const struct sfdp_bfpt *bfpt,
>>>> +                        struct spi_nor_flash_parameter *params)
>>>> +{
>>>> +       int ret;
>>>> +       u32 addr;
>>>> +       u8 cfr3v;
>>>> +
>>>> +       ret = spi_nor_set_4byte_addr_mode(nor, true);
>>>> +       if (ret)
>>>> +               return ret;
>>>> +       nor->addr_width = 4;
>>>
>>> Takahiro, you are bypassing the core functions. spansion_set_4byte_addr_mode()
>>> will be called at set_4byte_addr_mode() time, resulting in an illegal op?
>>>
>> Since the spansion_post_sfdp_fixups() adds the SNOR_F_4B_OPCODES flag,
>> spansion_set_4byte_addr_mode() is not called actually.
> 
> right. Do we have to undo this somewhere, i.e, call
> spi_nor_set_4byte_addr_mode(nor, false); ?
> 
No. The Read/Write Any Register commands take 3 or 4 byte address depending
on the Flash's current addr mode. Since the spansion_read/write_any_reg()
use nor->addr_width which is always 4, the Flash's addr mode must always be
in 4 byte mode.

>>
>>> Are we safe to modify the core and do the spi_nor_set_addr_width() and
>>> nor->params->set_4byte_addr_mode() before parsing SFDP?
>>>
>> It sounds not safe to me as there are some discussions about addr_width.
> 
> oh, yes, of course, because addr width and 4byte addr mode opcodes are
> discovered when parsing SFDP. I need to drink my coffee before writing
> emails :D.
> 
:)

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

Re: [PATCH v4 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups

[Tudor.Ambarus]

Hi,

On 4/9/21 5:05 AM, Takahiro Kuwano wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 4/8/2021 7:03 PM, Tudor.Ambarus@microchip.com wrote:
>> On 4/8/21 11:21 AM, Takahiro Kuwano wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Hi Tudor,
>>>
>>> On 4/8/2021 2:06 PM, Tudor.Ambarus@microchip.com wrote:
>>>> Hi,
>>>>
>>>> On 3/19/21 8:58 AM, tkuw584924@gmail.com wrote:
>>>>> +static int
>>>>> +s25hx_t_post_bfpt_fixups(struct spi_nor *nor,
>>>>> +                        const struct sfdp_parameter_header *bfpt_header,
>>>>> +                        const struct sfdp_bfpt *bfpt,
>>>>> +                        struct spi_nor_flash_parameter *params)
>>>>> +{
>>>>> +       int ret;
>>>>> +       u32 addr;
>>>>> +       u8 cfr3v;
>>>>> +
>>>>> +       ret = spi_nor_set_4byte_addr_mode(nor, true);
>>>>> +       if (ret)
>>>>> +               return ret;
>>>>> +       nor->addr_width = 4;
>>>>
>>>> Takahiro, you are bypassing the core functions. spansion_set_4byte_addr_mode()
>>>> will be called at set_4byte_addr_mode() time, resulting in an illegal op?
>>>>
>>> Since the spansion_post_sfdp_fixups() adds the SNOR_F_4B_OPCODES flag,
>>> spansion_set_4byte_addr_mode() is not called actually.
>>
>> right. Do we have to undo this somewhere, i.e, call
>> spi_nor_set_4byte_addr_mode(nor, false); ?
>>
> No. The Read/Write Any Register commands take 3 or 4 byte address depending
> on the Flash's current addr mode. Since the spansion_read/write_any_reg()
> use nor->addr_width which is always 4, the Flash's addr mode must always be
> in 4 byte mode.

If you reboot your board via cmdline, and the flash doesn't have a reset opcode,
the flash will remain in 4 byte mode when the driver loads again. Wouldn't that
affect the read sfdp command, since it is sent in 3 byte mode? Can you try a
"reboot" cmd and check what happens?

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

Re: [PATCH v4 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups

[Takahiro Kuwano]

Hi Tudor,

On 4/9/2021 11:37 AM, Tudor.Ambarus@microchip.com wrote:
> Hi,
> 
> On 4/9/21 5:05 AM, Takahiro Kuwano wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 4/8/2021 7:03 PM, Tudor.Ambarus@microchip.com wrote:
>>> On 4/8/21 11:21 AM, Takahiro Kuwano wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> Hi Tudor,
>>>>
>>>> On 4/8/2021 2:06 PM, Tudor.Ambarus@microchip.com wrote:
>>>>> Hi,
>>>>>
>>>>> On 3/19/21 8:58 AM, tkuw584924@gmail.com wrote:
>>>>>> +static int
>>>>>> +s25hx_t_post_bfpt_fixups(struct spi_nor *nor,
>>>>>> +                        const struct sfdp_parameter_header *bfpt_header,
>>>>>> +                        const struct sfdp_bfpt *bfpt,
>>>>>> +                        struct spi_nor_flash_parameter *params)
>>>>>> +{
>>>>>> +       int ret;
>>>>>> +       u32 addr;
>>>>>> +       u8 cfr3v;
>>>>>> +
>>>>>> +       ret = spi_nor_set_4byte_addr_mode(nor, true);
>>>>>> +       if (ret)
>>>>>> +               return ret;
>>>>>> +       nor->addr_width = 4;
>>>>>
>>>>> Takahiro, you are bypassing the core functions. spansion_set_4byte_addr_mode()
>>>>> will be called at set_4byte_addr_mode() time, resulting in an illegal op?
>>>>>
>>>> Since the spansion_post_sfdp_fixups() adds the SNOR_F_4B_OPCODES flag,
>>>> spansion_set_4byte_addr_mode() is not called actually.
>>>
>>> right. Do we have to undo this somewhere, i.e, call
>>> spi_nor_set_4byte_addr_mode(nor, false); ?
>>>
>> No. The Read/Write Any Register commands take 3 or 4 byte address depending
>> on the Flash's current addr mode. Since the spansion_read/write_any_reg()
>> use nor->addr_width which is always 4, the Flash's addr mode must always be
>> in 4 byte mode.
> 
> If you reboot your board via cmdline, and the flash doesn't have a reset opcode,
> the flash will remain in 4 byte mode when the driver loads again. Wouldn't that
> affect the read sfdp command, since it is sent in 3 byte mode? Can you try a
> "reboot" cmd and check what happens?
> 
The Flash always takes 3 byte address in Read SFDP command regardless of
address mode. I tried reboot and confirmed it's working correctly.

Best Regards,
Takahiro


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

Re: [PATCH v4 0/6] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t

[Takahiro Kuwano]

On 4/8/2021 2:35 PM, Tudor.Ambarus@microchip.com wrote:
> On 4/2/21 10:13 AM, Takahiro Kuwano wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Hi Tudor,
> 
> Hi!
> 
>>
>> On 4/1/2021 3:09 PM, Tudor.Ambarus@microchip.com wrote:
>>> Hi,
>>>
>>> On 3/19/21 8:51 AM, tkuw584924@gmail.com wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>>>
>>>> The S25HL-T/S25HS-T family is the Cypress Semper Flash with Quad SPI.
>>>>
>>>> The summary datasheets can be found in the following links.
>>>> https://www.cypress.com/file/424146/download (256Mb/512Mb/1Gb, single die)
>>>> https://www.cypress.com/file/499246/download (2Gb/4Gb, dual/quad die)
>>>>
>>>> The full version can be found in the following links (registration
>>>> required).
>>>> https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-Semper-Flash-with-Quad-SPI/ta-p/260789?attachment-id=19522
>>>> https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-2Gb-MCP-Semper-Flash-with-Quad-SPI/ta-p/260823?attachment-id=29503
>>>
>>> Takahiro, looks like I can't access the dual/quad die full datasheet, not enough rights,
>>> even after creating an account.
>>>
>> My colleague helped on this. I hope you could access the datasheet.
> 
> Takahiro, I can now access the datasheet, thanks! I see that Erase Chip
> requires an address. Is the erase chip with address common to other
> manufacturers? We'll have to update the core so that these flashe
> benefit of the erase chip opcode. Otherwise we'll have to set
> SNOR_F_NO_OP_CHIP_ERASE at flash declaration, which I don't really favor.
> 
I overlooked the erase chip for multi-die package parts. Thank you for
pointing this out. I checked Micron MT25QL02GCBB datasheet and found the
part supports DIE ERASE (C4h) instead of standard CHIP ERASE.
https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_l_02g_cbb_0.pdf?rev=43f7f66fc8da4d7d901b35fa51284c8f

To support die erase, the core needs to know die size. That will initiate
a discussion about overall support for multi-die package devices. I think
it's better to create another series of patches for that. In this series,
I would set SNOR_F_NO_OP_CHIP_ERASE as a temporary solution.

>>
>>> How are multi die ops handled when crossing a die boundary?
>>>
>> The existing erase/write ops are done by sector/page size alignment and
>> never cross a die boundary. The read ops does not care about die boundary
> 
> What do you mean? What will happen if I request to erase 512K starting from
> offset (die0 - 256K)?
> 
I meant the erase and write requests are split into multiple ops based on
sector or page size. So, if you request to erase 512K starting from there,
two erase ops will be performed. One is for the last sector in die0 and
another is for the first sector in die1.

>> but still works because the Semper dual/quad die parts continue to output
>> data from next die when crossing a die boundary.
> 
> cool
> 
>>
>>>>
>>>> Tested on Xilinx Zynq-7000 FPGA board.
>>>
>>> Have you tried to erase/read/write multiple dies with a single request?
>>>
>> Yes, I did 'mtd_debug erase/write/read' with address ranges that involve
>> two dies. You can find errata info in the datasheet about cross-die read.
>> My test does not cover the trigger condition of the failure actually.
>> I will submit another patch that work around the issue as needed.
>>
> 
> How will you differentiate between silicon revisions?
> 
No way to differentiate. So, if I apply a workaround like splitting a read
request into two read ops, that is performed for newer silicon even though
it is not needed. I would like to implement the workaround once I find it
is a real issue on many systems.

Best Regards,
Takahiro

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

Re: [PATCH v4 3/6] mtd: spi-nor: spansion: Add support for Read/Write Any Register

[Takahiro Kuwano]

On 3/19/2021 3:56 PM, tkuw584924@gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> Some of Spansion/Cypress chips support Read/Write Any Register commands.
> These commands are mainly used to write volatile registers and access to
> the registers in second and subsequent die for multi-die package parts.
> 
> The Read Any Register instruction (65h) is followed by register address
> and dummy cycles, then the selected register byte is returned.
> 
> The Write Any Register instruction (71h) is followed by register address
> and register byte to write.
> 
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> ---
> Changes in v4:
>   - Fix dummy cycle calculation in spansion_read_any_reg()
>   - Modify comment for spansion_write_any_reg()
>   
> Changes in v3:
>   - Cleanup implementation
> 
>  drivers/mtd/spi-nor/spansion.c | 106 +++++++++++++++++++++++++++++++++
>  1 file changed, 106 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index b0c5521c1e27..52a427d0af02 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -19,6 +19,112 @@
>  #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS	0
>  #define SPINOR_OP_CYPRESS_RD_FAST		0xee
>  
> +/**
> + * spansion_read_any_reg() - Read Any Register.
> + * @nor:	pointer to a 'struct spi_nor'
> + * @reg_addr:	register address
> + * @reg_dummy:	number of dummy cycles for register read
> + * @reg_val:	pointer to a buffer where the register value is copied into
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spansion_read_any_reg(struct spi_nor *nor, u32 reg_addr,
> +				 u8 reg_dummy, u8 *reg_val)
> +{
> +	ssize_t ret;
> +
> +	if (nor->spimem) {
> +		struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_ANY_REG, 0),
> +				   SPI_MEM_OP_ADDR(nor->addr_width, reg_addr, 0),
> +				   SPI_MEM_OP_DUMMY(reg_dummy, 0),
> +				   SPI_MEM_OP_DATA_IN(1, reg_val, 0));
> +
> +		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> +
> +		op.dummy.nbytes = (reg_dummy * op.dummy.buswidth) / 8;
> +		if (spi_nor_protocol_is_dtr(nor->reg_proto))
> +			op.dummy.nbytes *= 2;
> +
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		enum spi_nor_protocol proto = nor->read_proto;
> +		u8 opcode = nor->read_opcode;
> +		u8 dummy = nor->read_dummy;
> +
> +		nor->read_opcode = SPINOR_OP_RD_ANY_REG;
> +		nor->read_dummy = reg_dummy;
> +		nor->read_proto = nor->reg_proto;
> +
> +		ret = nor->controller_ops->read(nor, reg_addr, 1, reg_val);
> +
> +		nor->read_opcode = opcode;
> +		nor->read_dummy = dummy;
> +		nor->read_proto = proto;
> +
> +		if (ret == 1)
> +			return ret;

This is wrong. Will fix this to

		if (ret < 0)
			return ret;


> +		if (ret != 1)
> +			return -EIO;
> +
> +		ret = 0;
> +	}
> +
> +	return ret;
> +}

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