[PATCH] mtd: spi-nor: spansion: add clear sr fixup for s25fl-l family

[PATCH] mtd: spi-nor: spansion: add clear sr fixup for s25fl-l family

[yaliang.wang]

From: Yaliang Wang <Yaliang.Wang@windriver.com>

Spansion S25FL-L family flashes s25fl064l/s25fl128l/s25fl256l can't
automatically recover from programming/erase errors, the Status Register
error bits inflecting the errors will not change until a Clear Status
Register command be executed.

Same thing also happens on other Spansion flash families, they've
properly handled it, USE_CLSR manufacturer flag was introduced for this
purpose, but S25FL-L cannot simply reuse their work, because S25FL-L has
the different error bit settings. S25FL-L defines programming/erase
error bits in Status Register 2, whereas the other families define the
same error bits in Status Register 1, causing S25FL-L needs a different
method to handle this problem.

Cc: stable@vger.kernel.org
Fixes: 0074a8f3b303 ("mtd: spi-nor: Add support for s25fl128l and s25fl256l")
Fixes: d8b494a32889 ("mtd: spi-nor: Add support for Spansion S25FL064L")
Signed-off-by: Yaliang Wang <Yaliang.Wang@windriver.com>
---
 drivers/mtd/spi-nor/spansion.c | 119 ++++++++++++++++++++++++++-------
 1 file changed, 93 insertions(+), 26 deletions(-)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 0150049007be..8f353ddda5f5 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -14,6 +14,7 @@
 #define SPINOR_OP_CLSR		0x30	/* Clear status register 1 */
 #define SPINOR_OP_RD_ANY_REG			0x65	/* Read any register */
 #define SPINOR_OP_WR_ANY_REG			0x71	/* Write any register */
+#define SPINOR_REG_CYPRESS_SR2V			0x00800001
 #define SPINOR_REG_CYPRESS_CFR1V		0x00800002
 #define SPINOR_REG_CYPRESS_CFR1V_QUAD_EN	BIT(1)	/* Quad Enable */
 #define SPINOR_REG_CYPRESS_CFR2V		0x00800003
@@ -25,6 +26,10 @@
 #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS	0
 #define SPINOR_OP_CYPRESS_RD_FAST		0xee
 
+/* s25fl-l family specific */
+#define S25FL_L_SR2V_P_ERR			BIT(5)	/* Programming Error Occurred */
+#define S25FL_L_SR2V_E_ERR			BIT(6)	/* Erase Error Occurred */
+
 /* Cypress SPI NOR flash operations. */
 #define CYPRESS_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf)		\
 	SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 0),		\
@@ -44,6 +49,29 @@
 		   SPI_MEM_OP_NO_DUMMY,					\
 		   SPI_MEM_OP_NO_DATA)
 
+/**
+ * spansion_nor_clear_sr() - Clear the Status Register.
+ * @nor:	pointer to 'struct spi_nor'.
+ */
+static void spansion_nor_clear_sr(struct spi_nor *nor)
+{
+	int ret;
+
+	if (nor->spimem) {
+		struct spi_mem_op op = SPANSION_CLSR_OP;
+
+		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
+						       NULL, 0);
+	}
+
+	if (ret)
+		dev_dbg(nor->dev, "error %d clearing SR\n", ret);
+}
+
 static int cypress_nor_octal_dtr_en(struct spi_nor *nor)
 {
 	struct spi_mem_op op;
@@ -342,6 +370,65 @@ static const struct spi_nor_fixups s25fs_s_nor_fixups = {
 	.post_bfpt = s25fs_s_nor_post_bfpt_fixups,
 };
 
+/**
+ * s25fl_l_sr_ready_and_clear() - S25FL_L family flashes need to query
+ * Status Register 1 to check if the flash is ready and clear it if
+ * there are Programming/Erase errors in Status Register 2.
+ * @nor:	pointer to 'struct spi_nor'.
+ *
+ * Return: 1 if ready, 0 if not ready, -errno on errors.
+ */
+static int s25fl_l_sr_ready_and_clear(struct spi_nor *nor)
+{
+	int ret;
+	u8 addr_mode_nbytes = nor->params->addr_mode_nbytes;
+	struct spi_mem_op op =
+		CYPRESS_NOR_RD_ANY_REG_OP(addr_mode_nbytes,
+					  SPINOR_REG_CYPRESS_SR2V,
+					  &nor->bouncebuf[1]);
+
+	/* Read Status Register 1 */
+	ret = spi_nor_read_sr(nor, nor->bouncebuf);
+	if (ret)
+		return ret;
+
+	/* RDSR2 command isn't available in QPI mode, use RDAR instead  */
+	ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
+	if (ret)
+		return ret;
+
+	if (nor->bouncebuf[1] & (S25FL_L_SR2V_P_ERR | S25FL_L_SR2V_E_ERR)) {
+		if (nor->bouncebuf[1] & S25FL_L_SR2V_E_ERR)
+			dev_err(nor->dev, "Erase Error occurred\n");
+		else
+			dev_err(nor->dev, "Programming Error occurred\n");
+
+		spansion_nor_clear_sr(nor);
+
+		/*
+		 * WEL bit remains set to one when an erase or page program
+		 * error occurs. Issue a Write Disable command to protect
+		 * against inadvertent writes that can possibly corrupt the
+		 * contents of the memory.
+		 */
+		ret = spi_nor_write_disable(nor);
+		if (ret)
+			return ret;
+
+		return -EIO;
+	}
+
+	return !(nor->bouncebuf[0] & SR_WIP);
+}
+
+static void s25fl_l_late_init(struct spi_nor *nor)
+{
+	nor->params->ready = s25fl_l_sr_ready_and_clear;
+}
+
+static const struct spi_nor_fixups s25fl_l_fixups = {
+	.late_init = s25fl_l_late_init,
+};
 static const struct flash_info spansion_nor_parts[] = {
 	/* Spansion/Cypress -- single (large) sector size only, at least
 	 * for the chips listed here (without boot sectors).
@@ -428,13 +515,16 @@ static const struct flash_info spansion_nor_parts[] = {
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ) },
 	{ "s25fl064l",  INFO(0x016017,      0,  64 * 1024, 128)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
-		FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &s25fl_l_fixups },
 	{ "s25fl128l",  INFO(0x016018,      0,  64 * 1024, 256)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
-		FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &s25fl_l_fixups },
 	{ "s25fl256l",  INFO(0x016019,      0,  64 * 1024, 512)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
-		FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &s25fl_l_fixups },
 	{ "s25hl512t",  INFO6(0x342a1a, 0x0f0390, 256 * 1024, 256)
 		PARSE_SFDP
 		MFR_FLAGS(USE_CLSR)
@@ -460,29 +550,6 @@ static const struct flash_info spansion_nor_parts[] = {
 	},
 };
 
-/**
- * spansion_nor_clear_sr() - Clear the Status Register.
- * @nor:	pointer to 'struct spi_nor'.
- */
-static void spansion_nor_clear_sr(struct spi_nor *nor)
-{
-	int ret;
-
-	if (nor->spimem) {
-		struct spi_mem_op op = SPANSION_CLSR_OP;
-
-		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
-
-		ret = spi_mem_exec_op(nor->spimem, &op);
-	} else {
-		ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
-						       NULL, 0);
-	}
-
-	if (ret)
-		dev_dbg(nor->dev, "error %d clearing SR\n", ret);
-}
-
 /**
  * spansion_nor_sr_ready_and_clear() - Query the Status Register to see if the
  * flash is ready for new commands and clear it if there are any errors.
-- 
2.34.1

Re: [PATCH] mtd: spi-nor: spansion: add clear sr fixup for s25fl-l family

[Tudor.Ambarus]

+ Takahiro

Hi, Takahiro,

Would you please review/test this spansion patch?

Thanks,
ta

On 10/18/22 12:57, yaliang.wang@windriver.com wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> From: Yaliang Wang <Yaliang.Wang@windriver.com>
> 
> Spansion S25FL-L family flashes s25fl064l/s25fl128l/s25fl256l can't
> automatically recover from programming/erase errors, the Status Register
> error bits inflecting the errors will not change until a Clear Status
> Register command be executed.
> 
> Same thing also happens on other Spansion flash families, they've
> properly handled it, USE_CLSR manufacturer flag was introduced for this
> purpose, but S25FL-L cannot simply reuse their work, because S25FL-L has
> the different error bit settings. S25FL-L defines programming/erase
> error bits in Status Register 2, whereas the other families define the
> same error bits in Status Register 1, causing S25FL-L needs a different
> method to handle this problem.
> 
> Cc: stable@vger.kernel.org
> Fixes: 0074a8f3b303 ("mtd: spi-nor: Add support for s25fl128l and s25fl256l")
> Fixes: d8b494a32889 ("mtd: spi-nor: Add support for Spansion S25FL064L")
> Signed-off-by: Yaliang Wang <Yaliang.Wang@windriver.com>
> ---
>  drivers/mtd/spi-nor/spansion.c | 119 ++++++++++++++++++++++++++-------
>  1 file changed, 93 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index 0150049007be..8f353ddda5f5 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -14,6 +14,7 @@
>  #define SPINOR_OP_CLSR         0x30    /* Clear status register 1 */
>  #define SPINOR_OP_RD_ANY_REG                   0x65    /* Read any register */
>  #define SPINOR_OP_WR_ANY_REG                   0x71    /* Write any register */
> +#define SPINOR_REG_CYPRESS_SR2V                        0x00800001
>  #define SPINOR_REG_CYPRESS_CFR1V               0x00800002
>  #define SPINOR_REG_CYPRESS_CFR1V_QUAD_EN       BIT(1)  /* Quad Enable */
>  #define SPINOR_REG_CYPRESS_CFR2V               0x00800003
> @@ -25,6 +26,10 @@
>  #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS    0
>  #define SPINOR_OP_CYPRESS_RD_FAST              0xee
> 
> +/* s25fl-l family specific */
> +#define S25FL_L_SR2V_P_ERR                     BIT(5)  /* Programming Error Occurred */
> +#define S25FL_L_SR2V_E_ERR                     BIT(6)  /* Erase Error Occurred */
> +
>  /* Cypress SPI NOR flash operations. */
>  #define CYPRESS_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf)             \
>         SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 0),             \
> @@ -44,6 +49,29 @@
>                    SPI_MEM_OP_NO_DUMMY,                                 \
>                    SPI_MEM_OP_NO_DATA)
> 
> +/**
> + * spansion_nor_clear_sr() - Clear the Status Register.
> + * @nor:       pointer to 'struct spi_nor'.
> + */
> +static void spansion_nor_clear_sr(struct spi_nor *nor)
> +{
> +       int ret;
> +
> +       if (nor->spimem) {
> +               struct spi_mem_op op = SPANSION_CLSR_OP;
> +
> +               spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> +
> +               ret = spi_mem_exec_op(nor->spimem, &op);
> +       } else {
> +               ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
> +                                                      NULL, 0);
> +       }
> +
> +       if (ret)
> +               dev_dbg(nor->dev, "error %d clearing SR\n", ret);
> +}
> +
>  static int cypress_nor_octal_dtr_en(struct spi_nor *nor)
>  {
>         struct spi_mem_op op;
> @@ -342,6 +370,65 @@ static const struct spi_nor_fixups s25fs_s_nor_fixups = {
>         .post_bfpt = s25fs_s_nor_post_bfpt_fixups,
>  };
> 
> +/**
> + * s25fl_l_sr_ready_and_clear() - S25FL_L family flashes need to query
> + * Status Register 1 to check if the flash is ready and clear it if
> + * there are Programming/Erase errors in Status Register 2.
> + * @nor:       pointer to 'struct spi_nor'.
> + *
> + * Return: 1 if ready, 0 if not ready, -errno on errors.
> + */
> +static int s25fl_l_sr_ready_and_clear(struct spi_nor *nor)
> +{
> +       int ret;
> +       u8 addr_mode_nbytes = nor->params->addr_mode_nbytes;
> +       struct spi_mem_op op =
> +               CYPRESS_NOR_RD_ANY_REG_OP(addr_mode_nbytes,
> +                                         SPINOR_REG_CYPRESS_SR2V,
> +                                         &nor->bouncebuf[1]);
> +
> +       /* Read Status Register 1 */
> +       ret = spi_nor_read_sr(nor, nor->bouncebuf);
> +       if (ret)
> +               return ret;
> +
> +       /* RDSR2 command isn't available in QPI mode, use RDAR instead  */
> +       ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
> +       if (ret)
> +               return ret;
> +
> +       if (nor->bouncebuf[1] & (S25FL_L_SR2V_P_ERR | S25FL_L_SR2V_E_ERR)) {
> +               if (nor->bouncebuf[1] & S25FL_L_SR2V_E_ERR)
> +                       dev_err(nor->dev, "Erase Error occurred\n");
> +               else
> +                       dev_err(nor->dev, "Programming Error occurred\n");
> +
> +               spansion_nor_clear_sr(nor);
> +
> +               /*
> +                * WEL bit remains set to one when an erase or page program
> +                * error occurs. Issue a Write Disable command to protect
> +                * against inadvertent writes that can possibly corrupt the
> +                * contents of the memory.
> +                */
> +               ret = spi_nor_write_disable(nor);
> +               if (ret)
> +                       return ret;
> +
> +               return -EIO;
> +       }
> +
> +       return !(nor->bouncebuf[0] & SR_WIP);
> +}
> +
> +static void s25fl_l_late_init(struct spi_nor *nor)
> +{
> +       nor->params->ready = s25fl_l_sr_ready_and_clear;
> +}
> +
> +static const struct spi_nor_fixups s25fl_l_fixups = {
> +       .late_init = s25fl_l_late_init,
> +};
>  static const struct flash_info spansion_nor_parts[] = {
>         /* Spansion/Cypress -- single (large) sector size only, at least
>          * for the chips listed here (without boot sectors).
> @@ -428,13 +515,16 @@ static const struct flash_info spansion_nor_parts[] = {
>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ) },
>         { "s25fl064l",  INFO(0x016017,      0,  64 * 1024, 128)
>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> -               FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> +               FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> +               .fixups = &s25fl_l_fixups },
>         { "s25fl128l",  INFO(0x016018,      0,  64 * 1024, 256)
>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> -               FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> +               FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> +               .fixups = &s25fl_l_fixups },
>         { "s25fl256l",  INFO(0x016019,      0,  64 * 1024, 512)
>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> -               FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> +               FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> +               .fixups = &s25fl_l_fixups },
>         { "s25hl512t",  INFO6(0x342a1a, 0x0f0390, 256 * 1024, 256)
>                 PARSE_SFDP
>                 MFR_FLAGS(USE_CLSR)
> @@ -460,29 +550,6 @@ static const struct flash_info spansion_nor_parts[] = {
>         },
>  };
> 
> -/**
> - * spansion_nor_clear_sr() - Clear the Status Register.
> - * @nor:       pointer to 'struct spi_nor'.
> - */
> -static void spansion_nor_clear_sr(struct spi_nor *nor)
> -{
> -       int ret;
> -
> -       if (nor->spimem) {
> -               struct spi_mem_op op = SPANSION_CLSR_OP;
> -
> -               spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> -
> -               ret = spi_mem_exec_op(nor->spimem, &op);
> -       } else {
> -               ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
> -                                                      NULL, 0);
> -       }
> -
> -       if (ret)
> -               dev_dbg(nor->dev, "error %d clearing SR\n", ret);
> -}
> -
>  /**
>   * spansion_nor_sr_ready_and_clear() - Query the Status Register to see if the
>   * flash is ready for new commands and clear it if there are any errors.
> --
> 2.34.1
> 

-- 
Cheers,
ta

Re: [PATCH] mtd: spi-nor: spansion: add clear sr fixup for s25fl-l family

[Takahiro Kuwano]

Hi Yaliang and Tudor,

On 11/22/2022 7:28 PM, Tudor.Ambarus@microchip.com wrote:
> + Takahiro
> 
> Hi, Takahiro,
> 
> Would you please review/test this spansion patch?
> 
> Thanks,
> ta
> 
> On 10/18/22 12:57, yaliang.wang@windriver.com wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> From: Yaliang Wang <Yaliang.Wang@windriver.com>
>>
>> Spansion S25FL-L family flashes s25fl064l/s25fl128l/s25fl256l can't
>> automatically recover from programming/erase errors, the Status Register
>> error bits inflecting the errors will not change until a Clear Status
>> Register command be executed.
>>
>> Same thing also happens on other Spansion flash families, they've
>> properly handled it, USE_CLSR manufacturer flag was introduced for this
>> purpose, but S25FL-L cannot simply reuse their work, because S25FL-L has
>> the different error bit settings. S25FL-L defines programming/erase
>> error bits in Status Register 2, whereas the other families define the
>> same error bits in Status Register 1, causing S25FL-L needs a different
>> method to handle this problem.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 0074a8f3b303 ("mtd: spi-nor: Add support for s25fl128l and s25fl256l")
>> Fixes: d8b494a32889 ("mtd: spi-nor: Add support for Spansion S25FL064L")
>> Signed-off-by: Yaliang Wang <Yaliang.Wang@windriver.com>
>> ---
>>  drivers/mtd/spi-nor/spansion.c | 119 ++++++++++++++++++++++++++-------
>>  1 file changed, 93 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
>> index 0150049007be..8f353ddda5f5 100644
>> --- a/drivers/mtd/spi-nor/spansion.c
>> +++ b/drivers/mtd/spi-nor/spansion.c
>> @@ -14,6 +14,7 @@
>>  #define SPINOR_OP_CLSR         0x30    /* Clear status register 1 */
>>  #define SPINOR_OP_RD_ANY_REG                   0x65    /* Read any register */
>>  #define SPINOR_OP_WR_ANY_REG                   0x71    /* Write any register */
>> +#define SPINOR_REG_CYPRESS_SR2V                        0x00800001
>>  #define SPINOR_REG_CYPRESS_CFR1V               0x00800002
>>  #define SPINOR_REG_CYPRESS_CFR1V_QUAD_EN       BIT(1)  /* Quad Enable */
>>  #define SPINOR_REG_CYPRESS_CFR2V               0x00800003
>> @@ -25,6 +26,10 @@
>>  #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS    0
>>  #define SPINOR_OP_CYPRESS_RD_FAST              0xee
>>
>> +/* s25fl-l family specific */
>> +#define S25FL_L_SR2V_P_ERR                     BIT(5)  /* Programming Error Occurred */
>> +#define S25FL_L_SR2V_E_ERR                     BIT(6)  /* Erase Error Occurred */
>> +
>>  /* Cypress SPI NOR flash operations. */
>>  #define CYPRESS_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf)             \
>>         SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 0),             \
>> @@ -44,6 +49,29 @@
>>                    SPI_MEM_OP_NO_DUMMY,                                 \
>>                    SPI_MEM_OP_NO_DATA)
>>
>> +/**
>> + * spansion_nor_clear_sr() - Clear the Status Register.
>> + * @nor:       pointer to 'struct spi_nor'.
>> + */
>> +static void spansion_nor_clear_sr(struct spi_nor *nor)
>> +{
>> +       int ret;
>> +
>> +       if (nor->spimem) {
>> +               struct spi_mem_op op = SPANSION_CLSR_OP;
>> +
>> +               spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
>> +
>> +               ret = spi_mem_exec_op(nor->spimem, &op);
>> +       } else {
>> +               ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
>> +                                                      NULL, 0);
>> +       }
>> +
>> +       if (ret)
>> +               dev_dbg(nor->dev, "error %d clearing SR\n", ret);
>> +}
>> +
>>  static int cypress_nor_octal_dtr_en(struct spi_nor *nor)
>>  {
>>         struct spi_mem_op op;
>> @@ -342,6 +370,65 @@ static const struct spi_nor_fixups s25fs_s_nor_fixups = {
>>         .post_bfpt = s25fs_s_nor_post_bfpt_fixups,
>>  };
>>
>> +/**
>> + * s25fl_l_sr_ready_and_clear() - S25FL_L family flashes need to query
>> + * Status Register 1 to check if the flash is ready and clear it if
>> + * there are Programming/Erase errors in Status Register 2.
>> + * @nor:       pointer to 'struct spi_nor'.
>> + *
>> + * Return: 1 if ready, 0 if not ready, -errno on errors.
>> + */
>> +static int s25fl_l_sr_ready_and_clear(struct spi_nor *nor)
>> +{
>> +       int ret;
>> +       u8 addr_mode_nbytes = nor->params->addr_mode_nbytes;
>> +       struct spi_mem_op op =
>> +               CYPRESS_NOR_RD_ANY_REG_OP(addr_mode_nbytes,
>> +                                         SPINOR_REG_CYPRESS_SR2V,
>> +                                         &nor->bouncebuf[1]);
>> +
RDAR in S25FL-L family requires 8 dummy cycles by default. This is one of
discrepancies in Spansion/Cypress/Infineon SPI NOR families.

In S25FL-L and S25FS-S families, number of dummy cycles in RDAR is same
as read ops (Fast read, Quad output read, etc) latency regardless of
register type, volatile or non-volatile. It is 8 by default.

In SEMPER family (S25HL/HS-T and S28HL/HS-T), number of dummy cycles in
RDAR for Non-volatile registers is same as read ops latency. For volatile
registers, it depends on CFR3[7:6] and is 0 by default.

So, we may want to let CYPRESS_NOR_RD_ANY_REG_OP macro take 'ndummy' param
to handle this...

>> +       /* Read Status Register 1 */
>> +       ret = spi_nor_read_sr(nor, nor->bouncebuf);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* RDSR2 command isn't available in QPI mode, use RDAR instead  */
Or use RDSR2 instead. QPI (4-4-4) mode is not supported in this driver.

>> +       ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
>> +       if (ret)
>> +               return ret;
>> +
>> +       if (nor->bouncebuf[1] & (S25FL_L_SR2V_P_ERR | S25FL_L_SR2V_E_ERR)) {
>> +               if (nor->bouncebuf[1] & S25FL_L_SR2V_E_ERR)
>> +                       dev_err(nor->dev, "Erase Error occurred\n");
>> +               else
>> +                       dev_err(nor->dev, "Programming Error occurred\n");
>> +
>> +               spansion_nor_clear_sr(nor);
>> +
>> +               /*
>> +                * WEL bit remains set to one when an erase or page program
>> +                * error occurs. Issue a Write Disable command to protect
>> +                * against inadvertent writes that can possibly corrupt the
>> +                * contents of the memory.
>> +                */
>> +               ret = spi_nor_write_disable(nor);
>> +               if (ret)
>> +                       return ret;
>> +
>> +               return -EIO;
>> +       }
>> +
>> +       return !(nor->bouncebuf[0] & SR_WIP);
>> +}
>> +
>> +static void s25fl_l_late_init(struct spi_nor *nor)
>> +{
>> +       nor->params->ready = s25fl_l_sr_ready_and_clear;
>> +}
>> +
>> +static const struct spi_nor_fixups s25fl_l_fixups = {
>> +       .late_init = s25fl_l_late_init,
>> +};
>>  static const struct flash_info spansion_nor_parts[] = {
>>         /* Spansion/Cypress -- single (large) sector size only, at least
>>          * for the chips listed here (without boot sectors).
>> @@ -428,13 +515,16 @@ static const struct flash_info spansion_nor_parts[] = {
>>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ) },
>>         { "s25fl064l",  INFO(0x016017,      0,  64 * 1024, 128)
>>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>> -               FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
>> +               FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>> +               .fixups = &s25fl_l_fixups },
>>         { "s25fl128l",  INFO(0x016018,      0,  64 * 1024, 256)
>>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>> -               FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
>> +               FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>> +               .fixups = &s25fl_l_fixups },
>>         { "s25fl256l",  INFO(0x016019,      0,  64 * 1024, 512)
>>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>> -               FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
>> +               FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>> +               .fixups = &s25fl_l_fixups },
How about merging SR2V check into existing spansion_nor_sr_ready_and_clear()?

Introduce new MFR flag like USE_SR2V then

MFR_FLAGS(USE_CLSR|USE_SR2V)

static int spansion_nor_sr_ready_and_clear(struct spi_nor *nor)
{
	int ret;

	ret = spi_nor_read_sr(nor, nor->bouncebuf);
	if (ret)
		return ret;

	if (nor->info->mfr_flags & USE_SR2V) {
		/* check SR2V */
	} else {
		/* check SR */
	}

	...
}

