[PATCH v2 0/4] Add driver for AD3530R and AD3531R DACs

[PATCH v2 0/4] Add driver for AD3530R and AD3531R DACs

[Kim Seer Paller]

The AD3530R/AD3530 is an 8-Channel, 16-Bit Voltage Output DAC, while the
AD3531R/AD3531 is a 4-Channel, 16-Bit Voltage Output DAC. These devices
include software-programmable gain controls that provide full-scale
output spans of 2.5V or 5V for reference voltages of 2.5V. They operate
from a single supply voltage range of 2.7V to 5.5V and are guaranteed to
be monotonic by design. Additionally, these devices features a 2.5V,
5ppm/°C internal reference, which is disabled by default.

The AD3531R/AD3531 is not yet released, so the only available datasheet
for now is the AD3530R/AD3530. The only differences between the two is
the number of channels, and register addresses of some registers.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
Changes in v2:
Bindings:
- Updated commit message.
- Changed adi,double-output-range to adi,range-double property.

ad3530r:
- Changed data type to __be16 to resolve sparse warnings related to
  type mismatches.

- Link to v1: https://lore.kernel.org/r/20250319-togreg-v1-0-d8244a502f2c@analog.com

---
Kim Seer Paller (4):
      iio: ABI: add new DAC powerdown mode
      iio: dac: ad3530r: Add ABI file for the AD3530R DAC
      dt-bindings: iio: dac: Add adi,ad3530r.yaml
      iio: dac: ad3530r: Add driver for AD3530R and AD3531R

 Documentation/ABI/testing/sysfs-bus-iio            |   2 +
 .../ABI/testing/sysfs-bus-iio-dac-ad3530r          |  68 +++
 .../devicetree/bindings/iio/dac/adi,ad3530r.yaml   |  91 ++++
 MAINTAINERS                                        |   9 +
 drivers/iio/dac/Kconfig                            |  11 +
 drivers/iio/dac/Makefile                           |   1 +
 drivers/iio/dac/ad3530r.c                          | 586 +++++++++++++++++++++
 7 files changed, 768 insertions(+)
---
base-commit: 8dbeb413806f9f810d97d25284f585b201aa3bdc
change-id: 20250319-togreg-fc6a0af961ed

Best regards,
-- 
Kim Seer Paller <kimseer.paller@analog.com>

[PATCH v2 1/4] iio: ABI: add new DAC powerdown mode

[Kim Seer Paller]

Add a new powerdown mode for DACs with 7.7kohm and 32kohm resistor
to GND.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 722aa989baac43f694076074b307d134867b4533..85790f943fd858021c75d67375abbd8b2976bb8b 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -740,7 +740,9 @@ Description:
 		1kohm_to_gnd: connected to ground via an 1kOhm resistor,
 		2.5kohm_to_gnd: connected to ground via a 2.5kOhm resistor,
 		6kohm_to_gnd: connected to ground via a 6kOhm resistor,
+		7.7kohm_to_gnd: connected to ground via a 7.7kOhm resistor,
 		20kohm_to_gnd: connected to ground via a 20kOhm resistor,
+		32kohm_to_gnd: connected to ground via a 32kOhm resistor,
 		42kohm_to_gnd: connected to ground via a 42kOhm resistor,
 		90kohm_to_gnd: connected to ground via a 90kOhm resistor,
 		100kohm_to_gnd: connected to ground via an 100kOhm resistor,

-- 
2.34.1

[PATCH v2 2/4] iio: dac: ad3530r: Add ABI file for the AD3530R DAC

[Kim Seer Paller]

Define muxout_select and muxout_select_available sysfs interface for the
AD3530R and AD3531R DAC.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 .../ABI/testing/sysfs-bus-iio-dac-ad3530r          | 68 ++++++++++++++++++++++
 MAINTAINERS                                        |  7 +++
 2 files changed, 75 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ad3530r b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad3530r
new file mode 100644
index 0000000000000000000000000000000000000000..9d3126952fd1c5214afb895c4972dc4a891ed7d4
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad3530r
@@ -0,0 +1,68 @@
+What:		/sys/bus/iio/devices/iio:deviceX/muxout_select
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Selects which of the multiplexer's input signals will be
+		monitored on the MUX_OUT pin.
+		* powered_down - MUX_OUT pin is powered down. An 80kΩ impedance
+				 can be seen at the MUX_OUT pin.
+		* vout0 - Voltage representation of VOUT0.
+		* iout0_source - Voltage representation of IOUT0 (source mode).
+		* iout0_sink - Voltage representation of IOUT0 (sink mode).
+		* vout1 - Voltage representation of VOUT1.
+		* iout1_source - Voltage representation of IOUT1 (source mode).
+		* iout1_sink - Voltage representation of IOUT1 (sink mode).
+		* vout2 - Voltage representation of VOUT2.
+		* iout2_source - Voltage representation of IOUT2 (source mode).
+		* iout2_sink - Voltage representation of IOUT2 (sink mode).
+		* vout3 - Voltage representation of VOUT3
+		* iout3_source - Voltage representation of IOUT3 (source mode).
+		* iout3_sink - Voltage representation of IOUT3 (sink mode).
+		* vout4 - Voltage representation of VOUT4.
+		* iout4_source - Voltage representation of IOUT4 (source mode).
+		* iout4_sink - Voltage representation of IOUT4 (sink mode).
+		* vout5 - Voltage representation of VOUT5.
+		* iout5_source - Voltage representation of IOUT5 (source mode).
+		* iout5_sink - Voltage representation of IOUT5 (sink mode).
+		* vout6 - Voltage representation of VOUT6.
+		* iout6_source - Voltage representation of IOUT6 (source mode).
+		* iout6_sink - Voltage representation of IOUT6 (sink mode).
+		* vout7 - Voltage representation of VOUT7.
+		* iout7_source - Voltage representation of IOUT7 (source mode).
+		* iout7_sink - Voltage representation of IOUT7 (sink mode).
+		* die_temp - Voltage representation of internal die temperature.
+		* agnd - MUX_OUT pin internally tied to AGND.
+
+What:		/sys/bus/iio/devices/iio:deviceX/muxout_select_available
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Reading this returns the valid values that can be written to the
+		muxout_select attribute:
+		* powered_down
+		* vout0
+		* iout0_source
+		* iout0_sink
+		* vout1
+		* iout1_source
+		* iout1_sink
+		* vout2
+		* iout2_source
+		* iout2_sink
+		* vout3
+		* iout3_source
+		* iout3_sink
+		* vout4
+		* iout4_source
+		* iout4_sink
+		* vout5
+		* iout5_source
+		* iout5_sink
+		* vout6
+		* iout6_source
+		* iout6_sink
+		* vout7
+		* iout7_source
+		* iout7_sink
+		* die_temp
+		* agnd
diff --git a/MAINTAINERS b/MAINTAINERS
index ffdb3f21fc4fb35b349449afbb30fecd4fe72978..2d3c31c74594ca1934c67e7aad0a179feeaa39bf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1289,6 +1289,13 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
 F:	drivers/net/amt.c
 
+ANALOG DEVICES INC AD3530R DRIVER
+M:	Kim Seer Paller <kimseer.paller@analog.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/ABI/testing/sysfs-bus-iio-dac-ad3530r
+
 ANALOG DEVICES INC AD3552R DRIVER
 M:	Nuno Sá <nuno.sa@analog.com>
 L:	linux-iio@vger.kernel.org

-- 
2.34.1

[PATCH v2 3/4] dt-bindings: iio: dac: Add adi,ad3530r.yaml

[Kim Seer Paller]

