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

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

[Alexandru Ardelean]

Changelog v2 -> v3:
* https://lore.kernel.org/linux-iio/20210217083438.37865-4-alexandru.ardelean@analog.com/T/#m396545e0c6cc9d58e17f4d79b6fc707fd0373d89
* adding only infrastructure pieces for output DAC buffers, unfortunately I
  couldn't finish a complete DAC change to showcase these changes
* patch 'iio: Add output buffer support'
   - moved new 'bufferY/direction' attribute at the end and added
     comment about what it should be added at the end
* removed Lars' comment '/* need a way of knowing if there may be enough data... */'
* updated some various formatting;

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

Lars-Peter Clausen (5):
  iio: Add output buffer support
  iio: kfifo-buffer: Add output buffer support
  iio: buffer-dma: Allow to provide custom buffer ops
  iio: buffer-dma: Add output buffer support
  iio: buffer-dma: add support for cyclic DMA transfers

 Documentation/ABI/testing/sysfs-bus-iio       |   7 +
 drivers/iio/accel/adxl372.c                   |   1 +
 drivers/iio/accel/bmc150-accel-core.c         |   1 +
 drivers/iio/adc/adi-axi-adc.c                 |   4 +-
 drivers/iio/adc/at91-sama5d2_adc.c            |   4 +-
 drivers/iio/buffer/industrialio-buffer-dma.c  | 120 ++++++++++++++--
 .../buffer/industrialio-buffer-dmaengine.c    |  72 +++++++---
 .../buffer/industrialio-triggered-buffer.c    |   8 +-
 drivers/iio/buffer/kfifo_buf.c                |  50 +++++++
 .../cros_ec_sensors/cros_ec_sensors_core.c    |   1 +
 .../common/hid-sensors/hid-sensor-trigger.c   |   5 +-
 drivers/iio/industrialio-buffer.c             | 133 +++++++++++++++++-
 include/linux/iio/buffer-dma.h                |  11 +-
 include/linux/iio/buffer-dmaengine.h          |   8 +-
 include/linux/iio/buffer.h                    |   7 +
 include/linux/iio/buffer_impl.h               |  11 ++
 include/linux/iio/triggered_buffer.h          |  11 +-
 include/uapi/linux/iio/buffer.h               |   1 +
 18 files changed, 412 insertions(+), 43 deletions(-)

-- 
2.27.0

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

[Alexandru Ardelean]

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

Currently IIO only supports buffer mode for capture devices like ADCs. Add
support for buffered mode for output devices like DACs.

The output buffer implementation is analogous to the input buffer
implementation. Instead of using read() to get data from the buffer write()
is used to copy data into the buffer.

poll() with POLLOUT will wakeup if there is space available for more or
equal to the configured watermark of samples.

Drivers can remove data from a buffer using iio_buffer_remove_sample(), the
function can e.g. called from a trigger handler to write the data to
hardware.

A buffer can only be either a output buffer or an input, but not both. So,
for a device that has an ADC and DAC path, this will mean 2 IIO buffers
(one for each direction).

The direction of the buffer is decided by the new direction field of the
iio_buffer struct and should be set after allocating and before registering
it.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 Documentation/ABI/testing/sysfs-bus-iio |   7 ++
 drivers/iio/industrialio-buffer.c       | 133 +++++++++++++++++++++++-
 include/linux/iio/buffer.h              |   7 ++
 include/linux/iio/buffer_impl.h         |  11 ++
 4 files changed, 154 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 51feac826cb5..7a87bfb16b22 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1170,6 +1170,13 @@ Contact:	linux-iio@vger.kernel.org
 Description:
 		Number of scans contained by the buffer.
 
+What:		/sys/bus/iio/devices/iio:deviceX/bufferY/direction
+KernelVersion:	5.11
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Returns the direction of the data stream of the buffer.
+		The output is "in" or "out".
+
 What:		/sys/bus/iio/devices/iio:deviceX/buffer/enable
 KernelVersion:	2.6.35
 What:		/sys/bus/iio/devices/iio:deviceX/bufferY/enable
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 5d641f8adfbd..db1b91350987 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -162,6 +162,69 @@ static ssize_t iio_buffer_read(struct file *filp, char __user *buf,
 	return ret;
 }
 
+static size_t iio_buffer_space_available(struct iio_buffer *buf)
+{
+	if (buf->access->space_available)
+		return buf->access->space_available(buf);
+
+	return SIZE_MAX;
+}
+
+static ssize_t iio_buffer_write(struct file *filp, const char __user *buf,
+				size_t n, loff_t *f_ps)
+{
+	struct iio_dev_buffer_pair *ib = filp->private_data;
+	struct iio_buffer *rb = ib->buffer;
+	struct iio_dev *indio_dev = ib->indio_dev;
+	DEFINE_WAIT_FUNC(wait, woken_wake_function);
+	size_t datum_size;
+	size_t to_wait;
+	int ret;
+
+	if (!rb || !rb->access->write)
+		return -EINVAL;
+
+	datum_size = rb->bytes_per_datum;
+
+	/*
+	 * If datum_size is 0 there will never be anything to read from the
+	 * buffer, so signal end of file now.
+	 */
+	if (!datum_size)
+		return 0;
+
+	if (filp->f_flags & O_NONBLOCK)
+		to_wait = 0;
+	else
+		to_wait = min_t(size_t, n / datum_size, rb->watermark);
+
+	add_wait_queue(&rb->pollq, &wait);
+	do {
+		if (!indio_dev->info) {
+			ret = -ENODEV;
+			break;
+		}
+
+		if (iio_buffer_space_available(rb) < to_wait) {
+			if (signal_pending(current)) {
+				ret = -ERESTARTSYS;
+				break;
+			}
+
+			wait_woken(&wait, TASK_INTERRUPTIBLE,
+				   MAX_SCHEDULE_TIMEOUT);
+			continue;
+		}
+
+		ret = rb->access->write(rb, n, buf);
+		if (ret == 0 && (filp->f_flags & O_NONBLOCK))
+			ret = -EAGAIN;
+	} while (ret == 0);
+	remove_wait_queue(&rb->pollq, &wait);
+
+	return ret;
+}
+
 /**
  * iio_buffer_poll() - poll the buffer to find out if it has data
  * @filp:	File structure pointer for device access
@@ -182,8 +245,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) >= rb->watermark)
+			return EPOLLOUT | EPOLLWRNORM;
+		break;
+	}
+
 	return 0;
 }
 
@@ -232,6 +305,16 @@ void iio_buffer_wakeup_poll(struct iio_dev *indio_dev)
 	}
 }
 
+int iio_buffer_remove_sample(struct iio_buffer *buffer, u8 *data)
+{
+	if (!buffer || !buffer->access)
+		return -EINVAL;
+	if (!buffer->access->write)
+		return -ENOSYS;
+	return buffer->access->remove_from(buffer, data);
+}
+EXPORT_SYMBOL_GPL(iio_buffer_remove_sample);
+
 void iio_buffer_init(struct iio_buffer *buffer)
 {
 	INIT_LIST_HEAD(&buffer->demux_list);
@@ -803,6 +886,8 @@ static int iio_verify_update(struct iio_dev *indio_dev,
 	}
 
 	if (insert_buffer) {
+		if (insert_buffer->direction == IIO_BUFFER_DIRECTION_OUT)
+			strict_scanmask = true;
 		bitmap_or(compound_mask, compound_mask,
 			  insert_buffer->scan_mask, indio_dev->masklength);
 		scan_timestamp |= insert_buffer->scan_timestamp;
@@ -945,6 +1030,8 @@ static int iio_update_demux(struct iio_dev *indio_dev)
 	int ret;
 
 	list_for_each_entry(buffer, &iio_dev_opaque->buffer_list, buffer_list) {
+		if (buffer->direction == IIO_BUFFER_DIRECTION_OUT)
+			continue;
 		ret = iio_buffer_update_demux(indio_dev, buffer);
 		if (ret < 0)
 			goto error_clear_mux_table;
@@ -1155,6 +1242,11 @@ int iio_update_buffers(struct iio_dev *indio_dev,
 	mutex_lock(&indio_dev->info_exist_lock);
 	mutex_lock(&indio_dev->mlock);
 
+	if (insert_buffer->direction == IIO_BUFFER_DIRECTION_OUT) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
 	if (insert_buffer && iio_buffer_is_active(insert_buffer))
 		insert_buffer = NULL;
 
@@ -1273,6 +1365,22 @@ static ssize_t iio_dma_show_data_available(struct device *dev,
 	return sprintf(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,
@@ -1285,12 +1393,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 lengh/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)
@@ -1401,6 +1517,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,
 	.unlocked_ioctl = iio_buffer_ioctl,
 	.compat_ioctl = compat_ptr_ioctl,
@@ -1920,8 +2037,16 @@ static int iio_buffer_mmap(struct file *filep, struct vm_area_struct *vma)
 	if (!(vma->vm_flags & VM_SHARED))
 		return -EINVAL;
 
-	if (!(vma->vm_flags & VM_READ))
-		return -EINVAL;
+	switch (buffer->direction) {
+	case IIO_BUFFER_DIRECTION_IN:
+		if (!(vma->vm_flags & VM_READ))
+			return -EINVAL;
+		break;
+	case IIO_BUFFER_DIRECTION_OUT:
+		if (!(vma->vm_flags & VM_WRITE))
+			return -EINVAL;
+		break;
+	}
 
 	return buffer->access->mmap(buffer, vma);
 }
diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
index b6928ac5c63d..e87b8773253d 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_buffer_remove_sample(struct iio_buffer *buffer, u8 *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 1d57dc7ccb4f..47bdbf4a4519 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 sample from buffer. Drivers should calls this to
+ *			remove a sample 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
@@ -61,6 +66,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);
 
@@ -103,6 +111,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

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

[Alexandru Ardelean]

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>
---
 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 34289ce12f20..e8a434f84778 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) < r->bytes_per_datum)
+		return -EBUSY;
+
+	ret = kfifo_out(&kf->kf, data, r->bytes_per_datum);
+	if (ret != r->bytes_per_datum)
+		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 v3 3/6] iio: triggered-buffer: extend support to configure output buffers

[Alexandru Ardelean]

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>
---
 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 |  1 +
 drivers/iio/common/hid-sensors/hid-sensor-trigger.c   |  5 +++--
 include/linux/iio/triggered_buffer.h                  | 11 +++++++++--
 7 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index 8ba1453b8dbf..f2e077f72531 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 b0dbd12cbf42..3e0305b0065b 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -1743,6 +1743,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 a7826f097b95..fc134f9c0200 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -1680,8 +1680,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 b2b1b7d27af4..f400e978cd1e 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,6 +108,7 @@ 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)
 {
@@ -118,8 +122,8 @@ int devm_iio_triggered_buffer_setup_ext(struct device *dev,
 
 	*ptr = indio_dev;
 
-	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)
 		devres_add(dev, ptr);
 	else
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 f8824afe595e..afe8917f8c49 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
@@ -369,6 +369,7 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
 			 */
 			ret = devm_iio_triggered_buffer_setup_ext(
 					dev, indio_dev, NULL, trigger_capture,
+					IIO_BUFFER_DIRECTION_IN,
 					NULL, fifo_attrs);
 			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 064c32bec9c7..dfaadbcac08a 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -248,8 +248,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

[PATCH v3 4/6] iio: buffer-dma: Allow to provide custom buffer ops

[Alexandru Ardelean]

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

Some devices that want to make use of the DMA buffer might need to do
something special, like write a register when the buffer is enabled.

Extend the API to allow those drivers to provide their own buffer ops.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/adc/adi-axi-adc.c                 |  3 ++-
 drivers/iio/buffer/industrialio-buffer-dma.c  |  4 +++-
 .../buffer/industrialio-buffer-dmaengine.c    | 23 +++++++++++++------
 include/linux/iio/buffer-dma.h                |  5 +++-
 include/linux/iio/buffer-dmaengine.h          |  6 +++--
 5 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 2e84623f732e..bfa00100a631 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -113,7 +113,8 @@ static int adi_axi_adc_config_dma_buffer(struct device *dev,
 		dma_name = "rx";
 
 	return devm_iio_dmaengine_buffer_setup(indio_dev->dev.parent,
-					       indio_dev, dma_name);
+					       indio_dev, dma_name,
+					       NULL, NULL);
 }
 
 static int adi_axi_adc_read_raw(struct iio_dev *indio_dev,
diff --git a/drivers/iio/buffer/industrialio-buffer-dma.c b/drivers/iio/buffer/industrialio-buffer-dma.c
index 1ae47ed8ef22..aa56c10418d0 100644
--- a/drivers/iio/buffer/industrialio-buffer-dma.c
+++ b/drivers/iio/buffer/industrialio-buffer-dma.c
@@ -892,13 +892,15 @@ EXPORT_SYMBOL_GPL(iio_dma_buffer_set_length);
  * allocations are done from a memory region that can be accessed by the device.
  */
 int iio_dma_buffer_init(struct iio_dma_buffer_queue *queue,
-	struct device *dev, const struct iio_dma_buffer_ops *ops)
+	struct device *dev, const struct iio_dma_buffer_ops *ops,
+	void *driver_data)
 {
 	iio_buffer_init(&queue->buffer);
 	queue->buffer.length = PAGE_SIZE;
 	queue->buffer.watermark = queue->buffer.length / 2;
 	queue->dev = dev;
 	queue->ops = ops;
+	queue->driver_data = driver_data;
 
 	INIT_LIST_HEAD(&queue->incoming);
 	INIT_LIST_HEAD(&queue->outgoing);
diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index 6db24be7e11d..a4e7b97ce239 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -163,6 +163,8 @@ static const struct attribute *iio_dmaengine_buffer_attrs[] = {
  * iio_dmaengine_buffer_alloc() - Allocate new buffer which uses DMAengine
  * @dev: Parent device for the buffer
  * @channel: DMA channel name, typically "rx".
+ * @ops: Custom iio_dma_buffer_ops, if NULL default ops will be used
+ * @driver_data: Driver data to be passed to custom iio_dma_buffer_ops
  *
  * This allocates a new IIO buffer which internally uses the DMAengine framework
  * to perform its transfers. The parent device will be used to request the DMA
@@ -172,7 +174,8 @@ static const struct attribute *iio_dmaengine_buffer_attrs[] = {
  * release it.
  */
 static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
-	const char *channel)
+	const char *channel, const struct iio_dma_buffer_ops *ops,
+	void *driver_data)
 {
 	struct dmaengine_buffer *dmaengine_buffer;
 	unsigned int width, src_width, dest_width;
@@ -211,7 +214,7 @@ static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
 	dmaengine_buffer->max_size = dma_get_max_seg_size(chan->device->dev);
 
 	iio_dma_buffer_init(&dmaengine_buffer->queue, chan->device->dev,
-		&iio_dmaengine_default_ops);
+		ops ? ops : &iio_dmaengine_default_ops, driver_data);
 
 	dmaengine_buffer->queue.buffer.attrs = iio_dmaengine_buffer_attrs;
 	dmaengine_buffer->queue.buffer.access = &iio_dmaengine_buffer_ops;
@@ -249,6 +252,8 @@ static void __devm_iio_dmaengine_buffer_free(struct device *dev, void *res)
  * devm_iio_dmaengine_buffer_alloc() - Resource-managed iio_dmaengine_buffer_alloc()
  * @dev: Parent device for the buffer
  * @channel: DMA channel name, typically "rx".
+ * @ops: Custom iio_dma_buffer_ops, if NULL default ops will be used
+ * @driver_data: Driver data to be passed to custom iio_dma_buffer_ops
  *
  * This allocates a new IIO buffer which internally uses the DMAengine framework
  * to perform its transfers. The parent device will be used to request the DMA
@@ -257,7 +262,8 @@ static void __devm_iio_dmaengine_buffer_free(struct device *dev, void *res)
  * The buffer will be automatically de-allocated once the device gets destroyed.
  */
 static struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
-	const char *channel)
+	const char *channel, const struct iio_dma_buffer_ops *ops,
+	void *driver_data)
 {
 	struct iio_buffer **bufferp, *buffer;
 
@@ -266,7 +272,7 @@ static struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
 	if (!bufferp)
 		return ERR_PTR(-ENOMEM);
 
-	buffer = iio_dmaengine_buffer_alloc(dev, channel);
+	buffer = iio_dmaengine_buffer_alloc(dev, channel, ops, driver_data);
 	if (IS_ERR(buffer)) {
 		devres_free(bufferp);
 		return buffer;
@@ -283,6 +289,8 @@ static struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
  * @dev: Parent device for the buffer
  * @indio_dev: IIO device to which to attach this buffer.
  * @channel: DMA channel name, typically "rx".
+ * @ops: Custom iio_dma_buffer_ops, if NULL default ops will be used
+ * @driver_data: Driver data to be passed to custom iio_dma_buffer_ops
  *
  * This allocates a new IIO buffer with devm_iio_dmaengine_buffer_alloc()
  * and attaches it to an IIO device with iio_device_attach_buffer().
@@ -290,13 +298,14 @@ static struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
  * IIO device.
  */
 int devm_iio_dmaengine_buffer_setup(struct device *dev,
-				    struct iio_dev *indio_dev,
-				    const char *channel)
+	struct iio_dev *indio_dev, const char *channel,
+	const struct iio_dma_buffer_ops *ops,
+	void *driver_data)
 {
 	struct iio_buffer *buffer;
 
 	buffer = devm_iio_dmaengine_buffer_alloc(indio_dev->dev.parent,
-						 channel);
+						 channel, ops, driver_data);
 	if (IS_ERR(buffer))
 		return PTR_ERR(buffer);
 
diff --git a/include/linux/iio/buffer-dma.h b/include/linux/iio/buffer-dma.h
index f6f2ce3e2ed1..1eec7efe44cf 100644
--- a/include/linux/iio/buffer-dma.h
+++ b/include/linux/iio/buffer-dma.h
@@ -110,6 +110,8 @@ struct iio_dma_buffer_queue {
 
 	bool active;
 
+	void *driver_data;
+
 	unsigned int num_blocks;
 	struct iio_dma_buffer_block **blocks;
 	unsigned int max_offset;
@@ -144,7 +146,8 @@ int iio_dma_buffer_set_length(struct iio_buffer *buffer, unsigned int length);
 int iio_dma_buffer_request_update(struct iio_buffer *buffer);
 
 int iio_dma_buffer_init(struct iio_dma_buffer_queue *queue,
-	struct device *dma_dev, const struct iio_dma_buffer_ops *ops);
+	struct device *dma_dev, const struct iio_dma_buffer_ops *ops,
+	void *driver_data);
 void iio_dma_buffer_exit(struct iio_dma_buffer_queue *queue);
 void iio_dma_buffer_release(struct iio_dma_buffer_queue *queue);
 
diff --git a/include/linux/iio/buffer-dmaengine.h b/include/linux/iio/buffer-dmaengine.h
index 5c355be89814..1fca8cdbf14e 100644
--- a/include/linux/iio/buffer-dmaengine.h
+++ b/include/linux/iio/buffer-dmaengine.h
@@ -8,10 +8,12 @@
 #define __IIO_DMAENGINE_H__
 
 struct iio_dev;
+struct iio_dma_buffer_ops;
 struct device;
 
 int devm_iio_dmaengine_buffer_setup(struct device *dev,
-				    struct iio_dev *indio_dev,
-				    const char *channel);
+	struct iio_dev *indio_dev, const char *channel,
+	const struct iio_dma_buffer_ops *ops,
+	void *driver_data);
 
 #endif
-- 
2.27.0

[PATCH v3 5/6] iio: buffer-dma: Add output buffer support

[Alexandru Ardelean]

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

Add support for output buffers to the dma buffer implementation.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/adc/adi-axi-adc.c                 |   3 +-
 drivers/iio/buffer/industrialio-buffer-dma.c  | 116 ++++++++++++++++--
 .../buffer/industrialio-buffer-dmaengine.c    |  43 +++++--
 include/linux/iio/buffer-dma.h                |   6 +
 include/linux/iio/buffer-dmaengine.h          |   6 +-
 5 files changed, 151 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index bfa00100a631..98cc1e7caa69 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -104,6 +104,7 @@ static unsigned int adi_axi_adc_read(struct adi_axi_adc_state *st,
 static int adi_axi_adc_config_dma_buffer(struct device *dev,
 					 struct iio_dev *indio_dev)
 {
+	enum iio_buffer_direction dir = IIO_BUFFER_DIRECTION_IN;
 	const char *dma_name;
 
 	if (!device_property_present(dev, "dmas"))
@@ -113,7 +114,7 @@ static int adi_axi_adc_config_dma_buffer(struct device *dev,
 		dma_name = "rx";
 
 	return devm_iio_dmaengine_buffer_setup(indio_dev->dev.parent,
-					       indio_dev, dma_name,
+					       indio_dev, dir, dma_name,
 					       NULL, NULL);
 }
 
diff --git a/drivers/iio/buffer/industrialio-buffer-dma.c b/drivers/iio/buffer/industrialio-buffer-dma.c
index aa56c10418d0..83074d060535 100644
--- a/drivers/iio/buffer/industrialio-buffer-dma.c
+++ b/drivers/iio/buffer/industrialio-buffer-dma.c
@@ -223,7 +223,8 @@ void iio_dma_buffer_block_done(struct iio_dma_buffer_block *block)
 	spin_unlock_irqrestore(&queue->list_lock, flags);
 
 	iio_buffer_block_put_atomic(block);
-	wake_up_interruptible_poll(&queue->buffer.pollq, EPOLLIN | EPOLLRDNORM);
+	wake_up_interruptible_poll(&queue->buffer.pollq,
+		(uintptr_t)queue->poll_wakup_flags);
 }
 EXPORT_SYMBOL_GPL(iio_dma_buffer_block_done);
 
@@ -252,7 +253,8 @@ void iio_dma_buffer_block_list_abort(struct iio_dma_buffer_queue *queue,
 	}
 	spin_unlock_irqrestore(&queue->list_lock, flags);
 
-	wake_up_interruptible_poll(&queue->buffer.pollq, EPOLLIN | EPOLLRDNORM);
+	wake_up_interruptible_poll(&queue->buffer.pollq,
+		(uintptr_t)queue->poll_wakup_flags);
 }
 EXPORT_SYMBOL_GPL(iio_dma_buffer_block_list_abort);
 
@@ -353,9 +355,6 @@ int iio_dma_buffer_request_update(struct iio_buffer *buffer)
 		}
 
 		block->block.id = i;
-
-		block->state = IIO_BLOCK_STATE_QUEUED;
-		list_add_tail(&block->head, &queue->incoming);
 	}
 
 out_unlock:
