[PATCH 00/11] iio: Buffer cleanups and consolidations

[PATCH 00/11] iio: Buffer cleanups and consolidations

[Lars-Peter Clausen]

Hi,

This series contains a couple of cleanups and code consolidations for the
IIO buffer handling. The biggest change is probably the moving of the buffer
registration itself to the core rather than having to do this manually in
each driver. Having to do this manually had its place in earlier days, but
today it is just boilerplate code.

The other changes are mostly concerned with moving boilerplate code from
individual buffer implementations to the core. Given that there is only one
serious buffer implementation at the moment this doesn't do to much. But
this series is done in preparation for adding the DMA buffer support, so we
do not have to add the same duplicated code for the DMA buffer.

- Lars

Lars-Peter Clausen (11):
  staging:iio:ad5933: Don't enable channels by default
  staging:iio:sca3000: Don't enable channels by default
  iio: Unexport iio_scan_mask_set()
  staging:iio:sca3000: Register same channels for device and buffer
  staging:iio:dummy: Register same channels for device and buffer
  iio: Move buffer registration to the core
  iio: Remove get_bytes_per_datum() from iio_buffer_access_funcs
  iio: buffer: Move iio_buffer_alloc_sysfs and iio_buffer_free_sysfs
  iio: buffer: Allocate standard attributes in the core
  iio: buffer: Make length attribute read only for buffers without
    set_length
  iio: buffer: Drop get_length callback

 drivers/iio/adc/ti_am335x_adc.c                 |   9 -
 drivers/iio/iio_core.h                          |   9 +
 drivers/iio/industrialio-buffer.c               | 403 ++++++++++++------------
 drivers/iio/industrialio-core.c                 |  14 +-
 drivers/iio/industrialio-triggered-buffer.c     |   9 -
 drivers/iio/kfifo_buf.c                         |  27 --
 drivers/staging/iio/Documentation/ring.txt      |   8 +-
 drivers/staging/iio/accel/lis3l02dq_core.c      |  13 +-
 drivers/staging/iio/accel/sca3000_core.c        |  17 +-
 drivers/staging/iio/accel/sca3000_ring.c        |  29 +-
 drivers/staging/iio/iio_simple_dummy.c          |  13 +-
 drivers/staging/iio/iio_simple_dummy.h          |   3 +-
 drivers/staging/iio/iio_simple_dummy_buffer.c   |  10 +-
 drivers/staging/iio/impedance-analyzer/ad5933.c |  16 +-
 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                      |  76 +----
 18 files changed, 255 insertions(+), 422 deletions(-)

-- 
1.8.0

[PATCH 01/11] staging:iio:ad5933: Don't enable channels by default

[Lars-Peter Clausen]

The convention for IIO devices is that all channels are disabled by default.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/impedance-analyzer/ad5933.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
index b6bd609..aa6a368 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -757,10 +757,6 @@ static int ad5933_probe(struct i2c_client *client,
 	if (ret)
 		goto error_unreg_ring;
 
-	/* enable both REAL and IMAG channels by default */
-	iio_scan_mask_set(indio_dev, indio_dev->buffer, 0);
-	iio_scan_mask_set(indio_dev, indio_dev->buffer, 1);
-
 	ret = ad5933_setup(st);
 	if (ret)
 		goto error_uninitialize_ring;
-- 
1.8.0

Re: [PATCH 01/11] staging:iio:ad5933: Don't enable channels by default

[Daniel Baluta]

On Wed, Nov 26, 2014 at 7:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> The convention for IIO devices is that all channels are disabled by default.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Reviewed-by: Daniel Baluta <daniel.baluta@intel.com>

> ---
>  drivers/staging/iio/impedance-analyzer/ad5933.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
> index b6bd609..aa6a368 100644
> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> @@ -757,10 +757,6 @@ static int ad5933_probe(struct i2c_client *client,
>         if (ret)
>                 goto error_unreg_ring;
>
> -       /* enable both REAL and IMAG channels by default */
> -       iio_scan_mask_set(indio_dev, indio_dev->buffer, 0);
> -       iio_scan_mask_set(indio_dev, indio_dev->buffer, 1);
> -
>         ret = ad5933_setup(st);
>         if (ret)
>                 goto error_uninitialize_ring;
> --
> 1.8.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/11] staging:iio:ad5933: Don't enable channels by default

[Jonathan Cameron]

On 04/12/14 22:51, Daniel Baluta wrote:
> On Wed, Nov 26, 2014 at 7:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> The convention for IIO devices is that all channels are disabled by default.
To be fair - it wasn't the convention when these were written ;)
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> Reviewed-by: Daniel Baluta <daniel.baluta@intel.com>
Applied to the togreg branch of iio.git.
It's just much easier to ensure their are no bugs and
everything is predictable.  This way we don't end up with
people relying on a particular set of channels being enabled.
> 
>> ---
>>  drivers/staging/iio/impedance-analyzer/ad5933.c | 4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
>> index b6bd609..aa6a368 100644
>> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
>> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
>> @@ -757,10 +757,6 @@ static int ad5933_probe(struct i2c_client *client,
>>         if (ret)
>>                 goto error_unreg_ring;
>>
>> -       /* enable both REAL and IMAG channels by default */
>> -       iio_scan_mask_set(indio_dev, indio_dev->buffer, 0);
>> -       iio_scan_mask_set(indio_dev, indio_dev->buffer, 1);
>> -
>>         ret = ad5933_setup(st);
>>         if (ret)
>>                 goto error_uninitialize_ring;
>> --
>> 1.8.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 02/11] staging:iio:sca3000: Don't enable channels by default

[Lars-Peter Clausen]

The convention for IIO devices is that all channels are disabled by default.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/accel/sca3000_core.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
index e4e5639..cd46a61 100644
--- a/drivers/staging/iio/accel/sca3000_core.c
+++ b/drivers/staging/iio/accel/sca3000_core.c
@@ -1159,11 +1159,6 @@ static int sca3000_probe(struct spi_device *spi)
 				  ARRAY_SIZE(sca3000_channels));
 	if (ret < 0)
 		goto error_unregister_dev;
-	if (indio_dev->buffer) {
-		iio_scan_mask_set(indio_dev, indio_dev->buffer, 0);
-		iio_scan_mask_set(indio_dev, indio_dev->buffer, 1);
-		iio_scan_mask_set(indio_dev, indio_dev->buffer, 2);
-	}
 
 	if (spi->irq) {
 		ret = request_threaded_irq(spi->irq,
-- 
1.8.0

Re: [PATCH 02/11] staging:iio:sca3000: Don't enable channels by default

[Daniel Baluta]

On Wed, Nov 26, 2014 at 7:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> The convention for IIO devices is that all channels are disabled by default.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Reviewed-by: Daniel Baluta <daniel.baluta@intel.com>

> ---
>  drivers/staging/iio/accel/sca3000_core.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
> index e4e5639..cd46a61 100644
> --- a/drivers/staging/iio/accel/sca3000_core.c
> +++ b/drivers/staging/iio/accel/sca3000_core.c
> @@ -1159,11 +1159,6 @@ static int sca3000_probe(struct spi_device *spi)
>                                   ARRAY_SIZE(sca3000_channels));
>         if (ret < 0)
>                 goto error_unregister_dev;
> -       if (indio_dev->buffer) {
> -               iio_scan_mask_set(indio_dev, indio_dev->buffer, 0);
> -               iio_scan_mask_set(indio_dev, indio_dev->buffer, 1);
> -               iio_scan_mask_set(indio_dev, indio_dev->buffer, 2);
> -       }
>
>         if (spi->irq) {
>                 ret = request_threaded_irq(spi->irq,
> --
> 1.8.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/11] staging:iio:sca3000: Don't enable channels by default

[Jonathan Cameron]

On 04/12/14 22:51, Daniel Baluta wrote:
> On Wed, Nov 26, 2014 at 7:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> The convention for IIO devices is that all channels are disabled by default.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> Reviewed-by: Daniel Baluta <daniel.baluta@intel.com>
Applied.
> 
>> ---
>>  drivers/staging/iio/accel/sca3000_core.c | 5 -----
>>  1 file changed, 5 deletions(-)
>>
>> diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
>> index e4e5639..cd46a61 100644
>> --- a/drivers/staging/iio/accel/sca3000_core.c
>> +++ b/drivers/staging/iio/accel/sca3000_core.c
>> @@ -1159,11 +1159,6 @@ static int sca3000_probe(struct spi_device *spi)
>>                                   ARRAY_SIZE(sca3000_channels));
>>         if (ret < 0)
>>                 goto error_unregister_dev;
>> -       if (indio_dev->buffer) {
>> -               iio_scan_mask_set(indio_dev, indio_dev->buffer, 0);
>> -               iio_scan_mask_set(indio_dev, indio_dev->buffer, 1);
>> -               iio_scan_mask_set(indio_dev, indio_dev->buffer, 2);
>> -       }
>>
>>         if (spi->irq) {
>>                 ret = request_threaded_irq(spi->irq,
>> --
>> 1.8.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 03/11] iio: Unexport iio_scan_mask_set()

[Lars-Peter Clausen]

Individual drivers should not be messing with the scan mask that contains
the list of enabled channels. This is something that is supposed to be
managed by the core.

Now that the last few drivers that used it to configure a default scan mask
have been updated to not do this anymore we can unexport the function.

Note, this patch also requires moving a few functions around so they are all
declared before the first internal user.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-buffer.c | 149 +++++++++++++++++++-------------------
 include/linux/iio/buffer.h        |   9 ---
 2 files changed, 74 insertions(+), 84 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index f971f79..f667e4e 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -178,6 +178,80 @@ static ssize_t iio_scan_el_show(struct device *dev,
 	return sprintf(buf, "%d\n", ret);
 }
 
+/* Note NULL used as error indicator as it doesn't make sense. */
+static const unsigned long *iio_scan_mask_match(const unsigned long *av_masks,
+					  unsigned int masklength,
+					  const unsigned long *mask)
+{
+	if (bitmap_empty(mask, masklength))
+		return NULL;
+	while (*av_masks) {
+		if (bitmap_subset(mask, av_masks, masklength))
+			return av_masks;
+		av_masks += BITS_TO_LONGS(masklength);
+	}
+	return NULL;
+}
+
+static bool iio_validate_scan_mask(struct iio_dev *indio_dev,
+	const unsigned long *mask)
+{
+	if (!indio_dev->setup_ops->validate_scan_mask)
+		return true;
+
+	return indio_dev->setup_ops->validate_scan_mask(indio_dev, mask);
+}
+
+/**
+ * iio_scan_mask_set() - set particular bit in the scan mask
+ * @indio_dev: the iio device
+ * @buffer: the buffer whose scan mask we are interested in
+ * @bit: the bit to be set.
+ *
+ * Note that at this point we have no way of knowing what other
+ * buffers might request, hence this code only verifies that the
+ * individual buffers request is plausible.
+ */
+static int iio_scan_mask_set(struct iio_dev *indio_dev,
+		      struct iio_buffer *buffer, int bit)
+{
+	const unsigned long *mask;
+	unsigned long *trialmask;
+
+	trialmask = kmalloc(sizeof(*trialmask)*
+			    BITS_TO_LONGS(indio_dev->masklength),
+			    GFP_KERNEL);
+
+	if (trialmask == NULL)
+		return -ENOMEM;
+	if (!indio_dev->masklength) {
+		WARN_ON("Trying to set scanmask prior to registering buffer\n");
+		goto err_invalid_mask;
+	}
+	bitmap_copy(trialmask, buffer->scan_mask, indio_dev->masklength);
+	set_bit(bit, trialmask);
+
+	if (!iio_validate_scan_mask(indio_dev, trialmask))
+		goto err_invalid_mask;
+
+	if (indio_dev->available_scan_masks) {
+		mask = iio_scan_mask_match(indio_dev->available_scan_masks,
+					   indio_dev->masklength,
+					   trialmask);
+		if (!mask)
+			goto err_invalid_mask;
+	}
+	bitmap_copy(buffer->scan_mask, trialmask, indio_dev->masklength);
+
+	kfree(trialmask);
+
+	return 0;
+
+err_invalid_mask:
+	kfree(trialmask);
+	return -EINVAL;
+}
+
 static int iio_scan_mask_clear(struct iio_buffer *buffer, int bit)
 {
 	clear_bit(bit, buffer->scan_mask);
@@ -455,21 +529,6 @@ ssize_t iio_buffer_show_enable(struct device *dev,
 }
 EXPORT_SYMBOL(iio_buffer_show_enable);
 
-/* Note NULL used as error indicator as it doesn't make sense. */
-static const unsigned long *iio_scan_mask_match(const unsigned long *av_masks,
-					  unsigned int masklength,
-					  const unsigned long *mask)
-{
-	if (bitmap_empty(mask, masklength))
-		return NULL;
-	while (*av_masks) {
-		if (bitmap_subset(mask, av_masks, masklength))
-			return av_masks;
-		av_masks += BITS_TO_LONGS(masklength);
-	}
-	return NULL;
-}
-
 static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
 				const unsigned long *mask, bool timestamp)
 {
@@ -808,66 +867,6 @@ bool iio_validate_scan_mask_onehot(struct iio_dev *indio_dev,
 }
 EXPORT_SYMBOL_GPL(iio_validate_scan_mask_onehot);
 
-static bool iio_validate_scan_mask(struct iio_dev *indio_dev,
-	const unsigned long *mask)
-{
-	if (!indio_dev->setup_ops->validate_scan_mask)
-		return true;
-
-	return indio_dev->setup_ops->validate_scan_mask(indio_dev, mask);
-}
-
-/**
- * iio_scan_mask_set() - set particular bit in the scan mask
- * @indio_dev: the iio device
- * @buffer: the buffer whose scan mask we are interested in
- * @bit: the bit to be set.
- *
- * Note that at this point we have no way of knowing what other
- * buffers might request, hence this code only verifies that the
- * individual buffers request is plausible.
- */
-int iio_scan_mask_set(struct iio_dev *indio_dev,
-		      struct iio_buffer *buffer, int bit)
-{
-	const unsigned long *mask;
-	unsigned long *trialmask;
-
-	trialmask = kmalloc(sizeof(*trialmask)*
-			    BITS_TO_LONGS(indio_dev->masklength),
-			    GFP_KERNEL);
-
-	if (trialmask == NULL)
-		return -ENOMEM;
-	if (!indio_dev->masklength) {
-		WARN_ON("Trying to set scanmask prior to registering buffer\n");
-		goto err_invalid_mask;
-	}
-	bitmap_copy(trialmask, buffer->scan_mask, indio_dev->masklength);
-	set_bit(bit, trialmask);
-
-	if (!iio_validate_scan_mask(indio_dev, trialmask))
-		goto err_invalid_mask;
-
-	if (indio_dev->available_scan_masks) {
-		mask = iio_scan_mask_match(indio_dev->available_scan_masks,
-					   indio_dev->masklength,
-					   trialmask);
-		if (!mask)
-			goto err_invalid_mask;
-	}
-	bitmap_copy(buffer->scan_mask, trialmask, indio_dev->masklength);
-
-	kfree(trialmask);
-
-	return 0;
-
-err_invalid_mask:
-	kfree(trialmask);
-	return -EINVAL;
-}
-EXPORT_SYMBOL_GPL(iio_scan_mask_set);
-
 int iio_scan_mask_query(struct iio_dev *indio_dev,
 			struct iio_buffer *buffer, int bit)
 {
diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
index 5193927..8c8ce61 100644
--- a/include/linux/iio/buffer.h
+++ b/include/linux/iio/buffer.h
@@ -117,15 +117,6 @@ int iio_scan_mask_query(struct iio_dev *indio_dev,
 			struct iio_buffer *buffer, int bit);
 
 /**
- * iio_scan_mask_set() - set particular bit in the scan mask
- * @indio_dev		IIO device structure
- * @buffer:		the buffer whose scan mask we are interested in
- * @bit:		the bit to be set.
- **/
-int iio_scan_mask_set(struct iio_dev *indio_dev,
-		      struct iio_buffer *buffer, int bit);
-
-/**
  * iio_push_to_buffers() - push to a registered buffer.
  * @indio_dev:		iio_dev structure for device.
  * @data:		Full scan.
-- 
1.8.0

Re: [PATCH 03/11] iio: Unexport iio_scan_mask_set()

[Daniel Baluta]

On Wed, Nov 26, 2014 at 7:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> Individual drivers should not be messing with the scan mask that contains
> the list of enabled channels. This is something that is supposed to be
> managed by the core.
>
> Now that the last few drivers that used it to configure a default scan mask
> have been updated to not do this anymore we can unexport the function.
>
> Note, this patch also requires moving a few functions around so they are all
> declared before the first internal user.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Reviewed-by: Daniel Baluta <daniel.baluta@intel.com>

> ---
>  drivers/iio/industrialio-buffer.c | 149 +++++++++++++++++++-------------------
>  include/linux/iio/buffer.h        |   9 ---
>  2 files changed, 74 insertions(+), 84 deletions(-)
>
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index f971f79..f667e4e 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -178,6 +178,80 @@ static ssize_t iio_scan_el_show(struct device *dev,
>         return sprintf(buf, "%d\n", ret);
>  }
>
> +/* Note NULL used as error indicator as it doesn't make sense. */
> +static const unsigned long *iio_scan_mask_match(const unsigned long *av_masks,
> +                                         unsigned int masklength,
> +                                         const unsigned long *mask)
> +{
> +       if (bitmap_empty(mask, masklength))
> +               return NULL;
> +       while (*av_masks) {
> +               if (bitmap_subset(mask, av_masks, masklength))
> +                       return av_masks;
> +               av_masks += BITS_TO_LONGS(masklength);
> +       }
> +       return NULL;
> +}
> +
> +static bool iio_validate_scan_mask(struct iio_dev *indio_dev,
> +       const unsigned long *mask)
> +{
> +       if (!indio_dev->setup_ops->validate_scan_mask)
> +               return true;
> +
> +       return indio_dev->setup_ops->validate_scan_mask(indio_dev, mask);
> +}
> +
> +/**
> + * iio_scan_mask_set() - set particular bit in the scan mask
> + * @indio_dev: the iio device
> + * @buffer: the buffer whose scan mask we are interested in
> + * @bit: the bit to be set.
> + *
> + * Note that at this point we have no way of knowing what other
> + * buffers might request, hence this code only verifies that the
> + * individual buffers request is plausible.
> + */
> +static int iio_scan_mask_set(struct iio_dev *indio_dev,
> +                     struct iio_buffer *buffer, int bit)
> +{
> +       const unsigned long *mask;
> +       unsigned long *trialmask;
> +
> +       trialmask = kmalloc(sizeof(*trialmask)*
> +                           BITS_TO_LONGS(indio_dev->masklength),
> +                           GFP_KERNEL);
> +
> +       if (trialmask == NULL)
> +               return -ENOMEM;
> +       if (!indio_dev->masklength) {
> +               WARN_ON("Trying to set scanmask prior to registering buffer\n");
> +               goto err_invalid_mask;
> +       }
> +       bitmap_copy(trialmask, buffer->scan_mask, indio_dev->masklength);
> +       set_bit(bit, trialmask);
> +
> +       if (!iio_validate_scan_mask(indio_dev, trialmask))
> +               goto err_invalid_mask;
> +
> +       if (indio_dev->available_scan_masks) {
> +               mask = iio_scan_mask_match(indio_dev->available_scan_masks,
> +                                          indio_dev->masklength,
> +                                          trialmask);
> +               if (!mask)
> +                       goto err_invalid_mask;
> +       }
> +       bitmap_copy(buffer->scan_mask, trialmask, indio_dev->masklength);
> +
> +       kfree(trialmask);
> +
> +       return 0;
> +
> +err_invalid_mask:
> +       kfree(trialmask);
> +       return -EINVAL;
> +}
> +
>  static int iio_scan_mask_clear(struct iio_buffer *buffer, int bit)
>  {
>         clear_bit(bit, buffer->scan_mask);
> @@ -455,21 +529,6 @@ ssize_t iio_buffer_show_enable(struct device *dev,
>  }
>  EXPORT_SYMBOL(iio_buffer_show_enable);
>
> -/* Note NULL used as error indicator as it doesn't make sense. */
> -static const unsigned long *iio_scan_mask_match(const unsigned long *av_masks,
> -                                         unsigned int masklength,
> -                                         const unsigned long *mask)
> -{
> -       if (bitmap_empty(mask, masklength))
> -               return NULL;
> -       while (*av_masks) {
> -               if (bitmap_subset(mask, av_masks, masklength))
> -                       return av_masks;
> -               av_masks += BITS_TO_LONGS(masklength);
> -       }
> -       return NULL;
> -}
> -
>  static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
>                                 const unsigned long *mask, bool timestamp)
>  {
> @@ -808,66 +867,6 @@ bool iio_validate_scan_mask_onehot(struct iio_dev *indio_dev,
>  }
>  EXPORT_SYMBOL_GPL(iio_validate_scan_mask_onehot);
>
> -static bool iio_validate_scan_mask(struct iio_dev *indio_dev,
> -       const unsigned long *mask)
> -{
> -       if (!indio_dev->setup_ops->validate_scan_mask)
> -               return true;
> -
> -       return indio_dev->setup_ops->validate_scan_mask(indio_dev, mask);
> -}
> -
> -/**
> - * iio_scan_mask_set() - set particular bit in the scan mask
> - * @indio_dev: the iio device
> - * @buffer: the buffer whose scan mask we are interested in
> - * @bit: the bit to be set.
> - *
> - * Note that at this point we have no way of knowing what other
> - * buffers might request, hence this code only verifies that the
> - * individual buffers request is plausible.
> - */
> -int iio_scan_mask_set(struct iio_dev *indio_dev,
> -                     struct iio_buffer *buffer, int bit)
> -{
> -       const unsigned long *mask;
> -       unsigned long *trialmask;
> -
> -       trialmask = kmalloc(sizeof(*trialmask)*
> -                           BITS_TO_LONGS(indio_dev->masklength),
> -                           GFP_KERNEL);
> -
> -       if (trialmask == NULL)
> -               return -ENOMEM;
> -       if (!indio_dev->masklength) {
> -               WARN_ON("Trying to set scanmask prior to registering buffer\n");
> -               goto err_invalid_mask;
> -       }
> -       bitmap_copy(trialmask, buffer->scan_mask, indio_dev->masklength);
> -       set_bit(bit, trialmask);
> -
> -       if (!iio_validate_scan_mask(indio_dev, trialmask))
> -               goto err_invalid_mask;
> -
> -       if (indio_dev->available_scan_masks) {
> -               mask = iio_scan_mask_match(indio_dev->available_scan_masks,
> -                                          indio_dev->masklength,
> -                                          trialmask);
> -               if (!mask)
> -                       goto err_invalid_mask;
> -       }
> -       bitmap_copy(buffer->scan_mask, trialmask, indio_dev->masklength);
> -
> -       kfree(trialmask);
> -
> -       return 0;
> -
> -err_invalid_mask:
> -       kfree(trialmask);
> -       return -EINVAL;
> -}
> -EXPORT_SYMBOL_GPL(iio_scan_mask_set);
> -
>  int iio_scan_mask_query(struct iio_dev *indio_dev,
>                         struct iio_buffer *buffer, int bit)
>  {
> diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
> index 5193927..8c8ce61 100644
> --- a/include/linux/iio/buffer.h
> +++ b/include/linux/iio/buffer.h
> @@ -117,15 +117,6 @@ int iio_scan_mask_query(struct iio_dev *indio_dev,
>                         struct iio_buffer *buffer, int bit);
>
>  /**
> - * iio_scan_mask_set() - set particular bit in the scan mask
> - * @indio_dev          IIO device structure
> - * @buffer:            the buffer whose scan mask we are interested in
> - * @bit:               the bit to be set.
> - **/
> -int iio_scan_mask_set(struct iio_dev *indio_dev,
> -                     struct iio_buffer *buffer, int bit);
> -
> -/**
>   * iio_push_to_buffers() - push to a registered buffer.
>   * @indio_dev:         iio_dev structure for device.
>   * @data:              Full scan.
> --
> 1.8.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/11] iio: Unexport iio_scan_mask_set()

[Jonathan Cameron]

On 05/12/14 09:53, Daniel Baluta wrote:
> On Wed, Nov 26, 2014 at 7:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> Individual drivers should not be messing with the scan mask that contains
>> the list of enabled channels. This is something that is supposed to be
>> managed by the core.
>>
>> Now that the last few drivers that used it to configure a default scan mask
>> have been updated to not do this anymore we can unexport the function.
>>
>> Note, this patch also requires moving a few functions around so they are all
>> declared before the first internal user.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> Reviewed-by: Daniel Baluta <daniel.baluta@intel.com>
Applied to the togreg branch of iio.git - initially pushed out as testing.
Thanks,

J
> 
>> ---
>>  drivers/iio/industrialio-buffer.c | 149 +++++++++++++++++++-------------------
>>  include/linux/iio/buffer.h        |   9 ---
>>  2 files changed, 74 insertions(+), 84 deletions(-)
>>
>> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
>> index f971f79..f667e4e 100644
>> --- a/drivers/iio/industrialio-buffer.c
>> +++ b/drivers/iio/industrialio-buffer.c
>> @@ -178,6 +178,80 @@ static ssize_t iio_scan_el_show(struct device *dev,
>>         return sprintf(buf, "%d\n", ret);
>>  }
>>
>> +/* Note NULL used as error indicator as it doesn't make sense. */
>> +static const unsigned long *iio_scan_mask_match(const unsigned long *av_masks,
>> +                                         unsigned int masklength,
>> +                                         const unsigned long *mask)
>> +{
>> +       if (bitmap_empty(mask, masklength))
>> +               return NULL;
>> +       while (*av_masks) {
>> +               if (bitmap_subset(mask, av_masks, masklength))
>> +                       return av_masks;
>> +               av_masks += BITS_TO_LONGS(masklength);
>> +       }
>> +       return NULL;
>> +}
>> +
>> +static bool iio_validate_scan_mask(struct iio_dev *indio_dev,
>> +       const unsigned long *mask)
>> +{
>> +       if (!indio_dev->setup_ops->validate_scan_mask)
>> +               return true;
>> +
>> +       return indio_dev->setup_ops->validate_scan_mask(indio_dev, mask);
>> +}
>> +
>> +/**
>> + * iio_scan_mask_set() - set particular bit in the scan mask
>> + * @indio_dev: the iio device
>> + * @buffer: the buffer whose scan mask we are interested in
>> + * @bit: the bit to be set.
>> + *
>> + * Note that at this point we have no way of knowing what other
>> + * buffers might request, hence this code only verifies that the
>> + * individual buffers request is plausible.
>> + */
>> +static int iio_scan_mask_set(struct iio_dev *indio_dev,
>> +                     struct iio_buffer *buffer, int bit)
>> +{
>> +       const unsigned long *mask;
>> +       unsigned long *trialmask;
>> +
>> +       trialmask = kmalloc(sizeof(*trialmask)*
>> +                           BITS_TO_LONGS(indio_dev->masklength),
>> +                           GFP_KERNEL);
>> +
>> +       if (trialmask == NULL)
>> +               return -ENOMEM;
>> +       if (!indio_dev->masklength) {
>> +               WARN_ON("Trying to set scanmask prior to registering buffer\n");
>> +               goto err_invalid_mask;
>> +       }
>> +       bitmap_copy(trialmask, buffer->scan_mask, indio_dev->masklength);
>> +       set_bit(bit, trialmask);
>> +
>> +       if (!iio_validate_scan_mask(indio_dev, trialmask))
>> +               goto err_invalid_mask;
>> +
>> +       if (indio_dev->available_scan_masks) {
>> +               mask = iio_scan_mask_match(indio_dev->available_scan_masks,
>> +                                          indio_dev->masklength,
>> +                                          trialmask);
>> +               if (!mask)
>> +                       goto err_invalid_mask;
>> +       }
>> +       bitmap_copy(buffer->scan_mask, trialmask, indio_dev->masklength);
>> +
>> +       kfree(trialmask);
>> +
>> +       return 0;
>> +
>> +err_invalid_mask:
>> +       kfree(trialmask);
>> +       return -EINVAL;
>> +}
>> +
>>  static int iio_scan_mask_clear(struct iio_buffer *buffer, int bit)
>>  {
>>         clear_bit(bit, buffer->scan_mask);
>> @@ -455,21 +529,6 @@ ssize_t iio_buffer_show_enable(struct device *dev,
>>  }
>>  EXPORT_SYMBOL(iio_buffer_show_enable);
>>
>> -/* Note NULL used as error indicator as it doesn't make sense. */
>> -static const unsigned long *iio_scan_mask_match(const unsigned long *av_masks,
>> -                                         unsigned int masklength,
>> -                                         const unsigned long *mask)
>> -{
>> -       if (bitmap_empty(mask, masklength))
>> -               return NULL;
>> -       while (*av_masks) {
>> -               if (bitmap_subset(mask, av_masks, masklength))
>> -                       return av_masks;
>> -               av_masks += BITS_TO_LONGS(masklength);
>> -       }
>> -       return NULL;
>> -}
>> -
>>  static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
>>                                 const unsigned long *mask, bool timestamp)
>>  {
>> @@ -808,66 +867,6 @@ bool iio_validate_scan_mask_onehot(struct iio_dev *indio_dev,
>>  }
>>  EXPORT_SYMBOL_GPL(iio_validate_scan_mask_onehot);
>>
>> -static bool iio_validate_scan_mask(struct iio_dev *indio_dev,
>> -       const unsigned long *mask)
>> -{
>> -       if (!indio_dev->setup_ops->validate_scan_mask)
>> -               return true;
>> -
>> -       return indio_dev->setup_ops->validate_scan_mask(indio_dev, mask);
>> -}
>> -
>> -/**
>> - * iio_scan_mask_set() - set particular bit in the scan mask
>> - * @indio_dev: the iio device
>> - * @buffer: the buffer whose scan mask we are interested in
>> - * @bit: the bit to be set.
>> - *
>> - * Note that at this point we have no way of knowing what other
>> - * buffers might request, hence this code only verifies that the
>> - * individual buffers request is plausible.
>> - */
>> -int iio_scan_mask_set(struct iio_dev *indio_dev,
>> -                     struct iio_buffer *buffer, int bit)
>> -{
>> -       const unsigned long *mask;
>> -       unsigned long *trialmask;
>> -
>> -       trialmask = kmalloc(sizeof(*trialmask)*
>> -                           BITS_TO_LONGS(indio_dev->masklength),
>> -                           GFP_KERNEL);
>> -
>> -       if (trialmask == NULL)
>> -               return -ENOMEM;
>> -       if (!indio_dev->masklength) {
>> -               WARN_ON("Trying to set scanmask prior to registering buffer\n");
>> -               goto err_invalid_mask;
>> -       }
>> -       bitmap_copy(trialmask, buffer->scan_mask, indio_dev->masklength);
>> -       set_bit(bit, trialmask);
>> -
>> -       if (!iio_validate_scan_mask(indio_dev, trialmask))
>> -               goto err_invalid_mask;
>> -
>> -       if (indio_dev->available_scan_masks) {
>> -               mask = iio_scan_mask_match(indio_dev->available_scan_masks,
>> -                                          indio_dev->masklength,
>> -                                          trialmask);
>> -               if (!mask)
>> -                       goto err_invalid_mask;
>> -       }
>> -       bitmap_copy(buffer->scan_mask, trialmask, indio_dev->masklength);
>> -
>> -       kfree(trialmask);
>> -
>> -       return 0;
>> -
>> -err_invalid_mask:
>> -       kfree(trialmask);
>> -       return -EINVAL;
>> -}
>> -EXPORT_SYMBOL_GPL(iio_scan_mask_set);
>> -
>>  int iio_scan_mask_query(struct iio_dev *indio_dev,
>>                         struct iio_buffer *buffer, int bit)
>>  {
>> diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
>> index 5193927..8c8ce61 100644
>> --- a/include/linux/iio/buffer.h
>> +++ b/include/linux/iio/buffer.h
>> @@ -117,15 +117,6 @@ int iio_scan_mask_query(struct iio_dev *indio_dev,
>>                         struct iio_buffer *buffer, int bit);
>>
>>  /**
>> - * iio_scan_mask_set() - set particular bit in the scan mask
>> - * @indio_dev          IIO device structure
>> - * @buffer:            the buffer whose scan mask we are interested in
>> - * @bit:               the bit to be set.
>> - **/
>> -int iio_scan_mask_set(struct iio_dev *indio_dev,
>> -                     struct iio_buffer *buffer, int bit);
>> -
>> -/**
>>   * iio_push_to_buffers() - push to a registered buffer.
>>   * @indio_dev:         iio_dev structure for device.
>>   * @data:              Full scan.
>> --
>> 1.8.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 04/11] staging:iio:sca3000: Register same channels for device and buffer

[Lars-Peter Clausen]

In preparation for moving the buffer registration to the core make sure to
register the same channel array for the device and the buffer. Currently
the temperature channel is not registered for the buffer, setting its scan
index to -1 will make sure that it is skipped for the buffer.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/accel/sca3000_core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
index cd46a61..9eedb93 100644
--- a/drivers/staging/iio/accel/sca3000_core.c
+++ b/drivers/staging/iio/accel/sca3000_core.c
@@ -459,6 +459,7 @@ static const struct iio_chan_spec sca3000_channels_with_temp[] = {
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
 			BIT(IIO_CHAN_INFO_OFFSET),
+		.scan_index = -1,
 	},
 };
 
@@ -1154,9 +1155,8 @@ static int sca3000_probe(struct spi_device *spi)
 	if (ret < 0)
 		return ret;
 
-	ret = iio_buffer_register(indio_dev,
-				  sca3000_channels,
-				  ARRAY_SIZE(sca3000_channels));
+	ret = iio_buffer_register(indio_dev, indio_dev->channels,
+		indio_dev->num_channels);
 	if (ret < 0)
 		goto error_unregister_dev;
 
-- 
1.8.0

Re: [PATCH 04/11] staging:iio:sca3000: Register same channels for device and buffer

[Daniel Baluta]

Minor comment inline.

On Wed, Nov 26, 2014 at 7:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> In preparation for moving the buffer registration to the core make sure to
> register the same channel array for the device and the buffer. Currently
> the temperature channel is not registered for the buffer, setting its scan
> index to -1 will make sure that it is skipped for the buffer.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/staging/iio/accel/sca3000_core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
> index cd46a61..9eedb93 100644
> --- a/drivers/staging/iio/accel/sca3000_core.c
> +++ b/drivers/staging/iio/accel/sca3000_core.c
> @@ -459,6 +459,7 @@ static const struct iio_chan_spec sca3000_channels_with_temp[] = {
>                 .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>                 .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
>                         BIT(IIO_CHAN_INFO_OFFSET),
> +               .scan_index = -1,

Would be nice to have  /* No buffer support */ comment as in 5/11 patch.

Otherwise, it looks good to me.

Daniel.

Re: [PATCH 04/11] staging:iio:sca3000: Register same channels for device and buffer

[Jonathan Cameron]

On 04/12/14 22:56, Daniel Baluta wrote:
> Minor comment inline.
> 
> On Wed, Nov 26, 2014 at 7:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> In preparation for moving the buffer registration to the core make sure to
>> register the same channel array for the device and the buffer. Currently
>> the temperature channel is not registered for the buffer, setting its scan
>> index to -1 will make sure that it is skipped for the buffer.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> ---
>>  drivers/staging/iio/accel/sca3000_core.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
>> index cd46a61..9eedb93 100644
>> --- a/drivers/staging/iio/accel/sca3000_core.c
>> +++ b/drivers/staging/iio/accel/sca3000_core.c
>> @@ -459,6 +459,7 @@ static const struct iio_chan_spec sca3000_channels_with_temp[] = {
>>                 .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>>                 .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
>>                         BIT(IIO_CHAN_INFO_OFFSET),
>> +               .scan_index = -1,
> 
> Would be nice to have  /* No buffer support */ comment as in 5/11 patch.
Agreed - have added whilst applying the patch.
> 
> Otherwise, it looks good to me.
> 
> Daniel.
> 

Re: [PATCH 04/11] staging:iio:sca3000: Register same channels for device and buffer

[Hartmut Knaack]

Lars-Peter Clausen schrieb am 26.11.2014 um 18:55:
> In preparation for moving the buffer registration to the core make sure to
> register the same channel array for the device and the buffer. Currently
> the temperature channel is not registered for the buffer, setting its scan
> index to -1 will make sure that it is skipped for the buffer.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/staging/iio/accel/sca3000_core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
> index cd46a61..9eedb93 100644
> --- a/drivers/staging/iio/accel/sca3000_core.c
> +++ b/drivers/staging/iio/accel/sca3000_core.c
> @@ -459,6 +459,7 @@ static const struct iio_chan_spec sca3000_channels_with_temp[] = {
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
>  			BIT(IIO_CHAN_INFO_OFFSET),
> +		.scan_index = -1,
>  	},
>  };
>  
> @@ -1154,9 +1155,8 @@ static int sca3000_probe(struct spi_device *spi)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = iio_buffer_register(indio_dev,
> -				  sca3000_channels,
> -				  ARRAY_SIZE(sca3000_channels));
> +	ret = iio_buffer_register(indio_dev, indio_dev->channels,
> +		indio_dev->num_channels);
Indentation could be better here, otherwise looking good to me (including
previous patches in this series).
>  	if (ret < 0)
>  		goto error_unregister_dev;
>  
> 

Re: [PATCH 04/11] staging:iio:sca3000: Register same channels for device and buffer

[Jonathan Cameron]

On 10/12/14 22:35, Hartmut Knaack wrote:
> Lars-Peter Clausen schrieb am 26.11.2014 um 18:55:
>> In preparation for moving the buffer registration to the core make sure to
>> register the same channel array for the device and the buffer. Currently
>> the temperature channel is not registered for the buffer, setting its scan
>> index to -1 will make sure that it is skipped for the buffer.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Adjusted the indentation as Hartmut requested and applied to the togreg
branch of iio.git - pushed out as testing.
>> ---
>>  drivers/staging/iio/accel/sca3000_core.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
>> index cd46a61..9eedb93 100644
>> --- a/drivers/staging/iio/accel/sca3000_core.c
>> +++ b/drivers/staging/iio/accel/sca3000_core.c
>> @@ -459,6 +459,7 @@ static const struct iio_chan_spec sca3000_channels_with_temp[] = {
>>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
>>  			BIT(IIO_CHAN_INFO_OFFSET),
>> +		.scan_index = -1,
>>  	},
>>  };
>>  
>> @@ -1154,9 +1155,8 @@ static int sca3000_probe(struct spi_device *spi)
>>  	if (ret < 0)
>>  		return ret;
>>  
>> -	ret = iio_buffer_register(indio_dev,
>> -				  sca3000_channels,
>> -				  ARRAY_SIZE(sca3000_channels));
>> +	ret = iio_buffer_register(indio_dev, indio_dev->channels,
>> +		indio_dev->num_channels);
> Indentation could be better here, otherwise looking good to me (including
> previous patches in this series).
>>  	if (ret < 0)
>>  		goto error_unregister_dev;
>>  
>>
> 

[PATCH 05/11] staging:iio:dummy: Register same channels for device and buffer

[Lars-Peter Clausen]

In preparation for moving the buffer registration to the core make sure to
register the same channel array for the device and the buffer. Currently the
output voltage and the activity channels are not registered for the buffer,
setting its scan index to -1 will make sure that it is skipped for the
buffer.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/iio_simple_dummy.c        | 13 +++++--------
 drivers/staging/iio/iio_simple_dummy.h        |  3 +--
 drivers/staging/iio/iio_simple_dummy_buffer.c |  6 +++---
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/iio/iio_simple_dummy.c b/drivers/staging/iio/iio_simple_dummy.c
index 10a9e08..0b8611a 100644
--- a/drivers/staging/iio/iio_simple_dummy.c
+++ b/drivers/staging/iio/iio_simple_dummy.c
@@ -239,6 +239,7 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
 	{
 		.type = IIO_VOLTAGE,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.scan_index = -1, /* No buffer support */
 		.output = 1,
 		.indexed = 1,
 		.channel = 0,
@@ -248,7 +249,7 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_ENABLE) |
 			BIT(IIO_CHAN_INFO_CALIBHEIGHT),
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
-		.scan_index = -1,
+		.scan_index = -1, /* No buffer support */
 #ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
 		.event_spec = &step_detect_event,
 		.num_event_specs = 1,
@@ -259,6 +260,7 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_RUNNING,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+		.scan_index = -1, /* No buffer support */
 #ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
 		.event_spec = &iio_running_event,
 		.num_event_specs = 1,
@@ -269,6 +271,7 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_WALKING,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+		.scan_index = -1, /* No buffer support */
 #ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
 		.event_spec = &iio_walking_event,
 		.num_event_specs = 1,
@@ -638,13 +641,7 @@ static int iio_dummy_probe(int index)
 	if (ret < 0)
 		goto error_free_device;
 
-	/*
-	 * Configure buffered capture support and register the channels with the
-	 * buffer, but avoid the output channel being registered by reducing the
-	 * number of channels by 1.
-	 */
-	ret = iio_simple_dummy_configure_buffer(indio_dev,
-						iio_dummy_channels, 5);
+	ret = iio_simple_dummy_configure_buffer(indio_dev);
 	if (ret < 0)
 		goto error_unregister_events;
 
diff --git a/drivers/staging/iio/iio_simple_dummy.h b/drivers/staging/iio/iio_simple_dummy.h
index 3b714b4..af70126 100644
--- a/drivers/staging/iio/iio_simple_dummy.h
+++ b/drivers/staging/iio/iio_simple_dummy.h
@@ -114,8 +114,7 @@ enum iio_simple_dummy_scan_elements {
 };
 
 #ifdef CONFIG_IIO_SIMPLE_DUMMY_BUFFER
-int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev,
-	const struct iio_chan_spec *channels, unsigned int num_channels);
+int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev);
 void iio_simple_dummy_unconfigure_buffer(struct iio_dev *indio_dev);
 #else
 static inline int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev,
diff --git a/drivers/staging/iio/iio_simple_dummy_buffer.c b/drivers/staging/iio/iio_simple_dummy_buffer.c
index fd74f91..35d60d5 100644
--- a/drivers/staging/iio/iio_simple_dummy_buffer.c
+++ b/drivers/staging/iio/iio_simple_dummy_buffer.c
@@ -115,8 +115,7 @@ static const struct iio_buffer_setup_ops iio_simple_dummy_buffer_setup_ops = {
 	.predisable = &iio_triggered_buffer_predisable,
 };
 
-int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev,
-	const struct iio_chan_spec *channels, unsigned int num_channels)
+int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev)
 {
 	int ret;
 	struct iio_buffer *buffer;
@@ -173,7 +172,8 @@ int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev,
 	 */
 	indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
 
-	ret = iio_buffer_register(indio_dev, channels, num_channels);
+	ret = iio_buffer_register(indio_dev, indio_dev->channels,
+				  indio_dev->num_channels);
 	if (ret)
 		goto error_dealloc_pollfunc;
 
-- 
1.8.0

Re: [PATCH 05/11] staging:iio:dummy: Register same channels for device and buffer

[Daniel Baluta]

On Wed, Nov 26, 2014 at 7:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> In preparation for moving the buffer registration to the core make sure to
> register the same channel array for the device and the buffer. Currently the
> output voltage and the activity channels are not registered for the buffer,
> setting its scan index to -1 will make sure that it is skipped for the
> buffer.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/staging/iio/iio_simple_dummy.c        | 13 +++++--------
>  drivers/staging/iio/iio_simple_dummy.h        |  3 +--
>  drivers/staging/iio/iio_simple_dummy_buffer.c |  6 +++---
>  3 files changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/iio/iio_simple_dummy.c b/drivers/staging/iio/iio_simple_dummy.c
> index 10a9e08..0b8611a 100644
> --- a/drivers/staging/iio/iio_simple_dummy.c
> +++ b/drivers/staging/iio/iio_simple_dummy.c
> @@ -239,6 +239,7 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
>         {
>                 .type = IIO_VOLTAGE,
>                 .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +               .scan_index = -1, /* No buffer support */
>                 .output = 1,
>                 .indexed = 1,
>                 .channel = 0,
> @@ -248,7 +249,7 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
>                 .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_ENABLE) |
>                         BIT(IIO_CHAN_INFO_CALIBHEIGHT),
>                 .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> -               .scan_index = -1,
> +               .scan_index = -1, /* No buffer support */
>  #ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
>                 .event_spec = &step_detect_event,
>                 .num_event_specs = 1,
> @@ -259,6 +260,7 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
>                 .modified = 1,
>                 .channel2 = IIO_MOD_RUNNING,
>                 .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +               .scan_index = -1, /* No buffer support */
>  #ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
>                 .event_spec = &iio_running_event,
>                 .num_event_specs = 1,
> @@ -269,6 +271,7 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
>                 .modified = 1,
>                 .channel2 = IIO_MOD_WALKING,
>                 .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +               .scan_index = -1, /* No buffer support */
>  #ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
>                 .event_spec = &iio_walking_event,
>                 .num_event_specs = 1,
> @@ -638,13 +641,7 @@ static int iio_dummy_probe(int index)
>         if (ret < 0)
>                 goto error_free_device;
>
> -       /*
> -        * Configure buffered capture support and register the channels with the
> -        * buffer, but avoid the output channel being registered by reducing the
> -        * number of channels by 1.
> -        */
> -       ret = iio_simple_dummy_configure_buffer(indio_dev,
> -                                               iio_dummy_channels, 5);
> +       ret = iio_simple_dummy_configure_buffer(indio_dev);
>         if (ret < 0)
>                 goto error_unregister_events;
>
> diff --git a/drivers/staging/iio/iio_simple_dummy.h b/drivers/staging/iio/iio_simple_dummy.h
> index 3b714b4..af70126 100644
> --- a/drivers/staging/iio/iio_simple_dummy.h
> +++ b/drivers/staging/iio/iio_simple_dummy.h
> @@ -114,8 +114,7 @@ enum iio_simple_dummy_scan_elements {
>  };
>
>  #ifdef CONFIG_IIO_SIMPLE_DUMMY_BUFFER
> -int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev,
> -       const struct iio_chan_spec *channels, unsigned int num_channels);
> +int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev);
>  void iio_simple_dummy_unconfigure_buffer(struct iio_dev *indio_dev);
>  #else
>  static inline int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev,
> diff --git a/drivers/staging/iio/iio_simple_dummy_buffer.c b/drivers/staging/iio/iio_simple_dummy_buffer.c
> index fd74f91..35d60d5 100644
> --- a/drivers/staging/iio/iio_simple_dummy_buffer.c
> +++ b/drivers/staging/iio/iio_simple_dummy_buffer.c
> @@ -115,8 +115,7 @@ static const struct iio_buffer_setup_ops iio_simple_dummy_buffer_setup_ops = {
>         .predisable = &iio_triggered_buffer_predisable,
>  };
>
> -int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev,
> -       const struct iio_chan_spec *channels, unsigned int num_channels)
> +int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev)
>  {
>         int ret;
>         struct iio_buffer *buffer;
> @@ -173,7 +172,8 @@ int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev,
>          */
>         indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
>
> -       ret = iio_buffer_register(indio_dev, channels, num_channels);
> +       ret = iio_buffer_register(indio_dev, indio_dev->channels,
> +                                 indio_dev->num_channels);
>         if (ret)
>                 goto error_dealloc_pollfunc;
>

Looks good. I guess this can replace the following patch:

iio: dummy: set scan_index for unbuffered channels

http://marc.info/?l=linux-iio&m=141693190013582&w=2

thanks,
Daniel.

Re: [PATCH 05/11] staging:iio:dummy: Register same channels for device and buffer

[Jonathan Cameron]

On 04/12/14 14:27, Daniel Baluta wrote:
> On Wed, Nov 26, 2014 at 7:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> In preparation for moving the buffer registration to the core make sure to
>> register the same channel array for the device and the buffer. Currently the
>> output voltage and the activity channels are not registered for the buffer,
>> setting its scan index to -1 will make sure that it is skipped for the
>> buffer.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> ---
>>  drivers/staging/iio/iio_simple_dummy.c        | 13 +++++--------
>>  drivers/staging/iio/iio_simple_dummy.h        |  3 +--
>>  drivers/staging/iio/iio_simple_dummy_buffer.c |  6 +++---
>>  3 files changed, 9 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/staging/iio/iio_simple_dummy.c b/drivers/staging/iio/iio_simple_dummy.c
>> index 10a9e08..0b8611a 100644
>> --- a/drivers/staging/iio/iio_simple_dummy.c
>> +++ b/drivers/staging/iio/iio_simple_dummy.c
>> @@ -239,6 +239,7 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
>>         {
>>                 .type = IIO_VOLTAGE,
>>                 .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> +               .scan_index = -1, /* No buffer support */
>>                 .output = 1,
>>                 .indexed = 1,
>>                 .channel = 0,
>> @@ -248,7 +249,7 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
>>                 .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_ENABLE) |
>>                         BIT(IIO_CHAN_INFO_CALIBHEIGHT),
>>                 .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>> -               .scan_index = -1,
>> +               .scan_index = -1, /* No buffer support */
>>  #ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
>>                 .event_spec = &step_detect_event,
>>                 .num_event_specs = 1,
>> @@ -259,6 +260,7 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
>>                 .modified = 1,
>>                 .channel2 = IIO_MOD_RUNNING,
>>                 .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>> +               .scan_index = -1, /* No buffer support */
>>  #ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
>>                 .event_spec = &iio_running_event,
>>                 .num_event_specs = 1,
>> @@ -269,6 +271,7 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
>>                 .modified = 1,
>>                 .channel2 = IIO_MOD_WALKING,
>>                 .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>> +               .scan_index = -1, /* No buffer support */
>>  #ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
>>                 .event_spec = &iio_walking_event,
>>                 .num_event_specs = 1,
>> @@ -638,13 +641,7 @@ static int iio_dummy_probe(int index)
>>         if (ret < 0)
>>                 goto error_free_device;
>>
>> -       /*
>> -        * Configure buffered capture support and register the channels with the
>> -        * buffer, but avoid the output channel being registered by reducing the
>> -        * number of channels by 1.
>> -        */
>> -       ret = iio_simple_dummy_configure_buffer(indio_dev,
>> -                                               iio_dummy_channels, 5);
>> +       ret = iio_simple_dummy_configure_buffer(indio_dev);
>>         if (ret < 0)
>>                 goto error_unregister_events;
>>
>> diff --git a/drivers/staging/iio/iio_simple_dummy.h b/drivers/staging/iio/iio_simple_dummy.h
>> index 3b714b4..af70126 100644
>> --- a/drivers/staging/iio/iio_simple_dummy.h
>> +++ b/drivers/staging/iio/iio_simple_dummy.h
>> @@ -114,8 +114,7 @@ enum iio_simple_dummy_scan_elements {
>>  };
>>
>>  #ifdef CONFIG_IIO_SIMPLE_DUMMY_BUFFER
>> -int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev,
>> -       const struct iio_chan_spec *channels, unsigned int num_channels);
>> +int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev);
>>  void iio_simple_dummy_unconfigure_buffer(struct iio_dev *indio_dev);
>>  #else
>>  static inline int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev,
>> diff --git a/drivers/staging/iio/iio_simple_dummy_buffer.c b/drivers/staging/iio/iio_simple_dummy_buffer.c
>> index fd74f91..35d60d5 100644
>> --- a/drivers/staging/iio/iio_simple_dummy_buffer.c
>> +++ b/drivers/staging/iio/iio_simple_dummy_buffer.c
>> @@ -115,8 +115,7 @@ static const struct iio_buffer_setup_ops iio_simple_dummy_buffer_setup_ops = {
>>         .predisable = &iio_triggered_buffer_predisable,
>>  };
>>
>> -int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev,
>> -       const struct iio_chan_spec *channels, unsigned int num_channels)
>> +int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev)
>>  {
>>         int ret;
>>         struct iio_buffer *buffer;
>> @@ -173,7 +172,8 @@ int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev,
>>          */
>>         indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
>>
>> -       ret = iio_buffer_register(indio_dev, channels, num_channels);
>> +       ret = iio_buffer_register(indio_dev, indio_dev->channels,
>> +                                 indio_dev->num_channels);
>>         if (ret)
>>                 goto error_dealloc_pollfunc;
>>
> 
> Looks good. I guess this can replace the following patch:
> 
> iio: dummy: set scan_index for unbuffered channels
> 
> http://marc.info/?l=linux-iio&m=141693190013582&w=2
Thanks for pointing that out.  Applied this one to the togreg
branch of iio.git.


> 
> thanks,
> Daniel.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 06/11] iio: Move buffer registration to the core

[Lars-Peter Clausen]

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. 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@metafoo.de>
---
 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/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index b730864..d550ac7 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -264,16 +264,8 @@ static int tiadc_iio_buffered_hardware_setup(struct iio_dev *indio_dev,
 	indio_dev->setup_ops = setup_ops;
 	indio_dev->modes |= INDIO_BUFFER_HARDWARE;
 
-	ret = iio_buffer_register(indio_dev,
-				  indio_dev->channels,
-				  indio_dev->num_channels);
-	if (ret)
-		goto error_free_irq;
-
 	return 0;
 
-error_free_irq:
-	free_irq(irq, indio_dev);
 error_kfifo_free:
 	iio_kfifo_free(indio_dev->buffer);
 	return ret;
@@ -285,7 +277,6 @@ static void tiadc_iio_buffered_hardware_remove(struct iio_dev *indio_dev)
 
 	free_irq(adc_dev->mfd_tscadc->irq, indio_dev);
 	iio_kfifo_free(indio_dev->buffer);
-	iio_buffer_unregister(indio_dev);
 }
 
 
diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
index 5f0ea77..2631921 100644
--- a/drivers/iio/iio_core.h
+++ b/drivers/iio/iio_core.h
@@ -48,6 +48,8 @@ unsigned int iio_buffer_poll(struct file *filp,
 ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
 				      size_t n, loff_t *f_ps);
 
+int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev);
+void iio_buffer_free_sysfs(struct iio_dev *indio_dev);
 
 #define iio_buffer_poll_addr (&iio_buffer_poll)
 #define iio_buffer_read_first_n_outer_addr (&iio_buffer_read_first_n_outer)
@@ -60,6 +62,13 @@ void iio_buffer_wakeup_poll(struct iio_dev *indio_dev);
 #define iio_buffer_poll_addr NULL
 #define iio_buffer_read_first_n_outer_addr NULL
 
+static inline int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
+{
+	return 0;
+}
+
+static inline void iio_buffer_free_sysfs(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) {}
 
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)
 {
 	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/iio/industrialio-triggered-buffer.c b/drivers/iio/industrialio-triggered-buffer.c
index d6f54930..ac97685 100644
--- a/drivers/iio/industrialio-triggered-buffer.c
+++ b/drivers/iio/industrialio-triggered-buffer.c
@@ -78,16 +78,8 @@ int iio_triggered_buffer_setup(struct iio_dev *indio_dev,
 	/* Flag that polled ring buffering is possible */
 	indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
 
-	ret = iio_buffer_register(indio_dev,
-				  indio_dev->channels,
-				  indio_dev->num_channels);
-	if (ret)
-		goto error_dealloc_pollfunc;
-
 	return 0;
 
-error_dealloc_pollfunc:
-	iio_dealloc_pollfunc(indio_dev->pollfunc);
 error_kfifo_free:
 	iio_kfifo_free(indio_dev->buffer);
 error_ret:
@@ -101,7 +93,6 @@ EXPORT_SYMBOL(iio_triggered_buffer_setup);
  */
 void iio_triggered_buffer_cleanup(struct iio_dev *indio_dev)
 {
-	iio_buffer_unregister(indio_dev);
 	iio_dealloc_pollfunc(indio_dev->pollfunc);
 	iio_kfifo_free(indio_dev->buffer);
 }
diff --git a/drivers/staging/iio/accel/lis3l02dq_core.c b/drivers/staging/iio/accel/lis3l02dq_core.c
index f5e145c..b78c9c5 100644
--- a/drivers/staging/iio/accel/lis3l02dq_core.c
+++ b/drivers/staging/iio/accel/lis3l02dq_core.c
@@ -716,14 +716,6 @@ static int lis3l02dq_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = iio_buffer_register(indio_dev,
-				  lis3l02dq_channels,
-				  ARRAY_SIZE(lis3l02dq_channels));
-	if (ret) {
-		dev_err(&spi->dev, "failed to initialize the buffer\n");
-		goto error_unreg_buffer_funcs;
-	}
-
 	if (spi->irq) {
 		ret = request_threaded_irq(st->us->irq,
 					   &lis3l02dq_th,
@@ -732,7 +724,7 @@ static int lis3l02dq_probe(struct spi_device *spi)
 					   "lis3l02dq",
 					   indio_dev);
 		if (ret)
-			goto error_uninitialize_buffer;
+			goto error_unreg_buffer_funcs;
 
 		ret = lis3l02dq_probe_trigger(indio_dev);
 		if (ret)
@@ -756,8 +748,6 @@ error_remove_trigger:
 error_free_interrupt:
 	if (spi->irq)
 		free_irq(st->us->irq, indio_dev);
-error_uninitialize_buffer:
-	iio_buffer_unregister(indio_dev);
 error_unreg_buffer_funcs:
 	lis3l02dq_unconfigure_buffer(indio_dev);
 	return ret;
@@ -804,7 +794,6 @@ static int lis3l02dq_remove(struct spi_device *spi)
 		free_irq(st->us->irq, indio_dev);
 
 	lis3l02dq_remove_trigger(indio_dev);
-	iio_buffer_unregister(indio_dev);
 	lis3l02dq_unconfigure_buffer(indio_dev);
 
 	return 0;
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;
-
 	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;
diff --git a/drivers/staging/iio/iio_simple_dummy_buffer.c b/drivers/staging/iio/iio_simple_dummy_buffer.c
index 35d60d5..a2d72c1 100644
--- a/drivers/staging/iio/iio_simple_dummy_buffer.c
+++ b/drivers/staging/iio/iio_simple_dummy_buffer.c
@@ -172,15 +172,8 @@ int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev)
 	 */
 	indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
 
-	ret = iio_buffer_register(indio_dev, indio_dev->channels,
-				  indio_dev->num_channels);
-	if (ret)
-		goto error_dealloc_pollfunc;
-
 	return 0;
 
-error_dealloc_pollfunc:
-	iio_dealloc_pollfunc(indio_dev->pollfunc);
 error_free_buffer:
 	iio_kfifo_free(indio_dev->buffer);
 error_ret:
@@ -194,7 +187,6 @@ error_ret:
  */
 void iio_simple_dummy_unconfigure_buffer(struct iio_dev *indio_dev)
 {
-	iio_buffer_unregister(indio_dev);
 	iio_dealloc_pollfunc(indio_dev->pollfunc);
 	iio_kfifo_free(indio_dev->buffer);
 }
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
index aa6a368..c50b138 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -752,23 +752,16 @@ static int ad5933_probe(struct i2c_client *client,
 	if (ret)
 		goto error_disable_reg;
 
-	ret = iio_buffer_register(indio_dev, ad5933_channels,
-		ARRAY_SIZE(ad5933_channels));
-	if (ret)
-		goto error_unreg_ring;
-
 	ret = ad5933_setup(st);
 	if (ret)
-		goto error_uninitialize_ring;
+		goto error_unreg_ring;
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
-		goto error_uninitialize_ring;
+		goto error_unreg_ring;
 
 	return 0;
 
-error_uninitialize_ring:
-	iio_buffer_unregister(indio_dev);
 error_unreg_ring:
 	iio_kfifo_free(indio_dev->buffer);
 error_disable_reg:
@@ -784,7 +777,6 @@ static int ad5933_remove(struct i2c_client *client)
 	struct ad5933_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
-	iio_buffer_unregister(indio_dev);
 	iio_kfifo_free(indio_dev->buffer);
 	if (!IS_ERR(st->reg))
 		regulator_disable(st->reg);
diff --git a/drivers/staging/iio/meter/ade7758.h b/drivers/staging/iio/meter/ade7758.h
index 0731820..762d7dc 100644
--- a/drivers/staging/iio/meter/ade7758.h
+++ b/drivers/staging/iio/meter/ade7758.h
@@ -146,7 +146,6 @@ ssize_t ade7758_read_data_from_ring(struct device *dev,
 int ade7758_configure_ring(struct iio_dev *indio_dev);
 void ade7758_unconfigure_ring(struct iio_dev *indio_dev);
 
-void ade7758_uninitialize_ring(struct iio_dev *indio_dev);
 int ade7758_set_irq(struct device *dev, bool enable);
 
 int ade7758_spi_write_reg_8(struct device *dev,
diff --git a/drivers/staging/iio/meter/ade7758_core.c b/drivers/staging/iio/meter/ade7758_core.c
index abc6006..6e8b011 100644
--- a/drivers/staging/iio/meter/ade7758_core.c
+++ b/drivers/staging/iio/meter/ade7758_core.c
@@ -885,23 +885,15 @@ static int ade7758_probe(struct spi_device *spi)
 	if (ret)
 		goto error_free_tx;
 
-	ret = iio_buffer_register(indio_dev,
-				  &ade7758_channels[0],
-				  ARRAY_SIZE(ade7758_channels));
-	if (ret) {
-		dev_err(&spi->dev, "failed to initialize the ring\n");
-		goto error_unreg_ring_funcs;
-	}
-
 	/* Get the device into a sane initial state */
 	ret = ade7758_initial_setup(indio_dev);
 	if (ret)
-		goto error_uninitialize_ring;
+		goto error_unreg_ring_funcs;
 
 	if (spi->irq) {
 		ret = ade7758_probe_trigger(indio_dev);
 		if (ret)
-			goto error_uninitialize_ring;
+			goto error_unreg_ring_funcs;
 	}
 
 	ret = iio_device_register(indio_dev);
@@ -913,8 +905,6 @@ static int ade7758_probe(struct spi_device *spi)
 error_remove_trigger:
 	if (spi->irq)
 		ade7758_remove_trigger(indio_dev);
-error_uninitialize_ring:
-	ade7758_uninitialize_ring(indio_dev);
 error_unreg_ring_funcs:
 	ade7758_unconfigure_ring(indio_dev);
 error_free_tx:
@@ -932,7 +922,6 @@ static int ade7758_remove(struct spi_device *spi)
 	iio_device_unregister(indio_dev);
 	ade7758_stop_device(&indio_dev->dev);
 	ade7758_remove_trigger(indio_dev);
-	ade7758_uninitialize_ring(indio_dev);
 	ade7758_unconfigure_ring(indio_dev);
 	kfree(st->tx);
 	kfree(st->rx);
diff --git a/drivers/staging/iio/meter/ade7758_ring.c b/drivers/staging/iio/meter/ade7758_ring.c
index c0accf8..27c3ed6 100644
--- a/drivers/staging/iio/meter/ade7758_ring.c
+++ b/drivers/staging/iio/meter/ade7758_ring.c
@@ -181,8 +181,3 @@ error_iio_kfifo_free:
 	iio_kfifo_free(indio_dev->buffer);
 	return ret;
 }
-
-void ade7758_uninitialize_ring(struct iio_dev *indio_dev)
-{
-	iio_buffer_unregister(indio_dev);
-}
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) {}
 
-- 
1.8.0

Re: [PATCH 06/11] iio: Move buffer registration to the core

[Daniel Baluta]

On Wed, Nov 26, 2014 at 7:55 PM, Lars-Peter Clausen <lars@metafoo.de> 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. 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

s/summery/summary

> 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@metafoo.de>
> ---

<snip>

> diff --git a/drivers/iio/industrialio-triggered-buffer.c b/drivers/iio/industrialio-triggered-buffer.c
> index d6f54930..ac97685 100644
> --- a/drivers/iio/industrialio-triggered-buffer.c
> +++ b/drivers/iio/industrialio-triggered-buffer.c
> @@ -78,16 +78,8 @@ int iio_triggered_buffer_setup(struct iio_dev *indio_dev,

You should also update the comment for iio_triggered_buffer_setup. This function
doesn't register the buffer with IIO core anymore.


>         /* Flag that polled ring buffering is possible */
>         indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
>
> -       ret = iio_buffer_register(indio_dev,
> -                                 indio_dev->channels,
> -                                 indio_dev->num_channels);
> -       if (ret)
> -               goto error_dealloc_pollfunc;
> -
>         return 0;

<snip>

thanks,
Daniel.

Re: [PATCH 06/11] iio: Move buffer registration to the core

[Jonathan Cameron]

On 04/12/14 14:23, Daniel Baluta wrote:
> On Wed, Nov 26, 2014 at 7:55 PM, Lars-Peter Clausen <lars@metafoo.de> 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. 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
> 
> s/summery/summary
> 
>> 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@metafoo.de>
>> ---
> 
> <snip>
> 
>> diff --git a/drivers/iio/industrialio-triggered-buffer.c b/drivers/iio/industrialio-triggered-buffer.c
>> index d6f54930..ac97685 100644
>> --- a/drivers/iio/industrialio-triggered-buffer.c
>> +++ b/drivers/iio/industrialio-triggered-buffer.c
>> @@ -78,16 +78,8 @@ int iio_triggered_buffer_setup(struct iio_dev *indio_dev,
> 
> You should also update the comment for iio_triggered_buffer_setup. This function
> doesn't register the buffer with IIO core anymore.
> 
I took out the relevant sentence whilst applying the patch.

Thanks for your reviews of these Daniel.

Jonathan
> 
>>         /* Flag that polled ring buffering is possible */
>>         indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
>>
>> -       ret = iio_buffer_register(indio_dev,
>> -                                 indio_dev->channels,
>> -                                 indio_dev->num_channels);
>> -       if (ret)
>> -               goto error_dealloc_pollfunc;
>> -
>>         return 0;
> 
> <snip>
> 
> thanks,
> Daniel.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 06/11] iio: Move buffer registration to the core

[Jonathan Cameron]

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@metafoo.de>
> ---
>  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) {}
>  
> 

[PATCH 07/11] iio: Remove get_bytes_per_datum() from iio_buffer_access_funcs

[Lars-Peter Clausen]

There haven't been any users of the get_bytes_per_datum() callback for a
while. The core assumes that the number of bytes per datum can be calculated
based on the enabled channels and the storage size of the channel and
iio_compute_scan_bytes() is used to compute this number. So remove the
callback.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/kfifo_buf.c                    | 6 ------
 drivers/staging/iio/Documentation/ring.txt | 4 ++--
 drivers/staging/iio/accel/sca3000_ring.c   | 7 -------
 include/linux/iio/buffer.h                 | 2 --
 4 files changed, 2 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
index 7134e8a..1258b4e 100644
--- a/drivers/iio/kfifo_buf.c
+++ b/drivers/iio/kfifo_buf.c
@@ -66,11 +66,6 @@ static struct attribute_group iio_kfifo_attribute_group = {
 	.name = "buffer",
 };
 
-static int iio_get_bytes_per_datum_kfifo(struct iio_buffer *r)
-{
-	return r->bytes_per_datum;
-}
-
 static int iio_mark_update_needed_kfifo(struct iio_buffer *r)
 {
 	struct iio_kfifo *kf = iio_to_kfifo(r);
@@ -159,7 +154,6 @@ static const struct iio_buffer_access_funcs kfifo_access_funcs = {
 	.read_first_n = &iio_read_first_n_kfifo,
 	.data_available = iio_kfifo_buf_data_available,
 	.request_update = &iio_request_update_kfifo,
-	.get_bytes_per_datum = &iio_get_bytes_per_datum_kfifo,
 	.set_bytes_per_datum = &iio_set_bytes_per_datum_kfifo,
 	.get_length = &iio_get_length_kfifo,
 	.set_length = &iio_set_length_kfifo,
diff --git a/drivers/staging/iio/Documentation/ring.txt b/drivers/staging/iio/Documentation/ring.txt
index e1da433..434d63a 100644
--- a/drivers/staging/iio/Documentation/ring.txt
+++ b/drivers/staging/iio/Documentation/ring.txt
@@ -39,8 +39,8 @@ request_update
   If parameters have changed that require reinitialization or configuration of
   the buffer this will trigger it.
 
-get_bytes_per_datum, set_bytes_per_datum
-  Get/set the number of bytes for a complete scan. (All samples + timestamp)
+set_bytes_per_datum
+  Set the number of bytes for a complete scan. (All samples + timestamp)
 
 get_length / set_length
   Get/set the number of complete scans that may be held by the buffer.
diff --git a/drivers/staging/iio/accel/sca3000_ring.c b/drivers/staging/iio/accel/sca3000_ring.c
index 1578276..aa0e5d8 100644
--- a/drivers/staging/iio/accel/sca3000_ring.c
+++ b/drivers/staging/iio/accel/sca3000_ring.c
@@ -135,12 +135,6 @@ static int sca3000_ring_get_length(struct iio_buffer *r)
 	return 64;
 }
 
-/* only valid if resolution is kept at 11bits */
-static int sca3000_ring_get_bytes_per_datum(struct iio_buffer *r)
-{
-	return 6;
-}
-
 static bool sca3000_ring_buf_data_available(struct iio_buffer *r)
 {
 	return r->stufftoread;
@@ -278,7 +272,6 @@ static void sca3000_ring_release(struct iio_buffer *r)
 static const struct iio_buffer_access_funcs sca3000_ring_access_funcs = {
 	.read_first_n = &sca3000_read_first_n_hw_rb,
 	.get_length = &sca3000_ring_get_length,
-	.get_bytes_per_datum = &sca3000_ring_get_bytes_per_datum,
 	.data_available = sca3000_ring_buf_data_available,
 	.release = sca3000_ring_release,
 };
diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
index b0e006c..79cdb3d 100644
--- a/include/linux/iio/buffer.h
+++ b/include/linux/iio/buffer.h
@@ -25,7 +25,6 @@ struct iio_buffer;
  *			available.
  * @request_update:	if a parameter change has been marked, update underlying
  *			storage.
- * @get_bytes_per_datum:get current bytes per datum
  * @set_bytes_per_datum:set number of bytes per datum
  * @get_length:		get number of datums in buffer
  * @set_length:		set number of datums in buffer
@@ -49,7 +48,6 @@ struct iio_buffer_access_funcs {
 
 	int (*request_update)(struct iio_buffer *buffer);
 
-	int (*get_bytes_per_datum)(struct iio_buffer *buffer);
 	int (*set_bytes_per_datum)(struct iio_buffer *buffer, size_t bpd);
 	int (*get_length)(struct iio_buffer *buffer);
 	int (*set_length)(struct iio_buffer *buffer, int length);
-- 
1.8.0

Re: [PATCH 07/11] iio: Remove get_bytes_per_datum() from iio_buffer_access_funcs

[Jonathan Cameron]

On 26/11/14 17:55, Lars-Peter Clausen wrote:
> There haven't been any users of the get_bytes_per_datum() callback for a
> while. The core assumes that the number of bytes per datum can be calculated
> based on the enabled channels and the storage size of the channel and
> iio_compute_scan_bytes() is used to compute this number. So remove the
> callback.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied to the togreg branch of iio.git - etc etc.
> ---
>  drivers/iio/kfifo_buf.c                    | 6 ------
>  drivers/staging/iio/Documentation/ring.txt | 4 ++--
>  drivers/staging/iio/accel/sca3000_ring.c   | 7 -------
>  include/linux/iio/buffer.h                 | 2 --
>  4 files changed, 2 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
> index 7134e8a..1258b4e 100644
> --- a/drivers/iio/kfifo_buf.c
> +++ b/drivers/iio/kfifo_buf.c
> @@ -66,11 +66,6 @@ static struct attribute_group iio_kfifo_attribute_group = {
>  	.name = "buffer",
>  };
>  
> -static int iio_get_bytes_per_datum_kfifo(struct iio_buffer *r)
> -{
> -	return r->bytes_per_datum;
> -}
> -
>  static int iio_mark_update_needed_kfifo(struct iio_buffer *r)
>  {
>  	struct iio_kfifo *kf = iio_to_kfifo(r);
> @@ -159,7 +154,6 @@ static const struct iio_buffer_access_funcs kfifo_access_funcs = {
>  	.read_first_n = &iio_read_first_n_kfifo,
>  	.data_available = iio_kfifo_buf_data_available,
>  	.request_update = &iio_request_update_kfifo,
> -	.get_bytes_per_datum = &iio_get_bytes_per_datum_kfifo,
>  	.set_bytes_per_datum = &iio_set_bytes_per_datum_kfifo,
>  	.get_length = &iio_get_length_kfifo,
>  	.set_length = &iio_set_length_kfifo,
> diff --git a/drivers/staging/iio/Documentation/ring.txt b/drivers/staging/iio/Documentation/ring.txt
> index e1da433..434d63a 100644
> --- a/drivers/staging/iio/Documentation/ring.txt
> +++ b/drivers/staging/iio/Documentation/ring.txt
> @@ -39,8 +39,8 @@ request_update
>    If parameters have changed that require reinitialization or configuration of
>    the buffer this will trigger it.
>  
> -get_bytes_per_datum, set_bytes_per_datum
> -  Get/set the number of bytes for a complete scan. (All samples + timestamp)
> +set_bytes_per_datum
> +  Set the number of bytes for a complete scan. (All samples + timestamp)
>  
>  get_length / set_length
>    Get/set the number of complete scans that may be held by the buffer.
> diff --git a/drivers/staging/iio/accel/sca3000_ring.c b/drivers/staging/iio/accel/sca3000_ring.c
> index 1578276..aa0e5d8 100644
> --- a/drivers/staging/iio/accel/sca3000_ring.c
> +++ b/drivers/staging/iio/accel/sca3000_ring.c
> @@ -135,12 +135,6 @@ static int sca3000_ring_get_length(struct iio_buffer *r)
>  	return 64;
>  }
>  
> -/* only valid if resolution is kept at 11bits */
> -static int sca3000_ring_get_bytes_per_datum(struct iio_buffer *r)
> -{
> -	return 6;
> -}
> -
>  static bool sca3000_ring_buf_data_available(struct iio_buffer *r)
>  {
>  	return r->stufftoread;
> @@ -278,7 +272,6 @@ static void sca3000_ring_release(struct iio_buffer *r)
>  static const struct iio_buffer_access_funcs sca3000_ring_access_funcs = {
>  	.read_first_n = &sca3000_read_first_n_hw_rb,
>  	.get_length = &sca3000_ring_get_length,
> -	.get_bytes_per_datum = &sca3000_ring_get_bytes_per_datum,
>  	.data_available = sca3000_ring_buf_data_available,
>  	.release = sca3000_ring_release,
>  };
> diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
> index b0e006c..79cdb3d 100644
> --- a/include/linux/iio/buffer.h
> +++ b/include/linux/iio/buffer.h
> @@ -25,7 +25,6 @@ struct iio_buffer;
>   *			available.
>   * @request_update:	if a parameter change has been marked, update underlying
>   *			storage.
> - * @get_bytes_per_datum:get current bytes per datum
>   * @set_bytes_per_datum:set number of bytes per datum
>   * @get_length:		get number of datums in buffer
>   * @set_length:		set number of datums in buffer
> @@ -49,7 +48,6 @@ struct iio_buffer_access_funcs {
>  
>  	int (*request_update)(struct iio_buffer *buffer);
>  
> -	int (*get_bytes_per_datum)(struct iio_buffer *buffer);
>  	int (*set_bytes_per_datum)(struct iio_buffer *buffer, size_t bpd);
>  	int (*get_length)(struct iio_buffer *buffer);
>  	int (*set_length)(struct iio_buffer *buffer, int length);
> 

[PATCH 08/11] iio: buffer: Move iio_buffer_alloc_sysfs and iio_buffer_free_sysfs

[Lars-Peter Clausen]

The next patch will introduce new dependencies in iio_buffer_alloc_sysfs()
to functions which are currently defined after iio_buffer_alloc_sysfs(). To
avoid forward declarations move both iio_buffer_alloc_sysfs() and
iio_buffer_free_sysfs() after those function.

This is split into two patches one moving the functions and one adding the
dependencies to make review of the actual changes easier.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-buffer.c | 184 +++++++++++++++++++-------------------
 1 file changed, 91 insertions(+), 93 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index e1906ad..cdc2482 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -383,99 +383,6 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev,
 	return ret;
 }
 
-static const char * const iio_scan_elements_group_name = "scan_elements";
-
-int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
-{
-	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;
-
-	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;
-	if (channels) {
-		/* new magic */
-		for (i = 0; i < indio_dev->num_channels; i++) {
-			if (channels[i].scan_index < 0)
-				continue;
-
-			/* Establish necessary mask length */
-			if (channels[i].scan_index >
-			    (int)indio_dev->masklength - 1)
-				indio_dev->masklength
-					= channels[i].scan_index + 1;
-
-			ret = iio_buffer_add_channel_sysfs(indio_dev,
-							 &channels[i]);
-			if (ret < 0)
-				goto error_cleanup_dynamic;
-			attrcount += ret;
-			if (channels[i].type == IIO_TIMESTAMP)
-				indio_dev->scan_index_timestamp =
-					channels[i].scan_index;
-		}
-		if (indio_dev->masklength && buffer->scan_mask == NULL) {
-			buffer->scan_mask = kcalloc(BITS_TO_LONGS(indio_dev->masklength),
-						    sizeof(*buffer->scan_mask),
-						    GFP_KERNEL);
-			if (buffer->scan_mask == NULL) {
-				ret = -ENOMEM;
-				goto error_cleanup_dynamic;
-			}
-		}
-	}
-
-	buffer->scan_el_group.name = iio_scan_elements_group_name;
-
-	buffer->scan_el_group.attrs = kcalloc(attrcount + 1,
-					      sizeof(buffer->scan_el_group.attrs[0]),
-					      GFP_KERNEL);
-	if (buffer->scan_el_group.attrs == NULL) {
-		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)
-		buffer->scan_el_group.attrs[attrn++] = &p->dev_attr.attr;
-	indio_dev->groups[indio_dev->groupcounter++] = &buffer->scan_el_group;
-
-	return 0;
-
-error_free_scan_mask:
-	kfree(buffer->scan_mask);
-error_cleanup_dynamic:
-	iio_free_chan_devattr_list(&buffer->scan_el_dev_attr_list);
-
-	return ret;
-}
-
-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);
-}
 
 ssize_t iio_buffer_read_length(struct device *dev,
 			       struct device_attribute *attr,
@@ -855,6 +762,97 @@ done:
 }
 EXPORT_SYMBOL(iio_buffer_store_enable);
 
+static const char * const iio_scan_elements_group_name = "scan_elements";
+
+int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
+{
+	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->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;
+	if (channels) {
+		/* new magic */
+		for (i = 0; i < indio_dev->num_channels; i++) {
+			if (channels[i].scan_index < 0)
+				continue;
+
+			/* Establish necessary mask length */
+			if (channels[i].scan_index >
+			    (int)indio_dev->masklength - 1)
+				indio_dev->masklength
+					= channels[i].scan_index + 1;
+
+			ret = iio_buffer_add_channel_sysfs(indio_dev,
+							 &channels[i]);
+			if (ret < 0)
+				goto error_cleanup_dynamic;
+			attrcount += ret;
+			if (channels[i].type == IIO_TIMESTAMP)
+				indio_dev->scan_index_timestamp =
+					channels[i].scan_index;
+		}
+		if (indio_dev->masklength && buffer->scan_mask == NULL) {
+			buffer->scan_mask = kcalloc(BITS_TO_LONGS(indio_dev->masklength),
+						    sizeof(*buffer->scan_mask),
+						    GFP_KERNEL);
+			if (buffer->scan_mask == NULL) {
+				ret = -ENOMEM;
+				goto error_cleanup_dynamic;
+			}
+		}
+	}
+
+	buffer->scan_el_group.name = iio_scan_elements_group_name;
+
+	buffer->scan_el_group.attrs = kcalloc(attrcount + 1,
+					      sizeof(buffer->scan_el_group.attrs[0]),
+					      GFP_KERNEL);
+	if (buffer->scan_el_group.attrs == NULL) {
+		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)
+		buffer->scan_el_group.attrs[attrn++] = &p->dev_attr.attr;
+	indio_dev->groups[indio_dev->groupcounter++] = &buffer->scan_el_group;
+
+	return 0;
+
+error_free_scan_mask:
+	kfree(buffer->scan_mask);
+error_cleanup_dynamic:
+	iio_free_chan_devattr_list(&buffer->scan_el_dev_attr_list);
+
+	return ret;
+}
+
+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);
+}
+
 /**
  * iio_validate_scan_mask_onehot() - Validates that exactly one channel is selected
  * @indio_dev: the iio device
-- 
1.8.0

Re: [PATCH 08/11] iio: buffer: Move iio_buffer_alloc_sysfs and iio_buffer_free_sysfs

[Jonathan Cameron]

On 26/11/14 17:55, Lars-Peter Clausen wrote:
> The next patch will introduce new dependencies in iio_buffer_alloc_sysfs()
> to functions which are currently defined after iio_buffer_alloc_sysfs(). To
> avoid forward declarations move both iio_buffer_alloc_sysfs() and
> iio_buffer_free_sysfs() after those function.
> 
> This is split into two patches one moving the functions and one adding the
> dependencies to make review of the actual changes easier.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied though naturally I'm now kind of regretting renaming those functions earlier
as I had to chase that change through here. Not exactly challenging but I could
have been lazy and renamed them as a follow up patch ;)

J
> ---
>  drivers/iio/industrialio-buffer.c | 184 +++++++++++++++++++-------------------
>  1 file changed, 91 insertions(+), 93 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index e1906ad..cdc2482 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -383,99 +383,6 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev,
>  	return ret;
>  }
>  
> -static const char * const iio_scan_elements_group_name = "scan_elements";
> -
> -int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
> -{
> -	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;
> -
> -	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;
> -	if (channels) {
> -		/* new magic */
> -		for (i = 0; i < indio_dev->num_channels; i++) {
> -			if (channels[i].scan_index < 0)
> -				continue;
> -
> -			/* Establish necessary mask length */
> -			if (channels[i].scan_index >
> -			    (int)indio_dev->masklength - 1)
> -				indio_dev->masklength
> -					= channels[i].scan_index + 1;
> -
> -			ret = iio_buffer_add_channel_sysfs(indio_dev,
> -							 &channels[i]);
> -			if (ret < 0)
> -				goto error_cleanup_dynamic;
> -			attrcount += ret;
> -			if (channels[i].type == IIO_TIMESTAMP)
> -				indio_dev->scan_index_timestamp =
> -					channels[i].scan_index;
> -		}
> -		if (indio_dev->masklength && buffer->scan_mask == NULL) {
> -			buffer->scan_mask = kcalloc(BITS_TO_LONGS(indio_dev->masklength),
> -						    sizeof(*buffer->scan_mask),
> -						    GFP_KERNEL);
> -			if (buffer->scan_mask == NULL) {
> -				ret = -ENOMEM;
> -				goto error_cleanup_dynamic;
> -			}
> -		}
> -	}
> -
> -	buffer->scan_el_group.name = iio_scan_elements_group_name;
> -
> -	buffer->scan_el_group.attrs = kcalloc(attrcount + 1,
> -					      sizeof(buffer->scan_el_group.attrs[0]),
> -					      GFP_KERNEL);
> -	if (buffer->scan_el_group.attrs == NULL) {
> -		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)
> -		buffer->scan_el_group.attrs[attrn++] = &p->dev_attr.attr;
> -	indio_dev->groups[indio_dev->groupcounter++] = &buffer->scan_el_group;
> -
> -	return 0;
> -
> -error_free_scan_mask:
> -	kfree(buffer->scan_mask);
> -error_cleanup_dynamic:
> -	iio_free_chan_devattr_list(&buffer->scan_el_dev_attr_list);
> -
> -	return ret;
> -}
> -
> -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);
> -}
>  
>  ssize_t iio_buffer_read_length(struct device *dev,
>  			       struct device_attribute *attr,
> @@ -855,6 +762,97 @@ done:
>  }
>  EXPORT_SYMBOL(iio_buffer_store_enable);
>  
> +static const char * const iio_scan_elements_group_name = "scan_elements";
> +
> +int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
> +{
> +	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->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;
> +	if (channels) {
> +		/* new magic */
> +		for (i = 0; i < indio_dev->num_channels; i++) {
> +			if (channels[i].scan_index < 0)
> +				continue;
> +
> +			/* Establish necessary mask length */
> +			if (channels[i].scan_index >
> +			    (int)indio_dev->masklength - 1)
> +				indio_dev->masklength
> +					= channels[i].scan_index + 1;
> +
> +			ret = iio_buffer_add_channel_sysfs(indio_dev,
> +							 &channels[i]);
> +			if (ret < 0)
> +				goto error_cleanup_dynamic;
> +			attrcount += ret;
> +			if (channels[i].type == IIO_TIMESTAMP)
> +				indio_dev->scan_index_timestamp =
> +					channels[i].scan_index;
> +		}
> +		if (indio_dev->masklength && buffer->scan_mask == NULL) {
> +			buffer->scan_mask = kcalloc(BITS_TO_LONGS(indio_dev->masklength),
> +						    sizeof(*buffer->scan_mask),
> +						    GFP_KERNEL);
> +			if (buffer->scan_mask == NULL) {
> +				ret = -ENOMEM;
> +				goto error_cleanup_dynamic;
> +			}
> +		}
> +	}
> +
> +	buffer->scan_el_group.name = iio_scan_elements_group_name;
> +
> +	buffer->scan_el_group.attrs = kcalloc(attrcount + 1,
> +					      sizeof(buffer->scan_el_group.attrs[0]),
> +					      GFP_KERNEL);
> +	if (buffer->scan_el_group.attrs == NULL) {
> +		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)
> +		buffer->scan_el_group.attrs[attrn++] = &p->dev_attr.attr;
> +	indio_dev->groups[indio_dev->groupcounter++] = &buffer->scan_el_group;
> +
> +	return 0;
> +
> +error_free_scan_mask:
> +	kfree(buffer->scan_mask);
> +error_cleanup_dynamic:
> +	iio_free_chan_devattr_list(&buffer->scan_el_dev_attr_list);
> +
> +	return ret;
> +}
> +
> +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);
> +}
> +
>  /**
>   * iio_validate_scan_mask_onehot() - Validates that exactly one channel is selected
>   * @indio_dev: the iio device
> 

[PATCH 09/11] iio: buffer: Allocate standard attributes in the core

[Lars-Peter Clausen]

All buffers want at least the length and the enable attribute. Move the
creation of those attributes to the core instead of having to do this in
each individual buffer implementation. This allows us to get rid of some
boiler-plate code.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-buffer.c        | 55 +++++++++++++++++++++-----------
 drivers/iio/kfifo_buf.c                  | 15 ---------
 drivers/staging/iio/accel/sca3000_ring.c | 14 ++------
 include/linux/iio/buffer.h               | 37 ++-------------------
 4 files changed, 40 insertions(+), 81 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index cdc2482..a4d3ff6 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -383,10 +383,8 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev,
 	return ret;
 }
 
-
-ssize_t iio_buffer_read_length(struct device *dev,
-			       struct device_attribute *attr,
-			       char *buf)
+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;
@@ -397,12 +395,9 @@ ssize_t iio_buffer_read_length(struct device *dev,
 
 	return 0;
 }
-EXPORT_SYMBOL(iio_buffer_read_length);
 
-ssize_t iio_buffer_write_length(struct device *dev,
-				struct device_attribute *attr,
-				const char *buf,
-				size_t len)
+static ssize_t iio_buffer_write_length(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct iio_buffer *buffer = indio_dev->buffer;
@@ -429,16 +424,13 @@ ssize_t iio_buffer_write_length(struct device *dev,
 
 	return ret ? ret : len;
 }
-EXPORT_SYMBOL(iio_buffer_write_length);
 
-ssize_t iio_buffer_show_enable(struct device *dev,
-			       struct device_attribute *attr,
-			       char *buf)
+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);
 	return sprintf(buf, "%d\n", iio_buffer_is_active(indio_dev->buffer));
 }
-EXPORT_SYMBOL(iio_buffer_show_enable);
 
 static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
 				const unsigned long *mask, bool timestamp)
@@ -725,10 +717,8 @@ out_unlock:
 }
 EXPORT_SYMBOL_GPL(iio_update_buffers);
 
-ssize_t iio_buffer_store_enable(struct device *dev,
-				struct device_attribute *attr,
-				const char *buf,
-				size_t len)
+static ssize_t iio_buffer_store_enable(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t len)
 {
 	int ret;
 	bool requested_state;
@@ -760,10 +750,14 @@ done:
 	mutex_unlock(&indio_dev->mlock);
 	return (ret < 0) ? ret : len;
 }
-EXPORT_SYMBOL(iio_buffer_store_enable);
 
 static const char * const iio_scan_elements_group_name = "scan_elements";
 
+static DEVICE_ATTR(length, S_IRUGO | S_IWUSR, iio_buffer_read_length,
+	iio_buffer_write_length);
+static DEVICE_ATTR(enable, S_IRUGO | S_IWUSR,
+	iio_buffer_show_enable, iio_buffer_store_enable);
+
 int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
 {
 	struct iio_dev_attr *p;
@@ -775,6 +769,27 @@ int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
 	if (!buffer)
 		return 0;
 
+	attrcount = 0;
+	if (buffer->attrs) {
+		while (buffer->attrs[attrcount] != NULL)
+			attrcount++;
+	}
+
+	buffer->buffer_group.name = "buffer";
+	buffer->buffer_group.attrs = kcalloc(attrcount + 3,
+			sizeof(*buffer->buffer_group.attrs), GFP_KERNEL);
+	if (!buffer->buffer_group.attrs)
+		return -ENOMEM;
+
+	buffer->buffer_group.attrs[0] = &dev_attr_length.attr;
+	buffer->buffer_group.attrs[1] = &dev_attr_enable.attr;
+	if (buffer->attrs)
+		memcpy(&buffer->buffer_group.attrs[2], buffer->attrs,
+			sizeof(*&buffer->buffer_group.attrs) * (attrcount - 2));
+	buffer->buffer_group.attrs[attrcount+2] = NULL;
+
+	indio_dev->groups[indio_dev->groupcounter++] = &buffer->buffer_group;
+
 	if (buffer->scan_el_attrs != NULL) {
 		attr = buffer->scan_el_attrs->attrs;
 		while (*attr++ != NULL)
@@ -839,6 +854,7 @@ error_free_scan_mask:
 	kfree(buffer->scan_mask);
 error_cleanup_dynamic:
 	iio_free_chan_devattr_list(&buffer->scan_el_dev_attr_list);
+	kfree(indio_dev->buffer->buffer_group.attrs);
 
 	return ret;
 }
@@ -849,6 +865,7 @@ void iio_buffer_free_sysfs(struct iio_dev *indio_dev)
 		return;
 
 	kfree(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);
 }
diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
index 1258b4e..3b0a3bc 100644
--- a/drivers/iio/kfifo_buf.c
+++ b/drivers/iio/kfifo_buf.c
@@ -52,20 +52,6 @@ static int iio_get_length_kfifo(struct iio_buffer *r)
 	return r->length;
 }
 
-static IIO_BUFFER_ENABLE_ATTR;
-static IIO_BUFFER_LENGTH_ATTR;
-
-static struct attribute *iio_kfifo_attributes[] = {
-	&dev_attr_length.attr,
-	&dev_attr_enable.attr,
-	NULL,
-};
-
-static struct attribute_group iio_kfifo_attribute_group = {
-	.attrs = iio_kfifo_attributes,
-	.name = "buffer",
-};
-
 static int iio_mark_update_needed_kfifo(struct iio_buffer *r)
 {
 	struct iio_kfifo *kf = iio_to_kfifo(r);
@@ -169,7 +155,6 @@ struct iio_buffer *iio_kfifo_allocate(struct iio_dev *indio_dev)
 		return NULL;
 	kf->update_needed = true;
 	iio_buffer_init(&kf->buffer);
-	kf->buffer.attrs = &iio_kfifo_attribute_group;
 	kf->buffer.access = &kfifo_access_funcs;
 	kf->buffer.length = 2;
 	mutex_init(&kf->user_lock);
diff --git a/drivers/staging/iio/accel/sca3000_ring.c b/drivers/staging/iio/accel/sca3000_ring.c
index aa0e5d8..f2f260e 100644
--- a/drivers/staging/iio/accel/sca3000_ring.c
+++ b/drivers/staging/iio/accel/sca3000_ring.c
@@ -140,9 +140,6 @@ static bool sca3000_ring_buf_data_available(struct iio_buffer *r)
 	return r->stufftoread;
 }
 
-static IIO_BUFFER_ENABLE_ATTR;
-static IIO_BUFFER_LENGTH_ATTR;
-
 /**
  * sca3000_query_ring_int() is the hardware ring status interrupt enabled
  **/
