Re: [PATCH 1/2] mtd: brcmnand: Add check for erased page bitflips

[Brian Norris <computersforpeace@gmail.com>]

On Wed, Jun 01, 2016 at 12:46:18PM -0400, Kamal Dasu wrote:
> On Mon, May 30, 2016 at 4:42 AM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Fri, 29 Apr 2016 16:21:24 -0400
> > Kamal Dasu <kdasu.kdev@gmail.com> wrote:

> >> +     if (ret)
> >> +             return ret;
> >> +
> >> +     for (i = 0; i < chip->ecc.steps; i++, oob += sas) {
> >> +             unsigned int bitflips = 0;
> >> +
> >> +             bitflips += oob_nbits - bitmap_weight(oob, oob_nbits);
> >> +             bitflips += data_nbits - bitmap_weight(buf, data_nbits);
> >> +
> >> +             buf += chip->ecc.size;
> >> +             addr += chip->ecc.size;
> >
> > You seem to duplicate nand_check_erased_ecc_chunk() here. Do you have a
> > good reason for doing that?
> >
> 
> Hmmm I see what you are saying. Let me try setting the
> NAND_ECC_GENERIC_ERASED_CHECK option and see if we can get away
> without having to read raw.  I will have to test and make sure on
> uncorrectable error the hw leaves the return page data buffers and oob
> buffers in raw state.

I'm quite sure you can't make use of NAND_ECC_GENERIC_ERASED_CHECK
unless you do a substantial rewrite; brcmnand doesn't use any of the
nand_base ecc.read_{page,subpage} callbacks.

> If that works as expected I will get rid of this duplication and send
> a revised change which shall make use of the
> NAND_ECC_GENERIC_ERASED_CHECK option.

I suspect he was just suggesting calling the
nand_check_erased_ecc_chunk() helper instead of doing your own
bitmap_weight() calls.

Brian