LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Lars-Peter Clausen <lars@metafoo.de>
To: Pekka Korpinen <pekka.korpinen@iki.fi>
Cc: Michael Hennerich <Michael.Hennerich@analog.com>,
	Jonathan Cameron <jic23@kernel.org>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iio: dac: ad5446: Fix ad5622_write() return value
Date: Wed, 29 Sep 2021 11:59:59 +0200	[thread overview]
Message-ID: <20d11fbb-ba93-802c-1abc-60d7f5ec0c0c@metafoo.de> (raw)
In-Reply-To: <20210928195548.17846-1-pekka.korpinen@iki.fi>

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;
>   }
>   
>   /*



  parent reply	other threads:[~2021-09-29 10:00 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-28 19:55 Pekka Korpinen
2021-09-29  9:46 ` Alexandru Ardelean
2021-09-29  9:59 ` Lars-Peter Clausen [this message]
2021-09-29 18:57   ` [PATCH v2] " Pekka Korpinen
2021-10-17 10:43     ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20d11fbb-ba93-802c-1abc-60d7f5ec0c0c@metafoo.de \
    --to=lars@metafoo.de \
    --cc=Michael.Hennerich@analog.com \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pekka.korpinen@iki.fi \
    --subject='Re: [PATCH] iio: dac: ad5446: Fix ad5622_write() return value' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).