* [PATCH v2 0/2] Subject: [PATCH v1 0/3] iio: dac: Add support for MAX22007 DAC
@ 2026-01-08 12:58 Janani Sunil
2026-01-08 12:58 ` [PATCH v2 1/2] dt-bindings: iio: dac: Add max22007 Janani Sunil
2026-01-08 12:58 ` [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support Janani Sunil
0 siblings, 2 replies; 11+ messages in thread
From: Janani Sunil @ 2026-01-08 12:58 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Alexandru Ardelean,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet
Cc: linux-iio, devicetree, linux-kernel, linux-doc, jan.sun97,
Janani Sunil
This patch series introduces support for the Analog Devices MAX22007, a
quad-channel, 12-bit digital-to-analog converter (DAC) with integrated
precision output amplifiers and configurable voltage/current output capability.
**Device Overview:**
The MAX22007 features four independent DAC channels that can each be configured
for either voltage output (0-12.5V) or current output (0-25mA) mode. The device
communicates via SPI interface with built-in CRC8 error checking for data integrity.
**Features Implemented:**
- Support for all 4 DAC channels with 12-bit resolution
- Per-channel voltage/current mode configuration via device tree
property `adi,ch-func = [voltage, current]`
- Independent power control for each channel (attribute)
- Hardware reset support via GPIO (during probe)
- CRC8 error checking for SPI communication
**Patch Summary:**
1. dt-bindings: Binding documentation with channel configuration
2. driver: Implement IIO DAC driver
**Testing:**
The driver was hardware tested on a Raspberry Pi4 on top of v6.12.y
kernel using the MAX22007EVKIT evaluation board.
Janani Sunil (3):
dt-bindings: iio: dac: Add max22007
iio: dac: Add MAX22007 DAC driver support
---
To: Lars-Peter Clausen <lars@metafoo.de>
To: Michael Hennerich <Michael.Hennerich@analog.com>
To: Alexandru Ardelean <alexandru.ardelean@analog.com>
To: Jonathan Cameron <jic23@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Jonathan Corbet <corbet@lwn.net>
Cc: linux-iio@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Cc: jan.sun97@gmail.com
Signed-off-by: Janani Sunil <janani.sunil@analog.com>
---
Changes in v2:
- Wrap commit messages as per coding guidelines
- Removed all driver references from the hardware
- Update property description for reset-gpio
- Removed allOf
- Added minimum/maximum limits for channel number in the devicetree
binding
- Replaced adi,type with adi,ch-func.
- Added reference to required supplies in the binding, configured them
in the driver
- Channels are not a required property anymore.
- Replaced instances of 'channel' in macros to just 'ch'
- Added trailing commas wherever necessary, removed them as per comments
- Add explicit values for enum- max22007_channel_power
- Replace channel spec structure member 'iio_chan' with 'iio_chans'
- Use spi_write_then_read() API in the max22007_spi_read() API
- Check for reg_size ==1 and hardcode the size otherwise
- Wrap lines in the driver to 80 characters
- Update in-line comment on the resolution
- Separate declarations with assignment, from the ones that don't
- Update the usage of channel template
- Add a local device descriptor to point to the SPI device
- Add a transition of the Reset GPIO from low to high in the probe
- Make use of regmap_set_bits() instead of regmap_update_bits during CRC
Enable function call.
- Remove the documentation commit, as it is not needed anymore.
- Link to v1: https://lore.kernel.org/r/20251219-max22007-dev-v1-0-242da2c2b868@analog.com
---
Janani Sunil (2):
dt-bindings: iio: dac: Add max22007
iio: dac: Add MAX22007 DAC driver support
.../devicetree/bindings/iio/dac/adi,max22007.yaml | 136 ++++++
MAINTAINERS | 8 +
drivers/iio/dac/Kconfig | 13 +
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/max22007.c | 507 +++++++++++++++++++++
5 files changed, 665 insertions(+)
---
base-commit: a7b10f0963c651a6406d958a5f64b9c5594f84da
change-id: 20251219-max22007-dev-1aaf08db7890
Best regards,
--
Janani Sunil <janani.sunil@analog.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] dt-bindings: iio: dac: Add max22007
2026-01-08 12:58 [PATCH v2 0/2] Subject: [PATCH v1 0/3] iio: dac: Add support for MAX22007 DAC Janani Sunil
@ 2026-01-08 12:58 ` Janani Sunil
2026-01-09 8:26 ` Krzysztof Kozlowski
2026-01-09 12:44 ` Marcelo Schmitt
2026-01-08 12:58 ` [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support Janani Sunil
1 sibling, 2 replies; 11+ messages in thread
From: Janani Sunil @ 2026-01-08 12:58 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Alexandru Ardelean,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet
Cc: linux-iio, devicetree, linux-kernel, linux-doc, jan.sun97,
Janani Sunil
Devicetree bindings for MAX22007 4-channel 12-bit DAC that drives a
voltage or current output on each channel
Signed-off-by: Janani Sunil <janani.sunil@analog.com>
---
.../devicetree/bindings/iio/dac/adi,max22007.yaml | 136 +++++++++++++++++++++
MAINTAINERS | 7 ++
2 files changed, 143 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,max22007.yaml b/Documentation/devicetree/bindings/iio/dac/adi,max22007.yaml
new file mode 100644
index 000000000000..52c7c3217f90
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,max22007.yaml
@@ -0,0 +1,136 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,max22007.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices MAX22007 DAC
+
+maintainers:
+ - Janani Sunil <janani.sunil@analog.com>
+
+description:
+ The MAX22007 is a quad-channel, 12-bit digital-to-analog converter (DAC)
+ with integrated precision output amplifiers and current output capability.
+ Each channel can be independently configured for voltage or current output.
+ Datasheet available at https://www.analog.com/en/products/max22007.html
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+ const: adi,max22007
+
+ reg:
+ maxItems: 1
+
+ spi-max-frequency:
+ maximum: 500000
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ vdd-supply:
+ description: Low-Voltage Power Supply from +2.7V to +5.5V.
+
+ hvdd-supply:
+ description:
+ Positive High-Voltage Power Supply from +8V to (HVSS +24V) for
+ the Output Channels.
+
+ hvss-supply:
+ description:
+ Optional Negative High-Voltage Power Supply from -2V to 0V for the Output
+ Channels. For most applications HVSS can be connected to GND (0V), but for
+ applications requiring output down to true 0V or 0mA, connect to a -2V supply.
+
+ reset-gpios:
+ maxItems: 1
+ description:
+ Active low GPIO used for hardware reset.
+
+patternProperties:
+ "^channel@[0-3]$":
+ $ref: /schemas/iio/dac/dac.yaml#
+ type: object
+ description:
+ Represents the external channels which are connected to the DAC.
+
+ properties:
+ reg:
+ description: Channel number
+ items:
+ minimum: 0
+ maximum: 3
+
+ adi,ch-func:
+ description:
+ Channel output type. Use CH_FUNC_VOLTAGE_OUTPUT for voltage
+ output or CH_FUNC_CURRENT_OUTPUT for current output.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [1, 2]
+
+ required:
+ - reg
+ - adi,ch-func
+
+ unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - vdd-supply
+ - hvdd-supply
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/iio/addac/adi,ad74413r.h>
+
+ vdd_reg: regulator-vdd {
+ compatible = "regulator-fixed";
+ regulator-name = "vdd-3v3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+
+ hvdd_reg: regulator-hvdd {
+ compatible = "regulator-fixed";
+ regulator-name = "hvdd-12v";
+ regulator-min-microvolt = <12000000>;
+ regulator-max-microvolt = <12000000>;
+ regulator-always-on;
+ };
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dac@0 {
+ compatible = "adi,max22007";
+ reg = <0>;
+ spi-max-frequency = <500000>;
+ reset-gpios = <&gpio 19 GPIO_ACTIVE_LOW>;
+ vdd-supply = <&vdd_reg>;
+ hvdd-supply = <&hvdd_reg>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@0 {
+ reg = <0>;
+ adi,ch-func = <CH_FUNC_VOLTAGE_OUTPUT>;
+ };
+
+ channel@1 {
+ reg = <1>;
+ adi,ch-func = <CH_FUNC_CURRENT_OUTPUT>;
+ };
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 29340394ac9d..e1addbd21562 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1593,6 +1593,13 @@ W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/dac/adi,ad9739a.yaml
F: drivers/iio/dac/ad9739a.c
+ANALOG DEVICES INC MAX22007 DRIVER
+M: Janani Sunil <janani.sunil@analog.com>
+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
+
ANALOG DEVICES INC ADA4250 DRIVER
M: Antoniu Miclaus <antoniu.miclaus@analog.com>
L: linux-iio@vger.kernel.org
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support
2026-01-08 12:58 [PATCH v2 0/2] Subject: [PATCH v1 0/3] iio: dac: Add support for MAX22007 DAC Janani Sunil
2026-01-08 12:58 ` [PATCH v2 1/2] dt-bindings: iio: dac: Add max22007 Janani Sunil
@ 2026-01-08 12:58 ` Janani Sunil
2026-01-09 14:11 ` Marcelo Schmitt
` (2 more replies)
1 sibling, 3 replies; 11+ messages in thread
From: Janani Sunil @ 2026-01-08 12:58 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Alexandru Ardelean,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet
Cc: linux-iio, devicetree, linux-kernel, linux-doc, jan.sun97,
Janani Sunil
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.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: iio: dac: Add max22007
2026-01-08 12:58 ` [PATCH v2 1/2] dt-bindings: iio: dac: Add max22007 Janani Sunil
@ 2026-01-09 8:26 ` Krzysztof Kozlowski
2026-01-09 12:44 ` Marcelo Schmitt
1 sibling, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2026-01-09 8:26 UTC (permalink / raw)
To: Janani Sunil
Cc: Lars-Peter Clausen, Michael Hennerich, Alexandru Ardelean,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc,
jan.sun97
On Thu, Jan 08, 2026 at 01:58:23PM +0100, Janani Sunil wrote:
> +required:
> + - compatible
> + - reg
> + - vdd-supply
> + - hvdd-supply
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/iio/addac/adi,ad74413r.h>
> +
> + vdd_reg: regulator-vdd {
This was not here, drop entire node.
> + compatible = "regulator-fixed";
> + regulator-name = "vdd-3v3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> + };
> +
> + hvdd_reg: regulator-hvdd {
Same here.
With these two fixed:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: iio: dac: Add max22007
2026-01-08 12:58 ` [PATCH v2 1/2] dt-bindings: iio: dac: Add max22007 Janani Sunil
2026-01-09 8:26 ` Krzysztof Kozlowski
@ 2026-01-09 12:44 ` Marcelo Schmitt
2026-01-09 14:08 ` Janani Sunil
1 sibling, 1 reply; 11+ messages in thread
From: Marcelo Schmitt @ 2026-01-09 12:44 UTC (permalink / raw)
To: Janani Sunil
Cc: Lars-Peter Clausen, Michael Hennerich, Alexandru Ardelean,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc,
jan.sun97
Hi Janani,
One extra comment in addition to Krzysztof's.
On 01/08, Janani Sunil wrote:
> Devicetree bindings for MAX22007 4-channel 12-bit DAC that drives a
> voltage or current output on each channel
>
> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
> ---
...
> +
> +patternProperties:
> + "^channel@[0-3]$":
> + $ref: /schemas/iio/dac/dac.yaml#
> + type: object
> + description:
> + Represents the external channels which are connected to the DAC.
> +
> + properties:
> + reg:
> + description: Channel number
> + items:
> + minimum: 0
> + maximum: 3
> +
> + adi,ch-func:
> + description:
> + Channel output type. Use CH_FUNC_VOLTAGE_OUTPUT for voltage
> + output or CH_FUNC_CURRENT_OUTPUT for current output.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [1, 2]
adi,ad74413r.yaml has many possibilities for the channel output type.
max22007 is only either voltage or current.
Can't we do this with output-range-microamp and output-range-microvolt from dac.yaml?
Figure out the channel type from the output-range- property?
> +
> + required:
> + - reg
> + - adi,ch-func
> +
> + unevaluatedProperties: false
With best regards,
Marcelo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: iio: dac: Add max22007
2026-01-09 12:44 ` Marcelo Schmitt
@ 2026-01-09 14:08 ` Janani Sunil
0 siblings, 0 replies; 11+ messages in thread
From: Janani Sunil @ 2026-01-09 14:08 UTC (permalink / raw)
To: Marcelo Schmitt, Janani Sunil
Cc: Lars-Peter Clausen, Michael Hennerich, Alexandru Ardelean,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc,
jan.sun97
Hi Marcelo,
Thank you for the suggestion.
However, reusing the output-range-microamp and output-range-microvolt properties might mislead the users into thinking that they can configure the DAC output range, which the device does not support. The adi,ch-func fits better here.
On 1/9/26 13:44, Marcelo Schmitt wrote:
> Hi Janani,
>
> One extra comment in addition to Krzysztof's.
>
> On 01/08, Janani Sunil wrote:
>> Devicetree bindings for MAX22007 4-channel 12-bit DAC that drives a
>> voltage or current output on each channel
>>
>> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
>> ---
> ...
>> +
>> +patternProperties:
>> + "^channel@[0-3]$":
>> + $ref: /schemas/iio/dac/dac.yaml#
>> + type: object
>> + description:
>> + Represents the external channels which are connected to the DAC.
>> +
>> + properties:
>> + reg:
>> + description: Channel number
>> + items:
>> + minimum: 0
>> + maximum: 3
>> +
>> + adi,ch-func:
>> + description:
>> + Channel output type. Use CH_FUNC_VOLTAGE_OUTPUT for voltage
>> + output or CH_FUNC_CURRENT_OUTPUT for current output.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + enum: [1, 2]
> adi,ad74413r.yaml has many possibilities for the channel output type.
> max22007 is only either voltage or current.
> Can't we do this with output-range-microamp and output-range-microvolt from dac.yaml?
> Figure out the channel type from the output-range- property?
>
>> +
>> + required:
>> + - reg
>> + - adi,ch-func
>> +
>> + unevaluatedProperties: false
> With best regards,
> Marcelo
Best Regards,
Janani Sunil
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support
2026-01-08 12:58 ` [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support Janani Sunil
@ 2026-01-09 14:11 ` Marcelo Schmitt
2026-01-09 15:36 ` Janani Sunil
2026-01-11 16:09 ` Jonathan Cameron
2026-01-12 20:26 ` Jonathan Cameron
2 siblings, 1 reply; 11+ messages in thread
From: Marcelo Schmitt @ 2026-01-09 14:11 UTC (permalink / raw)
To: Janani Sunil
Cc: Lars-Peter Clausen, Michael Hennerich, Alexandru Ardelean,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc,
jan.sun97
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;
> + }
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support
2026-01-09 14:11 ` Marcelo Schmitt
@ 2026-01-09 15:36 ` Janani Sunil
2026-01-11 15:56 ` Jonathan Cameron
0 siblings, 1 reply; 11+ messages in thread
From: Janani Sunil @ 2026-01-09 15:36 UTC (permalink / raw)
To: Marcelo Schmitt, Janani Sunil
Cc: Lars-Peter Clausen, Michael Hennerich, Alexandru Ardelean,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support
2026-01-09 15:36 ` Janani Sunil
@ 2026-01-11 15:56 ` Jonathan Cameron
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2026-01-11 15:56 UTC (permalink / raw)
To: Janani Sunil
Cc: Marcelo Schmitt, Janani Sunil, Lars-Peter Clausen,
Michael Hennerich, Alexandru Ardelean, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, linux-iio,
devicetree, linux-kernel, linux-doc
> >> + 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support
2026-01-08 12:58 ` [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support Janani Sunil
2026-01-09 14:11 ` Marcelo Schmitt
@ 2026-01-11 16:09 ` Jonathan Cameron
2026-01-12 20:26 ` Jonathan Cameron
2 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2026-01-11 16:09 UTC (permalink / raw)
To: Janani Sunil
Cc: Lars-Peter Clausen, Michael Hennerich, Alexandru Ardelean,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
linux-iio, devicetree, linux-kernel, linux-doc, jan.sun97
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;
> +}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support
2026-01-08 12:58 ` [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support Janani Sunil
2026-01-09 14:11 ` Marcelo Schmitt
2026-01-11 16:09 ` Jonathan Cameron
@ 2026-01-12 20:26 ` Jonathan Cameron
2 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2026-01-12 20:26 UTC (permalink / raw)
To: Janani Sunil
Cc: Lars-Peter Clausen, Michael Hennerich, Alexandru Ardelean,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
linux-iio, devicetree, linux-kernel, linux-doc, jan.sun97
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>
> 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);
> +}
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-01-12 20:26 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-08 12:58 [PATCH v2 0/2] Subject: [PATCH v1 0/3] iio: dac: Add support for MAX22007 DAC Janani Sunil
2026-01-08 12:58 ` [PATCH v2 1/2] dt-bindings: iio: dac: Add max22007 Janani Sunil
2026-01-09 8:26 ` Krzysztof Kozlowski
2026-01-09 12:44 ` Marcelo Schmitt
2026-01-09 14:08 ` Janani Sunil
2026-01-08 12:58 ` [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support Janani Sunil
2026-01-09 14:11 ` Marcelo Schmitt
2026-01-09 15:36 ` Janani Sunil
2026-01-11 15:56 ` Jonathan Cameron
2026-01-11 16:09 ` Jonathan Cameron
2026-01-12 20:26 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox