[PATCH v2 0/2] Subject: [PATCH v1 0/3] iio: dac: Add support for MAX22007 DAC

[PATCH v2 0/2] Subject: [PATCH v1 0/3] iio: dac: Add support for MAX22007 DAC

[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>

[PATCH v2 1/2] dt-bindings: iio: dac: Add max22007

[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

Re: [PATCH v2 1/2] dt-bindings: iio: dac: Add max22007

[Krzysztof Kozlowski]

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

Re: [PATCH v2 1/2] dt-bindings: iio: dac: Add max22007

[Marcelo Schmitt]

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

Re: [PATCH v2 1/2] dt-bindings: iio: dac: Add max22007

[Janani Sunil]

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

[PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support

[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, &reg_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), &reg_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, &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", &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

Re: [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support

[Marcelo Schmitt]

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, &reg_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), &reg_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", &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;
> +	}

Re: [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support

[Janani Sunil]

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, &reg_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), &reg_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", &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

Re: [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support

[Jonathan Cameron]

> >> +		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

Re: [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support

[Jonathan Cameron]

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, &reg_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, &reg_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;
> +}

Re: [PATCH v2 2/2] iio: dac: Add MAX22007 DAC driver support

[Jonathan Cameron]

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, &reg_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);
> +}
>