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

[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.
-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner