Re: [PATCH] iio:dac: fix i2c_master_send return code misuse

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Tue, Nov 03, 2015 at 03:27:42PM +0100, Lars-Peter Clausen wrote:
> Hi,
> 
> On 10/27/2015 05:25 PM, Jean-Francois Dagenais wrote:
> > Contrary to most kernel functions, successful call to
> > i2c_master_send returns the number or written bytes,not 0.
> > 
> > However, these callers return the value as is without converting
> > the return style to conform to the expected standard.
> > 
> > Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
> 
> Thanks for the patch.
> 
> > ---
> >  drivers/iio/dac/ad5064.c  |  5 ++++-
> >  drivers/iio/dac/ad5446.c  |  4 +++-
> >  drivers/iio/dac/max517.c  |  6 ++++--
> >  drivers/iio/dac/max5821.c |  8 ++++++--
> >  drivers/iio/dac/mcp4725.c | 10 ++++++++--
> >  5 files changed, 25 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
> > index c067e68..0ff1107 100644
> > --- a/drivers/iio/dac/ad5064.c
> > +++ b/drivers/iio/dac/ad5064.c
> > @@ -598,10 +598,13 @@ static int ad5064_i2c_write(struct ad5064_state *st, unsigned int cmd,
> >  	unsigned int addr, unsigned int val)
> >  {
> >  	struct i2c_client *i2c = to_i2c_client(st->dev);
> > +	int res;
> >  
> >  	st->data.i2c[0] = (cmd << 4) | addr;
> >  	put_unaligned_be16(val, &st->data.i2c[1]);
> > -	return i2c_master_send(i2c, st->data.i2c, 3);
> > +	res = i2c_master_send(i2c, st->data.i2c, 3);
> > +
> > +	return res == 3 ? 0 : res;
> >  }
> 
> The ad5064 is already fixed in Jonathan's tree.
> 
> >  
> >  static int ad5064_i2c_probe(struct i2c_client *i2c,
> > diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
> > index 07e17d7..9a49941 100644
> > --- a/drivers/iio/dac/ad5446.c
> > +++ b/drivers/iio/dac/ad5446.c
> > @@ -512,7 +512,9 @@ static int ad5622_write(struct ad5446_state *st, unsigned val)
> >  	struct i2c_client *client = to_i2c_client(st->dev);
> >  	__be16 data = cpu_to_be16(val);
> >  
> > -	return i2c_master_send(client, (char *)&data, sizeof(data));
> > +	int res = i2c_master_send(client, (char *)&data, sizeof(data));
> > +
> > +	return res == sizeof(data)? 0 : res;
> 
> This still returns the wrong if res is between 0 and sizeof(data)-1. You
> need something like
> 
> 	if (res == sizeof(data))
> 		return 0
> 	else if (res >= 0)
> 		return -EIO;
> 	else
> 		return res;
> 
> Same is true for all the other drivers patched.
> 
> I really wish we could fix this in the I2C core to avoid having to copy and
> paste the same error handling to each driver.
> 
> There was this patch[1], but it looks like it was never followed by a v2.
> Dmitry I'm curious, what happened to that patch?

Fell through the cracks I'm afraid. I think facility like this is still
needed as majority of the drivers do not care about short reads in the
slightest.

-- 
Dmitry