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,
