Changeset eb928c4 in mainline

Timestamp:

2018-01-08T00:07:00Z (7 years ago)

Author:

Ondřej Hlavatý <aearsis@…>

Branches:

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

Children:

1102eca

Parents:

ecbad17

git-author:

Ondřej Hlavatý <aearsis@…> (2018-01-08 00:05:39)

git-committer:

Ondřej Hlavatý <aearsis@…> (2018-01-08 00:07:00)

Message:

xhci: documentation & cleanup

Also, a simple refactoring to remove functions that only wraps another
functions unused anywhere else.

Location:

uspace/drv/bus/usb/xhci

Files:

• bus.c (modified) (13 diffs)
• commands.c (modified) (25 diffs)
• debug.c (modified) (9 diffs)
• endpoint.c (modified) (4 diffs)
• hc.c (modified) (20 diffs)
• hc.h (modified) (1 diff)
• rh.c (modified) (12 diffs)
• scratchpad.c (modified) (3 diffs)
• transfers.c (modified) (2 diffs)
• transfers.h (modified) (1 diff)
• trb_ring.c (modified) (10 diffs)

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

@@ -57 +57 @@
 static endpoint_t *endpoint_create(device_t *, const usb_endpoint_descriptors_t *);
 
-/** Assign address and control endpoint to a new XHCI device.
+/** Ops receive generic bus_t pointer. */
+static inline xhci_bus_t *bus_to_xhci_bus(bus_t *bus_base)
+{
+        assert(bus_base);
+        return (xhci_bus_t *) bus_base;
+}
+
+/**
+ * Assign address and control endpoint to a new XHCI device. Once this function
+ * successfully returns, the device is online.
+ *
  * @param[in] bus XHCI bus, in which the address is assigned.
- * @param[in] dev New device to address and configure.
- *
+ * @param[in] dev New device to address and configure./e
  * @return Error code.
  */
@@ -103 +112 @@
 }
 
-/** Retrieve and set maximum packet size for endpoint zero of a XHCI device.
+/**
+ * Retrieve and set maximum packet size for endpoint zero of a XHCI device.
+ *
  * @param[in] hc Host controller, which manages the device.
  * @param[in] dev Device with operational endpoint zero.
- *
  * @return Error code.
  */
@@ -134 +144 @@
 }
 
-/** Respond to a new device on the XHCI bus. Address it, negotiate packet size
+/**
+ * Respond to a new device on the XHCI bus. Address it, negotiate packet size
  * and retrieve USB descriptors.
+ *
  * @param[in] bus XHCI bus, where the new device emerged.
  * @param[in] dev XHCI device, which has appeared on the bus.
@@ -141 +153 @@
  * @return Error code.
  */
-int xhci_bus_enumerate_device(xhci_bus_t *bus, device_t *dev)
-{
-        int err;
+static int device_enumerate(device_t *dev)
+{
+        int err;
+        xhci_bus_t *bus = bus_to_xhci_bus(dev->bus);
         xhci_device_t *xhci_dev = xhci_device_get(dev);
 
@@ -192 +205 @@
 static int endpoint_unregister(endpoint_t *);
 
-/** Remove device from XHCI bus. Transition it to the offline state, abort all
+/**
+ * Remove device from XHCI bus. Transition it to the offline state, abort all
  * ongoing transfers and unregister all of its endpoints.
+ *
+ * Bus callback.
+ *
  * @param[in] bus XHCI bus, from which the device is removed.
  * @param[in] dev XHCI device, which is removed from the bus.
- *
  * @return Error code.
  */
-int xhci_bus_remove_device(xhci_bus_t *bus, device_t *dev)
-{
-        int err;
+static int device_remove(device_t *dev)
+{
+        int err;
+        xhci_bus_t *bus = bus_to_xhci_bus(dev->bus);
         xhci_device_t *xhci_dev = xhci_device_get(dev);
 
@@ -255 +272 @@
 }
 
-/** Ops receive generic bus_t pointer. */
-static inline xhci_bus_t *bus_to_xhci_bus(bus_t *bus_base)
-{
-        assert(bus_base);
-        return (xhci_bus_t *) bus_base;
-}
-
-// TODO: Fill in docstrings for XHCI bus callbacks once generic bus callbacks get their docstrings.
-
-static int device_enumerate(device_t *dev)
-{
-        xhci_bus_t *bus = bus_to_xhci_bus(dev->bus);
-        return xhci_bus_enumerate_device(bus, dev);
-}
-
-static int device_remove(device_t *dev)
-{
-        xhci_bus_t *bus = bus_to_xhci_bus(dev->bus);
-        return xhci_bus_remove_device(bus, dev);
-}
-
+/**
+ * Reverts things device_offline did, getting the device back up.
+ *
+ * Bus callback.
+ */
 static int device_online(device_t *dev_base)
 {
@@ -304 +305 @@
 }
 
+/**
+ * Make given device offline. Offline the DDF function, tear down all
+ * endpoints, issue Deconfigure Device command to xHC.
+ *
+ * Bus callback.
+ */
 static int device_offline(device_t *dev_base)
 {
@@ -353 +360 @@
 }
 
+/**
+ * Create a new xHCI endpoint structure.
+ *
+ * Bus callback.
+ */
 static endpoint_t *endpoint_create(device_t *dev, const usb_endpoint_descriptors_t *desc)
 {
@@ -367 +379 @@
 }
 
+/**
+ * Destroy given xHCI endpoint structure.
+ *
+ * Bus callback.
+ */
 static void endpoint_destroy(endpoint_t *ep)
 {
@@ -375 +392 @@
 }
 
+/**
+ * Register an andpoint to the bus. Allocate its transfer ring(s) and inform
+ * xHC about it.
+ *
+ * Bus callback.
+ */
 static int endpoint_register(endpoint_t *ep_base)
 {
@@ -401 +424 @@
 }
 
+/**
+ * Unregister an endpoint. If the device is still available, inform the xHC
+ * about it. Destroy resources allocated when registering.
+ *
+ * Bus callback.
+ */
 static int endpoint_unregister(endpoint_t *ep_base)
 {
@@ -426 +455 @@
 }
 
-static usb_transfer_batch_t *batch_create(endpoint_t *ep)
-{
-        xhci_transfer_t *transfer = xhci_transfer_create(ep);
-        return &transfer->batch;
-}
-
-static void batch_destroy(usb_transfer_batch_t *batch)
-{
-        xhci_transfer_destroy(xhci_transfer_from_batch(batch));
-}
-
-/** Structure binding XHCI static callbacks to generic bus callbacks. */
+/**
+ * Schedule a batch for xHC.
+ *
+ * Bus callback.
+ */
+static int batch_schedule(usb_transfer_batch_t *batch)
+{
+        assert(batch);
+        xhci_hc_t *hc = bus_to_hc(endpoint_get_bus(batch->ep));
+
+        if (!batch->target.address) {
+                usb_log_error("Attempted to schedule transfer to address 0.");
+                return EINVAL;
+        }
+
+        return xhci_transfer_schedule(hc, batch);
+}
+
 static const bus_ops_t xhci_bus_ops = {
-// TODO: Is it good idea to use this macro? It blurrs the fact that the callbacks and static functions are called the same.
-#define BIND_OP(op) .op = op,
-        BIND_OP(device_enumerate)
-        BIND_OP(device_remove)
-        BIND_OP(device_online)
-        BIND_OP(device_offline)
-
-        BIND_OP(endpoint_create)
-        BIND_OP(endpoint_destroy)
-        BIND_OP(endpoint_register)
-        BIND_OP(endpoint_unregister)
-
-        BIND_OP(batch_create)
-        BIND_OP(batch_destroy)
-#undef BIND_OP
-
         .interrupt = hc_interrupt,
         .status = hc_status,
-        .batch_schedule = hc_schedule,
+
+        .device_enumerate = device_enumerate,
+        .device_remove = device_remove,
+        .device_online = device_online,
+        .device_offline = device_offline,
+
+        .endpoint_create = endpoint_create,
+        .endpoint_destroy = endpoint_destroy,
+        .endpoint_register = endpoint_register,
+        .endpoint_unregister = endpoint_unregister,
+
+        .batch_schedule = batch_schedule,
+        .batch_create = xhci_transfer_create,
+        .batch_destroy = xhci_transfer_destroy,
 };
 
@@ -486 +518 @@
 void xhci_bus_fini(xhci_bus_t *bus)
 {
-        // FIXME: Deallocate bus->devices_by_slot?
-        // FIXME: Should there be some bus_fini() to call?
+        // FIXME: Ensure there are no more devices?
+        free(bus->devices_by_slot);
         // FIXME: Something else we forgot?
 }

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

@@ -70 +70 @@
 }
 
+/**
+ * Initialize the command subsystem. Allocates the comand ring.
+ *
+ * Does not configure the CR pointer to the hardware, because the xHC will be
+ * reset before starting.
+ */
 int xhci_init_commands(xhci_hc_t *hc)
 {
@@ -89 +95 @@
 }
 
+/**
+ * Finish the command subsystem. Stops the hardware from running commands, then
+ * deallocates the ring.
+ */
 void xhci_fini_commands(xhci_hc_t *hc)
 {
+        assert(hc);
         xhci_stop_command_ring(hc);
-        assert(hc);
-}
-
+
+        xhci_cmd_ring_t *cr = get_cmd_ring(hc);
+
+        fibril_mutex_lock(&cr->guard);
+        xhci_trb_ring_fini(&cr->trb_ring);
+        fibril_mutex_unlock(&cr->guard);
+}
+
+/**
+ * Initialize a command structure for the given command.
+ */
 void xhci_cmd_init(xhci_cmd_t *cmd, xhci_cmd_type_t type)
 {
@@ -107 +126 @@
 }
 
+/**
+ * Finish the command structure. Some command invocation includes allocating
+ * a context structure. To have the convenience in calling commands, this
+ * method deallocates all resources.
+ */
 void xhci_cmd_fini(xhci_cmd_t *cmd)
 {
@@ -119 +143 @@
 }
 
-/** Call with guard locked. */
+/**
+ * Find a command issued by TRB at @c phys inside the command list.
+ *
+ * Call with guard locked only.
+ */
 static inline xhci_cmd_t *find_command(xhci_hc_t *hc, uint64_t phys)
 {
@@ -140 +168 @@
 }
 
-static inline int enqueue_command(xhci_hc_t *hc, xhci_cmd_t *cmd, unsigned doorbell, unsigned target)
+/**
+ * Enqueue a command on the TRB ring. Ring the doorbell to initiate processing.
+ * Register the command as waiting for completion inside the command list.
+ */
+static inline int enqueue_command(xhci_hc_t *hc, xhci_cmd_t *cmd)
 {
         xhci_cmd_ring_t *cr = get_cmd_ring(hc);
@@ -168 +200 @@
 }
 
+/**
+ * Stop the command ring. Stop processing commands, block issuing new ones.
+ * Wait until hardware acknowledges it is stopped.
+ */
 void xhci_stop_command_ring(xhci_hc_t *hc)
 {
@@ -187 +223 @@
 }
 
+/**
+ * Abort currently processed command. Note that it is only aborted when the
+ * command is "blocking" - see section 4.6.1.2 of xHCI spec.
+ */
 static void abort_command_ring(xhci_hc_t *hc)
 {
@@ -236 +276 @@
 };
 
+/**
+ * Report an error according to command completion code.
+ */
 static void report_error(int code)
 {
@@ -244 +287 @@
 }
 
+/**
+ * Handle a command completion. Feed the fibril waiting for result.
+ *
+ * @param trb The COMMAND_COMPLETION TRB found in event ring.
+ */
 int xhci_handle_command_completion(xhci_hc_t *hc, xhci_trb_t *trb)
 {
@@ -343 +391 @@
         TRB_SET_TYPE(cmd->_header.trb, XHCI_TRB_TYPE_NO_OP_CMD);
 
-        return enqueue_command(hc, cmd, 0, 0);
+        return enqueue_command(hc, cmd);
 }
 
@@ -355 +403 @@
         cmd->_header.trb.control |= host2xhci(32, XHCI_REG_RD(hc->xecp, XHCI_EC_SP_SLOT_TYPE) << 16);
 
-        return enqueue_command(hc, cmd, 0, 0);
+        return enqueue_command(hc, cmd);
 }
 
@@ -368 +416 @@
         TRB_SET_SLOT(cmd->_header.trb, cmd->slot_id);
 
-        return enqueue_command(hc, cmd, 0, 0);
+        return enqueue_command(hc, cmd);
 }
 
@@ -398 +446 @@
         TRB_SET_SLOT(cmd->_header.trb, cmd->slot_id);
 
-        return enqueue_command(hc, cmd, 0, 0);
+        return enqueue_command(hc, cmd);
 }
 
@@ -419 +467 @@
         TRB_SET_DC(cmd->_header.trb, cmd->deconfigure);
 
-        return enqueue_command(hc, cmd, 0, 0);
+        return enqueue_command(hc, cmd);
 }
 
@@ -441 +489 @@
         TRB_SET_SLOT(cmd->_header.trb, cmd->slot_id);
 
-        return enqueue_command(hc, cmd, 0, 0);
+        return enqueue_command(hc, cmd);
 }
 
@@ -460 +508 @@
         TRB_SET_SLOT(cmd->_header.trb, cmd->slot_id);
 
-        return enqueue_command(hc, cmd, 0, 0);
+        return enqueue_command(hc, cmd);
 }
 
@@ -475 +523 @@
         TRB_SET_SLOT(cmd->_header.trb, cmd->slot_id);
 
-        return enqueue_command(hc, cmd, 0, 0);
+        return enqueue_command(hc, cmd);
 }
 
@@ -495 +543 @@
          */
 
