Re: [PATCH V3] nvmem: add explicit config option to read OF fixed cells

[Rafał Miłecki]

On 9.03.2023 12:35, Miquel Raynal wrote:
> > diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
> > index 0262b86194eb..b3c14ce87a65 100644
> > --- a/include/linux/nvmem-provider.h
> > +++ b/include/linux/nvmem-provider.h
> > @@ -73,6 +73,7 @@ struct nvmem_cell_info {
> >    * @owner:	Pointer to exporter module. Used for refcounting.
> >    * @cells:	Optional array of pre-defined NVMEM cells.
> >    * @ncells:	Number of elements in cells.
> > + * @use_fixed_of_cells:	Read fixed NVMEM cells from OF.
>
> I'm still unhappy with the naming, especially since you explained in
> more details the whole plan which involves using a container to put
> these fixed cells from now on. In both cases you extract cells from
> fixed OF nodes but this boolean needs to be set to true in one
> case, and false in the other, which would not make sense.
>
> Also, regarding the bindings changes, I'm fairly happy with the idea,
> but if we go this way I would prefer a full series instead of
> individual changes with:
>
> - the boolean you introduce here (renamed, at the very least)
> - the new bindings

I assume you mean fixed-layout.yaml?


> - the update of the current provider bindings to take the new bindings
>    into account and deprecate the old ones officially

What has to be updated in current proceds? It seems to me that:
1. Current NVMEM providers reference nvmem.yaml
2. nvmem.yaml references nvmem-layout.yaml
3. nvmem-layout.yaml references fixed-layout.yaml

what else is missing?


> - support for the new bindings in the core

Please, don't get me wrong, but I'm not going to spend more hours on
actual coding without approval of chosen path.

I'll need to have
[PATCH V2] dt-bindings: nvmem: layouts: add fixed-layout
reviewed/acked first. If you can do that that's great.


> >    * @keepout:	Optional array of keepout ranges (sorted ascending by start).
> >    * @nkeepout:	Number of elements in the keepout array.
> >    * @type:	Type of the nvmem storage
> > @@ -103,6 +104,7 @@ struct nvmem_config {
> >   	struct module		*owner;
> >   	const struct nvmem_cell_info	*cells;
> >   	int			ncells;
> > +	bool			use_fixed_of_cells;
> >   	const struct nvmem_keepout *keepout;
> >   	unsigned int		nkeepout;
> >   	enum nvmem_type		type;
>
> Thanks,
> Miquèl