Received-SPF: pass (google.com: domain of linux-kernel+bounces-157185-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: "Hennerich, Michael" <Michael.Hennerich@analog.com>
To: Jonathan Cameron <jic23@kernel.org>,
        Guillaume Stols
	<gstols@baylibre.com>,
        David Lechner <dlechner@baylibre.com>
CC: Lars-Peter Clausen <lars@metafoo.de>,
        "linux-iio@vger.kernel.org"
	<linux-iio@vger.kernel.org>,
        "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>,
        jstephan <jstephan@baylibre.com>
Subject: RE: [PATCH] iio: adc: ad7606: remove frstdata check
Thread-Topic: [PATCH] iio: adc: ad7606: remove frstdata check
Thread-Index: AQHakKdB1bhEQ3vcBkiNpSXNM2Wd0LFxB+6AgAaMbuA=
Date: Wed, 24 Apr 2024 15:22:08 +0000
Message-ID: 
 <CO3PR03MB6774929012D27900E8DAED458E102@CO3PR03MB6774.namprd03.prod.outlook.com>
References: <20240417-cleanup-ad7606-v1-1-5c2a29662c0a@baylibre.com>
 <20240420121933.13a0e22f@jic23-huawei>
In-Reply-To: <20240420121933.13a0e22f@jic23-huawei>
Accept-Language: en-US, de-DE
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?us-ascii?Q?/5Vk4GdlwpQGsP1Rv1NvWdvD32z5OBV9ta9v7jxOr7VddD49Ib3QR3HxSwyl?=
 =?us-ascii?Q?0uJfpw86nHX5wE5gAxrTLa8mBxPKOXqPz7ZZ8nJalBC6hKkMD+oDm4/vqGf1?=
 =?us-ascii?Q?Jwss11ZWXo153XvPxbQXRXAUIW3qNNQ/AhUJuuCf1ednTj4kzZSFACcaneMo?=
 =?us-ascii?Q?JBp6Lvz7+u+gGuaDC269JCZt/mVp6XF3+3XVIKOPiOtlQkZdvwLcKgb/Qrty?=
 =?us-ascii?Q?jNPp9KdqoH4dGnlRdIU0vCNpwqZ0rManenGtD3PvVyNH10ziGBf4ujP9OLZA?=
 =?us-ascii?Q?E8yhST9zcGFoK0UXxueTJOcBputo7qV7xESJI27Zwk0tKpcHh2icNEwuBNet?=
 =?us-ascii?Q?/onhE+RD1xYMKXSPCZR9I2K1Nd632VtWr7zXWWuszXr7ra/x+Zm97XWBmr+T?=
 =?us-ascii?Q?BLuL55yS92ymhuu96FqpmIsysvv6oW9Nfl9SN5dgqBKAp5jDJNyz3PV7mFp0?=
 =?us-ascii?Q?P6t9JPkgOvnLqvgzvb8lWlotYyB0WgavGsvL8JlaX8H56N2Rc58/UHmWWjUx?=
 =?us-ascii?Q?XuC9JsDvxCm78xrX65dxaY5K/RKJ+1FzMRz8O/NRVrZRKkR8h1vhBUh2GEQQ?=
 =?us-ascii?Q?LgjGvwAPDU0iQFDrcLiI22gsHFBcY6rAubtAFsGgExhT+7sHqkWExDJRyxBr?=
 =?us-ascii?Q?z/zQP9F/bxxKm03HGg6qK/JJUA8Ms+n5FIm5WgYe+w9rhgp93Lwh0IUhzCeq?=
 =?us-ascii?Q?l/gmdRxygOZyXrgbtCWfiTqql/On1DEDhN6E+PaKCQGNoxYInuU2ElnLWTX5?=
 =?us-ascii?Q?Tj790I5/wKDy2LUfYMExsE3AwixpGRfzTv1ZW4TjJa2boki5WgNQmZWJbwF8?=
 =?us-ascii?Q?cpq0KhIQF1Nfj9SMwCI/V8XserTWS2N2XnJxDMMBxDTJ+0oJwWoAGVZ4d3pm?=
 =?us-ascii?Q?vasylBG32ZJv8vUdtY2ASJ8L55PYSyxCAbGYORGqDlI1gew961uD71LOhBZj?=
 =?us-ascii?Q?dOY0LUM/GJfXuYss7tO32oRQUzwcAfZSsS1HLscBRSYSj8ZYkPBQNXORjzsZ?=
 =?us-ascii?Q?zTBkECSjUXps4ZySAaTZlcLSGzUka62UGEVyr88B4dygeEoELlfOPqkKmMb2?=
 =?us-ascii?Q?PJndWmO2uVUV+vHil9tiNhmgQT5xIWstqTi0NFRpw3W2lMK6Y3YirPkrb/ST?=
 =?us-ascii?Q?QUVg/yoIfectuIyyVItiefComnI5LC5jBP65+kAXrSMApaRXSarL8MSeDkiQ?=
 =?us-ascii?Q?DagEuCWqcKgEOCdd6aycZezrqsU2BoIk4eKGdxe49guPCJlG8i1QdhxjttKH?=
 =?us-ascii?Q?ru7yQG4R1dTHPKJ/rwcu4TnR3xALBiYL8mpjjh6KFA5AYSTWuK/WH/PEHyuZ?=
 =?us-ascii?Q?oWmYJ/OTGpA7EbLrHW2Bb2/yavuJQVTKL4IRaL0nV9bikFzpvVoqHOKAsxCk?=
 =?us-ascii?Q?mP2b7RUzYlpENBX7Dkf7oJIM0nCtN8f3rNcpdP9sKdTvbPcCYBM8R8dHieVp?=
 =?us-ascii?Q?kUUgZCRMA8sni+4WGUuPTq3mZd92F78HhjnUpxbbD/dGeoD2gtmZeX4IwZBP?=
 =?us-ascii?Q?wAmAG7S9UV3aQGZvsSkF2hKeJPBC/Mhr7GI0aT0tCDTRy5UojtsBtCCHdNvj?=
 =?us-ascii?Q?czVNjtTM1LOfyr0tAgTXPzPO7MF4F3gUtPxaECw94wYTEUUQej1FnXwFK/bO?=
 =?us-ascii?Q?Kw=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: analog.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CO3PR03MB6774.namprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 287f294e-6f65-44fb-7aa3-08dc64724eb0
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Apr 2024 15:22:08.8859
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: eaa689b4-8f87-40e0-9c6f-7228de4d754a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: dMAaZbYL88PktYS7KQd6M3HVFD7Jfqi9m+e15j/H/sMuBJxq4IGRcRhNI7rBbvFunxRb7NkLLA3U/HUHnnRoAG2CpWsSzLMN4wNflAtMjko=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO3PR03MB6709
X-Proofpoint-GUID: SCB8Qwnj1QaNTbpCe7p2ukM7HwhDxtux
X-Proofpoint-ORIG-GUID: SCB8Qwnj1QaNTbpCe7p2ukM7HwhDxtux
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-04-24_12,2024-04-24_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1011
 priorityscore=1501 malwarescore=0 impostorscore=0 phishscore=0 spamscore=0
 bulkscore=0 mlxlogscore=999 lowpriorityscore=0 adultscore=0 mlxscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.19.0-2404010003 definitions=main-2404240059


> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, April 20, 2024 7:20 AM
> To: Guillaume Stols <gstols@baylibre.com>; David Lechner
> <dlechner@baylibre.com>
> Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; linux-iio@vger.kernel.org; linux-
> kernel@vger.kernel.org; jstephan <jstephan@baylibre.com>
> Subject: Re: [PATCH] iio: adc: ad7606: remove frstdata check
>=20
>=20
> On Wed, 17 Apr 2024 09:11:18 +0000
> Guillaume Stols <gstols@baylibre.com> wrote:
>=20
> > 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 samp=
le.
> > 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@baylibre.com>
>=20
> Michael, I'm sure you remember this well - it was only 13 years ago you w=
rote
> this...
>=20
> https://urldefense.com/v3/__https://lore.kernel.org/all/1296744691-24320-=
1-
> git-send-email-
> michael.hennerich@analog.com/__;!!A3Ni8CS0y2Y!9ozMtBwfPYIYOUVmrg1oq
> aTFP9BHcnV5_ENZ7u1U3TSBRbcCor2p4gzWA1lsyjxN2sjgNvG2a7n-RHA4lLY$
>=20
> 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...

Thanks for pulling me in here.
This code was originally put in place for the parallel interface.
So please don't remove it completely, just disable it for SPI.

-Michael=20
>=20
> Jonathan
>=20
> > ---
> > This is the first commit of cleanup series. It will be followed by
> > more cleanups and support for more parts and features.
>=20
> Sounds good.
>=20
> > ---
> >  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 =3D st->chip_info->num_channels - 1;
> >  	u16 *data =3D st->data;
> > -	int ret;
> > -
> > -	/*
> > -	 * The frstdata signal is set to high while and after reading the sam=
ple
> > -	 * of the first channel and low for all other channels. This can be u=
sed
> > -	 * to check that the incoming data is correctly aligned. During norma=
l
> > -	 * 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 =3D 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 =3D 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,