usb: xhci: change enumeration scheme to 'new scheme' by default

Change the default enumeration scheme for xhci attached non-SuperSpeed
devices from:

   Reset
   SetAddress [xhci address-device BSR = 0]
   GetDescriptor(8)
   GetDescriptor(18)

...to:

   Reset
   [xhci address-device BSR = 1]
   GetDescriptor(64)
   Reset
   SetAddress [xhci address-device BSR = 0]
   GetDescriptor(18)

...as some devices misbehave when encountering a SetAddress command
prior to GetDescriptor.  There are known legacy devices that require
this scheme, but testing has found at least one USB3 device that fails
enumeration when presented with this ordering.  For now, follow the ehci
case and enable 'new scheme' by default for non-SuperSpeed devices.

To support this enumeration scheme on xhci the AddressDevice operation
needs to be performed twice.  The first instance of the command enables
the HC's device and slot context info for the device, but omits sending
the device a SetAddress command (BSR == block set address request).
Then, after GetDescriptor completes, follow up with the full
AddressDevice+SetAddress operation.

As mentioned before, this ordering of events with USB3 devices causes an
extra state transition to be exposed to xhci.  Previously USB3 devices
would transition directly from 'enabled' to 'addressed' and never need
to underrun responses to 'get descriptor'. We do see the 64-byte
descriptor fetch the correct data, but the following 18-byte descriptor
read after the reset gets:

bLength            = 0
bDescriptorType    = 0
bcdUSB             = 0
bDeviceClass       = 0
bDeviceSubClass    = 0
bDeviceProtocol    = 0
bMaxPacketSize0    = 9

instead of:

bLength            = 12
bDescriptorType    = 1
bcdUSB             = 300
bDeviceClass       = 0
bDeviceSubClass    = 0
bDeviceProtocol    = 0
bMaxPacketSize0    = 9

which results in the discovery process looping until falling back to
'old scheme' enumeration.

Acked-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: David Moore <david.moore@gmail.com>
Suggested-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 32e1035d4f59d68437b1e529d483198cc4db2018..6a11eff74d3c62f76faf382074019de36bf2b09f 100644 (file)
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2498,6 +2498,21 @@ static unsigned hub_is_wusb(struct usb_hub *hub)
 #define HUB_LONG_RESET_TIME    200
 #define HUB_RESET_TIMEOUT      800
 
+/*
+ * "New scheme" enumeration causes an extra state transition to be
+ * exposed to an xhci host and causes USB3 devices to receive control
+ * commands in the default state.  This has been seen to cause
+ * enumeration failures, so disable this enumeration scheme for USB3
+ * devices.
+ */
+static bool use_new_scheme(struct usb_device *udev, int retry)
+{
+       if (udev->speed == USB_SPEED_SUPER)
+               return false;
+
+       return USE_NEW_SCHEME(retry);
+}
+
 static int hub_port_reset(struct usb_hub *hub, int port1,
                        struct usb_device *udev, unsigned int delay, bool warm);
 
@@ -3956,6 +3971,20 @@ static void hub_set_initial_usb2_lpm_policy(struct usb_device *udev)
        }
 }
 
