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

[Srinivas Kandagatla]

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

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