[RFC PATCH 0/3] ad9832: driver cleanup

[RFC PATCH 0/3] ad9832: driver cleanup

[Tomas Borquez]

This series is a general cleanup of ad9832, with the purpose of 
graduating it from staging. The main changes are removing legacy 
platform_data support, converting to IIO channels with read/write_raw 
callbacks, and adding devicetree support.

I'm sending this as an RFC because I have some concerns about the ABI 
design and would appreciate guidance before putting more time into this.

Patch 1 removes the legacy platform_data support as suggested by
Jonathan [1]. The driver now initializes to a safe state and lets
userspace configure frequencies/phases via sysfs.

Patch 2 converts frequency and phase configuration from custom sysfs
attributes to proper IIO channels using read_raw/write_raw callbacks
(This is the main area where I'd like feedback).

Patch 3 adds devicetree bindings documentation.

Design Concerns:
1) Channel Organization and ABI Break

   The device has 2 frequency registers and 4 phase registers. Since both
   frequency and phase must use IIO_ALTVOLTAGE since there's no better fit
   (as far as I know), I've organized channels as:

     out_altvoltage0_frequency  (FREQ0)
     out_altvoltage1_frequency  (FREQ1)
     out_altvoltage2_phase      (PHASE0)
     out_altvoltage3_phase      (PHASE1)
     out_altvoltage4_phase      (PHASE2)
     out_altvoltage5_phase      (PHASE3)

   The old ABI used out_altvoltage0_frequency0, out_altvoltage0_frequency1,
   out_altvoltage0_phase0, etc. 

   The new approach felt cleaner but I'm open to alternatives and better 
   ways of mapping them. Is this channel mapping reasonable, or would a 
   different organization be preferred? And is the ABI break okay?

2) Scale Attributes

   The frequency scale is 1 Hz and phase scale is 2*PI/4096 radians.
   I cannot use info_mask_shared_by_type for IIO_CHAN_INFO_SCALE because
   all channels share IIO_ALTVOLTAGE.

   So instead I'm using IIO_CONST_ATTR for the scales:

     out_altvoltage_frequency_scale = "1"
     out_altvoltage_phase_scale = "0.0015339808"

   Is there a better approach here? Or should I just document the units and
   skip scale attributes entirely?

3) Remaining Custom Attributes

   Other controls remain as custom sysfs attributes:

     - out_altvoltage_frequencysymbol: select active frequency register
     - out_altvoltage_phasesymbol: select active phase register  
     - out_altvoltage_pincontrol_en: hardware pin control enable
     - out_altvoltage_out_enable: output enable

   I'm not sure if these map cleanly to IIO interfaces. Should these be
   documented in ABI or is there a preferred way to handle them?

4) Implementation Notes

   - read_raw uses explicit address switching rather than channel index
     arithmetic for clarity, though phase values could alternatively be
     accessed via st->phase[chan->channel - 2] and directly in freq with
     st->freq[chan->channel].
   - I'm unsure if mutex guards on cached reads are necessary.

Link: https://lore.kernel.org/linux-iio/20250628161040.3d21e2c4@jic23-huawei/ [1]
Signed-off-by: Tomas Borquez <tomasborquez13@gmail.com>

Tomas Borquez (3):
  staging: iio: ad9832: remove platform_data support
  staging: iio: ad9832: convert to iio channels
  dt-bindings: iio: add analog devices ad9832/ad9835

 .../bindings/iio/frequency/adi,ad9832.yaml    |  65 +++++
 drivers/staging/iio/frequency/ad9832.c        | 264 +++++++++++-------
 drivers/staging/iio/frequency/ad9832.h        |  33 ---
 3 files changed, 233 insertions(+), 129 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,ad9832.yaml
 delete mode 100644 drivers/staging/iio/frequency/ad9832.h

-- 
2.43.0

[RFC PATCH 1/3] staging: iio: ad9832: remove platform_data support

[Tomas Borquez]

Remove legacy platform_data support as there are no in tree users and
this approach belongs to a long gone era. The policy decision on what
to output is a userspace problem, not something that should be provided
from firmware.

The driver now initializes the device to a safe state (SLEEP|RESET|CLR)
outputting nothing. Userspace can configure the desired frequencies and
phases via the existing sysfs attributes once the device is ready to be 
used.

Original discussion started here [1].

Link: https://lore.kernel.org/linux-iio/20250628161040.3d21e2c4@jic23-huawei/ [1]
Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Tomas Borquez <tomasborquez13@gmail.com>
---
 drivers/staging/iio/frequency/ad9832.c | 32 -------------------------
 drivers/staging/iio/frequency/ad9832.h | 33 --------------------------
 2 files changed, 65 deletions(-)
 delete mode 100644 drivers/staging/iio/frequency/ad9832.h

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 49388da5a6..e2ad3e5a7a 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -23,8 +23,6 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#include "ad9832.h"
-
 #include "dds.h"
 
 /* Registers */
@@ -299,16 +297,10 @@ static const struct iio_info ad9832_info = {
 
 static int ad9832_probe(struct spi_device *spi)
 {
-	struct ad9832_platform_data *pdata = dev_get_platdata(&spi->dev);
 	struct iio_dev *indio_dev;
 	struct ad9832_state *st;
 	int ret;
 
-	if (!pdata) {
-		dev_dbg(&spi->dev, "no platform data?\n");
-		return -ENODEV;
-	}
-
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
 	if (!indio_dev)
 		return -ENOMEM;
@@ -379,30 +371,6 @@ static int ad9832_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	ret = ad9832_write_frequency(st, AD9832_FREQ0HM, pdata->freq0);
-	if (ret)
-		return ret;
-
-	ret = ad9832_write_frequency(st, AD9832_FREQ1HM, pdata->freq1);
-	if (ret)
-		return ret;
-
-	ret = ad9832_write_phase(st, AD9832_PHASE0H, pdata->phase0);
-	if (ret)
-		return ret;
-
-	ret = ad9832_write_phase(st, AD9832_PHASE1H, pdata->phase1);
-	if (ret)
-		return ret;
-
-	ret = ad9832_write_phase(st, AD9832_PHASE2H, pdata->phase2);
-	if (ret)
-		return ret;
-
-	ret = ad9832_write_phase(st, AD9832_PHASE3H, pdata->phase3);
-	if (ret)
-		return ret;
-
 	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
diff --git a/drivers/staging/iio/frequency/ad9832.h b/drivers/staging/iio/frequency/ad9832.h
deleted file mode 100644
index d0d840edb8..0000000000
--- a/drivers/staging/iio/frequency/ad9832.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- * AD9832 SPI DDS driver
- *
- * Copyright 2011 Analog Devices Inc.
- */
-#ifndef IIO_DDS_AD9832_H_
-#define IIO_DDS_AD9832_H_
-
-/*
- * TODO: struct ad9832_platform_data needs to go into include/linux/iio
- */
-
-/**
- * struct ad9832_platform_data - platform specific information
- * @freq0:		power up freq0 tuning word in Hz
- * @freq1:		power up freq1 tuning word in Hz
- * @phase0:		power up phase0 value [0..4095] correlates with 0..2PI
- * @phase1:		power up phase1 value [0..4095] correlates with 0..2PI
- * @phase2:		power up phase2 value [0..4095] correlates with 0..2PI
- * @phase3:		power up phase3 value [0..4095] correlates with 0..2PI
- */
-
-struct ad9832_platform_data {
-	unsigned long		freq0;
-	unsigned long		freq1;
-	unsigned short		phase0;
-	unsigned short		phase1;
-	unsigned short		phase2;
-	unsigned short		phase3;
-};
-
-#endif /* IIO_DDS_AD9832_H_ */
-- 
2.43.0

Re: [RFC PATCH 1/3] staging: iio: ad9832: remove platform_data support

[Andy Shevchenko]

On Fri, Dec 05, 2025 at 05:27:41PM -0300, Tomas Borquez wrote:
> Remove legacy platform_data support as there are no in tree users and
> this approach belongs to a long gone era. The policy decision on what
> to output is a userspace problem, not something that should be provided
> from firmware.
> 
> The driver now initializes the device to a safe state (SLEEP|RESET|CLR)
> outputting nothing. Userspace can configure the desired frequencies and
> phases via the existing sysfs attributes once the device is ready to be 

Tailing space on the above line (and the only line with this issue).

> used.
> 
> Original discussion started here [1].

The change LGTM, Jonathan, can you amend the above and apply it?
(Yes, I have read the discussion about removal the driver, but meanwhile
 the change is good on itself even if we are going to remove the driver.
 It just makes an additional harmless step in my opinion.)

FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH 1/3] staging: iio: ad9832: remove platform_data support

[Jonathan Cameron]

On Sat, 6 Dec 2025 18:24:18 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Fri, Dec 05, 2025 at 05:27:41PM -0300, Tomas Borquez wrote:
> > Remove legacy platform_data support as there are no in tree users and
> > this approach belongs to a long gone era. The policy decision on what
> > to output is a userspace problem, not something that should be provided
> > from firmware.
> > 
> > The driver now initializes the device to a safe state (SLEEP|RESET|CLR)
> > outputting nothing. Userspace can configure the desired frequencies and
> > phases via the existing sysfs attributes once the device is ready to be   
> 
> Tailing space on the above line (and the only line with this issue).
> 
> > used.
> > 
> > Original discussion started here [1].  
> 
> The change LGTM, Jonathan, can you amend the above and apply it?
> (Yes, I have read the discussion about removal the driver, but meanwhile
>  the change is good on itself even if we are going to remove the driver.
>  It just makes an additional harmless step in my opinion.)
OK done. Small side note. If adding comments to link tags (e.g. the [1])
then use #[1].

b4 otherwise picks the thing up as two link tags. At least I think that is
what caused:

  [PATCH RFC 1/3] staging: iio: ad9832: remove platform_data support
    + Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
    + Link: ... [1]

Looks like something stripped the extra space anyway when I was applying.
I didn't bother to check what.

Jonathan

> 
> FWIW,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> 

[RFC PATCH 2/3] staging: iio: ad9832: convert to iio channels

[Tomas Borquez]

Replace the custom frequency and phase sysfs attributes with IIO channels
using read_raw()/write_raw() callbacks, as well as removing the dds.h
header.

Changes:
- Add iio_chan_spec definitions for 2 frequency and 4 phase channels.
- Implement read_raw/write_raw for IIO_CHAN_INFO_FREQUENCY/PHASE.
- Cache frequency and phase values in driver state for readback.
- Remove dependency on dds.h macros for sysfs.
- Use guard(mutex) for cleaner locking.
- Add input validation and consistent error messages.

NOTE: This changes the userspace ABI, see cover letter.
Signed-off-by: Tomas Borquez <tomasborquez13@gmail.com>
---
 drivers/staging/iio/frequency/ad9832.c | 232 ++++++++++++++++++-------
 1 file changed, 168 insertions(+), 64 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index e2ad3e5a7a..79d26009d1 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -9,6 +9,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -23,10 +24,7 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#include "dds.h"
-
 /* Registers */
-
 #define AD9832_FREQ0LL		0x0
 #define AD9832_FREQ0HL		0x1
 #define AD9832_FREQ0LM		0x2
@@ -50,7 +48,6 @@
 #define AD9832_OUTPUT_EN	0x13
 
 /* Command Control Bits */
-
 #define AD9832_CMD_PHA8BITSW	0x1
 #define AD9832_CMD_PHA16BITSW	0x0
 #define AD9832_CMD_FRE8BITSW	0x3
@@ -79,6 +76,8 @@
  * @ctrl_fp:		cached frequency/phase control word
  * @ctrl_ss:		cached sync/selsrc control word
  * @ctrl_src:		cached sleep/reset/clr word
+ * @freq:		cached frequencies
+ * @phase:		cached phases
  * @xfer:		default spi transfer
  * @msg:		default spi message
  * @freq_xfer:		tuning word spi transfer
@@ -90,13 +89,14 @@
  * @phase_data:		tuning word spi transmit buffer
  * @freq_data:		tuning word spi transmit buffer
  */
-
 struct ad9832_state {
 	struct spi_device		*spi;
 	struct clk			*mclk;
 	unsigned short			ctrl_fp;
 	unsigned short			ctrl_ss;
 	unsigned short			ctrl_src;
+	u32				freq[2];
+	u32				phase[4];
 	struct spi_transfer		xfer;
 	struct spi_message		msg;
 	struct spi_transfer		freq_xfer[4];
@@ -115,7 +115,7 @@ struct ad9832_state {
 	} __aligned(IIO_DMA_MINALIGN);
 };
 
-static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
+static unsigned long ad9832_calc_freqreg(unsigned long mclk, u32 fout)
 {
 	unsigned long long freqreg = (u64)fout *
 				     (u64)((u64)1L << AD9832_FREQ_BITS);
@@ -124,12 +124,24 @@ static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
 }
 
 static int ad9832_write_frequency(struct ad9832_state *st,
-				  unsigned int addr, unsigned long fout)
+				  unsigned int addr, u32 fout)
 {
 	unsigned long clk_freq;
 	unsigned long regval;
 	u8 regval_bytes[4];
 	u16 freq_cmd;
+	int ret, idx;
+
+	switch (addr) {
+	case AD9832_FREQ0HM:
+		idx = 0;
+		break;
+	case AD9832_FREQ1HM:
+		idx = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
 
 	clk_freq = clk_get_rate(st->mclk);
 
@@ -147,14 +159,37 @@ static int ad9832_write_frequency(struct ad9832_state *st,
 			FIELD_PREP(AD9832_DAT_MSK, regval_bytes[i]));
 	}
 
-	return spi_sync(st->spi, &st->freq_msg);
+	ret = spi_sync(st->spi, &st->freq_msg);
+	if (ret)
+		return ret;
+
+	st->freq[idx] = fout;
+	return 0;
 }
 
 static int ad9832_write_phase(struct ad9832_state *st,
-			      unsigned long addr, unsigned long phase)
+			      unsigned long addr, u32 phase)
 {
 	u8 phase_bytes[2];
 	u16 phase_cmd;
+	int ret, idx;
+
+	switch (addr) {
+	case AD9832_PHASE0H:
+		idx = 0;
+		break;
+	case AD9832_PHASE1H:
+		idx = 1;
+		break;
+	case AD9832_PHASE2H:
+		idx = 2;
+		break;
+	case AD9832_PHASE3H:
+		idx = 3;
+		break;
+	default:
+		return -EINVAL;
+	}
 
 	if (phase >= BIT(AD9832_PHASE_BITS))
 		return -EINVAL;
@@ -169,10 +204,77 @@ static int ad9832_write_phase(struct ad9832_state *st,
 			FIELD_PREP(AD9832_DAT_MSK, phase_bytes[i]));
 	}
 
-	return spi_sync(st->spi, &st->phase_msg);
+	ret = spi_sync(st->spi, &st->phase_msg);
+	if (ret)
+		return ret;
+
+	st->phase[idx] = phase;
+	return 0;
 }
 
-static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
+static int ad9832_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	struct ad9832_state *st = iio_priv(indio_dev);
+
+	if (val < 0)
+		return -EINVAL;
+
+	guard(mutex)(&st->lock);
+	switch (mask) {
+	case IIO_CHAN_INFO_FREQUENCY:
+		return ad9832_write_frequency(st, chan->address, val);
+	case IIO_CHAN_INFO_PHASE:
+		return ad9832_write_phase(st, chan->address, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad9832_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct ad9832_state *st = iio_priv(indio_dev);
+
+	guard(mutex)(&st->lock);
+	switch (mask) {
+	case IIO_CHAN_INFO_FREQUENCY:
+		switch (chan->address) {
+		case AD9832_FREQ0HM:
+			*val = st->freq[0];
+			return IIO_VAL_INT;
+		case AD9832_FREQ1HM:
+			*val = st->freq[1];
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_PHASE:
+		switch (chan->address) {
+		case AD9832_PHASE0H:
+			*val = st->phase[0];
+			return IIO_VAL_INT;
+		case AD9832_PHASE1H:
+			*val = st->phase[1];
+			return IIO_VAL_INT;
+		case AD9832_PHASE2H:
+			*val = st->phase[2];
+			return IIO_VAL_INT;
+		case AD9832_PHASE3H:
+			*val = st->phase[3];
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static ssize_t ad9832_store(struct device *dev,
+			    struct device_attribute *attr,
 			    const char *buf, size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
@@ -183,20 +285,10 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 
 	ret = kstrtoul(buf, 10, &val);
 	if (ret)
-		goto error_ret;
+		return ret;
 
-	mutex_lock(&st->lock);
-	switch ((u32)this_attr->address) {
-	case AD9832_FREQ0HM:
-	case AD9832_FREQ1HM:
-		ret = ad9832_write_frequency(st, this_attr->address, val);
-		break;
-	case AD9832_PHASE0H:
-	case AD9832_PHASE1H:
-	case AD9832_PHASE2H:
-	case AD9832_PHASE3H:
-		ret = ad9832_write_phase(st, this_attr->address, val);
-		break;
+	guard(mutex)(&st->lock);
+	switch (this_attr->address) {
 	case AD9832_PINCTRL_EN:
 		st->ctrl_ss &= ~AD9832_SELSRC;
 		st->ctrl_ss |= FIELD_PREP(AD9832_SELSRC, val ? 0 : 1);
@@ -206,13 +298,13 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 		ret = spi_sync(st->spi, &st->msg);
 		break;
 	case AD9832_FREQ_SYM:
-		if (val == 1 || val == 0) {
-			st->ctrl_fp &= ~AD9832_FREQ;
-			st->ctrl_fp |= FIELD_PREP(AD9832_FREQ, val ? 1 : 0);
-		} else {
+		if (val != 1 && val != 0) {
 			ret = -EINVAL;
 			break;
 		}
+
+		st->ctrl_fp &= ~AD9832_FREQ;
+		st->ctrl_fp |= FIELD_PREP(AD9832_FREQ, val);
 		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_FPSELECT) |
 						  st->ctrl_fp);
 		ret = spi_sync(st->spi, &st->msg);
@@ -243,47 +335,56 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 	default:
 		ret = -ENODEV;
 	}
-	mutex_unlock(&st->lock);
 
-error_ret:
 	return ret ? ret : len;
 }
 
-/*
- * see dds.h for further information
- */
+#define AD9832_CHAN_FREQ(_channel, _addr) {			\
+	.type = IIO_ALTVOLTAGE,					\
+	.output = 1,						\
+	.indexed = 1,						\
+	.channel = _channel,					\
+	.address = _addr,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_FREQUENCY),	\
+}
 
-static IIO_DEV_ATTR_FREQ(0, 0, 0200, NULL, ad9832_write, AD9832_FREQ0HM);
-static IIO_DEV_ATTR_FREQ(0, 1, 0200, NULL, ad9832_write, AD9832_FREQ1HM);
-static IIO_DEV_ATTR_FREQSYMBOL(0, 0200, NULL, ad9832_write, AD9832_FREQ_SYM);
-static IIO_CONST_ATTR_FREQ_SCALE(0, "1"); /* 1Hz */
+#define AD9832_CHAN_PHASE(_channel, _addr) {			\
+	.type = IIO_ALTVOLTAGE,					\
+	.output = 1,						\
+	.indexed = 1,						\
+	.channel = _channel,					\
+	.address = _addr,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_PHASE),		\
+}
+
+static const struct iio_chan_spec ad9832_channels[] = {
+	AD9832_CHAN_FREQ(0, AD9832_FREQ0HM),
+	AD9832_CHAN_FREQ(1, AD9832_FREQ1HM),
+	AD9832_CHAN_PHASE(2, AD9832_PHASE0H),
+	AD9832_CHAN_PHASE(3, AD9832_PHASE1H),
+	AD9832_CHAN_PHASE(4, AD9832_PHASE2H),
+	AD9832_CHAN_PHASE(5, AD9832_PHASE3H),
+};
 
-static IIO_DEV_ATTR_PHASE(0, 0, 0200, NULL, ad9832_write, AD9832_PHASE0H);
-static IIO_DEV_ATTR_PHASE(0, 1, 0200, NULL, ad9832_write, AD9832_PHASE1H);
-static IIO_DEV_ATTR_PHASE(0, 2, 0200, NULL, ad9832_write, AD9832_PHASE2H);
-static IIO_DEV_ATTR_PHASE(0, 3, 0200, NULL, ad9832_write, AD9832_PHASE3H);
-static IIO_DEV_ATTR_PHASESYMBOL(0, 0200, NULL,
-				ad9832_write, AD9832_PHASE_SYM);
-static IIO_CONST_ATTR_PHASE_SCALE(0, "0.0015339808"); /* 2PI/2^12 rad*/
+static IIO_CONST_ATTR(out_altvoltage_frequency_scale, "1");	   /* 1Hz */
+static IIO_CONST_ATTR(out_altvoltage_phase_scale, "0.0015339808"); /* 2PI / 2^12 rad */
 
-static IIO_DEV_ATTR_PINCONTROL_EN(0, 0200, NULL,
-				ad9832_write, AD9832_PINCTRL_EN);
-static IIO_DEV_ATTR_OUT_ENABLE(0, 0200, NULL,
-				ad9832_write, AD9832_OUTPUT_EN);
+static IIO_DEVICE_ATTR(out_altvoltage_frequencysymbol, 0200, NULL,
+		       ad9832_store, AD9832_FREQ_SYM);
+static IIO_DEVICE_ATTR(out_altvoltage_phasesymbol, 0200, NULL,
+		       ad9832_store, AD9832_PHASE_SYM);
+static IIO_DEVICE_ATTR(out_altvoltage_out_enable, 0200, NULL,
+		       ad9832_store, AD9832_OUTPUT_EN);
+static IIO_DEVICE_ATTR(out_altvoltage_pincontrol_en, 0200, NULL,
+		       ad9832_store, AD9832_PINCTRL_EN);
 
 static struct attribute *ad9832_attributes[] = {
-	&iio_dev_attr_out_altvoltage0_frequency0.dev_attr.attr,
-	&iio_dev_attr_out_altvoltage0_frequency1.dev_attr.attr,
-	&iio_const_attr_out_altvoltage0_frequency_scale.dev_attr.attr,
-	&iio_dev_attr_out_altvoltage0_phase0.dev_attr.attr,
-	&iio_dev_attr_out_altvoltage0_phase1.dev_attr.attr,
-	&iio_dev_attr_out_altvoltage0_phase2.dev_attr.attr,
-	&iio_dev_attr_out_altvoltage0_phase3.dev_attr.attr,
-	&iio_const_attr_out_altvoltage0_phase_scale.dev_attr.attr,
-	&iio_dev_attr_out_altvoltage0_pincontrol_en.dev_attr.attr,
-	&iio_dev_attr_out_altvoltage0_frequencysymbol.dev_attr.attr,
-	&iio_dev_attr_out_altvoltage0_phasesymbol.dev_attr.attr,
-	&iio_dev_attr_out_altvoltage0_out_enable.dev_attr.attr,
+	&iio_const_attr_out_altvoltage_frequency_scale.dev_attr.attr,
+	&iio_const_attr_out_altvoltage_phase_scale.dev_attr.attr,
+	&iio_dev_attr_out_altvoltage_frequencysymbol.dev_attr.attr,
+	&iio_dev_attr_out_altvoltage_phasesymbol.dev_attr.attr,
+	&iio_dev_attr_out_altvoltage_out_enable.dev_attr.attr,
+	&iio_dev_attr_out_altvoltage_pincontrol_en.dev_attr.attr,
 	NULL,
 };
 
@@ -292,6 +393,8 @@ static const struct attribute_group ad9832_attribute_group = {
 };
 
 static const struct iio_info ad9832_info = {
+	.write_raw = ad9832_write_raw,
+	.read_raw = ad9832_read_raw,
 	.attrs = &ad9832_attribute_group,
 };
 
@@ -309,15 +412,15 @@ static int ad9832_probe(struct spi_device *spi)
 
 	ret = devm_regulator_get_enable(&spi->dev, "avdd");
 	if (ret)
-		return dev_err_probe(&spi->dev, ret, "failed to enable specified AVDD voltage\n");
+		return dev_err_probe(&spi->dev, ret, "failed to enable AVDD supply\n");
 
 	ret = devm_regulator_get_enable(&spi->dev, "dvdd");
 	if (ret)
-		return dev_err_probe(&spi->dev, ret, "Failed to enable specified DVDD supply\n");
+		return dev_err_probe(&spi->dev, ret, "failed to enable DVDD supply\n");
 
 	st->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
 	if (IS_ERR(st->mclk))
-		return PTR_ERR(st->mclk);
+		return dev_err_probe(&spi->dev, PTR_ERR(st->mclk), "failed to enable MCLK\n");
 
 	st->spi = spi;
 	mutex_init(&st->lock);
@@ -325,9 +428,10 @@ static int ad9832_probe(struct spi_device *spi)
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->info = &ad9832_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = ad9832_channels;
+	indio_dev->num_channels = ARRAY_SIZE(ad9832_channels);
 
 	/* Setup default messages */
-
 	st->xfer.tx_buf = &st->data;
 	st->xfer.len = 2;
 
-- 
2.43.0

Re: [RFC PATCH 2/3] staging: iio: ad9832: convert to iio channels

[Jonathan Cameron]

On Fri,  5 Dec 2025 17:27:42 -0300
Tomas Borquez <tomasborquez13@gmail.com> wrote:

> Replace the custom frequency and phase sysfs attributes with IIO channels
> using read_raw()/write_raw() callbacks, as well as removing the dds.h
> header.
> 
> Changes:
> - Add iio_chan_spec definitions for 2 frequency and 4 phase channels.
> - Implement read_raw/write_raw for IIO_CHAN_INFO_FREQUENCY/PHASE.
> - Cache frequency and phase values in driver state for readback.
> - Remove dependency on dds.h macros for sysfs.
> - Use guard(mutex) for cleaner locking.
> - Add input validation and consistent error messages.
> 
Hi Tomas,

> NOTE: This changes the userspace ABI, see cover letter.
Given I responded there on ABI, I'll just ignore that aspect for this review.

Code is pretty clean, but there are a few things that belong in other
patches rather than being mixed in here.
For kernel code we are pretty strict on one patch, one type of change.

> Signed-off-by: Tomas Borquez <tomasborquez13@gmail.com>
> ---
>  drivers/staging/iio/frequency/ad9832.c | 232 ++++++++++++++++++-------
>  1 file changed, 168 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> index e2ad3e5a7a..79d26009d1 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -9,6 +9,7 @@
>  
>  #include <linux/bitfield.h>
>  #include <linux/bits.h>
> +#include <linux/cleanup.h>
>  #include <linux/clk.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
> @@ -23,10 +24,7 @@
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  
> -#include "dds.h"
> -
>  /* Registers */
> -
Trivial but don't make unrelated white space changes in a patch
doing anything else - they add noise and hurt reviewer speed.
Put them in a patch on their own.

>  #define AD9832_FREQ0LL		0x0
>  #define AD9832_FREQ0HL		0x1
>  #define AD9832_FREQ0LM		0x2
> @@ -50,7 +48,6 @@
>  #define AD9832_OUTPUT_EN	0x13
>  
>  /* Command Control Bits */
> -

>  	} __aligned(IIO_DMA_MINALIGN);
>  };
>  
> -static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
> +static unsigned long ad9832_calc_freqreg(unsigned long mclk, u32 fout)
>  {
>  	unsigned long long freqreg = (u64)fout *
>  				     (u64)((u64)1L << AD9832_FREQ_BITS);
> @@ -124,12 +124,24 @@ static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
>  }
>  
>  static int ad9832_write_frequency(struct ad9832_state *st,
> -				  unsigned int addr, unsigned long fout)
> +				  unsigned int addr, u32 fout)
>  {
>  	unsigned long clk_freq;
>  	unsigned long regval;
>  	u8 regval_bytes[4];
>  	u16 freq_cmd;
> +	int ret, idx;
> +
> +	switch (addr) {
> +	case AD9832_FREQ0HM:
> +		idx = 0;
> +		break;
> +	case AD9832_FREQ1HM:
> +		idx = 1;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
>  
>  	clk_freq = clk_get_rate(st->mclk);
>  
> @@ -147,14 +159,37 @@ static int ad9832_write_frequency(struct ad9832_state *st,
>  			FIELD_PREP(AD9832_DAT_MSK, regval_bytes[i]));
>  	}
>  
> -	return spi_sync(st->spi, &st->freq_msg);
> +	ret = spi_sync(st->spi, &st->freq_msg);
> +	if (ret)
> +		return ret;
> +
> +	st->freq[idx] = fout;

I'd put a blank line here.  Generally slightly helps readability before
simple return statements like this.

> +	return 0;
>  }
>  
>  static int ad9832_write_phase(struct ad9832_state *st,
> -			      unsigned long addr, unsigned long phase)
> +			      unsigned long addr, u32 phase)
>  {
>  	u8 phase_bytes[2];
>  	u16 phase_cmd;
> +	int ret, idx;
> +
> +	switch (addr) {
> +	case AD9832_PHASE0H:
> +		idx = 0;
> +		break;
> +	case AD9832_PHASE1H:
> +		idx = 1;
> +		break;
> +	case AD9832_PHASE2H:
> +		idx = 2;
> +		break;
> +	case AD9832_PHASE3H:
> +		idx = 3;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
>  
>  	if (phase >= BIT(AD9832_PHASE_BITS))
>  		return -EINVAL;
> @@ -169,10 +204,77 @@ static int ad9832_write_phase(struct ad9832_state *st,
>  			FIELD_PREP(AD9832_DAT_MSK, phase_bytes[i]));
>  	}
>  
> -	return spi_sync(st->spi, &st->phase_msg);
> +	ret = spi_sync(st->spi, &st->phase_msg);
> +	if (ret)
> +		return ret;
> +
> +	st->phase[idx] = phase;
> +	return 0;
>  }
>  
> -static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
> +static int ad9832_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
> +{
> +	struct ad9832_state *st = iio_priv(indio_dev);
> +
> +	if (val < 0)
> +		return -EINVAL;

Check val2 as well.  Should be zero.


> +
> +	guard(mutex)(&st->lock);
> +	switch (mask) {
> +	case IIO_CHAN_INFO_FREQUENCY:
> +		return ad9832_write_frequency(st, chan->address, val);
> +	case IIO_CHAN_INFO_PHASE:
> +		return ad9832_write_phase(st, chan->address, val);
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +
> +static ssize_t ad9832_store(struct device *dev,
> +			    struct device_attribute *attr,
>  			    const char *buf, size_t len)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> @@ -183,20 +285,10 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
>  
>  	ret = kstrtoul(buf, 10, &val);
>  	if (ret)
> -		goto error_ret;
> +		return ret;
>  
> -	mutex_lock(&st->lock);
> -	switch ((u32)this_attr->address) {
> -	case AD9832_FREQ0HM:
> -	case AD9832_FREQ1HM:
> -		ret = ad9832_write_frequency(st, this_attr->address, val);
> -		break;
> -	case AD9832_PHASE0H:
> -	case AD9832_PHASE1H:
> -	case AD9832_PHASE2H:
> -	case AD9832_PHASE3H:
> -		ret = ad9832_write_phase(st, this_attr->address, val);
> -		break;
> +	guard(mutex)(&st->lock);

Ideally do guard() changes in a separate patch as seems unrelated to the
other stuff going on here. I'd suggest making that change first.

> +	switch (this_attr->address) {
>  	case AD9832_PINCTRL_EN:
>  		st->ctrl_ss &= ~AD9832_SELSRC;
>  		st->ctrl_ss |= FIELD_PREP(AD9832_SELSRC, val ? 0 : 1);
> @@ -206,13 +298,13 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
>  		ret = spi_sync(st->spi, &st->msg);
>  		break;
>  	case AD9832_FREQ_SYM:
> -		if (val == 1 || val == 0) {
> -			st->ctrl_fp &= ~AD9832_FREQ;
> -			st->ctrl_fp |= FIELD_PREP(AD9832_FREQ, val ? 1 : 0);
> -		} else {
> +		if (val != 1 && val != 0) {
>  			ret = -EINVAL;

With guard, should be able to directly return in error cases
simplifying the flow and helping code readability.  That's one of the
nicest things guard() enables.

>  			break;
>  		}
> +
> +		st->ctrl_fp &= ~AD9832_FREQ;
> +		st->ctrl_fp |= FIELD_PREP(AD9832_FREQ, val);
>  		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_FPSELECT) |
>  						  st->ctrl_fp);
>  		ret = spi_sync(st->spi, &st->msg);
> @@ -243,47 +335,56 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
>  	default:
>  		ret = -ENODEV;
>  	}
> -	mutex_unlock(&st->lock);
>  
> -error_ret:
>  	return ret ? ret : len;
>  }

>  
> @@ -309,15 +412,15 @@ static int ad9832_probe(struct spi_device *spi)
>  
>  	ret = devm_regulator_get_enable(&spi->dev, "avdd");
>  	if (ret)
> -		return dev_err_probe(&spi->dev, ret, "failed to enable specified AVDD voltage\n");
> +		return dev_err_probe(&spi->dev, ret, "failed to enable AVDD supply\n");
>  
>  	ret = devm_regulator_get_enable(&spi->dev, "dvdd");
>  	if (ret)
> -		return dev_err_probe(&spi->dev, ret, "Failed to enable specified DVDD supply\n");
> +		return dev_err_probe(&spi->dev, ret, "failed to enable DVDD supply\n");
>  
>  	st->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
>  	if (IS_ERR(st->mclk))
> -		return PTR_ERR(st->mclk);
> +		return dev_err_probe(&spi->dev, PTR_ERR(st->mclk), "failed to enable MCLK\n");
This is an unrelated change.  Do it in a separate patch for just the dev_err_probe()
usage.

>  

[RFC PATCH 3/3] dt-bindings: iio: add analog devices ad9832/ad9835

[Tomas Borquez]

Add devicetree binding documentation for the AD9832 and AD9835
Digital Synthesizer chips.

Signed-off-by: Tomas Borquez <tomasborquez13@gmail.com>
---
 .../bindings/iio/frequency/adi,ad9832.yaml    | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,ad9832.yaml

diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,ad9832.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,ad9832.yaml
new file mode 100644
index 0000000000..f14e054ab2
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/frequency/adi,ad9832.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/frequency/adi,ad9832.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD9832/AD9835 Direct Digital Synthesizer
+
+maintainers:
+  - Michael Hennerich <michael.hennerich@analog.com>
+
+properties:
+  compatible:
+    enum:
+      - adi,ad9832
+      - adi,ad9835
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 20000000
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: mclk
+
+  avdd-supply:
+    description: Analog power supply.
+
+  dvdd-supply:
+    description: Digital power supply.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - avdd-supply
+  - dvdd-supply
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        dds@0 {
+            compatible = "adi,ad9832";
+            reg = <0>;
+            spi-max-frequency = <20000000>;
+            clocks = <&dds_clk>;
+            clock-names = "mclk";
+            avdd-supply = <&avdd_reg>;
+            dvdd-supply = <&dvdd_reg>;
+        };
+    };
+...
-- 
2.43.0

Re: [RFC PATCH 3/3] dt-bindings: iio: add analog devices ad9832/ad9835

[Jonathan Cameron]

On Fri,  5 Dec 2025 17:27:43 -0300
Tomas Borquez <tomasborquez13@gmail.com> wrote:

> Add devicetree binding documentation for the AD9832 and AD9835
> Digital Synthesizer chips.
> 
> Signed-off-by: Tomas Borquez <tomasborquez13@gmail.com>

Normally we only add a binding as part of the staging graduation
which typically involves a full review of the whole driver (make sure
to disable move detection in git if you send the code moving patch).

Anyhow, we can still give early feedback on this!

Whilst checking the pin mappings I finally noticed this a current source DAC
not a voltage one so all the channel types should be out_altcurrent0_...

Michael, I don't suppose you happen to remember why it is pretending to
be a voltage DAC?  

Jonathan

> ---
>  .../bindings/iio/frequency/adi,ad9832.yaml    | 65 +++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,ad9832.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,ad9832.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,ad9832.yaml
> new file mode 100644
> index 0000000000..f14e054ab2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/frequency/adi,ad9832.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/frequency/adi,ad9832.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD9832/AD9835 Direct Digital Synthesizer
> +
> +maintainers:
> +  - Michael Hennerich <michael.hennerich@analog.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad9832
> +      - adi,ad9835
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 20000000
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: mclk
> +
> +  avdd-supply:
> +    description: Analog power supply.
> +
> +  dvdd-supply:
> +    description: Digital power supply.
There looks to be a REFIN pin (and FS Adjust which is a bit of an oddity)
We probably need to desribe any resistor connected to fs adjust a bit like
a shunt resistor.


See cover letter discussion for the ways the various
inputs could be wired.  Some of the recent discussion on tied
GPIOs is also relevant here. I'm not up to date with where that
ended up yet though!

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - avdd-supply
> +  - dvdd-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        dds@0 {
> +            compatible = "adi,ad9832";
> +            reg = <0>;
> +            spi-max-frequency = <20000000>;
> +            clocks = <&dds_clk>;
> +            clock-names = "mclk";
> +            avdd-supply = <&avdd_reg>;
> +            dvdd-supply = <&dvdd_reg>;
> +        };
> +    };
> +...

Re: [RFC PATCH 0/3] ad9832: driver cleanup

[Jonathan Cameron]

On Fri,  5 Dec 2025 17:27:40 -0300
Tomas Borquez <tomasborquez13@gmail.com> wrote:

> This series is a general cleanup of ad9832, with the purpose of 
> graduating it from staging. The main changes are removing legacy 
> platform_data support, converting to IIO channels with read/write_raw 
> callbacks, and adding devicetree support.

Opening question for a cleanup of a driver like this is how you plan
to test it. Do you have the hardware, or are you emulating / stubbing
functions to test it? It is very brave to take on major refactoring
without a good way to test.

I was kind of planning to drop this driver this cycle on basis of no
interest in sorting it out, but clearly you are interested so great
as long as we can be sure it works well after your work on it
(or indeed that it works currently!)

