[PATCH] mtd: spi-nor: Use CLSR command for FL-L chips

Tudor.Ambarus at microchip.com Tudor.Ambarus at microchip.com
Sat Jan 23 12:45:00 EST 2021


Hi, Yaliang,

This is really useful, but we'll need it in a different form.
See below.

On 11/16/20 5:39 PM, yaliang.wang at windriver.com wrote:
> From: Yaliang Wang <Yaliang.Wang at windriver.com>
> 
> S25FL{064|128|256}L chips can't recover from errors, when there are
> program error or erase error, P_ERR or E_ERR bit will set to one, WIP
> bit will remain set to one, A Clear Status Register command must be sent
> to return the device to STANDBY state.
> 
> The error first recorded in commit <c4b3eacc1dfe> ("mtd: spi-nor: Recover
> from Spansion/Cypress errors"). Whlie FL-L chips shifted P_ERR or E_ERR
> bits to Status Register 2, which causing the current recover process
> doesn't work any more after enabling using CLSR.
> 
> Signed-off-by: Yaliang Wang <Yaliang.Wang at windriver.com>
> ---
>  drivers/mtd/spi-nor/core.c     | 96 +++++++++++++++++++++++++++++++++-
>  drivers/mtd/spi-nor/spansion.c |  6 +--
>  include/linux/mtd/spi-nor.h    |  1 +
>  3 files changed, 99 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index f0ae7a01703a..3aa1484deb85 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -537,6 +537,88 @@ static void spi_nor_clear_sr(struct spi_nor *nor)
>  		dev_dbg(nor->dev, "error %d clearing SR\n", ret);
>  }
>  
> +static const struct flash_info *spi_nor_read_id(struct spi_nor *nor);
> +
> +/**
> + * spi_nor_s25fl_l_read_sr2() - Read the Status Register 2 using the
> + * SPINOR_OP_FL_L_RDSR2 (07h) command.
> + * @nor:	pointer to 'struct spi_nor'.
> + * @sr2:	pointer to DMA-able buffer where the value of the
> + *		Status Register 2 will be written.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_s25fl_l_read_sr2(struct spi_nor *nor, u8 *sr2)
> +{
> +	int ret;
> +
> +	if (nor->spimem) {
> +		struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_FL_L_RDSR2, 1),

We really want manufacturer specific code out of the SPI NOR core.
Can you please check other manufacturer datasheets and verify if
others are using RDSR2 (07h) opcode?

> +				   SPI_MEM_OP_NO_ADDR,
> +				   SPI_MEM_OP_NO_DUMMY,
> +				   SPI_MEM_OP_DATA_IN(1, sr2, 1));
> +
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = nor->controller_ops->read_reg(nor, SPINOR_OP_FL_L_RDSR2,
> +						    sr2, 1);
> +	}
> +
> +	if (ret)
> +		dev_dbg(nor->dev, "error %d reading SR2\n", ret);
> +
> +	return ret;
> +}
> +
> +/*
> + * spi_nor_s25fl_l_sr_ready() - Query the Status Register to see if the flash
> + * is ready for new commands. Used by Cypress FL-L series chips.
> + * @nor:	pointer to 'struct spi_nor'.
> + *
> + * Return: 1 if ready, 0 if not ready, -errno on errors.
> + */
> +static int spi_nor_s25fl_l_sr_ready(struct spi_nor *nor)
> +{
> +	u8 *sr = nor->bouncebuf;
> +	int ret;
> +
> +	ret = spi_nor_read_sr(nor, sr);
> +	if (ret)
> +		return ret;
> +
> +	/**
> +	 * P_ERR and E_ERR bits are located in the Status Register 2
> +	 * of Cypress FL-L series chips.
> +	 */
> +	ret = spi_nor_s25fl_l_read_sr2(nor, &sr[1]);
> +	if (ret)
> +		return ret;
> +
> +	if (nor->flags & SNOR_F_USE_CLSR && sr[1] & (SR_E_ERR | SR_P_ERR)) {
If checking other manufacturer datasheets, would you please check if
CLSR is used by any other manufacturer?

> +		if (sr[1] & SR_E_ERR)
> +			dev_err(nor->dev, "Erase Error occurred\n");
> +		else
> +			dev_err(nor->dev, "Programming Error occurred\n");
> +
> +		spi_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 !(sr[0] & SR_WIP);
> +}
> +
>  /**
>   * spi_nor_sr_ready() - Query the Status Register to see if the flash is ready
>   * for new commands.
> @@ -546,7 +628,19 @@ static void spi_nor_clear_sr(struct spi_nor *nor)
>   */
>  static int spi_nor_sr_ready(struct spi_nor *nor)
>  {
> -	int ret = spi_nor_read_sr(nor, nor->bouncebuf);
> +	int ret;
> +	const struct flash_info *tmpinfo = nor->info ? nor->info : spi_nor_read_id(nor);
> +
> +	if (IS_ERR_OR_NULL(tmpinfo))
> +		return -ENOENT;
> +
> +	if (!strcmp(tmpinfo->name, "s25fl064l")
> +			|| !strcmp(tmpinfo->name, "s25fl128l")
> +			|| !strcmp(tmpinfo->name, "s25fl256l")) {
> +		return spi_nor_s25fl_l_sr_ready(nor);
> +	}

No, we can't accept flash name comparisons in the SPI NOR core.
If CLSR and RDSR2 are just spansion specific, we can provide a sr_ready
function pointer that can be filled by spansion flashes. Or some other
method depending on the CLSR and RDSR2 exposure through other
manufacturers.

Ideally one would skim through at least 2 - 3 datasheets from each
manufacturer available in SPI NOR. Preferable each datasheet from
a different manufacturer family. Unfortunately I'm not aware of any
standard that describes all the supported SPI NOR commands.
If you find this overwhelming, I can share the workload with you,
but at best effort. If you go through this by yourself, please
save the name of the datasheet flashes that you go through.

Cheers,
ta

> +
> +	ret = spi_nor_read_sr(nor, nor->bouncebuf);
>  
>  	if (ret)
>  		return ret;
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index 8429b4af999a..e0ccef793e12 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -95,13 +95,13 @@ static const struct flash_info spansion_parts[] = {
>  			     SECT_4K | SPI_NOR_DUAL_READ) },
>  	{ "s25fl064l",  INFO(0x016017,      0,  64 * 1024, 128,
>  			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> -			     SPI_NOR_4B_OPCODES) },
> +			     SPI_NOR_4B_OPCODES | USE_CLSR) },
>  	{ "s25fl128l",  INFO(0x016018,      0,  64 * 1024, 256,
>  			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> -			     SPI_NOR_4B_OPCODES) },
> +			     SPI_NOR_4B_OPCODES | USE_CLSR) },
>  	{ "s25fl256l",  INFO(0x016019,      0,  64 * 1024, 512,
>  			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> -			     SPI_NOR_4B_OPCODES) },
> +			     SPI_NOR_4B_OPCODES | USE_CLSR) },
>  	{ "cy15x104q",  INFO6(0x042cc2, 0x7f7f7f, 512 * 1024, 1,
>  			      SPI_NOR_NO_ERASE) },
>  };
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 60bac2c0ec45..a802f8f9e1e5 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -99,6 +99,7 @@
>  /* Used for Spansion flashes only. */
>  #define SPINOR_OP_BRWR		0x17	/* Bank register write */
>  #define SPINOR_OP_CLSR		0x30	/* Clear status register 1 */
> +#define SPINOR_OP_FL_L_RDSR2   0x07    /* FL-L chips Read status register 2 */
>  
>  /* Used for Micron flashes only. */
>  #define SPINOR_OP_RD_EVCR      0x65    /* Read EVCR register */
> 



More information about the linux-mtd mailing list