[PATCH v2 1/6] nand: spi: Add init/release function

[Peter Pan]

Hi Thomas,

On Wed, Mar 1, 2017 at 9:21 PM, Thomas Petazzoni
<thomas.petazzoni at free-electrons.com> wrote:
> Hello,
>
> On Wed, 1 Mar 2017 16:52:05 +0800, Peter Pan wrote:
>
>> +static bool spinand_scan_id_table(struct spinand_device *chip, u8 *id)
>> +{
>> +     struct nand_device *nand = &chip->base;
>> +     struct spinand_flash *type = spinand_table;
>> +     struct nand_memory_organization *memorg = &nand->memorg;
>> +     struct spinand_ecc_engine *ecc_engine = &chip->ecc_engine;
>> +
>> +     for (; type->name; type++) {
>> +             if (id[0] == type->mfr_id && id[1] == type->dev_id) {
>> +                     chip->name = type->name;
>> +                     memorg->eraseblocksize = type->page_size
>> +                                     * type->pages_per_blk;
>> +                     memorg->pagesize = type->page_size;
>> +                     memorg->oobsize = type->oob_size;
>> +                     memorg->diesize =
>> +                             memorg->eraseblocksize * type->blks_per_lun;
>> +                     memorg->ndies = type->luns_per_chip;
>> +                     ecc_engine->strength = type->ecc_strength;
>> +                     chip->rw_mode = type->rw_mode;
>> +
>> +                     return true;
>> +             }
>> +     }
>
> One minor nit, based on a very quick look. What about instead:
>
>         for (type = spinand_table; type->name; type++) {
>                 if (id[0] != type->mfr_id)
>                         continue;
>                 if (id[1] != type->dev_id)
>                         continue;
>                 chip->name = type->name;
>                 ...
>                 return true;
>         }
>
> I.e, use the initialization part of the for() loop, and inside the
> for() loop, avoid one indentation level by handling the non-interesting
> case first, and skipping to the next table entry if needed.

Thanks for your comment. I will fix this in v3

Thanks,
Peter Pan