Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	Khazhy Kumykov <khazhy@google.com>,
	USB mailing list <linux-usb@vger.kernel.org>
Subject: Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization
Date: Thu, 10 Aug 2023 22:39:36 +0000	[thread overview]
Message-ID: <20230810223824.6kgawiwerwkaj6vh@synopsys.com> (raw)
In-Reply-To: <c1a18876-c505-4d4f-9f58-264199135905@rowland.harvard.edu>

On Thu, Aug 10, 2023, Alan Stern wrote:
> On Wed, Aug 09, 2023 at 09:47:14PM -0400, Alan Stern wrote:
> > On Thu, Aug 10, 2023 at 12:28:27AM +0000, Thinh Nguyen wrote:
> 
> > > Testing from Greg's usb-next branch, this series causes problem with
> > > device enumeration:
> > > 
> > > [   31.650759] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 2 using xhci_hcd
> > > [   31.663107] usb 2-1: device descriptor read/8, error -71
> > > [   31.952697] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 3 using xhci_hcd
> > > [   31.965122] usb 2-1: device descriptor read/8, error -71
> > > [   32.080991] usb usb2-port1: attempt power cycle
> > > [   34.826893] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 5 using xhci_hcd
> > > [   34.839241] usb 2-1: device descriptor read/8, error -71
> > > [   35.129908] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 6 using xhci_hcd
> > > [   35.142264] usb 2-1: device descriptor read/8, error -71
> > > [   35.257155] usb usb2-port1: attempt power cycle
> > > [   37.258922] usb 1-1: new high-speed USB device number 5 using xhci_hcd
> > > [   38.115053] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 8 using xhci_hcd
> > > [   38.127407] usb 2-1: device descriptor read/8, error -71
> > > [   38.417066] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 9 using xhci_hcd
> > > [   38.429428] usb 2-1: device descriptor read/8, error -71
> > > [   38.545315] usb usb2-port1: attempt power cycle
> > > [   43.347312] usb 2-2: new SuperSpeed Plus Gen 2x1 USB device number 11 using xhci_hcd
> > > [   43.359659] usb 2-2: device descriptor read/8, error -71
> > > [   43.649323] usb 2-2: new SuperSpeed Plus Gen 2x1 USB device number 12 using xhci_hcd
> > > [   43.661681] usb 2-2: device descriptor read/8, error -71
> > > [   43.777566] usb usb2-port2: attempt power cycle
> > > 
> > > I was testing with our host along with other common vendor hosts with a
> > > few 3.x devices. Reverting this series resolves the issue. I didn't do
> > > extensive tests for various speeds or debug it. I just want to notify
> > > this first perhaps you can figure out the issue right away.
> > > 
> > > I can look into it and provide more info later this week. If you want to
> > > print any debug, let me know and I can provide later this week.
> > 
> > Thanks for the notification.  The problem is almost certainly caused by 
> > the first patch in the series, which changes the way that the initial 
> > read/8 thing is done.  However, I have no idea what part of the patch 
> > could be causing these errors.  I would appreciate anything you can find 
> > out.
> > 
> > You should concentrate your initial investigation on the new 
> > get_bMaxPacketSize0() routine.  In particular, does the -EPROTO error 
> > value arise as the return value from the usb_control_msg() call, or does 
> > it happen because of the values stored in buf?  In the first case, how 
> > does this call differ from the one that used to be made by 
> > usb_get_device_descriptor()?  And in the second case, what are the 
> > values of rc, buf->bMaxPacketSize0, and buf->bDescriptorType?
> 
> Never mind; I found the problem.  I had forgotten that at SuperSpeed or 
> faster, the device descriptor uses a logarithmic encoding for 
> bMaxPacketSize0.
> 
> The patch below should fix things up.  Let me know how it goes.
> 

Quick test for Gen 1 and 2 devices work fine now. Highspeed also works
as expected before. I didn't test Fullspeed with various MPS, but I
don't expect any problem looking at the change.

Thanks for the fix,
Thinh