linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] iio: buffer: Don't allow buffers without any channels enabled to be activated
       [not found] <20200320104031.31701-1-alexandru.ardelean@analog.com>
@ 2020-03-25 10:01 ` Alexandru Ardelean
  2020-03-25 10:12   ` Ardelean, Alexandru
  2020-03-25 15:57   ` Andy Shevchenko
  2020-03-26  9:30 ` [PATCH v3] " Alexandru Ardelean
  1 sibling, 2 replies; 8+ messages in thread
From: Alexandru Ardelean @ 2020-03-25 10:01 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: jic23, lars, knaack.h, pmeerw, Alexandru Ardelean

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

Before activating a buffer make sure that at least one channel is enabled.
Activating a buffer with 0 channels enabled doesn't make too much sense and
disallowing this case makes sure that individual driver don't have to add
special case code to handle it.

Currently, without this patch enabling a buffer is possible and no error is
produced. With this patch -EINVAL is returned.

An example of execution with this patch and some instrumented print-code:
   root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
   root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
   0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer->scan_mask 00000000
   1: iio_verify_update 753
   2:__iio_update_buffers 1115 ret -22
   3: iio_buffer_store_enable 1241 ret -22
   -bash: echo: write error: Invalid argument
1, 2 & 3 are exit-error paths. 0 the first print in iio_verify_update()
rergardless of error path.

Without this patch (and same instrumented print-code):
   root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
   root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
   0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer->scan_mask 00000000
   root@analog:/sys/bus/iio/devices/iio:device3/buffer#
Buffer is enabled with no error.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changelog v1 -> v2:
* moved check in iio_verify_update()
* added dev_dbg() message; should help some folks understand the message
* documented steps to reproduce
* added Fixes tag; hopefully the tag is the good one; if needed, it can be
  dropped; this has been around for ~8 years; no idea if it's worth
  backporting

 drivers/iio/industrialio-buffer.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 5ff34ce8b6a2..e6fa1a4e135d 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -686,6 +686,13 @@ static int iio_verify_update(struct iio_dev *indio_dev,
 	bool scan_timestamp;
 	unsigned int modes;
 
+	if (insert_buffer &&
+	    bitmap_empty(insert_buffer->scan_mask, indio_dev->masklength)) {
+		dev_dbg(&indio_dev->dev,
+			"At least one scan element must be enabled first\n");
+		return -EINVAL;
+	}
+
 	memset(config, 0, sizeof(*config));
 	config->watermark = ~0;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] iio: buffer: Don't allow buffers without any channels enabled to be activated
  2020-03-25 10:01 ` [PATCH v2] iio: buffer: Don't allow buffers without any channels enabled to be activated Alexandru Ardelean
@ 2020-03-25 10:12   ` Ardelean, Alexandru
  2020-03-25 15:57   ` Andy Shevchenko
  1 sibling, 0 replies; 8+ messages in thread
From: Ardelean, Alexandru @ 2020-03-25 10:12 UTC (permalink / raw)
  To: linux-kernel, linux-iio; +Cc: jic23, lars, knaack.h, pmeerw

On Wed, 2020-03-25 at 12:01 +0200, Alexandru Ardelean wrote:
> From: Lars-Peter Clausen <lars@metafoo.de>
> 
> Before activating a buffer make sure that at least one channel is enabled.
> Activating a buffer with 0 channels enabled doesn't make too much sense and
> disallowing this case makes sure that individual driver don't have to add
> special case code to handle it.
> 

oops, i goof-ed peter's email on this send;
[ copy + paste err ]
should be fixed now;

> Currently, without this patch enabling a buffer is possible and no error is
> produced. With this patch -EINVAL is returned.
> 
> An example of execution with this patch and some instrumented print-code:
>    root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
>    root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
>    0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer->scan_mask
> 00000000
>    1: iio_verify_update 753
>    2:__iio_update_buffers 1115 ret -22
>    3: iio_buffer_store_enable 1241 ret -22
>    -bash: echo: write error: Invalid argument
> 1, 2 & 3 are exit-error paths. 0 the first print in iio_verify_update()
> rergardless of error path.
> 
> Without this patch (and same instrumented print-code):
>    root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
>    root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
>    0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer->scan_mask
> 00000000
>    root@analog:/sys/bus/iio/devices/iio:device3/buffer#
> Buffer is enabled with no error.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> 
> Changelog v1 -> v2:
> * moved check in iio_verify_update()
> * added dev_dbg() message; should help some folks understand the message
> * documented steps to reproduce
> * added Fixes tag; hopefully the tag is the good one; if needed, it can be
>   dropped; this has been around for ~8 years; no idea if it's worth
>   backporting
> 
>  drivers/iio/industrialio-buffer.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-
> buffer.c
> index 5ff34ce8b6a2..e6fa1a4e135d 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -686,6 +686,13 @@ static int iio_verify_update(struct iio_dev *indio_dev,
>  	bool scan_timestamp;
>  	unsigned int modes;
>  
> +	if (insert_buffer &&
> +	    bitmap_empty(insert_buffer->scan_mask, indio_dev->masklength)) {
> +		dev_dbg(&indio_dev->dev,
> +			"At least one scan element must be enabled first\n");
> +		return -EINVAL;
> +	}
> +
>  	memset(config, 0, sizeof(*config));
>  	config->watermark = ~0;
>  

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] iio: buffer: Don't allow buffers without any channels enabled to be activated
  2020-03-25 10:01 ` [PATCH v2] iio: buffer: Don't allow buffers without any channels enabled to be activated Alexandru Ardelean
  2020-03-25 10:12   ` Ardelean, Alexandru
@ 2020-03-25 15:57   ` Andy Shevchenko
  2020-03-26  7:42     ` Ardelean, Alexandru
  1 sibling, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2020-03-25 15:57 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-iio, Linux Kernel Mailing List, Jonathan Cameron,
	Lars-Peter Clausen, Hartmut Knaack, Peter Meerwald

On Wed, Mar 25, 2020 at 12:02 PM Alexandru Ardelean
<alexandru.ardelean@analog.com> wrote:
>
> From: Lars-Peter Clausen <lars@metafoo.de>
>
> Before activating a buffer make sure that at least one channel is enabled.
> Activating a buffer with 0 channels enabled doesn't make too much sense and
> disallowing this case makes sure that individual driver don't have to add
> special case code to handle it.
>
> Currently, without this patch enabling a buffer is possible and no error is
> produced. With this patch -EINVAL is returned.
>
> An example of execution with this patch and some instrumented print-code:
>    root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
>    root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
>    0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer->scan_mask 00000000
>    1: iio_verify_update 753
>    2:__iio_update_buffers 1115 ret -22
>    3: iio_buffer_store_enable 1241 ret -22
>    -bash: echo: write error: Invalid argument
> 1, 2 & 3 are exit-error paths. 0 the first print in iio_verify_update()
> rergardless of error path.
>
> Without this patch (and same instrumented print-code):
>    root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
>    root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
>    0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer->scan_mask 00000000
>    root@analog:/sys/bus/iio/devices/iio:device3/buffer#
> Buffer is enabled with no error.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
>
> Changelog v1 -> v2:
> * moved check in iio_verify_update()
> * added dev_dbg() message; should help some folks understand the message
> * documented steps to reproduce

> * added Fixes tag; hopefully the tag is the good one; if needed, it can be
>   dropped; this has been around for ~8 years; no idea if it's worth
>   backporting

Where?

>
>  drivers/iio/industrialio-buffer.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 5ff34ce8b6a2..e6fa1a4e135d 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -686,6 +686,13 @@ static int iio_verify_update(struct iio_dev *indio_dev,
>         bool scan_timestamp;
>         unsigned int modes;
>
> +       if (insert_buffer &&
> +           bitmap_empty(insert_buffer->scan_mask, indio_dev->masklength)) {
> +               dev_dbg(&indio_dev->dev,
> +                       "At least one scan element must be enabled first\n");
> +               return -EINVAL;
> +       }
> +
>         memset(config, 0, sizeof(*config));
>         config->watermark = ~0;
>
> --
> 2.17.1
>


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] iio: buffer: Don't allow buffers without any channels enabled to be activated
  2020-03-25 15:57   ` Andy Shevchenko
@ 2020-03-26  7:42     ` Ardelean, Alexandru
  2020-03-26  7:44       ` Lars-Peter Clausen
  0 siblings, 1 reply; 8+ messages in thread
From: Ardelean, Alexandru @ 2020-03-26  7:42 UTC (permalink / raw)
  To: andy.shevchenko; +Cc: pmeerw, jic23, linux-kernel, linux-iio, knaack.h, lars

On Wed, 2020-03-25 at 17:57 +0200, Andy Shevchenko wrote:
> [External]
> 
> On Wed, Mar 25, 2020 at 12:02 PM Alexandru Ardelean
> <alexandru.ardelean@analog.com> wrote:
> > From: Lars-Peter Clausen <lars@metafoo.de>
> > 
> > Before activating a buffer make sure that at least one channel is enabled.
> > Activating a buffer with 0 channels enabled doesn't make too much sense and
> > disallowing this case makes sure that individual driver don't have to add
> > special case code to handle it.
> > 
> > Currently, without this patch enabling a buffer is possible and no error is
> > produced. With this patch -EINVAL is returned.
> > 
> > An example of execution with this patch and some instrumented print-code:
> >    root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
> >    root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
> >    0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer-
> > >scan_mask 00000000
> >    1: iio_verify_update 753
> >    2:__iio_update_buffers 1115 ret -22
> >    3: iio_buffer_store_enable 1241 ret -22
> >    -bash: echo: write error: Invalid argument
> > 1, 2 & 3 are exit-error paths. 0 the first print in iio_verify_update()
> > rergardless of error path.
> > 
> > Without this patch (and same instrumented print-code):
> >    root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
> >    root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
> >    0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer-
> > >scan_mask 00000000
> >    root@analog:/sys/bus/iio/devices/iio:device3/buffer#
> > Buffer is enabled with no error.
> > 
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > 
> > Changelog v1 -> v2:
> > * moved check in iio_verify_update()
> > * added dev_dbg() message; should help some folks understand the message
> > * documented steps to reproduce
> > * added Fixes tag; hopefully the tag is the good one; if needed, it can be
> >   dropped; this has been around for ~8 years; no idea if it's worth
> >   backporting
> 
> Where?

stable/fixes/lts-kernels
don't have a really good clue about where these things need backporting;

