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

[Peter Pan <peterpansjtu@gmail.com>]

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?

>
>>  {
>>       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)
>> +{
>> +     return __spinand_erase(mtd, einfo, 0);
>> +}
>> +
>> +
>> +static int spinand_erase_bbt(struct nand_device *nand, struct erase_info *einfo)
>> +{
>> +     return __spinand_erase(nand_to_mtd(nand), einfo, 1);
>> +}
>> +
>
> [...]
>
>
>> +static void spinand_fill_nandd(struct spinand_device *chip)
>> +{
>> +     struct nand_device *nand = &chip->base;
>> +
>> +     nand->ops = &spinand_ops;
>> +}
>
> This is not needed. I did that in the raw NAND framework to ease the
> transition to the generic NAND layer, but here, you're
> natively supporting it. Just assign nand->ops in spinand_scan_ident().

Noted/ Fix this in v3.

Thanks
Peter Pan