spitz: add gpio button support

spitz: add gpio button support

[Pavel Machek]

This is call for testing/comments. It adds support for power button,
SWA and SWB.. I'm not sure what AK_INT is, nor what event it should
generate. Unfortunately, this does _not_ fix suspend/resume, and
neither does switch back to old spitz-keyboard help.

Signed-off-by: Pavel Machek <pavel@ucw.cz>
								Pavel

--- linux-rc/arch/arm.ofic/mach-pxa/spitz.c	2009-09-28 12:09:26.000000000 +0200
+++ linux-rc/arch/arm/mach-pxa/spitz.c	2009-10-04 20:58:32.000000000 +0200
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
+#include <linux/gpio_keys.h>
 #include <linux/gpio.h>
 #include <linux/leds.h>
 #include <linux/mtd/physmap.h>
@@ -367,7 +368,8 @@
 };
 
 static struct platform_device spitzkbd_device = {
-	.name		= "matrix-keypad",
+  	.name		= "matrix-keypad",
+	//	.name		= "spitz-keyboard",
 	.id		= -1,
 	.dev		= {
 		.platform_data = &spitzkbd_pdata,
@@ -375,6 +377,48 @@
 };
 
 
+static struct gpio_keys_button spitz_gpio_keys[] = {
+	{
+		.type	= EV_KEY,
+		.code	= KEY_SUSPEND,
+		.gpio	= SPITZ_GPIO_ON_KEY,
+		.desc	= "Poweron",
+		.wakeup	= 1,
+	},
+	{
+		.type	= EV_SW,
+		.code	= 0,
+		.gpio	= SPITZ_GPIO_SWA,
+		.desc	= "SWA",
+	},
+	{
+		.type	= EV_SW,
+		.code	= 1,
+		.gpio	= SPITZ_GPIO_SWB,
+		.desc	= "SWB",
+	},
+	{
+		.type	= EV_KEY,
+		.code	= KEY_F13,
+		.gpio	= SPITZ_GPIO_AK_INT,
+		.desc	= "HP",
+	},
+};
+
+static struct gpio_keys_platform_data spitz_gpio_keys_platform_data = {
+	.buttons	= spitz_gpio_keys,
+	.nbuttons	= ARRAY_SIZE(spitz_gpio_keys),
+};
+
+static struct platform_device spitz_gpio_keys_device = {
+	.name	= "gpio-keys",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &spitz_gpio_keys_platform_data,
+	},
+};
+
+
 /*
  * Spitz LEDs
  */
@@ -689,6 +733,7 @@
 static struct platform_device *devices[] __initdata = {
 	&spitzscoop_device,
 	&spitzkbd_device,
+	&spitz_gpio_keys_device,
 	&spitzled_device,
 	&sharpsl_nand_device,
 	&sharpsl_rom_device,

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

spitz: add gpio button support (AK_INT and remote control description)

[Stanislav Brabec]

Pavel Machek wrote:
> I'm not sure what AK_INT is, nor what event it should
> generate.

AK_INT: remote control detect

How it should work on spitz:

The default state of ?the remote pin of audio barrel connector is a weak
pull-up.

Button is pressed. => Voltage ?on remote pin decreases. => Hardware
defined threshold voltage is reached. => AK_INT is triggered. => Driver
sets SPITZ_SCP2_AKIN_PULLUP (it connects stronger and more power hungry
pull-up), ignores AK_INT change, sets MAX1111 to channel 0, reads value
from A/D converter.

After some debouncing:

The value is below threshold. => Decode the value and generate input
event.

The value is over threshold. => Switch SPITZ_SCP2_AKIN_PULLUP back,
activate AK_INT interrupt and wait for next edge.

The value is near zero. => ?A standard 3-ring earphone connector seems
to be inserted. AK_INT will be "active" all the time. To save stronger
pull-up power, behave like the value over threshold and ignore AK_INT
until possible next change (remove earphone, insert remote).

Some of these actions should probably cause wake-up. It would be nice to
wake-up sleeping Zaurus by pressing Play button and start to play.

The driver is available e. g. in 2.6.26-RP kernel as
sharpsl-rc-r1.patch. I plan to rewrite it to a generic "resistor matrix
keyboard" and then sent to vanilla.


________________________________________________________________________
Stanislav Brabec
http://www.penguin.cz/~utx

spitz: add gpio button support (AK_INT and remote control description)

[Pavel Machek]

On Tue 2009-10-06 13:48:52, Stanislav Brabec wrote:
> Pavel Machek wrote:
> > I'm not sure what AK_INT is, nor what event it should
> > generate.
> 
> AK_INT: remote control detect

Thanks. So for now I'll do:

        /* This is for remote control support. Zaurus supports wired                  
           remote on headphones. */
        {
                .type   = EV_KEY,
                .code   = KEY_F13,
                .gpio   = SPITZ_GPIO_AK_INT,
                .desc   = "Wired remote (AK_INT)",
        },

..and leave it for proper driver to support nicely.

								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

spitz: add gpio button support (AK_INT and remote control description)

[Stanislav Brabec]

Pavel Machek wrote:
> On Tue 2009-10-06 13:48:52, Stanislav Brabec wrote:
> > Pavel Machek wrote:
> > > I'm not sure what AK_INT is, nor what event it should
> > > generate.
> > 
> > AK_INT: remote control detect
> 
> Thanks. So for now I'll do:
> 
>         /* This is for remote control support. Zaurus supports wired                  
>            remote on headphones. */
>         {
>                 .type   = EV_KEY,
>                 .code   = KEY_F13,
>                 .gpio   = SPITZ_GPIO_AK_INT,
>                 .desc   = "Wired remote (AK_INT)",
>         },
> 
> ..and leave it for proper driver to support nicely.

I think that AK_INT should not generate key press event. Permanently
pressed key could confuse many programs.

But we may want to use switch events. I propose to create new switch
event for it: SW_REMOTE_INSERT. But mapping of AK_INT to
SW_REMOTE_INSERT cannot be straightforward.

That is why I propose to ignore AK_INT for now.

Following implementation may be used later (with future resistor matrix
keyboard driver):

Map SPITZ_GPIO_HP_IN to SW_JACK_PHYSICAL_INSERT (after debouncing).

Generate SW_REMOTE_INSERT if:
- SW_JACK_PHYSICAL_INSERT is on AK_INT is active and remote pin has full
  voltage
or
- SW_JACK_PHYSICAL_INSERT is on and remote driver shows valid value

SW_REMOTE_INSERT should be recognized by the Jack abstraction layer
(sound/core/jack.c).

Well, I am thinking about defining extra values for resistors, that can
make stuff even more useful at cost of need of non-standard wiring on
fourth ring of the audio jack. It can generate SW_MICROPHONE_INSERT,
SW_LINEOUT_INSERT, and not yet defined SW_HEADESET_INSERT and
SW_LINEIN_INSERT (which would require hardware mod inside Zaurus for
stereo line-in).

The default may be SW_HEADPHONE_INSERT (short connection between remote
pin and ground).

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: sbrabec at suse.cz
Lihovarsk? 1060/12           tel: +420 284 028 966, +49 911 740538747
190 00 Praha 9                                  fax: +420 284 028 951
Czech Republic                                    http://www.suse.cz/

spitz: add gpio button support (AK_INT and remote control description)

[Mark Brown]

On Wed, Oct 07, 2009 at 01:29:51PM +0200, Stanislav Brabec wrote:

> SW_REMOTE_INSERT should be recognized by the Jack abstraction layer
> (sound/core/jack.c).

It's the other way around - the jack layer reports jacks for ALSA cards.
Also, for spitz you'd probably want to be using the ASoC wrapper layer.

> Well, I am thinking about defining extra values for resistors, that can
> make stuff even more useful at cost of need of non-standard wiring on
> fourth ring of the audio jack. It can generate SW_MICROPHONE_INSERT,
> SW_LINEOUT_INSERT, and not yet defined SW_HEADESET_INSERT and
> SW_LINEIN_INSERT (which would require hardware mod inside Zaurus for
> stereo line-in).

Headset should be represented by reporting headphone plus microphone on
one jack simultaneously.

spitz: add gpio button support (AK_INT and remote control description)

[Eric Miao]

So what's the conclusion of the patch? Any ACK/NAK or patch needs updating?

On Wed, Oct 7, 2009 at 8:06 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Oct 07, 2009 at 01:29:51PM +0200, Stanislav Brabec wrote:
>
>> SW_REMOTE_INSERT should be recognized by the Jack abstraction layer
>> (sound/core/jack.c).
>
> It's the other way around - the jack layer reports jacks for ALSA cards.
> Also, for spitz you'd probably want to be using the ASoC wrapper layer.
>
>> Well, I am thinking about defining extra values for resistors, that can
>> make stuff even more useful at cost of need of non-standard wiring on
>> fourth ring of the audio jack. It can generate SW_MICROPHONE_INSERT,
>> SW_LINEOUT_INSERT, and not yet defined SW_HEADESET_INSERT and
>> SW_LINEIN_INSERT (which would require hardware mod inside Zaurus for
>> stereo line-in).
>
> Headset should be represented by reporting headphone plus microphone on
> one jack simultaneously.
>

spitz: add gpio button support (fixes regression)

[Pavel Machek]

> So what's the conclusion of the patch? Any ACK/NAK or patch needs updating?

Please take this updated version.

---

Spitz switched to generic matrix-gpio keyboard driver in 2.6.32-rc0,
but that means that support for power button and lid switches was
lost. This restores it, using button-gpio code.

Signed-off-by: Pavel Machek <pavel@ucw.cz>

--- linux-rc/arch/arm.ofic/mach-pxa/spitz.c	2009-10-06 13:48:07.000000000 +0200
+++ linux-rc/arch/arm/mach-pxa/spitz.c	2009-10-06 21:17:19.000000000 +0200
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
+#include <linux/gpio_keys.h>
 #include <linux/gpio.h>
 #include <linux/leds.h>
 #include <linux/mtd/physmap.h>
@@ -375,6 +377,51 @@
 };
 
 
+static struct gpio_keys_button spitz_gpio_keys[] = {
+	{
+		.type	= EV_KEY,
+		.code	= KEY_SUSPEND,
+		.gpio	= SPITZ_GPIO_ON_KEY,
+		.desc	= "Power button",
+		.wakeup	= 1,
+	},
+	/* Two buttons detecting the LID state */
+	{
+		.type	= EV_SW,
+		.code	= 0,
+		.gpio	= SPITZ_GPIO_SWA,
+		.desc	= "Lid SWA",
+	},
+	{
+		.type	= EV_SW,
+		.code	= 1,
+		.gpio	= SPITZ_GPIO_SWB,
+		.desc	= "Lid SWB",
+	},
+	/* This is for remote control support. Zaurus supports wired
+	   remote on headphones. */
+	{
+		.type	= EV_SW,
+		.code	= 2,
+		.gpio	= SPITZ_GPIO_AK_INT,
+		.desc	= "Wired remote (AK_INT)",
+	},
+};
+
+static struct gpio_keys_platform_data spitz_gpio_keys_platform_data = {
+	.buttons	= spitz_gpio_keys,
+	.nbuttons	= ARRAY_SIZE(spitz_gpio_keys),
+};
+
+static struct platform_device spitz_gpio_keys_device = {
+	.name	= "gpio-keys",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &spitz_gpio_keys_platform_data,
+	},
+};
+
+
 /*
  * Spitz LEDs
  */
@@ -689,6 +736,7 @@
 static struct platform_device *devices[] __initdata = {
 	&spitzscoop_device,
 	&spitzkbd_device,
+	&spitz_gpio_keys_device,
 	&spitzled_device,
 	&sharpsl_nand_device,
 	&sharpsl_rom_device,


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

spitz: add gpio button support (fixes regression)

[Stanislav Brabec]

Pavel Machek wrote:
> > So what's the conclusion of the patch? Any ACK/NAK or patch needs updating?

Well, AK_INT as it is implemented does not give any useful information,
but it does not break anything. When inserting or removing remote, or
(any) remote key is pressed, it generates one or more switch events.
This code will have to be removed after implementing full remote support
(turning pull-up resistor may cause false events, ?insert/remove needs
more aggressive debounce), so I would prefer not adding AK_INT support.
(See patch below.)

> Please take this updated version.

> Spitz switched to generic matrix-gpio keyboard driver in 2.6.32-rc0,
> but that means that support for power button and lid switches was
> lost. This restores it, using button-gpio code.

I just tested it. It returns keys function, but suspend button does not
cause suspend. Just an input event occurs, see below for output from
evtest. Resume after "echo mem >/sys/power/state" is impossible. I think
it is not intended behavior.
?
Event: time 1255640047.262125, type 1 (Key), code 205 (Suspend), value 1
Event: time 1255640047.262173, -------------- Report Sync ------------
Event: time 1255640047.813401, type 1 (Key), code 205 (Suspend), value 0
Event: time 1255640047.813446, -------------- Report Sync ------------

diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index ee8d603..a8ef058 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
+#include <linux/gpio_keys.h>
 #include <linux/gpio.h>
 #include <linux/leds.h>
 #include <linux/mtd/physmap.h>
@@ -375,6 +376,43 @@ static struct platform_device spitzkbd_device = {
 };
 
 
+static struct gpio_keys_button spitz_gpio_keys[] = {
+	{
+		.type	= EV_KEY,
+		.code	= KEY_SUSPEND,
+		.gpio	= SPITZ_GPIO_ON_KEY,
+		.desc	= "Power button",
+		.wakeup	= 1,
+	},
+	/* Two buttons detecting the LID state */
+	{
+		.type	= EV_SW,
+		.code	= 0,
+		.gpio	= SPITZ_GPIO_SWA,
+		.desc	= "Lid SWA",
+	},
+	{
+		.type	= EV_SW,
+		.code	= 1,
+		.gpio	= SPITZ_GPIO_SWB,
+		.desc	= "Lid SWB",
+	},
+};
+
+static struct gpio_keys_platform_data spitz_gpio_keys_platform_data = {
+	.buttons	= spitz_gpio_keys,
+	.nbuttons	= ARRAY_SIZE(spitz_gpio_keys),
+};
+
+static struct platform_device spitz_gpio_keys_device = {
+	.name	= "gpio-keys",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &spitz_gpio_keys_platform_data,
+	},
+};
+
+
 /*
  * Spitz LEDs
  */
