USB: EHCI: add separate IAA watchdog timer

This patch (as1028) was mostly written by David Brownell; I made only
a few changes (extra log info and a small bug fix -- which might
account for why David's version had to be reverted).  It adds a new
watchdog timer to the ehci-hcd driver to be used exclusively for
detecting lost or missing IAA notifications.

Previously a shared timer had been used, which may have led to some
problems as reported by Christian Hoffmann.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Cc: David Brownell <david-b@pacbell.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

6 files changed, 75 insertions, 38 deletions

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 39673f8194d9..64ebfc5548a3 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -786,9 +786,7 @@ static ssize_t fill_registers_buffer(struct debug_buffer *buf)
         }
 
         if (ehci->reclaim) {
-                temp = scnprintf (next, size, "reclaim qh %p%s\n",
-                                ehci->reclaim,
-                                ehci->reclaim_ready ? " ready" : "");
+                temp = scnprintf(next, size, "reclaim qh %p\n", ehci->reclaim);
                 size -= temp;
                 next += temp;
         }
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 264182654f4c..5917c6b7ce33 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -110,7 +110,7 @@ static const char	hcd_name [] = "ehci_hcd";
 #define EHCI_TUNE_MULT_TT       1
 #define EHCI_TUNE_FLS           2       /* (small) 256 frame schedule */
 
-#define EHCI_IAA_JIFFIES        (HZ/100)        /* arbitrary; ~10 msec */
+#define EHCI_IAA_MSECS          10              /* arbitrary */
 #define EHCI_IO_JIFFIES         (HZ/10)         /* io watchdog > irq_thresh */
 #define EHCI_ASYNC_JIFFIES      (HZ/20)         /* async idle timeout */
 #define EHCI_SHRINK_JIFFIES     (HZ/200)        /* async qh unlink delay */
@@ -267,6 +267,7 @@ static void ehci_quiesce (struct ehci_hcd *ehci)
 
 /*-------------------------------------------------------------------------*/
 
+static void end_unlink_async(struct ehci_hcd *ehci);
 static void ehci_work(struct ehci_hcd *ehci);
 
 #include "ehci-hub.c"
@@ -276,25 +277,41 @@ static void ehci_work(struct ehci_hcd *ehci);
 
 /*-------------------------------------------------------------------------*/
 
-static void ehci_watchdog (unsigned long param)
+static void ehci_iaa_watchdog(unsigned long param)
 {
         struct ehci_hcd         *ehci = (struct ehci_hcd *) param;
         unsigned long           flags;
+        u32                     status, cmd;
 
         spin_lock_irqsave (&ehci->lock, flags);
+        WARN_ON(!ehci->reclaim);
 
-        /* lost IAA irqs wedge things badly; seen with a vt8235 */
+        status = ehci_readl(ehci, &ehci->regs->status);
+        cmd = ehci_readl(ehci, &ehci->regs->command);
+        ehci_dbg(ehci, "IAA watchdog: status %x cmd %x\n", status, cmd);
+
+        /* lost IAA irqs wedge things badly; seen first with a vt8235 */
         if (ehci->reclaim) {
-                u32             status = ehci_readl(ehci, &ehci->regs->status);
                 if (status & STS_IAA) {
                         ehci_vdbg (ehci, "lost IAA\n");
                         COUNT (ehci->stats.lost_iaa);
                         ehci_writel(ehci, STS_IAA, &ehci->regs->status);
-                        ehci->reclaim_ready = 1;
                 }
+                ehci_writel(ehci, cmd & ~CMD_IAAD, &ehci->regs->command);
+                end_unlink_async(ehci);
         }
 
-        /* stop async processing after it's idled a bit */
+        spin_unlock_irqrestore(&ehci->lock, flags);
+}
+
+static void ehci_watchdog(unsigned long param)
+{
+        struct ehci_hcd         *ehci = (struct ehci_hcd *) param;
+        unsigned long           flags;
+
+        spin_lock_irqsave(&ehci->lock, flags);
+
+        /* stop async processing after it's idled a bit */
         if (test_bit (TIMER_ASYNC_OFF, &ehci->actions))
                 start_unlink_async (ehci, ehci->async);
 
@@ -364,8 +381,6 @@ static void ehci_port_power (struct ehci_hcd *ehci, int is_on)
 static void ehci_work (struct ehci_hcd *ehci)
 {
         timer_action_done (ehci, TIMER_IO_WATCHDOG);
-        if (ehci->reclaim_ready)
-                end_unlink_async (ehci);
 
         /* another CPU may drop ehci->lock during a schedule scan while
          * it reports urb completions.  this flag guards against bogus
@@ -400,6 +415,7 @@ static void ehci_stop (struct usb_hcd *hcd)
 
         /* no more interrupts ... */
         del_timer_sync (&ehci->watchdog);
+        del_timer_sync(&ehci->iaa_watchdog);
 
         spin_lock_irq(&ehci->lock);
         if (HC_IS_RUNNING (hcd->state))
@@ -448,6 +464,10 @@ static int ehci_init(struct usb_hcd *hcd)
         ehci->watchdog.function = ehci_watchdog;
         ehci->watchdog.data = (unsigned long) ehci;
 
+        init_timer(&ehci->iaa_watchdog);
+        ehci->iaa_watchdog.function = ehci_iaa_watchdog;
+        ehci->iaa_watchdog.data = (unsigned long) ehci;
+
         /*
          * hw default: 1K periodic list heads, one per frame.
          * periodic_size can shrink by USBCMD update if hcc_params allows.
@@ -464,7 +484,6 @@ static int ehci_init(struct usb_hcd *hcd)
                 ehci->i_thresh = 2 + HCC_ISOC_THRES(hcc_params);
 
         ehci->reclaim = NULL;
-        ehci->reclaim_ready = 0;
         ehci->next_uframe = -1;
 
         /*
@@ -655,8 +674,7 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd)
         /* complete the unlinking of some qh [4.15.2.3] */
         if (status & STS_IAA) {
                 COUNT (ehci->stats.reclaim);
-                ehci->reclaim_ready = 1;
-                bh = 1;
+                end_unlink_async(ehci);
         }
 
         /* remote wakeup [4.3.1] */
@@ -762,10 +780,16 @@ static int ehci_urb_enqueue (
 
 static void unlink_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
 {
-        /* if we need to use IAA and it's busy, defer */
-        if (qh->qh_state == QH_STATE_LINKED
-                        && ehci->reclaim
-                        && HC_IS_RUNNING (ehci_to_hcd(ehci)->state)) {
+        /* failfast */
+        if (!HC_IS_RUNNING(ehci_to_hcd(ehci)->state))
+                end_unlink_async(ehci);
+
+        /* if it's not linked then there's nothing to do */
+        if (qh->qh_state != QH_STATE_LINKED)
+                ;
+
+        /* defer till later if busy */
+        else if (ehci->reclaim) {
                 struct ehci_qh          *last;
 
                 for (last = ehci->reclaim;
@@ -775,12 +799,8 @@ static void unlink_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
                 qh->qh_state = QH_STATE_UNLINK_WAIT;
                 last->reclaim = qh;
 
-        /* bypass IAA if the hc can't care */
-        } else if (!HC_IS_RUNNING (ehci_to_hcd(ehci)->state) && ehci->reclaim)
-                end_unlink_async (ehci);
-
-        /* something else might have unlinked the qh by now */
-        if (qh->qh_state == QH_STATE_LINKED)
+        /* start IAA cycle */
+        } else
                 start_unlink_async (ehci, qh);
 }
 
@@ -807,7 +827,19 @@ static int ehci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
                 qh = (struct ehci_qh *) urb->hcpriv;
                 if (!qh)
                         break;
-                unlink_async (ehci, qh);
+                switch (qh->qh_state) {
+                case QH_STATE_LINKED:
+                case QH_STATE_COMPLETING:
+                        unlink_async(ehci, qh);
+                        break;
+                case QH_STATE_UNLINK:
+                case QH_STATE_UNLINK_WAIT:
+                        /* already started */
+                        break;
+                case QH_STATE_IDLE:
+                        WARN_ON(1);
+                        break;
+                }
                 break;
 
         case PIPE_INTERRUPT:
@@ -899,6 +931,7 @@ rescan:
                 unlink_async (ehci, qh);
                 /* FALL THROUGH */
         case QH_STATE_UNLINK:           /* wait for hw to finish? */
+        case QH_STATE_UNLINK_WAIT:
 idle_timeout:
                 spin_unlock_irqrestore (&ehci->lock, flags);
                 schedule_timeout_uninterruptible(1);
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index a165e0a0961c..a249d03a5024 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -134,7 +134,7 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
         }
         ehci->command = ehci_readl(ehci, &ehci->regs->command);
         if (ehci->reclaim)
-                ehci->reclaim_ready = 1;
+                end_unlink_async(ehci);
         ehci_work(ehci);
 
         /* Unlike other USB host controller types, EHCI doesn't have
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 45e040000280..3ba01664f821 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -305,7 +305,7 @@ static int ehci_pci_resume(struct usb_hcd *hcd)
         /* emptying the schedule aborts any urbs */
         spin_lock_irq(&ehci->lock);
         if (ehci->reclaim)
-                ehci->reclaim_ready = 1;
+                end_unlink_async(ehci);
         ehci_work(ehci);
         spin_unlock_irq(&ehci->lock);
 
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index b10f39c047e9..853e5e6396a5 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -973,7 +973,7 @@ static void end_unlink_async (struct ehci_hcd *ehci)
         struct ehci_qh          *qh = ehci->reclaim;
         struct ehci_qh          *next;
 
-        timer_action_done (ehci, TIMER_IAA_WATCHDOG);
+        iaa_watchdog_done(ehci);
 
         // qh->hw_next = cpu_to_hc32(qh->qh_dma);
         qh->qh_state = QH_STATE_IDLE;
@@ -983,7 +983,6 @@ static void end_unlink_async (struct ehci_hcd *ehci)
         /* other unlink(s) may be pending (in QH_STATE_UNLINK_WAIT) */
         next = qh->reclaim;
         ehci->reclaim = next;
-        ehci->reclaim_ready = 0;
         qh->reclaim = NULL;
 
         qh_completions (ehci, qh);
@@ -1059,11 +1058,10 @@ static void start_unlink_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
                 return;
         }
 
-        ehci->reclaim_ready = 0;
         cmd |= CMD_IAAD;
         ehci_writel(ehci, cmd, &ehci->regs->command);
         (void)ehci_readl(ehci, &ehci->regs->command);
-        timer_action (ehci, TIMER_IAA_WATCHDOG);
+        iaa_watchdog_start(ehci);
 }
 
 /*-------------------------------------------------------------------------*/
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 10e71417c352..eeda4c88ebae 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -74,7 +74,6 @@ struct ehci_hcd {			/* one per controller */
         /* async schedule support */
         struct ehci_qh          *async;
         struct ehci_qh          *reclaim;
-        unsigned                reclaim_ready : 1;
         unsigned                scanning : 1;
 
         /* periodic schedule support */
@@ -105,6 +104,7 @@ struct ehci_hcd {			/* one per controller */
         struct dma_pool         *itd_pool;      /* itd per iso urb */
         struct dma_pool         *sitd_pool;     /* sitd per split iso urb */
 
+        struct timer_list       iaa_watchdog;
         struct timer_list       watchdog;
         unsigned long           actions;
         unsigned                stamp;
@@ -148,9 +148,21 @@ static inline struct usb_hcd *ehci_to_hcd (struct ehci_hcd *ehci)
 }
 
 
+static inline void
+iaa_watchdog_start(struct ehci_hcd *ehci)
+{
+        WARN_ON(timer_pending(&ehci->iaa_watchdog));
+        mod_timer(&ehci->iaa_watchdog,
+                        jiffies + msecs_to_jiffies(EHCI_IAA_MSECS));
+}
+
+static inline void iaa_watchdog_done(struct ehci_hcd *ehci)
+{
+        del_timer(&ehci->iaa_watchdog);
+}
+
 enum ehci_timer_action {
         TIMER_IO_WATCHDOG,
-        TIMER_IAA_WATCHDOG,
         TIMER_ASYNC_SHRINK,
         TIMER_ASYNC_OFF,
 };
@@ -168,9 +180,6 @@ timer_action (struct ehci_hcd *ehci, enum ehci_timer_action action)
                 unsigned long t;
 
                 switch (action) {
-                case TIMER_IAA_WATCHDOG:
-                        t = EHCI_IAA_JIFFIES;
-                        break;
                 case TIMER_IO_WATCHDOG:
                         t = EHCI_IO_JIFFIES;
                         break;
@@ -187,8 +196,7 @@ timer_action (struct ehci_hcd *ehci, enum ehci_timer_action action)
                 // async queue SHRINK often precedes IAA.  while it's ready
                 // to go OFF neither can matter, and afterwards the IO
                 // watchdog stops unless there's still periodic traffic.
-                if (action != TIMER_IAA_WATCHDOG
-                                && t > ehci->watchdog.expires
+                if (time_before_eq(t, ehci->watchdog.expires)
                                 && timer_pending (&ehci->watchdog))
                         return;
                 mod_timer (&ehci->watchdog, t);