RE: [PATCH] usb: hub: try old enumeration scheme first for high speed devices

[Zengtao (B)]

Hi Roger:

Thank you for review

>-----Original Message-----
>From: Roger Quadros [mailto:rogerq@xxxxxx]
>Sent: Friday, August 10, 2018 6:51 PM
>To: Zengtao (B) <prime.zeng@xxxxxxxxxxxxx>; gregkh@xxxxxxxxxxxxxxxxxxx;
>stern@xxxxxxxxxxxxxxxxxxx; mathias.nyman@xxxxxxxxxxxxxxx;
>drinkcat@xxxxxxxxxxxx; felipe.balbi@xxxxxxxxxxxxxxx; drake@xxxxxxxxxxxx;
>mike.looijmans@xxxxxxxx; joe@xxxxxxxxxxx
>Cc: linux-usb@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
>Subject: Re: [PATCH] usb: hub: try old enumeration scheme first for high speed
>devices
>
>Hi,
>
>On 10/08/18 18:01, Zeng Tao wrote:
>> The new scheme is required just to support legacy low and full-speed
>> devices. For high speed devices, it will slower the enumeration speed.
>> So in this patch we try the "old" enumeration scheme first for high
>> speed devices.
>
>How slow does it get? Is it significant?
>Do we risk breaking existing HS devices that work? I don't think we can be sure
>till we run this through testing.
>

We added the new scheme because this is what the windows did , and mainly for 
legacy low and full speed devices.
Now for new windows version(8.1 and later), the second port reset has already been
removed for high speed devices for better enumeration speed.
And In this patch if use_both_schemes is true, it will fallback to new scheme if the old
scheme fails.

So I think it's reasonable to follow the windows behavior again.

>>
>> Signed-off-by: Zeng Tao <prime.zeng@xxxxxxxxxxxxx>
>> ---
>>  drivers/usb/core/hub.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index
>> 1fb2668..d265b19 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -2661,10 +2661,13 @@ static bool use_new_scheme(struct usb_device
>*udev, int retry,
>>  	int old_scheme_first_port =
>>  		port_dev->quirks & USB_PORT_QUIRK_OLD_SCHEME;
>>
>> +	int quick_enumeration = (udev->speed == USB_SPEED_HIGH);
>> +
>>  	if (udev->speed >= USB_SPEED_SUPER)
>>  		return false;
>
>how about replacing the above if with
>
>	if (udev->speed >= USB_SPEED_HIGH)
>		return false;

No, for SS device, only use old scheme, but for speed device, we can fallback to new 
scheme if the old fails.

>>
>> -	return USE_NEW_SCHEME(retry, old_scheme_first_port ||
>old_scheme_first);
>> +	return USE_NEW_SCHEME(retry, old_scheme_first_port ||
>old_scheme_first
>> +			      || quick_enumeration);
>>  }
>
>Now we no longer respect the "old_scheme_first" parameter for most of the
>devices.
>
>It should be clarified in Documentation/admin/kernel-parameters.txt that
>"old_scheme_first" is only applicable to LOW/FULL speed devices.
>

On the contrary, new scheme is only applicable for LOW/FULL speed devices? 

>>
>>  /* Is a USB 3.0 port in the Inactive or Compliance Mode state?
>>
>
>--
>Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki