Re: [PATCH] iio: ltc2497: Fix reading conversion results

From: Jonathan Cameron <jic23@kernel.org>
To: Denys Zagorui <dzagorui@cisco.com>
Cc: Meng.Li@windriver.com, lars@metafoo.de,
	Michael.Hennerich@analog.com, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Subject: Re: [PATCH] iio: ltc2497: Fix reading conversion results
Date: Sat, 20 Aug 2022 13:06:48 +0100	[thread overview]
Message-ID: <20220820130648.5b9bc66f@jic23-huawei> (raw)
In-Reply-To: <20220815091647.1523532-1-dzagorui@cisco.com>

On Mon, 15 Aug 2022 09:16:47 +0000
Denys Zagorui <dzagorui@cisco.com> wrote:

> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> After the result of the previous conversion is read the chip
> automatically starts a new conversion and doesn't accept new i2c
> transfers until this conversion is completed which makes the function
> return failure.

That's rather nasty.

Could we add a cheeky sleep in the other path to ensure there is always
time for the conversion to be done?  Not ideal, but might ensure
there isn't a known problem path without introducing much complexity.

> 
> So add an early return iff the programming of the new address isn't
> needed. Note this will not fix the problem in general, but all cases
> that are currently used. Once this changes we get the failure back, but
> this can be addressed when the need arises.
> 
> Fixes: 69548b7c2c4f ("iio: adc: ltc2497: split protocol independent part in a separate module ")
> Reported-by: Meng Li <Meng.Li@windriver.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Tested-by: Denys Zagorui <dzagorui@cisco.com>
> ---
>  drivers/iio/adc/ltc2497.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
> index f7c786f37ceb..78b93c99cc47 100644
> --- a/drivers/iio/adc/ltc2497.c
> +++ b/drivers/iio/adc/ltc2497.c
> @@ -41,6 +41,19 @@ static int ltc2497_result_and_measure(struct ltc2497core_driverdata *ddata,
>  		}
>  
>  		*val = (be32_to_cpu(st->buf) >> 14) - (1 << 17);
> +
> +		/*
> +		 * The part started a new conversion at the end of the above i2c
> +		 * transfer, so if the address didn't change since the last call
> +		 * everything is fine and we can return early.
> +		 * If not (which should only happen when some sort of bulk
> +		 * conversion is implemented) we have to program the new
> +		 * address. Note that this probably fails as the conversion that
> +		 * was triggered above is like not complete yet and the two
> +		 * operations have to be done in a single transfer.
> +		 */
> +		if (ddata->addr_prev == address)
> +			return 0;
>  	}
>  
>  	ret = i2c_smbus_write_byte(st->client,