On Sun, Mar 08, 2009 at 09:56:45PM +0100, Rafael J. Wysocki wrote:

On Sunday 08 March 2009, Matthew Garrett wrote:

quoted

On Sun, Mar 08, 2009 at 08:45:59AM +0000, Richard Hughes wrote:

quoted

On Sat, Mar 7, 2009 at 8:39 PM, Matthew Garrett [off-list ref] wrote:

quoted

We don't have to at all - as far as I've been able to tell, the kernel
is utterly consistent in its current usage. The only drivers that emit
KEY_SLEEP are either embedded-specific (where it's clearly suspend to
RAM and not hibernate), the ACPI driver (where usage in other operating
systems is consistent with it being suspent to RAM) and the panasonic
and thinkpad drivers which use it consistently. If there's any
confusion, it's over the fact that KEY_SUSPEND is is used for suspend to
RAM in a (smaller) number of places.

The fact that we're mapping x->y and y->x is the reason people keep
getting it wrong.

Sure, doing things differently would have made sense several years ago 
when nobody was relying on this behaviour. We don't have that option now 
- making this change will break things, and we've got no idea how much 
it'll break.

Which is a good enough reason to avoid it.

Alternatively, we can add completely new definitions _along_ _with_ the old
ones, mark the old ones as obsolete (after some time) and try to make the
user space start using the new ones only (that may be difficult, though).

I said I liked the names, but I didn't realize that changing them would break
things.

I don't think we want to break anything if we can help it. The problem
with Richard's patch is that it changes meaning of KEY_SUSPEND from STD
to STR. I would prefer if we could do the following:

- KEY_SLEEP - leave the keycode, the action should be the default
system state defined by either platform or user. I expect that the vast
majority of system have default state similar to S3 so there should not
be anysurprises.

- KEY_SUSPEND - provide better comment for its intended usage and maybe
add KEY_HIBERNATE alias.

- KEY_SUSPEND2RAM - add a new definition.

Do you think this would this work?

I intend to back out the patch in question for the time being.

Thanks.

-- 
Dmitry