active_scan_mask

active_scan_mask

[Henk]

Dear all,

I created a custom IIO device in FPGA which in fact contains a bunch of 
ADC channels which I can acquire and stream to disk. I use analog 
devices' libiio and specifically their iio_readdev application together 
with my custom FPGA design and custom iio kernel driver.

Question: what mechanism is updating the indio_dev->active_scan_mask field.

With the iio_readdev application I can add command arguments where I can 
activate the channels that I want to retrieve (voltage0 voltage1 
...etc). When I use 8 channels as command argument in iio_readdev then 
the active_scan_mask seems to be updated with the enabled channels. When 
adding even more channels the field is extended. However, when trying to 
acquire new data and dropping some of the channel arguments then the 
active_scan_mask field does not seem to revert back to the less enabled 
(or requested) channels.

So.. I am searching which mechanisms updates that field with the aim to 
fix that bug.

Any help is appreciated.

Kind regards

Henk

Re: active_scan_mask

[Alexandru Ardelean]

On Wed, May 26, 2021 at 11:18 PM Henk <iio-developer@freedom.nl> wrote:
>
> Dear all,
>
> I created a custom IIO device in FPGA which in fact contains a bunch of
> ADC channels which I can acquire and stream to disk. I use analog
> devices' libiio and specifically their iio_readdev application together
> with my custom FPGA design and custom iio kernel driver.
>
> Question: what mechanism is updating the indio_dev->active_scan_mask field.

That should be happening in drivers/iio/industrialio-buffer.c in the
iio_enable_buffers() function.
The channels get enabled by writing via sysfs in
/sys/bus/iiio/devices/iio:deviceX/scan_elements  in the _en files.

libiio typically does this manipulation of files for you

>
> With the iio_readdev application I can add command arguments where I can
> activate the channels that I want to retrieve (voltage0 voltage1
> ...etc). When I use 8 channels as command argument in iio_readdev then
> the active_scan_mask seems to be updated with the enabled channels. When
> adding even more channels the field is extended. However, when trying to
> acquire new data and dropping some of the channel arguments then the
> active_scan_mask field does not seem to revert back to the less enabled
> (or requested) channels.
>
> So.. I am searching which mechanisms updates that field with the aim to
> fix that bug.

so, are you using an Analog Devices kernel?
or is this a vanilla mainline kernel? and which version?
ADI has some custom patches that deal with this some scanmask manipulation;
is this being reproduced purely with libiio? and which version of libiio?
steps to reproduce?

i'm still not sure if the issue is in kernel or in libiio;

>
> Any help is appreciated.
>
> Kind regards
>
> Henk
>

Re: active_scan_mask

[Henk Medenblik]

Dear Alexandru,

> That should be happening in drivers/iio/industrialio-buffer.c in the
> iio_enable_buffers() function.
> The channels get enabled by writing via sysfs in
> /sys/bus/iiio/devices/iio:deviceX/scan_elements  in the _en files.
>
> libiio typically does this manipulation of files for you

I thought so too and I can confirm that when I switch back to less 
channels that the others are disabled. However the scanmask is not updated.

See some logging here trying to debug:

Session 1, read 8 channels:

iio_readdev -b 262144 -s 1024 iio:device0 voltage0 voltage1 voltage2 
voltage3 voltage4 voltage5 voltage6 voltage7 > /mnt/test6.bin
8 active channels
[  513.529510] acq_buffer_preenable
[  513.529521] active scan mask: 0x000000FF
[  513.558286] acq_buffer_postenable
[  513.716868] acq_buffer_predisable

Session 2, read 16 channels:

iio_readdev -b 262144 -s 1024 iio:device0 voltage0 voltage1 voltage2 
voltage3 voltage4 voltage5 voltage6 voltage7 voltage8 voltage9 voltage10 
voltage11 voltage12 voltage13 voltage14 voltage15 > /mnt/test6.bin
16 active channels
[  523.834056] acq_buffer_preenable
[  523.834067] active scan mask: 0x0000FFFF
[  523.880440] acq_buffer_postenable
[  524.041383] acq_buffer_predisable


Session3 ,  read 8 channels again:

iio_readdev -b 262144 -s 1024 iio:device0 voltage0 voltage1 voltage2 
voltage3 voltage4 voltage5 voltage6 voltage7 > /mnt/test6.bin
8 active channels
[  532.002019] acq_buffer_preenable
[  532.002030] active scan mask: 0x0000FFFF
[  532.048375] onr_acq_buffer_postenable
[  532.131081] onr_acq_buffer_predisable


> so, are you using an Analog Devices kernel?
> or is this a vanilla mainline kernel? and which version?
> ADI has some custom patches that deal with this some scanmask manipulation;
> is this being reproduced purely with libiio? and which version of libiio?
> steps to reproduce?
>
> i'm still not sure if the issue is in kernel or in libiio;
>
Yes I am using a adi 4.14 kernel, git tag adi-4.14.0  15e1a16477

For iio_readdev I am currently using the v0.21 zerocopy checkout

Henk

Re: active_scan_mask

[Alexandru Ardelean]

On Thu, May 27, 2021 at 1:06 PM Henk Medenblik <iio-developer@freedom.nl> wrote:
>
> Dear Alexandru,
>
> > That should be happening in drivers/iio/industrialio-buffer.c in the
> > iio_enable_buffers() function.
> > The channels get enabled by writing via sysfs in
> > /sys/bus/iiio/devices/iio:deviceX/scan_elements  in the _en files.
> >
> > libiio typically does this manipulation of files for you
>
> I thought so too and I can confirm that when I switch back to less
> channels that the others are disabled. However the scanmask is not updated.
>
> See some logging here trying to debug:
>
> Session 1, read 8 channels:
>
> iio_readdev -b 262144 -s 1024 iio:device0 voltage0 voltage1 voltage2
> voltage3 voltage4 voltage5 voltage6 voltage7 > /mnt/test6.bin
> 8 active channels
> [  513.529510] acq_buffer_preenable
> [  513.529521] active scan mask: 0x000000FF
> [  513.558286] acq_buffer_postenable
> [  513.716868] acq_buffer_predisable
>
> Session 2, read 16 channels:
>
> iio_readdev -b 262144 -s 1024 iio:device0 voltage0 voltage1 voltage2
> voltage3 voltage4 voltage5 voltage6 voltage7 voltage8 voltage9 voltage10
> voltage11 voltage12 voltage13 voltage14 voltage15 > /mnt/test6.bin
> 16 active channels
> [  523.834056] acq_buffer_preenable
> [  523.834067] active scan mask: 0x0000FFFF
> [  523.880440] acq_buffer_postenable
> [  524.041383] acq_buffer_predisable
>
>
> Session3 ,  read 8 channels again:
>
> iio_readdev -b 262144 -s 1024 iio:device0 voltage0 voltage1 voltage2
> voltage3 voltage4 voltage5 voltage6 voltage7 > /mnt/test6.bin
> 8 active channels
> [  532.002019] acq_buffer_preenable
> [  532.002030] active scan mask: 0x0000FFFF
> [  532.048375] onr_acq_buffer_postenable
> [  532.131081] onr_acq_buffer_predisable
>
>
> > so, are you using an Analog Devices kernel?
> > or is this a vanilla mainline kernel? and which version?
> > ADI has some custom patches that deal with this some scanmask manipulation;
> > is this being reproduced purely with libiio? and which version of libiio?
> > steps to reproduce?
> >
> > i'm still not sure if the issue is in kernel or in libiio;
> >
> Yes I am using a adi 4.14 kernel, git tag adi-4.14.0  15e1a16477
>
> For iio_readdev I am currently using the v0.21 zerocopy checkout

Hmm, so the "zerocopy" branch isn't  at v0.21.
It's something much older [ ~6 years ? ].
Though the kernel active scan mask is unrelated to libiio, since
iio_readdev is being used in different sessions.

The 4.14 version of the kernel, isn't quite new.
There was some recent fix with regards to active scan-mask computation.
But I am not sure if it's related to this.

Will try to make some time to do some testing on this.


>
> Henk
>
>

Re: active_scan_mask

[Henk Medenblik]

Dear Alexandru,

> Hmm, so the "zerocopy" branch isn't  at v0.21.
> It's something much older [ ~6 years ? ].
> Though the kernel active scan mask is unrelated to libiio, since
> iio_readdev is being used in different sessions.
>
> The 4.14 version of the kernel, isn't quite new.
> There was some recent fix with regards to active scan-mask computation.
> But I am not sure if it's related to this.
>
> Will try to make some time to do some testing on this.
>
I believe it is a bug which still exists in the adi linux kernel sources.
In industrialio-buffer.c :

static int iio_channel_mask_clear(struct iio_dev *indio_dev,
     struct iio_buffer *buffer, int bit)
{
     unsigned int ch;

     clear_bit(bit, buffer->channel_mask);

     memset(buffer->scan_mask, 0, 
BITS_TO_LONGS(indio_dev->masklength));  <---------
     for_each_set_bit(ch, buffer->channel_mask, indio_dev->num_channels)
         set_bit(indio_dev->channels[ch].scan_index, buffer->scan_mask);
     return 0;
}

seems so unnecessary complex...
The memset does not clear the scan_mask completely.

Why not simply :

clear_bit(bit, buffer->scan_mask);

Regards

Henk

RE: active_scan_mask

[Sa, Nuno]

Hi Henk,

> From: Henk Medenblik <iio-developer@freedom.nl>
> Sent: Wednesday, June 9, 2021 9:04 AM
> To: Alexandru Ardelean <ardeleanalex@gmail.com>; Henk Medenblik
> <iio-developer@freedom.nl>
> Cc: linux-iio <linux-iio@vger.kernel.org>
> Subject: Re: active_scan_mask
> 
> [External]
> 
> Dear Alexandru,
> 
> > Hmm, so the "zerocopy" branch isn't  at v0.21.
> > It's something much older [ ~6 years ? ].
> > Though the kernel active scan mask is unrelated to libiio, since
> > iio_readdev is being used in different sessions.
> >
> > The 4.14 version of the kernel, isn't quite new.
> > There was some recent fix with regards to active scan-mask
> computation.
> > But I am not sure if it's related to this.
> >
> > Will try to make some time to do some testing on this.
> >
> I believe it is a bug which still exists in the adi linux kernel sources.
> In industrialio-buffer.c :
> 
> static int iio_channel_mask_clear(struct iio_dev *indio_dev,
>      struct iio_buffer *buffer, int bit)
> {
>      unsigned int ch;
> 
>      clear_bit(bit, buffer->channel_mask);
> 
>      memset(buffer->scan_mask, 0,
> BITS_TO_LONGS(indio_dev->masklength));  <---------
>      for_each_set_bit(ch, buffer->channel_mask, indio_dev-
> >num_channels)
>          set_bit(indio_dev->channels[ch].scan_index, buffer-
> >scan_mask);
>      return 0;
> }
> 

Take a look at [1] for more details. Long story short, I think this is here because we have
some devices where we have bit granularity. Hence, we map all channels to the same
scan_index and to distinguish which channels are enabled, we could not use the current
upstream implementation. I remember some talks about this but not actual patches 
came from it (I will have to go over those emails to see if I can come up with some
patches).

Regarding the memset, that looks like a bug. I guess we want:

memset(buffer->scan_mask, 0, BITS_TO_LONGS(indio_dev->masklength) * sizeof(*buffer->scan_mask));

Ccing Lars as he implemented this patches and might have something to add to
your questions.

ps: feel free to push a PR to ADI tree if you fix this :)

- Nuno Sá

[1]: https://github.com/analogdevicesinc/linux/commit/81d00795b1537a8cfd3eb5152b30ae407c0e16e7

Re: active_scan_mask

[Lars-Peter Clausen]

On 6/9/21 10:19 AM, Sa, Nuno wrote:
> Hi Henk,
>
>> From: Henk Medenblik <iio-developer@freedom.nl>
>> Sent: Wednesday, June 9, 2021 9:04 AM
>> To: Alexandru Ardelean <ardeleanalex@gmail.com>; Henk Medenblik
>> <iio-developer@freedom.nl>
>> Cc: linux-iio <linux-iio@vger.kernel.org>
>> Subject: Re: active_scan_mask
>>
>> [External]
>>
>> Dear Alexandru,
>>
>>> Hmm, so the "zerocopy" branch isn't  at v0.21.
>>> It's something much older [ ~6 years ? ].
>>> Though the kernel active scan mask is unrelated to libiio, since
>>> iio_readdev is being used in different sessions.
>>>
>>> The 4.14 version of the kernel, isn't quite new.
>>> There was some recent fix with regards to active scan-mask
>> computation.
>>> But I am not sure if it's related to this.
>>>
>>> Will try to make some time to do some testing on this.
>>>
>> I believe it is a bug which still exists in the adi linux kernel sources.
>> In industrialio-buffer.c :
>>
>> static int iio_channel_mask_clear(struct iio_dev *indio_dev,
>>       struct iio_buffer *buffer, int bit)
>> {
>>       unsigned int ch;
>>
>>       clear_bit(bit, buffer->channel_mask);
>>
>>       memset(buffer->scan_mask, 0,
>> BITS_TO_LONGS(indio_dev->masklength));  <---------
>>       for_each_set_bit(ch, buffer->channel_mask, indio_dev-
>>> num_channels)
>>           set_bit(indio_dev->channels[ch].scan_index, buffer-
>>> scan_mask);
>>       return 0;
>> }
>>
> Take a look at [1] for more details. Long story short, I think this is here because we have
> some devices where we have bit granularity. Hence, we map all channels to the same
> scan_index and to distinguish which channels are enabled, we could not use the current
> upstream implementation. I remember some talks about this but not actual patches
> came from it (I will have to go over those emails to see if I can come up with some
> patches).
>
> Regarding the memset, that looks like a bug. I guess we want:
>
> memset(buffer->scan_mask, 0, BITS_TO_LONGS(indio_dev->masklength) * sizeof(*buffer->scan_mask));
>
> Ccing Lars as he implemented this patches and might have something to add to
> your questions.
>
Yea, my fault, sorry. The patch looks good.

Re: active_scan_mask

[Henk Medenblik]

I agree,

The patch works.
@Lars, can you create the patch and push it in your analog devices 
kernel tree sources ?

Regards

Henk

On 6/9/21 10:55 AM, Lars-Peter Clausen wrote:
> On 6/9/21 10:19 AM, Sa, Nuno wrote:
>> Hi Henk,
>>
>>> From: Henk Medenblik <iio-developer@freedom.nl>
>>> Sent: Wednesday, June 9, 2021 9:04 AM
>>> To: Alexandru Ardelean <ardeleanalex@gmail.com>; Henk Medenblik
>>> <iio-developer@freedom.nl>
>>> Cc: linux-iio <linux-iio@vger.kernel.org>
>>> Subject: Re: active_scan_mask
>>>
>>> [External]
>>>
>>> Dear Alexandru,
>>>
>>>> Hmm, so the "zerocopy" branch isn't at v0.21.
>>>> It's something much older [ ~6 years ? ].
>>>> Though the kernel active scan mask is unrelated to libiio, since
>>>> iio_readdev is being used in different sessions.
>>>>
>>>> The 4.14 version of the kernel, isn't quite new.
>>>> There was some recent fix with regards to active scan-mask
>>> computation.
>>>> But I am not sure if it's related to this.
>>>>
>>>> Will try to make some time to do some testing on this.
>>>>
>>> I believe it is a bug which still exists in the adi linux kernel 
>>> sources.
>>> In industrialio-buffer.c :
>>>
>>> static int iio_channel_mask_clear(struct iio_dev *indio_dev,
>>>       struct iio_buffer *buffer, int bit)
>>> {
>>>       unsigned int ch;
>>>
>>>       clear_bit(bit, buffer->channel_mask);
>>>
>>>       memset(buffer->scan_mask, 0,
>>> BITS_TO_LONGS(indio_dev->masklength));  <---------
>>>       for_each_set_bit(ch, buffer->channel_mask, indio_dev-
>>>> num_channels)
>>>           set_bit(indio_dev->channels[ch].scan_index, buffer-
>>>> scan_mask);
>>>       return 0;
>>> }
>>>
>> Take a look at [1] for more details. Long story short, I think this 
>> is here because we have
>> some devices where we have bit granularity. Hence, we map all 
>> channels to the same
>> scan_index and to distinguish which channels are enabled, we could 
>> not use the current
>> upstream implementation. I remember some talks about this but not 
>> actual patches
>> came from it (I will have to go over those emails to see if I can 
>> come up with some
>> patches).
>>
>> Regarding the memset, that looks like a bug. I guess we want:
>>
>> memset(buffer->scan_mask, 0, BITS_TO_LONGS(indio_dev->masklength) * 
>> sizeof(*buffer->scan_mask));
>>
>> Ccing Lars as he implemented this patches and might have something to 
>> add to
>> your questions.
>>
> Yea, my fault, sorry. The patch looks good.

