Am 07.06.2017 um 18:19 schrieb Srinivas Kandagatla: > > On 04/06/17 12:06, Heiner Kallweit wrote: >> Add a device-managed version of nvmem_register. >> >> Signed-off-by: Heiner Kallweit <hkallwe...@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? > As stated in my answer to your other review comment: Currently no caller of nvmem_unregister checks the return code. Checking the refcount I see more as a debug feature and I think making nvmem_unregister return void plus a WARN() if refcount != 0 would be better.
Rgds, Heiner > >> +} >> + >> +/** >> + * 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); >> + >