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

[Miquel Raynal]

Hi Rafał,

zajec5@xxxxxxxxx wrote on Thu, 9 Mar 2023 13:01:19 +0100:

> 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?

Yes!

> > - 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?

That's the theory, but then provider bindings should reflect the
changes as well in their own binding documents.

IOW, this one (and all its cousins) becomes legacy and must be updated:

https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/nvmem/imx-ocotp.yaml#L92

(and there are many files to update then)

> 
> 
> > - 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.

Well, you want to make the bindings evolve. Just updating a single
binding file is not enough, I believe we should at least provide full
support for the new description. But of course I'm fine discussing what
description we want first.

> 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  
> 


Thanks,
Miquèl