1. Patchew
  2. linux
  3. [PATCH v2 0/6] staging: ad9832: driver cleanup
  4. View patch

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

Tomas Borquez posted 6 patches 1 month, 1 week ago
[PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency
Posted by Tomas Borquez 1 month, 1 week ago 
Remove dependency on dds.h by converting custom macros to standard IIO
attribute declarations.

Signed-off-by: Tomas Borquez <tomasborquez13@gmail.com>
---
 drivers/staging/iio/frequency/ad9832.c | 37 +++++++++++---------------
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 4bb203a67046..aa78973c3a3c 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -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,
-- 
2.43.0
Re: [PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency
Posted by Jonathan Cameron 1 month, 1 week ago 
On Tue, 30 Dec 2025 17:34:57 -0300
Tomas Borquez <tomasborquez13@gmail.com> wrote:

> Remove dependency on dds.h by converting custom macros to standard IIO
> attribute declarations.
> 
> Signed-off-by: Tomas Borquez <tomasborquez13@gmail.com>
Hi Tomas,

Happy new year (almost)

> ---
>  drivers/staging/iio/frequency/ad9832.c | 37 +++++++++++---------------
>  1 file changed, 15 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> index 4bb203a67046..aa78973c3a3c 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -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 */

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.

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

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

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


> +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,
Re: [PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency
Posted by Jonathan Cameron 1 month, 1 week ago 
On Wed, 31 Dec 2025 18:09:39 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue, 30 Dec 2025 17:34:57 -0300
> Tomas Borquez <tomasborquez13@gmail.com> wrote:
> 
> > Remove dependency on dds.h by converting custom macros to standard IIO
> > attribute declarations.
> > 
> > Signed-off-by: Tomas Borquez <tomasborquez13@gmail.com>  
> Hi Tomas,
> 
> Happy new year (almost)
> 
> > ---
> >  drivers/staging/iio/frequency/ad9832.c | 37 +++++++++++---------------
> >  1 file changed, 15 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> > index 4bb203a67046..aa78973c3a3c 100644
> > --- a/drivers/staging/iio/frequency/ad9832.c
> > +++ b/drivers/staging/iio/frequency/ad9832.c
> > @@ -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 */  
> 
> 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.
> 
> > +
> > +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.
> 
> >  
> > -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);  
> 
> 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!

> 
> Jonathan
> 
> 
> > +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,  
>
Re: [PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency
Posted by Tomas Borquez 1 month ago 
On Wed, Dec 31, 2025 at 06:11:53PM +0000, Jonathan Cameron wrote:
> On Wed, 31 Dec 2025 18:09:39 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Tue, 30 Dec 2025 17:34:57 -0300
> > Tomas Borquez <tomasborquez13@gmail.com> wrote:
> > 
> > > Remove dependency on dds.h by converting custom macros to standard IIO
> > > attribute declarations.
> > > 
> > > Signed-off-by: Tomas Borquez <tomasborquez13@gmail.com>  
> > Hi Tomas,
> > 
> > Happy new year (almost)
Hey Jonathan,

Happy new year!

...

> > 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?
Re: [PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency
Posted by Jonathan Cameron 3 weeks, 6 days ago 
On Sun, 4 Jan 2026 02:38:50 -0300
Tomas Borquez <tomasborquez13@gmail.com> wrote:

> On Wed, Dec 31, 2025 at 06:11:53PM +0000, Jonathan Cameron wrote:
> > On Wed, 31 Dec 2025 18:09:39 +0000
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >   
> > > On Tue, 30 Dec 2025 17:34:57 -0300
> > > Tomas Borquez <tomasborquez13@gmail.com> wrote:
> > >   
> > > > Remove dependency on dds.h by converting custom macros to standard IIO
> > > > attribute declarations.
> > > > 
> > > > Signed-off-by: Tomas Borquez <tomasborquez13@gmail.com>    
> > > Hi Tomas,
> > > 
> > > Happy new year (almost)  
> Hey Jonathan,
> 
> Happy new year!
> 
> ...
> 
> > > 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?

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

>
Re: [PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency
Posted by Andy Shevchenko 1 month, 1 week ago 
On Tue, Dec 30, 2025 at 10:35 PM Tomas Borquez <tomasborquez13@gmail.com> wrote:
>
> Remove dependency on dds.h by converting custom macros to standard IIO
> attribute declarations.


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

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency
Posted by Tomas Borquez 1 month ago 
On Wed, Dec 31, 2025 at 12:46:28AM +0200, Andy Shevchenko wrote:
> On Tue, Dec 30, 2025 at 10:35 PM Tomas Borquez <tomasborquez13@gmail.com> wrote:
> >
> > Remove dependency on dds.h by converting custom macros to standard IIO
> > attribute declarations.
> 
> 
> > +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

> -- 
> With Best Regards,
> Andy Shevchenko
Re: [PATCH v2 4/6] staging: iio: ad9832: remove dds.h dependency
Posted by Andy Shevchenko 1 month ago 
On Sun, Jan 04, 2026 at 02:25:23AM -0300, Tomas Borquez wrote:
> On Wed, Dec 31, 2025 at 12:46:28AM +0200, Andy Shevchenko wrote:
> > On Tue, Dec 30, 2025 at 10:35 PM Tomas Borquez <tomasborquez13@gmail.com> wrote:

...

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

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

-- 
With Best Regards,
Andy Shevchenko

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.