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

["Nuno Sá" <noname.nuno@gmail.com>]

On Mon, 2026-03-09 at 09:52 +0000, Rodrigo Alencar wrote:
> On 26/03/08 06:12PM, Jonathan Cameron wrote:
> > 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.
> 
> ...
> 
> > > > > > > > > > > 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 :).
> 
> ...
> 
> > > > > > > > > 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.
> > > > > > > 
> 
> ...
> 
> > > > > > 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.
> 
> Ok, I think we are reaching an agreement. With this, I think there
> is room for a small redesign, i.e., having a root channel to represent
> the physical DAC and child channels as follows (* on custom ABI):
> - out_altvoltage100: root/physical channel
> 	- profile*: for profile selection
> 	- sampling_frequency: sysclk frequency value
> 	- out_altvoltage110: single tone channel
> 		- frequency: single tone profile FTW
> 		- phase: single tone profile POW
> 		- scale: single tone profile ASF
> 	- out_altvoltage120: parallel port channel
> 		- en
> 		- frequency_scale* or frequency_gain*: for FM gain 
> 		- frequency or frequency_offset*
> 		- phase or phase_offset*
> 		- scale or scale_offset*
> 	- out_altvoltage130: drg channel
> 		- en
> 		- destination*
> 		- operating_mode*
> 		- out_altvoltage131: drg upper limit channel
> 			- frequency
> 			- frequency_step*
> 			- phase
> 			- phase_step*
> 			- scale
> 			- scale_step*
> 			- sampling_frequency
> 		- out_altvoltage132: drg lower limit channel
> 			- frequency
> 			- frequency_step*
> 			- phase
> 			- phase_step*
> 			- scale
> 			- scale_step*
> 			- sampling_frequency
> 	- out_altvoltage140: ram channel
> 		- en
> 		- frequency: global FTW
> 		- phase: global POW
> 		- sampling_frequency
> 		- destination*
> 		- operating_mode*
> 		- address_start*
> 		- address_end*
> 	- out_altvoltage150: osk channel
> 		- en
> 		- scale: global ASF
> 		- scale_step*
> 		- sampling_frequency
> 		- pinctrl_en*
> 
> so I ended up with 8 channels (rather than the initial 5). Breaking up the
> DRG channel, following the tree structure allows to explore more of the
> standard ABI. I suppose that even with this, documentation is no less
> important. How does it look?

FWIW, I kind of like this "child" channel concept and think it might be useful
for other things to. One thing that comes to mind is how do we want to handle this
in terms of scan_elements? Do we have it completely independent or do we want to play some
games with it? Like allowing arbitrary sizes (not power of 2) in child channels as long as the
sum matches what was defined in the parent/physical channel.

One usecase for the above would be the ad4030 ADC where we went with two channels (for the common
mode voltage) for one physical one and we define two scan elements where the total storage does
not directly map the actual physical size. And while we can get away with it for SW buffers, for HW
buffers it's not "so simple".

Can be a bit tricky to implement but the above concept might open the door for it.

- Nuno Sá