From 418add62c1391ae9195ec1c145906332732dc93c Mon Sep 17 00:00:00 2001 From: Sebastian Hesselbarth Date: Sat, 26 Jul 2014 17:24:41 +0200 Subject: USB: improve error paths and tear-down USB core isn't too strict about allocation/deallocation and add/remove sequences of usb devices. Especially, error paths and device tear-down are kind of broken and cause hangs on failing usb device detect. This patch improves the situation by introducing a usb_free_device() that tears down allocated resources by usb_alloc_new_device(). usb_remove_device() now only deals with resources that have been added by usb_new_device(). Also, error handling is fixed or improved to ensure that no devices are unregistered that have not been previously registered. Signed-off-by: Sebastian Hesselbarth Signed-off-by: Sascha Hauer --- drivers/usb/core/hub.c | 19 ++++++++++++------- drivers/usb/core/usb.c | 48 ++++++++++++++++++++++++++++-------------------- drivers/usb/core/usb.h | 1 + 3 files changed, 41 insertions(+), 27 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index c1f743cbed..d42b47cbe0 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -186,19 +186,21 @@ static void usb_hub_port_connect_change(struct usb_device *dev, int port) usb_clear_port_feature(dev, port + 1, USB_PORT_FEAT_C_CONNECTION); /* Disconnect any existing devices under this port */ - if (((!(portstatus & USB_PORT_STAT_CONNECTION)) && - (!(portstatus & USB_PORT_STAT_ENABLE))) || (dev->children[port])) { - dev_dbg(&dev->dev, "usb_disconnect(&hub->children[port]);\n"); + if (dev->children[port] && !(portstatus & USB_PORT_STAT_CONNECTION)) { + dev_dbg(&dev->dev, "disconnect detected on port %d\n", port + 1); usb_remove_device(dev->children[port]); - /* Return now if nothing is connected */ - if (!(portstatus & USB_PORT_STAT_CONNECTION)) - return; + return; } + + /* Remove disabled but connected devices */ + if (dev->children[port] && !(portstatus & USB_PORT_STAT_ENABLE)) + usb_remove_device(dev->children[port]); + wait_ms(200); /* Reset the port */ if (hub_port_reset(dev, port, &portstatus) < 0) { - printf("cannot reset port %i!?\n", port + 1); + dev_warn(&dev->dev, "cannot reset port %i!?\n", port + 1); return; } @@ -225,7 +227,10 @@ static void usb_hub_port_connect_change(struct usb_device *dev, int port) /* Woops, disable the port */ dev_dbg(&dev->dev, "hub: disabling port %d\n", port + 1); usb_clear_port_feature(dev, port + 1, USB_PORT_FEAT_ENABLE); + usb_free_device(usb); + return; } + device_detect(&usb->dev); } diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 7307a6f103..dff29361a8 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -430,7 +430,7 @@ int usb_new_device(struct usb_device *dev) err = register_device(&dev->dev); if (err) { printf("Failed to register device: %s\n", strerror(-err)); - return err; + goto err_out; } dev_add_param_int_ro(&dev->dev, "iManufacturer", @@ -457,42 +457,45 @@ err_out: return err; } +void usb_free_device(struct usb_device *usbdev) +{ + dma_free(usbdev->descriptor); + dma_free(usbdev->setup_packet); + free(usbdev); +} + void usb_remove_device(struct usb_device *usbdev) { - int i, ret; + int i; - for (i = 0; i < usbdev->maxchild; i++) { - if (usbdev->children[i]) - usb_remove_device(usbdev->children[i]); - } + if (!usbdev) + return; - dev_info(&usbdev->dev, "removing\n"); + for (i = 0; i < usbdev->maxchild; i++) + usb_remove_device(usbdev->children[i]); + if (usbdev->parent && usbdev->portnr) + usbdev->parent->children[usbdev->portnr - 1] = NULL; + list_del(&usbdev->list); + dev_count--; - ret = unregister_device(&usbdev->dev); - if (ret) + if (unregister_device(&usbdev->dev)) dev_err(&usbdev->dev, "failed to unregister\n"); + else + dev_info(&usbdev->dev, "removed\n"); - usbdev->parent->children[usbdev->portnr - 1] = NULL; - list_del(&usbdev->list); - free(usbdev); - dev_count--; + usb_free_device(usbdev); } struct usb_device *usb_alloc_new_device(void) { struct usb_device *usbdev = xzalloc(sizeof (*usbdev)); - if (!usbdev) - return NULL; - - usbdev->devnum = dev_index + 1; + usbdev->devnum = ++dev_index; usbdev->maxchild = 0; usbdev->dev.bus = &usb_bus_type; usbdev->setup_packet = dma_alloc(sizeof(*usbdev->setup_packet)); usbdev->descriptor = dma_alloc(sizeof(*usbdev->descriptor)); - dev_index++; - return usbdev; } @@ -508,7 +511,12 @@ int usb_host_detect(struct usb_host *host) host->root_dev = usb_alloc_new_device(); host->root_dev->dev.parent = host->hw_dev; host->root_dev->host = host; - usb_new_device(host->root_dev); + + ret = usb_new_device(host->root_dev); + if (ret) { + usb_free_device(host->root_dev); + return ret; + } } device_detect(&host->root_dev->dev); diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h index a0c05506dd..a5bb255121 100644 --- a/drivers/usb/core/usb.h +++ b/drivers/usb/core/usb.h @@ -2,6 +2,7 @@ #define __CORE_USB_H struct usb_device *usb_alloc_new_device(void); +void usb_free_device(struct usb_device *dev); int usb_new_device(struct usb_device *dev); void usb_remove_device(struct usb_device *dev); -- cgit v1.2.3