| Message ID | 20210928195548.17846-1-pekka.korpinen@iki.fi (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: dac: ad5446: Fix ad5622_write() return value | expand |
On Tue, Sep 28, 2021 at 10:57 PM Pekka Korpinen <pekka.korpinen@iki.fi> wrote: > > On success i2c_master_send() returns the number of bytes written. The > call from iio_write_channel_info(), however, expects the return value to > be zero on success. > Requires a Fixes tag. But other than that: Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com> > Signed-off-by: Pekka Korpinen <pekka.korpinen@iki.fi> > --- > This bug causes incorrect consumption of the sysfs buffer in > iio_write_channel_info(). When writing more than two characters to > out_voltage0_raw, the ad5446 write handler is called multiple times > causing unexpected behavior. > > A similar fix was applied for ad5064.c in 2015 - commit 03fe472ef33b > ("iio:ad5064: Make sure ad5064_i2c_write() returns 0 on success"). > > drivers/iio/dac/ad5446.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c > index 488ec69967d6..dfd541bbde5b 100644 > --- a/drivers/iio/dac/ad5446.c > +++ b/drivers/iio/dac/ad5446.c > @@ -531,8 +531,13 @@ 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); > + int ret; > + > + ret = i2c_master_send(client, (char *)&data, sizeof(data)); > + if (ret < 0) > + return ret; > > - return i2c_master_send(client, (char *)&data, sizeof(data)); > + return 0; > } > > /* > -- > 2.33.0 >
On 9/28/21 9:55 PM, Pekka Korpinen wrote: > On success i2c_master_send() returns the number of bytes written. The > call from iio_write_channel_info(), however, expects the return value to > be zero on success. > > Signed-off-by: Pekka Korpinen <pekka.korpinen@iki.fi> > --- > This bug causes incorrect consumption of the sysfs buffer in > iio_write_channel_info(). When writing more than two characters to > out_voltage0_raw, the ad5446 write handler is called multiple times > causing unexpected behavior. I'd put this into the commit message itself. This is useful information that should be part of the commit log. > > A similar fix was applied for ad5064.c in 2015 - commit 03fe472ef33b > ("iio:ad5064: Make sure ad5064_i2c_write() returns 0 on success"). > > drivers/iio/dac/ad5446.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c > index 488ec69967d6..dfd541bbde5b 100644 > --- a/drivers/iio/dac/ad5446.c > +++ b/drivers/iio/dac/ad5446.c > @@ -531,8 +531,13 @@ 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); > + int ret; > + > + ret = i2c_master_send(client, (char *)&data, sizeof(data)); > + if (ret < 0) > + return ret; Like you wrote in the commit message the function returns the number of bytes written. We should check that this matches the number of bytes we wanted to send and return an error (EIO) otherwise. > > - return i2c_master_send(client, (char *)&data, sizeof(data)); > + return 0; > } > > /*
diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c index 488ec69967d6..dfd541bbde5b 100644 --- a/drivers/iio/dac/ad5446.c +++ b/drivers/iio/dac/ad5446.c @@ -531,8 +531,13 @@ 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); + int ret; + + ret = i2c_master_send(client, (char *)&data, sizeof(data)); + if (ret < 0) + return ret; - return i2c_master_send(client, (char *)&data, sizeof(data)); + return 0; } /*
On success i2c_master_send() returns the number of bytes written. The call from iio_write_channel_info(), however, expects the return value to be zero on success. Signed-off-by: Pekka Korpinen <pekka.korpinen@iki.fi> --- This bug causes incorrect consumption of the sysfs buffer in iio_write_channel_info(). When writing more than two characters to out_voltage0_raw, the ad5446 write handler is called multiple times causing unexpected behavior. A similar fix was applied for ad5064.c in 2015 - commit 03fe472ef33b ("iio:ad5064: Make sure ad5064_i2c_write() returns 0 on success"). drivers/iio/dac/ad5446.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)