Changeset 867b375 in mainline for uspace/drv/bus/usb/xhci

Timestamp:

2017-10-15T02:04:10Z (8 years ago)

Author:

Ondřej Hlavatý <aearsis@…>

Branches:

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

Children:

20eaa82

Parents:

d7869d7e

Message:

hcd_ddf_new_device refactoring

This long function is now split into parts. Instead of passing dozens of arguments, it now creates the usb_dev_t right away, and uses that to pass it along. The address_device part is now modifiable by drivers.

There is still a work to be done. The biggest problem I see is in the addressing - currently, there is usb_address_t, and for high speed transaction translating there is another address. For (near) future extensibility, we should pass address as a structure. Or even better, make a way how to reference a device, maybe in a similar way how we work with endpoints.

Location:

uspace/drv/bus/usb/xhci

Files:

• main.c (modified) (6 diffs)
• rh.c (modified) (7 diffs)
• rh.h (modified) (3 diffs)

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

@@ -42 +42 @@
 
 #include "hc.h"
+#include "rh.h"
 #include "endpoint.h"
 
@@ -50 +51 @@
 static int hcd_claim(hcd_t *, ddf_dev_t *);
 static int hcd_start(hcd_t *, bool);
+static int hcd_setup_root_hub(hcd_t *, ddf_dev_t *);
 static int hcd_status(hcd_t *, uint32_t *);
 static void hcd_interrupt(hcd_t *, uint32_t);
 static int hcd_schedule(hcd_t *, usb_transfer_batch_t *);
+static int hcd_address_device(hcd_t *, usb_speed_t, usb_tt_address_t, usb_address_t *);
 static void hc_driver_fini(hcd_t *);
 
@@ -62 +65 @@
         .claim = hcd_claim,
         .start = hcd_start,
+        .setup_root_hub = hcd_setup_root_hub,
         .fini = hc_driver_fini,
         .ops = {
@@ -67 +71 @@
                 .irq_hook       = hcd_interrupt,
                 .status_hook    = hcd_status,
+                .address_device = hcd_address_device,
         }
 };
@@ -116 +121 @@
 }
 
+static int hcd_setup_root_hub(hcd_t *hcd, ddf_dev_t *dev)
+{
+        xhci_hc_t *hc = hcd_get_driver_data(hcd);
+        assert(hc);
+
+        hc->rh.hcd_rh = hcd_roothub_create(hcd, dev, USB_SPEED_SUPER);
+        return hc->rh.hcd_rh ? EOK : ENOMEM;
+}
+
 static int hcd_schedule(hcd_t *hcd, usb_transfer_batch_t *batch)
 {
@@ -139 +153 @@
 
         hc_interrupt(hc, status);
+}
+
+static int hcd_address_device(hcd_t *hcd, usb_speed_t speed, usb_tt_address_t tt, usb_address_t *address)
+{
+        xhci_hc_t *hc = hcd_get_driver_data(hcd);
+        assert(hc);
+
+        return xhci_rh_address_device(&hc->rh, speed, tt, address);
 }
 

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

@@ -38 +38 @@
 #include <usb/debug.h>
 #include <usb/host/utils/malloc32.h>
+#include <usb/host/ddf_helpers.h>
+
 #include "debug.h"
 #include "commands.h"
@@ -69 +71 @@
 // TODO: Check device deallocation, we free device_ctx in hc.c, not
 //       sure about the other structs.
-static int alloc_dev(xhci_hc_t *hc, uint8_t port, uint32_t route_str)
+// 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, usb_speed_t unused_speed, usb_tt_address_t tt, usb_address_t *address)
 {
         int err;
+        xhci_hc_t *hc = rh->hc;
 
         xhci_cmd_t cmd;
         xhci_cmd_init(&cmd);
 
-        const xhci_port_speed_t *speed = xhci_rh_get_port_speed(&hc->rh, port);
+        uint8_t port = tt.port;
+
+        /* XXX Certainly not generic solution. */
+        uint32_t route_str = 0;
+
+        const xhci_port_speed_t *speed = xhci_rh_get_port_speed(rh, port);
 
         xhci_send_enable_slot_command(hc, &cmd);
@@ -127 +137 @@
         XHCI_EP_ERROR_COUNT_SET(ictx->endpoint_ctx[0], 3);
 
-        // TODO: What's the alignment?
-        xhci_device_ctx_t *dctx = malloc(sizeof(xhci_device_ctx_t));
+        xhci_device_ctx_t *dctx = malloc32(sizeof(xhci_device_ctx_t));
         if (!dctx) {
                 err = ENOMEM;
@@ -149 +158 @@
         xhci_cmd_fini(&cmd);
 
-        usb_address_t address = XHCI_SLOT_DEVICE_ADDRESS(dctx->slot_ctx);
-        usb_log_debug2("Obtained USB address: %d.\n", address);
+        *address = XHCI_SLOT_DEVICE_ADDRESS(dctx->slot_ctx);
+        usb_log_debug2("Obtained USB address: %d.\n", *address);
 
         // TODO: Ask libusbhost to create a control endpoint for EP0.
@@ -174 +183 @@
 }
 
+static int rh_setup_device(xhci_rh_t *rh, uint8_t port_id)
+{
+        /** This should ideally use the libusbhost in a clean and elegant way,
+         * to create child function. The refactoring of libusbhost is not over
+         * yet, so for now it is still quirky.
+         */
+
+        return hcd_roothub_new_device(rh->hcd_rh, port_id);
+}
+
 static int handle_connected_device(xhci_rh_t *rh, uint8_t port_id)
 {
@@ -186 +205 @@
                 if (link_state == 0) {
                         /* USB3 is automatically advanced to enabled. */
-                        return alloc_dev(rh->hc, port_id, 0);
+                        return rh_setup_device(rh, port_id);
                 }
                 else if (link_state == 5) {
@@ -270 +289 @@
                                  * every time USB2 port is reset. This is a
                                  * temporary workaround. */
-                                alloc_dev(rh->hc, i, 0);
+                                rh_setup_device(rh, i);
                         }
                 }

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

@@ -51 +51 @@
 } xhci_port_speed_t;
 
+typedef struct hcd_roothub hcd_roothub_t;
+
 /* XHCI root hub instance */
 typedef struct {
@@ -64 +66 @@
         /* Number of hub ports. */
         uint8_t max_ports;
+
+        /* We need this to create child devices */
+        hcd_roothub_t *hcd_rh;
 } xhci_rh_t;
 
@@ -74 +79 @@
 void xhci_rh_handle_port_change(xhci_rh_t *);
 
+int xhci_rh_address_device(xhci_rh_t *, usb_speed_t, usb_tt_address_t, usb_address_t *);
+
 #endif