Re: [PATCH v2 4/4] iio: dac: ad3530r: Add support for AD3532R/AD3532

[Jonathan Cameron]

On Mon, 15 Jun 2026 13:05:44 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:

> On Mon, Jun 15, 2026 at 02:20:18PM +0800, Kim Seer Paller wrote:
> > The AD3532R/AD3532 is a 16-channel, 16-bit voltage output DAC with a
> > dual-bank register architecture (bank 0 at 0x1000 for channels 0-7,
> > bank 1 at 0x3000 for channels 8-15). It shares similar functionality
> > with AD3530R (channel configuration, LDAC triggering, powerdown control),
> > the main difference being the register address map due to the dual-bank
> > architecture, handled by table-driven helpers.
> > 
> > Add AD3532R-specific register definitions, channel specs, per-bank
> > register arrays, a dedicated ad3532r_set_dac_powerdown(), and per-chip
> > regmap_config to limit debugfs-exposed register space to each variant's
> > actual address range.  
> 
> ...
> 
> 
> >  	help
> > -	  Say yes here to build support for Analog Devices AD3530R, AD3531R
> > -	  Digital to Analog Converter.
> > +	  Say yes here to build support for Analog Devices AD3530/AD3530R,
> > +	  AD3531/AD3531R, and AD3532/AD3532R Digital to Analog Converters.  
> 
> This just shows how unscalable the above text is. That's why we usually
> recommend to make the list explicit and separated.
> 
> 	  Say yes here to build support for the following Analog Devices
> 	  Digital to Analog Converters:
> 	  - AD3530/AD3530R (8-channel)
> 	  - AD3531/AD3531R (4-channel)
> 	  - AD3532/AD3532R (16-channel)
> 
> (and looking into the C-file change, perhaps add here as well distinctive
>  information, such as number of channels, in the parentheses).
> 
> >  	  To compile this driver as a module, choose M here: the
> >  	  module will be called ad3530r.  
> 
> ...
> 
> > +#define AD3532R_INTERFACE_CONFIG_A_0		0x1000
> > +#define AD3532R_INTERFACE_CONFIG_A_1		0x3000
> > +#define AD3532R_OUTPUT_OPERATING_MODE_0		0x1020
> > +#define AD3532R_OUTPUT_OPERATING_MODE_1		0x1021
> > +#define AD3532R_OUTPUT_OPERATING_MODE_2		0x3020
> > +#define AD3532R_OUTPUT_OPERATING_MODE_3		0x3021
> > +#define AD3532R_OUTPUT_CONTROL_0		0x102A
> > +#define AD3532R_OUTPUT_CONTROL_1		0x302A
> > +#define AD3532R_REFERENCE_CONTROL_0		0x103C
> > +#define AD3532R_REFERENCE_CONTROL_1		0x303C
> > +#define AD3532R_SW_LDAC_TRIG_0			0x10E5
> > +#define AD3532R_SW_LDAC_TRIG_1			0x30E5
> > +#define AD3532R_INPUT_CH_0			0x10EB
> > +#define AD3532R_INPUT_CH_1			0x30EB
> > +#define AD3532R_MAX_REG_ADDR			0x30F9  
Whilst we are here, Sashiko thinks there is an off by one on that value
as it's the lower of the two registers that make up channel 15.
https://sashiko.dev/#/patchset/20260615-iio-ad3532r-support-v2-0-84a0af8b83fa%40analog.com
It also suggests an existing bug that it would be good to look into.

> 
> Hmm... I dunno if it's better to sort by values (so the "bank" 0 goes together
> followed by "bank" 1). Jonathan, what's your preference here? Nuno, David?
That is how people will typically check them vs the datasheet so I
agree with numeric order.  Maybe with a comment at the top about there
effectively being two banks. Many of the registers are effectively copies
for the new channels but not all of them, so a macro approach would
probably be even more confusing.

Jonathan