[PATCH v6 0/6] iio: Add output buffer support

[PATCH v6 0/6] iio: Add output buffer support

[Mihail Chindris]

Changelog v5 -> v6:
  * https://lore.kernel.org/all/20210916182914.1810-1-mihail.chindris@analog.com
  * Fix check in  iio_update_buffers adding test on insert_buffer 
  * Return if check in iio_update_buffers fails.
  * Fix length typo
  * Add rb->direction check in both read and write functions.
  * Add fixes tags
  * Add  Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com> line
  * Remove unnecessary (u8 *) cast
  * Remove unnecessary AD5766_NUM_CH define

Changelog v4 -> v5:
  * https://lore.kernel.org/all/20210820165927.4524-1-mihail.chindris@analog.com
  * Remove ad3552r example from series and replace with the update of an
    existing driver: ad5662. Will add ad3552r in other another series.
  * Make normal comment from kernel-doc comment. (A bot was complaining about that)
  * Add indio_dev->info check 
  * Rename iio_buffer_remove_sample -> iio_pop_from_buffer
  * Fix comment of remove_from: sample -> scan
  * Change iio_pop_from_buffer data type to void * to be consistent with
    iio_push_to_buffers
  * Remove use watermark, in our kernel is not used and I can't think of an
    usecase for it.
  * Reimplement write to increment buffer index and handle blocking and
    noblocking calls
  * Move `if (insert_buffer->direction == IIO_BUFFER_DIRECTION_OUT)` outside lock
  * Remove redundant checks of `if (insert_buffer->direction ==
    IIO_BUFFER_DIRECTION_OUT)`

Alexandru Ardelean (1):
  iio: triggered-buffer: extend support to configure output buffers

Lars-Peter Clausen (1):
  iio: kfifo-buffer: Add output buffer support

Mihail Chindris (4):
  iio: Add output buffer support
  drivers: iio: dac: ad5766: Fix dt property name
  Documentation:devicetree:bindings:iio:dac: Fix val
  drivers:iio:dac:ad5766.c: Add trigger buffer

 .../bindings/iio/dac/adi,ad5766.yaml          |   2 +-
 drivers/iio/accel/adxl372.c                   |   1 +
 drivers/iio/accel/bmc150-accel-core.c         |   1 +
 drivers/iio/adc/at91-sama5d2_adc.c            |   4 +-
 .../buffer/industrialio-triggered-buffer.c    |   8 +-
 drivers/iio/buffer/kfifo_buf.c                |  50 +++++++
 .../cros_ec_sensors/cros_ec_sensors_core.c    |   5 +-
 .../common/hid-sensors/hid-sensor-trigger.c   |   5 +-
 drivers/iio/dac/ad5766.c                      |  48 ++++++-
 drivers/iio/iio_core.h                        |   4 +
 drivers/iio/industrialio-buffer.c             | 127 +++++++++++++++++-
 drivers/iio/industrialio-core.c               |   1 +
 include/linux/iio/buffer.h                    |   7 +
 include/linux/iio/buffer_impl.h               |  11 ++
 include/linux/iio/triggered_buffer.h          |  11 +-
 15 files changed, 269 insertions(+), 16 deletions(-)


base-commit: 94a853eca720ac9e385e59f27e859b4a01123f58
-- 
2.27.0

[PATCH v6 1/6] iio: Add output buffer support

[Mihail Chindris]

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.

Drivers can remove data from a buffer using iio_pop_from_buffer(), 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 <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
---
 drivers/iio/iio_core.h            |   4 +
 drivers/iio/industrialio-buffer.c | 127 +++++++++++++++++++++++++++++-
 drivers/iio/industrialio-core.c   |   1 +
 include/linux/iio/buffer.h        |   7 ++
 include/linux/iio/buffer_impl.h   |  11 +++
 5 files changed, 148 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
index 8f4a9b264962..61e318431de9 100644
--- a/drivers/iio/iio_core.h
+++ b/drivers/iio/iio_core.h
@@ -68,12 +68,15 @@ __poll_t iio_buffer_poll_wrapper(struct file *filp,
 				 struct poll_table_struct *wait);
 ssize_t iio_buffer_read_wrapper(struct file *filp, char __user *buf,
 				size_t n, loff_t *f_ps);
+ssize_t iio_buffer_write_wrapper(struct file *filp, const char __user *buf,
+				 size_t n, loff_t *f_ps);
 
 int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev);
 void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev);
 
 #define iio_buffer_poll_addr (&iio_buffer_poll_wrapper)
 #define iio_buffer_read_outer_addr (&iio_buffer_read_wrapper)
+#define iio_buffer_write_outer_addr (&iio_buffer_write_wrapper)
 
 void iio_disable_all_buffers(struct iio_dev *indio_dev);
 void iio_buffer_wakeup_poll(struct iio_dev *indio_dev);
@@ -83,6 +86,7 @@ void iio_device_detach_buffers(struct iio_dev *indio_dev);
 
 #define iio_buffer_poll_addr NULL
 #define iio_buffer_read_outer_addr NULL
+#define iio_buffer_write_outer_addr NULL
 
 static inline int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
 {
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index a95cc2da56be..7286563e6234 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -120,6 +120,9 @@ static ssize_t iio_buffer_read(struct file *filp, char __user *buf,
 	if (!rb || !rb->access->read)
 		return -EINVAL;
 
+	if (rb->direction != IIO_BUFFER_DIRECTION_IN)
+		return -EPERM;
+
 	datum_size = rb->bytes_per_datum;
 
 	/*
@@ -161,6 +164,65 @@ 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);
+	int ret;
+	size_t written;
+
+	if (!indio_dev->info)
+		return -ENODEV;
+
+	if (!rb || !rb->access->write)
+		return -EINVAL;
+
+	if (rb->direction != IIO_BUFFER_DIRECTION_OUT)
+		return -EPERM;
+
+	written = 0;
+	add_wait_queue(&rb->pollq, &wait);
+	do {
+		if (indio_dev->info == NULL)
+			return -ENODEV;
+
+		if (!iio_buffer_space_available(rb)) {
+			if (signal_pending(current)) {
+				ret = -ERESTARTSYS;
+				break;
+			}
+
+			wait_woken(&wait, TASK_INTERRUPTIBLE,
+					MAX_SCHEDULE_TIMEOUT);
+			continue;
+		}
+
+		ret = rb->access->write(rb, n - written, buf + written);
+		if (ret == 0 && (filp->f_flags & O_NONBLOCK))
+			ret = -EAGAIN;
+
+		if (ret > 0) {
+			written += ret;
+			if (written != n && !(filp->f_flags & O_NONBLOCK))
+				continue;
+		}
+	} while (ret == 0);
+	remove_wait_queue(&rb->pollq, &wait);
+
+	return ret < 0 ? ret : n;
+}
+
 /**
  * iio_buffer_poll() - poll the buffer to find out if it has data
  * @filp:	File structure pointer for device access
@@ -181,8 +243,18 @@ static __poll_t iio_buffer_poll(struct file *filp,
 		return 0;
 
 	poll_wait(filp, &rb->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))
+			return EPOLLOUT | EPOLLWRNORM;
+		break;
+	}
+
 	return 0;
 }
 
@@ -199,6 +271,19 @@ ssize_t iio_buffer_read_wrapper(struct file *filp, char __user *buf,
 	return iio_buffer_read(filp, buf, n, f_ps);
 }
 
+ssize_t iio_buffer_write_wrapper(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;
+
+	/* check if buffer was opened through new API */
+	if (test_bit(IIO_BUSY_BIT_POS, &rb->flags))
+		return -EBUSY;
+
+	return iio_buffer_write(filp, buf, n, f_ps);
+}
+
 __poll_t iio_buffer_poll_wrapper(struct file *filp,
 				 struct poll_table_struct *wait)
 {
@@ -231,6 +316,15 @@ void iio_buffer_wakeup_poll(struct iio_dev *indio_dev)
 	}
 }
 
+int iio_pop_from_buffer(struct iio_buffer *buffer, void *data)
+{
+	if (!buffer || !buffer->access || !buffer->access->remove_from)
+		return -EINVAL;
+
+	return buffer->access->remove_from(buffer, data);
+}
+EXPORT_SYMBOL_GPL(iio_pop_from_buffer);
+
 void iio_buffer_init(struct iio_buffer *buffer)
 {
 	INIT_LIST_HEAD(&buffer->demux_list);
@@ -1156,6 +1250,10 @@ int iio_update_buffers(struct iio_dev *indio_dev,
 	if (insert_buffer == remove_buffer)
 		return 0;
 
+	if (insert_buffer &&
+	    (insert_buffer->direction == IIO_BUFFER_DIRECTION_OUT))
+		return -EINVAL;
+
 	mutex_lock(&iio_dev_opaque->info_exist_lock);
 	mutex_lock(&indio_dev->mlock);
 
@@ -1277,6 +1375,22 @@ static ssize_t iio_dma_show_data_available(struct device *dev,
 	return sysfs_emit(buf, "%zu\n", iio_buffer_data_available(buffer));
 }
 
+static ssize_t direction_show(struct device *dev,
+			      struct device_attribute *attr,
+			      char *buf)
+{
+	struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer;
+
+	switch (buffer->direction) {
+	case IIO_BUFFER_DIRECTION_IN:
+		return sprintf(buf, "in\n");
+	case IIO_BUFFER_DIRECTION_OUT:
+		return sprintf(buf, "out\n");
+	default:
+		return -EINVAL;
+	}
+}
+
 static DEVICE_ATTR(length, S_IRUGO | S_IWUSR, iio_buffer_read_length,
 		   iio_buffer_write_length);
 static struct device_attribute dev_attr_length_ro = __ATTR(length,
@@ -1289,12 +1403,20 @@ static struct device_attribute dev_attr_watermark_ro = __ATTR(watermark,
 	S_IRUGO, iio_buffer_show_watermark, NULL);
 static DEVICE_ATTR(data_available, S_IRUGO,
 		iio_dma_show_data_available, NULL);
+static DEVICE_ATTR_RO(direction);
 
+/*
+ * When adding new attributes here, put the at the end, at least until
+ * the code that handles the length/length_ro & watermark/watermark_ro
+ * assignments gets cleaned up. Otherwise these can create some weird
+ * duplicate attributes errors under some setups.
+ */
 static struct attribute *iio_buffer_attrs[] = {
 	&dev_attr_length.attr,
 	&dev_attr_enable.attr,
 	&dev_attr_watermark.attr,
 	&dev_attr_data_available.attr,
+	&dev_attr_direction.attr,
 };
 
 #define to_dev_attr(_attr) container_of(_attr, struct device_attribute, attr)
@@ -1397,6 +1519,7 @@ static const struct file_operations iio_buffer_chrdev_fileops = {
 	.owner = THIS_MODULE,
 	.llseek = noop_llseek,
 	.read = iio_buffer_read,
+	.write = iio_buffer_write,
 	.poll = iio_buffer_poll,
 	.release = iio_buffer_chrdev_release,
 };
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 2dbb37e09b8c..537a08549a69 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1822,6 +1822,7 @@ static const struct file_operations iio_buffer_fileops = {
 	.owner = THIS_MODULE,
 	.llseek = noop_llseek,
 	.read = iio_buffer_read_outer_addr,
+	.write = iio_buffer_write_outer_addr,
 	.poll = iio_buffer_poll_addr,
 	.unlocked_ioctl = iio_ioctl,
 	.compat_ioctl = compat_ptr_ioctl,
diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
index b6928ac5c63d..fe2e680d9b5e 100644
--- a/include/linux/iio/buffer.h
+++ b/include/linux/iio/buffer.h
@@ -11,8 +11,15 @@
 
 struct iio_buffer;
 
+enum iio_buffer_direction {
+	IIO_BUFFER_DIRECTION_IN,
+	IIO_BUFFER_DIRECTION_OUT,
+};
+
 int iio_push_to_buffers(struct iio_dev *indio_dev, const void *data);
 
+int iio_pop_from_buffer(struct iio_buffer *buffer, void *data);
+
 /**
  * iio_push_to_buffers_with_timestamp() - push data and timestamp to buffers
  * @indio_dev:		iio_dev structure for device.
diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h
index 245b32918ae1..e2ca8ea23e19 100644
--- a/include/linux/iio/buffer_impl.h
+++ b/include/linux/iio/buffer_impl.h
@@ -7,6 +7,7 @@
 #ifdef CONFIG_IIO_BUFFER
 
 #include <uapi/linux/iio/buffer.h>
+#include <linux/iio/buffer.h>
 
 struct iio_dev;
 struct iio_buffer;
@@ -23,6 +24,10 @@ struct iio_buffer;
  * @read:		try to get a specified number of bytes (must exist)
  * @data_available:	indicates how much data is available for reading from
  *			the buffer.
+ * @remove_from:	remove scan from buffer. Drivers should calls this to
+ *			remove a scan from a buffer.
+ * @write:		try to write a number of bytes
+ * @space_available:	returns the amount of bytes available in a buffer
  * @request_update:	if a parameter change has been marked, update underlying
  *			storage.
  * @set_bytes_per_datum:set number of bytes per datum
@@ -49,6 +54,9 @@ struct iio_buffer_access_funcs {
 	int (*store_to)(struct iio_buffer *buffer, const void *data);
 	int (*read)(struct iio_buffer *buffer, size_t n, char __user *buf);
 	size_t (*data_available)(struct iio_buffer *buffer);
+	int (*remove_from)(struct iio_buffer *buffer, void *data);
+	int (*write)(struct iio_buffer *buffer, size_t n, const char __user *buf);
+	size_t (*space_available)(struct iio_buffer *buffer);
 
 	int (*request_update)(struct iio_buffer *buffer);
 
@@ -80,6 +88,9 @@ struct iio_buffer {
 	/**  @bytes_per_datum: Size of individual datum including timestamp. */
 	size_t bytes_per_datum;
 
+	/* @direction: Direction of the data stream (in/out). */
+	enum iio_buffer_direction direction;
+
 	/**
 	 * @access: Buffer access functions associated with the
 	 * implementation.
-- 
2.27.0

Re: [PATCH v6 1/6] iio: Add output buffer support

[Jonathan Cameron]

On Thu, 7 Oct 2021 08:00:30 +0000
Mihail Chindris <mihail.chindris@analog.com> wrote:

> 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.
> 
> Drivers can remove data from a buffer using iio_pop_from_buffer(), 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 <lars@metafoo.de>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>

Hi Mihail,

I'm fine with this series now, but one question for this patch on whether
we can clarify the author chain.

The above might mean one of two things.
1) Lars wrote the code and Alex and yourself just 'handled' the patch on its
   way to posting.  If that were the case it should have a From: for Lars
2) All 3 were involved in changes to this patch.  In that case we should
have Co-developed-by: lines for lars and Alex as described:
https://elixir.bootlin.com/linux/latest/source/Documentation/process/submitting-patches.rst#L475

This patch has a history well predating the Co-developed-tag but I'm happy
to add that if you can confirm that matches with the intent.

Good to leave on the list for a few days anyway in case anyone else wants
to take a quick look.

I'm looking forwards to merging this and thinking back to when Lars
originally discussed this feature with me rather a lot of years back!

Jonathan



> ---
>  drivers/iio/iio_core.h            |   4 +
>  drivers/iio/industrialio-buffer.c | 127 +++++++++++++++++++++++++++++-
>  drivers/iio/industrialio-core.c   |   1 +
>  include/linux/iio/buffer.h        |   7 ++
>  include/linux/iio/buffer_impl.h   |  11 +++
>  5 files changed, 148 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
> index 8f4a9b264962..61e318431de9 100644
> --- a/drivers/iio/iio_core.h
> +++ b/drivers/iio/iio_core.h
> @@ -68,12 +68,15 @@ __poll_t iio_buffer_poll_wrapper(struct file *filp,
>  				 struct poll_table_struct *wait);
>  ssize_t iio_buffer_read_wrapper(struct file *filp, char __user *buf,
>  				size_t n, loff_t *f_ps);
> +ssize_t iio_buffer_write_wrapper(struct file *filp, const char __user *buf,
> +				 size_t n, loff_t *f_ps);
>  
>  int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev);
>  void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev);
>  
>  #define iio_buffer_poll_addr (&iio_buffer_poll_wrapper)
>  #define iio_buffer_read_outer_addr (&iio_buffer_read_wrapper)
> +#define iio_buffer_write_outer_addr (&iio_buffer_write_wrapper)
>  
>  void iio_disable_all_buffers(struct iio_dev *indio_dev);
>  void iio_buffer_wakeup_poll(struct iio_dev *indio_dev);
> @@ -83,6 +86,7 @@ void iio_device_detach_buffers(struct iio_dev *indio_dev);
>  
>  #define iio_buffer_poll_addr NULL
>  #define iio_buffer_read_outer_addr NULL
> +#define iio_buffer_write_outer_addr NULL
>  
>  static inline int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
>  {
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index a95cc2da56be..7286563e6234 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -120,6 +120,9 @@ static ssize_t iio_buffer_read(struct file *filp, char __user *buf,
>  	if (!rb || !rb->access->read)
>  		return -EINVAL;
>  
> +	if (rb->direction != IIO_BUFFER_DIRECTION_IN)
> +		return -EPERM;
> +
>  	datum_size = rb->bytes_per_datum;
>  
>  	/*
> @@ -161,6 +164,65 @@ 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);
> +	int ret;
> +	size_t written;
> +
> +	if (!indio_dev->info)
> +		return -ENODEV;
> +
> +	if (!rb || !rb->access->write)
> +		return -EINVAL;
> +
> +	if (rb->direction != IIO_BUFFER_DIRECTION_OUT)
> +		return -EPERM;
> +
> +	written = 0;
> +	add_wait_queue(&rb->pollq, &wait);
> +	do {
> +		if (indio_dev->info == NULL)
> +			return -ENODEV;
> +
> +		if (!iio_buffer_space_available(rb)) {
> +			if (signal_pending(current)) {
> +				ret = -ERESTARTSYS;
> +				break;
> +			}
> +
> +			wait_woken(&wait, TASK_INTERRUPTIBLE,
> +					MAX_SCHEDULE_TIMEOUT);
> +			continue;
> +		}
> +
> +		ret = rb->access->write(rb, n - written, buf + written);
> +		if (ret == 0 && (filp->f_flags & O_NONBLOCK))
> +			ret = -EAGAIN;
> +
> +		if (ret > 0) {
> +			written += ret;
> +			if (written != n && !(filp->f_flags & O_NONBLOCK))
> +				continue;
> +		}
> +	} while (ret == 0);
> +	remove_wait_queue(&rb->pollq, &wait);
> +
> +	return ret < 0 ? ret : n;
> +}
> +
>  /**
>   * iio_buffer_poll() - poll the buffer to find out if it has data
>   * @filp:	File structure pointer for device access
> @@ -181,8 +243,18 @@ static __poll_t iio_buffer_poll(struct file *filp,
>  		return 0;
>  
>  	poll_wait(filp, &rb->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))
> +			return EPOLLOUT | EPOLLWRNORM;
> +		break;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -199,6 +271,19 @@ ssize_t iio_buffer_read_wrapper(struct file *filp, char __user *buf,
>  	return iio_buffer_read(filp, buf, n, f_ps);
>  }
>  
> +ssize_t iio_buffer_write_wrapper(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;
> +
> +	/* check if buffer was opened through new API */
> +	if (test_bit(IIO_BUSY_BIT_POS, &rb->flags))
> +		return -EBUSY;
> +
> +	return iio_buffer_write(filp, buf, n, f_ps);
> +}
> +
>  __poll_t iio_buffer_poll_wrapper(struct file *filp,
>  				 struct poll_table_struct *wait)
>  {
> @@ -231,6 +316,15 @@ void iio_buffer_wakeup_poll(struct iio_dev *indio_dev)
>  	}
>  }
>  
> +int iio_pop_from_buffer(struct iio_buffer *buffer, void *data)
> +{
> +	if (!buffer || !buffer->access || !buffer->access->remove_from)
> +		return -EINVAL;
> +
> +	return buffer->access->remove_from(buffer, data);
> +}
> +EXPORT_SYMBOL_GPL(iio_pop_from_buffer);
> +
>  void iio_buffer_init(struct iio_buffer *buffer)
>  {
>  	INIT_LIST_HEAD(&buffer->demux_list);
> @@ -1156,6 +1250,10 @@ int iio_update_buffers(struct iio_dev *indio_dev,
>  	if (insert_buffer == remove_buffer)
>  		return 0;
>  
> +	if (insert_buffer &&
> +	    (insert_buffer->direction == IIO_BUFFER_DIRECTION_OUT))
> +		return -EINVAL;
> +
>  	mutex_lock(&iio_dev_opaque->info_exist_lock);
>  	mutex_lock(&indio_dev->mlock);
>  
> @@ -1277,6 +1375,22 @@ static ssize_t iio_dma_show_data_available(struct device *dev,
>  	return sysfs_emit(buf, "%zu\n", iio_buffer_data_available(buffer));
>  }
>  
> +static ssize_t direction_show(struct device *dev,
> +			      struct device_attribute *attr,
> +			      char *buf)
> +{
> +	struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer;
> +
> +	switch (buffer->direction) {
> +	case IIO_BUFFER_DIRECTION_IN:
> +		return sprintf(buf, "in\n");
> +	case IIO_BUFFER_DIRECTION_OUT:
> +		return sprintf(buf, "out\n");
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static DEVICE_ATTR(length, S_IRUGO | S_IWUSR, iio_buffer_read_length,
>  		   iio_buffer_write_length);
>  static struct device_attribute dev_attr_length_ro = __ATTR(length,
> @@ -1289,12 +1403,20 @@ static struct device_attribute dev_attr_watermark_ro = __ATTR(watermark,
>  	S_IRUGO, iio_buffer_show_watermark, NULL);
>  static DEVICE_ATTR(data_available, S_IRUGO,
>  		iio_dma_show_data_available, NULL);
> +static DEVICE_ATTR_RO(direction);
>  
> +/*
> + * When adding new attributes here, put the at the end, at least until
> + * the code that handles the length/length_ro & watermark/watermark_ro
> + * assignments gets cleaned up. Otherwise these can create some weird
> + * duplicate attributes errors under some setups.
> + */
>  static struct attribute *iio_buffer_attrs[] = {
>  	&dev_attr_length.attr,
>  	&dev_attr_enable.attr,
>  	&dev_attr_watermark.attr,
>  	&dev_attr_data_available.attr,
> +	&dev_attr_direction.attr,
>  };
>  
>  #define to_dev_attr(_attr) container_of(_attr, struct device_attribute, attr)
> @@ -1397,6 +1519,7 @@ static const struct file_operations iio_buffer_chrdev_fileops = {
>  	.owner = THIS_MODULE,
>  	.llseek = noop_llseek,
>  	.read = iio_buffer_read,
> +	.write = iio_buffer_write,
>  	.poll = iio_buffer_poll,
>  	.release = iio_buffer_chrdev_release,
>  };
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 2dbb37e09b8c..537a08549a69 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1822,6 +1822,7 @@ static const struct file_operations iio_buffer_fileops = {
>  	.owner = THIS_MODULE,
>  	.llseek = noop_llseek,
>  	.read = iio_buffer_read_outer_addr,
> +	.write = iio_buffer_write_outer_addr,
>  	.poll = iio_buffer_poll_addr,
>  	.unlocked_ioctl = iio_ioctl,
>  	.compat_ioctl = compat_ptr_ioctl,
> diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
> index b6928ac5c63d..fe2e680d9b5e 100644
> --- a/include/linux/iio/buffer.h
> +++ b/include/linux/iio/buffer.h
> @@ -11,8 +11,15 @@
>  
>  struct iio_buffer;
>  
> +enum iio_buffer_direction {
> +	IIO_BUFFER_DIRECTION_IN,
> +	IIO_BUFFER_DIRECTION_OUT,
> +};
> +
>  int iio_push_to_buffers(struct iio_dev *indio_dev, const void *data);
>  
> +int iio_pop_from_buffer(struct iio_buffer *buffer, void *data);
> +
>  /**
>   * iio_push_to_buffers_with_timestamp() - push data and timestamp to buffers
>   * @indio_dev:		iio_dev structure for device.
> diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h
> index 245b32918ae1..e2ca8ea23e19 100644
> --- a/include/linux/iio/buffer_impl.h
> +++ b/include/linux/iio/buffer_impl.h
> @@ -7,6 +7,7 @@
>  #ifdef CONFIG_IIO_BUFFER
>  
>  #include <uapi/linux/iio/buffer.h>
> +#include <linux/iio/buffer.h>
>  
>  struct iio_dev;
>  struct iio_buffer;
> @@ -23,6 +24,10 @@ struct iio_buffer;
>   * @read:		try to get a specified number of bytes (must exist)
>   * @data_available:	indicates how much data is available for reading from
>   *			the buffer.
> + * @remove_from:	remove scan from buffer. Drivers should calls this to
> + *			remove a scan from a buffer.
> + * @write:		try to write a number of bytes
> + * @space_available:	returns the amount of bytes available in a buffer
>   * @request_update:	if a parameter change has been marked, update underlying
>   *			storage.
>   * @set_bytes_per_datum:set number of bytes per datum
> @@ -49,6 +54,9 @@ struct iio_buffer_access_funcs {
>  	int (*store_to)(struct iio_buffer *buffer, const void *data);
>  	int (*read)(struct iio_buffer *buffer, size_t n, char __user *buf);
>  	size_t (*data_available)(struct iio_buffer *buffer);
> +	int (*remove_from)(struct iio_buffer *buffer, void *data);
> +	int (*write)(struct iio_buffer *buffer, size_t n, const char __user *buf);
> +	size_t (*space_available)(struct iio_buffer *buffer);
>  
>  	int (*request_update)(struct iio_buffer *buffer);
>  
> @@ -80,6 +88,9 @@ struct iio_buffer {
>  	/**  @bytes_per_datum: Size of individual datum including timestamp. */
>  	size_t bytes_per_datum;
>  
> +	/* @direction: Direction of the data stream (in/out). */
> +	enum iio_buffer_direction direction;
> +
>  	/**
>  	 * @access: Buffer access functions associated with the
>  	 * implementation.

RE: [PATCH v6 1/6] iio: Add output buffer support

[Chindris, Mihail]

> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Thursday, 7 October 2021 19:06
> To: Chindris, Mihail <Mihail.Chindris@analog.com>
> Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> lars@metafoo.de; Hennerich, Michael <Michael.Hennerich@analog.com>;
> Sa, Nuno <Nuno.Sa@analog.com>; Bogdan, Dragos
> <Dragos.Bogdan@analog.com>; alexandru.ardelean@analog.com
> Subject: Re: [PATCH v6 1/6] iio: Add output buffer support
> 
> On Thu, 7 Oct 2021 08:00:30 +0000
> Mihail Chindris <mihail.chindris@analog.com> wrote:
> 
> > 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.
> >
> > Drivers can remove data from a buffer using iio_pop_from_buffer(), 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 <lars@metafoo.de>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
> 
> Hi Mihail,
> 
> I'm fine with this series now, but one question for this patch on whether we
> can clarify the author chain.
> 
> The above might mean one of two things.
> 1) Lars wrote the code and Alex and yourself just 'handled' the patch on its
>    way to posting.  If that were the case it should have a From: for Lars
> 2) All 3 were involved in changes to this patch.  In that case we should have
> Co-developed-by: lines for lars and Alex as described:
> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/latest/source/
> Documentation/process/submitting-
> patches.rst*L475__;Iw!!A3Ni8CS0y2Y!t1IaOEZX3w2pZXl-
> RyAlXgPPvxFtCON74ppfGuMgV_pKZcNjsLs-dKSk_mA34IsmlOU$
> 
> This patch has a history well predating the Co-developed-tag but I'm happy
> to add that if you can confirm that matches with the intent.
> 
> Good to leave on the list for a few days anyway in case anyone else wants to
> take a quick look.
> 
> I'm looking forwards to merging this and thinking back to when Lars originally
> discussed this feature with me rather a lot of years back!
> 
> Jonathan
> 
> 

Hi Jonathan,

What I have done with the patch was to take the V3 that Alex has left and implement the feedback. So I think this is considered as 'handled' from my side.
What happened before V3 I think is better if Alex spokes for himself because I don't really know :)

Mihail

> 
> > ---
> >  drivers/iio/iio_core.h            |   4 +
> >  drivers/iio/industrialio-buffer.c | 127
> +++++++++++++++++++++++++++++-
> >  drivers/iio/industrialio-core.c   |   1 +
> >  include/linux/iio/buffer.h        |   7 ++
> >  include/linux/iio/buffer_impl.h   |  11 +++
> >  5 files changed, 148 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h index
> > 8f4a9b264962..61e318431de9 100644
> > --- a/drivers/iio/iio_core.h
> > +++ b/drivers/iio/iio_core.h
> > @@ -68,12 +68,15 @@ __poll_t iio_buffer_poll_wrapper(struct file *filp,
> >  				 struct poll_table_struct *wait);  ssize_t
> > iio_buffer_read_wrapper(struct file *filp, char __user *buf,
> >  				size_t n, loff_t *f_ps);
> > +ssize_t iio_buffer_write_wrapper(struct file *filp, const char __user *buf,
> > +				 size_t n, loff_t *f_ps);
> >
> >  int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev);
> > void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev);
> >
> >  #define iio_buffer_poll_addr (&iio_buffer_poll_wrapper)  #define
> > iio_buffer_read_outer_addr (&iio_buffer_read_wrapper)
> > +#define iio_buffer_write_outer_addr (&iio_buffer_write_wrapper)
> >
> >  void iio_disable_all_buffers(struct iio_dev *indio_dev);  void
> > iio_buffer_wakeup_poll(struct iio_dev *indio_dev); @@ -83,6 +86,7 @@
> > void iio_device_detach_buffers(struct iio_dev *indio_dev);
> >
> >  #define iio_buffer_poll_addr NULL
> >  #define iio_buffer_read_outer_addr NULL
> > +#define iio_buffer_write_outer_addr NULL
> >
> >  static inline int iio_buffers_alloc_sysfs_and_mask(struct iio_dev
> > *indio_dev)  { diff --git a/drivers/iio/industrialio-buffer.c
> > b/drivers/iio/industrialio-buffer.c
> > index a95cc2da56be..7286563e6234 100644
> > --- a/drivers/iio/industrialio-buffer.c
> > +++ b/drivers/iio/industrialio-buffer.c
> > @@ -120,6 +120,9 @@ static ssize_t iio_buffer_read(struct file *filp, char
> __user *buf,
> >  	if (!rb || !rb->access->read)
> >  		return -EINVAL;
> >
> > +	if (rb->direction != IIO_BUFFER_DIRECTION_IN)
> > +		return -EPERM;
> > +
> >  	datum_size = rb->bytes_per_datum;
> >
> >  	/*
> > @@ -161,6 +164,65 @@ 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);
> > +	int ret;
> > +	size_t written;
> > +
> > +	if (!indio_dev->info)
> > +		return -ENODEV;
> > +
> > +	if (!rb || !rb->access->write)
> > +		return -EINVAL;
> > +
> > +	if (rb->direction != IIO_BUFFER_DIRECTION_OUT)
> > +		return -EPERM;
> > +
> > +	written = 0;
> > +	add_wait_queue(&rb->pollq, &wait);
> > +	do {
> > +		if (indio_dev->info == NULL)
> > +			return -ENODEV;
> > +
> > +		if (!iio_buffer_space_available(rb)) {
> > +			if (signal_pending(current)) {
> > +				ret = -ERESTARTSYS;
> > +				break;
> > +			}
> > +
> > +			wait_woken(&wait, TASK_INTERRUPTIBLE,
> > +					MAX_SCHEDULE_TIMEOUT);
> > +			continue;
> > +		}
> > +
> > +		ret = rb->access->write(rb, n - written, buf + written);
> > +		if (ret == 0 && (filp->f_flags & O_NONBLOCK))
> > +			ret = -EAGAIN;
> > +
> > +		if (ret > 0) {
> > +			written += ret;
> > +			if (written != n && !(filp->f_flags & O_NONBLOCK))
> > +				continue;
> > +		}
> > +	} while (ret == 0);
> > +	remove_wait_queue(&rb->pollq, &wait);
> > +
> > +	return ret < 0 ? ret : n;
> > +}
> > +
> >  /**
> >   * iio_buffer_poll() - poll the buffer to find out if it has data
> >   * @filp:	File structure pointer for device access
> > @@ -181,8 +243,18 @@ static __poll_t iio_buffer_poll(struct file *filp,
> >  		return 0;
> >
> >  	poll_wait(filp, &rb->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))
> > +			return EPOLLOUT | EPOLLWRNORM;
> > +		break;
> > +	}
> > +
> >  	return 0;
> >  }
> >
> > @@ -199,6 +271,19 @@ ssize_t iio_buffer_read_wrapper(struct file *filp,
> char __user *buf,
> >  	return iio_buffer_read(filp, buf, n, f_ps);  }
> >
> > +ssize_t iio_buffer_write_wrapper(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;
> > +
> > +	/* check if buffer was opened through new API */
> > +	if (test_bit(IIO_BUSY_BIT_POS, &rb->flags))
> > +		return -EBUSY;
> > +
> > +	return iio_buffer_write(filp, buf, n, f_ps); }
> > +
> >  __poll_t iio_buffer_poll_wrapper(struct file *filp,
> >  				 struct poll_table_struct *wait)
> >  {
> > @@ -231,6 +316,15 @@ void iio_buffer_wakeup_poll(struct iio_dev
> *indio_dev)
> >  	}
> >  }
> >
> > +int iio_pop_from_buffer(struct iio_buffer *buffer, void *data) {
> > +	if (!buffer || !buffer->access || !buffer->access->remove_from)
> > +		return -EINVAL;
> > +
> > +	return buffer->access->remove_from(buffer, data); }
> > +EXPORT_SYMBOL_GPL(iio_pop_from_buffer);
> > +
> >  void iio_buffer_init(struct iio_buffer *buffer)  {
> >  	INIT_LIST_HEAD(&buffer->demux_list);
> > @@ -1156,6 +1250,10 @@ int iio_update_buffers(struct iio_dev
> *indio_dev,
> >  	if (insert_buffer == remove_buffer)
> >  		return 0;
> >
> > +	if (insert_buffer &&
> > +	    (insert_buffer->direction == IIO_BUFFER_DIRECTION_OUT))
> > +		return -EINVAL;
> > +
> >  	mutex_lock(&iio_dev_opaque->info_exist_lock);
> >  	mutex_lock(&indio_dev->mlock);
> >
> > @@ -1277,6 +1375,22 @@ static ssize_t
> iio_dma_show_data_available(struct device *dev,
> >  	return sysfs_emit(buf, "%zu\n", iio_buffer_data_available(buffer));
> >  }
> >
> > +static ssize_t direction_show(struct device *dev,
> > +			      struct device_attribute *attr,
> > +			      char *buf)
> > +{
> > +	struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer;
> > +
> > +	switch (buffer->direction) {
> > +	case IIO_BUFFER_DIRECTION_IN:
> > +		return sprintf(buf, "in\n");
> > +	case IIO_BUFFER_DIRECTION_OUT:
> > +		return sprintf(buf, "out\n");
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> >  static DEVICE_ATTR(length, S_IRUGO | S_IWUSR, iio_buffer_read_length,
> >  		   iio_buffer_write_length);
> >  static struct device_attribute dev_attr_length_ro = __ATTR(length, @@
> > -1289,12 +1403,20 @@ static struct device_attribute
> dev_attr_watermark_ro = __ATTR(watermark,
> >  	S_IRUGO, iio_buffer_show_watermark, NULL);  static
> > DEVICE_ATTR(data_available, S_IRUGO,
> >  		iio_dma_show_data_available, NULL);
> > +static DEVICE_ATTR_RO(direction);
> >
> > +/*
> > + * When adding new attributes here, put the at the end, at least
> > +until
> > + * the code that handles the length/length_ro &
> > +watermark/watermark_ro
> > + * assignments gets cleaned up. Otherwise these can create some weird
> > + * duplicate attributes errors under some setups.
> > + */
> >  static struct attribute *iio_buffer_attrs[] = {
> >  	&dev_attr_length.attr,
> >  	&dev_attr_enable.attr,
> >  	&dev_attr_watermark.attr,
> >  	&dev_attr_data_available.attr,
> > +	&dev_attr_direction.attr,
> >  };
> >
> >  #define to_dev_attr(_attr) container_of(_attr, struct
> > device_attribute, attr) @@ -1397,6 +1519,7 @@ static const struct
> file_operations iio_buffer_chrdev_fileops = {
> >  	.owner = THIS_MODULE,
> >  	.llseek = noop_llseek,
> >  	.read = iio_buffer_read,
> > +	.write = iio_buffer_write,
> >  	.poll = iio_buffer_poll,
> >  	.release = iio_buffer_chrdev_release,  }; diff --git
> > a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index 2dbb37e09b8c..537a08549a69 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -1822,6 +1822,7 @@ static const struct file_operations
> iio_buffer_fileops = {
> >  	.owner = THIS_MODULE,
> >  	.llseek = noop_llseek,
> >  	.read = iio_buffer_read_outer_addr,
> > +	.write = iio_buffer_write_outer_addr,
> >  	.poll = iio_buffer_poll_addr,
> >  	.unlocked_ioctl = iio_ioctl,
> >  	.compat_ioctl = compat_ptr_ioctl,
> > diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
> > index b6928ac5c63d..fe2e680d9b5e 100644
> > --- a/include/linux/iio/buffer.h
> > +++ b/include/linux/iio/buffer.h
> > @@ -11,8 +11,15 @@
> >
> >  struct iio_buffer;
> >
> > +enum iio_buffer_direction {
> > +	IIO_BUFFER_DIRECTION_IN,
> > +	IIO_BUFFER_DIRECTION_OUT,
> > +};
> > +
> >  int iio_push_to_buffers(struct iio_dev *indio_dev, const void *data);
> >
> > +int iio_pop_from_buffer(struct iio_buffer *buffer, void *data);
> > +
> >  /**
> >   * iio_push_to_buffers_with_timestamp() - push data and timestamp to
> buffers
> >   * @indio_dev:		iio_dev structure for device.
> > diff --git a/include/linux/iio/buffer_impl.h
> > b/include/linux/iio/buffer_impl.h index 245b32918ae1..e2ca8ea23e19
> > 100644
> > --- a/include/linux/iio/buffer_impl.h
> > +++ b/include/linux/iio/buffer_impl.h
> > @@ -7,6 +7,7 @@
> >  #ifdef CONFIG_IIO_BUFFER
> >
> >  #include <uapi/linux/iio/buffer.h>
> > +#include <linux/iio/buffer.h>
> >
> >  struct iio_dev;
> >  struct iio_buffer;
> > @@ -23,6 +24,10 @@ struct iio_buffer;
> >   * @read:		try to get a specified number of bytes (must exist)
> >   * @data_available:	indicates how much data is available for reading from
> >   *			the buffer.
> > + * @remove_from:	remove scan from buffer. Drivers should calls this to
> > + *			remove a scan from a buffer.
> > + * @write:		try to write a number of bytes
> > + * @space_available:	returns the amount of bytes available in a
> buffer
> >   * @request_update:	if a parameter change has been marked,
> update underlying
> >   *			storage.
> >   * @set_bytes_per_datum:set number of bytes per datum @@ -49,6
> +54,9
> > @@ struct iio_buffer_access_funcs {
> >  	int (*store_to)(struct iio_buffer *buffer, const void *data);
> >  	int (*read)(struct iio_buffer *buffer, size_t n, char __user *buf);
> >  	size_t (*data_available)(struct iio_buffer *buffer);
> > +	int (*remove_from)(struct iio_buffer *buffer, void *data);
> > +	int (*write)(struct iio_buffer *buffer, size_t n, const char __user
> *buf);
> > +	size_t (*space_available)(struct iio_buffer *buffer);
> >
> >  	int (*request_update)(struct iio_buffer *buffer);
> >
> > @@ -80,6 +88,9 @@ struct iio_buffer {
> >  	/**  @bytes_per_datum: Size of individual datum including
> timestamp. */
> >  	size_t bytes_per_datum;
> >
> > +	/* @direction: Direction of the data stream (in/out). */
> > +	enum iio_buffer_direction direction;
> > +
> >  	/**
> >  	 * @access: Buffer access functions associated with the
> >  	 * implementation.

RE: [PATCH v6 1/6] iio: Add output buffer support

[Chindris, Mihail]

> -----Original Message-----
> From: Chindris, Mihail
> Sent: Thursday, 7 October 2021 19:25
> To: Jonathan Cameron <jic23@kernel.org>
> Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> lars@metafoo.de; Hennerich, Michael <Michael.Hennerich@analog.com>;
> Sa, Nuno <Nuno.Sa@analog.com>; Bogdan, Dragos
> <Dragos.Bogdan@analog.com>; alexandru.ardelean@analog.com
> Subject: RE: [PATCH v6 1/6] iio: Add output buffer support
> 
> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Thursday, 7 October 2021 19:06
> > To: Chindris, Mihail <Mihail.Chindris@analog.com>
> > Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> > lars@metafoo.de; Hennerich, Michael <Michael.Hennerich@analog.com>;
> > Sa, Nuno <Nuno.Sa@analog.com>; Bogdan, Dragos
> > <Dragos.Bogdan@analog.com>; alexandru.ardelean@analog.com
> > Subject: Re: [PATCH v6 1/6] iio: Add output buffer support
> >
> > On Thu, 7 Oct 2021 08:00:30 +0000
> > Mihail Chindris <mihail.chindris@analog.com> wrote:
> >
> > > 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.
> > >
> > > Drivers can remove data from a buffer using iio_pop_from_buffer(),
> > > 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 <lars@metafoo.de>
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
> >
> > Hi Mihail,
> >
> > I'm fine with this series now, but one question for this patch on
> > whether we can clarify the author chain.
> >
> > The above might mean one of two things.
> > 1) Lars wrote the code and Alex and yourself just 'handled' the patch on its
> >    way to posting.  If that were the case it should have a From: for
> > Lars
> > 2) All 3 were involved in changes to this patch.  In that case we
> > should have
> > Co-developed-by: lines for lars and Alex as described:
> > https://urldefense.com/v3/__https://elixir.bootlin.com/linux/latest/so
> > urce/
> > Documentation/process/submitting-
> > patches.rst*L475__;Iw!!A3Ni8CS0y2Y!t1IaOEZX3w2pZXl-
> > RyAlXgPPvxFtCON74ppfGuMgV_pKZcNjsLs-dKSk_mA34IsmlOU$
> >
> > This patch has a history well predating the Co-developed-tag but I'm
> > happy to add that if you can confirm that matches with the intent.
> >
> > Good to leave on the list for a few days anyway in case anyone else
> > wants to take a quick look.
> >
> > I'm looking forwards to merging this and thinking back to when Lars
> > originally discussed this feature with me rather a lot of years back!
> >
> > Jonathan
> >
> >
> 
> Hi Jonathan,
> 
> What I have done with the patch was to take the V3 that Alex has left and
> implement the feedback. So I think this is considered as 'handled' from my
> side.
> What happened before V3 I think is better if Alex spokes for himself because
> I don't really know :)
> 
> Mihail
> 

CC: Alexandru Ardelean <ardeleanalex@gmail.com>

> >
> > > ---
> > >  drivers/iio/iio_core.h            |   4 +
> > >  drivers/iio/industrialio-buffer.c | 127
> > +++++++++++++++++++++++++++++-
> > >  drivers/iio/industrialio-core.c   |   1 +
> > >  include/linux/iio/buffer.h        |   7 ++
> > >  include/linux/iio/buffer_impl.h   |  11 +++
> > >  5 files changed, 148 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h index
> > > 8f4a9b264962..61e318431de9 100644
> > > --- a/drivers/iio/iio_core.h
> > > +++ b/drivers/iio/iio_core.h
> > > @@ -68,12 +68,15 @@ __poll_t iio_buffer_poll_wrapper(struct file *filp,
> > >  				 struct poll_table_struct *wait);  ssize_t
> > > iio_buffer_read_wrapper(struct file *filp, char __user *buf,
> > >  				size_t n, loff_t *f_ps);
> > > +ssize_t iio_buffer_write_wrapper(struct file *filp, const char __user
> *buf,
> > > +				 size_t n, loff_t *f_ps);
> > >
> > >  int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev);
> > > void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev);
> > >
> > >  #define iio_buffer_poll_addr (&iio_buffer_poll_wrapper)  #define
> > > iio_buffer_read_outer_addr (&iio_buffer_read_wrapper)
> > > +#define iio_buffer_write_outer_addr (&iio_buffer_write_wrapper)
> > >
> > >  void iio_disable_all_buffers(struct iio_dev *indio_dev);  void
> > > iio_buffer_wakeup_poll(struct iio_dev *indio_dev); @@ -83,6 +86,7 @@
> > > void iio_device_detach_buffers(struct iio_dev *indio_dev);
> > >
> > >  #define iio_buffer_poll_addr NULL
> > >  #define iio_buffer_read_outer_addr NULL
> > > +#define iio_buffer_write_outer_addr NULL
> > >
> > >  static inline int iio_buffers_alloc_sysfs_and_mask(struct iio_dev
> > > *indio_dev)  { diff --git a/drivers/iio/industrialio-buffer.c
> > > b/drivers/iio/industrialio-buffer.c
> > > index a95cc2da56be..7286563e6234 100644
> > > --- a/drivers/iio/industrialio-buffer.c
> > > +++ b/drivers/iio/industrialio-buffer.c
> > > @@ -120,6 +120,9 @@ static ssize_t iio_buffer_read(struct file
> > > *filp, char
> > __user *buf,
> > >  	if (!rb || !rb->access->read)
> > >  		return -EINVAL;
> > >
> > > +	if (rb->direction != IIO_BUFFER_DIRECTION_IN)
> > > +		return -EPERM;
> > > +
> > >  	datum_size = rb->bytes_per_datum;
> > >
> > >  	/*
> > > @@ -161,6 +164,65 @@ 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);
> > > +	int ret;
> > > +	size_t written;
> > > +
> > > +	if (!indio_dev->info)
> > > +		return -ENODEV;
> > > +
> > > +	if (!rb || !rb->access->write)
> > > +		return -EINVAL;
> > > +
> > > +	if (rb->direction != IIO_BUFFER_DIRECTION_OUT)
> > > +		return -EPERM;
> > > +
> > > +	written = 0;
> > > +	add_wait_queue(&rb->pollq, &wait);
> > > +	do {
> > > +		if (indio_dev->info == NULL)
> > > +			return -ENODEV;
> > > +
> > > +		if (!iio_buffer_space_available(rb)) {
> > > +			if (signal_pending(current)) {
> > > +				ret = -ERESTARTSYS;
> > > +				break;
> > > +			}
> > > +
> > > +			wait_woken(&wait, TASK_INTERRUPTIBLE,
> > > +					MAX_SCHEDULE_TIMEOUT);
> > > +			continue;
> > > +		}
> > > +
> > > +		ret = rb->access->write(rb, n - written, buf + written);
> > > +		if (ret == 0 && (filp->f_flags & O_NONBLOCK))
> > > +			ret = -EAGAIN;
> > > +
> > > +		if (ret > 0) {
> > > +			written += ret;
> > > +			if (written != n && !(filp->f_flags & O_NONBLOCK))
> > > +				continue;
> > > +		}
> > > +	} while (ret == 0);
> > > +	remove_wait_queue(&rb->pollq, &wait);
> > > +
> > > +	return ret < 0 ? ret : n;
> > > +}
> > > +
> > >  /**
> > >   * iio_buffer_poll() - poll the buffer to find out if it has data
> > >   * @filp:	File structure pointer for device access
> > > @@ -181,8 +243,18 @@ static __poll_t iio_buffer_poll(struct file *filp,
> > >  		return 0;
> > >
> > >  	poll_wait(filp, &rb->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))
> > > +			return EPOLLOUT | EPOLLWRNORM;
> > > +		break;
> > > +	}
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -199,6 +271,19 @@ ssize_t iio_buffer_read_wrapper(struct file
> > > *filp,
> > char __user *buf,
> > >  	return iio_buffer_read(filp, buf, n, f_ps);  }
> > >
> > > +ssize_t iio_buffer_write_wrapper(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;
> > > +
> > > +	/* check if buffer was opened through new API */
> > > +	if (test_bit(IIO_BUSY_BIT_POS, &rb->flags))
> > > +		return -EBUSY;
> > > +
> > > +	return iio_buffer_write(filp, buf, n, f_ps); }
> > > +
> > >  __poll_t iio_buffer_poll_wrapper(struct file *filp,
> > >  				 struct poll_table_struct *wait)  { @@ -231,6
> +316,15 @@ void
> > > iio_buffer_wakeup_poll(struct iio_dev
> > *indio_dev)
> > >  	}
> > >  }
> > >
> > > +int iio_pop_from_buffer(struct iio_buffer *buffer, void *data) {
> > > +	if (!buffer || !buffer->access || !buffer->access->remove_from)
> > > +		return -EINVAL;
> > > +
> > > +	return buffer->access->remove_from(buffer, data); }
> > > +EXPORT_SYMBOL_GPL(iio_pop_from_buffer);
> > > +
> > >  void iio_buffer_init(struct iio_buffer *buffer)  {
> > >  	INIT_LIST_HEAD(&buffer->demux_list);
> > > @@ -1156,6 +1250,10 @@ int iio_update_buffers(struct iio_dev
> > *indio_dev,
> > >  	if (insert_buffer == remove_buffer)
> > >  		return 0;
> > >
> > > +	if (insert_buffer &&
> > > +	    (insert_buffer->direction == IIO_BUFFER_DIRECTION_OUT))
> > > +		return -EINVAL;
> > > +
> > >  	mutex_lock(&iio_dev_opaque->info_exist_lock);
> > >  	mutex_lock(&indio_dev->mlock);
> > >
> > > @@ -1277,6 +1375,22 @@ static ssize_t
> > iio_dma_show_data_available(struct device *dev,
> > >  	return sysfs_emit(buf, "%zu\n",
> > > iio_buffer_data_available(buffer));
> > >  }
> > >
> > > +static ssize_t direction_show(struct device *dev,
> > > +			      struct device_attribute *attr,
> > > +			      char *buf)
> > > +{
> > > +	struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer;
> > > +
> > > +	switch (buffer->direction) {
> > > +	case IIO_BUFFER_DIRECTION_IN:
> > > +		return sprintf(buf, "in\n");
> > > +	case IIO_BUFFER_DIRECTION_OUT:
> > > +		return sprintf(buf, "out\n");
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +}
> > > +
> > >  static DEVICE_ATTR(length, S_IRUGO | S_IWUSR,
> iio_buffer_read_length,
> > >  		   iio_buffer_write_length);
> > >  static struct device_attribute dev_attr_length_ro = __ATTR(length,
> > > @@
> > > -1289,12 +1403,20 @@ static struct device_attribute
> > dev_attr_watermark_ro = __ATTR(watermark,
> > >  	S_IRUGO, iio_buffer_show_watermark, NULL);  static
> > > DEVICE_ATTR(data_available, S_IRUGO,
> > >  		iio_dma_show_data_available, NULL);
> > > +static DEVICE_ATTR_RO(direction);
> > >
> > > +/*
> > > + * When adding new attributes here, put the at the end, at least
> > > +until
> > > + * the code that handles the length/length_ro &
> > > +watermark/watermark_ro
> > > + * assignments gets cleaned up. Otherwise these can create some
> > > +weird
> > > + * duplicate attributes errors under some setups.
> > > + */
> > >  static struct attribute *iio_buffer_attrs[] = {
> > >  	&dev_attr_length.attr,
> > >  	&dev_attr_enable.attr,
> > >  	&dev_attr_watermark.attr,
> > >  	&dev_attr_data_available.attr,
> > > +	&dev_attr_direction.attr,
> > >  };
> > >
> > >  #define to_dev_attr(_attr) container_of(_attr, struct
> > > device_attribute, attr) @@ -1397,6 +1519,7 @@ static const struct
> > file_operations iio_buffer_chrdev_fileops = {
> > >  	.owner = THIS_MODULE,
> > >  	.llseek = noop_llseek,
> > >  	.read = iio_buffer_read,
> > > +	.write = iio_buffer_write,
> > >  	.poll = iio_buffer_poll,
> > >  	.release = iio_buffer_chrdev_release,  }; diff --git
> > > a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > > index 2dbb37e09b8c..537a08549a69 100644
> > > --- a/drivers/iio/industrialio-core.c
> > > +++ b/drivers/iio/industrialio-core.c
> > > @@ -1822,6 +1822,7 @@ static const struct file_operations
> > iio_buffer_fileops = {
> > >  	.owner = THIS_MODULE,
> > >  	.llseek = noop_llseek,
> > >  	.read = iio_buffer_read_outer_addr,
> > > +	.write = iio_buffer_write_outer_addr,
> > >  	.poll = iio_buffer_poll_addr,
> > >  	.unlocked_ioctl = iio_ioctl,
> > >  	.compat_ioctl = compat_ptr_ioctl,
> > > diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
> > > index b6928ac5c63d..fe2e680d9b5e 100644
> > > --- a/include/linux/iio/buffer.h
> > > +++ b/include/linux/iio/buffer.h
> > > @@ -11,8 +11,15 @@
> > >
> > >  struct iio_buffer;
> > >
> > > +enum iio_buffer_direction {
> > > +	IIO_BUFFER_DIRECTION_IN,
> > > +	IIO_BUFFER_DIRECTION_OUT,
> > > +};
> > > +
> > >  int iio_push_to_buffers(struct iio_dev *indio_dev, const void
> > > *data);
> > >
> > > +int iio_pop_from_buffer(struct iio_buffer *buffer, void *data);
> > > +
> > >  /**
> > >   * iio_push_to_buffers_with_timestamp() - push data and timestamp
> > > to
> > buffers
> > >   * @indio_dev:		iio_dev structure for device.
> > > diff --git a/include/linux/iio/buffer_impl.h
> > > b/include/linux/iio/buffer_impl.h index 245b32918ae1..e2ca8ea23e19
> > > 100644
> > > --- a/include/linux/iio/buffer_impl.h
> > > +++ b/include/linux/iio/buffer_impl.h
> > > @@ -7,6 +7,7 @@
> > >  #ifdef CONFIG_IIO_BUFFER
> > >
> > >  #include <uapi/linux/iio/buffer.h>
> > > +#include <linux/iio/buffer.h>
> > >
> > >  struct iio_dev;
> > >  struct iio_buffer;
> > > @@ -23,6 +24,10 @@ struct iio_buffer;
> > >   * @read:		try to get a specified number of bytes (must exist)
> > >   * @data_available:	indicates how much data is available for
> reading from
> > >   *			the buffer.
> > > + * @remove_from:	remove scan from buffer. Drivers should calls
> this to
> > > + *			remove a scan from a buffer.
> > > + * @write:		try to write a number of bytes
> > > + * @space_available:	returns the amount of bytes available in a
> > buffer
> > >   * @request_update:	if a parameter change has been marked,
> > update underlying
> > >   *			storage.
> > >   * @set_bytes_per_datum:set number of bytes per datum @@ -49,6
> > +54,9
> > > @@ struct iio_buffer_access_funcs {
> > >  	int (*store_to)(struct iio_buffer *buffer, const void *data);
> > >  	int (*read)(struct iio_buffer *buffer, size_t n, char __user *buf);
> > >  	size_t (*data_available)(struct iio_buffer *buffer);
> > > +	int (*remove_from)(struct iio_buffer *buffer, void *data);
> > > +	int (*write)(struct iio_buffer *buffer, size_t n, const char
> > > +__user
> > *buf);
> > > +	size_t (*space_available)(struct iio_buffer *buffer);
> > >
> > >  	int (*request_update)(struct iio_buffer *buffer);
> > >
> > > @@ -80,6 +88,9 @@ struct iio_buffer {
> > >  	/**  @bytes_per_datum: Size of individual datum including
> > timestamp. */
> > >  	size_t bytes_per_datum;
> > >
> > > +	/* @direction: Direction of the data stream (in/out). */
> > > +	enum iio_buffer_direction direction;
> > > +
> > >  	/**
> > >  	 * @access: Buffer access functions associated with the
> > >  	 * implementation.

Re: [PATCH v6 1/6] iio: Add output buffer support

[Jonathan Cameron]

On Thu, 7 Oct 2021 16:24:52 +0000
"Chindris, Mihail" <Mihail.Chindris@analog.com> wrote:

> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Thursday, 7 October 2021 19:06
> > To: Chindris, Mihail <Mihail.Chindris@analog.com>
> > Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> > lars@metafoo.de; Hennerich, Michael <Michael.Hennerich@analog.com>;
> > Sa, Nuno <Nuno.Sa@analog.com>; Bogdan, Dragos
> > <Dragos.Bogdan@analog.com>; alexandru.ardelean@analog.com
> > Subject: Re: [PATCH v6 1/6] iio: Add output buffer support
> > 
> > On Thu, 7 Oct 2021 08:00:30 +0000
> > Mihail Chindris <mihail.chindris@analog.com> wrote:
> >   
> > > 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.
> > >
> > > Drivers can remove data from a buffer using iio_pop_from_buffer(), 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 <lars@metafoo.de>
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>  
> > 
> > Hi Mihail,
> > 
> > I'm fine with this series now, but one question for this patch on whether we
> > can clarify the author chain.
> > 
> > The above might mean one of two things.
> > 1) Lars wrote the code and Alex and yourself just 'handled' the patch on its
> >    way to posting.  If that were the case it should have a From: for Lars
> > 2) All 3 were involved in changes to this patch.  In that case we should have
> > Co-developed-by: lines for lars and Alex as described:
> > https://urldefense.com/v3/__https://elixir.bootlin.com/linux/latest/source/
> > Documentation/process/submitting-
> > patches.rst*L475__;Iw!!A3Ni8CS0y2Y!t1IaOEZX3w2pZXl-
> > RyAlXgPPvxFtCON74ppfGuMgV_pKZcNjsLs-dKSk_mA34IsmlOU$
> > 
> > This patch has a history well predating the Co-developed-tag but I'm happy
> > to add that if you can confirm that matches with the intent.
> > 
> > Good to leave on the list for a few days anyway in case anyone else wants to
> > take a quick look.
> > 
> > I'm looking forwards to merging this and thinking back to when Lars originally
> > discussed this feature with me rather a lot of years back!
> > 
> > Jonathan
> > 
> >   
> 
> Hi Jonathan,
> 
> What I have done with the patch was to take the V3 that Alex has left and implement the feedback. So I think this is considered as 'handled' from my side.
> What happened before V3 I think is better if Alex spokes for himself because I don't really know :)

Alex / Lars, what would you consider a fair way to represent this?

(added some working addresses for Alex...)


> 
> Mihail
> 
> >   
> > > ---
> > >  drivers/iio/iio_core.h            |   4 +
> > >  drivers/iio/industrialio-buffer.c | 127  
> > +++++++++++++++++++++++++++++-  
> > >  drivers/iio/industrialio-core.c   |   1 +
> > >  include/linux/iio/buffer.h        |   7 ++
> > >  include/linux/iio/buffer_impl.h   |  11 +++
> > >  5 files changed, 148 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h index
> > > 8f4a9b264962..61e318431de9 100644
> > > --- a/drivers/iio/iio_core.h
> > > +++ b/drivers/iio/iio_core.h
> > > @@ -68,12 +68,15 @@ __poll_t iio_buffer_poll_wrapper(struct file *filp,
> > >  				 struct poll_table_struct *wait);  ssize_t
> > > iio_buffer_read_wrapper(struct file *filp, char __user *buf,
> > >  				size_t n, loff_t *f_ps);
> > > +ssize_t iio_buffer_write_wrapper(struct file *filp, const char __user *buf,
> > > +				 size_t n, loff_t *f_ps);
> > >
> > >  int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev);
> > > void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev);
> > >
> > >  #define iio_buffer_poll_addr (&iio_buffer_poll_wrapper)  #define
> > > iio_buffer_read_outer_addr (&iio_buffer_read_wrapper)
> > > +#define iio_buffer_write_outer_addr (&iio_buffer_write_wrapper)
> > >
> > >  void iio_disable_all_buffers(struct iio_dev *indio_dev);  void
> > > iio_buffer_wakeup_poll(struct iio_dev *indio_dev); @@ -83,6 +86,7 @@
> > > void iio_device_detach_buffers(struct iio_dev *indio_dev);
> > >
> > >  #define iio_buffer_poll_addr NULL
> > >  #define iio_buffer_read_outer_addr NULL
> > > +#define iio_buffer_write_outer_addr NULL
> > >
> > >  static inline int iio_buffers_alloc_sysfs_and_mask(struct iio_dev
> > > *indio_dev)  { diff --git a/drivers/iio/industrialio-buffer.c
> > > b/drivers/iio/industrialio-buffer.c
> > > index a95cc2da56be..7286563e6234 100644
> > > --- a/drivers/iio/industrialio-buffer.c
> > > +++ b/drivers/iio/industrialio-buffer.c
> > > @@ -120,6 +120,9 @@ static ssize_t iio_buffer_read(struct file *filp, char  
> > __user *buf,  
> > >  	if (!rb || !rb->access->read)
> > >  		return -EINVAL;
> > >
> > > +	if (rb->direction != IIO_BUFFER_DIRECTION_IN)
> > > +		return -EPERM;
> > > +
> > >  	datum_size = rb->bytes_per_datum;
> > >
> > >  	/*
> > > @@ -161,6 +164,65 @@ 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);
> > > +	int ret;
> > > +	size_t written;
> > > +
> > > +	if (!indio_dev->info)
> > > +		return -ENODEV;
> > > +
> > > +	if (!rb || !rb->access->write)
> > > +		return -EINVAL;
> > > +
> > > +	if (rb->direction != IIO_BUFFER_DIRECTION_OUT)
> > > +		return -EPERM;
> > > +
> > > +	written = 0;
> > > +	add_wait_queue(&rb->pollq, &wait);
> > > +	do {
> > > +		if (indio_dev->info == NULL)
> > > +			return -ENODEV;
> > > +
> > > +		if (!iio_buffer_space_available(rb)) {
> > > +			if (signal_pending(current)) {
> > > +				ret = -ERESTARTSYS;
> > > +				break;
> > > +			}
> > > +
> > > +			wait_woken(&wait, TASK_INTERRUPTIBLE,
> > > +					MAX_SCHEDULE_TIMEOUT);
> > > +			continue;
> > > +		}
> > > +
> > > +		ret = rb->access->write(rb, n - written, buf + written);
> > > +		if (ret == 0 && (filp->f_flags & O_NONBLOCK))
> > > +			ret = -EAGAIN;
> > > +
> > > +		if (ret > 0) {
> > > +			written += ret;
> > > +			if (written != n && !(filp->f_flags & O_NONBLOCK))
> > > +				continue;
> > > +		}
> > > +	} while (ret == 0);
> > > +	remove_wait_queue(&rb->pollq, &wait);
> > > +
> > > +	return ret < 0 ? ret : n;
> > > +}
> > > +
> > >  /**
> > >   * iio_buffer_poll() - poll the buffer to find out if it has data
> > >   * @filp:	File structure pointer for device access
> > > @@ -181,8 +243,18 @@ static __poll_t iio_buffer_poll(struct file *filp,
> > >  		return 0;
> > >
> > >  	poll_wait(filp, &rb->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))
> > > +			return EPOLLOUT | EPOLLWRNORM;
> > > +		break;
> > > +	}
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -199,6 +271,19 @@ ssize_t iio_buffer_read_wrapper(struct file *filp,  
> > char __user *buf,  
> > >  	return iio_buffer_read(filp, buf, n, f_ps);  }
> > >
> > > +ssize_t iio_buffer_write_wrapper(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;
> > > +
> > > +	/* check if buffer was opened through new API */
> > > +	if (test_bit(IIO_BUSY_BIT_POS, &rb->flags))
> > > +		return -EBUSY;
> > > +
> > > +	return iio_buffer_write(filp, buf, n, f_ps); }
> > > +
> > >  __poll_t iio_buffer_poll_wrapper(struct file *filp,
> > >  				 struct poll_table_struct *wait)
> > >  {
> > > @@ -231,6 +316,15 @@ void iio_buffer_wakeup_poll(struct iio_dev  
> > *indio_dev)  
> > >  	}
> > >  }
> > >
> > > +int iio_pop_from_buffer(struct iio_buffer *buffer, void *data) {
> > > +	if (!buffer || !buffer->access || !buffer->access->remove_from)
> > > +		return -EINVAL;
> > > +
> > > +	return buffer->access->remove_from(buffer, data); }
> > > +EXPORT_SYMBOL_GPL(iio_pop_from_buffer);
> > > +
> > >  void iio_buffer_init(struct iio_buffer *buffer)  {
> > >  	INIT_LIST_HEAD(&buffer->demux_list);
> > > @@ -1156,6 +1250,10 @@ int iio_update_buffers(struct iio_dev  
> > *indio_dev,  
> > >  	if (insert_buffer == remove_buffer)
> > >  		return 0;
> > >
> > > +	if (insert_buffer &&
> > > +	    (insert_buffer->direction == IIO_BUFFER_DIRECTION_OUT))
> > > +		return -EINVAL;
> > > +
> > >  	mutex_lock(&iio_dev_opaque->info_exist_lock);
> > >  	mutex_lock(&indio_dev->mlock);
> > >
> > > @@ -1277,6 +1375,22 @@ static ssize_t  
> > iio_dma_show_data_available(struct device *dev,  
> > >  	return sysfs_emit(buf, "%zu\n", iio_buffer_data_available(buffer));
> > >  }
> > >
> > > +static ssize_t direction_show(struct device *dev,
> > > +			      struct device_attribute *attr,
> > > +			      char *buf)
> > > +{
> > > +	struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer;
> > > +
> > > +	switch (buffer->direction) {
> > > +	case IIO_BUFFER_DIRECTION_IN:
> > > +		return sprintf(buf, "in\n");
> > > +	case IIO_BUFFER_DIRECTION_OUT:
> > > +		return sprintf(buf, "out\n");
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +}
> > > +
> > >  static DEVICE_ATTR(length, S_IRUGO | S_IWUSR, iio_buffer_read_length,
> > >  		   iio_buffer_write_length);
> > >  static struct device_attribute dev_attr_length_ro = __ATTR(length, @@
> > > -1289,12 +1403,20 @@ static struct device_attribute  
> > dev_attr_watermark_ro = __ATTR(watermark,  
> > >  	S_IRUGO, iio_buffer_show_watermark, NULL);  static
> > > DEVICE_ATTR(data_available, S_IRUGO,
> > >  		iio_dma_show_data_available, NULL);
> > > +static DEVICE_ATTR_RO(direction);
> > >
> > > +/*
> > > + * When adding new attributes here, put the at the end, at least
> > > +until
> > > + * the code that handles the length/length_ro &
> > > +watermark/watermark_ro
> > > + * assignments gets cleaned up. Otherwise these can create some weird
> > > + * duplicate attributes errors under some setups.
> > > + */
> > >  static struct attribute *iio_buffer_attrs[] = {
> > >  	&dev_attr_length.attr,
> > >  	&dev_attr_enable.attr,
> > >  	&dev_attr_watermark.attr,
> > >  	&dev_attr_data_available.attr,
> > > +	&dev_attr_direction.attr,
> > >  };
> > >
> > >  #define to_dev_attr(_attr) container_of(_attr, struct
> > > device_attribute, attr) @@ -1397,6 +1519,7 @@ static const struct  
> > file_operations iio_buffer_chrdev_fileops = {  
> > >  	.owner = THIS_MODULE,
> > >  	.llseek = noop_llseek,
> > >  	.read = iio_buffer_read,
> > > +	.write = iio_buffer_write,
> > >  	.poll = iio_buffer_poll,
> > >  	.release = iio_buffer_chrdev_release,  }; diff --git
> > > a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > > index 2dbb37e09b8c..537a08549a69 100644
> > > --- a/drivers/iio/industrialio-core.c
> > > +++ b/drivers/iio/industrialio-core.c
> > > @@ -1822,6 +1822,7 @@ static const struct file_operations  
> > iio_buffer_fileops = {  
> > >  	.owner = THIS_MODULE,
> > >  	.llseek = noop_llseek,
> > >  	.read = iio_buffer_read_outer_addr,
> > > +	.write = iio_buffer_write_outer_addr,
> > >  	.poll = iio_buffer_poll_addr,
> > >  	.unlocked_ioctl = iio_ioctl,
> > >  	.compat_ioctl = compat_ptr_ioctl,
> > > diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
> > > index b6928ac5c63d..fe2e680d9b5e 100644
> > > --- a/include/linux/iio/buffer.h
> > > +++ b/include/linux/iio/buffer.h
> > > @@ -11,8 +11,15 @@
> > >
> > >  struct iio_buffer;
> > >
> > > +enum iio_buffer_direction {
> > > +	IIO_BUFFER_DIRECTION_IN,
> > > +	IIO_BUFFER_DIRECTION_OUT,
> > > +};
> > > +
> > >  int iio_push_to_buffers(struct iio_dev *indio_dev, const void *data);
> > >
> > > +int iio_pop_from_buffer(struct iio_buffer *buffer, void *data);
> > > +
> > >  /**
> > >   * iio_push_to_buffers_with_timestamp() - push data and timestamp to  
> > buffers  
> > >   * @indio_dev:		iio_dev structure for device.
> > > diff --git a/include/linux/iio/buffer_impl.h
> > > b/include/linux/iio/buffer_impl.h index 245b32918ae1..e2ca8ea23e19
> > > 100644
> > > --- a/include/linux/iio/buffer_impl.h
> > > +++ b/include/linux/iio/buffer_impl.h
> > > @@ -7,6 +7,7 @@
> > >  #ifdef CONFIG_IIO_BUFFER
> > >
> > >  #include <uapi/linux/iio/buffer.h>
> > > +#include <linux/iio/buffer.h>
> > >
> > >  struct iio_dev;
> > >  struct iio_buffer;
> > > @@ -23,6 +24,10 @@ struct iio_buffer;
> > >   * @read:		try to get a specified number of bytes (must exist)
> > >   * @data_available:	indicates how much data is available for reading from
> > >   *			the buffer.
> > > + * @remove_from:	remove scan from buffer. Drivers should calls this to
> > > + *			remove a scan from a buffer.
> > > + * @write:		try to write a number of bytes
> > > + * @space_available:	returns the amount of bytes available in a  
> > buffer  
> > >   * @request_update:	if a parameter change has been marked,  
> > update underlying  
> > >   *			storage.
> > >   * @set_bytes_per_datum:set number of bytes per datum @@ -49,6  
> > +54,9  
> > > @@ struct iio_buffer_access_funcs {
> > >  	int (*store_to)(struct iio_buffer *buffer, const void *data);
> > >  	int (*read)(struct iio_buffer *buffer, size_t n, char __user *buf);
> > >  	size_t (*data_available)(struct iio_buffer *buffer);
> > > +	int (*remove_from)(struct iio_buffer *buffer, void *data);
> > > +	int (*write)(struct iio_buffer *buffer, size_t n, const char __user  
> > *buf);  
> > > +	size_t (*space_available)(struct iio_buffer *buffer);
> > >
> > >  	int (*request_update)(struct iio_buffer *buffer);
> > >
> > > @@ -80,6 +88,9 @@ struct iio_buffer {
> > >  	/**  @bytes_per_datum: Size of individual datum including  
> > timestamp. */  
> > >  	size_t bytes_per_datum;
> > >
> > > +	/* @direction: Direction of the data stream (in/out). */
> > > +	enum iio_buffer_direction direction;
> > > +
> > >  	/**
> > >  	 * @access: Buffer access functions associated with the
> > >  	 * implementation.  
> 

Re: [PATCH v6 1/6] iio: Add output buffer support

[Alexandru Ardelean]

On Thu, Oct 7, 2021 at 8:09 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Thu, 7 Oct 2021 16:24:52 +0000
> "Chindris, Mihail" <Mihail.Chindris@analog.com> wrote:
>
> > > -----Original Message-----
> > > From: Jonathan Cameron <jic23@kernel.org>
> > > Sent: Thursday, 7 October 2021 19:06
> > > To: Chindris, Mihail <Mihail.Chindris@analog.com>
> > > Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> > > lars@metafoo.de; Hennerich, Michael <Michael.Hennerich@analog.com>;
> > > Sa, Nuno <Nuno.Sa@analog.com>; Bogdan, Dragos
> > > <Dragos.Bogdan@analog.com>; alexandru.ardelean@analog.com
> > > Subject: Re: [PATCH v6 1/6] iio: Add output buffer support
> > >
> > > On Thu, 7 Oct 2021 08:00:30 +0000
> > > Mihail Chindris <mihail.chindris@analog.com> wrote:
> > >
> > > > 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.
> > > >
> > > > Drivers can remove data from a buffer using iio_pop_from_buffer(), 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 <lars@metafoo.de>
> > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > > Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
> > >
> > > Hi Mihail,
> > >
> > > I'm fine with this series now, but one question for this patch on whether we
> > > can clarify the author chain.
> > >
> > > The above might mean one of two things.
> > > 1) Lars wrote the code and Alex and yourself just 'handled' the patch on its
> > >    way to posting.  If that were the case it should have a From: for Lars
> > > 2) All 3 were involved in changes to this patch.  In that case we should have
> > > Co-developed-by: lines for lars and Alex as described:
> > > https://urldefense.com/v3/__https://elixir.bootlin.com/linux/latest/source/
> > > Documentation/process/submitting-
> > > patches.rst*L475__;Iw!!A3Ni8CS0y2Y!t1IaOEZX3w2pZXl-
> > > RyAlXgPPvxFtCON74ppfGuMgV_pKZcNjsLs-dKSk_mA34IsmlOU$
> > >
> > > This patch has a history well predating the Co-developed-tag but I'm happy
> > > to add that if you can confirm that matches with the intent.
> > >
> > > Good to leave on the list for a few days anyway in case anyone else wants to
> > > take a quick look.
> > >
> > > I'm looking forwards to merging this and thinking back to when Lars originally
> > > discussed this feature with me rather a lot of years back!
> > >
> > > Jonathan
> > >
> > >
> >
> > Hi Jonathan,
> >
> > What I have done with the patch was to take the V3 that Alex has left and implement the feedback. So I think this is considered as 'handled' from my side.
> > What happened before V3 I think is better if Alex spokes for himself because I don't really know :)
>
> Alex / Lars, what would you consider a fair way to represent this?

I'm fine with whatever representation you (Jonathan) consider.
I'm fine with either tag.

My main contribution here is that the "direction" parameter is part of
"struct iio_buffer", whereas Lars' implementation made it part of
"struct iio_dev".

I seem to recall that there was an "iio_buffer->direction"
implementation before the "iio_dev->direction" in the ADI kernel tree.
Maybe there was some change of mind somewhere.
But the history was pretty messy in those early days (at least that's
what the "git log" told me), as IIO was also moved out of staging and
heavy development was being done.

In any case, this patch works on top of the new multi-buffer support.
Which would mean that with this patch, a new class of ADDAC devices
(not sure how popular they are these days) can be supported.
As well as some basic transceivers.

>
> (added some working addresses for Alex...)
>
>
> >
> > Mihail
> >
> > >
> > > > ---
> > > >  drivers/iio/iio_core.h            |   4 +
> > > >  drivers/iio/industrialio-buffer.c | 127
> > > +++++++++++++++++++++++++++++-
> > > >  drivers/iio/industrialio-core.c   |   1 +
> > > >  include/linux/iio/buffer.h        |   7 ++
> > > >  include/linux/iio/buffer_impl.h   |  11 +++
> > > >  5 files changed, 148 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h index
> > > > 8f4a9b264962..61e318431de9 100644
> > > > --- a/drivers/iio/iio_core.h
> > > > +++ b/drivers/iio/iio_core.h
> > > > @@ -68,12 +68,15 @@ __poll_t iio_buffer_poll_wrapper(struct file *filp,
> > > >                            struct poll_table_struct *wait);  ssize_t
> > > > iio_buffer_read_wrapper(struct file *filp, char __user *buf,
> > > >                           size_t n, loff_t *f_ps);
> > > > +ssize_t iio_buffer_write_wrapper(struct file *filp, const char __user *buf,
> > > > +                          size_t n, loff_t *f_ps);
> > > >
> > > >  int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev);
> > > > void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev);
> > > >
> > > >  #define iio_buffer_poll_addr (&iio_buffer_poll_wrapper)  #define
> > > > iio_buffer_read_outer_addr (&iio_buffer_read_wrapper)
> > > > +#define iio_buffer_write_outer_addr (&iio_buffer_write_wrapper)
> > > >
> > > >  void iio_disable_all_buffers(struct iio_dev *indio_dev);  void
> > > > iio_buffer_wakeup_poll(struct iio_dev *indio_dev); @@ -83,6 +86,7 @@
> > > > void iio_device_detach_buffers(struct iio_dev *indio_dev);
> > > >
> > > >  #define iio_buffer_poll_addr NULL
> > > >  #define iio_buffer_read_outer_addr NULL
> > > > +#define iio_buffer_write_outer_addr NULL
> > > >
> > > >  static inline int iio_buffers_alloc_sysfs_and_mask(struct iio_dev
> > > > *indio_dev)  { diff --git a/drivers/iio/industrialio-buffer.c
> > > > b/drivers/iio/industrialio-buffer.c
> > > > index a95cc2da56be..7286563e6234 100644
> > > > --- a/drivers/iio/industrialio-buffer.c
> > > > +++ b/drivers/iio/industrialio-buffer.c
> > > > @@ -120,6 +120,9 @@ static ssize_t iio_buffer_read(struct file *filp, char
> > > __user *buf,
> > > >   if (!rb || !rb->access->read)
> > > >           return -EINVAL;
> > > >
> > > > + if (rb->direction != IIO_BUFFER_DIRECTION_IN)
> > > > +         return -EPERM;
> > > > +
> > > >   datum_size = rb->bytes_per_datum;
> > > >
> > > >   /*
> > > > @@ -161,6 +164,65 @@ 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);
> > > > + int ret;
> > > > + size_t written;
> > > > +
> > > > + if (!indio_dev->info)
> > > > +         return -ENODEV;
> > > > +
> > > > + if (!rb || !rb->access->write)
> > > > +         return -EINVAL;
> > > > +
> > > > + if (rb->direction != IIO_BUFFER_DIRECTION_OUT)
> > > > +         return -EPERM;
> > > > +
> > > > + written = 0;
> > > > + add_wait_queue(&rb->pollq, &wait);
> > > > + do {
> > > > +         if (indio_dev->info == NULL)
> > > > +                 return -ENODEV;
> > > > +
> > > > +         if (!iio_buffer_space_available(rb)) {
> > > > +                 if (signal_pending(current)) {
> > > > +                         ret = -ERESTARTSYS;
> > > > +                         break;
> > > > +                 }
> > > > +
> > > > +                 wait_woken(&wait, TASK_INTERRUPTIBLE,
> > > > +                                 MAX_SCHEDULE_TIMEOUT);
> > > > +                 continue;
> > > > +         }
> > > > +
> > > > +         ret = rb->access->write(rb, n - written, buf + written);
> > > > +         if (ret == 0 && (filp->f_flags & O_NONBLOCK))
> > > > +                 ret = -EAGAIN;
> > > > +
> > > > +         if (ret > 0) {
> > > > +                 written += ret;
> > > > +                 if (written != n && !(filp->f_flags & O_NONBLOCK))
> > > > +                         continue;
> > > > +         }
> > > > + } while (ret == 0);
> > > > + remove_wait_queue(&rb->pollq, &wait);
> > > > +
> > > > + return ret < 0 ? ret : n;
> > > > +}
> > > > +
> > > >  /**
> > > >   * iio_buffer_poll() - poll the buffer to find out if it has data
> > > >   * @filp:        File structure pointer for device access
> > > > @@ -181,8 +243,18 @@ static __poll_t iio_buffer_poll(struct file *filp,
> > > >           return 0;
> > > >
> > > >   poll_wait(filp, &rb->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))
> > > > +                 return EPOLLOUT | EPOLLWRNORM;
> > > > +         break;
> > > > + }
> > > > +
> > > >   return 0;
> > > >  }
> > > >
> > > > @@ -199,6 +271,19 @@ ssize_t iio_buffer_read_wrapper(struct file *filp,
> > > char __user *buf,
> > > >   return iio_buffer_read(filp, buf, n, f_ps);  }
> > > >
> > > > +ssize_t iio_buffer_write_wrapper(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;
> > > > +
> > > > + /* check if buffer was opened through new API */
> > > > + if (test_bit(IIO_BUSY_BIT_POS, &rb->flags))
> > > > +         return -EBUSY;
> > > > +
> > > > + return iio_buffer_write(filp, buf, n, f_ps); }
> > > > +
> > > >  __poll_t iio_buffer_poll_wrapper(struct file *filp,
> > > >                            struct poll_table_struct *wait)
> > > >  {
> > > > @@ -231,6 +316,15 @@ void iio_buffer_wakeup_poll(struct iio_dev
> > > *indio_dev)
> > > >   }
> > > >  }
> > > >
> > > > +int iio_pop_from_buffer(struct iio_buffer *buffer, void *data) {
> > > > + if (!buffer || !buffer->access || !buffer->access->remove_from)
> > > > +         return -EINVAL;
> > > > +
> > > > + return buffer->access->remove_from(buffer, data); }
> > > > +EXPORT_SYMBOL_GPL(iio_pop_from_buffer);
> > > > +
> > > >  void iio_buffer_init(struct iio_buffer *buffer)  {
> > > >   INIT_LIST_HEAD(&buffer->demux_list);
> > > > @@ -1156,6 +1250,10 @@ int iio_update_buffers(struct iio_dev
> > > *indio_dev,
> > > >   if (insert_buffer == remove_buffer)
> > > >           return 0;
> > > >
> > > > + if (insert_buffer &&
> > > > +     (insert_buffer->direction == IIO_BUFFER_DIRECTION_OUT))
> > > > +         return -EINVAL;
> > > > +
> > > >   mutex_lock(&iio_dev_opaque->info_exist_lock);
> > > >   mutex_lock(&indio_dev->mlock);
> > > >
> > > > @@ -1277,6 +1375,22 @@ static ssize_t
> > > iio_dma_show_data_available(struct device *dev,
> > > >   return sysfs_emit(buf, "%zu\n", iio_buffer_data_available(buffer));
> > > >  }
> > > >
> > > > +static ssize_t direction_show(struct device *dev,
> > > > +                       struct device_attribute *attr,
> > > > +                       char *buf)
> > > > +{
> > > > + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer;
> > > > +
> > > > + switch (buffer->direction) {
> > > > + case IIO_BUFFER_DIRECTION_IN:
> > > > +         return sprintf(buf, "in\n");
> > > > + case IIO_BUFFER_DIRECTION_OUT:
> > > > +         return sprintf(buf, "out\n");
> > > > + default:
> > > > +         return -EINVAL;
> > > > + }
> > > > +}
> > > > +
> > > >  static DEVICE_ATTR(length, S_IRUGO | S_IWUSR, iio_buffer_read_length,
> > > >              iio_buffer_write_length);
> > > >  static struct device_attribute dev_attr_length_ro = __ATTR(length, @@
> > > > -1289,12 +1403,20 @@ static struct device_attribute
> > > dev_attr_watermark_ro = __ATTR(watermark,
> > > >   S_IRUGO, iio_buffer_show_watermark, NULL);  static
> > > > DEVICE_ATTR(data_available, S_IRUGO,
> > > >           iio_dma_show_data_available, NULL);
> > > > +static DEVICE_ATTR_RO(direction);
> > > >
> > > > +/*
> > > > + * When adding new attributes here, put the at the end, at least
> > > > +until
> > > > + * the code that handles the length/length_ro &
> > > > +watermark/watermark_ro
> > > > + * assignments gets cleaned up. Otherwise these can create some weird
> > > > + * duplicate attributes errors under some setups.
> > > > + */
> > > >  static struct attribute *iio_buffer_attrs[] = {
> > > >   &dev_attr_length.attr,
> > > >   &dev_attr_enable.attr,
> > > >   &dev_attr_watermark.attr,
> > > >   &dev_attr_data_available.attr,
> > > > + &dev_attr_direction.attr,
> > > >  };
> > > >
> > > >  #define to_dev_attr(_attr) container_of(_attr, struct
> > > > device_attribute, attr) @@ -1397,6 +1519,7 @@ static const struct
> > > file_operations iio_buffer_chrdev_fileops = {
> > > >   .owner = THIS_MODULE,
> > > >   .llseek = noop_llseek,
> > > >   .read = iio_buffer_read,
> > > > + .write = iio_buffer_write,
> > > >   .poll = iio_buffer_poll,
> > > >   .release = iio_buffer_chrdev_release,  }; diff --git
> > > > a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > > > index 2dbb37e09b8c..537a08549a69 100644
> > > > --- a/drivers/iio/industrialio-core.c
> > > > +++ b/drivers/iio/industrialio-core.c
> > > > @@ -1822,6 +1822,7 @@ static const struct file_operations
> > > iio_buffer_fileops = {
> > > >   .owner = THIS_MODULE,
> > > >   .llseek = noop_llseek,
> > > >   .read = iio_buffer_read_outer_addr,
> > > > + .write = iio_buffer_write_outer_addr,
> > > >   .poll = iio_buffer_poll_addr,
> > > >   .unlocked_ioctl = iio_ioctl,
> > > >   .compat_ioctl = compat_ptr_ioctl,
> > > > diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
> > > > index b6928ac5c63d..fe2e680d9b5e 100644
> > > > --- a/include/linux/iio/buffer.h
> > > > +++ b/include/linux/iio/buffer.h
> > > > @@ -11,8 +11,15 @@
> > > >
> > > >  struct iio_buffer;
> > > >
> > > > +enum iio_buffer_direction {
> > > > + IIO_BUFFER_DIRECTION_IN,
> > > > + IIO_BUFFER_DIRECTION_OUT,
> > > > +};
> > > > +
> > > >  int iio_push_to_buffers(struct iio_dev *indio_dev, const void *data);
> > > >
> > > > +int iio_pop_from_buffer(struct iio_buffer *buffer, void *data);
> > > > +
> > > >  /**
> > > >   * iio_push_to_buffers_with_timestamp() - push data and timestamp to
> > > buffers
> > > >   * @indio_dev:           iio_dev structure for device.
> > > > diff --git a/include/linux/iio/buffer_impl.h
> > > > b/include/linux/iio/buffer_impl.h index 245b32918ae1..e2ca8ea23e19
> > > > 100644
> > > > --- a/include/linux/iio/buffer_impl.h
> > > > +++ b/include/linux/iio/buffer_impl.h
> > > > @@ -7,6 +7,7 @@
> > > >  #ifdef CONFIG_IIO_BUFFER
> > > >
> > > >  #include <uapi/linux/iio/buffer.h>
> > > > +#include <linux/iio/buffer.h>
> > > >
> > > >  struct iio_dev;
> > > >  struct iio_buffer;
> > > > @@ -23,6 +24,10 @@ struct iio_buffer;
> > > >   * @read:                try to get a specified number of bytes (must exist)
> > > >   * @data_available:      indicates how much data is available for reading from
> > > >   *                       the buffer.
> > > > + * @remove_from: remove scan from buffer. Drivers should calls this to
> > > > + *                       remove a scan from a buffer.
> > > > + * @write:               try to write a number of bytes
> > > > + * @space_available:     returns the amount of bytes available in a
> > > buffer
> > > >   * @request_update:      if a parameter change has been marked,
> > > update underlying
> > > >   *                       storage.
> > > >   * @set_bytes_per_datum:set number of bytes per datum @@ -49,6
> > > +54,9
> > > > @@ struct iio_buffer_access_funcs {
> > > >   int (*store_to)(struct iio_buffer *buffer, const void *data);
> > > >   int (*read)(struct iio_buffer *buffer, size_t n, char __user *buf);
> > > >   size_t (*data_available)(struct iio_buffer *buffer);
> > > > + int (*remove_from)(struct iio_buffer *buffer, void *data);
> > > > + int (*write)(struct iio_buffer *buffer, size_t n, const char __user
> > > *buf);
> > > > + size_t (*space_available)(struct iio_buffer *buffer);
> > > >
> > > >   int (*request_update)(struct iio_buffer *buffer);
> > > >
> > > > @@ -80,6 +88,9 @@ struct iio_buffer {
> > > >   /**  @bytes_per_datum: Size of individual datum including
> > > timestamp. */
> > > >   size_t bytes_per_datum;
> > > >
> > > > + /* @direction: Direction of the data stream (in/out). */
> > > > + enum iio_buffer_direction direction;
> > > > +
> > > >   /**
> > > >    * @access: Buffer access functions associated with the
> > > >    * implementation.
> >
>

Re: [PATCH v6 1/6] iio: Add output buffer support

[Jonathan Cameron]

On Fri, 8 Oct 2021 09:56:29 +0300
Alexandru Ardelean <ardeleanalex@gmail.com> wrote:

> On Thu, Oct 7, 2021 at 8:09 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Thu, 7 Oct 2021 16:24:52 +0000
> > "Chindris, Mihail" <Mihail.Chindris@analog.com> wrote:
> >  
> > > > -----Original Message-----
> > > > From: Jonathan Cameron <jic23@kernel.org>
> > > > Sent: Thursday, 7 October 2021 19:06
> > > > To: Chindris, Mihail <Mihail.Chindris@analog.com>
> > > > Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> > > > lars@metafoo.de; Hennerich, Michael <Michael.Hennerich@analog.com>;
> > > > Sa, Nuno <Nuno.Sa@analog.com>; Bogdan, Dragos
> > > > <Dragos.Bogdan@analog.com>; alexandru.ardelean@analog.com
> > > > Subject: Re: [PATCH v6 1/6] iio: Add output buffer support
> > > >
> > > > On Thu, 7 Oct 2021 08:00:30 +0000
> > > > Mihail Chindris <mihail.chindris@analog.com> wrote:
> > > >  
> > > > > 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.
> > > > >
> > > > > Drivers can remove data from a buffer using iio_pop_from_buffer(), 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 <lars@metafoo.de>
> > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > > > Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>  
> > > >
> > > > Hi Mihail,
> > > >
> > > > I'm fine with this series now, but one question for this patch on whether we
> > > > can clarify the author chain.
> > > >
> > > > The above might mean one of two things.
> > > > 1) Lars wrote the code and Alex and yourself just 'handled' the patch on its
> > > >    way to posting.  If that were the case it should have a From: for Lars
> > > > 2) All 3 were involved in changes to this patch.  In that case we should have
> > > > Co-developed-by: lines for lars and Alex as described:
> > > > https://urldefense.com/v3/__https://elixir.bootlin.com/linux/latest/source/
> > > > Documentation/process/submitting-
> > > > patches.rst*L475__;Iw!!A3Ni8CS0y2Y!t1IaOEZX3w2pZXl-
> > > > RyAlXgPPvxFtCON74ppfGuMgV_pKZcNjsLs-dKSk_mA34IsmlOU$
> > > >
> > > > This patch has a history well predating the Co-developed-tag but I'm happy
> > > > to add that if you can confirm that matches with the intent.
> > > >
> > > > Good to leave on the list for a few days anyway in case anyone else wants to
> > > > take a quick look.
> > > >
> > > > I'm looking forwards to merging this and thinking back to when Lars originally
> > > > discussed this feature with me rather a lot of years back!
> > > >
> > > > Jonathan
> > > >
> > > >  
> > >
> > > Hi Jonathan,
> > >
> > > What I have done with the patch was to take the V3 that Alex has left and implement the feedback. So I think this is considered as 'handled' from my side.
> > > What happened before V3 I think is better if Alex spokes for himself because I don't really know :)  
> >
> > Alex / Lars, what would you consider a fair way to represent this?  
> 
> I'm fine with whatever representation you (Jonathan) consider.
> I'm fine with either tag.

I'll add co-developed for Lars and Alex and Mihail is an author by merit
of being first sign off that isn't also co-developed. I think that fairly
reflects the flow.

> 
> My main contribution here is that the "direction" parameter is part of
> "struct iio_buffer", whereas Lars' implementation made it part of
> "struct iio_dev".
> 
> I seem to recall that there was an "iio_buffer->direction"
> implementation before the "iio_dev->direction" in the ADI kernel tree.
> Maybe there was some change of mind somewhere.
> But the history was pretty messy in those early days (at least that's
> what the "git log" told me), as IIO was also moved out of staging and
> heavy development was being done.
> 
> In any case, this patch works on top of the new multi-buffer support.
> Which would mean that with this patch, a new class of ADDAC devices
> (not sure how popular they are these days) can be supported.
> As well as some basic transceivers.
> 
> >
> > (added some working addresses for Alex...)
> >
> >  
> > >
> > > Mihail
> > >  
> > > >  
> > > > > ---
> > > > >  drivers/iio/iio_core.h            |   4 +
> > > > >  drivers/iio/industrialio-buffer.c | 127  
> > > > +++++++++++++++++++++++++++++-  
> > > > >  drivers/iio/industrialio-core.c   |   1 +
> > > > >  include/linux/iio/buffer.h        |   7 ++
> > > > >  include/linux/iio/buffer_impl.h   |  11 +++
> > > > >  5 files changed, 148 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h index
> > > > > 8f4a9b264962..61e318431de9 100644
> > > > > --- a/drivers/iio/iio_core.h
> > > > > +++ b/drivers/iio/iio_core.h
> > > > > @@ -68,12 +68,15 @@ __poll_t iio_buffer_poll_wrapper(struct file *filp,
> > > > >                            struct poll_table_struct *wait);  ssize_t
> > > > > iio_buffer_read_wrapper(struct file *filp, char __user *buf,
> > > > >                           size_t n, loff_t *f_ps);
> > > > > +ssize_t iio_buffer_write_wrapper(struct file *filp, const char __user *buf,
> > > > > +                          size_t n, loff_t *f_ps);
> > > > >
> > > > >  int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev);
> > > > > void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev);
> > > > >
> > > > >  #define iio_buffer_poll_addr (&iio_buffer_poll_wrapper)  #define
> > > > > iio_buffer_read_outer_addr (&iio_buffer_read_wrapper)
> > > > > +#define iio_buffer_write_outer_addr (&iio_buffer_write_wrapper)
> > > > >
> > > > >  void iio_disable_all_buffers(struct iio_dev *indio_dev);  void
> > > > > iio_buffer_wakeup_poll(struct iio_dev *indio_dev); @@ -83,6 +86,7 @@
> > > > > void iio_device_detach_buffers(struct iio_dev *indio_dev);
> > > > >
> > > > >  #define iio_buffer_poll_addr NULL
> > > > >  #define iio_buffer_read_outer_addr NULL
> > > > > +#define iio_buffer_write_outer_addr NULL
> > > > >
> > > > >  static inline int iio_buffers_alloc_sysfs_and_mask(struct iio_dev
> > > > > *indio_dev)  { diff --git a/drivers/iio/industrialio-buffer.c
> > > > > b/drivers/iio/industrialio-buffer.c
> > > > > index a95cc2da56be..7286563e6234 100644
> > > > > --- a/drivers/iio/industrialio-buffer.c
> > > > > +++ b/drivers/iio/industrialio-buffer.c
> > > > > @@ -120,6 +120,9 @@ static ssize_t iio_buffer_read(struct file *filp, char  
> > > > __user *buf,  
> > > > >   if (!rb || !rb->access->read)
> > > > >           return -EINVAL;
> > > > >
> > > > > + if (rb->direction != IIO_BUFFER_DIRECTION_IN)
> > > > > +         return -EPERM;
> > > > > +
> > > > >   datum_size = rb->bytes_per_datum;
> > > > >
> > > > >   /*
> > > > > @@ -161,6 +164,65 @@ 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);
> > > > > + int ret;
> > > > > + size_t written;
> > > > > +
> > > > > + if (!indio_dev->info)
> > > > > +         return -ENODEV;
> > > > > +
> > > > > + if (!rb || !rb->access->write)
> > > > > +         return -EINVAL;
> > > > > +
> > > > > + if (rb->direction != IIO_BUFFER_DIRECTION_OUT)
> > > > > +         return -EPERM;
> > > > > +
> > > > > + written = 0;
> > > > > + add_wait_queue(&rb->pollq, &wait);
> > > > > + do {
> > > > > +         if (indio_dev->info == NULL)
> > > > > +                 return -ENODEV;
> > > > > +
> > > > > +         if (!iio_buffer_space_available(rb)) {
> > > > > +                 if (signal_pending(current)) {
> > > > > +                         ret = -ERESTARTSYS;
> > > > > +                         break;
> > > > > +                 }
> > > > > +
> > > > > +                 wait_woken(&wait, TASK_INTERRUPTIBLE,
> > > > > +                                 MAX_SCHEDULE_TIMEOUT);
> > > > > +                 continue;
> > > > > +         }
> > > > > +
> > > > > +         ret = rb->access->write(rb, n - written, buf + written);
> > > > > +         if (ret == 0 && (filp->f_flags & O_NONBLOCK))
> > > > > +                 ret = -EAGAIN;
> > > > > +
> > > > > +         if (ret > 0) {
> > > > > +                 written += ret;
> > > > > +                 if (written != n && !(filp->f_flags & O_NONBLOCK))
> > > > > +                         continue;
> > > > > +         }
> > > > > + } while (ret == 0);
> > > > > + remove_wait_queue(&rb->pollq, &wait);
> > > > > +
> > > > > + return ret < 0 ? ret : n;
> > > > > +}
> > > > > +
> > > > >  /**
> > > > >   * iio_buffer_poll() - poll the buffer to find out if it has data
> > > > >   * @filp:        File structure pointer for device access
> > > > > @@ -181,8 +243,18 @@ static __poll_t iio_buffer_poll(struct file *filp,
> > > > >           return 0;
> > > > >
> > > > >   poll_wait(filp, &rb->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))
> > > > > +                 return EPOLLOUT | EPOLLWRNORM;
> > > > > +         break;
> > > > > + }
> > > > > +
> > > > >   return 0;
> > > > >  }
> > > > >
> > > > > @@ -199,6 +271,19 @@ ssize_t iio_buffer_read_wrapper(struct file *filp,  
> > > > char __user *buf,  
> > > > >   return iio_buffer_read(filp, buf, n, f_ps);  }
> > > > >
> > > > > +ssize_t iio_buffer_write_wrapper(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;
> > > > > +
> > > > > + /* check if buffer was opened through new API */
> > > > > + if (test_bit(IIO_BUSY_BIT_POS, &rb->flags))
> > > > > +         return -EBUSY;
> > > > > +
> > > > > + return iio_buffer_write(filp, buf, n, f_ps); }
> > > > > +
> > > > >  __poll_t iio_buffer_poll_wrapper(struct file *filp,
> > > > >                            struct poll_table_struct *wait)
> > > > >  {
> > > > > @@ -231,6 +316,15 @@ void iio_buffer_wakeup_poll(struct iio_dev  
> > > > *indio_dev)  
> > > > >   }
> > > > >  }
> > > > >
> > > > > +int iio_pop_from_buffer(struct iio_buffer *buffer, void *data) {
> > > > > + if (!buffer || !buffer->access || !buffer->access->remove_from)
> > > > > +         return -EINVAL;
> > > > > +
> > > > > + return buffer->access->remove_from(buffer, data); }
> > > > > +EXPORT_SYMBOL_GPL(iio_pop_from_buffer);
> > > > > +
> > > > >  void iio_buffer_init(struct iio_buffer *buffer)  {
> > > > >   INIT_LIST_HEAD(&buffer->demux_list);
> > > > > @@ -1156,6 +1250,10 @@ int iio_update_buffers(struct iio_dev  
> > > > *indio_dev,  
> > > > >   if (insert_buffer == remove_buffer)
> > > > >           return 0;
> > > > >
> > > > > + if (insert_buffer &&
> > > > > +     (insert_buffer->direction == IIO_BUFFER_DIRECTION_OUT))
> > > > > +         return -EINVAL;
> > > > > +
> > > > >   mutex_lock(&iio_dev_opaque->info_exist_lock);
> > > > >   mutex_lock(&indio_dev->mlock);
> > > > >
> > > > > @@ -1277,6 +1375,22 @@ static ssize_t  
> > > > iio_dma_show_data_available(struct device *dev,  
> > > > >   return sysfs_emit(buf, "%zu\n", iio_buffer_data_available(buffer));
> > > > >  }
> > > > >
> > > > > +static ssize_t direction_show(struct device *dev,
> > > > > +                       struct device_attribute *attr,
> > > > > +                       char *buf)
> > > > > +{
> > > > > + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer;
> > > > > +
> > > > > + switch (buffer->direction) {
> > > > > + case IIO_BUFFER_DIRECTION_IN:
> > > > > +         return sprintf(buf, "in\n");
> > > > > + case IIO_BUFFER_DIRECTION_OUT:
> > > > > +         return sprintf(buf, "out\n");
> > > > > + default:
> > > > > +         return -EINVAL;
> > > > > + }
> > > > > +}
> > > > > +
> > > > >  static DEVICE_ATTR(length, S_IRUGO | S_IWUSR, iio_buffer_read_length,
> > > > >              iio_buffer_write_length);
> > > > >  static struct device_attribute dev_attr_length_ro = __ATTR(length, @@
> > > > > -1289,12 +1403,20 @@ static struct device_attribute  
> > > > dev_attr_watermark_ro = __ATTR(watermark,  
> > > > >   S_IRUGO, iio_buffer_show_watermark, NULL);  static
> > > > > DEVICE_ATTR(data_available, S_IRUGO,
> > > > >           iio_dma_show_data_available, NULL);
> > > > > +static DEVICE_ATTR_RO(direction);
> > > > >
> > > > > +/*
> > > > > + * When adding new attributes here, put the at the end, at least
> > > > > +until
> > > > > + * the code that handles the length/length_ro &
> > > > > +watermark/watermark_ro
> > > > > + * assignments gets cleaned up. Otherwise these can create some weird
> > > > > + * duplicate attributes errors under some setups.
> > > > > + */
> > > > >  static struct attribute *iio_buffer_attrs[] = {
> > > > >   &dev_attr_length.attr,
> > > > >   &dev_attr_enable.attr,
> > > > >   &dev_attr_watermark.attr,
> > > > >   &dev_attr_data_available.attr,
> > > > > + &dev_attr_direction.attr,
> > > > >  };
> > > > >
> > > > >  #define to_dev_attr(_attr) container_of(_attr, struct
> > > > > device_attribute, attr) @@ -1397,6 +1519,7 @@ static const struct  
> > > > file_operations iio_buffer_chrdev_fileops = {  
> > > > >   .owner = THIS_MODULE,
> > > > >   .llseek = noop_llseek,
> > > > >   .read = iio_buffer_read,
> > > > > + .write = iio_buffer_write,
> > > > >   .poll = iio_buffer_poll,
> > > > >   .release = iio_buffer_chrdev_release,  }; diff --git
> > > > > a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > > > > index 2dbb37e09b8c..537a08549a69 100644
> > > > > --- a/drivers/iio/industrialio-core.c
> > > > > +++ b/drivers/iio/industrialio-core.c
> > > > > @@ -1822,6 +1822,7 @@ static const struct file_operations  
> > > > iio_buffer_fileops = {  
> > > > >   .owner = THIS_MODULE,
> > > > >   .llseek = noop_llseek,
> > > > >   .read = iio_buffer_read_outer_addr,
> > > > > + .write = iio_buffer_write_outer_addr,
> > > > >   .poll = iio_buffer_poll_addr,
> > > > >   .unlocked_ioctl = iio_ioctl,
> > > > >   .compat_ioctl = compat_ptr_ioctl,
> > > > > diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
> > > > > index b6928ac5c63d..fe2e680d9b5e 100644
> > > > > --- a/include/linux/iio/buffer.h
> > > > > +++ b/include/linux/iio/buffer.h
> > > > > @@ -11,8 +11,15 @@
> > > > >
> > > > >  struct iio_buffer;
> > > > >
> > > > > +enum iio_buffer_direction {
> > > > > + IIO_BUFFER_DIRECTION_IN,
> > > > > + IIO_BUFFER_DIRECTION_OUT,
> > > > > +};
> > > > > +
> > > > >  int iio_push_to_buffers(struct iio_dev *indio_dev, const void *data);
> > > > >
> > > > > +int iio_pop_from_buffer(struct iio_buffer *buffer, void *data);
> > > > > +
> > > > >  /**
> > > > >   * iio_push_to_buffers_with_timestamp() - push data and timestamp to  
> > > > buffers  
> > > > >   * @indio_dev:           iio_dev structure for device.
> > > > > diff --git a/include/linux/iio/buffer_impl.h
> > > > > b/include/linux/iio/buffer_impl.h index 245b32918ae1..e2ca8ea23e19
> > > > > 100644
> > > > > --- a/include/linux/iio/buffer_impl.h
> > > > > +++ b/include/linux/iio/buffer_impl.h
> > > > > @@ -7,6 +7,7 @@
> > > > >  #ifdef CONFIG_IIO_BUFFER
> > > > >
> > > > >  #include <uapi/linux/iio/buffer.h>
> > > > > +#include <linux/iio/buffer.h>
> > > > >
> > > > >  struct iio_dev;
> > > > >  struct iio_buffer;
> > > > > @@ -23,6 +24,10 @@ struct iio_buffer;
> > > > >   * @read:                try to get a specified number of bytes (must exist)
> > > > >   * @data_available:      indicates how much data is available for reading from
> > > > >   *                       the buffer.
> > > > > + * @remove_from: remove scan from buffer. Drivers should calls this to
> > > > > + *                       remove a scan from a buffer.
> > > > > + * @write:               try to write a number of bytes
> > > > > + * @space_available:     returns the amount of bytes available in a  
> > > > buffer  
> > > > >   * @request_update:      if a parameter change has been marked,  
> > > > update underlying  
> > > > >   *                       storage.
> > > > >   * @set_bytes_per_datum:set number of bytes per datum @@ -49,6  
> > > > +54,9  
> > > > > @@ struct iio_buffer_access_funcs {
> > > > >   int (*store_to)(struct iio_buffer *buffer, const void *data);
> > > > >   int (*read)(struct iio_buffer *buffer, size_t n, char __user *buf);
> > > > >   size_t (*data_available)(struct iio_buffer *buffer);
> > > > > + int (*remove_from)(struct iio_buffer *buffer, void *data);
> > > > > + int (*write)(struct iio_buffer *buffer, size_t n, const char __user  
> > > > *buf);  
> > > > > + size_t (*space_available)(struct iio_buffer *buffer);
> > > > >
> > > > >   int (*request_update)(struct iio_buffer *buffer);
> > > > >
> > > > > @@ -80,6 +88,9 @@ struct iio_buffer {
> > > > >   /**  @bytes_per_datum: Size of individual datum including  
> > > > timestamp. */  
> > > > >   size_t bytes_per_datum;
> > > > >
> > > > > + /* @direction: Direction of the data stream (in/out). */
> > > > > + enum iio_buffer_direction direction;
> > > > > +
> > > > >   /**
> > > > >    * @access: Buffer access functions associated with the
> > > > >    * implementation.  
> > >  
> >  

[PATCH v6 2/6] iio: kfifo-buffer: Add output buffer support

[Mihail Chindris]

From: Lars-Peter Clausen <lars@metafoo.de>

Add output buffer support to the kfifo buffer implementation.

The implementation is straight forward and mostly just wraps the kfifo
API to provide the required operations.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
---
 drivers/iio/buffer/kfifo_buf.c | 50 ++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/iio/buffer/kfifo_buf.c b/drivers/iio/buffer/kfifo_buf.c
index 516eb3465de1..7368db2d5c32 100644
--- a/drivers/iio/buffer/kfifo_buf.c
+++ b/drivers/iio/buffer/kfifo_buf.c
@@ -138,10 +138,60 @@ static void iio_kfifo_buffer_release(struct iio_buffer *buffer)
 	kfree(kf);
 }
 
+static size_t iio_kfifo_buf_space_available(struct iio_buffer *r)
+{
+	struct iio_kfifo *kf = iio_to_kfifo(r);
+	size_t avail;
+
+	mutex_lock(&kf->user_lock);
+	avail = kfifo_avail(&kf->kf);
+	mutex_unlock(&kf->user_lock);
+
+	return avail;
+}
+
+static int iio_kfifo_remove_from(struct iio_buffer *r, void *data)
+{
+	int ret;
+	struct iio_kfifo *kf = iio_to_kfifo(r);
+
+	if (kfifo_size(&kf->kf) < 1)
+		return -EBUSY;
+
+	ret = kfifo_out(&kf->kf, data, 1);
+	if (ret != 1)
+		return -EBUSY;
+
+	wake_up_interruptible_poll(&r->pollq, POLLOUT | POLLWRNORM);
+
+	return 0;
+}
+
+static int iio_kfifo_write(struct iio_buffer *r, size_t n,
+	const char __user *buf)
+{
+	struct iio_kfifo *kf = iio_to_kfifo(r);
+	int ret, copied;
+
+	mutex_lock(&kf->user_lock);
+	if (!kfifo_initialized(&kf->kf) || n < kfifo_esize(&kf->kf))
+		ret = -EINVAL;
+	else
+		ret = kfifo_from_user(&kf->kf, buf, n, &copied);
+	mutex_unlock(&kf->user_lock);
+	if (ret)
+		return ret;
+
+	return copied;
+}
+
 static const struct iio_buffer_access_funcs kfifo_access_funcs = {
 	.store_to = &iio_store_to_kfifo,
 	.read = &iio_read_kfifo,
 	.data_available = iio_kfifo_buf_data_available,
+	.remove_from = &iio_kfifo_remove_from,
+	.write = &iio_kfifo_write,
+	.space_available = &iio_kfifo_buf_space_available,
 	.request_update = &iio_request_update_kfifo,
 	.set_bytes_per_datum = &iio_set_bytes_per_datum_kfifo,
 	.set_length = &iio_set_length_kfifo,
-- 
2.27.0

[PATCH v6 3/6] iio: triggered-buffer: extend support to configure output buffers

[Mihail Chindris]

From: Alexandru Ardelean <alexandru.ardelean@analog.com>

Now that output (kfifo) buffers are supported, we need to extend the
{devm_}iio_triggered_buffer_setup_ext() parameter list to take a direction
parameter.

This allows us to attach an output triggered buffer to a DAC device.
Unfortunately it's a bit difficult to add another macro to avoid changing 5
drivers where {devm_}iio_triggered_buffer_setup_ext() is used.
Well, it's doable, but may not be worth the trouble vs just updating all
these 5 drivers.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
---
 drivers/iio/accel/adxl372.c                           |  1 +
 drivers/iio/accel/bmc150-accel-core.c                 |  1 +
 drivers/iio/adc/at91-sama5d2_adc.c                    |  4 ++--
 drivers/iio/buffer/industrialio-triggered-buffer.c    |  8 ++++++--
 .../iio/common/cros_ec_sensors/cros_ec_sensors_core.c |  5 +++--
 drivers/iio/common/hid-sensors/hid-sensor-trigger.c   |  5 +++--
 include/linux/iio/triggered_buffer.h                  | 11 +++++++++--
 7 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index fc9592407717..758952584f8c 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -1214,6 +1214,7 @@ int adxl372_probe(struct device *dev, struct regmap *regmap,
 	ret = devm_iio_triggered_buffer_setup_ext(dev,
 						  indio_dev, NULL,
 						  adxl372_trigger_handler,
+						  IIO_BUFFER_DIRECTION_IN,
 						  &adxl372_buffer_ops,
 						  adxl372_fifo_attributes);
 	if (ret < 0)
diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index e8693a42ad46..63216321cdb5 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -1734,6 +1734,7 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
 	ret = iio_triggered_buffer_setup_ext(indio_dev,
 					     &iio_pollfunc_store_time,
 					     bmc150_accel_trigger_handler,
+					     IIO_BUFFER_DIRECTION_IN,
 					     &bmc150_accel_buffer_ops,
 					     fifo_attrs);
 	if (ret < 0) {
diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index ea5ca163d879..7093611e321e 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -1681,8 +1681,8 @@ static int at91_adc_buffer_and_trigger_init(struct device *dev,
 		fifo_attrs = NULL;
 
 	ret = devm_iio_triggered_buffer_setup_ext(&indio->dev, indio,
-		&iio_pollfunc_store_time,
-		&at91_adc_trigger_handler, &at91_buffer_setup_ops, fifo_attrs);
+		&iio_pollfunc_store_time, &at91_adc_trigger_handler,
+		IIO_BUFFER_DIRECTION_IN, &at91_buffer_setup_ops, fifo_attrs);
 	if (ret < 0) {
 		dev_err(dev, "couldn't initialize the buffer.\n");
 		return ret;
diff --git a/drivers/iio/buffer/industrialio-triggered-buffer.c b/drivers/iio/buffer/industrialio-triggered-buffer.c
index f77c4538141e..8d4fc97d1005 100644
--- a/drivers/iio/buffer/industrialio-triggered-buffer.c
+++ b/drivers/iio/buffer/industrialio-triggered-buffer.c
@@ -19,6 +19,7 @@
  * @indio_dev:		IIO device structure
  * @h:			Function which will be used as pollfunc top half
  * @thread:		Function which will be used as pollfunc bottom half
+ * @direction:		Direction of the data stream (in/out).
  * @setup_ops:		Buffer setup functions to use for this device.
  *			If NULL the default setup functions for triggered
  *			buffers will be used.
@@ -38,6 +39,7 @@
 int iio_triggered_buffer_setup_ext(struct iio_dev *indio_dev,
 	irqreturn_t (*h)(int irq, void *p),
 	irqreturn_t (*thread)(int irq, void *p),
+	enum iio_buffer_direction direction,
 	const struct iio_buffer_setup_ops *setup_ops,
 	const struct attribute **buffer_attrs)
 {
@@ -68,6 +70,7 @@ int iio_triggered_buffer_setup_ext(struct iio_dev *indio_dev,
 	/* Flag that polled ring buffering is possible */
 	indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
 
+	buffer->direction = direction;
 	buffer->attrs = buffer_attrs;
 
 	ret = iio_device_attach_buffer(indio_dev, buffer);
@@ -105,13 +108,14 @@ int devm_iio_triggered_buffer_setup_ext(struct device *dev,
 					struct iio_dev *indio_dev,
 					irqreturn_t (*h)(int irq, void *p),
 					irqreturn_t (*thread)(int irq, void *p),
+					enum iio_buffer_direction direction,
 					const struct iio_buffer_setup_ops *ops,
 					const struct attribute **buffer_attrs)
 {
 	int ret;
 
-	ret = iio_triggered_buffer_setup_ext(indio_dev, h, thread, ops,
-					     buffer_attrs);
+	ret = iio_triggered_buffer_setup_ext(indio_dev, h, thread, direction,
+					     ops, buffer_attrs);
 	if (ret)
 		return ret;
 
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index 28bde13003b7..e9f64da06f89 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -360,8 +360,9 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
 			 * The only way to get samples in buffer is to set a
 			 * software trigger (systrig, hrtimer).
 			 */
-			ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
-					NULL, trigger_capture, NULL);
+			ret = devm_iio_triggered_buffer_setup_ext(dev,
+					indio_dev, NULL, trigger_capture,
+					IIO_BUFFER_DIRECTION_IN, NULL);
 			if (ret)
 				return ret;
 		}
diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
index a4ec11a3b68a..1151434038d4 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -241,8 +241,9 @@ int hid_sensor_setup_trigger(struct iio_dev *indio_dev, const char *name,
 		fifo_attrs = NULL;
 
 	ret = iio_triggered_buffer_setup_ext(indio_dev,
-					     &iio_pollfunc_store_time,
-					     NULL, NULL, fifo_attrs);
+					     &iio_pollfunc_store_time, NULL,
+					     IIO_BUFFER_DIRECTION_IN,
+					     NULL, fifo_attrs);
 	if (ret) {
 		dev_err(&indio_dev->dev, "Triggered Buffer Setup Failed\n");
 		return ret;
diff --git a/include/linux/iio/triggered_buffer.h b/include/linux/iio/triggered_buffer.h
index 7f154d1f8739..7490b05fc5b2 100644
--- a/include/linux/iio/triggered_buffer.h
+++ b/include/linux/iio/triggered_buffer.h
@@ -2,6 +2,7 @@
 #ifndef _LINUX_IIO_TRIGGERED_BUFFER_H_
 #define _LINUX_IIO_TRIGGERED_BUFFER_H_
 
+#include <linux/iio/buffer.h>
 #include <linux/interrupt.h>
 
 struct attribute;
@@ -11,21 +12,27 @@ struct iio_buffer_setup_ops;
 int iio_triggered_buffer_setup_ext(struct iio_dev *indio_dev,
 	irqreturn_t (*h)(int irq, void *p),
 	irqreturn_t (*thread)(int irq, void *p),
+	enum iio_buffer_direction direction,
 	const struct iio_buffer_setup_ops *setup_ops,
 	const struct attribute **buffer_attrs);
 void iio_triggered_buffer_cleanup(struct iio_dev *indio_dev);
 
 #define iio_triggered_buffer_setup(indio_dev, h, thread, setup_ops)		\
-	iio_triggered_buffer_setup_ext((indio_dev), (h), (thread), (setup_ops), NULL)
+	iio_triggered_buffer_setup_ext((indio_dev), (h), (thread),		\
+					IIO_BUFFER_DIRECTION_IN, (setup_ops),	\
+					NULL)
 
 int devm_iio_triggered_buffer_setup_ext(struct device *dev,
 					struct iio_dev *indio_dev,
 					irqreturn_t (*h)(int irq, void *p),
 					irqreturn_t (*thread)(int irq, void *p),
+					enum iio_buffer_direction direction,
 					const struct iio_buffer_setup_ops *ops,
 					const struct attribute **buffer_attrs);
 
 #define devm_iio_triggered_buffer_setup(dev, indio_dev, h, thread, setup_ops)	\
-	devm_iio_triggered_buffer_setup_ext((dev), (indio_dev), (h), (thread), (setup_ops), NULL)
+	devm_iio_triggered_buffer_setup_ext((dev), (indio_dev), (h), (thread),	\
+					    IIO_BUFFER_DIRECTION_IN,		\
+					    (setup_ops), NULL)
 
 #endif
-- 
2.27.0

Re: [PATCH v6 3/6] iio: triggered-buffer: extend support to configure output buffers

[Jonathan Cameron]

On Thu, 7 Oct 2021 08:00:32 +0000
Mihail Chindris <mihail.chindris@analog.com> wrote:

> From: Alexandru Ardelean <alexandru.ardelean@analog.com>
> 
> Now that output (kfifo) buffers are supported, we need to extend the
> {devm_}iio_triggered_buffer_setup_ext() parameter list to take a direction
> parameter.
> 
> This allows us to attach an output triggered buffer to a DAC device.
> Unfortunately it's a bit difficult to add another macro to avoid changing 5
> drivers where {devm_}iio_triggered_buffer_setup_ext() is used.
> Well, it's doable, but may not be worth the trouble vs just updating all
> these 5 drivers.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
This doesn't build... See inline and fixed whilst applying.
 
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index 28bde13003b7..e9f64da06f89 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -360,8 +360,9 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
>  			 * The only way to get samples in buffer is to set a
>  			 * software trigger (systrig, hrtimer).
>  			 */
> -			ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> -					NULL, trigger_capture, NULL);

This isn't the _ext form...
so dropped this change.
> +			ret = devm_iio_triggered_buffer_setup_ext(dev,
> +					indio_dev, NULL, trigger_capture,
> +					IIO_BUFFER_DIRECTION_IN, NULL);
>  			if (ret)
>  				return ret;
>  		}

[PATCH v6 4/6] drivers: iio: dac: ad5766: Fix dt property name

[Mihail Chindris]

In the documentation the name for the property is
output-range-microvolts which is a standard name, therefore this name
must be used.

Fixes: fd9373e41b9ba ("iio: dac: ad5766: add driver support for AD5766")
Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
---
 drivers/iio/dac/ad5766.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/dac/ad5766.c b/drivers/iio/dac/ad5766.c
index 3104ec32dfac..dafda84fdea3 100644
--- a/drivers/iio/dac/ad5766.c
+++ b/drivers/iio/dac/ad5766.c
@@ -503,13 +503,13 @@ static int ad5766_get_output_range(struct ad5766_state *st)
 	int i, ret, min, max, tmp[2];
 
 	ret = device_property_read_u32_array(&st->spi->dev,
-					     "output-range-voltage",
+					     "output-range-microvolts",
 					     tmp, 2);
 	if (ret)
 		return ret;
 
-	min = tmp[0] / 1000;
-	max = tmp[1] / 1000;
+	min = tmp[0] / 1000000;
+	max = tmp[1] / 1000000;
 	for (i = 0; i < ARRAY_SIZE(ad5766_span_tbl); i++) {
 		if (ad5766_span_tbl[i].min != min ||
 		    ad5766_span_tbl[i].max != max)
-- 
2.27.0

Re: [PATCH v6 4/6] drivers: iio: dac: ad5766: Fix dt property name

[Jonathan Cameron]

On Thu, 7 Oct 2021 08:00:34 +0000
Mihail Chindris <mihail.chindris@analog.com> wrote:

> In the documentation the name for the property is
> output-range-microvolts which is a standard name, therefore this name
> must be used.
> 
> Fixes: fd9373e41b9ba ("iio: dac: ad5766: add driver support for AD5766")
> Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
> Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Applied this one to the fixes-togreg branch of iio.git and marked it for stable.

Thankfully this is (I think) far enough from the other changes that we
should be able to take the other patches through the togreg tree and have
them all meet up in linux-next / upstream.

Thanks,

Jonathan

> ---
>  drivers/iio/dac/ad5766.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad5766.c b/drivers/iio/dac/ad5766.c
> index 3104ec32dfac..dafda84fdea3 100644
> --- a/drivers/iio/dac/ad5766.c
> +++ b/drivers/iio/dac/ad5766.c
> @@ -503,13 +503,13 @@ static int ad5766_get_output_range(struct ad5766_state *st)
>  	int i, ret, min, max, tmp[2];
>  
>  	ret = device_property_read_u32_array(&st->spi->dev,
> -					     "output-range-voltage",
> +					     "output-range-microvolts",
>  					     tmp, 2);
>  	if (ret)
>  		return ret;
>  
> -	min = tmp[0] / 1000;
> -	max = tmp[1] / 1000;
> +	min = tmp[0] / 1000000;
> +	max = tmp[1] / 1000000;
>  	for (i = 0; i < ARRAY_SIZE(ad5766_span_tbl); i++) {
>  		if (ad5766_span_tbl[i].min != min ||
>  		    ad5766_span_tbl[i].max != max)

[PATCH v6 5/6] Documentation:devicetree:bindings:iio:dac: Fix val

[Mihail Chindris]

A correct value for output-range-microvolts is -5 to 5 Volts
not -5 to 5 milivolts

Fixes: e904cc899293f ("dt-bindings: iio: dac: AD5766 yaml documentation")
Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
---
 Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
index d5c54813ce87..a8f7720d1e3e 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
@@ -54,7 +54,7 @@ examples:
 
           ad5766@0 {
               compatible = "adi,ad5766";
-              output-range-microvolts = <(-5000) 5000>;
+              output-range-microvolts = <(-5000000) 5000000>;
               reg = <0>;
               spi-cpol;
               spi-max-frequency = <1000000>;
-- 
2.27.0

Re: [PATCH v6 5/6] Documentation:devicetree:bindings:iio:dac: Fix val

[Jonathan Cameron]

On Thu, 7 Oct 2021 08:00:36 +0000
Mihail Chindris <mihail.chindris@analog.com> wrote:

> A correct value for output-range-microvolts is -5 to 5 Volts
> not -5 to 5 milivolts
> 
> Fixes: e904cc899293f ("dt-bindings: iio: dac: AD5766 yaml documentation")
> Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
> Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Whilst it's only a trivial fix to an example it seems to me a worthwhile
thing to backport and avoid people using old trees seeing the wrong example

Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks,

Jonathan

> ---
>  Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
> index d5c54813ce87..a8f7720d1e3e 100644
> --- a/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
> @@ -54,7 +54,7 @@ examples:
>  
>            ad5766@0 {
>                compatible = "adi,ad5766";
> -              output-range-microvolts = <(-5000) 5000>;
> +              output-range-microvolts = <(-5000000) 5000000>;
>                reg = <0>;
>                spi-cpol;
>                spi-max-frequency = <1000000>;

[PATCH v6 6/6] drivers:iio:dac:ad5766.c: Add trigger buffer

[Mihail Chindris]

This chip is able to generate waveform and using an
with the output trigger buffer will be easy to generate one.

Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
---
 drivers/iio/dac/ad5766.c | 42 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/iio/dac/ad5766.c b/drivers/iio/dac/ad5766.c
index dafda84fdea3..b0d220c3a126 100644
--- a/drivers/iio/dac/ad5766.c
+++ b/drivers/iio/dac/ad5766.c
@@ -5,10 +5,13 @@
  * Copyright 2019-2020 Analog Devices Inc.
  */
 #include <linux/bitfield.h>
+#include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/gpio/consumer.h>
 #include <linux/iio/iio.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
 #include <linux/module.h>
 #include <linux/spi/spi.h>
 #include <asm/unaligned.h>
@@ -455,6 +458,7 @@ static const struct iio_chan_spec_ext_info ad5766_ext_info[] = {
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
 	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |		\
 		BIT(IIO_CHAN_INFO_SCALE),				\
+	.scan_index = (_chan),						\
 	.scan_type = {							\
 		.sign = 'u',						\
 		.realbits = (_bits),					\
@@ -576,6 +580,35 @@ static int ad5766_default_setup(struct ad5766_state *st)
 	return  __ad5766_spi_write(st, AD5766_CMD_SPAN_REG, st->crt_range);
 }
 
+static irqreturn_t ad5766_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct iio_buffer *buffer = indio_dev->buffer;
+	struct ad5766_state *st = iio_priv(indio_dev);
+	int ret, ch, i;
+	u16 data[ARRAY_SIZE(ad5766_channels)];
+
+	ret = iio_pop_from_buffer(buffer, data);
+	if (ret)
+		goto done;
+
+	i = 0;
+	mutex_lock(&st->lock);
+	for_each_set_bit(ch, indio_dev->active_scan_mask,
+			 st->chip_info->num_channels - 1)
+		__ad5766_spi_write(st, AD5766_CMD_WR_IN_REG(ch), data[i++]);
+
+	__ad5766_spi_write(st, AD5766_CMD_SW_LDAC,
+			   *indio_dev->active_scan_mask);
+	mutex_unlock(&st->lock);
+
+done:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
 static int ad5766_probe(struct spi_device *spi)
 {
 	enum ad5766_type type;
@@ -609,6 +642,15 @@ static int ad5766_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
+	/* Configure trigger buffer */
+	ret = devm_iio_triggered_buffer_setup_ext(&spi->dev, indio_dev, NULL,
+						  ad5766_trigger_handler,
+						  IIO_BUFFER_DIRECTION_OUT,
+						  NULL,
+						  NULL);
+	if (ret)
+		return ret;
+
 	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
-- 
2.27.0

Re: [PATCH v6 6/6] drivers:iio:dac:ad5766.c: Add trigger buffer

[Jonathan Cameron]

On Thu, 7 Oct 2021 08:00:37 +0000
Mihail Chindris <mihail.chindris@analog.com> wrote:

> This chip is able to generate waveform and using an
> with the output trigger buffer will be easy to generate one.
> 
> Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
> Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>

Rest of series applied to the togreg branch of iio.git, initially pushed out as
testing for 0-day to poke at it.

Thanks,

Jonathan

> ---
>  drivers/iio/dac/ad5766.c | 42 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/iio/dac/ad5766.c b/drivers/iio/dac/ad5766.c
> index dafda84fdea3..b0d220c3a126 100644
> --- a/drivers/iio/dac/ad5766.c
> +++ b/drivers/iio/dac/ad5766.c
> @@ -5,10 +5,13 @@
>   * Copyright 2019-2020 Analog Devices Inc.
>   */
>  #include <linux/bitfield.h>
> +#include <linux/bitops.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/iio/iio.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
>  #include <linux/module.h>
>  #include <linux/spi/spi.h>
>  #include <asm/unaligned.h>
> @@ -455,6 +458,7 @@ static const struct iio_chan_spec_ext_info ad5766_ext_info[] = {
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
>  	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |		\
>  		BIT(IIO_CHAN_INFO_SCALE),				\
> +	.scan_index = (_chan),						\
>  	.scan_type = {							\
>  		.sign = 'u',						\
>  		.realbits = (_bits),					\
> @@ -576,6 +580,35 @@ static int ad5766_default_setup(struct ad5766_state *st)
>  	return  __ad5766_spi_write(st, AD5766_CMD_SPAN_REG, st->crt_range);
>  }
>  
> +static irqreturn_t ad5766_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct iio_buffer *buffer = indio_dev->buffer;
> +	struct ad5766_state *st = iio_priv(indio_dev);
> +	int ret, ch, i;
> +	u16 data[ARRAY_SIZE(ad5766_channels)];
> +
> +	ret = iio_pop_from_buffer(buffer, data);
> +	if (ret)
> +		goto done;
> +
> +	i = 0;
> +	mutex_lock(&st->lock);
> +	for_each_set_bit(ch, indio_dev->active_scan_mask,
> +			 st->chip_info->num_channels - 1)
> +		__ad5766_spi_write(st, AD5766_CMD_WR_IN_REG(ch), data[i++]);
> +
> +	__ad5766_spi_write(st, AD5766_CMD_SW_LDAC,
> +			   *indio_dev->active_scan_mask);
> +	mutex_unlock(&st->lock);
> +
> +done:
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static int ad5766_probe(struct spi_device *spi)
>  {
>  	enum ad5766_type type;
> @@ -609,6 +642,15 @@ static int ad5766_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> +	/* Configure trigger buffer */
> +	ret = devm_iio_triggered_buffer_setup_ext(&spi->dev, indio_dev, NULL,
> +						  ad5766_trigger_handler,
> +						  IIO_BUFFER_DIRECTION_OUT,
> +						  NULL,
> +						  NULL);
> +	if (ret)
> +		return ret;
> +
>  	return devm_iio_device_register(&spi->dev, indio_dev);
>  }
>  

RE: [PATCH v6 0/6] iio: Add output buffer support

[Chindris, Mihail]

> -----Original Message-----
> From: Mihail Chindris <mihail.chindris@analog.com>
> Sent: Thursday, 7 October 2021 11:00
> To: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org
> Cc: lars@metafoo.de; Hennerich, Michael
> <Michael.Hennerich@analog.com>; jic23@kernel.org; Sa, Nuno
> <Nuno.Sa@analog.com>; Bogdan, Dragos <Dragos.Bogdan@analog.com>;
> alexandru.ardelean@analog.com; Chindris, Mihail
> <Mihail.Chindris@analog.com>
> Subject: [PATCH v6 0/6] iio: Add output buffer support
> 
> Changelog v5 -> v6:
>   * https://lore.kernel.org/all/20210916182914.1810-1-
> mihail.chindris@analog.com
>   * Fix check in  iio_update_buffers adding test on insert_buffer
>   * Return if check in iio_update_buffers fails.
>   * Fix length typo
>   * Add rb->direction check in both read and write functions.
>   * Add fixes tags
>   * Add  Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com> line
>   * Remove unnecessary (u8 *) cast
>   * Remove unnecessary AD5766_NUM_CH define
* Write to input register in ad5766_trigger_handler for simultaneous update
> 
> Changelog v4 -> v5:
>   * https://lore.kernel.org/all/20210820165927.4524-1-
> mihail.chindris@analog.com
>   * Remove ad3552r example from series and replace with the update of an
>     existing driver: ad5662. Will add ad3552r in other another series.
>   * Make normal comment from kernel-doc comment. (A bot was
> complaining about that)
>   * Add indio_dev->info check
>   * Rename iio_buffer_remove_sample -> iio_pop_from_buffer
>   * Fix comment of remove_from: sample -> scan
>   * Change iio_pop_from_buffer data type to void * to be consistent with
>     iio_push_to_buffers
>   * Remove use watermark, in our kernel is not used and I can't think of an
>     usecase for it.
>   * Reimplement write to increment buffer index and handle blocking and
>     noblocking calls
>   * Move `if (insert_buffer->direction == IIO_BUFFER_DIRECTION_OUT)`
> outside lock
>   * Remove redundant checks of `if (insert_buffer->direction ==
>     IIO_BUFFER_DIRECTION_OUT)`
> 
> Alexandru Ardelean (1):
>   iio: triggered-buffer: extend support to configure output buffers
> 
> Lars-Peter Clausen (1):
>   iio: kfifo-buffer: Add output buffer support
> 
> Mihail Chindris (4):
>   iio: Add output buffer support
>   drivers: iio: dac: ad5766: Fix dt property name
>   Documentation:devicetree:bindings:iio:dac: Fix val
>   drivers:iio:dac:ad5766.c: Add trigger buffer
> 
>  .../bindings/iio/dac/adi,ad5766.yaml          |   2 +-
>  drivers/iio/accel/adxl372.c                   |   1 +
>  drivers/iio/accel/bmc150-accel-core.c         |   1 +
>  drivers/iio/adc/at91-sama5d2_adc.c            |   4 +-
>  .../buffer/industrialio-triggered-buffer.c    |   8 +-
>  drivers/iio/buffer/kfifo_buf.c                |  50 +++++++
>  .../cros_ec_sensors/cros_ec_sensors_core.c    |   5 +-
>  .../common/hid-sensors/hid-sensor-trigger.c   |   5 +-
>  drivers/iio/dac/ad5766.c                      |  48 ++++++-
>  drivers/iio/iio_core.h                        |   4 +
>  drivers/iio/industrialio-buffer.c             | 127 +++++++++++++++++-
>  drivers/iio/industrialio-core.c               |   1 +
>  include/linux/iio/buffer.h                    |   7 +
>  include/linux/iio/buffer_impl.h               |  11 ++
>  include/linux/iio/triggered_buffer.h          |  11 +-
>  15 files changed, 269 insertions(+), 16 deletions(-)
> 
> 
> base-commit: 94a853eca720ac9e385e59f27e859b4a01123f58
> --
> 2.27.0