changeset 3392:cc4cb48d90ad

io/nand: Fix uninitialised variable. Erase any obsolete copies of the BBT we may come across. Cope with BBT version overflow.
author Ross Younger <wry@ecoscentric.com>
date Sat, 22 Nov 2014 17:26:39 +1300
parents 46870ac5c312
children abe4f70912a8
files packages/io/nand/current/ChangeLog packages/io/nand/current/src/nand_bbt.c
diffstat 2 files changed, 38 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/packages/io/nand/current/ChangeLog
+++ b/packages/io/nand/current/ChangeLog
@@ -1,3 +1,9 @@
+2014-11-22  Ross Younger  <wry@ecoscentric.com>
+
+	* src/nand_bbt.c: Fix uninitialised variable which sometimes
+	caused test failures. Erase any obsolete copies of the BBT we may
+	come across; cope with BBT version overflowing.
+
 2014-10-20  Ross Younger  <wry@ecoscentric.com>
 
 	* src/nand.c include/nand_internal.h: Create internal
--- a/packages/io/nand/current/src/nand_bbt.c
+++ b/packages/io/nand/current/src/nand_bbt.c
@@ -406,6 +406,7 @@ static int nandi_find_bbt_locations(cyg_
     int i, rv=0;
     cyg_nand_block_addr pri, mir, maybepri, maybemir;
     pri = mir = maybepri = maybemir = 0xFFFFFFFF;
+    *pri_ver = 0; *mir_ver = 0;
 
     for (i=0; i<4; i++) {
         cyg_nand_block_addr blk = start-i;
@@ -430,6 +431,24 @@ static int nandi_find_bbt_locations(cyg_
         if (0==memcmp(patternbuf, nand_pattern_primary, NAND_PATTERN_SIZE)) {
             CYG_BYTE found_ver = *versionbuf;
             if (found_ver > *pri_ver) {
+                if (*pri_ver != 0) {
+                    NAND_CHATTER(1,dev, "Found defunct copy of primary BBT in block %u, erasing it\n", pri);
+                    int rv = dev->fns->erase_block(dev, pri);
+                    if (rv != 0) {
+                        NAND_CHATTER(1,dev, "Could not erase block %u, marking as bad\n", pri);
+                        bbti_mark_raw(dev, pri, CYG_NAND_BBT_WORNBAD);
+                        /* Only mark_raw, not cyg_nand_bbti_markany which
+                         * would send us round in an infinite loop.
+                         * If we are in the midst of
+                         * cyg_nand_bbti_write_tables, this edits the data
+                         * in-RAM before we write it, which is what we want.
+                         * If we are searching to read the BBT on device
+                         * start, the effect of mark_raw is thrown away, but
+                         * that's OK - the defunct block will be found by the
+                         * next call to cyg_nand_bbti_write_tables and the
+                         * erase will be retried. */
+                    }
+                }
                 *pri_ver = found_ver;
                 pri = blk;
                 continue;
@@ -438,6 +457,15 @@ static int nandi_find_bbt_locations(cyg_
         if (0==memcmp(patternbuf, nand_pattern_mirror,  NAND_PATTERN_SIZE)) {
             CYG_BYTE found_ver = *versionbuf;
             if (found_ver > *mir_ver) {
+                if (*mir_ver != 0) {
+                    NAND_CHATTER(1,dev, "Found defunct copy of mirror BBT in block %u, erasing it\n", mir);
+                    int rv = dev->fns->erase_block(dev, mir);
+                    if (rv != 0) {
+                        NAND_CHATTER(1,dev, "Could not erase block %u, marking as bad\n", mir);
+                        bbti_mark_raw(dev, mir, CYG_NAND_BBT_WORNBAD);
+                        /* See comment by the call to bbti_mark_raw() just above */
+                    }
+                }
                 *mir_ver = found_ver;
                 mir = blk;
                 continue;
@@ -665,8 +693,10 @@ static int cyg_nand_bbti_write_tables(cy
 top:
     ++retries;
 
-    if (dev->bbt.version == 255)
-        NAND_ERROR(dev,"Warning! NAND BBT version would overflow, doing the best we can\n");
+    if (dev->bbt.version == 255) {
+        NAND_ERROR(dev,"Warning! NAND BBT version overflowing, doing the best we can\n");
+        dev->bbt.version = 128; // Don't wrap to 0 in case a human sees it and thinks the nand is pristine. If the BBT version wraps, it really isn't.
+    }
     else
         dev->bbt.version ++;