RE: [linux-pm] [PATCH v2] [RFC] ehci: Disable wake on overcurrent(WKOC_E) and disconnect (WKDISC_E)

From: Du, Alek
Date: Thu May 06 2010 - 04:23:35 EST


>> Alan,
>>
>> As I tested, the patch breaks phy low power mode, the EHCI works, but it
>never
>> enter phy low power mode when suspending port...
>>
>> I guess this part of the diff breaks it.
>>
>>
>> - /* 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;
>> - }
>
>That seems very odd. Why should removing code that enables wakeup bits
>cause the suspend to fail? Can't the phy go into low-power mode when
>all the wakeup bits are disabled?
>
>Does applying this additional patch on top of the previous one help?
>
>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
>@@ -200,7 +200,7 @@ static int ehci_bus_suspend (struct usb_
> while (port--) {
> u32 __iomem *reg = &ehci->regs->port_status [port];
> u32 t1 = ehci_readl(ehci, reg) & ~PORT_RWC_BITS;
>- u32 t2 = t1;
>+ u32 t2;
>
> if (ehci->has_hostpc)
> hostpc_reg = (u32 __iomem *)((u8 *)ehci->regs
>@@ -209,6 +209,7 @@ static int ehci_bus_suspend (struct usb_
> if (t1 & PORT_OWNER)
> set_bit(port, &ehci->owned_ports);
> else if ((t1 & PORT_PE) && !(t1 & PORT_SUSPEND)) {
>+ t2 = t1 & ~PORT_WAKE_BITS;
> t2 |= PORT_SUSPEND;
> set_bit(port, &ehci->bus_suspended);
> ehci_vdbg (ehci, "port %d, %08x -> %08x\n",


Alan,

I think the problem is: For the original code, once t2 != t1, the HCD will try to put into phy low power mode. While after the patch, the HCD will only enter phy low power mode if PORT_PE is set and PORT_SUSPEND is not set.

Thanks,
Alek

--
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/