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

[Carl-Daniel Hailfinger]

Am 21.02.2012 18:04 schrieb Stefan Tauner:
> On Tue, 21 Feb 2012 14:51:38 +0100
> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
>
>> Please use "Fix SFDP probing" as commit message headline and commit the
>> whole batch of SFDP bugfixes in one commit once the patch is fixed.
>>
>> Am 20.02.2012 22:29 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.
>>>
>>> Postpone adding an erase function for the special 4k block erase
>>> opcode until we know the flash chip size.
>>> Check for zero chip size in sfdp_add_uniform_eraser.
>>>
>>> Fix the output of the parameter table contents.
>>>
>>> This patch fixes the index used to retrieve the eraser types, which
>>> was off one double word.
>> For me it looks like this part of the patch introduced a new bug.
>>
>>> -	dw = 8;
>>> +	/* 8. double word */
>>> +	dw = 7;
>>>  	for (j = 0; j < 4; j++) {
>>> -		/* 8 double words from the start + 2 words for every eraser */
>>> +		/* 8 double words from the start + 2 bytes for every eraser */
>>>  		tmp8 = buf[(4 * dw) + (2 * j)];
>> 8 double words from the start is 32 bytes from the start. Last time I
>> checked, 7*4 was not 32.
> Xth dw starts at (x - 1) * 4 because they start from offset 0 with the
> *first* dw.
> the second dw starts at (2 - 1) * 4 = 4,
> 3rd at (3 - 1) * 4 = 8,
> …
>
> that mistake was exactly what haunted me back then apparently. :)
>
> the standard talks about (1-based) Xth double words, so i kept that
> nomenclature, but used a 0-based offset (dw). we could leave the
> existing solution, change the nomenclature (bad idea imho) or use
> 1-based offsets and subtract 1 before using them... no ideal solution
> available imo.

There is a huge difference between "8 double words from the start" and
"8th double word". One of those comments is wrong, and if your
explanation is correct, the "8 double words from the start" is wrong.

Regards,
Carl-Daniel

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