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

[Thomas Petazzoni <thomas.petazzoni@free-electrons.com>]

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.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com