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

[Paller, Kim Seer]

> From: Jonathan Cameron <jic23@xxxxxxxxxx>
> Sent: Monday, June 22, 2026 12:46 AM
> To: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> Cc: Paller, Kim Seer <KimSeer.Paller@xxxxxxxxxx>; David Lechner
> <dlechner@xxxxxxxxxxxx>; Sa, Nuno <Nuno.Sa@xxxxxxxxxx>; Andy
> Shevchenko <andy@xxxxxxxxxx>; Hennerich, Michael
> <Michael.Hennerich@xxxxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof
> Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>;
> linux-iio@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux
> <linux@xxxxxxxxxx>; devicetree@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2 4/4] iio: dac: ad3530r: Add support for
> AD3532R/AD3532
> 
> [External]
> 
> 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://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260615-iio-
> ad3532r-support-v2-0-
> 84a0af8b83fa*40analog.com__;IyU!!A3Ni8CS0y2Y!88afCOStwucx32wuoeR
> SyZ9GpkZge9YDw5_PIMAf7SLs3OLykUC_qNRDUCnRw7wTwsxiIT1V-
> R8sH17sTg$
> It also suggests an existing bug that it would be good to look into.

I don't think it's off-by-one. INPUT_CHn registers are listed by LSB, so channel 15 is 0x30F8 (LSB) / 0x30F9 (MSB).
The driver addresses the MSB and the part defaults to descending mode, so the access goes 0x30F9 -> 0x30F8.
0x30F9 is also the highest valid address per the datasheet, so max_register looks correct same for AD3530R's 0xF9.
Does that match our understanding, or am I missing a case?

> 
> >
> > 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