Re: Two problems with system sleep

Re: Two problems with system sleep

[Alan Stern]

On Sun, 21 Feb 2010, Rafael J. Wysocki wrote:

> > > Anyway, we need to check if control gets back to acpi_suspend_enter().

Would PM_TRACE_RTC help?  What would I have to add in order to insert
an appropriate tracepoint?


> > > BTW, does anything change if you put acpi_sleep=sci_force_enable into the
> > > kernel command line?
> > 
> > Nothing changes.  I also added "pci=use_crs", based on a suggestion in 
> > the log, but that didn't make any difference either.
> 
> The failure to resume may be graphics-related, so please try to compile i915
> statically into the kernel and set CONFIG_DRM_I915_KMS.

Odd -- when I use "make menuconfig", that part of the menu 
doesn't show up.  Navigating through "Device Drivers -> Graphics 
support" gives me this screen:

    <*> /dev/agpgart (AGP Support)  --->                             
    <*> Direct Rendering Manager (XFree86 4.1.0 and higher DRI suppor 
    -*- Lowlevel video output switch controls                        
    -*- Support for frame buffer devices  --->                       
    [ ] Backlight & LCD device support  --->                         
        Display device support  --->                                 
        Console display driver support  --->                         
    [ ] Bootup logo  --->                                            

None of the entries in drivers/gpu/drm/Kconfig show up beyond the first 
one.  I had to go in and edit .config by hand to set CONFIG_DRM, 
CONFIG_DRM_KMS_HELPER, CONFIG_DRM_I915, and CONFIG_DRM_I915_KMS all to 
"y".  Is this a bug in menuconfig?  CC'ing the mailing list...

Anyway, it didn't make any difference.  The behavior was exactly the 
same.  Pressing the power button during suspend causes an 
almost-instant hang, before the screen or anything else revives.

By concentrating on the video drivers, you may be missing part of 
the problem.  Have you considered why pressing a key on the keyboard 
doesn't wake the system up?  Nothing happens -- the power LED on the 
desktop case just keeps on blinking.

Alan Stern

Re: Two problems with system sleep

[Alan Stern]

On Sun, 21 Feb 2010, Rafael J. Wysocki wrote:

> > > Anyway, we need to check if control gets back to acpi_suspend_enter().

Would PM_TRACE_RTC help?  What would I have to add in order to insert
an appropriate tracepoint?


> > > BTW, does anything change if you put acpi_sleep=sci_force_enable into the
> > > kernel command line?
> > 
> > Nothing changes.  I also added "pci=use_crs", based on a suggestion in 
> > the log, but that didn't make any difference either.
> 
> The failure to resume may be graphics-related, so please try to compile i915
> statically into the kernel and set CONFIG_DRM_I915_KMS.

Odd -- when I use "make menuconfig", that part of the menu 
doesn't show up.  Navigating through "Device Drivers -> Graphics 
support" gives me this screen:

    <*> /dev/agpgart (AGP Support)  --->                             
    <*> Direct Rendering Manager (XFree86 4.1.0 and higher DRI suppor 
    -*- Lowlevel video output switch controls                        
    -*- Support for frame buffer devices  --->                       
    [ ] Backlight & LCD device support  --->                         
        Display device support  --->                                 
        Console display driver support  --->                         
    [ ] Bootup logo  --->                                            

None of the entries in drivers/gpu/drm/Kconfig show up beyond the first 
one.  I had to go in and edit .config by hand to set CONFIG_DRM, 
CONFIG_DRM_KMS_HELPER, CONFIG_DRM_I915, and CONFIG_DRM_I915_KMS all to 
"y".  Is this a bug in menuconfig?  CC'ing the mailing list...

Anyway, it didn't make any difference.  The behavior was exactly the 
same.  Pressing the power button during suspend causes an 
almost-instant hang, before the screen or anything else revives.

By concentrating on the video drivers, you may be missing part of 
the problem.  Have you considered why pressing a key on the keyboard 
doesn't wake the system up?  Nothing happens -- the power LED on the 
desktop case just keeps on blinking.

Alan Stern

Re: Two problems with system sleep

[Rafael J. Wysocki]

On Monday 22 February 2010, Alan Stern wrote:
> On Sun, 21 Feb 2010, Rafael J. Wysocki wrote:
> 
> > > > Anyway, we need to check if control gets back to acpi_suspend_enter().
> 
> Would PM_TRACE_RTC help?

In my opinion that's worth doing.

> What would I have to add in order to insert an appropriate tracepoint?

Please follow Documentation/power/s2ram.txt .

> > > > BTW, does anything change if you put acpi_sleep=sci_force_enable into the
> > > > kernel command line?
> > > 
> > > Nothing changes.  I also added "pci=use_crs", based on a suggestion in 
> > > the log, but that didn't make any difference either.
> > 
> > The failure to resume may be graphics-related, so please try to compile i915
> > statically into the kernel and set CONFIG_DRM_I915_KMS.
> 
> Odd -- when I use "make menuconfig", that part of the menu 
> doesn't show up.  Navigating through "Device Drivers -> Graphics 
> support" gives me this screen:
> 
>     <*> /dev/agpgart (AGP Support)  --->                             
>     <*> Direct Rendering Manager (XFree86 4.1.0 and higher DRI suppor 
>     -*- Lowlevel video output switch controls                        
>     -*- Support for frame buffer devices  --->                       
>     [ ] Backlight & LCD device support  --->                         
>         Display device support  --->                                 
>         Console display driver support  --->                         
>     [ ] Bootup logo  --->                                            
> 
> None of the entries in drivers/gpu/drm/Kconfig show up beyond the first 
> one.  I had to go in and edit .config by hand to set CONFIG_DRM, 
> CONFIG_DRM_KMS_HELPER, CONFIG_DRM_I915, and CONFIG_DRM_I915_KMS all to 
> "y".  Is this a bug in menuconfig?  CC'ing the mailing list...
> 
> Anyway, it didn't make any difference.  The behavior was exactly the 
> same.  Pressing the power button during suspend causes an 
> almost-instant hang, before the screen or anything else revives.
> 
> By concentrating on the video drivers, you may be missing part of 
> the problem.  Have you considered why pressing a key on the keyboard 
> doesn't wake the system up?  Nothing happens -- the power LED on the 
> desktop case just keeps on blinking.

We may just not set up the keyboard as a wakeup device.  The other option is
that the BIOS has a problem with resume handling, in which case I have no
idea what to do.

Rafael

Re: Two problems with system sleep

[Rafael J. Wysocki]

On Monday 22 February 2010, Alan Stern wrote:
> On Sun, 21 Feb 2010, Rafael J. Wysocki wrote:
> 
> > > > Anyway, we need to check if control gets back to acpi_suspend_enter().
> 
> Would PM_TRACE_RTC help?

In my opinion that's worth doing.

> What would I have to add in order to insert an appropriate tracepoint?

Please follow Documentation/power/s2ram.txt .

> > > > BTW, does anything change if you put acpi_sleep=sci_force_enable into the
> > > > kernel command line?
> > > 
> > > Nothing changes.  I also added "pci=use_crs", based on a suggestion in 
> > > the log, but that didn't make any difference either.
> > 
> > The failure to resume may be graphics-related, so please try to compile i915
> > statically into the kernel and set CONFIG_DRM_I915_KMS.
> 
> Odd -- when I use "make menuconfig", that part of the menu 
> doesn't show up.  Navigating through "Device Drivers -> Graphics 
> support" gives me this screen:
> 
>     <*> /dev/agpgart (AGP Support)  --->                             
>     <*> Direct Rendering Manager (XFree86 4.1.0 and higher DRI suppor 
>     -*- Lowlevel video output switch controls                        
>     -*- Support for frame buffer devices  --->                       
>     [ ] Backlight & LCD device support  --->                         
>         Display device support  --->                                 
>         Console display driver support  --->                         
>     [ ] Bootup logo  --->                                            
> 
> None of the entries in drivers/gpu/drm/Kconfig show up beyond the first 
> one.  I had to go in and edit .config by hand to set CONFIG_DRM, 
> CONFIG_DRM_KMS_HELPER, CONFIG_DRM_I915, and CONFIG_DRM_I915_KMS all to 
> "y".  Is this a bug in menuconfig?  CC'ing the mailing list...
> 
> Anyway, it didn't make any difference.  The behavior was exactly the 
> same.  Pressing the power button during suspend causes an 
> almost-instant hang, before the screen or anything else revives.
> 
> By concentrating on the video drivers, you may be missing part of 
> the problem.  Have you considered why pressing a key on the keyboard 
> doesn't wake the system up?  Nothing happens -- the power LED on the 
> desktop case just keeps on blinking.

We may just not set up the keyboard as a wakeup device.  The other option is
that the BIOS has a problem with resume handling, in which case I have no
idea what to do.

Rafael

Re: Two problems with system sleep

[Alan Stern]

On Mon, 22 Feb 2010, Rafael J. Wysocki wrote:

> On Monday 22 February 2010, Alan Stern wrote:
> > On Sun, 21 Feb 2010, Rafael J. Wysocki wrote:
> > 
> > > > > Anyway, we need to check if control gets back to acpi_suspend_enter().
> > 
> > Would PM_TRACE_RTC help?
> 
> In my opinion that's worth doing.

Here's what I got:

[    3.349334] PM: Resume from disk failed.
[    3.350060]   Magic number: 0:141:321
[    3.352583]   hash matches drivers/base/power/main.c:477
[    3.355144] tty tty46: hash matches
[    3.357742] i915 0000:00:02.0: hash matches

So it appears that the video driver is indeed the culprit.  Is there 
any way to narrow it down further?

> > By concentrating on the video drivers, you may be missing part of 
> > the problem.  Have you considered why pressing a key on the keyboard 
> > doesn't wake the system up?  Nothing happens -- the power LED on the 
> > desktop case just keeps on blinking.
> 
> We may just not set up the keyboard as a wakeup device.  The other option is
> that the BIOS has a problem with resume handling, in which case I have no
> idea what to do.

Here's my /proc/acpi/wakeup:

Device	S-state	  Status   Sysfs node
P0P4	  S4	 disabled  pci:0000:00:1e.0
MC97	  S4	 disabled  
USB1	  S4	 disabled  pci:0000:00:1d.0
USB2	  S4	 disabled  pci:0000:00:1d.1
USB3	  S4	 disabled  pci:0000:00:1d.2
USB4	  S4	 disabled  pci:0000:00:1d.3
EUSB	  S4	 disabled  pci:0000:00:1d.7
PS2K	  S4	 disabled  pnp:00:09
PS2M	  S4	 disabled  pnp:00:0a
GBEN	  S4	 disabled  

It appears that PS2K is the keyboard.  If I write "PS2K" to 
/proc/acpi/wakeup, I get:

ACPI: 'PS2M' and 'PS2K' have the same GPE, can't disable/enable one seperately

Apart from the misspelling, this doesn't bode well for making the
keyboard a wakeup device.  Furthermore, these values are not
properly tied to the values in sysfs.  For example, after

	echo enabled >/sys/devices/pci0000:00/0000:00:1d.7/power/wakeup

the EUSB line in /proc/acpi/wakeup still says disabled.

What next?

Alan Stern

Re: Two problems with system sleep

[Rafael J. Wysocki]

On Monday 22 February 2010, Alan Stern wrote:
> On Mon, 22 Feb 2010, Rafael J. Wysocki wrote:
> 
> > On Monday 22 February 2010, Alan Stern wrote:
> > > On Sun, 21 Feb 2010, Rafael J. Wysocki wrote:
> > > 
> > > > > > Anyway, we need to check if control gets back to acpi_suspend_enter().
> > > 
> > > Would PM_TRACE_RTC help?
> > 
> > In my opinion that's worth doing.
> 
> Here's what I got:
> 
> [    3.349334] PM: Resume from disk failed.
> [    3.350060]   Magic number: 0:141:321
> [    3.352583]   hash matches drivers/base/power/main.c:477
> [    3.355144] tty tty46: hash matches
> [    3.357742] i915 0000:00:02.0: hash matches
> 
> So it appears that the video driver is indeed the culprit.  Is there 
> any way to narrow it down further?

Not without adding some debug code to the driver.

Which version of the kernel is this?

> > > By concentrating on the video drivers, you may be missing part of 
> > > the problem.  Have you considered why pressing a key on the keyboard 
> > > doesn't wake the system up?  Nothing happens -- the power LED on the 
> > > desktop case just keeps on blinking.
> > 
> > We may just not set up the keyboard as a wakeup device.  The other option is
> > that the BIOS has a problem with resume handling, in which case I have no
> > idea what to do.
> 
> Here's my /proc/acpi/wakeup:
> 
> Device	S-state	  Status   Sysfs node
> P0P4	  S4	 disabled  pci:0000:00:1e.0
> MC97	  S4	 disabled  
> USB1	  S4	 disabled  pci:0000:00:1d.0
> USB2	  S4	 disabled  pci:0000:00:1d.1
> USB3	  S4	 disabled  pci:0000:00:1d.2
> USB4	  S4	 disabled  pci:0000:00:1d.3
> EUSB	  S4	 disabled  pci:0000:00:1d.7
> PS2K	  S4	 disabled  pnp:00:09
> PS2M	  S4	 disabled  pnp:00:0a
> GBEN	  S4	 disabled  
> 
> It appears that PS2K is the keyboard.  If I write "PS2K" to 
> /proc/acpi/wakeup, I get:
> 
> ACPI: 'PS2M' and 'PS2K' have the same GPE, can't disable/enable one seperately
> 
> Apart from the misspelling, this doesn't bode well for making the
> keyboard a wakeup device.

This only means both will be enabled at the same time.

> Furthermore, these values are not properly tied to the values in sysfs.

They are independent and the sysfs values probably don't matter for these
devices.  What are the contents of /proc/acpi/wakeup after writing PS2K to it?

Rafael

Re: Two problems with system sleep

[Alan Stern]

On Mon, 22 Feb 2010, Rafael J. Wysocki wrote:

> > Here's what I got:
> > 
> > [    3.349334] PM: Resume from disk failed.
> > [    3.350060]   Magic number: 0:141:321
> > [    3.352583]   hash matches drivers/base/power/main.c:477
> > [    3.355144] tty tty46: hash matches
> > [    3.357742] i915 0000:00:02.0: hash matches
> > 
> > So it appears that the video driver is indeed the culprit.  Is there 
> > any way to narrow it down further?
> 
> Not without adding some debug code to the driver.
> 
> Which version of the kernel is this?

2.6.33-rc8.  The same problem occurs with earlier versions, such as
Fedora 12's 2.6.31.9 (which is what I'm running now).

> > Here's my /proc/acpi/wakeup:
> > 
> > Device	S-state	  Status   Sysfs node
> > P0P4	  S4	 disabled  pci:0000:00:1e.0
> > MC97	  S4	 disabled  
> > USB1	  S4	 disabled  pci:0000:00:1d.0
> > USB2	  S4	 disabled  pci:0000:00:1d.1
> > USB3	  S4	 disabled  pci:0000:00:1d.2
> > USB4	  S4	 disabled  pci:0000:00:1d.3
> > EUSB	  S4	 disabled  pci:0000:00:1d.7
> > PS2K	  S4	 disabled  pnp:00:09
> > PS2M	  S4	 disabled  pnp:00:0a
> > GBEN	  S4	 disabled  
> > 
> > It appears that PS2K is the keyboard.  If I write "PS2K" to 
> > /proc/acpi/wakeup, I get:
> > 
> > ACPI: 'PS2M' and 'PS2K' have the same GPE, can't disable/enable one seperately
> > 
> > Apart from the misspelling, this doesn't bode well for making the
> > keyboard a wakeup device.
> 
> This only means both will be enabled at the same time.
> 
> > Furthermore, these values are not properly tied to the values in sysfs.
> 
> They are independent and the sysfs values probably don't matter for these
> devices.  What are the contents of /proc/acpi/wakeup after writing PS2K to it?

Unchanged -- both PS2K and PS2M continue to be disabled.

Why are the values independent from the wakeup settings in sysfs?  
Aren't they supposed to mean the same thing?

Alan Stern

Re: Two problems with system sleep

[Rafael J. Wysocki]

On Monday 22 February 2010, you wrote:
> On Mon, 22 Feb 2010, Rafael J. Wysocki wrote:
> 
> > > Here's what I got:
> > > 
> > > [    3.349334] PM: Resume from disk failed.
> > > [    3.350060]   Magic number: 0:141:321
> > > [    3.352583]   hash matches drivers/base/power/main.c:477
> > > [    3.355144] tty tty46: hash matches
> > > [    3.357742] i915 0000:00:02.0: hash matches
> > > 
> > > So it appears that the video driver is indeed the culprit.  Is there 
> > > any way to narrow it down further?
> > 
> > Not without adding some debug code to the driver.
> > 
> > Which version of the kernel is this?
> 
> 2.6.33-rc8.  The same problem occurs with earlier versions, such as
> Fedora 12's 2.6.31.9 (which is what I'm running now).

I _think_ think the i915 KMS doesn't work on your box for some reason.

Doesn the screen switch to the graphics framebuffer when booted with
vga=0?

If not, you probably need to enable framebuffer console in .config (the i915
KMS depends on that actually).

> > > Here's my /proc/acpi/wakeup:
> > > 
> > > Device	S-state	  Status   Sysfs node
> > > P0P4	  S4	 disabled  pci:0000:00:1e.0
> > > MC97	  S4	 disabled  
> > > USB1	  S4	 disabled  pci:0000:00:1d.0
> > > USB2	  S4	 disabled  pci:0000:00:1d.1
> > > USB3	  S4	 disabled  pci:0000:00:1d.2
> > > USB4	  S4	 disabled  pci:0000:00:1d.3
> > > EUSB	  S4	 disabled  pci:0000:00:1d.7
> > > PS2K	  S4	 disabled  pnp:00:09
> > > PS2M	  S4	 disabled  pnp:00:0a
> > > GBEN	  S4	 disabled  
> > > 
> > > It appears that PS2K is the keyboard.  If I write "PS2K" to 
> > > /proc/acpi/wakeup, I get:
> > > 
> > > ACPI: 'PS2M' and 'PS2K' have the same GPE, can't disable/enable one seperately
> > > 
> > > Apart from the misspelling, this doesn't bode well for making the
> > > keyboard a wakeup device.
> > 
> > This only means both will be enabled at the same time.
> > 
> > > Furthermore, these values are not properly tied to the values in sysfs.
> > 
> > They are independent and the sysfs values probably don't matter for these
> > devices.  What are the contents of /proc/acpi/wakeup after writing PS2K to it?
> 
> Unchanged -- both PS2K and PS2M continue to be disabled.

That sounds like a bug.  Please try to write 'PS2M' to it too.

> Why are the values independent from the wakeup settings in sysfs?  
> Aren't they supposed to mean the same thing?

Not really.  There are two separate flags for wakeup.  One of them is a
property of the "physical" device object (eg. PCI device structure) and that
one is set/unset via sysfs.  The other is a property of the device's ACPI
"shadow" object and is set/unset through /proc/acpi/wakeup (this mechanism
is regarded as obsolete, but it looks like some devices have not been converted
to the sysfs-based one yet).

Rafael

Re: Two problems with system sleep

[Rafael J. Wysocki]

On Monday 22 February 2010, you wrote:
> On Mon, 22 Feb 2010, Rafael J. Wysocki wrote:
> 
> > > Here's what I got:
> > > 
> > > [    3.349334] PM: Resume from disk failed.
> > > [    3.350060]   Magic number: 0:141:321
> > > [    3.352583]   hash matches drivers/base/power/main.c:477
> > > [    3.355144] tty tty46: hash matches
> > > [    3.357742] i915 0000:00:02.0: hash matches
> > > 
> > > So it appears that the video driver is indeed the culprit.  Is there 
> > > any way to narrow it down further?
> > 
> > Not without adding some debug code to the driver.
> > 
> > Which version of the kernel is this?
> 
> 2.6.33-rc8.  The same problem occurs with earlier versions, such as
> Fedora 12's 2.6.31.9 (which is what I'm running now).

I _think_ think the i915 KMS doesn't work on your box for some reason.

Doesn the screen switch to the graphics framebuffer when booted with
vga=0?

If not, you probably need to enable framebuffer console in .config (the i915
KMS depends on that actually).

> > > Here's my /proc/acpi/wakeup:
> > > 
> > > Device	S-state	  Status   Sysfs node
> > > P0P4	  S4	 disabled  pci:0000:00:1e.0
> > > MC97	  S4	 disabled  
> > > USB1	  S4	 disabled  pci:0000:00:1d.0
> > > USB2	  S4	 disabled  pci:0000:00:1d.1
> > > USB3	  S4	 disabled  pci:0000:00:1d.2
> > > USB4	  S4	 disabled  pci:0000:00:1d.3
> > > EUSB	  S4	 disabled  pci:0000:00:1d.7
> > > PS2K	  S4	 disabled  pnp:00:09
> > > PS2M	  S4	 disabled  pnp:00:0a
> > > GBEN	  S4	 disabled  
> > > 
> > > It appears that PS2K is the keyboard.  If I write "PS2K" to 
> > > /proc/acpi/wakeup, I get:
> > > 
> > > ACPI: 'PS2M' and 'PS2K' have the same GPE, can't disable/enable one seperately
> > > 
> > > Apart from the misspelling, this doesn't bode well for making the
> > > keyboard a wakeup device.
> > 
> > This only means both will be enabled at the same time.
> > 
> > > Furthermore, these values are not properly tied to the values in sysfs.
> > 
> > They are independent and the sysfs values probably don't matter for these
> > devices.  What are the contents of /proc/acpi/wakeup after writing PS2K to it?
> 
> Unchanged -- both PS2K and PS2M continue to be disabled.

That sounds like a bug.  Please try to write 'PS2M' to it too.

> Why are the values independent from the wakeup settings in sysfs?  
> Aren't they supposed to mean the same thing?

Not really.  There are two separate flags for wakeup.  One of them is a
property of the "physical" device object (eg. PCI device structure) and that
one is set/unset via sysfs.  The other is a property of the device's ACPI
"shadow" object and is set/unset through /proc/acpi/wakeup (this mechanism
is regarded as obsolete, but it looks like some devices have not been converted
to the sysfs-based one yet).

Rafael

------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: Two problems with system sleep

[Alan Stern]

On Mon, 22 Feb 2010, Rafael J. Wysocki wrote:

> On Monday 22 February 2010, you wrote:
> > On Mon, 22 Feb 2010, Rafael J. Wysocki wrote:
> > 
> > > > Here's what I got:
> > > > 
> > > > [    3.349334] PM: Resume from disk failed.
> > > > [    3.350060]   Magic number: 0:141:321
> > > > [    3.352583]   hash matches drivers/base/power/main.c:477
> > > > [    3.355144] tty tty46: hash matches
> > > > [    3.357742] i915 0000:00:02.0: hash matches
> > > > 
> > > > So it appears that the video driver is indeed the culprit.  Is there 
> > > > any way to narrow it down further?
> > > 
> > > Not without adding some debug code to the driver.
> > > 
> > > Which version of the kernel is this?
> > 
> > 2.6.33-rc8.  The same problem occurs with earlier versions, such as
> > Fedora 12's 2.6.31.9 (which is what I'm running now).
> 
> I _think_ think the i915 KMS doesn't work on your box for some reason.
> 
> Doesn the screen switch to the graphics framebuffer when booted with
> vga=0?

Yes, it does switch.

> If not, you probably need to enable framebuffer console in .config (the i915
> KMS depends on that actually).

It's already enabled.  That is, CONFIG_FB and 
CONFIG_FRAMEBUFFER_CONSOLE are both set to 'y'.

> > > > Here's my /proc/acpi/wakeup:
> > > > 
> > > > Device	S-state	  Status   Sysfs node
> > > > P0P4	  S4	 disabled  pci:0000:00:1e.0
> > > > MC97	  S4	 disabled  
> > > > USB1	  S4	 disabled  pci:0000:00:1d.0
> > > > USB2	  S4	 disabled  pci:0000:00:1d.1
> > > > USB3	  S4	 disabled  pci:0000:00:1d.2
> > > > USB4	  S4	 disabled  pci:0000:00:1d.3
> > > > EUSB	  S4	 disabled  pci:0000:00:1d.7
> > > > PS2K	  S4	 disabled  pnp:00:09
> > > > PS2M	  S4	 disabled  pnp:00:0a
> > > > GBEN	  S4	 disabled  
> > > > 
> > > > It appears that PS2K is the keyboard.  If I write "PS2K" to 
> > > > /proc/acpi/wakeup, I get:
> > > > 
> > > > ACPI: 'PS2M' and 'PS2K' have the same GPE, can't disable/enable one seperately
> > > > 
> > > > Apart from the misspelling, this doesn't bode well for making the
> > > > keyboard a wakeup device.
> > > 
> > > This only means both will be enabled at the same time.
> > > 
> > > > Furthermore, these values are not properly tied to the values in sysfs.
> > > 
> > > They are independent and the sysfs values probably don't matter for these
> > > devices.  What are the contents of /proc/acpi/wakeup after writing PS2K to it?
> > 
> > Unchanged -- both PS2K and PS2M continue to be disabled.
> 
> That sounds like a bug.  Please try to write 'PS2M' to it too.

