Re: USB3.0 Port Speed Downgrade after port reset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 30, 2010 at 06:28:05PM -0600, Fajun Chen wrote:
> Hi Sarah,
> 
> I've traced the speed downgrade issue to port reset and the interpretation
> of port status in USB hub driver code.  The port status was 0x903 after port
> reset and the existing code (hub_port_wait_reset function) interpreted this
> as full speed, the USB3.0 port was disabled due to the speed change after
> reset. USB2.0 spec, USB3.0 spec and xHCI spec all have different definitions
> of port status, so this is a little ambiguous. However, the same hub code
> interpret port status 0x903 as super speed when the drive was first powered
> up.  The following patch adds super speed mapping from port status, no more
> speed downgrade issue with the fix and I can get USB3.0 performance in my
> system.

Thanks for catching this.  You're correct that hub.c is missing a case
for SuperSpeed after a port reset.  But I'm not convinced this is quite
the right fix.

> -bash-3.00$ diff -urNb ../../../../linux-2.6.34-stock/drivers/usb/core/hub.c
> hub.c
> --- ../../../../linux-2.6.34-stock/drivers/usb/core/hub.c       2010-06-16
> 13:20:39.000000000 -0600
> +++ hub.c       2010-06-30 18:08:09.000000000 -0600
> @@ -1972,6 +1972,8 @@
>                     (portstatus & USB_PORT_STAT_ENABLE)) {
>                         if (hub_is_wusb(hub))
>                                 udev->speed = USB_SPEED_WIRELESS;
> +            else if (portstatus & USB_PORT_FEAT_SUPERSPEED)
> +                udev->speed = USB_SPEED_SUPER;
>                         else if (portstatus & USB_PORT_STAT_HIGH_SPEED)
>                                 udev->speed = USB_SPEED_HIGH;
>                         else if (portstatus & USB_PORT_STAT_LOW_SPEED)

Why are you checking USB_PORT_FEAT_SUPERSPEED, instead of
USB_PORT_STAT_SUPER_SPEED?  The xhci_port_speed() function will return
(1 << USB_PORT_FEAT_SUPERSPEED) for the portstatus of a SuperSpeed port,
so I'm not sure why this patch worked.

#define USB_PORT_FEAT_SUPERSPEED       11
#define USB_PORT_STAT_SUPER_SPEED       0x8000

I think it only worked because USB_PORT_STAT_INDICATOR,
USB_PORT_STAT_ENABLE, and USB_PORT_STAT_CONNECTION was set, which
matched the #define of USB_PORT_FEAT_SUPERSPEED to decimal 11, or
0x1011.  I think if you had reset a configured high speed device with
this patch, it would have been detected as a SuperSpeed device.  I think
the correct patch is to test for
(portstatus & USB_PORT_STAT_SUPER_SPEED).

The patch also won't apply against 2.6.35, because
USB_PORT_FEAT_SUPERSPEED was removed by commit 288ead4.  So I think it's
better to use USB_PORT_STAT_SUPER_SPEED for fixes to both kernels.

Can you try the attached patch and see if it fixes your issue?

Sarah Sharp

> On Tue, Jun 29, 2010 at 4:48 PM, Fajun Chen <fajun.chen@xxxxxxxxxxx> wrote:
> 
> > Hi Sarah,
> >
> > I simplified the procedure once more to repeat the speed downgrade
> > problem.  No command timeout is required, a simple port reset can downgrade
> > the port speed from super speed to high speed. Please check out the logs
> > attached.  Can you tell if this is a host controller/driver issue or USB3.0
> > bridge issue? Note that power cycling the bridge can not recover super
> > speed, so I suspect this is a host issue.
> >
> > Here're some highlights of the log:
> > dmesg.log.0, Line 468:
> > Jun 29 15:22:20 2 user.debug kernel: [1277846540 885125] xhci_hcd
> > 0000:03:00.0: set port reset, actual port 0 status  = 0x1311
> >
> > dmesg.log.0 Line 726-727:
> > Jun 29 15:22:21 2 user.info kernel: [1277846541 961874] xhci_hcd
> > 0000:03:00.0: Can't reset device (slot ID 1) in         enabled/disabled
> > state
> > Jun 29 15:22:21 2 user.info kernel: [1277846541 961933] xhci_hcd
> > 0000:03:00.0: Not freeing device rings.
> > // Reset device at wrong state.  Can we prevent it from happening?
> >
> > dmesg.log.0, Line 730:
> > Jun 29 15:22:21 2 user.info kernel: [1277846541 962094] usb 9-3: new high
> > speed USB device using xhci_hcd and add        ress 0
> > // Based on the trace in another test log (not attached): Get Port Status
> > control request returned status 0x503. So the hub did detect high speed
> > device.
> > // Question: which information should be used for port status? Get Port
> > Status control message or Port Status register?
> >
> > Any insights from the logs attached? Really appreciate your help!
> >
> > Thanks,
> > Fajun
> >
> >
> >
> >
>From aa3e62c61a43ea518cd0112958ac91a1e2c059ba Mon Sep 17 00:00:00 2001
From: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
Date: Tue, 6 Jul 2010 15:34:33 +0200
Subject: [PATCH] USB: Fix USB3.0 Port Speed Downgrade after port reset

Without this fix, a USB 3.0 port is downgraded to full speed after a port
reset of a configured device.  The USB 3.0 terminations will be disabled
permanently, and USB 3.0 devices will always enumerate as full speed
devices, until the host controller is unplugged (if it is an ExpressCard)
or the computer is rebooted.

Fajun Chen traced this traced the speed downgrade issue to the port reset
and the interpretation of port status in USB hub driver code.  The hub
code was not testing for the port being a SuperSpeed port, and it fell
through to the else case of Full Speed.

The following patch adds SuperSpeed mapping from the port status, and
fixes the speed downgrade issue.

Reported-by: Fajun Chen <fajun.chen@xxxxxxxxxxx>
Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
---
 drivers/usb/core/hub.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 03b4f91..a25e000 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1972,6 +1972,8 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1,
 		    (portstatus & USB_PORT_STAT_ENABLE)) {
 			if (hub_is_wusb(hub))
 				udev->speed = USB_SPEED_WIRELESS;
+			else if (portstatus & USB_PORT_STAT_SUPER_SPEED)
+				udev->speed = USB_SPEED_SUPER;
 			else if (portstatus & USB_PORT_STAT_HIGH_SPEED)
 				udev->speed = USB_SPEED_HIGH;
 			else if (portstatus & USB_PORT_STAT_LOW_SPEED)
-- 
1.6.3.3


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux