Re: [PATCH v2 4/6] nand: spi: Add BBT support

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Thu, 9 Mar 2017 09:58:51 +0800
Peter Pan <peterpansjtu@gmail.com> wrote:

> Hi Boris,
> 
> On Wed, Mar 8, 2017 at 4:31 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Wed, 1 Mar 2017 16:52:08 +0800
> > Peter Pan <peterpandong@micron.com> wrote:
> >  
> >>  /**
> >> - * spinand_erase - [MTD Interface] erase block(s)
> >> + * __spinand_erase - erase block(s)
> >>   * @mtd: MTD device structure
> >>   * @einfo: erase instruction
> >> + * @allowbbt: allow to access bbt
> >>   *
> >>   * Erase one ore more blocks
> >>   */
> >> -static int spinand_erase(struct mtd_info *mtd, struct erase_info *einfo)
> >> +static int __spinand_erase(struct mtd_info *mtd, struct erase_info *einfo,
> >> +                     int allowbbt)  
> >
> > Please no __ prefixes. Not sure you need the wrappers you define below
> > BTW.  
> 
> mtd->_erase needs a function to treat BBT block as bad block while
> nand_ops->erase not. What's your suggestion if we don't use the wrappers?

Oh, I forgot about that. Then keep the wrapper, but find a better name
for this function.

> 
> >  
> >>  {
> >>       struct spinand_device *chip = mtd_to_spinand(mtd);
> >>       struct nand_device *nand = mtd_to_nand(mtd);
> >> @@ -1152,7 +1185,7 @@ static int spinand_erase(struct mtd_info *mtd, struct erase_info *einfo)
> >>
> >>       while (len) {
> >>               /* Check if we have a bad block, we do not erase bad blocks! */
> >> -             if (spinand_block_bad(mtd, offs, 0)) {
> >> +             if (spinand_block_checkbad(mtd, offs, 0, allowbbt)) {
> >>                       pr_warn("%s: attempt to erase a bad block at 0x%012llx\n",
> >>                       __func__, offs);
> >>                       einfo->state = MTD_ERASE_FAILED;
> >> @@ -1193,6 +1226,55 @@ static int spinand_erase(struct mtd_info *mtd, struct erase_info *einfo)
> >>       /* Return more or less happy */
> >>       return ret;
> >>  }
> >> +
> >> +/**
> >> + * spinand_erase - [MTD Interface] erase block(s)
> >> + * @mtd: MTD device structure
> >> + * @einfo: erase instruction
> >> + *
> >> + * Erase one ore more blocks
> >> + */
> >> +static int spinand_erase(struct mtd_info *mtd, struct erase_info *einfo)

Or maybe you can rename this function spinand_erase_skip_bbt(), and
keep spinand_erase() for the generic one.

> >> +{
> >> +     return __spinand_erase(mtd, einfo, 0);
> >> +}
> >> +
> >> +

Remove this extra empty line.

> >> +static int spinand_erase_bbt(struct nand_device *nand, struct erase_info *einfo)
> >> +{
> >> +     return __spinand_erase(nand_to_mtd(nand), einfo, 1);
> >> +}
> >> +  
> >