2018-10-18 11:06:24

by Stefan Popa

[permalink] [raw]
Subject: [PATCH 1/2] iio: adc: Add ad7124 support

The ad7124-4 and ad7124-8 are a family of 4 and 8 channel sigma-delta ADCs
with 24-bit precision and reference.

Three power modes are available which in turn affect the output data rate:
* Full power: 9.38 SPS to 19,200 SPS
* Mid power: 2.34 SPS to 4800 SPS
* Low power: 1.17 SPS to 2400 SPS

The ad7124-4 can be configured to have four differential inputs, while
ad7124-8 can have 8. Moreover, ad7124 also supports per channel
configuration. Each configuration consists of gain, reference source,
output data rate and bipolar/unipolar configuration.

Datasheets:
Link: http://www.analog.com/media/en/technical-documentation/data-sheets/AD7124-4.pdf
Link: http://www.analog.com/media/en/technical-documentation/data-sheets/ad7124-8.pdf

Signed-off-by: Stefan Popa <[email protected]>
---
MAINTAINERS | 7 +
drivers/iio/adc/Kconfig | 11 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad7124.c | 655 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 674 insertions(+)
create mode 100644 drivers/iio/adc/ad7124.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f642044..3a1bfcb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -839,6 +839,13 @@ S: Supported
F: drivers/iio/dac/ad5758.c
F: Documentation/devicetree/bindings/iio/dac/ad5758.txt

+ANALOG DEVICES INC AD7124 DRIVER
+M: Stefan Popa <[email protected]>
+L: [email protected]
+W: http://ez.analog.com/community/linux-device-drivers
+S: Supported
+F: drivers/iio/adc/ad7124.c
+
ANALOG DEVICES INC AD9389B DRIVER
M: Hans Verkuil <[email protected]>
L: [email protected]
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a52fea8..148a10f 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -10,6 +10,17 @@ config AD_SIGMA_DELTA
select IIO_BUFFER
select IIO_TRIGGERED_BUFFER

+config AD7124
+ tristate "Analog Devices AD7124 and similar sigma-delta ADCs driver"
+ depends on SPI_MASTER
+ select AD_SIGMA_DELTA
+ help
+ Say yes here to build support for Analog Devices AD7124-4 and AD7124-8
+ SPI analog to digital converters (ADC).
+
+ To compile this driver as a module, choose M here: the module will be
+ called ad7124.
+
config AD7266
tristate "Analog Devices AD7265/AD7266 ADC driver"
depends on SPI_MASTER
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index a6e6a0b..76168b2 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -5,6 +5,7 @@

