Paul:

Ever since we worked on getting UHCI suspend to work correctly with boards 
that hardwire the overcurrent input of unused USB ports, back last May, 
our solution has bothered me.  We never really tried the alternate 
approach of letting the thing go into suspend mode and then checking 
more carefully about when to resume.

Below is a patch to the current 2.6 driver that implements the other
scheme.  If you still have the computer you were using back then, could
you give it a try?  It does work correctly on a normal motherboard.

Things to look for: Does an irq arrive when you plug in a device?  Is the
RESUME DETECT flag in the USBSTS status register set correctly?  Is the
controller suspended when no devices are attached?  Does it stay suspended
until a device is plugged in?  When a device is plugged in, does the
controller get woken up reliably?  I'm concerned that the resume_allowed()
logic could be fooled by initial connection transients.

There's also the possibility of polling for new connections while the 
controller is suspended, instead of waiting for an interrupt.  The 
uhci_hub_status_data() routine would be a perfect place to do it.

Thanks,

Alan Stern


--- 2.6/drivers/usb/host/uhci-hcd.c.orig        Sun Feb  8 11:35:24 2004
+++ 2.6/drivers/usb/host/uhci-hcd.c     Sun Feb  8 11:32:17 2004
@@ -1924,6 +1924,7 @@
        if (!status)    /* shared interrupt, not mine */
                return;
        outw(status, io_addr + USBSTS);         /* Clear it */
+       printk(KERN_DEBUG "%x: irq status 0x%x\n", io_addr, status);
 
        if (status & ~(USBSTS_USBINT | USBSTS_ERROR | USBSTS_RD)) {
                if (status & USBSTS_HSE)
@@ -2044,31 +2045,30 @@
        return connection;
 }
 
-static int suspend_allowed(struct uhci_hcd *uhci)
+static int resume_allowed(struct uhci_hcd *uhci)
 {
        unsigned int io_addr = uhci->io_addr;
+       unsigned int st;
        int i;
 
-       if (!uhci->hcd.pdev || uhci->hcd.pdev->vendor != PCI_VENDOR_ID_INTEL)
-               return 1;
-
        /* Some of Intel's USB controllers have a bug that causes false
-        * resume indications if any port has an over current condition.
-        * To prevent problems, we will not allow a global suspend if
-        * any ports are OC.
+        * resume indications if any port has an overcurrent condition.
+        * To prevent problems, we will not allow a resume (wakeup) unless
+        * a non-OC port is connected.
         *
         * Some motherboards using Intel's chipsets (but not using all
-        * the USB ports) appear to hardwire the over current inputs active
-        * to disable the USB ports.
+        * the USB ports) hardwire the overcurrent input active in order
+        * to disable a USB port.
         */
 
-       /* check for over current condition on any port */
        for (i = 0; i < uhci->rh_numports; i++) {
-               if (inw(io_addr + USBPORTSC1 + i * 2) & USBPORTSC_OC)
-                       return 0;
+               st = inw(io_addr + USBPORTSC1 + i * 2);
+               printk(KERN_DEBUG "%x: port %d resume status: 0x%x\n",
+                               io_addr, i, st);
+               if (!(st & USBPORTSC_OC) && (st & USBPORTSC_CCS))
+                       return 1;
        }
-
-       return 1;
+       return 0;
 }
 
 static void hc_state_transitions(struct uhci_hcd *uhci)
@@ -2077,7 +2077,7 @@
                case UHCI_RUNNING:
 
                        /* global suspend if nothing connected for 1 second */
-                       if (!ports_active(uhci) && suspend_allowed(uhci)) {
+                       if (!ports_active(uhci)) {
                                uhci->state = UHCI_SUSPENDING_GRACE;
                                uhci->state_end = jiffies + HZ;
                        }
@@ -2093,8 +2093,11 @@
                case UHCI_SUSPENDED:
 
                        /* wakeup if requested by a device */
-                       if (uhci->resume_detect)
-                               wakeup_hc(uhci);
+                       if (uhci->resume_detect) {
+                               uhci->resume_detect = 0;
+                               if (resume_allowed(uhci))
+                                       wakeup_hc(uhci);
+                       }
                        break;
 
                case UHCI_RESUMING_1:
@@ -2482,11 +2485,7 @@
 {
        struct uhci_hcd *uhci = hcd_to_uhci(hcd);
 
-       /* Don't try to suspend broken motherboards, reset instead */
-       if (suspend_allowed(uhci))
-               suspend_hc(uhci);
-       else
-               reset_hc(uhci);
+       suspend_hc(uhci);
        return 0;
 }
 
@@ -2496,12 +2495,7 @@
 
        pci_set_master(uhci->hcd.pdev);
 
-       if (uhci->state == UHCI_SUSPENDED)
-               uhci->resume_detect = 1;
-       else {
-               reset_hc(uhci);
-               start_hc(uhci);
-       }
+       uhci->resume_detect = 1;
        uhci->hcd.state = USB_STATE_RUNNING;
        return 0;
 }




-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to