Re: [PATCH] iio: adc: ad7606: remove frstdata check

From: Jonathan Cameron
Date: Sat Apr 20 2024 - 07:19:52 EST


On Wed, 17 Apr 2024 09:11:18 +0000
Guillaume Stols <gstols@xxxxxxxxxxxx> wrote:

> Frstdata pin is set high during the first sample's transmission and
> then set low.
> This code chunk attempts to recover from an eventual glitch in the clock
> by checking frstdata state after reading the first channel's sample.
> Currently, in serial mode, this check happens AFTER the 16th pulse, and if
> frstdata is not set it resets the device and returns EINVAL.
> According to the datasheet, "The FRSTDATA output returns to a logic low
> following the 16th SCLK falling edge.", thus after the 16th pulse, the check
> will always be true, and the driver will not work as expected.
> Even if it was working, the usefulness of this check is limited, since
> it would only detect a glitch on the first channel, but not on the
> following ones, and the convst pulse will reset the communication sequence at
> each new conversion.
>
> Signed-off-by: Guillaume Stols <gstols@xxxxxxxxxxxx>

Michael, I'm sure you remember this well - it was only 13 years ago you wrote
this...

https://lore.kernel.org/all/1296744691-24320-1-git-send-email-michael.hennerich@xxxxxxxxxx/

Anyhow, I'd like an Ack or a statement of you have no idea any more and i should
go with what Guillaume has worked out...

Jonathan

> ---
> This is the first commit of cleanup series. It will be followed by more
> cleanups and support for more parts and features.

Sounds good.

> ---
> drivers/iio/adc/ad7606.c | 30 ------------------------------
> drivers/iio/adc/ad7606.h | 3 ---
> 2 files changed, 33 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> index 1928d9ae5bcf..f85eb0832703 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -88,31 +88,6 @@ static int ad7606_read_samples(struct ad7606_state *st)
> {
> unsigned int num = st->chip_info->num_channels - 1;
> u16 *data = st->data;
> - int ret;
> -
> - /*
> - * The frstdata signal is set to high while and after reading the sample
> - * of the first channel and low for all other channels. This can be used
> - * to check that the incoming data is correctly aligned. During normal
> - * operation the data should never become unaligned, but some glitch or
> - * electrostatic discharge might cause an extra read or clock cycle.
> - * Monitoring the frstdata signal allows to recover from such failure
> - * situations.
> - */
> -
> - if (st->gpio_frstdata) {
> - ret = st->bops->read_block(st->dev, 1, data);
> - if (ret)
> - return ret;
> -
> - if (!gpiod_get_value(st->gpio_frstdata)) {
> - ad7606_reset(st);
> - return -EIO;
> - }
> -
> - data++;
> - num--;
> - }
>
> return st->bops->read_block(st->dev, num, data);
> }
> @@ -450,11 +425,6 @@ static int ad7606_request_gpios(struct ad7606_state *st)
> if (IS_ERR(st->gpio_standby))
> return PTR_ERR(st->gpio_standby);
>
> - st->gpio_frstdata = devm_gpiod_get_optional(dev, "adi,first-data",
> - GPIOD_IN);
> - if (IS_ERR(st->gpio_frstdata))
> - return PTR_ERR(st->gpio_frstdata);
> -
> if (!st->chip_info->oversampling_num)
> return 0;
>
> diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
> index 0c6a88cc4695..eacb061de6f8 100644
> --- a/drivers/iio/adc/ad7606.h
> +++ b/drivers/iio/adc/ad7606.h
> @@ -80,8 +80,6 @@ struct ad7606_chip_info {
> * @gpio_range GPIO descriptor for range selection
> * @gpio_standby GPIO descriptor for stand-by signal (STBY),
> * controls power-down mode of device
> - * @gpio_frstdata GPIO descriptor for reading from device when data
> - * is being read on the first channel
> * @gpio_os GPIO descriptors to control oversampling on the device
> * @complete completion to indicate end of conversion
> * @trig The IIO trigger associated with the device.
> @@ -108,7 +106,6 @@ struct ad7606_state {
> struct gpio_desc *gpio_reset;
> struct gpio_desc *gpio_range;
> struct gpio_desc *gpio_standby;
> - struct gpio_desc *gpio_frstdata;
> struct gpio_descs *gpio_os;
> struct iio_trigger *trig;
> struct completion completion;
>
> ---
> base-commit: 62d3fb9dcc091ccdf25eb3b716e90e07e3ed861f
> change-id: 20240416-cleanup-ad7606-161e2ed9818b
>
> Best regards,