On 10.07.2010 20:30, Michael Karcher wrote: > Am Samstag, den 10.07.2010, 03:05 +0200 schrieb Carl-Daniel Hailfinger: > >> -int spi_disable_blockprotect(void); >> +int spi_disable_blockprotect(struct flashchip *flash); >> > This change makes sense, but you don't use the flash parameter yet. > > >> @@ -1392,6 +1408,7 @@ >> .block_erase = spi_block_erase_c7, >> } >> }, >> + .unlock = spi_disable_blockprotect, >> .write = spi_chip_write_1, >> .read = read_memmapped, >> }, >> > OUCH! "blame" me for committing that! read must not be "read_memmapped". > Should I submit a fixup patch for that or do you want to fix that in one > of your patches? >
Can you fix it up? Such a change is > Acked-by: Carl-Daniel Hailfinger <[email protected]> >> -int spi_disable_blockprotect(void) >> +int spi_disable_blockprotect(struct flashchip *flash) >> { >> uint8_t status; >> int result; >> @@ -855,6 +843,11 @@ >> msg_cerr("spi_write_status_register failed\n"); >> return result; >> } >> + status = spi_read_status_register(); >> + if ((status & 0x3c) != 0) { >> + msg_cerr("Block protection could not be disabled!\n"); >> + /* Should we error out here? */ >> > Good question. As long as we have no partial write, we really should > error out here. But when we get partial writes, it would be great to > have flashrom being able to flash the lower part of the chip even if the > upper part is write protected. This would need major code changes, > though (unlocking would need to get the range to be unlocked), so for > now erroring out seems like the best option and as soon as not erroring > out might be sensible, we need to touch the code anyway. > OK, will change the code to error out. Thanks for your review. Regards, Carl-Daniel -- http://www.hailfinger.org/ _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
