linux-kernel - Re: [PATCH v3 8/8] iio: adc: ad4030: Add support for ADAQ4216 and ADAQ4224

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

Message-ID: <20250928112628.7a1357ab@jic23-huawei>
Date: Sun, 28 Sep 2025 11:26:28 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Marcelo Schmitt <marcelo.schmitt@...log.com>
Cc: <linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
 <linux-doc@...r.kernel.org>, <linux-spi@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <michael.hennerich@...log.com>,
 <nuno.sa@...log.com>, <eblanc@...libre.com>, <dlechner@...libre.com>,
 <andy@...nel.org>, <robh@...nel.org>, <krzk+dt@...nel.org>,
 <conor+dt@...nel.org>, <corbet@....net>, <marcelo.schmitt1@...il.com>
Subject: Re: [PATCH v3 8/8] iio: adc: ad4030: Add support for ADAQ4216 and
 ADAQ4224

On Fri, 26 Sep 2025 17:41:04 -0300
Marcelo Schmitt <marcelo.schmitt@...log.com> wrote:

> ADAQ4216 and ADAQ4224 are similar to AD4030, but feature a PGA circuitry
> that scales the analog input signal prior to it reaching the ADC. The PGA
> is controlled through a pair of pins (A0 and A1) whose state define the
> gain that is applied to the input signal.
> 
> Add support for ADAQ4216 and ADAQ4224. Provide a list of PGA options
> through the IIO device channel scale available interface and enable control
> of the PGA through the channel scale interface.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@...log.com>

One trivial comment + a suggestion that we might want to consider pushing
the pin strap pga support to a future patch so we can not stall the driver
on that being resolved.  Note that would mean splitting the DT patch as well.

Anyhow, no rush on that given we have lots of time in this kernel cycle.

Jonathan

> @@ -996,6 +1116,17 @@ static int ad4030_write_raw(struct iio_dev *indio_dev,
>  	return ret;
>  }

> @@ -1307,6 +1439,50 @@ static int ad4030_spi_offload_setup(struct iio_dev *indio_dev,
>  							   IIO_BUFFER_DIRECTION_IN);
>  }
>  
> +static int ad4030_setup_pga(struct device *dev, struct iio_dev *indio_dev,
> +			    struct ad4030_state *st)
> +{
> +	unsigned int i;
> +	int pga_gain_dB;
> +	int ret;
> +
> +	ret = device_property_read_u32(dev, "adi,pga-gain-db", &pga_gain_dB);

I'm not sure the pga question will resolve quickly so perhaps we initially only
support the pins connected to gpios and add the pin strapped support later?

It's early in this cycle, so we can delay that decision for a few weeks and
see where we are then on that DT question.

> +	if (ret == -EINVAL) {
> +		/* Setup GPIOs for PGA control */
> +		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 != ADAQ4616_PGA_PINS)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Expected 2 GPIOs for PGA control.\n");
> +
> +		st->scale_avail_size = ARRAY_SIZE(adaq4216_hw_gains_db);
> +		st->pga_index = 0;
> +		return 0;
> +	} else if (ret != 0) {

if (ret) is fairly commonly used for error checking when non 0 is what we want.

> +		return dev_err_probe(dev, ret, "Failed to get PGA value.\n");
> +	}
> +
> +	/* Set ADC driver to handle pin-strapped PGA pins setup */
> +	for (i = 0; i < ARRAY_SIZE(adaq4216_hw_gains_db); i++) {
> +		if (pga_gain_dB != adaq4216_hw_gains_db[i])
> +			continue;
> +
> +		st->pga_index = i;
> +		break;
> +	}
> +	if (i == ARRAY_SIZE(adaq4216_hw_gains_db))
> +		return dev_err_probe(dev, -EINVAL, "Invalid PGA gain: %d.\n",
> +				     pga_gain_dB);
> +
> +	st->scale_avail_size = 1;
> +	st->pga_gpios = NULL;
> +
> +	return 0;
> +}