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

From: Boris Brezillon
Date: Fri Apr 15 2022 - 03:01:18 EST


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.