Changeset 747ef72 in mainline for uspace/lib/usbdev/src/hub.c

Timestamp:

2011-11-10T11:29:10Z (13 years ago)

Author:

Jan Vesely <jano.vesely@…>

Branches:

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

Children:

54464f6a, c2245a3, c6f189f7

Parents:

2e1b9dc (diff), 2d1ba51 (diff)
Note: this is a merge changeset, the changes displayed below correspond to the merge itself.
Use the (diff) links above to see all the changes relative to each parent.

Message:

Merge USB changes.

Interface changes:

• GET_ADDRESS has been renamed to GET_MY_ADDRESS and the handle parameter was dropped. Tis call no longer cascades up to the root hub, but it is answered in the first place the information is available (nearest hub)
• Reintroduced address reservation for USB_DEFAULT_ADDRESS. The interface now enables device drivers to request specific address on initialization and either insists on that address or accept any other if the address is not available. Note that it is not possible to get the default address if the driver does not insist.
• Any endpoint registered is removed when address is released and a warning is produced if there were any such endpoints.
• It is no longer necessary or possible to pass device speed information when registering endpoints.

Driver fixes: memory leaks and crashes (not only) in error paths.
Fixes or removes flaky device_remove implementation in device drivers.

File:

• uspace/lib/usbdev/src/hub.c (modified) (15 diffs)

uspace/lib/usbdev/src/hub.c

@@ -66 +66 @@
  *
  * @param connection Opened connection to host controller.
+ * @param preferred Preferred SUB address.
+ * @param strict Fail if the preferred address is not avialable.
  * @param speed Speed of the new device (device that will be assigned
  *    the returned address).
@@ -71 +73 @@
  */
 usb_address_t usb_hc_request_address(usb_hc_connection_t *connection,
-    usb_speed_t speed)
+    usb_address_t preferred, bool strict, usb_speed_t speed)
 {
         CHECK_CONNECTION(connection);
@@ -78 +80 @@
         
         sysarg_t address;
-        int rc = async_req_2_1(exch, DEV_IFACE_ID(USBHC_DEV_IFACE),
-            IPC_M_USBHC_REQUEST_ADDRESS, speed,
-            &address);
+        int rc = async_req_4_1(exch, DEV_IFACE_ID(USBHC_DEV_IFACE),
+            IPC_M_USBHC_REQUEST_ADDRESS, preferred, strict, speed, &address);
         
         async_exchange_end(exch);
@@ -132 +133 @@
 }
 
-
-static void unregister_control_endpoint_on_default_address(
-    usb_hc_connection_t *connection)
+/** Change address of connected device.
+ * This function automatically updates the backing connection to point to
+ * the new address. It also unregisterrs the old endpoint and registers
+ * a new one.
+ * This creates whole bunch of problems:
+ *  1. All pipes using this wire are broken because they are not
+ *     registered for new address
+ *  2. All other pipes for this device are using wrong address,
+ *     possibly targeting completely different device
+ *
+ * @param pipe Control endpoint pipe (session must be already started).
+ * @param new_address New USB address to be set (in native endianness).
+ * @return Error code.
+ */
+static int usb_request_set_address(usb_pipe_t *pipe, usb_address_t new_address,
+    usb_hc_connection_t *hc_conn)
 {
-        usb_device_connection_t dev_conn;
-        int rc = usb_device_connection_initialize_on_default_address(&dev_conn,
-            connection);
-        if (rc != EOK) {
-                return;
-        }
-
-        usb_pipe_t ctrl_pipe;
-        rc = usb_pipe_initialize_default_control(&ctrl_pipe, &dev_conn);
-        if (rc != EOK) {
-                return;
-        }
-
-        usb_pipe_unregister(&ctrl_pipe, connection);
+        if ((new_address < 0) || (new_address >= USB11_ADDRESS_MAX)) {
+                return EINVAL;
+        }
+        assert(pipe);
+        assert(hc_conn);
+        assert(pipe->wire != NULL);
+
+        const uint16_t addr = uint16_host2usb((uint16_t)new_address);
+
+        int rc = usb_control_request_set(pipe,
+            USB_REQUEST_TYPE_STANDARD, USB_REQUEST_RECIPIENT_DEVICE,
+            USB_DEVREQ_SET_ADDRESS, addr, 0, NULL, 0);
+
+        if (rc != EOK) {
+                return rc;
+        }
+
+        /* TODO: prevent others from accessing the wire now. */
+        if (usb_pipe_unregister(pipe, hc_conn) != EOK) {
+                usb_log_warning(
+                    "Failed to unregister the old pipe on address change.\n");
+        }
+        /* The address is already changed so set it in the wire */
+        pipe->wire->address = new_address;
+        rc = usb_pipe_register(pipe, 0, hc_conn);
+        if (rc != EOK)
+                return EADDRNOTAVAIL;
+
+        return EOK;
 }
 
