On Wed, Jun 12, 2024 at 06:05:54PM +0000, Joy Chakraborty wrote: > Read callbacks registered with nvmem core expect 0 to be returned on > success and a negative value to be returned on failure. > > abx80x_nvmem_xfer() on read calls i2c_smbus_read_i2c_block_data() which > returns the number of bytes read on success as per its api description, > this return value is handled as an error and returned to nvmem even on > success. > > Fix to handle all possible values that would be returned by > i2c_smbus_read_i2c_block_data(). > > Fixes: e90ff8ede777 ("rtc: abx80x: Add nvmem support") > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Joy Chakraborty <joychakr@xxxxxxxxxx> > --- > drivers/rtc/rtc-abx80x.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c > index fde2b8054c2e..0f5847d1ca2a 100644 > --- a/drivers/rtc/rtc-abx80x.c > +++ b/drivers/rtc/rtc-abx80x.c > @@ -711,9 +711,16 @@ static int abx80x_nvmem_xfer(struct abx80x_priv *priv, unsigned int offset, > else > ret = i2c_smbus_read_i2c_block_data(priv->client, reg, > len, val); > - if (ret) > + if (ret < 0) > return ret; > > + if (!write) { > + if (ret) > + len = ret; > + else > + return -EIO; > + } I guess this is the conservative approach. Ie. Don't break things which aren't already broken. But I suspect the correct approach is to say: if (ret != len) return -EIO; Ah well. Being conservative is good. It probably doesn't ever happen in real life so it probably doesn't matter either way. I don't really like the if (write) follow by and if (!write)... It would add more lines, but improve readability if we just duplicate the code a big: if (write) { ret = write(); if (ret) return ret; } else { ret = read(); if (ret <= 0) return ret ?: -EIO; len = ret; } regards, dan carpenter