Skip to content

add driver for ad5710r/ad5711r#3191

Open
kseerp wants to merge 3 commits into
analogdevicesinc:mirror_ci/jic23/iio/testingfrom
kseerp:dev/ad5710r
Open

add driver for ad5710r/ad5711r#3191
kseerp wants to merge 3 commits into
analogdevicesinc:mirror_ci/jic23/iio/testingfrom
kseerp:dev/ad5710r

Conversation

@kseerp
Copy link
Copy Markdown
Member

@kseerp kseerp commented Mar 16, 2026

PR Description

This PR adds support for the AD5710R (16-bit) and AD5711R (12-bit) 8-channel configurable IDAC/VDAC.
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5711r-ad5710r.pdf

The AD5710R/AD5711R shares a similar register interface with the
AD3530R family but introduces current output (IDAC) support alongside
voltage output (VDAC), independently configurable per channel. This
brings additional complexity in channel mode configuration and
powerdown handling (high_z for IMODE, 15kohm_to_gnd for VMODE).

Rather than extending the AD3530R driver - which is already growing
in complexity with AD3532R dual-bank support (#3177) - this is
introduced as a dedicated driver.

Looking for feedback on whether keeping this as a separate driver is
the right approach, or if it should be folded into the AD3530R driver.

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

@gastmaier
Copy link
Copy Markdown
Collaborator

is this targetting upstream? if so, can you target a mirroc_ci branch?
main is 6.12, upstream current cycle is 7.0
https://analogdevicesinc.github.io/linux/ci.html

Comment on lines +763 to +764
three_state: left floating,
high_z: left floating.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the difference between three_state and high_z ? max5821 uses three_state

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the difference is naming convention. Voltage DAC like max5821 describe the powered-down output as tri-state, while current DAC like the ad5710R describe it as high impedance. I used high_z here to match the AD5710R datasheet terminology. That said, both are defined as left floating in the IIO ABI. Should I just use three_state instead for consistency with existing DAC drivers?

@gastmaier
Copy link
Copy Markdown
Collaborator

gastmaier commented Mar 25, 2026

@kseerp kseerp changed the base branch from main to mirror_ci/jic23/iio/testing April 6, 2026 03:16
@kseerp kseerp force-pushed the dev/ad5710r branch 2 times, most recently from 37b5e11 to 80469f8 Compare April 6, 2026 03:58
@kseerp
Copy link
Copy Markdown
Member Author

kseerp commented Apr 6, 2026

rebase to mirror_ci/jic23/iio/testing branch

@gastmaier gastmaier force-pushed the mirror_ci/jic23/iio/testing branch 7 times, most recently from 5d18d61 to 353fec2 Compare April 13, 2026 00:12
@gastmaier gastmaier force-pushed the mirror_ci/jic23/iio/testing branch 7 times, most recently from 49a7412 to 4674362 Compare April 26, 2026 00:12
@gastmaier gastmaier force-pushed the mirror_ci/jic23/iio/testing branch 2 times, most recently from 78b10cd to be1b2a6 Compare April 28, 2026 00:13
kseerp added 3 commits April 28, 2026 14:08
Add high_z powerdown mode for DACs with high impedance output in
current mode (IDAC) and 15kohm_to_gnd resistor to GND. Also add
out_currentY_powerdown_mode, out_currentY_powerdown_mode_available,
and out_currentY_powerdown entries to document current output
powerdown support.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
Add device tree bindings for the Analog Devices AD5710R/AD5711R
8-channel 12-/16-bit Configurable IDAC/VDAC.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
The AD5710R (16-bit) and AD5711R (12-bit) are 8-channel, low-power,
configurable current/voltage output DACs with an on-chip 2.5V, 3ppm/°C
reference. Each channel can be independently configured as a voltage
output (0V to VREF or 0V to 2xVREF) or a current output (0mA to 50mA).
These devices operate from a single 2.7V to 5.5V supply and are
guaranteed monotonic by design.

Support for monitoring internal die temperature, output voltages, and
current of a selected channel via the MUXOUT pin using an external ADC
is currently not implemented.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
@kseerp kseerp added the llm review Request a review from a LLM Reviewer label Apr 28, 2026
@github-actions
Copy link
Copy Markdown
Contributor

LLM review

This series adds an IIO DAC driver for the AD5710R/AD5711R 8-channel configurable IDAC/VDAC,
along with DT bindings and ABI documentation updates.

run: 25038204558

00f607c50 - iio: dac: ad5710r: Add driver for AD5710R and AD5711R

Critical: Hardware reset GPIO never released — device stays in reset

The reset GPIO is asserted (driven LOW) but never released back to HIGH. The datasheet
states initialization occurs when RESET is deasserted (rising edge). As written, the
device is permanently held in reset after probe; all subsequent register writes are lost.

// drivers/iio/dac/ad5710r.c:359
if (reset_gpio) {
    fsleep(1 * USEC_PER_MSEC);
    gpiod_set_value_cansleep(reset_gpio, 0);  // asserted, never released
}
fsleep(10 * USEC_PER_MSEC);  // device is still in reset

The correct sequence: assert LOW → hold ≥ 160 ns → release HIGH → wait 10 ms.


High: Out-of-bounds heap write in channel array allocation

The iio_channels array is allocated with num_chan (count of DT child nodes) slots,
but indexed by reg (hardware channel number, 0–7). Any non-contiguous channel
configuration causes an out-of-bounds write. For example, configuring channels 4 and 5
allocates 2 slots but writes to iio_channels[4] and iio_channels[5].

// drivers/iio/dac/ad5710r.c:269
st->iio_channels = devm_kcalloc(dev, num_chan, ...);  // e.g. 2 slots
...
st->iio_channels[reg] = ...;  // reg=4 → out-of-bounds write

Fix: allocate AD5710R_NUM_CHANNELS (8) slots unconditionally.


Medium: regmap_set_bits() on write-only register

SW_LDAC_TRIG_A (0xE5) is write-only per the datasheet. regmap_set_bits() performs
a read-modify-write on a register that cannot be read. Use regmap_write() directly.


a4a6d5ddc - dt-bindings: iio: dac: add adi,ad5710r.yaml

Commit subject has a leading space: dt-bindings: iio: dac: add adi,ad5710r.yaml

CI warnings

checkpatch reports 3 line-length warnings (>100 columns) for dev_err_probe strings
in ad5710r.c at lines 299, 330, and 338.

Verification data

Datasheet ad5711r-ad5710r.pdf was obtained from analog.com and its pre-converted
markdown was fetched from the doctools repository to verify all register addresses, bit
field positions, SPI protocol details, and LDAC/RESET timing. The driver was compiled
for both arm (32-bit) and aarch64 — both build cleanly after applying the fixups.

Suggested patches

Apply the suggested patches with:

cd path/to/repository
git am 0001-fixup-iio-dac-ad5710r-Add-driver-for-AD5710R-and-AD5.patch
git am 0002-fixup-iio-dac-ad5710r-Add-driver-for-AD5710R-and-AD5.patch
git am 0003-fixup-iio-dac-ad5710r-Add-driver-for-AD5710R-and-AD5.patch
Install instructions

The following one-liner installs the script if not present already:

grep "/apply-patches.sh" ~/.bashrc ||  { curl "https://raw.githubusercontent.com/analogdevicesinc/doctools/refs/heads/main/ci/scripts/apply-patches.sh"    -o ~/.local/bin/apply-patches.sh &&  echo "source ~/.local/bin/apply-patches.sh" >> ~/.bashrc ; source ~/.bashrc ; }

More information at AI Usage.

@gastmaier gastmaier force-pushed the mirror_ci/jic23/iio/testing branch 8 times, most recently from e196cfc to fdad854 Compare May 6, 2026 00:11
@gastmaier gastmaier force-pushed the mirror_ci/jic23/iio/testing branch 7 times, most recently from 1719553 to 55ab0e3 Compare May 13, 2026 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llm review Request a review from a LLM Reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants