[flashrom] [PATCH] Allow programmers to disallow writes

[Carl-Daniel Hailfinger]

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 at 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 at 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. ");


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