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

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

[Marek Vasut]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

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 <marek.vasut+renesas@mailbox.org>
Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
Signed-off-by: Hai Pham <hai.pham.ud@renesas.com>
Signed-off-by: Cong Dang <cong.dang.xn@renesas.com>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
Cc: Vignesh R <vigneshr@ti.com>
---
 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, &cfr3v);
+	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 = &s25fl256l_fixups;
+
+	/* For FS-S (family ID = 0x81)  */
+	if (JEDEC_MFR(nor->info) == SNOR_MFR_SPANSION && nor->info->id[5] == 0x81)
+		nor->fixups = &s25fs_s_fixups;
 #endif
 
 #ifdef CONFIG_SPI_FLASH_MT35XU
-- 
2.39.2

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

[Jagan Teki]

On Sun, Apr 23, 2023 at 5:26 AM Marek Vasut
<marek.vasut+renesas@mailbox.org> wrote:
>
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>
> 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 <marek.vasut+renesas@mailbox.org>
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> Signed-off-by: Hai Pham <hai.pham.ud@renesas.com>
> Signed-off-by: Cong Dang <cong.dang.xn@renesas.com>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---

Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>

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

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

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 <Takahiro.Kuwano@infineon.com>
---
 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, &cfr3v);
+	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, &cfr3v);
+	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 = &s25fs_s_fixups;
 #endif
 
 #ifdef CONFIG_SPI_FLASH_S28HS512T
-- 
2.25.1

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

[Pratyush Yadav]

On 15/09/21 12:49PM, tkuw584924@gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> 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 <Takahiro.Kuwano@infineon.com>
> ---
>  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, &cfr3v);

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, &cfr3v);
> +	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 = &s25fs_s_fixups;

Ok.

>  #endif
>  
>  #ifdef CONFIG_SPI_FLASH_S28HS512T
> -- 
> 2.25.1
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

[Takahiro Kuwano]

On 9/18/2021 2:01 AM, Pratyush Yadav wrote:
> On 15/09/21 12:49PM, tkuw584924@gmail.com wrote:
>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>
>> 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 <Takahiro.Kuwano@infineon.com>
>> ---
>>  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, &cfr3v);
> 
> 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, &cfr3v);
>> +	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 = &s25fs_s_fixups;
> 
> Ok.
> 
>>  #endif
>>  
>>  #ifdef CONFIG_SPI_FLASH_S28HS512T
>> -- 
>> 2.25.1
>>
>