Re: PATCH: usb-uhci: interrupt out with urb->interval 0 [linux-usb-devel]

[Pete Zaitcev <zaitcev@redhat.com>]

> From: Frode Isaksen <fisaksen@bewan.com>
> Date: Wed, 16 Apr 2003 17:44:55 +0200

> A recent change (2.4.21) in the usb-uhci driver calls 
> "uhci_clean_iso_step2" after the completion of one-shot (urb->interval 
> 0) interrupt out transfers. This call clears the list of descriptors. 
> However, it crashes when trying to get the next desciptor in the "for" 
> loop in the "process_interrupt" function,

> --- drivers/usb/usb-uhci.c.orig	2003-04-16 15:39:04.000000000 +0200
> @@ -2628,6 +2628,7 @@
>  				// correct toggle after unlink
>  				usb_dotoggle (urb->dev, usb_pipeendpoint 
>  				(urb->pipe), usb_pipeout (urb->pipe));
>  				clr_td_ioc(desc); // inactivate TD
> +				break;
>  			}

Pretty obvious, but I'd rather see something like the attached
patch instead. The original code was obviously a case of a rash
copy-paste, complete with an unused "int i".

Is there a more interesting/popular/widespread device than
Lego Tower which uses these oneshots?

-- Pete

diff -urN -X dontdiff linux-2.4.21-rc1/drivers/usb/host/usb-uhci.c linux-2.4.21-rc1-nip/drivers/usb/host/usb-uhci.c
--- linux-2.4.21-rc1/drivers/usb/host/usb-uhci.c	2003-04-24 10:52:56.000000000 -0700
+++ linux-2.4.21-rc1-nip/drivers/usb/host/usb-uhci.c	2003-04-29 12:43:28.000000000 -0700
@@ -2430,9 +2430,9 @@
 
 _static int process_interrupt (uhci_t *s, struct urb *urb)
 {
-	int i, ret = -EINPROGRESS;
+	int ret = -EINPROGRESS;
 	urb_priv_t *urb_priv = urb->hcpriv;
-	struct list_head *p = urb_priv->desc_list.next;
+	struct list_head *p;
 	uhci_desc_t *desc = list_entry (urb_priv->desc_list.prev, uhci_desc_t, desc_list);
 
 	int actual_length;
@@ -2440,8 +2440,8 @@
 
 	//dbg("urb contains interrupt request");
 
-	for (i = 0; p != &urb_priv->desc_list; p = p->next, i++)	// Maybe we allow more than one TD later ;-)
-	{
+	// Maybe we allow more than one TD later ;-)
+	while ((p = urb_priv->desc_list.next) != &urb_priv->desc_list) {
 		desc = list_entry (p, uhci_desc_t, desc_list);
 
 		if (is_td_active(desc)) {