From: Wojciech Zajac Date: Mon, 16 Jun 2014 12:00:03 +0000 (+0200) Subject: Memory related fixes for DDE/USBD X-Git-Tag: v3.3.0~199 X-Git-Url: http://zhaoyanbai.com/repos/Bv9ARM.ch13.html?a=commitdiff_plain;h=bfff07c918383b5cb643378c1b6e6b1dbb8c0e7f;p=minix.git Memory related fixes for DDE/USBD DDEKit has proper thread termination now. No more memory leaks on usb stick removal. MUSB fixes for IRQ and memory mapping. --- diff --git a/drivers/usbd/README.txt b/drivers/usbd/README.txt index 265d30b98..25fd63da1 100755 --- a/drivers/usbd/README.txt +++ b/drivers/usbd/README.txt @@ -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 diff --git a/drivers/usbd/base/usbd.c b/drivers/usbd/base/usbd.c index 288f7f942..1925a5269 100755 --- a/drivers/usbd/base/usbd.c +++ b/drivers/usbd/base/usbd.c @@ -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; diff --git a/drivers/usbd/hcd/hcd_common.c b/drivers/usbd/hcd/hcd_common.c index 298d3f6df..562496612 100755 --- a/drivers/usbd/hcd/hcd_common.c +++ b/drivers/usbd/hcd/hcd_common.c @@ -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); diff --git a/drivers/usbd/hcd/musb/musb_am335x.c b/drivers/usbd/hcd/musb/musb_am335x.c index 04af74051..4c419904d 100755 --- a/drivers/usbd/hcd/musb/musb_am335x.c +++ b/drivers/usbd/hcd/musb/musb_am335x.c @@ -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"); } diff --git a/drivers/usbd/include/usb/hcd_common.h b/drivers/usbd/include/usb/hcd_common.h index 5a8acc171..72a55b9f1 100755 --- a/drivers/usbd/include/usb/hcd_common.h +++ b/drivers/usbd/include/usb/hcd_common.h @@ -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); diff --git a/lib/libddekit/src/thread.c b/lib/libddekit/src/thread.c index baa3868d3..bfca000fd 100644 --- a/lib/libddekit/src/thread.c +++ b/lib/libddekit/src/thread.c @@ -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 * ****************************************************************************/ diff --git a/lib/libddekit/src/thread.h b/lib/libddekit/src/thread.h index d79751c4b..bb05a45e9 100644 --- a/lib/libddekit/src/thread.h +++ b/lib/libddekit/src/thread.h @@ -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);