Index: uspace/drv/bus/usb/usbhub/usbhub.c =================================================================== --- uspace/drv/bus/usb/usbhub/usbhub.c (revision 9f685aace1dc51b76751b3d54828f261a0f0e875) +++ uspace/drv/bus/usb/usbhub/usbhub.c (revision 7278cbc90a2d93600a02b53f8b6fdbeeba7ca711) @@ -67,7 +67,17 @@ } -/** Hub status-change endpoint description. - * - * For more information see section 11.15.1 of USB 1.1 specification. +/** + * Hub status-change endpoint description. + * + * According to USB 2.0 specification, there are two possible arrangements of + * endpoints, depending on whether the hub has a MTT or not. + * + * Under any circumstances, there shall be exactly one endpoint descriptor. + * Though to be sure, let's map the protocol precisely. The possible + * combinations are: + * | bDeviceProtocol | bInterfaceProtocol + * Only single TT | 0 | 0 + * MTT in Single-TT mode | 2 | 1 + * MTT in MTT mode | 2 | 2 (iface alt. 1) */ static const usb_endpoint_description_t @@ -79,5 +89,4 @@ &status_change_mtt_available, }; - /** Standard get hub global status request */ @@ -90,16 +99,18 @@ }; -static int usb_set_first_configuration(usb_device_t *usb_device); -static int usb_hub_process_hub_specific_info(usb_hub_dev_t *hub_dev); -static void usb_hub_over_current(const usb_hub_dev_t *hub_dev, - usb_hub_status_t status); -static void usb_hub_global_interrupt(const usb_hub_dev_t *hub_dev); - -static bool usb_hub_polling_error_callback(usb_device_t *dev, int err_code, void *arg) +static int usb_set_first_configuration(usb_device_t *); +static int usb_hub_process_hub_specific_info(usb_hub_dev_t *); +static void usb_hub_over_current(const usb_hub_dev_t *, usb_hub_status_t); +static int usb_hub_polling_init(usb_hub_dev_t *, usb_endpoint_mapping_t *); +static void usb_hub_global_interrupt(const usb_hub_dev_t *); + +static bool usb_hub_polling_error_callback(usb_device_t *dev, + int err_code, void *arg) { assert(dev); assert(arg); - usb_log_error("Device %s polling error: %s", usb_device_get_name(dev), str_error(err_code)); + usb_log_error("Device %s polling error: %s", + usb_device_get_name(dev), str_error(err_code)); return true; @@ -116,5 +127,7 @@ int usb_hub_device_add(usb_device_t *usb_dev) { + int err; assert(usb_dev); + /* Create driver soft-state structure */ usb_hub_dev_t *hub_dev = @@ -127,21 +140,14 @@ hub_dev->speed = usb_device_get_speed(usb_dev); - fibril_mutex_initialize(&hub_dev->default_address_guard); - fibril_condvar_initialize(&hub_dev->default_address_cv); - /* Set hub's first configuration. (There should be only one) */ - int opResult = usb_set_first_configuration(usb_dev); - if (opResult != EOK) { - usb_log_error("Could not set hub configuration: %s", - str_error(opResult)); - return opResult; + if ((err = usb_set_first_configuration(usb_dev))) { + usb_log_error("Could not set hub configuration: %s", str_error(err)); + return err; } /* Get port count and create attached_devices. */ - opResult = usb_hub_process_hub_specific_info(hub_dev); - if (opResult != EOK) { - usb_log_error("Could process hub specific info, %s", - str_error(opResult)); - return opResult; + if ((err = usb_hub_process_hub_specific_info(hub_dev))) { + usb_log_error("Could process hub specific info, %s", str_error(err)); + return err; } @@ -167,26 +173,112 @@ /* Bind hub control function. */ - opResult = ddf_fun_bind(hub_dev->hub_fun); - if (opResult != EOK) { - usb_log_error("Failed to bind hub function: %s.", - str_error(opResult)); - ddf_fun_destroy(hub_dev->hub_fun); - return opResult; + if ((err = ddf_fun_bind(hub_dev->hub_fun))) { + usb_log_error("Failed to bind hub function: %s.", str_error(err)); + goto err_ddf_fun; } /* Start hub operation. */ + if ((err = usb_hub_polling_init(hub_dev, status_change_mapping))) { + usb_log_error("Failed to start polling: %s.", str_error(err)); + goto err_bound; + } + + usb_log_info("Controlling %s-speed hub '%s' (%p: %zu ports).", + usb_str_speed(hub_dev->speed), + usb_device_get_name(hub_dev->usb_device), hub_dev, + hub_dev->port_count); + + return EOK; + +err_bound: + ddf_fun_unbind(hub_dev->hub_fun); +err_ddf_fun: + ddf_fun_destroy(hub_dev->hub_fun); + return err; +} + +static int usb_hub_cleanup(usb_hub_dev_t *hub) +{ + free(hub->polling.buffer); + usb_polling_fini(&hub->polling); + + for (size_t port = 0; port < hub->port_count; ++port) { + usb_port_fini(&hub->ports[port].base); + } + free(hub->ports); + + const int ret = ddf_fun_unbind(hub->hub_fun); + if (ret != EOK) { + usb_log_error("(%p) Failed to unbind '%s' function: %s.", + hub, HUB_FNC_NAME, str_error(ret)); + return ret; + } + ddf_fun_destroy(hub->hub_fun); + + usb_log_info("(%p) USB hub driver stopped and cleaned.", hub); + + /* Device data (usb_hub_dev_t) will be freed by usbdev. */ + return EOK; +} + +/** + * Turn off power to all ports. + * + * @param usb_dev generic usb device information + * @return error code + */ +int usb_hub_device_remove(usb_device_t *usb_dev) +{ + assert(usb_dev); + usb_hub_dev_t *hub = usb_device_data_get(usb_dev); + assert(hub); + + usb_log_info("(%p) USB hub removed, joining polling fibril.", hub); + + /* Join polling fibril (ignoring error code). */ + usb_polling_join(&hub->polling); + usb_log_info("(%p) USB hub polling stopped, freeing memory.", hub); + + /* Destroy hub. */ + return usb_hub_cleanup(hub); +} + +/** + * Remove all attached devices + * @param usb_dev generic usb device information + * @return error code + */ +int usb_hub_device_gone(usb_device_t *usb_dev) +{ + assert(usb_dev); + usb_hub_dev_t *hub = usb_device_data_get(usb_dev); + assert(hub); + + usb_log_info("(%p) USB hub gone, joining polling fibril.", hub); + + /* Join polling fibril (ignoring error code). */ + usb_polling_join(&hub->polling); + usb_log_info("(%p) USB hub polling stopped, freeing memory.", hub); + + /* Destroy hub. */ + return usb_hub_cleanup(hub); +} + +/** + * Initialize and start the polling of the Status Change Endpoint. + * + * @param mapping The mapping of Status Change Endpoint + */ +static int usb_hub_polling_init(usb_hub_dev_t *hub_dev, + usb_endpoint_mapping_t *mapping) +{ + int err; usb_polling_t *polling = &hub_dev->polling; - opResult = usb_polling_init(polling); - if (opResult != EOK) { - /* Function is already bound */ - ddf_fun_unbind(hub_dev->hub_fun); - ddf_fun_destroy(hub_dev->hub_fun); - usb_log_error("Failed to initialize polling fibril: %s.", - str_error(opResult)); - return opResult; - } + + if ((err = usb_polling_init(polling))) + return err; polling->device = hub_dev->usb_device; - polling->ep_mapping = status_change_mapping; + polling->ep_mapping = mapping; polling->request_size = ((hub_dev->port_count + 1 + 7) / 8); polling->buffer = malloc(polling->request_size); @@ -195,92 +287,16 @@ polling->arg = hub_dev; - opResult = usb_polling_start(polling); - if (opResult != EOK) { + if ((err = usb_polling_start(polling))) { /* Polling is already initialized. */ free(polling->buffer); usb_polling_fini(polling); - ddf_fun_unbind(hub_dev->hub_fun); - ddf_fun_destroy(hub_dev->hub_fun); - usb_log_error("Failed to create polling fibril: %s.", - str_error(opResult)); - return opResult; - } - - usb_log_info("Controlling %s-speed hub '%s' (%p: %zu ports).", - usb_str_speed(hub_dev->speed), - usb_device_get_name(hub_dev->usb_device), hub_dev, - hub_dev->port_count); + return err; + } return EOK; } -static int usb_hub_cleanup(usb_hub_dev_t *hub) -{ - free(hub->polling.buffer); - usb_polling_fini(&hub->polling); - - for (size_t port = 0; port < hub->port_count; ++port) { - usb_port_fini(&hub->ports[port].base); - } - free(hub->ports); - - const int ret = ddf_fun_unbind(hub->hub_fun); - if (ret != EOK) { - usb_log_error("(%p) Failed to unbind '%s' function: %s.", - hub, HUB_FNC_NAME, str_error(ret)); - return ret; - } - ddf_fun_destroy(hub->hub_fun); - - usb_log_info("(%p) USB hub driver stopped and cleaned.", hub); - - /* Device data (usb_hub_dev_t) will be freed by usbdev. */ - return EOK; -} - -/** - * Turn off power to all ports. - * - * @param usb_dev generic usb device information - * @return error code - */ -int usb_hub_device_remove(usb_device_t *usb_dev) -{ - assert(usb_dev); - usb_hub_dev_t *hub = usb_device_data_get(usb_dev); - assert(hub); - - usb_log_info("(%p) USB hub removed, joining polling fibril.", hub); - - /* Join polling fibril (ignoring error code). */ - usb_polling_join(&hub->polling); - usb_log_info("(%p) USB hub polling stopped, freeing memory.", hub); - - /* Destroy hub. */ - return usb_hub_cleanup(hub); -} - -/** - * Remove all attached devices - * @param usb_dev generic usb device information - * @return error code - */ -int usb_hub_device_gone(usb_device_t *usb_dev) -{ - assert(usb_dev); - usb_hub_dev_t *hub = usb_device_data_get(usb_dev); - assert(hub); - - usb_log_info("(%p) USB hub gone, joining polling fibril.", hub); - - /* Join polling fibril (ignoring error code). */ - usb_polling_join(&hub->polling); - usb_log_info("(%p) USB hub polling stopped, freeing memory.", hub); - - /* Destroy hub. */ - return usb_hub_cleanup(hub); -} - -/** Callback for polling hub for changes. +/** + * Callback for polling hub for changes. * * @param dev Device where the change occured. @@ -307,5 +323,5 @@ } - /* N + 1 bit indicates change on port N */ + /* Nth bit indicates change on port N */ for (size_t port = 0; port < hub->port_count; ++port) { const size_t bit = port + 1; @@ -331,5 +347,6 @@ for (unsigned int port = 0; port < hub_dev->port_count; ++port) { usb_log_debug("(%p): Powering port %u.", hub_dev, port + 1); - const int ret = usb_hub_set_port_feature(hub_dev, port + 1, USB_HUB_FEATURE_PORT_POWER); + const int ret = usb_hub_set_port_feature(hub_dev, port + 1, + USB_HUB_FEATURE_PORT_POWER); if (ret != EOK) { @@ -356,5 +373,4 @@ assert(hub_dev); - /* Get hub descriptor. */ usb_log_debug("(%p): Retrieving descriptor.", hub_dev); usb_pipe_t *control_pipe = usb_device_get_default_pipe(hub_dev->usb_device); @@ -363,4 +379,5 @@ ? USB_DESCTYPE_SSPEED_HUB : USB_DESCTYPE_HUB; + /* Get hub descriptor. */ usb_hub_descriptor_header_t descriptor; size_t received_size; @@ -483,5 +500,6 @@ /* Over-current condition is gone, it is safe to turn the ports on. */ for (size_t port = 0; port < hub_dev->port_count; ++port) { - const int ret = usb_hub_set_port_feature(hub_dev, port, USB_HUB_FEATURE_PORT_POWER); + const int ret = usb_hub_set_port_feature(hub_dev, port, + USB_HUB_FEATURE_PORT_POWER); if (ret != EOK) { usb_log_warning("(%p-%u): HUB OVER-CURRENT GONE: Cannot" @@ -526,5 +544,6 @@ * @param feature Feature selector. */ -int usb_hub_set_port_feature(const usb_hub_dev_t *hub, size_t port_number, usb_hub_class_feature_t feature) +int usb_hub_set_port_feature(const usb_hub_dev_t *hub, size_t port_number, + usb_hub_class_feature_t feature) { assert(hub); @@ -546,5 +565,6 @@ * @param feature Feature selector. */ -int usb_hub_clear_port_feature(const usb_hub_dev_t *hub, size_t port_number, usb_hub_class_feature_t feature) +int usb_hub_clear_port_feature(const usb_hub_dev_t *hub, size_t port_number, + usb_hub_class_feature_t feature) { assert(hub); @@ -567,5 +587,6 @@ * @return Error code. */ -int usb_hub_get_port_status(const usb_hub_dev_t *hub, size_t port_number, usb_port_status_t *status) +int usb_hub_get_port_status(const usb_hub_dev_t *hub, size_t port_number, + usb_port_status_t *status) { assert(hub); @@ -669,5 +690,12 @@ } +/** + * Instead of just sleeping, we may as well sleep on a condition variable. + * This has the advantage that we may instantly wait other hub from the polling + * sleep, mitigating the delay of polling while still being synchronized with + * other devices in need of the default address (there shall not be any). + */ static FIBRIL_CONDVAR_INITIALIZE(global_hub_default_address_cv); +static FIBRIL_MUTEX_INITIALIZE(global_hub_default_address_guard); /** @@ -677,5 +705,6 @@ * is connected with already attached devices. */ -int usb_hub_reserve_default_address(usb_hub_dev_t *hub, async_exch_t *exch, usb_port_t *port) +int usb_hub_reserve_default_address(usb_hub_dev_t *hub, async_exch_t *exch, + usb_port_t *port) { assert(hub); @@ -684,26 +713,38 @@ assert(fibril_mutex_is_locked(&port->guard)); - fibril_mutex_lock(&hub->default_address_guard); - if (hub->default_address_requests++ == 0) { - /* We're the first to request the address, we can just do it */ - fibril_mutex_unlock(&hub->default_address_guard); - int err; - while ((err = usbhc_reserve_default_address(exch)) == EAGAIN) { - // We ignore the return value here, as we cannot give up now. - usb_port_condvar_wait_timeout(port, &global_hub_default_address_cv, 500000); - } - return err; - } else { + int err = usbhc_reserve_default_address(exch); + /* + * EINVAL signalls that its our hub (hopefully different port) that has + * this address reserved + */ + while (err == EAGAIN || err == EINVAL) { /* Drop the port guard, we're going to wait */ fibril_mutex_unlock(&port->guard); - /* Wait for a signal */ - fibril_condvar_wait(&hub->default_address_cv, &hub->default_address_guard); - - /* Remember ABBA, first drop the hub guard */ - fibril_mutex_unlock(&hub->default_address_guard); + /* This sleeping might be disturbed by other hub */ + fibril_mutex_lock(&global_hub_default_address_guard); + fibril_condvar_wait_timeout(&global_hub_default_address_cv, + &global_hub_default_address_guard, 2000000); + fibril_mutex_unlock(&global_hub_default_address_guard); + fibril_mutex_lock(&port->guard); - return EOK; - } + err = usbhc_reserve_default_address(exch); + } + + if (err) + return err; + + /* + * As we dropped the port guard, we need to check whether the device is + * still connected. If the release fails, we still hold the default + * address -- but then there is probably a bigger problem with the HC + * anyway. + */ + if (port->state != PORT_CONNECTING) { + err = usb_hub_release_default_address(hub, exch); + return err ? err : EINTR; + } + + return EOK; } @@ -713,17 +754,11 @@ int usb_hub_release_default_address(usb_hub_dev_t *hub, async_exch_t *exch) { - int ret = EOK; - - fibril_mutex_lock(&hub->default_address_guard); - if (--hub->default_address_requests == 0) { - // We must do it in critical section to prevent other fibril - // from requesting the address before we release - ret = usbhc_release_default_address(exch); - // This is optimistic optimization - it may wake one hub from polling sleep - fibril_condvar_signal(&global_hub_default_address_cv); - } else { - fibril_condvar_signal(&hub->default_address_cv); - } - fibril_mutex_unlock(&hub->default_address_guard); + const int ret = usbhc_release_default_address(exch); + + /* + * This is an optimistic optimization - it may wake + * one hub from polling sleep instantly. + */ + fibril_condvar_signal(&global_hub_default_address_cv); return ret; Index: uspace/drv/bus/usb/usbhub/usbhub.h =================================================================== --- uspace/drv/bus/usb/usbhub/usbhub.h (revision 9f685aace1dc51b76751b3d54828f261a0f0e875) +++ uspace/drv/bus/usb/usbhub/usbhub.h (revision 7278cbc90a2d93600a02b53f8b6fdbeeba7ca711) @@ -74,9 +74,4 @@ /** Whether MTT is available */ bool mtt_available; - - /** Default address management */ - unsigned default_address_requests; - fibril_mutex_t default_address_guard; - fibril_condvar_t default_address_cv; };