Re: [PATCH] mtd: spi-nor-core: Add fixups for s25fs512s
On Sun, Apr 23, 2023 at 5:26 AM Marek Vasut wrote: > > From: Takahiro Kuwano > > This patch adds fixups for s25fs512s to address the following issues > from reading SFDP: > > - Non-uniform sectors by factory default. The setting needs to be > checked and assign erase hook as needed. > - Page size is wrongly advertised in SFDP. > - READ_1_1_2 (3Bh/3Ch), READ_1_1_4 (6Bh/6Ch), and PP_1_1_4 (32h/34h) > are not supported. > - Bank Address Register (BAR) is not supported. > > In addition, volatile version of Quad Enable is used for safety. > > Based on patch by Takahiro Kuwano with s25fs_s_post_bfpt_fixup() updated > to use 4-byte address commands instead of extended address mode and the > page_size is fixed to 256 > > For future use, manufacturer code should be moved out from framework > code as same as in Linux. > > Reviewed-by: Marek Vasut > Signed-off-by: Takahiro Kuwano > Signed-off-by: Hai Pham > Signed-off-by: Cong Dang > Signed-off-by: Marek Vasut > --- Reviewed-by: Jagan Teki
[PATCH] mtd: spi-nor-core: Add fixups for s25fs512s
From: Takahiro Kuwano This patch adds fixups for s25fs512s to address the following issues from reading SFDP: - Non-uniform sectors by factory default. The setting needs to be checked and assign erase hook as needed. - Page size is wrongly advertised in SFDP. - READ_1_1_2 (3Bh/3Ch), READ_1_1_4 (6Bh/6Ch), and PP_1_1_4 (32h/34h) are not supported. - Bank Address Register (BAR) is not supported. In addition, volatile version of Quad Enable is used for safety. Based on patch by Takahiro Kuwano with s25fs_s_post_bfpt_fixup() updated to use 4-byte address commands instead of extended address mode and the page_size is fixed to 256 For future use, manufacturer code should be moved out from framework code as same as in Linux. Reviewed-by: Marek Vasut Signed-off-by: Takahiro Kuwano Signed-off-by: Hai Pham Signed-off-by: Cong Dang Signed-off-by: Marek Vasut --- Cc: Jagan Teki Cc: Takahiro Kuwano Cc: Vignesh R --- drivers/mtd/spi/spi-nor-core.c | 85 ++ 1 file changed, 85 insertions(+) diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 2c3116ee530..a107f71df80 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3199,6 +3199,87 @@ static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info, /* Use ID byte 4 to distinguish S25FS256T and S25Hx-T */ #define S25FS256T_ID4 (0x08) +/* Number of dummy cycle for Read Any Register (RDAR) op. */ +#define S25FS_S_RDAR_DUMMY 8 + +static int s25fs_s_quad_enable(struct spi_nor *nor) +{ + return spansion_quad_enable_volatile(nor, 0, S25FS_S_RDAR_DUMMY); +} + +static int s25fs_s_erase_non_uniform(struct spi_nor *nor, loff_t addr) +{ + /* Support 8 x 4KB sectors at bottom */ + return spansion_erase_non_uniform(nor, addr, SPINOR_OP_BE_4K_4B, 0, SZ_32K); +} + +static int s25fs_s_setup(struct spi_nor *nor, const struct flash_info *info, +const struct spi_nor_flash_parameter *params) +{ + int ret; + u8 cfr3v; + + /* Bank Address Register is not supported */ + if (CONFIG_IS_ENABLED(SPI_FLASH_BAR)) + return -EOPNOTSUPP; + + /* +* Read CR3V to check if uniform sector is selected. If not, assign an +* erase hook that supports non-uniform erase. +*/ + ret = spansion_read_any_reg(nor, SPINOR_REG_ADDR_CFR3V, + S25FS_S_RDAR_DUMMY, ); + if (ret) + return ret; + if (!(cfr3v & CFR3V_UNHYSA)) + nor->erase = s25fs_s_erase_non_uniform; + + return spi_nor_default_setup(nor, info, params); +} + +static void s25fs_s_default_init(struct spi_nor *nor) +{ + nor->setup = s25fs_s_setup; +} + +static int s25fs_s_post_bfpt_fixup(struct spi_nor *nor, + const struct sfdp_parameter_header *header, + const struct sfdp_bfpt *bfpt, + struct spi_nor_flash_parameter *params) +{ + /* The erase size is set to 4K from BFPT, but it's wrong. Fix it. */ + nor->erase_opcode = SPINOR_OP_SE; + nor->mtd.erasesize = nor->info->sector_size; + + /* The S25FS-S chip family reports 512-byte pages in BFPT but +* in reality the write buffer still wraps at the safe default +* of 256 bytes. Overwrite the page size advertised by BFPT +* to get the writes working. +*/ + params->page_size = 256; + + return 0; +} + +static void s25fs_s_post_sfdp_fixup(struct spi_nor *nor, + struct spi_nor_flash_parameter *params) +{ + /* READ_1_1_2 is not supported */ + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_2; + /* READ_1_1_4 is not supported */ + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_4; + /* 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 = s25fs_s_quad_enable; +} + +static struct spi_nor_fixups s25fs_s_fixups = { + .default_init = s25fs_s_default_init, + .post_bfpt = s25fs_s_post_bfpt_fixup, + .post_sfdp = s25fs_s_post_sfdp_fixup, +}; + static int s25_mdp_ready(struct spi_nor *nor) { u32 addr; @@ -3897,6 +3978,10 @@ void spi_nor_set_fixups(struct spi_nor *nor) if (CONFIG_IS_ENABLED(SPI_FLASH_BAR) && !strcmp(nor->info->name, "s25fl256l")) nor->fixups = _fixups; + + /* For FS-S (family ID = 0x81) */ + if (JEDEC_MFR(nor->info) == SNOR_MFR_SPANSION && nor->info->id[5] == 0x81) + nor->fixups = _s_fixups; #endif #ifdef CONFIG_SPI_FLASH_MT35XU -- 2.39.2
Re: [PATCH] mtd: spi-nor-core: Add fixups for s25fs512s
On 9/18/2021 2:01 AM, Pratyush Yadav wrote: > On 15/09/21 12:49PM, tkuw584...@gmail.com wrote: >> From: Takahiro Kuwano >> >> The current S25FS512S support has following issues that need to be fixed. >> >> - Non-uniform sectors by factory default. The setting needs to be >> checked and assign erase hook as needed. >> - Page size is wrongly advertised in SFDP. >> - READ_1_1_2 (3Bh/3Ch), READ_1_1_4 (6Bh/6Ch), and PP_1_1_4 (32h/34h) >> are not supported. >> - Bank Address Register (BAR) is not supported. >> >> In addtion, volatile version of Quad Enable is used for safety. >> >> For future use, the fixups is assigned for S25FS-S family. >> >> The datasheet can be found in the following link. >> https://www.cypress.com/file/216376/download >> >> Tested on Xilinx Zynq-7000 FPGA board. >> >> Signed-off-by: Takahiro Kuwano >> --- >> drivers/mtd/spi/spi-nor-core.c | 101 + >> 1 file changed, 101 insertions(+) >> >> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c >> index d5d905fa5a..5e847ebf6a 100644 >> --- a/drivers/mtd/spi/spi-nor-core.c >> +++ b/drivers/mtd/spi/spi-nor-core.c >> @@ -3097,6 +3097,102 @@ static int spi_nor_setup(struct spi_nor *nor, const >> struct flash_info *info, >> } >> >> #ifdef CONFIG_SPI_FLASH_SPANSION >> +static int s25fs_s_quad_enable(struct spi_nor *nor) >> +{ >> +return spansion_quad_enable_volatile(nor, 0, 8); >> +} >> + >> +static int s25fs_s_erase_non_uniform(struct spi_nor *nor, loff_t addr) >> +{ >> +/* Support 8 x 4KB sectors at bottom */ >> +return spansion_erase_non_uniform(nor, addr, SPINOR_OP_BE_4K_4B, 0, >> + SZ_32K); >> +} >> + >> +static int s25fs_s_setup(struct spi_nor *nor, const struct flash_info *info, >> + const struct spi_nor_flash_parameter *params) >> +{ >> +int ret; >> +u8 cfr3v; >> + >> +#ifdef CONFIG_SPI_FLASH_BAR > > Avoid using #ifdef. Prefer if (CONFIG_IS_ENABLED(SPI_FLASH_BAR) instead. > OK, I will do it. >> +return -ENOTSUPP; /* Bank Address Register is not supported */ > > Same question as the other patch I just reviewed. What would happen if > we don't do this? > If we don't do this, the spi-nor tries to use BAR with 3-byte addressing and the ops to the >16MB memory space will be performed in the first 16MB memory space. >> +#endif >> +/* >> + * Read CR3V to check if uniform sector is selected. If not, assign an >> + * erase hook that supports non-uniform erase. >> + */ >> +ret = spansion_read_any_reg(nor, SPINOR_REG_ADDR_CFR3V, 8, ); > > Don't hard-code the 8. Use a #define instead. > I will do it. >> +if (ret) >> +return ret; >> +if (!(cfr3v & CFR3V_UNHYSA)) >> +nor->erase = s25fs_s_erase_non_uniform; > > Ok, but you still don't check for top/bottom configuration. This is the > same as the S25Hx flashes, right? > Yes, right. >> + >> +return spi_nor_default_setup(nor, info, params); >> +} >> + >> +static void s25fs_s_default_init(struct spi_nor *nor) >> +{ >> +nor->setup = s25fs_s_setup; >> +} >> + >> +static int s25fs_s_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; >> +u8 cfr3v; >> + >> +/* The erase size is set to 4K from BFPT, but it's wrong. Fix it. */ >> +nor->erase_opcode = SPINOR_OP_SE; >> +nor->mtd.erasesize = nor->info->sector_size; >> + >> +if (params->size > SZ_16M) { >> +ret = nor->write_reg(nor, SPINOR_OP_EN4B, NULL, 0); > > set_4byte() should call this as well, right? Why call it here? > The s25fs-s has SNOR_MFR_SPANSION (0x01). The set_4byte() issues SPINOR_OP_BRWR for SNOR_MFR_SPANSION but the s25fs-s does not support it. The s25fs-s support SPINOR_OP_EN4B instead. >> +if (ret) >> +return ret; >> +nor->addr_width = 4; >> +} else { >> +nor->addr_width = 3; >> +} >> + >> +/* >> + * 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, SPINOR_REG_ADDR_CFR3V, 8, ); >> +if (ret) >> +return ret; >> + >> +if (cfr3v & CFR3V_PGMBUF) >> +params->page_size = 512; >> +else >> +params->page_size = 256; >> + >> +return 0; >> +} >> + >> +static void s25fs_s_post_sfdp_fixup(struct spi_nor *nor, >> +struct spi_nor_flash_parameter *params) >> +{ >> +/* READ_1_1_2 is not supported */ >> +params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_2; >> +/* READ_1_1_4 is not supported */ >> +params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_4; >> +
Re: [PATCH] mtd: spi-nor-core: Add fixups for s25fs512s
On 15/09/21 12:49PM, tkuw584...@gmail.com wrote: > From: Takahiro Kuwano > > The current S25FS512S support has following issues that need to be fixed. > > - Non-uniform sectors by factory default. The setting needs to be > checked and assign erase hook as needed. > - Page size is wrongly advertised in SFDP. > - READ_1_1_2 (3Bh/3Ch), READ_1_1_4 (6Bh/6Ch), and PP_1_1_4 (32h/34h) > are not supported. > - Bank Address Register (BAR) is not supported. > > In addtion, volatile version of Quad Enable is used for safety. > > For future use, the fixups is assigned for S25FS-S family. > > The datasheet can be found in the following link. > https://www.cypress.com/file/216376/download > > Tested on Xilinx Zynq-7000 FPGA board. > > Signed-off-by: Takahiro Kuwano > --- > drivers/mtd/spi/spi-nor-core.c | 101 + > 1 file changed, 101 insertions(+) > > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > index d5d905fa5a..5e847ebf6a 100644 > --- a/drivers/mtd/spi/spi-nor-core.c > +++ b/drivers/mtd/spi/spi-nor-core.c > @@ -3097,6 +3097,102 @@ static int spi_nor_setup(struct spi_nor *nor, const > struct flash_info *info, > } > > #ifdef CONFIG_SPI_FLASH_SPANSION > +static int s25fs_s_quad_enable(struct spi_nor *nor) > +{ > + return spansion_quad_enable_volatile(nor, 0, 8); > +} > + > +static int s25fs_s_erase_non_uniform(struct spi_nor *nor, loff_t addr) > +{ > + /* Support 8 x 4KB sectors at bottom */ > + return spansion_erase_non_uniform(nor, addr, SPINOR_OP_BE_4K_4B, 0, > + SZ_32K); > +} > + > +static int s25fs_s_setup(struct spi_nor *nor, const struct flash_info *info, > + const struct spi_nor_flash_parameter *params) > +{ > + int ret; > + u8 cfr3v; > + > +#ifdef CONFIG_SPI_FLASH_BAR Avoid using #ifdef. Prefer if (CONFIG_IS_ENABLED(SPI_FLASH_BAR) instead. > + return -ENOTSUPP; /* Bank Address Register is not supported */ Same question as the other patch I just reviewed. What would happen if we don't do this? > +#endif > + /* > + * Read CR3V to check if uniform sector is selected. If not, assign an > + * erase hook that supports non-uniform erase. > + */ > + ret = spansion_read_any_reg(nor, SPINOR_REG_ADDR_CFR3V, 8, ); Don't hard-code the 8. Use a #define instead. > + if (ret) > + return ret; > + if (!(cfr3v & CFR3V_UNHYSA)) > + nor->erase = s25fs_s_erase_non_uniform; Ok, but you still don't check for top/bottom configuration. This is the same as the S25Hx flashes, right? > + > + return spi_nor_default_setup(nor, info, params); > +} > + > +static void s25fs_s_default_init(struct spi_nor *nor) > +{ > + nor->setup = s25fs_s_setup; > +} > + > +static int s25fs_s_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; > + u8 cfr3v; > + > + /* The erase size is set to 4K from BFPT, but it's wrong. Fix it. */ > + nor->erase_opcode = SPINOR_OP_SE; > + nor->mtd.erasesize = nor->info->sector_size; > + > + if (params->size > SZ_16M) { > + ret = nor->write_reg(nor, SPINOR_OP_EN4B, NULL, 0); set_4byte() should call this as well, right? Why call it here? > + if (ret) > + return ret; > + nor->addr_width = 4; > + } else { > + nor->addr_width = 3; > + } > + > + /* > + * 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, SPINOR_REG_ADDR_CFR3V, 8, ); > + if (ret) > + return ret; > + > + if (cfr3v & CFR3V_PGMBUF) > + params->page_size = 512; > + else > + params->page_size = 256; > + > + return 0; > +} > + > +static void s25fs_s_post_sfdp_fixup(struct spi_nor *nor, > + struct spi_nor_flash_parameter *params) > +{ > + /* READ_1_1_2 is not supported */ > + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_2; > + /* READ_1_1_4 is not supported */ > + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_4; > + /* 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 = s25fs_s_quad_enable; > +} > + > +static struct spi_nor_fixups s25fs_s_fixups = { > + .default_init = s25fs_s_default_init, > + .post_bfpt = s25fs_s_post_bfpt_fixup, > + .post_sfdp = s25fs_s_post_sfdp_fixup, > +}; > + > static int s25hx_t_mdp_ready(struct spi_nor *nor) > { > u32 addr; > @@ -3644,6 +3740,11 @@ void spi_nor_set_fixups(struct
[PATCH] mtd: spi-nor-core: Add fixups for s25fs512s
From: Takahiro Kuwano The current S25FS512S support has following issues that need to be fixed. - Non-uniform sectors by factory default. The setting needs to be checked and assign erase hook as needed. - Page size is wrongly advertised in SFDP. - READ_1_1_2 (3Bh/3Ch), READ_1_1_4 (6Bh/6Ch), and PP_1_1_4 (32h/34h) are not supported. - Bank Address Register (BAR) is not supported. In addtion, volatile version of Quad Enable is used for safety. For future use, the fixups is assigned for S25FS-S family. The datasheet can be found in the following link. https://www.cypress.com/file/216376/download Tested on Xilinx Zynq-7000 FPGA board. Signed-off-by: Takahiro Kuwano --- drivers/mtd/spi/spi-nor-core.c | 101 + 1 file changed, 101 insertions(+) diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index d5d905fa5a..5e847ebf6a 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3097,6 +3097,102 @@ static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info, } #ifdef CONFIG_SPI_FLASH_SPANSION +static int s25fs_s_quad_enable(struct spi_nor *nor) +{ + return spansion_quad_enable_volatile(nor, 0, 8); +} + +static int s25fs_s_erase_non_uniform(struct spi_nor *nor, loff_t addr) +{ + /* Support 8 x 4KB sectors at bottom */ + return spansion_erase_non_uniform(nor, addr, SPINOR_OP_BE_4K_4B, 0, + SZ_32K); +} + +static int s25fs_s_setup(struct spi_nor *nor, const struct flash_info *info, +const struct spi_nor_flash_parameter *params) +{ + int ret; + u8 cfr3v; + +#ifdef CONFIG_SPI_FLASH_BAR + return -ENOTSUPP; /* Bank Address Register is not supported */ +#endif + /* +* Read CR3V to check if uniform sector is selected. If not, assign an +* erase hook that supports non-uniform erase. +*/ + ret = spansion_read_any_reg(nor, SPINOR_REG_ADDR_CFR3V, 8, ); + if (ret) + return ret; + if (!(cfr3v & CFR3V_UNHYSA)) + nor->erase = s25fs_s_erase_non_uniform; + + return spi_nor_default_setup(nor, info, params); +} + +static void s25fs_s_default_init(struct spi_nor *nor) +{ + nor->setup = s25fs_s_setup; +} + +static int s25fs_s_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; + u8 cfr3v; + + /* The erase size is set to 4K from BFPT, but it's wrong. Fix it. */ + nor->erase_opcode = SPINOR_OP_SE; + nor->mtd.erasesize = nor->info->sector_size; + + if (params->size > SZ_16M) { + ret = nor->write_reg(nor, SPINOR_OP_EN4B, NULL, 0); + if (ret) + return ret; + nor->addr_width = 4; + } else { + nor->addr_width = 3; + } + + /* +* 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, SPINOR_REG_ADDR_CFR3V, 8, ); + if (ret) + return ret; + + if (cfr3v & CFR3V_PGMBUF) + params->page_size = 512; + else + params->page_size = 256; + + return 0; +} + +static void s25fs_s_post_sfdp_fixup(struct spi_nor *nor, + struct spi_nor_flash_parameter *params) +{ + /* READ_1_1_2 is not supported */ + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_2; + /* READ_1_1_4 is not supported */ + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_4; + /* 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 = s25fs_s_quad_enable; +} + +static struct spi_nor_fixups s25fs_s_fixups = { + .default_init = s25fs_s_default_init, + .post_bfpt = s25fs_s_post_bfpt_fixup, + .post_sfdp = s25fs_s_post_sfdp_fixup, +}; + static int s25hx_t_mdp_ready(struct spi_nor *nor) { u32 addr; @@ -3644,6 +3740,11 @@ void spi_nor_set_fixups(struct spi_nor *nor) break; } } + + /* For FS-S (family ID = 0x81) */ + if (JEDEC_MFR(nor->info) == SNOR_MFR_SPANSION && + nor->info->id[5] == 0x81) + nor->fixups = _s_fixups; #endif #ifdef CONFIG_SPI_FLASH_S28HS512T -- 2.25.1