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

[Vadym Kochan]

Hi Srinivas,

On Fri, Sep 04, 2020 at 02:23:10PM +0300, Vadym Kochan wrote:
> 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,
> 

I am trying to re-work this approach, but still I need to clarify
something.

It looks strange that this cells parser should be a nvmem provider (or I
missed something) but I remember that you suggested about introducing
something like nvmem parser. And adding nvmem parser looks more clear
for me, because what it should do is just access the nvmem device during
its registration and provide list of cells, that's all:

struct nvmem_device *nvmem_register(const struct nvmem_config *config)
{
    struct nvmem_cell_table table = { };
...
    parser = find_nvmem_parser();
    /* I think that cell lookups may be added on the parser's probe
       statically */
    parser->parse_cells(parser->priv, nvmem, &table);
...
}

/* here I used struct nvmem_config, not sure it is a right way to
   mix nvmem's and parser's struct fields, so may be something like
   struct nvmem_parser_config might be introduced or fill the struct
   nvmem_parser directly by the driver and pass it to the registration func */
struct nvmem_parser *nvmem_parser_register(const struct nvmem_config *config)
{
...
}

void nvmem_parser_unregister(struct nvmem_parser *parser)
{
...
}

Regards,
Vadym Kochan