RE: [PATCH 3/3] mtd: spi-nor: sf: Add clear flag status register support

[Hou Zhiqiang]

Hi Jagan,

> -----Original Message-----
> From: Jagan Teki [mailto:jteki@xxxxxxxxxxxx]
> Sent: 2015年8月19日 17:57
> To: linux-mtd@xxxxxxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx; Jagan Teki; Hou Zhiqiang-B48286; Hu
> Mingkai-B21284; David Woodhouse; Brian Norris
> Subject: [PATCH 3/3] mtd: spi-nor: sf: Add clear flag status register
> support
> 
> The clear flag status register operation was required by Micron SPI-NOR
> chips, which support FSR. And if an error bit of FSR have been set like
> protection, voltage, erase, and program, it must be cleared by the clear
> FSR operation.
> 
> Signed-off-by: Jagan Teki <jteki@xxxxxxxxxxxx>
> Cc: Hou Zhiqiang <B48286@xxxxxxxxxxxxx>
> Cc: Mingkai.Hu <Mingkai.Hu@xxxxxxxxxxxxx>
> Cc: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
> Cc: Brian Norris <computersforpeace@xxxxxxxxx>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 35 +++++++++++++++++++++++++++++++----
>  include/linux/mtd/spi-nor.h   |  9 +++++++++
>  2 files changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-
> nor.c index f954d03..c5c472d5 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -100,6 +100,28 @@ static int read_fsr(struct spi_nor *nor)  }
> 
>  /*
> + * Read the clear flag status register.
> + * The clear flag status register operation was required by Micron
> + * SPI-NOR chips, which support FSR. And if an error bit of FSR
> + * have been set like protection, voltage, erase, and program,
> + * it must be cleared by the clear FSR operation.
> + * Returns zero for FSR bits cleared and negative if error occurred.
> + */
> +static int read_cfsr(struct spi_nor *nor) {
> +	int ret;
> +	u8 val;
> +
> +	ret = nor->read_reg(nor, SPINOR_OP_RDCFSR, &val, 1);

There should be a write_reg instead of read_reg.
There isn’t a register named CFSR, and the command SPINOR_OP_RDCFSR
is used to clear the FSR, another words reset FSR to default value.

> +	if (ret < 0) {
> +		pr_err("error %d reading CFSR\n", ret);
> +		return ret;
> +	}
> +
> +	return val;
> +}
> +
> +/*
>   * Read configuration register, returning its value in the
>   * location. Return the configuration register value.
>   * Returns negative if error occured.
> @@ -209,10 +231,15 @@ static inline int spi_nor_sr_ready(struct spi_nor
> *nor)  static inline int spi_nor_fsr_ready(struct spi_nor *nor)  {
>  	int fsr = read_fsr(nor);
> -	if (fsr < 0)
> -		return fsr;
> -	else
> -		return fsr & FSR_READY;
> +	if (fsr & FSR_ERR_MASK) {
> +		pr_err("flag status(0x%x) error occured\n", fsr);
> +		int cfsr = read_cfsr(nor);
> +		if (cfsr < 0)
> +			return cfsr;
> +		return -1;
> +	}
> +
> +	return fsr & FSR_READY;
>  }
> 
>  static int spi_nor_ready(struct spi_nor *nor) diff --git
> a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index
> c5a58c4..36c1681 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -35,6 +35,7 @@
>  #define SPINOR_OP_RDID		0x9f	/* Read JEDEC ID */
>  #define SPINOR_OP_RDCR		0x35	/* Read configuration register */
>  #define SPINOR_OP_RDFSR		0x70	/* Read flag status register */
> +#define SPINOR_OP_RDCFSR	0x50	/* Read clear flag status register */
> 
>  /* 4-byte address opcodes - used on Spansion and some Macronix flashes.
> */
>  #define SPINOR_OP_READ4		0x13	/* Read data bytes (low
> frequency) */
> @@ -74,6 +75,14 @@
>  /* Enhanced Volatile Configuration Register bits */
>  #define EVCR_QUAD_EN_MICRON    0x80    /* Micron Quad I/O */
> 
> +/* Flag Status Register Error bits */
> +#define FSR_ERR_PROT		0x2	/* Protection */
> +#define FSR_ERR_VOLT		0x8	/* Voltage on Vpp */
> +#define FSR_ERR_PROG		0x10	/* Program operation */
> +#define FSR_ERR_ERASE		0x20	/* Erase operation */
> +#define FSR_ERR_MASK		(FSR_ERR_PROT | FSR_ERR_VOLT | \
> +				FSR_ERR_PROG | FSR_ERR_ERASE)
> +
>  /* Flag Status Register bits */
>  #define FSR_READY		0x80
> 
> --
> 1.9.1

Thanks,
Zhiqiang