Following up from this thread [1]...
Unless I've overlooked something important, I think adding support for
multiple scan types per channels should be rather trivial, at least in
the kernel. Userspace tools will need to learn to re-read buffer _type
attributes though. For example, it looks like libiio caches these values.
I had to restart iiod to get a proper capture with the iio-oscilloscope
after changing the scan type at runtime.
Here is a bit of background for those not following along:
Up to now, the IIO subsystem has only supported a single scan type per
channel. This scan type determines the binary format of the data in the
buffer when doing buffered reads.
For simple devices, there is only one scan type and all is well. But
for more complex devices, there may be multiple scan types. For example,
ADCs with a resolution boost feature that adds more bits to the raw
sample data. Traditionally, for slow devices, we've just always used the
highest resolution mode, but for high performance ADCs, this may not be
always practical. Manipulating data after every read can hurt performance
and in the case of hardware buffers, it may not be possible to change the
format of the data in the buffer at all. There are also ADCs where
enabling the higher resolution can only be done if oversampling is also
enabled which may not be desireable.
To allow for more flexibility, we would like to add support for multiple
scan types per channel.
To avoid having to touch every driver, we implemented this in a way that
preserves the existing scan_type field. See the "iio: add support for
multiple scan types per channel" the details. The first couple of patches
are just preparation for this.
The last patch shows how to use this new feature in the ad7380 driver
which is still under review at [2].
[1]: https://lore.kernel.org/linux-iio/CAMknhBHOXaff__QyU-wFSNNENvs23vDX5n_ddH-Dw3s6-sQ9sg@mail.gmail.com/
[2]: https://lore.kernel.org/linux-iio/[email protected]/T/#t
---
David Lechner (4):
iio: introduce struct iio_scan_type
iio: buffer: use struct iio_scan_type to simplify code
iio: add support for multiple scan types per channel
iio: adc: ad7380: add support for multiple scan type
drivers/iio/adc/ad7380.c | 185 ++++++++++++++++++--------------------
drivers/iio/industrialio-buffer.c | 77 +++++++++++-----
include/linux/iio/iio.h | 74 +++++++++++----
3 files changed, 194 insertions(+), 142 deletions(-)
---
base-commit: 32cf3c865729172e67dced11c0b73e658444888a
change-id: 20240507-iio-add-support-for-multiple-scan-types-f4dbcf4c2cb8
This gives the channel scan_type a named type so that it can be used
to simplify code in later commits.
Signed-off-by: David Lechner <[email protected]>
---
include/linux/iio/iio.h | 41 ++++++++++++++++++++++-------------------
1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 55e2b22086a1..19de573a944a 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -173,6 +173,27 @@ struct iio_event_spec {
unsigned long mask_shared_by_all;
};
+/**
+ * struct iio_scan_type - specification for channel data format in buffer
+ * @sign: 's' or 'u' to specify signed or unsigned
+ * @realbits: Number of valid bits of data
+ * @storagebits: Realbits + padding
+ * @shift: Shift right by this before masking out realbits.
+ * @repeat: Number of times real/storage bits repeats. When the
+ * repeat element is more than 1, then the type element in
+ * sysfs will show a repeat value. Otherwise, the number
+ * of repetitions is omitted.
+ * @endianness: little or big endian
+ */
+struct iio_scan_type {
+ char sign;
+ u8 realbits;
+ u8 storagebits;
+ u8 shift;
+ u8 repeat;
+ enum iio_endian endianness;
+};
+
/**
* struct iio_chan_spec - specification of a single channel
* @type: What type of measurement is the channel making.
@@ -184,17 +205,6 @@ struct iio_event_spec {
* @scan_index: Monotonic index to give ordering in scans when read
* from a buffer.
* @scan_type: struct describing the scan type
- * @scan_type.sign: 's' or 'u' to specify signed or unsigned
- * @scan_type.realbits: Number of valid bits of data
- * @scan_type.storagebits: Realbits + padding
- * @scan_type.shift: Shift right by this before masking out
- * realbits.
- * @scan_type.repeat: Number of times real/storage bits repeats.
- * When the repeat element is more than 1, then
- * the type element in sysfs will show a repeat
- * value. Otherwise, the number of repetitions
- * is omitted.
- * @scan_type.endianness: little or big endian
* @info_mask_separate: What information is to be exported that is specific to
* this channel.
* @info_mask_separate_available: What availability information is to be
@@ -245,14 +255,7 @@ struct iio_chan_spec {
int channel2;
unsigned long address;
int scan_index;
- struct {
- char sign;
- u8 realbits;
- u8 storagebits;
- u8 shift;
- u8 repeat;
- enum iio_endian endianness;
- } scan_type;
+ struct iio_scan_type scan_type;
long info_mask_separate;
long info_mask_separate_available;
long info_mask_shared_by_type;
--
2.43.2
On Tue, 2024-05-07 at 14:02 -0500, David Lechner wrote:
> Following up from this thread [1]...
>
> Unless I've overlooked something important, I think adding support for
> multiple scan types per channels should be rather trivial, at least in
> the kernel. Userspace tools will need to learn to re-read buffer _type
> attributes though. For example, it looks like libiio caches these values.
> I had to restart iiod to get a proper capture with the iio-oscilloscope
> after changing the scan type at runtime.
No for now but to add more future fun, we may consider in having something
similar as hwmon [1]. Hence, userspace could do things like poll(2) on the
specific file rather than having to read it over and over...
[1]: https://elixir.bootlin.com/linux/latest/source/drivers/hwmon/hwmon.c#L649
- Nuno Sá
On Tue, 21 May 2024 11:18:24 +0200
Nuno Sá <[email protected]> wrote:
> On Tue, 2024-05-07 at 14:02 -0500, David Lechner wrote:
> > Following up from this thread [1]...
> >
> > Unless I've overlooked something important, I think adding support for
> > multiple scan types per channels should be rather trivial, at least in
> > the kernel. Userspace tools will need to learn to re-read buffer _type
> > attributes though. For example, it looks like libiio caches these values.
> > I had to restart iiod to get a proper capture with the iio-oscilloscope
> > after changing the scan type at runtime.
>
> No for now but to add more future fun, we may consider in having something
> similar as hwmon [1]. Hence, userspace could do things like poll(2) on the
> specific file rather than having to read it over and over...
>
> [1]: https://elixir.bootlin.com/linux/latest/source/drivers/hwmon/hwmon.c#L649
> - Nuno Sá
>
It would take a well reasoned usecase to convince me sysfs notifications
are useful in cases where an explicit userspace action caused the value that
would be read from another file to change immediately.
Jonathan