Re: PME via interrupt or SCI mechanism?

Re: PME via interrupt or SCI mechanism?

[Rafael J. Wysocki]

Hi,

Sorry for the delayed response, I was traveling during the last week too.

On Monday, September 12, 2011, Sarah Sharp wrote:
> Hi Rafael,
> 
> As I mentioned at LPC, I have a USB host controller that is failing to
> wakeup from D3 when a new USB device is connected to an external hub.
> The system is in S0 at this point.
> 
> You mentioned that there were two ways for hardware to generate PMEs:
> either through the standard PCI interrupt process, or via an ACPI SCI
> call.
> 
> I think the hardware engineers want Linux to set up the PCI device to
> generate PMEs via an SCI call, but I'm not sure if Linux is.  I've tried
> turning on ACPI debugging (with level and layers both set to 0xffffffff
> so I can see all debugging), and I don't see any output at all from ACPI
> functions like acpi_ev_sci_xrupt_handler when the host controller comes
> out of D3.  (It does come out of D3 if I plug in the device within 10
> seconds of PCI suspend, for whatever reason.)
> 
> Is there a way to tell if SCI is being used by a PCI device to generate
> PMEs?

Yes, there is.  First, if the native PCIe PME is used (which means SCI isn't),
there will be entries like these in /proc/interrupts:

 40:          0          0   PCI-MSI-edge      PCIe PME
 41:          0          0   PCI-MSI-edge      PCIe PME
 42:          0          0   PCI-MSI-edge      PCIe PME

If they are not present, it means that the kernel is _trying_ to use SCI
for PME signaling.  In that case, you can check if the number of ACPI
interrupts in /proc/interrupts is increasing when you try to trigger the
events.

In that case you can use the files under /sys/firmware/acpi/interrupts/
to see what GPEs are activated by the wakeup events.


> You also mentioned that Linux has to choose whether to use standard
> interrupts or an SCI to generate PMEs.  You said Linux asks the BIOS if
> the hardware can use interrupts to generate PMEs, and it always uses
> interrupt-based PME generation if the BIOS says yes.

That's correct.

> Do you know where that code is?

drivers/acpi/pci_root.c:acpi_pci_root_add()

> I'd like to see how the BIOS responds to that call, and perhaps get the
> BIOS guys to fix their response if the hardware is supposed to only use
> SCIs to generate PMEs.

The BIOS should respond by clearing bit 2 (PCIe PME) of the control field
returned from _OSC() invoked for the PCIe Root Complex (meaning that the
kernel is not granted control of the native PCIe feature).

Thanks,
Rafael

Re: PME via interrupt or SCI mechanism?

[Rafael J. Wysocki]

On Thursday, September 22, 2011, Sarah Sharp wrote:
> On Mon, Sep 19, 2011 at 11:43:33PM +0200, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > Sorry for the delayed response, I was traveling during the last week too.
> 
> No worries. :)
> 
> > On Monday, September 12, 2011, Sarah Sharp wrote:
> > > Hi Rafael,
> > > 
> > > As I mentioned at LPC, I have a USB host controller that is failing to
> > > wakeup from D3 when a new USB device is connected to an external hub.
> > > The system is in S0 at this point.
> > > 
> > > You mentioned that there were two ways for hardware to generate PMEs:
> > > either through the standard PCI interrupt process, or via an ACPI SCI
> > > call.
> > > 
> > > I think the hardware engineers want Linux to set up the PCI device to
> > > generate PMEs via an SCI call, but I'm not sure if Linux is.  I've tried
> > > turning on ACPI debugging (with level and layers both set to 0xffffffff
> > > so I can see all debugging), and I don't see any output at all from ACPI
> > > functions like acpi_ev_sci_xrupt_handler when the host controller comes
> > > out of D3.  (It does come out of D3 if I plug in the device within 10
> > > seconds of PCI suspend, for whatever reason.)
> > > 
> > > Is there a way to tell if SCI is being used by a PCI device to generate
> > > PMEs?
> > 
> > Yes, there is.  First, if the native PCIe PME is used (which means SCI isn't),
> > there will be entries like these in /proc/interrupts:
> > 
> >  40:          0          0   PCI-MSI-edge      PCIe PME
> >  41:          0          0   PCI-MSI-edge      PCIe PME
> >  42:          0          0   PCI-MSI-edge      PCIe PME
> > 
> > If they are not present, it means that the kernel is _trying_ to use SCI
> > for PME signaling.  In that case, you can check if the number of ACPI
> > interrupts in /proc/interrupts is increasing when you try to trigger the
> > events.
> 
> Ok, here's what /proc/interrupts shows for my system:
> 
>            CPU0       CPU1       CPU2       CPU3       
>   0:         28          0          0          0  IR-IO-APIC-edge      timer
>   8:          1          0          0          0  IR-IO-APIC-edge      rtc0
>   9:          0          0          0          0  IR-IO-APIC-fasteoi   acpi
>  16:        795          0          0          0  IR-IO-APIC-fasteoi   ehci_hcd:usb1
>  19:         16          0          0          0  IR-IO-APIC-fasteoi   firewire_ohci
>  23:         34          0          0          0  IR-IO-APIC-fasteoi   ehci_hcd:usb2
>  40:          0          0          0          0  DMAR_MSI-edge      dmar0
>  42:       2931          0          0          0  IR-PCI-MSI-edge      eth7
>  43:      13285          0          0          0  IR-PCI-MSI-edge      ahci
>  44:          0          0          0          0  IR-PCI-MSI-edge      xhci_hcd
>  45:        558          0          0          0  IR-PCI-MSI-edge      snd_hda_intel
>  46:        235          0        204          0  IR-PCI-MSI-edge      snd_hda_intel
>  47:        808          0          0          0  IR-PCI-MSI-edge      radeon
> NMI:          0          0          0          0   Non-maskable interrupts
> LOC:      14857      14209      44533      15822   Local timer interrupts
> SPU:          0          0          0          0   Spurious interrupts
> PMI:          0          0          0          0   Performance monitoring interrupts
> IWI:          0          0          0          0   IRQ work interrupts
> RES:      11671      13274       2835       4637   Rescheduling interrupts
> CAL:        210        311        396        470   Function call interrupts
> TLB:        287        242        135        296   TLB shootdowns
> TRM:          0          0          0          0   Thermal event interrupts
> THR:          0          0          0          0   Threshold APIC interrupts
> MCE:          0          0          0          0   Machine check exceptions
> MCP:          5          5          5          5   Machine check polls
> ERR:          0
> MIS:          0
> 
> I don't see any PCIe PME entries, so can I assume that ACPI is trying to use
> SCI?

Yes.

> > In that case you can use the files under /sys/firmware/acpi/interrupts/
> > to see what GPEs are activated by the wakeup events.
> 
> Ok, before I put the host controller into D3 by echoing auto to
> /sys/bus/pci/devices/0000:00:14.0/power/control, catting those files in
> /sys/firmware/acpi/interrupts/ shows all zeros (and some enabled GPEs).
> 
> After I enable runtime PM for the PCI device, wait for it to go into D3,
> and then plug a new USB host controller into an external hub under xHCI,
> I see the count in the file gpe0D increase:
> 
> sarah@talon:/sys/firmware/acpi/interrupts$ cat gpe0D
>   645925   disabled

Well, apparently, the GPE is disabled, although it should be enabled
at this point if it is supposed to be a wakeup GPE for the controller.

> But the host controller is never brought out of D3, and the port status
> change events was never reported.  The dmesg from the run is attached,
> with some additional debugging I added to the PCI and ACPI core.  The
> part where the host controller first goes into D3 is on line 3521.
> 
> I'm starting to think that the ACPI tables for this platform are not
> correct.  The BIOS guys haven't let me know whether wake from D3 is
> actually supported by the BIOS yet (this platform is still under
> development).  The disassembled ACPI tables are attached.

Without looking at the tables at the moment (I'll do that later),
I think that they are missing the information that GPE 0D is a wakeup
GPE for the xHCI device.

> In digging through the ACPI code, I noticed that acpi_bus_get_flags()
> looks for the ACPI methods _PR0 or _PS0 and sets
> device->flags.power_manageable to 1 if either of those methods are
> successfully invoked.  When I deassembled the ACPI tables, I didn't see
> either method for any of the USB host controllers in the system.
> device->flags.power_manageable is checked later when the runtime PM
> system attempts to put the PCI device into a lower state, but it seems
> to be ignored?  Is it supposed to be ignored?

Hmm, not really.  I'll have a look at that later.

> I'm wondering if both native PCIe PME and SCI PME generation are not
> supported by the PCIe device, why does runtime PM put the xHCI host
> controller into D3 when power/control is set to 'auto'?

Because it doesn't require that the devices support remote wakeup to
be put into low power states.  User space may simply want to save energy
by letting the device to go into D3 and it will wake it up later by
writing "on" into power/control, for example.

Thanks,
Rafael

Re: PME via interrupt or SCI mechanism?

[Rafael J. Wysocki]

On Sunday, September 25, 2011, Rafael J. Wysocki wrote:
> On Thursday, September 22, 2011, Sarah Sharp wrote:
> > On Mon, Sep 19, 2011 at 11:43:33PM +0200, Rafael J. Wysocki wrote:
> > > Hi,
> > > 
> > > Sorry for the delayed response, I was traveling during the last week too.
> > 
> > No worries. :)
> > 
> > > On Monday, September 12, 2011, Sarah Sharp wrote:
> > > > Hi Rafael,
> > > > 
> > > > As I mentioned at LPC, I have a USB host controller that is failing to
> > > > wakeup from D3 when a new USB device is connected to an external hub.
> > > > The system is in S0 at this point.
> > > > 
> > > > You mentioned that there were two ways for hardware to generate PMEs:
> > > > either through the standard PCI interrupt process, or via an ACPI SCI
> > > > call.
> > > > 
> > > > I think the hardware engineers want Linux to set up the PCI device to
> > > > generate PMEs via an SCI call, but I'm not sure if Linux is.  I've tried
> > > > turning on ACPI debugging (with level and layers both set to 0xffffffff
> > > > so I can see all debugging), and I don't see any output at all from ACPI
> > > > functions like acpi_ev_sci_xrupt_handler when the host controller comes
> > > > out of D3.  (It does come out of D3 if I plug in the device within 10
> > > > seconds of PCI suspend, for whatever reason.)
> > > > 
> > > > Is there a way to tell if SCI is being used by a PCI device to generate
> > > > PMEs?
> > > 
> > > Yes, there is.  First, if the native PCIe PME is used (which means SCI isn't),
> > > there will be entries like these in /proc/interrupts:
> > > 
> > >  40:          0          0   PCI-MSI-edge      PCIe PME
> > >  41:          0          0   PCI-MSI-edge      PCIe PME
> > >  42:          0          0   PCI-MSI-edge      PCIe PME
> > > 
> > > If they are not present, it means that the kernel is _trying_ to use SCI
> > > for PME signaling.  In that case, you can check if the number of ACPI
> > > interrupts in /proc/interrupts is increasing when you try to trigger the
> > > events.
> > 
> > Ok, here's what /proc/interrupts shows for my system:
> > 
> >            CPU0       CPU1       CPU2       CPU3       
> >   0:         28          0          0          0  IR-IO-APIC-edge      timer
> >   8:          1          0          0          0  IR-IO-APIC-edge      rtc0
> >   9:          0          0          0          0  IR-IO-APIC-fasteoi   acpi
> >  16:        795          0          0          0  IR-IO-APIC-fasteoi   ehci_hcd:usb1
> >  19:         16          0          0          0  IR-IO-APIC-fasteoi   firewire_ohci
> >  23:         34          0          0          0  IR-IO-APIC-fasteoi   ehci_hcd:usb2
> >  40:          0          0          0          0  DMAR_MSI-edge      dmar0
> >  42:       2931          0          0          0  IR-PCI-MSI-edge      eth7
> >  43:      13285          0          0          0  IR-PCI-MSI-edge      ahci
> >  44:          0          0          0          0  IR-PCI-MSI-edge      xhci_hcd
> >  45:        558          0          0          0  IR-PCI-MSI-edge      snd_hda_intel
> >  46:        235          0        204          0  IR-PCI-MSI-edge      snd_hda_intel
> >  47:        808          0          0          0  IR-PCI-MSI-edge      radeon
> > NMI:          0          0          0          0   Non-maskable interrupts
> > LOC:      14857      14209      44533      15822   Local timer interrupts
> > SPU:          0          0          0          0   Spurious interrupts
> > PMI:          0          0          0          0   Performance monitoring interrupts
> > IWI:          0          0          0          0   IRQ work interrupts
> > RES:      11671      13274       2835       4637   Rescheduling interrupts
> > CAL:        210        311        396        470   Function call interrupts
> > TLB:        287        242        135        296   TLB shootdowns
> > TRM:          0          0          0          0   Thermal event interrupts
> > THR:          0          0          0          0   Threshold APIC interrupts
> > MCE:          0          0          0          0   Machine check exceptions
> > MCP:          5          5          5          5   Machine check polls
> > ERR:          0
> > MIS:          0
> > 
> > I don't see any PCIe PME entries, so can I assume that ACPI is trying to use
> > SCI?
> 
> Yes.
> 
> > > In that case you can use the files under /sys/firmware/acpi/interrupts/
> > > to see what GPEs are activated by the wakeup events.
> > 
> > Ok, before I put the host controller into D3 by echoing auto to
> > /sys/bus/pci/devices/0000:00:14.0/power/control, catting those files in
> > /sys/firmware/acpi/interrupts/ shows all zeros (and some enabled GPEs).
> > 
> > After I enable runtime PM for the PCI device, wait for it to go into D3,
> > and then plug a new USB host controller into an external hub under xHCI,
> > I see the count in the file gpe0D increase:
> > 
> > sarah@talon:/sys/firmware/acpi/interrupts$ cat gpe0D
> >   645925   disabled
> 
> Well, apparently, the GPE is disabled, although it should be enabled
> at this point if it is supposed to be a wakeup GPE for the controller.
> 
> > But the host controller is never brought out of D3, and the port status
> > change events was never reported.  The dmesg from the run is attached,
> > with some additional debugging I added to the PCI and ACPI core.  The
> > part where the host controller first goes into D3 is on line 3521.
> > 
> > I'm starting to think that the ACPI tables for this platform are not
> > correct.  The BIOS guys haven't let me know whether wake from D3 is
> > actually supported by the BIOS yet (this platform is still under
> > development).  The disassembled ACPI tables are attached.
> 
> Without looking at the tables at the moment (I'll do that later),
> I think that they are missing the information that GPE 0D is a wakeup
> GPE for the xHCI device.

The DSDT appears to contain that information, so I'm not sure what's
going on.  Perhaps you can put a debug printk into acpi_dev_run_wake()
to see if that function is called for the xHCI controllers?

> > In digging through the ACPI code, I noticed that acpi_bus_get_flags()
> > looks for the ACPI methods _PR0 or _PS0 and sets
> > device->flags.power_manageable to 1 if either of those methods are
> > successfully invoked.  When I deassembled the ACPI tables, I didn't see
> > either method for any of the USB host controllers in the system.

However, the power_manageable flag only indicates that the device can
be put into low-power states through ACPI methods, it shouldn't have
any effect on the wakeup settings.

> > device->flags.power_manageable is checked later when the runtime PM
> > system attempts to put the PCI device into a lower state, but it seems
> > to be ignored?  Is it supposed to be ignored?
> 
> Hmm, not really.  I'll have a look at that later.

It is used to decide whether or not to call __acpi_bus_set_power(), AFAICS.
If it is not set, this function is not called, which is OK.  Still,
devices for which it is not set may be put into low-power states and may
generate wakeup signals.

For many PCI devices there are two possible power management interfaces,
the native one and the ACPI-based one.  All of the modern devices support
the native power management interface, so they can be put into low-power
states even if the ACPI-based interface is missing for them (which is the
case for your USB controllers).  For those devices, if the ACPI-based
interface is not present, we simply use the native one only.

As far as wakeup is concerned, we should enable them to generate PME
using the native interface and in addition to it we should use ACPI to
enable the wakeup GPEs that are supposed to be triggered in response to
the PME signals.

This apparently doesn't work correctly on your system and we need to figure
out why.

Thanks,
Rafael

Re: PME via interrupt or SCI mechanism?

[Sarah Sharp]

[-- Attachment #1: Type: text/plain, Size: 3753 bytes --]

On Tue, Sep 27, 2011 at 12:20:26AM +0200, Rafael J. Wysocki wrote:
> On Sunday, September 25, 2011, Rafael J. Wysocki wrote:
> > On Thursday, September 22, 2011, Sarah Sharp wrote:
> > > On Mon, Sep 19, 2011 at 11:43:33PM +0200, Rafael J. Wysocki wrote:
> > Without looking at the tables at the moment (I'll do that later),
> > I think that they are missing the information that GPE 0D is a wakeup
> > GPE for the xHCI device.
> 
> The DSDT appears to contain that information, so I'm not sure what's
> going on.  Perhaps you can put a debug printk into acpi_dev_run_wake()
> to see if that function is called for the xHCI controllers?

I put a printk in acpi_dev_run_wake(), and that shows up in the original
dmesg I sent:

Sep 22 10:47:09 talon kernel: [ 2026.211933] xhci_hcd 0000:00:14.0: acpi_pci_run_wake - enable dev wake
Sep 22 10:47:09 talon kernel: [ 2026.211936] acpi device:34: acpi_dev_run_wake - enable dev wake
Sep 22 10:47:09 talon kernel: [ 2026.211955] acpi device:34: acpi_dev_run_wake - return -19

The patch I've been using for adding debugging to the PCI and ACPI code
is attached.  I've been adding printks willy-nilly to try to understand
what's going on.

> > > In digging through the ACPI code, I noticed that acpi_bus_get_flags()
> > > looks for the ACPI methods _PR0 or _PS0 and sets
> > > device->flags.power_manageable to 1 if either of those methods are
> > > successfully invoked.  When I deassembled the ACPI tables, I didn't see
> > > either method for any of the USB host controllers in the system.
> 
> However, the power_manageable flag only indicates that the device can
> be put into low-power states through ACPI methods, it shouldn't have
> any effect on the wakeup settings.

Ok, good to know.

> > > device->flags.power_manageable is checked later when the runtime PM
> > > system attempts to put the PCI device into a lower state, but it seems
> > > to be ignored?  Is it supposed to be ignored?
> > 
> > Hmm, not really.  I'll have a look at that later.
> 
> It is used to decide whether or not to call __acpi_bus_set_power(), AFAICS.
> If it is not set, this function is not called, which is OK.  Still,
> devices for which it is not set may be put into low-power states and may
> generate wakeup signals.
> 
> For many PCI devices there are two possible power management interfaces,
> the native one and the ACPI-based one.  All of the modern devices support
> the native power management interface, so they can be put into low-power
> states even if the ACPI-based interface is missing for them (which is the
> case for your USB controllers).  For those devices, if the ACPI-based
> interface is not present, we simply use the native one only.

So if I understand you correctly, the kernel can use the native PCI power
management interface to put the PCI device into a lower power state...

> As far as wakeup is concerned, we should enable them to generate PME
> using the native interface and in addition to it we should use ACPI to
> enable the wakeup GPEs that are supposed to be triggered in response to
> the PME signals.

...but the kernel ACPI core will still be watching for the SCI that the
firmware generates on a PME wakeup?  Correct?

Is the native PCI power management interface you're referring to
implemented through the PCIe PME driver?  Because this particular host
controller is a PCIe device, not a PCI device.  I added some debug
statements to drivers/pci/pcie/pme.c, but the dmesg shows that none of
them got called.

> This apparently doesn't work correctly on your system and we need to figure
> out why.

Ok.  Do you have suggestions for any tests I can run or debugging
statements to add?  I'm really not an ACPI expert and I've been
basically fumbling in the dark.

Sarah Sharp

[-- Attachment #2: pci-acpi-debugging.patch --]
[-- Type: text/x-diff, Size: 24357 bytes --]

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 437ddbf..a8c7fad 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -370,20 +370,23 @@ int acpi_bus_update_power(acpi_handle handle, int *state_p)
 }
 EXPORT_SYMBOL_GPL(acpi_bus_update_power);
 
 
 bool acpi_bus_power_manageable(acpi_handle handle)
 {
 	struct acpi_device *device;
 	int result;
 
 	result = acpi_bus_get_device(handle, &device);
+	printk(KERN_DEBUG "%s - result %u, device->flags.power_manageable %u\n",
+			__func__, result,
+			result ? 0 : device->flags.power_manageable);
 	return result ? false : device->flags.power_manageable;
 }
 
 EXPORT_SYMBOL(acpi_bus_power_manageable);
 
 bool acpi_bus_can_wakeup(acpi_handle handle)
 {
 	struct acpi_device *device;
 	int result;
 
diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
index 2ef0409..94173e0 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -63,20 +63,21 @@ static int acpi_pci_bind(struct acpi_device *device)
 {
 	acpi_status status;
 	acpi_handle handle;
 	struct pci_bus *bus;
 	struct pci_dev *dev;
 
 	dev = acpi_get_pci_dev(device->handle);
 	if (!dev)
 		return 0;
 
+	dev_dbg(&dev->dev, "%s\n", __func__);
 	pci_acpi_add_pm_notifier(device, dev);
 	if (device->wakeup.flags.run_wake)
 		device_set_run_wake(&dev->dev, true);
 
 	/*
 	 * Install the 'bind' function to facilitate callbacks for
 	 * children of the P2P bridge.
 	 */
 	if (dev->subordinate) {
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
index 7f9eba9..2acdf70 100644
--- a/drivers/acpi/pci_irq.c
+++ b/drivers/acpi/pci_irq.c
@@ -416,20 +416,21 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
 {
 	struct acpi_prt_entry *entry;
 	int gsi;
 	u8 pin;
 	int triggering = ACPI_LEVEL_SENSITIVE;
 	int polarity = ACPI_ACTIVE_LOW;
 	char *link = NULL;
 	char link_desc[16];
 	int rc;
 
+	dev_dbg(&dev->dev, "%s\n", __func__);
 	pin = dev->pin;
 	if (!pin) {
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				  "No interrupt pin configured for device %s\n",
 				  pci_name(dev)));
 		return 0;
 	}
 
 	entry = acpi_pci_irq_lookup(dev, pin);
 	if (!entry) {
@@ -499,20 +500,21 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
 void __attribute__ ((weak)) acpi_unregister_gsi(u32 i)
 {
 }
 
 void acpi_pci_irq_disable(struct pci_dev *dev)
 {
 	struct acpi_prt_entry *entry;
 	int gsi;
 	u8 pin;
 
+	dev_dbg(&dev->dev, "%s\n", __func__);
 	pin = dev->pin;
 	if (!pin)
 		return;
 
 	entry = acpi_pci_irq_lookup(dev, pin);
 	if (!entry)
 		return;
 
 	if (entry->link)
 		gsi = acpi_pci_link_free_irq(entry->link);
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 2672c79..b508392 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -216,35 +216,37 @@ static acpi_status acpi_pci_run_osc(acpi_handle handle,
 				    const u32 *capbuf, u32 *retval)
 {
 	struct acpi_osc_context context = {
 		.uuid_str = pci_osc_uuid_str,
 		.rev = 1,
 		.cap.length = 12,
 		.cap.pointer = (void *)capbuf,
 	};
 	acpi_status status;
 
+	printk(KERN_DEBUG "%s\n", __func__);
 	status = acpi_run_osc(handle, &context);
 	if (ACPI_SUCCESS(status)) {
 		*retval = *((u32 *)(context.ret.pointer + 8));
 		kfree(context.ret.pointer);
 	}
 	return status;
 }
 
 static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
 					u32 support,
 					u32 *control)
 {
 	acpi_status status;
 	u32 result, capbuf[3];
 
+	printk(KERN_DEBUG "%s\n", __func__);
 	support &= OSC_PCI_SUPPORT_MASKS;
 	support |= root->osc_support_set;
 
 	capbuf[OSC_QUERY_TYPE] = OSC_QUERY_ENABLE;
 	capbuf[OSC_SUPPORT_TYPE] = support;
 	if (control) {
 		*control &= OSC_PCI_CONTROL_MASKS;
 		capbuf[OSC_CONTROL_TYPE] = *control | root->osc_control_set;
 	} else {
 		/* Run _OSC query for all possible controls. */
@@ -258,20 +260,21 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
 			*control = result;
 	}
 	return status;
 }
 
 static acpi_status acpi_pci_osc_support(struct acpi_pci_root *root, u32 flags)
 {
 	acpi_status status;
 	acpi_handle tmp;
 
+	printk(KERN_DEBUG "%s\n", __func__);
 	status = acpi_get_handle(root->device->handle, "_OSC", &tmp);
 	if (ACPI_FAILURE(status))
 		return status;
 	mutex_lock(&osc_lock);
 	status = acpi_pci_query_osc(root, flags, NULL);
 	mutex_unlock(&osc_lock);
 	return status;
 }
 
 struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
@@ -393,20 +396,21 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
  * _OSC bits the BIOS has granted control of, but its contents are meaningless
  * on failure.
  **/
 acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
 {
 	struct acpi_pci_root *root;
 	acpi_status status;
 	u32 ctrl, capbuf[3];
 	acpi_handle tmp;
 
+	printk(KERN_DEBUG "%s\n", __func__);
 	if (!mask)
 		return AE_BAD_PARAMETER;
 
 	ctrl = *mask & OSC_PCI_CONTROL_MASKS;
 	if ((ctrl & req) != req)
 		return AE_TYPE;
 
 	root = acpi_pci_find_root(handle);
 	if (!root)
 		return AE_NOT_EXIST;
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 449c556..e3e5f57 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -937,29 +937,37 @@ static int acpi_bus_get_flags(struct acpi_device *device)
 		if (ACPI_SUCCESS(status))
 			device->flags.ejectable = 1;
 	}
 
 	/* Presence of _LCK indicates 'lockable' */
 	status = acpi_get_handle(device->handle, "_LCK", &temp);
 	if (ACPI_SUCCESS(status))
 		device->flags.lockable = 1;
 
 	/* Power resources cannot be power manageable. */
-	if (device->device_type == ACPI_BUS_TYPE_POWER)
+	if (device->device_type == ACPI_BUS_TYPE_POWER) {
+		printk(KERN_DEBUG "%s - power resources are not "
+				"power manageable\n", __func__);
 		return 0;
+	}
 
 	/* Presence of _PS0|_PR0 indicates 'power manageable' */
 	status = acpi_get_handle(device->handle, "_PS0", &temp);
-	if (ACPI_FAILURE(status))
+	if (ACPI_FAILURE(status)) {
+		printk(KERN_DEBUG "%s no _PS0\n", __func__);
 		status = acpi_get_handle(device->handle, "_PR0", &temp);
-	if (ACPI_SUCCESS(status))
+	}
+	if (ACPI_SUCCESS(status)) {
+		printk(KERN_DEBUG "%s _PR0\n", __func__);
 		device->flags.power_manageable = 1;
+	}
+	printk(KERN_DEBUG "%s no _PR0\n", __func__);
 
 	/* TBD: Performance management */
 
 	return 0;
 }
 
 static void acpi_device_get_busid(struct acpi_device *device)
 {
 	char bus_id[5] = { '?', 0 };
 	struct acpi_buffer buffer = { sizeof(bus_id), bus_id };
@@ -1227,20 +1235,21 @@ static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
 
 static int acpi_add_single_object(struct acpi_device **child,
 				  acpi_handle handle, int type,
 				  unsigned long long sta,
 				  struct acpi_bus_ops *ops)
 {
 	int result;
 	struct acpi_device *device;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 
+	printk(KERN_DEBUG "%s\n", __func__);
 	device = kzalloc(sizeof(struct acpi_device), GFP_KERNEL);
 	if (!device) {
 		printk(KERN_ERR PREFIX "Memory allocation error\n");
 		return -ENOMEM;
 	}
 
 	INIT_LIST_HEAD(&device->pnp.ids);
 	device->device_type = type;
 	device->handle = handle;
 	device->parent = acpi_bus_get_parent(handle);
@@ -1325,20 +1334,21 @@ end:
 			  ACPI_STA_DEVICE_UI      | ACPI_STA_DEVICE_FUNCTIONING)
 
 static void acpi_bus_add_power_resource(acpi_handle handle)
 {
 	struct acpi_bus_ops ops = {
 		.acpi_op_add = 1,
 		.acpi_op_start = 1,
 	};
 	struct acpi_device *device = NULL;
 
+	printk(KERN_DEBUG "%s\n", __func__);
 	acpi_bus_get_device(handle, &device);
 	if (!device)
 		acpi_add_single_object(&device, handle, ACPI_BUS_TYPE_POWER,
 					ACPI_STA_DEFAULT, &ops);
 }
 
 static int acpi_bus_type_and_status(acpi_handle handle, int *type,
 				    unsigned long long *sta)
 {
 	acpi_status status;
@@ -1380,20 +1390,21 @@ static int acpi_bus_type_and_status(acpi_handle handle, int *type,
 static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl,
 				      void *context, void **return_value)
 {
 	struct acpi_bus_ops *ops = context;
 	int type;
 	unsigned long long sta;
 	struct acpi_device *device;
 	acpi_status status;
 	int result;
 
+	printk(KERN_DEBUG "%s\n", __func__);
 	result = acpi_bus_type_and_status(handle, &type, &sta);
 	if (result)
 		return AE_OK;
 
 	if (!(sta & ACPI_STA_DEVICE_PRESENT) &&
 	    !(sta & ACPI_STA_DEVICE_FUNCTIONING)) {
 		struct acpi_device_wakeup wakeup;
 		acpi_handle temp;
 
 		status = acpi_get_handle(handle, "_PRW", &temp);
@@ -1548,20 +1559,21 @@ EXPORT_SYMBOL_GPL(acpi_bus_trim);
 static int acpi_bus_scan_fixed(void)
 {
 	int result = 0;
 	struct acpi_device *device = NULL;
 	struct acpi_bus_ops ops;
 
 	memset(&ops, 0, sizeof(ops));
 	ops.acpi_op_add = 1;
 	ops.acpi_op_start = 1;
 
+	printk(KERN_DEBUG "%s\n", __func__);
 	/*
 	 * Enumerate all fixed-feature devices.
 	 */
 	if ((acpi_gbl_FADT.flags & ACPI_FADT_POWER_BUTTON) == 0) {
 		result = acpi_add_single_object(&device, NULL,
 						ACPI_BUS_TYPE_POWER_BUTTON,
 						ACPI_STA_DEFAULT,
 						&ops);
 	}
 
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index d36f41e..e7885fe 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -24,35 +24,39 @@ static DEFINE_MUTEX(pci_acpi_pm_notify_mtx);
 /**
  * pci_acpi_wake_bus - Wake-up notification handler for root buses.
  * @handle: ACPI handle of a device the notification is for.
  * @event: Type of the signaled event.
  * @context: PCI root bus to wake up devices on.
  */
 static void pci_acpi_wake_bus(acpi_handle handle, u32 event, void *context)
 {
 	struct pci_bus *pci_bus = context;
 
-	if (event == ACPI_NOTIFY_DEVICE_WAKE && pci_bus)
+	printk(KERN_DEBUG "%s\n", __func__);
+	if (event == ACPI_NOTIFY_DEVICE_WAKE && pci_bus) {
+		dev_dbg(&pci_bus->self->dev, "%s\n", __func__);
 		pci_pme_wakeup_bus(pci_bus);
+	}
 }
 
 /**
  * pci_acpi_wake_dev - Wake-up notification handler for PCI devices.
  * @handle: ACPI handle of a device the notification is for.
  * @event: Type of the signaled event.
  * @context: PCI device object to wake up.
  */
 static void pci_acpi_wake_dev(acpi_handle handle, u32 event, void *context)
 {
 	struct pci_dev *pci_dev = context;
 
 	if (event == ACPI_NOTIFY_DEVICE_WAKE && pci_dev) {
+		dev_dbg(&pci_dev->dev, "%s\n", __func__);
 		pci_wakeup_event(pci_dev);
 		pci_check_pme_status(pci_dev);
 		pm_runtime_resume(&pci_dev->dev);
 		if (pci_dev->subordinate)
 			pci_pme_wakeup_bus(pci_dev->subordinate);
 	}
 }
 
 /**
  * add_pm_notifier - Register PM notifier for given ACPI device.
@@ -136,20 +140,21 @@ acpi_status pci_acpi_remove_bus_pm_notifier(struct acpi_device *dev)
 }
 
 /**
  * pci_acpi_add_pm_notifier - Register PM notifier for given PCI device.
  * @dev: ACPI device to add the notifier for.
  * @pci_dev: PCI device to check for the PME status if an event is signaled.
  */
 acpi_status pci_acpi_add_pm_notifier(struct acpi_device *dev,
 				     struct pci_dev *pci_dev)
 {
+	dev_dbg(&pci_dev->dev, "%s\n", __func__);
 	return add_pm_notifier(dev, pci_acpi_wake_dev, pci_dev);
 }
 
 /**
  * pci_acpi_remove_pm_notifier - Unregister PCI device PM notifier.
  * @dev: ACPI device to remove the notifier from.
  */
 acpi_status pci_acpi_remove_pm_notifier(struct acpi_device *dev)
 {
 	return remove_pm_notifier(dev, pci_acpi_wake_dev);
@@ -198,20 +203,28 @@ static pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
 	case ACPI_STATE_D3_COLD:
 		return PCI_D3cold;
 	}
 	return PCI_POWER_ERROR;
 }
 
 static bool acpi_pci_power_manageable(struct pci_dev *dev)
 {
 	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
 
+	if (handle) {
+		dev_dbg(&dev->dev, "%s - dev is%s power manageable\n",
+				__func__,
+				 acpi_bus_power_manageable(handle) ?
+				 "" : " not");
+	} else
+		dev_dbg(&dev->dev, "%s - dev is NOT power manageable\n",
+				__func__);
 	return handle ? acpi_bus_power_manageable(handle) : false;
 }
 
 static int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 {
 	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
 	acpi_handle tmp;
 	static const u8 state_conv[] = {
 		[PCI_D0] = ACPI_STATE_D0,
 		[PCI_D1] = ACPI_STATE_D1,
@@ -238,38 +251,48 @@ static int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 		dev_printk(KERN_INFO, &dev->dev,
 				"power state changed by ACPI to D%d\n", state);
 
 	return error;
 }
 
 static bool acpi_pci_can_wakeup(struct pci_dev *dev)
 {
 	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
 
+	if (handle)
+		dev_dbg(&dev->dev, "%s - dev can%s wakeup\n",
+				__func__,
+				 acpi_bus_can_wakeup(handle) ?
+				 "" : " not");
+	else
+		dev_dbg(&dev->dev, "%s - dev is can wakeup\n",
+				__func__);
 	return handle ? acpi_bus_can_wakeup(handle) : false;
 }
 
 static void acpi_pci_propagate_wakeup_enable(struct pci_bus *bus, bool enable)
 {
 	while (bus->parent) {
 		if (!acpi_pm_device_sleep_wake(&bus->self->dev, enable))
 			return;
 		bus = bus->parent;
 	}
 
 	/* We have reached the root bus. */
 	if (bus->bridge)
 		acpi_pm_device_sleep_wake(bus->bridge, enable);
 }
 
 static int acpi_pci_sleep_wake(struct pci_dev *dev, bool enable)
 {
+	dev_dbg(&dev->dev, "%s - %s dev wake\n",
+			__func__, enable ? "enable" : "disable");
 	if (acpi_pci_can_wakeup(dev))
 		return acpi_pm_device_sleep_wake(&dev->dev, enable);
 
 	acpi_pci_propagate_wakeup_enable(dev->bus, enable);
 	return 0;
 }
 
 /**
  * acpi_dev_run_wake - Enable/disable wake-up for given device.
  * @phys_dev: Device to enable/disable the platform to wake-up the system for.
@@ -287,28 +310,31 @@ static int acpi_dev_run_wake(struct device *phys_dev, bool enable)
 	if (!device_run_wake(phys_dev))
 		return -EINVAL;
 
 	handle = DEVICE_ACPI_HANDLE(phys_dev);
 	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &dev))) {
 		dev_dbg(phys_dev, "ACPI handle has no context in %s!\n",
 			__func__);
 		return -ENODEV;
 	}
 
+	dev_dbg(&dev->dev, "%s - %s dev wake\n",
+			__func__, enable ? "enable" : "disable");
 	if (enable) {
 		acpi_enable_wakeup_device_power(dev, ACPI_STATE_S0);
 		acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number);
 	} else {
 		acpi_disable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number);
 		acpi_disable_wakeup_device_power(dev);
 	}
 
+	dev_dbg(&dev->dev, "%s - return %i\n", __func__, error);
 	return error;
 }
 
 static void acpi_pci_propagate_run_wake(struct pci_bus *bus, bool enable)
 {
 	while (bus->parent) {
 		struct pci_dev *bridge = bus->self;
 
 		if (bridge->pme_interrupt)
 			return;
@@ -317,20 +343,22 @@ static void acpi_pci_propagate_run_wake(struct pci_bus *bus, bool enable)
 		bus = bus->parent;
 	}
 
 	/* We have reached the root bus. */
 	if (bus->bridge)
 		acpi_dev_run_wake(bus->bridge, enable);
 }
 
 static int acpi_pci_run_wake(struct pci_dev *dev, bool enable)
 {
+	dev_dbg(&dev->dev, "%s - %s dev wake\n",
+			__func__, enable ? "enable" : "disable");
 	if (dev->pme_interrupt)
 		return 0;
 
 	if (!acpi_dev_run_wake(&dev->dev, enable))
 		return 0;
 
 	acpi_pci_propagate_run_wake(dev->bus, enable);
 	return 0;
 }
 
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 08a95b3..2fcfa42 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -536,20 +536,22 @@ static inline int platform_pci_run_wake(struct pci_dev *dev, bool enable)
  * -EIO if device does not support PCI PM or its PM capabilities register has a
  * wrong version, or device doesn't support the requested state.
  * 0 if device already is in the requested state.
  * 0 if device's power state has been successfully changed.
  */
 static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state)
 {
 	u16 pmcsr;
 	bool need_restore = false;
 
+	dev_dbg(&dev->dev, "%s from %d to %d\n", __func__,
+			dev->current_state, state);
 	/* Check if we're already there */
 	if (dev->current_state == state)
 		return 0;
 
 	if (!dev->pm_cap)
 		return -EIO;
 
 	if (state < PCI_D0 || state > PCI_D3hot)
 		return -EINVAL;
 
@@ -652,24 +654,26 @@ void pci_update_current_state(struct pci_dev *dev, pci_power_t state)
 /**
  * pci_platform_power_transition - Use platform to change device power state
  * @dev: PCI device to handle.
  * @state: State to put the device into.
  */
 static int pci_platform_power_transition(struct pci_dev *dev, pci_power_t state)
 {
 	int error;
 
 	if (platform_pci_power_manageable(dev)) {
+		dev_dbg(&dev->dev, "%s - is power managable\n", __func__);
 		error = platform_pci_set_power_state(dev, state);
 		if (!error)
 			pci_update_current_state(dev, state);
 	} else {
+		dev_dbg(&dev->dev, "%s - is NOT power managable\n", __func__);
 		error = -ENODEV;
 		/* Fall back to PCI_D0 if native PM is not supported */
 		if (!dev->pm_cap)
 			dev->current_state = PCI_D0;
 	}
 
 	return error;
 }
 
 /**
@@ -726,22 +730,24 @@ int pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 		 * If the device or the parent bridge do not support PCI PM,
 		 * ignore the request if we're doing anything other than putting
 		 * it into D0 (which would only happen on boot).
 		 */
 		return 0;
 
 	__pci_start_power_transition(dev, state);
 
 	/* This device is quirked not to be put into D3, so
 	   don't put it in D3 */
-	if (state == PCI_D3hot && (dev->dev_flags & PCI_DEV_FLAGS_NO_D3))
+	if (state == PCI_D3hot && (dev->dev_flags & PCI_DEV_FLAGS_NO_D3)) {
+		dev_dbg(&dev->dev, "%s - quirk: don't put in D3.\n", __func__);
 		return 0;
+	}
 
 	error = pci_raw_set_power_state(dev, state);
 
 	if (!__pci_complete_power_transition(dev, state))
 		error = 0;
 	/*
 	 * When aspm_policy is "powersave" this call ensures
 	 * that ASPM is configured.
 	 */
 	if (!error && dev->bus->self)
diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c
index 0057344..22c2f68 100644
--- a/drivers/pci/pcie/pme.c
+++ b/drivers/pci/pcie/pme.c
@@ -56,24 +56,27 @@ struct pcie_pme_service_data {
  * @enable: Enable or disable the interrupt.
  */
 void pcie_pme_interrupt_enable(struct pci_dev *dev, bool enable)
 {
 	int rtctl_pos;
 	u16 rtctl;
 
 	rtctl_pos = pci_pcie_cap(dev) + PCI_EXP_RTCTL;
 
 	pci_read_config_word(dev, rtctl_pos, &rtctl);
-	if (enable)
+	if (enable) {
 		rtctl |= PCI_EXP_RTCTL_PMEIE;
-	else
+		dev_dbg(&dev->dev, "%s - Enabling PME interrupt.\n", __func__);
+	} else {
 		rtctl &= ~PCI_EXP_RTCTL_PMEIE;
+		dev_dbg(&dev->dev, "%s - Disabling PME interrupt.\n", __func__);
+	}
 	pci_write_config_word(dev, rtctl_pos, rtctl);
 }
 
 /**
  * pcie_pme_walk_bus - Scan a PCI bus for devices asserting PME#.
  * @bus: PCI bus to scan.
  *
  * Scan given PCI bus and all buses under it for devices asserting PME#.
  */
 static bool pcie_pme_walk_bus(struct pci_bus *bus)
diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
index ce22f4a..5ecba68 100644
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -431,20 +431,21 @@ static int suspend_common(struct device *dev, bool do_wakeup)
 	pci_disable_device(pci_dev);
 	return retval;
 }
 
 static int resume_common(struct device *dev, int event)
 {
 	struct pci_dev		*pci_dev = to_pci_dev(dev);
 	struct usb_hcd		*hcd = pci_get_drvdata(pci_dev);
 	int			retval;
 
+	dev_dbg(dev, "%s\n", __func__);
 	if (HCD_RH_RUNNING(hcd) ||
 			(hcd->shared_hcd &&
 			 HCD_RH_RUNNING(hcd->shared_hcd))) {
 		dev_dbg(dev, "can't resume, not suspended!\n");
 		return 0;
 	}
 
 	retval = pci_enable_device(pci_dev);
 	if (retval < 0) {
 		dev_err(dev, "can't re-enable after resume, %d!\n", retval);
@@ -479,20 +480,21 @@ static int hcd_pci_suspend(struct device *dev)
 {
 	return suspend_common(dev, device_may_wakeup(dev));
 }
 
 static int hcd_pci_suspend_noirq(struct device *dev)
 {
 	struct pci_dev		*pci_dev = to_pci_dev(dev);
 	struct usb_hcd		*hcd = pci_get_drvdata(pci_dev);
 	int			retval;
 
+	dev_dbg(&pci_dev->dev, "%s\n", __func__);
 	retval = check_root_hub_suspended(dev);
 	if (retval)
 		return retval;
 
 	pci_save_state(pci_dev);
 
 	/* If the root hub is dead rather than suspended, disallow remote
 	 * wakeup.  usb_hc_died() should ensure that both hosts are marked as
 	 * dying, so we only need to check the primary roothub.
 	 */
@@ -518,27 +520,29 @@ static int hcd_pci_suspend_noirq(struct device *dev)
 	powermac_set_asic(pci_dev, 0);
 	return retval;
 }
 
 static int hcd_pci_resume_noirq(struct device *dev)
 {
 	struct pci_dev		*pci_dev = to_pci_dev(dev);
 
 	powermac_set_asic(pci_dev, 1);
 
+	dev_dbg(dev, "%s\n", __func__);
 	/* Go back to D0 and disable remote wakeup */
 	pci_back_from_sleep(pci_dev);
 	return 0;
 }
 
 static int hcd_pci_resume(struct device *dev)
 {
+	dev_dbg(dev, "%s\n", __func__);
 	return resume_common(dev, PM_EVENT_RESUME);
 }
 
 static int hcd_pci_restore(struct device *dev)
 {
 	return resume_common(dev, PM_EVENT_RESTORE);
 }
 
 #else
 
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 723f823..383b248 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -786,20 +786,21 @@ int xhci_hub_status_data(struct usb_hcd *hcd, char *buf)
 #ifdef CONFIG_PM
 
 int xhci_bus_suspend(struct usb_hcd *hcd)
 {
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	int max_ports, port_index;
 	__le32 __iomem **port_array;
 	struct xhci_bus_state *bus_state;
 	unsigned long flags;
 
+	xhci_dbg(xhci, "%s\n", __func__);
 	max_ports = xhci_get_ports(hcd, &port_array);
 	bus_state = &xhci->bus_state[hcd_index(hcd)];
 
 	spin_lock_irqsave(&xhci->lock, flags);
 
 	if (hcd->self.root_hub->do_remote_wakeup) {
 		port_index = max_ports;
 		while (port_index--) {
 			if (bus_state->resume_done[port_index] != 0) {
 				spin_unlock_irqrestore(&xhci->lock, flags);
@@ -871,20 +872,21 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
 
 int xhci_bus_resume(struct usb_hcd *hcd)
 {
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	int max_ports, port_index;
 	__le32 __iomem **port_array;
 	struct xhci_bus_state *bus_state;
 	u32 temp;
 	unsigned long flags;
 
+	xhci_dbg(xhci, "%s\n", __func__);
 	max_ports = xhci_get_ports(hcd, &port_array);
 	bus_state = &xhci->bus_state[hcd_index(hcd)];
 
 	if (time_before(jiffies, bus_state->next_statechange))
 		msleep(5);
 
 	spin_lock_irqsave(&xhci->lock, flags);
 	if (!HCD_HW_ACCESSIBLE(hcd)) {
 		spin_unlock_irqrestore(&xhci->lock, flags);
 		return -ESHUTDOWN;
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index cb16de2..fad2691 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -247,20 +247,21 @@ static void xhci_pci_remove(struct pci_dev *dev)
 	usb_hcd_pci_remove(dev);
 	kfree(xhci);
 }
 
 #ifdef CONFIG_PM
 static int xhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
 {
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	int	retval = 0;
 
+	xhci_dbg(xhci, "%s\n", __func__);
 	if (hcd->state != HC_STATE_SUSPENDED ||
 			xhci->shared_hcd->state != HC_STATE_SUSPENDED)
 		return -EINVAL;
 
 	retval = xhci_suspend(xhci);
 
 	return retval;
 }
 
 static int xhci_pci_resume(struct usb_hcd *hcd, bool hibernated)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 3a0f695..a75cc7f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -689,20 +689,21 @@ static void xhci_clear_command_ring(struct xhci_hcd *xhci)
  * This is called when the machine transition into S3/S4 mode.
  *
  */
 int xhci_suspend(struct xhci_hcd *xhci)
 {
 	int			rc = 0;
 	struct usb_hcd		*hcd = xhci_to_hcd(xhci);
 	u32			command;
 	int			i;
 
+	xhci_dbg(xhci, "%s\n", __func__);
 	spin_lock_irq(&xhci->lock);
 	clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
 	clear_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
 	/* step 1: stop endpoint */
 	/* skipped assuming that port suspend has done */
 
 	/* step 2: clear Run/Stop bit */
 	command = xhci_readl(xhci, &xhci->op_regs->command);
 	command &= ~CMD_RUN;
 	xhci_writel(xhci, command, &xhci->op_regs->command);
@@ -744,20 +745,21 @@ int xhci_suspend(struct xhci_hcd *xhci)
  * This is called when the machine transition from S3/S4 mode.
  *
  */
 int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 {
 	u32			command, temp = 0;
 	struct usb_hcd		*hcd = xhci_to_hcd(xhci);
 	struct usb_hcd		*secondary_hcd;
 	int			retval;
 
+	xhci_dbg(xhci, "%s\n", __func__);
 	/* Wait a bit if either of the roothubs need to settle from the
 	 * transition into bus suspend.
 	 */
 	if (time_before(jiffies, xhci->bus_state[0].next_statechange) ||
 			time_before(jiffies,
 				xhci->bus_state[1].next_statechange))
 		msleep(100);
 
 	spin_lock_irq(&xhci->lock);
 	if (xhci->quirks & XHCI_RESET_ON_RESUME)

Re: PME via interrupt or SCI mechanism?

[Luming Yu]

On Tue, Sep 27, 2011 at 7:48 AM, Sarah Sharp
<sarah.a.sharp@linux.intel.com> wrote:
>
> Ok.  Do you have suggestions for any tests I can run or debugging
> statements to add?  I'm really not an ACPI expert and I've been
> basically fumbling in the dark.

The following patch from Shaohua might be useful to help debugging the problem.
http://kerneltrap.org/mailarchive/linux-usb/2009/7/30/6251245

/l

Re: PME via interrupt or SCI mechanism?

[Rafael J. Wysocki]

On Tuesday, September 27, 2011, Luming Yu wrote:
> On Tue, Sep 27, 2011 at 7:48 AM, Sarah Sharp
> <sarah.a.sharp@linux.intel.com> wrote:
> >
> > Ok.  Do you have suggestions for any tests I can run or debugging
> > statements to add?  I'm really not an ACPI expert and I've been
> > basically fumbling in the dark.
> 
> The following patch from Shaohua might be useful to help debugging the problem.
> http://kerneltrap.org/mailarchive/linux-usb/2009/7/30/6251245

You probably don't realize that the patch is neither applicable nor
relevant any more, but it would be nice to check that before
recommending it to anyone, don't you think?

Rafael

Re: PME via interrupt or SCI mechanism?

[Luming Yu]

On Tue, Sep 27, 2011 at 4:32 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Tuesday, September 27, 2011, Luming Yu wrote:
>> On Tue, Sep 27, 2011 at 7:48 AM, Sarah Sharp
>> <sarah.a.sharp@linux.intel.com> wrote:
>> >
>> > Ok.  Do you have suggestions for any tests I can run or debugging
>> > statements to add?  I'm really not an ACPI expert and I've been
>> > basically fumbling in the dark.
>>
>> The following patch from Shaohua might be useful to help debugging the problem.
>> http://kerneltrap.org/mailarchive/linux-usb/2009/7/30/6251245
>
> You probably don't realize that the patch is neither applicable nor
> relevant any more, but it would be nice to check that before
> recommending it to anyone, don't you think?
>

As long as ACPI is involved in some aspect of the bug, I do believe
the patch I pointed out can point you to some interesting interfaces
in ACPI Linux and CA that could be useful to diagnose this type of
problem. At least it pointed out how things begun :)

Re: PME via interrupt or SCI mechanism?

[Rafael J. Wysocki]

On Tuesday, September 27, 2011, Sarah Sharp wrote:
> On Tue, Sep 27, 2011 at 12:20:26AM +0200, Rafael J. Wysocki wrote:
> > On Sunday, September 25, 2011, Rafael J. Wysocki wrote:
> > > On Thursday, September 22, 2011, Sarah Sharp wrote:
> > > > On Mon, Sep 19, 2011 at 11:43:33PM +0200, Rafael J. Wysocki wrote:
> > > Without looking at the tables at the moment (I'll do that later),
> > > I think that they are missing the information that GPE 0D is a wakeup
> > > GPE for the xHCI device.
> > 
> > The DSDT appears to contain that information, so I'm not sure what's
> > going on.  Perhaps you can put a debug printk into acpi_dev_run_wake()
> > to see if that function is called for the xHCI controllers?
> 
> I put a printk in acpi_dev_run_wake(), and that shows up in the original
> dmesg I sent:
> 
> Sep 22 10:47:09 talon kernel: [ 2026.211933] xhci_hcd 0000:00:14.0: acpi_pci_run_wake - enable dev wake
> Sep 22 10:47:09 talon kernel: [ 2026.211936] acpi device:34: acpi_dev_run_wake - enable dev wake
> Sep 22 10:47:09 talon kernel: [ 2026.211955] acpi device:34: acpi_dev_run_wake - return -19

OK, can you printk() dev->wakeup.gpe_number here?

[/me realizes that "error" is not really used in this function.  Bah.]

> The patch I've been using for adding debugging to the PCI and ACPI code
> is attached.  I've been adding printks willy-nilly to try to understand
> what's going on.

That's fine.

> > > > In digging through the ACPI code, I noticed that acpi_bus_get_flags()
> > > > looks for the ACPI methods _PR0 or _PS0 and sets
> > > > device->flags.power_manageable to 1 if either of those methods are
> > > > successfully invoked.  When I deassembled the ACPI tables, I didn't see
> > > > either method for any of the USB host controllers in the system.
> > 
> > However, the power_manageable flag only indicates that the device can
> > be put into low-power states through ACPI methods, it shouldn't have
> > any effect on the wakeup settings.
> 
> Ok, good to know.
> 
> > > > device->flags.power_manageable is checked later when the runtime PM
> > > > system attempts to put the PCI device into a lower state, but it seems
> > > > to be ignored?  Is it supposed to be ignored?
> > > 
> > > Hmm, not really.  I'll have a look at that later.
> > 
> > It is used to decide whether or not to call __acpi_bus_set_power(), AFAICS.
> > If it is not set, this function is not called, which is OK.  Still,
> > devices for which it is not set may be put into low-power states and may
> > generate wakeup signals.
> > 
> > For many PCI devices there are two possible power management interfaces,
> > the native one and the ACPI-based one.  All of the modern devices support
> > the native power management interface, so they can be put into low-power
> > states even if the ACPI-based interface is missing for them (which is the
> > case for your USB controllers).  For those devices, if the ACPI-based
> > interface is not present, we simply use the native one only.
> 
> So if I understand you correctly, the kernel can use the native PCI power
> management interface to put the PCI device into a lower power state...
> 
> > As far as wakeup is concerned, we should enable them to generate PME
> > using the native interface and in addition to it we should use ACPI to
> > enable the wakeup GPEs that are supposed to be triggered in response to
> > the PME signals.
> 
> ...but the kernel ACPI core will still be watching for the SCI that the
> firmware generates on a PME wakeup?  Correct?

Yes, it should work like this.

> Is the native PCI power management interface you're referring to
> implemented through the PCIe PME driver?

No, this is what pci_raw_set_power_state() does.

> Because this particular host controller is a PCIe device, not a PCI device.

Well, PCIe is PCI to some extent. :-)

> I added some debug statements to drivers/pci/pcie/pme.c, but the dmesg shows
> that none of them got called.

This one only handles the native PME signaling, which is a PCIe feature
(instead of using ACPI GPEs for that, it signals PME via interrupts from
Root Ports).  Of course, it only works if you have those "PCIe PME" entries
in /proc/interrupts.

> > This apparently doesn't work correctly on your system and we need to figure
> > out why.
> 
> Ok.  Do you have suggestions for any tests I can run or debugging
> statements to add?  I'm really not an ACPI expert and I've been
> basically fumbling in the dark.

Yes, please see above.  I need to know the number of the GPE it tries to
enable for the USB controller.

Thanks,
Rafael

Re: PME via interrupt or SCI mechanism?

[Sarah Sharp]

On Tue, Sep 27, 2011 at 10:54:47PM +0200, Rafael J. Wysocki wrote:
> On Tuesday, September 27, 2011, Sarah Sharp wrote:
> > On Tue, Sep 27, 2011 at 12:20:26AM +0200, Rafael J. Wysocki wrote:
> > > On Sunday, September 25, 2011, Rafael J. Wysocki wrote:
> > > > On Thursday, September 22, 2011, Sarah Sharp wrote:
> > > > > On Mon, Sep 19, 2011 at 11:43:33PM +0200, Rafael J. Wysocki wrote:
> > > > Without looking at the tables at the moment (I'll do that later),
> > > > I think that they are missing the information that GPE 0D is a wakeup
> > > > GPE for the xHCI device.
> > > 
> > > The DSDT appears to contain that information, so I'm not sure what's
> > > going on.  Perhaps you can put a debug printk into acpi_dev_run_wake()
> > > to see if that function is called for the xHCI controllers?
> > 
> > I put a printk in acpi_dev_run_wake(), and that shows up in the original
> > dmesg I sent:
> > 
> > Sep 22 10:47:09 talon kernel: [ 2026.211933] xhci_hcd 0000:00:14.0: acpi_pci_run_wake - enable dev wake
> > Sep 22 10:47:09 talon kernel: [ 2026.211936] acpi device:34: acpi_dev_run_wake - enable dev wake
> > Sep 22 10:47:09 talon kernel: [ 2026.211955] acpi device:34: acpi_dev_run_wake - return -19
> 
> OK, can you printk() dev->wakeup.gpe_number here?
> 
> [/me realizes that "error" is not really used in this function.  Bah.]

Ok, here's what dmesg says (I used %u not 0x%x for the GPE number):

Sep 27 16:30:34 talon kernel: [  882.301023] xhci_hcd 0000:00:14.0: xhci_pci_suspend
Sep 27 16:30:34 talon kernel: [  882.301029] xhci_hcd 0000:00:14.0: xhci_suspend
Sep 27 16:30:34 talon kernel: [  882.301039] xhci_hcd 0000:00:14.0: // Setting command ring address to 0x30c3f001
Sep 27 16:30:34 talon kernel: [  882.301090] xhci_hcd 0000:00:14.0: hcd_pci_runtime_suspend: 0
Sep 27 16:30:34 talon kernel: [  882.301140] acpi_bus_power_manageable - result 0, device->flags.power_manageable 0
Sep 27 16:30:34 talon kernel: [  882.301143] xhci_hcd 0000:00:14.0: acpi_pci_power_manageable - dev is not power manageable
Sep 27 16:30:34 talon kernel: [  882.301146] acpi_bus_power_manageable - result 0, device->flags.power_manageable 0
Sep 27 16:30:34 talon kernel: [  882.301149] xhci_hcd 0000:00:14.0: __pci_enable_wake - enable runtimewake
Sep 27 16:30:34 talon kernel: [  882.301156] xhci_hcd 0000:00:14.0: PME# enabled
Sep 27 16:30:34 talon kernel: [  882.301159] xhci_hcd 0000:00:14.0: acpi_pci_run_wake - enable dev wake
Sep 27 16:30:34 talon kernel: [  882.301162] acpi device:34: acpi_dev_run_wake - enable dev wake
Sep 27 16:30:34 talon kernel: [  882.301175] acpi device:34: acpi_dev_run_wake - enable gpe 13
Sep 27 16:30:34 talon kernel: [  882.301185] acpi device:34: acpi_dev_run_wake - return -19
Sep 27 16:30:34 talon kernel: [  882.301188] xhci_hcd 0000:00:14.0: pci_raw_set_power_state from 0 to 3
Sep 27 16:30:34 talon kernel: [  882.312288] acpi_bus_power_manageable - result 0, device->flags.power_manageable 0
Sep 27 16:30:34 talon kernel: [  882.312293] xhci_hcd 0000:00:14.0: acpi_pci_power_manageable - dev is not power manageable
Sep 27 16:30:34 talon kernel: [  882.312296] acpi_bus_power_manageable - result 0, device->flags.power_manageable 0
Sep 27 16:30:34 talon kernel: [  882.312299] xhci_hcd 0000:00:14.0: pci_platform_power_transition - is NOT power managable

If I wake up the host controller by plugging in a webcam into an external hub:

Sep 27 16:31:46 talon kernel: [  953.645014] ehci_hcd 0000:00:1d.0: pci_acpi_wake_dev
Sep 27 16:31:46 talon kernel: [  953.645023] ehci_hcd 0000:00:1a.0: pci_acpi_wake_dev
Sep 27 16:31:46 talon kernel: [  953.645029] snd_hda_intel 0000:00:1b.0: pci_acpi_wake_dev
Sep 27 16:31:46 talon kernel: [  953.645034] e1000e 0000:00:19.0: pci_acpi_wake_dev
Sep 27 16:31:46 talon kernel: [  953.645089] ehci_hcd 0000:00:1d.0: pci_acpi_wake_dev
Sep 27 16:31:46 talon kernel: [  953.645094] ehci_hcd 0000:00:1a.0: pci_acpi_wake_dev
Sep 27 16:31:46 talon kernel: [  953.645098] snd_hda_intel 0000:00:1b.0: pci_acpi_wake_dev
Sep 27 16:31:46 talon kernel: [  953.645103] e1000e 0000:00:19.0: pci_acpi_wake_dev

Those lines repeat constantly until I echo on to the power/control file
for the xHCI device.

If I cat the /sys/firmware/acpi/interrupts/gpe0D file *before* I disable
D3, I see:

root@talon:/sys/firmware/acpi/interrupts# cat gpe0D
  211327   enabled
root@talon:/sys/firmware/acpi/interrupts# 

I think I was catting the file after I ran `echo on > power/control`
previously.  Sorry for the confusion.

So it looks like gpe 0xD is enabled when the host goes into D3, and
acpi_dev_run_wake is calling acpi_enable_gpe() with GPE 13 (i.e. 0xD),
correct?

Sarah Sharp

Re: PME via interrupt or SCI mechanism?

[Rafael J. Wysocki]

On Wednesday, September 28, 2011, Sarah Sharp wrote:
> On Tue, Sep 27, 2011 at 10:54:47PM +0200, Rafael J. Wysocki wrote:
> > On Tuesday, September 27, 2011, Sarah Sharp wrote:
> > > On Tue, Sep 27, 2011 at 12:20:26AM +0200, Rafael J. Wysocki wrote:
> > > > On Sunday, September 25, 2011, Rafael J. Wysocki wrote:
> > > > > On Thursday, September 22, 2011, Sarah Sharp wrote:
> > > > > > On Mon, Sep 19, 2011 at 11:43:33PM +0200, Rafael J. Wysocki wrote:
> > > > > Without looking at the tables at the moment (I'll do that later),
> > > > > I think that they are missing the information that GPE 0D is a wakeup
> > > > > GPE for the xHCI device.
> > > > 
> > > > The DSDT appears to contain that information, so I'm not sure what's
> > > > going on.  Perhaps you can put a debug printk into acpi_dev_run_wake()
> > > > to see if that function is called for the xHCI controllers?
> > > 
> > > I put a printk in acpi_dev_run_wake(), and that shows up in the original
> > > dmesg I sent:
> > > 
> > > Sep 22 10:47:09 talon kernel: [ 2026.211933] xhci_hcd 0000:00:14.0: acpi_pci_run_wake - enable dev wake
> > > Sep 22 10:47:09 talon kernel: [ 2026.211936] acpi device:34: acpi_dev_run_wake - enable dev wake
> > > Sep 22 10:47:09 talon kernel: [ 2026.211955] acpi device:34: acpi_dev_run_wake - return -19
> > 
> > OK, can you printk() dev->wakeup.gpe_number here?
> > 
> > [/me realizes that "error" is not really used in this function.  Bah.]
> 
> Ok, here's what dmesg says (I used %u not 0x%x for the GPE number):
> 
> Sep 27 16:30:34 talon kernel: [  882.301023] xhci_hcd 0000:00:14.0: xhci_pci_suspend
> Sep 27 16:30:34 talon kernel: [  882.301029] xhci_hcd 0000:00:14.0: xhci_suspend
> Sep 27 16:30:34 talon kernel: [  882.301039] xhci_hcd 0000:00:14.0: // Setting command ring address to 0x30c3f001
> Sep 27 16:30:34 talon kernel: [  882.301090] xhci_hcd 0000:00:14.0: hcd_pci_runtime_suspend: 0
> Sep 27 16:30:34 talon kernel: [  882.301140] acpi_bus_power_manageable - result 0, device->flags.power_manageable 0
> Sep 27 16:30:34 talon kernel: [  882.301143] xhci_hcd 0000:00:14.0: acpi_pci_power_manageable - dev is not power manageable
> Sep 27 16:30:34 talon kernel: [  882.301146] acpi_bus_power_manageable - result 0, device->flags.power_manageable 0
> Sep 27 16:30:34 talon kernel: [  882.301149] xhci_hcd 0000:00:14.0: __pci_enable_wake - enable runtimewake
> Sep 27 16:30:34 talon kernel: [  882.301156] xhci_hcd 0000:00:14.0: PME# enabled
> Sep 27 16:30:34 talon kernel: [  882.301159] xhci_hcd 0000:00:14.0: acpi_pci_run_wake - enable dev wake
> Sep 27 16:30:34 talon kernel: [  882.301162] acpi device:34: acpi_dev_run_wake - enable dev wake
> Sep 27 16:30:34 talon kernel: [  882.301175] acpi device:34: acpi_dev_run_wake - enable gpe 13
> Sep 27 16:30:34 talon kernel: [  882.301185] acpi device:34: acpi_dev_run_wake - return -19
> Sep 27 16:30:34 talon kernel: [  882.301188] xhci_hcd 0000:00:14.0: pci_raw_set_power_state from 0 to 3
> Sep 27 16:30:34 talon kernel: [  882.312288] acpi_bus_power_manageable - result 0, device->flags.power_manageable 0
> Sep 27 16:30:34 talon kernel: [  882.312293] xhci_hcd 0000:00:14.0: acpi_pci_power_manageable - dev is not power manageable
> Sep 27 16:30:34 talon kernel: [  882.312296] acpi_bus_power_manageable - result 0, device->flags.power_manageable 0
> Sep 27 16:30:34 talon kernel: [  882.312299] xhci_hcd 0000:00:14.0: pci_platform_power_transition - is NOT power managable
> 
> If I wake up the host controller by plugging in a webcam into an external hub:
> 
> Sep 27 16:31:46 talon kernel: [  953.645014] ehci_hcd 0000:00:1d.0: pci_acpi_wake_dev
> Sep 27 16:31:46 talon kernel: [  953.645023] ehci_hcd 0000:00:1a.0: pci_acpi_wake_dev
> Sep 27 16:31:46 talon kernel: [  953.645029] snd_hda_intel 0000:00:1b.0: pci_acpi_wake_dev
> Sep 27 16:31:46 talon kernel: [  953.645034] e1000e 0000:00:19.0: pci_acpi_wake_dev
> Sep 27 16:31:46 talon kernel: [  953.645089] ehci_hcd 0000:00:1d.0: pci_acpi_wake_dev
> Sep 27 16:31:46 talon kernel: [  953.645094] ehci_hcd 0000:00:1a.0: pci_acpi_wake_dev
> Sep 27 16:31:46 talon kernel: [  953.645098] snd_hda_intel 0000:00:1b.0: pci_acpi_wake_dev
> Sep 27 16:31:46 talon kernel: [  953.645103] e1000e 0000:00:19.0: pci_acpi_wake_dev
> 
> Those lines repeat constantly until I echo on to the power/control file
> for the xHCI device.
> 
> If I cat the /sys/firmware/acpi/interrupts/gpe0D file *before* I disable
> D3, I see:
> 
> root@talon:/sys/firmware/acpi/interrupts# cat gpe0D
>   211327   enabled
> root@talon:/sys/firmware/acpi/interrupts# 
> 
> I think I was catting the file after I ran `echo on > power/control`
> previously.  Sorry for the confusion.
> 
> So it looks like gpe 0xD is enabled when the host goes into D3, and
> acpi_dev_run_wake is calling acpi_enable_gpe() with GPE 13 (i.e. 0xD),
> correct?

Yes, that's correct.

Moreover, evidently, the event is signaled and it causes pci_acpi_wake_dev()
to be called for multiple devices, _except_ for the xhci_hcd.  Perhaps
the notifier is not installed for that device for some reason.

Please add an additional debug printk()s to pci_acpi_add_pm_notifier()
for both pci_dev and dev and for the result returned by add_pm_notifier().

Thanks,
Rafael

Re: PME via interrupt or SCI mechanism?

[Matthew Garrett]

On Thu, Sep 29, 2011 at 12:21:28AM +0200, Rafael J. Wysocki wrote:

> Moreover, evidently, the event is signaled and it causes pci_acpi_wake_dev()
> to be called for multiple devices, _except_ for the xhci_hcd.  Perhaps
> the notifier is not installed for that device for some reason.

I suspect that we're just going to end up accepting defeat and having to 
poll *all* pci devices (including pcie) once a second.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: PME via interrupt or SCI mechanism?

[Rafael J. Wysocki]

On Thursday, September 29, 2011, Matthew Garrett wrote:
> On Thu, Sep 29, 2011 at 12:21:28AM +0200, Rafael J. Wysocki wrote:
> 
> > Moreover, evidently, the event is signaled and it causes pci_acpi_wake_dev()
> > to be called for multiple devices, _except_ for the xhci_hcd.  Perhaps
> > the notifier is not installed for that device for some reason.
> 
> I suspect that we're just going to end up accepting defeat and having to 
> poll *all* pci devices (including pcie) once a second.

Well, this case actually looks like a subtle bug in ACPICA.  I think we
need to debug it further before deciding anything.

Thanks,
Rafael

Re: PME via interrupt or SCI mechanism?

[Sarah Sharp]

On Thu, Sep 29, 2011 at 12:21:28AM +0200, Rafael J. Wysocki wrote:
> On Wednesday, September 28, 2011, Sarah Sharp wrote:
> > On Tue, Sep 27, 2011 at 10:54:47PM +0200, Rafael J. Wysocki wrote:
> > > On Tuesday, September 27, 2011, Sarah Sharp wrote:
> > So it looks like gpe 0xD is enabled when the host goes into D3, and
> > acpi_dev_run_wake is calling acpi_enable_gpe() with GPE 13 (i.e. 0xD),
> > correct?
> 
> Yes, that's correct.
> 
> Moreover, evidently, the event is signaled and it causes pci_acpi_wake_dev()
> to be called for multiple devices, _except_ for the xhci_hcd.  Perhaps
> the notifier is not installed for that device for some reason.
> 
> Please add an additional debug printk()s to pci_acpi_add_pm_notifier()
> for both pci_dev and dev and for the result returned by add_pm_notifier().

dmesg reports success from pci_acpi_add_pm_notifier for all PCI devices,
including the xHCI host (PCI device 0000:00:14.0):

[    0.936882] pci_bus 0000:00: bus scan returning with max=04
[    0.936884] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0._PRT]
[    0.936961] pci 0000:00:1f.0: pci_acpi_add_pm_notifier
[    0.936964] acpi device:02: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
[    0.936977] pci 0000:00:19.0: pci_acpi_add_pm_notifier
[    0.936978] acpi device:1f: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
[    0.936981] pci 0000:00:1d.0: pci_acpi_add_pm_notifier
[    0.936983] acpi device:20: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
[    0.936986] pci 0000:00:1a.0: pci_acpi_add_pm_notifier
[    0.936987] acpi device:2b: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
[    0.936990] pci 0000:00:14.0: pci_acpi_add_pm_notifier
[    0.936992] acpi device:34: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
[    0.936995] pci 0000:00:1b.0: pci_acpi_add_pm_notifier
[    0.936996] acpi device:3e: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
[    0.936999] pci 0000:00:1c.0: pci_acpi_add_pm_notifier
[    0.937001] acpi device:3f: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
[    0.937002] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.RP01._PRT]
[    0.937033] pci 0000:00:1c.7: pci_acpi_add_pm_notifier
[    0.937035] acpi device:4d: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
[    0.937036] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.RP08._PRT]
[    0.937056] pci 0000:03:00.0: pci_acpi_add_pm_notifier
[    0.937057] acpi device:4e: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
[    0.937060] pci 0000:00:1f.2: pci_acpi_add_pm_notifier
[    0.937062] acpi device:4f: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
[    0.937066] pci 0000:00:1f.3: pci_acpi_add_pm_notifier
[    0.937068] acpi device:52: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
[    0.937071] pci 0000:00:01.0: pci_acpi_add_pm_notifier
[    0.937072] acpi device:53: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
[    0.937074] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.PEG0._PRT]
[    0.937094] pci 0000:01:00.0: pci_acpi_add_pm_notifier
[    0.937095] acpi device:54: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
[    0.937104] acpi_pci_osc_support

This morning, I debugged an issue with the NEC xHCI host controller
issue in Keith Packard's Lenovo x220 machine.  The NEC host was not
giving port status changes when the host controller was suspended, and
it turns out Keith has a boot script that runs `echo auto > power/control`
for all his PCI devices.  When he disabled that script and rebooted, his
NEC host started working again.

So it's possible that other xHCI host controllers are also affected by
this D3 wakeup issue, which makes it less likely to be a hardware bug,
and more likely to be a PCI/ACPI/xHCI driver bug.

Sarah Sharp

Re: PME via interrupt or SCI mechanism?

[Rafael J. Wysocki]

On Thursday, September 29, 2011, Sarah Sharp wrote:
> On Thu, Sep 29, 2011 at 12:21:28AM +0200, Rafael J. Wysocki wrote:
> > On Wednesday, September 28, 2011, Sarah Sharp wrote:
> > > On Tue, Sep 27, 2011 at 10:54:47PM +0200, Rafael J. Wysocki wrote:
> > > > On Tuesday, September 27, 2011, Sarah Sharp wrote:
> > > So it looks like gpe 0xD is enabled when the host goes into D3, and
> > > acpi_dev_run_wake is calling acpi_enable_gpe() with GPE 13 (i.e. 0xD),
> > > correct?
> > 
> > Yes, that's correct.
> > 
> > Moreover, evidently, the event is signaled and it causes pci_acpi_wake_dev()
> > to be called for multiple devices, _except_ for the xhci_hcd.  Perhaps
> > the notifier is not installed for that device for some reason.
> > 
> > Please add an additional debug printk()s to pci_acpi_add_pm_notifier()
> > for both pci_dev and dev and for the result returned by add_pm_notifier().
> 
> dmesg reports success from pci_acpi_add_pm_notifier for all PCI devices,
> including the xHCI host (PCI device 0000:00:14.0):
> 
> [    0.936882] pci_bus 0000:00: bus scan returning with max=04
> [    0.936884] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0._PRT]
> [    0.936961] pci 0000:00:1f.0: pci_acpi_add_pm_notifier
> [    0.936964] acpi device:02: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
> [    0.936977] pci 0000:00:19.0: pci_acpi_add_pm_notifier
> [    0.936978] acpi device:1f: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
> [    0.936981] pci 0000:00:1d.0: pci_acpi_add_pm_notifier
> [    0.936983] acpi device:20: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
> [    0.936986] pci 0000:00:1a.0: pci_acpi_add_pm_notifier
> [    0.936987] acpi device:2b: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
> [    0.936990] pci 0000:00:14.0: pci_acpi_add_pm_notifier
> [    0.936992] acpi device:34: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
> [    0.936995] pci 0000:00:1b.0: pci_acpi_add_pm_notifier
> [    0.936996] acpi device:3e: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
> [    0.936999] pci 0000:00:1c.0: pci_acpi_add_pm_notifier
> [    0.937001] acpi device:3f: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
> [    0.937002] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.RP01._PRT]
> [    0.937033] pci 0000:00:1c.7: pci_acpi_add_pm_notifier
> [    0.937035] acpi device:4d: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
> [    0.937036] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.RP08._PRT]
> [    0.937056] pci 0000:03:00.0: pci_acpi_add_pm_notifier
> [    0.937057] acpi device:4e: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
> [    0.937060] pci 0000:00:1f.2: pci_acpi_add_pm_notifier
> [    0.937062] acpi device:4f: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
> [    0.937066] pci 0000:00:1f.3: pci_acpi_add_pm_notifier
> [    0.937068] acpi device:52: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
> [    0.937071] pci 0000:00:01.0: pci_acpi_add_pm_notifier
> [    0.937072] acpi device:53: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
> [    0.937074] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.PEG0._PRT]
> [    0.937094] pci 0000:01:00.0: pci_acpi_add_pm_notifier
> [    0.937095] acpi device:54: pci_acpi_add_pm_notifier add_pm_notifier returned 0 (success)
> [    0.937104] acpi_pci_osc_support
> 
> This morning, I debugged an issue with the NEC xHCI host controller
> issue in Keith Packard's Lenovo x220 machine.  The NEC host was not
> giving port status changes when the host controller was suspended, and
> it turns out Keith has a boot script that runs `echo auto > power/control`
> for all his PCI devices.  When he disabled that script and rebooted, his
> NEC host started working again.
> 
> So it's possible that other xHCI host controllers are also affected by
> this D3 wakeup issue, which makes it less likely to be a hardware bug,
> and more likely to be a PCI/ACPI/xHCI driver bug.

I'd recommend not to draw conclusions too early in this case.  It very
well may be a BIOS bug copy-pasted to may implementations or something
like this.

Please try the appended patch and check if you see the "Notification error
for GPE" message (please keep your previous debug patches applied).

Thanks,
Rafael

---
 drivers/acpi/acpica/evgpe.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Index: linux/drivers/acpi/acpica/evgpe.c
===================================================================
--- linux.orig/drivers/acpi/acpica/evgpe.c
+++ linux/drivers/acpi/acpica/evgpe.c
@@ -523,10 +523,14 @@ static void ACPI_SYSTEM_XFACE acpi_ev_as
 				ACPI_NOTIFY_DEVICE_WAKE);
 
 		notify_object = local_gpe_event_info->dispatch.device.next;
-		while (ACPI_SUCCESS(status) && notify_object) {
+		while (notify_object) {
 			status = acpi_ev_queue_notify_request(
 					notify_object->node,
 					ACPI_NOTIFY_DEVICE_WAKE);
+			if (ACPI_FAILURE(status))
+				ACPI_ERROR((AE_INFO,
+					"Notification error for GPE 0x%X",
+					local_gpe_event_info->gpe_number));
 			notify_object = notify_object->next;
 		}
 

Re: PME via interrupt or SCI mechanism?

[Sarah Sharp]

On Thu, Sep 29, 2011 at 09:39:56PM +0200, Rafael J. Wysocki wrote:
> On Thursday, September 29, 2011, Sarah Sharp wrote:
> > On Thu, Sep 29, 2011 at 12:21:28AM +0200, Rafael J. Wysocki wrote:
> Please try the appended patch and check if you see the "Notification error
> for GPE" message (please keep your previous debug patches applied).

Do I need to have the ACPI debug_level or debug_layer set to anything in
particular to see this message?  I don't see this message in dmesg with
debug_level and debug_layer set to 0x0.

Sarah Sharp

> ---
>  drivers/acpi/acpica/evgpe.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> Index: linux/drivers/acpi/acpica/evgpe.c
> ===================================================================
> --- linux.orig/drivers/acpi/acpica/evgpe.c
> +++ linux/drivers/acpi/acpica/evgpe.c
> @@ -523,10 +523,14 @@ static void ACPI_SYSTEM_XFACE acpi_ev_as
>  				ACPI_NOTIFY_DEVICE_WAKE);
>  
>  		notify_object = local_gpe_event_info->dispatch.device.next;
> -		while (ACPI_SUCCESS(status) && notify_object) {
> +		while (notify_object) {
>  			status = acpi_ev_queue_notify_request(
>  					notify_object->node,
>  					ACPI_NOTIFY_DEVICE_WAKE);
> +			if (ACPI_FAILURE(status))
> +				ACPI_ERROR((AE_INFO,
> +					"Notification error for GPE 0x%X",
> +					local_gpe_event_info->gpe_number));
>  			notify_object = notify_object->next;
>  		}
>  

Re: PME via interrupt or SCI mechanism?

[Rafael J. Wysocki]

On Thursday, September 29, 2011, Sarah Sharp wrote:
> On Thu, Sep 29, 2011 at 09:39:56PM +0200, Rafael J. Wysocki wrote:
> > On Thursday, September 29, 2011, Sarah Sharp wrote:
> > > On Thu, Sep 29, 2011 at 12:21:28AM +0200, Rafael J. Wysocki wrote:
> > Please try the appended patch and check if you see the "Notification error
> > for GPE" message (please keep your previous debug patches applied).
> 
> Do I need to have the ACPI debug_level or debug_layer set to anything in
> particular to see this message?

No, I don't think so, but just in case please try the patch below instead
of the previous one.

Thanks,
Rafael

---
 drivers/acpi/acpica/evgpe.c |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Index: linux/drivers/acpi/acpica/evgpe.c
===================================================================
--- linux.orig/drivers/acpi/acpica/evgpe.c
+++ linux/drivers/acpi/acpica/evgpe.c
@@ -521,12 +521,24 @@ static void ACPI_SYSTEM_XFACE acpi_ev_as
 		status = acpi_ev_queue_notify_request(
 				local_gpe_event_info->dispatch.device.node,
 				ACPI_NOTIFY_DEVICE_WAKE);
+		if (ACPI_FAILURE(status))
+			pr_err("Notification error for GPE 0x%X",
+				local_gpe_event_info->gpe_number);
+		else
+			pr_info("Notification queued up for GPE 0x%X",
+				local_gpe_event_info->gpe_number);
 
 		notify_object = local_gpe_event_info->dispatch.device.next;
-		while (ACPI_SUCCESS(status) && notify_object) {
+		while (notify_object) {
 			status = acpi_ev_queue_notify_request(
 					notify_object->node,
 					ACPI_NOTIFY_DEVICE_WAKE);
+			if (ACPI_FAILURE(status))
+				pr_err("Notification error for GPE 0x%X",
+					local_gpe_event_info->gpe_number);
+			else
+				pr_info("Notification queued up for GPE 0x%X",
+					local_gpe_event_info->gpe_number);
 			notify_object = notify_object->next;
 		}
 

Re: PME via interrupt or SCI mechanism?

[Rafael J. Wysocki]

On Thursday, September 29, 2011, Rafael J. Wysocki wrote:
> On Thursday, September 29, 2011, Sarah Sharp wrote:
> > On Thu, Sep 29, 2011 at 09:39:56PM +0200, Rafael J. Wysocki wrote:
> > > On Thursday, September 29, 2011, Sarah Sharp wrote:
> > > > On Thu, Sep 29, 2011 at 12:21:28AM +0200, Rafael J. Wysocki wrote:
> > > Please try the appended patch and check if you see the "Notification error
> > > for GPE" message (please keep your previous debug patches applied).
> > 
> > Do I need to have the ACPI debug_level or debug_layer set to anything in
> > particular to see this message?
> 
> No, I don't think so, but just in case please try the patch below instead
> of the previous one.

Actually, please don't, it's a BIOS-related issue after all.  Apparently,
wakeup from xHCD is not supported by the BIOS, because the DSDT defines
the _L0D method for GPE 0D (13), which is the following:

        Method (_L0D, 0, NotSerialized)
        {
            Notify (\_SB.PCI0.EHC1, 0x02)
            Notify (\_SB.PCI0.EHC2, 0x02)
            Notify (\_SB.PCI0.HDEF, 0x02)
            Notify (\_SB.PCI0.GLAN, 0x02)
        }

so it notifies some devices, but not the xHCD.

We might work around this by doing what Matthew has suggested (ie. polling
all PCI and PCIe devices to check if they have PME pending) or perhaps
we can do something about this in ACPICA.

Still, the right fix is to put Notify () for the ACPI objects
corresponding to xHCD into the above method.

Thanks,
Rafael

Re: PME via interrupt or SCI mechanism?

[Rafael J. Wysocki]

On Thursday, September 29, 2011, Rafael J. Wysocki wrote:
> On Thursday, September 29, 2011, Rafael J. Wysocki wrote:
> > On Thursday, September 29, 2011, Sarah Sharp wrote:
> > > On Thu, Sep 29, 2011 at 09:39:56PM +0200, Rafael J. Wysocki wrote:
> > > > On Thursday, September 29, 2011, Sarah Sharp wrote:
> > > > > On Thu, Sep 29, 2011 at 12:21:28AM +0200, Rafael J. Wysocki wrote:
> > > > Please try the appended patch and check if you see the "Notification error
> > > > for GPE" message (please keep your previous debug patches applied).
> > > 
> > > Do I need to have the ACPI debug_level or debug_layer set to anything in
> > > particular to see this message?
> > 
> > No, I don't think so, but just in case please try the patch below instead
> > of the previous one.
> 
> Actually, please don't, it's a BIOS-related issue after all.  Apparently,
> wakeup from xHCD is not supported by the BIOS, because the DSDT defines
> the _L0D method for GPE 0D (13), which is the following:
> 
>         Method (_L0D, 0, NotSerialized)
>         {
>             Notify (\_SB.PCI0.EHC1, 0x02)
>             Notify (\_SB.PCI0.EHC2, 0x02)
>             Notify (\_SB.PCI0.HDEF, 0x02)
>             Notify (\_SB.PCI0.GLAN, 0x02)
>         }
> 
> so it notifies some devices, but not the xHCD.
> 
> We might work around this by doing what Matthew has suggested (ie. polling
> all PCI and PCIe devices to check if they have PME pending) or perhaps
> we can do something about this in ACPICA.
> 
> Still, the right fix is to put Notify () for the ACPI objects
> corresponding to xHCD into the above method.

Or to remove this method altogether (in which case our ACPICA code
should take care of the notifications).

Below is a patch (untested!) that should help, but it's kind of
suboptimal.  Nevertheless, please try if it works for you.

Thanks,
Rafael


---
 drivers/pci/pci.c |   57 +++++++++++++++++++-----------------------------------
 1 file changed, 21 insertions(+), 36 deletions(-)

Index: linux/drivers/pci/pci.c
===================================================================
--- linux.orig/drivers/pci/pci.c
+++ linux/drivers/pci/pci.c
@@ -1458,19 +1458,6 @@ static void pci_pme_list_scan(struct wor
 }
 
 /**
- * pci_external_pme - is a device an external PCI PME source?
- * @dev: PCI device to check
- *
- */
-
-static bool pci_external_pme(struct pci_dev *dev)
-{
-	if (pci_is_pcie(dev) || dev->bus->number == 0)
-		return false;
-	return true;
-}
-
-/**
  * pci_pme_active - enable or disable PCI device's PME# function
  * @dev: PCI device to handle.
  * @enable: 'true' to enable PME# generation; 'false' to disable it.
@@ -1480,6 +1467,7 @@ static bool pci_external_pme(struct pci_
  */
 void pci_pme_active(struct pci_dev *dev, bool enable)
 {
+	struct pci_pme_device *pme_dev;
 	u16 pmcsr;
 
 	if (!dev->pm_cap)
@@ -1503,31 +1491,28 @@ void pci_pme_active(struct pci_dev *dev,
 	   hit, and the power savings from the devices will still be a
 	   win. */
 
-	if (pci_external_pme(dev)) {
-		struct pci_pme_device *pme_dev;
-		if (enable) {
-			pme_dev = kmalloc(sizeof(struct pci_pme_device),
-					  GFP_KERNEL);
-			if (!pme_dev)
-				goto out;
-			pme_dev->dev = dev;
-			mutex_lock(&pci_pme_list_mutex);
-			list_add(&pme_dev->list, &pci_pme_list);
-			if (list_is_singular(&pci_pme_list))
-				schedule_delayed_work(&pci_pme_work,
-						      msecs_to_jiffies(PME_TIMEOUT));
-			mutex_unlock(&pci_pme_list_mutex);
-		} else {
-			mutex_lock(&pci_pme_list_mutex);
-			list_for_each_entry(pme_dev, &pci_pme_list, list) {
-				if (pme_dev->dev == dev) {
-					list_del(&pme_dev->list);
-					kfree(pme_dev);
-					break;
-				}
+	if (enable) {
+		pme_dev = kmalloc(sizeof(struct pci_pme_device), GFP_KERNEL);
+		if (!pme_dev)
+			goto out;
+
+		pme_dev->dev = dev;
+		mutex_lock(&pci_pme_list_mutex);
+		list_add(&pme_dev->list, &pci_pme_list);
+		if (list_is_singular(&pci_pme_list))
+			schedule_delayed_work(&pci_pme_work,
+					      msecs_to_jiffies(PME_TIMEOUT));
+		mutex_unlock(&pci_pme_list_mutex);
+	} else {
+		mutex_lock(&pci_pme_list_mutex);
+		list_for_each_entry(pme_dev, &pci_pme_list, list) {
+			if (pme_dev->dev == dev) {
+				list_del(&pme_dev->list);
+				kfree(pme_dev);
+				break;
 			}
-			mutex_unlock(&pci_pme_list_mutex);
 		}
+		mutex_unlock(&pci_pme_list_mutex);
 	}
 
 out:

Re: PME via interrupt or SCI mechanism?

[Rafael J. Wysocki]

On Friday, September 30, 2011, Sarah Sharp wrote:
> On Thu, Sep 29, 2011 at 11:51:49PM +0200, Rafael J. Wysocki wrote:
> > On Thursday, September 29, 2011, Rafael J. Wysocki wrote:
> > > On Thursday, September 29, 2011, Rafael J. Wysocki wrote:
> > > > On Thursday, September 29, 2011, Sarah Sharp wrote:
> > > > > On Thu, Sep 29, 2011 at 09:39:56PM +0200, Rafael J. Wysocki wrote:
> > > > > > On Thursday, September 29, 2011, Sarah Sharp wrote:
> > > > > > > On Thu, Sep 29, 2011 at 12:21:28AM +0200, Rafael J. Wysocki wrote:
> > > > > > Please try the appended patch and check if you see the "Notification error
> > > > > > for GPE" message (please keep your previous debug patches applied).
> > > > > 
> > > > > Do I need to have the ACPI debug_level or debug_layer set to anything in
> > > > > particular to see this message?
> > > > 
> > > > No, I don't think so, but just in case please try the patch below instead
> > > > of the previous one.
> > > 
> > > Actually, please don't, it's a BIOS-related issue after all.  Apparently,
> > > wakeup from xHCD is not supported by the BIOS, because the DSDT defines
> > > the _L0D method for GPE 0D (13), which is the following:
> > > 
> > >         Method (_L0D, 0, NotSerialized)
> > >         {
> > >             Notify (\_SB.PCI0.EHC1, 0x02)
> > >             Notify (\_SB.PCI0.EHC2, 0x02)
> > >             Notify (\_SB.PCI0.HDEF, 0x02)
> > >             Notify (\_SB.PCI0.GLAN, 0x02)
> > >         }
> > > 
> > > so it notifies some devices, but not the xHCD.
> 
> Ok, I'll let our BIOS guys know they need to add the XHC Notify line.

As stated below, it actually may be better to remove the _L0D method
entirely, because our ACPICA GPE handling code should take care of
notifying all of the devices associated with GPE 0D in that case.

> I should see if the x220 ACPI tables have a similar issue or it's a
> different issue with D3.

Yes, that would be good to know.

> > > We might work around this by doing what Matthew has suggested (ie. polling
> > > all PCI and PCIe devices to check if they have PME pending) or perhaps
> > > we can do something about this in ACPICA.
> > > 
> > > Still, the right fix is to put Notify () for the ACPI objects
> > > corresponding to xHCD into the above method.
> > 
> > Or to remove this method altogether (in which case our ACPICA code
> > should take care of the notifications).
> > 
> > Below is a patch (untested!) that should help, but it's kind of
> > suboptimal.  Nevertheless, please try if it works for you.
> 
> The patch seems to finally get the host controller out of D3 when a
> remote wakeup is triggered.

OK

I'll try to prepare a better patch for you to test.

> I still see a lot of log spam from pci_acpi_wake_dev being called for ehci,
> e1000e, and snd_hcda_intel, but eventually the xHCI host gets put in D0.
> dmesg attached.

That's to be expected.  All of the devices mentioned in the _L0D method
above will be notified every time any of devices associated with GPE 0D
sends a PME message to the Root Complex, which happens quite often
while the polling used to check the PME bits is done only every second.

Thanks,
Rafael

Re: PME via interrupt or SCI mechanism?

[Rafael J. Wysocki]

On Friday, September 30, 2011, Rafael J. Wysocki wrote:
> On Friday, September 30, 2011, Sarah Sharp wrote:
> > On Thu, Sep 29, 2011 at 11:51:49PM +0200, Rafael J. Wysocki wrote:
> > > On Thursday, September 29, 2011, Rafael J. Wysocki wrote:
> > > > On Thursday, September 29, 2011, Rafael J. Wysocki wrote:
> > > > > On Thursday, September 29, 2011, Sarah Sharp wrote:
> > > > > > On Thu, Sep 29, 2011 at 09:39:56PM +0200, Rafael J. Wysocki wrote:
> > > > > > > On Thursday, September 29, 2011, Sarah Sharp wrote:
> > > > > > > > On Thu, Sep 29, 2011 at 12:21:28AM +0200, Rafael J. Wysocki wrote:
> > > > > > > Please try the appended patch and check if you see the "Notification error
> > > > > > > for GPE" message (please keep your previous debug patches applied).
> > > > > > 
> > > > > > Do I need to have the ACPI debug_level or debug_layer set to anything in
> > > > > > particular to see this message?
> > > > > 
> > > > > No, I don't think so, but just in case please try the patch below instead
> > > > > of the previous one.
> > > > 
> > > > Actually, please don't, it's a BIOS-related issue after all.  Apparently,
> > > > wakeup from xHCD is not supported by the BIOS, because the DSDT defines
> > > > the _L0D method for GPE 0D (13), which is the following:
> > > > 
> > > >         Method (_L0D, 0, NotSerialized)
> > > >         {
> > > >             Notify (\_SB.PCI0.EHC1, 0x02)
> > > >             Notify (\_SB.PCI0.EHC2, 0x02)
> > > >             Notify (\_SB.PCI0.HDEF, 0x02)
> > > >             Notify (\_SB.PCI0.GLAN, 0x02)
> > > >         }
> > > > 
> > > > so it notifies some devices, but not the xHCD.
> > 
> > Ok, I'll let our BIOS guys know they need to add the XHC Notify line.
> 
> As stated below, it actually may be better to remove the _L0D method
> entirely, because our ACPICA GPE handling code should take care of
> notifying all of the devices associated with GPE 0D in that case.
> 
> > I should see if the x220 ACPI tables have a similar issue or it's a
> > different issue with D3.
> 
> Yes, that would be good to know.
> 
> > > > We might work around this by doing what Matthew has suggested (ie. polling
> > > > all PCI and PCIe devices to check if they have PME pending) or perhaps
> > > > we can do something about this in ACPICA.
> > > > 
> > > > Still, the right fix is to put Notify () for the ACPI objects
> > > > corresponding to xHCD into the above method.
> > > 
> > > Or to remove this method altogether (in which case our ACPICA code
> > > should take care of the notifications).
> > > 
> > > Below is a patch (untested!) that should help, but it's kind of
> > > suboptimal.  Nevertheless, please try if it works for you.
> > 
> > The patch seems to finally get the host controller out of D3 when a
> > remote wakeup is triggered.
> 
> OK
> 
> I'll try to prepare a better patch for you to test.

OK, please test the appended patch.  The difference is that it should only
continuously poll devices that don't get notifications.  You'll still see
the debug messages from your previous patches, but this one should be a bit
less wasteful than the previous one in general.

Thanks,
Rafael


Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pci/pci-acpi.c |    3 +++
 drivers/pci/pci.c      |   41 ++++++++++++++++++++---------------------
 drivers/pci/pcie/pme.c |    9 +++++++++
 include/linux/pci.h    |    1 +
 4 files changed, 33 insertions(+), 21 deletions(-)

Index: linux/include/linux/pci.h
===================================================================
--- linux.orig/include/linux/pci.h
+++ linux/include/linux/pci.h
@@ -273,6 +273,7 @@ struct pci_dev {
 	unsigned int	pme_support:5;	/* Bitmask of states from which PME#
 					   can be generated */
 	unsigned int	pme_interrupt:1;
+	unsigned int	pme_poll:1;	/* Poll device's PME status bit */
 	unsigned int	d1_support:1;	/* Low power state D1 is supported */
 	unsigned int	d2_support:1;	/* Low power state D2 is supported */
 	unsigned int	no_d1d2:1;	/* Only allow D0 and D3 */
Index: linux/drivers/pci/pci-acpi.c
===================================================================
--- linux.orig/drivers/pci/pci-acpi.c
+++ linux/drivers/pci/pci-acpi.c
@@ -46,6 +46,9 @@ static void pci_acpi_wake_dev(acpi_handl
 	struct pci_dev *pci_dev = context;
 
 	if (event == ACPI_NOTIFY_DEVICE_WAKE && pci_dev) {
+		if (pci_dev->pme_poll)
+			pci_dev->pme_poll = false;
+
 		pci_wakeup_event(pci_dev);
 		pci_check_pme_status(pci_dev);
 		pm_runtime_resume(&pci_dev->dev);
Index: linux/drivers/pci/pci.c
===================================================================
--- linux.orig/drivers/pci/pci.c
+++ linux/drivers/pci/pci.c
@@ -1407,13 +1407,16 @@ bool pci_check_pme_status(struct pci_dev
 /**
  * pci_pme_wakeup - Wake up a PCI device if its PME Status bit is set.
  * @dev: Device to handle.
- * @ign: Ignored.
+ * @pme_poll_reset: Whether or not to reset the device's pme_poll flag.
  *
  * Check if @dev has generated PME and queue a resume request for it in that
  * case.
  */
-static int pci_pme_wakeup(struct pci_dev *dev, void *ign)
+static int pci_pme_wakeup(struct pci_dev *dev, void *pme_poll_reset)
 {
+	if (pme_poll_reset && dev->pme_poll)
+		dev->pme_poll = false;
+
 	if (pci_check_pme_status(dev)) {
 		pci_wakeup_event(dev);
 		pm_request_resume(&dev->dev);
@@ -1428,7 +1431,7 @@ static int pci_pme_wakeup(struct pci_dev
 void pci_pme_wakeup_bus(struct pci_bus *bus)
 {
 	if (bus)
-		pci_walk_bus(bus, pci_pme_wakeup, NULL);
+		pci_walk_bus(bus, pci_pme_wakeup, (void *)true);
 }
 
 /**
@@ -1446,31 +1449,26 @@ bool pci_pme_capable(struct pci_dev *dev
 
 static void pci_pme_list_scan(struct work_struct *work)
 {
-	struct pci_pme_device *pme_dev;
+	struct pci_pme_device *pme_dev, *n;
 
 	mutex_lock(&pci_pme_list_mutex);
 	if (!list_empty(&pci_pme_list)) {
-		list_for_each_entry(pme_dev, &pci_pme_list, list)
-			pci_pme_wakeup(pme_dev->dev, NULL);
-		schedule_delayed_work(&pci_pme_work, msecs_to_jiffies(PME_TIMEOUT));
+		list_for_each_entry_safe(pme_dev, n, &pci_pme_list, list) {
+			if (pme_dev->dev->pme_poll) {
+				pci_pme_wakeup(pme_dev->dev, NULL);
+			} else {
+				list_del(&pme_dev->list);
+				kfree(pme_dev);
+			}
+		}
+		if (!list_empty(&pci_pme_list))
+			schedule_delayed_work(&pci_pme_work,
+					      msecs_to_jiffies(PME_TIMEOUT));
 	}
 	mutex_unlock(&pci_pme_list_mutex);
 }
 
 /**
- * pci_external_pme - is a device an external PCI PME source?
- * @dev: PCI device to check
- *
- */
-
-static bool pci_external_pme(struct pci_dev *dev)
-{
-	if (pci_is_pcie(dev) || dev->bus->number == 0)
-		return false;
-	return true;
-}
-
-/**
  * pci_pme_active - enable or disable PCI device's PME# function
  * @dev: PCI device to handle.
  * @enable: 'true' to enable PME# generation; 'false' to disable it.
@@ -1503,7 +1501,7 @@ void pci_pme_active(struct pci_dev *dev,
 	   hit, and the power savings from the devices will still be a
 	   win. */
 
-	if (pci_external_pme(dev)) {
+	if (dev->pme_poll) {
 		struct pci_pme_device *pme_dev;
 		if (enable) {
 			pme_dev = kmalloc(sizeof(struct pci_pme_device),
@@ -1821,6 +1819,7 @@ void pci_pm_init(struct pci_dev *dev)
 			 (pmc & PCI_PM_CAP_PME_D3) ? " D3hot" : "",
 			 (pmc & PCI_PM_CAP_PME_D3cold) ? " D3cold" : "");
 		dev->pme_support = pmc >> PCI_PM_CAP_PME_SHIFT;
+		dev->pme_poll = true;
 		/*
 		 * Make device's PM flags reflect the wake-up capability, but
 		 * let the user space enable it to wake up the system as needed.
Index: linux/drivers/pci/pcie/pme.c
===================================================================
--- linux.orig/drivers/pci/pcie/pme.c
+++ linux/drivers/pci/pcie/pme.c
@@ -84,6 +84,9 @@ static bool pcie_pme_walk_bus(struct pci
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		/* Skip PCIe devices in case we started from a root port. */
 		if (!pci_is_pcie(dev) && pci_check_pme_status(dev)) {
+			if (dev->pme_poll)
+				dev->pme_poll = false;
+
 			pci_wakeup_event(dev);
 			pm_request_resume(&dev->dev);
 			ret = true;
@@ -142,6 +145,9 @@ static void pcie_pme_handle_request(stru
 
 	/* First, check if the PME is from the root port itself. */
 	if (port->devfn == devfn && port->bus->number == busnr) {
+		if (port->pme_poll)
+			port->pme_poll = false;
+
 		if (pci_check_pme_status(port)) {
 			pm_request_resume(&port->dev);
 			found = true;
@@ -187,6 +193,9 @@ static void pcie_pme_handle_request(stru
 		/* The device is there, but we have to check its PME status. */
 		found = pci_check_pme_status(dev);
 		if (found) {
+			if (dev->pme_poll)
+				dev->pme_poll = false;
+
 			pci_wakeup_event(dev);
 			pm_request_resume(&dev->dev);
 		}

Re: PME via interrupt or SCI mechanism?

[Sarah Sharp]

On Fri, Sep 30, 2011 at 10:21:06PM +0200, Rafael J. Wysocki wrote:
> OK, please test the appended patch.  The difference is that it should only
> continuously poll devices that don't get notifications.  You'll still see
> the debug messages from your previous patches, but this one should be a bit
> less wasteful than the previous one in general.

This patch seems to work.  The GPE fires and the xHCI host controller
comes out of D3 after a stream of:

Sep 30 17:07:32 talon kernel: [  197.745312] ehci_hcd 0000:00:1d.0: pci_acpi_wake_dev
Sep 30 17:07:32 talon kernel: [  197.745321] ehci_hcd 0000:00:1a.0: pci_acpi_wake_dev
Sep 30 17:07:32 talon kernel: [  197.745326] snd_hda_intel 0000:00:1b.0: pci_acpi_wake_dev
Sep 30 17:07:32 talon kernel: [  197.745331] e1000e 0000:00:19.0: pci_acpi_wake_dev

It does seem to take less time to come out of D3 than the previous patch
though.

I occasionally do not see the host controller come out of D3 when I plug
in a new device, but the interrupt count for GPE 0D doesn't increment
when that happens, so it's probably just a BIOS or hardware bug.  Did
mention this was bleeding edge hardware?

Sarah Sharp

Re: PME via interrupt or SCI mechanism?

[Rafael J. Wysocki]

On Saturday, October 01, 2011, Sarah Sharp wrote:
> On Fri, Sep 30, 2011 at 10:21:06PM +0200, Rafael J. Wysocki wrote:
> > OK, please test the appended patch.  The difference is that it should only
> > continuously poll devices that don't get notifications.  You'll still see
> > the debug messages from your previous patches, but this one should be a bit
> > less wasteful than the previous one in general.
> 
> This patch seems to work.  The GPE fires and the xHCI host controller
> comes out of D3 after a stream of:
> 
> Sep 30 17:07:32 talon kernel: [  197.745312] ehci_hcd 0000:00:1d.0: pci_acpi_wake_dev
> Sep 30 17:07:32 talon kernel: [  197.745321] ehci_hcd 0000:00:1a.0: pci_acpi_wake_dev
> Sep 30 17:07:32 talon kernel: [  197.745326] snd_hda_intel 0000:00:1b.0: pci_acpi_wake_dev
> Sep 30 17:07:32 talon kernel: [  197.745331] e1000e 0000:00:19.0: pci_acpi_wake_dev
> 
> It does seem to take less time to come out of D3 than the previous patch
> though.
> 
> I occasionally do not see the host controller come out of D3 when I plug
> in a new device, but the interrupt count for GPE 0D doesn't increment
> when that happens, so it's probably just a BIOS or hardware bug.

I think the controller doesn't signal PME in those cases.

> Did mention this was bleeding edge hardware?

No, you didn't, but that doesn't matter a lot.  Bugs of this kind are found in
systems being sold in the market too and we need to work around them anyway. :-(

Thanks for testing the patch, I'll resent it with a changelog shortly.

Rafael