* mmotm 2010-10-13 - GSPCA SPCA561 webcam driver broken
[not found] <201010140044.o9E0iuR3029069@imap1.linux-foundation.org>
@ 2010-10-14 20:06 ` Valdis.Kletnieks
2010-10-15 8:45 ` Hans Verkuil
0 siblings, 1 reply; 8+ messages in thread
From: Valdis.Kletnieks @ 2010-10-14 20:06 UTC (permalink / raw)
To: Andrew Morton, Hans Verkuil, Mauro Carvalho Chehab
Cc: linux-kernel, linux-media
[-- Attachment #1: Type: text/plain, Size: 1774 bytes --]
On Wed, 13 Oct 2010 17:13:25 PDT, akpm@linux-foundation.org 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@xs4all.nl>
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@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
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.
[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: mmotm 2010-10-13 - GSPCA SPCA561 webcam driver broken
2010-10-14 20:06 ` mmotm 2010-10-13 - GSPCA SPCA561 webcam driver broken Valdis.Kletnieks
@ 2010-10-15 8:45 ` Hans Verkuil
2010-10-15 9:05 ` Andrew Morton
0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2010-10-15 8:45 UTC (permalink / raw)
To: Valdis.Kletnieks
Cc: Andrew Morton, Mauro Carvalho Chehab, linux-kernel, linux-media
On Thursday, October 14, 2010 22:06:29 Valdis.Kletnieks@vt.edu wrote:
> On Wed, 13 Oct 2010 17:13:25 PDT, akpm@linux-foundation.org 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@xs4all.nl>
> 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@xs4all.nl>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
>
>
> 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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: mmotm 2010-10-13 - GSPCA SPCA561 webcam driver broken
2010-10-15 8:45 ` Hans Verkuil
@ 2010-10-15 9:05 ` Andrew Morton
2010-10-15 10:02 ` Hans Verkuil
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Morton @ 2010-10-15 9:05 UTC (permalink / raw)
To: Hans Verkuil
Cc: Valdis.Kletnieks, Mauro Carvalho Chehab, linux-kernel,
linux-media
On Fri, 15 Oct 2010 10:45:45 +0200 Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On Thursday, October 14, 2010 22:06:29 Valdis.Kletnieks@vt.edu wrote:
> > On Wed, 13 Oct 2010 17:13:25 PDT, akpm@linux-foundation.org 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@xs4all.nl>
> > 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@xs4all.nl>
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> >
> >
> > 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.
The patch which Valdis quoted is what is in linux-next. I'm not
at which stage the mangling happened?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: mmotm 2010-10-13 - GSPCA SPCA561 webcam driver broken
2010-10-15 9:05 ` Andrew Morton
@ 2010-10-15 10:02 ` Hans Verkuil
2010-10-15 12:05 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2010-10-15 10:02 UTC (permalink / raw)
To: Andrew Morton
Cc: Valdis.Kletnieks, Mauro Carvalho Chehab, linux-kernel,
linux-media
On Friday, October 15, 2010 11:05:26 Andrew Morton wrote:
> On Fri, 15 Oct 2010 10:45:45 +0200 Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> > On Thursday, October 14, 2010 22:06:29 Valdis.Kletnieks@vt.edu wrote:
> > > On Wed, 13 Oct 2010 17:13:25 PDT, akpm@linux-foundation.org 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@xs4all.nl>
> > > 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@xs4all.nl>
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> > >
> > >
> > > 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.
>
> The patch which Valdis quoted is what is in linux-next. I'm not
> at which stage the mangling happened?
>
OK, I see what happened. My original patch:
http://git.linuxtv.org/media_tree.git?a=commitdiff;h=30de1fa062e42a74c54e94c8977d7dcef9a5049f;hp=f39385558f50f0b5b2bc9b47c187b81a8188fb10
clashed with Arnd Bergmann's patch:
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=86a5ef7d777cdd61dfe82379d559dbea069aea3d
Someone tried to resolve the conflict but made a mistake.
The result is this:
static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
struct video_device *vdev = video_devdata(filp);
int ret;
if (!vdev->fops->ioctl)
return -ENOTTY;
if (vdev->fops->unlocked_ioctl) {
if (vdev->lock)
mutex_lock(vdev->lock);
ret = vdev->fops->unlocked_ioctl(filp, cmd, arg);
if (vdev->lock)
mutex_unlock(vdev->lock);
} else if (vdev->fops->ioctl) {
/* TODO: convert all drivers to unlocked_ioctl */
lock_kernel();
ret = vdev->fops->ioctl(filp, cmd, arg);
unlock_kernel();
} else
ret = -ENOTTY;
return ret;
}
But this is wrong.
What it should be is this:
static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
struct video_device *vdev = video_devdata(filp);
int ret = -ENODEV;
if (vdev->fops->unlocked_ioctl) {
if (vdev->lock)
mutex_lock(vdev->lock);
if (video_is_registered(vdev))
ret = vdev->fops->unlocked_ioctl(filp, cmd, arg);
if (vdev->lock)
mutex_unlock(vdev->lock);
} else if (vdev->fops->ioctl) {
/* TODO: convert all drivers to unlocked_ioctl */
lock_kernel();
if (video_is_registered(vdev))
ret = vdev->fops->ioctl(filp, cmd, arg);
unlock_kernel();
} else
ret = -ENOTTY;
return ret;
}
Mauro, is this something for you to fix?
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: mmotm 2010-10-13 - GSPCA SPCA561 webcam driver broken
2010-10-15 10:02 ` Hans Verkuil
@ 2010-10-15 12:05 ` Mauro Carvalho Chehab
2010-10-15 12:23 ` Hans Verkuil
0 siblings, 1 reply; 8+ messages in thread
From: Mauro Carvalho Chehab @ 2010-10-15 12:05 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Andrew Morton, Valdis.Kletnieks, linux-kernel, linux-media
Em 15-10-2010 07:02, Hans Verkuil escreveu:
> On Friday, October 15, 2010 11:05:26 Andrew Morton wrote:
>> On Fri, 15 Oct 2010 10:45:45 +0200 Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>>> On Thursday, October 14, 2010 22:06:29 Valdis.Kletnieks@vt.edu wrote:
>>>> On Wed, 13 Oct 2010 17:13:25 PDT, akpm@linux-foundation.org said:
>>>>> The mm-of-the-moment snapshot 2010-10-13-17-13 has been uploaded to
>
> Mauro, is this something for you to fix?
I have a patch fixing this conflict already:
http://git.linuxtv.org/mchehab/sbtvd.git?a=commit;h=88164fbe701a0a16e9044b74443dddb6188b54cc
The patch is currently on a separate tree, that I'm using to test some experimental
drivers for Brazilian Digital TV system (SBTVD). I'm planning to merge this patch, among
with other patches I received for .37 during this weekend.
The problem were due to a conflict between a BKL patch and your patch.
Cheers,
Mauro.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: mmotm 2010-10-13 - GSPCA SPCA561 webcam driver broken
2010-10-15 12:05 ` Mauro Carvalho Chehab
@ 2010-10-15 12:23 ` Hans Verkuil
2010-10-18 19:00 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2010-10-15 12:23 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Andrew Morton, Valdis.Kletnieks, linux-kernel, linux-media
On Friday, October 15, 2010 14:05:39 Mauro Carvalho Chehab wrote:
> Em 15-10-2010 07:02, Hans Verkuil escreveu:
> > On Friday, October 15, 2010 11:05:26 Andrew Morton wrote:
> >> On Fri, 15 Oct 2010 10:45:45 +0200 Hans Verkuil <hverkuil@xs4all.nl> wrote:
> >>
> >>> On Thursday, October 14, 2010 22:06:29 Valdis.Kletnieks@vt.edu wrote:
> >>>> On Wed, 13 Oct 2010 17:13:25 PDT, akpm@linux-foundation.org said:
> >>>>> The mm-of-the-moment snapshot 2010-10-13-17-13 has been uploaded to
> >
> > Mauro, is this something for you to fix?
>
> I have a patch fixing this conflict already:
>
> http://git.linuxtv.org/mchehab/sbtvd.git?a=commit;h=88164fbe701a0a16e9044b74443dddb6188b54cc
>
> The patch is currently on a separate tree, that I'm using to test some experimental
> drivers for Brazilian Digital TV system (SBTVD). I'm planning to merge this patch, among
> with other patches I received for .37 during this weekend.
No, this patch isn't sufficient. It backs out the wrong code but doesn't put
in the 'video_is_registered()' if statements that were in my original patch.
Those are really needed.
Regards,
Hans
> The problem were due to a conflict between a BKL patch and your patch.
>
> Cheers,
> Mauro.
>
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: mmotm 2010-10-13 - GSPCA SPCA561 webcam driver broken
2010-10-15 12:23 ` Hans Verkuil
@ 2010-10-18 19:00 ` Mauro Carvalho Chehab
2010-10-18 19:39 ` Hans Verkuil
0 siblings, 1 reply; 8+ messages in thread
From: Mauro Carvalho Chehab @ 2010-10-18 19:00 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Andrew Morton, Valdis.Kletnieks, linux-kernel, linux-media
Em 15-10-2010 09:23, Hans Verkuil escreveu:
> On Friday, October 15, 2010 14:05:39 Mauro Carvalho Chehab wrote:
>> Em 15-10-2010 07:02, Hans Verkuil escreveu:
>>> On Friday, October 15, 2010 11:05:26 Andrew Morton wrote:
>>>> On Fri, 15 Oct 2010 10:45:45 +0200 Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>>>
>>>>> On Thursday, October 14, 2010 22:06:29 Valdis.Kletnieks@vt.edu wrote:
>>>>>> On Wed, 13 Oct 2010 17:13:25 PDT, akpm@linux-foundation.org said:
>>>>>>> The mm-of-the-moment snapshot 2010-10-13-17-13 has been uploaded to
>>>
>>> Mauro, is this something for you to fix?
>>
>> I have a patch fixing this conflict already:
>>
>> http://git.linuxtv.org/mchehab/sbtvd.git?a=commit;h=88164fbe701a0a16e9044b74443dddb6188b54cc
>>
>> The patch is currently on a separate tree, that I'm using to test some experimental
>> drivers for Brazilian Digital TV system (SBTVD). I'm planning to merge this patch, among
>> with other patches I received for .37 during this weekend.
>
> No, this patch isn't sufficient. It backs out the wrong code but doesn't put
> in the 'video_is_registered()' if statements that were in my original patch.
>
> Those are really needed.
Ok, I've re-done the conflict fix patch:
http://git.kernel.org/?p=linux/kernel/git/mchehab/linux-next.git;a=commitdiff;h=f9fccbad2a67668240edeaa6ada5aea2281d10b3
Cheers,
Mauro.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: mmotm 2010-10-13 - GSPCA SPCA561 webcam driver broken
2010-10-18 19:00 ` Mauro Carvalho Chehab
@ 2010-10-18 19:39 ` Hans Verkuil
0 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2010-10-18 19:39 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Andrew Morton, Valdis.Kletnieks, linux-kernel, linux-media
On Monday, October 18, 2010 21:00:57 Mauro Carvalho Chehab wrote:
> Em 15-10-2010 09:23, Hans Verkuil escreveu:
> > On Friday, October 15, 2010 14:05:39 Mauro Carvalho Chehab wrote:
> >> Em 15-10-2010 07:02, Hans Verkuil escreveu:
> >>> On Friday, October 15, 2010 11:05:26 Andrew Morton wrote:
> >>>> On Fri, 15 Oct 2010 10:45:45 +0200 Hans Verkuil <hverkuil@xs4all.nl> wrote:
> >>>>
> >>>>> On Thursday, October 14, 2010 22:06:29 Valdis.Kletnieks@vt.edu wrote:
> >>>>>> On Wed, 13 Oct 2010 17:13:25 PDT, akpm@linux-foundation.org said:
> >>>>>>> The mm-of-the-moment snapshot 2010-10-13-17-13 has been uploaded to
> >>>
> >>> Mauro, is this something for you to fix?
> >>
> >> I have a patch fixing this conflict already:
> >>
> >> http://git.linuxtv.org/mchehab/sbtvd.git?a=commit;h=88164fbe701a0a16e9044b74443dddb6188b54cc
> >>
> >> The patch is currently on a separate tree, that I'm using to test some experimental
> >> drivers for Brazilian Digital TV system (SBTVD). I'm planning to merge this patch, among
> >> with other patches I received for .37 during this weekend.
> >
> > No, this patch isn't sufficient. It backs out the wrong code but doesn't put
> > in the 'video_is_registered()' if statements that were in my original patch.
> >
> > Those are really needed.
>
> Ok, I've re-done the conflict fix patch:
>
> http://git.kernel.org/?p=linux/kernel/git/mchehab/linux-next.git;a=commitdiff;h=f9fccbad2a67668240edeaa6ada5aea2281d10b3
I hate to say this, but it is still not right. This is the correct code:
static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
struct video_device *vdev = video_devdata(filp);
int ret = -ENODEV;
if (vdev->fops->unlocked_ioctl) {
if (vdev->lock)
mutex_lock(vdev->lock);
if (video_is_registered(vdev))
ret = vdev->fops->unlocked_ioctl(filp, cmd, arg);
if (vdev->lock)
mutex_unlock(vdev->lock);
} else if (vdev->fops->ioctl) {
/* TODO: convert all drivers to unlocked_ioctl */
lock_kernel();
if (video_is_registered(vdev))
ret = vdev->fops->ioctl(filp, cmd, arg);
unlock_kernel();
} else
ret = -ENOTTY;
return ret;
}
As you can see, the video_is_registered is called with the lock held.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-10-18 19:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <201010140044.o9E0iuR3029069@imap1.linux-foundation.org>
2010-10-14 20:06 ` mmotm 2010-10-13 - GSPCA SPCA561 webcam driver broken Valdis.Kletnieks
2010-10-15 8:45 ` Hans Verkuil
2010-10-15 9:05 ` Andrew Morton
2010-10-15 10:02 ` Hans Verkuil
2010-10-15 12:05 ` Mauro Carvalho Chehab
2010-10-15 12:23 ` Hans Verkuil
2010-10-18 19:00 ` Mauro Carvalho Chehab
2010-10-18 19:39 ` Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox