[PATCH][RFC] iio: buffer: remove 'scan_el_attrs' attribute group from buffer struct

[PATCH][RFC] iio: buffer: remove 'scan_el_attrs' attribute group from buffer struct

[Alexandru Ardelean]

This field doesn't seem used. It seems that only 'buffer->attrs' was ever
used to extend sysfs attributes for an IIO buffer.

Moving forward, it may not make sense to keep it. This patch removes the
field and it's initialization code.

Since we want to rework IIO buffer, to be able to add more buffers per IIO
device, we will merge [somehow] the 'buffer' & 'scan_elements' groups, and
we will continue to add the attributes to the 'buffer' group.

Removing it here, will also make the rework here a bit smaller, since
this code will not be present.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

I have no idea whether removing this is acceptable or not.
If it is acceptable, then this can be applied.
I am a bit vague on how this was ever used, of if it was ever used, but
it looks like dead code.


 drivers/iio/industrialio-buffer.c | 8 --------
 include/linux/iio/buffer_impl.h   | 6 ------
 2 files changed, 14 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index e6fa1a4e135d..221157136af6 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -1283,11 +1283,6 @@ int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
 
 	indio_dev->groups[indio_dev->groupcounter++] = &buffer->buffer_group;
 
-	if (buffer->scan_el_attrs != NULL) {
-		attr = buffer->scan_el_attrs->attrs;
-		while (*attr++ != NULL)
-			attrcount_orig++;
-	}
 	attrcount = attrcount_orig;
 	INIT_LIST_HEAD(&buffer->scan_el_dev_attr_list);
 	channels = indio_dev->channels;
@@ -1325,9 +1320,6 @@ int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
 		ret = -ENOMEM;
 		goto error_free_scan_mask;
 	}
-	if (buffer->scan_el_attrs)
-		memcpy(buffer->scan_el_group.attrs, buffer->scan_el_attrs,
-		       sizeof(buffer->scan_el_group.attrs[0])*attrcount_orig);
 	attrn = attrcount_orig;
 
 	list_for_each_entry(p, &buffer->scan_el_dev_attr_list, l)
diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h
index 1e7edf6bed96..a63dc07b7350 100644
--- a/include/linux/iio/buffer_impl.h
+++ b/include/linux/iio/buffer_impl.h
@@ -94,12 +94,6 @@ struct iio_buffer {
 	unsigned int watermark;
 
 	/* private: */
-	/*
-	 * @scan_el_attrs: Control of scan elements if that scan mode
-	 * control method is used.
-	 */
-	struct attribute_group *scan_el_attrs;
-
 	/* @scan_timestamp: Does the scan mode include a timestamp. */
 	bool scan_timestamp;
 
-- 
2.17.1

Re: [PATCH][RFC] iio: buffer: remove 'scan_el_attrs' attribute group from buffer struct

[Jonathan Cameron]

On Fri, 10 Apr 2020 12:36:07 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> This field doesn't seem used. It seems that only 'buffer->attrs' was ever
> used to extend sysfs attributes for an IIO buffer.
> 
> Moving forward, it may not make sense to keep it. This patch removes the
> field and it's initialization code.
> 
> Since we want to rework IIO buffer, to be able to add more buffers per IIO
> device, we will merge [somehow] the 'buffer' & 'scan_elements' groups, and
> we will continue to add the attributes to the 'buffer' group.
> 
> Removing it here, will also make the rework here a bit smaller, since
> this code will not be present.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Looks like dead code to me.  Applied to the togreg branch of iio.git and
pushed out as testing for the autobuilders to play with it.

Thanks,

Jonathan

> ---
> 
> I have no idea whether removing this is acceptable or not.
> If it is acceptable, then this can be applied.
> I am a bit vague on how this was ever used, of if it was ever used, but
> it looks like dead code.
> 
> 
>  drivers/iio/industrialio-buffer.c | 8 --------
>  include/linux/iio/buffer_impl.h   | 6 ------
>  2 files changed, 14 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index e6fa1a4e135d..221157136af6 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -1283,11 +1283,6 @@ int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
>  
>  	indio_dev->groups[indio_dev->groupcounter++] = &buffer->buffer_group;
>  
> -	if (buffer->scan_el_attrs != NULL) {
> -		attr = buffer->scan_el_attrs->attrs;
> -		while (*attr++ != NULL)
> -			attrcount_orig++;
> -	}
>  	attrcount = attrcount_orig;
>  	INIT_LIST_HEAD(&buffer->scan_el_dev_attr_list);
>  	channels = indio_dev->channels;
> @@ -1325,9 +1320,6 @@ int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
>  		ret = -ENOMEM;
>  		goto error_free_scan_mask;
>  	}
> -	if (buffer->scan_el_attrs)
> -		memcpy(buffer->scan_el_group.attrs, buffer->scan_el_attrs,
> -		       sizeof(buffer->scan_el_group.attrs[0])*attrcount_orig);
>  	attrn = attrcount_orig;
>  
>  	list_for_each_entry(p, &buffer->scan_el_dev_attr_list, l)
> diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h
> index 1e7edf6bed96..a63dc07b7350 100644
> --- a/include/linux/iio/buffer_impl.h
> +++ b/include/linux/iio/buffer_impl.h
> @@ -94,12 +94,6 @@ struct iio_buffer {
>  	unsigned int watermark;
>  
>  	/* private: */
> -	/*
> -	 * @scan_el_attrs: Control of scan elements if that scan mode
> -	 * control method is used.
> -	 */
> -	struct attribute_group *scan_el_attrs;
> -
>  	/* @scan_timestamp: Does the scan mode include a timestamp. */
>  	bool scan_timestamp;
>