Changeset 2b61945 in mainline

Timestamp:

2017-10-22T03:47:41Z (7 years ago)

Author:

Ondřej Hlavatý <aearsis@…>

Branches:

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

Children:

2e5aea1

Parents:

766043c

Message:

xhci: use device_t for bookkeeping

This started as a little refactoring to move active transfer batch to endpoint. Finding the EP in handler needs devices indexed by slot id. Then I found out we do not use the device_t extendable mechanism. Then there were a lot of errors found while doing all this…

Location:

uspace

Files:

• drv/bus/usb/xhci/bus.c (modified) (12 diffs)
• drv/bus/usb/xhci/bus.h (modified) (2 diffs)
• drv/bus/usb/xhci/endpoint.c (modified) (6 diffs)
• drv/bus/usb/xhci/endpoint.h (modified) (4 diffs)
• drv/bus/usb/xhci/hc.c (modified) (7 diffs)
• drv/bus/usb/xhci/hc.h (modified) (3 diffs)
• drv/bus/usb/xhci/hw_struct/trb.h (modified) (1 diff)
• drv/bus/usb/xhci/rh.c (modified) (4 diffs)
• drv/bus/usb/xhci/scratchpad.c (modified) (1 diff)
• drv/bus/usb/xhci/transfers.c (modified) (8 diffs)
• drv/bus/usb/xhci/transfers.h (modified) (1 diff)
• drv/bus/usb/xhci/trb_ring.c (modified) (3 diffs)
• lib/usbhost/src/hcd.c (modified) (2 diffs)
• lib/usbhost/src/usb_transfer_batch.c (modified) (5 diffs)

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

@@ -35 +35 @@
 #include <adt/hash_table.h>
 #include <adt/hash.h>
+#include <usb/host/utils/malloc32.h>
 #include <usb/host/ddf_helpers.h>
 #include <usb/host/endpoint.h>
@@ -58 +59 @@
 } hashed_device_t;
 
+static int hashed_device_insert(xhci_bus_t *bus, hashed_device_t **hashed_dev, xhci_device_t *dev);
+static int hashed_device_remove(xhci_bus_t *bus, hashed_device_t *hashed_dev);
+static int hashed_device_find_by_address(xhci_bus_t *bus, usb_address_t address, hashed_device_t **dev);
+
 /** TODO: Still some copy-pasta left...
  */
@@ -63 +68 @@
 {
         int err;
+        xhci_device_t *xhci_dev = xhci_device_get(dev);
 
         /* TODO: get speed from the default address reservation */
@@ -85 +91 @@
         }
 
+        assert(bus->devices_by_slot[xhci_dev->slot_id] == NULL);
+        bus->devices_by_slot[xhci_dev->slot_id] = xhci_dev;
+
+        hashed_device_t *hashed_dev = NULL;
+        if ((err = hashed_device_insert(bus, &hashed_dev, xhci_dev)))
+                goto err_address;
+
         /* Read the device descriptor, derive the match ids */
         if ((err = hcd_ddf_device_explore(hc->hcd, dev))) {
                 usb_log_error("Device(%d): Failed to explore device: %s", dev->address, str_error(err));
-                bus_release_address(&bus->base, dev->address);
-                return err;
-        }
-
-        return EOK;
+                goto err_hash;
+        }
+
+        return EOK;
+
+err_hash:
+        bus->devices_by_slot[xhci_dev->slot_id] = NULL;
+        hashed_device_remove(bus, hashed_dev);
+err_address:
+        bus_release_address(&bus->base, dev->address);
+        return err;
 }
 
 int xhci_bus_remove_device(xhci_bus_t *bus, xhci_hc_t *hc, device_t *dev)
 {
-        // TODO: Implement me!
+        xhci_device_t *xhci_dev = xhci_device_get(dev);
+
+        // TODO: Release remaining EPs
+
+        hashed_device_t *hashed_dev;
+        int res = hashed_device_find_by_address(bus, dev->address, &hashed_dev);
+        if (res)
+                return res;
+
+        res = hashed_device_remove(bus, hashed_dev);
+        if (res != EOK)
+                return res;
+
+        // XXX: Ugly here. Move to device_destroy at endpoint.c?
+        free32(xhci_dev->dev_ctx);
+        hc->dcbaa[xhci_dev->slot_id] = 0;
         return ENOTSUP;
 }
@@ -134 +168 @@
         xhci_bus_t *bus = bus_to_xhci_bus(base);
 
-        xhci_endpoint_t *ep = malloc(sizeof(xhci_endpoint_t));
+        xhci_endpoint_t *ep = calloc(1, sizeof(xhci_endpoint_t));
         if (!ep)
                 return NULL;
@@ -179 +213 @@
 }
 
-static int hashed_device_create(xhci_bus_t *bus, hashed_device_t **hashed_dev, usb_address_t address)
-{
-        int res;
-        xhci_device_t *dev = (xhci_device_t *) malloc(sizeof(xhci_device_t));
-        if (!dev) {
-                res = ENOMEM;
-                goto err_xhci_dev_alloc;
-        }
-
-        res = xhci_device_init(dev, bus, address);
-        if (res != EOK) {
-                goto err_xhci_dev_init;
-        }
-
-        hashed_device_t *ret_dev = (hashed_device_t *) malloc(sizeof(hashed_device_t));
-        if (!ret_dev) {
-                res = ENOMEM;
-                goto err_hashed_dev_alloc;
-        }
+static int hashed_device_insert(xhci_bus_t *bus, hashed_device_t **hashed_dev, xhci_device_t *dev)
+{
+        hashed_device_t *ret_dev = (hashed_device_t *) calloc(1, sizeof(hashed_device_t));
+        if (!ret_dev)
+                return ENOMEM;
 
         ret_dev->device = dev;
 
-        usb_log_info("Device(%d) registered to XHCI bus.", dev->address);
+        usb_log_info("Device(%d) registered to XHCI bus.", dev->base.address);
 
         hash_table_insert(&bus->devices, &ret_dev->link);
         *hashed_dev = ret_dev;
         return EOK;
-
-err_hashed_dev_alloc:
-err_xhci_dev_init:
-        free(dev);
-err_xhci_dev_alloc:
-        return res;
 }
 
 static int hashed_device_remove(xhci_bus_t *bus, hashed_device_t *hashed_dev)
 {
-        usb_log_info("Device(%d) released from XHCI bus.", hashed_dev->device->address);
-
-        hash_table_remove(&bus->devices, &hashed_dev->device->address);
-        xhci_device_fini(hashed_dev->device);
-        free(hashed_dev->device);
+        usb_log_info("Device(%d) released from XHCI bus.", hashed_dev->device->base.address);
+
+        hash_table_remove(&bus->devices, &hashed_dev->device->base.address);
         free(hashed_dev);
 
@@ -231 +243 @@
         assert(bus);
 
-        hashed_device_t *hashed_dev;
-        int res = hashed_device_find_by_address(bus, ep->target.address, &hashed_dev);
-        if (res != EOK && res != ENOENT)
-                return res;
-
-        if (res == ENOENT) {
-                res = hashed_device_create(bus, &hashed_dev, ep->target.address);
-
-                if (res != EOK)
-                        return res;
-        }
-
         usb_log_info("Endpoint(%d:%d) registered to XHCI bus.", ep->target.address, ep->target.endpoint);
 
-        return xhci_device_add_endpoint(hashed_dev->device, xhci_endpoint_get(ep));
+        xhci_device_t *xhci_dev = xhci_device_get(ep->device);
+        xhci_endpoint_t *xhci_ep = xhci_endpoint_get(ep);
+        return xhci_device_add_endpoint(xhci_dev, xhci_ep);
 }
 
@@ -255 +257 @@
         usb_log_info("Endpoint(%d:%d) released from XHCI bus.", ep->target.address, ep->target.endpoint);
 
-        hashed_device_t *hashed_dev;
-        int res = hashed_device_find_by_address(bus, ep->target.address, &hashed_dev);
+        xhci_device_t *xhci_dev = xhci_device_get(ep->device);
+        xhci_endpoint_t *xhci_ep = xhci_endpoint_get(ep);
+        const int res = xhci_device_remove_endpoint(xhci_dev, xhci_ep);
         if (res != EOK)
                 return res;
 
-        res = xhci_device_remove_endpoint(hashed_dev->device, xhci_endpoint_get(ep));
-        if (res != EOK)
-                return res;
-
-        if (hashed_dev->device->active_endpoint_count == 0) {
-                res = hashed_device_remove(bus, hashed_dev);
-
-                if (res != EOK)
-                        return res;
-        }
-
         return EOK;
 }
@@ -278 +270 @@
         xhci_bus_t *bus = bus_to_xhci_bus(bus_base);
         assert(bus);
-
+        
         xhci_endpoint_t *ep;
         int res = xhci_endpoint_find_by_target(bus, target, &ep);
@@ -349 +341 @@
 {
         hashed_device_t *dev = hash_table_get_inst(item, hashed_device_t, link);
-        return (size_t) hash_mix(dev->device->address);
+        return (size_t) hash_mix(dev->device->base.address);
 }
 
@@ -360 +352 @@
 {
         hashed_device_t *dev = hash_table_get_inst(item, hashed_device_t, link);
-        return dev->device->address == *(usb_address_t *) key;
+        return dev->device->base.address == *(usb_address_t *) key;
 }
 
@@ -372 +364 @@
 };
 
-int xhci_bus_init(xhci_bus_t *bus)
-{
-        assert(bus);
-
-        bus_init(&bus->base, sizeof(device_t));
+int xhci_bus_init(xhci_bus_t *bus, xhci_hc_t *hc)
+{
+        assert(bus);
+
+        bus_init(&bus->base, sizeof(xhci_device_t));
+
+        bus->devices_by_slot = calloc(hc->max_slots, sizeof(xhci_device_t *));
+        if (!bus->devices_by_slot)
+                return ENOMEM;
 
         if (!hash_table_create(&bus->devices, 0, 0, &device_ht_ops)) {
-                // FIXME: Dealloc base!
+                free(bus->devices_by_slot);
                 return ENOMEM;
         }

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

@@ -43 +43 @@
 
 typedef struct xhci_hc xhci_hc_t;
+typedef struct xhci_device xhci_device_t;
 
 /** Endpoint management structure */
@@ -48 +49 @@
         bus_t base;             /**< Inheritance. Keep this first. */
 
-        /** TODO: some mechanism to keep endpoints alive :)
-         * We may inspire in the usb2_bus, but keep in mind xHCI have much
-         * larger address space, thus simple array of lists for all available
-         * addresses can be just too big.
-         */
+        xhci_device_t **devices_by_slot;        /**< Devices by Slot ID */
 
-        hash_table_t devices;
+        /** TODO: Do we really need this? */
+        hash_table_t devices;           /**< Devices by address */
 } xhci_bus_t;
 
-int xhci_bus_init(xhci_bus_t *);
+int xhci_bus_init(xhci_bus_t *, xhci_hc_t *);
 void xhci_bus_fini(xhci_bus_t *);
 

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

@@ -53 +53 @@
 
         endpoint_init(ep, bus);
-        xhci_ep->device = NULL;
-
-        return EOK;
+
+        return xhci_trb_ring_init(&xhci_ep->ring);
 }
 
@@ -63 +62 @@
 
         /* FIXME: Tear down TR's? */
-}
-
-int xhci_device_init(xhci_device_t *dev, xhci_bus_t *bus, usb_address_t address)
-{
-        memset(&dev->endpoints, 0, sizeof(dev->endpoints));
-        dev->active_endpoint_count = 0;
-        dev->address = address;
-        dev->slot_id = 0;
-
-        return EOK;
-}
-
-void xhci_device_fini(xhci_device_t *dev)
-{
-        // TODO: Check that all endpoints are dead.
-        assert(dev);
+        xhci_trb_ring_fini(&xhci_ep->ring);
+}
+
+/** See section 4.5.1 of the xHCI spec.
+ */
+uint8_t xhci_endpoint_dci(xhci_endpoint_t *ep)
+{
+        return (2 * ep->base.target.endpoint) +
+                (ep->base.transfer_type == USB_TRANSFER_CONTROL
+                 || ep->base.direction == USB_DIRECTION_IN);
 }
 
@@ -93 +86 @@
 uint8_t xhci_endpoint_index(xhci_endpoint_t *ep)
 {
-        return  (2 * ep->base.target.endpoint)
-            - (ep->base.direction == USB_DIRECTION_OUT);
+        return xhci_endpoint_dci(ep) - 1;
 }
 
@@ -139 +131 @@
         XHCI_EP_TYPE_SET(*ctx, xhci_endpoint_type(ep));
         XHCI_EP_MAX_PACKET_SIZE_SET(*ctx, ep->base.max_packet_size);
-        XHCI_EP_MAX_BURST_SIZE_SET(*ctx, ep->device->usb3 ? ss_desc->max_burst : 0);
+        XHCI_EP_MAX_BURST_SIZE_SET(*ctx,
+            xhci_device_get(ep->base.device)->usb3 ? ss_desc->max_burst : 0);
         XHCI_EP_ERROR_COUNT_SET(*ctx, 3);
 
@@ -186 +179 @@
 int xhci_device_add_endpoint(xhci_device_t *dev, xhci_endpoint_t *ep)
 {
-        assert(dev->address == ep->base.target.address);
-        assert(!dev->endpoints[ep->base.target.endpoint]);
-        assert(!ep->device);
-
-        int err;
-        xhci_input_ctx_t *ictx = NULL;
-        xhci_trb_ring_t *ep_ring = NULL;
-        if (ep->base.target.endpoint > 0) {
-                // FIXME: Retrieve this from somewhere, if applicable.
-                usb_superspeed_endpoint_companion_descriptor_t ss_desc;
-                memset(&ss_desc, 0, sizeof(ss_desc));
-
-                // Prepare input context.
-                ictx = malloc32(sizeof(xhci_input_ctx_t));
-                if (!ictx) {
-                        return ENOMEM;
-                }
-
-                memset(ictx, 0, sizeof(xhci_input_ctx_t));
-
-                // Quoting sec. 4.6.6: A1, D0, D1 are down, A0 is up.
-                XHCI_INPUT_CTRL_CTX_ADD_CLEAR(ictx->ctrl_ctx, 1);
-                XHCI_INPUT_CTRL_CTX_DROP_CLEAR(ictx->ctrl_ctx, 0);
-                XHCI_INPUT_CTRL_CTX_DROP_CLEAR(ictx->ctrl_ctx, 1);
-                XHCI_INPUT_CTRL_CTX_ADD_SET(ictx->ctrl_ctx, 0);
-
-                const uint8_t ep_idx = xhci_endpoint_index(ep);
-                XHCI_INPUT_CTRL_CTX_ADD_SET(ictx->ctrl_ctx, ep_idx + 1); /* Preceded by slot ctx */
-
-                ep_ring = malloc(sizeof(xhci_trb_ring_t));
-                if (!ep_ring) {
-                        err = ENOMEM;
-                        goto err_ictx;
-                }
-
-                // FIXME: This ring need not be allocated all the time.
-                err = xhci_trb_ring_init(ep_ring);
-                if (err)
-                        goto err_ring;
-
-                switch (ep->base.transfer_type) {
-                case USB_TRANSFER_CONTROL:
-                        setup_control_ep_ctx(ep, &ictx->endpoint_ctx[ep_idx], ep_ring);
-                        break;
-
-                case USB_TRANSFER_BULK:
-                        setup_bulk_ep_ctx(ep, &ictx->endpoint_ctx[ep_idx], ep_ring, &ss_desc);
-                        break;
-
-                case USB_TRANSFER_ISOCHRONOUS:
-                        setup_isoch_ep_ctx(ep, &ictx->endpoint_ctx[ep_idx], ep_ring, &ss_desc);
-                        break;
-
-                case USB_TRANSFER_INTERRUPT:
-                        setup_interrupt_ep_ctx(ep, &ictx->endpoint_ctx[ep_idx], ep_ring, &ss_desc);
-                        break;
-
-                }
-
-                dev->hc->dcbaa_virt[dev->slot_id].trs[ep->base.target.endpoint] = ep_ring;
-
-                // Issue configure endpoint command (sec 4.3.5).
-                xhci_cmd_t cmd;
-                xhci_cmd_init(&cmd);
-
-                cmd.slot_id = dev->slot_id;
-                xhci_send_configure_endpoint_command(dev->hc, &cmd, ictx);
-                if ((err = xhci_cmd_wait(&cmd, XHCI_DEFAULT_TIMEOUT)) != EOK)
-                        goto err_cmd;
-
-                xhci_cmd_fini(&cmd);
+        assert(dev);
+        assert(ep);
+
+        int err = ENOMEM;
+        usb_endpoint_t ep_num = ep->base.target.endpoint;
+
+        assert(&dev->base == ep->base.device);
+        assert(dev->base.address == ep->base.target.address);
+        assert(!dev->endpoints[ep_num]);
+
+        dev->endpoints[ep_num] = ep;
+        ++dev->active_endpoint_count;
+
+        if (ep_num == 0)
+                /* EP 0 is initialized while setting up the device,
+                 * so we must not issue the command now. */
+                return EOK;
+
+        // FIXME: Retrieve this from somewhere, if applicable.
+        usb_superspeed_endpoint_companion_descriptor_t ss_desc;
+        memset(&ss_desc, 0, sizeof(ss_desc));
+
+        // Prepare input context.
+        xhci_input_ctx_t *ictx = malloc32(sizeof(xhci_input_ctx_t));
+        if (!ictx)
+                goto err;
+
+        memset(ictx, 0, sizeof(xhci_input_ctx_t));
+
+        // Quoting sec. 4.6.6: A1, D0, D1 are down, A0 is up.
+        XHCI_INPUT_CTRL_CTX_ADD_CLEAR(ictx->ctrl_ctx, 1);
+        XHCI_INPUT_CTRL_CTX_DROP_CLEAR(ictx->ctrl_ctx, 0);
+        XHCI_INPUT_CTRL_CTX_DROP_CLEAR(ictx->ctrl_ctx, 1);
+        XHCI_INPUT_CTRL_CTX_ADD_SET(ictx->ctrl_ctx, 0);
+
+        unsigned ep_idx = xhci_endpoint_index(ep);
+        XHCI_INPUT_CTRL_CTX_ADD_SET(ictx->ctrl_ctx, ep_idx + 1); /* Preceded by slot ctx */
+
+        xhci_trb_ring_t *ep_ring = &ep->ring;
+        xhci_ep_ctx_t *ep_ctx = &ictx->endpoint_ctx[ep_idx];
+
+        // TODO: Convert to table
+        switch (ep->base.transfer_type) {
+        case USB_TRANSFER_CONTROL:
+                setup_control_ep_ctx(ep, ep_ctx, ep_ring);
+                break;
+
+        case USB_TRANSFER_BULK:
+                setup_bulk_ep_ctx(ep, ep_ctx, ep_ring, &ss_desc);
+                break;
+
+        case USB_TRANSFER_ISOCHRONOUS:
+                setup_isoch_ep_ctx(ep, ep_ctx, ep_ring, &ss_desc);
+                break;
+
+        case USB_TRANSFER_INTERRUPT:
+                setup_interrupt_ep_ctx(ep, ep_ctx, ep_ring, &ss_desc);
+                break;
         }
 
-        ep->device = dev;
-        dev->endpoints[ep->base.target.endpoint] = ep;
-        ++dev->active_endpoint_count;
+        // Issue configure endpoint command (sec 4.3.5).
+        xhci_cmd_t cmd;
+        xhci_cmd_init(&cmd);
+
+        cmd.slot_id = dev->slot_id;
+        xhci_send_configure_endpoint_command(dev->hc, &cmd, ictx);
+        if ((err = xhci_cmd_wait(&cmd, XHCI_DEFAULT_TIMEOUT)) != EOK)
+                goto err_ictx;
+
+        xhci_cmd_fini(&cmd);
+
+        free32(ictx);
         return EOK;
 
-err_cmd:
-err_ring:
-        if (ep_ring) {
-                xhci_trb_ring_fini(ep_ring);
-                free(ep_ring);
-        }
 err_ictx:
-        free(ictx);
+        free32(ictx);
+err:
+        dev->endpoints[ep_idx] = NULL;
+        dev->active_endpoint_count--;
         return err;
 }
@@ -277 +263 @@
 int xhci_device_remove_endpoint(xhci_device_t *dev, xhci_endpoint_t *ep)
 {
-        assert(dev->address == ep->base.target.address);
+        assert(&dev->base == ep->base.device);
+        assert(dev->base.address == ep->base.target.address);
         assert(dev->endpoints[ep->base.target.endpoint]);
-        assert(dev == ep->device);
 
         // TODO: Issue configure endpoint command to drop this endpoint.
 
-        ep->device = NULL;
         dev->endpoints[ep->base.target.endpoint] = NULL;
         --dev->active_endpoint_count;

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

@@ -44 +44 @@
 
 #include "hc.h"
+#include "transfers.h"
 
 typedef struct xhci_device xhci_device_t;
@@ -64 +65 @@
         endpoint_t base;        /**< Inheritance. Keep this first. */
 
-        /** Parent device. */
-        xhci_device_t *device;
+        /** Main TRB ring */
+        xhci_trb_ring_t ring;
+
+        /** There shall be only one transfer active on an endpoint. The
+         * synchronization is performed using the active flag in base
+         * endpoint_t */
+        xhci_transfer_t active_transfer;
 } xhci_endpoint_t;
 
 typedef struct xhci_device {
-        /** Unique USB address assigned to the device. */
-        usb_address_t address;
+        device_t base;          /**< Inheritance. Keep this first. */
 
         /** Slot ID assigned to the device by xHC. */
         uint32_t slot_id;
 
-        /** Associated device in libusbhost. */
-        device_t *device;
+        /** Place to store virtual address for allocated context */
+        xhci_device_ctx_t *dev_ctx;
 
         /** All endpoints of the device. Inactive ones are NULL */
@@ -97 +102 @@
 void xhci_device_fini(xhci_device_t *);
 
+uint8_t xhci_endpoint_dci(xhci_endpoint_t *);
 uint8_t xhci_endpoint_index(xhci_endpoint_t *);
 
@@ -104 +110 @@
 int xhci_device_configure(xhci_device_t *, xhci_hc_t *);
 
+static inline xhci_device_t * xhci_device_get(device_t *dev)
+{
+        assert(dev);
+        return (xhci_device_t *) dev;
+}
+
 static inline xhci_endpoint_t * xhci_endpoint_get(endpoint_t *ep)
 {
         assert(ep);
         return (xhci_endpoint_t *) ep;
+}
+
+static inline xhci_device_t * xhci_ep_to_dev(xhci_endpoint_t *ep)
+{
+        assert(ep);
+        return xhci_device_get(ep->base.device);
 }
 

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

@@ -196 +196 @@
                 return ENOMEM;
 
-        hc->dcbaa_virt = malloc((1 + hc->max_slots) * sizeof(xhci_virt_device_ctx_t));
-        if (!hc->dcbaa_virt) {
-                err = ENOMEM;
+        if ((err = xhci_trb_ring_init(&hc->command_ring)))
                 goto err_dcbaa;
-        }
-
-        if ((err = xhci_trb_ring_init(&hc->command_ring)))
-                goto err_dcbaa_virt;
 
         if ((err = xhci_event_ring_init(&hc->event_ring)))
@@ -214 +208 @@
                 goto err_scratch;
 
-        if ((err = xhci_init_transfers(hc)))
+        if ((err = xhci_rh_init(&hc->rh, hc, device)))
                 goto err_cmd;
 
-        if ((err = xhci_rh_init(&hc->rh, hc, device)))
-                goto err_transfers;
-
-        if ((err = xhci_bus_init(&hc->bus)))
+        if ((err = xhci_bus_init(&hc->bus, hc)))
                 goto err_rh;
 
@@ -228 +219 @@
 err_rh:
         xhci_rh_fini(&hc->rh);
-err_transfers:
-        xhci_fini_transfers(hc);
 err_cmd:
         xhci_fini_commands(hc);
@@ -238 +227 @@
 err_cmd_ring:
         xhci_trb_ring_fini(&hc->command_ring);
-err_dcbaa_virt:
-        free32(hc->dcbaa_virt);
 err_dcbaa:
         free32(hc->dcbaa);
@@ -527 +514 @@
         /* Update the ERDP to make room in the ring. */
         usb_log_debug2("Copying from ring finished, updating ERDP.");
-        hc->event_ring.dequeue_ptr = host2xhci(64, addr_to_phys(hc->event_ring.dequeue_trb));
         uint64_t erdp = hc->event_ring.dequeue_ptr;
         XHCI_REG_WR(intr, XHCI_INTR_ERDP_LO, LOWER32(erdp));
@@ -580 +566 @@
 static void hc_dcbaa_fini(xhci_hc_t *hc)
 {
-        xhci_trb_ring_t* trb_ring;
         xhci_scratchpad_free(hc);
-
-        /* Idx 0 already deallocated by xhci_scratchpad_free. */
-        for (unsigned i = 1; i < hc->max_slots + 1; ++i) {
-                if (hc->dcbaa_virt[i].dev_ctx) {
-                        free32(hc->dcbaa_virt[i].dev_ctx);
-                        hc->dcbaa_virt[i].dev_ctx = NULL;
-                }
-
-                for (unsigned i = 0; i < XHCI_EP_COUNT; ++i) {
-                        trb_ring = hc->dcbaa_virt[i].trs[i];
-                        if (trb_ring) {
-                                hc->dcbaa_virt[i].trs[i] = NULL;
-                                xhci_trb_ring_fini(trb_ring);
-                                free32(trb_ring);
-                        }
-                }
-        }
-
         free32(hc->dcbaa);
-        free32(hc->dcbaa_virt);
 }
 
@@ -610 +576 @@
         xhci_event_ring_fini(&hc->event_ring);
         hc_dcbaa_fini(hc);
-        xhci_fini_transfers(hc);
         xhci_fini_commands(hc);
         xhci_rh_fini(&hc->rh);

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

@@ -46 +46 @@
 #include "bus.h"
 
-typedef struct xhci_virt_device_ctx {
-        xhci_device_ctx_t *dev_ctx;
-        xhci_trb_ring_t *trs[XHCI_EP_COUNT];
-} xhci_virt_device_ctx_t;
-
 typedef struct xhci_hc {
         /* MMIO range */
@@ -68 +63 @@
         xhci_event_ring_t event_ring;
         uint64_t *dcbaa;
-        xhci_virt_device_ctx_t *dcbaa_virt;
         xhci_scratchpad_t *scratchpad;
 
@@ -84 +78 @@
         list_t commands;
         fibril_mutex_t commands_mtx;
-
-        list_t transfers;
 
         /* TODO: Hack. Figure out a better way. */

uspace/drv/bus/usb/xhci/hw_struct/trb.h

@@ -100 +100 @@
 #define TRB_CYCLE(trb)          XHCI_DWORD_EXTRACT((trb).control, 0, 0)
 #define TRB_LINK_TC(trb)        XHCI_DWORD_EXTRACT((trb).control, 1, 1)
+#define TRB_IOC(trb)            XHCI_DWORD_EXTRACT((trb).control, 5, 5)
 
 #define TRB_TRANSFER_LENGTH(trb)        XHCI_DWORD_EXTRACT((trb).status, 23, 0)

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

@@ -92 +92 @@
 }
 
-// TODO: Check device deallocation, we free device_ctx in hc.c, not
-//       sure about the other structs.
 // TODO: This currently assumes the device is attached to rh directly.
 //       Also, we should consider moving a lot of functionailty to xhci bus
 int xhci_rh_address_device(xhci_rh_t *rh, device_t *dev, xhci_bus_t *bus)
 {
+        int err;
+        xhci_device_t *xhci_dev = xhci_device_get(dev);
+
         /* FIXME: Certainly not generic solution. */
         const uint32_t route_str = 0;
 
-        int err;
-        xhci_hc_t *hc = rh->hc;
+        xhci_dev->hc = rh->hc;
+
         const xhci_port_speed_t *speed = xhci_rh_get_port_speed(rh, dev->port);
+        xhci_dev->usb3 = speed->major == 3;
 
         /* Enable new slot */
-        uint32_t slot_id;
-        if ((err = hc_enable_slot(hc, &slot_id)) != EOK)
+        if ((err = hc_enable_slot(rh->hc, &xhci_dev->slot_id)) != EOK)
                 return err;
-        usb_log_debug2("Obtained slot ID: %u.\n", slot_id);
+        usb_log_debug2("Obtained slot ID: %u.\n", xhci_dev->slot_id);
 
         /* Setup input context */
-        xhci_input_ctx_t *ictx = malloc(sizeof(xhci_input_ctx_t));
+        xhci_input_ctx_t *ictx = malloc32(sizeof(xhci_input_ctx_t));
         if (!ictx)
                 return ENOMEM;
@@ -126 +127 @@
         XHCI_SLOT_ROUTE_STRING_SET(ictx->slot_ctx, route_str);
 
-        xhci_trb_ring_t *ep_ring = malloc(sizeof(xhci_trb_ring_t));
-        if (!ep_ring) {
-                err = ENOMEM;
+        endpoint_t *ep0_base = bus_create_endpoint(&rh->hc->bus.base);
+        if (!ep0_base)
                 goto err_ictx;
-        }
-
-        err = xhci_trb_ring_init(ep_ring);
-        if (err)
-                goto err_ring;
-        setup_control_ep0_ctx(&ictx->endpoint_ctx[0], ep_ring, speed);
+        xhci_endpoint_t *ep0 = xhci_endpoint_get(ep0_base);
+        setup_control_ep0_ctx(&ictx->endpoint_ctx[0], &ep0->ring, speed);
 
         /* Setup and register device context */
@@ -141 +137 @@
         if (!dctx) {
                 err = ENOMEM;
-                goto err_ring;
-        }
+                goto err_ep;
+        }
+        xhci_dev->dev_ctx = dctx;
+        rh->hc->dcbaa[xhci_dev->slot_id] = addr_to_phys(dctx);
         memset(dctx, 0, sizeof(xhci_device_ctx_t));
-        hc->dcbaa[slot_id] = addr_to_phys(dctx);
-
-        memset(&hc->dcbaa_virt[slot_id], 0, sizeof(xhci_virt_device_ctx_t));
-        hc->dcbaa_virt[slot_id].dev_ctx = dctx;
-        hc->dcbaa_virt[slot_id].trs[0] = ep_ring;
 
         /* Address device */
-        if ((err = hc_address_device(hc, slot_id, ictx)) != EOK)
+        if ((err = hc_address_device(rh->hc, xhci_dev->slot_id, ictx)) != EOK)
                 goto err_dctx;
         dev->address = XHCI_SLOT_DEVICE_ADDRESS(dctx->slot_ctx);
         usb_log_debug2("Obtained USB address: %d.\n", dev->address);
 
-        // Ask libusbhost to create a control endpoint for EP0.
-        bus_t *bus_base = &bus->base;
-        usb_target_t ep0_target = { .address = dev->address, .endpoint = 0 };
-        usb_direction_t ep0_direction = USB_DIRECTION_BOTH;
-
-        // TODO: Should this call be unified with other calls to `bus_add_ep()`?
-        err = bus_add_ep(bus_base, dev, ep0_target.endpoint, ep0_direction,
-            USB_TRANSFER_CONTROL, CTRL_PIPE_MIN_PACKET_SIZE, CTRL_PIPE_MIN_PACKET_SIZE, 1);
-
-        if (err != EOK)
-                goto err_add_ep;
-
-        // Save all data structures in the corresponding xhci_device_t.
-        endpoint_t *ep0_base = bus_find_endpoint(bus_base, ep0_target, ep0_direction);
-        xhci_endpoint_t *ep0 = xhci_endpoint_get(ep0_base);
-        xhci_device_t *xhci_dev = ep0->device;
-
-        xhci_dev->device = dev;
-        xhci_dev->slot_id = slot_id;
-        xhci_dev->usb3 = speed->major == 3;
-        xhci_dev->hc = hc;
+        // XXX: Going around bus, duplicating code
+        ep0_base->device = dev;
+        ep0_base->target.address = dev->address;
+        ep0_base->target.endpoint = 0;
+        ep0_base->direction = USB_DIRECTION_BOTH;
+        ep0_base->transfer_type = USB_TRANSFER_CONTROL;
+        ep0_base->max_packet_size = CTRL_PIPE_MIN_PACKET_SIZE;
+        ep0_base->packets = 1;
+        ep0_base->bandwidth = CTRL_PIPE_MIN_PACKET_SIZE;
+
+        bus_register_endpoint(&rh->hc->bus.base, ep0_base);
 
         if (!rh->devices[dev->port - 1]) {
@@ -183 +166 @@
         }
 
-        return EOK;
-
-err_add_ep:
+        free32(ictx);
+        return EOK;
+
 err_dctx:
-        if (dctx) {
-                free(dctx);
-                hc->dcbaa[slot_id] = 0;
-                memset(&hc->dcbaa_virt[slot_id], 0, sizeof(xhci_virt_device_ctx_t));
-        }
-err_ring:
-        if (ep_ring) {
-                xhci_trb_ring_fini(ep_ring);
-                free(ep_ring);
-        }
+        free32(dctx);
+        rh->hc->dcbaa[xhci_dev->slot_id] = 0;
+err_ep:
+        xhci_endpoint_fini(ep0);
+        free(ep0);
 err_ictx:
-        free(ictx);
+        free32(ictx);
         return err;
 }

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

@@ -132 +132 @@
 
         hc->dcbaa[0] = 0;
-        memset(&hc->dcbaa_virt[0], 0, sizeof(xhci_virt_device_ctx_t));
 
         return;

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

@@ -94 +94 @@
 }
 
-int xhci_init_transfers(xhci_hc_t *hc)
-{
-        assert(hc);
-
-        list_initialize(&hc->transfers);
-        return EOK;
-}
-
-void xhci_fini_transfers(xhci_hc_t *hc)
-{
-        // Note: Untested.
-        assert(hc);
-}
-
-xhci_transfer_t* xhci_transfer_create(endpoint_t* ep) {
-        xhci_transfer_t* transfer = calloc(1, sizeof(xhci_transfer_t));
-        if (!transfer)
-                return NULL;
+/**
+ * There can currently be only one active transfer, because
+ * usb_transfer_batch_init locks the endpoint by endpoint_use.
+ * Therefore, we store the only active transfer per endpoint there.
+ */
+xhci_transfer_t* xhci_transfer_create(endpoint_t* ep)
+{
+        xhci_endpoint_t *xhci_ep = xhci_endpoint_get(ep);
+        xhci_transfer_t *transfer = &xhci_ep->active_transfer;
+
+        /* Do not access the transfer yet, it may be still in use. */
 
         usb_transfer_batch_init(&transfer->batch, ep);
-
-        link_initialize(&transfer->link);
+        assert(ep->active);
+
+        /* Clean just our data. */
+        memset(((void *) transfer) + sizeof(usb_transfer_batch_t), 0,
+            sizeof(xhci_transfer_t) - sizeof(usb_transfer_batch_t));
 
         return transfer;
@@ -126 +122 @@
         if (transfer->hc_buffer)
                 free32(transfer->hc_buffer);
-
-        free(transfer);
 }
 
 static xhci_trb_ring_t *get_ring(xhci_hc_t *hc, xhci_transfer_t *transfer)
 {
-        xhci_endpoint_t *xhci_ep = xhci_endpoint_get(transfer->batch.ep);
-        uint8_t slot_id = xhci_ep->device->slot_id;
-
-        xhci_trb_ring_t* ring = hc->dcbaa_virt[slot_id].trs[transfer->batch.ep->target.endpoint];
-        assert(ring);
-        return ring;
+        return &xhci_endpoint_get(transfer->batch.ep)->ring;
 }
 
@@ -202 +191 @@
         // Issue a Configure Endpoint command, if needed.
         if (configure_endpoint_needed(setup)) {
-                const int err = xhci_device_configure(xhci_ep->device, hc);
+                const int err = xhci_device_configure(xhci_ep_to_dev(xhci_ep), hc);
                 if (err)
                         return err;
@@ -226 +215 @@
         TRB_CTRL_SET_TRB_TYPE(trb, XHCI_TRB_TYPE_NORMAL);
 
-        xhci_endpoint_t *xhci_ep = xhci_endpoint_get(transfer->batch.ep);
-        uint8_t slot_id = xhci_ep->device->slot_id;
-        xhci_trb_ring_t* ring = hc->dcbaa_virt[slot_id].trs[transfer->batch.ep->target.endpoint];
+        xhci_trb_ring_t* ring = get_ring(hc, transfer);
 
         return xhci_trb_ring_enqueue(ring, &trb, &transfer->interrupt_trb_phys);
@@ -249 +236 @@
         TRB_CTRL_SET_TRB_TYPE(trb, XHCI_TRB_TYPE_NORMAL);
 
-        xhci_endpoint_t *xhci_ep = xhci_endpoint_get(transfer->batch.ep);
-        uint8_t slot_id = xhci_ep->device->slot_id;
-        xhci_trb_ring_t* ring = hc->dcbaa_virt[slot_id].trs[transfer->batch.ep->target.endpoint];
+        xhci_trb_ring_t* ring = get_ring(hc, transfer);
 
         return xhci_trb_ring_enqueue(ring, &trb, &transfer->interrupt_trb_phys);
@@ -266 +251 @@
 {
         uintptr_t addr = trb->parameter;
-        xhci_transfer_t *transfer = NULL;
-
-        link_t *transfer_link = list_first(&hc->transfers);
-        while (transfer_link) {
-                transfer = list_get_instance(transfer_link, xhci_transfer_t, link);
-
-                if (transfer->interrupt_trb_phys == addr)
-                        break;
-
-                transfer_link = list_next(transfer_link, &hc->transfers);
-        }
-
-        if (!transfer_link) {
-                usb_log_warning("Transfer not found.");
+        const unsigned slot_id = XHCI_DWORD_EXTRACT(trb->control, 31, 24);
+        const unsigned ep_dci = XHCI_DWORD_EXTRACT(trb->control, 20, 16);
+
+        xhci_device_t *dev = hc->bus.devices_by_slot[slot_id];
+        if (!dev) {
+                usb_log_error("Transfer event on unknown device slot %u!", slot_id);
                 return ENOENT;
         }
 
-        list_remove(transfer_link);
+        const usb_endpoint_t ep_num = ep_dci / 2;
+        xhci_endpoint_t *ep = xhci_device_get_endpoint(dev, ep_num);
+        if (!ep) {
+                usb_log_error("Transfer event on unknown endpoint num %u, device slot %u!", ep_num, slot_id);
+                return ENOENT;
+        }
+
+        xhci_transfer_t *transfer = &ep->active_transfer;
+
+        /** FIXME: This is racy. Do we care? */
+        ep->ring.dequeue = addr;
+
         usb_transfer_batch_t *batch = &transfer->batch;
 
@@ -314 +302 @@
         xhci_transfer_t *transfer = xhci_transfer_from_batch(batch);
         xhci_endpoint_t *xhci_ep = xhci_endpoint_get(batch->ep);
-        uint8_t slot_id = xhci_ep->device->slot_id;
-
         assert(xhci_ep);
-        assert(slot_id);
 
         const usb_transfer_type_t type = batch->ep->transfer_type;
@@ -336 +321 @@
                 return err;
 
-        list_append(&transfer->link, &hc->transfers);
-
+        const uint8_t slot_id = xhci_ep_to_dev(xhci_ep)->slot_id;
         const uint8_t target = xhci_endpoint_index(xhci_ep) + 1; /* EP Doorbells start at 1 */
         return hc_ring_doorbell(hc, slot_id, target);

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

@@ -54 +54 @@
 } xhci_transfer_t;
 
-int xhci_init_transfers(xhci_hc_t*);
-void xhci_fini_transfers(xhci_hc_t*);
-
 xhci_transfer_t* xhci_transfer_create(endpoint_t *);
 int xhci_transfer_schedule(xhci_hc_t*, usb_transfer_batch_t *);

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

@@ -155 +155 @@
 }
 
+static bool trb_generates_interrupt(xhci_trb_t *trb)
+{
+        return TRB_TYPE(*trb) >= XHCI_TRB_TYPE_ENABLE_SLOT_CMD
+                || TRB_IOC(*trb);
+}
+
 /**
  * Enqueue TDs composed of TRBs.
@@ -188 +194 @@
         xhci_trb_t *trb = first_trb;
         for (size_t i = 0; i < trbs; ++i, ++trb) {
+                if (trb_generates_interrupt(trb)) {
+                        if (*phys)
+                                return ENOTSUP;
+                        *phys = trb_ring_enqueue_phys(ring);
+                }
+
                 ring->enqueue_trb++;
 
@@ -205 +217 @@
         trb = first_trb;
         for (size_t i = 0; i < trbs; ++i, ++trb) {
-                if (phys && i == trbs - 1)
-                        *phys = trb_ring_enqueue_phys(ring);
-
                 xhci_trb_set_cycle(trb, ring->pcs);
                 xhci_trb_copy(ring->enqueue_trb, trb);

uspace/lib/usbhost/src/hcd.c

@@ -41 +41 @@
 #include <errno.h>
 #include <usb_iface.h>
+#include <str_error.h>
 
 #include "hcd.h"
@@ -128 +129 @@
 
         const int ret = hcd->ops.schedule(hcd, batch);
-        if (ret != EOK)
+        if (ret != EOK) {
+                usb_log_warning("Batch %p failed to schedule: %s", batch, str_error(ret));
                 usb_transfer_batch_destroy(batch);
+        }
 
         /* Drop our own reference to ep. */

uspace/lib/usbhost/src/usb_transfer_batch.c

@@ -40 +40 @@
 #include <assert.h>
 #include <errno.h>
+#include <str_error.h>
 
 
@@ -62 +63 @@
 void usb_transfer_batch_init(usb_transfer_batch_t *batch, endpoint_t *ep)
 {
+        endpoint_use(ep);
+
         memset(batch, 0, sizeof(*batch));
-
         batch->ep = ep;
-
-        endpoint_use(ep);
 }
 
@@ -103 +103 @@
         assert(batch->ep->bus);
 
-        usb_log_debug2("batch %p " USB_TRANSFER_BATCH_FMT " disposing.\n",
-            batch, USB_TRANSFER_BATCH_ARGS(*batch));
-
         bus_t *bus = batch->ep->bus;
-        if (bus->ops.destroy_batch)
+        if (bus->ops.destroy_batch) {
+                usb_log_debug2("Batch %p " USB_TRANSFER_BATCH_FMT " destroying.\n",
+                    batch, USB_TRANSFER_BATCH_ARGS(*batch));
                 bus->ops.destroy_batch(batch);
-        else
+        }
+        else {
+                usb_log_debug2("Batch %p " USB_TRANSFER_BATCH_FMT " disposing.\n",
+                    batch, USB_TRANSFER_BATCH_ARGS(*batch));
                 free(batch);
+        }
 
         endpoint_release(batch->ep);
@@ -123 +126 @@
 void usb_transfer_batch_finish(usb_transfer_batch_t *batch)
 {
-        if (!batch_complete(batch))
-                usb_log_warning("failed to complete batch %p!", batch);
+        const int err = batch_complete(batch);
+        if (err)
+                usb_log_warning("batch %p failed to complete: %s", batch, str_error(err));
 
         usb_transfer_batch_destroy(batch);
@@ -159 +163 @@
         struct old_handler_wrapper_data *data = malloc(sizeof(*data));
 
+        assert((!in_callback) != (!out_callback));
+
         data->in_callback = in_callback;
         data->out_callback = out_callback;