linux-kernel - Re: [PATCH] iio: ltc2497: Fix reading conversion results

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220820130648.5b9bc66f@jic23-huawei>
Date:   Sat, 20 Aug 2022 13:06:48 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Denys Zagorui <dzagorui@...co.com>
Cc:     Meng.Li@...driver.com, lars@...afoo.de,
        Michael.Hennerich@...log.com, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Subject: Re: [PATCH] iio: ltc2497: Fix reading conversion results

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

> From: Uwe Kleine-König <u.kleine-koenig@...gutronix.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@...driver.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
> Tested-by: Denys Zagorui <dzagorui@...co.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,