> From: linux-iio-owner@xxxxxxxxxxxxxxx <linux-iio-owner@xxxxxxxxxxxxxxx>
> On Behalf Of Alexandru Ardelean
> Sent: Freitag, 24. April 2020 07:18
> To: linux-iio@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: jic23@xxxxxxxxxx; lars@xxxxxxxxxx; Ardelean, Alexandru
> <alexandru.Ardelean@xxxxxxxxxx>
> Subject: [RFC PATCH 4/4] iio: Track enabled channels on a per channel basis
>
> From: Lars-Peter Clausen <lars@xxxxxxxxxx>
>
> Now that we support multiple channels with the same scan index we can no
> longer use the scan mask to track which channels have been enabled.
> Otherwise it is not possible to enable channels with the same scan index
> independently.
>
> Introduce a new channel mask which is used instead of the scan mask to
> track which channels are enabled. Whenever the channel mask is changed a
> new scan mask is computed based on it.
>
> Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
> ---
> drivers/iio/industrialio-buffer.c | 62 +++++++++++++++++++++----------
> drivers/iio/inkern.c | 19 +++++++++-
> include/linux/iio/buffer_impl.h | 3 ++
> include/linux/iio/consumer.h | 2 +
> 4 files changed, 64 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index c06691281287..1821a3e32fb3 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -216,12 +216,20 @@ int iio_buffer_alloc_scanmask(struct iio_buffer
> *buffer,
> if (buffer->scan_mask == NULL)
> return -ENOMEM;
>
> + buffer->channel_mask = bitmap_zalloc(indio_dev->num_channels,
> + GFP_KERNEL);
> + if (buffer->channel_mask == NULL) {
> + bitmap_free(buffer->scan_mask);
> + return -ENOMEM;
> + }
> +
> return 0;
> }
> EXPORT_SYMBOL_GPL(iio_buffer_alloc_scanmask);
>
> void iio_buffer_free_scanmask(struct iio_buffer *buffer)
> {
> + bitmap_free(buffer->channel_mask);
> bitmap_free(buffer->scan_mask);
> }
> EXPORT_SYMBOL_GPL(iio_buffer_free_scanmask);
> @@ -285,7 +293,7 @@ static ssize_t iio_scan_el_show(struct device *dev,
>
> /* Ensure ret is 0 or 1. */
> ret = !!test_bit(to_iio_dev_attr(attr)->address,
> - indio_dev->buffer->scan_mask);
> + indio_dev->buffer->channel_mask);
>
> return sprintf(buf, "%d\n", ret);
> }
> @@ -330,11 +338,12 @@ static bool iio_validate_scan_mask(struct iio_dev
> *indio_dev,
> * 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)
> +static int iio_channel_mask_set(struct iio_dev *indio_dev,
> + struct iio_buffer *buffer, int bit)
> {
> const unsigned long *mask;
> unsigned long *trialmask;
> + unsigned int ch;
>
> trialmask = bitmap_zalloc(indio_dev->masklength, GFP_KERNEL);
> if (trialmask == NULL)
> @@ -343,8 +352,11 @@ static int iio_scan_mask_set(struct iio_dev
> *indio_dev,
> WARN(1, "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);
> +
> + set_bit(bit, buffer->channel_mask);
> +
> + for_each_set_bit(ch, buffer->channel_mask, indio_dev-
> >num_channels)
> + set_bit(indio_dev->channels[ch].scan_index, trialmask);
So, here if the channels all have the same scan_index, we will end up with a scan_mask which is
different that channel_mask, right? I saw that in our internal driver's we then just access the
channel_mask field directly to know what pieces/channels do we need to enable prior to
buffering, which implies including buffer_impl.h.
So, for me it would make sense to compute scan_mask so that it will be the same as channel_mask
(hmm but that would be a problem when computing the buffer size...) and drivers can correctly use
` validate_scan_mask ()` cb. Alternatively, we need to expose channel_mask either on a new cb or
change the ` validate_scan_mask ()` footprint.
- Nuno Sá
