linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/6] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t
@ 2021-04-27  7:08 tkuw584924
  2021-04-27  7:08 ` [PATCH v5 1/6] mtd: spi-nor: core: Add the ->ready() hook tkuw584924
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: tkuw584924 @ 2021-04-27  7:08 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, miquel.raynal, richard, vigneshr, p.yadav,
	tkuw584924, Bacem.Daassi, Takahiro Kuwano

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 v5:
  - Fix 'if (ret == 1)' to 'if (ret < 0)' in spansion_read_any_reg()
  - Add NO_CHIP_ERASE flag to S25HL02GT and S25HS02GT

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 | 327 +++++++++++++++++++++++++++++++++
 3 files changed, 336 insertions(+), 4 deletions(-)

-- 
2.25.1


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

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

* [PATCH v5 1/6] mtd: spi-nor: core: Add the ->ready() hook
  2021-04-27  7:08 [PATCH v5 0/6] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t tkuw584924
@ 2021-04-27  7:08 ` tkuw584924
  2021-04-27  7:08 ` [PATCH v5 2/6] mtd: spi-nor: core: Expose spi_nor_clear_sr() to manufacturer drivers tkuw584924
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: tkuw584924 @ 2021-04-27  7:08 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, miquel.raynal, richard, vigneshr, p.yadav,
	tkuw584924, Bacem.Daassi, Takahiro Kuwano

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 v5:
  - No change

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/

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

* [PATCH v5 2/6] mtd: spi-nor: core: Expose spi_nor_clear_sr() to manufacturer drivers
  2021-04-27  7:08 [PATCH v5 0/6] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t tkuw584924
  2021-04-27  7:08 ` [PATCH v5 1/6] mtd: spi-nor: core: Add the ->ready() hook tkuw584924
@ 2021-04-27  7:08 ` tkuw584924
  2021-04-27  7:08 ` [PATCH v5 3/6] mtd: spi-nor: spansion: Add support for Read/Write Any Register tkuw584924
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: tkuw584924 @ 2021-04-27  7:08 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, miquel.raynal, richard, vigneshr, p.yadav,
	tkuw584924, Bacem.Daassi, Takahiro Kuwano

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 v5:
  - No change
  
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/

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

* [PATCH v5 3/6] mtd: spi-nor: spansion: Add support for Read/Write Any Register
  2021-04-27  7:08 [PATCH v5 0/6] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t tkuw584924
  2021-04-27  7:08 ` [PATCH v5 1/6] mtd: spi-nor: core: Add the ->ready() hook tkuw584924
  2021-04-27  7:08 ` [PATCH v5 2/6] mtd: spi-nor: core: Expose spi_nor_clear_sr() to manufacturer drivers tkuw584924