I was wrong (don't know why).  Writing PS2K does indeed enable both.  
And sure enough, doing that allows the system to wake up in response to
a key press.  It still hangs during the video resume, though.

> > Why are the values independent from the wakeup settings in sysfs?  
> > Aren't they supposed to mean the same thing?
> 
> Not really.  There are two separate flags for wakeup.  One of them is a
> property of the "physical" device object (eg. PCI device structure) and that
> one is set/unset via sysfs.  The other is a property of the device's ACPI
> "shadow" object and is set/unset through /proc/acpi/wakeup (this mechanism
> is regarded as obsolete, but it looks like some devices have not been converted
> to the sysfs-based one yet).

It looks like the inline routines defined in include/linux/pm_wakeup.h 
should call corresponding platform-specific routines.  Apparently _no_ 
drivers have been converted in this way -- since the conversion needs 
to be part of the core.  In fact, there isn't even a platform-specific 
hook for enabling or disabling wakeup devices; we should have a 
platform_wakeup_ops structure.

Would such a thing be acceptable?

Alan Stern

Re: Two problems with system sleep

[Rafael J. Wysocki]

On Tuesday 23 February 2010, Alan Stern wrote:
> On Mon, 22 Feb 2010, Rafael J. Wysocki wrote:
> 
> > On Monday 22 February 2010, you wrote:
> > > On Mon, 22 Feb 2010, Rafael J. Wysocki wrote:
> > > 
> > > > > Here's what I got:
> > > > > 
> > > > > [    3.349334] PM: Resume from disk failed.
> > > > > [    3.350060]   Magic number: 0:141:321
> > > > > [    3.352583]   hash matches drivers/base/power/main.c:477
> > > > > [    3.355144] tty tty46: hash matches
> > > > > [    3.357742] i915 0000:00:02.0: hash matches
> > > > > 
> > > > > So it appears that the video driver is indeed the culprit.  Is there 
> > > > > any way to narrow it down further?
> > > > 
> > > > Not without adding some debug code to the driver.
> > > > 
> > > > Which version of the kernel is this?
> > > 
> > > 2.6.33-rc8.  The same problem occurs with earlier versions, such as
> > > Fedora 12's 2.6.31.9 (which is what I'm running now).
> > 
> > I _think_ think the i915 KMS doesn't work on your box for some reason.
> > 
> > Doesn the screen switch to the graphics framebuffer when booted with
> > vga=0?
> 
> Yes, it does switch.

OK

> > If not, you probably need to enable framebuffer console in .config (the i915
> > KMS depends on that actually).
> 
> It's already enabled.  That is, CONFIG_FB and 
> CONFIG_FRAMEBUFFER_CONSOLE are both set to 'y'.

Well, so the KMS suspend-resume doesn't work on your system.

I guess that would require some detailed debugging, so it may be a good idea to
open a Bugzilla entry for this issue.

...
> I was wrong (don't know why).  Writing PS2K does indeed enable both.  
> And sure enough, doing that allows the system to wake up in response to
> a key press.  It still hangs during the video resume, though.
> 
> > > Why are the values independent from the wakeup settings in sysfs?  
> > > Aren't they supposed to mean the same thing?
> > 
> > Not really.  There are two separate flags for wakeup.  One of them is a
> > property of the "physical" device object (eg. PCI device structure) and that
> > one is set/unset via sysfs.  The other is a property of the device's ACPI
> > "shadow" object and is set/unset through /proc/acpi/wakeup (this mechanism
> > is regarded as obsolete, but it looks like some devices have not been converted
> > to the sysfs-based one yet).
> 
> It looks like the inline routines defined in include/linux/pm_wakeup.h 
> should call corresponding platform-specific routines.  Apparently _no_ 
> drivers have been converted in this way -- since the conversion needs 
> to be part of the core.  In fact, there isn't even a platform-specific 
> hook for enabling or disabling wakeup devices; we should have a 
> platform_wakeup_ops structure.

There is one for PCI devices, actually.  It's struct pci_platform_pm_ops()
defined in drivers/pci/pci.h.

For PCI devices we have pci_enable_wake() that sets up the platform to
wake up the system using given device on the basis of the device's sysfs
setting.  The ACPI flag shown by /proc/acpi/wakeup is not taken into account
in that case.

Generally, you can think of two levels one can enable wakeup at.
First, there is the device level controlled by the sysfs wakeup setting.  This
is how we would like to control wakeup.  Second, however, there is the
/proc/acpi/wakeup interface allowing you to access the _internal_ ACPI
representation of devices directly and you can use that to set up the platform
to use the device for wakeup even if the device level mechanism doesn't work
or is not implemented for it (like in your case).

IOW, the /proc/acpi/wakeup thing only tells us if the device is forced to do
the wakeup at the low level and it only takes the ACPI aspect of the wakeup
into account, which may not be sufficient.  For exmaple, network adapters
(and other PCI devices) generally need to have the PME signaling enabled
in addition to the platform setup and the chip has to be enabled to take
wakeup packets from the network.

If /proc/acpi/wakeup shows "enabled" this means "turn on the wakeup power
for this devices and enable its wakeup GPE unconditionally before suspend".
If it shows "disabled", it means "leave that to someone else" (that's analogous
to the "on" and "auto" settings for devices/.../power/control, but the names
are misleading for historical reasons).

So, it looks like we need a counterpart of pci_enable_wake() for PNP devices. 

I guess it's better to invite Dmitry into the discussion at this point.

Rafael

Re: Two problems with system sleep

[Alan Stern]

On Tue, 23 Feb 2010, Rafael J. Wysocki wrote:

> > > I _think_ think the i915 KMS doesn't work on your box for some reason.
> > > 
> > > Doesn the screen switch to the graphics framebuffer when booted with
> > > vga=0?
> > 
> > Yes, it does switch.
> 
> OK
> 
> > > If not, you probably need to enable framebuffer console in .config (the i915
> > > KMS depends on that actually).
> > 
> > It's already enabled.  That is, CONFIG_FB and 
> > CONFIG_FRAMEBUFFER_CONSOLE are both set to 'y'.
> 
> Well, so the KMS suspend-resume doesn't work on your system.
> 
> I guess that would require some detailed debugging, so it may be a good idea to
> open a Bugzilla entry for this issue.

Okay, I'll set it up later.

> > > > Why are the values independent from the wakeup settings in sysfs?  
> > > > Aren't they supposed to mean the same thing?
> > > 
> > > Not really.  There are two separate flags for wakeup.  One of them is a
> > > property of the "physical" device object (eg. PCI device structure) and that
> > > one is set/unset via sysfs.  The other is a property of the device's ACPI
> > > "shadow" object and is set/unset through /proc/acpi/wakeup (this mechanism
> > > is regarded as obsolete, but it looks like some devices have not been converted
> > > to the sysfs-based one yet).
> > 
> > It looks like the inline routines defined in include/linux/pm_wakeup.h 
> > should call corresponding platform-specific routines.  Apparently _no_ 
> > drivers have been converted in this way -- since the conversion needs 
> > to be part of the core.  In fact, there isn't even a platform-specific 
> > hook for enabling or disabling wakeup devices; we should have a 
> > platform_wakeup_ops structure.
> 
> There is one for PCI devices, actually.  It's struct pci_platform_pm_ops()
> defined in drivers/pci/pci.h.
> 
> For PCI devices we have pci_enable_wake() that sets up the platform to
> wake up the system using given device on the basis of the device's sysfs
> setting.  The ACPI flag shown by /proc/acpi/wakeup is not taken into account
> in that case.

That's not what I meant.  Writing to /sys/devices/.../power/wakeup 
should have the same effect as writing to /proc/acpi/wakeup (if the 
device has a "shadow" ACPI counterpart, of course).  It looks like 
/proc/acpi/wakeup does nothing but set acpidev->wakeup.state.enabled 
and warn about duplicate GPE usage.

It would make sense to issue the warning when writing to the sysfs
file.  And wakeup.state.enabled shouldn't be needed at all; the ACPI
code should always use the physical device's may_wakeup flag instead
(unless there is no corresponding physical device).

If writes to the sysfs file called a platform hook, we could warn about 
duplicate GPEs when the wakeup setting is changed.

> Generally, you can think of two levels one can enable wakeup at.
> First, there is the device level controlled by the sysfs wakeup setting.  This
> is how we would like to control wakeup.  Second, however, there is the
> /proc/acpi/wakeup interface allowing you to access the _internal_ ACPI
> representation of devices directly and you can use that to set up the platform
> to use the device for wakeup even if the device level mechanism doesn't work
> or is not implemented for it (like in your case).
> 
> IOW, the /proc/acpi/wakeup thing only tells us if the device is forced to do
> the wakeup at the low level and it only takes the ACPI aspect of the wakeup
> into account, which may not be sufficient.  For exmaple, network adapters
> (and other PCI devices) generally need to have the PME signaling enabled
> in addition to the platform setup and the chip has to be enabled to take
> wakeup packets from the network.
> 
> If /proc/acpi/wakeup shows "enabled" this means "turn on the wakeup power
> for this devices and enable its wakeup GPE unconditionally before suspend".
> If it shows "disabled", it means "leave that to someone else" (that's analogous
> to the "on" and "auto" settings for devices/.../power/control, but the names
> are misleading for historical reasons).
> 
> So, it looks like we need a counterpart of pci_enable_wake() for PNP devices. 

That sounds reasonable.  ACPI could be made to work without such a 
thing, but other platforms probably would need it.

Alan Stern

Re: Two problems with system sleep

[Dmitry Torokhov]

On Tue, Feb 23, 2010 at 10:02:04PM +0100, Rafael J. Wysocki wrote:
> On Tuesday 23 February 2010, Alan Stern wrote:
> > On Mon, 22 Feb 2010, Rafael J. Wysocki wrote:
> > 
> > > On Monday 22 February 2010, you wrote:
> > > > On Mon, 22 Feb 2010, Rafael J. Wysocki wrote:
> > > > 
> > > > > > Here's what I got:
> > > > > > 
> > > > > > [    3.349334] PM: Resume from disk failed.
> > > > > > [    3.350060]   Magic number: 0:141:321
> > > > > > [    3.352583]   hash matches drivers/base/power/main.c:477
> > > > > > [    3.355144] tty tty46: hash matches
> > > > > > [    3.357742] i915 0000:00:02.0: hash matches
> > > > > > 
> > > > > > So it appears that the video driver is indeed the culprit.  Is there 
> > > > > > any way to narrow it down further?
> > > > > 
> > > > > Not without adding some debug code to the driver.
> > > > > 
> > > > > Which version of the kernel is this?
> > > > 
> > > > 2.6.33-rc8.  The same problem occurs with earlier versions, such as
> > > > Fedora 12's 2.6.31.9 (which is what I'm running now).
> > > 
> > > I _think_ think the i915 KMS doesn't work on your box for some reason.
> > > 
> > > Doesn the screen switch to the graphics framebuffer when booted with
> > > vga=0?
> > 
> > Yes, it does switch.
> 
> OK
> 
> > > If not, you probably need to enable framebuffer console in .config (the i915
> > > KMS depends on that actually).
> > 
> > It's already enabled.  That is, CONFIG_FB and 
> > CONFIG_FRAMEBUFFER_CONSOLE are both set to 'y'.
> 
> Well, so the KMS suspend-resume doesn't work on your system.
> 
> I guess that would require some detailed debugging, so it may be a good idea to
> open a Bugzilla entry for this issue.
> 
> ...
> > I was wrong (don't know why).  Writing PS2K does indeed enable both.  
> > And sure enough, doing that allows the system to wake up in response to
> > a key press.  It still hangs during the video resume, though.
> > 
> > > > Why are the values independent from the wakeup settings in sysfs?  
> > > > Aren't they supposed to mean the same thing?
> > > 
> > > Not really.  There are two separate flags for wakeup.  One of them is a
> > > property of the "physical" device object (eg. PCI device structure) and that
> > > one is set/unset via sysfs.  The other is a property of the device's ACPI
> > > "shadow" object and is set/unset through /proc/acpi/wakeup (this mechanism
> > > is regarded as obsolete, but it looks like some devices have not been converted
> > > to the sysfs-based one yet).
> > 
> > It looks like the inline routines defined in include/linux/pm_wakeup.h 
> > should call corresponding platform-specific routines.  Apparently _no_ 
> > drivers have been converted in this way -- since the conversion needs 
> > to be part of the core.  In fact, there isn't even a platform-specific 
> > hook for enabling or disabling wakeup devices; we should have a 
> > platform_wakeup_ops structure.
> 
> There is one for PCI devices, actually.  It's struct pci_platform_pm_ops()
> defined in drivers/pci/pci.h.
> 
> For PCI devices we have pci_enable_wake() that sets up the platform to
> wake up the system using given device on the basis of the device's sysfs
> setting.  The ACPI flag shown by /proc/acpi/wakeup is not taken into account
> in that case.
> 
> Generally, you can think of two levels one can enable wakeup at.
> First, there is the device level controlled by the sysfs wakeup setting.  This
> is how we would like to control wakeup.  Second, however, there is the
> /proc/acpi/wakeup interface allowing you to access the _internal_ ACPI
> representation of devices directly and you can use that to set up the platform
> to use the device for wakeup even if the device level mechanism doesn't work
> or is not implemented for it (like in your case).
> 
> IOW, the /proc/acpi/wakeup thing only tells us if the device is forced to do
> the wakeup at the low level and it only takes the ACPI aspect of the wakeup
> into account, which may not be sufficient.  For exmaple, network adapters
> (and other PCI devices) generally need to have the PME signaling enabled
> in addition to the platform setup and the chip has to be enabled to take
> wakeup packets from the network.
> 
> If /proc/acpi/wakeup shows "enabled" this means "turn on the wakeup power
> for this devices and enable its wakeup GPE unconditionally before suspend".
> If it shows "disabled", it means "leave that to someone else" (that's analogous
> to the "on" and "auto" settings for devices/.../power/control, but the names
> are misleading for historical reasons).
> 
> So, it looks like we need a counterpart of pci_enable_wake() for PNP devices. 
> 
> I guess it's better to invite Dmitry into the discussion at this point.
> 

Yes, I agree, we need a genric mechanism for PNP to emable wakups. It
was discussed a bit here:

	http://bugzilla.kernel.org/show_bug.cgi?id=8286

but David was too hung up on the fact that number of devices in ACPI
does not map directly onto number of serio ports when i8042 is in active
multiplexing mode that it id not go anywhere.

-- 
Dmitry

Re: Two problems with system sleep

[Rafael J. Wysocki]

On Tuesday 23 February 2010, Alan Stern wrote:
> On Tue, 23 Feb 2010, Rafael J. Wysocki wrote:
> 
> > > > I _think_ think the i915 KMS doesn't work on your box for some reason.
> > > > 
> > > > Doesn the screen switch to the graphics framebuffer when booted with
> > > > vga=0?
> > > 
> > > Yes, it does switch.
> > 
> > OK
> > 
> > > > If not, you probably need to enable framebuffer console in .config (the i915
> > > > KMS depends on that actually).
> > > 
> > > It's already enabled.  That is, CONFIG_FB and 
> > > CONFIG_FRAMEBUFFER_CONSOLE are both set to 'y'.
> > 
> > Well, so the KMS suspend-resume doesn't work on your system.
> > 
> > I guess that would require some detailed debugging, so it may be a good idea to
> > open a Bugzilla entry for this issue.
> 
> Okay, I'll set it up later.
> 
> > > > > Why are the values independent from the wakeup settings in sysfs?  
> > > > > Aren't they supposed to mean the same thing?
> > > > 
> > > > Not really.  There are two separate flags for wakeup.  One of them is a
> > > > property of the "physical" device object (eg. PCI device structure) and that
> > > > one is set/unset via sysfs.  The other is a property of the device's ACPI
> > > > "shadow" object and is set/unset through /proc/acpi/wakeup (this mechanism
> > > > is regarded as obsolete, but it looks like some devices have not been converted
> > > > to the sysfs-based one yet).
> > > 
> > > It looks like the inline routines defined in include/linux/pm_wakeup.h 
> > > should call corresponding platform-specific routines.  Apparently _no_ 
> > > drivers have been converted in this way -- since the conversion needs 
> > > to be part of the core.  In fact, there isn't even a platform-specific 
> > > hook for enabling or disabling wakeup devices; we should have a 
> > > platform_wakeup_ops structure.
> > 
> > There is one for PCI devices, actually.  It's struct pci_platform_pm_ops()
> > defined in drivers/pci/pci.h.
> > 
> > For PCI devices we have pci_enable_wake() that sets up the platform to
> > wake up the system using given device on the basis of the device's sysfs
> > setting.  The ACPI flag shown by /proc/acpi/wakeup is not taken into account
> > in that case.
> 
> That's not what I meant.  Writing to /sys/devices/.../power/wakeup 
> should have the same effect as writing to /proc/acpi/wakeup (if the 
> device has a "shadow" ACPI counterpart, of course).

That really is not worth it.  The information about the sharing of a GPE is
only useful for debugging purposes and only in "really broken" cases.

> It looks like /proc/acpi/wakeup does nothing but set
> acpidev->wakeup.state.enabled and warn about duplicate GPE usage.

That's correct, although the warning itself doesn't really make sense,
because the sharing of the GPE doesn't mean we _have_ _to_ enable both devices
to wake up (the devices usually require some more preparations than just
setting the GPE).

> It would make sense to issue the warning when writing to the sysfs file.

That's not worth the added code complexity IMO.

> And wakeup.state.enabled shouldn't be needed at all;

Right, but it was there before.

> the ACPI code should always use the physical device's may_wakeup flag instead
> (unless there is no corresponding physical device).

No, it shouldn't, because it doesn't know if the device has actually been set
up for wakeup at its bus type level.

> If writes to the sysfs file called a platform hook, we could warn about 
> duplicate GPEs when the wakeup setting is changed.

As I wrote above, this is not worth it IMO.  Moreover, that doesn't really
matter, because the GPEs will be enabled as needed anyway and there's no real
need to tell the user which of them are shared except for debugging (at the
fairly low level).  Furthermore, the GPE refrence counting code we're going to
add shortly changes that quite a bit.

Generally speaking, I'd like to get rid of /proc/acpi/wakeup altogether or
maybe leave it as read-only without the "disabled/enabled" column.

However, there still are devices that don't have an alternative mechanism for
enabling wakeup, so we can't just drop it right now.

Rafael

Re: Two problems with system sleep

[Alan Stern]

On Tue, 23 Feb 2010, Dmitry Torokhov wrote:

> Yes, I agree, we need a genric mechanism for PNP to emable wakups. It
> was discussed a bit here:
> 
> 	http://bugzilla.kernel.org/show_bug.cgi?id=8286
> 
> but David was too hung up on the fact that number of devices in ACPI
> does not map directly onto number of serio ports when i8042 is in active
> multiplexing mode that it id not go anywhere.

I tend to agree with your views.  Although there still is a question
about why my system has both /sys/devices/pnp0/00:09 and
/sys/devices/platform/i8042/serio0.  Since they refer to the same
keyboard device, why should there be two different sysfs nodes and two 
different drivers?

(Somewhat tangentially, why do the "i8042 aux" and "i8042 kbd"  
drivers on the pnp bus have names containing space characters?  Isn't
that likely to mess up shell scripts?)

At any rate, can the wakeup matter get fixed?  On my system I see two
issues related to the keyboard/mouse devices.  The first is that they
are not enabled for wakeup by default.  The second is that doing

	echo enabled >/sys/devices/pnp0/00:09/power/wakeup

does not actually enable the keyboard for wakeup; only writing to 
/proc/acpi/wakeup works.

Rafael's suggestion for a PNP analog of pci_enable_wake() should solve 
the second issue.  I don't know what's needed to solve the first.

Alan Stern

Re: Two problems with system sleep

[Alan Stern]

On Wed, 24 Feb 2010, Rafael J. Wysocki wrote:

> > > Well, so the KMS suspend-resume doesn't work on your system.
> > > 
> > > I guess that would require some detailed debugging, so it may be a good idea to
> > > open a Bugzilla entry for this issue.
> > 
> > Okay, I'll set it up later.

Created as Bugzilla #15385.  I added you to the CC: list.  Further 
communication should be CC'ed to linux-pm and dri-devel.

Alan Stern

[RFC] Wakeup for PNP

[Alan Stern]

On Tue, 23 Feb 2010, Dmitry Torokhov wrote:

> Yes, I agree, we need a genric mechanism for PNP to emable wakups. It
> was discussed a bit here:
> 
> 	http://bugzilla.kernel.org/show_bug.cgi?id=8286
> 
> but David was too hung up on the fact that number of devices in ACPI
> does not map directly onto number of serio ports when i8042 is in active
> multiplexing mode that it id not go anywhere.

Does this look reasonable?  I don't know anything about PNPBIOS or 
ISAPNP, so it handles only PNPACPI.  But at least it's a starting 
point -- and it does enable my system to wake up in response to 
hitting a key.

(This combines changes to the PNP core with changes to the i8042 
drivers.  For submission they can be broken out into separate patches.)

Alan Stern


Index: usb-2.6/drivers/input/serio/i8042-x86ia64io.h
===================================================================
--- usb-2.6.orig/drivers/input/serio/i8042-x86ia64io.h
+++ usb-2.6/drivers/input/serio/i8042-x86ia64io.h
@@ -625,6 +625,7 @@ static int i8042_pnp_kbd_probe(struct pn
 	}
 
 	i8042_pnp_kbd_devices++;
+	device_set_wakeup_enable(&dev->dev, true);
 	return 0;
 }
 
@@ -646,6 +647,7 @@ static int i8042_pnp_aux_probe(struct pn
 	}
 
 	i8042_pnp_aux_devices++;
+	device_set_wakeup_enable(&dev->dev, true);
 	return 0;
 }
 
@@ -656,7 +658,7 @@ static struct pnp_device_id pnp_kbd_devi
 };
 
 static struct pnp_driver i8042_pnp_kbd_driver = {
-	.name           = "i8042 kbd",
+	.name           = "i8042-kbd",
 	.id_table       = pnp_kbd_devids,
 	.probe          = i8042_pnp_kbd_probe,
 };
@@ -676,7 +678,7 @@ static struct pnp_device_id pnp_aux_devi
 };
 
 static struct pnp_driver i8042_pnp_aux_driver = {
-	.name           = "i8042 aux",
+	.name           = "i8042-aux",
 	.id_table       = pnp_aux_devids,
 	.probe          = i8042_pnp_aux_probe,
 };
Index: usb-2.6/drivers/pnp/core.c
===================================================================
--- usb-2.6.orig/drivers/pnp/core.c
+++ usb-2.6/drivers/pnp/core.c
@@ -164,6 +164,9 @@ int __pnp_add_device(struct pnp_dev *dev
 	list_add_tail(&dev->global_list, &pnp_global);
 	list_add_tail(&dev->protocol_list, &dev->protocol->devices);
 	spin_unlock(&pnp_lock);
+	device_set_wakeup_capable(&dev->dev,
+			dev->protocol->can_wakeup &&
+			dev->protocol->can_wakeup(dev));
 	return device_register(&dev->dev);
 }
 
