All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolas Boichat <drinkcat@chromium.org>
To: linux-usb@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Alan Stern <stern@rowland.harvard.edu>,
	Mathias Nyman <mathias.nyman@linux.intel.com>,
	Felipe Balbi <felipe.balbi@linux.intel.com>,
	Eugene Korenevsky <ekorenevsky@gmail.com>,
	Peter Chen <peter.chen@nxp.com>,
	Daniel Drake <drake@endlessm.com>, Joe Perches <joe@perches.com>,
	Nicolas Boichat <drinkcat@chromium.org>,
	Johan Hovold <johan@kernel.org>,
	Richard Leitner <richard.leitner@skidata.com>,
	linux-kernel@vger.kernel.org, groeck@chromium.org
Subject: [PATCH] usb: hub: Per-port setting to use old enumeration scheme
Date: Wed, 23 May 2018 10:16:56 +0800	[thread overview]
Message-ID: <20180523021656.122455-1-drinkcat@chromium.org> (raw)

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 <drinkcat@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,
 };
diff --git a/include/linux/usb.h b/include/linux/usb.h
index beffceec49158..2ade17992ed66 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -489,6 +489,13 @@ enum usb_port_connect_type {
 	USB_PORT_NOT_USED,
 };
 
+/*
+ * USB port quirks.
+ */
+
+/* For the given port, prefer the old (faster) enumeration scheme. */
+#define USB_PORT_QUIRK_OLD_SCHEME	BIT(0)
+
 /*
  * USB 2.0 Link Power Management (LPM) parameters.
  */
-- 
2.17.0.441.gb46fe60e1d-goog


WARNING: multiple messages have this Message-ID
From: Nicolas Boichat <drinkcat@chromium.org>
To: linux-usb@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Alan Stern <stern@rowland.harvard.edu>,
	Mathias Nyman <mathias.nyman@linux.intel.com>,
	Felipe Balbi <felipe.balbi@linux.intel.com>,
	Eugene Korenevsky <ekorenevsky@gmail.com>,
	Peter Chen <peter.chen@nxp.com>,
	Daniel Drake <drake@endlessm.com>, Joe Perches <joe@perches.com>,
	Nicolas Boichat <drinkcat@chromium.org>,
	Johan Hovold <johan@kernel.org>,
	Richard Leitner <richard.leitner@skidata.com>,
	linux-kernel@vger.kernel.org, groeck@chromium.org
Subject: usb: hub: Per-port setting to use old enumeration scheme
Date: Wed, 23 May 2018 10:16:56 +0800	[thread overview]
Message-ID: <20180523021656.122455-1-drinkcat@chromium.org> (raw)

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 <drinkcat@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,
 };
diff --git a/include/linux/usb.h b/include/linux/usb.h
index beffceec49158..2ade17992ed66 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -489,6 +489,13 @@ enum usb_port_connect_type {
 	USB_PORT_NOT_USED,
 };
 
+/*
+ * USB port quirks.
+ */
+
+/* For the given port, prefer the old (faster) enumeration scheme. */
+#define USB_PORT_QUIRK_OLD_SCHEME	BIT(0)
+
 /*
  * USB 2.0 Link Power Management (LPM) parameters.
  */

             reply	other threads:[~2018-05-23  2:17 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-23  2:16 Nicolas Boichat [this message]
2018-05-23  2:16 ` Nicolas Boichat
2018-05-23 14:03 ` [PATCH] " Alan Stern
2018-05-23 14:03   ` Alan Stern
2018-05-23 16:39   ` [PATCH] " Greg Kroah-Hartman
2018-05-23 16:39     ` Greg Kroah-Hartman
2018-05-23 23:42     ` [PATCH] " Nicolas Boichat
2018-05-23 23:42       ` Nicolas Boichat
2018-05-24 14:29       ` [PATCH] " Alan Stern
2018-05-24 14:29         ` Alan Stern
2018-05-24 16:21       ` [PATCH] " Greg Kroah-Hartman
2018-05-24 16:21         ` Greg Kroah-Hartman
2018-05-24 22:05         ` [PATCH] " Nicolas Boichat
2018-05-24 22:05           ` Nicolas Boichat
2018-05-25  6:29           ` [PATCH] " Greg Kroah-Hartman
2018-05-25  6:29             ` Greg Kroah-Hartman
2018-05-25  6:30 ` [PATCH] " Greg Kroah-Hartman
2018-05-25  6:30   ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180523021656.122455-1-drinkcat@chromium.org \
    --to=drinkcat@chromium.org \
    --cc=drake@endlessm.com \
    --cc=ekorenevsky@gmail.com \
    --cc=felipe.balbi@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=groeck@chromium.org \
    --cc=joe@perches.com \
    --cc=johan@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@linux.intel.com \
    --cc=peter.chen@nxp.com \
    --cc=richard.leitner@skidata.com \
    --cc=stern@rowland.harvard.edu \
    --subject='Re: [PATCH] usb: hub: Per-port setting to use old enumeration scheme' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.