linux-kernel - Re: [PATCH v2 0/2] iio: adc: ad4695: implement triggered buffer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240817151709.7aa1bf1a@jic23-huawei>
Date: Sat, 17 Aug 2024 15:17:09 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Michael Hennerich <Michael.Hennerich@...log.com>, Nuno Sá
 <nuno.sa@...log.com>, Jonathan Corbet <corbet@....net>,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 0/2] iio: adc: ad4695: implement triggered buffer

On Tue, 13 Aug 2024 12:26:39 -0500
David Lechner <dlechner@...libre.com> 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/20240807-iio-adc-ad4695-buffered-read-v1-0-bdafc39b2283@baylibre.com
> 
> ---
> 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,