[PATCH] mtd: nand: Add support for Micron on-die ECC controller.

[David Mosberger]

Gerhard, thanks for your feedback!

On Wed, Mar 26, 2014 at 3:13 PM, Gerhard Sittig <gsi at denx.de> wrote:
> On Fri, 2014-03-21 at 22:03 -0600, David Mosberger wrote:

>> +     if (status & NAND_STATUS_FAIL)
>> +             /* Page has invalid ECC. */
>> +             mtd->ecc_stats.failed++;
>> +     else if (status & NAND_STATUS_REWRITE) {
>
> coding style, there should be braces around all arms if one of
> them has some

Sure, I changed that.

>> +     return max_bitflips;
>> +}
>
> so the check_read_status_on_die() routine returns the number of
> bit flips (if they could be corrected)

Yes.

> are you certain about this non-zero return code from the
> chip->ecc.read_subpage() callback?  IIUC this will lead to fatal
> errors in nand_do_read_ops() -- am I missing something?

Yes, I'm certain.  See the documentation for include/mtd/nand.h
read_page and read_subpage (also, the other read_page/read_sub_page
functions have the same behavior).

I think the part that's easy to miss is that the mtd core (e.g.,
mtdcore.c:mtd_read) converts positive return-values into either
-EUCLEAN or 0, as needed).

Needless to say, we also tested this quite extensively (though that
was with earlier kernels).

>> +     if ((*chip->onfi_get_features)(mtd, chip, 0x90, features) >= 0) {
>> +             if (features[0] & 0x08) {
>
> if .onfi_get_features already is a function pointer, you need not
> dereference it before calling the routine (should apply elsewhere
> in the patch as well)

That's really a style issue.  Personally, I prefer to be explicit
about dereferencing a pointer (which is what's happening here), but
yes, I should remember that the Linux convention is to not do that.  I
changed these.

> want to replace those magic numbers with symbolic identifiers?
> especially when they get referenced from multiple locations

Sure, done.

I'll send an updated patch shortly.

Best regards,

  --david
-- 
eGauge Systems LLC, http://egauge.net/, 1.877-EGAUGE1, fax 720.545.9768