# When adding new entries keep the list in alphabetical order
obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
+obj-$(CONFIG_AD7124) += ad7124.o
obj-$(CONFIG_AD7266) += ad7266.o
obj-$(CONFIG_AD7291) += ad7291.o
obj-$(CONFIG_AD7298) += ad7298.o
diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
new file mode 100644
index 0000000..c6d9798
--- /dev/null
+++ b/drivers/iio/adc/ad7124.c
@@ -0,0 +1,655 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD7124 SPI ADC driver
+ *
+ * Copyright 2018 Analog Devices Inc.
+ */
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/adc/ad_sigma_delta.h>
+#include <linux/iio/sysfs.h>
+
+/* AD7124 registers */
+#define AD7124_COMMS 0x00
+#define AD7124_STATUS 0x00
+#define AD7124_ADC_CONTROL 0x01
+#define AD7124_DATA 0x02
+#define AD7124_IO_CONTROL_1 0x03
+#define AD7124_IO_CONTROL_2 0x04
+#define AD7124_ID 0x05
+#define AD7124_ERROR 0x06
+#define AD7124_ERROR_EN 0x07
+#define AD7124_MCLK_COUNT 0x08
+#define AD7124_CHANNEL(x) (0x09 + (x))
+#define AD7124_CONFIG(x) (0x19 + (x))
+#define AD7124_FILTER(x) (0x21 + (x))
+#define AD7124_OFFSET(x) (0x29 + (x))
+#define AD7124_GAIN(x) (0x31 + (x))
+
+/* AD7124_STATUS */
+#define AD7124_STATUS_POR_FLAG_MSK BIT(4)
+
+/* AD7124_ADC_CONTROL */
+#define AD7124_ADC_CTRL_PWR_MSK GENMASK(7, 6)
+#define AD7124_ADC_CTRL_PWR(x) FIELD_PREP(AD7124_ADC_CTRL_PWR_MSK, x)
+#define AD7124_ADC_CTRL_MODE_MSK GENMASK(5, 2)
+#define AD7124_ADC_CTRL_MODE(x) FIELD_PREP(AD7124_ADC_CTRL_MODE_MSK, x)
+
+/* AD7124_CHANNEL_X */
+#define AD7124_CHANNEL_EN_MSK BIT(15)
+#define AD7124_CHANNEL_EN(x) FIELD_PREP(AD7124_CHANNEL_EN_MSK, x)
+#define AD7124_CHANNEL_SETUP_MSK GENMASK(14, 12)
+#define AD7124_CHANNEL_SETUP(x) FIELD_PREP(AD7124_CHANNEL_SETUP_MSK, x)
+#define AD7124_CHANNEL_AINP_MSK GENMASK(9, 5)
+#define AD7124_CHANNEL_AINP(x) FIELD_PREP(AD7124_CHANNEL_AINP_MSK, x)
+#define AD7124_CHANNEL_AINM_MSK GENMASK(4, 0)
+#define AD7124_CHANNEL_AINM(x) FIELD_PREP(AD7124_CHANNEL_AINM_MSK, x)
+
+/* AD7124_CONFIG_X */
+#define AD7124_CONFIG_BIPOLAR_MSK BIT(11)
+#define AD7124_CONFIG_BIPOLAR(x) FIELD_PREP(AD7124_CONFIG_BIPOLAR_MSK, x)
+#define AD7124_CONFIG_REF_SEL_MSK GENMASK(4, 3)
+#define AD7124_CONFIG_REF_SEL(x) FIELD_PREP(AD7124_CONFIG_REF_SEL_MSK, x)
+#define AD7124_CONFIG_PGA_MSK GENMASK(2, 0)
+#define AD7124_CONFIG_PGA(x) FIELD_PREP(AD7124_CONFIG_PGA_MSK, x)
+
+/* AD7124_FILTER_X */
+#define AD7124_FILTER_FS_MSK GENMASK(10, 0)
+#define AD7124_FILTER_FS(x) FIELD_PREP(AD7124_FILTER_FS_MSK, x)
+
+enum ad7124_ids {
+ ID_AD7124_4,
+ ID_AD7124_8,
+};
+
+enum ad7124_ref_sel {
+ AD7124_REFIN1,
+ AD7124_REFIN2,
+ AD7124_INT_REF,
+ AD7124_AVDD_REF,
+};
+
+enum ad7124_power_mode {
+ AD7124_LOW_POWER,
+ AD7124_MID_POWER,
+ AD7124_FULL_POWER,
+};
+
+static const unsigned int ad7124_gain[8] = {
+ 1, 2, 4, 8, 16, 32, 64, 128
+};
+
+static const int ad7124_master_clk_freq_hz[3] = {
+ [AD7124_LOW_POWER] = 76800,
+ [AD7124_MID_POWER] = 153600,
+ [AD7124_FULL_POWER] = 614400,
+};
+
+static const char * const ad7124_ref_names[] = {
+ [AD7124_REFIN1] = "refin1",
+ [AD7124_REFIN2] = "refin2",
+ [AD7124_INT_REF] = "int",
+ [AD7124_AVDD_REF] = "avdd",
+};
+
+struct ad7124_chip_info {
+ unsigned int num_inputs;
+};
+
+struct ad7124_channel_config {
+ enum ad7124_ref_sel refsel;
+ bool bipolar;
+ unsigned int ain;
+ unsigned int vref_mv;
+ unsigned int pga_bits;
+ unsigned int odr;
+};
+
+struct ad7124_state {
+ const struct ad7124_chip_info *chip_info;
+ struct ad_sigma_delta sd;
+ struct ad7124_channel_config channel_config[4];
+ struct regulator *vref[4];
+ struct clk *mclk;
+ unsigned int adc_control;
+ unsigned int num_channels;
+};
+
+static const struct iio_chan_spec ad7124_channel_template = {
+ .type = IIO_VOLTAGE,
+ .indexed = 1,
+ .differential = 1,
+ .channel = 0,
+ .address = 0,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_OFFSET) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ .scan_index = 0,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 24,
+ .storagebits = 32,
+ .shift = 0,
+ },
+};
+
+static struct ad7124_chip_info ad7124_chip_info_tbl[] = {
+ [ID_AD7124_4] = {
+ .num_inputs = 8,
+ },
+ [ID_AD7124_8] = {
+ .num_inputs = 16,
+ },
+};
+
+static int ad7124_find_closest_match(const int *array,
+ unsigned int size, int val)
+{
+ int i;
+
+ for (i = 0; i < size; i++) {
+ if (val <= array[i])
+ return i;
+ }
+
+ return size - 1;
+}
+
+static int ad7124_spi_write_mask(struct ad7124_state *st,
+ unsigned int addr,
+ unsigned long mask,
+ unsigned int val,
+ unsigned int bytes)
+{
+ unsigned int readval;
+ int ret;
+
+ ret = ad_sd_read_reg(&st->sd, addr, bytes, &readval);
+ if (ret < 0)
+ return ret;
+
+ readval &= ~mask;
+ readval |= val;
+
+ return ad_sd_write_reg(&st->sd, addr, bytes, readval);
+}
+
+static int ad7124_set_mode(struct ad_sigma_delta *sd,
+ enum ad_sigma_delta_mode mode)
+{
+ struct ad7124_state *st = container_of(sd, struct ad7124_state, sd);
+
+ st->adc_control &= ~AD7124_ADC_CTRL_MODE_MSK;
+ st->adc_control |= AD7124_ADC_CTRL_MODE(mode);
+
+ return ad_sd_write_reg(&st->sd, AD7124_ADC_CONTROL, 2, st->adc_control);
+}
+
+static int ad7124_set_channel(struct ad_sigma_delta *sd, unsigned int channel)
+{
+ struct ad7124_state *st = container_of(sd, struct ad7124_state, sd);
+ unsigned int val;
+
+ val = st->channel_config[channel].ain | AD7124_CHANNEL_EN(1) |
+ AD7124_CHANNEL_SETUP(channel);
+
+ return ad_sd_write_reg(&st->sd, AD7124_CHANNEL(channel), 2, val);
+}
+
+static const struct ad_sigma_delta_info ad7124_sigma_delta_info = {
+ .set_channel = ad7124_set_channel,
+ .set_mode = ad7124_set_mode,
+ .has_registers = true,
+ .addr_shift = 0,
+ .read_mask = BIT(6),
+ .data_reg = AD7124_DATA,
+};
+
+static int ad7124_set_channel_odr(struct ad7124_state *st,
+ unsigned int channel,
+ unsigned int odr)
+{
+ unsigned int fclk, odr_sel_bits;
+ int ret;
+
+ fclk = clk_get_rate(st->mclk);
+ /*
+ * FS[10:0] = fCLK / (fADC x 32) where:
+ * fADC is the output data rate
+ * fCLK is the master clock frequency
+ * FS[10:0] are the bits in the filter register
+ * FS[10:0] can have a value from 1 to 2047
+ */
+ odr_sel_bits = DIV_ROUND_CLOSEST(fclk, odr * 32);
+ if (odr_sel_bits < 1)
+ odr_sel_bits = 1;
+ else if (odr_sel_bits > 2047)
+ odr_sel_bits = 2047;
+
+ ret = ad7124_spi_write_mask(st, AD7124_FILTER(channel),
+ AD7124_FILTER_FS_MSK,
+ AD7124_FILTER_FS(odr_sel_bits), 3);
+ if (ret < 0)
+ return ret;
+ /* fADC = fCLK / (FS[10:0] x 32) */
+ st->channel_config[channel].odr =
+ DIV_ROUND_CLOSEST(fclk, odr_sel_bits * 32);
+
+ return 0;
+}
+
+static int ad7124_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long info)
+{
+ struct ad7124_state *st = iio_priv(indio_dev);
+ int idx, ret;
+
+ switch (info) {
+ case IIO_CHAN_INFO_RAW:
+ ret = ad_sigma_delta_single_conversion(indio_dev, chan, val);
+ if (ret < 0)
+ return ret;
+
+ /* After the conversion is performed, disable the channel */
+ ret = ad_sd_write_reg(&st->sd,
+ AD7124_CHANNEL(chan->address), 2,
+ st->channel_config[chan->address].ain |
+ AD7124_CHANNEL_EN(0));
+ if (ret < 0)
+ return ret;
+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ idx = st->channel_config[chan->address].pga_bits;
+ *val = st->channel_config[chan->address].vref_mv /
+ ad7124_gain[idx];
+ if (st->channel_config[chan->address].bipolar)
+ *val2 = chan->scan_type.realbits - 1;
+ else
+ *val2 = chan->scan_type.realbits;
+
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_CHAN_INFO_OFFSET:
+ if (st->channel_config[chan->address].bipolar) {
+ /* Code = 2^(n − 1) × ((Ain × Gain / Vref) + 1) */
+ idx = st->channel_config[chan->address].pga_bits;
+ *val = -(st->channel_config[chan->address].vref_mv /
+ ad7124_gain[idx]);
+ } else {
+ *val = 0;
+ }
+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ *val = st->channel_config[chan->address].odr;
+
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad7124_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long info)
+{
+ struct ad7124_state *st = iio_priv(indio_dev);
+
+ switch (info) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ return ad7124_set_channel_odr(st, chan->address, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info ad7124_info = {
+ .read_raw = ad7124_read_raw,
+ .write_raw = ad7124_write_raw,
+};
+
+static int ad7124_soft_reset(struct ad7124_state *st)
+{
+ unsigned int readval, timeout;
+ int ret;
+
+ ret = ad_sd_reset(&st->sd, 64);
+ if (ret < 0)
+ return ret;
+
+ timeout = 100;
+ do {
+ ret = ad_sd_read_reg(&st->sd, AD7124_STATUS, 1, &readval);
+ if (ret < 0)
+ return ret;
+
+ if (!(readval & AD7124_STATUS_POR_FLAG_MSK))
+ return 0;
+
+ /* The AD7124 requires typically 2ms to power up and settle */
+ usleep_range(100, 2000);
+ } while (--timeout);
+
+ dev_err(&st->sd.spi->dev, "Soft reset failed\n");
+
+ return -EIO;
+}
+
+static int ad7124_init_channel_vref(struct ad7124_state *st,
+ unsigned int channel_number)
+{
+ unsigned int refsel = st->channel_config[channel_number].refsel;
+
+ switch (refsel) {
+ case AD7124_REFIN1:
+ case AD7124_REFIN2:
+ case AD7124_AVDD_REF:
+ if (IS_ERR(st->vref[refsel])) {
+ dev_err(&st->sd.spi->dev,
+ "Error, trying to use external voltage reference without a %s regulator.",
+ ad7124_ref_names[refsel]);
+ return PTR_ERR(st->vref[refsel]);
+ }
+ st->channel_config[channel_number].vref_mv =
+ regulator_get_voltage(st->vref[refsel]);
+ /* Conversion from uV to mV */
+ st->channel_config[channel_number].vref_mv /= 1000;
+ break;
+ case AD7124_INT_REF:
+ st->channel_config[channel_number].vref_mv = 2500;
+ break;
+ default:
+ dev_err(&st->sd.spi->dev, "Invalid reference %d\n", refsel);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
+ struct device_node *np)
+{
+ struct ad7124_state *st = iio_priv(indio_dev);
+ struct device_node *chan_node, *child;
+ struct iio_chan_spec *chan;
+ unsigned int ain[2], channel = 0, tmp;
+ int ret, res;
+
+ chan_node = of_get_child_by_name(np, "adi,channels");
+ if (!chan_node) {
+ dev_err(indio_dev->dev.parent,
+ "failed to find channels node\n");
+ return -ENODEV;
+ }
+
+ st->num_channels = of_get_available_child_count(chan_node);
+ if (!st->num_channels) {
+ dev_err(indio_dev->dev.parent, "no channel children\n");
+ return -ENODEV;
+ }
+
+ chan = devm_kcalloc(indio_dev->dev.parent, st->num_channels,
+ sizeof(*chan), GFP_KERNEL);
+ if (!chan)
+ return -ENOMEM;
+
+ indio_dev->channels = chan;
+ indio_dev->num_channels = st->num_channels;
+
+ for_each_available_child_of_node(chan_node, child) {
+ ret = of_property_read_u32_array(child, "reg", ain, 2);
+ if (ret)
+ goto err;
+
+ ret = of_property_read_u32(child,
+ "adi,channel-number", &channel);
+ if (ret)
+ goto err;
+
+ if (ain[0] >= st->chip_info->num_inputs ||
+ ain[1] >= st->chip_info->num_inputs) {
+ dev_err(indio_dev->dev.parent,
+ "Input pin number out of range.\n");
+ ret = -EINVAL;
+ goto err;
+ }
+ st->channel_config[channel].ain = AD7124_CHANNEL_AINP(ain[0]) |
+ AD7124_CHANNEL_AINM(ain[1]);
+ st->channel_config[channel].bipolar =
+ of_property_read_bool(child, "adi,bipolar");
+
+ ret = of_property_read_u32(child, "adi,reference-select", &tmp);
+ if (ret)
+ st->channel_config[channel].refsel = AD7124_INT_REF;
+ else
+ st->channel_config[channel].refsel = tmp;
+
+ ret = of_property_read_u32(child, "adi,gain", &tmp);
+ if (ret) {
+ st->channel_config[channel].pga_bits = 0;
+ } else {
+ res = ad7124_find_closest_match(ad7124_gain,
+ ARRAY_SIZE(ad7124_gain), tmp);
+ st->channel_config[channel].pga_bits = res;
+ }
+
+ ret = of_property_read_u32(child, "adi,odr-hz", &tmp);
+ if (ret)
+ /*
+ * 9 SPS is the minimum output data rate supported
+ * regardless of the selected power mode.
+ */
+ st->channel_config[channel].odr = 9;
+ else
+ st->channel_config[channel].odr = tmp;
+
+ *chan = ad7124_channel_template;
+ chan->address = channel;
+ chan->scan_index = channel;
+ chan->channel = ain[0];
+ chan->channel2 = ain[1];
+
+ chan++;
+ }
+ of_node_put(chan_node);
+
+ return 0;
+err:
+ of_node_put(chan_node);
+ of_node_put(child);
+
+ return ret;
+}
+
+static int ad7124_setup(struct ad7124_state *st)
+{
+ unsigned int val, fclk, power_mode;
+ int i, ret;
+
+ fclk = clk_get_rate(st->mclk);
+ if (!fclk)
+ return -EINVAL;
+
+ /* The power mode changes the master clock frequency */
+ power_mode = ad7124_find_closest_match(ad7124_master_clk_freq_hz,
+ ARRAY_SIZE(ad7124_master_clk_freq_hz),
+ fclk);
+ if (fclk != ad7124_master_clk_freq_hz[power_mode]) {
+ ret = clk_set_rate(st->mclk, fclk);
+ if (ret)
+ return ret;
+ }
+
+ /* Set the power mode */
+ st->adc_control &= ~AD7124_ADC_CTRL_PWR_MSK;
+ st->adc_control |= AD7124_ADC_CTRL_PWR(power_mode);
+ ret = ad_sd_write_reg(&st->sd, AD7124_ADC_CONTROL, 2, st->adc_control);
+ if (ret < 0)
+ return ret;
+
+ for (i = 0; i < st->num_channels; i++) {
+ val = st->channel_config[i].ain | AD7124_CHANNEL_SETUP(i);
+ ret = ad_sd_write_reg(&st->sd, AD7124_CHANNEL(i), 2, val);
+ if (ret < 0)
+ return ret;
+
+ ret = ad7124_init_channel_vref(st, i);
+ if (ret < 0)
+ return ret;
+
+ val = AD7124_CONFIG_BIPOLAR(st->channel_config[i].bipolar) |
+ AD7124_CONFIG_REF_SEL(st->channel_config[i].refsel) |
+ AD7124_CONFIG_PGA(st->channel_config[i].pga_bits);
+ ret = ad_sd_write_reg(&st->sd, AD7124_CONFIG(i), 2, val);
+ if (ret < 0)
+ return ret;
+
+ ret = ad7124_set_channel_odr(st, i, st->channel_config[i].odr);
+ if (ret < 0)
+ return ret;
+ }
+
+ return ret;
+}
+
+static int ad7124_probe(struct spi_device *spi)
+{
+ const struct spi_device_id *id;
+ struct ad7124_state *st;
+ struct iio_dev *indio_dev;
+ int i, ret;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+
+ id = spi_get_device_id(spi);
+ st->chip_info = &ad7124_chip_info_tbl[id->driver_data];
+
+ ad_sd_init(&st->sd, indio_dev, spi, &ad7124_sigma_delta_info);
+
+ spi_set_drvdata(spi, indio_dev);
+
+ indio_dev->dev.parent = &spi->dev;
+ indio_dev->name = spi_get_device_id(spi)->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &ad7124_info;
+
+ ret = ad7124_of_parse_channel_config(indio_dev, spi->dev.of_node);
+ if (ret < 0)
+ return ret;
+
+ for (i = 0; i < ARRAY_SIZE(st->vref); i++) {
+ if (i != AD7124_INT_REF)
+ st->vref[i] = devm_regulator_get_optional(&spi->dev,
+ ad7124_ref_names[i]);
+ if (IS_ERR_OR_NULL(st->vref[i]))
+ continue;
+
+ ret = regulator_enable(st->vref[i]);
+ if (ret)
+ return ret;
+ }
+
+ st->mclk = devm_clk_get(&spi->dev, "mclk");
+ if (IS_ERR(st->mclk)) {
+ ret = PTR_ERR(st->mclk);
+ goto error_regulator_disable;
+ }
+
+ ret = clk_prepare_enable(st->mclk);
+ if (ret < 0)
+ goto error_regulator_disable;
+
+ ret = ad7124_soft_reset(st);
+ if (ret < 0)
+ goto error_clk_disable_unprepare;
+
+ ret = ad7124_setup(st);
+ if (ret < 0)
+ goto error_clk_disable_unprepare;
+
+ ret = ad_sd_setup_buffer_and_trigger(indio_dev);
+ if (ret < 0)
+ goto error_clk_disable_unprepare;
+
+ ret = iio_device_register(indio_dev);
+ if (ret < 0) {
+ dev_err(&spi->dev, "Failed to register iio device\n");
+ goto error_remove_trigger;
+ }
+
+ return 0;
+
+error_remove_trigger:
+ ad_sd_cleanup_buffer_and_trigger(indio_dev);
+error_clk_disable_unprepare:
+ clk_disable_unprepare(st->mclk);
+error_regulator_disable:
+ for (i = ARRAY_SIZE(st->vref) - 1; i >= 0; i--) {
+ if (!IS_ERR_OR_NULL(st->vref[i]))
+ regulator_disable(st->vref[i]);
+ }
+
+ return ret;
+}
+
+static int ad7124_remove(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev = spi_get_drvdata(spi);
+ struct ad7124_state *st = iio_priv(indio_dev);
+ int i;
+
+ iio_device_unregister(indio_dev);
+ ad_sd_cleanup_buffer_and_trigger(indio_dev);
+ clk_disable_unprepare(st->mclk);
+
+ for (i = ARRAY_SIZE(st->vref) - 1; i >= 0; i--) {
+ if (!IS_ERR_OR_NULL(st->vref[i]))
+ regulator_disable(st->vref[i]);
+ }
+
+ return 0;
+}
+
+static const struct spi_device_id ad7124_id_table[] = {
+ { "ad7124-4", ID_AD7124_4 },
+ { "ad7124-8", ID_AD7124_8 },
+ {}
+};
+MODULE_DEVICE_TABLE(spi, ad7124_id_table);
+
+static const struct of_device_id ad7124_of_match[] = {
+ { .compatible = "adi,ad7124-4" },
+ { .compatible = "adi,ad7124-8" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, ad7124_of_match);
+
+static struct spi_driver ad71124_driver = {
+ .driver = {
+ .name = "ad7124",
+ .of_match_table = ad7124_of_match,
+ },
+ .probe = ad7124_probe,
+ .remove = ad7124_remove,
+ .id_table = ad7124_id_table,
+};
+module_spi_driver(ad71124_driver);
+
+MODULE_AUTHOR("Stefan Popa <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD7124 SPI driver");
+MODULE_LICENSE("GPL");
--
2.7.4



2018-10-19 04:46:30

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/2] iio: adc: Add ad7124 support

Hi Stefan,

I love your patch! Yet something to improve:

[auto build test ERROR on iio/togreg]
[also build test ERROR on v4.19-rc8 next-20181018]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Stefan-Popa/iio-adc-Add-ad7124-support/20181019-051737
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm

All error/warnings (new ones prefixed by >>):

>> drivers/iio//adc/ad7124.c:215:3: error: 'const struct ad_sigma_delta_info' has no member named 'data_reg'
.data_reg = AD7124_DATA,
^~~~~~~~
>> drivers/iio//adc/ad7124.c:25:23: warning: excess elements in struct initializer
#define AD7124_DATA 0x02
^
>> drivers/iio//adc/ad7124.c:215:14: note: in expansion of macro 'AD7124_DATA'
.data_reg = AD7124_DATA,
^~~~~~~~~~~
drivers/iio//adc/ad7124.c:25:23: note: (near initialization for 'ad7124_sigma_delta_info')
#define AD7124_DATA 0x02
^
>> drivers/iio//adc/ad7124.c:215:14: note: in expansion of macro 'AD7124_DATA'
.data_reg = AD7124_DATA,
^~~~~~~~~~~

vim +215 drivers/iio//adc/ad7124.c

20
21 /* AD7124 registers */
22 #define AD7124_COMMS 0x00
23 #define AD7124_STATUS 0x00
24 #define AD7124_ADC_CONTROL 0x01
> 25 #define AD7124_DATA 0x02
26 #define AD7124_IO_CONTROL_1 0x03
27 #define AD7124_IO_CONTROL_2 0x04
28 #define AD7124_ID 0x05
29 #define AD7124_ERROR 0x06
30 #define AD7124_ERROR_EN 0x07
31 #define AD7124_MCLK_COUNT 0x08
32 #define AD7124_CHANNEL(x) (0x09 + (x))
33 #define AD7124_CONFIG(x) (0x19 + (x))
34 #define AD7124_FILTER(x) (0x21 + (x))
35 #define AD7124_OFFSET(x) (0x29 + (x))
36 #define AD7124_GAIN(x) (0x31 + (x))
37
38 /* AD7124_STATUS */
39 #define AD7124_STATUS_POR_FLAG_MSK BIT(4)
40
41 /* AD7124_ADC_CONTROL */
42 #define AD7124_ADC_CTRL_PWR_MSK GENMASK(7, 6)
43 #define AD7124_ADC_CTRL_PWR(x) FIELD_PREP(AD7124_ADC_CTRL_PWR_MSK, x)
44 #define AD7124_ADC_CTRL_MODE_MSK GENMASK(5, 2)
45 #define AD7124_ADC_CTRL_MODE(x) FIELD_PREP(AD7124_ADC_CTRL_MODE_MSK, x)
46
47 /* AD7124_CHANNEL_X */
48 #define AD7124_CHANNEL_EN_MSK BIT(15)
49 #define AD7124_CHANNEL_EN(x) FIELD_PREP(AD7124_CHANNEL_EN_MSK, x)
50 #define AD7124_CHANNEL_SETUP_MSK GENMASK(14, 12)
51 #define AD7124_CHANNEL_SETUP(x) FIELD_PREP(AD7124_CHANNEL_SETUP_MSK, x)
52 #define AD7124_CHANNEL_AINP_MSK GENMASK(9, 5)
53 #define AD7124_CHANNEL_AINP(x) FIELD_PREP(AD7124_CHANNEL_AINP_MSK, x)
54 #define AD7124_CHANNEL_AINM_MSK GENMASK(4, 0)
55 #define AD7124_CHANNEL_AINM(x) FIELD_PREP(AD7124_CHANNEL_AINM_MSK, x)
56
57 /* AD7124_CONFIG_X */
58 #define AD7124_CONFIG_BIPOLAR_MSK BIT(11)
59 #define AD7124_CONFIG_BIPOLAR(x) FIELD_PREP(AD7124_CONFIG_BIPOLAR_MSK, x)
60 #define AD7124_CONFIG_REF_SEL_MSK GENMASK(4, 3)
61 #define AD7124_CONFIG_REF_SEL(x) FIELD_PREP(AD7124_CONFIG_REF_SEL_MSK, x)
62 #define AD7124_CONFIG_PGA_MSK GENMASK(2, 0)
63 #define AD7124_CONFIG_PGA(x) FIELD_PREP(AD7124_CONFIG_PGA_MSK, x)
64
65 /* AD7124_FILTER_X */
66 #define AD7124_FILTER_FS_MSK GENMASK(10, 0)
67 #define AD7124_FILTER_FS(x) FIELD_PREP(AD7124_FILTER_FS_MSK, x)
68
69 enum ad7124_ids {
70 ID_AD7124_4,
71 ID_AD7124_8,
72 };
73
74 enum ad7124_ref_sel {
75 AD7124_REFIN1,
76 AD7124_REFIN2,
77 AD7124_INT_REF,
78 AD7124_AVDD_REF,
79 };
80
81 enum ad7124_power_mode {
82 AD7124_LOW_POWER,
83 AD7124_MID_POWER,
84 AD7124_FULL_POWER,
85 };
86
87 static const unsigned int ad7124_gain[8] = {
88 1, 2, 4, 8, 16, 32, 64, 128
89 };
90
91 static const int ad7124_master_clk_freq_hz[3] = {
92 [AD7124_LOW_POWER] = 76800,
93 [AD7124_MID_POWER] = 153600,
94 [AD7124_FULL_POWER] = 614400,
95 };
96
97 static const char * const ad7124_ref_names[] = {
98 [AD7124_REFIN1] = "refin1",
99 [AD7124_REFIN2] = "refin2",
100 [AD7124_INT_REF] = "int",
101 [AD7124_AVDD_REF] = "avdd",
102 };
103
104 struct ad7124_chip_info {
105 unsigned int num_inputs;
106 };
107
108 struct ad7124_channel_config {
109 enum ad7124_ref_sel refsel;
110 bool bipolar;
111 unsigned int ain;
112 unsigned int vref_mv;
113 unsigned int pga_bits;
114 unsigned int odr;
115 };
116
117 struct ad7124_state {
118 const struct ad7124_chip_info *chip_info;
119 struct ad_sigma_delta sd;
120 struct ad7124_channel_config channel_config[4];
121 struct regulator *vref[4];
122 struct clk *mclk;
123 unsigned int adc_control;
124 unsigned int num_channels;
125 };
126
127 static const struct iio_chan_spec ad7124_channel_template = {
128 .type = IIO_VOLTAGE,
129 .indexed = 1,
130 .differential = 1,
131 .channel = 0,
132 .address = 0,
133 .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
134 BIT(IIO_CHAN_INFO_SCALE) |
135 BIT(IIO_CHAN_INFO_OFFSET) |
136 BIT(IIO_CHAN_INFO_SAMP_FREQ),
137 .scan_index = 0,
138 .scan_type = {
139 .sign = 'u',
140 .realbits = 24,
141 .storagebits = 32,
142 .shift = 0,
143 },
144 };
145
146 static struct ad7124_chip_info ad7124_chip_info_tbl[] = {
147 [ID_AD7124_4] = {
148 .num_inputs = 8,
149 },
150 [ID_AD7124_8] = {
151 .num_inputs = 16,
152 },
153 };
154
155 static int ad7124_find_closest_match(const int *array,
156 unsigned int size, int val)
157 {
158 int i;
159
160 for (i = 0; i < size; i++) {
161 if (val <= array[i])
162 return i;
163 }
164
165 return size - 1;
166 }
167
168 static int ad7124_spi_write_mask(struct ad7124_state *st,
169 unsigned int addr,
170 unsigned long mask,
171 unsigned int val,
172 unsigned int bytes)
173 {
174 unsigned int readval;
175 int ret;
176
177 ret = ad_sd_read_reg(&st->sd, addr, bytes, &readval);
178 if (ret < 0)
179 return ret;
180
181 readval &= ~mask;
182 readval |= val;
183
184 return ad_sd_write_reg(&st->sd, addr, bytes, readval);
185 }
186
187 static int ad7124_set_mode(struct ad_sigma_delta *sd,
188 enum ad_sigma_delta_mode mode)
189 {
190 struct ad7124_state *st = container_of(sd, struct ad7124_state, sd);
191
192 st->adc_control &= ~AD7124_ADC_CTRL_MODE_MSK;
193 st->adc_control |= AD7124_ADC_CTRL_MODE(mode);
194
195 return ad_sd_write_reg(&st->sd, AD7124_ADC_CONTROL, 2, st->adc_control);
196 }
197
198 static int ad7124_set_channel(struct ad_sigma_delta *sd, unsigned int channel)
199 {
200 struct ad7124_state *st = container_of(sd, struct ad7124_state, sd);
201 unsigned int val;
202
203 val = st->channel_config[channel].ain | AD7124_CHANNEL_EN(1) |
204 AD7124_CHANNEL_SETUP(channel);
205
206 return ad_sd_write_reg(&st->sd, AD7124_CHANNEL(channel), 2, val);
207 }
208
209 static const struct ad_sigma_delta_info ad7124_sigma_delta_info = {
210 .set_channel = ad7124_set_channel,
211 .set_mode = ad7124_set_mode,
212 .has_registers = true,
213 .addr_shift = 0,
214 .read_mask = BIT(6),
> 215 .data_reg = AD7124_DATA,
216 };
217

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (8.13 kB)
.config.gz (65.62 kB)
Download all attachments

2018-10-21 14:41:13

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/2] iio: adc: Add ad7124 support

On Thu, 18 Oct 2018 14:04:53 +0300
Stefan Popa <[email protected]> wrote:

> The ad7124-4 and ad7124-8 are a family of 4 and 8 channel sigma-delta ADCs
> with 24-bit precision and reference.
>
> Three power modes are available which in turn affect the output data rate:
> * Full power: 9.38 SPS to 19,200 SPS
> * Mid power: 2.34 SPS to 4800 SPS
> * Low power: 1.17 SPS to 2400 SPS
>
> The ad7124-4 can be configured to have four differential inputs, while
> ad7124-8 can have 8. Moreover, ad7124 also supports per channel
> configuration. Each configuration consists of gain, reference source,
> output data rate and bipolar/unipolar configuration.
>
> Datasheets:
> Link: http://www.analog.com/media/en/technical-documentation/data-sheets/AD7124-4.pdf
> Link: http://www.analog.com/media/en/technical-documentation/data-sheets/ad7124-8.pdf
>
> Signed-off-by: Stefan Popa <[email protected]>

Hi Stefan,

Just a couple of small things from me. See inline.

Thanks,

Jonathan
> ---
> MAINTAINERS | 7 +
> drivers/iio/adc/Kconfig | 11 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ad7124.c | 655 +++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 674 insertions(+)
> create mode 100644 drivers/iio/adc/ad7124.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f642044..3a1bfcb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -839,6 +839,13 @@ S: Supported
> F: drivers/iio/dac/ad5758.c
> F: Documentation/devicetree/bindings/iio/dac/ad5758.txt
>
> +ANALOG DEVICES INC AD7124 DRIVER
> +M: Stefan Popa <[email protected]>
> +L: [email protected]
> +W: http://ez.analog.com/community/linux-device-drivers
> +S: Supported
> +F: drivers/iio/adc/ad7124.c
> +
> ANALOG DEVICES INC AD9389B DRIVER
> M: Hans Verkuil <[email protected]>
> L: [email protected]
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index a52fea8..148a10f 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -10,6 +10,17 @@ config AD_SIGMA_DELTA
> select IIO_BUFFER
> select IIO_TRIGGERED_BUFFER
>
> +config AD7124
> + tristate "Analog Devices AD7124 and similar sigma-delta ADCs driver"
> + depends on SPI_MASTER
> + select AD_SIGMA_DELTA
> + help
> + Say yes here to build support for Analog Devices AD7124-4 and AD7124-8
> + SPI analog to digital converters (ADC).
> +
> + To compile this driver as a module, choose M here: the module will be
> + called ad7124.
> +
> config AD7266
> tristate "Analog Devices AD7265/AD7266 ADC driver"
> depends on SPI_MASTER
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index a6e6a0b..76168b2 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -5,6 +5,7 @@
>
> # When adding new entries keep the list in alphabetical order
> obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
> +obj-$(CONFIG_AD7124) += ad7124.o
> obj-$(CONFIG_AD7266) += ad7266.o
> obj-$(CONFIG_AD7291) += ad7291.o
> obj-$(CONFIG_AD7298) += ad7298.o
> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> new file mode 100644
> index 0000000..c6d9798
> --- /dev/null
> +++ b/drivers/iio/adc/ad7124.c
> @@ -0,0 +1,655 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * AD7124 SPI ADC driver
> + *
> + * Copyright 2018 Analog Devices Inc.
> + */
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/adc/ad_sigma_delta.h>
> +#include <linux/iio/sysfs.h>
> +
> +/* AD7124 registers */
> +#define AD7124_COMMS 0x00
> +#define AD7124_STATUS 0x00
> +#define AD7124_ADC_CONTROL 0x01
> +#define AD7124_DATA 0x02
> +#define AD7124_IO_CONTROL_1 0x03
> +#define AD7124_IO_CONTROL_2 0x04
> +#define AD7124_ID 0x05
> +#define AD7124_ERROR 0x06
> +#define AD7124_ERROR_EN 0x07
> +#define AD7124_MCLK_COUNT 0x08
> +#define AD7124_CHANNEL(x) (0x09 + (x))
> +#define AD7124_CONFIG(x) (0x19 + (x))
> +#define AD7124_FILTER(x) (0x21 + (x))
> +#define AD7124_OFFSET(x) (0x29 + (x))
> +#define AD7124_GAIN(x) (0x31 + (x))
> +
> +/* AD7124_STATUS */
> +#define AD7124_STATUS_POR_FLAG_MSK BIT(4)
> +
> +/* AD7124_ADC_CONTROL */
> +#define AD7124_ADC_CTRL_PWR_MSK GENMASK(7, 6)
> +#define AD7124_ADC_CTRL_PWR(x) FIELD_PREP(AD7124_ADC_CTRL_PWR_MSK, x)
> +#define AD7124_ADC_CTRL_MODE_MSK GENMASK(5, 2)
> +#define AD7124_ADC_CTRL_MODE(x) FIELD_PREP(AD7124_ADC_CTRL_MODE_MSK, x)
> +
> +/* AD7124_CHANNEL_X */
> +#define AD7124_CHANNEL_EN_MSK BIT(15)
> +#define AD7124_CHANNEL_EN(x) FIELD_PREP(AD7124_CHANNEL_EN_MSK, x)
> +#define AD7124_CHANNEL_SETUP_MSK GENMASK(14, 12)
> +#define AD7124_CHANNEL_SETUP(x) FIELD_PREP(AD7124_CHANNEL_SETUP_MSK, x)
> +#define AD7124_CHANNEL_AINP_MSK GENMASK(9, 5)
> +#define AD7124_CHANNEL_AINP(x) FIELD_PREP(AD7124_CHANNEL_AINP_MSK, x)
> +#define AD7124_CHANNEL_AINM_MSK GENMASK(4, 0)
> +#define AD7124_CHANNEL_AINM(x) FIELD_PREP(AD7124_CHANNEL_AINM_MSK, x)
> +
> +/* AD7124_CONFIG_X */
> +#define AD7124_CONFIG_BIPOLAR_MSK BIT(11)
> +#define AD7124_CONFIG_BIPOLAR(x) FIELD_PREP(AD7124_CONFIG_BIPOLAR_MSK, x)
> +#define AD7124_CONFIG_REF_SEL_MSK GENMASK(4, 3)
> +#define AD7124_CONFIG_REF_SEL(x) FIELD_PREP(AD7124_CONFIG_REF_SEL_MSK, x)
> +#define AD7124_CONFIG_PGA_MSK GENMASK(2, 0)
> +#define AD7124_CONFIG_PGA(x) FIELD_PREP(AD7124_CONFIG_PGA_MSK, x)
> +
> +/* AD7124_FILTER_X */
> +#define AD7124_FILTER_FS_MSK GENMASK(10, 0)
> +#define AD7124_FILTER_FS(x) FIELD_PREP(AD7124_FILTER_FS_MSK, x)
> +
> +enum ad7124_ids {
> + ID_AD7124_4,
> + ID_AD7124_8,
> +};
> +
> +enum ad7124_ref_sel {
> + AD7124_REFIN1,
> + AD7124_REFIN2,
> + AD7124_INT_REF,
> + AD7124_AVDD_REF,
> +};
> +
> +enum ad7124_power_mode {
> + AD7124_LOW_POWER,
> + AD7124_MID_POWER,
> + AD7124_FULL_POWER,
> +};
> +
> +static const unsigned int ad7124_gain[8] = {
> + 1, 2, 4, 8, 16, 32, 64, 128
> +};
> +
> +static const int ad7124_master_clk_freq_hz[3] = {
> + [AD7124_LOW_POWER] = 76800,
> + [AD7124_MID_POWER] = 153600,
> + [AD7124_FULL_POWER] = 614400,
> +};
> +
> +static const char * const ad7124_ref_names[] = {
> + [AD7124_REFIN1] = "refin1",
> + [AD7124_REFIN2] = "refin2",
> + [AD7124_INT_REF] = "int",
> + [AD7124_AVDD_REF] = "avdd",
> +};
> +
> +struct ad7124_chip_info {
> + unsigned int num_inputs;
> +};
> +
> +struct ad7124_channel_config {
> + enum ad7124_ref_sel refsel;
> + bool bipolar;
> + unsigned int ain;
> + unsigned int vref_mv;
> + unsigned int pga_bits;
> + unsigned int odr;
> +};
> +
> +struct ad7124_state {
> + const struct ad7124_chip_info *chip_info;
> + struct ad_sigma_delta sd;
> + struct ad7124_channel_config channel_config[4];
> + struct regulator *vref[4];
> + struct clk *mclk;
> + unsigned int adc_control;
> + unsigned int num_channels;
> +};
> +
> +static const struct iio_chan_spec ad7124_channel_template = {
> + .type = IIO_VOLTAGE,
> + .indexed = 1,
> + .differential = 1,
> + .channel = 0,
> + .address = 0,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_OFFSET) |
> + BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .scan_index = 0,

Given I would imagine this is always overwritten, probably
no point in setting it here. Same with channel and address
above.

> + .scan_type = {
> + .sign = 'u',
> + .realbits = 24,
> + .storagebits = 32,
> + .shift = 0,

A shift of 0 is a fairly obvious default and the c standard guarantees
the variable will be initialized. Hence no need to set it here.

> + },
> +};
> +
> +static struct ad7124_chip_info ad7124_chip_info_tbl[] = {
> + [ID_AD7124_4] = {
> + .num_inputs = 8,
> + },
> + [ID_AD7124_8] = {
> + .num_inputs = 16,
> + },
> +};
> +
> +static int ad7124_find_closest_match(const int *array,
> + unsigned int size, int val)
> +{
> + int i;
> +
> + for (i = 0; i < size; i++) {
> + if (val <= array[i])
> + return i;
> + }
> +
> + return size - 1;
> +}
> +
> +static int ad7124_spi_write_mask(struct ad7124_state *st,
> + unsigned int addr,
> + unsigned long mask,
> + unsigned int val,
> + unsigned int bytes)
> +{
> + unsigned int readval;
> + int ret;
> +
> + ret = ad_sd_read_reg(&st->sd, addr, bytes, &readval);
> + if (ret < 0)
> + return ret;
> +
> + readval &= ~mask;
> + readval |= val;
> +
> + return ad_sd_write_reg(&st->sd, addr, bytes, readval);
> +}
> +
> +static int ad7124_set_mode(struct ad_sigma_delta *sd,
> + enum ad_sigma_delta_mode mode)
> +{
> + struct ad7124_state *st = container_of(sd, struct ad7124_state, sd);
> +
> + st->adc_control &= ~AD7124_ADC_CTRL_MODE_MSK;
> + st->adc_control |= AD7124_ADC_CTRL_MODE(mode);
> +
> + return ad_sd_write_reg(&st->sd, AD7124_ADC_CONTROL, 2, st->adc_control);
> +}
> +
> +static int ad7124_set_channel(struct ad_sigma_delta *sd, unsigned int channel)
> +{
> + struct ad7124_state *st = container_of(sd, struct ad7124_state, sd);
> + unsigned int val;
> +
> + val = st->channel_config[channel].ain | AD7124_CHANNEL_EN(1) |
> + AD7124_CHANNEL_SETUP(channel);
> +
> + return ad_sd_write_reg(&st->sd, AD7124_CHANNEL(channel), 2, val);
> +}
> +
> +static const struct ad_sigma_delta_info ad7124_sigma_delta_info = {
> + .set_channel = ad7124_set_channel,
> + .set_mode = ad7124_set_mode,
> + .has_registers = true,
> + .addr_shift = 0,
> + .read_mask = BIT(6),
> + .data_reg = AD7124_DATA,
> +};
> +
> +static int ad7124_set_channel_odr(struct ad7124_state *st,
> + unsigned int channel,
> + unsigned int odr)
> +{
> + unsigned int fclk, odr_sel_bits;
> + int ret;
> +
> + fclk = clk_get_rate(st->mclk);
> + /*
> + * FS[10:0] = fCLK / (fADC x 32) where:
> + * fADC is the output data rate
> + * fCLK is the master clock frequency
> + * FS[10:0] are the bits in the filter register
> + * FS[10:0] can have a value from 1 to 2047
> + */
> + odr_sel_bits = DIV_ROUND_CLOSEST(fclk, odr * 32);
> + if (odr_sel_bits < 1)
> + odr_sel_bits = 1;
> + else if (odr_sel_bits > 2047)
> + odr_sel_bits = 2047;
> +
> + ret = ad7124_spi_write_mask(st, AD7124_FILTER(channel),
> + AD7124_FILTER_FS_MSK,
> + AD7124_FILTER_FS(odr_sel_bits), 3);
> + if (ret < 0)
> + return ret;
> + /* fADC = fCLK / (FS[10:0] x 32) */
> + st->channel_config[channel].odr =
> + DIV_ROUND_CLOSEST(fclk, odr_sel_bits * 32);
> +
> + return 0;
> +}
> +
> +static int ad7124_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long info)
> +{
> + struct ad7124_state *st = iio_priv(indio_dev);
> + int idx, ret;
> +
> + switch (info) {
> + case IIO_CHAN_INFO_RAW:
> + ret = ad_sigma_delta_single_conversion(indio_dev, chan, val);
> + if (ret < 0)
> + return ret;
> +
> + /* After the conversion is performed, disable the channel */
> + ret = ad_sd_write_reg(&st->sd,
> + AD7124_CHANNEL(chan->address), 2,
> + st->channel_config[chan->address].ain |
> + AD7124_CHANNEL_EN(0));
> + if (ret < 0)
> + return ret;
> +
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + idx = st->channel_config[chan->address].pga_bits;
> + *val = st->channel_config[chan->address].vref_mv /
> + ad7124_gain[idx];
> + if (st->channel_config[chan->address].bipolar)
> + *val2 = chan->scan_type.realbits - 1;
> + else
> + *val2 = chan->scan_type.realbits;
> +
> + return IIO_VAL_FRACTIONAL_LOG2;
> + case IIO_CHAN_INFO_OFFSET:
> + if (st->channel_config[chan->address].bipolar) {
> + /* Code = 2^(n − 1) × ((Ain × Gain / Vref) + 1) */
> + idx = st->channel_config[chan->address].pga_bits;
> + *val = -(st->channel_config[chan->address].vref_mv /
> + ad7124_gain[idx]);
> + } else {
> + *val = 0;
> + }
> +
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *val = st->channel_config[chan->address].odr;
> +
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad7124_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long info)
> +{
> + struct ad7124_state *st = iio_priv(indio_dev);
> +
> + switch (info) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + return ad7124_set_channel_odr(st, chan->address, val);

We should probably have a sanity check for val2 being zero given we
are just ignoring it...

> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct iio_info ad7124_info = {
> + .read_raw = ad7124_read_raw,
> + .write_raw = ad7124_write_raw,
> +};
> +
> +static int ad7124_soft_reset(struct ad7124_state *st)
> +{
> + unsigned int readval, timeout;
> + int ret;
> +
> + ret = ad_sd_reset(&st->sd, 64);
> + if (ret < 0)
> + return ret;
> +
> + timeout = 100;
> + do {
> + ret = ad_sd_read_reg(&st->sd, AD7124_STATUS, 1, &readval);
> + if (ret < 0)
> + return ret;
> +
> + if (!(readval & AD7124_STATUS_POR_FLAG_MSK))
> + return 0;
> +
> + /* The AD7124 requires typically 2ms to power up and settle */
> + usleep_range(100, 2000);
> + } while (--timeout);
> +
> + dev_err(&st->sd.spi->dev, "Soft reset failed\n");
> +
> + return -EIO;
> +}
> +
> +static int ad7124_init_channel_vref(struct ad7124_state *st,
> + unsigned int channel_number)
> +{
> + unsigned int refsel = st->channel_config[channel_number].refsel;
> +
> + switch (refsel) {
> + case AD7124_REFIN1:
> + case AD7124_REFIN2:
> + case AD7124_AVDD_REF:
> + if (IS_ERR(st->vref[refsel])) {
> + dev_err(&st->sd.spi->dev,
> + "Error, trying to use external voltage reference without a %s regulator.",
> + ad7124_ref_names[refsel]);
> + return PTR_ERR(st->vref[refsel]);
> + }
> + st->channel_config[channel_number].vref_mv =
> + regulator_get_voltage(st->vref[refsel]);
> + /* Conversion from uV to mV */
> + st->channel_config[channel_number].vref_mv /= 1000;
> + break;
> + case AD7124_INT_REF:
> + st->channel_config[channel_number].vref_mv = 2500;
> + break;
> + default:
> + dev_err(&st->sd.spi->dev, "Invalid reference %d\n", refsel);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
> + struct device_node *np)
> +{
> + struct ad7124_state *st = iio_priv(indio_dev);
> + struct device_node *chan_node, *child;
> + struct iio_chan_spec *chan;
> + unsigned int ain[2], channel = 0, tmp;
> + int ret, res;
> +
> + chan_node = of_get_child_by_name(np, "adi,channels");
> + if (!chan_node) {
> + dev_err(indio_dev->dev.parent,
> + "failed to find channels node\n");
> + return -ENODEV;
> + }
> +
> + st->num_channels = of_get_available_child_count(chan_node);
> + if (!st->num_channels) {
> + dev_err(indio_dev->dev.parent, "no channel children\n");
> + return -ENODEV;
> + }
> +
> + chan = devm_kcalloc(indio_dev->dev.parent, st->num_channels,
> + sizeof(*chan), GFP_KERNEL);
> + if (!chan)
> + return -ENOMEM;
> +
> + indio_dev->channels = chan;
> + indio_dev->num_channels = st->num_channels;
> +
> + for_each_available_child_of_node(chan_node, child) {
> + ret = of_property_read_u32_array(child, "reg", ain, 2);
> + if (ret)
> + goto err;
> +
> + ret = of_property_read_u32(child,
> + "adi,channel-number", &channel);
The line break here is a little odd. I think the string should be on the line
above probably (nitpick of the day ;)

> + if (ret)
> + goto err;
> +
> + if (ain[0] >= st->chip_info->num_inputs ||
> + ain[1] >= st->chip_info->num_inputs) {
> + dev_err(indio_dev->dev.parent,
> + "Input pin number out of range.\n");
> + ret = -EINVAL;
> + goto err;
> + }
> + st->channel_config[channel].ain = AD7124_CHANNEL_AINP(ain[0]) |
> + AD7124_CHANNEL_AINM(ain[1]);
> + st->channel_config[channel].bipolar =
> + of_property_read_bool(child, "adi,bipolar");
> +
> + ret = of_property_read_u32(child, "adi,reference-select", &tmp);
> + if (ret)
> + st->channel_config[channel].refsel = AD7124_INT_REF;
> + else
> + st->channel_config[channel].refsel = tmp;
> +
> + ret = of_property_read_u32(child, "adi,gain", &tmp);
> + if (ret) {
> + st->channel_config[channel].pga_bits = 0;
> + } else {
> + res = ad7124_find_closest_match(ad7124_gain,
> + ARRAY_SIZE(ad7124_gain), tmp);
> + st->channel_config[channel].pga_bits = res;
> + }
> +
> + ret = of_property_read_u32(child, "adi,odr-hz", &tmp);
> + if (ret)
> + /*
> + * 9 SPS is the minimum output data rate supported
> + * regardless of the selected power mode.
> + */
> + st->channel_config[channel].odr = 9;
> + else
> + st->channel_config[channel].odr = tmp;
> +
> + *chan = ad7124_channel_template;
> + chan->address = channel;
> + chan->scan_index = channel;
> + chan->channel = ain[0];
> + chan->channel2 = ain[1];
> +
> + chan++;
> + }
> + of_node_put(chan_node);
> +
> + return 0;
> +err:
> + of_node_put(chan_node);
> + of_node_put(child);
> +
> + return ret;
> +}
> +
> +static int ad7124_setup(struct ad7124_state *st)
> +{
> + unsigned int val, fclk, power_mode;
> + int i, ret;
> +
> + fclk = clk_get_rate(st->mclk);
> + if (!fclk)
> + return -EINVAL;
> +
> + /* The power mode changes the master clock frequency */
> + power_mode = ad7124_find_closest_match(ad7124_master_clk_freq_hz,
> + ARRAY_SIZE(ad7124_master_clk_freq_hz),
> + fclk);
> + if (fclk != ad7124_master_clk_freq_hz[power_mode]) {
> + ret = clk_set_rate(st->mclk, fclk);
> + if (ret)
> + return ret;
> + }
> +
> + /* Set the power mode */
> + st->adc_control &= ~AD7124_ADC_CTRL_PWR_MSK;
> + st->adc_control |= AD7124_ADC_CTRL_PWR(power_mode);
> + ret = ad_sd_write_reg(&st->sd, AD7124_ADC_CONTROL, 2, st->adc_control);
> + if (ret < 0)
> + return ret;
> +
> + for (i = 0; i < st->num_channels; i++) {
> + val = st->channel_config[i].ain | AD7124_CHANNEL_SETUP(i);
> + ret = ad_sd_write_reg(&st->sd, AD7124_CHANNEL(i), 2, val);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad7124_init_channel_vref(st, i);
> + if (ret < 0)
> + return ret;
> +
> + val = AD7124_CONFIG_BIPOLAR(st->channel_config[i].bipolar) |
> + AD7124_CONFIG_REF_SEL(st->channel_config[i].refsel) |
> + AD7124_CONFIG_PGA(st->channel_config[i].pga_bits);
> + ret = ad_sd_write_reg(&st->sd, AD7124_CONFIG(i), 2, val);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad7124_set_channel_odr(st, i, st->channel_config[i].odr);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static int ad7124_probe(struct spi_device *spi)
> +{
> + const struct spi_device_id *id;
> + struct ad7124_state *st;
> + struct iio_dev *indio_dev;
> + int i, ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> +
> + id = spi_get_device_id(spi);
> + st->chip_info = &ad7124_chip_info_tbl[id->driver_data];
> +
> + ad_sd_init(&st->sd, indio_dev, spi, &ad7124_sigma_delta_info);
> +
> + spi_set_drvdata(spi, indio_dev);
> +
> + indio_dev->dev.parent = &spi->dev;
> + indio_dev->name = spi_get_device_id(spi)->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &ad7124_info;
> +
> + ret = ad7124_of_parse_channel_config(indio_dev, spi->dev.of_node);
> + if (ret < 0)
> + return ret;
> +
> + for (i = 0; i < ARRAY_SIZE(st->vref); i++) {
> + if (i != AD7124_INT_REF)
> + st->vref[i] = devm_regulator_get_optional(&spi->dev,
> + ad7124_ref_names[i]);
The complexity of regulator_get_optional, is which cases actually mean there
hasn't been a regulator specified (which is fine) rather than it's not ready
or there has been an error getting the one that was specified.

So I 'believe' that you should only be continuing if you get -ENODEV.
Anything else and you should give up on the probe (perhaps to come
back later if it's a defer.


> + if (IS_ERR_OR_NULL(st->vref[i]))
> + continue;
> +
> + ret = regulator_enable(st->vref[i]);
> + if (ret)
> + return ret;
> + }
> +
> + st->mclk = devm_clk_get(&spi->dev, "mclk");
> + if (IS_ERR(st->mclk)) {
> + ret = PTR_ERR(st->mclk);
> + goto error_regulator_disable;
> + }
> +
> + ret = clk_prepare_enable(st->mclk);
> + if (ret < 0)
> + goto error_regulator_disable;
> +
> + ret = ad7124_soft_reset(st);
> + if (ret < 0)
> + goto error_clk_disable_unprepare;
> +
> + ret = ad7124_setup(st);
> + if (ret < 0)
> + goto error_clk_disable_unprepare;
> +
> + ret = ad_sd_setup_buffer_and_trigger(indio_dev);
> + if (ret < 0)
> + goto error_clk_disable_unprepare;
> +
> + ret = iio_device_register(indio_dev);
> + if (ret < 0) {
> + dev_err(&spi->dev, "Failed to register iio device\n");
> + goto error_remove_trigger;
> + }
> +
> + return 0;
> +
> +error_remove_trigger:
> + ad_sd_cleanup_buffer_and_trigger(indio_dev);
> +error_clk_disable_unprepare:
> + clk_disable_unprepare(st->mclk);
> +error_regulator_disable:
> + for (i = ARRAY_SIZE(st->vref) - 1; i >= 0; i--) {
> + if (!IS_ERR_OR_NULL(st->vref[i]))
> + regulator_disable(st->vref[i]);
> + }
> +
> + return ret;
> +}
> +
> +static int ad7124_remove(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
> + struct ad7124_state *st = iio_priv(indio_dev);
> + int i;
> +
> + iio_device_unregister(indio_dev);
> + ad_sd_cleanup_buffer_and_trigger(indio_dev);
> + clk_disable_unprepare(st->mclk);

I know it is probably fine, but I would definitely prefer the ordering
in here and the error paths in probe to be the same (and the reverse
of the probe order). That puts clk_disable_unprepare before
ad_sd_cleanup_buffer_and_trigger.

> +
> + for (i = ARRAY_SIZE(st->vref) - 1; i >= 0; i--) {
> + if (!IS_ERR_OR_NULL(st->vref[i]))
> + regulator_disable(st->vref[i]);
> + }
> +
> + return 0;
> +}
> +
> +static const struct spi_device_id ad7124_id_table[] = {
> + { "ad7124-4", ID_AD7124_4 },
> + { "ad7124-8", ID_AD7124_8 },
> + {}
> +};
> +MODULE_DEVICE_TABLE(spi, ad7124_id_table);
> +
> +static const struct of_device_id ad7124_of_match[] = {
> + { .compatible = "adi,ad7124-4" },
> + { .compatible = "adi,ad7124-8" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, ad7124_of_match);
> +
> +static struct spi_driver ad71124_driver = {
> + .driver = {
> + .name = "ad7124",
> + .of_match_table = ad7124_of_match,
> + },
> + .probe = ad7124_probe,
> + .remove = ad7124_remove,
> + .id_table = ad7124_id_table,
> +};
> +module_spi_driver(ad71124_driver);
> +
> +MODULE_AUTHOR("Stefan Popa <[email protected]>");
> +MODULE_DESCRIPTION("Analog Devices AD7124 SPI driver");
> +MODULE_LICENSE("GPL");