From d9a625744ed0e452f5c495cd8c51eed4b6623a4c Mon Sep 17 00:00:00 2001 From: Alexandru Ardelean Date: Mon, 15 Feb 2021 12:40:31 +0200 Subject: iio: core: merge buffer/ & scan_elements/ attributes With this change, we create a new directory for the IIO device called buffer0, under which both the old buffer/ and scan_elements/ are stored. This is done to simplify the addition of multiple IIO buffers per IIO device. Otherwise we would need to add a bufferX/ and scan_elementsX/ directory for each IIO buffer. With the current way of storing attribute groups, we can't have directories stored under each other (i.e. scan_elements/ under buffer/), so the best approach moving forward is to merge their attributes. The old/legacy buffer/ & scan_elements/ groups are not stored on the opaque IIO device object. This way the IIO buffer can have just a single attribute_group object, saving a bit of memory when adding multiple IIO buffers. Signed-off-by: Alexandru Ardelean Link: https://lore.kernel.org/r/20210215104043.91251-13-alexandru.ardelean@analog.com Signed-off-by: Jonathan Cameron --- include/linux/iio/buffer_impl.h | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'include/linux/iio/buffer_impl.h') diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h index a63dc07b7350..3e555e58475b 100644 --- a/include/linux/iio/buffer_impl.h +++ b/include/linux/iio/buffer_impl.h @@ -100,14 +100,11 @@ struct iio_buffer { /* @scan_el_dev_attr_list: List of scan element related attributes. */ struct list_head scan_el_dev_attr_list; - /* @buffer_group: Attributes of the buffer group. */ - struct attribute_group buffer_group; - /* - * @scan_el_group: Attribute group for those attributes not - * created from the iio_chan_info array. + * @buffer_group: Attributes of the new buffer group. + * Includes scan elements attributes. */ - struct attribute_group scan_el_group; + struct attribute_group buffer_group; /* @attrs: Standard attributes of the buffer. */ const struct attribute **attrs; -- cgit v1.2.3-70-g09d2 From 15097c7a1adc0554ce8eb6f5fd6758d063bfea44 Mon Sep 17 00:00:00 2001 From: Alexandru Ardelean Date: Mon, 15 Feb 2021 12:40:33 +0200 Subject: iio: buffer: wrap all buffer attributes into iio_dev_attr This change wraps all buffer attributes into iio_dev_attr objects, and assigns a reference to the IIO buffer they belong to. With the addition of multiple IIO buffers per one IIO device, we need a way to know which IIO buffer is being enabled/disabled/controlled. We know that all buffer attributes are device_attributes. So we can wrap them with a iio_dev_attr types. In the iio_dev_attr type, we can also hold a reference to an IIO buffer. So, we end up being able to allocate wrapped attributes for all buffer attributes (even the one from other drivers). The neat part with this mechanism, is that we don't need to add any extra cleanup, because these attributes are being added to a dynamic list that will get cleaned up via iio_free_chan_devattr_list(). With this change, the 'buffer->scan_el_dev_attr_list' list is being renamed to 'buffer->buffer_attr_list', effectively merging (or finalizing the merge) of the buffer/ & scan_elements/ attributes internally. Accessing these new buffer attributes can now be done via 'to_iio_dev_attr(attr)->buffer' inside the show/store handlers. Signed-off-by: Alexandru Ardelean Link: https://lore.kernel.org/r/20210215104043.91251-15-alexandru.ardelean@analog.com Signed-off-by: Jonathan Cameron --- drivers/iio/industrialio-buffer.c | 77 ++++++++++++++++++++++++++------------- drivers/iio/industrialio-core.c | 2 +- include/linux/iio/buffer_impl.h | 4 +- 3 files changed, 54 insertions(+), 29 deletions(-) (limited to 'include/linux/iio/buffer_impl.h') diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c index e6edec3bcb73..ea91f9578dee 100644 --- a/drivers/iio/industrialio-buffer.c +++ b/drivers/iio/industrialio-buffer.c @@ -253,8 +253,7 @@ static ssize_t iio_scan_el_show(struct device *dev, char *buf) { int ret; - struct iio_dev *indio_dev = dev_to_iio_dev(dev); - struct iio_buffer *buffer = indio_dev->buffer; + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; /* Ensure ret is 0 or 1. */ ret = !!test_bit(to_iio_dev_attr(attr)->address, @@ -367,8 +366,8 @@ static ssize_t iio_scan_el_store(struct device *dev, int ret; bool state; struct iio_dev *indio_dev = dev_to_iio_dev(dev); - struct iio_buffer *buffer = indio_dev->buffer; struct iio_dev_attr *this_attr = to_iio_dev_attr(attr); + struct iio_buffer *buffer = this_attr->buffer; ret = strtobool(buf, &state); if (ret < 0) @@ -402,8 +401,7 @@ static ssize_t iio_scan_el_ts_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct iio_dev *indio_dev = dev_to_iio_dev(dev); - struct iio_buffer *buffer = indio_dev->buffer; + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; return sprintf(buf, "%d\n", buffer->scan_timestamp); } @@ -415,7 +413,7 @@ static ssize_t iio_scan_el_ts_store(struct device *dev, { int ret; struct iio_dev *indio_dev = dev_to_iio_dev(dev); - struct iio_buffer *buffer = indio_dev->buffer; + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; bool state; ret = strtobool(buf, &state); @@ -448,7 +446,7 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev, IIO_SEPARATE, &indio_dev->dev, buffer, - &buffer->scan_el_dev_attr_list); + &buffer->buffer_attr_list); if (ret) return ret; attrcount++; @@ -460,7 +458,7 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev, 0, &indio_dev->dev, buffer, - &buffer->scan_el_dev_attr_list); + &buffer->buffer_attr_list); if (ret) return ret; attrcount++; @@ -473,7 +471,7 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev, 0, &indio_dev->dev, buffer, - &buffer->scan_el_dev_attr_list); + &buffer->buffer_attr_list); else ret = __iio_add_chan_devattr("en", chan, @@ -483,7 +481,7 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev, 0, &indio_dev->dev, buffer, - &buffer->scan_el_dev_attr_list); + &buffer->buffer_attr_list); if (ret) return ret; attrcount++; @@ -495,8 +493,7 @@ static ssize_t iio_buffer_read_length(struct device *dev, struct device_attribute *attr, char *buf) { - struct iio_dev *indio_dev = dev_to_iio_dev(dev); - struct iio_buffer *buffer = indio_dev->buffer; + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; return sprintf(buf, "%d\n", buffer->length); } @@ -506,7 +503,7 @@ static ssize_t iio_buffer_write_length(struct device *dev, const char *buf, size_t len) { struct iio_dev *indio_dev = dev_to_iio_dev(dev); - struct iio_buffer *buffer = indio_dev->buffer; + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; unsigned int val; int ret; @@ -538,8 +535,7 @@ static ssize_t iio_buffer_show_enable(struct device *dev, struct device_attribute *attr, char *buf) { - struct iio_dev *indio_dev = dev_to_iio_dev(dev); - struct iio_buffer *buffer = indio_dev->buffer; + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; return sprintf(buf, "%d\n", iio_buffer_is_active(buffer)); } @@ -1154,7 +1150,7 @@ static ssize_t iio_buffer_store_enable(struct device *dev, int ret; bool requested_state; struct iio_dev *indio_dev = dev_to_iio_dev(dev); - struct iio_buffer *buffer = indio_dev->buffer; + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; bool inlist; ret = strtobool(buf, &requested_state); @@ -1183,8 +1179,7 @@ static ssize_t iio_buffer_show_watermark(struct device *dev, struct device_attribute *attr, char *buf) { - struct iio_dev *indio_dev = dev_to_iio_dev(dev); - struct iio_buffer *buffer = indio_dev->buffer; + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; return sprintf(buf, "%u\n", buffer->watermark); } @@ -1195,7 +1190,7 @@ static ssize_t iio_buffer_store_watermark(struct device *dev, size_t len) { struct iio_dev *indio_dev = dev_to_iio_dev(dev); - struct iio_buffer *buffer = indio_dev->buffer; + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; unsigned int val; int ret; @@ -1228,8 +1223,7 @@ static ssize_t iio_dma_show_data_available(struct device *dev, struct device_attribute *attr, char *buf) { - struct iio_dev *indio_dev = dev_to_iio_dev(dev); - struct iio_buffer *buffer = indio_dev->buffer; + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; return sprintf(buf, "%zu\n", iio_buffer_data_available(buffer)); } @@ -1254,6 +1248,27 @@ static struct attribute *iio_buffer_attrs[] = { &dev_attr_data_available.attr, }; +#define to_dev_attr(_attr) container_of(_attr, struct device_attribute, attr) + +static struct attribute *iio_buffer_wrap_attr(struct iio_buffer *buffer, + struct attribute *attr) +{ + struct device_attribute *dattr = to_dev_attr(attr); + struct iio_dev_attr *iio_attr; + + iio_attr = kzalloc(sizeof(*iio_attr), GFP_KERNEL); + if (!iio_attr) + return NULL; + + iio_attr->buffer = buffer; + memcpy(&iio_attr->dev_attr, dattr, sizeof(iio_attr->dev_attr)); + iio_attr->dev_attr.attr.name = kstrdup_const(attr->name, GFP_KERNEL); + + list_add(&iio_attr->l, &buffer->buffer_attr_list); + + return &iio_attr->dev_attr.attr; +} + static int iio_buffer_register_legacy_sysfs_groups(struct iio_dev *indio_dev, struct attribute **buffer_attrs, int buffer_attrcount, @@ -1329,7 +1344,7 @@ static int __iio_buffer_alloc_sysfs_and_mask(struct iio_buffer *buffer, } scan_el_attrcount = 0; - INIT_LIST_HEAD(&buffer->scan_el_dev_attr_list); + INIT_LIST_HEAD(&buffer->buffer_attr_list); channels = indio_dev->channels; if (channels) { /* new magic */ @@ -1376,9 +1391,19 @@ static int __iio_buffer_alloc_sysfs_and_mask(struct iio_buffer *buffer, buffer_attrcount += ARRAY_SIZE(iio_buffer_attrs); - attrn = buffer_attrcount; + for (i = 0; i < buffer_attrcount; i++) { + struct attribute *wrapped; + + wrapped = iio_buffer_wrap_attr(buffer, attr[i]); + if (!wrapped) { + ret = -ENOMEM; + goto error_free_scan_mask; + } + attr[i] = wrapped; + } - list_for_each_entry(p, &buffer->scan_el_dev_attr_list, l) + attrn = 0; + list_for_each_entry(p, &buffer->buffer_attr_list, l) attr[attrn++] = &p->dev_attr.attr; buffer->buffer_group.name = kasprintf(GFP_KERNEL, "buffer%d", index); @@ -1412,7 +1437,7 @@ error_free_buffer_attrs: error_free_scan_mask: bitmap_free(buffer->scan_mask); error_cleanup_dynamic: - iio_free_chan_devattr_list(&buffer->scan_el_dev_attr_list); + iio_free_chan_devattr_list(&buffer->buffer_attr_list); return ret; } @@ -1443,7 +1468,7 @@ static void __iio_buffer_free_sysfs_and_mask(struct iio_buffer *buffer) bitmap_free(buffer->scan_mask); kfree(buffer->buffer_group.name); kfree(buffer->buffer_group.attrs); - iio_free_chan_devattr_list(&buffer->scan_el_dev_attr_list); + iio_free_chan_devattr_list(&buffer->buffer_attr_list); } void iio_buffer_free_sysfs_and_mask(struct iio_dev *indio_dev) diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index f4e5a08bbcd5..b8b149ccecef 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -1357,7 +1357,7 @@ void iio_free_chan_devattr_list(struct list_head *attr_list) struct iio_dev_attr *p, *n; list_for_each_entry_safe(p, n, attr_list, l) { - kfree(p->dev_attr.attr.name); + kfree_const(p->dev_attr.attr.name); list_del(&p->l); kfree(p); } diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h index 3e555e58475b..41044320e581 100644 --- a/include/linux/iio/buffer_impl.h +++ b/include/linux/iio/buffer_impl.h @@ -97,8 +97,8 @@ struct iio_buffer { /* @scan_timestamp: Does the scan mode include a timestamp. */ bool scan_timestamp; - /* @scan_el_dev_attr_list: List of scan element related attributes. */ - struct list_head scan_el_dev_attr_list; + /* @buffer_attr_list: List of buffer attributes. */ + struct list_head buffer_attr_list; /* * @buffer_group: Attributes of the new buffer group. -- cgit v1.2.3-70-g09d2 From ee708e6baacd3afdace9b721c25fbbe106cebb94 Mon Sep 17 00:00:00 2001 From: Alexandru Ardelean Date: Mon, 15 Feb 2021 12:40:38 +0200 Subject: iio: buffer: introduce support for attaching more IIO buffers With this change, calling iio_device_attach_buffer() will actually attach more buffers. Right now this doesn't do any validation of whether a buffer is attached twice; maybe that can be added later (if needed). Attaching a buffer more than once should yield noticeably bad results. The first buffer is the legacy buffer, so a reference is kept to it. At this point, accessing the data for the extra buffers (that are added after the first one) isn't possible yet. The iio_device_attach_buffer() is also changed to return an error code, which for now is -ENOMEM if the array could not be realloc-ed for more buffers. To adapt to this new change iio_device_attach_buffer() is called last in all place where it's called. The realloc failure is a bit difficult to handle during un-managed calls when unwinding, so it's better to have this as the last error in the setup_buffer calls. At this point, no driver should call iio_device_attach_buffer() directly, it should call one of the {devm_}iio_triggered_buffer_setup() or devm_iio_kfifo_buffer_setup() or devm_iio_dmaengine_buffer_setup() functions. This makes iio_device_attach_buffer() a bit easier to handle. Signed-off-by: Alexandru Ardelean Link: https://lore.kernel.org/r/20210215104043.91251-20-alexandru.ardelean@analog.com Signed-off-by: Jonathan Cameron --- drivers/iio/buffer/industrialio-buffer-dmaengine.c | 4 +- drivers/iio/buffer/industrialio-triggered-buffer.c | 10 ++- drivers/iio/buffer/kfifo_buf.c | 4 +- drivers/iio/iio_core.h | 10 ++- drivers/iio/industrialio-buffer.c | 100 +++++++++++++++++---- drivers/iio/industrialio-core.c | 12 +-- include/linux/iio/buffer.h | 4 +- include/linux/iio/buffer_impl.h | 3 + include/linux/iio/iio-opaque.h | 4 + 9 files changed, 111 insertions(+), 40 deletions(-) (limited to 'include/linux/iio/buffer_impl.h') diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c index a64b222289be..d76179878ff9 100644 --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c @@ -290,9 +290,7 @@ int devm_iio_dmaengine_buffer_setup(struct device *dev, indio_dev->modes |= INDIO_BUFFER_HARDWARE; - iio_device_attach_buffer(indio_dev, buffer); - - return 0; + return iio_device_attach_buffer(indio_dev, buffer); } EXPORT_SYMBOL_GPL(devm_iio_dmaengine_buffer_setup); diff --git a/drivers/iio/buffer/industrialio-triggered-buffer.c b/drivers/iio/buffer/industrialio-triggered-buffer.c index 92b8aea3e063..b2b1b7d27af4 100644 --- a/drivers/iio/buffer/industrialio-triggered-buffer.c +++ b/drivers/iio/buffer/industrialio-triggered-buffer.c @@ -50,8 +50,6 @@ int iio_triggered_buffer_setup_ext(struct iio_dev *indio_dev, goto error_ret; } - iio_device_attach_buffer(indio_dev, buffer); - indio_dev->pollfunc = iio_alloc_pollfunc(h, thread, IRQF_ONESHOT, @@ -72,10 +70,16 @@ int iio_triggered_buffer_setup_ext(struct iio_dev *indio_dev, buffer->attrs = buffer_attrs; + ret = iio_device_attach_buffer(indio_dev, buffer); + if (ret < 0) + goto error_dealloc_pollfunc; + return 0; +error_dealloc_pollfunc: + iio_dealloc_pollfunc(indio_dev->pollfunc); error_kfifo_free: - iio_kfifo_free(indio_dev->buffer); + iio_kfifo_free(buffer); error_ret: return ret; } diff --git a/drivers/iio/buffer/kfifo_buf.c b/drivers/iio/buffer/kfifo_buf.c index c35a625280b1..34289ce12f20 100644 --- a/drivers/iio/buffer/kfifo_buf.c +++ b/drivers/iio/buffer/kfifo_buf.c @@ -235,12 +235,10 @@ int devm_iio_kfifo_buffer_setup(struct device *dev, if (!buffer) return -ENOMEM; - iio_device_attach_buffer(indio_dev, buffer); - indio_dev->modes |= mode_flags; indio_dev->setup_ops = setup_ops; - return 0; + return iio_device_attach_buffer(indio_dev, buffer); } EXPORT_SYMBOL_GPL(devm_iio_kfifo_buffer_setup); diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h index 87868fff7d37..7990c759f1f5 100644 --- a/drivers/iio/iio_core.h +++ b/drivers/iio/iio_core.h @@ -69,29 +69,31 @@ __poll_t iio_buffer_poll(struct file *filp, ssize_t iio_buffer_read_outer(struct file *filp, char __user *buf, size_t n, loff_t *f_ps); -int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev); -void iio_buffer_free_sysfs_and_mask(struct iio_dev *indio_dev); +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) #define iio_buffer_read_outer_addr (&iio_buffer_read_outer) void iio_disable_all_buffers(struct iio_dev *indio_dev); void iio_buffer_wakeup_poll(struct iio_dev *indio_dev); +void iio_buffers_put(struct iio_dev *indio_dev); #else #define iio_buffer_poll_addr NULL #define iio_buffer_read_outer_addr NULL -static inline int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev) +static inline int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev) { return 0; } -static inline void iio_buffer_free_sysfs_and_mask(struct iio_dev *indio_dev) {} +static inline void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev) {} static inline void iio_disable_all_buffers(struct iio_dev *indio_dev) {} static inline void iio_buffer_wakeup_poll(struct iio_dev *indio_dev) {} +static inline void iio_buffers_put(struct iio_dev *indio_dev) {} #endif diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c index 349c0c011745..b60c2e66bd1c 100644 --- a/drivers/iio/industrialio-buffer.c +++ b/drivers/iio/industrialio-buffer.c @@ -193,12 +193,14 @@ __poll_t iio_buffer_poll(struct file *filp, */ void iio_buffer_wakeup_poll(struct iio_dev *indio_dev) { - struct iio_buffer *buffer = indio_dev->buffer; - - if (!buffer) - return; + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); + struct iio_buffer *buffer; + unsigned int i; - wake_up(&buffer->pollq); + for (i = 0; i < iio_dev_opaque->attached_buffers_cnt; i++) { + buffer = iio_dev_opaque->attached_buffers[i]; + wake_up(&buffer->pollq); + } } void iio_buffer_init(struct iio_buffer *buffer) @@ -212,6 +214,18 @@ void iio_buffer_init(struct iio_buffer *buffer) } EXPORT_SYMBOL(iio_buffer_init); +void iio_buffers_put(struct iio_dev *indio_dev) +{ + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); + struct iio_buffer *buffer; + unsigned int i; + + for (i = 0; i < iio_dev_opaque->attached_buffers_cnt; i++) { + buffer = iio_dev_opaque->attached_buffers[i]; + iio_buffer_put(buffer); + } +} + static ssize_t iio_show_scan_index(struct device *dev, struct device_attribute *attr, char *buf) @@ -1452,11 +1466,13 @@ static void __iio_buffer_free_sysfs_and_mask(struct iio_buffer *buffer) iio_free_chan_devattr_list(&buffer->buffer_attr_list); } -int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev) +int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev) { - struct iio_buffer *buffer = indio_dev->buffer; + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); const struct iio_chan_spec *channels; - int i; + struct iio_buffer *buffer; + int unwind_idx; + int ret, i; channels = indio_dev->channels; if (channels) { @@ -1467,22 +1483,46 @@ int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev) indio_dev->masklength = ml; } - if (!buffer) + if (!iio_dev_opaque->attached_buffers_cnt) return 0; - return __iio_buffer_alloc_sysfs_and_mask(buffer, indio_dev, 0); + for (i = 0; i < iio_dev_opaque->attached_buffers_cnt; i++) { + buffer = iio_dev_opaque->attached_buffers[i]; + ret = __iio_buffer_alloc_sysfs_and_mask(buffer, indio_dev, i); + if (ret) { + unwind_idx = i; + goto error_unwind_sysfs_and_mask; + } + } + + return 0; + +error_unwind_sysfs_and_mask: + for (; unwind_idx >= 0; unwind_idx--) { + buffer = iio_dev_opaque->attached_buffers[unwind_idx]; + __iio_buffer_free_sysfs_and_mask(buffer); + } + kfree(iio_dev_opaque->attached_buffers); + return ret; } -void iio_buffer_free_sysfs_and_mask(struct iio_dev *indio_dev) +void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev) { - struct iio_buffer *buffer = indio_dev->buffer; + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); + struct iio_buffer *buffer; + int i; - if (!buffer) + if (!iio_dev_opaque->attached_buffers_cnt) return; iio_buffer_unregister_legacy_sysfs_groups(indio_dev); - __iio_buffer_free_sysfs_and_mask(buffer); + for (i = iio_dev_opaque->attached_buffers_cnt - 1; i >= 0; i--) { + buffer = iio_dev_opaque->attached_buffers[i]; + __iio_buffer_free_sysfs_and_mask(buffer); + } + + kfree(iio_dev_opaque->attached_buffers); } /** @@ -1600,13 +1640,35 @@ EXPORT_SYMBOL_GPL(iio_buffer_put); * @indio_dev: The device the buffer should be attached to * @buffer: The buffer to attach to the device * + * Return 0 if successful, negative if error. + * * This function attaches a buffer to a IIO device. The buffer stays attached to - * the device until the device is freed. The function should only be called at - * most once per device. + * the device until the device is freed. For legacy reasons, the first attached + * buffer will also be assigned to 'indio_dev->buffer'. */ -void iio_device_attach_buffer(struct iio_dev *indio_dev, - struct iio_buffer *buffer) +int iio_device_attach_buffer(struct iio_dev *indio_dev, + struct iio_buffer *buffer) { - indio_dev->buffer = iio_buffer_get(buffer); + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); + struct iio_buffer **new, **old = iio_dev_opaque->attached_buffers; + unsigned int cnt = iio_dev_opaque->attached_buffers_cnt; + + cnt++; + + new = krealloc(old, sizeof(*new) * cnt, GFP_KERNEL); + if (!new) + return -ENOMEM; + iio_dev_opaque->attached_buffers = new; + + buffer = iio_buffer_get(buffer); + + /* first buffer is legacy; attach it to the IIO device directly */ + if (!indio_dev->buffer) + indio_dev->buffer = buffer; + + iio_dev_opaque->attached_buffers[cnt - 1] = buffer; + iio_dev_opaque->attached_buffers_cnt = cnt; + + return 0; } EXPORT_SYMBOL_GPL(iio_device_attach_buffer); diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index b96acfe13e92..1645c739c44f 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -1585,7 +1585,7 @@ static void iio_dev_release(struct device *device) iio_device_unregister_eventset(indio_dev); iio_device_unregister_sysfs(indio_dev); - iio_buffer_put(indio_dev->buffer); + iio_buffers_put(indio_dev); ida_simple_remove(&iio_ida, indio_dev->id); kfree(iio_dev_opaque); @@ -1862,7 +1862,7 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod) iio_device_register_debugfs(indio_dev); - ret = iio_buffer_alloc_sysfs_and_mask(indio_dev); + ret = iio_buffers_alloc_sysfs_and_mask(indio_dev); if (ret) { dev_err(indio_dev->dev.parent, "Failed to create buffer sysfs interfaces\n"); @@ -1888,12 +1888,12 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod) indio_dev->setup_ops == NULL) indio_dev->setup_ops = &noop_ring_setup_ops; - if (indio_dev->buffer) + if (iio_dev_opaque->attached_buffers_cnt) cdev_init(&indio_dev->chrdev, &iio_buffer_fileops); else if (iio_dev_opaque->event_interface) cdev_init(&indio_dev->chrdev, &iio_event_fileops); - if (indio_dev->buffer || iio_dev_opaque->event_interface) { + if (iio_dev_opaque->attached_buffers_cnt || iio_dev_opaque->event_interface) { indio_dev->dev.devt = MKDEV(MAJOR(iio_devt), indio_dev->id); indio_dev->chrdev.owner = this_mod; } @@ -1912,7 +1912,7 @@ error_unreg_eventset: error_free_sysfs: iio_device_unregister_sysfs(indio_dev); error_buffer_free_sysfs: - iio_buffer_free_sysfs_and_mask(indio_dev); + iio_buffers_free_sysfs_and_mask(indio_dev); error_unreg_debugfs: iio_device_unregister_debugfs(indio_dev); return ret; @@ -1946,7 +1946,7 @@ void iio_device_unregister(struct iio_dev *indio_dev) mutex_unlock(&indio_dev->info_exist_lock); - iio_buffer_free_sysfs_and_mask(indio_dev); + iio_buffers_free_sysfs_and_mask(indio_dev); } EXPORT_SYMBOL(iio_device_unregister); diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h index 8febc23f5f26..b6928ac5c63d 100644 --- a/include/linux/iio/buffer.h +++ b/include/linux/iio/buffer.h @@ -41,7 +41,7 @@ static inline int iio_push_to_buffers_with_timestamp(struct iio_dev *indio_dev, bool iio_validate_scan_mask_onehot(struct iio_dev *indio_dev, const unsigned long *mask); -void iio_device_attach_buffer(struct iio_dev *indio_dev, - struct iio_buffer *buffer); +int iio_device_attach_buffer(struct iio_dev *indio_dev, + struct iio_buffer *buffer); #endif /* _IIO_BUFFER_GENERIC_H_ */ diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h index 41044320e581..768b90c64412 100644 --- a/include/linux/iio/buffer_impl.h +++ b/include/linux/iio/buffer_impl.h @@ -112,6 +112,9 @@ struct iio_buffer { /* @demux_bounce: Buffer for doing gather from incoming scan. */ void *demux_bounce; + /* @attached_entry: Entry in the devices list of buffers attached by the driver. */ + struct list_head attached_entry; + /* @buffer_list: Entry in the devices list of current buffers. */ struct list_head buffer_list; diff --git a/include/linux/iio/iio-opaque.h b/include/linux/iio/iio-opaque.h index 1a9310b0145f..b6ebc04af3e7 100644 --- a/include/linux/iio/iio-opaque.h +++ b/include/linux/iio/iio-opaque.h @@ -7,6 +7,8 @@ * struct iio_dev_opaque - industrial I/O device opaque information * @indio_dev: public industrial I/O device information * @event_interface: event chrdevs associated with interrupt lines + * @attached_buffers: array of buffers statically attached by the driver + * @attached_buffers_cnt: number of buffers in the array of statically attached buffers * @buffer_list: list of all buffers currently attached * @channel_attr_list: keep track of automatically created channel * attributes @@ -24,6 +26,8 @@ struct iio_dev_opaque { struct iio_dev indio_dev; struct iio_event_interface *event_interface; + struct iio_buffer **attached_buffers; + unsigned int attached_buffers_cnt; struct list_head buffer_list; struct list_head channel_attr_list; struct attribute_group chan_attr_group; -- cgit v1.2.3-70-g09d2 From f73f7f4da581875f9b1f2fb8ebd1ab15ed634488 Mon Sep 17 00:00:00 2001 From: Alexandru Ardelean Date: Mon, 15 Feb 2021 12:40:39 +0200 Subject: iio: buffer: add ioctl() to support opening extra buffers for IIO device With this change, an ioctl() call is added to open a character device for a buffer. The ioctl() number is 'i' 0x91, which follows the IIO_GET_EVENT_FD_IOCTL ioctl. The ioctl() will return an FD for the requested buffer index. The indexes are the same from the /sys/iio/devices/iio:deviceX/bufferY (i.e. the Y variable). Since there doesn't seem to be a sane way to return the FD for buffer0 to be the same FD for the /dev/iio:deviceX, this ioctl() will return another FD for buffer0 (or the first buffer). This duplicate FD will be able to access the same buffer object (for buffer0) as accessing directly the /dev/iio:deviceX chardev. Also, there is no IIO_BUFFER_GET_BUFFER_COUNT ioctl() implemented, as the index for each buffer (and the count) can be deduced from the '/sys/bus/iio/devices/iio:deviceX/bufferY' folders (i.e the number of bufferY folders). Used following C code to test this: ------------------------------------------------------------------- #include #include #include #include #include #define IIO_BUFFER_GET_FD_IOCTL _IOWR('i', 0x91, int) int main(int argc, char *argv[]) { int fd; int fd1; int ret; if ((fd = open("/dev/iio:device0", O_RDWR))<0) { fprintf(stderr, "Error open() %d errno %d\n",fd, errno); return -1; } fprintf(stderr, "Using FD %d\n", fd); fd1 = atoi(argv[1]); ret = ioctl(fd, IIO_BUFFER_GET_FD_IOCTL, &fd1); if (ret < 0) { fprintf(stderr, "Error for buffer %d ioctl() %d errno %d\n", fd1, ret, errno); close(fd); return -1; } fprintf(stderr, "Got FD %d\n", fd1); close(fd1); close(fd); return 0; } ------------------------------------------------------------------- Results are: ------------------------------------------------------------------- # ./test 0 Using FD 3 Got FD 4 # ./test 1 Using FD 3 Got FD 4 # ./test 2 Using FD 3 Got FD 4 # ./test 3 Using FD 3 Got FD 4 # ls /sys/bus/iio/devices/iio\:device0 buffer buffer0 buffer1 buffer2 buffer3 dev in_voltage_sampling_frequency in_voltage_scale in_voltage_scale_available name of_node power scan_elements subsystem uevent ------------------------------------------------------------------- iio:device0 has some fake kfifo buffers attached to an IIO device. Signed-off-by: Alexandru Ardelean Link: https://lore.kernel.org/r/20210215104043.91251-21-alexandru.ardelean@analog.com Signed-off-by: Jonathan Cameron --- drivers/iio/iio_core.h | 12 ++-- drivers/iio/industrialio-buffer.c | 144 ++++++++++++++++++++++++++++++++++++-- include/linux/iio/buffer_impl.h | 5 ++ include/linux/iio/iio-opaque.h | 2 + include/uapi/linux/iio/buffer.h | 10 +++ 5 files changed, 162 insertions(+), 11 deletions(-) create mode 100644 include/uapi/linux/iio/buffer.h (limited to 'include/linux/iio/buffer_impl.h') diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h index 7990c759f1f5..062fe16c6c49 100644 --- a/drivers/iio/iio_core.h +++ b/drivers/iio/iio_core.h @@ -64,16 +64,16 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals); #ifdef CONFIG_IIO_BUFFER struct poll_table_struct; -__poll_t iio_buffer_poll(struct file *filp, - struct poll_table_struct *wait); -ssize_t iio_buffer_read_outer(struct file *filp, char __user *buf, - size_t n, loff_t *f_ps); +__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); 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) -#define iio_buffer_read_outer_addr (&iio_buffer_read_outer) +#define iio_buffer_poll_addr (&iio_buffer_poll_wrapper) +#define iio_buffer_read_outer_addr (&iio_buffer_read_wrapper) void iio_disable_all_buffers(struct iio_dev *indio_dev); void iio_buffer_wakeup_poll(struct iio_dev *indio_dev); diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c index b60c2e66bd1c..a48e494a9fbb 100644 --- a/drivers/iio/industrialio-buffer.c +++ b/drivers/iio/industrialio-buffer.c @@ -9,9 +9,11 @@ * - Better memory allocation techniques? * - Alternative access techniques? */ +#include #include #include #include +#include #include #include #include @@ -89,7 +91,7 @@ static bool iio_buffer_ready(struct iio_dev *indio_dev, struct iio_buffer *buf, } /** - * iio_buffer_read_outer() - chrdev read for buffer access + * iio_buffer_read() - chrdev read for buffer access * @filp: File structure pointer for the char device * @buf: Destination buffer for iio buffer read * @n: First n bytes to read @@ -101,8 +103,8 @@ static bool iio_buffer_ready(struct iio_dev *indio_dev, struct iio_buffer *buf, * Return: negative values corresponding to error codes or ret != 0 * for ending the reading activity **/ -ssize_t iio_buffer_read_outer(struct file *filp, char __user *buf, - size_t n, loff_t *f_ps) +static ssize_t iio_buffer_read(struct file *filp, 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; @@ -168,8 +170,8 @@ ssize_t iio_buffer_read_outer(struct file *filp, char __user *buf, * Return: (EPOLLIN | EPOLLRDNORM) if data is available for reading * or 0 for other cases */ -__poll_t iio_buffer_poll(struct file *filp, - struct poll_table_struct *wait) +static __poll_t iio_buffer_poll(struct file *filp, + struct poll_table_struct *wait) { struct iio_dev_buffer_pair *ib = filp->private_data; struct iio_buffer *rb = ib->buffer; @@ -184,6 +186,32 @@ __poll_t iio_buffer_poll(struct file *filp, return 0; } +ssize_t iio_buffer_read_wrapper(struct file *filp, 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_read(filp, buf, n, f_ps); +} + +__poll_t iio_buffer_poll_wrapper(struct file *filp, + struct poll_table_struct *wait) +{ + 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 0; + + return iio_buffer_poll(filp, wait); +} + /** * iio_buffer_wakeup_poll - Wakes up the buffer waitqueue * @indio_dev: The IIO device @@ -1344,6 +1372,96 @@ static void iio_buffer_unregister_legacy_sysfs_groups(struct iio_dev *indio_dev) kfree(iio_dev_opaque->legacy_scan_el_group.attrs); } +static int iio_buffer_chrdev_release(struct inode *inode, struct file *filep) +{ + struct iio_dev_buffer_pair *ib = filep->private_data; + struct iio_dev *indio_dev = ib->indio_dev; + struct iio_buffer *buffer = ib->buffer; + + wake_up(&buffer->pollq); + + kfree(ib); + clear_bit(IIO_BUSY_BIT_POS, &buffer->flags); + iio_device_put(indio_dev); + + return 0; +} + +static const struct file_operations iio_buffer_chrdev_fileops = { + .owner = THIS_MODULE, + .llseek = noop_llseek, + .read = iio_buffer_read, + .poll = iio_buffer_poll, + .release = iio_buffer_chrdev_release, +}; + +static long iio_device_buffer_getfd(struct iio_dev *indio_dev, unsigned long arg) +{ + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); + int __user *ival = (int __user *)arg; + struct iio_dev_buffer_pair *ib; + struct iio_buffer *buffer; + int fd, idx, ret; + + if (copy_from_user(&idx, ival, sizeof(idx))) + return -EFAULT; + + if (idx >= iio_dev_opaque->attached_buffers_cnt) + return -ENODEV; + + iio_device_get(indio_dev); + + buffer = iio_dev_opaque->attached_buffers[idx]; + + if (test_and_set_bit(IIO_BUSY_BIT_POS, &buffer->flags)) { + ret = -EBUSY; + goto error_iio_dev_put; + } + + ib = kzalloc(sizeof(*ib), GFP_KERNEL); + if (!ib) { + ret = -ENOMEM; + goto error_clear_busy_bit; + } + + ib->indio_dev = indio_dev; + ib->buffer = buffer; + + fd = anon_inode_getfd("iio:buffer", &iio_buffer_chrdev_fileops, + ib, O_RDWR | O_CLOEXEC); + if (fd < 0) { + ret = fd; + goto error_free_ib; + } + + if (copy_to_user(ival, &fd, sizeof(fd))) { + put_unused_fd(fd); + ret = -EFAULT; + goto error_free_ib; + } + + return fd; + +error_free_ib: + kfree(ib); +error_clear_busy_bit: + clear_bit(IIO_BUSY_BIT_POS, &buffer->flags); +error_iio_dev_put: + iio_device_put(indio_dev); + return ret; +} + +static long iio_device_buffer_ioctl(struct iio_dev *indio_dev, struct file *filp, + unsigned int cmd, unsigned long arg) +{ + switch (cmd) { + case IIO_BUFFER_GET_FD_IOCTL: + return iio_device_buffer_getfd(indio_dev, arg); + default: + return IIO_IOCTL_UNHANDLED; + } +} + static int __iio_buffer_alloc_sysfs_and_mask(struct iio_buffer *buffer, struct iio_dev *indio_dev, int index) @@ -1473,6 +1591,7 @@ int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev) struct iio_buffer *buffer; int unwind_idx; int ret, i; + size_t sz; channels = indio_dev->channels; if (channels) { @@ -1494,6 +1613,18 @@ int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev) goto error_unwind_sysfs_and_mask; } } + unwind_idx = iio_dev_opaque->attached_buffers_cnt - 1; + + sz = sizeof(*(iio_dev_opaque->buffer_ioctl_handler)); + iio_dev_opaque->buffer_ioctl_handler = kzalloc(sz, GFP_KERNEL); + if (!iio_dev_opaque->buffer_ioctl_handler) { + ret = -ENOMEM; + goto error_unwind_sysfs_and_mask; + } + + iio_dev_opaque->buffer_ioctl_handler->ioctl = iio_device_buffer_ioctl; + iio_device_ioctl_handler_register(indio_dev, + iio_dev_opaque->buffer_ioctl_handler); return 0; @@ -1515,6 +1646,9 @@ void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev) if (!iio_dev_opaque->attached_buffers_cnt) return; + iio_device_ioctl_handler_unregister(iio_dev_opaque->buffer_ioctl_handler); + kfree(iio_dev_opaque->buffer_ioctl_handler); + iio_buffer_unregister_legacy_sysfs_groups(indio_dev); for (i = iio_dev_opaque->attached_buffers_cnt - 1; i >= 0; i--) { diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h index 768b90c64412..245b32918ae1 100644 --- a/include/linux/iio/buffer_impl.h +++ b/include/linux/iio/buffer_impl.h @@ -6,6 +6,8 @@ #ifdef CONFIG_IIO_BUFFER +#include + struct iio_dev; struct iio_buffer; @@ -72,6 +74,9 @@ struct iio_buffer { /** @length: Number of datums in buffer. */ unsigned int length; + /** @flags: File ops flags including busy flag. */ + unsigned long flags; + /** @bytes_per_datum: Size of individual datum including timestamp. */ size_t bytes_per_datum; diff --git a/include/linux/iio/iio-opaque.h b/include/linux/iio/iio-opaque.h index b6ebc04af3e7..32addd5e790e 100644 --- a/include/linux/iio/iio-opaque.h +++ b/include/linux/iio/iio-opaque.h @@ -9,6 +9,7 @@ * @event_interface: event chrdevs associated with interrupt lines * @attached_buffers: array of buffers statically attached by the driver * @attached_buffers_cnt: number of buffers in the array of statically attached buffers + * @buffer_ioctl_handler: ioctl() handler for this IIO device's buffer interface * @buffer_list: list of all buffers currently attached * @channel_attr_list: keep track of automatically created channel * attributes @@ -28,6 +29,7 @@ struct iio_dev_opaque { struct iio_event_interface *event_interface; struct iio_buffer **attached_buffers; unsigned int attached_buffers_cnt; + struct iio_ioctl_handler *buffer_ioctl_handler; struct list_head buffer_list; struct list_head channel_attr_list; struct attribute_group chan_attr_group; diff --git a/include/uapi/linux/iio/buffer.h b/include/uapi/linux/iio/buffer.h new file mode 100644 index 000000000000..13939032b3f6 --- /dev/null +++ b/include/uapi/linux/iio/buffer.h @@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +/* industrial I/O buffer definitions needed both in and out of kernel + */ + +#ifndef _UAPI_IIO_BUFFER_H_ +#define _UAPI_IIO_BUFFER_H_ + +#define IIO_BUFFER_GET_FD_IOCTL _IOWR('i', 0x91, int) + +#endif /* _UAPI_IIO_BUFFER_H_ */ -- cgit v1.2.3-70-g09d2