On 26/11/14 17:55, Lars-Peter Clausen wrote:
> Originally device and buffer registration were kept as separate operations
> in IIO to allow to register two distinct sets of channels for buffered and
> non-buffered operations. This has since already been further restricted and
> the channel set registered for the buffer needs to be a subset of the
> channel set registered for the device.
The only case I can think of that might make us effectively separate
these out again is that of multiple buffers. However we don't support
that right now so I'll take this and if I get time start a conversation
on what the requirements for that will be. There are ways to move
that support into the core that may well make more sense anyway.
> Additionally the possibility to not
> have a raw (or processed) attribute for a channel which was registered for
> the device was added a while ago. This means it is possible to not register
> any device level attributes for a channel even if it is registered for the
> device. Also if a channel's scan_index is set to -1 and the channel is
> registered for the buffer it is ignored.
>
> So in summery it means it is possible to register the same channel array for
> both the device and the buffer yet still end up with distinctive sets of
> channels for both of them. This makes the argument for having to have to
> manually register the channels for both the device and the buffer invalid.
> Considering that the vast majority of all drivers want to register the same
> set of channels for both the buffer and the device it makes sense to move
> the buffer registration into the core to avoid some boiler-plate code in the
> device driver setup path.
>
> Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> ---
> drivers/iio/adc/ti_am335x_adc.c | 9 ---------
> drivers/iio/iio_core.h | 9 +++++++++
> drivers/iio/industrialio-buffer.c | 18 ++++++++++-------
> drivers/iio/industrialio-core.c | 14 ++++++++++++-
> drivers/iio/industrialio-triggered-buffer.c | 9 ---------
> drivers/staging/iio/accel/lis3l02dq_core.c | 13 +------------
> drivers/staging/iio/accel/sca3000_core.c | 10 +---------
> drivers/staging/iio/iio_simple_dummy_buffer.c | 8 --------
> drivers/staging/iio/impedance-analyzer/ad5933.c | 12 ++----------
> drivers/staging/iio/meter/ade7758.h | 1 -
> drivers/staging/iio/meter/ade7758_core.c | 15 ++------------
> drivers/staging/iio/meter/ade7758_ring.c | 5 -----
> include/linux/iio/buffer.h | 26 -------------------------
> 13 files changed, 39 insertions(+), 110 deletions(-)
>
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index f667e4e..e1906ad 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -385,14 +385,16 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev,
>
> static const char * const iio_scan_elements_group_name = "scan_elements";
>
> -int iio_buffer_register(struct iio_dev *indio_dev,
> - const struct iio_chan_spec *channels,
> - int num_channels)
> +int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
I've renamed this slightly as it also allocates the the bitmap (confusingly
called mask) of what channels are enabled for the buffer.
iio_buffer_alloc_sysfs_and_mask
Obviously renamed the free in a similar fashion. Doubt you'll mind, but
shout if you do.
> {
> struct iio_dev_attr *p;
> struct attribute **attr;
> struct iio_buffer *buffer = indio_dev->buffer;
> int ret, i, attrn, attrcount, attrcount_orig = 0;
> + const struct iio_chan_spec *channels;
> +
> + if (!buffer)
> + return 0;
>
> if (buffer->attrs)
> indio_dev->groups[indio_dev->groupcounter++] = buffer->attrs;
> @@ -404,9 +406,10 @@ int iio_buffer_register(struct iio_dev *indio_dev,
> }
> attrcount = attrcount_orig;
> INIT_LIST_HEAD(&buffer->scan_el_dev_attr_list);
> + channels = indio_dev->channels;
> if (channels) {
> /* new magic */
> - for (i = 0; i < num_channels; i++) {
> + for (i = 0; i < indio_dev->num_channels; i++) {
> if (channels[i].scan_index < 0)
> continue;
>
> @@ -463,15 +466,16 @@ error_cleanup_dynamic:
>
> return ret;
> }
> -EXPORT_SYMBOL(iio_buffer_register);
>
> -void iio_buffer_unregister(struct iio_dev *indio_dev)
> +void iio_buffer_free_sysfs(struct iio_dev *indio_dev)
> {
> + if (!indio_dev->buffer)
> + return;
> +
> kfree(indio_dev->buffer->scan_mask);
> kfree(indio_dev->buffer->scan_el_group.attrs);
> iio_free_chan_devattr_list(&indio_dev->buffer->scan_el_dev_attr_list);
> }
> -EXPORT_SYMBOL(iio_buffer_unregister);
>
> ssize_t iio_buffer_read_length(struct device *dev,
> struct device_attribute *attr,
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 45bb3a4..0e596b4 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1158,11 +1158,19 @@ int iio_device_register(struct iio_dev *indio_dev)
> "Failed to register debugfs interfaces\n");
> return ret;
> }
> +
> + ret = iio_buffer_alloc_sysfs(indio_dev);
> + if (ret) {
> + dev_err(indio_dev->dev.parent,
> + "Failed to create buffer sysfs interfaces\n");
> + goto error_unreg_debugfs;
> + }
> +
> ret = iio_device_register_sysfs(indio_dev);
> if (ret) {
> dev_err(indio_dev->dev.parent,
> "Failed to register sysfs interfaces\n");
> - goto error_unreg_debugfs;
> + goto error_buffer_free_sysfs;
> }
> ret = iio_device_register_eventset(indio_dev);
> if (ret) {
> @@ -1195,6 +1203,8 @@ error_unreg_eventset:
> iio_device_unregister_eventset(indio_dev);
> error_free_sysfs:
> iio_device_unregister_sysfs(indio_dev);
> +error_buffer_free_sysfs:
> + iio_buffer_free_sysfs(indio_dev);
> error_unreg_debugfs:
> iio_device_unregister_debugfs(indio_dev);
> return ret;
> @@ -1223,6 +1233,8 @@ void iio_device_unregister(struct iio_dev *indio_dev)
> iio_buffer_wakeup_poll(indio_dev);
>
> mutex_unlock(&indio_dev->info_exist_lock);
> +
> + iio_buffer_free_sysfs(indio_dev);
> }
> EXPORT_SYMBOL(iio_device_unregister);
>
> diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
> index 9eedb93..ba6d5ee 100644
> --- a/drivers/staging/iio/accel/sca3000_core.c
> +++ b/drivers/staging/iio/accel/sca3000_core.c
> @@ -1155,11 +1155,6 @@ static int sca3000_probe(struct spi_device *spi)
> if (ret < 0)
> return ret;
>
> - ret = iio_buffer_register(indio_dev, indio_dev->channels,
> - indio_dev->num_channels);
> - if (ret < 0)
> - goto error_unregister_dev;
> -
This caused some fuzz due to my reformatting of the earlier patch :(
Anyhow, the fix was trivial obviously.
> if (spi->irq) {
> ret = request_threaded_irq(spi->irq,
> NULL,
> @@ -1168,7 +1163,7 @@ static int sca3000_probe(struct spi_device *spi)
> "sca3000",
> indio_dev);
> if (ret)
> - goto error_unregister_ring;
> + goto error_unregister_dev;
> }
> sca3000_register_ring_funcs(indio_dev);
> ret = sca3000_clean_setup(st);
> @@ -1179,8 +1174,6 @@ static int sca3000_probe(struct spi_device *spi)
> error_free_irq:
> if (spi->irq)
> free_irq(spi->irq, indio_dev);
> -error_unregister_ring:
> - iio_buffer_unregister(indio_dev);
> error_unregister_dev:
> iio_device_unregister(indio_dev);
> return ret;
> @@ -1214,7 +1207,6 @@ static int sca3000_remove(struct spi_device *spi)
> if (spi->irq)
> free_irq(spi->irq, indio_dev);
> iio_device_unregister(indio_dev);
> - iio_buffer_unregister(indio_dev);
> sca3000_unconfigure_ring(indio_dev);
>
> return 0;
<snip>
> diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
> index 8c8ce61..b0e006c 100644
> --- a/include/linux/iio/buffer.h
> +++ b/include/linux/iio/buffer.h
> @@ -151,22 +151,6 @@ static inline int iio_push_to_buffers_with_timestamp(struct iio_dev *indio_dev,
> int iio_update_demux(struct iio_dev *indio_dev);
>
> /**
> - * iio_buffer_register() - register the buffer with IIO core
> - * @indio_dev: device with the buffer to be registered
> - * @channels: the channel descriptions used to construct buffer
> - * @num_channels: the number of channels
> - **/
> -int iio_buffer_register(struct iio_dev *indio_dev,
> - const struct iio_chan_spec *channels,
> - int num_channels);
> -
> -/**
> - * iio_buffer_unregister() - unregister the buffer from IIO core
> - * @indio_dev: the device with the buffer to be unregistered
> - **/
> -void iio_buffer_unregister(struct iio_dev *indio_dev);
> -
> -/**
> * iio_buffer_read_length() - attr func to get number of datums in the buffer
> **/
> ssize_t iio_buffer_read_length(struct device *dev,
> @@ -223,16 +207,6 @@ static inline void iio_device_attach_buffer(struct iio_dev *indio_dev,
>
> #else /* CONFIG_IIO_BUFFER */
>
> -static inline int iio_buffer_register(struct iio_dev *indio_dev,
> - const struct iio_chan_spec *channels,
> - int num_channels)
> -{
> - return 0;
> -}
> -
> -static inline void iio_buffer_unregister(struct iio_dev *indio_dev)
> -{}
> -
> static inline void iio_buffer_get(struct iio_buffer *buffer) {}
> static inline void iio_buffer_put(struct iio_buffer *buffer) {}
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
