Added support for ad7770,ad7771,ad7779 ADCs. The
data is streamed only on the spi-mode, without
using the data lines.
Signed-off-by: ranechita <[email protected]>
---
drivers/iio/adc/Kconfig | 11 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad7779.c | 951 +++++++++++++++++++++++++++++++++++++++
3 files changed, 963 insertions(+)
create mode 100644 drivers/iio/adc/ad7779.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 0d9282fa67f5..3e42cbc365d7 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -206,6 +206,17 @@ config AD7768_1
To compile this driver as a module, choose M here: the module will be
called ad7768-1.
+config AD7779
+ tristate "Analog Devices AD7779 ADC driver"
+ depends on SPI
+ select IIO_BUFFER
+ help
+ Say yes here to build support for Analog Devices AD7779 SPI
+ analog to digital converter (ADC)
+
+ To compile this driver as a module, choose M here: the module will be
+ called ad7779.
+
config AD7780
tristate "Analog Devices AD7780 and similar ADCs driver"
depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index b3c434722364..e25997e926bb 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
obj-$(CONFIG_AD7606) += ad7606.o
obj-$(CONFIG_AD7766) += ad7766.o
obj-$(CONFIG_AD7768_1) += ad7768-1.o
+obj-$(CONFIG_AD7779) += ad7779.o
obj-$(CONFIG_AD7780) += ad7780.o
obj-$(CONFIG_AD7791) += ad7791.o
obj-$(CONFIG_AD7793) += ad7793.o
diff --git a/drivers/iio/adc/ad7779.c b/drivers/iio/adc/ad7779.c
new file mode 100644
index 000000000000..089e352e2d40
--- /dev/null
+++ b/drivers/iio/adc/ad7779.c
@@ -0,0 +1,951 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD777X ADC
+ *
+ * Copyright 2023 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitmap.h>
+#include <linux/clk.h>
+#include <linux/crc8.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/string.h>
+#include <linux/units.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/kfifo_buf.h>
+#include <linux/iio/sysfs.h>
+
+#define AD777X_SPI_READ_CMD BIT(7)
+
+#define AD777X_DISABLE_SD BIT(7)
+
+#define AD777X_REG_CH_DISABLE 0x08
+#define AD777X_REG_CH_SYNC_OFFSET(ch) (0x09 + (ch))
+#define AD777X_REG_CH_CONFIG(ch) (0x00 + (ch))
+#define AD777X_REG_GENERAL_USER_CONFIG_1 0x11
+#define AD777X_REG_GENERAL_USER_CONFIG_2 0x12
+#define AD777X_REG_GENERAL_USER_CONFIG_3 0x13
+#define AD777X_REG_DOUT_FORMAT 0x14
+#define AD777X_REG_ADC_MUX_CONFIG 0x15
+#define AD777X_REG_GPIO_CONFIG 0x17
+#define AD777X_REG_BUFFER_CONFIG_1 0x19
+#define AD777X_REG_GLOBAL_MUX_CONFIG 0x16
+#define AD777X_REG_BUFFER_CONFIG_2 0x1A
+#define AD777X_REG_GPIO_DATA 0x18
+#define AD777X_REG_CH_OFFSET_UPPER_BYTE(ch) (0x1C + (ch) * 6)
+#define AD777X_REG_CH_OFFSET_LOWER_BYTE(ch) (0x1E + (ch) * 6)
+#define AD777X_REG_CH_GAIN_UPPER_BYTE(ch) (0x1F + (ch) * 6)
+#define AD777X_REG_CH_OFFSET_MID_BYTE(ch) (0x1D + (ch) * 6)
+#define AD777X_REG_CH_GAIN_MID_BYTE(ch) (0x20 + (ch) * 6)
+#define AD777X_REG_CH_ERR_REG(ch) (0x4C + (ch))
+#define AD777X_REG_CH0_1_SAT_ERR 0x54
+#define AD777X_REG_CH_GAIN_LOWER_BYTE(ch) (0x21 + (ch) * 6)
+#define AD777X_REG_CH2_3_SAT_ERR 0x55
+#define AD777X_REG_CH4_5_SAT_ERR 0x56
+#define AD777X_REG_CH6_7_SAT_ERR 0x57
+#define AD777X_REG_CHX_ERR_REG_EN 0x58
+#define AD777X_REG_GEN_ERR_REG_1 0x59
+#define AD777X_REG_GEN_ERR_REG_1_EN 0x5A
+#define AD777X_REG_GEN_ERR_REG_2 0x5B
+#define AD777X_REG_GEN_ERR_REG_2_EN 0x5C
+#define AD777X_REG_STATUS_REG_1 0x5D
+#define AD777X_REG_STATUS_REG_2 0x5E
+#define AD777X_REG_STATUS_REG_3 0x5F
+#define AD777X_REG_SRC_N_MSB 0x60
+#define AD777X_REG_SRC_N_LSB 0x61
+#define AD777X_REG_SRC_IF_MSB 0x62
+#define AD777X_REG_SRC_IF_LSB 0x63
+#define AD777X_REG_SRC_UPDATE 0x64
+
+#define AD777X_FILTER_MSK BIT(6)
+#define AD777X_MOD_POWERMODE_MSK BIT(6)
+#define AD777X_MOD_PDB_REFOUT_MSK BIT(4)
+#define AD777X_MOD_SPI_EN_MSK BIT(4)
+
+/* AD777X_REG_DOUT_FORMAT */
+#define AD777X_DOUT_FORMAT_MSK GENMASK(7, 6)
+#define AD777X_DOUT_HEADER_FORMAT BIT(5)
+#define AD777X_DCLK_CLK_DIV_MSK GENMASK(3, 1)
+
+#define AD777X_REFMUX_CTRL_MSK GENMASK(7, 6)
+#define AD777X_SPI_CRC_EN_MSK BIT(0)
+
+#define AD777X_MAXCLK_LOWPOWER 4096000
+#define AD777X_NUM_CHANNELS 8
+#define AD777X_RESET_BUF_SIZE 8
+
+#define AD777X_LOWPOWER_DIV 512
+#define AD777X_HIGHPOWER_DIV 2048
+
+#define AD777X_SINC3_MAXFREQ (16 * HZ_PER_KHZ)
+#define AD777X_SINC5_MAXFREQ (128 * HZ_PER_KHZ)
+
+#define AD777X_DEFAULT_SAMPLING_FREQ (8 * HZ_PER_KHZ)
+#define AD777X_DEFAULT_SAMPLING_2LINE (4 * HZ_PER_KHZ)
+#define AD777X_DEFAULT_SAMPLING_1LINE (2 * HZ_PER_KHZ)
+
+#define AD777X_SPIMODE_MAX_SAMP_FREQ (16 * HZ_PER_KHZ)
+
+#define GAIN_REL 0x555555
+#define AD777X_FREQ_MSB_MSK GENMASK(15, 8)
+#define AD777X_FREQ_LSB_MSK GENMASK(7, 0)
+#define AD777X_UPPER GENMASK(23, 16)
+#define AD777X_MID GENMASK(15, 8)
+#define AD777X_LOWER GENMASK(7, 0)
+
+#define AD777X_REG_READ_MSK GENMASK(6, 0)
+
+#define AD777X_CRC8_POLY 0x07
+DECLARE_CRC8_TABLE(ad777x_crc8_table);
+
+enum ad777x_filter {
+ AD777X_SINC3,
+ AD777X_SINC5,
+};
+
+enum ad777x_variant {
+ ad7770,
+ ad7771,
+ ad7779,
+};
+
+enum ad777x_power_mode {
+ AD777X_LOW_POWER,
+ AD777X_HIGH_POWER,
+};
+
+struct ad777x_chip_info {
+ const char *name;
+ struct iio_chan_spec const *channels;
+};
+
+struct ad777x_state {
+ struct spi_device *spi;
+ const struct ad777x_chip_info *chip_info;
+ struct clk *mclk;
+ struct regulator *vref;
+ unsigned int sampling_freq;
+ enum ad777x_power_mode power_mode;
+ enum ad777x_filter filter_enabled;
+ unsigned int active_ch;
+ unsigned int spidata_mode;
+ unsigned int crc_enabled;
+
+ /*
+ * DMA (thus cache coherency maintenance) requires the
+ * transfer buffers to live in their own cache lines.
+ */
+ u8 reg_rx_buf[3] ____cacheline_aligned;
+ u8 reg_tx_buf[3];
+ __be32 spidata_rx[8];
+ __be32 spidata_tx[8];
+ u8 reset_buf[8];
+};
+
+static const char * const ad777x_filter_type[] = {
+ [AD777X_SINC3] = "sinc3_filter",
+ [AD777X_SINC5] = "sinc5_filter",
+};
+
+static int ad777x_spi_read(struct ad777x_state *st, u8 reg, u8 *rbuf)
+{
+ int ret;
+ int length = 2;
+ u8 crc_buf[2];
+ u8 exp_crc = 0;
+ struct spi_transfer reg_read_tr[] = {
+ {
+ .tx_buf = st->reg_tx_buf,
+ .rx_buf = st->reg_rx_buf,
+ },
+ };
+
+ if (st->crc_enabled)
+ length = 3;
+ reg_read_tr[0].len = length;
+
+ st->reg_tx_buf[0] = AD777X_SPI_READ_CMD | (reg & 0x7F);
+ st->reg_tx_buf[1] = 0;
+ st->reg_tx_buf[2] = crc8(ad777x_crc8_table, st->reg_tx_buf, 2, 0);
+
+ ret = spi_sync_transfer(st->spi, reg_read_tr, ARRAY_SIZE(reg_read_tr));
+ if (ret)
+ return ret;
+
+ crc_buf[0] = AD777X_SPI_READ_CMD | FIELD_GET(AD777X_REG_READ_MSK, reg);
+ crc_buf[1] = st->reg_rx_buf[1];
+ exp_crc = crc8(ad777x_crc8_table, crc_buf, 2, 0);
+ if (st->crc_enabled && exp_crc != st->reg_rx_buf[2]) {
+ dev_err(&st->spi->dev, "Bad CRC %x, expected %x",
+ st->reg_rx_buf[2], exp_crc);
+ return -EINVAL;
+ }
+ *rbuf = st->reg_rx_buf[1];
+
+ return 0;
+}
+
+static int ad777x_spi_write(struct ad777x_state *st, u8 reg, u8 val)
+{
+ int length = 2;
+ struct spi_transfer reg_write_tr[] = {
+ {
+ .tx_buf = st->reg_tx_buf,
+ },
+ };
+
+ if (st->crc_enabled)
+ length = 3;
+ reg_write_tr[0].len = length;
+
+ st->reg_tx_buf[0] = reg & 0x7F;
+ st->reg_tx_buf[1] = val;
+ st->reg_tx_buf[2] = crc8(ad777x_crc8_table, st->reg_tx_buf, 2, 0);
+
+ return spi_sync_transfer(st->spi, reg_write_tr, ARRAY_SIZE(reg_write_tr));
+}
+
+static int ad777x_spi_write_mask(struct ad777x_state *st, u8 reg, u8 mask,
+ u8 val)
+{
+ int ret;
+ u8 regval, data;
+
+ ret = ad777x_spi_read(st, reg, &data);
+ if (ret)
+ return ret;
+
+ regval = data;
+ regval &= ~mask;
+ regval |= val;
+
+ if (regval == data)
+ return 0;
+
+ return ad777x_spi_write(st, reg, regval);
+
+}
+
+static int ad777x_reg_access(struct iio_dev *indio_dev,
+ unsigned int reg,
+ unsigned int writeval,
+ unsigned int *readval)
+{
+ struct ad777x_state *st = iio_priv(indio_dev);
+
+ if (readval)
+ return ad777x_spi_read(st, reg, (u8 *)readval);
+
+ return ad777x_spi_write(st, reg, writeval);
+}
+
+static int ad777x_set_sampling_frequency(struct ad777x_state *st,
+ unsigned int sampling_freq)
+{
+ int ret;
+ unsigned int dec;
+ unsigned int div;
+ unsigned int decimal;
+ int temp;
+ unsigned int kfreq;
+ u8 msb, lsb;
+
+ if (st->filter_enabled == AD777X_SINC3 &&
+ sampling_freq > AD777X_SINC3_MAXFREQ)
+ return -EINVAL;
+
+ if (st->filter_enabled == AD777X_SINC5 &&
+ sampling_freq > AD777X_SINC5_MAXFREQ)
+ return -EINVAL;
+
+ if (st->spidata_mode == 1 &&
+ sampling_freq > AD777X_SPIMODE_MAX_SAMP_FREQ)
+ return -EINVAL;
+
+ if (st->power_mode == AD777X_LOW_POWER)
+ div = AD777X_LOWPOWER_DIV;
+ else
+ div = AD777X_HIGHPOWER_DIV;
+
+ kfreq = sampling_freq / KILO;
+ dec = div / kfreq;
+
+ lsb = FIELD_GET(AD777X_FREQ_LSB_MSK, dec);
+ msb = FIELD_GET(AD777X_FREQ_MSB_MSK, dec);
+
+ ret = ad777x_spi_write(st, AD777X_REG_SRC_N_LSB, lsb);
+ if (ret)
+ return ret;
+ ret = ad777x_spi_write(st, AD777X_REG_SRC_N_MSB, msb);
+ if (ret)
+ return ret;
+
+ if (div % kfreq) {
+ temp = (div * KILO) / kfreq;
+ decimal = ((temp - dec * KILO) << 16) / KILO;
+ lsb = FIELD_GET(AD777X_FREQ_LSB_MSK, decimal);
+ msb = FIELD_GET(AD777X_FREQ_MSB_MSK, decimal);
+
+ ret = ad777x_spi_write(st, AD777X_REG_SRC_IF_LSB, lsb);
+ if (ret)
+ return ret;
+ ret = ad777x_spi_write(st, AD777X_REG_SRC_IF_MSB, msb);
+ if (ret)
+ return ret;
+ } else {
+ ret = ad777x_spi_write(st, AD777X_REG_SRC_IF_LSB, 0x0);
+ if (ret)
+ return ret;
+ ret = ad777x_spi_write(st, AD777X_REG_SRC_IF_MSB, 0x0);
+ if (ret)
+ return ret;
+ }
+ ret = ad777x_spi_write(st, AD777X_REG_SRC_UPDATE, 0x1);
+ if (ret)
+ return ret;
+ fsleep(15);
+ ret = ad777x_spi_write(st, AD777X_REG_SRC_UPDATE, 0x0);
+ if (ret)
+ return ret;
+ fsleep(15);
+
+ st->sampling_freq = sampling_freq;
+
+ return 0;
+}
+
+static int ad777x_get_filter(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan)
+{
+ struct ad777x_state *st = iio_priv(indio_dev);
+ u8 temp;
+ int ret;
+
+ ret = ad777x_spi_read(st, AD777X_REG_GENERAL_USER_CONFIG_2, &temp);
+ if (ret)
+ return ret;
+
+ return FIELD_GET(AD777X_FILTER_MSK, temp);
+}
+
+static int ad777x_set_filter(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ unsigned int mode)
+{
+ struct ad777x_state *st = iio_priv(indio_dev);
+ int ret;
+
+ ret = ad777x_spi_write_mask(st,
+ AD777X_REG_GENERAL_USER_CONFIG_2,
+ AD777X_FILTER_MSK,
+ FIELD_PREP(AD777X_FILTER_MSK, mode));
+ if (ret < 0)
+ return ret;
+
+ ret = ad777x_set_sampling_frequency(st, st->sampling_freq);
+ if (ret < 0)
+ return ret;
+
+ st->filter_enabled = mode;
+
+ return 0;
+}
+
+static int ad777x_get_calibscale(struct ad777x_state *st, int channel)
+{
+ int ret;
+ u8 low, mid, high;
+
+ ret = ad777x_spi_read(st, AD777X_REG_CH_GAIN_LOWER_BYTE(channel), &low);
+ if (ret)
+ return ret;
+ ret = ad777x_spi_read(st, AD777X_REG_CH_GAIN_MID_BYTE(channel), &mid);
+ if (ret)
+ return ret;
+ ret = ad777x_spi_read(st, AD777X_REG_CH_GAIN_UPPER_BYTE(channel), &high);
+ if (ret)
+ return ret;
+
+ return FIELD_PREP(AD777X_UPPER, high) | FIELD_PREP(AD777X_MID, mid) |
+ FIELD_PREP(AD777X_LOWER, low);
+}
+
+static int ad777x_set_calibscale(struct ad777x_state *st, int channel, int val)
+{
+ int ret;
+ u8 msb, mid, lsb;
+ unsigned int gain;
+ unsigned long long tmp;
+
+ tmp = val * 5592405LL;
+ gain = DIV_ROUND_CLOSEST_ULL(tmp, MEGA);
+ msb = FIELD_GET(AD777X_UPPER, gain);
+ mid = FIELD_GET(AD777X_MID, gain);
+ lsb = FIELD_GET(AD777X_LOWER, gain);
+ ret = ad777x_spi_write(st,
+ AD777X_REG_CH_GAIN_UPPER_BYTE(channel),
+ msb);
+ if (ret)
+ return ret;
+ ret = ad777x_spi_write(st,
+ AD777X_REG_CH_GAIN_MID_BYTE(channel),
+ mid);
+ if (ret)
+ return ret;
+ return ad777x_spi_write(st,
+ AD777X_REG_CH_GAIN_LOWER_BYTE(channel),
+ lsb);
+}
+
+static int ad777x_get_calibbias(struct ad777x_state *st, int channel)
+{
+ int ret;
+ u8 low, mid, high;
+
+ ret = ad777x_spi_read(st, AD777X_REG_CH_OFFSET_LOWER_BYTE(channel),
+ &low);
+ if (ret)
+ return ret;
+ ret = ad777x_spi_read(st, AD777X_REG_CH_OFFSET_MID_BYTE(channel), &mid);
+ if (ret)
+ return ret;
+ ret = ad777x_spi_read(st,
+ AD777X_REG_CH_OFFSET_UPPER_BYTE(channel),
+ &high);
+ if (ret)
+ return ret;
+
+ return FIELD_PREP(AD777X_UPPER, high) | FIELD_PREP(AD777X_MID, mid) |
+ FIELD_PREP(AD777X_LOWER, low);
+}
+
+static int ad777x_set_calibbias(struct ad777x_state *st, int channel, int val)
+{
+ int ret;
+ u8 msb, mid, lsb;
+
+ msb = FIELD_GET(AD777X_UPPER, val);
+ mid = FIELD_GET(AD777X_MID, val);
+ lsb = FIELD_GET(AD777X_LOWER, val);
+ ret = ad777x_spi_write(st,
+ AD777X_REG_CH_OFFSET_UPPER_BYTE(channel),
+ msb);
+ if (ret)
+ return ret;
+ ret = ad777x_spi_write(st,
+ AD777X_REG_CH_OFFSET_MID_BYTE(channel),
+ mid);
+ if (ret)
+ return ret;
+ return ad777x_spi_write(st,
+ AD777X_REG_CH_OFFSET_LOWER_BYTE(channel),
+ lsb);
+}
+
+static int ad777x_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val,
+ int *val2,
+ long mask)
+{
+ struct ad777x_state *st = iio_priv(indio_dev);
+ int ret;
+
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_CALIBSCALE:
+ *val = ad777x_get_calibscale(st, chan->channel);
+ iio_device_release_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+ *val2 = GAIN_REL;
+ return IIO_VAL_FRACTIONAL;
+ case IIO_CHAN_INFO_CALIBBIAS:
+ *val = ad777x_get_calibbias(st, chan->channel);
+ iio_device_release_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ *val = st->sampling_freq;
+ iio_device_release_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+ return IIO_VAL_INT;
+ }
+
+ iio_device_release_direct_mode(indio_dev);
+
+ return -EINVAL;
+}
+
+static int ad777x_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
+{
+ struct ad777x_state *st = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_CALIBSCALE:
+ return ad777x_set_calibscale(st, chan->channel, val2);
+ case IIO_CHAN_INFO_CALIBBIAS:
+ return ad777x_set_calibbias(st, chan->channel, val);
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ return ad777x_set_sampling_frequency(st, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad777x_update_scan_mode(struct iio_dev *indio_dev,
+ const unsigned long *scan_mask)
+{
+ struct ad777x_state *st = iio_priv(indio_dev);
+
+ bitmap_copy((unsigned long *)&st->active_ch, scan_mask, AD777X_NUM_CHANNELS);
+
+ return 0;
+}
+
+static int ad777x_buffer_postenable(struct iio_dev *indio_dev)
+{
+ int ret;
+ struct ad777x_state *st = iio_priv(indio_dev);
+
+ ret = ad777x_spi_write_mask(st,
+ AD777X_REG_GENERAL_USER_CONFIG_3,
+ AD777X_MOD_SPI_EN_MSK,
+ FIELD_PREP(AD777X_MOD_SPI_EN_MSK, 1));
+ if (ret)
+ return ret;
+ enable_irq(st->spi->irq);
+
+ return 0;
+}
+
+static int ad777x_buffer_predisable(struct iio_dev *indio_dev)
+{
+ int ret;
+ struct ad777x_state *st = iio_priv(indio_dev);
+
+ disable_irq_nosync(st->spi->irq);
+ ret = ad777x_spi_write(st,
+ AD777X_REG_GENERAL_USER_CONFIG_3,
+ AD777X_DISABLE_SD);
+ return ret;
+}
+
+static irqreturn_t ad777x_irq_handler(int irq, void *data)
+{
+ struct iio_dev *indio_dev = data;
+ struct ad777x_state *st = iio_priv(indio_dev);
+ int ret;
+ __be32 tmp[8];
+ int i;
+ int k = 0;
+
+ struct spi_transfer sd_readback_tr[] = {
+ {
+ .rx_buf = st->spidata_rx,
+ .tx_buf = st->spidata_tx,
+ .len = 32,
+ }
+ };
+
+ if (iio_buffer_enabled(indio_dev)) {
+ st->spidata_tx[0] = AD777X_SPI_READ_CMD;
+ ret = spi_sync_transfer(st->spi, sd_readback_tr,
+ ARRAY_SIZE(sd_readback_tr));
+ if (ret) {
+ dev_err(&st->spi->dev,
+ "spi transfer error in irq handler");
+ return IRQ_HANDLED;
+ }
+ for (i = 0; i < AD777X_NUM_CHANNELS; i++) {
+ if (st->active_ch & BIT(i))
+ tmp[k++] = __be32_to_cpu(st->spidata_rx[i]);
+ }
+ iio_push_to_buffers(indio_dev, &tmp[0]);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int ad777x_reset(struct iio_dev *indio_dev, struct gpio_desc *reset_gpio)
+{
+ struct ad777x_state *st = iio_priv(indio_dev);
+ int ret;
+ struct spi_transfer reg_read_tr[] = {
+ {
+ .tx_buf = st->reset_buf,
+ .len = 8,
+ },
+ };
+
+ memset(st->reset_buf, 0xff, sizeof(st->reset_buf));
+
+ if (reset_gpio) {
+ gpiod_set_value(reset_gpio, 1);
+ fsleep(230);
+ return 0;
+ }
+
+ ret = spi_sync_transfer(st->spi, reg_read_tr,
+ ARRAY_SIZE(reg_read_tr));
+ if (ret)
+ return ret;
+
+ fsleep(230);
+
+ return 0;
+}
+
+static const struct iio_info ad777x_info = {
+ .read_raw = ad777x_read_raw,
+ .write_raw = ad777x_write_raw,
+ .debugfs_reg_access = &ad777x_reg_access,
+ .update_scan_mode = &ad777x_update_scan_mode,
+};
+
+static const struct iio_enum ad777x_filter_enum = {
+ .items = ad777x_filter_type,
+ .num_items = ARRAY_SIZE(ad777x_filter_type),
+ .get = ad777x_get_filter,
+ .set = ad777x_set_filter,
+};
+
+static const struct iio_chan_spec_ext_info ad777x_ext_filter[] = {
+ IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &ad777x_filter_enum),
+ IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL,
+ &ad777x_filter_enum),
+ { }
+};
+
+#define AD777x_CHAN_S(index, _ext_info) \
+ { \
+ .type = IIO_VOLTAGE, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_CALIBSCALE) | \
+ BIT(IIO_CHAN_INFO_CALIBBIAS), \
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .address = (index), \
+ .indexed = 1, \
+ .channel = (index), \
+ .scan_index = (index), \
+ .ext_info = (_ext_info), \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 24, \
+ .storagebits = 32, \
+ }, \
+ }
+
+#define AD777x_CHAN_NO_FILTER_S(index) \
+ AD777x_CHAN_S(index, NULL)
+
+#define AD777x_CHAN_FILTER_S(index) \
+ AD777x_CHAN_S(index, ad777x_ext_filter)
+
+static const struct iio_chan_spec ad777x_channels[] = {
+ AD777x_CHAN_NO_FILTER_S(0),
+ AD777x_CHAN_NO_FILTER_S(1),
+ AD777x_CHAN_NO_FILTER_S(2),
+ AD777x_CHAN_NO_FILTER_S(3),
+ AD777x_CHAN_NO_FILTER_S(4),
+ AD777x_CHAN_NO_FILTER_S(5),
+ AD777x_CHAN_NO_FILTER_S(6),
+ AD777x_CHAN_NO_FILTER_S(7),
+};
+
+static const struct iio_chan_spec ad777x_channels_filter[] = {
+ AD777x_CHAN_FILTER_S(0),
+ AD777x_CHAN_FILTER_S(1),
+ AD777x_CHAN_FILTER_S(2),
+ AD777x_CHAN_FILTER_S(3),
+ AD777x_CHAN_FILTER_S(4),
+ AD777x_CHAN_FILTER_S(5),
+ AD777x_CHAN_FILTER_S(6),
+ AD777x_CHAN_FILTER_S(7),
+};
+
+const struct iio_buffer_setup_ops ad777x_buffer_setup_ops = {
+ .postenable = ad777x_buffer_postenable,
+ .predisable = ad777x_buffer_predisable,
+};
+
+static void ad777x_clk_disable(void *data)
+{
+ clk_disable_unprepare(data);
+}
+
+static void ad777x_reg_disable(void *data)
+{
+ regulator_disable(data);
+}
+
+static int ad777x_register(struct ad777x_state *st, struct iio_dev *indio_dev)
+{
+ int ret;
+ struct device *dev = &st->spi->dev;
+
+ indio_dev->name = st->chip_info->name;
+ indio_dev->info = &ad777x_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = st->chip_info->channels;
+ indio_dev->num_channels = ARRAY_SIZE(ad777x_channels);
+
+ ret = devm_request_threaded_irq(dev, st->spi->irq, NULL,
+ ad777x_irq_handler, IRQF_ONESHOT,
+ indio_dev->name, indio_dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "request irq %d failed\n",
+ st->spi->irq);
+
+ ret = devm_iio_kfifo_buffer_setup_ext(dev, indio_dev,
+ &ad777x_buffer_setup_ops,
+ NULL);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "setup buffer failed\n");
+
+ ret = ad777x_spi_write_mask(st, AD777X_REG_DOUT_FORMAT,
+ AD777X_DCLK_CLK_DIV_MSK,
+ FIELD_PREP(AD777X_DCLK_CLK_DIV_MSK, 7));
+ if (ret)
+ return ret;
+ st->spidata_mode = 1;
+
+ disable_irq_nosync(st->spi->irq);
+
+ return devm_iio_device_register(&st->spi->dev, indio_dev);
+}
+
+static int ad777x_powerup(struct ad777x_state *st, struct gpio_desc *start_gpio)
+{
+ int ret;
+
+ ret = ad777x_spi_write_mask(st, AD777X_REG_GENERAL_USER_CONFIG_1,
+ AD777X_MOD_POWERMODE_MSK,
+ FIELD_PREP(AD777X_MOD_POWERMODE_MSK, 1));
+ if (ret)
+ return ret;
+ ret = ad777x_spi_write_mask(st, AD777X_REG_GENERAL_USER_CONFIG_1,
+ AD777X_MOD_PDB_REFOUT_MSK,
+ FIELD_PREP(AD777X_MOD_PDB_REFOUT_MSK, 1));
+ if (ret)
+ return ret;
+ ret = ad777x_spi_write_mask(st, AD777X_REG_DOUT_FORMAT,
+ AD777X_DCLK_CLK_DIV_MSK,
+ FIELD_PREP(AD777X_DCLK_CLK_DIV_MSK, 1));
+ if (ret)
+ return ret;
+ ret = ad777x_spi_write_mask(st, AD777X_REG_ADC_MUX_CONFIG,
+ AD777X_REFMUX_CTRL_MSK,
+ FIELD_PREP(AD777X_REFMUX_CTRL_MSK, 1));
+ if (ret)
+ return ret;
+ ret = ad777x_spi_write_mask(st, AD777X_REG_GEN_ERR_REG_1_EN,
+ AD777X_SPI_CRC_EN_MSK,
+ FIELD_PREP(AD777X_SPI_CRC_EN_MSK, 1));
+ if (ret)
+ return ret;
+
+ st->power_mode = AD777X_HIGH_POWER;
+ st->crc_enabled = true;
+ ret = ad777x_set_sampling_frequency(st, AD777X_DEFAULT_SAMPLING_FREQ);
+ if (ret)
+ return ret;
+
+ gpiod_set_value(start_gpio, 0);
+ fsleep(15);
+ gpiod_set_value(start_gpio, 1);
+ fsleep(15);
+ gpiod_set_value(start_gpio, 0);
+ fsleep(15);
+
+ return 0;
+}
+
+static int ad777x_probe(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev;
+ struct ad777x_state *st;
+ struct gpio_desc *reset_gpio;
+ struct gpio_desc *start_gpio;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+
+ st->vref = devm_regulator_get_optional(&spi->dev, "vref");
+ if (IS_ERR(st->vref))
+ return PTR_ERR(st->vref);
+
+ ret = regulator_enable(st->vref);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(&spi->dev, ad777x_reg_disable,
+ st->vref);
+ if (ret)
+ return ret;
+
+ st->mclk = devm_clk_get(&spi->dev, "mclk");
+ if (IS_ERR(st->mclk))
+ return PTR_ERR(st->mclk);
+
+ ret = clk_prepare_enable(st->mclk);
+ if (ret < 0)
+ return ret;
+
+ ret = devm_add_action_or_reset(&spi->dev, ad777x_clk_disable,
+ st->mclk);
+ if (ret)
+ return ret;
+
+ reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(reset_gpio))
+ return PTR_ERR(reset_gpio);
+
+ start_gpio = devm_gpiod_get(&spi->dev, "start", GPIOD_OUT_HIGH);
+ if (IS_ERR(start_gpio))
+ return PTR_ERR(start_gpio);
+
+ crc8_populate_msb(ad777x_crc8_table, AD777X_CRC8_POLY);
+ st->spi = spi;
+
+ st->chip_info = spi_get_device_match_data(spi);
+ if (!st->chip_info)
+ return -ENODEV;
+
+ ret = ad777x_reset(indio_dev, start_gpio);
+ if (ret)
+ return ret;
+
+ ad777x_powerup(st, start_gpio);
+ if (ret)
+ return ret;
+
+ if (spi->irq)
+ ret = ad777x_register(st, indio_dev);
+
+ return ret;
+}
+
+static int __maybe_unused ad777x_suspend(struct device *dev)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct ad777x_state *st = iio_priv(indio_dev);
+ int ret;
+
+ ret = ad777x_spi_write_mask(st, AD777X_REG_GENERAL_USER_CONFIG_1,
+ AD777X_MOD_POWERMODE_MSK,
+ FIELD_PREP(AD777X_MOD_POWERMODE_MSK,
+ AD777X_LOW_POWER));
+ if (ret)
+ return ret;
+
+ st->power_mode = AD777X_LOW_POWER;
+ return 0;
+}
+
+static int __maybe_unused ad777x_resume(struct device *dev)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct ad777x_state *st = iio_priv(indio_dev);
+ int ret;
+
+ ret = ad777x_spi_write_mask(st, AD777X_REG_GENERAL_USER_CONFIG_1,
+ AD777X_MOD_POWERMODE_MSK,
+ FIELD_PREP(AD777X_MOD_POWERMODE_MSK,
+ AD777X_HIGH_POWER));
+ if (ret)
+ return ret;
+
+ st->power_mode = AD777X_HIGH_POWER;
+
+ return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(ad777x_pm_ops, ad777x_suspend, ad777x_resume);
+
+static const struct ad777x_chip_info ad7770_chip_info = {
+ .name = "ad7770",
+ .channels = ad777x_channels,
+};
+
+static const struct ad777x_chip_info ad7771_chip_info = {
+ .name = "ad7771",
+ .channels = ad777x_channels_filter,
+};
+
+static const struct ad777x_chip_info ad7779_chip_info = {
+ .name = "ad7779",
+ .channels = ad777x_channels,
+};
+
+static const struct spi_device_id ad777x_id[] = {
+ {
+ .name = "ad7770",
+ .driver_data = (__kernel_ulong_t)&ad7770_chip_info
+ },
+ {
+ .name = "ad7771",
+ .driver_data = (__kernel_ulong_t)&ad7771_chip_info
+ },
+ {
+ .name = "ad7779",
+ .driver_data = (__kernel_ulong_t)&ad7779_chip_info
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, ad777x_id);
+
+static const struct of_device_id ad777x_of_table[] = {
+ {
+ .compatible = "adi,ad7770",
+ .data = &ad7770_chip_info,
+ },
+ {
+ .compatible = "adi,ad7771",
+ .data = &ad7771_chip_info,
+ },
+ {
+ .compatible = "adi,ad7779",
+ .data = &ad7779_chip_info,
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad777x_of_table);
+
+static struct spi_driver ad777x_driver = {
+ .driver = {
+ .name = "ad777x",
+ .pm = pm_sleep_ptr(&ad777x_pm_ops),
+ .of_match_table = ad777x_of_table,
+ },
+ .probe = ad777x_probe,
+ .id_table = ad777x_id,
+};
+module_spi_driver(ad777x_driver);
+
+MODULE_AUTHOR("Ramona Alexandra Nechita <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD777X ADC");
+MODULE_LICENSE("GPL");
--
2.43.0
On Wed, May 29, 2024 at 06:03:09PM +0300, ranechita wrote:
> Added support for ad7770,ad7771,ad7779 ADCs. The
> data is streamed only on the spi-mode, without
> using the data lines.
Do not send new version if we have not settled down everything in the previous
review round, hence I even won't bother to look into the code. Will wait your
replies in v1 and, when we got into agreement, v3 after that.
> Signed-off-by: ranechita <[email protected]>
> ---
This misses a lot of information I asked about in the v1 round of review.
--
With Best Regards,
Andy Shevchenko
Hello,
It misses a chunk of it because part of the code added there was not meant for upstreaming due to the axi interface.
Best Regards,
Ramona
-----Original Message-----
From: Andy Shevchenko <[email protected]>
Sent: Wednesday, May 29, 2024 6:13 PM
To: Nechita, Ramona <[email protected]>
Cc: [email protected]; Jonathan Cameron <[email protected]>; Lars-Peter Clausen <[email protected]>; Hennerich, Michael <Michael.Hennerich@analogcom>; Liam Girdwood <[email protected]>; Mark Brown <[email protected]>; Sa, Nuno <[email protected]>; Schmitt, Marcelo <[email protected]>; Marius Cristea <[email protected]>; Maksim Kiselev <[email protected]>; Marcus Folkesson <[email protected]>; Sahin, Okan <[email protected]>; Mike Looijmans <[email protected]>; Liam Beguin <[email protected]>; Ivan Mikhaylov <[email protected]>; [email protected]
Subject: Re: [PATCH v2] drivers: iio: adc: add support for ad777x family
[External]
On Wed, May 29, 2024 at 06:03:09PM +0300, ranechita wrote:
> Added support for ad7770,ad7771,ad7779 ADCs. The data is streamed only
> on the spi-mode, without using the data lines.
Do not send new version if we have not settled down everything in the previous review round, hence I even won't bother to look into the code. Will wait your replies in v1 and, when we got into agreement, v3 after that.
> Signed-off-by: ranechita <[email protected]>
> ---
This misses a lot of information I asked about in the v1 round of review.
--
With Best Regards,
Andy Shevchenko
Hi ranechita,
kernel test robot noticed the following build warnings:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.10-rc1 next-20240529]
[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/ranechita/drivers-iio-adc-add-support-for-ad777x-family/20240529-230814
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20240529150322.28018-1-ramona.nechita%40analog.com
patch subject: [PATCH v2] drivers: iio: adc: add support for ad777x family
config: microblaze-randconfig-r113-20240531 (https://download.01.org/0day-ci/archive/20240531/[email protected]/config)
compiler: microblaze-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240531/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
sparse warnings: (new ones prefixed by >>)
>> drivers/iio/adc/ad7779.c:571:35: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __be32 @@ got unsigned long @@
drivers/iio/adc/ad7779.c:571:35: sparse: expected restricted __be32
drivers/iio/adc/ad7779.c:571:35: sparse: got unsigned long
>> drivers/iio/adc/ad7779.c:581:42: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __be32 @@ got unsigned int [usertype] @@
drivers/iio/adc/ad7779.c:581:42: sparse: expected restricted __be32
drivers/iio/adc/ad7779.c:581:42: sparse: got unsigned int [usertype]
>> drivers/iio/adc/ad7779.c:685:35: sparse: sparse: symbol 'ad777x_buffer_setup_ops' was not declared. Should it be static?
drivers/iio/adc/ad7779.c: note: in included file (through include/linux/mutex.h, include/linux/notifier.h, include/linux/clk.h):
include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
vim +571 drivers/iio/adc/ad7779.c
552
553 static irqreturn_t ad777x_irq_handler(int irq, void *data)
554 {
555 struct iio_dev *indio_dev = data;
556 struct ad777x_state *st = iio_priv(indio_dev);
557 int ret;
558 __be32 tmp[8];
559 int i;
560 int k = 0;
561
562 struct spi_transfer sd_readback_tr[] = {
563 {
564 .rx_buf = st->spidata_rx,
565 .tx_buf = st->spidata_tx,
566 .len = 32,
567 }
568 };
569
570 if (iio_buffer_enabled(indio_dev)) {
> 571 st->spidata_tx[0] = AD777X_SPI_READ_CMD;
572 ret = spi_sync_transfer(st->spi, sd_readback_tr,
573 ARRAY_SIZE(sd_readback_tr));
574 if (ret) {
575 dev_err(&st->spi->dev,
576 "spi transfer error in irq handler");
577 return IRQ_HANDLED;
578 }
579 for (i = 0; i < AD777X_NUM_CHANNELS; i++) {
580 if (st->active_ch & BIT(i))
> 581 tmp[k++] = __be32_to_cpu(st->spidata_rx[i]);
582 }
583 iio_push_to_buffers(indio_dev, &tmp[0]);
584 }
585
586 return IRQ_HANDLED;
587 }
588
589 static int ad777x_reset(struct iio_dev *indio_dev, struct gpio_desc *reset_gpio)
590 {
591 struct ad777x_state *st = iio_priv(indio_dev);
592 int ret;
593 struct spi_transfer reg_read_tr[] = {
594 {
595 .tx_buf = st->reset_buf,
596 .len = 8,
597 },
598 };
599
600 memset(st->reset_buf, 0xff, sizeof(st->reset_buf));
601
602 if (reset_gpio) {
603 gpiod_set_value(reset_gpio, 1);
604 fsleep(230);
605 return 0;
606 }
607
608 ret = spi_sync_transfer(st->spi, reg_read_tr,
609 ARRAY_SIZE(reg_read_tr));
610 if (ret)
611 return ret;
612
613 fsleep(230);
614
615 return 0;
616 }
617
618 static const struct iio_info ad777x_info = {
619 .read_raw = ad777x_read_raw,
620 .write_raw = ad777x_write_raw,
621 .debugfs_reg_access = &ad777x_reg_access,
622 .update_scan_mode = &ad777x_update_scan_mode,
623 };
624
625 static const struct iio_enum ad777x_filter_enum = {
626 .items = ad777x_filter_type,
627 .num_items = ARRAY_SIZE(ad777x_filter_type),
628 .get = ad777x_get_filter,
629 .set = ad777x_set_filter,
630 };
631
632 static const struct iio_chan_spec_ext_info ad777x_ext_filter[] = {
633 IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &ad777x_filter_enum),
634 IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL,
635 &ad777x_filter_enum),
636 { }
637 };
638
639 #define AD777x_CHAN_S(index, _ext_info) \
640 { \
641 .type = IIO_VOLTAGE, \
642 .info_mask_separate = BIT(IIO_CHAN_INFO_CALIBSCALE) | \
643 BIT(IIO_CHAN_INFO_CALIBBIAS), \
644 .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
645 .address = (index), \
646 .indexed = 1, \
647 .channel = (index), \
648 .scan_index = (index), \
649 .ext_info = (_ext_info), \
650 .scan_type = { \
651 .sign = 's', \
652 .realbits = 24, \
653 .storagebits = 32, \
654 }, \
655 }
656
657 #define AD777x_CHAN_NO_FILTER_S(index) \
658 AD777x_CHAN_S(index, NULL)
659
660 #define AD777x_CHAN_FILTER_S(index) \
661 AD777x_CHAN_S(index, ad777x_ext_filter)
662
663 static const struct iio_chan_spec ad777x_channels[] = {
664 AD777x_CHAN_NO_FILTER_S(0),
665 AD777x_CHAN_NO_FILTER_S(1),
666 AD777x_CHAN_NO_FILTER_S(2),
667 AD777x_CHAN_NO_FILTER_S(3),
668 AD777x_CHAN_NO_FILTER_S(4),
669 AD777x_CHAN_NO_FILTER_S(5),
670 AD777x_CHAN_NO_FILTER_S(6),
671 AD777x_CHAN_NO_FILTER_S(7),
672 };
673
674 static const struct iio_chan_spec ad777x_channels_filter[] = {
675 AD777x_CHAN_FILTER_S(0),
676 AD777x_CHAN_FILTER_S(1),
677 AD777x_CHAN_FILTER_S(2),
678 AD777x_CHAN_FILTER_S(3),
679 AD777x_CHAN_FILTER_S(4),
680 AD777x_CHAN_FILTER_S(5),
681 AD777x_CHAN_FILTER_S(6),
682 AD777x_CHAN_FILTER_S(7),
683 };
684
> 685 const struct iio_buffer_setup_ops ad777x_buffer_setup_ops = {
686 .postenable = ad777x_buffer_postenable,
687 .predisable = ad777x_buffer_predisable,
688 };
689
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Wed, 29 May 2024 18:03:09 +0300
ranechita <[email protected]> wrote:
> Added support for ad7770,ad7771,ad7779 ADCs. The
> data is streamed only on the spi-mode, without
> using the data lines.
>
> Signed-off-by: ranechita <[email protected]>
Others have commented on need to sort your patch submissions out.
Make sure that's fixed for next version. 1 series with driver
and bindings, fixed sign off etc.
Various comments inline.
Jonathan
> ---
> drivers/iio/adc/Kconfig | 11 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ad7779.c | 951 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 963 insertions(+)
> create mode 100644 drivers/iio/adc/ad7779.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 0d9282fa67f5..3e42cbc365d7 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -206,6 +206,17 @@ config AD7768_1
> To compile this driver as a module, choose M here: the module will be
> called ad7768-1.
>
> +config AD7779
> + tristate "Analog Devices AD7779 ADC driver"
> + depends on SPI
> + select IIO_BUFFER
> + help
> + Say yes here to build support for Analog Devices AD7779 SPI
In help text list all supported parts so that people can grep for them.
> + analog to digital converter (ADC)
It's not just an SPI converter. Seems to have a 4 wide serial interface
for directly clocking out the data as well. Might be worth mentioning that
even if the driver doesn't yet support it.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called ad7779.
> +
> diff --git a/drivers/iio/adc/ad7779.c b/drivers/iio/adc/ad7779.c
> new file mode 100644
> index 000000000000..089e352e2d40
> --- /dev/null
> +++ b/drivers/iio/adc/ad7779.c
> @@ -0,0 +1,951 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * AD777X ADC
> + *
> + * Copyright 2023 Analog Devices Inc.
Probably worth updating given how much this is changing!
> +#define AD777X_CRC8_POLY 0x07
> +DECLARE_CRC8_TABLE(ad777x_crc8_table);
> +
> +enum ad777x_filter {
Don't use wild cards for defines. Just name it after a suitable specific
part number. Wild cards go wrong far too often.
> + AD777X_SINC3,
> + AD777X_SINC5,
> +};
> +
> +enum ad777x_variant {
> + ad7770,
> + ad7771,
> + ad7779,
> +};
> +
> +enum ad777x_power_mode {
> + AD777X_LOW_POWER,
> + AD777X_HIGH_POWER,
> +};
> +
> +struct ad777x_chip_info {
> + const char *name;
> + struct iio_chan_spec const *channels;
> +};
> +
> +struct ad777x_state {
Choose a supported part and name after that. Wild cards go
wrong far too often because manufacturers love to put incompatible
and sometimes completely unrelated parts numbers between those used
for other devices.
> + struct spi_device *spi;
> + const struct ad777x_chip_info *chip_info;
> + struct clk *mclk;
> + struct regulator *vref;
> + unsigned int sampling_freq;
> + enum ad777x_power_mode power_mode;
> + enum ad777x_filter filter_enabled;
> + unsigned int active_ch;
> + unsigned int spidata_mode;
> + unsigned int crc_enabled;
> +
> + /*
> + * DMA (thus cache coherency maintenance) requires the
> + * transfer buffers to live in their own cache lines.
> + */
> + u8 reg_rx_buf[3] ____cacheline_aligned;
Comment is correct, but that alignment isn't. Unfortunately
____cacheline_aligned is (on a few platforms) not sufficient as
it is the l1 cacheline size and DMA is done from last level cache
which has a larger cacheline.
use __aligned(IIO_DMA_MINALIGN) which handles this corner case.
> + u8 reg_tx_buf[3];
> + __be32 spidata_rx[8];
> + __be32 spidata_tx[8];
> + u8 reset_buf[8];
> +};
> +
> +static const char * const ad777x_filter_type[] = {
> + [AD777X_SINC3] = "sinc3_filter",
> + [AD777X_SINC5] = "sinc5_filter",
> +};
> +
> +static int ad777x_spi_read(struct ad777x_state *st, u8 reg, u8 *rbuf)
> +{
> + int ret;
> + int length = 2;
> + u8 crc_buf[2];
> + u8 exp_crc = 0;
> + struct spi_transfer reg_read_tr[] = {
> + {
> + .tx_buf = st->reg_tx_buf,
> + .rx_buf = st->reg_rx_buf,
> + },
> + };
> +
> + if (st->crc_enabled)
> + length = 3;
> + reg_read_tr[0].len = length;
> +
> + st->reg_tx_buf[0] = AD777X_SPI_READ_CMD | (reg & 0x7F);
> + st->reg_tx_buf[1] = 0;
> + st->reg_tx_buf[2] = crc8(ad777x_crc8_table, st->reg_tx_buf, 2, 0);
> +
> + ret = spi_sync_transfer(st->spi, reg_read_tr, ARRAY_SIZE(reg_read_tr));
> + if (ret)
> + return ret;
> +
> + crc_buf[0] = AD777X_SPI_READ_CMD | FIELD_GET(AD777X_REG_READ_MSK, reg);
> + crc_buf[1] = st->reg_rx_buf[1];
> + exp_crc = crc8(ad777x_crc8_table, crc_buf, 2, 0);
> + if (st->crc_enabled && exp_crc != st->reg_rx_buf[2]) {
> + dev_err(&st->spi->dev, "Bad CRC %x, expected %x",
> + st->reg_rx_buf[2], exp_crc);
> + return -EINVAL;
> + }
> + *rbuf = st->reg_rx_buf[1];
> +
> + return 0;
> +}
> +
> +static int ad777x_spi_write(struct ad777x_state *st, u8 reg, u8 val)
> +{
> + int length = 2;
> + struct spi_transfer reg_write_tr[] = {
> + {
> + .tx_buf = st->reg_tx_buf,
> + },
> + };
> +
> + if (st->crc_enabled)
> + length = 3;
> + reg_write_tr[0].len = length;
> +
> + st->reg_tx_buf[0] = reg & 0x7F;
> + st->reg_tx_buf[1] = val;
> + st->reg_tx_buf[2] = crc8(ad777x_crc8_table, st->reg_tx_buf, 2, 0);
only fill that in if crc_enabled is set.
> +
> + return spi_sync_transfer(st->spi, reg_write_tr, ARRAY_SIZE(reg_write_tr));
> +}
> +
> +static int ad777x_spi_write_mask(struct ad777x_state *st, u8 reg, u8 mask,
> + u8 val)
> +{
> + int ret;
> + u8 regval, data;
> +
> + ret = ad777x_spi_read(st, reg, &data);
When I see this sort of helper, it's usually a good sign that the author
should consider a custom regmap. I'm not 100% sure it is a good fit here
but it seems likely looking at this section of code.
> + if (ret)
> + return ret;
> +
> + regval = data;
> + regval &= ~mask;
> + regval |= val;
> +
> + if (regval == data)
> + return 0;
> +
> + return ad777x_spi_write(st, reg, regval);
> +
> +}
> +static int ad777x_set_sampling_frequency(struct ad777x_state *st,
> + unsigned int sampling_freq)
> +{
> + int ret;
> + unsigned int dec;
> + unsigned int div;
> + unsigned int decimal;
> + int temp;
> + unsigned int kfreq;
> + u8 msb, lsb;
> +
> + if (st->filter_enabled == AD777X_SINC3 &&
> + sampling_freq > AD777X_SINC3_MAXFREQ)
> + return -EINVAL;
> +
> + if (st->filter_enabled == AD777X_SINC5 &&
> + sampling_freq > AD777X_SINC5_MAXFREQ)
Align after ( as done on the one above.
> + return -EINVAL;
> +
> + if (st->spidata_mode == 1 &&
> + sampling_freq > AD777X_SPIMODE_MAX_SAMP_FREQ)
> + return -EINVAL;
> +
> + if (st->power_mode == AD777X_LOW_POWER)
> + div = AD777X_LOWPOWER_DIV;
> + else
> + div = AD777X_HIGHPOWER_DIV;
> +
> + kfreq = sampling_freq / KILO;
> + dec = div / kfreq;
> +
> + lsb = FIELD_GET(AD777X_FREQ_LSB_MSK, dec);
> + msb = FIELD_GET(AD777X_FREQ_MSB_MSK, dec);
These local variables don't add much. Just use the
FIELD_GET() calls in appropriate places.
> +
> + ret = ad777x_spi_write(st, AD777X_REG_SRC_N_LSB, lsb);
> + if (ret)
> + return ret;
> + ret = ad777x_spi_write(st, AD777X_REG_SRC_N_MSB, msb);
> + if (ret)
> + return ret;
> +
> + if (div % kfreq) {
> + temp = (div * KILO) / kfreq;
> + decimal = ((temp - dec * KILO) << 16) / KILO;
> + lsb = FIELD_GET(AD777X_FREQ_LSB_MSK, decimal);
> + msb = FIELD_GET(AD777X_FREQ_MSB_MSK, decimal);
> +
> + ret = ad777x_spi_write(st, AD777X_REG_SRC_IF_LSB, lsb);
> + if (ret)
> + return ret;
> + ret = ad777x_spi_write(st, AD777X_REG_SRC_IF_MSB, msb);
> + if (ret)
> + return ret;
> + } else {
> + ret = ad777x_spi_write(st, AD777X_REG_SRC_IF_LSB, 0x0);
> + if (ret)
> + return ret;
> + ret = ad777x_spi_write(st, AD777X_REG_SRC_IF_MSB, 0x0);
> + if (ret)
> + return ret;
> + }
> + ret = ad777x_spi_write(st, AD777X_REG_SRC_UPDATE, 0x1);
> + if (ret)
> + return ret;
> + fsleep(15);
> + ret = ad777x_spi_write(st, AD777X_REG_SRC_UPDATE, 0x0);
> + if (ret)
> + return ret;
> + fsleep(15);
> +
> + st->sampling_freq = sampling_freq;
> +
> + return 0;
> +}
...
> +static int ad777x_set_calibscale(struct ad777x_state *st, int channel, int val)
> +{
> + int ret;
> + u8 msb, mid, lsb;
> + unsigned int gain;
> + unsigned long long tmp;
> +
> + tmp = val * 5592405LL;
> + gain = DIV_ROUND_CLOSEST_ULL(tmp, MEGA);
> + msb = FIELD_GET(AD777X_UPPER, gain);
> + mid = FIELD_GET(AD777X_MID, gain);
> + lsb = FIELD_GET(AD777X_LOWER, gain);
> + ret = ad777x_spi_write(st,
> + AD777X_REG_CH_GAIN_UPPER_BYTE(channel),
> + msb);
> + if (ret)
> + return ret;
> + ret = ad777x_spi_write(st,
> + AD777X_REG_CH_GAIN_MID_BYTE(channel),
> + mid);
> + if (ret)
> + return ret;
> + return ad777x_spi_write(st,
> + AD777X_REG_CH_GAIN_LOWER_BYTE(channel),
> + lsb);
I assume these regisers are next to each other. If so I think Andy suggested
creating your own bulk read /write. That would be a good cleanup.
> +}
> +
> +static int ad777x_get_calibbias(struct ad777x_state *st, int channel)
> +{
> + int ret;
> + u8 low, mid, high;
> +
> + ret = ad777x_spi_read(st, AD777X_REG_CH_OFFSET_LOWER_BYTE(channel),
> + &low);
> + if (ret)
> + return ret;
> + ret = ad777x_spi_read(st, AD777X_REG_CH_OFFSET_MID_BYTE(channel), &mid);
> + if (ret)
> + return ret;
> + ret = ad777x_spi_read(st,
> + AD777X_REG_CH_OFFSET_UPPER_BYTE(channel),
> + &high);
> + if (ret)
> + return ret;
> +
> + return FIELD_PREP(AD777X_UPPER, high) | FIELD_PREP(AD777X_MID, mid) |
> + FIELD_PREP(AD777X_LOWER, low);
get them directly into different bytes of a byte array then use a
get_unaligned_be24() call here to build this.
> +}
> +
> +static int ad777x_set_calibbias(struct ad777x_state *st, int channel, int val)
> +{
> + int ret;
> + u8 msb, mid, lsb;
> +
> + msb = FIELD_GET(AD777X_UPPER, val);
> + mid = FIELD_GET(AD777X_MID, val);
> + lsb = FIELD_GET(AD777X_LOWER, val);
> + ret = ad777x_spi_write(st,
> + AD777X_REG_CH_OFFSET_UPPER_BYTE(channel),
> + msb);
Put the FIELD_GET() inline. Doing as above doesn't h elp mcuh.
> + if (ret)
> + return ret;
As below blank lines in appropriate locations to separate the blocks of code.
> + ret = ad777x_spi_write(st,
> + AD777X_REG_CH_OFFSET_MID_BYTE(channel),
> + mid);
> + if (ret)
> + return ret;
> + return ad777x_spi_write(st,
> + AD777X_REG_CH_OFFSET_LOWER_BYTE(channel),
> + lsb);
> +}
> +
> +static int ad777x_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val,
> + int *val2,
> + long mask)
> +{
> + struct ad777x_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + ret = iio_device_claim_direct_mode(indio_dev);
Use the scoped version to simplify this quite a bit.
> + if (ret)
> + return ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_CALIBSCALE:
> + *val = ad777x_get_calibscale(st, chan->channel);
> + iio_device_release_direct_mode(indio_dev);
> + if (ret)
ret isn't set by anyone...
> + return ret;
> + *val2 = GAIN_REL;
> + return IIO_VAL_FRACTIONAL;
> + case IIO_CHAN_INFO_CALIBBIAS:
> + *val = ad777x_get_calibbias(st, chan->channel);
> + iio_device_release_direct_mode(indio_dev);
> + if (ret)
as above.
> + return ret;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *val = st->sampling_freq;
> + iio_device_release_direct_mode(indio_dev);
> + if (ret)
and here.
> + return ret;
> + return IIO_VAL_INT;
> + }
> +
> + iio_device_release_direct_mode(indio_dev);
> +
> + return -EINVAL;
> +}
> +
> +
> +static int ad777x_update_scan_mode(struct iio_dev *indio_dev,
> + const unsigned long *scan_mask)
> +{
> + struct ad777x_state *st = iio_priv(indio_dev);
> +
> + bitmap_copy((unsigned long *)&st->active_ch, scan_mask, AD777X_NUM_CHANNELS);
Why have your own local tracking? Just use the active_scan_mask directly.
Then this function can go away.
> +
> + return 0;
> +}
> +
> +static int ad777x_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + int ret;
> + struct ad777x_state *st = iio_priv(indio_dev);
> +
> + disable_irq_nosync(st->spi->irq);
I suspect to close the race of you thinking you are in buffered mode when
you aren't that the nosync variant isn't the right choice here.
> + ret = ad777x_spi_write(st,
> + AD777X_REG_GENERAL_USER_CONFIG_3,
> + AD777X_DISABLE_SD);
> + return ret;
return ad777x ..
> +}
> +
> +static irqreturn_t ad777x_irq_handler(int irq, void *data)
> +{
> + struct iio_dev *indio_dev = data;
> + struct ad777x_state *st = iio_priv(indio_dev);
> + int ret;
> + __be32 tmp[8];
> + int i;
> + int k = 0;
> +
> + struct spi_transfer sd_readback_tr[] = {
> + {
> + .rx_buf = st->spidata_rx,
> + .tx_buf = st->spidata_tx,
> + .len = 32,
> + }
> + };
> +
> + if (iio_buffer_enabled(indio_dev)) {
How do we get here without that being true? Add a comment given if we did I'd
expect to see an alternative set of things to do in here. Also invert condition
to reduce indent.
if (!iio_buffer_enabled(indio_dev))
return IRQ_HANDLED;
st->...
> + st->spidata_tx[0] = AD777X_SPI_READ_CMD;
> + ret = spi_sync_transfer(st->spi, sd_readback_tr,
> + ARRAY_SIZE(sd_readback_tr));
> + if (ret) {
> + dev_err(&st->spi->dev,
> + "spi transfer error in irq handler");
> + return IRQ_HANDLED;
> + }
> + for (i = 0; i < AD777X_NUM_CHANNELS; i++) {
> + if (st->active_ch & BIT(i))
> + tmp[k++] = __be32_to_cpu(st->spidata_rx[i]);
Why? We generally leave data reordering to userspace. Just report as a be32
channel if that's what it is.
> + }
> + iio_push_to_buffers(indio_dev, &tmp[0]);
Not obvious why you can't provide a timestamp given this is in the interrupt
handler for that capture completing (no fifo or similar to make that complex).
You will need to expand tmp though to allow for the timestamp to be inserted.
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int ad777x_reset(struct iio_dev *indio_dev, struct gpio_desc *reset_gpio)
> +{
> + struct ad777x_state *st = iio_priv(indio_dev);
> + int ret;
> + struct spi_transfer reg_read_tr[] = {
> + {
> + .tx_buf = st->reset_buf,
> + .len = 8,
> + },
> + };
> +
> + memset(st->reset_buf, 0xff, sizeof(st->reset_buf));
> +
> + if (reset_gpio) {
> + gpiod_set_value(reset_gpio, 1);
> + fsleep(230);
> + return 0;
> + }
> +
> + ret = spi_sync_transfer(st->spi, reg_read_tr,
> + ARRAY_SIZE(reg_read_tr));
> + if (ret)
> + return ret;
> +
> + fsleep(230);
Add a spec reference for these sleep times.
> +
> + return 0;
> +}
> +static int ad777x_register(struct ad777x_state *st, struct iio_dev *indio_dev)
> +{
There is no obvious reason to break this out from probe. Just put the
code inline. There may be reasons to break out some parts like the
irq setup, but currently the break doesn't help with readability.
> + int ret;
> + struct device *dev = &st->spi->dev;
> +
> + indio_dev->name = st->chip_info->name;
> + indio_dev->info = &ad777x_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = st->chip_info->channels;
> + indio_dev->num_channels = ARRAY_SIZE(ad777x_channels);
> +
> + ret = devm_request_threaded_irq(dev, st->spi->irq, NULL,
> + ad777x_irq_handler, IRQF_ONESHOT,
> + indio_dev->name, indio_dev);
> + if (ret)
> + return dev_err_probe(dev, ret, "request irq %d failed\n",
> + st->spi->irq);
> +
> + ret = devm_iio_kfifo_buffer_setup_ext(dev, indio_dev,
> + &ad777x_buffer_setup_ops,
> + NULL);
Does this device have a fifo or similar reason for directly managing
the buffer rather than providing a trigger?
So far I'm not seeing any code to indicate the need for not using
the more common approach of a data ready trigger and a pollfunc
etc to actually grab the data.
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "setup buffer failed\n");
> +
> + ret = ad777x_spi_write_mask(st, AD777X_REG_DOUT_FORMAT,
> + AD777X_DCLK_CLK_DIV_MSK,
> + FIELD_PREP(AD777X_DCLK_CLK_DIV_MSK, 7));
> + if (ret)
> + return ret;
> + st->spidata_mode = 1;
Always seems to be set when it might be queried. As such feels like a feature
you haven't implemented yet? If so don't have the code here.
> +
> + disable_irq_nosync(st->spi->irq);
Look at IRQF_NO_AUTOEN rather than turning it on then off again.
> +
> + return devm_iio_device_register(&st->spi->dev, indio_dev);
> +}
> +
> +static int ad777x_powerup(struct ad777x_state *st, struct gpio_desc *start_gpio)
> +{
> + int ret;
> +
> + ret = ad777x_spi_write_mask(st, AD777X_REG_GENERAL_USER_CONFIG_1,
> + AD777X_MOD_POWERMODE_MSK,
> + FIELD_PREP(AD777X_MOD_POWERMODE_MSK, 1));
> + if (ret)
> + return ret;
blank line here.
> + ret = ad777x_spi_write_mask(st, AD777X_REG_GENERAL_USER_CONFIG_1,
> + AD777X_MOD_PDB_REFOUT_MSK,
> + FIELD_PREP(AD777X_MOD_PDB_REFOUT_MSK, 1));
> + if (ret)
> + return ret;
and here etc. Basically separate every call + error handler block.
That helps readability a little.
> + ret = ad777x_spi_write_mask(st, AD777X_REG_DOUT_FORMAT,
> + AD777X_DCLK_CLK_DIV_MSK,
> + FIELD_PREP(AD777X_DCLK_CLK_DIV_MSK, 1));
> + if (ret)
> + return ret;
> + ret = ad777x_spi_write_mask(st, AD777X_REG_ADC_MUX_CONFIG,
> + AD777X_REFMUX_CTRL_MSK,
> + FIELD_PREP(AD777X_REFMUX_CTRL_MSK, 1));
> + if (ret)
> + return ret;
> + ret = ad777x_spi_write_mask(st, AD777X_REG_GEN_ERR_REG_1_EN,
> + AD777X_SPI_CRC_EN_MSK,
> + FIELD_PREP(AD777X_SPI_CRC_EN_MSK, 1));
> + if (ret)
> + return ret;
> +
> + st->power_mode = AD777X_HIGH_POWER;
> + st->crc_enabled = true;
I'd be tempted to enable crc earlier and open code the spi write for that
instead of using helpers. That way you can assume it is always on and
simplify the code.
No one ever wants to disable CRC on a chip that has it. It's rare enough
that people only fit such chips if they want that feature.
If there are reasons it can't be done earlier such as need to be
in particular power states or similar add a comment
> + ret = ad777x_set_sampling_frequency(st, AD777X_DEFAULT_SAMPLING_FREQ);
> + if (ret)
> + return ret;
> +
> + gpiod_set_value(start_gpio, 0);
> + fsleep(15);
> + gpiod_set_value(start_gpio, 1);
> + fsleep(15);
> + gpiod_set_value(start_gpio, 0);
> + fsleep(15);
> +
> + return 0;
> +}
> +
> +static int ad777x_probe(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev;
> + struct ad777x_state *st;
> + struct gpio_desc *reset_gpio;
> + struct gpio_desc *start_gpio;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> +
> + st->vref = devm_regulator_get_optional(&spi->dev, "vref");
> + if (IS_ERR(st->vref))
> + return PTR_ERR(st->vref);
> +
> + ret = regulator_enable(st->vref);
> + if (ret)
> + return ret;
> +
> + ret = devm_add_action_or_reset(&spi->dev, ad777x_reg_disable,
> + st->vref);
> + if (ret)
> + return ret;
I'm surprised not to see the voltage on vref being queried. I would think
the new
devm_regulator_get_enable_read_voltage()
may be appropriate.
Why is it optional? That can make sense if there is an internal
regulator but you aren't doing appropriate handling for that.
> +
> + st->mclk = devm_clk_get(&spi->dev, "mclk");
> + if (IS_ERR(st->mclk))
> + return PTR_ERR(st->mclk);
> +
> + ret = clk_prepare_enable(st->mclk);
> + if (ret < 0)
> + return ret;
> +
> + ret = devm_add_action_or_reset(&spi->dev, ad777x_clk_disable,
> + st->mclk);
> + if (ret)
> + return ret;
As Andy pointed out, there are helpers for these sequences of code.
> +
> + reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(reset_gpio))
> + return PTR_ERR(reset_gpio);
> +
> + start_gpio = devm_gpiod_get(&spi->dev, "start", GPIOD_OUT_HIGH);
> + if (IS_ERR(start_gpio))
> + return PTR_ERR(start_gpio);
> +
> + crc8_populate_msb(ad777x_crc8_table, AD777X_CRC8_POLY);
> + st->spi = spi;
> +
> + st->chip_info = spi_get_device_match_data(spi);
> + if (!st->chip_info)
> + return -ENODEV;
> +
> + ret = ad777x_reset(indio_dev, start_gpio);
> + if (ret)
> + return ret;
> +
> + ad777x_powerup(st, start_gpio);
> + if (ret)
> + return ret;
> +
> + if (spi->irq)
Why? If the device is only registered if the irq is present then
check that earlier and error out earlier.
Right now I think that a missing irq means the driver probe succeeds
but no user interfaces are provided. That doesn't make much sense.
> + ret = ad777x_register(st, indio_dev);
> +
> + return ret;
> +}
> +
> +static int __maybe_unused ad777x_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct ad777x_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + ret = ad777x_spi_write_mask(st, AD777X_REG_GENERAL_USER_CONFIG_1,
> + AD777X_MOD_POWERMODE_MSK,
> + FIELD_PREP(AD777X_MOD_POWERMODE_MSK,
> + AD777X_LOW_POWER));
> + if (ret)
> + return ret;
> +
> + st->power_mode = AD777X_LOW_POWER;
This is never queried - so don't store this info until you
add code that needs to know the current state and for some reason
can't just read it from the register.
> + return 0;
> +}
> +