[PATCH 0/3] MTD: Change meaning of -EUCLEAN return code on reads

[Shmulik Ladkani]

Hi Mike,

On Sat, 17 Mar 2012 13:18:04 -0700 Mike Dunn <mikedunn at newsguy.com> wrote:
> So if we're all convinced that thresholds must be evaluated at the ecc.size
> level, there's more work <sigh>.  As Ivan pointed out earlier, the nand drivers
> currently do not track bitflip corrections per ecc step, which means all the
> nand drivers must be modified.  Before I start on this, I'd like to get a
> blessing from Artem / David, since the chamges will touch many drivers in a
> somehat non-trivial manner.

Well the change would affect more places, that's for sure.
However it could be a trivial change; see below.

> I think the best approach is to have the nand drivers return max_bitflips over
> all ecc steps, same idea as what the previous patches returned to mtd, but at a
> finer granularity.  Then the nand code just passes this up to mtd, instead of
> examining the ecc_stats as previously.

Please consider the following scheme:

(1) 'ecc.read_page' interface is changed.
  The return value will denote max bitflips over all ecc steps.
  (currently all implementations return 0)
  Please examine nand_read_page_swecc/nand_read_page_hwecc (and other
  implementations) and observe how easy this can be implemeted.

  Downside: we'll have to do this (max calculation) for each 'read_page'
  implementation.
  There are about ~10 implementations.
  (BTW most implementations are SO similar, maybe the code can be
  consolidated?)

(2) Modify nand_base's 'nand_do_read_ops', similary to what you've
  already done, to aggregate the maximum bitflips according to the
  returned values of the 'chip->ecc.read_page()' calls.
  Meaning, instead of your aggregation:
+	max_bitflips = max(max_bitflips, mtd->ecc_stats.corrected - prev_corrected);
  let's use:
+	max_bitflips = max(max_bitflips, read_page_returned_max_bitflips);

(3)  Finally, 'nand_do_read_ops' may return -EUCLEAN if max_bitflips is
  above the threshold:
-	return  mtd->ecc_stats.corrected - stats.corrected ? -EUCLEAN : 0;
+	return  max_bitflips >= mtd->bitflip_threshold ? -EUCLEAN : 0;

  [ BTW, at a side note, I failed to understand why you prefer the
  threshold comparison at the generic 'mtd_read' wrapper, and not within
  nand_base.c.
  I guess you didn't want to duplicate the condition into onenand_base.c
  and alauda.c?
  Thing is, I feel mtd->bitflip_threshold is a NAND property, so it
  makes more sense if it is tested within the NAND infrastricture (and
  clones).
  Changing the 'mtd->_read' interface was less elegant IMO. ]

Obvisouly, the above said is 30000 feet high, cutting a few corners :-)
We must let the code talk.

The only immediate blocker for such a scheme would be those nand
controller drivers, where the controller is responsible of the ECC
correction, and it does not report per-step stats to the software.
Are there any?
(In that case, we have nothing else to do besides falling back to a
per-page decision. For example, such driver's 'read_page' may return
total_corrected_per_page/ecc.steps as an estimate of the number of
per-step corrected bits, or alike)

Regards,
Shmulik