Re: Questions about ad5933 driver

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 13 Jun 2023 12:37:39 +0200
David Schiller <david.schiller@jku.at> wrote:

> On Wed, 2023-06-07 at 15:52 +0100, Jonathan Cameron wrote:
> > Not 'official' though it is fairly commonly used, but the documented
> > ABI in
> > Documentation/ABI/testing/sysfs-bus-iio*
> > is and these aren't there either.
> > 
> > I'm not 100% sure this is the right way to solve this ABI gap though
> > so need to have a bit of a think about it.  Using a modifier means we
> > can't
> > use them for anything else, so need to consider if there are other
> > modifiers
> > (or it has meaning for differential channels) when deciding if this is
> > an ABI we want to add.  
> 
> The naming of the modifiers is also somewhat confusing. The HW registers
> represent the DFT of the input signal, which together with a gain factor
> is used to calculate the impedance. Dimensionally the quantities are
> admittances. "voltage_real" and "voltage_imag" are therefore not quite
> fitting.

In that case, sounds like we need a new channel type to correctly reflect
that they are admittance rather than pretending they are voltages.

> 
> Also the sweep setup commands are only run once per buffer (in
> ad5933_ring_preenable). This means a buffer can only be used for one
> sweep and then has to be disabled and enabled again. As the driver does
> not implement any triggers this is right now the only way to start a
> measurement.

This sounds a bit like the impact sensors - where an event triggers a series
of measurements of 'something'.

> 
> Do you think it would make sense to implement a user-space trigger to
> start a measurement sweep? This would mean that a buffer can be reused.
> Right now the "iio_readdev" test program from libiio does not work due
> to this behavior. I don't know how this is handled on other IIO drivers.

A single trigger normally results in the capture of a single set
of channels measurements.  Here that's sort of the case, but the set of
channels is huge - because each one corresponds to a particular frequency.
There can I think be up to 512 such points.  It would be messy to represent
so many channels (if nothing else some of the ways we store data in IIO -
particularly events, are limited to 256 channels).

My guess is that the calculations around a sweep are sufficiently complex
that, if libiio handled the re setup of a sweep neatly, it would be fine
to go through the disable / reenable of the buffer?
I think bodging in trigger support where a trigger causes 'N' samples would
be tricky.

There is another option though I'm not sure how closely it fits.
A channel can have a scan_type element of repeats which could be set
to a fixed value of 512 (likely all scan_type things it's not runtime
configurable) as this might be considered to be lots of repeated reads
of a single channel.


> 
> Lastly, should I CC the original author (Michael Hennerich) going
> forward?

Sure - +CC Michael.

> 
> Thanks!
> 
>