Re: [PATCH] USB Video Class driver

[Brandon Philips <brandon@xxxxxxxx>]

On 14:27 Wed 23 Apr 2008, Mauro Carvalho Chehab wrote:
> > + * ...  It implements the
> > + * mmap capture method only ...
> 
> You should consider moving to videobuf on a later version. videobuf also
> implements read() method, and will likely implement also USERPTR and maybe
> OVERLAY on future versions.

Lets shoot for doing this after 2.6.26 if Laurent signs-off.  Until then
lets not get into this argument again :D

> > +static int uvc_v4l2_do_ioctl(struct inode *inode, struct file *file,
> > +		     unsigned int cmd, void *arg)
> > +{
> > +	struct video_device *vdev = video_devdata(file);
> > +	struct uvc_video_device *video = video_get_drvdata(vdev);
> > +	struct uvc_fh *handle = (struct uvc_fh *)file->private_data;
> > +	int ret = 0;
> > +
> > +	if (uvc_trace_param & UVC_TRACE_IOCTL)
> > +		v4l_printk_ioctl(cmd);
> 
> The better is to remove the do_ioctl, in favor of video_ioctl2. Also, this will
> provide a much better debug than what's provided by v4l_printk_ioctl().

We discussed this months ago and everyone agreed that video_ioctl2 is
nice but it is not a requirement to be in the tree.

> Driver looks sane. Just a few comments.

Thanks for finding the other issues in your review Mauro; you picked up
on some good details that should be fixed up before the merge.

Cheers,

	Brandon

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@xxxxxxxxxx?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list