Re: USB disk speed regression WD Elements - with bisect result 22547c4cc4fe20698a6a85a55b8788859134b8e4

[Alan Stern]

On Fri, 28 Jul 2017, Zdenek Kabelac wrote:

> Dne 28.7.2017 v 20:33 Alan Stern napsal(a):
> > On Thu, 27 Jul 2017, Zdenek Kabelac wrote:
> > 
> >>> Zdenek, you check this explanation by commenting out these last two
> >>> lines at the end of hub_port_connect() in drivers/usb/core/hub.c:
> >>>
> >>> 	if (hcd->driver->relinquish_port && !hub->hdev->parent)
> >>> 		hcd->driver->relinquish_port(hcd, port1);
> >>>
> >>> That should prevent the connection from being handed over to the UHCI
> >>> companion, allowing the device to operate at high speed.
> >>>
> >>
> >> Hi
> >>
> >> Yep - seems this helped - I've dropped revert and commented those 2 lines
> >> and I've used the very same kernel - and speed was all good:
> >>
> >> usb 2-2: new high-speed USB device number 2 using ehci-pci
> >> usb 2-2: new high-speed USB device number 3 using ehci-pci
> >> usb 2-2: New USB device found, idVendor=1058, idProduct=10a8
> >> usb 2-2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> >>
> >>
> >> So while I'm not sure if this is final fix for USB - this solution surely
> >> solved my WD Element disk attachment issue.
> > 
> > Great!  Here's a real patch to test; I think this will be acceptable if
> > it fixes your problem.
> > 
> > Alan Stern
> > 
> > 
> > 
> > Index: usb-4.x/drivers/usb/core/hub.c
> > ===================================================================
> > --- usb-4.x.orig/drivers/usb/core/hub.c
> > +++ usb-4.x/drivers/usb/core/hub.c
> > @@ -4934,9 +4934,10 @@ loop:
> >   
> >   done:
> >   	hub_port_disable(hub, port1, 1);
> > -	if (hcd->driver->relinquish_port && !hub->hdev->parent)
> > -		hcd->driver->relinquish_port(hcd, port1);
> > -
> > +	if (hcd->driver->relinquish_port && !hub->hdev->parent) {
> > +		if (status != -ENOTCONN && status != -ENODEV)
> > +			hcd->driver->relinquish_port(hcd, port1);
> > +	}
> >   }
> >   
> >   /* Handle physical or logical connection change events.
> > 
> 
> 
> Hi
> 
> Yep - still good speed - however compilation reported this Warning:
> 
> drivers/usb/core/hub.c:4933:37: warning: âstatusâ may be used uninitialized in 
> this function [-Wmaybe-uninitialized]
>     if (status != -ENOTCONN && status != -ENODEV)
> drivers/usb/core/hub.c:4728:6: note: âstatusâ was declared here
>    int status, i;
> 
> 
> So I've just made assignment of 'status = 0' at declaration place -
> but then you have some other unneeded assigns of =0 left around...
> so possibly worth some minor cleanup...

The initialization actually should be -ENODEV.  I have changed the 
patch and will submit it after the weekend.

Alan Stern