gpio_keys ev_sw support
Subsystems (2)
PATCH 1/1 gpio_keys ev_sw support
This patch adds software event support to gpio_keys driver.
drivers/input/keyboard/gpio_keys.c 2007-04-05 22:53:02.000000000 +0200 renamed from drivers/input/keyboard/gpio_keys.c 2007-02-25 19:50:43.000000000 +0100 +0 -0
@@ -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);include/linux/gpio_keys.h 2007-04-05 20:23:31.000000000 +0200 renamed from include/linux/gpio_keys.h 2007-02-25 16:21:52.000000000 +0100 +1 -0
@@ -6,6 +6,7 @@ int gpio; int active_low; char *desc; + int type;
};
struct gpio_keys_platform_data {
Dmitry Torokhov
maintainerHi Roman,
Quoted 2 lines · Roman Moravcik
This patch adds software event support to
gpio_keysdriver.
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.
Quoted 23 lines
Signed-off-by Roman Moravcik <[email protected]>diff -Naur
a/drivers/input/keyboard/gpio_keys.cb/drivers/input/keyboard/gpio_keys.c
---a/drivers/input/keyboard/gpio_keys.c2007-02-25
19:50:43.000000000 +0100
+++b/drivers/input/keyboard/gpio_keys.c2007-04-05
22:53:02.000000000 +0200(quoted diff) +4
-1@@ -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);} }
Review line 66
Quoted 6 lines
(quoted diff) +1
-1@@ -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.
Quoted 18 lines
(quoted diff) +4
-1input->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.hb/include/linux/gpio_keys.h
include/linux/gpio_keys.h 2007-04-05 20:23:31.000000000 +0200 line 9
Quoted 4 lines
include/linux/gpio_keys.h 2007-04-05 20:23:31.000000000 +0200 renamed from include/linux/gpio_keys.h 2007-02-25 16:21:52.000000000 +0100 +1
-0@@ -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?
Signature · 1 line
Dmitry
Hi Dmitry,
Dmitry Torokhov wrote / napísal(a):
Quoted 6 lines
Quoted 2 lines
This patch adds software event support to
gpio_keysdriver.Just for the record
EV_SWis 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.
Quoted 8 lines
Review
Quoted 4 lines
(quoted diff) +1
-1- 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.
Quoted 10 lines
include/linux/gpio_keys.h 2007-04-05 20:23:31.000000000 +0200 line 9
Quoted 7 lines
include/linux/gpio_keys.h 2007-04-05 20:23:31.000000000 +0200 renamed from include/linux/gpio_keys.h 2007-02-25 16:21:52.000000000 +0100 +1
-0@@ -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.
drivers/input/keyboard/gpio_keys.c 2007-04-10 20:42:31.000000000 +0200 renamed from drivers/input/keyboard/gpio_keys.c 2007-02-25 19:50:43.000000000 +0100 +0 -0
@@ -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);include/linux/gpio_keys.h 2007-04-10 20:31:25.000000000 +0200 renamed from include/linux/gpio_keys.h 2007-02-25 16:21:52.000000000 +0100 +0 -0
@@ -3,6 +3,7 @@
struct gpio_keys_button {
/* Configuration parameters */
int keycode;
+ int type;
int gpio;
int active_low;
char *desc;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.
Index: include/linux/gpio_keys.h
===================================================================
RCS file: /cvs/linux/kernel26/include/linux/gpio_keys.h,v retrieving revision 1.1
linux/gpio_keys.h +1 -0
@@ -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
input/keyboard/gpio_keys.c +10 -2
@@ -39,7 +39,10 @@ static irqreturn_t gpio_keys_isr(int irqif (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);
Quoted 3 lines · you
Hi Dmitry,
Dmitry Torokhov wrote / napísal(a):
[]
Signature · 2 lines
Best regards, Paul mailto:[email protected]
Dmitry Torokhov
maintainerHi Paul,
Quoted 8 lines · Paul Sokolovsky
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 likeEV_KEYto 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.
Quoted 48 lines
Thanks,
Paul-------------
This patch adds switch event support togpio_keysdriver.Signed-off-by Roman Moravcik <[email protected]>Signed-off-by Paul Sokolovsky <[email protected]>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.1gpio_keys.hlinux/gpio_keys.h 13 Apr 2007 21:07:03 -0000 renamed from linux/gpio_keys.h 25 Feb 2007 15:21:52 -0000 1.1 +1
-0@@ -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.15gpio_keys.cinput/keyboard/gpio_keys.c 13 Apr 2007 21:07:03 -0000 renamed from input/keyboard/gpio_keys.c 25 Feb 2007 15:26:10 -0000 1.15 +4
-1@@ -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);
input/keyboard/gpio_keys.c 13 Apr 2007 21:07:03 -0000 static int __devinit gpio_keys_probe(str · line 96
Quoted 10 lines
input/keyboard/gpio_keys.c 13 Apr 2007 21:07:03 -0000 renamed from input/keyboard/gpio_keys.c 25 Feb 2007 15:26:10 -0000 1.15 +6
-1goto 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.
Signature · 1 line
Dmitry