Fix unlocking function for most Atmel AT2[56]D* chips

I broke unlocking them correctly in r1635 while refactoring (NB: the
commit log including the overly selfconfident statement about the
"bug in spi_disable_blockprotect_at25df()").

Affected chips have per sector protection bits and the write protection bits
in the status register do indicate if none, some or all sectors are protected.
It is possible to globally (un)lock all sectors at once but in a way that was
not anticipated when refactoring the spi25 unlocking functions into
spi_disable_blockprotect_generic(). To globally unprotect not only the
protection bits (2 and 3) have 0 to be written to them but also bits 4 and 5
which normally would not be touched by spi_disable_blockprotect_generic().
Some of the chips also support a permanent lockdown with fuses which we
do not handle yet.

To fix this without copying the whole method I introduce another mask
parameter to spi_disable_blockprotect_generic() namely unprotect_mask.
See verbose comments inline for details.

Also, prettyprint the status register after trying to disable the block
protection fails.

Corresponding to flashrom svn r1679.

Signed-off-by: Stefan Tauner <stefan.tauner@alumni.tuwien.ac.at>
Tested-by: Chi Zhang <zhangchi866@gmail.com>
Acked-by: Stefan Tauner <stefan.tauner@alumni.tuwien.ac.at>

4 files changed, 61 insertions, 38 deletions

diff --git a/chipdrivers.h b/chipdrivers.h
index dd20631..fd522a6 100644
--- a/chipdrivers.h
+++ b/chipdrivers.h
@@ -77,8 +77,8 @@ int spi_prettyprint_status_register_at25f4096(struct flashctx *flash);
 int spi_prettyprint_status_register_at25fs010(struct flashctx *flash);
 int spi_prettyprint_status_register_at25fs040(struct flashctx *flash);
 int spi_prettyprint_status_register_at26df081a(struct flashctx *flash);
-int spi_disable_blockprotect_at25df(struct flashctx *flash);
-int spi_disable_blockprotect_at25df_sec(struct flashctx *flash);
+int spi_disable_blockprotect_at2x_global_unprotect(struct flashctx *flash);
+int spi_disable_blockprotect_at2x_global_unprotect_sec(struct flashctx *flash);
 int spi_disable_blockprotect_at25f(struct flashctx *flash);
 int spi_disable_blockprotect_at25f512a(struct flashctx *flash);
 int spi_disable_blockprotect_at25f512b(struct flashctx *flash);
diff --git a/flashchips.c b/flashchips.c
index 6ec3749..d188617 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -1353,7 +1353,7 @@ const struct flashchip flashchips[] = {
 			}
 		},
 		.printlock	= spi_prettyprint_status_register_at25df,
-		.unlock		= spi_disable_blockprotect_at25df,
+		.unlock		= spi_disable_blockprotect_at2x_global_unprotect,
 		.write		= spi_chip_write_256,
 		.read		= spi_chip_read,
 		.voltage	= {2700, 3600}, /* 2.3-3.6V & 2.7-3.6V models available */
@@ -1391,7 +1391,7 @@ const struct flashchip flashchips[] = {
 			}
 		},
 		.printlock	= spi_prettyprint_status_register_at25df,
-		.unlock		= spi_disable_blockprotect_at25df,
+		.unlock		= spi_disable_blockprotect_at2x_global_unprotect,
 		.write		= spi_chip_write_256,
 		.read		= spi_chip_read,
 		.voltage	= {2700, 3600}, /* 2.3-3.6V & 2.7-3.6V models available */
@@ -1429,7 +1429,7 @@ const struct flashchip flashchips[] = {
 			}
 		},
 		.printlock	= spi_prettyprint_status_register_at25df,
-		.unlock		= spi_disable_blockprotect_at25df,
+		.unlock		= spi_disable_blockprotect_at2x_global_unprotect,
 		.write		= spi_chip_write_256,
 		.read		= spi_chip_read,
 		.voltage	= {1600, 2000}, /* Datasheet says range is 1.65-1.95 V */
@@ -1467,7 +1467,7 @@ const struct flashchip flashchips[] = {
 			}
 		},
 		.printlock	= spi_prettyprint_status_register_at25df_sec,
-		.unlock		= spi_disable_blockprotect_at25df_sec,
+		.unlock		= spi_disable_blockprotect_at2x_global_unprotect_sec,
 		.write		= spi_chip_write_256,
 		.read		= spi_chip_read,
 		.voltage	= {2700, 3600},
@@ -1505,7 +1505,7 @@ const struct flashchip flashchips[] = {
 			}
 		},
 		.printlock	= spi_prettyprint_status_register_at25df_sec,
-		.unlock		= spi_disable_blockprotect_at25df_sec,
+		.unlock		= spi_disable_blockprotect_at2x_global_unprotect_sec,
 		.write		= spi_chip_write_256,
 		.read		= spi_chip_read,
 		.voltage	= {2700, 3600},
@@ -1543,7 +1543,7 @@ const struct flashchip flashchips[] = {
 			}
 		},
 		.printlock	= spi_prettyprint_status_register_at25df,
-		.unlock		= spi_disable_blockprotect_at25df,
+		.unlock		= spi_disable_blockprotect_at2x_global_unprotect,
 		.write		= spi_chip_write_256,
 		.read		= spi_chip_read,
 		.voltage	= {2700, 3600},
@@ -1582,7 +1582,7 @@ const struct flashchip flashchips[] = {
 			}
 		},
 		.printlock	= spi_prettyprint_status_register_at25df_sec,
-		.unlock		= spi_disable_blockprotect_at25df_sec,
+		.unlock		= spi_disable_blockprotect_at2x_global_unprotect_sec,
 		.write		= spi_chip_write_256,
 		.read		= spi_chip_read,
 		.voltage	= {2700, 3600},
@@ -1620,7 +1620,7 @@ const struct flashchip flashchips[] = {
 			}
 		},
 		.printlock	= spi_prettyprint_status_register_at25df_sec,
-		.unlock		= spi_disable_blockprotect_at25df_sec,
+		.unlock		= spi_disable_blockprotect_at2x_global_unprotect_sec,
 		.write		= spi_chip_write_256,
 		.read		= spi_chip_read,
 		.voltage	= {2700, 3600},
@@ -1659,7 +1659,7 @@ const struct flashchip flashchips[] = {
 			}
 		},
 		.printlock	= spi_prettyprint_status_register_at25df_sec,
-		.unlock		= spi_disable_blockprotect_at25df_sec,
+		.unlock		= spi_disable_blockprotect_at2x_global_unprotect_sec,
 		.write		= spi_chip_write_256,
 		.read		= spi_chip_read,
 		.voltage	= {2700, 3600},
@@ -1959,7 +1959,7 @@ const struct flashchip flashchips[] = {
 				.block_erase = spi_block_erase_20,
 			}
 		},
-		.printlock	= spi_prettyprint_status_register_plain, /* TODO: improve */
+		.printlock	= spi_prettyprint_status_register_plain,
 		/* Supports also an incompatible page write (of exactly 256 B) and an auto-erasing write. */
 		.write		= spi_chip_write_1,
 		.read		= spi_chip_read, /* Fast read (0x0B) supported */
@@ -1998,7 +1998,7 @@ const struct flashchip flashchips[] = {
 			}
 		},
 		.printlock	= spi_prettyprint_status_register_at26df081a,
-		.unlock		= spi_disable_blockprotect_at25df,
+		.unlock		= spi_disable_blockprotect_at2x_global_unprotect,
 		.write		= spi_chip_write_256,
 		.read		= spi_chip_read,
 		.voltage	= {2700, 3600},
@@ -2036,7 +2036,7 @@ const struct flashchip flashchips[] = {
 			}
 		},
 		.printlock	= spi_prettyprint_status_register_at25df,
-		.unlock		= spi_disable_blockprotect,
+		.unlock		= spi_disable_blockprotect_at2x_global_unprotect,
 		.write		= spi_chip_write_256,
 		.read		= spi_chip_read,
 		.voltage	= {2700, 3600},
@@ -2051,7 +2051,7 @@ const struct flashchip flashchips[] = {
 		.total_size	= 2048,
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
-		.tested		= TEST_UNTESTED,
+		.tested		= TEST_OK_PREW,
 		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
@@ -2074,7 +2074,7 @@ const struct flashchip flashchips[] = {
 			}
 		},
 		.printlock	= spi_prettyprint_status_register_at26df081a,
-		.unlock		= spi_disable_blockprotect,
+		.unlock		= spi_disable_blockprotect_at2x_global_unprotect,
 		.write		= spi_chip_write_256,
 		.read		= spi_chip_read,
 		.voltage	= {2700, 3600},
diff --git a/flashchips.h b/flashchips.h
index 41903d2..d23c50d 100644
--- a/flashchips.h
+++ b/flashchips.h
@@ -130,13 +130,15 @@
 #define ATMEL_ID		0x1F	/* Atmel */
 #define ATMEL_AT25DF021		0x4300
 #define ATMEL_AT25DF041A	0x4401
-#define ATMEL_AT25DF081		0x4502
+#define ATMEL_AT25DF081		0x4502	/* EDI 0x00. AT25DL081 has same ID + EDI 0x0100 */
 #define ATMEL_AT25DF081A	0x4501	/* Yes, 81A has a lower number than 81 */
 #define ATMEL_AT25DF161		0x4602
 #define ATMEL_AT25DF321		0x4700	/* Same as 26DF321 */
 #define ATMEL_AT25DF321A	0x4701
 #define ATMEL_AT25DF641		0x4800
-#define ATMEL_AT25DQ161		0x8600
+#define ATMEL_AT25DL161		0x4603	/* EDI 0x0100 */
+#define ATMEL_AT25DQ161		0x8600	/* EDI 0x0100 */
+#define ATMEL_AT25DQ321		0x8700	/* EDI 0x0100 */
 #define ATMEL_AT25F512		0x65	/* guessed, no device ID in datasheet. Needs AT25F_RDID */
 #define ATMEL_AT25F512A		0x65	/* Needs AT25F_RDID */
 #define ATMEL_AT25F512B		0x6500
diff --git a/spi25_statusreg.c b/spi25_statusreg.c
index 53952fe..a6a4607 100644
--- a/spi25_statusreg.c
+++ b/spi25_statusreg.c
@@ -126,11 +126,24 @@ uint8_t spi_read_status_register(struct flashctx *flash)
 /* A generic block protection disable.
  * Tests if a protection is enabled with the block protection mask (bp_mask) and returns success otherwise.
  * Tests if the register bits are locked with the lock_mask (lock_mask).
- * Tests if a hardware protection is active (i.e. low) with the write protection mask (wp_mask) and bails out
- * in that case.
- * Finally tries to disable engaged protections and checks if any locks are still set.
+ * Tests if a hardware protection is active (i.e. low pin/high bit value) with the write protection mask
+ * (wp_mask) and bails out in that case.
+ * If there are register lock bits set we try to disable them by unsetting those bits of the previous register
+ * contents that are set in the lock_mask. We then check if removing the lock bits has worked and continue as if
+ * they never had been engaged:
+ * If the lock bits are out of the way try to disable engaged protections.
+ * To support uncommon global unprotects (e.g. on most AT2[56]xx1(A)) unprotect_mask can be used to force
+ * bits to 0 additionally to those set in bp_mask and lock_mask. Only bits set in unprotect_mask are potentially
+ * preserved when doing the final unprotect.
+ *
+ * To sum up:
+ * bp_mask: set those bits that correspond to the bits in the status register that indicate an active protection
+ *          (which should be unset after this function returns).
+ * lock_mask: set the bits that correspond to the bits that lock changing the bits above.
+ * wp_mask: set the bits that correspond to bits indicating non-software revocable protections.
+ * unprotect_mask: set the bits that should be preserved if possible when unprotecting.
  */
-static int spi_disable_blockprotect_generic(struct flashctx *flash, uint8_t bp_mask, uint8_t lock_mask, uint8_t wp_mask)
+static int spi_disable_blockprotect_generic(struct flashctx *flash, uint8_t bp_mask, uint8_t lock_mask, uint8_t wp_mask, uint8_t unprotect_mask)
 {
 	uint8_t status;
 	int result;
@@ -154,10 +167,15 @@ static int spi_disable_blockprotect_generic(struct flashctx *flash, uint8_t bp_m
 			msg_cerr("spi_write_status_register failed.\n");
 			return result;
 		}
+		status = spi_read_status_register(flash);
+		if ((status & lock_mask) != 0) {
+			msg_cerr("Unsetting lock bit(s) failed.\n");
+			return 1;
+		}
 		msg_cdbg("done.\n");
 	}
 	/* Global unprotect. Make sure to mask the register lock bit as well. */
-	result = spi_write_status_register(flash, status & ~(bp_mask | lock_mask));
+	result = spi_write_status_register(flash, status & ~(bp_mask | lock_mask) & unprotect_mask);
 	if (result) {
 		msg_cerr("spi_write_status_register failed.\n");
 		return result;
@@ -165,6 +183,7 @@ static int spi_disable_blockprotect_generic(struct flashctx *flash, uint8_t bp_m
 	status = spi_read_status_register(flash);
 	if ((status & bp_mask) != 0) {
 		msg_cerr("Block protection could not be disabled!\n");
+		flash->chip->printlock(flash);
 		return 1;
 	}
 	msg_cdbg("disabled.\n");
@@ -174,7 +193,7 @@ static int spi_disable_blockprotect_generic(struct flashctx *flash, uint8_t bp_m
 /* A common block protection disable that tries to unset the status register bits masked by 0x3C. */
 int spi_disable_blockprotect(struct flashctx *flash)
 {
-	return spi_disable_blockprotect_generic(flash, 0x3C, 0, 0);
+	return spi_disable_blockprotect_generic(flash, 0x3C, 0, 0, 0xFF);
 }
 
 
@@ -488,49 +507,51 @@ int spi_prettyprint_status_register_at26df081a(struct flashctx *flash)
 	return 0;
 }
 
-int spi_disable_blockprotect_at25df(struct flashctx *flash)
+/* Some Atmel DataFlash chips support per sector protection bits and the write protection bits in the status
+ * register do indicate if none, some or all sectors are protected. It is possible to globally (un)lock all
+ * sectors at once by writing 0 not only the protection bits (2 and 3) but also completely unrelated bits (4 and
+ * 5) which normally are not touched.
+ * Affected are all known Atmel chips matched by AT2[56]D[FLQ]..1A? but the AT26DF041. */
+int spi_disable_blockprotect_at2x_global_unprotect(struct flashctx *flash)
 {
-	return spi_disable_blockprotect_generic(flash, 0x0C, 1 << 7, 1 << 4);
+	return spi_disable_blockprotect_generic(flash, 0x0C, 1 << 7, 1 << 4, 0x00);
 }
 
-int spi_disable_blockprotect_at25df_sec(struct flashctx *flash)
+int spi_disable_blockprotect_at2x_global_unprotect_sec(struct flashctx *flash)
 {
 	/* FIXME: We should check the security lockdown. */
 	msg_cinfo("Ignoring security lockdown (if present)\n");
-	return spi_disable_blockprotect_at25df(flash);
+	return spi_disable_blockprotect_at2x_global_unprotect(flash);
 }
 
 int spi_disable_blockprotect_at25f(struct flashctx *flash)
 {
-	return spi_disable_blockprotect_generic(flash, 0x0C, 1 << 7, 0);
+	return spi_disable_blockprotect_generic(flash, 0x0C, 1 << 7, 0, 0xFF);
 }
 
 int spi_disable_blockprotect_at25f512a(struct flashctx *flash)
 {
-	return spi_disable_blockprotect_generic(flash, 0x04, 1 << 7, 0);
+	return spi_disable_blockprotect_generic(flash, 0x04, 1 << 7, 0, 0xFF);
 }
 
 int spi_disable_blockprotect_at25f512b(struct flashctx *flash)
 {
-	/* spi_disable_blockprotect_at25df is not really the right way to do
-	 * this, but the side effects of said function work here as well.
-	 */
-	return spi_disable_blockprotect_at25df(flash);
+	return spi_disable_blockprotect_generic(flash, 0x04, 1 << 7, 1 << 4, 0xFF);
 }
 
 int spi_disable_blockprotect_at25f4096(struct flashctx *flash)
 {
-	return spi_disable_blockprotect_generic(flash, 0x1C, 1 << 7, 0);
+	return spi_disable_blockprotect_generic(flash, 0x1C, 1 << 7, 0, 0xFF);
 }
 
 int spi_disable_blockprotect_at25fs010(struct flashctx *flash)
 {
-	return spi_disable_blockprotect_generic(flash, 0x6C, 1 << 7, 0);
+	return spi_disable_blockprotect_generic(flash, 0x6C, 1 << 7, 0, 0xFF);
  }
 
 int spi_disable_blockprotect_at25fs040(struct flashctx *flash)
 {
-	return spi_disable_blockprotect_generic(flash, 0x7C, 1 << 7, 0);
+	return spi_disable_blockprotect_generic(flash, 0x7C, 1 << 7, 0, 0xFF);
 }
 
 /* === Intel === */
@@ -538,7 +559,7 @@ int spi_disable_blockprotect_at25fs040(struct flashctx *flash)
 /* TODO: Clear P_FAIL and E_FAIL with Clear SR Fail Flags Command (30h) here? */
 int spi_disable_blockprotect_s33(struct flashctx *flash)
 {
-	return spi_disable_blockprotect_generic(flash, 0x1C, 1 << 7, 0);
+	return spi_disable_blockprotect_generic(flash, 0x1C, 1 << 7, 0, 0xFF);
 }
 
 int spi_prettyprint_status_register_s33(struct flashctx *flash)