On Wed, May 23, 2018 at 10:16:56AM +0800, 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 <drink...@chromium.org>
> ---
> 
> 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.
> 
>  drivers/usb/core/hub.c  | 13 +++++++++----
>  drivers/usb/core/hub.h  |  1 +
>  drivers/usb/core/port.c | 23 +++++++++++++++++++++++
>  include/linux/usb.h     |  7 +++++++
>  4 files changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index c2d993d3816f0..f900f66a62856 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2636,7 +2636,7 @@ static unsigned hub_is_wusb(struct usb_hub *hub)
>  #define SET_ADDRESS_TRIES    2
>  #define GET_DESCRIPTOR_TRIES 2
>  #define SET_CONFIG_TRIES     (2 * (use_both_schemes + 1))
> -#define USE_NEW_SCHEME(i)    ((i) / 2 == (int)old_scheme_first)
> +#define USE_NEW_SCHEME(i, scheme)    ((i) / 2 == (int)scheme)
>  
>  #define HUB_ROOT_RESET_TIME  60      /* times are in msec */
>  #define HUB_SHORT_RESET_TIME 10
> @@ -2651,12 +2651,16 @@ static unsigned hub_is_wusb(struct usb_hub *hub)
>   * enumeration failures, so disable this enumeration scheme for USB3
>   * devices.
>   */
> -static bool use_new_scheme(struct usb_device *udev, int retry)
> +static bool use_new_scheme(struct usb_device *udev, int retry,
> +                        struct usb_port *port_dev)
>  {
> +     int old_scheme_first_port =
> +             port_dev->quirks & USB_PORT_QUIRK_OLD_SCHEME;
> +
>       if (udev->speed >= USB_SPEED_SUPER)
>               return false;
>  
> -     return USE_NEW_SCHEME(retry);
> +     return USE_NEW_SCHEME(retry, old_scheme_first_port || old_scheme_first);
>  }
>  
>  /* Is a USB 3.0 port in the Inactive or Compliance Mode state?
> @@ -4392,6 +4396,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device 
> *udev, int port1,
>  {
>       struct usb_device       *hdev = hub->hdev;
>       struct usb_hcd          *hcd = bus_to_hcd(hdev->bus);
> +     struct usb_port         *port_dev = hub->ports[port1 - 1];
>       int                     retries, operations, retval, i;
>       unsigned                delay = HUB_SHORT_RESET_TIME;
>       enum usb_device_speed   oldspeed = udev->speed;
> @@ -4513,7 +4518,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device 
> *udev, int port1,
>       for (retries = 0; retries < GET_DESCRIPTOR_TRIES; (++retries, 
> msleep(100))) {
>               bool did_new_scheme = false;
>  
> -             if (use_new_scheme(udev, retry_counter)) {
> +             if (use_new_scheme(udev, retry_counter, port_dev)) {
>                       struct usb_device_descriptor *buf;
>                       int r = 0;
>  
> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> index 4dc769ee9c740..4accfb63f7dcb 100644
> --- a/drivers/usb/core/hub.h
> +++ b/drivers/usb/core/hub.h
> @@ -98,6 +98,7 @@ struct usb_port {
>       struct mutex status_lock;
>       u32 over_current_count;
>       u8 portnum;
> +     u32 quirks;
>       unsigned int is_superspeed:1;
>       unsigned int usb3_lpm_u1_permit:1;
>       unsigned int usb3_lpm_u2_permit:1;
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index 6979bde87d310..4a21431953953 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -50,6 +50,28 @@ static ssize_t over_current_count_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(over_current_count);
>  
> +static ssize_t quirks_show(struct device *dev,
> +                        struct device_attribute *attr, char *buf)
> +{
> +     struct usb_port *port_dev = to_usb_port(dev);
> +
> +     return sprintf(buf, "%08x\n", port_dev->quirks);
> +}
> +
> +static ssize_t quirks_store(struct device *dev, struct device_attribute 
> *attr,
> +                         const char *buf, size_t count)
> +{
> +     struct usb_port *port_dev = to_usb_port(dev);
> +     u32 value;
> +
> +     if (kstrtou32(buf, 16, &value))
> +             return -EINVAL;
> +
> +     port_dev->quirks = value;
> +     return count;
> +}
> +static DEVICE_ATTR_RW(quirks);
> +
>  static ssize_t usb3_lpm_permit_show(struct device *dev,
>                             struct device_attribute *attr, char *buf)
>  {
> @@ -118,6 +140,7 @@ static DEVICE_ATTR_RW(usb3_lpm_permit);
>  
>  static struct attribute *port_dev_attrs[] = {
>       &dev_attr_connect_type.attr,
> +     &dev_attr_quirks.attr,
>       &dev_attr_over_current_count.attr,
>       NULL,
>  };

Oops, you add a new sysfs file, but do not document it anywhere.  It
needs to go into Documentation/ABI/ along with the other files,
otherwise no one knows how to use it.

Please fix that up and resend.

thanks,

greg k-h

Reply via email to