Re: [PATCH 1/5] Documentation: ABI: IIO: add calibphase_delay documentation

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 1 May 2025 09:44:51 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 5/1/25 7:33 AM, Angelo Dureghello wrote:
> > On 30.04.2025 10:04, David Lechner wrote:  
> >> On 4/30/25 9:56 AM, David Lechner wrote:  
> >>> On 4/30/25 9:45 AM, Andy Shevchenko wrote:  
> >>>> On Wed, Apr 30, 2025 at 09:21:28AM -0500, David Lechner wrote:  
> >>>>> On 4/30/25 12:40 AM, Nuno Sá wrote:  
> >>>>>> On Tue, 2025-04-29 at 15:06 +0200, Angelo Dureghello wrote:  
> >>>>>>> From: Angelo Dureghello <adureghello@baylibre.com>
> >>>>>>>
> >>>>>>> Add new IIO calibphase_delay documentation.
> >>>>>>>
> >>>>>>> The delay suffix is added to specify that the phase, generally in
> >>>>>>> radiants, is for this case (needed from ad7606) in nanoseconds.  
> >>>>
> >>>> ...
> >>>>  
> >>>>>>> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_calibphase_delay  
> >>>>>>
> >>>>>> Not sure if I'm too convinced on the _delay suffix
> >>>>>>  
> >>>>> Phase is measured in radians, not seconds, so it seems wrong to use it here.
> >>>>>
> >>>>> https://en.wikipedia.org/wiki/Phase_(waves)
> >>>>>
> >>>>> And the delay here is with respect to individual samples in a simultaneous
> >>>>> conversion without regard for a sampling frequency, so I don't see how we could
> >>>>> convert the time to radians in any meaningful way.  
> >>>>
> >>>> And how this delay is aplicable to the phase in the hardware? Sounds to me that
> >>>> HW has some meaningful way of such a conversion?
> >>>>  
> >>>
> >>> It is a calibration to account for a phase difference between two input signals.
> >>> This is a simultaneous sampling ADC, so all channels normally sample at exactly
> >>> the same time. This phase delay calibration factor can introduce a small delay
> >>> on an individual channel so that it starts it's conversion some microseconds
> >>> after the others.
> >>>
> >>> There is a nice diagram here:
> >>>
> >>> https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-18.pdf#%5B%7B%22num%22%3A113%2C%22gen%22%3A0%7D%2C%7B%22name%22%3A%22XYZ%22%7D%2C34%2C594%2C0%5D
> >>>
> >>> To convert the phase delay to a phase angle and back would require also knowing
> >>> the frequency of the input voltage signals.  
> >>
> >> Maybe calling it "conversion delay" would make more sense? Since the phase part
> >> of it is really referring to the application rather than to what we are actually
> >> adjusting.  
> > 
> > Are there examples of a phase calibration in iio ? Becouse apply a radians 
> > calibration seems complicated and maybe non approrpiate for non-periodic 
> > signals as often used in real world applications.
> > 
> > So another viable idea could be to use a IIO_CHAN_INFO_CALIBDELAY instead.
> > 
> > Regards,
> > angelo  
> 
> I was looking at the datasheet on another ADC that popped up on the mailing list
> today. https://www.ti.com/product/ADS1262
> 
> It has a "conversion delay" register that does basically the same thing. So I'm
> liking that name even more now. Just calling it "delay" seems a bit too vague.
> We could make it IIO_CHAN_INFO_CALIBCONV_DELAY to try to keep it shorter.

This is wondering into a long term gap in IIO ABI.  Even if we ignore this particular
usecase, it could potentially be useful to indicate the timing offsets between the
sampling of specific channels.  In simultaneous sampling case we'd normally assume
0 (subject to tweaks like this one) whereas in a sequencer type situation it can get
complex.  IIRC there are sequencers that allow insertions of extra delays as well
as the simpler ones where it is just dependent on the previous channel sampling times.
What you have here is a relatively simple time delay control but I'd like
to cover the full gamut of things we might see.

To me it's really not got much to do with calibration so I'd drop the calib bit.
Define a baseline for all channels (which should probably be an arbitrary period
after trigger as measuring trigger to first sample gets complicated in some devices.)

Anyhow, I'll comment on v2 and point back at this.