* Dmitry Torokhov <dmitry.torok...@gmail.com> [210111 04:48]:
> Technically speaking, userspace is free to accumulate the events until
> it receives EV_SYN/SYN_REPORT event and process the events in the event
> packet in order it sees fit. So to achieve what you want, I think we
> should issue 2 input_sync()s, one for the release block, and another is
> for press. I think we can also simplify the code if we pass into the new
> scan function exact set of keys that are being released or pressed.
> 
> How about the version below?

Yes that works nicely.

Thanks,

Tony


> 
> Input: omap4-keypad - scan keys in two phases and simplify with bitmask
> 
> From: Tony Lindgren <t...@atomide.com>
> 
> Because of errata i689 the keyboard can idle with state where no key
> up interrupts are seen until after the next key press.
> 
> This means we need to first check for any lost key up events before
> scanning for new down events.
> 
> For example, rapidly pressing shift-shift-j can sometimes produce a J
> instead of j. Let's fix the issue by scanning the keyboard in two
> phases. First we scan for any key up events that we may have missed,
> and then we scan for key down events.
> 
> Let's also simplify things with for_each_set_bit() as suggested by
> Dmitry Torokhov <dmitry.torok...@gmail.com>.
> 
> Signed-off-by: Tony Lindgren <t...@atomide.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
> ---
>  drivers/input/keyboard/omap4-keypad.c |   73 
> ++++++++++++++++++++-------------
>  1 file changed, 45 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/input/keyboard/omap4-keypad.c 
> b/drivers/input/keyboard/omap4-keypad.c
> index ab761aa66b6d..6dcf27af856d 100644
> --- a/drivers/input/keyboard/omap4-keypad.c
> +++ b/drivers/input/keyboard/omap4-keypad.c
> @@ -78,7 +78,7 @@ struct omap4_keypad {
>       u32 irqreg_offset;
>       unsigned int row_shift;
>       bool no_autorepeat;
> -     unsigned char key_state[8];
> +     u64 keys;
>       unsigned short *keymap;
>  };
>  
> @@ -107,6 +107,33 @@ static void kbd_write_irqreg(struct omap4_keypad 
> *keypad_data,
>                    keypad_data->base + keypad_data->irqreg_offset + offset);
>  }
>  
> +static int omap4_keypad_report_keys(struct omap4_keypad *keypad_data,
> +                                 u64 keys, bool down)
> +{
> +     struct input_dev *input_dev = keypad_data->input;
> +     unsigned int col, row, code;
> +     DECLARE_BITMAP(mask, 64);
> +     unsigned long bit;
> +     int events = 0;
> +
> +     bitmap_from_u64(mask, keys);
> +
> +     for_each_set_bit(bit, mask, keypad_data->rows * BITS_PER_BYTE) {
> +             row = bit / BITS_PER_BYTE;
> +             col = bit % BITS_PER_BYTE;
> +             code = MATRIX_SCAN_CODE(row, col, keypad_data->row_shift);
> +
> +             input_event(input_dev, EV_MSC, MSC_SCAN, code);
> +             input_report_key(input_dev, keypad_data->keymap[code], down);
> +
> +             events++;
> +     }
> +
> +     if (events)
> +             input_sync(input_dev);
> +
> +     return events;
> +}
>  
>  /* Interrupt handlers */
>  static irqreturn_t omap4_keypad_irq_handler(int irq, void *dev_id)
> @@ -122,35 +149,25 @@ static irqreturn_t omap4_keypad_irq_handler(int irq, 
> void *dev_id)
>  static irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id)
>  {
>       struct omap4_keypad *keypad_data = dev_id;
> -     struct input_dev *input_dev = keypad_data->input;
> -     unsigned char key_state[ARRAY_SIZE(keypad_data->key_state)];
> -     unsigned int col, row, code, changed;
> -     u32 *new_state = (u32 *) key_state;
> -
> -     *new_state = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE31_0);
> -     *(new_state + 1) = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE63_32);
> -
> -     for (row = 0; row < keypad_data->rows; row++) {
> -             changed = key_state[row] ^ keypad_data->key_state[row];
> -             if (!changed)
> -                     continue;
> -
> -             for (col = 0; col < keypad_data->cols; col++) {
> -                     if (changed & (1 << col)) {
> -                             code = MATRIX_SCAN_CODE(row, col,
> -                                             keypad_data->row_shift);
> -                             input_event(input_dev, EV_MSC, MSC_SCAN, code);
> -                             input_report_key(input_dev,
> -                                              keypad_data->keymap[code],
> -                                              key_state[row] & (1 << col));
> -                     }
> -             }
> -     }
> +     u32 low, high;
> +     u64 keys, changed;
> +
> +     low = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE31_0);
> +     high = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE63_32);
> +     keys = low | (u64)high << 32;
> +
> +     changed = keys ^ keypad_data->keys;
> +
> +     /*
> +      * Report key up events separately and first. This matters in case we
> +      * lost key-up interrupt and just now catching up.
> +      */
> +     omap4_keypad_report_keys(keypad_data, changed & ~keys, false);
>  
> -     input_sync(input_dev);
> +     /* Report key down events */
> +     omap4_keypad_report_keys(keypad_data, changed & keys, true);
>  
> -     memcpy(keypad_data->key_state, key_state,
> -             sizeof(keypad_data->key_state));
> +     keypad_data->keys = keys;
>  
>       /* clear pending interrupts */
>       kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,

Reply via email to