[PATCH v3 0/2] next-kbd: convert to use qemu_input_handler_register()

[PATCH v3 0/2] next-kbd: convert to use qemu_input_handler_register()

[Mark Cave-Ayland]

This series converts the next-kbd device to use 
qemu_input_handler_register(), and then removes the now-unused legacy 
qemu_add_kbd_event_handler() function.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

v3:
- Rebase onto master
- Use Q_KEY_CODE__MAX for the size of qcode_to_nextkbd_keycode() array
- Fix shift key logic using example provided by Thomas
- Fix spelling of NEXTKBD_NO_KEY
- Add R-B tag from Alex for patch 2

v2:
- Rebase onto master
- Add patch 2 to remove the legacy qemu_add_kbd_event_handler()
  function


Mark Cave-Ayland (2):
  next-kbd: convert to use qemu_input_handler_register()
  ui/input-legacy.c: remove unused legacy qemu_add_kbd_event_handler()
    function

 hw/m68k/next-kbd.c   | 166 +++++++++++++++++++++++++++++--------------
 include/ui/console.h |   2 -
 ui/input-legacy.c    |  37 ----------
 3 files changed, 111 insertions(+), 94 deletions(-)

-- 
2.39.5

[PATCH v3 1/2] next-kbd: convert to use qemu_input_handler_register()

[Mark Cave-Ayland]

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.5

Re: [PATCH v3 1/2] next-kbd: convert to use qemu_input_handler_register()

[Thomas Huth]

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>

Re: [PATCH v3 1/2] next-kbd: convert to use qemu_input_handler_register()

[Daniel P. Berrangé]

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'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 :|

Re: [PATCH v3 1/2] next-kbd: convert to use qemu_input_handler_register()

[Daniel P. Berrangé]

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 :|

Re: [PATCH v3 1/2] next-kbd: convert to use qemu_input_handler_register()

[Mark Cave-Ayland]

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.

Re: [PATCH v3 1/2] next-kbd: convert to use qemu_input_handler_register()

[Daniel P. Berrangé]

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 :|

Re: [PATCH v3 1/2] next-kbd: convert to use qemu_input_handler_register()

[Thomas Huth]

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

Re: [PATCH v3 1/2] next-kbd: convert to use qemu_input_handler_register()

[Mark Cave-Ayland]

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.

Re: [PATCH v3 1/2] next-kbd: convert to use qemu_input_handler_register()

[Daniel P. Berrangé]

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 :|

[PATCH v3 2/2] ui/input-legacy.c: remove unused legacy qemu_add_kbd_event_handler() function

[Mark Cave-Ayland]

Since the last keyboard device has now been converted over to use
qemu_input_handler_register(), the legacy qemu_add_kbd_event_handler() function
is now unused and can be removed.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/ui/console.h |  2 --
 ui/input-legacy.c    | 37 -------------------------------------
 2 files changed, 39 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 5832d52a8a..46b3128185 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -70,8 +70,6 @@ typedef struct QEMUPutMouseEntry QEMUPutMouseEntry;
 typedef struct QEMUPutKbdEntry QEMUPutKbdEntry;
 typedef struct QEMUPutLEDEntry QEMUPutLEDEntry;
 
-QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func,
-                                            void *opaque);
 QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
                                                 void *opaque, int absolute,
                                                 const char *name);
diff --git a/ui/input-legacy.c b/ui/input-legacy.c
index 210ae5eaca..ca4bccb411 100644
--- a/ui/input-legacy.c
+++ b/ui/input-legacy.c
@@ -109,43 +109,6 @@ void qmp_send_key(KeyValueList *keys, bool has_hold_time, int64_t hold_time,
     g_free(up);
 }
 
-static void legacy_kbd_event(DeviceState *dev, QemuConsole *src,
-                             InputEvent *evt)
-{
-    QEMUPutKbdEntry *entry = (QEMUPutKbdEntry *)dev;
-    int scancodes[3], i, count;
-    InputKeyEvent *key = evt->u.key.data;
-
-    if (!entry || !entry->put_kbd) {
-        return;
-    }
-    count = qemu_input_key_value_to_scancode(key->key,
-                                             key->down,
-                                             scancodes);
-    for (i = 0; i < count; i++) {
-        entry->put_kbd(entry->opaque, scancodes[i]);
-    }
-}
-
-static const QemuInputHandler legacy_kbd_handler = {
-    .name  = "legacy-kbd",
-    .mask  = INPUT_EVENT_MASK_KEY,
-    .event = legacy_kbd_event,
-};
-
-QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque)
-{
-    QEMUPutKbdEntry *entry;
-
-    entry = g_new0(QEMUPutKbdEntry, 1);
-    entry->put_kbd = func;
-    entry->opaque = opaque;
-    entry->s = qemu_input_handler_register((DeviceState *)entry,
-                                           &legacy_kbd_handler);
-    qemu_input_handler_activate(entry->s);
-    return entry;
-}
-
 static void legacy_mouse_event(DeviceState *dev, QemuConsole *src,
                                InputEvent *evt)
 {
-- 
2.39.5

Re: [PATCH v3 2/2] ui/input-legacy.c: remove unused legacy qemu_add_kbd_event_handler() function

[Philippe Mathieu-Daudé]

On 1/11/24 17:11, Mark Cave-Ayland wrote:
> Since the last keyboard device has now been converted over to use
> qemu_input_handler_register(), the legacy qemu_add_kbd_event_handler() function
> is now unused and can be removed.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/ui/console.h |  2 --
>   ui/input-legacy.c    | 37 -------------------------------------
>   2 files changed, 39 deletions(-)

Yay!

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>