[PATCH V2 0/7] Add scan demux unit and use it in max1363

[PATCH V2 0/7] Add scan demux unit and use it in max1363

[Jonathan Cameron]

Hi All,

New version of this series.  Two changes as per Lars-Peter's
suggestions.  ALIGN macro usage in patch 4 and one of the two
for each bit set suggestions.  The second is subtly different
as it is finding bits after a certain point rather than from
the start.

As explained in patch 5 discussion, I personally feel that
the demux should occur prior to the buffer and avoiding the
extra copy should be done by allowing buffers to provide
callbacks for reserving (plus getting access to) space and
notifying that they are done filling it.   Either way, now
is not the time to do this change. Too much else going on!

Jonathan

Jonathan Cameron (7):
  staging:iio:find iio channel from scan index util function
  staging:iio:buffer add a cache of the timestamp scan index.
  staging:iio: add hook to allow core to perform scan related config.
  staging:iio: make iio_sw_buffer_preenable much more general.
  staging:iio: add demux optionally to path from device to buffer
  staging:iio:adc:max1363 use new demuxing support.
  staging:iio:adc:max1363 correctly set channels as big endian.

 drivers/staging/iio/adc/max1363.h         |   11 ++-
 drivers/staging/iio/adc/max1363_core.c    |   18 ++-
 drivers/staging/iio/adc/max1363_ring.c    |   51 ++-----
 drivers/staging/iio/buffer.h              |   16 ++
 drivers/staging/iio/iio.h                 |   13 ++-
 drivers/staging/iio/industrialio-buffer.c |  213 +++++++++++++++++++++++++----
 drivers/staging/iio/industrialio-core.c   |   11 ++
 7 files changed, 260 insertions(+), 73 deletions(-)

-- 
1.7.7.4

[PATCH 2/7] staging:iio:buffer add a cache of the timestamp scan index.

[Jonathan Cameron]

From: Jonathan Cameron <jic23@cam.ac.uk>

Basically avoids looking it up lots of times.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/buffer.h              |    1 +
 drivers/staging/iio/industrialio-buffer.c |    3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/buffer.h b/drivers/staging/iio/buffer.h
