Patchwork autotest mode

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2012-11-09 14:39:09
Message ID <509D158D.4030901@gmx.net>
Download mbox | patch
Permalink /patch/3786/
State Bitrotted
Headers show

Comments

Carl-Daniel Hailfinger - 2012-11-09 14:39:09
New version, better messages.

This patch needs the following changes to be mergeable:
- layout support
- man page
- message printing cosmetics
- Make sure flashrom returns an error code if autotest failed, even if
  recovering the chip contents succeeded.

Autotest mode tests all usable erase functions of a flash chip with a
few test patterns, then restores the original flash contents. To make
sure you can recover the original flash chip contents even if the
autotest run is aborted, you have to specify a filename parameter for
--autotest where the flash chip contents are saved.
Example usage:
flashrom -p dummy:emulate=SST25VF032B --output autotest.log --autotest
scratch.img
Please note that autotest uses roughly 10 erase cycles per usable erase
function and repeated usage will cause serious flash wear. Speed isn't
really good even with dummy, and I expect a 8 Mbit SPI chip on a real
fast programmer to take at least 10 minutes for autotest (10*erasefn+1
all-sectors chip write, 10*erasefn+1.all-sectors erase, 30*erasefn+2
all-sectors read).

The patch seems to work, at least during my tests with dummy.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Stefan Tauner - 2012-11-11 22:27:01
On Fri, 09 Nov 2012 15:39:09 +0100
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:

> New version, better messages.
> 
> This patch needs the following changes to be mergeable:
> - layout support
> - man page
> - message printing cosmetics
> - Make sure flashrom returns an error code if autotest failed, even if
>   recovering the chip contents succeeded.
> 
> Autotest mode tests all usable erase functions of a flash chip with a
> few test patterns, then restores the original flash contents. To make
> sure you can recover the original flash chip contents even if the
> autotest run is aborted, you have to specify a filename parameter for
> --autotest where the flash chip contents are saved.
> Example usage:
> flashrom -p dummy:emulate=SST25VF032B --output autotest.log --autotest
> scratch.img
> Please note that autotest uses roughly 10 erase cycles per usable erase
> function and repeated usage will cause serious flash wear. Speed isn't
> really good even with dummy, and I expect a 8 Mbit SPI chip on a real
> fast programmer to take at least 10 minutes for autotest (10*erasefn+1
> all-sectors chip write, 10*erasefn+1.all-sectors erase, 30*erasefn+2
> all-sectors read).
> 
> The patch seems to work, at least during my tests with dummy.
> 
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

i have tested this on my serprog implementation attached to a W25Q32
(32Mb, 4MB) on top of r1622. the complete autotest took over 7,5 hours.
i have attached the produced log after removing some serprog spam
(namely all "serprog_delay used, but programmer doesn't support delays
natively - emulating" strings, the original log has 58MB due to this. hm)

the non-verbose output looks like this, and i'll comment on this inline:
> flashrom -p serprog:dev=/dev/ttyU2flasher:2000000 --output autotest.log --autotest /tmp/scratch.img
> flashrom v0.9.6.1-r1622 on Linux 3.2.0-32-generic (x86_64)
> flashrom is free software, get the source code at http://www.flashrom.org
> 
> Calibrating delay loop... OK.
> serprog: Programmer name is "atmegaXXu2      "
> Found Winbond flash chip "W25Q32" (4096 kB, SPI) on serprog.
> Reading flash... done.
> Starting autotest.
> Reading current flash chip contents... done.

the first read is done twice. this is unnecessary and should be fixed
if possible.

> Erasing and writing flash chip with pattern 0... 
> Reading current flash chip contents... done.

"..." should be followed by done.
maybe we could get rid of the unneeded read outputs and print the done
only if the pattern was written and verified.

> Erasing and writing flash chip with pattern 4... 
> Reading current flash chip contents... done.
> Erasing and writing flash chip with pattern 2... 
> Reading current flash chip contents... done.
> Erasing and writing flash chip with pattern 5... 
> Reading current flash chip contents... done.
> Erasing and writing flash chip with pattern 1... 
> Reading current flash chip contents... done.
> Erasing and writing flash chip with pattern 3... 
> Reading current flash chip contents... done.
> Erasing and writing flash chip with pattern 8... 
> Reading current flash chip contents... done.
> Erasing and writing flash chip with pattern 9... 
> Reading current flash chip contents... done.
> Erasing and writing flash chip with pattern 13... 
> Reading current flash chip contents... done.
> Erasing and writing flash chip with pattern 12... 
> Reading current flash chip contents... done.

here we want a notification telling us that the first erase function
has been tested successfully. also something like "testing erase
function x of y" at the beginning of every loop would be nice. for
bonus points one could measure and estimate the time... :)

> Erasing and writing flash chip with pattern 0... 
> […]
> Erasing and writing flash chip with pattern 12... 
> Restoring original chip contents:Reading current flash chip contents... done.

missing space (at least; a "done" for the restore action would be a
good idea too). and a x of y tests failed, if any...

> Erasing and writing flash chip... Erase/write done.
> Verifying flash... VERIFIED.

i havent looked at the patterns used (the order is a bit wtf from a
user's perspective :) or the code... but maybe we could add different
levels of thoroughness. i think one that does just write a different
random image with each eraser would be enough for most cases, and slow
programmers ;)

Patch

Index: flashrom-autotest/flash.h
===================================================================
--- flashrom-autotest/flash.h	(Revision 1622)
+++ flashrom-autotest/flash.h	(Arbeitskopie)
@@ -239,7 +239,7 @@ 
 void print_banner(void);
 void list_programmers_linebreak(int startcol, int cols, int paren);
 int selfcheck(void);
-int doit(struct flashctx *flash, int force, const char *filename, int read_it, int write_it, int erase_it, int verify_it);
+int doit(struct flashctx *flash, int force, const char *filename, int read_it, int write_it, int erase_it, int verify_it, int autotest_it);
 int read_buf_from_file(unsigned char *buf, unsigned long size, const char *filename);
 int write_buf_to_file(unsigned char *buf, unsigned long size, const char *filename);
 
Index: flashrom-autotest/cli_classic.c
===================================================================
--- flashrom-autotest/cli_classic.c	(Revision 1622)
+++ flashrom-autotest/cli_classic.c	(Arbeitskopie)
@@ -118,17 +118,18 @@ 
 #if CONFIG_PRINT_WIKI == 1
 	int list_supported_wiki = 0;
 #endif
-	int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0;
+	int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0, autotest_it = 0;
 	int dont_verify_it = 0, list_supported = 0, operation_specified = 0;
 	enum programmer prog = PROGRAMMER_INVALID;
 	int ret = 0;
 
-	static const char optstring[] = "r:Rw:v:nVEfc:l:i:p:Lzho:";
+	static const char optstring[] = "r:Rw:v:a:nVEfc:l:i:p:Lzho:";
 	static const struct option long_options[] = {
 		{"read",		1, NULL, 'r'},
 		{"write",		1, NULL, 'w'},
 		{"erase",		0, NULL, 'E'},
 		{"verify",		1, NULL, 'v'},
+		{"autotest",		1, NULL, 'a'},
 		{"noverify",		0, NULL, 'n'},
 		{"chip",		1, NULL, 'c'},
 		{"verbose",		0, NULL, 'V'},
@@ -196,6 +197,15 @@ 
 			filename = strdup(optarg);
 			verify_it = 1;
 			break;
+		case 'a':
+			if (++operation_specified > 1) {
+				fprintf(stderr, "More than one operation "
+					"specified. Aborting.\n");
+				cli_classic_abort_usage();
+			}
+			filename = strdup(optarg);
+			autotest_it = 1;
+			break;
 		case 'n':
 			if (verify_it) {
 				fprintf(stderr, "--verify and --noverify are"
@@ -506,7 +516,7 @@ 
 		goto out_shutdown;
 	}
 
-	if (!(read_it | write_it | verify_it | erase_it)) {
+	if (!(read_it || write_it || verify_it || erase_it || autotest_it)) {
 		msg_ginfo("No operations were specified.\n");
 		goto out_shutdown;
 	}
@@ -520,7 +530,7 @@ 
 	 * Give the chip time to settle.
 	 */
 	programmer_delay(100000);
-	ret |= doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it);
+	ret |= doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it, autotest_it);
 	/* Note: doit() already calls programmer_shutdown(). */
 	goto out;
 
Index: flashrom-autotest/dediprog.c
===================================================================
--- flashrom-autotest/dediprog.c	(Revision 1622)
+++ flashrom-autotest/dediprog.c	(Arbeitskopie)
@@ -34,17 +34,6 @@ 
 #define DEDI_SPI_CMD_PAGEWRITE	0x1
 #define DEDI_SPI_CMD_AAIWRITE	0x4
 
-#if 0
-/* Might be useful for other pieces of code as well. */
-static void print_hex(void *buf, size_t len)
-{
-	size_t i;
-
-	for (i = 0; i < len; i++)
-		msg_pdbg(" %02x", ((uint8_t *)buf)[i]);
-}
-#endif
-
 /* Might be useful for other USB devices as well. static for now. */
 static struct usb_device *get_device_by_vid_pid(uint16_t vid, uint16_t pid)
 {
Index: flashrom-autotest/flashrom.c
===================================================================
--- flashrom-autotest/flashrom.c	(Revision 1622)
+++ flashrom-autotest/flashrom.c	(Arbeitskopie)
@@ -448,6 +448,14 @@ 
 	return i;
 }
 
+void print_hex(void *buf, size_t len)
+{
+	size_t i;
+
+	for (i = 0; i < len; i++)
+		msg_pdbg(" %02x", ((uint8_t *)buf)[i]);
+}
+
 void tolower_string(char *str)
 {
 	for (; *str != '\0'; str++)
@@ -873,6 +881,9 @@ 
 	case 13:
 		memset(buf, 0xff, size);
 		break;
+	default:
+		msg_gerr("Invalid pattern %i\n", variant);
+		return 1;
 	}
 
 	if ((variant >= 0) && (variant <= 7)) {
@@ -1328,8 +1339,7 @@ 
 	return 0;
 }
 
-int erase_and_write_flash(struct flashctx *flash, uint8_t *oldcontents,
-			  uint8_t *newcontents)
+int erase_and_write_flash(struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents)
 {
 	int k, ret = 1;
 	uint8_t *curcontents;
@@ -1720,12 +1730,60 @@ 
 	return 0;
 }
 
+int autotest_flash(struct flashctx *flash)
+{
+	const int patternlist[] = {0, 4, 2, 5, 1, 3, 8, 9, 13, 12};
+	unsigned long size = flash->chip->total_size * 1024;
+	int i, erasefn, pattern, ret = 0;
+	uint8_t *oldcontents, *newcontents;
+
+	oldcontents = malloc(size);
+	if (!oldcontents) {
+		msg_gerr("Out of memory!\n");
+		exit(1);
+	}
+	newcontents = malloc(size);
+	if (!newcontents) {
+		msg_gerr("Out of memory!\n");
+		exit(1);
+	}
+	msg_cinfo("Starting autotest.\n");
+	/* outer loop: erase functions, inner loop: test patterns */
+	for (erasefn = 0; erasefn < NUM_ERASEFUNCTIONS; erasefn++) {
+		msg_cdbg("Trying erase function %i... ", erasefn);
+		if (check_block_eraser(flash, erasefn, 1))
+			continue;
+		for (i = 0; i < ARRAY_SIZE(patternlist); i++) {
+			pattern = patternlist[i];
+			msg_cinfo("Reading current flash chip contents... ");
+			if (flash->chip->read(flash, oldcontents, 0, size)) {
+				msg_cinfo("FAILED.\n");
+				return 1;
+			}
+			msg_cinfo("done.\n");
+			msg_cinfo("Erasing and writing flash chip with pattern %i... ", pattern);
+			if (generate_testpattern(newcontents, size, pattern)) {
+				msg_gerr("Pattern %i generation failed!\n", pattern);
+				continue;
+			}
+			print_hex(newcontents, 16);
+			msg_cinfo("\n");
+			ret |= walk_eraseregions(flash, erasefn, &erase_and_write_block_helper, oldcontents,
+						 newcontents);
+			/* Work around chips which need some time to calm down. */
+			programmer_delay(1000*1000);
+			ret |= verify_range(flash, newcontents, 0, size);
+		}
+	}
+	return ret;
+}		
+
 /* This function signature is horrible. We need to design a better interface,
  * but right now it allows us to split off the CLI code.
  * Besides that, the function itself is a textbook example of abysmal code flow.
  */
-int doit(struct flashctx *flash, int force, const char *filename, int read_it,
-	 int write_it, int erase_it, int verify_it)
+int doit(struct flashctx *flash, int force, const char *filename, int read_it, int write_it, int erase_it,
+	 int verify_it, int autotest_it)
 {
 	uint8_t *oldcontents;
 	uint8_t *newcontents;
@@ -1744,9 +1802,11 @@ 
 	if (flash->chip->unlock)
 		flash->chip->unlock(flash);
 
-	if (read_it) {
+	if (read_it || autotest_it) {
 		ret = read_flash_to_file(flash, filename);
-		goto out_nofree;
+		/* Only quit if we are doing a pure read. */
+		if (read_it)
+			goto out_nofree;
 	}
 
 	oldcontents = malloc(size);
@@ -1802,14 +1862,27 @@ 
 		}
 #endif
 	}
+	if (autotest_it) {
+		//FIXME: We completely ignore layouts in autotest mode.
+		autotest_flash(flash);
+		/* Restore original contents. */
+		if (read_buf_from_file(newcontents, size, filename)) {
+			/* FIXME: Proper error message. */
+			ret = 1;
+			goto out;
+		}
+		msg_ginfo("Restoring original chip contents:");
+	}
 
+	
+
 	/* Read the whole chip to be able to check whether regions need to be
 	 * erased and to give better diagnostics in case write fails.
 	 * The alternative would be to read only the regions which are to be
 	 * preserved, but in that case we might perform unneeded erase which
 	 * takes time as well.
 	 */
-	msg_cinfo("Reading old flash chip contents... ");
+	msg_cinfo("Reading current flash chip contents... ");
 	if (flash->chip->read(flash, oldcontents, 0, size)) {
 		ret = 1;
 		msg_cinfo("FAILED.\n");
@@ -1817,17 +1890,19 @@ 
 	}
 	msg_cinfo("done.\n");
 
-	// This should be moved into each flash part's code to do it 
-	// cleanly. This does the job.
-	handle_romentries(flash, oldcontents, newcontents);
+	/* All included romentries will be written. In autotest mode, included
+	 * romentries will be tested with the patterns, the rest will be left
+	 * alone.
+	 */
+	if (!autotest_it)
+		handle_romentries(flash, oldcontents, newcontents);
 
-	// ////////////////////////////////////////////////////////////
-
-	if (write_it) {
+	if (write_it || autotest_it) {
 		if (erase_and_write_flash(flash, oldcontents, newcontents)) {
 			msg_cerr("Uh oh. Erase/write failed. Checking if "
 				 "anything changed.\n");
-			if (!flash->chip->read(flash, newcontents, 0, size)) {
+			/* If this is the restore step of autotest, we want to complain in any case. */
+			if (!autotest_it && !flash->chip->read(flash, newcontents, 0, size)) {
 				if (!memcmp(oldcontents, newcontents, size)) {
 					msg_cinfo("Good. It seems nothing was "
 						  "changed.\n");
@@ -1842,10 +1917,10 @@ 
 		}
 	}
 
-	if (verify_it) {
+	if (verify_it || autotest_it) {
 		msg_cinfo("Verifying flash... ");
 
-		if (write_it) {
+		if (write_it || autotest_it) {
 			/* Work around chips which need some time to calm down. */
 			programmer_delay(1000*1000);
 			ret = verify_range(flash, newcontents, 0, size);