[v2,0/2] iio: adc: ad4695: implement triggered buffer

Message

David Lechner Aug. 13, 2024, 5:26 p.m. UTC

This is a fairly simple series that adds support for triggered buffers
to the ad4695 driver.

Not directly related to this patch, but as a side discussion about
future possibilities with this chip while we are here...

The advanced sequencer on this chip can repeat the same channel multiple
times which, when combined with the autocycle feature, can be used to
create different effective sampling rates for individual channels.

For example if we set up the sequence [IN1, IN2, IN1, IN3] and the time
between each individual sample in the sequence is the same, then IN1 has
an effective sampling rate of 2x the other channels.

Have there ever been discussions before about implementing something
like this in the IIO subsystem? I didn't see anything that looked like
this already implemented in the kernel.

---
Changes in v2:
v2 changes:
* Don't error when only temperature channel is enabled, just add extra
  xfer instead.
* Drop use of reg/mask/val local variables.
* Use struct initializer for xfer in ad4695_exit_conversion_mode().
* Simplify return at end ad4695_buffer_preenable().
* Use t_ACQ instead of t_CNVL to ensure proper acquisition time.
* The temperature channel changes weren't quite trivial, so I didn't
  pick up Nuno's Reviewed-by.
* Link to v1: https://lore.kernel.org/r/[e-mail]

---
David Lechner (2):
      iio: adc: ad4695: implement triggered buffer
      doc: iio: ad4695: document buffered read

 Documentation/iio/ad4695.rst |   9 +-
 drivers/iio/adc/ad4695.c     | 250 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 255 insertions(+), 4 deletions(-)
---
base-commit: d505a9dc8de83df2a8ce8c9e780f7b29887f34af
change-id: 20240807-iio-adc-ad4695-buffered-read-f49eb511e300

Best regards,

Comments

Jonathan Cameron Aug. 17, 2024, 2:17 p.m. UTC | #1

On Tue, 13 Aug 2024 12:26:39 -0500
David Lechner <[e-mail]> wrote:

> This is a fairly simple series that adds support for triggered buffers
> to the ad4695 driver.
> 
> Not directly related to this patch, but as a side discussion about
> future possibilities with this chip while we are here...
> 
> The advanced sequencer on this chip can repeat the same channel multiple
> times which, when combined with the autocycle feature, can be used to
> create different effective sampling rates for individual channels.
> 
> For example if we set up the sequence [IN1, IN2, IN1, IN3] and the time
> between each individual sample in the sequence is the same, then IN1 has
> an effective sampling rate of 2x the other channels.
> 
> Have there ever been discussions before about implementing something
> like this in the IIO subsystem? I didn't see anything that looked like
> this already implemented in the kernel.

Yes.  There has been discussion in the past, but it's hard to define
an ABI around so we have either given up, or had the driver unwind the
data flow into multiple buffers.  There are some IMUs that do tagged
data in their hardware fifos and you can configure the sampling frequencies
for sets of channels independently.

So you get

1 tag_accel :  accel_x accel_y accel_z
2 tag_accel :  accel_x accel_y accel_z
3 tag_magn : magn_x, magn_y magn_z
4 tag_accel :  accel_x accel_y accel_z
5 tag_accel :  accel_x accel_y accel_z
6 tag_magn : magn_x, magn_y magn_z
7 tag_accel : accel_x accel_y accel_z

The driver then pushes to multiple buffers.

accel_buffer:
1 tag_accel :  accel_x accel_y accel_z
2 tag_accel :  accel_x accel_y accel_z
4 tag_accel :  accel_x accel_y accel_z
5 tag_accel :  accel_x accel_y accel_z
7 tag_accel : accel_x accel_y accel_z

magn_buffer: 
3 tag_magn : magn_x, magn_y magn_z
6 tag_magn : magn_x, magn_y magn_z

etc.

See imu/st_lsm6dsx/st_lsm6dsx_buffer.c
and st_lsm6dsx_push_tagged_data()

I think we discussed more complex setups where channels could be offered
for different buffers, each with own sampling rate and the driver (note not
the IIO core) would try to unwind what you'd asked for into each buffer +
figure out how to configure the device.  Don't think that got implemented
though as the simple multibuffer solution was enough. 

Note this predated per iio_dev multiple buffer support so it was done with additional
IIO devices.

Otherwise we do have option to do repeat scan types. That works for
IN0 IN0 IN0 IN0 IN1 IN0 IN0 IN0 IN0 IN1 IN0 etc

That is really meant for multi value channels - e.g. quarternions where
a single value on it's own is meaningless (you need the scaling across
all 4 elements).

Jonathan

> 
> ---
> Changes in v2:
> v2 changes:
> * Don't error when only temperature channel is enabled, just add extra
>   xfer instead.
> * Drop use of reg/mask/val local variables.
> * Use struct initializer for xfer in ad4695_exit_conversion_mode().
> * Simplify return at end ad4695_buffer_preenable().
> * Use t_ACQ instead of t_CNVL to ensure proper acquisition time.
> * The temperature channel changes weren't quite trivial, so I didn't
>   pick up Nuno's Reviewed-by.
> * Link to v1: https://lore.kernel.org/r/[e-mail]
> 
> ---
> David Lechner (2):
>       iio: adc: ad4695: implement triggered buffer
>       doc: iio: ad4695: document buffered read
> 
>  Documentation/iio/ad4695.rst |   9 +-
>  drivers/iio/adc/ad4695.c     | 250 ++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 255 insertions(+), 4 deletions(-)
> ---
> base-commit: d505a9dc8de83df2a8ce8c9e780f7b29887f34af
> change-id: 20240807-iio-adc-ad4695-buffered-read-f49eb511e300
> 
> Best regards,

Jonathan Cameron Aug. 17, 2024, 2:28 p.m. UTC | #2

On Tue, 13 Aug 2024 12:26:39 -0500
David Lechner <[e-mail]> wrote:

> This is a fairly simple series that adds support for triggered buffers
> to the ad4695 driver.
> 
Applied to the togreg branch of iio.git and pushed out as testing for
all the normal reasons...

Thanks,

Jonathan