[PATCH v3] mtd: gpmi: fix the bitflips for erased page

[Huang Shijie]

On Mon, Jan 13, 2014 at 08:20:24AM +0100, Elie De Brauwer wrote:
> On Mon, Jan 13, 2014 at 3:54 AM, Huang Shijie <b32955 at freescale.com> wrote:
> > +       /* Read out the page without ECC enabled, and check it again */
> > +       chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
> > +       chip->read_buf(mtd, (uint8_t *)buf, mtd->writesize);
> > +
> > +       /* Count the bitflips for the no ECC buffer */
> > +       for (i = 0; i < mtd->writesize / 8; i++)
> > +               flip_bits_noecc += hweight64(~buf[i]);
> 
> Is it safe to assume that all these chunks will be 8-byte-aligned ? I
> also thought about this optimization but didn't find any indication
> that this might be safe.

Firstly the page size is 8-tyte-aligned. do you meet a nand which is not
8-byte-aligned?

Secondely, even the page size is not 8-byte-aligned, it is okay too.
why? the gpmi will split the nand layout as:
    +---+----------+-+----------+-+----------+-+----------+-+-----+
    | M |   data   |E|   data   |E|   data   |E|   data   |E|     |
    +---+----------+-+----------+-+----------+-+----------+-+-----+

    E stands for the ECC parity area.

For the page contains the all 0xff data, the "E" is not 0xff, and 
we can get the enough bitflips from only one "E" area.


> 
> > +
> > +       if (flip_bits_noecc <= threshold) {
> > +               /* Tell the upper layer the bitflips we corrected. */
> > +               *max_bitflips = max_t(unsigned int, *max_bitflips, flip_bits);
> > +
> > +               /*
> > +                * The geo->payload_size maybe not equal to the page size
> > +                * when the Subpage-Read is enabled.
> > +                */
> > +               memset(data, 0xff, geo->payload_size);
> > +               ret = true;
> > +       }
> > +       dev_dbg(this->dev, "The page(%d) %s an erased page(%d,%d,%d,%d).\n",
> > +                       page, ret ? "is" : "isn't", chunk, threshold,
> > +                       flip_bits, flip_bits_noecc);
> > +       return ret;
> > +}
> > +
> >  static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> >                                 uint8_t *buf, int oob_required, int page)
> >  {
> > @@ -1007,6 +1057,10 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> >                         continue;
> >
> >                 if (*status == STATUS_UNCORRECTABLE) {
> > +                       if (gpmi_erased_check(this, payload_virt, i,
> > +                                               page, &max_bitflips))
> > +                               break;
> > +
> >                         mtd->ecc_stats.failed++;
> >                         continue;
> >                 }
> > --
> > 1.7.2.rc3
> >
> 
> A second note, you patch v2 is a lie, there in the v1->v2 you mention
> you return the number of bitflips to the upper layer, however your
> code was not modified accordingly, gpmi_erase_check still does not
> return the number of bitflips. Unless I'm missing something there too.
I added a new argument @max_bitflips for gpmi_erased_check which will
return the number of bitflips.

thanks
Huang Shijie