Re: Bug fix for pvrusb2 driver [was: Re: Kernel 2.6.26.5 -> 2.6.27.2 [USB REGRESSION] (USB -> D_STATE)]

["Michael Krufky" <mkrufky@linuxtv.org>]

[-- Attachment #1: Type: text/plain, Size: 1545 bytes --]

On Fri, Oct 24, 2008 at 1:43 AM, Mike Isely <isely@isely.net> wrote:
> On Thu, 23 Oct 2008, Justin Piszcz wrote:
>
>>
>>
>> On Thu, 23 Oct 2008, Michael Krufky wrote:
>>
>> > On Thu, Oct 23, 2008 at 3:54 PM, Mike Isely <isely@isely.net> wrote:
>> > > On Thu, 23 Oct 2008, Greg KH wrote:
>> > >
>> > > > On Thu, Oct 23, 2008 at 11:27:11AM -0500, Mike Isely wrote:
>> > > > > On Mon, 20 Oct 2008, Greg KH wrote:
>> > > > >
>> > >
>> > >   [...]
>> > >
>> > > > > >
>> > > > > > Will this patch be sent to the -stable group, to fix this regression
>> > > > > > in
>> > > > > > 2.6.27?  Or should they take Alan's fix instead?
>> > > > > >
>> > > > >
>> > > > > Greg:
>>
>> [ .. ]
>>
>> > The changeset in Linus tree is: c82732a42896364296599b0f73f01c5e3fd781ae
>> >
>> > Reviewed-by: Michael Krufky <mkrufky@linuxtv.org>
>> >
>> > Please wait on test results from Mike Isely or I confirming that
>> > digital mode still works properly before queuing this one.
>> >
>> > Thanks,
>> >
>> > Mike Krufky
>> >
>>
>> I only tested analog here for the 1950 FYI.
>>
>> Justin.
>
> Analog and digital HVR-1950 operation are unaffected by the reset fix
> here.  This is safe.
>
>  -Mike
>
> --
>
> Mike Isely
> isely @ pobox (dot) com
> PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
>

I concur.  This patch is fine for -stable.  (I tested digital myself
last night / this morning and its fine)

Greg -- any chance you can slip this in to 2.6.27.4 ?  The driver is
entirely broken otherwise.

cherry-picked changeset attached...

Thanks,

Mike Krufky

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-V4L-pvrusb2-Fix-deadlock-problem.patch --]
[-- Type: text/x-diff; name=0001-V4L-pvrusb2-Fix-deadlock-problem.patch, Size: 2856 bytes --]

From ac36d2de0e2187f322ce085618c3ddc2954a0c2e Mon Sep 17 00:00:00 2001
From: Mike Isely <isely@pobox.com>
Date: Sun, 19 Oct 2008 16:26:05 -0300
Subject: [PATCH] V4L: pvrusb2: Fix deadlock problem

Fix deadlock problem in 2.6.27 caused by new USB core behavior in
response to a USB device reset request.  With older kernels, the USB
device reset was "in line"; the reset simply took place and the driver
retained its association with the hardware.  However now this reset
triggers a disconnect, and worse still the disconnect callback happens
in the context of the caller who asked for the device reset.  This
results in an attempt by the pvrusb2 driver to recursively take a
mutex it already has, which deadlocks the driver's worker thread.
(Even if the disconnect callback were to happen on a different thread
we'd still have problems however - because while the driver should
survive and correctly disconnect / reconnect, it will then trigger
another device reset during the repeated initialization, which will
then cause another disconect, etc, forever.)  The fix here is simply
to not attempt the device reset (it was of marginal value anyway).

Signed-off-by: Mike Isely <isely@pobox.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Signed-off-by: Michael Krufky <mkrufky@linuxtv.org>
(cherry picked from commit c82732a42896364296599b0f73f01c5e3fd781ae)
---
 drivers/media/video/pvrusb2/pvrusb2-hdw.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.c b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
index f051c6a..7412258 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-hdw.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
@@ -60,7 +60,6 @@ static struct pvr2_hdw *unit_pointers[PVR_NUM] = {[ 0 ... PVR_NUM-1 ] = NULL};
 static DEFINE_MUTEX(pvr2_unit_mtx);
 
 static int ctlchg;
-static int initusbreset = 1;
 static int procreload;
 static int tuner[PVR_NUM] = { [0 ... PVR_NUM-1] = -1 };
 static int tolerance[PVR_NUM] = { [0 ... PVR_NUM-1] = 0 };
@@ -71,8 +70,6 @@ module_param(ctlchg, int, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(ctlchg, "0=optimize ctl change 1=always accept new ctl value");
 module_param(init_pause_msec, int, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(init_pause_msec, "hardware initialization settling delay");
-module_param(initusbreset, int, S_IRUGO|S_IWUSR);
-MODULE_PARM_DESC(initusbreset, "Do USB reset device on probe");
 module_param(procreload, int, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(procreload,
 		 "Attempt init failure recovery with firmware reload");
@@ -1698,9 +1695,6 @@ static void pvr2_hdw_setup_low(struct pvr2_hdw *hdw)
 	}
 	hdw->fw1_state = FW1_STATE_OK;
 
-	if (initusbreset) {
-		pvr2_hdw_device_reset(hdw);
-	}
 	if (!pvr2_hdw_dev_ok(hdw)) return;
 
 	for (idx = 0; idx < hdw->hdw_desc->client_modules.cnt; idx++) {
-- 
1.5.4.3