On Friday, April 08, 2011 17:19:40 Laurent Pinchart wrote:
> Hi Hans,
>
> Thanks for the patch.
> On Monday 04 April 2011 13:51:51 Hans Verkuil wrote:
>
> [snip]
>
> > diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
> > index 21d8f6a..8790e03 100644
> > --- a/drivers/media/video/vivi.c
> > +++ b/drivers/media/video/vivi.c
> > @@ -998,6 +1007,25 @@ static int vivi_s_ctrl(struct v4l2_ctrl *ctrl)
> > File operations for the device
> > ------------------------------------------------------------------*/
> >
> > +static int vivi_open(struct file *filp)
> > +{
> > + int ret = v4l2_fh_open(filp);
> > + struct v4l2_fh *fh;
> > +
> > + if (ret)
> > + return ret;
> > + fh = filp->private_data;
> > + ret = v4l2_event_init(fh);
> > + if (ret)
> > + goto rel_fh;
> > + ret = v4l2_event_alloc(fh, 10);
> > + if (!ret)
> > + return ret;
> > +rel_fh:
> > + v4l2_fh_release(filp);
> > + return ret;
> > +}
> > +
>
> Should the V4L2 core provide a helper function that does just that ?
Good idea.
>
> > static ssize_t
> > vivi_read(struct file *file, char __user *data, size_t count, loff_t
> > *ppos) {
> > @@ -1012,10 +1040,17 @@ static unsigned int
> > vivi_poll(struct file *file, struct poll_table_struct *wait)
> > {
> > struct vivi_dev *dev = video_drvdata(file);
> > + struct v4l2_fh *fh = file->private_data;
> > struct vb2_queue *q = &dev->vb_vidq;
> > + unsigned int res;
> >
> > dprintk(dev, 1, "%s\n", __func__);
> > - return vb2_poll(q, file, wait);
> > + res = vb2_poll(q, file, wait);
> > + if (v4l2_event_pending(fh))
> > + res |= POLLPRI;
> > + else
> > + poll_wait(file, &fh->events->wait, wait);
>
> Don't you need to call poll_wait unconditionally ?
No, setting POLLPRI will have poll() exit without waiting.
On the other hand, it may be more understandable if I do it unconditionally.
I've been back and forth about this a few times already :-)
I suspect that auditing the way drivers implement poll() would be a great
janitorial project. It's not the greatest API in the world...
Regards,
Hans
>
> > + return res;
> > }
> >
> > static int vivi_close(struct file *file)
>
> [snip]
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
