[PATCH] USB UHCI: Fix up loose ends

This patch tidies up a few loose ends left by the preceding patches.
It indicates the controller supports remote wakeup whenever the PM
capability is present -- which shouldn't cause any harm if the
assumption turns out to be wrong.  It refuses to suspend the
controller if the root hub is still active, and it refuses to resume
the root hub if the controller is suspended.  It adds checks for a
dead controller in several spots, and it adds memory barriers as
needed to insure that I/O operations are completed before moving on.

Actually I'm not certain the last part is being done correctly.  With
code like this:

        outw(..., ...);
        mb();
        udelay(5);

do we know for certain that the outw() will complete _before_ the
delay begins?  If not, how should this be written?

Signed-off-by: Alan Stern <[EMAIL PROTECTED]>
Signed-off-by: Greg Kroah-Hartman <[EMAIL PROTECTED]>

---
commit 4daaa87c8f19c5f1978470e9e91b74d9e0fb0f8e
tree ee4ea0e8f4d9912c246916f08f2b50fbc5b42a6a
parent a8bed8b6be75bc5a46aa599ab360d5f1db291c8f
author Alan Stern <[EMAIL PROTECTED]> Sat, 09 Apr 2005 17:30:08 -0400
committer Greg Kroah-Hartman <[EMAIL PROTECTED]> Mon, 27 Jun 2005 14:43:44 -0700

 drivers/usb/host/uhci-hcd.c |   86 +++++++++++++++++++++++++++++++++----------
 1 files changed, 65 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
--- a/drivers/usb/host/uhci-hcd.c
+++ b/drivers/usb/host/uhci-hcd.c
@@ -13,18 +13,13 @@
  * (C) Copyright 2000 Yggdrasil Computing, Inc. (port of new PCI interface
  *               support from usb-ohci.c by Adam Richter, [EMAIL PROTECTED]).
  * (C) Copyright 1999 Gregory P. Smith (from usb-ohci.c)
- * (C) Copyright 2004 Alan Stern, [EMAIL PROTECTED]
+ * (C) Copyright 2004-2005 Alan Stern, [EMAIL PROTECTED]
  *
  * Intel documents this fairly well, and as far as I know there
  * are no royalties or anything like that, but even so there are
  * people who decided that they want to do the same thing in a
  * completely different way.
  *
- * WARNING! The USB documentation is downright evil. Most of it
- * is just crap, written by a committee. You're better off ignoring
- * most of it, the important stuff is:
- *  - the low-level protocol (fairly simple but lots of small details)
- *  - working around the horridness of the rest
  */
 
 #include <linux/config.h>
@@ -147,6 +142,15 @@ static void reset_hc(struct uhci_hcd *uh
 }
 
 /*
+ * Last rites for a defunct/nonfunctional controller
+ */
+static void hc_died(struct uhci_hcd *uhci)
+{
+       reset_hc(uhci);
+       uhci->hc_inaccessible = 1;
+}
+
+/*
  * Initialize a controller that was newly discovered or has just been
  * resumed.  In either case we can't be sure of its previous state.
  */
@@ -287,6 +291,8 @@ __acquires(uhci->lock)
                spin_unlock_irq(&uhci->lock);
                msleep(1);
                spin_lock_irq(&uhci->lock);
+               if (uhci->hc_inaccessible)      /* Died */
+                       return;
        }
        if (!(inw(uhci->io_addr + USBSTS) & USBSTS_HCH))
                dev_warn(uhci_dev(uhci), "Controller not stopped yet!\n");
@@ -335,6 +341,8 @@ __acquires(uhci->lock)
                spin_unlock_irq(&uhci->lock);
                msleep(20);
                spin_lock_irq(&uhci->lock);
+               if (uhci->hc_inaccessible)      /* Died */
+                       return;
 
                /* End Global Resume and wait for EOP to be sent */
                outw(USBCMD_CF, uhci->io_addr + USBCMD);
@@ -387,9 +395,11 @@ static void stall_callback(unsigned long
        check_fsbr(uhci);
 
        /* Poll for and perform state transitions */
-       rh_state_transitions(uhci);
-       if (uhci->suspended_ports && !uhci->hc_inaccessible)
-               uhci_check_ports(uhci);
+       if (!uhci->hc_inaccessible) {
+               rh_state_transitions(uhci);
+               if (uhci->suspended_ports)
+                       uhci_check_ports(uhci);
+       }
 
        restart_timer(uhci);
        spin_unlock_irqrestore(&uhci->lock, flags);
@@ -399,6 +409,7 @@ static irqreturn_t uhci_irq(struct usb_h
 {
        struct uhci_hcd *uhci = hcd_to_uhci(hcd);
        unsigned short status;
+       unsigned long flags;
 
        /*
         * Read the interrupt status, and write it back to clear the
@@ -417,20 +428,26 @@ static irqreturn_t uhci_irq(struct usb_h
                if (status & USBSTS_HCPE)
                        dev_err(uhci_dev(uhci), "host controller process "
                                        "error, something bad happened!\n");
-               if ((status & USBSTS_HCH) &&
-                               uhci->rh_state >= UHCI_RH_RUNNING) {
-                       dev_err(uhci_dev(uhci), "host controller halted, "
+               if (status & USBSTS_HCH) {
+                       spin_lock_irqsave(&uhci->lock, flags);
+                       if (uhci->rh_state >= UHCI_RH_RUNNING) {
+                               dev_err(uhci_dev(uhci),
+                                       "host controller halted, "
                                        "very bad!\n");
-                       /* FIXME: Reset the controller, fix the offending TD */
+                               hc_died(uhci);
+                               spin_unlock_irqrestore(&uhci->lock, flags);
+                               return IRQ_HANDLED;
+                       }
+                       spin_unlock_irqrestore(&uhci->lock, flags);
                }
        }
 
        if (status & USBSTS_RD)
                uhci->resume_detect = 1;
 
