* [PATCH v3 0/2] iio: dac: Add support for AD5529R DAC
@ 2026-05-19 15:42 Janani Sunil
2026-05-19 15:42 ` [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R Janani Sunil
2026-05-19 15:42 ` [PATCH v3 2/2] iio: dac: Add AD5529R DAC driver support Janani Sunil
0 siblings, 2 replies; 19+ messages in thread
From: Janani Sunil @ 2026-05-19 15:42 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil,
Janani Sunil
This patch series adds support for Analog Devices AD5529R, a 16 channel
16 and 12 bit voltage Digital-to-Analog Converter (DAC) with integrated
precision reference. The AD5529R operates from both unipolar and
bipolar supplies. The device communicates via SPI interface.
**Device Overview:**
The AD5529R features 16 independent DAC channels, with 16 or 12 bit
resolution, allowing independently programmable output ranges. The
internal 4.096V precision reference sets the accuracy of the output
voltage.
**Features Implemented:**
- Automatic detection of 12/16 bit variant with product ID read.
- Reset support via GPIO.
- Dual regmap configuration to handle 8 and 16 bit registers.
**Patch Summary:**
1. **dt-bindings**: Binding documentation with channel configuration.
2. **driver**: Implement IIO DAC Driver with regmap support.
**Testing:**
The driver was compiled and tested on the EVAL-AD5529R-ARDZ using a
coraZ7 with a mainline v7.0 kernel.
**Driver Rationale:**
AD5529R introduces:
1. A unique register layout
2. Mixed 8-bit and 16-bit register accesses
3. Product ID based generic identification
4. Hardware specific features like function generators, multi-die
hotpath registers etc.
The device warrants its own drivers due to these fundamental
architectural differences, that would require substantial changes to
existing drivers without providing reusable benefits. The standalone
driver also allows future extensions for related devices in the same
family.
Signed-off-by: Janani Sunil <janani.sunil@analog.com>
---
Changes in v3:
- Split into adi,ad5529r-16 and adi,ad5529r-12 device tree compatibles
- Add DT-based output range configuration via adi,output-range-microvolt
- Expand DT binding: vref-supply, clear/tg GPIOs, interrupts, muxout
- Correct power supply voltage specifications as per datasheet
- Reduce SPI frequency limit to 25MHz as per datasheet specs
- Switch to autoincrement addressing mode, remove +1 register offsets
- Use DT match data instead of device ID detection for fallback support
- Implement dynamic scale/offset calculation per configured channel range
- Added explicit val_format_endian and reg_stride for 16-bit regmap bus
- Code cleanup: alphabetical includes, ARRAY_SIZE(), unused defines
- Minor: .sign→.format field, simplify read/write order, optional hvss-supply
- Remove redundant driver documentation ad5529r.rst
- Link to v2: https://lore.kernel.org/r/20260508-ad5529r-driver-v2-0-e315441685d7@analog.com
Changes in v2:
- Fix IIO scale to use millivolts per ABI requirement
- Fix documentation voltage calculations (2.5V not 2.048V)
- Fix bipolar ranges in documentation (±5V, ±10V, ±15V, ±20V)
- Fix alphabetical ordering in documentation index
- Add missing newline to documentation file
- Fix scale units description (millivolts not microvolts)
- Include a section for driver rationale in the cover letter
- Reword contents in cover letter 12/16 bit generic->variant
- Add dependency array for spi-cpha and spi-cpol properties
- Link to v1: https://lore.kernel.org/r/20260507-ad5529r-driver-v1-0-b4460f3cb44f@analog.com
---
Janani Sunil (2):
dt-bindings: iio: dac: Add AD5529R
iio: dac: Add AD5529R DAC driver support
.../devicetree/bindings/iio/dac/adi,ad5529r.yaml | 217 +++++++++
MAINTAINERS | 8 +
drivers/iio/dac/Kconfig | 17 +
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/ad5529r.c | 527 +++++++++++++++++++++
5 files changed, 770 insertions(+)
---
base-commit: 93df88612859e8e19dec93c69d563b4b73e9bd4b
change-id: 20260507-ad5529r-driver-866bbdd864de
Best regards,
--
Janani Sunil <janani.sunil@analog.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-05-19 15:42 [PATCH v3 0/2] iio: dac: Add support for AD5529R DAC Janani Sunil
@ 2026-05-19 15:42 ` Janani Sunil
2026-05-19 15:55 ` sashiko-bot
` (2 more replies)
2026-05-19 15:42 ` [PATCH v3 2/2] iio: dac: Add AD5529R DAC driver support Janani Sunil
1 sibling, 3 replies; 19+ messages in thread
From: Janani Sunil @ 2026-05-19 15:42 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil,
Janani Sunil
Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
buffered voltage output digital-to-analog converter (DAC) with an
integrated precision reference.
Signed-off-by: Janani Sunil <janani.sunil@analog.com>
---
.../devicetree/bindings/iio/dac/adi,ad5529r.yaml | 217 +++++++++++++++++++++
MAINTAINERS | 7 +
2 files changed, 224 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
new file mode 100644
index 000000000000..eb66f6ca063d
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
@@ -0,0 +1,217 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ad5529r.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD5529R 16-Channel 12/16-bit High Voltage DAC
+
+maintainers:
+ - Janani Sunil <janani.sunil@analog.com>
+
+description: |
+ The AD5529R is a 16-channel, 12-bit or 16-bit, high voltage, buffered voltage output
+ digital-to-analog converter (DAC) with an integrated precision reference.
+ The device operates from unipolar and bipolar supplies. It is guaranteed
+ monotonic and has built-in rail-to-rail output buffers that can source or
+ sink up to 25mA.
+
+ Specifications:
+ * 16 independent 12-bit or 16-bit DAC channels
+ * Independently programmable output ranges: 0V to 5V, 0V to 10V, 0V to 20V,
+ 0V to 40V, ±5V, ±10V, ±15V, and ±20V
+ * The device supports SPI communication with Mode 0 and Mode 3.
+ * 4.096V precision reference, 12ppm/°C maximum
+ * Built-in function generation: Toggle, Sinusoidal Dither, and Ramp waveforms
+ * Multiplexer for output voltage, load current sense and die temperature
+
+ Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5529r.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,ad5529r-16 # 16-bit variant
+ - adi,ad5529r-12 # 12-bit variant
+
+ reg:
+ maxItems: 1
+
+ spi-max-frequency:
+ maximum: 25000000
+ description:
+ Maximum SPI frequency. The device supports SPI Mode 0 and Mode 3.
+ Read operations are limited to 25MHz maximum.
+
+ reset-gpios:
+ maxItems: 1
+ description:
+ GPIO connected to the RESET pin. Active low. When asserted low,
+ performs a power-on reset and initializes the device to its default state.
+
+ clear-gpios:
+ maxItems: 1
+ description:
+ GPIO connected to the CLEAR pin. Active low. When asserted low,
+ clears all DAC data registers without affecting configuration settings.
+
+ interrupts:
+ maxItems: 1
+ description:
+ Interrupt connected to the ALARM pin. Active low interrupt output
+ for overtemperature conditions, SPI CRC errors, and function completion.
+
+ tg0-gpios:
+ maxItems: 1
+ description:
+ GPIO connected to the TG0 toggle pin. Pulsing this pin allows all
+ selected DACs to be updated synchronously for coordinated output changes.
+
+ tg1-gpios:
+ maxItems: 1
+ description:
+ GPIO connected to the TG1 toggle pin. Pulsing this pin allows all
+ selected DACs to be updated synchronously for coordinated output changes.
+
+ tg2-gpios:
+ maxItems: 1
+ description:
+ GPIO connected to the TG2 toggle pin. Pulsing this pin allows all
+ selected DACs to be updated synchronously for coordinated output changes.
+
+ tg3-gpios:
+ maxItems: 1
+ description:
+ GPIO connected to the TG3 toggle pin. Pulsing this pin allows all
+ selected DACs to be updated synchronously for coordinated output changes.
+
+ io-channels:
+ maxItems: 1
+ description:
+ ADC channel connected to the MUXOUT pin for monitoring output voltage,
+ load current sense, and die temperature.
+
+ io-channel-names:
+ const: muxout
+
+ vdd-supply:
+ description: Digital power supply (1.08V to 1.98V)
+
+ avdd-supply:
+ description: Analog power supply (4.75V to 5.25V)
+
+ hvdd-supply:
+ description:
+ High voltage positive supply (7V to 45V). Supply voltage should be chosen
+ based on configured output ranges (see datasheet Table 9).
+
+ hvss-supply:
+ description:
+ High voltage negative supply (-22.5V to 0V). Required only when using bipolar
+ output ranges (±5V, ±10V, ±15V, ±20V). Supply voltage should be
+ chosen based on configured output ranges (see datasheet Table 9).
+
+ vref-supply:
+ description:
+ External voltage reference supply (4.056V to 4.136V, typically 4.096V).
+ When specified, the device uses external reference mode and the VREF pin becomes an
+ input. The device uses the internal 4.096V precision reference otherwise.
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+patternProperties:
+ "^channel@([0-9]|1[0-5])$":
+ type: object
+ description: Child nodes for individual channel configuration
+
+ properties:
+ reg:
+ description: Channel number.
+ minimum: 0
+ maximum: 15
+
+ adi,output-range-microvolt:
+ description: |
+ Output voltage range for this channel as [min, max] in microvolts.
+ If not specified, defaults to 0V to 5V range.
+ oneOf:
+ - items:
+ - const: 0
+ - enum: [5000000, 10000000, 20000000, 40000000]
+ - items:
+ - const: -5000000
+ - const: 5000000
+ - items:
+ - const: -10000000
+ - const: 10000000
+ - items:
+ - const: -15000000
+ - const: 15000000
+ - items:
+ - const: -20000000
+ - const: 20000000
+
+ required:
+ - reg
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - vdd-supply
+ - avdd-supply
+ - hvdd-supply
+
+dependencies:
+ spi-cpha: [ spi-cpol ]
+ spi-cpol: [ spi-cpha ]
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dac@0 {
+ compatible = "adi,ad5529r-16";
+ reg = <0>;
+ spi-max-frequency = <25000000>;
+
+ vdd-supply = <&vdd_regulator>;
+ avdd-supply = <&avdd_regulator>;
+ hvdd-supply = <&hvdd_regulator>;
+ hvss-supply = <&hvss_regulator>;
+
+ reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@0 {
+ reg = <0>;
+ adi,output-range-microvolt = <0 5000000>;
+ };
+
+ channel@1 {
+ reg = <1>;
+ adi,output-range-microvolt = <(-10000000) 10000000>;
+ };
+
+ channel@2 {
+ reg = <2>;
+ adi,output-range-microvolt = <0 40000000>;
+ };
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index d6c3c7d22403..320e84765ce6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1507,6 +1507,13 @@ W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/adc/adi,ad4851.yaml
F: drivers/iio/adc/ad4851.c
+ANALOG DEVICES INC AD5529R 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,ad5529r.yaml
+
ANALOG DEVICES INC AD5706R DRIVER
M: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
L: linux-iio@vger.kernel.org
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 2/2] iio: dac: Add AD5529R DAC driver support
2026-05-19 15:42 [PATCH v3 0/2] iio: dac: Add support for AD5529R DAC Janani Sunil
2026-05-19 15:42 ` [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R Janani Sunil
@ 2026-05-19 15:42 ` Janani Sunil
2026-05-19 16:22 ` sashiko-bot
2026-05-22 17:24 ` Jonathan Cameron
1 sibling, 2 replies; 19+ messages in thread
From: Janani Sunil @ 2026-05-19 15:42 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil,
Janani Sunil
Add support for AD5529R 16-channel, 12/16 bit Digital to Analog Converter
Signed-off-by: Janani Sunil <janani.sunil@analog.com>
---
MAINTAINERS | 1 +
drivers/iio/dac/Kconfig | 17 ++
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/ad5529r.c | 527 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 546 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 320e84765ce6..143714e27d51 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1513,6 +1513,7 @@ L: linux-iio@vger.kernel.org
S: Supported
W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
+F: drivers/iio/dac/ad5529r.c
ANALOG DEVICES INC AD5706R DRIVER
M: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 657c68e75542..bb1d59889a2a 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -134,6 +134,23 @@ config AD5449
To compile this driver as a module, choose M here: the
module will be called ad5449.
+config AD5529R
+ tristate "Analog Devices AD5529R High Voltage DAC driver"
+ depends on SPI_MASTER
+ select REGMAP_SPI
+ help
+ Say yes here to build support for Analog Devices AD5529R
+ 16-Channel, 12-Bit/16-Bit, 40V High Voltage Precision Digital to Analog
+ Converter.
+
+ The device features multiple output voltage ranges from -20V to +20V,
+ built-in 4.096V voltage reference, and digital functions including
+ toggle, dither, and ramp modes. Supports both 12-bit and 16-bit
+ resolution variants.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad5529r.
+
config AD5592R_BASE
tristate
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 003431798498..f35e060b3643 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_AD5446) += ad5446.o
obj-$(CONFIG_AD5446_SPI) += ad5446-spi.o
obj-$(CONFIG_AD5446_I2C) += ad5446-i2c.o
obj-$(CONFIG_AD5449) += ad5449.o
+obj-$(CONFIG_AD5529R) += ad5529r.o
obj-$(CONFIG_AD5592R_BASE) += ad5592r-base.o
obj-$(CONFIG_AD5592R) += ad5592r.o
obj-$(CONFIG_AD5593R) += ad5593r.o
diff --git a/drivers/iio/dac/ad5529r.c b/drivers/iio/dac/ad5529r.c
new file mode 100644
index 000000000000..9bb63030db95
--- /dev/null
+++ b/drivers/iio/dac/ad5529r.c
@@ -0,0 +1,527 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * AD5529R Digital-to-Analog Converter Driver
+ * 16-Channel, 12/16-Bit, 40V High Voltage Precision DAC
+ *
+ * Copyright 2026 Analog Devices Inc.
+ * Author: Janani Sunil <janani.sunil@analog.com>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.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/reset.h>
+#include <linux/spi/spi.h>
+
+#define AD5529R_REG_INTERFACE_CONFIG_A 0x00
+#define AD5529R_REG_DEVICE_CONFIG 0x02
+#define AD5529R_REG_CHIP_GRADE 0x06
+#define AD5529R_REG_SCRATCH_PAD 0x0A
+#define AD5529R_REG_SPI_REVISION 0x0B
+#define AD5529R_REG_VENDOR_H 0x0D
+#define AD5529R_REG_STREAM_MODE 0x0E
+#define AD5529R_REG_INTERFACE_STATUS_A 0x11
+#define AD5529R_REG_MULTI_DAC_CH_SEL 0x14
+#define AD5529R_REG_OUT_RANGE_BASE 0x3C
+#define AD5529R_REG_OUT_RANGE(ch) (AD5529R_REG_OUT_RANGE_BASE + (ch) * 2)
+#define AD5529R_REG_DAC_INPUT_A_BASE 0x148
+#define AD5529R_REG_DAC_INPUT_A(ch) (AD5529R_REG_DAC_INPUT_A_BASE + (ch) * 2)
+#define AD5529R_REG_DAC_DATA_READBACK_BASE 0x16A
+#define AD5529R_REG_TSENS_ALERT_FLAG 0x18C
+#define AD5529R_REG_TSENS_SHTD_FLAG 0x18E
+#define AD5529R_REG_FUNC_BUSY 0x1A0
+#define AD5529R_REG_REF_SEL 0x1A2
+#define AD5529R_REG_INIT_CRC_ERR_STAT 0x1A4
+#define AD5529R_REG_MULTI_DAC_HOTPATH_SW_LDAC 0x1A8
+
+#define AD5529R_INTERFACE_CONFIG_A_SW_RESET (BIT(7) | BIT(0))
+#define AD5529R_INTERFACE_CONFIG_A_ADDR_ASCENSION BIT(5)
+#define AD5529R_INTERFACE_CONFIG_A_SDO_ENABLE BIT(4)
+#define AD5529R_REF_SEL_MASK BIT(0)
+#define AD5529R_MAX_REGISTER 0x232
+#define AD5529R_8BIT_REG_MAX 0x13
+#define AD5529R_SPI_READ_FLAG 0x80
+
+struct ad5529r_model_data {
+ const char *model_name;
+ unsigned int resolution;
+ const struct iio_chan_spec *channels;
+ unsigned int num_channels;
+};
+
+#define AD5529R_DAC_CHANNEL(chan, bits) { \
+ .type = IIO_VOLTAGE, \
+ .indexed = 1, \
+ .output = 1, \
+ .channel = (chan), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_OFFSET), \
+ .scan_type = { \
+ .format = 'u', \
+ .realbits = (bits), \
+ .storagebits = 16, \
+ }, \
+}
+
+static const char * const ad5529r_supply_names[] = {
+ "vdd",
+ "avdd",
+ "hvdd",
+};
+
+static const struct iio_chan_spec ad5529r_channels_16bit[] = {
+ AD5529R_DAC_CHANNEL(0, 16),
+ AD5529R_DAC_CHANNEL(1, 16),
+ AD5529R_DAC_CHANNEL(2, 16),
+ AD5529R_DAC_CHANNEL(3, 16),
+ AD5529R_DAC_CHANNEL(4, 16),
+ AD5529R_DAC_CHANNEL(5, 16),
+ AD5529R_DAC_CHANNEL(6, 16),
+ AD5529R_DAC_CHANNEL(7, 16),
+ AD5529R_DAC_CHANNEL(8, 16),
+ AD5529R_DAC_CHANNEL(9, 16),
+ AD5529R_DAC_CHANNEL(10, 16),
+ AD5529R_DAC_CHANNEL(11, 16),
+ AD5529R_DAC_CHANNEL(12, 16),
+ AD5529R_DAC_CHANNEL(13, 16),
+ AD5529R_DAC_CHANNEL(14, 16),
+ AD5529R_DAC_CHANNEL(15, 16),
+};
+
+static const struct iio_chan_spec ad5529r_channels_12bit[] = {
+ AD5529R_DAC_CHANNEL(0, 12),
+ AD5529R_DAC_CHANNEL(1, 12),
+ AD5529R_DAC_CHANNEL(2, 12),
+ AD5529R_DAC_CHANNEL(3, 12),
+ AD5529R_DAC_CHANNEL(4, 12),
+ AD5529R_DAC_CHANNEL(5, 12),
+ AD5529R_DAC_CHANNEL(6, 12),
+ AD5529R_DAC_CHANNEL(7, 12),
+ AD5529R_DAC_CHANNEL(8, 12),
+ AD5529R_DAC_CHANNEL(9, 12),
+ AD5529R_DAC_CHANNEL(10, 12),
+ AD5529R_DAC_CHANNEL(11, 12),
+ AD5529R_DAC_CHANNEL(12, 12),
+ AD5529R_DAC_CHANNEL(13, 12),
+ AD5529R_DAC_CHANNEL(14, 12),
+ AD5529R_DAC_CHANNEL(15, 12),
+};
+
+static const struct ad5529r_model_data ad5529r_16bit_model_data = {
+ .model_name = "ad5529r-16",
+ .resolution = 16,
+ .channels = ad5529r_channels_16bit,
+ .num_channels = ARRAY_SIZE(ad5529r_channels_16bit),
+};
+
+static const struct ad5529r_model_data ad5529r_12bit_model_data = {
+ .model_name = "ad5529r-12",
+ .resolution = 12,
+ .channels = ad5529r_channels_12bit,
+ .num_channels = ARRAY_SIZE(ad5529r_channels_12bit),
+};
+
+enum ad5529r_output_range {
+ AD5529R_RANGE_0V_5V,
+ AD5529R_RANGE_0V_10V,
+ AD5529R_RANGE_0V_20V,
+ AD5529R_RANGE_0V_40V,
+ AD5529R_RANGE_NEG5V_5V,
+ AD5529R_RANGE_NEG10V_10V,
+ AD5529R_RANGE_NEG15V_15V,
+ AD5529R_RANGE_NEG20V_20V,
+};
+
+static const s32 ad5529r_output_ranges_mv[8][2] = {
+ [AD5529R_RANGE_0V_5V] = { 0, 5000 },
+ [AD5529R_RANGE_0V_10V] = { 0, 10000 },
+ [AD5529R_RANGE_0V_20V] = { 0, 20000 },
+ [AD5529R_RANGE_0V_40V] = { 0, 40000 },
+ [AD5529R_RANGE_NEG5V_5V] = { -5000, 5000 },
+ [AD5529R_RANGE_NEG10V_10V] = { -10000, 10000 },
+ [AD5529R_RANGE_NEG15V_15V] = { -15000, 15000 },
+ [AD5529R_RANGE_NEG20V_20V] = { -20000, 20000 },
+};
+
+struct ad5529r_state {
+ struct spi_device *spi;
+ const struct ad5529r_model_data *model_data;
+ struct regmap *regmap_8bit;
+ struct regmap *regmap_16bit;
+ struct regulator *vref_regulator;
+ enum ad5529r_output_range output_range_idx[16];
+};
+
+static const struct regmap_range ad5529r_8bit_readable_ranges[] = {
+ regmap_reg_range(AD5529R_REG_INTERFACE_CONFIG_A, AD5529R_REG_CHIP_GRADE),
+ regmap_reg_range(AD5529R_REG_SCRATCH_PAD, AD5529R_REG_VENDOR_H),
+ regmap_reg_range(AD5529R_REG_STREAM_MODE, AD5529R_REG_INTERFACE_STATUS_A),
+};
+
+static const struct regmap_range ad5529r_16bit_readable_ranges[] = {
+ regmap_reg_range(AD5529R_REG_MULTI_DAC_CH_SEL, AD5529R_REG_INIT_CRC_ERR_STAT),
+ regmap_reg_range(AD5529R_REG_MULTI_DAC_HOTPATH_SW_LDAC, AD5529R_MAX_REGISTER),
+};
+
+static const struct regmap_access_table ad5529r_8bit_readable_table = {
+ .yes_ranges = ad5529r_8bit_readable_ranges,
+ .n_yes_ranges = ARRAY_SIZE(ad5529r_8bit_readable_ranges),
+};
+
+static const struct regmap_access_table ad5529r_16bit_readable_table = {
+ .yes_ranges = ad5529r_16bit_readable_ranges,
+ .n_yes_ranges = ARRAY_SIZE(ad5529r_16bit_readable_ranges),
+};
+
+static const struct regmap_range ad5529r_8bit_read_only_ranges[] = {
+ regmap_reg_range(AD5529R_REG_DEVICE_CONFIG, AD5529R_REG_CHIP_GRADE),
+ regmap_reg_range(AD5529R_REG_SPI_REVISION, AD5529R_REG_VENDOR_H),
+};
+
+static const struct regmap_range ad5529r_16bit_read_only_ranges[] = {
+ regmap_reg_range(AD5529R_REG_DAC_DATA_READBACK_BASE,
+ (AD5529R_REG_DAC_DATA_READBACK_BASE + 15 * 2)),
+ regmap_reg_range(AD5529R_REG_TSENS_ALERT_FLAG, AD5529R_REG_TSENS_SHTD_FLAG),
+ regmap_reg_range(AD5529R_REG_FUNC_BUSY, AD5529R_REG_FUNC_BUSY),
+ regmap_reg_range(AD5529R_REG_INIT_CRC_ERR_STAT, AD5529R_REG_INIT_CRC_ERR_STAT),
+};
+
+static const struct regmap_access_table ad5529r_8bit_writeable_table = {
+ .no_ranges = ad5529r_8bit_read_only_ranges,
+ .n_no_ranges = ARRAY_SIZE(ad5529r_8bit_read_only_ranges),
+};
+
+static const struct regmap_access_table ad5529r_16bit_writeable_table = {
+ .no_ranges = ad5529r_16bit_read_only_ranges,
+ .n_no_ranges = ARRAY_SIZE(ad5529r_16bit_read_only_ranges),
+};
+
+static const struct regmap_config ad5529r_regmap_8bit_config = {
+ .name = "ad5529r-8bit",
+ .reg_bits = 16,
+ .val_bits = 8,
+ .max_register = AD5529R_8BIT_REG_MAX,
+ .read_flag_mask = AD5529R_SPI_READ_FLAG,
+ .rd_table = &ad5529r_8bit_readable_table,
+ .wr_table = &ad5529r_8bit_writeable_table,
+};
+
+static const struct regmap_config ad5529r_regmap_16bit_config = {
+ .name = "ad5529r-16bit",
+ .reg_bits = 16,
+ .val_bits = 16,
+ .max_register = AD5529R_MAX_REGISTER,
+ .read_flag_mask = AD5529R_SPI_READ_FLAG,
+ .val_format_endian = REGMAP_ENDIAN_LITTLE,
+ .rd_table = &ad5529r_16bit_readable_table,
+ .wr_table = &ad5529r_16bit_writeable_table,
+ .reg_stride = 2,
+};
+
+static struct regmap *ad5529r_get_regmap(struct ad5529r_state *st,
+ unsigned int reg)
+{
+ if (reg <= AD5529R_8BIT_REG_MAX)
+ return st->regmap_8bit;
+
+ return st->regmap_16bit;
+}
+
+static int ad5529r_reset(struct ad5529r_state *st)
+{
+ struct reset_control *rst;
+ int ret;
+
+ rst = devm_reset_control_get_optional_exclusive(&st->spi->dev, NULL);
+ if (IS_ERR(rst))
+ return PTR_ERR(rst);
+
+ if (rst) {
+ ret = reset_control_deassert(rst);
+ if (ret)
+ return ret;
+ } else {
+ ret = regmap_write(st->regmap_8bit, AD5529R_REG_INTERFACE_CONFIG_A,
+ AD5529R_INTERFACE_CONFIG_A_SW_RESET);
+ if (ret)
+ return ret;
+ }
+
+ fsleep(10000);
+
+ return regmap_write(st->regmap_8bit, AD5529R_REG_INTERFACE_CONFIG_A,
+ AD5529R_INTERFACE_CONFIG_A_SDO_ENABLE |
+ AD5529R_INTERFACE_CONFIG_A_ADDR_ASCENSION);
+}
+
+static int ad5529r_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct ad5529r_state *st = iio_priv(indio_dev);
+ unsigned int reg_addr, reg_val_h;
+ int ret, range_idx, span_mv;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ reg_addr = AD5529R_REG_DAC_INPUT_A(chan->channel);
+ ret = regmap_read(st->regmap_16bit, reg_addr, ®_val_h);
+ if (ret)
+ return ret;
+
+ *val = reg_val_h;
+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ range_idx = st->output_range_idx[chan->channel];
+
+ span_mv = ad5529r_output_ranges_mv[range_idx][1] -
+ ad5529r_output_ranges_mv[range_idx][0];
+ *val = span_mv;
+ *val2 = st->model_data->resolution;
+
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_CHAN_INFO_OFFSET:
+ range_idx = st->output_range_idx[chan->channel];
+
+ if (ad5529r_output_ranges_mv[range_idx][0] < 0)
+ *val = -(1 << (st->model_data->resolution - 1));
+ else
+ *val = 0;
+
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad5529r_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct ad5529r_state *st = iio_priv(indio_dev);
+ unsigned int reg_addr;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ if (val < 0 || val > GENMASK(st->model_data->resolution - 1, 0))
+ return -EINVAL;
+
+ reg_addr = AD5529R_REG_DAC_INPUT_A(chan->channel);
+
+ return regmap_write(st->regmap_16bit, reg_addr, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad5529r_find_output_range(const s32 *vals)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(ad5529r_output_ranges_mv); i++) {
+ if (vals[0] == ad5529r_output_ranges_mv[i][0] * 1000 &&
+ vals[1] == ad5529r_output_ranges_mv[i][1] * 1000)
+ return i;
+ }
+
+ return -EINVAL;
+}
+
+static int ad5529r_parse_channel_ranges(struct device *dev,
+ struct ad5529r_state *st)
+{
+ int ret, ch, range_idx;
+ s32 vals[2];
+
+ device_for_each_child_node_scoped(dev, child) {
+ range_idx = AD5529R_RANGE_0V_5V;
+
+ ret = fwnode_property_read_u32(child, "reg", &ch);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Missing reg property in channel node\n");
+
+ if (ch >= 16)
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid channel number: %d\n", ch);
+
+ if (!fwnode_property_read_u32_array(child,
+ "adi,output-range-microvolt",
+ vals, 2)) {
+ range_idx = ad5529r_find_output_range(vals);
+ if (range_idx < 0)
+ return dev_err_probe(dev, range_idx,
+ "Invalid range [%d %d] for ch %d\n",
+ vals[0], vals[1], ch);
+ }
+
+ st->output_range_idx[ch] = range_idx;
+ ret = regmap_write(st->regmap_16bit,
+ AD5529R_REG_OUT_RANGE(ch), range_idx);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to configure range for ch %d\n",
+ ch);
+ }
+
+ return 0;
+}
+
+static int ad5529r_debugfs_reg_read(struct ad5529r_state *st, unsigned int reg,
+ unsigned int *val)
+{
+ return regmap_read(ad5529r_get_regmap(st, reg), reg, val);
+}
+
+static int ad5529r_debugfs_reg_write(struct ad5529r_state *st, unsigned int reg,
+ unsigned int val)
+{
+ return regmap_write(ad5529r_get_regmap(st, reg), reg, val);
+}
+
+static int ad5529r_reg_access(struct iio_dev *indio_dev,
+ unsigned int reg,
+ unsigned int writeval,
+ unsigned int *readval)
+{
+ struct ad5529r_state *st = iio_priv(indio_dev);
+
+ if (readval)
+ return ad5529r_debugfs_reg_read(st, reg, readval);
+
+ return ad5529r_debugfs_reg_write(st, reg, writeval);
+}
+
+static void ad5529r_disable_regulator(void *regulator)
+{
+ regulator_disable(regulator);
+}
+
+static const struct iio_info ad5529r_info = {
+ .read_raw = ad5529r_read_raw,
+ .write_raw = ad5529r_write_raw,
+ .debugfs_reg_access = ad5529r_reg_access,
+};
+
+static int ad5529r_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct iio_dev *indio_dev;
+ struct ad5529r_state *st;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+
+ st->spi = spi;
+
+ st->model_data = spi_get_device_match_data(spi);
+ if (!st->model_data)
+ return dev_err_probe(dev, -EINVAL, "Failed to identify device variant\n");
+
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(ad5529r_supply_names),
+ ad5529r_supply_names);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to get and enable regulators\n");
+
+ ret = devm_regulator_get_enable_optional(dev, "hvss");
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to get and enable hvss regulator\n");
+
+ st->vref_regulator = devm_regulator_get_optional(dev, "vref");
+ if (IS_ERR(st->vref_regulator)) {
+ if (PTR_ERR(st->vref_regulator) != -ENODEV)
+ return dev_err_probe(dev, PTR_ERR(st->vref_regulator),
+ "Failed to get vref regulator\n");
+ st->vref_regulator = NULL;
+ }
+
+ if (st->vref_regulator) {
+ ret = regulator_enable(st->vref_regulator);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to enable vref regulator\n");
+
+ ret = devm_add_action_or_reset(dev, ad5529r_disable_regulator,
+ st->vref_regulator);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to add vref regulator cleanup\n");
+ }
+
+ st->regmap_8bit = devm_regmap_init_spi(spi, &ad5529r_regmap_8bit_config);
+ if (IS_ERR(st->regmap_8bit))
+ return dev_err_probe(dev, PTR_ERR(st->regmap_8bit),
+ "Failed to initialize 8-bit regmap\n");
+
+ st->regmap_16bit = devm_regmap_init_spi(spi, &ad5529r_regmap_16bit_config);
+ if (IS_ERR(st->regmap_16bit))
+ return dev_err_probe(dev, PTR_ERR(st->regmap_16bit),
+ "Failed to initialize 16-bit regmap\n");
+
+ ret = ad5529r_reset(st);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to reset device\n");
+
+ ret = regmap_update_bits(st->regmap_16bit, AD5529R_REG_REF_SEL,
+ AD5529R_REF_SEL_MASK,
+ st->vref_regulator ? 0 : AD5529R_REF_SEL_MASK);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to configure reference\n");
+
+ ret = ad5529r_parse_channel_ranges(dev, st);
+ if (ret)
+ return ret;
+
+ indio_dev->name = st->model_data->model_name;
+ indio_dev->info = &ad5529r_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = st->model_data->channels;
+ indio_dev->num_channels = st->model_data->num_channels;
+
+ return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id ad5529r_of_match[] = {
+ { .compatible = "adi,ad5529r-16", .data = &ad5529r_16bit_model_data },
+ { .compatible = "adi,ad5529r-12", .data = &ad5529r_12bit_model_data },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad5529r_of_match);
+
+static const struct spi_device_id ad5529r_id[] = {
+ { "ad5529r-16", .driver_data = (kernel_ulong_t)&ad5529r_16bit_model_data },
+ { "ad5529r-12", .driver_data = (kernel_ulong_t)&ad5529r_12bit_model_data },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, ad5529r_id);
+
+static struct spi_driver ad5529r_driver = {
+ .driver = {
+ .name = "ad5529r",
+ .of_match_table = ad5529r_of_match,
+ },
+ .probe = ad5529r_probe,
+ .id_table = ad5529r_id,
+};
+module_spi_driver(ad5529r_driver);
+
+MODULE_AUTHOR("Janani Sunil <janani.sunil@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5529R 12/16-bit DAC driver");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-05-19 15:42 ` [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R Janani Sunil
@ 2026-05-19 15:55 ` sashiko-bot
2026-05-22 17:02 ` Jonathan Cameron
2026-05-26 13:11 ` Rodrigo Alencar
2 siblings, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-05-19 15:55 UTC (permalink / raw)
To: Janani Sunil; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `patternProperties` regex for child channel nodes incorrectly expects decimal unit addresses instead of hexadecimal, causing valid device tree nodes to fail validation.
--
commit 137584b720b9b8966a6cb2fb8c6b4d98d2bf3d55
Author: Janani Sunil <janani.sunil@analog.com>
dt-bindings: iio: dac: Add AD5529R
This commit adds devicetree bindings for the AD5529R 16-channel 12/16-bit
high voltage DAC. It defines properties for programmable output ranges, SPI
communication, and GPIO pins.
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
> new file mode 100644
> index 0000000000000..eb66f6ca063dc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
[ ... ]
> +patternProperties:
> + "^channel@([0-9]|1[0-5])$":
[Severity: Medium]
Does this pattern incorrectly expect decimal unit addresses instead of
hexadecimal?
According to the Devicetree specification, node unit addresses must be
written in lower-case hexadecimal without a "0x" prefix. Since the AD5529R
has 16 channels, valid reg values of 0 through 15 correspond to unit
addresses 0 through f (e.g., channel@a).
This regex expects decimal strings 10 through 15 for values 10-15. If a
user correctly writes channel@a for channel 10, it will fail to match this
regex.
Because the schema enforces unevaluatedProperties: false at the top level,
these nodes will be rejected by validation tools. Alternatively, if the
user incorrectly names the node channel@10 to pass schema validation, dtc
will produce a unit address format mismatch warning since "10" is evaluated
as hex 16.
Could the regex be updated to match single-character hexadecimal values
(e.g., "^channel@[0-9a-f]$")?
> + type: object
> + description: Child nodes for individual channel configuration
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-ad5529r-driver-v3-0-267c0731aa68@analog.com?part=1
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/2] iio: dac: Add AD5529R DAC driver support
2026-05-19 15:42 ` [PATCH v3 2/2] iio: dac: Add AD5529R DAC driver support Janani Sunil
@ 2026-05-19 16:22 ` sashiko-bot
2026-05-22 17:24 ` Jonathan Cameron
1 sibling, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-05-19 16:22 UTC (permalink / raw)
To: Janani Sunil; +Cc: conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Incorrect error handling for the optional 'hvss' regulator causes probe failure when it is absent.
- [High] A signedness error during the parsing of the 'reg' property allows an out-of-bounds array write using a negative channel index.
- [High] Incomplete hardware reset sequence fails to guarantee a known device state.
- [Medium] The driver allows an external voltage reference to be provided but fails to read its voltage to dynamically scale the output ranges.
- [Medium] Reading raw DAC values from the write-only input register instead of the dedicated readback registers.
- [Low] Passing incompatible pointer types to `fwnode_property_read_u32` functions causes compiler warnings/errors.
- [Low] The `scan_type` struct inside `struct iio_chan_spec` is initialized with a non-existent member `.format`, causing a build failure.
--
commit c4b761222e21c0ab2352b5405104edc721f22ee1
Author: Janani Sunil <janani.sunil@analog.com>
Subject: iio: dac: Add AD5529R DAC driver support
Add support for AD5529R 16-channel, 12/16 bit Digital to Analog Converter
> diff --git a/drivers/iio/dac/ad5529r.c b/drivers/iio/dac/ad5529r.c
> new file mode 100644
> index 0000000000000..9bb63030db952
> --- /dev/null
> +++ b/drivers/iio/dac/ad5529r.c
> @@ -0,0 +1,527 @@
[ ... ]
> +#define AD5529R_DAC_CHANNEL(chan, bits) { \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .output = 1, \
> + .channel = (chan), \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_OFFSET), \
> + .scan_type = { \
> + .format = 'u', \
[Severity: Low]
Is this member name correct? struct iio_scan_type (defined in
include/linux/iio/types.h) does not have a member named format. It appears
the correct member name is sign.
Will attempting to compile this code result in a compiler error about
the struct having no member named format?
> + .realbits = (bits), \
> + .storagebits = 16, \
> + }, \
> +}
[ ... ]
> +static int ad5529r_reset(struct ad5529r_state *st)
> +{
> + struct reset_control *rst;
> + int ret;
> +
> + rst = devm_reset_control_get_optional_exclusive(&st->spi->dev, NULL);
> + if (IS_ERR(rst))
> + return PTR_ERR(rst);
> +
> + if (rst) {
> + ret = reset_control_deassert(rst);
[Severity: High]
Does this sequence guarantee a hardware reset?
Since the driver calls reset_control_deassert(rst) without first asserting
the reset line, if the device was already out of reset (e.g., left running
by the bootloader or after a warm reboot), deasserting an already deasserted
line might do nothing.
Would using reset_control_reset(rst) ensure the device receives a reset edge?
> + if (ret)
> + return ret;
> + } else {
[ ... ]
> +static int ad5529r_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct ad5529r_state *st = iio_priv(indio_dev);
> + unsigned int reg_addr, reg_val_h;
> + int ret, range_idx, span_mv;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + reg_addr = AD5529R_REG_DAC_INPUT_A(chan->channel);
[Severity: Medium]
Is it intentional to read back from the input register AD5529R_REG_DAC_INPUT_A?
The driver explicitly defines a dedicated range of registers for reading back
DAC data starting at AD5529R_REG_DAC_DATA_READBACK_BASE, which are marked as
read-only in the regmap configuration.
Could reading from the input register instead of the readback register return
zeroes or stale data depending on the hardware latching mechanism?
> + ret = regmap_read(st->regmap_16bit, reg_addr, ®_val_h);
> + if (ret)
> + return ret;
[ ... ]
> +static int ad5529r_parse_channel_ranges(struct device *dev,
> + struct ad5529r_state *st)
> +{
> + int ret, ch, range_idx;
> + s32 vals[2];
> +
> + device_for_each_child_node_scoped(dev, child) {
> + range_idx = AD5529R_RANGE_0V_5V;
> +
> + ret = fwnode_property_read_u32(child, "reg", &ch);
[Severity: Low]
Will passing the address of an int to fwnode_property_read_u32() cause
incompatible pointer type warnings? fwnode_property_read_u32() expects
a u32 *.
[Severity: High]
Could passing the address of the signed int ch to fwnode_property_read_u32()
allow an out-of-bounds array write?
If the device tree provides a reg value with the most significant bit set
(e.g., 0x80000000), it would be written as an unsigned 32-bit integer but
interpreted as a negative value by the signed integer ch.
The subsequent bounds check:
if (ch >= 16)
would evaluate to false for negative values, allowing the check to be bypassed.
The driver would then write to st->output_range_idx[ch] using a negative index.
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Missing reg property in channel node\n");
> +
> + if (ch >= 16)
> + return dev_err_probe(dev, -EINVAL,
> + "Invalid channel number: %d\n", ch);
> +
> + if (!fwnode_property_read_u32_array(child,
> + "adi,output-range-microvolt",
> + vals, 2)) {
[Severity: Low]
Will passing an array of s32 vals to fwnode_property_read_u32_array() cause
incompatible pointer type warnings? It expects a u32 *.
> + range_idx = ad5529r_find_output_range(vals);
> + if (range_idx < 0)
> + return dev_err_probe(dev, range_idx,
> + "Invalid range [%d %d] for ch %d\n",
> + vals[0], vals[1], ch);
> + }
> +
> + st->output_range_idx[ch] = range_idx;
[ ... ]
> +static int ad5529r_probe(struct spi_device *spi)
> +{
[ ... ]
> + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(ad5529r_supply_names),
> + ad5529r_supply_names);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to get and enable regulators\n");
> +
> + ret = devm_regulator_get_enable_optional(dev, "hvss");
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to get and enable hvss regulator\n");
[Severity: High]
Does this error handling prevent the driver from probing when the optional
hvss regulator is absent?
devm_regulator_get_enable_optional() returns -ENODEV if the optional regulator
is not provided in the device tree. Because the code checks if (ret) without
ignoring -ENODEV, this causes dev_err_probe() to return -ENODEV and fail
the probe entirely, effectively making the optional regulator mandatory.
> +
> + st->vref_regulator = devm_regulator_get_optional(dev, "vref");
> + if (IS_ERR(st->vref_regulator)) {
> + if (PTR_ERR(st->vref_regulator) != -ENODEV)
> + return dev_err_probe(dev, PTR_ERR(st->vref_regulator),
> + "Failed to get vref regulator\n");
> + st->vref_regulator = NULL;
> + }
> +
> + if (st->vref_regulator) {
> + ret = regulator_enable(st->vref_regulator);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to enable vref regulator\n");
> +
> + ret = devm_add_action_or_reset(dev, ad5529r_disable_regulator,
> + st->vref_regulator);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to add vref regulator cleanup\n");
> + }
[Severity: Medium]
If an external voltage reference is provided, should the driver query its
voltage to dynamically scale the output ranges?
The driver requests the optional vref regulator and configures the hardware
to use it instead of the internal 4.096V reference, but it doesn't seem to
read the actual voltage of the external reference.
The IIO output scale calculations in ad5529r_read_raw() appear to hardcode
the spans based on standard ranges that assume the nominal internal 4.096V
reference. If an external reference has a different voltage, will the reported
IIO scale be mathematically incorrect?
> +
> + st->regmap_8bit = devm_regmap_init_spi(spi, &ad5529r_regmap_8bit_config);
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-ad5529r-driver-v3-0-267c0731aa68@analog.com?part=2
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-05-19 15:42 ` [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R Janani Sunil
2026-05-19 15:55 ` sashiko-bot
@ 2026-05-22 17:02 ` Jonathan Cameron
2026-05-25 16:30 ` Conor Dooley
2026-05-26 13:11 ` Rodrigo Alencar
2 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2026-05-22 17:02 UTC (permalink / raw)
To: Janani Sunil
Cc: Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil
On Tue, 19 May 2026 17:42:58 +0200
Janani Sunil <janani.sunil@analog.com> wrote:
> Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> buffered voltage output digital-to-analog converter (DAC) with an
> integrated precision reference.
Note that I'm seeing this as Changes Requested in the DT-binding patchwork
but didn't see any replies.
Seems I didn't get the sashiko reply - nor did the IIO patchwork.
Hopefully Janani did!
https://patchwork.kernel.org/project/devicetree/patch/20260519-ad5529r-driver-v3-1-267c0731aa68@analog.com/
>
> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
> ---
> .../devicetree/bindings/iio/dac/adi,ad5529r.yaml | 217 +++++++++++++++++++++
> MAINTAINERS | 7 +
> 2 files changed, 224 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
> new file mode 100644
> index 000000000000..eb66f6ca063d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
> @@ -0,0 +1,217 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/adi,ad5529r.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD5529R 16-Channel 12/16-bit High Voltage DAC
> +
> +maintainers:
> + - Janani Sunil <janani.sunil@analog.com>
> +
> +description: |
> + The AD5529R is a 16-channel, 12-bit or 16-bit, high voltage, buffered voltage output
Long line. Check the wrap.
> + digital-to-analog converter (DAC) with an integrated precision reference.
> + The device operates from unipolar and bipolar supplies. It is guaranteed
> + monotonic and has built-in rail-to-rail output buffers that can source or
> + sink up to 25mA.
> +
> + Specifications:
> + * 16 independent 12-bit or 16-bit DAC channels
> + * Independently programmable output ranges: 0V to 5V, 0V to 10V, 0V to 20V,
> + 0V to 40V, ±5V, ±10V, ±15V, and ±20V
> + * The device supports SPI communication with Mode 0 and Mode 3.
> + * 4.096V precision reference, 12ppm/°C maximum
> + * Built-in function generation: Toggle, Sinusoidal Dither, and Ramp waveforms
> + * Multiplexer for output voltage, load current sense and die temperature
> +
> + Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5529r.pdf
> +
> +patternProperties:
> + "^channel@([0-9]|1[0-5])$":
Sashiko commented that this should be hex. Why the forcing to decimal?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/2] iio: dac: Add AD5529R DAC driver support
2026-05-19 15:42 ` [PATCH v3 2/2] iio: dac: Add AD5529R DAC driver support Janani Sunil
2026-05-19 16:22 ` sashiko-bot
@ 2026-05-22 17:24 ` Jonathan Cameron
1 sibling, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2026-05-22 17:24 UTC (permalink / raw)
To: Janani Sunil
Cc: Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil
On Tue, 19 May 2026 17:42:59 +0200
Janani Sunil <janani.sunil@analog.com> wrote:
> Add support for AD5529R 16-channel, 12/16 bit Digital to Analog Converter
>
> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
Hi Janani,
A few more things inline from a fresh read.
Have a good weekend
Jonathan
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 003431798498..f35e060b3643 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_AD5446) += ad5446.o
> obj-$(CONFIG_AD5446_SPI) += ad5446-spi.o
> obj-$(CONFIG_AD5446_I2C) += ad5446-i2c.o
> obj-$(CONFIG_AD5449) += ad5449.o
> +obj-$(CONFIG_AD5529R) += ad5529r.o
> obj-$(CONFIG_AD5592R_BASE) += ad5592r-base.o
> obj-$(CONFIG_AD5592R) += ad5592r.o
> obj-$(CONFIG_AD5593R) += ad5593r.o
> diff --git a/drivers/iio/dac/ad5529r.c b/drivers/iio/dac/ad5529r.c
> new file mode 100644
> index 000000000000..9bb63030db95
> --- /dev/null
> +++ b/drivers/iio/dac/ad5529r.c
> @@ -0,0 +1,527 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * AD5529R Digital-to-Analog Converter Driver
> + * 16-Channel, 12/16-Bit, 40V High Voltage Precision DAC
> + *
> + * Copyright 2026 Analog Devices Inc.
> + * Author: Janani Sunil <janani.sunil@analog.com>
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
Generally don't include this in drivers unless you actually use
stuff in that header. dev_printk.h etc are preferred.
> +#include <linux/err.h>
> +#include <linux/errno.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/reset.h>
> +#include <linux/spi/spi.h>
> +
> +#define AD5529R_REG_INTERFACE_CONFIG_A 0x00
> +#define AD5529R_REG_DEVICE_CONFIG 0x02
> +#define AD5529R_REG_CHIP_GRADE 0x06
> +#define AD5529R_REG_SCRATCH_PAD 0x0A
> +#define AD5529R_REG_SPI_REVISION 0x0B
> +#define AD5529R_REG_VENDOR_H 0x0D
> +#define AD5529R_REG_STREAM_MODE 0x0E
> +#define AD5529R_REG_INTERFACE_STATUS_A 0x11
> +#define AD5529R_REG_MULTI_DAC_CH_SEL 0x14
> +#define AD5529R_REG_OUT_RANGE_BASE 0x3C
> +#define AD5529R_REG_OUT_RANGE(ch) (AD5529R_REG_OUT_RANGE_BASE + (ch) * 2)
> +#define AD5529R_REG_DAC_INPUT_A_BASE 0x148
> +#define AD5529R_REG_DAC_INPUT_A(ch) (AD5529R_REG_DAC_INPUT_A_BASE + (ch) * 2)
> +#define AD5529R_REG_DAC_DATA_READBACK_BASE 0x16A
> +#define AD5529R_REG_TSENS_ALERT_FLAG 0x18C
> +#define AD5529R_REG_TSENS_SHTD_FLAG 0x18E
> +#define AD5529R_REG_FUNC_BUSY 0x1A0
> +#define AD5529R_REG_REF_SEL 0x1A2
> +#define AD5529R_REG_INIT_CRC_ERR_STAT 0x1A4
> +#define AD5529R_REG_MULTI_DAC_HOTPATH_SW_LDAC 0x1A8
> +
> +#define AD5529R_INTERFACE_CONFIG_A_SW_RESET (BIT(7) | BIT(0))
> +#define AD5529R_INTERFACE_CONFIG_A_ADDR_ASCENSION BIT(5)
> +#define AD5529R_INTERFACE_CONFIG_A_SDO_ENABLE BIT(4)
> +#define AD5529R_REF_SEL_MASK BIT(0)
Often when it's a single bit we don't call it MASK, but instead express
what the value with the bit set means.
> +#define AD5529R_MAX_REGISTER 0x232
> +#define AD5529R_8BIT_REG_MAX 0x13
> +#define AD5529R_SPI_READ_FLAG 0x80
> +static int ad5529r_reset(struct ad5529r_state *st)
> +{
> + struct reset_control *rst;
> + int ret;
> +
> + rst = devm_reset_control_get_optional_exclusive(&st->spi->dev, NULL);
> + if (IS_ERR(rst))
> + return PTR_ERR(rst);
> +
> + if (rst) {
> + ret = reset_control_deassert(rst);
> + if (ret)
> + return ret;
> + } else {
> + ret = regmap_write(st->regmap_8bit, AD5529R_REG_INTERFACE_CONFIG_A,
> + AD5529R_INTERFACE_CONFIG_A_SW_RESET);
> + if (ret)
> + return ret;
> + }
> +
> + fsleep(10000);
Comment on why this value - typically a datasheet reference.
> +
> + return regmap_write(st->regmap_8bit, AD5529R_REG_INTERFACE_CONFIG_A,
> + AD5529R_INTERFACE_CONFIG_A_SDO_ENABLE |
> + AD5529R_INTERFACE_CONFIG_A_ADDR_ASCENSION);
> +}
> +
> +static int ad5529r_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct ad5529r_state *st = iio_priv(indio_dev);
> + unsigned int reg_addr, reg_val_h;
> + int ret, range_idx, span_mv;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + reg_addr = AD5529R_REG_DAC_INPUT_A(chan->channel);
Sashiko made an interesting point here about whether the readback register
makes more sense here. I think not but maybe we should add a comment on
why. My understanding is we are only dealing with the A value of the
toggle for now and this therefore always reflects the value set.
> + ret = regmap_read(st->regmap_16bit, reg_addr, ®_val_h);
> + if (ret)
> + return ret;
> +
> + *val = reg_val_h;
> +
> + return IIO_VAL_INT;
> +
> +static int ad5529r_find_output_range(const s32 *vals)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(ad5529r_output_ranges_mv); i++) {
for (unsigned int i = 0; ...
> + if (vals[0] == ad5529r_output_ranges_mv[i][0] * 1000 &&
> + vals[1] == ad5529r_output_ranges_mv[i][1] * 1000)
> + return i;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int ad5529r_parse_channel_ranges(struct device *dev,
> + struct ad5529r_state *st)
> +{
> + int ret, ch, range_idx;
> + s32 vals[2];
> +
> + device_for_each_child_node_scoped(dev, child) {
> + range_idx = AD5529R_RANGE_0V_5V;
> +
> + ret = fwnode_property_read_u32(child, "reg", &ch);
Another sashiko one. Type is wrong. Need to pass in a u32 point. In practice
unlikely to be a problem but lets match what is expected.
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Missing reg property in channel node\n");
> +
> + if (ch >= 16)
> + return dev_err_probe(dev, -EINVAL,
> + "Invalid channel number: %d\n", ch);
> +
> + if (!fwnode_property_read_u32_array(child,
> + "adi,output-range-microvolt",
> + vals, 2)) {
Here I think it is deliberately reading into signed storage. Add a comment on that.
Might get the bot to leave it alone ;) ARRAY_SIZE(vals) instead of 2.
> + range_idx = ad5529r_find_output_range(vals);
> + if (range_idx < 0)
> + return dev_err_probe(dev, range_idx,
> + "Invalid range [%d %d] for ch %d\n",
> + vals[0], vals[1], ch);
> + }
> +
> + st->output_range_idx[ch] = range_idx;
> + ret = regmap_write(st->regmap_16bit,
> + AD5529R_REG_OUT_RANGE(ch), range_idx);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to configure range for ch %d\n",
> + ch);
> + }
> +
> + return 0;
> +}
> +
> +static int ad5529r_debugfs_reg_read(struct ad5529r_state *st, unsigned int reg,
> + unsigned int *val)
> +{
> + return regmap_read(ad5529r_get_regmap(st, reg), reg, val);
> +}
> +
> +static int ad5529r_debugfs_reg_write(struct ad5529r_state *st, unsigned int reg,
> + unsigned int val)
> +{
> + return regmap_write(ad5529r_get_regmap(st, reg), reg, val);
> +}
These two helpers don't seem worth having over putting the calls inline.
Particularly as both are getting the regmap.
> +
> +static int ad5529r_reg_access(struct iio_dev *indio_dev,
> + unsigned int reg,
> + unsigned int writeval,
> + unsigned int *readval)
> +{
> + struct ad5529r_state *st = iio_priv(indio_dev);
> +
> + if (readval)
> + return ad5529r_debugfs_reg_read(st, reg, readval);
> +
> + return ad5529r_debugfs_reg_write(st, reg, writeval);
> +}
> +static int ad5529r_probe(struct spi_device *spi)
> +{
> + struct device *dev = &spi->dev;
> + struct iio_dev *indio_dev;
> + struct ad5529r_state *st;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> +
> + st->spi = spi;
> +
> + st->model_data = spi_get_device_match_data(spi);
> + if (!st->model_data)
> + return dev_err_probe(dev, -EINVAL, "Failed to identify device variant\n");
> +
> + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(ad5529r_supply_names),
> + ad5529r_supply_names);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to get and enable regulators\n");
> +
> + ret = devm_regulator_get_enable_optional(dev, "hvss");
Sashiko spotted this. Try dropping hvss from your dt and see what return value you get.
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to get and enable hvss regulator\n");
> +
> + st->vref_regulator = devm_regulator_get_optional(dev, "vref");
> + if (IS_ERR(st->vref_regulator)) {
> + if (PTR_ERR(st->vref_regulator) != -ENODEV)
> + return dev_err_probe(dev, PTR_ERR(st->vref_regulator),
> + "Failed to get vref regulator\n");
> + st->vref_regulator = NULL;
> + }
> +
> + if (st->vref_regulator) {
> + ret = regulator_enable(st->vref_regulator);
If you aren't going to use it except to enable, use
devm_regulator_get_optional_enabled() and a bool flag.
Sashiko had a comment about the oddity of not reading the voltage, but I think
that's fine as the datasheet seems pretty insistent it must be 4.096V to work
correctly. Bit odd as it also provides a range of values. Ah well.
Perhaps add a comment somewhere to remind us of this assumption.
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to enable vref regulator\n");
> +
> + ret = devm_add_action_or_reset(dev, ad5529r_disable_regulator,
> + st->vref_regulator);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to add vref regulator cleanup\n");
> + }
> +
> + ret = regmap_update_bits(st->regmap_16bit, AD5529R_REG_REF_SEL,
> + AD5529R_REF_SEL_MASK,
> + st->vref_regulator ? 0 : AD5529R_REF_SEL_MASK);
regmap_assign_bits() using a flag for vref presence as mentioned above
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to configure reference\n");
> +static const struct spi_device_id ad5529r_id[] = {
> + { "ad5529r-16", .driver_data = (kernel_ulong_t)&ad5529r_16bit_model_data },
> + { "ad5529r-12", .driver_data = (kernel_ulong_t)&ad5529r_12bit_model_data },
.name =
Might be long enough you need to do.
{
.name = ....
.driver_data = (kernel_ulong_t)&...
> + { }
> +};
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-05-22 17:02 ` Jonathan Cameron
@ 2026-05-25 16:30 ` Conor Dooley
2026-05-26 12:35 ` Jonathan Cameron
0 siblings, 1 reply; 19+ messages in thread
From: Conor Dooley @ 2026-05-25 16:30 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Janani Sunil, Lars-Peter Clausen, Michael Hennerich,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
Shuah Khan, linux-iio, devicetree, linux-kernel, linux-doc,
Janani Sunil
[-- Attachment #1: Type: text/plain, Size: 3127 bytes --]
On Fri, May 22, 2026 at 06:02:07PM +0100, Jonathan Cameron wrote:
> On Tue, 19 May 2026 17:42:58 +0200
> Janani Sunil <janani.sunil@analog.com> wrote:
>
> > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > buffered voltage output digital-to-analog converter (DAC) with an
> > integrated precision reference.
>
> Note that I'm seeing this as Changes Requested in the DT-binding patchwork
> but didn't see any replies.
>
> Seems I didn't get the sashiko reply - nor did the IIO patchwork.
> Hopefully Janani did!
> https://patchwork.kernel.org/project/devicetree/patch/20260519-ad5529r-driver-v3-1-267c0731aa68@analog.com/
The sashiko stuff is per subsystem or something, so dt stuff only goes
to us and the submitters. I think that's kinda stupid but also probably
required to avoid pissing people off. There's quite a bit of noise
generated by it, especially with it repeating the same incorrect stuff
on every version.
>
> >
> > Signed-off-by: Janani Sunil <janani.sunil@analog.com>
> > ---
> > .../devicetree/bindings/iio/dac/adi,ad5529r.yaml | 217 +++++++++++++++++++++
> > MAINTAINERS | 7 +
> > 2 files changed, 224 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
> > new file mode 100644
> > index 000000000000..eb66f6ca063d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
> > @@ -0,0 +1,217 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/dac/adi,ad5529r.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices AD5529R 16-Channel 12/16-bit High Voltage DAC
> > +
> > +maintainers:
> > + - Janani Sunil <janani.sunil@analog.com>
> > +
> > +description: |
> > + The AD5529R is a 16-channel, 12-bit or 16-bit, high voltage, buffered voltage output
>
> Long line. Check the wrap.
>
> > + digital-to-analog converter (DAC) with an integrated precision reference.
> > + The device operates from unipolar and bipolar supplies. It is guaranteed
> > + monotonic and has built-in rail-to-rail output buffers that can source or
> > + sink up to 25mA.
> > +
> > + Specifications:
> > + * 16 independent 12-bit or 16-bit DAC channels
> > + * Independently programmable output ranges: 0V to 5V, 0V to 10V, 0V to 20V,
> > + 0V to 40V, ±5V, ±10V, ±15V, and ±20V
> > + * The device supports SPI communication with Mode 0 and Mode 3.
> > + * 4.096V precision reference, 12ppm/°C maximum
> > + * Built-in function generation: Toggle, Sinusoidal Dither, and Ramp waveforms
> > + * Multiplexer for output voltage, load current sense and die temperature
> > +
> > + Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5529r.pdf
> > +
>
> > +patternProperties:
> > + "^channel@([0-9]|1[0-5])$":
>
> Sashiko commented that this should be hex. Why the forcing to decimal?
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-05-25 16:30 ` Conor Dooley
@ 2026-05-26 12:35 ` Jonathan Cameron
0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2026-05-26 12:35 UTC (permalink / raw)
To: Conor Dooley
Cc: Janani Sunil, Lars-Peter Clausen, Michael Hennerich,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
Shuah Khan, linux-iio, devicetree, linux-kernel, linux-doc,
Janani Sunil
On Mon, 25 May 2026 17:30:02 +0100
Conor Dooley <conor@kernel.org> wrote:
> On Fri, May 22, 2026 at 06:02:07PM +0100, Jonathan Cameron wrote:
> > On Tue, 19 May 2026 17:42:58 +0200
> > Janani Sunil <janani.sunil@analog.com> wrote:
> >
> > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > buffered voltage output digital-to-analog converter (DAC) with an
> > > integrated precision reference.
> >
> > Note that I'm seeing this as Changes Requested in the DT-binding patchwork
> > but didn't see any replies.
> >
> > Seems I didn't get the sashiko reply - nor did the IIO patchwork.
> > Hopefully Janani did!
> > https://patchwork.kernel.org/project/devicetree/patch/20260519-ad5529r-driver-v3-1-267c0731aa68@analog.com/
>
> The sashiko stuff is per subsystem or something, so dt stuff only goes
> to us and the submitters. I think that's kinda stupid but also probably
> required to avoid pissing people off. There's quite a bit of noise
> generated by it, especially with it repeating the same incorrect stuff
> on every version.
When I get on top of the long weekend's backlog I'll start a discussion
on whether we ask for emails from Sashiko be sent to linux-iio.
I was nervous about doing that until I had some experience of how
reliable the reviews are but given I'm either cutting and pasting stuff
into threads or telling people to go look, might as well have those emails.
Jonathan
>
> >
> > >
> > > Signed-off-by: Janani Sunil <janani.sunil@analog.com>
> > > ---
> > > .../devicetree/bindings/iio/dac/adi,ad5529r.yaml | 217 +++++++++++++++++++++
> > > MAINTAINERS | 7 +
> > > 2 files changed, 224 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
> > > new file mode 100644
> > > index 000000000000..eb66f6ca063d
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
> > > @@ -0,0 +1,217 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iio/dac/adi,ad5529r.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Analog Devices AD5529R 16-Channel 12/16-bit High Voltage DAC
> > > +
> > > +maintainers:
> > > + - Janani Sunil <janani.sunil@analog.com>
> > > +
> > > +description: |
> > > + The AD5529R is a 16-channel, 12-bit or 16-bit, high voltage, buffered voltage output
> >
> > Long line. Check the wrap.
> >
> > > + digital-to-analog converter (DAC) with an integrated precision reference.
> > > + The device operates from unipolar and bipolar supplies. It is guaranteed
> > > + monotonic and has built-in rail-to-rail output buffers that can source or
> > > + sink up to 25mA.
> > > +
> > > + Specifications:
> > > + * 16 independent 12-bit or 16-bit DAC channels
> > > + * Independently programmable output ranges: 0V to 5V, 0V to 10V, 0V to 20V,
> > > + 0V to 40V, ±5V, ±10V, ±15V, and ±20V
> > > + * The device supports SPI communication with Mode 0 and Mode 3.
> > > + * 4.096V precision reference, 12ppm/°C maximum
> > > + * Built-in function generation: Toggle, Sinusoidal Dither, and Ramp waveforms
> > > + * Multiplexer for output voltage, load current sense and die temperature
> > > +
> > > + Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5529r.pdf
> > > +
> >
> > > +patternProperties:
> > > + "^channel@([0-9]|1[0-5])$":
> >
> > Sashiko commented that this should be hex. Why the forcing to decimal?
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-05-19 15:42 ` [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R Janani Sunil
2026-05-19 15:55 ` sashiko-bot
2026-05-22 17:02 ` Jonathan Cameron
@ 2026-05-26 13:11 ` Rodrigo Alencar
2026-06-09 14:47 ` Janani Sunil
2 siblings, 1 reply; 19+ messages in thread
From: Rodrigo Alencar @ 2026-05-26 13:11 UTC (permalink / raw)
To: Janani Sunil, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
Jonathan Corbet, Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil
On 26/05/19 05:42PM, Janani Sunil wrote:
> Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> buffered voltage output digital-to-analog converter (DAC) with an
> integrated precision reference.
...
> +properties:
> + compatible:
> + enum:
> + - adi,ad5529r-16 # 16-bit variant
> + - adi,ad5529r-12 # 12-bit variant
> +
> + reg:
> + maxItems: 1
> +
> + spi-max-frequency:
> + maximum: 25000000
> + description:
> + Maximum SPI frequency. The device supports SPI Mode 0 and Mode 3.
> + Read operations are limited to 25MHz maximum.
> +
> + reset-gpios:
> + maxItems: 1
> + description:
> + GPIO connected to the RESET pin. Active low. When asserted low,
> + performs a power-on reset and initializes the device to its default state.
> +
> + clear-gpios:
> + maxItems: 1
> + description:
> + GPIO connected to the CLEAR pin. Active low. When asserted low,
> + clears all DAC data registers without affecting configuration settings.
> +
> + interrupts:
> + maxItems: 1
> + description:
> + Interrupt connected to the ALARM pin. Active low interrupt output
> + for overtemperature conditions, SPI CRC errors, and function completion.
> +
> + tg0-gpios:
> + maxItems: 1
> + description:
> + GPIO connected to the TG0 toggle pin. Pulsing this pin allows all
> + selected DACs to be updated synchronously for coordinated output changes.
> +
> + tg1-gpios:
> + maxItems: 1
> + description:
> + GPIO connected to the TG1 toggle pin. Pulsing this pin allows all
> + selected DACs to be updated synchronously for coordinated output changes.
> +
> + tg2-gpios:
> + maxItems: 1
> + description:
> + GPIO connected to the TG2 toggle pin. Pulsing this pin allows all
> + selected DACs to be updated synchronously for coordinated output changes.
> +
> + tg3-gpios:
> + maxItems: 1
> + description:
> + GPIO connected to the TG3 toggle pin. Pulsing this pin allows all
> + selected DACs to be updated synchronously for coordinated output changes.
Those toggle pins might need to be connected to a pwm source, rather than using
them as regular gpios (or maybe allow for both).
> +
> + io-channels:
> + maxItems: 1
> + description:
> + ADC channel connected to the MUXOUT pin for monitoring output voltage,
> + load current sense, and die temperature.
> +
> + io-channel-names:
> + const: muxout
> +
> + vdd-supply:
> + description: Digital power supply (1.08V to 1.98V)
> +
> + avdd-supply:
> + description: Analog power supply (4.75V to 5.25V)
> +
> + hvdd-supply:
> + description:
> + High voltage positive supply (7V to 45V). Supply voltage should be chosen
> + based on configured output ranges (see datasheet Table 9).
> +
> + hvss-supply:
> + description:
> + High voltage negative supply (-22.5V to 0V). Required only when using bipolar
> + output ranges (±5V, ±10V, ±15V, ±20V). Supply voltage should be
> + chosen based on configured output ranges (see datasheet Table 9).
> +
> + vref-supply:
> + description:
> + External voltage reference supply (4.056V to 4.136V, typically 4.096V).
> + When specified, the device uses external reference mode and the VREF pin becomes an
> + input. The device uses the internal 4.096V precision reference otherwise.
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
Probably others may comment on that, but...
This parent node may support device addressing for multi-device support through
those ID pins. I suppose that each device may have its own power supplies or
other resources like the toggle pins or reset and enable.
That way I suppose that an example would look like...
> +
> +patternProperties:
> + "^channel@([0-9]|1[0-5])$":
> + type: object
> + description: Child nodes for individual channel configuration
> +
> + properties:
> + reg:
> + description: Channel number.
> + minimum: 0
> + maximum: 15
> +
> + adi,output-range-microvolt:
> + description: |
> + Output voltage range for this channel as [min, max] in microvolts.
> + If not specified, defaults to 0V to 5V range.
> + oneOf:
> + - items:
> + - const: 0
> + - enum: [5000000, 10000000, 20000000, 40000000]
> + - items:
> + - const: -5000000
> + - const: 5000000
> + - items:
> + - const: -10000000
> + - const: 10000000
> + - items:
> + - const: -15000000
> + - const: 15000000
> + - items:
> + - const: -20000000
> + - const: 20000000
> +
> + required:
> + - reg
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - vdd-supply
> + - avdd-supply
> + - hvdd-supply
> +
> +dependencies:
> + spi-cpha: [ spi-cpol ]
> + spi-cpol: [ spi-cpha ]
> +
> +allOf:
> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + dac@0 {
> + compatible = "adi,ad5529r-16";
> + reg = <0>;
> + spi-max-frequency = <25000000>;
> +
> + vdd-supply = <&vdd_regulator>;
> + avdd-supply = <&avdd_regulator>;
> + hvdd-supply = <&hvdd_regulator>;
> + hvss-supply = <&hvss_regulator>;
> +
> + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + channel@0 {
> + reg = <0>;
> + adi,output-range-microvolt = <0 5000000>;
> + };
> +
> + channel@1 {
> + reg = <1>;
> + adi,output-range-microvolt = <(-10000000) 10000000>;
> + };
> +
> + channel@2 {
> + reg = <2>;
> + adi,output-range-microvolt = <0 40000000>;
> + };
> + };
> + };
...
spi {
#address-cells = <1>;
#size-cells = <0>;
multi-dac@0 {
compatible = "adi,ad5529r-16";
reg = <0>;
spi-max-frequency = <25000000>;
#address-cells = <1>;
#size-cells = <0>;
dac@0 {
reg = <0>;
vdd-supply = <&vdd_regulator>;
avdd-supply = <&avdd_regulator>;
hvdd-supply = <&hvdd_regulator>;
hvss-supply = <&hvss_regulator>;
reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
#address-cells = <1>;
#size-cells = <0>;
channel@0 {
reg = <0>;
adi,output-range-microvolt = <0 5000000>;
};
channel@1 {
reg = <1>;
adi,output-range-microvolt = <(-10000000) 10000000>;
};
channel@2 {
reg = <2>;
adi,output-range-microvolt = <0 40000000>;
};
}
dac@1 {
reg = <1>;
vdd-supply = <&vdd_regulator>;
avdd-supply = <&avdd_regulator>;
hvdd-supply = <&hvdd_regulator>;
hvss-supply = <&hvss_regulator>;
reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
#address-cells = <1>;
#size-cells = <0>;
channel@0 {
reg = <0>;
adi,output-range-microvolt = <0 5000000>;
};
channel@1 {
reg = <1>;
adi,output-range-microvolt = <(-10000000) 10000000>;
};
}
};
};
then you might need something like:
patternProperties:
"^dac@[0-3]$":
and put most of the things under this node pattern.
So the main driver that you're putting together might need to handle up to four instances.
Even if your current driver cannot handle this, the dt-bindings might need cover that.
Need to double check if each dac node needs a separate compatible, so you would maybe populate
a platform data to be shared with the child nodes, which would be a separate driver.
(not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
--
Kind regards,
Rodrigo Alencar
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-05-26 13:11 ` Rodrigo Alencar
@ 2026-06-09 14:47 ` Janani Sunil
2026-06-14 19:44 ` Jonathan Cameron
0 siblings, 1 reply; 19+ messages in thread
From: Janani Sunil @ 2026-06-09 14:47 UTC (permalink / raw)
To: Rodrigo Alencar, Janani Sunil, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Jonathan Corbet, Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-doc
On 5/26/26 15:11, Rodrigo Alencar wrote:
> On 26/05/19 05:42PM, Janani Sunil wrote:
>> Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
>> buffered voltage output digital-to-analog converter (DAC) with an
>> integrated precision reference.
> ...
> Probably others may comment on that, but...
>
> This parent node may support device addressing for multi-device support through
> those ID pins. I suppose that each device may have its own power supplies or
> other resources like the toggle pins or reset and enable.
>
> That way I suppose that an example would look like...
>
>> +
>> +patternProperties:
>> + "^channel@([0-9]|1[0-5])$":
>> + type: object
>> + description: Child nodes for individual channel configuration
>> +
>> + properties:
>> + reg:
>> + description: Channel number.
>> + minimum: 0
>> + maximum: 15
>> +
>> + adi,output-range-microvolt:
>> + description: |
>> + Output voltage range for this channel as [min, max] in microvolts.
>> + If not specified, defaults to 0V to 5V range.
>> + oneOf:
>> + - items:
>> + - const: 0
>> + - enum: [5000000, 10000000, 20000000, 40000000]
>> + - items:
>> + - const: -5000000
>> + - const: 5000000
>> + - items:
>> + - const: -10000000
>> + - const: 10000000
>> + - items:
>> + - const: -15000000
>> + - const: 15000000
>> + - items:
>> + - const: -20000000
>> + - const: 20000000
>> +
>> + required:
>> + - reg
>> +
>> + additionalProperties: false
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - vdd-supply
>> + - avdd-supply
>> + - hvdd-supply
>> +
>> +dependencies:
>> + spi-cpha: [ spi-cpol ]
>> + spi-cpol: [ spi-cpha ]
>> +
>> +allOf:
>> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/gpio/gpio.h>
>> +
>> + spi {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + dac@0 {
>> + compatible = "adi,ad5529r-16";
>> + reg = <0>;
>> + spi-max-frequency = <25000000>;
>> +
>> + vdd-supply = <&vdd_regulator>;
>> + avdd-supply = <&avdd_regulator>;
>> + hvdd-supply = <&hvdd_regulator>;
>> + hvss-supply = <&hvss_regulator>;
>> +
>> + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + channel@0 {
>> + reg = <0>;
>> + adi,output-range-microvolt = <0 5000000>;
>> + };
>> +
>> + channel@1 {
>> + reg = <1>;
>> + adi,output-range-microvolt = <(-10000000) 10000000>;
>> + };
>> +
>> + channel@2 {
>> + reg = <2>;
>> + adi,output-range-microvolt = <0 40000000>;
>> + };
>> + };
>> + };
> ...
>
> spi {
> #address-cells = <1>;
> #size-cells = <0>;
>
> multi-dac@0 {
> compatible = "adi,ad5529r-16";
> reg = <0>;
> spi-max-frequency = <25000000>;
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> dac@0 {
> reg = <0>;
> vdd-supply = <&vdd_regulator>;
> avdd-supply = <&avdd_regulator>;
> hvdd-supply = <&hvdd_regulator>;
> hvss-supply = <&hvss_regulator>;
>
> reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> channel@0 {
> reg = <0>;
> adi,output-range-microvolt = <0 5000000>;
> };
>
> channel@1 {
> reg = <1>;
> adi,output-range-microvolt = <(-10000000) 10000000>;
> };
>
> channel@2 {
> reg = <2>;
> adi,output-range-microvolt = <0 40000000>;
> };
> }
>
> dac@1 {
> reg = <1>;
> vdd-supply = <&vdd_regulator>;
> avdd-supply = <&avdd_regulator>;
> hvdd-supply = <&hvdd_regulator>;
> hvss-supply = <&hvss_regulator>;
>
> reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> channel@0 {
> reg = <0>;
> adi,output-range-microvolt = <0 5000000>;
> };
>
> channel@1 {
> reg = <1>;
> adi,output-range-microvolt = <(-10000000) 10000000>;
> };
> }
> };
> };
>
> then you might need something like:
>
> patternProperties:
> "^dac@[0-3]$":
>
> and put most of the things under this node pattern.
>
> So the main driver that you're putting together might need to handle up to four instances.
> Even if your current driver cannot handle this, the dt-bindings might need cover that.
>
> Need to double check if each dac node needs a separate compatible, so you would maybe populate
> a platform data to be shared with the child nodes, which would be a separate driver.
> (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
Hi Rodrigo,
Thank you for looking at this.
For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
hardware/use case we have only needs one device node and the driver is written around that model as well.
While the device addressing pins could allow multi-device topology, we do not have an actual platform using
that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
speculatively without a validating use case.
Best Regards,
Janani Sunil
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-06-09 14:47 ` Janani Sunil
@ 2026-06-14 19:44 ` Jonathan Cameron
2026-06-19 10:33 ` Janani Sunil
0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2026-06-14 19:44 UTC (permalink / raw)
To: Janani Sunil
Cc: Rodrigo Alencar, Janani Sunil, Lars-Peter Clausen,
Michael Hennerich, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
Jonathan Corbet, Shuah Khan, linux-iio, devicetree, linux-kernel,
linux-doc, Mark Brown
On Tue, 9 Jun 2026 16:47:23 +0200
Janani Sunil <jan.sun97@gmail.com> wrote:
> On 5/26/26 15:11, Rodrigo Alencar wrote:
> > On 26/05/19 05:42PM, Janani Sunil wrote:
> >> Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> >> buffered voltage output digital-to-analog converter (DAC) with an
> >> integrated precision reference.
> > ...
> > Probably others may comment on that, but...
> >
> > This parent node may support device addressing for multi-device support through
> > those ID pins. I suppose that each device may have its own power supplies or
> > other resources like the toggle pins or reset and enable.
> >
> > That way I suppose that an example would look like...
> >
> >> +
> >> +patternProperties:
> >> + "^channel@([0-9]|1[0-5])$":
> >> + type: object
> >> + description: Child nodes for individual channel configuration
> >> +
> >> + properties:
> >> + reg:
> >> + description: Channel number.
> >> + minimum: 0
> >> + maximum: 15
> >> +
> >> + adi,output-range-microvolt:
> >> + description: |
> >> + Output voltage range for this channel as [min, max] in microvolts.
> >> + If not specified, defaults to 0V to 5V range.
> >> + oneOf:
> >> + - items:
> >> + - const: 0
> >> + - enum: [5000000, 10000000, 20000000, 40000000]
> >> + - items:
> >> + - const: -5000000
> >> + - const: 5000000
> >> + - items:
> >> + - const: -10000000
> >> + - const: 10000000
> >> + - items:
> >> + - const: -15000000
> >> + - const: 15000000
> >> + - items:
> >> + - const: -20000000
> >> + - const: 20000000
> >> +
> >> + required:
> >> + - reg
> >> +
> >> + additionalProperties: false
> >> +
> >> +required:
> >> + - compatible
> >> + - reg
> >> + - vdd-supply
> >> + - avdd-supply
> >> + - hvdd-supply
> >> +
> >> +dependencies:
> >> + spi-cpha: [ spi-cpol ]
> >> + spi-cpol: [ spi-cpha ]
> >> +
> >> +allOf:
> >> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> >> +
> >> +unevaluatedProperties: false
> >> +
> >> +examples:
> >> + - |
> >> + #include <dt-bindings/gpio/gpio.h>
> >> +
> >> + spi {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + dac@0 {
> >> + compatible = "adi,ad5529r-16";
> >> + reg = <0>;
> >> + spi-max-frequency = <25000000>;
> >> +
> >> + vdd-supply = <&vdd_regulator>;
> >> + avdd-supply = <&avdd_regulator>;
> >> + hvdd-supply = <&hvdd_regulator>;
> >> + hvss-supply = <&hvss_regulator>;
> >> +
> >> + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> >> +
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + channel@0 {
> >> + reg = <0>;
> >> + adi,output-range-microvolt = <0 5000000>;
> >> + };
> >> +
> >> + channel@1 {
> >> + reg = <1>;
> >> + adi,output-range-microvolt = <(-10000000) 10000000>;
> >> + };
> >> +
> >> + channel@2 {
> >> + reg = <2>;
> >> + adi,output-range-microvolt = <0 40000000>;
> >> + };
> >> + };
> >> + };
> > ...
> >
> > spi {
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > multi-dac@0 {
> > compatible = "adi,ad5529r-16";
> > reg = <0>;
> > spi-max-frequency = <25000000>;
> >
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > dac@0 {
> > reg = <0>;
> > vdd-supply = <&vdd_regulator>;
> > avdd-supply = <&avdd_regulator>;
> > hvdd-supply = <&hvdd_regulator>;
> > hvss-supply = <&hvss_regulator>;
> >
> > reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> >
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > channel@0 {
> > reg = <0>;
> > adi,output-range-microvolt = <0 5000000>;
> > };
> >
> > channel@1 {
> > reg = <1>;
> > adi,output-range-microvolt = <(-10000000) 10000000>;
> > };
> >
> > channel@2 {
> > reg = <2>;
> > adi,output-range-microvolt = <0 40000000>;
> > };
> > }
> >
> > dac@1 {
> > reg = <1>;
> > vdd-supply = <&vdd_regulator>;
> > avdd-supply = <&avdd_regulator>;
> > hvdd-supply = <&hvdd_regulator>;
> > hvss-supply = <&hvss_regulator>;
> >
> > reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> >
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > channel@0 {
> > reg = <0>;
> > adi,output-range-microvolt = <0 5000000>;
> > };
> >
> > channel@1 {
> > reg = <1>;
> > adi,output-range-microvolt = <(-10000000) 10000000>;
> > };
> > }
> > };
> > };
> >
> > then you might need something like:
> >
> > patternProperties:
> > "^dac@[0-3]$":
> >
> > and put most of the things under this node pattern.
> >
> > So the main driver that you're putting together might need to handle up to four instances.
> > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> >
> > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > a platform data to be shared with the child nodes, which would be a separate driver.
> > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
>
> Hi Rodrigo,
>
> Thank you for looking at this.
>
> For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> hardware/use case we have only needs one device node and the driver is written around that model as well.
> While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> speculatively without a validating use case.
Interesting feature - kind of similar to address control on a typical i2c bus device, or
looking at it another way a kind of distributed SPI mux.
Challenge of a binding is we need to anticipate the future. So I think we do need something
like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
That would leave the path open to supporting the addressing at a later date.
An alternative might be to look at it like a chained device setup. In those we pretend there
is just one device with a lot of channels etc. The snag is that here things are more loosely
coupled whereas for those devices it tends to be you have to read / write the same register
in all devices in the chain as one big SPI message.
+CC Mark Brown as he may know of some precedence for this feature. For his reference..
- Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
value that matches that or they are ignored. Thus a single bus + 1 chip select can
be used to talk to 4 devices. Question is what that looks like in device tree + I guess
longer term how to support it cleanly in SPI.
Jonathan
>
> Best Regards,
> Janani Sunil
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-06-14 19:44 ` Jonathan Cameron
@ 2026-06-19 10:33 ` Janani Sunil
2026-06-19 11:31 ` Nuno Sá
2026-06-19 11:36 ` Conor Dooley
0 siblings, 2 replies; 19+ messages in thread
From: Janani Sunil @ 2026-06-19 10:33 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Rodrigo Alencar, Janani Sunil, Lars-Peter Clausen,
Michael Hennerich, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
Jonathan Corbet, Shuah Khan, linux-iio, devicetree, linux-kernel,
linux-doc, Mark Brown
On 6/14/26 21:44, Jonathan Cameron wrote:
> On Tue, 9 Jun 2026 16:47:23 +0200
> Janani Sunil <jan.sun97@gmail.com> wrote:
>
>> On 5/26/26 15:11, Rodrigo Alencar wrote:
>>> On 26/05/19 05:42PM, Janani Sunil wrote:
>>>> Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
>>>> buffered voltage output digital-to-analog converter (DAC) with an
>>>> integrated precision reference.
>>> ...
>>> Probably others may comment on that, but...
>>>
>>> This parent node may support device addressing for multi-device support through
>>> those ID pins. I suppose that each device may have its own power supplies or
>>> other resources like the toggle pins or reset and enable.
>>>
>>> That way I suppose that an example would look like...
>>>
>>>> +
>>>> +patternProperties:
>>>> + "^channel@([0-9]|1[0-5])$":
>>>> + type: object
>>>> + description: Child nodes for individual channel configuration
>>>> +
>>>> + properties:
>>>> + reg:
>>>> + description: Channel number.
>>>> + minimum: 0
>>>> + maximum: 15
>>>> +
>>>> + adi,output-range-microvolt:
>>>> + description: |
>>>> + Output voltage range for this channel as [min, max] in microvolts.
>>>> + If not specified, defaults to 0V to 5V range.
>>>> + oneOf:
>>>> + - items:
>>>> + - const: 0
>>>> + - enum: [5000000, 10000000, 20000000, 40000000]
>>>> + - items:
>>>> + - const: -5000000
>>>> + - const: 5000000
>>>> + - items:
>>>> + - const: -10000000
>>>> + - const: 10000000
>>>> + - items:
>>>> + - const: -15000000
>>>> + - const: 15000000
>>>> + - items:
>>>> + - const: -20000000
>>>> + - const: 20000000
>>>> +
>>>> + required:
>>>> + - reg
>>>> +
>>>> + additionalProperties: false
>>>> +
>>>> +required:
>>>> + - compatible
>>>> + - reg
>>>> + - vdd-supply
>>>> + - avdd-supply
>>>> + - hvdd-supply
>>>> +
>>>> +dependencies:
>>>> + spi-cpha: [ spi-cpol ]
>>>> + spi-cpol: [ spi-cpha ]
>>>> +
>>>> +allOf:
>>>> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>>> +
>>>> +unevaluatedProperties: false
>>>> +
>>>> +examples:
>>>> + - |
>>>> + #include <dt-bindings/gpio/gpio.h>
>>>> +
>>>> + spi {
>>>> + #address-cells = <1>;
>>>> + #size-cells = <0>;
>>>> +
>>>> + dac@0 {
>>>> + compatible = "adi,ad5529r-16";
>>>> + reg = <0>;
>>>> + spi-max-frequency = <25000000>;
>>>> +
>>>> + vdd-supply = <&vdd_regulator>;
>>>> + avdd-supply = <&avdd_regulator>;
>>>> + hvdd-supply = <&hvdd_regulator>;
>>>> + hvss-supply = <&hvss_regulator>;
>>>> +
>>>> + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
>>>> +
>>>> + #address-cells = <1>;
>>>> + #size-cells = <0>;
>>>> +
>>>> + channel@0 {
>>>> + reg = <0>;
>>>> + adi,output-range-microvolt = <0 5000000>;
>>>> + };
>>>> +
>>>> + channel@1 {
>>>> + reg = <1>;
>>>> + adi,output-range-microvolt = <(-10000000) 10000000>;
>>>> + };
>>>> +
>>>> + channel@2 {
>>>> + reg = <2>;
>>>> + adi,output-range-microvolt = <0 40000000>;
>>>> + };
>>>> + };
>>>> + };
>>> ...
>>>
>>> spi {
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>>
>>> multi-dac@0 {
>>> compatible = "adi,ad5529r-16";
>>> reg = <0>;
>>> spi-max-frequency = <25000000>;
>>>
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>>
>>> dac@0 {
>>> reg = <0>;
>>> vdd-supply = <&vdd_regulator>;
>>> avdd-supply = <&avdd_regulator>;
>>> hvdd-supply = <&hvdd_regulator>;
>>> hvss-supply = <&hvss_regulator>;
>>>
>>> reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
>>>
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>>
>>> channel@0 {
>>> reg = <0>;
>>> adi,output-range-microvolt = <0 5000000>;
>>> };
>>>
>>> channel@1 {
>>> reg = <1>;
>>> adi,output-range-microvolt = <(-10000000) 10000000>;
>>> };
>>>
>>> channel@2 {
>>> reg = <2>;
>>> adi,output-range-microvolt = <0 40000000>;
>>> };
>>> }
>>>
>>> dac@1 {
>>> reg = <1>;
>>> vdd-supply = <&vdd_regulator>;
>>> avdd-supply = <&avdd_regulator>;
>>> hvdd-supply = <&hvdd_regulator>;
>>> hvss-supply = <&hvss_regulator>;
>>>
>>> reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
>>>
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>>
>>> channel@0 {
>>> reg = <0>;
>>> adi,output-range-microvolt = <0 5000000>;
>>> };
>>>
>>> channel@1 {
>>> reg = <1>;
>>> adi,output-range-microvolt = <(-10000000) 10000000>;
>>> };
>>> }
>>> };
>>> };
>>>
>>> then you might need something like:
>>>
>>> patternProperties:
>>> "^dac@[0-3]$":
>>>
>>> and put most of the things under this node pattern.
>>>
>>> So the main driver that you're putting together might need to handle up to four instances.
>>> Even if your current driver cannot handle this, the dt-bindings might need cover that.
>>>
>>> Need to double check if each dac node needs a separate compatible, so you would maybe populate
>>> a platform data to be shared with the child nodes, which would be a separate driver.
>>> (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
>> Hi Rodrigo,
>>
>> Thank you for looking at this.
>>
>> For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
>> hardware/use case we have only needs one device node and the driver is written around that model as well.
>> While the device addressing pins could allow multi-device topology, we do not have an actual platform using
>> that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
>> speculatively without a validating use case.
> Interesting feature - kind of similar to address control on a typical i2c bus device, or
> looking at it another way a kind of distributed SPI mux.
>
> Challenge of a binding is we need to anticipate the future. So I think we do need something
> like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> That would leave the path open to supporting the addressing at a later date.
> An alternative might be to look at it like a chained device setup. In those we pretend there
> is just one device with a lot of channels etc. The snag is that here things are more loosely
> coupled whereas for those devices it tends to be you have to read / write the same register
> in all devices in the chain as one big SPI message.
>
> +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> - Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
> value that matches that or they are ignored. Thus a single bus + 1 chip select can
> be used to talk to 4 devices. Question is what that looks like in device tree + I guess
> longer term how to support it cleanly in SPI.
>
> Jonathan
Hi Jonathan, Rob, Krzysztof, Conor,
One possible model that would also allow mixing the 12-bit and 16-bit variants would be to treat the parent node
as the shared SPI transport only, and let each dac@N child carry its own compatible.
Rob, Krzysztof, Conor — wanted to get your input on whether this is an acceptable binding pattern.
properties:
compatible:
const: adi,ad5529r-bus
patternProperties:
"^dac@[0-3]$":
type: object
properties:
compatible:
enum:
- adi,ad5529r-16
- adi,ad5529r-12
reg:
minimum: 0
maximum: 3
With a DT example such as:
ad5529r@0 {
compatible = "adi,ad5529r-bus";
reg = <0>;
dac@0 {
compatible = "adi,ad5529r-16";
reg = <0>;
};
dac@1 {
compatible = "adi,ad5529r-12";
reg = <1>;
};
};
The downside is that it introduces adi,ad5529r-bus as a compatible that does not correspond to an actual
standalone device variant - it would require a parent driver to manage the shared SPI transport and enumerate the
child devices. The actual DAC functionality is handled by the matching per-child compatibles(12 or 16 bit).
Is this an acceptable pattern, or is there a preferred way to model this type of addressing scheme?
Regards,
Janani Sunil
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-06-19 10:33 ` Janani Sunil
@ 2026-06-19 11:31 ` Nuno Sá
2026-06-19 11:36 ` Conor Dooley
1 sibling, 0 replies; 19+ messages in thread
From: Nuno Sá @ 2026-06-19 11:31 UTC (permalink / raw)
To: Janani Sunil
Cc: Jonathan Cameron, Rodrigo Alencar, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:
>
> On 6/14/26 21:44, Jonathan Cameron wrote:
> > On Tue, 9 Jun 2026 16:47:23 +0200
> > Janani Sunil <jan.sun97@gmail.com> wrote:
> >
>
> Hi Jonathan, Rob, Krzysztof, Conor,
>
> One possible model that would also allow mixing the 12-bit and 16-bit variants would be to treat the parent node
> as the shared SPI transport only, and let each dac@N child carry its own compatible.
>
> Rob, Krzysztof, Conor — wanted to get your input on whether this is an acceptable binding pattern.
>
> properties:
> compatible:
> const: adi,ad5529r-bus
>
> patternProperties:
> "^dac@[0-3]$":
> type: object
> properties:
> compatible:
> enum:
> - adi,ad5529r-16
> - adi,ad5529r-12
> reg:
> minimum: 0
> maximum: 3
>
> With a DT example such as:
>
> ad5529r@0 {
> compatible = "adi,ad5529r-bus";
> reg = <0>;
>
> dac@0 {
> compatible = "adi,ad5529r-16";
> reg = <0>;
> };
>
> dac@1 {
> compatible = "adi,ad5529r-12";
> reg = <1>;
> };
> };
>
> The downside is that it introduces adi,ad5529r-bus as a compatible that does not correspond to an actual
> standalone device variant - it would require a parent driver to manage the shared SPI transport and enumerate the
> child devices. The actual DAC functionality is handled by the matching per-child compatibles(12 or 16 bit).
> Is this an acceptable pattern, or is there a preferred way to model this type of addressing scheme?
>
At some point, I wondered if we can't just have this at spi level? Like
(in the simplest terms) a new spi-peripheral property that would allow
devices to share the same CS. Then we would need an adi,pin-id kind of
property for this device but the bindings would be pretty much as if we
only supported one device.
I see Mark is already in the loop, maybe he has seen this kind of things
before.
- Nuno Sá
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-06-19 10:33 ` Janani Sunil
2026-06-19 11:31 ` Nuno Sá
@ 2026-06-19 11:36 ` Conor Dooley
2026-06-19 11:40 ` Conor Dooley
1 sibling, 1 reply; 19+ messages in thread
From: Conor Dooley @ 2026-06-19 11:36 UTC (permalink / raw)
To: Janani Sunil
Cc: Jonathan Cameron, Rodrigo Alencar, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
[-- Attachment #1: Type: text/plain, Size: 8557 bytes --]
On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:
>
> On 6/14/26 21:44, Jonathan Cameron wrote:
> > On Tue, 9 Jun 2026 16:47:23 +0200
> > Janani Sunil <jan.sun97@gmail.com> wrote:
> >
> > > On 5/26/26 15:11, Rodrigo Alencar wrote:
> > > > On 26/05/19 05:42PM, Janani Sunil wrote:
> > > > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > > > buffered voltage output digital-to-analog converter (DAC) with an
> > > > > integrated precision reference.
> > > > ...
> > > > Probably others may comment on that, but...
> > > >
> > > > This parent node may support device addressing for multi-device support through
> > > > those ID pins. I suppose that each device may have its own power supplies or
> > > > other resources like the toggle pins or reset and enable.
> > > >
> > > > That way I suppose that an example would look like...
> > > > > +
> > > > > +patternProperties:
> > > > > + "^channel@([0-9]|1[0-5])$":
> > > > > + type: object
> > > > > + description: Child nodes for individual channel configuration
> > > > > +
> > > > > + properties:
> > > > > + reg:
> > > > > + description: Channel number.
> > > > > + minimum: 0
> > > > > + maximum: 15
> > > > > +
> > > > > + adi,output-range-microvolt:
> > > > > + description: |
> > > > > + Output voltage range for this channel as [min, max] in microvolts.
> > > > > + If not specified, defaults to 0V to 5V range.
> > > > > + oneOf:
> > > > > + - items:
> > > > > + - const: 0
> > > > > + - enum: [5000000, 10000000, 20000000, 40000000]
> > > > > + - items:
> > > > > + - const: -5000000
> > > > > + - const: 5000000
> > > > > + - items:
> > > > > + - const: -10000000
> > > > > + - const: 10000000
> > > > > + - items:
> > > > > + - const: -15000000
> > > > > + - const: 15000000
> > > > > + - items:
> > > > > + - const: -20000000
> > > > > + - const: 20000000
> > > > > +
> > > > > + required:
> > > > > + - reg
> > > > > +
> > > > > + additionalProperties: false
> > > > > +
> > > > > +required:
> > > > > + - compatible
> > > > > + - reg
> > > > > + - vdd-supply
> > > > > + - avdd-supply
> > > > > + - hvdd-supply
> > > > > +
> > > > > +dependencies:
> > > > > + spi-cpha: [ spi-cpol ]
> > > > > + spi-cpol: [ spi-cpha ]
> > > > > +
> > > > > +allOf:
> > > > > + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > > > +
> > > > > +unevaluatedProperties: false
> > > > > +
> > > > > +examples:
> > > > > + - |
> > > > > + #include <dt-bindings/gpio/gpio.h>
> > > > > +
> > > > > + spi {
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > +
> > > > > + dac@0 {
> > > > > + compatible = "adi,ad5529r-16";
> > > > > + reg = <0>;
> > > > > + spi-max-frequency = <25000000>;
> > > > > +
> > > > > + vdd-supply = <&vdd_regulator>;
> > > > > + avdd-supply = <&avdd_regulator>;
> > > > > + hvdd-supply = <&hvdd_regulator>;
> > > > > + hvss-supply = <&hvss_regulator>;
> > > > > +
> > > > > + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > +
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > +
> > > > > + channel@0 {
> > > > > + reg = <0>;
> > > > > + adi,output-range-microvolt = <0 5000000>;
> > > > > + };
> > > > > +
> > > > > + channel@1 {
> > > > > + reg = <1>;
> > > > > + adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > + };
> > > > > +
> > > > > + channel@2 {
> > > > > + reg = <2>;
> > > > > + adi,output-range-microvolt = <0 40000000>;
> > > > > + };
> > > > > + };
> > > > > + };
> > > > ...
> > > >
> > > > spi {
> > > > #address-cells = <1>;
> > > > #size-cells = <0>;
> > > >
> > > > multi-dac@0 {
> > > > compatible = "adi,ad5529r-16";
> > > > reg = <0>;
> > > > spi-max-frequency = <25000000>;
> > > >
> > > > #address-cells = <1>;
> > > > #size-cells = <0>;
> > > >
> > > > dac@0 {
> > > > reg = <0>;
> > > > vdd-supply = <&vdd_regulator>;
> > > > avdd-supply = <&avdd_regulator>;
> > > > hvdd-supply = <&hvdd_regulator>;
> > > > hvss-supply = <&hvss_regulator>;
> > > >
> > > > reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > >
> > > > #address-cells = <1>;
> > > > #size-cells = <0>;
> > > >
> > > > channel@0 {
> > > > reg = <0>;
> > > > adi,output-range-microvolt = <0 5000000>;
> > > > };
> > > >
> > > > channel@1 {
> > > > reg = <1>;
> > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > };
> > > >
> > > > channel@2 {
> > > > reg = <2>;
> > > > adi,output-range-microvolt = <0 40000000>;
> > > > };
> > > > }
> > > >
> > > > dac@1 {
> > > > reg = <1>;
> > > > vdd-supply = <&vdd_regulator>;
> > > > avdd-supply = <&avdd_regulator>;
> > > > hvdd-supply = <&hvdd_regulator>;
> > > > hvss-supply = <&hvss_regulator>;
> > > >
> > > > reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> > > >
> > > > #address-cells = <1>;
> > > > #size-cells = <0>;
> > > >
> > > > channel@0 {
> > > > reg = <0>;
> > > > adi,output-range-microvolt = <0 5000000>;
> > > > };
> > > >
> > > > channel@1 {
> > > > reg = <1>;
> > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > };
> > > > }
> > > > };
> > > > };
> > > >
> > > > then you might need something like:
> > > >
> > > > patternProperties:
> > > > "^dac@[0-3]$":
> > > >
> > > > and put most of the things under this node pattern.
> > > >
> > > > So the main driver that you're putting together might need to handle up to four instances.
> > > > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> > > >
> > > > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > > > a platform data to be shared with the child nodes, which would be a separate driver.
> > > > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
> > > Hi Rodrigo,
> > >
> > > Thank you for looking at this.
> > >
> > > For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> > > hardware/use case we have only needs one device node and the driver is written around that model as well.
> > > While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> > > that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> > > speculatively without a validating use case.
> > Interesting feature - kind of similar to address control on a typical i2c bus device, or
> > looking at it another way a kind of distributed SPI mux.
> >
> > Challenge of a binding is we need to anticipate the future. So I think we do need something
> > like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> > That would leave the path open to supporting the addressing at a later date.
> > An alternative might be to look at it like a chained device setup. In those we pretend there
> > is just one device with a lot of channels etc. The snag is that here things are more loosely
> > coupled whereas for those devices it tends to be you have to read / write the same register
> > in all devices in the chain as one big SPI message.
> >
> > +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> > - Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
> > value that matches that or they are ignored. Thus a single bus + 1 chip select can
> > be used to talk to 4 devices. Question is what that looks like in device tree + I guess
> > longer term how to support it cleanly in SPI.
I'd swear I have seen this before, from some Microchip devices. Let me
see if I can find what I am thinking of...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-06-19 11:36 ` Conor Dooley
@ 2026-06-19 11:40 ` Conor Dooley
2026-06-19 13:01 ` Nuno Sá
0 siblings, 1 reply; 19+ messages in thread
From: Conor Dooley @ 2026-06-19 11:40 UTC (permalink / raw)
To: Janani Sunil
Cc: Jonathan Cameron, Rodrigo Alencar, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
[-- Attachment #1: Type: text/plain, Size: 10234 bytes --]
On Fri, Jun 19, 2026 at 12:36:55PM +0100, Conor Dooley wrote:
> On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:
> >
> > On 6/14/26 21:44, Jonathan Cameron wrote:
> > > On Tue, 9 Jun 2026 16:47:23 +0200
> > > Janani Sunil <jan.sun97@gmail.com> wrote:
> > >
> > > > On 5/26/26 15:11, Rodrigo Alencar wrote:
> > > > > On 26/05/19 05:42PM, Janani Sunil wrote:
> > > > > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > > > > buffered voltage output digital-to-analog converter (DAC) with an
> > > > > > integrated precision reference.
> > > > > ...
> > > > > Probably others may comment on that, but...
> > > > >
> > > > > This parent node may support device addressing for multi-device support through
> > > > > those ID pins. I suppose that each device may have its own power supplies or
> > > > > other resources like the toggle pins or reset and enable.
> > > > >
> > > > > That way I suppose that an example would look like...
> > > > > > +
> > > > > > +patternProperties:
> > > > > > + "^channel@([0-9]|1[0-5])$":
> > > > > > + type: object
> > > > > > + description: Child nodes for individual channel configuration
> > > > > > +
> > > > > > + properties:
> > > > > > + reg:
> > > > > > + description: Channel number.
> > > > > > + minimum: 0
> > > > > > + maximum: 15
> > > > > > +
> > > > > > + adi,output-range-microvolt:
> > > > > > + description: |
> > > > > > + Output voltage range for this channel as [min, max] in microvolts.
> > > > > > + If not specified, defaults to 0V to 5V range.
> > > > > > + oneOf:
> > > > > > + - items:
> > > > > > + - const: 0
> > > > > > + - enum: [5000000, 10000000, 20000000, 40000000]
> > > > > > + - items:
> > > > > > + - const: -5000000
> > > > > > + - const: 5000000
> > > > > > + - items:
> > > > > > + - const: -10000000
> > > > > > + - const: 10000000
> > > > > > + - items:
> > > > > > + - const: -15000000
> > > > > > + - const: 15000000
> > > > > > + - items:
> > > > > > + - const: -20000000
> > > > > > + - const: 20000000
> > > > > > +
> > > > > > + required:
> > > > > > + - reg
> > > > > > +
> > > > > > + additionalProperties: false
> > > > > > +
> > > > > > +required:
> > > > > > + - compatible
> > > > > > + - reg
> > > > > > + - vdd-supply
> > > > > > + - avdd-supply
> > > > > > + - hvdd-supply
> > > > > > +
> > > > > > +dependencies:
> > > > > > + spi-cpha: [ spi-cpol ]
> > > > > > + spi-cpol: [ spi-cpha ]
> > > > > > +
> > > > > > +allOf:
> > > > > > + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > > > > +
> > > > > > +unevaluatedProperties: false
> > > > > > +
> > > > > > +examples:
> > > > > > + - |
> > > > > > + #include <dt-bindings/gpio/gpio.h>
> > > > > > +
> > > > > > + spi {
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > +
> > > > > > + dac@0 {
> > > > > > + compatible = "adi,ad5529r-16";
> > > > > > + reg = <0>;
> > > > > > + spi-max-frequency = <25000000>;
> > > > > > +
> > > > > > + vdd-supply = <&vdd_regulator>;
> > > > > > + avdd-supply = <&avdd_regulator>;
> > > > > > + hvdd-supply = <&hvdd_regulator>;
> > > > > > + hvss-supply = <&hvss_regulator>;
> > > > > > +
> > > > > > + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > +
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > +
> > > > > > + channel@0 {
> > > > > > + reg = <0>;
> > > > > > + adi,output-range-microvolt = <0 5000000>;
> > > > > > + };
> > > > > > +
> > > > > > + channel@1 {
> > > > > > + reg = <1>;
> > > > > > + adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > + };
> > > > > > +
> > > > > > + channel@2 {
> > > > > > + reg = <2>;
> > > > > > + adi,output-range-microvolt = <0 40000000>;
> > > > > > + };
> > > > > > + };
> > > > > > + };
> > > > > ...
> > > > >
> > > > > spi {
> > > > > #address-cells = <1>;
> > > > > #size-cells = <0>;
> > > > >
> > > > > multi-dac@0 {
> > > > > compatible = "adi,ad5529r-16";
> > > > > reg = <0>;
> > > > > spi-max-frequency = <25000000>;
> > > > >
> > > > > #address-cells = <1>;
> > > > > #size-cells = <0>;
> > > > >
> > > > > dac@0 {
> > > > > reg = <0>;
> > > > > vdd-supply = <&vdd_regulator>;
> > > > > avdd-supply = <&avdd_regulator>;
> > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > hvss-supply = <&hvss_regulator>;
> > > > >
> > > > > reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > >
> > > > > #address-cells = <1>;
> > > > > #size-cells = <0>;
> > > > >
> > > > > channel@0 {
> > > > > reg = <0>;
> > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > };
> > > > >
> > > > > channel@1 {
> > > > > reg = <1>;
> > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > };
> > > > >
> > > > > channel@2 {
> > > > > reg = <2>;
> > > > > adi,output-range-microvolt = <0 40000000>;
> > > > > };
> > > > > }
> > > > >
> > > > > dac@1 {
> > > > > reg = <1>;
> > > > > vdd-supply = <&vdd_regulator>;
> > > > > avdd-supply = <&avdd_regulator>;
> > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > hvss-supply = <&hvss_regulator>;
> > > > >
> > > > > reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> > > > >
> > > > > #address-cells = <1>;
> > > > > #size-cells = <0>;
> > > > >
> > > > > channel@0 {
> > > > > reg = <0>;
> > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > };
> > > > >
> > > > > channel@1 {
> > > > > reg = <1>;
> > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > };
> > > > > }
> > > > > };
> > > > > };
> > > > >
> > > > > then you might need something like:
> > > > >
> > > > > patternProperties:
> > > > > "^dac@[0-3]$":
> > > > >
> > > > > and put most of the things under this node pattern.
> > > > >
> > > > > So the main driver that you're putting together might need to handle up to four instances.
> > > > > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> > > > >
> > > > > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > > > > a platform data to be shared with the child nodes, which would be a separate driver.
> > > > > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
> > > > Hi Rodrigo,
> > > >
> > > > Thank you for looking at this.
> > > >
> > > > For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> > > > hardware/use case we have only needs one device node and the driver is written around that model as well.
> > > > While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> > > > that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> > > > speculatively without a validating use case.
> > > Interesting feature - kind of similar to address control on a typical i2c bus device, or
> > > looking at it another way a kind of distributed SPI mux.
> > >
> > > Challenge of a binding is we need to anticipate the future. So I think we do need something
> > > like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> > > That would leave the path open to supporting the addressing at a later date.
> > > An alternative might be to look at it like a chained device setup. In those we pretend there
> > > is just one device with a lot of channels etc. The snag is that here things are more loosely
> > > coupled whereas for those devices it tends to be you have to read / write the same register
> > > in all devices in the chain as one big SPI message.
> > >
> > > +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> > > - Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
> > > value that matches that or they are ignored. Thus a single bus + 1 chip select can
> > > be used to talk to 4 devices. Question is what that looks like in device tree + I guess
> > > longer term how to support it cleanly in SPI.
>
> I'd swear I have seen this before, from some Microchip devices. Let me
> see if I can find what I am thinking of...
microchip,mcp3911 and microchip,mcp3564 both seem to do this with
slightly different properties.
microchip,device-addr:
description: Device address when multiple MCP3911 chips are present on the same SPI bus.
$ref: /schemas/types.yaml#/definitions/uint32
enum: [0, 1, 2, 3]
default: 0
and
microchip,hw-device-address:
$ref: /schemas/types.yaml#/definitions/uint32
minimum: 0
maximum: 3
description:
The address is set on a per-device basis by fuses in the factory,
configured on request. If not requested, the fuses are set for 0x1.
The device address is part of the device markings to avoid
potential confusion. This address is coded on two bits, so four possible
addresses are available when multiple devices are present on the same
SPI bus with only one Chip Select line for all devices.
Each device communication starts by a CS falling edge, followed by the
clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
which is first one on the wire).
This sounds exactly like the sort of feature that you're dealing with
here?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-06-19 11:40 ` Conor Dooley
@ 2026-06-19 13:01 ` Nuno Sá
2026-06-19 14:12 ` Conor Dooley
0 siblings, 1 reply; 19+ messages in thread
From: Nuno Sá @ 2026-06-19 13:01 UTC (permalink / raw)
To: Conor Dooley
Cc: Janani Sunil, Jonathan Cameron, Rodrigo Alencar, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
On Fri, Jun 19, 2026 at 12:40:54PM +0100, Conor Dooley wrote:
> On Fri, Jun 19, 2026 at 12:36:55PM +0100, Conor Dooley wrote:
> > On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:
> > >
> > > On 6/14/26 21:44, Jonathan Cameron wrote:
> > > > On Tue, 9 Jun 2026 16:47:23 +0200
> > > > Janani Sunil <jan.sun97@gmail.com> wrote:
> > > >
> > > > > On 5/26/26 15:11, Rodrigo Alencar wrote:
> > > > > > On 26/05/19 05:42PM, Janani Sunil wrote:
> > > > > > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > > > > > buffered voltage output digital-to-analog converter (DAC) with an
> > > > > > > integrated precision reference.
> > > > > > ...
> > > > > > Probably others may comment on that, but...
> > > > > >
> > > > > > This parent node may support device addressing for multi-device support through
> > > > > > those ID pins. I suppose that each device may have its own power supplies or
> > > > > > other resources like the toggle pins or reset and enable.
> > > > > >
> > > > > > That way I suppose that an example would look like...
> > > > > > > +
> > > > > > > +patternProperties:
> > > > > > > + "^channel@([0-9]|1[0-5])$":
> > > > > > > + type: object
> > > > > > > + description: Child nodes for individual channel configuration
> > > > > > > +
> > > > > > > + properties:
> > > > > > > + reg:
> > > > > > > + description: Channel number.
> > > > > > > + minimum: 0
> > > > > > > + maximum: 15
> > > > > > > +
> > > > > > > + adi,output-range-microvolt:
> > > > > > > + description: |
> > > > > > > + Output voltage range for this channel as [min, max] in microvolts.
> > > > > > > + If not specified, defaults to 0V to 5V range.
> > > > > > > + oneOf:
> > > > > > > + - items:
> > > > > > > + - const: 0
> > > > > > > + - enum: [5000000, 10000000, 20000000, 40000000]
> > > > > > > + - items:
> > > > > > > + - const: -5000000
> > > > > > > + - const: 5000000
> > > > > > > + - items:
> > > > > > > + - const: -10000000
> > > > > > > + - const: 10000000
> > > > > > > + - items:
> > > > > > > + - const: -15000000
> > > > > > > + - const: 15000000
> > > > > > > + - items:
> > > > > > > + - const: -20000000
> > > > > > > + - const: 20000000
> > > > > > > +
> > > > > > > + required:
> > > > > > > + - reg
> > > > > > > +
> > > > > > > + additionalProperties: false
> > > > > > > +
> > > > > > > +required:
> > > > > > > + - compatible
> > > > > > > + - reg
> > > > > > > + - vdd-supply
> > > > > > > + - avdd-supply
> > > > > > > + - hvdd-supply
> > > > > > > +
> > > > > > > +dependencies:
> > > > > > > + spi-cpha: [ spi-cpol ]
> > > > > > > + spi-cpol: [ spi-cpha ]
> > > > > > > +
> > > > > > > +allOf:
> > > > > > > + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > > > > > +
> > > > > > > +unevaluatedProperties: false
> > > > > > > +
> > > > > > > +examples:
> > > > > > > + - |
> > > > > > > + #include <dt-bindings/gpio/gpio.h>
> > > > > > > +
> > > > > > > + spi {
> > > > > > > + #address-cells = <1>;
> > > > > > > + #size-cells = <0>;
> > > > > > > +
> > > > > > > + dac@0 {
> > > > > > > + compatible = "adi,ad5529r-16";
> > > > > > > + reg = <0>;
> > > > > > > + spi-max-frequency = <25000000>;
> > > > > > > +
> > > > > > > + vdd-supply = <&vdd_regulator>;
> > > > > > > + avdd-supply = <&avdd_regulator>;
> > > > > > > + hvdd-supply = <&hvdd_regulator>;
> > > > > > > + hvss-supply = <&hvss_regulator>;
> > > > > > > +
> > > > > > > + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > +
> > > > > > > + #address-cells = <1>;
> > > > > > > + #size-cells = <0>;
> > > > > > > +
> > > > > > > + channel@0 {
> > > > > > > + reg = <0>;
> > > > > > > + adi,output-range-microvolt = <0 5000000>;
> > > > > > > + };
> > > > > > > +
> > > > > > > + channel@1 {
> > > > > > > + reg = <1>;
> > > > > > > + adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > + };
> > > > > > > +
> > > > > > > + channel@2 {
> > > > > > > + reg = <2>;
> > > > > > > + adi,output-range-microvolt = <0 40000000>;
> > > > > > > + };
> > > > > > > + };
> > > > > > > + };
> > > > > > ...
> > > > > >
> > > > > > spi {
> > > > > > #address-cells = <1>;
> > > > > > #size-cells = <0>;
> > > > > >
> > > > > > multi-dac@0 {
> > > > > > compatible = "adi,ad5529r-16";
> > > > > > reg = <0>;
> > > > > > spi-max-frequency = <25000000>;
> > > > > >
> > > > > > #address-cells = <1>;
> > > > > > #size-cells = <0>;
> > > > > >
> > > > > > dac@0 {
> > > > > > reg = <0>;
> > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > hvss-supply = <&hvss_regulator>;
> > > > > >
> > > > > > reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > >
> > > > > > #address-cells = <1>;
> > > > > > #size-cells = <0>;
> > > > > >
> > > > > > channel@0 {
> > > > > > reg = <0>;
> > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > };
> > > > > >
> > > > > > channel@1 {
> > > > > > reg = <1>;
> > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > };
> > > > > >
> > > > > > channel@2 {
> > > > > > reg = <2>;
> > > > > > adi,output-range-microvolt = <0 40000000>;
> > > > > > };
> > > > > > }
> > > > > >
> > > > > > dac@1 {
> > > > > > reg = <1>;
> > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > hvss-supply = <&hvss_regulator>;
> > > > > >
> > > > > > reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> > > > > >
> > > > > > #address-cells = <1>;
> > > > > > #size-cells = <0>;
> > > > > >
> > > > > > channel@0 {
> > > > > > reg = <0>;
> > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > };
> > > > > >
> > > > > > channel@1 {
> > > > > > reg = <1>;
> > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > };
> > > > > > }
> > > > > > };
> > > > > > };
> > > > > >
> > > > > > then you might need something like:
> > > > > >
> > > > > > patternProperties:
> > > > > > "^dac@[0-3]$":
> > > > > >
> > > > > > and put most of the things under this node pattern.
> > > > > >
> > > > > > So the main driver that you're putting together might need to handle up to four instances.
> > > > > > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> > > > > >
> > > > > > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > > > > > a platform data to be shared with the child nodes, which would be a separate driver.
> > > > > > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
> > > > > Hi Rodrigo,
> > > > >
> > > > > Thank you for looking at this.
> > > > >
> > > > > For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> > > > > hardware/use case we have only needs one device node and the driver is written around that model as well.
> > > > > While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> > > > > that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> > > > > speculatively without a validating use case.
> > > > Interesting feature - kind of similar to address control on a typical i2c bus device, or
> > > > looking at it another way a kind of distributed SPI mux.
> > > >
> > > > Challenge of a binding is we need to anticipate the future. So I think we do need something
> > > > like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> > > > That would leave the path open to supporting the addressing at a later date.
> > > > An alternative might be to look at it like a chained device setup. In those we pretend there
> > > > is just one device with a lot of channels etc. The snag is that here things are more loosely
> > > > coupled whereas for those devices it tends to be you have to read / write the same register
> > > > in all devices in the chain as one big SPI message.
> > > >
> > > > +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> > > > - Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
> > > > value that matches that or they are ignored. Thus a single bus + 1 chip select can
> > > > be used to talk to 4 devices. Question is what that looks like in device tree + I guess
> > > > longer term how to support it cleanly in SPI.
> >
> > I'd swear I have seen this before, from some Microchip devices. Let me
> > see if I can find what I am thinking of...
>
>
> microchip,mcp3911 and microchip,mcp3564 both seem to do this with
> slightly different properties.
>
> microchip,device-addr:
> description: Device address when multiple MCP3911 chips are present on the same SPI bus.
> $ref: /schemas/types.yaml#/definitions/uint32
> enum: [0, 1, 2, 3]
> default: 0
>
> and
>
>
> microchip,hw-device-address:
> $ref: /schemas/types.yaml#/definitions/uint32
> minimum: 0
> maximum: 3
> description:
> The address is set on a per-device basis by fuses in the factory,
> configured on request. If not requested, the fuses are set for 0x1.
> The device address is part of the device markings to avoid
> potential confusion. This address is coded on two bits, so four possible
> addresses are available when multiple devices are present on the same
> SPI bus with only one Chip Select line for all devices.
> Each device communication starts by a CS falling edge, followed by the
> clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
> which is first one on the wire).
>
> This sounds exactly like the sort of feature that you're dealing with
> here?
>
The core idea yes but for this chip, things are a bit more annoying (but
Janani can correct me if I'm wrong). Here, each device can, in theory,
have it's own supplies, pins and at the very least, channels with maybe
different scales. That is why Janani is proposing dac nodes. Given I
honestly don't like much of that "adi,ad5529r-bus" compatible I wondered
about solving this at the spi level.
Ah and to make it more annoying, we can also mix 12 and 16 bits variants
together in the same bus.
- Nuno Sá
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-06-19 13:01 ` Nuno Sá
@ 2026-06-19 14:12 ` Conor Dooley
2026-06-19 15:54 ` Nuno Sá
0 siblings, 1 reply; 19+ messages in thread
From: Conor Dooley @ 2026-06-19 14:12 UTC (permalink / raw)
To: Nuno Sá
Cc: Janani Sunil, Jonathan Cameron, Rodrigo Alencar, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
[-- Attachment #1: Type: text/plain, Size: 12325 bytes --]
On Fri, Jun 19, 2026 at 02:01:08PM +0100, Nuno Sá wrote:
> On Fri, Jun 19, 2026 at 12:40:54PM +0100, Conor Dooley wrote:
> > On Fri, Jun 19, 2026 at 12:36:55PM +0100, Conor Dooley wrote:
> > > On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:
> > > >
> > > > On 6/14/26 21:44, Jonathan Cameron wrote:
> > > > > On Tue, 9 Jun 2026 16:47:23 +0200
> > > > > Janani Sunil <jan.sun97@gmail.com> wrote:
> > > > >
> > > > > > On 5/26/26 15:11, Rodrigo Alencar wrote:
> > > > > > > On 26/05/19 05:42PM, Janani Sunil wrote:
> > > > > > > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > > > > > > buffered voltage output digital-to-analog converter (DAC) with an
> > > > > > > > integrated precision reference.
> > > > > > > ...
> > > > > > > Probably others may comment on that, but...
> > > > > > >
> > > > > > > This parent node may support device addressing for multi-device support through
> > > > > > > those ID pins. I suppose that each device may have its own power supplies or
> > > > > > > other resources like the toggle pins or reset and enable.
> > > > > > >
> > > > > > > That way I suppose that an example would look like...
> > > > > > > > +
> > > > > > > > +patternProperties:
> > > > > > > > + "^channel@([0-9]|1[0-5])$":
> > > > > > > > + type: object
> > > > > > > > + description: Child nodes for individual channel configuration
> > > > > > > > +
> > > > > > > > + properties:
> > > > > > > > + reg:
> > > > > > > > + description: Channel number.
> > > > > > > > + minimum: 0
> > > > > > > > + maximum: 15
> > > > > > > > +
> > > > > > > > + adi,output-range-microvolt:
> > > > > > > > + description: |
> > > > > > > > + Output voltage range for this channel as [min, max] in microvolts.
> > > > > > > > + If not specified, defaults to 0V to 5V range.
> > > > > > > > + oneOf:
> > > > > > > > + - items:
> > > > > > > > + - const: 0
> > > > > > > > + - enum: [5000000, 10000000, 20000000, 40000000]
> > > > > > > > + - items:
> > > > > > > > + - const: -5000000
> > > > > > > > + - const: 5000000
> > > > > > > > + - items:
> > > > > > > > + - const: -10000000
> > > > > > > > + - const: 10000000
> > > > > > > > + - items:
> > > > > > > > + - const: -15000000
> > > > > > > > + - const: 15000000
> > > > > > > > + - items:
> > > > > > > > + - const: -20000000
> > > > > > > > + - const: 20000000
> > > > > > > > +
> > > > > > > > + required:
> > > > > > > > + - reg
> > > > > > > > +
> > > > > > > > + additionalProperties: false
> > > > > > > > +
> > > > > > > > +required:
> > > > > > > > + - compatible
> > > > > > > > + - reg
> > > > > > > > + - vdd-supply
> > > > > > > > + - avdd-supply
> > > > > > > > + - hvdd-supply
> > > > > > > > +
> > > > > > > > +dependencies:
> > > > > > > > + spi-cpha: [ spi-cpol ]
> > > > > > > > + spi-cpol: [ spi-cpha ]
> > > > > > > > +
> > > > > > > > +allOf:
> > > > > > > > + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > > > > > > +
> > > > > > > > +unevaluatedProperties: false
> > > > > > > > +
> > > > > > > > +examples:
> > > > > > > > + - |
> > > > > > > > + #include <dt-bindings/gpio/gpio.h>
> > > > > > > > +
> > > > > > > > + spi {
> > > > > > > > + #address-cells = <1>;
> > > > > > > > + #size-cells = <0>;
> > > > > > > > +
> > > > > > > > + dac@0 {
> > > > > > > > + compatible = "adi,ad5529r-16";
> > > > > > > > + reg = <0>;
> > > > > > > > + spi-max-frequency = <25000000>;
> > > > > > > > +
> > > > > > > > + vdd-supply = <&vdd_regulator>;
> > > > > > > > + avdd-supply = <&avdd_regulator>;
> > > > > > > > + hvdd-supply = <&hvdd_regulator>;
> > > > > > > > + hvss-supply = <&hvss_regulator>;
> > > > > > > > +
> > > > > > > > + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > +
> > > > > > > > + #address-cells = <1>;
> > > > > > > > + #size-cells = <0>;
> > > > > > > > +
> > > > > > > > + channel@0 {
> > > > > > > > + reg = <0>;
> > > > > > > > + adi,output-range-microvolt = <0 5000000>;
> > > > > > > > + };
> > > > > > > > +
> > > > > > > > + channel@1 {
> > > > > > > > + reg = <1>;
> > > > > > > > + adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > + };
> > > > > > > > +
> > > > > > > > + channel@2 {
> > > > > > > > + reg = <2>;
> > > > > > > > + adi,output-range-microvolt = <0 40000000>;
> > > > > > > > + };
> > > > > > > > + };
> > > > > > > > + };
> > > > > > > ...
> > > > > > >
> > > > > > > spi {
> > > > > > > #address-cells = <1>;
> > > > > > > #size-cells = <0>;
> > > > > > >
> > > > > > > multi-dac@0 {
> > > > > > > compatible = "adi,ad5529r-16";
> > > > > > > reg = <0>;
> > > > > > > spi-max-frequency = <25000000>;
> > > > > > >
> > > > > > > #address-cells = <1>;
> > > > > > > #size-cells = <0>;
> > > > > > >
> > > > > > > dac@0 {
> > > > > > > reg = <0>;
> > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > >
> > > > > > > reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > >
> > > > > > > #address-cells = <1>;
> > > > > > > #size-cells = <0>;
> > > > > > >
> > > > > > > channel@0 {
> > > > > > > reg = <0>;
> > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > };
> > > > > > >
> > > > > > > channel@1 {
> > > > > > > reg = <1>;
> > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > };
> > > > > > >
> > > > > > > channel@2 {
> > > > > > > reg = <2>;
> > > > > > > adi,output-range-microvolt = <0 40000000>;
> > > > > > > };
> > > > > > > }
> > > > > > >
> > > > > > > dac@1 {
> > > > > > > reg = <1>;
> > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > >
> > > > > > > reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> > > > > > >
> > > > > > > #address-cells = <1>;
> > > > > > > #size-cells = <0>;
> > > > > > >
> > > > > > > channel@0 {
> > > > > > > reg = <0>;
> > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > };
> > > > > > >
> > > > > > > channel@1 {
> > > > > > > reg = <1>;
> > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > };
> > > > > > > }
> > > > > > > };
> > > > > > > };
> > > > > > >
> > > > > > > then you might need something like:
> > > > > > >
> > > > > > > patternProperties:
> > > > > > > "^dac@[0-3]$":
> > > > > > >
> > > > > > > and put most of the things under this node pattern.
> > > > > > >
> > > > > > > So the main driver that you're putting together might need to handle up to four instances.
> > > > > > > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> > > > > > >
> > > > > > > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > > > > > > a platform data to be shared with the child nodes, which would be a separate driver.
> > > > > > > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
> > > > > > Hi Rodrigo,
> > > > > >
> > > > > > Thank you for looking at this.
> > > > > >
> > > > > > For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> > > > > > hardware/use case we have only needs one device node and the driver is written around that model as well.
> > > > > > While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> > > > > > that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> > > > > > speculatively without a validating use case.
> > > > > Interesting feature - kind of similar to address control on a typical i2c bus device, or
> > > > > looking at it another way a kind of distributed SPI mux.
> > > > >
> > > > > Challenge of a binding is we need to anticipate the future. So I think we do need something
> > > > > like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> > > > > That would leave the path open to supporting the addressing at a later date.
> > > > > An alternative might be to look at it like a chained device setup. In those we pretend there
> > > > > is just one device with a lot of channels etc. The snag is that here things are more loosely
> > > > > coupled whereas for those devices it tends to be you have to read / write the same register
> > > > > in all devices in the chain as one big SPI message.
> > > > >
> > > > > +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> > > > > - Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
> > > > > value that matches that or they are ignored. Thus a single bus + 1 chip select can
> > > > > be used to talk to 4 devices. Question is what that looks like in device tree + I guess
> > > > > longer term how to support it cleanly in SPI.
> > >
> > > I'd swear I have seen this before, from some Microchip devices. Let me
> > > see if I can find what I am thinking of...
> >
> >
> > microchip,mcp3911 and microchip,mcp3564 both seem to do this with
> > slightly different properties.
> >
> > microchip,device-addr:
> > description: Device address when multiple MCP3911 chips are present on the same SPI bus.
> > $ref: /schemas/types.yaml#/definitions/uint32
> > enum: [0, 1, 2, 3]
> > default: 0
> >
> > and
> >
> >
> > microchip,hw-device-address:
> > $ref: /schemas/types.yaml#/definitions/uint32
> > minimum: 0
> > maximum: 3
> > description:
> > The address is set on a per-device basis by fuses in the factory,
> > configured on request. If not requested, the fuses are set for 0x1.
> > The device address is part of the device markings to avoid
> > potential confusion. This address is coded on two bits, so four possible
> > addresses are available when multiple devices are present on the same
> > SPI bus with only one Chip Select line for all devices.
> > Each device communication starts by a CS falling edge, followed by the
> > clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
> > which is first one on the wire).
> >
> > This sounds exactly like the sort of feature that you're dealing with
> > here?
> >
>
> The core idea yes but for this chip, things are a bit more annoying (but
> Janani can correct me if I'm wrong). Here, each device can, in theory,
> have it's own supplies, pins and at the very least, channels with maybe
> different scales. That is why Janani is proposing dac nodes. Given I
> honestly don't like much of that "adi,ad5529r-bus" compatible I wondered
> about solving this at the spi level.
>
> Ah and to make it more annoying, we can also mix 12 and 16 bits variants
> together in the same bus.
I'm definitely missing something, because that property for the
microchip devices is not impacted what else is on the bus. AFAICT, you
could have an mcp3911 and an mcp3564 on the same bus even though both
are completely different devices with different drivers. They have
individual device nodes and their own supplies etc etc. These aren't
per-channel properties on an adc or dac, they're per child device on a
spi bus.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
2026-06-19 14:12 ` Conor Dooley
@ 2026-06-19 15:54 ` Nuno Sá
0 siblings, 0 replies; 19+ messages in thread
From: Nuno Sá @ 2026-06-19 15:54 UTC (permalink / raw)
To: Conor Dooley
Cc: Janani Sunil, Jonathan Cameron, Rodrigo Alencar, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
On Fri, Jun 19, 2026 at 03:12:07PM +0100, Conor Dooley wrote:
> On Fri, Jun 19, 2026 at 02:01:08PM +0100, Nuno Sá wrote:
> > On Fri, Jun 19, 2026 at 12:40:54PM +0100, Conor Dooley wrote:
> > > On Fri, Jun 19, 2026 at 12:36:55PM +0100, Conor Dooley wrote:
> > > > On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:
> > > > >
> > > > > On 6/14/26 21:44, Jonathan Cameron wrote:
> > > > > > On Tue, 9 Jun 2026 16:47:23 +0200
> > > > > > Janani Sunil <jan.sun97@gmail.com> wrote:
> > > > > >
> > > > > > > On 5/26/26 15:11, Rodrigo Alencar wrote:
> > > > > > > > On 26/05/19 05:42PM, Janani Sunil wrote:
> > > > > > > > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > > > > > > > buffered voltage output digital-to-analog converter (DAC) with an
> > > > > > > > > integrated precision reference.
> > > > > > > > ...
> > > > > > > > Probably others may comment on that, but...
> > > > > > > >
> > > > > > > > This parent node may support device addressing for multi-device support through
> > > > > > > > those ID pins. I suppose that each device may have its own power supplies or
> > > > > > > > other resources like the toggle pins or reset and enable.
> > > > > > > >
> > > > > > > > That way I suppose that an example would look like...
> > > > > > > > > +
> > > > > > > > > +patternProperties:
> > > > > > > > > + "^channel@([0-9]|1[0-5])$":
> > > > > > > > > + type: object
> > > > > > > > > + description: Child nodes for individual channel configuration
> > > > > > > > > +
> > > > > > > > > + properties:
> > > > > > > > > + reg:
> > > > > > > > > + description: Channel number.
> > > > > > > > > + minimum: 0
> > > > > > > > > + maximum: 15
> > > > > > > > > +
> > > > > > > > > + adi,output-range-microvolt:
> > > > > > > > > + description: |
> > > > > > > > > + Output voltage range for this channel as [min, max] in microvolts.
> > > > > > > > > + If not specified, defaults to 0V to 5V range.
> > > > > > > > > + oneOf:
> > > > > > > > > + - items:
> > > > > > > > > + - const: 0
> > > > > > > > > + - enum: [5000000, 10000000, 20000000, 40000000]
> > > > > > > > > + - items:
> > > > > > > > > + - const: -5000000
> > > > > > > > > + - const: 5000000
> > > > > > > > > + - items:
> > > > > > > > > + - const: -10000000
> > > > > > > > > + - const: 10000000
> > > > > > > > > + - items:
> > > > > > > > > + - const: -15000000
> > > > > > > > > + - const: 15000000
> > > > > > > > > + - items:
> > > > > > > > > + - const: -20000000
> > > > > > > > > + - const: 20000000
> > > > > > > > > +
> > > > > > > > > + required:
> > > > > > > > > + - reg
> > > > > > > > > +
> > > > > > > > > + additionalProperties: false
> > > > > > > > > +
> > > > > > > > > +required:
> > > > > > > > > + - compatible
> > > > > > > > > + - reg
> > > > > > > > > + - vdd-supply
> > > > > > > > > + - avdd-supply
> > > > > > > > > + - hvdd-supply
> > > > > > > > > +
> > > > > > > > > +dependencies:
> > > > > > > > > + spi-cpha: [ spi-cpol ]
> > > > > > > > > + spi-cpol: [ spi-cpha ]
> > > > > > > > > +
> > > > > > > > > +allOf:
> > > > > > > > > + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > > > > > > > +
> > > > > > > > > +unevaluatedProperties: false
> > > > > > > > > +
> > > > > > > > > +examples:
> > > > > > > > > + - |
> > > > > > > > > + #include <dt-bindings/gpio/gpio.h>
> > > > > > > > > +
> > > > > > > > > + spi {
> > > > > > > > > + #address-cells = <1>;
> > > > > > > > > + #size-cells = <0>;
> > > > > > > > > +
> > > > > > > > > + dac@0 {
> > > > > > > > > + compatible = "adi,ad5529r-16";
> > > > > > > > > + reg = <0>;
> > > > > > > > > + spi-max-frequency = <25000000>;
> > > > > > > > > +
> > > > > > > > > + vdd-supply = <&vdd_regulator>;
> > > > > > > > > + avdd-supply = <&avdd_regulator>;
> > > > > > > > > + hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > + hvss-supply = <&hvss_regulator>;
> > > > > > > > > +
> > > > > > > > > + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > +
> > > > > > > > > + #address-cells = <1>;
> > > > > > > > > + #size-cells = <0>;
> > > > > > > > > +
> > > > > > > > > + channel@0 {
> > > > > > > > > + reg = <0>;
> > > > > > > > > + adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > + };
> > > > > > > > > +
> > > > > > > > > + channel@1 {
> > > > > > > > > + reg = <1>;
> > > > > > > > > + adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > + };
> > > > > > > > > +
> > > > > > > > > + channel@2 {
> > > > > > > > > + reg = <2>;
> > > > > > > > > + adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > + };
> > > > > > > > > + };
> > > > > > > > > + };
> > > > > > > > ...
> > > > > > > >
> > > > > > > > spi {
> > > > > > > > #address-cells = <1>;
> > > > > > > > #size-cells = <0>;
> > > > > > > >
> > > > > > > > multi-dac@0 {
> > > > > > > > compatible = "adi,ad5529r-16";
> > > > > > > > reg = <0>;
> > > > > > > > spi-max-frequency = <25000000>;
> > > > > > > >
> > > > > > > > #address-cells = <1>;
> > > > > > > > #size-cells = <0>;
> > > > > > > >
> > > > > > > > dac@0 {
> > > > > > > > reg = <0>;
> > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > >
> > > > > > > > reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > >
> > > > > > > > #address-cells = <1>;
> > > > > > > > #size-cells = <0>;
> > > > > > > >
> > > > > > > > channel@0 {
> > > > > > > > reg = <0>;
> > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > };
> > > > > > > >
> > > > > > > > channel@1 {
> > > > > > > > reg = <1>;
> > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > };
> > > > > > > >
> > > > > > > > channel@2 {
> > > > > > > > reg = <2>;
> > > > > > > > adi,output-range-microvolt = <0 40000000>;
> > > > > > > > };
> > > > > > > > }
> > > > > > > >
> > > > > > > > dac@1 {
> > > > > > > > reg = <1>;
> > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > >
> > > > > > > > reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> > > > > > > >
> > > > > > > > #address-cells = <1>;
> > > > > > > > #size-cells = <0>;
> > > > > > > >
> > > > > > > > channel@0 {
> > > > > > > > reg = <0>;
> > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > };
> > > > > > > >
> > > > > > > > channel@1 {
> > > > > > > > reg = <1>;
> > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > };
> > > > > > > > }
> > > > > > > > };
> > > > > > > > };
> > > > > > > >
> > > > > > > > then you might need something like:
> > > > > > > >
> > > > > > > > patternProperties:
> > > > > > > > "^dac@[0-3]$":
> > > > > > > >
> > > > > > > > and put most of the things under this node pattern.
> > > > > > > >
> > > > > > > > So the main driver that you're putting together might need to handle up to four instances.
> > > > > > > > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> > > > > > > >
> > > > > > > > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > > > > > > > a platform data to be shared with the child nodes, which would be a separate driver.
> > > > > > > > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
> > > > > > > Hi Rodrigo,
> > > > > > >
> > > > > > > Thank you for looking at this.
> > > > > > >
> > > > > > > For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> > > > > > > hardware/use case we have only needs one device node and the driver is written around that model as well.
> > > > > > > While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> > > > > > > that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> > > > > > > speculatively without a validating use case.
> > > > > > Interesting feature - kind of similar to address control on a typical i2c bus device, or
> > > > > > looking at it another way a kind of distributed SPI mux.
> > > > > >
> > > > > > Challenge of a binding is we need to anticipate the future. So I think we do need something
> > > > > > like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> > > > > > That would leave the path open to supporting the addressing at a later date.
> > > > > > An alternative might be to look at it like a chained device setup. In those we pretend there
> > > > > > is just one device with a lot of channels etc. The snag is that here things are more loosely
> > > > > > coupled whereas for those devices it tends to be you have to read / write the same register
> > > > > > in all devices in the chain as one big SPI message.
> > > > > >
> > > > > > +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> > > > > > - Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
> > > > > > value that matches that or they are ignored. Thus a single bus + 1 chip select can
> > > > > > be used to talk to 4 devices. Question is what that looks like in device tree + I guess
> > > > > > longer term how to support it cleanly in SPI.
> > > >
> > > > I'd swear I have seen this before, from some Microchip devices. Let me
> > > > see if I can find what I am thinking of...
> > >
> > >
> > > microchip,mcp3911 and microchip,mcp3564 both seem to do this with
> > > slightly different properties.
> > >
> > > microchip,device-addr:
> > > description: Device address when multiple MCP3911 chips are present on the same SPI bus.
> > > $ref: /schemas/types.yaml#/definitions/uint32
> > > enum: [0, 1, 2, 3]
> > > default: 0
> > >
> > > and
> > >
> > >
> > > microchip,hw-device-address:
> > > $ref: /schemas/types.yaml#/definitions/uint32
> > > minimum: 0
> > > maximum: 3
> > > description:
> > > The address is set on a per-device basis by fuses in the factory,
> > > configured on request. If not requested, the fuses are set for 0x1.
> > > The device address is part of the device markings to avoid
> > > potential confusion. This address is coded on two bits, so four possible
> > > addresses are available when multiple devices are present on the same
> > > SPI bus with only one Chip Select line for all devices.
> > > Each device communication starts by a CS falling edge, followed by the
> > > clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
> > > which is first one on the wire).
> > >
> > > This sounds exactly like the sort of feature that you're dealing with
> > > here?
> > >
> >
> > The core idea yes but for this chip, things are a bit more annoying (but
> > Janani can correct me if I'm wrong). Here, each device can, in theory,
> > have it's own supplies, pins and at the very least, channels with maybe
> > different scales. That is why Janani is proposing dac nodes. Given I
> > honestly don't like much of that "adi,ad5529r-bus" compatible I wondered
> > about solving this at the spi level.
> >
> > Ah and to make it more annoying, we can also mix 12 and 16 bits variants
> > together in the same bus.
>
> I'm definitely missing something, because that property for the
> microchip devices is not impacted what else is on the bus. AFAICT, you
> could have an mcp3911 and an mcp3564 on the same bus even though both
> are completely different devices with different drivers. They have
> individual device nodes and their own supplies etc etc. These aren't
> per-channel properties on an adc or dac, they're per child device on a
> spi bus.
Maybe I'm the one missing something :). IIRC, spi would not allow two
devices on the same CS right? Because for this chip we would need
something like:
spi {
dac@0 {
reg = <0>;
adi,pin-id = <0>;
};
dac@1 {
reg = <0>; // which seems already problematic?
adi,pin-id <1>;
};
...
//up to 4
};
- Nuno Sá
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2026-06-19 15:53 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-19 15:42 [PATCH v3 0/2] iio: dac: Add support for AD5529R DAC Janani Sunil
2026-05-19 15:42 ` [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R Janani Sunil
2026-05-19 15:55 ` sashiko-bot
2026-05-22 17:02 ` Jonathan Cameron
2026-05-25 16:30 ` Conor Dooley
2026-05-26 12:35 ` Jonathan Cameron
2026-05-26 13:11 ` Rodrigo Alencar
2026-06-09 14:47 ` Janani Sunil
2026-06-14 19:44 ` Jonathan Cameron
2026-06-19 10:33 ` Janani Sunil
2026-06-19 11:31 ` Nuno Sá
2026-06-19 11:36 ` Conor Dooley
2026-06-19 11:40 ` Conor Dooley
2026-06-19 13:01 ` Nuno Sá
2026-06-19 14:12 ` Conor Dooley
2026-06-19 15:54 ` Nuno Sá
2026-05-19 15:42 ` [PATCH v3 2/2] iio: dac: Add AD5529R DAC driver support Janani Sunil
2026-05-19 16:22 ` sashiko-bot
2026-05-22 17:24 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox