Kernel 2.6.26.5 -> 2.6.27.2 [USB REGRESSION] (USB -> D_STATE)

Kernel 2.6.26.5 -> 2.6.27.2 [USB REGRESSION] (USB -> D_STATE)

[Justin Piszcz]

With kernel 2.6.27.2, any processes attempting to use USB hang and go into 
D-state, I have never had a problem like this before until 2.6.27.2 (I 
have not tried 2.6.27.1 or 2.6.27)

# lsusb -v
<hangs>

open("/dev/bus/usb", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 3
fstat64(3, {st_mode=S_IFDIR|0755, st_size=140, ...}) = 0
getdents(3, /* 7 entries */, 4096)      = 112
getdents(3, /* 0 entries */, 4096)      = 0
close(3)                                = 0
open("/dev/bus/usb/001", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 3
fstat64(3, {st_mode=S_IFDIR|0755, st_size=80, ...}) = 0
getdents(3, /* 4 entries */, 4096)      = 64
open("/dev/bus/usb/001/004", O_RDWR)    = 4
ioctl(4, USBDEVFS_CONNECTINFO

nut (for UPS) does not start.

nut       4078  0.0  0.0   1964   856 pts/8    D    07:09   0:00 /lib/nut/usbhid-ups -a belkin
root      4170  0.0  0.0   2204  1072 pts/8    D+   07:11   0:00 lsusb -v

Processes go directly to D-state with 2.6.27.2 for USB.

Booting back to 2.6.26.5 now to see if everything works again.

Back to 2.6.25.5, everything works fine:

# lsusb 
Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 001 Device 004: ID 2040:7501 Hauppauge 
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 003 Device 002: ID 050d:1100 Belkin Components 
Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 002 Device 002: ID 045e:0039 Microsoft Corp. IntelliMouse Optical
Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub

Relevant configuration files and dmesg output:

http://home.comcast.net/~jpiszcz/2.6.25.5-usb-working.txt
http://home.comcast.net/~jpiszcz/2.6.27.2-usb-broken.txt

http://home.comcast.net/~jpiszcz/linux-2.6.25.5-config
http://home.comcast.net/~jpiszcz/linux-2.6.27.2-config

Re: Kernel 2.6.26.5 -> 2.6.27.2 [USB REGRESSION] (USB -> D_STATE)

[Justin Piszcz]

On Sun, 19 Oct 2008, Justin Piszcz wrote:

> With kernel 2.6.27.2, any processes attempting to use USB hang and go into 
> D-state, I have never had a problem like this before until 2.6.27.2 (I have 
> not tried 2.6.27.1 or 2.6.27)
>
> # lsusb -v
> <hangs>
>
> open("/dev/bus/usb", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 
> 3
> fstat64(3, {st_mode=S_IFDIR|0755, st_size=140, ...}) = 0
> getdents(3, /* 7 entries */, 4096)      = 112
> getdents(3, /* 0 entries */, 4096)      = 0
> close(3)                                = 0
> open("/dev/bus/usb/001", 
> O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 3
> fstat64(3, {st_mode=S_IFDIR|0755, st_size=80, ...}) = 0
> getdents(3, /* 4 entries */, 4096)      = 64
> open("/dev/bus/usb/001/004", O_RDWR)    = 4
> ioctl(4, USBDEVFS_CONNECTINFO
>
> nut (for UPS) does not start.
>
> nut       4078  0.0  0.0   1964   856 pts/8    D    07:09   0:00 
> /lib/nut/usbhid-ups -a belkin
> root      4170  0.0  0.0   2204  1072 pts/8    D+   07:11   0:00 lsusb -v
>
> Processes go directly to D-state with 2.6.27.2 for USB.
>
> Booting back to 2.6.26.5 now to see if everything works again.
>
> Back to 2.6.25.5, everything works fine:
>
> # lsusb Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 001 Device 004: ID 2040:7501 Hauppauge Bus 001 Device 001: ID 1d6b:0002 
> Linux Foundation 2.0 root hub
> Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 003 Device 002: ID 050d:1100 Belkin Components Bus 003 Device 001: ID 
> 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 002 Device 002: ID 045e:0039 Microsoft Corp. IntelliMouse Optical
> Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
>
> Relevant configuration files and dmesg output:
>

http://home.comcast.net/~jpiszcz/20081019/2.6.25.5-usb-working.txt
http://home.comcast.net/~jpiszcz/20081019/2.6.27.2-usb-broken.txt
http://home.comcast.net/~jpiszcz/20081019/linux-2.6.25.5-config.txt
http://home.comcast.net/~jpiszcz/20081019/linux-2.6.27.2-config.txt

Re: Kernel 2.6.26.5 -> 2.6.27.2 [USB REGRESSION] (USB -> D_STATE)

[Oliver Neukum]

Am Sonntag, 19. Oktober 2008 13:28:24 schrieb Justin Piszcz:
> 
> On Sun, 19 Oct 2008, Justin Piszcz wrote:
> 
> > With kernel 2.6.27.2, any processes attempting to use USB hang and go into 
> > D-state, I have never had a problem like this before until 2.6.27.2 (I have 
> > not tried 2.6.27.1 or 2.6.27)

Please get sysrq-t of a hang.

	Regards
		Oliver

Re: Kernel 2.6.26.5 -> 2.6.27.2 [USB REGRESSION] (USB -> D_STATE)

[Alan Stern]

On Sun, 19 Oct 2008, Justin Piszcz wrote:

> On Sun, 19 Oct 2008, Justin Piszcz wrote:
> 
> > With kernel 2.6.27.2, any processes attempting to use USB hang and go into 
> > D-state, I have never had a problem like this before until 2.6.27.2 (I have 
> > not tried 2.6.27.1 or 2.6.27)
> >
> > # lsusb -v
> > <hangs>
> >
> > open("/dev/bus/usb", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 
> > 3
> > fstat64(3, {st_mode=S_IFDIR|0755, st_size=140, ...}) = 0
> > getdents(3, /* 7 entries */, 4096)      = 112
> > getdents(3, /* 0 entries */, 4096)      = 0
> > close(3)                                = 0
> > open("/dev/bus/usb/001", 
> > O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 3
> > fstat64(3, {st_mode=S_IFDIR|0755, st_size=80, ...}) = 0
> > getdents(3, /* 4 entries */, 4096)      = 64
> > open("/dev/bus/usb/001/004", O_RDWR)    = 4
> > ioctl(4, USBDEVFS_CONNECTINFO
> >
> > nut (for UPS) does not start.
> >
> > nut       4078  0.0  0.0   1964   856 pts/8    D    07:09   0:00 
> > /lib/nut/usbhid-ups -a belkin
> > root      4170  0.0  0.0   2204  1072 pts/8    D+   07:11   0:00 lsusb -v
> >
> > Processes go directly to D-state with 2.6.27.2 for USB.
> >
> > Booting back to 2.6.26.5 now to see if everything works again.
> >
> > Back to 2.6.25.5, everything works fine:
> >
> > # lsusb Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> > Bus 001 Device 004: ID 2040:7501 Hauppauge Bus 001 Device 001: ID 1d6b:0002 
> > Linux Foundation 2.0 root hub
> > Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> > Bus 003 Device 002: ID 050d:1100 Belkin Components Bus 003 Device 001: ID 
> > 1d6b:0001 Linux Foundation 1.1 root hub
> > Bus 002 Device 002: ID 045e:0039 Microsoft Corp. IntelliMouse Optical
> > Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> >
> > Relevant configuration files and dmesg output:
> >
> 
> http://home.comcast.net/~jpiszcz/20081019/2.6.25.5-usb-working.txt
> http://home.comcast.net/~jpiszcz/20081019/2.6.27.2-usb-broken.txt
> http://home.comcast.net/~jpiszcz/20081019/linux-2.6.25.5-config.txt
> http://home.comcast.net/~jpiszcz/20081019/linux-2.6.27.2-config.txt

You should turn on the usbfs_snoop module parameter for usbcore and see
what shows up in the system log.

Alan Stern

Re: Kernel 2.6.26.5 -> 2.6.27.2 [USB REGRESSION] (USB -> D_STATE)

[Justin Piszcz]

On Sun, 19 Oct 2008, Oliver Neukum wrote:

> You should turn on the usbfs_snoop module parameter for usbcore and see
> what shows up in the system log.

> Alan Stern

Tried this, kernel would not boot up any further when I had that enabled
alongside the 'extra' debugging options in the kernel:

http://home.comcast.net/~jpiszcz/20081019/2.6.27.2-usb-extra-debug.txt

My netconsole is not working right at the moment either so I was not
able to pull a sysrq-of this, but it hung twice, once on one device, rebooted
then it hung on another device the second time.

>
> Please get sysrq-t of a hang.
>
> 	Regards
> 		Oliver
>

Before I tried the above, I was able to get a sysrq-t of the hang:

http://home.comcast.net/~jpiszcz/20081019/trace_before_trigger.txt
http://home.comcast.net/~jpiszcz/20081019/trace_after_trigger.txt

Justin.

Re: Kernel 2.6.26.5 -> 2.6.27.2 [USB REGRESSION] (USB -> D_STATE)

[Alan Stern]

On Sun, 19 Oct 2008, Justin Piszcz wrote:

> > You should turn on the usbfs_snoop module parameter for usbcore and see
> > what shows up in the system log.
> 
> > Alan Stern
> 
> Tried this, kernel would not boot up any further when I had that enabled
> alongside the 'extra' debugging options in the kernel:
> 
> http://home.comcast.net/~jpiszcz/20081019/2.6.27.2-usb-extra-debug.txt

I'm not interested in your config file; I need to see the dmesg log.

> > Please get sysrq-t of a hang.
> >
> > 	Regards
> > 		Oliver
> >
> 
> Before I tried the above, I was able to get a sysrq-t of the hang:
> 
> http://home.comcast.net/~jpiszcz/20081019/trace_before_trigger.txt
> http://home.comcast.net/~jpiszcz/20081019/trace_after_trigger.txt

Your trace is incomplete.  You might need to increase the size of the 
kernel log buffer (CONFIG_LOG_BUF_SHIFT) or the size of the buffer used 
by dmesg (the -s option).

Alan Stern

Re: Kernel 2.6.26.5 -> 2.6.27.2 [USB REGRESSION] (USB -> D_STATE)

[Justin Piszcz]

On Mon, 20 Oct 2008, Alan Stern wrote:

> On Sun, 19 Oct 2008, Justin Piszcz wrote:
>
>>> You should turn on the usbfs_snoop module parameter for usbcore and see
>>> what shows up in the system log.
>>
>>> Alan Stern
>>
>> Tried this, kernel would not boot up any further when I had that enabled
>> alongside the 'extra' debugging options in the kernel:
>>
>> http://home.comcast.net/~jpiszcz/20081019/2.6.27.2-usb-extra-debug.txt
>
> I'm not interested in your config file; I need to see the dmesg log.
>
>>> Please get sysrq-t of a hang.
>>>
>>> 	Regards
>>> 		Oliver
>>>
>>
>> Before I tried the above, I was able to get a sysrq-t of the hang:
>>
>> http://home.comcast.net/~jpiszcz/20081019/trace_before_trigger.txt
>> http://home.comcast.net/~jpiszcz/20081019/trace_after_trigger.txt
>
> Your trace is incomplete.  You might need to increase the size of the
> kernel log buffer (CONFIG_LOG_BUF_SHIFT) or the size of the buffer used
> by dmesg (the -s option).
>
> Alan Stern
>

You're right, I set it to 128 KiB and here is the full log, including the 
initial dmesg:

http://home.comcast.net/~jpiszcz/20081019/kern.log

Justin.

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

[Alan Stern]

On Mon, 20 Oct 2008, Justin Piszcz wrote:

> >> http://home.comcast.net/~jpiszcz/20081019/trace_before_trigger.txt
> >> http://home.comcast.net/~jpiszcz/20081019/trace_after_trigger.txt
> >
> > Your trace is incomplete.  You might need to increase the size of the
> > kernel log buffer (CONFIG_LOG_BUF_SHIFT) or the size of the buffer used
> > by dmesg (the -s option).
> >
> > Alan Stern
> >
> 
> You're right, I set it to 128 KiB and here is the full log, including the 
> initial dmesg:
> 
> http://home.comcast.net/~jpiszcz/20081019/kern.log

Okay, I see the problem. It's the same as we saw with the speedtouch
driver last week: The pvrusb2 driver resets its device but doesn't
define a pre_reset or a post_reset method.  As a result it gets
disconnected during the reset, and the recursive call causes it to
hang.

This patch should fix the problem.  It's not entirely correct, but it 
should at least allow the driver to work like it did in 2.6.26.

Alan Stern



Index: usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
===================================================================
--- usb-2.6.orig/drivers/media/video/pvrusb2/pvrusb2-main.c
+++ usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
@@ -68,6 +68,16 @@ static void pvr_setup_attach(struct pvr2
 #endif /* CONFIG_VIDEO_PVRUSB2_SYSFS */
 }
 
+static int pvr_pre_reset(struct usb_interface *intf)
+{
+	return 0;
+}
+
+static int pvr_post_reset(struct usb_interface *intf)
+{
+	return 0;
+}
+
 static int pvr_probe(struct usb_interface *intf,
 		     const struct usb_device_id *devid)
 {
@@ -109,7 +119,9 @@ static struct usb_driver pvr_driver = {
 	.name =         "pvrusb2",
 	.id_table =     pvr2_device_table,
 	.probe =        pvr_probe,
-	.disconnect =   pvr_disconnect
+	.disconnect =   pvr_disconnect,
+	.pre_reset =	pvr_pre_reset,
+	.post_reset =	pvr_post_reset,
 };
 
 /*

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

[Mike Isely]

There's already a patch coming up through v4l-dvb that should remove the 
need for the reset completely for the pvrusb2 driver.  The reset had 
been there as "chicken soup" previously - it didn't hurt but its 
utility wasn't really that great at the time.  Now that it is hurting, 
I just removed it.

  -Mike


On Mon, 20 Oct 2008, Alan Stern wrote:

> On Mon, 20 Oct 2008, Justin Piszcz wrote:
> 
> > >> http://home.comcast.net/~jpiszcz/20081019/trace_before_trigger.txt
> > >> http://home.comcast.net/~jpiszcz/20081019/trace_after_trigger.txt
> > >
> > > Your trace is incomplete.  You might need to increase the size of the
> > > kernel log buffer (CONFIG_LOG_BUF_SHIFT) or the size of the buffer used
> > > by dmesg (the -s option).
> > >
> > > Alan Stern
> > >
> > 
> > You're right, I set it to 128 KiB and here is the full log, including the 
> > initial dmesg:
> > 
> > http://home.comcast.net/~jpiszcz/20081019/kern.log
> 
> Okay, I see the problem. It's the same as we saw with the speedtouch
> driver last week: The pvrusb2 driver resets its device but doesn't
> define a pre_reset or a post_reset method.  As a result it gets
> disconnected during the reset, and the recursive call causes it to
> hang.
> 
> This patch should fix the problem.  It's not entirely correct, but it 
> should at least allow the driver to work like it did in 2.6.26.
> 
> Alan Stern
> 
> 
> 
> Index: usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
> ===================================================================
> --- usb-2.6.orig/drivers/media/video/pvrusb2/pvrusb2-main.c
> +++ usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
> @@ -68,6 +68,16 @@ static void pvr_setup_attach(struct pvr2
>  #endif /* CONFIG_VIDEO_PVRUSB2_SYSFS */
>  }
>  
> +static int pvr_pre_reset(struct usb_interface *intf)
> +{
> +	return 0;
> +}
> +
> +static int pvr_post_reset(struct usb_interface *intf)
> +{
> +	return 0;
> +}
> +
>  static int pvr_probe(struct usb_interface *intf,
>  		     const struct usb_device_id *devid)
>  {
> @@ -109,7 +119,9 @@ static struct usb_driver pvr_driver = {
>  	.name =         "pvrusb2",
>  	.id_table =     pvr2_device_table,
>  	.probe =        pvr_probe,
> -	.disconnect =   pvr_disconnect
> +	.disconnect =   pvr_disconnect,
> +	.pre_reset =	pvr_pre_reset,
> +	.post_reset =	pvr_post_reset,
>  };
>  
>  /*
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

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

[Justin Piszcz]

On Mon, 20 Oct 2008, Mike Isely wrote:

>
> There's already a patch coming up through v4l-dvb that should remove the
> need for the reset completely for the pvrusb2 driver.  The reset had
> been there as "chicken soup" previously - it didn't hurt but its
> utility wasn't really that great at the time.  Now that it is hurting,
> I just removed it.
>
>  -Mike
>

Should I wait for the patch coming up through v4l-dvb or test Alan's patch?
2.6.27.2 has quite a few issues:

1. xfs is broken w/barrier (there is a patch for this)
2. usb hangs (patch below)
3. abit-guru (comsetic issue only, doesn't find mobo) (there is a patch 
for this now as well)

So I am sticking with 2.6.26.5 right now, let me know if you need me
to test Alan's patch to see if it fixes the issue.

Justin.

>>
>> Index: usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
>> ===================================================================
>> --- usb-2.6.orig/drivers/media/video/pvrusb2/pvrusb2-main.c
>> +++ usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
>> @@ -68,6 +68,16 @@ static void pvr_setup_attach(struct pvr2
>>  #endif /* CONFIG_VIDEO_PVRUSB2_SYSFS */
>>  }
>>
>> +static int pvr_pre_reset(struct usb_interface *intf)
>> +{
>> +	return 0;
>> +}
>> +
>> +static int pvr_post_reset(struct usb_interface *intf)
>> +{
>> +	return 0;
>> +}
>> +
>>  static int pvr_probe(struct usb_interface *intf,
>>  		     const struct usb_device_id *devid)
>>  {
>> @@ -109,7 +119,9 @@ static struct usb_driver pvr_driver = {
>>  	.name =         "pvrusb2",
>>  	.id_table =     pvr2_device_table,
>>  	.probe =        pvr_probe,
>> -	.disconnect =   pvr_disconnect
>> +	.disconnect =   pvr_disconnect,
>> +	.pre_reset =	pvr_pre_reset,
>> +	.post_reset =	pvr_post_reset,
>>  };
>>
>>  /*

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

[Mike Isely]

I was incomplete in my previous response.  See further below for nack 
and another patch...


On Mon, 20 Oct 2008, Alan Stern wrote:

   [...]

> 
> Index: usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
> ===================================================================
> --- usb-2.6.orig/drivers/media/video/pvrusb2/pvrusb2-main.c
> +++ usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
> @@ -68,6 +68,16 @@ static void pvr_setup_attach(struct pvr2
>  #endif /* CONFIG_VIDEO_PVRUSB2_SYSFS */
>  }
>  
> +static int pvr_pre_reset(struct usb_interface *intf)
> +{
> +	return 0;
> +}
> +
> +static int pvr_post_reset(struct usb_interface *intf)
> +{
> +	return 0;
> +}
> +
>  static int pvr_probe(struct usb_interface *intf,
>  		     const struct usb_device_id *devid)
>  {
> @@ -109,7 +119,9 @@ static struct usb_driver pvr_driver = {
>  	.name =         "pvrusb2",
>  	.id_table =     pvr2_device_table,
>  	.probe =        pvr_probe,
> -	.disconnect =   pvr_disconnect
> +	.disconnect =   pvr_disconnect,
> +	.pre_reset =	pvr_pre_reset,
> +	.post_reset =	pvr_post_reset,
>  };
>  
>  /*
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Nacked-by: Mike Isely <isely@pobox.com>

There is already another patch ready to go which eliminates the reset 
entirely.  It can be found here:

http://linuxtv.org/hg/~mcisely/pvrusb2/rev/0bb411d8d2e4

  -Mike


-- 

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

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

[Mike Isely]

On Mon, 20 Oct 2008, Justin Piszcz wrote:

> 
> 
> On Mon, 20 Oct 2008, Mike Isely wrote:
> 
> > 
> > There's already a patch coming up through v4l-dvb that should remove the
> > need for the reset completely for the pvrusb2 driver.  The reset had
> > been there as "chicken soup" previously - it didn't hurt but its
> > utility wasn't really that great at the time.  Now that it is hurting,
> > I just removed it.
> > 
> >  -Mike
> > 
> 
> Should I wait for the patch coming up through v4l-dvb or test Alan's patch?
> 2.6.27.2 has quite a few issues:
> 
> 1. xfs is broken w/barrier (there is a patch for this)
> 2. usb hangs (patch below)
> 3. abit-guru (comsetic issue only, doesn't find mobo) (there is a patch for
> this now as well)
> 
> So I am sticking with 2.6.26.5 right now, let me know if you need me
> to test Alan's patch to see if it fixes the issue.

I'm sorry I didn't post my patch immediately in my previous response.  
It is however posted as a link in a followup, and for the record here it 
is again:

http://linuxtv.org/hg/~mcisely/pvrusb2/rev/0bb411d8d2e4

I have tested it, and it works as expected.  I've done all I can 
according to the process there to ready it for kernel inclusion and I've 
asked in my pull request there to get this patch into the next 2.6.27.x 
stable release.

The driver really didn't "need" the reset anyway, and so from where I'm 
sitting the simplest thing is just to remove it which is what my patch 
does.  (I had put in that reset a long time ago when working on 
improving hardware stability.  I had never positively established that 
this actually helped during initialization, but it didn't hurt either so 
I left it in place.  Now that it is causing pain, it's best just to get 
it out of there.)  I don't see much point in testing Alan's patch.

  -Mike


-- 

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

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

[Justin Piszcz]

On Mon, 20 Oct 2008, Mike Isely wrote:

> On Mon, 20 Oct 2008, Justin Piszcz wrote:
>
>>
>>
>> On Mon, 20 Oct 2008, Mike Isely wrote:
>>
>>>
>>> There's already a patch coming up through v4l-dvb that should remove the
>>> need for the reset completely for the pvrusb2 driver.  The reset had
>>> been there as "chicken soup" previously - it didn't hurt but its
>>> utility wasn't really that great at the time.  Now that it is hurting,
>>> I just removed it.
>>>
>>>  -Mike
>>>
>>
>> Should I wait for the patch coming up through v4l-dvb or test Alan's patch?
>> 2.6.27.2 has quite a few issues:
>>
>> 1. xfs is broken w/barrier (there is a patch for this)
>> 2. usb hangs (patch below)
>> 3. abit-guru (comsetic issue only, doesn't find mobo) (there is a patch for
>> this now as well)
>>
>> So I am sticking with 2.6.26.5 right now, let me know if you need me
>> to test Alan's patch to see if it fixes the issue.
>
> I'm sorry I didn't post my patch immediately in my previous response.
> It is however posted as a link in a followup, and for the record here it
> is again:
>
> http://linuxtv.org/hg/~mcisely/pvrusb2/rev/0bb411d8d2e4
>
> I have tested it, and it works as expected.  I've done all I can
> according to the process there to ready it for kernel inclusion and I've
> asked in my pull request there to get this patch into the next 2.6.27.x
> stable release.
>
> The driver really didn't "need" the reset anyway, and so from where I'm
> sitting the simplest thing is just to remove it which is what my patch
> does.  (I had put in that reset a long time ago when working on
> improving hardware stability.  I had never positively established that
> this actually helped during initialization, but it didn't hurt either so
> I left it in place.  Now that it is causing pain, it's best just to get
> it out of there.)  I don't see much point in testing Alan's patch.

Ok, so the following patche are what I have applied to 2.6.27.2:

1. http://linuxtv.org/hg/~mcisely/pvrusb2/rev/0bb411d8d2e4

--

1. aw9d-max-dmi-detection.diff
2. pvrusb2-hdw.patch
3. xfs-fix-remount-rw-with-unrecognized-options.patch

Rebooting 2.6.27.2 with applied patches (all 3) and will check to make 
sure everything is back to normal.

Done, after reboot with the [3] applied patches above, it has fixed all 
the problems I was having.

Justin.

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

[Alan Stern]

On Mon, 20 Oct 2008, Mike Isely wrote:

> > Should I wait for the patch coming up through v4l-dvb or test Alan's patch?
> > 2.6.27.2 has quite a few issues:
> > 
> > 1. xfs is broken w/barrier (there is a patch for this)
> > 2. usb hangs (patch below)
> > 3. abit-guru (comsetic issue only, doesn't find mobo) (there is a patch for
> > this now as well)
> > 
> > So I am sticking with 2.6.26.5 right now, let me know if you need me
> > to test Alan's patch to see if it fixes the issue.
> 
> I'm sorry I didn't post my patch immediately in my previous response.  
> It is however posted as a link in a followup, and for the record here it 
> is again:
> 
> http://linuxtv.org/hg/~mcisely/pvrusb2/rev/0bb411d8d2e4
> 
> I have tested it, and it works as expected.  I've done all I can 
> according to the process there to ready it for kernel inclusion and I've 
> asked in my pull request there to get this patch into the next 2.6.27.x 
> stable release.
> 
> The driver really didn't "need" the reset anyway, and so from where I'm 
> sitting the simplest thing is just to remove it which is what my patch 
> does.  (I had put in that reset a long time ago when working on 
> improving hardware stability.  I had never positively established that 
> this actually helped during initialization, but it didn't hurt either so 
> I left it in place.  Now that it is causing pain, it's best just to get 
> it out of there.)  I don't see much point in testing Alan's patch.

I agree with Mike.  His changes make the patch I sent totally 
irrelevant, and they should fix the problem you're facing.

Alan Stern

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

[Greg KH]

On Mon, Oct 20, 2008 at 11:37:41AM -0500, Mike Isely wrote:
> 
> I was incomplete in my previous response.  See further below for nack 
> and another patch...
> 
> 
> On Mon, 20 Oct 2008, Alan Stern wrote:
> 
>    [...]
> 
> > 
> > Index: usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
> > ===================================================================
> > --- usb-2.6.orig/drivers/media/video/pvrusb2/pvrusb2-main.c
> > +++ usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
> > @@ -68,6 +68,16 @@ static void pvr_setup_attach(struct pvr2
> >  #endif /* CONFIG_VIDEO_PVRUSB2_SYSFS */
> >  }
> >  
> > +static int pvr_pre_reset(struct usb_interface *intf)
> > +{
> > +	return 0;
> > +}
> > +
> > +static int pvr_post_reset(struct usb_interface *intf)
> > +{
> > +	return 0;
> > +}
> > +
> >  static int pvr_probe(struct usb_interface *intf,
> >  		     const struct usb_device_id *devid)
> >  {
> > @@ -109,7 +119,9 @@ static struct usb_driver pvr_driver = {
> >  	.name =         "pvrusb2",
> >  	.id_table =     pvr2_device_table,
> >  	.probe =        pvr_probe,
> > -	.disconnect =   pvr_disconnect
> > +	.disconnect =   pvr_disconnect,
> > +	.pre_reset =	pvr_pre_reset,
> > +	.post_reset =	pvr_post_reset,
> >  };
> >  
> >  /*
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> Nacked-by: Mike Isely <isely@pobox.com>
> 
> There is already another patch ready to go which eliminates the reset 
> entirely.  It can be found here:
> 
> http://linuxtv.org/hg/~mcisely/pvrusb2/rev/0bb411d8d2e4

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?

thanks,

greg k-h

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

[Michael Krufky]

On Mon, Oct 20, 2008 at 1:29 PM, Greg KH <greg@kroah.com> wrote:
> On Mon, Oct 20, 2008 at 11:37:41AM -0500, Mike Isely wrote:
>>
>> I was incomplete in my previous response.  See further below for nack
>> and another patch...
>>
>>
>> On Mon, 20 Oct 2008, Alan Stern wrote:
>>
>>    [...]
>>
>> >
>> > Index: usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
>> > ===================================================================
>> > --- usb-2.6.orig/drivers/media/video/pvrusb2/pvrusb2-main.c
>> > +++ usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
>> > @@ -68,6 +68,16 @@ static void pvr_setup_attach(struct pvr2
>> >  #endif /* CONFIG_VIDEO_PVRUSB2_SYSFS */
>> >  }
>> >
>> > +static int pvr_pre_reset(struct usb_interface *intf)
>> > +{
>> > +   return 0;
>> > +}
>> > +
>> > +static int pvr_post_reset(struct usb_interface *intf)
>> > +{
>> > +   return 0;
>> > +}
>> > +
>> >  static int pvr_probe(struct usb_interface *intf,
>> >                  const struct usb_device_id *devid)
>> >  {
>> > @@ -109,7 +119,9 @@ static struct usb_driver pvr_driver = {
>> >     .name =         "pvrusb2",
>> >     .id_table =     pvr2_device_table,
>> >     .probe =        pvr_probe,
>> > -   .disconnect =   pvr_disconnect
>> > +   .disconnect =   pvr_disconnect,
>> > +   .pre_reset =    pvr_pre_reset,
>> > +   .post_reset =   pvr_post_reset,
>> >  };
>> >
>> >  /*
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >
>>
>> Nacked-by: Mike Isely <isely@pobox.com>
>>
>> There is already another patch ready to go which eliminates the reset
>> entirely.  It can be found here:
>>
>> http://linuxtv.org/hg/~mcisely/pvrusb2/rev/0bb411d8d2e4
>
> 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,

Please queue the patch that Mike Isely sent for 2.6.27.y -- It's not
in Mauro's tree yet, but it will get there eventually.  (it might take
him some before he does his next sync to git)

Reviewed-by: Michael Krufky <mkrufky@linuxtv.org>

Regards,

Mike Krufky

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

[Mike Isely]

On Mon, 20 Oct 2008, Greg KH wrote:

> On Mon, Oct 20, 2008 at 11:37:41AM -0500, Mike Isely wrote:
> > 
> > I was incomplete in my previous response.  See further below for nack 
> > and another patch...
> > 
> > 
> > On Mon, 20 Oct 2008, Alan Stern wrote:
> > 
> >    [...]
> > 
> > > 
> > > Index: usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
> > > ===================================================================
> > > --- usb-2.6.orig/drivers/media/video/pvrusb2/pvrusb2-main.c
> > > +++ usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
> > > @@ -68,6 +68,16 @@ static void pvr_setup_attach(struct pvr2
> > >  #endif /* CONFIG_VIDEO_PVRUSB2_SYSFS */
> > >  }
> > >  
> > > +static int pvr_pre_reset(struct usb_interface *intf)
> > > +{
> > > +	return 0;
> > > +}
> > > +
> > > +static int pvr_post_reset(struct usb_interface *intf)
> > > +{
> > > +	return 0;
> > > +}
> > > +
> > >  static int pvr_probe(struct usb_interface *intf,
> > >  		     const struct usb_device_id *devid)
> > >  {
> > > @@ -109,7 +119,9 @@ static struct usb_driver pvr_driver = {
> > >  	.name =         "pvrusb2",
> > >  	.id_table =     pvr2_device_table,
> > >  	.probe =        pvr_probe,
> > > -	.disconnect =   pvr_disconnect
> > > +	.disconnect =   pvr_disconnect,
> > > +	.pre_reset =	pvr_pre_reset,
> > > +	.post_reset =	pvr_post_reset,
> > >  };
> > >  
> > >  /*
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > 
> > 
> > Nacked-by: Mike Isely <isely@pobox.com>
> > 
> > There is already another patch ready to go which eliminates the reset 
> > entirely.  It can be found here:
> > 
> > http://linuxtv.org/hg/~mcisely/pvrusb2/rev/0bb411d8d2e4
> 
> 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:

I didn't directly answer your question here because I had figured it was 
answered in a previous post on this thread, that Mike Krufky had already 
explicitly asked that it be queued (and added his Reviewed-By tag), and 
that I figured it best not to add yet more noise to an already noisy 
group.

However now I see that this patch didn't get into 2.6.27.3.  It's a 
pretty important fix; without it the pvrusb2 driver is worse than 
useless (unless one adds initusbreset=0 as a module option).

My POV for this process in the past is that I generate a stable fix at 
linuxtv.org, ensure that Mike Krufky and Mauro see the incoming change, 
and then it gets into the next stable release.  This is what I thought I 
did here, but the results are different - 2.6.27.3 still has the driver 
in a totally broken state.  Quite likely I missed something that caused 
the fix not to be pulled.  Is there anything I can do to ensure this get 
into 2.6.27.4?

  -Mike


-- 

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

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

[Greg KH]

On Thu, Oct 23, 2008 at 11:27:11AM -0500, Mike Isely wrote:
> On Mon, 20 Oct 2008, Greg KH wrote:
> 
> > On Mon, Oct 20, 2008 at 11:37:41AM -0500, Mike Isely wrote:
> > > 
> > > I was incomplete in my previous response.  See further below for nack 
> > > and another patch...
> > > 
> > > 
> > > On Mon, 20 Oct 2008, Alan Stern wrote:
> > > 
> > >    [...]
> > > 
> > > > 
> > > > Index: usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
> > > > ===================================================================
> > > > --- usb-2.6.orig/drivers/media/video/pvrusb2/pvrusb2-main.c
> > > > +++ usb-2.6/drivers/media/video/pvrusb2/pvrusb2-main.c
> > > > @@ -68,6 +68,16 @@ static void pvr_setup_attach(struct pvr2
> > > >  #endif /* CONFIG_VIDEO_PVRUSB2_SYSFS */
> > > >  }
> > > >  
> > > > +static int pvr_pre_reset(struct usb_interface *intf)
> > > > +{
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int pvr_post_reset(struct usb_interface *intf)
> > > > +{
> > > > +	return 0;
> > > > +}
> > > > +
> > > >  static int pvr_probe(struct usb_interface *intf,
> > > >  		     const struct usb_device_id *devid)
> > > >  {
> > > > @@ -109,7 +119,9 @@ static struct usb_driver pvr_driver = {
> > > >  	.name =         "pvrusb2",
> > > >  	.id_table =     pvr2_device_table,
> > > >  	.probe =        pvr_probe,
> > > > -	.disconnect =   pvr_disconnect
> > > > +	.disconnect =   pvr_disconnect,
> > > > +	.pre_reset =	pvr_pre_reset,
> > > > +	.post_reset =	pvr_post_reset,
> > > >  };
> > > >  
> > > >  /*
> > > > 
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > > 
> > > 
> > > Nacked-by: Mike Isely <isely@pobox.com>
> > > 
> > > There is already another patch ready to go which eliminates the reset 
> > > entirely.  It can be found here:
> > > 
> > > http://linuxtv.org/hg/~mcisely/pvrusb2/rev/0bb411d8d2e4
> > 
> > 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:
> 
> I didn't directly answer your question here because I had figured it was 
> answered in a previous post on this thread, that Mike Krufky had already 
> explicitly asked that it be queued (and added his Reviewed-By tag), and 
> that I figured it best not to add yet more noise to an already noisy 
> group.
> 
> However now I see that this patch didn't get into 2.6.27.3.  It's a 
> pretty important fix; without it the pvrusb2 driver is worse than 
> useless (unless one adds initusbreset=0 as a module option).

As we were discussing this on Monday, and the review cycle for 2.6.27.3
started last Saturday, it would have been pretty hard to get it into
2.6.27.3 :)

I need to see the patch in Linus's tree first, before it can go into any
stable release.  Is it in there yet?  If so, please send me the git
commit id and I'll queue it up for the next -stable round.

thanks,

greg k-h

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

[Mike Isely]

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:
> > 
> > I didn't directly answer your question here because I had figured it was 
> > answered in a previous post on this thread, that Mike Krufky had already 
> > explicitly asked that it be queued (and added his Reviewed-By tag), and 
> > that I figured it best not to add yet more noise to an already noisy 
> > group.
> > 
> > However now I see that this patch didn't get into 2.6.27.3.  It's a 
> > pretty important fix; without it the pvrusb2 driver is worse than 
> > useless (unless one adds initusbreset=0 as a module option).
> 
> As we were discussing this on Monday, and the review cycle for 2.6.27.3
> started last Saturday, it would have been pretty hard to get it into
> 2.6.27.3 :)
> 
> I need to see the patch in Linus's tree first, before it can go into any
> stable release.  Is it in there yet?  If so, please send me the git
> commit id and I'll queue it up for the next -stable round.
> 

OK, I understand.

As far as I know, Linus has not yet pulled the changes from Mauro (part 
of a larger set).  But that information is a few hours old.  I or Mike 
Krufky will let you know if/when one of us finds out.

Thanks for following up.

  -Mike


-- 

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

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

[Michael Krufky]

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:
>> >
>> > I didn't directly answer your question here because I had figured it was
>> > answered in a previous post on this thread, that Mike Krufky had already
>> > explicitly asked that it be queued (and added his Reviewed-By tag), and
>> > that I figured it best not to add yet more noise to an already noisy
>> > group.
>> >
>> > However now I see that this patch didn't get into 2.6.27.3.  It's a
>> > pretty important fix; without it the pvrusb2 driver is worse than
>> > useless (unless one adds initusbreset=0 as a module option).
>>
>> As we were discussing this on Monday, and the review cycle for 2.6.27.3
>> started last Saturday, it would have been pretty hard to get it into
>> 2.6.27.3 :)
>>
>> I need to see the patch in Linus's tree first, before it can go into any
>> stable release.  Is it in there yet?  If so, please send me the git
>> commit id and I'll queue it up for the next -stable round.
>>
>
> OK, I understand.
>
> As far as I know, Linus has not yet pulled the changes from Mauro (part
> of a larger set).  But that information is a few hours old.  I or Mike
> Krufky will let you know if/when one of us finds out.
>
> Thanks for following up.
>
>  -Mike

Mike,

Have you tested to confirm that FX2 firmware download without USB
reset still leaves the DTV firmware commands operable?

In other words, I'm not entirely sure about this fix.  If both Digital
and Analog still work with the patch included, then as far as I am
concerned, its fine for stable.

Greg, this is already in Linus' tree as of a few hours ago.  I had
already cherry-picked it and had planned to send it off to you after
testing it myself, but I probably wont get to that until Saturday or
Sunday.

If Mike Isely can confirm that Digital modes still work on the HVR1950
after a firmware download without an FX2 reset, then we don't need to
wait for my own testing.

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

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

[Justin Piszcz]

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.

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

[Mike Isely]

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

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

[Michael Krufky]

[-- 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

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

[Greg KH]

On Fri, Oct 24, 2008 at 11:29:36AM -0400, Michael Krufky wrote:
> 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.

Yes, I will add it right now.

thanks for your persistance.

greg k-h