This is just my preference. What do you think?

>>         { "s25hl512t",  INFO6(0x342a1a, 0x0f0390, 256 * 1024, 256)
>>                 PARSE_SFDP
>>                 MFR_FLAGS(USE_CLSR)
>> @@ -460,29 +550,6 @@ static const struct flash_info spansion_nor_parts[] = {
>>         },
>>  };
>>
>> -/**
>> - * spansion_nor_clear_sr() - Clear the Status Register.
>> - * @nor:       pointer to 'struct spi_nor'.
>> - */
>> -static void spansion_nor_clear_sr(struct spi_nor *nor)
>> -{
>> -       int ret;
>> -
>> -       if (nor->spimem) {
>> -               struct spi_mem_op op = SPANSION_CLSR_OP;
>> -
>> -               spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
>> -
>> -               ret = spi_mem_exec_op(nor->spimem, &op);
>> -       } else {
>> -               ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
>> -                                                      NULL, 0);
>> -       }
>> -
>> -       if (ret)
>> -               dev_dbg(nor->dev, "error %d clearing SR\n", ret);
>> -}
>> -
>>  /**
>>   * spansion_nor_sr_ready_and_clear() - Query the Status Register to see if the
>>   * flash is ready for new commands and clear it if there are any errors.
>> --
>> 2.34.1
>>
> 

Thanks,
Takahiro