Re: [PATCH 05/12] mtd: nand: remove AG-AND support

[Artem Bityutskiy <dedekind1@gmail.com>]

On Mon, 2013-03-04 at 10:56 -0800, Brian Norris wrote:
> On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> >
> > We have only one AG-AND driver and it was not touched since 2005. It looks
> > like AG-AND was not really make it to mass-production and can be considered
> > a dead technology.
> 
> You might want to include in your commit message that BBT_AUTO_REFRESH
> was only needed for AG-AND. You drop it here (which is perfectly
> reasonable) but it is technically provided as a generic feature which
> *could* be used outside of AG-AND.

OK, will do, thanks!

> > @@ -22,7 +21,6 @@
> >   *     Enable cached programming for 2k page size chips
> >   *     Check, if mtd->ecctype should be set to MTD_ECC_HW
> >   *     if we have HW ECC support.
> > - *     The AG-AND chips have nice features for speed improvement,
> >   *     which are not supported yet. Read / program 4 pages in one go.
> >   *     BBT table is not serialized, has to be fixed
> >   *
> 
> You cut this off mid-sentence. Did you mean to cut three lines here,
> instead of just one?

Oops.

> > -       if ((state == FL_ERASING) && (chip->options & NAND_IS_AND))
> > -               chip->cmdfunc(mtd, NAND_CMD_STATUS_MULTI, -1, -1);
> > -       else
> > +       if (state == FL_ERASING)
> >                 chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
> 
> This is not a precise refactor. All non-AND flash would previously
> have run the STATUS command, but now you make it only run when
> FL_ERASING. Shouldn't it just be an unconditional cmdfunc(STATUS)?
 
OOPS!

-- 
Best Regards,
Artem Bityutskiy