Re: [linux-pm] [PATCH v2] [RFC] ehci: Disable wake on overcurrent(WKOC_E) and disconnect (WKDISC_E)
From: Du, Alek
Date: Mon May 10 2010 - 23:35:14 EST
On Tue, 11 May 2010 05:05:34 +0800
Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, 10 May 2010, Du, Alek wrote:
>
> > Alan,
> >
> > The following patch (based on your previous two patches) works for me, could you
> > help to review and test on your machine? Basically, this will only affect those
> > "has_hostpc" HCDs.
>
> Based on your suggestion, I completely redid both of my patches. They
> are now combined into a single patch, which is meant to go on top of
> the patch you submitted this morning. It adds the stuff we talked
> about, and it cleans up some of the changes you made.
>
> Does it look good to you?
>
> Alan Stern
>
Alan,
It looks good and works for my hardware. Thanks a lot!
Will you submit it soon?
Alek
>
>
> 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,12 +106,72 @@ static void ehci_handover_companion_port
> ehci->owned_ports = 0;
> }
>
> +static void ehci_adjust_port_wakeup_flags(struct ehci_hcd *ehci, bool resuming)
> +{
> + int port;
> + u32 temp;
> +
> + /* If remote wakeup is enabled for the root hub but disabled
> + * for the controller, we must adjust all the port wakeup flags.
> + */
> + if (!ehci_to_hcd(ehci)->self.root_hub->do_remote_wakeup ||
> + device_may_wakeup(ehci_to_hcd(ehci)->self.controller))
> + return;
> +
> + /* clear phy low-power mode before changing wakeup flags */
> + if (ehci->has_hostpc) {
> + port = HCS_N_PORTS(ehci->hcs_params);
> + while (port--) {
> + u32 __iomem *hostpc_reg;
> +
> + hostpc_reg = (u32 __iomem *)((u8 *) ehci->regs
> + + HOSTPC0 + 4 * port);
> + temp = ehci_readl(ehci, hostpc_reg);
> + ehci_writel(ehci, temp & ~HOSTPC_PHCD, hostpc_reg);
> + }
> + msleep(5);
> + }
> +
> + 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;
> +
> + /* If we are suspending the controller, clear the flags.
> + * If we are resuming the controller, set the wakeup flags.
> + */
> + if (resuming) {
> + 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);
> + }
> +
> + /* enter phy low-power mode again */
> + if (ehci->has_hostpc) {
> + port = HCS_N_PORTS(ehci->hcs_params);
> + while (port--) {
> + u32 __iomem *hostpc_reg;
> +
> + hostpc_reg = (u32 __iomem *)((u8 *) ehci->regs
> + + HOSTPC0 + 4 * port);
> + temp = ehci_readl(ehci, hostpc_reg);
> + ehci_writel(ehci, temp | HOSTPC_PHCD, hostpc_reg);
> + }
> + }
> +}
> +
> static int ehci_bus_suspend (struct usb_hcd *hcd)
> {
> struct ehci_hcd *ehci = hcd_to_ehci (hcd);
> int port;
> int mask;
> - u32 __iomem *hostpc_reg = NULL;
> + int changed;
>
> ehci_dbg(ehci, "suspend root hub\n");
>
> @@ -155,15 +215,13 @@ static int ehci_bus_suspend (struct usb_
> */
> ehci->bus_suspended = 0;
> ehci->owned_ports = 0;
> + changed = 0;
> 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;
> + u32 t2 = t1 & ~PORT_WAKE_BITS;
>
> - if (ehci->has_hostpc)
> - hostpc_reg = (u32 __iomem *)((u8 *)ehci->regs
> - + HOSTPC0 + 4 * (port & 0xff));
> /* keep track of which ports we suspend */
> if (t1 & PORT_OWNER)
> set_bit(port, &ehci->owned_ports);
> @@ -172,40 +230,45 @@ static int ehci_bus_suspend (struct usb_
> set_bit(port, &ehci->bus_suspended);
> }
>
> - /* enable remote wakeup on all ports */
> + /* enable remote wakeup on all ports, if told to do so */
> 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) {
> + if (t1 & PORT_CONNECT)
> t2 |= PORT_WKOC_E | PORT_WKDISC_E;
> - t2 &= ~PORT_WKCONN_E;
> - } else {
> + 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);
> - if (hostpc_reg) {
> - u32 t3;
> + changed = 1;
> + }
> + }
>
> - spin_unlock_irq(&ehci->lock);
> - msleep(5);/* 5ms for HCD enter low pwr mode */
> - spin_lock_irq(&ehci->lock);
> - t3 = ehci_readl(ehci, hostpc_reg);
> - ehci_writel(ehci, t3 | HOSTPC_PHCD, hostpc_reg);
> - t3 = ehci_readl(ehci, hostpc_reg);
> - ehci_dbg(ehci, "Port%d phy low pwr mode %s\n",
> + if (changed && ehci->has_hostpc) {
> + spin_unlock_irq(&ehci->lock);
> + msleep(5); /* 5 ms for HCD to enter low-power mode */
> + spin_lock_irq(&ehci->lock);
> +
> + port = HCS_N_PORTS(ehci->hcs_params);
> + while (port--) {
> + u32 __iomem *hostpc_reg;
> + u32 t3;
> +
> + hostpc_reg = (u32 __iomem *)((u8 *) ehci->regs
> + + HOSTPC0 + 4 * port);
> + t3 = ehci_readl(ehci, hostpc_reg);
> + ehci_writel(ehci, t3 | HOSTPC_PHCD, hostpc_reg);
> + t3 = ehci_readl(ehci, hostpc_reg);
> + ehci_dbg(ehci, "Port %d phy low-power mode %s\n",
> port, (t3 & HOSTPC_PHCD) ?
> "succeeded" : "failed");
> - }
> }
> }
>
> @@ -291,19 +354,28 @@ static int ehci_bus_resume (struct usb_h
> msleep(8);
> spin_lock_irq(&ehci->lock);
>
> + /* clear phy low-power mode before resume */
> + if (ehci->bus_suspended && ehci->has_hostpc) {
> + i = HCS_N_PORTS (ehci->hcs_params);
> + while (i--) {
> + if (test_bit(i, &ehci->bus_suspended)) {
> + u32 __iomem *hostpc_reg;
> +
> + hostpc_reg = (u32 __iomem *)((u8 *) ehci->regs
> + + HOSTPC0 + 4 * i);
> + temp = ehci_readl(ehci, hostpc_reg);
> + ehci_writel(ehci, temp & ~HOSTPC_PHCD,
> + hostpc_reg);
> + }
> + }
> + spin_unlock_irq(&ehci->lock);
> + msleep(5);
> + spin_lock_irq(&ehci->lock);
> + }
> +
> /* manually resume the ports we suspended during bus_suspend() */
> i = HCS_N_PORTS (ehci->hcs_params);
> while (i--) {
> - /* clear phy low power mode before resume */
> - if (ehci->has_hostpc) {
> - u32 __iomem *hostpc_reg =
> - (u32 __iomem *)((u8 *)ehci->regs
> - + HOSTPC0 + 4 * (i & 0xff));
> - temp = ehci_readl(ehci, hostpc_reg);
> - ehci_writel(ehci, temp & ~HOSTPC_PHCD,
> - hostpc_reg);
> - mdelay(5);
> - }
> temp = ehci_readl(ehci, &ehci->regs->port_status [i]);
> temp &= ~(PORT_RWC_BITS | PORT_WAKE_BITS);
> if (test_bit(i, &ehci->bus_suspended) &&
> @@ -685,23 +757,25 @@ static int ehci_hub_control (
> goto error;
> if (ehci->no_selective_suspend)
> break;
> - if (temp & PORT_SUSPEND) {
> - if ((temp & PORT_PE) == 0)
> - goto error;
> - /* clear phy low power mode before resume */
> - if (hostpc_reg) {
> - temp1 = ehci_readl(ehci, hostpc_reg);
> - ehci_writel(ehci, temp1 & ~HOSTPC_PHCD,
> - hostpc_reg);
> - mdelay(5);
> - }
> - /* resume signaling for 20 msec */
> - temp &= ~(PORT_RWC_BITS | PORT_WAKE_BITS);
> - ehci_writel(ehci, temp | PORT_RESUME,
> - status_reg);
> - ehci->reset_done [wIndex] = jiffies
> - + msecs_to_jiffies (20);
> + if (!(temp & PORT_SUSPEND))
> + break;
> + if ((temp & PORT_PE) == 0)
> + goto error;
> +
> + /* clear phy low-power mode before resume */
> + if (hostpc_reg) {
> + temp1 = ehci_readl(ehci, hostpc_reg);
> + ehci_writel(ehci, temp1 & ~HOSTPC_PHCD,
> + hostpc_reg);
> + spin_unlock_irqrestore(&ehci->lock, flags);
> + msleep(5);/* wait to leave low-power mode */
> + spin_lock_irqsave(&ehci->lock, flags);
> }
> + /* resume signaling for 20 msec */
> + temp &= ~(PORT_RWC_BITS | PORT_WAKE_BITS);
> + ehci_writel(ehci, temp | PORT_RESUME, status_reg);
> + ehci->reset_done[wIndex] = jiffies
> + + msecs_to_jiffies(20);
> break;
> case USB_PORT_FEAT_C_SUSPEND:
> clear_bit(wIndex, &ehci->port_c_suspend);
> 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
> @@ -287,23 +287,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_adjust_port_wakeup_flags(ehci, false);
> 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
> @@ -333,6 +325,7 @@ static int ehci_pci_resume(struct usb_hc
> !hibernated) {
> int mask = INTR_MASK;
>
> + ehci_adjust_port_wakeup_flags(ehci, true);
> 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
> @@ -224,26 +224,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_adjust_port_wakeup_flags(ehci, false);
> 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
> @@ -273,6 +264,7 @@ static int ehci_hcd_au1xxx_drv_resume(st
> if (ehci_readl(ehci, &ehci->regs->configured_flag) == FLAG_CF) {
> int mask = INTR_MASK;
>
> + ehci_adjust_port_wakeup_flags(ehci, true);
> 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_adjust_port_wakeup_flags(ehci, false);
> 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_adjust_port_wakeup_flags(ehci, true);
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/