* [PATCH] iio: dac: ad5446: Fix ad5622_write() return value
@ 2021-09-28 19:55 Pekka Korpinen
2021-09-29 9:46 ` Alexandru Ardelean
2021-09-29 9:59 ` Lars-Peter Clausen
0 siblings, 2 replies; 5+ messages in thread
From: Pekka Korpinen @ 2021-09-28 19:55 UTC (permalink / raw)
Cc: Pekka Korpinen, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, linux-iio, linux-kernel
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(-)
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] iio: dac: ad5446: Fix ad5622_write() return value
2021-09-28 19:55 [PATCH] iio: dac: ad5446: Fix ad5622_write() return value Pekka Korpinen
@ 2021-09-29 9:46 ` Alexandru Ardelean
2021-09-29 9:59 ` Lars-Peter Clausen
1 sibling, 0 replies; 5+ messages in thread
From: Alexandru Ardelean @ 2021-09-29 9:46 UTC (permalink / raw)
To: Pekka Korpinen
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron, linux-iio, LKML
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] iio: dac: ad5446: Fix ad5622_write() return value
2021-09-28 19:55 [PATCH] iio: dac: ad5446: Fix ad5622_write() return value Pekka Korpinen
2021-09-29 9:46 ` Alexandru Ardelean
@ 2021-09-29 9:59 ` Lars-Peter Clausen
2021-09-29 18:57 ` [PATCH v2] " Pekka Korpinen
1 sibling, 1 reply; 5+ messages in thread
From: Lars-Peter Clausen @ 2021-09-29 9:59 UTC (permalink / raw)
To: Pekka Korpinen
Cc: Michael Hennerich, Jonathan Cameron, linux-iio, linux-kernel
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;
> }
>
> /*
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2] iio: dac: ad5446: Fix ad5622_write() return value
2021-09-29 9:59 ` Lars-Peter Clausen
@ 2021-09-29 18:57 ` Pekka Korpinen
2021-10-17 10:43 ` Jonathan Cameron
0 siblings, 1 reply; 5+ messages in thread
From: Pekka Korpinen @ 2021-09-29 18:57 UTC (permalink / raw)
Cc: ardeleanalex, Pekka Korpinen, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Jean-Francois Dagenais,
linux-iio, linux-kernel
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.
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.
Fixes: 3ec36a2cf0d5 ("iio:ad5446: Add support for I2C based DACs")
Signed-off-by: Pekka Korpinen <pekka.korpinen@iki.fi>
---
v1->v2: Check against expected result, otherwise -EIO. Add Fixes tag.
A similar bug was fixed for ad5064.c in 2015 - commit 03fe472ef33b
("iio:ad5064: Make sure ad5064_i2c_write() returns 0 on success").
drivers/iio/dac/ad5446.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
index 488ec69967d6..e50718422411 100644
--- a/drivers/iio/dac/ad5446.c
+++ b/drivers/iio/dac/ad5446.c
@@ -531,8 +531,15 @@ 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;
+ if (ret != sizeof(data))
+ return -EIO;
- return i2c_master_send(client, (char *)&data, sizeof(data));
+ return 0;
}
/*
--
2.33.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] iio: dac: ad5446: Fix ad5622_write() return value
2021-09-29 18:57 ` [PATCH v2] " Pekka Korpinen
@ 2021-10-17 10:43 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2021-10-17 10:43 UTC (permalink / raw)
To: Pekka Korpinen
Cc: ardeleanalex, Lars-Peter Clausen, Michael Hennerich,
Jean-Francois Dagenais, linux-iio, linux-kernel
On Wed, 29 Sep 2021 21:57:55 +0300
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.
>
> 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.
>
> Fixes: 3ec36a2cf0d5 ("iio:ad5446: Add support for I2C based DACs")
> Signed-off-by: Pekka Korpinen <pekka.korpinen@iki.fi>
Applied to the fixes-togreg branch of iio.git and marked for stable.
Thanks,
Jonathan
> ---
> v1->v2: Check against expected result, otherwise -EIO. Add Fixes tag.
>
> A similar bug was fixed for ad5064.c in 2015 - commit 03fe472ef33b
> ("iio:ad5064: Make sure ad5064_i2c_write() returns 0 on success").
>
> drivers/iio/dac/ad5446.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
> index 488ec69967d6..e50718422411 100644
> --- a/drivers/iio/dac/ad5446.c
> +++ b/drivers/iio/dac/ad5446.c
> @@ -531,8 +531,15 @@ 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;
> + if (ret != sizeof(data))
> + return -EIO;
>
> - return i2c_master_send(client, (char *)&data, sizeof(data));
> + return 0;
> }
>
> /*
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-17 10:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28 19:55 [PATCH] iio: dac: ad5446: Fix ad5622_write() return value Pekka Korpinen
2021-09-29 9:46 ` Alexandru Ardelean
2021-09-29 9:59 ` Lars-Peter Clausen
2021-09-29 18:57 ` [PATCH v2] " Pekka Korpinen
2021-10-17 10:43 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).