]> Zhao Yanbai Git Server - minix.git/commitdiff
Memory related fixes for DDE/USBD
authorWojciech Zajac <wzajac@jpembedded.eu>
Mon, 16 Jun 2014 12:00:03 +0000 (14:00 +0200)
committerLionel Sambuc <lionel@minix3.org>
Mon, 28 Jul 2014 15:05:51 +0000 (17:05 +0200)
DDEKit has proper thread termination now. No more memory leaks on usb stick removal.

MUSB fixes for IRQ and memory mapping.

drivers/usbd/README.txt
drivers/usbd/base/usbd.c
drivers/usbd/hcd/hcd_common.c
drivers/usbd/hcd/musb/musb_am335x.c
drivers/usbd/include/usb/hcd_common.h
lib/libddekit/src/thread.c
lib/libddekit/src/thread.h

index 265d30b984cf02405fd8995a4c166bd010f28d91..25fd63da1a3f22b3bd03469f59764567afcdcd5b 100755 (executable)
@@ -10,12 +10,10 @@ created march-june 2014, JPEmbedded (info@jpembedded.eu)
 -------------------------------------------------------------------------------
 - Only first configuration can be selected for attached device
 - Only one device can be handled at a time, no hub functionality
-- DDEKit does not implement resource deallocation for corresponding thread
-  creation (see ddekit_thread_terminate, ddekit_thread_create) thus resources
-  are spilled
 - Driver assumes that there is no preemption for DDEKit threading
 - URBs are enqueued in DDEKit but not in USBD itself
 - DDEKit way of handling interface numbers is not explicitly defined, bitmask
   formatting for it, is therefore hardcoded into USBD
 - Waiting for USB0 clock to leave IDLEST.Disable state, by nanosleep, was
