Re: [PATCH RFC 0/8] AD9910 Direct Digital Synthesizer

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 7 Mar 2026 14:01:14 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 3/7/26 12:54 PM, Rodrigo Alencar wrote:
> > On 26/03/07 04:58PM, Jonathan Cameron wrote:  
> >> On Sat, 7 Mar 2026 10:50:14 -0600
> >> David Lechner <dlechner@baylibre.com> wrote:
> >>  
> >>> On 3/7/26 8:09 AM, Jonathan Cameron wrote:  
> >>>> On Mon, 2 Mar 2026 10:22:47 +0000
> >>>> Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> >>>>     
> >>>>> On 26/03/01 01:38PM, Jonathan Cameron wrote:    
> >>>>>> On Mon, 23 Feb 2026 10:02:00 +0000
> >>>>>> Nuno Sá <noname.nuno@gmail.com> wrote:
> >>>>>>       
> >>>>>>> On Sun, 2026-02-22 at 14:32 -0600, David Lechner wrote:      
> >>>>>>>> On 2/22/26 4:01 AM, Rodrigo Alencar wrote:        
> >>>>>>>>> On 26/02/21 02:16PM, David Lechner wrote:        
> >>>>>>>>>> On 2/20/26 10:46 AM, Rodrigo Alencar via B4 Relay wrote:        
> >>>>>>>>>>> This patch series adds support for the Analog Devices AD9910 DDS.
> >>>>>>>>>>> This is an RFC so that we can agree/discuss on the design that follows:
> >>>>>>>>>>>         
> >>>>>>>>
> >>>>>>>> ...
> >>>>>>>>         
> >>>>>>>>>>> represents a distinct signal path into the DDS accumulator, so the driver
> >>>>>>>>>>> models them as separate IIO output channels (all IIO_ALTVOLTAGE type).        
> >>>>>>>>>>
> >>>>>>>>>> Generally IIO channels represent the physical input/output, not the
> >>>>>>>>>> internal channels.        
> >>>>>>>>>
> >>>>>>>>> That is part of the reason for this RFC. Dividing those top-level modes
> >>>>>>>>> into channels allows for better organization, as they can operate together,
> >>>>>>>>> i.e., phase or scale can be provided by single-tone profile, while
> >>>>>>>>> frequency is controlled by the digital ramp generator (see Mode Priority
> >>>>>>>>> section in the datasheet). Also, it allows to explore the most of standard
> >>>>>>>>> ABIs like, scale, frequency, phase, sampling_frequency and enable.
> >>>>>>>>> Putting everything into a single channel would make things a lot messy
> >>>>>>>>> to interface with.
> >>>>>>>>>         
> >>>>>>>>>> Ideally we would just have the one channel here with a mode selection
> >>>>>>>>>> attribute. Documentation can tell us which modes use which attributes.
> >>>>>>>>>>         
> >>>>>>>>>>> This per-channel separation allows userspace to configure each mode
> >>>>>>>>>>> independently through its own set of sysfs attributes, and to
> >>>>>>>>>>> enable/disable modes individually via IIO_CHAN_INFO_ENABLE, relying on
> >>>>>>>>>>> the hardware's own mode selection architecture.
> >>>>>>>>>>>         
> >>>>>>>>
> >>>>>>>> Looking at Table 5 in the datasheet really helped me understand this better.
> >>>>>>>> I think this series could benefit from a documentation patch that explains
> >>>>>>>> more about how the driver works with some diagrams.
> >>>>>>>>
> >>>>>>>> So really what we have here are a bunch of digital data generators rather
> >>>>>>>> than a bunch of altvotlage output channels. And the same data channels can be
> >>>>>>>> mixed and match as the source for up to 3 different components of the output
> >>>>>>>> (frequency, phase, amplitude) depending on the priority rules defined in
> >>>>>>>> Table 5.        
> >>>>>>>
> >>>>>>> More bellow... But note that all of the (or most of it) generators are going to
> >>>>>>> be feed into a DAC. Your output is altvoltage but maybe we can treat the
> >>>>>>> internals as voltage. Not sure.
> >>>>>>>        
> >>>>>>>>
> >>>>>>>> Digital data sources are really more like a buffer in IIO terms than a
> >>>>>>>> channel. And before we added the IIO backend stuff, there wasn't really
> >>>>>>>> any other digital data source/sink that I am aware of other than buffers
> >>>>>>>> (but there are certainly a lot of odd corners of IIO that I haven't explored
> >>>>>>>> yet, so maybe I missed some).
> >>>>>>>>
> >>>>>>>> In a recent discussion, the idea of possibly needing a way to provide
> >>>>>>>> some userspace interface to be able to tweak knobs of an IIO backend
> >>>>>>>> was also brought up.
> >>>>>>>>
> >>>>>>>> Putting those ideas together, I'm wondering if we need some new channel
> >>>>>>>> type or even a whole new interface (e.g. a new sysfs directory like buffers
> >>>>>>>> and events) for managing these digital data sources/sinks that are not an
> >>>>>>>> IIO buffer.
> >>>>>>>>         
> >>>>>>>
> >>>>>>> But what would be that channel? In the end of the day, we typically have voltage or
> >>>>>>> current DACs and a DDS primary function is indeed to generate alternating waveforms
> >>>>>>> that you then typically feed into a DAC (and in some cases from the DAC into a
> >>>>>>> power amplifier). So the DDS is just part of the data/signal path. Anyways, not sure
> >>>>>>> on the new type and I think we already have the "blocks" in IIO for dealing with this:
> >>>>>>>
> >>>>>>> . frequency
> >>>>>>> . phase
> >>>>>>> . amplitude (raw + scale + offset)
> >>>>>>>
> >>>>>>> But you're right that maybe it's time to think in a better way to fit them together. 
> >>>>>>> Maybe a new type (as buffers or events) can make sense where the above are treated as, example, scan
> >>>>>>> elements. Maybe it's overcomplicating, not sure. It surely needs  discussion and thinking :).
> >>>>>>>
> >>>>>>> And spoiler alert, as you might have guessed already, the parallel port stuff is to be
> >>>>>>> used with DMA buffers (and IIO backends). At least, that was the plan IIRC. But Rodrigo
> >>>>>>> can confirm it.
> >>>>>>>       
> >>>>>>>> I think we've seen enough of these already to know that things like a
> >>>>>>>> "tone generator" and a "ramp generator" are going to be common and could
> >>>>>>>> share some standard attributes. 
> >>>>>>>>         
> >>>>>>>
> >>>>>>> I tend to agree. For example, there already some DACs (with dithering) that make use of a similar
> >>>>>>> interface (but with a custom prefix). Though the end goal is different, the interface is not that
> >>>>>>> far off:
> >>>>>>>
> >>>>>>>
> >>>>>>> https://elixir.bootlin.com/linux/v6.19.3/source/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688
> >>>>>>>
> >>>>>>> Anyways, I knew this one would be an interesting one for upstream :)      
> >>>>>>
> >>>>>> For history buffs, we had a bunch of DDS chips in staging at one point and never
> >>>>>> manage to figure out the questions being raised here :(  They are complex
> >>>>>> beasts.  Clarity of ABI proposal and documentation is going to be key to driving
> >>>>>> this series forwards. In a sense the code is the easy part.      
> >>>>>
> >>>>> Does that mean that once good documentation is provided, the presented design can
> >>>>> be accepted? Even though data generators/sources might not be interpreted as
> >>>>> altvoltage channels?    
> >>>>
> >>>> I'm not sure yet :(  It's a pretty complex design and we haven't really come to a conclusion
> >>>> on how to handle this channel 'mixing' case.
> >>>>
> >>>> If we did go this way, we'd need to figure out a way to describe the mixing part.
> >>>> So either we describe it as one channel (which is going to be really complex)
> >>>> or we describe it as multiple channels but add extra ABI to make it clear they
> >>>> are mixed into a single 'physical' channel.
> >>>>
> >>>> Jonathan
> >>>>     
> >>>>>    
> >>>>     
> >>>
> >>> Some ideas have crossed my mind, like adding new option to the in_/out_
> >>> prefix for "internal" channels. But I it would take a long time to teach
> >>> existing generic userspace libraries/tools about this.
> >>>
> >>> What has popped into my head just now is that perhaps we could do like
> >>> Rodrigo is proposing here reusing existing channels and standard attributes
> >>> as much as possible and add a new "subcomponent_of" attribute to provide
> >>> the link, similar to "current_trigger" for triggers.
> >>>
> >>> This way, it would still work with existing userspace tools (even if it
> >>> looks a bit confusing). And userspace tools could eventually be taught
> >>> to present the channels as a tree-like structure with the main channel
> >>> and subcomponents nested under it.
> >>>
> >>> We would want to spell out up front what all of the anticipated ways of
> >>> using it are. For example, I suspect eventually someone will want this
> >>> attribute to be writeable to assign a specific limited resource to a
> >>> specific channel. An I expect that we would eventually see something were
> >>> a single subcomponent is shared between multiple physical channels. In
> >>> this case, we would want the value of the "subcomponent_of" attribute to
> >>> be able to be a list.  
> >>
> >> Something along those lines might work. I'd not thought about the case
> >> of one 'internal' going to multiple 'external'.  Otherwise I was wondering
> >> if something informal related to labels would work.  We've done that where
> >> we've been associating things like voltage and power measurement from a single
> >> pin.  It's rather adhoc though.  Possibly we could roll it into a newer
> >> more general scheme.  
> 
> Hmm... in this case, it sounds more flat where there isn't a clear channel
> that would be the "root" of a tree relation.
> 
> >>
> >> If the association is done as meta data attributes alongside existing
> >> channels then as you say existing tools will kind of work, just need some
> >> human understanding of what is actually being controlled until they catch
> >> up with the newer schemes.
> >>
> >> Lots of ways we could actually represent the graphs.  Going to take some
> >> figuring out!  
> > 
> > I like the idea of subchannels to create logical tree-structures. It opens
> > up for other possibilities.
> > 
> > I wonder if the varying channel index would still confuse a user, even
> > with this metadata attribute, like:
> > - out_altvoltage0
> > - out_altvoltage1
> > 	- out_altvoltage1_subcomponent_of = out_altvoltage0
> > 
> > would there be a different way to name the full_postfix in
> > __iio_device_attr_init() that would allow to keep channel index the same
> > (e.g. altvoltage0 for multiple internal sub-channel) and still be
> > compatible with userspace tools? I don't know, something like:
> > - out_altvoltage0
> > - out_altvoltage0_0
> > - out_altvoltage0_1
> > - out_altvoltage0_2
> > 
> > userspace tools would understand out_altvoltage0_0_frequency as:
> > - direction: out
> > - type: altvoltage
> > - channel idx: 0
> > - attr name: 0_frequency
> > 
> > and that would be a problem?
> >   
> 
> I have a feeling that could be problematic for existing attribute
> parsers.

Agreed.  This smells like extend_name and that caused all sorts
of annoying problems for the userspace folk.

> 
> How about using higher numbered channel indexes instead?
> 
> out_altvoltage100_* = physical channel
> 
> out_altvoltage101_* = subcomponent
> out_altvoltage102_* = subcomponent
> ...
Have to be careful we don't run out of space for events.
#define IIO_EVENT_CODE_EXTRACT_CHAN(mask) ((__s16)(mask & 0xFFFF))
So we do have 16 bits hence this might work.

I'm not sure we want to make rules around this though.

Jonathan


>