Le dimanche 01 novembre 2015 à 16:11 +0100, Nico Huber a écrit :
On 24.10.2015 13:24, Paul Kocialkowski wrote:
Most flash chips are erased to ones and programmed to zeros. However, some other flash chips, such as the ENE KB9012 internal flash, work the opposite way.
Signed-off-by: Paul Kocialkowski contact@paulk.fr
Two comments below.
Thanks for the review Nico!
flash.h | 5 ++++- flashrom.c | 39 +++++++++++++++++++++------------------ 2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/flash.h b/flash.h index 24861ba..55d02de 100644 --- a/flash.h +++ b/flash.h @@ -123,6 +123,9 @@ enum write_granularity { #define FEATURE_WRSR_EITHER (FEATURE_WRSR_EWSR | FEATURE_WRSR_WREN) #define FEATURE_OTP (1 << 8) #define FEATURE_QPI (1 << 9) +#define FEATURE_ERASED_ZERO (1 << 10)
+#define ERASED_VALUE(flash) ((flash->chip->feature_bits & FEATURE_ERASED_ZERO) ? 0x00 : 0xff)
For macros, it's a good habit to put references to a pointer parameter in parantheses. This prevents problems when the actual argument is not a plain identifier but an expression with lower precedence operators. So something like ERASED_VALUE(*pflash) wouldn't work with the definition above, but with #define ERASED_VALUE(flash) (((flash)-> ... it would.
Good point!
enum test_state { OK = 0, @@ -275,7 +278,7 @@ int probe_flash(struct registered_master *mst, int startchip, struct flashctx *f int read_flash_to_file(struct flashctx *flash, const char *filename); char *extract_param(const char *const *haystack, const char *needle, const char *delim); int verify_range(struct flashctx *flash, const uint8_t *cmpbuf, unsigned int start, unsigned int len); -int need_erase(const uint8_t *have, const uint8_t *want, unsigned int len, enum write_granularity gran); +int need_erase(const uint8_t *have, const uint8_t *want, unsigned int len, enum write_granularity gran, const uint8_t erased_value);
Minor: Actually, the const in `const uint8_t erased_value` isn't part of the contract and can be left out here. It just doesn't matter to the caller.
Well, it doesn't matter to have the const in the prototype, but it is meaningful in the function definition, and I'd rather have the prototype match the actual function definition, for consistency and easiness (to keep the habit of simply using copy-paste for prototypes).