From: Bartosz Golaszewski bgolaszewski@baylibre.com
blocking_notifier_call_chain() returns the value returned by the last registered callback. A positive return value doesn't indicate an error so check only if it's negative.
Fixes: bee1138bea15 ("nvmem: add a notifier chain") Cc: stable@vger.kernel.org Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com --- drivers/nvmem/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c index f7301bb4ef3b..a3bed2d9aec7 100644 --- a/drivers/nvmem/core.c +++ b/drivers/nvmem/core.c @@ -687,7 +687,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) goto err_remove_cells;
rval = blocking_notifier_call_chain(&nvmem_notifier, NVMEM_ADD, nvmem); - if (rval) + if (rval < 0) goto err_remove_cells;
return nvmem;
On 14/02/2019 16:23, Bartosz Golaszewski wrote:
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c index f7301bb4ef3b..a3bed2d9aec7 100644 --- a/drivers/nvmem/core.c +++ b/drivers/nvmem/core.c @@ -687,7 +687,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) goto err_remove_cells; rval = blocking_notifier_call_chain(&nvmem_notifier, NVMEM_ADD, nvmem);
- if (rval)
- if (rval < 0) goto err_remove_cells;
rval will be masked with STOP MASK, so the above statement could be false even if we have error. So you should consider returning an errono which can be understood by user:
may be something like this:
if (rval & NOTIFY_STOP_MASK) { rval = notifier_to_errno(rval); goto err_remove_cells }
--srini
pt., 15 lut 2019 o 10:28 Srinivas Kandagatla srinivas.kandagatla@linaro.org napisał(a):
On 14/02/2019 16:23, Bartosz Golaszewski wrote:
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c index f7301bb4ef3b..a3bed2d9aec7 100644 --- a/drivers/nvmem/core.c +++ b/drivers/nvmem/core.c @@ -687,7 +687,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) goto err_remove_cells;
rval = blocking_notifier_call_chain(&nvmem_notifier, NVMEM_ADD, nvmem);
if (rval)
if (rval < 0) goto err_remove_cells;rval will be masked with STOP MASK, so the above statement could be false even if we have error. So you should consider returning an errono which can be understood by user:
may be something like this:
if (rval & NOTIFY_STOP_MASK) { rval = notifier_to_errno(rval); goto err_remove_cells }
Actually I'm now thinking we can remove this check at all - most users never check the return values of notifier chain calls. This function cannot fail in itself. What do you think?
Bart
On 15/02/2019 09:41, Bartosz Golaszewski wrote:
rval will be masked with STOP MASK, so the above statement could be false even if we have error. So you should consider returning an errono which can be understood by user:
may be something like this:
if (rval & NOTIFY_STOP_MASK) { rval = notifier_to_errno(rval); goto err_remove_cells }
Actually I'm now thinking we can remove this check at all - most users never check the return values of notifier chain calls. This function cannot fail in itself. What do you think?
Thats even better, I was about to suggest the same on the fact that we should allow nvmem provider to register to be successful irrespective of the notifier callback failures.
--srini
pt., 15 lut 2019 o 11:26 Srinivas Kandagatla srinivas.kandagatla@linaro.org napisał(a):
On 15/02/2019 09:41, Bartosz Golaszewski wrote:
rval will be masked with STOP MASK, so the above statement could be false even if we have error. So you should consider returning an errono which can be understood by user:
may be something like this:
if (rval & NOTIFY_STOP_MASK) { rval = notifier_to_errno(rval); goto err_remove_cells }
Actually I'm now thinking we can remove this check at all - most users never check the return values of notifier chain calls. This function cannot fail in itself. What do you think?
Thats even better, I was about to suggest the same on the fact that we should allow nvmem provider to register to be successful irrespective of the notifier callback failures.
--srini
Right, I sent a different patch.
Bart
linux-stable-mirror@lists.linaro.org
-
Bartosz Golaszewski -
Srinivas Kandagatla