On 14/08/18 10:33, Zengtao (B) wrote: > 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. You are correct. > >>> >>> - 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? No. What I meant is that the "old_scheme_first" module parameter is now only applicable for LOW/FULL speed devices. As HIGH_SPEED devices will always use old scheme first after this patch. cheers, -roger -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
