Attention is currently required from: Anastasia Klimchuk, Miklós Márton.
Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84203?usp=email )
Change subject: Fix FEATURE_NO_ERASE chips and add test for them ......................................................................
Patch Set 4:
(2 comments)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/84203/comment/369f0e75_d38fa01e?usp... : PS3, Line 487: for (int i = 0; opcode[i]; i++) {
I would say "chips that don't have erase operation capability, and no erase opcodes"?
I don't think it's correct to say that, but we kind of need to *decide* what meaning this has because is only has a few users and none of them are currently well-documented. There are currently only two cases where `FEATURE_NO_ERASE` can be enabled:
* `linux_mtd` enables it if the kernel reports `MTD_NO_ERASE`, which is described by a comment as "no erase necessary" * M95M02 EEPROM has it enabled statically
For MTD devices, the description seems fairly clear that there may be erase functionality but it doesn't need to be erased before writing. M92M02 doesn't have an erase command, but that's common for EEPROMs because there's usually on reason to do bulk erases. I [found one person claiming some EEPROMs have erase commands](https://forums.anandtech.com/threads/erasing-an-eeprom-chip.1327148/#post-11...) but no specific part numbers aside from built-in EEPROMs in microcontrollers which often do have dedicated erase commands.
Given these, I think we need to use the same interpretation as Linux does, and make `FEATURE_NO_ERASE` indicate that erase is not required before writing but can still be done (and on some chips erasing may be faster than writing an "erased" value).
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/84203/comment/00c840ed_6c823244?usp... : PS4, Line 485: !(flash->chip->feature_bits & FEATURE_NO_ERASE) I don't think you need this check, and for a chip that doesn't need erase but supports a fast erase it would be undesirable.
-
Peter Marheine (Code Review)