Re: [linux-usb-devel] spontaneous disconnect with "usb-storage: implement autosuspend"

Previous thread: Re: + serial-turn-serial-console-suspend-a-boot-rather-than-compile-time-option.patch added to -mm tree by Pavel Machek on Tuesday, August 14, 2007 - 6:39 am. (4 messages)

Next thread: [PATCH] Fix rmmod of asus_laptop by Guillaume Chazarain on Tuesday, August 14, 2007 - 7:22 am. (1 message)
To: Alan Stern <stern@...>
Cc: <linux-usb-devel@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Tuesday, August 14, 2007 - 7:15 am

Hi,

with "CONFIG_USB_SUSPEND=y", since commit:

8dfe4b14869fd185ca25ee88b02ada58a3005eaf
usb-storage: implement autosuspend

This patch (as930) implements autosuspend for usb-storage. It is
adapted from a patch by Oliver Neukum. Autosuspend is allowed except
during LUN scanning, resets, and command execution.

my USB photo-camera gets automagically disconnected before I can do
anything with it ;)

Relevant dmesg (what I get by just attaching it):

[ 70.722898] scsi 8:0:0:0: Direct-Access HP PHOTOSMART E317 A001 PQ: 0 ANSI: 0 CCS
[ 70.727880] sd 8:0:0:0: [sdb] 488448 512-byte hardware sectors (250 MB)
[ 70.730876] sd 8:0:0:0: [sdb] Write Protect is off
[ 70.730879] sd 8:0:0:0: [sdb] Mode Sense: 00 46 00 00
[ 70.730882] sd 8:0:0:0: [sdb] Assuming drive cache: write through
[ 70.742874] sd 8:0:0:0: [sdb] 488448 512-byte hardware sectors (250 MB)
[ 70.745873] sd 8:0:0:0: [sdb] Write Protect is off
[ 70.745876] sd 8:0:0:0: [sdb] Mode Sense: 00 46 00 00
[ 70.745878] sd 8:0:0:0: [sdb] Assuming drive cache: write through
[ 70.745882] sdb: sdb1
[ 70.755192] sd 8:0:0:0: [sdb] Attached SCSI removable disk
[ 70.755446] sd 8:0:0:0: Attached scsi generic sg3 type 0
[ 70.755657] usb-storage: device scan complete
[ 73.071490] usb 6-2: usb auto-suspend
[ 73.321445] hub 6-0:1.0: state 7 ports 2 chg 0000 evt 0004
[ 73.321453] uhci_hcd 0000:00:1d.1: port 2 portsc 108a,00
[ 73.321462] hub 6-0:1.0: port 2, status 0100, change 0003, 12 Mb/s
[ 73.321465] usb 6-2: USB disconnect, address 2
[ 73.321467] usb 6-2: unregistering device

The relevant differences of dmesg between pre and post "autosuspend"
patch:

@@ -1,4 +1,4 @@
- Linux version 2.6.22-gb0e2a705 ...
+ Linux version 2.6.22-g8dfe4b14 ...
@@ -606,6 +606,12 @@
hub 1-0:1.0: hub_suspend
usb usb1: bus auto-suspend
ehci_hcd 0000:00:1a.7: suspend root hub
+ hub 4-0:1.0: hub_suspend
+ usb usb4: bus auto-suspend
+ usb usb4: suspend_rh
+ hub 5-0:1.0: hub_...

To: <linux-usb-devel@...>
Cc: Paolo Ornati <ornati@...>, Alan Stern <stern@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Tuesday, August 14, 2007 - 9:30 am

hi,

I need vendor:product. Please provide "lsusb".
Generally a bug report for a specific usb device without vendor:product
is a bad idea.

Regards
Oliver

-

To: Oliver Neukum <oliver@...>
Cc: <linux-usb-devel@...>, Alan Stern <stern@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Tuesday, August 14, 2007 - 9:47 am

On Tue, 14 Aug 2007 15:30:27 +0200

oopss :)

HP PHOTOSMART E317

tux ~ # lsusb
Bus 007 Device 001: ID 0000:0000
Bus 006 Device 002: ID 03f0:4002 Hewlett-Packard PhotoSmart 720 / PhotoSmart 935 (storage)
...

--
Paolo Ornati
Linux 2.6.23-rc3 on x86_64
-

To: Paolo Ornati <ornati@...>
Cc: <linux-usb-devel@...>, Alan Stern <stern@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Tuesday, August 14, 2007 - 11:46 am

Please try this patch.

Regards
Oliver

-----

--- a/drivers/usb/core/quirks.c 2007-08-14 17:42:22.000000000 +0200
+++ b/drivers/usb/core/quirks.c 2007-08-14 17:43:51.000000000 +0200
@@ -30,6 +30,8 @@
static const struct usb_device_id usb_quirk_list[] = {
/* HP 5300/5370C scanner */
{ USB_DEVICE(0x03f0, 0x0701), .driver_info = USB_QUIRK_STRING_FETCH_255 },
+ /* Hewlett-Packard PhotoSmart 720 / PhotoSmart 935 (storage) */
+ { USB_DEVICE(0x03f0, 0x4002), .driver_info = USB_QUIRK_RESET_RESUME },
/* Acer Peripherals Inc. (now BenQ Corp.) Prisa 640BU */
{ USB_DEVICE(0x04a5, 0x207e), .driver_info = USB_QUIRK_NO_AUTOSUSPEND },
/* Benq S2W 3300U */

-

To: Oliver Neukum <oliver@...>
Cc: <linux-usb-devel@...>, Alan Stern <stern@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Tuesday, August 14, 2007 - 12:38 pm

On Tue, 14 Aug 2007 17:46:16 +0200

Tried on -rc3 but it doesn't work, dmesg attached.

However I've found that if "hald" is running the problems doesn't
happen (I think it's just hidden by the fact that hald do some polling
on it preventing autosuspend to trigger).

--
Paolo Ornati
Linux 2.6.23-rc3 on x86_64

To: Paolo Ornati <ornati@...>
Cc: <linux-usb-devel@...>, Alan Stern <stern@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Tuesday, August 14, 2007 - 2:33 pm

Exactly. This is not reliable. It needs to be done in kernel. This patch
should do it.

Regards
Oliver

---

--- a/drivers/usb/core/quirks.c 2007-08-14 17:42:22.000000000 +0200
+++ b/drivers/usb/core/quirks.c 2007-08-14 20:30:28.000000000 +0200
@@ -30,6 +30,8 @@
static const struct usb_device_id usb_quirk_list[] = {
/* HP 5300/5370C scanner */
{ USB_DEVICE(0x03f0, 0x0701), .driver_info = USB_QUIRK_STRING_FETCH_255 },
+ /* Hewlett-Packard PhotoSmart 720 / PhotoSmart 935 (storage) */
+ { USB_DEVICE(0x03f0, 0x4002), .driver_info = USB_QUIRK_NO_AUTOSUSPEND },
/* Acer Peripherals Inc. (now BenQ Corp.) Prisa 640BU */
{ USB_DEVICE(0x04a5, 0x207e), .driver_info = USB_QUIRK_NO_AUTOSUSPEND },
/* Benq S2W 3300U */
-

To: Oliver Neukum <oliver@...>
Cc: Paolo Ornati <ornati@...>, <linux-usb-devel@...>, Alan Stern <stern@...>, Linux Kernel Mailing List <linux-kernel@...>, Andrew Morton <akpm@...>, Linus Torvalds <torvalds@...>
Date: Wednesday, September 12, 2007 - 6:14 pm

I believe the offending commit needs to be reverted.

Ditto for several other devices that are being slowly special-cased,
and many that have yet to be tested. This commit is (unfortunately)
a disaster with many regressions.

Andrew, Linus?
-

To: Mark Lord <lkml@...>
Cc: Oliver Neukum <oliver@...>, Paolo Ornati <ornati@...>, <linux-usb-devel@...>, Alan Stern <stern@...>, Linux Kernel Mailing List <linux-kernel@...>, Andrew Morton <akpm@...>, Linus Torvalds <torvalds@...>
Date: Wednesday, September 12, 2007 - 7:40 pm

There are many regressions right now, _ONLY_ if you enable
CONFIG_USB_SUSPEND. If you disable that, your problems will go away,
right?

This option is a new option, and we have found out the hard way that
a very large class of hardware really does not like working with usb
suspend at all.

Because of this, we have a patch queued up for 2.6.24 that will disable
suspend for devices, and have to be enabled from a white-list that we
will put in userspace based on the history of what other operating
systems have determined are devices that can sleep properly.

I can send this patch in now to Linus, but as it changes functionality
from previous -rc patches, I've been hesitant to do so. Especially when
a mere config option change will solve your problem.

Oh, and currently no distro will enable this option due to the hardware
problems, so the only people that could get hit by this are those who
build their own kernels, and they can easily disable the option.

Does this help explain things?

thanks,

greg k-h
-

To: Greg KH <greg@...>
Cc: Oliver Neukum <oliver@...>, Paolo Ornati <ornati@...>, <linux-usb-devel@...>, Alan Stern <stern@...>, Linux Kernel Mailing List <linux-kernel@...>, Andrew Morton <akpm@...>, Linus Torvalds <torvalds@...>
Date: Wednesday, September 12, 2007 - 10:33 pm

Ubuntu has CONFIG_USB_SUSPEND=y

There are probably lots of other distros that use it as well.

This new behaviour (failed operation of USB flash keys) is a *new* regression
in 2.6.23, and we still have time to fix it by either reverting the changed
functionality, or by changing the code to default OFF.

Here's my patch for 2.6.23-rc6+ :

Fix USB Storage failures.

Signed-Off-By: Mark Lord <mlord@pobox.com>
---

--- linux/drivers/usb/storage/usb.c.orig 2007-09-11 11:52:14.000000000 -0400
+++ linux/drivers/usb/storage/usb.c 2007-09-12 18:18:35.000000000 -0400
@@ -1065,7 +1065,7 @@
.pre_reset = storage_pre_reset,
.post_reset = storage_post_reset,
.id_table = storage_usb_ids,
- .supports_autosuspend = 1,
+ .supports_autosuspend = 0,
};

static int __init usb_stor_init(void)
-

To: Mark Lord <lkml@...>
Cc: Oliver Neukum <oliver@...>, Paolo Ornati <ornati@...>, <linux-usb-devel@...>, Alan Stern <stern@...>, Linux Kernel Mailing List <linux-kernel@...>, Andrew Morton <akpm@...>, Linus Torvalds <torvalds@...>
Date: Thursday, September 13, 2007 - 5:40 am

They also have a patch that disables suspend automatically for all

That's way too simple, I want to revert the whole usb-storage suspend
logic as Oliver has found some core problems with it working properly,
even with devices that have no problems.

Is that ok for you?

thanks,

greg k-h
-

To: Greg KH <greg@...>
Cc: Oliver Neukum <oliver@...>, Paolo Ornati <ornati@...>, <linux-usb-devel@...>, Alan Stern <stern@...>, Linux Kernel Mailing List <linux-kernel@...>, Andrew Morton <akpm@...>, Linus Torvalds <torvalds@...>
Date: Wednesday, September 12, 2007 - 10:29 pm

CONFIG_USB_SUSPEND is *not* a new config option.
It's been around for quite some time now, and I also had it
enabled in 2.6.22 without any troubles. Definite regression here, folks!

Cheers
-

To: Oliver Neukum <oliver@...>
Cc: <linux-usb-devel@...>, Alan Stern <stern@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Tuesday, August 14, 2007 - 3:02 pm

On Tue, 14 Aug 2007 20:33:22 +0200

ok, this one works :)

I suspect that there are many more broken devices out there ;)

thanks,

--
Paolo Ornati
Linux 2.6.23-rc3-gbfefa254 on x86_64
-

Previous thread: Re: + serial-turn-serial-console-suspend-a-boot-rather-than-compile-time-option.patch added to -mm tree by Pavel Machek on Tuesday, August 14, 2007 - 6:39 am. (4 messages)

Next thread: [PATCH] Fix rmmod of asus_laptop by Guillaume Chazarain on Tuesday, August 14, 2007 - 7:22 am. (1 message)