[RFC PATCH 0/3] iio: buffer: add output buffer support for chrdev

* [RFC PATCH 0/3] iio: buffer: add output buffer support for chrdev
@ 2020-03-30 14:56 Alexandru Ardelean
  2020-03-30 14:56 ` [RFC PATCH 1/3] iio: core: rename 'indio_dev->buffer_list' -> 'indio_dev->active_buffers' Alexandru Ardelean
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Alexandru Ardelean @ 2020-03-30 14:56 UTC (permalink / raw)
  To: linux-iio
  Cc: jic23, michael.henneric, nuno.sa, lars, dragos.bogdan,
	Alexandru Ardelean

[This description is also present on patch 3/3, there have been some
cosmetic stuff since that one that I did not remove. These patches
probably won't make it in a final version, but they're there because
I changed my mind a few times and got to this RFC]

There have been some offline discussions about how to go about this.
Since I wasn't involved in any of those discussions, this kind of tries to
re-build things from various bits.
It's incomplete work, since I don't have a clear idea of what the accepted/acceptable
approach would be.

1. First approach is: we keep 1 buffer per device, and we make it either
in/out, which means that for devices that for devices that have both in/out
2 iio_dev instances are required, or an ADC needs to be connected on the in
path and a DAC on out-path. This is predominantly done in the ADI tree.

2. One discussion/proposal was to have multiple buffers per-device. But the
details are vague since they were relayed to me.
One detail was, to have indexes for devices that have more than 1
buffer:

Iio_deviceX:
        buffer
        scan_elements

Iio_deviceX:
        BufferY
        scan_elementsY
        BufferZ
        scan_elementsZ

I am not sure how feasible this is for a single chrdev, as when you look at
the fileops that get assigned to a chrdev, it looks like it can have at
most two buffers (one for input, out for output).

Multiplexing input buffers can work (from ADCs), but demultiplexing output
buffers into a DAC, not so well. Especially on a single chrdev.

Question 1: do we want to support more than 2 buffers per chrdev?

This is what ADI currently has in it's tree (and works).

Example, ADC:
 # ls iio\:device3/buffer/
 data_available  enable  length  length_align_bytes  watermark
 #  ls iio\:device3/scan_elements/
 in_voltage0_en  in_voltage0_index  in_voltage0_type  in_voltage1_en  in_voltage1_index  in_voltage1_type

Example, DAC:
 #  ls iio\:device4/buffer/
 data_available  enable  length  length_align_bytes  watermark
 # ls iio\:device4/scan_elements/
 out_voltage0_en     out_voltage0_type  out_voltage1_index  out_voltage2_en     out_voltage2_type  out_voltage3_index
 out_voltage0_index  out_voltage1_en    out_voltage1_type   out_voltage2_index  out_voltage3_en    out_voltage3_type

The direction of each element is encoded into the filename of each channel.

Another question is:
 Does it make sense to have more than 1 'scan_elements' folder?
 That is, for devices that would have both in & out channels.

For 'buffer' folders I was thinking that it may make sense to have,
'buffer_in' && 'buffer_out'.

So, one idea is:

Iio_deviceX:
        buffer_in
        buffer_out
        scan_elements

Currently, this patch kind of implements 2 buffers per iio_dev/chrdev.
But the format is:

Iio_deviceX:
        buffer_in
        buffer_out
        scan_elements_in
        scan_elements_out

Obviously it shouldn't work as-is [as it wasn't tested], but at least gives
some glimpse of where this could go.

3. A side question is about the 'iio_buffer -> pollq' field. I was
wondering if it would make sense to move that on to 'iio_dev  pollq' if
adding multiple buffers are added per-device. It almost makes sense to
unify the 'pollq' on indio_dev.
But, it looks a bit difficult, and would require some more change [which is
doable] if it makes sense for whatever reason.
The only reason to do it, is because the iio_buffer_fileops has a .poll =
iio_buffer_poll() function attached to it. Adding multiple buffers for an
IIO device may require some consideration on the iio_buffer_poll() function
as well.

Alexandru Ardelean (3):
  iio: core: rename 'indio_dev->buffer_list' ->
    'indio_dev->active_buffers'
  iio: buffer: extend short-hand use for 'indio_dev->buffer'
  iio: buffer: add output buffer support for chrdev

 .../buffer/industrialio-buffer-dmaengine.c    |   3 +-
 drivers/iio/industrialio-buffer.c             | 267 +++++++++++++-----
 drivers/iio/industrialio-core.c               |   5 +-
 include/linux/iio/buffer.h                    |   9 +
 include/linux/iio/buffer_impl.h               |   7 +
 include/linux/iio/iio.h                       |  12 +-
 6 files changed, 225 insertions(+), 78 deletions(-)

-- 
2.20.1

^ permalink raw reply	[flat|nested] 5+ messages in thread