Index: uspace/drv/bus/usb/uhci/hc.c =================================================================== --- uspace/drv/bus/usb/uhci/hc.c (revision 3ac86a42de2e8948db2e3047654b63338c68e55f) +++ uspace/drv/bus/usb/uhci/hc.c (revision 4db49344094408c5a4e6654a2a2813509a0c351b) @@ -97,5 +97,5 @@ static void hc_init_hw(const hc_t *instance); -static int hc_init_mem_structures(hc_t *instance, hc_device_t *); +static int hc_init_mem_structures(hc_t *instance); static int hc_init_transfer_lists(hc_t *instance); @@ -164,21 +164,10 @@ /* Lower 2 bits are transaction error and transaction complete */ if (status & (UHCI_STATUS_INTERRUPT | UHCI_STATUS_ERROR_INTERRUPT)) { - LIST_INITIALIZE(done); - transfer_list_remove_finished( - &instance->transfers_interrupt, &done); - transfer_list_remove_finished( - &instance->transfers_control_slow, &done); - transfer_list_remove_finished( - &instance->transfers_control_full, &done); - transfer_list_remove_finished( - &instance->transfers_bulk_full, &done); - - list_foreach_safe(done, current, next) { - list_remove(current); - uhci_transfer_batch_t *batch = - uhci_transfer_batch_from_link(current); - usb_transfer_batch_finish(&batch->base); - } - } + transfer_list_check_finished(&instance->transfers_interrupt); + transfer_list_check_finished(&instance->transfers_control_slow); + transfer_list_check_finished(&instance->transfers_control_full); + transfer_list_check_finished(&instance->transfers_bulk_full); + } + /* Resume interrupts are not supported */ if (status & UHCI_STATUS_RESUME) { @@ -239,5 +228,5 @@ hw_res->io_ranges.ranges[0].size); - ret = hc_init_mem_structures(instance, hcd); + ret = hc_init_mem_structures(instance); if (ret != EOK) { usb_log_error("Failed to init UHCI memory structures: %s.", @@ -328,10 +317,30 @@ } +static int endpoint_register(endpoint_t *ep) +{ + hc_t * const hc = bus_to_hc(endpoint_get_bus(ep)); + + const int err = usb2_bus_ops.endpoint_register(ep); + if (err) + return err; + + transfer_list_t *list = hc->transfers[ep->device->speed][ep->transfer_type]; + if (!list) + /* + * We don't support this combination (e.g. isochronous). Do not + * fail early, because that would block any device with these + * endpoints from connecting. Instead, make sure these transfers + * are denied soon enough with ENOTSUP not to fail on asserts. + */ + return EOK; + + endpoint_set_online(ep, &list->guard); + return EOK; +} + static void endpoint_unregister(endpoint_t *ep) { hc_t * const hc = bus_to_hc(endpoint_get_bus(ep)); usb2_bus_ops.endpoint_unregister(ep); - - uhci_transfer_batch_t *batch = NULL; // Check for the roothub, as it does not schedule into lists @@ -344,5 +353,4 @@ transfer_list_t *list = hc->transfers[ep->device->speed][ep->transfer_type]; - if (!list) /* @@ -352,23 +360,40 @@ return; - // To avoid ABBA deadlock, we need to take the list first fibril_mutex_lock(&list->guard); - fibril_mutex_lock(&ep->guard); - if (ep->active_batch) { - batch = uhci_transfer_batch_get(ep->active_batch); - endpoint_deactivate_locked(ep); - transfer_list_remove_batch(list, batch); - } - fibril_mutex_unlock(&ep->guard); + + endpoint_set_offline_locked(ep); + /* From now on, no other transfer will be scheduled. */ + + if (!ep->active_batch) { + fibril_mutex_unlock(&list->guard); + return; + } + + /* First, offer the batch a short chance to be finished. */ + endpoint_wait_timeout_locked(ep, 10000); + + if (!ep->active_batch) { + fibril_mutex_unlock(&list->guard); + return; + } + + uhci_transfer_batch_t * const batch = + uhci_transfer_batch_get(ep->active_batch); + + /* Remove the batch from the schedule to stop it from being finished. */ + endpoint_deactivate_locked(ep); + transfer_list_remove_batch(list, batch); + fibril_mutex_unlock(&list->guard); - if (batch) { - // The HW could have been looking at the batch. - // Better wait two frames before we release the buffers. - async_usleep(2000); - batch->base.error = EINTR; - batch->base.transferred_size = 0; - usb_transfer_batch_finish(&batch->base); - } + /* + * We removed the batch from software schedule only, it's still possible + * that HC has it in its caches. Better wait a while before we release + * the buffers. + */ + async_usleep(20000); + batch->base.error = EINTR; + batch->base.transferred_size = 0; + usb_transfer_batch_finish(&batch->base); } @@ -382,4 +407,5 @@ .status = hc_status, + .endpoint_register = endpoint_register, .endpoint_unregister = endpoint_unregister, .endpoint_count_bw = bandwidth_count_usb11, @@ -400,5 +426,5 @@ * - frame list page (needs to be one UHCI hw accessible 4K page) */ -int hc_init_mem_structures(hc_t *instance, hc_device_t *hcd) +int hc_init_mem_structures(hc_t *instance) { assert(instance); @@ -425,4 +451,5 @@ return ENOMEM; } + list_initialize(&instance->pending_endpoints); usb_log_debug("Initialized transfer lists."); @@ -514,11 +541,10 @@ } -/** Schedule batch for execution. +/** + * Schedule batch for execution. * * @param[in] instance UHCI structure to use. * @param[in] batch Transfer batch to schedule. * @return Error code - * - * Checks for bandwidth availability and appends the batch to the proper queue. */ static int hc_schedule(usb_transfer_batch_t *batch) @@ -531,20 +557,15 @@ return uhci_rh_schedule(&hc->rh, batch); - - const int err = uhci_transfer_batch_prepare(uhci_batch); - if (err) + transfer_list_t * const list = + hc->transfers[ep->device->speed][ep->transfer_type]; + + if (!list) + return ENOTSUP; + + int err; + if ((err = uhci_transfer_batch_prepare(uhci_batch))) return err; - transfer_list_t *list = hc->transfers[ep->device->speed][ep->transfer_type]; - assert(list); - transfer_list_add_batch(list, uhci_batch); - - return EOK; -} - -int hc_unschedule_batch(usb_transfer_batch_t *batch) -{ - - return EOK; + return transfer_list_add_batch(list, uhci_batch); } Index: uspace/drv/bus/usb/uhci/hc.h =================================================================== --- uspace/drv/bus/usb/uhci/hc.h (revision 3ac86a42de2e8948db2e3047654b63338c68e55f) +++ uspace/drv/bus/usb/uhci/hc.h (revision 4db49344094408c5a4e6654a2a2813509a0c351b) @@ -123,4 +123,12 @@ transfer_list_t *transfers[2][4]; + /** + * Guard for the pending list. Can be locked under EP guard, but not + * vice versa. + */ + fibril_mutex_t guard; + /** List of endpoints with a transfer scheduled */ + list_t pending_endpoints; + /** Number of hw failures detected. */ unsigned hw_failures; Index: uspace/drv/bus/usb/uhci/transfer_list.c =================================================================== --- uspace/drv/bus/usb/uhci/transfer_list.c (revision 3ac86a42de2e8948db2e3047654b63338c68e55f) +++ uspace/drv/bus/usb/uhci/transfer_list.c (revision 4db49344094408c5a4e6654a2a2813509a0c351b) @@ -102,12 +102,14 @@ } -/** Add transfer batch to the list and queue. - * - * @param[in] instance List to use. - * @param[in] batch Transfer batch to submit. +/** + * Add transfer batch to the list and queue. * * The batch is added to the end of the list and queue. - */ -void transfer_list_add_batch( + * + * @param[in] instance List to use. + * @param[in] batch Transfer batch to submit. After return, the batch must + * not be used further. + */ +int transfer_list_add_batch( transfer_list_t *instance, uhci_transfer_batch_t *uhci_batch) { @@ -117,13 +119,14 @@ endpoint_t *ep = uhci_batch->base.ep; - /* First, wait until the endpoint is free to use */ - fibril_mutex_lock(&ep->guard); - endpoint_activate_locked(ep, &uhci_batch->base); - fibril_mutex_unlock(&ep->guard); + fibril_mutex_lock(&instance->guard); + + const int err = endpoint_activate_locked(ep, &uhci_batch->base); + if (err) { + fibril_mutex_unlock(&instance->guard); + return err; + } usb_log_debug2("Batch %p adding to queue %s.", uhci_batch, instance->name); - - fibril_mutex_lock(&instance->guard); /* Assume there is nothing scheduled */ @@ -155,4 +158,5 @@ USB_TRANSFER_BATCH_ARGS(uhci_batch->base), instance->name); fibril_mutex_unlock(&instance->guard); + return EOK; } @@ -171,28 +175,19 @@ * @param[in] done list to fill */ -void transfer_list_remove_finished(transfer_list_t *instance, list_t *done) -{ - assert(instance); - assert(done); +void transfer_list_check_finished(transfer_list_t *instance) +{ + assert(instance); fibril_mutex_lock(&instance->guard); - link_t *current = list_first(&instance->batch_list); - while (current && current != &instance->batch_list.head) { - link_t * const next = current->next; - uhci_transfer_batch_t *batch = - uhci_transfer_batch_from_link(current); + list_foreach_safe(instance->batch_list, current, next) { + uhci_transfer_batch_t *batch = uhci_transfer_batch_from_link(current); if (uhci_transfer_batch_check_completed(batch)) { - /* Remove from schedule, save for processing */ - fibril_mutex_lock(&batch->base.ep->guard); assert(batch->base.ep->active_batch == &batch->base); + endpoint_deactivate_locked(batch->base.ep); hc_reset_toggles(&batch->base, &uhci_reset_toggle); - endpoint_deactivate_locked(batch->base.ep); transfer_list_remove_batch(instance, batch); - fibril_mutex_unlock(&batch->base.ep->guard); - - list_append(current, done); + usb_transfer_batch_finish(&batch->base); } - current = next; } fibril_mutex_unlock(&instance->guard); Index: uspace/drv/bus/usb/uhci/transfer_list.h =================================================================== --- uspace/drv/bus/usb/uhci/transfer_list.h (revision 3ac86a42de2e8948db2e3047654b63338c68e55f) +++ uspace/drv/bus/usb/uhci/transfer_list.h (revision 4db49344094408c5a4e6654a2a2813509a0c351b) @@ -59,7 +59,7 @@ int transfer_list_init(transfer_list_t *, const char *); void transfer_list_set_next(transfer_list_t *, transfer_list_t *); -void transfer_list_add_batch(transfer_list_t *, uhci_transfer_batch_t *); +int transfer_list_add_batch(transfer_list_t *, uhci_transfer_batch_t *); void transfer_list_remove_batch(transfer_list_t *, uhci_transfer_batch_t *); -void transfer_list_remove_finished(transfer_list_t *, list_t *); +void transfer_list_check_finished(transfer_list_t *); void transfer_list_abort_all(transfer_list_t *);