Index: usb-2.6/drivers/pnp/pnpacpi/core.c
===================================================================
--- usb-2.6.orig/drivers/pnp/pnpacpi/core.c
+++ usb-2.6/drivers/pnp/pnpacpi/core.c
@@ -121,12 +121,27 @@ static int pnpacpi_disable_resources(str
 }
 
 #ifdef CONFIG_ACPI_SLEEP
+static bool pnpacpi_can_wakeup(struct pnp_dev *dev)
+{
+	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
+
+	return handle && acpi_bus_can_wakeup(handle);
+}
+
 static int pnpacpi_suspend(struct pnp_dev *dev, pm_message_t state)
 {
 	struct acpi_device *acpi_dev = dev->data;
 	acpi_handle handle = acpi_dev->handle;
 	int power_state;
 
+	if (device_can_wakeup(&dev->dev)) {
+		int ret;
+
+		ret = acpi_pm_device_sleep_wake(&dev->dev,
+				device_may_wakeup(&dev->dev));
+		if (ret)
+			return ret;
+	}
 	power_state = acpi_pm_device_sleep_state(&dev->dev, NULL);
 	if (power_state < 0)
 		power_state = (state.event == PM_EVENT_ON) ?
@@ -140,6 +155,8 @@ static int pnpacpi_resume(struct pnp_dev
 	struct acpi_device *acpi_dev = dev->data;
 	acpi_handle handle = acpi_dev->handle;
 
+	if (device_can_wakeup(&dev->dev))
+		acpi_pm_device_sleep_wake(&dev->dev, false);
 	return acpi_bus_set_power(handle, ACPI_STATE_D0);
 }
 #endif
@@ -150,6 +167,7 @@ struct pnp_protocol pnpacpi_protocol = {
 	.set	 = pnpacpi_set_resources,
 	.disable = pnpacpi_disable_resources,
 #ifdef CONFIG_ACPI_SLEEP
+	.can_wakeup = pnpacpi_can_wakeup,
 	.suspend = pnpacpi_suspend,
 	.resume = pnpacpi_resume,
 #endif
Index: usb-2.6/include/linux/pnp.h
===================================================================
--- usb-2.6.orig/include/linux/pnp.h
+++ usb-2.6/include/linux/pnp.h
@@ -414,6 +414,7 @@ struct pnp_protocol {
 	int (*disable) (struct pnp_dev *dev);
 
 	/* protocol specific suspend/resume */
+	bool (*can_wakeup) (struct pnp_dev *dev);
 	int (*suspend) (struct pnp_dev * dev, pm_message_t state);
 	int (*resume) (struct pnp_dev * dev);
 

Re: [RFC] Wakeup for PNP

[Bjorn Helgaas]

On Tuesday 02 March 2010 01:13:30 pm Alan Stern wrote:
> On Tue, 23 Feb 2010, Dmitry Torokhov wrote:
> 
> > Yes, I agree, we need a genric mechanism for PNP to emable wakups. It
> > was discussed a bit here:
> > 
> > 	http://bugzilla.kernel.org/show_bug.cgi?id=8286
> > 
> > but David was too hung up on the fact that number of devices in ACPI
> > does not map directly onto number of serio ports when i8042 is in active
> > multiplexing mode that it id not go anywhere.
> 
> Does this look reasonable?  I don't know anything about PNPBIOS or 
> ISAPNP, so it handles only PNPACPI.  But at least it's a starting 
> point -- and it does enable my system to wake up in response to 
> hitting a key.

I don't know much about power management, but your patch looks
reasonable to me.

> --- usb-2.6.orig/drivers/pnp/pnpacpi/core.c
> +++ usb-2.6/drivers/pnp/pnpacpi/core.c
> @@ -121,12 +121,27 @@ static int pnpacpi_disable_resources(str
>  }
>  
>  #ifdef CONFIG_ACPI_SLEEP
> +static bool pnpacpi_can_wakeup(struct pnp_dev *dev)
> +{
> +	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);

I would have used:

	struct acpi_device *acpi_dev = dev->data;
	acpi_handle handle = acpi_dev->handle;

here because that's what the rest of the PNPACPI code does.

Bjorn

Re: [RFC] Wakeup for PNP

[Bjorn Helgaas]

On Tuesday 02 March 2010 01:13:30 pm Alan Stern wrote:
> On Tue, 23 Feb 2010, Dmitry Torokhov wrote:
> 
> > Yes, I agree, we need a genric mechanism for PNP to emable wakups. It
> > was discussed a bit here:
> > 
> > 	http://bugzilla.kernel.org/show_bug.cgi?id=8286
> > 
> > but David was too hung up on the fact that number of devices in ACPI
> > does not map directly onto number of serio ports when i8042 is in active
> > multiplexing mode that it id not go anywhere.
> 
> Does this look reasonable?  I don't know anything about PNPBIOS or 
> ISAPNP, so it handles only PNPACPI.  But at least it's a starting 
> point -- and it does enable my system to wake up in response to 
> hitting a key.

I don't know much about power management, but your patch looks
reasonable to me.

> --- usb-2.6.orig/drivers/pnp/pnpacpi/core.c
> +++ usb-2.6/drivers/pnp/pnpacpi/core.c
> @@ -121,12 +121,27 @@ static int pnpacpi_disable_resources(str
>  }
>  
>  #ifdef CONFIG_ACPI_SLEEP
> +static bool pnpacpi_can_wakeup(struct pnp_dev *dev)
> +{
> +	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);

I would have used:

	struct acpi_device *acpi_dev = dev->data;
	acpi_handle handle = acpi_dev->handle;

here because that's what the rest of the PNPACPI code does.

Bjorn

Re: [RFC] Wakeup for PNP

[Alan Stern]

On Tue, 2 Mar 2010, Bjorn Helgaas wrote:

> > Does this look reasonable?  I don't know anything about PNPBIOS or 
> > ISAPNP, so it handles only PNPACPI.  But at least it's a starting 
> > point -- and it does enable my system to wake up in response to 
> > hitting a key.
> 
> I don't know much about power management, but your patch looks
> reasonable to me.

Thanks.

> > --- usb-2.6.orig/drivers/pnp/pnpacpi/core.c
> > +++ usb-2.6/drivers/pnp/pnpacpi/core.c
> > @@ -121,12 +121,27 @@ static int pnpacpi_disable_resources(str
> >  }
> >  
> >  #ifdef CONFIG_ACPI_SLEEP
> > +static bool pnpacpi_can_wakeup(struct pnp_dev *dev)
> > +{
> > +	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
> 
> I would have used:
> 
> 	struct acpi_device *acpi_dev = dev->data;
> 	acpi_handle handle = acpi_dev->handle;
> 
> here because that's what the rest of the PNPACPI code does.

Ah yes.  I just copied the code from the corresponding PCI function 
without thinking about it very much.  I'll update the patch and wait to 
hear from other people.

Alan Stern

Re: [RFC] Wakeup for PNP

[Alan Stern]

On Tue, 2 Mar 2010, Bjorn Helgaas wrote:

> > Does this look reasonable?  I don't know anything about PNPBIOS or 
> > ISAPNP, so it handles only PNPACPI.  But at least it's a starting 
> > point -- and it does enable my system to wake up in response to 
> > hitting a key.
> 
> I don't know much about power management, but your patch looks
> reasonable to me.

Thanks.

> > --- usb-2.6.orig/drivers/pnp/pnpacpi/core.c
> > +++ usb-2.6/drivers/pnp/pnpacpi/core.c
> > @@ -121,12 +121,27 @@ static int pnpacpi_disable_resources(str
> >  }
> >  
> >  #ifdef CONFIG_ACPI_SLEEP
> > +static bool pnpacpi_can_wakeup(struct pnp_dev *dev)
> > +{
> > +	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
> 
> I would have used:
> 
> 	struct acpi_device *acpi_dev = dev->data;
> 	acpi_handle handle = acpi_dev->handle;
> 
> here because that's what the rest of the PNPACPI code does.

Ah yes.  I just copied the code from the corresponding PCI function 
without thinking about it very much.  I'll update the patch and wait to 
hear from other people.

Alan Stern

Re: [RFC] Wakeup for PNP

[Dmitry Torokhov]

On Tue, Mar 02, 2010 at 03:13:30PM -0500, Alan Stern wrote:
> On Tue, 23 Feb 2010, Dmitry Torokhov wrote:
> 
> > Yes, I agree, we need a genric mechanism for PNP to emable wakups. It
> > was discussed a bit here:
> > 
> > 	http://bugzilla.kernel.org/show_bug.cgi?id=8286
> > 
> > but David was too hung up on the fact that number of devices in ACPI
> > does not map directly onto number of serio ports when i8042 is in active
> > multiplexing mode that it id not go anywhere.
> 
> Does this look reasonable?  I don't know anything about PNPBIOS or 
> ISAPNP, so it handles only PNPACPI.  But at least it's a starting 
> point -- and it does enable my system to wake up in response to 
> hitting a key.
> 
> (This combines changes to the PNP core with changes to the i8042 
> drivers.  For submission they can be broken out into separate patches.)
> 
> Alan Stern
> 
> 
> Index: usb-2.6/drivers/input/serio/i8042-x86ia64io.h
> ===================================================================
> --- usb-2.6.orig/drivers/input/serio/i8042-x86ia64io.h
> +++ usb-2.6/drivers/input/serio/i8042-x86ia64io.h
> @@ -625,6 +625,7 @@ static int i8042_pnp_kbd_probe(struct pn
>  	}
>  
>  	i8042_pnp_kbd_devices++;
> +	device_set_wakeup_enable(&dev->dev, true);
>  	return 0;
>  }
>  
> @@ -646,6 +647,7 @@ static int i8042_pnp_aux_probe(struct pn
>  	}
>  
>  	i8042_pnp_aux_devices++;
> +	device_set_wakeup_enable(&dev->dev, true);
>  	return 0;
>  }
>  
> @@ -656,7 +658,7 @@ static struct pnp_device_id pnp_kbd_devi
>  };
>  
>  static struct pnp_driver i8042_pnp_kbd_driver = {
> -	.name           = "i8042 kbd",
> +	.name           = "i8042-kbd",

Why is this needed? I don't think spaces are more dangerous than a colon
which we do use...

Other than that - looks reasonable to me...

-- 
Dmitry

Re: [RFC] Wakeup for PNP

[Dmitry Torokhov]

On Tue, Mar 02, 2010 at 03:13:30PM -0500, Alan Stern wrote:
> On Tue, 23 Feb 2010, Dmitry Torokhov wrote:
> 
> > Yes, I agree, we need a genric mechanism for PNP to emable wakups. It
> > was discussed a bit here:
> > 
> > 	http://bugzilla.kernel.org/show_bug.cgi?id=8286
> > 
> > but David was too hung up on the fact that number of devices in ACPI
> > does not map directly onto number of serio ports when i8042 is in active
> > multiplexing mode that it id not go anywhere.
> 
> Does this look reasonable?  I don't know anything about PNPBIOS or 
> ISAPNP, so it handles only PNPACPI.  But at least it's a starting 
> point -- and it does enable my system to wake up in response to 
> hitting a key.
> 
> (This combines changes to the PNP core with changes to the i8042 
> drivers.  For submission they can be broken out into separate patches.)
> 
> Alan Stern
> 
> 
> Index: usb-2.6/drivers/input/serio/i8042-x86ia64io.h
> ===================================================================
> --- usb-2.6.orig/drivers/input/serio/i8042-x86ia64io.h
> +++ usb-2.6/drivers/input/serio/i8042-x86ia64io.h
> @@ -625,6 +625,7 @@ static int i8042_pnp_kbd_probe(struct pn
>  	}
>  
>  	i8042_pnp_kbd_devices++;
> +	device_set_wakeup_enable(&dev->dev, true);
>  	return 0;
>  }
>  
> @@ -646,6 +647,7 @@ static int i8042_pnp_aux_probe(struct pn
>  	}
>  
>  	i8042_pnp_aux_devices++;
> +	device_set_wakeup_enable(&dev->dev, true);
>  	return 0;
>  }
>  
> @@ -656,7 +658,7 @@ static struct pnp_device_id pnp_kbd_devi
>  };
>  
>  static struct pnp_driver i8042_pnp_kbd_driver = {
> -	.name           = "i8042 kbd",
> +	.name           = "i8042-kbd",

Why is this needed? I don't think spaces are more dangerous than a colon
which we do use...

Other than that - looks reasonable to me...

-- 
Dmitry

Re: [RFC] Wakeup for PNP

[Alan Stern]

On Tue, 2 Mar 2010, Dmitry Torokhov wrote:

> > @@ -656,7 +658,7 @@ static struct pnp_device_id pnp_kbd_devi
> >  };
> >  
> >  static struct pnp_driver i8042_pnp_kbd_driver = {
> > -	.name           = "i8042 kbd",
> > +	.name           = "i8042-kbd",
> 
> Why is this needed? I don't think spaces are more dangerous than a colon
> which we do use...

This string becomes the name of a directory in sysfs.  Having a ' '
character in a directory name just doesn't seem like a good idea; at
the very least it's likely to confuse shell scripts.  (A ':' character
is a lot less subject to misinterpretation.)  I could change the '-' to 
':' if you prefer.

> Other than that - looks reasonable to me...

Thanks.  I'll separate out the driver name change from the rest of the 
patch.

There are a couple of questions remaining.  The patch enables wakeup on 
the i8042-aux device -- is this a good idea?  I haven't tested that 
part of it yet, but we wouldn't want suspended systems waking up just 
because somebody moves the mouse around.  On the other hand, on some 
machines (like mine!) it may not be possible to enable keyboard-wakeup 
without also enabling mouse-wakeup, since they use the same GPE.

Also, the patch effectively tells the kernel that ISAPNP and PNPBIOS 
devices are incapable of issuing wakeup requests.  Is that going to 
cause trouble for people?  Would it be better to change the patch so 
that it doesn't call device_set_wakeup_capable() if 
dev->protocol->can_wakeup isn't defined?

Alan Stern

Re: [RFC] Wakeup for PNP

[Alan Stern]

On Tue, 2 Mar 2010, Dmitry Torokhov wrote:

> > @@ -656,7 +658,7 @@ static struct pnp_device_id pnp_kbd_devi
> >  };
> >  
> >  static struct pnp_driver i8042_pnp_kbd_driver = {
> > -	.name           = "i8042 kbd",
> > +	.name           = "i8042-kbd",
> 
> Why is this needed? I don't think spaces are more dangerous than a colon
> which we do use...

This string becomes the name of a directory in sysfs.  Having a ' '
character in a directory name just doesn't seem like a good idea; at
the very least it's likely to confuse shell scripts.  (A ':' character
is a lot less subject to misinterpretation.)  I could change the '-' to 
':' if you prefer.

