Re: [PATCH v5 12/13] Documentation: ABI: testing: add docs for ad9910 sysfs entries

[Rodrigo Alencar]

On 26/05/17 07:37PM, Rodrigo Alencar via B4 Relay wrote:
> From: Rodrigo Alencar <rodrigo.alencar@xxxxxxxxxx>
> 
> Add custom ABI documentation file for the DDS AD9910 with sysfs entries to
> control Parallel Port, Digital Ramp Generator and OSK parameters.

...

> +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_dwell_en
> +KernelVersion:
> +Contact:	linux-iio@xxxxxxxxxxxxxxx
> +Description:
> +		For a channel that produces parametric sweeps, this attribute controls
> +		the sweep behavior at the configured limits. It enables dwell mode at a
> +		sweep limit when set to 1. Otherwise, the sweep may stop at the initial
> +		position or restart from that initial position or continue by reversing
> +		its direction.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency_roc
> +KernelVersion:
> +Contact:	linux-iio@xxxxxxxxxxxxxxx
> +Description:
> +		Frequency rate of change in Hz/s for channels that produce linear
> +		frequency sweeps. This value may be influenced by the channel's
> +		sampling_frequency setting.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_phase_roc
> +KernelVersion:
> +Contact:	linux-iio@xxxxxxxxxxxxxxx
> +Description:
> +		Phase rate of change in rad/s for channels that produce linear
> +		phase sweeps. This value may be influenced by the channel's
> +		sampling_frequency setting.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_scale_roc
> +KernelVersion:
> +Contact:	linux-iio@xxxxxxxxxxxxxxx
> +Description:
> +		Amplitude scale rate of change in 1/s for channels that ramp
> +		amplitude. This value may be influenced by the channel's
> +		sampling_frequency setting.

I am thinking about adopting IIO_FREQUENCY and IIO_PHASE to the DRG control
(It is considered for the Parallel port at this point).
That would make the destination configuration a bit more clearer:

* out_frequencyY_en
* out_phaseY_en
* out_altcurrentY_en

which would enable DRG for a given destination.

I would not want to have 6 channels (3 for ramp up and 3 for ramp down),
so I would develop the idea here with that in mind. Otherwise, it may
be ok to have the 6 channels for that.

The DRG have this output pin called DROVER, which indicates when the ramp reaches
a limit (upper or lower), then I was thinking on bring that to be an iio event.
At least initially, to treat the configuration as threshold event control, and not
necessarily having to processs the DROVER interrupt. Mostly because that interrupt
might be to frequent and would be better handled in hardware (by an FPGA IP).
In fact, for certain configurations (slower ramps), CPU would be able to handle
it just fine, specially if user is not very much concerned about deterministic
timing when handling the event.

DROVER indicates both threshold rising and falling events, so I would have something
like:

* events/out_frequencyY_thresh_rising_value
* events/out_frequencyY_thresh_falling_value
* events/out_phaseY_thresh_rising_value
* events/out_phaseY_thresh_falling_value
* events/out_altcurrentY_thresh_rising_value
* events/out_altcurrentY_thresh_falling_value

to configure the DRG limits (upper and lower), which fits well with the idea that
DROVER is the event we are interested in.

Now, we are missing two things: step rate (sampling_frequency) and step (configured with roc).
The problem here is that both of those configuration are applicable to rising (ramp up) and
falling (ramp down) cases.

There is no RoC threshold event, so I am not sure it would make sense to create an
event spec to configure the step with:

* events/out_frequencyY_roc_rising_value
* events/out_frequencyY_roc_falling_value
* events/out_phaseY_roc_rising_value
* events/out_phaseY_roc_falling_value
* events/out_altcurrentY_roc_rising_value
* events/out_altcurrentY_roc_falling_value

But that would allow to reuse that existing iio core/event code to configure the
ramp up/down step with new ABI.
Otherwise, trying to get both ramp up/down configs into a single channel we would
need:

* directly to the device channel attr group:
	* out_frequencyY_rising_roc and out_frequencyY_falling_roc
	* out_phaseY_rising_roc and out_phaseY_falling_roc
	* out_altcurrentY_rising_roc and out_altcurrentY_falling_roc
* or, under the event group with a new event info: IIO_EV_INFO_ROC 
	* events/out_frequencyY_thresh_rising_roc
	* events/out_frequencyY_thresh_falling_roc
	* events/out_phaseY_thresh_rising_roc
	* events/out_phaseY_thresh_falling_roc
	* events/out_altcurrentY_thresh_rising_roc
	* events/out_altcurrentY_thresh_falling_roc 

For the sampling_frequency configuration, which would also require new
ABI to fit both ramp up/down in a single channel:

* directly to the device channel attr group:
        * out_frequencyY_rising_sampling_frequency and out_frequencyY_falling_sampling_frequency
        * out_phaseY_rising_sampling_frequency and out_phaseY_falling_sampling_frequency
        * out_altcurrentY_rising_sampling_frequency and out_altcurrentY_falling_sampling_frequency
* or, under the event group with a new event info: IIO_EV_INFO_SAMP_FREQ   
        * events/out_frequencyY_thresh_rising_sampling_frequency
        * events/out_frequencyY_thresh_falling_sampling_frequency
        * events/out_phaseY_thresh_rising_sampling_frequency
        * events/out_phaseY_thresh_falling_sampling_frequency
        * events/out_altcurrentY_thresh_rising_sampling_frequency
        * events/out_altcurrentY_thresh_falling_sampling_frequency

Lastly, we also have the dwell enable config:

* directly to the device channel attr group:
        * out_frequencyY_rising_dwell_en and out_frequencyY_falling_dwell_en
        * out_phaseY_rising_dwell_en and out_phaseY_falling_dwell_en
        * out_altcurrentY_rising_dwell_en and out_altcurrentY_falling_dwell_en
* or, under the event group with a new event info: IIO_EV_INFO_DWELL_EN
        * events/out_frequencyY_thresh_rising_dwell_en
        * events/out_frequencyY_thresh_falling_dwell_en
        * events/out_phaseY_thresh_rising_dwell_en
        * events/out_phaseY_thresh_falling_dwell_en
        * events/out_altcurrentY_thresh_rising_dwell_en
        * events/out_altcurrentY_thresh_falling_dwell_en

For custom event attributes, maybe iio_info.event_attrs can be used instead
of new event info (or a mix of both).
Another thing we may consider if going for the 6 channel case, would be having
new modifiers: IIO_MOD_FALLING and IIO_MOD_RISING, which might simplify things
a bit.

Let me know about your thoughts on this.

-- 
Kind regards,

Rodrigo Alencar