Re: [PATCH] iio: buffer: extend short-hand use for 'indio_dev->buffer'

From: Jonathan Cameron
Date: Sat Apr 25 2020 - 12:59:14 EST


On Fri, 24 Apr 2020 18:22:43 +0300
Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> wrote:

> This change is both cosmetic and a prequel to adding support for attaching
> multiple buffers per IIO device.
>
> The IIO buffer sysfs attrs are mostly designed to support only one attached
> buffer, and in order to support more, we need to centralize [in each attr
> function] the buffer which is being accessed.
>
> This also makes it a bit more uniform, as in some functions there is a
> short-hand 'buffer' variable and at the same time the 'indio_dev->buffer'
> is still access directly.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
Seems like a harmless bit of tidying up in of itself, so fair enough
even without seeing what you are going to do next :)

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to see if I missed anything.

Thanks,

Jonathan

> ---
> drivers/iio/industrialio-buffer.c | 61 +++++++++++++++++--------------
> 1 file changed, 34 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 221157136af6..eae39eaf49af 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -189,10 +189,12 @@ __poll_t iio_buffer_poll(struct file *filp,
> */
> void iio_buffer_wakeup_poll(struct iio_dev *indio_dev)
> {
> - if (!indio_dev->buffer)
> + struct iio_buffer *buffer = indio_dev->buffer;
> +
> + if (!buffer)
> return;
>
> - wake_up(&indio_dev->buffer->pollq);
> + wake_up(&buffer->pollq);
> }
>
> void iio_buffer_init(struct iio_buffer *buffer)
> @@ -262,10 +264,11 @@ static ssize_t iio_scan_el_show(struct device *dev,
> {
> int ret;
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct iio_buffer *buffer = indio_dev->buffer;
>
> /* Ensure ret is 0 or 1. */
> ret = !!test_bit(to_iio_dev_attr(attr)->address,
> - indio_dev->buffer->scan_mask);
> + buffer->scan_mask);
>
> return sprintf(buf, "%d\n", ret);
> }
> @@ -381,7 +384,7 @@ static ssize_t iio_scan_el_store(struct device *dev,
> if (ret < 0)
> return ret;
> mutex_lock(&indio_dev->mlock);
> - if (iio_buffer_is_active(indio_dev->buffer)) {
> + if (iio_buffer_is_active(buffer)) {
> ret = -EBUSY;
> goto error_ret;
> }
> @@ -410,7 +413,9 @@ static ssize_t iio_scan_el_ts_show(struct device *dev,
> char *buf)
> {
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - return sprintf(buf, "%d\n", indio_dev->buffer->scan_timestamp);
> + struct iio_buffer *buffer = indio_dev->buffer;
> +
> + return sprintf(buf, "%d\n", buffer->scan_timestamp);
> }
>
> static ssize_t iio_scan_el_ts_store(struct device *dev,
> @@ -420,6 +425,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;
> bool state;
>
> ret = strtobool(buf, &state);
> @@ -427,11 +433,11 @@ static ssize_t iio_scan_el_ts_store(struct device *dev,
> return ret;
>
> mutex_lock(&indio_dev->mlock);
> - if (iio_buffer_is_active(indio_dev->buffer)) {
> + if (iio_buffer_is_active(buffer)) {
> ret = -EBUSY;
> goto error_ret;
> }
> - indio_dev->buffer->scan_timestamp = state;
> + buffer->scan_timestamp = state;
> error_ret:
> mutex_unlock(&indio_dev->mlock);
>
> @@ -439,10 +445,10 @@ static ssize_t iio_scan_el_ts_store(struct device *dev,
> }
>
> static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev,
> + struct iio_buffer *buffer,
> const struct iio_chan_spec *chan)
> {
> int ret, attrcount = 0;
> - struct iio_buffer *buffer = indio_dev->buffer;
>
> ret = __iio_add_chan_devattr("index",
> chan,
> @@ -518,7 +524,7 @@ static ssize_t iio_buffer_write_length(struct device *dev,
> return len;
>
> mutex_lock(&indio_dev->mlock);
> - if (iio_buffer_is_active(indio_dev->buffer)) {
> + if (iio_buffer_is_active(buffer)) {
> ret = -EBUSY;
> } else {
> buffer->access->set_length(buffer, val);
> @@ -539,7 +545,9 @@ static ssize_t iio_buffer_show_enable(struct device *dev,
> char *buf)
> {
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - return sprintf(buf, "%d\n", iio_buffer_is_active(indio_dev->buffer));
> + struct iio_buffer *buffer = indio_dev->buffer;
> +
> + return sprintf(buf, "%d\n", iio_buffer_is_active(buffer));
> }
>
> static unsigned int iio_storage_bytes_for_si(struct iio_dev *indio_dev,
> @@ -1129,6 +1137,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;
> bool inlist;
>
> ret = strtobool(buf, &requested_state);
> @@ -1138,17 +1147,15 @@ static ssize_t iio_buffer_store_enable(struct device *dev,
> mutex_lock(&indio_dev->mlock);
>
> /* Find out if it is in the list */
> - inlist = iio_buffer_is_active(indio_dev->buffer);
> + inlist = iio_buffer_is_active(buffer);
> /* Already in desired state */
> if (inlist == requested_state)
> goto done;
>
> if (requested_state)
> - ret = __iio_update_buffers(indio_dev,
> - indio_dev->buffer, NULL);
> + ret = __iio_update_buffers(indio_dev, buffer, NULL);
> else
> - ret = __iio_update_buffers(indio_dev,
> - NULL, indio_dev->buffer);
> + ret = __iio_update_buffers(indio_dev, NULL, buffer);
>
> done:
> mutex_unlock(&indio_dev->mlock);
> @@ -1190,7 +1197,7 @@ static ssize_t iio_buffer_store_watermark(struct device *dev,
> goto out;
> }
>
> - if (iio_buffer_is_active(indio_dev->buffer)) {
> + if (iio_buffer_is_active(buffer)) {
> ret = -EBUSY;
> goto out;
> }
> @@ -1207,11 +1214,9 @@ static ssize_t iio_dma_show_data_available(struct device *dev,
> char *buf)
> {
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - size_t bytes;
> -
> - bytes = iio_buffer_data_available(indio_dev->buffer);
> + struct iio_buffer *buffer = indio_dev->buffer;
>
> - return sprintf(buf, "%zu\n", bytes);
> + return sprintf(buf, "%zu\n", iio_buffer_data_available(buffer));
> }
>
> static DEVICE_ATTR(length, S_IRUGO | S_IWUSR, iio_buffer_read_length,
> @@ -1292,7 +1297,7 @@ int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
> if (channels[i].scan_index < 0)
> continue;
>
> - ret = iio_buffer_add_channel_sysfs(indio_dev,
> + ret = iio_buffer_add_channel_sysfs(indio_dev, buffer,
> &channels[i]);
> if (ret < 0)
> goto error_cleanup_dynamic;
> @@ -1332,20 +1337,22 @@ int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
> bitmap_free(buffer->scan_mask);
> error_cleanup_dynamic:
> iio_free_chan_devattr_list(&buffer->scan_el_dev_attr_list);
> - kfree(indio_dev->buffer->buffer_group.attrs);
> + kfree(buffer->buffer_group.attrs);
>
> return ret;
> }
>
> void iio_buffer_free_sysfs_and_mask(struct iio_dev *indio_dev)
> {
> - if (!indio_dev->buffer)
> + struct iio_buffer *buffer = indio_dev->buffer;
> +
> + if (!buffer)
> return;
>
> - bitmap_free(indio_dev->buffer->scan_mask);
> - kfree(indio_dev->buffer->buffer_group.attrs);
> - kfree(indio_dev->buffer->scan_el_group.attrs);
> - iio_free_chan_devattr_list(&indio_dev->buffer->scan_el_dev_attr_list);
> + bitmap_free(buffer->scan_mask);
> + kfree(buffer->buffer_group.attrs);
> + kfree(buffer->scan_el_group.attrs);
> + iio_free_chan_devattr_list(&buffer->scan_el_dev_attr_list);
> }
>
> /**