Changeset 41924f30 in mainline for uspace/lib/usbhost/src

Timestamp:

2017-10-12T14:07:27Z (8 years ago)

Author:

Ondřej Hlavatý <aearsis@…>

Branches:

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

Children:

a5976973

Parents:

7e74911

Message:

WIP usbhost refactoring

This commit replaces callbacks with more systematic virtual-like inheritance-like solution. Currently breaks build of HelenOS, but both xhci and usbhost are buildable. More refactoring follows…

Location:

uspace/lib/usbhost/src

Files:

• bandwidth.c (added)
• bus.c (added)
• ddf_helpers.c (modified) (6 diffs)
• endpoint.c (modified) (2 diffs)
• hcd.c (modified) (12 diffs)
• usb2_bus.c (added)
• usb_bus.c (deleted)

uspace/lib/usbhost/src/ddf_helpers.c

@@ -469 +469 @@
 
         /* This checks whether the default address is reserved and gets speed */
-        int ret = usb_bus_get_speed(&hcd->bus, USB_ADDRESS_DEFAULT, &speed);
+        int ret = bus_get_speed(hcd->bus, USB_ADDRESS_DEFAULT, &speed);
         if (ret != EOK) {
                 usb_log_error("Failed to verify speed: %s.", str_error(ret));
@@ -623 +623 @@
         assert(hcd);
 
-        hcd_reserve_default_address(hcd, hcd->bus.max_speed);
+        hcd_reserve_default_address(hcd, USB_SPEED_MAX);
         const int ret = hcd_ddf_new_device(device, NULL, 0);
         hcd_release_default_address(hcd);
@@ -639 +639 @@
  * This function does all the ddf work for hc driver.
  */
-int hcd_ddf_setup_hc(ddf_dev_t *device, usb_speed_t max_speed,
-    size_t bw, bw_count_func_t bw_count)
+int hcd_ddf_setup_hc(ddf_dev_t *device)
 {
         assert(device);
@@ -650 +649 @@
         }
         instance->root_hub = NULL;
-        hcd_init(&instance->hcd, max_speed, bw, bw_count);
+        hcd_init(&instance->hcd);
 
         int ret = ENOMEM;
@@ -851 +850 @@
 {
         assert(driver);
-        static const struct { size_t bw; bw_count_func_t bw_count; }bw[] = {
-            [USB_SPEED_FULL] = { .bw = BANDWIDTH_AVAILABLE_USB11,
-                                 .bw_count = bandwidth_count_usb11 },
-            [USB_SPEED_HIGH] = { .bw = BANDWIDTH_AVAILABLE_USB11,
-                                 .bw_count = bandwidth_count_usb11 },
-            [USB_SPEED_SUPER] = { .bw = BANDWIDTH_AVAILABLE_USB11,
-                                 .bw_count = bandwidth_count_usb11 },
-        };
 
         int ret = EOK;
-        const usb_speed_t speed = driver->hc_speed;
-        if (speed >= ARRAY_SIZE(bw) || bw[speed].bw == 0) {
-                usb_log_error("Driver `%s' reported unsupported speed: %s",
-                    driver->name, usb_str_speed(speed));
-                return ENOTSUP;
-        }
 
         hw_res_list_parsed_t hw_res;
@@ -877 +862 @@
         }
 
-        ret = hcd_ddf_setup_hc(device, speed, bw[speed].bw, bw[speed].bw_count);
+        ret = hcd_ddf_setup_hc(device);
         if (ret != EOK) {
                 usb_log_error("Failed to setup generic HCD.\n");

uspace/lib/usbhost/src/endpoint.c

@@ -1 +1 @@
 /*
  * Copyright (c) 2011 Jan Vesely
+ * Copyright (c) 2017 Ondrej Hlavaty <aearsis@eideo.cz>
  * All rights reserved.
  *
@@ -35 +36 @@
 
 #include <usb/host/endpoint.h>
+#include <usb/host/bus.h>
 
 #include <assert.h>
+#include <atomic.h>
+#include <mem.h>
 #include <stdlib.h>
-#include <atomic.h>
 
-/** Allocate ad initialize endpoint_t structure.
- * @param address USB address.
- * @param endpoint USB endpoint number.
- * @param direction Communication direction.
- * @param type USB transfer type.
- * @param speed Communication speed.
- * @param max_packet_size Maximum size of data packets.
- * @param bw Required bandwidth.
- * @return Pointer to initialized endpoint_t structure, NULL on failure.
+/** Initialize provided endpoint structure.
  */
-endpoint_t * endpoint_create(usb_address_t address, usb_endpoint_t endpoint,
-    usb_direction_t direction, usb_transfer_type_t type, usb_speed_t speed,
-    size_t max_packet_size, unsigned packets, size_t bw,
-    usb_address_t tt_address, unsigned tt_p)
+void endpoint_init(endpoint_t *ep, bus_t *bus)
 {
-        endpoint_t *instance = malloc(sizeof(endpoint_t));
-        if (instance) {
-                atomic_set(&instance->refcnt, 0);
-                instance->address = address;
-                instance->endpoint = endpoint;
-                instance->direction = direction;
-                instance->transfer_type = type;
-                instance->speed = speed;
-                instance->max_packet_size = max_packet_size;
-                instance->packets = packets;
-                instance->bandwidth = bw;
-                instance->toggle = 0;
-                instance->active = false;
-                instance->tt.address = tt_address;
-                instance->tt.port = tt_p;
-                instance->hc_data.data = NULL;
-                instance->hc_data.toggle_get = NULL;
-                instance->hc_data.toggle_set = NULL;
-                link_initialize(&instance->link);
-                fibril_mutex_initialize(&instance->guard);
-                fibril_condvar_initialize(&instance->avail);
-        }
-        return instance;
+        memset(ep, 0, sizeof(endpoint_t));
+
+        ep->bus = bus;
+        atomic_set(&ep->refcnt, 0);
+        link_initialize(&ep->link);
+        fibril_mutex_initialize(&ep->guard);
+        fibril_condvar_initialize(&ep->avail);
 }
 
-/** Properly dispose of endpoint_t structure.
- * @param instance endpoint_t structure.
- */
-void endpoint_destroy(endpoint_t *instance)
+void endpoint_add_ref(endpoint_t *ep)
 {
-        assert(instance);
-        assert(!instance->active);
-        assert(instance->hc_data.data == NULL);
-        free(instance);
+        atomic_inc(&ep->refcnt);
 }
 
-void endpoint_add_ref(endpoint_t *instance)
+void endpoint_del_ref(endpoint_t *ep)
 {
-        atomic_inc(&instance->refcnt);
-}
+        if (atomic_predec(&ep->refcnt) == 0) {
+                if (ep->bus->ops.destroy_endpoint) {
+                        ep->bus->ops.destroy_endpoint(ep);
+                }
+                else {
+                        assert(!ep->active);
 
-void endpoint_del_ref(endpoint_t *instance)
-{
-        if (atomic_predec(&instance->refcnt) == 0)
-                endpoint_destroy(instance);
-}
-
-/** Set device specific data and hooks.
- * @param instance endpoint_t structure.
- * @param data device specific data.
- * @param toggle_get Hook to call when retrieving value of toggle bit.
- * @param toggle_set Hook to call when setting the value of toggle bit.
- */
-void endpoint_set_hc_data(endpoint_t *instance,
-    void *data, int (*toggle_get)(void *), void (*toggle_set)(void *, int))
-{
-        assert(instance);
-        fibril_mutex_lock(&instance->guard);
-        instance->hc_data.data = data;
-        instance->hc_data.toggle_get = toggle_get;
-        instance->hc_data.toggle_set = toggle_set;
-        fibril_mutex_unlock(&instance->guard);
-}
-
-/** Clear device specific data and hooks.
- * @param instance endpoint_t structure.
- * @note This function does not free memory pointed to by data pointer.
- */
-void endpoint_clear_hc_data(endpoint_t *instance)
-{
-        assert(instance);
-        endpoint_set_hc_data(instance, NULL, NULL, NULL);
+                        /* Assume mostly the eps will be allocated by malloc. */
+                        free(ep);
+                }
+        }
 }
 
 /** Mark the endpoint as active and block access for further fibrils.
- * @param instance endpoint_t structure.
+ * @param ep endpoint_t structure.
  */
-void endpoint_use(endpoint_t *instance)
+void endpoint_use(endpoint_t *ep)
 {
-        assert(instance);
+        assert(ep);
         /* Add reference for active endpoint. */
-        endpoint_add_ref(instance);
-        fibril_mutex_lock(&instance->guard);
-        while (instance->active)
-                fibril_condvar_wait(&instance->avail, &instance->guard);
-        instance->active = true;
-        fibril_mutex_unlock(&instance->guard);
+        endpoint_add_ref(ep);
+        fibril_mutex_lock(&ep->guard);
+        while (ep->active)
+                fibril_condvar_wait(&ep->avail, &ep->guard);
+        ep->active = true;
+        fibril_mutex_unlock(&ep->guard);
 }
 
 /** Mark the endpoint as inactive and allow access for further fibrils.
- * @param instance endpoint_t structure.
+ * @param ep endpoint_t structure.
  */
-void endpoint_release(endpoint_t *instance)
+void endpoint_release(endpoint_t *ep)
 {
-        assert(instance);
-        fibril_mutex_lock(&instance->guard);
-        instance->active = false;
-        fibril_mutex_unlock(&instance->guard);
-        fibril_condvar_signal(&instance->avail);
+        assert(ep);
+        fibril_mutex_lock(&ep->guard);
+        ep->active = false;
+        fibril_mutex_unlock(&ep->guard);
+        fibril_condvar_signal(&ep->avail);
         /* Drop reference for active endpoint. */
-        endpoint_del_ref(instance);
+        endpoint_del_ref(ep);
 }
 
-/** Get the value of toggle bit.
- * @param instance endpoint_t structure.
- * @note Will use provided hook.
+/** Get the value of toggle bit. Either uses the toggle_get op, or just returns
+ * the value of the toggle.
+ * @param ep endpoint_t structure.
  */
-int endpoint_toggle_get(endpoint_t *instance)
+int endpoint_toggle_get(endpoint_t *ep)
 {
-        assert(instance);
-        fibril_mutex_lock(&instance->guard);
-        if (instance->hc_data.toggle_get)
-                instance->toggle =
-                    instance->hc_data.toggle_get(instance->hc_data.data);
-        const int ret = instance->toggle;
-        fibril_mutex_unlock(&instance->guard);
+        assert(ep);
+        fibril_mutex_lock(&ep->guard);
+        const int ret = ep->bus->ops.endpoint_get_toggle
+            ? ep->bus->ops.endpoint_get_toggle(ep)
+            : ep->toggle;
+        fibril_mutex_unlock(&ep->guard);
         return ret;
 }
 
-/** Set the value of toggle bit.
- * @param instance endpoint_t structure.
- * @note Will use provided hook.
+/** Set the value of toggle bit. Either uses the toggle_set op, or just sets
+ * the toggle inside.
+ * @param ep endpoint_t structure.
  */
-void endpoint_toggle_set(endpoint_t *instance, int toggle)
+void endpoint_toggle_set(endpoint_t *ep, unsigned toggle)
 {
-        assert(instance);
+        assert(ep);
         assert(toggle == 0 || toggle == 1);
-        fibril_mutex_lock(&instance->guard);
-        instance->toggle = toggle;
-        if (instance->hc_data.toggle_set)
-                instance->hc_data.toggle_set(instance->hc_data.data, toggle);
-        fibril_mutex_unlock(&instance->guard);
+        fibril_mutex_lock(&ep->guard);
+        if (ep->bus->ops.endpoint_set_toggle) {
+                ep->bus->ops.endpoint_set_toggle(ep, toggle);
+        }
+        else {
+                ep->toggle = toggle;
+        }
+        fibril_mutex_unlock(&ep->guard);
 }
+
 
 /**

uspace/lib/usbhost/src/hcd.c

@@ -44 +44 @@
 #include "hcd.h"
 
-/** Calls ep_add_hook upon endpoint registration.
+
+/*[>* Calls ep_add_hook upon endpoint registration.
  * @param ep Endpoint to be registered.
  * @param arg hcd_t in disguise.
  * @return Error code.
- */
+ * OH TODO: remove
+ <]
 static int register_helper(endpoint_t *ep, void *arg)
 {
@@ -59 +61 @@
 }
 
-/** Calls ep_remove_hook upon endpoint removal.
+[>* Calls ep_remove_hook upon endpoint removal.
  * @param ep Endpoint to be unregistered.
  * @param arg hcd_t in disguise.
- */
+ * OH TODO: remove
+ <]
 static void unregister_helper(endpoint_t *ep, void *arg)
 {
@@ -72 +75 @@
 }
 
