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

uspace/lib/usbdev/src/recognise.c

@@ -35 +35 @@
 #include <sys/types.h>
 #include <fibril_synch.h>
+#include <usb/debug.h>
+#include <usb/dev/hub.h>
 #include <usb/dev/pipes.h>
 #include <usb/dev/recognise.h>
@@ -50 +52 @@
 
 /** DDF operations of child devices. */
-ddf_dev_ops_t child_ops = {
+static ddf_dev_ops_t child_ops = {
         .interfaces[USB_DEV_IFACE] = &usb_iface_hub_child_impl
 };
@@ -64 +66 @@
 #define BCD_ARGS(a) BCD_INT((a)), BCD_FRAC((a))
 
-/* FIXME: make this dynamic */
-#define MATCH_STRING_MAX 256
-
 /** Add formatted match id.
  *
@@ -75 +74 @@
  */
 static int usb_add_match_id(match_id_list_t *matches, int score,
-    const char *format, ...)
+    const char *match_str)
 {
-        char *match_str = NULL;
-        match_id_t *match_id = NULL;
-        int rc;
-        
-        match_str = malloc(MATCH_STRING_MAX + 1);
-        if (match_str == NULL) {
-                rc = ENOMEM;
-                goto failure;
-        }
-
-        /*
-         * FIXME: replace with dynamic allocation of exact size
-         */
-        va_list args;
-        va_start(args, format   );
-        vsnprintf(match_str, MATCH_STRING_MAX, format, args);
-        match_str[MATCH_STRING_MAX] = 0;
-        va_end(args);
-
-        match_id = create_match_id();
+        assert(matches);
+
+        match_id_t *match_id = create_match_id();
         if (match_id == NULL) {
-                rc = ENOMEM;
-                goto failure;
+                return ENOMEM;
         }
 
@@ -107 +88 @@
 
         return EOK;
-        
-failure:
-        if (match_str != NULL) {
-                free(match_str);
-        }
-        if (match_id != NULL) {
-                match_id->id = NULL;
-                delete_match_id(match_id);
-        }
-        
-        return rc;
 }
 
@@ -129 +99 @@
 #define ADD_MATCHID_OR_RETURN(match_ids, score, format, ...) \
         do { \
-                int __rc = usb_add_match_id((match_ids), (score), \
-                    format, ##__VA_ARGS__); \
+                char *str = NULL; \
+                int __rc = asprintf(&str, format, ##__VA_ARGS__); \
+                if (__rc > 0) { \
+                        __rc = usb_add_match_id((match_ids), (score), str); \
+                } \
                 if (__rc != EOK) { \
+                        free(str); \
                         return __rc; \
                 } \
@@ -150 +124 @@
     match_id_list_t *matches)
 {
-        if (desc_interface == NULL) {
-                return EINVAL;
-        }
-        if (matches == NULL) {
+        if (desc_interface == NULL || matches == NULL) {
                 return EINVAL;
         }
@@ -314 +285 @@
     match_id_list_t *matches)
 {
+        assert(ctrl_pipe);
         int rc;
         /*
@@ -336 +308 @@
 /** Probe for device kind and register it in devman.
  *
- * @param[in] address Address of the (unknown) attached device.
- * @param[in] hc_handle Handle of the host controller.
+ * @param[in] ctrl_pipe Control pipe to the device.
  * @param[in] parent Parent device.
- * @param[in] dev_ops Child device ops.
+ * @param[in] dev_ops Child device ops. Default child_ops will be used if NULL.
  * @param[in] dev_data Arbitrary pointer to be stored in the child
  *      as @c driver_data.
@@ -346 +317 @@
  * @return Error code.
  */
-int usb_device_register_child_in_devman(usb_address_t address,
-    devman_handle_t hc_handle, ddf_dev_t *parent,
-    ddf_dev_ops_t *dev_ops, void *dev_data, ddf_fun_t **child_fun)
+int usb_device_register_child_in_devman(usb_pipe_t *ctrl_pipe,
+    ddf_dev_t *parent, ddf_dev_ops_t *dev_ops, void *dev_data,
+    ddf_fun_t **child_fun)
 {
-        size_t this_device_name_index;
+        if (child_fun == NULL || ctrl_pipe == NULL)
+                return EINVAL;
+
+        if (!dev_ops && dev_data) {
+                usb_log_warning("Using standard fun ops with arbitrary "
+                    "driver data. This does not have to work.\n");
+        }
 
         fibril_mutex_lock(&device_name_index_mutex);
-        this_device_name_index = device_name_index;
-        device_name_index++;
+        const size_t this_device_name_index = device_name_index++;
         fibril_mutex_unlock(&device_name_index_mutex);
 
         ddf_fun_t *child = NULL;
-        char *child_name = NULL;
         int rc;
-        usb_device_connection_t dev_connection;
-        usb_pipe_t ctrl_pipe;
-
-        rc = usb_device_connection_initialize(&dev_connection, hc_handle, address);
-        if (rc != EOK) {
-                goto failure;
-        }
-
-        rc = usb_pipe_initialize_default_control(&ctrl_pipe,
-            &dev_connection);
-        if (rc != EOK) {
-                goto failure;
-        }
-        rc = usb_pipe_probe_default_control(&ctrl_pipe);
-        if (rc != EOK) {
-                goto failure;
-        }
 
         /*
@@ -382 +340 @@
          * naming etc., something more descriptive could be created.
          */
-        rc = asprintf(&child_name, "usb%02zu_a%d",
-            this_device_name_index, address);
+        char child_name[12]; /* The format is: "usbAB_aXYZ", length 11 */
+        rc = snprintf(child_name, sizeof(child_name),
+            "usb%02zu_a%d", this_device_name_index, ctrl_pipe->wire->address);
         if (rc < 0) {
                 goto failure;
@@ -401 +360 @@
 
         child->driver_data = dev_data;
-
-        rc = usb_device_create_match_ids(&ctrl_pipe, &child->match_ids);
+        /* Store the attached device in fun driver data if there is no
+         * other data */
+        if (!dev_data) {
+                usb_hub_attached_device_t *new_device = ddf_fun_data_alloc(
+                    child, sizeof(usb_hub_attached_device_t));
+                if (!new_device) {
+                        rc = ENOMEM;
+                        goto failure;
+                }
+                new_device->address = ctrl_pipe->wire->address;
+                new_device->fun = child;
+        }
+
+
+        rc = usb_device_create_match_ids(ctrl_pipe, &child->match_ids);
         if (rc != EOK) {
                 goto failure;
@@ -412 +384 @@
         }
 
-        if (child_fun != NULL) {
-                *child_fun = child;
-        }
-
+        *child_fun = child;
         return EOK;
 
 failure:
         if (child != NULL) {
-                child->name = NULL;
+                /* We know nothing about the data if it came from outside. */
+                if (dev_data) {
+                        child->driver_data = NULL;
+                }
                 /* This takes care of match_id deallocation as well. */
                 ddf_fun_destroy(child);
         }
-        if (child_name != NULL) {
-                free(child_name);
-        }
 
         return rc;