[flashrom] [PATCH v2 1/2] Flag-driven erased bit value

[Nico Huber]

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 at paulk.fr>
Two comments below.

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.

>  
>  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.