+static int hub_enable_device(struct usb_device *udev)
+{
+       struct usb_hcd *hcd = bus_to_hcd(udev->bus);
+
+       if (!hcd->driver->enable_device)
+               return 0;
+       if (udev->state == USB_STATE_ADDRESS)
+               return 0;
+       if (udev->state != USB_STATE_DEFAULT)
+               return -EINVAL;
+
+       return hcd->driver->enable_device(hcd, udev);
+}
+
 /* Reset device, (re)assign address, get device descriptor.
  * Device connection must be stable, no more debouncing needed.
  * Returns device in USB_STATE_ADDRESS, except on error.
@@ -4068,7 +4097,7 @@ hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port1,
         * this area, and this is how Linux has done it for ages.
         * Change it cautiously.
         *
-        * NOTE:  If USE_NEW_SCHEME() is true we will start by issuing
+        * NOTE:  If use_new_scheme() is true we will start by issuing
         * a 64-byte GET_DESCRIPTOR request.  This is what Windows does,
         * so it may help with some non-standards-compliant devices.
         * Otherwise we start with SET_ADDRESS and then try to read the
@@ -4076,10 +4105,17 @@ hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port1,
         * value.
         */
        for (i = 0; i < GET_DESCRIPTOR_TRIES; (++i, msleep(100))) {
-               if (USE_NEW_SCHEME(retry_counter) && !(hcd->driver->flags & HCD_USB3)) {
+               bool did_new_scheme = false;
+
+               if (use_new_scheme(udev, retry_counter)) {
                        struct usb_device_descriptor *buf;
                        int r = 0;
 
+                       did_new_scheme = true;
+                       retval = hub_enable_device(udev);
+                       if (retval < 0)
+                               goto fail;
+
 #define GET_DESCRIPTOR_BUFSIZE 64
                        buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
                        if (!buf) {
@@ -4168,7 +4204,11 @@ hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port1,
                         *  - read ep0 maxpacket even for high and low speed,
                         */
                        msleep(10);
-                       if (USE_NEW_SCHEME(retry_counter) && !(hcd->driver->flags & HCD_USB3))
+                       /* use_new_scheme() checks the speed which may have
+                        * changed since the initial look so we cache the result
+                        * in did_new_scheme
+                        */
+                       if (did_new_scheme)
                                break;
                }
 

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index b8dffd59eb256e52786328e5e4f1919846a80d9c..4221dee924b57ffd61cd0cd0d1a174345024ec36 100644 (file)
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -331,6 +331,7 @@ static const struct hc_driver xhci_pci_hc_driver = {
        .check_bandwidth =      xhci_check_bandwidth,
        .reset_bandwidth =      xhci_reset_bandwidth,
        .address_device =       xhci_address_device,
+       .enable_device =        xhci_enable_device,
        .update_hub_device =    xhci_update_hub_device,
        .reset_device =         xhci_discover_or_reset_device,
 

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 9d29aa1b1bc8f8301b8fb7c7b0f6bc3b18cd27c8..8abda5c73ca1e846aaff80a2e6096dff73d04853 100644 (file)
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -69,6 +69,7 @@ static const struct hc_driver xhci_plat_xhci_driver = {
        .check_bandwidth =      xhci_check_bandwidth,
        .reset_bandwidth =      xhci_reset_bandwidth,
        .address_device =       xhci_address_device,
+       .enable_device =        xhci_enable_device,
        .update_hub_device =    xhci_update_hub_device,
        .reset_device =         xhci_discover_or_reset_device,
 

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index fe9208a5d10355f5f8c30b20084abcd92eb25b00..d26cd9474aa604494448265d71d28f66da1e0ad9 100644 (file)
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -4004,12 +4004,12 @@ int xhci_queue_slot_control(struct xhci_hcd *xhci, u32 trb_type, u32 slot_id)
 
 /* Queue an address device command TRB */
 int xhci_queue_address_device(struct xhci_hcd *xhci, dma_addr_t in_ctx_ptr,
-               u32 slot_id)
+                             u32 slot_id, enum xhci_setup_dev setup)
 {
        return queue_command(xhci, lower_32_bits(in_ctx_ptr),
                        upper_32_bits(in_ctx_ptr), 0,
-                       TRB_TYPE(TRB_ADDR_DEV) | SLOT_ID_FOR_TRB(slot_id),
-                       false);
+                       TRB_TYPE(TRB_ADDR_DEV) | SLOT_ID_FOR_TRB(slot_id)
+                       | (setup == SETUP_CONTEXT_ONLY ? TRB_BSR : 0), false);
 }
 
 int xhci_queue_vendor_command(struct xhci_hcd *xhci,

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 7fe6f664054ff3b574a025b1558f8473521c11b0..6598f7ee7938ff94959a2b0444e99c0f7a491b7e 100644 (file)
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3709,12 +3709,13 @@ disable_slot:
 }
 
 /*
- * Issue an Address Device command (which will issue a SetAddress request to
- * the device).
+ * Issue an Address Device command and optionally send a corresponding
+ * SetAddress request to the device.
  * We should be protected by the usb_address0_mutex in khubd's hub_port_init, so
  * we should only issue and wait on one address command at the same time.
  */
-int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev)
+static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
+                            enum xhci_setup_dev setup)
 {
        unsigned long flags;
        int timeleft;
@@ -3773,7 +3774,7 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev)
        spin_lock_irqsave(&xhci->lock, flags);
        cmd_trb = xhci_find_next_enqueue(xhci->cmd_ring);
        ret = xhci_queue_address_device(xhci, virt_dev->in_ctx->dma,
-                                       udev->slot_id);
+                                       udev->slot_id, setup);
        if (ret) {
                spin_unlock_irqrestore(&xhci->lock, flags);
                xhci_dbg_trace(xhci, trace_xhci_dbg_address,
@@ -3868,6 +3869,16 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev)
        return 0;
 }
 
+int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev)
+{
+       return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ADDRESS);
+}
+
+int xhci_enable_device(struct usb_hcd *hcd, struct usb_device *udev)
+{
+       return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY);
+}
+
 /*
  * Transfer the port index into real index in the HW port status
  * registers. Caculate offset between the port's PORTSC register

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 7807f621a7133ea9ffd0c80cf9a5ab3c3e79e361..24344aab210703b7e9a2cc1a06f9d90c9f935f1c 100644 (file)
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1108,6 +1108,14 @@ struct xhci_event_cmd {
 };
 
 /* flags bitmasks */
+
+/* Address device - disable SetAddress */
+#define TRB_BSR                (1<<9)
+enum xhci_setup_dev {
+       SETUP_CONTEXT_ONLY,
+       SETUP_CONTEXT_ADDRESS,
+};
+
 /* bits 16:23 are the virtual function ID */
 /* bits 24:31 are the slot ID */
 #define TRB_TO_SLOT_ID(p)      (((p) & (0xff<<24)) >> 24)
@@ -1760,6 +1768,7 @@ int xhci_free_streams(struct usb_hcd *hcd, struct usb_device *udev,
                struct usb_host_endpoint **eps, unsigned int num_eps,
                gfp_t mem_flags);
 int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev);
+int xhci_enable_device(struct usb_hcd *hcd, struct usb_device *udev);
 int xhci_update_device(struct usb_hcd *hcd, struct usb_device *udev);
 int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
                                struct usb_device *udev, int enable);
@@ -1783,7 +1792,7 @@ int xhci_is_vendor_info_code(struct xhci_hcd *xhci, unsigned int trb_comp_code);
 void xhci_ring_cmd_db(struct xhci_hcd *xhci);
 int xhci_queue_slot_control(struct xhci_hcd *xhci, u32 trb_type, u32 slot_id);
 int xhci_queue_address_device(struct xhci_hcd *xhci, dma_addr_t in_ctx_ptr,
-               u32 slot_id);
+               u32 slot_id, enum xhci_setup_dev);
 int xhci_queue_vendor_command(struct xhci_hcd *xhci,
                u32 field1, u32 field2, u32 field3, u32 field4);
 int xhci_queue_stop_endpoint(struct xhci_hcd *xhci, int slot_id,

diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 758ce80d085ff9b814b3b6578fc97b0bbc379a8f..efe8d8a7c7addebb55cc152afffa80c060d4725d 100644 (file)
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -353,6 +353,8 @@ struct hc_driver {
        void    (*reset_bandwidth)(struct usb_hcd *, struct usb_device *);
                /* Returns the hardware-chosen device address */
        int     (*address_device)(struct usb_hcd *, struct usb_device *udev);
+               /* prepares the hardware to send commands to the device */
+       int     (*enable_device)(struct usb_hcd *, struct usb_device *udev);
                /* Notifies the HCD after a hub descriptor is fetched.
                 * Will block.
                 */