[PATCH] nvmem: core: WARN_ONCE on nvmem reg_read/write() returning positive values

[PATCH] nvmem: core: WARN_ONCE on nvmem reg_read/write() returning positive values

[Joy Chakraborty]

Nvmem core currently expects 0 to be returned on successful read/write
and negative for failure from nvmem producers.
Warn incase any nvmem producer returns positive values which might
happen if any producer ends up returning the number of bytes read or
written.

Signed-off-by: Joy Chakraborty <joychakr@google.com>
---
 drivers/nvmem/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index e1ec3b7200d7..0a42e6a0e8bb 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -58,10 +58,13 @@ static BLOCKING_NOTIFIER_HEAD(nvmem_notifier);
 static int __nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
 			    void *val, size_t bytes)
 {
+	int ret = -EINVAL;
+
 	if (nvmem->reg_read)
-		return nvmem->reg_read(nvmem->priv, offset, val, bytes);
+		ret = nvmem->reg_read(nvmem->priv, offset, val, bytes);
 
-	return -EINVAL;
+	WARN_ONCE(ret > 0, "nvmem reg_read should not return positive value, please return 0 on success");
+	return ret;
 }
 
 static int __nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
@@ -73,6 +76,7 @@ static int __nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
 		gpiod_set_value_cansleep(nvmem->wp_gpio, 0);
 		ret = nvmem->reg_write(nvmem->priv, offset, val, bytes);
 		gpiod_set_value_cansleep(nvmem->wp_gpio, 1);
+		WARN_ONCE(ret > 0, "nvmem reg_write should not return positive value, please return 0 on success");
 		return ret;
 	}
 
-- 
2.45.2.1089.g2a221341d9-goog

Re: [PATCH] nvmem: core: WARN_ONCE on nvmem reg_read/write() returning positive values

[Dan Carpenter]

On Thu, Jul 25, 2024 at 11:21:26AM +0000, Joy Chakraborty wrote:
> Nvmem core currently expects 0 to be returned on successful read/write
> and negative for failure from nvmem producers.
> Warn incase any nvmem producer returns positive values which might
> happen if any producer ends up returning the number of bytes read or
> written.
> 
> Signed-off-by: Joy Chakraborty <joychakr@google.com>

Thanks for fixing all these.  I've double checked and I believe you did
catch them all.  Plus if this patch triggers any warnings, then it will
be annoying but it's better to know about the bugs instead of running
into subtle issues down the road which is what the previous code did.

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter