Skip to contentSkip to conversation

staging: ad9832: driver cleanup

PATCH In review Tomas Borquez 5 participants 25 messages Dec 30, 2025 – Jan 14

Subsystems (4)
Staging IIO Analog Devices IIO drivers Staging - industrial io
lore
Share & tools
lore

Tomas Borquez

author
#0 lore raw

v2 cover staging: ad9832: driver cleanup

This series cleans up the ad9832 driver with the goal of (eventually) graduating it from staging. The main change is converting custom sysfs attributes to IIO channel interface and adding sysfs documentation.

Changes since v1:
- Reordered patches: moved dev_err_probe cleanup before guard(mutex) conversion (Marcelo)
- Split channel conversion into two patches:
- dds.h removal
- channel interface conversion (ABI change) (Jonathan)
- Added devm_mutex_init() conversion as separate patch (Marcelo)
- Used MICRO macro for phase calculations instead of hardcoded values for better readability (Marcelo)
- Fixed FIELD_PREP usage in pinctrl_en for consistency (Marcelo)
- Renamed ext_info macro parameter from _channel to _select since it represents register selection not channel number (Marcelo)
- Ensured patches apply cleanly on top of IIO testing branch
- Made sysfs ABI documentation generic using capital letters for channel and symbol indices (Marcelo, Jonathan)
- Changed attribute naming to hierarchical structure (out_altcurrentY_frequency_symbolZ) (Marcelo)
- Made descriptions hardware-agnostic and removed chip-specific implementation details (Jonathan)
- Renamed documentation file to sysfs-bus-iio-frequency instead of chip-specific name (Jonathan)

Tomas Borquez (6):
staging: iio: ad9832: cleanup dev_err_probe()
staging: iio: ad9832: convert to guard(mutex)
staging: iio: ad9832: convert to devm_mutex_init()
staging: iio: ad9832: remove dds.h dependency
staging: iio: ad9832: convert to iio channels and ext_info attrs
staging: iio: ad9832: add sysfs documentation

 .../iio/Documentation/sysfs-bus-iio-frequency |  40 +++
 drivers/staging/iio/frequency/ad9832.c        | 327 +++++++++++++-----
 2 files changed, 279 insertions(+), 88 deletions(-)
 create mode 100644 drivers/staging/iio/Documentation/sysfs-bus-iio-frequency
Signature · 1 line
2.43.0
author
#1 lore raw

PATCH v2 1/6 staging: iio: ad9832: cleanup dev_err_probe()

Cleanup dev_err_probe() by keeping messages consistent and adding error message for clock acquisition failure.

Signed-off-by Tomas Borquez <[email protected]>
---
 drivers/staging/iio/frequency/ad9832.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
drivers/staging/iio/frequency/ad9832.c +8 -7
index b87ea1781b27..a5523e2210bf 100644
@@ -293,27 +293,28 @@ static const struct iio_info ad9832_info = {
 
 static int ad9832_probe(struct spi_device *spi)
 {
+	struct device *dev = &spi->dev;
 	struct iio_dev *indio_dev;
 	struct ad9832_state *st;
 	int ret;
 
-	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (!indio_dev)
 		return -ENOMEM;
 
 	st = iio_priv(indio_dev);
 
-	ret = devm_regulator_get_enable(&spi->dev, "avdd");
+	ret = devm_regulator_get_enable(dev, "avdd");
 	if (ret)
-		return dev_err_probe(&spi->dev, ret, "failed to enable specified AVDD voltage\n");
+		return dev_err_probe(dev, ret, "failed to enable AVDD supply\n");
 
-	ret = devm_regulator_get_enable(&spi->dev, "dvdd");
+	ret = devm_regulator_get_enable(dev, "dvdd");
 	if (ret)
-		return dev_err_probe(&spi->dev, ret, "Failed to enable specified DVDD supply\n");
+		return dev_err_probe(dev, ret, "failed to enable DVDD supply\n");
 
-	st->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
+	st->mclk = devm_clk_get_enabled(dev, "mclk");
 	if (IS_ERR(st->mclk))
-		return PTR_ERR(st->mclk);
+		return dev_err_probe(dev, PTR_ERR(st->mclk), "failed to enable MCLK\n");
 
 	st->spi = spi;
 	mutex_init(&st->lock);
Signature · 1 line
2.43.0
#2 lore raw

Re: [PATCH v2 1/6] staging: iio: ad9832: cleanup dev_err_probe()

Quoted 3 lines · Tomas Borquez

Cleanup dev_err_probe() by keeping messages consistent and adding error message for clock acquisition failure.

AFAICS this does two things, the second one is reuse of the temporary variable for struct device.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#3 lore raw

Re: [PATCH v2 1/6] staging: iio: ad9832: cleanup dev_err_probe()

Quoted 8 lines · On Wed, 31 Dec 2025 00:47:43 +0200 Andy Shevchenko <[email protected]>…
Quoted 3 lines · Tomas Borquez

Cleanup dev_err_probe() by keeping messages consistent and adding error message for clock acquisition failure.

AFAICS this does two things, the second one is reuse of the temporary variable for struct device.

Agreed. Ideal patch break up would be to not just mention them but do the struct device *dev first, then the dev_err_probe() change.

author
#4 lore raw

PATCH v2 2/6 staging: iio: ad9832: convert to guard(mutex)

In reply to Tomas Borquez

Use guard(mutex) for cleaner lock handling and simpler error paths.

Signed-off-by Tomas Borquez <[email protected]>
---
 drivers/staging/iio/frequency/ad9832.c | 41 +++++++++++---------------
 1 file changed, 17 insertions(+), 24 deletions(-)
drivers/staging/iio/frequency/ad9832.c +17 -24
index a5523e2210bf..35d8d51d5c2b 100644
@@ -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>

@@ -179,20 +180,20 @@ 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);
+	guard(mutex)(&st->lock);
 	switch ((u32)this_attr->address) {
 	case AD9832_FREQ0HM:
 	case AD9832_FREQ1HM:
 		ret = ad9832_write_frequency(st, this_attr->address, val);
-		break;
+		return ret ?: len;
 	case AD9832_PHASE0H:
 	case AD9832_PHASE1H:
 	case AD9832_PHASE2H:
 	case AD9832_PHASE3H:
 		ret = ad9832_write_phase(st, this_attr->address, val);
-		break;
+		return ret ?: len;
 	case AD9832_PINCTRL_EN:
 		st->ctrl_ss &= ~AD9832_SELSRC;
 		st->ctrl_ss |= FIELD_PREP(AD9832_SELSRC, val ? 0 : 1);

@@ -200,24 +201,20 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_SYNCSELSRC) |
 						  st->ctrl_ss);
 		ret = spi_sync(st->spi, &st->msg);
-		break;
+		return ret ?: len;
 	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 {
-			ret = -EINVAL;
-			break;
-		}
+		if (val != 1 && val != 0)
+			return -EINVAL;
+
+		st->ctrl_fp &= ~AD9832_FREQ;
+		st->ctrl_fp |= FIELD_PREP(AD9832_FREQ, val ? 1 : 0);
 		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_FPSELECT) |
 						  st->ctrl_fp);
 		ret = spi_sync(st->spi, &st->msg);
-		break;
+		return ret ?: len;
 	case AD9832_PHASE_SYM:
-		if (val > 3) {
-			ret = -EINVAL;
-			break;
-		}
+		if (val > 3)
+			return -EINVAL;
 
 		st->ctrl_fp &= ~AD9832_PHASE_MASK;
 		st->ctrl_fp |= FIELD_PREP(AD9832_PHASE_MASK, val);

@@ -225,7 +222,7 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_FPSELECT) |
 						  st->ctrl_fp);
 		ret = spi_sync(st->spi, &st->msg);
-		break;
+		return ret ?: len;
 	case AD9832_OUTPUT_EN:
 		if (val)
 			st->ctrl_src &= ~(AD9832_RESET | AD9832_SLEEP | AD9832_CLR);

@@ -235,14 +232,10 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_SLEEPRESCLR) |
 						  st->ctrl_src);
 		ret = spi_sync(st->spi, &st->msg);
-		break;
+		return ret ?: len;
 	default:
-		ret = -ENODEV;
+		return -ENODEV;
 	}
-	mutex_unlock(&st->lock);
-
-error_ret:
-	return ret ? ret : len;
 }
 
 /*
Signature · 1 line
2.43.0
#5 lore raw

Re: [PATCH v2 2/6] staging: iio: ad9832: convert to guard(mutex)

Quoted 2 lines · Tomas Borquez

Use guard(mutex) for cleaner lock handling and simpler error paths.

...

Don't remember if it was in the previous rounds of review or somewhere else, but Jonathan (? IIRC) suggestd to use

  ret = foo(...);
  if (ret)
   return ret;
  break;
  ...
  return len;

However, this one just duplicates what is already in use. Have you checked the bloat-o-meter before and after and see if there is any difference in the compiled object file?

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#6 lore raw

Re: [PATCH v2 2/6] staging: iio: ad9832: convert to guard(mutex)

Quoted 9 lines · Andy Shevchenko
Quoted 2 lines · Tomas Borquez

Use guard(mutex) for cleaner lock handling and simpler error paths.

...

Don't remember if it was in the previous rounds of review or somewhere else, but Jonathan (? IIRC) suggestd to use

Quoted 6 lines
  ret = foo(...);
  if (ret)
   return ret;
  break;
  ...
  return len;

You are right, I should have sticked to that one since anyways I ended up using that pattern on patch 5.

Quoted 3 lines

However, this one just duplicates what is already in use. Have you checked the bloat-o-meter before and after and see if there is any difference in the compiled object file?

Jonathan's is better by 7 delta, so I'll stick with his

Quoted 3 lines
Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#7 lore raw

PATCH v2 3/6 staging: iio: ad9832: convert to devm_mutex_init()

In reply to Tomas Borquez

Use devm_mutex_init() for automatic resource cleanup when the device is removed or probe fails.

Signed-off-by Tomas Borquez <[email protected]>
---
 drivers/staging/iio/frequency/ad9832.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/iio/frequency/ad9832.c +3 -1
index 35d8d51d5c2b..4bb203a67046 100644
@@ -310,7 +310,9 @@ static int ad9832_probe(struct spi_device *spi)
 		return dev_err_probe(dev, PTR_ERR(st->mclk), "failed to enable MCLK\n");
 
 	st->spi = spi;
-	mutex_init(&st->lock);
+	ret = devm_mutex_init(&spi->dev, &st->lock);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to initialize mutex\n");
 
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->info = &ad9832_info;
Signature · 1 line
2.43.0
Reviewed-by on v2 3/6
#8 lore raw

Re: [PATCH v2 3/6] staging: iio: ad9832: convert to devm_mutex_init()

Quoted 13 lines · Tomas Borquez

Use devm_mutex_init() for automatic resource cleanup when the device is removed or probe fails.

Signed-off-by Tomas Borquez <[email protected]>
---
 drivers/staging/iio/frequency/ad9832.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/iio/frequency/ad9832.c +0 -0
index 35d8d51d5c2b..4bb203a67046 100644

drivers/staging/iio/frequency/ad9832.c static int ad9832_probe(struct spi_device *spi) · line 313

Quoted 5 lines
drivers/staging/iio/frequency/ad9832.c +1 -1
@@ -310,7 +310,9 @@ static int ad9832_probe(struct spi_device *spi)
 		return dev_err_probe(dev, PTR_ERR(st->mclk), "failed to enable MCLK\n");
 
 	st->spi = spi;
-	mutex_init(&st->lock);
+	ret = devm_mutex_init(&spi->dev, &st->lock);

Can use just 'dev' instead of '&spi->dev' after the update from patch 1.

With that

Reviewed-by Marcelo Schmitt <[email protected]>
Quoted 9 lines
drivers/staging/iio/frequency/ad9832.c +2 -0
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to initialize mutex\n");
 
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->info = &ad9832_info;
Signature · 1 line
2.43.0
author
#9 lore raw

PATCH v2 4/6 staging: iio: ad9832: remove dds.h dependency

In reply to Tomas Borquez

Remove dependency on dds.h by converting custom macros to standard IIO attribute declarations.

Signed-off-by Tomas Borquez <[email protected]>
---
 drivers/staging/iio/frequency/ad9832.c | 37 +++++++++++---------------
 1 file changed, 15 insertions(+), 22 deletions(-)
drivers/staging/iio/frequency/ad9832.c +15 -22
index 4bb203a67046..aa78973c3a3c 100644
@@ -24,8 +24,6 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#include "dds.h"
-
 /* Registers */
 #define AD9832_FREQ0LL		0x0
 #define AD9832_FREQ0HL		0x1

@@ -238,27 +236,22 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 	}
 }
 
-/*
- * see dds.h for further information
- */
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency0, 0200, NULL, ad9832_write, AD9832_FREQ0HM);
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency1, 0200, NULL, ad9832_write, AD9832_FREQ1HM);
+
+static IIO_DEVICE_ATTR(out_altvoltage0_frequencysymbol, 0200, NULL, ad9832_write, AD9832_FREQ_SYM);
+static IIO_CONST_ATTR(out_altvoltage0_frequency_scale, "1"); /* 1Hz */
+
+static IIO_DEVICE_ATTR(out_altvoltage0_phase0, 0200, NULL, ad9832_write, AD9832_PHASE0H);
+static IIO_DEVICE_ATTR(out_altvoltage0_phase1, 0200, NULL, ad9832_write, AD9832_PHASE1H);
+static IIO_DEVICE_ATTR(out_altvoltage0_phase2, 0200, NULL, ad9832_write, AD9832_PHASE2H);
+static IIO_DEVICE_ATTR(out_altvoltage0_phase3, 0200, NULL, ad9832_write, AD9832_PHASE3H);
+
+static IIO_DEVICE_ATTR(out_altvoltage0_phasesymbol, 0200, NULL, ad9832_write, AD9832_PHASE_SYM);
+static IIO_CONST_ATTR(out_altvoltage0_phase_scale, "0.0015339808"); /* 2PI/2^12 rad */
 
-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 */
-
-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_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_altvoltage0_pincontrol_en, 0200, NULL, ad9832_write, AD9832_PINCTRL_EN);
+static IIO_DEVICE_ATTR(out_altvoltage0_out_enable, 0200, NULL, ad9832_write, AD9832_OUTPUT_EN);
 
 static struct attribute *ad9832_attributes[] = {
 	&iio_dev_attr_out_altvoltage0_frequency0.dev_attr.attr,
Signature · 1 line
2.43.0
#10 lore raw

Re: [PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 240

Quoted 7 lines · Tomas Borquez

Remove dependency on dds.h by converting custom macros to standard IIO attribute declarations.

drivers/staging/iio/frequency/ad9832.c +2 -0
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency0, 0200, NULL, ad9832_write, AD9832_FREQ0HM);
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency1, 0200, NULL, ad9832_write, AD9832_FREQ1HM);

Any particular point in not using _WO() / _RO() variants of the IIO_DEVICE_ATTR_*() macros?

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#11 lore raw

Re: [PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency

Quoted 11 lines · Andy Shevchenko

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 240

Quoted 7 lines · Tomas Borquez

Remove dependency on dds.h by converting custom macros to standard IIO attribute declarations.

drivers/staging/iio/frequency/ad9832.c +2 -0
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency0, 0200, NULL, ad9832_write, AD9832_FREQ0HM);
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency1, 0200, NULL, ad9832_write, AD9832_FREQ1HM);

Any particular point in not using _WO() / _RO() variants of the IIO_DEVICE_ATTR_*() macros?

I was looking into this and saw that the definition for both _WO() and _RO() only takes _name and _addr:

#define IIO_DEVICE_ATTR_WO(_name, _addr) \
struct iio_dev_attr iio_dev_attr_##_name = IIO_ATTR_WO(_name, _addr)

So if we use it for frequency0 for example, it assumes the store function since we don't pass it:

static IIO_DEVICE_ATTR_WO(out_altvoltage0_frequency0, AD9832_FREQ0HM);

// Expands to

struct iio_dev_attr iio_dev_attr_out_altvoltage0_frequency0 = {
    .dev_attr = {
        . attr = {
          ...
          .store = out_altvoltage0_frequency0_store,
        }
    }
}

Meaning we would have to create a store for each one instead of using just one write function

Quoted 3 lines
Signature · 2 lines
With Best Regards,
Andy Shevchenko
#12 lore raw

Re: [PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency

Quoted 2 lines · Tomas Borquez
Quoted 1 line · Andy Shevchenko

On Tue, Dec 30, 2025 at 10:35 PM Tomas Borquez <[email protected]> wrote:

...

Quoted 25 lines
Quoted 5 lines

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 240

Quoted 2 lines
drivers/staging/iio/frequency/ad9832.c +2 -0
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency0, 0200, NULL, ad9832_write, AD9832_FREQ0HM);
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency1, 0200, NULL, ad9832_write, AD9832_FREQ1HM);

Any particular point in not using _WO() / _RO() variants of the IIO_DEVICE_ATTR_*() macros?

I was looking into this and saw that the definition for both _WO() and _RO() only takes _name and _addr:

#define IIO_DEVICE_ATTR_WO(_name, _addr) \
struct iio_dev_attr iio_dev_attr_##_name = IIO_ATTR_WO(_name, _addr)

So if we use it for frequency0 for example, it assumes the store function since we don't pass it:

static IIO_DEVICE_ATTR_WO(out_altvoltage0_frequency0, AD9832_FREQ0HM);

// Expands to

struct iio_dev_attr iio_dev_attr_out_altvoltage0_frequency0 = {
    .dev_attr = {
        . attr = {
          ...
          .store = out_altvoltage0_frequency0_store,
        }
    }
}
Quoted 2 lines

Meaning we would have to create a store for each one instead of using just one write function

Yes, and it will be fine, no? Explicit is better than implicit (at least in this case).

Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#13 lore raw

Re: [PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency

In reply to Tomas Borquez
Quoted 4 lines · On Tue, 30 Dec 2025 17:34:57 -0300 Tomas Borquez <[email protected]>…

Remove dependency on dds.h by converting custom macros to standard IIO attribute declarations.

Signed-off-by Tomas Borquez <[email protected]>

Hi Tomas,

Happy new year (almost)

Quoted 19 lines
---
 drivers/staging/iio/frequency/ad9832.c | 37 +++++++++++---------------
 1 file changed, 15 insertions(+), 22 deletions(-)
drivers/staging/iio/frequency/ad9832.c +0 -2
index 4bb203a67046..aa78973c3a3c 100644
@@ -24,8 +24,6 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#include "dds.h"
-
 /* Registers */
 #define AD9832_FREQ0LL		0x0
 #define AD9832_FREQ0HL		0x1

@@ -238,27 +236,22 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 	}

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 243

Quoted 10 lines
drivers/staging/iio/frequency/ad9832.c +5 -3
 }
 
-/*
- * see dds.h for further information
- */
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency0, 0200, NULL, ad9832_write, AD9832_FREQ0HM);
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency1, 0200, NULL, ad9832_write, AD9832_FREQ1HM);
+
+static IIO_DEVICE_ATTR(out_altvoltage0_frequencysymbol, 0200, NULL, ad9832_write, AD9832_FREQ_SYM);
+static IIO_CONST_ATTR(out_altvoltage0_frequency_scale, "1"); /* 1Hz */

This seems like a pointless attribute. Default scaling for everything in IIO when attributes don't tell us otherwise is 1 so should be fine dropping this one.

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 251

Quoted 8 lines
drivers/staging/iio/frequency/ad9832.c +8 -0
+
+static IIO_DEVICE_ATTR(out_altvoltage0_phase0, 0200, NULL, ad9832_write, AD9832_PHASE0H);
+static IIO_DEVICE_ATTR(out_altvoltage0_phase1, 0200, NULL, ad9832_write, AD9832_PHASE1H);
+static IIO_DEVICE_ATTR(out_altvoltage0_phase2, 0200, NULL, ad9832_write, AD9832_PHASE2H);
+static IIO_DEVICE_ATTR(out_altvoltage0_phase3, 0200, NULL, ad9832_write, AD9832_PHASE3H);
+
+static IIO_DEVICE_ATTR(out_altvoltage0_phasesymbol, 0200, NULL, ad9832_write, AD9832_PHASE_SYM);
+static IIO_CONST_ATTR(out_altvoltage0_phase_scale, "0.0015339808"); /* 2PI/2^12 rad */

I can't immediately think of precedence for scaling of an attribute other than _raw. Whilst it's painful, this isn't a high perf path, so we should probably just do fixed point inputs for phase0,phase1 etc and deal with the scaling in the driver. That avoids adding new ABI for this very rare case.

Quoted 9 lines
drivers/staging/iio/frequency/ad9832.c +0 -8
-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 */
-
-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);

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 253

Quoted 10 lines
drivers/staging/iio/frequency/ad9832.c +1 -9
-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_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_altvoltage0_pincontrol_en, 0200, NULL, ad9832_write, AD9832_PINCTRL_EN);

I'm not that keen on having the documentation only several patches later. Drag that before this patch or combine adding the new ABI and documentation in the same patch

Jonathan

Quoted 4 lines
drivers/staging/iio/frequency/ad9832.c +1 -0
+static IIO_DEVICE_ATTR(out_altvoltage0_out_enable, 0200, NULL, ad9832_write, AD9832_OUTPUT_EN);
 
 static struct attribute *ad9832_attributes[] = {
 	&iio_dev_attr_out_altvoltage0_frequency0.dev_attr.attr,
maintainer
#14 lore raw

Re: [PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency

Quoted 72 lines · On Wed, 31 Dec 2025 18:09:39 +0000 Jonathan Cameron <[email protected]> wrote:
Quoted 4 lines · On Tue, 30 Dec 2025 17:34:57 -0300 Tomas Borquez <[email protected]>…

Remove dependency on dds.h by converting custom macros to standard IIO attribute declarations.

Signed-off-by Tomas Borquez <[email protected]>

Hi Tomas,

Happy new year (almost)

Quoted 19 lines
---
 drivers/staging/iio/frequency/ad9832.c | 37 +++++++++++---------------
 1 file changed, 15 insertions(+), 22 deletions(-)
drivers/staging/iio/frequency/ad9832.c +0 -2
index 4bb203a67046..aa78973c3a3c 100644
@@ -24,8 +24,6 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#include "dds.h"
-
 /* Registers */
 #define AD9832_FREQ0LL		0x0
 #define AD9832_FREQ0HL		0x1

@@ -238,27 +236,22 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 	}

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 243

Quoted 10 lines
drivers/staging/iio/frequency/ad9832.c +5 -3
 }
 
-/*
- * see dds.h for further information
- */
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency0, 0200, NULL, ad9832_write, AD9832_FREQ0HM);
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency1, 0200, NULL, ad9832_write, AD9832_FREQ1HM);
+
+static IIO_DEVICE_ATTR(out_altvoltage0_frequencysymbol, 0200, NULL, ad9832_write, AD9832_FREQ_SYM);
+static IIO_CONST_ATTR(out_altvoltage0_frequency_scale, "1"); /* 1Hz */

This seems like a pointless attribute. Default scaling for everything in IIO when attributes don't tell us otherwise is 1 so should be fine dropping this one.

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 251

Quoted 8 lines
drivers/staging/iio/frequency/ad9832.c +8 -0
+
+static IIO_DEVICE_ATTR(out_altvoltage0_phase0, 0200, NULL, ad9832_write, AD9832_PHASE0H);
+static IIO_DEVICE_ATTR(out_altvoltage0_phase1, 0200, NULL, ad9832_write, AD9832_PHASE1H);
+static IIO_DEVICE_ATTR(out_altvoltage0_phase2, 0200, NULL, ad9832_write, AD9832_PHASE2H);
+static IIO_DEVICE_ATTR(out_altvoltage0_phase3, 0200, NULL, ad9832_write, AD9832_PHASE3H);
+
+static IIO_DEVICE_ATTR(out_altvoltage0_phasesymbol, 0200, NULL, ad9832_write, AD9832_PHASE_SYM);
+static IIO_CONST_ATTR(out_altvoltage0_phase_scale, "0.0015339808"); /* 2PI/2^12 rad */

I can't immediately think of precedence for scaling of an attribute other than _raw. Whilst it's painful, this isn't a high perf path, so we should probably just do fixed point inputs for phase0,phase1 etc and deal with the scaling in the driver. That avoids adding new ABI for this very rare case.

Quoted 14 lines
drivers/staging/iio/frequency/ad9832.c +0 -13
-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 */
-
-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*/
-
Quoted 8 lines

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 253

Quoted 5 lines
drivers/staging/iio/frequency/ad9832.c +1 -4
-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_altvoltage0_pincontrol_en, 0200, NULL, ad9832_write, AD9832_PINCTRL_EN);

I'm not that keen on having the documentation only several patches later. Drag that before this patch or combine adding the new ABI and documentation in the same patch

Ah. I'd missed that this is deliberately a no change patch with old abi.

So ignore the stuff that doesn't make sense with that in mind!

Quoted 9 lines

Jonathan

Quoted 4 lines
drivers/staging/iio/frequency/ad9832.c +1 -0
+static IIO_DEVICE_ATTR(out_altvoltage0_out_enable, 0200, NULL, ad9832_write, AD9832_OUTPUT_EN);
 
 static struct attribute *ad9832_attributes[] = {
 	&iio_dev_attr_out_altvoltage0_frequency0.dev_attr.attr,
author
#15 lore raw

Re: [PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency

Quoted 6 lines · Jonathan Cameron
Quoted 3 lines · On Wed, 31 Dec 2025 18:09:39 +0000 Jonathan Cameron <[email protected]> wrote:

On Tue, 30 Dec 2025 17:34:57 -0300
Tomas Borquez <[email protected]> wrote:

Quoted 7 lines
Quoted 7 lines
Quoted 4 lines

Remove dependency on dds.h by converting custom macros to standard IIO attribute declarations.

Signed-off-by Tomas Borquez <[email protected]>

Hi Tomas,

Happy new year (almost)

Hey Jonathan,

Happy new year!

...

Quoted 5 lines
Quoted 2 lines

I'm not that keen on having the documentation only several patches later. Drag that before this patch or combine adding the new ABI and documentation in the same patch

Ah. I'd missed that this is deliberately a no change patch with old abi.

So ignore the stuff that doesn't make sense with that in mind!

Just to make sure I understood:
- I should just remove out_altvoltage0_frequency_scale
- And add documentation in the same patch with all the ABI changes "staging: iio: ad9832: convert to iio channels and ext_info attrs" or as a separate patch like it is now?

maintainer
#16 lore raw

Re: [PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency

Quoted 24 lines · On Sun, 4 Jan 2026 02:38:50 -0300 Tomas Borquez <[email protected]>…
Quoted 6 lines · Jonathan Cameron
Quoted 3 lines · On Wed, 31 Dec 2025 18:09:39 +0000 Jonathan Cameron <[email protected]> wrote:

On Tue, 30 Dec 2025 17:34:57 -0300
Tomas Borquez <[email protected]> wrote:

Quoted 7 lines
Quoted 7 lines
Quoted 4 lines

Remove dependency on dds.h by converting custom macros to standard IIO attribute declarations.

Signed-off-by Tomas Borquez <[email protected]>

Hi Tomas,

Happy new year (almost)

Hey Jonathan,

Happy new year!

...

Quoted 4 lines
Quoted 2 lines

I'm not that keen on having the documentation only several patches later. Drag that before this patch or combine adding the new ABI and documentation in the same patch

Ah. I'd missed that this is deliberately a no change patch with old abi.

Quoted 7 lines
Quoted 1 line

So ignore the stuff that doesn't make sense with that in mind!

Just to make sure I understood:
- I should just remove out_altvoltage0_frequency_scale
- And add documentation in the same patch with all the ABI changes "staging: iio: ad9832: convert to iio channels and ext_info attrs" or as a separate patch like it is now?

It's fine as you have it already. Ultimately out_altvotage0_frequency_scale should go away but that can come in the ABI update patch.

I'd just misunderstood that this was simply a 'get rid of dds.h' usage patch and the real ABI changes are later.

Jonathan

Quoted 1 line
author
#17 lore raw

PATCH v2 5/6 staging: iio: ad9832: convert to iio channels and ext_info attrs

In reply to Tomas Borquez

Convert ad9832 from sysfs attributes to standard channel interface using a single IIO_ALTCURRENT channel with ext_info attributes, as this device is a current source DAC with one output.

Signed-off-by Tomas Borquez <[email protected]>
---
 drivers/staging/iio/frequency/ad9832.c | 278 +++++++++++++++++++------
 1 file changed, 220 insertions(+), 58 deletions(-)
drivers/staging/iio/frequency/ad9832.c +220 -58
index aa78973c3a3c..47b41b9eb14f 100644
@@ -20,6 +20,7 @@
 #include <linux/spi/spi.h>
 #include <linux/sysfs.h>
 #include <linux/unaligned.h>
+#include <linux/units.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>

@@ -64,6 +65,7 @@
 #define AD9832_CLR		BIT(11)
 #define AD9832_FREQ_BITS	32
 #define AD9832_PHASE_BITS	12
+#define AD9832_2PI_URAD		6283185UL
 #define AD9832_CMD_MSK		GENMASK(15, 12)
 #define AD9832_ADD_MSK		GENMASK(11, 8)
 #define AD9832_DAT_MSK		GENMASK(7, 0)

@@ -75,6 +77,12 @@
  * @ctrl_fp:		cached frequency/phase control word
  * @ctrl_ss:		cached sync/selsrc control word
  * @ctrl_src:		cached sleep/reset/clr word
+ * @freq:		cached frequencies
+ * @freq_sym:		cached frequency symbol selection
+ * @phase:		cached phases
+ * @phase_sym:		cached phase symbol selection
+ * @output_en:		cached output enable state
+ * @pinctrl_en:		cached pinctrl enable state
  * @xfer:		default spi transfer
  * @msg:		default spi message
  * @freq_xfer:		tuning word spi transfer

@@ -92,6 +100,12 @@ struct ad9832_state {
 	unsigned short			ctrl_fp;
 	unsigned short			ctrl_ss;
 	unsigned short			ctrl_src;
+	u32				freq[2];
+	bool				freq_sym;
+	u32				phase[4];
+	u32				phase_sym;
+	bool				output_en;
+	bool				pinctrl_en;
 	struct spi_transfer		xfer;
 	struct spi_message		msg;
 	struct spi_transfer		freq_xfer[4];

@@ -110,7 +124,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);

@@ -118,19 +132,33 @@ static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
 	return freqreg;
 }
 
-static int ad9832_write_frequency(struct ad9832_state *st,
-				  unsigned int addr, unsigned long fout)
+static ssize_t ad9832_write_frequency(struct iio_dev *indio_dev,
+				      uintptr_t private,
+				      struct iio_chan_spec const *chan,
+				      const char *buf, size_t len)
 {
+	struct ad9832_state *st = iio_priv(indio_dev);
 	unsigned long clk_freq;
 	unsigned long regval;
 	u8 regval_bytes[4];
 	u16 freq_cmd;
+	u32 fout, addr;
+	int ret;
 
-	clk_freq = clk_get_rate(st->mclk);
+	if (private > 1)
+		return -EINVAL;
+
+	addr = (private == 0) ? AD9832_FREQ0HM : AD9832_FREQ1HM;
+
+	ret = kstrtou32(buf, 10, &fout);
+	if (ret)
+		return ret;
 
+	clk_freq = clk_get_rate(st->mclk);
 	if (!clk_freq || fout > (clk_freq / 2))
 		return -EINVAL;
 
+	guard(mutex)(&st->lock);
 	regval = ad9832_calc_freqreg(clk_freq, fout);
 	put_unaligned_be32(regval, regval_bytes);
 

@@ -142,32 +170,102 @@ 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[private] = fout;
+
+	return len;
+}
+
+static ssize_t ad9832_read_frequency(struct iio_dev *indio_dev,
+				     uintptr_t private,
+				     struct iio_chan_spec const *chan,
+				     char *buf)
+{
+	struct ad9832_state *st = iio_priv(indio_dev);
+	u32 val;
+
+	if (private > 1)
+		return -EINVAL;
+
+	guard(mutex)(&st->lock);
+	val = st->freq[private];
+
+	return sysfs_emit(buf, "%u\n", val);
 }
 
-static int ad9832_write_phase(struct ad9832_state *st,
-			      unsigned long addr, unsigned long phase)
+static const u32 ad9832_phase_addr[] = {
+	AD9832_PHASE0H, AD9832_PHASE1H, AD9832_PHASE2H, AD9832_PHASE3H
+};
+
+static ssize_t ad9832_write_phase(struct iio_dev *indio_dev,
+				  uintptr_t private,
+				  struct iio_chan_spec const *chan,
+				  const char *buf, size_t len)
 {
+	struct ad9832_state *st = iio_priv(indio_dev);
 	u8 phase_bytes[2];
 	u16 phase_cmd;
+	u32 phase_urad, phase;
+	int val, val2, ret;
 
-	if (phase >= BIT(AD9832_PHASE_BITS))
+	if (private >= ARRAY_SIZE(ad9832_phase_addr))
 		return -EINVAL;
 
-	put_unaligned_be16(phase, phase_bytes);
+	ret = iio_str_to_fixpoint(buf, 100000, &val, &val2);
+	if (ret)
+		return ret;
+
+	if (val < 0 || val2 < 0)
+		return -EINVAL;
+
+	phase_urad = val * MICRO + val2;
+	if (phase_urad >= AD9832_2PI_URAD)
+		return -EINVAL;
 
+	/* Convert microradians to 12-bit phase register value (0 to 4095) */
+	phase = ((u64)phase_urad << AD9832_PHASE_BITS) / AD9832_2PI_URAD;
+
+	guard(mutex)(&st->lock);
+	put_unaligned_be16(phase, phase_bytes);
 	for (int i = 0; i < ARRAY_SIZE(phase_bytes); i++) {
 		phase_cmd = (i % 2 == 0) ? AD9832_CMD_PHA8BITSW : AD9832_CMD_PHA16BITSW;
 
 		st->phase_data[i] = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, phase_cmd) |
-			FIELD_PREP(AD9832_ADD_MSK, addr - i) |
+			FIELD_PREP(AD9832_ADD_MSK, ad9832_phase_addr[private] - i) |
 			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[private] = phase;
+
+	return len;
 }
 
-static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
+static ssize_t ad9832_read_phase(struct iio_dev *indio_dev,
+				 uintptr_t private,
+				 struct iio_chan_spec const *chan,
+				 char *buf)
+{
+	struct ad9832_state *st = iio_priv(indio_dev);
+	u32 phase_urad;
+
+	if (private >= ARRAY_SIZE(ad9832_phase_addr))
+		return -EINVAL;
+
+	guard(mutex)(&st->lock);
+	phase_urad = ((u64)st->phase[private] * AD9832_2PI_URAD) >> AD9832_PHASE_BITS;
+
+	return sysfs_emit(buf, "%u.%06u\n", phase_urad / (u32)MICRO, phase_urad % (u32)MICRO);
+}
+
+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);

@@ -181,25 +279,7 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 		return ret;
 
 	guard(mutex)(&st->lock);
-	switch ((u32)this_attr->address) {
-	case AD9832_FREQ0HM:
-	case AD9832_FREQ1HM:
-		ret = ad9832_write_frequency(st, this_attr->address, val);
-		return ret ?: len;
-	case AD9832_PHASE0H:
-	case AD9832_PHASE1H:
-	case AD9832_PHASE2H:
-	case AD9832_PHASE3H:
-		ret = ad9832_write_phase(st, this_attr->address, val);
-		return ret ?: len;
-	case AD9832_PINCTRL_EN:
-		st->ctrl_ss &= ~AD9832_SELSRC;
-		st->ctrl_ss |= FIELD_PREP(AD9832_SELSRC, val ? 0 : 1);
-
-		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_SYNCSELSRC) |
-						  st->ctrl_ss);
-		ret = spi_sync(st->spi, &st->msg);
-		return ret ?: len;
+	switch (this_attr->address) {
 	case AD9832_FREQ_SYM:
 		if (val != 1 && val != 0)
 			return -EINVAL;

@@ -209,7 +289,11 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_FPSELECT) |
 						  st->ctrl_fp);
 		ret = spi_sync(st->spi, &st->msg);
-		return ret ?: len;
+		if (ret)
+			return ret;
+
+		st->freq_sym = val;
+		break;
 	case AD9832_PHASE_SYM:
 		if (val > 3)
 			return -EINVAL;

@@ -220,8 +304,15 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_FPSELECT) |
 						  st->ctrl_fp);
 		ret = spi_sync(st->spi, &st->msg);
-		return ret ?: len;
+		if (ret)
+			return ret;
+
+		st->phase_sym = val;
+		break;
 	case AD9832_OUTPUT_EN:
+		if (val != 1 && val != 0)
+			return -EINVAL;
+
 		if (val)
 			st->ctrl_src &= ~(AD9832_RESET | AD9832_SLEEP | AD9832_CLR);
 		else

@@ -230,42 +321,111 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_SLEEPRESCLR) |
 						  st->ctrl_src);
 		ret = spi_sync(st->spi, &st->msg);
-		return ret ?: len;
+		if (ret)
+			return ret;
+
+		st->output_en = val;
+		break;
+	case AD9832_PINCTRL_EN:
+		if (val != 1 && val != 0)
+			return -EINVAL;
+
+		st->ctrl_ss &= ~AD9832_SELSRC;
+		st->ctrl_ss |= FIELD_PREP(AD9832_SELSRC, !val);
+
+		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_SYNCSELSRC) |
+						 st->ctrl_ss);
+		ret = spi_sync(st->spi, &st->msg);
+		if (ret)
+			return ret;
+
+		st->pinctrl_en = val;
+		break;
 	default:
 		return -ENODEV;
 	}
+
+	return len;
 }
 
-static IIO_DEVICE_ATTR(out_altvoltage0_frequency0, 0200, NULL, ad9832_write, AD9832_FREQ0HM);
-static IIO_DEVICE_ATTR(out_altvoltage0_frequency1, 0200, NULL, ad9832_write, AD9832_FREQ1HM);
+static ssize_t ad9832_show(struct device *dev,
+			   struct device_attribute *attr,
+			   char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct ad9832_state *st = iio_priv(indio_dev);
+	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
 
-static IIO_DEVICE_ATTR(out_altvoltage0_frequencysymbol, 0200, NULL, ad9832_write, AD9832_FREQ_SYM);
-static IIO_CONST_ATTR(out_altvoltage0_frequency_scale, "1"); /* 1Hz */
+	guard(mutex)(&st->lock);
+	switch (this_attr->address) {
+	case AD9832_FREQ_SYM:
+		return sysfs_emit(buf, "%u\n", st->freq_sym);
+	case AD9832_PHASE_SYM:
+		return sysfs_emit(buf, "%u\n", st->phase_sym);
+	case AD9832_OUTPUT_EN:
+		return sysfs_emit(buf, "%u\n", st->output_en);
+	case AD9832_PINCTRL_EN:
+		return sysfs_emit(buf, "%u\n", st->pinctrl_en);
+	default:
+		return -ENODEV;
+	}
+}
 
-static IIO_DEVICE_ATTR(out_altvoltage0_phase0, 0200, NULL, ad9832_write, AD9832_PHASE0H);
-static IIO_DEVICE_ATTR(out_altvoltage0_phase1, 0200, NULL, ad9832_write, AD9832_PHASE1H);
-static IIO_DEVICE_ATTR(out_altvoltage0_phase2, 0200, NULL, ad9832_write, AD9832_PHASE2H);
-static IIO_DEVICE_ATTR(out_altvoltage0_phase3, 0200, NULL, ad9832_write, AD9832_PHASE3H);
+#define AD9832_CHAN_FREQ(_name, _select) {			\
+	.name = _name,						\
+	.write = ad9832_write_frequency,			\
+	.read = ad9832_read_frequency,				\
+	.private = _select,					\
+	.shared = IIO_SEPARATE,					\
+}
+
+#define AD9832_CHAN_PHASE(_name, _select) {			\
+	.name = _name,						\
+	.write = ad9832_write_phase,				\
+	.read = ad9832_read_phase,				\
+	.private = _select,					\
+	.shared = IIO_SEPARATE,					\
+}
 
-static IIO_DEVICE_ATTR(out_altvoltage0_phasesymbol, 0200, NULL, ad9832_write, AD9832_PHASE_SYM);
-static IIO_CONST_ATTR(out_altvoltage0_phase_scale, "0.0015339808"); /* 2PI/2^12 rad */
+static const struct iio_chan_spec_ext_info ad9832_ext_info[] = {
+	AD9832_CHAN_FREQ("frequency0", 0),
+	AD9832_CHAN_FREQ("frequency1", 1),
+	AD9832_CHAN_PHASE("phase0", 0),
+	AD9832_CHAN_PHASE("phase1", 1),
+	AD9832_CHAN_PHASE("phase2", 2),
+	AD9832_CHAN_PHASE("phase3", 3),
+	{ }
+};
 
-static IIO_DEVICE_ATTR(out_altvoltage0_pincontrol_en, 0200, NULL, ad9832_write, AD9832_PINCTRL_EN);
-static IIO_DEVICE_ATTR(out_altvoltage0_out_enable, 0200, NULL, ad9832_write, AD9832_OUTPUT_EN);
+static const struct iio_chan_spec ad9832_channels[] = {
+	{
+		.type = IIO_ALTCURRENT,
+		.output = 1,
+		.indexed = 1,
+		.channel = 0,
+		.ext_info = ad9832_ext_info,
+	},
+};
+
+static IIO_DEVICE_ATTR(out_altcurrent0_frequency_symbol, 0644,
+		       ad9832_show, ad9832_store, AD9832_FREQ_SYM);
+static IIO_DEVICE_ATTR(out_altcurrent0_phase_symbol, 0644,
+		       ad9832_show, ad9832_store, AD9832_PHASE_SYM);
+static IIO_DEVICE_ATTR(out_altcurrent0_enable, 0644,
+		       ad9832_show, ad9832_store, AD9832_OUTPUT_EN);
+
+/*
+ * TODO: Convert to DT property when graduating from staging.
+ * Pin control configuration depends on hardware wiring.
+ */
+static IIO_DEVICE_ATTR(out_altcurrent0_pincontrol_en, 0644,
+		       ad9832_show, 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_dev_attr_out_altcurrent0_frequency_symbol.dev_attr.attr,
+	&iio_dev_attr_out_altcurrent0_phase_symbol.dev_attr.attr,
+	&iio_dev_attr_out_altcurrent0_enable.dev_attr.attr,
+	&iio_dev_attr_out_altcurrent0_pincontrol_en.dev_attr.attr,
 	NULL,
 };
 

@@ -310,6 +470,8 @@ 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;
Signature · 1 line
2.43.0
#18 lore raw

Re: [PATCH v2 5/6] staging: iio: ad9832: convert to iio channels and ext_info attrs

Quoted 4 lines · Tomas Borquez

Convert ad9832 from sysfs attributes to standard channel interface using a single IIO_ALTCURRENT channel with ext_info attributes, as this device is a current source DAC with one output.

...

drivers/staging/iio/frequency/ad9832.c static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout) · line 136

Quoted 4 lines
drivers/staging/iio/frequency/ad9832.c +2 -2
-static int ad9832_write_frequency(struct ad9832_state *st,
-                                 unsigned int addr, unsigned long fout)
+static ssize_t ad9832_write_frequency(struct iio_dev *indio_dev,
+                                     uintptr_t private,

Torvalds said that uintptr_t shouldn't be used in the Linux kernel, the unsigned long suffice and enough. Why do we need it here?

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 415

Quoted 6 lines
drivers/staging/iio/frequency/ad9832.c +6 -0
+static IIO_DEVICE_ATTR(out_altcurrent0_frequency_symbol, 0644,
+                      ad9832_show, ad9832_store, AD9832_FREQ_SYM);
+static IIO_DEVICE_ATTR(out_altcurrent0_phase_symbol, 0644,
+                      ad9832_show, ad9832_store, AD9832_PHASE_SYM);
+static IIO_DEVICE_ATTR(out_altcurrent0_enable, 0644,
+                      ad9832_show, ad9832_store, AD9832_OUTPUT_EN);

Why not IIO_DEVICE_ATTR_RW()?

...

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 429

Quoted 5 lines
drivers/staging/iio/frequency/ad9832.c +4 -0
+       &iio_dev_attr_out_altcurrent0_frequency_symbol.dev_attr.attr,
+       &iio_dev_attr_out_altcurrent0_phase_symbol.dev_attr.attr,
+       &iio_dev_attr_out_altcurrent0_enable.dev_attr.attr,
+       &iio_dev_attr_out_altcurrent0_pincontrol_en.dev_attr.attr,
        NULL,

At some point we may also drop the comma in the terminator entry.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#19 lore raw

Re: [PATCH v2 5/6] staging: iio: ad9832: convert to iio channels and ext_info attrs

Quoted 5 lines · Andy Shevchenko
Quoted 4 lines · Tomas Borquez

Convert ad9832 from sysfs attributes to standard channel interface using a single IIO_ALTCURRENT channel with ext_info attributes, as this device is a current source DAC with one output.

...

Quoted 7 lines

drivers/staging/iio/frequency/ad9832.c static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout) · line 136