Document the AD3530R/AD3530, an 8-Channel, 16-bit Voltage Output DAC,
while the AD3531R/AD3531 is a 4-Channel, 16-Bit Voltage Output DAC.
These devices include software-programmable gain controls that provide
full-scale output spans of 2.5V or 5V for reference voltages of 2.5V.
They operate from a single supply voltage range of 2.7V to 5.5V and are
guaranteed to be monotonic by design. Additionally, these devices
features a 2.5V, 5ppm/°C internal reference, which is disabled by default.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 .../devicetree/bindings/iio/dac/adi,ad3530r.yaml   | 91 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 92 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..e581472b50048bedda7422748035423b9b020382
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ad3530r.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD3530R and Similar DACs
+
+maintainers:
+  - Kim Seer Paller <kimseer.paller@analog.com>
+
+description: |
+  The AD3530/AD3530R are low power, 8-channel, 16-bit, buffered voltage output,
+  digital-to-analog converters (DACs) that include software-programmable gain
+  controls that result in full-scale output spans of 2.5V or 5V for reference
+  voltages of 2.5V. The devices operate from single, 2.7V to 5.5V supply ranges
+  and are guaranteed monotonic by design. The AD3530R also offers a 2.5V,
+  5ppm/°C internal reference that is disabled by default.
+  Datasheet can be found here:
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad3530_ad530r.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,ad3530r
+      - adi,ad3531r
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 50000000
+
+  vdd-supply:
+    description: Power Supply Input.
+
+  iovdd-supply:
+    description: Digital Power Supply Input.
+
+  ref-supply:
+    description:
+      Reference Input/Output. The voltage at the REF pin sets the full-scale
+      range of all channels. If not provided the internal reference is used and
+      also provided on the VREF pin.
+
+  reset-gpios:
+    description:
+      Active low signal that is falling edge sensitive. When it is deasserted,
+      the digital core initialization is performed and all DAC registers except
+      the Interface Configuration A register are reset to their default values.
+    maxItems: 1
+
+  ldac-gpios:
+    description:
+      LDAC pin to be used as a hardware trigger to update the DAC channels. If
+      not present, the DAC channels are updated by Software LDAC.
+    maxItems: 1
+
+  adi,range-double:
+    description:
+      Configure the output range for all channels. If the property is present,
+      the output will range from 0V to 2Vref. If the property is not
+      present, the output will range from 0V to Vref.
+    type: boolean
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+  - iovdd-supply
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+additionalProperties: false
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        dac@0 {
+            compatible = "adi,ad3530r";
+            reg = <0>;
+            spi-max-frequency = <1000000>;
+
+            vdd-supply = <&vdd>;
+            iovdd-supply = <&iovdd>;
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 2d3c31c74594ca1934c67e7aad0a179feeaa39bf..7862469226f52375adc219115ef68d03662127be 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1295,6 +1295,7 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/ABI/testing/sysfs-bus-iio-dac-ad3530r
+F:	Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
 
 ANALOG DEVICES INC AD3552R DRIVER
 M:	Nuno Sá <nuno.sa@analog.com>

-- 
2.34.1

[PATCH v2 4/4] iio: dac: ad3530r: Add driver for AD3530R and AD3531R

[Kim Seer Paller]

The AD3530R/AD3530 is an 8-Channel, 16-Bit Voltage Output DAC, while the
AD3531R/AD3531 is a 4-Channel, 16-Bit Voltage Output DAC. These devices
include software-programmable gain controls that provide full-scale
output spans of 2.5V or 5V for reference voltages of 2.5V. They operate
from a single supply voltage range of 2.7V to 5.5V and are guaranteed to
be monotonic by design. Additionally, these devices features a 2.5V,
5ppm/°C internal reference, which is disabled by default.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 MAINTAINERS               |   1 +
 drivers/iio/dac/Kconfig   |  11 +
 drivers/iio/dac/Makefile  |   1 +
 drivers/iio/dac/ad3530r.c | 586 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 599 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7862469226f52375adc219115ef68d03662127be..ff99f4fb3d675d6012dc5d30ef4d6b17b11313b4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1296,6 +1296,7 @@ S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/ABI/testing/sysfs-bus-iio-dac-ad3530r
 F:	Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
+F:	drivers/iio/dac/ad3530r.c
 
 ANALOG DEVICES INC AD3552R DRIVER
 M:	Nuno Sá <nuno.sa@analog.com>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 4811ea973125a0dea1f8a9cdee1e0c045bc21981..e0996dc014a3d538ab6b4e0d50ff54ede50f1527 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -6,6 +6,17 @@
 
 menu "Digital to analog converters"
 
+config AD3530R
+	tristate "Analog Devices AD3530R and Similar DACs driver"
+	depends on SPI
+	select REGMAP_SPI
+	help
+	  Say yes here to build support for Analog Devices AD3530R, AD3531R
+	  Digital to Analog Converter.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad3530r.
+
 config AD3552R_HS
 	tristate "Analog Devices AD3552R DAC High Speed driver"
 	select AD3552R_LIB
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 8dd6cce81ed1152be4cf0af9ef877b5482ceb347..3684cd52b7fa9bc0ad9f855323dcbb2e4965c404 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -4,6 +4,7 @@
 #
 
 # When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_AD3530R) += ad3530r.o
 obj-$(CONFIG_AD3552R_HS) += ad3552r-hs.o
 obj-$(CONFIG_AD3552R_LIB) += ad3552r-common.o
 obj-$(CONFIG_AD3552R) += ad3552r.o
diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
new file mode 100644
index 0000000000000000000000000000000000000000..6fe657220f33d7adb41b1307a514f509127ebbe4
--- /dev/null
+++ b/drivers/iio/dac/ad3530r.c
@@ -0,0 +1,586 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AD3530R/AD3530 8-channel, 16-bit Voltage Output DAC Driver
+ * AD3531R/AD3531 4-channel, 16-bit Voltage Output DAC Driver
+ *
+ * Copyright 2025 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#define AD3530R_INTERFACE_CONFIG_A		0x00
+#define AD3530R_OUTPUT_OPERATING_MODE_0		0x20
+#define AD3530R_OUTPUT_OPERATING_MODE_1		0x21
+#define AD3530R_OUTPUT_CONTROL_0		0x2A
+#define AD3530R_REFERENCE_CONTROL_0		0x3C
+#define AD3530R_MUX_OUT_SELECT			0x93
+#define AD3530R_SW_LDAC_TRIG_A			0xE5
+#define AD3530R_INPUT_CH(c)			(2 * (c) + 0xEB)
+
+#define AD3531R_SW_LDAC_TRIG_A			0xDD
+#define AD3531R_INPUT_CH(c)			(2 * (c) + 0xE3)
+
+#define AD3530R_SW_LDAC_TRIG_MASK		BIT(7)
+#define AD3530R_OUTPUT_CONTROL_MASK		BIT(2)
+#define AD3530R_REFERENCE_CONTROL_MASK		BIT(0)
+#define AD3530R_REG_VAL_MASK			GENMASK(15, 0)
+
+#define AD3530R_SW_RESET			(BIT(7) | BIT(0))
+#define AD3530R_MAX_CHANNELS			8
+#define AD3531R_MAX_CHANNELS			4
+#define AD3530R_CH(c)				(c)
+#define AD3530R_32KOHM_POWERDOWN_MODE		3
+#define AD3530R_INTERNAL_VREF_MV		2500
+#define AD3530R_LDAC_PULSE_US			100
+
+enum {
+	AD3530R_MUXOUT_POWERED_DOWN,
+	AD3530R_MUXOUT_VOUT0,
+	AD3530R_MUXOUT_IOUT0_SOURCE,
+	AD3530R_MUXOUT_IOUT0_SINK,
+	AD3530R_MUXOUT_VOUT1,
+	AD3530R_MUXOUT_IOUT1_SOURCE,
+	AD3530R_MUXOUT_IOUT1_SINK,
+	AD3530R_MUXOUT_VOUT2,
+	AD3530R_MUXOUT_IOUT2_SOURCE,
+	AD3530R_MUXOUT_IOUT2_SINK,
+	AD3530R_MUXOUT_VOUT3,
+	AD3530R_MUXOUT_IOUT3_SOURCE,
+	AD3530R_MUXOUT_IOUT3_SINK,
+	AD3530R_MUXOUT_VOUT4,
+	AD3530R_MUXOUT_IOUT4_SOURCE,
+	AD3530R_MUXOUT_IOUT4_SINK,
+	AD3530R_MUXOUT_VOUT5,
+	AD3530R_MUXOUT_IOUT5_SOURCE,
+	AD3530R_MUXOUT_IOUT5_SINK,
+	AD3530R_MUXOUT_VOUT6,
+	AD3530R_MUXOUT_IOUT6_SOURCE,
+	AD3530R_MUXOUT_IOUT6_SINK,
+	AD3530R_MUXOUT_VOUT7,
+	AD3530R_MUXOUT_IOUT7_SOURCE,
+	AD3530R_MUXOUT_IOUT7_SINK,
+	AD3530R_MUXOUT_DIE_TEMP,
+	AD3530R_MUXOUT_AGND,
+};
+
+struct ad3530r_chan {
+	unsigned int powerdown_mode;
+	bool powerdown;
+};
+
+struct ad3530r_chip_info {
+	const char *name;
+	const struct iio_chan_spec *channels;
+	int (*input_ch_reg)(unsigned int c);
+	const int iio_chan;
+	unsigned int num_channels;
+	unsigned int sw_ldac_trig_reg;
+};
+
+struct ad3530r_state {
+	struct spi_device *spi;
+	struct regmap *regmap;
+	struct mutex lock; /* protect the state of the device */
+	struct ad3530r_chan chan[AD3530R_MAX_CHANNELS];
+	const struct ad3530r_chip_info *chip_info;
+	struct gpio_desc *ldac_gpio;
+	int vref_mv;
+	u8 ldac;
+	bool range_multiplier;
+};
+
+static int ad3530r_input_ch_reg(unsigned int c)
+{
+	return AD3530R_INPUT_CH(c);
+}
+
+static int ad3531r_input_ch_reg(unsigned int c)
+{
+	return AD3531R_INPUT_CH(c);
+}
+
+static const char * const ad3530r_powerdown_modes[] = {
+	"1kohm_to_gnd",
+	"7.7kohm_to_gnd",
+	"32kohm_to_gnd",
+};
+
+static int ad3530r_get_powerdown_mode(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+
+	guard(mutex)(&st->lock);
+	return st->chan[chan->channel].powerdown_mode - 1;
+}
+
+static int ad3530r_set_powerdown_mode(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      unsigned int mode)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+
+	guard(mutex)(&st->lock);
+	st->chan[chan->channel].powerdown_mode = mode + 1;
+
+	return 0;
+}
+
+static const struct iio_enum ad3530r_powerdown_mode_enum = {
+	.items = ad3530r_powerdown_modes,
+	.num_items = ARRAY_SIZE(ad3530r_powerdown_modes),
+	.get = ad3530r_get_powerdown_mode,
+	.set = ad3530r_set_powerdown_mode,
+};
+
+static ssize_t ad3530r_get_dac_powerdown(struct iio_dev *indio_dev,
+					 uintptr_t private,
+					 const struct iio_chan_spec *chan,
+					 char *buf)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+
+	guard(mutex)(&st->lock);
+	return sysfs_emit(buf, "%d\n", st->chan[chan->channel].powerdown);
+}
+
+static ssize_t ad3530r_set_dac_powerdown(struct iio_dev *indio_dev,
+					 uintptr_t private,
+					 const struct iio_chan_spec *chan,
+					 const char *buf, size_t len)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+	int ret;
+	unsigned int mask, val;
+	bool powerdown;
+
+	ret = kstrtobool(buf, &powerdown);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+	switch (chan->channel) {
+	case AD3530R_CH(0) ... AD3530R_CH(AD3531R_MAX_CHANNELS - 1):
+		mask = GENMASK(chan->channel * 2 + 1, chan->channel * 2);
+		val = (powerdown ? st->chan[chan->channel].powerdown_mode : 0)
+		      << (chan->channel * 2);
+
+		ret = regmap_update_bits(st->regmap,
+					 AD3530R_OUTPUT_OPERATING_MODE_0,
+					 mask, val);
+		if (ret)
+			return ret;
+
+		st->chan[chan->channel].powerdown = powerdown;
+		return len;
+	case AD3530R_CH(AD3531R_MAX_CHANNELS) ...
+	     AD3530R_CH(AD3530R_MAX_CHANNELS - 1):
+		mask = GENMASK((chan->channel - 4) * 2 + 1,
+			       (chan->channel - 4) * 2);
+		val = (powerdown ? st->chan[chan->channel].powerdown_mode : 0)
+		      << ((chan->channel - 4) * 2);
+
+		ret = regmap_update_bits(st->regmap,
+					 AD3530R_OUTPUT_OPERATING_MODE_1,
+					 mask, val);
+		if (ret)
+			return ret;
+
+		st->chan[chan->channel].powerdown = powerdown;
+		return len;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const char * const ad3530r_muxout_select[] = {
+	[AD3530R_MUXOUT_POWERED_DOWN] = "powered_down",
+	[AD3530R_MUXOUT_VOUT0] = "vout0",
+	[AD3530R_MUXOUT_IOUT0_SOURCE] = "iout0_source",
+	[AD3530R_MUXOUT_IOUT0_SINK] = "iout0_sink",
+	[AD3530R_MUXOUT_VOUT1] = "vout1",
+	[AD3530R_MUXOUT_IOUT1_SOURCE] = "iout1_source",
+	[AD3530R_MUXOUT_IOUT1_SINK] = "iout1_sink",
+	[AD3530R_MUXOUT_VOUT2] = "vout2",
+	[AD3530R_MUXOUT_IOUT2_SOURCE] = "iout2_source",
+	[AD3530R_MUXOUT_IOUT2_SINK] = "iout2_sink",
+	[AD3530R_MUXOUT_VOUT3] = "vout3",
+	[AD3530R_MUXOUT_IOUT3_SOURCE] = "iout3_source",
+	[AD3530R_MUXOUT_IOUT3_SINK] = "iout3_sink",
+	[AD3530R_MUXOUT_VOUT4] = "vout4",
+	[AD3530R_MUXOUT_IOUT4_SOURCE] = "iout4_source",
+	[AD3530R_MUXOUT_IOUT4_SINK] = "iout4_sink",
+	[AD3530R_MUXOUT_VOUT5] = "vout5",
+	[AD3530R_MUXOUT_IOUT5_SOURCE] = "iout5_source",
+	[AD3530R_MUXOUT_IOUT5_SINK] = "iout5_sink",
+	[AD3530R_MUXOUT_VOUT6] = "vout6",
+	[AD3530R_MUXOUT_IOUT6_SOURCE] = "iout6_source",
+	[AD3530R_MUXOUT_IOUT6_SINK] = "iout6_sink",
+	[AD3530R_MUXOUT_VOUT7] = "vout7",
+	[AD3530R_MUXOUT_IOUT7_SOURCE] = "iout7_source",
+	[AD3530R_MUXOUT_IOUT7_SINK] = "iout7_sink",
+	[AD3530R_MUXOUT_DIE_TEMP] = "die_temp",
+	[AD3530R_MUXOUT_AGND] = "agnd",
+};
+
+static int ad3530r_get_muxout_select(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(st->regmap, AD3530R_MUX_OUT_SELECT, &val);
+	if (ret)
+		return ret;
+
+	return val;
+}
+
+static int ad3530r_set_muxout_select(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     unsigned int val)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+
+	return regmap_write(st->regmap, AD3530R_MUX_OUT_SELECT, val);
+}
+
+static const struct iio_enum ad3530r_muxout_select_enum = {
+	.items = ad3530r_muxout_select,
+	.num_items = ARRAY_SIZE(ad3530r_muxout_select),
+	.get = ad3530r_get_muxout_select,
+	.set = ad3530r_set_muxout_select,
+};
+
+static int ad3530r_trigger_hw_ldac(struct gpio_desc *ldac_gpio)
+{
+	gpiod_set_value_cansleep(ldac_gpio, 0);
+	usleep_range(AD3530R_LDAC_PULSE_US, AD3530R_LDAC_PULSE_US + 10);
+	gpiod_set_value_cansleep(ldac_gpio, 1);
+
+	return 0;
+}
+
+static int ad3530r_dac_write(struct ad3530r_state *st, unsigned int chan,
+			     unsigned int val)
+{
+	int ret;
+	__be16 reg_val;
+
+	guard(mutex)(&st->lock);
+	reg_val = cpu_to_be16(val);
+
+	ret = regmap_bulk_write(st->regmap, st->chip_info->input_ch_reg(chan),
+				&reg_val, 2);
+	if (ret)
+		return ret;
+
+	if (st->ldac_gpio)
+		return ad3530r_trigger_hw_ldac(st->ldac_gpio);
+
+	return regmap_update_bits(st->regmap, st->chip_info->sw_ldac_trig_reg,
+				  AD3530R_SW_LDAC_TRIG_MASK,
+				  FIELD_PREP(AD3530R_SW_LDAC_TRIG_MASK, 1));
+}
+
+static int ad3530r_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long info)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+	int ret;
+	__be16 reg_val;
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		ret = regmap_bulk_read(st->regmap,
+				       st->chip_info->input_ch_reg(chan->channel),
+				       &reg_val, 2);
+		if (ret)
+			return ret;
+
+		*val = FIELD_GET(AD3530R_REG_VAL_MASK, be16_to_cpu(reg_val));
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = st->vref_mv;
+		*val2 = 16;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = 0;
+
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad3530r_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long info)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		if (val < 0 || val > U16_MAX)
+			return -EINVAL;
+
+		return ad3530r_dac_write(st, chan->channel, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad3530r_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+			      unsigned int writeval, unsigned int *readval)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+
+	if (readval)
+		return regmap_read(st->regmap, reg, readval);
+
+	return regmap_write(st->regmap, reg, writeval);
+}
+
+#define AD3530R_CHAN_EXT_INFO(_name, _what, _read, _write) {		\
+	.name = (_name),						\
+	.read = (_read),						\
+	.write = (_write),						\
+	.private = (_what),						\
+	.shared = IIO_SEPARATE,						\
+}
+
+static const struct iio_chan_spec_ext_info ad3530r_ext_info[] = {
+	AD3530R_CHAN_EXT_INFO("powerdown", 0, ad3530r_get_dac_powerdown,
+			      ad3530r_set_dac_powerdown),
+	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &ad3530r_powerdown_mode_enum),
+	IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE,
+			   &ad3530r_powerdown_mode_enum),
+	IIO_ENUM("muxout_select", IIO_SHARED_BY_ALL, &ad3530r_muxout_select_enum),
+	IIO_ENUM_AVAILABLE("muxout_select", IIO_SHARED_BY_ALL,
+			   &ad3530r_muxout_select_enum),
+	{ },
+};
+
+#define AD3530R_CHAN(_chan) {						\
+	.type = IIO_VOLTAGE,						\
+	.indexed = 1,							\
+	.channel = _chan,						\
+	.output = 1,							\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
+			      BIT(IIO_CHAN_INFO_SCALE) |		\
+			      BIT(IIO_CHAN_INFO_OFFSET),		\
+	.ext_info = ad3530r_ext_info,					\
+}
+
+static const struct iio_chan_spec ad3530r_channels[] = {
+	AD3530R_CHAN(0),
+	AD3530R_CHAN(1),
+	AD3530R_CHAN(2),
+	AD3530R_CHAN(3),
+	AD3530R_CHAN(4),
+	AD3530R_CHAN(5),
+	AD3530R_CHAN(6),
+	AD3530R_CHAN(7),
+};
+
+static const struct iio_chan_spec ad3531r_channels[] = {
+	AD3530R_CHAN(0),
+	AD3530R_CHAN(1),
+	AD3530R_CHAN(2),
+	AD3530R_CHAN(3),
+};
+
+static const struct ad3530r_chip_info ad3530r_chip = {
+	.name = "ad3530r",
+	.channels = ad3530r_channels,
+	.num_channels = ARRAY_SIZE(ad3530r_channels),
+	.sw_ldac_trig_reg = AD3530R_SW_LDAC_TRIG_A,
+	.input_ch_reg = ad3530r_input_ch_reg,
+};
+
+static const struct ad3530r_chip_info ad3531r_chip = {
+	.name = "ad3531r",
+	.channels = ad3531r_channels,
+	.num_channels = ARRAY_SIZE(ad3531r_channels),
+	.sw_ldac_trig_reg = AD3531R_SW_LDAC_TRIG_A,
+	.input_ch_reg = ad3531r_input_ch_reg,
+};
+
+static int ad3530r_setup(struct ad3530r_state *st)
+{
+	const struct ad3530r_chip_info *chip_info = st->chip_info;
+	struct device *dev = &st->spi->dev;
+	struct gpio_desc *reset_gpio;
+	int i, ret;
+
+	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(reset_gpio),
+				     "Failed to get reset GPIO\n");
+
+	if (reset_gpio) {
+		/* Perform hardware reset */
+		usleep_range(20, 25);
+		gpiod_set_value_cansleep(reset_gpio, 0);
+	} else {
+		/* Perform software reset */
+		ret = regmap_update_bits(st->regmap, AD3530R_INTERFACE_CONFIG_A,
+					 AD3530R_SW_RESET, AD3530R_SW_RESET);
+		if (ret)
+			return ret;
+	}
+
+	usleep_range(10000, 15000);
+
+	/* Set operating mode to normal operation. */
+	ret = regmap_write(st->regmap, AD3530R_OUTPUT_OPERATING_MODE_0, 0);
+	if (ret)
+		return ret;
+
+	if (chip_info->num_channels > AD3531R_MAX_CHANNELS) {
+		ret = regmap_write(st->regmap, AD3530R_OUTPUT_OPERATING_MODE_1, 0);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < chip_info->num_channels; i++)
+		st->chan[i].powerdown_mode = AD3530R_32KOHM_POWERDOWN_MODE;
+
+	st->ldac_gpio = devm_gpiod_get_optional(dev, "ldac", GPIOD_OUT_HIGH);
+	if (IS_ERR(st->ldac_gpio))
+		return dev_err_probe(&st->spi->dev, PTR_ERR(st->ldac_gpio),
+				     "Failed to get ldac GPIO\n");
+
+	if (device_property_present(dev, "adi,double-output-range")) {
+		st->range_multiplier = true;
+
+		return regmap_update_bits(st->regmap, AD3530R_OUTPUT_CONTROL_0,
+					  AD3530R_OUTPUT_CONTROL_MASK,
+					  FIELD_PREP(AD3530R_OUTPUT_CONTROL_MASK, 1));
+	}
+
+	return 0;
+}
+
+static const struct regmap_config ad3530r_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+};
+
+static const struct iio_info ad3530r_info = {
+	.read_raw = ad3530r_read_raw,
+	.write_raw = ad3530r_write_raw,
+	.debugfs_reg_access = &ad3530r_reg_access,
+};
+
+static int ad3530r_probe(struct spi_device *spi)
+{
+	static const char * const regulators[] = { "vdd", "iovdd" };
+	const struct ad3530r_chip_info *chip_info;
+	struct device *dev = &spi->dev;
+	struct iio_dev *indio_dev;
+	struct ad3530r_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+
+	st->regmap = devm_regmap_init_spi(spi, &ad3530r_regmap_config);
+	if (IS_ERR(st->regmap))
+		return dev_err_probe(dev, PTR_ERR(st->regmap),
+				     "Failed to init regmap");
+
+	mutex_init(&st->lock);
+
+	chip_info = spi_get_device_match_data(spi);
+	if (!chip_info)
+		return -ENODEV;
+
+	st->chip_info = chip_info;
+
+	ret = ad3530r_setup(st);
+	if (ret)
+		return ret;
+
+	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators),
+					     regulators);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable regulators\n");
+
+	ret = devm_regulator_get_enable_read_voltage(dev, "ref");
+	if (ret < 0 && ret != -ENODEV)
+		return ret;
+
+	if (ret > 0) {
+		st->vref_mv = st->range_multiplier ? 2 * ret / 1000 : ret / 1000;
+	} else {
+		/* Internal reference. */
+		ret = regmap_update_bits(st->regmap, AD3530R_REFERENCE_CONTROL_0,
+					 AD3530R_REFERENCE_CONTROL_MASK,
+					 FIELD_PREP(AD3530R_REFERENCE_CONTROL_MASK, 1));
+		if (ret)
+			return ret;
+
+		st->vref_mv = st->range_multiplier ?
+			      2 * AD3530R_INTERNAL_VREF_MV :
+			      AD3530R_INTERNAL_VREF_MV;
+	}
+
+	indio_dev->name = chip_info->name;
+	indio_dev->info = &ad3530r_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = chip_info->channels;
+	indio_dev->num_channels = chip_info->num_channels;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id ad3530r_id[] = {
+	{ "ad3530r", (kernel_ulong_t)&ad3530r_chip },
+	{ "ad3531r", (kernel_ulong_t)&ad3531r_chip },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad3530r_id);
+
+static const struct of_device_id ad3530r_of_match[] = {
+	{ .compatible = "adi,ad3530r", .data = &ad3530r_chip },
+	{ .compatible = "adi,ad3531r", .data = &ad3531r_chip },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad3530r_of_match);
+
+static struct spi_driver ad3530r_driver = {
+	.driver = {
+		.name = "ad3530r",
+		.of_match_table = ad3530r_of_match,
+	},
+	.probe = ad3530r_probe,
+	.id_table = ad3530r_id,
+};
+module_spi_driver(ad3530r_driver);
+
+MODULE_AUTHOR("Kim Seer Paller <kimseer.paller@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD3530R and Similar DACs Driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v2 3/4] dt-bindings: iio: dac: Add adi,ad3530r.yaml

[Krzysztof Kozlowski]

On 24/03/2025 12:22, Kim Seer Paller wrote:
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +  - iovdd-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +additionalProperties: false

This should be unevaluatedProperties. I don't get why you switched to
additionalProps here and your other bindings like ltc26*.yaml


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 3/4] dt-bindings: iio: dac: Add adi,ad3530r.yaml

