linux-kernel - Re: [PATCH v4 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family

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

Message-ID: <20251207182423.54e4b33d@jic23-huawei>
Date: Sun, 7 Dec 2025 18:24:23 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Jonathan Santos <Jonathan.Santos@...log.com>
Cc: <linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <Michael.Hennerich@...log.com>,
 <dlechner@...libre.com>, <nuno.sa@...log.com>, <andy@...nel.org>,
 <robh@...nel.org>, <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
 <marcelo.schmitt@...log.com>, <jonath4nns@...il.com>
Subject: Re: [PATCH v4 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1
 ADC Family

On Wed, 26 Nov 2025 18:56:35 -0300
Jonathan Santos <Jonathan.Santos@...log.com> wrote:

> Add support for ADAQ7767/68/69-1 series, which includes PGIA and
> Anti-aliasing filter (AAF) gains. Unlike the AD7768-1, they do not
> provide a VCM regulator interface.
> 
> The PGA gain is configured in run-time through the scale attribute,
> if supported by the device. PGA is controlled by GPIOs provided in
> the device tree.
> 
> The AAF gain is defined by hardware connections and should be specified
> in the device tree.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@...log.com>
> ---
> v4 Changes:
> * replaced shift_right() with '>>' operator in the ad7768_fill_scale_tbl()
>   function.
> * Refactored ad7768_parse_aaf_gain () as requested.
> * renamed ad7768_register_regulators() to ad7768_register_vcm_regulator()
>   to better reflect its purpose (not sure if this is ok to do).
> * Replaced u64_fract with u32_fract -> after reviewing the numbers again, I
>   realized that u32_fract is sufficient for these calculations.
> * addressed minor suggestions.
> * Adopted a new approach to manage the PGA GPIOs, using pga-gpios property.
>   This avoids possible conflicts when the internal gpio controller is used
>   externally (and also allows hardwiring, as soon as the gpio interface 
>   supports it). However, using GPIOs from the device's own controller causes
>   a deadlock when claiming direct mode in the ad7768_gpio_get() function.
>   This happens because the direct mode remains locked by the ad7768_write_raw()
>   function. I have kept this approach for now to discuss a way around this
>   problem. It would be good to have the flexibility provided by pga-gpios 
>   property.

See below. I 'think' this is easy enough to resolve by relaxing the need
to claim direct mode for scale changes - if it were an external gpio that
should be safe anyway and if it is internal, then the inner direct mode
claim should do the job.

Other than that, to me this code looks nice and clean so hopefully we can
merge it once that fix is in place.

Thanks,

Jonathan




> @@ -464,6 +531,42 @@ static int ad7768_reg_access(struct iio_dev *indio_dev,
>  	return ret;
>  }

>  
> +/*
> + * FIXME: Using GPIOs from the device's own controller causes the device
> + * to get stuck when claiming direct mode in the ad7768_gpio_get() function.
> + * This happens because the direct mode remains locked by the
> + * ad7768_write_raw() function.

Ouch. I'm not spotting why we need to claim direct mode for this particular
use of write_raw().  Seems safe enough to modify the gain whilst in buffered
mode.  Bit confusing perhaps if a user does this, but not a functional problem.
Sometimes we over protect in that path because userspace rarely uses write_raw
with the buffer mode enabled as it doesn't work on many devices.

So I'd squash the __ad7768_write_raw() back into ad7768_write_raw()
and claim and release direct mode only in the switch legs that need it
(oversampling and sampling frequency probably).  That should resolve
the deadlock.


> + */
> +static int ad7768_setup_pga(struct device *dev, struct ad7768_state *st)
> +{
> +	st->pga_gpios = devm_gpiod_get_array(dev, "pga", GPIOD_OUT_LOW);
> +	if (IS_ERR(st->pga_gpios))
> +		return dev_err_probe(dev, PTR_ERR(st->pga_gpios),
> +				     "Failed to get PGA gpios.\n");
> +
> +	if (st->pga_gpios->ndescs != ADAQ7768_PGA_PINS)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Expected %d GPIOs for PGA control.\n",
> +				     ADAQ7768_PGA_PINS);
> +	return 0;
> +}