> Other than that - looks reasonable to me...

Thanks.  I'll separate out the driver name change from the rest of the 
patch.

There are a couple of questions remaining.  The patch enables wakeup on 
the i8042-aux device -- is this a good idea?  I haven't tested that 
part of it yet, but we wouldn't want suspended systems waking up just 
because somebody moves the mouse around.  On the other hand, on some 
machines (like mine!) it may not be possible to enable keyboard-wakeup 
without also enabling mouse-wakeup, since they use the same GPE.

Also, the patch effectively tells the kernel that ISAPNP and PNPBIOS 
devices are incapable of issuing wakeup requests.  Is that going to 
cause trouble for people?  Would it be better to change the patch so 
that it doesn't call device_set_wakeup_capable() if 
dev->protocol->can_wakeup isn't defined?

Alan Stern

Re: [RFC] Wakeup for PNP

[Dmitry Torokhov]

Hi Alan,

On Wed, Mar 03, 2010 at 10:29:09AM -0500, Alan Stern wrote:
> On Tue, 2 Mar 2010, Dmitry Torokhov wrote:
> 
> > > @@ -656,7 +658,7 @@ static struct pnp_device_id pnp_kbd_devi
> > >  };
> > >  
> > >  static struct pnp_driver i8042_pnp_kbd_driver = {
> > > -	.name           = "i8042 kbd",
> > > +	.name           = "i8042-kbd",
> > 
> > Why is this needed? I don't think spaces are more dangerous than a colon
> > which we do use...
> 
> This string becomes the name of a directory in sysfs.  Having a ' '
> character in a directory name just doesn't seem like a good idea; at
> the very least it's likely to confuse shell scripts.  (A ':' character
> is a lot less subject to misinterpretation.)  I could change the '-' to 
> ':' if you prefer.
> 

We have not heard complains about this causing trouble ever since PNP
support was added (so what 5 years at least?). At this point I'd be more
worried about breaking existing users...

Thanks.

-- 
Dmitry

Re: [RFC] Wakeup for PNP

[Dmitry Torokhov]

Hi Alan,

On Wed, Mar 03, 2010 at 10:29:09AM -0500, Alan Stern wrote:
> On Tue, 2 Mar 2010, Dmitry Torokhov wrote:
> 
> > > @@ -656,7 +658,7 @@ static struct pnp_device_id pnp_kbd_devi
> > >  };
> > >  
> > >  static struct pnp_driver i8042_pnp_kbd_driver = {
> > > -	.name           = "i8042 kbd",
> > > +	.name           = "i8042-kbd",
> > 
> > Why is this needed? I don't think spaces are more dangerous than a colon
> > which we do use...
> 
> This string becomes the name of a directory in sysfs.  Having a ' '
> character in a directory name just doesn't seem like a good idea; at
> the very least it's likely to confuse shell scripts.  (A ':' character
> is a lot less subject to misinterpretation.)  I could change the '-' to 
> ':' if you prefer.
> 

We have not heard complains about this causing trouble ever since PNP
support was added (so what 5 years at least?). At this point I'd be more
worried about breaking existing users...

Thanks.

-- 
Dmitry