Re: [PATCH v3 3/4] iio: adc: ad4691: add triggered buffer support

[David Lechner <dlechner@baylibre.com>]

On 3/16/26 8:22 AM, Sabau, Radu bogdan wrote:
> 
> 
>> -----Original Message-----
>> From: David Lechner <dlechner@baylibre.com>
>> Sent: Saturday, March 14, 2026 8:38 PM
> 
> ...
> 
>>> Both operating modes share a single IIO trigger and trigger handler.
>>> The handler builds a complete scan — one u32 slot per channel at its
>>> scan_index position, followed by a timestamp — and pushes it to the
>>> IIO buffer in a single iio_push_to_buffers_with_ts() call.
>>
>> It would really help here to see some timing diagrams to know if we
>> are implementing this right.
>>
>> For example, it isn't clear that in clocked mode if CNV triggers a
>> single conversion in the sequencer (i.e. IIO_SAMP_FREQ should be
>> info_mask_separate) or if it triggers the sequence (i.e. IIO_SAMP_FREQ
>> should be info_mask_shared_by_all).
>>
> 
> The CNV triggers the sequence and IIO_SAMP_FREQ is info_mask_shared_by_all.
> 
> As per datasheet page 31 (Accumulator Section), when each accumulator
> receives a sample, the ACC_COUNT is increased. In clocked mode we
> are setting the ACC_COUNT limit to 1, therefore having one sample per
> channel (no oversampling as discussed in previous versions). So each
> period of the CNV PWM is respective to one sample of a channel.

Assuming that "a" channel means "one" channel...

In this case then sampling_frequency should be per channel (separate).

A sampling_frequency that is shared_by_all means that each period of
CNV should trigger one sample each for _all_ channels. In other words,
the sampling frequency gives one complete set of samples for all enabled
channels pushed to the buffer.

> 
>>>
>>> For CNV Clock Mode the GP0 pin is configured as DATA_READY output. The
>>> IRQ handler stops conversions and fires the IIO trigger; the trigger
>>> handler reads accumulated results from the AVG_IN registers via regmap
>>> and restarts conversions for the next cycle.
>>
>> This seems OK, but I would kind of would expect that PWM as CNV to
>> only be used for SPI offloading and not without SPI offloading.
>>
>> The ADC also has an internal oscillator, so it seems like it would
>> be more useful to use that as a conversion trigger rather than
>> requiring external hardware.
>>
> 
> This CNV is used in triggered buffer mode as well, not only in offload.
> In this mode, CNV replaces the internal oscillator so CNV is the
> conversion trigger (offload or not), which also introduces the advantage
> of having a more flexible sampling rate.

Yes, I understand that. We just never did that for any other chip yet.
Usually, we would just use the internal oscillator on the chip instead
for this sort of thing. But if you have applications engineers telling
you that this is a setup they want to support, then we can do it.

>>>
>>> Manual mode channels use storagebits=32 (shift=8, realbits=16) so all
>>> channel slots in the scan buffer are uniformly sized regardless of the
>>> SPI wire format (24-bit transfer, 16-bit ADC data in bits[23:8]).
>>
>> I also don't understand why we are including the status bits in manual
>> mode but not in CNV clock mode.
>>
> 
> In Manual Mode, status bits are received through SPI, because that's how
> the hardware works. However, they are masked by the driver and thus not used.

Usually there are registers to turn status on and off independently. If
there isn't it could be helpful to add some comments in the code to
remind us.