* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.