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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200904112310.GD10654@plvision.eu>
Date:   Fri, 4 Sep 2020 14:23:10 +0300
From:   Vadym Kochan <vadym.kochan@...ision.eu>
To:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Cc:     Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Maxime Ripard <maxime.ripard@...e-electrons.com>,
        linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/3] nvmem: core: allow to register cells during nvmem
 registration

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@...ision.eu>
> > ---
> > 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)
> >