qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 4/4] adb.c: add power key support


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH v2 4/4] adb.c: add power key support
Date: Thu, 5 May 2016 16:28:45 +0100

On 24 March 2016 at 14:07, Programmingkid <address@hidden> wrote:
> Add support for the power key. It has to be handled differently from the other
> keys because it is the only 16-bit value key.
>
> Signed-off-by: John Arbuckle <address@hidden>
> ---
>  hw/input/adb.c | 32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/hw/input/adb.c b/hw/input/adb.c
> index 8413db9..fab241b 100644
> --- a/hw/input/adb.c
> +++ b/hw/input/adb.c
> @@ -358,10 +358,17 @@ static int adb_kbd_poll(ADBDevice *d, uint8_t *obuf)
>          s->rptr = 0;
>      }
>      s->count--;
> -    obuf[0] = keycode;
> -    /* NOTE: could put a second keycode if needed */
> -    obuf[1] = 0xff;
> -    olen = 2;
> +    /* The power key is the only two byte value key, so it is a special 
> case. */
> +    if (keycode == 0x7f) {
> +        obuf[0] = 0x7f;
> +        obuf[1] = 0x7f;
> +        olen = 2;
> +    } else {
> +        obuf[0] = keycode;
> +        /* NOTE: could put a second keycode if needed */
> +        obuf[1] = 0xff;

This codepath is also now handling "power key key-up" (which is 0xff 0xff),
so that should have a comment. I suggest changing the NOTE to something like:
 /* NOTE: the power key key-up is the two byte sequence 0xff 0xff; otherwise
  * we could in theory send a second keycode in the second byte, but choose
  * not to bother.
  */

> +        olen = 2;
> +    }
>
>      return olen;
>  }
> @@ -440,11 +447,22 @@ static void adb_keyboard_event(DeviceState *dev, 
> QemuConsole *src,
>      }
>      keycode = qcode_to_adb_keycode[qcode];
>
> -    if (evt->u.key.data->down == false) { /* if key release event */
> -        keycode = keycode | 0x80;   /* create keyboard break code */
> +    /* The power button is a special case because it is a 16-bit value */

useful also to say "Since 0x7f is not a used keycode for ADB we overload
it to indicate the power button when we're storing keycodes in our internal
buffer, and expand it out to two bytes when we send to the guest."

> +    if (qcode == Q_KEY_CODE_POWER) {
> +        if (evt->u.key.data->down == true) { /* Power button pushed keycode 
> */
> +            adb_kbd_put_keycode(s, 0x7f);
> +        } else {                           /* Power button released keycode 
> */
> +            adb_kbd_put_keycode(s, 0xff);
> +        }
>      }
>
> -    adb_kbd_put_keycode(s, keycode);
> +    /* For all non-power keys - safe for 8-bit keycodes */
> +    else {

Your braces here are wrong: the "else" should be on the same line as
the closing } of the preceding if clause (so "} else {".)

> +        if (evt->u.key.data->down == false) { /* if key release event */
> +            keycode = keycode | 0x80;   /* create keyboard break code */
> +        }
> +        adb_kbd_put_keycode(s, keycode);
> +    }
>  }
>
>  static const VMStateDescription vmstate_adb_kbd = {

Otherwise looks good.

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]