Messages in this thread | | | Date | Wed, 2 Feb 2022 09:07:17 +0100 | From | Ondřej Jirman <> | Subject | Re: [PATCH 2/5] Input: pinephone-keyboard - Add PinePhone keyboard driver |
| |
Hello Samuel,
On Sat, Jan 29, 2022 at 05:00:39PM -0600, Samuel Holland wrote: > The official Pine64 PinePhone keyboard case contains a matrix keypad and > a MCU which runs a libre firmware. Add support for its I2C interface. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > ---
> [...]
> + > + ppkb->buf_swap = !ppkb->buf_swap; > + > + keymap = ppkb->fn_state ? ppkb->fn_keymap : ppkb->input->keycode; > + for (col = 0; col < ppkb->cols; ++col) { > + u8 old = *(++old_buf); > + u8 new = *(++new_buf); > + u8 changed = old ^ new; > + > + for (row = 0; row < ppkb->rows; ++row) { > + int code = MATRIX_SCAN_CODE(row, col, ppkb->row_shift); > + int value = new & BIT(row); > + > + if (!(changed & BIT(row))) > + continue; > + > + dev_dbg(dev, "row %u col %u %sed\n", > + row, col, value ? "press" : "releas"); > + if (keymap[code] == KEY_FN) { > + dev_dbg(dev, "FN is now %sed\n", > + value ? "press" : "releas"); > + keymap = value ? ppkb->fn_keymap > + : ppkb->input->keycode; > + ppkb->fn_state = value; > + } > + input_event(ppkb->input, EV_MSC, MSC_SCAN, code); > + input_report_key(ppkb->input, keymap[code], value);
I think there's a logic issue here with the Fn layer. Consider what happens when you press Fn press F1 and then release Fn and release F1. In that case input_report_key will report press of F1 (in fn layer) but release of '1' which is not in Fn layer, because Fn layer was de-activated before releasing the modified key.
From the PoV of the user, this will probably lead to auto-repeat of F1 and spurious '1' release without preceding press event. So the userspace sees F1 as stuck and auto-repeats it.
kind regards, o.
> + } > + } > + input_sync(ppkb->input); > +} > +
| |