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 <[email protected]> >> 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 <[email protected]> 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. "); -- http://www.hailfinger.org/ _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
