From: Jonathan Cameron <jic23@kernel.org>
To: Janani Sunil <janani.sunil@analog.com>
Cc: "Lars-Peter Clausen" <lars@metafoo.de>,
"Michael Hennerich" <Michael.Hennerich@analog.com>,
"David Lechner" <dlechner@baylibre.com>,
"Nuno Sá" <nuno.sa@analog.com>,
"Andy Shevchenko" <andy@kernel.org>,
"Rob Herring" <robh@kernel.org>,
"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
"Conor Dooley" <conor+dt@kernel.org>,
"Philipp Zabel" <p.zabel@pengutronix.de>,
"Jonathan Corbet" <corbet@lwn.net>,
"Shuah Khan" <skhan@linuxfoundation.org>,
"Mark Brown" <broonie@kernel.org>,
"Marius Cristea" <marius.cristea@microchip.com>,
"Marcus Folkesson" <marcus.folkesson@gmail.com>,
"Kent Gustavsson" <kent@minoris.se>,
"Conor Dooley" <conor.dooley@microchip.com>,
"Daire McNamara" <daire.mcnamara@microchip.com>,
"Paul Walmsley" <pjw@kernel.org>,
"Palmer Dabbelt" <palmer@dabbelt.com>,
"Albert Ou" <aou@eecs.berkeley.edu>,
"Alexandre Ghiti" <alex@ghiti.fr>,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
"Janani Sunil" <jan.sun97@gmail.com>,
linux-spi@vger.kernel.org, "Kent Gustavsson" <nedo80@gmail.com>,
linux-riscv@lists.infradead.org
Subject: Re: [PATCH v8 0/3] iio: dac: Add support for AD5529R DAC
Date: Sat, 15 Aug 2026 22:48:38 +0100 [thread overview]
Message-ID: <20260815224838.2bf9fdde@jic23-huawei> (raw)
In-Reply-To: <20260805-ad5529r-driver-v8-0-b41781f5632a@analog.com>
On Wed, 5 Aug 2026 09:17:40 +0200
Janani Sunil <janani.sunil@analog.com> wrote:
> 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.
https://sashiko.dev/#/patchset/20260805-ad5529r-driver-v8-0-b41781f5632a%40analog.com
has some comments.
I think the dt ones are fine as we do want to explicitly list all
the dac.yaml properties in here to constrain them further.
The one about sleeping after enabling regulators looks correct though.
I'd add an fsleep() to ensure nothing else happens immediately after
switching the power on.
The other sleep related thing about allowing extra time. Don't bother
unless we get a report of problems as there will be extra delay anyway
due to the call overhead etc and datasheet values tend to be conservative.
For the scale related one, perhaps add a comment there referring
to datasheet specifying the reference voltage if an external reference
is used should be same as output of internal reference.
Given your regmap is working I'll assume the one about mixed
endian is a false positive.
Jonathan
>
> **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.
>
> The device can encode a hardware address in its SPI command frame,
> allowing up to 4 devices to share a single chip select. This series
> introduces a generic spi-device-addr property for describing this type
> of addressing.
>
> **Features Implemented:**
> - Support for AD5529R 12-bit and 16-bit variants via device match data.
> - Support for a single DAC device using any hardware address between 0
> and 3.
> - Reset support via reset controller framework.
> - Dual regmap configuration to handle 8 and 16 bit registers.
> - Per-channel output range configuration from devicetree.
> - Optional external reference and bipolar supply handling.
>
> **Patch Summary:**
> 1. Add the generic spi-device-addr SPI peripheral property.
> 2. AD5529R binding documentation with channel configuration.
> 3. Implement AD5529R 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. Hardware specific features like function generators, multi-die
> hotpath registers etc.
>
> The device warrants its own driver 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.
>
> **Not Implemented in this Series:**
> The binding includes generic, peripheral level device addressing needed for
> multi-device support using a shared CS, but the driver presently
> supports only a single device.
>
> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
> ---
> Changes in v8:
> - Rename the AD5529R supply properties to match the datasheet supply
> names.
> - Describe and enable all four positive and four negative high-voltage
> supplies.
> - Reject duplicate channel nodes and configurations with too many
> channels.
> - Add the required 20 ns delay between reset assertion and deassertion.
> - Add a macro to represent the number of channels.
> - Return -ENODATA when device cannot be identified.
> - Clarify that spi-device-addr contains one address per physical device.
> - Drop the Microchip binding and driver changes from the series.
> - Drop brackets around scalar spi-device-addr defaults.
> - Simplify the regmap_assign_bits() reference selection value.
> - Link to v7: https://lore.kernel.org/r/20260722-ad5529r-driver-v7-0-7781cd74ad75@analog.com
>
> Changes in v7:
> - Drop #include <linux/mod_devicetable.h>
> - Add comment on minimum HW reset low width.
> - Flip the vref regulator error check.
> - Clarify why spi-device-addr is an array, with one address per physical
> device sharing a chip select.
> - Add default addresses for mcp3564 and mcp3911 bindings.
> - Initialize AD5529R regmap configurations locally using designated
> initializers.
> - Update "Patch Summary" section in the cover letter.
> - Link to v6: https://lore.kernel.org/r/20260715-ad5529r-driver-v6-0-cfdf8b9f5ee3@analog.com
>
> Changes in v6:
> - Rename spi,device-addr to spi-device-addr and define it as an array at
> peripheral node level.
> - Fix the SPI binding patch subject prefix to use "spi: dt-bindings:".
> - Add spi-device-addr to the MCP3564 and MCP3911 bindings.
> - Deprecate the existing Microchip-specific device-address properties.
> - Replace adi,output-range-microvolt with output-range-microvolt.
> - Move AD5529R device addressing from channel nodes to the peripheral
> node.
> - Include linux/types.h
> - Honour non-zero hardware address in the AD5529R driver.
> - Dynamically expose only the channels described by devicetree and
> remove unused scan type definitions.
> - Use explicit reset assertion and deassertion as the GPIO reset
> controller does not implement reset_control_reset().
> - Use the MICRO and MILLI unit constants when matching output ranges.
> - Simplify optional reference regulator handling and remove the
> external reference comment.
> - Simplify error messages and stick to 80 col.
> - Link to v5: https://lore.kernel.org/r/20260701-ad5529r-driver-v5-0-ed087900e642@analog.com
>
> Changes in v5:
> - Move register bitfield definitions next to their parent register
> addresses.
> - Remove spurious extra indent.
> - Rename ad5529r_output_ranges_mv[] to ad5529r_output_ranges_mV[].
> - Remove extra parentheses in regmap_reg_range() for the readback range.
> - Use reset_control_reset() instead of reset_control_deassert().
> - Use 10 * USEC_PER_MSEC instead of a bare 10000 in fsleep().
> - Use fwnode_property_present() to explicitly guard the optional property.
> - Rewrite external_vref detection using explicit if/else.
> - Follow reverse christmas tree variable declaration order.
> - Improve invalid channel error message to include the maximum.
> - Add a new spi property to include the SPI device address.
> - Update ad5529r devicetree binding to allow more than 16 channels to include multiple DACs.
> - Update cover letter to add a mention about the multi device support.
> - Update driver commit message to describe the device further.
> - Link to v4: https://lore.kernel.org/r/20260609-ad5529r-driver-v4-0-2e4c02234a1a@analog.com
>
> Changes in v4:
> - Fix DT child-node regex for hexadecimal channel addresses.
> - Wrap long DT binding description lines.
> - Simplify optional `vref-supply` and `hvss-supply` handling.
> - Update REF_SEL programming for optional external reference use.
> - Clean up range parsing and error messages.
> - Simplify debugfs register access by calling regmap helpers directly.
> - Add clarifying comments for reset settling time and RAW reads from `DAC_INPUT_A`.
> - Remove an unused vref regulator pointer and an include.
> - Rename the REF_SEL bit define and clean up small driver details.
> - Toggle pins defined as PWM pins, instead of GPIOs
> - Update cover letter to sync up latest changes.
> - Link to v3: https://lore.kernel.org/r/20260519-ad5529r-driver-v3-0-267c0731aa68@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 (3):
> spi: dt-bindings: Add spi-device-addr peripheral property
> dt-bindings: iio: dac: Add AD5529R
> iio: dac: Add AD5529R DAC driver support
>
> .../devicetree/bindings/iio/dac/adi,ad5529r.yaml | 254 ++++++++++
> .../bindings/spi/spi-peripheral-props.yaml | 7 +
> MAINTAINERS | 8 +
> drivers/iio/dac/Kconfig | 17 +
> drivers/iio/dac/Makefile | 1 +
> drivers/iio/dac/ad5529r.c | 530 +++++++++++++++++++++
> 6 files changed, 817 insertions(+)
> ---
> base-commit: 93df88612859e8e19dec93c69d563b4b73e9bd4b
> change-id: 20260507-ad5529r-driver-866bbdd864de
>
> Best regards,
prev parent reply other threads:[~2026-08-15 21:48 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-05 7:17 [PATCH v8 0/3] iio: dac: Add support for AD5529R DAC Janani Sunil
2026-08-05 7:17 ` [PATCH v8 1/3] spi: dt-bindings: Add spi-device-addr peripheral property Janani Sunil
2026-08-05 7:17 ` [PATCH v8 2/3] dt-bindings: iio: dac: Add AD5529R Janani Sunil
2026-08-05 16:29 ` Conor Dooley
2026-08-05 7:17 ` [PATCH v8 3/3] iio: dac: Add AD5529R DAC driver support Janani Sunil
2026-08-15 21:53 ` Jonathan Cameron
2026-08-15 21:48 ` Jonathan Cameron [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260815224838.2bf9fdde@jic23-huawei \
--to=jic23@kernel.org \
--cc=Michael.Hennerich@analog.com \
--cc=alex@ghiti.fr \
--cc=andy@kernel.org \
--cc=aou@eecs.berkeley.edu \
--cc=broonie@kernel.org \
--cc=conor+dt@kernel.org \
--cc=conor.dooley@microchip.com \
--cc=corbet@lwn.net \
--cc=daire.mcnamara@microchip.com \
--cc=devicetree@vger.kernel.org \
--cc=dlechner@baylibre.com \
--cc=jan.sun97@gmail.com \
--cc=janani.sunil@analog.com \
--cc=kent@minoris.se \
--cc=krzk+dt@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-doc@vger.kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux-spi@vger.kernel.org \
--cc=marcus.folkesson@gmail.com \
--cc=marius.cristea@microchip.com \
--cc=nedo80@gmail.com \
--cc=nuno.sa@analog.com \
--cc=p.zabel@pengutronix.de \
--cc=palmer@dabbelt.com \
--cc=pjw@kernel.org \
--cc=robh@kernel.org \
--cc=skhan@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox