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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `max_register` configuration for AD3532R is off-by-one, permanently breaking writes to the highest channel.
--

commit 471c601807c7303ea879a6246deaa17af7989f08
Author: Kim Seer Paller <kimseer.paller@analog.com>

iio: dac: ad3530r: Add support for AD3532R/AD3532

The AD3532R/AD3532 is a 16-channel, 16-bit voltage output DAC with a
dual-bank register architecture. This commit adds AD3532R-specific register
definitions, channel specs, per-bank arrays, and per-chip regmap_config.

> diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
> index 3f7c983739fc7..8e17e738d4c8c 100644
> --- a/drivers/iio/dac/ad3530r.c
> +++ b/drivers/iio/dac/ad3530r.c

[ ... ]

> @@ -39,6 +40,23 @@
>  #define AD3531R_SW_LDAC_TRIG_A			0xDD
>  #define AD3531R_INPUT_CH			0xE3
>  
> +/* AD3532R/AD3532 has two register banks: bank 0 at 0x10xx, bank 1 at 0x30xx */
> +#define AD3532R_INTERFACE_CONFIG_A_0		0x1000
> +#define AD3532R_OUTPUT_OPERATING_MODE_0		0x1020
> +#define AD3532R_OUTPUT_OPERATING_MODE_1		0x1021
> +#define AD3532R_OUTPUT_CONTROL_0		0x102A
> +#define AD3532R_REFERENCE_CONTROL_0		0x103C
> +#define AD3532R_SW_LDAC_TRIG_0			0x10E5
> +#define AD3532R_INPUT_CH_0			0x10EB
> +#define AD3532R_INTERFACE_CONFIG_A_1		0x3000
> +#define AD3532R_OUTPUT_OPERATING_MODE_2		0x3020
> +#define AD3532R_OUTPUT_OPERATING_MODE_3		0x3021
> +#define AD3532R_OUTPUT_CONTROL_1		0x302A
> +#define AD3532R_REFERENCE_CONTROL_1		0x303C
> +#define AD3532R_SW_LDAC_TRIG_1			0x30E5
> +#define AD3532R_INPUT_CH_1			0x30EB
> +#define AD3532R_MAX_REG_ADDR			0x30F9

[Severity: High]
Does this max register bound leave out the second byte for channel 15?

Looking at ad3532r_input_ch_reg(), the base register for channel 15 is
calculated as:
2 * (15 - 8) + AD3532R_INPUT_CH_1 (0x30EB) = 0x30F9.

Since this is a 16-bit DAC, writing a value to channel 15 requires writing
to both 0x30F9 and 0x30FA. Because ad3532r_regmap_config sets max_register
to 0x30F9, will regmap reject bulk writes to the final byte and return
-EINVAL, effectively making channel 15 unusable?

It looks like this off-by-one pattern might have been copied from the
pre-existing AD3530R_MAX_REG_ADDR configuration.

> +
>  #define AD3530R_SLD_TRIG_A			BIT(7)
>  #define AD3530R_OUTPUT_CONTROL_RANGE		BIT(2)
>  #define AD3530R_REFERENCE_CONTROL_SEL		BIT(0)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-iio-ad3532r-support-v3-0-f6e4f4abebbe@analog.com?part=4