Re: [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
>
> 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 "
> 	 "[e-mail]\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