[Rob Herring (Arm)]

On Mon, 24 Mar 2025 19:22:57 +0800, Kim Seer Paller wrote:
> Document the AD3530R/AD3530, an 8-Channel, 16-bit Voltage Output DAC,
> while the AD3531R/AD3531 is a 4-Channel, 16-Bit Voltage Output DAC.
> These devices include software-programmable gain controls that provide
> full-scale output spans of 2.5V or 5V for reference voltages of 2.5V.
> They operate from a single supply voltage range of 2.7V to 5.5V and are
> guaranteed to be monotonic by design. Additionally, these devices
> features a 2.5V, 5ppm/°C internal reference, which is disabled by default.
> 
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> ---
>  .../devicetree/bindings/iio/dac/adi,ad3530r.yaml   | 91 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 92 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.example.dtb: adc@0: pwm-names: ['convst1'] is too short
	from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad7606.yaml#

doc reference errors (make refcheckdocs):
Warning: Documentation/arch/powerpc/cxl.rst references a file that doesn't exist: Documentation/ABI/testing/sysfs-class-cxl
Warning: lib/Kconfig.debug references a file that doesn't exist: Documentation/dev-tools/fault-injection/fault-injection.rst
Documentation/arch/powerpc/cxl.rst: Documentation/ABI/testing/sysfs-class-cxl
lib/Kconfig.debug: Documentation/dev-tools/fault-injection/fault-injection.rst

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250324-togreg-v2-3-f211d781923e@analog.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v2 2/4] iio: dac: ad3530r: Add ABI file for the AD3530R DAC

