Re: [PATCH] nvmem: core: add managed version of nvmem_register

[Srinivas Kandagatla <srinivas.kandagatla@linaro.org>]

On 04/06/17 12:06, Heiner Kallweit wrote:
> Add a device-managed version of nvmem_register.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  Documentation/nvmem/nvmem.txt  |  1 +
>  drivers/nvmem/core.c           | 35 +++++++++++++++++++++++++++++++++++
>  include/linux/nvmem-provider.h |  7 +++++++
>  3 files changed, 43 insertions(+)
>

Thanks for the patch, one comments..
> diff --git a/Documentation/nvmem/nvmem.txt b/Documentation/nvmem/nvmem.txt
> index dbd40d87..b4ff7862 100644
> --- a/Documentation/nvmem/nvmem.txt
> +++ b/Documentation/nvmem/nvmem.txt
> @@ -37,6 +37,7 @@ and write the non-volatile memory.
>  A NVMEM provider can register with NVMEM core by supplying relevant
>  nvmem configuration to nvmem_register(), on success core would return a valid
>  nvmem_device pointer.
> +devm_nvmem_register() is a device-managed version of nvmem_register.
>
>  nvmem_unregister(nvmem) is used to unregister a previously registered provider.
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 783eb431..55db219f 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -531,6 +531,41 @@ int nvmem_unregister(struct nvmem_device *nvmem)
>  }
>  EXPORT_SYMBOL_GPL(nvmem_unregister);
>
> +static void devm_nvmem_release(struct device *dev, void *res)
> +{
> +	nvmem_unregister(*(struct nvmem_device **)res);

nvmem_unregister() can fail, how are you going to deal with this error 
cases?


> +}
> +
> +/**
> + * devm_nvmem_register() - managed version of nvmem_register
> + *
> + * @config: nvmem device configuration with which nvmem device is created.
> + *
> + * Return: Will be an ERR_PTR() on error or a valid pointer to nvmem_device
> + * on success.
> + */
> +
> +struct nvmem_device *devm_nvmem_register(const struct nvmem_config *config)

For consistency reasons, devm versions of apis should always have dev at 
as first argument.

> +{
> +	struct nvmem_device *nv, **dr;
> +
> +	dr = devres_alloc(devm_nvmem_release, sizeof(*dr), GFP_KERNEL);
> +	if (!dr)
> +		return ERR_PTR(-ENOMEM);
> +
> +	nv = nvmem_register(config);
> +	if (IS_ERR(nv)) {
> +		devres_free(dr);
> +		return nv;
> +	}
> +
> +	*dr = nv;
> +	devres_add(config->dev, dr);
> +
> +	return nv;
> +}
> +EXPORT_SYMBOL_GPL(devm_nvmem_register);
> +