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

[PATCH v7 0/3] 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)

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

Tested on Xilinx Zynq-7000 FPGA board.

Device ID and SFDP dumps:
------------------------------------------------------------
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
s25hl512t
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
342a1a0f0390
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
spansion
zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
53464450080103ff00000114000100ff84000102500100ff81000116c801
00ff8700011c580100ffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffe720faffffffff1f48eb086b00ff
88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b82e7ffe3
ec031c608a857a75f766805c8cd6ddfff938f8a1000000000000bc000000
0000f7f5ffff7b920ffe21ffffdc0000800000000000c0ffc3ebc8ffe3eb
00650090060500a10065009600650095716503d0716503d000000000b02e
000088a489aa716503967165039600000000000000000000000000000000
000000000000000000000000000000000000000000000000716505d57165
05d50000a015fc65ff0804008000fc65ff4002008000fd65ff0402008000
fe0002fff1ff0100f8ff0100f8fffb03fe0302fff8fffb03f8ff0100f1ff
0100fe0104fff1ff0000f8ff0200f8fff703f8ff0200f1ff0000ff0400ff
f8ffff03
zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
8a0aa90112e154ae3a797df2c211ef61  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
------------------------------------------------------------

------------------------------------------------------------
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
s25hl01gt
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
342a1b0f0390
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
spansion
zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
53464450080103ff00000114000100ff84000102500100ff81000116c801
00ff8700011c580100ffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffe720faffffffff3f48eb086b00ff
88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b82e7ffe6
ec031c608a857a75f766805c8cd6ddfff938f8a1000000000000bc000000
0000f7f5ffff7b920ffe21ffffdc0000800000000000c0ffc3ebc8ffe3eb
00650090060500a10065009600650095716503d0716503d000000000b02e
000088a489aa716503967165039600000000000000000000000000000000
000000000000000000000000000000000000000000000000716505d57165
05d50000a015fc65ff0804008000fc65ff4002008000fd65ff0402008000
fe0002fff1ff0100f8ff0100f8fffb07fe0302fff8fffb07f8ff0100f1ff
0100fe0104fff1ff0000f8ff0200f8fff707f8ff0200f1ff0000ff0400ff
f8ffff07
zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
1ad5a0d7d7e0e656986c1e678c416a7e  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
------------------------------------------------------------

------------------------------------------------------------
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
s25hs512t
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
342b1a0f0390
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
spansion
zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
53464450080103ff00000114000100ff84000102500100ff81000116c801
00ff8700011c580100ffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffe720faffffffff1f48eb086b00ff
88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b91e8ffe3
ec031c608a857a75f766805c84d6ddfff938f8a100000000000080000000
0000f7f5ffff7b920ffe20ffffd80000800000000000c0ffc3ebc8ffe3eb
00650090060500a10065009600650095716503d0716503d000000000b02e
000088a489aa716503967165039600000000000000000000000000000000
000000000000000000000000000000000000000000000000716505d57165
05d50000ee72fc65ff0804008000fc65ff4002008000fd65ff0402008000
fe0002fff1ff0100f8ff0100f8fffb03fe0302fff8fffb03f8ff0100f1ff
0100fe0104fff1ff0100f8ff0200f8fff703f8ff0200f1ff0100ff0400ff
f8ffff03
zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
f17d9e784602187a0933edec3688e30f  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
------------------------------------------------------------

------------------------------------------------------------
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
s25hs01gt
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
342b1b0f0390
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
spansion
zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
53464450080103ff00000114000100ff84000102500100ff81000116c801
00ff8700011c580100ffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffe720faffffffff3f48eb086b00ff
88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b82e7ffe6
ec031c608a857a75f766805c8cd6ddfff938f8a1000000000000bc000000
0000f7f5ffff7b920ffe21ffffdc0000800000000000c0ffc3ebc8ffe3eb
00650090060500a10065009600650095716503d0716503d000000000b02e
000088a489aa716503967165039600000000000000000000000000000000
000000000000000000000000000000000000000000000000716505d57165
05d50000a015fc65ff0804008000fc65ff4002008000fd65ff0402008000
fe0002fff1ff0100f8ff0100f8fffb07fe0302fff8fffb07f8ff0100f1ff
0100fe0104fff1ff0000f8ff0200f8fff707f8ff0200f1ff0000ff0400ff
f8ffff07
zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
1ad5a0d7d7e0e656986c1e678c416a7e  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
------------------------------------------------------------


---
Changes in v7:
  - Some changes were missing in v6 patch. Fix it

Changes in v6:
  - Remove 2Gb dual die package parts and related changes to split mulit
    die package support into another series of patches  

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 (3):
  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 s25hl-t/s25hs-t IDs and fixups

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

-- 
2.25.1


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

[PATCH v7 1/3] 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.

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

Changes in v6:
  - Add helper functions for controller_ops
  - Add 'reg_addr_width' parameter to spansion_read/write_any_reg()
  - Remove spi_nor_write_enable() from spansion_write_any_reg() and modified
    function header comment

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

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index ee82dcd75310..5e7e62704e11 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -19,6 +19,148 @@
 #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS	0
 #define SPINOR_OP_CYPRESS_RD_FAST		0xee
 
+/**
+ * controller_ops_read_any_reg() - Read Any Register using controller_ops.
+ * @nor:		pointer to a 'struct spi_nor'
+ * @reg_addr:		register address
+ * @reg_addr_width:	number of address bytes
+ * @reg_dummy:		number of dummy cycles for register read
+ * @reg_val:		pointer to a buffer where the register value is copied
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int controller_ops_read_any_reg(struct spi_nor *nor, u32 reg_addr,
+				       u8 reg_addr_width, u8 reg_dummy,
+				       u8 *reg_val)
+{
+	ssize_t ret;
+	enum spi_nor_protocol proto = nor->read_proto;
+	u8 opcode = nor->read_opcode;
+	u8 dummy = nor->read_dummy;
+	u8 addr_width = nor->addr_width;
+
+	nor->read_opcode = SPINOR_OP_RD_ANY_REG;
+	nor->read_dummy = reg_dummy;
+	nor->read_proto = nor->reg_proto;
+	nor->addr_width = reg_addr_width;
+
+	ret = nor->controller_ops->read(nor, reg_addr, 1, reg_val);
+
+	nor->read_opcode = opcode;
+	nor->read_dummy = dummy;
+	nor->read_proto = proto;
+	nor->addr_width = addr_width;
+
+	if (ret < 0)
+		return ret;
+	if (ret != 1)
+		return -EIO;
+
+	return 0;
+}
+
+/**
+ * controller_ops_write_any_reg() - Write Any Register using controller_ops.
+ * @nor:		pointer to a 'struct spi_nor'
+ * @reg_addr:		register address
+ * @reg_addr_width:	number of address bytes
+ * @reg_val:		register value to be written
+ * *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int controller_ops_write_any_reg(struct spi_nor *nor, u32 reg_addr,
+					u8 reg_addr_width, u8 reg_val)
+{
+	ssize_t ret;
+	enum spi_nor_protocol proto = nor->write_proto;
+	u8 opcode = nor->program_opcode;
+	u8 addr_width = nor->addr_width;
+
+	nor->program_opcode = SPINOR_OP_WR_ANY_REG;
+	nor->write_proto = nor->reg_proto;
+	nor->addr_width = reg_addr_width;
+
+	ret = nor->controller_ops->write(nor, reg_addr, 1, &reg_val);
+
+	nor->program_opcode = opcode;
+	nor->write_proto = proto;
+	nor->addr_width = addr_width;
+
+	if (ret < 0)
+		return ret;
+	if (ret != 1)
+		return -EIO;
+
+	return 0;
+}
+
+/**
+ * spansion_read_any_reg() - Read Any Register.
+ * @nor:		pointer to a 'struct spi_nor'
+ * @reg_addr:		register address
+ * @reg_addr_width:	number of address bytes
+ * @reg_dummy:		number of dummy cycles for register read
+ * @reg_val:		pointer to a buffer where the register value is copied
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spansion_read_any_reg(struct spi_nor *nor, u32 reg_addr,
+				 u8 reg_addr_width, u8 reg_dummy, u8 *reg_val)
+{
+	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(reg_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);
+
+		/* convert the dummy cycles to the number of bytes */
+		op.dummy.nbytes = (reg_dummy * op.dummy.buswidth) / 8;
+		if (spi_nor_protocol_is_dtr(nor->reg_proto))
+			op.dummy.nbytes *= 2;
+
+		return spi_mem_exec_op(nor->spimem, &op);
+	}
+
+	return controller_ops_read_any_reg(nor, reg_addr, reg_addr_width,
+					   reg_dummy, reg_val);
+}
+
+/**
+ * spansion_write_any_reg() - Write Any Register.
+ * @nor:		pointer to a 'struct spi_nor'
+ * @reg_addr:		register address
+ * @reg_addr_width:	number of address bytes
+ * @reg_val:		register value to be written
+ *
+ * spi_nor_write_enable() and spi_nor_write_disable() need to be called before
+ * and after this function. Caller also need to poll status for non-volatile
+ * register. No need to poll status for volatile registers since volatile
+ * register write will be effective immediately after the operation.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spansion_write_any_reg(struct spi_nor *nor, u32 reg_addr,
+				  u8 reg_addr_width, u8 reg_val)
+{
+	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(reg_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);
+
+		return spi_mem_exec_op(nor->spimem, &op);
+	}
+
+	return controller_ops_write_any_reg(nor, reg_addr, reg_addr_width,
+					    reg_val);
+}
+
 /**
  * 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 v7 2/3] 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.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
Changes in v7:
  - Add missing macro definitions in v6
  
Changes in v6:
  - Remove multi die package support

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

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 5e7e62704e11..ca50e77b4220 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
@@ -161,6 +163,63 @@ static int spansion_write_any_reg(struct spi_nor *nor, u32 reg_addr,
 					    reg_val);
 }
 
+/**
+ * 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
+ *
+ * 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 reg_addr = SPINOR_REG_CYPRESS_CFR1V;
+	u8 cfr1v, cfr1v_written;
+	int ret;
+
+	ret = spansion_read_any_reg(nor, reg_addr, nor->addr_width, reg_dummy,
+				    &cfr1v);
+	if (ret)
+		return ret;
+
+	if (cfr1v & SPINOR_REG_CYPRESS_CFR1V_QUAD_EN)
+		return 0;
+
+	ret = spi_nor_write_enable(nor);
+	if (ret)
+		return ret;
+
+	/* Update the Quad Enable bit. */
+	cfr1v |= SPINOR_REG_CYPRESS_CFR1V_QUAD_EN;
+
+	ret = spansion_write_any_reg(nor, reg_addr, nor->addr_width, cfr1v);
+
+	if (ret)
+		return ret;
+
+	cfr1v_written = cfr1v;
+
+	/* Read back and check it. */
+	ret = spansion_read_any_reg(nor, reg_addr, nor->addr_width, 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 v7 3/3] 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. 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>
---
Changes in v7:
  - Add missing device info table in v6
  
