Re: mmotm 2010-10-13 - GSPCA SPCA561 webcam driver broken



On Thursday, October 14, 2010 22:06:29 Valdis.Kletnieks@xxxxxx wrote:
On Wed, 13 Oct 2010 17:13:25 PDT, akpm@xxxxxxxxxxxxxxxxxxxx said:
The mm-of-the-moment snapshot 2010-10-13-17-13 has been uploaded to

http://userweb.kernel.org/~akpm/mmotm/

This broke my webcam. I bisected it down to this commit, and things
work again after reverting the 2 code lines of change.

commit 9e4d79a98ebd857ec729f5fa8f432f35def4d0da
Author: Hans Verkuil <hverkuil@xxxxxxxxx>
Date: Sun Sep 26 08:16:56 2010 -0300

V4L/DVB: v4l2-dev: after a disconnect any ioctl call will be blocked

Until now all fops except release and (unlocked_)ioctl returned an error
after the device node was unregistered. Extend this as well to the ioctl
fops. There is nothing useful that an application can do here and it
complicates the driver code unnecessarily.

Signed-off-by: Hans Verkuil <hverkuil@xxxxxxxxx>
Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>


diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index d4a3532..f069c61 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -221,8 +221,8 @@ static long v4l2_ioctl(struct file *filp, unsigned int cmd,
struct video_device *vdev = video_devdata(filp);
int ret;

- /* Allow ioctl to continue even if the device was unregistered.
- Things like dequeueing buffers might still be useful. */
+ if (!vdev->fops->ioctl)
+ return -ENOTTY;
if (vdev->fops->unlocked_ioctl) {
ret = vdev->fops->unlocked_ioctl(filp, cmd, arg);
} else if (vdev->fops->ioctl) {

I suspect this doesn't do what's intended if a driver is using ->unlocked_ioctl
rather than ->ioctl, and it should be reverted - it only saves at most one
if statement.



I'm not sure what is going on here. It looks like this patch is mangled in your
tree since the same patch in the v4l-dvb repository looks like this:

diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 32575a6..26d39c4 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -222,8 +222,8 @@ static int v4l2_ioctl(struct inode *inode, struct file *filp,

if (!vdev->fops->ioctl)
return -ENOTTY;
- /* Allow ioctl to continue even if the device was unregistered.
- Things like dequeueing buffers might still be useful. */
+ if (!video_is_registered(vdev))
+ return -ENODEV;
return vdev->fops->ioctl(filp, cmd, arg);
}

@@ -234,8 +234,8 @@ static long v4l2_unlocked_ioctl(struct file *filp,

if (!vdev->fops->unlocked_ioctl)
return -ENOTTY;
- /* Allow ioctl to continue even if the device was unregistered.
- Things like dequeueing buffers might still be useful. */
+ if (!video_is_registered(vdev))
+ return -ENODEV;
return vdev->fops->unlocked_ioctl(filp, cmd, arg);
}

In your diff there is a mismatch between ioctl and unlocked_ioctl which no doubt
is causing all the problems for you.

Regards,

Hans

--
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



Relevant Pages

  • Re: .version keeps being updated
    ... 8993780a6e44fb4e7ed34e33458506a775356c6e is first bad commit ... Reverting this from 2.6.20-rc1 made the build behave again, ... this patch started all the mess: ... Fix linux banner utsname information ...
    (Linux-Kernel)
  • Re: .version keeps being updated
    ... 8993780a6e44fb4e7ed34e33458506a775356c6e is first bad commit ... Reverting this from 2.6.20-rc1 made the build behave again, ... this patch started all the mess: ... Fix linux banner utsname information ...
    (Linux-Kernel)
  • Re: Performance regression in 2.6.22-git1 (new sched code?)
    ... does reverting this patch improve performance: ... commit 9c4801cebc2add1fe514bc8eb201b16372eee11a ... sched: more agressive idle balancing ...
    (Linux-Kernel)
  • Re: [BUG 2.6.36-rc5] of_i2c.ko <-> i2c-core.ko dependency loop
    ... Yes, reverting the above commit from 2.6.36-rc5 eliminated the warnings, ... My preferred solution is the first option because the tested code path ... The second option is a smaller patch, but I can only test one of the ...
    (Linux-Kernel)
  • Re: mmotm 2010-10-13 - GSPCA SPCA561 webcam driver broken
    ... I bisected it down to this commit, ... work again after reverting the 2 code lines of change. ... /* Allow ioctl to continue even if the device was unregistered. ... It looks like this patch is mangled in your ...
    (Linux-Kernel)