Re: [PATCH v3 1/3] nvmem: core: allow to register cells during nvmem registration

[Vadym Kochan]

Hi Srinivas,

On Fri, Sep 04, 2020 at 12:02:40PM +0100, Srinivas Kandagatla wrote:
> Hi Vadym,
> 
> Thanks for the patch,
> On 31/08/2020 02:55, Vadym Kochan wrote:
> > Add NVMEM_PRE_ADD notification step which is called before any cells
> > binding - from lookup table or config, this allows to register cells
> > in some specific layout (tlv) which should be parsed first and then
> > registered. So there might be a cell parser driver which can register
> > lookup table during this notification step.
> > 
> This is going in right direction but totally not correct way to do it.
> 
> 1> this is not scalable as any consumer that will register for this even
> will have no idea of which what kind of parsing that provider needs.
> It can work in your case but not really useful.
> 
> 2> this is a consumer API, not the provider api.
> 
> How about adding a "parse_cells" callback in struct nvmem_config along with
> encoding type.
> 
> 
> thanks,
> srini
> 

Looks like I missed main point here that this cells parser should be
registered as nvmem provider. I will think on it.

Thanks,

> > Signed-off-by: Vadym Kochan <vadym.kochan@xxxxxxxxxxx>
> > ---
> > v3:
> >      1) Update core.c changes by extending notification mechanism
> >         by adding new NVMEM_PRE_ADD event id which is called before lookup
> >         table cells binding, this allows for notification handler to
> >         register cells which require nvmem parsing.
> > 
> >   drivers/nvmem/core.c           | 2 ++
> >   include/linux/nvmem-consumer.h | 1 +
> >   2 files changed, 3 insertions(+)
> > 
> > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > index 6cd3edb2eaf6..c48a69e0ebbe 100644
> > --- a/drivers/nvmem/core.c
> > +++ b/drivers/nvmem/core.c
> > @@ -668,6 +668,8 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
> >   			goto err_device_del;
> >   	}
> > +	blocking_notifier_call_chain(&nvmem_notifier, NVMEM_PRE_ADD, nvmem);
> > +
> >   	if (config->cells) {
> >   		rval = nvmem_add_cells(nvmem, config->cells, config->ncells);
> >   		if (rval)
> > diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> > index 052293f4cbdb..0f7107276756 100644
> > --- a/include/linux/nvmem-consumer.h
> > +++ b/include/linux/nvmem-consumer.h
> > @@ -50,6 +50,7 @@ enum {
> >   	NVMEM_REMOVE,
> >   	NVMEM_CELL_ADD,
> >   	NVMEM_CELL_REMOVE,
> > +	NVMEM_PRE_ADD,
> >   };
> >   #if IS_ENABLED(CONFIG_NVMEM)
> >