Return-Path: X-Original-To: colinomail@colino.net Delivered-To: colinomail@colino.net Received: by paperstreet.colino.net (Postfix, from userid 1015) id 65F2C101D9; Tue, 5 Apr 2005 23:00:28 +0200 (CEST) Received: from ylpvm43.prodigy.net (ylpvm43-ext.prodigy.net [207.115.57.74]) by paperstreet.colino.net (Postfix) with ESMTP id A0233101D8 for ; Tue, 5 Apr 2005 23:00:17 +0200 (CEST) Received: from pimout5-ext.prodigy.net (pimout5-ext.prodigy.net [207.115.63.73]) by ylpvm43.prodigy.net (8.12.10 outbound/8.12.10) with ESMTP id j35L0GtF026117 for ; Tue, 5 Apr 2005 17:00:16 -0400 X-ORBL: [69.107.61.180] Received: from ascent (adsl-69-107-61-180.dsl.pltn13.pacbell.net [69.107.61.180]) by pimout5-ext.prodigy.net (8.12.10 milter /8.12.10) with ESMTP id j35L06nG086356; Tue, 5 Apr 2005 17:00:10 -0400 From: David Brownell To: linux-usb-devel@lists.sourceforge.net Subject: [patch 2.6.12-rc2] usb hcd/pci suspend/resume fixes Date: Tue, 5 Apr 2005 14:00:05 -0700 User-Agent: KMail/1.7.1 Cc: Colin Leroy References: <20050405204449.5ab0cdea@jack.colino.net> In-Reply-To: <20050405204449.5ab0cdea@jack.colino.net> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_VxvUCPiyUY60lfh" Message-Id: <200504051400.05582.david-b@pacbell.net> X-Spam-Checker-Version: SpamAssassin 2.64 (2004-01-11) on paperstreet.colino.net X-Spam-Level: X-Spam-Status: No, hits=0.2 required=5.0 tests=AWL autolearn=no version=2.64 X-UIDL: @O_"!]`Y"!*Pp!!PHQ"! --Boundary-00=_VxvUCPiyUY60lfh Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline This is a resend; the patch is in 2.6.12-rc2-mm1 already. - Dave --Boundary-00=_VxvUCPiyUY60lfh Content-Type: text/x-diff; charset="us-ascii"; name="susp-0330a.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="susp-0330a.patch" This has a variety of updates to the shared suspend/resume code for PCI based USB host controllers. - Cope with pm_message_t replacing the target system state. This is actually a loss of functionality; PCI D1 and D2 states will no longer be used, and it's no longer knowable that D3cold is on the way so power will be lost. - Most importantly, some of the resume paths are reworked and cleaned up. They're now an exact mirror of suspend paths, and more care is taken to ensure the hardware is reactivated before the hardware re-enables interrupts. Plus comment and diagnostic cleanups; there are some nasty cases here especially combined with swsusp, now they're somewhat commented. Signed-off-by: David Brownell Signed-off-by: Andrew Morton 25-akpm/drivers/usb/core/hcd-pci.c | 151 ++++++++++++++++++++++--------------- 1 files changed, 91 insertions(+), 60 deletions(-) --- 25/drivers/usb/core/hcd-pci.c~usb-resume-fixes Wed Mar 30 13:26:29 2005 +++ 25-akpm/drivers/usb/core/hcd-pci.c Wed Mar 30 13:26:29 2005 @@ -33,7 +33,7 @@ #include "hcd.h" -/* PCI-based HCs are normal, but custom bus glue should be ok */ +/* PCI-based HCs are common, but plenty of non-PCI HCs are used too */ /*-------------------------------------------------------------------------*/ @@ -67,8 +67,8 @@ int usb_hcd_pci_probe (struct pci_dev *d if (pci_enable_device (dev) < 0) return -ENODEV; - dev->current_state = 0; - dev->dev.power.power_state = 0; + dev->current_state = PCI_D0; + dev->dev.power.power_state = PMSG_ON; if (!dev->irq) { dev_err (&dev->dev, @@ -186,26 +186,14 @@ EXPORT_SYMBOL (usb_hcd_pci_remove); #ifdef CONFIG_PM -static char __attribute_used__ *pci_state(u32 state) -{ - switch (state) { - case 0: return "D0"; - case 1: return "D1"; - case 2: return "D2"; - case 3: return "D3hot"; - case 4: return "D3cold"; - } - return NULL; -} - /** * usb_hcd_pci_suspend - power management suspend of a PCI-based HCD * @dev: USB Host Controller being suspended - * @state: state that the controller is going into + * @message: semantics in flux * * Store this function in the HCD's struct pci_driver as suspend(). */ -int usb_hcd_pci_suspend (struct pci_dev *dev, u32 state) +int usb_hcd_pci_suspend (struct pci_dev *dev, pm_message_t message) { struct usb_hcd *hcd; int retval = 0; @@ -213,13 +201,23 @@ int usb_hcd_pci_suspend (struct pci_dev hcd = pci_get_drvdata(dev); + /* FIXME until the generic PM interfaces change a lot more, this + * can't use PCI D1 and D2 states. For example, the confusion + * between messages and states will need to vanish, and messages + * will need to provide a target system state again. + * + * It'll be important to learn characteristics of the target state, + * especially on embedded hardware where the HCD will often be in + * charge of an external VBUS power supply and one or more clocks. + * Some target system states will leave them active; others won't. + * (With PCI, that's often handled by platform BIOS code.) + */ + /* even when the PCI layer rejects some of the PCI calls * below, HCs can try global suspend and reduce DMA traffic. * PM-sensitive HCDs may already have done this. */ has_pci_pm = pci_find_capability(dev, PCI_CAP_ID_PM); - if (state > 4) - state = 4; switch (hcd->state) { @@ -228,7 +226,7 @@ int usb_hcd_pci_suspend (struct pci_dev */ case HC_STATE_RUNNING: hcd->state = HC_STATE_QUIESCING; - retval = hcd->driver->suspend (hcd, state); + retval = hcd->driver->suspend (hcd, message); if (retval) { dev_dbg (hcd->self.controller, "suspend fail, retval %d\n", @@ -246,14 +244,11 @@ int usb_hcd_pci_suspend (struct pci_dev * have been called, otherwise root hub timers still run ... */ case HC_STATE_SUSPENDED: - if (state <= dev->current_state) - break; - - /* no DMA or IRQs except in D0 */ - if (!dev->current_state) { + /* no DMA or IRQs except when HC is active */ + if (dev->current_state == PCI_D0) { + free_irq (hcd->irq, hcd); pci_save_state (dev); pci_disable_device (dev); - free_irq (hcd->irq, hcd); } if (!has_pci_pm) { @@ -261,25 +256,19 @@ int usb_hcd_pci_suspend (struct pci_dev break; } - /* POLICY: ignore D1/D2/D3hot differences; - * we know D3hot will always work. + /* NOTE: dev->current_state becomes nonzero only here, and + * only for devices that support PCI PM. Also, exiting + * PCI_D3 (but not PCI_D1 or PCI_D2) is allowed to reset + * some device state (e.g. as part of clock reinit). */ - retval = pci_set_power_state (dev, state); - if (retval < 0 && state < 3) { - retval = pci_set_power_state (dev, 3); - if (retval == 0) - state = 3; - } + retval = pci_set_power_state (dev, PCI_D3hot); if (retval == 0) { - dev_dbg (hcd->self.controller, "--> PCI %s\n", - pci_state(dev->current_state)); -#ifdef CONFIG_USB_SUSPEND - pci_enable_wake (dev, state, hcd->remote_wakeup); - pci_enable_wake (dev, 4, hcd->remote_wakeup); -#endif + dev_dbg (hcd->self.controller, "--> PCI D3\n"); + pci_enable_wake (dev, PCI_D3hot, hcd->remote_wakeup); + pci_enable_wake (dev, PCI_D3cold, hcd->remote_wakeup); } else if (retval < 0) { - dev_dbg (&dev->dev, "PCI %s suspend fail, %d\n", - pci_state(state), retval); + dev_dbg (&dev->dev, "PCI D3 suspend fail, %d\n", + retval); (void) usb_hcd_pci_resume (dev); break; } @@ -287,13 +276,14 @@ int usb_hcd_pci_suspend (struct pci_dev default: dev_dbg (hcd->self.controller, "hcd state %d; not suspended\n", hcd->state); + WARN_ON(1); retval = -EINVAL; break; } /* update power_state **ONLY** to make sysfs happier */ if (retval == 0) - dev->dev.power.power_state = state; + dev->dev.power.power_state = message; return retval; } EXPORT_SYMBOL (usb_hcd_pci_suspend); @@ -308,7 +298,6 @@ int usb_hcd_pci_resume (struct pci_dev * { struct usb_hcd *hcd; int retval; - int has_pci_pm; hcd = pci_get_drvdata(dev); if (hcd->state != HC_STATE_SUSPENDED) { @@ -316,31 +305,73 @@ int usb_hcd_pci_resume (struct pci_dev * "can't resume, not suspended!\n"); return 0; } - has_pci_pm = pci_find_capability(dev, PCI_CAP_ID_PM); - /* D3cold resume isn't usually reported this way... */ - dev_dbg(hcd->self.controller, "resume from PCI %s%s\n", - pci_state(dev->current_state), - has_pci_pm ? "" : " (legacy)"); + /* NOTE: chip docs cover clean "real suspend" cases (what Linux + * calls "standby", "suspend to RAM", and so on). There are also + * dirty cases when swsusp fakes a suspend in "shutdown" mode. + */ + if (dev->current_state != PCI_D0) { +#ifdef DEBUG + int pci_pm; + u16 pmcr; + + pci_pm = pci_find_capability(dev, PCI_CAP_ID_PM); + pci_read_config_word(dev, pci_pm + PCI_PM_CTRL, &pmcr); + pmcr &= PCI_PM_CTRL_STATE_MASK; + if (pmcr) { + /* Clean case: power to USB and to HC registers was + * maintained; remote wakeup is easy. + */ + dev_dbg(hcd->self.controller, "resume from PCI D%d\n", + pmcr); + } else { + /* Clean: HC lost Vcc power, D0 uninitialized + * + Vaux may have preserved port and transceiver + * state ... for remote wakeup from D3cold + * + or not; HCD must reinit + re-enumerate + * + * Dirty: D0 semi-initialized cases with swsusp + * + after BIOS init + * + after Linux init (HCD statically linked) + */ + dev_dbg(hcd->self.controller, + "PCI D0, from previous PCI D%d\n", + dev->current_state); + } +#endif + pci_enable_wake (dev, dev->current_state, 0); + pci_enable_wake (dev, PCI_D3cold, 0); + } else { + /* Same basic cases: clean (powered/not), dirty */ + dev_dbg(hcd->self.controller, "PCI legacy resume\n"); + } - hcd->state = HC_STATE_RESUMING; + /* NOTE: the PCI API itself is asymmetric here. We don't need to + * pci_set_power_state(PCI_D0) since that's part of re-enabling; + * but that won't re-enable bus mastering. Yet pci_disable_device() + * explicitly disables bus mastering... + */ + retval = pci_enable_device (dev); + if (retval < 0) { + dev_err (hcd->self.controller, + "can't re-enable after resume, %d!\n", retval); + return retval; + } + pci_set_master (dev); + pci_restore_state (dev); - if (has_pci_pm) - pci_set_power_state (dev, 0); - dev->dev.power.power_state = 0; + dev->dev.power.power_state = PMSG_ON; + + hcd->state = HC_STATE_RESUMING; + hcd->saw_irq = 0; retval = request_irq (dev->irq, usb_hcd_irq, SA_SHIRQ, - hcd->driver->description, hcd); + hcd->irq_descr, hcd); if (retval < 0) { dev_err (hcd->self.controller, "can't restore IRQ after resume!\n"); + usb_hc_died (hcd); return retval; } - hcd->saw_irq = 0; - pci_restore_state (dev); -#ifdef CONFIG_USB_SUSPEND - pci_enable_wake (dev, dev->current_state, 0); - pci_enable_wake (dev, 4, 0); -#endif retval = hcd->driver->resume (hcd); if (!HC_IS_RUNNING (hcd->state)) { _ --Boundary-00=_VxvUCPiyUY60lfh--