Attention is currently required from: Sergii Dmytruk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60230 )
Change subject: spi25_statusreg.c: add SR3 read/write support ......................................................................
Patch Set 9:
(1 comment)
File flash.h:
https://review.coreboot.org/c/flashrom/+/60230/comment/d4df43d2_fe7f1d2e PS2, Line 148: FEATURE_SR3
Thank for explaining, I understand why there is one flag. […]
As mentioned above SR2 is a different case no matter what we do, as different chips have different commands to access it.
The question is what we want to achieve with the FEATURE_SR3 flag. In the current PS, it's used to confirm that SR3 is supported before we actually send a related command. But, what does it mean if this check fails? It would be a bug in flashrom or the chip database. If we know that for sure, we should tell the user to tell us.
Also, if we consider the database fields for write-protection bits could be wrong what does that say about possibly wrong FEATURE_SR3 flags? :) One might even say due to the redundancy there is more that can go wrong. (Avoiding redundancy was part of my first data- base lessons.)
Now, if it's worth the hassle is hard to tell. We already have other mechanisms like the test state to keep track of the sanity of data- base entries. (just noticed, WP lacks a field in `struct tested`)
-
Nico Huber (Code Review)