Add support for AD7770, AD7771, AD7779 ADCs. The device is capable of
sending out data both on DOUT lines interface,as on the SDO line.
The driver currently implements only theSDO data streaming mode. SPI
communication is used alternatively foraccessingregisters and streaming
data. Register access are protected by crc8.
Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@analog.com>
---
drivers/iio/adc/Kconfig | 11 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad7779.c | 952 +++++++++++++++++++++++++++++++++++++++
3 files changed, 964 insertions(+)
create mode 100644 drivers/iio/adc/ad7779.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 0d9282fa67f5..05d3719c2458 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 AD777X family
+ (AD7770, AD7771, AD7779) 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..7a83977fd00c
--- /dev/null
+++ b/drivers/iio/adc/ad7779.c
@@ -0,0 +1,952 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD7770, AD7771, AD7779 ADC
+ *
+ * Copyright 2023-2024 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/mod_devicetable.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/string.h>
+#include <linux/units.h>
+
+#include <asm/unaligned.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+
+#define AD7779_SPI_READ_CMD BIT(7)
+
+#define AD7779_DISABLE_SD BIT(7)
+
+#define AD7779_REG_CH_DISABLE 0x08
+#define AD7779_REG_CH_SYNC_OFFSET(ch) (0x09 + (ch))
+#define AD7779_REG_CH_CONFIG(ch) (0x00 + (ch))
+#define AD7779_REG_GENERAL_USER_CONFIG_1 0x11
+#define AD7779_REG_GENERAL_USER_CONFIG_2 0x12
+#define AD7779_REG_GENERAL_USER_CONFIG_3 0x13
+#define AD7779_REG_DOUT_FORMAT 0x14
+#define AD7779_REG_ADC_MUX_CONFIG 0x15
+#define AD7779_REG_GPIO_CONFIG 0x17
+#define AD7779_REG_BUFFER_CONFIG_1 0x19
+#define AD7779_REG_GLOBAL_MUX_CONFIG 0x16
+#define AD7779_REG_BUFFER_CONFIG_2 0x1A
+#define AD7779_REG_GPIO_DATA 0x18
+#define AD7779_REG_CH_OFFSET_UPPER_BYTE(ch) (0x1C + (ch) * 6)
+#define AD7779_REG_CH_OFFSET_LOWER_BYTE(ch) (0x1E + (ch) * 6)
+#define AD7779_REG_CH_GAIN_UPPER_BYTE(ch) (0x1F + (ch) * 6)
+#define AD7779_REG_CH_OFFSET_MID_BYTE(ch) (0x1D + (ch) * 6)
+#define AD7779_REG_CH_GAIN_MID_BYTE(ch) (0x20 + (ch) * 6)
+#define AD7779_REG_CH_ERR_REG(ch) (0x4C + (ch))
+#define AD7779_REG_CH0_1_SAT_ERR 0x54
+#define AD7779_REG_CH_GAIN_LOWER_BYTE(ch) (0x21 + (ch) * 6)
+#define AD7779_REG_CH2_3_SAT_ERR 0x55
+#define AD7779_REG_CH4_5_SAT_ERR 0x56
+#define AD7779_REG_CH6_7_SAT_ERR 0x57
+#define AD7779_REG_CHX_ERR_REG_EN 0x58
+#define AD7779_REG_GEN_ERR_REG_1 0x59
+#define AD7779_REG_GEN_ERR_REG_1_EN 0x5A
+#define AD7779_REG_GEN_ERR_REG_2 0x5B
+#define AD7779_REG_GEN_ERR_REG_2_EN 0x5C
+#define AD7779_REG_STATUS_REG_1 0x5D
+#define AD7779_REG_STATUS_REG_2 0x5E
+#define AD7779_REG_STATUS_REG_3 0x5F
+#define AD7779_REG_SRC_N_MSB 0x60
+#define AD7779_REG_SRC_N_LSB 0x61
+#define AD7779_REG_SRC_IF_MSB 0x62
+#define AD7779_REG_SRC_IF_LSB 0x63
+#define AD7779_REG_SRC_UPDATE 0x64
+
+#define AD7779_FILTER_MSK BIT(6)
+#define AD7779_MOD_POWERMODE_MSK BIT(6)
+#define AD7779_MOD_PDB_REFOUT_MSK BIT(4)
+#define AD7779_MOD_SPI_EN_MSK BIT(4)
+
+/* AD7779_REG_DOUT_FORMAT */
+#define AD7779_DOUT_FORMAT_MSK GENMASK(7, 6)
+#define AD7779_DOUT_HEADER_FORMAT BIT(5)
+#define AD7779_DCLK_CLK_DIV_MSK GENMASK(3, 1)
+
+#define AD7779_REFMUX_CTRL_MSK GENMASK(7, 6)
+#define AD7779_SPI_CRC_EN_MSK BIT(0)
+
+#define AD7779_MAXCLK_LOWPOWER 4096000
+#define AD7779_NUM_CHANNELS 8
+#define AD7779_RESET_BUF_SIZE 8
+
+#define AD7779_LOWPOWER_DIV 512
+#define AD7779_HIGHPOWER_DIV 2048
+
+#define AD7779_SINC3_MAXFREQ (16 * HZ_PER_KHZ)
+#define AD7779_SINC5_MAXFREQ (128 * HZ_PER_KHZ)
+
+#define AD7779_DEFAULT_SAMPLING_FREQ (8 * HZ_PER_KHZ)
+#define AD7779_DEFAULT_SAMPLING_2LINE (4 * HZ_PER_KHZ)
+#define AD7779_DEFAULT_SAMPLING_1LINE (2 * HZ_PER_KHZ)
+
+#define AD7779_SPIMODE_MAX_SAMP_FREQ (16 * HZ_PER_KHZ)
+
+#define GAIN_REL 0x555555
+#define AD7779_FREQ_MSB_MSK GENMASK(15, 8)
+#define AD7779_FREQ_LSB_MSK GENMASK(7, 0)
+#define AD7779_UPPER GENMASK(23, 16)
+#define AD7779_MID GENMASK(15, 8)
+#define AD7779_LOWER GENMASK(7, 0)
+
+#define AD7779_REG_MSK GENMASK(6, 0)
+
+#define AD7779_CRC8_POLY 0x07
+DECLARE_CRC8_TABLE(ad7779_crc8_table);
+
+enum ad7779_filter {
+ AD7779_SINC3,
+ AD7779_SINC5,
+};
+
+enum ad7779_variant {
+ ad7770,
+ ad7771,
+ AD7779,
+};
+
+enum ad7779_power_mode {
+ AD7779_LOW_POWER,
+ AD7779_HIGH_POWER,
+};
+
+struct ad7779_chip_info {
+ const char *name;
+ struct iio_chan_spec const *channels;
+};
+
+struct ad7779_state {
+ struct spi_device *spi;
+ const struct ad7779_chip_info *chip_info;
+ struct clk *mclk;
+ struct iio_trigger *trig;
+ struct completion completion;
+ unsigned int sampling_freq;
+ enum ad7779_power_mode power_mode;
+ enum ad7779_filter filter_enabled;
+ /*
+ * DMA (thus cache coherency maintenance) requires the
+ * transfer buffers to live in their own cache lines.
+ */
+ u8 reg_rx_buf[3] __aligned(IIO_DMA_MINALIGN);
+ u8 reg_tx_buf[3];
+ u32 spidata_rx[8];
+ u32 spidata_tx[8];
+ u8 reset_buf[8];
+};
+
+static const char * const ad7779_filter_type[] = {
+ [AD7779_SINC3] = "sinc3",
+ [AD7779_SINC5] = "sinc5",
+};
+
+static int ad7779_spi_read(struct ad7779_state *st, u8 reg, u8 *rbuf)
+{
+ int ret;
+ int length = 3;
+ 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 (reg == AD7779_REG_GEN_ERR_REG_1_EN)
+ length = 2;
+ reg_read_tr[0].len = length;
+
+ st->reg_tx_buf[0] = AD7779_SPI_READ_CMD | FIELD_GET(AD7779_REG_MSK, reg);
+ st->reg_tx_buf[1] = 0;
+ st->reg_tx_buf[2] = crc8(ad7779_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] = AD7779_SPI_READ_CMD | FIELD_GET(AD7779_REG_MSK, reg);
+ crc_buf[1] = st->reg_rx_buf[1];
+ exp_crc = crc8(ad7779_crc8_table, crc_buf, 2, 0);
+ if (reg != AD7779_REG_GEN_ERR_REG_1_EN && 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 ad7779_spi_write(struct ad7779_state *st, u8 reg, u8 val)
+{
+ int length = 3;
+ struct spi_transfer reg_write_tr[] = {
+ {
+ .tx_buf = st->reg_tx_buf,
+ },
+ };
+
+ if (reg == AD7779_REG_GEN_ERR_REG_1_EN)
+ length = 2;
+ reg_write_tr[0].len = length;
+
+ st->reg_tx_buf[0] = FIELD_GET(AD7779_REG_MSK, reg);
+ st->reg_tx_buf[1] = val;
+ st->reg_tx_buf[2] = crc8(ad7779_crc8_table, st->reg_tx_buf, 2, 0);
+
+ return spi_sync_transfer(st->spi, reg_write_tr, ARRAY_SIZE(reg_write_tr));
+}
+
+static int ad7779_spi_write_mask(struct ad7779_state *st, u8 reg, u8 mask,
+ u8 val)
+{
+ int ret;
+ u8 regval, data;
+
+ ret = ad7779_spi_read(st, reg, &data);
+ if (ret)
+ return ret;
+
+ regval = data;
+ regval &= ~mask;
+ regval |= val;
+
+ if (regval == data)
+ return 0;
+
+ return ad7779_spi_write(st, reg, regval);
+}
+
+static int ad7779_reg_access(struct iio_dev *indio_dev,
+ unsigned int reg,
+ unsigned int writeval,
+ unsigned int *readval)
+{
+ struct ad7779_state *st = iio_priv(indio_dev);
+
+ if (readval)
+ return ad7779_spi_read(st, reg, (u8 *)readval);
+
+ return ad7779_spi_write(st, reg, writeval);
+}
+
+static int ad7779_set_sampling_frequency(struct ad7779_state *st,
+ unsigned int sampling_freq)
+{
+ int ret;
+ unsigned int dec;
+ unsigned int div;
+ unsigned int decimal;
+ int temp;
+ unsigned int kfreq;
+
+ if (st->filter_enabled == AD7779_SINC3 &&
+ sampling_freq > AD7779_SINC3_MAXFREQ)
+ return -EINVAL;
+
+ if (st->filter_enabled == AD7779_SINC5 &&
+ sampling_freq > AD7779_SINC5_MAXFREQ)
+ return -EINVAL;
+
+ if (sampling_freq > AD7779_SPIMODE_MAX_SAMP_FREQ)
+ return -EINVAL;
+
+ if (st->power_mode == AD7779_LOW_POWER)
+ div = AD7779_LOWPOWER_DIV;
+ else
+ div = AD7779_HIGHPOWER_DIV;
+
+ kfreq = sampling_freq / KILO;
+ dec = div / kfreq;
+
+ ret = ad7779_spi_write(st, AD7779_REG_SRC_N_MSB,
+ FIELD_GET(AD7779_FREQ_MSB_MSK, dec));
+ if (ret)
+ return ret;
+ ret = ad7779_spi_write(st, AD7779_REG_SRC_N_LSB,
+ FIELD_GET(AD7779_FREQ_LSB_MSK, dec));
+ if (ret)
+ return ret;
+
+ if (div % kfreq) {
+ temp = (div * KILO) / kfreq;
+ decimal = ((temp - dec * KILO) << 16) / KILO;
+ ret = ad7779_spi_write(st, AD7779_REG_SRC_N_MSB,
+ FIELD_GET(AD7779_FREQ_MSB_MSK, decimal));
+ if (ret)
+ return ret;
+ ret = ad7779_spi_write(st, AD7779_REG_SRC_N_LSB,
+ FIELD_GET(AD7779_FREQ_LSB_MSK, decimal));
+ if (ret)
+ return ret;
+ } else {
+ ret = ad7779_spi_write(st, AD7779_REG_SRC_N_MSB,
+ FIELD_GET(AD7779_FREQ_MSB_MSK, 0x0));
+ if (ret)
+ return ret;
+ ret = ad7779_spi_write(st, AD7779_REG_SRC_N_LSB,
+ FIELD_GET(AD7779_FREQ_LSB_MSK, 0x0));
+ if (ret)
+ return ret;
+ }
+ ret = ad7779_spi_write(st, AD7779_REG_SRC_UPDATE, 0x1);
+ if (ret)
+ return ret;
+
+ /* SRC update settling time */
+ fsleep(15);
+ ret = ad7779_spi_write(st, AD7779_REG_SRC_UPDATE, 0x0);
+ if (ret)
+ return ret;
+
+ /* SRC update settling time */
+ fsleep(15);
+
+ st->sampling_freq = sampling_freq;
+
+ return 0;
+}
+
+static int ad7779_get_filter(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan)
+{
+ struct ad7779_state *st = iio_priv(indio_dev);
+ u8 temp;
+ int ret;
+
+ ret = ad7779_spi_read(st, AD7779_REG_GENERAL_USER_CONFIG_2, &temp);
+ if (ret)
+ return ret;
+
+ return FIELD_GET(AD7779_FILTER_MSK, temp);
+}
+
+static int ad7779_set_filter(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ unsigned int mode)
+{
+ struct ad7779_state *st = iio_priv(indio_dev);
+ int ret;
+
+ ret = ad7779_spi_write_mask(st,
+ AD7779_REG_GENERAL_USER_CONFIG_2,
+ AD7779_FILTER_MSK,
+ FIELD_PREP(AD7779_FILTER_MSK, mode));
+ if (ret < 0)
+ return ret;
+
+ ret = ad7779_set_sampling_frequency(st, st->sampling_freq);
+ if (ret < 0)
+ return ret;
+
+ st->filter_enabled = mode;
+
+ return 0;
+}
+
+static int ad7779_get_calibscale(struct ad7779_state *st, int channel)
+{
+ int ret;
+ u8 calibscale[3];
+ // u8 low, mid, high;
+ ret = ad7779_spi_read(st, AD7779_REG_CH_GAIN_LOWER_BYTE(channel), &calibscale[0]);
+ if (ret)
+ return ret;
+
+ ret = ad7779_spi_read(st, AD7779_REG_CH_GAIN_MID_BYTE(channel), &calibscale[1]);
+ if (ret)
+ return ret;
+
+ ret = ad7779_spi_read(st, AD7779_REG_CH_GAIN_UPPER_BYTE(channel), &calibscale[2]);
+ if (ret)
+ return ret;
+
+ // return FIELD_PREP(AD7779_UPPER, high) | FIELD_PREP(AD7779_MID, mid) |
+ // FIELD_PREP(AD7779_LOWER, low);
+ return get_unaligned_be24(calibscale);
+}
+
+static int ad7779_set_calibscale(struct ad7779_state *st, int channel, int val)
+{
+ int ret;
+ unsigned int gain;
+ unsigned long long tmp;
+ u8 gain_bytes[3];
+
+ tmp = val * 5592405LL;
+ gain = DIV_ROUND_CLOSEST_ULL(tmp, MEGA);
+ put_unaligned_be24(gain, gain_bytes);
+ ret = ad7779_spi_write(st,
+ AD7779_REG_CH_GAIN_UPPER_BYTE(channel),
+ gain_bytes[0]);
+ if (ret)
+ return ret;
+
+ ret = ad7779_spi_write(st,
+ AD7779_REG_CH_GAIN_MID_BYTE(channel),
+ gain_bytes[1]);
+ if (ret)
+ return ret;
+
+ return ad7779_spi_write(st,
+ AD7779_REG_CH_GAIN_LOWER_BYTE(channel),
+ gain_bytes[2]);
+}
+
+static int ad7779_get_calibbias(struct ad7779_state *st, int channel)
+{
+ int ret;
+ u8 calibbias[3];
+ u8 low, mid, high;
+
+ ret = ad7779_spi_read(st, AD7779_REG_CH_OFFSET_LOWER_BYTE(channel),
+ &calibbias[0]);
+ if (ret)
+ return ret;
+
+ ret = ad7779_spi_read(st, AD7779_REG_CH_OFFSET_MID_BYTE(channel),
+ &calibbias[1]);
+ if (ret)
+ return ret;
+
+ ret = ad7779_spi_read(st,
+ AD7779_REG_CH_OFFSET_UPPER_BYTE(channel),
+ &calibbias[2]);
+ if (ret)
+ return ret;
+
+ return get_unaligned_be24(calibbias);
+}
+
+static int ad7779_set_calibbias(struct ad7779_state *st, int channel, int val)
+{
+ int ret;
+ u8 calibbias[3];
+ u8 msb, mid, lsb;
+
+ msb = FIELD_GET(AD7779_UPPER, val);
+ mid = FIELD_GET(AD7779_MID, val);
+ lsb = FIELD_GET(AD7779_LOWER, val);
+ put_unaligned_be24(val, calibbias);
+ ret = ad7779_spi_write(st,
+ AD7779_REG_CH_OFFSET_UPPER_BYTE(channel),
+ calibbias[0]);
+ if (ret)
+ return ret;
+
+ ret = ad7779_spi_write(st,
+ AD7779_REG_CH_OFFSET_MID_BYTE(channel),
+ calibbias[1]);
+ if (ret)
+ return ret;
+
+ return ad7779_spi_write(st,
+ AD7779_REG_CH_OFFSET_LOWER_BYTE(channel),
+ calibbias[2]);
+}
+
+static int ad7779_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val,
+ int *val2,
+ long mask)
+{
+ struct ad7779_state *st = iio_priv(indio_dev);
+ int ret;
+
+ iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
+ switch (mask) {
+ case IIO_CHAN_INFO_CALIBSCALE:
+ *val = ad7779_get_calibscale(st, chan->channel);
+ iio_device_release_direct_mode(indio_dev);
+ if (*val < 0)
+ return -EINVAL;
+ *val2 = GAIN_REL;
+ return IIO_VAL_FRACTIONAL;
+ case IIO_CHAN_INFO_CALIBBIAS:
+ *val = ad7779_get_calibbias(st, chan->channel);
+ iio_device_release_direct_mode(indio_dev);
+ if (*val < 0)
+ return -EINVAL;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ *val = st->sampling_freq;
+ iio_device_release_direct_mode(indio_dev);
+ if (*val < 0)
+ return -EINVAL;
+ return IIO_VAL_INT;
+ }
+ return -EINVAL;
+ }
+ unreachable();
+}
+
+static int ad7779_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
+{
+ struct ad7779_state *st = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_CALIBSCALE:
+ return ad7779_set_calibscale(st, chan->channel, val2);
+ case IIO_CHAN_INFO_CALIBBIAS:
+ return ad7779_set_calibbias(st, chan->channel, val);
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ return ad7779_set_sampling_frequency(st, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad7779_buffer_preenable(struct iio_dev *indio_dev)
+{
+ int ret;
+ struct ad7779_state *st = iio_priv(indio_dev);
+
+ ret = ad7779_spi_write_mask(st,
+ AD7779_REG_GENERAL_USER_CONFIG_3,
+ AD7779_MOD_SPI_EN_MSK,
+ FIELD_PREP(AD7779_MOD_SPI_EN_MSK, 1));
+ if (ret)
+ return ret;
+
+ /* DRDY output cannot be disabled at device level
+ * therefore we mask the irq at host end.
+ */
+ enable_irq(st->spi->irq);
+
+ return 0;
+}
+
+static int ad7779_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct ad7779_state *st = iio_priv(indio_dev);
+
+ disable_irq(st->spi->irq);
+
+ return ad7779_spi_write(st, AD7779_REG_GENERAL_USER_CONFIG_3,
+ AD7779_DISABLE_SD);
+}
+
+static irqreturn_t ad7779_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct ad7779_state *st = iio_priv(indio_dev);
+ int ret;
+ int bit;
+ int k = 0;
+ /* Each channel shifts out HEADER + 24 bits of data
+ * therefore 8 * u32 for the data and 64 bits for
+ * the timestamp
+ */
+ u32 tmp[10];
+
+ struct spi_transfer sd_readback_tr[] = {
+ {
+ .rx_buf = st->spidata_rx,
+ .tx_buf = st->spidata_tx,
+ .len = 32,
+ }
+ };
+
+ if (!iio_buffer_enabled(indio_dev)){
+ iio_trigger_notify_done(indio_dev->trig);
+ return IRQ_HANDLED;
+ }
+
+ st->spidata_tx[0] = AD7779_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");
+ iio_trigger_notify_done(indio_dev->trig);
+ return IRQ_HANDLED;
+ }
+
+ for_each_set_bit(bit, indio_dev->active_scan_mask, AD7779_NUM_CHANNELS - 1)
+ tmp[k++] = st->spidata_rx[bit];
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &tmp[0], pf->timestamp);
+
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
+static int ad7779_reset(struct iio_dev *indio_dev, struct gpio_desc *reset_gpio)
+{
+ struct ad7779_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) {
+ /* Delay for reset to occur is 225 microseconds*/
+ 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;
+
+ /* Delay for reset to occur is 225 microseconds*/
+ fsleep(230);
+
+ return 0;
+}
+
+static const struct iio_info ad7779_info = {
+ .read_raw = ad7779_read_raw,
+ .write_raw = ad7779_write_raw,
+ .debugfs_reg_access = &ad7779_reg_access,
+};
+
+static const struct iio_enum ad7779_filter_enum = {
+ .items = ad7779_filter_type,
+ .num_items = ARRAY_SIZE(ad7779_filter_type),
+ .get = ad7779_get_filter,
+ .set = ad7779_set_filter,
+};
+
+static const struct iio_chan_spec_ext_info ad7779_ext_filter[] = {
+ IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &ad7779_filter_enum),
+ IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL,
+ &ad7779_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, \
+ .endianness = IIO_BE, \
+ }, \
+ }
+
+#define AD777x_CHAN_NO_FILTER_S(index) \
+ AD777x_CHAN_S(index, NULL)
+
+#define AD777x_CHAN_FILTER_S(index) \
+ AD777x_CHAN_S(index, ad7779_ext_filter)
+
+static const struct iio_chan_spec ad7779_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),
+ IIO_CHAN_SOFT_TIMESTAMP(8),
+};
+
+static const struct iio_chan_spec ad7779_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),
+ IIO_CHAN_SOFT_TIMESTAMP(8),
+};
+
+static const struct iio_buffer_setup_ops ad7779_buffer_setup_ops = {
+ .preenable = ad7779_buffer_preenable,
+ .postdisable = ad7779_buffer_postdisable,
+};
+
+static const struct iio_trigger_ops ad7779_trigger_ops = {
+ .validate_device = iio_trigger_validate_own_device,
+};
+
+static int ad7779_powerup(struct ad7779_state *st, struct gpio_desc *start_gpio)
+{
+ int ret;
+
+ ret = ad7779_spi_write_mask(st, AD7779_REG_GEN_ERR_REG_1_EN,
+ AD7779_SPI_CRC_EN_MSK,
+ FIELD_PREP(AD7779_SPI_CRC_EN_MSK, 1));
+ if (ret)
+ return ret;
+
+ ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
+ AD7779_MOD_POWERMODE_MSK,
+ FIELD_PREP(AD7779_MOD_POWERMODE_MSK, 1));
+ if (ret)
+ return ret;
+
+ ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
+ AD7779_MOD_PDB_REFOUT_MSK,
+ FIELD_PREP(AD7779_MOD_PDB_REFOUT_MSK, 1));
+ if (ret)
+ return ret;
+
+ ret = ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
+ AD7779_DCLK_CLK_DIV_MSK,
+ FIELD_PREP(AD7779_DCLK_CLK_DIV_MSK, 1));
+ if (ret)
+ return ret;
+
+ ret = ad7779_spi_write_mask(st, AD7779_REG_ADC_MUX_CONFIG,
+ AD7779_REFMUX_CTRL_MSK,
+ FIELD_PREP(AD7779_REFMUX_CTRL_MSK, 1));
+ if (ret)
+ return ret;
+
+
+ st->power_mode = AD7779_HIGH_POWER;
+ ret = ad7779_set_sampling_frequency(st, AD7779_DEFAULT_SAMPLING_FREQ);
+ if (ret)
+ return ret;
+
+ gpiod_set_value(start_gpio, 0);
+ /* Start setup time */
+ fsleep(15);
+ gpiod_set_value(start_gpio, 1);
+ fsleep(15);
+ gpiod_set_value(start_gpio, 0);
+ fsleep(15);
+
+ return 0;
+}
+
+static int ad7779_probe(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev;
+ struct ad7779_state *st;
+ struct gpio_desc *reset_gpio, *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->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
+ if (IS_ERR(st->mclk))
+ return PTR_ERR(st->mclk);
+
+ if (!spi->irq)
+ return dev_err_probe(&spi->dev, ret,
+ "DRDY irq not present\n");
+
+ 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(ad7779_crc8_table, AD7779_CRC8_POLY);
+ st->spi = spi;
+
+ st->chip_info = spi_get_device_match_data(spi);
+ if (!st->chip_info)
+ return -ENODEV;
+
+ ret = ad7779_reset(indio_dev, start_gpio);
+ if (ret)
+ return ret;
+
+ ad7779_powerup(st, start_gpio);
+ if (ret)
+ return ret;
+
+ indio_dev->name = st->chip_info->name;
+ indio_dev->info = &ad7779_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = st->chip_info->channels;
+ indio_dev->num_channels = ARRAY_SIZE(ad7779_channels);
+
+ st->trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
+ indio_dev->name, iio_device_id(indio_dev));
+ if (!st->trig)
+ return -ENOMEM;
+
+ st->trig->ops= &ad7779_trigger_ops;
+ st->trig->dev.parent = &spi->dev;
+
+ iio_trigger_set_drvdata(st->trig, st);
+
+ ret = devm_request_irq(&spi->dev, spi->irq,
+ iio_trigger_generic_data_rdy_poll,
+ IRQF_ONESHOT | IRQF_NO_AUTOEN,
+ indio_dev->name, st->trig);
+ if (ret)
+ return dev_err_probe(&spi->dev, ret, "request irq %d failed\n",
+ st->spi->irq);
+
+ ret = devm_iio_trigger_register(&spi->dev, st->trig);
+ if (ret)
+ return ret;
+
+ indio_dev->trig = iio_trigger_get(st->trig);
+
+ init_completion(&st->completion);
+
+ ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
+ &iio_pollfunc_store_time,
+ &ad7779_trigger_handler,
+ &ad7779_buffer_setup_ops);
+ if (ret)
+ return ret;
+
+ ret = ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
+ AD7779_DCLK_CLK_DIV_MSK,
+ FIELD_PREP(AD7779_DCLK_CLK_DIV_MSK, 7));
+ if (ret)
+ return ret;
+
+ return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static int __maybe_unused ad7779_suspend(struct device *dev)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct ad7779_state *st = iio_priv(indio_dev);
+ int ret;
+
+ ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
+ AD7779_MOD_POWERMODE_MSK,
+ FIELD_PREP(AD7779_MOD_POWERMODE_MSK,
+ AD7779_LOW_POWER));
+ if (ret)
+ return ret;
+
+ st->power_mode = AD7779_LOW_POWER;
+ return 0;
+}
+
+static int __maybe_unused ad7779_resume(struct device *dev)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct ad7779_state *st = iio_priv(indio_dev);
+ int ret;
+
+ ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
+ AD7779_MOD_POWERMODE_MSK,
+ FIELD_PREP(AD7779_MOD_POWERMODE_MSK,
+ AD7779_HIGH_POWER));
+ if (ret)
+ return ret;
+
+ st->power_mode = AD7779_HIGH_POWER;
+
+ return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(ad7779_pm_ops, ad7779_suspend, ad7779_resume);
+
+static const struct ad7779_chip_info ad7770_chip_info = {
+ .name = "ad7770",
+ .channels = ad7779_channels,
+};
+
+static const struct ad7779_chip_info ad7771_chip_info = {
+ .name = "ad7771",
+ .channels = ad7779_channels_filter,
+};
+
+static const struct ad7779_chip_info AD7779_chip_info = {
+ .name = "AD7779",
+ .channels = ad7779_channels,
+};
+
+static const struct spi_device_id ad7779_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, ad7779_id);
+
+static const struct of_device_id ad7779_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, ad7779_of_table);
+
+static struct spi_driver ad7779_driver = {
+ .driver = {
+ .name = "ad7779",
+ .pm = pm_sleep_ptr(&ad7779_pm_ops),
+ .of_match_table = ad7779_of_table,
+ },
+ .probe = ad7779_probe,
+ .id_table = ad7779_id,
+};
+module_spi_driver(ad7779_driver);
+
+MODULE_AUTHOR("Ramona Alexandra Nechita <ramona.nechita@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD7779 ADC");
+MODULE_LICENSE("GPL");
--
2.43.0Hi Ramona,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ramona-Alexandra-Nechita/dt-bindings-iio-adc-add-a7779-doc/20240725-000001
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20240724155517.12470-5-ramona.nechita%40analog.com
patch subject: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family
config: hexagon-randconfig-r081-20240728 (https://download.01.org/0day-ci/archive/20240729/202407290151.pCUNfKeG-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project ccae7b461be339e717d02f99ac857cf0bc7d17fc)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202407290151.pCUNfKeG-lkp@intel.com/
smatch warnings:
drivers/iio/adc/ad7779.c:779 ad7779_probe() error: uninitialized symbol 'ret'.
vim +/ret +779 drivers/iio/adc/ad7779.c
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 761 static int ad7779_probe(struct spi_device *spi)
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 762 {
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 763 struct iio_dev *indio_dev;
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 764 struct ad7779_state *st;
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 765 struct gpio_desc *reset_gpio, *start_gpio;
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 766 int ret;
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 767
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 768 indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 769 if (!indio_dev)
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 770 return -ENOMEM;
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 771
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 772 st = iio_priv(indio_dev);
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 773
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 774 st->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 775 if (IS_ERR(st->mclk))
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 776 return PTR_ERR(st->mclk);
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 777
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 778 if (!spi->irq)
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 @779 return dev_err_probe(&spi->dev, ret,
^^^
s/ret/-EINVAL/
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 780 "DRDY irq not present\n");
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 781
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 782 reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 783 if (IS_ERR(reset_gpio))
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Wed, 24 Jul 2024 18:54:41 +0300
Ramona Alexandra Nechita <ramona.nechita@analog.com> wrote:
> Add support for AD7770, AD7771, AD7779 ADCs. The device is capable of
> sending out data both on DOUT lines interface,as on the SDO line.
> The driver currently implements only theSDO data streaming mode. SPI
> communication is used alternatively foraccessingregisters and streaming
> data. Register access are protected by crc8.
>
> Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@analog.com>
Hi Ramona,
Various comments inline. Key one though is make sure you read your
own code before posting as there is a bunch of left over stuff
from updates still in the code.
Jonathan
> diff --git a/drivers/iio/adc/ad7779.c b/drivers/iio/adc/ad7779.c
> new file mode 100644
> index 000000000000..7a83977fd00c
> --- /dev/null
> +++ b/drivers/iio/adc/ad7779.c
> +
> +static int ad7779_spi_write(struct ad7779_state *st, u8 reg, u8 val)
> +{
> + int length = 3;
> + struct spi_transfer reg_write_tr[] = {
> + {
> + .tx_buf = st->reg_tx_buf,
> + },
> + };
> +
> + if (reg == AD7779_REG_GEN_ERR_REG_1_EN)
> + length = 2;
> + reg_write_tr[0].len = length;
> +
> + st->reg_tx_buf[0] = FIELD_GET(AD7779_REG_MSK, reg);
> + st->reg_tx_buf[1] = val;
> + st->reg_tx_buf[2] = crc8(ad7779_crc8_table, st->reg_tx_buf, 2, 0);
> +
> + return spi_sync_transfer(st->spi, reg_write_tr, ARRAY_SIZE(reg_write_tr));
spi_write()?
> +}
> +static int ad7779_set_sampling_frequency(struct ad7779_state *st,
> + unsigned int sampling_freq)
> +{
...
> +
> + if (st->power_mode == AD7779_LOW_POWER)
At the moment we only end up in low power mode via suspend I think.
So this logic and indeed the tracking of power mode isn't yet needed.
> + div = AD7779_LOWPOWER_DIV;
> + else
> + div = AD7779_HIGHPOWER_DIV;
> +
> +
> +static int ad7779_get_calibscale(struct ad7779_state *st, int channel)
> +{
> + int ret;
> + u8 calibscale[3];
> + // u8 low, mid, high;
> + ret = ad7779_spi_read(st, AD7779_REG_CH_GAIN_LOWER_BYTE(channel), &calibscale[0]);
> + if (ret)
> + return ret;
> +
> + ret = ad7779_spi_read(st, AD7779_REG_CH_GAIN_MID_BYTE(channel), &calibscale[1]);
> + if (ret)
> + return ret;
> +
> + ret = ad7779_spi_read(st, AD7779_REG_CH_GAIN_UPPER_BYTE(channel), &calibscale[2]);
> + if (ret)
> + return ret;
> +
> + // return FIELD_PREP(AD7779_UPPER, high) | FIELD_PREP(AD7779_MID, mid) |
> + // FIELD_PREP(AD7779_LOWER, low);
Remember to cleanup before posting. Always read through your own patch for this
sort of leftover stuff or get someone else to do it before posting (as can be hard
to spot in your own code).
> + return get_unaligned_be24(calibscale);
> +}
> +
> +static int ad7779_set_calibbias(struct ad7779_state *st, int channel, int val)
> +{
> + int ret;
> + u8 calibbias[3];
> + u8 msb, mid, lsb;
> +
> + msb = FIELD_GET(AD7779_UPPER, val);
> + mid = FIELD_GET(AD7779_MID, val);
> + lsb = FIELD_GET(AD7779_LOWER, val);
? left over code?
> + put_unaligned_be24(val, calibbias);
> + ret = ad7779_spi_write(st,
> + AD7779_REG_CH_OFFSET_UPPER_BYTE(channel),
> + calibbias[0]);
> + if (ret)
> + return ret;
> +
> + ret = ad7779_spi_write(st,
> + AD7779_REG_CH_OFFSET_MID_BYTE(channel),
> + calibbias[1]);
> + if (ret)
> + return ret;
> +
> + return ad7779_spi_write(st,
> + AD7779_REG_CH_OFFSET_LOWER_BYTE(channel),
> + calibbias[2]);
> +}
> +
> +static int ad7779_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val,
> + int *val2,
> + long mask)
> +{
> + struct ad7779_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> + switch (mask) {
> + case IIO_CHAN_INFO_CALIBSCALE:
> + *val = ad7779_get_calibscale(st, chan->channel);
> + iio_device_release_direct_mode(indio_dev);
> + if (*val < 0)
> + return -EINVAL;
> + *val2 = GAIN_REL;
> + return IIO_VAL_FRACTIONAL;
> + case IIO_CHAN_INFO_CALIBBIAS:
> + *val = ad7779_get_calibbias(st, chan->channel);
> + iio_device_release_direct_mode(indio_dev);
> + if (*val < 0)
> + return -EINVAL;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *val = st->sampling_freq;
> + iio_device_release_direct_mode(indio_dev);
> + if (*val < 0)
> + return -EINVAL;
> + return IIO_VAL_INT;
> + }
> + return -EINVAL;
> + }
> + unreachable();
> +}
> +
> +static int ad7779_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val,
> + int val2,
No need to wrap so much. Stick a few of these on one line.
> + long mask)
> +{
> + struct ad7779_state *st = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_CALIBSCALE:
> + return ad7779_set_calibscale(st, chan->channel, val2);
We have to take the direct_mode lock to read them but not write?
That seems backwards. Should always be able to read, but write
should be stopped if buffered capture in flight.
> + case IIO_CHAN_INFO_CALIBBIAS:
> + return ad7779_set_calibbias(st, chan->channel, val);
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + return ad7779_set_sampling_frequency(st, val);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad7779_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + int ret;
> + struct ad7779_state *st = iio_priv(indio_dev);
> +
> + ret = ad7779_spi_write_mask(st,
> + AD7779_REG_GENERAL_USER_CONFIG_3,
> + AD7779_MOD_SPI_EN_MSK,
> + FIELD_PREP(AD7779_MOD_SPI_EN_MSK, 1));
> + if (ret)
> + return ret;
> +
> + /* DRDY output cannot be disabled at device level
Comment syntax needs fixing.
> + * therefore we mask the irq at host end.
> + */
> + enable_irq(st->spi->irq);
> +
> + return 0;
> +}
> +
> +static int ad7779_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + struct ad7779_state *st = iio_priv(indio_dev);
> +
> + disable_irq(st->spi->irq);
> +
> + return ad7779_spi_write(st, AD7779_REG_GENERAL_USER_CONFIG_3,
> + AD7779_DISABLE_SD);
> +}
> +
> +static irqreturn_t ad7779_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct ad7779_state *st = iio_priv(indio_dev);
> + int ret;
> + int bit;
> + int k = 0;
> + /* Each channel shifts out HEADER + 24 bits of data
Wrong comment syntax, and also early than necessary line wrap.
> + * therefore 8 * u32 for the data and 64 bits for
> + * the timestamp
> + */
> + u32 tmp[10];
> +
> + struct spi_transfer sd_readback_tr[] = {
> + {
> + .rx_buf = st->spidata_rx,
> + .tx_buf = st->spidata_tx,
> + .len = 32,
Why 32? Good to add some maths or a comment on the sizing.
> + }
> + };
> +
> + if (!iio_buffer_enabled(indio_dev)){
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;
use a goto.
> + }
> +
> + st->spidata_tx[0] = AD7779_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");
goto.
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;
> + }
> +
> + for_each_set_bit(bit, indio_dev->active_scan_mask, AD7779_NUM_CHANNELS - 1)
> + tmp[k++] = st->spidata_rx[bit];
Update this to use Nuno's new macros for iterating over the scan mask.
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, &tmp[0], pf->timestamp);
> +
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +static int ad7779_powerup(struct ad7779_state *st, struct gpio_desc *start_gpio)
> +{
> + int ret;
> +
> + ret = ad7779_spi_write_mask(st, AD7779_REG_GEN_ERR_REG_1_EN,
> + AD7779_SPI_CRC_EN_MSK,
> + FIELD_PREP(AD7779_SPI_CRC_EN_MSK, 1));
> + if (ret)
> + return ret;
> +
> + ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
> + AD7779_MOD_POWERMODE_MSK,
> + FIELD_PREP(AD7779_MOD_POWERMODE_MSK, 1));
Do these need to be done separately? Or can a single write be used to write
both fields?
> + if (ret)
> + return ret;
> +
> + ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
> + AD7779_MOD_PDB_REFOUT_MSK,
> + FIELD_PREP(AD7779_MOD_PDB_REFOUT_MSK, 1));
> + if (ret)
> + return ret;
> +
> + ret = ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
> + AD7779_DCLK_CLK_DIV_MSK,
> + FIELD_PREP(AD7779_DCLK_CLK_DIV_MSK, 1));
> + if (ret)
> + return ret;
> +
> + ret = ad7779_spi_write_mask(st, AD7779_REG_ADC_MUX_CONFIG,
> + AD7779_REFMUX_CTRL_MSK,
> + FIELD_PREP(AD7779_REFMUX_CTRL_MSK, 1));
> + if (ret)
> + return ret;
> +
> +
> + st->power_mode = AD7779_HIGH_POWER;
> + ret = ad7779_set_sampling_frequency(st, AD7779_DEFAULT_SAMPLING_FREQ);
> + if (ret)
> + return ret;
> +
> + gpiod_set_value(start_gpio, 0);
> + /* Start setup time */
> + fsleep(15);
> + gpiod_set_value(start_gpio, 1);
> + fsleep(15);
> + gpiod_set_value(start_gpio, 0);
> + fsleep(15);
> +
> + return 0;
> +}
> +
> +static int ad7779_probe(struct spi_device *spi)
> +{
...
> + indio_dev->name = st->chip_info->name;
> + indio_dev->info = &ad7779_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = st->chip_info->channels;
> + indio_dev->num_channels = ARRAY_SIZE(ad7779_channels);
> +
> + st->trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
> + indio_dev->name, iio_device_id(indio_dev));
As has already been commented on. Fix all this alignment stuff.
> + if (!st->trig)
> + return -ENOMEM;
> +
> + st->trig->ops= &ad7779_trigger_ops;
> + st->trig->dev.parent = &spi->dev;
devm_iio_trigger_alloc does this for you.
https://elixir.bootlin.com/linux/v6.10/source/drivers/iio/industrialio-trigger.c#L563
> +
> + iio_trigger_set_drvdata(st->trig, st);
> +
> + ret = devm_request_irq(&spi->dev, spi->irq,
> + iio_trigger_generic_data_rdy_poll,
> + IRQF_ONESHOT | IRQF_NO_AUTOEN,
> + indio_dev->name, st->trig);
> + if (ret)
> + return dev_err_probe(&spi->dev, ret, "request irq %d failed\n",
> + st->spi->irq);
...
> + return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
> +static int __maybe_unused ad7779_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct ad7779_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
> + AD7779_MOD_POWERMODE_MSK,
> + FIELD_PREP(AD7779_MOD_POWERMODE_MSK,
> + AD7779_LOW_POWER));
> + if (ret)
> + return ret;
> +
> + st->power_mode = AD7779_LOW_POWER;
As above. There should be no need to track this as you only change it
in suspend / resume.
> + return 0;
> +}
> +
> +static int __maybe_unused ad7779_resume(struct device *dev)
Don't need __maybe_unused as DEFINE_SIMPLE_DEV_PM_OPS and pm_sleep_ptr() ensure
these functions are visible to the compiler, but that it them removes them
if they aren't in use.
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct ad7779_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
> + AD7779_MOD_POWERMODE_MSK,
> + FIELD_PREP(AD7779_MOD_POWERMODE_MSK,
> + AD7779_HIGH_POWER));
> + if (ret)
> + return ret;
> +
> + st->power_mode = AD7779_HIGH_POWER;
> +
> + return 0;
> +}
Hi Ramona,
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 next-20240724]
[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/Ramona-Alexandra-Nechita/dt-bindings-iio-adc-add-a7779-doc/20240725-000001
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20240724155517.12470-5-ramona.nechita%40analog.com
patch subject: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240725/202407251042.LUZ78skF-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240725/202407251042.LUZ78skF-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407251042.LUZ78skF-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/iio/adc/ad7779.c:420:5: warning: unused variable 'low' [-Wunused-variable]
420 | u8 low, mid, high;
| ^~~
drivers/iio/adc/ad7779.c:420:10: warning: unused variable 'mid' [-Wunused-variable]
420 | u8 low, mid, high;
| ^~~
drivers/iio/adc/ad7779.c:420:15: warning: unused variable 'high' [-Wunused-variable]
420 | u8 low, mid, high;
| ^~~~
drivers/iio/adc/ad7779.c:445:5: warning: variable 'msb' set but not used [-Wunused-but-set-variable]
445 | u8 msb, mid, lsb;
| ^
drivers/iio/adc/ad7779.c:445:10: warning: variable 'mid' set but not used [-Wunused-but-set-variable]
445 | u8 msb, mid, lsb;
| ^
drivers/iio/adc/ad7779.c:445:15: warning: variable 'lsb' set but not used [-Wunused-but-set-variable]
445 | u8 msb, mid, lsb;
| ^
drivers/iio/adc/ad7779.c:475:6: warning: unused variable 'ret' [-Wunused-variable]
475 | int ret;
| ^~~
>> drivers/iio/adc/ad7779.c:779:35: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
779 | return dev_err_probe(&spi->dev, ret,
| ^~~
drivers/iio/adc/ad7779.c:766:9: note: initialize the variable 'ret' to silence this warning
766 | int ret;
| ^
| = 0
8 warnings generated.
vim +/ret +779 drivers/iio/adc/ad7779.c
760
761 static int ad7779_probe(struct spi_device *spi)
762 {
763 struct iio_dev *indio_dev;
764 struct ad7779_state *st;
765 struct gpio_desc *reset_gpio, *start_gpio;
766 int ret;
767
768 indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
769 if (!indio_dev)
770 return -ENOMEM;
771
772 st = iio_priv(indio_dev);
773
774 st->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
775 if (IS_ERR(st->mclk))
776 return PTR_ERR(st->mclk);
777
778 if (!spi->irq)
> 779 return dev_err_probe(&spi->dev, ret,
780 "DRDY irq not present\n");
781
782 reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
783 if (IS_ERR(reset_gpio))
784 return PTR_ERR(reset_gpio);
785
786 start_gpio = devm_gpiod_get(&spi->dev, "start", GPIOD_OUT_HIGH);
787 if (IS_ERR(start_gpio))
788 return PTR_ERR(start_gpio);
789
790 crc8_populate_msb(ad7779_crc8_table, AD7779_CRC8_POLY);
791 st->spi = spi;
792
793 st->chip_info = spi_get_device_match_data(spi);
794 if (!st->chip_info)
795 return -ENODEV;
796
797 ret = ad7779_reset(indio_dev, start_gpio);
798 if (ret)
799 return ret;
800
801 ad7779_powerup(st, start_gpio);
802 if (ret)
803 return ret;
804
805 indio_dev->name = st->chip_info->name;
806 indio_dev->info = &ad7779_info;
807 indio_dev->modes = INDIO_DIRECT_MODE;
808 indio_dev->channels = st->chip_info->channels;
809 indio_dev->num_channels = ARRAY_SIZE(ad7779_channels);
810
811 st->trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
812 indio_dev->name, iio_device_id(indio_dev));
813 if (!st->trig)
814 return -ENOMEM;
815
816 st->trig->ops= &ad7779_trigger_ops;
817 st->trig->dev.parent = &spi->dev;
818
819 iio_trigger_set_drvdata(st->trig, st);
820
821 ret = devm_request_irq(&spi->dev, spi->irq,
822 iio_trigger_generic_data_rdy_poll,
823 IRQF_ONESHOT | IRQF_NO_AUTOEN,
824 indio_dev->name, st->trig);
825 if (ret)
826 return dev_err_probe(&spi->dev, ret, "request irq %d failed\n",
827 st->spi->irq);
828
829 ret = devm_iio_trigger_register(&spi->dev, st->trig);
830 if (ret)
831 return ret;
832
833 indio_dev->trig = iio_trigger_get(st->trig);
834
835 init_completion(&st->completion);
836
837 ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
838 &iio_pollfunc_store_time,
839 &ad7779_trigger_handler,
840 &ad7779_buffer_setup_ops);
841 if (ret)
842 return ret;
843
844 ret = ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
845 AD7779_DCLK_CLK_DIV_MSK,
846 FIELD_PREP(AD7779_DCLK_CLK_DIV_MSK, 7));
847 if (ret)
848 return ret;
849
850 return devm_iio_device_register(&spi->dev, indio_dev);
851 }
852
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Ramona,
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 next-20240724]
[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/Ramona-Alexandra-Nechita/dt-bindings-iio-adc-add-a7779-doc/20240725-000001
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20240724155517.12470-5-ramona.nechita%40analog.com
patch subject: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family
config: xtensa-allyesconfig (https://download.01.org/0day-ci/archive/20240725/202407250808.qn23hGFg-lkp@intel.com/config)
compiler: xtensa-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240725/202407250808.qn23hGFg-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407250808.qn23hGFg-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/iio/adc/ad7779.c: In function 'ad7779_get_calibbias':
>> drivers/iio/adc/ad7779.c:420:22: warning: unused variable 'high' [-Wunused-variable]
420 | u8 low, mid, high;
| ^~~~
>> drivers/iio/adc/ad7779.c:420:17: warning: unused variable 'mid' [-Wunused-variable]
420 | u8 low, mid, high;
| ^~~
>> drivers/iio/adc/ad7779.c:420:12: warning: unused variable 'low' [-Wunused-variable]
420 | u8 low, mid, high;
| ^~~
drivers/iio/adc/ad7779.c: In function 'ad7779_set_calibbias':
>> drivers/iio/adc/ad7779.c:445:22: warning: variable 'lsb' set but not used [-Wunused-but-set-variable]
445 | u8 msb, mid, lsb;
| ^~~
>> drivers/iio/adc/ad7779.c:445:17: warning: variable 'mid' set but not used [-Wunused-but-set-variable]
445 | u8 msb, mid, lsb;
| ^~~
>> drivers/iio/adc/ad7779.c:445:12: warning: variable 'msb' set but not used [-Wunused-but-set-variable]
445 | u8 msb, mid, lsb;
| ^~~
drivers/iio/adc/ad7779.c: In function 'ad7779_read_raw':
>> drivers/iio/adc/ad7779.c:475:13: warning: unused variable 'ret' [-Wunused-variable]
475 | int ret;
| ^~~
vim +/high +420 drivers/iio/adc/ad7779.c
415
416 static int ad7779_get_calibbias(struct ad7779_state *st, int channel)
417 {
418 int ret;
419 u8 calibbias[3];
> 420 u8 low, mid, high;
421
422 ret = ad7779_spi_read(st, AD7779_REG_CH_OFFSET_LOWER_BYTE(channel),
423 &calibbias[0]);
424 if (ret)
425 return ret;
426
427 ret = ad7779_spi_read(st, AD7779_REG_CH_OFFSET_MID_BYTE(channel),
428 &calibbias[1]);
429 if (ret)
430 return ret;
431
432 ret = ad7779_spi_read(st,
433 AD7779_REG_CH_OFFSET_UPPER_BYTE(channel),
434 &calibbias[2]);
435 if (ret)
436 return ret;
437
438 return get_unaligned_be24(calibbias);
439 }
440
441 static int ad7779_set_calibbias(struct ad7779_state *st, int channel, int val)
442 {
443 int ret;
444 u8 calibbias[3];
> 445 u8 msb, mid, lsb;
446
447 msb = FIELD_GET(AD7779_UPPER, val);
448 mid = FIELD_GET(AD7779_MID, val);
449 lsb = FIELD_GET(AD7779_LOWER, val);
450 put_unaligned_be24(val, calibbias);
451 ret = ad7779_spi_write(st,
452 AD7779_REG_CH_OFFSET_UPPER_BYTE(channel),
453 calibbias[0]);
454 if (ret)
455 return ret;
456
457 ret = ad7779_spi_write(st,
458 AD7779_REG_CH_OFFSET_MID_BYTE(channel),
459 calibbias[1]);
460 if (ret)
461 return ret;
462
463 return ad7779_spi_write(st,
464 AD7779_REG_CH_OFFSET_LOWER_BYTE(channel),
465 calibbias[2]);
466 }
467
468 static int ad7779_read_raw(struct iio_dev *indio_dev,
469 struct iio_chan_spec const *chan,
470 int *val,
471 int *val2,
472 long mask)
473 {
474 struct ad7779_state *st = iio_priv(indio_dev);
> 475 int ret;
476
477 iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
478 switch (mask) {
479 case IIO_CHAN_INFO_CALIBSCALE:
480 *val = ad7779_get_calibscale(st, chan->channel);
481 iio_device_release_direct_mode(indio_dev);
482 if (*val < 0)
483 return -EINVAL;
484 *val2 = GAIN_REL;
485 return IIO_VAL_FRACTIONAL;
486 case IIO_CHAN_INFO_CALIBBIAS:
487 *val = ad7779_get_calibbias(st, chan->channel);
488 iio_device_release_direct_mode(indio_dev);
489 if (*val < 0)
490 return -EINVAL;
491 return IIO_VAL_INT;
492 case IIO_CHAN_INFO_SAMP_FREQ:
493 *val = st->sampling_freq;
494 iio_device_release_direct_mode(indio_dev);
495 if (*val < 0)
496 return -EINVAL;
497 return IIO_VAL_INT;
498 }
499 return -EINVAL;
500 }
501 unreachable();
502 }
503
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 24/07/2024 17:54, Ramona Alexandra Nechita wrote:
> Add support for AD7770, AD7771, AD7779 ADCs. The device is capable of
> sending out data both on DOUT lines interface,as on the SDO line.
> The driver currently implements only theSDO data streaming mode. SPI
> communication is used alternatively foraccessingregisters and streaming
Typo, please run spell check.
Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
There is no "drivers".
> data. Register access are protected by crc8.
>
> Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@analog.com>
> ---
> drivers/iio/adc/Kconfig | 11 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ad7779.c | 952 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 964 insertions(+)
> create mode 100644 drivers/iio/adc/ad7779.c
>
The driver has several trivial style issues. Please be sure such
trivialities are fixed. Get internal review on this. You do need to ask
community to tell you that you must run checkpatch. Or to tell them that
indentation/alignment is entirely broken. Grab some colleague of yours
and perform internal review first. This applies to entire Analog,
because there is increased amount of contributions from Analog and not
all of them look like passing basic sanity checks.
By sending code full of silly trivialities, community reviewers might
feel overwhelmed and quite grumpy.
> +
> + indio_dev->trig = iio_trigger_get(st->trig);
> +
> + init_completion(&st->completion);
> +
> + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
> + &iio_pollfunc_store_time,
> + &ad7779_trigger_handler,
> + &ad7779_buffer_setup_ops);
Sorry, tha's not a Linux coding style. Checkpatch will tell you that.
Please run scripts/checkpatch.pl and fix reported warnings. Then please
run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.
> + if (ret)
> + return ret;
> +
> + ret = ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
> + AD7779_DCLK_CLK_DIV_MSK,
> + FIELD_PREP(AD7779_DCLK_CLK_DIV_MSK, 7));
> + if (ret)
> + return ret;
> +
> + return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
> +static int __maybe_unused ad7779_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct ad7779_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
> + AD7779_MOD_POWERMODE_MSK,
> + FIELD_PREP(AD7779_MOD_POWERMODE_MSK,
> + AD7779_LOW_POWER));
> + if (ret)
> + return ret;
> +
> + st->power_mode = AD7779_LOW_POWER;
> + return 0;
> +}
> +
> +static int __maybe_unused ad7779_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct ad7779_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
> + AD7779_MOD_POWERMODE_MSK,
> + FIELD_PREP(AD7779_MOD_POWERMODE_MSK,
> + AD7779_HIGH_POWER));
> + if (ret)
> + return ret;
> +
> + st->power_mode = AD7779_HIGH_POWER;
> +
> + return 0;
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(ad7779_pm_ops, ad7779_suspend, ad7779_resume);
> +
> +static const struct ad7779_chip_info ad7770_chip_info = {
> + .name = "ad7770",
> + .channels = ad7779_channels,
> +};
> +
> +static const struct ad7779_chip_info ad7771_chip_info = {
> + .name = "ad7771",
> + .channels = ad7779_channels_filter,
> +};
> +
> +static const struct ad7779_chip_info AD7779_chip_info = {
> + .name = "AD7779",
Hm...
> + .channels = ad7779_channels,
> +};
> +
> +static const struct spi_device_id ad7779_id[] = {
> + {
> + .name = "ad7770",
> + .driver_data = (kernel_ulong_t)&ad7770_chip_info
> + },
> + {
> + .name = "ad7771",
> + .driver_data = (kernel_ulong_t)&ad7771_chip_info
> + },
> + {
> + .name = "AD7779",
Eh...
> + .driver_data = (kernel_ulong_t)&AD7779_chip_info
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(spi, ad7779_id);
> +
> +static const struct of_device_id ad7779_of_table[] = {
> + {
> + .compatible = "adi,ad7770",
> + .data = &ad7770_chip_info,
> + },
> + {
> + .compatible = "adi,ad7771",
> + .data = &ad7771_chip_info,
> + },
> + {
> + .compatible = "adi,AD7779",
That's not a valid compatible. Only lower-case are allowed.
> + .data = &AD7779_chip_info,
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ad7779_of_table);
> +
Best regards,
Krzysztof
On Wed, Jul 24, 2024 at 06:14:52PM +0200, Krzysztof Kozlowski wrote: > On 24/07/2024 17:54, Ramona Alexandra Nechita wrote: ... > Please use subject prefixes matching the subsystem. You can get them for > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > your patch is touching. I would be more precise `git log --oneline --no-merges -- DIRECTORY_OR_FILE` -- With Best Regards, Andy Shevchenko
On 24/07/2024 18:14, Krzysztof Kozlowski wrote: > On 24/07/2024 17:54, Ramona Alexandra Nechita wrote: >> Add support for AD7770, AD7771, AD7779 ADCs. The device is capable of >> sending out data both on DOUT lines interface,as on the SDO line. >> The driver currently implements only theSDO data streaming mode. SPI >> communication is used alternatively foraccessingregisters and streaming > > Typo, please run spell check. > > Please use subject prefixes matching the subsystem. You can get them for > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > your patch is touching. For bindings, the preferred subjects are > explained here: > https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters > > There is no "drivers". > >> data. Register access are protected by crc8. >> >> Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@analog.com> >> --- >> drivers/iio/adc/Kconfig | 11 + >> drivers/iio/adc/Makefile | 1 + >> drivers/iio/adc/ad7779.c | 952 +++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 964 insertions(+) >> create mode 100644 drivers/iio/adc/ad7779.c >> > > The driver has several trivial style issues. Please be sure such > trivialities are fixed. Get internal review on this. You do need to ask > community to tell you that you must run checkpatch. Or to tell them that > indentation/alignment is entirely broken. Grab some colleague of yours > and perform internal review first. This applies to entire Analog, > because there is increased amount of contributions from Analog and not > all of them look like passing basic sanity checks. > > By sending code full of silly trivialities, community reviewers might > feel overwhelmed and quite grumpy. The amount of trivial warnings pointed out by compilation in separate email is as well disappointing. You do not need community to check if all variables are used - compilers tell it, so use them. Toolchains and static checkers will point some issues without any need of using community reviewers. Look, isn't it great? Instead of using human force you can use tools... This applies to all Analog contributions - *you must check* your code with W=1, sparse, smatch and coccinelle. Best regards, Krzysztof
© 2016 - 2025 Red Hat, Inc.