2.6.10-rc1 OHCI usb error messages

2.6.10-rc1 OHCI usb error messages

[Colin Leroy]

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

Re: [linux-usb-devel] 2.6.10-rc1 OHCI usb error messages

[David Brownell]

On Tuesday 26 October 2004 08:28, Colin Leroy wrote:
> 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

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

[PATCH] usb: don't spit out too much errors with suspended devices

[Colin Leroy]

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

Hi, 

> 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!)

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

Signed-off-by: Colin Leroy <colin@colino.net>

--- a/drivers/usb/core/message.c	2004-10-27 09:09:34.332120419 +0200
+++ b/drivers/usb/core/message.c	2004-10-27 09:15:05.323320163 +0200
@@ -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;
--- a/drivers/usb/core/devio.c	2004-10-27 09:08:45.401856241 +0200
+++ b/drivers/usb/core/devio.c	2004-10-27 09:16:09.079846340 +0200
@@ -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)))

Re: [linux-usb-devel] 2.6.10-rc1 OHCI usb error messages

[Colin Leroy]

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

Hi, 

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

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

Re: [linux-usb-devel] 2.6.10-rc1 OHCI usb error messages

[David Brownell]

On Wednesday 27 October 2004 02:07, Colin Leroy wrote:
> On 26 Oct 2004 at 09h10, David Brownell wrote:
> 
> > 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.
> 
> Btw, there's something else wrong, because the root hub shouldn't be 
> suspended during boot, should it ?

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.


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

Re: [PATCH] usb: don't spit out too much errors with suspended devices

[David Brownell]

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

On Wednesday 27 October 2004 00:19, Colin Leroy wrote:
> On 26 Oct 2004 at 09h10, David Brownell wrote:
> 
> Hi, 
> 
> > 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!)
> 
> Here's a patch that does it. Hope it's fine.

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



[-- Attachment #2: Diff --]
[-- Type: text/x-diff, Size: 1123 bytes --]

--- 1.67/drivers/usb/core/message.c	Tue Oct 26 09:42:33 2004
+++ edited/drivers/usb/core/message.c	Wed Oct 27 16:02:03 2004
@@ -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;
--- 1.67/drivers/usb/core/devio.c	Wed Oct 20 11:45:30 2004
+++ edited/drivers/usb/core/devio.c	Wed Oct 27 16:07:40 2004
@@ -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) {

Re: [PATCH] usb: don't spit out too much errors with suspended devices

[Colin Leroy]

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

Hi, 

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

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:

> Well, I'd suggest not running that kernel until paul's signal fix gets
> in, there is a FP register corruption in there... There are other issues
> with 2.6.10 currently, so it's not really a good kernel to play with for
> now.

-- 
Colin

Re: [linux-usb-devel] 2.6.10-rc1 OHCI usb error messages

[Colin Leroy]

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

Hi, 

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

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

> > 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've never observed "lsusb" ever timing out when accessing a
> suspended USB device; the URB submissions fail right away.

Strange. Something else maybe...

> 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.)

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

Re: [linux-usb-devel] 2.6.10-rc1 OHCI usb error messages

[David Brownell]

On Thursday 28 October 2004 02:07, Colin Leroy wrote:
> On 27 Oct 2004 at 15h10, David Brownell wrote:
> 
> > 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.
> 
> That's right. Just that it didn't do so previously, so i didn't think 
> of that.

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


> > 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.)
> 
> 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:

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

Re: [linux-usb-devel] 2.6.10-rc1 OHCI usb error messages

[Alan Stern]

On Wed, 27 Oct 2004, David Brownell wrote:

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

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

Re: [linux-usb-devel] 2.6.10-rc1 OHCI usb error messages

[David Brownell]

On Thursday 28 October 2004 07:46, Alan Stern wrote:
> On Wed, 27 Oct 2004, David Brownell wrote:
> 
> > 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.
> 
> 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?

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