Re: flash bbt broken due to unitialized bitflip_threshold?

[Shmulik Ladkani <shmulik.ladkani@gmail.com>]

Hi Artem,

On Wed, 06 Jun 2012 16:30:53 +0300 Artem Bityutskiy <artem.bityutskiy@linux.intel.com> wrote:
> On Wed, 2012-06-06 at 12:50 +0300, Shmulik Ladkani wrote:
> > +	if (!mtd->bitflip_threshold)
> > +		mtd->bitflip_threshold = mtd->ecc_strength;
> 
> Hmm, why se default is to report bit-flips only when one more flipping
> bit would cause an ECC error? The default be 1 - report on any bit-flip.
> The same should be done in 'add_mtd_device()' - we should preserve the
> old behavior, as it was before these patches. Do I miss something?

Mike's patchset modified behavior of mtd_read() to return EUCLEAN only
if max number of bitflips (per ecc step) exceeds the bitflip_threshold:

	return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0;

mtd->bitflip_threshold can be set in the following ways:
- By the lowlevel driver
- By 'add_mtd_device'. Here it defaults to 'ecc_strength' if NOT
  previously set by the driver.
- Using sysfs attribute

Currently, no driver explicitly sets bitflip_threshold, so it defaults
to ecc_strength. This is not necessarily 1; it depends on the hardware
driver, or software algorithm, etc...

So the "old behavior" was not preserved by the patchset, at least for
those setups with ecc_strength greater than 1. And I don't think the
intention was to preserve 'bitflip_threshold' as 1.

As Sascha spotted, the patchset created a problem for 'scan_bbt'.
This is since 'scan_bbt' calls 'mtd_read()' to read the BBT pages,
however 'bitflip_threshold' is usually NOT YET assigned at that point,
which results in mtd_read's condition (quoted above) to ALWAYS
return -EUCLEAN.
This leads to constantly scrubbing the BBT. Ouch.

My suggestion is to assign the default value of 'ecc_strength' to
'bitflip_threshold' at the end of 'nand_scan_tail', PRIOR the call to
scan_bbt().
Hence, BBT will be scrubbed only if maximum bitflips exceeded the
'ecc_strength' of this mtd (which is the default behavior).

Does it make sense to you?

Or would you like to scrub the BBT upon every bitflip, regardless
ecc_strength?

Regards,
Shmulik