On 02.07.2010 04:10, Carl-Daniel Hailfinger wrote:
On 01.07.2010 18:10, Michael Karcher wrote:
Am Donnerstag, den 01.07.2010, 17:32 +0200 schrieb Carl-Daniel Hailfinger:
+/* Is writing allowed with this programmer? */ +int programmer_may_write = 1;
... libflashrom guys are going to hate you for another global variable,
Haha, true. But since I'm one of the developers who want libflashrom, I'd like to deal with all globals in one big patch, not in some piecemeal fashion. Globals are actually not such a big problem for libflashrom because we should use a special annotation for exported symbols of the library anyway.
I don't see a way to make the variable non-global if it should be settable from inside programmer_init functions. Ideas welcome.
I guess. And they *will* hate you for statically initialized global variables, because if a GUI wants to flash twice, and the first attempt sets programmer_may_write to zero, the second flashing process will also run into the "you don't want to write" test. Can't you set this variable to 1 at the top of the generic programmer_init function?
Done.
This patch (and a patch dynamically initializing the variable at said point in code) are Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
New version. The variable is still global, but it is set on programmer_init.
If a programmer has untested or non-working write/erase code, but probing/reading works, it makes sense to protect the user against write/erase accidents. This feature will be used by the Nvidia MCP SPI code, and it also might make sense for the gfxnvidia driver which has non-working write/erase.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-programmer_may_write/flash.h =================================================================== --- flashrom-programmer_may_write/flash.h (Revision 1068) +++ flashrom-programmer_may_write/flash.h (Arbeitskopie) @@ -569,6 +569,7 @@ uint32_t spi; }; extern struct decode_sizes max_rom_decode; +extern int programmer_may_write; extern char *programmer_param; extern unsigned long flashbase; extern int verbose; Index: flashrom-programmer_may_write/flashrom.c =================================================================== --- flashrom-programmer_may_write/flashrom.c (Revision 1068) +++ flashrom-programmer_may_write/flashrom.c (Arbeitskopie) @@ -103,6 +103,9 @@ /* If nonzero, used as the start address of bottom-aligned flash. */ unsigned long flashbase;
+/* Is writing allowed with this programmer? */ +int programmer_may_write; + const struct programmer_entry programmer_table[] = { #if CONFIG_INTERNAL == 1 { @@ -447,6 +450,8 @@ flashbase = 0; /* Registering shutdown functions is now allowed. */ may_register_shutdown = 1; + /* Default to allowing writes. Broken programmers set this to 0. */ + programmer_may_write = 1;
programmer_param = param; msg_pdbg("Initializing %s programmer\n", @@ -1383,6 +1388,21 @@ size = flash->total_size * 1024; buf = (uint8_t *) calloc(size, sizeof(char));
+ if (!programmer_may_write && (write_it || erase_it)) { + msg_perr("Write/erase is not working yet on your programmer in " + "its current configuration.\n"); + /* --force is the wrong approach, but it's the best we can do + * until the generic programmer parameter parser is merged. + */ + if (!force) { + msg_perr("Aborting.\n"); + programmer_shutdown(); + return 1; + } else { + msg_cerr("Continuing anyway.\n"); + } + } + if (erase_it) { if (flash->tested & TEST_BAD_ERASE) { msg_cerr("Erase is not working on this chip. ");