diff mbox series

[v2,2/2] iio: adc: Add support for AD4000

Message ID 1d95d7d023dad69b894a2d0e7b0bad9d569ae382.1712585500.git.marcelo.schmitt@analog.com (mailing list archive)
State Changes Requested
Headers show
Series Add support for AD4000 series | expand

Commit Message

Marcelo Schmitt April 8, 2024, 2:31 p.m. UTC
Add support for AD4000 family of low noise, low power, high speed,
successive aproximation register (SAR) ADCs.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 MAINTAINERS              |   1 +
 drivers/iio/adc/Kconfig  |  12 +
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad4000.c | 649 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 663 insertions(+)
 create mode 100644 drivers/iio/adc/ad4000.c

Comments

David Lechner April 9, 2024, 3:05 a.m. UTC | #1
On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
<marcelo.schmitt@analog.com> wrote:
>
> Add support for AD4000 family of low noise, low power, high speed,
> successive aproximation register (SAR) ADCs.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
>  MAINTAINERS              |   1 +
>  drivers/iio/adc/Kconfig  |  12 +
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/ad4000.c | 649 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 663 insertions(+)
>  create mode 100644 drivers/iio/adc/ad4000.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5dfe118a5dd3..86aa96115f5a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1165,6 +1165,7 @@ L:        linux-iio@vger.kernel.org
>  S:     Supported
>  W:     https://ez.analog.com/linux-software-drivers
>  F:     Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> +F:     drivers/iio/adc/ad4000.c
>
>  ANALOG DEVICES INC AD4130 DRIVER
>  M:     Cosmin Tanislav <cosmin.tanislav@analog.com>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 8db68b80b391..9c9d13d4b74f 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -21,6 +21,18 @@ config AD_SIGMA_DELTA
>         select IIO_BUFFER
>         select IIO_TRIGGERED_BUFFER
>
> +config AD4000
> +       tristate "Analog Devices AD4000 ADC Driver"
> +       depends on SPI
> +       select IIO_BUFFER
> +       select IIO_TRIGGERED_BUFFER
> +       help
> +         Say yes here to build support for Analog Devices AD4000 high speed
> +         SPI analog to digital converters (ADC).
> +
> +         To compile this driver as a module, choose M here: the module will be
> +         called ad4000.
> +
>  config AD4130
>         tristate "Analog Device AD4130 ADC Driver"
>         depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index edb32ce2af02..aa52068d864b 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -6,6 +6,7 @@
>  # When adding new entries keep the list in alphabetical order
>  obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
>  obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
> +obj-$(CONFIG_AD4000) += ad4000.o
>  obj-$(CONFIG_AD4130) += ad4130.o
>  obj-$(CONFIG_AD7091R) += ad7091r-base.o
>  obj-$(CONFIG_AD7091R5) += ad7091r5.o
> diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
> new file mode 100644
> index 000000000000..7997d9d98743
> --- /dev/null
> +++ b/drivers/iio/adc/ad4000.c
> @@ -0,0 +1,649 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * AD4000 SPI ADC driver
> + *
> + * Copyright 2024 Analog Devices Inc.
> + */
> +#include <asm/unaligned.h>
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/math.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sysfs.h>
> +#include <linux/units.h>
> +#include <linux/util_macros.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +
> +#define AD400X_READ_COMMAND    0x54
> +#define AD400X_WRITE_COMMAND   0x14
> +
> +/* AD4000 Configuration Register programmable bits */
> +#define AD4000_STATUS          BIT(4) /* Status bits output */
> +#define AD4000_SPAN_COMP       BIT(3) /* Input span compression  */
> +#define AD4000_HIGHZ           BIT(2) /* High impedance mode  */
> +#define AD4000_TURBO           BIT(1) /* Turbo mode */

Usually bits of the same register share a similar prefix, e.g.
AD4000_CFG_TURBO, AD4000_CFG_HIGHZ, etc.

> +
> +#define AD4000_TQUIET2_NS              60
> +
> +#define AD4000_18BIT_MSK       GENMASK(31, 14)
> +#define AD4000_20BIT_MSK       GENMASK(31, 12)
> +
> +#define AD4000_DIFF_CHANNEL(_sign, _real_bits)                         \
> +       {                                                               \
> +               .type = IIO_VOLTAGE,                                    \
> +               .indexed = 1,                                           \
> +               .differential = 1,                                      \
> +               .channel = 0,                                           \
> +               .channel2 = 1,                                          \
> +               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |          \
> +                                     BIT(IIO_CHAN_INFO_SCALE),         \
> +               .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),\
> +               .scan_type = {                                          \
> +                       .sign = _sign,                                  \
> +                       .realbits = _real_bits,                         \
> +                       .storagebits = _real_bits > 16 ? 32 : 16,       \
> +                       .shift = _real_bits > 16 ? 32 - _real_bits : 0, \
> +                       .endianness = IIO_BE,                           \
> +               },                                                      \
> +       }                                                               \
> +
> +#define AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits)                  \
> +       {                                                               \
> +               .type = IIO_VOLTAGE,                                    \
> +               .indexed = 1,                                           \
> +               .channel = 0,                                           \
> +               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |          \
> +                                     BIT(IIO_CHAN_INFO_SCALE) |        \
> +                                     BIT(IIO_CHAN_INFO_OFFSET),        \
> +               .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),\
> +               .scan_type = {                                          \
> +                       .sign = _sign,                                  \
> +                       .realbits = _real_bits,                         \
> +                       .storagebits = _real_bits > 16 ? 32 : 16,       \
> +                       .shift = _real_bits > 16 ? 32 - _real_bits : 0, \
> +                       .endianness = IIO_BE,                           \
> +               },                                                      \
> +       }                                                               \

It looks like all differential chips are signed and all
pseduo-differential chips are unsigned, so I don't think we need the
_sign parameter in these macros.

I also still have doubts about using IIO_BE and 8-bit xfers when it
comes to adding support later to achieve max sample rate with a SPI
offload. For example to get 2MSPS with an 18-bit chip, it will require
an approx 33% faster SPI clock than the actual slowest clock possible
because it will have to read 6 extra bits per sample. I didn't check
the specs, but this may not even be physically possible without
exceeding the datasheet max SPI clock rate. Also errors could be
reduced if we could actually use the slowest allowable SPI clock rate.
Furthermore, the offload hardware would have to be capable of adding
an extra byte per sample for 18 and 20-bit chips when piping the data
to DMA in order to get the 32-bit alignment in the buffer required by
IIO scan_type and the natural alignment requirements of IIO buffers in
general.

> +
> +enum ad4000_ids {
> +       ID_AD4000,
> +       ID_AD4001,
> +       ID_AD4002,
> +       ID_AD4003,
> +       ID_AD4004,
> +       ID_AD4005,
> +       ID_AD4006,
> +       ID_AD4007,
> +       ID_AD4008,
> +       ID_AD4010,
> +       ID_AD4011,
> +       ID_AD4020,
> +       ID_AD4021,
> +       ID_AD4022,
> +       ID_ADAQ4001,
> +       ID_ADAQ4003,
> +};
> +
> +struct ad4000_chip_info {
> +       const char *dev_name;
> +       struct iio_chan_spec chan_spec;
> +};
> +
> +static const struct ad4000_chip_info ad4000_chips[] = {
> +       [ID_AD4000] = {
> +               .dev_name = "ad4000",
> +               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
> +       },
> +       [ID_AD4001] = {
> +               .dev_name = "ad4001",
> +               .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
> +       },
> +       [ID_AD4002] = {
> +               .dev_name = "ad4002",
> +               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
> +       },
> +       [ID_AD4003] = {
> +               .dev_name = "ad4003",
> +               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
> +       },
> +       [ID_AD4004] = {
> +               .dev_name = "ad4004",
> +               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
> +       },
> +       [ID_AD4005] = {
> +               .dev_name = "ad4005",
> +               .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
> +       },
> +       [ID_AD4006] = {
> +               .dev_name = "ad4006",
> +               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
> +       },
> +       [ID_AD4007] = {
> +               .dev_name = "ad4007",
> +               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
> +       },
> +       [ID_AD4008] = {
> +               .dev_name = "ad4008",
> +               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
> +       },
> +       [ID_AD4010] = {
> +               .dev_name = "ad4010",
> +               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
> +       },
> +       [ID_AD4011] = {
> +               .dev_name = "ad4011",
> +               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
> +       },
> +       [ID_AD4020] = {
> +               .dev_name = "ad4020",
> +               .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
> +       },
> +       [ID_AD4021] = {
> +               .dev_name = "ad4021",
> +               .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
> +       },
> +       [ID_AD4022] = {
> +               .dev_name = "ad4022",
> +               .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
> +       },
> +       [ID_ADAQ4001] = {
> +               .dev_name = "adaq4001",
> +               .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
> +       },
> +       [ID_ADAQ4003] = {
> +               .dev_name = "adaq4003",
> +               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
> +       },
> +};
> +
> +enum ad4000_gains {
> +       AD4000_0454_GAIN = 0,
> +       AD4000_0909_GAIN = 1,
> +       AD4000_1_GAIN = 2,

AD4000_1000_GAIN would be more consistent with the others.

> +       AD4000_1900_GAIN = 3,
> +       AD4000_GAIN_LEN
> +};
> +
> +/*
> + * Gains stored and computed as fractions to avoid introducing rounding errors.
> + */
> +static const int ad4000_gains_frac[AD4000_GAIN_LEN][2] = {
> +       [AD4000_0454_GAIN] = { 227, 500 },
> +       [AD4000_0909_GAIN] = { 909, 1000 },
> +       [AD4000_1_GAIN] = { 1, 1 },
> +       [AD4000_1900_GAIN] = { 19, 10 },
> +};

Why not just store the numerator in milli units and always use 1000
for the denominator? It seems like it would simplify the code and make
it easier to read and understand. Also, these values are coming from
the adi,gain-milli property already, so we could avoid the enum and
the lookup table entirely and simplify things even more.

> +
> +struct ad4000_state {
> +       struct spi_device *spi;
> +       struct gpio_desc *cnv_gpio;
> +       int vref;
> +       bool status_bits;
> +       bool span_comp;
> +       bool turbo_mode;
> +       bool high_z_mode;
> +
> +       enum ad4000_gains pin_gain;
> +       int scale_tbl[AD4000_GAIN_LEN][2][2];
> +
> +       /*
> +        * DMA (thus cache coherency maintenance) requires the
> +        * transfer buffers to live in their own cache lines.
> +        */
> +       struct {
> +               union {
> +                       u16 sample_buf16;
> +                       u32 sample_buf32;

Technically, these are holding big-endian data, so __be16 and __be32
would be more correct.

> +               } data;
> +               s64 timestamp __aligned(8);
> +       } scan;
> +       __be16 tx_buf __aligned(IIO_DMA_MINALIGN);
> +       __be16 rx_buf;
> +};

scan.data is used as SPI rx_buf so __aligned(IIO_DMA_MINALIGN); needs
to be moved to the scan field.

> +
> +static void ad4000_fill_scale_tbl(struct ad4000_state *st, int scale_bits,
> +                                 const struct ad4000_chip_info *chip)
> +{
> +       int diff = chip->chan_spec.differential;
> +       int val, val2, tmp0, tmp1, i;
> +       u64 tmp2;
> +
> +       val2 = scale_bits;
> +       for (i = 0; i < AD4000_GAIN_LEN; i++) {

Only one gain is selected by the devicetree, so why do we need to do
this for all 4 gains?

> +               val = st->vref / 1000;
> +               /* Multiply by MILLI here to avoid losing precision */
> +               val = mult_frac(val, ad4000_gains_frac[i][1] * MILLI,
> +                               ad4000_gains_frac[i][0]);
> +               /* 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);
> +               /* Store scale for when span compression is disabled */
> +               st->scale_tbl[i][0][0] = tmp0; /* Integer part */
> +               st->scale_tbl[i][0][1] = abs(tmp1); /* Fractional part */
> +               /* Store scale for when span compression is enabled */
> +               st->scale_tbl[i][1][0] = tmp0;
> +               if (diff)
> +                       st->scale_tbl[i][1][1] = DIV_ROUND_CLOSEST(abs(tmp1) * 4, 5);
> +               else
> +                       st->scale_tbl[i][1][1] = DIV_ROUND_CLOSEST(abs(tmp1) * 9, 10);
> +       }
> +}
> +
> +static int ad4000_write_reg(struct ad4000_state *st, uint8_t val)
> +{
> +       put_unaligned_be16(AD400X_WRITE_COMMAND << BITS_PER_BYTE | val,
> +                          &st->tx_buf);
> +       return spi_write(st->spi, &st->tx_buf, 2);
> +}
> +
> +static int ad4000_read_reg(struct ad4000_state *st, unsigned int *val)
> +{
> +       struct spi_transfer t[] = {
> +               {
> +                       .tx_buf = &st->tx_buf,
> +                       .rx_buf = &st->rx_buf,
> +                       .len = 2,
> +               },
> +       };
> +       int ret;
> +
> +       put_unaligned_be16(AD400X_READ_COMMAND << BITS_PER_BYTE, &st->tx_buf);
> +       ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
> +       if (ret < 0)
> +               return ret;
> +
> +       *val = get_unaligned_be16(&st->rx_buf);
> +
> +       return ret;
> +}
> +

It would be very helpful to have comments here explaining the exact
expected wiring configuration and signal timing here since there are
so many possibilities for this chip.

> +static int ad4000_read_sample(struct ad4000_state *st,
> +                             const struct iio_chan_spec *chan)
> +{
> +       struct spi_transfer t[] = {

Don't really need [] here since there is only one xfer.

> +               {
> +                       .rx_buf = &st->scan.data,
> +                       .len = BITS_TO_BYTES(chan->scan_type.storagebits),
> +                       .delay = {
> +                               .value = AD4000_TQUIET2_NS,
> +                               .unit = SPI_DELAY_UNIT_NSECS,
> +                       },
> +               },
> +       };
> +       int ret;
> +
> +       ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
> +       if (ret < 0)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +static int ad4000_single_conversion(struct iio_dev *indio_dev,
> +                                   const struct iio_chan_spec *chan, int *val)
> +{
> +       struct ad4000_state *st = iio_priv(indio_dev);
> +       u32 sample;
> +       int ret;
> +
> +       if (st->cnv_gpio)
> +               gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_HIGH);

It would make more sense and be less redundant to move the gpio code
into ad4000_read_sample().

Also, gpiod_set_value_cansleep() checks for NULL, so the if () is redundant.

> +
> +       ret = ad4000_read_sample(st, chan);
> +       if (ret)
> +               return ret;
> +
> +       if (st->cnv_gpio)
> +               gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_LOW);
> +
> +       if (chan->scan_type.storagebits > 16)
> +               sample = get_unaligned_be32(&st->scan.data);
> +       else
> +               sample = get_unaligned_be16(&st->scan.data);

data is aligned, so be32/16_to_cpu() should be fine. Also, Jonathan
will suggest to use &st->scan.data.sample_b32/16 here too. :-)

> +
> +       switch (chan->scan_type.realbits) {
> +       case 16:
> +               break;
> +       case 18:
> +               sample = FIELD_GET(AD4000_18BIT_MSK, sample);
> +               break;
> +       case 20:
> +               sample = FIELD_GET(AD4000_20BIT_MSK, sample);
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       if (chan->scan_type.sign == 's')
> +               *val = sign_extend32(sample, chan->scan_type.realbits - 1);
> +
> +       return IIO_VAL_INT;
> +}
> +
> +static int ad4000_read_raw(struct iio_dev *indio_dev,
> +                          struct iio_chan_spec const *chan, int *val,
> +                          int *val2, long info)
> +{
> +       struct ad4000_state *st = iio_priv(indio_dev);
> +
> +       switch (info) {
> +       case IIO_CHAN_INFO_RAW:
> +               iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
> +                       return ad4000_single_conversion(indio_dev, chan, val);
> +               unreachable();
> +       case IIO_CHAN_INFO_SCALE:
> +               *val = st->scale_tbl[st->pin_gain][st->span_comp][0];
> +               *val2 = st->scale_tbl[st->pin_gain][st->span_comp][1];
> +               return IIO_VAL_INT_PLUS_NANO;
> +       case IIO_CHAN_INFO_OFFSET:
> +               *val = 0;
> +               if (st->span_comp)
> +                       *val = mult_frac(st->vref / 1000, 1, 10);
> +
> +               return IIO_VAL_INT;
> +       default:
> +               break;
> +       }
> +
> +       return -EINVAL;
> +}
> +
> +static int ad4000_read_avail(struct iio_dev *indio_dev,
> +                            struct iio_chan_spec const *chan,
> +                            const int **vals, int *type, int *length,
> +                            long info)
> +{
> +       struct ad4000_state *st = iio_priv(indio_dev);
> +
> +       switch (info) {
> +       case IIO_CHAN_INFO_SCALE:
> +               *vals = (int *)st->scale_tbl[st->pin_gain];
> +               *length = 2 * 2;
> +               *type = IIO_VAL_INT_PLUS_NANO;
> +               return IIO_AVAIL_LIST;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static int ad4000_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;

not reachable because of default, so can be left out

> +}
> +
> +static int ad4000_write_raw(struct iio_dev *indio_dev,
> +                           struct iio_chan_spec const *chan, int val, int val2,
> +                           long mask)
> +{
> +       struct ad4000_state *st = iio_priv(indio_dev);
> +       unsigned int reg_val;
> +       bool span_comp_en;
> +       int ret;
> +
> +       switch (mask) {
> +       case IIO_CHAN_INFO_SCALE:
> +               iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> +                       ret = ad4000_read_reg(st, &reg_val);
> +                       if (ret < 0)
> +                               return ret;
> +
> +                       span_comp_en = (val2 == st->scale_tbl[st->pin_gain][1][1]);
> +                       reg_val &= ~AD4000_SPAN_COMP;
> +                       reg_val |= FIELD_PREP(AD4000_SPAN_COMP, span_comp_en);
> +
> +                       ret = ad4000_write_reg(st, reg_val);
> +                       if (ret < 0)
> +                               return ret;
> +
> +                       st->span_comp = span_comp_en;
> +                       return 0;
> +               }
> +               unreachable();

Can bring out the return 0 to avoid unreachable.

> +       default:
> +               break;

Can return -EINVAL to avoid break;

> +       }
> +
> +       return -EINVAL;
> +}
> +
> +static irqreturn_t ad4000_trigger_handler(int irq, void *p)
> +{
> +       struct iio_poll_func *pf = p;
> +       struct iio_dev *indio_dev = pf->indio_dev;
> +       struct ad4000_state *st = iio_priv(indio_dev);
> +       int ret;
> +
> +       if (st->cnv_gpio)
> +               gpiod_set_value(st->cnv_gpio, GPIOD_OUT_HIGH);
> +
> +       ret = ad4000_read_sample(st, &indio_dev->channels[0]);
> +       if (ret < 0)
> +               goto err_out;
> +
> +       if (st->cnv_gpio)
> +               gpiod_set_value(st->cnv_gpio, GPIOD_OUT_LOW);
> +
> +       iio_push_to_buffers_with_timestamp(indio_dev, &st->scan,
> +                                          iio_get_time_ns(indio_dev));
> +
> +err_out:
> +       iio_trigger_notify_done(indio_dev->trig);
> +       return IRQ_HANDLED;
> +}
> +
> +static const struct iio_info ad4000_info = {
> +       .read_raw = &ad4000_read_raw,
> +       .read_avail = &ad4000_read_avail,
> +       .write_raw = &ad4000_write_raw,
> +       .write_raw_get_fmt = &ad4000_write_raw_get_fmt,
> +};
> +
> +static void ad4000_config(struct ad4000_state *st)
> +{
> +       unsigned int reg_val;
> +       int ret;
> +
> +       reg_val = FIELD_PREP(AD4000_TURBO, 1);

Since the driver in it's current state can get anywhere near the max
sample rate of ~1MSPS, I don't think it makes sense to enable turbo at
this point.

> +
> +       if (device_property_present(&st->spi->dev, "adi,high-z-input"))
> +               reg_val |= FIELD_PREP(AD4000_HIGHZ, 1);
> +
> +       /*
> +        * The ADC SDI pin might be connected to controller CS line in which
> +        * case the write might fail. This, however, does not prevent the device
> +        * from functioning even though in a configuration other than the
> +        * requested one.
> +        */
> +       ret = ad4000_write_reg(st, reg_val);
> +       if (ret < 0)
> +               dev_dbg(&st->spi->dev, "Failed to config device\n");

If writing fails because there is no CS line wired up, we won't get an
error returned here. The SPI controller has no way of knowing this
happened, so it can only assume the write was successful and return 0.
So this should return ret.

Ideally, the devicetree should tell us if CS is wired up or not.

> +}
> +
> +static void ad4000_regulator_disable(void *reg)
> +{
> +       regulator_disable(reg);
> +}
> +
> +static int ad4000_probe(struct spi_device *spi)
> +{
> +       const struct ad4000_chip_info *chip;
> +       struct regulator *vref_reg;
> +       struct iio_dev *indio_dev;
> +       struct ad4000_state *st;
> +       int ret;

We need a check somewhere in here to make sure that adi,spi-mode is in
a supported configuration. E.g. chain mode is not currently
implemented.

> +
> +       indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +       if (!indio_dev)
> +               return -ENOMEM;
> +
> +       chip = spi_get_device_match_data(spi);
> +       if (!chip)
> +               return -EINVAL;
> +
> +       st = iio_priv(indio_dev);
> +       st->spi = spi;
> +
> +       ret = devm_regulator_get_enable(&spi->dev, "vdd");
> +       if (ret)
> +               return dev_err_probe(&spi->dev, ret, "Failed to enable VDD supply\n");
> +
> +       ret = devm_regulator_get_enable(&spi->dev, "vio");
> +       if (ret)
> +               return dev_err_probe(&spi->dev, ret, "Failed to enable VIO supply\n");
> +
> +       vref_reg = devm_regulator_get(&spi->dev, "ref");
> +       if (IS_ERR(vref_reg))
> +               return dev_err_probe(&spi->dev, PTR_ERR(vref_reg),
> +                                    "Failed to get vref regulator\n");
> +
> +       ret = regulator_enable(vref_reg);
> +       if (ret < 0)
> +               return dev_err_probe(&spi->dev, ret,
> +                                    "Failed to enable voltage regulator\n");
> +
> +       ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg);
> +       if (ret)
> +               return dev_err_probe(&spi->dev, ret,
> +                                    "Failed to add regulator disable action\n");
> +
> +       st->vref = regulator_get_voltage(vref_reg);
> +       if (st->vref < 0)
> +               return dev_err_probe(&spi->dev, st->vref, "Failed to get vref\n");
> +
> +       st->cnv_gpio = devm_gpiod_get_optional(&spi->dev, "cnv", GPIOD_OUT_HIGH);
> +       if (IS_ERR(st->cnv_gpio)) {
> +               if (PTR_ERR(st->cnv_gpio) == -EPROBE_DEFER)
> +                       return -EPROBE_DEFER;

EPROBE_DEFER check is not needed with dev_err_probe();, it already does that.


> +
> +               return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio),
> +                                    "Failed to get CNV GPIO");
> +       }
> +
> +       ad4000_config(st);
> +
> +       indio_dev->name = chip->dev_name;
> +       indio_dev->info = &ad4000_info;
> +       indio_dev->channels = &chip->chan_spec;
> +       indio_dev->num_channels = 1;
> +
> +       st->pin_gain = AD4000_1_GAIN;
> +       if (device_property_present(&spi->dev, "adi,gain-milli")) {
> +               u32 val;

Should it be an error if adi,gain-milli is set on non-adaq chips?

> +
> +               ret = device_property_read_u32(&spi->dev, "adi,gain-milli", &val);
> +               if (ret)
> +                       return ret;
> +
> +               switch (val) {
> +               case 454:
> +                       st->pin_gain = AD4000_0454_GAIN;
> +                       break;
> +               case 909:
> +                       st->pin_gain = AD4000_0909_GAIN;
> +                       break;
> +               case 1000:
> +                       st->pin_gain = AD4000_1_GAIN;
> +                       break;
> +               case 1900:
> +                       st->pin_gain = AD4000_1900_GAIN;
> +                       break;
> +               default:
> +                       return dev_err_probe(&spi->dev, -EINVAL,
> +                                            "Invalid firmware provided gain\n");

Could help debugging if val is included in the error message.


> +               }
> +       }
> +
> +       /*
> +        * ADCs that output twos complement code have one less bit to express
> +        * voltage magnitude.
> +        */
> +       if (chip->chan_spec.scan_type.sign == 's')
> +               ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits - 1,
> +                                     chip);
> +       else
> +               ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits,
> +                                     chip);
> +
> +       ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
> +                                             &iio_pollfunc_store_time,
> +                                             &ad4000_trigger_handler, NULL);
> +       if (ret)
> +               return ret;
> +
> +       return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
> +static const struct spi_device_id ad4000_id[] = {
> +       { "ad4000", (kernel_ulong_t)&ad4000_chips[ID_AD4000] },
> +       { "ad4001", (kernel_ulong_t)&ad4000_chips[ID_AD4001] },
> +       { "ad4002", (kernel_ulong_t)&ad4000_chips[ID_AD4002] },
> +       { "ad4003", (kernel_ulong_t)&ad4000_chips[ID_AD4003] },
> +       { "ad4004", (kernel_ulong_t)&ad4000_chips[ID_AD4004] },
> +       { "ad4005", (kernel_ulong_t)&ad4000_chips[ID_AD4005] },
> +       { "ad4006", (kernel_ulong_t)&ad4000_chips[ID_AD4006] },
> +       { "ad4007", (kernel_ulong_t)&ad4000_chips[ID_AD4007] },
> +       { "ad4008", (kernel_ulong_t)&ad4000_chips[ID_AD4008] },
> +       { "ad4010", (kernel_ulong_t)&ad4000_chips[ID_AD4010] },
> +       { "ad4011", (kernel_ulong_t)&ad4000_chips[ID_AD4011] },
> +       { "ad4020", (kernel_ulong_t)&ad4000_chips[ID_AD4020] },
> +       { "ad4021", (kernel_ulong_t)&ad4000_chips[ID_AD4021] },
> +       { "ad4022", (kernel_ulong_t)&ad4000_chips[ID_AD4022] },
> +       { "adaq4001", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4001] },
> +       { "adaq4003", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4003] },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(spi, ad4000_id);
> +
> +static const struct of_device_id ad4000_of_match[] = {
> +       { .compatible = "adi,ad4000", .data = &ad4000_chips[ID_AD4000] },
> +       { .compatible = "adi,ad4001", .data = &ad4000_chips[ID_AD4001] },
> +       { .compatible = "adi,ad4002", .data = &ad4000_chips[ID_AD4002] },
> +       { .compatible = "adi,ad4003", .data = &ad4000_chips[ID_AD4003] },
> +       { .compatible = "adi,ad4004", .data = &ad4000_chips[ID_AD4004] },
> +       { .compatible = "adi,ad4005", .data = &ad4000_chips[ID_AD4005] },
> +       { .compatible = "adi,ad4006", .data = &ad4000_chips[ID_AD4006] },
> +       { .compatible = "adi,ad4007", .data = &ad4000_chips[ID_AD4007] },
> +       { .compatible = "adi,ad4008", .data = &ad4000_chips[ID_AD4008] },
> +       { .compatible = "adi,ad4010", .data = &ad4000_chips[ID_AD4010] },
> +       { .compatible = "adi,ad4011", .data = &ad4000_chips[ID_AD4011] },
> +       { .compatible = "adi,ad4020", .data = &ad4000_chips[ID_AD4020] },
> +       { .compatible = "adi,ad4021", .data = &ad4000_chips[ID_AD4021] },
> +       { .compatible = "adi,ad4022", .data = &ad4000_chips[ID_AD4022] },
> +       { .compatible = "adi,adaq4001", .data = &ad4000_chips[ID_ADAQ4001] },
> +       { .compatible = "adi,adaq4003", .data = &ad4000_chips[ID_ADAQ4003] },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, ad4000_of_match);
> +
> +static struct spi_driver ad4000_driver = {
> +       .driver = {
> +               .name   = "ad4000",
> +               .of_match_table = ad4000_of_match,
> +       },
> +       .probe          = ad4000_probe,
> +       .id_table       = ad4000_id,
> +};
> +module_spi_driver(ad4000_driver);
> +
> +MODULE_AUTHOR("Mircea Caprioru <mircea.caprioru@analog.com>");
> +MODULE_AUTHOR("Marcelo Schmitt <marcelo.schmitt@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD4000 ADC driver");
> +MODULE_LICENSE("GPL");
> --
> 2.43.0
>
>
Marcelo Schmitt April 9, 2024, 4:09 p.m. UTC | #2
On 04/08, David Lechner wrote:
> On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> <marcelo.schmitt@analog.com> wrote:
> >
> > Add support for AD4000 family of low noise, low power, high speed,
> > successive aproximation register (SAR) ADCs.
> >
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> >  MAINTAINERS              |   1 +
> >  drivers/iio/adc/Kconfig  |  12 +
> >  drivers/iio/adc/Makefile |   1 +
> >  drivers/iio/adc/ad4000.c | 649 +++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 663 insertions(+)
> >  create mode 100644 drivers/iio/adc/ad4000.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 5dfe118a5dd3..86aa96115f5a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1165,6 +1165,7 @@ L:        linux-iio@vger.kernel.org
> >  S:     Supported
> >  W:     https://ez.analog.com/linux-software-drivers
> >  F:     Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > +F:     drivers/iio/adc/ad4000.c
> >
> >  ANALOG DEVICES INC AD4130 DRIVER
> >  M:     Cosmin Tanislav <cosmin.tanislav@analog.com>
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 8db68b80b391..9c9d13d4b74f 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -21,6 +21,18 @@ config AD_SIGMA_DELTA
> >         select IIO_BUFFER
> >         select IIO_TRIGGERED_BUFFER
> >
> > +config AD4000
> > +       tristate "Analog Devices AD4000 ADC Driver"
> > +       depends on SPI
> > +       select IIO_BUFFER
> > +       select IIO_TRIGGERED_BUFFER
> > +       help
> > +         Say yes here to build support for Analog Devices AD4000 high speed
> > +         SPI analog to digital converters (ADC).
> > +
> > +         To compile this driver as a module, choose M here: the module will be
> > +         called ad4000.
> > +
> >  config AD4130
> >         tristate "Analog Device AD4130 ADC Driver"
> >         depends on SPI
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index edb32ce2af02..aa52068d864b 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -6,6 +6,7 @@
> >  # When adding new entries keep the list in alphabetical order
> >  obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
> >  obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
> > +obj-$(CONFIG_AD4000) += ad4000.o
> >  obj-$(CONFIG_AD4130) += ad4130.o
> >  obj-$(CONFIG_AD7091R) += ad7091r-base.o
> >  obj-$(CONFIG_AD7091R5) += ad7091r5.o
> > diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
> > new file mode 100644
> > index 000000000000..7997d9d98743
> > --- /dev/null
> > +++ b/drivers/iio/adc/ad4000.c
> > @@ -0,0 +1,649 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * AD4000 SPI ADC driver
> > + *
> > + * Copyright 2024 Analog Devices Inc.
> > + */
> > +#include <asm/unaligned.h>
> > +#include <linux/bits.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/kernel.h>
> > +#include <linux/math.h>
> > +#include <linux/module.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/spi/spi.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/units.h>
> > +#include <linux/util_macros.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +#include <linux/iio/buffer.h>
> > +#include <linux/iio/triggered_buffer.h>
> > +#include <linux/iio/trigger_consumer.h>
> > +
> > +#define AD400X_READ_COMMAND    0x54
> > +#define AD400X_WRITE_COMMAND   0x14
> > +
> > +/* AD4000 Configuration Register programmable bits */
> > +#define AD4000_STATUS          BIT(4) /* Status bits output */
> > +#define AD4000_SPAN_COMP       BIT(3) /* Input span compression  */
> > +#define AD4000_HIGHZ           BIT(2) /* High impedance mode  */
> > +#define AD4000_TURBO           BIT(1) /* Turbo mode */
> 
> Usually bits of the same register share a similar prefix, e.g.
> AD4000_CFG_TURBO, AD4000_CFG_HIGHZ, etc.

This only has one register, but if this makes things look better will do it.

> 
> > +
> > +#define AD4000_TQUIET2_NS              60
> > +
> > +#define AD4000_18BIT_MSK       GENMASK(31, 14)
> > +#define AD4000_20BIT_MSK       GENMASK(31, 12)
> > +
> > +#define AD4000_DIFF_CHANNEL(_sign, _real_bits)                         \
> > +       {                                                               \
> > +               .type = IIO_VOLTAGE,                                    \
> > +               .indexed = 1,                                           \
> > +               .differential = 1,                                      \
> > +               .channel = 0,                                           \
> > +               .channel2 = 1,                                          \
> > +               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |          \
> > +                                     BIT(IIO_CHAN_INFO_SCALE),         \
> > +               .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),\
> > +               .scan_type = {                                          \
> > +                       .sign = _sign,                                  \
> > +                       .realbits = _real_bits,                         \
> > +                       .storagebits = _real_bits > 16 ? 32 : 16,       \
> > +                       .shift = _real_bits > 16 ? 32 - _real_bits : 0, \
> > +                       .endianness = IIO_BE,                           \
> > +               },                                                      \
> > +       }                                                               \
> > +
> > +#define AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits)                  \
> > +       {                                                               \
> > +               .type = IIO_VOLTAGE,                                    \
> > +               .indexed = 1,                                           \
> > +               .channel = 0,                                           \
> > +               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |          \
> > +                                     BIT(IIO_CHAN_INFO_SCALE) |        \
> > +                                     BIT(IIO_CHAN_INFO_OFFSET),        \
> > +               .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),\
> > +               .scan_type = {                                          \
> > +                       .sign = _sign,                                  \
> > +                       .realbits = _real_bits,                         \
> > +                       .storagebits = _real_bits > 16 ? 32 : 16,       \
> > +                       .shift = _real_bits > 16 ? 32 - _real_bits : 0, \
> > +                       .endianness = IIO_BE,                           \
> > +               },                                                      \
> > +       }                                                               \
> 
> It looks like all differential chips are signed and all
> pseduo-differential chips are unsigned, so I don't think we need the
> _sign parameter in these macros.

That's correct, the _sign param can be removed after the split of channel macros.
Will do it for v3.

> 
> I also still have doubts about using IIO_BE and 8-bit xfers when it
> comes to adding support later to achieve max sample rate with a SPI
> offload. For example to get 2MSPS with an 18-bit chip, it will require
> an approx 33% faster SPI clock than the actual slowest clock possible
> because it will have to read 6 extra bits per sample. I didn't check
> the specs, but this may not even be physically possible without
> exceeding the datasheet max SPI clock rate. Also errors could be
> reduced if we could actually use the slowest allowable SPI clock rate.
> Furthermore, the offload hardware would have to be capable of adding
> an extra byte per sample for 18 and 20-bit chips when piping the data
> to DMA in order to get the 32-bit alignment in the buffer required by
> IIO scan_type and the natural alignment requirements of IIO buffers in
> general.

Maybe I should already implement support for reading with SPI offload
rather than doing it after this set is merged?
So we can test what happens at faster sample rates before we commit to a solution.

> 
> > +
> > +enum ad4000_ids {
> > +       ID_AD4000,
> > +       ID_AD4001,
> > +       ID_AD4002,
> > +       ID_AD4003,
> > +       ID_AD4004,
> > +       ID_AD4005,
> > +       ID_AD4006,
> > +       ID_AD4007,
> > +       ID_AD4008,
> > +       ID_AD4010,
> > +       ID_AD4011,
> > +       ID_AD4020,
> > +       ID_AD4021,
> > +       ID_AD4022,
> > +       ID_ADAQ4001,
> > +       ID_ADAQ4003,
> > +};
> > +
> > +struct ad4000_chip_info {
> > +       const char *dev_name;
> > +       struct iio_chan_spec chan_spec;
> > +};
> > +
> > +static const struct ad4000_chip_info ad4000_chips[] = {
> > +       [ID_AD4000] = {
> > +               .dev_name = "ad4000",
> > +               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
> > +       },
> > +       [ID_AD4001] = {
> > +               .dev_name = "ad4001",
> > +               .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
> > +       },
> > +       [ID_AD4002] = {
> > +               .dev_name = "ad4002",
> > +               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
> > +       },
> > +       [ID_AD4003] = {
> > +               .dev_name = "ad4003",
> > +               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
> > +       },
> > +       [ID_AD4004] = {
> > +               .dev_name = "ad4004",
> > +               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
> > +       },
> > +       [ID_AD4005] = {
> > +               .dev_name = "ad4005",
> > +               .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
> > +       },
> > +       [ID_AD4006] = {
> > +               .dev_name = "ad4006",
> > +               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
> > +       },
> > +       [ID_AD4007] = {
> > +               .dev_name = "ad4007",
> > +               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
> > +       },
> > +       [ID_AD4008] = {
> > +               .dev_name = "ad4008",
> > +               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
> > +       },
> > +       [ID_AD4010] = {
> > +               .dev_name = "ad4010",
> > +               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
> > +       },
> > +       [ID_AD4011] = {
> > +               .dev_name = "ad4011",
> > +               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
> > +       },
> > +       [ID_AD4020] = {
> > +               .dev_name = "ad4020",
> > +               .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
> > +       },
> > +       [ID_AD4021] = {
> > +               .dev_name = "ad4021",
> > +               .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
> > +       },
> > +       [ID_AD4022] = {
> > +               .dev_name = "ad4022",
> > +               .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
> > +       },
> > +       [ID_ADAQ4001] = {
> > +               .dev_name = "adaq4001",
> > +               .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
> > +       },
> > +       [ID_ADAQ4003] = {
> > +               .dev_name = "adaq4003",
> > +               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
> > +       },
> > +};
> > +
> > +enum ad4000_gains {
> > +       AD4000_0454_GAIN = 0,
> > +       AD4000_0909_GAIN = 1,
> > +       AD4000_1_GAIN = 2,
> 
> AD4000_1000_GAIN would be more consistent with the others.

Ack

> 
> > +       AD4000_1900_GAIN = 3,
> > +       AD4000_GAIN_LEN
> > +};
> > +
> > +/*
> > + * Gains stored and computed as fractions to avoid introducing rounding errors.
> > + */
> > +static const int ad4000_gains_frac[AD4000_GAIN_LEN][2] = {
> > +       [AD4000_0454_GAIN] = { 227, 500 },
> > +       [AD4000_0909_GAIN] = { 909, 1000 },
> > +       [AD4000_1_GAIN] = { 1, 1 },
> > +       [AD4000_1900_GAIN] = { 19, 10 },
> > +};
> 
> Why not just store the numerator in milli units and always use 1000
> for the denominator? It seems like it would simplify the code and make
> it easier to read and understand. Also, these values are coming from
> the adi,gain-milli property already, so we could avoid the enum and
> the lookup table entirely and simplify things even more.

Makes sense. Will do it.

> 
> > +
> > +struct ad4000_state {
> > +       struct spi_device *spi;
> > +       struct gpio_desc *cnv_gpio;
> > +       int vref;
> > +       bool status_bits;
> > +       bool span_comp;
> > +       bool turbo_mode;
> > +       bool high_z_mode;
> > +
> > +       enum ad4000_gains pin_gain;
> > +       int scale_tbl[AD4000_GAIN_LEN][2][2];
> > +
> > +       /*
> > +        * DMA (thus cache coherency maintenance) requires the
> > +        * transfer buffers to live in their own cache lines.
> > +        */
> > +       struct {
> > +               union {
> > +                       u16 sample_buf16;
> > +                       u32 sample_buf32;
> 
> Technically, these are holding big-endian data, so __be16 and __be32
> would be more correct.

Ack

> 
> > +               } data;
> > +               s64 timestamp __aligned(8);
> > +       } scan;
> > +       __be16 tx_buf __aligned(IIO_DMA_MINALIGN);
> > +       __be16 rx_buf;
> > +};
> 
> scan.data is used as SPI rx_buf so __aligned(IIO_DMA_MINALIGN); needs
> to be moved to the scan field.

I have already tried it. Maybe I did something wrong besides buffer alignment
at that time. Will give it another try.

> 
> > +
> > +static void ad4000_fill_scale_tbl(struct ad4000_state *st, int scale_bits,
> > +                                 const struct ad4000_chip_info *chip)
> > +{
> > +       int diff = chip->chan_spec.differential;
> > +       int val, val2, tmp0, tmp1, i;
> > +       u64 tmp2;
> > +
> > +       val2 = scale_bits;
> > +       for (i = 0; i < AD4000_GAIN_LEN; i++) {
> 
> Only one gain is selected by the devicetree, so why do we need to do
> this for all 4 gains?
> 

Good point. Will think better how to simplify this.

> > +               val = st->vref / 1000;
> > +               /* Multiply by MILLI here to avoid losing precision */
> > +               val = mult_frac(val, ad4000_gains_frac[i][1] * MILLI,
> > +                               ad4000_gains_frac[i][0]);
> > +               /* 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);
> > +               /* Store scale for when span compression is disabled */
> > +               st->scale_tbl[i][0][0] = tmp0; /* Integer part */
> > +               st->scale_tbl[i][0][1] = abs(tmp1); /* Fractional part */
> > +               /* Store scale for when span compression is enabled */
> > +               st->scale_tbl[i][1][0] = tmp0;
> > +               if (diff)
> > +                       st->scale_tbl[i][1][1] = DIV_ROUND_CLOSEST(abs(tmp1) * 4, 5);
> > +               else
> > +                       st->scale_tbl[i][1][1] = DIV_ROUND_CLOSEST(abs(tmp1) * 9, 10);
> > +       }
> > +}
> > +
> > +static int ad4000_write_reg(struct ad4000_state *st, uint8_t val)
> > +{
> > +       put_unaligned_be16(AD400X_WRITE_COMMAND << BITS_PER_BYTE | val,
> > +                          &st->tx_buf);
> > +       return spi_write(st->spi, &st->tx_buf, 2);
> > +}
> > +
> > +static int ad4000_read_reg(struct ad4000_state *st, unsigned int *val)
> > +{
> > +       struct spi_transfer t[] = {
> > +               {
> > +                       .tx_buf = &st->tx_buf,
> > +                       .rx_buf = &st->rx_buf,
> > +                       .len = 2,
> > +               },
> > +       };
> > +       int ret;
> > +
> > +       put_unaligned_be16(AD400X_READ_COMMAND << BITS_PER_BYTE, &st->tx_buf);
> > +       ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       *val = get_unaligned_be16(&st->rx_buf);
> > +
> > +       return ret;
> > +}
> > +
> 
> It would be very helpful to have comments here explaining the exact
> expected wiring configuration and signal timing here since there are
> so many possibilities for this chip.
> 

Ok, will be spliting this into different handling for the wiring modes so
will add comments to make clear what supports each configuration.

> > +static int ad4000_read_sample(struct ad4000_state *st,
> > +                             const struct iio_chan_spec *chan)
> > +{
> > +       struct spi_transfer t[] = {
> 
> Don't really need [] here since there is only one xfer.
> 
Ack

> > +               {
> > +                       .rx_buf = &st->scan.data,
> > +                       .len = BITS_TO_BYTES(chan->scan_type.storagebits),
> > +                       .delay = {
> > +                               .value = AD4000_TQUIET2_NS,
> > +                               .unit = SPI_DELAY_UNIT_NSECS,
> > +                       },
> > +               },
> > +       };
> > +       int ret;
> > +
> > +       ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       return 0;
> > +}
> > +
> > +static int ad4000_single_conversion(struct iio_dev *indio_dev,
> > +                                   const struct iio_chan_spec *chan, int *val)
> > +{
> > +       struct ad4000_state *st = iio_priv(indio_dev);
> > +       u32 sample;
> > +       int ret;
> > +
> > +       if (st->cnv_gpio)
> > +               gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_HIGH);
> 
> It would make more sense and be less redundant to move the gpio code
> into ad4000_read_sample().
> 
> Also, gpiod_set_value_cansleep() checks for NULL, so the if () is redundant.
> 
Good point. I think the execution flow migth change a bit here but will try
to avoid things like that.

> > +
> > +       ret = ad4000_read_sample(st, chan);
> > +       if (ret)
> > +               return ret;
> > +
> > +       if (st->cnv_gpio)
> > +               gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_LOW);
> > +
> > +       if (chan->scan_type.storagebits > 16)
> > +               sample = get_unaligned_be32(&st->scan.data);
> > +       else
> > +               sample = get_unaligned_be16(&st->scan.data);
> 
> data is aligned, so be32/16_to_cpu() should be fine. Also, Jonathan
> will suggest to use &st->scan.data.sample_b32/16 here too. :-)

Ack
> 
> > +
> > +       switch (chan->scan_type.realbits) {
> > +       case 16:
> > +               break;
> > +       case 18:
> > +               sample = FIELD_GET(AD4000_18BIT_MSK, sample);
> > +               break;
> > +       case 20:
> > +               sample = FIELD_GET(AD4000_20BIT_MSK, sample);
> > +               break;
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +
> > +       if (chan->scan_type.sign == 's')
> > +               *val = sign_extend32(sample, chan->scan_type.realbits - 1);
> > +
> > +       return IIO_VAL_INT;
> > +}
> > +
> > +static int ad4000_read_raw(struct iio_dev *indio_dev,
> > +                          struct iio_chan_spec const *chan, int *val,
> > +                          int *val2, long info)
> > +{
> > +       struct ad4000_state *st = iio_priv(indio_dev);
> > +
> > +       switch (info) {
> > +       case IIO_CHAN_INFO_RAW:
> > +               iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
> > +                       return ad4000_single_conversion(indio_dev, chan, val);
> > +               unreachable();
> > +       case IIO_CHAN_INFO_SCALE:
> > +               *val = st->scale_tbl[st->pin_gain][st->span_comp][0];
> > +               *val2 = st->scale_tbl[st->pin_gain][st->span_comp][1];
> > +               return IIO_VAL_INT_PLUS_NANO;
> > +       case IIO_CHAN_INFO_OFFSET:
> > +               *val = 0;
> > +               if (st->span_comp)
> > +                       *val = mult_frac(st->vref / 1000, 1, 10);
> > +
> > +               return IIO_VAL_INT;
> > +       default:
> > +               break;
> > +       }
> > +
> > +       return -EINVAL;
> > +}
> > +
> > +static int ad4000_read_avail(struct iio_dev *indio_dev,
> > +                            struct iio_chan_spec const *chan,
> > +                            const int **vals, int *type, int *length,
> > +                            long info)
> > +{
> > +       struct ad4000_state *st = iio_priv(indio_dev);
> > +
> > +       switch (info) {
> > +       case IIO_CHAN_INFO_SCALE:
> > +               *vals = (int *)st->scale_tbl[st->pin_gain];
> > +               *length = 2 * 2;
> > +               *type = IIO_VAL_INT_PLUS_NANO;
> > +               return IIO_AVAIL_LIST;
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +}
> > +
> > +static int ad4000_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;
> 
> not reachable because of default, so can be left out
> 
Ack

> > +}
> > +
> > +static int ad4000_write_raw(struct iio_dev *indio_dev,
> > +                           struct iio_chan_spec const *chan, int val, int val2,
> > +                           long mask)
> > +{
> > +       struct ad4000_state *st = iio_priv(indio_dev);
> > +       unsigned int reg_val;
> > +       bool span_comp_en;
> > +       int ret;
> > +
> > +       switch (mask) {
> > +       case IIO_CHAN_INFO_SCALE:
> > +               iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> > +                       ret = ad4000_read_reg(st, &reg_val);
> > +                       if (ret < 0)
> > +                               return ret;
> > +
> > +                       span_comp_en = (val2 == st->scale_tbl[st->pin_gain][1][1]);
> > +                       reg_val &= ~AD4000_SPAN_COMP;
> > +                       reg_val |= FIELD_PREP(AD4000_SPAN_COMP, span_comp_en);
> > +
> > +                       ret = ad4000_write_reg(st, reg_val);
> > +                       if (ret < 0)
> > +                               return ret;
> > +
> > +                       st->span_comp = span_comp_en;
> > +                       return 0;
> > +               }
> > +               unreachable();
> 
> Can bring out the return 0 to avoid unreachable.

Ack
> 
> > +       default:
> > +               break;
> 
> Can return -EINVAL to avoid break;
> 
Ack

> > +       }
> > +
> > +       return -EINVAL;
> > +}
> > +
> > +static irqreturn_t ad4000_trigger_handler(int irq, void *p)
> > +{
> > +       struct iio_poll_func *pf = p;
> > +       struct iio_dev *indio_dev = pf->indio_dev;
> > +       struct ad4000_state *st = iio_priv(indio_dev);
> > +       int ret;
> > +
> > +       if (st->cnv_gpio)
> > +               gpiod_set_value(st->cnv_gpio, GPIOD_OUT_HIGH);
> > +
> > +       ret = ad4000_read_sample(st, &indio_dev->channels[0]);
> > +       if (ret < 0)
> > +               goto err_out;
> > +
> > +       if (st->cnv_gpio)
> > +               gpiod_set_value(st->cnv_gpio, GPIOD_OUT_LOW);
> > +
> > +       iio_push_to_buffers_with_timestamp(indio_dev, &st->scan,
> > +                                          iio_get_time_ns(indio_dev));
> > +
> > +err_out:
> > +       iio_trigger_notify_done(indio_dev->trig);
> > +       return IRQ_HANDLED;
> > +}
> > +
> > +static const struct iio_info ad4000_info = {
> > +       .read_raw = &ad4000_read_raw,
> > +       .read_avail = &ad4000_read_avail,
> > +       .write_raw = &ad4000_write_raw,
> > +       .write_raw_get_fmt = &ad4000_write_raw_get_fmt,
> > +};
> > +
> > +static void ad4000_config(struct ad4000_state *st)
> > +{
> > +       unsigned int reg_val;
> > +       int ret;
> > +
> > +       reg_val = FIELD_PREP(AD4000_TURBO, 1);
> 
> Since the driver in it's current state can get anywhere near the max
> sample rate of ~1MSPS, I don't think it makes sense to enable turbo at
> this point.
> 

This is just enabling turbo at start up. If not enabling turbo during probe,
we would want(need?) to provide some interface for that, which might not be
much desired.

> > +
> > +       if (device_property_present(&st->spi->dev, "adi,high-z-input"))
> > +               reg_val |= FIELD_PREP(AD4000_HIGHZ, 1);
> > +
> > +       /*
> > +        * The ADC SDI pin might be connected to controller CS line in which
> > +        * case the write might fail. This, however, does not prevent the device
> > +        * from functioning even though in a configuration other than the
> > +        * requested one.
> > +        */
> > +       ret = ad4000_write_reg(st, reg_val);
> > +       if (ret < 0)
> > +               dev_dbg(&st->spi->dev, "Failed to config device\n");
> 
> If writing fails because there is no CS line wired up, we won't get an
> error returned here. The SPI controller has no way of knowing this
> happened, so it can only assume the write was successful and return 0.
> So this should return ret.
> 
Ok, ack.

> Ideally, the devicetree should tell us if CS is wired up or not.
> 
> > +}
> > +
> > +static void ad4000_regulator_disable(void *reg)
> > +{
> > +       regulator_disable(reg);
> > +}
> > +
> > +static int ad4000_probe(struct spi_device *spi)
> > +{
> > +       const struct ad4000_chip_info *chip;
> > +       struct regulator *vref_reg;
> > +       struct iio_dev *indio_dev;
> > +       struct ad4000_state *st;
> > +       int ret;
> 
> We need a check somewhere in here to make sure that adi,spi-mode is in
> a supported configuration. E.g. chain mode is not currently
> implemented.

ok, will add that.

> 
> > +
> > +       indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> > +       if (!indio_dev)
> > +               return -ENOMEM;
> > +
> > +       chip = spi_get_device_match_data(spi);
> > +       if (!chip)
> > +               return -EINVAL;
> > +
> > +       st = iio_priv(indio_dev);
> > +       st->spi = spi;
> > +
> > +       ret = devm_regulator_get_enable(&spi->dev, "vdd");
> > +       if (ret)
> > +               return dev_err_probe(&spi->dev, ret, "Failed to enable VDD supply\n");
> > +
> > +       ret = devm_regulator_get_enable(&spi->dev, "vio");
> > +       if (ret)
> > +               return dev_err_probe(&spi->dev, ret, "Failed to enable VIO supply\n");
> > +
> > +       vref_reg = devm_regulator_get(&spi->dev, "ref");
> > +       if (IS_ERR(vref_reg))
> > +               return dev_err_probe(&spi->dev, PTR_ERR(vref_reg),
> > +                                    "Failed to get vref regulator\n");
> > +
> > +       ret = regulator_enable(vref_reg);
> > +       if (ret < 0)
> > +               return dev_err_probe(&spi->dev, ret,
> > +                                    "Failed to enable voltage regulator\n");
> > +
> > +       ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg);
> > +       if (ret)
> > +               return dev_err_probe(&spi->dev, ret,
> > +                                    "Failed to add regulator disable action\n");
> > +
> > +       st->vref = regulator_get_voltage(vref_reg);
> > +       if (st->vref < 0)
> > +               return dev_err_probe(&spi->dev, st->vref, "Failed to get vref\n");
> > +
> > +       st->cnv_gpio = devm_gpiod_get_optional(&spi->dev, "cnv", GPIOD_OUT_HIGH);
> > +       if (IS_ERR(st->cnv_gpio)) {
> > +               if (PTR_ERR(st->cnv_gpio) == -EPROBE_DEFER)
> > +                       return -EPROBE_DEFER;
> 
> EPROBE_DEFER check is not needed with dev_err_probe();, it already does that.

Ack
> 
> 
> > +
> > +               return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio),
> > +                                    "Failed to get CNV GPIO");
> > +       }
> > +
> > +       ad4000_config(st);
> > +
> > +       indio_dev->name = chip->dev_name;
> > +       indio_dev->info = &ad4000_info;
> > +       indio_dev->channels = &chip->chan_spec;
> > +       indio_dev->num_channels = 1;
> > +
> > +       st->pin_gain = AD4000_1_GAIN;
> > +       if (device_property_present(&spi->dev, "adi,gain-milli")) {
> > +               u32 val;
> 
> Should it be an error if adi,gain-milli is set on non-adaq chips?

Maybe. We should not change the scale if it's a chip that don't have the
amplifier in front of the ADC. I think the best handling would be to just
ignore adi,gain-milli if it's not an ADAQ device. Maybe better add a DT
constraint,
  - if:
      properties:
        compatible:
          contains:
            enum:
              - adi,adaq4001
              - adi,adaq4003
    then:
      properties:
        adi,gain-milli: false
?

> 
> > +
> > +               ret = device_property_read_u32(&spi->dev, "adi,gain-milli", &val);
> > +               if (ret)
> > +                       return ret;
> > +
> > +               switch (val) {
> > +               case 454:
> > +                       st->pin_gain = AD4000_0454_GAIN;
> > +                       break;
> > +               case 909:
> > +                       st->pin_gain = AD4000_0909_GAIN;
> > +                       break;
> > +               case 1000:
> > +                       st->pin_gain = AD4000_1_GAIN;
> > +                       break;
> > +               case 1900:
> > +                       st->pin_gain = AD4000_1900_GAIN;
> > +                       break;
> > +               default:
> > +                       return dev_err_probe(&spi->dev, -EINVAL,
> > +                                            "Invalid firmware provided gain\n");
> 
> Could help debugging if val is included in the error message.

Ack
> 
> 
> > +               }
> > +       }
> > +
> > +       /*
> > +        * ADCs that output twos complement code have one less bit to express
> > +        * voltage magnitude.
> > +        */
> > +       if (chip->chan_spec.scan_type.sign == 's')
> > +               ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits - 1,
> > +                                     chip);
> > +       else
> > +               ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits,
> > +                                     chip);
> > +
> > +       ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
> > +                                             &iio_pollfunc_store_time,
> > +                                             &ad4000_trigger_handler, NULL);
> > +       if (ret)
> > +               return ret;
> > +
> > +       return devm_iio_device_register(&spi->dev, indio_dev);
> > +}
> > +
> > +static const struct spi_device_id ad4000_id[] = {
> > +       { "ad4000", (kernel_ulong_t)&ad4000_chips[ID_AD4000] },
> > +       { "ad4001", (kernel_ulong_t)&ad4000_chips[ID_AD4001] },
> > +       { "ad4002", (kernel_ulong_t)&ad4000_chips[ID_AD4002] },
> > +       { "ad4003", (kernel_ulong_t)&ad4000_chips[ID_AD4003] },
> > +       { "ad4004", (kernel_ulong_t)&ad4000_chips[ID_AD4004] },
> > +       { "ad4005", (kernel_ulong_t)&ad4000_chips[ID_AD4005] },
> > +       { "ad4006", (kernel_ulong_t)&ad4000_chips[ID_AD4006] },
> > +       { "ad4007", (kernel_ulong_t)&ad4000_chips[ID_AD4007] },
> > +       { "ad4008", (kernel_ulong_t)&ad4000_chips[ID_AD4008] },
> > +       { "ad4010", (kernel_ulong_t)&ad4000_chips[ID_AD4010] },
> > +       { "ad4011", (kernel_ulong_t)&ad4000_chips[ID_AD4011] },
> > +       { "ad4020", (kernel_ulong_t)&ad4000_chips[ID_AD4020] },
> > +       { "ad4021", (kernel_ulong_t)&ad4000_chips[ID_AD4021] },
> > +       { "ad4022", (kernel_ulong_t)&ad4000_chips[ID_AD4022] },
> > +       { "adaq4001", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4001] },
> > +       { "adaq4003", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4003] },
> > +       { }
> > +};
> > +MODULE_DEVICE_TABLE(spi, ad4000_id);
> > +
> > +static const struct of_device_id ad4000_of_match[] = {
> > +       { .compatible = "adi,ad4000", .data = &ad4000_chips[ID_AD4000] },
> > +       { .compatible = "adi,ad4001", .data = &ad4000_chips[ID_AD4001] },
> > +       { .compatible = "adi,ad4002", .data = &ad4000_chips[ID_AD4002] },
> > +       { .compatible = "adi,ad4003", .data = &ad4000_chips[ID_AD4003] },
> > +       { .compatible = "adi,ad4004", .data = &ad4000_chips[ID_AD4004] },
> > +       { .compatible = "adi,ad4005", .data = &ad4000_chips[ID_AD4005] },
> > +       { .compatible = "adi,ad4006", .data = &ad4000_chips[ID_AD4006] },
> > +       { .compatible = "adi,ad4007", .data = &ad4000_chips[ID_AD4007] },
> > +       { .compatible = "adi,ad4008", .data = &ad4000_chips[ID_AD4008] },
> > +       { .compatible = "adi,ad4010", .data = &ad4000_chips[ID_AD4010] },
> > +       { .compatible = "adi,ad4011", .data = &ad4000_chips[ID_AD4011] },
> > +       { .compatible = "adi,ad4020", .data = &ad4000_chips[ID_AD4020] },
> > +       { .compatible = "adi,ad4021", .data = &ad4000_chips[ID_AD4021] },
> > +       { .compatible = "adi,ad4022", .data = &ad4000_chips[ID_AD4022] },
> > +       { .compatible = "adi,adaq4001", .data = &ad4000_chips[ID_ADAQ4001] },
> > +       { .compatible = "adi,adaq4003", .data = &ad4000_chips[ID_ADAQ4003] },
> > +       { }
> > +};
> > +MODULE_DEVICE_TABLE(of, ad4000_of_match);
> > +
> > +static struct spi_driver ad4000_driver = {
> > +       .driver = {
> > +               .name   = "ad4000",
> > +               .of_match_table = ad4000_of_match,
> > +       },
> > +       .probe          = ad4000_probe,
> > +       .id_table       = ad4000_id,
> > +};
> > +module_spi_driver(ad4000_driver);
> > +
> > +MODULE_AUTHOR("Mircea Caprioru <mircea.caprioru@analog.com>");
> > +MODULE_AUTHOR("Marcelo Schmitt <marcelo.schmitt@analog.com>");
> > +MODULE_DESCRIPTION("Analog Devices AD4000 ADC driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.43.0
> >
> >
David Lechner April 9, 2024, 4:44 p.m. UTC | #3
On Tue, Apr 9, 2024 at 11:09 AM Marcelo Schmitt
<marcelo.schmitt1@gmail.com> wrote:
>
> On 04/08, David Lechner wrote:
> > On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> > <marcelo.schmitt@analog.com> wrote:
> > >

...

> >
> > I also still have doubts about using IIO_BE and 8-bit xfers when it
> > comes to adding support later to achieve max sample rate with a SPI
> > offload. For example to get 2MSPS with an 18-bit chip, it will require
> > an approx 33% faster SPI clock than the actual slowest clock possible
> > because it will have to read 6 extra bits per sample. I didn't check
> > the specs, but this may not even be physically possible without
> > exceeding the datasheet max SPI clock rate. Also errors could be
> > reduced if we could actually use the slowest allowable SPI clock rate.
> > Furthermore, the offload hardware would have to be capable of adding
> > an extra byte per sample for 18 and 20-bit chips when piping the data
> > to DMA in order to get the 32-bit alignment in the buffer required by
> > IIO scan_type and the natural alignment requirements of IIO buffers in
> > general.
>
> Maybe I should already implement support for reading with SPI offload
> rather than doing it after this set is merged?
> So we can test what happens at faster sample rates before we commit to a solution.
>

Yes, that sounds like a wise thing to do.

>
> >
> > > +               } data;
> > > +               s64 timestamp __aligned(8);
> > > +       } scan;
> > > +       __be16 tx_buf __aligned(IIO_DMA_MINALIGN);
> > > +       __be16 rx_buf;
> > > +};
> >
> > scan.data is used as SPI rx_buf so __aligned(IIO_DMA_MINALIGN); needs
> > to be moved to the scan field.
>
> I have already tried it. Maybe I did something wrong besides buffer alignment
> at that time. Will give it another try.

