From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E92BA10F1; Sun, 17 May 2026 12:14:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779020055; cv=none; b=hgB0WM0N4orH+rmtVy8KsGHfDVtyUC1UwqhRs5/uH7phvsQM4wW+vIbncke2SAuN3DzZzLCGFE8bD9r7E4mOzAguJFpsTS3Ls8D6FnCMQU0gxkBxdIbzSBZci4ErwB+e2gUEYBSRten6/OpBjFxt5d7A/GTQk5vShkFcNbt2K2E= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779020055; c=relaxed/simple; bh=bFcGiGJfu9Ac5rl1wi1J5n2/JD7kyJwnntbvyDfKYpo=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=c0B094BRSiDh98qxQCJp6O7QB8JYYjfE5eVY3pmATou5029OHpdbr+slBa3dh+2bHLGTfBEDWKMkF1GZ2x24HYbZrYI97/YtdiOZ9TpJ7vhvko1e1BKhWZpQeWcPfC/Fc0SxqhrR9wSgJuWQC9e4QqkiNCMKX/rH4S0Jk6SZSmk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=EjVoQvKz; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="EjVoQvKz" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4ED72C2BCB0; Sun, 17 May 2026 12:14:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1779020054; bh=bFcGiGJfu9Ac5rl1wi1J5n2/JD7kyJwnntbvyDfKYpo=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=EjVoQvKzHIEG2gPB3ClUsCxLPGrics1u2frI9totgFxSZmbHtPMyJgP7T2ly1h38s /f9H6uHDliTgPx5BURcLPYVM5Jxl5ha1ki6UOSCpzUFVRzwnBRT257Ke+Iq6V/zx8O RYAI/lF3qWpRTu9mlElAslwSWb5sYAftrzN1XRc7tFvgL3sFOBGuo3hrXzt0bFz2bw ylJFyoB6WroMcdLQG5Cqf1psjwFzu4H2MQ13aN/4l+qUbWn1DcJfJj6QVfKdiqrW3I s5AchMDeVOzPiV2MxYInItIxAktpOQjXiKNXvBJ58VNP5ErYhHE/kdjwc4W0kniRKh bGcKnhf/ZL8Tg== Date: Sun, 17 May 2026 13:14:01 +0100 From: Jonathan Cameron To: David Lechner Cc: radu.sabau@analog.com, Lars-Peter Clausen , Michael Hennerich , Nuno =?UTF-8?B?U8Oh?= , Andy Shevchenko , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Uwe =?UTF-8?B?S2xlaW5lLUvDtm5pZw==?= , Liam Girdwood , Mark Brown , Linus Walleij , Bartosz Golaszewski , Philipp Zabel , Jonathan Corbet , Shuah Khan , linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org, linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org Subject: Re: [PATCH v11 2/6] iio: adc: ad4691: add initial driver for AD4691 family Message-ID: <20260517131401.32118f4c@jic23-huawei> In-Reply-To: <0696b662-f478-4d1a-95e0-0338bbdb719e@baylibre.com> References: <20260515-ad4692-multichannel-sar-adc-driver-v11-0-eab27d852ac2@analog.com> <20260515-ad4692-multichannel-sar-adc-driver-v11-2-eab27d852ac2@analog.com> <0696b662-f478-4d1a-95e0-0338bbdb719e@baylibre.com> X-Mailer: Claws Mail 4.4.0 (GTK 3.24.52; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-doc@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Sat, 16 May 2026 12:11:16 -0500 David Lechner wrote: > On 5/15/26 8:31 AM, Radu Sabau via B4 Relay wrote: > > From: Radu Sabau > >=20 > > Add support for the Analog Devices AD4691 family of high-speed, > > low-power multichannel SAR ADCs: AD4691 (16-ch, 500 kSPS), > > AD4692 (16-ch, 1 MSPS), AD4693 (8-ch, 500 kSPS) and > > AD4694 (8-ch, 1 MSPS). > >=20 > > The driver implements a custom regmap layer over raw SPI to handle the > > device's mixed 1/2/3/4-byte register widths and uses the standard IIO > > read_raw/write_raw interface for single-channel reads. > >=20 > > The chip idles in Autonomous Mode so that single-shot read_raw can use > > the internal oscillator without disturbing the hardware configuration. > >=20 > > Three voltage supply domains are managed: avdd (required), vio, and a > > reference supply on either the REF pin (ref-supply, external buffer) > > or the REFIN pin (refin-supply, uses the on-chip reference buffer; > > REFBUF_EN is set accordingly). Hardware reset is performed via > > the reset controller framework; a software reset through SPI_CONFIG_A > > is used as fallback when no hardware reset is available. > >=20 > > Accumulator channel masking for single-shot reads uses ACC_MASK_REG via > > an ADDR_DESCENDING SPI write, which covers both mask bytes in a single > > 16-bit transfer. > >=20 > > IIO_CHAN_INFO_SAMP_FREQ is exposed as info_mask_separate. The oscillator > > is shared hardware =E2=80=94 writing any channel's sampling_frequency a= ttribute > > sets it for all others =E2=80=94 but per-channel attributes are used th= roughout > > the series to avoid an ABI change when per-channel oversampling ratios > > are introduced in a later commit, at which point the effective output > > rate (osc_freq / osr[N]) becomes genuinely per-channel. > >=20 > > Reviewed-by: David Lechner > > Signed-off-by: Radu Sabau One follow up as I was commenting on same code... > > diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c > > new file mode 100644 > > index 000000000000..ba77e1bfef16 > > --- /dev/null > > +++ b/drivers/iio/adc/ad4691.c > > +static int ad4691_get_sampling_freq(struct ad4691_state *st, int *val) > > +{ > > + unsigned int reg_val; > > + int ret; > > + =20 >=20 > No mutex lock here? Maybe without OK since it is a read. Agreed. It's not a bug, but also not a fast path and it will save reasoning and need for comment to just take the lock. >=20 > > + /* > > + * AD4691_OSC_FREQ_REG is non-volatile and written during > > + * ad4691_config(), so regmap returns the cached value here without > > + * touching the SPI bus. No lock is needed. > > + */ > > + ret =3D regmap_read(st->regmap, AD4691_OSC_FREQ_REG, ®_val); > > + if (ret) > > + return ret; > > + > > + *val =3D ad4691_osc_freqs_Hz[FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val)= ]; > > + return IIO_VAL_INT; > > +} > > + > > +static int ad4691_set_sampling_freq(struct iio_dev *indio_dev, int fre= q) > > +{ > > + struct ad4691_state *st =3D iio_priv(indio_dev); > > + unsigned int start =3D ad4691_samp_freq_start(st->info); > > + > > + IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim); > > + if (IIO_DEV_ACQUIRE_FAILED(claim)) > > + return -EBUSY; > > + > > + for (unsigned int i =3D start; i < ARRAY_SIZE(ad4691_osc_freqs_Hz); i= ++) { > > + if (ad4691_osc_freqs_Hz[i] !=3D freq) > > + continue; =20 >=20 > mutex lock? Agreed. Whilst the direct mode acquire will serialize that's an internal im= plementation detail. Where a driver needs to ensure some sequences are not interrupted (like I think for the single short read?) then it should take the local lock. >=20 > > + return regmap_update_bits(st->regmap, AD4691_OSC_FREQ_REG, > > + AD4691_OSC_FREQ_MASK, i); > > + } > > + > > + return -EINVAL; > > +}