Quoted 4 lines
drivers/staging/iio/frequency/ad9832.c +2 -2
-static int ad9832_write_frequency(struct ad9832_state *st,
-                                 unsigned int addr, unsigned long fout)
+static ssize_t ad9832_write_frequency(struct iio_dev *indio_dev,
+                                     uintptr_t private,

Torvalds said that uintptr_t shouldn't be used in the Linux kernel, the unsigned long suffice and enough. Why do we need it here?

Copied it from the definition of iio_chan_spec_ext_info:

struct iio_chan_spec_ext_info {
	const char *name;
	enum iio_shared_by shared;
	ssize_t (*read)(struct iio_dev *, uintptr_t private,
			struct iio_chan_spec const *, char *buf);
	ssize_t (*write)(struct iio_dev *, uintptr_t private,
			 struct iio_chan_spec const *, const char *buf,
			 size_t len);
	uintptr_t private;
};

But can change it

Quoted 9 lines

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 415

Quoted 7 lines
drivers/staging/iio/frequency/ad9832.c +6 -0
+static IIO_DEVICE_ATTR(out_altcurrent0_frequency_symbol, 0644,
+                      ad9832_show, ad9832_store, AD9832_FREQ_SYM);
+static IIO_DEVICE_ATTR(out_altcurrent0_phase_symbol, 0644,
+                      ad9832_show, ad9832_store, AD9832_PHASE_SYM);
+static IIO_DEVICE_ATTR(out_altcurrent0_enable, 0644,
+                      ad9832_show, ad9832_store, AD9832_OUTPUT_EN);

Why not IIO_DEVICE_ATTR_RW()?

Not any good reason just didn't know it existed.

Quoted 9 lines

...

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 429

Quoted 5 lines
drivers/staging/iio/frequency/ad9832.c +4 -0
+       &iio_dev_attr_out_altcurrent0_frequency_symbol.dev_attr.attr,
+       &iio_dev_attr_out_altcurrent0_phase_symbol.dev_attr.attr,
+       &iio_dev_attr_out_altcurrent0_enable.dev_attr.attr,
+       &iio_dev_attr_out_altcurrent0_pincontrol_en.dev_attr.attr,
        NULL,

At some point we may also drop the comma in the terminator entry.

Will remove it as part of V3

Quoted 3 lines
Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#20 lore raw

Re: [PATCH v2 5/6] staging: iio: ad9832: convert to iio channels and ext_info attrs

Quoted 30 lines · On Wed, 31 Dec 2025 14:08:52 -0300 Tomas Borquez <[email protected]>…
Quoted 5 lines · Andy Shevchenko
Quoted 4 lines · Tomas Borquez

Convert ad9832 from sysfs attributes to standard channel interface using a single IIO_ALTCURRENT channel with ext_info attributes, as this device is a current source DAC with one output.

...

Quoted 7 lines

drivers/staging/iio/frequency/ad9832.c static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout) · line 136

Quoted 4 lines
drivers/staging/iio/frequency/ad9832.c +2 -2
-static int ad9832_write_frequency(struct ad9832_state *st,
-                                 unsigned int addr, unsigned long fout)
+static ssize_t ad9832_write_frequency(struct iio_dev *indio_dev,
+                                     uintptr_t private,

Torvalds said that uintptr_t shouldn't be used in the Linux kernel, the unsigned long suffice and enough. Why do we need it here?

Copied it from the definition of iio_chan_spec_ext_info:

struct iio_chan_spec_ext_info {
	const char *name;
	enum iio_shared_by shared;
	ssize_t (*read)(struct iio_dev *, uintptr_t private,
			struct iio_chan_spec const *, char *buf);
	ssize_t (*write)(struct iio_dev *, uintptr_t private,
			 struct iio_chan_spec const *, const char *buf,
			 size_t len);
	uintptr_t private;
};
Quoted 1 line

But can change it

As Linus has been clear on his preferences on this, we should probably clean that iio_chan_spec_ext_info up as well at some
point. Not urgent though and don't gate this patch on it. Better to clean up all existing users in one go.

This will do as a reference for why:

https://lore.kernel.org/all/CAHk-=wgSvPVGZp56uFCjOZoKcgQp7xpsj3P-Hhg+NXvhPnzszg@mail.gmail.com/

Jonathan

Quoted 28 lines
Quoted 10 lines

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 415

Quoted 8 lines
drivers/staging/iio/frequency/ad9832.c +6 -0
+static IIO_DEVICE_ATTR(out_altcurrent0_frequency_symbol, 0644,
+                      ad9832_show, ad9832_store, AD9832_FREQ_SYM);
+static IIO_DEVICE_ATTR(out_altcurrent0_phase_symbol, 0644,
+                      ad9832_show, ad9832_store, AD9832_PHASE_SYM);
+static IIO_DEVICE_ATTR(out_altcurrent0_enable, 0644,
+                      ad9832_show, ad9832_store, AD9832_OUTPUT_EN);

Why not IIO_DEVICE_ATTR_RW()?

Not any good reason just didn't know it existed.

Quoted 9 lines

...

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 429

Quoted 5 lines
drivers/staging/iio/frequency/ad9832.c +4 -0
+       &iio_dev_attr_out_altcurrent0_frequency_symbol.dev_attr.attr,
+       &iio_dev_attr_out_altcurrent0_phase_symbol.dev_attr.attr,
+       &iio_dev_attr_out_altcurrent0_enable.dev_attr.attr,
+       &iio_dev_attr_out_altcurrent0_pincontrol_en.dev_attr.attr,
        NULL,

At some point we may also drop the comma in the terminator entry.

Will remove it as part of V3

Quoted 3 lines
Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#21 lore raw

Re: [PATCH v2 5/6] staging: iio: ad9832: convert to iio channels and ext_info attrs

In reply to Tomas Borquez
Quoted 5 lines · On Tue, 30 Dec 2025 17:34:58 -0300 Tomas Borquez <[email protected]>…

Convert ad9832 from sysfs attributes to standard channel interface using a single IIO_ALTCURRENT channel with ext_info attributes, as this device is a current source DAC with one output.

Signed-off-by Tomas Borquez <[email protected]>

A couple of things inline.

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 327

Quoted 9 lines
drivers/staging/iio/frequency/ad9832.c +7 -0
+

@@ -230,42 +321,111 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 
+	case AD9832_PINCTRL_EN:
+		if (val != 1 && val != 0)
+			return -EINVAL;
+
+		st->ctrl_ss &= ~AD9832_SELSRC;
+		st->ctrl_ss |= FIELD_PREP(AD9832_SELSRC, !val);

It's not particularly important as this pattern is common, but there is FIELD_MODIFY() available to make the above a single thing without needing the mask twice

Quoted 34 lines
drivers/staging/iio/frequency/ad9832.c +24 -4
+
+		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_SYNCSELSRC) |
+						 st->ctrl_ss);
+		ret = spi_sync(st->spi, &st->msg);
+		if (ret)
+			return ret;
+
+		st->pinctrl_en = val;
+		break;
 	default:
 		return -ENODEV;
 	}
+
+	return len;
 }
 
-static IIO_DEVICE_ATTR(out_altvoltage0_phasesymbol, 0200, NULL, ad9832_write, AD9832_PHASE_SYM);
-static IIO_CONST_ATTR(out_altvoltage0_phase_scale, "0.0015339808"); /* 2PI/2^12 rad */
+static const struct iio_chan_spec_ext_info ad9832_ext_info[] = {
+	AD9832_CHAN_FREQ("frequency0", 0),
+	AD9832_CHAN_FREQ("frequency1", 1),
+	AD9832_CHAN_PHASE("phase0", 0),
+	AD9832_CHAN_PHASE("phase1", 1),
+	AD9832_CHAN_PHASE("phase2", 2),
+	AD9832_CHAN_PHASE("phase3", 3),
+	{ }
+};
 
