* [PATCH] staging:iio: allow event usage when no buffer support present.
@ 2011-10-21 11:34 Jonathan Cameron
2011-10-21 11:34 ` [PATCH] staging:iio: core. Allow for event chrdev obtaining ioctl if no buffer present Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2011-10-21 11:34 UTC (permalink / raw)
To: linux-iio; +Cc: lars, Jonathan Cameron
Lars-Peter pointed out this issue.
Basically the checks for whether we had a buffer were in the open
wheras they should be in the read.
I've tested this on the tsl2563 driver and it all seems fine.
Lars-Peter - could you hammer this a little to see if I have
missed any corner cases?
Thanks,
Jonathan
Jonathan Cameron (1):
staging:iio: core. Allow for event chrdev obtaining ioctl if no
buffer present.
drivers/staging/iio/iio_core.h | 2 +-
drivers/staging/iio/industrialio-buffer.c | 6 +++++-
2 files changed, 6 insertions(+), 2 deletions(-)
--
1.7.7
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] staging:iio: core. Allow for event chrdev obtaining ioctl if no buffer present.
2011-10-21 11:34 [PATCH] staging:iio: allow event usage when no buffer support present Jonathan Cameron
@ 2011-10-21 11:34 ` Jonathan Cameron
2011-10-21 11:46 ` Lars-Peter Clausen
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2011-10-21 11:34 UTC (permalink / raw)
To: linux-iio; +Cc: lars, Jonathan Cameron
Logic bug meant the chrdev would fail to open if there was no buffer support
in a driver or in the core. This meant the ioctl to get the event chrdev
would fail and hence events were not available.
Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
drivers/staging/iio/iio_core.h | 2 +-
drivers/staging/iio/industrialio-buffer.c | 6 +++++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/iio/iio_core.h b/drivers/staging/iio/iio_core.h
index 36159e0..ff27f13 100644
--- a/drivers/staging/iio/iio_core.h
+++ b/drivers/staging/iio/iio_core.h
@@ -49,7 +49,7 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
static inline int iio_chrdev_buffer_open(struct iio_dev *indio_dev)
{
- return -EINVAL;
+ return 0;
}
static inline void iio_chrdev_buffer_release(struct iio_dev *indio_dev)
diff --git a/drivers/staging/iio/industrialio-buffer.c b/drivers/staging/iio/industrialio-buffer.c
index bac672d..1a71171 100644
--- a/drivers/staging/iio/industrialio-buffer.c
+++ b/drivers/staging/iio/industrialio-buffer.c
@@ -42,6 +42,8 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
struct iio_dev *indio_dev = filp->private_data;
struct iio_buffer *rb = indio_dev->buffer;
+ if (!rb)
+ return -ENODEV;
if (!rb->access->read_first_n)
return -EINVAL;
return rb->access->read_first_n(rb, n, buf);
@@ -67,7 +69,7 @@ int iio_chrdev_buffer_open(struct iio_dev *indio_dev)
{
struct iio_buffer *rb = indio_dev->buffer;
if (!rb)
- return -EINVAL;
+ return 0;
if (rb->access->mark_in_use)
rb->access->mark_in_use(rb);
return 0;
@@ -77,6 +79,8 @@ void iio_chrdev_buffer_release(struct iio_dev *indio_dev)
{
struct iio_buffer *rb = indio_dev->buffer;
+ if (!rb)
+ return;
clear_bit(IIO_BUSY_BIT_POS, &rb->flags);
if (rb->access->unmark_in_use)
rb->access->unmark_in_use(rb);
--
1.7.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] staging:iio: core. Allow for event chrdev obtaining ioctl if no buffer present.
2011-10-21 11:34 ` [PATCH] staging:iio: core. Allow for event chrdev obtaining ioctl if no buffer present Jonathan Cameron
@ 2011-10-21 11:46 ` Lars-Peter Clausen
2011-10-21 11:47 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2011-10-21 11:46 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio
On 10/21/2011 01:34 PM, Jonathan Cameron wrote:
> Logic bug meant the chrdev would fail to open if there was no buffer support
> in a driver or in the core. This meant the ioctl to get the event chrdev
> would fail and hence events were not available.
>
> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
> ---
> I've tested this on the tsl2563 driver and it all seems fine.
> Lars-Peter - could you hammer this a little to see if I have
> missed any corner cases?
This is basically what I had locally for testing my tool, so it should work.
One minor comment though.
> drivers/staging/iio/iio_core.h | 2 +-
> drivers/staging/iio/industrialio-buffer.c | 6 +++++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/iio/iio_core.h b/drivers/staging/iio/iio_core.h
> index 36159e0..ff27f13 100644
> --- a/drivers/staging/iio/iio_core.h
> +++ b/drivers/staging/iio/iio_core.h
> @@ -49,7 +49,7 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
>
> static inline int iio_chrdev_buffer_open(struct iio_dev *indio_dev)
> {
> - return -EINVAL;
> + return 0;
> }
>
> static inline void iio_chrdev_buffer_release(struct iio_dev *indio_dev)
> diff --git a/drivers/staging/iio/industrialio-buffer.c b/drivers/staging/iio/industrialio-buffer.c
> index bac672d..1a71171 100644
> --- a/drivers/staging/iio/industrialio-buffer.c
> +++ b/drivers/staging/iio/industrialio-buffer.c
> @@ -42,6 +42,8 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
> struct iio_dev *indio_dev = filp->private_data;
> struct iio_buffer *rb = indio_dev->buffer;
>
> + if (!rb)
> + return -ENODEV;
I think -EINVAL would be a better return code here.
Quote from the read(3) manpage:
"EINVAL: fd is attached to an object which is unsuitable for reading;"
> if (!rb->access->read_first_n)
> return -EINVAL;
> return rb->access->read_first_n(rb, n, buf);
> @@ -67,7 +69,7 @@ int iio_chrdev_buffer_open(struct iio_dev *indio_dev)
> {
> struct iio_buffer *rb = indio_dev->buffer;
> if (!rb)
> - return -EINVAL;
> + return 0;
> if (rb->access->mark_in_use)
> rb->access->mark_in_use(rb);
> return 0;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging:iio: core. Allow for event chrdev obtaining ioctl if no buffer present.
2011-10-21 11:46 ` Lars-Peter Clausen
@ 2011-10-21 11:47 ` Jonathan Cameron
2011-10-21 11:52 ` [PATCH V2] staging:iio: allow event usage when no buffer support present Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2011-10-21 11:47 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: linux-iio
On 10/21/11 12:46, Lars-Peter Clausen wrote:
> On 10/21/2011 01:34 PM, Jonathan Cameron wrote:
>> Logic bug meant the chrdev would fail to open if there was no buffer support
>> in a driver or in the core. This meant the ioctl to get the event chrdev
>> would fail and hence events were not available.
>>
>> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
>> ---
>
>> I've tested this on the tsl2563 driver and it all seems fine.
>> Lars-Peter - could you hammer this a little to see if I have
>> missed any corner cases?
>
> This is basically what I had locally for testing my tool, so it should work.
> One minor comment though.
>
>
>> drivers/staging/iio/iio_core.h | 2 +-
>> drivers/staging/iio/industrialio-buffer.c | 6 +++++-
>> 2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/iio/iio_core.h b/drivers/staging/iio/iio_core.h
>> index 36159e0..ff27f13 100644
>> --- a/drivers/staging/iio/iio_core.h
>> +++ b/drivers/staging/iio/iio_core.h
>> @@ -49,7 +49,7 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
>>
>> static inline int iio_chrdev_buffer_open(struct iio_dev *indio_dev)
>> {
>> - return -EINVAL;
>> + return 0;
>> }
>>
>> static inline void iio_chrdev_buffer_release(struct iio_dev *indio_dev)
>> diff --git a/drivers/staging/iio/industrialio-buffer.c b/drivers/staging/iio/industrialio-buffer.c
>> index bac672d..1a71171 100644
>> --- a/drivers/staging/iio/industrialio-buffer.c
>> +++ b/drivers/staging/iio/industrialio-buffer.c
>> @@ -42,6 +42,8 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
>> struct iio_dev *indio_dev = filp->private_data;
>> struct iio_buffer *rb = indio_dev->buffer;
>>
>> + if (!rb)
>> + return -ENODEV;
>
> I think -EINVAL would be a better return code here.
> Quote from the read(3) manpage:
> "EINVAL: fd is attached to an object which is unsuitable for reading;"
Hmm. arguably you are reading form a buffer than doesn't exist, but I guess
that bit of semantics might make no sense to users. Will change that for v2.
Jonathan
>
>> if (!rb->access->read_first_n)
>> return -EINVAL;
>> return rb->access->read_first_n(rb, n, buf);
>> @@ -67,7 +69,7 @@ int iio_chrdev_buffer_open(struct iio_dev *indio_dev)
>> {
>> struct iio_buffer *rb = indio_dev->buffer;
>> if (!rb)
>> - return -EINVAL;
>> + return 0;
>> if (rb->access->mark_in_use)
>> rb->access->mark_in_use(rb);
>> return 0;
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V2] staging:iio: allow event usage when no buffer support present.
2011-10-21 11:47 ` Jonathan Cameron
@ 2011-10-21 11:52 ` Jonathan Cameron
2011-10-21 11:53 ` [PATCH] staging:iio: core. Allow for event chrdev obtaining ioctl if no buffer present Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2011-10-21 11:52 UTC (permalink / raw)
To: linux-iio; +Cc: lars, Jonathan Cameron
v2: Changed error to -EINVAL as per Lars-Peter's suggestion.
v1:
Lars-Peter pointed out this issue.
Basically the checks for whether we had a buffer were in the open
wheras they should be in the read.
I've tested this on the tsl2563 driver and it all seems fine.
Lars-Peter - could you hammer this a little to see if I have
missed any corner cases?
Thanks,
Jonathan
Jonathan Cameron (1):
staging:iio: core. Allow for event chrdev obtaining ioctl if no
buffer present.
drivers/staging/iio/iio_core.h | 2 +-
drivers/staging/iio/industrialio-buffer.c | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
--
1.7.7
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] staging:iio: core. Allow for event chrdev obtaining ioctl if no buffer present.
2011-10-21 11:52 ` [PATCH V2] staging:iio: allow event usage when no buffer support present Jonathan Cameron
@ 2011-10-21 11:53 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2011-10-21 11:53 UTC (permalink / raw)
To: linux-iio; +Cc: lars, Jonathan Cameron
Logic bug meant the chrdev would fail to open if there was no buffer support
in a driver or in the core. This meant the ioctl to get the event chrdev
would fail and hence events were not available.
V2: change error to -EINVAL to mark as unsuitable for reading rather than
not there. Both are true depending on how you look at it.
Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
drivers/staging/iio/iio_core.h | 2 +-
drivers/staging/iio/industrialio-buffer.c | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/iio_core.h b/drivers/staging/iio/iio_core.h
index 36159e0..ff27f13 100644
--- a/drivers/staging/iio/iio_core.h
+++ b/drivers/staging/iio/iio_core.h
@@ -49,7 +49,7 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
static inline int iio_chrdev_buffer_open(struct iio_dev *indio_dev)
{
- return -EINVAL;
+ return 0;
}
static inline void iio_chrdev_buffer_release(struct iio_dev *indio_dev)
diff --git a/drivers/staging/iio/industrialio-buffer.c b/drivers/staging/iio/industrialio-buffer.c
index 515abef..03c114a 100644
--- a/drivers/staging/iio/industrialio-buffer.c
+++ b/drivers/staging/iio/industrialio-buffer.c
@@ -42,7 +42,7 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
struct iio_dev *indio_dev = filp->private_data;
struct iio_buffer *rb = indio_dev->buffer;
- if (!rb->access->read_first_n)
+ if (!rb || !rb->access->read_first_n)
return -EINVAL;
return rb->access->read_first_n(rb, n, buf);
}
@@ -67,7 +67,7 @@ int iio_chrdev_buffer_open(struct iio_dev *indio_dev)
{
struct iio_buffer *rb = indio_dev->buffer;
if (!rb)
- return -EINVAL;
+ return 0;
if (rb->access->mark_in_use)
rb->access->mark_in_use(rb);
return 0;
@@ -77,6 +77,8 @@ void iio_chrdev_buffer_release(struct iio_dev *indio_dev)
{
struct iio_buffer *rb = indio_dev->buffer;
+ if (!rb)
+ return;
clear_bit(IIO_BUSY_BIT_POS, &rb->flags);
if (rb->access->unmark_in_use)
rb->access->unmark_in_use(rb);
--
1.7.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-10-21 11:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-21 11:34 [PATCH] staging:iio: allow event usage when no buffer support present Jonathan Cameron
2011-10-21 11:34 ` [PATCH] staging:iio: core. Allow for event chrdev obtaining ioctl if no buffer present Jonathan Cameron
2011-10-21 11:46 ` Lars-Peter Clausen
2011-10-21 11:47 ` Jonathan Cameron
2011-10-21 11:52 ` [PATCH V2] staging:iio: allow event usage when no buffer support present Jonathan Cameron
2011-10-21 11:53 ` [PATCH] staging:iio: core. Allow for event chrdev obtaining ioctl if no buffer present Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox