Re: [PATCH v9 7/8] iio: adc: ad4030: Add support for ADAQ4216 and ADAQ4224

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Mon, Feb 16, 2026 at 12:01:12PM -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.

...

> +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++) {

Can be

	for (unsigned int i = 0; i < ARRAY_SIZE(adaq4216_hw_gains_frac); i++) {

which is still under 80 and makes the top definition cleaner and code robust
by not exposing loop iterator out of the loop's scope.

> +		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);
> +		st->scale_avail[i][0] = int_part;
> +		st->scale_avail[i][1] = fract_part;
> +	}
> +}

...

> +static int ad4030_set_pga(struct iio_dev *indio_dev, int gain_int, int gain_fract)
> +{
> +	struct ad4030_state *st = iio_priv(indio_dev);
> +	unsigned int mag_bits = st->chip->precision_bits - 1;
> +	u64 gain_nano, tmp;
> +
> +	if (!st->pga_gpios)
> +		return -EINVAL;
> +
> +	gain_nano = gain_int * NANO + gain_fract;
> +	if (!in_range(gain_nano, 1, ADAQ4616_PGA_GAIN_MAX_NANO))
> +		return -EINVAL;
> +
> +	tmp = DIV_ROUND_CLOSEST_ULL(gain_nano << mag_bits, NANO);

> +	gain_nano = DIV_ROUND_CLOSEST_ULL(st->vref_uv, tmp);

This (second one only) doesn't sound like a 64-bit division.
Can tmp be bigger than 32-bit?

> +	st->pga_index = find_closest(gain_nano, adaq4216_hw_gains_vpv,
> +				     ARRAY_SIZE(adaq4216_hw_gains_vpv));
> +
> +	return ad4030_set_pga_gain(st);
> +}

...

> +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);
> +	if (ret == -EINVAL) {

Actually instead of custom error hunting, this should be rather

	if (device_property_present(dev, "adi,pga-gain-db")) {
		ret = device_property_read_u32(dev, "adi,pga-gain-db", &pga_gain_dB);
		if (ret)
			return dev_err_probe(dev, ret, "Failed to get PGA value.\n");
	} else {

> +		/* 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) {
> +		return dev_err_probe(dev, ret, "Failed to get PGA value.\n");
> +	}

(this goes above)

> +	/* 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;
> +}

-- 
With Best Regards,
Andy Shevchenko