Re: [PATCH RFC v4 10/10] docs: iio: add documentation for ad9910 driver

[Rodrigo Alencar]

On 26/05/11 09:46AM, David Lechner wrote:
> On 5/10/26 4:30 AM, Rodrigo Alencar wrote:
> > On 26/05/09 06:42PM, David Lechner wrote:
> >> On 5/8/26 12:00 PM, Rodrigo Alencar via B4 Relay wrote:
> >>> From: Rodrigo Alencar <rodrigo.alencar@xxxxxxxxxx>
> >>>
> >>> Add documentation for the AD9910 DDS IIO driver, which describes channels,
> >>> DDS modes, attributes and ABI usage examples.
> > 
> > ...
> > 
> >>> +       must be a power of 2.
> >>> +
> >>> +   * - ``frequency_offset``
> >>> +     - Hz
> >>> +     - Base FTW to which scaled parallel data is added. Range :math:`[0, f_{SYSCLK}/2)`.
> >>> +
> >>> +   * - ``phase_offset``
> >>> +     - rad
> >>> +     - Base phase for polar modulation. Lower 8 bits of POW register.
> >>> +       Range :math:`[0, 2\pi/256)`.
> >>> +
> >>> +   * - ``scale_offset``
> >>> +     - fractional
> >>> +     - Base amplitude for polar modulation. Lower 6 bits of ASF register.
> >>> +       Range :math:`[0, 1/256)`.
> >>> +
> >>
> >> I guess there was some discussion on these attributes. I see some of these in the
> >> ad9832 driver in staging, but I'm guessing they are new ABI. It isn't clear to
> >> me from the documentation here what they actually do though. I guess they are
> >> just basic transformations on the input signal?
> > 
> > Not sure how the ABI is not clear:
> > 
> > 	For a channel that allows amplitude control through buffers, this
> > 	represents the value for a base amplitude scale. The actual output
> > 	amplitude scale is a result with the sum of this value.
> > 
> > So yes, it is a basic transformation.
> 
> I didn't have time to read the ABI docs yet. For scale_offset though,
> how is that different from the existing offset attribute?

I suppose that existing offset ABI is applied to (raw * scale), mostly for
voltage channels, here the scale_offset is an offset to the scale itself.
 
> > 
> >>
> >> And a practical note, they should be "frequencyscale". I don't like that it is
> >> harder to read, but it is easier for a machine to parse.
> > 
> > Parsers like the ones in libiio is not having problems with that.
> > 
> >>> +Usage examples
> >>> +^^^^^^^^^^^^^^
> >>> +
> >>> +Set parallel port frequency modulation with a scale of 16 and a 50 MHz
> >>> +offset:
> >>> +
> >>> +.. code-block:: bash
> >>> +
> >>> +  echo 16 > /sys/bus/iio/devices/iio:device0/out_altvoltage113_frequency_scale
> >>> +  echo 50000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage113_frequency_offset
> >>> +
> >>> +Digital ramp generator (DRG)
> >>> +----------------------------
> >>> +
> >>> +The DRG produces linear frequency, phase or amplitude sweeps using dedicated
> >>> +hardware. It is controlled through three channels: a parent control channel
> >>> +(``digital_ramp_generator``) and two child ramp channels
> >>> +(``digital_ramp_up``, ``digital_ramp_down``). DRG destination is set when
> >>> +ramp attributes are written, i.e. writing to ``frequency`` or ``frequency_roc``
> >>> +sets the destination to frequency.
> >>
> >> Would it be better to say that the destination is set when the the
> >> value is non-zero? Otherwise, how would one change the destination
> >> once set?
> > 
> > Destination is only one, so you just need to write phase or phase_roc, if you want
> > to target phase then. Does that not sound intuitive?
> 
> I was thinking about if you needed to change the configuration.
> If you set it to phase, then want to change it to frequency, how
> could you do that if 0 is a valid value for phase?
> 
> Also how could you know which is selected by reading back the
> values if 0 is a valid value?

This is where Jonathan raised some concerns, so it is a good oportunity for you
to provide your inputs! Right now, I am returning -EBUSY on read of an attribute
where its destination is not selected. As pointed out, the destination selection
is happening when writting to the attribute. In the previous patch, Jonathan
suggested frequency_active, phase_active and scale_active to track mode priority,
and It could be leveraged here for DRG destination selection. I havent gone for
that because I was not willing to add that to all the channels given that it is
mostly used for debugging, so I added frequency_source, phase_source and
amplitude_source to debugfs instead.

Destination selection for RAM mode is firmware based at this point.
Destination selection for Parallel mode is still not clear... could use
those *_active attributes or separate channels.

> > 
> > Zero is a valid value to be written.
> > 
> >>

-- 
Kind regards,

Rodrigo Alencar