Convert the next-kbd device from the legacy UI qemu_add_kbd_event_handler()
function to use qemu_input_handler_register().
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/m68k/next-kbd.c | 166 ++++++++++++++++++++++++++++++---------------
1 file changed, 111 insertions(+), 55 deletions(-)
diff --git a/hw/m68k/next-kbd.c b/hw/m68k/next-kbd.c
index bc67810f31..283e98e9eb 100644
--- a/hw/m68k/next-kbd.c
+++ b/hw/m68k/next-kbd.c
@@ -68,7 +68,6 @@ struct NextKBDState {
uint16_t shift;
};
-static void queue_code(void *opaque, int code);
/* lots of magic numbers here */
static uint32_t kbd_read_byte(void *opaque, hwaddr addr)
@@ -166,68 +165,79 @@ static const MemoryRegionOps kbd_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static void nextkbd_event(void *opaque, int ch)
-{
- /*
- * Will want to set vars for caps/num lock
- * if (ch & 0x80) -> key release
- * there's also e0 escaped scancodes that might need to be handled
- */
- queue_code(opaque, ch);
-}
-
-static const unsigned char next_keycodes[128] = {
- 0x00, 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x50, 0x4F,
- 0x4E, 0x1E, 0x1F, 0x20, 0x1D, 0x1C, 0x1B, 0x00,
- 0x42, 0x43, 0x44, 0x45, 0x48, 0x47, 0x46, 0x06,
- 0x07, 0x08, 0x00, 0x00, 0x2A, 0x00, 0x39, 0x3A,
- 0x3B, 0x3C, 0x3D, 0x40, 0x3F, 0x3E, 0x2D, 0x2C,
- 0x2B, 0x26, 0x00, 0x00, 0x31, 0x32, 0x33, 0x34,
- 0x35, 0x37, 0x36, 0x2e, 0x2f, 0x30, 0x00, 0x00,
- 0x00, 0x38, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
+#define NEXTKBD_NO_KEY 0xff
+
+static const int qcode_to_nextkbd_keycode[] = {
+ /* Make sure future additions are automatically set to NEXTKBD_NO_KEY */
+ [0 ... Q_KEY_CODE__MAX] = NEXTKBD_NO_KEY,
+
+ [Q_KEY_CODE_ESC] = 0x49,
+ [Q_KEY_CODE_1] = 0x4a,
+ [Q_KEY_CODE_2] = 0x4b,
+ [Q_KEY_CODE_3] = 0x4c,
+ [Q_KEY_CODE_4] = 0x4d,
+ [Q_KEY_CODE_5] = 0x50,
+ [Q_KEY_CODE_6] = 0x4f,
+ [Q_KEY_CODE_7] = 0x4e,
+ [Q_KEY_CODE_8] = 0x1e,
+ [Q_KEY_CODE_9] = 0x1f,
+ [Q_KEY_CODE_0] = 0x20,
+ [Q_KEY_CODE_MINUS] = 0x1d,
+ [Q_KEY_CODE_EQUAL] = 0x1c,
+ [Q_KEY_CODE_BACKSPACE] = 0x1b,
+ [Q_KEY_CODE_TAB] = 0x00,
+
+ [Q_KEY_CODE_Q] = 0x42,
+ [Q_KEY_CODE_W] = 0x43,
+ [Q_KEY_CODE_E] = 0x44,
+ [Q_KEY_CODE_R] = 0x45,
+ [Q_KEY_CODE_T] = 0x48,
+ [Q_KEY_CODE_Y] = 0x47,
+ [Q_KEY_CODE_U] = 0x46,
+ [Q_KEY_CODE_I] = 0x06,
+ [Q_KEY_CODE_O] = 0x07,
+ [Q_KEY_CODE_P] = 0x08,
+ [Q_KEY_CODE_RET] = 0x2a,
+ [Q_KEY_CODE_A] = 0x39,
+ [Q_KEY_CODE_S] = 0x3a,
+
+ [Q_KEY_CODE_D] = 0x3b,
+ [Q_KEY_CODE_F] = 0x3c,
+ [Q_KEY_CODE_G] = 0x3d,
+ [Q_KEY_CODE_H] = 0x40,
+ [Q_KEY_CODE_J] = 0x3f,
+ [Q_KEY_CODE_K] = 0x3e,
+ [Q_KEY_CODE_L] = 0x2d,
+ [Q_KEY_CODE_SEMICOLON] = 0x2c,
+ [Q_KEY_CODE_APOSTROPHE] = 0x2b,
+ [Q_KEY_CODE_GRAVE_ACCENT] = 0x26,
+ [Q_KEY_CODE_SHIFT] = 0x00,
+ [Q_KEY_CODE_BACKSLASH] = 0x00,
+ [Q_KEY_CODE_Z] = 0x31,
+ [Q_KEY_CODE_X] = 0x32,
+ [Q_KEY_CODE_C] = 0x33,
+ [Q_KEY_CODE_V] = 0x34,
+
+ [Q_KEY_CODE_B] = 0x35,
+ [Q_KEY_CODE_N] = 0x37,
+ [Q_KEY_CODE_M] = 0x36,
+ [Q_KEY_CODE_COMMA] = 0x2e,
+ [Q_KEY_CODE_DOT] = 0x2f,
+ [Q_KEY_CODE_SLASH] = 0x30,
+ [Q_KEY_CODE_SHIFT_R] = 0x00,
+
+ [Q_KEY_CODE_SPC] = 0x38,
};
-static void queue_code(void *opaque, int code)
+static void nextkbd_put_keycode(NextKBDState *s, int keycode)
{
- NextKBDState *s = NEXTKBD(opaque);
KBDQueue *q = &s->queue;
- int key = code & KD_KEYMASK;
- int release = code & 0x80;
- static int ext;
-
- if (code == 0xE0) {
- ext = 1;
- }
-
- if (code == 0x2A || code == 0x1D || code == 0x36) {
- if (code == 0x2A) {
- s->shift = KD_LSHIFT;
- } else if (code == 0x36) {
- s->shift = KD_RSHIFT;
- ext = 0;
- } else if (code == 0x1D && !ext) {
- s->shift = KD_LCOMM;
- } else if (code == 0x1D && ext) {
- ext = 0;
- s->shift = KD_RCOMM;
- }
- return;
- } else if (code == (0x2A | 0x80) || code == (0x1D | 0x80) ||
- code == (0x36 | 0x80)) {
- s->shift = 0;
- return;
- }
if (q->count >= KBD_QUEUE_SIZE) {
return;
}
- q->data[q->wptr] = next_keycodes[key] | release;
-
+ q->data[q->wptr] = keycode;
if (++q->wptr == KBD_QUEUE_SIZE) {
q->wptr = 0;
}
@@ -241,6 +251,52 @@ static void queue_code(void *opaque, int code)
/* s->update_irq(s->update_arg, 1); */
}
+static void nextkbd_event(DeviceState *dev, QemuConsole *src, InputEvent *evt)
+{
+ NextKBDState *s = NEXTKBD(dev);
+ int qcode, keycode;
+ bool key_down = evt->u.key.data->down;
+
+ qcode = qemu_input_key_value_to_qcode(evt->u.key.data->key);
+ if (qcode >= ARRAY_SIZE(qcode_to_nextkbd_keycode)) {
+ return;
+ }
+
+ keycode = qcode_to_nextkbd_keycode[qcode];
+ if (keycode == NEXTKBD_NO_KEY) {
+ return;
+ }
+
+ if (qcode == Q_KEY_CODE_SHIFT) {
+ if (key_down) {
+ s->shift |= KD_LSHIFT;
+ } else {
+ s->shift &= ~KD_LSHIFT;
+ }
+ }
+
+ if (qcode == Q_KEY_CODE_SHIFT_R) {
+ if (key_down) {
+ s->shift |= KD_RSHIFT;
+ } else {
+ s->shift &= ~KD_RSHIFT;
+ }
+ }
+
+ /* If key release event, create keyboard break code */
+ if (!key_down) {
+ keycode |= 0x80;
+ }
+
+ nextkbd_put_keycode(s, keycode);
+}
+
+static const QemuInputHandler nextkbd_handler = {
+ .name = "QEMU NeXT Keyboard",
+ .mask = INPUT_EVENT_MASK_KEY,
+ .event = nextkbd_event,
+};
+
static void nextkbd_reset(DeviceState *dev)
{
NextKBDState *nks = NEXTKBD(dev);
@@ -256,7 +312,7 @@ static void nextkbd_realize(DeviceState *dev, Error **errp)
memory_region_init_io(&s->mr, OBJECT(dev), &kbd_ops, s, "next.kbd", 0x1000);
sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mr);
- qemu_add_kbd_event_handler(nextkbd_event, s);
+ qemu_input_handler_register(dev, &nextkbd_handler);
}
static const VMStateDescription nextkbd_vmstate = {
--
2.39.5On Fri, Nov 01, 2024 at 08:11:05PM +0000, Mark Cave-Ayland wrote:
> Convert the next-kbd device from the legacy UI qemu_add_kbd_event_handler()
> function to use qemu_input_handler_register().
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/m68k/next-kbd.c | 166 ++++++++++++++++++++++++++++++---------------
> 1 file changed, 111 insertions(+), 55 deletions(-)
>
> diff --git a/hw/m68k/next-kbd.c b/hw/m68k/next-kbd.c
> index bc67810f31..283e98e9eb 100644
> --- a/hw/m68k/next-kbd.c
> +++ b/hw/m68k/next-kbd.c
> -static const unsigned char next_keycodes[128] = {
> - 0x00, 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x50, 0x4F,
> - 0x4E, 0x1E, 0x1F, 0x20, 0x1D, 0x1C, 0x1B, 0x00,
> - 0x42, 0x43, 0x44, 0x45, 0x48, 0x47, 0x46, 0x06,
> - 0x07, 0x08, 0x00, 0x00, 0x2A, 0x00, 0x39, 0x3A,
> - 0x3B, 0x3C, 0x3D, 0x40, 0x3F, 0x3E, 0x2D, 0x2C,
> - 0x2B, 0x26, 0x00, 0x00, 0x31, 0x32, 0x33, 0x34,
> - 0x35, 0x37, 0x36, 0x2e, 0x2f, 0x30, 0x00, 0x00,
> - 0x00, 0x38, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
The original code used '0' (or equivalently 0x00) to
indicate an unmapped scancode.
> +#define NEXTKBD_NO_KEY 0xff
Now you're using 0xff for missing mappings, but....
> +
> +static const int qcode_to_nextkbd_keycode[] = {
> + /* Make sure future additions are automatically set to NEXTKBD_NO_KEY */
> + [0 ... Q_KEY_CODE__MAX] = NEXTKBD_NO_KEY,
> +
> + [Q_KEY_CODE_ESC] = 0x49,
> + [Q_KEY_CODE_1] = 0x4a,
> + [Q_KEY_CODE_2] = 0x4b,
> + [Q_KEY_CODE_3] = 0x4c,
> + [Q_KEY_CODE_4] = 0x4d,
> + [Q_KEY_CODE_5] = 0x50,
> + [Q_KEY_CODE_6] = 0x4f,
> + [Q_KEY_CODE_7] = 0x4e,
> + [Q_KEY_CODE_8] = 0x1e,
> + [Q_KEY_CODE_9] = 0x1f,
> + [Q_KEY_CODE_0] = 0x20,
> + [Q_KEY_CODE_MINUS] = 0x1d,
> + [Q_KEY_CODE_EQUAL] = 0x1c,
> + [Q_KEY_CODE_BACKSPACE] = 0x1b,
> + [Q_KEY_CODE_TAB] = 0x00,
...you've left 0x00 here and....
> +
> + [Q_KEY_CODE_Q] = 0x42,
> + [Q_KEY_CODE_W] = 0x43,
> + [Q_KEY_CODE_E] = 0x44,
> + [Q_KEY_CODE_R] = 0x45,
> + [Q_KEY_CODE_T] = 0x48,
> + [Q_KEY_CODE_Y] = 0x47,
> + [Q_KEY_CODE_U] = 0x46,
> + [Q_KEY_CODE_I] = 0x06,
> + [Q_KEY_CODE_O] = 0x07,
> + [Q_KEY_CODE_P] = 0x08,
> + [Q_KEY_CODE_RET] = 0x2a,
> + [Q_KEY_CODE_A] = 0x39,
> + [Q_KEY_CODE_S] = 0x3a,
> +
> + [Q_KEY_CODE_D] = 0x3b,
> + [Q_KEY_CODE_F] = 0x3c,
> + [Q_KEY_CODE_G] = 0x3d,
> + [Q_KEY_CODE_H] = 0x40,
> + [Q_KEY_CODE_J] = 0x3f,
> + [Q_KEY_CODE_K] = 0x3e,
> + [Q_KEY_CODE_L] = 0x2d,
> + [Q_KEY_CODE_SEMICOLON] = 0x2c,
> + [Q_KEY_CODE_APOSTROPHE] = 0x2b,
> + [Q_KEY_CODE_GRAVE_ACCENT] = 0x26,
> + [Q_KEY_CODE_SHIFT] = 0x00,
> + [Q_KEY_CODE_BACKSLASH] = 0x00,
...here, and ...
> + [Q_KEY_CODE_Z] = 0x31,
> + [Q_KEY_CODE_X] = 0x32,
> + [Q_KEY_CODE_C] = 0x33,
> + [Q_KEY_CODE_V] = 0x34,
> +
> + [Q_KEY_CODE_B] = 0x35,
> + [Q_KEY_CODE_N] = 0x37,
> + [Q_KEY_CODE_M] = 0x36,
> + [Q_KEY_CODE_COMMA] = 0x2e,
> + [Q_KEY_CODE_DOT] = 0x2f,
> + [Q_KEY_CODE_SLASH] = 0x30,
> + [Q_KEY_CODE_SHIFT_R] = 0x00,
...here, which is surely not a correct conversion
> +
> + [Q_KEY_CODE_SPC] = 0x38,
> };
Those missing mappings definitely have values too.
I can see the 'shift' and 'tab' keys on the real Next
keyboard I have in front of me right now :-)
Finding a reliable reference for the NeXT scancodes is very hard
in my googling so far. The best I've come across so far is
https://github.com/spenczar/usb-next/blob/main/keymap.h
who has defined a mapping to USB HID codes, which seems to
broadly match what's above here, plus has many of the gaps
fixed.
Do you know of any other decent references for scancodes ?
I'm going to see about adding NeXT scancodes to the giant
database of keycodes at:
https://gitlab.com/keycodemap/keycodemapdb
then we can auto-generate this table as we do for most of
the other QEMU keyboard drivers.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 04/11/2024 10:04, Daniel P. Berrangé wrote:
> On Fri, Nov 01, 2024 at 08:11:05PM +0000, Mark Cave-Ayland wrote:
>> Convert the next-kbd device from the legacy UI qemu_add_kbd_event_handler()
>> function to use qemu_input_handler_register().
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> hw/m68k/next-kbd.c | 166 ++++++++++++++++++++++++++++++---------------
>> 1 file changed, 111 insertions(+), 55 deletions(-)
>>
>> diff --git a/hw/m68k/next-kbd.c b/hw/m68k/next-kbd.c
>> index bc67810f31..283e98e9eb 100644
>> --- a/hw/m68k/next-kbd.c
>> +++ b/hw/m68k/next-kbd.c
>
>
>> -static const unsigned char next_keycodes[128] = {
>> - 0x00, 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x50, 0x4F,
>> - 0x4E, 0x1E, 0x1F, 0x20, 0x1D, 0x1C, 0x1B, 0x00,
>> - 0x42, 0x43, 0x44, 0x45, 0x48, 0x47, 0x46, 0x06,
>> - 0x07, 0x08, 0x00, 0x00, 0x2A, 0x00, 0x39, 0x3A,
>> - 0x3B, 0x3C, 0x3D, 0x40, 0x3F, 0x3E, 0x2D, 0x2C,
>> - 0x2B, 0x26, 0x00, 0x00, 0x31, 0x32, 0x33, 0x34,
>> - 0x35, 0x37, 0x36, 0x2e, 0x2f, 0x30, 0x00, 0x00,
>> - 0x00, 0x38, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
>> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
>> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
>> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
>
> The original code used '0' (or equivalently 0x00) to
> indicate an unmapped scancode.
>
>> +#define NEXTKBD_NO_KEY 0xff
>
> Now you're using 0xff for missing mappings, but....
>
>> +
>> +static const int qcode_to_nextkbd_keycode[] = {
>> + /* Make sure future additions are automatically set to NEXTKBD_NO_KEY */
>> + [0 ... Q_KEY_CODE__MAX] = NEXTKBD_NO_KEY,
>> +
>> + [Q_KEY_CODE_ESC] = 0x49,
>> + [Q_KEY_CODE_1] = 0x4a,
>> + [Q_KEY_CODE_2] = 0x4b,
>> + [Q_KEY_CODE_3] = 0x4c,
>> + [Q_KEY_CODE_4] = 0x4d,
>> + [Q_KEY_CODE_5] = 0x50,
>> + [Q_KEY_CODE_6] = 0x4f,
>> + [Q_KEY_CODE_7] = 0x4e,
>> + [Q_KEY_CODE_8] = 0x1e,
>> + [Q_KEY_CODE_9] = 0x1f,
>> + [Q_KEY_CODE_0] = 0x20,
>> + [Q_KEY_CODE_MINUS] = 0x1d,
>> + [Q_KEY_CODE_EQUAL] = 0x1c,
>> + [Q_KEY_CODE_BACKSPACE] = 0x1b,
>> + [Q_KEY_CODE_TAB] = 0x00,
>
> ...you've left 0x00 here and....
Ooops yes that line can be removed.
>> + [Q_KEY_CODE_Q] = 0x42,
>> + [Q_KEY_CODE_W] = 0x43,
>> + [Q_KEY_CODE_E] = 0x44,
>> + [Q_KEY_CODE_R] = 0x45,
>> + [Q_KEY_CODE_T] = 0x48,
>> + [Q_KEY_CODE_Y] = 0x47,
>> + [Q_KEY_CODE_U] = 0x46,
>> + [Q_KEY_CODE_I] = 0x06,
>> + [Q_KEY_CODE_O] = 0x07,
>> + [Q_KEY_CODE_P] = 0x08,
>> + [Q_KEY_CODE_RET] = 0x2a,
>> + [Q_KEY_CODE_A] = 0x39,
>> + [Q_KEY_CODE_S] = 0x3a,
>> +
>> + [Q_KEY_CODE_D] = 0x3b,
>> + [Q_KEY_CODE_F] = 0x3c,
>> + [Q_KEY_CODE_G] = 0x3d,
>> + [Q_KEY_CODE_H] = 0x40,
>> + [Q_KEY_CODE_J] = 0x3f,
>> + [Q_KEY_CODE_K] = 0x3e,
>> + [Q_KEY_CODE_L] = 0x2d,
>> + [Q_KEY_CODE_SEMICOLON] = 0x2c,
>> + [Q_KEY_CODE_APOSTROPHE] = 0x2b,
>> + [Q_KEY_CODE_GRAVE_ACCENT] = 0x26,
>> + [Q_KEY_CODE_SHIFT] = 0x00,
This should be kept.
>> + [Q_KEY_CODE_BACKSLASH] = 0x00,
>
> ...here, and ...
Indeed, that line can also be removed.
>> + [Q_KEY_CODE_Z] = 0x31,
>> + [Q_KEY_CODE_X] = 0x32,
>> + [Q_KEY_CODE_C] = 0x33,
>> + [Q_KEY_CODE_V] = 0x34,
>> +
>> + [Q_KEY_CODE_B] = 0x35,
>> + [Q_KEY_CODE_N] = 0x37,
>> + [Q_KEY_CODE_M] = 0x36,
>> + [Q_KEY_CODE_COMMA] = 0x2e,
>> + [Q_KEY_CODE_DOT] = 0x2f,
>> + [Q_KEY_CODE_SLASH] = 0x30,
>> + [Q_KEY_CODE_SHIFT_R] = 0x00,
>
> ...here, which is surely not a correct conversion
And this one should also be kept.
The reason the two shift keys need to be kept as zero is so that they pass the "if
(keycode == NEXTKBD_NO_KEY) { return; }" check in nextkbd_event().
>> +
>> + [Q_KEY_CODE_SPC] = 0x38,
>> };
>
>
> Those missing mappings definitely have values too.
>
> I can see the 'shift' and 'tab' keys on the real Next
> keyboard I have in front of me right now :-)
That's proof enough for me ;)
> Finding a reliable reference for the NeXT scancodes is very hard
> in my googling so far. The best I've come across so far is
>
> https://github.com/spenczar/usb-next/blob/main/keymap.h
>
> who has defined a mapping to USB HID codes, which seems to
> broadly match what's above here, plus has many of the gaps
> fixed.
>
> Do you know of any other decent references for scancodes ?
>
> I'm going to see about adding NeXT scancodes to the giant
> database of keycodes at:
>
> https://gitlab.com/keycodemap/keycodemapdb
>
> then we can auto-generate this table as we do for most of
> the other QEMU keyboard drivers.
That would be great! Is this also possible for the ADB keyboard device, since that is
where I looked for inspiration when looking at next-kbd?
ATB,
Mark.
On Mon, Nov 04, 2024 at 10:46:45PM +0000, Mark Cave-Ayland wrote:
> On 04/11/2024 10:04, Daniel P. Berrangé wrote:
>
> > On Fri, Nov 01, 2024 at 08:11:05PM +0000, Mark Cave-Ayland wrote:
> > > Convert the next-kbd device from the legacy UI qemu_add_kbd_event_handler()
> > > function to use qemu_input_handler_register().
> > >
> > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > > ---
> > > hw/m68k/next-kbd.c | 166 ++++++++++++++++++++++++++++++---------------
> > > 1 file changed, 111 insertions(+), 55 deletions(-)
> > >
> > > diff --git a/hw/m68k/next-kbd.c b/hw/m68k/next-kbd.c
> > > index bc67810f31..283e98e9eb 100644
> > > --- a/hw/m68k/next-kbd.c
> > > +++ b/hw/m68k/next-kbd.c
> >
> >
> > > -static const unsigned char next_keycodes[128] = {
> > > - 0x00, 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x50, 0x4F,
> > > - 0x4E, 0x1E, 0x1F, 0x20, 0x1D, 0x1C, 0x1B, 0x00,
> > > - 0x42, 0x43, 0x44, 0x45, 0x48, 0x47, 0x46, 0x06,
> > > - 0x07, 0x08, 0x00, 0x00, 0x2A, 0x00, 0x39, 0x3A,
> > > - 0x3B, 0x3C, 0x3D, 0x40, 0x3F, 0x3E, 0x2D, 0x2C,
> > > - 0x2B, 0x26, 0x00, 0x00, 0x31, 0x32, 0x33, 0x34,
> > > - 0x35, 0x37, 0x36, 0x2e, 0x2f, 0x30, 0x00, 0x00,
> > > - 0x00, 0x38, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > > - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> > > - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> > > - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> > > - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
> >
> > The original code used '0' (or equivalently 0x00) to
> > indicate an unmapped scancode.
> >
> > > +#define NEXTKBD_NO_KEY 0xff
> >
> > Now you're using 0xff for missing mappings, but....
> >
> > > +
> > > +static const int qcode_to_nextkbd_keycode[] = {
> > > + /* Make sure future additions are automatically set to NEXTKBD_NO_KEY */
> > > + [0 ... Q_KEY_CODE__MAX] = NEXTKBD_NO_KEY,
> > > +
> > > + [Q_KEY_CODE_ESC] = 0x49,
> > > + [Q_KEY_CODE_1] = 0x4a,
> > > + [Q_KEY_CODE_2] = 0x4b,
> > > + [Q_KEY_CODE_3] = 0x4c,
> > > + [Q_KEY_CODE_4] = 0x4d,
> > > + [Q_KEY_CODE_5] = 0x50,
> > > + [Q_KEY_CODE_6] = 0x4f,
> > > + [Q_KEY_CODE_7] = 0x4e,
> > > + [Q_KEY_CODE_8] = 0x1e,
> > > + [Q_KEY_CODE_9] = 0x1f,
> > > + [Q_KEY_CODE_0] = 0x20,
> > > + [Q_KEY_CODE_MINUS] = 0x1d,
> > > + [Q_KEY_CODE_EQUAL] = 0x1c,
> > > + [Q_KEY_CODE_BACKSPACE] = 0x1b,
> > > + [Q_KEY_CODE_TAB] = 0x00,
> >
> > ...you've left 0x00 here and....
>
> Ooops yes that line can be removed.
>
> > > + [Q_KEY_CODE_Q] = 0x42,
> > > + [Q_KEY_CODE_W] = 0x43,
> > > + [Q_KEY_CODE_E] = 0x44,
> > > + [Q_KEY_CODE_R] = 0x45,
> > > + [Q_KEY_CODE_T] = 0x48,
> > > + [Q_KEY_CODE_Y] = 0x47,
> > > + [Q_KEY_CODE_U] = 0x46,
> > > + [Q_KEY_CODE_I] = 0x06,
> > > + [Q_KEY_CODE_O] = 0x07,
> > > + [Q_KEY_CODE_P] = 0x08,
> > > + [Q_KEY_CODE_RET] = 0x2a,
> > > + [Q_KEY_CODE_A] = 0x39,
> > > + [Q_KEY_CODE_S] = 0x3a,
> > > +
> > > + [Q_KEY_CODE_D] = 0x3b,
> > > + [Q_KEY_CODE_F] = 0x3c,
> > > + [Q_KEY_CODE_G] = 0x3d,
> > > + [Q_KEY_CODE_H] = 0x40,
> > > + [Q_KEY_CODE_J] = 0x3f,
> > > + [Q_KEY_CODE_K] = 0x3e,
> > > + [Q_KEY_CODE_L] = 0x2d,
> > > + [Q_KEY_CODE_SEMICOLON] = 0x2c,
> > > + [Q_KEY_CODE_APOSTROPHE] = 0x2b,
> > > + [Q_KEY_CODE_GRAVE_ACCENT] = 0x26,
> > > + [Q_KEY_CODE_SHIFT] = 0x00,
>
> This should be kept.
>
> > > + [Q_KEY_CODE_BACKSLASH] = 0x00,
> >
> > ...here, and ...
>
> Indeed, that line can also be removed.
>
> > > + [Q_KEY_CODE_Z] = 0x31,
> > > + [Q_KEY_CODE_X] = 0x32,
> > > + [Q_KEY_CODE_C] = 0x33,
> > > + [Q_KEY_CODE_V] = 0x34,
> > > +
> > > + [Q_KEY_CODE_B] = 0x35,
> > > + [Q_KEY_CODE_N] = 0x37,
> > > + [Q_KEY_CODE_M] = 0x36,
> > > + [Q_KEY_CODE_COMMA] = 0x2e,
> > > + [Q_KEY_CODE_DOT] = 0x2f,
> > > + [Q_KEY_CODE_SLASH] = 0x30,
> > > + [Q_KEY_CODE_SHIFT_R] = 0x00,
> >
> > ...here, which is surely not a correct conversion
>
> And this one should also be kept.
>
> The reason the two shift keys need to be kept as zero is so that they pass
> the "if (keycode == NEXTKBD_NO_KEY) { return; }" check in nextkbd_event().
IMHO that code sould just be flipped in ordering. Do the
special case
if (qcode == Q_KEY_CODE_SHIFT_R)
handling before even running the table lookup, since the
lookup is redundant, at which point you don't need to
have 2 distinct special values in the lookup table.
> > I'm going to see about adding NeXT scancodes to the giant
> > database of keycodes at:
> >
> > https://gitlab.com/keycodemap/keycodemapdb
> >
> > then we can auto-generate this table as we do for most of
> > the other QEMU keyboard drivers.
>
> That would be great! Is this also possible for the ADB keyboard device,
> since that is where I looked for inspiration when looking at next-kbd?
Yes, that's likely possible - we've already got data in keycodemapdb for
Apple ADB
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Am Mon, 4 Nov 2024 10:04:49 +0000
schrieb Daniel P. Berrangé <berrange@redhat.com>:
> On Fri, Nov 01, 2024 at 08:11:05PM +0000, Mark Cave-Ayland wrote:
> > Convert the next-kbd device from the legacy UI qemu_add_kbd_event_handler()
> > function to use qemu_input_handler_register().
> >
> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > ---
> > hw/m68k/next-kbd.c | 166 ++++++++++++++++++++++++++++++---------------
> > 1 file changed, 111 insertions(+), 55 deletions(-)
> >
> > diff --git a/hw/m68k/next-kbd.c b/hw/m68k/next-kbd.c
> > index bc67810f31..283e98e9eb 100644
> > --- a/hw/m68k/next-kbd.c
> > +++ b/hw/m68k/next-kbd.c
>
>
> > -static const unsigned char next_keycodes[128] = {
> > - 0x00, 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x50, 0x4F,
> > - 0x4E, 0x1E, 0x1F, 0x20, 0x1D, 0x1C, 0x1B, 0x00,
> > - 0x42, 0x43, 0x44, 0x45, 0x48, 0x47, 0x46, 0x06,
> > - 0x07, 0x08, 0x00, 0x00, 0x2A, 0x00, 0x39, 0x3A,
> > - 0x3B, 0x3C, 0x3D, 0x40, 0x3F, 0x3E, 0x2D, 0x2C,
> > - 0x2B, 0x26, 0x00, 0x00, 0x31, 0x32, 0x33, 0x34,
> > - 0x35, 0x37, 0x36, 0x2e, 0x2f, 0x30, 0x00, 0x00,
> > - 0x00, 0x38, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> > - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> > - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> > - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
>
> The original code used '0' (or equivalently 0x00) to
> indicate an unmapped scancode.
>
> > +#define NEXTKBD_NO_KEY 0xff
>
> Now you're using 0xff for missing mappings, but....
>
> > +
> > +static const int qcode_to_nextkbd_keycode[] = {
> > + /* Make sure future additions are automatically set to NEXTKBD_NO_KEY */
> > + [0 ... Q_KEY_CODE__MAX] = NEXTKBD_NO_KEY,
> > +
> > + [Q_KEY_CODE_ESC] = 0x49,
> > + [Q_KEY_CODE_1] = 0x4a,
> > + [Q_KEY_CODE_2] = 0x4b,
> > + [Q_KEY_CODE_3] = 0x4c,
> > + [Q_KEY_CODE_4] = 0x4d,
> > + [Q_KEY_CODE_5] = 0x50,
> > + [Q_KEY_CODE_6] = 0x4f,
> > + [Q_KEY_CODE_7] = 0x4e,
> > + [Q_KEY_CODE_8] = 0x1e,
> > + [Q_KEY_CODE_9] = 0x1f,
> > + [Q_KEY_CODE_0] = 0x20,
> > + [Q_KEY_CODE_MINUS] = 0x1d,
> > + [Q_KEY_CODE_EQUAL] = 0x1c,
> > + [Q_KEY_CODE_BACKSPACE] = 0x1b,
> > + [Q_KEY_CODE_TAB] = 0x00,
>
> ...you've left 0x00 here and....
>
> > +
> > + [Q_KEY_CODE_Q] = 0x42,
> > + [Q_KEY_CODE_W] = 0x43,
> > + [Q_KEY_CODE_E] = 0x44,
> > + [Q_KEY_CODE_R] = 0x45,
> > + [Q_KEY_CODE_T] = 0x48,
> > + [Q_KEY_CODE_Y] = 0x47,
> > + [Q_KEY_CODE_U] = 0x46,
> > + [Q_KEY_CODE_I] = 0x06,
> > + [Q_KEY_CODE_O] = 0x07,
> > + [Q_KEY_CODE_P] = 0x08,
> > + [Q_KEY_CODE_RET] = 0x2a,
> > + [Q_KEY_CODE_A] = 0x39,
> > + [Q_KEY_CODE_S] = 0x3a,
> > +
> > + [Q_KEY_CODE_D] = 0x3b,
> > + [Q_KEY_CODE_F] = 0x3c,
> > + [Q_KEY_CODE_G] = 0x3d,
> > + [Q_KEY_CODE_H] = 0x40,
> > + [Q_KEY_CODE_J] = 0x3f,
> > + [Q_KEY_CODE_K] = 0x3e,
> > + [Q_KEY_CODE_L] = 0x2d,
> > + [Q_KEY_CODE_SEMICOLON] = 0x2c,
> > + [Q_KEY_CODE_APOSTROPHE] = 0x2b,
> > + [Q_KEY_CODE_GRAVE_ACCENT] = 0x26,
> > + [Q_KEY_CODE_SHIFT] = 0x00,
> > + [Q_KEY_CODE_BACKSLASH] = 0x00,
>
> ...here, and ...
>
>
>
> > + [Q_KEY_CODE_Z] = 0x31,
> > + [Q_KEY_CODE_X] = 0x32,
> > + [Q_KEY_CODE_C] = 0x33,
> > + [Q_KEY_CODE_V] = 0x34,
> > +
> > + [Q_KEY_CODE_B] = 0x35,
> > + [Q_KEY_CODE_N] = 0x37,
> > + [Q_KEY_CODE_M] = 0x36,
> > + [Q_KEY_CODE_COMMA] = 0x2e,
> > + [Q_KEY_CODE_DOT] = 0x2f,
> > + [Q_KEY_CODE_SLASH] = 0x30,
> > + [Q_KEY_CODE_SHIFT_R] = 0x00,
>
> ...here, which is surely not a correct conversion
>
> > +
> > + [Q_KEY_CODE_SPC] = 0x38,
> > };
>
>
> Those missing mappings definitely have values too.
>
> I can see the 'shift' and 'tab' keys on the real Next
> keyboard I have in front of me right now :-)
>
> Finding a reliable reference for the NeXT scancodes is very hard
> in my googling so far. The best I've come across so far is
>
> https://github.com/spenczar/usb-next/blob/main/keymap.h
>
> who has defined a mapping to USB HID codes, which seems to
> broadly match what's above here, plus has many of the gaps
> fixed.
>
> Do you know of any other decent references for scancodes ?
I guess the emulator "Previous" is the best and easiest resource:
https://sourceforge.net/p/previous/code/HEAD/tree/trunk/src/keymap.c#l25
HTH,
Thomas
On Mon, Nov 04, 2024 at 10:04:49AM +0000, Daniel P. Berrangé wrote: > On Fri, Nov 01, 2024 at 08:11:05PM +0000, Mark Cave-Ayland wrote: > > Convert the next-kbd device from the legacy UI qemu_add_kbd_event_handler() > > function to use qemu_input_handler_register(). > > > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > > --- > > hw/m68k/next-kbd.c | 166 ++++++++++++++++++++++++++++++--------------- > > 1 file changed, 111 insertions(+), 55 deletions(-) > > > > diff --git a/hw/m68k/next-kbd.c b/hw/m68k/next-kbd.c > > index bc67810f31..283e98e9eb 100644 > > --- a/hw/m68k/next-kbd.c > > +++ b/hw/m68k/next-kbd.c > Finding a reliable reference for the NeXT scancodes is very hard > in my googling so far. The best I've come across so far is > > https://github.com/spenczar/usb-next/blob/main/keymap.h > > who has defined a mapping to USB HID codes, which seems to > broadly match what's above here, plus has many of the gaps > fixed. > > Do you know of any other decent references for scancodes ? > > I'm going to see about adding NeXT scancodes to the giant > database of keycodes at: > > https://gitlab.com/keycodemap/keycodemapdb > > then we can auto-generate this table as we do for most of > the other QEMU keyboard drivers. FYI, I've opened this: https://gitlab.com/keycodemap/keycodemapdb/-/merge_requests/21 if someone wants to sanity-check it, please comment there. Otherwise I'll merge it after a short while if no one points out mistakes. To use this from QEMU we would need: * update ui/keycodemapdb submodule hash to point to the above (once merged) * update ui/meson.build 'keymaps' list to add ['qcode', 'next'] to the generated map list * import the new generated "ui/input-keymap-qcode-to-next.c.inc" from next-kbd.c With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 04/11/2024 11:44, Daniel P. Berrangé wrote: > On Mon, Nov 04, 2024 at 10:04:49AM +0000, Daniel P. Berrangé wrote: >> On Fri, Nov 01, 2024 at 08:11:05PM +0000, Mark Cave-Ayland wrote: >>> Convert the next-kbd device from the legacy UI qemu_add_kbd_event_handler() >>> function to use qemu_input_handler_register(). >>> >>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >>> --- >>> hw/m68k/next-kbd.c | 166 ++++++++++++++++++++++++++++++--------------- >>> 1 file changed, 111 insertions(+), 55 deletions(-) >>> >>> diff --git a/hw/m68k/next-kbd.c b/hw/m68k/next-kbd.c >>> index bc67810f31..283e98e9eb 100644 >>> --- a/hw/m68k/next-kbd.c >>> +++ b/hw/m68k/next-kbd.c > >> Finding a reliable reference for the NeXT scancodes is very hard >> in my googling so far. The best I've come across so far is >> >> https://github.com/spenczar/usb-next/blob/main/keymap.h >> >> who has defined a mapping to USB HID codes, which seems to >> broadly match what's above here, plus has many of the gaps >> fixed. >> >> Do you know of any other decent references for scancodes ? >> >> I'm going to see about adding NeXT scancodes to the giant >> database of keycodes at: >> >> https://gitlab.com/keycodemap/keycodemapdb >> >> then we can auto-generate this table as we do for most of >> the other QEMU keyboard drivers. > > FYI, I've opened this: > > https://gitlab.com/keycodemap/keycodemapdb/-/merge_requests/21 > > if someone wants to sanity-check it, please comment there. Otherwise I'll > merge it after a short while if no one points out mistakes. Nice! > To use this from QEMU we would need: > > * update ui/keycodemapdb submodule hash to point to the above (once > merged) > * update ui/meson.build 'keymaps' list to add ['qcode', 'next'] to > the generated map list > * import the new generated "ui/input-keymap-qcode-to-next.c.inc" from > next-kbd.c What would you recommend would be the best way forward for 9.2? To issue a v4 with just the erroneous 0x00 values removed, or to try and use keycodemapdb? ATB, Mark.
On Mon, Nov 04, 2024 at 10:51:53PM +0000, Mark Cave-Ayland wrote: > On 04/11/2024 11:44, Daniel P. Berrangé wrote: > > > On Mon, Nov 04, 2024 at 10:04:49AM +0000, Daniel P. Berrangé wrote: > > > On Fri, Nov 01, 2024 at 08:11:05PM +0000, Mark Cave-Ayland wrote: > > > > Convert the next-kbd device from the legacy UI qemu_add_kbd_event_handler() > > > > function to use qemu_input_handler_register(). > > > > > > > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > > > > --- > > > > hw/m68k/next-kbd.c | 166 ++++++++++++++++++++++++++++++--------------- > > > > 1 file changed, 111 insertions(+), 55 deletions(-) > > > > > > > > diff --git a/hw/m68k/next-kbd.c b/hw/m68k/next-kbd.c > > > > index bc67810f31..283e98e9eb 100644 > > > > --- a/hw/m68k/next-kbd.c > > > > +++ b/hw/m68k/next-kbd.c > > > Finding a reliable reference for the NeXT scancodes is very hard > > > in my googling so far. The best I've come across so far is > > > > > > https://github.com/spenczar/usb-next/blob/main/keymap.h > > > > > > who has defined a mapping to USB HID codes, which seems to > > > broadly match what's above here, plus has many of the gaps > > > fixed. > > > > > > Do you know of any other decent references for scancodes ? > > > > > > I'm going to see about adding NeXT scancodes to the giant > > > database of keycodes at: > > > > > > https://gitlab.com/keycodemap/keycodemapdb > > > > > > then we can auto-generate this table as we do for most of > > > the other QEMU keyboard drivers. > > > > FYI, I've opened this: > > > > https://gitlab.com/keycodemap/keycodemapdb/-/merge_requests/21 > > > > if someone wants to sanity-check it, please comment there. Otherwise I'll > > merge it after a short while if no one points out mistakes. > > Nice! > > > To use this from QEMU we would need: > > > > * update ui/keycodemapdb submodule hash to point to the above (once > > merged) > > * update ui/meson.build 'keymaps' list to add ['qcode', 'next'] to > > the generated map list > > * import the new generated "ui/input-keymap-qcode-to-next.c.inc" from > > next-kbd.c > > What would you recommend would be the best way forward for 9.2? To issue a > v4 with just the erroneous 0x00 values removed, or to try and use > keycodemapdb? Since we're at soft-freeze, best just todo a simple v4, and we can do keycodemapdb in the next cycle. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Am Fri, 1 Nov 2024 20:11:05 +0000 schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>: > Convert the next-kbd device from the legacy UI qemu_add_kbd_event_handler() > function to use qemu_input_handler_register(). > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/m68k/next-kbd.c | 166 ++++++++++++++++++++++++++++++--------------- > 1 file changed, 111 insertions(+), 55 deletions(-) Reviewed-by: Thomas Huth <huth@tuxfamily.org>
© 2016 - 2026 Red Hat, Inc.