Re: [PATCH v2 2/5] iio: Add output buffer support
On Thu, Feb 18, 2021 at 5:30 PM Jonathan Cameron wrote:
>
> On Wed, 17 Feb 2021 10:34:35 +0200
> Alexandru Ardelean wrote:
>
> > From: Lars-Peter Clausen
> >
> > Currently IIO only supports buffer mode for capture devices like ADCs. Add
> > support for buffered mode for output devices like DACs.
> >
> > The output buffer implementation is analogous to the input buffer
> > implementation. Instead of using read() to get data from the buffer write()
> > is used to copy data into the buffer.
> >
> > poll() with POLLOUT will wakeup if there is space available for more or
> > equal to the configured watermark of samples.
> >
> > Drivers can remove data from a buffer using iio_buffer_remove_sample(), the
> > function can e.g. called from a trigger handler to write the data to
> > hardware.
> >
> > A buffer can only be either a output buffer or an input, but not both. So,
> > for a device that has an ADC and DAC path, this will mean 2 IIO buffers
> > (one for each direction).
> >
> > The direction of the buffer is decided by the new direction field of the
> > iio_buffer struct and should be set after allocating and before registering
> > it.
> >
> > Signed-off-by: Lars-Peter Clausen
> > Signed-off-by: Alexandru Ardelean
>
> Just one question on this, otherwise looks good to me.
>
> Jonathan
>
> > diff --git a/drivers/iio/industrialio-buffer.c
> > b/drivers/iio/industrialio-buffer.c
> > index 5d641f8adfbd..b9970c68005d 100644
> > --- a/drivers/iio/industrialio-buffer.c
> > +++ b/drivers/iio/industrialio-buffer.c
> > @@ -162,6 +162,69 @@ static ssize_t iio_buffer_read(struct file *filp, char
> > __user *buf,
> > return ret;
> > }
> >
> ...
>
> > /**
> > * iio_buffer_poll() - poll the buffer to find out if it has data
> > * @filp:File structure pointer for device access
> > @@ -182,8 +245,19 @@ static __poll_t iio_buffer_poll(struct file *filp,
> > return 0;
> >
> > poll_wait(filp, >pollq, wait);
> > - if (iio_buffer_ready(indio_dev, rb, rb->watermark, 0))
> > - return EPOLLIN | EPOLLRDNORM;
> > +
> > + switch (rb->direction) {
> > + case IIO_BUFFER_DIRECTION_IN:
> > + if (iio_buffer_ready(indio_dev, rb, rb->watermark, 0))
> > + return EPOLLIN | EPOLLRDNORM;
> > + break;
> > + case IIO_BUFFER_DIRECTION_OUT:
> > + if (iio_buffer_space_available(rb) >= rb->watermark)
> > + return EPOLLOUT | EPOLLWRNORM;
> > + break;
> > + }
> > +
> > + /* need a way of knowing if there may be enough data... */
>
> Curious on what this comment is referring to?
I'm also a bit curious about what this comment is referring to.
I can remove it.
Maybe Lars can give some insight.
>
> > return 0;
> > }
> >
> > @@ -232,6 +306,16 @@ void iio_buffer_wakeup_poll(struct iio_dev *indio_dev)
> > }
> > }
> ...
Re: [PATCH v2 2/5] iio: Add output buffer support
On Wed, 17 Feb 2021 10:34:35 +0200
Alexandru Ardelean wrote:
> From: Lars-Peter Clausen
>
> Currently IIO only supports buffer mode for capture devices like ADCs. Add
> support for buffered mode for output devices like DACs.
>
> The output buffer implementation is analogous to the input buffer
> implementation. Instead of using read() to get data from the buffer write()
> is used to copy data into the buffer.
>
> poll() with POLLOUT will wakeup if there is space available for more or
> equal to the configured watermark of samples.
>
> Drivers can remove data from a buffer using iio_buffer_remove_sample(), the
> function can e.g. called from a trigger handler to write the data to
> hardware.
>
> A buffer can only be either a output buffer or an input, but not both. So,
> for a device that has an ADC and DAC path, this will mean 2 IIO buffers
> (one for each direction).
>
> The direction of the buffer is decided by the new direction field of the
> iio_buffer struct and should be set after allocating and before registering
> it.
>
> Signed-off-by: Lars-Peter Clausen
> Signed-off-by: Alexandru Ardelean
Just one question on this, otherwise looks good to me.
Jonathan
> diff --git a/drivers/iio/industrialio-buffer.c
> b/drivers/iio/industrialio-buffer.c
> index 5d641f8adfbd..b9970c68005d 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -162,6 +162,69 @@ static ssize_t iio_buffer_read(struct file *filp, char
> __user *buf,
> return ret;
> }
>
...
> /**
> * iio_buffer_poll() - poll the buffer to find out if it has data
> * @filp:File structure pointer for device access
> @@ -182,8 +245,19 @@ static __poll_t iio_buffer_poll(struct file *filp,
> return 0;
>
> poll_wait(filp, >pollq, wait);
> - if (iio_buffer_ready(indio_dev, rb, rb->watermark, 0))
> - return EPOLLIN | EPOLLRDNORM;
> +
> + switch (rb->direction) {
> + case IIO_BUFFER_DIRECTION_IN:
> + if (iio_buffer_ready(indio_dev, rb, rb->watermark, 0))
> + return EPOLLIN | EPOLLRDNORM;
> + break;
> + case IIO_BUFFER_DIRECTION_OUT:
> + if (iio_buffer_space_available(rb) >= rb->watermark)
> + return EPOLLOUT | EPOLLWRNORM;
> + break;
> + }
> +
> + /* need a way of knowing if there may be enough data... */
Curious on what this comment is referring to?
> return 0;
> }
>
> @@ -232,6 +306,16 @@ void iio_buffer_wakeup_poll(struct iio_dev *indio_dev)
> }
> }
...
[PATCH v2 2/5] iio: Add output buffer support
From: Lars-Peter Clausen
Currently IIO only supports buffer mode for capture devices like ADCs. Add
support for buffered mode for output devices like DACs.
The output buffer implementation is analogous to the input buffer
implementation. Instead of using read() to get data from the buffer write()
is used to copy data into the buffer.
poll() with POLLOUT will wakeup if there is space available for more or
equal to the configured watermark of samples.
Drivers can remove data from a buffer using iio_buffer_remove_sample(), the
function can e.g. called from a trigger handler to write the data to
hardware.
A buffer can only be either a output buffer or an input, but not both. So,
for a device that has an ADC and DAC path, this will mean 2 IIO buffers
(one for each direction).
The direction of the buffer is decided by the new direction field of the
iio_buffer struct and should be set after allocating and before registering
it.
Signed-off-by: Lars-Peter Clausen
Signed-off-by: Alexandru Ardelean
---
Documentation/ABI/testing/sysfs-bus-iio | 7 ++
drivers/iio/industrialio-buffer.c | 128 +++-
include/linux/iio/buffer.h | 7 ++
include/linux/iio/buffer_impl.h | 11 ++
4 files changed, 149 insertions(+), 4 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio
b/Documentation/ABI/testing/sysfs-bus-iio
index f2a72d7fbacb..a42b915bf585 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1124,6 +1124,13 @@ Contact: linux-...@vger.kernel.org
Description:
Number of scans contained by the buffer.
+What: /sys/bus/iio/devices/iio:deviceX/bufferY/direction
+KernelVersion: 5.11
+Contact: linux-...@vger.kernel.org
+Description:
+ Returns the direction of the data stream of the buffer.
+ The output is "in" or "out".
+
What: /sys/bus/iio/devices/iio:deviceX/buffer/enable
KernelVersion: 2.6.35
What: /sys/bus/iio/devices/iio:deviceX/bufferY/enable
diff --git a/drivers/iio/industrialio-buffer.c
b/drivers/iio/industrialio-buffer.c
index 5d641f8adfbd..b9970c68005d 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -162,6 +162,69 @@ static ssize_t iio_buffer_read(struct file *filp, char
__user *buf,
return ret;
}
+static size_t iio_buffer_space_available(struct iio_buffer *buf)
+{
+ if (buf->access->space_available)
+ return buf->access->space_available(buf);
+
+ return SIZE_MAX;
+}
+
+static ssize_t iio_buffer_write(struct file *filp, const char __user *buf,
+ size_t n, loff_t *f_ps)
+{
+ struct iio_dev_buffer_pair *ib = filp->private_data;
+ struct iio_buffer *rb = ib->buffer;
+ struct iio_dev *indio_dev = ib->indio_dev;
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
+ size_t datum_size;
+ size_t to_wait;
+ int ret;
+
+ if (!rb || !rb->access->write)
+ return -EINVAL;
+
+ datum_size = rb->bytes_per_datum;
+
+ /*
+* If datum_size is 0 there will never be anything to read from the
+* buffer, so signal end of file now.
+*/
+ if (!datum_size)
+ return 0;
+
+ if (filp->f_flags & O_NONBLOCK)
+ to_wait = 0;
+ else
+ to_wait = min_t(size_t, n / datum_size, rb->watermark);
+
+ add_wait_queue(>pollq, );
+ do {
+ if (!indio_dev->info) {
+ ret = -ENODEV;
+ break;
+ }
+
+ if (iio_buffer_space_available(rb) < to_wait) {
+ if (signal_pending(current)) {
+ ret = -ERESTARTSYS;
+ break;
+ }
+
+ wait_woken(, TASK_INTERRUPTIBLE,
+ MAX_SCHEDULE_TIMEOUT);
+ continue;
+ }
+
+ ret = rb->access->write(rb, n, buf);
+ if (ret == 0 && (filp->f_flags & O_NONBLOCK))
+ ret = -EAGAIN;
+ } while (ret == 0);
+ remove_wait_queue(>pollq, );
+
+ return ret;
+}
+
/**
* iio_buffer_poll() - poll the buffer to find out if it has data
* @filp: File structure pointer for device access
@@ -182,8 +245,19 @@ static __poll_t iio_buffer_poll(struct file *filp,
return 0;
poll_wait(filp, >pollq, wait);
- if (iio_buffer_ready(indio_dev, rb, rb->watermark, 0))
- return EPOLLIN | EPOLLRDNORM;
+
+ switch (rb->direction) {
+ case IIO_BUFFER_DIRECTION_IN:
+ if (iio_buffer_ready(indio_dev, rb, rb->watermark, 0))
+ return EPOLLIN | EPOLLRDNORM;
+ break;
+ case IIO_BUFFER_DIRECTION_OUT:
+ if (iio_buffer_space_available(rb) >=
