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: [email protected]
Signed-off-by: Joy Chakraborty <[email protected]>
---
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
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: [email protected]
> Signed-off-by: Joy Chakraborty <[email protected]>
> ---
Thanks!
Reviewed-by: Dan Carpenter <[email protected]>
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
On Wed, Jun 12, 2024 at 2:53 PM Dan Carpenter <[email protected]> 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: [email protected]
> > Signed-off-by: Joy Chakraborty <[email protected]>
> > ---
>
> Thanks!
>
> Reviewed-by: Dan Carpenter <[email protected]>
>
> 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
>