>-----Original Message----- >From: Alan Stern [mailto:stern@xxxxxxxxxxxxxxxxxxx] >Sent: Friday, April 30, 2010 1:46 AM >To: Ondrej Zary; Du, Alek >Cc: Linux-pm mailing list; USB list; Kernel development list >Subject: Re: [PATCH v2] [RFC] ehci: Disable wake on overcurrent >(WKOC_E) and disconnect (WKDISC_E) > >On Thu, 29 Apr 2010, Alan Stern wrote: > >> > > > > You said earlier that the host controller was disabled for remote >> > > > > wakeup ("/sys/devices/pci0000:00/0000:00:1d.7/power/wakeup is >disabled >> > > > > by default"). So even though the root hub might issue a wakeup >> > > > > request, the controller hardware should not forward that request to >the >> > > > > PCI bus and it should not cause the system to wake up. >> > > > >> > > > Maybe it should not - but it wakes up this board and probably also >> > > > P4P800, P4P800-E and P4C800-E at least: >> > > > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/75497 >> > > >> > > Okay, evidently the hardware or firmware on these boards is buggy. >> > > Other systems do not have the same problem, as far as I know. >> >> I take this back. The same thing happens on my machine (Intel ICH5 >> chipset). I'd guess there is a bug in the PCI or ACPI subsystem, but >> more testing is needed before I can be sure. Somehow the PCI or >> platform wakeup mechanism gets activated even when it is supposed to be >> disabled. > >This patch fixes the problem on my system. Does it work for you? >I still think that disabling wakeup at the PCI or platform level should >override the port wakeup flags, but apparently it doesn't. > >Alek, can you confirm that the changes in this patch are okay for the >Moorestown EHCI controller? I had to guess at how to handle the >HOSTPCx register settings. Alan, I will test this patch when back to office, probably at 4 May. Thanks, Alek > >Alan Stern > > > >Index: usb-2.6/drivers/usb/host/ehci-hub.c >=================================================================== >--- usb-2.6.orig/drivers/usb/host/ehci-hub.c >+++ usb-2.6/drivers/usb/host/ehci-hub.c >@@ -106,6 +106,47 @@ static void ehci_handover_companion_port > ehci->owned_ports = 0; > } > >+static void ehci_set_wakeup_flags(struct ehci_hcd *ehci) >+{ >+ int port; >+ >+ /* enable remote wakeup on all ports, if allowed */ >+ port = HCS_N_PORTS(ehci->hcs_params); >+ while (port--) { >+ u32 __iomem *reg = &ehci->regs->port_status[port]; >+ u32 t1 = ehci_readl(ehci, reg) & ~PORT_RWC_BITS; >+ u32 t2 = t1 & ~PORT_WAKE_BITS; >+ >+ /* only enable appropriate wake bits, otherwise the >+ * hardware can not go phy low power mode. If a race >+ * condition happens here(connection change during bits >+ * set), the port change detection will finally fix it. >+ */ >+ if (device_may_wakeup(ehci_to_hcd(ehci)->self.controller)) { >+ if (t1 & PORT_CONNECT) >+ t2 |= PORT_WKOC_E | PORT_WKDISC_E; >+ else >+ t2 |= PORT_WKOC_E | PORT_WKCONN_E; >+ ehci_vdbg(ehci, "port %d, %08x -> %08x\n", >+ port + 1, t1, t2); >+ } >+ ehci_writel(ehci, t2, reg); >+ } >+} >+ >+static void ehci_clear_wakeup_flags(struct ehci_hcd *ehci) >+{ >+ int port; >+ >+ port = HCS_N_PORTS(ehci->hcs_params); >+ while (port--) { >+ u32 __iomem *reg = &ehci->regs->port_status[port]; >+ u32 t1 = ehci_readl(ehci, reg) & ~PORT_RWC_BITS; >+ >+ ehci_writel(ehci, t1 & ~PORT_WAKE_BITS, reg); >+ } >+} >+ > static int ehci_bus_suspend (struct usb_hcd *hcd) > { > struct ehci_hcd *ehci = hcd_to_ehci (hcd); >@@ -170,26 +211,6 @@ static int ehci_bus_suspend (struct usb_ > else if ((t1 & PORT_PE) && !(t1 & PORT_SUSPEND)) { > t2 |= PORT_SUSPEND; > set_bit(port, &ehci->bus_suspended); >- } >- >- /* enable remote wakeup on all ports */ >- if (hcd->self.root_hub->do_remote_wakeup) { >- /* only enable appropriate wake bits, otherwise the >- * hardware can not go phy low power mode. If a race >- * condition happens here(connection change during bits >- * set), the port change detection will finally fix it. >- */ >- if (t1 & PORT_CONNECT) { >- t2 |= PORT_WKOC_E | PORT_WKDISC_E; >- t2 &= ~PORT_WKCONN_E; >- } else { >- t2 |= PORT_WKOC_E | PORT_WKCONN_E; >- t2 &= ~PORT_WKDISC_E; >- } >- } else >- t2 &= ~PORT_WAKE_BITS; >- >- if (t1 != t2) { > ehci_vdbg (ehci, "port %d, %08x -> %08x\n", > port + 1, t1, t2); > ehci_writel(ehci, t2, reg); >@@ -907,12 +928,7 @@ static int ehci_hub_control ( > || (temp & PORT_RESET) != 0) > goto error; > >- /* After above check the port must be connected. >- * Set appropriate bit thus could put phy into low power >- * mode if we have hostpc feature >- */ >- temp &= ~PORT_WKCONN_E; >- temp |= PORT_WKDISC_E | PORT_WKOC_E; >+ temp &= ~PORT_WAKE_BITS; > ehci_writel(ehci, temp | PORT_SUSPEND, status_reg); > if (hostpc_reg) { > spin_unlock_irqrestore(&ehci->lock, flags); >Index: usb-2.6/drivers/usb/host/ehci-pci.c >=================================================================== >--- usb-2.6.orig/drivers/usb/host/ehci-pci.c >+++ usb-2.6/drivers/usb/host/ehci-pci.c >@@ -284,23 +284,15 @@ static int ehci_pci_suspend(struct usb_h > msleep(10); > > /* Root hub was already suspended. Disable irq emission and >- * mark HW unaccessible, bail out if RH has been resumed. Use >- * the spinlock to properly synchronize with possible pending >- * RH suspend or resume activity. >- * >- * This is still racy as hcd->state is manipulated outside of >- * any locks =P But that will be a different fix. >+ * mark HW unaccessible. The PM and USB cores make sure that >+ * the root hub is either suspended or stopped. > */ > spin_lock_irqsave (&ehci->lock, flags); >- if (hcd->state != HC_STATE_SUSPENDED) { >- rc = -EINVAL; >- goto bail; >- } >+ ehci_set_wakeup_flags(ehci); > ehci_writel(ehci, 0, &ehci->regs->intr_enable); > (void)ehci_readl(ehci, &ehci->regs->intr_enable); > > clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); >- bail: > spin_unlock_irqrestore (&ehci->lock, flags); > > // could save FLADJ in case of Vaux power loss >@@ -330,6 +322,7 @@ static int ehci_pci_resume(struct usb_hc > !hibernated) { > int mask = INTR_MASK; > >+ ehci_clear_wakeup_flags(ehci); > if (!hcd->self.root_hub->do_remote_wakeup) > mask &= ~STS_PCD; > ehci_writel(ehci, mask, &ehci->regs->intr_enable); >Index: usb-2.6/drivers/usb/host/ehci-au1xxx.c >=================================================================== >--- usb-2.6.orig/drivers/usb/host/ehci-au1xxx.c >+++ usb-2.6/drivers/usb/host/ehci-au1xxx.c >@@ -215,26 +215,17 @@ static int ehci_hcd_au1xxx_drv_suspend(s > msleep(10); > > /* Root hub was already suspended. Disable irq emission and >- * mark HW unaccessible, bail out if RH has been resumed. Use >- * the spinlock to properly synchronize with possible pending >- * RH suspend or resume activity. >- * >- * This is still racy as hcd->state is manipulated outside of >- * any locks =P But that will be a different fix. >+ * mark HW unaccessible. The PM and USB cores make sure that >+ * the root hub is either suspended or stopped. > */ > spin_lock_irqsave(&ehci->lock, flags); >- if (hcd->state != HC_STATE_SUSPENDED) { >- rc = -EINVAL; >- goto bail; >- } >+ ehci_set_wakeup_flags(ehci); > ehci_writel(ehci, 0, &ehci->regs->intr_enable); > (void)ehci_readl(ehci, &ehci->regs->intr_enable); > > clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); > > au1xxx_stop_ehc(); >- >-bail: > spin_unlock_irqrestore(&ehci->lock, flags); > > // could save FLADJ in case of Vaux power loss >@@ -264,6 +255,7 @@ static int ehci_hcd_au1xxx_drv_resume(st > if (ehci_readl(ehci, &ehci->regs->configured_flag) == FLAG_CF) { > int mask = INTR_MASK; > >+ ehci_clear_wakeup_flags(ehci); > if (!hcd->self.root_hub->do_remote_wakeup) > mask &= ~STS_PCD; > ehci_writel(ehci, mask, &ehci->regs->intr_enable); >Index: usb-2.6/drivers/usb/host/ehci-fsl.c >=================================================================== >--- usb-2.6.orig/drivers/usb/host/ehci-fsl.c >+++ usb-2.6/drivers/usb/host/ehci-fsl.c >@@ -313,6 +313,7 @@ static int ehci_fsl_drv_suspend(struct d > struct ehci_fsl *ehci_fsl = hcd_to_ehci_fsl(hcd); > void __iomem *non_ehci = hcd->regs; > >+ ehci_set_wakeup_flags(ehci); > if (!fsl_deep_sleep()) > return 0; > >@@ -327,6 +328,7 @@ static int ehci_fsl_drv_resume(struct de > struct ehci_hcd *ehci = hcd_to_ehci(hcd); > void __iomem *non_ehci = hcd->regs; > >+ ehci_clear_wakeup_flags(ehci); > if (!fsl_deep_sleep()) > return 0; > _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
