Attention is currently required from: Miklós Márton, Peter Marheine.
Anastasia Klimchuk 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:
(1 comment)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/84203/comment/db3d63e0_6e7fe218?usp... : PS3, Line 487: for (int i = 0; opcode[i]; i++) {
This will always crash if spi_get_opcode_from_erasefn doesn't know the eraser, which should probably be fixed to gracefully bail out.
100% yes, I added a check for null. There are 22 opcodes known to `spi_get_opcode_from_erasefn` and 34 erase functions known to `lookup_erase_func_ptr` ! oh :( yes, we need to check for null.
About `FEATURE_NO_ERASE`, I thought those are chips that don't have any erase opcodes, and they just don't support erase operation at all - whether it's erase-before-write or just erase, doesn't matter. The way the feature is implemented is that when such a chip is asked to be erased, the special `SPI_BLOCK_ERASE_EMULATION` is quietly doing write instead of erase, and the end result for the chip memory is the same.
So, I added a check for null, but haven't added any comment to FEATURE_NO_ERASE yet. If I would be adding a comment, I would say "chips that don't have erase operation capability, and no erase opcodes"?
-
Anastasia Klimchuk (Code Review)