@@ -171 +200 @@
  *
  * @param[in] parent Parent device (i.e. the hub device).
- * @param[in] connection Connection to host controller.
+ * @param[in] connection Connection to host controller. Must be non-null.
  * @param[in] dev_speed New device speed.
  * @param[in] enable_port Function for enabling signaling through the port the
@@ -177 +206 @@
  * @param[in] arg Any data argument to @p enable_port.
  * @param[out] assigned_address USB address of the device.
- * @param[in] dev_ops Child device ops.
+ * @param[in] dev_ops Child device ops. Will use default if not provided.
  * @param[in] new_dev_data Arbitrary pointer to be stored in the child
- *      as @c driver_data.
+ *      as @c driver_data. Will allocate and assign usb_hub_attached_device_t
+ *      structure if NULL.
  * @param[out] new_fun Storage where pointer to allocated child function
- *      will be written.
+ *      will be written. Must be non-null.
  * @return Error code.
+ * @retval EINVAL Either connection or new_fun is a NULL pointer.
  * @retval ENOENT Connection to HC not opened.
  * @retval EADDRNOTAVAIL Failed retrieving free address from host controller.
@@ -195 +226 @@
     ddf_dev_ops_t *dev_ops, void *new_dev_data, ddf_fun_t **new_fun)
 {
-        assert(connection != NULL);
+        if (new_fun == NULL || connection == NULL)
+                return EINVAL;
+
         // FIXME: this is awful, we are accessing directly the structure.
+        // TODO: Why not use provided connection?
         usb_hc_connection_t hc_conn = {
                 .hc_handle = connection->hc_handle,
@@ -215 +249 @@
         }
 
-
         /*
          * Request new address.
          */
-        usb_address_t dev_addr = usb_hc_request_address(&hc_conn, dev_speed);
+        usb_address_t dev_addr =
+            usb_hc_request_address(&hc_conn, 0, false, dev_speed);
         if (dev_addr < 0) {
                 rc = EADDRNOTAVAIL;
@@ -240 +274 @@
 
         usb_pipe_t ctrl_pipe;
-        rc = usb_pipe_initialize_default_control(&ctrl_pipe,
-            &dev_conn);
+        rc = usb_pipe_initialize_default_control(&ctrl_pipe, &dev_conn);
         if (rc != EOK) {
                 rc = ENOTCONN;
@@ -248 +281 @@
 
         do {
-                rc = usb_pipe_register_with_speed(&ctrl_pipe, dev_speed, 0,
-                    &hc_conn);
-                if (rc != EOK) {
+                rc = usb_hc_request_address(&hc_conn, USB_ADDRESS_DEFAULT,
+                    true, dev_speed);
+                if (rc == ENOENT) {
                         /* Do not overheat the CPU ;-). */
                         async_usleep(ENDPOINT_0_0_REGISTER_ATTEMPT_DELAY_USEC);
                 }
-        } while (rc != EOK);
+        } while (rc == ENOENT);
+        if (rc < 0) {
+                goto leave_release_free_address;
+        }
+
+        /* Register control pipe on default address. */
+        rc = usb_pipe_register(&ctrl_pipe, 0, &hc_conn);
+        if (rc != EOK) {
+                rc = ENOTCONN;
+                goto leave_release_default_address;
+        }
+
         struct timeval end_time;
 
@@ -267 +311 @@
          * above might use much of this time so we should only wait to fill
          * up the 100ms quota*/
-        suseconds_t elapsed = tv_sub(&end_time, &start_time);
+        const suseconds_t elapsed = tv_sub(&end_time, &start_time);
         if (elapsed < 100000) {
                 async_usleep(100000 - elapsed);
         }
 
-        /*
-         * Endpoint is registered. We can enable the port and change
-         * device address.
-         */
+        /* Endpoint is registered. We can enable the port and change address. */
         rc = enable_port(arg);
         if (rc != EOK) {
@@ -287 +328 @@
         async_usleep(10000);
 
+        /* Get max_packet_size value. */
         rc = usb_pipe_probe_default_control(&ctrl_pipe);
         if (rc != EOK) {
@@ -293 +335 @@
         }
 
-        rc = usb_request_set_address(&ctrl_pipe, dev_addr);
+        rc = usb_request_set_address(&ctrl_pipe, dev_addr, &hc_conn);
         if (rc != EOK) {
                 rc = ESTALL;
@@ -299 +341 @@
         }
 
-        /*
-         * Address changed. We can release the original endpoint, thus
-         * allowing other to access the default address.
-         */
-        unregister_control_endpoint_on_default_address(&hc_conn);
-
-        /*
-         * Time to register the new endpoint.
-         */
-        rc = usb_pipe_register(&ctrl_pipe, 0, &hc_conn);
-        if (rc != EOK) {
-                goto leave_release_free_address;
-        }
-
-        /*
-         * It is time to register the device with devman.
-         */
+        /* Address changed. We can release the default, thus
+         * allowing other to access the default address. */
+        usb_hc_unregister_device(&hc_conn, USB_ADDRESS_DEFAULT);
+
+        /* Register the device with devman. */
         /* FIXME: create device_register that will get opened ctrl pipe. */
         ddf_fun_t *child_fun;
-        rc = usb_device_register_child_in_devman(dev_addr, dev_conn.hc_handle,
+        rc = usb_device_register_child_in_devman(&ctrl_pipe,
             parent, dev_ops, new_dev_data, &child_fun);
         if (rc != EOK) {
-                rc = ESTALL;
                 goto leave_release_free_address;
         }
 
-        /*
-         * And now inform the host controller about the handle.
-         */
-        usb_hub_attached_device_t new_device = {
+        const usb_hub_attached_device_t new_device = {
                 .address = dev_addr,
                 .fun = child_fun,
         };
+
+
+        /* Inform the host controller about the handle. */
         rc = usb_hc_register_device(&hc_conn, &new_device);
         if (rc != EOK) {
+                /* We know nothing about that data. */
+                if (new_dev_data)
+                        child_fun->driver_data = NULL;
+                /* The child function is already created. */
+                ddf_fun_destroy(child_fun);
                 rc = EDESTADDRREQ;
                 goto leave_release_free_address;
         }
 
-
-        /*
-         * And we are done.
-         */
         if (assigned_address != NULL) {
                 *assigned_address = dev_addr;
         }
-        if (new_fun != NULL) {
-                *new_fun = child_fun;
-        }
+
+        *new_fun = child_fun;
 
         rc = EOK;
@@ -357 +386 @@
          */
 leave_release_default_address:
-        usb_pipe_unregister(&ctrl_pipe, &hc_conn);
+        usb_hc_unregister_device(&hc_conn, USB_ADDRESS_DEFAULT);
 
 leave_release_free_address:
-        usb_hc_unregister_device(&hc_conn, dev_addr);
+        /* This might be either 0:0 or dev_addr:0 */
+        if (usb_pipe_unregister(&ctrl_pipe, &hc_conn) != EOK)
+                usb_log_warning("%s: Failed to unregister default pipe.\n",
+                    __FUNCTION__);
+
+        if (usb_hc_unregister_device(&hc_conn, dev_addr) != EOK)
+                usb_log_warning("%s: Failed to unregister device.\n",
+                    __FUNCTION__);
 
 close_connection:
         if (usb_hc_connection_close(&hc_conn) != EOK)
-                usb_log_warning("usb_hc_new_device_wrapper(): Failed to close "
-                    "connection.\n");
+                usb_log_warning("%s: Failed to close hc connection.\n",
+                    __FUNCTION__);
 
         return rc;