diff mbox series

[v7,1/5] mtd: cfi_cmdset_0002: Add support for polling status register

Message ID 20190620172250.9102-2-vigneshr@ti.com (mailing list archive)
State New, archived
Headers show
Series MTD: Add Initial HyperBus support | expand

Commit Message

Vignesh Raghavendra June 20, 2019, 5:22 p.m. UTC
HyperFlash devices are compliant with CFI AMD/Fujitsu Extended Command
Set (0x0002) for flash operations, therefore
drivers/mtd/chips/cfi_cmdset_0002.c can be used as is. But these devices
do not support DQ polling method of determining chip ready/good status.
These flashes provide Status Register whose bits can be polled to know
status of flash operation.

Cypress HyperFlash datasheet here[1], talks about CFI Amd/Fujitsu
Extended Query version 1.5. Bit 0 of "Software Features supported" field
of CFI Primary Vendor-Specific Extended Query table indicates
presence/absence of status register and Bit 1 indicates whether or not
DQ polling is supported. Using these bits, its possible to determine
whether flash supports DQ polling or need to use Status Register.

Add support for polling Status Register to know device ready/status of
erase/write operations when DQ polling is not supported.
Print error messages on erase/program failure by looking at related
Status Register bits.

[1] https://www.cypress.com/file/213346/download

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
---
v7: No change

 drivers/mtd/chips/cfi_cmdset_0002.c | 90 +++++++++++++++++++++++++++++
 include/linux/mtd/cfi.h             |  5 ++
 2 files changed, 95 insertions(+)

Comments

Tokunori Ikegami June 24, 2019, 4:46 p.m. UTC | #1
On 2019/06/21 2:22, Vignesh Raghavendra wrote:
> HyperFlash devices are compliant with CFI AMD/Fujitsu Extended Command
> Set (0x0002) for flash operations, therefore
> drivers/mtd/chips/cfi_cmdset_0002.c can be used as is. But these devices
> do not support DQ polling method of determining chip ready/good status.
> These flashes provide Status Register whose bits can be polled to know
> status of flash operation.
>
> Cypress HyperFlash datasheet here[1], talks about CFI Amd/Fujitsu
> Extended Query version 1.5. Bit 0 of "Software Features supported" field
> of CFI Primary Vendor-Specific Extended Query table indicates
> presence/absence of status register and Bit 1 indicates whether or not
> DQ polling is supported. Using these bits, its possible to determine
> whether flash supports DQ polling or need to use Status Register.
>
> Add support for polling Status Register to know device ready/status of
> erase/write operations when DQ polling is not supported.
> Print error messages on erase/program failure by looking at related
> Status Register bits.
>
> [1] https://www.cypress.com/file/213346/download
>
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> ---
> v7: No change
>
>   drivers/mtd/chips/cfi_cmdset_0002.c | 90 +++++++++++++++++++++++++++++
>   include/linux/mtd/cfi.h             |  5 ++
>   2 files changed, 95 insertions(+)
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index c8fa5906bdf9..0f571f162e3b 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -49,6 +49,16 @@
>   #define SST49LF008A		0x005a
>   #define AT49BV6416		0x00d6
>   
> +/*
> + * Status Register bit description. Used by flash devices that don't
> + * support DQ polling (e.g. HyperFlash)
> + */
> +#define CFI_SR_DRB		BIT(7)
> +#define CFI_SR_ESB		BIT(5)
> +#define CFI_SR_PSB		BIT(4)
> +#define CFI_SR_WBASB		BIT(3)
> +#define CFI_SR_SLSB		BIT(1)
> +
>   static int cfi_amdstd_read (struct mtd_info *, loff_t, size_t, size_t *, u_char *);
>   static int cfi_amdstd_write_words(struct mtd_info *, loff_t, size_t, size_t *, const u_char *);
>   static int cfi_amdstd_write_buffers(struct mtd_info *, loff_t, size_t, size_t *, const u_char *);
> @@ -97,6 +107,48 @@ static struct mtd_chip_driver cfi_amdstd_chipdrv = {
>   	.module		= THIS_MODULE
>   };
>   
> +/*
> + * Use status register to poll for Erase/write completion when DQ is not
> + * supported. This is indicated by Bit[1:0] of SoftwareFeatures field in
> + * CFI Primary Vendor-Specific Extended Query table 1.5
> + */
> +static int cfi_use_status_reg(struct cfi_private *cfi)
> +{
> +	struct cfi_pri_amdstd *extp = cfi->cmdset_priv;
> +
> +	return extp->MinorVersion >= '5' &&
> +		(extp->SoftwareFeatures & 0x3) == 0x1;

Seems to be better to use defined values instead of 0x3 and 0x1 hard 
coded values.

> +}
> +
> +static void cfi_check_err_status(struct map_info *map, unsigned long adr)
> +{
> +	struct cfi_private *cfi = map->fldrv_priv;
> +	map_word status;
> +
> +	if (!cfi_use_status_reg(cfi))
> +		return;
> +
> +	cfi_send_gen_cmd(0x70, cfi->addr_unlock1, 0, map, cfi,

Is it not necessary to set chip->start as the base parameter for 
cfi_send_gen_cmd()?

> +			 cfi->device_type, NULL);
> +	status = map_read(map, adr);
> +
> +	if (map_word_bitsset(map, status, CMD(0x3a))) {
> +		unsigned long chipstatus = MERGESTATUS(status);
> +
> +		if (chipstatus & CFI_SR_ESB)
> +			pr_err("%s erase operation failed, status %lx\n",
> +			       map->name, chipstatus);
> +		if (chipstatus & CFI_SR_PSB)
> +			pr_err("%s program operation failed, status %lx\n",
> +			       map->name, chipstatus);
> +		if (chipstatus & CFI_SR_WBASB)
> +			pr_err("%s buffer program command aborted, status %lx\n",
> +			       map->name, chipstatus);
> +		if (chipstatus & CFI_SR_SLSB)
> +			pr_err("%s sector write protected, status %lx\n",
> +			       map->name, chipstatus);
> +	}
> +}
>   
>   /* #define DEBUG_CFI_FEATURES */
>   
> @@ -744,8 +796,22 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
>    */
>   static int __xipram chip_ready(struct map_info *map, unsigned long addr)
>   {
> +	struct cfi_private *cfi = map->fldrv_priv;
>   	map_word d, t;
>   
> +	if (cfi_use_status_reg(cfi)) {
> +		map_word ready = CMD(CFI_SR_DRB);
> +		/*
> +		 * For chips that support status register, check device
> +		 * ready bit
> +		 */
> +		cfi_send_gen_cmd(0x70, cfi->addr_unlock1, 0, map, cfi,

Same comment as cfi_check_err_status() about the base address.

> +				 cfi->device_type, NULL);
> +		d = map_read(map, addr);
> +
> +		return map_word_andequal(map, d, ready, ready);
> +	}
> +
>   	d = map_read(map, addr);
>   	t = map_read(map, addr);
>   
> @@ -769,8 +835,27 @@ static int __xipram chip_ready(struct map_info *map, unsigned long addr)
>    */
>   static int __xipram chip_good(struct map_info *map, unsigned long addr, map_word expected)
>   {
> +	struct cfi_private *cfi = map->fldrv_priv;
>   	map_word oldd, curd;
>   
> +	if (cfi_use_status_reg(cfi)) {
> +		map_word ready = CMD(CFI_SR_DRB);
> +		map_word err = CMD(CFI_SR_PSB | CFI_SR_ESB);

Is it not necessary to check CFI_SR_WBASB and CFI_SR_SLSB that are 
checked by cfi_check_err_status()?

> +		/*
> +		 * For chips that support status register, check device
> +		 * ready bit and Erase/Program status bit to know if
> +		 * operation succeeded.
> +		 */
> +		cfi_send_gen_cmd(0x70, cfi->addr_unlock1, 0, map, cfi,

Same as cfi_check_err_status() and chip_ready() about the base address.

> +				 cfi->device_type, NULL);
> +		curd = map_read(map, addr);
> +
> +		if (map_word_andequal(map, curd, ready, ready))
> +			return !map_word_bitsset(map, curd, err);
> +
> +		return 0;
> +	}
> +
>   	oldd = map_read(map, addr);
>   	curd = map_read(map, addr);
>   
> @@ -1644,6 +1729,7 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
>   	/* Did we succeed? */
>   	if (!chip_good(map, adr, datum)) {
>   		/* reset on all failures. */
> +		cfi_check_err_status(map, adr);
>   		map_write(map, CMD(0xF0), chip->start);
>   		/* FIXME - should have reset delay before continuing */
>   
> @@ -1901,6 +1987,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
>   	 * See e.g.
>   	 * http://www.spansion.com/Support/Application%20Notes/MirrorBit_Write_Buffer_Prog_Page_Buffer_Read_AN.pdf
>   	 */
> +	cfi_check_err_status(map, adr);
>   	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
>   			 cfi->device_type, NULL);
>   	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
> @@ -2107,6 +2194,7 @@ static int do_panic_write_oneword(struct map_info *map, struct flchip *chip,
>   
>   	if (!chip_good(map, adr, datum)) {
>   		/* reset on all failures. */
> +		cfi_check_err_status(map, adr);
>   		map_write(map, CMD(0xF0), chip->start);
>   		/* FIXME - should have reset delay before continuing */
>   
> @@ -2316,6 +2404,7 @@ static int __xipram do_erase_chip(struct map_info *map, struct flchip *chip)
>   	/* Did we succeed? */
>   	if (ret) {
>   		/* reset on all failures. */
> +		cfi_check_err_status(map, adr);
>   		map_write(map, CMD(0xF0), chip->start);
>   		/* FIXME - should have reset delay before continuing */
>   
> @@ -2412,6 +2501,7 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip,
>   	/* Did we succeed? */
>   	if (ret) {
>   		/* reset on all failures. */
> +		cfi_check_err_status(map, adr);
>   		map_write(map, CMD(0xF0), chip->start);
>   		/* FIXME - should have reset delay before continuing */
>   
> diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h
> index 208c87cf2e3e..b50416169049 100644
> --- a/include/linux/mtd/cfi.h
> +++ b/include/linux/mtd/cfi.h
> @@ -219,6 +219,11 @@ struct cfi_pri_amdstd {
>   	uint8_t  VppMin;
>   	uint8_t  VppMax;
>   	uint8_t  TopBottom;
> +	/* Below field are added from version 1.5 */
> +	uint8_t  ProgramSuspend;
> +	uint8_t  UnlockBypass;
> +	uint8_t  SecureSiliconSector;
> +	uint8_t  SoftwareFeatures;
>   } __packed;
>   
>   /* Vendor-Specific PRI for Atmel chips (command set 0x0002) */
Vignesh Raghavendra June 25, 2019, 5:52 a.m. UTC | #2
Hi,

On 24/06/19 10:16 PM, Tokunori Ikegami wrote:
> 
[...]
>>   +/*
>> + * Use status register to poll for Erase/write completion when DQ is not
>> + * supported. This is indicated by Bit[1:0] of SoftwareFeatures field in
>> + * CFI Primary Vendor-Specific Extended Query table 1.5
>> + */
>> +static int cfi_use_status_reg(struct cfi_private *cfi)
>> +{
>> +    struct cfi_pri_amdstd *extp = cfi->cmdset_priv;
>> +
>> +    return extp->MinorVersion >= '5' &&
>> +        (extp->SoftwareFeatures & 0x3) == 0x1;
> 
> Seems to be better to use defined values instead of 0x3 and 0x1 hard
> coded values.
> 

Ok

>> +}
>> +
>> +static void cfi_check_err_status(struct map_info *map, unsigned long
>> adr)
>> +{
>> +    struct cfi_private *cfi = map->fldrv_priv;
>> +    map_word status;
>> +
>> +    if (!cfi_use_status_reg(cfi))
>> +        return;
>> +
>> +    cfi_send_gen_cmd(0x70, cfi->addr_unlock1, 0, map, cfi,
> 
> Is it not necessary to set chip->start as the base parameter for
> cfi_send_gen_cmd()?
> 

Right now, I am not aware of any flash that supports status registers
and are banked (that's when  chip->start can be non zero). Therefore I
did not think of using chip->start.
But anyways, I will fix this up to use chip->start here and elsewhere
for next version, assuming there will be such chips in the future.

>> +             cfi->device_type, NULL);
>> +    status = map_read(map, adr);
>> +
>> +    if (map_word_bitsset(map, status, CMD(0x3a))) {
>> +        unsigned long chipstatus = MERGESTATUS(status);
>> +
>> +        if (chipstatus & CFI_SR_ESB)
>> +            pr_err("%s erase operation failed, status %lx\n",
>> +                   map->name, chipstatus);
>> +        if (chipstatus & CFI_SR_PSB)
>> +            pr_err("%s program operation failed, status %lx\n",
>> +                   map->name, chipstatus);
>> +        if (chipstatus & CFI_SR_WBASB)
>> +            pr_err("%s buffer program command aborted, status %lx\n",
>> +                   map->name, chipstatus);
>> +        if (chipstatus & CFI_SR_SLSB)
>> +            pr_err("%s sector write protected, status %lx\n",
>> +                   map->name, chipstatus);
>> +    }
>> +}
>>     /* #define DEBUG_CFI_FEATURES */
>>   @@ -744,8 +796,22 @@ static struct mtd_info *cfi_amdstd_setup(struct
>> mtd_info *mtd)
>>    */
>>   static int __xipram chip_ready(struct map_info *map, unsigned long
>> addr)
>>   {
>> +    struct cfi_private *cfi = map->fldrv_priv;
>>       map_word d, t;
>>   +    if (cfi_use_status_reg(cfi)) {
>> +        map_word ready = CMD(CFI_SR_DRB);
>> +        /*
>> +         * For chips that support status register, check device
>> +         * ready bit
>> +         */
>> +        cfi_send_gen_cmd(0x70, cfi->addr_unlock1, 0, map, cfi,
> 
> Same comment as cfi_check_err_status() about the base address.
> 
>> +                 cfi->device_type, NULL);
>> +        d = map_read(map, addr);
>> +
>> +        return map_word_andequal(map, d, ready, ready);
>> +    }
>> +
>>       d = map_read(map, addr);
>>       t = map_read(map, addr);
>>   @@ -769,8 +835,27 @@ static int __xipram chip_ready(struct map_info
>> *map, unsigned long addr)
>>    */
>>   static int __xipram chip_good(struct map_info *map, unsigned long
>> addr, map_word expected)
>>   {
>> +    struct cfi_private *cfi = map->fldrv_priv;
>>       map_word oldd, curd;
>>   +    if (cfi_use_status_reg(cfi)) {
>> +        map_word ready = CMD(CFI_SR_DRB);
>> +        map_word err = CMD(CFI_SR_PSB | CFI_SR_ESB);
> 
> Is it not necessary to check CFI_SR_WBASB and CFI_SR_SLSB that are
> checked by cfi_check_err_status()?
> 

chip_good() is used to verify whether write or erase operation really
succeeded. Looking at Cypress HyperFlash datasheets and app notes on
status register polling, its enough to see if CFI_SR_PSB or CFI_SR_PSB
is set to know if write or erase failed. Now, the reason for program or
erase failure can be known by looking at CFI_SR_WBASB and CFI_SR_SLSB
which is done for cfi_check_err_status().
Therefore, I feel, its enough to look for CFI_SR_PSB or CFI_SR_ESB here.

Thanks for the review!

Regards
Vignesh

>> +        /*
>> +         * For chips that support status register, check device
>> +         * ready bit and Erase/Program status bit to know if
>> +         * operation succeeded.
>> +         */
>> +        cfi_send_gen_cmd(0x70, cfi->addr_unlock1, 0, map, cfi,
> 
> Same as cfi_check_err_status() and chip_ready() about the base address.
> 
>> +                 cfi->device_type, NULL);
>> +        curd = map_read(map, addr);
>> +
>> +        if (map_word_andequal(map, curd, ready, ready))
>> +            return !map_word_bitsset(map, curd, err);
>> +
>> +        return 0;
>> +    }
>> +
>>       oldd = map_read(map, addr);
>>       curd = map_read(map, addr);
>>   @@ -1644,6 +1729,7 @@ static int __xipram do_write_oneword(struct
>> map_info *map, struct flchip *chip,
>>       /* Did we succeed? */
>>       if (!chip_good(map, adr, datum)) {
>>           /* reset on all failures. */
>> +        cfi_check_err_status(map, adr);
>>           map_write(map, CMD(0xF0), chip->start);
>>           /* FIXME - should have reset delay before continuing */
>>   @@ -1901,6 +1987,7 @@ static int __xipram do_write_buffer(struct
>> map_info *map, struct flchip *chip,
>>        * See e.g.
>>        *
>> http://www.spansion.com/Support/Application%20Notes/MirrorBit_Write_Buffer_Prog_Page_Buffer_Read_AN.pdf
>>
>>        */
>> +    cfi_check_err_status(map, adr);
>>       cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
>>                cfi->device_type, NULL);
>>       cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
>> @@ -2107,6 +2194,7 @@ static int do_panic_write_oneword(struct
>> map_info *map, struct flchip *chip,
>>         if (!chip_good(map, adr, datum)) {
>>           /* reset on all failures. */
>> +        cfi_check_err_status(map, adr);
>>           map_write(map, CMD(0xF0), chip->start);
>>           /* FIXME - should have reset delay before continuing */
>>   @@ -2316,6 +2404,7 @@ static int __xipram do_erase_chip(struct
>> map_info *map, struct flchip *chip)
>>       /* Did we succeed? */
>>       if (ret) {
>>           /* reset on all failures. */
>> +        cfi_check_err_status(map, adr);
>>           map_write(map, CMD(0xF0), chip->start);
>>           /* FIXME - should have reset delay before continuing */
>>   @@ -2412,6 +2501,7 @@ static int __xipram do_erase_oneblock(struct
>> map_info *map, struct flchip *chip,
>>       /* Did we succeed? */
>>       if (ret) {
>>           /* reset on all failures. */
>> +        cfi_check_err_status(map, adr);
>>           map_write(map, CMD(0xF0), chip->start);
>>           /* FIXME - should have reset delay before continuing */
>>   diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h
>> index 208c87cf2e3e..b50416169049 100644
>> --- a/include/linux/mtd/cfi.h
>> +++ b/include/linux/mtd/cfi.h
>> @@ -219,6 +219,11 @@ struct cfi_pri_amdstd {
>>       uint8_t  VppMin;
>>       uint8_t  VppMax;
>>       uint8_t  TopBottom;
>> +    /* Below field are added from version 1.5 */
>> +    uint8_t  ProgramSuspend;
>> +    uint8_t  UnlockBypass;
>> +    uint8_t  SecureSiliconSector;
>> +    uint8_t  SoftwareFeatures;
>>   } __packed;
>>     /* Vendor-Specific PRI for Atmel chips (command set 0x0002) */
diff mbox series

Patch

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index c8fa5906bdf9..0f571f162e3b 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -49,6 +49,16 @@ 
 #define SST49LF008A		0x005a
 #define AT49BV6416		0x00d6
 
+/*
+ * Status Register bit description. Used by flash devices that don't
+ * support DQ polling (e.g. HyperFlash)
+ */
+#define CFI_SR_DRB		BIT(7)
+#define CFI_SR_ESB		BIT(5)
+#define CFI_SR_PSB		BIT(4)
+#define CFI_SR_WBASB		BIT(3)
+#define CFI_SR_SLSB		BIT(1)
+
 static int cfi_amdstd_read (struct mtd_info *, loff_t, size_t, size_t *, u_char *);
 static int cfi_amdstd_write_words(struct mtd_info *, loff_t, size_t, size_t *, const u_char *);
 static int cfi_amdstd_write_buffers(struct mtd_info *, loff_t, size_t, size_t *, const u_char *);
@@ -97,6 +107,48 @@  static struct mtd_chip_driver cfi_amdstd_chipdrv = {
 	.module		= THIS_MODULE
 };
 
+/*
+ * Use status register to poll for Erase/write completion when DQ is not
+ * supported. This is indicated by Bit[1:0] of SoftwareFeatures field in
+ * CFI Primary Vendor-Specific Extended Query table 1.5
+ */
+static int cfi_use_status_reg(struct cfi_private *cfi)
+{
+	struct cfi_pri_amdstd *extp = cfi->cmdset_priv;
+
+	return extp->MinorVersion >= '5' &&
+		(extp->SoftwareFeatures & 0x3) == 0x1;
+}
+
+static void cfi_check_err_status(struct map_info *map, unsigned long adr)
+{
+	struct cfi_private *cfi = map->fldrv_priv;
+	map_word status;
+
+	if (!cfi_use_status_reg(cfi))
+		return;
+
+	cfi_send_gen_cmd(0x70, cfi->addr_unlock1, 0, map, cfi,
+			 cfi->device_type, NULL);
+	status = map_read(map, adr);
+
+	if (map_word_bitsset(map, status, CMD(0x3a))) {
+		unsigned long chipstatus = MERGESTATUS(status);
+
+		if (chipstatus & CFI_SR_ESB)
+			pr_err("%s erase operation failed, status %lx\n",
+			       map->name, chipstatus);
+		if (chipstatus & CFI_SR_PSB)
+			pr_err("%s program operation failed, status %lx\n",
+			       map->name, chipstatus);
+		if (chipstatus & CFI_SR_WBASB)
+			pr_err("%s buffer program command aborted, status %lx\n",
+			       map->name, chipstatus);
+		if (chipstatus & CFI_SR_SLSB)
+			pr_err("%s sector write protected, status %lx\n",
+			       map->name, chipstatus);
+	}
+}
 
 /* #define DEBUG_CFI_FEATURES */
 
@@ -744,8 +796,22 @@  static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
  */
 static int __xipram chip_ready(struct map_info *map, unsigned long addr)
 {
+	struct cfi_private *cfi = map->fldrv_priv;
 	map_word d, t;
 
+	if (cfi_use_status_reg(cfi)) {
+		map_word ready = CMD(CFI_SR_DRB);
+		/*
+		 * For chips that support status register, check device
+		 * ready bit
+		 */
+		cfi_send_gen_cmd(0x70, cfi->addr_unlock1, 0, map, cfi,
+				 cfi->device_type, NULL);
+		d = map_read(map, addr);
+
+		return map_word_andequal(map, d, ready, ready);
+	}
+
 	d = map_read(map, addr);
 	t = map_read(map, addr);
 
@@ -769,8 +835,27 @@  static int __xipram chip_ready(struct map_info *map, unsigned long addr)
  */
 static int __xipram chip_good(struct map_info *map, unsigned long addr, map_word expected)
 {
+	struct cfi_private *cfi = map->fldrv_priv;
 	map_word oldd, curd;
 
+	if (cfi_use_status_reg(cfi)) {
+		map_word ready = CMD(CFI_SR_DRB);
+		map_word err = CMD(CFI_SR_PSB | CFI_SR_ESB);
+		/*
+		 * For chips that support status register, check device
+		 * ready bit and Erase/Program status bit to know if
+		 * operation succeeded.
+		 */
+		cfi_send_gen_cmd(0x70, cfi->addr_unlock1, 0, map, cfi,
+				 cfi->device_type, NULL);
+		curd = map_read(map, addr);
+
+		if (map_word_andequal(map, curd, ready, ready))
+			return !map_word_bitsset(map, curd, err);
+
+		return 0;
+	}
+
 	oldd = map_read(map, addr);
 	curd = map_read(map, addr);
 
@@ -1644,6 +1729,7 @@  static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
 	/* Did we succeed? */
 	if (!chip_good(map, adr, datum)) {
 		/* reset on all failures. */
+		cfi_check_err_status(map, adr);
 		map_write(map, CMD(0xF0), chip->start);
 		/* FIXME - should have reset delay before continuing */
 
@@ -1901,6 +1987,7 @@  static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 	 * See e.g.
 	 * http://www.spansion.com/Support/Application%20Notes/MirrorBit_Write_Buffer_Prog_Page_Buffer_Read_AN.pdf
 	 */
+	cfi_check_err_status(map, adr);
 	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
 			 cfi->device_type, NULL);
 	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
@@ -2107,6 +2194,7 @@  static int do_panic_write_oneword(struct map_info *map, struct flchip *chip,
 
 	if (!chip_good(map, adr, datum)) {
 		/* reset on all failures. */
+		cfi_check_err_status(map, adr);
 		map_write(map, CMD(0xF0), chip->start);
 		/* FIXME - should have reset delay before continuing */
 
@@ -2316,6 +2404,7 @@  static int __xipram do_erase_chip(struct map_info *map, struct flchip *chip)
 	/* Did we succeed? */
 	if (ret) {
 		/* reset on all failures. */
+		cfi_check_err_status(map, adr);
 		map_write(map, CMD(0xF0), chip->start);
 		/* FIXME - should have reset delay before continuing */
 
@@ -2412,6 +2501,7 @@  static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip,
 	/* Did we succeed? */
 	if (ret) {
 		/* reset on all failures. */
+		cfi_check_err_status(map, adr);
 		map_write(map, CMD(0xF0), chip->start);
 		/* FIXME - should have reset delay before continuing */
 
diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h
index 208c87cf2e3e..b50416169049 100644
--- a/include/linux/mtd/cfi.h
+++ b/include/linux/mtd/cfi.h
@@ -219,6 +219,11 @@  struct cfi_pri_amdstd {
 	uint8_t  VppMin;
 	uint8_t  VppMax;
 	uint8_t  TopBottom;
+	/* Below field are added from version 1.5 */
+	uint8_t  ProgramSuspend;
+	uint8_t  UnlockBypass;
+	uint8_t  SecureSiliconSector;
+	uint8_t  SoftwareFeatures;
 } __packed;
 
 /* Vendor-Specific PRI for Atmel chips (command set 0x0002) */