]> git.proxmox.com Git - mirror_ubuntu-jammy-kernel.git/commitdiff
usb: dwc2: host: do not delay retries for CONTROL IN transfers
authorDmitry Torokhov <dmitry.torokhov@gmail.com>
Fri, 15 Jun 2018 22:01:29 +0000 (15:01 -0700)
committerFelipe Balbi <felipe.balbi@linux.intel.com>
Tue, 17 Jul 2018 07:12:51 +0000 (10:12 +0300)
When handling split transactions we will try to delay retry after
getting a NAK from the device. This works well for BULK transfers that
can be polled for essentially forever. Unfortunately, on slower systems
at boot time, when the kernel is busy enumerating all the devices (USB
or not), we issue a bunch of control requests (reading device
descriptors, etc). If we get a NAK for the IN part of the control
request and delay retry for too long (because the system is busy), we
may confuse the device when we finally get to reissue SSPLIT/CSPLIT IN
and the device will respond with STALL. As a result we end up with
failure to get device descriptor and will fail to enumerate the device:

[    3.428801] usb 2-1.2.1: new full-speed USB device number 9 using dwc2
[    3.508576] usb 2-1.2.1: device descriptor read/8, error -32
[    3.699150] usb 2-1.2.1: device descriptor read/8, error -32
[    3.891653] usb 2-1.2.1: new full-speed USB device number 10 using dwc2
[    3.968859] usb 2-1.2.1: device descriptor read/8, error -32
...

Let's not delay retries of split CONTROL IN transfers, as this allows us
to reliably enumerate devices at boot time.

Fixes: 38d2b5fb75c1 ("usb: dwc2: host: Don't retry NAKed transactions right away")
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
drivers/usb/dwc2/hcd_intr.c

index ed7f05cf490637ba554e8be2fabea46bc118bb8c..8ce10caf3e1981fa7b5132d44a3aa760c5252d0f 100644 (file)
@@ -1231,7 +1231,10 @@ static void dwc2_hc_nak_intr(struct dwc2_hsotg *hsotg,
         * avoid interrupt storms we'll wait before retrying if we've got
         * several NAKs. If we didn't do this we'd retry directly from the
         * interrupt handler and could end up quickly getting another
-        * interrupt (another NAK), which we'd retry.
+        * interrupt (another NAK), which we'd retry. Note that we do not
+        * delay retries for IN parts of control requests, as those are expected
+        * to complete fairly quickly, and if we delay them we risk confusing
+        * the device and cause it issue STALL.
         *
         * Note that in DMA mode software only gets involved to re-send NAKed
         * transfers for split transactions, so we only need to apply this
@@ -1244,7 +1247,9 @@ static void dwc2_hc_nak_intr(struct dwc2_hsotg *hsotg,
                        qtd->error_count = 0;
                qtd->complete_split = 0;
                qtd->num_naks++;
-               qtd->qh->want_wait = qtd->num_naks >= DWC2_NAKS_BEFORE_DELAY;
+               qtd->qh->want_wait = qtd->num_naks >= DWC2_NAKS_BEFORE_DELAY &&
+                               !(chan->ep_type == USB_ENDPOINT_XFER_CONTROL &&
+                                 chan->ep_is_in);
                dwc2_halt_channel(hsotg, chan, qtd, DWC2_HC_XFER_NAK);
                goto handle_nak_done;
        }