-        return enqueue_command(hc, cmd, 0, 0);
+        return enqueue_command(hc, cmd);
 }
 
@@ -508 +556 @@
         TRB_SET_SLOT(cmd->_header.trb, cmd->slot_id);
 
-        return enqueue_command(hc, cmd, 0, 0);
+        return enqueue_command(hc, cmd);
 }
 
@@ -524 +572 @@
         TRB_SET_DEV_SPEED(cmd->_header.trb, cmd->device_speed);
 
-        return enqueue_command(hc, cmd, 0, 0);
+        return enqueue_command(hc, cmd);
 }
 
@@ -541 +589 @@
         [XHCI_CMD_SET_TR_DEQUEUE_POINTER] = set_tr_dequeue_pointer_cmd,
         [XHCI_CMD_RESET_DEVICE] = reset_device_cmd,
-        // TODO: Force event (optional normative, for VMM, section 4.6.12).
         [XHCI_CMD_FORCE_EVENT] = NULL,
-        // TODO: Negotiate bandwidth (optional normative, section 4.6.13).
         [XHCI_CMD_NEGOTIATE_BANDWIDTH] = NULL,
-        // TODO: Set latency tolerance value (optional normative, section 4.6.14).
         [XHCI_CMD_SET_LATENCY_TOLERANCE_VALUE] = NULL,
-        // TODO: Get port bandwidth (mandatory, but needs root hub implementation, section 4.6.15).
         [XHCI_CMD_GET_PORT_BANDWIDTH] = get_port_bandwidth_cmd,
-        // TODO: Force header (mandatory, but needs root hub implementation, section 4.6.16).
         [XHCI_CMD_FORCE_HEADER] = NULL,
         [XHCI_CMD_NO_OP] = no_op_cmd
 };
 
+/**
+ * Try to abort currently processed command. This is tricky, because
+ * calling fibril is not necessarily the one which issued the blocked command.
+ * Also, the trickiness intensifies by the fact that stopping a CR is denoted by
+ * event, which is again handled in different fibril. but, once we go to sleep
+ * on waiting for that event, another fibril may wake up and try to abort the
+ * blocked command.
+ *
+ * So, we mark the command ring as being restarted, wait for it to stop, and
+ * then start it again. If there was a blocked command, it will be satisfied by
+ * COMMAND_ABORTED event.
+ */
 static int try_abort_current_command(xhci_hc_t *hc)
 {
@@ -603 +658 @@
 }
 
+/**
+ * Wait, until the command is completed. The completion is triggered by
+ * COMMAND_COMPLETION event. As we do not want to rely on HW completing the
+ * command in timely manner, we timeout. Note that we can't just return an
+ * error after the timeout pass - it may be other command blocking the ring,
+ * and ours can be completed afterwards. Therefore, it is not guaranteed that
+ * this function will return in XHCI_COMMAND_TIMEOUT. It will continue waiting
+ * until COMMAND_COMPLETION event arrives.
+ */
 static int wait_for_cmd_completion(xhci_hc_t *hc, xhci_cmd_t *cmd)
 {
@@ -630 +694 @@
 }
 
-/** Issue command and block the current fibril until it is completed or timeout
- *  expires. Nothing is deallocated. Caller should always execute `xhci_cmd_fini`.
+/**
+ * Issue command and block the current fibril until it is completed or timeout
+ * expires. Nothing is deallocated. Caller should always execute `xhci_cmd_fini`.
  */
 int xhci_cmd_sync(xhci_hc_t *hc, xhci_cmd_t *cmd)
@@ -658 +723 @@
 }
 
-/** Does the same thing as `xhci_cmd_sync` and executes `xhci_cmd_fini`. This
- *  is a useful shorthand for issuing commands without out parameters.
+/**
+ * Does the same thing as `xhci_cmd_sync` and executes `xhci_cmd_fini`. This
+ * is a useful shorthand for issuing commands without out parameters.
  */
 int xhci_cmd_sync_fini(xhci_hc_t *hc, xhci_cmd_t *cmd)
@@ -669 +735 @@
 }
 
-/** Does the same thing as `xhci_cmd_sync_fini` without blocking the current
- *  fibril. The command is copied to stack memory and `fini` is called upon its completion.
+/**
+ * Does the same thing as `xhci_cmd_sync_fini` without blocking the current
+ * fibril. The command is copied to stack memory and `fini` is called upon its completion.
  */
 int xhci_cmd_async_fini(xhci_hc_t *hc, xhci_cmd_t *stack_cmd)

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

@@ -31 +31 @@
  */
 /** @file
- * Memory-mapped register structures of the xHC.
+ * Various functions to examine current state of the xHC.
  */
 
@@ -100 +100 @@
 }
 
+/**
+ * Dumps registers of one port.
+ */
 void xhci_dump_port(const xhci_port_regs_t *port)
 {
@@ -126 +129 @@
 }
 
+/**
+ * Dumps all registers that define state of the HC.
+ */
 void xhci_dump_state(const xhci_hc_t *hc)
 {
@@ -173 +179 @@
 }
 
+/**
+ * Dump registers of all ports.
+ */
 void xhci_dump_ports(const xhci_hc_t *hc)
 {
@@ -221 +230 @@
 };
 
+/**
+ * Stringify XHCI_TRB_TYPE_*.
+ */
 const char *xhci_trb_str_type(unsigned type)
 {
@@ -232 +244 @@
 }
 
+/**
+ * Dump a TRB.
+ */
 void xhci_dump_trb(const xhci_trb_t *trb)
 {
@@ -252 +267 @@
 };
 
+/**
+ * Dump Extended Capability ID.
+ */
 const char *xhci_ec_str_id(unsigned id)
 {
@@ -263 +281 @@
 }
 
+/**
+ * Dump Protocol Speed ID.
+ */
 static void xhci_dump_psi(const xhci_psi_t *psi)
 {
@@ -276 +297 @@
 }
 
+/**
+ * Dump given Extended Capability.
+ */
 void xhci_dump_extcap(const xhci_extcap_t *ec)
 {

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

@@ -45 +45 @@
 #include "endpoint.h"
 
-/** Initialize new XHCI endpoint.
+/**
+ * Initialize new XHCI endpoint.
  * @param[in] xhci_ep Allocated XHCI endpoint to initialize.
  * @param[in] dev Device, to which the endpoint belongs.
@@ -98 +99 @@
 }
 
-/** Finalize XHCI endpoint.
+/**
+ * Finalize XHCI endpoint.
  * @param[in] xhci_ep XHCI endpoint to finalize.
  */
@@ -108 +110 @@
 }
 
-/** Determine the type of a XHCI endpoint.
+/**
+ * Determine the type of a XHCI endpoint.
  * @param[in] ep XHCI endpoint to query.
  *
@@ -137 +140 @@
 }
 
-/** Test whether an XHCI endpoint uses streams.
+/**
+ * Test whether an XHCI endpoint uses streams.
  * @param[in] xhci_ep XHCI endpoint to query.
  *

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

@@ -66 +66 @@
 /**
  * Walk the list of extended capabilities.
+ *
+ * The most interesting thing hidden in extended capabilities is the mapping of
+ * ports to protocol versions and speeds.
  */
 static int hc_parse_ec(xhci_hc_t *hc)
@@ -145 +148 @@
 }
 
+/**
+ * Initialize MMIO spaces of xHC.
+ */
 int hc_init_mmio(xhci_hc_t *hc, const hw_res_list_parsed_t *hw_res)
 {
@@ -195 +201 @@
 }
 
+/**
+ * Initialize structures kept in allocated memory.
+ */
 int hc_init_memory(xhci_hc_t *hc, ddf_dev_t *device)
 {
@@ -338 +347 @@
 }
 
+/**
+ * Claim xHC from BIOS. Implements handoff as per Section 4.22.1 of xHCI spec.
+ */
 int hc_claim(xhci_hc_t *hc, ddf_dev_t *dev)
 {
@@ -344 +356 @@
                 return EOK;
 
-        /* Section 4.22.1 */
         /* TODO: Test this with USB3-aware BIOS */
         usb_log_debug2("LEGSUP: bios: %x, os: %x", hc->legsup->sem_bios, hc->legsup->sem_os);
@@ -363 +374 @@
 }
 
+/**
+ * Ask the xHC to reset its state. Implements sequence 
+ */
 static int hc_reset(xhci_hc_t *hc)
 {
@@ -454 +468 @@
 }
 
-int hc_schedule(usb_transfer_batch_t *batch)
-{
-        assert(batch);
-        xhci_hc_t *hc = bus_to_hc(endpoint_get_bus(batch->ep));
-
-        if (!batch->target.address) {
-                usb_log_error("Attempted to schedule transfer to address 0.");
-                return EINVAL;
-        }
-
-        return xhci_transfer_schedule(hc, batch);
-}
-
 typedef int (*event_handler) (xhci_hc_t *, xhci_trb_t *trb);
 
@@ -484 +485 @@
 }
 
+/**
+ * Dequeue from event ring and handle dequeued events.
+ *
+ * As there can be events, that blocks on waiting for subsequent events,
+ * we solve this problem by first copying the event TRBs from the event ring,
+ * then asserting EHB and only after, handling the events.
+ *
+ * Whenever the event handling blocks, it switches fibril, and incoming
+ * IPC notification will create new event handling fibril for us.
+ */
 static void hc_run_event_ring(xhci_hc_t *hc, xhci_event_ring_t *event_ring, xhci_interrupter_regs_t *intr)
 {
@@ -541 +552 @@
 }
 
+/**
+ * Handle an interrupt request from xHC. Resolve all situations that trigger an
+ * interrupt separately.
+ *
+ * Note that all RW1C bits in USBSTS register are cleared at the time of
+ * handling the interrupt in irq_code. This method is the top-half.
+ *
+ * @param status contents of USBSTS register at the time of the interrupt.
+ */
 void hc_interrupt(bus_t *bus, uint32_t status)
 {
@@ -573 +593 @@
 }
 
+/**
+ * Tear down all in-memory structures.
+ */
 void hc_fini(xhci_hc_t *hc)
 {
@@ -585 +608 @@
 }
 
+/**
+ * Ring a xHC Doorbell. Implements section 4.7.
+ */
 int hc_ring_doorbell(xhci_hc_t *hc, unsigned doorbell, unsigned target)
 {
@@ -594 +620 @@
 }
 
+/**
+ * Issue an Enable Slot command, returning the obtained Slot ID.
+ *
+ * @param slot_id Pointer where to store the obtained Slot ID.
+ */
 int hc_enable_slot(xhci_hc_t *hc, uint32_t *slot_id)
 {
@@ -615 +646 @@
 }
 
+/**
+ * Issue a Disable Slot command for a slot occupied by device.
+ *
+ * Frees the device context 
+ */
 int hc_disable_slot(xhci_hc_t *hc, xhci_device_t *dev)
 {
@@ -634 +670 @@
 }
 
+/**
+ * Prepare an empty Endpoint Input Context inside a dma buffer.
+ */
 static int create_configure_ep_input_ctx(dma_buffer_t *dma_buf)
 {
@@ -649 +688 @@
 }
 
+/**
+ * Initialize a device, assigning it an address. Implements section 4.3.4.
+ *
+ * @param dev Device to assing an address (unconfigured yet)
+ * @param ep0 EP0 of device TODO remove, can be fetched from dev
+ */
 int hc_address_device(xhci_hc_t *hc, xhci_device_t *dev, xhci_endpoint_t *ep0)
 {
@@ -713 +758 @@
 }
 
+/**
+ * Issue a Configure Device command for a device in slot.
+ *
+ * @param slot_id Slot ID assigned to the device.
+ */
 int hc_configure_device(xhci_hc_t *hc, uint32_t slot_id)
 {
@@ -726 +776 @@
 }
 
+/**
+ * Issue a Deconfigure Device command for a device in slot.
+ *
+ * @param slot_id Slot ID assigned to the device.
+ */
 int hc_deconfigure_device(xhci_hc_t *hc, uint32_t slot_id)
 {
@@ -732 +787 @@
 }
 
+/**
+ * Instruct xHC to add an endpoint with supplied endpoint context.
+ *
+ * @param slot_id Slot ID assigned to the device.
+ * @param ep_idx Endpoint index (number + direction) in question
+ * @param ep_ctx Endpoint context of the endpoint
+ */
 int hc_add_endpoint(xhci_hc_t *hc, uint32_t slot_id, uint8_t ep_idx, xhci_ep_ctx_t *ep_ctx)
 {
@@ -748 +810 @@
 }
 
+/**
+ * Instruct xHC to drop an endpoint.
+ *
+ * @param slot_id Slot ID assigned to the device.
+ * @param ep_idx Endpoint index (number + direction) in question
+ */
 int hc_drop_endpoint(xhci_hc_t *hc, uint32_t slot_id, uint8_t ep_idx)
 {
@@ -763 +831 @@
 }
 
+/**
+ * Instruct xHC to update information about an endpoint, using supplied
+ * endpoint context.
+ *
+ * @param slot_id Slot ID assigned to the device.
+ * @param ep_idx Endpoint index (number + direction) in question
+ * @param ep_ctx Endpoint context of the endpoint
+ */
 int hc_update_endpoint(xhci_hc_t *hc, uint32_t slot_id, uint8_t ep_idx, xhci_ep_ctx_t *ep_ctx)
 {

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

@@ -115 +115 @@
 int hc_update_endpoint(xhci_hc_t *, uint32_t, uint8_t, xhci_ep_ctx_t *);
 
-int hc_schedule(usb_transfer_batch_t *batch);
 int hc_status(bus_t *, uint32_t *);
 void hc_interrupt(bus_t *, uint32_t);

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

@@ -61 +61 @@
         XHCI_REG_MASK(XHCI_PORT_CEC);
 
+/**
+ * Initialize the roothub subsystem.
+ */
 int xhci_rh_init(xhci_rh_t *rh, xhci_hc_t *hc)
 {
@@ -81 +84 @@
 }
 
-/** Create a device node for device directly connected to RH.
+/**
+ * Finalize the RH subsystem.
+ */
+int xhci_rh_fini(xhci_rh_t *rh)
+{
+        assert(rh);
+        free(rh->devices_by_port);
+        return EOK;
+}
+
+/**
+ * Create and setup a device directly connected to RH. As the xHCI is not using
+ * a virtual usbhub device for RH, this routine is called for devices directly.
  */
 static int rh_setup_device(xhci_rh_t *rh, uint8_t port_id)
@@ -89 +104 @@
 
         assert(rh->devices_by_port[port_id - 1] == NULL);
-
-        xhci_bus_t *bus = &rh->hc->bus;
 
         device_t *dev = hcd_ddf_fun_create(&rh->hc->base);
@@ -107 +120 @@
         dev->speed = port_speed->usb_speed;
 
-        if ((err = xhci_bus_enumerate_device(bus, dev))) {
+        if ((err = bus_device_enumerate(dev))) {
                 usb_log_error("Failed to enumerate USB device: %s", str_error(err));
                 return err;
@@ -134 +147 @@
 }
 
+/**
+ * Handle a device connection. USB 3+ devices are set up directly, USB 2 and
+ * below first need to have their port reset.
+ */
 static int handle_connected_device(xhci_rh_t *rh, uint8_t port_id)
 {
@@ -161 +178 @@
                 usb_log_debug("USB 2 device attached, issuing reset.");
                 xhci_rh_reset_port(rh, port_id);
-                /*
-                        FIXME: we need to wait for the event triggered by the reset
-                        and then alloc_dev()... can't it be done directly instead of
-                        going around?
-                */
                 return EOK;
         }
 }
 
-/** Deal with a detached device.
+/**
+ * Deal with a detached device.
  */
 static int handle_disconnected_device(xhci_rh_t *rh, uint8_t port_id)
@@ -194 +207 @@
 
         /* Remove device from XHCI bus. */
-        if ((err = xhci_bus_remove_device(&rh->hc->bus, &dev->base))) {
+        if ((err = bus_device_remove(&dev->base))) {
                 usb_log_warning("Failed to remove device " XHCI_DEV_FMT " from XHCI bus: %s",
                     XHCI_DEV_ARGS(*dev), str_error(err));
@@ -202 +215 @@
 }
 
-/** Handle an incoming Port Change Detected Event.
+/**
+ * Handle an incoming Port Change Detected Event.
  */
 int xhci_rh_handle_port_status_change_event(xhci_hc_t *hc, xhci_trb_t *trb)
@@ -219 +233 @@
 }
 
+/**
+ * Handle all changes on all ports.
+ */
 void xhci_rh_handle_port_change(xhci_rh_t *rh)
 {
@@ -303 +320 @@
 }
 
-static inline int get_hub_available_bandwidth(xhci_hc_t *hc, xhci_device_t* dev, uint8_t speed, xhci_port_bandwidth_ctx_t *ctx)
-{
-        int err = EOK;
-
-        // TODO: find a correct place for this function + API
-        // We need speed, because a root hub device has both USB 2 and USB 3 speeds
-        // and the command can query only one of them
-        // ctx is an out parameter as of now
-        assert(dev);
-        assert(ctx);
-
-        xhci_cmd_t cmd;
-        xhci_cmd_init(&cmd, XHCI_CMD_GET_PORT_BANDWIDTH);
-
-        if ((err = dma_buffer_alloc(&cmd.bandwidth_ctx, sizeof(xhci_port_bandwidth_ctx_t))))
-                goto end;
-
-        cmd.device_speed = speed;
-
-        if ((err = xhci_cmd_sync(hc, &cmd)))
-                goto end;
-
-        memcpy(ctx, cmd.bandwidth_ctx.virt, sizeof(xhci_port_bandwidth_ctx_t));
-
-end:
-        xhci_cmd_fini(&cmd);
-        return err;
-}
-
+/**
+ * Get a port speed for a given port id.
+ */
 const xhci_port_speed_t *xhci_rh_get_port_speed(xhci_rh_t *rh, uint8_t port)
 {
@@ -340 +331 @@
 }
 
+/**
+ * Issue a port reset for a given port.
+ */
 int xhci_rh_reset_port(xhci_rh_t* rh, uint8_t port)
 {
@@ -349 +343 @@
 }
 
-int xhci_rh_fini(xhci_rh_t *rh)
-{
-        assert(rh);
-        free(rh->devices_by_port);
-        return EOK;
-}
-
 /**
  * @}

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

@@ -41 +41 @@
 #include "scratchpad.h"
 
+/**
+ * Get the number of scratchpad buffers needed.
+ */
 static inline unsigned xhci_scratchpad_count(xhci_hc_t *hc)
 {
@@ -51 +54 @@
 }
 
+/**
+ * Allocate all scratchpad buffers, and configure the xHC.
+ */
 int xhci_scratchpad_alloc(xhci_hc_t *hc)
 {
@@ -79 +85 @@
 }
 
+/**
+ * Deallocate the scratchpads and deconfigure xHC.
+ */
 void xhci_scratchpad_free(xhci_hc_t *hc)
 {

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

@@ -94 +94 @@
 
 /**
- * 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)
+ * Create a xHCI-specific transfer batch.
+ *
+ * Bus callback.
+ */
+usb_transfer_batch_t * xhci_transfer_create(endpoint_t* ep)
 {
         xhci_transfer_t *transfer = calloc(1, sizeof(xhci_transfer_t));
@@ -105 +105 @@
 
         usb_transfer_batch_init(&transfer->batch, ep);
-        return transfer;
-}
-
-void xhci_transfer_destroy(xhci_transfer_t* transfer)
-{
-        assert(transfer);
+        return &transfer->batch;
+}
+
+/**
+ * Destroy a xHCI transfer.
+ */
+void xhci_transfer_destroy(usb_transfer_batch_t* batch)
+{
+        xhci_transfer_t *transfer = xhci_transfer_from_batch(batch);
 
         dma_buffer_free(&transfer->hc_buffer);

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

@@ -64 +64 @@
 } xhci_isoch_transfer_t;
 
-xhci_transfer_t* xhci_transfer_create(endpoint_t *);
+usb_transfer_batch_t* xhci_transfer_create(endpoint_t *);
 int xhci_transfer_schedule(xhci_hc_t *, usb_transfer_batch_t *);
 int xhci_handle_transfer_event(xhci_hc_t *, xhci_trb_t *);
-void xhci_transfer_destroy(xhci_transfer_t *);
+void xhci_transfer_destroy(usb_transfer_batch_t *);
 
 static inline xhci_transfer_t *xhci_transfer_from_batch(usb_transfer_batch_t *batch)

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

@@ -52 +52 @@
 
 
+/**
+ * Get the first TRB of a segment.
+ */
 static inline xhci_trb_t *segment_begin(trb_segment_t *segment)
 {
@@ -57 +60 @@
 }
 
+/**
+ * Get the one-past-end TRB of a segment.
+ */
 static inline xhci_trb_t *segment_end(trb_segment_t *segment)
 {
@@ -91 +97 @@
 /**
  * Initializes the ring with one segment.
- * Event when it fails, the structure needs to be finalized.
  */
 int xhci_trb_ring_init(xhci_trb_ring_t *ring)
@@ -117 +122 @@
         fibril_mutex_initialize(&ring->guard);
 
-        usb_log_debug2("Initialized new TRB ring.");
-
-        return EOK;
-}
-
+        return EOK;
+}
+
+/**
+ * Free all segments inside the ring.
+ */
 void xhci_trb_ring_fini(xhci_trb_ring_t *ring)
 {
@@ -150 +156 @@
 }
 
+/**
+ * Get the physical address of the enqueue pointer.
+ */
 static uintptr_t trb_ring_enqueue_phys(xhci_trb_ring_t *ring)
 {
@@ -156 +165 @@
 }
 
+/**
+ * Decides whether the TRB will trigger an interrupt after being processed.
+ */
 static bool trb_generates_interrupt(xhci_trb_t *trb)
 {
@@ -163 +175 @@
 
 /**
- * Enqueue TDs composed of TRBs.
+ * Enqueue TD composed of TRBs.
  *
  * This will copy specified number of TRBs chained together into the ring. The
@@ -256 +268 @@
 /**
  * Initializes an event ring.
- * Even when it fails, the structure needs to be finalized.
  */
 int xhci_event_ring_init(xhci_event_ring_t *ring)
@@ -275 +286 @@
         ring->dequeue_ptr = segment->phys;
 
-        if (dma_buffer_alloc(&ring->erst, PAGE_SIZE))
+        if (dma_buffer_alloc(&ring->erst, PAGE_SIZE)) {
+                xhci_event_ring_fini(ring);
                 return ENOMEM;
+        }
         xhci_erst_entry_t *erst = ring->erst.virt;
 
@@ -301 +314 @@
 }
 
+/**
+ * Get the physical address of the dequeue pointer.
+ */
 static uintptr_t event_ring_dequeue_phys(xhci_event_ring_t *ring)
 {