iio: dac: add ad5706 (part 2) by actorreno · Pull Request #3327 · analogdevicesinc/linux

actorreno · 2026-05-19T08:48:11Z

PR Description

This is part 2 of series of patches to fully support ad5706r
refer to part 1 here: #3325

Adding device attributes to AD5706R, a quad 16-bit current output digital-to-analog converter with integrated precision reference.
Datasheet of AD5706R.
Features Implemented:
- dev_addr, spi cmd can change to communicate up to 4 devices on same bus
- addr_ascension pertaining to how multibyte registers are accessed
- single_instr to choose whether streaming or single transaction
- sampling_frequency for PWM frequency
- hw_ldac_tg_state for PWM control of low or high
- hw_ldac_tg_pwm to make the ldac pin into a pwm
- multi_dac_input_a content for multi select channel load
- multi_dac_sw_ldac_trigger to trigger the multi channel load
- mux_out_sel for internal monitoring
- hw_shutdown_state controls device enable pin

This PR is related to #3096
The upstream asked this to be split.
Part 1 - basic driver
Part 2 - device attributes (this)
Part 3 - Channel attributes
Part 4 - debugfs attributes

PR Type

Bug fix (a change that fixes an issue)
New feature (a change that adds new functionality)
Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

I have conducted a self-review of my own code changes
I have compiled my changes, including the documentation
I have tested the changes on the relevant hardware
I have updated the documentation outside this repo accordingly
I have provided links for the relevant upstream lore

rodrigo455 · 2026-05-20T10:27:17Z

+	st->sampling_frequency = HZ_PER_MHZ;
+
+	/* Get optional PWM for LDAC triggering */
+	st->ldac_pwm = devm_pwm_get(dev, "ldac");


What is your use case? why ldac needs to be a pwm?

you could use a triggered buffer + hrtimer trigger and ldac as a gpio to update all the channels at once based on the sampling frequency configured in the hrtimer.

So you would leave sysfs attrs to write immediate changes to the DAC channels (LDAC low) and use the character dev to write to all the channels at once (LDAC up then low)

Although it's just called ldac here, the pin actually shares some functions, that is:

ldac

tgp - toggle dac pin

dck - dither clock

Although this is my first time learning about hrtimer, I still think PWM is more apt as it can satisfy being a static high/low state using duty% 100 or 0. 50% for the dither clock.

rodrigo455 · 2026-05-20T10:29:28Z

+		 &ad5706r_multi_dac_sw_ldac_trigger_enum),
+	IIO_ENUM_AVAILABLE("multi_dac_sw_ldac_trigger", IIO_SHARED_BY_ALL,
+			   &ad5706r_multi_dac_sw_ldac_trigger_enum),
+	IIO_ENUM("hw_shutdown_state", IIO_SHARED_BY_ALL, &ad5706r_hw_shutdown_state_enum),


use "powerdown" ABI, or IIO_CHAN_INFO_ENABLE.

this makes sense will add it to POWERDOWN as info_mask_shared_by_all

powerdown would still be in ext_info, check for examples.

after checking, since pin=1 is normal operation, the IIO_CHAN_INFO_ENABLE should be better, the pin is named enable in the dts too

rodrigo455 · 2026-05-20T10:33:18Z

+	IIO_ENUM("hw_ldac_tg_pwm", IIO_SHARED_BY_ALL, &ad5706r_hw_ldac_tg_pwm_enum),
+	IIO_ENUM_AVAILABLE("hw_ldac_tg_pwm", IIO_SHARED_BY_ALL, &ad5706r_hw_ldac_tg_pwm_enum),
+	IIO_ENUM("mux_out_sel", IIO_SHARED_BY_ALL, &ad5706r_mux_out_sel_enum),
+	IIO_ENUM_AVAILABLE("mux_out_sel", IIO_SHARED_BY_ALL, &ad5706r_mux_out_sel_enum),


Muxout is often for diagnotic/debug purposes. It is better create a debug attribute for this one.

the device does use this for hwmon like stuff it seems.
some previous upstream comments was thinking of making this as part of dts
but the nature of monitoring more 20+ outputs didn't look like it would work well

until there's a different opinion on this from others, for now i'll remove this feature
as this PR was ideally for device attributes, will add muxout during part 4 with other debugfs

rodrigo455 · 2026-05-20T10:43:35Z

+	IIO_ENUM("addr_ascension", IIO_SHARED_BY_ALL, &ad5706r_addr_ascension_enum),
+	IIO_ENUM_AVAILABLE("addr_ascension", IIO_SHARED_BY_ALL, &ad5706r_addr_ascension_enum),
+	IIO_ENUM("single_instr", IIO_SHARED_BY_ALL, &ad5706r_single_instr_enum),
+	IIO_ENUM_AVAILABLE("single_instr", IIO_SHARED_BY_ALL, &ad5706r_single_instr_enum),


How are you using this streaming mode? It does not seem like something that the user should control.

streaming mode could be used when using buffers and single instruction when using sysfs attrs

streaming mode here actually is for updating the registers in a continuous sense
we declare the starting address, and we can keep sending data and the register address auto increment

I'm not even sure if that is a good driver practice.

Either way, I understand whether it is streaming or single instr, if the driver needs something then it should just automatically toggle to it. I agree this shouldn't be exposed. Will remove

rodrigo455 · 2026-05-20T10:50:58Z

+	AD5706R_CHAN_EXT_INFO("dev_addr", 0, IIO_SHARED_BY_ALL,
+			      ad5706r_dev_addr_read, ad5706r_dev_addr_write),
+	IIO_ENUM("addr_ascension", IIO_SHARED_BY_ALL, &ad5706r_addr_ascension_enum),
+	IIO_ENUM_AVAILABLE("addr_ascension", IIO_SHARED_BY_ALL, &ad5706r_addr_ascension_enum),


There is no reason to support this at this point. I suppose this driver would need to support multiple instances of the same device first (up to 4 devices, so this could handle up to 16 channels). Also, I don't see a problem on hardcoding this to be in incrementing order.

for addr_ascension I guess there is no upside in even having to choose one or the other.
will have to remove

If dev_addr is for dts then yes, I would have to support the multiple device instance first.

yes, multi-device support will require more stuff on the device-tree, e.g. #address-cells and #size-cells in a parent node (child node of the spi dev), and each child with a separate reg value (0, 1, 2 or 3) and their own power supplies and vref config. I suppose you could register up to 4 different iio devices, but your (spi device) driver would have to handle them all. However, each device might end up sharing resources, (like this pwm) or they might have common configurations, especially for streaming data into a buffer... need to design this carefully...

rodrigo455 · 2026-05-20T10:55:21Z

+
+static struct iio_chan_spec_ext_info ad5706r_ext_info[] = {
+	AD5706R_CHAN_EXT_INFO("dev_addr", 0, IIO_SHARED_BY_ALL,
+			      ad5706r_dev_addr_read, ad5706r_dev_addr_write),


mmm... dev_addr to indicate which device we are targeting.. Not sure if this is good.
I suppose that would need to go to the device-tree, where you specify the instances you have in your system and what are their addresses.

(replied above)

rodrigo455 · 2026-05-20T10:59:15Z

 				     "Failed to init regmap\n");

+	/* Voltage reference - external or internal */
+	st->vref = devm_regulator_get_optional(dev, "vref");


consider the other supplies: iovdd, avdd, pvddx

the others were perpetually on supplies, so I didn't add them.
it seems to be proper to still fetch them, will add them

Adds an attribute entry for device address. This edits the SPI command at bits 13:12 to communicate with multiple ad5706r. Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

This commit adds the following device attributes to ad5706r - dev_addr, spi cmd can change to communicate up to 4 devices on same bus - addr_ascension pertaining to how multibyte registers are accessed - single_instr to choose whether streaming or single transaction - sampling_frequency for PWM frequency - hw_ldac_tg_state for PWM control of low or high - hw_ldac_tg_pwm to make the ldac pin into a pwm - multi_dac_input_a content for multi select channel load - multi_dac_sw_ldac_trigger to trigger the multi channel load - mux_out_sel for internal monitoring - hw_shutdown_state controls device enable pin Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

This commit adds the documentation for the device attributes added on the ad5706r driver Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

actorreno · 2026-05-25T07:06:55Z

v2:

Replaced custom hw_shutdown_state with IIO_CHAN_INFO_ENABLE
Removed runtime-configurable attributes (addr_ascension, single_instr) uses boot defaults
mux_out_sel - removed, for future debugfs patch
Added proper regulator handling for all power supplies (iovdd, avdd, pvddx)
Converted dev_addr to device tree property (adi,device-address)
driver only supports single device
added decrement mode register offseting for multibyte reg
Removed obsolete sysfs documentation

rodrigo455 · 2026-05-25T07:21:26Z

      Optional external 2.5V voltage reference. If not provided, the
      internal 2.5V reference is used.

+  adi,device-address:


Again (#3325 (comment)), we might want comments from others, but device addressing is normally achieved with #address-cells in a parent and reg in child nodes. If you don't plan the feature for now, assume that it is 0 and don't even create this property. I understand that device-tree bindings is also to be treated as an ABI... so once it becomes available to users, the only thing you can do is to extend it. So you might want to get this right from the beginning. How would you get a second device to be supported with property? Isn't it locking the multiple-device feature?

that makes sense...this would lock it out
ok, i'll wait for other comments for the meantime

rodrigo455 · 2026-05-25T07:36:33Z

+	}
+
+	/* Voltage reference - external or internal */
+	st->vref = devm_regulator_get_optional(dev, "vref");


maybe use devm_regulator_get_enable_optional() and you would not need to hold the regulator reference? and let the device management to disable them when necessary. do you need regulator control?
optional should already return NULL if the device was not found... not sure you need to check for -ENODEV

alright this can work, I don't really need to control them after boot up

rodrigo455 · 2026-05-25T07:39:15Z

+	st->pvdd[1].supply = "pvdd1";
+	st->pvdd[2].supply = "pvdd2";
+	st->pvdd[3].supply = "pvdd3";
+	ret = devm_regulator_bulk_get(dev, 4, st->pvdd);


maybe use devm_regulator_bulk_get_enable() and include iovdd and avdd into the bullk.

will combine

rodrigo455 · 2026-05-25T08:34:37Z

+Description:
+		Returns the available PWM states: "disable enable"
+
+What:		/sys/bus/iio/devices/iio:deviceX/multi_dac_input_a


How you are doing multi-DAC selection? I am not sure how this can be supported properly. I understand that Multi INPUT_A treats INPUT_A for all the selected channels as the same register.
Maybe a single multi_en attribute for each channel is enough... and you would treat the RAW attribute to write to this register 0x5C if that is selected, instead of the dedicated channel input A.

the selector is in the channel attributes which I reserved for part 3.
I suppose yeah... in part 2 this is unusable.

rodrigo455 · 2026-05-25T09:14:55Z

@@ -0,0 +1,64 @@
+What:		/sys/bus/iio/devices/iio:deviceX/hw_ldac_tg_state


We might want others to comment on this too...

Maybe we could leverage channel hierarchy (where iio channels may have subchannels) here:
(This is something we are discussing upstream in the AD9910 patches)

current channel 10

label attr: phy1

en attr: powerdown control

raw attr: INPUT A (Async Load Mode - immediate DAC data update)

current channel 11 (child channel)

label attr: toggle1

en attr: enable toggle function

raw attr: INPUT B

sampling_frequency: toggling freq of the pwm

current channel 12 (child channel)

label attr: dither1

en attr: enable dither function

phase attr: $\phi_0$

frequency attr: $2\pi/N$ (controls a divider of the sample rate)

sampling_frequency attr: pwm frequency

raw attr: INPUT B

current channel 20

label attr: phy2

en attr: powerdown control

raw attr: INPUT A (Async Load Mode - immediate DAC data update)

current channel 21 (child channel)

label attr: toggle2

en attr: enable toggle function

raw attr: INPUT B

sampling_frequency: toggling freq of the pwm

current channel 22 (child channel)

label attr: dither2

en attr: enable dither function

phase attr: $\phi_0$

frequency attr: $2\pi/N$ (controls a divider of the sample rate)

sampling_frequency attr: pwm frequency

raw attr: INPUT B
...

I might be getting things wrong, so this is just a draft. when both enables for toggle and/or
dither are disabled, we may assume that LDAC load is the default. This is an idea to include the
support for toggle and dither modes.

I suppose LDAC control might not need to be exposed to the user... the signal could be ignored
for regular sysfs attr control (using Async mode). When using buffers we might want to have
the LDAC pin toggling in HW mode (or using SW mode when no pin is available). The LDAC handling
with buffers will allow to synchronize the update of multiple channels when they are enabled for
buffer transmission.
Manual toggling is preferred if we are sending the (multi-channel) data with triggered buffers.
If sending the data with DMA, offloading the spi, probably toggling control should be done in
hardware (in FPGA), not by a PWM, unless it is the sync source for the spi write cadence.

channel hierarchy is not upstream yet, but opens up for a lot of possibilities and ABI reuse.

ok will wait for other comments, this feels like a relatively big(?) change if ever

rodrigo455 · 2026-05-25T09:19:06Z

+		selected channels' input registers. Write operations update the
+		Multi-DAC Input A register (0x5C).
+
+What:		/sys/bus/iio/devices/iio:deviceX/multi_dac_sw_ldac_trigger


not sure why you need this. LDAC trigger seems to be a separate feature compared to the multi input one.

not sure what you mean by this
the software trigger for multidac and just ldac are on separate registers.
the sw_ldac is under channel attributes so it would be part of part 3 PR.

actorreno changed the title ~~iio: dac: ad5706: add device attributes~~ iio: dac: add ad5706 (part 2) May 19, 2026

actorreno force-pushed the dev_ad5706r_2 branch from 0474fb9 to 2365431 Compare May 19, 2026 08:57

actorreno marked this pull request as ready for review May 20, 2026 00:53

actorreno requested review from CiprianRegus, amiclaus, btogorean, buha, chegbeli, danmois, dbogdan, machschmitt, mhennerich, nunojsa, rbolboac and stefpopa as code owners May 20, 2026 00:53

actorreno mentioned this pull request May 20, 2026

iio: dac: add ad5706r (part 1) #3325

Open

8 tasks

rodrigo455 reviewed May 20, 2026

View reviewed changes

dt-bindings: iio: dac: ad5706r: add address attribute

d8194f4

Adds an attribute entry for device address. This edits the SPI command at bits 13:12 to communicate with multiple ad5706r. Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

actorreno force-pushed the dev_ad5706r_2 branch 3 times, most recently from c239e0d to d76eef2 Compare May 25, 2026 03:45

actorreno added 2 commits May 25, 2026 14:41

Documentation: ABI: testing: add ad5706r

103bf88

This commit adds the documentation for the device attributes added on the ad5706r driver Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

actorreno force-pushed the dev_ad5706r_2 branch from d76eef2 to 103bf88 Compare May 25, 2026 06:42

actorreno requested a review from rodrigo455 May 25, 2026 07:07

rodrigo455 reviewed May 25, 2026

View reviewed changes

		@@ -0,0 +1,64 @@
		What: /sys/bus/iio/devices/iio:deviceX/hw_ldac_tg_state

Conversation

actorreno commented May 19, 2026

PR Description

PR Type

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rodrigo455 May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rodrigo455 May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

actorreno commented May 25, 2026

Uh oh!

rodrigo455 May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rodrigo455 May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

rodrigo455 May 21, 2026 •

edited

Loading

rodrigo455 May 20, 2026 •

edited

Loading

rodrigo455 May 25, 2026 •

edited

Loading

rodrigo455 May 25, 2026 •

edited

Loading