usb/isp1760: Replace period calculation for INT packets with something readable

Replace the period calculation for INT packets with something readable. Seems
to fix a rare bug with quickly repeated insertion/removal of several USB
devices simultaneously (hub control INT packets).

Signed-off-by: Arvid Brodin <arvid.brodin@enea.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c
index 0f5b489..fd718ff 100644 (file)
--- a/drivers/usb/host/isp1760-hcd.c
+++ b/drivers/usb/host/isp1760-hcd.c
@@ -97,9 +97,6 @@ struct isp1760_qh {
        /* first part defined by EHCI spec */
        struct list_head qtd_list;
 
-       /* periodic schedule info */
-       unsigned short period;          /* polling interval */
-
        u32 toggle;
        u32 ping;
 };
@@ -673,60 +670,51 @@ static void transform_into_atl(struct isp1760_qh *qh,
 static void transform_add_int(struct isp1760_qh *qh,
                        struct isp1760_qtd *qtd, struct ptd *ptd)
 {
-       u32 maxpacket;
-       u32 multi;
-       u32 numberofusofs;
-       u32 i;
-       u32 usofmask, usof;
+       u32 usof;
        u32 period;
 
-       maxpacket = usb_maxpacket(qtd->urb->dev, qtd->urb->pipe,
-                                               usb_pipeout(qtd->urb->pipe));
-       multi =  1 + ((maxpacket >> 11) & 0x3);
-       maxpacket &= 0x7ff;
-       /* length of the data per uframe */
-       maxpacket = multi * maxpacket;
-
-       numberofusofs = qtd->urb->transfer_buffer_length / maxpacket;
-       if (qtd->urb->transfer_buffer_length % maxpacket)
-               numberofusofs += 1;
-
-       usofmask = 1;
-       usof = 0;
-       for (i = 0; i < numberofusofs; i++) {
-               usof |= usofmask;
-               usofmask <<= 1;
-       }
-
-       if (qtd->urb->dev->speed != USB_SPEED_HIGH) {
-               /* split */
-               ptd->dw5 = 0x1c;
+       /*
+        * Most of this is guessing. ISP1761 datasheet is quite unclear, and
+        * the algorithm from the original Philips driver code, which was
+        * pretty much used in this driver before as well, is quite horrendous
+        * and, i believe, incorrect. The code below follows the datasheet and
+        * USB2.0 spec as far as I can tell, and plug/unplug seems to be much
+        * more reliable this way (fingers crossed...).
+        */
 
-               if (qh->period >= 32)
-                       period = qh->period / 2;
+       if (qtd->urb->dev->speed == USB_SPEED_HIGH) {
+               /* urb->interval is in units of microframes (1/8 ms) */
+               period = qtd->urb->interval >> 3;
+
+               if (qtd->urb->interval > 4)
+                       usof = 0x01; /* One bit set =>
+                                               interval 1 ms * uFrame-match */
+               else if (qtd->urb->interval > 2)
+                       usof = 0x22; /* Two bits set => interval 1/2 ms */
+               else if (qtd->urb->interval > 1)
+                       usof = 0x55; /* Four bits set => interval 1/4 ms */
                else
-                       period = qh->period;
-
+                       usof = 0xff; /* All bits set => interval 1/8 ms */
        } else {
+               /* urb->interval is in units of frames (1 ms) */
+               period = qtd->urb->interval;
+               usof = 0x0f;            /* Execute Start Split on any of the
+                                          four first uFrames */
 
-               if (qh->period >= 8)
-                       period = qh->period/8;
-               else
-                       period = qh->period;
-
-               if (period >= 32)
-                       period  = 16;
-
-               if (qh->period >= 8) {
-                       /* millisecond period */
-                       period = (period << 3);
-               } else {
-                       /* usof based tranmsfers */
-                       /* minimum 4 usofs */
-                       usof = 0x11;
-               }
+               /*
+                * First 8 bits in dw5 is uSCS and "specifies which uSOF the
+                * complete split needs to be sent. Valid only for IN." Also,
+                * "All bits can be set to one for every transfer." (p 82,
+                * ISP1761 data sheet.) 0x1c is from Philips driver. Where did
+                * that number come from? 0xff seems to work fine...
+                */
+               /* ptd->dw5 = 0x1c; */
+               ptd->dw5 = 0xff; /* Execute Complete Split on any uFrame */
        }
 
+       period = period >> 1;/* Ensure equal or shorter period than requested */
+       period &= 0xf8; /* Mask off too large values and lowest unused 3 bits */
+
        ptd->dw2 |= period;
        ptd->dw4 = usof;
 }
@@ -1328,26 +1316,6 @@ static struct isp1760_qh *qh_make(struct usb_hcd *hcd, struct urb *urb,
        is_input = usb_pipein(urb->pipe);
        type = usb_pipetype(urb->pipe);
 
-       if (type == PIPE_INTERRUPT) {
-
-               if (urb->dev->speed == USB_SPEED_HIGH) {
-
-                       qh->period = urb->interval >> 3;
-                       if (qh->period == 0 && urb->interval != 1) {
-                               /* NOTE interval 2 or 4 uframes could work.
-                                * But interval 1 scheduling is simpler, and
-                                * includes high bandwidth.
-                                */
-                               dev_err(hcd->self.controller, "intr period %d uframes, NYET!",
-                                               urb->interval);
-                               qh_destroy(qh);
-                               return NULL;
-                       }
-               } else {
-                       qh->period = urb->interval;
-               }
-       }
-
        if (!usb_pipecontrol(urb->pipe))
                usb_settoggle(urb->dev, usb_pipeendpoint(urb->pipe), !is_input,
                                1);