-       spin_lock(&uhci->lock);
+       spin_lock_irqsave(&uhci->lock, flags);
        uhci_scan_schedule(uhci, regs);
-       spin_unlock(&uhci->lock);
+       spin_unlock_irqrestore(&uhci->lock, flags);
 
        return IRQ_HANDLED;
 }
@@ -525,10 +542,15 @@ static int uhci_start(struct usb_hcd *hc
        struct dentry *dentry;
 
        io_size = (unsigned) hcd->rsrc_len;
+       if (pci_find_capability(to_pci_dev(uhci_dev(uhci)), PCI_CAP_ID_PM))
+               hcd->can_wakeup = 1;            /* Assume it supports PME# */
 
-       dentry = debugfs_create_file(hcd->self.bus_name, 
S_IFREG|S_IRUGO|S_IWUSR, uhci_debugfs_root, uhci, &uhci_debug_operations);
+       dentry = debugfs_create_file(hcd->self.bus_name,
+                       S_IFREG|S_IRUGO|S_IWUSR, uhci_debugfs_root, uhci,
+                       &uhci_debug_operations);
        if (!dentry) {
-               dev_err(uhci_dev(uhci), "couldn't create uhci debugfs entry\n");
+               dev_err(uhci_dev(uhci),
+                               "couldn't create uhci debugfs entry\n");
                retval = -ENOMEM;
                goto err_create_debug_entry;
        }
@@ -765,7 +787,8 @@ static int uhci_rh_suspend(struct usb_hc
        struct uhci_hcd *uhci = hcd_to_uhci(hcd);
 
        spin_lock_irq(&uhci->lock);
-       suspend_rh(uhci, UHCI_RH_SUSPENDED);
+       if (!uhci->hc_inaccessible)             /* Not dead */
+               suspend_rh(uhci, UHCI_RH_SUSPENDED);
        spin_unlock_irq(&uhci->lock);
        return 0;
 }
@@ -773,26 +796,44 @@ static int uhci_rh_suspend(struct usb_hc
 static int uhci_rh_resume(struct usb_hcd *hcd)
 {
        struct uhci_hcd *uhci = hcd_to_uhci(hcd);
+       int rc = 0;
 
        spin_lock_irq(&uhci->lock);
-       wakeup_rh(uhci);
+       if (uhci->hc_inaccessible) {
+               if (uhci->rh_state == UHCI_RH_SUSPENDED) {
+                       dev_warn(uhci_dev(uhci), "HC isn't running!\n");
+                       rc = -ENODEV;
+               }
+               /* Otherwise the HC is dead */
+       } else
+               wakeup_rh(uhci);
        spin_unlock_irq(&uhci->lock);
-       return 0;
+       return rc;
 }
 
 static int uhci_suspend(struct usb_hcd *hcd, pm_message_t message)
 {
        struct uhci_hcd *uhci = hcd_to_uhci(hcd);
+       int rc = 0;
 
        dev_dbg(uhci_dev(uhci), "%s\n", __FUNCTION__);
 
        spin_lock_irq(&uhci->lock);
+       if (uhci->hc_inaccessible)      /* Dead or already suspended */
+               goto done;
 
 #ifndef CONFIG_USB_SUSPEND
        /* Otherwise this would never happen */
        suspend_rh(uhci, UHCI_RH_SUSPENDED);
 #endif
 
+       if (uhci->rh_state > UHCI_RH_SUSPENDED) {
+               dev_warn(uhci_dev(uhci), "Root hub isn't suspended!\n");
+               hcd->state = HC_STATE_RUNNING;
+               rc = -EBUSY;
+               goto done;
+       };
+
        /* All PCI host controllers are required to disable IRQ generation
         * at the source, so we must turn off PIRQ.
         */
@@ -801,8 +842,9 @@ static int uhci_suspend(struct usb_hcd *
 
        /* FIXME: Enable non-PME# remote wakeup? */
 
+done:
        spin_unlock_irq(&uhci->lock);
-       return 0;
+       return rc;
 }
 
 static int uhci_resume(struct usb_hcd *hcd)
@@ -811,6 +853,8 @@ static int uhci_resume(struct usb_hcd *h
 
        dev_dbg(uhci_dev(uhci), "%s\n", __FUNCTION__);
 
+       if (uhci->rh_state == UHCI_RH_RESET)    /* Dead */
+               return 0;
        spin_lock_irq(&uhci->lock);
 
        /* FIXME: Disable non-PME# remote wakeup? */



-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_idt77&alloc_id492&op=click
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to