Hi,
Sorry for being late, but ->
On Wednesday 07 January 2009, Greg Kroah-Hartman wrote:
> From: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
>
> This patch (as1192) rearranges the USB PCI host controller suspend and
> resume and resume routines:
>
> Use pci_wake_from_d3() for enabling and disabling wakeup,
> instead of pci_enable_wake().
>
> Carry out the actual state change while interrupts are
> disabled.
>
> Change the order of the preparations to agree with the
> general recommendation for PCI devices, instead of
> messing around with the wakeup settings while the device
> is in D3.
>
> In .suspend:
> Call the underlying driver to disable IRQ
> generation;
> pci_wake_from_d3(device_may_wakeup());
> pci_disable_device();
>
> In .suspend_late:
> pci_save_state();
> pci_set_power_state(D3hot);
->
pci_set_power_state() does things that shouldn't be done with interrupts off,
so I'd move the above two into .suspend.
> (for PPC_PMAC) Disable ASIC clocks
> In .resume_early:
> (for PPC_PMAC) Enable ASIC clocks
> pci_set_power_state(D0);
the pci_set_power_state(D0) is not necessary here IMO. If the device is
accessible at all, its config space is accessible too, even in D3.
pci_enable_device() does pci_set_power_state(D0) anyway.
> pci_restore_state();
>
> In .resume:
> pci_enable_device();
> pci_set_master();
> pci_wake_from_d3(0);
> Call the underlying driver to reenable IRQ
> generation
>
> Add the necessary .suspend_late and .resume_early method
> pointers to the PCI host controller drivers.
Thanks,
Rafael
> Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> CC: Rafael J. Wysocki <rjw@xxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>
> ---
> drivers/usb/core/hcd-pci.c | 200 +++++++++++++++++++++++--------------------
> drivers/usb/core/hcd.h | 4 +-
> drivers/usb/host/ehci-pci.c | 2 +
> drivers/usb/host/ohci-pci.c | 2 +
> drivers/usb/host/uhci-hcd.c | 2 +
> 5 files changed, 115 insertions(+), 95 deletions(-)
>
> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
> index 5b87ae7..9943278 100644
> --- a/drivers/usb/core/hcd-pci.c
> +++ b/drivers/usb/core/hcd-pci.c
> @@ -191,17 +191,15 @@ EXPORT_SYMBOL_GPL(usb_hcd_pci_remove);
> /**
> * usb_hcd_pci_suspend - power management suspend of a PCI-based HCD
> * @dev: USB Host Controller being suspended
> - * @message: semantics in flux
> + * @message: Power Management message describing this state transition
> *
> - * Store this function in the HCD's struct pci_driver as suspend().
> + * Store this function in the HCD's struct pci_driver as .suspend.
> */
> int usb_hcd_pci_suspend(struct pci_dev *dev, pm_message_t message)
> {
> - struct usb_hcd *hcd;
> + struct usb_hcd *hcd = pci_get_drvdata(dev);
> int retval = 0;
> - int has_pci_pm;
> -
> - hcd = pci_get_drvdata(dev);
> + int wake, w;
>
> /* Root hub suspend should have stopped all downstream traffic,
> * and all bus master traffic. And done so for both the interface
> @@ -212,8 +210,15 @@ int usb_hcd_pci_suspend(struct pci_dev *dev, pm_message_t message)
> * otherwise the swsusp will save (and restore) garbage state.
> */
> if (!(hcd->state == HC_STATE_SUSPENDED ||
> - hcd->state == HC_STATE_HALT))
> - return -EBUSY;
> + hcd->state == HC_STATE_HALT)) {
> + dev_warn(&dev->dev, "Root hub is not suspended\n");
> + retval = -EBUSY;
> + goto done;
> + }
> +
> + /* We might already be suspended (runtime PM -- not yet written) */
> + if (dev->current_state != PCI_D0)
> + goto done;
>
> if (hcd->driver->pci_suspend) {
> retval = hcd->driver->pci_suspend(hcd, message);
> @@ -221,49 +226,60 @@ int usb_hcd_pci_suspend(struct pci_dev *dev, pm_message_t message)
> if (retval)
> goto done;
> }
> - synchronize_irq(dev->irq);
>
> - /* 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.)
> - */
> + synchronize_irq(dev->irq);
>
> - /* 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.
> + /* Don't fail on error to enable wakeup. We rely on pci code
> + * to reject requests the hardware can't implement, rather
> + * than coding the same thing.
> */
> - has_pci_pm = pci_find_capability(dev, PCI_CAP_ID_PM);
> + wake = (hcd->state == HC_STATE_SUSPENDED &&
> + device_may_wakeup(&dev->dev));
> + w = pci_wake_from_d3(dev, wake);
> + if (w < 0)
> + wake = w;
> + dev_dbg(&dev->dev, "wakeup: %d\n", wake);
>
> /* Downstream ports from this root hub should already be quiesced, so
> * there will be no DMA activity. Now we can shut down the upstream
> * link (except maybe for PME# resume signaling) and enter some PCI
> * low power state, if the hardware allows.
> */
> - if (hcd->state == HC_STATE_SUSPENDED) {
> + pci_disable_device(dev);
> + done:
> + return retval;
> +}
> +EXPORT_SYMBOL_GPL(usb_hcd_pci_suspend);
>
> - /* no DMA or IRQs except when HC is active */
> - if (dev->current_state == PCI_D0) {
> - pci_save_state(dev);
> - pci_disable_device(dev);
> - }
> +/**
> + * usb_hcd_pci_suspend_late - suspend a PCI-based HCD after IRQs are disabled
> + * @dev: USB Host Controller being suspended
> + * @message: Power Management message describing this state transition
> + *
> + * Store this function in the HCD's struct pci_driver as .suspend_late.
> + */
> +int usb_hcd_pci_suspend_late(struct pci_dev *dev, pm_message_t message)
> +{
> + int retval = 0;
> + int has_pci_pm;
>
> - if (message.event == PM_EVENT_FREEZE ||
> - message.event == PM_EVENT_PRETHAW) {
> - dev_dbg(hcd->self.controller, "--> no state change\n");
> - goto done;
> - }
> + /* We might already be suspended (runtime PM -- not yet written) */
> + if (dev->current_state != PCI_D0)
> + goto done;
>
> - if (!has_pci_pm) {
> - dev_dbg(hcd->self.controller, "--> PCI D0/legacy\n");
> - goto done;
> - }
> + pci_save_state(dev);
> +
> + /* Don't change state if we don't need to */
> + if (message.event == PM_EVENT_FREEZE ||
> + message.event == PM_EVENT_PRETHAW) {
> + dev_dbg(&dev->dev, "--> no state change\n");
> + goto done;
> + }
> +
> + has_pci_pm = pci_find_capability(dev, PCI_CAP_ID_PM);
> + if (!has_pci_pm) {
> + dev_dbg(&dev->dev, "--> PCI D0 legacy\n");
> + } else {
>
> /* NOTE: dev->current_state becomes nonzero only here, and
> * only for devices that support PCI PM. Also, exiting
> @@ -273,35 +289,16 @@ int usb_hcd_pci_suspend(struct pci_dev *dev, pm_message_t message)
> retval = pci_set_power_state(dev, PCI_D3hot);
> suspend_report_result(pci_set_power_state, retval);
> if (retval == 0) {
> - int wake = device_can_wakeup(&hcd->self.root_hub->dev);
> -
> - wake = wake && device_may_wakeup(hcd->self.controller);
> -
> - dev_dbg(hcd->self.controller, "--> PCI D3%s\n",
> - wake ? "/wakeup" : "");
> -
> - /* Ignore these return values. We rely on pci code to
> - * reject requests the hardware can't implement, rather
> - * than coding the same thing.
> - */
> - (void) pci_enable_wake(dev, PCI_D3hot, wake);
> - (void) pci_enable_wake(dev, PCI_D3cold, wake);
> + dev_dbg(&dev->dev, "--> PCI D3\n");
> } else {
> dev_dbg(&dev->dev, "PCI D3 suspend fail, %d\n",
> retval);
> - (void) usb_hcd_pci_resume(dev);
> + pci_restore_state(dev);
> }
> -
> - } else if (hcd->state != HC_STATE_HALT) {
> - dev_dbg(hcd->self.controller, "hcd state %d; not suspended\n",
> - hcd->state);
> - WARN_ON(1);
> - retval = -EINVAL;
> }
>
> -done:
> - if (retval == 0) {
> #ifdef CONFIG_PPC_PMAC
> + if (retval == 0) {
> /* Disable ASIC clocks for USB */
> if (machine_is(powermac)) {
> struct device_node *of_node;
> @@ -311,30 +308,24 @@ done:
> pmac_call_feature(PMAC_FTR_USB_ENABLE,
> of_node, 0, 0);
> }
> -#endif
> }
> +#endif
>
> + done:
> return retval;
> }
> -EXPORT_SYMBOL_GPL(usb_hcd_pci_suspend);
> +EXPORT_SYMBOL_GPL(usb_hcd_pci_suspend_late);
>
> /**
> - * usb_hcd_pci_resume - power management resume of a PCI-based HCD
> + * usb_hcd_pci_resume_early - resume a PCI-based HCD before IRQs are enabled
> * @dev: USB Host Controller being resumed
> *
> - * Store this function in the HCD's struct pci_driver as resume().
> + * Store this function in the HCD's struct pci_driver as .resume_early.
> */
> -int usb_hcd_pci_resume(struct pci_dev *dev)
> +int usb_hcd_pci_resume_early(struct pci_dev *dev)
> {
> - struct usb_hcd *hcd;
> - int retval;
> -
> - hcd = pci_get_drvdata(dev);
> - if (hcd->state != HC_STATE_SUSPENDED) {
> - dev_dbg(hcd->self.controller,
> - "can't resume, not suspended!\n");
> - return 0;
> - }
> + int retval = 0;
> + pci_power_t state = dev->current_state;
>
> #ifdef CONFIG_PPC_PMAC
> /* Reenable ASIC clocks for USB */
> @@ -352,7 +343,7 @@ int usb_hcd_pci_resume(struct pci_dev *dev)
> * 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) {
> + if (state != PCI_D0) {
> #ifdef DEBUG
> int pci_pm;
> u16 pmcr;
> @@ -364,8 +355,7 @@ int usb_hcd_pci_resume(struct pci_dev *dev)
> /* 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);
> + dev_dbg(&dev->dev, "resume from PCI D%d\n", pmcr);
> } else {
> /* Clean: HC lost Vcc power, D0 uninitialized
> * + Vaux may have preserved port and transceiver
> @@ -376,32 +366,55 @@ int usb_hcd_pci_resume(struct pci_dev *dev)
> * + 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);
> + dev_dbg(&dev->dev, "resume from previous PCI D%d\n",
> + state);
> }
> #endif
> - /* yes, ignore these results too... */
> - (void) pci_enable_wake(dev, dev->current_state, 0);
> - (void) pci_enable_wake(dev, PCI_D3cold, 0);
> +
> + retval = pci_set_power_state(dev, PCI_D0);
> } else {
> /* Same basic cases: clean (powered/not), dirty */
> - dev_dbg(hcd->self.controller, "PCI legacy resume\n");
> + dev_dbg(&dev->dev, "PCI legacy resume\n");
> + }
> +
> + if (retval < 0)
> + dev_err(&dev->dev, "can't resume: %d\n", retval);
> + else
> + pci_restore_state(dev);
> +
> + return retval;
> +}
> +EXPORT_SYMBOL_GPL(usb_hcd_pci_resume_early);
> +
> +/**
> + * usb_hcd_pci_resume - power management resume of a PCI-based HCD
> + * @dev: USB Host Controller being resumed
> + *
> + * Store this function in the HCD's struct pci_driver as .resume.
> + */
> +int usb_hcd_pci_resume(struct pci_dev *dev)
> +{
> + struct usb_hcd *hcd;
> + int retval;
> +
> + hcd = pci_get_drvdata(dev);
> + if (hcd->state != HC_STATE_SUSPENDED) {
> + dev_dbg(hcd->self.controller,
> + "can't resume, not suspended!\n");
> + return 0;
> }
>
> - /* 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);
> + dev_err(&dev->dev, "can't re-enable after resume, %d!\n",
> + retval);
> return retval;
> }
> +
> pci_set_master(dev);
> - pci_restore_state(dev);
> +
> + /* yes, ignore this result too... */
> + (void) pci_wake_from_d3(dev, 0);
>
> clear_bit(HCD_FLAG_SAW_IRQ, &hcd->flags);
>
> @@ -413,7 +426,6 @@ int usb_hcd_pci_resume(struct pci_dev *dev)
> usb_hc_died(hcd);
> }
> }
> -
> return retval;
> }
> EXPORT_SYMBOL_GPL(usb_hcd_pci_resume);
> diff --git a/drivers/usb/core/hcd.h b/drivers/usb/core/hcd.h
> index aa5da82..572d2cf 100644
> --- a/drivers/usb/core/hcd.h
> +++ b/drivers/usb/core/hcd.h
> @@ -256,7 +256,9 @@ extern int usb_hcd_pci_probe(struct pci_dev *dev,
> extern void usb_hcd_pci_remove(struct pci_dev *dev);
>
> #ifdef CONFIG_PM
> -extern int usb_hcd_pci_suspend(struct pci_dev *dev, pm_message_t state);
> +extern int usb_hcd_pci_suspend(struct pci_dev *dev, pm_message_t msg);
> +extern int usb_hcd_pci_suspend_late(struct pci_dev *dev, pm_message_t msg);
> +extern int usb_hcd_pci_resume_early(struct pci_dev *dev);
> extern int usb_hcd_pci_resume(struct pci_dev *dev);
> #endif /* CONFIG_PM */
>
> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
> index 36864f9..6af47a0 100644
> --- a/drivers/usb/host/ehci-pci.c
> +++ b/drivers/usb/host/ehci-pci.c
> @@ -428,6 +428,8 @@ static struct pci_driver ehci_pci_driver = {
>
> #ifdef CONFIG_PM
> .suspend = usb_hcd_pci_suspend,
> + .suspend_late = usb_hcd_pci_suspend_late,
> + .resume_early = usb_hcd_pci_resume_early,
> .resume = usb_hcd_pci_resume,
> #endif
> .shutdown = usb_hcd_pci_shutdown,
> diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
> index a9c2ae3..8380cc2 100644
> --- a/drivers/usb/host/ohci-pci.c
> +++ b/drivers/usb/host/ohci-pci.c
> @@ -487,6 +487,8 @@ static struct pci_driver ohci_pci_driver = {
>
> #ifdef CONFIG_PM
> .suspend = usb_hcd_pci_suspend,
> + .suspend_late = usb_hcd_pci_suspend_late,
> + .resume_early = usb_hcd_pci_resume_early,
> .resume = usb_hcd_pci_resume,
> #endif
>
> diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
> index cf5e4cf..4e22106 100644
> --- a/drivers/usb/host/uhci-hcd.c
> +++ b/drivers/usb/host/uhci-hcd.c
> @@ -942,6 +942,8 @@ static struct pci_driver uhci_pci_driver = {
>
> #ifdef CONFIG_PM
> .suspend = usb_hcd_pci_suspend,
> + .suspend_late = usb_hcd_pci_suspend_late,
> + .resume_early = usb_hcd_pci_resume_early,
> .resume = usb_hcd_pci_resume,
> #endif /* PM */
> };
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