@@ -232,20 +229,13 @@ static IIO_DEVICE_ATTR(in_accel_scale,
  * only apply to the ring buffer.  At all times full rate and accuracy
  * is available via direct reading from registers.
  */
-static struct attribute *sca3000_ring_attributes[] = {
-	&dev_attr_length.attr,
-	&dev_attr_enable.attr,
+static const struct attribute *sca3000_ring_attributes[] = {
 	&iio_dev_attr_50_percent.dev_attr.attr,
 	&iio_dev_attr_75_percent.dev_attr.attr,
 	&iio_dev_attr_in_accel_scale.dev_attr.attr,
 	NULL,
 };
 
-static struct attribute_group sca3000_ring_attr = {
-	.attrs = sca3000_ring_attributes,
-	.name = "buffer",
-};
-
 static struct iio_buffer *sca3000_rb_allocate(struct iio_dev *indio_dev)
 {
 	struct iio_buffer *buf;
@@ -258,7 +248,7 @@ static struct iio_buffer *sca3000_rb_allocate(struct iio_dev *indio_dev)
 	ring->private = indio_dev;
 	buf = &ring->buf;
 	buf->stufftoread = 0;
-	buf->attrs = &sca3000_ring_attr;
+	buf->attrs = sca3000_ring_attributes;
 	iio_buffer_init(buf);
 
 	return buf;
diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
index 79cdb3d..16b7663 100644
--- a/include/linux/iio/buffer.h
+++ b/include/linux/iio/buffer.h
@@ -83,10 +83,11 @@ struct iio_buffer {
 	bool					scan_timestamp;
 	const struct iio_buffer_access_funcs	*access;
 	struct list_head			scan_el_dev_attr_list;
+	struct attribute_group			buffer_group;
 	struct attribute_group			scan_el_group;
 	wait_queue_head_t			pollq;
 	bool					stufftoread;
-	const struct attribute_group *attrs;
+	const struct attribute			**attrs;
 	struct list_head			demux_list;
 	void					*demux_bounce;
 	struct list_head			buffer_list;
@@ -148,40 +149,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_read_length() - attr func to get number of datums in the buffer
- **/
-ssize_t iio_buffer_read_length(struct device *dev,
-			       struct device_attribute *attr,
-			       char *buf);
-/**
- * iio_buffer_write_length() - attr func to set number of datums in the buffer
- **/
-ssize_t iio_buffer_write_length(struct device *dev,
-			      struct device_attribute *attr,
-			      const char *buf,
-			      size_t len);
-/**
- * iio_buffer_store_enable() - attr to turn the buffer on
- **/
-ssize_t iio_buffer_store_enable(struct device *dev,
-				struct device_attribute *attr,
-				const char *buf,
-				size_t len);
-/**
- * iio_buffer_show_enable() - attr to see if the buffer is on
- **/
-ssize_t iio_buffer_show_enable(struct device *dev,
-			       struct device_attribute *attr,
-			       char *buf);
-#define IIO_BUFFER_LENGTH_ATTR DEVICE_ATTR(length, S_IRUGO | S_IWUSR,	\
-					   iio_buffer_read_length,	\
-					   iio_buffer_write_length)
-
-#define IIO_BUFFER_ENABLE_ATTR DEVICE_ATTR(enable, S_IRUGO | S_IWUSR,	\
-					   iio_buffer_show_enable,	\
-					   iio_buffer_store_enable)
-
 bool iio_validate_scan_mask_onehot(struct iio_dev *indio_dev,
 	const unsigned long *mask);
 
-- 
1.8.0

Re: [PATCH 09/11] iio: buffer: Allocate standard attributes in the core

[Hartmut Knaack]

Lars-Peter Clausen schrieb am 26.11.2014 um 18:55:
> All buffers want at least the length and the enable attribute. Move the
> creation of those attributes to the core instead of having to do this in
> each individual buffer implementation. This allows us to get rid of some
> boiler-plate code.
> 
There are some indentation issues in here, as well. I'll point them out.
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/iio/industrialio-buffer.c        | 55 +++++++++++++++++++++-----------
>  drivers/iio/kfifo_buf.c                  | 15 ---------
>  drivers/staging/iio/accel/sca3000_ring.c | 14 ++------
>  include/linux/iio/buffer.h               | 37 ++-------------------
>  4 files changed, 40 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index cdc2482..a4d3ff6 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -383,10 +383,8 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev,
>  	return ret;
>  }
>  
> -
> -ssize_t iio_buffer_read_length(struct device *dev,
> -			       struct device_attribute *attr,
> -			       char *buf)
> +static ssize_t iio_buffer_read_length(struct device *dev,
> +	struct device_attribute *attr, char *buf)
Here.
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct iio_buffer *buffer = indio_dev->buffer;
> @@ -397,12 +395,9 @@ ssize_t iio_buffer_read_length(struct device *dev,
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL(iio_buffer_read_length);
>  
> -ssize_t iio_buffer_write_length(struct device *dev,
> -				struct device_attribute *attr,
> -				const char *buf,
> -				size_t len)
> +static ssize_t iio_buffer_write_length(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t len)
Here.
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct iio_buffer *buffer = indio_dev->buffer;
> @@ -429,16 +424,13 @@ ssize_t iio_buffer_write_length(struct device *dev,
>  
>  	return ret ? ret : len;
>  }
> -EXPORT_SYMBOL(iio_buffer_write_length);
>  
> -ssize_t iio_buffer_show_enable(struct device *dev,
> -			       struct device_attribute *attr,
> -			       char *buf)
> +static ssize_t iio_buffer_show_enable(struct device *dev,
> +	struct device_attribute *attr, char *buf)
Here.
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	return sprintf(buf, "%d\n", iio_buffer_is_active(indio_dev->buffer));
>  }
> -EXPORT_SYMBOL(iio_buffer_show_enable);
>  
>  static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
>  				const unsigned long *mask, bool timestamp)
> @@ -725,10 +717,8 @@ out_unlock:
>  }
>  EXPORT_SYMBOL_GPL(iio_update_buffers);
>  
> -ssize_t iio_buffer_store_enable(struct device *dev,
> -				struct device_attribute *attr,
> -				const char *buf,
> -				size_t len)
> +static ssize_t iio_buffer_store_enable(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t len)
Here.
>  {
>  	int ret;
>  	bool requested_state;
> @@ -760,10 +750,14 @@ done:
>  	mutex_unlock(&indio_dev->mlock);
>  	return (ret < 0) ? ret : len;
>  }
> -EXPORT_SYMBOL(iio_buffer_store_enable);
>  
>  static const char * const iio_scan_elements_group_name = "scan_elements";
>  
> +static DEVICE_ATTR(length, S_IRUGO | S_IWUSR, iio_buffer_read_length,
> +	iio_buffer_write_length);
> +static DEVICE_ATTR(enable, S_IRUGO | S_IWUSR,
> +	iio_buffer_show_enable, iio_buffer_store_enable);
> +
And these two here.
>  int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
>  {
>  	struct iio_dev_attr *p;
> @@ -775,6 +769,27 @@ int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
>  	if (!buffer)
>  		return 0;
>  
> +	attrcount = 0;
> +	if (buffer->attrs) {
> +		while (buffer->attrs[attrcount] != NULL)
> +			attrcount++;
> +	}
> +
> +	buffer->buffer_group.name = "buffer";
> +	buffer->buffer_group.attrs = kcalloc(attrcount + 3,
> +			sizeof(*buffer->buffer_group.attrs), GFP_KERNEL);
> +	if (!buffer->buffer_group.attrs)
> +		return -ENOMEM;
> +
> +	buffer->buffer_group.attrs[0] = &dev_attr_length.attr;
> +	buffer->buffer_group.attrs[1] = &dev_attr_enable.attr;
> +	if (buffer->attrs)
> +		memcpy(&buffer->buffer_group.attrs[2], buffer->attrs,
> +			sizeof(*&buffer->buffer_group.attrs) * (attrcount - 2));
> +	buffer->buffer_group.attrs[attrcount+2] = NULL;
> +
> +	indio_dev->groups[indio_dev->groupcounter++] = &buffer->buffer_group;
> +
>  	if (buffer->scan_el_attrs != NULL) {
>  		attr = buffer->scan_el_attrs->attrs;
>  		while (*attr++ != NULL)
> @@ -839,6 +854,7 @@ error_free_scan_mask:
>  	kfree(buffer->scan_mask);
>  error_cleanup_dynamic:
>  	iio_free_chan_devattr_list(&buffer->scan_el_dev_attr_list);
> +	kfree(indio_dev->buffer->buffer_group.attrs);
>  
>  	return ret;
>  }
> @@ -849,6 +865,7 @@ void iio_buffer_free_sysfs(struct iio_dev *indio_dev)
>  		return;
>  
>  	kfree(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);
>  }
> diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
> index 1258b4e..3b0a3bc 100644
> --- a/drivers/iio/kfifo_buf.c
> +++ b/drivers/iio/kfifo_buf.c
> @@ -52,20 +52,6 @@ static int iio_get_length_kfifo(struct iio_buffer *r)
>  	return r->length;
>  }
>  
> -static IIO_BUFFER_ENABLE_ATTR;
> -static IIO_BUFFER_LENGTH_ATTR;
> -
> -static struct attribute *iio_kfifo_attributes[] = {
> -	&dev_attr_length.attr,
> -	&dev_attr_enable.attr,
> -	NULL,
> -};
> -
> -static struct attribute_group iio_kfifo_attribute_group = {
> -	.attrs = iio_kfifo_attributes,
> -	.name = "buffer",
> -};
> -
>  static int iio_mark_update_needed_kfifo(struct iio_buffer *r)
>  {
>  	struct iio_kfifo *kf = iio_to_kfifo(r);
> @@ -169,7 +155,6 @@ struct iio_buffer *iio_kfifo_allocate(struct iio_dev *indio_dev)
>  		return NULL;
>  	kf->update_needed = true;
>  	iio_buffer_init(&kf->buffer);
> -	kf->buffer.attrs = &iio_kfifo_attribute_group;
>  	kf->buffer.access = &kfifo_access_funcs;
>  	kf->buffer.length = 2;
>  	mutex_init(&kf->user_lock);
> diff --git a/drivers/staging/iio/accel/sca3000_ring.c b/drivers/staging/iio/accel/sca3000_ring.c
> index aa0e5d8..f2f260e 100644
> --- a/drivers/staging/iio/accel/sca3000_ring.c
> +++ b/drivers/staging/iio/accel/sca3000_ring.c
> @@ -140,9 +140,6 @@ static bool sca3000_ring_buf_data_available(struct iio_buffer *r)
>  	return r->stufftoread;
>  }
>  
> -static IIO_BUFFER_ENABLE_ATTR;
> -static IIO_BUFFER_LENGTH_ATTR;
> -
>  /**
>   * sca3000_query_ring_int() is the hardware ring status interrupt enabled
>   **/
> @@ -232,20 +229,13 @@ static IIO_DEVICE_ATTR(in_accel_scale,
>   * only apply to the ring buffer.  At all times full rate and accuracy
>   * is available via direct reading from registers.
>   */
> -static struct attribute *sca3000_ring_attributes[] = {
> -	&dev_attr_length.attr,
> -	&dev_attr_enable.attr,
> +static const struct attribute *sca3000_ring_attributes[] = {
>  	&iio_dev_attr_50_percent.dev_attr.attr,
>  	&iio_dev_attr_75_percent.dev_attr.attr,
>  	&iio_dev_attr_in_accel_scale.dev_attr.attr,
>  	NULL,
>  };
>  
> -static struct attribute_group sca3000_ring_attr = {
> -	.attrs = sca3000_ring_attributes,
> -	.name = "buffer",
> -};
> -
>  static struct iio_buffer *sca3000_rb_allocate(struct iio_dev *indio_dev)
>  {
>  	struct iio_buffer *buf;
> @@ -258,7 +248,7 @@ static struct iio_buffer *sca3000_rb_allocate(struct iio_dev *indio_dev)
>  	ring->private = indio_dev;
>  	buf = &ring->buf;
>  	buf->stufftoread = 0;
> -	buf->attrs = &sca3000_ring_attr;
> +	buf->attrs = sca3000_ring_attributes;
>  	iio_buffer_init(buf);
>  
>  	return buf;
> diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
> index 79cdb3d..16b7663 100644
> --- a/include/linux/iio/buffer.h
> +++ b/include/linux/iio/buffer.h
> @@ -83,10 +83,11 @@ struct iio_buffer {
>  	bool					scan_timestamp;
>  	const struct iio_buffer_access_funcs	*access;
>  	struct list_head			scan_el_dev_attr_list;
> +	struct attribute_group			buffer_group;
>  	struct attribute_group			scan_el_group;
>  	wait_queue_head_t			pollq;
>  	bool					stufftoread;
> -	const struct attribute_group *attrs;
> +	const struct attribute			**attrs;
>  	struct list_head			demux_list;
>  	void					*demux_bounce;
>  	struct list_head			buffer_list;
> @@ -148,40 +149,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_read_length() - attr func to get number of datums in the buffer
> - **/
> -ssize_t iio_buffer_read_length(struct device *dev,
> -			       struct device_attribute *attr,
> -			       char *buf);
> -/**
> - * iio_buffer_write_length() - attr func to set number of datums in the buffer
> - **/
> -ssize_t iio_buffer_write_length(struct device *dev,
> -			      struct device_attribute *attr,
> -			      const char *buf,
> -			      size_t len);
> -/**
> - * iio_buffer_store_enable() - attr to turn the buffer on
> - **/
> -ssize_t iio_buffer_store_enable(struct device *dev,
> -				struct device_attribute *attr,
> -				const char *buf,
> -				size_t len);
> -/**
> - * iio_buffer_show_enable() - attr to see if the buffer is on
> - **/
> -ssize_t iio_buffer_show_enable(struct device *dev,
> -			       struct device_attribute *attr,
> -			       char *buf);
> -#define IIO_BUFFER_LENGTH_ATTR DEVICE_ATTR(length, S_IRUGO | S_IWUSR,	\
> -					   iio_buffer_read_length,	\
> -					   iio_buffer_write_length)
> -
> -#define IIO_BUFFER_ENABLE_ATTR DEVICE_ATTR(enable, S_IRUGO | S_IWUSR,	\
> -					   iio_buffer_show_enable,	\
> -					   iio_buffer_store_enable)
> -
>  bool iio_validate_scan_mask_onehot(struct iio_dev *indio_dev,
>  	const unsigned long *mask);
>  
> 

Re: [PATCH 09/11] iio: buffer: Allocate standard attributes in the core

[Jonathan Cameron]

On 10/12/14 22:42, Hartmut Knaack wrote:
> Lars-Peter Clausen schrieb am 26.11.2014 um 18:55:
>> All buffers want at least the length and the enable attribute. Move the
>> creation of those attributes to the core instead of having to do this in
>> each individual buffer implementation. This allows us to get rid of some
>> boiler-plate code.
>>
> There are some indentation issues in here, as well. I'll point them out.
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Adjusted the alignment as suggested and applied to the togreg branch of iio.git.
As ever pushed out sometime later today as testing.  If you could check I
haven't fouled anything up that would be great.

Thanks.


Jonathan
>> ---
>>  drivers/iio/industrialio-buffer.c        | 55 +++++++++++++++++++++-----------
>>  drivers/iio/kfifo_buf.c                  | 15 ---------
>>  drivers/staging/iio/accel/sca3000_ring.c | 14 ++------
>>  include/linux/iio/buffer.h               | 37 ++-------------------
>>  4 files changed, 40 insertions(+), 81 deletions(-)
>>
>> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
>> index cdc2482..a4d3ff6 100644
>> --- a/drivers/iio/industrialio-buffer.c
>> +++ b/drivers/iio/industrialio-buffer.c
>> @@ -383,10 +383,8 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev,
>>  	return ret;
>>  }
>>  
>> -
>> -ssize_t iio_buffer_read_length(struct device *dev,
>> -			       struct device_attribute *attr,
>> -			       char *buf)
>> +static ssize_t iio_buffer_read_length(struct device *dev,
>> +	struct device_attribute *attr, char *buf)
> Here.
>>  {
>>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>  	struct iio_buffer *buffer = indio_dev->buffer;
>> @@ -397,12 +395,9 @@ ssize_t iio_buffer_read_length(struct device *dev,
>>  
>>  	return 0;
>>  }
>> -EXPORT_SYMBOL(iio_buffer_read_length);
>>  
>> -ssize_t iio_buffer_write_length(struct device *dev,
>> -				struct device_attribute *attr,
>> -				const char *buf,
>> -				size_t len)
>> +static ssize_t iio_buffer_write_length(struct device *dev,
>> +	struct device_attribute *attr, const char *buf, size_t len)
> Here.
>>  {
>>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>  	struct iio_buffer *buffer = indio_dev->buffer;
>> @@ -429,16 +424,13 @@ ssize_t iio_buffer_write_length(struct device *dev,
>>  
>>  	return ret ? ret : len;
>>  }
>> -EXPORT_SYMBOL(iio_buffer_write_length);
>>  
>> -ssize_t iio_buffer_show_enable(struct device *dev,
>> -			       struct device_attribute *attr,
>> -			       char *buf)
>> +static ssize_t iio_buffer_show_enable(struct device *dev,
>> +	struct device_attribute *attr, char *buf)
> Here.
>>  {
>>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>  	return sprintf(buf, "%d\n", iio_buffer_is_active(indio_dev->buffer));
>>  }
>> -EXPORT_SYMBOL(iio_buffer_show_enable);
>>  
>>  static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
>>  				const unsigned long *mask, bool timestamp)
>> @@ -725,10 +717,8 @@ out_unlock:
>>  }
>>  EXPORT_SYMBOL_GPL(iio_update_buffers);
>>  
>> -ssize_t iio_buffer_store_enable(struct device *dev,
>> -				struct device_attribute *attr,
>> -				const char *buf,
>> -				size_t len)
>> +static ssize_t iio_buffer_store_enable(struct device *dev,
>> +	struct device_attribute *attr, const char *buf, size_t len)
> Here.
>>  {
>>  	int ret;
>>  	bool requested_state;
>> @@ -760,10 +750,14 @@ done:
>>  	mutex_unlock(&indio_dev->mlock);
>>  	return (ret < 0) ? ret : len;
>>  }
>> -EXPORT_SYMBOL(iio_buffer_store_enable);
>>  
>>  static const char * const iio_scan_elements_group_name = "scan_elements";
>>  
>> +static DEVICE_ATTR(length, S_IRUGO | S_IWUSR, iio_buffer_read_length,
>> +	iio_buffer_write_length);
>> +static DEVICE_ATTR(enable, S_IRUGO | S_IWUSR,
>> +	iio_buffer_show_enable, iio_buffer_store_enable);
>> +
> And these two here.
>>  int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
>>  {
>>  	struct iio_dev_attr *p;
>> @@ -775,6 +769,27 @@ int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
>>  	if (!buffer)
>>  		return 0;
>>  
>> +	attrcount = 0;
>> +	if (buffer->attrs) {
>> +		while (buffer->attrs[attrcount] != NULL)
>> +			attrcount++;
>> +	}
>> +
>> +	buffer->buffer_group.name = "buffer";
>> +	buffer->buffer_group.attrs = kcalloc(attrcount + 3,
>> +			sizeof(*buffer->buffer_group.attrs), GFP_KERNEL);
>> +	if (!buffer->buffer_group.attrs)
>> +		return -ENOMEM;
>> +
>> +	buffer->buffer_group.attrs[0] = &dev_attr_length.attr;
>> +	buffer->buffer_group.attrs[1] = &dev_attr_enable.attr;
>> +	if (buffer->attrs)
>> +		memcpy(&buffer->buffer_group.attrs[2], buffer->attrs,
>> +			sizeof(*&buffer->buffer_group.attrs) * (attrcount - 2));
>> +	buffer->buffer_group.attrs[attrcount+2] = NULL;
>> +
>> +	indio_dev->groups[indio_dev->groupcounter++] = &buffer->buffer_group;
>> +
>>  	if (buffer->scan_el_attrs != NULL) {
>>  		attr = buffer->scan_el_attrs->attrs;
>>  		while (*attr++ != NULL)
>> @@ -839,6 +854,7 @@ error_free_scan_mask:
>>  	kfree(buffer->scan_mask);
>>  error_cleanup_dynamic:
>>  	iio_free_chan_devattr_list(&buffer->scan_el_dev_attr_list);
>> +	kfree(indio_dev->buffer->buffer_group.attrs);
>>  
>>  	return ret;
>>  }
>> @@ -849,6 +865,7 @@ void iio_buffer_free_sysfs(struct iio_dev *indio_dev)
>>  		return;
>>  
>>  	kfree(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);
>>  }
>> diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
>> index 1258b4e..3b0a3bc 100644
>> --- a/drivers/iio/kfifo_buf.c
>> +++ b/drivers/iio/kfifo_buf.c
>> @@ -52,20 +52,6 @@ static int iio_get_length_kfifo(struct iio_buffer *r)
>>  	return r->length;
>>  }
>>  
>> -static IIO_BUFFER_ENABLE_ATTR;
>> -static IIO_BUFFER_LENGTH_ATTR;
>> -
>> -static struct attribute *iio_kfifo_attributes[] = {
>> -	&dev_attr_length.attr,
>> -	&dev_attr_enable.attr,
>> -	NULL,
>> -};
>> -
>> -static struct attribute_group iio_kfifo_attribute_group = {
>> -	.attrs = iio_kfifo_attributes,
>> -	.name = "buffer",
>> -};
>> -
>>  static int iio_mark_update_needed_kfifo(struct iio_buffer *r)
>>  {
>>  	struct iio_kfifo *kf = iio_to_kfifo(r);
>> @@ -169,7 +155,6 @@ struct iio_buffer *iio_kfifo_allocate(struct iio_dev *indio_dev)
>>  		return NULL;
>>  	kf->update_needed = true;
>>  	iio_buffer_init(&kf->buffer);
>> -	kf->buffer.attrs = &iio_kfifo_attribute_group;
>>  	kf->buffer.access = &kfifo_access_funcs;
>>  	kf->buffer.length = 2;
>>  	mutex_init(&kf->user_lock);
>> diff --git a/drivers/staging/iio/accel/sca3000_ring.c b/drivers/staging/iio/accel/sca3000_ring.c
>> index aa0e5d8..f2f260e 100644
>> --- a/drivers/staging/iio/accel/sca3000_ring.c
>> +++ b/drivers/staging/iio/accel/sca3000_ring.c
>> @@ -140,9 +140,6 @@ static bool sca3000_ring_buf_data_available(struct iio_buffer *r)
>>  	return r->stufftoread;
>>  }
>>  
>> -static IIO_BUFFER_ENABLE_ATTR;
>> -static IIO_BUFFER_LENGTH_ATTR;
>> -
>>  /**
>>   * sca3000_query_ring_int() is the hardware ring status interrupt enabled
>>   **/
>> @@ -232,20 +229,13 @@ static IIO_DEVICE_ATTR(in_accel_scale,
>>   * only apply to the ring buffer.  At all times full rate and accuracy
>>   * is available via direct reading from registers.
>>   */
>> -static struct attribute *sca3000_ring_attributes[] = {
>> -	&dev_attr_length.attr,
>> -	&dev_attr_enable.attr,
>> +static const struct attribute *sca3000_ring_attributes[] = {
>>  	&iio_dev_attr_50_percent.dev_attr.attr,
>>  	&iio_dev_attr_75_percent.dev_attr.attr,
>>  	&iio_dev_attr_in_accel_scale.dev_attr.attr,
>>  	NULL,
>>  };
>>  
>> -static struct attribute_group sca3000_ring_attr = {
>> -	.attrs = sca3000_ring_attributes,
>> -	.name = "buffer",
>> -};
>> -
>>  static struct iio_buffer *sca3000_rb_allocate(struct iio_dev *indio_dev)
>>  {
>>  	struct iio_buffer *buf;
>> @@ -258,7 +248,7 @@ static struct iio_buffer *sca3000_rb_allocate(struct iio_dev *indio_dev)
>>  	ring->private = indio_dev;
>>  	buf = &ring->buf;
>>  	buf->stufftoread = 0;
>> -	buf->attrs = &sca3000_ring_attr;
>> +	buf->attrs = sca3000_ring_attributes;
>>  	iio_buffer_init(buf);
>>  
>>  	return buf;
>> diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
>> index 79cdb3d..16b7663 100644
>> --- a/include/linux/iio/buffer.h
>> +++ b/include/linux/iio/buffer.h
>> @@ -83,10 +83,11 @@ struct iio_buffer {
>>  	bool					scan_timestamp;
>>  	const struct iio_buffer_access_funcs	*access;
>>  	struct list_head			scan_el_dev_attr_list;
>> +	struct attribute_group			buffer_group;
>>  	struct attribute_group			scan_el_group;
>>  	wait_queue_head_t			pollq;
>>  	bool					stufftoread;
>> -	const struct attribute_group *attrs;
>> +	const struct attribute			**attrs;
>>  	struct list_head			demux_list;
>>  	void					*demux_bounce;
>>  	struct list_head			buffer_list;
>> @@ -148,40 +149,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_read_length() - attr func to get number of datums in the buffer
>> - **/
>> -ssize_t iio_buffer_read_length(struct device *dev,
>> -			       struct device_attribute *attr,
>> -			       char *buf);
>> -/**
>> - * iio_buffer_write_length() - attr func to set number of datums in the buffer
>> - **/
>> -ssize_t iio_buffer_write_length(struct device *dev,
>> -			      struct device_attribute *attr,
>> -			      const char *buf,
>> -			      size_t len);
>> -/**
>> - * iio_buffer_store_enable() - attr to turn the buffer on
>> - **/
>> -ssize_t iio_buffer_store_enable(struct device *dev,
>> -				struct device_attribute *attr,
>> -				const char *buf,
>> -				size_t len);
>> -/**
>> - * iio_buffer_show_enable() - attr to see if the buffer is on
>> - **/
>> -ssize_t iio_buffer_show_enable(struct device *dev,
>> -			       struct device_attribute *attr,
>> -			       char *buf);
>> -#define IIO_BUFFER_LENGTH_ATTR DEVICE_ATTR(length, S_IRUGO | S_IWUSR,	\
>> -					   iio_buffer_read_length,	\
>> -					   iio_buffer_write_length)
>> -
>> -#define IIO_BUFFER_ENABLE_ATTR DEVICE_ATTR(enable, S_IRUGO | S_IWUSR,	\
>> -					   iio_buffer_show_enable,	\
>> -					   iio_buffer_store_enable)
>> -
>>  bool iio_validate_scan_mask_onehot(struct iio_dev *indio_dev,
>>  	const unsigned long *mask);
>>  
>>
> 

[PATCH 10/11] iio: buffer: Make length attribute read only for buffers without set_length

[Lars-Peter Clausen]

If a buffer implementation does not implement the set_length() callback the
length will be static and can not be changed by userspace. Mark the length
attribute as a read only property in this case so userspace is aware of this
rather than just silently accepting any length value.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-buffer.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index a4d3ff6..3e0c3a9 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -416,8 +416,7 @@ static ssize_t iio_buffer_write_length(struct device *dev,
 	if (iio_buffer_is_active(indio_dev->buffer)) {
 		ret = -EBUSY;
 	} else {
-		if (buffer->access->set_length)
-			buffer->access->set_length(buffer, val);
+		buffer->access->set_length(buffer, val);
 		ret = 0;
 	}
 	mutex_unlock(&indio_dev->mlock);
@@ -755,6 +754,8 @@ static const char * const iio_scan_elements_group_name = "scan_elements";
 
 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);
 
@@ -781,7 +782,10 @@ int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
 	if (!buffer->buffer_group.attrs)
 		return -ENOMEM;
 
-	buffer->buffer_group.attrs[0] = &dev_attr_length.attr;
+	if (buffer->access->set_length)
+		buffer->buffer_group.attrs[0] = &dev_attr_length.attr;
+	else
+		buffer->buffer_group.attrs[0] = &dev_attr_length_ro.attr;
 	buffer->buffer_group.attrs[1] = &dev_attr_enable.attr;
 	if (buffer->attrs)
 		memcpy(&buffer->buffer_group.attrs[2], buffer->attrs,
-- 
1.8.0

Re: [PATCH 10/11] iio: buffer: Make length attribute read only for buffers without set_length

[Jonathan Cameron]

On 26/11/14 17:55, Lars-Peter Clausen wrote:
> If a buffer implementation does not implement the set_length() callback the
> length will be static and can not be changed by userspace. Mark the length
> attribute as a read only property in this case so userspace is aware of this
> rather than just silently accepting any length value.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Nice little change.

Thanks
Jonathan
> ---
>  drivers/iio/industrialio-buffer.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index a4d3ff6..3e0c3a9 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -416,8 +416,7 @@ static ssize_t iio_buffer_write_length(struct device *dev,
>  	if (iio_buffer_is_active(indio_dev->buffer)) {
>  		ret = -EBUSY;
>  	} else {
> -		if (buffer->access->set_length)
> -			buffer->access->set_length(buffer, val);
> +		buffer->access->set_length(buffer, val);
>  		ret = 0;
>  	}
>  	mutex_unlock(&indio_dev->mlock);
> @@ -755,6 +754,8 @@ static const char * const iio_scan_elements_group_name = "scan_elements";
>  
>  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);
>  
> @@ -781,7 +782,10 @@ int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
>  	if (!buffer->buffer_group.attrs)
>  		return -ENOMEM;
>  
> -	buffer->buffer_group.attrs[0] = &dev_attr_length.attr;
> +	if (buffer->access->set_length)
> +		buffer->buffer_group.attrs[0] = &dev_attr_length.attr;
> +	else
> +		buffer->buffer_group.attrs[0] = &dev_attr_length_ro.attr;
>  	buffer->buffer_group.attrs[1] = &dev_attr_enable.attr;
>  	if (buffer->attrs)
>  		memcpy(&buffer->buffer_group.attrs[2], buffer->attrs,
> 

Re: [PATCH 10/11] iio: buffer: Make length attribute read only for buffers without set_length

[Jonathan Cameron]

On 12/12/14 11:08, Jonathan Cameron wrote:
> On 26/11/14 17:55, Lars-Peter Clausen wrote:
>> If a buffer implementation does not implement the set_length() callback the
>> length will be static and can not be changed by userspace. Mark the length
>> attribute as a read only property in this case so userspace is aware of this
>> rather than just silently accepting any length value.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Nice little change.
> 
Having said that I doubt that longterm this will apply to many devices
as we'll be routing more and more hardware buffers through a software
front end and that front end will be variable size...

Anyhow, applied to the togreg branch of iio.git with fuzz etc
from my earlier patch tweaking.

> Thanks
> Jonathan
>> ---
>>  drivers/iio/industrialio-buffer.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
>> index a4d3ff6..3e0c3a9 100644
>> --- a/drivers/iio/industrialio-buffer.c
>> +++ b/drivers/iio/industrialio-buffer.c
>> @@ -416,8 +416,7 @@ static ssize_t iio_buffer_write_length(struct device *dev,
>>  	if (iio_buffer_is_active(indio_dev->buffer)) {
>>  		ret = -EBUSY;
>>  	} else {
>> -		if (buffer->access->set_length)
>> -			buffer->access->set_length(buffer, val);
>> +		buffer->access->set_length(buffer, val);
>>  		ret = 0;
>>  	}
>>  	mutex_unlock(&indio_dev->mlock);
>> @@ -755,6 +754,8 @@ static const char * const iio_scan_elements_group_name = "scan_elements";
>>  
>>  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);
>>  
>> @@ -781,7 +782,10 @@ int iio_buffer_alloc_sysfs(struct iio_dev *indio_dev)
>>  	if (!buffer->buffer_group.attrs)
>>  		return -ENOMEM;
>>  
>> -	buffer->buffer_group.attrs[0] = &dev_attr_length.attr;
>> +	if (buffer->access->set_length)
>> +		buffer->buffer_group.attrs[0] = &dev_attr_length.attr;
>> +	else
>> +		buffer->buffer_group.attrs[0] = &dev_attr_length_ro.attr;
>>  	buffer->buffer_group.attrs[1] = &dev_attr_enable.attr;
>>  	if (buffer->attrs)
>>  		memcpy(&buffer->buffer_group.attrs[2], buffer->attrs,
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 10/11] iio: buffer: Make length attribute read only for buffers without set_length

[Lars-Peter Clausen]

On 12/12/2014 12:11 PM, Jonathan Cameron wrote:
> On 12/12/14 11:08, Jonathan Cameron wrote:
>> On 26/11/14 17:55, Lars-Peter Clausen wrote:
>>> If a buffer implementation does not implement the set_length() callback the
>>> length will be static and can not be changed by userspace. Mark the length
>>> attribute as a read only property in this case so userspace is aware of this
>>> rather than just silently accepting any length value.
>>>
>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> Nice little change.
>>
> Having said that I doubt that longterm this will apply to many devices
> as we'll be routing more and more hardware buffers through a software
> front end and that front end will be variable size...

We'll probably have something like a buffer_index field in channel struct.

>
> Anyhow, applied to the togreg branch of iio.git with fuzz etc
> from my earlier patch tweaking.

Thanks.

[PATCH 11/11] iio: buffer: Drop get_length callback

[Lars-Peter Clausen]

We already do have the length field in the struct iio_buffer which is
expected to be in sync with the current size of the buffer. And currently
all implementations of the get_length callback either return this field or a
constant number.

This patch removes the get_length callback and replaces all occurrences in
the IIO core with directly accessing the length field of the buffer.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-buffer.c          | 11 +++--------
 drivers/iio/kfifo_buf.c                    |  6 ------
 drivers/staging/iio/Documentation/ring.txt |  4 ++--
 drivers/staging/iio/accel/sca3000_ring.c   |  8 +-------
 include/linux/iio/buffer.h                 |  2 --
 5 files changed, 6 insertions(+), 25 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 3e0c3a9..6a97b38 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -389,11 +389,7 @@ static ssize_t iio_buffer_read_length(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct iio_buffer *buffer = indio_dev->buffer;
 
-	if (buffer->access->get_length)
-		return sprintf(buf, "%d\n",
-			       buffer->access->get_length(buffer));
-
-	return 0;
+	return sprintf(buf, "%d\n", buffer->length);
 }
 
 static ssize_t iio_buffer_write_length(struct device *dev,
@@ -408,9 +404,8 @@ static ssize_t iio_buffer_write_length(struct device *dev,
 	if (ret)
 		return ret;
 
-	if (buffer->access->get_length)
-		if (val == buffer->access->get_length(buffer))
-			return len;
+	if (val == buffer->length)
+		return len;
 
 	mutex_lock(&indio_dev->mlock);
 	if (iio_buffer_is_active(indio_dev->buffer)) {
diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
index 3b0a3bc..b20a9cf 100644
--- a/drivers/iio/kfifo_buf.c
+++ b/drivers/iio/kfifo_buf.c
@@ -47,11 +47,6 @@ static int iio_request_update_kfifo(struct iio_buffer *r)
 	return ret;
 }
 
-static int iio_get_length_kfifo(struct iio_buffer *r)
-{
-	return r->length;
-}
-
 static int iio_mark_update_needed_kfifo(struct iio_buffer *r)
 {
 	struct iio_kfifo *kf = iio_to_kfifo(r);
@@ -141,7 +136,6 @@ static const struct iio_buffer_access_funcs kfifo_access_funcs = {
 	.data_available = iio_kfifo_buf_data_available,
 	.request_update = &iio_request_update_kfifo,
 	.set_bytes_per_datum = &iio_set_bytes_per_datum_kfifo,
-	.get_length = &iio_get_length_kfifo,
 	.set_length = &iio_set_length_kfifo,
 	.release = &iio_kfifo_buffer_release,
 };
diff --git a/drivers/staging/iio/Documentation/ring.txt b/drivers/staging/iio/Documentation/ring.txt
index 434d63a..18718fc 100644
--- a/drivers/staging/iio/Documentation/ring.txt
+++ b/drivers/staging/iio/Documentation/ring.txt
@@ -42,6 +42,6 @@ request_update
 set_bytes_per_datum
   Set the number of bytes for a complete scan. (All samples + timestamp)
 
-get_length / set_length
-  Get/set the number of complete scans that may be held by the buffer.
+set_length
+  Set the number of complete scans that may be held by the buffer.
 
diff --git a/drivers/staging/iio/accel/sca3000_ring.c b/drivers/staging/iio/accel/sca3000_ring.c
index f2f260e..f76a268 100644
--- a/drivers/staging/iio/accel/sca3000_ring.c
+++ b/drivers/staging/iio/accel/sca3000_ring.c
@@ -129,12 +129,6 @@ error_ret:
 	return ret ? ret : num_read;
 }
 
-/* This is only valid with all 3 elements enabled */
-static int sca3000_ring_get_length(struct iio_buffer *r)
-{
-	return 64;
-}
-
 static bool sca3000_ring_buf_data_available(struct iio_buffer *r)
 {
 	return r->stufftoread;
@@ -248,6 +242,7 @@ static struct iio_buffer *sca3000_rb_allocate(struct iio_dev *indio_dev)
 	ring->private = indio_dev;
 	buf = &ring->buf;
 	buf->stufftoread = 0;
+	buf->length = 64;
 	buf->attrs = sca3000_ring_attributes;
 	iio_buffer_init(buf);
 
@@ -261,7 +256,6 @@ static void sca3000_ring_release(struct iio_buffer *r)
 
 static const struct iio_buffer_access_funcs sca3000_ring_access_funcs = {
 	.read_first_n = &sca3000_read_first_n_hw_rb,
-	.get_length = &sca3000_ring_get_length,
 	.data_available = sca3000_ring_buf_data_available,
 	.release = sca3000_ring_release,
 };
diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
index 16b7663..b65850a 100644
--- a/include/linux/iio/buffer.h
+++ b/include/linux/iio/buffer.h
@@ -26,7 +26,6 @@ struct iio_buffer;
  * @request_update:	if a parameter change has been marked, update underlying
  *			storage.
  * @set_bytes_per_datum:set number of bytes per datum
- * @get_length:		get number of datums in buffer
  * @set_length:		set number of datums in buffer
  * @release:		called when the last reference to the buffer is dropped,
  *			should free all resources allocated by the buffer.
@@ -49,7 +48,6 @@ struct iio_buffer_access_funcs {
 	int (*request_update)(struct iio_buffer *buffer);
 
 	int (*set_bytes_per_datum)(struct iio_buffer *buffer, size_t bpd);
-	int (*get_length)(struct iio_buffer *buffer);
 	int (*set_length)(struct iio_buffer *buffer, int length);
 
 	void (*release)(struct iio_buffer *buffer);
-- 
1.8.0

Re: [PATCH 11/11] iio: buffer: Drop get_length callback

[Jonathan Cameron]

On 26/11/14 17:55, Lars-Peter Clausen wrote:
> We already do have the length field in the struct iio_buffer which is
> expected to be in sync with the current size of the buffer. And currently
> all implementations of the get_length callback either return this field or a
> constant number.
> 
> This patch removes the get_length callback and replaces all occurrences in
> the IIO core with directly accessing the length field of the buffer.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Applied.  Thanks for this series - all good stuff.
I'm looking forward to the dma stuff (though perhaps not reviewing it ;)
> ---
>  drivers/iio/industrialio-buffer.c          | 11 +++--------
>  drivers/iio/kfifo_buf.c                    |  6 ------
>  drivers/staging/iio/Documentation/ring.txt |  4 ++--
>  drivers/staging/iio/accel/sca3000_ring.c   |  8 +-------
>  include/linux/iio/buffer.h                 |  2 --
>  5 files changed, 6 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 3e0c3a9..6a97b38 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -389,11 +389,7 @@ static ssize_t iio_buffer_read_length(struct device *dev,
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct iio_buffer *buffer = indio_dev->buffer;
>  
> -	if (buffer->access->get_length)
> -		return sprintf(buf, "%d\n",
> -			       buffer->access->get_length(buffer));
> -
> -	return 0;
> +	return sprintf(buf, "%d\n", buffer->length);
>  }
>  
>  static ssize_t iio_buffer_write_length(struct device *dev,
> @@ -408,9 +404,8 @@ static ssize_t iio_buffer_write_length(struct device *dev,
>  	if (ret)
>  		return ret;
>  
> -	if (buffer->access->get_length)
> -		if (val == buffer->access->get_length(buffer))
> -			return len;
> +	if (val == buffer->length)
> +		return len;
>  
>  	mutex_lock(&indio_dev->mlock);
>  	if (iio_buffer_is_active(indio_dev->buffer)) {
> diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
> index 3b0a3bc..b20a9cf 100644
> --- a/drivers/iio/kfifo_buf.c
> +++ b/drivers/iio/kfifo_buf.c
> @@ -47,11 +47,6 @@ static int iio_request_update_kfifo(struct iio_buffer *r)
>  	return ret;
>  }
>  
> -static int iio_get_length_kfifo(struct iio_buffer *r)
> -{
> -	return r->length;
> -}
> -
>  static int iio_mark_update_needed_kfifo(struct iio_buffer *r)
>  {
>  	struct iio_kfifo *kf = iio_to_kfifo(r);
> @@ -141,7 +136,6 @@ static const struct iio_buffer_access_funcs kfifo_access_funcs = {
>  	.data_available = iio_kfifo_buf_data_available,
>  	.request_update = &iio_request_update_kfifo,
>  	.set_bytes_per_datum = &iio_set_bytes_per_datum_kfifo,
> -	.get_length = &iio_get_length_kfifo,
>  	.set_length = &iio_set_length_kfifo,
>  	.release = &iio_kfifo_buffer_release,
>  };
> diff --git a/drivers/staging/iio/Documentation/ring.txt b/drivers/staging/iio/Documentation/ring.txt
> index 434d63a..18718fc 100644
> --- a/drivers/staging/iio/Documentation/ring.txt
> +++ b/drivers/staging/iio/Documentation/ring.txt
> @@ -42,6 +42,6 @@ request_update
>  set_bytes_per_datum
>    Set the number of bytes for a complete scan. (All samples + timestamp)
>  
> -get_length / set_length
> -  Get/set the number of complete scans that may be held by the buffer.
> +set_length
> +  Set the number of complete scans that may be held by the buffer.
>  
> diff --git a/drivers/staging/iio/accel/sca3000_ring.c b/drivers/staging/iio/accel/sca3000_ring.c
> index f2f260e..f76a268 100644
> --- a/drivers/staging/iio/accel/sca3000_ring.c
> +++ b/drivers/staging/iio/accel/sca3000_ring.c
> @@ -129,12 +129,6 @@ error_ret:
>  	return ret ? ret : num_read;
>  }
>  
> -/* This is only valid with all 3 elements enabled */
> -static int sca3000_ring_get_length(struct iio_buffer *r)
> -{
> -	return 64;
> -}
> -
>  static bool sca3000_ring_buf_data_available(struct iio_buffer *r)
>  {
>  	return r->stufftoread;
> @@ -248,6 +242,7 @@ static struct iio_buffer *sca3000_rb_allocate(struct iio_dev *indio_dev)
>  	ring->private = indio_dev;
>  	buf = &ring->buf;
>  	buf->stufftoread = 0;
> +	buf->length = 64;
>  	buf->attrs = sca3000_ring_attributes;
>  	iio_buffer_init(buf);
>  
> @@ -261,7 +256,6 @@ static void sca3000_ring_release(struct iio_buffer *r)
>  
>  static const struct iio_buffer_access_funcs sca3000_ring_access_funcs = {
>  	.read_first_n = &sca3000_read_first_n_hw_rb,
> -	.get_length = &sca3000_ring_get_length,
>  	.data_available = sca3000_ring_buf_data_available,
>  	.release = sca3000_ring_release,
>  };
> diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
> index 16b7663..b65850a 100644
> --- a/include/linux/iio/buffer.h
> +++ b/include/linux/iio/buffer.h
> @@ -26,7 +26,6 @@ struct iio_buffer;
>   * @request_update:	if a parameter change has been marked, update underlying
>   *			storage.
>   * @set_bytes_per_datum:set number of bytes per datum
> - * @get_length:		get number of datums in buffer
>   * @set_length:		set number of datums in buffer
>   * @release:		called when the last reference to the buffer is dropped,
>   *			should free all resources allocated by the buffer.
> @@ -49,7 +48,6 @@ struct iio_buffer_access_funcs {
>  	int (*request_update)(struct iio_buffer *buffer);
>  
>  	int (*set_bytes_per_datum)(struct iio_buffer *buffer, size_t bpd);
> -	int (*get_length)(struct iio_buffer *buffer);
>  	int (*set_length)(struct iio_buffer *buffer, int length);
>  
>  	void (*release)(struct iio_buffer *buffer);
>