@@ -437,7 +436,29 @@ int iio_dma_buffer_enable(struct iio_buffer *buffer,
 	struct iio_dma_buffer_block *block, *_block;
 
 	mutex_lock(&queue->lock);
+
+	if (buffer->direction == IIO_BUFFER_DIRECTION_IN)
+		queue->poll_wakup_flags = POLLIN | POLLRDNORM;
+	else
+		queue->poll_wakup_flags = POLLOUT | POLLWRNORM;
+
 	queue->fileio.enabled = !queue->num_blocks;
+	if (queue->fileio.enabled) {
+		unsigned int i;
+
+		for (i = 0; i < ARRAY_SIZE(queue->fileio.blocks); i++) {
+			struct iio_dma_buffer_block *block =
+			    queue->fileio.blocks[i];
+			if (buffer->direction == IIO_BUFFER_DIRECTION_IN) {
+				block->state = IIO_BLOCK_STATE_QUEUED;
+				list_add_tail(&block->head, &queue->incoming);
+			} else {
+				block->state = IIO_BLOCK_STATE_DEQUEUED;
+				list_add_tail(&block->head, &queue->outgoing);
+			}
+		}
+	}
+
 	queue->active = true;
 	list_for_each_entry_safe(block, _block, &queue->incoming, head) {
 		list_del(&block->head);
@@ -567,6 +588,61 @@ int iio_dma_buffer_read(struct iio_buffer *buffer, size_t n,
 }
 EXPORT_SYMBOL_GPL(iio_dma_buffer_read);
 
+int iio_dma_buffer_write(struct iio_buffer *buf, size_t n,
+	const char __user *user_buffer)
+{
+	struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buf);
+	struct iio_dma_buffer_block *block;
+	int ret;
+
+	if (n < buf->bytes_per_datum)
+		return -EINVAL;
+
+	mutex_lock(&queue->lock);
+
+	if (!queue->fileio.enabled) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
+	if (!queue->fileio.active_block) {
+		block = iio_dma_buffer_dequeue(queue);
+		if (block == NULL) {
+			ret = 0;
+			goto out_unlock;
+		}
+		queue->fileio.pos = 0;
+		queue->fileio.active_block = block;
+	} else {
+		block = queue->fileio.active_block;
+	}
+
+	n = rounddown(n, buf->bytes_per_datum);
+	if (n > block->block.size - queue->fileio.pos)
+		n = block->block.size - queue->fileio.pos;
+
+	if (copy_from_user(block->vaddr + queue->fileio.pos, user_buffer, n)) {
+		ret = -EFAULT;
+		goto out_unlock;
+	}
+
+	queue->fileio.pos += n;
+
+	if (queue->fileio.pos == block->block.size) {
+		queue->fileio.active_block = NULL;
+		block->block.bytes_used = block->block.size;
+		iio_dma_buffer_enqueue(queue, block);
+	}
+
+	ret = n;
+
+out_unlock:
+	mutex_unlock(&queue->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iio_dma_buffer_write);
+
 /**
  * iio_dma_buffer_data_available() - DMA buffer data_available callback
  * @buf: Buffer to check for data availability
@@ -588,12 +664,14 @@ size_t iio_dma_buffer_data_available(struct iio_buffer *buf)
 	 */
 
 	mutex_lock(&queue->lock);
-	if (queue->fileio.active_block)
-		data_available += queue->fileio.active_block->block.size;
+	if (queue->fileio.active_block) {
+		data_available += queue->fileio.active_block->block.bytes_used -
+			queue->fileio.pos;
+	}
 
 	spin_lock_irq(&queue->list_lock);
 	list_for_each_entry(block, &queue->outgoing, head)
-		data_available += block->block.size;
+		data_available += block->block.bytes_used;
 	spin_unlock_irq(&queue->list_lock);
 	mutex_unlock(&queue->lock);
 
@@ -601,6 +679,28 @@ size_t iio_dma_buffer_data_available(struct iio_buffer *buf)
 }
 EXPORT_SYMBOL_GPL(iio_dma_buffer_data_available);
 
+size_t iio_dma_buffer_space_available(struct iio_buffer *buf)
+{
+	struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buf);
+	struct iio_dma_buffer_block *block;
+	size_t space_available = 0;
+
+	mutex_lock(&queue->lock);
+	if (queue->fileio.active_block) {
+		space_available += queue->fileio.active_block->block.size -
+			queue->fileio.pos;
+	}
+
+	spin_lock_irq(&queue->list_lock);
+	list_for_each_entry(block, &queue->outgoing, head)
+		space_available += block->block.size;
+	spin_unlock_irq(&queue->list_lock);
+	mutex_unlock(&queue->lock);
+
+	return space_available;
+}
+EXPORT_SYMBOL_GPL(iio_dma_buffer_space_available);
+
 int iio_dma_buffer_alloc_blocks(struct iio_buffer *buffer,
 				struct iio_buffer_block_alloc_req *req)
 {
diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index a4e7b97ce239..65458a6cc81a 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -37,6 +37,8 @@ struct dmaengine_buffer {
 
 	u32 align;
 	u32 max_size;
+
+	bool is_tx;
 };
 
 static struct dmaengine_buffer *iio_buffer_to_dmaengine_buffer(
@@ -64,9 +66,12 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
 	struct dmaengine_buffer *dmaengine_buffer =
 		iio_buffer_to_dmaengine_buffer(&queue->buffer);
 	struct dma_async_tx_descriptor *desc;
+	enum dma_transfer_direction direction;
 	dma_cookie_t cookie;
 
-	block->block.bytes_used = min(block->block.size,
+	if (!dmaengine_buffer->is_tx)
+		block->block.bytes_used = block->block.size;
+	block->block.bytes_used = min(block->block.bytes_used,
 		dmaengine_buffer->max_size);
 	block->block.bytes_used = rounddown(block->block.bytes_used,
 		dmaengine_buffer->align);
@@ -75,8 +80,10 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
 		return 0;
 	}
 
+	direction = dmaengine_buffer->is_tx ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM;
+
 	desc = dmaengine_prep_slave_single(dmaengine_buffer->chan,
-		block->phys_addr, block->block.bytes_used, DMA_DEV_TO_MEM,
+		block->phys_addr, block->block.bytes_used, direction,
 		DMA_PREP_INTERRUPT);
 	if (!desc)
 		return -ENOMEM;
@@ -117,12 +124,14 @@ static void iio_dmaengine_buffer_release(struct iio_buffer *buf)
 
 static const struct iio_buffer_access_funcs iio_dmaengine_buffer_ops = {
 	.read = iio_dma_buffer_read,
+	.write = iio_dma_buffer_write,
 	.set_bytes_per_datum = iio_dma_buffer_set_bytes_per_datum,
 	.set_length = iio_dma_buffer_set_length,
 	.request_update = iio_dma_buffer_request_update,
 	.enable = iio_dma_buffer_enable,
 	.disable = iio_dma_buffer_disable,
 	.data_available = iio_dma_buffer_data_available,
+	.space_available = iio_dma_buffer_space_available,
 	.release = iio_dmaengine_buffer_release,
 
 	.alloc_blocks = iio_dma_buffer_alloc_blocks,
@@ -162,6 +171,7 @@ static const struct attribute *iio_dmaengine_buffer_attrs[] = {
 /**
  * iio_dmaengine_buffer_alloc() - Allocate new buffer which uses DMAengine
  * @dev: Parent device for the buffer
+ * @direction: Set the direction of the data.
  * @channel: DMA channel name, typically "rx".
  * @ops: Custom iio_dma_buffer_ops, if NULL default ops will be used
  * @driver_data: Driver data to be passed to custom iio_dma_buffer_ops
@@ -174,11 +184,12 @@ static const struct attribute *iio_dmaengine_buffer_attrs[] = {
  * release it.
  */
 static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
-	const char *channel, const struct iio_dma_buffer_ops *ops,
-	void *driver_data)
+	enum iio_buffer_direction direction, const char *channel,
+	const struct iio_dma_buffer_ops *ops, void *driver_data)
 {
 	struct dmaengine_buffer *dmaengine_buffer;
 	unsigned int width, src_width, dest_width;
+	bool is_tx = (direction == IIO_BUFFER_DIRECTION_OUT);
 	struct dma_slave_caps caps;
 	struct dma_chan *chan;
 	int ret;
@@ -187,6 +198,9 @@ static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
 	if (!dmaengine_buffer)
 		return ERR_PTR(-ENOMEM);
 
+	if (!channel)
+		channel = is_tx ? "tx" : "rx";
+
 	chan = dma_request_chan(dev, channel);
 	if (IS_ERR(chan)) {
 		ret = PTR_ERR(chan);
@@ -212,6 +226,7 @@ static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
 	dmaengine_buffer->chan = chan;
 	dmaengine_buffer->align = width;
 	dmaengine_buffer->max_size = dma_get_max_seg_size(chan->device->dev);
+	dmaengine_buffer->is_tx = is_tx;
 
 	iio_dma_buffer_init(&dmaengine_buffer->queue, chan->device->dev,
 		ops ? ops : &iio_dmaengine_default_ops, driver_data);
@@ -251,7 +266,8 @@ static void __devm_iio_dmaengine_buffer_free(struct device *dev, void *res)
 /**
  * devm_iio_dmaengine_buffer_alloc() - Resource-managed iio_dmaengine_buffer_alloc()
  * @dev: Parent device for the buffer
- * @channel: DMA channel name, typically "rx".
+ * @direction: Direction of the data stream (in/out).
+ * @channel: DMA channel name, typically "rx" for input, "tx" for output.
  * @ops: Custom iio_dma_buffer_ops, if NULL default ops will be used
  * @driver_data: Driver data to be passed to custom iio_dma_buffer_ops
  *
@@ -262,8 +278,8 @@ static void __devm_iio_dmaengine_buffer_free(struct device *dev, void *res)
  * The buffer will be automatically de-allocated once the device gets destroyed.
  */
 static struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
-	const char *channel, const struct iio_dma_buffer_ops *ops,
-	void *driver_data)
+	enum iio_buffer_direction direction, const char *channel,
+	const struct iio_dma_buffer_ops *ops, void *driver_data)
 {
 	struct iio_buffer **bufferp, *buffer;
 
@@ -272,7 +288,8 @@ static struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
 	if (!bufferp)
 		return ERR_PTR(-ENOMEM);
 
-	buffer = iio_dmaengine_buffer_alloc(dev, channel, ops, driver_data);
+	buffer = iio_dmaengine_buffer_alloc(dev, direction, channel, ops,
+					    driver_data);
 	if (IS_ERR(buffer)) {
 		devres_free(bufferp);
 		return buffer;
@@ -288,7 +305,8 @@ static struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
  * devm_iio_dmaengine_buffer_setup() - Setup a DMA buffer for an IIO device
  * @dev: Parent device for the buffer
  * @indio_dev: IIO device to which to attach this buffer.
- * @channel: DMA channel name, typically "rx".
+ * @direction: Direction of the data stream (in/out).
+ * @channel: DMA channel name, typically "rx" for input, "tx" for output.
  * @ops: Custom iio_dma_buffer_ops, if NULL default ops will be used
  * @driver_data: Driver data to be passed to custom iio_dma_buffer_ops
  *
@@ -298,14 +316,15 @@ static struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
  * IIO device.
  */
 int devm_iio_dmaengine_buffer_setup(struct device *dev,
-	struct iio_dev *indio_dev, const char *channel,
-	const struct iio_dma_buffer_ops *ops,
+	struct iio_dev *indio_dev, enum iio_buffer_direction direction,
+	const char *channel, const struct iio_dma_buffer_ops *ops,
 	void *driver_data)
 {
 	struct iio_buffer *buffer;
 
 	buffer = devm_iio_dmaengine_buffer_alloc(indio_dev->dev.parent,
-						 channel, ops, driver_data);
+						 direction, channel, ops,
+						 driver_data);
 	if (IS_ERR(buffer))
 		return PTR_ERR(buffer);
 
diff --git a/include/linux/iio/buffer-dma.h b/include/linux/iio/buffer-dma.h
index 1eec7efe44cf..9a99c74fab16 100644
--- a/include/linux/iio/buffer-dma.h
+++ b/include/linux/iio/buffer-dma.h
@@ -112,6 +112,8 @@ struct iio_dma_buffer_queue {
 
 	void *driver_data;
 
+	unsigned int poll_wakup_flags;
+
 	unsigned int num_blocks;
 	struct iio_dma_buffer_block **blocks;
 	unsigned int max_offset;
@@ -145,6 +147,10 @@ int iio_dma_buffer_set_bytes_per_datum(struct iio_buffer *buffer, size_t bpd);
 int iio_dma_buffer_set_length(struct iio_buffer *buffer, unsigned int length);
 int iio_dma_buffer_request_update(struct iio_buffer *buffer);
 
+int iio_dma_buffer_write(struct iio_buffer *buf, size_t n,
+	const char __user *user_buffer);
+size_t iio_dma_buffer_space_available(struct iio_buffer *buf);
+
 int iio_dma_buffer_init(struct iio_dma_buffer_queue *queue,
 	struct device *dma_dev, const struct iio_dma_buffer_ops *ops,
 	void *driver_data);
diff --git a/include/linux/iio/buffer-dmaengine.h b/include/linux/iio/buffer-dmaengine.h
index 1fca8cdbf14e..1d70d35946b1 100644
--- a/include/linux/iio/buffer-dmaengine.h
+++ b/include/linux/iio/buffer-dmaengine.h
@@ -7,13 +7,15 @@
 #ifndef __IIO_DMAENGINE_H__
 #define __IIO_DMAENGINE_H__
 
+#include <linux/iio/buffer.h>
+
 struct iio_dev;
 struct iio_dma_buffer_ops;
 struct device;
 
 int devm_iio_dmaengine_buffer_setup(struct device *dev,
-	struct iio_dev *indio_dev, const char *channel,
-	const struct iio_dma_buffer_ops *ops,
+	struct iio_dev *indio_dev, enum iio_buffer_direction direction,
+       	const char *channel, const struct iio_dma_buffer_ops *ops,
 	void *driver_data);
 
 #endif
-- 
2.27.0

[PATCH v3 6/6] iio: buffer-dma: add support for cyclic DMA transfers

[Alexandru Ardelean]

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

This change adds support for cyclic DMA transfers using the IIO buffer DMA
infrastructure.
To do this, userspace must set the IIO_BUFFER_BLOCK_FLAG_CYCLIC flag on the
block when enqueueing them via the ENQUEUE_BLOCK ioctl().

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 .../buffer/industrialio-buffer-dmaengine.c    | 24 ++++++++++++-------
 include/uapi/linux/iio/buffer.h               |  1 +
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index 65458a6cc81a..39cc230c7991 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -82,14 +82,22 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
 
 	direction = dmaengine_buffer->is_tx ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM;
 
-	desc = dmaengine_prep_slave_single(dmaengine_buffer->chan,
-		block->phys_addr, block->block.bytes_used, direction,
-		DMA_PREP_INTERRUPT);
-	if (!desc)
-		return -ENOMEM;
-
-	desc->callback_result = iio_dmaengine_buffer_block_done;
-	desc->callback_param = block;
+	if (block->block.flags & IIO_BUFFER_BLOCK_FLAG_CYCLIC) {
+		desc = dmaengine_prep_dma_cyclic(dmaengine_buffer->chan,
+			block->phys_addr, block->block.bytes_used,
+			block->block.bytes_used, direction, 0);
+		if (!desc)
+			return -ENOMEM;
+	} else {
+		desc = dmaengine_prep_slave_single(dmaengine_buffer->chan,
+			block->phys_addr, block->block.bytes_used, direction,
+			DMA_PREP_INTERRUPT);
+		if (!desc)
+			return -ENOMEM;
+
+		desc->callback_result = iio_dmaengine_buffer_block_done;
+		desc->callback_param = block;
+	}
 
 	cookie = dmaengine_submit(desc);
 	if (dma_submit_error(cookie))
diff --git a/include/uapi/linux/iio/buffer.h b/include/uapi/linux/iio/buffer.h
index 4e4ee9befea1..1bde508fe1b9 100644
--- a/include/uapi/linux/iio/buffer.h
+++ b/include/uapi/linux/iio/buffer.h
@@ -33,6 +33,7 @@ struct iio_buffer_block_alloc_req {
 
 /* A function will be assigned later for BIT(0) */
 #define IIO_BUFFER_BLOCK_FLAG_RESERVED		(1 << 0)
+#define IIO_BUFFER_BLOCK_FLAG_CYCLIC		(1 << 1)
 
 /**
  * struct iio_buffer_block - Descriptor for a single IIO block
-- 
2.27.0

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

[Jonathan Cameron]

On Fri, 19 Feb 2021 14:40:06 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> Changelog v2 -> v3:
> * https://lore.kernel.org/linux-iio/20210217083438.37865-4-alexandru.ardelean@analog.com/T/#m396545e0c6cc9d58e17f4d79b6fc707fd0373d89
> * adding only infrastructure pieces for output DAC buffers, unfortunately I
>   couldn't finish a complete DAC change to showcase these changes

For that I wonder if the driver you did previously would work with the hrtimer
trigger (so just drop the pwm stuff).  Obviously we'd want someone to sanity
check that with the hardware.  An alternative (for now) would be to add
a simple example to the dummy driver.

I'm not keen to take this series without the user, but I'll review it on basis
we'll get that sorted fairly soon in some fashion.

Jonathan


> * patch 'iio: Add output buffer support'
>    - moved new 'bufferY/direction' attribute at the end and added
>      comment about what it should be added at the end
> * removed Lars' comment '/* need a way of knowing if there may be enough data... */'
> * updated some various formatting;
> 
> Alexandru Ardelean (1):
>   iio: triggered-buffer: extend support to configure output buffers
> 
> Lars-Peter Clausen (5):
>   iio: Add output buffer support
>   iio: kfifo-buffer: Add output buffer support
>   iio: buffer-dma: Allow to provide custom buffer ops
>   iio: buffer-dma: Add output buffer support
>   iio: buffer-dma: add support for cyclic DMA transfers
> 
>  Documentation/ABI/testing/sysfs-bus-iio       |   7 +
>  drivers/iio/accel/adxl372.c                   |   1 +
>  drivers/iio/accel/bmc150-accel-core.c         |   1 +
>  drivers/iio/adc/adi-axi-adc.c                 |   4 +-
>  drivers/iio/adc/at91-sama5d2_adc.c            |   4 +-
>  drivers/iio/buffer/industrialio-buffer-dma.c  | 120 ++++++++++++++--
>  .../buffer/industrialio-buffer-dmaengine.c    |  72 +++++++---
>  .../buffer/industrialio-triggered-buffer.c    |   8 +-
>  drivers/iio/buffer/kfifo_buf.c                |  50 +++++++
>  .../cros_ec_sensors/cros_ec_sensors_core.c    |   1 +
>  .../common/hid-sensors/hid-sensor-trigger.c   |   5 +-
>  drivers/iio/industrialio-buffer.c             | 133 +++++++++++++++++-
>  include/linux/iio/buffer-dma.h                |  11 +-
>  include/linux/iio/buffer-dmaengine.h          |   8 +-
>  include/linux/iio/buffer.h                    |   7 +
>  include/linux/iio/buffer_impl.h               |  11 ++
>  include/linux/iio/triggered_buffer.h          |  11 +-
>  include/uapi/linux/iio/buffer.h               |   1 +
>  18 files changed, 412 insertions(+), 43 deletions(-)
> 

Re: [PATCH v3 6/6] iio: buffer-dma: add support for cyclic DMA transfers

[Jonathan Cameron]

On Fri, 19 Feb 2021 14:40:12 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> From: Lars-Peter Clausen <lars@metafoo.de>
> 
> This change adds support for cyclic DMA transfers using the IIO buffer DMA
> infrastructure.
> To do this, userspace must set the IIO_BUFFER_BLOCK_FLAG_CYCLIC flag on the
> block when enqueueing them via the ENQUEUE_BLOCK ioctl().
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Series in general looks good to me, but this change needs a little more
detail + probably some level of example userspace flow.

I don't really understand how this is used!

Also, it's easy to test output buffers with the kfifo support so we
should be able to move forward quickly with that part (1-3, 4 is probably
fine as well as clearly harmless).

The dma stuff worries me more, at least partly based on the experience
of the original dma buffers which basically sat their unused (in upstream)
for a very long time.   So to move these forward, they need to come
with users...

Thanks,

Jonathan

> ---
>  .../buffer/industrialio-buffer-dmaengine.c    | 24 ++++++++++++-------
>  include/uapi/linux/iio/buffer.h               |  1 +
>  2 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> index 65458a6cc81a..39cc230c7991 100644
> --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> @@ -82,14 +82,22 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
>  
>  	direction = dmaengine_buffer->is_tx ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM;
>  
> -	desc = dmaengine_prep_slave_single(dmaengine_buffer->chan,
> -		block->phys_addr, block->block.bytes_used, direction,
> -		DMA_PREP_INTERRUPT);
> -	if (!desc)
> -		return -ENOMEM;
> -
> -	desc->callback_result = iio_dmaengine_buffer_block_done;
> -	desc->callback_param = block;
> +	if (block->block.flags & IIO_BUFFER_BLOCK_FLAG_CYCLIC) {
> +		desc = dmaengine_prep_dma_cyclic(dmaengine_buffer->chan,
> +			block->phys_addr, block->block.bytes_used,
> +			block->block.bytes_used, direction, 0);
> +		if (!desc)
> +			return -ENOMEM;
> +	} else {
> +		desc = dmaengine_prep_slave_single(dmaengine_buffer->chan,
> +			block->phys_addr, block->block.bytes_used, direction,
> +			DMA_PREP_INTERRUPT);
> +		if (!desc)
> +			return -ENOMEM;
> +
> +		desc->callback_result = iio_dmaengine_buffer_block_done;
> +		desc->callback_param = block;
> +	}
>  
>  	cookie = dmaengine_submit(desc);
>  	if (dma_submit_error(cookie))
> diff --git a/include/uapi/linux/iio/buffer.h b/include/uapi/linux/iio/buffer.h
> index 4e4ee9befea1..1bde508fe1b9 100644
> --- a/include/uapi/linux/iio/buffer.h
> +++ b/include/uapi/linux/iio/buffer.h
> @@ -33,6 +33,7 @@ struct iio_buffer_block_alloc_req {
>  
>  /* A function will be assigned later for BIT(0) */
>  #define IIO_BUFFER_BLOCK_FLAG_RESERVED		(1 << 0)
> +#define IIO_BUFFER_BLOCK_FLAG_CYCLIC		(1 << 1)
>  
>  /**
>   * struct iio_buffer_block - Descriptor for a single IIO block

Re: [PATCH v3 6/6] iio: buffer-dma: add support for cyclic DMA transfers

[Alexandru Ardelean]

On Sun, Feb 21, 2021 at 2:11 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Fri, 19 Feb 2021 14:40:12 +0200
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
> > From: Lars-Peter Clausen <lars@metafoo.de>
> >
> > This change adds support for cyclic DMA transfers using the IIO buffer DMA
> > infrastructure.
> > To do this, userspace must set the IIO_BUFFER_BLOCK_FLAG_CYCLIC flag on the
> > block when enqueueing them via the ENQUEUE_BLOCK ioctl().
> >
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> Series in general looks good to me, but this change needs a little more
> detail + probably some level of example userspace flow.
>
> I don't really understand how this is used!
>
> Also, it's easy to test output buffers with the kfifo support so we
> should be able to move forward quickly with that part (1-3, 4 is probably
> fine as well as clearly harmless).
>
> The dma stuff worries me more, at least partly based on the experience
> of the original dma buffers which basically sat their unused (in upstream)
> for a very long time.   So to move these forward, they need to come
> with users...

So, this series will need to be re-sent/re-tested by someone else.
I'm on my last week at ADI and I'm on vacation.

Maybe I can manage to setup something to test as well, but it will take a while.



>
> Thanks,
>
> Jonathan
>
> > ---
> >  .../buffer/industrialio-buffer-dmaengine.c    | 24 ++++++++++++-------
> >  include/uapi/linux/iio/buffer.h               |  1 +
> >  2 files changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > index 65458a6cc81a..39cc230c7991 100644
> > --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > @@ -82,14 +82,22 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> >
> >       direction = dmaengine_buffer->is_tx ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM;
> >
> > -     desc = dmaengine_prep_slave_single(dmaengine_buffer->chan,
> > -             block->phys_addr, block->block.bytes_used, direction,
> > -             DMA_PREP_INTERRUPT);
> > -     if (!desc)
> > -             return -ENOMEM;
> > -
> > -     desc->callback_result = iio_dmaengine_buffer_block_done;
> > -     desc->callback_param = block;
> > +     if (block->block.flags & IIO_BUFFER_BLOCK_FLAG_CYCLIC) {
> > +             desc = dmaengine_prep_dma_cyclic(dmaengine_buffer->chan,
> > +                     block->phys_addr, block->block.bytes_used,
> > +                     block->block.bytes_used, direction, 0);
> > +             if (!desc)
> > +                     return -ENOMEM;
> > +     } else {
> > +             desc = dmaengine_prep_slave_single(dmaengine_buffer->chan,
> > +                     block->phys_addr, block->block.bytes_used, direction,
> > +                     DMA_PREP_INTERRUPT);
> > +             if (!desc)
> > +                     return -ENOMEM;
> > +
> > +             desc->callback_result = iio_dmaengine_buffer_block_done;
> > +             desc->callback_param = block;
> > +     }
> >
> >       cookie = dmaengine_submit(desc);
> >       if (dma_submit_error(cookie))
> > diff --git a/include/uapi/linux/iio/buffer.h b/include/uapi/linux/iio/buffer.h
> > index 4e4ee9befea1..1bde508fe1b9 100644
> > --- a/include/uapi/linux/iio/buffer.h
> > +++ b/include/uapi/linux/iio/buffer.h
> > @@ -33,6 +33,7 @@ struct iio_buffer_block_alloc_req {
> >
> >  /* A function will be assigned later for BIT(0) */
> >  #define IIO_BUFFER_BLOCK_FLAG_RESERVED               (1 << 0)
> > +#define IIO_BUFFER_BLOCK_FLAG_CYCLIC         (1 << 1)
> >
> >  /**
> >   * struct iio_buffer_block - Descriptor for a single IIO block
>

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

[Hennerich, Michael]

Hi Jonathan and others,

With output/dac buffer support the semantics of the scan_element type may change.

Today the Format is [be|le]:[s|u]bits/storagebitsXrepeat[>>shift].

While shift (if specified) is the shift that needs to be applied prior to masking out unused bits.

So far so good and it sounds universal. 

However, we use the right shift (operator) for that, which makes sense for capture devices.
For output devices the more logical operator would be the left shift.

I'm not proposing a new Format here. I just want to get some agreement that for an output device

le:s12/16>>4

is understood as a left shift of 4, since the unused bits are then on the LSB.

Thoughts?

Best Regards,
Michael

Analog Devices GmbH
Michael Hennerich                       
Otl-Aicher Strasse 60-64
D-80807 Muenchen; Germany

Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Stefan Steyerl, Michael Paul Sondel, Yoon Ah Oh

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

[Jonathan Cameron]

On Fri, 5 Mar 2021 08:57:08 +0000
"Hennerich, Michael" <Michael.Hennerich@analog.com> wrote:

> Hi Jonathan and others,
> 
> With output/dac buffer support the semantics of the scan_element type may change.
> 
> Today the Format is [be|le]:[s|u]bits/storagebitsXrepeat[>>shift].
> 
> While shift (if specified) is the shift that needs to be applied prior to masking out unused bits.
> 
> So far so good and it sounds universal. 
> 
> However, we use the right shift (operator) for that, which makes sense for capture devices.
> For output devices the more logical operator would be the left shift.
> 
> I'm not proposing a new Format here. I just want to get some agreement that for an output device
> 
> le:s12/16>>4  
> 
> is understood as a left shift of 4, since the unused bits are then on the LSB.

Good question. Guess I wasn't thinking ahead when I came up with that :)

I'm not sure I'd mind if we did decide to define a new format for output
buffers. Feels like it should be easy to do.

What do others think?

Jonathan


> 
> Thoughts?
> 
> Best Regards,
> Michael
> 
> Analog Devices GmbH
> Michael Hennerich                       
> Otl-Aicher Strasse 60-64
> D-80807 Muenchen; Germany
> 
> Sitz der Gesellschaft München, Registergericht München HRB 40368,
> Geschäftsführer: Stefan Steyerl, Michael Paul Sondel, Yoon Ah Oh
> 
> 

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

[Sa, Nuno]

> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, March 6, 2021 6:35 PM
> To: Hennerich, Michael <Michael.Hennerich@analog.com>
> Cc: zzzzArdelean, zzzzAlexandru <alexandru.Ardelean@analog.com>;
> linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> lars@metafoo.de; Sa, Nuno <Nuno.Sa@analog.com>; Bogdan, Dragos
> <Dragos.Bogdan@analog.com>
> Subject: Re: [PATCH v3 0/6] iio: Add output buffer support
> 
> On Fri, 5 Mar 2021 08:57:08 +0000
> "Hennerich, Michael" <Michael.Hennerich@analog.com> wrote:
> 
> > Hi Jonathan and others,
> >
> > With output/dac buffer support the semantics of the scan_element
> type may change.
> >
> > Today the Format is [be|le]:[s|u]bits/storagebitsXrepeat[>>shift].
> >
> > While shift (if specified) is the shift that needs to be applied prior to
> masking out unused bits.
> >
> > So far so good and it sounds universal.
> >
> > However, we use the right shift (operator) for that, which makes
> sense for capture devices.
> > For output devices the more logical operator would be the left shift.
> >
> > I'm not proposing a new Format here. I just want to get some
> agreement that for an output device
> >
> > le:s12/16>>4
> >
> > is understood as a left shift of 4, since the unused bits are then on
> the LSB.
> 
> Good question. Guess I wasn't thinking ahead when I came up with
> that :)
> 
> I'm not sure I'd mind if we did decide to define a new format for
> output
> buffers. Feels like it should be easy to do.
> 
> What do others think?
> 

I guess the most straight forward thing would be just to add a 'shift_l' variable
to 'struct scan_type'' and make sure either 'shift_l' or 'shift' is defined and then
properly export either ">>" or "<<" to userspace?

- Nuno Sá 

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

[Jonathan Cameron]

On Mon, 8 Mar 2021 10:07:05 +0000
"Sa, Nuno" <Nuno.Sa@analog.com> wrote:

> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Saturday, March 6, 2021 6:35 PM
> > To: Hennerich, Michael <Michael.Hennerich@analog.com>
> > Cc: zzzzArdelean, zzzzAlexandru <alexandru.Ardelean@analog.com>;
> > linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> > lars@metafoo.de; Sa, Nuno <Nuno.Sa@analog.com>; Bogdan, Dragos
> > <Dragos.Bogdan@analog.com>
> > Subject: Re: [PATCH v3 0/6] iio: Add output buffer support
> > 
> > On Fri, 5 Mar 2021 08:57:08 +0000
> > "Hennerich, Michael" <Michael.Hennerich@analog.com> wrote:
> >   
> > > Hi Jonathan and others,
> > >
> > > With output/dac buffer support the semantics of the scan_element  
> > type may change.  
> > >
> > > Today the Format is [be|le]:[s|u]bits/storagebitsXrepeat[>>shift].
> > >
> > > While shift (if specified) is the shift that needs to be applied prior to  
> > masking out unused bits.  
> > >
> > > So far so good and it sounds universal.
> > >
> > > However, we use the right shift (operator) for that, which makes  
> > sense for capture devices.  
> > > For output devices the more logical operator would be the left shift.
> > >
> > > I'm not proposing a new Format here. I just want to get some  
> > agreement that for an output device  
> > >  
> > > le:s12/16>>4  
> > >
> > > is understood as a left shift of 4, since the unused bits are then on  
> > the LSB.
> > 
> > Good question. Guess I wasn't thinking ahead when I came up with
> > that :)
> > 
> > I'm not sure I'd mind if we did decide to define a new format for
> > output
> > buffers. Feels like it should be easy to do.
> > 
> > What do others think?
> >   
> 
> I guess the most straight forward thing would be just to add a 'shift_l' variable
> to 'struct scan_type'' and make sure either 'shift_l' or 'shift' is defined and then
> properly export either ">>" or "<<" to userspace?

Given we already know it's an output channel, can we not just use that
to make the decision?

Jonathan

> 
> - Nuno Sá 
> 

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

[Sa, Nuno]

> -----Original Message-----
> From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Sent: Monday, March 8, 2021 12:52 PM
> To: Sa, Nuno <Nuno.Sa@analog.com>; Jonathan Cameron
> <jic23@kernel.org>
> Cc: Hennerich, Michael <Michael.Hennerich@analog.com>;
> zzzzArdelean, zzzzAlexandru <alexandru.Ardelean@analog.com>;
> linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> lars@metafoo.de; Bogdan, Dragos <Dragos.Bogdan@analog.com>
> Subject: Re: [PATCH v3 0/6] iio: Add output buffer support
> 
> [External]
> 
> On Mon, 8 Mar 2021 10:07:05 +0000
> "Sa, Nuno" <Nuno.Sa@analog.com> wrote:
> 
> > > -----Original Message-----
> > > From: Jonathan Cameron <jic23@kernel.org>
> > > Sent: Saturday, March 6, 2021 6:35 PM
> > > To: Hennerich, Michael <Michael.Hennerich@analog.com>
> > > Cc: zzzzArdelean, zzzzAlexandru
> <alexandru.Ardelean@analog.com>;
> > > linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> > > lars@metafoo.de; Sa, Nuno <Nuno.Sa@analog.com>; Bogdan,
> Dragos
> > > <Dragos.Bogdan@analog.com>
> > > Subject: Re: [PATCH v3 0/6] iio: Add output buffer support
> > >
> > > On Fri, 5 Mar 2021 08:57:08 +0000
> > > "Hennerich, Michael" <Michael.Hennerich@analog.com> wrote:
> > >
> > > > Hi Jonathan and others,
> > > >
> > > > With output/dac buffer support the semantics of the
> scan_element
> > > type may change.
> > > >
> > > > Today the Format is
> [be|le]:[s|u]bits/storagebitsXrepeat[>>shift].
> > > >
> > > > While shift (if specified) is the shift that needs to be applied prior
> to
> > > masking out unused bits.
> > > >
> > > > So far so good and it sounds universal.
> > > >
> > > > However, we use the right shift (operator) for that, which makes
> > > sense for capture devices.
> > > > For output devices the more logical operator would be the left
> shift.
> > > >
> > > > I'm not proposing a new Format here. I just want to get some
> > > agreement that for an output device
> > > >
> > > > le:s12/16>>4
> > > >
> > > > is understood as a left shift of 4, since the unused bits are then
> on
> > > the LSB.
> > >
> > > Good question. Guess I wasn't thinking ahead when I came up with
> > > that :)
> > >
> > > I'm not sure I'd mind if we did decide to define a new format for
> > > output
> > > buffers. Feels like it should be easy to do.
> > >
> > > What do others think?
> > >
> >
> > I guess the most straight forward thing would be just to add a 'shift_l'
> variable
> > to 'struct scan_type'' and make sure either 'shift_l' or 'shift' is defined
> and then
> > properly export either ">>" or "<<" to userspace?
> 
> Given we already know it's an output channel, can we not just use that
> to make the decision?
> 
> Jonathan

I would argue that having two variables gives us more flexibility for whatever
the future brings us :). But if we can sanely say that an output buffer will
always use left shifts, then we could definitely use that information... I mean,
we are already doing that assumption for input buffers and right shifts...

- Nuno Sá

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

[Sa, Nuno]

> -----Original Message-----
> From: Sa, Nuno <Nuno.Sa@analog.com>
> Sent: Monday, March 8, 2021 2:01 PM
> To: Jonathan Cameron <Jonathan.Cameron@Huawei.com>; Jonathan
> Cameron <jic23@kernel.org>
> Cc: Hennerich, Michael <Michael.Hennerich@analog.com>;
> zzzzArdelean, zzzzAlexandru <alexandru.Ardelean@analog.com>;
> linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> lars@metafoo.de; Bogdan, Dragos <Dragos.Bogdan@analog.com>
> Subject: RE: [PATCH v3 0/6] iio: Add output buffer support
> 
> [External]
> 
> 
> 
> > -----Original Message-----
> > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> > Sent: Monday, March 8, 2021 12:52 PM
> > To: Sa, Nuno <Nuno.Sa@analog.com>; Jonathan Cameron
> > <jic23@kernel.org>
> > Cc: Hennerich, Michael <Michael.Hennerich@analog.com>;
> > zzzzArdelean, zzzzAlexandru <alexandru.Ardelean@analog.com>;
> > linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> > lars@metafoo.de; Bogdan, Dragos <Dragos.Bogdan@analog.com>
> > Subject: Re: [PATCH v3 0/6] iio: Add output buffer support
> >
> > [External]
> >
> > On Mon, 8 Mar 2021 10:07:05 +0000
> > "Sa, Nuno" <Nuno.Sa@analog.com> wrote:
> >
> > > > -----Original Message-----
> > > > From: Jonathan Cameron <jic23@kernel.org>
> > > > Sent: Saturday, March 6, 2021 6:35 PM
> > > > To: Hennerich, Michael <Michael.Hennerich@analog.com>
> > > > Cc: zzzzArdelean, zzzzAlexandru
> > <alexandru.Ardelean@analog.com>;
> > > > linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> > > > lars@metafoo.de; Sa, Nuno <Nuno.Sa@analog.com>; Bogdan,
> > Dragos
> > > > <Dragos.Bogdan@analog.com>
> > > > Subject: Re: [PATCH v3 0/6] iio: Add output buffer support
> > > >
> > > > On Fri, 5 Mar 2021 08:57:08 +0000
> > > > "Hennerich, Michael" <Michael.Hennerich@analog.com> wrote:
> > > >
> > > > > Hi Jonathan and others,
> > > > >
> > > > > With output/dac buffer support the semantics of the
> > scan_element
> > > > type may change.
> > > > >
> > > > > Today the Format is
> > [be|le]:[s|u]bits/storagebitsXrepeat[>>shift].
> > > > >
> > > > > While shift (if specified) is the shift that needs to be applied
> prior
> > to
> > > > masking out unused bits.
> > > > >
> > > > > So far so good and it sounds universal.
> > > > >
> > > > > However, we use the right shift (operator) for that, which
> makes
> > > > sense for capture devices.
> > > > > For output devices the more logical operator would be the left
> > shift.
> > > > >
> > > > > I'm not proposing a new Format here. I just want to get some
> > > > agreement that for an output device
> > > > >
> > > > > le:s12/16>>4
> > > > >
> > > > > is understood as a left shift of 4, since the unused bits are then
> > on
> > > > the LSB.
> > > >
> > > > Good question. Guess I wasn't thinking ahead when I came up
> with
> > > > that :)
> > > >
> > > > I'm not sure I'd mind if we did decide to define a new format for
> > > > output
> > > > buffers. Feels like it should be easy to do.
> > > >
> > > > What do others think?
> > > >
> > >
> > > I guess the most straight forward thing would be just to add a
> 'shift_l'
> > variable
> > > to 'struct scan_type'' and make sure either 'shift_l' or 'shift' is
> defined
> > and then
> > > properly export either ">>" or "<<" to userspace?
> >
> > Given we already know it's an output channel, can we not just use
> that
> > to make the decision?
> >
> > Jonathan
> 
> I would argue that having two variables gives us more flexibility for
> whatever
> the future brings us :). But if we can sanely say that an output buffer
> will
> always use left shifts, then we could definitely use that information... I
> mean,
> we are already doing that assumption for input buffers and right
> shifts...

Hmm, giving it a bit more thought I think you can disregard the above.
Using the information that it's an output channel should be more than
enough...

- Nuno Sá