Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R

["Nuno Sá" <noname.nuno@gmail.com>]

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á