Changeset 0f79283b in mainline

Timestamp:

2018-01-18T12:39:27Z (7 years ago)

Author:

Ondřej Hlavatý <aearsis@…>

Branches:

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

Children:

2bff2cc2

Parents:

babcc423

Message:

usb: remove speed storing (and fix that misuse in xhci rh)

Location:

uspace

Files:

• drv/bus/usb/usbhub/port.c (modified) (2 diffs)
• drv/bus/usb/usbhub/port.h (modified) (1 diff)
• drv/bus/usb/xhci/rh.c (modified) (2 diffs)
• lib/usb/include/usb/port.h (modified) (3 diffs)
• lib/usb/src/port.c (modified) (2 diffs)

uspace/drv/bus/usb/usbhub/port.c ¶

@@ -134 +134 @@
         port_log(debug, port, "Port reset, enumerating device.");
 
-        if ((err = usbhc_device_enumerate(exch, port->port_number, port->base.speed))) {
+        if ((err = usbhc_device_enumerate(exch, port->port_number, port->speed))) {
                 port_log(error, port, "Failed to enumerate device: %s", str_error(err));
                 /* Disable the port */
@@ -200 +200 @@
         const bool enabled = !!(status & USB_HUB_PORT_STATUS_ENABLED);
 
-        if (enabled)
-                usb_port_enabled(&port->base, usb_port_speed(status));
-        else
+        if (enabled) {
+                // The connecting fibril do not touch speed until the port is enabled,
+                // so we do not have to lock
+                port->speed = usb_port_speed(status);
+                usb_port_enabled(&port->base);
+        } else
                 usb_port_disabled(&port->base, &remove_device);
 }

uspace/drv/bus/usb/usbhub/port.h ¶

@@ -52 +52 @@
         /** Port number as reported in descriptors. */
         unsigned int port_number;
+        /** Speed at the time of enabling the port */
+        usb_speed_t speed;
 } usb_hub_port_t;
 

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

@@ -136 +136 @@
         }
 
-        device_t *dev = hcd_ddf_fun_create(&port->rh->hc->base, port->base.speed);
+        /*
+         * We cannot know in advance, whether the speed in the status register
+         * is valid - it depends on the protocol. So we read it later, but then
+         * we have to check if the port is still enabled.
+         */
+        uint32_t status = XHCI_REG_RD_FIELD(&port->regs->portsc, 32);
+
+        bool enabled = !!(status & XHCI_REG_MASK(XHCI_PORT_PED));
+        if (!enabled)
+                return ENOENT;
+
+        unsigned psiv = (status & XHCI_REG_MASK(XHCI_PORT_PS)) >> XHCI_REG_SHIFT(XHCI_PORT_PS);
+        const usb_speed_t speed = port->rh->hc->speeds[psiv].usb_speed;
+
+        device_t *dev = hcd_ddf_fun_create(&port->rh->hc->base, speed);
         if (!dev) {
                 usb_log_error("Failed to create USB device function.");
@@ -223 +237 @@
 
                         if (enabled) {
-                                unsigned psiv = (status & XHCI_REG_MASK(XHCI_PORT_PS)) >> XHCI_REG_SHIFT(XHCI_PORT_PS);
-                                const usb_speed_t speed = rh->hc->speeds[psiv].usb_speed;
-                                usb_port_enabled(&port->base, speed);
+                                usb_port_enabled(&port->base);
                         } else {
                                 usb_port_disabled(&port->base, &rh_remove_device);

uspace/lib/usb/include/usb/port.h ¶

@@ -41 +41 @@
  *
  * This subsystem abstracts the rather complicated state machine, and offers
- * a simple call interface to announce events, and a callback structure for
- * implementations to supply the hardware-dependent part.
+ * a simple interface to announce events and leave the fibril management on the
+ * library.
  */
 
@@ -65 +65 @@
         /** Current state of the port */
         usb_port_state_t state;
-        /** A speed of the device connected (if any). Valid unless state == PORT_DISABLED. */
-        usb_speed_t speed;
         /** CV signalled on fibril exit. */
         fibril_condvar_t finished_cv;
@@ -87 +85 @@
 void usb_port_init(usb_port_t *);
 int usb_port_connected(usb_port_t *, usb_port_enumerate_t);
-void usb_port_enabled(usb_port_t *, usb_speed_t);
+void usb_port_enabled(usb_port_t *);
 void usb_port_disabled(usb_port_t *, usb_port_remove_t);
 void usb_port_fini(usb_port_t *);

uspace/lib/usb/src/port.c ¶

@@ -41 +41 @@
  *
  * This subsystem abstracts the rather complicated state machine, and offers
- * a simple call interface to announce events, and a callback structure for
- * implementations to supply the hardware-dependent part.
+ * a simple interface to announce events and leave the fibril management on the
+ * library.
  */
 
@@ -127 +127 @@
 }
 
-void usb_port_enabled(usb_port_t *port, usb_speed_t speed)
-{
-        assert(port);
-
-        fibril_mutex_lock(&port->guard);
-        port->speed = speed;
+void usb_port_enabled(usb_port_t *port)
+{
+        assert(port);
+
+        fibril_mutex_lock(&port->guard);
         fibril_condvar_broadcast(&port->enabled_cv);
         fibril_mutex_unlock(&port->guard);