mtd: cfi_cmdset_0002: fix delayed error detection on HyperFlash
authorSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Thu, 31 Oct 2019 20:39:39 +0000 (23:39 +0300)
committerVignesh Raghavendra <vigneshr@ti.com>
Sat, 9 Nov 2019 09:13:53 +0000 (14:43 +0530)
The commit 4844ef80305d ("mtd: cfi_cmdset_0002: Add support for polling
status register") added checking for the status register error bits into
chip_good() to only return 1 if these bits are 0s.  Unfortunately, this
means that polling using chip_good() always reaches a timeout condition
when erase or program failure bits are set. Let's fully delegate the task
of determining the error conditions to cfi_check_err_status() and make
chip_good() only look for the Device Ready/Busy condition.

Fixes: 4844ef80305d ("mtd: cfi_cmdset_0002: Add support for polling status register")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
drivers/mtd/chips/cfi_cmdset_0002.c

index d5c2e54..04b383b 100644 (file)
@@ -123,14 +123,14 @@ static int cfi_use_status_reg(struct cfi_private *cfi)
                (extp->SoftwareFeatures & poll_mask) == CFI_POLL_STATUS_REG;
 }
 
-static void cfi_check_err_status(struct map_info *map, struct flchip *chip,
-                                unsigned long adr)
+static int cfi_check_err_status(struct map_info *map, struct flchip *chip,
+                               unsigned long adr)
 {
        struct cfi_private *cfi = map->fldrv_priv;
        map_word status;
 
        if (!cfi_use_status_reg(cfi))
-               return;
+               return 0;
 
        cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi,
                         cfi->device_type, NULL);
@@ -138,7 +138,7 @@ static void cfi_check_err_status(struct map_info *map, struct flchip *chip,
 
        /* The error bits are invalid while the chip's busy */
        if (!map_word_bitsset(map, status, CMD(CFI_SR_DRB)))
-               return;
+               return 0;
 
        if (map_word_bitsset(map, status, CMD(0x3a))) {
                unsigned long chipstatus = MERGESTATUS(status);
@@ -155,7 +155,12 @@ static void cfi_check_err_status(struct map_info *map, struct flchip *chip,
                if (chipstatus & CFI_SR_SLSB)
                        pr_err("%s sector write protected, status %lx\n",
                               map->name, chipstatus);
+
+               /* Erase/Program status bits are set on the operation failure */
+               if (chipstatus & (CFI_SR_ESB | CFI_SR_PSB))
+                       return 1;
        }
+       return 0;
 }
 
 /* #define DEBUG_CFI_FEATURES */
@@ -851,20 +856,16 @@ static int __xipram chip_good(struct map_info *map, struct flchip *chip,
 
        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.
+                * ready bit
                 */
                cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, 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;
+               return map_word_andequal(map, curd, ready, ready);
        }
 
        oldd = map_read(map, addr);
@@ -1702,8 +1703,11 @@ static int __xipram do_write_oneword_once(struct map_info *map,
                        break;
                }
 
-               if (chip_good(map, chip, adr, datum))
+               if (chip_good(map, chip, adr, datum)) {
+                       if (cfi_check_err_status(map, chip, adr))
+                               ret = -EIO;
                        break;
+               }
 
                /* Latency issues. Drop the lock, wait a while and retry */
                UDELAY(map, chip, adr, 1);
@@ -1776,7 +1780,6 @@ static int __xipram do_write_oneword_retry(struct map_info *map,
        ret = do_write_oneword_once(map, chip, adr, datum, mode, cfi);
        if (ret) {
                /* reset on all failures. */
-               cfi_check_err_status(map, chip, adr);
                map_write(map, CMD(0xF0), chip->start);
                /* FIXME - should have reset delay before continuing */
 
@@ -1973,12 +1976,17 @@ static int __xipram do_write_buffer_wait(struct map_info *map,
                 */
                if (time_after(jiffies, timeo) &&
                    !chip_good(map, chip, adr, datum)) {
+                       pr_err("MTD %s(): software timeout, address:0x%.8lx.\n",
+                              __func__, adr);
                        ret = -EIO;
                        break;
                }
 
-               if (chip_good(map, chip, adr, datum))
+               if (chip_good(map, chip, adr, datum)) {
+                       if (cfi_check_err_status(map, chip, adr))
+                               ret = -EIO;
                        break;
+               }
 
                /* Latency issues. Drop the lock, wait a while and retry */
                UDELAY(map, chip, adr, 1);
@@ -2074,12 +2082,8 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
                                chip->word_write_time);
 
        ret = do_write_buffer_wait(map, chip, adr, datum);
-       if (ret) {
-               cfi_check_err_status(map, chip, adr);
+       if (ret)
                do_write_buffer_reset(map, chip, cfi);
-               pr_err("MTD %s(): software timeout, address:0x%.8lx.\n",
-                      __func__, adr);
-       }
 
        xip_enable(map, chip, adr);
 
@@ -2274,9 +2278,9 @@ retry:
                udelay(1);
        }
 
-       if (!chip_good(map, chip, adr, datum)) {
+       if (!chip_good(map, chip, adr, datum) ||
+           cfi_check_err_status(map, chip, adr)) {
                /* reset on all failures. */
-               cfi_check_err_status(map, chip, adr);
                map_write(map, CMD(0xF0), chip->start);
                /* FIXME - should have reset delay before continuing */
 
@@ -2470,8 +2474,11 @@ static int __xipram do_erase_chip(struct map_info *map, struct flchip *chip)
                        chip->erase_suspended = 0;
                }
 
-               if (chip_good(map, chip, adr, map_word_ff(map)))
+               if (chip_good(map, chip, adr, map_word_ff(map))) {
+                       if (cfi_check_err_status(map, chip, adr))
+                               ret = -EIO;
                        break;
+               }
 
                if (time_after(jiffies, timeo)) {
                        printk(KERN_WARNING "MTD %s(): software timeout\n",
@@ -2486,7 +2493,6 @@ 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, chip, adr);
                map_write(map, CMD(0xF0), chip->start);
                /* FIXME - should have reset delay before continuing */
 
@@ -2567,8 +2573,11 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip,
                        chip->erase_suspended = 0;
                }
 
-               if (chip_good(map, chip, adr, map_word_ff(map)))
+               if (chip_good(map, chip, adr, map_word_ff(map))) {
+                       if (cfi_check_err_status(map, chip, adr))
+                               ret = -EIO;
                        break;
+               }
 
                if (time_after(jiffies, timeo)) {
                        printk(KERN_WARNING "MTD %s(): software timeout\n",
@@ -2583,7 +2592,6 @@ 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, chip, adr);
                map_write(map, CMD(0xF0), chip->start);
                /* FIXME - should have reset delay before continuing */