[PATCH v2 1/6] iio: Add callback to check whether a scan mask is valid

[PATCH v2 1/6] iio: Add callback to check whether a scan mask is valid

[Lars-Peter Clausen]

This is useful for cases where the number of valid scan masks grows
exponentially, but it is rather easy to check whether a mask is valid or not
programmatically.

An example of such a case is a device with multiple ADCs where each ADC has a
upstream MUX, which allows to select from a number of physical channels.

  +-------+   +-------+
  |       |   |       | --- Channel 1
  | ADC 1 |---| MUX 1 | ---   ...
  |       |   |       | --- Channel M
  +-------+   +-------+

     .            .            .
     .            .            .
     .            .            .

  +-------+   +-------+
  |       |   |       | --- Channel M * N + 1
  | ADC N |---| MUX N | ---       ...
  |       |   |       | --- Channel M * N + M
  +-------+   +-------+

The number of necessary scan masks for this case is (M+1)**N - 1, on the other
hand it is easy to check whether subsets for each ADC of the scanmask have only
one bit set.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

---
No changes since v1
---
 drivers/iio/industrialio-buffer.c |   27 ++++++++++++++++++++-------
 include/linux/iio/iio.h           |    4 ++++
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 3d8d187..cc5db36 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -570,6 +570,15 @@ int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
 }
 EXPORT_SYMBOL(iio_sw_buffer_preenable);
 
+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
  * @buffer: the buffer whose scan mask we are interested in
@@ -589,27 +598,31 @@ int iio_scan_mask_set(struct iio_dev *indio_dev,
 		return -ENOMEM;
 	if (!indio_dev->masklength) {
 		WARN_ON("trying to set scanmask prior to registering buffer\n");
-		kfree(trialmask);
-		return -EINVAL;
+		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) {
-			kfree(trialmask);
-			return -EINVAL;
-		}
+		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,
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 2afbb6f..be82936 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -363,12 +363,16 @@ struct iio_info {
  * @predisable:		[DRIVER] function to run prior to marking buffer
  *			disabled
  * @postdisable:	[DRIVER] function to run after marking buffer disabled
+ * @validate_scan_mask: [DRIVER] function callback to check whether a given
+ *			scan mask is valid for the device.
  */
 struct iio_buffer_setup_ops {
 	int				(*preenable)(struct iio_dev *);
 	int				(*postenable)(struct iio_dev *);
 	int				(*predisable)(struct iio_dev *);
 	int				(*postdisable)(struct iio_dev *);
+	bool (*validate_scan_mask)(struct iio_dev *indio_dev,
+				   const unsigned long *scan_mask);
 };
 
 /**
-- 
1.7.10.4

[PATCH v2 2/6] iio: Introduce iio_validate_scan_mask_onehot

[Lars-Peter Clausen]

Add a helper function for validating a scan mask for devices where exactly one
channel must be selected during sampling. This is a common case among devices
which have scan mask restrictions so it makes sense to provide this function in
the core.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
Make the check more strict and only accept scanmasks with exactly one bit set
instead of one or less bits set.
---
 drivers/iio/industrialio-buffer.c |   16 ++++++++++++++++
 include/linux/iio/buffer.h        |    3 +++
 2 files changed, 19 insertions(+)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index cc5db36..6e00042 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -570,6 +570,22 @@ int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
 }
 EXPORT_SYMBOL(iio_sw_buffer_preenable);
 
+/**
+ * iio_validate_scan_mask_onehot() - Validates that exactly one channel is selected
+ * @indio_dev: the iio device
+ * @mask: scan mask to be checked
+ *
+ * Return true if exactly one bit is set in the scan mask, false otherwise. It
+ * can be used for devices where only one channel can be active for sampling at
+ * a time.
+ */
+bool iio_validate_scan_mask_onehot(struct iio_dev *indio_dev,
+	const unsigned long *mask)
+{
+	return bitmap_weight(mask, indio_dev->masklength) == 1;
+}
+EXPORT_SYMBOL_GPL(iio_validate_scan_mask_onehot);
+
 static bool iio_validate_scan_mask(struct iio_dev *indio_dev,
 	const unsigned long *mask)
 {
diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
index ad4fb1a..cc92713 100644
--- a/include/linux/iio/buffer.h
+++ b/include/linux/iio/buffer.h
@@ -174,6 +174,9 @@ ssize_t iio_buffer_show_enable(struct device *dev,
 
 int iio_sw_buffer_preenable(struct iio_dev *indio_dev);
 
+bool iio_validate_scan_mask_onehot(struct iio_dev *indio_dev,
+	const unsigned long *mask);
+
 #else /* CONFIG_IIO_BUFFER */
 
 static inline int iio_buffer_register(struct iio_dev *indio_dev,
-- 
1.7.10.4

[PATCH v2 3/6] staging:iio:ad7192: Use iio_validate_scan_mask_onehot

[Lars-Peter Clausen]

Only one of the channels of the ad7192 may be sampled at a time. Use the new
validate_scan_mask callback and the iio_validate_scan_mask_onehot function to
implement this restriction. Previously this was implemented using
available_scan_masks, but this requires a individual scan mask for each channel.

Also the previous code was adding the scan index of the timestamp channel to
each available scan mask, this is not required though since the timestamp
channel is not restricted by the available scan masks and can be enabled or
disabled independently. So the new code does not have to take care of this.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
No changes since v1
---
 drivers/staging/iio/adc/ad7192.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index 839f17c..22c3923 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -146,7 +146,6 @@ struct ad7192_state {
 	u32				mode;
 	u32				conf;
 	u32				scale_avail[8][2];
-	long				available_scan_masks[9];
 	u8				gpocon;
 	u8				devid;
 	/*
@@ -538,6 +537,7 @@ static const struct iio_buffer_setup_ops ad7192_ring_setup_ops = {
 	.postenable = &iio_triggered_buffer_postenable,
 	.predisable = &iio_triggered_buffer_predisable,
 	.postdisable = &ad7192_ring_postdisable,
+	.validate_scan_mask = &iio_validate_scan_mask_onehot,
 };
 
 static int ad7192_register_ring_funcs_and_init(struct iio_dev *indio_dev)
@@ -984,7 +984,7 @@ static int __devinit ad7192_probe(struct spi_device *spi)
 	struct ad7192_platform_data *pdata = spi->dev.platform_data;
 	struct ad7192_state *st;
 	struct iio_dev *indio_dev;
-	int ret, i , voltage_uv = 0;
+	int ret , voltage_uv = 0;
 
 	if (!pdata) {
 		dev_err(&spi->dev, "no platform data?\n");
@@ -1028,17 +1028,11 @@ static int __devinit ad7192_probe(struct spi_device *spi)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = ad7192_channels;
 	indio_dev->num_channels = ARRAY_SIZE(ad7192_channels);
-	indio_dev->available_scan_masks = st->available_scan_masks;
 	if (st->devid == ID_AD7195)
 		indio_dev->info = &ad7195_info;
 	else
 		indio_dev->info = &ad7192_info;
 
-	for (i = 0; i < indio_dev->num_channels; i++)
-		st->available_scan_masks[i] = (1 << i) | (1 <<
-			indio_dev->channels[indio_dev->num_channels - 1].
-			scan_index);
-
 	init_waitqueue_head(&st->wq_data_avail);
 
 	ret = ad7192_register_ring_funcs_and_init(indio_dev);
-- 
1.7.10.4

[PATCH v2 4/6] staging:iio:ad7793: Use iio_validate_scan_mask_onehot

[Lars-Peter Clausen]

Only one of the channels of the ad7793 may be sampled at a time. Use the new
validate_scan_mask callback and the iio_validate_scan_mask_onehot function to
implement this restriction. Previously this was implemented using
available_scan_masks, but this requires a individual scan mask for each channel.

Also the previous code was adding the scan index of the timestamp channel to
each available scan mask, this is not required though since the timestamp
channel is not restricted by the available scan masks and can be enabled or
disabled independently. So the new code does not have to take care of this.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
No changes since v1
---
 drivers/staging/iio/adc/ad7793.c |   14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7793.c b/drivers/staging/iio/adc/ad7793.c
index eaa0cc9..76fdd71 100644
--- a/drivers/staging/iio/adc/ad7793.c
+++ b/drivers/staging/iio/adc/ad7793.c
@@ -52,8 +52,7 @@ struct ad7793_state {
 	u16				mode;
 	u16				conf;
 	u32				scale_avail[8][2];
-	/* Note this uses fact that 8 the mask always fits in a long */
-	unsigned long			available_scan_masks[7];
+
 	/*
 	 * DMA (thus cache coherency maintenance) requires the
 	 * transfer buffers to live in their own cache lines.
@@ -403,6 +402,7 @@ static const struct iio_buffer_setup_ops ad7793_ring_setup_ops = {
 	.postenable = &iio_triggered_buffer_postenable,
 	.predisable = &iio_triggered_buffer_predisable,
 	.postdisable = &ad7793_ring_postdisable,
+	.validate_scan_mask = &iio_validate_scan_mask_onehot,
 };
 
 static int ad7793_register_ring_funcs_and_init(struct iio_dev *indio_dev)
@@ -864,7 +864,7 @@ static int __devinit ad7793_probe(struct spi_device *spi)
 	struct ad7793_platform_data *pdata = spi->dev.platform_data;
 	struct ad7793_state *st;
 	struct iio_dev *indio_dev;
-	int ret, i, voltage_uv = 0;
+	int ret, voltage_uv = 0;
 
 	if (!pdata) {
 		dev_err(&spi->dev, "no platform data?\n");
@@ -910,17 +910,9 @@ static int __devinit ad7793_probe(struct spi_device *spi)
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = st->chip_info->channel;
-	indio_dev->available_scan_masks = st->available_scan_masks;
 	indio_dev->num_channels = 7;
 	indio_dev->info = &ad7793_info;
 
-	for (i = 0; i < indio_dev->num_channels; i++) {
-		set_bit(i, &st->available_scan_masks[i]);
-		set_bit(indio_dev->
-			channels[indio_dev->num_channels - 1].scan_index,
-			&st->available_scan_masks[i]);
-	}
-
 	init_waitqueue_head(&st->wq_data_avail);
 
 	ret = ad7793_register_ring_funcs_and_init(indio_dev);
-- 
1.7.10.4

[PATCH v2 5/6] staging:iio:ade7758: Use iio_validate_scan_mask_onehot

[Lars-Peter Clausen]

Only one of the channels of the ade7758 may be sampled at a time. Use the new
validate_scan_mask callback and the iio_validate_scan_mask_onehot function to
implement this restriction. Previously this was implemented using
available_scan_masks, but this requires a individual scan mask for each channel.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
No changes since v1
---
 drivers/staging/iio/meter/ade7758.h      |    1 -
 drivers/staging/iio/meter/ade7758_core.c |    7 +------
 drivers/staging/iio/meter/ade7758_ring.c |    1 +
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7758.h b/drivers/staging/iio/meter/ade7758.h
index bdd1b05..ec202b4 100644
--- a/drivers/staging/iio/meter/ade7758.h
+++ b/drivers/staging/iio/meter/ade7758.h
@@ -122,7 +122,6 @@ struct ade7758_state {
 	u8			*tx;
 	u8			*rx;
 	struct mutex		buf_lock;
-	unsigned long		available_scan_masks[AD7758_NUM_WAVESRC];
 	struct iio_chan_spec	*ade7758_ring_channels;
 	struct spi_transfer	ring_xfer[4];
 	struct spi_message	ring_msg;
diff --git a/drivers/staging/iio/meter/ade7758_core.c b/drivers/staging/iio/meter/ade7758_core.c
index 96d6114..7014a00 100644
--- a/drivers/staging/iio/meter/ade7758_core.c
+++ b/drivers/staging/iio/meter/ade7758_core.c
@@ -883,7 +883,7 @@ static const struct iio_info ade7758_info = {
 
 static int __devinit ade7758_probe(struct spi_device *spi)
 {
-	int i, ret;
+	int ret;
 	struct ade7758_state *st;
 	struct iio_dev *indio_dev = iio_device_alloc(sizeof(*st));
 
@@ -916,11 +916,6 @@ static int __devinit ade7758_probe(struct spi_device *spi)
 	indio_dev->info = &ade7758_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	for (i = 0; i < AD7758_NUM_WAVESRC; i++)
-		set_bit(i, &st->available_scan_masks[i]);
-
-	indio_dev->available_scan_masks = st->available_scan_masks;
-
 	ret = ade7758_configure_ring(indio_dev);
 	if (ret)
 		goto error_free_tx;
diff --git a/drivers/staging/iio/meter/ade7758_ring.c b/drivers/staging/iio/meter/ade7758_ring.c
index 92159f2..1ce10b2 100644
--- a/drivers/staging/iio/meter/ade7758_ring.c
+++ b/drivers/staging/iio/meter/ade7758_ring.c
@@ -114,6 +114,7 @@ static const struct iio_buffer_setup_ops ade7758_ring_setup_ops = {
 	.preenable = &ade7758_ring_preenable,
 	.postenable = &iio_triggered_buffer_postenable,
 	.predisable = &iio_triggered_buffer_predisable,
+	.validate_scan_mask = &iio_validate_scan_mask_onehot,
 };
 
 void ade7758_unconfigure_ring(struct iio_dev *indio_dev)
-- 
1.7.10.4

[PATCH v2 6/6] iio: iio_buffer_register: Use correct channel when calculating masklength

[Lars-Peter Clausen]

The channel set assigned to the iio device is not necessarily the same has the
channel set passed to iio_buffer_register. So to avoid possible complications
always work with the channel set pass to iio_buffer_register and ignore the
channel set assigned to the iio device.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
No changes since v1
---
 drivers/iio/industrialio-buffer.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 6e00042..e33a9dc 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -292,7 +292,7 @@ int iio_buffer_register(struct iio_dev *indio_dev,
 			if (channels[i].scan_index >
 			    (int)indio_dev->masklength - 1)
 				indio_dev->masklength
-					= indio_dev->channels[i].scan_index + 1;
+					= channels[i].scan_index + 1;
 
 			ret = iio_buffer_add_channel_sysfs(indio_dev,
 							 &channels[i]);
-- 
1.7.10.4

Re: [PATCH v2 6/6] iio: iio_buffer_register: Use correct channel when calculating masklength

[Lars-Peter Clausen]

On 07/09/2012 11:00 AM, Lars-Peter Clausen wrote:
> The channel set assigned to the iio device is not necessarily the same has the
> channel set passed to iio_buffer_register. So to avoid possible complications
> always work with the channel set pass to iio_buffer_register and ignore the
> channel set assigned to the iio device.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Oops, this one was not supposed to be in this series... please just ignore it.

> ---
> No changes since v1
> ---
>  drivers/iio/industrialio-buffer.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 6e00042..e33a9dc 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -292,7 +292,7 @@ int iio_buffer_register(struct iio_dev *indio_dev,
>  			if (channels[i].scan_index >
>  			    (int)indio_dev->masklength - 1)
>  				indio_dev->masklength
> -					= indio_dev->channels[i].scan_index + 1;
> +					= channels[i].scan_index + 1;
>  
>  			ret = iio_buffer_add_channel_sysfs(indio_dev,
>  							 &channels[i]);

Re: [PATCH v2 1/6] iio: Add callback to check whether a scan mask is valid

[Jonathan Cameron]

On 07/09/2012 10:00 AM, Lars-Peter Clausen wrote:
> This is useful for cases where the number of valid scan masks grows
> exponentially, but it is rather easy to check whether a mask is valid or not
> programmatically.
> 
> An example of such a case is a device with multiple ADCs where each ADC has a
> upstream MUX, which allows to select from a number of physical channels.
> 
>   +-------+   +-------+
>   |       |   |       | --- Channel 1
>   | ADC 1 |---| MUX 1 | ---   ...
>   |       |   |       | --- Channel M
>   +-------+   +-------+
> 
>      .            .            .
>      .            .            .
>      .            .            .
> 
>   +-------+   +-------+
>   |       |   |       | --- Channel M * N + 1
>   | ADC N |---| MUX N | ---       ...
>   |       |   |       | --- Channel M * N + M
>   +-------+   +-------+
> 
> The number of necessary scan masks for this case is (M+1)**N - 1, on the other
> hand it is easy to check whether subsets for each ADC of the scanmask have only
> one bit set.
> 
merged first 5 to togreg branch.  Pull request may be a few days.

Jonathan
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> ---
> No changes since v1
> ---
>  drivers/iio/industrialio-buffer.c |   27 ++++++++++++++++++++-------
>  include/linux/iio/iio.h           |    4 ++++
>  2 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 3d8d187..cc5db36 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -570,6 +570,15 @@ int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
>  }
>  EXPORT_SYMBOL(iio_sw_buffer_preenable);
>  
> +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
>   * @buffer: the buffer whose scan mask we are interested in
> @@ -589,27 +598,31 @@ int iio_scan_mask_set(struct iio_dev *indio_dev,
>  		return -ENOMEM;
>  	if (!indio_dev->masklength) {
>  		WARN_ON("trying to set scanmask prior to registering buffer\n");
> -		kfree(trialmask);
> -		return -EINVAL;
> +		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) {
> -			kfree(trialmask);
> -			return -EINVAL;
> -		}
> +		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,
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 2afbb6f..be82936 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -363,12 +363,16 @@ struct iio_info {
>   * @predisable:		[DRIVER] function to run prior to marking buffer
>   *			disabled
>   * @postdisable:	[DRIVER] function to run after marking buffer disabled
> + * @validate_scan_mask: [DRIVER] function callback to check whether a given
> + *			scan mask is valid for the device.
>   */
>  struct iio_buffer_setup_ops {
>  	int				(*preenable)(struct iio_dev *);
>  	int				(*postenable)(struct iio_dev *);
>  	int				(*predisable)(struct iio_dev *);
>  	int				(*postdisable)(struct iio_dev *);
> +	bool (*validate_scan_mask)(struct iio_dev *indio_dev,
> +				   const unsigned long *scan_mask);
>  };
>  
>  /**
>