mtd: spi-nor: Add clear flag status register support
Subsystems (2)
PATCH v2 3/3 mtd: spi-nor: Add clear flag status register support
The clear flag status register operation is required by Micron SPI-NOR chips, which support FSR. And if error bits of FSR have been set like protection, voltage, erase, and program, it must be cleared by executing clear FSR operation.
---
Changes for v2:
- Write cfsr instead of reading it.
- Return -EINVAL instead of -1
drivers/mtd/spi-nor/spi-nor.c | 23 +++++++++++++++++++---- include/linux/mtd/spi-nor.h | 9 +++++++++ 2 files changed, 28 insertions(+), 4 deletions(-)
drivers/mtd/spi-nor/spi-nor.c +19 -4
@@ -163,6 +163,18 @@ static inline int write_disable(struct spi_nor *nor)return nor->write_reg (nor,SPINOR_OP_WRDI ,NULL ,0 ); } +/* +* The clear flag status register operation is required by Micron +* SPI-NOR chips, which support FSR. And if error bits of FSR +* have been set like protection, voltage, erase, and program, +* it must be cleared by executing clear FSR operation. +* Returns negative if error occurred. +*/ +static inline int write_cfsr (struct spi_nor *nor) +{ +return nor->write_reg (nor,SPINOR_OP_WRCFSR ,NULL ,0 ); +} +static inline struct spi_nor *mtd_to_spi_nor (struct mtd_info *mtd) {return mtd->priv;
@@ -209,10 +221,13 @@ 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); +write_cfsr (nor); +return -EINVAL ; + } + +return fsr &FSR_READY ; }static int spi_nor_ready (struct spi_nor *nor)
include/linux/mtd/spi-nor.h +9 -0
@@ -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_WRCFSR 0x50/* Write 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
Signature · 1 line
1.9.1
Quoted 10 lines · Jagan Teki
The clear flag status register operation is required by Micron SPI-NOR chips, which support FSR. And if error bits of FSR have been set like protection, voltage, erase, and program, it must be cleared by executing clear FSR operation.
Signed-off-by Jagan Teki <[email protected]>Cc Hou Zhiqiang <[email protected]>Cc Mingkai.Hu <[email protected]>Cc David Woodhouse <[email protected]>Cc Brian Norris <[email protected]>
Any response, please comment
Quoted 78 lines
---
Changes for v2:
- Write cfsr instead of reading it.
- Return-EINVALinstead of -1drivers/mtd/spi-nor/spi-nor.c | 23 +++++++++++++++++++---- include/linux/mtd/spi-nor.h | 9 +++++++++ 2 files changed, 28 insertions(+), 4 deletions(-)drivers/mtd/spi-nor/spi-nor.c +19
-4@@ -163,6 +163,18 @@ static inline int write_disable(struct spi_nor *nor)return nor->write_reg (nor,SPINOR_OP_WRDI ,NULL ,0 ); } +/* +* The clear flag status register operation is required by Micron +* SPI-NOR chips, which support FSR. And if error bits of FSR +* have been set like protection, voltage, erase, and program, +* it must be cleared by executing clear FSR operation. +* Returns negative if error occurred. +*/ +static inline int write_cfsr (struct spi_nor *nor) +{ +return nor->write_reg (nor,SPINOR_OP_WRCFSR ,NULL ,0 ); +} +static inline struct spi_nor *mtd_to_spi_nor (struct mtd_info *mtd) {return mtd->priv;@@ -209,10 +221,13 @@ 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); +write_cfsr (nor); +return -EINVAL ; + } + +return fsr &FSR_READY ; }static int spi_nor_ready (struct spi_nor *nor)include/linux/mtd/spi-nor.h +9
-1@@ -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_WRCFSR 0x50/* Write 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 --
Quoted 2 lines
1.9.1
thanks!
Signature · 1 line
Jagan.
Quoted 91 lines · Jagan Teki
Quoted 10 lines · Jagan Teki
The clear flag status register operation is required by Micron SPI-NOR chips, which support FSR. And if error bits of FSR have been set like protection, voltage, erase, and program, it must be cleared by executing clear FSR operation.
Signed-off-by Jagan Teki <[email protected]>Cc Hou Zhiqiang <[email protected]>Cc Mingkai.Hu <[email protected]>Cc David Woodhouse <[email protected]>Cc Brian Norris <[email protected]>Any response, please comment
Quoted 77 lines
---
Changes for v2:
- Write cfsr instead of reading it.
- Return-EINVALinstead of -1drivers/mtd/spi-nor/spi-nor.c | 23 +++++++++++++++++++---- include/linux/mtd/spi-nor.h | 9 +++++++++ 2 files changed, 28 insertions(+), 4 deletions(-)drivers/mtd/spi-nor/spi-nor.c +19
-4@@ -163,6 +163,18 @@ static inline int write_disable(struct spi_nor *nor)return nor->write_reg (nor,SPINOR_OP_WRDI ,NULL ,0 ); } +/* +* The clear flag status register operation is required by Micron +* SPI-NOR chips, which support FSR. And if error bits of FSR +* have been set like protection, voltage, erase, and program, +* it must be cleared by executing clear FSR operation. +* Returns negative if error occurred. +*/ +static inline int write_cfsr (struct spi_nor *nor) +{ +return nor->write_reg (nor,SPINOR_OP_WRCFSR ,NULL ,0 ); +} +static inline struct spi_nor *mtd_to_spi_nor (struct mtd_info *mtd) {return mtd->priv;@@ -209,10 +221,13 @@ 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); +write_cfsr (nor); +return -EINVAL ; + } + +return fsr &FSR_READY ; }static int spi_nor_ready (struct spi_nor *nor)include/linux/mtd/spi-nor.h +9
-0@@ -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_WRCFSR 0x50/* Write 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
Quoted 2 lines
Quoted 2 lines
--
1.9.1
Ping!
thanks!
Signature · 1 line
Jagan | openedev.
Quoted 43 lines · Jagan Teki
The clear flag status register operation is required by Micron SPI-NOR chips, which support FSR. And if error bits of FSR have been set like protection, voltage, erase, and program, it must be cleared by executing clear FSR operation.
Signed-off-by Jagan Teki <[email protected]>Cc Hou Zhiqiang <[email protected]>Cc Mingkai.Hu <[email protected]>Cc David Woodhouse <[email protected]>Cc Brian Norris <[email protected]>---
Changes for v2:
- Write cfsr instead of reading it.
- Return-EINVALinstead of -1drivers/mtd/spi-nor/spi-nor.c | 23 +++++++++++++++++++---- include/linux/mtd/spi-nor.h | 9 +++++++++ 2 files changed, 28 insertions(+), 4 deletions(-)drivers/mtd/spi-nor/spi-nor.c +12
-0@@ -163,6 +163,18 @@ static inline int write_disable(struct spi_nor *nor)return nor->write_reg (nor,SPINOR_OP_WRDI ,NULL ,0 ); } +/* +* The clear flag status register operation is required by Micron +* SPI-NOR chips, which support FSR. And if error bits of FSR +* have been set like protection, voltage, erase, and program, +* it must be cleared by executing clear FSR operation. +* Returns negative if error occurred. +*/ +static inline int write_cfsr (struct spi_nor *nor) +{ +return nor->write_reg (nor,SPINOR_OP_WRCFSR ,NULL ,0 ); +} +static inline struct spi_nor *mtd_to_spi_nor (struct mtd_info *mtd) {return mtd->priv;
drivers/mtd/spi-nor/spi-nor.c static inline int spi_nor_sr_ready(struct spi_nor *nor) · line 213
Quoted 5 lines
drivers/mtd/spi-nor/spi-nor.c +0
-2@@ -209,10 +221,13 @@ 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;
While it's rather unlikely that read_fsr() will return -EBFONT (which has neither of the FSR_ERR_MASK bits set), you should keep the check for negative return value intact.
drivers/mtd/spi-nor/spi-nor.c static inline int spi_nor_sr_ready(struct spi_nor *nor) · line 224
Quoted 3 lines
drivers/mtd/spi-nor/spi-nor.c +1
-2-else -return fsr &FSR_READY ; +if (fsr &FSR_ERR_MASK ) {
Also the N25Q032 data sheet says that these bits are only valid if FSR_READY is set. so you should bail out early if it isn't, and check the error bits only if it is set.
drivers/mtd/spi-nor/spi-nor.c static inline int spi_nor_sr_ready(struct spi_nor *nor) · line 225
Quoted 1 line
drivers/mtd/spi-nor/spi-nor.c +1
-0+pr_err ("flag status(0x%x) error occured\n" , fsr);
I suggest using a better error message than "flag status() error occured" and maybe even decode the error bits, so it's obvious what kind of error is there without looking at the #defines or a data sheet. Not sure about a better wording though, "last operation failed for the following reason:%s%s%s\n", fsr & FSR_ERR_PROT ? " PROT" :
"", fsr & ...
drivers/mtd/spi-nor/spi-nor.c static inline int spi_nor_sr_ready(struct spi_nor *nor) · line 226
Quoted 1 line
drivers/mtd/spi-nor/spi-nor.c +1
-0+write_cfsr (nor);
I'm not sure if a is_ready() function should clear the error bits, and rather the functions that can cause these to be set should be made aware of it and checking them (and then cleaning them), but maybe that would require a rewrite how some functions are done.
drivers/mtd/spi-nor/spi-nor.c static inline int spi_nor_sr_ready(struct spi_nor *nor) · line 227
Quoted 1 line
drivers/mtd/spi-nor/spi-nor.c +1
-0+return -EINVAL ;
Also I wonder if there are more appropriate error values here than -EINVAL, e.g. -EPERM if FSR_ERR_PROT is set, else -EIO if FSR_ERR_ERASE or FSR_ERR_PROG are set.
drivers/mtd/spi-nor/spi-nor.c static inline int spi_nor_sr_ready(struct spi_nor *nor) · line 231
Quoted 4 lines
drivers/mtd/spi-nor/spi-nor.c +3
-0+ } + +return fsr &FSR_READY ; }
Jonas