Re: [PATCH v2 2/3] mtd: spinand: add support for detection with param page

[Boris Brezillon]

On Fri, 15 Apr 2022 11:48:43 +0800
Chuanhong Guo <gch981213@xxxxxxxxx> wrote:

> +
> +static const struct spinand_manufacturer *spinand_onfi_manufacturers[] = {};

Do we really need a separate manufacturer array? Looks like we could
re-use the one we have in core.c and do the matching against it (we
just need an extra NULL sentinel to detect the end of this array).

> +
> +static const struct spinand_onfi_info *
> +spinand_onfi_chip_match(struct nand_onfi_params *p,
> +			const struct spinand_manufacturer *m)
> +{
> +	size_t i, j;
> +
> +	for (i = 0; i < m->nchips; i++)
> +		for (j = 0; m->onfi_chips[i].models[j]; j++)
> +			if (!strcasecmp(m->onfi_chips[i].models[j], p->model))
> +				return &m->onfi_chips[i];
> +	return NULL;
> +}

> +/**
> + * struct spinand_onfi_info - Structure used to describe SPI NAND with ONFI
> + *			      parameter page
> + * @models: Model name array. Null terminated.
> + * @flags: OR-ing of the SPINAND_XXX flags
> + * @eccinfo: on-die ECC info
> + * @op_variants: operations variants
> + * @op_variants.read_cache: variants of the read-cache operation
> + * @op_variants.write_cache: variants of the write-cache operation
> + * @op_variants.update_cache: variants of the update-cache operation
> + * @select_target: function used to select a target/die. Required only for
> + *		   multi-die chips
> + *
> + * Each SPI NAND manufacturer driver should have a spinand_onfi_info table
> + * describing all the chips supported by the driver.
> + */
> +struct spinand_onfi_info {
> +	const char **const models;
> +	u32 flags;
> +	struct spinand_ecc_info eccinfo;
> +	struct {
> +		const struct spinand_op_variants *read_cache;
> +		const struct spinand_op_variants *write_cache;
> +		const struct spinand_op_variants *update_cache;
> +	} op_variants;
> +	int (*select_target)(struct spinand_device *spinand,
> +			     unsigned int target);
> +};

Can't we just extend spinand_info instead of defining a new struct.
AFAICT, the only difference is that model is replaced by a model array,
and devid is dropped, and I think we can rework the existing ID-based
matching logic to return ->models[0] instead of ->model.