On Sat, Aug 30, 2025 at 3:46 AM Marcelo Schmitt
<marcelo.schmitt@xxxxxxxxxx> 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.
...
> /* Datasheet says 9.8ns, so use the closest integer value */
> #define AD4030_TQUIET_CNV_DELAY_NS 10
You already used that in one of the previous patches, can you move
there this one and use instead of magic += 10?
> +/* HARDWARE_GAIN */
> +#define ADAQ4616_PGA_PINS 2
> +#define ADAQ4616_GAIN_MAX_NANO 6666666667
Can we use calculus instead (people can't count properly after 3 :-)?
Something like this
(NANO * 2 / 3) // whoever in the above it's 20 and this puzzles me how
something with _NANO can be so big :-)
...
> +/*
> + * Gains computed as fractions of 1000 so they can be expressed by integers.
> + */
> +static const int ad4030_hw_gains[] = {
> + 333, 556, 2222, 6667,
Again, instead of comment (or in addition to) this can be written as
1000 / 3, 5000 / 9, 20000 / 9, 20000 / 3,
Let the compiler do its job.
> +};
> +
> +static const int ad4030_hw_gains_frac[4][2] = {
Drop 4
> + { 1, 3 }, /* 1/3 gain */
> + { 5, 9 }, /* 5/9 gain */
> + { 20, 9 }, /* 20/9 gain */
> + { 20, 3 }, /* 20/3 gain */
> +};
...
> +static int ad4030_write_raw_get_fmt(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + return IIO_VAL_INT_PLUS_NANO;
> + default:
> + return IIO_VAL_INT_PLUS_MICRO;
> + }
> + return -EINVAL;
What's the point of this return?
> +}
...
> +static int ad4030_setup_pga(struct device *dev, struct iio_dev *indio_dev,
> + struct ad4030_state *st)
> +{
> + unsigned int i;
> + int pga_value;
> + int ret;
> +
> + ret = device_property_read_u32(dev, "adi,pga-value", &pga_value);
> + if (ret && ret != -EINVAL)
> + return dev_err_probe(dev, ret, "Failed to get PGA value.\n"
> +
> + if (ret == -EINVAL) {
This can be done differently, i.e. check for EINVAL first and in
'else' branch check for other ret != 0. This will deduplicate the
EINVAL check.
> + /* 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 != 2)
> + return dev_err_probe(dev, -EINVAL,
> + "Expected 2 GPIOs for PGA control.\n");
> +
> + st->scale_avail_size = ARRAY_SIZE(ad4030_hw_gains);
> + st->pga_index = 0;
> + return ad4030_set_pga_gain(st);
> + }
...
> + .max_sample_rate_hz = 2 * MEGA,
HZ_PER_MHZ
...
> + .max_sample_rate_hz = 2 * MEGA,
Ditto.
--
With Best Regards,
Andy Shevchenko
