Changeset 49e62998 in mainline

Timestamp:

2018-01-11T23:06:57Z (7 years ago)

Author:

Ondřej Hlavatý <aearsis@…>

Branches:

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

Children:

50be3c6

Parents:

f3baab1

git-author:

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

git-committer:

Ondřej Hlavatý <aearsis@…> (2018-01-11 23:06:57)

Message:

xhci: defer roothub events processing

There are two new generic mechanisms, which are used in only one
instance.

First one is a simple wrapper for roothub event handlers that should run
in separate fibril. The usage can be seen at line 337, for example. The
mechanism just passes arguments to a newly created fibril.

The second is a bit more complex, and its purpose is to broadcast roothub
events to all fibrils that could wait for it. It's used to wait for port
reset complete at USB 2 ports. See rh_port_reset_sync.

Sorry for naming both mechanisms "rh_event", which could create
a confusion.

Location:

uspace/drv/bus/usb/xhci

Files:

• rh.c (modified) (7 diffs)
• rh.h (modified) (2 diffs)

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

@@ -81 +81 @@
         rh->device.tier = 0;
 
+        fibril_mutex_initialize(&rh->event_guard);
+        fibril_condvar_initialize(&rh->event_ready);
+        fibril_condvar_initialize(&rh->event_handled);
+
         return EOK;
 }
@@ -92 +96 @@
         free(rh->devices_by_port);
         return EOK;
+}
+
+static int rh_event_wait_timeout(xhci_rh_t *rh, suseconds_t timeout)
+{
+        assert(fibril_mutex_is_locked(&rh->event_guard));
+
+        ++rh->event_readers_waiting;
+        const int r = fibril_condvar_wait_timeout(&rh->event_ready, &rh->event_guard, timeout);
+        --rh->event_readers_waiting;
+        if (--rh->event_readers_to_go == 0)
+                fibril_condvar_broadcast(&rh->event_handled);
+        return r;
+}
+
+static void rh_event_run_handlers(xhci_rh_t *rh)
+{
+        fibril_mutex_lock(&rh->event_guard);
+        assert(rh->event_readers_to_go == 0);
+
+        rh->event_readers_to_go = rh->event_readers_waiting;
+        fibril_condvar_broadcast(&rh->event_ready);
+        while (rh->event_readers_to_go)
+                fibril_condvar_wait(&rh->event_handled, &rh->event_guard);
+        fibril_mutex_unlock(&rh->event_guard);
 }
 
@@ -147 +175 @@
 }
 
