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

[Zengtao (B)]

Hi Alan:

Thanks for your reply.

>-----Original Message-----
>From: Alan Stern [mailto:stern@xxxxxxxxxxxxxxxxxxx]
>Sent: Friday, August 17, 2018 10:19 PM
>To: Zengtao (B) <prime.zeng@xxxxxxxxxxxxx>
>Cc: gregkh@xxxxxxxxxxxxxxxxxxx; mathias.nyman@xxxxxxxxxxxxxxx;
>drinkcat@xxxxxxxxxxxx; felipe.balbi@xxxxxxxxxxxxxxx;
>drake@xxxxxxxxxxxx; mike.looijmans@xxxxxxxx; joe@xxxxxxxxxxx;
>Jonathan Corbet <corbet@xxxxxxx>; Thomas Gleixner <tglx@xxxxxxxxxxxxx>;
>Ingo Molnar <mingo@xxxxxxxxxx>; Rafael J. Wysocki
><rafael.j.wysocki@xxxxxxxxx>; Marc Zyngier <marc.zyngier@xxxxxxx>;
>Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>; Thymo van Beers
><thymovanbeers@xxxxxxxxx>; Frederic Weisbecker <frederic@xxxxxxxxxx>;
>Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>; David Rientjes
><rientjes@xxxxxxxxxx>; linux-doc@xxxxxxxxxxxxxxx;
>linux-kernel@xxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx
>Subject: Re: [PATCH v2] usb: hub: try old enumeration scheme first for high
>speed devices
>
>On Fri, 17 Aug 2018, 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, and this is what Windows does since Windows 8.
>>
>> Signed-off-by: Zeng Tao <prime.zeng@xxxxxxxxxxxxx>
>> ---
>> Changes in v2:
>> 1. As suggested by Alan, mention in the commit log that this change is
>> follow what the Window does.
>> 2. As suggested by Roger, update the kernel-parameter description.
>> ---
>>  Documentation/admin-guide/kernel-parameters.txt | 3 ++-
>>  drivers/usb/core/hub.c                          | 5 ++++-
>>  2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt
>> b/Documentation/admin-guide/kernel-parameters.txt
>> index 533ff5c..95db23c 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -4453,7 +4453,8 @@
>>
>>  	usbcore.old_scheme_first=
>>  			[USB] Start with the old device initialization
>> -			scheme (default 0 = off).
>> +			scheme,  applies only to low and full-speed devices
>> +			 (default 0 = off).
>>
>>  	usbcore.usbfs_memory_mb=
>>  			[USB] Memory limit (in MB) for buffers allocated by 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);
>
Okey, will fix in v3

>Minor style point: the preceding blank line should be eliminated.
>
>> +
>>  	if (udev->speed >= USB_SPEED_SUPER)
>>  		return false;
>>
>> -	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);
>>  }
>>
>>  /* Is a USB 3.0 port in the Inactive or Compliance Mode state?
>
>Aside from that:
>
>Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

Okay , tag will be applied in v3.