-/** Calls ep_remove_hook upon endpoint removal. Prints warning.
+[>* Calls ep_remove_hook upon endpoint removal. Prints warning.
  *  * @param ep Endpoint to be unregistered.
  *   * @param arg hcd_t in disguise.
- *    */
+ * OH TODO: remove
+ *    <]
 static void unregister_helper_warn(endpoint_t *ep, void *arg)
 {
         assert(ep);
         usb_log_warning("Endpoint %d:%d %s was left behind, removing.\n",
-            ep->address, ep->endpoint, usb_str_direction(ep->direction));
+            ep->target.address, ep->target.endpoint, usb_str_direction(ep->direction));
         unregister_helper(ep, arg);
 }
-
+*/
 
 /** Initialize hcd_t structure.
@@ -93 +97 @@
  * @param bw_count Bandwidth compute function, passed to endpoint manager.
  */
-void hcd_init(hcd_t *hcd, usb_speed_t max_speed, size_t bandwidth,
-    bw_count_func_t bw_count)
-{
-        assert(hcd);
-        usb_bus_init(&hcd->bus, bandwidth, bw_count, max_speed);
-
-        hcd_set_implementation(hcd, NULL, NULL);
+void hcd_init(hcd_t *hcd) {
+        assert(hcd);
+
+        hcd_set_implementation(hcd, NULL, NULL, NULL);
 }
 
@@ -106 +107 @@
         assert(hcd);
         usb_address_t address = 0;
-        const int ret = usb_bus_request_address(
-            &hcd->bus, &address, false, speed);
+        const int ret = bus_request_address(hcd->bus, &address, false, speed);
         if (ret != EOK)
                 return ret;
@@ -116 +116 @@
 {
         assert(hcd);
-        return usb_bus_remove_address(&hcd->bus, address,
-            unregister_helper_warn, hcd);
+        return bus_release_address(hcd->bus, address);
+        // OH TODO removed helper
 }
 
@@ -124 +124 @@
         assert(hcd);
         usb_address_t address = 0;
-        return usb_bus_request_address(&hcd->bus, &address, true, speed);
+        return bus_request_address(hcd->bus, &address, true, speed);
 }
 
