linux-kernel - [PATCH] iio: adc: ad7606: remove frstdata check

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-next>] [day] [month] [year] [list]

Message-Id: <20240417-cleanup-ad7606-v1-1-5c2a29662c0a@baylibre.com>
Date: Wed, 17 Apr 2024 09:11:18 +0000
From: Guillaume Stols <gstols@...libre.com>
To: Lars-Peter Clausen <lars@...afoo.de>, 
 Michael Hennerich <Michael.Hennerich@...log.com>, 
 Jonathan Cameron <jic23@...nel.org>
Cc: linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, 
 jstephan@...libre.com, dlechner@...libre.com, 
 Guillaume Stols <gstols@...libre.com>
Subject: [PATCH] iio: adc: ad7606: remove frstdata check

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@...libre.com>
---
This is the first commit of cleanup series. It will be followed by more
cleanups and support for more parts and features.
---
 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,
-- 
Guillaume Stols <gstols@...libre.com>