* Fwd: [PATCH] ngene: blocking and nonblocking io for sec0
@ 2011-06-01 21:01 Mauro Carvalho Chehab
2011-06-18 11:07 ` Oliver Endriss
0 siblings, 1 reply; 3+ messages in thread
From: Mauro Carvalho Chehab @ 2011-06-01 21:01 UTC (permalink / raw)
To: Oliver Endriss, Ralph Metzler; +Cc: Linux Media Mailing List
Oliver/Ralph,
Could you please review this patch? On a quick look, it looks
fine on my eyes, but I don't have any ngene hardware here for testing.
Thanks!
Mauro
-------- Mensagem original --------
Date: Thu, 12 May 2011 15:47:09 +0200
From: Issa Gorissen <flop.m@usa.net>
To: linux-media@vger.kernel.org
Subject: [PATCH] ngene: blocking and nonblocking io for sec0
Patch allows for blocking or nonblocking io on the ngene sec0 device.
It also enforces one reader and one writer at a time.
Signed-off-by: Issa Gorissen <flop.m@usa.net>
--
--- a/linux/drivers/media/dvb/ngene/ngene-dvb.c 2011-05-10 19:11:21.000000000 +0200
+++ b/linux/drivers/media/dvb/ngene/ngene-dvb.c 2011-05-12 15:28:53.573185365 +0200
@@ -53,15 +53,29 @@ static ssize_t ts_write(struct file *fil
struct dvb_device *dvbdev = file->private_data;
struct ngene_channel *chan = dvbdev->priv;
struct ngene *dev = chan->dev;
+ int avail = 0;
+ char nonblock = file->f_flags & O_NONBLOCK;
- if (wait_event_interruptible(dev->tsout_rbuf.queue,
- dvb_ringbuffer_free
- (&dev->tsout_rbuf) >= count) < 0)
+ if (!count)
return 0;
- dvb_ringbuffer_write(&dev->tsout_rbuf, buf, count);
+ if (nonblock) {
+ avail = dvb_ringbuffer_avail(&dev->tsout_rbuf);
+ if (!avail)
+ return -EAGAIN;
+ } else {
+ while (1) {
+ if (wait_event_interruptible(dev->tsout_rbuf.queue,
+ dvb_ringbuffer_free
+ (&dev->tsout_rbuf) >= count) >= 0)
+ break;
+ }
+ avail = count;
+ }
+
+ dvb_ringbuffer_write(&dev->tsout_rbuf, buf, avail);
+ return avail;
- return count;
}
static ssize_t ts_read(struct file *file, char *buf,
@@ -70,22 +84,35 @@ static ssize_t ts_read(struct file *file
struct dvb_device *dvbdev = file->private_data;
struct ngene_channel *chan = dvbdev->priv;
struct ngene *dev = chan->dev;
- int left, avail;
+ int avail = 0;
+ char nonblock = file->f_flags & O_NONBLOCK;
- left = count;
- while (left) {
- if (wait_event_interruptible(
- dev->tsin_rbuf.queue,
- dvb_ringbuffer_avail(&dev->tsin_rbuf) > 0) < 0)
- return -EAGAIN;
+ if (!count)
+ return 0;
+
+ if (nonblock) {
avail = dvb_ringbuffer_avail(&dev->tsin_rbuf);
- if (avail > left)
- avail = left;
- dvb_ringbuffer_read_user(&dev->tsin_rbuf, buf, avail);
- left -= avail;
- buf += avail;
+ } else {
+ while (!avail) {
+ if (wait_event_interruptible(
+ dev->tsin_rbuf.queue,
+ dvb_ringbuffer_avail(&dev->tsin_rbuf) > 0) < 0)
+ continue;
+
+ avail = dvb_ringbuffer_avail(&dev->tsin_rbuf);
+ }
}
- return count;
+
+ if (avail > count)
+ avail = count;
+ if (avail > 0)
+ dvb_ringbuffer_read_user(&dev->tsin_rbuf, buf, avail);
+
+ if (!avail)
+ return -EAGAIN;
+ else
+ return avail;
+
}
static const struct file_operations ci_fops = {
@@ -98,9 +125,9 @@ static const struct file_operations ci_f
struct dvb_device ngene_dvbdev_ci = {
.priv = 0,
- .readers = -1,
- .writers = -1,
- .users = -1,
+ .readers = 1,
+ .writers = 1,
+ .users = 2,
.fops = &ci_fops,
};
--
To unsubscribe from this list: send the line "unsubscribe linux-media" 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] 3+ messages in thread
* Re: Fwd: [PATCH] ngene: blocking and nonblocking io for sec0
2011-06-01 21:01 Fwd: [PATCH] ngene: blocking and nonblocking io for sec0 Mauro Carvalho Chehab
@ 2011-06-18 11:07 ` Oliver Endriss
2011-06-21 21:58 ` Issa Gorissen
0 siblings, 1 reply; 3+ messages in thread
From: Oliver Endriss @ 2011-06-18 11:07 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Ralph Metzler, Linux Media Mailing List
Hi,
sorry for the delay, I don't have much time atm.
On Wednesday 01 June 2011 23:01:52 Mauro Carvalho Chehab wrote:
> Oliver/Ralph,
>
> Could you please review this patch? On a quick look, it looks
> fine on my eyes, but I don't have any ngene hardware here for testing.
>
> Thanks!
> Mauro
The patch is not correct. See my comments below.
> -------- Mensagem original --------
> Date: Thu, 12 May 2011 15:47:09 +0200
> From: Issa Gorissen <flop.m@usa.net>
> To: linux-media@vger.kernel.org
> Subject: [PATCH] ngene: blocking and nonblocking io for sec0
>
> Patch allows for blocking or nonblocking io on the ngene sec0 device.
> It also enforces one reader and one writer at a time.
>
> Signed-off-by: Issa Gorissen <flop.m@usa.net>
> --
>
> --- a/linux/drivers/media/dvb/ngene/ngene-dvb.c 2011-05-10 19:11:21.000000000 +0200
> +++ b/linux/drivers/media/dvb/ngene/ngene-dvb.c 2011-05-12 15:28:53.573185365 +0200
> @@ -53,15 +53,29 @@ static ssize_t ts_write(struct file *fil
> struct dvb_device *dvbdev = file->private_data;
> struct ngene_channel *chan = dvbdev->priv;
> struct ngene *dev = chan->dev;
> + int avail = 0;
> + char nonblock = file->f_flags & O_NONBLOCK;
>
> - if (wait_event_interruptible(dev->tsout_rbuf.queue,
> - dvb_ringbuffer_free
> - (&dev->tsout_rbuf) >= count) < 0)
> + if (!count)
> return 0;
>
> - dvb_ringbuffer_write(&dev->tsout_rbuf, buf, count);
> + if (nonblock) {
> + avail = dvb_ringbuffer_avail(&dev->tsout_rbuf);
> + if (!avail)
> + return -EAGAIN;
Wrong. dvb_ringbuffer_avail() returns the number of bytes waiting in the
ring buffer. The code must use dvb_ringbuffer_free() instead.
Furthermore the code completely ignores count (the number of bytes to be
written).
> + } else {
> + while (1) {
> + if (wait_event_interruptible(dev->tsout_rbuf.queue,
> + dvb_ringbuffer_free
> + (&dev->tsout_rbuf) >= count) >= 0)
> + break;
> + }
What is this loop supposed to do?
> + avail = count;
> + }
> +
> + dvb_ringbuffer_write(&dev->tsout_rbuf, buf, avail);
> + return avail;
>
> - return count;
> }
>
> static ssize_t ts_read(struct file *file, char *buf,
> @@ -70,22 +84,35 @@ static ssize_t ts_read(struct file *file
> struct dvb_device *dvbdev = file->private_data;
> struct ngene_channel *chan = dvbdev->priv;
> struct ngene *dev = chan->dev;
> - int left, avail;
> + int avail = 0;
> + char nonblock = file->f_flags & O_NONBLOCK;
>
> - left = count;
> - while (left) {
> - if (wait_event_interruptible(
> - dev->tsin_rbuf.queue,
> - dvb_ringbuffer_avail(&dev->tsin_rbuf) > 0) < 0)
> - return -EAGAIN;
> + if (!count)
> + return 0;
> +
> + if (nonblock) {
> avail = dvb_ringbuffer_avail(&dev->tsin_rbuf);
> - if (avail > left)
> - avail = left;
> - dvb_ringbuffer_read_user(&dev->tsin_rbuf, buf, avail);
> - left -= avail;
> - buf += avail;
> + } else {
> + while (!avail) {
> + if (wait_event_interruptible(
> + dev->tsin_rbuf.queue,
> + dvb_ringbuffer_avail(&dev->tsin_rbuf) > 0) < 0)
> + continue;
> +
> + avail = dvb_ringbuffer_avail(&dev->tsin_rbuf);
> + }
This piece of code is also wrong. The loop does not wait, until there is
enough data available. It does not consider 'count'.
Furthermore 'count' might be larger than the size of the ring buffer.
The original code was correct, except that it did not handle O_NONBLOCK.
> }
> - return count;
> +
> + if (avail > count)
> + avail = count;
> + if (avail > 0)
> + dvb_ringbuffer_read_user(&dev->tsin_rbuf, buf, avail);
> +
> + if (!avail)
> + return -EAGAIN;
> + else
> + return avail;
> +
> }
>
> static const struct file_operations ci_fops = {
> @@ -98,9 +125,9 @@ static const struct file_operations ci_f
>
> struct dvb_device ngene_dvbdev_ci = {
> .priv = 0,
> - .readers = -1,
> - .writers = -1,
> - .users = -1,
> + .readers = 1,
> + .writers = 1,
> + .users = 2,
Ok.
> .fops = &ci_fops,
> };
CU
Oliver
--
----------------------------------------------------------------
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/
Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/
----------------------------------------------------------------
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Fwd: [PATCH] ngene: blocking and nonblocking io for sec0
2011-06-18 11:07 ` Oliver Endriss
@ 2011-06-21 21:58 ` Issa Gorissen
0 siblings, 0 replies; 3+ messages in thread
From: Issa Gorissen @ 2011-06-21 21:58 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Oliver Endriss, Mauro Carvalho Chehab, Ralph Metzler
Haven't fully understood the usage of wake_event_interruptible, now I have read its description. Sorry for the lame patch.
Please find a fixed version below.
Patch allows for blocking or nonblocking io on the ngene sec0 device.
It also enforces one reader and one writer at a time.
Signed-off-by: Issa Gorissen <flop.m@usa.net>
--
--- a/linux/drivers/media/dvb/ngene/ngene-dvb.c 2011-05-10 19:11:21.000000000 +0200
+++ b/linux/drivers/media/dvb/ngene/ngene-dvb.c 2011-06-21 23:46:09.000000000 +0200
@@ -53,15 +53,30 @@ static ssize_t ts_write(struct file *fil
struct dvb_device *dvbdev = file->private_data;
struct ngene_channel *chan = dvbdev->priv;
struct ngene *dev = chan->dev;
+ int avail = 0;
+ char nonblock = file->f_flags & O_NONBLOCK;
- if (wait_event_interruptible(dev->tsout_rbuf.queue,
- dvb_ringbuffer_free
- (&dev->tsout_rbuf) >= count) < 0)
+ if (!count)
return 0;
- dvb_ringbuffer_write(&dev->tsout_rbuf, buf, count);
+ if (nonblock) {
+ avail = dvb_ringbuffer_free(&dev->tsout_rbuf);
+ if (!avail)
+ return -EAGAIN;
+ if (count < avail)
+ avail = count;
+ } else {
+ if (wait_event_interruptible(dev->tsout_rbuf.queue,
+ dvb_ringbuffer_free
+ (&dev->tsout_rbuf) >= count) < 0)
+ return -EINTR;
+
+ avail = count;
+ }
+
+ dvb_ringbuffer_write(&dev->tsout_rbuf, buf, avail);
+ return avail;
- return count;
}
static ssize_t ts_read(struct file *file, char *buf,
@@ -70,22 +85,29 @@ static ssize_t ts_read(struct file *file
struct dvb_device *dvbdev = file->private_data;
struct ngene_channel *chan = dvbdev->priv;
struct ngene *dev = chan->dev;
- int left, avail;
+ int avail = 0;
+ char nonblock = file->f_flags & O_NONBLOCK;
- left = count;
- while (left) {
- if (wait_event_interruptible(
- dev->tsin_rbuf.queue,
- dvb_ringbuffer_avail(&dev->tsin_rbuf) > 0) < 0)
- return -EAGAIN;
+ if (!count)
+ return 0;
+
+ if (nonblock) {
avail = dvb_ringbuffer_avail(&dev->tsin_rbuf);
- if (avail > left)
- avail = left;
- dvb_ringbuffer_read_user(&dev->tsin_rbuf, buf, avail);
- left -= avail;
- buf += avail;
+ if (!avail)
+ return -EAGAIN;
+ if (avail > count)
+ avail = count;
+ } else {
+ if (wait_event_interruptible(dev->tsin_rbuf.queue,
+ dvb_ringbuffer_avail
+ (&dev->tsin_rbuf) >= count) < 0)
+ return -EINTR;
+
+ avail = count;
}
- return count;
+
+ dvb_ringbuffer_read_user(&dev->tsin_rbuf, buf, avail);
+ return avail;
}
static const struct file_operations ci_fops = {
@@ -98,9 +120,9 @@ static const struct file_operations ci_f
struct dvb_device ngene_dvbdev_ci = {
.priv = 0,
- .readers = -1,
- .writers = -1,
- .users = -1,
+ .readers = 1,
+ .writers = 1,
+ .users = 2,
.fops = &ci_fops,
};
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-06-21 21:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-01 21:01 Fwd: [PATCH] ngene: blocking and nonblocking io for sec0 Mauro Carvalho Chehab
2011-06-18 11:07 ` Oliver Endriss
2011-06-21 21:58 ` Issa Gorissen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox