Anastasia Klimchuk wrote in #note-2:
The flag which is used to detect verify_after_write
is actually -n
or dont_verify_it
, which makes sense because it should verify by default. It is described as Skip the automatic verification of flash ROM contents after writing.
which is consistent with the name.
So the name and description says it's for writing operation only, and the code is consistent.
I don't think that we can change the implementation of the existing flag which has been around for ages.
We can however, add a new flag for erase operation specifically.
What is the exact use case for how you would use the new flag? Do you erase a lot of chips with the script?
I worded this a bit weird and it should be clarified. It's been a while since I thought about this so I'm trying to jog my own memory...
I think some of this is a semantic thing. I interpret "writing" to a flash chip any action which commits new values to be persisted, so an erase operation is a "write" because it's flipping bits on flash.
I think the biggest issues are:
- when a user says "do not verify these writes", erasures are verified anyway. Regardless of call path (erase or write),
check_erased_range
performs verification after blocks are erased and there is no shield from this.
- For the
flashrom_image_write
call path, this means we're performing verification on erase and then performing verification after the writes occur (so we're verifying twice) unless dont_verify_it
is specified, but this doesn't shield verifying erasures
- For the
flashrom_image_erase
call path, we only perform the forced verification via check_erased_range
- If an erase is unaligned, there is a subsequent write to fill in data that should not have been erased. Some write functions implement a
verify_range
call, but not all functions look to verify the write was persisted (in fact, it only looks like the WRITE_JEDEC{1} verifies writes). flashrom_image_write
can get away with this because it performs a post-write verification (unless told otherwise) and can spot a problem, however this is not true for flashrom_image_erase
as it only verifies on erase and implements whatever checks are intrinsic to the device write function, which may be nothing.
Of course, i may have misread the code, but this is what it looks like to me.
As for the CLI changes, I realize this wouldn't happen for a while, but large API/interface changes and CLI changes to redefine flags should be allowed for major version bumps, otherwise legacy codebases are stuck maintaining old code forever. Adding new flags don't require this however.