Re: [RFC PATCH] staging: iio: ad9832: modernize ABI and remove dds.h dependency

[Jonathan Cameron]

On Mon, 9 Mar 2026 04:49:01 +0530
Bhargav Joshi <rougueprince47@xxxxxxxxx> wrote:

> Hello Jonathan,
> 
> On Mon, Mar 9, 2026 at 12:01 AM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> >
> > On Fri,  6 Mar 2026 02:33:47 +0530
> > Bhargav Joshi <rougueprince47@xxxxxxxxx> wrote:
> >  
> > > The AD9832 driver currently relies on legacy custom IIO macros defined
> > > in dds.h. This triggers checkpatch.pl warnings (NON_OCTAL_PERMISSIONS)
> > > and, more importantly, exposes a non-standard sysfs ABI (e.g.,
> > > frequency0, frequency1, phase0-3) directly to user space.
> > >
> > > This patch removes the custom macros and migrates the driver to standard
> > > IIO API mechanisms:
> > > - Standard attributes (frequency, phase) now use info_mask_separate.
> > > - Non standard specific toggles (frequencysymbol, phasesymbol,
> > >   pincontrol) have been migrated to an ext_info array.
> > > - Remove dds.h header dependency.
> > > - Pointless frequency_scale and phase_scale attributes are dropped as
> > >   suggested by Jonathan in
> > >   https://lore.kernel.org/linux-iio/20251231180939.422e9e62@jic23-huawei/
> > >
> > > NOTE: This patch introduces an intentional ABI changes. The non-standard
> > > attributes (out_altvoltage0_frequency0, etc.) have been removed. They
> > > are replaced by standard attributes (out_altvoltage0_frequency and
> > > out_altvoltage0_phase). Routing to correct register while writing is
> > > handled by checking currently active frequencysymbol or phasesymbol.  
> >
> > Ah I think maybe the discussions around this bit got confused as this was
> > not the direction I was expecting it to go in.
> >
> > This doesn't align with the existing cases of how we do symbol selection
> > devices.
> >
> > Often the last thing we want to do is change the active symbol before we've set
> > the value. We could have done a multiplexing write function but if
> > we had it would need to be separate from the controls on which symbol is
> > in use.
> >  
> I initially thought multiplexing would help perfectly standardize the
> frequency and
> phase attributes, but you are right. Multiplexing would break
> hardware's modulation
> leading to unwanted glitches.
> 
> While we can still use a single standard attribute by introducing
> separate freq_write_target
> (and phase_write_target) selector, but that will overcomplicate it for the user.
> 
> So, for v2, I should drop the multiplexing approach and keep
> frequency0/1 and phase0-3
> separated using the ext_info array.
> 
> > There are a few examples of existing ABI for this in tree.
> > e.g.
> > dac/ad8460.c which has 16 different rawX outputs and a symbol to pick between those.
> > dac/ltc2644.c which has _raw0 and _raw1 for similar purposes
> > dac/ltc2688.c is similar.
> >
> > We've never added more 'standard' control for channels with symbol controls simply
> > because there aren't that many users yet.
> >
> >  
> > >
> > > Testing: This patch has been strictly compile-tested. I do not have
> > > access to physical AD9832 hardware. I am submitting this as an RFC to
> > > see if these changes are acceptable, and to ask if someone with physical
> > > hardware could test thisg and provide a Tested-by tag.
> > >  
> Considering David's valid reply: I would not be able to get hands on
> AD9832 hardware
> to validate these changes. So I am not quite sure if I should continue
> with v2 or not?

This sort of ABI update can in my view be safely done against emulation,
but for that you'd need to first emulate the device carefully then verify
that emulation (to some degree anyway) using the existing driver.

So up to you on how large a project you want to take on!

Jonathan

> 
> Thank you for the feedback!
> 
> Best regards,
> Bhargav
> 
> > > Signed-off-by: Bhargav Joshi <rougueprince47@xxxxxxxxx>  
> >