* [RFC][PATCH] iio: buffer: Don't allow buffers without any channels enabled to be activated @ 2020-03-20 10:40 Alexandru Ardelean 2020-03-20 10:55 ` Lars-Peter Clausen ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Alexandru Ardelean @ 2020-03-20 10:40 UTC (permalink / raw) To: linux-iio; +Cc: lars, jic23, 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. Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> --- * Found this relic-patch in our tree, from 6 years ago: https://github.com/analogdevicesinc/linux/commit/6d680e49d459c It got moved around a bit, and this is the current form in the ADI tree. So, this is also a bit of an RFC, but if the idea is valid, maybe it's worth considering upstream. I don't know of any arguments against it, but I could be surprised. drivers/iio/industrialio-buffer.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c index 4ada5592aa2b..f222a118d0d3 100644 --- a/drivers/iio/industrialio-buffer.c +++ b/drivers/iio/industrialio-buffer.c @@ -1031,6 +1031,12 @@ static int __iio_update_buffers(struct iio_dev *indio_dev, return ret; if (insert_buffer) { + if (bitmap_empty(insert_buffer->scan_mask, + indio_dev->masklength)) { + ret = -EINVAL; + goto err_free_config; + } + ret = iio_buffer_request_update(indio_dev, insert_buffer); if (ret) goto err_free_config; -- 2.20.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] iio: buffer: Don't allow buffers without any channels enabled to be activated 2020-03-20 10:40 [RFC][PATCH] iio: buffer: Don't allow buffers without any channels enabled to be activated Alexandru Ardelean @ 2020-03-20 10:55 ` Lars-Peter Clausen 2020-03-20 11:16 ` Ardelean, Alexandru 2020-03-25 10:01 ` [PATCH v2] " Alexandru Ardelean 2020-03-26 9:30 ` [PATCH v3] " Alexandru Ardelean 2 siblings, 1 reply; 15+ messages in thread From: Lars-Peter Clausen @ 2020-03-20 10:55 UTC (permalink / raw) To: Alexandru Ardelean, linux-iio; +Cc: jic23, Alexandru Ardelean On 3/20/20 11:40 AM, 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. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > --- > > * Found this relic-patch in our tree, from 6 years ago: > https://github.com/analogdevicesinc/linux/commit/6d680e49d459c > It got moved around a bit, and this is the current form in the ADI tree. > So, this is also a bit of an RFC, but if the idea is valid, maybe it's > worth considering upstream. I don't know of any arguments against it, > but I could be surprised. Hm, a bit weird that this one never made it upstream considering how simple it is. Did you check that the issue still occurs? I can't see anything in the code that prevents it, but who knows, maybe it was fixed by something else. > > drivers/iio/industrialio-buffer.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c > index 4ada5592aa2b..f222a118d0d3 100644 > --- a/drivers/iio/industrialio-buffer.c > +++ b/drivers/iio/industrialio-buffer.c > @@ -1031,6 +1031,12 @@ static int __iio_update_buffers(struct iio_dev *indio_dev, > return ret; > > if (insert_buffer) { > + if (bitmap_empty(insert_buffer->scan_mask, > + indio_dev->masklength)) { > + ret = -EINVAL; > + goto err_free_config; > + } Since the check is so simple it might make sense to do it as the very first thing before iio_verify_update(). > + > ret = iio_buffer_request_update(indio_dev, insert_buffer); > if (ret) > goto err_free_config; > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] iio: buffer: Don't allow buffers without any channels enabled to be activated 2020-03-20 10:55 ` Lars-Peter Clausen @ 2020-03-20 11:16 ` Ardelean, Alexandru 2020-03-21 18:21 ` Jonathan Cameron 0 siblings, 1 reply; 15+ messages in thread From: Ardelean, Alexandru @ 2020-03-20 11:16 UTC (permalink / raw) To: ardeleanalex, lars, linux-iio; +Cc: jic23 On Fri, 2020-03-20 at 11:55 +0100, Lars-Peter Clausen wrote: > On 3/20/20 11:40 AM, 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. > > > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > --- > > > > * Found this relic-patch in our tree, from 6 years ago: > > https://github.com/analogdevicesinc/linux/commit/6d680e49d459c > > It got moved around a bit, and this is the current form in the ADI tree. > > So, this is also a bit of an RFC, but if the idea is valid, maybe it's > > worth considering upstream. I don't know of any arguments against it, > > but I could be surprised. > > Hm, a bit weird that this one never made it upstream considering how > simple it is. > > Did you check that the issue still occurs? I can't see anything in the > code that prevents it, but who knows, maybe it was fixed by something else. i did not think to check behavior/issues; i'll try to make some time for that; i caught this one while diff-ing the upstream & ADI trees, and i needed to dig a bit more into the ADI git history on it; i was a bit puzzled for a while, because some rework patches were upstreamed without this patch: https://lore.kernel.org/linux-iio/55585CAA.6000506@kernel.org/ https://lore.kernel.org/linux-iio/5560685A.5060504@kernel.org/ i also did not find any discussions/upstream attempt for this patch particularly so, it was easier for me just to RFC this > > > drivers/iio/industrialio-buffer.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio- > > buffer.c > > index 4ada5592aa2b..f222a118d0d3 100644 > > --- a/drivers/iio/industrialio-buffer.c > > +++ b/drivers/iio/industrialio-buffer.c > > @@ -1031,6 +1031,12 @@ static int __iio_update_buffers(struct iio_dev > > *indio_dev, > > return ret; > > > > if (insert_buffer) { > > + if (bitmap_empty(insert_buffer->scan_mask, > > + indio_dev->masklength)) { > > + ret = -EINVAL; > > + goto err_free_config; > > + } > > Since the check is so simple it might make sense to do it as the very > first thing before iio_verify_update(). works for me; > > > + > > ret = iio_buffer_request_update(indio_dev, insert_buffer); > > if (ret) > > goto err_free_config; > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] iio: buffer: Don't allow buffers without any channels enabled to be activated 2020-03-20 11:16 ` Ardelean, Alexandru @ 2020-03-21 18:21 ` Jonathan Cameron 2020-03-21 21:44 ` Andy Shevchenko 2020-03-25 8:17 ` Ardelean, Alexandru 0 siblings, 2 replies; 15+ messages in thread From: Jonathan Cameron @ 2020-03-21 18:21 UTC (permalink / raw) To: Ardelean, Alexandru; +Cc: ardeleanalex, lars, linux-iio On Fri, 20 Mar 2020 11:16:12 +0000 "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > On Fri, 2020-03-20 at 11:55 +0100, Lars-Peter Clausen wrote: > > On 3/20/20 11:40 AM, 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. > > > > > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > > --- > > > > > > * Found this relic-patch in our tree, from 6 years ago: > > > https://github.com/analogdevicesinc/linux/commit/6d680e49d459c > > > It got moved around a bit, and this is the current form in the ADI tree. > > > So, this is also a bit of an RFC, but if the idea is valid, maybe it's > > > worth considering upstream. I don't know of any arguments against it, > > > but I could be surprised. > > > > Hm, a bit weird that this one never made it upstream considering how > > simple it is. > > > > Did you check that the issue still occurs? I can't see anything in the > > code that prevents it, but who knows, maybe it was fixed by something else. > > i did not think to check behavior/issues; > i'll try to make some time for that; I can't immediately think of anything that would stop this case. However, good if you could confirm it. (I don't have a setup running right now to test against) > i caught this one while diff-ing the upstream & ADI trees, and i needed to dig a > bit more into the ADI git history on it; > > i was a bit puzzled for a while, because some rework patches were upstreamed > without this patch: > > https://lore.kernel.org/linux-iio/55585CAA.6000506@kernel.org/ > https://lore.kernel.org/linux-iio/5560685A.5060504@kernel.org/ > > i also did not find any discussions/upstream attempt for this patch particularly > > so, it was easier for me just to RFC this > > > > > > drivers/iio/industrialio-buffer.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio- > > > buffer.c > > > index 4ada5592aa2b..f222a118d0d3 100644 > > > --- a/drivers/iio/industrialio-buffer.c > > > +++ b/drivers/iio/industrialio-buffer.c > > > @@ -1031,6 +1031,12 @@ static int __iio_update_buffers(struct iio_dev > > > *indio_dev, > > > return ret; > > > > > > if (insert_buffer) { > > > + if (bitmap_empty(insert_buffer->scan_mask, > > > + indio_dev->masklength)) { > > > + ret = -EINVAL; > > > + goto err_free_config; > > > + } > > > > Since the check is so simple it might make sense to do it as the very > > first thing before iio_verify_update(). > > works for me; > > > > > > + > > > ret = iio_buffer_request_update(indio_dev, insert_buffer); > > > if (ret) > > > goto err_free_config; > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] iio: buffer: Don't allow buffers without any channels enabled to be activated 2020-03-21 18:21 ` Jonathan Cameron @ 2020-03-21 21:44 ` Andy Shevchenko 2020-03-25 8:17 ` Ardelean, Alexandru 1 sibling, 0 replies; 15+ messages in thread From: Andy Shevchenko @ 2020-03-21 21:44 UTC (permalink / raw) To: Jonathan Cameron; +Cc: Ardelean, Alexandru, ardeleanalex, lars, linux-iio On Sat, Mar 21, 2020 at 8:22 PM Jonathan Cameron <jic23@kernel.org> wrote: > On Fri, 20 Mar 2020 11:16:12 +0000 > "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > > On Fri, 2020-03-20 at 11:55 +0100, Lars-Peter Clausen wrote: > > > On 3/20/20 11:40 AM, Alexandru Ardelean wrote: > > > > + if (bitmap_empty(insert_buffer->scan_mask, > > > > + indio_dev->masklength)) { I guess it's much better to put on one line (despite possible over 80 limit). -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] iio: buffer: Don't allow buffers without any channels enabled to be activated 2020-03-21 18:21 ` Jonathan Cameron 2020-03-21 21:44 ` Andy Shevchenko @ 2020-03-25 8:17 ` Ardelean, Alexandru 2020-03-25 8:23 ` Ardelean, Alexandru 1 sibling, 1 reply; 15+ messages in thread From: Ardelean, Alexandru @ 2020-03-25 8:17 UTC (permalink / raw) To: jic23; +Cc: lars, linux-iio On Sat, 2020-03-21 at 18:21 +0000, Jonathan Cameron wrote: > On Fri, 20 Mar 2020 11:16:12 +0000 > "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > > > On Fri, 2020-03-20 at 11:55 +0100, Lars-Peter Clausen wrote: > > > On 3/20/20 11:40 AM, 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. > > > > > > > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > > > --- > > > > > > > > * Found this relic-patch in our tree, from 6 years ago: > > > > https://github.com/analogdevicesinc/linux/commit/6d680e49d459c > > > > It got moved around a bit, and this is the current form in the ADI > > > > tree. > > > > So, this is also a bit of an RFC, but if the idea is valid, maybe > > > > it's > > > > worth considering upstream. I don't know of any arguments against it, > > > > but I could be surprised. > > > > > > Hm, a bit weird that this one never made it upstream considering how > > > simple it is. > > > > > > Did you check that the issue still occurs? I can't see anything in the > > > code that prevents it, but who knows, maybe it was fixed by something > > > else. > > > > i did not think to check behavior/issues; > > i'll try to make some time for that; > > I can't immediately think of anything that would stop this case. > > However, good if you could confirm it. (I don't have a setup running > right now to test against) I've instrumented the code a bit. So, with this change [moved in iio_verify_update() as Lars suggested]: root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable 000000000 iio_verify_update 748 indio_dev->masklength 2 *insert_buffer->scan_mask 00000000 11111111 iio_verify_update 753 222222 __iio_update_buffers 1115 ret -22 333333333 iio_buffer_store_enable 1241 ret -22 -bash: echo: write error: Invalid argument so, 11111111, 222222 & 333333333 are all error paths. Without the patch: root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable 000000000 iio_verify_update 748 indio_dev->masklength 2 *insert_buffer->scan_mask 00000000 root@analog:/sys/bus/iio/devices/iio:device3/buffer# no error path is hit; error paths are still there, but the bitmap_empty() check removed; So, buffer gets enabled, but scan_mask is empty. Will follow-up a V2 on this and attach this information. > > > > i caught this one while diff-ing the upstream & ADI trees, and i needed to > > dig a > > bit more into the ADI git history on it; > > > > i was a bit puzzled for a while, because some rework patches were upstreamed > > without this patch: > > > > https://lore.kernel.org/linux-iio/55585CAA.6000506@kernel.org/ > > https://lore.kernel.org/linux-iio/5560685A.5060504@kernel.org/ > > > > i also did not find any discussions/upstream attempt for this patch > > particularly > > > > so, it was easier for me just to RFC this > > > > > > > > > drivers/iio/industrialio-buffer.c | 6 ++++++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git a/drivers/iio/industrialio-buffer.c > > > > b/drivers/iio/industrialio- > > > > buffer.c > > > > index 4ada5592aa2b..f222a118d0d3 100644 > > > > --- a/drivers/iio/industrialio-buffer.c > > > > +++ b/drivers/iio/industrialio-buffer.c > > > > @@ -1031,6 +1031,12 @@ static int __iio_update_buffers(struct iio_dev > > > > *indio_dev, > > > > return ret; > > > > > > > > if (insert_buffer) { > > > > + if (bitmap_empty(insert_buffer->scan_mask, > > > > + indio_dev->masklength)) { > > > > + ret = -EINVAL; > > > > + goto err_free_config; > > > > + } > > > > > > Since the check is so simple it might make sense to do it as the very > > > first thing before iio_verify_update(). > > > > works for me; > > > > > > > > > + > > > > ret = iio_buffer_request_update(indio_dev, > > > > insert_buffer); > > > > if (ret) > > > > goto err_free_config; > > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] iio: buffer: Don't allow buffers without any channels enabled to be activated 2020-03-25 8:17 ` Ardelean, Alexandru @ 2020-03-25 8:23 ` Ardelean, Alexandru 0 siblings, 0 replies; 15+ messages in thread From: Ardelean, Alexandru @ 2020-03-25 8:23 UTC (permalink / raw) To: jic23; +Cc: lars, linux-iio On Wed, 2020-03-25 at 08:17 +0000, Ardelean, Alexandru wrote: > On Sat, 2020-03-21 at 18:21 +0000, Jonathan Cameron wrote: > > On Fri, 20 Mar 2020 11:16:12 +0000 > > "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > > > > > On Fri, 2020-03-20 at 11:55 +0100, Lars-Peter Clausen wrote: > > > > On 3/20/20 11:40 AM, 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. > > > > > > > > > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > > > > --- > > > > > > > > > > * Found this relic-patch in our tree, from 6 years ago: > > > > > https://github.com/analogdevicesinc/linux/commit/6d680e49d459c > > > > > It got moved around a bit, and this is the current form in the ADI > > > > > tree. > > > > > So, this is also a bit of an RFC, but if the idea is valid, maybe > > > > > it's > > > > > worth considering upstream. I don't know of any arguments against > > > > > it, > > > > > but I could be surprised. > > > > > > > > Hm, a bit weird that this one never made it upstream considering how > > > > simple it is. > > > > > > > > Did you check that the issue still occurs? I can't see anything in the > > > > code that prevents it, but who knows, maybe it was fixed by something > > > > else. > > > > > > i did not think to check behavior/issues; > > > i'll try to make some time for that; > > > > I can't immediately think of anything that would stop this case. > > > > However, good if you could confirm it. (I don't have a setup running > > right now to test against) > > I've instrumented the code a bit. > So, with this change [moved in iio_verify_update() as Lars suggested]: > > root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer > root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable > 000000000 iio_verify_update 748 > indio_dev->masklength 2 > *insert_buffer->scan_mask 00000000 > 11111111 iio_verify_update 753 > 222222 __iio_update_buffers 1115 ret -22 > 333333333 iio_buffer_store_enable 1241 ret -22 > -bash: echo: write error: Invalid argument > > so, 11111111, 222222 & 333333333 are all error paths. > > Without the patch: > > root@analog:~# cd /sys/bus/iio/devices/iio\:device3/buffer > root@analog:/sys/bus/iio/devices/iio:device3/buffer# echo 1 > enable > 000000000 iio_verify_update 748 > indio_dev->masklength 2 > *insert_buffer->scan_mask 00000000 > root@analog:/sys/bus/iio/devices/iio:device3/buffer# > > no error path is hit; > error paths are still there, but the bitmap_empty() check removed; > > So, buffer gets enabled, but scan_mask is empty. > > Will follow-up a V2 on this and attach this information. Right. As a quick follow-up here: if the scan_mask is not empty [i.e. at least one scan_element is enabled] all works fine. > > > > > > > i caught this one while diff-ing the upstream & ADI trees, and i needed to > > > dig a > > > bit more into the ADI git history on it; > > > > > > i was a bit puzzled for a while, because some rework patches were > > > upstreamed > > > without this patch: > > > > > > https://lore.kernel.org/linux-iio/55585CAA.6000506@kernel.org/ > > > https://lore.kernel.org/linux-iio/5560685A.5060504@kernel.org/ > > > > > > i also did not find any discussions/upstream attempt for this patch > > > particularly > > > > > > so, it was easier for me just to RFC this > > > > > > > > > > > > drivers/iio/industrialio-buffer.c | 6 ++++++ > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > diff --git a/drivers/iio/industrialio-buffer.c > > > > > b/drivers/iio/industrialio- > > > > > buffer.c > > > > > index 4ada5592aa2b..f222a118d0d3 100644 > > > > > --- a/drivers/iio/industrialio-buffer.c > > > > > +++ b/drivers/iio/industrialio-buffer.c > > > > > @@ -1031,6 +1031,12 @@ static int __iio_update_buffers(struct iio_dev > > > > > *indio_dev, > > > > > return ret; > > > > > > > > > > if (insert_buffer) { > > > > > + if (bitmap_empty(insert_buffer->scan_mask, > > > > > + indio_dev->masklength)) { > > > > > + ret = -EINVAL; > > > > > + goto err_free_config; > > > > > + } > > > > > > > > Since the check is so simple it might make sense to do it as the very > > > > first thing before iio_verify_update(). > > > > > > works for me; > > > > > > > > > > > > + > > > > > ret = iio_buffer_request_update(indio_dev, > > > > > insert_buffer); > > > > > if (ret) > > > > > goto err_free_config; > > > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] iio: buffer: Don't allow buffers without any channels enabled to be activated 2020-03-20 10:40 [RFC][PATCH] iio: buffer: Don't allow buffers without any channels enabled to be activated Alexandru Ardelean 2020-03-20 10:55 ` Lars-Peter Clausen @ 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 2 siblings, 2 replies; 15+ 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] 15+ 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] " Alexandru Ardelean @ 2020-03-25 10:12 ` Ardelean, Alexandru 2020-03-25 15:57 ` Andy Shevchenko 1 sibling, 0 replies; 15+ 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] 15+ 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] " 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; 15+ 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] 15+ 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; 15+ 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] 15+ 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; 15+ 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] 15+ 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; 15+ 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] 15+ messages in thread
* [PATCH v3] iio: buffer: Don't allow buffers without any channels enabled to be activated 2020-03-20 10:40 [RFC][PATCH] iio: buffer: Don't allow buffers without any channels enabled to be activated Alexandru Ardelean 2020-03-20 10:55 ` Lars-Peter Clausen 2020-03-25 10:01 ` [PATCH v2] " Alexandru Ardelean @ 2020-03-26 9:30 ` Alexandru Ardelean 2020-03-28 13:13 ` Jonathan Cameron 2 siblings, 1 reply; 15+ 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] 15+ 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; 15+ 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] 15+ messages in thread
end of thread, other threads:[~2020-03-28 13:13 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-20 10:40 [RFC][PATCH] iio: buffer: Don't allow buffers without any channels enabled to be activated Alexandru Ardelean 2020-03-20 10:55 ` Lars-Peter Clausen 2020-03-20 11:16 ` Ardelean, Alexandru 2020-03-21 18:21 ` Jonathan Cameron 2020-03-21 21:44 ` Andy Shevchenko 2020-03-25 8:17 ` Ardelean, Alexandru 2020-03-25 8:23 ` Ardelean, Alexandru 2020-03-25 10:01 ` [PATCH v2] " 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).