Changeset 04df063 in mainline

Timestamp:

2017-10-02T19:16:29Z (7 years ago)

Author:

Ondřej Hlavatý <aearsis@…>

Branches:

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

Children:

1f76b7d

Parents:

370a1c8

Message:

xhci commands: enable (and encourage) keeping commands on the stack

It is now not necessary to use the heap for simple commands.

Location:

uspace/drv/bus/usb/xhci

Files:

• commands.c (modified) (10 diffs)
• commands.h (modified) (3 diffs)
• rh.c (modified) (4 diffs)

uspace/drv/bus/usb/xhci/commands.c

@@ -73 +73 @@
         // Note: Untested.
         assert(hc);
-
-        // We assume that the hc is dying/stopping, so we ignore
-        // the ownership of the commands.
-        list_foreach(hc->commands, link, xhci_cmd_t, cmd) {
-                xhci_free_command(cmd);
-        }
-}
-
-int xhci_wait_for_command(xhci_cmd_t *cmd, suseconds_t timeout)
+}
+
+int xhci_cmd_wait(xhci_cmd_t *cmd, suseconds_t timeout)
 {
         int rv = EOK;
@@ -99 +93 @@
 }
 
-xhci_cmd_t *xhci_alloc_command(void)
+xhci_cmd_t *xhci_cmd_alloc(void)
 {
         xhci_cmd_t *cmd = malloc32(sizeof(xhci_cmd_t));
         xhci_cmd_init(cmd);
+
+        usb_log_debug2("Allocating cmd on the heap. Don't forget to deallocate it!");
         return cmd;
 }
@@ -114 +110 @@
         fibril_mutex_initialize(&cmd->completed_mtx);
         fibril_condvar_initialize(&cmd->completed_cv);
-
-        /**
-         * Internal functions will set this to false, other are implicit
-         * owners unless they overwrite this field.
-         * TODO: Is this wise?
-         */
-        cmd->has_owner = true;
-}
-
-void xhci_free_command(xhci_cmd_t *cmd)
+}
+
+void xhci_cmd_fini(xhci_cmd_t *cmd)
 {
         list_remove(&cmd->link);
-
-        if (cmd->ictx)
-                free32(cmd->ictx);
-
+}
+
+void xhci_cmd_free(xhci_cmd_t *cmd)
+{
+        xhci_cmd_fini(cmd);
         free32(cmd);
 }
@@ -299 +289 @@
 }
 
-int xhci_send_address_device_command(xhci_hc_t *hc, xhci_cmd_t *cmd)
-{
-        assert(hc);
-        assert(cmd);
-        assert(cmd->ictx);
+int xhci_send_address_device_command(xhci_hc_t *hc, xhci_cmd_t *cmd, xhci_input_ctx_t *ictx)
+{
+        assert(hc);
+        assert(cmd);
+        assert(ictx);
 
         /**
@@ -311 +301 @@
          *           other should be ignored at this point (see section 4.6.5).
          */
-        xhci_trb_clean(&cmd->trb);
-
-        uint64_t phys_addr = (uint64_t) addr_to_phys(cmd->ictx);
+
+        xhci_trb_clean(&cmd->trb);
+
+        uint64_t phys_addr = (uint64_t) addr_to_phys(ictx);
         TRB_SET_ICTX(cmd->trb, phys_addr);
 
@@ -329 +320 @@
 }
 
-int xhci_send_configure_endpoint_command(xhci_hc_t *hc, xhci_cmd_t *cmd)
-{
-        assert(hc);
-        assert(cmd);
-        assert(cmd->ictx);
-
-        xhci_trb_clean(&cmd->trb);
-
-        uint64_t phys_addr = (uint64_t) addr_to_phys(cmd->ictx);
+int xhci_send_configure_endpoint_command(xhci_hc_t *hc, xhci_cmd_t *cmd, xhci_input_ctx_t *ictx)
+{
+        assert(hc);
+        assert(cmd);
+        assert(ictx);
+
+        xhci_trb_clean(&cmd->trb);
+
+        uint64_t phys_addr = (uint64_t) addr_to_phys(ictx);
         TRB_SET_ICTX(cmd->trb, phys_addr);
 
@@ -346 +337 @@
 }
 
-int xhci_send_evaluate_context_command(xhci_hc_t *hc, xhci_cmd_t *cmd)
-{
-        assert(hc);
-        assert(cmd);
-        assert(cmd->ictx);
+int xhci_send_evaluate_context_command(xhci_hc_t *hc, xhci_cmd_t *cmd, xhci_input_ctx_t *ictx)
+{
+        assert(hc);
+        assert(cmd);
+        assert(ictx);
 
         /**
@@ -360 +351 @@
         xhci_trb_clean(&cmd->trb);
 
-        uint64_t phys_addr = (uint64_t) addr_to_phys(cmd->ictx);
+        uint64_t phys_addr = (uint64_t) addr_to_phys(ictx);
         TRB_SET_ICTX(cmd->trb, phys_addr);
 
@@ -455 +446 @@
         if (command == NULL) {
                 // TODO: STOP & ABORT may not have command structs in the list!
-                usb_log_error("No command struct for this completion event");
+                usb_log_debug("No command struct for this completion event found.");
 
                 if (code != XHCI_TRBC_SUCCESS)
@@ -509 +500 @@
         fibril_mutex_unlock(&command->completed_mtx);
 
-
-        if (!command->has_owner) {
-                usb_log_debug2("Command has no owner, deallocating.");
-                xhci_free_command(command);
-        } else {
-                usb_log_debug2("Command has owner, don't forget to deallocate!");
-        }
-
         return EOK;
 }

uspace/drv/bus/usb/xhci/commands.h

@@ -50 +50 @@
         xhci_trb_t trb;
         uintptr_t trb_phys;
-        xhci_input_ctx_t *ictx;
+
         uint32_t slot_id;
         uint32_t status;
 
         bool completed;
-        bool has_owner;
-        bool owns_trb;
 
-        /* Will be unlocked after command completes */
+        /* Will broadcast after command completes. */
         fibril_mutex_t completed_mtx;
         fibril_condvar_t completed_cv;
@@ -65 +63 @@
 int xhci_init_commands(xhci_hc_t *);
 void xhci_fini_commands(xhci_hc_t *);
-int xhci_wait_for_command(xhci_cmd_t *, uint32_t);
-xhci_cmd_t *xhci_alloc_command(void);
+
+xhci_cmd_t *xhci_cmd_alloc(void);
 void xhci_cmd_init(xhci_cmd_t *);
-void xhci_free_command(xhci_cmd_t *);
+int xhci_cmd_wait(xhci_cmd_t *, suseconds_t);
+void xhci_cmd_fini(xhci_cmd_t *);
+void xhci_cmd_free(xhci_cmd_t *);
 
 void xhci_stop_command_ring(xhci_hc_t *);
@@ -77 +77 @@
 int xhci_send_enable_slot_command(xhci_hc_t *, xhci_cmd_t *);
 int xhci_send_disable_slot_command(xhci_hc_t *, xhci_cmd_t *);
-int xhci_send_address_device_command(xhci_hc_t *, xhci_cmd_t *);
-int xhci_send_configure_endpoint_command(xhci_hc_t *, xhci_cmd_t *);
-int xhci_send_evaluate_context_command(xhci_hc_t *, xhci_cmd_t *);
+int xhci_send_address_device_command(xhci_hc_t *, xhci_cmd_t *, xhci_input_ctx_t *);
+int xhci_send_configure_endpoint_command(xhci_hc_t *, xhci_cmd_t *, xhci_input_ctx_t *);
+int xhci_send_evaluate_context_command(xhci_hc_t *, xhci_cmd_t *, xhci_input_ctx_t *);
 int xhci_send_reset_endpoint_command(xhci_hc_t *, xhci_cmd_t *, uint32_t, uint8_t);
 int xhci_send_stop_endpoint_command(xhci_hc_t *, xhci_cmd_t *, uint32_t, uint8_t);

uspace/drv/bus/usb/xhci/rh.c

@@ -63 +63 @@
         int err;
 
-        xhci_cmd_t *cmd = xhci_alloc_command();
-        if (!cmd)
-                return ENOMEM;
-
-        xhci_send_enable_slot_command(hc, cmd);
-        if ((err = xhci_wait_for_command(cmd, 100000)) != EOK) {
-                usb_log_error("Failed to enable a slot for the device.");
-                goto err_command;
-        }
-
-        uint32_t slot_id = cmd->slot_id;
+        xhci_cmd_t cmd;
+        xhci_cmd_init(&cmd);
+
+        xhci_send_enable_slot_command(hc, &cmd);
+        if ((err = xhci_cmd_wait(&cmd, 100000)) != EOK)
+                return err;
+
+        uint32_t slot_id = cmd.slot_id;
+
         usb_log_debug2("Obtained slot ID: %u.\n", slot_id);
-
-        xhci_free_command(cmd);
-        cmd = NULL;
+        xhci_cmd_fini(&cmd);
 
         xhci_input_ctx_t *ictx = malloc32(sizeof(xhci_input_ctx_t));
         if (!ictx) {
-                err = ENOMEM;
-                goto err_command;
+                return ENOMEM;
         }
 
@@ -90 +85 @@
         XHCI_INPUT_CTRL_CTX_ADD_SET(ictx->ctrl_ctx, 1);
 
-        /* Initialize slot_ctx according to section 4.3.3 point 3. */
+        /* Initialize slot_ctx according to section 4.3.3 point 3. */
         /* Attaching to root hub port, root string equals to 0. */
         XHCI_SLOT_ROOT_HUB_PORT_SET(ictx->slot_ctx, port);
@@ -133 +128 @@
         hc->dcbaa_virt[slot_id].dev_ctx = dctx;
 
-        cmd = xhci_alloc_command();
-        cmd->ictx = ictx;
-        xhci_send_address_device_command(hc, cmd);
-        if ((err = xhci_wait_for_command(cmd, 100000)) != EOK)
+        xhci_cmd_init(&cmd);
+        xhci_send_address_device_command(hc, &cmd, ictx);
+        if ((err = xhci_cmd_wait(&cmd, 100000)) != EOK)
                 goto err_dctx;
 
-        xhci_free_command(cmd);
-        ictx = NULL;
+        xhci_cmd_fini(&cmd);
 
         // TODO: Issue configure endpoint commands (sec 4.3.5).
@@ -158 +151 @@
         }
 err_ictx:
-        if (ictx) {
-                /* Avoid double free. */
-                if (cmd && cmd->ictx && cmd->ictx == ictx)
-                        cmd->ictx = NULL;
-                free32(ictx);
-        }
-err_command:
-        if (cmd)
-                xhci_free_command(cmd);
+        free32(ictx);
         return err;
 }