On Thu, 2020-04-16 at 17:31 +0300, Alexandru Ardelean wrote:
> [External]
>
> The kernel provides a facility for attribute groups to specify the stat
> flags of a sysfs file with an 'is_visible()' hook. This mechanism is
> usually more flexible than assigning read-only attributes at construction
> time.
> It's added-value is a bit more apparent when the number of attributes
> grows, so for sysfs buffer attributes this change may not be that be useful
> quite now.
>
> It should become more useful as the sysfs structure for buffer attributes
> gets changed a bit.
Let's disregard this for now.
It may not be worth doing this, until a better context/reason appears.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
> ---
> drivers/iio/industrialio-buffer.c | 48 ++++++++++++++++++++++---------
> 1 file changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-
> buffer.c
> index 221157136af6..60bb03e72afc 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -1214,24 +1214,50 @@ static ssize_t iio_dma_show_data_available(struct
> device *dev,
> return sprintf(buf, "%zu\n", bytes);
> }
>
> +enum {
> + IIO_BUFFER_ATTR_LENGTH,
> + IIO_BUFFER_ATTR_ENABLE,
> + IIO_BUFFER_ATTR_WATERMARK,
> + IIO_BUFFER_ATTR_DATA_AVAILABLE,
> + __IIO_BUFFER_ATTR_MAX
> +};
> +
> +static umode_t iio_buffer_attr_group_is_visible(struct kobject *kobj,
> + struct attribute *attr,
> + int index)
> +{
> + struct device *dev = container_of(kobj, struct device, kobj);
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct iio_buffer *buffer = indio_dev->buffer;
> +
> + switch (index) {
> + case IIO_BUFFER_ATTR_LENGTH:
> + if (!buffer->access->set_length)
> + return S_IRUGO;
> + return attr->mode;
> + case IIO_BUFFER_ATTR_WATERMARK:
> + if (buffer->access->flags & INDIO_BUFFER_FLAG_FIXED_WATERMARK)
> + return S_IRUGO;
> + return attr->mode;
> + default:
> + return attr->mode;
> + }
> +}
> +
> 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,
> - S_IRUGO, iio_buffer_read_length, NULL);
> static DEVICE_ATTR(enable, S_IRUGO | S_IWUSR,
> iio_buffer_show_enable, iio_buffer_store_enable);
> static DEVICE_ATTR(watermark, S_IRUGO | S_IWUSR,
> iio_buffer_show_watermark, iio_buffer_store_watermark);
> -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 struct attribute *iio_buffer_attrs[] = {
> - &dev_attr_length.attr,
> - &dev_attr_enable.attr,
> - &dev_attr_watermark.attr,
> - &dev_attr_data_available.attr,
> + [IIO_BUFFER_ATTR_LENGTH] = &dev_attr_length.attr,
> + [IIO_BUFFER_ATTR_ENABLE] = &dev_attr_enable.attr,
> + [IIO_BUFFER_ATTR_WATERMARK] = &dev_attr_watermark.attr,
> + [IIO_BUFFER_ATTR_DATA_AVAILABLE] = &dev_attr_data_available.attr,
> };
>
> int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
> @@ -1266,11 +1292,6 @@ int iio_buffer_alloc_sysfs_and_mask(struct iio_dev
> *indio_dev)
> return -ENOMEM;
>
> memcpy(attr, iio_buffer_attrs, sizeof(iio_buffer_attrs));
> - if (!buffer->access->set_length)
> - attr[0] = &dev_attr_length_ro.attr;
> -
> - if (buffer->access->flags & INDIO_BUFFER_FLAG_FIXED_WATERMARK)
> - attr[2] = &dev_attr_watermark_ro.attr;
>
> if (buffer->attrs)
> memcpy(&attr[ARRAY_SIZE(iio_buffer_attrs)], buffer->attrs,
> @@ -1279,6 +1300,7 @@ int iio_buffer_alloc_sysfs_and_mask(struct iio_dev
> *indio_dev)
> attr[attrcount + ARRAY_SIZE(iio_buffer_attrs)] = NULL;
>
> buffer->buffer_group.name = "buffer";
> + buffer->buffer_group.is_visible = iio_buffer_attr_group_is_visible;
> buffer->buffer_group.attrs = attr;
>
> indio_dev->groups[indio_dev->groupcounter++] = &buffer->buffer_group;
