Am 07.06.2017 um 17:30 schrieb Srinivas Kandagatla:
Thanks for the quick review.
On 04/06/17 12:01, Heiner Kallweit wrote:
Member users is used only to check whether we're allowed to remove
the module. So in case of built-in it's not used at all and in case
nvmem providers doesn't have to be independent drivers, providers could be part of the other driver which can dynamically register and unregister nvmem providers. For example at24 and at25 drivers.
This patch will break such cases !!
I don't think this patch breaks e.g. at24 / at25. Let me try to explain:
at24 / at25 set themself as owner in struct nvmem_device and nvmem_unregister
is called from at24/25_remove only. These remove callbacks are called only if
all references to the respective module have been released.
In current kernel code I don't see any nvmem use broken by the proposed patch.
However in general you're right, there may be future use cases where
nvmem_unregister isn't called only from a remove callback.
If the refcount isn't zero when calling nvmem_unregister then there's a bigger
problem, I don't think there's any normal use case where this can happen.
Instead of just returning -EBUSY I think a WARN() would be appropriate.
Currently no caller of nvmem_unregister checks the return code anyway.
My opinion would be that the refcount here is more a debug feature.
Whilst we're talking about nvmem_unregister:
I think the device_del() at the end should be a device_unregister().
Else we miss put_device as second part of destroying a device.
Rgds, Heiner
that owner is a module we have the module refcount for the same
purpose already. Whenever users is incremented the owner's refcount
is incremented too. Therefore users isn't needed.
Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
---
drivers/nvmem/core.c | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 8c830a80..4e07f3f8 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -33,7 +33,6 @@ struct nvmem_device {
int word_size;
int ncells;
int id;
- int users;
size_t size;
bool read_only;
int flags;
@@ -517,13 +516,6 @@ EXPORT_SYMBOL_GPL(nvmem_register);
*/
int nvmem_unregister(struct nvmem_device *nvmem)
{
- mutex_lock(&nvmem_mutex);
- if (nvmem->users) {
- mutex_unlock(&nvmem_mutex);
- return -EBUSY;
- }
- mutex_unlock(&nvmem_mutex);
-
if (nvmem->flags & FLAG_COMPAT)
device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
@@ -562,7 +554,6 @@ static struct nvmem_device *__nvmem_device_get(struct device_node *np,
}
}
- nvmem->users++;
mutex_unlock(&nvmem_mutex);
if (!try_module_get(nvmem->owner)) {
@@ -570,10 +561,6 @@ static struct nvmem_device *__nvmem_device_get(struct device_node *np,
"could not increase module refcount for cell %s\n",
nvmem->name);
- mutex_lock(&nvmem_mutex);
- nvmem->users--;
- mutex_unlock(&nvmem_mutex);
-
return ERR_PTR(-EINVAL);
}
@@ -583,9 +570,6 @@ static struct nvmem_device *__nvmem_device_get(struct device_node *np,
static void __nvmem_device_put(struct nvmem_device *nvmem)
{
module_put(nvmem->owner);
- mutex_lock(&nvmem_mutex);
- nvmem->users--;
- mutex_unlock(&nvmem_mutex);
}
static int nvmem_match(struct device *dev, void *data)