[PATCH] small fixes for usb-uhci-hcd

the attached patch for usb-uhci-hcd includes the possibility to specify the
FSBR-mode and depth-first-search-modes via module parameters. Thanks go to
Kevin (kjsisson@bellsouth.net) for this nice idea. He had problems with
stv0680-based cameras when using the default (breadth first) methods.

The interval-value for isochronous transfers is also now supported.

Additionally the patch removes a few typos, obsolete comments+code and
a few non-portable variable declarations.

drivers/usb/host/usb-uhci-dbg.c

@@ -5,7 +5,7 @@
     Georg Acher      +    Deti Fliegl    +    Thomas Sailer
     georg@acher.org      deti@fliegl.de   sailer@ife.ee.ethz.ch
   
-    $Id: usb-uhci-dbg.c,v 1.1 2002/05/14 20:36:57 acher Exp $
+    $Id: usb-uhci-dbg.c,v 1.2 2002/05/21 21:40:16 acher Exp $
 */
 
 #ifdef DEBUG
@@ -105,7 +105,7 @@ static void __attribute__((__unused__)) uhci_show_sc (int port, unsigned short s
 
 void uhci_show_status (struct uhci_hcd *uhci)
 {
-	unsigned int io_addr = (int)uhci->hcd.regs;
+	unsigned long io_addr = (unsigned long)uhci->hcd.regs;
 	unsigned short usbcmd, usbstat, usbint, usbfrnum;
 	unsigned int flbaseadd;
 	unsigned char sof;

drivers/usb/host/usb-uhci-hcd.c

@@ -13,7 +13,7 @@
     HW-initalization based on material of
     Randy Dunlap + Johannes Erdfelt + Gregory P. Smith + Linus Torvalds 
 
-    $Id: usb-uhci-hcd.c,v 1.1 2002/05/14 20:36:57 acher Exp $
+    $Id: usb-uhci-hcd.c,v 1.3 2002/05/25 16:42:41 acher Exp $
  */
 
 #include <linux/config.h>
@@ -37,7 +37,6 @@
 #include <asm/byteorder.h>
 #include <linux/usb.h>
 
-#define CONFIG_USB_DEBUG
 #ifdef CONFIG_USB_DEBUG
 	#define DEBUG
 #else
@@ -47,34 +46,34 @@
 #include "../core/hcd.h"
 #include "usb-uhci-hcd.h"
 
-#define DRIVER_VERSION "$Revision: 1.1 $"
+#define DRIVER_VERSION "$Revision: 1.3 $"
 #define DRIVER_AUTHOR "Georg Acher, Deti Fliegl, Thomas Sailer"
 #define DRIVER_DESC "USB 1.1 Universal Host Controller Interface driver (HCD)"
-/*--------------------------------------------------------------------------*/
-// Values you may tweak
 
-/* CONFIG_USB_UHCI_HIGH_BANDWITH turns on Full Speed Bandwidth
- * Reclamation: feature that puts loop on descriptor loop when
+/*--------------------------------------------------------------------------*/
+/* Values you may tweak with module parameters
+ *  
+ * high_bw: 1=on (default), 0=off
+ * Turns on Full Speed Bandwidth Reclamation: 
+ * Feature that puts a loop on the descriptor chain when
  * there's some transfer going on. With FSBR, USB performance
  * is optimal, but PCI can be slowed down up-to 5 times, slowing down
  * system performance (eg. framebuffer devices).
- */
-#define CONFIG_USB_UHCI_HIGH_BANDWIDTH 
-
-/* *_DEPTH_FIRST puts descriptor in depth-first mode. This has
- * somehow similar effect to FSBR (higher speed), but does not
+ *
+ * bulk_depth/ctrl_depth: 0=off (default), 1:on
+ * Puts descriptors for bulk/control transfers in depth-first mode. 
+ * This has somehow similar effect to FSBR (higher speed), but does not
  * slow PCI down. OTOH USB performace is slightly slower than
  * in FSBR case and single device could hog whole USB, starving
- * other devices.
- */
-#define USE_CTRL_DEPTH_FIRST 0  // 0: Breadth first, 1: Depth first
-#define USE_BULK_DEPTH_FIRST 0  // 0: Breadth first, 1: Depth first
-
-/* Turning off both CONFIG_USB_UHCI_HIGH_BANDWITH and *_DEPTH_FIRST
+ * other devices. Some devices (e.g. STV680-based cameras) NEED this depth 
+ * first search to work properly.
+ *
+ * Turning off both high_bw and bulk_depth/ctrl_depth
  * will lead to <64KB/sec performance over USB for bulk transfers targeting
  * one device's endpoint. You probably do not want to do that.
  */
 
+// Other constants, there's usually no need to change them.
 // stop bandwidth reclamation after (roughly) 50ms
 #define IDLE_TIMEOUT  (HZ/20)
 
@@ -100,6 +99,14 @@
 //                   NO serviceable parts below!
 /*--------------------------------------------------------------------------*/
 
+/* Can be set by module parameters */
+static int high_bw = 1;
+static int ctrl_depth = 0;  /* 0: Breadth first, 1: Depth first */
+static int bulk_depth = 0;  /* 0: Breadth first, 1: Depth first */
+
+// How much URBs with ->next are walked
+#define MAX_NEXT_COUNT 2048
+
 static struct uhci *devs = NULL;
 
 /* used by userspace UHCI data structure dumper */
@@ -155,7 +162,6 @@ static int uhci_urb_enqueue (struct usb_hcd *hcd, struct urb *urb, int mem_flags
 
 	spin_lock_irqsave (&uhci->urb_list_lock, flags);
 
-
 	queued_urb = search_dev_ep (uhci, urb); // returns already queued urb for that pipe
 
 	if (queued_urb) {
@@ -165,7 +171,7 @@ static int uhci_urb_enqueue (struct usb_hcd *hcd, struct urb *urb, int mem_flags
 		    ((type == PIPE_BULK) &&
 		     (!(urb->transfer_flags & USB_QUEUE_BULK) || !(queued_urb->transfer_flags & USB_QUEUE_BULK)))) {
 			spin_unlock_irqrestore (&uhci->urb_list_lock, flags);
-			err("ENXIO (%s)  %08x, flags %x, urb %p, burb %p, propably device driver bug...", 
+			err("ENXIO (%s)  %08x, flags %x, urb %p, burb %p, probably device driver bug...", 
 			    PIPESTRING(type),
 			    urb->pipe,urb->transfer_flags,urb,queued_urb);
 			return -ENXIO;	// urb already queued
@@ -196,7 +202,7 @@ static int uhci_urb_enqueue (struct usb_hcd *hcd, struct urb *urb, int mem_flags
 							       PCI_DMA_TODEVICE);
 
 	// for bulk queuing it is essential that interrupts are disabled until submission
-	// all other type enable interrupts again
+	// all other types enable interrupts again
 	switch (type) {
 	case PIPE_BULK:
 		if (queued_urb) {
@@ -293,7 +299,7 @@ static int uhci_get_frame (struct usb_hcd *hcd)
 /*--------------------------------------------------------------------------*/
 static int hc_reset (struct uhci_hcd *uhci)
 {
-	unsigned int io_addr = (int)uhci->hcd.regs;
+	unsigned long io_addr = (unsigned long)uhci->hcd.regs;
 
 	uhci->apm_state = 0;
 	uhci->running = 0;
@@ -306,7 +312,7 @@ static int hc_reset (struct uhci_hcd *uhci)
 /*--------------------------------------------------------------------------*/
 static int hc_irq_run(struct uhci_hcd *uhci)
 {
-	unsigned int io_addr = (int)uhci->hcd.regs;
+	unsigned long io_addr = (unsigned long)uhci->hcd.regs;
 	/* Turn on all interrupts */
 	outw (USBINTR_TIMEOUT | USBINTR_RESUME | USBINTR_IOC | USBINTR_SP, io_addr + USBINTR);
 
@@ -324,7 +330,7 @@ static int hc_irq_run(struct uhci_hcd *uhci)
 /*--------------------------------------------------------------------------*/
 static int hc_start (struct uhci_hcd *uhci)
 {
-	unsigned int io_addr = (int)uhci->hcd.regs;
+	unsigned long io_addr = (unsigned long)uhci->hcd.regs;
 	int timeout = 10;
 	struct usb_device	*udev;
 	init_dbg("hc_start uhci %p",uhci);
@@ -351,7 +357,6 @@ static int hc_start (struct uhci_hcd *uhci)
 	uhci->hcd.state = USB_STATE_READY;
 	if (!udev) {
 	    uhci->running = 0;
-// FIXME cleanup
 	    return -ENOMEM;
 	}
 
@@ -360,7 +365,6 @@ static int hc_start (struct uhci_hcd *uhci)
 	if (usb_register_root_hub (udev, &uhci->hcd.pdev->dev) != 0) {
 		usb_free_dev (udev); 
 		uhci->running = 0;
-// FIXME cleanup
 		return -ENODEV;
 	}
 	
@@ -374,7 +378,7 @@ static int __devinit uhci_start (struct usb_hcd *hcd)
 {
 	struct uhci_hcd	*uhci = hcd_to_uhci (hcd);
 	int ret;
-	int io_addr=(int)hcd->regs, io_size=0x20; // FIXME
+	unsigned long io_addr=(unsigned long)hcd->regs, io_size=0x20;
 	
 	init_dbg("uhci_start hcd %p uhci %p, pdev %p",hcd,uhci,hcd->pdev);
 	/* disable legacy emulation, Linux takes over... */
@@ -448,7 +452,7 @@ static void uhci_stop (struct usb_hcd *hcd)
 static void uhci_irq (struct usb_hcd *hcd)
 {	
 	struct uhci_hcd	*uhci = hcd_to_uhci (hcd);
-	unsigned int io_addr = (int)hcd->regs;
+	unsigned long io_addr = (unsigned long)hcd->regs;
 	unsigned short status;
 	struct list_head *p, *p2;
 	int restarts, work_done;
@@ -589,6 +593,12 @@ static const struct hc_driver uhci_driver = {
 MODULE_AUTHOR (DRIVER_AUTHOR);
 MODULE_DESCRIPTION (DRIVER_INFO);
 MODULE_LICENSE ("GPL");
+MODULE_PARM (high_bw, "i");
+MODULE_PARM_DESC (high_bw, "high_hw: Enable high bandwidth mode, 1=on (default), 0=off"); 
+MODULE_PARM (bulk_depth, "i");
+MODULE_PARM_DESC (bulk_depth, "bulk_depth: Depth first processing for bulk transfers, 0=off (default), 1=on");
+MODULE_PARM (ctrl_depth, "i");
+MODULE_PARM_DESC (ctrl_depth, "ctrl_depth: Depth first processing for control transfers, 0=off (default), 1=on");
 
 static const struct pci_device_id __devinitdata pci_ids [] = { {
 
@@ -627,6 +637,10 @@ static int __init uhci_hcd_init (void)
 	init_dbg (DRIVER_INFO);
 	init_dbg ("block sizes: hq %d td %d",
 		sizeof (struct qh), sizeof (struct td));
+	info("High bandwidth mode %s.%s%s",
+	     high_bw?"enabled":"disabled",
+	     ctrl_depth?"CTRL depth first enabled":"",
+	     bulk_depth?"BULK depth first enabled":"");
 	return pci_module_init (&uhci_pci_driver);
 }
 

drivers/usb/host/usb-uhci-hub.c

@@ -13,7 +13,7 @@
     HW-initalization based on material of
     Randy Dunlap + Johannes Erdfelt + Gregory P. Smith + Linus Torvalds 
 
-    $Id: usb-uhci-hub.c,v 1.1 2002/05/14 20:36:57 acher Exp $
+    $Id: usb-uhci-hub.c,v 1.2 2002/05/21 21:40:16 acher Exp $
 */
 
 #define CLR_RH_PORTSTAT(x) \
@@ -34,7 +34,7 @@ static int oldval=-1;
 static int uhci_hub_status_data (struct usb_hcd *hcd, char *buf)
 {
 	struct uhci_hcd *uhci = hcd_to_uhci (hcd);
-	unsigned int io_addr = (int)uhci->hcd.regs;
+	unsigned long io_addr = (unsigned long)uhci->hcd.regs;
 	int i, len=0, data = 0, portstate;
 	int changed=0;
 
@@ -99,7 +99,7 @@ static int uhci_hub_control (
 	int status = 0;
 	int stat = 0;
 	int cstatus;
-	unsigned int io_addr = (int)uhci->hcd.regs;
+	unsigned long io_addr = (unsigned long)uhci->hcd.regs;
 	int		ports = uhci->maxports;
 
 	switch (typeReq) {	

drivers/usb/host/usb-uhci-mem.c

@@ -14,7 +14,7 @@
     HW-initalization based on material of
     Randy Dunlap + Johannes Erdfelt + Gregory P. Smith + Linus Torvalds 
 
-    $Id: usb-uhci-mem.c,v 1.1 2002/05/14 20:36:57 acher Exp $
+    $Id: usb-uhci-mem.c,v 1.3 2002/05/25 16:42:41 acher Exp $
  */
 
 /*###########################################################################*/
@@ -387,10 +387,9 @@ static void uhci_switch_timer_int(struct uhci_hcd *uhci)
 	wmb();
 }
 /*-------------------------------------------------------------------*/
-#ifdef CONFIG_USB_UHCI_HIGH_BANDWIDTH
 static void enable_desc_loop(struct uhci_hcd *uhci, struct urb *urb)
 {
-	int flags;
+	unsigned long flags;
 
 	if (urb->transfer_flags & USB_NO_FSBR)
 		return;
@@ -405,7 +404,7 @@ static void enable_desc_loop(struct uhci_hcd *uhci, struct urb *urb)
 /*-------------------------------------------------------------------*/
 static void disable_desc_loop(struct uhci_hcd *uhci, struct urb *urb)
 {
-	int flags;
+	unsigned long flags;
 
 	if (urb->transfer_flags & USB_NO_FSBR)
 		return;
@@ -422,18 +421,16 @@ static void disable_desc_loop(struct uhci_hcd *uhci, struct urb *urb)
 	}
 	spin_unlock_irqrestore (&uhci->qh_lock, flags);
 }
-#endif
 /*-------------------------------------------------------------------*/
 static void queue_urb_unlocked (struct uhci_hcd *uhci, struct urb *urb)
 {
 	urb_priv_t *priv=(urb_priv_t*)urb->hcpriv;
-#ifdef CONFIG_USB_UHCI_HIGH_BANDWIDTH
 	int type;
 	type=usb_pipetype (urb->pipe);
 	
-	if ((type == PIPE_BULK) || (type == PIPE_CONTROL))
+	if (high_bw && ((type == PIPE_BULK) || (type == PIPE_CONTROL)))
 			enable_desc_loop(uhci, urb);
-#endif
+
 	urb->status = -EINPROGRESS;
 	priv->started=jiffies;
 
@@ -455,14 +452,12 @@ static void queue_urb (struct uhci_hcd *uhci, struct urb *urb)
 static void dequeue_urb (struct uhci_hcd *uhci, struct urb *urb)
 {
 	urb_priv_t *priv=(urb_priv_t*)urb->hcpriv;
-#ifdef CONFIG_USB_UHCI_HIGH_BANDWIDTH
 	int type;
 	dbg("dequeue URB %p",urb);
 	type=usb_pipetype (urb->pipe);
 
-	if ((type == PIPE_BULK) || (type == PIPE_CONTROL))
+	if (high_bw && ((type == PIPE_BULK) || (type == PIPE_CONTROL)))
 		disable_desc_loop(uhci, urb);
-#endif
 
 	list_del (&priv->urb_list);
 	if (urb->timeout && uhci->timeout_urbs)
@@ -624,10 +619,10 @@ static int init_skel (struct uhci_hcd *uhci)
 	insert_qh (uhci, uhci->bulk_chain, qh, 0);
 	uhci->control_chain = qh;
 
-#ifdef	CONFIG_USB_UHCI_HIGH_BANDWIDTH
 	// disabled reclamation loop
-	set_qh_head(uhci->chain_end, uhci->control_chain->dma_addr | UHCI_PTR_QH | UHCI_PTR_TERM);
-#endif
+	if (high_bw)
+		set_qh_head(uhci->chain_end, uhci->control_chain->dma_addr | UHCI_PTR_QH | UHCI_PTR_TERM);
+
 
 	init_dbg("allocating qh: ls_control_chain");
 	if (alloc_qh (uhci, &qh))

drivers/usb/host/usb-uhci-q.c

@@ -13,7 +13,7 @@
     HW-initalization based on material of
     Randy Dunlap + Johannes Erdfelt + Gregory P. Smith + Linus Torvalds 
 
-    $Id: usb-uhci-q.c,v 1.1 2002/05/14 20:36:57 acher Exp $
+    $Id: usb-uhci-q.c,v 1.3 2002/05/25 16:42:41 acher Exp $
 */
 
 /*-------------------------------------------------------------------*/
@@ -59,7 +59,7 @@ static int uhci_submit_control_urb (struct uhci_hcd *uhci, struct urb *urb)
 	urb_priv_t *urb_priv = urb->hcpriv;
 	unsigned long destination, status;
 	int maxsze = usb_maxpacket (urb->dev, urb->pipe, usb_pipeout (urb->pipe));
-	int depth_first=USE_CTRL_DEPTH_FIRST;  // UHCI descriptor chasing method
+	int depth_first = ctrl_depth;  // UHCI descriptor chasing method
 	unsigned long len;
 	char *data;
 
@@ -175,7 +175,7 @@ static int uhci_submit_bulk_urb (struct uhci_hcd *uhci, struct urb *urb, struct
 	unsigned int pipe = urb->pipe;
 	int maxsze = usb_maxpacket (urb->dev, pipe, usb_pipeout (pipe));
 	int info, len, last;
-	int depth_first=USE_BULK_DEPTH_FIRST;  // UHCI descriptor chasing method
+	int depth_first = bulk_depth;  // UHCI descriptor chasing method
 
 	if (usb_endpoint_halted (urb->dev, usb_pipeendpoint (pipe), usb_pipeout (pipe)))
 		return -EPIPE;
@@ -305,7 +305,7 @@ static int uhci_submit_bulk_urb (struct uhci_hcd *uhci, struct urb *urb, struct
 static int uhci_submit_int_urb (struct uhci_hcd *uhci, struct urb *urb)
 {
 	urb_priv_t *urb_priv = urb->hcpriv;
-	int nint, n;
+	int nint;
 	uhci_desc_t *td;
 	int status, destination;
 	int info;
@@ -313,16 +313,14 @@ static int uhci_submit_int_urb (struct uhci_hcd *uhci, struct urb *urb)
 	
 	if (urb->interval == 0)
 		nint = 0;
-	else {  // round interval down to 2^n
-		for (nint = 0, n = 1; nint <= 8; nint++, n += n)	
-			if (urb->interval < n) {
-				urb->interval = n / 2;
-				break;
-			}
-		nint--;
+	else {  
+		// log2-function (urb->interval already 2^n)
+		nint = ffs(urb->interval);
+		if (nint>7)
+			nint=7;
 	}
 
-	dbg("Rounded interval to %i, chain  %i", urb->interval, nint);
+	dbg("INT-interval %i, chain  %i", urb->interval, nint);
 	// remember start frame, just in case...
 	urb->start_frame = UHCI_GET_CURRENT_FRAME (uhci) & 1023;	
 
@@ -381,7 +379,7 @@ static int find_iso_limits (struct uhci_hcd *uhci, struct urb *urb, unsigned int
 	}
 	
 	if (last_urb) {
-		*end = (last_urb->start_frame + last_urb->number_of_packets) & 1023;
+		*end = (last_urb->start_frame + last_urb->number_of_packets*last_urb->interval) & 1023;
 		ret=0;
 	}
 	
@@ -395,12 +393,14 @@ static int find_iso_limits (struct uhci_hcd *uhci, struct urb *urb, unsigned int
 static int iso_find_start (struct uhci_hcd *uhci, struct urb *urb)
 {
 	unsigned int now;
-	unsigned int start_limit = 0, stop_limit = 0, queued_size;
+	unsigned int start_limit = 0, stop_limit = 0, queued_size, number_of_frames;
 	int limits;
 
 	now = UHCI_GET_CURRENT_FRAME (uhci) & 1023;
 
-	if ((unsigned) urb->number_of_packets > 900)
+	number_of_frames = (unsigned) (urb->number_of_packets*urb->interval);
+
+	if ( number_of_frames > 900)
 		return -EFBIG;
 	
 	limits = find_iso_limits (uhci, urb, &start_limit, &stop_limit);
@@ -415,17 +415,17 @@ static int iso_find_start (struct uhci_hcd *uhci, struct urb *urb)
 		else {
 			urb->start_frame = stop_limit;		// seamless linkage
 
-			if (((now - urb->start_frame) & 1023) <= (unsigned) urb->number_of_packets) {
+			if (((now - urb->start_frame) & 1023) <= (unsigned) number_of_frames) {
 				info("iso_find_start: gap in seamless isochronous scheduling");
-				dbg("iso_find_start: now %u start_frame %u number_of_packets %u pipe 0x%08x",
-					now, urb->start_frame, urb->number_of_packets, urb->pipe);
+				dbg("iso_find_start: now %u start_frame %u number_of_packets %u interval %u pipe 0x%08x",
+					now, urb->start_frame, urb->number_of_packets, urb->interval, urb->pipe);
 				urb->start_frame = (now + 5) & 1023;	// 5ms setup should be enough
 			}
 		}
 	}
 	else {
 		urb->start_frame &= 1023;
-		if (((now - urb->start_frame) & 1023) < (unsigned) urb->number_of_packets) {
+		if (((now - urb->start_frame) & 1023) < number_of_frames) {
 			dbg("iso_find_start: now between start_frame and end");
 			return -EAGAIN;
 		}
@@ -436,7 +436,7 @@ static int iso_find_start (struct uhci_hcd *uhci, struct urb *urb)
 		return 0;
 
 	if (((urb->start_frame - start_limit) & 1023) < queued_size ||
-	    ((urb->start_frame + urb->number_of_packets - 1 - start_limit) & 1023) < queued_size) {
+	    ((urb->start_frame + number_of_frames - 1 - start_limit) & 1023) < queued_size) {
 		dbg("iso_find_start: start_frame %u number_of_packets %u start_limit %u stop_limit %u",
 			urb->start_frame, urb->number_of_packets, start_limit, stop_limit);
 		return -EAGAIN;
@@ -507,7 +507,7 @@ static int uhci_submit_iso_urb (struct uhci_hcd *uhci, struct urb *urb, int mem_
 			 urb_priv->transfer_buffer_dma + urb->iso_frame_desc[n].offset);
 		list_add_tail (&td->desc_list, &urb_priv->desc_list);
 	
-		insert_td_horizontal (uhci, uhci->iso_td[(urb->start_frame + n) & 1023], td);	// store in iso-tds
+		insert_td_horizontal (uhci, uhci->iso_td[(urb->start_frame + n*urb->interval) & 1023], td);	// store in iso-tds
 	}
 
 	kfree (tdm);
@@ -742,6 +742,7 @@ static int uhci_unlink_urb_async (struct uhci_hcd *uhci, struct urb *urb, int mo
 
 	switch (usb_pipetype (urb->pipe)) {
 	case PIPE_INTERRUPT:
+		urb_priv->flags = 0; // mark as deleted (if called from completion)
 		uhci_do_toggle (urb);
 
 	case PIPE_ISOCHRONOUS:
@@ -853,12 +854,9 @@ static void uhci_check_timeouts(struct uhci_hcd *uhci)
 			async_dbg("uhci_check_timeout: timeout for %p",urb);
 			uhci_unlink_urb_async(uhci, urb, UNLINK_ASYNC_STORE_URB);
 		}
-#ifdef CONFIG_USB_UHCI_HIGH_BANDWIDTH
-		else if (((type == PIPE_BULK) || (type == PIPE_CONTROL)) &&  
+		else if (high_bw && ((type == PIPE_BULK) || (type == PIPE_CONTROL)) &&  
 			 (hcpriv->use_loop) && time_after(jiffies, hcpriv->started + IDLE_TIMEOUT))
 			disable_desc_loop(uhci, urb);
-#endif
-
 	}
 	uhci->timeout_check=jiffies;
 }
@@ -1040,9 +1038,8 @@ static int process_transfer (struct uhci_hcd *uhci, struct urb *urb, int mode)
 	uhci_clean_transfer(uhci, urb, qh, mode);
 	urb->status = status;
 
-#ifdef CONFIG_USB_UHCI_HIGH_BANDWIDTH	
-	disable_desc_loop(uhci,urb);
-#endif	
+	if (high_bw)
+		disable_desc_loop(uhci,urb);
 
 	dbg("process_transfer: (end) urb %p, wanted len %d, len %d status %x err %d",
 		urb,urb->transfer_buffer_length,urb->actual_length, urb->status, urb->error_count);
@@ -1088,21 +1085,19 @@ static int process_interrupt (struct uhci_hcd *uhci, struct urb *urb, int mode)
 			urb->actual_length = actual_length;
 
 	recycle:
+		((urb_priv_t*)urb->hcpriv)->flags=1; // set to detect unlink during completion
+
 		uhci_urb_dma_sync(uhci, urb, urb->hcpriv);
 		if (urb->complete) {
 			//dbg("process_interrupt: calling completion, status %i",status);
 			urb->status = status;
-			((urb_priv_t*)urb->hcpriv)->flags=1; // if unlink_urb is called during completion
-
 			spin_unlock(&uhci->urb_list_lock);
 			urb->complete ((struct urb *) urb);
 			spin_lock(&uhci->urb_list_lock);
-
-			((urb_priv_t*)urb->hcpriv)->flags=0; // FIXME: unlink in completion not handled...
 		}
 		
 		if ((urb->status != -ECONNABORTED) && (urb->status != ECONNRESET) &&
-			    (urb->status != -ENOENT)) {
+			    (urb->status != -ENOENT) && ((urb_priv_t*)urb->hcpriv)->flags) {
 
 			urb->status = -EINPROGRESS;
 
@@ -1125,7 +1120,7 @@ static int process_interrupt (struct uhci_hcd *uhci, struct urb *urb, int mode)
 				mb();
 			}
 			else {
-				uhci_unlink_urb_async(uhci, urb, UNLINK_ASYNC_STORE_URB);				
+				uhci_unlink_urb_async(uhci, urb, UNLINK_ASYNC_STORE_URB);
 				uhci_do_toggle (urb); // correct toggle after unlink
 				clr_td_ioc(desc);    // inactivate TD
 			}
@@ -1199,7 +1194,6 @@ static int process_iso (struct uhci_hcd *uhci, struct urb *urb, int mode)
 		p_tmp = p;
 		p = p->next;
 		list_del (p_tmp);
-//		delete_desc (uhci, desc);
 
 		// add to cool down pool
 		INIT_LIST_HEAD(&desc->horizontal);
@@ -1243,19 +1237,18 @@ static int process_urb (struct uhci_hcd *uhci, struct list_head *p)
 		break;
 	}
 
-	if (urb->status != -EINPROGRESS) {
+	if (urb->status != -EINPROGRESS && type != PIPE_INTERRUPT) {
 
 		dequeue_urb (uhci, urb);
+ 		uhci_free_priv(uhci, urb, urb->hcpriv);
 
-		uhci_free_priv(uhci, urb, urb->hcpriv);
+		spin_unlock(&uhci->urb_list_lock);
+		dbg("giveback urb %p, status %i, length %i\n", 
+		    urb, urb->status, urb->transfer_buffer_length);
+		
+		usb_hcd_giveback_urb(&uhci->hcd, urb);
+		spin_lock(&uhci->urb_list_lock);
 
-		if (type != PIPE_INTERRUPT) {  // process_interrupt does completion on its own		
-			spin_unlock(&uhci->urb_list_lock);
-			dbg("giveback urb %p, status %i, length %i\n", 
-			    urb, urb->status, urb->transfer_buffer_length);
-			usb_hcd_giveback_urb(&uhci->hcd, urb);
-			spin_lock(&uhci->urb_list_lock);
-		}
 	}
 	return ret;
 }