[coreboot-gerrit] Patch set updated for coreboot: UNTESTED libpayload: Use interrupt transfers for USB hubs

Nico Huber (nico.h@gmx.de) gerrit at coreboot.org
Sun Feb 26 13:16:56 CET 2017


Nico Huber (nico.h at gmx.de) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/18499

-gerrit

commit febf939e2d202646762ec452527baae43ac8efe7
Author: Nico Huber <nico.huber at secunet.com>
Date:   Wed Jun 12 16:06:08 2013 +0200

    UNTESTED libpayload: Use interrupt transfers for USB hubs
    
    In interactive payloads, the USB stack's poll procedure is implicitly
    called from the UI loop. Since all USB control transfers are handled
    synchronously, polling hubs with these slows the UI significantly down.
    
    So switch to interrupt transfers that are done asynchronously and only
    perform control transfers when the hub reported a status change.
    
    Change-Id: I5af02d63e4b8e1451b160b77f3611b93658a7a48
    Signed-off-by: Nico Huber <nico.h at gmx.de>
---
 payloads/libpayload/drivers/usb/usbhub.c | 115 ++++++++++++++++++++++++++++---
 1 file changed, 107 insertions(+), 8 deletions(-)

diff --git a/payloads/libpayload/drivers/usb/usbhub.c b/payloads/libpayload/drivers/usb/usbhub.c
index f75141e..5763ba8 100644
--- a/payloads/libpayload/drivers/usb/usbhub.c
+++ b/payloads/libpayload/drivers/usb/usbhub.c
@@ -35,15 +35,33 @@
 /* assume that host_to_device is overwritten if necessary */
 #define DR_PORT gen_bmRequestType(host_to_device, class_type, other_recp)
 /* status (and status change) bits */
-#define PORT_CONNECTION 0x1
-#define PORT_ENABLE 0x2
-#define PORT_RESET 0x10
+#define PORT_CONNECTION		0x01
+#define PORT_ENABLE		0x02
+#define PORT_SUSPEND		0x04
+#define PORT_OVER_CURRENT	0x08
+#define PORT_RESET		0x10
 /* feature selectors (for setting / clearing features) */
-#define SEL_PORT_RESET 0x4
-#define SEL_PORT_POWER 0x8
-#define SEL_C_PORT_CONNECTION 0x10
+#define SEL_PORT_RESET		0x04
+#define SEL_PORT_POWER		0x08
+#define SEL_C_PORT_CONNECTION	0x10
+#define SEL_C_PORT_ENABLE	0x11
+#define SEL_C_PORT_SUSPEND	0x12
+#define SEL_C_PORT_OVER_CURRENT	0x13
+#define SEL_C_PORT_RESET	0x14
 /* request type (USB 3.0 hubs only) */
-#define SET_HUB_DEPTH 12
+#define SET_HUB_DEPTH		12
+
+static endpoint_t *
+usb_hub_interrupt_ep(usbdev_t *const dev)
+{
+	int i;
+	for (i = 0; i < dev->num_endp; ++i) {
+		if (dev->endpoints[i].type == INTERRUPT &&
+				dev->endpoints[i].direction == IN)
+			return &dev->endpoints[i];
+	}
+	return NULL;
+}
 
 static int
 usb_hub_port_status_changed(usbdev_t *const dev, const int port)
@@ -158,9 +176,71 @@ static const generic_hub_ops_t usb_hub_ops = {
 	.reset_port		= generic_hub_resetport,
 };
 
+static int
+usb_hub_handle_port_change(usbdev_t *const dev, const int port)
+{
+	unsigned short buf[2] = { 0, 0 };
+	get_status(dev, port, DR_PORT, 4, buf);
+
+	/*
+	 * Second word holds the change bits. The interrupt transfer shows
+	 * a logical or of these bits, so we have to clear them all.
+	 */
+	if (buf[1] & PORT_CONNECTION) {
+		clear_feature(dev, port, SEL_C_PORT_CONNECTION, DR_PORT);
+		usb_debug("usbhub: Port change at %d\n", port);
+		const int ret = generic_hub_scanport(dev, port);
+		if (ret < 0)
+			return ret;
+	}
+	if (buf[1] & PORT_ENABLE)
+		clear_feature(dev, port, SEL_C_PORT_ENABLE, DR_PORT);
+	if (buf[1] & PORT_SUSPEND)
+		clear_feature(dev, port, SEL_C_PORT_SUSPEND, DR_PORT);
+	if (buf[1] & PORT_OVER_CURRENT)
+		clear_feature(dev, port, SEL_C_PORT_OVER_CURRENT, DR_PORT);
+	if (buf[1] & PORT_RESET)
+		clear_feature(dev, port, SEL_C_PORT_RESET, DR_PORT);
+	if (buf[1] & ~(PORT_CONNECTION | PORT_ENABLE |
+			PORT_SUSPEND | PORT_OVER_CURRENT | PORT_RESET))
+		usb_debug("usbhub: Spurious change bit at port %d\n", port);
+	return 0;
+}
+
+static void
+usb_hub_poll(usbdev_t *const dev)
+{
+	int port;
+	const u8 *buf;
+	while ((buf = dev->controller->poll_intr_queue(GEN_HUB(dev)->data))) {
+		for (port = 1; port <= GEN_HUB(dev)->num_ports; ++port) {
+			/* ports start at bit1; bit0 is hub status change */
+			if (buf[port / 8] & (1 << (port % 8))) {
+				if (usb_hub_handle_port_change(dev, port) < 0)
+					return;
+			}
+		}
+	}
+}
+
+static void
+usb_hub_destroy(usbdev_t *const dev)
+{
+	endpoint_t *const intr_ep = usb_hub_interrupt_ep(dev);
+	dev->controller->destroy_intr_queue(intr_ep, GEN_HUB(dev)->data);
+	generic_hub_destroy(dev);
+}
+
 void
 usb_hub_init(usbdev_t *const dev)
 {
+	endpoint_t *const intr_ep = usb_hub_interrupt_ep(dev);
+	if (!intr_ep) {
+		usb_debug("usbhub: ERROR: No interrupt-in endpoint found\n");
+		return;
+	}
+
+	/* Get number of ports from hub decriptor */
 	int type = dev->speed == SUPER_SPEED ? 0x2a : 0x29; /* similar enough */
 	hub_descriptor_t desc;	/* won't fit the whole thing, we don't care */
 	if (get_descriptor(dev, gen_bmRequestType(device_to_host, class_type,
@@ -172,5 +252,24 @@ usb_hub_init(usbdev_t *const dev)
 
 	if (dev->speed == SUPER_SPEED)
 		usb_hub_set_hub_depth(dev);
-	generic_hub_init(dev, desc.bNbrPorts, &usb_hub_ops);
+
+	/*
+	 * Register interrupt transfer:
+	 *   one bit per port + one bit for the hub,
+	 *   20 transfers in the queue, like our HID driver,
+	 *   one transfer per 256ms
+	 */
+	void *const intrq = dev->controller->create_intr_queue(
+				intr_ep, (desc.bNbrPorts + 8) / 8, 20, 256);
+	if (!intrq)
+		return;
+
+	if (generic_hub_init(dev, desc.bNbrPorts, &usb_hub_ops)) {
+		dev->controller->destroy_intr_queue(intr_ep, intrq);
+		return;
+	}
+
+	GEN_HUB(dev)->data = intrq;
+	dev->poll = usb_hub_poll;
+	dev->destroy = usb_hub_destroy;
 }



More information about the coreboot-gerrit mailing list