[flashrom] [PATCH] Fix sfdp_add_uniform_eraser and its usage.

[Carl-Daniel Hailfinger]

Am 19.02.2012 20:05 schrieb Stefan Tauner:
> sfdp_add_uniform_eraser checks for existing erasers. Due to a bug it looked for
> eraser slots that have no erase functions set instead of those that have one set.
>
> Also, the special 4k opcode encoded in the first double word was tried to add
> with sfdp_add_uniform_eraser

-ENOPARSE

> although the total size of the chip was not yet
> known. This is now postponed and an additional check to sfdp_add_uniform_eraser
> is added.
>
> Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
> ---
> this is the last fixup combined with what is described in the second half
> of the commit log above.
>
>  sfdp.c |   18 +++++++++++++-----
>  1 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/sfdp.c b/sfdp.c
> index 75dfb5f..80faeca 100644
> --- a/sfdp.c
> +++ b/sfdp.c
> @@ -78,8 +78,10 @@ static int sfdp_add_uniform_eraser(struct flashctx *flash, uint8_t opcode, uint3
>  	uint32_t total_size = flash->total_size * 1024;
>  	erasefunc_t *erasefn = spi_get_erasefn_from_opcode(opcode);
>  
> -	if (erasefn == NULL || block_size == 0 || total_size % block_size != 0) {
> -		msg_cdbg("%s: invalid input\n", __func__);
> +	if (erasefn == NULL || total_size == 0 || block_size == 0 ||
> +	    total_size % block_size != 0) {
> +		msg_cdbg("%s: invalid input, please report to "
> +			 "flashrom at flashrom.org\n", __func__);
>  		return 1;
>  	}
>  
> @@ -89,11 +91,12 @@ static int sfdp_add_uniform_eraser(struct flashctx *flash, uint8_t opcode, uint3
>  		if (eraser->eraseblocks[0].size == block_size &&
>  		    eraser->block_erase == erasefn) {
>  			msg_cdbg2("  Tried to add a duplicate block eraser: "
> -				  "%d x %d B with opcode 0x%02x\n",
> +				  "%d x %d B with opcode 0x%02x.\n",
>  				  total_size/block_size, block_size, opcode);
>  			return 1;
>  		}
> -		if (eraser->eraseblocks[0].size != 0 || !eraser->block_erase) {
> +		if (eraser->eraseblocks[0].size != 0 ||
> +		    eraser->block_erase != NULL) {
>  			msg_cspew("  Block Eraser %d is already occupied.\n",
>  				  i);
>  			continue;
> @@ -115,6 +118,7 @@ static int sfdp_add_uniform_eraser(struct flashctx *flash, uint8_t opcode, uint3
>  
>  static int sfdp_fill_flash(struct flashctx *flash, uint8_t *buf, uint16_t len)
>  {
> +	uint8_t opcode_4k = 0xFF;

rename to opcode_4k_erase?


>  	uint32_t tmp32;
>  	uint8_t tmp8;
>  	uint32_t total_size; /* in bytes */
> @@ -181,7 +185,8 @@ static int sfdp_fill_flash(struct flashctx *flash, uint8_t *buf, uint16_t len)
>  	}
>  
>  	if ((tmp32 & 0x3) == 0x1) {
> -		sfdp_add_uniform_eraser(flash, (tmp32 >> 8) & 0xFF, 4 * 1024);
> +		/* add the eraser later, because we don't know total_size yet */
> +		opcode_4k = (tmp32 >> 8) & 0xFF;
>  	}
>  
>  	/* 2. double word */
> @@ -204,6 +209,9 @@ static int sfdp_fill_flash(struct flashctx *flash, uint8_t *buf, uint16_t len)
>  		return 1;
>  	}
>  
> +	if (opcode_4k != 0xFF)
> +		sfdp_add_uniform_eraser(flash, opcode_4k, 4 * 1024);
> +
>  	/* FIXME: double words 3-7 contain unused fast read information */
>  
>  	if (len == 4 * 4) {

How well-tested is this? I'd rather not have a fixup for the fixup.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/