Messages in this thread Patch in this message | | | Date | Tue, 19 Jan 2010 16:31:31 +0800 | From | alek du <> | Subject | [PATCH] ehci: phy low power mode bug fixing |
| |
This patch should fix the bug and I tested it with Moorestown platform... But the strange thing is that even the original code does not trigger the "scheduling while atomic" issue...
From ca7833d854867e91d6aa6bccf1f3224862b3a25c Mon Sep 17 00:00:00 2001 From: Alek Du <alek.du@intel.com> Date: Tue, 19 Jan 2010 16:05:15 +0800 Subject: [PATCH] ehci: phy low power mode bug fixing
1. There are two msleep calls inside two spin lock sections, need to unlock and lock again after msleep. 2. Save a extra status reg setting.
Signed-off-by: Alek Du <alek.du@intel.com> --- drivers/usb/host/ehci-hub.c | 13 ++++++++----- 1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c index 2c6571c..7d77fbb 100644 --- a/drivers/usb/host/ehci-hub.c +++ b/drivers/usb/host/ehci-hub.c @@ -178,7 +178,9 @@ static int ehci_bus_suspend (struct usb_hcd *hcd) if (hostpc_reg) { u32 t3; + 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); @@ -886,17 +888,18 @@ static int ehci_hub_control ( if ((temp & PORT_PE) == 0 || (temp & PORT_RESET) != 0) goto error; - ehci_writel(ehci, temp | PORT_SUSPEND, status_reg); + /* 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; + ehci_writel(ehci, temp | PORT_SUSPEND, status_reg); if (hostpc_reg) { - temp &= ~PORT_WKCONN_E; - temp |= (PORT_WKDISC_E | PORT_WKOC_E); - ehci_writel(ehci, temp | PORT_SUSPEND, - status_reg); + spin_unlock_irqrestore(&ehci->lock, flags); msleep(5);/* 5ms for HCD enter low pwr mode */ + spin_lock_irqsave(&ehci->lock, flags); temp1 = ehci_readl(ehci, hostpc_reg); ehci_writel(ehci, temp1 | HOSTPC_PHCD, hostpc_reg); -- 1.6.3.3
On Tue, 19 Jan 2010 06:25:10 +0800 Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 18 January 2010, Julia Lawall wrote: > > > That code looks indeed broken as was added las July as part of > > > 331ac6b288d9 "USB: EHCI: Add Intel Moorestown EHCI controller > > > HOSTPCx extensions and support phy low power mode". The reason > > > that this hasn't triggered is probably the lack of Moorestown > > > machines in the field. > > > > The fix is just msleep -> mdelay? > > No, that would just kill the warning but still leave horrible code. > There should really be some way to move the sleeping operation > outside of the spinlock. > > From a brief look at the code, I think the sequence could be converted > from > > lock(); > suspend(); > sleep(); > clock_disable(); > unlock(); > > to > > lock(); > again: > suspend(); > unlock(); > sleep(); > lock(); > if (!suspended()) > goto again; > clock_disable(); > unlock(); > > Arnd
| |