index 9de581e..4b8f619 100644
--- a/drivers/staging/iio/buffer.h
+++ b/drivers/staging/iio/buffer.h
@@ -106,6 +106,7 @@ struct iio_buffer {
 	int					scan_count;
 	long					*scan_mask;
 	bool					scan_timestamp;
+	unsigned				scan_index_timestamp;
 	const struct iio_buffer_access_funcs	*access;
 	const struct iio_buffer_setup_ops		*setup_ops;
 	struct list_head			scan_el_dev_attr_list;
diff --git a/drivers/staging/iio/industrialio-buffer.c b/drivers/staging/iio/industrialio-buffer.c
index 8c55980..b2cf3e3 100644
--- a/drivers/staging/iio/industrialio-buffer.c
+++ b/drivers/staging/iio/industrialio-buffer.c
@@ -313,6 +313,9 @@ int iio_buffer_register(struct iio_dev *indio_dev,
 			if (ret < 0)
 				goto error_cleanup_dynamic;
 			attrcount += ret;
+			if (channels[i].type == IIO_TIMESTAMP)
+				buffer->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),
-- 
1.7.7.4

[PATCH 3/7] staging:iio: add hook to allow core to perform scan related config.

[Jonathan Cameron]

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/iio.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
index 95d6318..4fb8f2f 100644
--- a/drivers/staging/iio/iio.h
+++ b/drivers/staging/iio/iio.h
@@ -262,7 +262,8 @@ struct iio_info {
 				 int val);
 	int (*validate_trigger)(struct iio_dev *indio_dev,
 				struct iio_trigger *trig);
-
+	int (*update_scan_mode)(struct iio_dev *indio_dev,
+				const unsigned long *scan_mask);
 };
 
 /**
-- 
1.7.7.4

[PATCH 4/7] staging:iio: make iio_sw_buffer_preenable much more general.

[Jonathan Cameron]

Also introduces active_scan_mask storage to tell the core what is
really being currently captured from the device (different from
what is desired as often has bonus channels).

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/iio.h                 |    2 +
 drivers/staging/iio/industrialio-buffer.c |   64 +++++++++++++++++------------
 2 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
index 4fb8f2f..c225542 100644
--- a/drivers/staging/iio/iio.h
+++ b/drivers/staging/iio/iio.h
@@ -280,6 +280,7 @@ struct iio_info {
  * @available_scan_masks: [DRIVER] optional array of allowed bitmasks
  * @masklength:		[INTERN] the length of the mask established from
  *			channels
+ * @active_scan_mask:	[INTERN] union of all scan masks requested by buffers
  * @trig:		[INTERN] current device trigger (buffer modes)
  * @pollfunc:		[DRIVER] function run on trigger being received
  * @channels:		[DRIVER] channel specification structure table
@@ -307,6 +308,7 @@ struct iio_dev {
 
 	unsigned long			*available_scan_masks;
 	unsigned			masklength;
+	unsigned long			*active_scan_mask;
 	struct iio_trigger		*trig;
 	struct iio_poll_func		*pollfunc;
 
diff --git a/drivers/staging/iio/industrialio-buffer.c b/drivers/staging/iio/industrialio-buffer.c
index b2cf3e3..7dedeca 100644
--- a/drivers/staging/iio/industrialio-buffer.c
+++ b/drivers/staging/iio/industrialio-buffer.c
@@ -531,32 +531,6 @@ ssize_t iio_buffer_show_enable(struct device *dev,
 }
 EXPORT_SYMBOL(iio_buffer_show_enable);
 
-int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
-{
-	struct iio_buffer *buffer = indio_dev->buffer;
-	size_t size;
-	dev_dbg(&indio_dev->dev, "%s\n", __func__);
-	/* Check if there are any scan elements enabled, if not fail*/
-	if (!(buffer->scan_count || buffer->scan_timestamp))
-		return -EINVAL;
-	if (buffer->scan_timestamp)
-		if (buffer->scan_count)
-			/* Timestamp (aligned to s64) and data */
-			size = (((buffer->scan_count * buffer->bpe)
-					+ sizeof(s64) - 1)
-				& ~(sizeof(s64) - 1))
-				+ sizeof(s64);
-		else /* Timestamp only  */
-			size = sizeof(s64);
-	else /* Data only */
-		size = buffer->scan_count * buffer->bpe;
-	buffer->access->set_bytes_per_datum(buffer, size);
-
-	return 0;
-}
-EXPORT_SYMBOL(iio_sw_buffer_preenable);
-
-
 /* note NULL used as error indicator as it doesn't make sense. */
 static unsigned long *iio_scan_mask_match(unsigned long *av_masks,
 					  unsigned int masklength,
@@ -572,6 +546,44 @@ static unsigned long *iio_scan_mask_match(unsigned long *av_masks,
 	return NULL;
 }
 
+int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct iio_buffer *buffer = indio_dev->buffer;
+	const struct iio_chan_spec *ch;
+	unsigned bytes = 0;
+	int length, i;
+	dev_dbg(&indio_dev->dev, "%s\n", __func__);
+
+	/* How much space will the demuxed element take? */
+	for_each_set_bit(i, buffer->scan_mask,
+			 indio_dev->masklength) {
+		ch = iio_find_channel_from_si(indio_dev, i);
+		length = ch->scan_type.storagebits/8;
+		bytes = ALIGN(bytes, length);
+		bytes += length;
+	}
+	if (buffer->scan_timestamp) {
+		ch = iio_find_channel_from_si(indio_dev,
+					      buffer->scan_index_timestamp);
+		length = ch->scan_type.storagebits/8;
+		if (bytes % length)
+			bytes += length - bytes % length;
+		bytes += length;
+	}
+	buffer->access->set_bytes_per_datum(buffer, bytes);
+
+	/* What scan mask do we actually have ?*/
+	if (indio_dev->available_scan_masks)
+		indio_dev->active_scan_mask =
+			iio_scan_mask_match(indio_dev->available_scan_masks,
+					    indio_dev->masklength,
+					    buffer->scan_mask);
+	else
+		indio_dev->active_scan_mask = buffer->scan_mask;
+	return 0;
+}
+EXPORT_SYMBOL(iio_sw_buffer_preenable);
+
 /**
  * iio_scan_mask_set() - set particular bit in the scan mask
  * @buffer: the buffer whose scan mask we are interested in
-- 
1.7.7.4

[PATCH 7/7] staging:iio:adc:max1363 correctly set channels as big endian.

[Jonathan Cameron]

From: Jonathan Cameron <jic23@cam.ac.uk>

Also, the differential channels should always have been signed.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/adc/max1363_core.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/adc/max1363_core.c b/drivers/staging/iio/adc/max1363_core.c
index 7e078fc..9febd1b 100644
--- a/drivers/staging/iio/adc/max1363_core.c
+++ b/drivers/staging/iio/adc/max1363_core.c
@@ -298,7 +298,12 @@ static const enum max1363_modes max1363_mode_list[] = {
 		.channel = num,						\
 		.address = addr,					\
 		.info_mask = MAX1363_INFO_MASK,				\
-		.scan_type = IIO_ST('u', bits, (bits > 8) ? 16 : 8, 0), \
+		.scan_type = {						\
+			.sign = 'u',					\
+			.realbits = bits,				\
+			.storagebits = (bits > 8) ? 16 : 8,		\
+			.endianness = IIO_BE,				\
+		},							\
 		.scan_index = si,					\
 		.event_mask = evmask,					\
 	}
@@ -313,7 +318,12 @@ static const enum max1363_modes max1363_mode_list[] = {
 		.channel2 = num2,					\
 		.address = addr,					\
 		.info_mask = MAX1363_INFO_MASK,				\
-		.scan_type = IIO_ST('u', bits, (bits > 8) ? 16 : 8, 0), \
+		.scan_type = {						\
+			.sign = 's',					\
+			.realbits = bits,				\
+			.storagebits = (bits > 8) ? 16 : 8,		\
+			.endianness = IIO_BE,				\
+		},							\
 		.scan_index = si,					\
 		.event_mask = evmask,					\
 	}
-- 
1.7.7.4

Re: [PATCH 5/7] staging:iio: add demux optionally to path from device to buffer

[Lars-Peter Clausen]

On 12/04/2011 09:57 PM, Jonathan Cameron wrote:
> From: Jonathan Cameron <jic23@cam.ac.uk>
> 
> This gives you only what you ask for which is handy
> for some devices with weird scan combinations.
> 
> Routes all data flow through a core utility function.
> That and this demuxing support will be needed to do
> demuxing to multiple destinations in kernel.
> 
> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>

Tested-and-Acked-by: Lars-Peter Clausen <lars@metafoo.de>

> ---
>  drivers/staging/iio/buffer.h              |   15 +++
>  drivers/staging/iio/industrialio-buffer.c |  146 ++++++++++++++++++++++++++++-
>  2 files changed, 157 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/buffer.h b/drivers/staging/iio/buffer.h
> index 4b8f619..5282441 100644
> --- a/drivers/staging/iio/buffer.h
> +++ b/drivers/staging/iio/buffer.h
> @@ -95,6 +95,8 @@ struct iio_buffer_setup_ops {
>   * @access:		[DRIVER] buffer access functions associated with the
>   *			implementation.
>   * @flags:		[INTERN] file ops related flags including busy flag.
> + * @demux_list:		[INTERN] list of operations required to demux the scan.
> + * @demux_bounce:	[INTERN] buffer for doing gather from incoming scan.
>   **/
>  struct iio_buffer {
>  	struct iio_dev				*indio_dev;
> @@ -115,6 +117,8 @@ struct iio_buffer {
>  	bool					stufftoread;
>  	unsigned long				flags;
>  	const struct attribute_group *attrs;
> +	struct list_head			demux_list;
> +	unsigned char				*demux_bounce;
>  };
>  
>  /**
> @@ -153,6 +157,17 @@ int iio_scan_mask_set(struct iio_buffer *buffer, int bit);
>  	container_of(d, struct iio_buffer, dev)
>  
>  /**
> + * iio_push_to_buffer() - push to a registered buffer.
> + * @buffer:		IIO buffer structure for device
> + * @scan:		Full scan.
> + * @timestamp:
> + */
> +int iio_push_to_buffer(struct iio_buffer *buffer, unsigned char *data,
> +		       s64 timestamp);
> +
> +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
>   **/
> diff --git a/drivers/staging/iio/industrialio-buffer.c b/drivers/staging/iio/industrialio-buffer.c
> index 7dedeca..20b61a1 100644
> --- a/drivers/staging/iio/industrialio-buffer.c
> +++ b/drivers/staging/iio/industrialio-buffer.c
> @@ -87,6 +87,7 @@ void iio_chrdev_buffer_release(struct iio_dev *indio_dev)
>  
>  void iio_buffer_init(struct iio_buffer *buffer, struct iio_dev *indio_dev)
>  {
> +	INIT_LIST_HEAD(&buffer->demux_list);
>  	buffer->indio_dev = indio_dev;
>  	init_waitqueue_head(&buffer->pollq);
>  }
> @@ -128,10 +129,9 @@ static ssize_t iio_scan_el_show(struct device *dev,
>  	int ret;
>  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  
> -	ret = iio_scan_mask_query(indio_dev->buffer,
> -				  to_iio_dev_attr(attr)->address);
> -	if (ret < 0)
> -		return ret;
> +	ret = test_bit(to_iio_dev_attr(attr)->address,
> +		       indio_dev->buffer->scan_mask);
> +
>  	return sprintf(buf, "%d\n", ret);
>  }
>  
> @@ -580,6 +580,12 @@ int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
>  					    buffer->scan_mask);
>  	else
>  		indio_dev->active_scan_mask = buffer->scan_mask;
> +	iio_update_demux(indio_dev);
> +
> +	if (indio_dev->info->update_scan_mode)
> +		return indio_dev->info
> +			->update_scan_mode(indio_dev,
> +					   indio_dev->active_scan_mask);
>  	return 0;
>  }
>  EXPORT_SYMBOL(iio_sw_buffer_preenable);
> @@ -649,3 +655,135 @@ int iio_scan_mask_query(struct iio_buffer *buffer, int bit)
>  	return test_bit(bit, mask);
>  };
>  EXPORT_SYMBOL_GPL(iio_scan_mask_query);
> +
> +/**
> + * struct iio_demux_table() - table describing demux memcpy ops
> + * @from:	index to copy from
> + * @to:	index to copy to
> + * @length:	how many bytes to copy
> + * @l:		list head used for management
> + */
> +struct iio_demux_table {
> +	unsigned from;
> +	unsigned to;
> +	unsigned length;
> +	struct list_head l;
> +};
> +
> +static unsigned char *iio_demux(struct iio_buffer *buffer,
> +				 unsigned char *datain)
> +{
> +	struct iio_demux_table *t;
> +
> +	if (list_empty(&buffer->demux_list))
> +		return datain;
> +	list_for_each_entry(t, &buffer->demux_list, l)
> +		memcpy(buffer->demux_bounce + t->to,
> +		       datain + t->from, t->length);
> +
> +	return buffer->demux_bounce;
> +}
> +
> +int iio_push_to_buffer(struct iio_buffer *buffer, unsigned char *data,
> +		       s64 timestamp)
> +{
> +	unsigned char *dataout = iio_demux(buffer, data);
> +
> +	return buffer->access->store_to(buffer, dataout, timestamp);
> +}
> +EXPORT_SYMBOL_GPL(iio_push_to_buffer);
> +
> +int iio_update_demux(struct iio_dev *indio_dev)
> +{
> +	const struct iio_chan_spec *ch;
> +	struct iio_buffer *buffer = indio_dev->buffer;
> +	int ret, in_ind = -1, out_ind, length;
> +	unsigned in_loc = 0, out_loc = 0;
> +	struct iio_demux_table *p, *q;
> +
> +	/* Clear out any old demux */
> +	list_for_each_entry_safe(p, q, &buffer->demux_list, l) {
> +		list_del(&p->l);
> +		kfree(p);
> +	}
> +	kfree(buffer->demux_bounce);
> +	buffer->demux_bounce = NULL;
> +
> +	/* First work out which scan mode we will actually have */
> +	if (bitmap_equal(indio_dev->active_scan_mask,
> +			 buffer->scan_mask,
> +			 indio_dev->masklength))
> +		return 0;
> +
> +	/* Now we have the two masks, work from least sig and build up sizes */
> +	for_each_set_bit(out_ind,
> +			 indio_dev->active_scan_mask,
> +			 indio_dev->masklength) {
> +		in_ind = find_next_bit(indio_dev->active_scan_mask,
> +				       indio_dev->masklength,
> +				       in_ind + 1);
> +		while (in_ind != out_ind) {
> +			in_ind = find_next_bit(indio_dev->active_scan_mask,
> +					       indio_dev->masklength,
> +					       in_ind + 1);
> +			ch = iio_find_channel_from_si(indio_dev, in_ind);
> +			length = ch->scan_type.storagebits/8;
> +			/* Make sure we are aligned */
> +			in_loc += length;
> +			if (in_loc % length)
> +				in_loc += length - in_loc % length;
> +		}
> +		p = kmalloc(sizeof(*p), GFP_KERNEL);
> +		if (p == NULL) {
> +			ret = -ENOMEM;
> +			goto error_clear_mux_table;
> +		}
> +		ch = iio_find_channel_from_si(indio_dev, in_ind);
> +		length = ch->scan_type.storagebits/8;
> +		if (out_loc % length)
> +			out_loc += length - out_loc % length;
> +		if (in_loc % length)
> +			in_loc += length - in_loc % length;
> +		p->from = in_loc;
> +		p->to = out_loc;
> +		p->length = length;
> +		list_add_tail(&p->l, &buffer->demux_list);
> +		out_loc += length;
> +		in_loc += length;
> +	}
> +	/* Relies on scan_timestamp being last */
> +	if (buffer->scan_timestamp) {
> +		p = kmalloc(sizeof(*p), GFP_KERNEL);
> +		if (p == NULL) {
> +			ret = -ENOMEM;
> +			goto error_clear_mux_table;
> +		}
> +		ch = iio_find_channel_from_si(indio_dev,
> +			buffer->scan_index_timestamp);
> +		length = ch->scan_type.storagebits/8;
> +		if (out_loc % length)
> +			out_loc += length - out_loc % length;
> +		if (in_loc % length)
> +			in_loc += length - in_loc % length;
> +		p->from = in_loc;
> +		p->to = out_loc;
> +		p->length = length;
> +		list_add_tail(&p->l, &buffer->demux_list);
> +		out_loc += length;
> +		in_loc += length;
> +	}
> +	buffer->demux_bounce = kzalloc(out_loc, GFP_KERNEL);
> +	if (buffer->demux_bounce == NULL) {
> +		ret = -ENOMEM;
> +		goto error_clear_mux_table;
> +	}
> +	return 0;
> +
> +error_clear_mux_table:
> +	list_for_each_entry_safe(p, q, &buffer->demux_list, l) {
> +		list_del(&p->l);
> +		kfree(p);
> +	}
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(iio_update_demux);

Re: [PATCH 4/7] staging:iio: make iio_sw_buffer_preenable much more general.

[Lars-Peter Clausen]

On 12/04/2011 09:56 PM, Jonathan Cameron wrote:
> Also introduces active_scan_mask storage to tell the core what is
> really being currently captured from the device (different from
> what is desired as often has bonus channels).
> 
> Signed-off-by: Jonathan Cameron <jic23@kernel.org>

Tested-and-Acked-by: Lars-Peter Clausen <lars@metafoo.de>

One minor comment inline though.

> ---
>  drivers/staging/iio/iio.h                 |    2 +
>  drivers/staging/iio/industrialio-buffer.c |   64 +++++++++++++++++------------
>  2 files changed, 40 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
> index 4fb8f2f..c225542 100644
> --- a/drivers/staging/iio/iio.h
> +++ b/drivers/staging/iio/iio.h
> @@ -280,6 +280,7 @@ struct iio_info {
>   * @available_scan_masks: [DRIVER] optional array of allowed bitmasks
>   * @masklength:		[INTERN] the length of the mask established from
>   *			channels
> + * @active_scan_mask:	[INTERN] union of all scan masks requested by buffers
>   * @trig:		[INTERN] current device trigger (buffer modes)
>   * @pollfunc:		[DRIVER] function run on trigger being received
>   * @channels:		[DRIVER] channel specification structure table
> @@ -307,6 +308,7 @@ struct iio_dev {
>  
>  	unsigned long			*available_scan_masks;
>  	unsigned			masklength;
> +	unsigned long			*active_scan_mask;
>  	struct iio_trigger		*trig;
>  	struct iio_poll_func		*pollfunc;
>  
> diff --git a/drivers/staging/iio/industrialio-buffer.c b/drivers/staging/iio/industrialio-buffer.c
> index b2cf3e3..7dedeca 100644
> --- a/drivers/staging/iio/industrialio-buffer.c
> +++ b/drivers/staging/iio/industrialio-buffer.c
> @@ -531,32 +531,6 @@ ssize_t iio_buffer_show_enable(struct device *dev,
>  }
>  EXPORT_SYMBOL(iio_buffer_show_enable);
>  
> -int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
> -{
> -	struct iio_buffer *buffer = indio_dev->buffer;
> -	size_t size;
> -	dev_dbg(&indio_dev->dev, "%s\n", __func__);
> -	/* Check if there are any scan elements enabled, if not fail*/
> -	if (!(buffer->scan_count || buffer->scan_timestamp))
> -		return -EINVAL;
> -	if (buffer->scan_timestamp)
> -		if (buffer->scan_count)
> -			/* Timestamp (aligned to s64) and data */
> -			size = (((buffer->scan_count * buffer->bpe)
> -					+ sizeof(s64) - 1)
> -				& ~(sizeof(s64) - 1))
> -				+ sizeof(s64);
> -		else /* Timestamp only  */
> -			size = sizeof(s64);
> -	else /* Data only */
> -		size = buffer->scan_count * buffer->bpe;
> -	buffer->access->set_bytes_per_datum(buffer, size);
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(iio_sw_buffer_preenable);
> -
> -
>  /* note NULL used as error indicator as it doesn't make sense. */
>  static unsigned long *iio_scan_mask_match(unsigned long *av_masks,
>  					  unsigned int masklength,
> @@ -572,6 +546,44 @@ static unsigned long *iio_scan_mask_match(unsigned long *av_masks,
>  	return NULL;
>  }
>  
> +int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct iio_buffer *buffer = indio_dev->buffer;
> +	const struct iio_chan_spec *ch;
> +	unsigned bytes = 0;
> +	int length, i;
> +	dev_dbg(&indio_dev->dev, "%s\n", __func__);
> +
> +	/* How much space will the demuxed element take? */
> +	for_each_set_bit(i, buffer->scan_mask,
> +			 indio_dev->masklength) {
> +		ch = iio_find_channel_from_si(indio_dev, i);
> +		length = ch->scan_type.storagebits/8;
> +		bytes = ALIGN(bytes, length);
> +		bytes += length;
> +	}
> +	if (buffer->scan_timestamp) {
> +		ch = iio_find_channel_from_si(indio_dev,
> +					      buffer->scan_index_timestamp);
> +		length = ch->scan_type.storagebits/8;
> +		if (bytes % length)
> +			bytes += length - bytes % length;
> +		bytes += length;

Could use ALIGN as-well. Btw. ALIGN only works on powers of two. Do we
expect other storage sizes? If yes we should probably use roundup() instead.

> +	}
> +	buffer->access->set_bytes_per_datum(buffer, bytes);
> +
> +	/* What scan mask do we actually have ?*/
> +	if (indio_dev->available_scan_masks)
> +		indio_dev->active_scan_mask =
> +			iio_scan_mask_match(indio_dev->available_scan_masks,
> +					    indio_dev->masklength,
> +					    buffer->scan_mask);
> +	else
> +		indio_dev->active_scan_mask = buffer->scan_mask;
> +	return 0;
> +}
> +EXPORT_SYMBOL(iio_sw_buffer_preenable);
> +
>  /**
>   * iio_scan_mask_set() - set particular bit in the scan mask
>   * @buffer: the buffer whose scan mask we are interested in

Re: [PATCH 4/7] staging:iio: make iio_sw_buffer_preenable much more general.

[Jonathan Cameron]

Lars-Peter Clausen <lars@metafoo.de> wrote:

>On 12/04/2011 09:56 PM, Jonathan Cameron wrote:
>> Also introduces active_scan_mask storage to tell the core what is
>> really being currently captured from the device (different from
>> what is desired as often has bonus channels).
>> 
>> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
>
>Tested-and-Acked-by: Lars-Peter Clausen <lars@metafoo.de>
>
>One minor comment inline though.
>
>> ---
>>  drivers/staging/iio/iio.h                 |    2 +
>>  drivers/staging/iio/industrialio-buffer.c |   64
>+++++++++++++++++------------
>>  2 files changed, 40 insertions(+), 26 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
>> index 4fb8f2f..c225542 100644
>> --- a/drivers/staging/iio/iio.h
>> +++ b/drivers/staging/iio/iio.h
>> @@ -280,6 +280,7 @@ struct iio_info {
>>   * @available_scan_masks: [DRIVER] optional array of allowed
>bitmasks
>>   * @masklength:		[INTERN] the length of the mask established from
>>   *			channels
>> + * @active_scan_mask:	[INTERN] union of all scan masks requested by
>buffers
>>   * @trig:		[INTERN] current device trigger (buffer modes)
>>   * @pollfunc:		[DRIVER] function run on trigger being received
>>   * @channels:		[DRIVER] channel specification structure table
>> @@ -307,6 +308,7 @@ struct iio_dev {
>>  
>>  	unsigned long			*available_scan_masks;
>>  	unsigned			masklength;
>> +	unsigned long			*active_scan_mask;
>>  	struct iio_trigger		*trig;
>>  	struct iio_poll_func		*pollfunc;
>>  
>> diff --git a/drivers/staging/iio/industrialio-buffer.c
>b/drivers/staging/iio/industrialio-buffer.c
>> index b2cf3e3..7dedeca 100644
>> --- a/drivers/staging/iio/industrialio-buffer.c
>> +++ b/drivers/staging/iio/industrialio-buffer.c
>> @@ -531,32 +531,6 @@ ssize_t iio_buffer_show_enable(struct device
>*dev,
>>  }
>>  EXPORT_SYMBOL(iio_buffer_show_enable);
>>  
>> -int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
>> -{
>> -	struct iio_buffer *buffer = indio_dev->buffer;
>> -	size_t size;
>> -	dev_dbg(&indio_dev->dev, "%s\n", __func__);
>> -	/* Check if there are any scan elements enabled, if not fail*/
>> -	if (!(buffer->scan_count || buffer->scan_timestamp))
>> -		return -EINVAL;
>> -	if (buffer->scan_timestamp)
>> -		if (buffer->scan_count)
>> -			/* Timestamp (aligned to s64) and data */
>> -			size = (((buffer->scan_count * buffer->bpe)
>> -					+ sizeof(s64) - 1)
>> -				& ~(sizeof(s64) - 1))
>> -				+ sizeof(s64);
>> -		else /* Timestamp only  */
>> -			size = sizeof(s64);
>> -	else /* Data only */
>> -		size = buffer->scan_count * buffer->bpe;
>> -	buffer->access->set_bytes_per_datum(buffer, size);
>> -
>> -	return 0;
>> -}
>> -EXPORT_SYMBOL(iio_sw_buffer_preenable);
>> -
>> -
>>  /* note NULL used as error indicator as it doesn't make sense. */
>>  static unsigned long *iio_scan_mask_match(unsigned long *av_masks,
>>  					  unsigned int masklength,
>> @@ -572,6 +546,44 @@ static unsigned long
>*iio_scan_mask_match(unsigned long *av_masks,
>>  	return NULL;
>>  }
>>  
>> +int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
>> +{
>> +	struct iio_buffer *buffer = indio_dev->buffer;
>> +	const struct iio_chan_spec *ch;
>> +	unsigned bytes = 0;
>> +	int length, i;
>> +	dev_dbg(&indio_dev->dev, "%s\n", __func__);
>> +
>> +	/* How much space will the demuxed element take? */
>> +	for_each_set_bit(i, buffer->scan_mask,
>> +			 indio_dev->masklength) {
>> +		ch = iio_find_channel_from_si(indio_dev, i);
>> +		length = ch->scan_type.storagebits/8;
>> +		bytes = ALIGN(bytes, length);
>> +		bytes += length;
>> +	}
>> +	if (buffer->scan_timestamp) {
>> +		ch = iio_find_channel_from_si(indio_dev,
>> +					      buffer->scan_index_timestamp);
>> +		length = ch->scan_type.storagebits/8;
>> +		if (bytes % length)
>> +			bytes += length - bytes % length;
>> +		bytes += length;
>
>Could use ALIGN as-well. Btw. ALIGN only works on powers of two. Do we
>expect other storage sizes? If yes we should probably use roundup()
>instead.
>
Probably only powers of 2.  We can change if /when it becomes necessary.
>> +	}
>> +	buffer->access->set_bytes_per_datum(buffer, bytes);
>> +
>> +	/* What scan mask do we actually have ?*/
>> +	if (indio_dev->available_scan_masks)
>> +		indio_dev->active_scan_mask =
>> +			iio_scan_mask_match(indio_dev->available_scan_masks,
>> +					    indio_dev->masklength,
>> +					    buffer->scan_mask);
>> +	else
>> +		indio_dev->active_scan_mask = buffer->scan_mask;
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(iio_sw_buffer_preenable);
>> +
>>  /**
>>   * iio_scan_mask_set() - set particular bit in the scan mask
>>   * @buffer: the buffer whose scan mask we are interested in

-- 
Sent from my Android phone 
with K-9 Mail. Please excuse my brevity.

[PATCH 4/7] staging:iio: make iio_sw_buffer_preenable much more general.

[Jonathan Cameron]

Also introduces active_scan_mask storage to tell the core what is
really being currently captured from the device (different from
what is desired as often has bonus channels).

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
Tested-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/iio.h                 |    2 +
 drivers/staging/iio/industrialio-buffer.c |   63 +++++++++++++++++------------
 2 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
index 4fb8f2f..c225542 100644
--- a/drivers/staging/iio/iio.h
+++ b/drivers/staging/iio/iio.h
@@ -280,6 +280,7 @@ struct iio_info {
  * @available_scan_masks: [DRIVER] optional array of allowed bitmasks
  * @masklength:		[INTERN] the length of the mask established from
  *			channels
+ * @active_scan_mask:	[INTERN] union of all scan masks requested by buffers
  * @trig:		[INTERN] current device trigger (buffer modes)
  * @pollfunc:		[DRIVER] function run on trigger being received
  * @channels:		[DRIVER] channel specification structure table
@@ -307,6 +308,7 @@ struct iio_dev {
 
 	unsigned long			*available_scan_masks;
 	unsigned			masklength;
+	unsigned long			*active_scan_mask;
 	struct iio_trigger		*trig;
 	struct iio_poll_func		*pollfunc;
 
diff --git a/drivers/staging/iio/industrialio-buffer.c b/drivers/staging/iio/industrialio-buffer.c
index b2cf3e3..e088a5c 100644
--- a/drivers/staging/iio/industrialio-buffer.c
+++ b/drivers/staging/iio/industrialio-buffer.c
@@ -531,32 +531,6 @@ ssize_t iio_buffer_show_enable(struct device *dev,
 }
 EXPORT_SYMBOL(iio_buffer_show_enable);
 
-int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
-{
-	struct iio_buffer *buffer = indio_dev->buffer;
-	size_t size;
-	dev_dbg(&indio_dev->dev, "%s\n", __func__);
-	/* Check if there are any scan elements enabled, if not fail*/
-	if (!(buffer->scan_count || buffer->scan_timestamp))
-		return -EINVAL;
-	if (buffer->scan_timestamp)
-		if (buffer->scan_count)
-			/* Timestamp (aligned to s64) and data */
-			size = (((buffer->scan_count * buffer->bpe)
-					+ sizeof(s64) - 1)
-				& ~(sizeof(s64) - 1))
-				+ sizeof(s64);
-		else /* Timestamp only  */
-			size = sizeof(s64);
-	else /* Data only */
-		size = buffer->scan_count * buffer->bpe;
-	buffer->access->set_bytes_per_datum(buffer, size);
-
-	return 0;
-}
-EXPORT_SYMBOL(iio_sw_buffer_preenable);
-
-
 /* note NULL used as error indicator as it doesn't make sense. */
 static unsigned long *iio_scan_mask_match(unsigned long *av_masks,
 					  unsigned int masklength,
@@ -572,6 +546,43 @@ static unsigned long *iio_scan_mask_match(unsigned long *av_masks,
 	return NULL;
 }
 
+int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct iio_buffer *buffer = indio_dev->buffer;
+	const struct iio_chan_spec *ch;
+	unsigned bytes = 0;
+	int length, i;
+	dev_dbg(&indio_dev->dev, "%s\n", __func__);
+
+	/* How much space will the demuxed element take? */
+	for_each_set_bit(i, buffer->scan_mask,
+			 indio_dev->masklength) {
+		ch = iio_find_channel_from_si(indio_dev, i);
+		length = ch->scan_type.storagebits/8;
+		bytes = ALIGN(bytes, length);
+		bytes += length;
+	}
+	if (buffer->scan_timestamp) {
+		ch = iio_find_channel_from_si(indio_dev,
+					      buffer->scan_index_timestamp);
+		length = ch->scan_type.storagebits/8;
+		bytes = ALIGN(bytes, length);
+		bytes += length;
+	}
+	buffer->access->set_bytes_per_datum(buffer, bytes);
+
+	/* What scan mask do we actually have ?*/
+	if (indio_dev->available_scan_masks)
+		indio_dev->active_scan_mask =
+			iio_scan_mask_match(indio_dev->available_scan_masks,
+					    indio_dev->masklength,
+					    buffer->scan_mask);
+	else
+		indio_dev->active_scan_mask = buffer->scan_mask;
+	return 0;
+}
+EXPORT_SYMBOL(iio_sw_buffer_preenable);
+
 /**
  * iio_scan_mask_set() - set particular bit in the scan mask
  * @buffer: the buffer whose scan mask we are interested in
-- 
1.7.7.4

Re: [PATCH 4/7] staging:iio: make iio_sw_buffer_preenable much more general.

[Jonathan Cameron]

...
>> +int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
>> +{
>> +	struct iio_buffer *buffer = indio_dev->buffer;
>> +	const struct iio_chan_spec *ch;
>> +	unsigned bytes = 0;
>> +	int length, i;
>> +	dev_dbg(&indio_dev->dev, "%s\n", __func__);
>> +
>> +	/* How much space will the demuxed element take? */
>> +	for_each_set_bit(i, buffer->scan_mask,
>> +			 indio_dev->masklength) {
>> +		ch = iio_find_channel_from_si(indio_dev, i);
>> +		length = ch->scan_type.storagebits/8;
>> +		if (bytes % length)
>> +			bytes += length - bytes % length;
> 
> bytes = ALIGN(bytes, length); would probably make it more obvious what is
> going on here.
Good idea. Thanks.

Re: [PATCH 4/7] staging:iio: make iio_sw_buffer_preenable much more general.

[Lars-Peter Clausen]

On 11/27/2011 02:10 PM, Jonathan Cameron wrote:
> Also introduces active_scan_mask storage to tell the core what is
> really being currently captured from the device (different from
> what is desired as often has bonus channels).
> 
> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/iio.h                 |    2 +
>  drivers/staging/iio/industrialio-buffer.c |   65 +++++++++++++++++-----------
>  2 files changed, 41 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
> index 4fb8f2f..c225542 100644
> --- a/drivers/staging/iio/iio.h
> +++ b/drivers/staging/iio/iio.h
> @@ -280,6 +280,7 @@ struct iio_info {
>   * @available_scan_masks: [DRIVER] optional array of allowed bitmasks
>   * @masklength:		[INTERN] the length of the mask established from
>   *			channels
> + * @active_scan_mask:	[INTERN] union of all scan masks requested by buffers
>   * @trig:		[INTERN] current device trigger (buffer modes)
>   * @pollfunc:		[DRIVER] function run on trigger being received
>   * @channels:		[DRIVER] channel specification structure table
> @@ -307,6 +308,7 @@ struct iio_dev {
>  
>  	unsigned long			*available_scan_masks;
>  	unsigned			masklength;
> +	unsigned long			*active_scan_mask;
>  	struct iio_trigger		*trig;
>  	struct iio_poll_func		*pollfunc;
>  
> diff --git a/drivers/staging/iio/industrialio-buffer.c b/drivers/staging/iio/industrialio-buffer.c
> index cae7922..fb14e37 100644
> --- a/drivers/staging/iio/industrialio-buffer.c
> +++ b/drivers/staging/iio/industrialio-buffer.c
> @@ -533,32 +533,6 @@ ssize_t iio_buffer_show_enable(struct device *dev,
>  }
>  EXPORT_SYMBOL(iio_buffer_show_enable);
>  
> -int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
> -{
> -	struct iio_buffer *buffer = indio_dev->buffer;
> -	size_t size;
> -	dev_dbg(&indio_dev->dev, "%s\n", __func__);
> -	/* Check if there are any scan elements enabled, if not fail*/
> -	if (!(buffer->scan_count || buffer->scan_timestamp))
> -		return -EINVAL;
> -	if (buffer->scan_timestamp)
> -		if (buffer->scan_count)
> -			/* Timestamp (aligned to s64) and data */
> -			size = (((buffer->scan_count * buffer->bpe)
> -					+ sizeof(s64) - 1)
> -				& ~(sizeof(s64) - 1))
> -				+ sizeof(s64);
> -		else /* Timestamp only  */
> -			size = sizeof(s64);
> -	else /* Data only */
> -		size = buffer->scan_count * buffer->bpe;
> -	buffer->access->set_bytes_per_datum(buffer, size);
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(iio_sw_buffer_preenable);
> -
> -
>  /* note NULL used as error indicator as it doesn't make sense. */
>  static unsigned long *iio_scan_mask_match(unsigned long *av_masks,
>  					  unsigned int masklength,
> @@ -574,6 +548,45 @@ static unsigned long *iio_scan_mask_match(unsigned long *av_masks,
>  	return NULL;
>  }
>  
> +int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct iio_buffer *buffer = indio_dev->buffer;
> +	const struct iio_chan_spec *ch;
> +	unsigned bytes = 0;
> +	int length, i;
> +	dev_dbg(&indio_dev->dev, "%s\n", __func__);
> +
> +	/* How much space will the demuxed element take? */
> +	for_each_set_bit(i, buffer->scan_mask,
> +			 indio_dev->masklength) {
> +		ch = iio_find_channel_from_si(indio_dev, i);
> +		length = ch->scan_type.storagebits/8;
> +		if (bytes % length)
> +			bytes += length - bytes % length;

bytes = ALIGN(bytes, length); would probably make it more obvious what is
going on here.

> +		bytes += length;
> +	}
> +	if (buffer->scan_timestamp) {
> +		ch = iio_find_channel_from_si(indio_dev,
> +					      buffer->scan_index_timestamp);
> +		length = ch->scan_type.storagebits/8;
> +		if (bytes % length)
> +			bytes += length - bytes % length;
> +		bytes += length;
> +	}
> +	buffer->access->set_bytes_per_datum(buffer, bytes);
> +
> +	/* What scan mask do we actually have ?*/
> +	if (indio_dev->available_scan_masks)
> +		indio_dev->active_scan_mask =
> +			iio_scan_mask_match(indio_dev->available_scan_masks,
> +					    indio_dev->masklength,
> +					    buffer->scan_mask);
> +	else
> +		indio_dev->active_scan_mask = buffer->scan_mask;
> +	return 0;
> +}
> +EXPORT_SYMBOL(iio_sw_buffer_preenable);
> +
>  /**
>   * iio_scan_mask_set() - set particular bit in the scan mask
>   * @buffer: the buffer whose scan mask we are interested in

[PATCH 4/7] staging:iio: make iio_sw_buffer_preenable much more general.

[Jonathan Cameron]

Also introduces active_scan_mask storage to tell the core what is
really being currently captured from the device (different from
what is desired as often has bonus channels).

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/iio.h                 |    2 +
 drivers/staging/iio/industrialio-buffer.c |   65 +++++++++++++++++-----------
 2 files changed, 41 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
index 4fb8f2f..c225542 100644
--- a/drivers/staging/iio/iio.h
+++ b/drivers/staging/iio/iio.h
@@ -280,6 +280,7 @@ struct iio_info {
  * @available_scan_masks: [DRIVER] optional array of allowed bitmasks
  * @masklength:		[INTERN] the length of the mask established from
  *			channels
+ * @active_scan_mask:	[INTERN] union of all scan masks requested by buffers
  * @trig:		[INTERN] current device trigger (buffer modes)
  * @pollfunc:		[DRIVER] function run on trigger being received
  * @channels:		[DRIVER] channel specification structure table
@@ -307,6 +308,7 @@ struct iio_dev {
 
 	unsigned long			*available_scan_masks;
 	unsigned			masklength;
+	unsigned long			*active_scan_mask;
 	struct iio_trigger		*trig;
 	struct iio_poll_func		*pollfunc;
 
diff --git a/drivers/staging/iio/industrialio-buffer.c b/drivers/staging/iio/industrialio-buffer.c
index cae7922..fb14e37 100644
--- a/drivers/staging/iio/industrialio-buffer.c
+++ b/drivers/staging/iio/industrialio-buffer.c
@@ -533,32 +533,6 @@ ssize_t iio_buffer_show_enable(struct device *dev,
 }
 EXPORT_SYMBOL(iio_buffer_show_enable);
 
-int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
-{
-	struct iio_buffer *buffer = indio_dev->buffer;
-	size_t size;
-	dev_dbg(&indio_dev->dev, "%s\n", __func__);
-	/* Check if there are any scan elements enabled, if not fail*/
-	if (!(buffer->scan_count || buffer->scan_timestamp))
-		return -EINVAL;
-	if (buffer->scan_timestamp)
-		if (buffer->scan_count)
-			/* Timestamp (aligned to s64) and data */
-			size = (((buffer->scan_count * buffer->bpe)
-					+ sizeof(s64) - 1)
-				& ~(sizeof(s64) - 1))
-				+ sizeof(s64);
-		else /* Timestamp only  */
-			size = sizeof(s64);
-	else /* Data only */
-		size = buffer->scan_count * buffer->bpe;
-	buffer->access->set_bytes_per_datum(buffer, size);
-
-	return 0;
-}
-EXPORT_SYMBOL(iio_sw_buffer_preenable);
-
-
 /* note NULL used as error indicator as it doesn't make sense. */
 static unsigned long *iio_scan_mask_match(unsigned long *av_masks,
 					  unsigned int masklength,
@@ -574,6 +548,45 @@ static unsigned long *iio_scan_mask_match(unsigned long *av_masks,
 	return NULL;
 }
 
+int iio_sw_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct iio_buffer *buffer = indio_dev->buffer;
+	const struct iio_chan_spec *ch;
+	unsigned bytes = 0;
+	int length, i;
+	dev_dbg(&indio_dev->dev, "%s\n", __func__);
+
+	/* How much space will the demuxed element take? */
+	for_each_set_bit(i, buffer->scan_mask,
+			 indio_dev->masklength) {
+		ch = iio_find_channel_from_si(indio_dev, i);
+		length = ch->scan_type.storagebits/8;
+		if (bytes % length)
+			bytes += length - bytes % length;
+		bytes += length;
+	}
+	if (buffer->scan_timestamp) {
+		ch = iio_find_channel_from_si(indio_dev,
+					      buffer->scan_index_timestamp);
+		length = ch->scan_type.storagebits/8;
+		if (bytes % length)
+			bytes += length - bytes % length;
+		bytes += length;
+	}
+	buffer->access->set_bytes_per_datum(buffer, bytes);
+
+	/* What scan mask do we actually have ?*/
+	if (indio_dev->available_scan_masks)
+		indio_dev->active_scan_mask =
+			iio_scan_mask_match(indio_dev->available_scan_masks,
+					    indio_dev->masklength,
+					    buffer->scan_mask);
+	else
+		indio_dev->active_scan_mask = buffer->scan_mask;
+	return 0;
+}
+EXPORT_SYMBOL(iio_sw_buffer_preenable);
+
 /**
  * iio_scan_mask_set() - set particular bit in the scan mask
  * @buffer: the buffer whose scan mask we are interested in
-- 
1.7.7.3