> 
> >  drivers/iio/industrialio-buffer.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-
> > buffer.c
> > index 5ff34ce8b6a2..e6fa1a4e135d 100644
> > --- a/drivers/iio/industrialio-buffer.c
> > +++ b/drivers/iio/industrialio-buffer.c
> > @@ -686,6 +686,13 @@ static int iio_verify_update(struct iio_dev *indio_dev,
> >         bool scan_timestamp;
> >         unsigned int modes;
> > 
> > +       if (insert_buffer &&
> > +           bitmap_empty(insert_buffer->scan_mask, indio_dev->masklength)) {
> > +               dev_dbg(&indio_dev->dev,
> > +                       "At least one scan element must be enabled
> > first\n");
> > +               return -EINVAL;
> > +       }
> > +
> >         memset(config, 0, sizeof(*config));
> >         config->watermark = ~0;
> > 
> > --
> > 2.17.1
> > 
> 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] iio: buffer: Don't allow buffers without any channels enabled to be activated
  2020-03-26  7:42     ` Ardelean, Alexandru
@ 2020-03-26  7:44       ` Lars-Peter Clausen
  2020-03-26  7:46         ` Ardelean, Alexandru
  0 siblings, 1 reply; 8+ messages in thread
From: Lars-Peter Clausen @ 2020-03-26  7:44 UTC (permalink / raw)
  To: Ardelean, Alexandru, andy.shevchenko
  Cc: pmeerw, jic23, linux-kernel, linux-iio, knaack.h

On 3/26/20 8:42 AM, Ardelean, Alexandru wrote:
> On Wed, 2020-03-25 at 17:57 +0200, Andy Shevchenko wrote:
>> [External]
>>
>> On Wed, Mar 25, 2020 at 12:02 PM Alexandru Ardelean
>> <alexandru.ardelean@analog.com> wrote:
>>> From: Lars-Peter Clausen <lars@metafoo.de>
>>>
>>> Before activating a buffer make sure that at least one channel is enabled.
>>> Activating a buffer with 0 channels enabled doesn't make too much sense and
>>> disallowing this case makes sure that individual driver don't have to add
>>> special case code to handle it.
>>>
>>> Currently, without this patch enabling a buffer is possible and no error is
>>> produced. With this patch -EINVAL is returned.
>>>
>>> An example of execution with this patch and some instrumented print-code:
>>>     root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
>>>     root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
>>>     0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer-
>>>> scan_mask 00000000
>>>     1: iio_verify_update 753
>>>     2:__iio_update_buffers 1115 ret -22
>>>     3: iio_buffer_store_enable 1241 ret -22
>>>     -bash: echo: write error: Invalid argument
>>> 1, 2 & 3 are exit-error paths. 0 the first print in iio_verify_update()
>>> rergardless of error path.
>>>
>>> Without this patch (and same instrumented print-code):
>>>     root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
>>>     root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
>>>     0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer-
>>>> scan_mask 00000000
>>>     root@analog:/sys/bus/iio/devices/iio:device3/buffer#
>>> Buffer is enabled with no error.
>>>
>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>>> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>>> ---
>>>
>>> Changelog v1 -> v2:
>>> * moved check in iio_verify_update()
>>> * added dev_dbg() message; should help some folks understand the message
>>> * documented steps to reproduce
>>> * added Fixes tag; hopefully the tag is the good one; if needed, it can be
>>>    dropped; this has been around for ~8 years; no idea if it's worth
>>>    backporting
>> Where?
> stable/fixes/lts-kernels
> don't have a really good clue about where these things need backporting;
What Andy means is that there is no Fixes tag :)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] iio: buffer: Don't allow buffers without any channels enabled to be activated
  2020-03-26  7:44       ` Lars-Peter Clausen
@ 2020-03-26  7:46         ` Ardelean, Alexandru
  0 siblings, 0 replies; 8+ messages in thread
From: Ardelean, Alexandru @ 2020-03-26  7:46 UTC (permalink / raw)
  To: andy.shevchenko, lars; +Cc: pmeerw, jic23, linux-kernel, linux-iio, knaack.h

On Thu, 2020-03-26 at 08:44 +0100, Lars-Peter Clausen wrote:
> [External]
> 
> On 3/26/20 8:42 AM, Ardelean, Alexandru wrote:
> > On Wed, 2020-03-25 at 17:57 +0200, Andy Shevchenko wrote:
> > > [External]
> > > 
> > > On Wed, Mar 25, 2020 at 12:02 PM Alexandru Ardelean
> > > <alexandru.ardelean@analog.com> wrote:
> > > > From: Lars-Peter Clausen <lars@metafoo.de>
> > > > 
> > > > Before activating a buffer make sure that at least one channel is
> > > > enabled.
> > > > Activating a buffer with 0 channels enabled doesn't make too much sense
> > > > and
> > > > disallowing this case makes sure that individual driver don't have to
> > > > add
> > > > special case code to handle it.
> > > > 
> > > > Currently, without this patch enabling a buffer is possible and no error
> > > > is
> > > > produced. With this patch -EINVAL is returned.
> > > > 
> > > > An example of execution with this patch and some instrumented print-
> > > > code:
> > > >     root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
> > > >     root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
> > > >     0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer-
> > > > > scan_mask 00000000
> > > >     1: iio_verify_update 753
> > > >     2:__iio_update_buffers 1115 ret -22
> > > >     3: iio_buffer_store_enable 1241 ret -22
> > > >     -bash: echo: write error: Invalid argument
> > > > 1, 2 & 3 are exit-error paths. 0 the first print in iio_verify_update()
> > > > rergardless of error path.
> > > > 
> > > > Without this patch (and same instrumented print-code):
> > > >     root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
> > > >     root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
> > > >     0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer-
> > > > > scan_mask 00000000
> > > >     root@analog:/sys/bus/iio/devices/iio:device3/buffer#
> > > > Buffer is enabled with no error.
> > > > 
> > > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > > ---
> > > > 
> > > > Changelog v1 -> v2:
> > > > * moved check in iio_verify_update()
> > > > * added dev_dbg() message; should help some folks understand the message
> > > > * documented steps to reproduce
> > > > * added Fixes tag; hopefully the tag is the good one; if needed, it can
> > > > be
> > > >    dropped; this has been around for ~8 years; no idea if it's worth
> > > >    backporting
> > > Where?
> > stable/fixes/lts-kernels
> > don't have a really good clue about where these things need backporting;
> What Andy means is that there is no Fixes tag :)

oh crap....
i added one but seem to have lost it when moving the patch to the server from
where i do git-send
i'll add one;

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3] iio: buffer: Don't allow buffers without any channels enabled to be activated
       [not found] <20200320104031.31701-1-alexandru.ardelean@analog.com>
  2020-03-25 10:01 ` [PATCH v2] iio: buffer: Don't allow buffers without any channels enabled to be activated Alexandru Ardelean
@ 2020-03-26  9:30 ` Alexandru Ardelean
  2020-03-28 13:13   ` Jonathan Cameron
  1 sibling, 1 reply; 8+ messages in thread
From: Alexandru Ardelean @ 2020-03-26  9:30 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: jic23, lars, knaack.h, pmeerw, Alexandru Ardelean

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

Before activating a buffer make sure that at least one channel is enabled.
Activating a buffer with 0 channels enabled doesn't make too much sense and
disallowing this case makes sure that individual driver don't have to add
special case code to handle it.

Currently, without this patch enabling a buffer is possible and no error is
produced. With this patch -EINVAL is returned.

An example of execution with this patch and some instrumented print-code:
   root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
   root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
   0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer->scan_mask 00000000
   1: iio_verify_update 753
   2:__iio_update_buffers 1115 ret -22
   3: iio_buffer_store_enable 1241 ret -22
   -bash: echo: write error: Invalid argument
1, 2 & 3 are exit-error paths. 0 the first print in iio_verify_update()
rergardless of error path.

Without this patch (and same instrumented print-code):
   root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
   root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
   0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer->scan_mask 00000000
   root@analog:/sys/bus/iio/devices/iio:device3/buffer#
Buffer is enabled with no error.

Fixes: 84b36ce5f79c0 ("staging:iio: Add support for multiple buffers")
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changelog v2 -> v3:
* actually added Fixes tag; seems I forgot it in v2
  hopefully the tag is the good one; if needed, it can be
  dropped; this has been around for ~8 years; no idea if it's worth
  backporting

Changelog v1 -> v2:
* moved check in iio_verify_update()
* added dev_dbg() message; should help some folks understand the message
* documented steps to reproduce

 drivers/iio/industrialio-buffer.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 5ff34ce8b6a2..e6fa1a4e135d 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -686,6 +686,13 @@ static int iio_verify_update(struct iio_dev *indio_dev,
 	bool scan_timestamp;
 	unsigned int modes;
 
+	if (insert_buffer &&
+	    bitmap_empty(insert_buffer->scan_mask, indio_dev->masklength)) {
+		dev_dbg(&indio_dev->dev,
+			"At least one scan element must be enabled first\n");
+		return -EINVAL;
+	}
+
 	memset(config, 0, sizeof(*config));
 	config->watermark = ~0;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] iio: buffer: Don't allow buffers without any channels enabled to be activated
  2020-03-26  9:30 ` [PATCH v3] " Alexandru Ardelean
@ 2020-03-28 13:13   ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2020-03-28 13:13 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, lars, knaack.h, pmeerw

On Thu, 26 Mar 2020 11:30:12 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> From: Lars-Peter Clausen <lars@metafoo.de>
> 
> Before activating a buffer make sure that at least one channel is enabled.
> Activating a buffer with 0 channels enabled doesn't make too much sense and
> disallowing this case makes sure that individual driver don't have to add
> special case code to handle it.
> 
> Currently, without this patch enabling a buffer is possible and no error is
> produced. With this patch -EINVAL is returned.
> 
> An example of execution with this patch and some instrumented print-code:
>    root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
>    root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
>    0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer->scan_mask 00000000
>    1: iio_verify_update 753
>    2:__iio_update_buffers 1115 ret -22
>    3: iio_buffer_store_enable 1241 ret -22
>    -bash: echo: write error: Invalid argument
> 1, 2 & 3 are exit-error paths. 0 the first print in iio_verify_update()
> rergardless of error path.
> 
> Without this patch (and same instrumented print-code):
>    root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer
>    root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable
>    0: iio_verify_update 748 indio_dev->masklength 2 *insert_buffer->scan_mask 00000000
>    root@analog:/sys/bus/iio/devices/iio:device3/buffer#
> Buffer is enabled with no error.
> 
> Fixes: 84b36ce5f79c0 ("staging:iio: Add support for multiple buffers")
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
I've added a note saying that I don't think this is suitable for
'automatic' back porting to stable.   I'm not against it being requested
for particular kernels though if someone specifically wants it.

I'd kind of count this as an interface improvement rather than
it being broken as such before :)

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> ---
> 
> Changelog v2 -> v3:
> * actually added Fixes tag; seems I forgot it in v2
>   hopefully the tag is the good one; if needed, it can be
>   dropped; this has been around for ~8 years; no idea if it's worth
>   backporting
> 
> Changelog v1 -> v2:
> * moved check in iio_verify_update()
> * added dev_dbg() message; should help some folks understand the message
> * documented steps to reproduce
> 
>  drivers/iio/industrialio-buffer.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 5ff34ce8b6a2..e6fa1a4e135d 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -686,6 +686,13 @@ static int iio_verify_update(struct iio_dev *indio_dev,
>  	bool scan_timestamp;
>  	unsigned int modes;
>  
> +	if (insert_buffer &&
> +	    bitmap_empty(insert_buffer->scan_mask, indio_dev->masklength)) {
> +		dev_dbg(&indio_dev->dev,
> +			"At least one scan element must be enabled first\n");
> +		return -EINVAL;
> +	}
> +
>  	memset(config, 0, sizeof(*config));
>  	config->watermark = ~0;
>  


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-03-28 13:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200320104031.31701-1-alexandru.ardelean@analog.com>
2020-03-25 10:01 ` [PATCH v2] iio: buffer: Don't allow buffers without any channels enabled to be activated Alexandru Ardelean
2020-03-25 10:12   ` Ardelean, Alexandru
2020-03-25 15:57   ` Andy Shevchenko
2020-03-26  7:42     ` Ardelean, Alexandru
2020-03-26  7:44       ` Lars-Peter Clausen
2020-03-26  7:46         ` Ardelean, Alexandru
2020-03-26  9:30 ` [PATCH v3] " Alexandru Ardelean
2020-03-28 13:13   ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).