@@ -689,6 +727,7 @@ static struct platform_device sharpsl_rom_device = {
 static struct platform_device *devices[] __initdata = {
 	&spitzscoop_device,
 	&spitzkbd_device,
+	&spitz_gpio_keys_device,
 	&spitzled_device,
 	&sharpsl_nand_device,
 	&sharpsl_rom_device,



________________________________________________________________________
Stanislav Brabec
http://www.penguin.cz/~utx/zaurus

spitz: add gpio button support (fixes regression)

[Pavel Machek]

Hi!

> > > So what's the conclusion of the patch? Any ACK/NAK or patch needs updating?
> 
> Well, AK_INT as it is implemented does not give any useful information,
> but it does not break anything. When inserting or removing remote, or
> (any) remote key is pressed, it generates one or more switch events.
> This code will have to be removed after implementing full remote support
> (turning pull-up resistor may cause false events, ???insert/remove needs
> more aggressive debounce), so I would prefer not adding AK_INT support.
> (See patch below.)

Ok.

> > Please take this updated version.
> 
> > Spitz switched to generic matrix-gpio keyboard driver in 2.6.32-rc0,
> > but that means that support for power button and lid switches was
> > lost. This restores it, using button-gpio code.
> 
> I just tested it. It returns keys function, but suspend button does not
> cause suspend. Just an input event occurs, see below for output from
> evtest. Resume after "echo mem >/sys/power/state" is impossible. I think
> it is not intended behavior.
> ???

Well, resume is hit by second regression.

Suspend key.... I restored it to the level of functionality in
2.6.31 :-).

> diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
> index ee8d603..a8ef058 100644
> --- a/arch/arm/mach-pxa/spitz.c
> +++ b/arch/arm/mach-pxa/spitz.c
> @@ -15,6 +15,7 @@
>  #include <linux/kernel.h>
>  #include <linux/platform_device.h>
>  #include <linux/delay.h>
> +#include <linux/gpio_keys.h>
>  #include <linux/gpio.h>
>  #include <linux/leds.h>
>  #include <linux/mtd/physmap.h>
> @@ -375,6 +376,43 @@ static struct platform_device spitzkbd_device = {
>  };
>  
>  
> +static struct gpio_keys_button spitz_gpio_keys[] = {
> +	{
> +		.type	= EV_KEY,
> +		.code	= KEY_SUSPEND,
> +		.gpio	= SPITZ_GPIO_ON_KEY,
> +		.desc	= "Power button",
> +		.wakeup	= 1,
> +	},
> +	/* Two buttons detecting the LID state */
> +	{
> +		.type	= EV_SW,
> +		.code	= 0,
> +		.gpio	= SPITZ_GPIO_SWA,
> +		.desc	= "Lid SWA",
> +	},
> +	{
> +		.type	= EV_SW,
> +		.code	= 1,
> +		.gpio	= SPITZ_GPIO_SWB,
> +		.desc	= "Lid SWB",
> +	},
> +};
> +
> +static struct gpio_keys_platform_data spitz_gpio_keys_platform_data = {
> +	.buttons	= spitz_gpio_keys,
> +	.nbuttons	= ARRAY_SIZE(spitz_gpio_keys),
> +};
> +
> +static struct platform_device spitz_gpio_keys_device = {
> +	.name	= "gpio-keys",
> +	.id	= -1,
> +	.dev	= {
> +		.platform_data	= &spitz_gpio_keys_platform_data,
> +	},
> +};
> +
> +
>  /*
>   * Spitz LEDs
>   */
> @@ -689,6 +727,7 @@ static struct platform_device sharpsl_rom_device = {
>  static struct platform_device *devices[] __initdata = {
>  	&spitzscoop_device,
>  	&spitzkbd_device,
> +	&spitz_gpio_keys_device,
>  	&spitzled_device,
>  	&sharpsl_nand_device,
>  	&sharpsl_rom_device,
> 

Ack. Someone please apply it.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

spitz: add gpio button support (fixes regression)

[Stanislav Brabec]

Pavel Machek wrote:
>Well, resume is hit by second regression.
>
>Suspend key.... I restored it to the level of functionality in
>2.6.31 :-).

> > +		.type	= EV_KEY,
> > +		.code	= KEY_SUSPEND,

I guess it should be EV_PWR. Then it will at least go to suspend.

Here is a patch with just this change. Also changing .desc to
"On/Off" (the text on the case).

diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index ee8d603..c690522 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
+#include <linux/gpio_keys.h>
 #include <linux/gpio.h>
 #include <linux/leds.h>
 #include <linux/mtd/physmap.h>
@@ -375,6 +376,43 @@ static struct platform_device spitzkbd_device = {
 };
 
 
+static struct gpio_keys_button spitz_gpio_keys[] = {
+	{
+		.type	= EV_PWR,
+		.code	= KEY_SUSPEND,
+		.gpio	= SPITZ_GPIO_ON_KEY,
+		.desc	= "On/Off",
+		.wakeup	= 1,
+	},
+	/* Two buttons detecting the LID state */
+	{
+		.type	= EV_SW,
+		.code	= 0,
+		.gpio	= SPITZ_GPIO_SWA,
+		.desc	= "Lid SWA",
+	},
+	{
+		.type	= EV_SW,
+		.code	= 1,
+		.gpio	= SPITZ_GPIO_SWB,
+		.desc	= "Lid SWB",
+	},
+};
+
+static struct gpio_keys_platform_data spitz_gpio_keys_platform_data = {
+	.buttons	= spitz_gpio_keys,
+	.nbuttons	= ARRAY_SIZE(spitz_gpio_keys),
+};
+
+static struct platform_device spitz_gpio_keys_device = {
+	.name	= "gpio-keys",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &spitz_gpio_keys_platform_data,
+	},
+};
+
+
 /*
  * Spitz LEDs
  */
@@ -689,6 +727,7 @@ static struct platform_device sharpsl_rom_device = {
 static struct platform_device *devices[] __initdata = {
 	&spitzscoop_device,
 	&spitzkbd_device,
+	&spitz_gpio_keys_device,
 	&spitzled_device,
 	&sharpsl_nand_device,
 	&sharpsl_rom_device,



________________________________________________________________________
Stanislav Brabec
http://www.penguin.cz/~utx

spitz: add gpio button support (fixes regression)

[Pavel Machek]

On Thu 2009-10-15 23:47:32, Stanislav Brabec wrote:
> Pavel Machek wrote:
> >Well, resume is hit by second regression.
> >
> >Suspend key.... I restored it to the level of functionality in
> >2.6.31 :-).
> 
> > > +		.type	= EV_KEY,
> > > +		.code	= KEY_SUSPEND,
> 
> I guess it should be EV_PWR. Then it will at least go to suspend.

Does it actually go to suspend with that change?

(Actually, I wonder how it could have ever worked: userspace should
certainly be in a loop there; kernel should not suspend machine on its
own. In -rp kernels, kernel _does_ suspend on its own, but... I'm not
sure if that's suitable for mainline).
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

spitz: add gpio button support (fixes regression)

[Eric Miao]

On Fri, Oct 16, 2009 at 5:52 AM, Pavel Machek <pavel@ucw.cz> wrote:
> On Thu 2009-10-15 23:47:32, Stanislav Brabec wrote:
>> Pavel Machek wrote:
>> >Well, resume is hit by second regression.
>> >
>> >Suspend key.... I restored it to the level of functionality in
>> >2.6.31 :-).
>>
>> > > + ? ? ? ? .type ? = EV_KEY,
>> > > + ? ? ? ? .code ? = KEY_SUSPEND,
>>
>> I guess it should be EV_PWR. Then it will at least go to suspend.
>
> Does it actually go to suspend with that change?
>
> (Actually, I wonder how it could have ever worked: userspace should
> certainly be in a loop there; kernel should not suspend machine on its
> own. In -rp kernels, kernel _does_ suspend on its own, but... I'm not
> sure if that's suitable for mainline).

EV_PWR is what's in the original spitzkbd.c, so I assume that's
something we should keep. And the reason that EV_PWR makes
it actually suspend might related the APM emulation things,
which I'm not very sure about.

spitz: add gpio button support (fixes regression)

[Stanislav Brabec]

Pavel Machek wrote:
> On Thu 2009-10-15 23:47:32, Stanislav Brabec wrote:
> > Pavel Machek wrote:
> > >Well, resume is hit by second regression.
> > >
> > >Suspend key.... I restored it to the level of functionality in
> > >2.6.31 :-).
> > 
> > > > +		.type	= EV_KEY,
> > > > +		.code	= KEY_SUSPEND,
> > 
> > I guess it should be EV_PWR. Then it will at least go to suspend.
> 
> Does it actually go to suspend with that change?

Yes.

> (Actually, I wonder how it could have ever worked: userspace should
> certainly be in a loop there; kernel should not suspend machine on its
> own. In -rp kernels, kernel _does_ suspend on its own, but... I'm not
> sure if that's suitable for mainline).

At least userspace apmd should be able to enter into suspend process
(tested in 2.6.26-RP, not yet verified in 2.6.32-rc*).

I don't know, what is the preferred way in recent days, but Tosa does
the same.

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: sbrabec at suse.cz
Lihovarsk? 1060/12           tel: +420 284 028 966, +49 911 740538747
190 00 Praha 9                                  fax: +420 284 028 951
Czech Republic                                    http://www.suse.cz/

spitz: add gpio button support (fixes regression)

[Pavel Machek]

> > (Actually, I wonder how it could have ever worked: userspace should
> > certainly be in a loop there; kernel should not suspend machine on its
> > own. In -rp kernels, kernel _does_ suspend on its own, but... I'm not
> > sure if that's suitable for mainline).
> 
> At least userspace apmd should be able to enter into suspend process
> (tested in 2.6.26-RP, not yet verified in 2.6.32-rc*).
> 
> I don't know, what is the preferred way in recent days, but Tosa does
> the same.

Ok, lets do it that way, then. I guess you need to post patch with
your signed-off-by, and you can add my acked-by.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH] Re: spitz: add gpio button support (fixes regression)

[Stanislav Brabec]

Pavel Machek wrote:
>Ok, lets do it that way, then.

Updating desc for lid keys and resending patch with proper comments:
?
Define Spitz buttons as GPIO keys in a way compatible with the old driver:

On/Off: As Suspend EV_PWR key

?Raw values of? lid sensors ?SWA and SWB: As EV_SW switches
SWA: Display Down
SWB: Lid Closed
Recommended user space decoding:
?SWA==0 & SWB==0: lid opened (landscape mode)
??SWA==1 & SWB==0: ?invalid (or mechanic race condition)
SWA==0 & SWB==1: ?lid closed with display up (portrait mode or ?mechanic
                 race condition while closing to ?display-less mode)
SWA==1 & SWB==1: lid closed with display down (display-less mode)

?AK_INT remote trigger is not mapped as input event. Without complete
remote driver and remote pull-up control it has no useful
interpretation.

Signed-off-by: Stanislav Brabec <utx@penguin.cz>
Acked-by: Pavel Machek <pavel@ucw.cz>

diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index ee8d603..c690522 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
+#include <linux/gpio_keys.h>
 #include <linux/gpio.h>
 #include <linux/leds.h>
 #include <linux/mtd/physmap.h>
@@ -375,6 +376,43 @@ static struct platform_device spitzkbd_device = {
 };
 
 
+static struct gpio_keys_button spitz_gpio_keys[] = {
+	{
+		.type	= EV_PWR,
+		.code	= KEY_SUSPEND,
+		.gpio	= SPITZ_GPIO_ON_KEY,
+		.desc	= "On/Off",
+		.wakeup	= 1,
+	},
+	/* Two buttons detecting the lid state */
+	{
+		.type	= EV_SW,
+		.code	= 0,
+		.gpio	= SPITZ_GPIO_SWA,
+		.desc	= "?Display Down",
+	},
+	{
+		.type	= EV_SW,
+		.code	= 1,
+		.gpio	= SPITZ_GPIO_SWB,
+		.desc	= "?Lid Closed",
+	},
+};
+
+static struct gpio_keys_platform_data spitz_gpio_keys_platform_data = {
+	.buttons	= spitz_gpio_keys,
+	.nbuttons	= ARRAY_SIZE(spitz_gpio_keys),
+};
+
+static struct platform_device spitz_gpio_keys_device = {
+	.name	= "gpio-keys",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &spitz_gpio_keys_platform_data,
+	},
+};
+
+
 /*
  * Spitz LEDs
  */
@@ -689,6 +727,7 @@ static struct platform_device sharpsl_rom_device = {
 static struct platform_device *devices[] __initdata = {
 	&spitzscoop_device,
 	&spitzkbd_device,
+	&spitz_gpio_keys_device,
 	&spitzled_device,
 	&sharpsl_nand_device,
 	&sharpsl_rom_device,


________________________________________________________________________
Stanislav Brabec
http://www.penguin.cz/~utx

[PATCH] Re: spitz: add gpio button support (fixes regression)

[Russell King - ARM Linux]

I'm not sure we want to apply this patch - it has UTF <U+FEFF>
sequences embedded randomly in it.  I notice these because they
completely screw up the ability to read this email in mutt running
under a Linux VT (resulting in bits of the screen scrolling which
mutt didn't intend.)

I suggest you try re-sending it as an attachment.

On Fri, Oct 16, 2009 at 08:50:05PM +0200, Stanislav Brabec wrote:
> Pavel Machek wrote:
> >Ok, lets do it that way, then.
> 
> Updating desc for lid keys and resending patch with proper comments:
> ?
> Define Spitz buttons as GPIO keys in a way compatible with the old driver:
> 
> On/Off: As Suspend EV_PWR key
> 
> ?Raw values of? lid sensors ?SWA and SWB: As EV_SW switches
> SWA: Display Down
> SWB: Lid Closed
> Recommended user space decoding:
> ?SWA==0 & SWB==0: lid opened (landscape mode)
> ??SWA==1 & SWB==0: ?invalid (or mechanic race condition)
> SWA==0 & SWB==1: ?lid closed with display up (portrait mode or ?mechanic
>                  race condition while closing to ?display-less mode)
> SWA==1 & SWB==1: lid closed with display down (display-less mode)
> 
> ?AK_INT remote trigger is not mapped as input event. Without complete
> remote driver and remote pull-up control it has no useful
> interpretation.
> 
> Signed-off-by: Stanislav Brabec <utx@penguin.cz>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> 
> diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
> index ee8d603..c690522 100644
> --- a/arch/arm/mach-pxa/spitz.c
> +++ b/arch/arm/mach-pxa/spitz.c
> @@ -15,6 +15,7 @@
>  #include <linux/kernel.h>
>  #include <linux/platform_device.h>
>  #include <linux/delay.h>
> +#include <linux/gpio_keys.h>
>  #include <linux/gpio.h>
>  #include <linux/leds.h>
>  #include <linux/mtd/physmap.h>
> @@ -375,6 +376,43 @@ static struct platform_device spitzkbd_device = {
>  };
>  
>  
> +static struct gpio_keys_button spitz_gpio_keys[] = {
> +	{
> +		.type	= EV_PWR,
> +		.code	= KEY_SUSPEND,
> +		.gpio	= SPITZ_GPIO_ON_KEY,
> +		.desc	= "On/Off",
> +		.wakeup	= 1,
> +	},
> +	/* Two buttons detecting the lid state */
> +	{
> +		.type	= EV_SW,
> +		.code	= 0,
> +		.gpio	= SPITZ_GPIO_SWA,
> +		.desc	= "?Display Down",
> +	},
> +	{
> +		.type	= EV_SW,
> +		.code	= 1,
> +		.gpio	= SPITZ_GPIO_SWB,
> +		.desc	= "?Lid Closed",
> +	},
> +};
> +
> +static struct gpio_keys_platform_data spitz_gpio_keys_platform_data = {
> +	.buttons	= spitz_gpio_keys,
> +	.nbuttons	= ARRAY_SIZE(spitz_gpio_keys),
> +};
> +
> +static struct platform_device spitz_gpio_keys_device = {
> +	.name	= "gpio-keys",
> +	.id	= -1,
> +	.dev	= {
> +		.platform_data	= &spitz_gpio_keys_platform_data,
> +	},
> +};
> +
> +
>  /*
>   * Spitz LEDs
>   */
> @@ -689,6 +727,7 @@ static struct platform_device sharpsl_rom_device = {
>  static struct platform_device *devices[] __initdata = {
>  	&spitzscoop_device,
>  	&spitzkbd_device,
> +	&spitz_gpio_keys_device,
>  	&spitzled_device,
>  	&sharpsl_nand_device,
>  	&sharpsl_rom_device,
> 
> 
> ________________________________________________________________________
> Stanislav Brabec
> http://www.penguin.cz/~utx
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] Re: spitz: add gpio button support (fixes regression)

[Stanislav Brabec]

Russell King - ARM Linux wrote: 
> I'm not sure we want to apply this patch - it has UTF <U+FEFF>
> sequences embedded randomly in it.

These character were added by Evolution. I don't know why. I just wanted
to inline a mail and edit few characters.

Define Spitz buttons as GPIO keys in a way compatible with the old driver:

On/Off: As Suspend EV_PWR key

Raw values of lid sensors SWA and SWB: As EV_SW switches
SWA: Display Down
SWB: Lid Closed
Recommended user space decoding:
SWA==0 & SWB==0: lid opened (landscape mode)
SWA==1 & SWB==0: invalid (or mechanic race condition)
SWA==0 & SWB==1: lid closed with display up (portrait mode or mechanic
                 race condition while closing to display-less mode)
SWA==1 & SWB==1: lid closed with display down (display-less mode)

AK_INT remote trigger is not mapped as input event. Without complete
remote driver and remote pull-up control it has no useful
interpretation.

Signed-off-by: Stanislav Brabec <utx@penguin.cz>
Acked-by: Pavel Machek <pavel@ucw.cz>

diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index ee8d603..c690522 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
+#include <linux/gpio_keys.h>
 #include <linux/gpio.h>
 #include <linux/leds.h>
 #include <linux/mtd/physmap.h>
@@ -375,6 +376,43 @@ static struct platform_device spitzkbd_device = {
 };
 

+static struct gpio_keys_button spitz_gpio_keys[] = {
+	{
+		.type	= EV_PWR,
+		.code	= KEY_SUSPEND,
+		.gpio	= SPITZ_GPIO_ON_KEY,
+		.desc	= "On/Off",
+		.wakeup	= 1,
+	},
+	/* Two buttons detecting the lid state */
+	{
+		.type	= EV_SW,
+		.code	= 0,
+		.gpio	= SPITZ_GPIO_SWA,
+		.desc	= "Display Down",
+	},
+	{
+		.type	= EV_SW,
+		.code	= 1,
+		.gpio	= SPITZ_GPIO_SWB,
+		.desc	= "Lid Closed",
+	},
+};
+
+static struct gpio_keys_platform_data spitz_gpio_keys_platform_data = {
+	.buttons	= spitz_gpio_keys,
+	.nbuttons	= ARRAY_SIZE(spitz_gpio_keys),
+};
+
+static struct platform_device spitz_gpio_keys_device = {
+	.name	= "gpio-keys",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &spitz_gpio_keys_platform_data,
+	},
+};
+
+
 /*
  * Spitz LEDs
  */
@@ -689,6 +727,7 @@ static struct platform_device sharpsl_rom_device = {
 static struct platform_device *devices[] __initdata = {
 	&spitzscoop_device,
 	&spitzkbd_device,
+	&spitz_gpio_keys_device,
 	&spitzled_device,
 	&sharpsl_nand_device,
 	&sharpsl_rom_device,



________________________________________________________________________

Stanislav Brabec
http://www.penguin.cz/~utx

[PATCH] Re: spitz: add gpio button support (fixes regression)

[Eric Miao]

On Sat, Oct 17, 2009 at 6:22 AM, Stanislav Brabec <utx@penguin.cz> wrote:
> Russell King - ARM Linux wrote:
>> I'm not sure we want to apply this patch - it has UTF <U+FEFF>
>> sequences embedded randomly in it.

That's ZWNBSP - Zero-Width No-Break Space (unicode character FEFF).
I think Stanislav needs to train his evolution a little bit for this.

Anyway, applied to 'fix' with the above special character removed.