Commit 8f7c6b8d authored by David Brownell's avatar David Brownell Committed by Kevin Hilman

dm355evm_keys review updates

Address feedback from Dmitry:
 - input_sync() per event
 - maintain dev->keybit when remapping keys
 - since we handle remapping, keycodemax and keycodesize aren't used
 - on probe error, don't input_free_device() unless it registered

Also:
 - avoid reporting excess events
 - more bad-parameter paranoia in the remapping code

The excess event issue is basically that we don't have a way to
distinguish "button press" events from "button release" ones or
autorepeat events, so we should try being a bit smarter about
synthesizing them.
Signed-off-by: default avatarDavid Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: default avatarKevin Hilman <khilman@deeprootsystems.com>
parent 909444c6
...@@ -28,6 +28,8 @@ ...@@ -28,6 +28,8 @@
* using a work_struct. The IRQ is active low, but we use it through * using a work_struct. The IRQ is active low, but we use it through
* the GPIO controller so we can trigger on falling edges. * the GPIO controller so we can trigger on falling edges.
* *
* Note that physically there can only be one of these devices.
*
* This driver was tested with firmware revision A4. * This driver was tested with firmware revision A4.
*/ */
struct dm355evm_keys { struct dm355evm_keys {
...@@ -120,6 +122,8 @@ static void dm355evm_keys_work(struct work_struct *work) ...@@ -120,6 +122,8 @@ static void dm355evm_keys_work(struct work_struct *work)
* Reading INPUT_LOW decrements the count. * Reading INPUT_LOW decrements the count.
*/ */
for (;;) { for (;;) {
static u16 last_event;
u16 event; u16 event;
int keycode; int keycode;
int i; int i;
...@@ -142,6 +146,23 @@ static void dm355evm_keys_work(struct work_struct *work) ...@@ -142,6 +146,23 @@ static void dm355evm_keys_work(struct work_struct *work)
if (event == 0xdead) if (event == 0xdead)
break; break;
/* Press and release a button: two events, same code.
* Press and hold (autorepeat), then release: N events
* (N > 2), same code. For RC5 buttons the toggle bits
* distinguish (for example) "1-autorepeat" from "1 1";
* but PCB buttons don't support that bit.
*
* So we must synthesize release events. We do that by
* mapping events to a press/release event pair; then
* to avoid adding extra events, skip the second event
* of each pair.
*/
if (event == last_event) {
last_event = 0;
continue;
}
last_event = event;
/* ignore the RC5 toggle bit */ /* ignore the RC5 toggle bit */
event &= ~0x0800; event &= ~0x0800;
...@@ -157,28 +178,38 @@ static void dm355evm_keys_work(struct work_struct *work) ...@@ -157,28 +178,38 @@ static void dm355evm_keys_work(struct work_struct *work)
"input event 0x%04x--> keycode %d\n", "input event 0x%04x--> keycode %d\n",
event, keycode); event, keycode);
/* Report press + release ... we can't tell if /* report press + release */
* this is an autorepeat, and we need to guess
* about the release.
*/
input_report_key(keys->input, keycode, 1); input_report_key(keys->input, keycode, 1);
input_sync(keys->input);
input_report_key(keys->input, keycode, 0); input_report_key(keys->input, keycode, 0);
input_sync(keys->input);
} }
input_sync(keys->input);
} }
static int dm355evm_setkeycode(struct input_dev *dev, int index, int keycode) static int dm355evm_setkeycode(struct input_dev *dev, int index, int keycode)
{ {
if (index >= ARRAY_SIZE(dm355evm_keys)) u16 old_keycode;
unsigned i;
if (((unsigned)index) >= ARRAY_SIZE(dm355evm_keys))
return -EINVAL; return -EINVAL;
old_keycode = dm355evm_keys[index].keycode;
dm355evm_keys[index].keycode = keycode; dm355evm_keys[index].keycode = keycode;
set_bit(keycode, dev->keybit);
for (i = 0; i < ARRAY_SIZE(dm355evm_keys); i++) {
if (dm355evm_keys[index].keycode == old_keycode)
goto done;
}
clear_bit(old_keycode, dev->keybit);
done:
return 0; return 0;
} }
static int dm355evm_getkeycode(struct input_dev *dev, int index, int *keycode) static int dm355evm_getkeycode(struct input_dev *dev, int index, int *keycode)
{ {
if (index >= ARRAY_SIZE(dm355evm_keys)) if (((unsigned)index) >= ARRAY_SIZE(dm355evm_keys))
return -EINVAL; return -EINVAL;
return dm355evm_keys[index].keycode; return dm355evm_keys[index].keycode;
...@@ -219,8 +250,6 @@ static int __devinit dm355evm_keys_probe(struct platform_device *pdev) ...@@ -219,8 +250,6 @@ static int __devinit dm355evm_keys_probe(struct platform_device *pdev)
for (i = 0; i < ARRAY_SIZE(dm355evm_keys); i++) for (i = 0; i < ARRAY_SIZE(dm355evm_keys); i++)
set_bit(dm355evm_keys[i].keycode, input->keybit); set_bit(dm355evm_keys[i].keycode, input->keybit);
input->keycodemax = ARRAY_SIZE(dm355evm_keys);
input->keycodesize = sizeof(dm355evm_keys[0]);
input->keycode = dm355evm_keys; input->keycode = dm355evm_keys;
input->setkeycode = dm355evm_setkeycode; input->setkeycode = dm355evm_setkeycode;
input->getkeycode = dm355evm_getkeycode; input->getkeycode = dm355evm_getkeycode;
...@@ -237,7 +266,7 @@ static int __devinit dm355evm_keys_probe(struct platform_device *pdev) ...@@ -237,7 +266,7 @@ static int __devinit dm355evm_keys_probe(struct platform_device *pdev)
/* register */ /* register */
status = input_register_device(input); status = input_register_device(input);
if (status < 0) if (status < 0)
goto fail1; goto fail0;
/* start reporting events */ /* start reporting events */
status = request_irq(keys->irq, dm355evm_keys_irq, status = request_irq(keys->irq, dm355evm_keys_irq,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment