* [PATCH] gpio_keys ev_sw support
@ 2007-04-10 16:53 Roman Moravcik
2007-04-10 18:20 ` Dmitry Torokhov
0 siblings, 1 reply; 5+ messages in thread
From: Roman Moravcik @ 2007-04-10 16:53 UTC (permalink / raw)
To: linux-input; +Cc: kernel-discuss
This patch adds software event support to gpio_keys driver.
Signed-off-by: Roman Moravcik <roman.moravcik@gmail.com>
diff -Naur a/drivers/input/keyboard/gpio_keys.c
b/drivers/input/keyboard/gpio_keys.c
--- a/drivers/input/keyboard/gpio_keys.c 2007-02-25
19:50:43.000000000 +0100
+++ b/drivers/input/keyboard/gpio_keys.c 2007-04-05
22:53:02.000000000 +0200
@@ -39,7 +39,10 @@
if (irq == gpio_to_irq(gpio)) {
int state = (gpio_get_value(gpio) ? 1 : 0) ^
(pdata->buttons[i].active_low);
- input_report_key(input, pdata->buttons[i].keycode, state);
+ if (pdata->buttons[i].type == EV_SW)
+ input_report_switch(input, pdata->buttons[i].keycode,
state);
+ else
+ input_report_key(input, pdata->buttons[i].keycode, state);
input_sync(input);
}
}
@@ -59,7 +62,7 @@
platform_set_drvdata(pdev, input);
- input->evbit[0] = BIT(EV_KEY);
+ input->evbit[0] = BIT(EV_KEY) | BIT(EV_SW);
input->name = pdev->name;
input->phys = "gpio-keys/input0";
@@ -84,7 +87,10 @@
irq, error);
goto fail;
}
- set_bit(code, input->keybit);
+ if (pdata->buttons[i].type == EV_SW)
+ set_bit(code, input->swbit);
+ else
+ set_bit(code, input->keybit);
}
error = input_register_device(input);
diff -Naur a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
--- a/include/linux/gpio_keys.h 2007-02-25 16:21:52.000000000 +0100
+++ b/include/linux/gpio_keys.h 2007-04-05 20:23:31.000000000 +0200
@@ -6,6 +6,7 @@
int gpio;
int active_low;
char *desc;
+ int type;
};
struct gpio_keys_platform_data {
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio_keys ev_sw support
2007-04-10 16:53 [PATCH] gpio_keys ev_sw support Roman Moravcik
@ 2007-04-10 18:20 ` Dmitry Torokhov
2007-04-11 8:06 ` Roman Moravcik
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2007-04-10 18:20 UTC (permalink / raw)
To: Roman Moravcik; +Cc: linux-input, kernel-discuss
Hi Roman,
On 4/10/07, Roman Moravcik <roman.moravcik@gmail.com> wrote:
> This patch adds software event support to gpio_keys driver.
>
Just for the record EV_SW is not software event but switch event.
Switch, as opposed to a key that returns to "off" state when released,
keeps it's state until toggled again.
> Signed-off-by: Roman Moravcik <roman.moravcik@gmail.com>
>
> diff -Naur a/drivers/input/keyboard/gpio_keys.c
> b/drivers/input/keyboard/gpio_keys.c
> --- a/drivers/input/keyboard/gpio_keys.c 2007-02-25
> 19:50:43.000000000 +0100
> +++ b/drivers/input/keyboard/gpio_keys.c 2007-04-05
> 22:53:02.000000000 +0200
> @@ -39,7 +39,10 @@
> if (irq == gpio_to_irq(gpio)) {
> int state = (gpio_get_value(gpio) ? 1 : 0) ^
> (pdata->buttons[i].active_low);
>
> - input_report_key(input, pdata->buttons[i].keycode, state);
> + if (pdata->buttons[i].type == EV_SW)
> + input_report_switch(input, pdata->buttons[i].keycode,
> state);
> + else
> + input_report_key(input, pdata->buttons[i].keycode, state);
> input_sync(input);
> }
> }
> @@ -59,7 +62,7 @@
>
> platform_set_drvdata(pdev, input);
>
> - input->evbit[0] = BIT(EV_KEY);
> + input->evbit[0] = BIT(EV_KEY) | BIT(EV_SW);
>
You should not set these bits unconditionally but rather scan supplied
keymap and set appropriately.
> input->name = pdev->name;
> input->phys = "gpio-keys/input0";
> @@ -84,7 +87,10 @@
> irq, error);
> goto fail;
> }
> - set_bit(code, input->keybit);
> + if (pdata->buttons[i].type == EV_SW)
> + set_bit(code, input->swbit);
> + else
> + set_bit(code, input->keybit);
> }
>
> error = input_register_device(input);
> diff -Naur a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
> --- a/include/linux/gpio_keys.h 2007-02-25 16:21:52.000000000 +0100
> +++ b/include/linux/gpio_keys.h 2007-04-05 20:23:31.000000000 +0200
> @@ -6,6 +6,7 @@
> int gpio;
> int active_low;
> char *desc;
> + int type;
Would not it make sense to keep type close to keycode element?
--
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio_keys ev_sw support
2007-04-10 18:20 ` Dmitry Torokhov
@ 2007-04-11 8:06 ` Roman Moravcik
2007-04-13 21:13 ` Paul Sokolovsky
0 siblings, 1 reply; 5+ messages in thread
From: Roman Moravcik @ 2007-04-11 8:06 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, kernel-discuss
Hi Dmitry,
Dmitry Torokhov wrote / napísal(a):
>> This patch adds software event support to gpio_keys driver.
>>
>
> Just for the record EV_SW is not software event but switch event.
> Switch, as opposed to a key that returns to "off" state when released,
> keeps it's state until toggled again.
Ops, sorry. Yes, EV_SW is switch event not software event.
I don't why i wrote software.
>>
>> - input->evbit[0] = BIT(EV_KEY);
>> + input->evbit[0] = BIT(EV_KEY) | BIT(EV_SW);
>>
>
> You should not set these bits unconditionally but rather scan supplied
> keymap and set appropriately.
>
Ok, EV_SW bit is set conditionally now.
>> --- a/include/linux/gpio_keys.h 2007-02-25 16:21:52.000000000 +0100
>> +++ b/include/linux/gpio_keys.h 2007-04-05 20:23:31.000000000 +0200
>> @@ -6,6 +6,7 @@
>> int gpio;
>> int active_low;
>> char *desc;
>> + int type;
>
> Would not it make sense to keep type close to keycode element?
>
Moved behind the keycode.
Roman
This patch adds switch event support to gpio_keys driver.
Signed-off-by: Roman Moravcik <roman.moravcik@gmail.com>
diff -Naur a/drivers/input/keyboard/gpio_keys.c
b/drivers/input/keyboard/gpio_keys.c
--- a/drivers/input/keyboard/gpio_keys.c 2007-02-25
19:50:43.000000000 +0100
+++ b/drivers/input/keyboard/gpio_keys.c 2007-04-10
20:42:31.000000000 +0200
@@ -39,7 +39,10 @@
if (irq == gpio_to_irq(gpio)) {
int state = (gpio_get_value(gpio) ? 1 : 0) ^
(pdata->buttons[i].active_low);
- input_report_key(input, pdata->buttons[i].keycode, state);
+ if (pdata->buttons[i].type == EV_SW)
+ input_report_switch(input, pdata->buttons[i].keycode,
state);
+ else
+ input_report_key(input, pdata->buttons[i].keycode, state);
input_sync(input);
}
}
@@ -84,7 +87,12 @@
irq, error);
goto fail;
}
- set_bit(code, input->keybit);
+ if (pdata->buttons[i].type == EV_SW) {
+ input->evbit[0] |= BIT(EV_SW);
+ set_bit(code, input->swbit);
+ } else {
+ set_bit(code, input->keybit);
+ }
}
error = input_register_device(input);
diff -Naur a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
--- a/include/linux/gpio_keys.h 2007-02-25 16:21:52.000000000 +0100
+++ b/include/linux/gpio_keys.h 2007-04-10 20:31:25.000000000 +0200
@@ -3,6 +3,7 @@
struct gpio_keys_button {
/* Configuration parameters */
int keycode;
+ int type;
int gpio;
int active_low;
char *desc;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio_keys ev_sw support
2007-04-11 8:06 ` Roman Moravcik
@ 2007-04-13 21:13 ` Paul Sokolovsky
2007-04-13 21:24 ` Dmitry Torokhov
0 siblings, 1 reply; 5+ messages in thread
From: Paul Sokolovsky @ 2007-04-13 21:13 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Roman Moravcik, linux-input, kernel-discuss
Hello Dmitry,
I wonder if you're satisfied with Roman patch now. I guess it could just fall to the bottom of your queue due to bad formatting. So, here's resend, hopefully correct. One change is that we had to put "type" member to the end of struct gpio_keys_button, the reason is backwards compatibility. As you see, we'd like EV_KEY to be still default, and nay other type to be used only if explicitly specified. And as structure is already big, and there're usually several keys to define, we use positional initialization to save space:
{ KEY_ENTER, GPIO_NR_H4000_ACTION_BUTTON_N, 1, "Action button" },
So, putting type after keycode will break this, and moreover compiler issues only warning, so that may go unnoticed.
Thanks,
Paul
-------------
This patch adds switch event support to gpio_keys driver.
Signed-off-by: Roman Moravcik <roman.moravcik@gmail.com>
Signed-off-by: Paul Sokolovsky <pmiscml@gmail.com>
Index: include/linux/gpio_keys.h
===================================================================
RCS file: /cvs/linux/kernel26/include/linux/gpio_keys.h,v
retrieving revision 1.1
diff -u -p -r1.1 gpio_keys.h
--- include/linux/gpio_keys.h 25 Feb 2007 15:21:52 -0000 1.1
+++ include/linux/gpio_keys.h 13 Apr 2007 21:07:03 -0000
@@ -6,6 +6,7 @@ struct gpio_keys_button {
int gpio;
int active_low;
char *desc;
+ int type;
};
struct gpio_keys_platform_data {
Index: drivers/input/keyboard/gpio_keys.c
===================================================================
RCS file: /cvs/linux/kernel26/drivers/input/keyboard/gpio_keys.c,v
retrieving revision 1.15
diff -u -p -r1.15 gpio_keys.c
--- drivers/input/keyboard/gpio_keys.c 25 Feb 2007 15:26:10 -0000 1.15
+++ drivers/input/keyboard/gpio_keys.c 13 Apr 2007 21:07:03 -0000
@@ -39,7 +39,10 @@ static irqreturn_t gpio_keys_isr(int irq
if (irq == gpio_to_irq(gpio)) {
int state = (gpio_get_value(gpio) ? 1 : 0) ^ (pdata->buttons[i].active_low);
- input_report_key(input, pdata->buttons[i].keycode, state);
+ if (pdata->buttons[i].type == EV_SW)
+ input_report_switch(input, pdata->buttons[i].keycode, state);
+ else
+ input_report_key(input, pdata->buttons[i].keycode, state);
input_sync(input);
}
}
@@ -84,7 +87,12 @@ static int __devinit gpio_keys_probe(str
irq, error);
goto fail;
}
- set_bit(code, input->keybit);
+ if (pdata->buttons[i].type == EV_SW) {
+ input->evbit[0] |= BIT(EV_SW);
+ set_bit(code, input->swbit);
+ } else {
+ set_bit(code, input->keybit);
+ }
}
error = input_register_device(input);
Wednesday, April 11, 2007, 11:06:17 AM, you wrote:
> Hi Dmitry,
> Dmitry Torokhov wrote / napísal(a):
[]
--
Best regards,
Paul mailto:pmiscml@gmail.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio_keys ev_sw support
2007-04-13 21:13 ` Paul Sokolovsky
@ 2007-04-13 21:24 ` Dmitry Torokhov
0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2007-04-13 21:24 UTC (permalink / raw)
To: Paul Sokolovsky; +Cc: Roman Moravcik, linux-input, kernel-discuss
Hi Paul,
On 4/13/07, Paul Sokolovsky <pmiscml@gmail.com> wrote:
> Hello Dmitry,
>
> I wonder if you're satisfied with Roman patch now. I guess it could just fall to the bottom of your queue due to bad formatting. So, here's resend, hopefully correct. One change is that we had to put "type" member to the end of struct gpio_keys_button, the reason is backwards compatibility. As you see, we'd like EV_KEY to be still default, and nay other type to be used only if explicitly specified. And as structure is already big, and there're usually several keys to define, we use positional initialization to save space:
>
> { KEY_ENTER, GPIO_NR_H4000_ACTION_BUTTON_N, 1, "Action button" },
>
> So, putting type after keycode will break this, and moreover compiler issues only warning, so that may go unnoticed.
>
OK.
>
> Thanks,
> Paul
>
>
> -------------
> This patch adds switch event support to gpio_keys driver.
>
> Signed-off-by: Roman Moravcik <roman.moravcik@gmail.com>
> Signed-off-by: Paul Sokolovsky <pmiscml@gmail.com>
>
>
> Index: include/linux/gpio_keys.h
> ===================================================================
> RCS file: /cvs/linux/kernel26/include/linux/gpio_keys.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 gpio_keys.h
> --- include/linux/gpio_keys.h 25 Feb 2007 15:21:52 -0000 1.1
> +++ include/linux/gpio_keys.h 13 Apr 2007 21:07:03 -0000
> @@ -6,6 +6,7 @@ struct gpio_keys_button {
> int gpio;
> int active_low;
> char *desc;
> + int type;
> };
>
> struct gpio_keys_platform_data {
> Index: drivers/input/keyboard/gpio_keys.c
> ===================================================================
> RCS file: /cvs/linux/kernel26/drivers/input/keyboard/gpio_keys.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 gpio_keys.c
> --- drivers/input/keyboard/gpio_keys.c 25 Feb 2007 15:26:10 -0000 1.15
> +++ drivers/input/keyboard/gpio_keys.c 13 Apr 2007 21:07:03 -0000
> @@ -39,7 +39,10 @@ static irqreturn_t gpio_keys_isr(int irq
> if (irq == gpio_to_irq(gpio)) {
> int state = (gpio_get_value(gpio) ? 1 : 0) ^ (pdata->buttons[i].active_low);
>
> - input_report_key(input, pdata->buttons[i].keycode, state);
> + if (pdata->buttons[i].type == EV_SW)
> + input_report_switch(input, pdata->buttons[i].keycode, state);
> + else
> + input_report_key(input, pdata->buttons[i].keycode, state);
> input_sync(input);
> }
> }
> @@ -84,7 +87,12 @@ static int __devinit gpio_keys_probe(str
> irq, error);
> goto fail;
> }
> - set_bit(code, input->keybit);
> + if (pdata->buttons[i].type == EV_SW) {
> + input->evbit[0] |= BIT(EV_SW);
> + set_bit(code, input->swbit);
> + } else {
> + set_bit(code, input->keybit);
> + }
> }
One thing that I was just going to write Roman about is that we need
to set EV_KEY in the same fashion we set EV_SW - conditionally, on a
slim chance there is a keymap with switches only. I will fix it up,
you don't need to resend.
--
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-04-13 21:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-04-10 16:53 [PATCH] gpio_keys ev_sw support Roman Moravcik
2007-04-10 18:20 ` Dmitry Torokhov
2007-04-11 8:06 ` Roman Moravcik
2007-04-13 21:13 ` Paul Sokolovsky
2007-04-13 21:24 ` Dmitry Torokhov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).