[Jonathan Cameron]

On Mon, 24 Mar 2025 19:22:56 +0800
Kim Seer Paller <kimseer.paller@analog.com> wrote:

> Define muxout_select and muxout_select_available sysfs interface for the
> AD3530R and AD3531R DAC.
> 
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> ---
>  .../ABI/testing/sysfs-bus-iio-dac-ad3530r          | 68 ++++++++++++++++++++++
>  MAINTAINERS                                        |  7 +++
>  2 files changed, 75 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ad3530r b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad3530r
> new file mode 100644
> index 0000000000000000000000000000000000000000..9d3126952fd1c5214afb895c4972dc4a891ed7d4
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad3530r
> @@ -0,0 +1,68 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/muxout_select
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Selects which of the multiplexer's input signals will be
> +		monitored on the MUX_OUT pin.
Hi Kim,

Do we have a use case where the monitoring would not be by an ADC attached
to the host CPU? (i.e. using a channel on an ADC that has it's own IIO driver)

If no other use case, then support this as a consumer of an ADC channel from
another device with all these exposed as different channels when a read is requested.
There are quite a few drivers doing this already.

The source vs sink thing may need to be done via labels as it isn't a concept
that maps directly to ADC channel characteristics.

If this is being routed to some external hardware monitoring then most like
this should be in device tree as doesn't make sense to switch dynamically in
any cases that I recall from previous similar drivers.

Jonathan

> +		* powered_down - MUX_OUT pin is powered down. An 80kΩ impedance
> +				 can be seen at the MUX_OUT pin.
> +		* vout0 - Voltage representation of VOUT0.
> +		* iout0_source - Voltage representation of IOUT0 (source mode).
> +		* iout0_sink - Voltage representation of IOUT0 (sink mode).
> +		* vout1 - Voltage representation of VOUT1.
> +		* iout1_source - Voltage representation of IOUT1 (source mode).
> +		* iout1_sink - Voltage representation of IOUT1 (sink mode).
> +		* vout2 - Voltage representation of VOUT2.
> +		* iout2_source - Voltage representation of IOUT2 (source mode).
> +		* iout2_sink - Voltage representation of IOUT2 (sink mode).
> +		* vout3 - Voltage representation of VOUT3
> +		* iout3_source - Voltage representation of IOUT3 (source mode).
> +		* iout3_sink - Voltage representation of IOUT3 (sink mode).
> +		* vout4 - Voltage representation of VOUT4.
> +		* iout4_source - Voltage representation of IOUT4 (source mode).
> +		* iout4_sink - Voltage representation of IOUT4 (sink mode).
> +		* vout5 - Voltage representation of VOUT5.
> +		* iout5_source - Voltage representation of IOUT5 (source mode).
> +		* iout5_sink - Voltage representation of IOUT5 (sink mode).
> +		* vout6 - Voltage representation of VOUT6.
> +		* iout6_source - Voltage representation of IOUT6 (source mode).
> +		* iout6_sink - Voltage representation of IOUT6 (sink mode).
> +		* vout7 - Voltage representation of VOUT7.
> +		* iout7_source - Voltage representation of IOUT7 (source mode).
> +		* iout7_sink - Voltage representation of IOUT7 (sink mode).
> +		* die_temp - Voltage representation of internal die temperature.
> +		* agnd - MUX_OUT pin internally tied to AGND.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/muxout_select_available
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Reading this returns the valid values that can be written to the
> +		muxout_select attribute:
> +		* powered_down
> +		* vout0
> +		* iout0_source
> +		* iout0_sink
> +		* vout1
> +		* iout1_source
> +		* iout1_sink
> +		* vout2
> +		* iout2_source
> +		* iout2_sink
> +		* vout3
> +		* iout3_source
> +		* iout3_sink
> +		* vout4
> +		* iout4_source
> +		* iout4_sink
> +		* vout5
> +		* iout5_source
> +		* iout5_sink
> +		* vout6
> +		* iout6_source
> +		* iout6_sink
> +		* vout7
> +		* iout7_source
> +		* iout7_sink
> +		* die_temp
> +		* agnd
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ffdb3f21fc4fb35b349449afbb30fecd4fe72978..2d3c31c74594ca1934c67e7aad0a179feeaa39bf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1289,6 +1289,13 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
>  F:	drivers/net/amt.c
>  
> +ANALOG DEVICES INC AD3530R DRIVER
> +M:	Kim Seer Paller <kimseer.paller@analog.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Supported
> +W:	https://ez.analog.com/linux-software-drivers
> +F:	Documentation/ABI/testing/sysfs-bus-iio-dac-ad3530r
> +
>  ANALOG DEVICES INC AD3552R DRIVER
>  M:	Nuno Sá <nuno.sa@analog.com>
>  L:	linux-iio@vger.kernel.org
> 

Re: [PATCH v2 3/4] dt-bindings: iio: dac: Add adi,ad3530r.yaml

[Jonathan Cameron]

On Mon, 24 Mar 2025 19:22:57 +0800
Kim Seer Paller <kimseer.paller@analog.com> wrote:

> Document the AD3530R/AD3530, an 8-Channel, 16-bit Voltage Output DAC,
> while the AD3531R/AD3531 is a 4-Channel, 16-Bit Voltage Output DAC.
> These devices include software-programmable gain controls that provide
> full-scale output spans of 2.5V or 5V for reference voltages of 2.5V.
> They operate from a single supply voltage range of 2.7V to 5.5V and are
> guaranteed to be monotonic by design. Additionally, these devices
> features a 2.5V, 5ppm/°C internal reference, which is disabled by default.
> 
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> ---
>  .../devicetree/bindings/iio/dac/adi,ad3530r.yaml   | 91 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 92 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..e581472b50048bedda7422748035423b9b020382
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
> @@ -0,0 +1,91 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/adi,ad3530r.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD3530R and Similar DACs
> +
> +maintainers:
> +  - Kim Seer Paller <kimseer.paller@analog.com>
> +
> +description: |
> +  The AD3530/AD3530R are low power, 8-channel, 16-bit, buffered voltage output,
> +  digital-to-analog converters (DACs) that include software-programmable gain
> +  controls that result in full-scale output spans of 2.5V or 5V for reference
> +  voltages of 2.5V. The devices operate from single, 2.7V to 5.5V supply ranges
> +  and are guaranteed monotonic by design. The AD3530R also offers a 2.5V,
> +  5ppm/°C internal reference that is disabled by default.
> +  Datasheet can be found here:
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad3530_ad530r.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad3530r

You mention this one as well as a variant without the r postfix in the 'description'. 
So why not compatible for that?  If it's software compatible with the r version than
a fallback compatible makes sense.  We probably still want to have separate
compatibles though in case we get an errata that only applies to one of them.

If they are the same silicon, perhaps with different ratings then make that clear
in the description and perhaps it is fine to not have both compatibles listed.
 
> +      - adi,ad3531r

This isn't mentioned in the description text.

> +
> +  reg:
> +    maxItems: 1

Thanks,

Jonathan

Re: [PATCH v2 4/4] iio: dac: ad3530r: Add driver for AD3530R and AD3531R

[Jonathan Cameron]

On Mon, 24 Mar 2025 19:22:58 +0800
Kim Seer Paller <kimseer.paller@analog.com> wrote:

> The AD3530R/AD3530 is an 8-Channel, 16-Bit Voltage Output DAC, while the
> AD3531R/AD3531 is a 4-Channel, 16-Bit Voltage Output DAC. These devices
> include software-programmable gain controls that provide full-scale
> output spans of 2.5V or 5V for reference voltages of 2.5V. They operate
> from a single supply voltage range of 2.7V to 5.5V and are guaranteed to
> be monotonic by design. Additionally, these devices features a 2.5V,
> 5ppm/°C internal reference, which is disabled by default.
> 
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>

This is a bit of an 'experiment' in reviewing after a red eye flight
so it may be even less coherent than I normally am!

> diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c

> +
> +struct ad3530r_state {
> +	struct spi_device *spi;

As below. This seems unnecessary.

> +	struct regmap *regmap;
> +	struct mutex lock; /* protect the state of the device */

Be more specific on that comment.  What state needs protecting
beyond the bus locking that also goes on?  Typically look
for sequences that need to be atomic and say what data causes
that to be the problem here.

> +	struct ad3530r_chan chan[AD3530R_MAX_CHANNELS];
> +	const struct ad3530r_chip_info *chip_info;
> +	struct gpio_desc *ldac_gpio;
> +	int vref_mv;
> +	u8 ldac;
> +	bool range_multiplier;
> +};

> +
> +static int ad3530r_trigger_hw_ldac(struct gpio_desc *ldac_gpio)
> +{
> +	gpiod_set_value_cansleep(ldac_gpio, 0);
> +	usleep_range(AD3530R_LDAC_PULSE_US, AD3530R_LDAC_PULSE_US + 10);

fsleep()  unless this window is there for some reason I'm not seeing.

Maybe just usleep() is more appropriate.

> +	gpiod_set_value_cansleep(ldac_gpio, 1);
> +
> +	return 0;
> +}

> +static int ad3530r_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long info)
> +{
> +	struct ad3530r_state *st = iio_priv(indio_dev);
> +	int ret;
> +	__be16 reg_val;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = regmap_bulk_read(st->regmap,
> +				       st->chip_info->input_ch_reg(chan->channel),
> +				       &reg_val, 2);

sizeof(regval) instead of that 2.

> +		if (ret)
> +			return ret;
> +
> +		*val = FIELD_GET(AD3530R_REG_VAL_MASK, be16_to_cpu(reg_val));
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = st->vref_mv;
> +		*val2 = 16;
> +
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = 0;

Don't report an offset at all if it is always zero.  That is the default
when it is not reported.

> +
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
>

> +
> +#define AD3530R_CHAN_EXT_INFO(_name, _what, _read, _write) {		\
> +	.name = (_name),						\
> +	.read = (_read),						\
> +	.write = (_write),						\
> +	.private = (_what),						\
> +	.shared = IIO_SEPARATE,						\
> +}

