[flashrom] [PATCH] Move implicit erase out of chip drivers, clean up

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Wed Jul 14 23:57:33 CEST 2010


flashrom had an implicit erase-on-write for most flash chip and
programmer drivers, but it was not entirely consistent. Some drivers had
their own hand-rolled partial update functionality which made handling
partial updates from generic code impossible.

Move implicit erase out of chip drivers, and kill some dead erase
functions at the same time.
A full chip erase is now performed in the generic code for all flash
chips on write, and after that the whole chip is written.
This is needed before we can change write function signature of the
write functions to take start+len.

Compile tested and proofread, but that's it. The patch may cause
flashrom to eat your dog or it may cause your dog to eat burned flash chips.
Please test on real hardware. Thanks!

I have a cleanup patch for the write functions which will apply on top
of this, but I wanted to keep this patch readable and self-contained.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Index: flashrom-no_implicit_erase_inside_write/spi25.c
===================================================================
--- flashrom-no_implicit_erase_inside_write/spi25.c	(Revision 1082)
+++ flashrom-no_implicit_erase_inside_write/spi25.c	(Arbeitskopie)
@@ -976,14 +976,6 @@
 
 int spi_chip_write_1(struct flashchip *flash, uint8_t *buf)
 {
-	/* Erase first */
-	msg_cinfo("Erasing flash before programming... ");
-	if (erase_flash(flash)) {
-		msg_cerr("ERASE FAILED!\n");
-		return -1;
-	}
-	msg_cinfo("done.\n");
-
 	return spi_chip_write_1_new(flash, buf, 0, flash->total_size * 1024);
 }
 
Index: flashrom-no_implicit_erase_inside_write/jedec.c
===================================================================
--- flashrom-no_implicit_erase_inside_write/jedec.c	(Revision 1082)
+++ flashrom-no_implicit_erase_inside_write/jedec.c	(Arbeitskopie)
@@ -402,11 +402,6 @@
 
 	mask = getaddrmask(flash);
 
-	if (erase_chip_jedec(flash)) {
-		msg_cerr("ERASE FAILED!\n");
-		return -1;
-	}
-	
 	msg_cinfo("Programming page: ");
 	for (i = 0; i < total_size / page_size; i++) {
 		msg_cinfo("%04d at address: 0x%08x", i, i * page_size);
@@ -429,12 +424,6 @@
 
 	mask = getaddrmask(flash);
 
-	programmer_delay(10);
-	if (erase_flash(flash)) {
-		msg_cerr("ERASE FAILED!\n");
-		return -1;
-	}
-
 	msg_cinfo("Programming page: ");
 	for (i = 0; i < flash->total_size; i++) {
 		if ((i & 0x3) == 0)
Index: flashrom-no_implicit_erase_inside_write/sst49lfxxxc.c
===================================================================
--- flashrom-no_implicit_erase_inside_write/sst49lfxxxc.c	(Revision 1082)
+++ flashrom-no_implicit_erase_inside_write/sst49lfxxxc.c	(Arbeitskopie)
@@ -86,11 +86,6 @@
 	write_lockbits_49lfxxxc(flash, 0);
 	msg_cinfo("Programming page: ");
 	for (i = 0; i < total_size / page_size; i++) {
-		/* erase the page before programming */
-		if (erase_sector_49lfxxxc(flash, i * page_size, flash->page_size)) {
-			msg_cerr("ERASE FAILED!\n");
-			return -1;
-		}
 
 		/* write to the sector */
 		msg_cinfo("%04d at address: 0x%08x", i, i * page_size);
Index: flashrom-no_implicit_erase_inside_write/sharplhf00l04.c
===================================================================
--- flashrom-no_implicit_erase_inside_write/sharplhf00l04.c	(Revision 1082)
+++ flashrom-no_implicit_erase_inside_write/sharplhf00l04.c	(Arbeitskopie)
@@ -65,10 +65,6 @@
 	int page_size = flash->page_size;
 	chipaddr bios = flash->virtual_memory;
 
-	if (erase_flash(flash)) {
-		msg_cerr("ERASE FAILED!\n");
-		return -1;
-	}
 	msg_cinfo("Programming page: ");
 	for (i = 0; i < total_size / page_size; i++) {
 		msg_cinfo("%04d at address: 0x%08x", i, i * page_size);
Index: flashrom-no_implicit_erase_inside_write/spi.c
===================================================================
--- flashrom-no_implicit_erase_inside_write/spi.c	(Revision 1082)
+++ flashrom-no_implicit_erase_inside_write/spi.c	(Arbeitskopie)
@@ -217,12 +217,6 @@
 {
 	int ret;
 
-	msg_pinfo("Erasing flash before programming... ");
-	if (erase_flash(flash)) {
-		msg_perr("ERASE FAILED!\n");
-		return -1;
-	}
-	msg_pinfo("done.\n");
 	msg_pinfo("Programming flash... ");
 	ret = spi_chip_write_256_new(flash, buf, 0, flash->total_size * 1024);
 	if (!ret)
Index: flashrom-no_implicit_erase_inside_write/sst28sf040.c
===================================================================
--- flashrom-no_implicit_erase_inside_write/sst28sf040.c	(Revision 1082)
+++ flashrom-no_implicit_erase_inside_write/sst28sf040.c	(Arbeitskopie)
@@ -122,12 +122,6 @@
 
 	msg_cinfo("Programming page: ");
 	for (i = 0; i < total_size / page_size; i++) {
-		/* erase the page before programming */
-		if (erase_sector_28sf040(flash, i * page_size, page_size)) {
-			msg_cerr("ERASE FAILED!\n");
-			return -1;
-		}
-
 		/* write to the sector */
 		msg_cinfo("%04d at address: 0x%08x", i, i * page_size);
 		write_sector_28sf040(bios, buf + i * page_size,
Index: flashrom-no_implicit_erase_inside_write/stm50flw0x0x.c
===================================================================
--- flashrom-no_implicit_erase_inside_write/stm50flw0x0x.c	(Revision 1082)
+++ flashrom-no_implicit_erase_inside_write/stm50flw0x0x.c	(Arbeitskopie)
@@ -93,6 +93,7 @@
 	return 0;
 }
 
+/* This function is unused. */
 int erase_sector_stm50flw0x0x(struct flashchip *flash, unsigned int sector, unsigned int sectorsize)
 {
 	chipaddr bios = flash->virtual_memory + sector;
@@ -116,6 +117,7 @@
 	return 0;
 }
 
+/* FIXME: This function is not a real chip erase function. */
 int erase_chip_stm50flw0x0x(struct flashchip *flash, unsigned int addr, unsigned int blocklen)
 {
 	int i;
Index: flashrom-no_implicit_erase_inside_write/flashrom.c
===================================================================
--- flashrom-no_implicit_erase_inside_write/flashrom.c	(Revision 1082)
+++ flashrom-no_implicit_erase_inside_write/flashrom.c	(Arbeitskopie)
@@ -1421,6 +1421,7 @@
 
 /* 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 flashchip *flash, int force, char *filename, int read_it, int write_it, int erase_it, int verify_it)
 {
@@ -1475,12 +1476,7 @@
 			programmer_shutdown();
 			return 1;
 		}
-	} else {
-		struct stat image_stat;
-
-		if (flash->unlock)
-			flash->unlock(flash);
-
+	} else if (write_it) {
 		if (flash->tested & TEST_BAD_ERASE) {
 			msg_cerr("Erase is not working on this chip "
 				"and erase is needed for write. ");
@@ -1502,6 +1498,18 @@
 				msg_cerr("Continuing anyway.\n");
 			}
 		}
+		if (!flash->write) {
+			msg_cerr("Error: flashrom has no write function for this flash chip.\n");
+			programmer_shutdown();
+			return 1;
+		}
+		if (flash->unlock)
+			flash->unlock(flash);
+
+	}
+	if (write_it || verify_it) {
+		struct stat image_stat;
+
 		if ((image = fopen(filename, "rb")) == NULL) {
 			perror(filename);
 			programmer_shutdown();
@@ -1537,12 +1545,12 @@
 	// ////////////////////////////////////////////////////////////
 
 	if (write_it) {
-		msg_cinfo("Writing flash chip... ");
-		if (!flash->write) {
-			msg_cerr("Error: flashrom has no write function for this flash chip.\n");
+		if (erase_flash(flash)) {
+			emergency_help_message();
 			programmer_shutdown();
 			return 1;
 		}
+		msg_cinfo("Writing flash chip... ");
 		ret = flash->write(flash, buf);
 		if (ret) {
 			msg_cerr("FAILED!\n");
Index: flashrom-no_implicit_erase_inside_write/82802ab.c
===================================================================
--- flashrom-no_implicit_erase_inside_write/82802ab.c	(Revision 1082)
+++ flashrom-no_implicit_erase_inside_write/82802ab.c	(Arbeitskopie)
@@ -144,23 +144,6 @@
 	return 0;
 }
 
-int erase_82802ab(struct flashchip *flash)
-{
-	int i;
-	unsigned int total_size = flash->total_size * 1024;
-
-	msg_cspew("total_size is %d; flash->page_size is %d\n",
-	       total_size, flash->page_size);
-	for (i = 0; i < total_size; i += flash->page_size)
-		if (erase_block_82802ab(flash, i, flash->page_size)) {
-			msg_cerr("ERASE FAILED!\n");
-			return -1;
-		}
-	msg_cinfo("DONE ERASE\n");
-
-	return 0;
-}
-
 void write_page_82802ab(chipaddr bios, uint8_t *src,
 			chipaddr dst, int page_size)
 {
@@ -179,11 +162,6 @@
 	int i;
 	chipaddr bios = flash->virtual_memory;
 
-	if (erase_flash(flash)) {
-		msg_cerr("ERASE FAILED!\n");
-		return -1;
-	}
-
 	msg_cinfo("Programming at: ");
 	for (i = 0; i < flash->total_size; i++) {
 		if ((i & 0x3) == 0)
Index: flashrom-no_implicit_erase_inside_write/chipdrivers.h
===================================================================
--- flashrom-no_implicit_erase_inside_write/chipdrivers.h	(Revision 1082)
+++ flashrom-no_implicit_erase_inside_write/chipdrivers.h	(Arbeitskopie)
@@ -33,8 +33,6 @@
 int probe_spi_res2(struct flashchip *flash);
 int spi_write_enable(void);
 int spi_write_disable(void);
-int spi_chip_erase_60(struct flashchip *flash);
-int spi_chip_erase_c7(struct flashchip *flash);
 int spi_block_erase_20(struct flashchip *flash, unsigned int addr, unsigned int blocklen);
 int spi_block_erase_52(struct flashchip *flash, unsigned int addr, unsigned int blocklen);
 int spi_block_erase_d7(struct flashchip *flash, unsigned int addr, unsigned int blocklen);
@@ -58,7 +56,6 @@
 /* 82802ab.c */
 uint8_t wait_82802ab(chipaddr bios);
 int probe_82802ab(struct flashchip *flash);
-int erase_82802ab(struct flashchip *flash);
 int erase_block_82802ab(struct flashchip *flash, unsigned int page, unsigned int pagesize);
 int write_82802ab(struct flashchip *flash, uint8_t *buf);
 void print_status_82802ab(uint8_t status);
@@ -73,7 +70,6 @@
 int write_byte_program_jedec(chipaddr bios, uint8_t *src,
 			     chipaddr dst);
 int probe_jedec(struct flashchip *flash);
-int erase_chip_jedec(struct flashchip *flash);
 int write_jedec(struct flashchip *flash, uint8_t *buf);
 int write_jedec_1(struct flashchip *flash, uint8_t *buf);
 int erase_sector_jedec(struct flashchip *flash, unsigned int page, unsigned int pagesize);
@@ -83,7 +79,6 @@
 
 /* m29f400bt.c */
 int probe_m29f400bt(struct flashchip *flash);
-int erase_m29f400bt(struct flashchip *flash);
 int block_erase_m29f400bt(struct flashchip *flash, unsigned int start, unsigned int len);
 int block_erase_chip_m29f400bt(struct flashchip *flash, unsigned int start, unsigned int len);
 int write_m29f400bt(struct flashchip *flash, uint8_t *buf);
Index: flashrom-no_implicit_erase_inside_write/m29f400bt.c
===================================================================
--- flashrom-no_implicit_erase_inside_write/m29f400bt.c	(Revision 1082)
+++ flashrom-no_implicit_erase_inside_write/m29f400bt.c	(Arbeitskopie)
@@ -143,7 +143,6 @@
 	int page_size = flash->page_size;
 	chipaddr bios = flash->virtual_memory;
 
-	//erase_m29f400bt (flash);
 	msg_cinfo("Programming page:\n ");
 	/*********************************
 	*Pages for M29F400BT:
@@ -163,41 +162,21 @@
 	msg_cinfo("total_size/page_size = %d\n", total_size / page_size);
 	for (i = 0; i < (total_size / page_size) - 1; i++) {
 		msg_cinfo("%04d at address: 0x%08x\n", i, i * page_size);
-		if (block_erase_m29f400bt(flash, i * page_size, page_size)) {
-			msg_cerr("ERASE FAILED!\n");
-			return -1;
-		}
 		write_page_m29f400bt(bios, buf + i * page_size,
 				     bios + i * page_size, page_size);
 		msg_cinfo("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
 	}
 
 	msg_cinfo("%04d at address: 0x%08x\n", 7, 0x70000);
-	if (block_erase_m29f400bt(flash, 0x70000, 32 * 1024)) {
-		msg_cerr("ERASE FAILED!\n");
-		return -1;
-	}
 	write_page_m29f400bt(bios, buf + 0x70000, bios + 0x70000, 32 * 1024);
 
 	msg_cinfo("%04d at address: 0x%08x\n", 8, 0x78000);
-	if (block_erase_m29f400bt(flash, 0x78000, 8 * 1024)) {
-		msg_cerr("ERASE FAILED!\n");
-		return -1;
-	}
 	write_page_m29f400bt(bios, buf + 0x78000, bios + 0x78000, 8 * 1024);
 
 	msg_cinfo("%04d at address: 0x%08x\n", 9, 0x7a000);
-	if (block_erase_m29f400bt(flash, 0x7a000, 8 * 1024)) {
-		msg_cerr("ERASE FAILED!\n");
-		return -1;
-	}
 	write_page_m29f400bt(bios, buf + 0x7a000, bios + 0x7a000, 8 * 1024);
 
 	msg_cinfo("%04d at address: 0x%08x\n", 10, 0x7c000);
-	if (block_erase_m29f400bt(flash, 0x7c000, 16 * 1024)) {
-		msg_cerr("ERASE FAILED!\n");
-		return -1;
-	}
 	write_page_m29f400bt(bios, buf + 0x7c000, bios + 0x7c000, 16 * 1024);
 
 	msg_cinfo("\n");
@@ -226,31 +205,15 @@
 	* 64	0x00000		0x0ffff		BOTTOM
 	*********************************/
 	msg_cinfo("%04d at address: 0x%08x\n", 7, 0x00000);
-	if (block_erase_m29f400bt(flash, 0x00000, 64 * 1024)) {
-		msg_cerr("ERASE FAILED!\n");
-		return -1;
-	}
 	write_page_m29f400bt(bios, buf + 0x00000, bios + 0x00000, 64 * 1024);
 
 	msg_cinfo("%04d at address: 0x%08x\n", 7, 0x10000);
-	if (block_erase_m29f400bt(flash, 0x10000, 64 * 1024)) {
-		msg_cerr("ERASE FAILED!\n");
-		return -1;
-	}
 	write_page_m29f400bt(bios, buf + 0x10000, bios + 0x10000, 64 * 1024);
 
 	msg_cinfo("%04d at address: 0x%08x\n", 7, 0x20000);
-	if (block_erase_m29f400bt(flash, 0x20000, 64 * 1024)) {
-		msg_cerr("ERASE FAILED!\n");
-		return -1;
-	}
 	write_page_m29f400bt(bios, buf + 0x20000, bios + 0x20000, 64 * 1024);
 
 	msg_cinfo("%04d at address: 0x%08x\n", 7, 0x30000);
-	if (block_erase_m29f400bt(flash, 0x30000, 64 * 1024)) {
-		msg_cerr("ERASE FAILED!\n");
-		return -1;
-	}
 	write_page_m29f400bt(bios, buf + 0x30000, bios + 0x30000, 64 * 1024);
 
 	msg_cinfo("\n");


-- 
http://www.hailfinger.org/





More information about the flashrom mailing list