[PATCH] mtd: spi-nor: spansion: CLPEF as an alternative to CLSR

[Tudor Ambarus]

On 6/9/23 09:14, tkuw584924 at gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> 
> Infineon S28Hx (SEMPER Octal) and S25FS256T (SEMPER Nano) support Clear
> Program and Erase Failure Flags (CLPEF, 82h) instead of CLSR(30h).
> Introduce a new mfr_flag and rework spansion_nor_clear_sr() to add a way
> to clear status in these devices. S25Hx (SEMPER QSPI) supports CLSR but
> it may be disabled by CFR3x[2] while CLPEF is always available.
> Therefore, the mfr_flag is also applied to S25Hx for safety.
> 
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> ---
>  drivers/mtd/spi-nor/spansion.c | 37 ++++++++++++++++++++++++++--------
>  1 file changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index 15f9a80c10b9..f2f4bc060f5e 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -10,8 +10,10 @@
>  
>  /* flash_info mfr_flag. Used to clear sticky prorietary SR bits. */
>  #define USE_CLSR	BIT(0)
> +#define USE_CLPEF	BIT(1)
>  
>  #define SPINOR_OP_CLSR		0x30	/* Clear status register 1 */
> +#define SPINOR_OP_CLPEF		0x82	/* Clear program/erase failure flags */
>  #define SPINOR_OP_RD_ANY_REG			0x65	/* Read any register */
>  #define SPINOR_OP_WR_ANY_REG			0x71	/* Write any register */
>  #define SPINOR_REG_CYPRESS_VREG			0x00800000
> @@ -63,23 +65,35 @@
>  		   SPI_MEM_OP_NO_DUMMY,					\
>  		   SPI_MEM_OP_NO_DATA)
>  
> +#define SPANSION_CLPEF_OP						\
> +	SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLPEF, 0),			\
> +		   SPI_MEM_OP_NO_ADDR,					\
> +		   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)
>  {
> +	u8 use_clpef = nor->info->mfr_flags & USE_CLPEF;
>  	int ret;
>  
>  	if (nor->spimem) {
> -		struct spi_mem_op op = SPANSION_CLSR_OP;
> +		struct spi_mem_op op;
> +
> +		if (use_clpef)
> +			op = (struct spi_mem_op)SPANSION_CLPEF_OP;
> +		else
> +			op = (struct spi_mem_op)SPANSION_CLSR_OP;

we can get rid of the if else if we introduce a MFR OP struct and use
the opcode directly.

>  
>  		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);
> +		ret = spi_nor_controller_ops_write_reg(nor,
> +			use_clpef ? SPINOR_OP_CLPEF : SPINOR_OP_CLSR, NULL, 0);

no new support for the controllers under spi-nor/, let the else case as
it was.

>  	}
>  
>  	if (ret)
> @@ -792,47 +806,54 @@ static const struct flash_info spansion_nor_parts[] = {
>  		FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
>  	{ "s25fs256t",  INFO6(0x342b19, 0x0f0890, 0, 0)
>  		PARSE_SFDP
> +		MFR_FLAGS(USE_CLPEF)
>  		.fixups = &s25fs256t_fixups },
>  	{ "s25hl512t",  INFO6(0x342a1a, 0x0f0390, 256 * 1024, 256)
>  		PARSE_SFDP
> -		MFR_FLAGS(USE_CLSR)
> +		MFR_FLAGS(USE_CLPEF)
>  		.fixups = &s25hx_t_fixups },
>  	{ "s25hl01gt",  INFO6(0x342a1b, 0x0f0390, 256 * 1024, 512)
>  		PARSE_SFDP
> -		MFR_FLAGS(USE_CLSR)
> +		MFR_FLAGS(USE_CLPEF)
>  		.fixups = &s25hx_t_fixups },
>  	{ "s25hl02gt",  INFO6(0x342a1c, 0x0f0090, 0, 0)
>  		PARSE_SFDP
> +		MFR_FLAGS(USE_CLPEF)
>  		FLAGS(NO_CHIP_ERASE)
>  		.fixups = &s25hx_t_fixups },
>  	{ "s25hs512t",  INFO6(0x342b1a, 0x0f0390, 256 * 1024, 256)
>  		PARSE_SFDP
> -		MFR_FLAGS(USE_CLSR)
> +		MFR_FLAGS(USE_CLPEF)
>  		.fixups = &s25hx_t_fixups },
>  	{ "s25hs01gt",  INFO6(0x342b1b, 0x0f0390, 256 * 1024, 512)
>  		PARSE_SFDP
> -		MFR_FLAGS(USE_CLSR)
> +		MFR_FLAGS(USE_CLPEF)
>  		.fixups = &s25hx_t_fixups },
>  	{ "s25hs02gt",  INFO6(0x342b1c, 0x0f0090, 0, 0)
>  		PARSE_SFDP
> +		MFR_FLAGS(USE_CLPEF)
>  		FLAGS(NO_CHIP_ERASE)
>  		.fixups = &s25hx_t_fixups },
>  	{ "cy15x104q",  INFO6(0x042cc2, 0x7f7f7f, 512 * 1024, 1)
>  		FLAGS(SPI_NOR_NO_ERASE) },
>  	{ "s28hl512t",   INFO(0x345a1a,      0, 256 * 1024, 256)
>  		PARSE_SFDP
> +		MFR_FLAGS(USE_CLPEF)
>  		.fixups = &s28hx_t_fixups,
>  	},
>  	{ "s28hl01gt",   INFO(0x345a1b,      0, 256 * 1024, 512)
>  		PARSE_SFDP
> +		MFR_FLAGS(USE_CLPEF)
>  		.fixups = &s28hx_t_fixups,
>  	},
>  	{ "s28hs512t",   INFO(0x345b1a,      0, 256 * 1024, 256)
>  		PARSE_SFDP
> +		MFR_FLAGS(USE_CLPEF)
>  		.fixups = &s28hx_t_fixups,
>  	},
>  	{ "s28hs01gt",   INFO(0x345b1b,      0, 256 * 1024, 512)
>  		PARSE_SFDP
> +		MFR_FLAGS(USE_CLPEF)
>  		.fixups = &s28hx_t_fixups,
>  	},
>  };
> @@ -885,7 +906,7 @@ static void spansion_nor_late_init(struct spi_nor *nor)
>  		nor->mtd.erasesize = nor->info->sector_size;
>  	}
>  
> -	if (nor->info->mfr_flags & USE_CLSR)
> +	if (nor->info->mfr_flags & (USE_CLSR|USE_CLPEF))

have you run ./scripts/checkpatch --strict and it did not complain on this?

>  		nor->params->ready = spansion_nor_sr_ready_and_clear;
>  }
>