-  removed, as this should be implemented for all clocks in clkconf_set
\ No newline at end of file
+  removed, as this should be implemented for all clocks in clkconf_set
+- Control transfers can only be performed with EP0
\ No newline at end of file
index 288f7f942d221367c5f318260c3a46ea7eb3db15..1925a5269f5db3892e37de22dc7f442cf5b486be 100755 (executable)
@@ -100,12 +100,21 @@ usbd_sef_handler(int type, sef_init_info_t * UNUSED(info))
 static int
 usbd_start(void)
 {
+       ddekit_thread_t * usbd_th;
+
        DEBUG_DUMP;
 
        /* Driver's "main loop" is within DDEKit server thread */
-       if (NULL != ddekit_thread_create(usbd_server_thread, NULL, "USBD")) {
-               /* This will lock current thread until DDEKit terminates */
+       usbd_th = ddekit_thread_create(usbd_server_thread, NULL, "USBD");
+
+       /* After spawning, allow server thread to work */
+       if (NULL != usbd_th) {
+               /* This will lock current thread until DDEKit exits */
                ddekit_minix_wait_exit();
+
+               /* Cleanup */
+               ddekit_thread_terminate(usbd_th);
+
                return EXIT_SUCCESS;
        } else
                return EXIT_FAILURE;
index 298d3f6dfff45dd1211822b8b28096679140cd73..56249661298c7132f1ef72c7f4601b8ea361ff68 100755 (executable)
@@ -81,7 +81,7 @@ hcd_os_interrupt_disable(int irq)
  *    hcd_os_regs_init                                                       *
  *===========================================================================*/
 void *
-hcd_os_regs_init(unsigned long base_addr, unsigned long addr_len)
+hcd_os_regs_init(hcd_addr phys_addr, unsigned long addr_len)
 {
        /* Memory range where we need privileged access */
        struct minix_mem_range mr;
@@ -94,18 +94,18 @@ hcd_os_regs_init(unsigned long base_addr, unsigned long addr_len)
        virt_reg_base = NULL;
 
        /* Must have been set before */
-       USB_ASSERT(0 != base_addr, "Invalid base address!");
+       USB_ASSERT(0 != phys_addr, "Invalid base address!");
        USB_ASSERT(0 != addr_len, "Invalid base length!");
 
        /* Set memory range for peripheral */
-       mr.mr_base = base_addr;
-       mr.mr_limit = base_addr + addr_len;
+       mr.mr_base = phys_addr;
+       mr.mr_limit = phys_addr + addr_len;
 
        /* Try getting access to memory range */
        if (EXIT_SUCCESS == sys_privctl(SELF, SYS_PRIV_ADD_MEM, &mr)) {
 
                /* And map it where we want it */
-               virt_reg_base = vm_map_phys(SELF, (void *)base_addr, addr_len);
+               virt_reg_base = vm_map_phys(SELF, (void *)phys_addr, addr_len);
 
                /* Check for mapping errors to allow us returning NULL */
                if (MAP_FAILED == virt_reg_base) {
@@ -124,12 +124,12 @@ hcd_os_regs_init(unsigned long base_addr, unsigned long addr_len)
  *    hcd_os_regs_deinit                                                     *
  *===========================================================================*/
 int
-hcd_os_regs_deinit(unsigned long base_addr, unsigned long addr_len)
+hcd_os_regs_deinit(hcd_addr virt_addr, unsigned long addr_len)
 {
        DEBUG_DUMP;
 
        /* To keep USBD return value convention */
-       return (0 == vm_unmap_phys(SELF, (void*)base_addr, addr_len)) ?
+       return (0 == vm_unmap_phys(SELF, (void*)virt_addr, addr_len)) ?
                EXIT_SUCCESS : EXIT_FAILURE;
 }
 
@@ -226,8 +226,6 @@ hcd_disconnect_device(hcd_device_state * this_device)
 
        hcd_tree_cleanup(&(this_device->config_tree));
 
-       /* TODO: Spilled resources */
-       /* DDEKit does no resource deallocation, when terminating thread */
        ddekit_thread_terminate(this_device->thread);
        ddekit_sem_deinit(this_device->lock);
 
index 04af7405100b3c6c1ba9fc84b60d863a1a33aad8..4c419904d6976e19838d609f35c47b4f3bd9d1b0 100755 (executable)
@@ -404,9 +404,18 @@ musb_am335x_deinit(void)
 
        musb_am335x_internal_deinit();
 
+       /* Release IRQ resources */
+       /* TODO: DDEKit has no checks on NULL IRQ and may crash on detach
+        * if interrupts were not attached properly */
+       hcd_os_interrupt_detach(AM335X_USB1_IRQ);
+#ifdef AM335X_USE_USB0
+       hcd_os_interrupt_detach(AM335X_USB0_IRQ);
+#endif
+       hcd_os_interrupt_detach(AM335X_USBSS_IRQ);
+
        /* Release maps if anything was assigned */
        if (NULL != am335x.ss.regs)
-               if (EXIT_SUCCESS != hcd_os_regs_deinit(AM335X_USBSS_BASE_ADDR,
+               if (EXIT_SUCCESS != hcd_os_regs_deinit((hcd_addr)am335x.ss.regs,
                                                AM335X_USBSS_TOTAL_REG_LEN))
                        USB_MSG("Failed to release USBSS mapping");
 }
index 5a8acc1715ab2b00601156a69ddc497021943613..72a55b9f15c15d569dd4d27dd05f758f107ff9cd 100755 (executable)
@@ -285,10 +285,10 @@ void hcd_os_interrupt_enable(int);
 void hcd_os_interrupt_disable(int);
 
 /* Returns pointer to memory mapped for given arguments */
-void * hcd_os_regs_init(unsigned long, unsigned long);
+void * hcd_os_regs_init(hcd_addr, unsigned long);
 
 /* Unregisters mapped memory */
-int hcd_os_regs_deinit(unsigned long, unsigned long);
+int hcd_os_regs_deinit(hcd_addr, unsigned long);
 
 /* Configure clocking */
 int hcd_os_clkconf(unsigned long, unsigned long, unsigned long);
index baa3868d3a77058cf839ab8050c58c4d4a04bfc4..bfca000fdc7f5b048340432b0da4f14acd28156b 100644 (file)
@@ -31,6 +31,7 @@ static ddekit_thread_t *current = NULL;
 
 static void _ddekit_thread_start(ddekit_thread_t *th);
 static void _ddekit_thread_sleep(unsigned long until);
+static void _ddekit_dump_queues(void);
 
 /*****************************************************************************
  *    _ddekit_thread_start                                                   *
@@ -54,6 +55,46 @@ static void _ddekit_thread_sleep(unsigned long until)
 
 }
 
+/*****************************************************************************
+ *    _ddekit_dump_queues                                                    *
+ ****************************************************************************/
+static void
+_ddekit_dump_queues(void)
+{
+#if DDEBUG >= DDEBUG_VERBOSE
+       ddekit_thread_t * current_thread;
+       int i;
+
+       for (i = 0; i < DDEKIT_THREAD_PRIOS; i++) {
+               current_thread = ready_queue[i];
+
+               ddekit_printf("Ready queue #%d: ", i);
+
+               while (NULL != current_thread) {
+                       ddekit_printf("0x%08X ", (int)current_thread);
+                       current_thread = current_thread->next;
+               }
+
+               ddekit_printf("\n");
+       }
+
+       {
+               current_thread = sleep_queue;
+
+               ddekit_printf("Sleep queue: ");
+
+               while (NULL != current_thread) {
+                       ddekit_printf("0x%08X ", (int)current_thread);
+                       current_thread = current_thread->next;
+               }
+
+               ddekit_printf("\n");
+       }
+
+       ddekit_printf("Current thread: 0x%08X\n", (int)current);
+#endif
+}
+
 /*****************************************************************************
  *    DDEKIT public thread API (ddekit/thread.h)                             *
  ****************************************************************************/
@@ -274,9 +315,23 @@ void  ddekit_thread_exit()
 /*****************************************************************************
  *    ddekit_thread_terminate                                                *
  ****************************************************************************/
-void  ddekit_thread_terminate(ddekit_thread_t *thread)
+void
+ddekit_thread_terminate(ddekit_thread_t * thread)
 {
-       /* todo */
+       if (thread == ddekit_thread_myself()) {
+               /* TODO: Whether or not this is an error, is to be decided.
+                * Memory (especially stack) freeing should be somehow
+                * postponed when such termination is legal. */
+               ddekit_panic("Thread attempted termination of itself!\n");
+       }
+
+       _ddekit_thread_dequeue(thread);
+
+       ddekit_sem_deinit(thread->sleep_sem);
+
+       ddekit_simple_free(thread->stack);
+
+       ddekit_simple_free(thread);
 }
 
 /*****************************************************************************
@@ -390,9 +445,8 @@ void _ddekit_thread_schedule()
  ****************************************************************************/
 void _ddekit_thread_enqueue(ddekit_thread_t *th) 
 {
-       
-       DDEBUG_MSG_VERBOSE("enqueueing thread: id: %d name %s, prio: %d",
-               th->id, th->name, th->prio);
+       DDEBUG_MSG_VERBOSE("Enqueuing thread 0x%08X: id %d, name %s, prio %d",
+                       (int)th, th->id, th->name, th->prio);
 
 #if DDEBUG >= 4
        _ddekit_print_backtrace(th);
@@ -411,6 +465,83 @@ void _ddekit_thread_enqueue(ddekit_thread_t *th)
        }
 }
 
+/*****************************************************************************
+ *    _ddekit_thread_dequeue                                                 *
+ ****************************************************************************/
+void
+_ddekit_thread_dequeue(ddekit_thread_t * th)
+{
+       ddekit_thread_t * current_thread;
+       ddekit_thread_t * previous_thread;
+
+       DDEBUG_MSG_VERBOSE("Dequeuing thread 0x%08X: id %d, name %s, prio %d",
+                       (int)th, th->id, th->name, th->prio);
+
+       ddekit_assert((th->prio < DDEKIT_THREAD_PRIOS) && (th->prio >= 0));
+
+       /* Dump queues when debugging */
+       _ddekit_dump_queues();
+
+       /* Check ready queue (based on thread's priority) for thread */
+       current_thread = ready_queue[th->prio];
+       previous_thread = NULL;
+
+       while (NULL != current_thread) {
+
+               /* On match... */
+               if (th == current_thread) {
+
+                       if (previous_thread) {
+                               /* ...fix previous element to remove current */
+                               previous_thread->next = current_thread->next;
+                       } else {
+                               /* ...alter queue start to reflect removal */
+                               ready_queue[th->prio] = current_thread->next;
+                       }
+
+                       /* Thread found and dequeued */
+                       DDEBUG_MSG_VERBOSE("Dequeued 'ready[%d]': 0x%08X",
+                                       th->prio, (int)th);
+                       return;
+               }
+
+               /* Next thread */
+               previous_thread = current_thread;
+               current_thread = current_thread->next;
+       }
+
+       /* When previous loop fails, check if thread is sleeping */
+       current_thread = sleep_queue;
+       previous_thread = NULL;
+
+       while (NULL != current_thread) {
+
+               /* On match... */
+               if (th == current_thread) {
+
+                       if (previous_thread) {
+                               /* ...fix previous element to remove current */
+                               previous_thread->next = current_thread->next;
+                       } else {
+                               /* ...alter queue start to reflect removal */
+                               sleep_queue = current_thread->next;
+                       }
+
+                       /* Thread found and dequeued */
+                       DDEBUG_MSG_VERBOSE("Dequeued 'sleep': 0x%08X", (int)th);
+                       return;
+               }
+
+               /* Next thread */
+               previous_thread = current_thread;
+               current_thread = current_thread->next;
+       }
+
+       /* Thread may exist and not be enqueued at
+        * all (is bound to semaphore for instance) */
+       DDEBUG_MSG_VERBOSE("Thread 0x%08X was not enqueued!", (int)th);
+}
+
 /*****************************************************************************
  *    _ddekit_thread_set_myprio                                              *
  ****************************************************************************/
index d79751c4b2a1d1d6db3d61c3b09bba0bad9c1a9e..bb05a45e904c20a9e2f19ddee6a1488bcd38f88c 100644 (file)
@@ -33,7 +33,8 @@ struct ddekit_thread {
 
 
 void _ddekit_thread_set_myprio(int prio);
-void _ddekit_thread_enqueue(ddekit_thread_t *th); 
+void _ddekit_thread_enqueue(ddekit_thread_t *th);
+void _ddekit_thread_dequeue(ddekit_thread_t *th);
 void _ddekit_thread_schedule();
 void _ddekit_thread_wakeup_sleeping();
 void _ddekit_print_backtrace(ddekit_thread_t *th);