This is the alignment for DMA cache coherency. So it should not have
any affect on the actual data read, only performance.


> > > +static void ad4000_config(struct ad4000_state *st)
> > > +{
> > > +       unsigned int reg_val;
> > > +       int ret;
> > > +
> > > +       reg_val = FIELD_PREP(AD4000_TURBO, 1);
> >
> > Since the driver in it's current state can get anywhere near the max
> > sample rate of ~1MSPS, I don't think it makes sense to enable turbo at
> > this point.
> >
>
> This is just enabling turbo at start up. If not enabling turbo during probe,
> we would want(need?) to provide some interface for that, which might not be
> much desired.
>

TURBO is only needed to achieve the max sample rate of 500k/1M/2MSPS
on the various chips by skipping powering down some circuitry between
samples. We can't get anywhere close to that in Linux without some
sort of SPI offloading. So, for now, we might as well leave it
disabled and save some power.


> > > +
> > > +       st->pin_gain = AD4000_1_GAIN;
> > > +       if (device_property_present(&spi->dev, "adi,gain-milli")) {
> > > +               u32 val;
> >
> > Should it be an error if adi,gain-milli is set on non-adaq chips?
>
> Maybe. We should not change the scale if it's a chip that don't have the
> amplifier in front of the ADC. I think the best handling would be to just
> ignore adi,gain-milli if it's not an ADAQ device. Maybe better add a DT
> constraint,
>   - if:
>       properties:
>         compatible:
>           contains:
>             enum:
>               - adi,adaq4001
>               - adi,adaq4003
>     then:
>       properties:
>         adi,gain-milli: false
> ?

I think this is missing a not:, but otherwise yes this should be in
the DT bindings.

Even with that though, I would still be helpful to readers of the
driver to at least have a comment here pointing out that this property
and related gain scaling only applies to ADAQ chips.
Marcelo Schmitt April 9, 2024, 6:12 p.m. UTC | #4
On 04/09, David Lechner wrote:
> On Tue, Apr 9, 2024 at 11:09 AM Marcelo Schmitt
> <marcelo.schmitt1@gmail.com> wrote:
> >
> > On 04/08, David Lechner wrote:
> > > On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> > > <marcelo.schmitt@analog.com> wrote:
> > > >
> 
...
> 
> 
> > > > +static void ad4000_config(struct ad4000_state *st)
> > > > +{
> > > > +       unsigned int reg_val;
> > > > +       int ret;
> > > > +
> > > > +       reg_val = FIELD_PREP(AD4000_TURBO, 1);
> > >
> > > Since the driver in it's current state can get anywhere near the max
> > > sample rate of ~1MSPS, I don't think it makes sense to enable turbo at
> > > this point.
> > >
> >
> > This is just enabling turbo at start up. If not enabling turbo during probe,
> > we would want(need?) to provide some interface for that, which might not be
> > much desired.
> >
> 
> TURBO is only needed to achieve the max sample rate of 500k/1M/2MSPS
> on the various chips by skipping powering down some circuitry between
> samples. We can't get anywhere close to that in Linux without some
> sort of SPI offloading. So, for now, we might as well leave it
> disabled and save some power.
> 

Humm, ad4000 datasheets don't really mention power usage differences for turbo
mode like ad7944 datasheet do. Though, they should be similar.
Yeah, will leave turbo disabled until providing offload support.

> 
> > > > +
> > > > +       st->pin_gain = AD4000_1_GAIN;
> > > > +       if (device_property_present(&spi->dev, "adi,gain-milli")) {
> > > > +               u32 val;
> > >
> > > Should it be an error if adi,gain-milli is set on non-adaq chips?
> >
> > Maybe. We should not change the scale if it's a chip that don't have the
> > amplifier in front of the ADC. I think the best handling would be to just
> > ignore adi,gain-milli if it's not an ADAQ device. Maybe better add a DT
> > constraint,
> >   - if:
> >       properties:
> >         compatible:
> >           contains:
> >             enum:
> >               - adi,adaq4001
> >               - adi,adaq4003
> >     then:
> >       properties:
> >         adi,gain-milli: false
> > ?
> 
> I think this is missing a not:, but otherwise yes this should be in
> the DT bindings.

Oops, yeap, was missing the not:.

> 
> Even with that though, I would still be helpful to readers of the
> driver to at least have a comment here pointing out that this property
> and related gain scaling only applies to ADAQ chips.

ok, will add a comment.
Jonathan Cameron April 13, 2024, 4:19 p.m. UTC | #5
> 
> > +
> > +       ret = ad4000_read_sample(st, chan);
> > +       if (ret)
> > +               return ret;
> > +
> > +       if (st->cnv_gpio)
> > +               gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_LOW);
> > +
> > +       if (chan->scan_type.storagebits > 16)
> > +               sample = get_unaligned_be32(&st->scan.data);
> > +       else
> > +               sample = get_unaligned_be16(&st->scan.data);  
> 
> data is aligned, so be32/16_to_cpu() should be fine. Also, Jonathan
> will suggest to use &st->scan.data.sample_b32/16 here too. :-)

Sparse may well complain as well and no one likes build warnings ;)
Jonathan Cameron April 13, 2024, 4:34 p.m. UTC | #6
On Tue, 9 Apr 2024 11:44:26 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On Tue, Apr 9, 2024 at 11:09 AM Marcelo Schmitt
> <marcelo.schmitt1@gmail.com> wrote:
> >
> > On 04/08, David Lechner wrote:  
> > > On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> > > <marcelo.schmitt@analog.com> wrote:  
> > > >  
> 
> ...
> 
> > >
> > > I also still have doubts about using IIO_BE and 8-bit xfers when it
> > > comes to adding support later to achieve max sample rate with a SPI
> > > offload. For example to get 2MSPS with an 18-bit chip, it will require
> > > an approx 33% faster SPI clock than the actual slowest clock possible
> > > because it will have to read 6 extra bits per sample. I didn't check
> > > the specs, but this may not even be physically possible without
> > > exceeding the datasheet max SPI clock rate. Also errors could be
> > > reduced if we could actually use the slowest allowable SPI clock rate.
> > > Furthermore, the offload hardware would have to be capable of adding
> > > an extra byte per sample for 18 and 20-bit chips when piping the data
> > > to DMA in order to get the 32-bit alignment in the buffer required by
> > > IIO scan_type and the natural alignment requirements of IIO buffers in
> > > general.  
> >
> > Maybe I should already implement support for reading with SPI offload
> > rather than doing it after this set is merged?
> > So we can test what happens at faster sample rates before we commit to a solution.
> >  
> 
> Yes, that sounds like a wise thing to do.
> 
> >  
> > >  
> > > > +               } data;
> > > > +               s64 timestamp __aligned(8);
> > > > +       } scan;
> > > > +       __be16 tx_buf __aligned(IIO_DMA_MINALIGN);
> > > > +       __be16 rx_buf;
> > > > +};  
> > >
> > > scan.data is used as SPI rx_buf so __aligned(IIO_DMA_MINALIGN); needs
> > > to be moved to the scan field.  
> >
> > I have already tried it. Maybe I did something wrong besides buffer alignment
> > at that time. Will give it another try.  
> 
> This is the alignment for DMA cache coherency. So it should not have
> any affect on the actual data read, only performance.

Nope. It's a correctness issue not a performance one (though you may get
advantages there as well)  You can get potential corruption of
other fields that end up in the same cacheline - so the aim is to make
sure that nothing that we might use concurrently is in that cacheline.
 
There was a good description of what is going on here in a talk Wolfram
gave a few years back when he was exploring how to avoid bounce buffers
in I2C https://www.youtube.com/watch?v=JDwaMClvV-s that includes links to descriptions
of the fun that can happen.  The short description is that a DMA controller is
allowed to grab the whole of a cacheline (typically 64 bytes, can be bigger)
in coherently from the host (basically takes a copy).  It can then merrily
do it's operations before finally copying it back to the actual memory.
The problem lies in that there may be other data in that cacheline that
is accessed at whilst the DMA controller was working on it's own prviate
copy.  Those changes will be wiped out.

Now you probably didn't see it because:
a) Many controllers don't do this - either they don't cache stale data, or
   are sufficiently coherent with CPU caches etc that any changes in this
   'near by' data are correctly handled.

b) It's really hard to hit the races anyway. I've only seen it once when
   debugging real systems, but people run into this occasionally on IIO
   drivers and it is really nasty to debug.

c) On arm64 at least in many cases the DMA core will bounce buffer anyway
   after some changes made a couple of years back.  Unfortunately that isn't
   true on all architectures yet (I think anyway) so we still need to be
   careful around this.

Note some architectures (x86 I think) never allowed this cacheline corruption
path in the first place so the DMA_MINALIGN value is 8 bytes (IIRC).

Coherency is hard (but fun if you have time, particularly the places where
it is allowed to break and how they are avoided :)

Jonathan
Jonathan Cameron April 13, 2024, 4:39 p.m. UTC | #7
On Mon, 8 Apr 2024 11:31:42 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:

> Add support for AD4000 family of low noise, low power, high speed,
> successive aproximation register (SAR) ADCs.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
A few trivial comments inline to add to David's much more thorough review.

> +#include <asm/unaligned.h>
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/math.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sysfs.h>
> +#include <linux/units.h>
> +#include <linux/util_macros.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>

You don't have custom attributes, so doubt you need that include.

> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>

> +
> +static int ad4000_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan, int *val,
> +			   int *val2, long info)
> +{
> +	struct ad4000_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
> +			return ad4000_single_conversion(indio_dev, chan, val);
> +		unreachable();
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = st->scale_tbl[st->pin_gain][st->span_comp][0];
> +		*val2 = st->scale_tbl[st->pin_gain][st->span_comp][1];
> +		return IIO_VAL_INT_PLUS_NANO;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = 0;
> +		if (st->span_comp)
> +			*val = mult_frac(st->vref / 1000, 1, 10);
> +
> +		return IIO_VAL_INT;
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
As below - I'd push into the default: and drop down here.

> +}
> +
> +static int ad4000_read_avail(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     const int **vals, int *type, int *length,
> +			     long info)
> +{
> +	struct ad4000_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_SCALE:
> +		*vals = (int *)st->scale_tbl[st->pin_gain];
> +		*length = 2 * 2;
> +		*type = IIO_VAL_INT_PLUS_NANO;
> +		return IIO_AVAIL_LIST;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad4000_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;

dead code.

> +}
> +
> +static int ad4000_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int val, int val2,
> +			    long mask)
> +{
> +	struct ad4000_state *st = iio_priv(indio_dev);
> +	unsigned int reg_val;
> +	bool span_comp_en;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> +			ret = ad4000_read_reg(st, &reg_val);
> +			if (ret < 0)
> +				return ret;
> +
> +			span_comp_en = (val2 == st->scale_tbl[st->pin_gain][1][1]);
> +			reg_val &= ~AD4000_SPAN_COMP;
> +			reg_val |= FIELD_PREP(AD4000_SPAN_COMP, span_comp_en);
> +
> +			ret = ad4000_write_reg(st, reg_val);
> +			if (ret < 0)
> +				return ret;
> +
> +			st->span_comp = span_comp_en;
> +			return 0;
> +		}
> +		unreachable();
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
Trivial but could push up into the default and make it clear there are no other ways out
of the switch statement.
> +}
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 5dfe118a5dd3..86aa96115f5a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1165,6 +1165,7 @@  L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
+F:	drivers/iio/adc/ad4000.c
 
 ANALOG DEVICES INC AD4130 DRIVER
 M:	Cosmin Tanislav <cosmin.tanislav@analog.com>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 8db68b80b391..9c9d13d4b74f 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -21,6 +21,18 @@  config AD_SIGMA_DELTA
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
 
+config AD4000
+	tristate "Analog Devices AD4000 ADC Driver"
+	depends on SPI
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  Say yes here to build support for Analog Devices AD4000 high speed
+	  SPI analog to digital converters (ADC).
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ad4000.
+
 config AD4130
 	tristate "Analog Device AD4130 ADC Driver"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index edb32ce2af02..aa52068d864b 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -6,6 +6,7 @@ 
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
 obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
+obj-$(CONFIG_AD4000) += ad4000.o
 obj-$(CONFIG_AD4130) += ad4130.o
 obj-$(CONFIG_AD7091R) += ad7091r-base.o
 obj-$(CONFIG_AD7091R5) += ad7091r5.o
diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
new file mode 100644
index 000000000000..7997d9d98743
--- /dev/null
+++ b/drivers/iio/adc/ad4000.c
@@ -0,0 +1,649 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD4000 SPI ADC driver
+ *
+ * Copyright 2024 Analog Devices Inc.
+ */
+#include <asm/unaligned.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+#include <linux/units.h>
+#include <linux/util_macros.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+
+#define AD400X_READ_COMMAND	0x54
+#define AD400X_WRITE_COMMAND	0x14
+
+/* AD4000 Configuration Register programmable bits */
+#define AD4000_STATUS		BIT(4) /* Status bits output */
+#define AD4000_SPAN_COMP	BIT(3) /* Input span compression  */
+#define AD4000_HIGHZ		BIT(2) /* High impedance mode  */
+#define AD4000_TURBO		BIT(1) /* Turbo mode */
+
+#define AD4000_TQUIET2_NS		60
+
+#define AD4000_18BIT_MSK	GENMASK(31, 14)
+#define AD4000_20BIT_MSK	GENMASK(31, 12)
+
+#define AD4000_DIFF_CHANNEL(_sign, _real_bits)				\
+	{								\
+		.type = IIO_VOLTAGE,					\
+		.indexed = 1,						\
+		.differential = 1,					\
+		.channel = 0,						\
+		.channel2 = 1,						\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
+				      BIT(IIO_CHAN_INFO_SCALE),		\
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),\
+		.scan_type = {						\
+			.sign = _sign,					\
+			.realbits = _real_bits,				\
+			.storagebits = _real_bits > 16 ? 32 : 16,	\
+			.shift = _real_bits > 16 ? 32 - _real_bits : 0,	\
+			.endianness = IIO_BE,				\
+		},							\
+	}								\
+
+#define AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits)			\
+	{								\
+		.type = IIO_VOLTAGE,					\
+		.indexed = 1,						\
+		.channel = 0,						\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
+				      BIT(IIO_CHAN_INFO_SCALE) |	\
+				      BIT(IIO_CHAN_INFO_OFFSET),	\
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),\
+		.scan_type = {						\
+			.sign = _sign,					\
+			.realbits = _real_bits,				\
+			.storagebits = _real_bits > 16 ? 32 : 16,	\
+			.shift = _real_bits > 16 ? 32 - _real_bits : 0,	\
+			.endianness = IIO_BE,				\
+		},							\
+	}								\
+
+enum ad4000_ids {
+	ID_AD4000,
+	ID_AD4001,
+	ID_AD4002,
+	ID_AD4003,
+	ID_AD4004,
+	ID_AD4005,
+	ID_AD4006,
+	ID_AD4007,
+	ID_AD4008,
+	ID_AD4010,
+	ID_AD4011,
+	ID_AD4020,
+	ID_AD4021,
+	ID_AD4022,
+	ID_ADAQ4001,
+	ID_ADAQ4003,
+};
+
+struct ad4000_chip_info {
+	const char *dev_name;
+	struct iio_chan_spec chan_spec;
+};
+
+static const struct ad4000_chip_info ad4000_chips[] = {
+	[ID_AD4000] = {
+		.dev_name = "ad4000",
+		.chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
+	},
+	[ID_AD4001] = {
+		.dev_name = "ad4001",
+		.chan_spec = AD4000_DIFF_CHANNEL('s', 16),
+	},
+	[ID_AD4002] = {
+		.dev_name = "ad4002",
+		.chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
+	},
+	[ID_AD4003] = {
+		.dev_name = "ad4003",
+		.chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+	},
+	[ID_AD4004] = {
+		.dev_name = "ad4004",
+		.chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
+	},
+	[ID_AD4005] = {
+		.dev_name = "ad4005",
+		.chan_spec = AD4000_DIFF_CHANNEL('s', 16),
+	},
+	[ID_AD4006] = {
+		.dev_name = "ad4006",
+		.chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
+	},
+	[ID_AD4007] = {
+		.dev_name = "ad4007",
+		.chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+	},
+	[ID_AD4008] = {
+		.dev_name = "ad4008",
+		.chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
+	},
+	[ID_AD4010] = {
+		.dev_name = "ad4010",
+		.chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
+	},
+	[ID_AD4011] = {
+		.dev_name = "ad4011",
+		.chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+	},
+	[ID_AD4020] = {
+		.dev_name = "ad4020",
+		.chan_spec = AD4000_DIFF_CHANNEL('s', 20),
+	},
+	[ID_AD4021] = {
+		.dev_name = "ad4021",
+		.chan_spec = AD4000_DIFF_CHANNEL('s', 20),
+	},
+	[ID_AD4022] = {
+		.dev_name = "ad4022",
+		.chan_spec = AD4000_DIFF_CHANNEL('s', 20),
+	},
+	[ID_ADAQ4001] = {
+		.dev_name = "adaq4001",
+		.chan_spec = AD4000_DIFF_CHANNEL('s', 16),
+	},
+	[ID_ADAQ4003] = {
+		.dev_name = "adaq4003",
+		.chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+	},
+};
+
+enum ad4000_gains {
+	AD4000_0454_GAIN = 0,
+	AD4000_0909_GAIN = 1,
+	AD4000_1_GAIN = 2,
+	AD4000_1900_GAIN = 3,
+	AD4000_GAIN_LEN
+};
+
+/*
+ * Gains stored and computed as fractions to avoid introducing rounding errors.
+ */
+static const int ad4000_gains_frac[AD4000_GAIN_LEN][2] = {
+	[AD4000_0454_GAIN] = { 227, 500 },
+	[AD4000_0909_GAIN] = { 909, 1000 },
+	[AD4000_1_GAIN] = { 1, 1 },
+	[AD4000_1900_GAIN] = { 19, 10 },
+};
+
+struct ad4000_state {
+	struct spi_device *spi;
+	struct gpio_desc *cnv_gpio;
+	int vref;
+	bool status_bits;
+	bool span_comp;
+	bool turbo_mode;
+	bool high_z_mode;
+
+	enum ad4000_gains pin_gain;
+	int scale_tbl[AD4000_GAIN_LEN][2][2];
+
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 */
+	struct {
+		union {
+			u16 sample_buf16;
+			u32 sample_buf32;
+		} data;
+		s64 timestamp __aligned(8);
+	} scan;
+	__be16 tx_buf __aligned(IIO_DMA_MINALIGN);
+	__be16 rx_buf;
+};
+
+static void ad4000_fill_scale_tbl(struct ad4000_state *st, int scale_bits,
+				  const struct ad4000_chip_info *chip)
+{
+	int diff = chip->chan_spec.differential;
+	int val, val2, tmp0, tmp1, i;
+	u64 tmp2;
+
+	val2 = scale_bits;
+	for (i = 0; i < AD4000_GAIN_LEN; i++) {
+		val = st->vref / 1000;
+		/* Multiply by MILLI here to avoid losing precision */
+		val = mult_frac(val, ad4000_gains_frac[i][1] * MILLI,
+				ad4000_gains_frac[i][0]);
+		/* 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);
+		/* Store scale for when span compression is disabled */
+		st->scale_tbl[i][0][0] = tmp0; /* Integer part */
+		st->scale_tbl[i][0][1] = abs(tmp1); /* Fractional part */
+		/* Store scale for when span compression is enabled */
+		st->scale_tbl[i][1][0] = tmp0;
+		if (diff)
+			st->scale_tbl[i][1][1] = DIV_ROUND_CLOSEST(abs(tmp1) * 4, 5);
+		else
+			st->scale_tbl[i][1][1] = DIV_ROUND_CLOSEST(abs(tmp1) * 9, 10);
+	}
+}
+
+static int ad4000_write_reg(struct ad4000_state *st, uint8_t val)
+{
+	put_unaligned_be16(AD400X_WRITE_COMMAND << BITS_PER_BYTE | val,
+			   &st->tx_buf);
+	return spi_write(st->spi, &st->tx_buf, 2);
+}
+
+static int ad4000_read_reg(struct ad4000_state *st, unsigned int *val)
+{
+	struct spi_transfer t[] = {
+		{
+			.tx_buf = &st->tx_buf,
+			.rx_buf = &st->rx_buf,
+			.len = 2,
+		},
+	};
+	int ret;
+
+	put_unaligned_be16(AD400X_READ_COMMAND << BITS_PER_BYTE, &st->tx_buf);
+	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
+	if (ret < 0)
+		return ret;
+
+	*val = get_unaligned_be16(&st->rx_buf);
+
+	return ret;
+}
+
+static int ad4000_read_sample(struct ad4000_state *st,
+			      const struct iio_chan_spec *chan)
+{
+	struct spi_transfer t[] = {
+		{
+			.rx_buf = &st->scan.data,
+			.len = BITS_TO_BYTES(chan->scan_type.storagebits),
+			.delay = {
+				.value = AD4000_TQUIET2_NS,
+				.unit = SPI_DELAY_UNIT_NSECS,
+			},
+		},
+	};
+	int ret;
+
+	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int ad4000_single_conversion(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan, int *val)
+{
+	struct ad4000_state *st = iio_priv(indio_dev);
+	u32 sample;
+	int ret;
+
+	if (st->cnv_gpio)
+		gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_HIGH);
+
+	ret = ad4000_read_sample(st, chan);
+	if (ret)
+		return ret;
+
+	if (st->cnv_gpio)
+		gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_LOW);
+
+	if (chan->scan_type.storagebits > 16)
+		sample = get_unaligned_be32(&st->scan.data);
+	else
+		sample = get_unaligned_be16(&st->scan.data);
+
+	switch (chan->scan_type.realbits) {
+	case 16:
+		break;
+	case 18:
+		sample = FIELD_GET(AD4000_18BIT_MSK, sample);
+		break;
+	case 20:
+		sample = FIELD_GET(AD4000_20BIT_MSK, sample);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (chan->scan_type.sign == 's')
+		*val = sign_extend32(sample, chan->scan_type.realbits - 1);
+
+	return IIO_VAL_INT;
+}
+
+static int ad4000_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan, int *val,
+			   int *val2, long info)
+{
+	struct ad4000_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
+			return ad4000_single_conversion(indio_dev, chan, val);
+		unreachable();
+	case IIO_CHAN_INFO_SCALE:
+		*val = st->scale_tbl[st->pin_gain][st->span_comp][0];
+		*val2 = st->scale_tbl[st->pin_gain][st->span_comp][1];
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = 0;
+		if (st->span_comp)
+			*val = mult_frac(st->vref / 1000, 1, 10);
+
+		return IIO_VAL_INT;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int ad4000_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long info)
+{
+	struct ad4000_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (int *)st->scale_tbl[st->pin_gain];
+		*length = 2 * 2;
+		*type = IIO_VAL_INT_PLUS_NANO;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad4000_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 ad4000_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int val, int val2,
+			    long mask)
+{
+	struct ad4000_state *st = iio_priv(indio_dev);
+	unsigned int reg_val;
+	bool span_comp_en;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
+			ret = ad4000_read_reg(st, &reg_val);
+			if (ret < 0)
+				return ret;
+
+			span_comp_en = (val2 == st->scale_tbl[st->pin_gain][1][1]);
+			reg_val &= ~AD4000_SPAN_COMP;
+			reg_val |= FIELD_PREP(AD4000_SPAN_COMP, span_comp_en);
+
+			ret = ad4000_write_reg(st, reg_val);
+			if (ret < 0)
+				return ret;
+
+			st->span_comp = span_comp_en;
+			return 0;
+		}
+		unreachable();
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static irqreturn_t ad4000_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct ad4000_state *st = iio_priv(indio_dev);
+	int ret;
+
+	if (st->cnv_gpio)
+		gpiod_set_value(st->cnv_gpio, GPIOD_OUT_HIGH);
+
+	ret = ad4000_read_sample(st, &indio_dev->channels[0]);
+	if (ret < 0)
+		goto err_out;
+
+	if (st->cnv_gpio)
+		gpiod_set_value(st->cnv_gpio, GPIOD_OUT_LOW);
+
+	iio_push_to_buffers_with_timestamp(indio_dev, &st->scan,
+					   iio_get_time_ns(indio_dev));
+
+err_out:
+	iio_trigger_notify_done(indio_dev->trig);
+	return IRQ_HANDLED;
+}
+
+static const struct iio_info ad4000_info = {
+	.read_raw = &ad4000_read_raw,
+	.read_avail = &ad4000_read_avail,
+	.write_raw = &ad4000_write_raw,
+	.write_raw_get_fmt = &ad4000_write_raw_get_fmt,
+};
+
+static void ad4000_config(struct ad4000_state *st)
+{
+	unsigned int reg_val;
+	int ret;
+
+	reg_val = FIELD_PREP(AD4000_TURBO, 1);
+
+	if (device_property_present(&st->spi->dev, "adi,high-z-input"))
+		reg_val |= FIELD_PREP(AD4000_HIGHZ, 1);
+
+	/*
+	 * The ADC SDI pin might be connected to controller CS line in which
+	 * case the write might fail. This, however, does not prevent the device
+	 * from functioning even though in a configuration other than the
+	 * requested one.
+	 */
+	ret = ad4000_write_reg(st, reg_val);
+	if (ret < 0)
+		dev_dbg(&st->spi->dev, "Failed to config device\n");
+}
+
+static void ad4000_regulator_disable(void *reg)
+{
+	regulator_disable(reg);
+}
+
+static int ad4000_probe(struct spi_device *spi)
+{
+	const struct ad4000_chip_info *chip;
+	struct regulator *vref_reg;
+	struct iio_dev *indio_dev;
+	struct ad4000_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	chip = spi_get_device_match_data(spi);
+	if (!chip)
+		return -EINVAL;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+
+	ret = devm_regulator_get_enable(&spi->dev, "vdd");
+	if (ret)
+		return dev_err_probe(&spi->dev, ret, "Failed to enable VDD supply\n");
+
+	ret = devm_regulator_get_enable(&spi->dev, "vio");
+	if (ret)
+		return dev_err_probe(&spi->dev, ret, "Failed to enable VIO supply\n");
+
+	vref_reg = devm_regulator_get(&spi->dev, "ref");
+	if (IS_ERR(vref_reg))
+		return dev_err_probe(&spi->dev, PTR_ERR(vref_reg),
+				     "Failed to get vref regulator\n");
+
+	ret = regulator_enable(vref_reg);
+	if (ret < 0)
+		return dev_err_probe(&spi->dev, ret,
+				     "Failed to enable voltage regulator\n");
+
+	ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret,
+				     "Failed to add regulator disable action\n");
+
+	st->vref = regulator_get_voltage(vref_reg);
+	if (st->vref < 0)
+		return dev_err_probe(&spi->dev, st->vref, "Failed to get vref\n");
+
+	st->cnv_gpio = devm_gpiod_get_optional(&spi->dev, "cnv", GPIOD_OUT_HIGH);
+	if (IS_ERR(st->cnv_gpio)) {
+		if (PTR_ERR(st->cnv_gpio) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
+		return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio),
+				     "Failed to get CNV GPIO");
+	}
+
+	ad4000_config(st);
+
+	indio_dev->name = chip->dev_name;
+	indio_dev->info = &ad4000_info;
+	indio_dev->channels = &chip->chan_spec;
+	indio_dev->num_channels = 1;
+
+	st->pin_gain = AD4000_1_GAIN;
+	if (device_property_present(&spi->dev, "adi,gain-milli")) {
+		u32 val;
+
+		ret = device_property_read_u32(&spi->dev, "adi,gain-milli", &val);
+		if (ret)
+			return ret;
+
+		switch (val) {
+		case 454:
+			st->pin_gain = AD4000_0454_GAIN;
+			break;
+		case 909:
+			st->pin_gain = AD4000_0909_GAIN;
+			break;
+		case 1000:
+			st->pin_gain = AD4000_1_GAIN;
+			break;
+		case 1900:
+			st->pin_gain = AD4000_1900_GAIN;
+			break;
+		default:
+			return dev_err_probe(&spi->dev, -EINVAL,
+					     "Invalid firmware provided gain\n");
+		}
+	}
+
+	/*
+	 * ADCs that output twos complement code have one less bit to express
+	 * voltage magnitude.
+	 */
+	if (chip->chan_spec.scan_type.sign == 's')
+		ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits - 1,
+				      chip);
+	else
+		ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits,
+				      chip);
+
+	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
+					      &iio_pollfunc_store_time,
+					      &ad4000_trigger_handler, NULL);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id ad4000_id[] = {
+	{ "ad4000", (kernel_ulong_t)&ad4000_chips[ID_AD4000] },
+	{ "ad4001", (kernel_ulong_t)&ad4000_chips[ID_AD4001] },
+	{ "ad4002", (kernel_ulong_t)&ad4000_chips[ID_AD4002] },
+	{ "ad4003", (kernel_ulong_t)&ad4000_chips[ID_AD4003] },
+	{ "ad4004", (kernel_ulong_t)&ad4000_chips[ID_AD4004] },
+	{ "ad4005", (kernel_ulong_t)&ad4000_chips[ID_AD4005] },
+	{ "ad4006", (kernel_ulong_t)&ad4000_chips[ID_AD4006] },
+	{ "ad4007", (kernel_ulong_t)&ad4000_chips[ID_AD4007] },
+	{ "ad4008", (kernel_ulong_t)&ad4000_chips[ID_AD4008] },
+	{ "ad4010", (kernel_ulong_t)&ad4000_chips[ID_AD4010] },
+	{ "ad4011", (kernel_ulong_t)&ad4000_chips[ID_AD4011] },
+	{ "ad4020", (kernel_ulong_t)&ad4000_chips[ID_AD4020] },
+	{ "ad4021", (kernel_ulong_t)&ad4000_chips[ID_AD4021] },
+	{ "ad4022", (kernel_ulong_t)&ad4000_chips[ID_AD4022] },
+	{ "adaq4001", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4001] },
+	{ "adaq4003", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4003] },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad4000_id);
+
+static const struct of_device_id ad4000_of_match[] = {
+	{ .compatible = "adi,ad4000", .data = &ad4000_chips[ID_AD4000] },
+	{ .compatible = "adi,ad4001", .data = &ad4000_chips[ID_AD4001] },
+	{ .compatible = "adi,ad4002", .data = &ad4000_chips[ID_AD4002] },
+	{ .compatible = "adi,ad4003", .data = &ad4000_chips[ID_AD4003] },
+	{ .compatible = "adi,ad4004", .data = &ad4000_chips[ID_AD4004] },
+	{ .compatible = "adi,ad4005", .data = &ad4000_chips[ID_AD4005] },
+	{ .compatible = "adi,ad4006", .data = &ad4000_chips[ID_AD4006] },
+	{ .compatible = "adi,ad4007", .data = &ad4000_chips[ID_AD4007] },
+	{ .compatible = "adi,ad4008", .data = &ad4000_chips[ID_AD4008] },
+	{ .compatible = "adi,ad4010", .data = &ad4000_chips[ID_AD4010] },
+	{ .compatible = "adi,ad4011", .data = &ad4000_chips[ID_AD4011] },
+	{ .compatible = "adi,ad4020", .data = &ad4000_chips[ID_AD4020] },
+	{ .compatible = "adi,ad4021", .data = &ad4000_chips[ID_AD4021] },
+	{ .compatible = "adi,ad4022", .data = &ad4000_chips[ID_AD4022] },
+	{ .compatible = "adi,adaq4001", .data = &ad4000_chips[ID_ADAQ4001] },
+	{ .compatible = "adi,adaq4003", .data = &ad4000_chips[ID_ADAQ4003] },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad4000_of_match);
+
+static struct spi_driver ad4000_driver = {
+	.driver = {
+		.name   = "ad4000",
+		.of_match_table = ad4000_of_match,
+	},
+	.probe          = ad4000_probe,
+	.id_table       = ad4000_id,
+};
+module_spi_driver(ad4000_driver);
+
+MODULE_AUTHOR("Mircea Caprioru <mircea.caprioru@analog.com>");
+MODULE_AUTHOR("Marcelo Schmitt <marcelo.schmitt@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD4000 ADC driver");
+MODULE_LICENSE("GPL");