lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 29 Apr 2010 13:45:33 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Ondrej Zary <linux@...nbow-software.org>,
	Alek Du <alek.du@...el.com>
cc:	Linux-pm mailing list <linux-pm@...ts.linux-foundation.org>,
	USB list <linux-usb@...r.kernel.org>,
	Kernel development list <linux-kernel@...r.kernel.org>
Subject: Re: [linux-pm] [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 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;
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