+
+static int rh_port_reset_sync(xhci_rh_t *rh, uint8_t port_id)
+{
+        int r;
+        xhci_port_regs_t *regs = &rh->hc->op_regs->portrs[port_id - 1];
+
+        fibril_mutex_lock(&rh->event_guard);
+        XHCI_REG_SET(regs, XHCI_PORT_PR, 1);
+
+        while (true) {
+                r = rh_event_wait_timeout(rh, 0);
+                if (r != EOK)
+                        break;
+                if (rh->event.port_id == port_id
+                    && rh->event.events & XHCI_REG_MASK(XHCI_PORT_PRC))
+                        break;
+        }
+        fibril_mutex_unlock(&rh->event_guard);
+
+        return r;
+}
+
 /**
  * Handle a device connection. USB 3+ devices are set up directly, USB 2 and
@@ -177 +227 @@
         else {
                 usb_log_debug("USB 2 device attached, issuing reset.");
-                xhci_rh_reset_port(rh, port_id);
+                const int err = rh_port_reset_sync(rh, port_id);
+                if (err)
+                        return err;
+
+                rh_setup_device(rh, port_id);
                 return EOK;
         }
@@ -229 +283 @@
 }
 
+typedef int (*rh_event_handler_t)(xhci_rh_t *, uint8_t);
+
+typedef struct rh_event_args {
+        xhci_rh_t *rh;
+        uint8_t port_id;
+        rh_event_handler_t handler;
+} rh_event_args_t;
+
+static int rh_event_handler_fibril(void *arg) {
+        rh_event_args_t *rh_args = arg;
+        xhci_rh_t *rh = rh_args->rh;
+        uint8_t port_id = rh_args->port_id;
+        rh_event_handler_t handler = rh_args->handler;
+
+        free(rh_args);
+
+        return handler(rh, port_id);
+}
+
+static fid_t handle_in_fibril(xhci_rh_t *rh, uint8_t port_id, rh_event_handler_t handler)
+{
+        rh_event_args_t *args = malloc(sizeof(*args));
+        *args = (rh_event_args_t) {
+                .rh = rh,
+                .port_id = port_id,
+                .handler = handler,
+        };
+
+        const fid_t fid = fibril_create(rh_event_handler_fibril, args);
+        fibril_add_ready(fid);
+        return fid;
+}
+
 /**
  * Handle all changes on all ports.
@@ -248 +335 @@
                         bool connected = XHCI_REG_RD(regs, XHCI_PORT_CCS);
                         if (connected) {
-                                handle_connected_device(rh, i);
+                                handle_in_fibril(rh, i, handle_connected_device);
                         } else {
-                                handle_disconnected_device(rh, i);
+                                handle_in_fibril(rh, i, handle_disconnected_device);
                         }
                 }
 
-                if (events & XHCI_REG_MASK(XHCI_PORT_PEC)) {
-                        usb_log_info("Port enabled changed on port %u.", i);
-                        events &= ~XHCI_REG_MASK(XHCI_PORT_PEC);
-                }
-
-                if (events & XHCI_REG_MASK(XHCI_PORT_WRC)) {
-                        usb_log_info("Warm port reset on port %u completed.", i);
-                        events &= ~XHCI_REG_MASK(XHCI_PORT_WRC);
-                }
-
-                if (events & XHCI_REG_MASK(XHCI_PORT_OCC)) {
-                        usb_log_info("Over-current change on port %u.", i);
-                        events &= ~XHCI_REG_MASK(XHCI_PORT_OCC);
-                }
-
-                if (events & XHCI_REG_MASK(XHCI_PORT_PRC)) {
-                        usb_log_info("Port reset on port %u completed.", i);
-                        events &= ~XHCI_REG_MASK(XHCI_PORT_PRC);
-
-                        const xhci_port_speed_t *speed = xhci_rh_get_port_speed(rh, i);
-                        if (speed->major != 3) {
-                                /* FIXME: We probably don't want to do this
-                                 * every time USB2 port is reset. This is a
-                                 * temporary workaround. */
-                                rh_setup_device(rh, i);
-                        }
-                }
-
-                if (events & XHCI_REG_MASK(XHCI_PORT_PLC)) {
-                        usb_log_info("Port link state changed on port %u.", i);
-                        events &= ~XHCI_REG_MASK(XHCI_PORT_PLC);
-                }
-
-                if (events & XHCI_REG_MASK(XHCI_PORT_CEC)) {
-                        usb_log_info("Port %u failed to configure link.", i);
-                        events &= ~XHCI_REG_MASK(XHCI_PORT_CEC);
-                }
-
-                if (events) {
-                        usb_log_warning("Port change (0x%08x) ignored on port %u.", events, i);
+                if (events != 0) {
+                        rh->event.port_id = i;
+                        rh->event.events = events;
+                        rh_event_run_handlers(rh);
                 }
         }
@@ -333 +384 @@
 {
         usb_log_debug2("Resetting port %u.", port);
-        xhci_port_regs_t *regs = &rh->hc->op_regs->portrs[port-1];
-        XHCI_REG_SET(regs, XHCI_PORT_PR, 1);
 
         return EOK;

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

@@ -75 +75 @@
         /* Device pointers connected to RH ports or NULL. (size is `max_ports`) */
         xhci_device_t **devices_by_port;
+
+        /* Roothub events. */
+        fibril_mutex_t event_guard;
+        fibril_condvar_t event_ready, event_handled;
+        unsigned event_readers_waiting, event_readers_to_go;
+        struct {
+                uint8_t port_id;
+                uint32_t events;
+        } event;
 } xhci_rh_t;
 
@@ -80 +89 @@
 int xhci_rh_fini(xhci_rh_t *);
 const xhci_port_speed_t *xhci_rh_get_port_speed(xhci_rh_t *, uint8_t);
-int xhci_rh_reset_port(xhci_rh_t *, uint8_t);
 
 int xhci_rh_handle_port_status_change_event(xhci_hc_t *, xhci_trb_t *);