[flashrom] [PATCH 2/3] add check_block_erasers which returns the number of well-defined erasers for a chip

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Fri Jul 1 00:27:35 CEST 2011


Sorry, another comment...

Am 01.07.2011 00:25 schrieb Carl-Daniel Hailfinger:
> Am 30.06.2011 22:37 schrieb Stefan Tauner:
>   
>> add count_usable_erasers which returns the number of well-defined erasers for a chip
>>
>> This can be used in various situations (including one in the upcoming SFDP patch) and
>> removes one FIXME in current HEAD. Needed to add a declaration of check_block_eraser.
>>
>> Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
>>
>> diff --git a/flashrom.c b/flashrom.c
>> index 13d398e..10035c4 100644
>> --- a/flashrom.c
>> +++ b/flashrom.c
>> @@ -1818,13 +1830,13 @@ int chip_safety_check(struct flashchip *flash, int force, int read_it, int write
>>  	}
>>  	if (erase_it || write_it) {
>>  		/* Write needs erase. */
>> -		if (flash->tested & TEST_BAD_ERASE) {
>> +		if (flash->tested & TEST_BAD_ERASE ||
>> +		    !count_usable_erasers(flash, 0)) {
>>   
>>     
> Please split that if statement. Allowing --force makes sense for
> TEST_BAD_ERASE, but it makes no sense if no erase functions are present.
> You can reuse the error message, the compiler/linker will only store the
> string once.
>   

Would it make sense to remove the check for usable_erasefunctions!=0 in
erase_and_write_flash() if we already do it here?


>>  			msg_cerr("Erase is not working on this chip. ");
>>  			if (!force)
>>  				return 1;
>>  			msg_cerr("Continuing anyway.\n");
>>  		}
>> -		/* FIXME: Check if at least one erase function exists. */
>>  	}
>>  	if (write_it) {
>>  		if (flash->tested & TEST_BAD_WRITE) {
>>   
>>     
> With the split if statement, this is
> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>   

Regards,
Carl-Daniel

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





More information about the flashrom mailing list