2.6.10-rc1 OHCI usb error messages

Hi,

2.6.10-rc1 gives the following error messages on my iBook G4, which uses the ohci-hcd driver:

usb usb2: usbfs: USBDEVFS_CONTROL failed cmd usbmodules rqt 128 rq 6 len 18 ret -113 usb usb2: usbfs: USBDEVFS_CONTROL failed cmd usbmodules rqt 128 rq 6 len 18 ret -113 usb usb2: usbfs: USBDEVFS_CONTROL failed cmd usbmodules rqt 128 rq 6 len 18 ret -113 usb usb3: usbfs: USBDEVFS_CONTROL failed cmd usbmodules rqt 128 rq 6 len 18 ret -113 usb usb3: usbfs: USBDEVFS_CONTROL failed cmd usbmodules rqt 128 rq 6 len 18 ret -113 usb usb3: usbfs: USBDEVFS_CONTROL failed cmd usbmodules rqt 128 rq 6 len 18 ret -113 usb usb3: string descriptor 0 read error: -113
usb usb3: string descriptor 0 read error: -113
usb usb3: string descriptor 0 read error: -113
usb usb2: string descriptor 0 read error: -113
usb usb2: string descriptor 0 read error: -113
...
usb usb2: string descriptor 0 read error: -113
usb usb2: string descriptor 0 read error: -113

It worked correctly under 2.6.9. Give the error number, I'd guess the culprit is this hunk:
--- a/drivers/usb/host/ohci-hub.c 2004-10-22 14:40:57 -07:00
+++ b/drivers/usb/host/ohci-hub.c 2004-10-22 14:40:57 -07:00
...
@@ -146,10 +120,11 @@

        ohci->next_statechange = jiffies + msecs_to_jiffies (5);

succeed:
- /* it's not USB_STATE_SUSPENDED unless access to this
+ /* it's not HCD_STATE_SUSPENDED unless access to this

         * hub from the non-usb side (PCI, SOC, etc) stopped
         */
        root->dev.power.power_state = 3;
+       usb_set_device_state (root, USB_STATE_SUSPENDED);
 done:
        spin_unlock_irq (&ohci->lock);
        return status;

(Note also that this may be due to the patch to swsusp to make it work on PPC, but I had this patch with 2.6.9 too.)

Colin

ret -113

ret -113

ret -113

ret -113

ret -113

ret -113

What's wrong there is emitting voluminous diagnostics for something that's not an error ... the root hub is suspended, and as with any suspended device, you can't talk to it.

The descriptor read logic can skip retries in that case, and usbfs should refuse up front to talk to suspended devices. (Silently!)

- Dave

On 26 Oct 2004 at 09h10, David Brownell wrote:

Hi,

Here's a patch that does it. Hope it's fine.

@@ -703,6 +705,8 @@
 	int err;
 	unsigned int u, idx;
 
+	if (dev->state == USB_STATE_SUSPENDED)
+		return -EHOSTUNREACH;
 	if (size <= 0 || !buf || !index)
 		return -EINVAL;
 	buf[0] = 0;

@@ -541,6 +541,8 @@
 	unsigned char *tbuf;
 	int i, j, ret;
 
+	if (dev->state == USB_STATE_SUSPENDED)
+		return -EHOSTUNREACH;
 	if (copy_from_user(&ctrl, arg, sizeof(ctrl)))
 		return -EFAULT;
 	if ((ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.wIndex)))

The "message.c" part is fine, but the usbfs/devio change only tests one of several paths for device access. (My first whack at this only covered a different one!) Can you verify this one instead?

- Dave

@@ -704,6 +704,8 @@
 	int err;
 	unsigned int u, idx;
 
+	if (dev->state == USB_STATE_SUSPENDED)
+		return -EHOSTUNREACH;
 	if (size <= 0 || !buf || !index)
 		return -EINVAL;
 	buf[0] = 0;

@@ -411,6 +411,8 @@
 
 static int checkintf(struct dev_state *ps, unsigned int ifnum)
 {
+	if (ps->dev->state != USB_STATE_CONFIGURED)
+		return -EHOSTUNREACH;
 	if (ifnum >= 8*sizeof(ps->ifclaimed))
 		return -EINVAL;
 	if (test_bit(ifnum, &ps->ifclaimed))

@@ -450,6 +452,8 @@
 {
 	int ret = 0;
 
+	if (ps->dev->state != USB_STATE_CONFIGURED)
+		return -EHOSTUNREACH;
 	if (USB_TYPE_VENDOR == (USB_TYPE_MASK & requesttype))
 		return 0;
 

@@ -1131,7 +1135,7 @@
 	}
 
 	if (ps->dev->state != USB_STATE_CONFIGURED)
-		retval = -ENODEV;
+		retval = -EHOSTUNREACH;
 	else if (!(intf = usb_ifnum_to_if (ps->dev, ctrl.ifno)))
                retval = -EINVAL;
 	else switch (ctrl.ioctl_code) {

On 27 Oct 2004 at 16h10, David Brownell wrote:

Hi,

I'll do; I'll get back to you next week probably, because I'll be busy this week-end, and I'm avoiding 2.6.10-rc1 until ppc fixes go in: Citing Benjamin Herrenschmidt:

Colin

On 26 Oct 2004 at 09h10, David Brownell wrote:

Hi,

Btw, there's something else wrong, because the root hub shouldn't be suspended during boot, should it ? At least, it's not on 2.6.9. Also, lsusb -v fails with long timeouts due to that on 2.6.10-rc1, not on 2.6.9.

I really think there's something wrong with the drivers/usb/host/ohci-hub.c patch I posted in the first message...

Colin

You're not reporting that it fails to activate when there's an active device connected; and you obviously enabled CONFIG_PM, which Kconfig says means that

... parts of your computer are shut off or put into a power conserving "sleep" mode if they are not being used.

So: since it's not being actively used then, why shouldn't the root hub (or any other device) be suspended? During boot, or at any other time. So long as it works when you plug in a USB device, it looks to me like everything is behaving quite reasonably.

I've never observed "lsusb" ever timing out when accessing a suspended USB device; the URB submissions fail right away. So if something's timing out, it's for some other reason. (Such as bugs in "lsusb"; the "usbutils" package is overdue for a new release, it's changed a lot since the 0.11 tarball that's widely available.)

- Dave

On 27 Oct 2004 at 15h10, David Brownell wrote:

Hi,

That's right. Just that it didn't do so previously, so i didn't think of that.

Strange. Something else maybe...

Yes, btw, I once sent a patch about lsusb endianness problems, and didn't hear anything back about it. I ended up sending the patch to the gentoo guys so that at least my distro is fixed:

http://bugs.gentoo.org/show_bug.cgi?id=43565

Colin

You won't be alone either ... so I'll be glad to get rid of those pointless usbcore "non-error" messages!

Thomas put "usbutils" into CVS at linux-usb.sf.net and that version has the Debian patches to make it use the system's "libusb" instead of its own older snapshot. Plus bugfixes, and printing more descriptors.

It looks like libusb-0.1.8 still expects the kernel to return a little-endian device descriptor rather than its private byteswapped version.

- Dave

The root hub _is_ actively being used during initial probing and enumeration, even though no devices may be plugged into it. Is it guaranteed that the root hub isn't suspended until after usb_register_root_hub returns?

Alan Stern

It's never going to be suspended until after the hub driver has set everything up -- that's the essential constraint. Since that routine is called in a preemptible context, some systems could end up preempting that call, so it'd suspend before that call manages to return. That'd be OK, since everything's set up.

- Dave