Re: active_scan_mask

[Lars-Peter Clausen]

On 6/9/21 11:37 AM, Henk Medenblik wrote:
> I agree,
>
> The patch works.
> @Lars, can you create the patch and push it in your analog devices 
> kernel tree sources ?

Hi,

I can't :) But I'm sure Nuno will be able to do it.

- Lars

>
> Regards
>
> Henk
>
> On 6/9/21 10:55 AM, Lars-Peter Clausen wrote:
>> On 6/9/21 10:19 AM, Sa, Nuno wrote:
>>> Hi Henk,
>>>
>>>> From: Henk Medenblik <iio-developer@freedom.nl>
>>>> Sent: Wednesday, June 9, 2021 9:04 AM
>>>> To: Alexandru Ardelean <ardeleanalex@gmail.com>; Henk Medenblik
>>>> <iio-developer@freedom.nl>
>>>> Cc: linux-iio <linux-iio@vger.kernel.org>
>>>> Subject: Re: active_scan_mask
>>>>
>>>> [External]
>>>>
>>>> Dear Alexandru,
>>>>
>>>>> Hmm, so the "zerocopy" branch isn't at v0.21.
>>>>> It's something much older [ ~6 years ? ].
>>>>> Though the kernel active scan mask is unrelated to libiio, since
>>>>> iio_readdev is being used in different sessions.
>>>>>
>>>>> The 4.14 version of the kernel, isn't quite new.
>>>>> There was some recent fix with regards to active scan-mask
>>>> computation.
>>>>> But I am not sure if it's related to this.
>>>>>
>>>>> Will try to make some time to do some testing on this.
>>>>>
>>>> I believe it is a bug which still exists in the adi linux kernel 
>>>> sources.
>>>> In industrialio-buffer.c :
>>>>
>>>> static int iio_channel_mask_clear(struct iio_dev *indio_dev,
>>>>       struct iio_buffer *buffer, int bit)
>>>> {
>>>>       unsigned int ch;
>>>>
>>>>       clear_bit(bit, buffer->channel_mask);
>>>>
>>>>       memset(buffer->scan_mask, 0,
>>>> BITS_TO_LONGS(indio_dev->masklength));  <---------
>>>>       for_each_set_bit(ch, buffer->channel_mask, indio_dev-
>>>>> num_channels)
>>>>           set_bit(indio_dev->channels[ch].scan_index, buffer-
>>>>> scan_mask);
>>>>       return 0;
>>>> }
>>>>
>>> Take a look at [1] for more details. Long story short, I think this 
>>> is here because we have
>>> some devices where we have bit granularity. Hence, we map all 
>>> channels to the same
>>> scan_index and to distinguish which channels are enabled, we could 
>>> not use the current
>>> upstream implementation. I remember some talks about this but not 
>>> actual patches
>>> came from it (I will have to go over those emails to see if I can 
>>> come up with some
>>> patches).
>>>
>>> Regarding the memset, that looks like a bug. I guess we want:
>>>
>>> memset(buffer->scan_mask, 0, BITS_TO_LONGS(indio_dev->masklength) * 
>>> sizeof(*buffer->scan_mask));
>>>
>>> Ccing Lars as he implemented this patches and might have something 
>>> to add to
>>> your questions.
>>>
>> Yea, my fault, sorry. The patch looks good.

RE: active_scan_mask

[Sa, Nuno]

> From: Lars-Peter Clausen <lars@metafoo.de>
> Sent: Wednesday, June 9, 2021 3:43 PM
> To: Henk Medenblik <iio-developer@freedom.nl>; Sa, Nuno
> <Nuno.Sa@analog.com>; Alexandru Ardelean
> <ardeleanalex@gmail.com>
> Cc: linux-iio <linux-iio@vger.kernel.org>
> Subject: Re: active_scan_mask
> 
> [External]
> 
> On 6/9/21 11:37 AM, Henk Medenblik wrote:
> > I agree,
> >
> > The patch works.
> > @Lars, can you create the patch and push it in your analog devices
> > kernel tree sources ?
> 
> Hi,
> 
> I can't :) But I'm sure Nuno will be able to do it.
> 

I can, I can't just promise when :). Will try to do it soon enough...
Henk, If you are in a hurry for it, you are more than welcome to send a pull request.

- Nuno Sá