[PATCH] rtc: cmos: Fix return value of nvmem callbacks

[PATCH] rtc: cmos: Fix return value of nvmem callbacks

[Joy Chakraborty]

Read/write callbacks registered with nvmem core expect 0 to be returned
on success and a negative value to be returned on failure.

cmos_nvram_read()/cmos_nvram_write() currently return the number of
bytes read or written, fix to return 0 on success and -EIO incase number
of bytes requested was not read or written.

Fixes: 8b5b7958fd1c ("rtc: cmos: use generic nvmem")
Cc: stable@vger.kernel.org
Signed-off-by: Joy Chakraborty <joychakr@google.com>
---
 drivers/rtc/rtc-cmos.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 7d99cd2c37a0..35dca2accbb8 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -643,11 +643,10 @@ static int cmos_nvram_read(void *priv, unsigned int off, void *val,
 			   size_t count)
 {
 	unsigned char *buf = val;
-	int	retval;
 
 	off += NVRAM_OFFSET;
 	spin_lock_irq(&rtc_lock);
-	for (retval = 0; count; count--, off++, retval++) {
+	for (; count; count--, off++) {
 		if (off < 128)
 			*buf++ = CMOS_READ(off);
 		else if (can_bank2)
@@ -657,7 +656,7 @@ static int cmos_nvram_read(void *priv, unsigned int off, void *val,
 	}
 	spin_unlock_irq(&rtc_lock);
 
-	return retval;
+	return count ? -EIO : 0;
 }
 
 static int cmos_nvram_write(void *priv, unsigned int off, void *val,
@@ -665,7 +664,6 @@ static int cmos_nvram_write(void *priv, unsigned int off, void *val,
 {
 	struct cmos_rtc	*cmos = priv;
 	unsigned char	*buf = val;
-	int		retval;
 
 	/* NOTE:  on at least PCs and Ataris, the boot firmware uses a
 	 * checksum on part of the NVRAM data.  That's currently ignored
@@ -674,7 +672,7 @@ static int cmos_nvram_write(void *priv, unsigned int off, void *val,
 	 */
 	off += NVRAM_OFFSET;
 	spin_lock_irq(&rtc_lock);
-	for (retval = 0; count; count--, off++, retval++) {
+	for (; count; count--, off++) {
 		/* don't trash RTC registers */
 		if (off == cmos->day_alrm
 				|| off == cmos->mon_alrm
@@ -689,7 +687,7 @@ static int cmos_nvram_write(void *priv, unsigned int off, void *val,
 	}
 	spin_unlock_irq(&rtc_lock);
 
-	return retval;
+	return count ? -EIO : 0;
 }
 
 /*----------------------------------------------------------------*/
-- 
2.45.2.505.gda0bf45e8d-goog

Re: [PATCH] rtc: cmos: Fix return value of nvmem callbacks

[Dan Carpenter]

On Wed, Jun 12, 2024 at 08:36:35AM +0000, Joy Chakraborty wrote:
> Read/write callbacks registered with nvmem core expect 0 to be returned
> on success and a negative value to be returned on failure.
> 
> cmos_nvram_read()/cmos_nvram_write() currently return the number of
> bytes read or written, fix to return 0 on success and -EIO incase number
> of bytes requested was not read or written.
> 
> Fixes: 8b5b7958fd1c ("rtc: cmos: use generic nvmem")
> Cc: stable@vger.kernel.org
> Signed-off-by: Joy Chakraborty <joychakr@google.com>
> ---

Thanks!

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

After we fix all the these, can we add a warning once message to detect
when people introduce new bugs?  It could either go into
__nvmem_reg_read/write() or bin_attr_nvmem_read/write().  I think
bin_attr_nvmem_read() is the only caller where the buggy functions work
but that's the caller that most people use I guess.

regards,
dan carpenter

Re: [PATCH] rtc: cmos: Fix return value of nvmem callbacks

[Joy Chakraborty]

On Wed, Jun 12, 2024 at 2:53 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Wed, Jun 12, 2024 at 08:36:35AM +0000, Joy Chakraborty wrote:
> > Read/write callbacks registered with nvmem core expect 0 to be returned
> > on success and a negative value to be returned on failure.
> >
> > cmos_nvram_read()/cmos_nvram_write() currently return the number of
> > bytes read or written, fix to return 0 on success and -EIO incase number
> > of bytes requested was not read or written.
> >
> > Fixes: 8b5b7958fd1c ("rtc: cmos: use generic nvmem")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Joy Chakraborty <joychakr@google.com>
> > ---
>
> Thanks!
>
> Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
>
> After we fix all the these, can we add a warning once message to detect
> when people introduce new bugs?  It could either go into
> __nvmem_reg_read/write() or bin_attr_nvmem_read/write().  I think
> bin_attr_nvmem_read() is the only caller where the buggy functions work
> but that's the caller that most people use I guess.
>

Sure I can do that.
Yes, I think most users use this via sysfs using bin_attr_nvmem_read()
hence it works.

Thanks
Joy

> regards,
> dan carpenter
>

Re: [PATCH] rtc: cmos: Fix return value of nvmem callbacks

[Srinivas Kandagatla]

On Wed, 12 Jun 2024 08:36:35 +0000, Joy Chakraborty wrote:
> Read/write callbacks registered with nvmem core expect 0 to be returned
> on success and a negative value to be returned on failure.
> 
> cmos_nvram_read()/cmos_nvram_write() currently return the number of
> bytes read or written, fix to return 0 on success and -EIO incase number
> of bytes requested was not read or written.
> 
> [...]

Applied, thanks!

[1/1] rtc: cmos: Fix return value of nvmem callbacks
      commit: ac9b2633e85f6b53cf368f90ae7589553e8998b6

Best regards,
-- 
Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Re: [PATCH] rtc: cmos: Fix return value of nvmem callbacks

[Alexandre Belloni]

On Wed, 12 Jun 2024 08:36:35 +0000, Joy Chakraborty wrote:
> Read/write callbacks registered with nvmem core expect 0 to be returned
> on success and a negative value to be returned on failure.
> 
> cmos_nvram_read()/cmos_nvram_write() currently return the number of
> bytes read or written, fix to return 0 on success and -EIO incase number
> of bytes requested was not read or written.
> 
> [...]

Applied, thanks!

[1/1] rtc: cmos: Fix return value of nvmem callbacks
      https://git.kernel.org/abelloni/c/1c184baccf0d

Best regards,

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com