Re: [flashrom] [PATCH] Improve error messages, protect the user

[Carl-Daniel Hailfinger]

On 12.08.2009 11:39, Stefan Reinauer wrote:
> On 8/11/09 5:25 PM, Carl-Daniel Hailfinger wrote:
> > if (flash->tested & TEST_BAD_ERASE) {
> > 	fprintf(stderr, "Erase is not working on this chip "
> > 		"and erase is needed for write. Aborting.\n");
> > 	return 1;
> > }
> > if (flash->tested & TEST_BAD_WRITE) {
> > 	fprintf(stderr, "Write is not working on this chip. "
> > 		"Aborting.\n");
> > 	return 1;
> > }
>
> I would agree to this as long as there's a --force option

Updated patch attached.
> I think the IRC note should go to the documentation, not the code.

Looking at the experience of most failed flashes where the users came to
us after it was too late or rebooted directly after joining IRC, I'd say
directing them with a program message to a realtime support channel
instead of some piece of text in the documentation is the right way to
go. After all, quite a few distro packages do not install the README.

New version:
- Allow BAD override with --force
- Wait 1 second between erase and verify. This fixes a few reports where
verify directly after erase had unpleasant side effects like corrupting
flash or at least getting incorrect verify results.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-errormessages/flashrom.c
===================================================================
--- flashrom-errormessages/flashrom.c	(Revision 678)
+++ flashrom-errormessages/flashrom.c	(Arbeitskopie)
@@ -222,6 +222,7 @@
 {
    size_t size = flash->total_size * 1024;
    /* Flash registers live 4 MByte below the flash. */
+	/* FIXME: This is incorrect for nonstandard flashbase. */
    flash->virtual_registers = (chipaddr)programmer_map_flash_region("flash chip registers", (0xFFFFFFFF - 0x400000 - size + 1), size);
 }

@@ -476,6 +477,15 @@
    return 0;
 }

+void emergency_help_message()
+{
+	fprintf(stderr, "Your flash chip is in an unknown state.\n"
+		"Get help on IRC at irc.freenode.net channel #flashrom or\n"
+		"mail [e-mail]\n"
+		"------------------------------------------------------------\n"
+		"DO NOT REBOOT OR POWEROFF!\n");
+}
+
 void usage(const char *name)
 {
    printf("usage: %s [-VfLzhR] [-E|-r file|-w file|-v file] [-c chipname]\n"
@@ -744,6 +754,7 @@
    			printf("Run flashrom -L to view the hardware supported in this flashrom version.\n");
    			exit(1);
    		}
+			printf("Please note that forced reads most likely contain garbage.\n");
    		return read_flash(flashes[0], filename);
    	}
    	// FIXME: flash writes stay enabled!
@@ -809,14 +820,44 @@
    buf = (uint8_t *) calloc(size, sizeof(char));

if (erase_it) {
-		if (erase_flash(flash))
+		if (flash->tested & TEST_BAD_ERASE) {
+			fprintf(stderr, "Erase is not working on this chip. ");
+			if (!force) {
+				fprintf(stderr, "Aborting.\n");
+				return 1;
+			} else {
+				fprintf(stderr, "Continuing anyway.\n");
+			}
+		}
+		if (erase_flash(flash)) {
+			emergency_help_message();
    		return 1;
+		}
    } else if (read_it) {
    	if (read_flash(flash, filename))
    		return 1;
    } else {
    	struct stat image_stat;

+		if (flash->tested & TEST_BAD_ERASE) {
+			fprintf(stderr, "Erase is not working on this chip "
+				"and erase is needed for write. ");
+			if (!force) {
+				fprintf(stderr, "Aborting.\n");
+				return 1;
+			} else {
+				fprintf(stderr, "Continuing anyway.\n");
+			}
+		}
+		if (flash->tested & TEST_BAD_WRITE) {
+			fprintf(stderr, "Write is not working on this chip. ");
+			if (!force) {
+				fprintf(stderr, "Aborting.\n");
+				return 1;
+			} else {
+				fprintf(stderr, "Continuing anyway.\n");
+			}
+		}
    	if ((image = fopen(filename, "r")) == NULL) {
    		perror(filename);
    		exit(1);
@@ -855,14 +896,21 @@
    	ret = flash->write(flash, buf);
    	if (ret) {
    		fprintf(stderr, "FAILED!\n");
+			emergency_help_message();
    		return 1;
    	} else {
    		printf("COMPLETE.\n");
    	}
    }

-	if (verify_it)
+	if (verify_it) {
+		/* Work around chips which need some time to calm down. */
+		if (write_it)
+			programmer_delay(1000*1000);
    	ret = verify_flash(flash, buf);
+		if (ret)
+			emergency_help_message();
+	}

programmer_shutdown();