linux-kernel - Re: [PATCH v5 7/7] 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: <aO-oCoCLNY7fPQEB@smile.fi.intel.com>
Date: Wed, 15 Oct 2025 16:56:26 +0300
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Marcelo Schmitt <marcelo.schmitt@...log.com>
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	jic23@...nel.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 v5 7/7] iio: adc: ad4030: Add support for ADAQ4216 and
 ADAQ4224

On Tue, Oct 14, 2025 at 07:22:51PM -0300, Marcelo Schmitt 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.

...

> +/*
> + * Gains computed as fractions of 1000 so they can be expressed by integers.
> + */
> +static const int adaq4216_hw_gains_vpv[] = {
> +	MILLI / 3,		/* 333 */
> +	(5 * MILLI / 9),	/* 555 */
> +	(20 * MILLI / 9),	/* 2222 */
> +	(20 * MILLI / 3),	/* 6666 */

Redundant parentheses, or do you mean to make multiplication first?
E.g., (5 * MILL) / 9 ?

> +};

...

> +static void ad4030_fill_scale_avail(struct ad4030_state *st)
> +{
> +	unsigned int mag_bits, int_part, fract_part, i;
> +	u64 range;
> +
> +	/*
> +	 * The maximum precision of differential channels is retrieved from the
> +	 * chip properties. The output code of differential channels is in two's
> +	 * complement format (i.e. signed), so the MSB is the sign bit and only
> +	 * (precision_bits - 1) bits express voltage magnitude.
> +	 */
> +	mag_bits = st->chip->precision_bits - 1;
> +
> +	for (i = 0; i < ARRAY_SIZE(adaq4216_hw_gains_frac); i++) {
> +		range = mult_frac(st->vref_uv, adaq4216_hw_gains_frac[i][1],
> +				  adaq4216_hw_gains_frac[i][0]);
> +		/*
> +		 * If range were in mV, we would multiply it by NANO below.
> +		 * Though, range is in µV so multiply it by MICRO only so the
> +		 * result after right shift and division scales output codes to
> +		 * millivolts.
> +		 */
> +		int_part = div_u64_rem(((u64)range * MICRO) >> mag_bits, NANO, &fract_part);

The "range" is of type u64. Any specific reason why cast?

> +		st->scale_avail[i][0] = int_part;
> +		st->scale_avail[i][1] = fract_part;
> +	}
> +}

...

> +static int ad4030_setup_pga(struct device *dev, struct iio_dev *indio_dev,
> +			    struct ad4030_state *st)
> +{
> +	/* 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");

2 --> %d and constant or __stringify(MY_COOL_CONSTANT). However, I am not sure
if the latter is acceptable in IIO.

> +
> +	st->scale_avail_size = ARRAY_SIZE(adaq4216_hw_gains_vpv);
> +	st->pga_index = 0;
> +
> +	return 0;
> +}


-- 
With Best Regards,
Andy Shevchenko