From 2af2a1b8d05a1a64c5005ed930137632b9d5aa22 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 22 Nov 2011 12:39:58 +0100 Subject: [PATCH 1/7] usb: make usb_create_simple catch and pass up errors. Use qdev_init() instead of qdev_init_nofail(), usb device initialization can fail, most common case being port and device speed mismatch. Handle failures correctly and pass up NULL pointers then. Also fixup usb_create_simple() callers (only one was buggy) to properly check for NULL pointers before referncing the usb_create_simple() return value. Signed-off-by: Gerd Hoffmann --- hw/usb-bt.c | 3 +++ hw/usb-bus.c | 11 +++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/hw/usb-bt.c b/hw/usb-bt.c index 529fa3355..f30eec1ea 100644 --- a/hw/usb-bt.c +++ b/hw/usb-bt.c @@ -528,6 +528,9 @@ USBDevice *usb_bt_init(HCIInfo *hci) if (!hci) return NULL; dev = usb_create_simple(NULL /* FIXME */, "usb-bt-dongle"); + if (!dev) { + return NULL; + } s = DO_UPCAST(struct USBBtState, dev, dev); s->dev.opaque = s; diff --git a/hw/usb-bus.c b/hw/usb-bus.c index 93f640d37..f8b980723 100644 --- a/hw/usb-bus.c +++ b/hw/usb-bus.c @@ -139,10 +139,17 @@ USBDevice *usb_create(USBBus *bus, const char *name) USBDevice *usb_create_simple(USBBus *bus, const char *name) { USBDevice *dev = usb_create(bus, name); + int rc; + if (!dev) { - hw_error("Failed to create USB device '%s'\n", name); + error_report("Failed to create USB device '%s'\n", name); + return NULL; + } + rc = qdev_init(&dev->qdev); + if (rc < 0) { + error_report("Failed to initialize USB device '%s'\n", name); + return NULL; } - qdev_init_nofail(&dev->qdev); return dev; } From f462141f18ffdd75847f6459ef83d90b831d12c0 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 22 Nov 2011 12:48:14 +0100 Subject: [PATCH 2/7] usb: fix usb_qdev_init error handling. qdev doesn't call the ->exit callback on ->init failures, so we have to take care ourself that we cleanup property on errors. Signed-off-by: Gerd Hoffmann --- hw/usb-bus.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/hw/usb-bus.c b/hw/usb-bus.c index f8b980723..8cafb76ff 100644 --- a/hw/usb-bus.c +++ b/hw/usb-bus.c @@ -9,6 +9,7 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent); static char *usb_get_dev_path(DeviceState *dev); static char *usb_get_fw_dev_path(DeviceState *qdev); +static int usb_qdev_exit(DeviceState *qdev); static struct BusInfo usb_bus_info = { .name = "USB", @@ -75,12 +76,23 @@ static int usb_qdev_init(DeviceState *qdev, DeviceInfo *base) dev->auto_attach = 1; QLIST_INIT(&dev->strings); rc = usb_claim_port(dev); - if (rc == 0) { - rc = dev->info->init(dev); + if (rc != 0) { + goto err; } - if (rc == 0 && dev->auto_attach) { + rc = dev->info->init(dev); + if (rc != 0) { + goto err; + } + if (dev->auto_attach) { rc = usb_device_attach(dev); + if (rc != 0) { + goto err; + } } + return 0; + +err: + usb_qdev_exit(qdev); return rc; } From be35cbbc8802eeba750c300c35339d69929989a6 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 22 Nov 2011 13:20:14 +0100 Subject: [PATCH 3/7] usb-hub: wakeup on detach too. When detaching devices from the usb hub we must wakeup too, otherwise the host misses the detach event. Commit 4a33a9ea06f6fbb08d8311a7cfed72975344f9ab does the same for device attach. Found by hkran@linux.vnet.ibm.com Signed-off-by: Gerd Hoffmann --- hw/usb-hub.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/usb-hub.c b/hw/usb-hub.c index 3eb0f1aa0..5b4876331 100644 --- a/hw/usb-hub.c +++ b/hw/usb-hub.c @@ -171,6 +171,8 @@ static void usb_hub_detach(USBPort *port1) USBHubState *s = port1->opaque; USBHubPort *port = &s->ports[port1->index]; + usb_wakeup(&s->dev); + /* Let upstream know the device on this port is gone */ s->dev.port->ops->child_detach(s->dev.port, port1->dev); From 20d183b6f0e14424d245493e7bc6fd6d14a08436 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 23 Nov 2011 13:31:08 +0100 Subject: [PATCH 4/7] usb-hub: implement reset based on a patch from hkran@linux.vnet.ibm.com Signed-off-by: Gerd Hoffmann --- hw/usb-hub.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/hw/usb-hub.c b/hw/usb-hub.c index 5b4876331..e1959372e 100644 --- a/hw/usb-hub.c +++ b/hw/usb-hub.c @@ -222,7 +222,22 @@ static void usb_hub_complete(USBPort *port, USBPacket *packet) static void usb_hub_handle_reset(USBDevice *dev) { - /* XXX: do it */ + USBHubState *s = DO_UPCAST(USBHubState, dev, dev); + USBHubPort *port; + int i; + + for (i = 0; i < NUM_PORTS; i++) { + port = s->ports + i; + port->wPortStatus = PORT_STAT_POWER; + port->wPortChange = 0; + if (port->port.dev && port->port.dev->attached) { + port->wPortStatus |= PORT_STAT_CONNECTION; + port->wPortChange |= PORT_STAT_C_CONNECTION; + if (port->port.dev->speed == USB_SPEED_LOW) { + port->wPortStatus |= PORT_STAT_LOW_SPEED; + } + } + } } static int usb_hub_handle_control(USBDevice *dev, USBPacket *p, @@ -497,9 +512,8 @@ static int usb_hub_initfn(USBDevice *dev) &port->port, s, i, &usb_hub_port_ops, USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL); usb_port_location(&port->port, dev->port, i+1); - port->wPortStatus = PORT_STAT_POWER; - port->wPortChange = 0; } + usb_hub_handle_reset(dev); return 0; } From aac882e7ce52f12f1b0712534c2456acea4f03d7 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 18 Nov 2011 10:48:47 +0100 Subject: [PATCH 5/7] usb-ehci: codestyle fixups Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c | 56 +++++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index 3eea94d09..2609aba34 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -437,37 +437,37 @@ struct EHCIState { } while(0) static const char *ehci_state_names[] = { - [ EST_INACTIVE ] = "INACTIVE", - [ EST_ACTIVE ] = "ACTIVE", - [ EST_EXECUTING ] = "EXECUTING", - [ EST_SLEEPING ] = "SLEEPING", - [ EST_WAITLISTHEAD ] = "WAITLISTHEAD", - [ EST_FETCHENTRY ] = "FETCH ENTRY", - [ EST_FETCHQH ] = "FETCH QH", - [ EST_FETCHITD ] = "FETCH ITD", - [ EST_ADVANCEQUEUE ] = "ADVANCEQUEUE", - [ EST_FETCHQTD ] = "FETCH QTD", - [ EST_EXECUTE ] = "EXECUTE", - [ EST_WRITEBACK ] = "WRITEBACK", - [ EST_HORIZONTALQH ] = "HORIZONTALQH", + [EST_INACTIVE] = "INACTIVE", + [EST_ACTIVE] = "ACTIVE", + [EST_EXECUTING] = "EXECUTING", + [EST_SLEEPING] = "SLEEPING", + [EST_WAITLISTHEAD] = "WAITLISTHEAD", + [EST_FETCHENTRY] = "FETCH ENTRY", + [EST_FETCHQH] = "FETCH QH", + [EST_FETCHITD] = "FETCH ITD", + [EST_ADVANCEQUEUE] = "ADVANCEQUEUE", + [EST_FETCHQTD] = "FETCH QTD", + [EST_EXECUTE] = "EXECUTE", + [EST_WRITEBACK] = "WRITEBACK", + [EST_HORIZONTALQH] = "HORIZONTALQH", }; static const char *ehci_mmio_names[] = { - [ CAPLENGTH ] = "CAPLENGTH", - [ HCIVERSION ] = "HCIVERSION", - [ HCSPARAMS ] = "HCSPARAMS", - [ HCCPARAMS ] = "HCCPARAMS", - [ USBCMD ] = "USBCMD", - [ USBSTS ] = "USBSTS", - [ USBINTR ] = "USBINTR", - [ FRINDEX ] = "FRINDEX", - [ PERIODICLISTBASE ] = "P-LIST BASE", - [ ASYNCLISTADDR ] = "A-LIST ADDR", - [ PORTSC_BEGIN ] = "PORTSC #0", - [ PORTSC_BEGIN + 4] = "PORTSC #1", - [ PORTSC_BEGIN + 8] = "PORTSC #2", - [ PORTSC_BEGIN + 12] = "PORTSC #3", - [ CONFIGFLAG ] = "CONFIGFLAG", + [CAPLENGTH] = "CAPLENGTH", + [HCIVERSION] = "HCIVERSION", + [HCSPARAMS] = "HCSPARAMS", + [HCCPARAMS] = "HCCPARAMS", + [USBCMD] = "USBCMD", + [USBSTS] = "USBSTS", + [USBINTR] = "USBINTR", + [FRINDEX] = "FRINDEX", + [PERIODICLISTBASE] = "P-LIST BASE", + [ASYNCLISTADDR] = "A-LIST ADDR", + [PORTSC_BEGIN] = "PORTSC #0", + [PORTSC_BEGIN + 4] = "PORTSC #1", + [PORTSC_BEGIN + 8] = "PORTSC #2", + [PORTSC_BEGIN + 12] = "PORTSC #3", + [CONFIGFLAG] = "CONFIGFLAG", }; static const char *nr2str(const char **n, size_t len, uint32_t nr) From 335b8d2068cbc56ce86b3c89b0fcd1c1eb4b61e2 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 18 Nov 2011 10:49:25 +0100 Subject: [PATCH 6/7] usb-ehci: add register names The mmio register name list only had the names for four port status registers. We emulate a EHCI adapter with six ports though, the last two ones are listed as "unknown" in traces. Fix it. Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index 2609aba34..a946e1d1f 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -467,6 +467,8 @@ static const char *ehci_mmio_names[] = { [PORTSC_BEGIN + 4] = "PORTSC #1", [PORTSC_BEGIN + 8] = "PORTSC #2", [PORTSC_BEGIN + 12] = "PORTSC #3", + [PORTSC_BEGIN + 16] = "PORTSC #4", + [PORTSC_BEGIN + 20] = "PORTSC #5", [CONFIGFLAG] = "CONFIGFLAG", }; From c7662daaa2898fa5a9a39bc51985cd1dd08af988 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 16 Nov 2011 12:37:17 +0100 Subject: [PATCH 7/7] usb-host: add usb_host_do_reset function. Add a special function to reset the host usb device. It tracks the time needed by the USBDEVFS_RESET ioctl and prints a warning in case it needs too long. Usually it should be finished in 200 - 300 miliseconds. Warning threshold is one second. Intention is to help troubleshooting by indicating that the usb device stopped responding even to a reset request and is possibly broken. Signed-off-by: Gerd Hoffmann --- usb-linux.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/usb-linux.c b/usb-linux.c index d4426ea73..ab4c6930c 100644 --- a/usb-linux.c +++ b/usb-linux.c @@ -148,6 +148,25 @@ static int usb_host_read_file(char *line, size_t line_size, const char *device_file, const char *device_name); static int usb_linux_update_endp_table(USBHostDevice *s); +static int usb_host_do_reset(USBHostDevice *dev) +{ + struct timeval s, e; + uint32_t usecs; + int ret; + + gettimeofday(&s, NULL); + ret = ioctl(dev->fd, USBDEVFS_RESET); + gettimeofday(&e, NULL); + usecs = (e.tv_sec - s.tv_sec) * 1000000; + usecs += e.tv_usec - s.tv_usec; + if (usecs > 1000000) { + /* more than a second, something is fishy, broken usb device? */ + fprintf(stderr, "husb: device %d:%d reset took %d.%06d seconds\n", + dev->bus_num, dev->addr, usecs / 1000000, usecs % 1000000); + } + return ret; +} + static struct endp_data *get_endp(USBHostDevice *s, int pid, int ep) { struct endp_data *eps = pid == USB_TOKEN_IN ? s->ep_in : s->ep_out; @@ -606,7 +625,7 @@ static void usb_host_handle_reset(USBDevice *dev) trace_usb_host_reset(s->bus_num, s->addr); - ioctl(s->fd, USBDEVFS_RESET); + usb_host_do_reset(s);; usb_host_claim_interfaces(s, 0); usb_linux_update_endp_table(s); @@ -1370,7 +1389,7 @@ static int usb_host_close(USBHostDevice *dev) if (dev->dev.attached) { usb_device_detach(&dev->dev); } - ioctl(dev->fd, USBDEVFS_RESET); + usb_host_do_reset(dev); close(dev->fd); dev->fd = -1; return 0; @@ -1381,7 +1400,7 @@ static void usb_host_exit_notifier(struct Notifier *n, void *data) USBHostDevice *s = container_of(n, USBHostDevice, exit); if (s->fd != -1) { - ioctl(s->fd, USBDEVFS_RESET); + usb_host_do_reset(s);; } }