Changeset 40a3bfa in mainline

Timestamp:

2017-10-28T11:25:11Z (6 years ago)

Author:

Petr Manek <petr.manek@…>

Branches:

lfn, master, serial, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

d46ceb2b

Parents:

58ac3ec

Message:

Refactoring. Moved a lot of device deallocation from root hub to bus. Also zeroing DCBAA to avoid assertion fails upon reconnection. Temporarily disabled freeing endpoint data structures due to suspected memory corruption.

Location:

uspace/drv/bus/usb/xhci

Files:

• bus.c (modified) (3 diffs)
• endpoint.h (modified) (1 diff)
• rh.c (modified) (3 diffs)

uspace/drv/bus/usb/xhci/bus.c

@@ -185 +185 @@
 int xhci_bus_remove_device(xhci_bus_t *bus, xhci_hc_t *hc, device_t *dev)
 {
+        int err;
         xhci_device_t *xhci_dev = xhci_device_get(dev);
+
+        /* Block creation of new endpoints and transfers. */
+        usb_log_debug2("Device '%s' going offline.", ddf_fun_get_name(dev->fun));
+        fibril_mutex_lock(&dev->guard);
+        xhci_dev->online = false;
+        fibril_mutex_unlock(&dev->guard);
+
+        /* Abort running transfers. */
+        usb_log_debug2("Aborting all active transfers to '%s'.", ddf_fun_get_name(dev->fun));
+        for (size_t i = 0; i < ARRAY_SIZE(xhci_dev->endpoints); ++i) {
+                xhci_endpoint_t *ep = xhci_dev->endpoints[i];
+                if (!ep || !ep->base.active)
+                        continue;
+
+                /* FIXME: This is racy. */
+                if ((err = xhci_transfer_abort(&ep->active_transfer))) {
+                        usb_log_warning("Failed to abort active %s transfer to "
+                            " endpoint %d of detached device '%s': %s",
+                            usb_str_transfer_type(ep->base.transfer_type),
+                            ep->base.endpoint, ddf_fun_get_name(dev->fun),
+                            str_error(err));
+                }
+        }
+
+        /* TODO: Figure out how to handle errors here. So far, they are reported and skipped. */
+
+        /* Make DDF (and all drivers) forget about the device. */
+        if ((err = ddf_fun_unbind(dev->fun))) {
+                usb_log_warning("Failed to unbind DDF function of detached device '%s': %s",
+                    ddf_fun_get_name(dev->fun), str_error(err));
+        }
+
+        /* Deconfigure device if it's still attached. */
+        if (!xhci_dev->detached) {
+                if ((err = hc_deconfigure_device(hc, xhci_dev->slot_id))) {
+                        usb_log_warning("Failed to deconfigure detached device '%s': %s",
+                            ddf_fun_get_name(dev->fun), str_error(err));
+                }
+        }
 
         /* Unregister remaining endpoints. */
@@ -192 +232 @@
                         continue;
 
-                const int err = unregister_endpoint(&bus->base, &xhci_dev->endpoints[i]->base);
-                if (err)
+                if ((err = unregister_endpoint(&bus->base, &xhci_dev->endpoints[i]->base))) {
                         usb_log_warning("Failed to unregister EP (%u:%u): %s", dev->address, i, str_error(err));
+                }
         }
 
         // XXX: Ugly here. Move to device_destroy at endpoint.c?
+        if ((err = hc_disable_slot(hc, xhci_dev->slot_id))) {
+                usb_log_warning("Failed to disable slot %d for device '%s': %s",
+                    xhci_dev->slot_id, ddf_fun_get_name(dev->fun), str_error(err));
+        }
+
         free32(xhci_dev->dev_ctx);
         hc->dcbaa[xhci_dev->slot_id] = 0;
+
+        bus->devices_by_slot[xhci_dev->slot_id] = NULL;
+
         return EOK;
 }
@@ -305 +353 @@
         /* Tear down TRB ring / PSA. */
         /* TODO: Make this method "noexcept" */
-        if ((err = xhci_endpoint_free_transfer_ds(ep))) {
+        /* FIXME: There is some memory corruption going on, causing this to crash. */
+        /*if ((err = xhci_endpoint_free_transfer_ds(ep))) {
                 usb_log_error("Failed to free resources of an endpoint.");
-        }
+        }*/
 
         return EOK;

uspace/drv/bus/usb/xhci/endpoint.h

@@ -125 +125 @@
         /** True if the device can add new endpoints and schedule transfers. */
         volatile bool online;
+
+        /** True if the device has been physically detached from the socket. */
+        volatile bool detached;
 } xhci_device_t;
 

uspace/drv/bus/usb/xhci/rh.c

@@ -187 +187 @@
         usb_log_info("Device '%s' at port %u has been disconnected.", ddf_fun_get_name(dev->base.fun), port_id);
 
-        /* Block creation of new endpoints and transfers. */
+        /* Mark the device as detached. */
         fibril_mutex_lock(&dev->base.guard);
-        dev->online = false;
+        dev->detached = true;
         fibril_mutex_unlock(&dev->base.guard);
 
@@ -197 +197 @@
         fibril_mutex_unlock(&rh->device.base.guard);
 
-        usb_log_debug2("Aborting all active transfers to '%s'.", ddf_fun_get_name(dev->base.fun));
-
-        /* Abort running transfers. */
-        for (size_t i = 0; i < ARRAY_SIZE(dev->endpoints); ++i) {
-                xhci_endpoint_t *ep = dev->endpoints[i];
-                if (!ep || !ep->base.active)
-                        continue;
-
-                /* FIXME: This is racy. */
-                if ((err = xhci_transfer_abort(&ep->active_transfer))) {
-                        usb_log_warning("Failed to abort active %s transfer to "
-                            " endpoint %d of detached device '%s': %s",
-                            usb_str_transfer_type(ep->base.transfer_type),
-                            ep->base.endpoint, ddf_fun_get_name(dev->base.fun),
-                            str_error(err));
-                }
-        }
-
-        /* TODO: Figure out how to handle errors here. So far, they are reported and skipped. */
-        /* TODO: Move parts of the code below to xhci_bus_remove_device() */
-
-        /* Make DDF (and all drivers) forget about the device. */
-        if ((err = ddf_fun_unbind(dev->base.fun))) {
-                usb_log_warning("Failed to unbind DDF function of detached device '%s': %s",
-                    ddf_fun_get_name(dev->base.fun), str_error(err));
-        }
-
-        /* Unregister EP0. */
-        if ((err = bus_remove_endpoint(&rh->hc->bus.base, &dev->endpoints[0]->base))) {
-                usb_log_warning("Failed to unregister configuration endpoint of device '%s' from XHCI bus: %s",
-                    ddf_fun_get_name(dev->base.fun), str_error(err));
-        }
-
-        /* Deconfigure device. */
-        if ((err = hc_deconfigure_device(rh->hc, dev->slot_id))) {
-                usb_log_warning("Failed to deconfigure detached device '%s': %s",
-                    ddf_fun_get_name(dev->base.fun), str_error(err));
-        }
-
-        /* TODO: Free EP0 structures. */
-        /* TODO: Destroy EP0 by removing its last reference. */
-
         /* Remove device from XHCI bus. */
         if ((err = xhci_bus_remove_device(&rh->hc->bus, rh->hc, &dev->base))) {
@@ -245 +203 @@
         }
 
-        /* Disable device slot. */
-        if ((err = hc_disable_slot(rh->hc, dev->slot_id))) {
-                usb_log_warning("Failed to disable slot for device '%s': %s",
-                    ddf_fun_get_name(dev->base.fun), str_error(err));
-        }
-
         /* Destroy DDF device. */
         hcd_ddf_device_destroy(&dev->base);
 
-        // TODO: Free device context.
-        // TODO: Free TRB rings.
-        // TODO: Figure out what was forgotten and free that as well.
+        /* TODO: Figure out what was forgotten and free that as well. */
 
         return EOK;