Message-ID: <20200508135348.15229-1-alexandru.ardelean@analog.com>
Date:   Fri, 8 May 2020 16:53:34 +0300
From:   Alexandru Ardelean <alexandru.ardelean@...log.com>
To:     <linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC:     <jic23@...nel.org>, <lars@...afoo.de>,
        Alexandru Ardelean <alexandru.ardelean@...log.com>
Subject: [RFC PATCH 00/14] iio: buffer: add support for multiple buffers

I blew a few fuses trying to get to this combination.
And I still don't like this one, but it works.

Some patches here [like Lars' patches] are here just make things easier
to apply when moving from the ADI tree to the upstream one.
For a final patchset I'll need to re-apply/re-test.

A working version of this patchset but also with output buffer support
is here:
 https://github.com/analogdevicesinc/linux/tree/adi-iio-buffers-work
[ it's based on 4.19 ]

It will take a few more patches to bring output support to this tree,
but this should add initial support for multiple IIO buffers per device.
I haven't tested this patchset, I've tested the one in the link above.
I currently don't have [at hand] any devices that would benefit from
using multiple buffers per IIO device. Maybe when I get back to the
office.

Firstly: I need a bit of help in trying to symlink from the kernel
files in the /dev folder. I could not find any examples of how this is
done [without cracking open some deep kernel/kernfs code], so maybe it's
done via userspace?
Any suggestions are welcome.

I noticed that symlinking via sysfs_create_link() is fine outside of the
fs/ folder, so that's what I used.

I think I also partially understand why Lars was initially against doing
this 'multiple IIO buffers per IIO device'.
I've tried various combinations of doing things, and nothing fits
perfect, without feeling like I'm doing some sort of mini-frankenstein,
just in order to preserve some backwards compatibility.

So, to make this work, what I need to do now, is:
------------
root@...log:~# cd /dev/
root@...log:/dev# ln -s iio\:buffer3\:0 iio\:device3
root@...log:/dev# ln -s iio\:buffer4\:0 iio\:device4
root@...log:/dev# ls | grep iio
iio:buffer3:0
iio:buffer3:1
iio:buffer3:2
iio:buffer3:3
iio:buffer4:0
iio:device0
iio:device1
iio:device3
iio:device4
------------

device3 is an ADC with 4 buffers attached, only the first one is
currently being used (in legacy mode); the other 3 aren't being used.

The IIO buffer devices are named '/dev/iio:bufferX:Y', X being the
ID of the IIO device, and Y being the ID of the buffer of device X.
These could be renamed to '/dev/iio:deviceX:Y', but that would be
confusing I guess.

Then
------------
root@...log:/# ls /sys/bus/iio/devices/
iio:buffer3:0  iio:buffer3:1  iio:buffer3:2  iio:buffer3:3  iio:buffer4:0  iio:device0  iio:device1  iio:device2  iio:device3  iio:device4
------------
root@...log:/# ls /sys/bus/iio/devices/iio:buffer3:0
data_available  dev  enable  length  length_align_bytes  power  scan_elements  subsystem  uevent  watermark
------------
root@...log:/# ls -la /sys/bus/iio/devices/iio\:device3/
total 0
drwxr-xr-x 8 root root    0 May  8 13:00 .
drwxr-xr-x 4 root root    0 May  8 12:53 ..
lrwxrwxrwx 1 root root    0 May  8 13:00 buffer -> iio:buffer3:0
drwxrwxrwx 2 root root    0 May  8 12:53 events
drwxrwxrwx 4 root root    0 May  8 12:53 iio:buffer3:0
drwxrwxrwx 4 root root    0 May  8 12:53 iio:buffer3:1
drwxrwxrwx 4 root root    0 May  8 12:53 iio:buffer3:2
drwxrwxrwx 4 root root    0 May  8 12:53 iio:buffer3:3
-rw-rw-rw- 1 root root 4096 May  8 12:54 in_voltage0_test_mode
-rw-rw-rw- 1 root root 4096 May  8 12:54 in_voltage1_test_mode
-rw-rw-rw- 1 root root 4096 May  8 12:54 in_voltage_sampling_frequency
-rw-rw-rw- 1 root root 4096 May  8 12:53 in_voltage_scale
-rw-rw-rw- 1 root root 4096 May  8 12:53 in_voltage_scale_available
-rw-rw-rw- 1 root root 4096 May  8 12:53 in_voltage_test_mode_available
-rw-rw-rw- 1 root root 4096 May  8 12:53 name
lrwxrwxrwx 1 root root    0 May  8 13:00 of_node -> ../../../../../firmware/devicetree/base/fpga-axi@...xi-ad9680-hpc@...10000
drwxrwxrwx 2 root root    0 May  8 12:53 power
lrwxrwxrwx 1 root root    0 May  8 13:00 scan_elements -> iio:buffer3:0/scan_elements
lrwxrwxrwx 1 root root    0 May  8 13:00 subsystem -> ../../../../../bus/iio
-rw-rw-rw- 1 root root 4096 May  8 12:53 uevent
------------
root@...log:/# ls -la /sys/bus/iio/devices/iio:device3/iio:buffer3:0/
total 0
drwxrwxrwx 4 root root    0 May  8 12:53 .
drwxr-xr-x 8 root root    0 May  8 13:00 ..
-rw-rw-rw- 1 root root 4096 May  8 12:53 data_available
-rw-rw-rw- 1 root root 4096 May  8 12:53 dev
-rw-rw-rw- 1 root root 4096 May  8 12:53 enable
-rw-rw-rw- 1 root root 4096 May  8 12:53 length
-rw-rw-rw- 1 root root 4096 May  8 12:53 length_align_bytes
drwxrwxrwx 2 root root    0 May  8 12:53 power
drwxrwxrwx 2 root root    0 May  8 12:53 scan_elements
lrwxrwxrwx 1 root root    0 May  8 12:53 subsystem -> ../../../../../../bus/iio
-rw-rw-rw- 1 root root 4096 May  8 12:53 uevent
-rw-rw-rw- 1 root root 4096 May  8 12:53 watermark
------------

Using our fancy IIO-oscilloscope[1] app with this format, seems to work.
[1] https://github.com/analogdevicesinc/iio-oscilloscope

Linking the 'scan_elements' folder from the first buffer looks a
bit fishy [the code I mean]; it requires that the buffer device be added
first, so that the 'scan_elements' be attached via kobject, so that we
can have the kobject (as a symlink target).
Again, I did not want to crack open the kernfs.

What I don't like, is that iio:device3 has iio:buffer3:0 (to 3).
This is because the 'buffer->dev.parent = &indio_dev->dev'.
But I do feel this is correct.
So, now I don't know whether to leave it like that or symlink to shorter
versions like 'iio:buffer3:Y' -> 'iio:device3/bufferY'.
The reason for naming the IIO buffer devices to 'iio:bufferX:Y' is
mostly to make the names unique. It would have looked weird to do
'/dev/buffer1' if I would have named the buffer devices 'bufferX'.

So, now I'm thinking of whether all this is acceptable.
Or what is acceptable?
Should I symlink 'iio:device3/iio:buffer3:0' -> 'iio:device3/buffer0'?
What else should I consider moving forward?
What means forward?
Where did I leave my beer?

Well, if you've found my beer, it means you got this far :)
Thanks for reading all of this [including this bit of quarantine humor].

Alexandru Ardelean (12):
  iio: buffer: add back-ref from iio_buffer to iio_dev
  iio: core,buffer: wrap iio_buffer_put() call into iio_buffers_put()
  iio: core: register chardev only if needed
  iio: buffer,event: duplicate chardev creation for buffers & events
  iio: core: add simple centralized mechanism for ioctl() handlers
  iio: core: use new common ioctl() mechanism
  iio: buffer: split buffer sysfs creation to take buffer as primary arg
  iio: buffer: remove attrcount_orig var from sysfs creation
  iio: buffer: add underlying device object and convert buffers to
    devices
  iio: buffer: symlink the scan_elements dir back into IIO device's dir
  iio: unpack all iio buffer attributes correctly
  iio: buffer: convert single buffer to list of buffers

Lars-Peter Clausen (2):
  iio: Move scan mask management to the core
  iio: hw_consumer: use new scanmask functions

 drivers/iio/accel/adxl372.c                   |   6 +-
 drivers/iio/accel/bmc150-accel-core.c         |   6 +-
 drivers/iio/buffer/industrialio-buffer-cb.c   |  16 +-
 .../buffer/industrialio-buffer-dmaengine.c    |   4 +-
 drivers/iio/buffer/industrialio-hw-consumer.c |  19 +-
 drivers/iio/iio_core.h                        |  40 +-
 drivers/iio/industrialio-buffer.c             | 468 ++++++++++++++----
 drivers/iio/industrialio-core.c               | 174 +++----
 drivers/iio/industrialio-event.c              | 100 +++-
 drivers/iio/inkern.c                          |  15 +
 include/linux/iio/buffer.h                    |   3 +
 include/linux/iio/buffer_impl.h               |  24 +-
 include/linux/iio/consumer.h                  |  10 +
 include/linux/iio/iio.h                       |  12 +-
 14 files changed, 688 insertions(+), 209 deletions(-)

-- 
2.17.1

Powered by blists - more mailing lists