Re: [PATCH v2] mtd: spi-nor-core: Add fixups for s25fs512s

[Jagan Teki]

On Thu, Sep 30, 2021 at 11:02 AM  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 
> ---
> Changes in v2:
>   - Add #define S25FS_S_RDAR_DUMMY and remove hard-coding
>   - Remove #ifdef CONFIG_SPI_FLASH_BAR
>
>  drivers/mtd/spi/spi-nor-core.c | 108 +
>  1 file changed, 108 insertions(+)
>
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index f1b4e5ea8e..349b2f3f23 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -3099,6 +3099,109 @@ static int spi_nor_setup(struct spi_nor *nor, const 
> struct flash_info *info,
>  }
>
>  #ifdef CONFIG_SPI_FLASH_SPANSION
> +
> +/* 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 -ENOTSUPP;
> +
> +   /*
> +* 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)
> +{
> +   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,
> +   S25FS_S_RDAR_DUMMY, );
> +   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 

Re: [PATCH v2] mtd: spi-nor-core: Add fixups for s25fs512s

[Pratyush Yadav]

On 30/09/21 02:32PM, 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 

Acked-by: Pratyush Yadav 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

[PATCH v2] mtd: spi-nor-core: Add fixups for s25fs512s

[tkuw584924]

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 
---
Changes in v2:
  - Add #define S25FS_S_RDAR_DUMMY and remove hard-coding
  - Remove #ifdef CONFIG_SPI_FLASH_BAR

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

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index f1b4e5ea8e..349b2f3f23 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -3099,6 +3099,109 @@ static int spi_nor_setup(struct spi_nor *nor, const 
struct flash_info *info,
 }
 
 #ifdef CONFIG_SPI_FLASH_SPANSION
+
+/* 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 -ENOTSUPP;
+
+   /*
+* 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)
+{
+   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,
+   S25FS_S_RDAR_DUMMY, );
+   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;
@@ -3646,6 +3749,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)
+