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

[PATCH 1/5] 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 the subsets for each ADC of the scanmask have
only one bit set.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 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

[PATCH 2/5] iio: Introduce iio_validate_scan_mask_onehot

[Lars-Peter Clausen]

Add a helper function for validating a scan mask for devices where at most one
channel may be selected at a time. 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>
---
 drivers/iio/industrialio-buffer.c |   15 +++++++++++++++
 include/linux/iio/buffer.h        |    3 +++
 2 files changed, 18 insertions(+)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index cc5db36..bb88377 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -570,6 +570,21 @@ int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
 }
 EXPORT_SYMBOL(iio_sw_buffer_preenable);
 
+/**
+ * iio_validate_scan_mask_onehot() - checks whether one or less bits are set
+ * @indio_dev: the iio device
+ * @mask: scan mask to be checked
+ *
+ * Return true if zero or one bits are set in the scan mask, false otherwise.
+ * It can be used for devices where only one channel can be active 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

[PATCH 3/5] 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>
---
 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

[PATCH 4/5] 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>
---
 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

[PATCH 5/5] 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>
---
 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

Re: [PATCH 2/5] iio: Introduce iio_validate_scan_mask_onehot

[Peter Meerwald]

> Add a helper function for validating a scan mask for devices where at most one
> channel may be selected at a time. This is a common case among devices which
> have scan mask restrictions so it makes sense to provide this function in the
> core.

personally, would find iio_validate_scan_mask_atmost_one clearer than
iio_validate_scan_mask_onehot, but it is a bit longer also

p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH 2/5] iio: Introduce iio_validate_scan_mask_onehot

[Lars-Peter Clausen]

On 07/02/2012 02:52 PM, Peter Meerwald wrote:
> 
>> Add a helper function for validating a scan mask for devices where at most one
>> channel may be selected at a time. This is a common case among devices which
>> have scan mask restrictions so it makes sense to provide this function in the
>> core.
> 
> personally, would find iio_validate_scan_mask_atmost_one clearer than
> iio_validate_scan_mask_onehot, but it is a bit longer also

I think the code can even be changed to be more strict and just check if
bitmap_weight == 1. A mask is only validating after enabling a channel and
not after disabling it, so there will always be at least one bit set when
the validate function is called.

- Lars

Re: [PATCH 2/5] iio: Introduce iio_validate_scan_mask_onehot

[Jonathan Cameron]

On 07/02/2012 02:13 PM, Lars-Peter Clausen wrote:
> On 07/02/2012 02:52 PM, Peter Meerwald wrote:
>>
>>> Add a helper function for validating a scan mask for devices where at most one
>>> channel may be selected at a time. This is a common case among devices which
>>> have scan mask restrictions so it makes sense to provide this function in the
>>> core.
>>
>> personally, would find iio_validate_scan_mask_atmost_one clearer than
>> iio_validate_scan_mask_onehot, but it is a bit longer also
> 
> I think the code can even be changed to be more strict and just check if
> bitmap_weight == 1. A mask is only validating after enabling a channel and
> not after disabling it, so there will always be at least one bit set when
> the validate function is called.
Any of the above is fine with me.  I like this patch set as is, but if
you want to change things feel free!  Either way let me know.
> 
> - Lars
> --
> 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
>