Add support for ADAQ7767/68/69-1 series, which includes PGIA and
Anti-aliasing filter (AAF) gains.
The PGA gain is configured in run-time through the scale attribute,
if supported by the device. The scale options are updated according
to the output data width.
The AAF gain is configured in the devicetree and it should correspond to
the AAF channel selected in hardware.
Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
drivers/iio/adc/ad7768-1.c | 286 ++++++++++++++++++++++++++++++++++++-
1 file changed, 279 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index d0b9764a8f92..4397d044f5de 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -16,6 +16,7 @@
#include <linux/interrupt.h>
#include <linux/minmax.h>
#include <linux/module.h>
+#include <linux/rational.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/regulator/driver.h>
@@ -98,16 +99,22 @@
/* AD7768_REG_GPIO_CONTROL */
#define AD7768_GPIO_UNIVERSAL_EN BIT(7)
#define AD7768_GPIO_CONTROL_MSK GENMASK(3, 0)
+#define AD7768_GPIO_PGIA_EN (AD7768_GPIO_UNIVERSAL_EN | GENMASK(2, 0))
/* AD7768_REG_GPIO_WRITE */
#define AD7768_GPIO_WRITE_MSK GENMASK(3, 0)
+#define AD7768_GPIO_WRITE(x) FIELD_PREP(AD7768_GPIO_WRITE_MSK, x)
/* AD7768_REG_GPIO_READ */
#define AD7768_GPIO_READ_MSK GENMASK(3, 0)
+#define AD7768_GPIO_READ(x) FIELD_PREP(AD7768_GPIO_READ_MSK, x)
#define AD7768_VCM_OFF 0x07
#define AD7768_CHAN_INFO_NONE 0
+#define ADAQ776X_GAIN_MAX_NANO (128 * NANO)
+#define ADAQ776X_MAX_GAIN_MODES 8
+
#define AD7768_TRIGGER_SOURCE_SYNC_IDX 0
#define AD7768_MAX_CHANNELS 1
@@ -154,6 +161,52 @@ enum ad7768_scan_type {
AD7768_SCAN_TYPE_HIGH_SPEED,
};
+enum {
+ AD7768_PGA_GAIN_0,
+ AD7768_PGA_GAIN_1,
+ AD7768_PGA_GAIN_2,
+ AD7768_PGA_GAIN_3,
+ AD7768_PGA_GAIN_4,
+ AD7768_PGA_GAIN_5,
+ AD7768_PGA_GAIN_6,
+ AD7768_PGA_GAIN_7,
+ AD7768_MAX_PGA_GAIN,
+};
+
+enum {
+ AD7768_AAF_IN1,
+ AD7768_AAF_IN2,
+ AD7768_AAF_IN3,
+};
+
+/* PGA and AAF gains in V/V */
+static const int adaq7768_gains[7] = {
+ [AD7768_PGA_GAIN_0] = 325, /* 0.325 */
+ [AD7768_PGA_GAIN_1] = 650, /* 0.650 */
+ [AD7768_PGA_GAIN_2] = 1300, /* 1.300 */
+ [AD7768_PGA_GAIN_3] = 2600, /* 2.600 */
+ [AD7768_PGA_GAIN_4] = 5200, /* 5.200 */
+ [AD7768_PGA_GAIN_5] = 10400, /* 10.400 */
+ [AD7768_PGA_GAIN_6] = 20800 /* 20.800 */
+};
+
+static const int adaq7769_gains[8] = {
+ [AD7768_PGA_GAIN_0] = 1000, /* 1.000 */
+ [AD7768_PGA_GAIN_1] = 2000, /* 2.000 */
+ [AD7768_PGA_GAIN_2] = 4000, /* 4.000 */
+ [AD7768_PGA_GAIN_3] = 8000, /* 8.000 */
+ [AD7768_PGA_GAIN_4] = 16000, /* 16.000 */
+ [AD7768_PGA_GAIN_5] = 32000, /* 32.000 */
+ [AD7768_PGA_GAIN_6] = 64000, /* 64.000 */
+ [AD7768_PGA_GAIN_7] = 128000 /* 128.000 */
+};
+
+static const int ad7768_aaf_gains[3] = {
+ [AD7768_AAF_IN1] = 1000, /* 1.000 */
+ [AD7768_AAF_IN2] = 364, /* 0.364 */
+ [AD7768_AAF_IN3] = 143 /* 0.143 */
+};
+
/* -3dB cutoff frequency multipliers (relative to ODR) for each filter type. */
static const int ad7768_filter_3db_odr_multiplier[] = {
[AD7768_FILTER_SINC5] = 204, /* 0.204 */
@@ -216,6 +269,12 @@ static const struct iio_scan_type ad7768_scan_type[] = {
struct ad7768_chip_info {
const char *name;
+ bool has_variable_aaf;
+ bool has_pga;
+ int num_pga_modes;
+ int default_pga_mode;
+ int pgia_mode2pin_offset;
+ const int *pga_gains;
const struct iio_chan_spec *channel_spec;
const unsigned long *available_masks;
int num_channels;
@@ -236,6 +295,9 @@ struct ad7768_state {
unsigned int samp_freq;
unsigned int samp_freq_avail[ARRAY_SIZE(ad7768_mclk_div_rates)];
unsigned int samp_freq_avail_len;
+ int pga_gain_mode;
+ int aaf_gain;
+ int scale_tbl[ADAQ776X_MAX_GAIN_MODES][2];
struct completion completion;
struct iio_trigger *trig;
struct gpio_desc *gpio_sync_in;
@@ -466,6 +528,43 @@ static int ad7768_reg_access(struct iio_dev *indio_dev,
return ret;
}
+static void ad7768_fill_scale_tbl(struct iio_dev *dev)
+{
+ struct ad7768_state *st = iio_priv(dev);
+ const struct iio_scan_type *scan_type;
+ int val, val2, tmp0, tmp1, i;
+ unsigned long denominator, numerator;
+ u64 tmp2;
+
+ scan_type = iio_get_current_scan_type(dev, &dev->channels[0]);
+ if (scan_type->sign == 's')
+ val2 = scan_type->realbits - 1;
+ else
+ val2 = scan_type->realbits;
+
+ for (i = 0; i < st->chip->num_pga_modes; i++) {
+ /* Convert gain to a fraction format */
+ numerator = st->chip->pga_gains[i];
+ denominator = MILLI;
+ if (st->chip->has_variable_aaf) {
+ numerator *= ad7768_aaf_gains[st->aaf_gain];
+ denominator *= MILLI;
+ }
+
+ rational_best_approximation(numerator, denominator, __INT_MAX__, __INT_MAX__,
+ &numerator, &denominator);
+
+ val = st->vref_uv / 1000;
+ /* Multiply by MILLI here to avoid losing precision */
+ val = mult_frac(val, denominator * MILLI, numerator);
+ /* Would multiply by NANO here but we already multiplied by MILLI */
+ tmp2 = shift_right((u64)val * MICRO, val2);
+ tmp0 = (int)div_s64_rem(tmp2, NANO, &tmp1);
+ st->scale_tbl[i][0] = tmp0; /* Integer part */
+ st->scale_tbl[i][1] = abs(tmp1); /* Fractional part */
+ }
+}
+
static int ad7768_set_sinc3_dec_rate(struct ad7768_state *st,
unsigned int dec_rate)
{
@@ -567,12 +666,68 @@ static int ad7768_configure_dig_fil(struct iio_dev *dev,
st->oversampling_ratio = ad7768_dec_rate_values[dec_rate_idx];
}
+ /* Update scale table: scale values vary according to the precision */
+ ad7768_fill_scale_tbl(dev);
+
ad7768_fill_samp_freq_tbl(st);
/* A sync-in pulse is required after every configuration change */
return ad7768_send_sync_pulse(st);
}
+static int ad7768_calc_pga_gain(struct ad7768_state *st, int gain_int,
+ int gain_fract, int precision)
+{
+ u64 gain_nano, tmp;
+ int gain_idx;
+
+ precision--;
+ gain_nano = gain_int * NANO + gain_fract;
+ if (gain_nano < 0 || gain_nano > ADAQ776X_GAIN_MAX_NANO)
+ return -EINVAL;
+
+ tmp = DIV_ROUND_CLOSEST_ULL(gain_nano << precision, NANO);
+ gain_nano = DIV_ROUND_CLOSEST_ULL(st->vref_uv, tmp);
+ if (st->chip->has_variable_aaf)
+ /* remove the AAF gain from the overall gain */
+ gain_nano = DIV_ROUND_CLOSEST_ULL(gain_nano * MILLI,
+ ad7768_aaf_gains[st->aaf_gain]);
+ tmp = st->chip->num_pga_modes;
+ gain_idx = find_closest(gain_nano, st->chip->pga_gains, tmp);
+
+ return gain_idx;
+}
+
+static int ad7768_set_pga_gain(struct ad7768_state *st,
+ int gain_mode)
+{
+ int pgia_pins_value = abs(gain_mode - st->chip->pgia_mode2pin_offset);
+ int check_val;
+ int ret;
+
+ /* Check GPIO control register */
+ ret = regmap_read(st->regmap, AD7768_REG_GPIO_CONTROL, &check_val);
+ if (ret < 0)
+ return ret;
+
+ if ((check_val & AD7768_GPIO_PGIA_EN) != AD7768_GPIO_PGIA_EN) {
+ /* Enable PGIA GPIOs and set them as output */
+ ret = regmap_write(st->regmap, AD7768_REG_GPIO_CONTROL, AD7768_GPIO_PGIA_EN);
+ if (ret < 0)
+ return ret;
+ }
+
+ /* Write the respective gain values to GPIOs 0, 1, 2 */
+ ret = regmap_write(st->regmap, AD7768_REG_GPIO_WRITE,
+ AD7768_GPIO_WRITE(pgia_pins_value));
+ if (ret < 0)
+ return ret;
+
+ st->pga_gain_mode = gain_mode;
+
+ return 0;
+}
+
static int ad7768_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
{
struct iio_dev *indio_dev = gpiochip_get_data(chip);
@@ -782,13 +937,17 @@ static const struct iio_chan_spec ad7768_channels[] = {
AD7768_CHAN(0, AD7768_CHAN_INFO_NONE),
};
+static const struct iio_chan_spec adaq776x_channels[] = {
+ AD7768_CHAN(0, BIT(IIO_CHAN_INFO_SCALE)),
+};
+
static int ad7768_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int *val, int *val2, long info)
{
struct ad7768_state *st = iio_priv(indio_dev);
const struct iio_scan_type *scan_type;
- int scale_uv, ret, temp;
+ int ret, temp;
scan_type = iio_get_current_scan_type(indio_dev, chan);
if (IS_ERR(scan_type))
@@ -809,12 +968,19 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
- scale_uv = st->vref_uv;
- if (scale_uv < 0)
- return scale_uv;
-
- *val = (scale_uv * 2) / 1000;
- *val2 = scan_type->realbits;
+ if (st->chip->has_pga) {
+ *val = st->scale_tbl[st->pga_gain_mode][0];
+ *val2 = st->scale_tbl[st->pga_gain_mode][1];
+ return IIO_VAL_INT_PLUS_NANO;
+ }
+ *val = st->vref_uv / 1000;
+ if (st->chip->has_variable_aaf)
+ *val = (*val * MILLI) / ad7768_aaf_gains[st->aaf_gain];
+ /*
+ * ADC output code is two's complement so only (realbits - 1)
+ * bits express voltage magnitude.
+ */
+ *val2 = scan_type->realbits - 1;
return IIO_VAL_FRACTIONAL_LOG2;
@@ -869,18 +1035,42 @@ static int ad7768_read_avail(struct iio_dev *indio_dev,
*length = st->samp_freq_avail_len;
*type = IIO_VAL_INT;
return IIO_AVAIL_LIST;
+ case IIO_CHAN_INFO_SCALE:
+ *vals = (int *)st->scale_tbl;
+ *length = st->chip->num_pga_modes * 2;
+ *type = IIO_VAL_INT_PLUS_NANO;
+ return IIO_AVAIL_LIST;
default:
return -EINVAL;
}
}
+static int ad7768_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;
+}
+
static int __ad7768_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int val, int val2, long info)
{
struct ad7768_state *st = iio_priv(indio_dev);
+ const struct iio_scan_type *scan_type;
+ int gain_mode;
int ret;
+ scan_type = iio_get_current_scan_type(indio_dev, chan);
+ if (IS_ERR(scan_type))
+ return PTR_ERR(scan_type);
+
switch (info) {
case IIO_CHAN_INFO_SAMP_FREQ:
return ad7768_set_freq(st, val);
@@ -892,6 +1082,13 @@ static int __ad7768_write_raw(struct iio_dev *indio_dev,
/* Update sampling frequency */
return ad7768_set_freq(st, st->samp_freq);
+ case IIO_CHAN_INFO_SCALE:
+ if (!st->chip->has_pga)
+ return -EOPNOTSUPP;
+
+ gain_mode = ad7768_calc_pga_gain(st, val, val2,
+ scan_type->realbits);
+ return ad7768_set_pga_gain(st, gain_mode);
default:
return -EINVAL;
}
@@ -933,6 +1130,7 @@ static const struct iio_info ad7768_info = {
.read_raw = &ad7768_read_raw,
.read_avail = &ad7768_read_avail,
.write_raw = &ad7768_write_raw,
+ .write_raw_get_fmt = &ad7768_write_raw_get_fmt,
.read_label = ad7768_read_label,
.get_current_scan_type = &ad7768_get_current_scan_type,
.debugfs_reg_access = &ad7768_reg_access,
@@ -1351,10 +1549,46 @@ static const struct ad7768_chip_info ad7768_chip_info = {
.available_masks = ad7768_channel_masks,
};
+static const struct ad7768_chip_info adaq7767_chip_info = {
+ .name = "adaq7767-1",
+ .channel_spec = ad7768_channels,
+ .num_channels = ARRAY_SIZE(ad7768_channels),
+ .available_masks = ad7768_channel_masks,
+ .has_pga = false,
+ .has_variable_aaf = true
+};
+
+static const struct ad7768_chip_info adaq7768_chip_info = {
+ .name = "adaq7768-1",
+ .channel_spec = adaq776x_channels,
+ .num_channels = ARRAY_SIZE(adaq776x_channels),
+ .available_masks = ad7768_channel_masks,
+ .pga_gains = adaq7768_gains,
+ .default_pga_mode = AD7768_PGA_GAIN_2,
+ .num_pga_modes = ARRAY_SIZE(adaq7768_gains),
+ .pgia_mode2pin_offset = 6,
+ .has_pga = true,
+ .has_variable_aaf = false
+};
+
+static const struct ad7768_chip_info adaq7769_chip_info = {
+ .name = "adaq7769-1",
+ .channel_spec = adaq776x_channels,
+ .num_channels = ARRAY_SIZE(adaq776x_channels),
+ .available_masks = ad7768_channel_masks,
+ .pga_gains = adaq7769_gains,
+ .default_pga_mode = AD7768_PGA_GAIN_0,
+ .num_pga_modes = ARRAY_SIZE(adaq7769_gains),
+ .pgia_mode2pin_offset = 0,
+ .has_pga = true,
+ .has_variable_aaf = true
+};
+
static int ad7768_probe(struct spi_device *spi)
{
struct ad7768_state *st;
struct iio_dev *indio_dev;
+ u32 val;
int ret;
indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
@@ -1418,6 +1652,35 @@ static int ad7768_probe(struct spi_device *spi)
if (ret)
return ret;
+ st->aaf_gain = AD7768_AAF_IN1;
+ ret = device_property_read_u32(&spi->dev, "adi,aaf-gain", &val);
+ if (ret) {
+ /* AAF gain required, but not specified */
+ if (st->chip->has_variable_aaf)
+ return dev_err_probe(&spi->dev, -EINVAL, "AAF gain not specified\n");
+
+ } else if (!st->chip->has_variable_aaf) {
+ /* AAF gain provided, but not supported */
+ return dev_err_probe(&spi->dev, -EINVAL, "AAF gain not supported for %s\n",
+ st->chip->name);
+ } else {
+ /* Device supports variable AAF gain, validate and set the gain */
+ switch (val) {
+ case 1000:
+ st->aaf_gain = AD7768_AAF_IN1;
+ break;
+ case 364:
+ st->aaf_gain = AD7768_AAF_IN2;
+ break;
+ case 143:
+ st->aaf_gain = AD7768_AAF_IN3;
+ break;
+ default:
+ return dev_err_probe(&spi->dev, -EINVAL,
+ "Invalid firmware provided gain\n");
+ }
+ }
+
ret = ad7768_setup(indio_dev);
if (ret < 0) {
dev_err(&spi->dev, "AD7768 setup failed\n");
@@ -1426,6 +1689,9 @@ static int ad7768_probe(struct spi_device *spi)
init_completion(&st->completion);
+ if (st->chip->has_pga)
+ ad7768_set_pga_gain(st, st->chip->default_pga_mode);
+
ret = ad7768_set_channel_label(indio_dev, st->chip->num_channels);
if (ret)
return ret;
@@ -1446,12 +1712,18 @@ static int ad7768_probe(struct spi_device *spi)
static const struct spi_device_id ad7768_id_table[] = {
{ "ad7768-1", (kernel_ulong_t)&ad7768_chip_info },
+ { "adaq7767-1", (kernel_ulong_t)&adaq7767_chip_info },
+ { "adaq7768-1", (kernel_ulong_t)&adaq7768_chip_info },
+ { "adaq7769-1", (kernel_ulong_t)&adaq7769_chip_info },
{ }
};
MODULE_DEVICE_TABLE(spi, ad7768_id_table);
static const struct of_device_id ad7768_of_match[] = {
{ .compatible = "adi,ad7768-1", .data = &ad7768_chip_info },
+ { .compatible = "adi,adaq7767-1", .data = &adaq7767_chip_info },
+ { .compatible = "adi,adaq7768-1", .data = &adaq7768_chip_info },
+ { .compatible = "adi,adaq7769-1", .data = &adaq7769_chip_info },
{ }
};
MODULE_DEVICE_TABLE(of, ad7768_of_match);
--
2.34.1
Hi Jonathan, A few thoughts from me. On 08/12, Jonathan Santos wrote: > Add support for ADAQ7767/68/69-1 series, which includes PGIA and > Anti-aliasing filter (AAF) gains. > > The PGA gain is configured in run-time through the scale attribute, > if supported by the device. The scale options are updated according > to the output data width. This could provide more information/context. How the PGA is controlled/configured? > > The AAF gain is configured in the devicetree and it should correspond to > the AAF channel selected in hardware. Would this be more clear if stated the other way around? The AAF gain is defined by hardware connections and thus is specified in device tree. ? > > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> > --- > drivers/iio/adc/ad7768-1.c | 286 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 279 insertions(+), 7 deletions(-) > ... > > +static void ad7768_fill_scale_tbl(struct iio_dev *dev) > +{ > + struct ad7768_state *st = iio_priv(dev); > + const struct iio_scan_type *scan_type; > + int val, val2, tmp0, tmp1, i; > + unsigned long denominator, numerator; > + u64 tmp2; > + > + scan_type = iio_get_current_scan_type(dev, &dev->channels[0]); > + if (scan_type->sign == 's') > + val2 = scan_type->realbits - 1; > + else > + val2 = scan_type->realbits; > + > + for (i = 0; i < st->chip->num_pga_modes; i++) { > + /* Convert gain to a fraction format */ > + numerator = st->chip->pga_gains[i]; > + denominator = MILLI; > + if (st->chip->has_variable_aaf) { > + numerator *= ad7768_aaf_gains[st->aaf_gain]; > + denominator *= MILLI; > + } > + > + rational_best_approximation(numerator, denominator, __INT_MAX__, __INT_MAX__, > + &numerator, &denominator); > + > + val = st->vref_uv / 1000; What about keeping the reference in µV (not dividing by MILLI)? > + /* Multiply by MILLI here to avoid losing precision */ > + val = mult_frac(val, denominator * MILLI, numerator); then this can be just val = mult_frac(val, denominator, numerator); > + /* Would multiply by NANO here but we already multiplied by MILLI */ > + tmp2 = shift_right((u64)val * MICRO, val2); > + tmp0 = (int)div_s64_rem(tmp2, NANO, &tmp1); val is never negative here so can use div_u64_rem() or maybe do_div(). > + st->scale_tbl[i][0] = tmp0; /* Integer part */ > + st->scale_tbl[i][1] = abs(tmp1); /* Fractional part */ > + } > +} > + ... > > +static int ad7768_calc_pga_gain(struct ad7768_state *st, int gain_int, > + int gain_fract, int precision) > +{ > + u64 gain_nano, tmp; > + int gain_idx; > + > + precision--; This is odd out of context. Also, it only applies to ADCs that provide output codes in two's complement format. See comment below. > + gain_nano = gain_int * NANO + gain_fract; > + if (gain_nano < 0 || gain_nano > ADAQ776X_GAIN_MAX_NANO) I've seen some build tools complain about comparisons like gain_nano < 0 with gain_nano being u64. Since that's unsigned, it can never be < 0. And in the context of gain/attenuation, we know gain_nano shall never be negative. Would just drop the gain_nano < 0 comparison. Or maybe clamp() the value? > + return -EINVAL; > + > + tmp = DIV_ROUND_CLOSEST_ULL(gain_nano << precision, NANO); > + gain_nano = DIV_ROUND_CLOSEST_ULL(st->vref_uv, tmp); > + if (st->chip->has_variable_aaf) > + /* remove the AAF gain from the overall gain */ > + gain_nano = DIV_ROUND_CLOSEST_ULL(gain_nano * MILLI, > + ad7768_aaf_gains[st->aaf_gain]); > + tmp = st->chip->num_pga_modes; > + gain_idx = find_closest(gain_nano, st->chip->pga_gains, tmp); > + > + return gain_idx; > +} > + ... > + > + /* Write the respective gain values to GPIOs 0, 1, 2 */ > + ret = regmap_write(st->regmap, AD7768_REG_GPIO_WRITE, > + AD7768_GPIO_WRITE(pgia_pins_value)); > + if (ret < 0) I think regmap_write() doesn't return positive values so we can have 'if (ret)' here. > + return ret; > + > + st->pga_gain_mode = gain_mode; > + > + return 0; > +} > + ... > case IIO_CHAN_INFO_SCALE: > - scale_uv = st->vref_uv; > - if (scale_uv < 0) > - return scale_uv; > - > - *val = (scale_uv * 2) / 1000; > - *val2 = scan_type->realbits; > + if (st->chip->has_pga) { > + *val = st->scale_tbl[st->pga_gain_mode][0]; > + *val2 = st->scale_tbl[st->pga_gain_mode][1]; > + return IIO_VAL_INT_PLUS_NANO; > + } > + *val = st->vref_uv / 1000; > + if (st->chip->has_variable_aaf) > + *val = (*val * MILLI) / ad7768_aaf_gains[st->aaf_gain]; Similar thing here. Would it make sense to use st->vref_uv without dividing it by MILLI? > + /* > + * ADC output code is two's complement so only (realbits - 1) > + * bits express voltage magnitude. > + */ > + *val2 = scan_type->realbits - 1; I see the rationally for this. Instead of doing 'scale_uv * 2' to account for the range of negative values, this is now using one less precision bit which shall lead to the same result after going through IIO_VAL_FRACTIONAL_LOG2 handling. I personally prefer the realbits - 1 logic, but others may prefer to avoid this change since it was already working with 'scale_uv * 2'. > > return IIO_VAL_FRACTIONAL_LOG2; > > @@ -869,18 +1035,42 @@ static int ad7768_read_avail(struct iio_dev *indio_dev, > *length = st->samp_freq_avail_len; > *type = IIO_VAL_INT; > return IIO_AVAIL_LIST; > + case IIO_CHAN_INFO_SCALE: > + *vals = (int *)st->scale_tbl; > + *length = st->chip->num_pga_modes * 2; > + *type = IIO_VAL_INT_PLUS_NANO; > + return IIO_AVAIL_LIST; > default: > return -EINVAL; > } > } > ... > @@ -892,6 +1082,13 @@ static int __ad7768_write_raw(struct iio_dev *indio_dev, > > /* Update sampling frequency */ > return ad7768_set_freq(st, st->samp_freq); > + case IIO_CHAN_INFO_SCALE: > + if (!st->chip->has_pga) > + return -EOPNOTSUPP; > + > + gain_mode = ad7768_calc_pga_gain(st, val, val2, > + scan_type->realbits); Check scan_type.sign and pass scan_type->realbits - 1 to ad7768_calc_pga_gain() if the ADC output codes are in two's complement format. > + return ad7768_set_pga_gain(st, gain_mode); > default: > return -EINVAL; > } ... > +static const struct ad7768_chip_info adaq7767_chip_info = { > + .name = "adaq7767-1", > + .channel_spec = ad7768_channels, > + .num_channels = ARRAY_SIZE(ad7768_channels), > + .available_masks = ad7768_channel_masks, > + .has_pga = false, I think these flag initialization can be omitted when they are false. > + .has_variable_aaf = true > +}; > + > +static const struct ad7768_chip_info adaq7768_chip_info = { > + .name = "adaq7768-1", > + .channel_spec = adaq776x_channels, > + .num_channels = ARRAY_SIZE(adaq776x_channels), > + .available_masks = ad7768_channel_masks, > + .pga_gains = adaq7768_gains, > + .default_pga_mode = AD7768_PGA_GAIN_2, > + .num_pga_modes = ARRAY_SIZE(adaq7768_gains), > + .pgia_mode2pin_offset = 6, > + .has_pga = true, > + .has_variable_aaf = false Same here. > +}; > + ... > @@ -1418,6 +1652,35 @@ static int ad7768_probe(struct spi_device *spi) > if (ret) > return ret; > > + st->aaf_gain = AD7768_AAF_IN1; > + ret = device_property_read_u32(&spi->dev, "adi,aaf-gain", &val); > + if (ret) { > + /* AAF gain required, but not specified */ > + if (st->chip->has_variable_aaf) > + return dev_err_probe(&spi->dev, -EINVAL, "AAF gain not specified\n"); > + > + } else if (!st->chip->has_variable_aaf) { > + /* AAF gain provided, but not supported */ > + return dev_err_probe(&spi->dev, -EINVAL, "AAF gain not supported for %s\n", > + st->chip->name); Not really sure what to do in these cases. Can't we just ignore or warn on properties for unsupported features? Best regards, Marcelo
On Tue, Aug 19, 2025 at 10:14 PM Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote: > On 08/12, Jonathan Santos wrote: > > Add support for ADAQ7767/68/69-1 series, which includes PGIA and ... > > +static int ad7768_calc_pga_gain(struct ad7768_state *st, int gain_int, > > + int gain_fract, int precision) > > +{ > > + u64 gain_nano, tmp; > > + int gain_idx; > > + > > + precision--; > This is odd out of context. > Also, it only applies to ADCs that provide output codes in two's complement > format. See comment below. > > > > + gain_nano = gain_int * NANO + gain_fract; > > + if (gain_nano < 0 || gain_nano > ADAQ776X_GAIN_MAX_NANO) > I've seen some build tools complain about comparisons like gain_nano < 0 with > gain_nano being u64. Since that's unsigned, it can never be < 0. And in the > context of gain/attenuation, we know gain_nano shall never be negative. > Would just drop the gain_nano < 0 comparison. Or maybe clamp() the value? in_range() can be used as well. > > + return -EINVAL; > > + > > + tmp = DIV_ROUND_CLOSEST_ULL(gain_nano << precision, NANO); > > + gain_nano = DIV_ROUND_CLOSEST_ULL(st->vref_uv, tmp); > > + if (st->chip->has_variable_aaf) > > + /* remove the AAF gain from the overall gain */ > > + gain_nano = DIV_ROUND_CLOSEST_ULL(gain_nano * MILLI, > > + ad7768_aaf_gains[st->aaf_gain]); > > + tmp = st->chip->num_pga_modes; > > + gain_idx = find_closest(gain_nano, st->chip->pga_gains, tmp); > > + > > + return gain_idx; > > +} -- With Best Regards, Andy Shevchenko
On Tue, 2025-08-12 at 23:49 -0300, Jonathan Santos wrote: > Add support for ADAQ7767/68/69-1 series, which includes PGIA and > Anti-aliasing filter (AAF) gains. > > The PGA gain is configured in run-time through the scale attribute, > if supported by the device. The scale options are updated according > to the output data width. > > The AAF gain is configured in the devicetree and it should correspond to > the AAF channel selected in hardware. > > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> > --- Hi Jonathan, Some comments from me... > drivers/iio/adc/ad7768-1.c | 286 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 279 insertions(+), 7 deletions(-) > > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c > index d0b9764a8f92..4397d044f5de 100644 > --- a/drivers/iio/adc/ad7768-1.c > +++ b/drivers/iio/adc/ad7768-1.c > @@ -16,6 +16,7 @@ > #include <linux/interrupt.h> > #include <linux/minmax.h> > #include <linux/module.h> > +#include <linux/rational.h> > #include <linux/regmap.h> > #include <linux/regulator/consumer.h> > #include <linux/regulator/driver.h> > @@ -98,16 +99,22 @@ > /* AD7768_REG_GPIO_CONTROL */ > #define AD7768_GPIO_UNIVERSAL_EN BIT(7) > #define AD7768_GPIO_CONTROL_MSK GENMASK(3, 0) > +#define AD7768_GPIO_PGIA_EN (AD7768_GPIO_UNIVERSAL_EN | GENMASK(2, 0)) > > /* AD7768_REG_GPIO_WRITE */ > #define AD7768_GPIO_WRITE_MSK GENMASK(3, 0) > +#define AD7768_GPIO_WRITE(x) FIELD_PREP(AD7768_GPIO_WRITE_MSK, x) > > /* AD7768_REG_GPIO_READ */ > #define AD7768_GPIO_READ_MSK GENMASK(3, 0) > +#define AD7768_GPIO_READ(x) FIELD_PREP(AD7768_GPIO_READ_MSK, x) > > #define AD7768_VCM_OFF 0x07 > #define AD7768_CHAN_INFO_NONE 0 > > +#define ADAQ776X_GAIN_MAX_NANO (128 * NANO) > +#define ADAQ776X_MAX_GAIN_MODES 8 > + > #define AD7768_TRIGGER_SOURCE_SYNC_IDX 0 > > #define AD7768_MAX_CHANNELS 1 > @@ -154,6 +161,52 @@ enum ad7768_scan_type { > AD7768_SCAN_TYPE_HIGH_SPEED, > }; > > +enum { > + AD7768_PGA_GAIN_0, > + AD7768_PGA_GAIN_1, > + AD7768_PGA_GAIN_2, > + AD7768_PGA_GAIN_3, > + AD7768_PGA_GAIN_4, > + AD7768_PGA_GAIN_5, > + AD7768_PGA_GAIN_6, > + AD7768_PGA_GAIN_7, > + AD7768_MAX_PGA_GAIN, > +}; > + > +enum { > + AD7768_AAF_IN1, > + AD7768_AAF_IN2, > + AD7768_AAF_IN3, > +}; > + > +/* PGA and AAF gains in V/V */ > +static const int adaq7768_gains[7] = { > + [AD7768_PGA_GAIN_0] = 325, /* 0.325 */ > + [AD7768_PGA_GAIN_1] = 650, /* 0.650 */ > + [AD7768_PGA_GAIN_2] = 1300, /* 1.300 */ > + [AD7768_PGA_GAIN_3] = 2600, /* 2.600 */ > + [AD7768_PGA_GAIN_4] = 5200, /* 5.200 */ > + [AD7768_PGA_GAIN_5] = 10400, /* 10.400 */ > + [AD7768_PGA_GAIN_6] = 20800 /* 20.800 */ > +}; > + > +static const int adaq7769_gains[8] = { > + [AD7768_PGA_GAIN_0] = 1000, /* 1.000 */ > + [AD7768_PGA_GAIN_1] = 2000, /* 2.000 */ > + [AD7768_PGA_GAIN_2] = 4000, /* 4.000 */ > + [AD7768_PGA_GAIN_3] = 8000, /* 8.000 */ > + [AD7768_PGA_GAIN_4] = 16000, /* 16.000 */ > + [AD7768_PGA_GAIN_5] = 32000, /* 32.000 */ > + [AD7768_PGA_GAIN_6] = 64000, /* 64.000 */ > + [AD7768_PGA_GAIN_7] = 128000 /* 128.000 */ > +}; > + > +static const int ad7768_aaf_gains[3] = { > + [AD7768_AAF_IN1] = 1000, /* 1.000 */ > + [AD7768_AAF_IN2] = 364, /* 0.364 */ > + [AD7768_AAF_IN3] = 143 /* 0.143 */ > +}; > + > /* -3dB cutoff frequency multipliers (relative to ODR) for each filter type. */ > static const int ad7768_filter_3db_odr_multiplier[] = { > [AD7768_FILTER_SINC5] = 204, /* 0.204 */ > @@ -216,6 +269,12 @@ static const struct iio_scan_type ad7768_scan_type[] = { > > struct ad7768_chip_info { > const char *name; > + bool has_variable_aaf; > + bool has_pga; > + int num_pga_modes; > + int default_pga_mode; > + int pgia_mode2pin_offset; > + const int *pga_gains; > const struct iio_chan_spec *channel_spec; > const unsigned long *available_masks; > int num_channels; > @@ -236,6 +295,9 @@ struct ad7768_state { > unsigned int samp_freq; > unsigned int samp_freq_avail[ARRAY_SIZE(ad7768_mclk_div_rates)]; > unsigned int samp_freq_avail_len; > + int pga_gain_mode; > + int aaf_gain; > + int scale_tbl[ADAQ776X_MAX_GAIN_MODES][2]; > struct completion completion; > struct iio_trigger *trig; > struct gpio_desc *gpio_sync_in; > @@ -466,6 +528,43 @@ static int ad7768_reg_access(struct iio_dev *indio_dev, > return ret; > } > > +static void ad7768_fill_scale_tbl(struct iio_dev *dev) > +{ > + struct ad7768_state *st = iio_priv(dev); > + const struct iio_scan_type *scan_type; > + int val, val2, tmp0, tmp1, i; > + unsigned long denominator, numerator; > + u64 tmp2; > + > + scan_type = iio_get_current_scan_type(dev, &dev->channels[0]); > + if (scan_type->sign == 's') > + val2 = scan_type->realbits - 1; > + else > + val2 = scan_type->realbits; > + > + for (i = 0; i < st->chip->num_pga_modes; i++) { > + /* Convert gain to a fraction format */ > + numerator = st->chip->pga_gains[i]; > + denominator = MILLI; > + if (st->chip->has_variable_aaf) { > + numerator *= ad7768_aaf_gains[st->aaf_gain]; > + denominator *= MILLI; > + } > + > + rational_best_approximation(numerator, denominator, __INT_MAX__, > __INT_MAX__, > + &numerator, &denominator); > + Hmm, I would user kernel limits.h (INT_MAX) > + val = st->vref_uv / 1000; > + /* Multiply by MILLI here to avoid losing precision */ > + val = mult_frac(val, denominator * MILLI, numerator); > + /* Would multiply by NANO here but we already multiplied by MILLI > */ > + tmp2 = shift_right((u64)val * MICRO, val2); > + tmp0 = (int)div_s64_rem(tmp2, NANO, &tmp1); > + st->scale_tbl[i][0] = tmp0; /* Integer part */ > + st->scale_tbl[i][1] = abs(tmp1); /* Fractional part */ > + } > +} > + > static int ad7768_set_sinc3_dec_rate(struct ad7768_state *st, > unsigned int dec_rate) > { > @@ -567,12 +666,68 @@ static int ad7768_configure_dig_fil(struct iio_dev *dev, > st->oversampling_ratio = ad7768_dec_rate_values[dec_rate_idx]; > } > > + /* Update scale table: scale values vary according to the precision */ > + ad7768_fill_scale_tbl(dev); > + > ad7768_fill_samp_freq_tbl(st); > > /* A sync-in pulse is required after every configuration change */ > return ad7768_send_sync_pulse(st); > } > > +static int ad7768_calc_pga_gain(struct ad7768_state *st, int gain_int, > + int gain_fract, int precision) > +{ > + u64 gain_nano, tmp; > + int gain_idx; > + > + precision--; > + gain_nano = gain_int * NANO + gain_fract; > + if (gain_nano < 0 || gain_nano > ADAQ776X_GAIN_MAX_NANO) > + return -EINVAL; > + > + tmp = DIV_ROUND_CLOSEST_ULL(gain_nano << precision, NANO); > + gain_nano = DIV_ROUND_CLOSEST_ULL(st->vref_uv, tmp); Does not look like we need u64 divison above... > + if (st->chip->has_variable_aaf) > + /* remove the AAF gain from the overall gain */ > + gain_nano = DIV_ROUND_CLOSEST_ULL(gain_nano * MILLI, > + ad7768_aaf_gains[st->aaf_gain]); > + tmp = st->chip->num_pga_modes; > + gain_idx = find_closest(gain_nano, st->chip->pga_gains, tmp); > + > + return gain_idx; > +} > + > +static int ad7768_set_pga_gain(struct ad7768_state *st, > + int gain_mode) > +{ > + int pgia_pins_value = abs(gain_mode - st->chip->pgia_mode2pin_offset); > + int check_val; > + int ret; > + > + /* Check GPIO control register */ > + ret = regmap_read(st->regmap, AD7768_REG_GPIO_CONTROL, &check_val); > + if (ret < 0) > + return ret; > + > + if ((check_val & AD7768_GPIO_PGIA_EN) != AD7768_GPIO_PGIA_EN) { > + /* Enable PGIA GPIOs and set them as output */ > + ret = regmap_write(st->regmap, AD7768_REG_GPIO_CONTROL, > AD7768_GPIO_PGIA_EN); > + if (ret < 0) > + return ret; > + } > + > + /* Write the respective gain values to GPIOs 0, 1, 2 */ > + ret = regmap_write(st->regmap, AD7768_REG_GPIO_WRITE, > + AD7768_GPIO_WRITE(pgia_pins_value)); > + if (ret < 0) > + return ret; > + > + st->pga_gain_mode = gain_mode; > + It looks the above function could use some locking. > + return 0; > +} > + > static int ad7768_gpio_direction_input(struct gpio_chip *chip, unsigned int > offset) > { > struct iio_dev *indio_dev = gpiochip_get_data(chip); > @@ -782,13 +937,17 @@ static const struct iio_chan_spec ad7768_channels[] = { > AD7768_CHAN(0, AD7768_CHAN_INFO_NONE), > }; > > +static const struct iio_chan_spec adaq776x_channels[] = { > + AD7768_CHAN(0, BIT(IIO_CHAN_INFO_SCALE)), > +}; > + > static int ad7768_read_raw(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan, > int *val, int *val2, long info) > { > struct ad7768_state *st = iio_priv(indio_dev); > const struct iio_scan_type *scan_type; > - int scale_uv, ret, temp; > + int ret, temp; > > scan_type = iio_get_current_scan_type(indio_dev, chan); > if (IS_ERR(scan_type)) > @@ -809,12 +968,19 @@ static int ad7768_read_raw(struct iio_dev *indio_dev, > return IIO_VAL_INT; > > case IIO_CHAN_INFO_SCALE: > - scale_uv = st->vref_uv; > - if (scale_uv < 0) > - return scale_uv; > - > - *val = (scale_uv * 2) / 1000; > - *val2 = scan_type->realbits; > + if (st->chip->has_pga) { > + *val = st->scale_tbl[st->pga_gain_mode][0]; > + *val2 = st->scale_tbl[st->pga_gain_mode][1]; > + return IIO_VAL_INT_PLUS_NANO; > + } > + *val = st->vref_uv / 1000; > + if (st->chip->has_variable_aaf) > + *val = (*val * MILLI) / ad7768_aaf_gains[st->aaf_gain]; > + /* > + * ADC output code is two's complement so only (realbits - 1) > + * bits express voltage magnitude. > + */ > + *val2 = scan_type->realbits - 1; > I'm a bit confused. Is there something wrong with the original code that needs to be fixed? The above change seems to effectively change behavior of the code with had before. - Nuno Sá > return IIO_VAL_FRACTIONAL_LOG2; > > @@ -869,18 +1035,42 @@ static int ad7768_read_avail(struct iio_dev *indio_dev, > *length = st->samp_freq_avail_len; > *type = IIO_VAL_INT; > return IIO_AVAIL_LIST; > + case IIO_CHAN_INFO_SCALE: > + *vals = (int *)st->scale_tbl; > + *length = st->chip->num_pga_modes * 2; > + *type = IIO_VAL_INT_PLUS_NANO; > + return IIO_AVAIL_LIST; > default: > return -EINVAL; > } > } > > +static int ad7768_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; > +} > + > static int __ad7768_write_raw(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan, > int val, int val2, long info) > { > struct ad7768_state *st = iio_priv(indio_dev); > + const struct iio_scan_type *scan_type; > + int gain_mode; > int ret; > > + scan_type = iio_get_current_scan_type(indio_dev, chan); > + if (IS_ERR(scan_type)) > + return PTR_ERR(scan_type); > + > switch (info) { > case IIO_CHAN_INFO_SAMP_FREQ: > return ad7768_set_freq(st, val); > @@ -892,6 +1082,13 @@ static int __ad7768_write_raw(struct iio_dev *indio_dev, > > /* Update sampling frequency */ > return ad7768_set_freq(st, st->samp_freq); > + case IIO_CHAN_INFO_SCALE: > + if (!st->chip->has_pga) > + return -EOPNOTSUPP; > + > + gain_mode = ad7768_calc_pga_gain(st, val, val2, > + scan_type->realbits); > + return ad7768_set_pga_gain(st, gain_mode); > default: > return -EINVAL; > } > @@ -933,6 +1130,7 @@ static const struct iio_info ad7768_info = { > .read_raw = &ad7768_read_raw, > .read_avail = &ad7768_read_avail, > .write_raw = &ad7768_write_raw, > + .write_raw_get_fmt = &ad7768_write_raw_get_fmt, > .read_label = ad7768_read_label, > .get_current_scan_type = &ad7768_get_current_scan_type, > .debugfs_reg_access = &ad7768_reg_access, > @@ -1351,10 +1549,46 @@ static const struct ad7768_chip_info ad7768_chip_info = { > .available_masks = ad7768_channel_masks, > }; > > +static const struct ad7768_chip_info adaq7767_chip_info = { > + .name = "adaq7767-1", > + .channel_spec = ad7768_channels, > + .num_channels = ARRAY_SIZE(ad7768_channels), > + .available_masks = ad7768_channel_masks, > + .has_pga = false, > + .has_variable_aaf = true > +}; > + > +static const struct ad7768_chip_info adaq7768_chip_info = { > + .name = "adaq7768-1", > + .channel_spec = adaq776x_channels, > + .num_channels = ARRAY_SIZE(adaq776x_channels), > + .available_masks = ad7768_channel_masks, > + .pga_gains = adaq7768_gains, > + .default_pga_mode = AD7768_PGA_GAIN_2, > + .num_pga_modes = ARRAY_SIZE(adaq7768_gains), > + .pgia_mode2pin_offset = 6, > + .has_pga = true, > + .has_variable_aaf = false > +}; > + > +static const struct ad7768_chip_info adaq7769_chip_info = { > + .name = "adaq7769-1", > + .channel_spec = adaq776x_channels, > + .num_channels = ARRAY_SIZE(adaq776x_channels), > + .available_masks = ad7768_channel_masks, > + .pga_gains = adaq7769_gains, > + .default_pga_mode = AD7768_PGA_GAIN_0, > + .num_pga_modes = ARRAY_SIZE(adaq7769_gains), > + .pgia_mode2pin_offset = 0, > + .has_pga = true, > + .has_variable_aaf = true > +}; > + > static int ad7768_probe(struct spi_device *spi) > { > struct ad7768_state *st; > struct iio_dev *indio_dev; > + u32 val; > int ret; > > indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > @@ -1418,6 +1652,35 @@ static int ad7768_probe(struct spi_device *spi) > if (ret) > return ret; > > + st->aaf_gain = AD7768_AAF_IN1; > + ret = device_property_read_u32(&spi->dev, "adi,aaf-gain", &val); > + if (ret) { > + /* AAF gain required, but not specified */ > + if (st->chip->has_variable_aaf) > + return dev_err_probe(&spi->dev, -EINVAL, "AAF gain not > specified\n"); > + > + } else if (!st->chip->has_variable_aaf) { > + /* AAF gain provided, but not supported */ > + return dev_err_probe(&spi->dev, -EINVAL, "AAF gain not supported > for %s\n", > + st->chip->name); > + } else { > + /* Device supports variable AAF gain, validate and set the gain */ > + switch (val) { > + case 1000: > + st->aaf_gain = AD7768_AAF_IN1; > + break; > + case 364: > + st->aaf_gain = AD7768_AAF_IN2; > + break; > + case 143: > + st->aaf_gain = AD7768_AAF_IN3; > + break; > + default: > + return dev_err_probe(&spi->dev, -EINVAL, > + "Invalid firmware provided gain\n"); > + } > + } > + > ret = ad7768_setup(indio_dev); > if (ret < 0) { > dev_err(&spi->dev, "AD7768 setup failed\n"); > @@ -1426,6 +1689,9 @@ static int ad7768_probe(struct spi_device *spi) > > init_completion(&st->completion); > > + if (st->chip->has_pga) > + ad7768_set_pga_gain(st, st->chip->default_pga_mode); > + > ret = ad7768_set_channel_label(indio_dev, st->chip->num_channels); > if (ret) > return ret; > @@ -1446,12 +1712,18 @@ static int ad7768_probe(struct spi_device *spi) > > static const struct spi_device_id ad7768_id_table[] = { > { "ad7768-1", (kernel_ulong_t)&ad7768_chip_info }, > + { "adaq7767-1", (kernel_ulong_t)&adaq7767_chip_info }, > + { "adaq7768-1", (kernel_ulong_t)&adaq7768_chip_info }, > + { "adaq7769-1", (kernel_ulong_t)&adaq7769_chip_info }, > { } > }; > MODULE_DEVICE_TABLE(spi, ad7768_id_table); > > static const struct of_device_id ad7768_of_match[] = { > { .compatible = "adi,ad7768-1", .data = &ad7768_chip_info }, > + { .compatible = "adi,adaq7767-1", .data = &adaq7767_chip_info }, > + { .compatible = "adi,adaq7768-1", .data = &adaq7768_chip_info }, > + { .compatible = "adi,adaq7769-1", .data = &adaq7769_chip_info }, > { } > }; > MODULE_DEVICE_TABLE(of, ad7768_of_match);
On 08/14, Nuno Sá wrote: > On Tue, 2025-08-12 at 23:49 -0300, Jonathan Santos wrote: > > Add support for ADAQ7767/68/69-1 series, which includes PGIA and > > Anti-aliasing filter (AAF) gains. > > > > The PGA gain is configured in run-time through the scale attribute, > > if supported by the device. The scale options are updated according > > to the output data width. > > > > The AAF gain is configured in the devicetree and it should correspond to > > the AAF channel selected in hardware. > > > > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> > > --- > > Hi Jonathan, > > Some comments from me... > ... > > +static int ad7768_set_pga_gain(struct ad7768_state *st, > > + int gain_mode) > > +{ > > + int pgia_pins_value = abs(gain_mode - st->chip->pgia_mode2pin_offset); > > + int check_val; > > + int ret; > > + > > + /* Check GPIO control register */ > > + ret = regmap_read(st->regmap, AD7768_REG_GPIO_CONTROL, &check_val); > > + if (ret < 0) > > + return ret; > > + > > + if ((check_val & AD7768_GPIO_PGIA_EN) != AD7768_GPIO_PGIA_EN) { > > + /* Enable PGIA GPIOs and set them as output */ > > + ret = regmap_write(st->regmap, AD7768_REG_GPIO_CONTROL, > > AD7768_GPIO_PGIA_EN); > > + if (ret < 0) > > + return ret; > > + } > > + > > + /* Write the respective gain values to GPIOs 0, 1, 2 */ > > + ret = regmap_write(st->regmap, AD7768_REG_GPIO_WRITE, > > + AD7768_GPIO_WRITE(pgia_pins_value)); > > + if (ret < 0) > > + return ret; > > + > > + st->pga_gain_mode = gain_mode; > > + > > It looks the above function could use some locking. > > > + return 0; > > +} > > + > > static int ad7768_gpio_direction_input(struct gpio_chip *chip, unsigned int > > offset) > > { > > struct iio_dev *indio_dev = gpiochip_get_data(chip); > > @@ -782,13 +937,17 @@ static const struct iio_chan_spec ad7768_channels[] = { > > AD7768_CHAN(0, AD7768_CHAN_INFO_NONE), > > }; > > > > +static const struct iio_chan_spec adaq776x_channels[] = { > > + AD7768_CHAN(0, BIT(IIO_CHAN_INFO_SCALE)), > > +}; > > + > > static int ad7768_read_raw(struct iio_dev *indio_dev, > > struct iio_chan_spec const *chan, > > int *val, int *val2, long info) > > { > > struct ad7768_state *st = iio_priv(indio_dev); > > const struct iio_scan_type *scan_type; > > - int scale_uv, ret, temp; > > + int ret, temp; > > > > scan_type = iio_get_current_scan_type(indio_dev, chan); > > if (IS_ERR(scan_type)) > > @@ -809,12 +968,19 @@ static int ad7768_read_raw(struct iio_dev *indio_dev, > > return IIO_VAL_INT; > > > > case IIO_CHAN_INFO_SCALE: > > - scale_uv = st->vref_uv; > > - if (scale_uv < 0) > > - return scale_uv; > > - > > - *val = (scale_uv * 2) / 1000; > > - *val2 = scan_type->realbits; > > + if (st->chip->has_pga) { > > + *val = st->scale_tbl[st->pga_gain_mode][0]; > > + *val2 = st->scale_tbl[st->pga_gain_mode][1]; > > + return IIO_VAL_INT_PLUS_NANO; > > + } > > + *val = st->vref_uv / 1000; > > + if (st->chip->has_variable_aaf) > > + *val = (*val * MILLI) / ad7768_aaf_gains[st->aaf_gain]; > > + /* > > + * ADC output code is two's complement so only (realbits - 1) > > + * bits express voltage magnitude. > > + */ > > + *val2 = scan_type->realbits - 1; > > > > I'm a bit confused. Is there something wrong with the original code that needs to be > fixed? The above change seems to effectively change behavior of the code with had > before. > > - Nuno Sá I think it is more of a semantic clarification than a behavioral change per se. Previously, the calculation gave the impression of a bipolar unsigned input, since it was using the full scale. With the update, we’re explicitly considering the sign bit, which makes it clear that the input is a two’s complement signal. Altough pehaps this should not be mixed into the patch. > > return IIO_VAL_FRACTIONAL_LOG2; > > > > @@ -869,18 +1035,42 @@ static int ad7768_read_avail(struct iio_dev *indio_dev, > > *length = st->samp_freq_avail_len; > > *type = IIO_VAL_INT; > > return IIO_AVAIL_LIST; > > + case IIO_CHAN_INFO_SCALE: > > + *vals = (int *)st->scale_tbl; > > + *length = st->chip->num_pga_modes * 2; > > + *type = IIO_VAL_INT_PLUS_NANO; > > + return IIO_AVAIL_LIST; > > default: > > return -EINVAL; > > } > > } ... - Jonathan Santos
Hi Jonathan, kernel test robot noticed the following build errors: [auto build test ERROR on 0a686b9c4f847dc21346df8e56d5b119918fefef] url: https://github.com/intel-lab-lkp/linux/commits/Jonathan-Santos/dt-bindings-iio-adc-ad7768-1-add-new-supported-parts/20250813-145315 base: 0a686b9c4f847dc21346df8e56d5b119918fefef patch link: https://lore.kernel.org/r/f0c1cbc9c2994a90113788cad57df1f32f9db45e.1754617360.git.Jonathan.Santos%40analog.com patch subject: [PATCH 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family config: hexagon-randconfig-002-20250814 (https://download.01.org/0day-ci/archive/20250814/202508140742.AhFMglnF-lkp@intel.com/config) compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 3769ce013be2879bf0b329c14a16f5cb766f26ce) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250814/202508140742.AhFMglnF-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202508140742.AhFMglnF-lkp@intel.com/ All errors (new ones prefixed by >>): >> ld.lld: error: undefined symbol: rational_best_approximation >>> referenced by ad7768-1.c:554 (drivers/iio/adc/ad7768-1.c:554) >>> drivers/iio/adc/ad7768-1.o:(ad7768_configure_dig_fil) in archive vmlinux.a >>> referenced by ad7768-1.c:554 (drivers/iio/adc/ad7768-1.c:554) >>> drivers/iio/adc/ad7768-1.o:(ad7768_configure_dig_fil) in archive vmlinux.a -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
On Wed, Aug 13, 2025 at 4:49 AM Jonathan Santos <Jonathan.Santos@analog.com> wrote: > > Add support for ADAQ7767/68/69-1 series, which includes PGIA and > Anti-aliasing filter (AAF) gains. > > The PGA gain is configured in run-time through the scale attribute, > if supported by the device. The scale options are updated according > to the output data width. > > The AAF gain is configured in the devicetree and it should correspond to > the AAF channel selected in hardware. > +#define AD7768_GPIO_READ(x) FIELD_PREP(AD7768_GPIO_READ_MSK, x) > > #define AD7768_VCM_OFF 0x07 > #define AD7768_CHAN_INFO_NONE 0 > > +#define ADAQ776X_GAIN_MAX_NANO (128 * NANO) > +#define ADAQ776X_MAX_GAIN_MODES 8 > + > #define AD7768_TRIGGER_SOURCE_SYNC_IDX 0 > > #define AD7768_MAX_CHANNELS 1 > @@ -154,6 +161,52 @@ enum ad7768_scan_type { > AD7768_SCAN_TYPE_HIGH_SPEED, > }; > > +enum { > + AD7768_PGA_GAIN_0, > + AD7768_PGA_GAIN_1, > + AD7768_PGA_GAIN_2, > + AD7768_PGA_GAIN_3, > + AD7768_PGA_GAIN_4, > + AD7768_PGA_GAIN_5, > + AD7768_PGA_GAIN_6, > + AD7768_PGA_GAIN_7, > + AD7768_MAX_PGA_GAIN, > +}; > + > +enum { > + AD7768_AAF_IN1, > + AD7768_AAF_IN2, > + AD7768_AAF_IN3, > +}; > + > +/* PGA and AAF gains in V/V */ > +static const int adaq7768_gains[7] = { > + [AD7768_PGA_GAIN_0] = 325, /* 0.325 */ > + [AD7768_PGA_GAIN_1] = 650, /* 0.650 */ > + [AD7768_PGA_GAIN_2] = 1300, /* 1.300 */ > + [AD7768_PGA_GAIN_3] = 2600, /* 2.600 */ > + [AD7768_PGA_GAIN_4] = 5200, /* 5.200 */ > + [AD7768_PGA_GAIN_5] = 10400, /* 10.400 */ > + [AD7768_PGA_GAIN_6] = 20800 /* 20.800 */ > +}; > + > +static const int adaq7769_gains[8] = { > + [AD7768_PGA_GAIN_0] = 1000, /* 1.000 */ > + [AD7768_PGA_GAIN_1] = 2000, /* 2.000 */ > + [AD7768_PGA_GAIN_2] = 4000, /* 4.000 */ > + [AD7768_PGA_GAIN_3] = 8000, /* 8.000 */ > + [AD7768_PGA_GAIN_4] = 16000, /* 16.000 */ > + [AD7768_PGA_GAIN_5] = 32000, /* 32.000 */ > + [AD7768_PGA_GAIN_6] = 64000, /* 64.000 */ > + [AD7768_PGA_GAIN_7] = 128000 /* 128.000 */ > +}; > + > +static const int ad7768_aaf_gains[3] = { > + [AD7768_AAF_IN1] = 1000, /* 1.000 */ > + [AD7768_AAF_IN2] = 364, /* 0.364 */ > + [AD7768_AAF_IN3] = 143 /* 0.143 */ > +}; > + > /* -3dB cutoff frequency multipliers (relative to ODR) for each filter type. */ > static const int ad7768_filter_3db_odr_multiplier[] = { > [AD7768_FILTER_SINC5] = 204, /* 0.204 */ > @@ -216,6 +269,12 @@ static const struct iio_scan_type ad7768_scan_type[] = { > > struct ad7768_chip_info { > const char *name; > + bool has_variable_aaf; > + bool has_pga; > + int num_pga_modes; > + int default_pga_mode; > + int pgia_mode2pin_offset; > + const int *pga_gains; > const struct iio_chan_spec *channel_spec; > const unsigned long *available_masks; > int num_channels; > @@ -236,6 +295,9 @@ struct ad7768_state { > unsigned int samp_freq; > unsigned int samp_freq_avail[ARRAY_SIZE(ad7768_mclk_div_rates)]; > unsigned int samp_freq_avail_len; > + int pga_gain_mode; > + int aaf_gain; > + int scale_tbl[ADAQ776X_MAX_GAIN_MODES][2]; > struct completion completion; > struct iio_trigger *trig; > struct gpio_desc *gpio_sync_in; > @@ -466,6 +528,43 @@ static int ad7768_reg_access(struct iio_dev *indio_dev, > return ret; > } > > +static void ad7768_fill_scale_tbl(struct iio_dev *dev) > +{ > + struct ad7768_state *st = iio_priv(dev); > + const struct iio_scan_type *scan_type; > + int val, val2, tmp0, tmp1, i; > + unsigned long denominator, numerator; > + u64 tmp2; > + > + scan_type = iio_get_current_scan_type(dev, &dev->channels[0]); > + if (scan_type->sign == 's') > + val2 = scan_type->realbits - 1; > + else > + val2 = scan_type->realbits; > + > + for (i = 0; i < st->chip->num_pga_modes; i++) { > + /* Convert gain to a fraction format */ > + numerator = st->chip->pga_gains[i]; > + denominator = MILLI; > + if (st->chip->has_variable_aaf) { > + numerator *= ad7768_aaf_gains[st->aaf_gain]; > + denominator *= MILLI; > + } > + > + rational_best_approximation(numerator, denominator, __INT_MAX__, __INT_MAX__, > + &numerator, &denominator); > + > + val = st->vref_uv / 1000; > + /* Multiply by MILLI here to avoid losing precision */ > + val = mult_frac(val, denominator * MILLI, numerator); > + /* Would multiply by NANO here but we already multiplied by MILLI */ > + tmp2 = shift_right((u64)val * MICRO, val2); > + tmp0 = (int)div_s64_rem(tmp2, NANO, &tmp1); > + st->scale_tbl[i][0] = tmp0; /* Integer part */ > + st->scale_tbl[i][1] = abs(tmp1); /* Fractional part */ > + } > +} > + > static int ad7768_set_sinc3_dec_rate(struct ad7768_state *st, > unsigned int dec_rate) > { > @@ -567,12 +666,68 @@ static int ad7768_configure_dig_fil(struct iio_dev *dev, > st->oversampling_ratio = ad7768_dec_rate_values[dec_rate_idx]; > } > > + /* Update scale table: scale values vary according to the precision */ > + ad7768_fill_scale_tbl(dev); > + > ad7768_fill_samp_freq_tbl(st); > > /* A sync-in pulse is required after every configuration change */ > return ad7768_send_sync_pulse(st); > } > > +static int ad7768_calc_pga_gain(struct ad7768_state *st, int gain_int, > + int gain_fract, int precision) > +{ > + u64 gain_nano, tmp; > + int gain_idx; > + > + precision--; > + gain_nano = gain_int * NANO + gain_fract; > + if (gain_nano < 0 || gain_nano > ADAQ776X_GAIN_MAX_NANO) > + return -EINVAL; > + > + tmp = DIV_ROUND_CLOSEST_ULL(gain_nano << precision, NANO); > + gain_nano = DIV_ROUND_CLOSEST_ULL(st->vref_uv, tmp); > + if (st->chip->has_variable_aaf) > + /* remove the AAF gain from the overall gain */ > + gain_nano = DIV_ROUND_CLOSEST_ULL(gain_nano * MILLI, > + ad7768_aaf_gains[st->aaf_gain]); > + tmp = st->chip->num_pga_modes; > + gain_idx = find_closest(gain_nano, st->chip->pga_gains, tmp); > + > + return gain_idx; > +} > + > +static int ad7768_set_pga_gain(struct ad7768_state *st, > + int gain_mode) > +{ > + int pgia_pins_value = abs(gain_mode - st->chip->pgia_mode2pin_offset); > + int check_val; > + int ret; > + > + /* Check GPIO control register */ > + ret = regmap_read(st->regmap, AD7768_REG_GPIO_CONTROL, &check_val); > + if (ret < 0) > + return ret; > + > + if ((check_val & AD7768_GPIO_PGIA_EN) != AD7768_GPIO_PGIA_EN) { > + /* Enable PGIA GPIOs and set them as output */ > + ret = regmap_write(st->regmap, AD7768_REG_GPIO_CONTROL, AD7768_GPIO_PGIA_EN); > + if (ret < 0) > + return ret; > + } > + > + /* Write the respective gain values to GPIOs 0, 1, 2 */ > + ret = regmap_write(st->regmap, AD7768_REG_GPIO_WRITE, > + AD7768_GPIO_WRITE(pgia_pins_value)); > + if (ret < 0) > + return ret; > + > + st->pga_gain_mode = gain_mode; > + > + return 0; > +} > + > static int ad7768_gpio_direction_input(struct gpio_chip *chip, unsigned int offset) > { > struct iio_dev *indio_dev = gpiochip_get_data(chip); > @@ -782,13 +937,17 @@ static const struct iio_chan_spec ad7768_channels[] = { > AD7768_CHAN(0, AD7768_CHAN_INFO_NONE), > }; > > +static const struct iio_chan_spec adaq776x_channels[] = { > + AD7768_CHAN(0, BIT(IIO_CHAN_INFO_SCALE)), > +}; > + > static int ad7768_read_raw(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan, > int *val, int *val2, long info) > { > struct ad7768_state *st = iio_priv(indio_dev); > const struct iio_scan_type *scan_type; > - int scale_uv, ret, temp; > + int ret, temp; > > scan_type = iio_get_current_scan_type(indio_dev, chan); > if (IS_ERR(scan_type)) > @@ -809,12 +968,19 @@ static int ad7768_read_raw(struct iio_dev *indio_dev, > return IIO_VAL_INT; > > case IIO_CHAN_INFO_SCALE: > - scale_uv = st->vref_uv; > - if (scale_uv < 0) > - return scale_uv; > - > - *val = (scale_uv * 2) / 1000; > - *val2 = scan_type->realbits; > + if (st->chip->has_pga) { > + *val = st->scale_tbl[st->pga_gain_mode][0]; > + *val2 = st->scale_tbl[st->pga_gain_mode][1]; > + return IIO_VAL_INT_PLUS_NANO; > + } > + *val = st->vref_uv / 1000; > + if (st->chip->has_variable_aaf) > + *val = (*val * MILLI) / ad7768_aaf_gains[st->aaf_gain]; > + /* > + * ADC output code is two's complement so only (realbits - 1) > + * bits express voltage magnitude. > + */ > + *val2 = scan_type->realbits - 1; > > return IIO_VAL_FRACTIONAL_LOG2; > > @@ -869,18 +1035,42 @@ static int ad7768_read_avail(struct iio_dev *indio_dev, > *length = st->samp_freq_avail_len; > *type = IIO_VAL_INT; > return IIO_AVAIL_LIST; > + case IIO_CHAN_INFO_SCALE: > + *vals = (int *)st->scale_tbl; > + *length = st->chip->num_pga_modes * 2; > + *type = IIO_VAL_INT_PLUS_NANO; > + return IIO_AVAIL_LIST; > default: > return -EINVAL; > } > } > > +static int ad7768_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; > +} > + > static int __ad7768_write_raw(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan, > int val, int val2, long info) > { > struct ad7768_state *st = iio_priv(indio_dev); > + const struct iio_scan_type *scan_type; > + int gain_mode; > int ret; > > + scan_type = iio_get_current_scan_type(indio_dev, chan); > + if (IS_ERR(scan_type)) > + return PTR_ERR(scan_type); > + > switch (info) { > case IIO_CHAN_INFO_SAMP_FREQ: > return ad7768_set_freq(st, val); > @@ -892,6 +1082,13 @@ static int __ad7768_write_raw(struct iio_dev *indio_dev, > > /* Update sampling frequency */ > return ad7768_set_freq(st, st->samp_freq); > + case IIO_CHAN_INFO_SCALE: > + if (!st->chip->has_pga) > + return -EOPNOTSUPP; > + > + gain_mode = ad7768_calc_pga_gain(st, val, val2, > + scan_type->realbits); > + return ad7768_set_pga_gain(st, gain_mode); > default: > return -EINVAL; > } > @@ -933,6 +1130,7 @@ static const struct iio_info ad7768_info = { > .read_raw = &ad7768_read_raw, > .read_avail = &ad7768_read_avail, > .write_raw = &ad7768_write_raw, > + .write_raw_get_fmt = &ad7768_write_raw_get_fmt, > .read_label = ad7768_read_label, > .get_current_scan_type = &ad7768_get_current_scan_type, > .debugfs_reg_access = &ad7768_reg_access, > @@ -1351,10 +1549,46 @@ static const struct ad7768_chip_info ad7768_chip_info = { > .available_masks = ad7768_channel_masks, > }; > > +static const struct ad7768_chip_info adaq7767_chip_info = { > + .name = "adaq7767-1", > + .channel_spec = ad7768_channels, > + .num_channels = ARRAY_SIZE(ad7768_channels), > + .available_masks = ad7768_channel_masks, > + .has_pga = false, > + .has_variable_aaf = true > +}; > + > +static const struct ad7768_chip_info adaq7768_chip_info = { > + .name = "adaq7768-1", > + .channel_spec = adaq776x_channels, > + .num_channels = ARRAY_SIZE(adaq776x_channels), > + .available_masks = ad7768_channel_masks, > + .pga_gains = adaq7768_gains, > + .default_pga_mode = AD7768_PGA_GAIN_2, > + .num_pga_modes = ARRAY_SIZE(adaq7768_gains), > + .pgia_mode2pin_offset = 6, > + .has_pga = true, > + .has_variable_aaf = false > +}; > + > +static const struct ad7768_chip_info adaq7769_chip_info = { > + .name = "adaq7769-1", > + .channel_spec = adaq776x_channels, > + .num_channels = ARRAY_SIZE(adaq776x_channels), > + .available_masks = ad7768_channel_masks, > + .pga_gains = adaq7769_gains, > + .default_pga_mode = AD7768_PGA_GAIN_0, > + .num_pga_modes = ARRAY_SIZE(adaq7769_gains), > + .pgia_mode2pin_offset = 0, > + .has_pga = true, > + .has_variable_aaf = true > +}; > + > static int ad7768_probe(struct spi_device *spi) > { > struct ad7768_state *st; > struct iio_dev *indio_dev; > + u32 val; > int ret; > ... > + st->aaf_gain = AD7768_AAF_IN1; > + ret = device_property_read_u32(&spi->dev, "adi,aaf-gain", &val); > + if (ret) { > + /* AAF gain required, but not specified */ > + if (st->chip->has_variable_aaf) > + return dev_err_probe(&spi->dev, -EINVAL, "AAF gain not specified\n"); > + Stray blank line. > + } else if (!st->chip->has_variable_aaf) { > + /* AAF gain provided, but not supported */ > + return dev_err_probe(&spi->dev, -EINVAL, "AAF gain not supported for %s\n", > + st->chip->name); I would rewrite these conditionals as if (ret && ->has_variable_aaf) return dev_err_probe(...); if (!ret && ! ->has_variable_aaf) return dev_err_probe(...); > + } else { It will be redundant 'else' after the above being applied. > + /* Device supports variable AAF gain, validate and set the gain */ > + switch (val) { > + case 1000: > + st->aaf_gain = AD7768_AAF_IN1; > + break; > + case 364: > + st->aaf_gain = AD7768_AAF_IN2; > + break; > + case 143: > + st->aaf_gain = AD7768_AAF_IN3; > + break; > + default: > + return dev_err_probe(&spi->dev, -EINVAL, > + "Invalid firmware provided gain\n"); > + } > + } -- With Best Regards, Andy Shevchenko
© 2016 - 2025 Red Hat, Inc.