Don't define a macro for filling these in unless you need it lots of times.
Just put the full initialisation inline.

> +
> +static const struct iio_chan_spec_ext_info ad3530r_ext_info[] = {
> +	AD3530R_CHAN_EXT_INFO("powerdown", 0, ad3530r_get_dac_powerdown,
> +			      ad3530r_set_dac_powerdown),
> +	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &ad3530r_powerdown_mode_enum),
> +	IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE,
> +			   &ad3530r_powerdown_mode_enum),
> +	IIO_ENUM("muxout_select", IIO_SHARED_BY_ALL, &ad3530r_muxout_select_enum),

For the muxout stuff see comments on the ABI docs patch.

> +	IIO_ENUM_AVAILABLE("muxout_select", IIO_SHARED_BY_ALL,
> +			   &ad3530r_muxout_select_enum),
> +	{ },
> +};
> +
> +#define AD3530R_CHAN(_chan) {						\
> +	.type = IIO_VOLTAGE,						\
> +	.indexed = 1,							\
> +	.channel = _chan,						\
> +	.output = 1,							\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> +			      BIT(IIO_CHAN_INFO_SCALE) |		\
> +			      BIT(IIO_CHAN_INFO_OFFSET),		\
> +	.ext_info = ad3530r_ext_info,					\
> +}

...

> +
> +static int ad3530r_setup(struct ad3530r_state *st)
> +{
> +	const struct ad3530r_chip_info *chip_info = st->chip_info;
> +	struct device *dev = &st->spi->dev;

st->spi is only ever used to get the dev.  You can get that 
from the regmap instead.  If that looks too messy for some reason then
you could also just store dev in st.

> +	struct gpio_desc *reset_gpio;
> +	int i, ret;
> +
> +	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(reset_gpio),
> +				     "Failed to get reset GPIO\n");
> +
> +	if (reset_gpio) {
> +		/* Perform hardware reset */
> +		usleep_range(20, 25);
> +		gpiod_set_value_cansleep(reset_gpio, 0);
> +	} else {
> +		/* Perform software reset */
> +		ret = regmap_update_bits(st->regmap, AD3530R_INTERFACE_CONFIG_A,
> +					 AD3530R_SW_RESET, AD3530R_SW_RESET);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	usleep_range(10000, 15000);

fsleep() preferred.

> +
> +	/* Set operating mode to normal operation. */
> +	ret = regmap_write(st->regmap, AD3530R_OUTPUT_OPERATING_MODE_0, 0);
> +	if (ret)
> +		return ret;

...

> +static int ad3530r_probe(struct spi_device *spi)
> +{
> +	static const char * const regulators[] = { "vdd", "iovdd" };
> +	const struct ad3530r_chip_info *chip_info;
> +	struct device *dev = &spi->dev;
> +	struct iio_dev *indio_dev;
> +	struct ad3530r_state *st;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +
> +	st->regmap = devm_regmap_init_spi(spi, &ad3530r_regmap_config);
> +	if (IS_ERR(st->regmap))
> +		return dev_err_probe(dev, PTR_ERR(st->regmap),
> +				     "Failed to init regmap");
> +
> +	mutex_init(&st->lock);
For new drivers, I'd prefer we use 
	ret = devm_mutex_init(&st->lock);
	if (ret)
		return ret;

It only brings a small benefit in debug cases but still nice to have
given we can now do it without explicit mutex_destroy() calls.

> +
> +	chip_info = spi_get_device_match_data(spi);
> +	if (!chip_info)
> +		return -ENODEV;
> +
> +	st->chip_info = chip_info;
> +
> +	ret = ad3530r_setup(st);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators),
> +					     regulators);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to enable regulators\n");
> +
> +	ret = devm_regulator_get_enable_read_voltage(dev, "ref");
> +	if (ret < 0 && ret != -ENODEV)
> +		return ret;
> +
> +	if (ret > 0) {

This is going to do something odd if the voltage returned is 0. 
I'd just have ret >= 0 for this case.  We'll never see it in a real system, but
nice to have anyway.

> +		st->vref_mv = st->range_multiplier ? 2 * ret / 1000 : ret / 1000;
> +	} else {
> +		/* Internal reference. */
> +		ret = regmap_update_bits(st->regmap, AD3530R_REFERENCE_CONTROL_0,
> +					 AD3530R_REFERENCE_CONTROL_MASK,
> +					 FIELD_PREP(AD3530R_REFERENCE_CONTROL_MASK, 1));
> +		if (ret)
> +			return ret;
> +
> +		st->vref_mv = st->range_multiplier ?
> +			      2 * AD3530R_INTERNAL_VREF_MV :
> +			      AD3530R_INTERNAL_VREF_MV;
> +	}

Jonathan

RE: [PATCH v2 2/4] iio: dac: ad3530r: Add ABI file for the AD3530R DAC

[Paller, Kim Seer]

> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Friday, March 28, 2025 4:58 PM
> To: Paller, Kim Seer <KimSeer.Paller@analog.com>
> Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; linux-
> iio@vger.kernel.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH v2 2/4] iio: dac: ad3530r: Add ABI file for the AD3530R
> DAC
> 
> [External]
> 
> On Mon, 24 Mar 2025 19:22:56 +0800
> Kim Seer Paller <kimseer.paller@analog.com> wrote:
> 
> > Define muxout_select and muxout_select_available sysfs interface for the
> > AD3530R and AD3531R DAC.
> >
> > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> > ---
> >  .../ABI/testing/sysfs-bus-iio-dac-ad3530r          | 68 ++++++++++++++++++++++
> >  MAINTAINERS                                        |  7 +++
> >  2 files changed, 75 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ad3530r
> b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad3530r
> > new file mode 100644
> > index
> 0000000000000000000000000000000000000000..9d3126952fd1c5214afb89
> 5c4972dc4a891ed7d4
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad3530r
> > @@ -0,0 +1,68 @@
> > +What:		/sys/bus/iio/devices/iio:deviceX/muxout_select
> > +KernelVersion:
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Selects which of the multiplexer's input signals will be
> > +		monitored on the MUX_OUT pin.
> Hi Kim,
> 
> Do we have a use case where the monitoring would not be by an ADC attached
> to the host CPU? (i.e. using a channel on an ADC that has it's own IIO driver)
>

No, we don’t have a use case without an ADC. I’ll proceed with implementing this
as a consumer of an ADC channel. Thanks

> If no other use case, then support this as a consumer of an ADC channel from
> another device with all these exposed as different channels when a read is
> requested.
> There are quite a few drivers doing this already.
> 
> The source vs sink thing may need to be done via labels as it isn't a concept
> that maps directly to ADC channel characteristics.
> 
> If this is being routed to some external hardware monitoring then most like
> this should be in device tree as doesn't make sense to switch dynamically in
> any cases that I recall from previous similar drivers.
> 
> Jonathan
> 
> > +		* powered_down - MUX_OUT pin is powered down. An 80kΩ
> impedance
> > +				 can be seen at the MUX_OUT pin.
> > +		* vout0 - Voltage representation of VOUT0.
> > +		* iout0_source - Voltage representation of IOUT0 (source
> mode).
> > +		* iout0_sink - Voltage representation of IOUT0 (sink mode).
> > +		* vout1 - Voltage representation of VOUT1.
> > +		* iout1_source - Voltage representation of IOUT1 (source
> mode).
> > +		* iout1_sink - Voltage representation of IOUT1 (sink mode).
> > +		* vout2 - Voltage representation of VOUT2.
> > +		* iout2_source - Voltage representation of IOUT2 (source
> mode).
> > +		* iout2_sink - Voltage representation of IOUT2 (sink mode).
> > +		* vout3 - Voltage representation of VOUT3
> > +		* iout3_source - Voltage representation of IOUT3 (source
> mode).
> > +		* iout3_sink - Voltage representation of IOUT3 (sink mode).
> > +		* vout4 - Voltage representation of VOUT4.
> > +		* iout4_source - Voltage representation of IOUT4 (source
> mode).
> > +		* iout4_sink - Voltage representation of IOUT4 (sink mode).
> > +		* vout5 - Voltage representation of VOUT5.
> > +		* iout5_source - Voltage representation of IOUT5 (source
> mode).
> > +		* iout5_sink - Voltage representation of IOUT5 (sink mode).
> > +		* vout6 - Voltage representation of VOUT6.
> > +		* iout6_source - Voltage representation of IOUT6 (source
> mode).
> > +		* iout6_sink - Voltage representation of IOUT6 (sink mode).
> > +		* vout7 - Voltage representation of VOUT7.
> > +		* iout7_source - Voltage representation of IOUT7 (source
> mode).
> > +		* iout7_sink - Voltage representation of IOUT7 (sink mode).
> > +		* die_temp - Voltage representation of internal die
> temperature.
> > +		* agnd - MUX_OUT pin internally tied to AGND.
> > +
> > +What:
> 	/sys/bus/iio/devices/iio:deviceX/muxout_select_available
> > +KernelVersion:
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Reading this returns the valid values that can be written to the
> > +		muxout_select attribute:
> > +		* powered_down
> > +		* vout0
> > +		* iout0_source
> > +		* iout0_sink
> > +		* vout1
> > +		* iout1_source
> > +		* iout1_sink
> > +		* vout2
> > +		* iout2_source
> > +		* iout2_sink
> > +		* vout3
> > +		* iout3_source
> > +		* iout3_sink
> > +		* vout4
> > +		* iout4_source
> > +		* iout4_sink
> > +		* vout5
> > +		* iout5_source
> > +		* iout5_sink
> > +		* vout6
> > +		* iout6_source
> > +		* iout6_sink
> > +		* vout7
> > +		* iout7_source
> > +		* iout7_sink
> > +		* die_temp
> > +		* agnd
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index
> ffdb3f21fc4fb35b349449afbb30fecd4fe72978..2d3c31c74594ca1934c67e7aad
> 0a179feeaa39bf 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1289,6 +1289,13 @@ T:	git
> git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git
> >  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
> >  F:	drivers/net/amt.c
> >
> > +ANALOG DEVICES INC AD3530R DRIVER
> > +M:	Kim Seer Paller <kimseer.paller@analog.com>
> > +L:	linux-iio@vger.kernel.org
> > +S:	Supported
> > +W:	https://ez.analog.com/linux-software-drivers
> > +F:	Documentation/ABI/testing/sysfs-bus-iio-dac-ad3530r
> > +
> >  ANALOG DEVICES INC AD3552R DRIVER
> >  M:	Nuno Sá <nuno.sa@analog.com>
> >  L:	linux-iio@vger.kernel.org
> >

RE: [PATCH v2 3/4] dt-bindings: iio: dac: Add adi,ad3530r.yaml

[Paller, Kim Seer]

> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Friday, March 28, 2025 5:04 PM
> To: Paller, Kim Seer <KimSeer.Paller@analog.com>
> Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; linux-
> iio@vger.kernel.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH v2 3/4] dt-bindings: iio: dac: Add adi,ad3530r.yaml
> 
> [External]
> 
> On Mon, 24 Mar 2025 19:22:57 +0800
> Kim Seer Paller <kimseer.paller@analog.com> wrote:
> 
> > Document the AD3530R/AD3530, an 8-Channel, 16-bit Voltage Output DAC,
> > while the AD3531R/AD3531 is a 4-Channel, 16-Bit Voltage Output DAC.
> > These devices include software-programmable gain controls that provide
> > full-scale output spans of 2.5V or 5V for reference voltages of 2.5V.
> > They operate from a single supply voltage range of 2.7V to 5.5V and are
> > guaranteed to be monotonic by design. Additionally, these devices
> > features a 2.5V, 5ppm/°C internal reference, which is disabled by default.
> >
> > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> > ---
> >  .../devicetree/bindings/iio/dac/adi,ad3530r.yaml   | 91
> ++++++++++++++++++++++
> >  MAINTAINERS                                        |  1 +
> >  2 files changed, 92 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
> b/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
> > new file mode 100644
> > index
> 0000000000000000000000000000000000000000..e581472b50048bedda742
> 2748035423b9b020382
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
> > @@ -0,0 +1,91 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> https://urldefense.com/v3/__http://devicetree.org/schemas/iio/dac/adi,ad353
> 0r.yaml*__;Iw!!A3Ni8CS0y2Y!-bEqh-bKz-a-
> ZzfbJZwXi9KWwKlsk_Pcaj5XQLeLaDBpO8MaryRedRHaL0GUFfNz35tDsFaJkV45P
> fyEzA$
> > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-
> schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!-bEqh-bKz-a-
> ZzfbJZwXi9KWwKlsk_Pcaj5XQLeLaDBpO8MaryRedRHaL0GUFfNz35tDsFaJkV5zI4
> zMaQ$
> > +
> > +title: Analog Devices AD3530R and Similar DACs
> > +
> > +maintainers:
> > +  - Kim Seer Paller <kimseer.paller@analog.com>
> > +
> > +description: |
> > +  The AD3530/AD3530R are low power, 8-channel, 16-bit, buffered voltage
> output,
> > +  digital-to-analog converters (DACs) that include software-programmable
> gain
> > +  controls that result in full-scale output spans of 2.5V or 5V for reference
> > +  voltages of 2.5V. The devices operate from single, 2.7V to 5.5V supply
> ranges
> > +  and are guaranteed monotonic by design. The AD3530R also offers a 2.5V,
> > +  5ppm/°C internal reference that is disabled by default.
> > +  Datasheet can be found here:
> > +  https://www.analog.com/media/en/technical-documentation/data-
> sheets/ad3530_ad530r.pdf
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,ad3530r
> 
> You mention this one as well as a variant without the r postfix in the
> 'description'.
> So why not compatible for that?  If it's software compatible with the r version
> than
> a fallback compatible makes sense.  We probably still want to have separate
> compatibles though in case we get an errata that only applies to one of them.
> 
> If they are the same silicon, perhaps with different ratings then make that clear
> in the description and perhaps it is fine to not have both compatibles listed.