> 
> I'm sending this as an RFC because I have some concerns about the ABI 
> design and would appreciate guidance before putting more time into this.

Very sensible!
 
> 
> Patch 1 removes the legacy platform_data support as suggested by
> Jonathan [1]. The driver now initializes to a safe state and lets
> userspace configure frequencies/phases via sysfs.
> 
> Patch 2 converts frequency and phase configuration from custom sysfs
> attributes to proper IIO channels using read_raw/write_raw callbacks
> (This is the main area where I'd like feedback).
> 
> Patch 3 adds devicetree bindings documentation.
> 
> Design Concerns:
> 1) Channel Organization and ABI Break
> 
>    The device has 2 frequency registers and 4 phase registers. Since both
>    frequency and phase must use IIO_ALTVOLTAGE since there's no better fit
>    (as far as I know), I've organized channels as:
> 
>      out_altvoltage0_frequency  (FREQ0)
>      out_altvoltage1_frequency  (FREQ1)
>      out_altvoltage2_phase      (PHASE0)
>      out_altvoltage3_phase      (PHASE1)
>      out_altvoltage4_phase      (PHASE2)
>      out_altvoltage5_phase      (PHASE3)
> 
>    The old ABI used out_altvoltage0_frequency0, out_altvoltage0_frequency1,
>    out_altvoltage0_phase0, etc. 
> 
>    The new approach felt cleaner but I'm open to alternatives and better 
>    ways of mapping them. Is this channel mapping reasonable, or would a 
>    different organization be preferred? And is the ABI break okay?

