From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1034669AbeEXWFU (ORCPT ); Thu, 24 May 2018 18:05:20 -0400 Received: from mail-pg0-f67.google.com ([74.125.83.67]:38108 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966031AbeEXWFS (ORCPT ); Thu, 24 May 2018 18:05:18 -0400 X-Google-Smtp-Source: AB8JxZp0ZNmh+tNmfac9S+kNSAcqhN8E/txwO1kG3Fz5s25JSFYqwgUs6Fo8hzfBnfFondZuthXfiAnMrGLp8agDyuU= MIME-Version: 1.0 In-Reply-To: <20180524162157.GA26662@kroah.com> References: <20180523021656.122455-1-drinkcat@chromium.org> <20180523163936.GE12456@kroah.com> <20180524162157.GA26662@kroah.com> From: Nicolas Boichat Date: Fri, 25 May 2018 06:05:16 +0800 Message-ID: Subject: Re: [PATCH] usb: hub: Per-port setting to use old enumeration scheme To: Greg Kroah-Hartman Cc: Alan Stern , linux-usb@vger.kernel.org, Mathias Nyman , Felipe Balbi , Eugene Korenevsky , Peter Chen , Daniel Drake , Joe Perches , Johan Hovold , Richard Leitner , lkml , Guenter Roeck Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 25, 2018 at 12:21 AM, Greg Kroah-Hartman wrote: > On Thu, May 24, 2018 at 07:42:00AM +0800, Nicolas Boichat wrote: >> On Thu, May 24, 2018 at 12:39 AM, Greg Kroah-Hartman >> wrote: >> > On Wed, May 23, 2018 at 10:03:55AM -0400, Alan Stern wrote: >> >> On Wed, 23 May 2018, Nicolas Boichat wrote: >> >> >> >> > The "old" enumeration scheme is considerably faster (it takes >> >> > ~294ms instead of ~439ms to get the descriptor). >> >> > >> >> > It is currently only possible to use the old scheme globally >> >> > (/sys/module/usbcore/parameters/old_scheme_first), which is not >> >> > desirable as the new scheme was introduced to increase compatibility >> >> > with more devices. >> >> > >> >> > However, in our case, we care about time-to-active for a specific >> >> > USB device (which we make the firmware for), on a specific port >> >> > (that is pogo-pin based: not a standard USB port). This new >> >> > sysfs option makes it possible to use the old scheme on a single >> >> > port only. >> >> > >> >> > Signed-off-by: Nicolas Boichat >> >> > --- >> >> > >> >> > There are other "quirks" that we could add to reduce further >> >> > enumeration time (e.g. reduce USB debounce time, reduce TRSTRCY >> >> > to 10ms instead of 50ms as used currently), but the logic is quite >> >> > similar, so it'd be good to have this reviewed first. >> >> >> >> I'm not opposed to the idea in principle, although I don't like your >> >> implementation because it breaks the original old_scheme_first >> >> parameter. >> >> I don't think it breaks the original parameter? I mean, >> /sys/module/usbcore/parameters/old_scheme_first is still a global >> default, while bit 0 of /sys/bus/usb/devices/x/y/z/quirks becomes a >> port-specific override. >> >> >> Let's see what some other people think. >> >> >> >> Yours is a rather special case, because you know exactly what device >> >> will be attached to a specific port. Still, I can see that sort of >> >> thing happening in constrained and special-purpose settings. >> >> >> >> How do you arrange to set the new quirk before the device is >> >> discovered? >> > >> > Yeah, this last question is what I had when looking at this. Or does it >> > not matter at first boot and only matters for wake-up? >> >> It does not matter on boot, we have plenty of time to enumerate the >> device. We use USB (auto-)suspend and remote wake, so no >> re-enumeration there either. It only matters on unplug/replug where >> the device needs to be re-enumerated. > > How does this device get unplugged/replugged if it is connected directly > to the device? It is external. Essentially, this is a tablet with a detachable keyboard/touchpad. The interface between tablet and base is USB, over pogo pins. The port is non-standard (pogo, not normal USB), and we fully control the firmware on the base side as well, which allows us to take shortcuts like this: we know exactly what device will be connected on that port. >> Somewhere in an init script, we would do this (we know in advance that >> usb1 port2 is the bus/port where we have our pogo-pin USB interface, >> so we can hard-code the path): >> echo 1 > /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port2/quirks >> >> We could try to add ACPI support (just like connect_type), but we >> don't strictly need it for our application. > > Isn't there an "internal" ACPI flag for USB ports, or is that > what connect_type is? Why wouldn't that work here instead? What we may need here is something like "external but non-standard, so we can ignore compatibility constraints when enumerating"... Thanks, From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: usb: hub: Per-port setting to use old enumeration scheme From: Nicolas Boichat Message-Id: Date: Fri, 25 May 2018 06:05:16 +0800 To: Greg Kroah-Hartman Cc: Alan Stern , linux-usb@vger.kernel.org, Mathias Nyman , Felipe Balbi , Eugene Korenevsky , Peter Chen , Daniel Drake , Joe Perches , Johan Hovold , Richard Leitner , lkml , Guenter Roeck List-ID: T24gRnJpLCBNYXkgMjUsIDIwMTggYXQgMTI6MjEgQU0sIEdyZWcgS3JvYWgtSGFydG1hbgo8Z3Jl Z2toQGxpbnV4Zm91bmRhdGlvbi5vcmc+IHdyb3RlOgo+IE9uIFRodSwgTWF5IDI0LCAyMDE4IGF0 IDA3OjQyOjAwQU0gKzA4MDAsIE5pY29sYXMgQm9pY2hhdCB3cm90ZToKPj4gT24gVGh1LCBNYXkg MjQsIDIwMTggYXQgMTI6MzkgQU0sIEdyZWcgS3JvYWgtSGFydG1hbgo+PiA8Z3JlZ2toQGxpbnV4 Zm91bmRhdGlvbi5vcmc+IHdyb3RlOgo+PiA+IE9uIFdlZCwgTWF5IDIzLCAyMDE4IGF0IDEwOjAz OjU1QU0gLTA0MDAsIEFsYW4gU3Rlcm4gd3JvdGU6Cj4+ID4+IE9uIFdlZCwgMjMgTWF5IDIwMTgs IE5pY29sYXMgQm9pY2hhdCB3cm90ZToKPj4gPj4KPj4gPj4gPiBUaGUgIm9sZCIgZW51bWVyYXRp b24gc2NoZW1lIGlzIGNvbnNpZGVyYWJseSBmYXN0ZXIgKGl0IHRha2VzCj4+ID4+ID4gfjI5NG1z IGluc3RlYWQgb2YgfjQzOW1zIHRvIGdldCB0aGUgZGVzY3JpcHRvcikuCj4+ID4+ID4KPj4gPj4g PiBJdCBpcyBjdXJyZW50bHkgb25seSBwb3NzaWJsZSB0byB1c2UgdGhlIG9sZCBzY2hlbWUgZ2xv YmFsbHkKPj4gPj4gPiAoL3N5cy9tb2R1bGUvdXNiY29yZS9wYXJhbWV0ZXJzL29sZF9zY2hlbWVf Zmlyc3QpLCB3aGljaCBpcyBub3QKPj4gPj4gPiBkZXNpcmFibGUgYXMgdGhlIG5ldyBzY2hlbWUg d2FzIGludHJvZHVjZWQgdG8gaW5jcmVhc2UgY29tcGF0aWJpbGl0eQo+PiA+PiA+IHdpdGggbW9y ZSBkZXZpY2VzLgo+PiA+PiA+Cj4+ID4+ID4gSG93ZXZlciwgaW4gb3VyIGNhc2UsIHdlIGNhcmUg YWJvdXQgdGltZS10by1hY3RpdmUgZm9yIGEgc3BlY2lmaWMKPj4gPj4gPiBVU0IgZGV2aWNlICh3 aGljaCB3ZSBtYWtlIHRoZSBmaXJtd2FyZSBmb3IpLCBvbiBhIHNwZWNpZmljIHBvcnQKPj4gPj4g PiAodGhhdCBpcyBwb2dvLXBpbiBiYXNlZDogbm90IGEgc3RhbmRhcmQgVVNCIHBvcnQpLiBUaGlz IG5ldwo+PiA+PiA+IHN5c2ZzIG9wdGlvbiBtYWtlcyBpdCBwb3NzaWJsZSB0byB1c2UgdGhlIG9s ZCBzY2hlbWUgb24gYSBzaW5nbGUKPj4gPj4gPiBwb3J0IG9ubHkuCj4+ID4+ID4KPj4gPj4gPiBT aWduZWQtb2ZmLWJ5OiBOaWNvbGFzIEJvaWNoYXQgPGRyaW5rY2F0QGNocm9taXVtLm9yZz4KPj4g Pj4gPiAtLS0KPj4gPj4gPgo+PiA+PiA+IFRoZXJlIGFyZSBvdGhlciAicXVpcmtzIiB0aGF0IHdl IGNvdWxkIGFkZCB0byByZWR1Y2UgZnVydGhlcgo+PiA+PiA+IGVudW1lcmF0aW9uIHRpbWUgKGUu Zy4gcmVkdWNlIFVTQiBkZWJvdW5jZSB0aW1lLCByZWR1Y2UgVFJTVFJDWQo+PiA+PiA+IHRvIDEw bXMgaW5zdGVhZCBvZiA1MG1zIGFzIHVzZWQgY3VycmVudGx5KSwgYnV0IHRoZSBsb2dpYyBpcyBx dWl0ZQo+PiA+PiA+IHNpbWlsYXIsIHNvIGl0J2QgYmUgZ29vZCB0byBoYXZlIHRoaXMgcmV2aWV3 ZWQgZmlyc3QuCj4+ID4+Cj4+ID4+IEknbSBub3Qgb3Bwb3NlZCB0byB0aGUgaWRlYSBpbiBwcmlu Y2lwbGUsIGFsdGhvdWdoIEkgZG9uJ3QgbGlrZSB5b3VyCj4+ID4+IGltcGxlbWVudGF0aW9uIGJl Y2F1c2UgaXQgYnJlYWtzIHRoZSBvcmlnaW5hbCBvbGRfc2NoZW1lX2ZpcnN0Cj4+ID4+IHBhcmFt ZXRlci4KPj4KPj4gSSBkb24ndCB0aGluayBpdCBicmVha3MgdGhlIG9yaWdpbmFsIHBhcmFtZXRl cj8gSSBtZWFuLAo+PiAvc3lzL21vZHVsZS91c2Jjb3JlL3BhcmFtZXRlcnMvb2xkX3NjaGVtZV9m aXJzdCBpcyBzdGlsbCBhIGdsb2JhbAo+PiBkZWZhdWx0LCB3aGlsZSBiaXQgMCBvZiAvc3lzL2J1 cy91c2IvZGV2aWNlcy94L3kvei9xdWlya3MgYmVjb21lcyBhCj4+IHBvcnQtc3BlY2lmaWMgb3Zl cnJpZGUuCj4+Cj4+ID4+IExldCdzIHNlZSB3aGF0IHNvbWUgb3RoZXIgcGVvcGxlIHRoaW5rLgo+ PiA+Pgo+PiA+PiBZb3VycyBpcyBhIHJhdGhlciBzcGVjaWFsIGNhc2UsIGJlY2F1c2UgeW91IGtu b3cgZXhhY3RseSB3aGF0IGRldmljZQo+PiA+PiB3aWxsIGJlIGF0dGFjaGVkIHRvIGEgc3BlY2lm aWMgcG9ydC4gIFN0aWxsLCBJIGNhbiBzZWUgdGhhdCBzb3J0IG9mCj4+ID4+IHRoaW5nIGhhcHBl bmluZyBpbiBjb25zdHJhaW5lZCBhbmQgc3BlY2lhbC1wdXJwb3NlIHNldHRpbmdzLgo+PiA+Pgo+ PiA+PiBIb3cgZG8geW91IGFycmFuZ2UgdG8gc2V0IHRoZSBuZXcgcXVpcmsgYmVmb3JlIHRoZSBk ZXZpY2UgaXMKPj4gPj4gZGlzY292ZXJlZD8KPj4gPgo+PiA+IFllYWgsIHRoaXMgbGFzdCBxdWVz dGlvbiBpcyB3aGF0IEkgaGFkIHdoZW4gbG9va2luZyBhdCB0aGlzLiAgT3IgZG9lcyBpdAo+PiA+ IG5vdCBtYXR0ZXIgYXQgZmlyc3QgYm9vdCBhbmQgb25seSBtYXR0ZXJzIGZvciB3YWtlLXVwPwo+ Pgo+PiBJdCBkb2VzIG5vdCBtYXR0ZXIgb24gYm9vdCwgd2UgaGF2ZSBwbGVudHkgb2YgdGltZSB0 byBlbnVtZXJhdGUgdGhlCj4+IGRldmljZS4gV2UgdXNlIFVTQiAoYXV0by0pc3VzcGVuZCBhbmQg cmVtb3RlIHdha2UsIHNvIG5vCj4+IHJlLWVudW1lcmF0aW9uIHRoZXJlIGVpdGhlci4gSXQgb25s eSBtYXR0ZXJzIG9uIHVucGx1Zy9yZXBsdWcgd2hlcmUKPj4gdGhlIGRldmljZSBuZWVkcyB0byBi ZSByZS1lbnVtZXJhdGVkLgo+Cj4gSG93IGRvZXMgdGhpcyBkZXZpY2UgZ2V0IHVucGx1Z2dlZC9y ZXBsdWdnZWQgaWYgaXQgaXMgY29ubmVjdGVkIGRpcmVjdGx5Cj4gdG8gdGhlIGRldmljZT8KCkl0 IGlzIGV4dGVybmFsLiBFc3NlbnRpYWxseSwgdGhpcyBpcyBhIHRhYmxldCB3aXRoIGEgZGV0YWNo YWJsZQprZXlib2FyZC90b3VjaHBhZC4gVGhlIGludGVyZmFjZSBiZXR3ZWVuIHRhYmxldCBhbmQg YmFzZSBpcyBVU0IsIG92ZXIKcG9nbyBwaW5zLiBUaGUgcG9ydCBpcyBub24tc3RhbmRhcmQgKHBv Z28sIG5vdCBub3JtYWwgVVNCKSwgYW5kIHdlCmZ1bGx5IGNvbnRyb2wgdGhlIGZpcm13YXJlIG9u IHRoZSBiYXNlIHNpZGUgYXMgd2VsbCwgd2hpY2ggYWxsb3dzIHVzCnRvIHRha2Ugc2hvcnRjdXRz IGxpa2UgdGhpczogd2Uga25vdyBleGFjdGx5IHdoYXQgZGV2aWNlIHdpbGwgYmUKY29ubmVjdGVk IG9uIHRoYXQgcG9ydC4KCj4+IFNvbWV3aGVyZSBpbiBhbiBpbml0IHNjcmlwdCwgd2Ugd291bGQg ZG8gdGhpcyAod2Uga25vdyBpbiBhZHZhbmNlIHRoYXQKPj4gdXNiMSBwb3J0MiBpcyB0aGUgYnVz L3BvcnQgd2hlcmUgd2UgaGF2ZSBvdXIgcG9nby1waW4gVVNCIGludGVyZmFjZSwKPj4gc28gd2Ug Y2FuIGhhcmQtY29kZSB0aGUgcGF0aCk6Cj4+IGVjaG8gMSA+IC9zeXMvYnVzL3VzYi9kZXZpY2Vz L3VzYjEvMS0wOjEuMC91c2IxLXBvcnQyL3F1aXJrcwo+Pgo+PiBXZSBjb3VsZCB0cnkgdG8gYWRk IEFDUEkgc3VwcG9ydCAoanVzdCBsaWtlIGNvbm5lY3RfdHlwZSksIGJ1dCB3ZQo+PiBkb24ndCBz dHJpY3RseSBuZWVkIGl0IGZvciBvdXIgYXBwbGljYXRpb24uCj4KPiBJc24ndCB0aGVyZSBhbiAi aW50ZXJuYWwiIEFDUEkgZmxhZyBmb3IgVVNCIHBvcnRzLCBvciBpcyB0aGF0Cj4gd2hhdCBjb25u ZWN0X3R5cGUgaXM/ICBXaHkgd291bGRuJ3QgdGhhdCB3b3JrIGhlcmUgaW5zdGVhZD8KCldoYXQg d2UgbWF5IG5lZWQgaGVyZSBpcyBzb21ldGhpbmcgbGlrZSAiZXh0ZXJuYWwgYnV0IG5vbi1zdGFu ZGFyZCwgc28Kd2UgY2FuIGlnbm9yZSBjb21wYXRpYmlsaXR5IGNvbnN0cmFpbnRzIHdoZW4gZW51 bWVyYXRpbmciLi4uCgpUaGFua3MsCi0tLQpUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlzdDog c2VuZCB0aGUgbGluZSAidW5zdWJzY3JpYmUgbGludXgtdXNiIiBpbgp0aGUgYm9keSBvZiBhIG1l c3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZwpNb3JlIG1ham9yZG9tbyBpbmZvIGF0 ICBodHRwOi8vdmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwK