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.

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
+  removed, as this should be implemented for all clocks in clkconf_set
+- Control transfers can only be performed with EP0

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")) {
+	usbd_th = ddekit_thread_create(usbd_server_thread, NULL, "USBD");
-		/* This will lock current thread until DDEKit terminates */
+
+	/* 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;

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_base = phys_addr;
-	mr.mr_limit = base_addr + addr_len;
+	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);
 

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");
 }

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);

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("Enqueuing thread 0x%08X: id %d, name %s, prio %d",
-	DDEBUG_MSG_VERBOSE("enqueueing thread: id: %d name %s, prio: %d",
+			(int)th, th->id, th->name, th->prio);
-		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                                              *
  ****************************************************************************/

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);