@ 2021-04-27  7:08 ` tkuw584924
  2021-05-27 11:06   ` Vignesh Raghavendra
  2021-04-27  7:08 ` [PATCH v5 4/6] mtd: spi-nor: spansion: Add support for volatile QE bit tkuw584924
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: tkuw584924 @ 2021-04-27  7:08 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, miquel.raynal, richard, vigneshr, p.yadav,
	tkuw584924, Bacem.Daassi, Takahiro Kuwano

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>
---
Changes in v5:
  - Fix 'if (ret == 1)' to 'if (ret < 0)' in spansion_read_any_reg()

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..436db8933dcf 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 < 0)
+			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/

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

* [PATCH v5 4/6] mtd: spi-nor: spansion: Add support for volatile QE bit
  2021-04-27  7:08 [PATCH v5 0/6] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t tkuw584924
                   ` (2 preceding siblings ...)
  2021-04-27  7:08 ` [PATCH v5 3/6] mtd: spi-nor: spansion: Add support for Read/Write Any Register tkuw584924
@ 2021-04-27  7:08 ` tkuw584924
  2021-04-27  7:09 ` [PATCH v5 5/6] mtd: spi-nor: spansion: Add status check for multi-die parts tkuw584924
  2021-04-27  7:09 ` [PATCH v5 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups tkuw584924
  5 siblings, 0 replies; 11+ messages in thread
From: tkuw584924 @ 2021-04-27  7:08 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, miquel.raynal, richard, vigneshr, p.yadav,
	tkuw584924, Bacem.Daassi, Takahiro Kuwano

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 v5:
  - No change
  
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 436db8933dcf..0eac092f6c95 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/

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

* [PATCH v5 5/6] mtd: spi-nor: spansion: Add status check for multi-die parts
  2021-04-27  7:08 [PATCH v5 0/6] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t tkuw584924
                   ` (3 preceding siblings ...)
  2021-04-27  7:08 ` [PATCH v5 4/6] mtd: spi-nor: spansion: Add support for volatile QE bit tkuw584924
@ 2021-04-27  7:09 ` tkuw584924
  2021-04-27  7:09 ` [PATCH v5 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups tkuw584924
  5 siblings, 0 replies; 11+ messages in thread
From: tkuw584924 @ 2021-04-27  7:09 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, miquel.raynal, richard, vigneshr, p.yadav,
	tkuw584924, Bacem.Daassi, Takahiro Kuwano

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 v5:
  - No change
  
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 0eac092f6c95..79d3249ed0aa 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/

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

* [PATCH v5 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups
  2021-04-27  7:08 [PATCH v5 0/6] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t tkuw584924
                   ` (4 preceding siblings ...)
  2021-04-27  7:09 ` [PATCH v5 5/6] mtd: spi-nor: spansion: Add status check for multi-die parts tkuw584924
@ 2021-04-27  7:09 ` tkuw584924
  2021-05-27 13:20   ` Vignesh Raghavendra
  5 siblings, 1 reply; 11+ messages in thread
From: tkuw584924 @ 2021-04-27  7:09 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, miquel.raynal, richard, vigneshr, p.yadav,
	tkuw584924, Bacem.Daassi, Takahiro Kuwano

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 v5:
  - Add NO_CHIP_ERASE flag to S25HL02GT and S25HS02GT

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 | 117 +++++++++++++++++++++++++++++++++
 1 file changed, 117 insertions(+)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 79d3249ed0aa..b3b006874b4f 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,26 @@ 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 |
+			     NO_CHIP_ERASE)
+	  .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 |
+			     NO_CHIP_ERASE)
+	  .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/

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

* Re: [PATCH v5 3/6] mtd: spi-nor: spansion: Add support for Read/Write Any Register
  2021-04-27  7:08 ` [PATCH v5 3/6] mtd: spi-nor: spansion: Add support for Read/Write Any Register tkuw584924
@ 2021-05-27 11:06   ` Vignesh Raghavendra
  2021-06-01  7:53     ` Takahiro Kuwano
  0 siblings, 1 reply; 11+ messages in thread
From: Vignesh Raghavendra @ 2021-05-27 11:06 UTC (permalink / raw)
  To: tkuw584924, linux-mtd
  Cc: tudor.ambarus, miquel.raynal, richard, p.yadav, Bacem.Daassi,
	Takahiro Kuwano

Hi,

On 4/27/21 12:38 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>
> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
> ---
> Changes in v5:
>   - Fix 'if (ret == 1)' to 'if (ret < 0)' in spansion_read_any_reg()
> 
> 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..436db8933dcf 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));

This isn't compatible with DDR mode. We should able to use
spansion_read_any_reg() helper in place of some of the open coding being
done today in spi_nor_cypress_octal_dtr_enable(). Same applies for
spansion_write_any_reg().

> +
> +		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;

spi_nor_spimem_setup_op() should care of above right?

> +
> +		ret = spi_mem_exec_op(nor->spimem, &op);

		return spi_mem_exec_op(nor->spimem, &op);

Thus avoid extra level indentation below.

> +	} 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 < 0)
> +			return ret;
> +		if (ret != 1)
> +			return -EIO;
> +


> +		ret = 0;

		return 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)


To be safe, lets explicitly reject writes to non-volatile register
addresses.

> + * @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;


Hmm, I don't see spi_nor_write_disable().

Either both spi_nor_write_enable() and spi_nor_write_disable() should be
responsibility of caller or both needs to be taken care of here.

And the same needs to be documented in the function description

> +
> +	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'
> 

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

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

* Re: [PATCH v5 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups
  2021-04-27  7:09 ` [PATCH v5 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups tkuw584924
@ 2021-05-27 13:20   ` Vignesh Raghavendra
  2021-06-01  8:09     ` Takahiro Kuwano
  0 siblings, 1 reply; 11+ messages in thread
From: Vignesh Raghavendra @ 2021-05-27 13:20 UTC (permalink / raw)
  To: tkuw584924, linux-mtd
  Cc: tudor.ambarus, miquel.raynal, richard, p.yadav, Bacem.Daassi,
	Takahiro Kuwano



On 4/27/21 12:39 PM, tkuw584924@gmail.com wrote:
> 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 v5:
>   - Add NO_CHIP_ERASE flag to S25HL02GT and S25HS02GT
> 
> 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 | 117 +++++++++++++++++++++++++++++++++
>  1 file changed, 117 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index 79d3249ed0aa..b3b006874b4f 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);

Number of dies should be part of struct flash_info
Die size can then be derived from total_size / die_num


> +}
> +
> +static int s25hx_t_mdp_ready(struct spi_nor *nor)
> +{
> +	return spansion_mdp_ready(nor, 0, SZ_128M);

Hmm, is it necessary to poll second die if writes/erases were targeted
to first die only?

Please split multi die support into a separate series. SPI NOR core
should be aware of multiple dies and thus call poll on each of the dies
separately.

This will also allow base S25hx support to be accepted faster.

> +}
> +
> +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;

Don't we need to check if flash size is actual >16M before enabling 4
byte addressing?

> +
> +	/* 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;
> +}
> +
Regards
Vignesh

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

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

* Re: [PATCH v5 3/6] mtd: spi-nor: spansion: Add support for Read/Write Any Register
  2021-05-27 11:06   ` Vignesh Raghavendra
@ 2021-06-01  7:53     ` Takahiro Kuwano
  0 siblings, 0 replies; 11+ messages in thread
From: Takahiro Kuwano @ 2021-06-01  7:53 UTC (permalink / raw)
  To: Vignesh Raghavendra, linux-mtd
  Cc: tudor.ambarus, miquel.raynal, richard, p.yadav, Bacem.Daassi,
	Takahiro Kuwano

Hi Vignesh,

On 5/27/2021 8:06 PM, Vignesh Raghavendra wrote:
> Hi,
> 
> On 4/27/21 12:38 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>
>> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
>> ---
>> Changes in v5:
>>   - Fix 'if (ret == 1)' to 'if (ret < 0)' in spansion_read_any_reg()
>>
>> 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..436db8933dcf 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));
> 
> This isn't compatible with DDR mode. We should able to use
> spansion_read_any_reg() helper in place of some of the open coding being
> done today in spi_nor_cypress_octal_dtr_enable(). Same applies for
> spansion_write_any_reg().
> 
I don't see why this isn't compatible with DDR mode.
The spi_nor_spimem_setup_op() and dummy adjustment take care of DDR.

To use this helper in spi_nor_cypress_octal_dtr_enable(), addr_width needs
to be passed by parameter instead of using 'nor->addr_width'. Please let
me know if any other changes are required.

>> +
>> +		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;
> 
> spi_nor_spimem_setup_op() should care of above right?
> 
This is same as spi_nor_spimem_read_data() in core.c. We need to convert
the dummy 'cycles' to the dummy 'bytes' after spi_nor_spimem_setup_op().
I will add a comment about it.

>> +
>> +		ret = spi_mem_exec_op(nor->spimem, &op);
> 
> 		return spi_mem_exec_op(nor->spimem, &op);
> 
> Thus avoid extra level indentation below.
> 
>> +	} 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 < 0)
>> +			return ret;
>> +		if (ret != 1)
>> +			return -EIO;
>> +
> 
> 
>> +		ret = 0;
> 
> 		return 0;
> 
For better readability, how about adding a helper of controller ops?

	if (nor->spimem) {
		[...]
		return spi_mem_exec_op(nor->spimem, &op);
	}

	return spansion_controller_ops_read_any_reg(nor, reg_addr, reg_dummy, reg_val);


>> +	}
>> +
>> +	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)
> 
> 
> To be safe, lets explicitly reject writes to non-volatile register
> addresses.
> 
OK.

>> + * @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;
> 
> 
> Hmm, I don't see spi_nor_write_disable().
> 
> Either both spi_nor_write_enable() and spi_nor_write_disable() should be
> responsibility of caller or both needs to be taken care of here.
> 
> And the same needs to be documented in the function description
> 
OK. I will call both from outside.

>> +
>> +	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'
>>

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

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

* Re: [PATCH v5 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups
  2021-05-27 13:20   ` Vignesh Raghavendra
@ 2021-06-01  8:09     ` Takahiro Kuwano
  0 siblings, 0 replies; 11+ messages in thread
From: Takahiro Kuwano @ 2021-06-01  8:09 UTC (permalink / raw)
  To: Vignesh Raghavendra, linux-mtd
  Cc: tudor.ambarus, miquel.raynal, richard, p.yadav, Bacem.Daassi,
	Takahiro Kuwano

Hi Vignesh,

On 5/27/2021 10:20 PM, Vignesh Raghavendra wrote:
> 
> 
> On 4/27/21 12:39 PM, tkuw584924@gmail.com wrote:
>> 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 v5:
>>   - Add NO_CHIP_ERASE flag to S25HL02GT and S25HS02GT
>>
>> 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 | 117 +++++++++++++++++++++++++++++++++
>>  1 file changed, 117 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
>> index 79d3249ed0aa..b3b006874b4f 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);
> 
> Number of dies should be part of struct flash_info
> Die size can then be derived from total_size / die_num
> 
> 
>> +}
>> +
>> +static int s25hx_t_mdp_ready(struct spi_nor *nor)
>> +{
>> +	return spansion_mdp_ready(nor, 0, SZ_128M);
> 
> Hmm, is it necessary to poll second die if writes/erases were targeted
> to first die only?
> 
> Please split multi die support into a separate series. SPI NOR core
> should be aware of multiple dies and thus call poll on each of the dies
> separately.
> 
> This will also allow base S25hx support to be accepted faster.
> 
OK, I will do it.

>> +}
>> +
>> +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;
> 
> Don't we need to check if flash size is actual >16M before enabling 4
> byte addressing?
> 
S25HL/HS-T family has >16M devices only so we don't have to check it.

>> +
>> +	/* 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;
>> +}
>> +
> Regards
> Vignesh
> 

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

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

end of thread, other threads:[~2021-06-01  8:10 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-27  7:08 [PATCH v5 0/6] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t tkuw584924
2021-04-27  7:08 ` [PATCH v5 1/6] mtd: spi-nor: core: Add the ->ready() hook tkuw584924
2021-04-27  7:08 ` [PATCH v5 2/6] mtd: spi-nor: core: Expose spi_nor_clear_sr() to manufacturer drivers tkuw584924
2021-04-27  7:08 ` [PATCH v5 3/6] mtd: spi-nor: spansion: Add support for Read/Write Any Register tkuw584924
2021-05-27 11:06   ` Vignesh Raghavendra
2021-06-01  7:53     ` Takahiro Kuwano
2021-04-27  7:08 ` [PATCH v5 4/6] mtd: spi-nor: spansion: Add support for volatile QE bit tkuw584924
2021-04-27  7:09 ` [PATCH v5 5/6] mtd: spi-nor: spansion: Add status check for multi-die parts tkuw584924
2021-04-27  7:09 ` [PATCH v5 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups tkuw584924
2021-05-27 13:20   ` Vignesh Raghavendra
2021-06-01  8:09     ` Takahiro Kuwano

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