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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 6/11/24 13:00, BALATON Zoltan wrote:
> On Wed, 6 Nov 2024, 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>
>> Reviewed-by: Thomas Huth <huth@tuxfamily.org>
>> ---
>> hw/m68k/next-kbd.c | 163 ++++++++++++++++++++++++++++++---------------
>> 1 file changed, 108 insertions(+), 55 deletions(-)


>> -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
> 
> Now you don't need this 0xff define any more because you can use 0 as no 
> key value then the [0 ... Q_KEY_CODE__MAX] init below can also be 
> dropped because static variables are 0 init automatically.

Whether 0 or 0xff is best for NO_KEY, I don't know.
However, definitions are useful when reviewing ...

> 
> Regards,
> BALATON Zoltan
> 
>> +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,
[...]

>> +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;
>> +    }
>> +
>> +    /* Shift key currently has no keycode, so handle separately */
>> +    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;
>> +        }
>> +    }
>> +
>> +    keycode = qcode_to_nextkbd_keycode[qcode];
>> +    if (keycode == NEXTKBD_NO_KEY) {

... here ^

>> +        return;
>> +    }
>> +
>> +    /* If key release event, create keyboard break code */
>> +    if (!key_down) {
>> +        keycode |= 0x80;
>> +    }
>> +
>> +    nextkbd_put_keycode(s, keycode);
>> +}