* [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