Add support for AD4080 high-speed, low noise, low distortion,
20-bit, Easy Drive, successive approximation register (SAR)
analog-to-digital converter (ADC).
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v3:
- name the defines to make associacion with register name.
- drop redundant defines.
- add trailing comma when needed.
- drop explicit masking and use field_prep instead
- add fsleep during sync process.
- do not wrap where 80 chars is not exceeded.
- use space for { }
- drop channel definition macro
- return dev_info on chip id mismatch.
- flip expression to `if (!st->lvds_cnv_en)`
- rework num_lanes property parse.
- update the driver based on the new edits on the backend interface related to
this part and the last disscussions in v2.
drivers/iio/adc/Kconfig | 14 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad4080.c | 618 +++++++++++++++++++++++++++++++++++++++
3 files changed, 633 insertions(+)
create mode 100644 drivers/iio/adc/ad4080.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 27413516216c..17df328f5322 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -47,6 +47,20 @@ config AD4030
To compile this driver as a module, choose M here: the module will be
called ad4030.
+config AD4080
+ tristate "Analog Devices AD4080 high speed ADC"
+ depends on SPI
+ select REGMAP_SPI
+ select IIO_BACKEND
+ help
+ Say yes here to build support for Analog Devices AD4080
+ high speed, low noise, low distortion, 20-bit, Easy Drive,
+ successive approximation register (SAR) analog-to-digital
+ converter (ADC). Supports iio_backended devices for AD4080.
+
+ To compile this driver as a module, choose M here: the module will be
+ called ad4080.
+
config AD4130
tristate "Analog Device AD4130 ADC Driver"
depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 9f26d5eca822..e6efed5b4e7a 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
obj-$(CONFIG_AD4000) += ad4000.o
obj-$(CONFIG_AD4030) += ad4030.o
+obj-$(CONFIG_AD4080) += ad4080.o
obj-$(CONFIG_AD4130) += ad4130.o
obj-$(CONFIG_AD4695) += ad4695.o
obj-$(CONFIG_AD4851) += ad4851.o
diff --git a/drivers/iio/adc/ad4080.c b/drivers/iio/adc/ad4080.c
new file mode 100644
index 000000000000..b51893253941
--- /dev/null
+++ b/drivers/iio/adc/ad4080.c
@@ -0,0 +1,618 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices AD4080 SPI ADC driver
+ *
+ * Copyright 2025 Analog Devices Inc.
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+#include <linux/units.h>
+
+#include <linux/iio/backend.h>
+#include <linux/iio/iio.h>
+
+#include <linux/clk.h>
+
+/* Register Definition */
+#define AD4080_REG_INTERFACE_CONFIG_A 0x00
+#define AD4080_REG_INTERFACE_CONFIG_B 0x01
+#define AD4080_REG_DEVICE_CONFIG 0x02
+#define AD4080_REG_CHIP_TYPE 0x03
+#define AD4080_REG_PRODUCT_ID_L 0x04
+#define AD4080_REG_PRODUCT_ID_H 0x05
+#define AD4080_REG_CHIP_GRADE 0x06
+#define AD4080_REG_SCRATCH_PAD 0x0A
+#define AD4080_REG_SPI_REVISION 0x0B
+#define AD4080_REG_VENDOR_L 0x0C
+#define AD4080_REG_VENDOR_H 0x0D
+#define AD4080_REG_STREAM_MODE 0x0E
+#define AD4080_REG_TRANSFER_CONFIG 0x0F
+#define AD4080_REG_INTERFACE_CONFIG_C 0x10
+#define AD4080_REG_INTERFACE_STATUS_A 0x11
+#define AD4080_REG_DEVICE_STATUS 0x14
+#define AD4080_REG_ADC_DATA_INTF_CONFIG_A 0x15
+#define AD4080_REG_ADC_DATA_INTF_CONFIG_B 0x16
+#define AD4080_REG_ADC_DATA_INTF_CONFIG_C 0x17
+#define AD4080_REG_PWR_CTRL 0x18
+#define AD4080_REG_GPIO_CONFIG_A 0x19
+#define AD4080_REG_GPIO_CONFIG_B 0x1A
+#define AD4080_REG_GPIO_CONFIG_C 0x1B
+#define AD4080_REG_GENERAL_CONFIG 0x1C
+#define AD4080_REG_FIFO_WATERMARK_LSB 0x1D
+#define AD4080_REG_FIFO_WATERMARK_MSB 0x1E
+#define AD4080_REG_EVENT_HYSTERESIS_LSB 0x1F
+#define AD4080_REG_EVENT_HYSTERESIS_MSB 0x20
+#define AD4080_REG_EVENT_DETECTION_HI_LSB 0x21
+#define AD4080_REG_EVENT_DETECTION_HI_MSB 0x22
+#define AD4080_REG_EVENT_DETECTION_LO_LSB 0x23
+#define AD4080_REG_EVENT_DETECTION_LO_MSB 0x24
+#define AD4080_REG_OFFSET_LSB 0x25
+#define AD4080_REG_OFFSET_MSB 0x26
+#define AD4080_REG_GAIN_LSB 0x27
+#define AD4080_REG_GAIN_MSB 0x28
+#define AD4080_REG_FILTER_CONFIG 0x29
+
+/* AD4080_REG_INTERFACE_CONFIG_A Bit Definition */
+#define AD4080_INTERFACE_CONFIG_A_SW_RESET_MSK (BIT(7) | BIT(0))
+#define AD4080_INTERFACE_CONFIG_A_ADDR_ASC_MSK BIT(5)
+#define AD4080_INTERFACE_CONFIG_A_SDO_ENABLE_MSK BIT(4)
+
+/* AD4080_REG_INTERFACE_CONFIG_B Bit Definition */
+#define AD4080_INTERFACE_CONFIG_B_SINGLE_INST_MSK BIT(7)
+#define AD4080_INTERFACE_CONFIG_B_SHORT_INST_MSK BIT(3)
+
+/* AD4080_REG_DEVICE_CONFIG Bit Definition */
+#define AD4080_DEVICE_CONFIG_OPERATING_MODES_MSK GENMASK(1, 0)
+
+/* AD4080_REG_TRANSFER_CONFIG Bit Definition */
+#define AD4080_TRANSFER_CONFIG_KEEP_STREAM_LENGTH_VAL_MSK BIT(2)
+
+/* AD4080_REG_INTERFACE_CONFIG_C Bit Definition */
+#define AD4080_INTERFACE_CONFIG_C_STRICT_REG_ACCESS_MSK BIT(5)
+
+/* AD4080_REG_ADC_DATA_INTF_CONFIG_A Bit Definition */
+#define AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK BIT(6)
+#define AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK BIT(4)
+#define AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK BIT(2)
+#define AD4080_ADC_DATA_INTF_CONFIG_A_DATA_INTF_MODE_MSK BIT(0)
+
+/* AD4080_REG_ADC_DATA_INTF_CONFIG_B Bit Definition */
+#define AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK GENMASK(7, 4)
+#define AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_SELF_CLK_MODE_MSK BIT(3)
+#define AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_EN_MSK BIT(0)
+
+/* AD4080_REG_ADC_DATA_INTF_CONFIG_C Bit Definition */
+#define AD4080_ADC_DATA_INTF_CONFIG_C_LVDS_VOD_MSK GENMASK(6, 4)
+
+/* AD4080_REG_PWR_CTRL Bit Definition */
+#define AD4080_PWR_CTRL_ANA_DIG_LDO_PD_MSK BIT(1)
+#define AD4080_PWR_CTRL_INTF_LDO_PD_MSK BIT(0)
+
+/* AD4080_REG_GPIO_CONFIG_A Bit Definition */
+#define AD4080_GPIO_CONFIG_A_GPO_1_EN_MSK BIT(1)
+#define AD4080_GPIO_CONFIG_A_GPO_0_EN_MSK BIT(0)
+
+/* AD4080_REG_GPIO_CONFIG_B Bit Definition */
+#define AD4080_GPIO_CONFIG_B_GPIO_1_SEL GENMASK(7, 4)
+#define AD4080_GPIO_CONFIG_B_GPIO_0_SEL GENMASK(3, 0)
+
+/* AD4080_REG_FIFO_CONFIG Bit Definition */
+#define AD4080_FIFO_CONFIG_FIFO_MODE_MSK GENMASK(1, 0)
+
+/* AD4080_REG_FILTER_CONFIG Bit Definition */
+#define AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK GENMASK(6, 3)
+#define AD4080_FILTER_CONFIG_FILTER_SEL_MSK GENMASK(1, 0)
+
+/* Miscellaneous Definitions */
+#define AD4080_SPI_READ BIT(7)
+#define AD4080_CHIP_ID GENMASK(2, 0)
+
+#define AD4080_MAX_SAMP_FREQ 40000000
+#define AD4080_MIN_SAMP_FREQ 1250000
+
+enum ad4080_filter_type {
+ FILTER_DISABLE,
+ SINC_1,
+ SINC_5,
+ SINC_5_COMP
+};
+
+static const unsigned int ad4080_scale_table[][2] = {
+ { 6000, 0},
+};
+
+static const char *const ad4080_filter_type_iio_enum[] = {
+ [FILTER_DISABLE] = "disabled",
+ [SINC_1] = "sinc1",
+ [SINC_5] = "sinc5",
+ [SINC_5_COMP] = "sinc5_plus_compensation",
+};
+
+static const int ad4080_dec_rate_iio_enum[] = {
+ 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024,
+};
+
+static const char * const ad4080_power_supplies[] = {
+ "vdd33", "vdd11", "vddldo", "iovdd", "vrefin",
+};
+
+struct ad4080_chip_info {
+ const char *name;
+ unsigned int product_id;
+ int num_scales;
+ const unsigned int (*scale_table)[2];
+ const struct iio_chan_spec *channels;
+ unsigned int num_channels;
+};
+
+struct ad4080_state {
+ struct spi_device *spi;
+ struct regmap *regmap;
+ struct clk *clk;
+ struct iio_backend *back;
+ const struct ad4080_chip_info *info;
+ /*
+ * Synchronize access to members the of driver state, and ensure
+ * atomicity of consecutive regmap operations.
+ */
+ struct mutex lock;
+ unsigned int num_lanes;
+ unsigned int dec_rate;
+ enum ad4080_filter_type filter_type;
+ bool lvds_cnv_en;
+};
+
+static const struct regmap_config ad4080_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 8,
+ .read_flag_mask = BIT(7),
+ .max_register = 0x29,
+};
+
+static int ad4080_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+ unsigned int writeval, unsigned int *readval)
+{
+ struct ad4080_state *st = iio_priv(indio_dev);
+
+ if (readval)
+ return regmap_read(st->regmap, reg, readval);
+
+ return regmap_write(st->regmap, reg, writeval);
+}
+
+static int ad4080_get_scale(struct ad4080_state *st, int *val, int *val2)
+{
+ unsigned int tmp;
+
+ tmp = (st->info->scale_table[0][0] * 1000000ULL) >>
+ st->info->channels[0].scan_type.realbits;
+ *val = tmp / 1000000;
+ *val2 = tmp % 1000000;
+
+ return IIO_VAL_INT_PLUS_NANO;
+}
+
+static unsigned int ad4080_get_dec_rate(struct iio_dev *dev,
+ const struct iio_chan_spec *chan)
+{
+ struct ad4080_state *st = iio_priv(dev);
+ int ret;
+ unsigned int data;
+
+ ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &data);
+ if (ret)
+ return ret;
+
+ return (1 << (FIELD_GET(AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK, data) + 1));
+}
+
+static int ad4080_set_dec_rate(struct iio_dev *dev,
+ const struct iio_chan_spec *chan,
+ unsigned int mode)
+{
+ struct ad4080_state *st = iio_priv(dev);
+ int ret;
+
+ if (st->filter_type >= SINC_5 && mode >= 512)
+ return -EINVAL;
+
+ guard(mutex)(&st->lock);
+ ret = regmap_update_bits(st->regmap, AD4080_REG_FILTER_CONFIG,
+ AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK,
+ FIELD_PREP(AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK,
+ (ilog2(mode) - 1)));
+ if (ret)
+ return ret;
+
+ st->dec_rate = mode;
+
+ return 0;
+}
+
+static int ad4080_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long m)
+{
+ struct ad4080_state *st = iio_priv(indio_dev);
+ int dec_rate;
+
+ switch (m) {
+ case IIO_CHAN_INFO_SCALE:
+ return ad4080_get_scale(st, val, val2);
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ if (st->filter_type == SINC_5_COMP)
+ dec_rate = st->dec_rate * 2;
+ else
+ dec_rate = st->dec_rate;
+ if (st->filter_type)
+ *val = DIV_ROUND_CLOSEST(clk_get_rate(st->clk), dec_rate);
+ else
+ *val = clk_get_rate(st->clk);
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ *val = ad4080_get_dec_rate(indio_dev, chan);
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad4080_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ return -EINVAL;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ return -EINVAL;
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ return ad4080_set_dec_rate(indio_dev, chan, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad4080_lvds_sync_write(struct ad4080_state *st)
+{
+ unsigned int timeout = 100;
+ bool sync_en;
+ int ret;
+
+ guard(mutex)(&st->lock);
+ if (st->num_lanes == 1)
+ ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+ AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
+ AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK);
+ else
+ ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+ AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
+ AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK |
+ AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
+ if (ret)
+ return ret;
+
+ ret = iio_backend_data_alignment_enable(st->back);
+ if (ret)
+ return ret;
+
+ do {
+ ret = iio_backend_sync_status_get(st->back, &sync_en);
+ if (ret)
+ return ret;
+
+ if (!sync_en)
+ dev_dbg(&st->spi->dev, "Not Locked: Running Bit Slip\n");
+
+ fsleep(500);
+ } while (--timeout && !sync_en);
+
+ if (timeout) {
+ dev_info(&st->spi->dev, "Success: Pattern correct and Locked!\n");
+ if (st->num_lanes == 1)
+ return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+ AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
+ else
+ return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+ AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
+ AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
+ } else {
+ dev_info(&st->spi->dev, "LVDS Sync Timeout.\n");
+ if (st->num_lanes == 1) {
+ ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+ AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
+ if (ret)
+ return ret;
+ } else {
+ ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+ AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
+ AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
+ if (ret)
+ return ret;
+ }
+
+ return -ETIMEDOUT;
+ }
+}
+
+static ssize_t ad4080_get_filter_type(struct iio_dev *dev,
+ const struct iio_chan_spec *chan)
+{
+ struct ad4080_state *st = iio_priv(dev);
+ unsigned int data;
+ int ret;
+
+ ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &data);
+ if (ret)
+ return ret;
+
+ return FIELD_GET(AD4080_FILTER_CONFIG_FILTER_SEL_MSK, data);
+}
+
+static int ad4080_set_filter_type(struct iio_dev *dev,
+ const struct iio_chan_spec *chan,
+ unsigned int mode)
+{
+ struct ad4080_state *st = iio_priv(dev);
+ int ret;
+
+ if (mode >= SINC_5 && st->dec_rate >= 512)
+ return -EINVAL;
+
+ guard(mutex)(&st->lock);
+ ret = iio_backend_filter_type_set(st->back, mode);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(st->regmap, AD4080_REG_FILTER_CONFIG,
+ AD4080_FILTER_CONFIG_FILTER_SEL_MSK,
+ FIELD_PREP(AD4080_FILTER_CONFIG_FILTER_SEL_MSK,
+ mode));
+ if (ret)
+ return ret;
+
+ st->filter_type = mode;
+
+ return 0;
+}
+
+static int ad4080_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ *vals = ad4080_dec_rate_iio_enum;
+ *length = ARRAY_SIZE(ad4080_dec_rate_iio_enum);
+ *type = IIO_VAL_INT;
+ return IIO_AVAIL_LIST;
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info ad4080_iio_info = {
+ .debugfs_reg_access = ad4080_reg_access,
+ .read_raw = ad4080_read_raw,
+ .write_raw = ad4080_write_raw,
+ .read_avail = ad4080_read_avail,
+};
+
+static const struct iio_enum ad4080_filter_type_enum = {
+ .items = ad4080_filter_type_iio_enum,
+ .num_items = ARRAY_SIZE(ad4080_filter_type_iio_enum),
+ .set = ad4080_set_filter_type,
+ .get = ad4080_get_filter_type,
+};
+
+static struct iio_chan_spec_ext_info ad4080_ext_info[] = {
+ IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &ad4080_filter_type_enum),
+ IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL,
+ &ad4080_filter_type_enum),
+ { }
+};
+
+static const struct iio_chan_spec ad4080_channels[] = {
+ {
+ .type = IIO_VOLTAGE,
+ .indexed = 1,
+ .channel = 0,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE),
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .info_mask_shared_by_all_available =
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .ext_info = ad4080_ext_info,
+ .scan_index = 0,
+ .scan_type = {
+ .sign = 's',
+ .realbits = 20,
+ .storagebits = 32,
+ .shift = 0,
+ },
+ }
+};
+
+static const struct ad4080_chip_info ad4080_chip_info = {
+ .name = "AD4080",
+ .product_id = AD4080_CHIP_ID,
+ .scale_table = ad4080_scale_table,
+ .num_scales = ARRAY_SIZE(ad4080_scale_table),
+ .num_channels = 1,
+ .channels = ad4080_channels,
+};
+
+static int ad4080_setup(struct iio_dev *indio_dev)
+{
+ struct ad4080_state *st = iio_priv(indio_dev);
+ unsigned int id;
+ int ret;
+
+ ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A,
+ AD4080_INTERFACE_CONFIG_A_SW_RESET_MSK);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A,
+ AD4080_INTERFACE_CONFIG_A_SDO_ENABLE_MSK);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(st->regmap, AD4080_REG_CHIP_TYPE, &id);
+ if (ret)
+ return ret;
+
+ if (id != AD4080_CHIP_ID)
+ dev_info(&st->spi->dev, "Unrecognized CHIP_ID 0x%X\n", id);
+
+ ret = regmap_set_bits(st->regmap, AD4080_REG_GPIO_CONFIG_A,
+ AD4080_GPIO_CONFIG_A_GPO_1_EN_MSK);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(st->regmap, AD4080_REG_GPIO_CONFIG_B,
+ FIELD_PREP(AD4080_GPIO_CONFIG_B_GPIO_1_SEL, 3));
+ if (ret)
+ return ret;
+
+ ret = iio_backend_num_lanes_set(st->back, st->num_lanes);
+ if (ret)
+ return ret;
+
+ if (!st->lvds_cnv_en)
+ return 0;
+
+ if (st->num_lanes) {
+ ret = regmap_update_bits(st->regmap,
+ AD4080_REG_ADC_DATA_INTF_CONFIG_B,
+ AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK,
+ FIELD_PREP(AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK,
+ 7));
+ if (ret)
+ return ret;
+ }
+
+ ret = regmap_set_bits(st->regmap,
+ AD4080_REG_ADC_DATA_INTF_CONFIG_B,
+ AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_EN_MSK);
+ if (ret)
+ return ret;
+
+ return ad4080_lvds_sync_write(st);
+}
+
+static void ad4080_properties_parse(struct ad4080_state *st)
+{
+ st->lvds_cnv_en = device_property_read_bool(&st->spi->dev,
+ "adi,lvds-cnv-enable");
+
+ st->num_lanes = 1;
+ device_property_read_u32(&st->spi->dev, "adi,num_lanes", &st->num_lanes);
+}
+
+static int ad4080_probe(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev;
+ struct device *dev = &spi->dev;
+ struct ad4080_state *st;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+ st->spi = spi;
+
+ ret = devm_regulator_bulk_get_enable(dev,
+ ARRAY_SIZE(ad4080_power_supplies),
+ ad4080_power_supplies);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to get and enable supplies\n");
+
+ st->regmap = devm_regmap_init_spi(spi, &ad4080_regmap_config);
+ if (IS_ERR(st->regmap))
+ return PTR_ERR(st->regmap);
+
+ st->info = spi_get_device_match_data(spi);
+ if (!st->info)
+ return -ENODEV;
+
+ ret = devm_mutex_init(dev, &st->lock);
+ if (ret)
+ return ret;
+
+ st->info = spi_get_device_match_data(spi);
+ if (!st->info)
+ return -ENODEV;
+
+ indio_dev->name = st->info->name;
+ indio_dev->channels = st->info->channels;
+ indio_dev->num_channels = st->info->num_channels;
+ indio_dev->info = &ad4080_iio_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ ad4080_properties_parse(st);
+
+ st->clk = devm_clk_get_enabled(&spi->dev, "cnv");
+ if (IS_ERR(st->clk))
+ return PTR_ERR(st->clk);
+
+ st->back = devm_iio_backend_get(dev, NULL);
+ if (IS_ERR(st->back))
+ return PTR_ERR(st->back);
+
+ ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev);
+ if (ret)
+ return ret;
+
+ ret = devm_iio_backend_enable(dev, st->back);
+ if (ret)
+ return ret;
+
+ ret = ad4080_setup(indio_dev);
+ if (ret)
+ return ret;
+
+ return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id ad4080_id[] = {
+ { "ad4080", (kernel_ulong_t)&ad4080_chip_info },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, ad4080_id);
+
+static const struct of_device_id ad4080_of_match[] = {
+ { .compatible = "adi,ad4080", &ad4080_chip_info },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad4080_of_match);
+
+static struct spi_driver ad4080_driver = {
+ .driver = {
+ .name = "ad4080",
+ .of_match_table = ad4080_of_match,
+ },
+ .probe = ad4080_probe,
+ .id_table = ad4080_id,
+};
+module_spi_driver(ad4080_driver);
+
+MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@analog.com");
+MODULE_DESCRIPTION("Analog Devices AD4080");
+MODULE_LICENSE("GPL");
--
2.49.0On 4/25/25 6:25 AM, Antoniu Miclaus wrote:
> Add support for AD4080 high-speed, low noise, low distortion,
> 20-bit, Easy Drive, successive approximation register (SAR)
> analog-to-digital converter (ADC).
>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
...
> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>
> +
> +#include <linux/iio/backend.h>
> +#include <linux/iio/iio.h>
> +
> +#include <linux/clk.h>
Should be grouped with the others.
> +
> +/* Register Definition */
...
> +
> +enum ad4080_filter_type {
> + FILTER_DISABLE,
> + SINC_1,
> + SINC_5,
> + SINC_5_COMP
> +};
> +
> +static const unsigned int ad4080_scale_table[][2] = {
> + { 6000, 0},
> +};
> +
> +static const char *const ad4080_filter_type_iio_enum[] = {
So far, only "sinc5" is documented in Documentation/ABI/testing/sysfs-bus-iio
so we will to add the rest there.
> + [FILTER_DISABLE] = "disabled",
IMHO, "disabled" doesn't make sense as a "type". I would call it "none" instead.
> + [SINC_1] = "sinc1",
> + [SINC_5] = "sinc5",
> + [SINC_5_COMP] = "sinc5_plus_compensation",
To follow the existing naming patterns it would make sense to call this one:
"sinc5+compensation" - Sinc5 + ???
Or even more generic like the existing sinc3+pfX options:
"sinc5+pf1" - Sinc5 + device specific Post Filter 1.
> +};
> +
> +static const int ad4080_dec_rate_iio_enum[] = {
> + 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024,
> +};
The datasheet says that 512 and 1024 only apply to sinc1 and that for
sinc5+compensation, the values are N * 2. And I would assume with the filter
disabled, the only option would be 1.
So I think we need 4 different arrays for this with the selection depending
on the filter type.
> +
> +static const char * const ad4080_power_supplies[] = {
> + "vdd33", "vdd11", "vddldo", "iovdd", "vrefin",
> +};
From the datasheet, it sounds like VDDLDO is tecnically optional. Given the
way regulators work in Linux though, I guess this is OK for simplicity.
> +
> +struct ad4080_chip_info {
> + const char *name;
> + unsigned int product_id;
> + int num_scales;
> + const unsigned int (*scale_table)[2];
> + const struct iio_chan_spec *channels;
> + unsigned int num_channels;
> +};
I guess this is preparing the driver to support more than one chip?
> +
> +struct ad4080_state {
> + struct spi_device *spi;
It looks like this is only ever used to get &spi->dev. We could drop this and
get dev from regmap instead.
> + struct regmap *regmap;
> + struct clk *clk;
> + struct iio_backend *back;
> + const struct ad4080_chip_info *info;
> + /*
> + * Synchronize access to members the of driver state, and ensure
> + * atomicity of consecutive regmap operations.
> + */
> + struct mutex lock;
> + unsigned int num_lanes;
> + unsigned int dec_rate;
> + enum ad4080_filter_type filter_type;
> + bool lvds_cnv_en;
> +};
> +
> +static const struct regmap_config ad4080_regmap_config = {
> + .reg_bits = 16,
> + .val_bits = 8,
> + .read_flag_mask = BIT(7),
> + .max_register = 0x29,
> +};
> +
> +static int ad4080_reg_access(struct iio_dev *indio_dev, unsigned int reg,
> + unsigned int writeval, unsigned int *readval)
> +{
> + struct ad4080_state *st = iio_priv(indio_dev);
> +
Missing guard(mutex)(&st->lock); ?
> + if (readval)
> + return regmap_read(st->regmap, reg, readval);
> +
> + return regmap_write(st->regmap, reg, writeval);
> +}
> +
> +static int ad4080_get_scale(struct ad4080_state *st, int *val, int *val2)
> +{
> + unsigned int tmp;
> +
> + tmp = (st->info->scale_table[0][0] * 1000000ULL) >>
> + st->info->channels[0].scan_type.realbits;
> + *val = tmp / 1000000;
> + *val2 = tmp % 1000000;
> +
> + return IIO_VAL_INT_PLUS_NANO;
Seems like this could be simplifed by using IIO_VAL_FRACTIONAL_LOG2 instead.
> +}
> +
> +static unsigned int ad4080_get_dec_rate(struct iio_dev *dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct ad4080_state *st = iio_priv(dev);
> + int ret;
> + unsigned int data;
> +
Missing guard(mutex)(&st->lock); ?
> + ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &data);
> + if (ret)
> + return ret;
> +
> + return (1 << (FIELD_GET(AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK, data) + 1));
nit: doen't need outermost ().
> +}
> +
> +static int ad4080_set_dec_rate(struct iio_dev *dev,
> + const struct iio_chan_spec *chan,
> + unsigned int mode)
> +{
> + struct ad4080_state *st = iio_priv(dev);
> + int ret;
> +
Don't we need to check for < 2 as well?
> + if (st->filter_type >= SINC_5 && mode >= 512)
> + return -EINVAL;
> +
> + guard(mutex)(&st->lock);
> + ret = regmap_update_bits(st->regmap, AD4080_REG_FILTER_CONFIG,
> + AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK,
> + FIELD_PREP(AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK,
> + (ilog2(mode) - 1)));
Otherwise ilog2(mode) - 1 could be < 0.
> + if (ret)
> + return ret;
> +
> + st->dec_rate = mode;
This saves the value the user entered, not what the hardware is actually doing.
It should be saving the power of 2 value instead.
> +
> + return 0;
> +}
> +
> +static int ad4080_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long m)
> +{
> + struct ad4080_state *st = iio_priv(indio_dev);
> + int dec_rate;
> +
> + switch (m) {
> + case IIO_CHAN_INFO_SCALE:
> + return ad4080_get_scale(st, val, val2);
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (st->filter_type == SINC_5_COMP)
> + dec_rate = st->dec_rate * 2;
> + else
> + dec_rate = st->dec_rate;
As a concequence of the above, this will return incorrect information if the
user didn't enter an exact value.
> + if (st->filter_type)
> + *val = DIV_ROUND_CLOSEST(clk_get_rate(st->clk), dec_rate);
> + else
> + *val = clk_get_rate(st->clk);
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + *val = ad4080_get_dec_rate(indio_dev, chan);
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad4080_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + return -EINVAL;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + return -EINVAL;
Can leave these 2 out and just let them fall through to the default.
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + return ad4080_set_dec_rate(indio_dev, chan, val);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad4080_lvds_sync_write(struct ad4080_state *st)
> +{
> + unsigned int timeout = 100;
> + bool sync_en;
> + int ret;
nit: some comments in this function would be helpful to readers not familiar
with the part.
> +
> + guard(mutex)(&st->lock);
> + if (st->num_lanes == 1)
> + ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> + AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> + AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK);
> + else
> + ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> + AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> + AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK |
> + AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
> + if (ret)
> + return ret;
> +
> + ret = iio_backend_data_alignment_enable(st->back);
> + if (ret)
> + return ret;
> +
> + do {
> + ret = iio_backend_sync_status_get(st->back, &sync_en);
> + if (ret)
> + return ret;
> +
> + if (!sync_en)
> + dev_dbg(&st->spi->dev, "Not Locked: Running Bit Slip\n");
> +
> + fsleep(500);
> + } while (--timeout && !sync_en);
> +
> + if (timeout) {
> + dev_info(&st->spi->dev, "Success: Pattern correct and Locked!\n");
> + if (st->num_lanes == 1)
> + return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> + AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
> + else
> + return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> + AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> + AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
> + } else {
> + dev_info(&st->spi->dev, "LVDS Sync Timeout.\n");
> + if (st->num_lanes == 1) {
> + ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> + AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
> + if (ret)
> + return ret;
> + } else {
> + ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> + AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> + AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
> + if (ret)
> + return ret;
> + }
> +
> + return -ETIMEDOUT;
> + }
> +}
> +
> +static ssize_t ad4080_get_filter_type(struct iio_dev *dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct ad4080_state *st = iio_priv(dev);
> + unsigned int data;
> + int ret;
> +
Missing guard(mutex)(&st->lock); ?
> + ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &data);
> + if (ret)
> + return ret;
> +
> + return FIELD_GET(AD4080_FILTER_CONFIG_FILTER_SEL_MSK, data);
> +}
> +
...
> +static struct iio_chan_spec_ext_info ad4080_ext_info[] = {
> + IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &ad4080_filter_type_enum),
> + IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL,
> + &ad4080_filter_type_enum),
> + { }
> +};
> +
> +static const struct iio_chan_spec ad4080_channels[] = {
Array with one element doesn't make sense. It can just be a single struct.
> + {
> + .type = IIO_VOLTAGE,
> + .indexed = 1,
> + .channel = 0,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> + .info_mask_shared_by_all_available =
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> + .ext_info = ad4080_ext_info,
> + .scan_index = 0,
> + .scan_type = {
> + .sign = 's',
> + .realbits = 20,
> + .storagebits = 32,
> + .shift = 0,
> + },
> + }
> +};
> +
> +static const struct ad4080_chip_info ad4080_chip_info = {
> + .name = "AD4080",
> + .product_id = AD4080_CHIP_ID,
> + .scale_table = ad4080_scale_table,
> + .num_scales = ARRAY_SIZE(ad4080_scale_table),
> + .num_channels = 1,
> + .channels = ad4080_channels,
> +};
> +
> +static int ad4080_setup(struct iio_dev *indio_dev)
> +{
> + struct ad4080_state *st = iio_priv(indio_dev);
> + unsigned int id;
> + int ret;
> +
> + ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A,
> + AD4080_INTERFACE_CONFIG_A_SW_RESET_MSK);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A,
> + AD4080_INTERFACE_CONFIG_A_SDO_ENABLE_MSK);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(st->regmap, AD4080_REG_CHIP_TYPE, &id);
> + if (ret)
> + return ret;
> +
> + if (id != AD4080_CHIP_ID)
> + dev_info(&st->spi->dev, "Unrecognized CHIP_ID 0x%X\n", id);
> +
> + ret = regmap_set_bits(st->regmap, AD4080_REG_GPIO_CONFIG_A,
> + AD4080_GPIO_CONFIG_A_GPO_1_EN_MSK);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD4080_REG_GPIO_CONFIG_B,
> + FIELD_PREP(AD4080_GPIO_CONFIG_B_GPIO_1_SEL, 3));
> + if (ret)
> + return ret;
> +
> + ret = iio_backend_num_lanes_set(st->back, st->num_lanes);
> + if (ret)
> + return ret;
> +
> + if (!st->lvds_cnv_en)
> + return 0;
> +
> + if (st->num_lanes) {
Since the defualt is st->num_lanes = 1, it seems like this would always be
true, so we can leave out the "if".
> + ret = regmap_update_bits(st->regmap,
> + AD4080_REG_ADC_DATA_INTF_CONFIG_B,
> + AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK,
> + FIELD_PREP(AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK,
> + 7));
> + if (ret)
> + return ret;
> + }
> +
> + ret = regmap_set_bits(st->regmap,
> + AD4080_REG_ADC_DATA_INTF_CONFIG_B,
> + AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_EN_MSK);
> + if (ret)
> + return ret;
> +
> + return ad4080_lvds_sync_write(st);
> +}
> +
> +static void ad4080_properties_parse(struct ad4080_state *st)
> +{
> + st->lvds_cnv_en = device_property_read_bool(&st->spi->dev,
> + "adi,lvds-cnv-enable");
> +
> + st->num_lanes = 1;
> + device_property_read_u32(&st->spi->dev, "adi,num_lanes", &st->num_lanes);
nit: odd that other property names use "-" but this one uses "_". Typical would
be "adi,num-lanes".
> +}
> +
> +static int ad4080_probe(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev;
> + struct device *dev = &spi->dev;
> + struct ad4080_state *st;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + st->spi = spi;
> +
> + ret = devm_regulator_bulk_get_enable(dev,
> + ARRAY_SIZE(ad4080_power_supplies),
> + ad4080_power_supplies);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to get and enable supplies\n");
> +
> + st->regmap = devm_regmap_init_spi(spi, &ad4080_regmap_config);
> + if (IS_ERR(st->regmap))
> + return PTR_ERR(st->regmap);
> +
> + st->info = spi_get_device_match_data(spi);
> + if (!st->info)
> + return -ENODEV;
> +
> + ret = devm_mutex_init(dev, &st->lock);
> + if (ret)
> + return ret;
> +
> + st->info = spi_get_device_match_data(spi);
> + if (!st->info)
> + return -ENODEV;
reduandant assignement
> +
> + indio_dev->name = st->info->name;
> + indio_dev->channels = st->info->channels;
> + indio_dev->num_channels = st->info->num_channels;
> + indio_dev->info = &ad4080_iio_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
There is not IIO_CHAN_INFO_RAW (or _PROCESSED), so INDIO_DIRECT_MODE does not
apply.
> +
> + ad4080_properties_parse(st);
> +
> + st->clk = devm_clk_get_enabled(&spi->dev, "cnv");
> + if (IS_ERR(st->clk))
> + return PTR_ERR(st->clk);
> +
> + st->back = devm_iio_backend_get(dev, NULL);
> + if (IS_ERR(st->back))
> + return PTR_ERR(st->back);
> +
> + ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev);
> + if (ret)
> + return ret;
> +
> + ret = devm_iio_backend_enable(dev, st->back);
> + if (ret)
> + return ret;
> +
> + ret = ad4080_setup(indio_dev);
> + if (ret)
> + return ret;
> +
> + return devm_iio_device_register(&spi->dev, indio_dev);
> +}
Hi Antoniu,
On Fri, 2025-04-25 at 14:25 +0300, Antoniu Miclaus wrote:
> Add support for AD4080 high-speed, low noise, low distortion,
> 20-bit, Easy Drive, successive approximation register (SAR)
> analog-to-digital converter (ADC).
>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v3:
> - name the defines to make associacion with register name.
> - drop redundant defines.
> - add trailing comma when needed.
> - drop explicit masking and use field_prep instead
> - add fsleep during sync process.
> - do not wrap where 80 chars is not exceeded.
> - use space for { }
> - drop channel definition macro
> - return dev_info on chip id mismatch.
> - flip expression to `if (!st->lvds_cnv_en)`
> - rework num_lanes property parse.
> - update the driver based on the new edits on the backend interface related
> to
> this part and the last disscussions in v2.
> drivers/iio/adc/Kconfig | 14 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ad4080.c | 618 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 633 insertions(+)
> create mode 100644 drivers/iio/adc/ad4080.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 27413516216c..17df328f5322 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -47,6 +47,20 @@ config AD4030
> To compile this driver as a module, choose M here: the module will
> be
> called ad4030.
>
> +config AD4080
> + tristate "Analog Devices AD4080 high speed ADC"
> + depends on SPI
> + select REGMAP_SPI
> + select IIO_BACKEND
> + help
> + Say yes here to build support for Analog Devices AD4080
> + high speed, low noise, low distortion, 20-bit, Easy Drive,
> + successive approximation register (SAR) analog-to-digital
> + converter (ADC). Supports iio_backended devices for AD4080.
> +
> + To compile this driver as a module, choose M here: the module will
> be
> + called ad4080.
> +
> config AD4130
> tristate "Analog Device AD4130 ADC Driver"
> depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 9f26d5eca822..e6efed5b4e7a 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
> obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
> obj-$(CONFIG_AD4000) += ad4000.o
> obj-$(CONFIG_AD4030) += ad4030.o
> +obj-$(CONFIG_AD4080) += ad4080.o
> obj-$(CONFIG_AD4130) += ad4130.o
> obj-$(CONFIG_AD4695) += ad4695.o
> obj-$(CONFIG_AD4851) += ad4851.o
> diff --git a/drivers/iio/adc/ad4080.c b/drivers/iio/adc/ad4080.c
> new file mode 100644
> index 000000000000..b51893253941
> --- /dev/null
> +++ b/drivers/iio/adc/ad4080.c
> @@ -0,0 +1,618 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices AD4080 SPI ADC driver
> + *
> + * Copyright 2025 Analog Devices Inc.
> + */
> +
>
...
> +
> +static int ad4080_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long m)
> +{
> + struct ad4080_state *st = iio_priv(indio_dev);
> + int dec_rate;
> +
> + switch (m) {
> + case IIO_CHAN_INFO_SCALE:
> + return ad4080_get_scale(st, val, val2);
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (st->filter_type == SINC_5_COMP)
> + dec_rate = st->dec_rate * 2;
> + else
> + dec_rate = st->dec_rate;
> + if (st->filter_type)
> + *val = DIV_ROUND_CLOSEST(clk_get_rate(st->clk),
> dec_rate);
> + else
> + *val = clk_get_rate(st->clk);
Kind of a nit comment (and likely personal preference) but I would rather get
the clock rate during probe. Most of the times, the clock never changes so I
rather prefer doing the above when actually needed. Or is this one those cases
were it actually changes?
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + *val = ad4080_get_dec_rate(indio_dev, chan);
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad4080_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + return -EINVAL;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + return -EINVAL;
Why not handle this in 'default'?
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + return ad4080_set_dec_rate(indio_dev, chan, val);
IIRC, you mentioned at some point that after changing the sampling frequency we
should align the interface again. Isn't this setting affecting it?
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad4080_lvds_sync_write(struct ad4080_state *st)
> +{
> + unsigned int timeout = 100;
> + bool sync_en;
> + int ret;
> +
> + guard(mutex)(&st->lock);
> + if (st->num_lanes == 1)
> + ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +
> AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK);
> + else
> + ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +
> AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK |
> +
> AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
> + if (ret)
> + return ret;
> +
> + ret = iio_backend_data_alignment_enable(st->back);
> + if (ret)
> + return ret;
So it looks like you never disable the internal alignment. Is that on purpose? I
also failed to reply to your comments in the previous version so I'll bring them
back:
"In this particular case, yes, one backend would fit for the sync process. But
taking into account that these two features are part also from the common core
of the AXI ADC, in other cases they might be used separately."
Not sure if that is true... I guess these are in the default register map but
are they always implemented for all the designs? They might just be no-ops for
some designs. Example, I do not think bitslip is always implemented. But even in
that case, the goal here is to align the interface for data sampling. So,
really, something like:
iio_backend_interface_data_align(st->back, u32 timeout);
looks fairly generic to me (given that the process is for the complete interface
at once. IOW, there's no specific timing points or stuff like that that we need
to probe independently)... So doing the polling on the backend side seems
reasonable to me (and there you can use regmap_poll APIs).
And as Jonathan puts it, this is all in kernel APIs so we can easily change it
afterwards :)
"The CNV signal is mainly used for sampling (an input pin according to the
datasheet -
conversion is initiated on the rising edge of the convert signal).
We use it only for determining the sampling frequency."
Ok, from your previous versions I got the impression this pin could also be used
for aligning the interface.
> +
> + do {
> + ret = iio_backend_sync_status_get(st->back, &sync_en);
> + if (ret)
> + return ret;
> +
> + if (!sync_en)
> + dev_dbg(&st->spi->dev, "Not Locked: Running Bit
> Slip\n");
> +
> + fsleep(500);
> + } while (--timeout && !sync_en);
> +
> + if (timeout) {
> + dev_info(&st->spi->dev, "Success: Pattern correct and
> Locked!\n");
> + if (st->num_lanes == 1)
> + return regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
> + else
> + return regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +
> AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
redundant else
> + } else {
> + dev_info(&st->spi->dev, "LVDS Sync Timeout.\n");
> + if (st->num_lanes == 1) {
> + ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
> + if (ret)
> + return ret;
> + } else {
> + ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +
> AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
> + if (ret)
> + return ret;
> + }
> +
> + return -ETIMEDOUT;
> + }
> +}
>
Hi Antoniu, kernel test robot noticed the following build warnings: [auto build test WARNING on robh/for-next] [also build test WARNING on linus/master v6.15-rc3 next-20250424] [cannot apply to jic23-iio/togreg] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Antoniu-Miclaus/iio-backend-add-support-for-filter-config/20250425-192951 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next patch link: https://lore.kernel.org/r/20250425112538.59792-12-antoniu.miclaus%40analog.com patch subject: [PATCH v3 11/11] iio: adc: ad4080: add driver support config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20250427/202504270010.w3ZsLDZR-lkp@intel.com/config) compiler: m68k-linux-gcc (GCC) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250427/202504270010.w3ZsLDZR-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202504270010.w3ZsLDZR-lkp@intel.com/ All warnings (new ones prefixed by >>, old ones prefixed by <<): WARNING: modpost: missing MODULE_DESCRIPTION() in lib/tests/slub_kunit.o >> WARNING: modpost: module ad4080 uses symbol iio_backend_filter_type_set from namespace IIO_BACKEND, but does not import it. >> WARNING: modpost: module ad4080 uses symbol iio_backend_data_alignment_enable from namespace IIO_BACKEND, but does not import it. >> WARNING: modpost: module ad4080 uses symbol iio_backend_sync_status_get from namespace IIO_BACKEND, but does not import it. >> WARNING: modpost: module ad4080 uses symbol iio_backend_num_lanes_set from namespace IIO_BACKEND, but does not import it. >> WARNING: modpost: module ad4080 uses symbol devm_iio_backend_get from namespace IIO_BACKEND, but does not import it. >> WARNING: modpost: module ad4080 uses symbol devm_iio_backend_request_buffer from namespace IIO_BACKEND, but does not import it. >> WARNING: modpost: module ad4080 uses symbol devm_iio_backend_enable from namespace IIO_BACKEND, but does not import it. -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
On Fri, 25 Apr 2025 14:25:38 +0300
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:
> Add support for AD4080 high-speed, low noise, low distortion,
> 20-bit, Easy Drive, successive approximation register (SAR)
> analog-to-digital converter (ADC).
>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Hi Antoniu
I noticed a few things on a fresh read through I'd missed on earlier versions.
Sorry about that!
In particular I think moving to explicitly just updating the bits you need
to in a few paths around sync will simplify the code quite a bit and make
sure the stuff related to sync is all that happens in there.
The use of _MSK defines for single bits is fine if you also use FIELD_PREP()
to set them to 0 / 1 as appropriate, but it is confusing if you just
use them directly to write the values as we can't see at that point in the
code that they are single bit masks. I'd drop the _MSK postfix on those.
Thanks
Jonathan
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 9f26d5eca822..e6efed5b4e7a 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
> obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
> obj-$(CONFIG_AD4000) += ad4000.o
> obj-$(CONFIG_AD4030) += ad4030.o
> +obj-$(CONFIG_AD4080) += ad4080.o
> obj-$(CONFIG_AD4130) += ad4130.o
> obj-$(CONFIG_AD4695) += ad4695.o
> obj-$(CONFIG_AD4851) += ad4851.o
> diff --git a/drivers/iio/adc/ad4080.c b/drivers/iio/adc/ad4080.c
> new file mode 100644
> index 000000000000..b51893253941
> --- /dev/null
> +++ b/drivers/iio/adc/ad4080.c
> @@ -0,0 +1,618 @@
> +/* AD4080_REG_INTERFACE_CONFIG_A Bit Definition */
> +#define AD4080_INTERFACE_CONFIG_A_SW_RESET_MSK (BIT(7) | BIT(0))
> +#define AD4080_INTERFACE_CONFIG_A_ADDR_ASC_MSK BIT(5)
> +#define AD4080_INTERFACE_CONFIG_A_SDO_ENABLE_MSK BIT(4)
> +
> +/* AD4080_REG_INTERFACE_CONFIG_B Bit Definition */
> +#define AD4080_INTERFACE_CONFIG_B_SINGLE_INST_MSK BIT(7)
> +#define AD4080_INTERFACE_CONFIG_B_SHORT_INST_MSK BIT(3)
> +
> +/* AD4080_REG_DEVICE_CONFIG Bit Definition */
> +#define AD4080_DEVICE_CONFIG_OPERATING_MODES_MSK GENMASK(1, 0)
> +
> +/* AD4080_REG_TRANSFER_CONFIG Bit Definition */
> +#define AD4080_TRANSFER_CONFIG_KEEP_STREAM_LENGTH_VAL_MSK BIT(2)
> +
> +/* AD4080_REG_INTERFACE_CONFIG_C Bit Definition */
> +#define AD4080_INTERFACE_CONFIG_C_STRICT_REG_ACCESS_MSK BIT(5)
> +
> +/* AD4080_REG_ADC_DATA_INTF_CONFIG_A Bit Definition */
> +#define AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK BIT(6)
If we are going to call these masks we should be using field prep to specify
the values. Dropping the _MSK postfix is also fine for single bit values
ant which point just | them into register values looks less odd.
> +#define AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK BIT(4)
> +#define AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK BIT(2)
> +#define AD4080_ADC_DATA_INTF_CONFIG_A_DATA_INTF_MODE_MSK BIT(0)
> +
> +/* AD4080_REG_ADC_DATA_INTF_CONFIG_B Bit Definition */
> +#define AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK GENMASK(7, 4)
> +#define AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_SELF_CLK_MODE_MSK BIT(3)
> +#define AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_EN_MSK BIT(0)
> +
> +/* AD4080_REG_ADC_DATA_INTF_CONFIG_C Bit Definition */
> +#define AD4080_ADC_DATA_INTF_CONFIG_C_LVDS_VOD_MSK GENMASK(6, 4)
> +
> +/* AD4080_REG_PWR_CTRL Bit Definition */
> +#define AD4080_PWR_CTRL_ANA_DIG_LDO_PD_MSK BIT(1)
> +#define AD4080_PWR_CTRL_INTF_LDO_PD_MSK BIT(0)
> +
> +/* AD4080_REG_GPIO_CONFIG_A Bit Definition */
> +#define AD4080_GPIO_CONFIG_A_GPO_1_EN_MSK BIT(1)
> +#define AD4080_GPIO_CONFIG_A_GPO_0_EN_MSK BIT(0)
> +
> +/* AD4080_REG_GPIO_CONFIG_B Bit Definition */
> +#define AD4080_GPIO_CONFIG_B_GPIO_1_SEL GENMASK(7, 4)
> +#define AD4080_GPIO_CONFIG_B_GPIO_0_SEL GENMASK(3, 0)
> +
> +/* AD4080_REG_FIFO_CONFIG Bit Definition */
> +#define AD4080_FIFO_CONFIG_FIFO_MODE_MSK GENMASK(1, 0)
> +
> +/* AD4080_REG_FILTER_CONFIG Bit Definition */
> +#define AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK GENMASK(6, 3)
> +#define AD4080_FILTER_CONFIG_FILTER_SEL_MSK GENMASK(1, 0)
> +
> +/* Miscellaneous Definitions */
> +#define AD4080_SPI_READ BIT(7)
> +#define AD4080_CHIP_ID GENMASK(2, 0)
> +
> +#define AD4080_MAX_SAMP_FREQ 40000000
> +#define AD4080_MIN_SAMP_FREQ 1250000
> +
> +enum ad4080_filter_type {
> + FILTER_DISABLE,
> + SINC_1,
> + SINC_5,
> + SINC_5_COMP
> +};
> +
> +static const unsigned int ad4080_scale_table[][2] = {
> + { 6000, 0},
space before }
Check for other instances of this.
Also, why does this exist? If you have other device support on the way where
this will change I don't mind having this table, but if not then just encode the
values inline.
> +};
> +
> +static const char *const ad4080_filter_type_iio_enum[] = {
> + [FILTER_DISABLE] = "disabled",
> + [SINC_1] = "sinc1",
> + [SINC_5] = "sinc5",
> + [SINC_5_COMP] = "sinc5_plus_compensation",
> +};
> +
> +static const int ad4080_dec_rate_iio_enum[] = {
> + 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024,
> +};
> +
> +static const char * const ad4080_power_supplies[] = {
> + "vdd33", "vdd11", "vddldo", "iovdd", "vrefin",
> +};
> +
> +struct ad4080_chip_info {
> + const char *name;
> + unsigned int product_id;
> + int num_scales;
> + const unsigned int (*scale_table)[2];
> + const struct iio_chan_spec *channels;
> + unsigned int num_channels;
> +};
> +
> +struct ad4080_state {
> + struct spi_device *spi;
> + struct regmap *regmap;
> + struct clk *clk;
> + struct iio_backend *back;
> + const struct ad4080_chip_info *info;
> + /*
> + * Synchronize access to members the of driver state, and ensure
> + * atomicity of consecutive regmap operations.
> + */
> + struct mutex lock;
> + unsigned int num_lanes;
> + unsigned int dec_rate;
> + enum ad4080_filter_type filter_type;
> + bool lvds_cnv_en;
> +};
> +
> +static int ad4080_lvds_sync_write(struct ad4080_state *st)
> +{
> + unsigned int timeout = 100;
> + bool sync_en;
> + int ret;
> +
> + guard(mutex)(&st->lock);
> + if (st->num_lanes == 1)
> + ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> + AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
Writing a reserved bit is unusual... (though I see it defaults to on)
Maybe you are better off just not touching that bit in the driver explicitly and instead
use.
ret = regmap_set_bits(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK);
> + AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK);
> + else
> + ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> + AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> + AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK |
> + AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
The lanes field doesn't seem to have anything directly to do with sync.
So should that perhaps be a separate regmap_set_bits() call before this function?
There is already some num_lanes related code in ad4080_setup().
> + if (ret)
> + return ret;
> +
> + ret = iio_backend_data_alignment_enable(st->back);
> + if (ret)
> + return ret;
> +
> + do {
> + ret = iio_backend_sync_status_get(st->back, &sync_en);
> + if (ret)
> + return ret;
> +
> + if (!sync_en)
> + dev_dbg(&st->spi->dev, "Not Locked: Running Bit Slip\n");
> +
> + fsleep(500);
> + } while (--timeout && !sync_en);
> +
> + if (timeout) {
> + dev_info(&st->spi->dev, "Success: Pattern correct and Locked!\n");
dev_dbg() on success tings.
> + if (st->num_lanes == 1)
> + return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> + AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
These triggered comment above on using FIELD_PREP() for things called _MSK even
if they are single bit. The _MSK naming normally implies they are multi bit making this
code look somewhat odd.
If you were to use regmap_clear_bits(st->regmap, AD4080_REG_ADC_DATA_INF_CONFIG_A,
AD4080_DATA_INF_CONFIG_A_INTF_CHK_EN_MSK);
then you don't need to make it depend on the number of lanes.
> + else
> + return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> + AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> + AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
> + } else {
> + dev_info(&st->spi->dev, "LVDS Sync Timeout.\n");
dev_err() given it's a failure is probably appropriate.
> + if (st->num_lanes == 1) {
> + ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> + AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
> + if (ret)
> + return ret;
> + } else {
> + ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> + AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> + AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
Here you can use regmap_clear_bits() as well. That will also reduce the amount of duplicate
code so there is less reason to try and share it.
> + if (ret)
> + return ret;
Not clear to me we should return this error rather than -ETIMEDOUT in these cases
(as that was the first thing to go wrong).
> + }
> +
> + return -ETIMEDOUT;
> + }
> +}
> +static const struct iio_chan_spec ad4080_channels[] = {
> + {
> + .type = IIO_VOLTAGE,
> + .indexed = 1,
> + .channel = 0,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> + .info_mask_shared_by_all_available =
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> + .ext_info = ad4080_ext_info,
> + .scan_index = 0,
> + .scan_type = {
> + .sign = 's',
> + .realbits = 20,
> + .storagebits = 32,
> + .shift = 0,
Trivial but we normally don't bother specifying shift explicitly if it is 0
as that's (kind of) the obvious default value.
> + },
> + }
> +};
© 2016 - 2026 Red Hat, Inc.