Messages in this thread | | | From | Chuanhong Guo <> | Date | Fri, 15 Apr 2022 15:28:57 +0800 | Subject | Re: [PATCH v2 2/3] mtd: spinand: add support for detection with param page |
| |
Hi!
On Fri, Apr 15, 2022 at 3:01 PM Boris Brezillon <boris.brezillon@collabora.com> wrote: > +#define SPINAND_IDR_EN BIT(6)
Looks like you're redefining CFG_OTP_ENABLE, why not use that definition directly?
Oh. I didn't know that was available. I'll use it. (IDR_EN comes from a toshiba datasheet if I remember correctly.)
> > On Fri, 15 Apr 2022 11:48:43 +0800 > Chuanhong Guo <gch981213@gmail.com> 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).
I'll do it. I'll also add a onfi_nchips to spinand_manufacturer to share the same manufacturer entry between two types of ID list.
> > > + > > +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.
Yeah, that's better. I'll do it.
> 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.
In fact we don't even use this model string anywhere at the moment.
-- Regards, Chuanhong Guo
| |