@@ -132 +132 @@
 {
         assert(hcd);
-        return usb_bus_add_ep(&hcd->bus, target.address,
-            target.endpoint, dir, type, max_packet_size, packets, size,
-            register_helper, hcd, tt_address, tt_port);
+
+        /* Temporary reference */
+        endpoint_t *ep = bus_create_endpoint(hcd->bus);
+        if (!ep)
+                return ENOMEM;
+
+        ep->target = target;
+        ep->direction = dir;
+        ep->transfer_type = type;
+        ep->max_packet_size = max_packet_size;
+        ep->packets = packets;
+
+        ep->tt.address = tt_address;
+        ep->tt.port = tt_port;
+
+        const int err = bus_register_endpoint(hcd->bus, ep);
+
+        /* drop Temporary reference */
+        endpoint_del_ref(ep);
+
+        return err;
 }
 
@@ -140 +158 @@
 {
         assert(hcd);
-        return usb_bus_remove_ep(&hcd->bus, target.address,
-            target.endpoint, dir, unregister_helper, hcd);
+        endpoint_t *ep = bus_find_endpoint(hcd->bus, target, dir);
+        if (!ep)
+                return ENOENT;
+
+        return bus_release_endpoint(hcd->bus, ep);
+        // OH TODO removed helper
 }
 
@@ -159 +181 @@
                 usb_log_debug2("Reseting toggle on %d:%d.\n",
                     toggle->target.address, toggle->target.endpoint);
-                usb_bus_reset_toggle(&toggle->hcd->bus,
+                bus_reset_toggle(toggle->hcd->bus,
                     toggle->target, toggle->target.endpoint == 0);
         }
@@ -185 +207 @@
         assert(hcd);
 
-        endpoint_t *ep = usb_bus_find_ep(&hcd->bus,
-            target.address, target.endpoint, direction);
+        endpoint_t *ep = bus_find_endpoint(hcd->bus, target, direction);
         if (ep == NULL) {
                 usb_log_error("Endpoint(%d:%d) not registered for %s.\n",
@@ -196 +217 @@
             name, target.address, target.endpoint, size, ep->max_packet_size);
 
-        const size_t bw = bandwidth_count_usb11(
-            ep->speed, ep->transfer_type, size, ep->max_packet_size);
+        const size_t bw = bus_count_bw(ep, size);
         /* Check if we have enough bandwidth reserved */
         if (ep->bandwidth < bw) {
                 usb_log_error("Endpoint(%d:%d) %s needs %zu bw "
                     "but only %zu is reserved.\n",
-                    ep->address, ep->endpoint, name, bw, ep->bandwidth);
+                    ep->target.address, ep->target.endpoint, name, bw, ep->bandwidth);
                 return ENOSPC;
         }