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

[David Lechner]

On 3/16/26 10:56 AM, Sabau, Radu bogdan wrote:
> 
> 
>> -----Original Message-----
>> From: David Lechner <dlechner@xxxxxxxxxxxx>
>> Sent: Monday, March 16, 2026 5:38 PM
>> To: Sabau, Radu bogdan <Radu.Sabau@xxxxxxxxxx>; Lars-Peter Clausen
>> <lars@xxxxxxxxxx>; Hennerich, Michael <Michael.Hennerich@xxxxxxxxxx>;
>> Jonathan Cameron <jic23@xxxxxxxxxx>; Sa, Nuno <Nuno.Sa@xxxxxxxxxx>;
>> Andy Shevchenko <andy@xxxxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>;
>> Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley
>> <conor+dt@xxxxxxxxxx>; Uwe Kleine-König <ukleinek@xxxxxxxxxx>; Liam
>> Girdwood <lgirdwood@xxxxxxxxx>; Mark Brown <broonie@xxxxxxxxxx>; Linus
>> Walleij <linusw@xxxxxxxxxx>; Bartosz Golaszewski <brgl@xxxxxxxxxx>; Philipp
>> Zabel <p.zabel@xxxxxxxxxxxxxx>
>> Cc: linux-iio@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
>> kernel@xxxxxxxxxxxxxxx; linux-pwm@xxxxxxxxxxxxxxx; linux-
>> gpio@xxxxxxxxxxxxxxx
>> Subject: Re: [PATCH v3 3/4] iio: adc: ad4691: add triggered buffer support
>>
>> [External]
>>
>> On 3/16/26 8:22 AM, Sabau, Radu bogdan wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: David Lechner <dlechner@xxxxxxxxxxxx>
>>>> 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.
>>
> 
> Oh, ok then, will have them separate. I assumed that since the PWM period
> is constant with each pulse, then the sampling rate will be the same for
> each channel, thus having them as shared_by_all, but I assume you are
> right about this in this case, I will have them as separate in this case, the
> update will happen in the previous patch upon next version.
> 
Does the sampling stop after one "burst" (reading each enabled channel once)?

If yes, then what controls when the next set of samples starts?

Looking at Figure 63 in the datasheet for CNV Clock mode, it looks like it
depends entirely on how long the SPI message takes. So the actual sample rate
is going to be quite random instead of the sum of each channel as the IIO ABI
says it should. It seems a waste of the PWM to do it this way since we end
up with a random sample rate.

It seems to me like the CNV Burst mode would actually be better suited to
how IIO usually does things. In this case, the PWM frequency would control
the effective sample rate (one PWM pulse triggers one complete set of
samples) and the internal oscillator controls triggering each individual
conversion.

In this setup, we would still have the info_mask_separate IIO_SAMP_FREQ,
but it would control the internal oscillator. Then we would have a separate
buffer0/sampling_frequency attribute that controlled the PWM frequency.

Then, as long as the PWM frequency was slow enough that the SPI message
can be done, it can make samples with almost no jitter. This is why I would
expect PWM to almost always be used with SPI offload though, otherwise
it has to be quite slow compared to what the chip is capable of.

I suppose the CNV Clock mode could also be made to work with the typical
IIO trigger so that we could control the actual sample rate. It just
wouldn't be as precise.



If you have some examples of how this chip should actually be used in the
real world, that could help pick what is the right thing to do here.