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)
> > }
> > }
> ...