Changes in v6:
  - Remove 2Gb multi die pacakge support

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

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index ca50e77b4220..c7170477e706 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -220,6 +220,84 @@ static int spansion_quad_enable_volatile(struct spi_nor *nor, u8 reg_dummy)
 	return 0;
 }
 
+static int s25hx_t_quad_enable(struct spi_nor *nor)
+{
+	int ret = spansion_quad_enable_volatile(nor, 0);
+
+	/* Reset WEL bit in any error cases */
+	spi_nor_write_disable(nor);
+
+	return ret;
+}
+
+static int
+s25hx_t_post_bfpt_fixups(struct spi_nor *nor,
+			 const struct sfdp_parameter_header *bfpt_header,
+			 const struct sfdp_bfpt *bfpt)
+{
+	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 */
+	nor->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.
+	 */
+	ret = spansion_read_any_reg(nor, addr + SPINOR_REG_CYPRESS_CFR3V,
+				    nor->addr_width, 0, &cfr3v);
+	if (ret)
+		return ret;
+
+	if (!(cfr3v & SPINOR_REG_CYPRESS_CFR3V_PGSZ))
+		nor->params->page_size = 256;
+	else
+		nor->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
+	 *  -------------------------------------------------------------------
+	 */
+}
+
+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'
@@ -468,6 +546,18 @@ 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 },
+	{ "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 },
 	{ "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 v7 0/3] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t

[Takahiro Kuwano]

Hello,

Any feedback on this series of patches?

On 7/19/2021 5:03 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.
> 
> The summary datasheets can be found in the following links.
> https://www.cypress.com/file/424146/download (256Mb/512Mb/1Gb, single 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
> 
> Tested on Xilinx Zynq-7000 FPGA board.
> 
> Device ID and SFDP dumps:
> ------------------------------------------------------------
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
> s25hl512t
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
> 342a1a0f0390
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
> spansion
> zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 53464450080103ff00000114000100ff84000102500100ff81000116c801
> 00ff8700011c580100ffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffe720faffffffff1f48eb086b00ff
> 88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b82e7ffe3
> ec031c608a857a75f766805c8cd6ddfff938f8a1000000000000bc000000
> 0000f7f5ffff7b920ffe21ffffdc0000800000000000c0ffc3ebc8ffe3eb
> 00650090060500a10065009600650095716503d0716503d000000000b02e
> 000088a489aa716503967165039600000000000000000000000000000000
> 000000000000000000000000000000000000000000000000716505d57165
> 05d50000a015fc65ff0804008000fc65ff4002008000fd65ff0402008000
> fe0002fff1ff0100f8ff0100f8fffb03fe0302fff8fffb03f8ff0100f1ff
> 0100fe0104fff1ff0000f8ff0200f8fff703f8ff0200f1ff0000ff0400ff
> f8ffff03
> zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 8a0aa90112e154ae3a797df2c211ef61  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> ------------------------------------------------------------
> 
> ------------------------------------------------------------
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
> s25hl01gt
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
> 342a1b0f0390
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
> spansion
> zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 53464450080103ff00000114000100ff84000102500100ff81000116c801
> 00ff8700011c580100ffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffe720faffffffff3f48eb086b00ff
> 88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b82e7ffe6
> ec031c608a857a75f766805c8cd6ddfff938f8a1000000000000bc000000
> 0000f7f5ffff7b920ffe21ffffdc0000800000000000c0ffc3ebc8ffe3eb
> 00650090060500a10065009600650095716503d0716503d000000000b02e
> 000088a489aa716503967165039600000000000000000000000000000000
> 000000000000000000000000000000000000000000000000716505d57165
> 05d50000a015fc65ff0804008000fc65ff4002008000fd65ff0402008000
> fe0002fff1ff0100f8ff0100f8fffb07fe0302fff8fffb07f8ff0100f1ff
> 0100fe0104fff1ff0000f8ff0200f8fff707f8ff0200f1ff0000ff0400ff
> f8ffff07
> zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 1ad5a0d7d7e0e656986c1e678c416a7e  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> ------------------------------------------------------------
> 
> ------------------------------------------------------------
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
> s25hs512t
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
> 342b1a0f0390
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
> spansion
> zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 53464450080103ff00000114000100ff84000102500100ff81000116c801
> 00ff8700011c580100ffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffe720faffffffff1f48eb086b00ff
> 88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b91e8ffe3
> ec031c608a857a75f766805c84d6ddfff938f8a100000000000080000000
> 0000f7f5ffff7b920ffe20ffffd80000800000000000c0ffc3ebc8ffe3eb
> 00650090060500a10065009600650095716503d0716503d000000000b02e
> 000088a489aa716503967165039600000000000000000000000000000000
> 000000000000000000000000000000000000000000000000716505d57165
> 05d50000ee72fc65ff0804008000fc65ff4002008000fd65ff0402008000
> fe0002fff1ff0100f8ff0100f8fffb03fe0302fff8fffb03f8ff0100f1ff
> 0100fe0104fff1ff0100f8ff0200f8fff703f8ff0200f1ff0100ff0400ff
> f8ffff03
> zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> f17d9e784602187a0933edec3688e30f  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> ------------------------------------------------------------
> 
> ------------------------------------------------------------
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
> s25hs01gt
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
> 342b1b0f0390
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
> spansion
> zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 53464450080103ff00000114000100ff84000102500100ff81000116c801
> 00ff8700011c580100ffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffe720faffffffff3f48eb086b00ff
> 88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b82e7ffe6
> ec031c608a857a75f766805c8cd6ddfff938f8a1000000000000bc000000
> 0000f7f5ffff7b920ffe21ffffdc0000800000000000c0ffc3ebc8ffe3eb
> 00650090060500a10065009600650095716503d0716503d000000000b02e
> 000088a489aa716503967165039600000000000000000000000000000000
> 000000000000000000000000000000000000000000000000716505d57165
> 05d50000a015fc65ff0804008000fc65ff4002008000fd65ff0402008000
> fe0002fff1ff0100f8ff0100f8fffb07fe0302fff8fffb07f8ff0100f1ff
> 0100fe0104fff1ff0000f8ff0200f8fff707f8ff0200f1ff0000ff0400ff
> f8ffff07
> zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 1ad5a0d7d7e0e656986c1e678c416a7e  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> ------------------------------------------------------------
> 
> 
> ---
> Changes in v7:
>   - Some changes were missing in v6 patch. Fix it
> 
> Changes in v6:
>   - Remove 2Gb dual die package parts and related changes to split mulit
>     die package support into another series of patches  
> 
> 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 (3):
>   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 s25hl-t/s25hs-t IDs and fixups
> 
>  drivers/mtd/spi-nor/spansion.c | 291 +++++++++++++++++++++++++++++++++
>  1 file changed, 291 insertions(+)
> 

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

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

[Tudor.Ambarus]

On 11/17/21 11:23 AM, Takahiro Kuwano wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hello,
> 
> Any feedback on this series of patches?

Hi, Takahiro,

We're in the process of fixing how the flash_info flags are used.
We already have an agreement on how things are going to look like,
I just need a bit of time to submit a new version of
https://lore.kernel.org/linux-mtd/20211029172633.886453-1-tudor.ambarus@microchip.com/

Until we pave the way on how to handle the flags, we don't accept
new flash additions, otherwise the flash_info entries will soon
become a maintenance burden. I'll soon finish the work, I won't
stall you much longer.

Cheers,
ta

> 
> On 7/19/2021 5:03 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.
>>
>> The summary datasheets can be found in the following links.
>> https://www.cypress.com/file/424146/download (256Mb/512Mb/1Gb, single 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
>>
>> Tested on Xilinx Zynq-7000 FPGA board.
>>
>> Device ID and SFDP dumps:
>> ------------------------------------------------------------
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
>> s25hl512t
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
>> 342a1a0f0390
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
>> spansion
>> zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> 53464450080103ff00000114000100ff84000102500100ff81000116c801
>> 00ff8700011c580100ffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffe720faffffffff1f48eb086b00ff
>> 88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b82e7ffe3
>> ec031c608a857a75f766805c8cd6ddfff938f8a1000000000000bc000000
>> 0000f7f5ffff7b920ffe21ffffdc0000800000000000c0ffc3ebc8ffe3eb
>> 00650090060500a10065009600650095716503d0716503d000000000b02e
>> 000088a489aa716503967165039600000000000000000000000000000000
>> 000000000000000000000000000000000000000000000000716505d57165
>> 05d50000a015fc65ff0804008000fc65ff4002008000fd65ff0402008000
>> fe0002fff1ff0100f8ff0100f8fffb03fe0302fff8fffb03f8ff0100f1ff
>> 0100fe0104fff1ff0000f8ff0200f8fff703f8ff0200f1ff0000ff0400ff
>> f8ffff03
>> zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> 8a0aa90112e154ae3a797df2c211ef61  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> ------------------------------------------------------------
>>
>> ------------------------------------------------------------
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
>> s25hl01gt
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
>> 342a1b0f0390
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
>> spansion
>> zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> 53464450080103ff00000114000100ff84000102500100ff81000116c801
>> 00ff8700011c580100ffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffe720faffffffff3f48eb086b00ff
>> 88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b82e7ffe6
>> ec031c608a857a75f766805c8cd6ddfff938f8a1000000000000bc000000
>> 0000f7f5ffff7b920ffe21ffffdc0000800000000000c0ffc3ebc8ffe3eb
>> 00650090060500a10065009600650095716503d0716503d000000000b02e
>> 000088a489aa716503967165039600000000000000000000000000000000
>> 000000000000000000000000000000000000000000000000716505d57165
>> 05d50000a015fc65ff0804008000fc65ff4002008000fd65ff0402008000
>> fe0002fff1ff0100f8ff0100f8fffb07fe0302fff8fffb07f8ff0100f1ff
>> 0100fe0104fff1ff0000f8ff0200f8fff707f8ff0200f1ff0000ff0400ff
>> f8ffff07
>> zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> 1ad5a0d7d7e0e656986c1e678c416a7e  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> ------------------------------------------------------------
>>
>> ------------------------------------------------------------
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
>> s25hs512t
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
>> 342b1a0f0390
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
>> spansion
>> zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> 53464450080103ff00000114000100ff84000102500100ff81000116c801
>> 00ff8700011c580100ffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffe720faffffffff1f48eb086b00ff
>> 88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b91e8ffe3
>> ec031c608a857a75f766805c84d6ddfff938f8a100000000000080000000
>> 0000f7f5ffff7b920ffe20ffffd80000800000000000c0ffc3ebc8ffe3eb
>> 00650090060500a10065009600650095716503d0716503d000000000b02e
>> 000088a489aa716503967165039600000000000000000000000000000000
>> 000000000000000000000000000000000000000000000000716505d57165
>> 05d50000ee72fc65ff0804008000fc65ff4002008000fd65ff0402008000
>> fe0002fff1ff0100f8ff0100f8fffb03fe0302fff8fffb03f8ff0100f1ff
>> 0100fe0104fff1ff0100f8ff0200f8fff703f8ff0200f1ff0100ff0400ff
>> f8ffff03
>> zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> f17d9e784602187a0933edec3688e30f  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> ------------------------------------------------------------
>>
>> ------------------------------------------------------------
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
>> s25hs01gt
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
>> 342b1b0f0390
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
>> spansion
>> zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> 53464450080103ff00000114000100ff84000102500100ff81000116c801
>> 00ff8700011c580100ffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffe720faffffffff3f48eb086b00ff
>> 88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b82e7ffe6
>> ec031c608a857a75f766805c8cd6ddfff938f8a1000000000000bc000000
>> 0000f7f5ffff7b920ffe21ffffdc0000800000000000c0ffc3ebc8ffe3eb
>> 00650090060500a10065009600650095716503d0716503d000000000b02e
>> 000088a489aa716503967165039600000000000000000000000000000000
>> 000000000000000000000000000000000000000000000000716505d57165
>> 05d50000a015fc65ff0804008000fc65ff4002008000fd65ff0402008000
>> fe0002fff1ff0100f8ff0100f8fffb07fe0302fff8fffb07f8ff0100f1ff
>> 0100fe0104fff1ff0000f8ff0200f8fff707f8ff0200f1ff0000ff0400ff
>> f8ffff07
>> zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> 1ad5a0d7d7e0e656986c1e678c416a7e  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> ------------------------------------------------------------
>>
>>
>> ---
>> Changes in v7:
>>   - Some changes were missing in v6 patch. Fix it
>>
>> Changes in v6:
>>   - Remove 2Gb dual die package parts and related changes to split mulit
>>     die package support into another series of patches
>>
>> 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 (3):
>>   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 s25hl-t/s25hs-t IDs and fixups
>>
>>  drivers/mtd/spi-nor/spansion.c | 291 +++++++++++++++++++++++++++++++++
>>  1 file changed, 291 insertions(+)
>>

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

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

[Takahiro Kuwano]

Hi Tudor,

On 11/19/2021 6:09 PM, Tudor.Ambarus@microchip.com wrote:
> On 11/17/21 11:23 AM, Takahiro Kuwano wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Hello,
>>
>> Any feedback on this series of patches?
> 
> Hi, Takahiro,
> 
> We're in the process of fixing how the flash_info flags are used.
> We already have an agreement on how things are going to look like,
> I just need a bit of time to submit a new version of
> https://lore.kernel.org/linux-mtd/20211029172633.886453-1-tudor.ambarus@microchip.com/
> 
> Until we pave the way on how to handle the flags, we don't accept
> new flash additions, otherwise the flash_info entries will soon
> become a maintenance burden. I'll soon finish the work, I won't
> stall you much longer.
> 
> Cheers,
> ta
> 
I saw the changes about flash_info were accepted and pulled.
Please let me know if it would be better to rebase this series with v5.17-rc1 in advance.

Best Regards,
Takahiro

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

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

[Tudor.Ambarus]

On 1/26/22 11:41, Takahiro Kuwano wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor,

Hi!

> 
> On 11/19/2021 6:09 PM, Tudor.Ambarus@microchip.com wrote:
>> On 11/17/21 11:23 AM, Takahiro Kuwano wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Hello,
>>>
>>> Any feedback on this series of patches?
>>
>> Hi, Takahiro,
>>
>> We're in the process of fixing how the flash_info flags are used.
>> We already have an agreement on how things are going to look like,
>> I just need a bit of time to submit a new version of
>> https://lore.kernel.org/linux-mtd/20211029172633.886453-1-tudor.ambarus@microchip.com/
>>
>> Until we pave the way on how to handle the flags, we don't accept
>> new flash additions, otherwise the flash_info entries will soon
>> become a maintenance burden. I'll soon finish the work, I won't
>> stall you much longer.
>>
>> Cheers,
>> ta
>>
> I saw the changes about flash_info were accepted and pulled.
> Please let me know if it would be better to rebase this series with v5.17-rc1 in advance.
> 

I've quickly skimmed through this series and it seems fine. Please rebase, yes,
meanwhile I'll take a second look.

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

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

[Tudor.Ambarus]

On 7/19/21 11:03, 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>
> 
> Some of Spansion/Cypress chips support Read/Write Any Register commands.
> These commands are mainly used to write volatile registers.
> 
> 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 v7:
>   - No change
> 
> Changes in v6:
>   - Add helper functions for controller_ops
>   - Add 'reg_addr_width' parameter to spansion_read/write_any_reg()
>   - Remove spi_nor_write_enable() from spansion_write_any_reg() and modified
>     function header comment
> 
> 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 | 142 +++++++++++++++++++++++++++++++++
>  1 file changed, 142 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index ee82dcd75310..5e7e62704e11 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -19,6 +19,148 @@
>  #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS    0
>  #define SPINOR_OP_CYPRESS_RD_FAST              0xee
> 
> +/**
> + * controller_ops_read_any_reg() - Read Any Register using controller_ops.
> + * @nor:               pointer to a 'struct spi_nor'
> + * @reg_addr:          register address
> + * @reg_addr_width:    number of address bytes
> + * @reg_dummy:         number of dummy cycles for register read
> + * @reg_val:           pointer to a buffer where the register value is copied
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int controller_ops_read_any_reg(struct spi_nor *nor, u32 reg_addr,
> +                                      u8 reg_addr_width, u8 reg_dummy,
> +                                      u8 *reg_val)
> +{
> +       ssize_t ret;
> +       enum spi_nor_protocol proto = nor->read_proto;
> +       u8 opcode = nor->read_opcode;
> +       u8 dummy = nor->read_dummy;
> +       u8 addr_width = nor->addr_width;
> +
> +       nor->read_opcode = SPINOR_OP_RD_ANY_REG;
> +       nor->read_dummy = reg_dummy;
> +       nor->read_proto = nor->reg_proto;
> +       nor->addr_width = reg_addr_width;
> +
> +       ret = nor->controller_ops->read(nor, reg_addr, 1, reg_val);
> +
> +       nor->read_opcode = opcode;
> +       nor->read_dummy = dummy;
> +       nor->read_proto = proto;
> +       nor->addr_width = addr_width;
> +
> +       if (ret < 0)
> +               return ret;
> +       if (ret != 1)
> +               return -EIO;
> +
> +       return 0;
> +}
> +
> +/**
> + * controller_ops_write_any_reg() - Write Any Register using controller_ops.
> + * @nor:               pointer to a 'struct spi_nor'
> + * @reg_addr:          register address
> + * @reg_addr_width:    number of address bytes
> + * @reg_val:           register value to be written
> + * *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int controller_ops_write_any_reg(struct spi_nor *nor, u32 reg_addr,
> +                                       u8 reg_addr_width, u8 reg_val)
> +{
> +       ssize_t ret;
> +       enum spi_nor_protocol proto = nor->write_proto;
> +       u8 opcode = nor->program_opcode;
> +       u8 addr_width = nor->addr_width;
> +
> +       nor->program_opcode = SPINOR_OP_WR_ANY_REG;
> +       nor->write_proto = nor->reg_proto;
> +       nor->addr_width = reg_addr_width;
> +
> +       ret = nor->controller_ops->write(nor, reg_addr, 1, &reg_val);
> +
> +       nor->program_opcode = opcode;
> +       nor->write_proto = proto;
> +       nor->addr_width = addr_width;
> +
> +       if (ret < 0)
> +               return ret;
> +       if (ret != 1)
> +               return -EIO;
> +
> +       return 0;
> +}
> +
> +/**
> + * spansion_read_any_reg() - Read Any Register.
> + * @nor:               pointer to a 'struct spi_nor'
> + * @reg_addr:          register address
> + * @reg_addr_width:    number of address bytes
> + * @reg_dummy:         number of dummy cycles for register read
> + * @reg_val:           pointer to a buffer where the register value is copied
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spansion_read_any_reg(struct spi_nor *nor, u32 reg_addr,
> +                                u8 reg_addr_width, u8 reg_dummy, u8 *reg_val)

how about making this a generic core method? I'm sure there are other SPI NOR
vendors that use registers indexed by address. How about introducing:

struct spi_nor_reg {
	u32 addr;
	u8 addr_width;
	u8 dummy;
	u8 opcode;
	u8 val;
};

and passing a pointer to this struct as argument.

> +{
> +       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(reg_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);
> +
> +               /* convert the dummy cycles to the number of bytes */
> +               op.dummy.nbytes = (reg_dummy * op.dummy.buswidth) / 8;
> +               if (spi_nor_protocol_is_dtr(nor->reg_proto))
> +                       op.dummy.nbytes *= 2;
> +
> +               return spi_mem_exec_op(nor->spimem, &op);
> +       }
> +
else
	return -EOPNOTSUPP;

we'd like to move all the SPI NOR controller drivers under SPI MEM, let's
not add support for them.

> +       return controller_ops_read_any_reg(nor, reg_addr, reg_addr_width,
> +                                          reg_dummy, reg_val);
> +}
> +
> +/**
> + * spansion_write_any_reg() - Write Any Register.
> + * @nor:               pointer to a 'struct spi_nor'
> + * @reg_addr:          register address
> + * @reg_addr_width:    number of address bytes
> + * @reg_val:           register value to be written
> + *
> + * spi_nor_write_enable() and spi_nor_write_disable() need to be called before
> + * and after this function. Caller also need to poll status for non-volatile
> + * register. No need to poll status for volatile registers since volatile
> + * register write will be effective immediately after the operation.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spansion_write_any_reg(struct spi_nor *nor, u32 reg_addr,
> +                                 u8 reg_addr_width, u8 reg_val)
> +{
> +       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(reg_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);
> +
> +               return spi_mem_exec_op(nor->spimem, &op);
> +       }
> +
> +       return controller_ops_write_any_reg(nor, reg_addr, reg_addr_width,
> +                                           reg_val);
> +}
> +
>  /**
>   * 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/

Re: [PATCH v7 2/3] mtd: spi-nor: spansion: Add support for volatile QE bit

[Tudor.Ambarus]

On 7/19/21 11:03, 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>
> 
> 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.
> 
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> ---
> Changes in v7:
>   - Add missing macro definitions in v6
> 
> Changes in v6:
>   - Remove multi die package support
> 
> 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 | 59 ++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index 5e7e62704e11..ca50e77b4220 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
> @@ -161,6 +163,63 @@ static int spansion_write_any_reg(struct spi_nor *nor, u32 reg_addr,
>                                             reg_val);
>  }
> 
> +/**
> + * 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
> + *
> + * 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

Why do you duplicate the setting of QE bit in CFR1V if QE bit is already set in
CFR1NV? Does the Volatile Register has a higher priority than the Non-Volatile one?
What happens when QE is zero in CFR1NV and QE is one in CFR1V?

> + * 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 reg_addr = SPINOR_REG_CYPRESS_CFR1V;
> +       u8 cfr1v, cfr1v_written;
> +       int ret;
> +
> +       ret = spansion_read_any_reg(nor, reg_addr, nor->addr_width, reg_dummy,
> +                                   &cfr1v);
> +       if (ret)
> +               return ret;
> +
> +       if (cfr1v & SPINOR_REG_CYPRESS_CFR1V_QUAD_EN)
> +               return 0;
> +
> +       ret = spi_nor_write_enable(nor);
> +       if (ret)
> +               return ret;
> +
> +       /* Update the Quad Enable bit. */
> +       cfr1v |= SPINOR_REG_CYPRESS_CFR1V_QUAD_EN;
> +
> +       ret = spansion_write_any_reg(nor, reg_addr, nor->addr_width, cfr1v);
you'll need to pass nor->bouncebuf and not variables on stack, they are not DMA-able.

> +
you have an extra blank line here
> +       if (ret)
> +               return ret;
> +
> +       cfr1v_written = cfr1v;
> +
> +       /* Read back and check it. */
> +       ret = spansion_read_any_reg(nor, reg_addr, nor->addr_width, 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/

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

[Tudor.Ambarus]

On 7/19/21 11:03, 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.
> 
> For the single-die package parts (512Mb and 1Gb), only bottom 4KB and
> uniform sector sizes are supported. 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>
> ---
> Changes in v7:
>   - Add missing device info table in v6
> 
> Changes in v6:
>   - Remove 2Gb multi die pacakge support
> 
> 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 | 90 ++++++++++++++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index ca50e77b4220..c7170477e706 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -220,6 +220,84 @@ static int spansion_quad_enable_volatile(struct spi_nor *nor, u8 reg_dummy)
>         return 0;
>  }
> 
> +static int s25hx_t_quad_enable(struct spi_nor *nor)
> +{
> +       int ret = spansion_quad_enable_volatile(nor, 0);
> +
> +       /* Reset WEL bit in any error cases */

the comment suggests an if (ret) then spi_nor_write_disable(nor);
Should you call write disable in all cases or just in case of errors?

> +       spi_nor_write_disable(nor);
> +
> +       return ret;
> +}
> +
> +static int
> +s25hx_t_post_bfpt_fixups(struct spi_nor *nor,
> +                        const struct sfdp_parameter_header *bfpt_header,
> +                        const struct sfdp_bfpt *bfpt)
> +{
> +       int ret;
> +       u32 addr;
> +       u8 cfr3v;
> +
> +       ret = spi_nor_set_4byte_addr_mode(nor, true);

why don't you let the core handle this for you? Why do you call it here?
> +       if (ret)
> +               return ret;
> +       nor->addr_width = 4;

so SFDP gets the addr_width wrong?

> +
> +       /* Replace Quad Enable with volatile version */
> +       nor->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.
> +        */
> +       ret = spansion_read_any_reg(nor, addr + SPINOR_REG_CYPRESS_CFR3V,
> +                                   nor->addr_width, 0, &cfr3v);
> +       if (ret)
> +               return ret;
> +
> +       if (!(cfr3v & SPINOR_REG_CYPRESS_CFR3V_PGSZ))
> +               nor->params->page_size = 256;
> +       else
> +               nor->params->page_size = 512;

isn't the else case already covered by BFPT?

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

Is the BFPT wrong? Why do you update the num_mode_clocks here?
> +
> +       /* 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
> +        *  -------------------------------------------------------------------
> +        */

did you miss to update erases here?
> +}
> +
> +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'
> @@ -468,6 +546,18 @@ 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 },
> +       { "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 },
>         { "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 v7 1/3] mtd: spi-nor: spansion: Add support for Read/Write Any Register

[Pratyush Yadav]

On 28/01/22 09:43AM, Tudor.Ambarus@microchip.com wrote:
> On 7/19/21 11:03, 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>
> > 
> > Some of Spansion/Cypress chips support Read/Write Any Register commands.
> > These commands are mainly used to write volatile registers.
> > 
> > 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 v7:
> >   - No change
> > 
> > Changes in v6:
> >   - Add helper functions for controller_ops
> >   - Add 'reg_addr_width' parameter to spansion_read/write_any_reg()
> >   - Remove spi_nor_write_enable() from spansion_write_any_reg() and modified
> >     function header comment
> > 
> > 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 | 142 +++++++++++++++++++++++++++++++++
> >  1 file changed, 142 insertions(+)
> > 
> > +/**
> > + * spansion_read_any_reg() - Read Any Register.
> > + * @nor:               pointer to a 'struct spi_nor'
> > + * @reg_addr:          register address
> > + * @reg_addr_width:    number of address bytes
> > + * @reg_dummy:         number of dummy cycles for register read
> > + * @reg_val:           pointer to a buffer where the register value is copied
> > + *
> > + * Return: 0 on success, -errno otherwise.
> > + */
> > +static int spansion_read_any_reg(struct spi_nor *nor, u32 reg_addr,
> > +                                u8 reg_addr_width, u8 reg_dummy, u8 *reg_val)
> 
> how about making this a generic core method? I'm sure there are other SPI NOR
> vendors that use registers indexed by address. How about introducing:
> 
> struct spi_nor_reg {
> 	u32 addr;
> 	u8 addr_width;
> 	u8 dummy;
> 	u8 opcode;
> 	u8 val;
> };
> 
> and passing a pointer to this struct as argument.

Wouldn't it be simpler if they fill it in a struct spi_mem_op, and then 
just pass that in as a "template"?

> 
> > +{
> > +       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(reg_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);
> > +
> > +               /* convert the dummy cycles to the number of bytes */
> > +               op.dummy.nbytes = (reg_dummy * op.dummy.buswidth) / 8;
> > +               if (spi_nor_protocol_is_dtr(nor->reg_proto))
> > +                       op.dummy.nbytes *= 2;
> > +
> > +               return spi_mem_exec_op(nor->spimem, &op);
> > +       }
> > +
> else
> 	return -EOPNOTSUPP;
> 
> we'd like to move all the SPI NOR controller drivers under SPI MEM, let's
> not add support for them.

Sounds good to me.

> 
> > +       return controller_ops_read_any_reg(nor, reg_addr, reg_addr_width,
> > +                                          reg_dummy, reg_val);
> > +}

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

Re: [PATCH v7 2/3] mtd: spi-nor: spansion: Add support for volatile QE bit

[Pratyush Yadav]

On 28/01/22 10:12AM, Tudor.Ambarus@microchip.com wrote:
> On 7/19/21 11:03, 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>
> > 
> > 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.
> > 
> > Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> > ---
> > Changes in v7:
> >   - Add missing macro definitions in v6
> > 
> > Changes in v6:
> >   - Remove multi die package support
> > 
> > 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 | 59 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> > 
> > diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> > index 5e7e62704e11..ca50e77b4220 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 */

I have the Cypress S28 flash and this bit is marked as "reserved" there. 
Maybe we should have a flash family specific name? Dunno. But I guess it 
does not matter so much since it will be very localised anyway.

> >  #define SPINOR_REG_CYPRESS_CFR2V               0x00800003
> >  #define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24  0xb
> >  #define SPINOR_REG_CYPRESS_CFR3V               0x00800004
> > @@ -161,6 +163,63 @@ static int spansion_write_any_reg(struct spi_nor *nor, u32 reg_addr,
> >                                             reg_val);
> >  }
> > 
> > +/**
> > + * 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
> > + *
> > + * 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
> 
> Why do you duplicate the setting of QE bit in CFR1V if QE bit is already set in
> CFR1NV? Does the Volatile Register has a higher priority than the Non-Volatile one?
> What happens when QE is zero in CFR1NV and QE is one in CFR1V?

I have the Cypress S28 flash family, and on there I see that the 
volatile bit takes precedence over non-volatile bit*. At powerup data in 
NV bits is transferred to V bits to set their default state.

What I am more worried about is if quad mode is enabled in a 
non-volatile register (which should NEVER be done by Linux), will we be 
able to properly detect and initialize the flash? We certainly can't do 
it for 8D-8D-8D mode flashes.

* The exception is when the volatile bit is marked read-only. Then 
non-volatile bit takes precedence and volatile bit is ignored.

> 
> > + * 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 reg_addr = SPINOR_REG_CYPRESS_CFR1V;
> > +       u8 cfr1v, cfr1v_written;
> > +       int ret;
> > +
> > +       ret = spansion_read_any_reg(nor, reg_addr, nor->addr_width, reg_dummy,
> > +                                   &cfr1v);
> > +       if (ret)
> > +               return ret;
> > +
> > +       if (cfr1v & SPINOR_REG_CYPRESS_CFR1V_QUAD_EN)
> > +               return 0;
> > +
> > +       ret = spi_nor_write_enable(nor);
> > +       if (ret)
> > +               return ret;
> > +
> > +       /* Update the Quad Enable bit. */
> > +       cfr1v |= SPINOR_REG_CYPRESS_CFR1V_QUAD_EN;
> > +
> > +       ret = spansion_write_any_reg(nor, reg_addr, nor->addr_width, cfr1v);
> you'll need to pass nor->bouncebuf and not variables on stack, they are not DMA-able.

Good catch! I see there is function called object_is_on_stack(). 
spi_mem_exec_op() should probably run that check.

> 
> > +
> you have an extra blank line here
> > +       if (ret)
> > +               return ret;
> > +
> > +       cfr1v_written = cfr1v;
> > +
> > +       /* Read back and check it. */
> > +       ret = spansion_read_any_reg(nor, reg_addr, nor->addr_width, 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
> > 
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

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

[Tudor.Ambarus]

On 1/28/22 18:20, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 28/01/22 09:43AM, Tudor.Ambarus@microchip.com wrote:
>> On 7/19/21 11:03, 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>
>>>
>>> Some of Spansion/Cypress chips support Read/Write Any Register commands.
>>> These commands are mainly used to write volatile registers.
>>>
>>> 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 v7:
>>>   - No change
>>>
>>> Changes in v6:
>>>   - Add helper functions for controller_ops
>>>   - Add 'reg_addr_width' parameter to spansion_read/write_any_reg()
>>>   - Remove spi_nor_write_enable() from spansion_write_any_reg() and modified
>>>     function header comment
>>>
>>> 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 | 142 +++++++++++++++++++++++++++++++++
>>>  1 file changed, 142 insertions(+)
>>>
>>> +/**
>>> + * spansion_read_any_reg() - Read Any Register.
>>> + * @nor:               pointer to a 'struct spi_nor'
>>> + * @reg_addr:          register address
>>> + * @reg_addr_width:    number of address bytes
>>> + * @reg_dummy:         number of dummy cycles for register read
>>> + * @reg_val:           pointer to a buffer where the register value is copied
>>> + *
>>> + * Return: 0 on success, -errno otherwise.
>>> + */
>>> +static int spansion_read_any_reg(struct spi_nor *nor, u32 reg_addr,
>>> +                                u8 reg_addr_width, u8 reg_dummy, u8 *reg_val)
>>
>> how about making this a generic core method? I'm sure there are other SPI NOR
>> vendors that use registers indexed by address. How about introducing:
>>
>> struct spi_nor_reg {
>>       u32 addr;
>>       u8 addr_width;
>>       u8 dummy;
>>       u8 opcode;
>>       u8 val;
>> };
>>
>> and passing a pointer to this struct as argument.
> 
> Wouldn't it be simpler if they fill it in a struct spi_mem_op, and then
> just pass that in as a "template"?
> 
sure, sounds fine. I would like a generic method with less function parameters,
if possible. Let's see what Takahiro will propose.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[Takahiro Kuwano]

On 1/31/2022 4:50 PM, Tudor.Ambarus@microchip.com wrote:
> On 1/28/22 18:20, Pratyush Yadav wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 28/01/22 09:43AM, Tudor.Ambarus@microchip.com wrote:
>>> On 7/19/21 11:03, 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>
>>>>
>>>> Some of Spansion/Cypress chips support Read/Write Any Register commands.
>>>> These commands are mainly used to write volatile registers.
>>>>
>>>> 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 v7:
>>>>   - No change
>>>>
>>>> Changes in v6:
>>>>   - Add helper functions for controller_ops
>>>>   - Add 'reg_addr_width' parameter to spansion_read/write_any_reg()
>>>>   - Remove spi_nor_write_enable() from spansion_write_any_reg() and modified
>>>>     function header comment
>>>>
>>>> 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 | 142 +++++++++++++++++++++++++++++++++
>>>>  1 file changed, 142 insertions(+)
>>>>
>>>> +/**
>>>> + * spansion_read_any_reg() - Read Any Register.
>>>> + * @nor:               pointer to a 'struct spi_nor'
>>>> + * @reg_addr:          register address
>>>> + * @reg_addr_width:    number of address bytes
>>>> + * @reg_dummy:         number of dummy cycles for register read
>>>> + * @reg_val:           pointer to a buffer where the register value is copied
>>>> + *
>>>> + * Return: 0 on success, -errno otherwise.
>>>> + */
>>>> +static int spansion_read_any_reg(struct spi_nor *nor, u32 reg_addr,
>>>> +                                u8 reg_addr_width, u8 reg_dummy, u8 *reg_val)
>>>
>>> how about making this a generic core method? I'm sure there are other SPI NOR
>>> vendors that use registers indexed by address. How about introducing:
>>>
>>> struct spi_nor_reg {
>>>       u32 addr;
>>>       u8 addr_width;
>>>       u8 dummy;
>>>       u8 opcode;
>>>       u8 val;
>>> };
>>>
>>> and passing a pointer to this struct as argument.
>>
>> Wouldn't it be simpler if they fill it in a struct spi_mem_op, and then
>> just pass that in as a "template"?
>>
> sure, sounds fine. I would like a generic method with less function parameters,
> if possible. Let's see what Takahiro will propose.

So, I would propose...
In core, spi_nor_spimem_{read,write}_reg(struct spi_nor *, struct spi_mem_op *)
that calls spi_nor_spimem_setup() and spi_mem_exec_op().
In vendor specific, spi_mem_op is filled and passed to the core method.

Thank you for your comments,
Takahiro

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

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

[Tudor.Ambarus]

On 1/31/22 13:10, Takahiro Kuwano wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 1/31/2022 4:50 PM, Tudor.Ambarus@microchip.com wrote:
>> On 1/28/22 18:20, Pratyush Yadav wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 28/01/22 09:43AM, Tudor.Ambarus@microchip.com wrote:
>>>> On 7/19/21 11:03, 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>
>>>>>
>>>>> Some of Spansion/Cypress chips support Read/Write Any Register commands.
>>>>> These commands are mainly used to write volatile registers.
>>>>>
>>>>> 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 v7:
>>>>>   - No change
>>>>>
>>>>> Changes in v6:
>>>>>   - Add helper functions for controller_ops
>>>>>   - Add 'reg_addr_width' parameter to spansion_read/write_any_reg()
>>>>>   - Remove spi_nor_write_enable() from spansion_write_any_reg() and modified
>>>>>     function header comment
>>>>>
>>>>> 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 | 142 +++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 142 insertions(+)
>>>>>
>>>>> +/**
>>>>> + * spansion_read_any_reg() - Read Any Register.
>>>>> + * @nor:               pointer to a 'struct spi_nor'
>>>>> + * @reg_addr:          register address
>>>>> + * @reg_addr_width:    number of address bytes
>>>>> + * @reg_dummy:         number of dummy cycles for register read
>>>>> + * @reg_val:           pointer to a buffer where the register value is copied
>>>>> + *
>>>>> + * Return: 0 on success, -errno otherwise.
>>>>> + */
>>>>> +static int spansion_read_any_reg(struct spi_nor *nor, u32 reg_addr,
>>>>> +                                u8 reg_addr_width, u8 reg_dummy, u8 *reg_val)
>>>>
>>>> how about making this a generic core method? I'm sure there are other SPI NOR
>>>> vendors that use registers indexed by address. How about introducing:
>>>>
>>>> struct spi_nor_reg {
>>>>       u32 addr;
>>>>       u8 addr_width;
>>>>       u8 dummy;
>>>>       u8 opcode;
>>>>       u8 val;
>>>> };
>>>>
>>>> and passing a pointer to this struct as argument.
>>>
>>> Wouldn't it be simpler if they fill it in a struct spi_mem_op, and then
>>> just pass that in as a "template"?
>>>
>> sure, sounds fine. I would like a generic method with less function parameters,
>> if possible. Let's see what Takahiro will propose.
> 
> So, I would propose...
> In core, spi_nor_spimem_{read,write}_reg(struct spi_nor *, struct spi_mem_op *)
> that calls spi_nor_spimem_setup() and spi_mem_exec_op().
> In vendor specific, spi_mem_op is filled and passed to the core method.
> 

how about the following?

static ssize_t spi_nor_spimem_read_reg(struct spi_nor *nor, struct spi_mem_op *op)                    
{                                                                                  
        bool usebouncebuf;                                                      
        ssize_t nbytes;                                                         
        int error;                                                              
                                                                                
        spi_nor_spimem_setup_op(nor, &op, nor->read_proto);                     
                                                                                
        /* convert the dummy cycles to the number of bytes */                   
        op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;            
        if (spi_nor_protocol_is_dtr(nor->read_proto))                           
                op.dummy.nbytes *= 2;                                           
                                                                                
        usebouncebuf = spi_nor_spimem_bounce(nor, &op);                         
                                                                                                                                          
        ret = spi_nor_spimem_exec_op(nor, &op);                       
        if (ret)                                                      
               return ret;                                           
        nbytes = op.data.nbytes;                                                                                                
                                                                               
        if (usebouncebuf && nbytes > 0)                                         
                memcpy(buf, op.data.buf.in, nbytes);                            
                                                                                
        return nbytes;                                                          
}  

ssize_t spi_nor_read_reg(struct spi_nor *nor, struct spi_mem_op *op)
{                                                                               
        if (nor->spimem)                                                        
                return spi_nor_spimem_read_reg(nor, from, len, buf);           
                                                                                
        return -EOPNOTSUPP;                  
} 
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[Tudor.Ambarus]

On 2/1/22 09:37, Tudor Ambarus - M18064 wrote:
> On 1/31/22 13:10, Takahiro Kuwano wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 1/31/2022 4:50 PM, Tudor.Ambarus@microchip.com wrote:
>>> On 1/28/22 18:20, Pratyush Yadav wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> On 28/01/22 09:43AM, Tudor.Ambarus@microchip.com wrote:
>>>>> On 7/19/21 11:03, 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>
>>>>>>
>>>>>> Some of Spansion/Cypress chips support Read/Write Any Register commands.
>>>>>> These commands are mainly used to write volatile registers.
>>>>>>
>>>>>> 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 v7:
>>>>>>   - No change
>>>>>>
>>>>>> Changes in v6:
>>>>>>   - Add helper functions for controller_ops
>>>>>>   - Add 'reg_addr_width' parameter to spansion_read/write_any_reg()
>>>>>>   - Remove spi_nor_write_enable() from spansion_write_any_reg() and modified
>>>>>>     function header comment
>>>>>>
>>>>>> 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 | 142 +++++++++++++++++++++++++++++++++
>>>>>>  1 file changed, 142 insertions(+)
>>>>>>
>>>>>> +/**
>>>>>> + * spansion_read_any_reg() - Read Any Register.
>>>>>> + * @nor:               pointer to a 'struct spi_nor'
>>>>>> + * @reg_addr:          register address
>>>>>> + * @reg_addr_width:    number of address bytes
>>>>>> + * @reg_dummy:         number of dummy cycles for register read
>>>>>> + * @reg_val:           pointer to a buffer where the register value is copied
>>>>>> + *
>>>>>> + * Return: 0 on success, -errno otherwise.
>>>>>> + */
>>>>>> +static int spansion_read_any_reg(struct spi_nor *nor, u32 reg_addr,
>>>>>> +                                u8 reg_addr_width, u8 reg_dummy, u8 *reg_val)
>>>>>
>>>>> how about making this a generic core method? I'm sure there are other SPI NOR
>>>>> vendors that use registers indexed by address. How about introducing:
>>>>>
>>>>> struct spi_nor_reg {
>>>>>       u32 addr;
>>>>>       u8 addr_width;
>>>>>       u8 dummy;
>>>>>       u8 opcode;
>>>>>       u8 val;
>>>>> };
>>>>>
>>>>> and passing a pointer to this struct as argument.
>>>>
>>>> Wouldn't it be simpler if they fill it in a struct spi_mem_op, and then
>>>> just pass that in as a "template"?
>>>>
>>> sure, sounds fine. I would like a generic method with less function parameters,
>>> if possible. Let's see what Takahiro will propose.
>>
>> So, I would propose...
>> In core, spi_nor_spimem_{read,write}_reg(struct spi_nor *, struct spi_mem_op *)
>> that calls spi_nor_spimem_setup() and spi_mem_exec_op().
>> In vendor specific, spi_mem_op is filled and passed to the core method.
>>
> 
> how about the following?
> 
> static ssize_t spi_nor_spimem_read_reg(struct spi_nor *nor, struct spi_mem_op *op)                    
> {                                                                                  
>         bool usebouncebuf;                                                      
>         ssize_t nbytes;                                                         
>         int error;      
s/error/ret
                                                        
>                                                                                 
>         spi_nor_spimem_setup_op(nor, &op, nor->read_proto);
s/&op/op
                    
>                                                                                 
>         /* convert the dummy cycles to the number of bytes */                   
>         op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;   

op->dummy.nbytes and op->dummy.buswidth
         
>         if (spi_nor_protocol_is_dtr(nor->read_proto))                           
>                 op.dummy.nbytes *= 2;   
op->dummy.nbytes
                                        
>                                                                                 
>         usebouncebuf = spi_nor_spimem_bounce(nor, &op); 
s/&op/op
                        
>                                                                                                                                           
>         ret = spi_nor_spimem_exec_op(nor, &op);    
s/&op/op
                   
>         if (ret)                                                      
>                return ret;                                           
>         nbytes = op.data.nbytes;
op->data.nbytes
                                                                                                
>                                                                                
>         if (usebouncebuf && nbytes > 0)                                         
>                 memcpy(buf, op.data.buf.in, nbytes);    
op->data.buf.in
                        
>                                                                                 
>         return nbytes;                                                          
> }  
> 
> ssize_t spi_nor_read_reg(struct spi_nor *nor, struct spi_mem_op *op)
> {                                                                               
>         if (nor->spimem)                                                        
>                 return spi_nor_spimem_read_reg(nor, from, len, buf);           
>                                                                                 
>         return -EOPNOTSUPP;                  
> } 

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

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

[Takahiro Kuwano]

Hi Tudor,

Thank you for your suggestions.
I will revise the series

Best Regards,
Takahiro

On 2/1/2022 4:40 PM, Tudor.Ambarus@microchip.com wrote:
> On 2/1/22 09:37, Tudor Ambarus - M18064 wrote:
>> On 1/31/22 13:10, Takahiro Kuwano wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 1/31/2022 4:50 PM, Tudor.Ambarus@microchip.com wrote:
>>>> On 1/28/22 18:20, Pratyush Yadav wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>
>>>>> On 28/01/22 09:43AM, Tudor.Ambarus@microchip.com wrote:
>>>>>> On 7/19/21 11:03, 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>
>>>>>>>
>>>>>>> Some of Spansion/Cypress chips support Read/Write Any Register commands.
>>>>>>> These commands are mainly used to write volatile registers.
>>>>>>>
>>>>>>> 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 v7:
>>>>>>>   - No change
>>>>>>>
>>>>>>> Changes in v6:
>>>>>>>   - Add helper functions for controller_ops
>>>>>>>   - Add 'reg_addr_width' parameter to spansion_read/write_any_reg()
>>>>>>>   - Remove spi_nor_write_enable() from spansion_write_any_reg() and modified
>>>>>>>     function header comment
>>>>>>>
>>>>>>> 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 | 142 +++++++++++++++++++++++++++++++++
>>>>>>>  1 file changed, 142 insertions(+)
>>>>>>>
>>>>>>> +/**
>>>>>>> + * spansion_read_any_reg() - Read Any Register.
>>>>>>> + * @nor:               pointer to a 'struct spi_nor'
>>>>>>> + * @reg_addr:          register address
>>>>>>> + * @reg_addr_width:    number of address bytes
>>>>>>> + * @reg_dummy:         number of dummy cycles for register read
>>>>>>> + * @reg_val:           pointer to a buffer where the register value is copied
>>>>>>> + *
>>>>>>> + * Return: 0 on success, -errno otherwise.
>>>>>>> + */
>>>>>>> +static int spansion_read_any_reg(struct spi_nor *nor, u32 reg_addr,
>>>>>>> +                                u8 reg_addr_width, u8 reg_dummy, u8 *reg_val)
>>>>>>
>>>>>> how about making this a generic core method? I'm sure there are other SPI NOR
>>>>>> vendors that use registers indexed by address. How about introducing:
>>>>>>
>>>>>> struct spi_nor_reg {
>>>>>>       u32 addr;
>>>>>>       u8 addr_width;
>>>>>>       u8 dummy;
>>>>>>       u8 opcode;
>>>>>>       u8 val;
>>>>>> };
>>>>>>
>>>>>> and passing a pointer to this struct as argument.
>>>>>
>>>>> Wouldn't it be simpler if they fill it in a struct spi_mem_op, and then
>>>>> just pass that in as a "template"?
>>>>>
>>>> sure, sounds fine. I would like a generic method with less function parameters,
>>>> if possible. Let's see what Takahiro will propose.
>>>
>>> So, I would propose...
>>> In core, spi_nor_spimem_{read,write}_reg(struct spi_nor *, struct spi_mem_op *)
>>> that calls spi_nor_spimem_setup() and spi_mem_exec_op().
>>> In vendor specific, spi_mem_op is filled and passed to the core method.
>>>
>>
>> how about the following?
>>
>> static ssize_t spi_nor_spimem_read_reg(struct spi_nor *nor, struct spi_mem_op *op)                    
>> {                                                                                  
>>         bool usebouncebuf;                                                      
>>         ssize_t nbytes;                                                         
>>         int error;      
> s/error/ret
>                                                         
>>                                                                                 
>>         spi_nor_spimem_setup_op(nor, &op, nor->read_proto);
> s/&op/op
>                     
>>                                                                                 
>>         /* convert the dummy cycles to the number of bytes */                   
>>         op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;   
> 
> op->dummy.nbytes and op->dummy.buswidth
>          
>>         if (spi_nor_protocol_is_dtr(nor->read_proto))                           
>>                 op.dummy.nbytes *= 2;   
> op->dummy.nbytes
>                                         
>>                                                                                 
>>         usebouncebuf = spi_nor_spimem_bounce(nor, &op); 
> s/&op/op
>                         
>>                                                                                                                                           
>>         ret = spi_nor_spimem_exec_op(nor, &op);    
> s/&op/op
>                    
>>         if (ret)                                                      
>>                return ret;                                           
>>         nbytes = op.data.nbytes;
> op->data.nbytes
>                                                                                                 
>>                                                                                
>>         if (usebouncebuf && nbytes > 0)                                         
>>                 memcpy(buf, op.data.buf.in, nbytes);    
> op->data.buf.in
>                         
>>                                                                                 
>>         return nbytes;                                                          
>> }  
>>
>> ssize_t spi_nor_read_reg(struct spi_nor *nor, struct spi_mem_op *op)
>> {                                                                               
>>         if (nor->spimem)                                                        
>>                 return spi_nor_spimem_read_reg(nor, from, len, buf);           
>>                                                                                 
>>         return -EOPNOTSUPP;                  
>> } 
> 

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

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

[Takahiro Kuwano]

On 1/28/2022 7:27 PM, Tudor.Ambarus@microchip.com wrote:
> On 7/19/21 11:03, 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.
>>
>> For the single-die package parts (512Mb and 1Gb), only bottom 4KB and
>> uniform sector sizes are supported. 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>
>> ---
>> Changes in v7:
>>   - Add missing device info table in v6
>>
>> Changes in v6:
>>   - Remove 2Gb multi die pacakge support
>>
>> 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 | 90 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 90 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
>> index ca50e77b4220..c7170477e706 100644
>> --- a/drivers/mtd/spi-nor/spansion.c
>> +++ b/drivers/mtd/spi-nor/spansion.c
>> @@ -220,6 +220,84 @@ static int spansion_quad_enable_volatile(struct spi_nor *nor, u8 reg_dummy)
>>         return 0;
>>  }
>>
>> +static int s25hx_t_quad_enable(struct spi_nor *nor)
>> +{
>> +       int ret = spansion_quad_enable_volatile(nor, 0);
>> +
>> +       /* Reset WEL bit in any error cases */
> 
> the comment suggests an if (ret) then spi_nor_write_disable(nor);
> Should you call write disable in all cases or just in case of errors?
> 

I need the if (ret).

>> +       spi_nor_write_disable(nor);
>> +
>> +       return ret;
>> +}
>> +
>> +static int
>> +s25hx_t_post_bfpt_fixups(struct spi_nor *nor,
>> +                        const struct sfdp_parameter_header *bfpt_header,
>> +                        const struct sfdp_bfpt *bfpt)
>> +{
>> +       int ret;
>> +       u32 addr;
>> +       u8 cfr3v;
>> +
>> +       ret = spi_nor_set_4byte_addr_mode(nor, true);
> 

The core does not handle this due to SNOR_F_4B_OPCODES flag set in the
spansion_late_init(). And... 

> why don't you let the core handle this for you? Why do you call it here?
>> +       if (ret)
>> +               return ret;
>> +       nor->addr_width = 4;
> 
> so SFDP gets the addr_width wrong?
> 

the BFPT parse gets addr_width = 3 (BFPT_DWORD1_ADDRESS_BYTES_3_OR_4).
Since  Read and Write Any Register rely on addr mode, I would like to sync
device's addr mode and addr_width here.

>> +
>> +       /* Replace Quad Enable with volatile version */
>> +       nor->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.
>> +        */
>> +       ret = spansion_read_any_reg(nor, addr + SPINOR_REG_CYPRESS_CFR3V,
>> +                                   nor->addr_width, 0, &cfr3v);
>> +       if (ret)
>> +               return ret;
>> +
>> +       if (!(cfr3v & SPINOR_REG_CYPRESS_CFR3V_PGSZ))
>> +               nor->params->page_size = 256;
>> +       else
>> +               nor->params->page_size = 512;
> 
> isn't the else case already covered by BFPT?
> 

Yes, it is covered. I will remove the else case.

>> +
>> +       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;
> 
> Is the BFPT wrong? Why do you update the num_mode_clocks here?

There is no entry of Fast Read (1-1-1) params in the BFPT.
However, let me check this again.

>> +
>> +       /* 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
>> +        *  -------------------------------------------------------------------
>> +        */
> 
> did you miss to update erases here?

No. The table is just as info for users. I will move the table to function header.
The uniform (typically used) and bottom (factory default) are detected correctly.
So, I would like to leave this as it is. 

>> +}
>> +
>> +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'
>> @@ -468,6 +546,18 @@ 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 },
>> +       { "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 },
>>         { "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 v7 2/3] mtd: spi-nor: spansion: Add support for volatile QE bit

[Takahiro Kuwano]

On 1/29/2022 1:38 AM, Pratyush Yadav wrote:
> On 28/01/22 10:12AM, Tudor.Ambarus@microchip.com wrote:
>> On 7/19/21 11:03, 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>
>>>
>>> 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.
>>>
>>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>> ---
>>> Changes in v7:
>>>   - Add missing macro definitions in v6
>>>
>>> Changes in v6:
>>>   - Remove multi die package support
>>>
>>> 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 | 59 ++++++++++++++++++++++++++++++++++
>>>  1 file changed, 59 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
>>> index 5e7e62704e11..ca50e77b4220 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 */
> 
> I have the Cypress S28 flash and this bit is marked as "reserved" there. 
> Maybe we should have a flash family specific name? Dunno. But I guess it 
> does not matter so much since it will be very localised anyway.
> 
>>>  #define SPINOR_REG_CYPRESS_CFR2V               0x00800003
>>>  #define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24  0xb
>>>  #define SPINOR_REG_CYPRESS_CFR3V               0x00800004
>>> @@ -161,6 +163,63 @@ static int spansion_write_any_reg(struct spi_nor *nor, u32 reg_addr,
>>>                                             reg_val);
>>>  }
>>>
>>> +/**
>>> + * 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
>>> + *
>>> + * 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
>>
>> Why do you duplicate the setting of QE bit in CFR1V if QE bit is already set in
>> CFR1NV? Does the Volatile Register has a higher priority than the Non-Volatile one?
>> What happens when QE is zero in CFR1NV and QE is one in CFR1V?
> 
> I have the Cypress S28 flash family, and on there I see that the 
> volatile bit takes precedence over non-volatile bit*. At powerup data in 
> NV bits is transferred to V bits to set their default state.
> 
> What I am more worried about is if quad mode is enabled in a 
> non-volatile register (which should NEVER be done by Linux), will we be 
> able to properly detect and initialize the flash? We certainly can't do 
> it for 8D-8D-8D mode flashes.
> 
If quad mode is enabled, 1S-1S-4S/1S-4S-4S/1S-4D-4D ops are supported.
The 1S-1S-1S ops like read ID and registers used for initializationare still available.

Thanks,
Takahiro

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