Changeset a94cbfa in mainline

Timestamp:

2018-01-26T00:55:38Z (7 years ago)

Author:

Ondřej Hlavatý <aearsis@…>

Branches:

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

Children:

77733a9

Parents:

047fbc8

git-author:

Ondřej Hlavatý <aearsis@…> (2018-01-26 00:15:17)

git-committer:

Ondřej Hlavatý <aearsis@…> (2018-01-26 00:55:38)

Message:

ehci+ohci: proper handling of unfinished transfer

Because transfers can be aborted, the RH may not store a pointer to
them. Also, it's no longer true that the batch automatically locks the
endpoint, the mechanism must be used explicitly.

Location:

uspace/drv/bus/usb

Files:

• ehci/ehci_rh.c (modified) (3 diffs)
• ehci/ehci_rh.h (modified) (1 diff)
• ohci/ohci_rh.c (modified) (3 diffs)
• ohci/ohci_rh.h (modified) (1 diff)

uspace/drv/bus/usb/ehci/ehci_rh.c

@@ -127 +127 @@
 
         ehci_rh_hub_desc_init(instance, EHCI_RD(caps->hcsparams));
-        instance->unfinished_interrupt_transfer = NULL;
+        instance->status_change_endpoint = NULL;
 
         return virthub_base_init(&instance->base, name, &ops, instance,
@@ -150 +150 @@
                 usb_log_debug("RH(%p): BATCH(%p) adding as unfinished",
                     instance, batch);
-                /* This is safe because only status change interrupt transfers
-                 * return NAK. The assertion holds true because the batch
-                 * existence prevents communication with that ep */
-                assert(instance->unfinished_interrupt_transfer == NULL);
-                instance->unfinished_interrupt_transfer = batch;
+
+                /* Lock the HC guard */
+                fibril_mutex_lock(batch->ep->guard);
+                const int err = endpoint_activate_locked(batch->ep, batch);
+                if (err) {
+                        fibril_mutex_unlock(batch->ep->guard);
+                        return err;
+                }
+
+                /*
+                 * Asserting that the HC do not run two instances of the status
+                 * change endpoint - shall be true.
+                 */
+                assert(!instance->status_change_endpoint);
+
+                endpoint_add_ref(batch->ep);
+                instance->status_change_endpoint = batch->ep;
+                fibril_mutex_unlock(batch->ep->guard);
         } else {
                 usb_log_debug("RH(%p): BATCH(%p) virtual request complete: %s",
@@ -172 +185 @@
 int ehci_rh_interrupt(ehci_rh_t *instance)
 {
-        //TODO atomic swap needed
-        usb_transfer_batch_t *batch = instance->unfinished_interrupt_transfer;
-        instance->unfinished_interrupt_transfer = NULL;
-        usb_log_debug2("RH(%p): Interrupt. Processing batch: %p",
-            instance, batch);
+        endpoint_t *ep = instance->status_change_endpoint;
+        if (!ep)
+                return EOK;
+
+        fibril_mutex_lock(ep->guard);
+        usb_transfer_batch_t * const batch = ep->active_batch;
+        endpoint_deactivate_locked(ep);
+        instance->status_change_endpoint = NULL;
+        fibril_mutex_unlock(ep->guard);
+
+        endpoint_del_ref(ep);
+
         if (batch) {
+                usb_log_debug2("RH(%p): Interrupt. Processing batch: %p",
+                    instance, batch);
                 batch->error = virthub_base_request(&instance->base, batch->target,
                     batch->dir, (void*) batch->setup.buffer,

uspace/drv/bus/usb/ehci/ehci_rh.h

@@ -61 +61 @@
                 uint8_t rempow[STATUS_BYTES(EHCI_MAX_PORTS) * 2];
         } __attribute__((packed)) hub_descriptor;
-        /** interrupt transfer waiting for an actual interrupt to occur */
-        usb_transfer_batch_t *unfinished_interrupt_transfer;
         bool reset_flag[EHCI_MAX_PORTS];
         bool resume_flag[EHCI_MAX_PORTS];
+
+        /*
+         * This is sort of hacky, but better than duplicating functionality.
+         * We cannot simply store a pointer to a transfer in-progress, in order
+         * to allow it to be aborted. We can however store a reference to the
+         * Status Change Endpoint. Note that this is mixing two worlds together
+         * - otherwise, the RH is "a device" and have no clue about HC, apart
+         * from accessing its registers.
+         */
+        endpoint_t *status_change_endpoint;
+
 } ehci_rh_t;
 

uspace/drv/bus/usb/ohci/ohci_rh.c

@@ -162 +162 @@
 
         ohci_rh_hub_desc_init(instance);
-        instance->unfinished_interrupt_transfer = NULL;
+        instance->status_change_endpoint = NULL;
         return virthub_base_init(&instance->base, name, &ops, instance,
             NULL, &instance->hub_descriptor.header, HUB_STATUS_CHANGE_PIPE);
@@ -182 +182 @@
             batch->buffer, batch->buffer_size, &batch->transferred_size);
         if (batch->error == ENAK) {
-                /* This is safe because only status change interrupt transfers
-                 * return NAK. The assertion holds true because the batch
-                 * existence prevents communication with that ep */
-                assert(instance->unfinished_interrupt_transfer == NULL);
-                instance->unfinished_interrupt_transfer = batch;
+                /* Lock the HC guard */
+                fibril_mutex_lock(batch->ep->guard);
+                const int err = endpoint_activate_locked(batch->ep, batch);
+                if (err) {
+                        fibril_mutex_unlock(batch->ep->guard);
+                        return err;
+                }
+
+                /*
+                 * Asserting that the HC do not run two instances of the status
+                 * change endpoint - shall be true.
+                 */
+                assert(!instance->status_change_endpoint);
+
+                endpoint_add_ref(batch->ep);
+                instance->status_change_endpoint = batch->ep;
+                fibril_mutex_unlock(batch->ep->guard);
         } else {
                 usb_transfer_batch_finish(batch);
@@ -202 +214 @@
 int ohci_rh_interrupt(ohci_rh_t *instance)
 {
-        //TODO atomic swap needed
-        usb_transfer_batch_t *batch = instance->unfinished_interrupt_transfer;
-        instance->unfinished_interrupt_transfer = NULL;
+        endpoint_t *ep = instance->status_change_endpoint;
+        if (!ep)
+                return EOK;
+
+        fibril_mutex_lock(ep->guard);
+        usb_transfer_batch_t * const batch = ep->active_batch;
+        endpoint_deactivate_locked(ep);
+        instance->status_change_endpoint = NULL;
+        fibril_mutex_unlock(ep->guard);
+
+        endpoint_del_ref(ep);
+
         if (batch) {
                 batch->error = virthub_base_request(&instance->base, batch->target,

uspace/drv/bus/usb/ohci/ohci_rh.h

@@ -61 +61 @@
                 uint8_t rempow[STATUS_BYTES(OHCI_MAX_PORTS) * 2];
         } __attribute__((packed)) hub_descriptor;
-        /** interrupt transfer waiting for an actual interrupt to occur */
-        usb_transfer_batch_t *unfinished_interrupt_transfer;
+        /** A hacky way to emulate interrupts over polling. See ehci_rh. */
+        endpoint_t *status_change_endpoint;
 } ohci_rh_t;