Re: [PATCH 1/3] usb: dwc2: host: do not delay retries for CONTROL IN transfers

[Doug Anderson]

Hi,

On Fri, Jun 15, 2018 at 3:01 PM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> 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")

nit: I think there's supposed to be a ":" after "Fixes"

> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> ---
>  drivers/usb/dwc2/hcd_intr.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

This seems like a sane fix to me.

Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>