Add support for the MAX22007, a 4-channel 12-bit DAC that drives
voltage or current output on each channel.
Signed-off-by: Janani Sunil <janani.sunil@analog.com>
---
MAINTAINERS | 1 +
drivers/iio/dac/Kconfig | 13 ++
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/max22007.c | 507 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 522 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index e1addbd21562..99dd3c947629 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1599,6 +1599,7 @@ L: linux-iio@vger.kernel.org
S: Supported
W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/dac/adi,max22007.yaml
+F: drivers/iio/dac/max22007.c
ANALOG DEVICES INC ADA4250 DRIVER
M: Antoniu Miclaus <antoniu.miclaus@analog.com>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 7cd3caec1262..4a31993f5b14 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -482,6 +482,19 @@ config MAX517
This driver can also be built as a module. If so, the module
will be called max517.
+config MAX22007
+ tristate "Analog Devices MAX22007 DAC Driver"
+ depends on SPI
+ select REGMAP_SPI
+ select CRC8
+ help
+ Say Y here if you want to build a driver for Analog Devices MAX22007.
+
+ MAX22007 is a quad-channel, 12-bit, voltage-output digital to
+ analog converter (DAC) with SPI interface.
+
+ If compiled as a module, it will be called max22007.
+
config MAX5522
tristate "Maxim MAX5522 DAC driver"
depends on SPI_MASTER
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index e6ac4c67e337..0bbc6d09d22c 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_LTC2664) += ltc2664.o
obj-$(CONFIG_LTC2688) += ltc2688.o
obj-$(CONFIG_M62332) += m62332.o
obj-$(CONFIG_MAX517) += max517.o
+obj-$(CONFIG_MAX22007) += max22007.o
obj-$(CONFIG_MAX5522) += max5522.o
obj-$(CONFIG_MAX5821) += max5821.o
obj-$(CONFIG_MCP4725) += mcp4725.o
diff --git a/drivers/iio/dac/max22007.c b/drivers/iio/dac/max22007.c
new file mode 100644
index 000000000000..19557c008554
--- /dev/null
+++ b/drivers/iio/dac/max22007.c
@@ -0,0 +1,507 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * max22007.c - MAX22007 DAC driver
+ *
+ * Driver for Analog Devices MAX22007 Digital to Analog Converter.
+ *
+ * Copyright (c) 2026 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/crc8.h>
+#include <linux/dev_printk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+#define MAX22007_NUM_CHANNELS 4
+#define MAX22007_REV_ID_REG 0x00
+#define MAX22007_STAT_INTR_REG 0x01
+#define MAX22007_INTERRUPT_EN_REG 0x02
+#define MAX22007_CONFIG_REG 0x03
+#define MAX22007_CONTROL_REG 0x04
+#define MAX22007_CHANNEL_MODE_REG 0x05
+#define MAX22007_SOFT_RESET_REG 0x06
+#define MAX22007_DAC_CHANNEL_REG(ch) (0x07 + (ch))
+#define MAX22007_GPIO_CTRL_REG 0x0B
+#define MAX22007_GPIO_DATA_REG 0x0C
+#define MAX22007_GPI_EDGE_INT_CTRL_REG 0x0D
+#define MAX22007_GPI_INT_STATUS_REG 0x0E
+
+/* Channel mask definitions */
+#define MAX22007_CH_MODE_CH_MASK(ch) BIT(12 + (ch))
+#define MAX22007_CH_PWRON_CH_MASK(ch) BIT(8 + (ch))
+#define MAX22007_DAC_LATCH_MODE_MASK(ch) BIT(12 + (ch))
+#define MAX22007_LDAC_UPDATE_MASK(ch) BIT(12 + (ch))
+#define MAX22007_SW_RST_MASK BIT(8)
+#define MAX22007_SW_CLR_MASK BIT(12)
+#define MAX22007_SOFT_RESET_BITS_MASK (MAX22007_SW_RST_MASK | \
+ MAX22007_SW_CLR_MASK)
+#define MAX22007_DAC_DATA_MASK GENMASK(15, 4)
+#define MAX22007_DAC_MAX_RAW GENMASK(11, 0)
+#define MAX22007_CRC8_POLYNOMIAL 0x8C
+#define MAX22007_CRC_EN_MASK BIT(0)
+#define MAX22007_RW_MASK BIT(0)
+#define MAX22007_CRC_OVERHEAD 1
+#define MAX22007_NUM_SUPPLIES 3
+
+/* Field value preparation macros with masking */
+#define MAX22007_CH_PWR_VAL(ch, val) (((val) & 0x1) << (8 + (ch)))
+#define MAX22007_CH_MODE_VAL(ch, val) (((val) & 0x1) << (12 + (ch)))
+#define MAX22007_DAC_LATCH_MODE_VAL(ch, val) (((val) & 0x1) << (12 + (ch)))
+
+static u8 max22007_crc8_table[256];
+
+static const char * const max22007_supply_names[MAX22007_NUM_SUPPLIES] = {
+ "vdd",
+ "hvdd",
+ "hvss",
+};
+
+enum max22007_channel_mode {
+ MAX22007_VOLTAGE_MODE = 0,
+ MAX22007_CURRENT_MODE = 1,
+};
+
+struct max22007_state {
+ struct spi_device *spi;
+ struct regmap *regmap;
+ struct iio_chan_spec *iio_chans;
+ struct regulator_bulk_data supplies[MAX22007_NUM_SUPPLIES];
+ u8 tx_buf[4] __aligned(IIO_DMA_MINALIGN);
+ u8 rx_buf[4];
+};
+
+static int max22007_spi_read(void *context, const void *reg, size_t reg_size,
+ void *val, size_t val_size)
+{
+ struct max22007_state *st = context;
+ u8 reg_byte = *(u8 *)reg;
+ u8 calculated_crc, received_crc;
+ u8 crc_data[3];
+ u8 rx_buf[4];
+ int ret;
+
+ if (reg_size != 1)
+ return -EINVAL;
+
+ ret = spi_write_then_read(st->spi, ®_byte, 1, rx_buf,
+ val_size + MAX22007_CRC_OVERHEAD);
+ if (ret) {
+ dev_err(&st->spi->dev, "SPI transfer failed: %d\n", ret);
+ return ret;
+ }
+
+ crc_data[0] = reg_byte;
+ crc_data[1] = rx_buf[0];
+ crc_data[2] = rx_buf[1];
+
+ calculated_crc = crc8(max22007_crc8_table, crc_data, 3, 0x00);
+ received_crc = rx_buf[val_size];
+
+ if (calculated_crc != received_crc) {
+ dev_err(&st->spi->dev, "CRC mismatch on read register %02x\n", reg_byte);
+ return -EIO;
+ }
+
+ memcpy(val, rx_buf, val_size);
+
+ return 0;
+}
+
+static int max22007_spi_write(void *context, const void *data, size_t count)
+{
+ struct max22007_state *st = context;
+ struct spi_transfer xfer = {
+ .tx_buf = st->tx_buf,
+ .rx_buf = st->rx_buf,
+ };
+
+ memset(st->tx_buf, 0, sizeof(st->tx_buf));
+
+ xfer.len = count + MAX22007_CRC_OVERHEAD;
+
+ memcpy(st->tx_buf, data, count);
+ st->tx_buf[count] = crc8(max22007_crc8_table, st->tx_buf,
+ sizeof(st->tx_buf) - 1, 0x00);
+
+ return spi_sync_transfer(st->spi, &xfer, 1);
+}
+
+static bool max22007_reg_readable(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case MAX22007_REV_ID_REG:
+ case MAX22007_STAT_INTR_REG:
+ case MAX22007_CONFIG_REG:
+ case MAX22007_CONTROL_REG:
+ case MAX22007_CHANNEL_MODE_REG:
+ case MAX22007_SOFT_RESET_REG:
+ case MAX22007_GPIO_CTRL_REG:
+ case MAX22007_GPIO_DATA_REG:
+ case MAX22007_GPI_EDGE_INT_CTRL_REG:
+ case MAX22007_GPI_INT_STATUS_REG:
+ return true;
+ case MAX22007_DAC_CHANNEL_REG(0) ... MAX22007_DAC_CHANNEL_REG(MAX22007_NUM_CHANNELS - 1):
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool max22007_reg_writable(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case MAX22007_CONFIG_REG:
+ case MAX22007_CONTROL_REG:
+ case MAX22007_CHANNEL_MODE_REG:
+ case MAX22007_SOFT_RESET_REG:
+ case MAX22007_GPIO_CTRL_REG:
+ case MAX22007_GPIO_DATA_REG:
+ case MAX22007_GPI_EDGE_INT_CTRL_REG:
+ return true;
+ case MAX22007_DAC_CHANNEL_REG(0) ... MAX22007_DAC_CHANNEL_REG(MAX22007_NUM_CHANNELS - 1):
+ return true;
+ default:
+ return false;
+ }
+}
+
+static const struct regmap_bus max22007_regmap_bus = {
+ .read = max22007_spi_read,
+ .write = max22007_spi_write,
+ .read_flag_mask = MAX22007_RW_MASK,
+ .reg_format_endian_default = REGMAP_ENDIAN_BIG,
+ .val_format_endian_default = REGMAP_ENDIAN_BIG,
+};
+
+static const struct regmap_config max22007_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 16,
+ .reg_shift = -1,
+ .readable_reg = max22007_reg_readable,
+ .writeable_reg = max22007_reg_writable,
+ .max_register = 0x0E,
+};
+
+static int max22007_write_channel_data(struct max22007_state *state,
+ unsigned int channel, unsigned int data)
+{
+ unsigned int reg_val;
+
+ if (data > MAX22007_DAC_MAX_RAW)
+ return -EINVAL;
+
+ reg_val = FIELD_PREP(MAX22007_DAC_DATA_MASK, data);
+
+ return regmap_write(state->regmap, MAX22007_DAC_CHANNEL_REG(channel), reg_val);
+}
+
+static int max22007_read_channel_data(struct max22007_state *state,
+ unsigned int channel, int *data)
+{
+ int ret;
+ unsigned int reg_val;
+
+ ret = regmap_read(state->regmap, MAX22007_DAC_CHANNEL_REG(channel), ®_val);
+ if (ret)
+ return ret;
+
+ *data = FIELD_GET(MAX22007_DAC_DATA_MASK, reg_val);
+
+ return 0;
+}
+
+static int max22007_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct max22007_state *st = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = max22007_read_channel_data(st, chan->channel, val);
+ if (ret)
+ return ret;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ if (chan->type == IIO_VOLTAGE)
+ *val = 5 * 2500; /* 5 * Vref(2.5V) in mV */
+ else
+ *val = 25; /* Vref / (2 * Rsense) = 2500mV / 100 */
+ *val2 = 12; /* 12-bit DAC resolution */
+ return IIO_VAL_FRACTIONAL_LOG2;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int max22007_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct max22007_state *st = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ return max22007_write_channel_data(st, chan->channel, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info max22007_info = {
+ .read_raw = max22007_read_raw,
+ .write_raw = max22007_write_raw,
+};
+
+static ssize_t max22007_read_dac_powerdown(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ char *buf)
+{
+ struct max22007_state *st = iio_priv(indio_dev);
+ unsigned int reg_val;
+ bool powerdown;
+ int ret;
+
+ ret = regmap_read(st->regmap, MAX22007_CHANNEL_MODE_REG, ®_val);
+ if (ret)
+ return ret;
+
+ powerdown = !(reg_val & MAX22007_CH_PWRON_CH_MASK(chan->channel));
+
+ return sysfs_emit(buf, "%d\n", powerdown);
+}
+
+static ssize_t max22007_write_dac_powerdown(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ const char *buf, size_t len)
+{
+ struct max22007_state *st = iio_priv(indio_dev);
+ bool powerdown;
+ int ret;
+
+ ret = kstrtobool(buf, &powerdown);
+ if (ret)
+ return ret;
+
+ if (powerdown)
+ ret = regmap_update_bits(st->regmap, MAX22007_CHANNEL_MODE_REG,
+ MAX22007_CH_PWRON_CH_MASK(chan->channel),
+ MAX22007_CH_PWR_VAL(chan->channel, 0));
+ else
+ ret = regmap_update_bits(st->regmap, MAX22007_CHANNEL_MODE_REG,
+ MAX22007_CH_PWRON_CH_MASK(chan->channel),
+ MAX22007_CH_PWR_VAL(chan->channel, 1));
+ if (ret)
+ return ret;
+
+ return len;
+}
+
+static const struct iio_chan_spec_ext_info max22007_ext_info[] = {
+ {
+ .name = "powerdown",
+ .read = max22007_read_dac_powerdown,
+ .write = max22007_write_dac_powerdown,
+ .shared = IIO_SEPARATE,
+ },
+ { }
+};
+
+static int max22007_parse_channel_cfg(struct max22007_state *st, u8 *num_channels)
+{
+ struct device *dev = &st->spi->dev;
+ int ret, num_chan;
+ int i = 0;
+ u32 reg;
+
+ num_chan = device_get_child_node_count(dev);
+ if (!num_chan)
+ return dev_err_probe(dev, -ENODEV, "no channels configured\n");
+
+ st->iio_chans = devm_kcalloc(dev, num_chan, sizeof(*st->iio_chans), GFP_KERNEL);
+ if (!st->iio_chans)
+ return -ENOMEM;
+
+ device_for_each_child_node_scoped(dev, child) {
+ u32 ch_func;
+ enum max22007_channel_mode mode;
+ enum iio_chan_type chan_type;
+
+ ret = fwnode_property_read_u32(child, "reg", ®);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to read reg property of %pfwP\n", child);
+
+ if (reg >= MAX22007_NUM_CHANNELS)
+ return dev_err_probe(dev, -EINVAL,
+ "reg out of range in %pfwP\n", child);
+
+ ret = fwnode_property_read_u32(child, "adi,ch-func", &ch_func);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "missing adi,ch-func property for %pfwP\n", child);
+
+ if (ch_func == 1) {
+ mode = MAX22007_VOLTAGE_MODE;
+ chan_type = IIO_VOLTAGE;
+ } else if (ch_func == 2) {
+ mode = MAX22007_CURRENT_MODE;
+ chan_type = IIO_CURRENT;
+ } else {
+ return dev_err_probe(dev, -EINVAL,
+ "invalid adi,ch-func %u for %pfwP\n",
+ ch_func, child);
+ }
+
+ st->iio_chans[i++] = (struct iio_chan_spec) {
+ .output = 1,
+ .indexed = 1,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
+ .ext_info = max22007_ext_info,
+ .channel = reg,
+ .type = chan_type,
+ };
+
+ ret = regmap_update_bits(st->regmap, MAX22007_CHANNEL_MODE_REG,
+ MAX22007_CH_MODE_CH_MASK(reg),
+ MAX22007_CH_MODE_VAL(reg, mode));
+ if (ret)
+ return ret;
+
+ /* Set DAC to transparent mode (immediate update) */
+ ret = regmap_update_bits(st->regmap, MAX22007_CONFIG_REG,
+ MAX22007_DAC_LATCH_MODE_MASK(reg),
+ MAX22007_DAC_LATCH_MODE_VAL(reg, 1));
+ if (ret)
+ return ret;
+ }
+
+ *num_channels = num_chan;
+
+ return 0;
+}
+
+static void max22007_regulator_disable(void *data)
+{
+ struct max22007_state *state = data;
+
+ regulator_bulk_disable(MAX22007_NUM_SUPPLIES, state->supplies);
+}
+
+static int max22007_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct iio_dev *indio_dev;
+ struct max22007_state *state;
+ struct gpio_desc *reset_gpio;
+ u8 num_channels;
+ int ret, i;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ state = iio_priv(indio_dev);
+ state->spi = spi;
+
+ crc8_populate_lsb(max22007_crc8_table, MAX22007_CRC8_POLYNOMIAL);
+
+ state->regmap = devm_regmap_init(dev, &max22007_regmap_bus, state,
+ &max22007_regmap_config);
+ if (IS_ERR(state->regmap))
+ return dev_err_probe(dev, PTR_ERR(state->regmap),
+ "Failed to initialize regmap\n");
+
+ for (i = 0; i < MAX22007_NUM_SUPPLIES; i++)
+ state->supplies[i].supply = max22007_supply_names[i];
+
+ ret = devm_regulator_bulk_get(dev, MAX22007_NUM_SUPPLIES, state->supplies);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to get regulators\n");
+
+ ret = regulator_bulk_enable(MAX22007_NUM_SUPPLIES, state->supplies);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to enable regulators\n");
+
+ ret = devm_add_action_or_reset(dev, max22007_regulator_disable, state);
+ if (ret)
+ return ret;
+
+ reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(reset_gpio),
+ "Failed to get reset GPIO\n");
+
+ if (reset_gpio) {
+ usleep_range(1000, 5000);
+ gpiod_set_value_cansleep(reset_gpio, 1);
+ usleep_range(1000, 5000);
+ } else {
+ ret = regmap_write(state->regmap, MAX22007_SOFT_RESET_REG,
+ MAX22007_SOFT_RESET_BITS_MASK);
+ if (ret)
+ return ret;
+ }
+
+ ret = regmap_set_bits(state->regmap, MAX22007_CONFIG_REG,
+ MAX22007_CRC_EN_MASK);
+ if (ret)
+ return ret;
+
+ ret = max22007_parse_channel_cfg(state, &num_channels);
+ if (ret)
+ return ret;
+
+ indio_dev->info = &max22007_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = state->iio_chans;
+ indio_dev->num_channels = num_channels;
+ indio_dev->name = "max22007";
+
+ return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct spi_device_id max22007_id[] = {
+ { "max22007" },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, max22007_id);
+
+static const struct of_device_id max22007_of_match[] = {
+ { .compatible = "adi,max22007" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max22007_of_match);
+
+static struct spi_driver max22007_driver = {
+ .driver = {
+ .name = "max22007",
+ .of_match_table = max22007_of_match,
+ },
+ .probe = max22007_probe,
+ .id_table = max22007_id,
+};
+module_spi_driver(max22007_driver);
+
+MODULE_AUTHOR("Janani Sunil <janani.sunil@analog.com>");
+MODULE_DESCRIPTION("Analog Devices MAX22007 DAC");
+MODULE_LICENSE("GPL");
--
2.43.0On Thu, 8 Jan 2026 13:58:24 +0100
Janani Sunil <janani.sunil@analog.com> wrote:
> Add support for the MAX22007, a 4-channel 12-bit DAC that drives
> voltage or current output on each channel.
>
> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
> diff --git a/drivers/iio/dac/max22007.c b/drivers/iio/dac/max22007.c
> new file mode 100644
> index 000000000000..19557c008554
> --- /dev/null
> +++ b/drivers/iio/dac/max22007.c
> @@ -0,0 +1,507 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * max22007.c - MAX22007 DAC driver
> + *
> + * Driver for Analog Devices MAX22007 Digital to Analog Converter.
> + *
> + * Copyright (c) 2026 Analog Devices Inc.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/crc8.h>
> +#include <linux/dev_printk.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +#include <linux/spi/spi.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
Hi.
I decided to use your driver (well v1 actually but comments still apply)
to mess around with https://github.com/masoncl/review-prompts (I'm trying to see
if it is worth the effort!) and I queried if the includes were correct.
(rather than running IWYU directly).
It correctly noted:
The driver has 2 missing includes and 1 unnecessary include:
// Should ADD:
#include <linux/kstrtox.h> // for kstrtobool()
#include <linux/sysfs.h> // for sysfs_emit()
// Should REMOVE:
#include <linux/unaligned.h> // not used
Now I keep mean to start running IWYU against commits at time
of merge but it's enough of a pain that I don't do it when just
reviewing.
Interestingly, claude argued itself out of reporting a bug around the reset
in v1 (which is now fixed)
For fun I asked it what smatch would have reported... See below.
> +static int max22007_spi_read(void *context, const void *reg, size_t reg_size,
> + void *val, size_t val_size)
> +{
> + struct max22007_state *st = context;
> + u8 reg_byte = *(u8 *)reg;
> + u8 calculated_crc, received_crc;
> + u8 crc_data[3];
> + u8 rx_buf[4];
> + int ret;
> +
> + if (reg_size != 1)
One from claude that I think is sensible, if not a bug.
Should sanity check val_size as well.
> + return -EINVAL;
> +
> + ret = spi_write_then_read(st->spi, ®_byte, 1, rx_buf,
> + val_size + MAX22007_CRC_OVERHEAD);
> + if (ret) {
> + dev_err(&st->spi->dev, "SPI transfer failed: %d\n", ret);
> + return ret;
> + }
> +
> + crc_data[0] = reg_byte;
> + crc_data[1] = rx_buf[0];
> + crc_data[2] = rx_buf[1];
> +
> + calculated_crc = crc8(max22007_crc8_table, crc_data, 3, 0x00);
> + received_crc = rx_buf[val_size];
> +
> + if (calculated_crc != received_crc) {
> + dev_err(&st->spi->dev, "CRC mismatch on read register %02x\n", reg_byte);
> + return -EIO;
> + }
> +
> + memcpy(val, rx_buf, val_size);
> +
> + return 0;
> +}
> +
> +static int max22007_spi_write(void *context, const void *data, size_t count)
> +{
> + struct max22007_state *st = context;
> + struct spi_transfer xfer = {
> + .tx_buf = st->tx_buf,
> + .rx_buf = st->rx_buf,
> + };
Similarly claude reported a false positive here but it seems sensible to
cover it: Check the size of count is <= ARRAY_SIZE(st->tx_buf) - 1;
> +
> + memset(st->tx_buf, 0, sizeof(st->tx_buf));
> +
> + xfer.len = count + MAX22007_CRC_OVERHEAD;
> +
> + memcpy(st->tx_buf, data, count);
> + st->tx_buf[count] = crc8(max22007_crc8_table, st->tx_buf,
> + sizeof(st->tx_buf) - 1, 0x00);
> +
> + return spi_sync_transfer(st->spi, &xfer, 1);
> +}
...
> +
> +static int max22007_write_channel_data(struct max22007_state *state,
> + unsigned int channel, unsigned int data)
Another one Claude raised that I think is worth cleaning up though not technically
a bug. This is passed int val at the caller as the data parameter here.
I'd keep that as an int and then add a check on it being positive below.
Wrap around means any negative would end up as a big positive and fail the test
below (I think anyway) but we can make the check more obvious!
> +{
> + unsigned int reg_val;
> +
> + if (data > MAX22007_DAC_MAX_RAW)
> + return -EINVAL;
> +
> + reg_val = FIELD_PREP(MAX22007_DAC_DATA_MASK, data);
> +
> + return regmap_write(state->regmap, MAX22007_DAC_CHANNEL_REG(channel), reg_val);
> +}
>
On Thu, 8 Jan 2026 13:58:24 +0100
Janani Sunil <janani.sunil@analog.com> wrote:
> Add support for the MAX22007, a 4-channel 12-bit DAC that drives
> voltage or current output on each channel.
>
> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
A few minor things to add to Marcelo's detailed review.
Thanks,
Jonathan
> diff --git a/drivers/iio/dac/max22007.c b/drivers/iio/dac/max22007.c
> new file mode 100644
> index 000000000000..19557c008554
> --- /dev/null
> +++ b/drivers/iio/dac/max22007.c
> +static int max22007_spi_read(void *context, const void *reg, size_t reg_size,
> + void *val, size_t val_size)
> +{
> + struct max22007_state *st = context;
> + u8 reg_byte = *(u8 *)reg;
> + u8 calculated_crc, received_crc;
> + u8 crc_data[3];
> + u8 rx_buf[4];
> + int ret;
> +
> + if (reg_size != 1)
> + return -EINVAL;
> +
> + ret = spi_write_then_read(st->spi, ®_byte, 1, rx_buf,
> + val_size + MAX22007_CRC_OVERHEAD);
> + if (ret) {
> + dev_err(&st->spi->dev, "SPI transfer failed: %d\n", ret);
> + return ret;
> + }
> +
> + crc_data[0] = reg_byte;
> + crc_data[1] = rx_buf[0];
> + crc_data[2] = rx_buf[1];
> +
> + calculated_crc = crc8(max22007_crc8_table, crc_data, 3, 0x00);
I think you can chain CRCs as follows and avoid the need for a local array
just to marshal the data.
calculated_crc = crc8(max22007_crc8_table, ®_byte, 1, 0x00);
calculated_crc = crc8(max22007_crc8_table, rx_buf, 2, caculated_crc);
> + received_crc = rx_buf[val_size];
> +
> + if (calculated_crc != received_crc) {
> + dev_err(&st->spi->dev, "CRC mismatch on read register %02x\n", reg_byte);
> + return -EIO;
> + }
> +
> + memcpy(val, rx_buf, val_size);
> +
> + return 0;
> +}
> +static ssize_t max22007_write_dac_powerdown(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + const char *buf, size_t len)
> +{
> + struct max22007_state *st = iio_priv(indio_dev);
> + bool powerdown;
> + int ret;
> +
> + ret = kstrtobool(buf, &powerdown);
> + if (ret)
> + return ret;
> +
> + if (powerdown)
> + ret = regmap_update_bits(st->regmap, MAX22007_CHANNEL_MODE_REG,
> + MAX22007_CH_PWRON_CH_MASK(chan->channel),
> + MAX22007_CH_PWR_VAL(chan->channel, 0));
> + else
> + ret = regmap_update_bits(st->regmap, MAX22007_CHANNEL_MODE_REG,
> + MAX22007_CH_PWRON_CH_MASK(chan->channel),
> + MAX22007_CH_PWR_VAL(chan->channel, 1));
> + if (ret)
> + return ret;
> +
Something like the following reduces duplication:
ret = regmap_update_bits(st->regmap, MAX22007_CHANNEL_MODE_REG,
MAX2207_CH_PWRON_CH_MASK(chan->channel),
MAX2207_CH_PWR_VAL(chan->channel, powerdown ? 1 : 0);
> + return len;
> +}
Overall, this is looking much better than previous versions.
Here comes another round of review for this.
On 01/08, Janani Sunil wrote:
> Add support for the MAX22007, a 4-channel 12-bit DAC that drives
> voltage or current output on each channel.
>
> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
> ---
> MAINTAINERS | 1 +
> drivers/iio/dac/Kconfig | 13 ++
> drivers/iio/dac/Makefile | 1 +
> drivers/iio/dac/max22007.c | 507 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 522 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e1addbd21562..99dd3c947629 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1599,6 +1599,7 @@ L: linux-iio@vger.kernel.org
> S: Supported
> W: https://ez.analog.com/linux-software-drivers
> F: Documentation/devicetree/bindings/iio/dac/adi,max22007.yaml
> +F: drivers/iio/dac/max22007.c
>
> ANALOG DEVICES INC ADA4250 DRIVER
> M: Antoniu Miclaus <antoniu.miclaus@analog.com>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 7cd3caec1262..4a31993f5b14 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -482,6 +482,19 @@ config MAX517
> This driver can also be built as a module. If so, the module
> will be called max517.
>
> +config MAX22007
> + tristate "Analog Devices MAX22007 DAC Driver"
> + depends on SPI
> + select REGMAP_SPI
> + select CRC8
> + help
> + Say Y here if you want to build a driver for Analog Devices MAX22007.
> +
> + MAX22007 is a quad-channel, 12-bit, voltage-output digital to
> + analog converter (DAC) with SPI interface.
> +
> + If compiled as a module, it will be called max22007.
> +
> config MAX5522
> tristate "Maxim MAX5522 DAC driver"
> depends on SPI_MASTER
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index e6ac4c67e337..0bbc6d09d22c 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_LTC2664) += ltc2664.o
> obj-$(CONFIG_LTC2688) += ltc2688.o
> obj-$(CONFIG_M62332) += m62332.o
> obj-$(CONFIG_MAX517) += max517.o
> +obj-$(CONFIG_MAX22007) += max22007.o
> obj-$(CONFIG_MAX5522) += max5522.o
> obj-$(CONFIG_MAX5821) += max5821.o
> obj-$(CONFIG_MCP4725) += mcp4725.o
> diff --git a/drivers/iio/dac/max22007.c b/drivers/iio/dac/max22007.c
> new file mode 100644
> index 000000000000..19557c008554
> --- /dev/null
> +++ b/drivers/iio/dac/max22007.c
> @@ -0,0 +1,507 @@
...
> +static u8 max22007_crc8_table[256];
Can use CRC8_TABLE_SIZE to define the table size.
...
> +
> +static int max22007_spi_read(void *context, const void *reg, size_t reg_size,
> + void *val, size_t val_size)
> +{
> + struct max22007_state *st = context;
> + u8 reg_byte = *(u8 *)reg;
Odd casting. Not sure the const qualifier is needed for the reg parameter.
See how other IIO drivers implement regmap_bus. ad7091r8 is one I recall from
the top of my mind.
> + u8 calculated_crc, received_crc;
> + u8 crc_data[3];
> + u8 rx_buf[4];
> + int ret;
> +
> + if (reg_size != 1)
> + return -EINVAL;
> +
> + ret = spi_write_then_read(st->spi, ®_byte, 1, rx_buf,
> + val_size + MAX22007_CRC_OVERHEAD);
> + if (ret) {
> + dev_err(&st->spi->dev, "SPI transfer failed: %d\n", ret);
> + return ret;
> + }
...
> +static int max22007_read_channel_data(struct max22007_state *state,
> + unsigned int channel, int *data)
> +{
> + int ret;
> + unsigned int reg_val;
nitpicking: why not following reverse xmas tree convection here too?
unsigned int reg_val;
int ret;
> +
> + ret = regmap_read(state->regmap, MAX22007_DAC_CHANNEL_REG(channel), ®_val);
> + if (ret)
> + return ret;
> +
> + *data = FIELD_GET(MAX22007_DAC_DATA_MASK, reg_val);
> +
> + return 0;
> +}
> +
> +static int max22007_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct max22007_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = max22007_read_channel_data(st, chan->channel, val);
> + if (ret)
> + return ret;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + if (chan->type == IIO_VOLTAGE)
> + *val = 5 * 2500; /* 5 * Vref(2.5V) in mV */
Interesting that the external reference (if provided) is also expected to be 2.5 V.
I'd set a define for that, e.g.
#define MAX22007_REF_mV 2500
Btw, what is that 5 in '5 * 2500'?
> + else
> + *val = 25; /* Vref / (2 * Rsense) = 2500mV / 100 */
> + *val2 = 12; /* 12-bit DAC resolution */
> + return IIO_VAL_FRACTIONAL_LOG2;
> + default:
> + return -EINVAL;
> + }
> +}
> +
...
> +
> +static int max22007_parse_channel_cfg(struct max22007_state *st, u8 *num_channels)
> +{
> + struct device *dev = &st->spi->dev;
> + int ret, num_chan;
> + int i = 0;
> + u32 reg;
> +
> + num_chan = device_get_child_node_count(dev);
> + if (!num_chan)
> + return dev_err_probe(dev, -ENODEV, "no channels configured\n");
> +
> + st->iio_chans = devm_kcalloc(dev, num_chan, sizeof(*st->iio_chans), GFP_KERNEL);
> + if (!st->iio_chans)
> + return -ENOMEM;
> +
> + device_for_each_child_node_scoped(dev, child) {
> + u32 ch_func;
> + enum max22007_channel_mode mode;
> + enum iio_chan_type chan_type;
> +
> + ret = fwnode_property_read_u32(child, "reg", ®);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to read reg property of %pfwP\n", child);
> +
> + if (reg >= MAX22007_NUM_CHANNELS)
> + return dev_err_probe(dev, -EINVAL,
> + "reg out of range in %pfwP\n", child);
> +
> + ret = fwnode_property_read_u32(child, "adi,ch-func", &ch_func);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "missing adi,ch-func property for %pfwP\n", child);
> +
> + if (ch_func == 1) {
> + mode = MAX22007_VOLTAGE_MODE;
> + chan_type = IIO_VOLTAGE;
> + } else if (ch_func == 2) {
> + mode = MAX22007_CURRENT_MODE;
> + chan_type = IIO_CURRENT;
> + } else {
> + return dev_err_probe(dev, -EINVAL,
> + "invalid adi,ch-func %u for %pfwP\n",
> + ch_func, child);
> + }
> +
> + st->iio_chans[i++] = (struct iio_chan_spec) {
> + .output = 1,
> + .indexed = 1,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .ext_info = max22007_ext_info,
> + .channel = reg,
> + .type = chan_type,
> + };
IMHO, this would look cleaner with a channel template. See ad7124, ad4130,
ad4170 for examples.
> +
> + ret = regmap_update_bits(st->regmap, MAX22007_CHANNEL_MODE_REG,
> + MAX22007_CH_MODE_CH_MASK(reg),
> + MAX22007_CH_MODE_VAL(reg, mode));
> + if (ret)
> + return ret;
> +
> + /* Set DAC to transparent mode (immediate update) */
> + ret = regmap_update_bits(st->regmap, MAX22007_CONFIG_REG,
> + MAX22007_DAC_LATCH_MODE_MASK(reg),
> + MAX22007_DAC_LATCH_MODE_VAL(reg, 1));
> + if (ret)
> + return ret;
> + }
> +
> + *num_channels = num_chan;
> +
> + return 0;
> +}
> +
...
> +static int max22007_probe(struct spi_device *spi)
> +{
> + struct device *dev = &spi->dev;
> + struct iio_dev *indio_dev;
> + struct max22007_state *state;
> + struct gpio_desc *reset_gpio;
> + u8 num_channels;
> + int ret, i;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + state = iio_priv(indio_dev);
> + state->spi = spi;
> +
> + crc8_populate_lsb(max22007_crc8_table, MAX22007_CRC8_POLYNOMIAL);
> +
> + state->regmap = devm_regmap_init(dev, &max22007_regmap_bus, state,
> + &max22007_regmap_config);
> + if (IS_ERR(state->regmap))
> + return dev_err_probe(dev, PTR_ERR(state->regmap),
> + "Failed to initialize regmap\n");
> +
> + for (i = 0; i < MAX22007_NUM_SUPPLIES; i++)
> + state->supplies[i].supply = max22007_supply_names[i];
> +
> + ret = devm_regulator_bulk_get(dev, MAX22007_NUM_SUPPLIES, state->supplies);
devm_regulator_bulk_get_enable(), so max22007_regulator_disable() and
state->supplies won't be needed anymore.
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to get regulators\n");
> +
> + ret = regulator_bulk_enable(MAX22007_NUM_SUPPLIES, state->supplies);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to enable regulators\n");
> +
> + ret = devm_add_action_or_reset(dev, max22007_regulator_disable, state);
> + if (ret)
> + return ret;
> +
> + reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(reset_gpio))
> + return dev_err_probe(dev, PTR_ERR(reset_gpio),
> + "Failed to get reset GPIO\n");
I'm not against the conventional way we've been getting and using reset
GPIOs but, if other reviewers request to use the reset framework, you may do so
with devm_reset_control_get_optional_exclusive_deasserted(dev, NULL).
See [1] for an example (if needed).
[1]: https://lore.kernel.org/linux-iio/6ae8e203f6fb6e9718271132bd35daef790ab574.1767795849.git.marcelo.schmitt@analog.com/
> +
> + if (reset_gpio) {
> + usleep_range(1000, 5000);
> + gpiod_set_value_cansleep(reset_gpio, 1);
> + usleep_range(1000, 5000);
> + } else {
> + ret = regmap_write(state->regmap, MAX22007_SOFT_RESET_REG,
> + MAX22007_SOFT_RESET_BITS_MASK);
> + if (ret)
> + return ret;
> + }
Hi Marcelo,
Thank you for reviewing the patch.
On 1/9/26 15:11, Marcelo Schmitt wrote:
> Overall, this is looking much better than previous versions.
> Here comes another round of review for this.
>
> On 01/08, Janani Sunil wrote:
>> Add support for the MAX22007, a 4-channel 12-bit DAC that drives
>> voltage or current output on each channel.
>>
>> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
>> ---
>> MAINTAINERS | 1 +
>> drivers/iio/dac/Kconfig | 13 ++
>> drivers/iio/dac/Makefile | 1 +
>> drivers/iio/dac/max22007.c | 507 +++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 522 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index e1addbd21562..99dd3c947629 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1599,6 +1599,7 @@ L: linux-iio@vger.kernel.org
>> S: Supported
>> W: https://ez.analog.com/linux-software-drivers
>> F: Documentation/devicetree/bindings/iio/dac/adi,max22007.yaml
>> +F: drivers/iio/dac/max22007.c
>>
>> ANALOG DEVICES INC ADA4250 DRIVER
>> M: Antoniu Miclaus <antoniu.miclaus@analog.com>
>> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
>> index 7cd3caec1262..4a31993f5b14 100644
>> --- a/drivers/iio/dac/Kconfig
>> +++ b/drivers/iio/dac/Kconfig
>> @@ -482,6 +482,19 @@ config MAX517
>> This driver can also be built as a module. If so, the module
>> will be called max517.
>>
>> +config MAX22007
>> + tristate "Analog Devices MAX22007 DAC Driver"
>> + depends on SPI
>> + select REGMAP_SPI
>> + select CRC8
>> + help
>> + Say Y here if you want to build a driver for Analog Devices MAX22007.
>> +
>> + MAX22007 is a quad-channel, 12-bit, voltage-output digital to
>> + analog converter (DAC) with SPI interface.
>> +
>> + If compiled as a module, it will be called max22007.
>> +
>> config MAX5522
>> tristate "Maxim MAX5522 DAC driver"
>> depends on SPI_MASTER
>> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
>> index e6ac4c67e337..0bbc6d09d22c 100644
>> --- a/drivers/iio/dac/Makefile
>> +++ b/drivers/iio/dac/Makefile
>> @@ -48,6 +48,7 @@ obj-$(CONFIG_LTC2664) += ltc2664.o
>> obj-$(CONFIG_LTC2688) += ltc2688.o
>> obj-$(CONFIG_M62332) += m62332.o
>> obj-$(CONFIG_MAX517) += max517.o
>> +obj-$(CONFIG_MAX22007) += max22007.o
>> obj-$(CONFIG_MAX5522) += max5522.o
>> obj-$(CONFIG_MAX5821) += max5821.o
>> obj-$(CONFIG_MCP4725) += mcp4725.o
>> diff --git a/drivers/iio/dac/max22007.c b/drivers/iio/dac/max22007.c
>> new file mode 100644
>> index 000000000000..19557c008554
>> --- /dev/null
>> +++ b/drivers/iio/dac/max22007.c
>> @@ -0,0 +1,507 @@
> ...
>> +static u8 max22007_crc8_table[256];
> Can use CRC8_TABLE_SIZE to define the table size.
Will take this up.
>
> ...
>> +
>> +static int max22007_spi_read(void *context, const void *reg, size_t reg_size,
>> + void *val, size_t val_size)
>> +{
>> + struct max22007_state *st = context;
>> + u8 reg_byte = *(u8 *)reg;
> Odd casting. Not sure the const qualifier is needed for the reg parameter.
> See how other IIO drivers implement regmap_bus. ad7091r8 is one I recall from
> the top of my mind.
Noted. Will update this.
>> + u8 calculated_crc, received_crc;
>> + u8 crc_data[3];
>> + u8 rx_buf[4];
>> + int ret;
>> +
>> + if (reg_size != 1)
>> + return -EINVAL;
>> +
>> + ret = spi_write_then_read(st->spi, ®_byte, 1, rx_buf,
>> + val_size + MAX22007_CRC_OVERHEAD);
>> + if (ret) {
>> + dev_err(&st->spi->dev, "SPI transfer failed: %d\n", ret);
>> + return ret;
>> + }
> ...
>
>> +static int max22007_read_channel_data(struct max22007_state *state,
>> + unsigned int channel, int *data)
>> +{
>> + int ret;
>> + unsigned int reg_val;
> nitpicking: why not following reverse xmas tree convection here too?
>
> unsigned int reg_val;
> int ret;
Shall follow this format.
>
>> +
>> + ret = regmap_read(state->regmap, MAX22007_DAC_CHANNEL_REG(channel), ®_val);
>> + if (ret)
>> + return ret;
>> +
>> + *data = FIELD_GET(MAX22007_DAC_DATA_MASK, reg_val);
>> +
>> + return 0;
>> +}
>> +
>> +static int max22007_read_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan,
>> + int *val, int *val2, long mask)
>> +{
>> + struct max22007_state *st = iio_priv(indio_dev);
>> + int ret;
>> +
>> + switch (mask) {
>> + case IIO_CHAN_INFO_RAW:
>> + ret = max22007_read_channel_data(st, chan->channel, val);
>> + if (ret)
>> + return ret;
>> + return IIO_VAL_INT;
>> + case IIO_CHAN_INFO_SCALE:
>> + if (chan->type == IIO_VOLTAGE)
>> + *val = 5 * 2500; /* 5 * Vref(2.5V) in mV */
> Interesting that the external reference (if provided) is also expected to be 2.5 V.
> I'd set a define for that, e.g.
> #define MAX22007_REF_mV 2500
> Btw, what is that 5 in '5 * 2500'
Will add a macro for the reference voltage.
5 is the gain coefficient in the output stage (driver).
>
>> + else
>> + *val = 25; /* Vref / (2 * Rsense) = 2500mV / 100 */
>> + *val2 = 12; /* 12-bit DAC resolution */
>> + return IIO_VAL_FRACTIONAL_LOG2;
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
> ...
>> +
>> +static int max22007_parse_channel_cfg(struct max22007_state *st, u8 *num_channels)
>> +{
>> + struct device *dev = &st->spi->dev;
>> + int ret, num_chan;
>> + int i = 0;
>> + u32 reg;
>> +
>> + num_chan = device_get_child_node_count(dev);
>> + if (!num_chan)
>> + return dev_err_probe(dev, -ENODEV, "no channels configured\n");
>> +
>> + st->iio_chans = devm_kcalloc(dev, num_chan, sizeof(*st->iio_chans), GFP_KERNEL);
>> + if (!st->iio_chans)
>> + return -ENOMEM;
>> +
>> + device_for_each_child_node_scoped(dev, child) {
>> + u32 ch_func;
>> + enum max22007_channel_mode mode;
>> + enum iio_chan_type chan_type;
>> +
>> + ret = fwnode_property_read_u32(child, "reg", ®);
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "failed to read reg property of %pfwP\n", child);
>> +
>> + if (reg >= MAX22007_NUM_CHANNELS)
>> + return dev_err_probe(dev, -EINVAL,
>> + "reg out of range in %pfwP\n", child);
>> +
>> + ret = fwnode_property_read_u32(child, "adi,ch-func", &ch_func);
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "missing adi,ch-func property for %pfwP\n", child);
>> +
>> + if (ch_func == 1) {
>> + mode = MAX22007_VOLTAGE_MODE;
>> + chan_type = IIO_VOLTAGE;
>> + } else if (ch_func == 2) {
>> + mode = MAX22007_CURRENT_MODE;
>> + chan_type = IIO_CURRENT;
>> + } else {
>> + return dev_err_probe(dev, -EINVAL,
>> + "invalid adi,ch-func %u for %pfwP\n",
>> + ch_func, child);
>> + }
>> +
>> + st->iio_chans[i++] = (struct iio_chan_spec) {
>> + .output = 1,
>> + .indexed = 1,
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> + BIT(IIO_CHAN_INFO_SCALE),
>> + .ext_info = max22007_ext_info,
>> + .channel = reg,
>> + .type = chan_type,
>> + };
> IMHO, this would look cleaner with a channel template. See ad7124, ad4130,
> ad4170 for examples.
A channel template approach was followed initially and this approach was taken up based on the latest suggestions from the reviewers, since the template is not being reused elsewhere.
>
>> +
>> + ret = regmap_update_bits(st->regmap, MAX22007_CHANNEL_MODE_REG,
>> + MAX22007_CH_MODE_CH_MASK(reg),
>> + MAX22007_CH_MODE_VAL(reg, mode));
>> + if (ret)
>> + return ret;
>> +
>> + /* Set DAC to transparent mode (immediate update) */
>> + ret = regmap_update_bits(st->regmap, MAX22007_CONFIG_REG,
>> + MAX22007_DAC_LATCH_MODE_MASK(reg),
>> + MAX22007_DAC_LATCH_MODE_VAL(reg, 1));
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + *num_channels = num_chan;
>> +
>> + return 0;
>> +}
>> +
> ...
>> +static int max22007_probe(struct spi_device *spi)
>> +{
>> + struct device *dev = &spi->dev;
>> + struct iio_dev *indio_dev;
>> + struct max22007_state *state;
>> + struct gpio_desc *reset_gpio;
>> + u8 num_channels;
>> + int ret, i;
>> +
>> + indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
>> + if (!indio_dev)
>> + return -ENOMEM;
>> +
>> + state = iio_priv(indio_dev);
>> + state->spi = spi;
>> +
>> + crc8_populate_lsb(max22007_crc8_table, MAX22007_CRC8_POLYNOMIAL);
>> +
>> + state->regmap = devm_regmap_init(dev, &max22007_regmap_bus, state,
>> + &max22007_regmap_config);
>> + if (IS_ERR(state->regmap))
>> + return dev_err_probe(dev, PTR_ERR(state->regmap),
>> + "Failed to initialize regmap\n");
>> +
>> + for (i = 0; i < MAX22007_NUM_SUPPLIES; i++)
>> + state->supplies[i].supply = max22007_supply_names[i];
>> +
>> + ret = devm_regulator_bulk_get(dev, MAX22007_NUM_SUPPLIES, state->supplies);
> devm_regulator_bulk_get_enable(), so max22007_regulator_disable() and
> state->supplies won't be needed anymore.
Will update the same.
>
>> + if (ret)
>> + return dev_err_probe(dev, ret, "Failed to get regulators\n");
>> +
>> + ret = regulator_bulk_enable(MAX22007_NUM_SUPPLIES, state->supplies);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "Failed to enable regulators\n");
>> +
>> + ret = devm_add_action_or_reset(dev, max22007_regulator_disable, state);
>> + if (ret)
>> + return ret;
>> +
>> + reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
>> + if (IS_ERR(reset_gpio))
>> + return dev_err_probe(dev, PTR_ERR(reset_gpio),
>> + "Failed to get reset GPIO\n");
> I'm not against the conventional way we've been getting and using reset
> GPIOs but, if other reviewers request to use the reset framework, you may do so
> with devm_reset_control_get_optional_exclusive_deasserted(dev, NULL).
> See [1] for an example (if needed).
>
> [1]: https://lore.kernel.org/linux-iio/6ae8e203f6fb6e9718271132bd35daef790ab574.1767795849.git.marcelo.schmitt@analog.com/
Sure.
>
>> +
>> + if (reset_gpio) {
>> + usleep_range(1000, 5000);
>> + gpiod_set_value_cansleep(reset_gpio, 1);
>> + usleep_range(1000, 5000);
>> + } else {
>> + ret = regmap_write(state->regmap, MAX22007_SOFT_RESET_REG,
>> + MAX22007_SOFT_RESET_BITS_MASK);
>> + if (ret)
>> + return ret;
>> + }
Best Regards,
Janani Sunil
> >> + st->iio_chans[i++] = (struct iio_chan_spec) {
> >> + .output = 1,
> >> + .indexed = 1,
> >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> >> + BIT(IIO_CHAN_INFO_SCALE),
> >> + .ext_info = max22007_ext_info,
> >> + .channel = reg,
> >> + .type = chan_type,
> >> + };
> > IMHO, this would look cleaner with a channel template. See ad7124, ad4130,
> > ad4170 for examples.
>
> A channel template approach was followed initially and this approach was taken up based on the latest suggestions from the reviewers, since the template is not being reused elsewhere.
FWIW I prefer this option when it is only used in one place.
Jonathan
© 2016 - 2026 Red Hat, Inc.