* [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).