The main difference is that the r variant supports both internal and external references,
while the non-r variant supports only external references. I agree with having separate
compatibles. Well, I'm thinking of adding a parameter in chip_info into the driver to
identify internal reference support. What do you think?

> 
> > +      - adi,ad3531r
> 
> This isn't mentioned in the description text.
> 
> > +
> > +  reg:
> > +    maxItems: 1
> 
> Thanks,
> 
> Jonathan

Re: [PATCH v2 3/4] dt-bindings: iio: dac: Add adi,ad3530r.yaml

[Jonathan Cameron]

On Mon, 31 Mar 2025 01:31:10 +0000
"Paller, Kim Seer" <KimSeer.Paller@analog.com> wrote:

> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Friday, March 28, 2025 5:04 PM
> > To: Paller, Kim Seer <KimSeer.Paller@analog.com>
> > Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael
> > <Michael.Hennerich@analog.com>; Rob Herring <robh@kernel.org>; Krzysztof
> > Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; linux-
> > iio@vger.kernel.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> > Subject: Re: [PATCH v2 3/4] dt-bindings: iio: dac: Add adi,ad3530r.yaml
> > 
> > [External]
> > 
> > On Mon, 24 Mar 2025 19:22:57 +0800
> > Kim Seer Paller <kimseer.paller@analog.com> wrote:
> >   
> > > Document the AD3530R/AD3530, an 8-Channel, 16-bit Voltage Output DAC,
> > > while the AD3531R/AD3531 is a 4-Channel, 16-Bit Voltage Output DAC.
> > > These devices include software-programmable gain controls that provide
> > > full-scale output spans of 2.5V or 5V for reference voltages of 2.5V.
> > > They operate from a single supply voltage range of 2.7V to 5.5V and are
> > > guaranteed to be monotonic by design. Additionally, these devices
> > > features a 2.5V, 5ppm/°C internal reference, which is disabled by default.
> > >
> > > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> > > ---
> > >  .../devicetree/bindings/iio/dac/adi,ad3530r.yaml   | 91  
> > ++++++++++++++++++++++  
> > >  MAINTAINERS                                        |  1 +
> > >  2 files changed, 92 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml  
> > b/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml  
> > > new file mode 100644
> > > index  
> > 0000000000000000000000000000000000000000..e581472b50048bedda742
> > 2748035423b9b020382  
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
> > > @@ -0,0 +1,91 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id:  
> > https://urldefense.com/v3/__http://devicetree.org/schemas/iio/dac/adi,ad353
> > 0r.yaml*__;Iw!!A3Ni8CS0y2Y!-bEqh-bKz-a-
> > ZzfbJZwXi9KWwKlsk_Pcaj5XQLeLaDBpO8MaryRedRHaL0GUFfNz35tDsFaJkV45P
> > fyEzA$  
> > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-  
> > schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!-bEqh-bKz-a-
> > ZzfbJZwXi9KWwKlsk_Pcaj5XQLeLaDBpO8MaryRedRHaL0GUFfNz35tDsFaJkV5zI4
> > zMaQ$  
> > > +
> > > +title: Analog Devices AD3530R and Similar DACs
> > > +
> > > +maintainers:
> > > +  - Kim Seer Paller <kimseer.paller@analog.com>
> > > +
> > > +description: |
> > > +  The AD3530/AD3530R are low power, 8-channel, 16-bit, buffered voltage  
> > output,  
> > > +  digital-to-analog converters (DACs) that include software-programmable  
> > gain  
> > > +  controls that result in full-scale output spans of 2.5V or 5V for reference
> > > +  voltages of 2.5V. The devices operate from single, 2.7V to 5.5V supply  
> > ranges  
> > > +  and are guaranteed monotonic by design. The AD3530R also offers a 2.5V,
> > > +  5ppm/°C internal reference that is disabled by default.
> > > +  Datasheet can be found here:
> > > +  https://www.analog.com/media/en/technical-documentation/data-  
> > sheets/ad3530_ad530r.pdf  
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - adi,ad3530r  
> > 
> > You mention this one as well as a variant without the r postfix in the
> > 'description'.
> > So why not compatible for that?  If it's software compatible with the r version
> > than
> > a fallback compatible makes sense.  We probably still want to have separate
> > compatibles though in case we get an errata that only applies to one of them.
> > 
> > If they are the same silicon, perhaps with different ratings then make that clear
> > in the description and perhaps it is fine to not have both compatibles listed.  
> 
> The main difference is that the r variant supports both internal and external references,
> while the non-r variant supports only external references. I agree with having separate
> compatibles. Well, I'm thinking of adding a parameter in chip_info into the driver to
> identify internal reference support. What do you think?
Yes, combination of separate compatibles and a parameter in chip_info to make the
code simple sounds like the right solution to me.

Jonathan

> 
> >   
> > > +      - adi,ad3531r  
> > 
> > This isn't mentioned in the description text.
> >   
> > > +
> > > +  reg:
> > > +    maxItems: 1  
> > 
> > Thanks,
> > 
> > Jonathan