Index: uspace/lib/usbhost/include/usb/host/bus.h =================================================================== --- uspace/lib/usbhost/include/usb/host/bus.h (revision bad4a050d11dd44a634a911a7e5a42918d984aab) +++ uspace/lib/usbhost/include/usb/host/bus.h (revision 0892663adc814c89139511bb2bea33fffe20ff46) @@ -105,5 +105,5 @@ void (*device_remove)(device_t *); int (*device_online)(device_t *); /**< Optional */ - int (*device_offline)(device_t *); /**< Optional */ + void (*device_offline)(device_t *); /**< Optional */ endpoint_t *(*endpoint_create)(device_t *, const usb_endpoint_descriptors_t *); Index: uspace/lib/usbhost/include/usb/host/endpoint.h =================================================================== --- uspace/lib/usbhost/include/usb/host/endpoint.h (revision bad4a050d11dd44a634a911a7e5a42918d984aab) +++ uspace/lib/usbhost/include/usb/host/endpoint.h (revision 0892663adc814c89139511bb2bea33fffe20ff46) @@ -44,4 +44,5 @@ #include #include +#include #include #include @@ -93,17 +94,7 @@ extern void endpoint_del_ref(endpoint_t *); -/* Pay atention to synchronization of batch access wrt to aborting & finishing from another fibril. */ - -/* Set currently active batch. The common case is to activate in the same - * critical section as scheduling to HW. - */ +extern void endpoint_wait_timeout_locked(endpoint_t *ep, suseconds_t); extern void endpoint_activate_locked(endpoint_t *, usb_transfer_batch_t *); - -/* Deactivate the endpoint, allowing others to activate it again. Batch shall - * already have an error set. */ extern void endpoint_deactivate_locked(endpoint_t *); - -/* Abort the currenty active batch. */ -void endpoint_abort(endpoint_t *); /* Calculate bandwidth */ Index: uspace/lib/usbhost/include/usb/host/usb_transfer_batch.h =================================================================== --- uspace/lib/usbhost/include/usb/host/usb_transfer_batch.h (revision bad4a050d11dd44a634a911a7e5a42918d984aab) +++ uspace/lib/usbhost/include/usb/host/usb_transfer_batch.h (revision 0892663adc814c89139511bb2bea33fffe20ff46) @@ -112,5 +112,4 @@ /** Batch finalization. */ -void usb_transfer_batch_abort(usb_transfer_batch_t *); void usb_transfer_batch_finish(usb_transfer_batch_t *); Index: uspace/lib/usbhost/src/bus.c =================================================================== --- uspace/lib/usbhost/src/bus.c (revision bad4a050d11dd44a634a911a7e5a42918d984aab) +++ uspace/lib/usbhost/src/bus.c (revision 0892663adc814c89139511bb2bea33fffe20ff46) @@ -43,4 +43,5 @@ #include #include +#include #include @@ -97,4 +98,6 @@ /** * Invoke the device_enumerate bus operation. + * + * There's no need to synchronize here, because no one knows the device yet. */ int bus_device_enumerate(device_t *dev) @@ -106,46 +109,189 @@ return ENOTSUP; - return ops->device_enumerate(dev); -} - -/** - * Invoke the device_remove bus operation. + if (dev->online) { + fibril_mutex_unlock(&dev->guard); + return EINVAL; + } + + const int r = ops->device_enumerate(dev); + if (!r) { + dev->online = true; + + fibril_mutex_lock(&dev->hub->guard); + list_append(&dev->link, &dev->hub->devices); + fibril_mutex_unlock(&dev->hub->guard); + } + + return r; +} + +/** + * Clean endpoints and children that could have been left behind after + * asking the driver of device to offline/remove a device. + * + * Note that EP0's lifetime is shared with the device, and as such is not + * touched. + */ +static void device_clean_ep_children(device_t *dev, const char *op) +{ + assert(fibril_mutex_is_locked(&dev->guard)); + + /* Unregister endpoints left behind. */ + for (usb_endpoint_t i = 1; i < USB_ENDPOINT_MAX; ++i) { + if (!dev->endpoints[i]) + continue; + + usb_log_warning("USB device '%s' driver left endpoint %u registered after %s.", + ddf_fun_get_name(dev->fun), i, op); + + endpoint_t * const ep = dev->endpoints[i]; + endpoint_add_ref(ep); + + fibril_mutex_unlock(&dev->guard); + bus_endpoint_remove(ep); + fibril_mutex_lock(&dev->guard); + + assert(dev->endpoints[i] == NULL); + } + + /* Remove also orphaned children. */ + while (!list_empty(&dev->devices)) { + device_t * const child = list_get_instance(list_first(&dev->devices), device_t, link); + + usb_log_warning("USB device '%s' driver left device '%s' behind after %s.", + ddf_fun_get_name(dev->fun), ddf_fun_get_name(child->fun), op); + /* + * The child node won't disappear, because its parent's driver + * is already dead. And the child will need the guard to remove + * itself from the list. + */ + fibril_mutex_unlock(&dev->guard); + bus_device_remove(child); + fibril_mutex_lock(&dev->guard); + } + assert(list_empty(&dev->devices)); +} + +/** + * Resolve a USB device that is gone. */ void bus_device_remove(device_t *dev) { assert(dev); + assert(dev->fun == NULL); const bus_ops_t *ops = BUS_OPS_LOOKUP(dev->bus->ops, device_remove); assert(ops); - return ops->device_remove(dev); -} - -/** - * Invoke the device_online bus operation. + /* First, block new transfers and operations. */ + fibril_mutex_lock(&dev->guard); + dev->online = false; + + /* Unbinding will need guard unlocked. */ + fibril_mutex_unlock(&dev->guard); + + /* Remove our device from our hub's children. */ + fibril_mutex_lock(&dev->hub->guard); + list_remove(&dev->link); + fibril_mutex_unlock(&dev->hub->guard); + + /* + * Unbind the DDF function. That will result in dev_gone called in + * driver, which shall destroy its pipes and remove its children. + */ + const int err = ddf_fun_unbind(dev->fun); + if (err) { + usb_log_error("Failed to unbind USB device '%s': %s", + ddf_fun_get_name(dev->fun), str_error(err)); + return; + } + + /* Remove what driver left behind */ + fibril_mutex_lock(&dev->guard); + device_clean_ep_children(dev, "removing"); + + /* Tell the HC to release its resources. */ + ops->device_remove(dev); + + /* Release the EP0 bus reference */ + endpoint_del_ref(dev->endpoints[0]); + + /* Destroy the function, freeing also the device, unlocking mutex. */ + ddf_fun_destroy(dev->fun); +} + +/** + * The user wants this device back online. */ int bus_device_online(device_t *dev) { + int err; assert(dev); + fibril_mutex_lock(&dev->guard); + if (dev->online) { + fibril_mutex_unlock(&dev->guard); + return EINVAL; + } + + /* First, tell the HC driver. */ const bus_ops_t *ops = BUS_OPS_LOOKUP(dev->bus->ops, device_online); - if (!ops) - return ENOTSUP; - - return ops->device_online(dev); -} - -/** - * Invoke the device_offline bus operation. + if (ops && (err = ops->device_online(dev))) { + usb_log_warning("Host controller refused to make device '%s' online: %s", + ddf_fun_get_name(dev->fun), str_error(err)); + return err; + } + + /* Allow creation of new endpoints and communication with the device. */ + dev->online = true; + + /* Onlining will need the guard */ + fibril_mutex_unlock(&dev->guard); + + if ((err = ddf_fun_online(dev->fun))) { + usb_log_warning("Failed to take device '%s' online: %s", + ddf_fun_get_name(dev->fun), str_error(err)); + return err; + } + + usb_log_info("USB Device '%s' offlined.", ddf_fun_get_name(dev->fun)); + return EOK; +} + +/** + * The user requested to take this device offline. */ int bus_device_offline(device_t *dev) { + int err; assert(dev); + /* Make sure we're the one who offlines this device */ + if (!dev->online) + return ENOENT; + + /* + * XXX: If the device is removed/offlined just now, this can fail on + * assertion. We most probably need some kind of enum status field to + * make the synchronization work. + */ + + /* Tear down all drivers working with the device. */ + if ((err = ddf_fun_offline(dev->fun))) { + return err; + } + + fibril_mutex_lock(&dev->guard); + dev->online = false; + device_clean_ep_children(dev, "offlining"); + + /* Tell also the HC driver. */ const bus_ops_t *ops = BUS_OPS_LOOKUP(dev->bus->ops, device_offline); - if (!ops) - return ENOTSUP; - - return ops->device_offline(dev); + if (ops) + ops->device_offline(dev); + + fibril_mutex_unlock(&dev->guard); + usb_log_info("USB Device '%s' offlined.", ddf_fun_get_name(dev->fun)); + return EOK; } @@ -269,7 +415,4 @@ fibril_mutex_unlock(&device->guard); - /* Abort a transfer batch, if there was any */ - endpoint_abort(ep); - /* Bus reference */ endpoint_del_ref(ep); Index: uspace/lib/usbhost/src/ddf_helpers.c =================================================================== --- uspace/lib/usbhost/src/ddf_helpers.c (revision bad4a050d11dd44a634a911a7e5a42918d984aab) +++ uspace/lib/usbhost/src/ddf_helpers.c (revision 0892663adc814c89139511bb2bea33fffe20ff46) @@ -336,12 +336,7 @@ assert(device); - hc_device_t *hcd = dev_to_hcd(device); - assert(hcd); - assert(hcd->bus); + device_t *victim = NULL; fibril_mutex_lock(&hub->guard); - - device_t *victim = NULL; - list_foreach(hub->devices, link, device_t, it) { if (it->port == port) { @@ -350,22 +345,15 @@ } } - if (victim) { - assert(victim->fun); - assert(victim->port == port); - assert(victim->hub == hub); - list_remove(&victim->link); - fibril_mutex_unlock(&hub->guard); - const int ret = ddf_fun_unbind(victim->fun); - if (ret == EOK) { - bus_device_remove(victim); - ddf_fun_destroy(victim->fun); - } else { - usb_log_warning("Failed to unbind device `%s': %s\n", - ddf_fun_get_name(victim->fun), str_error(ret)); - } - return EOK; - } fibril_mutex_unlock(&hub->guard); - return ENOENT; + + if (!victim) + return ENOENT; + + assert(victim->fun); + assert(victim->port == port); + assert(victim->hub == hub); + + bus_device_remove(victim); + return EOK; } @@ -477,8 +465,4 @@ } - fibril_mutex_lock(&hub->guard); - list_append(&dev->link, &hub->devices); - fibril_mutex_unlock(&hub->guard); - return EOK; Index: uspace/lib/usbhost/src/endpoint.c =================================================================== --- uspace/lib/usbhost/src/endpoint.c (revision bad4a050d11dd44a634a911a7e5a42918d984aab) +++ uspace/lib/usbhost/src/endpoint.c (revision 0892663adc814c89139511bb2bea33fffe20ff46) @@ -124,4 +124,15 @@ /** + * Wait until the endpoint have no transfer scheduled. + */ +void endpoint_wait_timeout_locked(endpoint_t *ep, suseconds_t timeout) +{ + assert(fibril_mutex_is_locked(&ep->guard)); + + while (ep->active_batch != NULL) + fibril_condvar_wait_timeout(&ep->avail, &ep->guard, timeout); +} + +/** * Mark the endpoint as active and block access for further fibrils. If the * endpoint is already active, it will block on ep->avail condvar. @@ -138,8 +149,6 @@ assert(batch); assert(batch->ep == ep); - assert(fibril_mutex_is_locked(&ep->guard)); - - while (ep->active_batch != NULL) - fibril_condvar_wait(&ep->avail, &ep->guard); + + endpoint_wait_timeout_locked(ep, 0); ep->active_batch = batch; } @@ -160,22 +169,4 @@ ep->active_batch = NULL; fibril_condvar_signal(&ep->avail); -} - -/** - * Abort an active batch on endpoint, if any. - * - * @param[in] ep endpoint_t structure. - */ -void endpoint_abort(endpoint_t *ep) -{ - assert(ep); - - fibril_mutex_lock(&ep->guard); - usb_transfer_batch_t *batch = ep->active_batch; - endpoint_deactivate_locked(ep); - fibril_mutex_unlock(&ep->guard); - - if (batch) - usb_transfer_batch_abort(batch); } Index: uspace/lib/usbhost/src/usb_transfer_batch.c =================================================================== --- uspace/lib/usbhost/src/usb_transfer_batch.c (revision bad4a050d11dd44a634a911a7e5a42918d984aab) +++ uspace/lib/usbhost/src/usb_transfer_batch.c (revision 0892663adc814c89139511bb2bea33fffe20ff46) @@ -128,16 +128,4 @@ /** - * Finish a transfer batch as an aborted one. - */ -void usb_transfer_batch_abort(usb_transfer_batch_t *batch) -{ - assert(batch); - assert(batch->ep); - - batch->error = EAGAIN; - usb_transfer_batch_finish(batch); -} - -/** * @} */