Changeset 747ef72 in mainline for uspace/lib/usbdev/src/devdrv.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/devdrv.c (modified) (15 diffs)

uspace/lib/usbdev/src/devdrv.c

@@ -81 +81 @@
  * @return Number of pipes (excluding default control pipe).
  */
-static size_t count_other_pipes(const usb_endpoint_description_t **endpoints)
-{
-        size_t count = 0;
-        if (endpoints == NULL) {
-                return 0;
-        }
-
-        while (endpoints[count] != NULL) {
-                count++;
-        }
-
+static inline size_t count_other_pipes(
+    const usb_endpoint_description_t **endpoints)
+{
+        size_t count;
+        for (count = 0; endpoints && endpoints[count] != NULL; ++count);
         return count;
 }
@@ -104 +98 @@
     usb_device_t *dev, int alternate_setting)
 {
+        assert(dev);
+
         if (endpoints == NULL) {
                 dev->pipes = NULL;
@@ -300 +296 @@
 
         return rc;
+}
+
+/** Cleanup structure initialized via usb_device_retrieve_descriptors.
+ *
+ * @param[in] descriptors Where to store the descriptors.
+ */
+void usb_device_release_descriptors(usb_device_descriptors_t *descriptors)
+{
+        assert(descriptors);
+        free(descriptors->configuration);
+        descriptors->configuration = NULL;
 }
 
@@ -319 +326 @@
  *      (not NULL terminated).
  * @param[out] pipes_count_ptr Where to store number of pipes
- *      (set to if you wish to ignore the count).
+ *      (set to NULL if you wish to ignore the count).
  * @return Error code.
  */
@@ -340 +347 @@
         const size_t pipe_count = count_other_pipes(endpoints);
         if (pipe_count == 0) {
-                *pipes_count_ptr = pipe_count;
+                if (pipes_count_ptr)
+                        *pipes_count_ptr = pipe_count;
                 *pipes_ptr = NULL;
                 return EOK;
@@ -346 +354 @@
 
         usb_endpoint_mapping_t *pipes
-            = malloc(sizeof(usb_endpoint_mapping_t) * pipe_count);
+            = calloc(pipe_count, sizeof(usb_endpoint_mapping_t));
         if (pipes == NULL) {
                 return ENOMEM;
         }
 
-        /* Initialize to NULL to allow smooth rollback. */
-        for (i = 0; i < pipe_count; i++) {
-                pipes[i].pipe = NULL;
-        }
-
         /* Now allocate and fully initialize. */
         for (i = 0; i < pipe_count; i++) {
-                pipes[i].pipe = malloc(sizeof(usb_pipe_t));
-                if (pipes[i].pipe == NULL) {
-                        rc = ENOMEM;
-                        goto rollback_free_only;
-                }
                 pipes[i].description = endpoints[i];
                 pipes[i].interface_no = interface_no;
@@ -389 +387 @@
         for (i = 0; i < pipe_count; i++) {
                 if (pipes[i].present) {
-                        rc = usb_pipe_register(pipes[i].pipe,
+                        rc = usb_pipe_register(&pipes[i].pipe,
                             pipes[i].descriptor->poll_interval, &hc_conn);
                         if (rc != EOK) {
@@ -398 +396 @@
 
         if (usb_hc_connection_close(&hc_conn) != EOK)
-                usb_log_warning("usb_device_create_pipes(): "
-                    "Failed to close connection.\n");
+                usb_log_warning("%s: Failed to close connection.\n",
+                    __FUNCTION__);
 
         *pipes_ptr = pipes;
@@ -417 +415 @@
         for (i = 0; i < pipe_count; i++) {
                 if (pipes[i].present) {
-                        usb_pipe_unregister(pipes[i].pipe, &hc_conn);
+                        usb_pipe_unregister(&pipes[i].pipe, &hc_conn);
                 }
         }
@@ -431 +429 @@
          */
 rollback_free_only:
-        for (i = 0; i < pipe_count; i++) {
-                if (pipes[i].pipe != NULL) {
-                        free(pipes[i].pipe);
-                }
-        }
         free(pipes);
 
@@ -477 +470 @@
                     i, pipes[i].present ? "" : "not ");
                 if (pipes[i].present)
-                        usb_pipe_unregister(pipes[i].pipe, &hc_conn);
-                free(pipes[i].pipe);
+                        usb_pipe_unregister(&pipes[i].pipe, &hc_conn);
         }
 
@@ -489 +481 @@
         return EOK;
 }
-
-/** Initialize control pipe in a device.
- *
- * @param dev USB device in question.
- * @param errmsg Where to store error context.
- * @return
- */
-static int init_wire_and_ctrl_pipe(usb_device_t *dev, const char **errmsg)
-{
-        int rc;
-
-        rc = usb_device_connection_initialize_from_device(&dev->wire,
-            dev->ddf_dev);
-        if (rc != EOK) {
-                *errmsg = "device connection initialization";
-                return rc;
-        }
-
-        rc = usb_pipe_initialize_default_control(&dev->ctrl_pipe,
-            &dev->wire);
-        if (rc != EOK) {
-                *errmsg = "default control pipe initialization";
-                return rc;
-        }
-
-        return EOK;
-}
-
 
 /** Initialize new instance of USB device.
@@ -533 +497 @@
         assert(ddf_dev != NULL);
 
+        *errstr_ptr = NULL;
+
         usb_dev->ddf_dev = ddf_dev;
         usb_dev->driver_data = NULL;
         usb_dev->descriptors.configuration = NULL;
-        usb_dev->alternate_interfaces = NULL;
         usb_dev->pipes_count = 0;
         usb_dev->pipes = NULL;
 
         /* Initialize backing wire and control pipe. */
-        int rc = init_wire_and_ctrl_pipe(usb_dev, errstr_ptr);
-        if (rc != EOK) {
+        int rc = usb_device_connection_initialize_from_device(
+            &usb_dev->wire, ddf_dev);
+        if (rc != EOK) {
+                *errstr_ptr = "device connection initialization";
+                return rc;
+        }
+
+        /* This pipe was registered by the hub driver,
+         * during device initialization. */
+        rc = usb_pipe_initialize_default_control(&usb_dev->ctrl_pipe,
+            &usb_dev->wire);
+        if (rc != EOK) {
+                *errstr_ptr = "default control pipe initialization";
                 return rc;
         }
@@ -553 +529 @@
             &usb_dev->descriptors);
         if (rc != EOK) {
-                /* Nothing allocated, nothing to free. */
                 *errstr_ptr = "descriptor retrieval";
                 return rc;
         }
 
-        /* Create alternate interfaces. We will silently ignore failure. */
-        //TODO Why ignore?
-        usb_alternate_interfaces_create(usb_dev->descriptors.configuration,
-            usb_dev->descriptors.configuration_size, usb_dev->interface_no,
-            &usb_dev->alternate_interfaces);
-
-        rc = initialize_other_pipes(endpoints, usb_dev, 0);
+        /* Create alternate interfaces. We will silently ignore failure.
+         * We might either control one interface or an entire device,
+         * it makes no sense to speak about alternate interfaces when
+         * controlling a device. */
+        rc = usb_alternate_interfaces_init(&usb_dev->alternate_interfaces,
+            usb_dev->descriptors.configuration,
+            usb_dev->descriptors.configuration_size, usb_dev->interface_no);
+        const int alternate_iface =
+            (rc == EOK) ? usb_dev->alternate_interfaces.current : 0;
+
+        /* TODO Add comment here. */
+        rc = initialize_other_pipes(endpoints, usb_dev, alternate_iface);
         if (rc != EOK) {
                 /* Full configuration descriptor is allocated. */
-                free(usb_dev->descriptors.configuration);
+                usb_device_release_descriptors(&usb_dev->descriptors);
                 /* Alternate interfaces may be allocated */
-                usb_alternate_interfaces_destroy(usb_dev->alternate_interfaces);
+                usb_alternate_interfaces_deinit(&usb_dev->alternate_interfaces);
                 *errstr_ptr = "pipes initialization";
                 return rc;
         }
-
-        *errstr_ptr = NULL;
 
         return EOK;
@@ -591 +569 @@
                 destroy_current_pipes(dev);
 
-                usb_alternate_interfaces_destroy(dev->alternate_interfaces);
-                free(dev->descriptors.configuration);
+                usb_alternate_interfaces_deinit(&dev->alternate_interfaces);
+                usb_device_release_descriptors(&dev->descriptors);
                 free(dev->driver_data);
         }