When fixing up a non standard ABI, ABI breakage is fine, but...

This device only has one output, so there is only one channel.  We are controlling
aspects of that channel.  Hence it should not be split across multiple indexed
channels like you outline. What the number in the attributes indicates here is
the input symbol for phase or frequency modulation.  The applications information
section of the datasheet talks about using this part ofr FSK, GMSK (I had to google that
one ;), QPSK etc.

Oddly I had it in my head that we had a standardised interface for PSK / FSK
parameter control but I guess that discussion was probably for a driver that
never merged.  There is similar stuff for DACs though - see
out_currentY_symbol
out_currentY_rawN 
in sysfs-bus-iio-dac
In those particular cases the thing being switched is of the channel type rather
than modulation on top of that.  But similar approach applies.
Note the symbol control may not be present if the control pins are wired
not to GPIOs but to external circuitry.

So more or less the currently ABI is the way to go, not the one you suggest.


> 2) Scale Attributes
> 
>    The frequency scale is 1 Hz and phase scale is 2*PI/4096 radians.
>    I cannot use info_mask_shared_by_type for IIO_CHAN_INFO_SCALE because
>    all channels share IIO_ALTVOLTAGE.
> 
>    So instead I'm using IIO_CONST_ATTR for the scales:
> 
>      out_altvoltage_frequency_scale = "1"
>      out_altvoltage_phase_scale = "0.0015339808"
> 
>    Is there a better approach here? Or should I just document the units and
>    skip scale attributes entirely?

Good question.  I think right option is to just do the maths in the driver and
have out_altvoltage0_frequencyN take the scaled value rather than the register
value.  Then do some fixed point maths to get to the required register value.

> 
> 3) Remaining Custom Attributes
> 
>    Other controls remain as custom sysfs attributes:
> 
>      - out_altvoltage_frequencysymbol: select active frequency register
>      - out_altvoltage_phasesymbol: select active phase register  

For those two the ADC symbol example above should generalize but we'll need
to extend the ABI to
	out_alvoltage0_phase_symbol / frequency_symbol I think.

>      - out_altvoltage_pincontrol_en: hardware pin control enable

I'm not sure this is something that userspace should control at all.
To me it seems most likely to be wiring question.
1) those pins are ground tied, and we are doing software control -
   corresponds to no DT description of the pins.
2) those pins are couple to GPIOs on the SoC.  Maybe we prefer those over
   software because expectation is that they are quicker to set.
3) Wired to something unknown - expectation is always use those pins
   as we can't meet the documented recommendation to tie them to zero
   when using software control.

The tricky corner is we need firmware to tell us if they are 0 tied
vs wired to something outside our control.  This comes back to a recent
discussion about fake GPIOs that just allow you to read the state but
not set it (that is probably still going on but I'm reading my rather long
inbox backwards so haven't gotten to it yet).   If that isn't resolved
well need a firmware property to distinguish 1 and 3.


>      - out_altvoltage_out_enable: output enable

out_altvoltage0_enable should be fine.  That's standard enough for DAC channels
and this is kind of a fancy DAC.

>    I'm not sure if these map cleanly to IIO interfaces. Should these be
>    documented in ABI or is there a preferred way to handle them?

So a few additions that need documenting but mostly aligns with standard
ABI.

> 
> 4) Implementation Notes
> 
>    - read_raw uses explicit address switching rather than channel index
>      arithmetic for clarity, though phase values could alternatively be
>      accessed via st->phase[chan->channel - 2] and directly in freq with
>      st->freq[chan->channel].
>    - I'm unsure if mutex guards on cached reads are necessary.
> 
> Link: https://lore.kernel.org/linux-iio/20250628161040.3d21e2c4@jic23-huawei/ [1]
> Signed-off-by: Tomas Borquez <tomasborquez13@gmail.com>
> 
> Tomas Borquez (3):
>   staging: iio: ad9832: remove platform_data support
>   staging: iio: ad9832: convert to iio channels
>   dt-bindings: iio: add analog devices ad9832/ad9835
> 
>  .../bindings/iio/frequency/adi,ad9832.yaml    |  65 +++++
>  drivers/staging/iio/frequency/ad9832.c        | 264 +++++++++++-------
>  drivers/staging/iio/frequency/ad9832.h        |  33 ---
>  3 files changed, 233 insertions(+), 129 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,ad9832.yaml
>  delete mode 100644 drivers/staging/iio/frequency/ad9832.h
>