[PATCH v5 00/10] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
The S25HL-T/S25HS-T family is the Cypress Semper Flash with Quad SPI.
The summary datasheets can be found in the following links. https://www.cypress.com/file/424146/download (256Mb/512Mb/1Gb, single die) https://www.cypress.com/file/499246/download (2Gb/4Gb, dual/quad die)
The full version can be found in the following links (registration required). https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-Sempe... https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-2Gb-M...
Tested on Xilinx Zynq-7000 FPGA board.
Changes since v5: - Removed 256Mb and 4Gb parts support - Fixed register offset issue in spansion_quad_enable_volatile() - Added spi_nor_default_ready() and moved existing code into it - Separated spansion_sr_read() to new patch - Renamed spansion_overlaid_erase() to spansion_non_uniform_erase() and changed the implementation to issue the proper erase command based on the address - Added s25hx_t_erase_non_uniform() - Changed mtd.writesize and mtd.flags in s25hx_t_setup() - Fixed page size and erase size issues in s25hx_t_post_bfpt_fixup()
Changes since v4: - Added Read/Write Any Register support - Added the ->ready() hook to support multi-die package parts - Added S25HL02GT/S25HL04GT/S25HS02GT/S25HS04GT support
Changes since v3: - Split into multiple patches
Changes since v2: - Fixed typo in comment for spansion_overlaid_erase() - Fixed expressions for addr and len check in spansion_overlaid_erase() - Added device ID check to make the changes effective for S25 only - Added nor->setup() and fixup hooks based on the following patches https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-7-p.yad... https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-8-p.yad... https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-9-p.yad...
Takahiro Kuwano (10): mtd: spi-nor: Add Cypress manufacturer ID mtd: spi-nor-ids: Add Cypress s25hl-t/s25hs-t mtd: spi-nor-core: Add support for Read/Write Any Register mtd: spi-nor-core: Add support for volatile QE bit mtd: spi-nor-core: Add the ->ready() hook mtd: spi-nor-core: Read status by Read Any Register mtd: spi-nor-core: Add non-uniform erase for Spansion/Cypress mtd: spi-nor-core: Add Cypress manufacturer ID in set_4byte mtd: spi-nor-core: Add fixups for Cypress s25hl-t/s25hs-t mtd: spi-nor-tiny: Add fixups for Cypress s25hl-t/s25hs-t
drivers/mtd/spi/spi-nor-core.c | 345 ++++++++++++++++++++++++++++++++- drivers/mtd/spi/spi-nor-ids.c | 27 +++ drivers/mtd/spi/spi-nor-tiny.c | 90 +++++++++ include/linux/mtd/spi-nor.h | 10 + 4 files changed, 471 insertions(+), 1 deletion(-)
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
This patch adds Cypress manufacturer ID (34h) definition.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com Reviewed-by: Pratyush Yadav p.yadav@ti.com --- include/linux/mtd/spi-nor.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 8e6744ac2e..d4f105df0f 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -27,6 +27,7 @@ #define SNOR_MFR_SPANSION CFI_MFR_AMD #define SNOR_MFR_SST CFI_MFR_SST #define SNOR_MFR_WINBOND 0xef /* Also used by some Spansion */ +#define SNOR_MFR_CYPRESS 0x34
/* * Note on opcode nomenclature: some opcodes have a format like
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
The S25HL-T/S25HS-T family is the Cypress Semper Flash with Quad SPI.
https://www.cypress.com/file/424146/download (256Mb/512Mb/1Gb, single die) https://www.cypress.com/file/499246/download (2Gb/4Gb, dual/quad die)
The full version can be found in the following links (registration required). https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-Sempe... https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-2Gb-M...
Tested on Xilinx Zynq-7000 FPGA board.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com Reviewed-by: Pratyush Yadav p.yadav@ti.com --- Changes in v5: - Remove 256Mb and 4Gb parts
drivers/mtd/spi/spi-nor-ids.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c index 5bd5dd3003..052a0b62ee 100644 --- a/drivers/mtd/spi/spi-nor-ids.c +++ b/drivers/mtd/spi/spi-nor-ids.c @@ -217,6 +217,33 @@ const struct flash_info spi_nor_ids[] = { { INFO("s25fl208k", 0x014014, 0, 64 * 1024, 16, SECT_4K | SPI_NOR_DUAL_READ) }, { INFO("s25fl064l", 0x016017, 0, 64 * 1024, 128, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) }, { INFO("s25fl128l", 0x016018, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) }, + + /* + * S25HL/HS-T (Semper Flash with Quad SPI) Family has user-configurable + * sector architecture. By default, the 512Mb and 1Gb, single-die + * package parts are configured to non-uniform that 4KB sectors overlaid + * on bottom address. To support this, an erase hook makes overlaid + * sectors appear as uniform sectors. The 2Gb, dual-die package parts + * are configured to uniform by default. + */ + { INFO6("s25hl512t", 0x342a1a, 0x0f0390, 256 * 1024, 256, + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | + USE_CLSR) }, + { INFO6("s25hl01gt", 0x342a1b, 0x0f0390, 256 * 1024, 512, + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | + USE_CLSR) }, + { INFO6("s25hl02gt", 0x342a1c, 0x0f0090, 256 * 1024, 1024, + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | + USE_CLSR) }, + { INFO6("s25hs512t", 0x342b1a, 0x0f0390, 256 * 1024, 256, + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | + USE_CLSR) }, + { INFO6("s25hs01gt", 0x342b1b, 0x0f0390, 256 * 1024, 512, + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | + USE_CLSR) }, + { INFO6("s25hs02gt", 0x342b1c, 0x0f0090, 256 * 1024, 1024, + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | + USE_CLSR) }, #endif #ifdef CONFIG_SPI_FLASH_SST /* SST */ /* SST -- large erase sizes are "overlays", "sectors" are 4K */
On 19/02/21 10:55AM, 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.
https://www.cypress.com/file/424146/download (256Mb/512Mb/1Gb, single die) https://www.cypress.com/file/499246/download (2Gb/4Gb, dual/quad die)
The full version can be found in the following links (registration required). https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-Sempe... https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-2Gb-M...
I created an account to view these. I get "Access denied" on both.
On Fri, Feb 19, 2021 at 7:26 AM 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.
https://www.cypress.com/file/424146/download (256Mb/512Mb/1Gb, single die) https://www.cypress.com/file/499246/download (2Gb/4Gb, dual/quad die)
The full version can be found in the following links (registration required). https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-Sempe... https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-2Gb-M...
Tested on Xilinx Zynq-7000 FPGA board.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com Reviewed-by: Pratyush Yadav p.yadav@ti.com
Changes in v5:
- Remove 256Mb and 4Gb parts
drivers/mtd/spi/spi-nor-ids.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c index 5bd5dd3003..052a0b62ee 100644 --- a/drivers/mtd/spi/spi-nor-ids.c +++ b/drivers/mtd/spi/spi-nor-ids.c @@ -217,6 +217,33 @@ const struct flash_info spi_nor_ids[] = { { INFO("s25fl208k", 0x014014, 0, 64 * 1024, 16, SECT_4K | SPI_NOR_DUAL_READ) }, { INFO("s25fl064l", 0x016017, 0, 64 * 1024, 128, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) }, { INFO("s25fl128l", 0x016018, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
/** S25HL/HS-T (Semper Flash with Quad SPI) Family has user-configurable* sector architecture. By default, the 512Mb and 1Gb, single-die* package parts are configured to non-uniform that 4KB sectors overlaid* on bottom address. To support this, an erase hook makes overlaid* sectors appear as uniform sectors. The 2Gb, dual-die package parts* are configured to uniform by default.*/
Nice to have these comments in commit instead of id's sections.
Hi Jagan,
On 2/26/2021 7:35 PM, Jagan Teki wrote:
On Fri, Feb 19, 2021 at 7:26 AM 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.
https://www.cypress.com/file/424146/download (256Mb/512Mb/1Gb, single die) https://www.cypress.com/file/499246/download (2Gb/4Gb, dual/quad die)
The full version can be found in the following links (registration required). https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-Sempe... https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-2Gb-M...
Tested on Xilinx Zynq-7000 FPGA board.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com Reviewed-by: Pratyush Yadav p.yadav@ti.com
Changes in v5:
- Remove 256Mb and 4Gb parts
drivers/mtd/spi/spi-nor-ids.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c index 5bd5dd3003..052a0b62ee 100644 --- a/drivers/mtd/spi/spi-nor-ids.c +++ b/drivers/mtd/spi/spi-nor-ids.c @@ -217,6 +217,33 @@ const struct flash_info spi_nor_ids[] = { { INFO("s25fl208k", 0x014014, 0, 64 * 1024, 16, SECT_4K | SPI_NOR_DUAL_READ) }, { INFO("s25fl064l", 0x016017, 0, 64 * 1024, 128, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) }, { INFO("s25fl128l", 0x016018, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
/** S25HL/HS-T (Semper Flash with Quad SPI) Family has user-configurable* sector architecture. By default, the 512Mb and 1Gb, single-die* package parts are configured to non-uniform that 4KB sectors overlaid* on bottom address. To support this, an erase hook makes overlaid* sectors appear as uniform sectors. The 2Gb, dual-die package parts* are configured to uniform by default.*/Nice to have these comments in commit instead of id's sections.
I will move it to commit in v6.
Thanks, Takahiro
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Some of Spansion/Cypress chips support Read/Write Any Register commands. These commands are mainly used to write volatile registers and access to the registers in second and subsequent die for multi-die package parts.
The Read Any Register instruction (65h) is followed by register address and dummy cycles, then the selected register byte is returned.
The Write Any Register instruction (71h) is followed by register address and register byte to write.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com Reviewed-by: Pratyush Yadav p.yadav@ti.com --- Changes in v5: - Remove unused defines from spi-nor.h
drivers/mtd/spi/spi-nor-core.c | 25 +++++++++++++++++++++++++ include/linux/mtd/spi-nor.h | 2 ++ 2 files changed, 27 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 34c15f1561..2803536ed5 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -211,6 +211,31 @@ static int spi_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len) return spi_nor_read_write_reg(nor, &op, buf); }
+#ifdef CONFIG_SPI_FLASH_SPANSION +static int spansion_read_any_reg(struct spi_nor *nor, u32 addr, u8 dummy, + u8 *val) +{ + struct spi_mem_op op = + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDAR, 1), + SPI_MEM_OP_ADDR(nor->addr_width, addr, 1), + SPI_MEM_OP_DUMMY(dummy / 8, 1), + SPI_MEM_OP_DATA_IN(1, NULL, 1)); + + return spi_nor_read_write_reg(nor, &op, val); +} + +static int spansion_write_any_reg(struct spi_nor *nor, u32 addr, u8 val) +{ + struct spi_mem_op op = + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRAR, 1), + SPI_MEM_OP_ADDR(nor->addr_width, addr, 1), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_OUT(1, NULL, 1)); + + return spi_nor_read_write_reg(nor, &op, &val); +} +#endif + static ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t from, size_t len, u_char *buf) { diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index d4f105df0f..04bf59f6f9 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -121,6 +121,8 @@ #define SPINOR_OP_BRWR 0x17 /* Bank register write */ #define SPINOR_OP_BRRD 0x16 /* Bank register read */ #define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */ +#define SPINOR_OP_RDAR 0x65 /* Read any register */ +#define SPINOR_OP_WRAR 0x71 /* Write any register */
/* Used for Micron flashes only. */ #define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Some of Spansion/Cypress chips support volatile version of configuration registers and it is recommended to update volatile registers in the field application due to a risk of the non-volatile registers corruption by power interrupt. This patch adds a function to set Quad Enable bit in CFR1 volatile.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com --- Changes in v5: - Fix register address calculation, 'base | offset' -> 'base + offset'
drivers/mtd/spi/spi-nor-core.c | 53 ++++++++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h | 1 + 2 files changed, 54 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 2803536ed5..87c9fce408 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -1576,6 +1576,59 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor) return 0; }
+/** + * spansion_quad_enable_volatile() - enable Quad I/O mode in volatile register. + * @nor: pointer to a 'struct spi_nor' + * @addr_base: base address of register (can be >0 in multi-die parts) + * @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. + * + * Return: 0 on success, -errno otherwise. + */ +static int spansion_quad_enable_volatile(struct spi_nor *nor, u32 addr_base, + u8 dummy) +{ + u32 addr = addr_base + SPINOR_REG_ADDR_CFR1V; + + u8 cr; + int ret; + + /* Check current Quad Enable bit value. */ + ret = spansion_read_any_reg(nor, addr, dummy, &cr); + if (ret < 0) { + dev_dbg(nor->dev, + "error while reading configuration register\n"); + return -EINVAL; + } + + if (cr & CR_QUAD_EN_SPAN) + return 0; + + cr |= CR_QUAD_EN_SPAN; + + write_enable(nor); + + ret = spansion_write_any_reg(nor, addr, cr); + + if (ret < 0) { + dev_dbg(nor->dev, + "error while writing configuration register\n"); + return -EINVAL; + } + + /* Read back and check it. */ + ret = spansion_read_any_reg(nor, addr, dummy, &cr); + if (ret || !(cr & CR_QUAD_EN_SPAN)) { + dev_dbg(nor->dev, "Spansion Quad bit not set\n"); + return -EINVAL; + } + + return 0; +} + #if CONFIG_IS_ENABLED(SPI_FLASH_SFDP_SUPPORT) /** * spansion_no_read_cr_quad_enable() - set QE bit in Configuration Register. diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 04bf59f6f9..d79f8b37ef 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -123,6 +123,7 @@ #define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */ #define SPINOR_OP_RDAR 0x65 /* Read any register */ #define SPINOR_OP_WRAR 0x71 /* Write any register */ +#define SPINOR_REG_ADDR_CFR1V 0x00800002
/* Used for Micron flashes only. */ #define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */
On Fri, Feb 19, 2021 at 7:26 AM tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Some of Spansion/Cypress chips support volatile version of configuration registers and it is recommended to update volatile registers in the field application due to a risk of the non-volatile registers corruption by power interrupt. This patch adds a function to set Quad Enable bit in CFR1 volatile.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Changes in v5:
- Fix register address calculation, 'base | offset' -> 'base + offset'
drivers/mtd/spi/spi-nor-core.c | 53 ++++++++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h | 1 + 2 files changed, 54 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 2803536ed5..87c9fce408 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -1576,6 +1576,59 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor) return 0; }
+/**
- spansion_quad_enable_volatile() - enable Quad I/O mode in volatile register.
- @nor: pointer to a 'struct spi_nor'
- @addr_base: base address of register (can be >0 in multi-die parts)
- @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.
- Return: 0 on success, -errno otherwise.
- */
+static int spansion_quad_enable_volatile(struct spi_nor *nor, u32 addr_base,
u8 dummy)+{
u32 addr = addr_base + SPINOR_REG_ADDR_CFR1V;u8 cr;int ret;/* Check current Quad Enable bit value. */ret = spansion_read_any_reg(nor, addr, dummy, &cr);
What if we can use the exiting quad_enable hook by identifying volatile QE at the function beginning instead of having a separate call?
Jagan.
Hi Jagan,
On 2/26/2021 7:42 PM, Jagan Teki wrote:
On Fri, Feb 19, 2021 at 7:26 AM tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Some of Spansion/Cypress chips support volatile version of configuration registers and it is recommended to update volatile registers in the field application due to a risk of the non-volatile registers corruption by power interrupt. This patch adds a function to set Quad Enable bit in CFR1 volatile.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Changes in v5:
- Fix register address calculation, 'base | offset' -> 'base + offset'
drivers/mtd/spi/spi-nor-core.c | 53 ++++++++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h | 1 + 2 files changed, 54 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 2803536ed5..87c9fce408 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -1576,6 +1576,59 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor) return 0; }
+/**
- spansion_quad_enable_volatile() - enable Quad I/O mode in volatile register.
- @nor: pointer to a 'struct spi_nor'
- @addr_base: base address of register (can be >0 in multi-die parts)
- @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.
- Return: 0 on success, -errno otherwise.
- */
+static int spansion_quad_enable_volatile(struct spi_nor *nor, u32 addr_base,
u8 dummy)+{
u32 addr = addr_base + SPINOR_REG_ADDR_CFR1V;u8 cr;int ret;/* Check current Quad Enable bit value. */ret = spansion_read_any_reg(nor, addr, dummy, &cr);What if we can use the exiting quad_enable hook by identifying volatile QE at the function beginning instead of having a separate call?
Do you mean something like this?
static int spansion_read_cr_quad_enable(struct spi_nor *nor) { u8 sr_cr[2]; int ret;
if (JEDEC_MFR(nor->info) == SNOR_MFR_CYPRESS) { u32 base;
for (base = 0; base < nor->mtd.size; base += SZ_128M) { u32 addr = base + SPINOR_REG_ADDR_CFR1V;
/* Check current Quad Enable bit value. */ ret = spansion_read_any_reg(nor, addr, 0, &sr_cr[1]);
[...]
ret = spansion_write_any_reg(nor, addr, sr_cr[1]);
[...]
/* Read back and check it. */ ret = spansion_read_any_reg(nor, addr, 0, &sr_cr[1]);
[...] }
return 0; }
/* Check current Quad Enable bit value. */ ret = read_cr(nor); if (ret < 0) { dev_dbg(nor->dev, "error while reading configuration register\n"); return -EINVAL; }
[...] }
Or defining a new flag like 'SNOR_F_HAS_VOLATILE_QE'?
Jagan.
Best Regards, Takahiro
On 08/03/21 06:10PM, Takahiro Kuwano wrote:
Hi Jagan,
On 2/26/2021 7:42 PM, Jagan Teki wrote:
On Fri, Feb 19, 2021 at 7:26 AM tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Some of Spansion/Cypress chips support volatile version of configuration registers and it is recommended to update volatile registers in the field application due to a risk of the non-volatile registers corruption by power interrupt. This patch adds a function to set Quad Enable bit in CFR1 volatile.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Changes in v5:
- Fix register address calculation, 'base | offset' -> 'base + offset'
drivers/mtd/spi/spi-nor-core.c | 53 ++++++++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h | 1 + 2 files changed, 54 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 2803536ed5..87c9fce408 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -1576,6 +1576,59 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor) return 0; }
+/**
- spansion_quad_enable_volatile() - enable Quad I/O mode in volatile register.
- @nor: pointer to a 'struct spi_nor'
- @addr_base: base address of register (can be >0 in multi-die parts)
- @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.
- Return: 0 on success, -errno otherwise.
- */
+static int spansion_quad_enable_volatile(struct spi_nor *nor, u32 addr_base,
u8 dummy)+{
u32 addr = addr_base + SPINOR_REG_ADDR_CFR1V;u8 cr;int ret;/* Check current Quad Enable bit value. */ret = spansion_read_any_reg(nor, addr, dummy, &cr);What if we can use the exiting quad_enable hook by identifying volatile QE at the function beginning instead of having a separate call?
Do you mean something like this?
static int spansion_read_cr_quad_enable(struct spi_nor *nor) { u8 sr_cr[2]; int ret;
if (JEDEC_MFR(nor->info) == SNOR_MFR_CYPRESS) { u32 base;
for (base = 0; base < nor->mtd.size; base += SZ_128M) { u32 addr = base + SPINOR_REG_ADDR_CFR1V; /* Check current Quad Enable bit value. */ ret = spansion_read_any_reg(nor, addr, 0, &sr_cr[1]); [...] ret = spansion_write_any_reg(nor, addr, sr_cr[1]); [...] /* Read back and check it. */ ret = spansion_read_any_reg(nor, addr, 0, &sr_cr[1]); [...] } return 0;}
/* Check current Quad Enable bit value. */ ret = read_cr(nor); if (ret < 0) { dev_dbg(nor->dev, "error while reading configuration register\n"); return -EINVAL; }
[...] }
Or defining a new flag like 'SNOR_F_HAS_VOLATILE_QE'?
FWIW I think your current implementation is better than both these alternatives.
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
For dual/quad die package devices from Spansion/Cypress, the device's status needs to be checked by reading status registers in all dies, by using Read Any Register command. To support this, a Flash specific hook that can overwrite the legacy status check is needed.
The change in spi-nor.h depends on: https://patchwork.ozlabs.org/project/uboot/patch/20210205041119.145784-4-sea...
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com --- Changes in v5: - Move spansion_sr_ready() to different patch - Move original code in spi_nor_ready() to newly created spi_nor_default_ready()
drivers/mtd/spi/spi-nor-core.c | 10 +++++++++- include/linux/mtd/spi-nor.h | 2 ++ 2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 87c9fce408..821617bc0d 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -566,7 +566,7 @@ static int spi_nor_fsr_ready(struct spi_nor *nor) return fsr & FSR_READY; }
-static int spi_nor_ready(struct spi_nor *nor) +static int spi_nor_default_ready(struct spi_nor *nor) { int sr, fsr;
@@ -579,6 +579,14 @@ static int spi_nor_ready(struct spi_nor *nor) return sr && fsr; }
+static int spi_nor_ready(struct spi_nor *nor) +{ + if (nor->ready) + return nor->ready(nor); + + return spi_nor_default_ready(nor); +} + /* * Service routine to read status register until ready, or timeout occurs. * Returns non-zero if error. diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index d79f8b37ef..2f2ad20e8e 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -435,6 +435,7 @@ struct flash_info; * @flash_is_locked: [FLASH-SPECIFIC] check if a region of the SPI NOR is * completely locked * @quad_enable: [FLASH-SPECIFIC] enables SPI NOR quad mode + * @ready: [FLASH-SPECIFIC] check if the flash is ready * @priv: the private data */ struct spi_nor { @@ -480,6 +481,7 @@ struct spi_nor { int (*flash_unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len); int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len); int (*quad_enable)(struct spi_nor *nor); + int (*ready)(struct spi_nor *nor);
void *priv; /* Compatibility for spi_flash, remove once sf layer is merged with mtd */
On 19/02/21 10:55AM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
For dual/quad die package devices from Spansion/Cypress, the device's status needs to be checked by reading status registers in all dies, by using Read Any Register command. To support this, a Flash specific hook that can overwrite the legacy status check is needed.
The change in spi-nor.h depends on: https://patchwork.ozlabs.org/project/uboot/patch/20210205041119.145784-4-sea...
This line should not be a part of the commit message. This is "metadata" that says how the patch should be applied. One it is applied this has no use in the Git history. So it should go below the 3 dashed lines, like the changelog.
Other than this,
Reviewed-by: Pratyush Yadav p.yadav@ti.com
On 2/19/2021 6:57 PM, Pratyush Yadav wrote:
On 19/02/21 10:55AM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
For dual/quad die package devices from Spansion/Cypress, the device's status needs to be checked by reading status registers in all dies, by using Read Any Register command. To support this, a Flash specific hook that can overwrite the legacy status check is needed.
The change in spi-nor.h depends on: https://patchwork.ozlabs.org/project/uboot/patch/20210205041119.145784-4-sea...
This line should not be a part of the commit message. This is "metadata" that says how the patch should be applied. One it is applied this has no use in the Git history. So it should go below the 3 dashed lines, like the changelog.
Other than this,
Reviewed-by: Pratyush Yadav p.yadav@ti.com
I will make it goes below the "---" line. Thank you.
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
The spansion_sr_ready() reads status register 1 by Read Any Register commnad. This function is called from Flash specific hook with die address and dummy cycles to support multi-die package parts from Spansion/Cypress.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com --- Changes in v5: - New in v5, separated from another patch
drivers/mtd/spi/spi-nor-core.c | 29 +++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h | 1 + 2 files changed, 30 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 821617bc0d..e5fc0e7965 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -522,6 +522,35 @@ static int set_4byte(struct spi_nor *nor, const struct flash_info *info, } }
+#ifdef CONFIG_SPI_FLASH_SPANSION +/* + * Read status register 1 by using Read Any Register command to support multi + * die package parts. + */ +static int spansion_sr_ready(struct spi_nor *nor, u32 addr_base, u8 dummy) +{ + u32 reg_addr = addr_base + SPINOR_REG_ADDR_STR1V; + u8 sr; + int ret; + + ret = spansion_read_any_reg(nor, reg_addr, dummy, &sr); + if (ret < 0) + return ret; + + if (sr & (SR_E_ERR | SR_P_ERR)) { + if (sr & SR_E_ERR) + dev_dbg(nor->dev, "Erase Error occurred\n"); + else + dev_dbg(nor->dev, "Programming Error occurred\n"); + + nor->write_reg(nor, SPINOR_OP_CLSR, NULL, 0); + return -EIO; + } + + return !(sr & SR_WIP); +} +#endif + static int spi_nor_sr_ready(struct spi_nor *nor) { int sr = read_sr(nor); diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 2f2ad20e8e..25234177de 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -123,6 +123,7 @@ #define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */ #define SPINOR_OP_RDAR 0x65 /* Read any register */ #define SPINOR_OP_WRAR 0x71 /* Write any register */ +#define SPINOR_REG_ADDR_STR1V 0x00800000 #define SPINOR_REG_ADDR_CFR1V 0x00800002
/* Used for Micron flashes only. */
On 19/02/21 10:56AM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
The spansion_sr_ready() reads status register 1 by Read Any Register commnad. This function is called from Flash specific hook with die address and dummy cycles to support multi-die package parts from Spansion/Cypress.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Reviewed-by: Pratyush Yadav p.yadav@ti.com
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Some of Spansion/Cypress chips have overlaid 4KB sectors at top and/or bottom, depending on the device configuration, while U-Boot supports uniform sector layout only.
The spansion_erase_non_uniform() erases overlaid 4KB sectors, non-overlaid portion of normal sector, and remaining normal sectors, by selecting correct erase command and size based on the address to erase and size of overlaid portion in parameters.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com --- Changes in v5: - New in v5, introduce spansion_erase_non_uniform() as a replacement for spansion_overlaid_erase() in v4
drivers/mtd/spi/spi-nor-core.c | 72 ++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index e5fc0e7965..46948ed41b 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -793,6 +793,78 @@ erase_err: return ret; }
+#ifdef CONFIG_SPI_FLASH_SPANSION +/** + * spansion_erase_non_uniform() - erase non-uniform sectors for Spansion/Cypress + * chips + * @mtd: pointer to a 'struct mtd_info' + * @instr: pointer to a 'struct erase_info' + * @ovlsz_top: size of overlaid portion at the top address + * @ovlsz_btm: size of overlaid portion at the bottom address + * + * Erase an address range on the nor chip that can contain 4KB sectors overlaid + * on top and/or bottom. The appropriate erase opcode and size are chosen by + * address to erase and size of overlaid portion. + * + * Return: 0 on success, -errno otherwise. + */ +static int spansion_erase_non_uniform(struct mtd_info *mtd, + struct erase_info *instr, u32 ovlsz_top, + u32 ovlsz_btm) +{ + struct spi_nor *nor = mtd_to_spi_nor(mtd); + struct spi_mem_op op = + SPI_MEM_OP(SPI_MEM_OP_CMD(nor->erase_opcode, 1), + SPI_MEM_OP_ADDR(nor->addr_width, instr->addr, 1), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_NO_DATA); + u32 len = instr->len; + u32 erasesize; + int ret; + + while (len) { + /* 4KB sectors */ + if (op.addr.val < ovlsz_btm || + op.addr.val >= mtd->size - ovlsz_top) { + op.cmd.opcode = SPINOR_OP_BE_4K; + erasesize = SZ_4K; + + /* Non-overlaid portion in the normal sector at the bottom */ + } else if (op.addr.val == ovlsz_btm) { + op.cmd.opcode = nor->erase_opcode; + erasesize = mtd->erasesize - ovlsz_btm; + + /* Non-overlaid portion in the normal sector at the top */ + } else if (op.addr.val == mtd->size - mtd->erasesize) { + op.cmd.opcode = nor->erase_opcode; + erasesize = mtd->erasesize - ovlsz_top; + + /* Normal sectors */ + } else { + op.cmd.opcode = nor->erase_opcode; + erasesize = mtd->erasesize; + } + + write_enable(nor); + + ret = spi_mem_exec_op(nor->spi, &op); + if (ret) + break; + + op.addr.val += erasesize; + len -= erasesize; + + ret = spi_nor_wait_till_ready(nor); + if (ret) + break; + } + + write_disable(nor); + + return ret; +} +#endif + #if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST) /* Write status register and ensure bits in mask match written values */ static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask)
On 19/02/21 10:56AM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Some of Spansion/Cypress chips have overlaid 4KB sectors at top and/or bottom, depending on the device configuration, while U-Boot supports uniform sector layout only.
The spansion_erase_non_uniform() erases overlaid 4KB sectors, non-overlaid portion of normal sector, and remaining normal sectors, by selecting correct erase command and size based on the address to erase and size of overlaid portion in parameters.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Changes in v5:
- New in v5, introduce spansion_erase_non_uniform() as a replacement for spansion_overlaid_erase() in v4
drivers/mtd/spi/spi-nor-core.c | 72 ++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index e5fc0e7965..46948ed41b 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -793,6 +793,78 @@ erase_err: return ret; }
+#ifdef CONFIG_SPI_FLASH_SPANSION +/**
- spansion_erase_non_uniform() - erase non-uniform sectors for Spansion/Cypress
chips
- @mtd: pointer to a 'struct mtd_info'
- @instr: pointer to a 'struct erase_info'
- @ovlsz_top: size of overlaid portion at the top address
- @ovlsz_btm: size of overlaid portion at the bottom address
- Erase an address range on the nor chip that can contain 4KB sectors overlaid
- on top and/or bottom. The appropriate erase opcode and size are chosen by
- address to erase and size of overlaid portion.
- Return: 0 on success, -errno otherwise.
- */
+static int spansion_erase_non_uniform(struct mtd_info *mtd,
struct erase_info *instr, u32 ovlsz_top,u32 ovlsz_btm)
Is there any reason why you are not using the nor->erase() hook? As far as I can see it should also be able to perform the same erase while avoiding all the boilerplate needed for keeping track of address, remaining erase, write enable, error handling, etc.
One problem I can see is that you don't always increment address by nor->erasesize. That can change depending on which sector got erased. spi_nor_erase() can't currently handle that. But IMO a reasonable fix for this would be to return the size actually erased in nor->erase(), like how it is done for the unix read() and write() system calls. A negative value would still mean an error but now a positive return value will tell the caller how much data was actually erased.
I think it is a relatively easy refactor and worth doing.
+{
- struct spi_nor *nor = mtd_to_spi_nor(mtd);
- struct spi_mem_op op =
SPI_MEM_OP(SPI_MEM_OP_CMD(nor->erase_opcode, 1),SPI_MEM_OP_ADDR(nor->addr_width, instr->addr, 1),SPI_MEM_OP_NO_DUMMY,SPI_MEM_OP_NO_DATA);- u32 len = instr->len;
- u32 erasesize;
- int ret;
spi_nor_erase() does some sanity checking for instr->len. Even more reason to use nor->erase() so we don't have to duplicate that code.
- while (len) {
/* 4KB sectors */if (op.addr.val < ovlsz_btm ||op.addr.val >= mtd->size - ovlsz_top) {op.cmd.opcode = SPINOR_OP_BE_4K;erasesize = SZ_4K;
Ok.
/* Non-overlaid portion in the normal sector at the bottom */} else if (op.addr.val == ovlsz_btm) {op.cmd.opcode = nor->erase_opcode;erasesize = mtd->erasesize - ovlsz_btm;/* Non-overlaid portion in the normal sector at the top */} else if (op.addr.val == mtd->size - mtd->erasesize) {op.cmd.opcode = nor->erase_opcode;erasesize = mtd->erasesize - ovlsz_top;
Patch 9/10 does not check for uniform sector size configuration. But if the check is to be added later, passing 0 to ovlsz_top and ovlsz_btm will do the right thing because erasesize will end up equal to mtd->erasesize in both cases. Neat!
/* Normal sectors */} else {op.cmd.opcode = nor->erase_opcode;erasesize = mtd->erasesize;}
Ok.
write_enable(nor);ret = spi_mem_exec_op(nor->spi, &op);if (ret)break;op.addr.val += erasesize;len -= erasesize;
I recall a patch for Linux by Tudor recently that moved some code like this to after spi_nor_wait_till_ready(). Let's do so here as well. Of course, this will not matter if you are using nor->erase() instead. The problem will still be there since spi_nor_erase() also does this but that is a separate fix.
ret = spi_nor_wait_till_ready(nor);if (ret)break;- }
- write_disable(nor);
- return ret;
+} +#endif
#if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST) /* Write status register and ensure bits in mask match written values */ static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask) -- 2.25.1
Hi Pratyush,
On 2/24/2021 9:05 PM, Pratyush Yadav wrote:
On 19/02/21 10:56AM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Some of Spansion/Cypress chips have overlaid 4KB sectors at top and/or bottom, depending on the device configuration, while U-Boot supports uniform sector layout only.
The spansion_erase_non_uniform() erases overlaid 4KB sectors, non-overlaid portion of normal sector, and remaining normal sectors, by selecting correct erase command and size based on the address to erase and size of overlaid portion in parameters.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Changes in v5:
- New in v5, introduce spansion_erase_non_uniform() as a replacement for spansion_overlaid_erase() in v4
drivers/mtd/spi/spi-nor-core.c | 72 ++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index e5fc0e7965..46948ed41b 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -793,6 +793,78 @@ erase_err: return ret; }
+#ifdef CONFIG_SPI_FLASH_SPANSION +/**
- spansion_erase_non_uniform() - erase non-uniform sectors for Spansion/Cypress
chips
- @mtd: pointer to a 'struct mtd_info'
- @instr: pointer to a 'struct erase_info'
- @ovlsz_top: size of overlaid portion at the top address
- @ovlsz_btm: size of overlaid portion at the bottom address
- Erase an address range on the nor chip that can contain 4KB sectors overlaid
- on top and/or bottom. The appropriate erase opcode and size are chosen by
- address to erase and size of overlaid portion.
- Return: 0 on success, -errno otherwise.
- */
+static int spansion_erase_non_uniform(struct mtd_info *mtd,
struct erase_info *instr, u32 ovlsz_top,u32 ovlsz_btm)Is there any reason why you are not using the nor->erase() hook? As far as I can see it should also be able to perform the same erase while avoiding all the boilerplate needed for keeping track of address, remaining erase, write enable, error handling, etc.
I tried to use the nor-erase() hook, but I saw the erasesize problem you mentioned below and didn't think about changing the return value of existing function.
One problem I can see is that you don't always increment address by nor->erasesize. That can change depending on which sector got erased. spi_nor_erase() can't currently handle that. But IMO a reasonable fix for this would be to return the size actually erased in nor->erase(), like how it is done for the unix read() and write() system calls. A negative value would still mean an error but now a positive return value will tell the caller how much data was actually erased.
I think it is a relatively easy refactor and worth doing.
I agree. Let me introduce it in v6.
+{
- struct spi_nor *nor = mtd_to_spi_nor(mtd);
- struct spi_mem_op op =
SPI_MEM_OP(SPI_MEM_OP_CMD(nor->erase_opcode, 1),SPI_MEM_OP_ADDR(nor->addr_width, instr->addr, 1),SPI_MEM_OP_NO_DUMMY,SPI_MEM_OP_NO_DATA);- u32 len = instr->len;
- u32 erasesize;
- int ret;
spi_nor_erase() does some sanity checking for instr->len. Even more reason to use nor->erase() so we don't have to duplicate that code.
- while (len) {
/* 4KB sectors */if (op.addr.val < ovlsz_btm ||op.addr.val >= mtd->size - ovlsz_top) {op.cmd.opcode = SPINOR_OP_BE_4K;erasesize = SZ_4K;Ok.
/* Non-overlaid portion in the normal sector at the bottom */} else if (op.addr.val == ovlsz_btm) {op.cmd.opcode = nor->erase_opcode;erasesize = mtd->erasesize - ovlsz_btm;/* Non-overlaid portion in the normal sector at the top */} else if (op.addr.val == mtd->size - mtd->erasesize) {op.cmd.opcode = nor->erase_opcode;erasesize = mtd->erasesize - ovlsz_top;Patch 9/10 does not check for uniform sector size configuration. But if the check is to be added later, passing 0 to ovlsz_top and ovlsz_btm will do the right thing because erasesize will end up equal to mtd->erasesize in both cases. Neat!
/* Normal sectors */} else {op.cmd.opcode = nor->erase_opcode;erasesize = mtd->erasesize;}Ok.
write_enable(nor);ret = spi_mem_exec_op(nor->spi, &op);if (ret)break;op.addr.val += erasesize;len -= erasesize;I recall a patch for Linux by Tudor recently that moved some code like this to after spi_nor_wait_till_ready(). Let's do so here as well. Of course, this will not matter if you are using nor->erase() instead. The problem will still be there since spi_nor_erase() also does this but that is a separate fix.
ret = spi_nor_wait_till_ready(nor);if (ret)break;- }
- write_disable(nor);
- return ret;
+} +#endif
#if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST) /* Write status register and ensure bits in mask match written values */ static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask) -- 2.25.1
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Cypress chips support SPINOR_OP_EN4B(B7h)/SPINOR_OP_EX4B(E9h) to enable/disable 4-byte addressing mode.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com --- drivers/mtd/spi/spi-nor-core.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 46948ed41b..8d63681cb3 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -492,6 +492,7 @@ static int set_4byte(struct spi_nor *nor, const struct flash_info *info, case SNOR_MFR_ISSI: case SNOR_MFR_MACRONIX: case SNOR_MFR_WINBOND: + case SNOR_MFR_CYPRESS: if (need_wren) write_enable(nor);
On 19/02/21 10:56AM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Cypress chips support SPINOR_OP_EN4B(B7h)/SPINOR_OP_EX4B(E9h) to
The datasheet says the EN4B command is indeed B7h but EX4B is listed as B8h. The command E9h is for "Password Unlock". So exiting 4 byte mode will do something completely different.
enable/disable 4-byte addressing mode.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
drivers/mtd/spi/spi-nor-core.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 46948ed41b..8d63681cb3 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -492,6 +492,7 @@ static int set_4byte(struct spi_nor *nor, const struct flash_info *info, case SNOR_MFR_ISSI: case SNOR_MFR_MACRONIX: case SNOR_MFR_WINBOND:
- case SNOR_MFR_CYPRESS: if (need_wren) write_enable(nor);
-- 2.25.1
On 2/24/2021 9:11 PM, Pratyush Yadav wrote:
On 19/02/21 10:56AM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Cypress chips support SPINOR_OP_EN4B(B7h)/SPINOR_OP_EX4B(E9h) to
The datasheet says the EN4B command is indeed B7h but EX4B is listed as B8h. The command E9h is for "Password Unlock". So exiting 4 byte mode will do something completely different.
How stupid of me! Thank you for thorough review.
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
This patch adds Flash specific fixups and hooks for Cypress S25HL-T/S25HS-T family, based on the features introduced in [0][1][2].
The nor->ready() and spansion_sr_ready() introduced in #5 and #6 in this series are used for multi-die package parts.
The nor->quad_enable() sets the volatile QE bit on each die.
The mtd._erase() is hooked if the device is single-die package and not configured to uniform sectors, assuming it is in factory default configuration that has 32 x 4KB sectors overlaid on bottom address. Other configurations, top and split, are not supported at this point. Will submit additional patches to support it as needed.
The post_bfpt/sfdp() fixes the params wrongly advertised in SFDP.
[0] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-7-p.yad... [1] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-8-p.yad... [2] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-9-p.yad...
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com --- Changes in v5: - Add s25hx_t_erase_non_uniform() - Change mtd.writesize and mtd.flags in s25hx_t_setup() - Fix page size and erase size issues in s25hx_t_post_bfpt_fixup()
drivers/mtd/spi/spi-nor-core.c | 155 +++++++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h | 3 + 2 files changed, 158 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 8d63681cb3..315e26ab27 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -2677,8 +2677,163 @@ static int spi_nor_init(struct spi_nor *nor) return 0; }
+#ifdef CONFIG_SPI_FLASH_SPANSION +static int s25hx_t_mdp_ready(struct spi_nor *nor) +{ + u32 addr; + int ret; + + for (addr = 0; addr < nor->mtd.size; addr += SZ_128M) { + ret = spansion_sr_ready(nor, addr, 0); + if (ret != 1) + return ret; + } + + return 1; +} + +static int s25hx_t_quad_enable(struct spi_nor *nor) +{ + u32 addr; + int ret; + + for (addr = 0; addr < nor->mtd.size; addr += SZ_128M) { + ret = spansion_quad_enable_volatile(nor, addr, 0); + if (ret) + return ret; + } + + return 0; +} + +static int s25hx_t_erase_non_uniform(struct mtd_info *mtd, + struct erase_info *instr) +{ + /* Support factory default configuration (32 x 4KB sectors at bottom) */ + return spansion_erase_non_uniform(mtd, instr, 0, SZ_128K); +} + +static int s25hx_t_setup(struct spi_nor *nor, const struct flash_info *info, + const struct spi_nor_flash_parameter *params, + const struct spi_nor_hwcaps *hwcaps) +{ +#ifdef CONFIG_SPI_FLASH_BAR + return -ENOTSUPP; /* Bank Address Register is not supported */ +#endif + /* + * The Cypress Semper family has transparent ECC. To preserve + * ECC enabled, multi-pass programming within the same 16-byte + * ECC data unit needs to be avoided. + */ + nor->mtd.writesize = 16; + + if (nor->mtd.size > SZ_128M) { + /* + * For the multi-die package parts, the ready() hook is needed + * to check all dies' status via read any register. + */ + nor->ready = s25hx_t_mdp_ready; + } else { + int ret; + u8 cfr3v; + + /* + * Read CFR3V to check if uniform sector is selected. If not, + * assign an erase hook that supports non-uniform erase, + * assuming the part has factory default configuration, + * 32 x 4KB sectors at bottom. + */ + ret = spansion_read_any_reg(nor, SPINOR_REG_ADDR_CFR3V, 0, + &cfr3v); + if (ret) + return ret; + + if (!(cfr3v & CFR3V_UNHYSA)) + nor->mtd._erase = s25hx_t_erase_non_uniform; + } + + return spi_nor_default_setup(nor, info, params, hwcaps); +} + +static void s25hx_t_default_init(struct spi_nor *nor) +{ + nor->setup = s25hx_t_setup; +} + +static int s25hx_t_post_bfpt_fixup(struct spi_nor *nor, + const struct sfdp_parameter_header *header, + const struct sfdp_bfpt *bfpt, + struct spi_nor_flash_parameter *params) +{ + int ret; + u32 addr; + u8 cfr3v; + + /* erase size in case it is set to 4K from BFPT */ + nor->erase_opcode = SPINOR_OP_SE_4B; + nor->mtd.erasesize = nor->info->sector_size; + + /* Enter 4-byte addressing mode for Read Any Register */ + ret = set_4byte(nor, nor->info, 1); + if (ret) + return ret; + nor->addr_width = 4; + + /* + * The page_size is set to 512B from BFPT, but it actually depends on + * the configuration register. Look up the CFR3V and determine the + * page_size. For multi-die package parts, use 512B only when the all + * dies are configured to 512B buffer. + */ + for (addr = 0; addr < params->size; addr += SZ_128M) { + ret = spansion_read_any_reg(nor, addr + SPINOR_REG_ADDR_CFR3V, + 0, &cfr3v); + if (ret) + return ret; + + if (!(cfr3v & CFR3V_PGMBUF)) { + params->page_size = 256; + return 0; + } + } + params->page_size = 512; + + return 0; +} + +static void s25hx_t_post_sfdp_fixup(struct spi_nor *nor, + struct spi_nor_flash_parameter *params) +{ + /* READ_FAST_4B (0Ch) requires mode cycles*/ + params->reads[SNOR_CMD_READ_FAST].num_mode_clocks = 8; + /* PP_1_1_4 is not supported */ + params->hwcaps.mask &= ~SNOR_HWCAPS_PP_1_1_4; + /* Use volatile register to enable quad */ + params->quad_enable = s25hx_t_quad_enable; +} + +static struct spi_nor_fixups s25hx_t_fixups = { + .default_init = s25hx_t_default_init, + .post_bfpt = s25hx_t_post_bfpt_fixup, + .post_sfdp = s25hx_t_post_sfdp_fixup, +}; +#endif + static void spi_nor_set_fixups(struct spi_nor *nor) { +#ifdef CONFIG_SPI_FLASH_SPANSION + if (JEDEC_MFR(nor->info) == SNOR_MFR_CYPRESS) { + switch (nor->info->id[1]) { + case 0x2a: /* S25HL (QSPI, 3.3V) */ + case 0x2b: /* S25HS (QSPI, 1.8V) */ + nor->fixups = &s25hx_t_fixups; + break; + + default: + break; + } + } +#endif }
int spi_nor_scan(struct spi_nor *nor) diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 25234177de..cfb2104fee 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -125,6 +125,9 @@ #define SPINOR_OP_WRAR 0x71 /* Write any register */ #define SPINOR_REG_ADDR_STR1V 0x00800000 #define SPINOR_REG_ADDR_CFR1V 0x00800002 +#define SPINOR_REG_ADDR_CFR3V 0x00800004 +#define CFR3V_UNHYSA BIT(3) /* Uniform sectors or not */ +#define CFR3V_PGMBUF BIT(4) /* Program buffer size */
/* Used for Micron flashes only. */ #define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */
On 19/02/21 10:56AM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
This patch adds Flash specific fixups and hooks for Cypress S25HL-T/S25HS-T family, based on the features introduced in [0][1][2].
Instead of linking the patches like this, it would be a better idea to simply include them in your series. This will make your series independent from mine and will make it easier for the maintainer to apply it.
The nor->ready() and spansion_sr_ready() introduced in #5 and #6 in this series are used for multi-die package parts.
Nitpick: Once this patch makes it into the Git history there is no patch #5 or #6. Probably a better idea to just say "introduced earlier" or something similar.
The nor->quad_enable() sets the volatile QE bit on each die.
The mtd._erase() is hooked if the device is single-die package and not configured to uniform sectors, assuming it is in factory default configuration that has 32 x 4KB sectors overlaid on bottom address. Other configurations, top and split, are not supported at this point. Will submit additional patches to support it as needed.
Ah, this patch does check for uniform sector map. Nice. I assume you have tested with both configurations.
The post_bfpt/sfdp() fixes the params wrongly advertised in SFDP.
[0] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-7-p.yad... [1] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-8-p.yad... [2] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-9-p.yad...
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Changes in v5:
- Add s25hx_t_erase_non_uniform()
- Change mtd.writesize and mtd.flags in s25hx_t_setup()
- Fix page size and erase size issues in s25hx_t_post_bfpt_fixup()
drivers/mtd/spi/spi-nor-core.c | 155 +++++++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h | 3 + 2 files changed, 158 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 8d63681cb3..315e26ab27 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -2677,8 +2677,163 @@ static int spi_nor_init(struct spi_nor *nor) return 0; }
+#ifdef CONFIG_SPI_FLASH_SPANSION +static int s25hx_t_mdp_ready(struct spi_nor *nor) +{
- u32 addr;
- int ret;
- for (addr = 0; addr < nor->mtd.size; addr += SZ_128M) {
ret = spansion_sr_ready(nor, addr, 0);if (ret != 1)
Nitpick: Use if (!ret) since spansion_sr_ready() returns a boolean value.
return ret;- }
- return 1;
+}
+static int s25hx_t_quad_enable(struct spi_nor *nor) +{
- u32 addr;
- int ret;
- for (addr = 0; addr < nor->mtd.size; addr += SZ_128M) {
ret = spansion_quad_enable_volatile(nor, addr, 0);if (ret)return ret;- }
- return 0;
+}
+static int s25hx_t_erase_non_uniform(struct mtd_info *mtd,
struct erase_info *instr)+{
- /* Support factory default configuration (32 x 4KB sectors at bottom) */
- return spansion_erase_non_uniform(mtd, instr, 0, SZ_128K);
+}
+static int s25hx_t_setup(struct spi_nor *nor, const struct flash_info *info,
const struct spi_nor_flash_parameter *params,const struct spi_nor_hwcaps *hwcaps)+{ +#ifdef CONFIG_SPI_FLASH_BAR
- return -ENOTSUPP; /* Bank Address Register is not supported */
+#endif
- /*
* The Cypress Semper family has transparent ECC. To preserve* ECC enabled, multi-pass programming within the same 16-byte* ECC data unit needs to be avoided.*/- nor->mtd.writesize = 16;
Sorry, I forgot what your end goal was with setting the writesize. Which layer is doing multi-pass writes? You would need to check if those layers actually respect this value. In Linux some consumers of the MTD layer simply assumed the writesize for a NOR flash is 1 (like UBI for example). I had to patch them before everything was back to normal.
- if (nor->mtd.size > SZ_128M) {
/** For the multi-die package parts, the ready() hook is needed* to check all dies' status via read any register.*/nor->ready = s25hx_t_mdp_ready;- } else {
int ret;u8 cfr3v;/** Read CFR3V to check if uniform sector is selected. If not,* assign an erase hook that supports non-uniform erase,* assuming the part has factory default configuration,* 32 x 4KB sectors at bottom.*/ret = spansion_read_any_reg(nor, SPINOR_REG_ADDR_CFR3V, 0,&cfr3v);if (ret)return ret;if (!(cfr3v & CFR3V_UNHYSA))nor->mtd._erase = s25hx_t_erase_non_uniform;
Multi-die parts can also have uniform sector sizes. Why not do this check for them as well?
- }
- return spi_nor_default_setup(nor, info, params, hwcaps);
+}
+static void s25hx_t_default_init(struct spi_nor *nor) +{
- nor->setup = s25hx_t_setup;
+}
Ok.
+static int s25hx_t_post_bfpt_fixup(struct spi_nor *nor,
const struct sfdp_parameter_header *header,const struct sfdp_bfpt *bfpt,struct spi_nor_flash_parameter *params)+{
- int ret;
- u32 addr;
- u8 cfr3v;
- /* erase size in case it is set to 4K from BFPT */
- nor->erase_opcode = SPINOR_OP_SE_4B;
- nor->mtd.erasesize = nor->info->sector_size;
- /* Enter 4-byte addressing mode for Read Any Register */
- ret = set_4byte(nor, nor->info, 1);
- if (ret)
return ret;
You enter 4byte addressing but don't exit it before returning. Is this intentional?
- nor->addr_width = 4;
- /*
* The page_size is set to 512B from BFPT, but it actually depends on* the configuration register. Look up the CFR3V and determine the* page_size. For multi-die package parts, use 512B only when the all* dies are configured to 512B buffer.*/- for (addr = 0; addr < params->size; addr += SZ_128M) {
ret = spansion_read_any_reg(nor, addr + SPINOR_REG_ADDR_CFR3V,0, &cfr3v);if (ret)return ret;if (!(cfr3v & CFR3V_PGMBUF)) {params->page_size = 256;return 0;}- }
- params->page_size = 512;
Ok.
- return 0;
+}
+static void s25hx_t_post_sfdp_fixup(struct spi_nor *nor,
struct spi_nor_flash_parameter *params)+{
- /* READ_FAST_4B (0Ch) requires mode cycles*/
- params->reads[SNOR_CMD_READ_FAST].num_mode_clocks = 8;
- /* PP_1_1_4 is not supported */
- params->hwcaps.mask &= ~SNOR_HWCAPS_PP_1_1_4;
- /* Use volatile register to enable quad */
- params->quad_enable = s25hx_t_quad_enable;
+}
+static struct spi_nor_fixups s25hx_t_fixups = {
- .default_init = s25hx_t_default_init,
- .post_bfpt = s25hx_t_post_bfpt_fixup,
- .post_sfdp = s25hx_t_post_sfdp_fixup,
+}; +#endif
static void spi_nor_set_fixups(struct spi_nor *nor) { +#ifdef CONFIG_SPI_FLASH_SPANSION
- if (JEDEC_MFR(nor->info) == SNOR_MFR_CYPRESS) {
switch (nor->info->id[1]) {case 0x2a: /* S25HL (QSPI, 3.3V) */case 0x2b: /* S25HS (QSPI, 1.8V) */nor->fixups = &s25hx_t_fixups;break;
Ok.
default:break;}- }
+#endif }
int spi_nor_scan(struct spi_nor *nor) diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 25234177de..cfb2104fee 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -125,6 +125,9 @@ #define SPINOR_OP_WRAR 0x71 /* Write any register */ #define SPINOR_REG_ADDR_STR1V 0x00800000 #define SPINOR_REG_ADDR_CFR1V 0x00800002 +#define SPINOR_REG_ADDR_CFR3V 0x00800004 +#define CFR3V_UNHYSA BIT(3) /* Uniform sectors or not */ +#define CFR3V_PGMBUF BIT(4) /* Program buffer size */
Ok.
/* Used for Micron flashes only. */
#define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */
2.25.1
On 2/24/2021 9:40 PM, Pratyush Yadav wrote:
On 19/02/21 10:56AM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
This patch adds Flash specific fixups and hooks for Cypress S25HL-T/S25HS-T family, based on the features introduced in [0][1][2].
Instead of linking the patches like this, it would be a better idea to simply include them in your series. This will make your series independent from mine and will make it easier for the maintainer to apply it.
Noted with thanks.
The nor->ready() and spansion_sr_ready() introduced in #5 and #6 in this series are used for multi-die package parts.
Nitpick: Once this patch makes it into the Git history there is no patch #5 or #6. Probably a better idea to just say "introduced earlier" or something similar.
OK.
The nor->quad_enable() sets the volatile QE bit on each die.
The mtd._erase() is hooked if the device is single-die package and not configured to uniform sectors, assuming it is in factory default configuration that has 32 x 4KB sectors overlaid on bottom address. Other configurations, top and split, are not supported at this point. Will submit additional patches to support it as needed.
Ah, this patch does check for uniform sector map. Nice. I assume you have tested with both configurations.
Yes. I tested both.
The post_bfpt/sfdp() fixes the params wrongly advertised in SFDP.
[0] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-7-p.yad... [1] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-8-p.yad... [2] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-9-p.yad...
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Changes in v5:
- Add s25hx_t_erase_non_uniform()
- Change mtd.writesize and mtd.flags in s25hx_t_setup()
- Fix page size and erase size issues in s25hx_t_post_bfpt_fixup()
drivers/mtd/spi/spi-nor-core.c | 155 +++++++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h | 3 + 2 files changed, 158 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 8d63681cb3..315e26ab27 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -2677,8 +2677,163 @@ static int spi_nor_init(struct spi_nor *nor) return 0; }
+#ifdef CONFIG_SPI_FLASH_SPANSION +static int s25hx_t_mdp_ready(struct spi_nor *nor) +{
- u32 addr;
- int ret;
- for (addr = 0; addr < nor->mtd.size; addr += SZ_128M) {
ret = spansion_sr_ready(nor, addr, 0);if (ret != 1)Nitpick: Use if (!ret) since spansion_sr_ready() returns a boolean value.
Will fix.
return ret;- }
- return 1;
+}
+static int s25hx_t_quad_enable(struct spi_nor *nor) +{
- u32 addr;
- int ret;
- for (addr = 0; addr < nor->mtd.size; addr += SZ_128M) {
ret = spansion_quad_enable_volatile(nor, addr, 0);if (ret)return ret;- }
- return 0;
+}
+static int s25hx_t_erase_non_uniform(struct mtd_info *mtd,
struct erase_info *instr)+{
- /* Support factory default configuration (32 x 4KB sectors at bottom) */
- return spansion_erase_non_uniform(mtd, instr, 0, SZ_128K);
+}
+static int s25hx_t_setup(struct spi_nor *nor, const struct flash_info *info,
const struct spi_nor_flash_parameter *params,const struct spi_nor_hwcaps *hwcaps)+{ +#ifdef CONFIG_SPI_FLASH_BAR
- return -ENOTSUPP; /* Bank Address Register is not supported */
+#endif
- /*
* The Cypress Semper family has transparent ECC. To preserve* ECC enabled, multi-pass programming within the same 16-byte* ECC data unit needs to be avoided.*/- nor->mtd.writesize = 16;
Sorry, I forgot what your end goal was with setting the writesize. Which layer is doing multi-pass writes? You would need to check if those layers actually respect this value. In Linux some consumers of the MTD layer simply assumed the writesize for a NOR flash is 1 (like UBI for example). I had to patch them before everything was back to normal.
My goal so far is enabling access this Flash via cmd/sf that does not respect the writesize. I think I can remove this.
- if (nor->mtd.size > SZ_128M) {
/** For the multi-die package parts, the ready() hook is needed* to check all dies' status via read any register.*/nor->ready = s25hx_t_mdp_ready;- } else {
int ret;u8 cfr3v;/** Read CFR3V to check if uniform sector is selected. If not,* assign an erase hook that supports non-uniform erase,* assuming the part has factory default configuration,* 32 x 4KB sectors at bottom.*/ret = spansion_read_any_reg(nor, SPINOR_REG_ADDR_CFR3V, 0,&cfr3v);if (ret)return ret;if (!(cfr3v & CFR3V_UNHYSA))nor->mtd._erase = s25hx_t_erase_non_uniform;Multi-die parts can also have uniform sector sizes. Why not do this check for them as well?
Multi-die parts are configured to uniform by default and this patch assumes they are in default sector size setting. But I have realized I can easily add support for bottom in multi-die parts. Will do it in v6.
- }
- return spi_nor_default_setup(nor, info, params, hwcaps);
+}
+static void s25hx_t_default_init(struct spi_nor *nor) +{
- nor->setup = s25hx_t_setup;
+}
Ok.
+static int s25hx_t_post_bfpt_fixup(struct spi_nor *nor,
const struct sfdp_parameter_header *header,const struct sfdp_bfpt *bfpt,struct spi_nor_flash_parameter *params)+{
- int ret;
- u32 addr;
- u8 cfr3v;
- /* erase size in case it is set to 4K from BFPT */
- nor->erase_opcode = SPINOR_OP_SE_4B;
- nor->mtd.erasesize = nor->info->sector_size;
- /* Enter 4-byte addressing mode for Read Any Register */
- ret = set_4byte(nor, nor->info, 1);
- if (ret)
return ret;You enter 4byte addressing but don't exit it before returning. Is this intentional?
Yes. The nor->addr_width must be 4 for read/write/erase to work correctly. I think device's addressing mode should be in sync with nor->addr_width.
- nor->addr_width = 4;
- /*
* The page_size is set to 512B from BFPT, but it actually depends on* the configuration register. Look up the CFR3V and determine the* page_size. For multi-die package parts, use 512B only when the all* dies are configured to 512B buffer.*/- for (addr = 0; addr < params->size; addr += SZ_128M) {
ret = spansion_read_any_reg(nor, addr + SPINOR_REG_ADDR_CFR3V,0, &cfr3v);if (ret)return ret;if (!(cfr3v & CFR3V_PGMBUF)) {params->page_size = 256;return 0;}- }
- params->page_size = 512;
Ok.
- return 0;
+}
+static void s25hx_t_post_sfdp_fixup(struct spi_nor *nor,
struct spi_nor_flash_parameter *params)+{
- /* READ_FAST_4B (0Ch) requires mode cycles*/
- params->reads[SNOR_CMD_READ_FAST].num_mode_clocks = 8;
- /* PP_1_1_4 is not supported */
- params->hwcaps.mask &= ~SNOR_HWCAPS_PP_1_1_4;
- /* Use volatile register to enable quad */
- params->quad_enable = s25hx_t_quad_enable;
+}
+static struct spi_nor_fixups s25hx_t_fixups = {
- .default_init = s25hx_t_default_init,
- .post_bfpt = s25hx_t_post_bfpt_fixup,
- .post_sfdp = s25hx_t_post_sfdp_fixup,
+}; +#endif
static void spi_nor_set_fixups(struct spi_nor *nor) { +#ifdef CONFIG_SPI_FLASH_SPANSION
- if (JEDEC_MFR(nor->info) == SNOR_MFR_CYPRESS) {
switch (nor->info->id[1]) {case 0x2a: /* S25HL (QSPI, 3.3V) */case 0x2b: /* S25HS (QSPI, 1.8V) */nor->fixups = &s25hx_t_fixups;break;Ok.
default:break;}- }
+#endif }
int spi_nor_scan(struct spi_nor *nor) diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 25234177de..cfb2104fee 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -125,6 +125,9 @@ #define SPINOR_OP_WRAR 0x71 /* Write any register */ #define SPINOR_REG_ADDR_STR1V 0x00800000 #define SPINOR_REG_ADDR_CFR1V 0x00800002 +#define SPINOR_REG_ADDR_CFR3V 0x00800004 +#define CFR3V_UNHYSA BIT(3) /* Uniform sectors or not */ +#define CFR3V_PGMBUF BIT(4) /* Program buffer size */
Ok.
/* Used for Micron flashes only. */
#define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */
2.25.1
On 08/03/21 05:47PM, Takahiro Kuwano wrote:
On 2/24/2021 9:40 PM, Pratyush Yadav wrote:
On 19/02/21 10:56AM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
This patch adds Flash specific fixups and hooks for Cypress S25HL-T/S25HS-T family, based on the features introduced in [0][1][2].
Instead of linking the patches like this, it would be a better idea to simply include them in your series. This will make your series independent from mine and will make it easier for the maintainer to apply it.
Noted with thanks.
The nor->ready() and spansion_sr_ready() introduced in #5 and #6 in this series are used for multi-die package parts.
Nitpick: Once this patch makes it into the Git history there is no patch #5 or #6. Probably a better idea to just say "introduced earlier" or something similar.
OK.
The nor->quad_enable() sets the volatile QE bit on each die.
The mtd._erase() is hooked if the device is single-die package and not configured to uniform sectors, assuming it is in factory default configuration that has 32 x 4KB sectors overlaid on bottom address. Other configurations, top and split, are not supported at this point. Will submit additional patches to support it as needed.
Ah, this patch does check for uniform sector map. Nice. I assume you have tested with both configurations.
Yes. I tested both.
The post_bfpt/sfdp() fixes the params wrongly advertised in SFDP.
[0] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-7-p.yad... [1] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-8-p.yad... [2] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-9-p.yad...
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Changes in v5:
- Add s25hx_t_erase_non_uniform()
- Change mtd.writesize and mtd.flags in s25hx_t_setup()
- Fix page size and erase size issues in s25hx_t_post_bfpt_fixup()
drivers/mtd/spi/spi-nor-core.c | 155 +++++++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h | 3 + 2 files changed, 158 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 8d63681cb3..315e26ab27 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c
[...]
+static int s25hx_t_post_bfpt_fixup(struct spi_nor *nor,
const struct sfdp_parameter_header *header,const struct sfdp_bfpt *bfpt,struct spi_nor_flash_parameter *params)+{
- int ret;
- u32 addr;
- u8 cfr3v;
- /* erase size in case it is set to 4K from BFPT */
- nor->erase_opcode = SPINOR_OP_SE_4B;
- nor->mtd.erasesize = nor->info->sector_size;
- /* Enter 4-byte addressing mode for Read Any Register */
- ret = set_4byte(nor, nor->info, 1);
- if (ret)
return ret;You enter 4byte addressing but don't exit it before returning. Is this intentional?
Yes. The nor->addr_width must be 4 for read/write/erase to work correctly. I think device's addressing mode should be in sync with nor->addr_width.
Right. But the comment above implies that 4-byte mode is only enabled for the "Read Any Register" command. Either remove the comment completely or make it clear that you are changing to 4-byte for all further operations, not just the Read Any Register command.
- nor->addr_width = 4;
- /*
* The page_size is set to 512B from BFPT, but it actually depends on* the configuration register. Look up the CFR3V and determine the* page_size. For multi-die package parts, use 512B only when the all* dies are configured to 512B buffer.*/- for (addr = 0; addr < params->size; addr += SZ_128M) {
ret = spansion_read_any_reg(nor, addr + SPINOR_REG_ADDR_CFR3V,0, &cfr3v);if (ret)return ret;if (!(cfr3v & CFR3V_PGMBUF)) {params->page_size = 256;return 0;}- }
- params->page_size = 512;
Ok.
- return 0;
+}
On 3/8/2021 5:54 PM, Pratyush Yadav wrote:
On 08/03/21 05:47PM, Takahiro Kuwano wrote:
On 2/24/2021 9:40 PM, Pratyush Yadav wrote:
On 19/02/21 10:56AM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
This patch adds Flash specific fixups and hooks for Cypress S25HL-T/S25HS-T family, based on the features introduced in [0][1][2].
Instead of linking the patches like this, it would be a better idea to simply include them in your series. This will make your series independent from mine and will make it easier for the maintainer to apply it.
Noted with thanks.
The nor->ready() and spansion_sr_ready() introduced in #5 and #6 in this series are used for multi-die package parts.
Nitpick: Once this patch makes it into the Git history there is no patch #5 or #6. Probably a better idea to just say "introduced earlier" or something similar.
OK.
The nor->quad_enable() sets the volatile QE bit on each die.
The mtd._erase() is hooked if the device is single-die package and not configured to uniform sectors, assuming it is in factory default configuration that has 32 x 4KB sectors overlaid on bottom address. Other configurations, top and split, are not supported at this point. Will submit additional patches to support it as needed.
Ah, this patch does check for uniform sector map. Nice. I assume you have tested with both configurations.
Yes. I tested both.
The post_bfpt/sfdp() fixes the params wrongly advertised in SFDP.
[0] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-7-p.yad... [1] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-8-p.yad... [2] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-9-p.yad...
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Changes in v5:
- Add s25hx_t_erase_non_uniform()
- Change mtd.writesize and mtd.flags in s25hx_t_setup()
- Fix page size and erase size issues in s25hx_t_post_bfpt_fixup()
drivers/mtd/spi/spi-nor-core.c | 155 +++++++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h | 3 + 2 files changed, 158 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 8d63681cb3..315e26ab27 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c
[...]
+static int s25hx_t_post_bfpt_fixup(struct spi_nor *nor,
const struct sfdp_parameter_header *header,const struct sfdp_bfpt *bfpt,struct spi_nor_flash_parameter *params)+{
- int ret;
- u32 addr;
- u8 cfr3v;
- /* erase size in case it is set to 4K from BFPT */
- nor->erase_opcode = SPINOR_OP_SE_4B;
- nor->mtd.erasesize = nor->info->sector_size;
- /* Enter 4-byte addressing mode for Read Any Register */
- ret = set_4byte(nor, nor->info, 1);
- if (ret)
return ret;You enter 4byte addressing but don't exit it before returning. Is this intentional?
Yes. The nor->addr_width must be 4 for read/write/erase to work correctly. I think device's addressing mode should be in sync with nor->addr_width.
Right. But the comment above implies that 4-byte mode is only enabled for the "Read Any Register" command. Either remove the comment completely or make it clear that you are changing to 4-byte for all further operations, not just the Read Any Register command.
Indeed. The comment is bad. Will fix.
Thanks, Takahiro
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Fixes mode clocks for SPINOR_OP_READ_FAST_4B and volatile QE bit in tiny. The volatile QE bit function, spansion_quad_enable_volatile() supports dual/quad die package parts, by taking 'die_size' parameter that is used to iterate register update for all dies in the device.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com --- Changes in v5: - Add a comment about Flash models and respective IDs
drivers/mtd/spi/spi-nor-tiny.c | 90 ++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-tiny.c b/drivers/mtd/spi/spi-nor-tiny.c index 5cc2b7d996..66e4c99cc6 100644 --- a/drivers/mtd/spi/spi-nor-tiny.c +++ b/drivers/mtd/spi/spi-nor-tiny.c @@ -555,6 +555,85 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor) } #endif /* CONFIG_SPI_FLASH_SPANSION */
+#ifdef CONFIG_SPI_FLASH_SPANSION +/** + * spansion_quad_enable_volatile() - enable Quad I/O mode in volatile register. + * @nor: pointer to a 'struct spi_nor' + * @die_size: maximum number of bytes per die ('mtd.size' > 'die_size' in + * multi die package parts). + * @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. + * + * Return: 0 on success, -errno otherwise. + */ +static int spansion_quad_enable_volatile(struct spi_nor *nor, u32 die_size, + u8 dummy) +{ + struct spi_mem_op op = + SPI_MEM_OP(SPI_MEM_OP_CMD(0, 1), + SPI_MEM_OP_ADDR(4, 0, 1), + SPI_MEM_OP_DUMMY(0, 1), + SPI_MEM_OP_DATA_IN(1, NULL, 1)); + u32 addr; + u8 cr; + int ret; + + /* Use 4-byte address for RDAR/WRAR */ + ret = spi_nor_write_reg(nor, SPINOR_OP_EN4B, NULL, 0); + if (ret < 0) { + dev_dbg(nor->dev, + "error while enabling 4-byte address\n"); + return ret; + } + + for (addr = 0; addr < nor->mtd.size; addr += die_size) { + op.addr.val = addr + SPINOR_REG_ADDR_CFR1V; + + /* Check current Quad Enable bit value. */ + op.cmd.opcode = SPINOR_OP_RDAR; + op.dummy.nbytes = dummy / 8; + op.data.dir = SPI_MEM_DATA_IN; + ret = spi_nor_read_write_reg(nor, &op, &cr); + if (ret < 0) { + dev_dbg(nor->dev, + "error while reading configuration register\n"); + return -EINVAL; + } + + if (cr & CR_QUAD_EN_SPAN) + return 0; + + /* Write new value. */ + cr |= CR_QUAD_EN_SPAN; + op.cmd.opcode = SPINOR_OP_WRAR; + op.dummy.nbytes = 0; + op.data.dir = SPI_MEM_DATA_OUT; + write_enable(nor); + ret = spi_nor_read_write_reg(nor, &op, &cr); + if (ret < 0) { + dev_dbg(nor->dev, + "error while writing configuration register\n"); + return -EINVAL; + } + + /* Read back and check it. */ + op.cmd.opcode = SPINOR_OP_RDAR; + op.dummy.nbytes = dummy / 8; + op.data.dir = SPI_MEM_DATA_IN; + ret = spi_nor_read_write_reg(nor, &op, &cr); + if (ret || !(cr & CR_QUAD_EN_SPAN)) { + dev_dbg(nor->dev, "Spansion Quad bit not set\n"); + return -EINVAL; + } + } + + return 0; +} +#endif + static void spi_nor_set_read_settings(struct spi_nor_read_command *read, u8 num_mode_clocks, @@ -583,6 +662,12 @@ static int spi_nor_init_params(struct spi_nor *nor, spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_FAST], 0, 8, SPINOR_OP_READ_FAST, SNOR_PROTO_1_1_1); +#ifdef CONFIG_SPI_FLASH_SPANSION + if (JEDEC_MFR(info) == SNOR_MFR_CYPRESS && + (info->id[1] == 0x2a || info->id[1] == 0x2b)) + /* 0x2a: S25HL (QSPI, 3.3V), 0x2b: S25HS (QSPI, 1.8V) */ + params->reads[SNOR_CMD_READ_FAST].num_mode_clocks = 8; +#endif }
if (info->flags & SPI_NOR_QUAD_READ) { @@ -659,6 +744,11 @@ static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info, case SNOR_MFR_MACRONIX: err = macronix_quad_enable(nor); break; +#endif +#ifdef CONFIG_SPI_FLASH_SPANSION + case SNOR_MFR_CYPRESS: + err = spansion_quad_enable_volatile(nor, SZ_128M, 0); + break; #endif case SNOR_MFR_ST: case SNOR_MFR_MICRON:
On 19/02/21 10:56AM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Fixes mode clocks for SPINOR_OP_READ_FAST_4B and volatile QE bit in tiny. The volatile QE bit function, spansion_quad_enable_volatile() supports dual/quad die package parts, by taking 'die_size' parameter that is used to iterate register update for all dies in the device.
As explained in v4 [0],
Nacked-by: Pratyush Yadav p.yadav@ti.com
[0] https://patchwork.ozlabs.org/project/uboot/patch/a5c3cf1353d9a621379e2fcfefc...
On 2/24/2021 9:43 PM, Pratyush Yadav wrote:
On 19/02/21 10:56AM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Fixes mode clocks for SPINOR_OP_READ_FAST_4B and volatile QE bit in tiny. The volatile QE bit function, spansion_quad_enable_volatile() supports dual/quad die package parts, by taking 'die_size' parameter that is used to iterate register update for all dies in the device.
As explained in v4 [0],
Nacked-by: Pratyush Yadav p.yadav@ti.com
[0] https://patchwork.ozlabs.org/project/uboot/patch/a5c3cf1353d9a621379e2fcfefc...
I will remove this in v6.
Hi all,
On 19/02/21 10:55AM, 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) https://www.cypress.com/file/499246/download (2Gb/4Gb, dual/quad die)
The full version can be found in the following links (registration required). https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-Sempe... https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-2Gb-M...
Tested on Xilinx Zynq-7000 FPGA board.
I think this series is in very good shape apart from a few minor issues. Thanks for all the work :-)
participants (4)
-
Jagan Teki -
Pratyush Yadav -
Takahiro Kuwano -
tkuw584924＠gmail.com