-static IIO_DEVICE_ATTR(out_altvoltage0_pincontrol_en, 0200, NULL, ad9832_write, AD9832_PINCTRL_EN);
-static IIO_DEVICE_ATTR(out_altvoltage0_out_enable, 0200, NULL, ad9832_write, AD9832_OUTPUT_EN);
+static const struct iio_chan_spec ad9832_channels[] = {
+	{
+		.type = IIO_ALTCURRENT,
+		.output = 1,

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 413

Quoted 10 lines
drivers/staging/iio/frequency/ad9832.c +10 -0
+		.indexed = 1,
+		.channel = 0,
+		.ext_info = ad9832_ext_info,
+	},
+};
+
+static IIO_DEVICE_ATTR(out_altcurrent0_frequency_symbol, 0644,
+		       ad9832_show, ad9832_store, AD9832_FREQ_SYM);
+static IIO_DEVICE_ATTR(out_altcurrent0_phase_symbol, 0644,
+		       ad9832_show, ad9832_store, AD9832_PHASE_SYM);

Why not do these two via ext_info?

drivers/staging/iio/frequency/ad9832.c static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr, · line 414

Quoted 1 line
drivers/staging/iio/frequency/ad9832.c +1 -0
+static IIO_DEVICE_ATTR(out_altcurrent0_enable, 0644,

This one can be done with standard ABI + infomask element at read_raw so do it that way rather than via IIO_DEVICE_ATTR() which should be used only when none of the standard stuff is possible because these direct attr declarations hide things from internal kernel usage and mean we have to much more carefully check them against ABI documentation.

Quoted 28 lines
drivers/staging/iio/frequency/ad9832.c +12 -12
+		       ad9832_show, ad9832_store, AD9832_OUTPUT_EN);
+
+/*
+ * TODO: Convert to DT property when graduating from staging.
+ * Pin control configuration depends on hardware wiring.
+ */
+static IIO_DEVICE_ATTR(out_altcurrent0_pincontrol_en, 0644,
+		       ad9832_show, 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_dev_attr_out_altcurrent0_frequency_symbol.dev_attr.attr,
+	&iio_dev_attr_out_altcurrent0_phase_symbol.dev_attr.attr,
+	&iio_dev_attr_out_altcurrent0_enable.dev_attr.attr,
+	&iio_dev_attr_out_altcurrent0_pincontrol_en.dev_attr.attr,
 	NULL,
 };
author
#22 lore raw

PATCH v2 6/6 staging: iio: ad9832: add sysfs documentation

In reply to Tomas Borquez

Add sysfs ABI documentation for the AD9832/AD9835 Direct Digital Synthesizer chips, documenting frequency, phase, output control, and pin control attributes.

Signed-off-by Tomas Borquez <[email protected]>
---
 .../iio/Documentation/sysfs-bus-iio-frequency | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 drivers/staging/iio/Documentation/sysfs-bus-iio-frequency
drivers/staging/iio/Documentation/sysfs-bus-iio-frequency new file +40 -0
new file mode 100644
index 000000000000..10627c19bdb7
@@ -0,0 +1,40 @@
+What:		/sys/bus/iio/devices/iio:deviceX/out_altcurrentY_frequencyZ
+KernelVersion:	6.19
+Contact:	[email protected]
+Description:
+    Frequency in Hz for symbol Z of channel Y. The active
+    frequency symbol is selected via out_altcurrentY_frequency_symbol.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altcurrentY_phaseZ
+KernelVersion:	6.19
+Contact:	[email protected]
+Description:
+    Phase offset in radians for symbol Z of channel Y. Valid range
+    is 0 to 2*PI (exclusive). The active phase symbol is selected
+    via out_altcurrentY_phase_symbol.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altcurrentY_frequency_symbol
+KernelVersion:	6.19
+Contact:	[email protected]
+Description:
+    Selects which frequency symbol is active for channel Y.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altcurrentY_phase_symbol
+KernelVersion:	6.19
+Contact:	[email protected]
+Description:
+    Selects which phase symbol is active for channel Y.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altcurrentY_enable
+KernelVersion:	6.19
+Contact:	[email protected]
+Description:
+    Enables (1) or disables (0) the output for channel Y.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altcurrentY_pincontrol_en
+KernelVersion:	6.19
+Contact:	[email protected]
+Description:
+    Enables (1) or disables (0) hardware pin control for frequency
+    and phase symbol selection on channel Y. When enabled, external
+    pins control symbol selection instead of software.
Signature · 1 line
2.43.0
#23 lore raw

Re: [PATCH v2 6/6] staging: iio: ad9832: add sysfs documentation

Quoted 4 lines · Tomas Borquez

Add sysfs ABI documentation for the AD9832/AD9835 Direct Digital Synthesizer chips, documenting frequency, phase, output control, and pin control attributes.

...

Quoted 1 line

+KernelVersion: 6.19

6.20. The 6.19 boar already sailed.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#24 lore raw

Re: [PATCH v2 6/6] staging: iio: ad9832: add sysfs documentation

In reply to Tomas Borquez
Quoted 5 lines · On Tue, 30 Dec 2025 17:34:59 -0300 Tomas Borquez <[email protected]>…

Add sysfs ABI documentation for the AD9832/AD9835 Direct Digital Synthesizer chips, documenting frequency, phase, output control, and pin control attributes.

Signed-off-by Tomas Borquez <[email protected]>

Hi Tomas,

Good docs, one entry of which reminded me we tend to use powerdown for output devices rather than _en (for enable)

Jonathan

Quoted 29 lines
---
 .../iio/Documentation/sysfs-bus-iio-frequency | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 drivers/staging/iio/Documentation/sysfs-bus-iio-frequency
drivers/staging/iio/Documentation/sysfs-bus-iio-frequency new file +18 -0
new file mode 100644
index 000000000000..10627c19bdb7
@@ -0,0 +1,40 @@
+What:		/sys/bus/iio/devices/iio:deviceX/out_altcurrentY_frequencyZ
+KernelVersion:	6.19
+Contact:	[email protected]
+Description:
+    Frequency in Hz for symbol Z of channel Y. The active
+    frequency symbol is selected via out_altcurrentY_frequency_symbol.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altcurrentY_phaseZ
+KernelVersion:	6.19
+Contact:	[email protected]
+Description:
+    Phase offset in radians for symbol Z of channel Y. Valid range
+    is 0 to 2*PI (exclusive). The active phase symbol is selected
+    via out_altcurrentY_phase_symbol.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altcurrentY_frequency_symbol
+KernelVersion:	6.19
+Contact:	[email protected]

drivers/staging/iio/Documentation/sysfs-bus-iio-frequency line 28

Quoted 10 lines
drivers/staging/iio/Documentation/sysfs-bus-iio-frequency new file +10 -0
+Description:
+    Selects which frequency symbol is active for channel Y.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altcurrentY_phase_symbol
+KernelVersion:	6.19
+Contact:	[email protected]
+Description:
+    Selects which phase symbol is active for channel Y.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altcurrentY_enable

Ah. I probably led you astray on this in an earlier review. Would be _en
and added to the standard docs in sysfs-bus-iio in the block that starts with in_energyY_en

However, I think that might not be the right attribute to use based on this being more similar to a DAC channel than those which are about enabling internal accumulators. For a DAC the common terminology is about powerdown (so effectively !enable). That lines up with the use of SLEEP here on the datasheet.

So I think this should be out_altcurrentY_powerdown and documented in the block with out_altvoltageY_powerdown in sysfs-bus-iio with a tweak to make the descriptive text cover currents as well as voltages.

Obviously remember to flip the sense of the control though.

Quoted 12 lines
drivers/staging/iio/Documentation/sysfs-bus-iio-frequency new file +12 -0
+KernelVersion:	6.19
+Contact:	[email protected]
+Description:
+    Enables (1) or disables (0) the output for channel Y.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altcurrentY_pincontrol_en
+KernelVersion:	6.19
+Contact:	[email protected]
+Description:
+    Enables (1) or disables (0) hardware pin control for frequency
+    and phase symbol selection on channel Y. When enabled, external
+    pins control symbol selection instead of software.